Skip to content

Commit 8ff0fbf

Browse files
author
yongkangc
committed
fix(codex): commit active input ownership atomically
Persist accepted input before publishing successor turn ownership across the queue, event bus, operation tracker, and Codex runtime. Transfer abortability at the same synchronous boundary and cover failure, restart, and settlement races deterministically.
1 parent d1edf53 commit 8ff0fbf

18 files changed

Lines changed: 501 additions & 171 deletions

File tree

server-agents/codex/src/agents/codex/__tests__/execution.test.js

Lines changed: 109 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { AssistantMessage, UserMessage } from '@garcon/common/chat-types';
33
import { renderTranscriptSeed } from '@garcon/common/transcript-seed';
44
import { AgentEventEmitterRuntime } from '@garcon/server-agent-common/shared/event-emitter-runtime';
55
import { createPathNativeSessionCodec } from '@garcon/server-agent-common/native-session/path-native-session';
6+
import { AgentEventBus } from '../../../../../../server/agents/event-bus.ts';
7+
import { QueueExecutionAttempt } from '../../../../../../server/chat-execution/execution-attempt.ts';
68
import { CodexExecution } from '../execution.ts';
79

810
function createRuntime() {
@@ -67,7 +69,12 @@ function startRequest(overrides = {}) {
6769
};
6870
}
6971

