fix(claude): a Stop naming the running turn acts at once, and a Stop in the gap is no longer lost by brennanb2025 · Pull Request #23861 · stablyai/orca

brennanb2025 · 2026-09-29T10:47:12Z

	Files	Added	Deleted	Net
Test	7	$\color{#1a7f37}{\Huge{\mathbf{+}}}$600	$\color{#cf222e}{\Huge{\mathbf{−}}}$280	$\color{#1a7f37}{\Huge{\mathbf{+}}}$320
Prod	4	$\color{#1a7f37}{\Huge{\mathbf{+}}}$80	$\color{#cf222e}{\Huge{\mathbf{−}}}$81	$\color{#cf222e}{\Huge{\mathbf{−}}}$1

ELI5

Stopping a Claude chat from the phone app, while a follow-up message was queued, took about 3 seconds to do anything, and the chat kept saying Working. And if you pressed Stop just as a reply finished and your follow-up was about to start, the Stop was ignored and the follow-up ran. Now the Stop acts at once in both cases.

What Changed

Before

The phone app, and older desktop apps, send a Stop that names the turn they can see running. The desktop app now sends one that names no turn (fix(native-chat): Stop is there from the moment a message is sent #23026), which never waited.
For a named Stop with a queued follow-up, Orca waited up to 3 seconds for the follow-up to be accepted before interrupting. A queued follow-up is never accepted until it runs, so the Stop always waited the full 3 seconds.
The phone names the turn from its own copy of the chat, which can lag. If you pressed Stop as that turn ended, with your follow-up already handed to Claude, the Stop was refused: the follow-up ran anyway. The 3-second wait only delayed that refusal.
When the follow-up was still waiting on the host, the Stop did withdraw it but then wrote "The provider had already finished this turn.", which was not true.

After

A Stop that names the running turn is handled exactly like a Stop that names no turn: it interrupts at once, and Claude's queued follow-up is withdrawn.
A Stop that names a turn that just ended, while a message is still waiting on Claude, is handled the same way, so that message does not run.
A Stop that names an older turn while a different, newer turn is running is still refused. The turn check still protects a newer turn you did not mean to stop.
What any Stop reports now follows one rule, whether or not it names a turn: it succeeded if it withdrew what was waiting and nothing reads as running afterwards. So a Stop that withdrew a host-queued follow-up reports success with no false line, and a Stop that names no turn and withdrew a follow-up as its turn ended no longer reports failure.
Before judging what is still running, a Stop waits for the rows the agent has already sent to land. Claude records a message as delivered one step before it records the turn that runs it; reading in between saw nothing running, so a Stop could report success while that turn ran, or, for a Stop that names no turn, skip the interrupt and let the turn run. If waiting for those rows fails, the Stop still interrupts: bookkeeping never blocks a Stop.

Behaviour change: a client that names a turn that is no longer running, including an older client that names a placeholder id in the gap before a turn opens, now has its Stop honoured when a message is still waiting, instead of refused. A Stop means "don't run what I sent", so honouring it is the right outcome.

Mechanism

claude-structured-prompt-ownership.ts: a Stop with no prompt and no compaction goes to the conversation-stop path when its named turn is the live turn (read the same way the existing owner check reads it), or when nothing is live and a written message has not opened its turn yet. The waiting-message check is the one the conversation path already uses.
The 3-second admission wait (waitForClaudeDispatchAdmission and its timeout) is deleted. Once the cases above take the conversation path, the only Stop left behind the wait named a turn that had ended, where it only delayed a refusal.
structured-agent-session-turns.ts: one success rule in performCancel for Stops that name a turn and Stops that do not, read from the journal after the Stop. (The provider's separate "unconfirmed" Stop result is gone since fix(codex): a Codex Stop is the interrupt alone, so it no longer says "Cancellation was not confirmed" #23850, which this PR now includes.)
isMainAgentWorkingOnceFlushed drains the streamed rows before the working check, in both performCancel and the no-turn Stop's gate in structured-agent-session-host-mutations.ts. A failed drain reads as working, so the Stop interrupts.
Nothing new is stored, there are no timers, and nothing changes on the wire. The fix is on the host, so phones already out there get it with no app update.

Why

The wait stood between a Stop and a decision that was already made: Claude's interrupt covers the whole session, so a queued follow-up is no reason to wait. Removing the wait, rather than shortening it, takes out the only thing that could hold an ordinary Stop, and the conversation-stop path already settles the queued follow-up correctly.

Alternatives considered:

Make the phone app send the no-turn Stop. It fixes new phone builds only; phones already installed would keep the delay.
Shorten the 3-second wait. It still delays every such Stop and still loses the Stop in the gap.

Linked Issue

No issue. Found while rechecking the 3-second Stop from #23553's live QA after #23026 landed.

Visual Proof

Live QA with the real Claude CLI on a second Mac, at head 9242db5f17d. The Stop was sent the way the phone app sends it (naming the running turn), through the host from the app's own page, as an agreed stand-in for phone QA, because typing into the phone emulator has been unreliable.

Scenario	Before	After
Stop naming the running turn, follow-up queued (3 runs)	about 3 s before anything happened	interrupted in 24–39 ms; the follow-up withdrawn; the chat left Working; no "already finished" or "not confirmed" line
The same with a Stop naming no turn (control)	fast	28 ms, same result
Follow-up sent just as the reply ended, then a Stop naming that reply (3 runs)	the Stop was refused and the follow-up ran	the follow-up never replied: withdrawn in 2 runs (text back in the message box); in 1 it had already reached the host, so its turn opened and was interrupted 6 ms later with no output

Before (current main, the same way of sending the Stop, on the same second Mac):

Stop naming the running turn with a follow-up queued, 2 s after the Stop: the reply is still streaming, the chat still says Working, and the follow-up is still queued:

The same run once settled: the Stop answered after 3.4 s having interrupted nothing, the count ran to 600, and then the follow-up ran:

A follow-up sent just as the reply ended, then a Stop naming that reply: the follow-up ran anyway, and the chat says the turn had already finished:

The same case when the follow-up was still waiting on the host: it was withdrawn, but the chat still wrote the false "already finished" line:

After (this PR):

Stop naming the running turn with a follow-up queued: the reply stops, and the follow-up's text is back in the message box:

A follow-up sent just as the reply ended, then a Stop naming that reply: the follow-up did not run, and its text is back in the message box:

The moment between a follow-up being delivered and its turn starting could not be caught from the app on this path (the turn row arrives first); a Stop fired at the closest point interrupted the turn with no output. That gap is covered by the new host tests.

Testing

I manually tested these changes locally
Automated tests added/updated, or explained why not below
claude-structured-queued-stop.test.ts: a named-turn Stop with a queued follow-up now interrupts with no clock movement and the follow-up is withdrawn (red on main: the old test advanced the clock the full 3 seconds). New: a Stop naming an older turn is refused and leaves the newer turn running; a Stop naming a turn that just ended withdraws the follow-up behind it.
structured-agent-session-claude-queued-stop.test.ts (real host, journal and fake Claude): the handed-over case (follow-up rejected as withdrawn, chat idle) and the host-queued case (follow-up never written, chat idle, no "already finished" row).
claude-turn-ownership.test.ts: five tests that pinned the wait now assert the Stop happens at once; one that took 3003 ms now takes 1 ms.
Removing each fix fails its test by assertion.
pnpm tc:node and tc:cli, oxlint, check:code-quality:changed, check:react-doctor:changed and the anti-slop audit pass.

AI Disclosure

Review

Five review rounds. Round 1 found that a withdrawal hid an unconfirmed or refused Stop behind a silent success. Round 2 found that Claude and Codex report a refusal differently, so a Stop naming an older turn could read as a silent success. Because the same success check had needed three fixes, round 3 was an architecture challenge: it replaced the check with the one rule above, shared by Stops that name a turn and Stops that do not, and found two more wrong rows on the way. Round 4 found the delivered-before-running gap and the same gap in the no-turn Stop, both fixed by waiting for the sent rows. Round 5 was clean: the wait cannot deadlock (it only waits for rows already queued, with no timer), and a failed wait never blocks or hides a Stop.

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

Merge order: #23850 has merged, and main (with it) is merged into this PR, so the rule above no longer reads an unconfirmed Stop outcome.

No wire change: the Stop request and its reply keep their shapes.
Older clients: a Stop they send is handled better, never worse.
Known, not changed here: a Claude refusal of the live named turn still reads "already finished"; and the adapter's live-turn lookup reads the journal unflushed, which in a rare race can route a stale named Stop to the session-wide interrupt.
Same on macOS, Linux, Windows and SSH hosts: the host decides.

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)

Author: @BrennanKB5

…eued A Stop that named the running Claude turn waited up to 3 s whenever a later send's handover was still unresolved, for example a follow-up Claude had queued. The phone app and older desktop clients always name the turn. Claude's interrupt is session-scoped, so a Stop naming the live turn now takes the same path as a Stop naming no turn: it interrupts at once, and the queue sweep settles the follow-up as withdrawn. A Stop naming a turn that is no longer live is still refused. With that case gone, the admission wait could only delay a refusal, so it is deleted rather than left as a second gate.

…w-up behind it The phone names the turn it shows, and its copy of the chat can trail the host's. When that turn ends just before its Stop lands, with a follow-up already written to Claude but not yet started, the Stop was refused and the follow-up ran anyway. With nothing live, a written follow-up whose turn has not opened is work the naming client has not seen start, so the Stop takes the conversation path: it interrupts and Claude's queued follow-up settles as withdrawn. A Stop naming an older turn while a newer one runs is still refused.

…thing finished When a Stop named a turn that had already ended, the host still withdrew the message the user had sent after it, yet the Stop reported nothing cancelled and the chat read "The provider had already finished this turn." That withdrawal is the Stop taking effect, so it now reports cancelled and writes no row, as a Stop naming no turn already does in the same case.

greptile-apps · 2026-09-29T10:51:00Z

Confidence Score: 5/5

[Medium risk] Refactors how conversation stops are named and processed.

The PR appears safe to merge based on the reviewed Stop paths and outcomes.

Summary

The PR makes named Claude Stops interrupt the live turn immediately, handles Stops in the gap before a follow-up turn opens, and bases the reported outcome on the remaining journal state. The changes since the previous review also remove an unused unconfirmed outcome and update tests for the current journal database.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Stop[Stop request] --> Host[Host withdraws queued messages]
  Host --> Adapter[Adapter checks live turn and interrupts]
  Adapter --> Journal[Host reads flushed journal state]
  Journal --> Result[Report cancellation outcome]

_{Reviews (9) · Last reviewed commit: "test(native-chat): open the Stop-report ..."}

coderabbitai · 2026-09-29T10:52:23Z

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

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

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: 45d38bb7-720c-4571-a12e-ad1b149a2713

📥 Commits

Reviewing files that changed from the base of the PR and between fee6489 and b7ab836.

📒 Files selected for processing (7)

src/main/claude/claude-structured-prompt-ownership.ts
src/main/claude/claude-turn-ownership.test.ts
src/main/native-chat/agent-session-wire/structured-agent-session-adapter.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-host-mutations.ts
src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts
src/main/native-chat/agent-session-wire/structured-agent-session-turns.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts

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

📝 Walkthrough

Walkthrough

Claude cancellation no longer waits for dispatch-admission polling and uses a shared live-turn lookup. Native chat cancellation flushes streamed events before checking main-agent work and handles queued-message withdrawal based on the provider outcome. Tests cover live, ended, stale, and unresolved turns, as well as queued and delivered follow-ups.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to b7ab8

Stop now acts on the running turn immediately instead of waiting about three seconds. A Stop naming an older turn while a newer one is running is still refused. No merge-blocking issue remains. The author documents two rare edge cases that were left unchanged.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to b7ab8

A named Stop can now act immediately while a follow-up is waiting, which fixes a missed cancellation. In that gap, the Stop can also affect session work without establishing that the waiting work belongs to the turn it names. The impact depends on who can control the session and how concurrent sends are handled.

Retained concerns

Medium · architecture · inferred: When no turn is live, a Stop naming a turn can issue a session-wide interrupt solely because a dispatch waiter exists. The waiter is not bound to the named turn, so a stale or unrelated name may affect later session work in that gap. Whether this crosses a caller-ownership boundary is unverified.

Security review details

Security Blast Radius

inferred — The independently affected scope is the provider session and its pending work, not just the named turn: provider interrupt is session-scoped, and the host withdraws every queued submission. The evidence does not establish exposure beyond an authorized session.

Trust Boundaries and Controls

observed — When a different turn is already live, the named-turn route does not take the conversation branch; the remaining turn-specific path checks current turn ownership and dispatch admission before interruption.

Resilience and Maintainability Implications

observed — A failed event flush is treated as ongoing work rather than evidence that it is safe to skip interruption; repeated status writes use the client operation ID to upsert the Stop note.

Hardening Proposals

proposed — Define explicitly whether a named Stop authorizes session-wide cancellation in the no-live-turn gap. If it does not, bind the waiting submission to an eligible turn or operation before interrupting it; verify the rule against concurrent admission and repeated Stops.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 35.71% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 28 functions across 12 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 describes the main change: immediate handling of named Claude Stop requests and prevention of lost Stops during the transition between turns.
Description check	✅ Passed	The description is detailed and covers the user impact, changes, rationale, testing, visual proof, review history, compatibility notes, and checklist. It states that no linked issue exists and leaves …

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🛠️ Fix failing CI checks 💡

Commit to this branch
Create a new PR

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.}

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 5c17df2c-85c4-4a20-9f11-1263f230ab52

📥 Commits

Reviewing files that changed from the base of the PR and between 31012ae and ca2355f.

📒 Files selected for processing (9)

src/main/claude/claude-structured-conversation-stop.test.ts
src/main/claude/claude-structured-prompt-ownership.ts
src/main/claude/claude-structured-queued-stop.test.ts
src/main/claude/claude-structured-session-adapter-turns.test.ts
src/main/claude/claude-turn-ownership.test.ts
src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts
src/main/native-chat/agent-session-wire/structured-agent-session-claude-queued-stop.test.ts
src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts
src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts

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

coderabbitai · 2026-09-29T10:52:26Z

+    if (!cancelled && input.turnId !== undefined && input.withdrewQueued) {
+      // The Stop ended what was queued, which is how a Stop naming no turn reports the same case.
+      cancelled = true
+      note = null
+    } else if (!cancelled && input.turnId !== undefined) {


🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

sed -n '170,220p' src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts sed -n '240,335p' src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts sed -n '90,175p' src/main/claude/claude-structured-prompt-ownership.ts sed -n '215,285p' src/main/native-chat/agent-session-wire/structured-agent-session-claude-queued-stop.test.ts

Repository: stablyai/orca

Length of output: 12603

🏁 Script executed:

set -e printf '%s\n' '--- claude cancellation implementation ---' rg -n -A120 -B20 'async function cancelClaudeTurn|function cancelClaudeTurn|cancelClaudeTurn\(' src/main/claude src/main/native-chat printf '%s\n' '--- activeTurnId declarations and implementations ---' rg -n -A35 -B15 'activeTurnId\s*\(' src/main/native-chat src/main/claude printf '%s\n' '--- relevant cancellation tests ---' rg -n -A35 -B15 'withdrewQueued|newer|already finished|cancel.*false|cancel.*error|interrupt' src/main/native-chat/agent-session-wire src/main/claude

Repository: stablyai/orca

Length of output: 45664

🏁 Script executed:

set -e rg -n -A140 -B20 'async function cancelClaudeTurn|function cancelClaudeTurn|cancelClaudeTurn\(' src/main/claude src/main/native-chat rg -n -A40 -B20 'activeTurnId\s*\(' src/main/native-chat src/main/claude rg -n -A35 -B15 'withdrewQueued|newer|already finished|cancel.*false|cancel.*error|interrupt' src/main/native-chat/agent-session-wire src/main/claude

Repository: stablyai/orca

Length of output: 45664

🏁 Script executed:

set -e printf '%s\n' '--- cancellation helper location ---' rg -l 'function cancelClaudeTurn|export .*cancelClaudeTurn|async .*cancelClaudeTurn' src/main/claude printf '%s\n' '--- cancellation helper outcomes ---' for f in $(rg -l 'function cancelClaudeTurn|export .*cancelClaudeTurn|async .*cancelClaudeTurn' src/main/claude); do echo "FILE:$f" rg -n -A180 -B15 'function cancelClaudeTurn|export .*cancelClaudeTurn|async .*cancelClaudeTurn' "$f" done printf '%s\n' '--- journal active-turn declarations ---' rg -l 'activeTurnId' src/main/native-chat src/main/claude | head -20 rg -n -A35 -B15 'activeTurnId' src/main/native-chat/agent-session-wire src/main/native-chat | head -500

Repository: stablyai/orca

Length of output: 39969

🏁 Script executed:

set -e printf '%s\n' '--- cancelTurn adapter bindings ---' rg -n -l 'cancelTurn\s*[:=]|cancelTurn\s*\(' src/main | head -40 rg -n -A80 -B25 'cancelClaudeStructuredTurn' src/main printf '%s\n' '--- cancel outcome types and refusal mapping ---' rg -n -A80 -B20 'AgentSessionCancelOutcome|refusal\??:|unconfirmed\??:' src/main/native-chat src/main/claude | head -1000

Repository: stablyai/orca

Length of output: 42833

Do not infer a no-live-turn cancellation from activeTurnId().

The host withdraws queued messages before performCancel. Claude returns { cancelled: false } when the requested turn is no longer current. It also returns { cancelled: false } for a ClaudeControlRequestError. performCancel converts both outcomes to success when withdrewQueued is true.

activeTurnId() === null is not sufficient. The journal can have no published live turn while Claude still has an in-memory turn. Propagate an explicit cancellation outcome that distinguishes the host-queued-only case from a stale-turn refusal and an unconfirmed interrupt failure. Apply the queued-withdrawal override only to the host-queued-only outcome. Keep thrown non-control errors on the existing unconfirmed path.

…eported as a success A named Stop that withdrew a host-queued message was reported cancelled, with no row, whatever the provider answered. When the provider took the interrupt without confirming it (or refused it), the turn may still be running, so the Stop now keeps the provider's answer instead of claiming success.

pullfrog

ℹ️ No critical issues — one edge case worth a look.

Reviewed changes

Named Stop acts at once — claude-structured-prompt-ownership.ts routes a no-prompt Stop that names the live turn (or names an ended turn while nothing is live but a written message is still waiting) into the conversation-stop path, and deletes the 3s waitForClaudeDispatchAdmission gate. The old wait only ever applied to named no-prompt stops; no stale references to CLAUDE_DISPATCH_ADMISSION_TIMEOUT_MS remain.
Newer-turn protection kept — a Stop naming an older turn while a newer one runs still fails ownsRequestedTurn() and is refused without an interrupt.
No identity race introduced — the conversation-path isCurrent only checks "some turn live / a waiter exists", not requestedTurnId, but the branch decision and the isCurrent guard run in the same synchronous tick, so the branch-time identity check still holds. The deleted wait was the only await that could have reopened it.
Host withdrawal reporting — structured-agent-session-host-mutations.ts now calls performCancel directly with withdrewQueued, and structured-agent-session-turns.ts promotes a named Stop that withdrew queued submissions to cancelled: true with no status row, removing the false "The provider had already finished this turn." line.
Tests — five files rewritten/added to pin immediate stop, the older-turn refusal, the ended-turn-with-pending-follow-up withdrawal, and the host-queued withdrawal. Verified locally: 37 tests pass, pnpm tc:node and oxlint clean on the changed files.

ℹ️ Nitpicks

claude-structured-prompt-ownership.ts:78 — the conversation guard re-verifies only that some turn is live, not that it is requestedTurnId. Correct today only because the branch and the guard share a synchronous tick; a future await between them would silently let a named Stop interrupt a turn the client did not name. A one-line assertion or comment would make that invariant explicit.

^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

pullfrog

Important

The new !outcome.refusal guard does not cover Claude: its cancel path returns { cancelled: false } with neither flag when a newer turn supersedes the named one, so the older-turn case from the prior review is still reported as success.

Reviewed changes (delta since the prior pullfrog review at ca2355f)

structured-agent-session-turns.ts — the withdrewQueued promotion now also requires !outcome.unconfirmed && !outcome.refusal; the added comment states that a provider which took or refused the interrupt still has a turn the withdrawal did not end.
structured-agent-session-conversation-stop.test.ts — two new host tests: a host-queued message plus ended turn reports cancelled: true with no row; a host-queued message plus unconfirmed reports cancelled: false and writes a row.

ℹ️ Nitpicks

structured-agent-session-conversation-stop.test.ts:306 — expect(await statusRows()).not.toEqual([]) accepts any non-empty row. The sibling tests in this file pin the exact text, and this branch writes a specific 'Cancellation was not confirmed.' row, so asserting that string would make the test fail for the right reason.

^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

… still runs A named Stop that withdrew a host-queued message was reported as a success unless the provider's answer carried a refusal or was unconfirmed. Providers report a refusal differently: Claude never sets one, so a Stop naming an older turn while a newer one ran came back cancelled with no row; Codex refuses any turn that has ended, so the ended-turn case still wrote "already finished". The success is now decided by the rule the no-turn Stop uses: nothing still reads working on the journal.

A conversation Stop that withdrew what was queued and left nothing working on the journal is a success, whether or not it named a turn. The rule was scoped to named Stops, so a Stop naming no turn whose turn ended under it reported that the agent had no turn to stop after it withdrew a message. An unconfirmed interrupt is now reported as unconfirmed before the named-turn row, which said the provider had already finished a turn it had just taken an interrupt for.

pullfrog

✅ No new issues found.

Reviewed changes (delta since the prior pullfrog review at 7817f81)

Journal-based promotion gate — structured-agent-session-turns.ts drops the provider-flag check (!outcome.refusal) and promotes a refused named Stop to cancelled: true only when the journal no longer reads working (isStructuredAgentSessionMainAgentWorking). This is the rule the no-turn Stop already uses and it covers Claude, which never sets refusal.
New host test for the superseded case — a Stop naming an older turn while a newer turn runs and a queued follow-up was withdrawn now asserts cancelled: false, no interrupt, and the already finished row. I confirmed it fails under the pre-acacb8a guard (cancelled: true).
New provider-refusal test — a provider (Codex) refusing an ended turn after the host withdrew a queued message still reports success with no row, matching the stated intent.

Both prior threads are retired: the earlier !outcome.refusal finding is resolved by the journal read, and the host-level named-older + newer-running + host-queued path is now covered.

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

pullfrog

✅ No new issues found.

Reviewed changes (delta since the prior pullfrog review at acacb8a)

One rule for every conversation Stop — structured-agent-session-turns.ts drops the input.turnId !== undefined scoping from the withdrewQueued promotion, so a no-turn Stop that withdrew queued work and left nothing working is reported the same way a named one already was. The journal working-check still vetoes it whenever a turn or unanswered dispatch remains, so a newer running turn is unaffected.
Unconfirmed outranks the ended-turn row — the outcome.unconfirmed branch now precedes the input.turnId branch, so a named Stop the provider left unconfirmed writes Cancellation was not confirmed. instead of the false The provider had already finished this turn.
New performCancel unit tests — structured-agent-session-turns.test.ts pins the no-turn promotion, the still-working veto, and the named-unconfirmed row for both withdrewQueued values.
Tightened host assertion — structured-agent-session-conversation-stop.test.ts now asserts the exact Cancellation was not confirmed. row instead of any non-empty set.

Verified locally: all five touched test files pass (45 tests) and pnpm tc:node is clean. Both prior threads are already resolved.

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

…ws land Claude's send echo accepts the send one sink write before the turn row it opens lands, so a Stop that withdrew a queued message could read the journal idle in that gap and report success with no row while the turn then ran. Drain the streamed events before the read, as the prompt Stop already does.

…s streamed rows land The no-turn Stop decides whether to interrupt from the journal. Claude's send echo accepts the send one sink write before its turn row lands, so in that gap the journal read idle and the Stop skipped the interrupt while the turn ran. Drain the streamed events first, through the same read the Stop's report uses. A failed drain reads working, so bookkeeping never keeps a Stop from interrupting.

pullfrog

ℹ️ No critical issues — one robustness nit inline.

Reviewed changes (delta since the prior pullfrog review at fee6489f)

Flush before reading what a Stop left working — structured-agent-session-turns.ts adds isMainAgentWorkingOnceFlushed, which drains the session's deferred event sink and only then reads isStructuredAgentSessionMainAgentWorking, treating a failed drain as working so bookkeeping can never talk a Stop out of stopping.
Both Stop decisions use it — structured-agent-session-host-mutations.ts uses the helper for the no-turn inFlight gate, and performCancel uses it for the withdrewQueued promotion gate, closing the window where Claude's echo has marked the send accepted but its turn row is still queued in the sink.
Tests — a host test that injects the late turn row during the mocked drain (plus a drain-rejects variant that still interrupts), and a performCancel unit test pinning that a newer running turn vetoes promotion once the row lands.

I confirmed the premise against the production Claude path: the echo submits the turn row to the sink before the settlement marks the send accepted, so the flush reliably lands it. Awaiting the sink barrier inside the serialized cancel mutation introduces no deadlock or re-entrancy.

^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

pullfrog · 2026-09-29T11:36:13Z

+  ctx: Pick<AgentSessionTurnContext, 'journal' | 'fence' | 'flushStreamedEvents'>
+): Promise<boolean> {
+  try {
+    await ctx.flushStreamedEvents()


ctx.flushStreamedEvents() here is unbounded, but the codebase bounds the identical sink drain on a stop-adjacent path with withTimeout(..., SNAPSHOT_DRAIN_TIMEOUT_MS, null) (structured-agent-session-eviction.ts:54-55), whose comment says a stalled sink must not hold a stop behind it. Reusing that constant — treating a timeout as true, as the catch above already does — keeps the normal case identical while preventing a wedged sink from hanging the Stop and, since the cancel runs under the session's serialize key, every later operation on that session.

Technical details

# Bound the Stop's sink drain ## Affected sites - `src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts:248` — `await ctx.flushStreamedEvents()` never times out; a wedged sink queue leaves this promise unresolved. - `src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts:190` — call site for the no-turn Stop's `inFlight` gate. - `src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts:307` — call site for the `withdrewQueued` promotion gate. ## Required outcome - Waiting for the sink drain must not be able to hold a Stop (and the session's serialize chain) indefinitely if a journal write wedges. ## Suggested approach - Wrap the flush in `withTimeout` (from `../../../shared/promise-timeout-fallback`) using the existing `SNAPSHOT_DRAIN_TIMEOUT_MS` constant, and treat a timeout as `true` (working), matching the `catch` above. This preserves normal-case behavior and keeps the conservative "assume working" default.

brennanb2025 · 2026-09-29T12:52:34Z

Status

Ready. Head 9242db5f17d, based on main. CI green (35 checks).

The failure:

A Claude Stop that named the running turn, which is what the phone app sends, waited up to 3 seconds for a queued follow-up that could never be accepted.
A Stop naming a turn that had just ended, while a follow-up was already with Claude, was refused, and the follow-up ran.

The fix: both now take the path a Stop naming no turn uses, and the 3-second wait is gone. What any Stop reports follows one rule, read from the chat after the Stop has settled. A Stop naming an older turn while a newer one runs is still refused.

Review:

Five rounds, including an architecture challenge. The success check had needed three fixes, so it was replaced with one rule shared by every conversation Stop, and that surfaced two more wrong lines.
A later round found that Claude records a message as delivered one step before its turn, so both the named and no-turn Stops now wait for the sent rows before judging. If that wait fails, the Stop still interrupts.
The last round was clean.

Validation:

Live on a second Mac with the real Claude CLI: the named Stop interrupted in 24–39 ms instead of about 3 s. In 3 of 3 runs, a follow-up sent just as the reply ended did not run.
The Stop was sent through the host, as the phone app sends it, as an agreed stand-in for phone QA.
Host and adapter tests cover every case; each fix fails its test when removed.
tc:node, tc:cli, oxlint, the changed-lines gate, React Doctor and the anti-slop audit all pass.

Known, not changed:

A Claude refusal of the live named turn still reads "already finished".
In a rare race, a stale named Stop can interrupt a message another client sent. This follows the rule that a Stop means "don't run what I sent".

…fter the main merge

pullfrog

✅ No new issues found in this delta.

Reviewed changes (delta since the prior pullfrog review at 9242db5)

Merged origin/main and re-applied the Stop fix on its command-turn refactor — the branch merged main's command-turn machinery (translator.commandTurnId, performCancel moved into the new structured-agent-session-turns-cancel.ts, and the stopChild command-end path). The named/no-turn Stop early branch now excludes a command turn with session.translator?.commandTurnId !== requestedTurnId, the exact negation of main's deleted compactions.ownsTurn, so the routing is unchanged; the flush-before-read helper and the withdrawal rule moved verbatim into the new module.
Thread-scoped Stop test rows — the Stop host tests now stamp turnScope: AGENT_JOURNAL_THREAD_SCOPE on the turn rows they inject, matching how production writes turn rows after main's thread-scoping change.

I traced the merged branch, ownership, and host-withdrawal paths against both parent sides; the resolution is semantically equivalent to the reviewed pre-merge fix. Verified on the merged tree: pnpm tc:node, oxlint on the four changed source files, and all seven affected test files (63 tests) pass.

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

brennanb2025 · 2026-09-29T21:58:00Z

Status update: merged current main in (head a10036bfc5e), CI green (22 passed). Main moved the Stop code into structured-agent-session-turns-cancel.ts and compaction onto the command turn; this PR's rule is ported there. The plain merge would have dropped main's new step that stops a running /compact on this path; it is passed explicitly now, and main's compaction-Stop test fails without it. Merge #23850 first; main then gets merged into this PR once more so it takes #23850's change to the same code. Not merged.

pullfrog

✅ No new issues found in this merge delta.

Reviewed changes (delta since the prior pullfrog review at b7ab836)

Merged origin/main (#23726, host-owned mid-turn queue) over the Stop fix — main had rewritten cancelStructuredAgentSessionTurn to run the no-turn/no-prompt Stop inside runStopWithQueuePause with explicit tookEffect() queue-pause points and an early-return if (!child || !inFlight) { if (withdrawn.length > 0) await tookEffect(); return { cancelled: withdrawn.length > 0 } }. The merge adopted main's structure and re-applied the PR's two changes: isMainAgentWorkingOnceFlushed(ctx) for the no-turn inFlight gate, and a direct performCancel(..., { withdrewQueued }) call — main's plan.run cannot thread withdrewQueued through. The two shapes are De Morgan-equivalent, so the routing and reporting rule are unchanged.
Took main's adapter change — structured-agent-session-adapter.ts dropped the now-unused onReverted rewind callback while keeping the PR's resolveLiveTurnId JSDoc update (the deleted admission wait is why it no longer says the guard re-checks after a fence may have waited).
No PR source lost main's refactor — claude-structured-prompt-ownership.ts and structured-agent-session-turns-cancel.ts are byte-identical to the pre-merge branch; main did not touch either between merge-base 2807332735 and its tip a57863ba. The other eight PR files are unchanged by the merge.

Traced the merged cancel path against both parents: the early-return and performCancel branches cover the same cases as the pre-merge fix (named live turn, named ended turn with a written follow-up, older-turn refusal, no-turn gate), and main's queue-pause bookkeeping cannot gate the interrupt (runStopWithQueuePause only records and reports). Verified on the merged tree: all seven affected test files pass (73 tests), pnpm tc:node clean, oxlint clean on the two merged source files.

The only open Pullfrog thread remains the prior unbounded-drain suggestion at structured-agent-session-turns-cancel.ts:18 — untouched by this merge and not re-raised here.

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

brennanb2025 · 2026-09-30T00:54:40Z

Visual Proof update: live BEFORE screenshots on current main are now in the description, next to the existing AFTER ones, sent the same way (a Stop naming the running turn, as the phone sends it). On main the Stop takes about 3.4 s, interrupts nothing, and the queued follow-up runs; a Stop naming a reply that just ended lets the follow-up run and writes a false "already finished" line, including when the follow-up was withdrawn. Not merged.

…med Stop outcome)

…after the main merge

pullfrog

✅ No new issues found in this delta.

Reviewed changes (delta since the prior pullfrog review at e71e661)

Adopted main's removal of the unconfirmed Stop outcome (#23850) — the origin/main merge drops the outcome.unconfirmed branch from performCancel and the !outcome.unconfirmed term from the withdrewQueued promotion gate. AgentSessionCancelOutcome no longer carries unconfirmed, so the removal is forced; a Stop Codex never answers still reads Cancellation was not confirmed. through the thrown-error path, and the named/no-turn reporting rule is otherwise unchanged.
Journal opener updated after the merge — structured-agent-session-turns.test.ts opens its Stop-report journal on the host database (stateDirectory) instead of the retired journalDir key, matching the other journal test suites.
Tests pinning the deleted outcome removed — the performCancel it.each and the host test that fed { cancelled: false, unconfirmed: true } are gone; the merged host suite drives the same Cancellation was not confirmed. row through a rejected cancelTurn instead.

I re-ran the seven affected test files on the merged tree (60 tests) and pnpm tc:node; both are clean. The only open Pullfrog thread remains the prior unbounded-drain suggestion at structured-agent-session-turns-cancel.ts:18, which this delta does not touch.

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

brennanb2025 added 3 commits September 29, 2026 03:28

greptile-apps Bot reviewed Sep 29, 2026

View reviewed changes

Comment thread src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts Outdated

coderabbitai Bot reviewed Sep 29, 2026

View reviewed changes

greptile-apps Bot reviewed Sep 29, 2026

View reviewed changes

Comment thread src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts Outdated