Repository navigation
Conversation
…t rows A turn that fails with model_request_failed/model_request_timeout (5xx server error, provider overload, retryable network failure) no longer fails the bb turn immediately. The bridge holds the turn open, emits a provider.error reconnect row (willRetry), and re-runs the turn through the native v4 retryTurn control - the same rewind-to-last-input action as ZCode Desktop's Retry button, without duplicating the message. The retried native turn streams into the same bb turn. - Budget: two automatic retries per turn (~15s, ~45s waits; the native CLI has already exhausted its internal 11-attempt ladder), then the turn fails explicitly with the original error. - Guards: turns that already ran tools are never retried (a re-run would repeat side effects), idle-time ticket runs keep their own requeue path, interrupting the thread cancels a pending hold, and a duplicate turn.failed cannot settle a held turn. - Visibility: native apiRetry state on the event envelope (attempt, max, delay, HTTP status) is surfaced deduplicated as reconnect rows while the CLI climbs its internal ladder. Docs: configuration.md section, CHANGELOG unreleased entry, knowledge base invariants/log. Tests: tests/retry.test.ts (8 regressions).
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 385645e9f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🔵 Needs a closer look
The change adds intricate asynchronous turn-lifecycle state machinery (holds, timers, interrupt races, native retryTurn) whose correctness depends on SDK grammar and timing behavior that cannot be fully verified here, so it warrants final human review.
1 open issue
What changed in this PR
This PR makes the bridge resilient to transient upstream provider failures. Previously any turn.failed with a transient condition (the 2026-10-25 Z.ai 5xx outage) surfaced immediately as a failed turn. Now the bridge holds the bb turn open and re-runs it via ZCode's native v4 retryTurn control, keeping each attempt visible through provider.error reconnect rows. It fits into the existing session/turn lifecycle in src/bridge.ts, reusing the History.command channel and the Timeline tool-activity signal.
Changes:
- Adds a bounded, conservative automatic-retry path (2 retries at ~15 s/45 s): only for transient codes, only for turns that ran no tools, never for idle-time (off-peak) runs; interrupts and duplicate
turn.failedevents are handled. - Surfaces native
apiRetryladder state from the event envelope as deduplicatedprovider.errorreconnect rows. - Tracks per-turn tool activity in
Timeline(toolActivity/noteToolActivity) so tool-bearing turns are never retried.
Per the repository's knowledge-maintenance guideline, this PR updates the knowledge base alongside the behavior change: knowledge/log.md, knowledge/architecture/session-invariants.md, plus user docs (docs/configuration.md) and CHANGELOG.md.
| File | Description |
|---|---|
| src/bridge.ts | Core retry logic: transient classification, hold/retry loop via retryTurn, exhaustion failure, apiRetry surfacing, retry-budget reset. |
| src/timeline.ts | Adds ranTools tracking with toolActivity getter and noteToolActivity() to gate retries on tool side effects. |
| tests/retry.test.ts | 8 new regression tests covering retry success, exhaustion, tool/idle exclusions, interrupt, timeout, and apiRetry rows. |
| docs/configuration.md | Documents the upstream-failure retry behavior for users. |
| knowledge/architecture/session-invariants.md | Records the retry invariant and adds a hard-bounds table row. |
| knowledge/log.md | Pins verified native-protocol facts behind the retry implementation. |
| CHANGELOG.md | Unreleased entry describing the new retry behavior. |
Review effort: Balanced
…retry guard Codex P2 fixes on 385645e: - Bind the retry-start watcher to its original hold (bridge.ts:910): when a retried turn starts and fails again between two watcher polls, the next hold is installed before the first watcher notices. The watcher loop and failRetriedTurn now compare session.retry against the captured hold object, so a superseded watcher stands down instead of expiring against the newer hold's longer delay and failing the turn with a false "did not start the retried turn". - Handle already-streamed text before rewinding (bridge.ts:790): retryTurn rewinds the native conversation, but the bb timeline cannot retract the failed attempt's streamed text - a retry after partial output would leave the abandoned answer next to (or concatenated with) the replacement. Timeline now tracks streamActivity, and canHoldForRetry excludes turns with any streamed text or reasoning; they fail explicitly instead. Regressions: fast-second-failure watcher race completes the turn; streamed partial answer fails without retryTurn. 222 tests green.
|
Review round 1 fixes pushed in d95fcc9:
222 tests green, typecheck/format pass. |
Copilot review: every thread/delta this suite produces - including the new provider.error reconnect rows - is now checked against threadDeltaNotificationParamsSchema in the harness write hook, with explicit grammarFailures assertions on the retry-success, retry-exhaustion, and apiRetry-surfacing regressions.
|
Copilot's grammar-coverage point fixed in 1e420e4: the harness now validates every emitted |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Stacked on #2 (needs the off-peak session markers it guards on).
When the model provider fails a turn with a transient upstream condition — the Z.ai 5xx outage on 2026-10-25 failed every turn with "Provider returned a server error." — the bridge now retries the turn instead of surfacing the error immediately.
retryTurncontrol (the CLI's own Retry action: rewind to the last user input, no message duplication). Up to 2 automatic retries per turn (~15s, ~45s), then explicit failure with the original native error.provider.errorreconnect row (willRetry: true), and nativeapiRetryladder state on the event envelope is surfaced deduplicated ("ZCode retry 3/11 in 5 s").turn.failedcannot settle a held turn, older CLIs without v4 controls fail explicitly.Verified facts about the native protocol (pinned in
knowledge/log.md):turn.failedcarriescode/type = model_request_failedwithoutretryable;retryTurntargets the conversation tip row withactions.canRetry; bb rendersprovider.error+willRetry: trueas a reconnect row.8 new regressions in
tests/retry.test.ts; full suite 220 green; typecheck/format/marketplace/build pass.