Skip to content

upstream: recover discovery after a restart, drop stale personal links - #46

Merged
selic merged 1 commit into
mainfrom
fix/upstream-recovery-and-personal-links
Aug 20, 2026
Merged

upstream: recover discovery after a restart, drop stale personal links#46
selic merged 1 commit into
mainfrom
fix/upstream-recovery-and-personal-links

Conversation

@selic

@selic selic commented Aug 20, 2026

Copy link
Copy Markdown
Member

Fixes #42, fixes #44 — two ways the gateway kept serving state that no longer existed.

#42 — a restarted upstream lost its tools permanently

connection.ts already had isStaleSession() and the retry-once dance, but only on callTool(). listTools() had neither, so when an upstream restarted and 404'd our forgotten session, discovery threw, doRefresh omitted that upstream, and every one of its tools left the catalog — with no self-recovery, because the local transport still looked healthy and connect() kept succeeding. Discovery now recovers exactly the way a call does.

The other half of the report was that this was invisible: connected: true / lastError: null / toolCount: 0 reads exactly like "this server has no tools". The manager now records why an upstream contributed nothing and surfaces it as lastError, clearing it when discovery works again.

#44 — rotated credentials kept being ignored

Personal per-user links are memoized per principal and were only flushed by the upstream lifecycle, so a credential saved on /me had no effect until someone bounced the upstream. The credential write and delete paths now drop that caller's own link — and only theirs — and return reconnected so the UI can say what happened.

Found while fixing it: the flush was dead code

The pool was keyed ${upstreamId} ${sessionKey} (a space) while closePersonalLinks() searched for a control-character prefix, so it had never matched anything. The existing "upstream removal closes and forgets personal links" test passed for the wrong reason: upsertUpstream rebuilds the shared link and the assertion counted factory calls.

Both sites now go through a single PERSONAL_KEY_SEP constant, and manager.ts is a text file again — the raw NUL bytes that made this impossible to see (git and grep both treated it as binary) are gone.

Tests

  • connection.test.ts — discovery re-initializes exactly once against the fake upstream that expires sessions. Verified to fail without the fix.
  • manager.test.ts — a discovery failure is reported as lastError and cleared on the next good refresh.
  • personal-sessions.test.ts — a credential write rebuilds only the writer's link; another principal's link survives; a delete drops the link and the next call falls back to the shared credential rather than a ref that no longer exists.
  • The pre-existing "no needless session churn" assertion is now relative, so cases added above it can't shift an absolute count.

npm run build clean, npm test 288 passed. Not deployed.

🤖 Generated with Claude Code

Fixes #42 and #44 — two ways the gateway kept serving a state that no longer
existed.

#42: connection.ts already had isStaleSession() and the retry-once dance, but
only on callTool(). listTools() had neither, so when an upstream restarted and
404'd our forgotten session, discovery threw, the manager omitted that upstream,
and every one of its tools left the catalog until somebody bounced it by hand.
Discovery now recovers the same way a call does.

The second half of #42 was that this was invisible: the transport still reported
connected, so health showed connected:true / lastError:null / toolCount:0, which
reads exactly like "this server has no tools". The manager now records why an
upstream contributed nothing and surfaces it as lastError, clearing it when
discovery succeeds again.

#44: personal per-user links are memoized per principal and were only ever
flushed by the upstream lifecycle, so a credential rotated on /me kept being
ignored — silently — until the upstream was bounced. The /me credential write
and delete paths now drop that caller's own link (and only theirs), and report
`reconnected` so the UI can say so.

While fixing #44 the flush turned out to be dead code: the pool was keyed with
`${upstreamId} ${sessionKey}` (space) while closePersonalLinks searched for a
control-character prefix, so it had never matched anything — the existing
"upstream removal forgets personal links" test passed only because upsert
rebuilds the shared link and the assertion counted factory calls. Both sites now
go through one PERSONAL_KEY_SEP constant, and manager.ts is text again (the raw
NUL bytes that hid this made the file binary to git and grep).

Tests: discovery re-initializes exactly once against the fake upstream that
expires sessions (fails without the fix); a discovery failure is reported and
then cleared; a credential write rebuilds only the writer's link, another
principal's link survives, and a delete drops the link and falls back to the
shared credential. The pre-existing "no needless session churn" assertion is now
relative, so adding cases above it can't shift an absolute count.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@selic
selic merged commit f368ca3 into main Aug 20, 2026
1 check passed
@selic
selic deleted the fix/upstream-recovery-and-personal-links branch August 20, 2026 11:31
selic pushed a commit that referenced this pull request Aug 20, 2026
Minor: admins can forget a user - row, prefs, credential refs, refresh tokens
and live sessions in one call (#41); tool discovery recovers from an upstream
restart instead of silently emptying that upstream's half of the catalog, and
a failed discovery is surfaced as lastError rather than looking healthy (#42,
#46); a personal credential written or deleted on /me drops the caller's pooled
per-user link so the change takes effect immediately (#44, #46).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant