Skip to content

fix(orchestrator): keep stop and steer responsive#3865

Open
PixPMusic wants to merge 5 commits into
pingdotgg:t3code/codex-turn-mappingfrom
PixPMusic:pixpmusic/fix-stop-steer
Open

fix(orchestrator): keep stop and steer responsive#3865
PixPMusic wants to merge 5 commits into
pingdotgg:t3code/codex-turn-mappingfrom
PixPMusic:pixpmusic/fix-stop-steer

Conversation

@PixPMusic

@PixPMusic PixPMusic commented Jul 10, 2026

Copy link
Copy Markdown

Summary

  • run stop/session controls on a dedicated per-thread lane instead of queuing behind mutations
  • target the known active run directly and report when no interruptible run exists
  • keep Stop available beside steering Send and wait for Codex terminal confirmation with a bounded timeout

Validation

  • vp check
  • vp run typecheck
  • focused web/client tests (23 tests)
  • Codex adapter and replay tests (16 tests)

Addresses #3607 and is stacked on #2829.


Note

Medium Risk
Changes interrupt dispatch, concurrency lanes, and Codex turn-interrupt synchronization; incorrect retry or timeout handling could mis-target runs or surface spurious errors, though behavior is heavily covered by new tests.

Overview
Makes Stop and steer Send usable without waiting behind other thread mutations, and tightens interrupt behavior end-to-end when sends and stops race.

Client runtime: interruptTurn and stopSession run on a per-thread control lane separate from mutation commands. interruptThreadTurn now fails with ThreadTurnNotInterruptibleError when there is no interruptible run (instead of returning a no-op success), skips a projection fetch when the caller passes an explicit runId, and is wrapped by coordinateInterruptWithPendingStarts so a quick Stop after Send retries once after the in-flight startTurn settles.

Codex: Turn completion uses shared settleCodexTurnTerminal / registerCodexTurnTerminalWaiter so interrupt can wait for terminal confirmation (10s cap) without losing to completion-vs-interrupt races.

UI: While a turn is running and the composer has text, Stop stays visible next to steer Send; interrupt includes activeRuntime.activeRunId when known.

Reviewed by Cursor Bugbot for commit ccd1303. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Fix stop and steer responsiveness by coordinating interrupt with pending turn starts

  • Interrupt commands now run on a dedicated control concurrency lane (separate from mutation), preventing them from being blocked by queued sends.
  • Adds coordinateInterruptWithPendingStarts in threadCommands.ts: if an interrupt fails with ThreadTurnNotInterruptibleError while a start is in flight for the same thread, it waits for the start to settle and retries once.
  • The interrupt handler in CodexAdapterV2.ts now registers a terminal waiter, sends turn/interrupt, and waits up to 10 seconds for the turn to reach a terminal state before returning a protocol error on timeout.
  • Stop button in ComposerPrimaryActions.tsx is now always visible when a turn is running, even when the composer has sendable content.
  • Risk: interrupt requests that time out waiting for terminal state now return a protocol error after 10 seconds instead of completing silently.

Macroscope summarized ccd1303.

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 9f216029-5ab6-451c-85d3-41c2d473e4e0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jul 10, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a43c69b. Configure here.

Comment thread apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts
Comment thread apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts Outdated
Comment thread packages/client-runtime/src/state/threadCommands.ts Outdated
@macroscopeapp

macroscopeapp Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

This PR adds significant new concurrency coordination logic for start/interrupt race conditions, introduces new command lanes, error types, and timeout handling across multiple layers. The scope and complexity of these changes - including new control flow patterns and UI behavior changes - warrant human review.

You can customize Macroscope's approvability policy. Learn more.

Interrupting a Codex turn now registers its terminal waiter before
re-checking the active set, and turn completion retires the active turn
before draining waiters, so a completion landing between the interrupt's
lookup and registration resolves immediately instead of timing out.

On the client, an interrupt that finds no active run while a start for
the same thread is still in flight waits for that start to settle and
retries once, so a quick Stop after Send targets the run the send
creates while staying on the fast control lane.
Comment thread packages/client-runtime/src/state/threadCommands.ts Outdated
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Caution

Review failed

An error occurred during the review process. Please try again later.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

The mutation lane is serial per thread, so only the earliest pending
start is executing. Waiting for every captured start made Stop drain
the whole queued send backlog before retrying; retry now fires as soon
as the first start settles.
Comment thread packages/client-runtime/src/state/threadCommands.ts
The dispatch snapshot could capture a start whose settlement cleanup
was still queued, making a Stop with nothing in flight retry against a
later, unrelated start. Snapshot after one microtask yield so earlier
settlements drain first and only genuinely in-flight starts count.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant