Repository navigation
Retry failed agent starts before rejecting your message - #33045
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.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 48 functions across 51 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
| Description check | |
The description explains the change, rationale, behavior, constraints, testing, and QA results in detail. However, it does not follow the required template structure, does not provide an actual linked… | Reformat the description using the repository template. Add the required section headings, provide an actual issue link or Fixes #..., state N/A with a reason for Visual Proof if no UI change exists, complete the Testing and Checklist i… |
-
Check name Status Explanation Title check The title clearly summarizes the primary change: retrying failed agent starts before rejecting the requesting message. 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: Description check
-
Explanation
The description explains the change, rationale, behavior, constraints, testing, and QA results in detail. However, it does not follow the required template structure, does not provide an actual linked issue in the required format, and does not include the required Visual Proof, checklist, notes, or AI Disclosure sections.
Resolution
Reformat the description using the repository template. Add the required section headings, provide an actual issue link or
Fixes #..., stateN/Awith a reason for Visual Proof if no UI change exists, complete the Testing and Checklist items, address Notes, and complete or explicitly apply the AI Disclosure guidance.
Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
⚔️ Resolve merge conflicts: 💡
-
- Resolve merge conflict in branch
brennanb2025/start-retry
- Resolve merge conflict in branch
-
- 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.
|
A transient agent startup failure previously failed the first message immediately and could also fail messages waiting behind it. Native-chat message and operation starts now retry eligible failures within the original start flight, up to five attempts. A final failure belongs only to the original requester, and accepted Stop prevents any later attempt or late startup-error settlement. Full review completed in two functional rounds. The loop fixed the accepted-Stop/final-settlement race and removed duplicate retained child identities. Later changes corrected loop-added test fixtures and added real recorded-protocol contracts; no deferred production P0/P1 remains. The architecture follows the common single-owner pattern. Intentional differences remain: positive owning-host exit proof, immediate finality for typed refusals/deadlines/host faults, one original startup ceiling, process-local retries, one-shot legacy direct attach, and the existing final presentation. Failed final journal writes retain evidence and recover on the next ordinary wake; no autonomous recovery scheduler was added. Verification: CI on One targeted M4Air QA round used real Claude and Codex. Previous-build chats survived graceful restart with retained history/title/session and two new replies each. One actual startup failure retried without resend; accepted Stop prevented further attempts and the next send worked; five failures rejected only the first requester while the queued later message received a real reply. Fault scenarios used bounded, locally committed transport instrumentation with real CLI processes; retry/settlement policy was unchanged. The original incomplete Claude queue attempt was retained as incomplete, then only that missing case was completed. A literal Stop-label assertion failed because the existing UI renders “Stopped manually”/“Cancellation requested” before a provider turn exists; the same rendering is present on the baseline, and the lifecycle assertions passed. No UI copy change is included. The QA production head was For Brennan’s review
Judgment points: retry ownership and final cancellation check, retry admission, requester-bound final row. Since QA (10-25): main merged once (d1e2d54) to clear conflicts from the repository-wide comment cleanup and the move of the Codex app-server files into |
Resolves comment-cleanup conflicts in three Claude startup tests (keeps five-attempt assertions) and repoints the Codex request-error import after the app-server colocation move.
A transient agent startup failure currently rejects the first message immediately. Messages waiting behind that same start can inherit its failure even though they did not request it.
Native-chat message and operation starts now silently retry eligible failures up to five attempts, with 100/200/400/800 ms between attempts. A successful retry delivers waiting messages in order. Exhaustion settles only the original requester; an operation-owned start does not reject a message that joins later. Accepted Stop prevents another attempt and suppresses a late startup-error settlement. Tracked in STA-11117 and STA-8496.
The existing start flight owns acquisition, provider opening, readiness, cancellation and final settlement. Each attempt retains its physical child identity; duplicate publications retain one copy. Intermediate failures add no error row or status churn. No saved field, wire operation, setting or minimum paired-server version is added.
Differences from the common pattern are intentional:
Validation: CI on
5b9c8e5passed all five Node shards, static analysis/typecheck, relay, cross-version compatibility, mobile and Windows packaging; expected headless/WSL workflows also passed. The completed shard logs explicitly record seven recorded-output cases, eight host-settlement cases and two registry cases passing. Full independent review completed in two functional rounds with no open production P0/P1. Host N4/40 contracts verify 20/200 failed starts, exact requester settlements, ordered successors, Stop precedence and retirement/isolation; registry N10/100 covers duplicate identity retention. Seven recorded-output contracts use real Claude/Codex/Pi/ACP rejection frames with command/version/exit provenance. Claude exercises its SDK callback boundary; these are representative protocol errors, not startup-auth tests or Pi/ACP app QA.One targeted real-agent QA round passed with Claude and Codex: previous-build chats through graceful restart and two new replies each; automatic retry without resend; Stop followed by a successful new send; and five failures settling only the original requester while a queued message gets a real reply. Unmodified builds covered restart/upgrade; bounded QA-only transport instrumentation exercised actual CLI failures without changing retry or settlement policy. The incomplete initial Claude queue attempt was retained as incomplete and only that missing case was completed. The pre-turn Stop wording remains the baseline’s “Stopped manually”/“Cancellation requested”; lifecycle invariants passed, and no copy change is included. QA also passed 38 named contracts through tc-gate. Screenshots and exact evidence limits are in the review-loop status comment.
QA used production head def5845; subsequent ccf820d, c881593 and 5b9c8e5 change only tests/fixtures. Local dependencies are stale; no manual installation was performed here. All QA processes and transfer terminals stopped, five real config hashes were unchanged, and the QA slot was released.
Author: @BrennanKB5