Skip to content

Hold messages in the host until a chat's agent has finished starting - #26416

Merged
brennanb2025 merged 7 commits into
mainfrom
brennanb2025/agent-startup-host-attempt
Oct 8, 2026
Merged

brennanb2025 merged 7 commits into
mainfrom
brennanb2025/agent-startup-host-attempt

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 32 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1475 $\color{#cf222e}{\Huge{\mathbf{−}}}$​144 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1331
Prod 46 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1168 $\color{#cf222e}{\Huge{\mathbf{−}}}$​393 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​775

ELI5

When you open a native chat, Orca starts the agent behind it. Until now, each agent's own code decided whether the chat could take your message while that agent was still starting, so Claude, Codex, Grok, OpenCode and Pi all behaved a little differently. This PR moves that decision into the one shared part of Orca every agent goes through: your message is always accepted and saved straight away, and Orca hands it to the agent only once the agent has actually finished starting. If the agent never finishes starting, gets stopped, or the chat is closed first, Orca knows for certain the message was never sent and says so.

It also gives every agent the same rule for "this start is stuck": an agent that has started its process but says nothing at all for a minute is stopped, and no start may run longer than ten minutes. An agent that is slow but still working (a long upgrade migration, replaying a big history) is left alone.

This is the groundwork (PR 1). The follow-up moves Codex, Grok, OpenCode and Pi onto the "chat is usable while starting" path that Claude chats already have.

What Changed

The problem. Each agent's adapter decided when a chat became usable during startup:

  • Claude publishes the chat as soon as its process spawns and marks it "starting"; the host then wrote your message straight into Claude's input queue while Claude was still answering its first handshake.
  • Codex, Grok, OpenCode and Pi only hand the chat over after their whole handshake, so the chat waits on them (Grok took 5–8 s on the user's machine).

So the rules for "is this chat usable yet", "what happens to a message sent while starting" and "what if the start fails" were per-agent, and nothing bounded a start that hung (Codex, Grok, OpenCode and Pi handshake requests have no overall time limit). Worse, when a write to a starting agent threw, the host guessed "the agent was still starting, so the message can't have run" from the child's phase, rather than from what it actually handed over.

What the user sees now.

  • Claude: a message sent while Claude is starting is still accepted and shown at once. It is now written to Claude as soon as Claude answers its initialize handshake, instead of being pushed into Claude's input queue during it. Claude's optional settings read (effort, Fast) no longer delays that: it runs right after, and what it reports updates the chat's options a moment later. Measured against the real Claude CLI on this machine: process spawned and published 42 ms after the acquire began, started 433 ms later. If Claude dies or is stopped first, the message reads as not sent (Stop withdraws it; closing the chat keeps it as a waiting card), never as "delivery unconfirmed".
  • Every agent (Claude, Codex, Grok, OpenCode, Pi): once the agent's process exists, Orca stops the start if the agent produces no output at all (no stdout frame, no stderr line) for 60 seconds, or if it still has not finished starting 10 minutes after its process was spawned. Anything waiting on it is rejected with the existing "{agent} never finished starting, so Orca stopped it." line. Time spent before the process exists (finding the binary, the account) does not count. Codex, Grok, OpenCode and Pi otherwise look the same as before: they still finish their handshake before the chat is published (the follow-up changes that), but a handshake that hangs after the agent's process has started now ends instead of waiting forever. Finding the binary and the account before the process exists still has no limit.
  • An agent that keeps printing but never becomes ready holds your messages for up to 10 minutes while the chat shows "starting". Stop works during that time and withdraws them.
  • Model and effort picks stick. A pick made just after the agent started can no longer be undone by the agent's own report of its settings that was read before the pick landed (the chat would have said one model while the agent ran another, and the next reopen would have launched the old one).

The mechanism.

  1. One host-owned startup attempt (structured-agent-session-startup-attempt.ts). Minted by the host before any adapter is called, it carries the conversation identity, lease fence, spawn token, a host-generated attempt id, the record's pinned execution location and account, the saved options as intent, the event sink, a slot for attempt-scoped background-work reporting (unused until the running-tasks work), the cancellation signal, and an onOutput callback. The adapter contract's acquire input is this attempt, so the host always passes the whole thing.
  2. The startup clock. The host's attempt tracker starts the clock when the adapter reports the spawn (onSpawned), the first output, or the publication of a starting child, whichever comes first. Claude's clock starts at its first output or its publication (about 42 ms after the spawn), so its recorded start time slightly understates. Output reaches it through the one place every agent's process is spawned (spawnManagedProviderProcess): stderr chunks are already drained there, and stdout is observed only after the agent's own reader subscribes, so no frame is lost or reordered. Codex, the ACP agents (Grok, OpenCode), Pi and Claude each pass the attempt's onOutput to that spawn. The timer fires at whichever comes first of "last output + 60 s" and "spawn + 10 min" and re-arms when output arrived meanwhile, so a chatty agent costs no timer churn. Both limits are host constants, the same for every agent. Each start's outcome (ready, silent, ceiling, ended) and its spawn-to-outcome time is recorded as an agentSession.startup trace span with the agent id, so the limits can be tuned from real data. A start whose acquire the limit aborted is recorded once that acquire settles, so one that still comes back ready is recorded as ready, not silent.
  3. Hold-and-deliver barrier in the shared delivery loop. Sends are still journaled at once. The loop hands nothing to a child whose phase is starting: no dispatch pending row is written and the adapter is not called. The host's single started settlement (structured-agent-session-provider-started.ts) flips the child to ready, ends the attempt and wakes the loop, which re-checks the child's generation and fence, a close and a Stop before handing over. Conversation commands (/compact) go through the same loop and barrier. Nothing awaits startup inside a session's serialized step, so Stop, close and later sends never queue behind a handshake.
  4. Nothing optional ahead of the first handover, and no report undoes a pick. The started step only marks the child ready and wakes delivery. Persisting the options the agent reported runs afterwards, in its own serialized step, once the delivery loop has handed over what it was holding; a failure is logged, never thrown. Claude's later settings report (a new options-reported adapter event) is persisted the same way. Every report goes through one check (structured-agent-session-option-revisions.ts): the host keeps an option revision per chat, moved by every pick a running child takes (when the write returns, whatever its outcome), and by Claude's "that saved model does not exist" heal. The startup attempt, which the host cannot mint without it, hands the adapter a reader for that revision, and each report carries the value read when its read began. A report is accepted only if nothing moved the revision since its read began; reports are ordered among themselves by when the host takes them, and a report is persisted only if the revision is unchanged and no later report was accepted since. So a settings read that was out while you picked a model, or the start's report landing after a newer settings report, is dropped instead of writing the older value back. Because accepting a report does not move the revision, the settings read's stamp holds however late the host takes started; it does not depend on that event being handled synchronously.
  5. Recovery, one rule for every agent. If the limit passes while the adapter's acquire is still running, the host aborts it with a startup-limit reason (told apart from a user's Stop/close/quit), and the queued messages fail as "never finished starting". If it passes while a published child is still starting, the host stops that child exactly as the idle sweep already does (host-stop). The option read the host does for an agent that comes back already ready (Codex, Grok, OpenCode, Pi today) is still inside the start, so the limit, a Stop or a close now ends it too instead of waiting on an agent that never answers. The attempt's entry and timer are removed whenever its child ends (exit, crash, Stop, close), when the start is ready, abandoned or replaced, and once an expiry has stopped a published child. A late started from a stale child (other generation or fence) is ignored.
  6. No more guessing from the phase. dispatchSafely and the command handover no longer turn "the adapter threw while the child was starting" into "definitely not sent". Only a ready child is ever handed anything, so a throw after handover is unknown and never replayed; messages that never crossed the handover are settled as unsent by the start-failure, Stop or close path.

Supporting refactors to stay under file-length limits: the provider-history read moved to structured-agent-session-provider-history-window.ts, the "how did the child end" failure table moved to structured-agent-session-ended-child-failure.ts, and Claude's init-proof latch (createClaudeInitProof) moved into claude-structured-init-proof.ts (all verbatim).

Why

The goal is the same lifecycle rules for every agent with as little per-agent logic as possible. Holding messages in the host's existing delivery loop reuses the queue, journal and Stop/close/quit settlement that already exist, so "what happens to a message sent while starting" is decided once. Alternatives considered:

  • Let each adapter buffer during startup (Claude's previous behaviour). That is the per-agent divergence being removed, and it leaves the host unable to say whether a message was written when a start fails.
  • Await readiness inside the session's serialized step. Simpler, but Stop, close and the next send would wait behind a handshake that may never answer.
  • A fixed deadline from when the start was requested. Simple, but it killed legitimately slow starts (an OpenCode post-upgrade migration, a long Grok history replay) mid-way, and a resend killed them again. Measuring silence from the spawn ends a wedged start just as fast while leaving one that is visibly working alone; the 10-minute ceiling keeps "a start is still owed" from living forever.
  • Per-agent limits. Rejected for now so every agent follows one rule; the recorded spawn-to-ready times are there to revisit the two constants with data.

Differences from the common pattern

  • Orca starts the agent when the chat opens, not on the first message — intended: an earlier product decision, now with the same rules for every agent. Cost: an agent may start for a chat nobody types into.
  • Messages are held on the execution host — intended: the execution host owns everything that touches execution (local, SSH, WSL alike), so the hold and the startup clock live with it.
  • Codex, Grok, OpenCode and Pi still finish their handshake inside acquire — temporary: the bridge in AgentSessionAcquisition.providerChildPhase (absent means ready) is removed in PR 2 ("Open chats before agents finish starting"), which moves those adapters to publish at spawn and report readiness through the same started settlement. Their optional reads inside that handshake and the host's option restore after it still run before the chat is published until then; the startup limit, Stop and close now end that restore.
  • REAL_ADAPTER_PUBLISHES_AT_SPAWN in the registration test — temporary: it lists the one adapter (Claude) that already publishes at spawn and is removed in PR 2, when every registration does.
  • The registration matrix drives a scripted fake adapter per registration — temporary: it proves the host treats every registered agent the same way (the attempt, the barrier, the clock), not that each real adapter does. PR 2 switches it to each registration's real adapter with a scripted transport.
  • Claude messages wait for its initialize answer — intended: acceptance stays immediate; delivery waits for proof the CLI is ready (about half a second measured), which is what makes Stop and failure settle as "not sent" with certainty.
  • Attempt-scoped background-work sink is defined but not wired — temporary: the running-tasks PRs (4/5) route child-work evidence through it.

Linked Issue

None (internal: agent lifecycle unification, PR 1 of the plan).

Visual Proof

No rendered UI changes. The user-visible differences are timing (a Claude message sent during startup is written after Claude's initialize answer instead of during it) and that a start that stops producing output is now ended with an existing chat line. The proof is therefore test output, captured at head eea7a2ba083.

Real Claude CLI (the repo's real-CLI tests, 13/13). A chat started with nothing sent reports started, then answers a send handed over after it. Measured on this run: acquire to published 3 ms, published to started 5,414 ms, dispatch to first output 182 ms.

Real Claude CLI run, 13 of 13 passing, with the startup timing line

Host startup attempt and hold-and-deliver tests (32/32). The header maps each claim to the test that proves it: a send is held until the start proves itself, the 60 s silence limit, the 10-minute ceiling, Stop during start, and a late report never undoing a pick.

Host startup attempt tests, 32 of 32 passing, with a claim-to-test map

Testing

Automated, run locally on macOS, by explicit file name with --maxWorkers=2. pnpm test under the repo's Bun runtime cannot load node:sqlite on this machine (the local Bun is older than the pin), so the host suites ran under Node via pnpm run test:node <files>.

  • structured-agent-session-startup-attempt.test.ts (fake clock): the clock does not run before the spawn; a silent agent fails at exactly 60 s; a chatty-but-slow agent survives three times the silence limit and is measured when ready; silence is timed from the last output; a chatty agent that never becomes ready fails at exactly 10 minutes; output or publication starts the clock when no spawn was reported; no entry or timer remains after readiness, child end, expiry of a published child, abandon of an aborted acquire, a late publication after expiry, replacement, or quit; each start is measured once.
  • structured-agent-session-startup-attempt-host.test.ts (real host, store and journal, scripted adapter): send during starting is journaled but not handed over until started; held messages and a repeated client message id are handed over once, in order; stale readiness is ignored; crash, Stop and close during start settle held messages as never sent without waiting on the handshake and leave no attempt behind; a silent published start and a silent in-flight acquire are stopped; a chatty start outlives the silence limit and is stopped at the ceiling; reported options are persisted only after the held message's dispatch, and never over a pick made in between; a settings report whose read began before a pick is dropped even when it queues behind that pick (fails without the fix); the start's report never lands after a newer settings report; a settings read stamped before the host took started is kept (fails without the fix); a report read after a pick that failed is persisted; the minted attempt's revision reader answers the host's current revision; a ready agent's option read that never answers is ended by the limit and a close ends it too (fails without the fix). Also a start that turns ready as the limit fires is recorded as ready.
  • managed-provider-process-output.test.ts: stdout and stderr chunks count as output; a stdout reader that subscribes late still receives every frame.
  • claude-structured-session-startup.test.ts: started is reported on the initialize answer while get_settings is still unanswered; the settings report (options-reported) and auth diagnostic follow when it answers. Existing Claude tests that read settings-derived options from started now read them from options-reported.
  • structured-agent-session-startup-attempt-registrations.test.ts iterates the runtime's real registration list (pi, codex, claude, grok, opencode) through a scripted fake per registration (see Differences).
  • claude-structured-retired-saved-model.test.ts: a settings report that lands after Claude says the saved model does not exist no longer writes it back. This test had been failing on this branch since started moved to the initialize answer; it now follows that order.
  • Neighbours, all passing: the native-chat host suites touched by this PR, the delivery-loop/host/provider-started/child-exit suites, every src/main/pi test, the ACP/Codex acquire and connection tests, the JSON-RPC connection tests, the managed-process tests, and the Claude adapter, startup, options and runtime tests (two overlapping runs: 59 files with 571 passed, 1 skipped and 4 todo; and 58 files with 642 passed; no failures).
  • Real Claude CLI, run once as AGENTS.md requires: ORCA_REAL_CLAUDE_CLI_TEST=1 pnpm test src/main/claude/claude-structured-real-cli.test.ts src/main/claude/claude-structured-real-cli-fold.test.ts → 13/13 passed, including a new case that acquires, waits for started with nothing sent, then sends. Vitest hid that case's timing line on a pass, so the one new case was run once more with console output on to read it: acquire→published 42 ms, published→started 433 ms, send→first CLI frame 23 ms.
  • pnpm tc:node clean; orca-ci-checks 22/23 on the first run, the one failure (a test-support file over the line limit) fixed and oxlint plus tc:node re-run clean.
  • Review round 3 (option revision, restore limit, outcome recording): 82 touched and neighbouring test files under Node, 990 passed and 4 todo; the real Claude CLI test once more, 13/13; pnpm tc:node clean; orca-ci-checks 22/22; the mobile tests typecheck ratchet passes. The startup-attempt host and registration suites are now on the repo's Node SQLite runtime list, so pnpm test runs them under Node.
  • Final round (required revision, report order, merge of main at 8ae5264): pnpm tc:node and pnpm tc:web clean; the mobile tests typecheck ratchet passes; orca-ci-checks 22/23, and the 23rd (the native import-cycle check) failed only because a local mobile install had run first; run alone without it, as CI runs it, it passes; the 30 test files this PR changes, by name, pass under Node (five need node:sqlite, which the local bun runner cannot load; CI runs them under Node); the real Claude CLI test once, 13/13, timing acquire->published 3 ms, published->started 5414 ms, dispatch->first output 182 ms.

Not verified: Codex, Grok, OpenCode and Pi real CLIs (the new clock was not exercised against a real slow migration or history replay), Electron UI, Linux/Windows, SSH and WSL hosts.

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

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

  • SSH / remote and mixed versions: no wire change. The hold, the attempt and the startup clock all run on the execution host, where the agent's process and its output are; hostExecutionPhase keeps its meaning, and older clients see the same starting→ready status. options-reported is a host-internal adapter event, not a wire message.
  • Folder workspaces, WSL, Windows: nothing here is workspace-type or platform specific; the clock uses real elapsed time and an unref'd timer, and output is observed in the shared spawn used on every platform.
  • Contact loss is untouched: an unreachable host is still unverifiable, never exited.
  • Performance: one timer per start in flight, re-armed at most once per silence window; output only records a timestamp.

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)

…rrier

The host now mints one startup attempt before any adapter acquires (lease
identity, spawn token, pinned launch context, cancellation, a 60 s deadline
from the mint that activity never renews, option intent, and the event and
child-work sinks), and the shared delivery loop hands nothing to a child
still proving its start. A send is still accepted and journaled at once;
readiness (the normalized `started` settlement) wakes delivery, which
re-checks the child, fence, close and Stop before handover. A start that
fails, crashes, or misses its deadline leaves held messages definitely
unsent; a dispatch that throws after handover is `unknown`, never inferred
unsent from the child's phase.
…se Claude on initialize

Review round 1 on the host-owned startup attempt.

- The startup clock now starts when the process exists and fails a start
  only after 60 s with no stdout frame or stderr line, or 10 minutes after
  spawn however chatty it is. Every agent's child reports output through
  the one managed-process spawn, so a slow migration or history replay is
  no longer killed mid-way. Each start's spawn-to-outcome time is recorded
  as an `agentSession.startup` span for tuning.
- Claude reports `started` on its initialize answer; the settings read
  follows and is applied and persisted as a later `options-reported`
  update, so messages no longer wait on it.
- A started child's reported options are persisted in their own step after
  the held messages are handed over, and only if no pick changed the
  record's options since.
- An attempt now ends whenever its child ends or an expiry stopped it.
- Removed `optionsAsOf`, which was not an option revision, and comments
  that described the old publish-and-write-at-once behaviour.
…d process code

The adapter contract imported the attempt type from the module that also runs
its clock and imports the host types, which pulled the model catalog and
process-scan code into the mobile test typecheck. The attempt's shape and limits
now live in a dependency-free contract module.
A report of the child's options is now persisted only if no pick, no
model-not-found heal and no newer report reached the chat since its read
began. The host keeps one option revision per chat; the startup attempt
hands the adapter a reader for it, and each report carries the revision
it was read at. This replaces the record-snapshot check `started` had and
covers the settings report, which had none, so a settings read answered
just after a pick can no longer write the older model back.

Also: the option read a ready child does inside its start now ends with
the start (limit, Stop or close); a start that turns ready as the limit
fires is recorded as ready; the startup-attempt host tests run under the
Node SQLite runtime; and the retired-model test follows the new startup
order.
…rival

The host can no longer mint a startup attempt without the conversation's
option revision, so no host path can stamp a report at a stale zero and
drop every report after the first pick. Adapter tests may still omit it.

A report is now dropped only when a pick or a value the child showed it
cannot run came after its read. Reports are ordered among themselves by
when the host takes them, and only the newest admitted one is persisted.
The settings read's stamp therefore holds however late the host takes
the start's report, instead of relying on that report being handled
synchronously before the read begins.

Tests: a settings read stamped before the host took the start's report is
kept; a report read after a failed pick is persisted; the minted attempt's
revision reader answers the host's current revision.
Resolve against #25666 (signed-out / missing-CLI notice): a proven start still
tells the model catalog to re-check its held reason.

- Attach: an acquire mints the startup attempt, so the published block notes a
  start the acquire already proved (phase ready); a re-attach notes nothing.
- An accepted `started` readies the attempt, wakes delivery, then notes it. A
  stale child's `started` notes nothing.
- Main's persistStartedOptions is dropped: persistReportedOptions writes the
  same fields after delivery, only while the report is the newest word.
- endReleasedChild moves beside the post-acquire failure it serves, keeping the
  attach file within its line limit.
@brennanb2025

brennanb2025 commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor Author

Review summary (coordinator)

State: head b0708115271 (includes current main with #25666 and #26336). CI on this head: 23 passed, 17 skipped by path, 0 failed. Merge state is clean. The PR is still a draft, waiting for the owner's merge decision.

What was reviewed

Three independent review rounds (plus one CI fix), each by a fresh reviewer reading the diff against the goal: every agent's chat can be typed into right away, and a message sent while the agent starts is handed over exactly once, after the agent proves it is ready.

Round Found Fixed in
1 No hand-off bug. The single startup timeout was too blunt: a slow but chatty start was killed, and a silent one waited too long. Claude reported ready only after its settings read. Options were saved before the held message was handed over. Some startup records were never cleared. 60 s silence limit plus a 10-minute ceiling counted from spawn. Claude reports ready on its initialize answer. Options are saved after hand-off. Startup records are cleared on every ending.
2 (CI) The phone app's test typecheck pulled in desktop-only startup code. The startup types moved into a dependency-free contract file.
2 A settings report read before the user picked a model could overwrite that pick. Each chat carries an option revision, and a report older than the newest pick is dropped (tests fail without the fix).
3 No correctness bug. Two hardening items: the option revision was optional (forgetting it would silently drop reports), and the settings read relied on synchronous hand-off order. The revision is now required. The settings read takes its stamp from the accepted start. Added a test that a report read after a failed pick is kept.
Main merge (#25666) Main added a "provider started" hook: a proven start makes the model list re-check "signed out / not installed" sooner. Its call sites conflicted with this PR's startup path. The hook is called when a start is accepted (never for a stale child) and when an attach publishes an already-ready child. Main's separate option save was dropped because this PR's save writes the same fields with stricter checks. New test: one call per accepted start, none for a stale child.

Verified

  • Real Claude CLI tests, 13/13 (screenshot in the description). Timing: chat published 3 ms after acquire; Claude ready 5.4 s later; first output 182 ms after the held message was handed over.
  • Host startup tests, 32/32, with a header mapping each claim to its test (screenshot in the description).
  • Locally: desktop typechecks (node and web), the phone tests typecheck, the touched test files plus Native chat: say when Codex is signed out or the agent isn't installed, without disabling Send #25666's tests by name, and the repo's CI static checks. The one local failure is a phone import-cycle audit on files this PR does not touch; it passes in CI.

Not verified

Process note

While capturing the proof, the CLI version was recorded by running claude --version by bare name instead of by absolute path. It is read-only and touched no account state, but it broke our own rule for test runs, so it is noted here.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 8, 2026 18:05
@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

📝 Walkthrough

Walkthrough

The change adds startup-attempt tracking and expiry for structured agent sessions. Messages remain queued until a matching provider child reports readiness. Claude now reports initialization before settings-derived options, which are delivered through a later event. Provider output callbacks feed startup progress across several connection paths, and startup duration is recorded in instrumentation.

Priority: ➖ Normal

Merge Risk: 🔵 Low · up to b0708

Messages are now held until the agent finishes starting. Two small follow-ups remain. A rare error while reporting Claude's settings after startup could surface as a startup failure. One timing-based test may fail intermittently on slow CI machines. Neither should block merging.

🚥 Pre-merge checks | ✅ 4 | ❓ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ❓ Inconclusive Docstring coverage is 40.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 47 functions across 50 files. (26 skipped… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: holding messages until the agent finishes starting.
Description check ✅ Passed The description is detailed and covers the required explanation, changes, rationale, testing, visual proof, compatibility notes, and checklist. Some checklist items remain unchecked, but the testing l…
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 40.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 47 functions across 50 files. (26 skipped: 26 over the file limit.)

✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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: 2


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 579782c5-1ae0-4e1a-9ce7-78a41b358f05
📥 Commits

Reviewing files that changed from the base of the PR and between 496ee9b and b070811.

📒 Files selected for processing (78)
  • config/scripts/vitest-sqlite-runtime-files.mjs
  • src/main/acp/acp-agent-connection.ts
  • src/main/acp/acp-structured-acquire.ts
  • src/main/claude/claude-agent-sdk-process-spawn.ts
  • src/main/claude/claude-stream-json-connection.ts
  • src/main/claude/claude-structured-applied-options.test.ts
  • src/main/claude/claude-structured-init-proof.ts
  • src/main/claude/claude-structured-real-cli.test.ts
  • src/main/claude/claude-structured-session-acquisition.ts
  • src/main/claude/claude-structured-session-adapter.test.ts
  • src/main/claude/claude-structured-session-journal-control.ts
  • src/main/claude/claude-structured-session-startup-state.ts
  • src/main/claude/claude-structured-session-startup.test.ts
  • src/main/claude/claude-structured-session-startup.ts
  • src/main/claude/claude-structured-session-state.ts
  • src/main/codex/codex-app-server-connection-types.ts
  • src/main/codex/codex-app-server-connection.ts
  • src/main/codex/codex-structured-session-acquire.ts
  • src/main/jsonl-rpc/agent-connection.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-accept-then-deliver.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-acquire-aborts.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-acquisition-options.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-acquisition-runtime-stamp.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-adopted-import.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-agent-start.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-attach-failure.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.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-hung-start-stop.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-conversation-close.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-drivability.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-ended-child-failure.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-event-recovery.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-runtime-state.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-mutation-admission.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-mutation-context.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-mutation-turn-context.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-option-revisions.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-owed-work-release.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-pre-spawn-first-answer.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-processless-reservation.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-provider-child-record.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-provider-history-window.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-provider-started-catalog.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-send-restarts-failed-start.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-writer.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-contract.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-registrations.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts
  • 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.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-unrun-send-hold.test.ts
  • src/main/observability/agent-session-instrumentation.ts
  • src/main/pi/rpc-session.ts
  • src/main/provider-process/managed-provider-process-output.test.ts
  • src/main/provider-process/managed-provider-process.ts
  • src/main/runtime/claude-structured-retired-saved-model.test.ts
  • src/main/runtime/claude-structured-send-during-startup.test.ts
  • src/main/runtime/claude-structured-send-restart-dies-before-dispatch.test.ts
  • src/main/runtime/claude-structured-startup-fault-is-not-an-exit.test.ts
  • src/main/runtime/rpc/methods/structured-agent-session-at-rest.test.ts
  • src/main/runtime/structured-agent-session-failure-logging.test.ts
  • src/main/runtime/structured-claude-runtime-adapter.ts
💤 Files with no reviewable changes (2)
  • src/main/native-chat/agent-session-wire/structured-agent-session-mutation-admission.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-mutation-turn-context.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.

Comment on lines +276 to +301
const facts: ClaudeStartupFacts = {
...initialized,
init,
settings,
prepared: prepareClaudeStructuredSessionAcquisitionOptions({
settings,
initialization: initialized.initialization
})
}
input.diagnose(claudeAuthDiagnostic(facts.initialization, facts.init, settings))
const writtenSinceRead = sequence !== session.optionMutationSequence
applyClaudeSettingsFacts(session, facts, writtenSinceRead)
const startFastMode = writtenSinceRead ? null : admitClaudeStartFastMode(session, facts)
input.report({
type: 'options-reported',
reportedOptions: claudeStartedReportedOptions(session, readClaudeModels(facts.initialization)),
restoreSkippedOptions: [...session.restoreSkippedOptions],
...claudeRetiredOptions(session),
optionRevision
})
if (startFastMode !== null) {
void applyClaudeStartFastMode(session, facts, startFastMode, (event) => {
if (input.isCurrent()) {
input.report(event)
}
})

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.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
rg -nP -C3 'startup\.settled' --type=ts -g '!**/*.test.ts'
ast-grep outline src/main/claude/claude-structured-session-acquisition-options.ts --items all
rg -nP -C8 'export function prepareClaudeStructuredSessionAcquisitionOptions|export function admitClaudeStartFastMode' --type=ts

Repository: stablyai/orca

Length of output: 8804


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- startup implementation ---'
nl -ba src/main/claude/claude-structured-session-startup.ts | sed -n '1,360p'
printf '%s\n' '--- acquisition startup race ---'
nl -ba src/main/claude/claude-structured-session-acquisition.ts | sed -n '220,310p'
printf '%s\n' '--- option helpers ---'
nl -ba src/main/claude/claude-structured-session-acquisition-options.ts | sed -n '1,120p'
nl -ba src/main/claude/claude-structured-start-fast-mode.ts | sed -n '1,180p'
printf '%s\n' '--- diagnostics and report consumers ---'
rg -n -F -- 'claudeAuthDiagnostic' src/main/claude
rg -n -F -- 'type: '\''options-reported'\''' src/main/claude
rg -n -F -- 'settleClaudeSessionStartup(' src/main/claude

Repository: stablyai/orca

Length of output: 24715


🏁 Script executed:

nl -ba src/main/claude/claude-structured-session-startup.ts | sed -n '230,330p'
nl -ba src/main/claude/claude-structured-session-acquisition.ts | sed -n '245,285p'
nl -ba src/main/claude/claude-structured-session-acquisition-options.ts | sed -n '1,40p'
nl -ba src/main/claude/claude-structured-start-fast-mode.ts | sed -n '1,90p'
rg -n -C4 -F -- 'claudeAuthDiagnostic' src/main/claude
rg -n -C4 -F -- 'type: '\''options-reported'\''' src/main/claude

Repository: stablyai/orca

Length of output: 16601


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- settings and diagnostic implementations ---'
nl -ba src/main/claude/claude-structured-session-options.ts | sed -n '1,280p'
nl -ba src/main/claude/claude-structured-init-proof.ts | sed -n '1,180p'
printf '%s\n' '--- direct helper implementations ---'
rg -n -C8 -F -- 'function observeClaudeSettingsApplied' src/main/claude
rg -n -C8 -F -- 'function claudeRetiredOptions' src/main/claude
rg -n -C8 -F -- 'function claudeStartedReportedOptions' src/main/claude/claude-structured-session-startup.ts
printf '%s\n' '--- report/event binding ---'
rg -n -C8 -F -- 'const emit' src/main/claude/claude-structured-session-acquisition.ts
rg -n -C8 -F -- 'function emit' src/main/claude

Repository: stablyai/orca

Length of output: 27061


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- exact option readers and settings application ---'
rg -n -C10 -F -- 'function readClaudeSettingsFastMode' src/main/claude
rg -n -C10 -F -- 'function readClaudeSettingsFastModePerSessionOptIn' src/main/claude
rg -n -C10 -F -- 'function readClaudeFastModeFacts' src/main/claude
rg -n -C12 -F -- 'function readClaudeSettingsEffort' src/main/claude
rg -n -C12 -F -- 'function observeClaudeSettingsApplied' src/main/claude
printf '%s\n' '--- diagnostic and retired options ---'
nl -ba src/main/claude/claude-structured-init-proof.ts | sed -n '80,145p'
rg -n -C12 -F -- 'function claudeRetiredOptions' src/main/claude
printf '%s\n' '--- bound event emitter ---'
rg -n -C12 -F -- 'emit =|function emit|const emit' src/main/claude/claude-structured-session-acquisition.ts src/main/claude/claude-structured-session-*.ts

Repository: stablyai/orca

Length of output: 16998


🏁 Script executed:

set -e
rg -n -C12 -F -- 'deliver:' src/main/claude src/main/native-chat
rg -n -C12 -F -- 'deliver(' src/main/claude/claude-structured-session-acquisition.ts
rg -n -C12 -F -- 'emit:' src/main/claude src/main/native-chat
rg -n -C12 -F -- 'callbacks.emit' src/main/claude

Repository: stablyai/orca

Length of output: 45663


🏁 Script executed:

set -e
nl -ba src/main/claude/claude-structured-session-adapter.ts | sed -n '1,180p'
rg -n -C12 -F -- 'deliver(attempt' src/main/claude
rg -n -C12 -F -- 'private deliver' src/main/claude
rg -n -C12 -F -- 'private emit' src/main/claude

Repository: stablyai/orca

Length of output: 21855


Guard the post-started settings step.

The settings readers handle malformed unknown values safely. However, input.report reaches ClaudeStructuredSessionAdapter.deliver, which invokes the event callback without a catch. Its emit path also delegates without a catch. A synchronous event-delivery exception can reject settleClaudeSessionStartup, which is part of the session.startup.settled race.

Suggested fix
-  const facts: ClaudeStartupFacts = {
+  try {
+  const facts: ClaudeStartupFacts = {
     ...initialized,
 ...
   if (startFastMode !== null) {
     void applyClaudeStartFastMode(session, facts, startFastMode, (event) => {
       if (input.isCurrent()) {
         input.report(event)
       }
     })
   }
+  } catch {
+    // The optional settings report must not fail an already-started session.
+  }

Comment on lines +362 to +382
it('keeps a slow start that is still talking, and stops one that never readies at the ceiling', async () => {
startupLimits = { silenceMs: 50, ceilingMs: 400 }
const chatter: ReturnType<typeof setInterval>[] = []
await restartWith(async (input) => {
await input.onSpawned?.(acquisition(input).process)
chatter.push(setInterval(() => input.onOutput?.(), 10))
return { ...acquisition(input), providerChildPhase: 'starting' }
})
try {
const id = await heldBehindStart('hello')
await new Promise((resolve) => setTimeout(resolve, 200))
expect(closeSession).not.toHaveBeenCalled()
expect(startupAttemptOpen()).toBe(true)

await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected'))
expect(await submission(id)).toMatchObject(HOST_STOPPED)
expect(dispatch).not.toHaveBeenCalled()
} finally {
chatter.forEach(clearInterval)
}
})

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.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Widen the ceiling margin: this test can fail on a slow CI machine.

The clock starts at onSpawned inside the acquire. heldBehindStart then waits for the send, the publish and the starting status projection. After that, the test sleeps 200 ms with real timers. The ceiling is 400 ms from spawn. If the setup before the sleep takes more than about 200 ms on a loaded runner, the ceiling fires first. Then expect(closeSession).not.toHaveBeenCalled() fails.

Set the ceiling well above the sleep plus setup. The eventually wait (10 s) still covers the later ceiling stop.

Proposed fix
-    startupLimits = { silenceMs: 50, ceilingMs: 400 }
+    startupLimits = { silenceMs: 50, ceilingMs: 1_500 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('keeps a slow start that is still talking, and stops one that never readies at the ceiling', async () => {
startupLimits = { silenceMs: 50, ceilingMs: 400 }
const chatter: ReturnType<typeof setInterval>[] = []
await restartWith(async (input) => {
await input.onSpawned?.(acquisition(input).process)
chatter.push(setInterval(() => input.onOutput?.(), 10))
return { ...acquisition(input), providerChildPhase: 'starting' }
})
try {
const id = await heldBehindStart('hello')
await new Promise((resolve) => setTimeout(resolve, 200))
expect(closeSession).not.toHaveBeenCalled()
expect(startupAttemptOpen()).toBe(true)
await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected'))
expect(await submission(id)).toMatchObject(HOST_STOPPED)
expect(dispatch).not.toHaveBeenCalled()
} finally {
chatter.forEach(clearInterval)
}
})
it('keeps a slow start that is still talking, and stops one that never readies at the ceiling', async () => {
startupLimits = { silenceMs: 50, ceilingMs: 1_500 }
const chatter: ReturnType<typeof setInterval>[] = []
await restartWith(async (input) => {
await input.onSpawned?.(acquisition(input).process)
chatter.push(setInterval(() => input.onOutput?.(), 10))
return { ...acquisition(input), providerChildPhase: 'starting' }
})
try {
const id = await heldBehindStart('hello')
await new Promise((resolve) => setTimeout(resolve, 200))
expect(closeSession).not.toHaveBeenCalled()
expect(startupAttemptOpen()).toBe(true)
await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected'))
expect(await submission(id)).toMatchObject(HOST_STOPPED)
expect(dispatch).not.toHaveBeenCalled()
} finally {
chatter.forEach(clearInterval)
}
})

@brennanb2025
brennanb2025 merged commit 04ac6d6 into main Oct 8, 2026
64 checks passed
brennanb2025 added a commit that referenced this pull request Oct 8, 2026
#26416 pushed structured-agent-session-host.ts one line over the 300-line limit,
so main's oxlint is red and merges from main fail the commit hook. Move the
pure knownAgentIds logic into structured-agent-session-provider-support.ts
beside the other agent-support checks.
brennanb2025 added a commit that referenced this pull request Oct 9, 2026
…ain)

Rebuilds #26561 on main now that #26416 is squash-merged. Same change as
before, re-applied onto what main has since:

- Every adapter answers acquire at spawn and reports `started` on protocol
  readiness with its real handle (Claude initialize, Codex thread, ACP
  session new/load plus restore, Pi session file plus restore). No bridge,
  no per-adapter phase or Stop opt-outs.
- Picks made while starting are intents applied model then effort before
  the start is accepted; rewind, goal, rewind recovery and compact wait for
  a proven start outside the session queue.
- One Stop rule: Stop ends the session for every agent (Codex: interrupt,
  up to 2 s for the turn to end, then close).
- Orchestration worker starts share one deadline; a send's answer copies
  its row; a re-attach during start keeps the start wait.

Integrated with main:
- ACP: the subagent-stop probe, child-work delivery and new/loaded origin
  move into the handshake; the sign-in rule (#26544) classifies a failed
  handshake; acpAgentName stays in acp-structured-agent-definitions.
- Codex: the session journal translator and account kind (#26407) reach the
  start step.
- Pi: the start choice (#26821) and the signed-out verdict (#26743) are read
  in Pi's start, so both exist when `started` is reported; commands follow.
- openWithAgent keeps main's pre-agent check and replay rule.
- A report with no model drops the saved one, as #26587 decided.
- The renderer re-reads a start's sign-in verdict once the start is proven.
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