Repository navigation
Wait for agent readiness without blocking native chat - #27925
Merged
brennanb2025 merged 50 commits intoOct 16, 2026
Merged
Conversation
This was referenced Oct 12, 2026
brennanb2025
force-pushed
the
brennanb2025/agent-start-split-1-host-readiness
branch
from
October 12, 2026 21:31
6b535a2 to
88c4c5d
Compare
# Conflicts: # 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-host-mutations.ts # src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts # src/main/runtime/structured-agent-session-runtime.ts # src/renderer/src/components/native-chat/use-structured-agent-session-option-state.ts
brennanb2025
marked this pull request as ready for review
October 13, 2026 02:01
Contributor
|
Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 32.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 56 functions across 50 files. (51 skipped… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title clearly summarizes the primary change: waiting for agent readiness without blocking native chat input and work. Description check The description is complete and directly addresses the template sections, including user impact, implementation, rationale, linked work, visual proof limitations, testing, review status, compatibility…
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 32.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 56 functions across 50 files. (51 skipped: 51 over the file limit.)
✨ Finishing Touches 💡 1
-
📝 Generate docstrings 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
brennanb2025
marked this pull request as draft
October 13, 2026 02:11
brennanb2025
changed the title
Contributor
Author
|
Review loop: done on What this PR fixes. Two problems in how a native chat starts. (1) Retrying a goal after an unsuccessful attempt could be refused before it reached a ready agent, with an "outcome unknown" style error. It now follows the existing recorded-operation policy, so the retry runs once the agent is ready. (2) When the chat's launch bookkeeping was retired, the saved or accepted model and effort could be discarded, so the first message used a different choice. The latest choice is now kept through that step, with accepted-session and pending-pick precedence unchanged. 127 production / 147 test changed lines; no new saved field or wire operation. Review. Six earlier rounds, then a reduced repair at CI. Full PR workflow green on this head (18 passed, the rest skipped by path filters). Rendered QA (M4Air, macOS, real signed-in Claude and Codex, isolated test copies, Codex as driver). Whole-window screenshots, PR head and current main:
Slow-host check (simulated). The host's "chat created" reply was held for 8 seconds in the app's main process (no product code changed; real host and real Claude). PR and main behave the same: an effort picked during that window is reset at the first send (Opus High went out as PR before / after: Differences from the common pattern, labelled.
Earlier QA attempts on Windows high spec did not run their cases (packaged PowerShell can't start under a throwaway test profile, then the machine stayed below the memory gate). A real Codex settings change seen there was Codex adding a trust entry for the QA folder. |
# Conflicts: # src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.test.ts
brennanb2025
marked this pull request as ready for review
October 16, 2026 17:52
brennanb2025
added a commit
that referenced
this pull request
Oct 17, 2026
Merged with df2a984 as the base (the branch's fork point from #27925's pre-squash history), so only main's own changes since then are merged. Message-carried choices keep main's shipped names: a send's options record, agent-session.send-options.v1, the saved submission options and the queued options_json column. One host parse at admission turns the record into the branch's typed choices; they still apply when the agent is handed the message, and Codex still waits for its running turn. Main's apply-at-handover is removed, and a pick between messages only saves.
This was referenced Oct 19, 2026
brennanb2025
added a commit
that referenced
this pull request
Oct 21, 2026
…nd (#30645) * refactor(native-chat): remove three chat mechanisms nothing drives - The adapter's optional startAnswered hook lost its only implementer in #27925; it was still forwarded through four layers and always read as answered. A start that never answered is read from the child's phase, as the exit already does. - The saved lease's journalCheckpoint is only ever written as null and read only by validation. Records an older build saved with it still load: it joins the retired lease fields dropped at load, so the next write sheds it. - The record store's setConversationCommand had no production caller. The tests that stood in for an older build with it now seed the saved row and reopen the store; three that only re-pinned another test are removed. * test(native-chat): an older build's null journal checkpoint also loads * Keep startAnswered cleanup in its owning PR * Keep historical chat fixtures aligned with their readers
This was referenced Oct 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI5
A chat could hold up later messages while a goal or rewind waited for the agent to finish starting. The new readiness path also mishandled a retried goal, and retiring launch bookkeeping could lose the model and effort chosen for the chat before its first message.
The host separates starting an agent from waiting for it to become ready. Messages can be recorded while an earlier goal or rewind waits for readiness, subject to the existing opening wait described below. A retried goal uses its recorded answer when one exists and otherwise waits for a ready agent before running. The chat keeps its chosen model and effort after launch bookkeeping retires, so its first message carries what the picker shows.
What Changed
The execution host publishes the spawned process before awaiting provider initialization and recovered history. Claude uses the split opening path; concurrent attachments keep separate cancellation handles. Startup-dependent goals and rewinds reserve places alongside message delivery and release those places when they finish or their wait ends. Replacing history preserves accepted messages that have not reached the agent.
Goal retries reuse the existing recorded-operation policy, and admission reads the operation again after readiness so another completed attempt is answered without running twice. The mounted launch view keeps the latest accepted selection after the registry retires it; held picks and authoritative session options keep their existing precedence.
A model or effort pick on a running chat still writes through the existing setter. Startup picks wait for readiness, while Claude messages carry model and effort and apply them through that same setter. Provider reports remain live facts and do not overwrite saved user choices.
Why
The execution host already owns the process, conversation and delivery order. Keeping readiness there lets it hold provider-dependent work without holding the conversation queue for the whole wait. Reusing the existing replay policy avoids a second definition of whether an operation can run again. Keeping selection intent in the mounted chat prevents launch bookkeeping from deciding what the first message uses.
Differences from the common pattern
unverifiable, never proof of exit.Stopping a chat start when the app quits is an existing gap on main and is unchanged by this PR. Quit can wait on a held launch resolver, and that start can spawn after abort. It is tracked as a separate follow-up on
brennanb2025/chat-start-cancel-on-quit; the archived candidate is unvalidated and is excluded from this PR. Historical-delivery scan performance (C2) remains deferred pending measurement.Linked Issue
Replaces the host-readiness portion of #26561 and folds in #27627. Maintainer change; builds on #26416 and #26332.
Visual Proof
Final-head Windows rendered comparison is blocked: the isolated app could not detect its signed-in agents because shell-path discovery returned spawn EPERM, and the retry fell below the memory gate. The sole screenshot shows initial onboarding, not a passed requested flow; no main comparison ran. No new notice or banner is added.
Testing
On 76b1c74, 12 focused tests pass across the goal/readiness, repeat-operation and launch-picker/send suites. The same tests fail when the two production repairs are removed: the goal retry is refused, and saved/accepted launch selections are lost. The goal boundary also holds real startup readiness while a competing durable success arrives, then replays after eviction without acquiring another child. The renderer test uses the actual option and send owners, verifies the first request carries the shown choices, and checks authoritative accepted options and session reset. Earlier broader test results are prior evidence only. Linux, WSL and actual SSH UI validation are not claimed.
Review
The review scope is reduced to goal retry (C1) and lost launch selections (UF1). Quit-cancellation rework (A1) is excluded under the coordinator ruling. Final correctness and performance source reviews are complete. Hosted checks on this head failed lint in two files unchanged by this PR: the mobile chat-view test has 806 counted lines against an 800-line limit, and the Git binary compatibility test has 820. The latter has a newer fix on main, but this run used the earlier merge snapshot. Typecheck did not run after preflight failed, and the full PR aggregate failed because static analysis failed and downstream tests and packages were skipped. Remote rendered QA is blocked; this PR stays in draft and is not marked ready by this worker.
Agent skill upstream boundary
Notes
No new wire operation or saved field is introduced by these repairs. Folder workspaces use the same host path. Existing peers keep their supported behavior.
Checklist