From 8f2d6f0ac09e6f4630a7346bfea42aa20f3bd777 Mon Sep 17 00:00:00 2001 From: Will Sheldon Date: Sun, 11 Oct 2026 16:37:07 -0700 Subject: [PATCH 1/2] fix(server): restore MCP credentials when idle sessions resume --- .../server/src/mcp/McpSessionRegistry.test.ts | 101 ++++++++++++++++++ apps/server/src/mcp/McpSessionRegistry.ts | 29 +++-- .../orchestration-v2/RunExecutionService.ts | 3 +- 3 files changed, 115 insertions(+), 18 deletions(-) diff --git a/apps/server/src/mcp/McpSessionRegistry.test.ts b/apps/server/src/mcp/McpSessionRegistry.test.ts index a3f74c3e28c2..09f609feed1c 100644 --- a/apps/server/src/mcp/McpSessionRegistry.test.ts +++ b/apps/server/src/mcp/McpSessionRegistry.test.ts @@ -178,3 +178,104 @@ it.effect("does not keep credentials of other threads alive", () => expect(yield* registry.resolve(token)).toBeUndefined(); }), ); + +it.effect( + "restores the existing credential when its provider resumes after the liveness window", + () => + Effect.gen(function* () { + let timestamp = 1_000; + const registry = yield* makeRegistry(() => timestamp); + const threadId = ThreadId.make("thread-resumed"); + const issued = yield* registry.issue({ + threadId, + providerInstanceId: ProviderInstanceId.make("codex"), + capabilities: new Set(["preview"]), + }); + const token = issued.config.authorizationHeader.replace(/^Bearer\s+/, ""); + + timestamp += 101; + yield* registry.touch(threadId); + + expect(yield* registry.resolve(token)).toMatchObject({ + thread: { + threadId, + providerSessionId: issued.config.providerSessionId, + providerInstanceId: ProviderInstanceId.make("codex"), + }, + capabilities: new Set(["preview", "orchestration", "worktree", "pull-requests"]), + issuedAt: 1_000, + }); + + timestamp += 101; + expect(yield* registry.resolve(token)).toBeUndefined(); + }), +); + +it.effect( + "keeps an expired credential recoverable across rejected requests and other thread activity", + () => + Effect.gen(function* () { + let timestamp = 1_000; + const registry = yield* makeRegistry(() => timestamp); + const threadId = ThreadId.make("thread-idle"); + const issued = yield* registry.issue({ + threadId, + providerInstanceId: ProviderInstanceId.make("claude"), + capabilities: new Set(), + }); + const token = issued.config.authorizationHeader.replace(/^Bearer\s+/, ""); + + timestamp += 101; + expect(yield* registry.resolve(token)).toBeUndefined(); + const otherThreadId = ThreadId.make("thread-active"); + const other = yield* registry.issue({ + threadId: otherThreadId, + providerInstanceId: ProviderInstanceId.make("codex"), + capabilities: new Set(["device"]), + }); + yield* registry.touch(otherThreadId); + expect( + (yield* registry.resolve(other.config.authorizationHeader.replace(/^Bearer\s+/, ""))) + ?.thread.threadId, + ).toBe(otherThreadId); + expect(yield* registry.resolve("unknown-token")).toBeUndefined(); + expect(yield* registry.resolve(token)).toBeUndefined(); + + timestamp += 101; + expect(yield* registry.resolve(token)).toBeUndefined(); + yield* registry.touch(threadId); + expect((yield* registry.resolve(token))?.thread.providerSessionId).toBe( + issued.config.providerSessionId, + ); + expect( + yield* registry.resolve(other.config.authorizationHeader.replace(/^Bearer\s+/, "")), + ).toBeUndefined(); + }), +); + +it.effect("never restores explicitly revoked credentials on later provider turns", () => + Effect.gen(function* () { + for (const revocation of ["provider-session", "thread", "all"] as const) { + let timestamp = 1_000; + const registry = yield* makeRegistry(() => timestamp); + const threadId = ThreadId.make(`thread-revoked-${revocation}`); + const issued = yield* registry.issue({ + threadId, + providerInstanceId: ProviderInstanceId.make("codex"), + capabilities: new Set(["preview"]), + }); + const token = issued.config.authorizationHeader.replace(/^Bearer\s+/, ""); + + timestamp += 101; + if (revocation === "provider-session") { + yield* registry.revokeProviderSession(issued.config.providerSessionId); + } else if (revocation === "thread") { + yield* registry.revokeThread(threadId); + } else { + yield* registry.revokeAll; + } + yield* registry.touch(threadId); + expect(yield* registry.resolve(token)).toBeUndefined(); + } + }), +); diff --git a/apps/server/src/mcp/McpSessionRegistry.ts b/apps/server/src/mcp/McpSessionRegistry.ts index b16529924a43..67cfd570528a 100644 --- a/apps/server/src/mcp/McpSessionRegistry.ts +++ b/apps/server/src/mcp/McpSessionRegistry.ts @@ -60,6 +60,10 @@ export interface McpSessionRegistryOptions { } /** + * Credentials stop authenticating after this long without a sign of life. + * Expired ones cannot refresh themselves through MCP traffic, but stay registered so a provider turn's `touch` can restore the + * token its subprocess still holds. `stopSession` and `stopAll` revoke eagerly. + * * The bound matters because `/mcp` is mounted outside the environment auth * stack and is reachable on whatever host the server binds to, so this token is * the only thing guarding the `t3-code` toolkits on a remote-reachable server. @@ -95,15 +99,6 @@ const makeWithOptions = Effect.fn("McpSessionRegistry.make")(function* ( .digest("SHA-256", new TextEncoder().encode(token)) .pipe(Effect.map(bytesToHex), Effect.orDie); - const pruneDead = (records: ReadonlyMap, timestamp: number) => { - const next = new Map( - Array.from(records).filter( - ([, record]) => timestamp - record.lastAliveAt <= livenessWindowMs, - ), - ); - return next.size === records.size ? records : next; - }; - const issue: McpSessionRegistryShape["issue"] = Effect.fn("McpSessionRegistry.issue")( function* (request) { const issuedAt = yield* currentTimeMillis; @@ -129,7 +124,7 @@ const makeWithOptions = Effect.fn("McpSessionRegistry.make")(function* ( issuedAt, }; yield* SynchronizedRef.update(state, ({ records }) => { - const next = new Map(pruneDead(records, issuedAt)); + const next = new Map(records); next.set(tokenHash, { tokenHash, scope, lastAliveAt: issuedAt }); return { records: next }; }); @@ -154,10 +149,11 @@ const makeWithOptions = Effect.fn("McpSessionRegistry.make")(function* ( const tokenHash = yield* hashToken(rawToken); const timestamp = yield* currentTimeMillis; return yield* SynchronizedRef.modify(state, ({ records }) => { - const current = pruneDead(records, timestamp); - const record = current.get(tokenHash); - if (!record) return [undefined, { records: current }] as const; - const next = new Map(current); + const record = records.get(tokenHash); + if (!record || timestamp - record.lastAliveAt > livenessWindowMs) { + return [undefined, { records }] as const; + } + const next = new Map(records); next.set(tokenHash, { ...record, lastAliveAt: timestamp }); return [record.scope, { records: next }] as const; }); @@ -168,9 +164,8 @@ const makeWithOptions = Effect.fn("McpSessionRegistry.make")(function* ( function* (threadId) { const timestamp = yield* currentTimeMillis; yield* SynchronizedRef.update(state, ({ records }) => { - const current = pruneDead(records, timestamp); - const next = new Map(current); - for (const [tokenHash, record] of current) { + const next = new Map(records); + for (const [tokenHash, record] of records) { if (record.scope.thread.threadId === threadId) { next.set(tokenHash, { ...record, lastAliveAt: timestamp }); } diff --git a/apps/server/src/orchestration-v2/RunExecutionService.ts b/apps/server/src/orchestration-v2/RunExecutionService.ts index 61ab0a3f0954..0ed10751489c 100644 --- a/apps/server/src/orchestration-v2/RunExecutionService.ts +++ b/apps/server/src/orchestration-v2/RunExecutionService.ts @@ -1359,7 +1359,8 @@ export const layer: Layer.Layer< // A provider turn is a sign that the session is still alive. Keep // its already-issued MCP credential valid even when the agent goes - // a long time between browser-tool calls. + // a long time between browser-tool calls, and restore it if the + // thread sat idle past the liveness window. yield* McpSessionRegistry.touchActiveMcpThread(input.run.threadId); const appContext = (yield* mcpAppModelContext .forThread(input.run.threadId) From 662bd0df976920a47018ddde3709059111ff2ff5 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 02:36:55 +0000 Subject: [PATCH 2/2] fix(server): retain renewable MCP credentials with focused tests --- .../server/src/mcp/McpSessionRegistry.test.ts | 125 ++++++------------ apps/server/src/mcp/McpSessionRegistry.ts | 10 +- .../orchestration-v2/RunExecutionService.ts | 3 +- 3 files changed, 47 insertions(+), 91 deletions(-) diff --git a/apps/server/src/mcp/McpSessionRegistry.test.ts b/apps/server/src/mcp/McpSessionRegistry.test.ts index 09f609feed1c..94f6aa677799 100644 --- a/apps/server/src/mcp/McpSessionRegistry.test.ts +++ b/apps/server/src/mcp/McpSessionRegistry.test.ts @@ -179,86 +179,50 @@ it.effect("does not keep credentials of other threads alive", () => }), ); -it.effect( - "restores the existing credential when its provider resumes after the liveness window", - () => - Effect.gen(function* () { - let timestamp = 1_000; - const registry = yield* makeRegistry(() => timestamp); - const threadId = ThreadId.make("thread-resumed"); - const issued = yield* registry.issue({ - threadId, - providerInstanceId: ProviderInstanceId.make("codex"), - capabilities: new Set(["preview"]), - }); - const token = issued.config.authorizationHeader.replace(/^Bearer\s+/, ""); - - timestamp += 101; - yield* registry.touch(threadId); +it.effect("only a trusted touch restores an expired token with its original scope", () => + Effect.gen(function* () { + let timestamp = 1_000; + const registry = yield* makeRegistry(() => timestamp); + const threadId = ThreadId.make("thread-idle"); + const issued = yield* registry.issue({ + threadId, + providerInstanceId: ProviderInstanceId.make("claude"), + capabilities: new Set(["preview"]), + }); + const token = issued.config.authorizationHeader.replace(/^Bearer\s+/, ""); + const scope = yield* registry.resolve(token); + expect(scope).toBeDefined(); - expect(yield* registry.resolve(token)).toMatchObject({ - thread: { - threadId, - providerSessionId: issued.config.providerSessionId, - providerInstanceId: ProviderInstanceId.make("codex"), - }, - capabilities: new Set(["preview", "orchestration", "worktree", "pull-requests"]), - issuedAt: 1_000, - }); + timestamp += 101; + expect(yield* registry.resolve(token)).toBeUndefined(); + const otherThreadId = ThreadId.make("thread-active"); + const other = yield* registry.issue({ + threadId: otherThreadId, + providerInstanceId: ProviderInstanceId.make("codex"), + capabilities: new Set(["device"]), + }); + yield* registry.touch(otherThreadId); + expect( + (yield* registry.resolve(other.config.authorizationHeader.replace(/^Bearer\s+/, "")))?.thread + .threadId, + ).toBe(otherThreadId); + expect(yield* registry.resolve("unknown-token")).toBeUndefined(); + expect(yield* registry.resolve(token)).toBeUndefined(); - timestamp += 101; - expect(yield* registry.resolve(token)).toBeUndefined(); - }), + yield* registry.touch(threadId); + expect(yield* registry.resolve(token)).toEqual(scope); + timestamp += 101; + expect(yield* registry.resolve(token)).toBeUndefined(); + }), ); -it.effect( - "keeps an expired credential recoverable across rejected requests and other thread activity", - () => +it.effect.each(["provider-session", "thread", "all"] as const)( + "never restores an expired token after %s revocation", + (revocation) => Effect.gen(function* () { let timestamp = 1_000; const registry = yield* makeRegistry(() => timestamp); - const threadId = ThreadId.make("thread-idle"); - const issued = yield* registry.issue({ - threadId, - providerInstanceId: ProviderInstanceId.make("claude"), - capabilities: new Set(), - }); - const token = issued.config.authorizationHeader.replace(/^Bearer\s+/, ""); - - timestamp += 101; - expect(yield* registry.resolve(token)).toBeUndefined(); - const otherThreadId = ThreadId.make("thread-active"); - const other = yield* registry.issue({ - threadId: otherThreadId, - providerInstanceId: ProviderInstanceId.make("codex"), - capabilities: new Set(["device"]), - }); - yield* registry.touch(otherThreadId); - expect( - (yield* registry.resolve(other.config.authorizationHeader.replace(/^Bearer\s+/, ""))) - ?.thread.threadId, - ).toBe(otherThreadId); - expect(yield* registry.resolve("unknown-token")).toBeUndefined(); - expect(yield* registry.resolve(token)).toBeUndefined(); - - timestamp += 101; - expect(yield* registry.resolve(token)).toBeUndefined(); - yield* registry.touch(threadId); - expect((yield* registry.resolve(token))?.thread.providerSessionId).toBe( - issued.config.providerSessionId, - ); - expect( - yield* registry.resolve(other.config.authorizationHeader.replace(/^Bearer\s+/, "")), - ).toBeUndefined(); - }), -); - -it.effect("never restores explicitly revoked credentials on later provider turns", () => - Effect.gen(function* () { - for (const revocation of ["provider-session", "thread", "all"] as const) { - let timestamp = 1_000; - const registry = yield* makeRegistry(() => timestamp); - const threadId = ThreadId.make(`thread-revoked-${revocation}`); + const threadId = ThreadId.make("thread-revoked"); const issued = yield* registry.issue({ threadId, providerInstanceId: ProviderInstanceId.make("codex"), @@ -267,15 +231,12 @@ it.effect("never restores explicitly revoked credentials on later provider turns const token = issued.config.authorizationHeader.replace(/^Bearer\s+/, ""); timestamp += 101; - if (revocation === "provider-session") { - yield* registry.revokeProviderSession(issued.config.providerSessionId); - } else if (revocation === "thread") { - yield* registry.revokeThread(threadId); - } else { - yield* registry.revokeAll; - } + yield* { + "provider-session": registry.revokeProviderSession(issued.config.providerSessionId), + thread: registry.revokeThread(threadId), + all: registry.revokeAll, + }[revocation]; yield* registry.touch(threadId); expect(yield* registry.resolve(token)).toBeUndefined(); - } - }), + }), ); diff --git a/apps/server/src/mcp/McpSessionRegistry.ts b/apps/server/src/mcp/McpSessionRegistry.ts index 67cfd570528a..ae05529cd250 100644 --- a/apps/server/src/mcp/McpSessionRegistry.ts +++ b/apps/server/src/mcp/McpSessionRegistry.ts @@ -60,13 +60,9 @@ export interface McpSessionRegistryOptions { } /** - * Credentials stop authenticating after this long without a sign of life. - * Expired ones cannot refresh themselves through MCP traffic, but stay registered so a provider turn's `touch` can restore the - * token its subprocess still holds. `stopSession` and `stopAll` revoke eagerly. - * - * The bound matters because `/mcp` is mounted outside the environment auth - * stack and is reachable on whatever host the server binds to, so this token is - * the only thing guarding the `t3-code` toolkits on a remote-reachable server. + * `/mcp` is outside environment auth, so idle tokens stop authenticating. + * Keep expired records until explicit revocation: a trusted provider turn can + * refresh the token its subprocess still holds; MCP requests cannot. */ const DEFAULT_LIVENESS_WINDOW_MS = 24 * 60 * 60 * 1_000; diff --git a/apps/server/src/orchestration-v2/RunExecutionService.ts b/apps/server/src/orchestration-v2/RunExecutionService.ts index a3b479770c0c..9abe812c3d12 100644 --- a/apps/server/src/orchestration-v2/RunExecutionService.ts +++ b/apps/server/src/orchestration-v2/RunExecutionService.ts @@ -1391,8 +1391,7 @@ export const layer: Layer.Layer< // A provider turn is a sign that the session is still alive. Keep // its already-issued MCP credential valid even when the agent goes - // a long time between browser-tool calls, and restore it if the - // thread sat idle past the liveness window. + // a long time between browser-tool calls. yield* McpSessionRegistry.touchActiveMcpThread(input.run.threadId); const appContext = (yield* mcpAppModelContext .forThread(input.run.threadId)