Skip to content

Fix repeated steering and reliable stop - #3903

Closed
Quicksaver wants to merge 61 commits into
pingdotgg:mainfrom
Quicksaver:fix/repeated-steering-and-stop
Closed

Fix repeated steering and reliable stop#3903
Quicksaver wants to merge 61 commits into
pingdotgg:mainfrom
Quicksaver:fix/repeated-steering-and-stop

Conversation

@Quicksaver

@Quicksaver Quicksaver commented Jul 12, 2026

Copy link
Copy Markdown

Summary

Allow users to send consecutive steering prompts while a conversation is running and reliably stop the active Codex turn after any number of steers. Steering dispatches clear as soon as the server projects the exact submitted user message, even when the frozen dispatch snapshot has stale or absent session status, while root interruption resolves the provider's live active turn instead of relying on potentially stale projected state.

Explicit child-turn interruptions continue to target the requested child directly. Root lookup is bounded and failure-tolerant: typed failures, timeouts, and unexpected defects fall back to the cached active turn, while a successful read with no active turn suppresses a stale interrupt.

What Changed

  • Track the expected user-message id for each local dispatch and acknowledge only that exact projected message, independently of the frozen dispatch snapshot's session status.
  • Wire generated message ids through new-thread, follow-up, and in-turn steering send paths so the composer can re-enable after an acknowledged steer without waiting for a turn or session transition.
  • Resolve root Codex interruptions from a live thread/read request with turns included, selecting the newest in-progress turn by timestamp and falling back to provider response order when timestamps are absent.
  • Honor explicitly requested child turn ids directly without performing root active-turn lookup or fallback.
  • Bound active-turn lookup time, log typed failures and unexpected defects, and fall back to the cached session turn only when the lookup fails.
  • Suppress stale interrupts when a successful live lookup reports no active turn.
  • Add focused regression coverage for consecutive steering, exact-message acknowledgement, timestamp and response-order turn selection, timeout/failure/defect fallback, and successful empty live reads.

Why

Running conversations could leave the composer busy after steering because acknowledgement depended on turn or session transitions that do not occur for an in-turn steer. After steering, stop could also target a stale cached turn rather than the provider's actual active turn. Together, these behaviors prevented repeated steering and made interruption unreliable precisely when users needed it most.

Validation

  • pnpm exec vp test run apps/web/src/components/ChatView.logic.test.ts apps/server/src/provider/Layers/CodexInterruptResolution.test.ts apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts (83 tests passed)
  • pnpm exec vp fmt --check BRANCH_DETAILS.md apps/web/src/components/ChatView.tsx apps/web/src/components/ChatView.logic.ts apps/web/src/components/ChatView.logic.test.ts apps/server/src/provider/Layers/CodexSessionRuntime.ts apps/server/src/provider/Layers/CodexSessionRuntime.test.ts apps/server/src/provider/Layers/CodexInterruptResolution.test.ts
  • pnpm exec vp lint apps/web/src/components/ChatView.tsx apps/web/src/components/ChatView.logic.ts apps/web/src/components/ChatView.logic.test.ts apps/server/src/provider/Layers/CodexSessionRuntime.ts apps/server/src/provider/Layers/CodexSessionRuntime.test.ts apps/server/src/provider/Layers/CodexInterruptResolution.test.ts
  • git diff --check upstream/main...HEAD
  • Playwright on isolated ports 5738/13778: two consecutive exact-id steers each re-enabled the composer while the turn remained running, and Stop after two later steers settled the thread without a stale completion reply.

Proof

  • Automated regression coverage in the changed web and server test suites.

Note

Medium Risk
Changes live conversation send/stop behavior and Codex interrupt targeting with timeout/fallback paths; well-covered by tests but affects core orchestration UX when projections or provider state diverge.

Overview
Fixes repeated in-turn steering and reliable Stop by correlating local “sending” state to the exact outbound user message id and resolving Codex interrupts against the provider’s live active turn.

On the web client, each dispatch records an expectedUserMessageId (from newMessageId() before beginLocalDispatch) and hasServerAcknowledgedLocalDispatch clears the busy composer only when projected thread messages contain that id—not when any user message appears or turn/session fields lag during steers. Consecutive steers require acknowledgement of the next id.

On the server, root interruptTurn without an explicit turn id calls resolveCodexInterruptTurnId, which **thread/read**s with includeTurns: true, picks the newest inProgress turn (timestamp order, provider order when startedAt is missing), applies a 2s timeout, and falls back to the session’s projected activeTurnId only when lookup fails. A successful read with no active turn returns without interrupting, avoiding stale cached ids. Explicit child turn ids still pass through unchanged.

