diff --git a/.changeset/reconcile-codex-connection-restarts.md b/.changeset/reconcile-codex-connection-restarts.md new file mode 100644 index 000000000..2eb59d761 --- /dev/null +++ b/.changeset/reconcile-codex-connection-restarts.md @@ -0,0 +1,5 @@ +--- +"@sapiom/harness": minor +--- + +Bring Codex into Studio's Sapiom connection lifecycle: report stale credentials, explicitly restart resumable sessions with the current credential, and stop credential-bearing sessions and background tasks on disconnect. diff --git a/packages/harness/src/core/session-manager.test.ts b/packages/harness/src/core/session-manager.test.ts index cba9cf517..f5f59982d 100644 --- a/packages/harness/src/core/session-manager.test.ts +++ b/packages/harness/src/core/session-manager.test.ts @@ -4395,44 +4395,55 @@ describe("SessionManager", () => { expect(session.mcpAuthState).toBe("not-applicable"); }); - it("restarts the exact stale runtime through the existing resume path", async () => { - let generation = 1; - const { manager, adapter, spawns } = makeManager({ - currentCredentialGeneration: () => generation, - buildLaunchOpts: async () => ({ - mcpCredentialLaunch: { - generation, - credentialBearing: true, - }, - }), - }); - const states: Array = []; - manager.onStatusChange((updated) => states.push(updated.mcpAuthState)); - const session = await manager.create({ - cwd: "/tmp/proj", - harness: "claude-code", - }); - await manager.setAgentSessionId(session.id, "agent-session-1"); - generation = 2; - manager.reconcileMcpCredentialGeneration(generation); + it.each(["claude-code", "codex"] as const)( + "restarts the exact stale %s runtime through the existing resume path", + async (harness) => { + let generation = 1; + const adapter = createFakeAdapter({ id: harness }); + const adapters: Partial> = { + [harness]: adapter, + }; + const { manager, spawns } = makeManager({ + adapter, + adapters, + currentCredentialGeneration: () => generation, + buildLaunchOpts: async () => ({ + mcpCredentialLaunch: { + generation, + credentialBearing: true, + }, + }), + }); + const states: Array = []; + manager.onStatusChange((updated) => states.push(updated.mcpAuthState)); + const session = await manager.create({ + cwd: "/tmp/proj", + harness, + }); + await manager.setAgentSessionId(session.id, "agent-session-1"); + generation = 2; + manager.reconcileMcpCredentialGeneration(generation); - const restarting = manager.restartForMcpCredentials(session.id); - await vi.waitFor(() => expect(spawns[0]!.pty.kill).toHaveBeenCalledOnce()); - expect(manager.get(session.id)?.mcpAuthState).toBe("restarting"); - spawns[0]!.emitExit(0); + const restarting = manager.restartForMcpCredentials(session.id); + await vi.waitFor(() => + expect(spawns[0]!.pty.kill).toHaveBeenCalledOnce(), + ); + expect(manager.get(session.id)?.mcpAuthState).toBe("restarting"); + spawns[0]!.emitExit(0); - await expect(restarting).resolves.toMatchObject({ - id: session.id, - status: "running", - mcpAuthState: "current", - }); - expect(spawns).toHaveLength(2); - expect(adapter.resume).toHaveBeenCalledWith( - "agent-session-1", - expect.objectContaining({ harnessSessionId: session.id }), - ); - expect(states).toContain("restarting"); - }); + await expect(restarting).resolves.toMatchObject({ + id: session.id, + status: "running", + mcpAuthState: "current", + }); + expect(spawns).toHaveLength(2); + expect(adapter.resume).toHaveBeenCalledWith( + "agent-session-1", + expect.objectContaining({ harnessSessionId: session.id }), + ); + expect(states).toContain("restarting"); + }, + ); it("keeps an unresumable stale runtime running and restores restart-required", async () => { let generation = 1; @@ -4461,12 +4472,11 @@ describe("SessionManager", () => { }); }); - it("rejects current, unstamped, Codex, and already-stopping runtimes", async () => { + it("rejects current, unstamped, and already-stopping runtimes", async () => { let generation = 1; const claude = createFakeAdapter(); const { manager, spawns } = makeManager({ adapter: claude, - adapters: { "claude-code": claude, codex: createFakeAdapter() }, currentCredentialGeneration: () => generation, buildLaunchOpts: async (_id, request) => request.cwd.endsWith("unstamped") @@ -4497,27 +4507,18 @@ describe("SessionManager", () => { ).rejects.toBeInstanceOf(McpSessionRestartUnavailableError); expect(spawns[1]!.pty.kill).not.toHaveBeenCalled(); - const codex = await manager.create({ - cwd: "/tmp/codex", - harness: "codex", - }); const stopping = await manager.create({ cwd: "/tmp/stopping", harness: "claude-code", }); generation = 2; manager.reconcileMcpCredentialGeneration(generation); - await expect( - manager.restartForMcpCredentials(codex.id), - ).rejects.toBeInstanceOf(McpSessionRestartUnavailableError); - expect(spawns[2]!.pty.kill).not.toHaveBeenCalled(); - const termination = manager.kill(stopping.id); await expect( manager.restartForMcpCredentials(stopping.id), ).rejects.toBeInstanceOf(McpSessionRestartUnavailableError); - expect(spawns[3]!.pty.kill).toHaveBeenCalledOnce(); - spawns[3]!.emitExit(0); + expect(spawns[2]!.pty.kill).toHaveBeenCalledOnce(); + spawns[2]!.emitExit(0); await termination; }); diff --git a/packages/harness/src/core/session-manager.ts b/packages/harness/src/core/session-manager.ts index 23f92fa59..f93db33c4 100644 --- a/packages/harness/src/core/session-manager.ts +++ b/packages/harness/src/core/session-manager.ts @@ -1243,20 +1243,22 @@ export class SessionManager { } } - /** Replace one stale Claude runtime only when its conversation is resumable. */ + /** Replace one stale coding-agent runtime when its conversation is resumable. */ async restartForMcpCredentials(id: string): Promise { if (this.closing) throw new SessionManagerClosingError(); const session = this.sessions.get(id); if (!session) throw new UnknownSessionError(id); const handle = this.ptys.get(id); if ( - session.harness !== "claude-code" || session.mcpAuthState !== "restart-required" || !handle?.mcpCredentialLaunch || handle.killed ) { throw new McpSessionRestartUnavailableError(); } + const harnessLabel = + listHarnessAdapters().find((adapter) => adapter.id === session.harness) + ?.label ?? session.harness; const runtimeEpoch = handle.runtimeEpoch; const restoreRestartRequired = (): void => { @@ -1272,7 +1274,7 @@ export class SessionManager { restoreRestartRequired(); throw new SessionNotResumeableError( id, - "Claude Code has not saved this conversation yet, so it cannot be restarted safely. Start a new session instead.", + `${harnessLabel} has not saved this conversation yet, so it cannot be restarted safely. Start a new session instead.`, ); } let resumable: boolean; @@ -1289,7 +1291,7 @@ export class SessionManager { restoreRestartRequired(); throw new SessionNotResumeableError( id, - "Claude Code no longer has this conversation, so it cannot be restarted safely. Start a new session instead.", + `${harnessLabel} no longer has this conversation, so it cannot be restarted safely. Start a new session instead.`, ); } if (this.ptys.get(id) !== handle || handle.killed) { diff --git a/packages/harness/src/server/auth-mcp-wiring.test.ts b/packages/harness/src/server/auth-mcp-wiring.test.ts index d4be43e26..94c908874 100644 --- a/packages/harness/src/server/auth-mcp-wiring.test.ts +++ b/packages/harness/src/server/auth-mcp-wiring.test.ts @@ -1,5 +1,5 @@ /** - * Lifecycle-level regression coverage for SAP-3114 and SAP-3122. + * Lifecycle-level regression coverage for SAP-3114, SAP-3116, and SAP-3122. * * These tests boot the real server and exercise the shared launch builder used * by interactive create/resume and headless background tasks. OAuth and the @@ -91,7 +91,12 @@ import { writeCredentials, } from "@sapiom/mcp/auth"; import { startServer, type HarnessServer } from "./index.js"; -import type { HarnessAdapter, LaunchOpts, SpawnSpec } from "../shared/types.js"; +import type { + HarnessAdapter, + HarnessKind, + LaunchOpts, + SpawnSpec, +} from "../shared/types.js"; import { CodexAdapter } from "../core/adapters/codex.js"; type LaunchKind = "create" | "resume" | "background"; @@ -135,7 +140,10 @@ function capturingCodexAdapter( }; } -function capturingClaudeAdapter(captures: CapturedLaunch[]): HarnessAdapter { +function capturingAdapter( + harness: HarnessKind, + captures: CapturedLaunch[], +): HarnessAdapter { const capture = (kind: LaunchKind, opts: LaunchOpts): void => { if (!opts.mcpConfigFile) throw new Error("expected an MCP config file"); const config = JSON.parse(readFileSync(opts.mcpConfigFile, "utf-8")) as { @@ -152,8 +160,8 @@ function capturingClaudeAdapter(captures: CapturedLaunch[]): HarnessAdapter { }; return { - id: "claude-code", - eventSource: "hooks", + id: harness, + eventSource: harness === "claude-code" ? "hooks" : "transcript-tail", doctor: async () => [], launch: (opts) => interactiveSpec("create", opts), resume: (_agentSessionId, opts) => interactiveSpec("resume", opts), @@ -262,7 +270,10 @@ describe("Agent Studio MCP authentication wiring", () => { autoCreateSession: false, stateRoot: root, launchDir: projectRoot, - adapters: { "claude-code": capturingClaudeAdapter(captures) }, + adapters: { + "claude-code": capturingAdapter("claude-code", captures), + codex: capturingAdapter("codex", captures), + }, loadSystemPrompt: async () => "test system prompt", ...options, }); @@ -312,83 +323,93 @@ describe("Agent Studio MCP authentication wiring", () => { expect(injectedKey(captures[0])).toBe("browser-key"); }); - it("marks a live signed-out Claude session restart-required after login", async () => { - await boot(); - const session = await server!.sessionManager.create({ - cwd: projectRoot, - harness: "claude-code", - }); - expect(session.mcpAuthState).toBe("current"); + it.each(["claude-code", "codex"] as const)( + "marks a live signed-out %s session restart-required after login", + async (harness) => { + await boot(); + const session = await server!.sessionManager.create({ + cwd: projectRoot, + harness, + }); + expect(session.mcpAuthState).toBe("current"); + + expect((await post("/api/auth/start")).status).toBe(200); + await vi.waitFor(() => + expect(server!.sessionManager.get(session.id)).toMatchObject({ + status: "running", + mcpAuthState: "restart-required", + }), + ); + }, + ); + + it.each(["claude-code", "codex"] as const)( + "explicitly restarts a stale %s session with a rotated key", + async (harness) => { + authFixture.credential = credential("key-a"); + await boot({ + identity: { + userId: "test-tenant", + tenantId: "test-tenant", + organizationName: "Test Org", + apiKey: "key-a", + source: "cached", + }, + }); + const session = await server!.sessionManager.create({ + cwd: projectRoot, + harness, + }); + await server!.sessionManager.setAgentSessionId( + session.id, + "agent-session-1", + ); - expect((await post("/api/auth/start")).status).toBe(200); - await vi.waitFor(() => + authFixture.credential = credential("key-b"); + await writeFile(authFixture.credentialsPath, "external rotation signal"); + await vi.waitFor(() => + expect(server!.sessionManager.get(session.id)?.mcpAuthState).toBe( + "restart-required", + ), + ); + + const restart = await post(`/api/sessions/${session.id}/restart-mcp`); + const restartBody = (await restart.json()) as Record; + expect(restart.status, JSON.stringify(restartBody)).toBe(200); expect(server!.sessionManager.get(session.id)).toMatchObject({ status: "running", - mcpAuthState: "restart-required", - }), - ); - }); - - it("explicitly restarts a stale session with a rotated key", async () => { - authFixture.credential = credential("key-a"); - await boot({ - identity: { - userId: "test-tenant", - tenantId: "test-tenant", - organizationName: "Test Org", - apiKey: "key-a", - source: "cached", - }, - }); - const session = await server!.sessionManager.create({ - cwd: projectRoot, - harness: "claude-code", - }); - await server!.sessionManager.setAgentSessionId( - session.id, - "agent-session-1", - ); - - authFixture.credential = credential("key-b"); - await writeFile(authFixture.credentialsPath, "external rotation signal"); - await vi.waitFor(() => - expect(server!.sessionManager.get(session.id)?.mcpAuthState).toBe( - "restart-required", - ), - ); - - const restart = await post(`/api/sessions/${session.id}/restart-mcp`); - const restartBody = (await restart.json()) as Record; - expect(restart.status, JSON.stringify(restartBody)).toBe(200); - expect(server!.sessionManager.get(session.id)).toMatchObject({ - status: "running", - mcpAuthState: "current", - }); - expect(captures.at(-1)).toMatchObject({ kind: "resume" }); - expect(injectedKey(captures.at(-1)!)).toBe("key-b"); - }); - - it("waits for a credential-bearing session to exit before disconnect succeeds", async () => { - authFixture.credential = credential("key-a"); - await boot({ - identity: { - userId: "test-tenant", - tenantId: "test-tenant", - organizationName: "Test Org", - apiKey: "key-a", - source: "cached", - }, - }); - const session = await server!.sessionManager.create({ - cwd: projectRoot, - harness: "claude-code", - }); - - const disconnect = await post("/api/auth/disconnect"); - - expect(disconnect.status).toBe(200); - expect(server!.sessionManager.get(session.id)?.status).toBe("exited"); - }, 20_000); + mcpAuthState: "current", + }); + expect(captures.at(-1)).toMatchObject({ kind: "resume" }); + expect(injectedKey(captures.at(-1)!)).toBe("key-b"); + }, + ); + + it.each(["claude-code", "codex"] as const)( + "waits for a credential-bearing %s session to exit before disconnect succeeds", + async (harness) => { + authFixture.credential = credential("key-a"); + await boot({ + identity: { + userId: "test-tenant", + tenantId: "test-tenant", + organizationName: "Test Org", + apiKey: "key-a", + source: "cached", + }, + }); + const session = await server!.sessionManager.create({ + cwd: projectRoot, + harness, + }); + + const disconnect = await post("/api/auth/disconnect"); + + expect(disconnect.status).toBe(200); + expect(server!.sessionManager.get(session.id)?.status).toBe("exited"); + }, + 20_000, + ); it("terminates a credential-bearing session when an external logout changes the shared store", async () => { authFixture.credential = credential("key-a"); diff --git a/packages/harness/src/server/index.ts b/packages/harness/src/server/index.ts index b00f2d428..688357a77 100644 --- a/packages/harness/src/server/index.ts +++ b/packages/harness/src/server/index.ts @@ -663,7 +663,7 @@ function createDefaultBuildLaunchOpts( systemPromptFile, ...(context?.agentMapMcp ? { agentMapMcp: context.agentMapMcp } : {}), ...(pluginDir ? { pluginDir } : {}), - ...(req.harness === "claude-code" + ...(req.harness === "claude-code" || req.harness === "codex" ? { mcpCredentialLaunch: { generation, diff --git a/packages/harness/web/src/components/McpAuthRestartNotice.tsx b/packages/harness/web/src/components/McpAuthRestartNotice.tsx index 3fb4dd440..c35aac6aa 100644 --- a/packages/harness/web/src/components/McpAuthRestartNotice.tsx +++ b/packages/harness/web/src/components/McpAuthRestartNotice.tsx @@ -8,7 +8,7 @@ interface McpAuthRestartNoticeProps { onRestart: () => Promise; } -/** A scoped, non-blocking recovery action for one stale Claude runtime. */ +/** A scoped, non-blocking recovery action for one stale coding-agent runtime. */ export function McpAuthRestartNotice({ restarting, onRestart, diff --git a/packages/harness/web/src/lib/api.ts b/packages/harness/web/src/lib/api.ts index 6998d917b..0ddcb85b6 100644 --- a/packages/harness/web/src/lib/api.ts +++ b/packages/harness/web/src/lib/api.ts @@ -416,7 +416,7 @@ export interface HarnessApi { */ sessionRecord(id: string): Promise; resumeSession(id: string): Promise; - /** Restart one live Claude session whose launch-time MCP auth is stale. */ + /** Restart one live coding-agent session whose launch-time MCP auth is stale. */ restartMcpSession(id: string): Promise; /** Take a transcript-only history row (`resumeMode: "agent-resume"`, no * `harnessSessionId`) into the registry and resume it — the honest diff --git a/packages/harness/web/src/lib/use-harness-state.ts b/packages/harness/web/src/lib/use-harness-state.ts index dfda870dd..c0c574490 100644 --- a/packages/harness/web/src/lib/use-harness-state.ts +++ b/packages/harness/web/src/lib/use-harness-state.ts @@ -190,7 +190,7 @@ export interface HarnessStateHook { * recorded for it). Stable identity — safe as an effect dependency. */ sessionRecord: (id: string) => Promise; resumeSession: (harnessSessionId: string) => Promise; - /** Explicitly replace one live Claude runtime with stale MCP auth. */ + /** Explicitly replace one live coding-agent runtime with stale MCP auth. */ restartMcpSession: (harnessSessionId: string) => Promise; /** * Portable continue: a fresh session in `cwd`, seeded with our own