70-
function activeInputRequest(operation, beforeDelivery = async () => undefined) {
72+
async function commitHandoff(handoff) {
73+
handoff.validate();
74+
handoff.commit();
75+
}
76+
77+
function activeInputRequest(operation, beforeDelivery = commitHandoff) {
7178
return startRequest({
7279
agentSessionId: 'thread-1',
7380
nativeSession: {
@@ -236,7 +243,10 @@ describe('CodexExecution', () => {
236243
execution.subscribe((event) => events.push(event));
237244
await execution.start(startRequest());
238245
runtime.submitActiveInput.mockImplementation(async (_request, beforeDelivery) => {
239-
await beforeDelivery();
246+
await beforeDelivery({
247+
validate: () => undefined,
248+
commit: () => undefined,
249+
});
240250
throw new Error('delivery outcome unknown');
241251
});
242252

@@ -252,4 +262,101 @@ describe('CodexExecution', () => {
252262
operation: successor,
253263
}));
254264
});
265+
266+
it('commits retained, routed, tracked, and runtime ownership at one persistence boundary', async () => {
267+
const runtime = createRuntime();
268+
const execution = new CodexExecution(
269+
createHost(),
270+
runtime,
271+
createPathNativeSessionCodec('codex'),
272+
createConfig(),
273+
);
274+
const bus = new AgentEventBus({ list: () => [{ execution }] });
275+
const predecessor = startRequest().operation;
276+
const rejected = { ...predecessor, clientRequestId: 'request-rejected', turnId: 'turn-rejected' };
277+
const successor = { ...predecessor, clientRequestId: 'request-b', turnId: 'turn-b' };
278+
const attempt = new QueueExecutionAttempt(predecessor);
279+
const successorAbortable = mock(() => undefined);
280+
let runtimeMetadata = {
281+
clientRequestId: predecessor.clientRequestId,
282+
turnId: predecessor.turnId,
283+
};
284+
const trackedMessages = [];
285+
const routedMessages = [];
286+
execution.subscribe((event) => {
287+
if (event.type === 'messages') {
288+
trackedMessages.push({ content: event.messages[0].content, turnId: event.operation.turnId });
289+
}
290+
});
291+
bus.onMessages((_chatId, messages, metadata) => {
292+
routedMessages.push({ content: messages[0].content, turnId: metadata.turnId });
293+
});
294+
const emitOutput = (content) => runtime.emitMessages(
295+
'chat-1',
296+
[new AssistantMessage('2026-07-24T00:00:00.000Z', content)],
297+
runtimeMetadata,
298+
);
299+
const assertOwner = (turnId, content) => {
300+
expect(runtimeMetadata.turnId).toBe(turnId);
301+
expect(attempt.identity().turnId).toBe(turnId);
302+
expect(bus.getActiveTurn('chat-1').turnId).toBe(turnId);
303+
expect(trackedMessages.at(-1)).toEqual({ content, turnId });
304+
expect(routedMessages.at(-1)).toEqual({ content, turnId });
305+
};
306+
const composeHandoff = (operationHandoff, next) => attempt.handoffTurn(
307+
predecessor,
308+
next,
309+
bus.handoffTurn('chat-1', predecessor, next, operationHandoff),
310+
);
311+
312+
bus.trackTurn('chat-1', predecessor);
313+
bus.markTurnAbortable('chat-1', predecessor);
314+
attempt.markAbortable();
315+
await execution.start(startRequest());
316+
runtime.submitActiveInput.mockImplementation(async (request, beforeDelivery) => {
317+
await beforeDelivery({
318+
validate: () => undefined,
319+
commit: () => {
320+
runtimeMetadata = {
321+
clientRequestId: request.clientRequestId,
322+
turnId: request.turnId,
323+
};
324+
request.onAbortable?.();
325+
},
326+
});
327+
return true;
328+
});
329+
330+
await expect(execution.submitActiveInput(activeInputRequest(rejected, async (operationHandoff) => {
331+
const handoff = composeHandoff(operationHandoff, rejected);
332+
handoff.validate();
333+
await Promise.resolve();
334+
emitOutput('A while persistence fails');
335+
assertOwner('turn-1', 'A while persistence fails');
336+
throw new Error('persistence failed');
337+
}))).rejects.toThrow('persistence failed');
338+
emitOutput('A after persistence failed');
339+
assertOwner('turn-1', 'A after persistence failed');
340+
341+
await expect(execution.submitActiveInput({
342+
...activeInputRequest(successor, async (operationHandoff) => {
343+
const handoff = composeHandoff(operationHandoff, successor);
344+
handoff.validate();
345+
await Promise.resolve();
346+
emitOutput('A before persistence commits');
347+
assertOwner('turn-1', 'A before persistence commits');
348+
handoff.validate();
349+
handoff.commit();
350+
emitOutput('B after the atomic commit');
351+
assertOwner('turn-b', 'B after the atomic commit');
352+
}),
353+
admission: {
354+
signal: new AbortController().signal,
355+
markStarted: mock(() => undefined),
356+
markAbortable: successorAbortable,
357+
},
358+
})).resolves.toBe(true);
359+
expect(successorAbortable).toHaveBeenCalledTimes(1);
360+
await expect(bus.waitUntilTurnAbortable('chat-1', successor)).resolves.toBe(true);
361+
});
255362
});

server-agents/codex/src/agents/codex/app-server/__tests__/app-server.test.js

Lines changed: 94 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3780,6 +3780,87 @@ describe('CodexAppServerRuntime', () => {
37803780
expect(fake.shutdown).not.toHaveBeenCalled();
37813781
});
37823782

3783+
it('transfers an already-abortable active session to the successor callback at commit', async () => {
3784+
let fake;
3785+
fake = new FakeClient({
3786+
getThreadGoal: async () => ({ goal: null }),
3787+
setThreadGoal: async (threadId, params) => {
3788+
queueMicrotask(() => fake.emit('notification', {
3789+
method: 'turn/started',
3790+
params: { threadId, turn: makeTurn({ id: 'goal-turn', status: 'inProgress' }) },
3791+
}));
3792+
return { goal: makeGoal(threadId, params.objective) };
3793+
},
3794+
});
3795+
const provider = new CodexAppServerRuntime({ createClient: () => fake });
3796+
const predecessorAbortable = mock(() => undefined);
3797+
const successorAbortable = mock(() => undefined);
3798+
await provider.runTurn(makeRequest({
3799+
agentSessionId: 'thread-1',
3800+
codexGoalCommand: { kind: 'set', objective: 'Long-running work' },
3801+
nativePath: null,
3802+
onAbortable: predecessorAbortable,
3803+
}));
3804+
expect(predecessorAbortable).toHaveBeenCalledTimes(1);
3805+
3806+
await provider.submitActiveInput(makeRequest({
3807+
agentSessionId: 'thread-1',
3808+
command: 'Take ownership now',
3809+
nativePath: null,
3810+
onAbortable: successorAbortable,
3811+
}), async (handoff) => {
3812+
expect(successorAbortable).not.toHaveBeenCalled();
3813+
handoff.validate();
3814+
handoff.commit();
3815+
expect(successorAbortable).toHaveBeenCalledTimes(1);
3816+
});
3817+
});
3818+
3819+
it('targets a successor when the provider becomes abortable after handoff commit', async () => {
3820+
const turnStarted = createDeferred();
3821+
const fake = new FakeClient({
3822+
getThreadGoal: async () => ({
3823+
goal: makeGoal('thread-1', 'Restored work'),
3824+
}),
3825+
steerTurn: async () => ({ turnId: 'continuation-turn' }),
3826+
});
3827+
const provider = new CodexAppServerRuntime({ createClient: () => fake });
3828+
const predecessorAbortable = mock(() => undefined);
3829+
const successorAbortable = mock(() => undefined);
3830+
await provider.runTurn(makeRequest({
3831+
agentSessionId: 'thread-1',
3832+
command: '/goal status',
3833+
codexGoalCommand: { kind: 'status' },
3834+
nativePath: null,
3835+
onAbortable: predecessorAbortable,
3836+
}));
3837+
expect(predecessorAbortable).not.toHaveBeenCalled();
3838+
3839+
const delivery = provider.submitActiveInput(makeRequest({
3840+
agentSessionId: 'thread-1',
3841+
command: 'Continue after restoration',
3842+
nativePath: null,
3843+
onAbortable: successorAbortable,
3844+
}), async (handoff) => {
3845+
handoff.validate();
3846+
handoff.commit();
3847+
turnStarted.resolve();
3848+
});
3849+
await turnStarted.promise;
3850+
expect(successorAbortable).not.toHaveBeenCalled();
3851+
fake.emit('notification', {
3852+
method: 'turn/started',
3853+
params: {
3854+
threadId: 'thread-1',
3855+
turn: makeTurn({ id: 'continuation-turn', status: 'inProgress' }),
3856+
},
3857+
});
3858+
3859+
await expect(delivery).resolves.toBe(true);
3860+
expect(predecessorAbortable).not.toHaveBeenCalled();
3861+
expect(successorAbortable).toHaveBeenCalledTimes(1);
3862+
});
3863+
37833864
it('reconciles active input through the delivered payload and native history loader', async () => {
37843865
const content = 'Preserve active input & literal markup <exactly>';
37853866
const nativePath = path.join(tmpDir, 'active-input.jsonl');
@@ -3819,12 +3900,14 @@ describe('CodexAppServerRuntime', () => {
38193900
command: content,
38203901
clientMessageId: 'message-steer',
38213902
nativePath,
3822-
}), async () => {
3903+
}), async (handoff) => {
38233904
await pendingInputs.register('chat-1', content, {
38243905
clientRequestId: 'request-steer',
38253906
clientMessageId: 'message-steer',
38263907
createdAt: '2026-06-01T00:00:00.000Z',
38273908
});
3909+
handoff.validate();
3910+
handoff.commit();
38283911
})).resolves.toBe(true);
38293912

38303913
expect(pendingInputs.listForChat('chat-1')).toHaveLength(1);
@@ -4364,7 +4447,11 @@ describe('CodexAppServerRuntime', () => {
43644447
agentSessionId: 'thread-1',
43654448
command: `Deliver after ${turnKind}`,
43664449
nativePath: null,
4367-
}), async () => { accepted = true; });
4450+
}), async (handoff) => {
4451+
accepted = true;
4452+
handoff.validate();
4453+
handoff.commit();
4454+
});
43684455
await nonSteerableRejected;
43694456
expect(accepted).toBe(true);
43704457
fake.emit('notification', {
@@ -4465,7 +4552,7 @@ describe('CodexAppServerRuntime', () => {
44654552
expect(fake.startTurn).not.toHaveBeenCalled();
44664553
});
44674554

4468-
it('adopts active-input metadata before flushing a terminal finish pending at delivery', async () => {
4555+
it('keeps predecessor metadata when persistence fails with a terminal finish pending', async () => {
44694556
let fake;
44704557
fake = new FakeClient({
44714558
getThreadGoal: async () => ({ goal: null }),
@@ -4505,14 +4592,15 @@ describe('CodexAppServerRuntime', () => {
45054592
error: { message: 'terminal failure', codexErrorInfo: null, additionalDetails: null },
45064593
},
45074594
});
4595+
throw new Error('registration failed');
45084596
});
4509-
await expect(first).rejects.toThrow('terminal failure');
4597+
await expect(first).rejects.toThrow('registration failed');
45104598
await expect(failed).resolves.toEqual({
45114599
chatId: 'chat-1',
45124600
message: 'terminal failure',
45134601
metadata: {
4514-
clientRequestId: 'request-b',
4515-
turnId: 'turn-b',
4602+
clientRequestId: 'request-a',
4603+
turnId: 'turn-a',
45164604
},
45174605
});
45184606
let accepted = false;

server-agents/codex/src/agents/codex/app-server/runtime.ts

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { AssistantMessage, ErrorMessage, PermissionCancelledMessage, PermissionR
22
import { promises as fs } from 'fs';
33
import { AgentEventEmitterRuntime } from '@garcon/server-agent-common/shared/event-emitter-runtime';
44
import type { RuntimeEventMetadata } from '@garcon/server-agent-common/shared/event-emitter-runtime';
5-
import type { AgentLogger } from '@garcon/server-agent-interface';
5+
import type { AgentActiveInputHandoff, AgentLogger } from '@garcon/server-agent-interface';
66
import { CodexHistoryService } from '../history-source.js';
77
import {
88
assertCodexExecutionOpen,
@@ -374,7 +374,10 @@ export class CodexAppServerRuntime extends AgentEventEmitterRuntime {
374374

375375
async submitActiveInput(
376376
request: CodexResumeRequest,
377-
beforeDelivery: () => Promise<void> = async () => {},
377+
beforeDelivery: (handoff: AgentActiveInputHandoff) => Promise<void> = async (handoff) => {
378+
handoff.validate();
379+
handoff.commit();
380+
},
378381
): Promise<boolean> {
379382
const session = this.#sessions.get(request.agentSessionId);
380383
if (!session || session.status === 'completed' || session.status === 'failed' || session.status === 'aborted') {
@@ -392,8 +395,27 @@ export class CodexAppServerRuntime extends AgentEventEmitterRuntime {
392395
) return false;
393396
session.activeDeliveryReservations += 1;
394397
try {
395-
await beforeDelivery();
396-
session.eventMetadata = codexEventMetadata(request);
398+
const validate = () => {
399+
if (
400+
this.#sessions.get(request.agentSessionId) !== session
401+
|| hasTerminalPendingFinish(session)
402+
|| isTerminalSessionStatus(session.status)
403+
) {
404+
throw new Error(session.pendingFinish?.failedMessage ?? 'Codex session ended before active input delivery');
405+
}
406+
};
407+
let committed = false;
408+
validate();
409+
await beforeDelivery({
410+
validate,
411+
commit: () => {
412+
session.eventMetadata = codexEventMetadata(request);
413+
session.onAbortable = request.onAbortable;
414+
committed = true;
415+
if (session.abortableNotified) session.onAbortable?.();
416+
},
417+
});
418+
if (!committed) throw new Error('Codex active input handoff was not committed');
397419
if (hasTerminalPendingFinish(session) || isTerminalSessionStatus(session.status)) {
398420
throw new Error(session.pendingFinish?.failedMessage ?? 'Codex session ended before active input delivery');
399421
}
@@ -457,6 +479,7 @@ export class CodexAppServerRuntime extends AgentEventEmitterRuntime {
457479
const turn = await session.client.startTurn(startParams);
458480
if (!this.#canApplyTurnAttempt(session, turnAttemptGeneration)) return;
459481
session.activeTurnId = turn.turn.id;
482+
this.#notifyAbortable(session);
460483
return;
461484
} catch (error) {
462485
const isTurnTransition = isActiveTurnConflictError(error) || isActiveTurnNotSteerableError(error);

server-agents/codex/src/agents/codex/execution.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,12 +106,12 @@ export class CodexExecution implements AgentExecution {
106106
);
107107
return this.runtime.submitActiveInput(
108108
runtimeRequest,
109-
() => this.#operations.handoff(
109+
(handoff) => request.beforeDelivery(this.#operations.handoff(
110110
request.chatId,
111111
predecessor,
112112
request.operation,
113-
request.beforeDelivery,
114-
),
113+
handoff,
114+
)),
115115
);
116116
}
117117

0 commit comments

Comments
 (0)