Skip to content

fix(native-chat): phone Stop reaches a message the agent hasn't answered; unreadable task list reports unconfirmed - #28243

Merged
brennanb2025 merged 6 commits into
mainfrom
brennanb2025/ssh-chat-stop-close-ref
Oct 14, 2026
Merged

fix(native-chat): phone Stop reaches a message the agent hasn't answered; unreadable task list reports unconfirmed#28243
brennanb2025 merged 6 commits into
mainfrom
brennanb2025/ssh-chat-stop-close-ref

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 13, 2026 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 9 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​421 $\color{#cf222e}{\Huge{\mathbf{−}}}$​3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​418
Prod 8 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​35 $\color{#cf222e}{\Huge{\mathbf{−}}}$​11 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​24

ELI5

You press Stop and expect the agent to stop. Over an SSH host there were two places where that went wrong:

  • On the phone: pressing Stop right after sending a message, before the agent's first reply, said "Stop not sent" and the agent kept going.
  • On the background-tasks strip: pressing Stop on a task while the host briefly couldn't read its own task list answered "done" without stopping anything.

Now the phone sends the same Stop the desktop already sends, and the host stops whatever the chat has in flight, including a message the agent hasn't answered yet. A task Stop that the host can't check says "unconfirmed" instead of claiming success.

What Changed

Before → after, as you see it:

  • Phone, Stop pressed before the agent's first reply: before, "Stop not sent" and the agent ran on. After, the agent is stopped, exactly as on the desktop.
  • Background-tasks strip, Stop while the host can't read its task list: before, a quiet "ok" while the task kept running. After, the Stop reports that it isn't confirmed, so you know to check and press again.
  • Everything else about Stop and Close is unchanged.

Mechanism:

  • Phone: when the host advertises agent-session.conversation-stop.v1, a working chat shows Stop before the agent opens a turn, and the phone's Stop names no turn. That is the form the desktop already uses, and it means "stop whatever the chat has in flight". The host already counts a message the agent hasn't echoed yet as work in flight. A question or permission card's Cancel still names the turn that raised the card, whether or not the host takes the card's own identity. An older host without the capability gets today's turn-named Stop.
  • Host: a background-task Stop that can't read the session's task list now fails as "outcome unknown". That is the same refusal a background Stop already returns when the stop request itself times out, so every client already shows it. Before, the unreadable list read as "no tasks", so the Stop answered cancelled: false as if the task had already ended.
  • Tests pin the races the earlier attempt (fix: keep chat Stop and Close independent of receipt bookkeeping #26461) was built around, now on the existing plain Stop. All of them already pass on main:
    • a Codex turn that started, or was admitted, before the user echo;
    • an admitted turn that can't take an interrupt yet: the Stop succeeds only once the host has let go of the agent, and otherwise says "unconfirmed";
    • a turn opened before the echo links it to the send;
    • a later message that reaches the host just before the Stop, which is withdrawn while the waiting turn is interrupted;
    • a retried Stop with the same operation id, which replays and never interrupts a later turn;
    • a Stop after a Stop that failed, which tries again.

No new capability, no new wire field, no new message type.

Why

The common pattern is that the execution host decides what a Stop reaches, and the work has an identity from the moment a message is accepted. Orca's host already works this way. It accepts a Stop that names nothing, and it treats a send the agent hasn't answered as work in flight. Codex's opening turn is waited for and interrupted, and agents whose Stop ends their session end their process.

#26461 instead had clients name the turn, waiting message or task they saw, and added host-side rules for proving that work had ended. That cost about 770 production lines and a new capability. This PR keeps the existing mechanism, adds tests showing it already covers those races, and fixes only the two gaps above. #26461 will be closed in favour of this one.

Differences from the common pattern:

  • Intended: Stop carries an operation id whose receipt replays a retried request. A Stop retried after a lost reply therefore never stops later work. If the receipt row can't be written, Stop still runs.
  • Intended: a background-task Stop names the agent's own task id together with the turn. The host stops only tasks its own records list as stoppable right now.
  • Intended: an unreadable task list answers "unconfirmed". The host's child-task store can fail to read, and a failed read is never proof that a task ended.
  • Intended: Orca has no multi-round "stop until settled" loop. The Stop and the agent's end run on the session's one serialized lane, and sends made meanwhile wait behind it, so no new turn can open during a Stop. A Stop whose interrupt can't be confirmed ends the agent instead.
  • Temporary: a plain Stop still names a turn when the host lacks agent-session.conversation-stop.v1. It goes away once no supported host lacks it.
  • Existing behaviour, not changed here; temporary: closing a chat tab through session.tabs.close stops the agent even when the hidden-tab record can't be written (refactor(native-chat): keep agent-session records in the chat journal database #24006). The reason is that a database written by a newer Orca opens read-only. That only happens after a downgrade, which users don't do, so this is a candidate to make strict (no stop, report the failure) in a follow-up.
  • Existing behaviour, not changed here; temporary: agentSession.close waits for the agent to stop before it answers. The common pattern answers once the close is recorded and lets the stop finish on its own. Candidate follow-up.
  • Intended: messages still queued when a chat closes are held so they are never sent on their own. That is the same meaning as the common pattern, through Orca's existing hold.

Linked Issue

Coordinator's structured SSH chat fix plan, U21 and U23 (replaces #26461). No separate public issue.

Visual Proof

Live check on the phone (iOS Simulator running this branch's Orca Mobile, paired to a desktop Orca built from this branch, real signed-in Claude), head 06506a29542.

1. Right after sending, before any reply: the phone shows Working and a Stop button. On main the phone has no turn yet here, so Stop answered "Stop not sent" and Claude kept going. (The numbers above are from an earlier turn.)

working with Stop visible

2. Stop pressed about 1 s after sending: the turn is interrupted with no reply. The desktop shows the same.

interrupted after 0s

3. Stop while numbers are streaming still stops it (stopped at 248 of 400).

stopped mid-reply

4. A question card's Cancel from the phone still settles that card (it names its own turn, not "everything in flight").

question card cancelled

Not run live: an SSH host, Windows/Linux desktops, Android.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

On macOS:

  • Regression tests, each failing on main before its fix:
    • structured-agent-session-background-stop-unreadable.test.ts: the one-task and every-task Stop with an unreadable list.
    • mobile-structured-agent-session-unnamed-stop.test.ts: the phone's Stop before the first reply, its Stop that names no turn, and a card's Cancel that keeps its turn when the host can't take the card's identity.
    • use-mobile-native-chat-controller.test.ts: the controller offers Stop for a working chat before a turn opens on a capable host, while a card still offers no Cancel without a turn.
  • Step-0 race tests (structured-agent-session-unnamed-stop-races.test.ts, 7 tests): all pass on main unchanged. One uses the real Codex adapter over a scripted Codex app-server.
  • Other suites:
    • neighbouring desktop Stop suites (conversation stop, repeat press, RPC methods): 110 tests;
    • every mobile session test: 254 files, 2,401 tests.
  • Typechecks: tc:node, tc:web, the mobile typecheck and the mobile test-typecheck ratchet all pass.
  • orca-ci-checks: 21 of 23 pass. The two failures are not from this change:
    • main's existing mobile import-cycle warnings, in files this PR doesn't touch;
    • the local bun typecheck runner failing with Script not found "check". The tsc typechecks above pass, and CI runs its own.

Not run: a live SSH host, the phone, or Windows/Linux desktops. The change is host logic and a phone request shape, both covered by the tests above.

Review

Draft.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Host: all the logic is on the execution host, in shared code, so local, SSH and WSL hosts and folder and Git workspaces behave the same.
  • Mixed versions:
    • An old desktop or phone talking to a new host is unchanged on the wire. Old clients get the background-task fix for free.
    • A new phone checks the host's capability before sending a Stop that names no turn.
  • Platform: nothing platform-specific.

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
fix(native-chat): phone Stop reaches unanswered sends; unreadable tas…
d182dab 
…k list is unconfirmed

The phone named the running turn on every Stop, so a Stop pressed before the
agent echoed the message said "Stop not sent" and the agent ran on. Against a
host advertising agent-session.conversation-stop.v1 the phone now sends the
Stop that names no turn, as the desktop already does; the host stops whatever
the chat has in flight. A card's Cancel still names its turn, and older hosts
keep the turn-named Stop.

A background-task Stop whose host could not read the session's task list read
the failure as "no tasks" and answered cancelled:false. It now refuses as
outcome unknown, the refusal a timed-out background stop already returns.

Adds host tests pinning the Stop-before-echo races on the unnamed Stop: Codex
turns admitted or started before the echo, a later send just before the Stop,
a retried Stop, and a Stop after a failed one. All pass on main unchanged.
@brennanb2025
fix(mobile): show Stop before the turn opens; a card's Cancel keeps i…
833e5cf 
…ts turn

The phone offered Stop only once a turn existed, so the Stop that names no turn
was unreachable while a send waited for the agent's echo. Against a host that
advertises conversation stop, a working chat now offers Stop.

A card's Cancel chose the untargeted form from the prompt after the
prompt-cancel capability filtered it out, so against a host with conversation
stop but no prompt cancel it sent a Stop naming nothing. It now always names
the card's turn.

The race test for an admitted turn that cannot take an interrupt accepts
success only once the host has let go of the agent, and otherwise requires the
unconfirmed refusal.
@brennanb2025
ci: rerun against current main
06506a2
@brennanb2025

brennanb2025 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Review summary (head 06506a29542)

What this fixes: Stop and Close over SSH and from the phone, rebuilt on the common pattern: the host stops whatever the chat has in flight, without a target. It replaces #26461, which was 773 production lines; this is 26 lines added and 9 removed.

  • Phone: Stop works right after sending, before the agent's first reply. It used to say "Stop not sent" while the agent kept going.
  • Background tasks: a background-task Stop says "unconfirmed" when the host can't read the task list, instead of a false "ok".
  • Close: no change. Main already writes the close record before stopping. The PR body labels three existing behaviours.

How it was reviewed

  • First, fix: keep chat Stop and Close independent of receipt bookkeeping #26461's 7 race tests were ported to main's plain Stop: a Stop pressed before the agent echoes the message, a later send arriving just before the Stop, a retried Stop, a Stop after a failed Stop, and more. All pass unchanged. They stay as regression tests.

  • Round 1 of independent review found three issues:

    • the phone still hid Stop until a turn existed;
    • a card's Cancel could become an untargeted Stop;
    • one race test accepted "stopped" while the agent might still run.

    All three are fixed. The final pass on 833e5cf71cc approved, with no blocking issues. The later head is an empty commit to rerun CI.

Live check: phone (iOS Simulator) paired to a desktop built from this branch, with the real signed-in Claude. See Visual Proof: Stop right after sending, Stop mid-reply, and a question card's Cancel all behaved as expected.

CI: green except one test in shard 5, serialize-grid-transcript-replay.test.ts ("codex-model-capacity-fullscreen"). It fails on a clean checkout of current main (2cbb719) too; the recording came from #28231. This PR touches no terminal or daemon code.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 13, 2026 10:26
@brennanb2025 brennanb2025 mentioned this pull request Oct 13, 2026
Closed
8 of 9 tasks
@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: c12a43c9-9fb5-4136-8ec7-b28f4f90aa2a
📥 Commits

Reviewing files that changed from the base of the PR and between 458a9bc and 3716542.

📒 Files selected for processing: 1
  • config/scripts/vitest-sqlite-runtime-files.mjs

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

The mobile session now detects host support for conversation-level Stop and can send a Stop request without a running turn when that capability is present. Server-side tests cover unnamed Stop timing and retry cases, plus background-task cancellation when the child-work list cannot be read. The SQLite runtime test include list now contains both added server test files.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 37165

The supplied review identifies no actionable issue; normal checks remain appropriate before merging.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 17 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly summarizes both primary changes: mobile Stop handling for unanswered messages and unconfirmed results when the task list is unreadable.
Description check Passed The description is detailed and covers the required sections, including user impact, implementation, rationale, testing, visual proof, compatibility, and checklist status. It does not provide an actua…
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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[bot]
coderabbitai Bot reviewed Oct 13, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 726aee90-f4b4-4b5a-960b-a04dc17da7f6
📥 Commits

Reviewing files that changed from the base of the PR and between 1b9a408 and 06506a2.

📒 Files selected for processing (16)
  • config/scripts/vitest-sqlite-runtime-files.mjs
  • mobile/src/session/mobile-native-chat-merged-snapshot-parity-hooks.test.tsx
  • mobile/src/session/mobile-structured-agent-session-cancel.ts
  • mobile/src/session/mobile-structured-agent-session-host-support.test.ts
  • mobile/src/session/mobile-structured-agent-session-host-support.ts
  • mobile/src/session/mobile-structured-agent-session-unnamed-stop.test.ts
  • mobile/src/session/mobile-structured-operation-id-retirement.test.ts
  • mobile/src/session/use-mobile-native-chat-controller.test.ts
  • mobile/src/session/use-mobile-native-chat-controller.ts
  • mobile/src/session/use-mobile-structured-agent-session-prompt-cancel.test.tsx
  • mobile/src/session/use-mobile-structured-agent-session-queued.test-fixture.ts
  • mobile/src/session/use-mobile-structured-agent-session.ts
  • mobile/src/session/use-mobile-structured-turn-indicator.test.tsx
  • src/main/native-chat/agent-session-wire/structured-agent-session-background-stop-unreadable.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-background-stop.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-unnamed-stop-races.test.ts

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

Comment thread src/main/native-chat/agent-session-wire/structured-agent-session-background-stop.ts
@brennanb2025

brennanb2025 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

CI is now fully green at c682fcc3c56 (an empty commit to rerun against current main; no code change). The replay test that failed before was fixed on main in #28286 and #28295.

@brennanb2025
Merge remote-tracking branch 'origin/main' into brennanb2025/ssh-chat…
458a9bc 
…-stop-close-ref

# Conflicts:
#	src/main/native-chat/agent-session-wire/structured-agent-session-background-stop.ts
@brennanb2025
Merge remote-tracking branch 'origin/main' into brennanb2025/ssh-chat…
3716542 
…-stop-close-ref
@brennanb2025
brennanb2025 merged commit a622bb0 into main Oct 14, 2026
33 of 34 checks passed
@brennanb2025
brennanb2025 deleted the brennanb2025/ssh-chat-stop-close-ref branch October 14, 2026 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@brennanb2025