From 9baf6e905ff12dd8133ae98e7b5f558646a92f90 Mon Sep 17 00:00:00 2001 From: Furquan Uddin Date: Sun, 10 May 2026 18:33:13 +0700 Subject: [PATCH 1/3] fix(smithy): update codex interactive sandbox flag Extract the Codex interactive argument construction into buildCodexInteractiveArgs so the unit test exercises the same production arg-building logic used by spawn instead of duplicating that logic in the test. The helper preserves the existing working-directory, resume, and model behavior. Codex now documents --sandbox workspace-write as the replacement for the deprecated --full-auto shortcut, so use the documented sandbox flag for fresh and resumed interactive sessions. Refs stoneforge-ai/stoneforge#114. --- .changeset/calm-codex-flags.md | 5 ++ .../smithy/src/providers/codex/interactive.ts | 43 +++++++++------ .../src/providers/codex/provider.bun.test.ts | 54 +++++++------------ 3 files changed, 51 insertions(+), 51 deletions(-) create mode 100644 .changeset/calm-codex-flags.md diff --git a/.changeset/calm-codex-flags.md b/.changeset/calm-codex-flags.md new file mode 100644 index 00000000..ea350353 --- /dev/null +++ b/.changeset/calm-codex-flags.md @@ -0,0 +1,5 @@ +--- +"@stoneforge/smithy": patch +--- + +Update Codex interactive sessions to use the documented workspace-write sandbox flag. diff --git a/packages/smithy/src/providers/codex/interactive.ts b/packages/smithy/src/providers/codex/interactive.ts index f2d28c9a..f022117e 100644 --- a/packages/smithy/src/providers/codex/interactive.ts +++ b/packages/smithy/src/providers/codex/interactive.ts @@ -21,6 +21,31 @@ import { shellQuote } from '../shell-quote.js'; // Helpers // ============================================================================ +type CodexInteractiveArgOptions = Pick< + InteractiveSpawnOptions, + 'resumeSessionId' | 'workingDirectory' | 'model' +>; + +export function buildCodexInteractiveArgs( + options: CodexInteractiveArgOptions, + platform: NodeJS.Platform = process.platform, +): string[] { + const quote = (value: string) => shellQuote(value, platform); + const args: string[] = []; + + if (options.resumeSessionId) { + args.push('resume', quote(options.resumeSessionId), '--sandbox', 'workspace-write'); + } else { + args.push('--sandbox', 'workspace-write', '--cd', quote(options.workingDirectory)); + } + + if (options.model) { + args.push('--model', quote(options.model)); + } + + return args; +} + // ============================================================================ // Codex Interactive Session // ============================================================================ @@ -95,7 +120,7 @@ export class CodexInteractiveProvider implements InteractiveProvider { } async spawn(options: InteractiveSpawnOptions): Promise { - const args = this.buildArgs(options); + const args = buildCodexInteractiveArgs(options); const env: Record = { ...(process.env as Record), @@ -156,20 +181,4 @@ export class CodexInteractiveProvider implements InteractiveProvider { } } - private buildArgs(options: InteractiveSpawnOptions): string[] { - const args: string[] = []; - - if (options.resumeSessionId) { - args.push('resume', shellQuote(options.resumeSessionId), '--full-auto'); - } else { - args.push('--full-auto', '--cd', shellQuote(options.workingDirectory)); - } - - // Add model flag if provided - if (options.model) { - args.push('--model', shellQuote(options.model)); - } - - return args; - } } diff --git a/packages/smithy/src/providers/codex/provider.bun.test.ts b/packages/smithy/src/providers/codex/provider.bun.test.ts index 02ccf979..e2344d8e 100644 --- a/packages/smithy/src/providers/codex/provider.bun.test.ts +++ b/packages/smithy/src/providers/codex/provider.bun.test.ts @@ -6,6 +6,7 @@ import { describe, it, expect, mock, beforeEach, afterEach } from 'bun:test'; import type { CodexClient, CodexModelInfo } from './server-manager.js'; +import { buildCodexInteractiveArgs } from './interactive.js'; import { posixShellQuote, shellQuote } from '../shell-quote.js'; // --------------------------------------------------------------------------- @@ -204,68 +205,53 @@ describe('CodexHeadlessProvider model passthrough', () => { // --------------------------------------------------------------------------- describe('CodexInteractiveProvider model flag', () => { - // Use the POSIX form directly so assertions are deterministic regardless of - // the host OS the tests run on. A separate suite below covers the Windows - // quoter. - function buildArgs(options: { - resumeSessionId?: string; - workingDirectory: string; - model?: string; - }): string[] { - const shellQuote = posixShellQuote; - const args: string[] = []; - - if (options.resumeSessionId) { - args.push('resume', shellQuote(options.resumeSessionId), '--full-auto'); - } else { - args.push('--full-auto', '--cd', shellQuote(options.workingDirectory)); - } - - // Add model flag if provided - if (options.model) { - args.push('--model', shellQuote(options.model)); - } - - return args; - } + it('builds interactive spawn args with the documented workspace-write sandbox', () => { + const args = buildCodexInteractiveArgs({ + workingDirectory: '/workspace', + model: 'gpt-4o', + }, 'linux'); + + expect(args).toEqual(['--sandbox', 'workspace-write', '--cd', "'/workspace'", '--model', "'gpt-4o'"]); + }); it('should include --model flag when model is provided', () => { - const args = buildArgs({ + const args = buildCodexInteractiveArgs({ workingDirectory: '/workspace', model: 'gpt-4o', - }); + }, 'linux'); expect(args).toContain('--model'); expect(args).toContain("'gpt-4o'"); - expect(args).toEqual(['--full-auto', '--cd', "'/workspace'", '--model', "'gpt-4o'"]); + expect(args).toEqual(['--sandbox', 'workspace-write', '--cd', "'/workspace'", '--model', "'gpt-4o'"]); }); it('should not include --model flag when model is undefined', () => { - const args = buildArgs({ + const args = buildCodexInteractiveArgs({ workingDirectory: '/workspace', - }); + }, 'linux'); expect(args).not.toContain('--model'); - expect(args).toEqual(['--full-auto', '--cd', "'/workspace'"]); + expect(args).toEqual(['--sandbox', 'workspace-write', '--cd', "'/workspace'"]); }); it('should include --model flag when resuming with model', () => { - const args = buildArgs({ + const args = buildCodexInteractiveArgs({ resumeSessionId: 'thr_abc123', workingDirectory: '/workspace', model: 'o3-mini', - }); + }, 'linux'); expect(args).toContain('resume'); expect(args).toContain('--model'); expect(args).toContain("'o3-mini'"); + expect(args).toEqual(['resume', "'thr_abc123'", '--sandbox', 'workspace-write', '--model', "'o3-mini'"]); }); it('should properly quote model names with special characters', () => { - const args = buildArgs({ + const args = buildCodexInteractiveArgs({ workingDirectory: '/workspace', model: "model's-name", - }); + }, 'linux'); expect(args).toContain("--model"); expect(args).toContain("'model'\\''s-name'"); From cc9cc159ab9d9f0e5b7f16f8726be7cbcbd6faa2 Mon Sep 17 00:00:00 2001 From: Furquan Uddin Date: Sun, 10 May 2026 23:28:15 +0700 Subject: [PATCH 2/3] fix: preserve codex interactive resume ids --- apps/smithy-server/src/routes/sessions.ts | 12 +++ apps/smithy-web/src/api/hooks/useAgents.ts | 47 ++++++----- .../smithy/src/providers/codex/interactive.ts | 60 +++++++++++++-- .../src/providers/codex/provider.bun.test.ts | 77 ++++++++++++++++++- packages/smithy/src/providers/types.ts | 2 + .../smithy/src/runtime/session-manager.ts | 20 +++-- .../smithy/src/runtime/spawner.bun.test.ts | 55 +++++++++++++ packages/smithy/src/runtime/spawner.ts | 6 +- packages/smithy/src/server/routes/sessions.ts | 12 +++ 9 files changed, 254 insertions(+), 37 deletions(-) diff --git a/apps/smithy-server/src/routes/sessions.ts b/apps/smithy-server/src/routes/sessions.ts index 1371674b..0131a04d 100644 --- a/apps/smithy-server/src/routes/sessions.ts +++ b/apps/smithy-server/src/routes/sessions.ts @@ -15,6 +15,7 @@ import { formatSessionRecord } from '../formatters.js'; import { notifySSEClientsOfNewSession } from './events.js'; const logger = createLogger('sessions'); +const CODEX_RESUME_SESSION_ID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; type NotifyClientsCallback = ( agentId: EntityId, @@ -593,6 +594,17 @@ Please begin working on this task. Use \`sf task get ${taskResult.id}\` to see f providerSessionId = resumable.providerSessionId; } + const meta = getAgentMetadata(agent); + if ((meta as { provider?: string } | undefined)?.provider === 'codex' + && !CODEX_RESUME_SESSION_ID_PATTERN.test(providerSessionId)) { + return c.json({ + error: { + code: 'INVALID_PROVIDER_SESSION_ID', + message: 'Codex resume requires a valid UUID session ID', + }, + }, 400); + } + const { session, events, uwpCheck } = await sessionManager.resumeSession(agentId, { providerSessionId, workingDirectory: body.workingDirectory, diff --git a/apps/smithy-web/src/api/hooks/useAgents.ts b/apps/smithy-web/src/api/hooks/useAgents.ts index eeace80c..e4ca1fce 100644 --- a/apps/smithy-web/src/api/hooks/useAgents.ts +++ b/apps/smithy-web/src/api/hooks/useAgents.ts @@ -25,6 +25,17 @@ import type { // ============================================================================ const API_BASE = '/api'; +const CODEX_RESUME_SESSION_ID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +function hasValidProviderSessionIdForAgent(agent: Agent, session: unknown): session is { providerSessionId: string } { + const providerSessionId = (session as { providerSessionId?: unknown } | null)?.providerSessionId; + if (typeof providerSessionId !== 'string' || providerSessionId.length === 0) { + return false; + } + + return agent.metadata?.agent?.provider !== 'codex' + || CODEX_RESUME_SESSION_ID_PATTERN.test(providerSessionId); +} async function fetchApi(path: string, options?: RequestInit): Promise { const response = await fetch(`${API_BASE}${path}`, { @@ -211,25 +222,23 @@ export function useDirectors(): { })), }); - const directors: DirectorInfo[] = useMemo(() => { - return (directorAgents ?? []).map((director, i) => { - const query = statusQueries[i]; - const statusData = query?.data; - const history = statusData?.recentHistory ?? []; - const lastResumableSession = history.find((h) => !!h.providerSessionId) ?? null; - - return { - director, - hasActiveSession: statusData?.hasActiveSession ?? false, - activeSession: statusData?.activeSession ?? null, - recentHistory: history, - lastResumableSession, - hasResumableSession: lastResumableSession !== null, - isLoading: query?.isLoading ?? true, - error: (query?.error as Error) ?? null, - }; - }); - }, [directorAgents, statusQueries]); + const directors: DirectorInfo[] = (directorAgents ?? []).map((director, i) => { + const query = statusQueries[i]; + const statusData = query?.data; + const history = statusData?.recentHistory ?? []; + const lastResumableSession = history.find((h) => hasValidProviderSessionIdForAgent(director, h)) ?? null; + + return { + director, + hasActiveSession: statusData?.hasActiveSession ?? false, + activeSession: statusData?.activeSession ?? null, + recentHistory: history, + lastResumableSession, + hasResumableSession: lastResumableSession !== null, + isLoading: query?.isLoading ?? true, + error: (query?.error as Error) ?? null, + }; + }); const isLoading = agentsLoading || statusQueries.some(q => q.isLoading); const combinedError = agentsError ?? statusQueries.find(q => q.error)?.error ?? null; diff --git a/packages/smithy/src/providers/codex/interactive.ts b/packages/smithy/src/providers/codex/interactive.ts index f022117e..d9ff68a3 100644 --- a/packages/smithy/src/providers/codex/interactive.ts +++ b/packages/smithy/src/providers/codex/interactive.ts @@ -26,6 +26,42 @@ type CodexInteractiveArgOptions = Pick< 'resumeSessionId' | 'workingDirectory' | 'model' >; +const CODEX_RESUME_SESSION_ID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; +const CODEX_CONTINUE_SESSION_PATTERN = + /To continue this session,\s+run\s+codex\s+resume\s+([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})/i; +const ANSI_ESCAPE_PATTERN = /\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])/g; +const MAX_CODEX_OUTPUT_BUFFER = 4096; + +export function isCodexResumeSessionId(value: string): value is ProviderSessionId { + return CODEX_RESUME_SESSION_ID_PATTERN.test(value); +} + +export function extractCodexResumeSessionId(output: string): ProviderSessionId | undefined { + const normalizedOutput = output.replace(ANSI_ESCAPE_PATTERN, ''); + return normalizedOutput.match(CODEX_CONTINUE_SESSION_PATTERN)?.[1]; +} + +export function createCodexResumeSessionIdDetector( + maxBufferLength = MAX_CODEX_OUTPUT_BUFFER, +): (chunk: string) => ProviderSessionId | undefined { + let buffer = ''; + + return (chunk: string) => { + buffer = (buffer + chunk).slice(-maxBufferLength); + return extractCodexResumeSessionId(buffer); + }; +} + +export function writeCodexExitCommand( + write: (data: string) => void, + scheduleEnter: (callback: () => void) => void = (callback) => { + setTimeout(callback, 50); + }, +): void { + write('/exit'); + scheduleEnter(() => write('\r')); +} + export function buildCodexInteractiveArgs( options: CodexInteractiveArgOptions, platform: NodeJS.Platform = process.platform, @@ -34,6 +70,9 @@ export function buildCodexInteractiveArgs( const args: string[] = []; if (options.resumeSessionId) { + if (!isCodexResumeSessionId(options.resumeSessionId)) { + throw new Error(`Invalid Codex resume session ID: ${options.resumeSessionId}`); + } args.push('resume', quote(options.resumeSessionId), '--sandbox', 'workspace-write'); } else { args.push('--sandbox', 'workspace-write', '--cd', quote(options.workingDirectory)); @@ -53,14 +92,16 @@ export function buildCodexInteractiveArgs( class CodexInteractiveSession implements InteractiveSession { private ptyProcess: IPty; private sessionId: ProviderSessionId | undefined; + private readonly detectResumeSessionId = createCodexResumeSessionIdDetector(); readonly pid?: number; - constructor(ptyProcess: IPty) { + constructor(ptyProcess: IPty, resumeSessionId?: ProviderSessionId) { this.ptyProcess = ptyProcess; this.pid = ptyProcess.pid; + this.sessionId = resumeSessionId; - // Listen for thread/session ID in output and auto-respond to terminal queries. + // Listen for Codex's continuation footer and auto-respond to terminal queries. // Codex sends DSR (Device Status Report) \x1b[6n on startup to query cursor // position. When no terminal emulator (e.g. xterm.js) is connected to respond, // codex times out and exits. We auto-respond with a default cursor position @@ -71,9 +112,9 @@ class CodexInteractiveSession implements InteractiveSession { } if (!this.sessionId) { - const match = data.match(/(?:Thread|Session|thr_)[:=\s]*([a-z0-9_-]+)/i); - if (match) { - this.sessionId = match[1]; + const detectedSessionId = this.detectResumeSessionId(data); + if (detectedSessionId) { + this.sessionId = detectedSessionId; } } }); @@ -83,6 +124,10 @@ class CodexInteractiveSession implements InteractiveSession { this.ptyProcess.write(data); } + requestExit(): void { + writeCodexExitCommand((data) => this.ptyProcess.write(data)); + } + resize(cols: number, rows: number): void { this.ptyProcess.resize(cols, rows); } @@ -156,7 +201,10 @@ export class CodexInteractiveProvider implements InteractiveProvider { env, }); - const session = new CodexInteractiveSession(ptyProcess); + const resumeSessionId = options.resumeSessionId && isCodexResumeSessionId(options.resumeSessionId) + ? options.resumeSessionId + : undefined; + const session = new CodexInteractiveSession(ptyProcess, resumeSessionId); // On Windows, write the command to cmd.exe stdin (bash -c handles this on Unix) if (process.platform === 'win32') { diff --git a/packages/smithy/src/providers/codex/provider.bun.test.ts b/packages/smithy/src/providers/codex/provider.bun.test.ts index e2344d8e..9ff24ee9 100644 --- a/packages/smithy/src/providers/codex/provider.bun.test.ts +++ b/packages/smithy/src/providers/codex/provider.bun.test.ts @@ -6,7 +6,13 @@ import { describe, it, expect, mock, beforeEach, afterEach } from 'bun:test'; import type { CodexClient, CodexModelInfo } from './server-manager.js'; -import { buildCodexInteractiveArgs } from './interactive.js'; +import { + buildCodexInteractiveArgs, + createCodexResumeSessionIdDetector, + extractCodexResumeSessionId, + isCodexResumeSessionId, + writeCodexExitCommand, +} from './interactive.js'; import { posixShellQuote, shellQuote } from '../shell-quote.js'; // --------------------------------------------------------------------------- @@ -205,6 +211,8 @@ describe('CodexHeadlessProvider model passthrough', () => { // --------------------------------------------------------------------------- describe('CodexInteractiveProvider model flag', () => { + const codexSessionId = '019e1277-6206-74e1-bb66-bbd8e9534628'; + it('builds interactive spawn args with the documented workspace-write sandbox', () => { const args = buildCodexInteractiveArgs({ workingDirectory: '/workspace', @@ -236,7 +244,7 @@ describe('CodexInteractiveProvider model flag', () => { it('should include --model flag when resuming with model', () => { const args = buildCodexInteractiveArgs({ - resumeSessionId: 'thr_abc123', + resumeSessionId: codexSessionId, workingDirectory: '/workspace', model: 'o3-mini', }, 'linux'); @@ -244,7 +252,14 @@ describe('CodexInteractiveProvider model flag', () => { expect(args).toContain('resume'); expect(args).toContain('--model'); expect(args).toContain("'o3-mini'"); - expect(args).toEqual(['resume', "'thr_abc123'", '--sandbox', 'workspace-write', '--model', "'o3-mini'"]); + expect(args).toEqual(['resume', `'${codexSessionId}'`, '--sandbox', 'workspace-write', '--model', "'o3-mini'"]); + }); + + it('should reject non-UUID resume session IDs', () => { + expect(() => buildCodexInteractiveArgs({ + resumeSessionId: 'thr_abc123', + workingDirectory: '/workspace', + }, 'linux')).toThrow('Invalid Codex resume session ID'); }); it('should properly quote model names with special characters', () => { @@ -258,6 +273,62 @@ describe('CodexInteractiveProvider model flag', () => { }); }); +describe('Codex interactive resume session ID parsing', () => { + const codexSessionId = '019e1277-6206-74e1-bb66-bbd8e9534628'; + + it('extracts UUID from Codex continuation footer', () => { + expect(extractCodexResumeSessionId( + `Token usage: total=19,927\nTo continue this session, run codex resume ${codexSessionId}\n`, + )).toBe(codexSessionId); + }); + + it('ignores ordinary session management text', () => { + expect(extractCodexResumeSessionId('Agent Session Management')).toBeUndefined(); + }); + + it('ignores Codex session-not-found errors', () => { + expect(extractCodexResumeSessionId( + 'No saved session found with ID management. Run `codex resume` without an ID to choose from existing sessions.', + )).toBeUndefined(); + }); + + it('ignores non-UUID continuation footer values', () => { + expect(extractCodexResumeSessionId('To continue this session, run codex resume management')).toBeUndefined(); + }); + + it('detects footer split across PTY chunks', () => { + const detect = createCodexResumeSessionIdDetector(); + + expect(detect('Token usage: total=19,927\nTo continue this session, ')).toBeUndefined(); + expect(detect(`run codex resume ${codexSessionId}\n`)).toBe(codexSessionId); + }); + + it('validates Codex resume IDs as UUIDs', () => { + expect(isCodexResumeSessionId(codexSessionId)).toBe(true); + expect(isCodexResumeSessionId('management')).toBe(false); + expect(isCodexResumeSessionId('thr_abc123')).toBe(false); + }); +}); + +describe('Codex interactive graceful exit', () => { + it('writes slash exit and schedules Enter as separate PTY writes', () => { + const writes: string[] = []; + const scheduled: (() => void)[] = []; + + writeCodexExitCommand( + (data) => writes.push(data), + (callback) => scheduled.push(callback), + ); + + expect(writes).toEqual(['/exit']); + expect(scheduled).toHaveLength(1); + + scheduled[0](); + + expect(writes).toEqual(['/exit', '\r']); + }); +}); + describe('CodexInteractiveProvider Windows quoting (issue #51)', () => { it('quotes the executable with double quotes on win32, not single quotes', () => { // Regression guard: on Windows, shellQuote must never wrap the command diff --git a/packages/smithy/src/providers/types.ts b/packages/smithy/src/providers/types.ts index 5c228d34..3ac9ffab 100644 --- a/packages/smithy/src/providers/types.ts +++ b/packages/smithy/src/providers/types.ts @@ -124,6 +124,8 @@ export interface InteractiveSession { readonly pid?: number; /** Write data to the PTY */ write(data: string): void; + /** Request a graceful provider-specific exit, if different from shell exit */ + requestExit?(): void; /** Resize the terminal */ resize(cols: number, rows: number): void; /** Kill the PTY */ diff --git a/packages/smithy/src/runtime/session-manager.ts b/packages/smithy/src/runtime/session-manager.ts index bc235466..7def1ab3 100644 --- a/packages/smithy/src/runtime/session-manager.ts +++ b/packages/smithy/src/runtime/session-manager.ts @@ -746,9 +746,6 @@ export class SessionManagerImpl implements SessionManager { throw new Error(`Session not found: ${sessionId}`); } - // Clean up event listeners to prevent leaks - this.cleanupSessionEventListeners(sessionId); - // Update session state BEFORE terminating to prevent race with exit event handler const updatedSession: InternalSessionState = { ...session, @@ -764,12 +761,19 @@ export class SessionManagerImpl implements SessionManager { this.agentSessions.delete(session.agentId); } - // Add to history (do this before terminate to avoid race with exit handler) - this.addToHistory(session.agentId, updatedSession); - - // Now terminate via spawner (may trigger exit event, but status already terminated) + // Now terminate via spawner (may trigger exit/provider-session-id events, but status already terminated). + // Keep listeners attached until this completes so provider-specific graceful shutdown output + // can still update providerSessionId before the session is added to history. await this.spawner.terminate(sessionId, options?.graceful ?? true); + const finalSession = this.sessions.get(sessionId) ?? updatedSession; + + // Clean up event listeners after graceful termination output has been observed. + this.cleanupSessionEventListeners(sessionId); + + // Add to history after terminate so providerSessionId captured during shutdown is included. + this.addToHistory(session.agentId, finalSession); + // Update agent's session status in database await this.registry.updateAgentSession(session.agentId, undefined, 'idle'); @@ -780,7 +784,7 @@ export class SessionManagerImpl implements SessionManager { this.scheduleTerminatedSessionCleanup(sessionId); // Emit status change - session.events.emit('status', 'terminated'); + finalSession.events.emit('status', 'terminated'); // Log session termination this.operationLog?.write('info', 'session', `Session terminated for agent ${session.agentId}${options?.reason ? `: ${options.reason}` : ''}`, { agentId: session.agentId, sessionId }); diff --git a/packages/smithy/src/runtime/spawner.bun.test.ts b/packages/smithy/src/runtime/spawner.bun.test.ts index a923c4f4..f1924536 100644 --- a/packages/smithy/src/runtime/spawner.bun.test.ts +++ b/packages/smithy/src/runtime/spawner.bun.test.ts @@ -102,6 +102,61 @@ describe('SpawnerService', () => { 'Session not found' ); }); + + test('uses provider-specific requestExit for graceful interactive termination', async () => { + let exitCallback: ((code: number, signal?: number) => void) | undefined; + const writes: string[] = []; + + const interactiveProvider: InteractiveProvider = { + name: 'mock-interactive', + spawn: async (): Promise => ({ + pid: 12345, + write: (data: string) => { + writes.push(data); + }, + requestExit: () => { + writes.push('requestExit'); + exitCallback?.(0); + }, + resize: () => {}, + kill: () => { + writes.push('kill'); + }, + onData: () => {}, + onExit: (callback: (code: number, signal?: number) => void) => { + exitCallback = callback; + }, + getSessionId: () => undefined, + }), + isAvailable: async () => true, + }; + + const provider: AgentProvider = { + name: 'mock-provider', + headless: createMockHeadlessProvider(createImmediateExitHeadlessSession), + interactive: interactiveProvider, + isAvailable: async () => true, + getInstallInstructions: () => 'No install needed for mock', + listModels: async (): Promise => [], + }; + + const service = new SpawnerServiceImpl({ + provider, + workingDirectory: '/tmp', + timeout: 1000, + }); + + const result = await service.spawn(testAgentId, 'director', { + mode: 'interactive', + }); + + await service.terminate(result.session.id, true); + + expect(writes).toContain('requestExit'); + expect(writes).not.toContain('exit\r'); + expect(writes).not.toContain('kill'); + expect(service.getSession(result.session.id)?.status).toBe('terminated'); + }); }); describe('suspend', () => { diff --git a/packages/smithy/src/runtime/spawner.ts b/packages/smithy/src/runtime/spawner.ts index 891c840c..b613aa79 100644 --- a/packages/smithy/src/runtime/spawner.ts +++ b/packages/smithy/src/runtime/spawner.ts @@ -584,7 +584,11 @@ export class SpawnerServiceImpl implements SpawnerService { // Handle provider interactive sessions if (session.interactiveSession) { if (graceful) { - session.interactiveSession.write('exit\r'); + if (session.interactiveSession.requestExit) { + session.interactiveSession.requestExit(); + } else { + session.interactiveSession.write('exit\r'); + } await new Promise((resolve) => { const timeout = setTimeout(() => { diff --git a/packages/smithy/src/server/routes/sessions.ts b/packages/smithy/src/server/routes/sessions.ts index 68cf017a..30df1a30 100644 --- a/packages/smithy/src/server/routes/sessions.ts +++ b/packages/smithy/src/server/routes/sessions.ts @@ -21,6 +21,7 @@ import { notifySSEClientsOfNewSession } from './events.js'; import { createLogger } from '../../utils/logger.js'; const logger = createLogger('sessions'); +const CODEX_RESUME_SESSION_ID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; type NotifyClientsCallback = ( agentId: EntityId, @@ -605,6 +606,17 @@ Please begin working on this task. Use \`sf task get ${taskResult.id}\` to see f providerSessionId = resumable.providerSessionId; } + const meta = getAgentMetadata(agent); + if ((meta as { provider?: string } | undefined)?.provider === 'codex' + && !CODEX_RESUME_SESSION_ID_PATTERN.test(providerSessionId)) { + return c.json({ + error: { + code: 'INVALID_PROVIDER_SESSION_ID', + message: 'Codex resume requires a valid UUID session ID', + }, + }, 400); + } + const { session, events, uwpCheck } = await sessionManager.resumeSession(agentId, { providerSessionId, workingDirectory: body.workingDirectory, From f745d573d84e6fd6c52ae31287157d2f15a84b4f Mon Sep 17 00:00:00 2001 From: Furquan Uddin Date: Sun, 10 May 2026 23:41:06 +0700 Subject: [PATCH 3/3] chore: add codex resume changeset --- .changeset/quiet-codex-resume.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/quiet-codex-resume.md diff --git a/.changeset/quiet-codex-resume.md b/.changeset/quiet-codex-resume.md new file mode 100644 index 00000000..f4f3c69b --- /dev/null +++ b/.changeset/quiet-codex-resume.md @@ -0,0 +1,5 @@ +--- +"@stoneforge/smithy": patch +--- + +Fix Codex interactive resume so only UUID session IDs from the Codex continuation footer are persisted. Stopping an interactive Codex session now requests a clean `/exit` shutdown, allowing Stoneforge to capture the resume ID and avoid offering invalid resume targets from ordinary terminal text.