Stop retry amplification: director no longer re-wraps harness-exhausted retries - #595
Merged
TheGreatAxios merged 2 commits intoAug 24, 2026
Conversation
TheGreatAxios
force-pushed
the
cl-6910-retry-amplification-up-to-9-identical-full-context-sends-per
branch
from
August 24, 2026 05:13
701d314 to
761492b
Compare
The harness's own retry policy already retries and exhausts timeout/retryable/quota_exhausted errors (up to 3 attempts) before an inference.error of one of those categories reaches the director. The director's recovery layer re-issued another full-context infer() call for the same categories, multiplying with the harness's own attempts instead of composing with them -- up to 9 identical full-context sends per logical turn with no wall-clock ceiling. Director recovery now only handles internal-recovery-abort, the one category the harness never retries on its own, so the two layers no longer multiply. Attempt counts are logged on each recovery.
The vendored DefaultDirector's ERROR_PREAMBLE map has no timeout entry
and falls back to the fatal ('unrecoverable inference error') wording.
CL-6910 makes an exhausted timeout the routine terminal state instead
of a rarity, so intercept it in ChatDirector and reply with accurate,
calm wording instead of patching the vendored map.
TheGreatAxios
force-pushed
the
cl-6910-retry-amplification-up-to-9-identical-full-context-sends-per
branch
from
August 24, 2026 05:16
761492b to
fa80d56
Compare
TheGreatAxios
enabled auto-merge (squash)
August 24, 2026 05:17
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
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.
Closes CL-6910.
Guardrail 4 applies (director changes): needs-sawyer-review, no auto-merge.
Verified layer map (before)
Two layers, each contributing attempts to a single logical turn:
Harness retry policy (
vendor/intx-inference/src/retry-policy.ts,createDefaultRetryPolicy) — a per-infer()-call mechanical retry. Forretryable/timeout/quota_exhausted, up toMAX_ATTEMPTS = 3attempts with backoff before it returns{ kind: "abort" }. Only then does the harness surface aninference.errorevent to the caller (confirmed invendor/intx-inference/src/harness.ts:1479-1549— the loop only yieldsinference.errorafterdecision.kind === "abort").Director recovery (
src/agent/director.ts,decideInner) — on receiving aninference.errorof categorytimeout,retryable, oraborted+internal-recovery, calledcapabilities.infer()again, up toMAX_INFERENCE_RECOVERIES = 2more times, resetting at each turn boundary (inference.done) and onmessage.received. Each recovery also fired acheckpoint(a git commit viacontextStore.commit()).The bug: for
timeout/retryable, aninference.erroronly ever reaches the director after the harness has already exhausted its own 3-attempt budget. The director's recovery then started a brand-newinfer()call with a fresh harness-level 3-attempt budget — up to 2 more times. Worst case:(1 initial + 2 director recoveries) * 3 harness attempts = 9identical full-context sends for a single logical turn, each director recovery also committing a checkpoint. This matches the ticket's "up to 9" exactly.quota_exhaustedwas never in the director's recovery trigger, before or after this change — its director-facing behavior is unchanged by this PR. It's included above only because it shares the harness-layer 3-attempt budget withtimeout/retryable; separately,src/agent/retry-policy.ts'screateCorbitsRetryPolicyalready abortsquota_exhaustedon the first attempt whenerror.retryAfterMs > MAX_BLIND_WAIT_MS(30s) — a deliberate short-circuit so the harness doesn't blind-wait through a long provider-declared backoff, and unaffected by this PR.RetrySituation.elapsedMsis threaded through by the harness (vendor/intx-inference/src/harness.ts:1532) but never read by either retry policy (createDefaultRetryPolicy/createCorbitsRetryPolicyboth destructure onlyerror/attempt) — confirming "the retry policy receives elapsed time but ignores it."What changed (structural, not a cap)
aborted+internal-recovery is the only category the harness's own policy never retries (it's in the harness's explicit "never retry" list). So it's the only category where director-level recovery was ever additive rather than duplicative.src/agent/director.ts: the recovery trigger now only fires foraborted+internal-recovery-abort.timeout/retryableerrors that reach the director are already-exhausted harness attempts, so they now fall through to the baseDefaultDirector's existinginference.errorhandling (checkpoint + reply), instead of being re-wrapped in anotherinfer()call.quota_exhaustedwas already excluded from the director's recovery trigger before this PR and is untouched by this change.This removes the multiplication for
timeout/retryablerather than capping it: the two layers no longer share those two error categories, so there is nothing left to compound for them. No retry threshold, backoff constant, or attempt count changed —MAX_ATTEMPTS(harness) andMAX_INFERENCE_RECOVERIES(director) are both untouched.Worst-case send count after
timeout/retryable: bounded at the harness's own 3 attempts, director does not add to it (this is the fix — before this PR the director could add up to 2 more full 3-attempt cycles).quota_exhausted: bounded at the harness's own 3 attempts (or aborted after attempt 1 ifretryAfterMsexceeds the 30s blind-wait cap) — unchanged by this PR, since the director never recovered this category.aborted/internal-recovery: bounded at 3 (1 initial +MAX_INFERENCE_RECOVERIES=2 director-triggered re-invocations), each a single harness attempt since the harness never retriesaborted.infer()calls, the true worst case across a whole turn is bounded (not open-ended) at up to 5 (e.g. abort, abort, then a final call that itself exhausts 3 harness attempts) — nowhere near the prior 9, and it cannot grow further because neither layer's own cap changed.Before: up to 9 (via
timeout/retryablecompounding). After: up to 5, and 3 in the common single-category case.Number I chose, and why
No retry threshold, backoff, or attempt count was changed. The only new number is 0 — no wall-clock ceiling was added. Tonight's eval matrix recorded a single legitimate grok-4.6 turn lasting 746 seconds; any ceiling picked without data risks aborting healthy turns like that one. Per the ticket's own guidance, de-compounding is implemented here and the wall-clock ceiling is left as a follow-up once there's data on what a genuinely stalled retry sequence looks like versus a slow-but-healthy one.
Also added: a
logger.warnline on each director recovery attempt and on exhaustion, stating the attempt count and category, so a retry storm is visible in traces (outcome bullet 3).UX: timeout preamble
An exhausted
timeoutnow routinely reaches the director's terminal reply path (previously it was retried and rarely surfaced). The vendoredDefaultDirector'sERROR_PREAMBLEmap (vendor/intx-inference/src/default-director.ts) has notimeoutentry, so it falls back to thefatalpreamble ("...unrecoverable inference error"), which is misleading for an ordinary exhausted timeout. Rather than editing the vendored file directly (out of scope for this wave perscripts/verify-corbits-only-scope.sh),src/agent/director.tsnow interceptsinference.errorwith categorytimeoutbefore it falls through to the vendored handler, and replies with calm, accurate wording instead (the provider didn't respond in time; the request was retried and gave up).Tests
src/agent/retry-policy.test.ts— three new tests proving the harness-layer policy aborts by the 3rd attempt for each error class named in the ticket: rate limit (quota_exhausted), gateway error (retryable), and malformed response (protocol_mismatchHTML gateway page, normalized toretryable).src/agent/director.test.ts— newdescribe("ChatDirector inference-error recovery (CL-6910)")block: parametrized test provingretryable/timeout/quota_exhaustedno longer produce aninferaction from the director; internal-recovery-abort still recovers up toMAX_INFERENCE_RECOVERIES, then replies; an unrelated (user-stop) abort is never recovered; the recovery budget resets at the next turn boundary; and a worst-case-count test asserting the director-owned path issues exactly1 + MAX_INFERENCE_RECOVERIESinfer calls, never more.src/director.test.ts— updated the pre-existing "retries recoverable inference failures" test (which asserted the old, buggy re-wrapping behavior fortimeout) to assert the new pass-through behavior instead.src/agent/director.test.ts— new test"timeout category produces the timeout preamble, not the fatal fallback"asserting the reply text for atimeoutinference.errorcontains the new timeout wording and not thefatalpreamble's "unrecoverable inference error".All assertions are on action/send counts, not timing.
Gate
bun run check(lint, typecheck, build, test) green in the foreground.