You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Show one actionable error when an agent launch cannot prepare its workspace - #37220
Starting an agent from New Chat, a shortcut, Hand Off, Source Control or Fix Checks could show two error toasts for one failed start. The step that prepares the launch showed its own error and returned nothing; every caller then treated "nothing" as an unknown failure and showed a second, internal-sounding "Could not build launch command" message. "Nothing" also meant the user had cancelled, so callers were guessing. While a remote host was still connecting, clicking a terminal choice also produced an error telling the user to connect, although it was already connecting.
What changes for users
A failed start shows exactly one message, naming the requested or selected agent and saying what to do (connect to the SSH host, reconnect or update the workspace's Orca server, refresh projects, open a terminal and start the agent there, or fix an argument with an unclosed quote). Before an automatic agent choice is known, Start work keeps its existing operation recovery without guessing an agent name. Cancelling shows nothing.
While the workspace's host is still connecting or setting up, terminal choices stay visible but disabled with "Connecting…", in the New Chat menu, its setup panel, the agent picker and the agent combobox. Chat and Configure stay usable, and an agent whose preferred surface is terminal stays on terminal. Keyboard shortcuts and late clicks during that window do nothing instead of showing an error.
A chat that a host supports is no longer refused because terminal-only launch information is missing.
When a visible chat is refused by its host and the terminal fallback can't start either, the chat's own failure notice carries the advice instead of a second toast.
The Hand Off dialog stays open (with its request) if nothing actually started, and can always be closed while a start is in progress.
Fix Checks also reports failures before workspace creation once, including when the pull request cannot be read. Opening an existing fallback dialog or workspace view still works.
Connected SSH hosts missing launch information are tracked in STA-13357.
How it works
Launch preparation now only returns facts (the host's OS, or why it isn't known and whether that is still pending). The launch action is the single place that reports a failure: it shows one localized, named-agent message and returns the existing refused result with a notification receipt, which every caller now checks explicitly instead of inferring failure from an empty result. Chat routing runs before the terminal-only OS check. Fix Checks uses its existing per-attempt notification callback so the action and its fallback cannot repeat the same failure. The menus derive one "terminal still waiting" value from the host's live connection state and apply it where a terminal choice is shown or chosen; it is not stored and is kept out of the picker's identity keys, so it never resets drafts or inventory.
Tracked in STA-13205.
Review path: full. This changes launch failure handling and routing order and has a reviewed plan; it is outside the light path's local-effect and low-risk criteria. The change introduces no saved field or wire format.
Intended: terminal launches still require the execution host's reported launch information to preserve shell quoting; chat admission does not depend on it. Intended: only a host that is still connecting or setting up disables terminal choices; settled missing information (STA-13357) still reaches the one accurate refusal, per the coordinator's scope ruling. Intended: the readiness value is per menu rather than per agent row, and chat and terminal readiness are separate, because Orca offers both surfaces for one agent.
…onnects
Replace per-row pending metadata with one derived terminal-readiness flag on
the agent menu model, kept out of picker identity keys. Terminal buttons and
rows stay mounted and disabled while SSH or a paired server is still
connecting; chat and Configure stay usable, and the terminal default is not
changed. Repo targets with no workspace yet read the repo host's facts.
Invalid Source Control arguments keep their existing actionable message.
…ckers and fallbacks
A host that is still connecting now declines a terminal launch silently at the
launcher, including shortcuts and fallbacks, instead of reporting a failure.
A terminal fallback refused behind a visible chat reports through that chat's
notice rather than a second toast. The agent picker stays usable while only a
terminal waits; an armed terminal rechecks readiness on Enter; a configured
chat option no longer inherits the terminal's waiting state; waiting terminal
controls say why. Invalid CLI arguments name the actual problem for every
caller, the no-agent shortcut message is restored, and prospective repo
readiness no longer rescans repos on unrelated store writes.
…thing
A session continuation launched through the host's admission now waits for
that launch to settle before reporting the handoff. When the host declines the
chat and its terminal fallback is declined while the host is still connecting,
nothing opens, so the dialog stays open for a retry instead of closing with no
session and no message.
Cancel, Escape and outside clicks now close the Hand Off dialog even while a
start is in flight; Start stays disabled with its spinner. Closing aborts agent
detection if it is still running, and a dialog closed during detection starts
nothing. A launch already sent settles on its own and reports through its
existing owners. The in-flight start belongs to the request it was made for, so
a late finish never closes, completes or blocks a dialog reopened since.
It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.
No actionable comments were generated in the recent review.
Walkthrough
The changes derive terminal-pending state from execution-host facts and use it to gate terminal choices in agent menus while keeping available chat choices usable. Launch preparation failures now return explicit refusal outcomes with recovery guidance, and callers handle refused results. Structured terminal fallback results include refusal details. Continuation dialogs can close during a pending launch, and late or aborted completions do not change the current dialog.
Merge Risk: Low · previous review f60ce
Tests may become order-dependent because the Windows platform stub leaks into later tests. The change is otherwise mergeable with this test-isolation follow-up.
Pre-merge checks: 3 · 2
Failed checks: 2 warnings
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 43.08% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check
The description explains the problem, user impact, and mechanism. It omits required visual proof, testing details, and a risk assessment. It also lacks a linked issue and several template sections.
Use the template headings and add a plain-language ELI5. Explain why this approach is better than the alternatives. Link the issue if required for the contributor. Add before-and-after visual proof, reviewer test steps, tested platforms, an…
Passed checks: 3 passed
Check name
Status
Explanation
Title check
The title names the main change: when an agent launch cannot prepare its workspace, the user sees one actionable error instead of a second generic toast. The changeset matches this. Launch preparation…
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.
Full details: Title check
Explanation
The title names the main change: when an agent launch cannot prepare its workspace, the user sees one actionable error instead of a second generic toast. The changeset matches this. Launch preparation now returns facts, the launch action reports one localized message naming the agent, and callers check the explicit refused result. The title does not cover the terminal-pending readiness work, which the description covers as a secondary part.
Full details: Docstring Coverage
Explanation
Docstring coverage is 43.08% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 65 functions across 66 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Full details: Description check
Resolution
Use the template headings and add a plain-language ELI5. Explain why this approach is better than the alternatives. Link the issue if required for the contributor. Add before-and-after visual proof, reviewer test steps, tested platforms, and test command results. State the risk level, affected users and boundaries, and rollback plan. Complete the applicable disclosure, review, skill-boundary, and checklist sections.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.
The terminal readiness check rebuilt the target's ownership chain and ran
the full OS resolver on every store write, for every mounted picker. Each
picker now keeps its target host and re-resolves it only when an ownership
slice changes; other writes read just that host's connection and contact
facts. Local hosts read nothing. The launcher's own pending decline uses
the same host pieces from execution-host-facts, so the rule has one copy.
Wording the failed chat's notice after its terminal fallback was refused
re-saved every retained launch record, though the wording itself is never
saved. Listeners now hear about it without a save; lifecycle changes still
save. The refused-fallback test moves to its own file and counts saves at
two sizes.
launchAgentInNewTab now returns a refusal instead of a tab when the
launcher declines, so tests that read the opened tab's surface or
completion narrow it first. Fixes the web typecheck.
While the host connected, each New Chat agent row was marked
aria-disabled, so its Configure button read as disabled to assistive
tech and keyboard. The row now only blocks its launch and points to its
"Connecting…" reason. A row shows one reason, the one that stops its
launch now, and the agent name keeps its width so it is not cut to a
single letter.
…rf contract
Pickers for workspaces not created yet now count repo reads cold and after
another host's row is republished, at two scales with a repeated-target
control, and assert no unaffected renders. Two freshness controls follow
the cached host: an owner moving from local to a connecting SSH host and
back, and a paired server that is still checking versus verified with its
OS.
… stay usable
An agent row's launch item also held its Configure button (New Chat) or
its start-in-chat button (the + menu). Disabling the row while the host
connected disabled those too; leaving it enabled made the waiting launch
read as available and then do nothing.
The launch item and those controls are now siblings. While the host
connects the launch item is marked unavailable, dimmed and described
"Connecting…", and stays focusable so arrow keys still reach the
controls beside it. Configure and chat keep their own state. A chat
started from Configure or the chat button goes through the row's guarded
launch and closes the menu, without choosing the waiting item.
…ancelled
Each press on the button beside an agent row added a window listener that
only a pointerup removed, so cancelled touches and closed menus left them
behind. A press now owns one handler that removes both listeners on
release, cancel, the next press or the row closing; drag release onto the
button still chooses it.
The launch item beside Configure or the chat button restyled the shared
menu item (transparent focus, dimmed while unavailable), which the design
system gate rejects. That treatment is now the menu item's launch-row
variant; the rendered rows are unchanged.
…a checks
The menu's terminal readiness now reads workspace records through the
workspace record selectors and picks a not-yet-created workspace's repo by
the host it names, without the focused-host fallback; with no host named
it settles only when the id has one row, which reads the same. Its cached
host is re-checked when the settings slice changes instead of reading the
focus setting. E2E unit tests narrow refused launch outcomes, and the two
new ja strings write Agent as the rest of the catalog does.
… fallback
Terminal readiness re-resolved every mounted menu's host on any settings
write, and an unstamped worktree whose project is listed both here and by
a paired server walked the whole repo catalog to resolve. Readiness now
resolves an existing workspace's host from its resource owner only, never
the focused default a row-less setup terminal is created on, so settings
writes leave it alone. The shared-project check reads that project's rows
from the existing per-snapshot repo index instead of the whole catalog.
Setup terminals and every route keep their behavior.
The detected-twin perf contract now checks render deltas per update: a
catalog republication renders no picker, and stamping the first workspace
with a checking server, then that server reporting its OS, renders only
that workspace's pickers.
…ce host
The fixture replaces the whole worktree owner module with a stub, so the
menu readiness check's new resource-host export was missing and the
picker path threw before launching. The stub now answers both exports
with the scenario's owner host.
The reason will be displayed to describe this comment to others. Learn more.
Follow-up Comments
Mock remote detection in the performance fixture.
571-580: Major · Stability & Availability · Quick win
Mock remote detection in the performance fixture.
The SSH mount reaches the missing window.api.preflight.detectRemoteAgents bridge. The unhandled error fails the ordinary unit command. Add the typed mock below.
The detected-twin test currently counts only createAgentMenuTerminalPendingSelector renders. useAgentMenuObservation has additional subscriptions, so the readiness assertions do not establish the complete picker render contract for catalog republication or readiness updates. This is a missing coverage case, not an observed current regression. Reuse mountPickers or an equivalent full-observation counter for this scenario.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
Starting an agent could report one failure twice: launch preparation showed an error, then its caller treated the empty result as another failure. Cancellation used that same empty result. The launch action now reports one localized message with recovery advice, names the requested or selected agent, and lets callers recognize an already-reported refusal. Cancellation and a host still connecting stay silent.
While a host is preparing, terminal choices remain visible and disabled with a reason. Chat and Configure remain usable, drafts survive readiness changes, and the selected surface stays unchanged. Start work and Fix Checks also report failures before workspace creation once, while preserving their functional fallback actions. Existing native submenus, menu closing, host routing and shell selection remain intact. No saved field or wire format changed.
Independent Claude Opus-medium and Codex-high reviews are complete on 5b5b5b8c98d767b57bf25f4d4f0982ca31dadb5e, with no open required findings. The final reference comparison and independent performance review are complete. Performance proof retains N/10N operation bounds, render isolation and live invalidation checks. Intended differences for multiple hosts and separate chat/terminal readiness are documented in the PR.
After main moved again, merge 02c15d63e2023570bb62590239dedae57e928d0a brought in main 228263d5664fca3ece26171dbeaa87ff24517d27, following the previously approved sync cc25a3108b540299bb7c44352d1f164084a1e405. The conflict resolution preserves main’s retained app-state views and observer lifecycle, Source Control placement, startup directory and workspace-trust behavior. It also preserves the approved terminal-pending selectors, stable menu drafts, explicit launch refusals and single action-owned recovery message. Main’s removal of the background launch argument stays in place. The unchanged unavailable-host refusal branch moved into the existing refusal module to satisfy the combined file-size check, and existing test narrowing wraps the new accepted host-launch cases without changing their assertions. The staged-file lint, plugins, source scans and formatting checks passed before the merge commit. No dependency install was needed.
A further main update was merged in c2b0608350536069f314329df3d441845b06181a, bringing in 08c8c33026b2116e2b151a29fe57baa3095155a3. Its only conflict was the import block at lines 1–4 of the direct work-item launcher. The resolution keeps main’s setup-approval helper and the existing failure-message and setup imports; no launch logic changed. Both continuing independent reviewers checked that hunk in one pass, confirmed that main’s automatically merged setup-approval call is preserved, and found no issues. Main was then merged without conflicts or manual source edits in 20b941a802b43c05bf09dd9e8ecad1b4cf7d4e19, bringing in 5adbcac51122a47fe465288f6169171128a4725f and #39426’s registry lint correction. Both continuing reviewers independently verified the empty resolution diff and unchanged launch patch, with no findings. The earlier five conflict resolutions and launch/view boundaries remain reviewed. The existing screenshots, renderer/integration tests, references and performance proof carry. No additional QA run was needed. Merge-specific staged-file checks passed again, with no dependency install.
Main moved again and was merged in 401335abbb17c46b7c6acf6b8eb7033f61e85701, bringing in cecfa9865d1e03603fdb27efde5966ee73be4c8b. All four conflict blocks were imports: the menu-owner test, menu observation hook, structured launch admission, and paired-web launcher test. The resolution follows main’s moved runtime owners while retaining the launch refusal and facts imports. Five supporting import or mock paths in lane-only code were adapted to those same owners; the obsolete direct revision import stays removed because its call lives in the extracted facts selector. Both continuing independent reviewers checked the exact commit in one pass and found no changes to launch logic, test bodies or assertions. All moved modules remain owned by main; none was restored. The four affected named suites from that sync passed 48 tests, and merge-specific staged-file checks passed. No dependency install or additional app run was needed; the existing QA, reference and performance evidence carries.
A further conflict-only merge 5f4e1121d1b6bae861668845d073cea74f6ee6d6 brings in main bc215d95ec0dc38f367fc156f7c4259b96025f81. The two conflicts retain the moved composer-focus type and main’s extracted direct-launch failure reporter. The extracted reporter reads the same current requested or resolved agent when a failure is reported, preserves the approved named-agent message, and calls the existing notification callback once. Main’s new direct-start inputs and accepted prompt-delivery path remain intact. Seven supporting imports and mocks now target main’s moved owners. The combined file’s first CI run found that formatting expanded one argument-destructuring declaration past its existing file-size limit. Commit 15d974882aacd604a99758c98805c9913159adb9 restores the two original declarations with identical field-read order; the normal hook passed and the continuing Codex reviewer verified that narrow correction. Claude’s unchanged receipt carries across it. The six affected named suites passed all 48 tests. Main’s reporter extraction and routing exports remain intact. Both continuing independent reviewers checked the exact merge in one pass and found no issues; no launch outcome, assertion or user flow changed in the resolution. Unchanged UI evidence carries. No dependency install or new app run was needed.
Validation: current-head CI is green, including static analysis/typecheck, all 12 unit shards, both packages, layout checks and changed end-to-end specs. Managed-SSH checks were not selected by this run; prior unchanged evidence carries. Five focused merge suites passed 45 tests; the final launcher and paired-web suites also passed on the committed contents. Retained focused tests cover every changed launch surface, refusal/cancellation/pending outcomes, selected-host agent identity, prompt retention and a literal French recovery message. The web-runtime suite passed 10/10. The previous sync’s first CI attempt had a worker SIGSEGV while running the unchanged plugin reconciliation test, without a failed assertion. One failed-job retry passed that shard and the aggregate; no source or test assertions changed for the retry, and the crash cause remains undiagnosed. The merge-specific staged-file checks passed.
QA: the retained native menu check at 5b5b5b8c98d767b57bf25f4d4f0982ca31dadb5e passed English/dark and French/light, pending and settled, on Windows. All four screenshots show full left-aligned labels; pending rows have one linked reason, disabled primary launch and enabled Configure, while settled rows have no reason. Enter and Space while pending opened no agent tabs and produced no toast. The Codex driver matched the recorded app identity. The paired server and client are stopped, their ports are clear, and real config hashes are unchanged. The prior branch-only screenshot run passed Configure pending→settled with the draft preserved, French/light Configure, and searched Configure reopened by Enter. Those unchanged flows were captured at 94b0ac14be4c357f65a9a37051cfee851d0022a7; subsequent tests-only commits preserve their evidence. The label alignment change has its own row proof at that head; the later main merges preserve it. These are the UI/tests rows of the QA table; no agent was launched for screenshots. Launch outcomes are covered by renderer/integration tests.
Deferred: a trivial launcher pass-through alias (P3), and test-only cleanup to restore the original window API after a Windows fixture. The continuing independent reviewer traced every subsequent case: its remote, performance and liveness proof remains intact. A separate automated claim that the settlement helper returns null was refuted: the conditional expression returns undefined for cancellation. No shipped correction or weakened assertion resulted from either comment. The existing settled SSH missing-platform issue is tracked separately in STA-13357. The inherited registry lint blocker is corrected by main; the current-head CI run is green. The coordinator owns a separate performance check requested before merge; this status does not claim that separate check is complete.
For Brennan’s review:
Native menu while its host is connecting (English/dark):
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
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
The problem
Starting an agent from New Chat, a shortcut, Hand Off, Source Control or Fix Checks could show two error toasts for one failed start. The step that prepares the launch showed its own error and returned nothing; every caller then treated "nothing" as an unknown failure and showed a second, internal-sounding "Could not build launch command" message. "Nothing" also meant the user had cancelled, so callers were guessing. While a remote host was still connecting, clicking a terminal choice also produced an error telling the user to connect, although it was already connecting.
What changes for users
How it works
Launch preparation now only returns facts (the host's OS, or why it isn't known and whether that is still pending). The launch action is the single place that reports a failure: it shows one localized, named-agent message and returns the existing refused result with a notification receipt, which every caller now checks explicitly instead of inferring failure from an empty result. Chat routing runs before the terminal-only OS check. Fix Checks uses its existing per-attempt notification callback so the action and its fallback cannot repeat the same failure. The menus derive one "terminal still waiting" value from the host's live connection state and apply it where a terminal choice is shown or chosen; it is not stored and is kept out of the picker's identity keys, so it never resets drafts or inventory.
Tracked in STA-13205.
Review path: full. This changes launch failure handling and routing order and has a reviewed plan; it is outside the light path's local-effect and low-risk criteria. The change introduces no saved field or wire format.
Intended: terminal launches still require the execution host's reported launch information to preserve shell quoting; chat admission does not depend on it. Intended: only a host that is still connecting or setting up disables terminal choices; settled missing information (STA-13357) still reaches the one accurate refusal, per the coordinator's scope ruling. Intended: the readiness value is per menu rather than per agent row, and chat and terminal readiness are separate, because Orca offers both surfaces for one agent.