-
Notifications
You must be signed in to change notification settings - Fork 0
fix(server): keep codex turn ids aligned #106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9674cdd
009bff1
92b925d
5657b35
86f870d
322717d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -902,6 +902,11 @@ interface PendingCodexSubagentTurnStarted { | |
| readonly startedAt: DateTime.Utc; | ||
| } | ||
|
|
||
| interface PendingCodexRootTurn { | ||
| readonly turnInput: ProviderAdapterV2TurnInput; | ||
| readonly started: Deferred.Deferred<ActiveCodexTurnContext, never>; | ||
| } | ||
|
|
||
| type PendingCodexRuntimeRequest = | ||
| | { | ||
| readonly type: "approval"; | ||
|
|
@@ -1473,7 +1478,7 @@ export function makeCodexAdapterV2(adapterOptions: CodexAdapterV2Options): Provi | |
| }); | ||
| const events = yield* Queue.unbounded<ProviderAdapterV2Event>(); | ||
| const activeTurns = yield* Ref.make(new Map<string, ActiveCodexTurnContext>()); | ||
| const pendingRootTurns = yield* Ref.make(new Map<string, ProviderAdapterV2TurnInput>()); | ||
| const pendingRootTurns = yield* Ref.make(new Map<string, PendingCodexRootTurn>()); | ||
| const turnWaiters = yield* Ref.make(new Map<string, Deferred.Deferred<void, never>>()); | ||
| const subagentThreads = yield* Ref.make(new Map<string, CodexSubagentThreadContext>()); | ||
| const pendingSubagentTurns = yield* Ref.make( | ||
|
|
@@ -3215,11 +3220,12 @@ export function makeCodexAdapterV2(adapterOptions: CodexAdapterV2Options): Provi | |
| } | ||
| const pendingRootTurn = (yield* Ref.get(pendingRootTurns)).get(payload.threadId); | ||
| if (pendingRootTurn !== undefined) { | ||
| yield* registerRootTurn({ | ||
| turnInput: pendingRootTurn, | ||
| const rootTurn = yield* registerRootTurn({ | ||
| turnInput: pendingRootTurn.turnInput, | ||
| nativeTurnId: payload.turn.id, | ||
| startedAt: codexTimestamp(payload.turn.startedAt), | ||
| }); | ||
| yield* Deferred.succeed(pendingRootTurn.started, rootTurn); | ||
| yield* Ref.update(pendingRootTurns, (current) => { | ||
| const updated = new Map(current); | ||
| updated.delete(payload.threadId); | ||
|
|
@@ -4486,20 +4492,21 @@ export function makeCodexAdapterV2(adapterOptions: CodexAdapterV2Options): Provi | |
| hasT3Mcp: | ||
| McpProviderSession.readMcpProviderSession(turnInput.threadId) !== undefined, | ||
| }); | ||
| const turnStarted = yield* Deferred.make<ActiveCodexTurnContext>(); | ||
| yield* Ref.update(pendingRootTurns, (current) => { | ||
| const updated = new Map(current); | ||
| updated.set(threadId, turnInput); | ||
| updated.set(threadId, { turnInput, started: turnStarted }); | ||
| return updated; | ||
| }); | ||
| const started = yield* client.request("turn/start", turnStartParams); | ||
| const nativeTurnId = started.turn.id; | ||
| const startedAt = codexTimestamp(started.turn.startedAt); | ||
| yield* registerRootTurn({ turnInput, nativeTurnId, startedAt }); | ||
| yield* Ref.update(pendingRootTurns, (current) => { | ||
| const updated = new Map(current); | ||
| updated.delete(threadId); | ||
| return updated; | ||
| }); | ||
| const rootTurn = yield* Deferred.await(turnStarted); | ||
| if (started.turn.id !== rootTurn.nativeTurnId) { | ||
| yield* Effect.logWarning("orchestration-v2.codex-turn-id-mismatch", { | ||
| nativeThreadId: threadId, | ||
| responseNativeTurnId: started.turn.id, | ||
| notificationNativeTurnId: rootTurn.nativeTurnId, | ||
| }); | ||
|
Comment on lines
+4503
to
+4508
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This backend behavior change has no focused regression test: the existing replay preamble supplies the same ID in the AGENTS.md reference: AGENTS.md:L110-L110 Useful? React with 👍 / 👎. |
||
| } | ||
| }).pipe( | ||
| Effect.ensuring( | ||
| Effect.flatMap(getNativeThreadId(turnInput.providerThread), (threadId) => | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When Codex returns a successful
turn/startresponse and then exits or emits an invalid/missingturn/startednotification, this local deferred is never completed or failed. The existing replay preamble inCodexAdapterV2.test.tsexplicitly permits the response to arrive before the notification, so an exit in that interval leavesstartTurnsuspended indefinitely, the run stuck during startup, and the provider session marked busy; race this wait against transport termination or otherwise provide a failure path.Useful? React with 👍 / 👎.