Repository navigation
Explain agent startup failures in plain words - #31932
Conversation
|
Note Reviews pausedIt 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
Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 8 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly describes the main change: replacing internal agent startup errors with plain-language explanations. 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. Description check The description is detailed and covers the problem, user-facing behavior, rationale, scope, testing, QA evidence, review classification, and known limitations. It does not use every template heading, … Full details: Description check
-
Explanation
The description is detailed and covers the problem, user-facing behavior, rationale, scope, testing, QA evidence, review classification, and known limitations. It does not use every template heading, and it does not provide a direct issue link in the required
Fixes #format, but the content is mostly complete.
-
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
792f27f6-95c6-4206-8f5f-858a739c3b76
📒 Files selected for processing: 11
src/renderer/src/components/terminal-pane/TerminalErrorToast.test.tssrc/renderer/src/components/terminal-pane/TerminalErrorToast.tsxsrc/renderer/src/components/terminal-pane/agent-launch-pane-notice-text.tssrc/renderer/src/components/terminal-pane/agent-launch-pane-notice.test.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/fr.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/pt.jsonsrc/renderer/src/i18n/locales/zh.json
🚧 Files skipped from review as they are similar to previous changes: 7
- src/renderer/src/i18n/locales/es.json
- src/renderer/src/i18n/locales/zh.json
- src/renderer/src/i18n/locales/en.json
- src/renderer/src/i18n/locales/ko.json
- src/renderer/src/i18n/locales/pt.json
- src/renderer/src/i18n/locales/fr.json
- src/renderer/src/i18n/locales/ja.json
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
|
Agent startup failures could show an internal token in the terminal toast while the pane notice explained the same failure separately. This PR gives both existing surfaces one localized explanation naming the known agent and a next step, while preserving uncertain outcomes and older-server compatibility. Review complete: three source rounds, no outstanding source findings. Production review and actual agent QA ran on Fixed: missing-workspace guidance, startup strings incorrectly described as definite failures, suppression of reporting for unrelated accumulated errors, and a duplicate agent-identity lookup. A real-render contract bounds token matching and translations at 2 and 20 accumulated rows. Intended differences from the common pattern are compatibility with independently updated servers, existing terminal surfaces, and controlled localized wording. No lifecycle, restored-reply, process, persistence or wire behavior changes. QA is classified per changed flow under the October 23 rules:
The marker uses Evidence limits: the bounded native-Codex failure attempts showed the ordinary “Terminal exited” overlay, so neither changed startup surface was reproduced live. The images below render actual component DOM from the real mapping with historical producer-shaped payloads; a live agent startup failure could not be triggered on demand. The missing-workspace token has no current or shipped producer found in the bounded search, so that input is an explicit contract fixture. The older published 1.4.237 server paired successfully and returned a real Codex reply, but its failed-start notice was not induced. No new QA run or reviewer was started for this completion. Restart is outside this PR: the diff changes no restored-reply code. The historical restored Codex reply remained unverified; it is not a changed flow or a passing result. Actual recovery replies after failure are verified separately above. Underlying agent exit causes remain outside scope. A hypothetical independent diagnostic on the same line remains deferred/unproven; distinct accumulated errors retain reporting and have renderer coverage. No known broken changed flow remains. All owned test copies, keepers, drivers and shell terminals are stopped, and ticket 1700 is released. Main was merged once before QA; no additional sync occurred. For Brennan's review These are renderer fixtures of the changed surfaces, using actual component DOM and the actual stylesheet, not running-app screenshots. Each image includes known early exit, missing workspace and uncertain outcome in English and Chinese. Startup toast:
Typed pane notice (including the existing localized Copy prompt label):
The earlier whole-window comparisons show the attempted live triggers honestly:
Actual recovery replies on production head:
Actual released-server normal reply:
Supplementary restart captures, outside the changed flows: retained terminal action below, followed by restored Codex still Working (new reply unverified).
Judgment points: recovery and uncertainty wording, older-server string interpretation, and reporting-link scope. |
Problem
On macOS and Windows, an agent that exits while starting can leave a terminal toast displaying an internal startup token and asking the user to file an issue. The pane notice already translated startup reasons, but the generic toast displayed the same reason as raw text. Tracked in STA-11432, STA-11463, and STA-11471. The underlying reasons the agents exited are outside this change.
User-facing change
For a known exit, the toast and pane now say, for example: “Codex exited while starting. Run it in a terminal in this folder to see why, then try again.” If the agent identity is unavailable, the message uses the translated generic agent label. All seven locale catalogs carry the named messages and recovery copy.
The message preserves what the host actually knows. A typed host outcome remains authoritative. Older host errors sent only as strings assert failure for the observed-exit and missing-workspace cases; other startup tokens, including unknown outcomes, ambiguous claim results and future codes, say: “Couldn't confirm Codex started. Check its terminal before trying again.” Internal tokens and transport wrappers are hidden rather than treated as proof that another launch is safe.
A missing workspace gets a possible next step: “Restore the workspace folder or open an available workspace, then try again.” This works for both folder workspaces and Git worktrees without requiring a command to run in a missing folder.
A fully explained startup toast omits the generic issue request. If an independent error such as “Paste failed” shares the toast, that text and its reporting link remain available. Existing environment footers do not count as independent errors for this decision.
Both surfaces reuse the existing formatter and active pane's existing agent identity. No startup detection, process handling, launch admission, retry behavior, saved state or wire format changes. Older paired servers keep their existing launch paths and strings.
Design and review classification
The common pattern separates the outcome from the words displayed in an existing error surface. Intended differences: this client retains compatibility with older server strings and translates them into localized, controlled copy because raw provider diagnostics are not suitable user-facing messages. No new visual surface is added.
This PR takes the FULL review path. Its initial light classification missed the recent restart incident in #28043 affecting a touched notice component. The architecture plan and its independent checks were written retrospectively after the initial implementation; they are not pre-code approval. The accepted design keeps one renderer presentation boundary over existing typed outcomes and compatible strings, preserving uncertainty and distinct recovery actions.
The startup-error wording in stale overlapping draft #26551 is superseded by this PR's shared formatter. That draft is unchanged; this PR does not close it.
Validation
The full three-round source review and final decider are complete. No outstanding source findings remain. Current production and actual agent QA head:
003bbcf266f069a1c079810fe54cb1fda10b9e08. The only later commit,ada55dc0b085ffa9036df964573441b277efbe04, changes an existing renderer test only; actual agent evidence is retained under the nonshipping-change rule. Full current-head PR CI and normal precommit checks passed.Each changed flow gets its own QA row. Startup-toast wording, typed-pane wording and missing-workspace guidance are Light UI or copy (
qa=tests): actual component renders exercise the real code-to-copy mapping and shipped English/Chinese strings, with one screenshot per surface attached to the review status comment. Both surfaces render known exits, missing workspaces and uncertain outcomes. The maintained exact three-file suite passes 77 tests; one temporary screenshot test also passed in the same named run (78 total / four files). It was archived outside the repository after capture. The 2/20-row operation-count contract still passes.Actual Claude and Codex recovery replies after failure passed on baseline and production head (
qa=done). The exact-head client also paired with published Orca 1.4.237, browsed its filesystem and received a real Codex reply. The completion marker recordsqa=doneas the heaviest row actually run; the copy flows themselves use renderer tests and screenshots.The live startup notice could not be triggered on demand: bounded actual Codex failure candidates produced ordinary Terminal exited overlays. The new screenshots honestly show actual component DOM, real mapping and stylesheet with historical producer-shaped payloads; missing workspace is a contract fixture because no exact current/shipped producer was found in the bounded history search. They are renderer evidence, not a claim of successful live failure reproduction. The released-server failed-start notice was not induced; its string contract is covered by the renderer tests.
Restart is outside this change: no restored-reply, saved-state or lifecycle code changed. The historical restored Codex reply remained unverified and is not claimed as a pass. Existing actual recovery replies above passed independently. Underlying agent exit causes remain outside scope. No changed flow remains unverified under its required QA row.
All owned rigs, keepers, drivers and shell terminals are stopped and ticket1700 is released. Main was merged once before QA; this completion added no main sync, new QA run or reviewer. Screenshots, intended differences, fixed findings and remaining evidence limits are in the single review status comment.