Skip to content

fix(server): keep codex turn ids aligned - #106

Merged
tarik02 merged 6 commits into
orchestrator-v2from
fix/codex-turn-id-reconciliation
Aug 1, 2026
Merged

fix(server): keep codex turn ids aligned#106
tarik02 merged 6 commits into
orchestrator-v2from
fix/codex-turn-id-reconciliation

Conversation

@tarik02

@tarik02 tarik02 commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

What Changed

  • Register root turns from the turn/started notification, which carries the ID Codex uses for later events.
  • Wait for that notification before completing turn startup.
  • Log when the turn/start response returns a different ID.

Why

Codex 0.145.0 can return a turn ID from turn/start that differs from the turn/started notification during persistent-goal continuation. The adapter treated the response ID as strong, so it dropped later messages and tool events and sent a stale expectedTurnId when steering.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • UI screenshots are not applicable
  • UI motion video is not applicable

Model: GPT-5.6; harness: T3 Code.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S labels Aug 1, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9674cdd679

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

updated.delete(threadId);
return updated;
});
const rootTurn = yield* Deferred.await(turnStarted);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Stop waiting when Codex exits before turn/started

When Codex returns a successful turn/start response and then exits or emits an invalid/missing turn/started notification, this local deferred is never completed or failed. The existing replay preamble in CodexAdapterV2.test.ts explicitly permits the response to arrive before the notification, so an exit in that interval leaves startTurn suspended 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 👍 / 👎.

Comment on lines +4503 to +4508
if (started.turn.id !== rootTurn.nativeTurnId) {
yield* Effect.logWarning("orchestration-v2.codex-turn-id-mismatch", {
nativeThreadId: threadId,
responseNativeTurnId: started.turn.id,
notificationNativeTurnId: rootTurn.nativeTurnId,
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add coverage for divergent Codex turn IDs

This backend behavior change has no focused regression test: the existing replay preamble supplies the same ID in the turn/start response and turn/started notification, so it cannot prove that provider events and later steering use the notification ID when the two diverge. Add a fixture with distinct IDs and assert that the registered provider turn and expectedTurnId follow the notification.

AGENTS.md reference: AGENTS.md:L110-L110

Useful? React with 👍 / 👎.

@github-actions github-actions Bot added size:M and removed size:S labels Aug 1, 2026
@tarik02
tarik02 merged commit bc35750 into orchestrator-v2 Aug 1, 2026
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant