Skip to content

fix(native-chat): bring back Working-only-while-held (#30134) without shutting down Claude chat workers - #30577

Merged
brennanb2025 merged 14 commits into
mainfrom
brennanb2025/claude-worker-start-ack
Oct 21, 2026
Merged

brennanb2025 merged 14 commits into
mainfrom
brennanb2025/claude-worker-start-ack

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 10 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​228 $\color{#cf222e}{\Huge{\mathbf{−}}}$​185 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​43
Prod 39 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​238 $\color{#cf222e}{\Huge{\mathbf{−}}}$​163 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​75

ELI5

#30134 made Claude, Pi and ACP chats show "Working" only while the agent actually holds a message, but it also broke starting a Claude chat as an orchestration worker: the moment Claude acknowledged its task, Orca decided it could not confirm the task was received and shut the new chat down. #30134 was reverted on main (#30572) as a stopgap. This PR brings #30134 back with the timing mistake fixed, so both work: chats stop showing Working when the agent holds nothing, and Claude chat workers keep running.

What Changed

The bug this fixes (main with #30134, and the 1.4.230 ad hoc before its revert):

  • orca orchestration worker-start --agent claude failed with "The dispatch preamble was submitted but not acknowledged (unknown): provider_ended_before_acknowledgement", and the chat said "closed by Orca", usually before Claude had replied. On one Mac, 90 of 92 Claude chat workers started after 10-18 17:00 UTC died this way (Codex workers: 242 of 242 fine).
  • The saved chat history always looked fine (sent, then received by Claude); the doubt existed only in memory for one instant.
  • Every other place that waits for a message's answer got the same false "unknown" (older clients sending a message, compaction, resuming after restart), and every Claude chat client was briefly told the message was unconfirmed when Claude echoed it.

After this PR:

  • A Claude chat worker starts, Claude answers its task, and later messages reach it.
  • A Claude message goes from sent to received with no unconfirmed moment in between.
  • fix(native-chat): Claude, Pi and ACP chats are Working only while the agent holds the send #30134's behavior is back (its own description has the details): a Claude, Pi or ACP chat shows Working only while the agent is running a turn or still holds a message, so one failed save of the agent's answer no longer keeps a chat stuck on Working; such a message is marked unconfirmed right away instead.

Mechanism:

  • fix(native-chat): Claude, Pi and ACP chats are Working only while the agent holds the send #30134 (re-applied unchanged) has each agent report which messages it still holds. A message with no saved answer that the agent no longer holds is marked unconfirmed. That needs the agent's hold and the host's hold to overlap: the host starts holding a message (its handover window) the moment the agent reports the answer.
  • Pi and ACP release a message and report its answer in the same step. Claude did not: when Claude echoed a message, its adapter dropped it first, then emitted the turn the echo opens, and reported the answer only after that (deliberately, so the turn is saved before the answer, from fix(native-chat): Stop is there from the moment a message is sent #23026). Emitting the turn can publish immediately, and that publish saw the message held by nobody.
  • Now the messages a Claude frame settles stay in Claude's held set until their answers have been reported, so any publish in between sees them held. The emit-then-report order from fix(native-chat): Stop is there from the moment a message is sent #23026 is unchanged.
  • A Stop that cancels several queued Claude messages had the same shape (drop all, then report each); it now releases and reports them one at a time.
  • If emitting a frame throws, its answers are still reported; before, they were lost after the message had been dropped.

Why

The one structural fact behind the bug: Claude's adapter changed what it holds at a different moment than it handed the answer to the host. Fixing that ordering removes the gap for every reader at once (worker start, every other answer wait, and what clients are shown), with no host, saved-data or wire change. The common pattern does the same: set the new state before clearing the old, so a reader can see both but never neither. Codex already had no gap (its echo hands the answer over in the same step; its republish waits a microtask).

Alternatives considered:

  • Treat "unknown" as still pending in worker-start. Rejected: hides the symptom in one caller and leaves the others and the client flash.
  • Hold a Claude message until its turn ends. Rejected: bigger change for the same fix, and an already-received message would count as unfinished work if Claude exited mid-turn.
  • Keep fix(native-chat): Claude, Pi and ACP chats are Working only while the agent holds the send #30134 reverted. Rejected: chats would again stay stuck on Working after a failed save.
  • Never mark a message unconfirmed because the agent no longer holds it (positive confirmation only, the common pattern). Removes the class entirely, but is the planned next stage of the native-chat send-lifecycle work (tracked in STA-11448); this PR doesn't block it.

Differences from the common pattern:

  • A message can still be marked unconfirmed because the agent no longer holds it. Temporary: removed by the send-lifecycle work's positive-confirmation stage, tracked in STA-11448.
  • Worker start still closes a worker whose first message is genuinely unconfirmed; the common pattern never closes a child over that. Temporary: this PR only removes the false doubt; whether a genuine doubt should keep the worker open is a product decision, tracked in STA-11449.
  • fix(native-chat): Claude, Pi and ACP chats are Working only while the agent holds the send #30134's own labelled differences carry over unchanged (ownership from the live agent; doubt kept until the agent process ends; Pi and ACP receipt rules temporary).

Linked Issue

Tracked in STA-11160 (Linear). Re-lands #30134 (reverted in #30572).

Visual Proof

To follow: a real Orca build where a Claude chat worker started with worker-start completes two turns, main versus this branch.

Testing

  • New regression test on the real Claude adapter and host (structured-agent-session-claude-opening-send-hold.test.ts): a message's answer across Claude's echo reads "received", never "unknown". Fails without the fix (unknown), passes with it.
  • fix(native-chat): Claude, Pi and ACP chats are Working only while the agent holds the send #30134's tests come back with it. 48 named Claude, Pi, ACP and host test files (393 tests) pass locally.
  • I manually tested these changes locally (real-build QA to follow)
  • Automated tests added/updated

Review

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-local: no wire, saved-data or cross-version effect; runs wherever the agent runs (macOS, Linux, Windows, SSH and WSL hosts).

Author: @BrennanKB5

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)

…nowledges its task

Claude's adapter dropped an echoed send from the sends it holds before
reporting the send's answer to the host, and emitting the echo's turn
can publish at once. That publish read the still-pending send as held
by nobody, so it read as "couldn't confirm": orchestration worker-start
then closed the new Claude worker, and every other answer wait and
client saw a one-frame doubt.

A frame's settled sends now stay held until their answers are reported,
when the host's handover window takes them over; a Stop's rejected
sends are released and reported one at a time, as Pi and ACP already do.

Tracked in STA-11160.
@brennanb2025
brennanb2025 marked this pull request as ready for review October 19, 2026 05:36
@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: b157c036-8440-449e-8814-0a39e1459fd8
📥 Commits

Reviewing files that changed from the base of the PR and between 6149209 and c5358a5.

📒 Files selected for processing: 2
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
💤 Files with no reviewable changes: 1
  • src/main/native-chat/agent-session-wire/structured-agent-session-host.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

Provider sessions expose held-send identifiers and record held sends and observation timestamps for host exit handling. Late dispatch settlement remains held during journal writes, and failed writes publish work-changed notifications. Provider-idle release of unanswered dispatches is removed. Claude treats completed without started as an uncertain outcome and releases the waiter in doubt. Tests cover provider send tracking, settlement failures, and exit handling.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to c5358

The reviewed lifecycle settles accepted sends and leaves queued prompts pending until their turn begins. No concrete user-facing regression remains established, so the change appears ready to merge.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 48.48% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 45 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 identifies the main change: restoring Working-only-while-held behavior without shutting down Claude chat workers.
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.
Description check Passed The description covers the required sections and clearly explains the user impact, mechanism, rationale, testing, scope, and checklist status. It identifies STA-11160 but does not provide the required…
Full details: Description check

Explanation

The description covers the required sections and clearly explains the user impact, mechanism, rationale, testing, scope, and checklist status. It identifies STA-11160 but does not provide the required issue link or use the required Fixes format. Visual proof is still pending, and the manual-test and command-check boxes remain unchecked.

✨ 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 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: 6e2a5d80-387c-4cc4-917b-76a61e90f23c
📥 Commits

Reviewing files that changed from the base of the PR and between 07bf286 and abc2ebf.

📒 Files selected for processing: 5
  • src/main/claude/claude-command-lifecycle.ts
  • src/main/claude/claude-structured-dispatch.ts
  • src/main/claude/claude-structured-session-acquisition.ts
  • src/main/claude/claude-structured-session-state.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-claude-opening-send-hold.test.ts

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

Comment thread src/main/claude/claude-structured-session-acquisition.ts
@brennanb2025 brennanb2025 changed the title fix(native-chat): a Claude chat worker keeps running after Claude acknowledges its task fix(native-chat): bring back Working-only-while-held (#30134) without shutting down Claude chat workers Oct 19, 2026
@brennanb2025

brennanb2025 commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor Author

Review loop status (head 23e7cc2ce40)

Problem. With #30134 on main, orca orchestration worker-start --agent claude shut a new Claude chat worker down the moment Claude acknowledged its task ("The dispatch preamble was submitted but not acknowledged (unknown)", chat "closed by Orca"). Cause: when Claude echoed a message, its adapter stopped counting the message as held before the host started holding it, and emitting the echo's turn publishes immediately; that one update read the message as held by nobody, so it read as unconfirmed, and worker-start closed the chat. #30134 was reverted on main (#30572) as a stopgap.

User-facing change. #30134 is back (Claude, Pi and ACP chats show Working only while the agent holds a message, so a failed save no longer leaves a chat stuck on Working), and Claude chat workers started from orchestration keep running and answer. Messages go from sent to received with no unconfirmed moment.

Fixed in review. Claude's deliberate close (Stop) released held messages before the host had taken over; it now releases them only after (same rule as the echo fix). Merges of main resolved: #28046 (accept messages while any agent starts), the Pi session's sealed end event, #30982's options-reported event (kept inside the hold, which matters because it publishes immediately).

Deferred, each tracked: answer waits still accept an in-memory "unknown" (send-lifecycle Stage B, STA-11448); worker-start still closes a worker whose first message is genuinely unconfirmed (product call, STA-11449). Pre-existing option-check refusals found by the final reference review are outside this PR: Claude fixed by #30982 (merged), Pi and ACP tracked in STA-11450.

Verified.

  • Review: 4 rounds (fresh reviewers, a continuation reviewer, a performance pass, a final whole-mechanism comparison); no open P0–P2. The last merge was checked narrowly (resolution correct, nothing dropped).
  • CI green on this head. 57 targeted Claude, Pi, ACP and host test files (497 tests) pass on this head; the regression test fails without the fix.
  • Real QA on this head (M4Air, Claude QA lead, Codex driver, real Claude/Codex/Pi through the load balancers, throwaway homes under /tmp): worker-start Claude task + follow-up 2/2 PASS; Claude message sent 328 ms after opening the chat accepted and answered; Codex two turns PASS; Pi two turns PASS (Pi 1.3.0; the machine's Pi 0.73.1 is below Orca's chat minimum); restart PASS.
  • Real QA on the previous head 9246b8a10f0 (Windows high spec), main vs branch: worker-start 3/3, send while starting, Codex/Pi/OMP turns, Stop with a queued follow-up, upgrade from main's saved chats, restart: no regression vs main.

Not verified: Pi or ACP sends while the chat is still starting are silently dropped on both main and this branch (not caused by this PR).

For Brennan's review

Claude chat worker started with worker-start, task then follow-up (this head):
branch-A-1-alpha
branch-A-1-beta
branch-A-1-after

Codex and Pi chats (this head):
branch-C-codex-5
branch-C-pi3-5

After restart (this head):
branch-D-3

Three places where a human judgment matters:

  1. claude-structured-session-acquisition.ts#L127-L171: the core fix. Messages a frame settled stay counted as held until their answers reach the host. Intended side effect: if emitting the frame throws, the answers are still reported.
  2. claude-structured-session-close.ts: when Orca can't prove the Claude process exited, queued messages stay held and the chat keeps showing Working until a later close, the exit, or quitting Orca (same as main).
  3. orchestration-structured-worker-session.ts (unchanged): worker-start closes a worker on a genuine first-message doubt (STA-11449).

…orker-start-ack

# Conflicts:
#	src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
…orker-start-ack

# Conflicts:
#	src/main/acp/acp-structured-acquire.ts
#	src/main/acp/acp-structured-session.ts
#	src/main/claude/claude-structured-session-adapter.ts
#	src/main/codex/codex-structured-session-acquire.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-late-dispatch.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts
#	src/main/pi/rpc-configured-default.test.ts
#	src/main/pi/rpc-session-adapter.test.ts
#	src/main/pi/rpc-session-adapter.ts
#	src/main/pi/rpc-session.ts
#	src/main/runtime/structured-agent-session-failure-logging.test.ts

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

🧹 Nitpick comments: 1
src/main/acp/acp-structured-host-lifecycle.test.ts: 1

297-297: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test the handover while acceptLate is pending.

mockRejectedValue rejects immediately. The working-state assertion runs while Grok still holds the send, so this test cannot detect a regression that drops the handover during a pending journal write. Use a deferred acceptLate result. Assert working() after Grok releases the send but before the write settles, then reject the write and retain the final assertions.

Based on learnings: use a controllable settlement when a test must assert an intermediate asynchronous state.

Source: Learnings


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 6874ae72-455b-4583-a571-a40651822a97
📥 Commits

Reviewing files that changed from the base of the PR and between 0c85113 and 6149209.

📒 Files selected for processing: 28
  • src/main/acp/acp-structured-acquire.ts
  • src/main/acp/acp-structured-child.ts
  • src/main/acp/acp-structured-host-lifecycle.test.ts
  • src/main/acp/acp-structured-host.test-support.ts
  • src/main/acp/acp-structured-session-adapter.ts
  • src/main/acp/acp-structured-session.ts
  • src/main/claude/claude-structured-dispatch.ts
  • src/main/claude/claude-structured-session-acquisition.ts
  • src/main/claude/claude-structured-session-adapter.ts
  • src/main/claude/claude-structured-session-state.ts
  • src/main/codex/codex-structured-session-start.ts
  • src/main/codex/codex-structured-session-state.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-attach-publish.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-exit-ends-record.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-event-recovery.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handing-over.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-late-dispatch.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-mutation-context.ts
  • src/main/pi/rpc-message-dispatch.ts
  • src/main/pi/rpc-runtime-registration.ts
  • src/main/pi/rpc-session-adapter.test-fixture.ts
  • src/main/pi/rpc-session-adapter.test.ts
  • src/main/pi/rpc-session-adapter.ts
  • src/main/pi/rpc-session.ts
💤 Files with no reviewable changes: 3
  • src/main/pi/rpc-runtime-registration.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
  • src/main/pi/rpc-session-adapter.test-fixture.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; 3 remain after this review.

@brennanb2025
brennanb2025 merged commit 92732bb into main Oct 21, 2026
44 checks passed
@brennanb2025
brennanb2025 deleted the brennanb2025/claude-worker-start-ack branch October 21, 2026 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant