[brennanb2025]

	Files	Added	Deleted	Net
Test	3	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​358	$\color{#cf222e}{\Huge{\mathbf{−}}}$​12	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​346
Prod	6	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​122	$\color{#cf222e}{\Huge{\mathbf{−}}}$​32	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​90

What this fixes

The problem. In a Codex structured chat, pressing Stop sends Codex an "interrupt" request for the running turn. Codex answers that request only once the turn has actually ended, so its answer is what confirms the Stop. Two things can go wrong while the turn keeps running:

• Codex never answers. The turn does not end (for example, it is stuck in a tool call), so the interrupt request times out after 30 seconds.
• Codex reports an internal failure. It answers "failed to interrupt turn: …" because it could not pass the interrupt to its own engine. The turn was never interrupted.

In both cases Orca tried nothing else. The turn kept running, the chat showed "Cancellation was not confirmed." or "Codex didn't stop: failed to interrupt turn: …", and the only way to stop the agent was to close the chat. Pressing Stop again hit the same wall.

These cases come from reading Codex's source. We have no captured Orca transcript of either one, and they can't be produced on demand against a live Codex (Visual Proof uses a stand-in Codex server).

Why it happened. A Codex Stop is deliberately just the interrupt. A Stop that Codex accepts keeps the Codex process alive, so the background terminals it started (dev servers, watchers) keep running. That was an earlier decision, and this PR keeps it. But nothing handled an interrupt that failed. Since #24235, a Claude Stop always ends Claude's process after the interrupt. A Codex Stop never did, even when the interrupt failed.

Scope.

• Affected: Codex structured chats, and only when Codex either never answered the Stop's interrupt or reported that it failed internally, while the turn the Stop was for is still running.
• Not affected:
  • A Stop that Codex accepts. It keeps the process and its background terminals, exactly as before.
  • A Stop that Codex refuses because the turn isn't running, for example "no active turn to interrupt" or "expected active turn id X but found Y". It keeps the process and writes today's row: "Codex didn't stop: …" for a Stop that names no turn, and no row for a Stop that names a turn (the rule from fix(native-chat): every chat action press is its own action (re-land #23916 on main) #24301: a Stop that stopped nothing writes nothing).
  • Claude Stops, prompt-card Cancels, and the background-task Stop are unchanged.

ELI5

When you press Stop, Orca asks Codex to stop the turn. If Codex never answers, or says something broke while it tried, the turn used to keep going, and the chat only told you it hadn't stopped. Now Orca ends Codex's process instead, the same way a Claude Stop already does. Your next message starts it again on the same conversation. If Codex says the turn already finished, nothing is ended.

What changes for the user

Before: a Stop whose interrupt Codex never answered, or failed internally, left the turn running and showed "Cancellation was not confirmed." or "Codex didn't stop: …".

After: that Stop ends the Codex process.

• The turn shows as interrupted by you, and the turn gets one "Cancellation requested." row. Pressing Stop on that turn again rewrites the same row instead of adding a second one.
• Your next message starts a new Codex process on the same conversation, the same way it does after an idle chat is put to rest.
• Background terminals that Codex started end along with the process. That is the cost of stopping a turn that Codex would not stop.

Which turn a Stop is about. A Stop only ends the process when the turn it was meant for is still running:

• The desktop Stop button names no turn. It applies to the turn that was running when you pressed it.
• The phone, and older desktop clients, name the turn they show. A Stop that names a turn applies only to that turn.
• If that turn already finished, the process is kept and, as on main since fix(native-chat): every chat action press is its own action (re-land #23916 on main) #24301, the Stop writes no row. This holds even when a newer message has just been handed to Codex and its turn hasn't started yet: a Stop aimed at the finished turn does not end the process that is now working on your next message.

When Codex says the turn isn't running. Codex can send this answer a moment before the message that tells Orca the turn ended. The process is kept, so a Stop pressed just as a turn finishes never kills the process and its background terminals.

When ending the process can't be confirmed. If Orca can't prove that the process exited, it logs the failure and shows "Codex didn't stop: …" (with Codex's reason) or "Cancellation was not confirmed." (when Codex never answered). This holds for a Stop that names its turn too. Since #24301, a named Stop that stopped nothing writes no row, but here Orca has just seen the turn still running, so the Stop says it didn't stop instead of saying nothing. That row is accurate, because the process may still be running. Orca keeps track of the process and retries ending it on the next Stop, or when the chat goes idle.

When the process exited but cleanup failed. If the process provably exited but a later bookkeeping step failed (for example, releasing the chat's claim on the process), the Stop still reads "Cancellation requested." The process is gone, so the turn did stop. The failure is logged, and the leftover cleanup is retried later.

How it works

The change sits in the chat Stop's shared step (performCancel in structured-agent-session-turns-cancel.ts). It runs after the interrupt attempt, in the same step on the session's queue, which handles one operation at a time. That means no send or exit handling can run in between.

1. Classify the interrupt. The interrupt counts as failed only when its effect is unknown: Codex's internal-error answer (JSON-RPC code -32603), a timeout, or any other error. The Codex adapter marks Codex's "invalid request" answer (code -32600) as turnNotRunning on the cancel outcome. Codex sends that code for "no active turn", "another turn is active", and "thread not loaded". This uses the error code, not Codex's wording. A failure with that mark never ends the process.
2. Re-check the chat. After a failed interrupt, the Stop first processes every Codex message Orca has already received. It then re-reads the conversation's journal (the host's durable record of the chat). It ends the process only if the session is still working on the same turn the Stop was for. A Stop that names no turn means the turn the journal showed when the Stop arrived. The one exception is a Stop sent before any turn opened: there, the message still waiting for its turn is what the Stop was for.
3. End the process. This goes through the existing stopChild → context.stopAgent(sessionId) path, the same one fix(claude): a Stop ends Claude's process, and the next message resumes the conversation #24235 and the running-command Stop use. That path proves the process exited, records the cut turn as the user's cancellation, and releases the session's claim on the process.
   • If the exit proof fails, the host keeps the process and still owes its shutdown. The next Stop or the idle sweep retries it.
   • If the exit was proven and a later step failed, the host has already dropped the process. The Stop reads that from the host's own record of the session's process, so no new state is needed, and answers as cancelled.

Why this approach.

• It closes the one case where a user's Stop could stop nothing, and keeps the decision that a successful Codex Stop leaves the process running.
• Ending the process reuses the existing proven-exit path, so there is no second way to kill a process.
• Deciding by the error code and then by the re-read journal keeps the decision tied to what Codex said and what the chat is doing, not to Codex's wording.

Alternatives considered.

• End the process on every refusal. Codex records a turn as ended before it writes the message announcing it, so a "no active turn" refusal can reach Orca while the journal still shows the turn running. Ending on every refusal would kill the process, and its background terminals, after a turn that finished on its own.
• Wait a short grace period after a refusal, then re-check. This adds delay to every refused Stop, and Codex's error code already says the turn isn't running.
• End the process in a later step on the queue, the way fix(claude): a Stop ends Claude's process, and the next message resumes the conversation #24235 does for Claude. That step answers the Stop before the exit is proven, so an unproven exit would still say "Cancellation requested.". Ending it inline keeps the honest row.

Mixed versions. Nothing changes on the wire, and turnNotRunning never leaves the host. An older client gets a Stop answer of cancelled: true, a "Cancellation requested." row, an interrupted turn, and a session that reads not running. A Claude Stop already produces all of these, so older clients render them as they do today.

Differences from the common pattern

Matches: the core rule matches the comparable hosts' interrupt-failure recovery: end the session only while it still runs the stopped turn.

No differences are temporary. Each one is intended:

• A refusal that says the turn isn't running keeps the process. One comparable host ends the provider session on any failed interrupt. Another never ends the Codex process on a refused interrupt: the turn keeps running, which is the bug this PR fixes. Orca ends the process only for an interrupt whose effect is unknown. Intended: Codex's "not running" refusal can arrive before the frame that ends the turn, so ending on it would kill a process whose turn just finished normally (see Alternatives).
• An unanswered interrupt ends the process after the 30-second timeout. One comparable host's interrupt request has no timeout, so it waits indefinitely. Another times out, marks the thread stopped, and leaves the process running. Intended: a turn that Codex never confirmed stopping is still running as far as the journal knows, and only ending the process stops it. The exit is proven before the Stop says so.
• The re-check processes every received Codex message first. A comparable host re-reads its session state as of that moment, without waiting for messages still being processed. Intended: without that step, a turn change Orca has received but not yet recorded would look like the old turn still running.
• A Stop that names a turn requires that same turn. In a comparable host, a session with no current turn passes the check even for a Stop that names a turn. Intended: Orca's phone client always names the turn. After that turn has ended, "no current turn" means a newer message is in flight, and ending the process would lose it. A test reproduces this.
• A Stop that names no turn is matched against the turn the journal showed when it arrived. Comparable clients name the running turn themselves, so the effect is the same. Intended.
• An unproven exit keeps the row and the process. Comparable hosts mark the session stopped after a failed stop, or after a timeout. Intended: Orca has already seen what forgetting an unproven close does. It "stranded the process forever and reported success" (structured-agent-session-eviction.ts:15-18). Orca never treats an unproven exit as a dead process.
• The Stop's answer and row on a proven exit. These match a comparable host, which records a separate "interrupt failed" note only when its stop itself failed or lagged. A clean stop there writes none, as here. Ending the process inline, in the Stop's own step, also matches that host.

Testing

Tests with the real host, the real journal, and the real Codex adapter over a fake Codex connection (structured-agent-session-codex-stop-row.test.ts):

• An interrupt that Codex failed internally (-32603), or never answered (a real request timeout), while the turn runs ends the process through the host's stop, with you as the cause. The turn reads interrupted as a cancellation. 4 cases, covering a Stop that names its turn and one that names none.
• Codex's "not running" refusal arrives before the end-of-turn message: the process is kept and the turn completes normally. 2 cases.
• A Stop names a turn that ended while a newer message's turn hasn't opened: the process is kept, whether Codex refuses or never answers. 2 cases.
• The re-check works from the messages Orca had already received when the interrupt failed. Journal writes are held back, so only processing the received messages first shows the turn change.
• Codex moved on to another turn, the turn ended before the interrupt arrived, a Stop names a turn that ended while the chat is at rest (no row, per fix(native-chat): every chat action press is its own action (re-land #23916 on main) #24301), and the process exits during the interrupt: the process is kept, or left to its own exit.
• A Stop naming the running turn whose interrupt Codex failed internally, where the process exit can't be proven, says "Codex didn't stop: …" instead of writing no row.
• A Stop that Codex accepts keeps the process.

Host tests with a mock adapter (structured-agent-session-conversation-stop.test.ts):

• A failed or unanswered interrupt with a message in flight ends the process.
• A "not running" refusal keeps the process and shows "Codex didn't stop: …".
• When the exit is unproven, the "didn't stop" or "not confirmed" row stays and the failure is reported.
• A proven exit followed by a cleanup failure reads "Cancellation requested." and still reports the failure.
• A failed interrupt while the chat is at rest keeps the process, and a Stop naming its turn writes no row.

Adapter test (codex-structured-conversation-stop.test.ts): an "expected active turn id … but found …" refusal carries turnNotRunning.

Ablations. Before the merge of #24301, each guard was removed in turn and both host test files (42 tests) re-run:

• Without the same-turn requirement for a named Stop: 1 test fails (the newer-message case where Codex never answers).
• With a "not running" refusal counted as failed: 3 fail (both refusal-before-end cases, and the mock "not running" case).
• Without the adapter's error-code check: 2 fail.
• Re-reading without processing received messages first: 1 fails (the held-writes test).
• Without the proven-exit check after a cleanup failure: 1 fails.
• Without the "didn't stop" row for a named Stop whose exit is unproven: 1 fails (it showed "The provider had already finished this turn." again).

After merging main (#24301), re-run on both host test files and the adapter test (48 tests):

• Without the failed-interrupt process end: 10 fail, every "ends the child" case plus the unproven-exit and cleanup-failure rows.
• With fix(native-chat): every chat action press is its own action (re-land #23916 on main) #24301's "a named Stop that stopped nothing writes no row" rule applied after the failed-interrupt step instead of as its alternative: 1 fails (a named Stop whose turn is still running and whose exit is unproven loses its "Codex didn't stop: …" row).

Wider runs and checks:

• Every test file that imports a changed module, plus the Codex and Claude stop and cancel tests, eviction, unexpected exit, and runtime exit: 23 files, 225 tests, all passing. The runtime integration, Codex turn-end settlement and Codex compact-exit tests: 4 files, 25 tests, all passing.

• After merging main: the two host stop files, the adapter stop test, and every native-chat test file fix(native-chat): every chat action press is its own action (re-land #23916 on main) #24301 added or changed (14 files, 175 tests), all passing.

• No real-CLI or real-binary tests were run.

• oxlint on the changed files, the changed-code quality gate, and the anti-slop audit pass after the merge of main. The node typecheck passes after the merge too.

• I manually tested these changes (live QA on a separate Mac, see Visual Proof)

• Automated tests added/updated

Linked Issue

Follow-up to #24235.

Visual Proof

Live QA on a separate Mac, running a dev build of this branch at efa1651 in the background with its own home folders. A structured Codex chat ran against a stand-in Codex server that can refuse or ignore the interrupt on demand. Later commits change only comments, the rare-path row, and merges of main (including #24301's Stop-note rules: a named Stop that stopped nothing writes no row, and a turn's Stop row is keyed to that turn). No real Codex binary was used.

Case	Result
1. Codex never answers the interrupt	After the 30 s timeout the Codex process ends; the turn reads interrupted, the row reads "Cancellation requested.", and the chat stops running
2. Codex answers "failed to interrupt turn"	Same as case 1, about 2 ms after the error
3. Next message after case 1 or 2	A new Codex process starts and resumes the same conversation (same thread id)
4. Codex says "no active turn to interrupt" before the turn-end message arrives	Process kept (same pid); row reads "Codex didn't stop: no active turn to interrupt."; the turn completes normally
5. Codex accepts the interrupt	Process kept (same pid); turn interrupted; "Cancellation requested." (unchanged)

1. Never answered:

2. Failed internally:

3. Next message resumes the same conversation (after case 1, then after case 2):

4. "No active turn" refusal racing the turn's end, process kept:

5. Accepted Stop, process kept:

Review

Agent skill upstream boundary

• Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

• Security: no new inputs or processes.
• Performance: the extra work runs only after a failed interrupt. The Stop's answer then also waits for the process's exit proof, up to about 5.5 seconds on macOS and Linux. The 30-second wait for an unanswered interrupt already existed.
• Cross-platform / SSH: the decision and the process end run on the host that owns the Codex process, through the existing per-platform exit proof. Codex structured chats run only on the local host today.
• Mobile: nothing new to render; mobile shows the same rows a Claude Stop already produces. For an unanswered interrupt, the phone's 15-second request timeout still fires before Codex's 30-second one, as it did before this PR. The chat then shows the interrupted turn instead of "not confirmed".
• Known edge: a request Codex could not parse is also answered with -32600. It reads as "not running", which keeps the process and today's row, so it is no worse than before.
• Backwards compatibility: see "Mixed versions" above.
• Related: fix(native-chat): after a Stop whose exit can't be confirmed, the next message retries the stop instead of failing #24333 retries an owed stop before every operation, which also covers the unproven-exit case here. Whichever lands second reconciles the two.

Checklist

• This PR is small and focused
• I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

[brennanb2025]

Review status: ready for your merge decision

The problem. In a Codex chat, pressing Stop sends Codex an interrupt for the running turn. When Codex never answered that interrupt (it times out after 30 s) or answered that it failed internally ("failed to interrupt turn"), Orca tried nothing else. The turn kept running, the chat said "Cancellation was not confirmed." or "Codex didn't stop: …", and the only way out was to close the chat.

What changes for you. In those two cases, Stop now ends the Codex process if the turn you stopped is still running. The turn reads as interrupted, the row says "Cancellation requested.", and your next message starts Codex again on the same conversation. A Stop that Codex accepts still keeps the process and its background terminals, exactly as before. If Codex says the turn isn't running, or the turn you stopped has already finished, nothing is ended.

Fixed during review (head eacb88ed7f9):

• (P2) A Stop that named a turn that had already finished could kill the Codex process while your next message was waiting for its turn to start. The phone always names the turn, so it could hit this. Now a Stop ends the process only if the journal still shows that same turn running.
• (P2) A Stop pressed just as a turn finished could kill Codex and its background terminals. Codex can send "no active turn" a moment before the message saying the turn ended, so this was a real race. Codex's "invalid request" answer (code -32600) now never ends the process. Only an internal error, a timeout or a transport failure can.
• (P3) If the process provably exited but a later cleanup step failed, the Stop now says "Cancellation requested." instead of "didn't stop".
• (P3) A named Stop whose process end couldn't be confirmed said "already finished this turn". It now says "Codex didn't stop: …".
• (P3) Corrected comments and PR-body claims that contradicted Codex's actual behaviour.
• Tests reframed onto refusals Codex really sends, plus new tests: the race, an unopened follow-up, draining Codex's received messages before re-reading the journal, and a failed cleanup after a proven exit.
• Merged main to resolve a conflict with refactor(native-chat): move the provider-exit proof out of structured-agent-session-adapter.ts so main's lint passes #24323.

Deferred (P3, not fixed here):

• The Stop answer now waits for the process to end, on top of the existing 30 s interrupt timeout. A Stop that already waits on a running command behaves the same way.
• No unit test checks that the next send resumes the same conversation. The test fixture can't tell resume from a fresh start. Live QA verified it instead.
• A Codex build without turn/interrupt would have its process ended on every Stop of a running turn. No supported Codex is known to lack it.
• A turn that ends naturally at the exact instant the 30 s timeout fires could still be ended. The window is very narrow and was not reproduced.
• The previous PR-body section comparing this design to other hosts was corrected in the body.

Overlap with #24333: #24333 retries an owed stop before every operation, which also covers this PR's "exit couldn't be confirmed" case. Whichever lands second reconciles.

Verified:

• 225 tests in the 23 test files that import the changed code, plus 25 related interrupt tests, all pass. Each new guard was ablated and its test failed.
• The Node typecheck (tc:node) passes through the shared gate. Lint and the changed-code quality gate are clean, and CI is green.
• Live QA on a separate Mac (screenshots in the body) used a dev build of this branch with isolated home folders and a stand-in Codex server:
  • never-answered and internal-failure interrupts end the process;
  • the next message resumes the same conversation (same thread id);
  • a "no active turn" refusal racing the turn's end keeps the process;
  • an accepted Stop keeps it.

Not verified:

• No run against a real Codex binary. A real Codex can't refuse or ignore an interrupt on demand.
• No transport-failure case, no Windows run, no mobile run.
• Codex chats run only on the local machine, so SSH doesn't apply.

Already true before this PR, not changed:

• The Stop row is only visible after expanding the collapsed turn.
• An ended-and-restarted Codex looks the same to the user as an accepted Stop.

[greptile-apps]

Confidence Score: 5/5

[Medium risk] Fixes how the chat stop handles provider refusals and turn state.

The PR appears safe to merge; no actionable new issue or outstanding prior finding was established.

Summary

The PR ends a structured Codex process when its interrupt fails or goes unanswered and the intended turn is still running, while retaining the process after an accepted interrupt or a not-running refusal.

• Re-checks received provider events and journal state before ending the child.
• Adds adapter classification and host-level tests for failures, turn races, and exit-proof outcomes.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Stop Codex turn] --> B[Request interrupt]
  B --> C{Interrupt accepted?}
  C -->|Yes| D[Keep process]
  C -->|No, turn not running| E[Keep process]
  C -->|Failed or unanswered| F[Flush received events and check intended turn]
  F -->|No longer running| E
  F -->|Still running| G[Stop child and verify exit]
  G -->|Exit proven| H[Report cancellation]
  G -->|Exit unproven| I[Retain failure status and process tracking]

Loading

_{Reviews (2) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..."}

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 4da9177c-6e4b-4fe8-8799-84d306ea5073

📥 Commits

Reviewing files that changed from the base of the PR and between eacb88e and b292795.

📒 Files selected for processing (3)

• src/main/native-chat/agent-session-wire/structured-agent-session-codex-stop-row.test.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

---

📝 Walkthrough

Walkthrough

Codex interrupt refusals now identify cases where the targeted turn is no longer running. The host distinguishes those refusals from failed or unanswered interrupts. When a failed interrupt leaves the targeted turn running, the host attempts to end the child and checks whether the session released it. The adapter exposes the cancellation outcome type, and Stop tests cover these outcomes and related turn and session states.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to b2927

When a Codex Stop fails or goes unanswered while the targeted turn is still running, the Codex process is now ended. When Codex reports that the turn is no longer running, the process is kept. One narrow edge case was already raised: a failed event flush during a named Stop could end a follow-up turn. No other blocking issue was found, and the change looks ready to merge.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b2927

Stop now ends the chat’s agent when interruption fails, while checking that the requested turn is still running. No introduced security issue was established, but caller-access controls and shutdown behavior against a real agent remain partially verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The directly selected shutdown target is one session’s provider child; the request does not introduce an arbitrary process identifier. Exit confirmation concerns the provider root and does not independently establish termination of every tool descendant or background workload.

Trust Boundaries and Controls

• observed — The shared admission path recomputes the request fingerprint before running cancellation. Durable replay reconstructs an outcome rather than interrupting again. Stop can proceed after ledger-write failure through evaluated admission, so durable deduplication is not unconditional during storage failure.
• observed — Ordinary conversation Stop is admitted without writer-lease enforcement, and expectedRuntimeFence is intentionally not checked. Caller identity participates in operation admission, but the inspected host-layer code does not establish upstream authentication or caller-to-session access authorization.

Resilience and Maintainability Implications

• observed — The generic session-ending branch permits wind-down after an unknown interrupt effect, including some named stale-turn requests. Its relevant condition is present in both the available target revision and head, before the new failed-interrupt fallback; this is not established as a PR-introduced regression.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 9 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely identifies the main change: ending the Codex process when a Stop interrupt is refused or unanswered.
Description check	✅ Passed	The description is complete and directly addresses the template. It explains the problem, user impact, implementation, alternatives, linked issue, visual proof, testing, platform considerations, and c…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: a70c84a7-152f-411c-9008-db1212990d23

📥 Commits

Reviewing files that changed from the base of the PR and between 9ed7b39 and eacb88e.

📒 Files selected for processing (9)

• src/main/codex/codex-structured-conversation-stop.test.ts
• src/main/codex/codex-structured-session-adapter-fixture.ts
• src/main/codex/codex-structured-turn-cancellation.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-adapter-stop.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-chat-stop.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-codex-stop-row.test.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

[coderabbitai]

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

sed -n '1,80p' src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts
sed -n '140,270p' src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts
sed -n '80,125p' src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts
rg -n 'drained\(|flushEventSink|sink.*fail|sink.*error' src/main/native-chat/agent-session-wire/structured-agent-session*test.ts src/main/native-chat/agent-session-wire/structured-agent-session-event*

Repository: stablyai/orca

Length of output: 19107

---

🏁 Script executed:

set -eu
printf '%s\n' '--- cancellation implementation ---'
sed -n '1,290p' src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts
printf '%s\n' '--- focused stop tests ---'
sed -n '330,430p' src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts
printf '%s\n' '--- cancellation test references ---'
rg -n -C 5 'stillRunsStoppedTurn|flushStreamedEvents|sink barrier|childReleased|onStopChildError|cancelUnconfirmed' src/main/native-chat/agent-session-wire/*cancel*.test.ts src/main/native-chat/agent-session-wire/*stop*.test.ts src/main/native-chat/agent-session-wire/structured-agent-session-codex-stop-row.test.ts
printf '%s\n' '--- stopChild callback bindings ---'
rg -n -C 8 'stopChild:|onStopChildError|childReleased|stopAgent\\(' src/main/native-chat/agent-session-wire/structured-agent-session-*.ts

Repository: stablyai/orca

Length of output: 42702

---

🏁 Script executed:

set -eu
printf '%s\n' '--- relevant cancellation code ---'
sed -n '25,55p' src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts
sed -n '180,275p' src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts
printf '%s\n' '--- focused sink-failure test ---'
sed -n '350,415p' src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts
printf '%s\n' '--- flush and journal bindings ---'
rg -n -C 10 'flushStreamedEvents|flushEventSink|drained\\(|journal.*append|append.*journal' src/main/native-chat/agent-session-wire/structured-agent-session-host*.ts src/main/native-chat/agent-session-wire/structured-agent-session-event-sink*.ts src/main/native-chat/agent-session-wire/structured-agent-session-turns*.ts
printf '%s\n' '--- barrier failure implementation/tests ---'
sed -n '130,235p' src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.ts
sed -n '180,225p' src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.test.ts

Repository: stablyai/orca

Length of output: 7313

---

🏁 Script executed:

set -eu
printf '%s\n' '--- flush bindings ---'
rg -n -C 8 'flushStreamedEvents|flushEventSink|drained' src/main/native-chat/agent-session-wire
printf '%s\n' '--- notification bindings ---'
rg -n -C 8 'notify\\(|turn/started|turn/completed|turnLifecycle' src/main/native-chat/agent-session-wire/structured-agent-session-codex* src/main/native-chat/agent-session-wire/structured-agent-session-*-adapter* 2>/dev/null || true
printf '%s\n' '--- event sink implementation ---'
sed -n '1,230p' src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.ts

Repository: stablyai/orca

Length of output: 45665

---

🏁 Script executed:

set -eu
printf '%s\n' '--- sink queue drain/failure semantics ---'
sed -n '1,280p' src/main/native-chat/agent-session-wire/structured-agent-session-event-sink-queue.ts
printf '%s\n' '--- lifecycle notification files ---'
rg -l 'turn/started|turn/completed|notify' src/main | head -40

Repository: stablyai/orca

Length of output: 10912

---

Do not stop a named turn after a failed event flush.

When the provider interrupt throws after queuing completion for the stopped turn and the start of a follow-up turn, the sink queue can fail and clear the queued rows. The journal can still report the stopped turn. stillRunsStoppedTurn then matches the old ID, and stopChild calls stopAgent(sessionId), which can terminate the follow-up turn.

For a named Stop, return false when the flush fails. Preserve the current conservative behavior for an unnamed Stop.

Suggested fix

 async function stillRunsStoppedTurn(
   ctx: Pick<AgentSessionTurnContext, 'journal' | 'fence' | 'flushStreamedEvents'>,
   stoppedTurnId: string | null
 ): Promise<boolean> {
+  if (stoppedTurnId !== null) {
+    try {
+      await ctx.flushStreamedEvents()
+    } catch {
+      return false
+    }
+    if (
+      !isStructuredAgentSessionMainAgentWorking(
+        ctx.journal.activeTurnId(),
+        ctx.journal.submissions(),
+        ctx.fence
+      )
+    ) {
+      return false
+    }
+    return ctx.journal.activeTurnId() === stoppedTurnId
+  }
   if (!(await isMainAgentWorkingOnceFlushed(ctx))) {
     return false
   }
-  // Working with no turn open after the Stop's turn is a later send whose turn has not opened.
-  return stoppedTurnId === null || ctx.journal.activeTurnId() === stoppedTurnId
+  return true
 }

[pullfrog]

✅ No new issues found.

Reviewed changes

The Codex Stop now ends the Codex child when the interrupt it sent either failed internally (-32603) or went unanswered, instead of leaving the turn running behind a "not confirmed" row. A Stop that Codex accepted, or refused with its "turn not running" code (-32600), still keeps the child and its background terminals.

• performCancel failed-interrupt branch — after the interrupt, if it failed while the session still runs the turn the Stop meant, ends the child through the existing proven-exit stopChild path; a proven exit followed by a failed cleanup still reads "Cancellation requested", and an unproven exit keeps the row and the child for retry.
• stillRunsStoppedTurn — re-flushes received Codex frames before re-reading the journal, then requires the same turn (or a still-unopened send for an unnamed Stop) to be running.
• Codex adapter classification — -32600 is carried as turnNotRunning, and AgentSessionCancelOutcome now lives with the adapter-stop contract (re-exported unchanged).
• Tests — real adapter + real journal + fake Codex in structured-agent-session-codex-stop-row.test.ts pin child-end via disposeSession(..., 'user-stop'), plus host-level cases for proven/unproven exit and cleanup failure.

I traced the new inline stopChild against the session's non-reentrant serialize queue and the lifetime/eviction machinery: the eviction entry runs under-serialize and no awaited step re-enters the lane, and childReleased can only flip once the eviction's own proven-exit step has dropped the child, so neither a deadlock nor a false "exit proven" is reachable from this branch. The intentional decisions (broad -32600, keeping the child on a not-running refusal, ending the child on an unknown-effect interrupt) are documented in the PR body and consistent with the stated goal.

  ^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

This run re-reviewed the commits since the prior pullfrog review (eacb88e). Since then the only new commit is a merge of main (no new PR-authored non-merge commits); the substantive delta is the merge's conflict resolution in performCancel plus the test rows it adjusts.

• performCancel — named-Stop rule ordering — the merge added #24301's "a named Stop that stopped nothing writes no row" rule as the final else if (!cancelled && taken === false && input.turnId !== undefined), placed after this PR's failed-interrupt child-end branch rather than before it, so a named Stop still running behind an unproven exit keeps its "Codex didn't stop: …" row. Confirmed with an ablation: turning that else if into a standalone if fails only structured-agent-session-codex-stop-row.test.ts → "says Codex did not stop when a Stop naming the running turn could not prove the exit".
• Test expectations — the host and Codex-row cases for a named Stop whose turn already ended now expect no status row ([]) instead of "The provider had already finished this turn.", matching #24301; the unnamed-"not running" case keeps its "Codex didn't stop: …" row.

Focused tests pass locally: the Codex adapter stop test, the host conversation-stop test, and the Codex stop-row test (48 tests). Traced the final state end-to-end against #24301's branches — the unnamed Stop keeps its refusal/unconfirmed row, the named Stop falls through to no row when it stopped nothing, and the failed-interrupt row survives an unproven exit.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}