Adds BRANCH_DETAILS.md and regression tests in ChatView.logic.test.ts and CodexInterruptResolution.test.ts.

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

Note

Fix repeated steering acknowledgement and reliable turn interrupt resolution

  • Replaces the heuristic "any latest user message change" acknowledgement logic with exact user message id correlation: a local dispatch is now acknowledged only when the projected messages contain the specific message id generated at send time.
  • Adds resolveCodexInterruptTurnId to read the live provider thread (includeTurns: true) and select the most recent inProgress turn, bounded by a 2-second timeout, instead of relying on a cached activeTurnId.
  • On timeout, failure, or defect, the interrupt falls back to the session's projected active turn id; on a successful read with no active turn, the interrupt is skipped rather than reviving a stale id.
  • Behavioral Change: LocalDispatchSnapshot fields are renamed (latestUserMessageIdexpectedUserMessageId, latestTurnTurnIdlatestTurnId); callers must pass projectedMessages instead of latestUserMessageId to hasServerAcknowledgedLocalDispatch.

Macroscope summarized 0517393.

- Resolve the provider runtime’s authoritative active Codex turn
- Treat projected steer messages as server dispatch acknowledgement
- Avoid stale root turn IDs when interrupting for steering
- Cover active-turn lookup and running-thread steer behavior
- Resolve active Codex turns from timed, observable thread reads
- Match projected steer acknowledgements to the dispatched message
- Add realistic regression coverage for interrupt and steer paths
@coderabbitai

coderabbitai Bot commented Jul 12, 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 Plus

Run ID: a3309e26-1573-4b1e-af1a-88ae5e6aaba2

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 12, 2026
Comment thread apps/server/src/provider/Layers/CodexSessionRuntime.ts
@macroscopeapp

macroscopeapp Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

This PR introduces new interrupt resolution logic that queries the live provider thread with timeout/fallback, and changes steering acknowledgement from detecting any user message change to exact message ID matching. These are significant behavioral changes to core real-time agent interaction features that warrant human review.

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

- Fall back to provider response order when start times are absent
- Cover both mixed timestamp ordering cases
- Merge upstream/main through Android mobile support
- Keep Quicksaver Expo ownership in the mobile config
- Preserve repeated steering and live Codex interruption behavior
- Extract and test active turn ordering and interrupt fallbacks
Comment thread apps/mobile/app.config.ts
- Use the Quicksaver EAS project id for the OTA endpoint
Comment thread apps/mobile/app.config.ts Outdated
- Capture exact message acknowledgement and interrupt routing contracts
- Record Codex live-turn selection and fallback safeguards
- Reserve branch-specific web and server development ports
…ng-and-stop

# Conflicts:
#	apps/server/src/provider/Layers/CodexSessionRuntime.test.ts
#	apps/web/src/components/ChatView.logic.ts
#	apps/web/src/components/ChatView.tsx
- Match dispatched steering IDs across all projected user messages
- Read the session fallback only after live Codex lookup failures
- Cover multi-client steering and timeout races with regression tests
- Make timeout tests wait for the live lookup explicitly
- Clarify replacement and dispatch snapshot naming
- Scan projected messages from the latest entry first
- Record assessment fixes and review outcomes
- Update validation counts and branch divergence
- Document conflict-resolution guidance for web and Codex runtime changes
- Preserve dispatch-correlation and interruption-resolution coverage
- Enable the global virtual store
- Correct prerelease package extensions and refresh the lockfile
- Match exact projected messages independently of snapshot status
- Cover ready and missing dispatch session status regressions
- Document authoritative exact-id acknowledgement
Comment thread apps/server/src/provider/Layers/CodexSessionRuntime.ts
Quicksaver and others added 22 commits July 24, 2026 18:49
- Log unexpected live thread lookup defects
- Fall back to the latest projected active turn
- Cover defect recovery in focused interrupt tests
@juliusmarminge

Copy link
Copy Markdown
Member

Closing in favor of #2829 (orchestration V2).

#2829 deletes the V1 orchestration layer this PR builds on — apps/server/src/orchestration/**, provider/Layers/*Adapter.ts and provider/Services/** are removed and replaced by apps/server/src/orchestration-v2/**, with the IPC surface renamed to ORCHESTRATION_V2_WS_METHODS. The files this PR touches either no longer exist or are rewritten, so it can't be rebased — it would need reimplementing against the V2 adapters.

This is not a judgement on the change itself. Several of these are real gaps we still want fixed; the base just moved out from under them.

Once #2829 merges, please rebase onto main, port the change to the V2 equivalent, and reopen (or open a fresh PR). Ping me and I'll prioritise the review.

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.

2 participants