diff --git a/apps/server/src/peer/PeerLinks.test.ts b/apps/server/src/peer/PeerLinks.test.ts index f35e760bea4d7..c06beac0b6757 100644 --- a/apps/server/src/peer/PeerLinks.test.ts +++ b/apps/server/src/peer/PeerLinks.test.ts @@ -575,18 +575,27 @@ it.effect("tells the agent to link again once the peer revokes the link or it ex const pairing = yield* b.auth.issuePairingCredential(); yield* a.links.link({ url: b.url, pairingCode: pairing.credential, access: "auto" }); yield* probe(a); + const before = yield* a.links.resolve(box.environmentId); const [session] = yield* linkedSessions(b); yield* b.auth.revokeSession(session!.sessionId); const revoked = yield* probe(a).pipe(Effect.flip); expect(revoked.code).toBe("capability_denied"); expect(revoked.message).toContain("revoked there"); + // The peer refuses this link's token, so listing must not call it usable. + expect((yield* a.links.list).map((link) => link.status)).not.toContain("reachable"); yield* a.links.link({ url: b.url, pairingCode: (yield* b.auth.issuePairingCredential()).credential, access: "auto", }); + // A refusal of the old token that arrives late leaves the new link alone. + yield* a.links.recordOutcome(box.environmentId, { + error: revoked.message, + refusedSecret: before.secretName, + }); + expect((yield* a.links.list).map((link) => link.status)).toEqual(["reachable"]); // A link made with a peer that later answers without the fence, as // one made before the fence or a peer moved back to such a version. yield* Ref.set(b.descriptor, descriptorOf("environment-box", "Box", { linkFence: false })); diff --git a/apps/server/src/peer/PeerLinks.ts b/apps/server/src/peer/PeerLinks.ts index 48f61e6554a17..4401a8c332eec 100644 --- a/apps/server/src/peer/PeerLinks.ts +++ b/apps/server/src/peer/PeerLinks.ts @@ -78,6 +78,8 @@ export interface ResolvedPeerLink { /** The first address that answered as this peer. */ readonly url: string; readonly token: string; + /** Names the stored token, so an outcome for it never lands on a later link. */ + readonly secretName: string; } export const LINKED_ENVIRONMENTS_OFF = @@ -106,10 +108,14 @@ export class PeerLinks extends Context.Service< readonly resolve: ( environmentId: EnvironmentId, ) => Effect.Effect; - /** Records the outcome of a call, so listing shows whether the peer answered last time. */ + /** + * Records the outcome of a call, so listing shows whether the peer answered last time. + * `refusedSecret` names a token the peer no longer accepts; while it is still the link's, + * only linking again helps. + */ readonly recordOutcome: ( environmentId: EnvironmentId, - outcome: { readonly error: string | null }, + outcome: { readonly error: string | null; readonly refusedSecret?: string }, ) => Effect.Effect; } >()("t3/peer/PeerLinks") {} @@ -673,6 +679,7 @@ const make = Effect.gen(function* () { link: yield* withLabel(link, label), url: url.value, token: new TextDecoder().decode(stored.value), + secretName: row.value.secret_name, }; }, ); @@ -686,10 +693,16 @@ const make = Effect.gen(function* () { SET last_reached_at = ${DateTime.formatIso(now)}, last_error = NULL WHERE environment_id = ${environmentId} ` - : sql` - UPDATE peer_environment_links SET last_error = ${outcome.error} - WHERE environment_id = ${environmentId} - `, + : outcome.refusedSecret !== undefined + ? sql` + UPDATE peer_environment_links + SET last_error = ${outcome.error}, expires_at = ${DateTime.formatIso(now)} + WHERE environment_id = ${environmentId} AND secret_name = ${outcome.refusedSecret} + ` + : sql` + UPDATE peer_environment_links SET last_error = ${outcome.error} + WHERE environment_id = ${environmentId} + `, ), Effect.asVoid, Effect.ignoreCause({ log: true }), diff --git a/apps/server/src/peer/PeerMcpClient.test.ts b/apps/server/src/peer/PeerMcpClient.test.ts index ef8a3aeff6a65..d11d58620def8 100644 --- a/apps/server/src/peer/PeerMcpClient.test.ts +++ b/apps/server/src/peer/PeerMcpClient.test.ts @@ -253,6 +253,7 @@ it.effect("renews a missing peer session once and reuses its replacement", () => }, url: "https://peer.example", token: "fixture", + secretName: "peer-link-fixture", }), recordOutcome: (_, outcome) => Effect.sync(() => outcomes.push(outcome.error)), }), diff --git a/apps/server/src/peer/PeerMcpClient.ts b/apps/server/src/peer/PeerMcpClient.ts index e800333fb1e08..0dfc0bdfcc1ae 100644 --- a/apps/server/src/peer/PeerMcpClient.ts +++ b/apps/server/src/peer/PeerMcpClient.ts @@ -90,6 +90,13 @@ const linkedEnvironmentsOffThere = new OrchestratorMcpFailure({ "The linked environment has Linked environments turned off. Turn it on in its Settings → Labs.", }); +/** The peer turned away the link's token: it was revoked or expired there. */ +const linkRefused = new OrchestratorMcpFailure({ + code: "capability_denied", + message: + "The linked environment no longer accepts this link. It may have been revoked there; link it again.", +}); + /** The body of an MCP response: JSON, or a single server-sent event carrying JSON. */ const responseJson = (contentType: string | undefined, text: string) => contentType?.includes("text/event-stream") === true @@ -119,11 +126,20 @@ const make = Effect.gen(function* () { ), ); + /** Ends the link only while the refused token is still its own, not one a newer link replaced. */ + const refuse = (resolved: PeerLinks.ResolvedPeerLink) => + links + .recordOutcome(resolved.link.environmentId, { + error: linkRefused.message, + refusedSecret: resolved.secretName, + }) + .pipe(Effect.andThen(Effect.fail(linkRefused))); + const openSession = Effect.fn("PeerMcpClient.openSession")(function* ( - url: string, - token: string, + resolved: PeerLinks.ResolvedPeerLink, limitHeader: Record, ) { + const { url, token } = resolved; const initialized = yield* post( url, token, @@ -140,14 +156,12 @@ const make = Effect.gen(function* () { limitHeader, ); const sessionId = initialized.headers["mcp-session-id"]; + if (initialized.status === 401) return yield* refuse(resolved); if (initialized.status === 403) return yield* linkedEnvironmentsOffThere; if (initialized.status !== 200 || sessionId === undefined) { return yield* new OrchestratorMcpFailure({ - code: initialized.status === 401 ? "capability_denied" : "orchestration_error", - message: - initialized.status === 401 - ? "The linked environment no longer accepts this link. It may have been revoked there; link it again." - : `The linked environment refused to start an MCP session (HTTP ${initialized.status}).`, + code: "orchestration_error", + message: `The linked environment refused to start an MCP session (HTTP ${initialized.status}).`, }); } yield* post( @@ -200,24 +214,18 @@ const make = Effect.gen(function* () { }, ); const existing = (yield* Ref.get(sessions)).get(sessionKey); - const sessionId = existing ?? (yield* openSession(resolved.url, resolved.token, limitHeader)); + const sessionId = existing ?? (yield* openSession(resolved, limitHeader)); // The initialized session outlives a tool request, including a long wait or cancellation. if (existing === undefined) { yield* Ref.update(sessions, (current) => new Map(current).set(sessionKey, sessionId)); } let response = yield* callOnce(sessionId); if (response.status === 404) { - const renewed = yield* openSession(resolved.url, resolved.token, limitHeader); + const renewed = yield* openSession(resolved, limitHeader); yield* Ref.update(sessions, (current) => new Map(current).set(sessionKey, renewed)); response = yield* callOnce(renewed); } - if (response.status === 401) { - return yield* new OrchestratorMcpFailure({ - code: "capability_denied", - message: - "The linked environment no longer accepts this link. It may have been revoked there; link it again.", - }); - } + if (response.status === 401) return yield* refuse(resolved); if (response.status === 403) return yield* linkedEnvironmentsOffThere; if (response.status !== 200) { return yield* new OrchestratorMcpFailure({