Repository navigation
fix(server): Codex turns started by codex queue appear in the thread #34930
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
base: main
Are you sure you want to change the base?
Changes from all commits
6077ab4
ffac496
da875b9
22d338c
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 |
|---|---|---|
|
|
@@ -1224,6 +1224,8 @@ interface ActiveCodexTurnContext { | |
| hookStopReason?: string; | ||
| capacityRetryAttempt?: number; | ||
| capacityRetryPending?: Deferred.Deferred<ActiveCodexTurnContext | undefined>; | ||
| /** Codex started this root turn from its own queue, so its prompt is not a T3 message. */ | ||
| startedByCodex?: boolean; | ||
| closedCommandItemIds?: ReadonlySet<string>; | ||
| latestProviderFailure?: { | ||
| readonly nativeMessage: string; | ||
|
|
@@ -1872,14 +1874,46 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( | |
| }), | ||
| ), | ||
| ); | ||
| const client = yield* clientFactory.open({ | ||
| const openedClient = yield* clientFactory.open({ | ||
| instanceId: adapterOptions.instanceId, | ||
| threadId: input.threadId, | ||
| providerSessionId: input.providerSessionId, | ||
| runtimePolicy: input.runtimePolicy, | ||
| settings: resolvedRuntime?.config ?? adapterOptions.settings, | ||
| environment: resolvedRuntime?.environment ?? adapterOptions.environment, | ||
| }); | ||
| // Root turns Codex started on its own (`codex queue`) wait here for T3 to open a run. | ||
| // Their notifications are held in order and replayed once the run owns the turn. | ||
| const codexStartedTurns = new Map< | ||
| string, | ||
| { | ||
| readonly nativeThreadId: string; | ||
| readonly nativeTurnId: string; | ||
| readonly startedAt: DateTime.Utc; | ||
| readonly held: Array<Effect.Effect<void, CodexErrors.CodexAppServerError>>; | ||
| readonly registered: Deferred.Deferred<boolean>; | ||
| } | ||
| >(); | ||
| const client: typeof openedClient = { | ||
| ...openedClient, | ||
| handleServerNotification: (method, handler) => | ||
| openedClient.handleServerNotification(method, (payload) => { | ||
| const turnId = | ||
| "turnId" in payload && typeof payload.turnId === "string" | ||
| ? payload.turnId | ||
| : "turn" in payload && | ||
| typeof payload.turn === "object" && | ||
| payload.turn !== null && | ||
| "id" in payload.turn && | ||
| typeof payload.turn.id === "string" | ||
| ? payload.turn.id | ||
| : undefined; | ||
| const waiting = turnId === undefined ? undefined : codexStartedTurns.get(turnId); | ||
| if (waiting === undefined) return handler(payload); | ||
|
Comment on lines
+1911
to
+1912
Contributor
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. Hold native subagent events until their parent replay registers them. If a queued root turn spawns a native subagent before adoption, the parent's spawn notification is held here. The child's turn ID is not in The child's Retain child notifications until subagent registration, including child completion. Cover a child that finishes before root adoption.
|
||
| waiting.held.push(handler(payload)); | ||
| return Effect.void; | ||
| }), | ||
| }; | ||
| const additionalContextByThread = yield* Ref.make( | ||
| new Map<string, NonNullable<CodexTurnStartParams["additionalContext"]>>(), | ||
| ); | ||
|
|
@@ -2432,6 +2466,12 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( | |
| if (context !== undefined || attemptsRemaining <= 0) { | ||
| return context; | ||
| } | ||
| // Approvals for a queue-started turn wait for its run; held notifications never reach here. | ||
| const codexStarted = codexStartedTurns.get(nativeTurnId); | ||
| if (codexStarted !== undefined) { | ||
| yield* Deferred.await(codexStarted.registered); | ||
| return (yield* Ref.get(activeTurns)).get(nativeTurnId); | ||
| } | ||
| yield* Effect.yieldNow; | ||
| return yield* awaitActiveTurn(nativeTurnId, attemptsRemaining - 1); | ||
| }); | ||
|
|
@@ -2447,6 +2487,68 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( | |
| return context === undefined ? undefined : ({ context, settled: false } as const); | ||
| }); | ||
|
|
||
| const releaseCodexStartedTurn = Effect.fnUntraced(function* (nativeTurnId: string) { | ||
| const waiting = codexStartedTurns.get(nativeTurnId); | ||
| if (waiting === undefined) return; | ||
| codexStartedTurns.delete(nativeTurnId); | ||
| yield* Deferred.succeed(waiting.registered, false); | ||
|
Comment on lines
+2493
to
+2494
Contributor
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. Handle the native turn when an unstarted continuation is released. When a queued continuation ends before Decide how the release path handles the native turn before it discards the record. Either send
|
||
| }); | ||
|
|
||
| const awaitRunForCodexStartedTurn = Effect.fn("CodexAdapterV2.awaitRunForCodexStartedTurn")( | ||
| function* (input: { | ||
| readonly appThreadId: ThreadId; | ||
| readonly providerThread: OrchestrationV2ProviderThread; | ||
| readonly nativeThreadId: string; | ||
| readonly nativeTurnId: string; | ||
| readonly startedAt: DateTime.Utc; | ||
| }) { | ||
| if (continuationRequests === undefined) return; | ||
| codexStartedTurns.set(input.nativeTurnId, { | ||
| nativeThreadId: input.nativeThreadId, | ||
| nativeTurnId: input.nativeTurnId, | ||
| startedAt: input.startedAt, | ||
| held: [], | ||
| registered: yield* Deferred.make<boolean>(), | ||
| }); | ||
|
Comment on lines
+2506
to
+2512
Contributor
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. Count pending Codex turns in both background-work probes. A queue-started turn stays in Suggested fix
|
||
| yield* continuationRequests.offer({ | ||
| threadId: input.appThreadId, | ||
| providerThreadId: input.providerThread.id, | ||
| driver: CODEX_PROVIDER, | ||
| detail: null, | ||
| clearIfCurrent: () => releaseCodexStartedTurn(input.nativeTurnId), | ||
| onRunEndedWithoutStarting: () => releaseCodexStartedTurn(input.nativeTurnId), | ||
| }); | ||
| }, | ||
| ); | ||
|
|
||
| /** Gives a queue-started Codex turn to the continuation run T3 opened for it. */ | ||
| const adoptCodexStartedTurn = Effect.fn("CodexAdapterV2.adoptCodexStartedTurn")(function* ( | ||
| turnInput: ProviderAdapter.ProviderAdapterV2TurnInput, | ||
| ) { | ||
| if ( | ||
| turnInput.message.createdBy !== "agent" || | ||
| turnInput.message.creationSource !== "provider" | ||
| ) | ||
| return false; | ||
| const nativeThreadId = turnInput.providerThread.nativeThreadRef?.nativeId; | ||
| const waiting = [...codexStartedTurns.values()].find( | ||
| (candidate) => candidate.nativeThreadId === nativeThreadId, | ||
| ); | ||
| if (waiting === undefined || (yield* Deferred.isDone(waiting.registered))) return false; | ||
| const context = yield* registerRootTurn({ | ||
| turnInput, | ||
| nativeTurnId: waiting.nativeTurnId, | ||
| startedAt: waiting.startedAt, | ||
| }); | ||
| context.startedByCodex = true; | ||
| yield* Deferred.succeed(waiting.registered, true); | ||
|
Contributor
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. Signal approval waiters after the held replay completes. For a queue-started turn, Approval requests use Release the waiters after the replay. Use a finalizer so waiters still resume if the replay is interrupted: Suggested fix
|
||
| while (waiting.held.length > 0) { | ||
| yield* waiting.held.shift()!.pipe(Effect.ignore); | ||
|
Contributor
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. Drain held notifications without repeated A long turn can accumulate many token-delta notifications while its continuation waits. Removing each entry from the front of an array makes replay quadratic in the number of held notifications. Use an index or a deque while preserving the order of notifications that arrive during replay. (raw.githubusercontent.com)
|
||
| } | ||
| codexStartedTurns.delete(waiting.nativeTurnId); | ||
|
Contributor
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. Keep the started-turn entry consistent when replay is interrupted. Line 2459 removes A cleanup that deletes the entry in a finalizer must also handle notifications that have not replayed yet. Deleting the entry first would drop them, and leaving the entry in place keeps the stuck state. The correct handling of those notifications is not established by the inspected code, so this comment gives no patch.
|
||
| return true; | ||
| }); | ||
|
|
||
| const trackRunningCommandItem = (nativeTurnId: string, item: TrackedRunningCommandItem) => | ||
| Ref.update(runningCommandItemsByTurn, (current) => { | ||
| const updated = new Map(current); | ||
|
|
@@ -3838,7 +3940,11 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( | |
| >, | ||
| nativeStartedAt?: DateTime.Utc, | ||
| ) { | ||
| if (context.subagent === null || context.providerTurnOrdinal === 1) { | ||
| if ( | ||
| context.subagent === null | ||
| ? context.startedByCodex !== true | ||
| : context.providerTurnOrdinal === 1 | ||
| ) { | ||
| return false; | ||
| } | ||
| const text = codexUserMessageText(item.content); | ||
|
|
@@ -3851,7 +3957,9 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( | |
| nativeStartedAt, | ||
| ); | ||
| const artifacts = makeSubagentConversationArtifacts({ | ||
| senderThreadId: context.subagent.parentContext.projectionThreadId, | ||
| ...(context.subagent === null | ||
| ? {} | ||
| : { senderThreadId: context.subagent.parentContext.projectionThreadId }), | ||
| messageId: idAllocator.derive.messageFromProviderItem({ | ||
| driver: CODEX_PROVIDER, | ||
| nativeItemId: item.id, | ||
|
|
@@ -3870,15 +3978,16 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( | |
| ordinal, | ||
| now: startedAt, | ||
| }); | ||
| const runId = context.subagent === null ? context.projectionRunId : null; | ||
| yield* emitProviderEvent({ | ||
| type: "message.updated", | ||
| driver: CODEX_PROVIDER, | ||
| message: artifacts.message, | ||
| message: { ...artifacts.message, runId }, | ||
| }); | ||
| yield* emitProviderEvent({ | ||
| type: "turn_item.updated", | ||
| driver: CODEX_PROVIDER, | ||
| turnItem: artifacts.turnItem, | ||
| turnItem: { ...artifacts.turnItem, runId }, | ||
| }); | ||
| return true; | ||
| }); | ||
|
|
@@ -5026,6 +5135,22 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( | |
| ); | ||
| return; | ||
| } | ||
| const rootProviderThread = rootProviderThreads.get(payload.threadId); | ||
| if ( | ||
| continuationRequests !== undefined && | ||
| rootProviderThread?.appThreadId != null && | ||
| !(yield* Ref.get(subagentThreads)).has(payload.threadId) && | ||
| (yield* findActiveTurnByNativeThreadId(payload.threadId)) === undefined | ||
|
Contributor
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. Keep subsequent queued root turns pending during replay. If Codex starts the next queued turn while adoption replays the previous turn's held notifications, Retain subsequent root turns during replay and serialize their adoption. Add this sequence to the replay regression test.
|
||
| ) { | ||
| yield* awaitRunForCodexStartedTurn({ | ||
|
Contributor
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. Wait for a pending queue-started turn in the legacy approval handlers.
The legacy requests carry Suggested fix
|
||
| appThreadId: rootProviderThread.appThreadId, | ||
| providerThread: rootProviderThread, | ||
| nativeThreadId: payload.threadId, | ||
| nativeTurnId: payload.turn.id, | ||
| startedAt: codexTimestamp(payload.turn.startedAt), | ||
| }); | ||
| return; | ||
| } | ||
| yield* rememberSubagentTurnStarted({ | ||
| nativeThreadId: payload.threadId, | ||
| nativeTurnId: payload.turn.id, | ||
|
|
@@ -7814,6 +7939,7 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( | |
| if (goalCommand !== null) return runGoalCommand(turnInput, goalCommand); | ||
| const nativeThreadId = turnInput.providerThread.nativeThreadRef?.nativeId; | ||
| return Effect.gen(function* () { | ||
| if (yield* adoptCodexStartedTurn(turnInput)) return; | ||
| if ( | ||
| turnInput.restartContinuationOfRunId === undefined && | ||
| nativeThreadId != null && | ||
|
|
||
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.
🟠 High
Adapters/CodexAdapterV2.ts:1819When a queue-started root turn spawns a subagent before the continuation run is adopted, the child’s notifications are processed immediately while the parent’s spawn notification is held. The child’s item events and
turn/completedare then dropped because no active child context exists; replaying only the saved start leaves the child turn running with missing messages and tools. The hold lookup atcodexStartedTurns.get(turnId)covers only the root turn ID, so buffer descendant notifications too or register the child’s ownership before holding the parent events.🚀 Reply "fix it for me" or copy this AI Prompt for your agent: