From 1fcc10ce74924d72344d157ee6c59a3cf6406b6d Mon Sep 17 00:00:00 2001 From: Eugene Samotija Date: Thu, 20 Aug 2026 07:27:24 -0400 Subject: [PATCH] upstream: recover discovery after a restart, drop stale personal links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- CLAUDE.md | 4 +- packages/gateway/src/http/me-api.ts | 21 ++++--- .../gateway/src/upstream/connection.test.ts | 25 +++++++- packages/gateway/src/upstream/connection.ts | 26 +++++++- packages/gateway/src/upstream/manager.test.ts | 26 ++++++++ packages/gateway/src/upstream/manager.ts | Bin 9032 -> 10794 bytes .../src/upstream/personal-sessions.test.ts | 57 ++++++++++++++++++ 7 files changed, 147 insertions(+), 12 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index cb90d14..e517db4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -20,8 +20,8 @@ Self-hosted MCP manager/gateway: one streamable-HTTP `/mcp` endpoint federating - `domain/policy.ts` — `PolicyService`: toolEnabled ∧ (override(allow) ∨ (tier ≤ maxTier ∧ ¬deny)); maxTier = per-upstream grant ?? role default. Same function gates tools/list AND tools/call. `allowsFor(principal, entry)` = envelope ∧ personal prefs (deny rows in `user_prefs`; "enable" deletes the row — narrowing can never widen). Spec `userDefault: "off"` inverts the personal layer for that upstream: nothing is live until an explicit opt-in row exists (per tool or server-wide `''`), still capped by the envelope — for servers with hundreds of tools. `/api/me/access` derives its `enabled` flags from `allowsFor` so the page can't disagree with the boundary - `auth/` — `static-tokens.ts` (timing-safe bearer match), `oidc.ts` (jose JWKS resource-server verifier for inbound *access* tokens), `login.ts` (interactive login: openid-client cookie+PKCE confidential-client flow consuming an *id-token*; signed identity-only session cookie, HMAC + freshness; `safeReturnTo`), `authz-server.ts` (OAuth AS facade: RFC 8414 metadata, RFC 7591 DCR for public clients, single-use hashed 60s codes + PKCE S256, HS256 gateway JWTs keyed by `GATEWAY_JWT_SECRET` (default derived from `SESSION_SECRET`), rotating refresh tokens — 30d sliding, family-revoked on replay, client-bound consume that can't burn a live token — register rate limit; clients managed via `/api/oauth-clients` + Users tab), `prm.ts` (RFC 9728 doc + WWW-Authenticate; lists the gateway itself as AS when login is configured, else the raw IdP), `directory.ts` (app-only Graph search of Entra users/groups via the login app's own creds — powers the admin UI group-mapping typeahead at `/api/directory/search`; null for non-Entra issuers → UI degrades to paste-an-id), `principal.ts` (session binding key; a Principal carries EVERY role it holds and the envelope is their union — `resolveOidcRoles` returns all mapped groups' roles, an explicit `users.role_id` override replaces them, and `user_login_roles` remembers the group-derived set for the cookie/JWT paths that see no group claims). Four inbound auth paths in `createAuthResolver`: static token, gateway-issued JWT (routed by unverified `iss == PUBLIC_URL`, then fully verified), OIDC bearer, and the cookie session — the cookie/JWT carry only identity and the roles are re-resolved every request (group-derived set persisted at callback via `setLoginRoles`), so a session id never carries privilege. `loginUpsert()` is shared by the bearer + callback paths so they can't drift. `/oauth/authorize` brokers user auth to Entra by piggybacking the interactive login: the pending request rides in the signed transient cookie and `/auth/callback` mints the code. - `secrets/` — `SecretStore` interface (scheme-tagged: `bao` | `kv`), `openbao.ts` (KV v2, AppRole or token, 5-min cache), `keyvault.ts` (Azure Key Vault, `DefaultAzureCredential`, lazy SDK import, same 5-min cache; `put(path, field)` writes `path-field`), `memory.ts` (tests). Refs: `bao:path#field` / `kv:secret-name`; env refs: `${VAR}` — all resolved only at upstream connect time. One store at a time (`BAO_ADDR` xor `KEY_VAULT_URI`) -- `upstream/connection.ts` — one pooled SDK `Client` per upstream; header/env injection; backoff reconnect (1s→60s) + `onRecovered`; retry-once on dropped transport AND on server-side session expiry (upstream 404 "unknown session" → transparent re-initialize + retry, per MCP spec). Optional spec `auth` block (`oauth2-client-credentials`): the gateway mints the upstream's bearer itself (secret via `${VAR}`/`bao:`/`kv:` ref), caches it, and rebuilds the connection when it nears expiry (60s skew) — for third-party servers that want a finished token, e.g. CIPP behind Easy Auth. Neither secret nor token is ever logged -- `upstream/manager.ts` also pools **per-principal links** for `sessionMode:"per-user"` upstreams: spec clone with the caller's credential REFS layered over headers/env (still resolved via the secret store at connect — anti-passthrough intact); catalog discovery stays on the shared link; personal pool flushed on upstream upsert/remove. `requirePersonalCredentials` refuses the shared fallback +- `upstream/connection.ts` — one pooled SDK `Client` per upstream; header/env injection; backoff reconnect (1s→60s) + `onRecovered`; retry-once on dropped transport AND on server-side session expiry (upstream 404 "unknown session" → transparent re-initialize + retry, per MCP spec) — on `callTool` **and** `listTools` (issue #42: discovery had no recovery, so one upstream restart emptied its half of the catalog until a human bounced it). Optional spec `auth` block (`oauth2-client-credentials`): the gateway mints the upstream's bearer itself (secret via `${VAR}`/`bao:`/`kv:` ref), caches it, and rebuilds the connection when it nears expiry (60s skew) — for third-party servers that want a finished token, e.g. CIPP behind Easy Auth. Neither secret nor token is ever logged +- `upstream/manager.ts` also pools **per-principal links** for `sessionMode:"per-user"` upstreams: spec clone with the caller's credential REFS layered over headers/env (still resolved via the secret store at connect — anti-passthrough intact); catalog discovery stays on the shared link; personal pool flushed on upstream upsert/remove AND on a `/me` credential write or delete (issue #44 — the link outlived the credential it was built from). Pool keys go through the single `PERSONAL_KEY_SEP` constant: writer and flusher used different separators, so the flush had never matched anything. A failed discovery is recorded per upstream and surfaced as `lastError`, because `connected: true / toolCount: 0` is indistinguishable from an upstream that genuinely has no tools. `requirePersonalCredentials` refuses the shared fallback - `upstream/manager.ts` — policy-free catalog owner; hot `upsertUpstream`/`removeUpstream`; `summaries()` for the UI - `mcp/gateway-server.ts` — low-level SDK `Server` per session, closes over the Principal; unknown and forbidden tools get the same error (no existence oracle). The ONE exception is a denial the caller owns — `PolicyService.denialReason` returns `personal`/`optIn` only when the admin envelope allows the tool and the user's own layer closes it, and such a tool is already listed on their /me page, so the message names it and points at /me; role denials, kill switches and unknown names keep the vague wording verbatim - `http/app.ts` — `/mcp` (origin check → resolveAuth → principal-bound sessions), PRM endpoints, per-session fingerprint-diffed `list_changed` (fingerprint is per PRINCIPAL — `visibleEntriesFor`, so a user's own `/me` switch notifies too; role-level diffing missed it), mounts `/api` + `/admin`. `http/event-store.ts` gives each session a bounded replay window (`ReplayEventStore`): a notification emitted while the client's GET stream is closed is buffered and replayed on `Last-Event-ID` resume instead of silently dropped by the SDK, and a missing stream is logged since it is otherwise unobservable diff --git a/packages/gateway/src/http/me-api.ts b/packages/gateway/src/http/me-api.ts index 1bce23f..fe884f9 100644 --- a/packages/gateway/src/http/me-api.ts +++ b/packages/gateway/src/http/me-api.ts @@ -257,22 +257,27 @@ export function createMeRouter(deps: AppDeps, me: MeDeps): Router { await secretStore.put(path, body.field, body.value); const ref = secretStore.refFor(path, body.field); repo.upsertUserCredential(prefsIdentity(principal), upstreamId, body.field, ref); + // The caller's pooled per-user link was built with the OLD refs and is + // memoized until the upstream is bounced — drop it here or the credential + // they just rotated keeps being ignored (#44). + const dropped = await manager.closePersonalLink(upstreamId, prefsIdentity(principal)); // Never echo the value. - res.json({ ok: true, ref }); + res.json({ ok: true, ref, reconnected: dropped }); }) ); router.delete( "/credentials/:upstreamId/:field", - h((req, res) => { - const removed = repo.deleteUserCredential( - prefsIdentity(req.principal!), - String(req.params.upstreamId ?? ""), - String(req.params.field ?? "") - ); + h(async (req, res) => { + const who = prefsIdentity(req.principal!); + const upstreamId = String(req.params.upstreamId ?? ""); + const removed = repo.deleteUserCredential(who, upstreamId, String(req.params.field ?? "")); + // Same reason as the write path: the pooled link still holds the ref we + // just deleted, so it has to go too (#44). + const dropped = await manager.closePersonalLink(upstreamId, who); // The secret store copy is left for rotation-history; re-registering // overwrites it. (Store-side cleanup can come with sessionMode.) - res.json({ ok: removed }); + res.json({ ok: removed, reconnected: dropped }); }) ); diff --git a/packages/gateway/src/upstream/connection.test.ts b/packages/gateway/src/upstream/connection.test.ts index 0186cbb..b1ece9d 100644 --- a/packages/gateway/src/upstream/connection.test.ts +++ b/packages/gateway/src/upstream/connection.test.ts @@ -114,9 +114,32 @@ describe("UpstreamConnection — stale streamable-HTTP sessions", () => { expect(connection.connected).toBe(true); }); + it("lists tools over the pooled session", async () => { + expect((await connection.listTools()).map((t) => t.name)).toEqual(["ping"]); + }); + + /** + * Regression for #42: discovery had no stale-session recovery, so after an + * upstream restart listTools() threw, the manager omitted the upstream, and + * every one of its tools disappeared from the catalog until a human bounced it. + */ + it("re-initializes and retries once when DISCOVERY hits an expired session", async () => { + await connection.listTools(); + const initializesBefore = upstream.initializeCount; + + upstream.expireAllSessions(); + const tools = await connection.listTools(); + + expect(tools.map((t) => t.name)).toEqual(["ping"]); + expect(upstream.initializeCount).toBe(initializesBefore + 1); + }); + it("does not re-initialize for genuine tool errors", async () => { + const initializesBefore = upstream.initializeCount; const result = await connection.callTool("does-not-exist", {}); expect(result.isError).toBe(true); // the upstream's error passes through - expect(upstream.initializeCount).toBe(2); // no needless session churn + // No needless session churn — counted relative to this test, so adding + // cases above cannot shift an absolute number. + expect(upstream.initializeCount).toBe(initializesBefore); }); }); diff --git a/packages/gateway/src/upstream/connection.ts b/packages/gateway/src/upstream/connection.ts index bd64b08..565ff69 100644 --- a/packages/gateway/src/upstream/connection.ts +++ b/packages/gateway/src/upstream/connection.ts @@ -256,9 +256,33 @@ export class UpstreamConnection { this.reconnectTimer.unref?.(); } - /** List all tools, following pagination. */ + /** + * List all tools, following pagination. Same stale-session recovery as + * `callTool`: an upstream that restarted answers 404 for our forgotten + * session while the local transport still looks healthy, and without a retry + * discovery would throw, the manager would omit the upstream, and every one of + * its tools would drop out of the catalog until someone bounced it by hand. + */ async listTools(): Promise { const client = await this.requireClient(); + try { + return await this.collectTools(client); + } catch (err) { + if (this.connected && !isStaleSession(err)) { + throw err; // upstream answered with a real error — not a session problem + } + console.error( + this.connected + ? `[upstream:${this.spec.id}] tool discovery hit an expired upstream session — re-initializing` + : `[upstream:${this.spec.id}] tool discovery hit a dropped connection — retrying once` + ); + if (this.connected) await this.resetClient(); + const fresh = await this.requireClient(); + return await this.collectTools(fresh); + } + } + + private async collectTools(client: Client): Promise { const tools: Tool[] = []; let cursor: string | undefined; do { diff --git a/packages/gateway/src/upstream/manager.test.ts b/packages/gateway/src/upstream/manager.test.ts index 028a7db..0e3824a 100644 --- a/packages/gateway/src/upstream/manager.test.ts +++ b/packages/gateway/src/upstream/manager.test.ts @@ -19,6 +19,7 @@ class FakeLink implements UpstreamLink { onRecovered: (() => void) | null = null; connectCalls = 0; failConnect = false; + failDiscovery = false; lastCall: { name: string; args: Record } | null = null; constructor( @@ -32,6 +33,7 @@ class FakeLink implements UpstreamLink { } async listTools(): Promise { + if (this.failDiscovery) throw new Error("Unknown or expired MCP session"); return this.tools; } @@ -99,6 +101,30 @@ describe("UpstreamManager", () => { expect(exposedNames(manager)).toEqual(["demo_echo"]); }); + /** + * #42: discovery can fail while the transport still reports connected, and + * `connected: true, lastError: null, toolCount: 0` reads exactly like "this + * server has no tools" — which is how an emptied catalog went unnoticed. + */ + it("reports WHY an upstream contributed no tools, instead of looking healthy and empty", async () => { + const link = new FakeLink(spec("everything", "demo"), [tool("echo")]); + const { manager } = setup([link]); + await manager.start(); + expect(manager.summaries()[0]!.toolCount).toBe(1); + + link.failDiscovery = true; + await manager.refreshCatalog(); + + const summary = manager.summaries()[0]!; + expect(summary.toolCount).toBe(0); + expect(summary.lastError).toMatch(/tool discovery failed/i); + + // …and the error clears once discovery works again. + link.failDiscovery = false; + await manager.refreshCatalog(); + expect(manager.summaries()[0]!).toMatchObject({ toolCount: 1, lastError: null }); + }); + it("skips disabled upstreams but still reports them in summaries", async () => { const { manager } = setup( [new FakeLink(spec("everything", "demo"), [tool("echo")])], diff --git a/packages/gateway/src/upstream/manager.ts b/packages/gateway/src/upstream/manager.ts index 305889344f00048d7c7caac377056c0bcde949c3..6cc166f96b314d14ec627381b170c3321cc499b5 100644 GIT binary patch delta 3669 zcmaJ^U2Ggz6;|S(*kdPN|NnchcO7}{tk^Ofwl~(n5_O5r5*_qYM zY~00NOrAjPlXc1i52#dqD}ss#o>D;27lZ%_5aO-kClUgQNJT>B0SUfyXJ*&2B39b9 z@11kcJ@=gN`_7%8fBu6z4YSkJ`;9jmG%>yZ<(vB-tnL4Movt3aOm81qq2C`_qvX-E z{E+mctMt92=bP~VtVaJlx}o=6l}#&mS@(6z*-q1bLvJ4@{YpuS zyCu(erCz+_IF^^DA2-D5lZMqJ8l@X=(pMV=z1tY4pEOR>-y2U-tZ9U1ngp#k-J>WQ6#X$4{Bt+H1@ zT)Jx%V8yBC(;mtw=ZC|i-*j|eo)R8b+rCv1Mcr4+uyFwp$-rh-Q|A{O%eQXdSbypI z>#NJJyncImL!1|hSIcwwB(fTP*nGMZ05H7}8opbWY5J`BuBC^*)7nU{9-E*?$G$iaIG)(`q{l^Cm6hsd2C*o^{~nve-*Z%KIZf}k zEHEe)!~_oXr9uVutRq96+}tYr$c61FfFk3zN-856HORZtD`dnK(=!}ZA)pN|;ViG4 z)`8Ueu5JTHpseScmW8D}@b);pwggafX@w<}(H5JvNmP)N5x6|S9wtrtAZ6w@kv{+> zXTra|T{gV}b5P`6rzjFAh=;nm2vGoRfjd6ZG7#e>#Gk+R4##Q~*h#_JLD?VbHheIG z17#804p3HRE|~_VQ_02oDbyHeRhdm==O72FiXKXA%N?<%m(B%}0oW+%xR9F4(jQuG z)2Y_Em<9(5B5v6LyHjMh4ny_?z0lH)qMoNuT9eHfPod7O+5%KiK$5O#v*c^ZL5u*K zyhx3EC23?r*_Sz>pLI>4o?GM|NVIY5m<^GBBZe_vefZSyy3tWm`JbtfT%>%+J z>i3Udqo>Eu(Ww)wLF8POj4QWMGU=xH2*?cF%_w{P=w8J(5O@*TinKhXeAbmXO;1j| z*o(=jC9#2Go1Q%P&@potkOk2;OzRz5ELeg(v0id|?stsRH#%P6%yKe*+A(=j{i`B6 zRLD$DTGE#(Gl%H@)-g+iF-04Q^8xN$ah;O5vA$fF(MeC~5ul<}nw7K8Ev2!*y6FNX zZ?M*a6RMFxIyn>&h^Ym-$v~}kAbKoz7HpxmQ3e%DcQRrl><(&QWfC|n zB9G=*V6|1La~$e_svdK|L8x?&^_UeMhzDPp@`00}ZS3hqsJEqZG{Uf$WiOvbS+L@wKXZC~&o9V{kSLvfp5mNO@=N&rR zH9b;4S4cmFGo+k(@u=%8J?;_#VX!_7tV&P2MnXSb-7|ElJ59yzOe73GI`c04(#4VC zp)dd;A{xnDri8Yd+NHmAPf$zGG%fT@ao-6((H=bdfAr%jWR%P?5yRzGR%fVo3YvrX zYfhi*$dkSj0QS;rWK7_7lP_JhNGo)HbVw=Rw|boVLF&6H|Ipvb-V2oP)lvFi^iBpD zRXawl;yiAE(0b`*siW`ZFmbQ+CFsq*GvVlQ-)Z`^FA$oDR zow46r{qyvr{&;Boq(4b51BDL2e zi71~@Vxf5T?uu46m!;0Z1zH|liW~(9tAVzjdnL`>j*wvba4<=~!}qhnIQ0)L-m1kn zJB!O|1d)omEtH{UXurYD0DOVk*-#Yx@ZGL{fv_Cs0bewA%P&+ibbV-IPh7nCeDH8W zBhavsx;NBDbsB-5pz{_r|U%*}}C`u%Pv+U9dKOT_owd5QSfg4H_L^fv_g& zt#OUMIewG=I=)H^iAjD@t09O9&glo<{e&1$kMtcKFCt-E&!7mYd-VOpMS7aJ8i>zp JAw{a%{{WcWcG3U< delta 2121 zcmZ8iOKcle6s==B*d}8qb{v0WKmTHn6`YW?g*v}NS|AC5LOxxTlsNVz8R5)WGvlC; zT2R!i5JKUJ6%rB>jTD5$js-{&Y*02tR01SKQChL3LTVRmXv@8C#vbVA&b<5X_uS9> zeE#q`+dw|g?OI>9b1niu3$IwX-Mv{L_g=Da^cl+}&})_{pg&lEO~Y+xbWyt4HUsBR z+r}KII>5O&`bXOZU=3S`rZ|_lQIi#oY{Mm4sf*RhWJxPk<+XlfXj?Ub7>x6hrfiwq zg_^1|y%0tsVY$YH;9~l_LXCH+Yca=q(2({?IWBbi}#X zeImWB3aTo~(u}axST0~i=)UtDT;Fk`5brt#^s;LhXm`gP(6x@iAuy#Q1_{^8nylpT zOo&<&%4(4`i!lesOG~UsA36i{X2+SwbvZ&L;Z1tO>T~B%g-0$AMW^H3F!lvu8EtX{ z;O{z@is)M6D#;~KzF@${RBm$@15K-npw$%}M6^NQ@dv1s>!7k5t6q0!fZpZ;wC3^C z@7;ZH`qQ0s>e>w`o8OZF-f2e1A_4lPJ3x0lF&j$IJMM(H=?H{b!r|^)v(I(MCpm5s*c%dr*%K0j-MC8;B!I2S#jNR13&c3l*U% zXhKe`K&W)r`y|Vf!FuS;LU`RiEYGCxKNX`rF97Seo}}%>GAMZiJRC;?CxOa=G)xRO z2~F^TY@h>}-vmawnw|j)yahxXa32XeO;q{dJP0>}`M55_8i7Ts2&;;quEWr=9O%Pf zklqa9wEhz$Y#f+P9c*U2KNK|$&xW1=(YHeVK=(ogcQY1PEr`~wPz){}gmB`6;WW_E zFs_W%Fy9gdqlnK@xCbtvXMoN}WFt^?-t>*$jSOKBe~*lVWF%T*`>7GlvK?gLU)FaX z8H4=*8%0pT!MD+8&A1=yO@cPx`;^HNdT~(i_lD?E+~G8v>$_fTtNXoqQ?x%e1gy)k zxXF4qmIBuG*h#(BuljrF-Ixzp_IQD<9n7S)BXKjWmZ~bQm)SVB>S{dJ8f_e;_ZfFD zj%(xBc+u#$76b&^7$?>2J7a(1aElc%J3%m@6-IXm+tvizD4$5{3q=n|Yi>ifWRTaI zOdWR|FhE_OopPyK#Rnp%7XrcEDn9a{&>s`%`CfuOWJ&bAoRo}KVvX0F7OZgXvgLs}~q3c)}-@zMjVC@{2TXfxp@R zxeWe)keO!HVhj2>6AbJ6b>rEaYCS%Hou?0Nyw-F5wqi#cr?@C(alHv R1kj)O=MA8iT2%qa{sl^f2K4{{ diff --git a/packages/gateway/src/upstream/personal-sessions.test.ts b/packages/gateway/src/upstream/personal-sessions.test.ts index ecb83ed..898f030 100644 --- a/packages/gateway/src/upstream/personal-sessions.test.ts +++ b/packages/gateway/src/upstream/personal-sessions.test.ts @@ -197,6 +197,63 @@ describe("per-user upstream sessions", () => { expect(after).toMatch(/^who via bao:gw-user-/); }); + /** + * Regression for #44: the personal link was memoized per principal and only + * ever flushed by the upstream lifecycle, so a credential the user rotated on + * /me kept being ignored — silently — until someone bounced the upstream. + */ + it("a credential write drops that caller's pooled link so the new value is picked up", async () => { + await putCredential("tok-alice", "peruser", "first-secret"); + await callTool("tok-alice", "peruser_who"); // builds + pools the link + const before = linksCreated.length; + + const rotated = await fetch(`${base}/api/me/credentials/peruser`, { + method: "PUT", + headers: { "Content-Type": "application/json", Authorization: "Bearer tok-alice" }, + body: JSON.stringify({ field: "Authorization", value: "rotated-secret" }), + }); + expect((await rotated.json()) as { reconnected: boolean }).toMatchObject({ reconnected: true }); + + await callTool("tok-alice", "peruser_who"); + expect(linksCreated.length).toBeGreaterThan(before); // rebuilt, not reused + }); + + it("only the writer's link is dropped — other principals keep theirs", async () => { + await putCredential("tok-bob", "peruser", "bob-secret"); + await callTool("tok-bob", "peruser_who"); + await callTool("tok-alice", "peruser_who"); + const before = linksCreated.length; + + await putCredential("tok-alice", "peruser", "alice-again"); + await callTool("tok-bob", "peruser_who"); // bob's link untouched + + expect(linksCreated.length).toBe(before); + }); + + it("a credential delete drops the link too, so the stale ref cannot linger", async () => { + await putCredential("tok-alice", "peruser", "to-be-deleted"); + await callTool("tok-alice", "peruser_who"); + const before = linksCreated.length; + + const removed = await fetch(`${base}/api/me/credentials/peruser/Authorization`, { + method: "DELETE", + headers: { Authorization: "Bearer tok-alice" }, + }); + expect((await removed.json()) as { ok: boolean; reconnected: boolean }).toMatchObject({ + ok: true, + reconnected: true, + }); + + // No creds left → the call falls back to the SHARED link (this upstream + // allows it), so the caller now travels on the gateway's own credential + // rather than on a personal ref that no longer exists. + const text = await callTool("tok-alice", "peruser_who"); + expect(text).toContain("bao:upstreams/peruser"); + expect(text).not.toContain("gw-user-"); + // The shared link already existed, so nothing new had to be built. + expect(linksCreated.length).toBe(before); + }); + it("upstream removal closes and forgets personal links", async () => { const before = linksCreated.length; await manager.upsertUpstream(perUserSpec); // upsert = remove + re-register