Repository navigation
fix(claude): start a Claude chat with its saved options and send the first message at once - #25152
Conversation
…rame only a SessionStart hook sends Startup waited for system/init or a SessionStart hook frame as well as the initialize answer. Before the first turn only a SessionStart hook sends one, and Orca adds that hook only through its optional status hooks, so with them off the first message was held forever. Startup now lands on the initialize answer; a start frame already seen is still checked, and one naming another session ends a started session. The deadline drops to 90 s so it fires inside the host's 120 s start wait.
… another session's frame Claude answers initialize only after its SessionStart hooks finish, so a total-time deadline would fail every start behind a slow hook. Each start frame now restarts the clock. A frame naming another session fails a start still waiting on initialize at once, as before.
…n the hook-less real test
Review and QA summaryThe problem. In a Claude chat, the first message could show "Working…" with a timer and a Stop button forever, and Claude never got it. There were two causes:
What changes for you.
What the review changed. It ran three review rounds, an architecture check against the common pattern, and a release-readiness pass.
Deferred (none blocking).
Verified.
Not verified.
|
📝 WalkthroughWalkthroughClaude sessions now launch with saved structured options and can accept messages before initialization completes. Startup and exit handling distinguish unanswered initialization from queued work and from work handed to a proven child. The change also adds launched-model retirement reporting and updates session-option persistence and test coverage. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to The queued card currently reaches adapter dispatch after host-stop clears the pause. Adding the dispatch assertion would protect that behavior from regression; no concrete production failure remains, so the change is mergeable. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Chats can become ready without a startup notification, while silent starts are stopped. The reviewed paths retain session ownership, message-delivery gating, and permission checks. Wider deployment and security coverage remains incomplete. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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. Comment |
There was a problem hiding this comment.
Important
One race in the new proof state machine: onRefusal is installed one async step after the last proof read, so a foreign-session frame delivered in that window is dropped and the held message is sent. Details inline.
Reviewed changes
- Start on the
initializeanswer.readClaudeStartupFactsno longer waits for a session frame; it wrapsconnection.initializationResult()in the startup deadline and takes the frame only if it arrived (initProof.seen()), otherwiseinitisnull. - 90s silence deadline.
CLAUDE_STARTUP_DEADLINE_MS = STRUCTURED_AGENT_SESSION_START_WAIT_MS - 30_000, re-armed on each matchingSessionStartproof frame, with anORCA_E2E_CLAUDE_STARTUP_DEADLINE_MSoverride clamped to the default. - Foreign-session handling. Acquisition calls
initProof.refuse();settleClaudeSessionStartupinstallsonRefusalso a post-start foreign frame ends the session.createClaudeInitProofnow tracks anoutcomeso a matching proof is final. - Auth diagnostic source.
claudeAuthDiagnostic(initialization, init, settings)readsaccount.apiKeySourcefrom theinitializeresult (the SDK'sAccountInfocarries it) and falls back to the init frame. - Tests. Startup deadline / re-arm / no-frame / wrong-session unit cases, host held-message and fault cases, a real-CLI no-hooks case, plus scripted-runtime
sendsNoStartFrameand fakeinitProofsupport.
ℹ️ Nitpicks
readClaudeStartupFactsstill declaresproviderSessionIdin its input type (claude-structured-session-startup.ts:159) and acquisition still passes it (claude-structured-session-acquisition.ts:285), but the explicitinit.providerSessionId !== input.providerSessionIdcheck was removed and nothing reads it now. Drop the field and the call-site argument, or the parameter reads as if the check still exists.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
| return | ||
| } | ||
| // From here no read of the proof is pending, so a frame naming another session ends it. | ||
| facts.initProof.onRefusal = (error) => { |
There was a problem hiding this comment.
onRefusal is installed only after await input.facts, one async step after readClaudeStartupFacts does its last seen(). A foreign-session frame delivered in that window calls refuse(), which stores the error in outcome and invokes onRefusal while it is still null — the refusal is dropped, startup lands proven, and the held message is delivered to a child that named another session. The SDK resolves control_response and enqueues message frames in the same read loop, so this interleaving is reachable.
Technical details
# A refusal can be dropped between the last proof read and the `onRefusal` install
## Affected sites
- `src/main/claude/claude-structured-session-startup.ts:191` — the last `seen()` runs inside the continuation that resolves `input.facts`; after it there is no yield, so the facts promise resolves at the end of that continuation.
- `src/main/claude/claude-structured-session-startup.ts:277-287` — `settleClaudeSessionStartup` awaits `input.facts` (one microtask) and only then assigns `onRefusal`. A frame delivered in that microtask calls `refuse()` first.
- `src/main/claude/claude-structured-session-startup.ts:72-80` — `refuse()` writes `outcome` then calls `onRefusal?.()`; a null handler discards it. Nothing re-reads `outcome` afterwards (`applyClaudeStartupFacts` never calls `seen()`).
## Required outcome
A refusal that lands after the second `seen()` and before startup lands must still fail the start, so the held message is rejected instead of delivered against a foreign session.
## Suggested approach
After assigning `onRefusal`, consult the proof once more (`facts.initProof.seen()` throws when it was refused, landing in the existing `catch`), or have `refuse()` retain a pending handler that `settle` drains. A test that delivers a foreign frame after `initialize` answers but before startup lands would pin it.…es its first message at once Saved model, effort, Fast and permission mode are passed as launch options, checked against the account's cached model catalog, instead of restored by control requests after initialize. With nothing left to restore, the host no longer holds a message until the CLI answers initialize, and the 90 s startup deadline is gone. A Stop on a start that never answers ends the child and settles what it was handed as stopped. A failed result that repeats the turn's own API error reply writes no second row.
…was handed, with one row With no start-hold the delivery loop no longer sees a host stop of a start it waited on. The child's end now rejects what it handed over with the host-stopped words and writes the one row, as an exit of its own would; an idle start the host stops still goes quietly.
…itialize answers Rewrites the tests that encoded the start-hold, the startup deadline and the option restore to the new contract, and adds: saved options at launch (catalog checks, bypass, fresh-session Fast), a message written before initialize answers (adapter and runtime), Stop on a start that never answers (stopped, child closed, nothing working), and an API error said once.
The shared turn fold already shows a failed turn's error once after it settles, as an error; dropping the row left the CLI's synthetic reply looking like something Claude said.
… model on the CLI's word, and never leave an unrun message in doubt - Saved model, effort and Fast are launched as picked; only values no Claude can parse are left out. The pre-spawn cache check is gone. - A saved Fast on for a new conversation is applied once the settings readback shows no per-session opt-in (dropped when there is one, or when the model is listed without Fast), with nothing waiting on it; the record keeps the pick. - Under an Agent Permissions bypass, a saved narrower mode launches with the allow flag so bypass stays reachable. - A turn whose reply is the CLI's model_not_found for the launched model drops that model from the record; the launch's own row for it is kept out of the account model cache. - A child that ends before it answered initialize, for any reason, settles every message it was handed as not sent (cancelled for a Stop). - A launched effort the CLI reports only as `applied.effort` is confirmed from there. - The untimed-initialize comment is back to main's text. - A real-CLI test for a message written before initialize answers, under saved options.
…t fixtures The close's ended event is typed as the adapter event so its optional startupUnanswered spread fits exactOptionalPropertyTypes; two tests guard the fixture's optional generation, and the hung-start fixture records initialize on the fake connection it holds.
…ast is dropped, no allow flag - `options-skipped` carries the retired value; the record drops it only while it still holds it. - `started` carries the values a heal retired, and the record does not take them back from the CLI's report of the same value. - A saved Fast on a new conversation is applied before `started`: a refusal drops it and records it skipped, as main's refused restore did; silence keeps it wanted and unconfirmed. - A saved narrower mode under an Agent Permissions bypass launches without any bypass flag again: the allow flag is one older CLIs reject at start. Kept as a known limit. - The real-CLI test asserts the message was written before initialize answered. - The fake reports a launch effort only under `applied`, and a misplaced doc comment moves back.
…tart-deadline # Conflicts: # src/main/claude/claude-structured-options-context-window.test.ts # src/main/claude/claude-structured-session-acquisition.ts # src/main/claude/claude-structured-session-adapter.ts # src/main/claude/claude-structured-session-startup.test.ts
…Fast is applied The Fast apply on a new conversation now runs after `started`, so a Stop interrupts a running first turn and an option write is not refused while the round trip is out. A refusal drops the pick through `options-skipped`, in order after `started`; silence keeps it unconfirmed. The launch's unreachable skipped-model branch is gone.
… live; comments match the no-hold design When the CLI says the launched model does not exist, the live child is also put back on the CLI's own default (set_model with no model, fire-and-forget), so later messages in the same chat run; a user's pick sent after it wins, and a refused or unanswered reset only logs. Comments that still described the start-hold or the option restore now describe the launch options and the handed-over, never-echoed rule.
…tart-deadline # Conflicts: # src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts # src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts
…red is kept as main keeps an unsent one A child that ended before it answered initialize ran nothing it was handed, the same fact as a send accepted and never handed over. Its end now settles those sends exactly as the chat settles a queued send for that end: a quit keeps a person's message as a held card (restart words), a close keeps it as a held card (closed words), a person's Stop withdraws it as cancelled, and a host stop fails the start with one row.
…fers no resume for the message it keeps as a card The restart snapshot now reads the same never-answered fact the exit does, so a message handed to such a start counts as queued work, not as work to resume. The retired-model reset comment names the default it really applies.
…with the spawn options that use them Keeps claude-structured-launch-resolution.ts within max-lines once merged with main, and names the hung-start test envelope's field type.
…tart-deadline # Conflicts: # src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts
…the agent Arguments' own flags Main now passes the saved agent Arguments to the Claude child, and the SDK writes them after its own options. An Arguments --model or --effort therefore reached the CLI as a second flag after the chat's saved pick (a commander CLI keeps the last), and a saved Fast's launch settings replaced an Arguments --settings file outright. The saved model and effort now stand in for the Arguments' flags, and a saved Fast beside an Arguments --settings is applied by the start instead of at launch.
…carries fastModeAtStart
An unannotated vi.fn() inferred @vitest/spy's internal Procedure, which CI's typecheck cannot name in the factories' inferred return types (TS2883).
Lists the hung-start Stop test, renames the send-during-startup entry from its old name, and carries main's own two entries from #26010 so the boundary test passes before the next merge.
Review and QA summary (reworked design)The problem. A Claude chat's first message could show "Working…" forever and never reach Claude, in two ways:
Both came from Orca holding every message until Claude finished starting, so it could re-apply the chat's saved model, permission mode, effort and Fast afterwards. What changes for you.
How the design got here.
What the review rounds fixed in the reworked design:
Deferred (none blocking).
Verified.
Not verified.
CI. Green on the final head dbcc7b5: all checks pass, including typecheck, lint, all 5 unit shards, packaging (macOS, Linux, Windows) and Mobile Checks. Red checks seen on earlier heads and how they were classified:
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
64170f52-60b0-47e3-a7d4-7f8e91304a77
📒 Files selected for processing (103)
config/scripts/vitest-sqlite-database-runtime-files.mjsconfig/scripts/vitest-sqlite-runtime-files.mjssrc/main/claude/claude-adapter-capture-replay.test.tssrc/main/claude/claude-command-lifecycle.test.tssrc/main/claude/claude-fold-steer-receipt.test.tssrc/main/claude/claude-stream-json-connection.test.tssrc/main/claude/claude-structured-acquisition-launch.tssrc/main/claude/claude-structured-child-env.tssrc/main/claude/claude-structured-dispatch-test-support.tssrc/main/claude/claude-structured-effort-default-at-rest.test.tssrc/main/claude/claude-structured-effort-reporting.test.tssrc/main/claude/claude-structured-event-delivery.tssrc/main/claude/claude-structured-init-proof.tssrc/main/claude/claude-structured-launch-resolution.tssrc/main/claude/claude-structured-model-preflight.test.tssrc/main/claude/claude-structured-options-context-window.test.tssrc/main/claude/claude-structured-options.test.tssrc/main/claude/claude-structured-options.tssrc/main/claude/claude-structured-prompt-ownership.tssrc/main/claude/claude-structured-real-cli-fold.test.tssrc/main/claude/claude-structured-real-cli.test.tssrc/main/claude/claude-structured-retired-model.test.tssrc/main/claude/claude-structured-retired-model.tssrc/main/claude/claude-structured-session-acquisition-options.tssrc/main/claude/claude-structured-session-acquisition.tssrc/main/claude/claude-structured-session-adapter.test.tssrc/main/claude/claude-structured-session-adapter.tssrc/main/claude/claude-structured-session-close.tssrc/main/claude/claude-structured-session-exit-lifecycle.tssrc/main/claude/claude-structured-session-options.tssrc/main/claude/claude-structured-session-publication.tssrc/main/claude/claude-structured-session-recovery.test.tssrc/main/claude/claude-structured-session-startup-state.tssrc/main/claude/claude-structured-session-startup.test.tssrc/main/claude/claude-structured-session-startup.tssrc/main/claude/claude-structured-session-state.tssrc/main/claude/claude-structured-session-test-support.tssrc/main/claude/claude-structured-spawn-options.test.tssrc/main/claude/claude-structured-spawn-options.tssrc/main/claude/claude-structured-start-fast-mode.test.tssrc/main/claude/claude-structured-start-fast-mode.tssrc/main/claude/claude-structured-startup-settled-test-support.tssrc/main/claude/claude-subagent-permission-request.test.tssrc/main/native-chat/agent-session-journal/journal-pending-submission-recovery.tssrc/main/native-chat/agent-session-journal/journal-store.tssrc/main/native-chat/agent-session-journal/journal-unsent-send-hold.tssrc/main/native-chat/agent-session-wire/structured-agent-session-accept-then-deliver.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.tssrc/main/native-chat/agent-session-wire/structured-agent-session-adapter.tssrc/main/native-chat/agent-session-wire/structured-agent-session-chat-stop.tssrc/main/native-chat/agent-session-wire/structured-agent-session-child-close.tssrc/main/native-chat/agent-session-wire/structured-agent-session-child-exit.tssrc/main/native-chat/agent-session-wire/structured-agent-session-claude-compact-stop.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-claude-echo-working.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-claude-hung-start-stop.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-claude-queued-stop.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-ends-session.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-exit-ends-record.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-note-end.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-turn-end.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-command-turn.tssrc/main/native-chat/agent-session-wire/structured-agent-session-conversation-close.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.tssrc/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-dead-generation-settlement.tssrc/main/native-chat/agent-session-wire/structured-agent-session-delivery-hold.test-fixture.tssrc/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.tssrc/main/native-chat/agent-session-wire/structured-agent-session-event-recovery.tssrc/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.tssrc/main/native-chat/agent-session-wire/structured-agent-session-host-types.tssrc/main/native-chat/agent-session-wire/structured-agent-session-idle-sweep.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-main-agent-working-agreement.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-option-restoration.tssrc/main/native-chat/agent-session-wire/structured-agent-session-owed-work-release.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-provider-child-record.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-provider-started.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-provider-started.tssrc/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig-provider.test-fixture.tssrc/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.tssrc/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-queued-pause-lift.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-queued-withdrawn-draft.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-host.tssrc/main/native-chat/agent-session-wire/structured-agent-session-restart-witnesses.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-restart-witnesses.tssrc/main/native-chat/agent-session-wire/structured-agent-session-start-failure-writer.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-steer-while-stopping.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-stop-event-binding.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-stop-event-entries.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-stop-event.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-turns.tssrc/main/native-chat/agent-session-wire/structured-agent-session-unrun-send-hold.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-unsent-send-hold.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-working-at-teardown.tssrc/main/native-chat/agent-session-wire/structured-conversation-compaction.test.tssrc/main/runtime/claude-structured-retired-saved-model.test.tssrc/main/runtime/claude-structured-send-during-startup.test.tssrc/main/runtime/claude-structured-send-restart-dies-before-dispatch.test.tssrc/main/runtime/claude-structured-startup-fault-is-not-an-exit.test.tssrc/main/runtime/claude-structured-startup-unanswered-control-request.test.tssrc/main/runtime/structured-agent-session-lifecycle-delivery.tssrc/main/runtime/structured-claude-runtime-adapter.tssrc/main/runtime/structured-claude-scripted-runtime-test-support.ts
💤 Files with no reviewable changes (1)
- src/main/native-chat/agent-session-wire/structured-agent-session-main-agent-working-agreement.test.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.
| /** A send handed to the agent that it has neither echoed nor refused. Handed to a child that never | ||
| * answered its start, it ran nowhere. */ | ||
| export function isUnansweredHandedOverSubmission( | ||
| entry: Pick< | ||
| AgentJournalSubmission, | ||
| 'handoverRecorded' | 'dispatchState' | 'handedOverAt' | 'recovered' | ||
| > | ||
| ): boolean { | ||
| return ( | ||
| !isQueuedAgentJournalSubmission(entry) && | ||
| (entry.dispatchState === 'pending' || | ||
| (entry.dispatchState === 'unknown' && entry.recovered !== true)) | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '1,115p' src/main/native-chat/agent-session-journal/journal-unsent-send-hold.ts
sed -n '175,205p' src/main/native-chat/agent-session-wire/structured-agent-session-working-at-teardown.ts
sed -n '120,220p' src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts
rg -n 'interface AgentJournalSubmission|type AgentJournalSubmission|holdUnsentSends\(' src/main/native-chat/agent-session-journal src/main/native-chat/agent-session-wireRepository: stablyai/orca
Length of output: 12920
🏁 Script executed:
set -e
printf '%s\n' '--- holdUnrunSends declarations/callers ---'
rg -n -F -- 'holdUnrunSends' src/main/native-chat
printf '%s\n' '--- predicate declarations/consumers ---'
rg -n -F -- 'isUnansweredHandedOverSubmission' src/main/native-chat
printf '%s\n' '--- fence/generation comparisons near relevant code ---'
rg -n -e 'submission\.(fence|generation|owner|handover)' -e '(\.fence|\.generation).*(submission|handover)' src/main/native-chat/agent-session-journal src/main/native-chat/agent-session-wire || true
printf '%s\n' '--- child-exit tail and teardown block ---'
sed -n '175,245p' src/main/native-chat/agent-session-child-exit.ts
sed -n '150,225p' src/main/native-chat/agent-session-wire/structured-agent-session-working-at-teardown.ts
printf '%s\n' '--- hold-unrun implementation context ---'
sed -n '130,190p' src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts
printf '%s\n' '--- holdUnsentSends call contexts ---'
sed -n '140,180p' src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts
sed -n '90,125p' src/main/native-chat/agent-session-wire/structured-agent-session-conversation-open.ts
sed -n '45,85p' src/main/native-chat/agent-session-wire/structured-agent-session-event-recovery.ts
sed -n '55,85p' src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.tsRepository: stablyai/orca
Length of output: 2320
🏁 Script executed:
set -e
printf '%s\n' '--- hold function remainder ---'
sed -n '80,190p' src/main/native-chat/agent-session-journal/journal-unsent-send-hold.ts
printf '%s\n' '--- unrun wiring ---'
rg -n -C 8 -F -- 'holdUnrunSends' src/main/native-chat
printf '%s\n' '--- teardown complete relevant function ---'
rg -n -C 12 -F -- 'startAnswered(sessionId) === false' src/main/native-chat/agent-session-wire/structured-agent-session-working-at-teardown.ts
sed -n '1,120p' src/main/native-chat/agent-session-wire/structured-agent-session-working-at-teardown.ts
sed -n '120,230p' src/main/native-chat/agent-session-wire/structured-agent-session-working-at-teardown.ts
printf '%s\n' '--- submission type and journal row ownership fields ---'
rg -n -C 15 -e 'export (interface|type) AgentJournalSubmission' -e 'interface AgentJournalSubmission' src/shared src/main/native-chat
rg -n -C 8 -e 'fence' src/main/native-chat/agent-session-journal/journal-row-writer.ts src/main/native-chat/agent-session-journal/journal-store.tsRepository: stablyai/orca
Length of output: 11108
🏁 Script executed:
set -e
printf '%s\n' '--- submission type ---'
rg -n -C 18 -e 'export (interface|type) AgentJournalSubmission' -e 'interface AgentJournalSubmission' src/shared src/main/native-chat
printf '%s\n' '--- dispatch observation ownership ---'
sed -n '1,80p' src/main/native-chat/agent-session-journal/journal-dispatch-observation.ts
printf '%s\n' '--- dispatch row assignment ---'
sed -n '1,45p' src/main/native-chat/agent-session-journal/journal-dispatch-reducer.ts
sed -n '90,125p' src/main/native-chat/agent-session-journal/journal-submission-fold.ts
printf '%s\n' '--- teardown selection and consumers ---'
sed -n '175,215p' src/main/native-chat/agent-session-wire/structured-agent-session-working-at-teardown.ts
printf '%s\n' '--- relevant tests ---'
rg -n -C 6 -e 'unrun|UnansweredHandedOver|startAnswered|fence' src/main/native-chat/agent-session-journal/journal-unsent-send-hold.test.ts src/main/native-chat/agent-session-wire 2>/dev/null | head -240Repository: stablyai/orca
Length of output: 36730
Filter unrun submissions by the current execution fence
AgentJournalSubmission.fence identifies the latest dispatch attempt or recovery. The unanswered-child exit passes its current fence to holdUnsentSends, but the unrun predicate ignores that fence. A pending submission from an answered predecessor can therefore be rejected by the replacement child. For a person message, this can create an incorrect held card.
The teardown filter has the same defect and can omit the predecessor submission from restart-work calculation. Apply the same fence check in both callers.
Suggested fix
export function isUnansweredHandedOverSubmission(
entry: Pick<
AgentJournalSubmission,
- 'handoverRecorded' | 'dispatchState' | 'handedOverAt' | 'recovered'
- >
+ 'handoverRecorded' | 'dispatchState' | 'handedOverAt' | 'recovered' | 'fence'
+ >,
+ fence: number
): boolean {
return (
+ entry.fence === fence &&
!isQueuedAgentJournalSubmission(entry) &&
(entry.dispatchState === 'pending' ||
(entry.dispatchState === 'unknown' && entry.recovered !== true))
)
}- ? isUnansweredHandedOverSubmission(entry)
+ ? isUnansweredHandedOverSubmission(entry, input.fence)- !(unanswered && isUnansweredHandedOverSubmission(submission))
+ !(
+ unanswered &&
+ isUnansweredHandedOverSubmission(submission, session.child.fence)
+ )…tart-deadline # Conflicts: # src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-binding.test.ts # src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-entries.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-entries.test.ts (1)
425-425: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert adapter dispatch for the held card.
rig.handoff(held)can return the journal submission before delivery runs, so this assertion can pass without dispatching the card to the adapter. The current host-stop path reachesrig.dispatch; assert that it receives this handoff ID.🐛 Suggested fix
- await eventually(async () => expect(await rig.handoff(held)).toBeDefined()) + await eventually(async () => { + const handoffId = await rig.handoffId(held) + expect(rig.dispatch).toHaveBeenCalledWith( + expect.objectContaining({ clientMessageId: handoffId }) + ) + })
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
bc6b7c3f-a4c9-4ade-8308-86e286c4ed04
📒 Files selected for processing (6)
src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-note-end.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-queued-card-holds.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-queued-pause-lift.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-stop-event-binding.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-stop-event-entries.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-unsent-send-hold.test.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.
…nary Main's #25152 keeps a message handed to a Claude start that never answered as a card. Here that card is ordinary: the settlement returns its newest sequence as this branch's callers expect, and the close's or the reopen's mark holds it until the next turn, so its test expects no kept hold.
…rough the held start's release Main's #25152 holds the start these tests send into; quitting without releasing it left the quit waiting.
… and follows the chat's next turn (#25960) * fix(native-chat): drop the queue-paused header and Resume button A Stop, a restart or /clear holds the queued cards. The hold stays; only the header row naming why, and its Resume button, go. A held card shows no caption, and its own Steer, or any new message, releases the queue. * test(native-chat): type the unknown hold reason a newer host may publish * fix(native-chat): a held card offers Send, not Steer, when no turn runs Steer vs Send now follows whether a turn is running, not the card's hold, so a card held after a Stop, a restart or /clear reads Send. * fix(native-chat): the queue sends past held cards instead of stalling behind them A card queued after a Stop (or written after a restart or /clear) sent only once the cards held before it were released; with no header to explain or release the hold, it sat silently. The next sendable card now skips held cards; a returned card still blocks what is behind it. * fix(native-chat): the queue's send of a card is the person's turn, so held cards follow it After a Stop, a card queued later sent past the held cards, but the queue recorded that send as Orca's own turn. It never ended the Stop's pause, so the held cards then waited forever with nothing on the card saying why. A queued card is always something the person wrote: only the client send RPC may now create one. The queue's send of it is therefore recorded as the person's turn, which ends the Stop's pause once the agent takes it, and the held cards then drain in order. * fix(native-chat): a queued card carries its author, so the queue's send of it is that author's turn Main now lets Orca's own sends ask to queue (sendAgentTurn's 'queue' delivery), so "every card is a person's" no longer holds by refusing host sends. Each card records who wrote it (the submission's client/host vocabulary) in a new nullable column; the drain records that origin, so a person's card ends a Stop's pause and Orca's does not. /clear carries the author. Rows from before the column read as a person's. The userSend-only admission gate is removed. * docs(native-chat): state why an unrecorded card author reads as a person's * fix(native-chat): a restart holds only cards written before it, and an idle held queue offers Resume A restart's pause held every waiting card, including one a person typed after the restart while Orca's own continuation ran, and nothing released it except a per-card Send. It now holds only cards another host process wrote, the same way a Stop holds only cards queued before it. The composer's primary button becomes Resume (Play) while nothing is typed, no turn runs and the host holds a card Resume would send, whatever held it (Stop, restart or /clear). It calls the existing agentSession.queuedMessagesResume, guarded against a second press in flight. A card nothing holds keeps the run going between a turn's end and the queue's send of it, so its Steer no longer flips to Send for the frame in between. * fix(native-chat): the host publishes which pause holds each queued card The host published one pause for the whole queue, so a client held every waiting card while it was set. Between a turn's end and the queue's send of a card queued after a Stop or restart, the composer could flash Resume and the cards Send, and a card queued after a Stop lost its "Waiting for your answer" caption. Each published card now carries an optional `heldBy`: the pause holding it, or null, derived from the same rule the drain reads. A client holds only those cards; against a host without the field it falls back to the queue-level pause. * test(native-chat): Resume needs the queue capability and is disabled whenever Send is * docs(native-chat): describe per-card holds in the queue contract and table comments * fix(native-chat): the composer goes from Resume straight to Stop, and Resume returns focus After Resume, the host lifts the hold in one update and sends the first card in a later one. In between nothing was running, so the composer's button flashed a disabled Send. A card nothing holds now keeps the queue's run going for the button too: an empty composer shows Stop, disabled until the turn starts. Not when the host refuses every send (a rewind whose outcome is unknown, read from its status), where nothing is coming. The same fix removes the Stop, Send, Stop flip between queued turns. Resume disables the button, which dropped keyboard focus; focus now returns to the composer. * fix(native-chat): the host names the card its queue sends next, so the chat stays working across the gap A turn's end, or a Resume, and the queue's send of the next card commit as two host updates. In between nothing was running, so the working status, timer, pickers and composer button flipped for one update. The client guessed the drain from its own copy of the host's gates, which missed a /clear-replaced source and covered only the button. The queue publication now carries `nextQueuedMessageId`: the drain's own next card through the drain's own gate (`nextStructuredQueuedMessage`, which the drain step now calls), null whenever the host would refuse the send. The client derives one fact, the queue is about to send, and every working reader follows it; Stop stays disabled until a turn can be stopped. The client-side copy of the gates and the status-feed rewind read are removed. * test(native-chat): the queue's next card survives the coalescer, the reducer and a history page * test(native-chat): build the snapshot that names the next card through its helper * feat(native-chat): a held queue keeps its header row, and a new message asks before passing it The queue's header row ("Queue paused because you interrupted", or Orca restarted, or you cleared the conversation) comes back above the cards it holds, with Resume; it names the oldest held card's pause, as the host publishes it per card, and hides over cards held only on their own or returned. The header's Resume and the composer's share one in-flight guard. A held card reads Steer again whether or not a turn runs; a card held on its own or returned keeps Send. Sending a message while the header shows (Enter or the button) first asks "Send message?": Clear queue deletes every card and then sends (a failed delete sends nothing), Send message sends and keeps the cards, which follow the new turn, and dismissing sends nothing and keeps the draft. Host commands send as they are. * fix(native-chat): the paused row goes while your own message is on its way to lift it After "Send message" over a held queue, the row kept saying "Queue paused…" until the agent accepted the new turn. The chat now reads that gap from the outbox: while this composer's direct send is recorded by the host and not yet accepted, the controller shows no paused row (and so no Resume or confirmation). A refusal settles the entry and the row comes back, since the hold did not lift. Orca's own sends never enter this outbox, and the queue's send of a card goes under a fresh id, so neither hides it. Nothing is stored. * fix(native-chat): a "Send message?" choice is taken once, and a failed Clear queue is one toast The closing dialog stays mounted and clickable through its exit animation, and a double-click or a held Enter lands twice before any re-render, so Send message (or Clear queue) could send the captured message twice. The pending send now lives in a ref that the first choice takes; a second one finds nothing. Clear queue deletes one card at a time and stops at the first failure, so a failed press shows one toast instead of one per card. The dialog keeps its compact width at desktop sizes and the primitive's narrow-window gutter (`max-w-sm sm:max-w-sm`, as the other compact confirmations). * fix(native-chat): Clear queue's message goes out once, and keeps text typed while it waits After Clear queue, the message waited in the composer while the cards were deleted one by one. A second Enter in that window sent it again, and text typed meanwhile was wiped when the chained send was accepted. From the Clear queue choice until its message has gone out, the composer's structured send does nothing. The chained send (and Send message's) now carries the composition it was taken from, and the composer is cleared on acceptance only if it still holds exactly that, as host commands already do. Also: the v1 contract comment names `nextQueuedMessageId` and its absent- means-null fallback, and the own-send check returns at once on an empty outbox. * fix(native-chat): the queue carries on after any turn, in order, and a restart sends nothing by itself - Any accepted turn ends a Stop's or a /clear's pause, whoever sent it (a person, Orca's own messages, or the queue), and so does Resume. The card and submission author fields that only fed the old person-only rule are gone. - The queue sends strictly in order: a card never overtakes a held one. - After a restart nothing sends by itself and no paused row shows: the chat's next turn (the carry-on, or the person's own message) runs first, then the cards. - Resume and "Send message?" are offered only while nothing runs and no prompt waits. * fix(native-chat): after a restart no queue pause shows, and a card written before the next turn waits for it too * fix(native-chat): a quit hands no queued card off, and the paused row goes while any turn that will lift it is on its way - The queue stops handing cards off when the host tears down. A card sent during a quit was refused at close, and that refused send withdrew the chat's restart offer, so resuming after the relaunch sent nothing. - The host publishes no pause while a turn sent after it (your message, Steer, or Orca's own) waits for the agent; a refusal shows it again. This replaces the client's own-send check. - A card written after a restart is an ordinary card again: it waits while any card from before the restart still waits. * refactor(native-chat): the host's paused-row-while-a-turn-is-on-its-way check in one expression * refactor(native-chat): the composer's queue Resume rides the structured transport beside the held queue * fix(native-chat): the "Send message?" choice ends with the pause it asked about; tests follow main's draft props - The open dialog closes when the queue's pause lifts under it (Orca's mail, another client's Resume, any accepted turn): nothing is sent, the draft stays, and the next Enter sends as usual. The pending choice records the hold it was asked under; nothing new is stored. - The composer-field Resume test passes main's dropScopeKey/draftScopeKey. - The dialog test expects main's rule: only the sent text leaves the composer. * fix(native-chat): a message kept after a quit or close waits like every other card, and follows the chat's next turn A message Orca accepted but never handed to the agent before a quit, crash or close came back as a card held on its own ("Not sent yet — press Send"): the queue skipped past it, so later cards sent first, and only the person's own Send released it. It is now an ordinary card at the head of the queue that waits, with every card the chat closed with, for the chat's next accepted turn. One rule for a chat that was not running, derived from the journal: when this host first opens a chat (after a restart or crash), or a person closes it, and cards are waiting, a reopen mark is written (a tombstone carrier key, like the Stop and Resume marks). The cards queued before it wait until a turn is accepted or Resume comes after it; nothing sends by itself, and no paused row shows, a Stop's included. The idle sweep's own eviction writes nothing and changes nothing the person sees. A mark that cannot be written leaves the open working and holds from the open itself until the next turn; the next open marks again. A rewind restates the mark. /clear's carried cards also wait unshown. This replaces the host-instance comparison and its adoption write, and the 'kept' hold (stored 'kept' and legacy 'stopped' holds now read as none). * fix(native-chat): mark the reopen in the one open path, and keep an idle chat with waiting cards open Review round 1: the startup restore opened chats past the per-host first-open mark, so their cards could send by themselves after a quit or crash; a card mid-hand-off at the open got no mark; a close that left the chat open re-marked after every new send. - Every open marks when a card waits, or is mid-hand-off with its send unanswered. - The idle sweep keeps a chat's handle while cards wait, so its eviction never reopens one and stays invisible; it drops it on the next sweep once they leave. - A person's close marks once; its delivery re-check marks only when it settled a send. - A failed mark holds from where the mark would have gone. - A Stop made after a reopen shows its row. - The rig's restart is a real quit and relaunch. * fix(native-chat): review round 2: restore the dropped Resume and failed-Stop tests; a late mark starts where the chat stopped - Restores eight tests the previous commit dropped by mistake. - A mark the delivery loop or a close's re-check writes after a later send starts where the chat stopped, so that send still lifts it; a later mark never narrows an earlier, wider one. - Only the idle sweep's own close keeps a chat with a card waiting (or mid-hand-off) open, and it still releases an ended child's lease first; a person's close drops it as before. * test(mobile): a host-kept card's test stands in a Stop's pause, as this host publishes no restart pause Main's #24660 test published queuePause 'restarted', which this branch's wire type no longer lists, so the mobile tests typecheck ratchet failed. * refactor(native-chat): settle a restart's leftovers and mark them in one host-lifetime step Keeps the delivery loop under its line limit after main's Stopping change; no behaviour change. * test(native-chat): a kept card's Resume and failed-mark tests quit through the held start's release Main's #25152 holds the start these tests send into; quitting without releasing it left the quit waiting.

ELI5
A Claude chat's first message could stay on "Working…" forever, and Claude never got it. It happened in two ways:
Both came from one design choice: Orca held every message until Claude finished starting, so it could then re-apply the chat's saved model, permission mode, effort and Fast setting. Now Orca passes those saved settings when it launches Claude, and hands your message to Claude right away, the way the common pattern does. There is no start timer: a Claude that never answers looks like any turn that doesn't progress, and Stop ends it.
What Changed
The problem. A Claude chat starts a
claudeprocess. Orca held every message until that process answered itsinitializerequest and sent a frame naming its session, then re-applied the chat's saved options one by one, and only then delivered the message. I checked a real Claude CLI (2.1.280) with no message sent:SessionStarthook configured, Claude answeredinitializewithin 3 to 9 seconds and sent no session frame (watched for 25 seconds);SessionStarthook in its settings, the hook frame arrived at 6.5 seconds.Claude sends its own session frame only when a turn begins, and the turn couldn't begin until the start finished, so without the hook the first message waited forever. Separately, a Claude that stayed alive but never answered
initializeheld the message forever too.Before / After (what you see)
Mechanism
claudeStructuredSpawnOptions(claude-structured-spawn-options.ts) maps the chat's saved options onto the Claude Agent SDK's launch options:model,effort,permissionMode, and Fast through the launchsettings. Values no Claude can parse are left out. A saved permission mode is never wider than the Agent Permissions setting.awaitStartedis removed from the agent adapter interface; only the Claude adapter had it). The SDK writesinitializefirst and the message right after; Claude reads them in order.initializeanswer and its settings) to show the model list, confirm the options Claude actually runs with, and save them, but nothing waits on it. The old start-time option re-apply and the 90 second start timer this PR first added are both removed.initialize, it ran nothing, so a message already handed to it is settled exactly as Orca settles a message it never handed over: quitting or closing the chat keeps it as a held card, your Stop withdraws it, and Orca stopping an idle start fails the start. A quit then offers no "resume" for it.model_not_founderror for the model this process was launched with, Orca switches the running Claude back to its default model and clears that saved model from the chat. A model you picked meanwhile is kept. The model Claude adds to its own list just because it was launched with it is not saved into Orca's model list. Claude also usesmodel_not_foundfor a model not enabled on a Bedrock or Vertex deployment; the saved model is cleared there too, and you can pick it again once it's enabled.--modelor--effort, the chat's own saved pick replaces that flag (Claude keeps the last of two, so without this your Arguments would win). With nothing saved in the chat, your Arguments decide, as before. If your Arguments pass--settings, Orca keeps your settings and applies the chat's saved Fast right after start instead of at launch.Why
Differences from the common pattern
initializeor a session frame; no start timer.model_not_founderror. The common pattern keeps passing it.main.--settingsturn Fast on can run its first message in Fast, because Fast is applied right after start there. A retired saved model in a chat whose Arguments set--modelmoves to the account default until the chat restarts.Composes with in-flight work
Linked Issue
N/A (found by a user-side check of #24564).
Visual Proof
Hidden, isolated test builds on a second Mac with a stand-in
claude(never the real CLI) and agent status hooks off. All screenshots were taken by a non-Claude driver. After = this PR at ba2a171 (the later commits change lint, test types, and how saved options combine with your agent Arguments; the screens below are unaffected). Before =main(captured earlier at 3cc5e1d;mainstill holds the first message the same way).A. Status hooks off (Claude sends no session-started signal).
B. A slow start (the stand-in answers Orca's start request after 20 seconds). The message is handed over at once and shows as sent with Working and Stop; the reply arrives when Claude is ready. The stand-in's log shows the message reached it 19 seconds before it answered the start request.
C. Claude hangs (never answers). No timer ends it; Stop does.
D. The saved model applies to the first message after a restart. Pick "Sonnet (stand-in)", send, quit, relaunch the same profile, reopen the chat, send again. The relaunched
claudeprocess was started with--model sonnet(from the stand-in's log), and no model change was sent after it started.Not covered by these screenshots: permission mode (no picker writes it today; tests cover it), a retired model (needs the real API; tests cover it), the agent Arguments combinations (tests and a real-CLI probe cover them), mobile (it shows the same host records).
Testing
Unit and host tests (the production runtime, adapter, record store and host, with only the CLI scripted):
initialize, including with no session frame (hooks off);model_not_found, a model picked meanwhile is kept, and a late start report doesn't write it back;startedarrives before the Fast apply, so Stop and option changes don't wait on it.Each production change was checked by undoing it: its test fails.
New real-CLI test: a real Claude launched with saved options (sonnet, plan mode, low effort) runs a message written before it answered
initialize, under those options. Existing real-CLI test: a real Claude chat with every hook disabled starts and answers.Real Claude CLI suite on 6462ce7: 12 passed.
Typecheck:
tsconfig.node.json.I manually tested these changes locally
Automated tests added/updated, or explained why not below
AI Disclosure
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Cross-platform: launch options go through the Claude Agent SDK's own argument handling on every platform. Remote: Claude chats run on the Orca host that owns them (they are refused over SSH and WSL); nothing new crosses the connection to a client. Mobile and paired clients read the same host records.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)