Skip to content

[Feature]: Automatic turn-level retry (10 attempts) for retryable failures — today a transient 529 requires a manual /retry #501

Open

Description

@Hylouis233

Product or interface

Desktop app (the same shared local-runtime-v2 turn runtime backs CLI - interactive TUI, so this is a runtime-level request rather than a UI-only one).

Use case and problem

While a run is in flight it sometimes fails for reasons that are clearly transient — provider overload (529), rate limiting, upstream/network errors, a dropped connection before the first token. In those cases the turn terminates immediately, the session lands in error, and the only recovery is a manual user action: pressing the retry affordance (/retry in the TUI, catalog.ts:246, gated on context.canRetry).

That manual step is the problem. It costs the user one interaction per transient failure, and in unattended or long-running work (background tasks, goal loops, batch sessions) there is nobody there to press it, so a single 529 ends the turn even though the next request would very likely have succeeded.

Today the retry machinery is below the turn level and does not cover this case:

  • withLLMRetry (packages/agent-core/src/pi-turn-runner/llm-retry.ts) retries a single logical LLM call — one provider request that failed before output was committed. Once output commits (or a tool call has already run), a later stream failure stays on that attempt by design (preflightAttempt returns committed; discardAttempt at :304 throws away uncommitted events).
  • PiTurnRunner.runTurn drives pi's tool-calling loop exactly once. There is no outer loop that re-runs a turn.
  • LocalAgentTurnRunner.runAttempts (packages/local-runtime-v2/src/service/turn-system/agent-host/runner/local-agent-turn-runner.ts:173) is a whole-turn loop, but it only iterates for output-safety regeneration (OUTPUT_SAFETY_MAX_REGENERATIONS = 3, :80) and rethrows every attempt failure (:200-210).

The retryability verdict is in fact already computed and then thrown away: isProductionRuntimeErrorRetryable (runtime-error-retry-policy.ts:10, covering LLM_RATE_LIMITED, LLM_UPSTREAM_ERROR, LLM_TPM_RATE_LIMITED, LLM_CLUSTER_OVERLOADED) is wired into the commit pipeline (production-composition.ts:508) and lands on LocalRuntimeTerminalError.retryable (turn-commit-pipeline.ts:63,73) — which has no reader anywhere in the tree. The information needed to make this decision is already produced at the exact moment it is needed, and nothing consumes it.

Raising DEFAULT_LLM_RETRY_POLICY.maxRetries alone would not address this. It is a per-provider-request budget, and the companion maxRetryElapsedMs: 120_000 cap would cut a 10-retry budget short anyway (with baseDelayMs: 1000 / maxDelayMs: 30000, ten backoffs already exceed 120s).

Desired behavior

When a turn terminates with a retryable error classification, retry the turn automatically instead of surfacing a terminal error — up to 10 automatic retries (11 total attempts) by default.

Acceptance criteria:

  1. A turn that fails with a retryable classification (timeout, network, rate limited, TPM rate limited, overloaded/529) is re-run automatically without user action.
  2. The budget is 10 retries by default, and is configurable (users who hit Usage-limit retry loop after rapid verifier/session fan-out #108-style loops must be able to lower it or disable it).
  3. Only retryable classifications retry. These must stay terminal immediately: usage limit / quota exhaustion, credits exhausted, authentication failure, bad request, not found, content filter, and any user cancellation or abort. The shared classifier already encodes this — toLLMRetryDecision in packages/shared/src/llm-error-classifier.ts:222 retries only timeout/network/rate_limited/tpm_rate_limited/overloaded plus HTTP 529 — and a turn-level policy should reuse it rather than re-derive a second list.
  4. Backoff between attempts is bounded and jittered, with a total elapsed-time budget so a wedged provider cannot hold a turn open indefinitely.
  5. Retry is abortable: cancelling during backoff stops immediately and does not start another attempt.
  6. Attempts are bounded by what has already been shown to the user. An automatic retry must not silently duplicate output the user has already seen, so a failure that arrives after content has been released must not be blindly re-run.
  7. Each attempt is observable: attempt ordinal, error classification, delay, and final give-up reason are emitted, and a metric records retries per turn.
  8. When the budget is exhausted, the original terminal error is surfaced unchanged — no new error text, no degraded message.

Alternatives and additional context

  • The opposite failure mode already exists. Usage-limit retry loop after rapid verifier/session fan-out #108 reports ~80 Token Plan usage limit reached errors from a retry loop. That is why criterion 3 matters: quota exhaustion must never enter this path, and the classifier already treats USAGE_LIMIT_EXCEEDED as non-retryable.
  • [Bug]: Session remains permanently stuck after temporary provider failure during /goal #425 ("Session remains permanently stuck after temporary provider failure") was a different root cause — history re-sent every image until 400 Too many images in request: 31 > 30 made every retry fail — fixed by capping images. It is a useful precedent that retry affordances can themselves become the failure point.
  • Why not just document /retry? It does not help unattended runs, and it cannot help when the failure repeats deterministically at the start of a turn.
  • Scope note for maintainers: gateTerminalOnInputReview (local-agent-turn-runner.ts:507) already suppresses terminal events behind an async predicate, so holding a retryable terminal SESSION_STATUS/FAILED while the retry budget remains is a natural extension rather than a new mechanism. The open design questions are (a) whether to retry in place in runAttempts versus re-dispatching a new turn the way /retry does, and (b) how attempt history is retracted, since recallAttempt currently throws unless the display-retraction hooks are present.

No private project content, logs or account data is included.

Activity

  1. Hylouis233 commented on Oct 16, 2026

    @Hylouis233
    Author

    Reference implementation

    I put together a working implementation of the behaviour requested above so the proposal is concrete rather than hypothetical. It is on a fork branch:

    A pull request could not be opened from here: this repository does not accept pull requests from non-collaborator accounts (the API returns 404 on POST /pulls), which matches CONTRIBUTING.md. So this is posted on the issue instead. If a maintainer wants it, the branch is ready to pull; if you would rather implement it yourselves, the design notes below should still save the exploration.

    Where it hooks in

    gateTerminalOnInputReview already suppresses terminal events behind an async predicate. I extended it to hold a retryable SESSION_STATUS/FAILED instead of forwarding it, then re-run the whole tool loop from runAttempts. Releasing the original event when the budget ends means the user still sees the real error — no synthesized "gave up" message, and no duplicate text, because the intermediate failures never reach the client.

    Decisions worth reviewing

    • The retryable set is not re-derived. toLLMRetryDecision from @mavis/shared/llm-error-classifier stays the single source of truth; isProductionRuntimeErrorRetryable adds four MiniMax business codes (50111 / 50113 / 50150 / 50151). Usage limit (42212), credits (50110) and auth (50112) are excluded, which is what keeps this from reproducing Usage-limit retry loop after rapid verifier/session fan-out #108.
    • Retries stop once output has been shown. A retry re-runs the tool loop, so it would duplicate visible content. Failures after assistant output is released keep today's terminal behaviour. This narrows applicability to failures that happen before any visible output — a real limitation worth weighing against simply letting /retry handle the rest.
    • The held failure is consumed only when an attempt actually starts. My first version consumed it at decision time, which meant a cancelled backoff ended the turn with no terminal frame at all — the stuck-session class of bug in [Bug]: Session remains permanently stuck after temporary provider failure during /goal #425. There is a regression test for exactly that.
    • 10 is the requested default but may be too aggressive. Given Usage-limit retry loop after rapid verifier/session fan-out #108, defaulting lower and documenting how to raise it might be wiser; the elapsed-time budget (10 min) is the real guard against a wedged provider. maxRetries: 0 restores current behaviour exactly.

    Verification

    pnpm typecheck and pnpm check:source pass. 24 new tests cover the policy (classification, backoff, budget, abort, disabled) and the runner end-to-end (turn re-run three times with failures hidden, original error surfaced after exhaustion, usage limit and post-output failures not retried, cancelled backoff still delivers the terminal event).

    The full pnpm test:capabilities run had 4 failures on this branch and 3 on a clean main checkout; all are pre-existing flakes or environment issues (Homebrew libnode dylib, two filesystem-watcher timing tests, and one test that passes 3/3 in isolation). Details are in the branch's commit message and I can post the full attribution if useful.

    NOT RUN: full pnpm verify, the Windows profile, and any live-provider or desktop acceptance.

  2. Hylouis233 commented on Oct 16, 2026

    @Hylouis233
    Author

    Implementation diff (source only)

    Correcting my previous comment: the branch has moved on. It now has three commits, and two blocking defects that an independent audit caught in my first version have been fixed. Details and the audit findings below the diff.

    diff --git a/packages/local-runtime-v2/src/service/turn-system/agent-host/runner/local-agent-turn-runner.ts b/packages/local-runtime-v2/src/service/turn-system/agent-host/runner/local-agent-turn-runner.ts
    index 107bfa8..6d91daa 100644
    --- a/packages/local-runtime-v2/src/service/turn-system/agent-host/runner/local-agent-turn-runner.ts
    +++ b/packages/local-runtime-v2/src/service/turn-system/agent-host/runner/local-agent-turn-runner.ts
    @@ -14,7 +14,7 @@ import {
       buildAbortedTerminalStatusEvent,
     } from '@mavis/agent-core/event-bridge';
     import { getRuntimeBuildEnv } from '@mavis/config';
    -import { RuntimeEventStatus, RuntimeEventType } from '@mavis/protocol';
    +import { RuntimeEventStatus, RuntimeEventType, type IRuntimeEvent } from '@mavis/protocol';
     import type { MetricsClient } from '@mavis/shared/metrics-proxy';
     
     import {
    @@ -33,6 +33,13 @@ import type {
     import { LocalOutputSafetyEventWriter } from './output-safety-event-writer.js';
     import { runWithLocalEvalReporter, type LocalEvalReporterFactoryPort } from './eval-reporter.js';
     import { deriveLocalTurnRuntimeOutcome } from './turn-outcome.js';
    +import {
    +  createTurnAutoRetryCoordinator,
    +  turnAutoRetrySleep,
    +  type TurnAutoRetryCoordinator,
    +  type TurnAutoRetryPolicy,
    +  type TurnAutoRetryStopReason,
    +} from './turn-auto-retry.js';
     import { appendSafetyReplacement } from './safety-replacement.js';
     import { DEFAULT_OUTPUT_REVISION_INSTRUCTION } from '../preparation/config/prompt-templates.js';
     
    @@ -51,6 +58,17 @@ export interface LocalAgentTurnRunnerOptions {
       readonly onLLMRequestFailure?: PiLLMRequestFailureHook;
       readonly observeLLMRequest?: PiLLMRequestObserver;
       readonly evalReporterFactory?: LocalEvalReporterFactoryPort;
    +  /**
    +   * Turn-level automatic retry budget for transient turn failures. Defaults to
    +   * {@link DEFAULT_TURN_AUTO_RETRY_POLICY} (10 automatic retries). A policy with
    +   * `maxRetries: 0` disables the behaviour and restores the previous terminal
    +   * behaviour exactly.
    +   */
    +  readonly turnAutoRetryPolicy?: Partial<TurnAutoRetryPolicy>;
    +  /** Injectable backoff sleep; tests substitute a deterministic implementation. */
    +  readonly turnAutoRetrySleep?: (delayMs: number, signal?: AbortSignal) => Promise<void>;
    +  /** Injectable clock for the turn-level retry elapsed-time budget. */
    +  readonly turnAutoRetryNowMs?: () => number;
       /**
        * Per-turn recorder factory for the development LLM Context Inspector; when omitted, Pi assembly
        * is identical to behavior before injection.
    @@ -83,6 +101,9 @@ export class LocalAgentTurnRunner implements LocalRuntimeTurnRunnerPort {
       readonly acceptsEventSummary: boolean;
       private readonly piRunner: LocalPiTurnRunner;
       private readonly outputSafetyMaxRegenerations: number;
    +  private readonly turnAutoRetryPolicy: Partial<TurnAutoRetryPolicy> | undefined;
    +  private readonly turnAutoRetrySleep: (delayMs: number, signal?: AbortSignal) => Promise<void>;
    +  private readonly turnAutoRetryNowMs: (() => number) | undefined;
     
       constructor(private readonly options: LocalAgentTurnRunnerOptions) {
         // Injected runners and evaluation reporters retain their full event contract.
    @@ -90,13 +111,16 @@ export class LocalAgentTurnRunner implements LocalRuntimeTurnRunnerPort {
         this.piRunner = options.piRunner ?? new PiTurnRunner(buildLocalPiTurnRunnerOptions(options));
         this.outputSafetyMaxRegenerations =
           options.outputSafetyMaxRegenerations ?? OUTPUT_SAFETY_MAX_REGENERATIONS;
    +    this.turnAutoRetryPolicy = options.turnAutoRetryPolicy;
    +    this.turnAutoRetrySleep = options.turnAutoRetrySleep ?? turnAutoRetrySleep;
    +    this.turnAutoRetryNowMs = options.turnAutoRetryNowMs;
       }
     
       async runTurn(input: LocalRuntimeTurnRunnerInput): Promise<LocalAgentTurnRunnerResult> {
         const isUserTurn =
           input.reviewUserInput !== undefined &&
           (input.caller === undefined || input.caller === 'chat' || input.caller === 'channel_feishu');
    -    const observation = { regenerations: 0 };
    +    const observation = { regenerations: 0, autoRetries: 0 };
         try {
           return await runWithLocalEvalReporter(
             this.options.evalReporterFactory,
    @@ -105,13 +129,16 @@ export class LocalAgentTurnRunner implements LocalRuntimeTurnRunnerPort {
             this.options.logger,
           );
         } finally {
    -      if (isUserTurn) this.recordRegenerations(input, observation.regenerations);
    +      if (isUserTurn) {
    +        this.recordRegenerations(input, observation.regenerations);
    +        this.recordAutoRetries(input, observation.autoRetries);
    +      }
         }
       }
     
       private async runTurnCore(
         input: LocalRuntimeTurnRunnerInput,
    -    observation: { regenerations: number },
    +    observation: { regenerations: number; autoRetries: number },
       ): Promise<LocalAgentTurnRunnerResult> {
         const baseToolContext = requireToolContext(input);
         const observedLlm = withLlmResponseIdentifierLogging(input.llm, {
    @@ -125,7 +152,27 @@ export class LocalAgentTurnRunner implements LocalRuntimeTurnRunnerPort {
           await appendCancelledTurn(input, state);
           return buildRunnerResult(input, state);
         }
    -    await this.runAttempts({ input, toolContext, observedLlm, state, observation });
    +    const autoRetry = createTurnAutoRetryCoordinator(this.turnAutoRetryPolicy, {
    +      ...(this.turnAutoRetryNowMs ? { nowMs: this.turnAutoRetryNowMs } : {}),
    +      // `state.activeWriter` is the attempt currently in flight, so this reads
    +      // the authoritative record of what the user has already been shown.
    +      hasReleasedOutput: () => attemptReleasedOutput(state),
    +      isAborted: () => input.signal?.aborted === true,
    +    });
    +    try {
    +      await this.runAttempts({
    +        input,
    +        toolContext,
    +        observedLlm,
    +        state,
    +        observation,
    +        autoRetry,
    +      });
    +    } finally {
    +      // Unconditional: a held failure must reach the client even when an attempt
    +      // throws, otherwise the turn ends with a swallowed error.
    +      await this.releaseHeldFailure(input, autoRetry);
    +    }
         const inputRejected = await waitForInputReview(state.inputReview, input.signal);
         if (input.signal?.aborted) return buildRunnerResult(input, state);
         if (input.deferInputReviewResolution === true && shouldReportInputReviewResolution(input)) {
    @@ -151,6 +198,40 @@ export class LocalAgentTurnRunner implements LocalRuntimeTurnRunnerPort {
         }
       }
     
    +  private recordAutoRetries(input: LocalRuntimeTurnRunnerInput, autoRetries: number): void {
    +    if (autoRetries <= 0) return;
    +    try {
    +      this.options.metricsClient?.histogram('turn_auto_retries_per_user_turn', autoRetries, {
    +        provider: String(input.llm.model.provider),
    +        model: String(input.llm.model.id),
    +        caller: input.caller ?? 'unknown',
    +      });
    +    } catch {
    +      // Metrics must not change or mask the product-path outcome.
    +    }
    +  }
    +
    +  /**
    +   * Re-publish a terminal failure that was held back while the automatic retry
    +   * budget was still available. The client must always end up seeing the original
    +   * error, never a synthesized "gave up" message.
    +   */
    +  private async releaseHeldFailure(
    +    input: LocalRuntimeTurnRunnerInput,
    +    autoRetry: TurnAutoRetryCoordinator,
    +  ): Promise<void> {
    +    const held = autoRetry.release();
    +    if (!held) return;
    +    try {
    +      await input.eventWriter.pushRuntime(held);
    +    } catch (error) {
    +      this.options.logger?.warn?.(
    +        { error, sessionId: input.sessionId, turnId: input.turnId },
    +        'Failed to release the held turn failure after automatic retries stopped.',
    +      );
    +    }
    +  }
    +
       private createRunState(input: LocalRuntimeTurnRunnerInput): MutableRunnerState {
         const state = createMutableRunnerState(input);
         state.inputReview = this.reviewInput(
    @@ -175,14 +256,15 @@ export class LocalAgentTurnRunner implements LocalRuntimeTurnRunnerPort {
         readonly toolContext: LocalRuntimeTurnToolContext;
         readonly observedLlm: LLMModelConfig;
         readonly state: MutableRunnerState;
    -    readonly observation: { regenerations: number };
    +    readonly observation: { regenerations: number; autoRetries: number };
    +    readonly autoRetry: TurnAutoRetryCoordinator;
       }): Promise<void> {
    -    const { input, toolContext, observedLlm, state, observation } = scope;
    +    const { input, toolContext, observedLlm, state, observation, autoRetry } = scope;
         let regenerationCount = 0;
         let attemptIndex = 0;
         while (regenerationCount <= this.outputSafetyMaxRegenerations) {
           const safetyAbort = new AbortController();
    -      const safetyWriter = this.createSafetyWriter(input, state, safetyAbort);
    +      const safetyWriter = this.createSafetyWriter(input, state, safetyAbort, autoRetry);
           state.activeWriter = safetyWriter;
           if (state.inputRejected) safetyWriter.block();
           state.finalAttemptEventStart = (input.eventWriter.outcome?.eventCount ?? input.eventWriter.events.length);
    @@ -217,7 +299,13 @@ export class LocalAgentTurnRunner implements LocalRuntimeTurnRunnerPort {
             regenerationCount,
             attemptIndex,
           });
    -      if (decision === 'break') return;
    +      if (decision === 'break') {
    +        if (await this.runAutomaticRetry(input, state, observation, autoRetry)) {
    +          attemptIndex += 1;
    +          continue;
    +        }
    +        return;
    +      }
           attemptIndex += 1;
           if (decision === 'continue') {
             regenerationCount += 1;
    @@ -226,32 +314,93 @@ export class LocalAgentTurnRunner implements LocalRuntimeTurnRunnerPort {
         }
       }
     
    +  /**
    +   * Re-run a turn whose retryable terminal failure was held back by the gate.
    +   * Returns true when another attempt was started; false leaves the held event
    +   * to be released by the caller so the original error still reaches the user.
    +   */
    +  private async runAutomaticRetry(
    +    input: LocalRuntimeTurnRunnerInput,
    +    state: MutableRunnerState,
    +    observation: { regenerations: number; autoRetries: number },
    +    autoRetry: TurnAutoRetryCoordinator,
    +  ): Promise<boolean> {
    +    const logStop = (reason: string): false => {
    +      this.options.logger?.info?.(
    +        {
    +          sessionId: input.sessionId,
    +          turnId: input.turnId,
    +          reason,
    +          retries: autoRetry.retries,
    +          maxRetries: autoRetry.policy.maxRetries,
    +        },
    +        'Automatic turn retry stopped; surfacing the original turn failure.',
    +      );
    +      return false;
    +    };
    +    if (!autoRetry.hasHeld()) {
    +      // The hold was refused outright. Without this the most common give-up —
    +      // output already streamed to the user — would be invisible.
    +      if (state.lastGiveUpReason) return logStop(state.lastGiveUpReason);
    +      return false;
    +    }
    +    const next = autoRetry.nextAttempt();
    +    if (!next.retry) return logStop(next.reason);
    +    try {
    +      await this.turnAutoRetrySleep(next.delayMs, input.signal);
    +    } catch {
    +      // Cancelled during backoff: never start another attempt. The hold is kept
    +      // so `releaseHeldFailure` still surfaces the original terminal error.
    +      return logStop('aborted');
    +    }
    +    if (input.signal?.aborted) return logStop('aborted');
    +    // Spend the hold only now that the attempt is really about to run.
    +    autoRetry.markAttemptStarted();
    +    observation.autoRetries += 1;
    +    this.options.logger?.info?.(
    +      {
    +        sessionId: input.sessionId,
    +        turnId: input.turnId,
    +        retryReason: next.retryReason,
    +        retryAttempt: autoRetry.retries,
    +        maxRetries: autoRetry.policy.maxRetries,
    +        delayMs: next.delayMs,
    +      },
    +      'Retrying a turn after a retryable failure.',
    +    );
    +    return true;
    +  }
    +
       private createSafetyWriter(
         input: LocalRuntimeTurnRunnerInput,
         state: MutableRunnerState,
         safetyAbort: AbortController,
    +    autoRetry: TurnAutoRetryCoordinator,
       ): LocalOutputSafetyEventWriter {
    -    return new LocalOutputSafetyEventWriter(gateTerminalOnInputReview(input, state), {
    -      sessionId: input.sessionId,
    -      turnId: input.turnId,
    -      ...(input.streamUnreviewedOutput === true && input.contentReviewRequired === false
    -        ? { reviewRequired: false, checkText: async () => ({ pass: true as const }) }
    -        : {
    -            checkText:
    -              input.contentReviewRequired === false
    -                ? async () => ({ pass: true as const })
    -                : this.options.reviewContent,
    -          }),
    -      onBlocked: () => safetyAbort.abort(state.inputRejected ? 'input_safety' : 'output_safety'),
    -      ...(this.options.outputSafetyRetryDelay
    -        ? { retryDelay: this.options.outputSafetyRetryDelay }
    -        : {}),
    -      ...(this.options.metricsClient ? { metricsClient: this.options.metricsClient } : {}),
    -      ...(input.contentReviewRequired === true ? { authErrorVariant: 'auth' as const } : {}),
    -      ...(input.persistApprovedPartialOnNetworkStop === false
    -        ? { persistApprovedPartialOnNetworkStop: false }
    -        : {}),
    -    });
    +    return new LocalOutputSafetyEventWriter(
    +      gateTerminalOnInputReview(input, state, autoRetry),
    +      {
    +        sessionId: input.sessionId,
    +        turnId: input.turnId,
    +        ...(input.streamUnreviewedOutput === true && input.contentReviewRequired === false
    +          ? { reviewRequired: false, checkText: async () => ({ pass: true as const }) }
    +          : {
    +              checkText:
    +                input.contentReviewRequired === false
    +                  ? async () => ({ pass: true as const })
    +                  : this.options.reviewContent,
    +            }),
    +        onBlocked: () => safetyAbort.abort(state.inputRejected ? 'input_safety' : 'output_safety'),
    +        ...(this.options.outputSafetyRetryDelay
    +          ? { retryDelay: this.options.outputSafetyRetryDelay }
    +          : {}),
    +        ...(this.options.metricsClient ? { metricsClient: this.options.metricsClient } : {}),
    +        ...(input.contentReviewRequired === true ? { authErrorVariant: 'auth' as const } : {}),
    +        ...(input.persistApprovedPartialOnNetworkStop === false
    +          ? { persistApprovedPartialOnNetworkStop: false }
    +          : {}),
    +      },
    +    );
       }
     
       private async runPiAttempt(scope: {
    @@ -464,6 +613,10 @@ interface MutableRunnerState {
       retractionVariant?: LocalRuntimeTurnRunnerResult['retractionVariant'];
       retractionSource?: 'input-review' | 'output-review';
       finalAttemptEventStart: number;
    +  /** True once any non-terminal event from this turn reached the client. */
    +  streamedToClient: boolean;
    +  /** Why the most recent hold was refused, for observability. */
    +  lastGiveUpReason?: TurnAutoRetryStopReason;
       readonly eventIdGenerator: NonNullable<LocalRuntimeTurnRunnerInput['eventIdGenerator']>;
       readonly runtimeSeqGenerator: NonNullable<LocalRuntimeTurnRunnerInput['runtimeSeqGenerator']>;
     }
    @@ -480,6 +633,7 @@ function createMutableRunnerState(input: LocalRuntimeTurnRunnerInput): MutableRu
         networkStopped: false,
         inputSafetyReviewUnknown: false,
         finalAttemptEventStart: (input.eventWriter.outcome?.eventCount ?? input.eventWriter.events.length),
    +    streamedToClient: false,
         eventIdGenerator:
           input.eventIdGenerator ??
           ((kind: string) => `evt_${input.turnId}_${kind}_${++eventIdentitySequence}`),
    @@ -507,25 +661,59 @@ function applyInputCorrection(state: MutableRunnerState, result?: SafetyCheckRes
     function gateTerminalOnInputReview(
       input: LocalRuntimeTurnRunnerInput,
       state: MutableRunnerState,
    +  autoRetry: TurnAutoRetryCoordinator,
     ): PiEventWriter {
       const suppressTerminal = async (): Promise<boolean> =>
         (await waitForInputReview(state.inputReview, input.signal)) ||
         state.inputCorrectionPending ||
         input.signal?.aborted === true;
    +  /**
    +   * A retryable terminal failure is held back instead of being surfaced, so the
    +   * attempt loop can re-run the turn. `hasHeld()` keeps only the first failure:
    +   * a later attempt's failure is held again only after that one was consumed.
    +   */
    +  const holdsForRetry = async (event: IRuntimeEvent): Promise<boolean> => {
    +    if (!isTerminalStatus(event)) return false;
    +    if (autoRetry.hasHeld()) return false;
    +    const held = autoRetry.hold(event, deriveLocalTurnRuntimeOutcome([event]));
    +    if (!held.held && held.reason === 'output_already_released') {
    +      state.lastGiveUpReason = held.reason;
    +    }
    +    return held.held;
    +  };
    +  /** Record that an event reached the client, so a retry cannot duplicate it. */
    +  const forward = async (event: IRuntimeEvent): Promise<void> => {
    +    if (!isTerminalStatus(event)) state.streamedToClient = true;
    +    await input.eventWriter.pushRuntime(event);
    +  };
    +  const forwardAll = async (events: readonly IRuntimeEvent[]): Promise<void> => {
    +    if (events.some((event) => !isTerminalStatus(event))) state.streamedToClient = true;
    +    await input.eventWriter.appendEvents([...events]);
    +  };
       return {
         pushRuntime: async (event) => {
    +      if (isTerminalStatus(event) && (await holdsForRetry(event))) return;
           if (isTerminalStatus(event) && (await suppressTerminal())) return;
    -      await input.eventWriter.pushRuntime(event);
    +      await forward(event);
         },
         appendEvents: async (events) => {
           if (!events.some(isTerminalStatus)) {
    -        await input.eventWriter.appendEvents(events);
    +        await forwardAll(events);
             return;
           }
    -      const accepted = (await suppressTerminal())
    -        ? events.filter((event) => !isTerminalStatus(event))
    -        : events;
    -      if (accepted.length > 0) await input.eventWriter.appendEvents(accepted);
    +      const accepted: IRuntimeEvent[] = [];
    +      let heldTerminal = false;
    +      for (const event of events) {
    +        if (isTerminalStatus(event) && !heldTerminal && (await holdsForRetry(event))) {
    +          heldTerminal = true;
    +          continue;
    +        }
    +        accepted.push(event);
    +      }
    +      const visible = (await suppressTerminal())
    +        ? accepted.filter((event) => !isTerminalStatus(event))
    +        : accepted;
    +      if (visible.length > 0) await forwardAll(visible);
         },
       };
     }
    @@ -540,6 +728,26 @@ function isTerminalStatus(event: Parameters<PiEventWriter['pushRuntime']>[0]): b
       );
     }
     
    +/**
    + * True once anything from this turn has been streamed to the client.
    + *
    + * The gate is the only place that observes every event on its way out, so
    + * counting here covers both writer modes. The safety writer's approved-partial
    + * record is not enough on its own: with unreviewed streaming
    + * (`streamUnreviewedOutput`) the writer forwards events through a fast path
    + * that never populates it, so a writer-only check would report "nothing shown"
    + * while the user is watching the answer arrive. A retry re-runs the whole tool
    + * loop, so any already-streamed event — including tool results — must block it.
    + */
    +function attemptReleasedOutput(state: MutableRunnerState): boolean {
    +  if (state.streamedToClient) return true;
    +  const writer = state.activeWriter;
    +  if (!writer) return false;
    +  const partial = writer.getApprovedPartial();
    +  if (partial.content.trim() || partial.thinking.trim()) return true;
    +  return writer.getAttemptMessageIds().length > 0;
    +}
    +
     async function waitForInputReview(
       review: Promise<boolean>,
       signal?: AbortSignal,
    diff --git a/packages/local-runtime-v2/src/service/turn-system/agent-host/runner/turn-auto-retry.ts b/packages/local-runtime-v2/src/service/turn-system/agent-host/runner/turn-auto-retry.ts
    new file mode 100644
    index 0000000..db9a420
    --- /dev/null
    +++ b/packages/local-runtime-v2/src/service/turn-system/agent-host/runner/turn-auto-retry.ts
    @@ -0,0 +1,249 @@
    +import {
    +  LLM_ERROR_STATUS_CODES,
    +  normalizeLLMError,
    +  toLLMRetryDecision,
    +  type LLMRetryReason,
    +} from '@mavis/shared/llm-error-classifier';
    +import type { IRuntimeEvent } from '@mavis/protocol';
    +
    +import type { LocalRuntimeTurnRuntimeOutcome } from '../execution/executor.js';
    +
    +/**
    + * Turn-level automatic retry for transient turn failures.
    + *
    + * The framework already retries a single provider request (`withLLMRetry` in
    + * `@mavis/agent-core/pi-turn-runner`), but that budget is spent *inside* one
    + * logical LLM call and is deliberately abandoned once output commits. Nothing
    + * re-runs a whole turn, so a turn that ends on a transient classification
    + * (overload, rate limit, dropped connection) terminates and waits for a manual
    + * user retry. This module owns the turn-level budget.
    + *
    + * The retryable classification is deliberately not re-derived here: the shared
    + * error classifier already encodes which conditions a resend can fix, and quota
    + * exhaustion must stay terminal (see #108, where an unconditional retry loop
    + * produced ~80 usage-limit errors).
    + */
    +export interface TurnAutoRetryPolicy {
    +  /** Automatic re-runs allowed after the first failed attempt. */
    +  readonly maxRetries: number;
    +  readonly baseDelayMs: number;
    +  readonly maxDelayMs: number;
    +  /** Total wall-clock budget for the whole retry sequence of one turn. */
    +  readonly maxElapsedMs: number;
    +}
    +
    +export const DEFAULT_TURN_AUTO_RETRY_POLICY: Readonly<TurnAutoRetryPolicy> = {
    +  maxRetries: 10,
    +  baseDelayMs: 1_000,
    +  maxDelayMs: 30_000,
    +  maxElapsedMs: 600_000,
    +};
    +
    +/** Why the coordinator refused to hold a failure, or why it stopped retrying. */
    +export type TurnAutoRetryStopReason =
    +  | 'disabled'
    +  | 'exhausted'
    +  | 'elapsed_budget_exceeded'
    +  | 'output_already_released'
    +  | 'not_retryable'
    +  | 'aborted';
    +
    +export type TurnAutoRetryDecision =
    +  | { readonly retry: false; readonly reason: TurnAutoRetryStopReason }
    +  | { readonly retry: true; readonly retryReason: LLMRetryReason; readonly delayMs: number };
    +
    +export type TurnAutoRetryHoldResult =
    +  | { readonly held: true }
    +  | { readonly held: false; readonly reason: TurnAutoRetryStopReason };
    +
    +export interface TurnAutoRetryClassification {
    +  readonly retryable: boolean;
    +  readonly reason?: LLMRetryReason;
    +}
    +
    +/**
    + * MiniMax business codes that name a specific transient condition and can
    + * therefore be trusted even when the message text does not classify.
    + *
    + * `LLM_UPSTREAM_ERROR` (50113) is deliberately absent. It is the generic
    + * 4xx/5xx fallback bucket — `mapUpstreamHttpError` assigns it to any status of
    + * 400 or above except 402/401/403/429/529 — so treating it as retryable would
    + * resend requests the provider has already rejected as malformed, and would
    + * turn an HTTP 400 or 404 into ten identical attempts. The shared classifier
    + * fails closed on that code for the same reason, and this list must not reopen
    + * it.
    + */
    +const TRANSIENT_PROTOCOL_CODES = new Map<number, LLMRetryReason>([
    +  [LLM_ERROR_STATUS_CODES.LLM_RATE_LIMITED, 'rate_limited'],
    +  [LLM_ERROR_STATUS_CODES.LLM_TPM_RATE_LIMITED, 'tpm_rate_limited'],
    +  [LLM_ERROR_STATUS_CODES.LLM_CLUSTER_OVERLOADED, 'overloaded'],
    +]);
    +
    +/**
    + * Classify a failed turn outcome.
    + *
    + * The shared classifier is the only authority on what a resend can fix. The
    + * narrow protocol-code table above only adds cases where the transport had no
    + * status to classify from; it never widens a refusal the classifier made.
    + */
    +export function classifyTurnAutoRetryableFailure(
    +  outcome: LocalRuntimeTurnRuntimeOutcome,
    +): TurnAutoRetryClassification {
    +  if (outcome.status !== 'failed') return { retryable: false };
    +  const decision = toLLMRetryDecision(
    +    normalizeLLMError(outcome.errorMessage ? { errorMessage: outcome.errorMessage } : {}),
    +  );
    +  if (decision.retryable) return { retryable: true, reason: decision.reason };
    +  // Only consulted after the classifier refused, and only for codes that name a
    +  // specific transient condition. Usage limit (42212), credits (50110) and auth
    +  // (50112) are not members.
    +  const byCode = outcome.errorCode === undefined ? undefined : TRANSIENT_PROTOCOL_CODES.get(outcome.errorCode);
    +  return byCode ? { retryable: true, reason: byCode } : { retryable: false };
    +}
    +
    +export function resolveTurnAutoRetryPolicy(
    +  policy: Partial<TurnAutoRetryPolicy> | undefined,
    +): TurnAutoRetryPolicy {
    +  return { ...DEFAULT_TURN_AUTO_RETRY_POLICY, ...(policy ?? {}) };
    +}
    +
    +/** Full-jitter bounded exponential backoff, matching the transport retry shape. */
    +export function turnAutoRetryDelayMs(
    +  retryAttempt: number,
    +  policy: TurnAutoRetryPolicy,
    +  random: () => number = Math.random,
    +): number {
    +  const exponential = Math.min(policy.maxDelayMs, policy.baseDelayMs * 2 ** (retryAttempt - 1));
    +  return Math.floor(exponential / 2 + (clampRandom(random()) * exponential) / 2);
    +}
    +
    +function clampRandom(value: number): number {
    +  return Number.isFinite(value) ? Math.max(0, Math.min(1, value)) : 0;
    +}
    +
    +export interface TurnAutoRetryCoordinator {
    +  readonly policy: TurnAutoRetryPolicy;
    +  /** Automatic retries actually started for this turn. */
    +  readonly retries: number;
    +  /** Hold a retryable terminal failure instead of surfacing it to the client. */
    +  hold(event: IRuntimeEvent, outcome: LocalRuntimeTurnRuntimeOutcome): TurnAutoRetryHoldResult;
    +  /**
    +   * Decide whether the held failure gets another attempt, or why it stops. This
    +   * is a pure query: it never consumes the hold, so a cancelled backoff still
    +   * leaves the original terminal event available for {@link release}.
    +   */
    +  nextAttempt(): TurnAutoRetryDecision;
    +  /**
    +   * Called only once an attempt has actually started. Consumes the hold and
    +   * spends one retry from the budget.
    +   */
    +  markAttemptStarted(): void;
    +  /** Hand back the held terminal event so the client receives the original error. */
    +  release(): IRuntimeEvent | undefined;
    +  /** Whether a failure is currently being held back. */
    +  hasHeld(): boolean;
    +}
    +
    +export interface TurnAutoRetryCoordinatorDeps {
    +  readonly nowMs?: () => number;
    +  readonly random?: () => number;
    +  /** True once this attempt released user-visible output that a retry would duplicate. */
    +  readonly hasReleasedOutput: () => boolean;
    +  readonly isAborted: () => boolean;
    +}
    +
    +/**
    + * Per-turn coordinator shared by the terminal-event gate and the attempt loop.
    + *
    + * The gate asks whether a retryable terminal failure may be held back; the loop
    + * asks for the next delay; whoever stops releases the held event so the user
    + * still sees the original terminal error.
    + */
    +export function createTurnAutoRetryCoordinator(
    +  policy: Partial<TurnAutoRetryPolicy> | undefined,
    +  deps: TurnAutoRetryCoordinatorDeps,
    +): TurnAutoRetryCoordinator {
    +  const resolved = resolveTurnAutoRetryPolicy(policy);
    +  const nowMs = deps.nowMs ?? Date.now;
    +  const random = deps.random ?? Math.random;
    +  let heldEvent: IRuntimeEvent | undefined;
    +  let heldOutcome: LocalRuntimeTurnRuntimeOutcome | undefined;
    +  let retries = 0;
    +  let startedAtMs: number | undefined;
    +
    +  const stop = (reason: TurnAutoRetryStopReason): TurnAutoRetryHoldResult => ({
    +    held: false,
    +    reason,
    +  });
    +
    +  return {
    +    policy: resolved,
    +    get retries() {
    +      return retries;
    +    },
    +    hasHeld: () => heldEvent !== undefined,
    +    hold(event, outcome) {
    +      if (heldEvent !== undefined) return stop('disabled');
    +      if (resolved.maxRetries <= 0) return stop('disabled');
    +      if (deps.isAborted()) return stop('aborted');
    +      // Re-running a turn after its output was shown would duplicate visible
    +      // content, so those failures keep today's terminal behaviour.
    +      if (deps.hasReleasedOutput()) return stop('output_already_released');
    +      if (!classifyTurnAutoRetryableFailure(outcome).retryable) return stop('not_retryable');
    +      heldEvent = event;
    +      heldOutcome = outcome;
    +      startedAtMs ??= nowMs();
    +      return { held: true };
    +    },
    +    nextAttempt() {
    +      if (heldEvent === undefined || heldOutcome === undefined) {
    +        return { retry: false, reason: 'not_retryable' };
    +      }
    +      if (deps.isAborted()) return { retry: false, reason: 'aborted' };
    +      if (deps.hasReleasedOutput()) return { retry: false, reason: 'output_already_released' };
    +      const classified = classifyTurnAutoRetryableFailure(heldOutcome);
    +      if (!classified.retryable) return { retry: false, reason: 'not_retryable' };
    +      if (retries >= resolved.maxRetries) return { retry: false, reason: 'exhausted' };
    +      const delayMs = turnAutoRetryDelayMs(retries + 1, resolved, random);
    +      if (nowMs() + delayMs - (startedAtMs ?? nowMs()) > resolved.maxElapsedMs) {
    +        return { retry: false, reason: 'elapsed_budget_exceeded' };
    +      }
    +      return { retry: true, retryReason: classified.reason ?? 'network', delayMs };
    +    },
    +    markAttemptStarted() {
    +      retries += 1;
    +      // The held failure is only discarded now that another attempt is really
    +      // running; a cancelled backoff leaves it to be released.
    +      heldEvent = undefined;
    +      heldOutcome = undefined;
    +    },
    +    release() {
    +      const held = heldEvent;
    +      heldEvent = undefined;
    +      heldOutcome = undefined;
    +      return held;
    +    },
    +  };
    +}
    +
    +/** Abortable backoff sleep used between automatic turn retries. */
    +export function turnAutoRetrySleep(delayMs: number, signal?: AbortSignal): Promise<void> {
    +  if (signal?.aborted) return Promise.reject(abortReason(signal));
    +  if (delayMs <= 0) return Promise.resolve();
    +  return new Promise((resolve, reject) => {
    +    const onAbort = () => {
    +      clearTimeout(timer);
    +      signal?.removeEventListener('abort', onAbort);
    +      if (signal) reject(abortReason(signal));
    +    };
    +    const timer = setTimeout(() => {
    +      signal?.removeEventListener('abort', onAbort);
    +      resolve();
    +    }, delayMs);
    +    signal?.addEventListener('abort', onAbort, { once: true });
    +  });
    +}
    +
    +function abortReason(signal: AbortSignal): Error {
    +  return signal.reason instanceof Error ? signal.reason : new Error('Turn auto-retry cancelled');
    +}
    diff --git a/release/public-source.json b/release/public-source.json
    index 0d4938e..9932adb 100644
    --- a/release/public-source.json
    +++ b/release/public-source.json
    @@ -1404,6 +1404,7 @@
         "packages/local-runtime-v2/src/service/turn-system/agent-host/runner/eval-reporter.ts",
         "packages/local-runtime-v2/src/service/turn-system/agent-host/runner/execution-budget-reminder.ts",
         "packages/local-runtime-v2/src/service/turn-system/agent-host/runner/index.ts",
    +    "packages/local-runtime-v2/src/service/turn-system/agent-host/runner/local-agent-turn-runner-auto-retry.test.ts",
         "packages/local-runtime-v2/src/service/turn-system/agent-host/runner/local-agent-turn-runner.ts",
         "packages/local-runtime-v2/src/service/turn-system/agent-host/runner/local-cu-screenshot-pruner.ts",
         "packages/local-runtime-v2/src/service/turn-system/agent-host/runner/output-safety-event-writer.ts",
    @@ -1412,6 +1413,8 @@
         "packages/local-runtime-v2/src/service/turn-system/agent-host/runner/policy/process-local-tool-result-policy.ts",
         "packages/local-runtime-v2/src/service/turn-system/agent-host/runner/policy/side-session-approval.ts",
         "packages/local-runtime-v2/src/service/turn-system/agent-host/runner/safety-replacement.ts",
    +    "packages/local-runtime-v2/src/service/turn-system/agent-host/runner/turn-auto-retry.test.ts",
    +    "packages/local-runtime-v2/src/service/turn-system/agent-host/runner/turn-auto-retry.ts",
         "packages/local-runtime-v2/src/service/turn-system/agent-host/runner/turn-close-error.ts",
         "packages/local-runtime-v2/src/service/turn-system/agent-host/runner/turn-outcome.ts",
         "packages/local-runtime-v2/src/service/turn-system/agent-host/tools/codex-post-tool-response.ts",

    What the audit changed

    An independent reviewer ran the implementation against the acceptance criteria in this issue and measured two blocking defects. Both were reproduced before fixing, and both now have regression tests that were confirmed to fail against the previous behaviour.

    1. A rejected request would have been resent ten times. My first version fell back to isProductionRuntimeErrorRetryable after the shared classifier had already refused. That allowlist contains LLM_UPSTREAM_ERROR (50113), which is the catch-all bucket mapUpstreamHttpError assigns to any status ≥ 400 except 402/401/403/429/529. So an HTTP 400 or 404 — both on the "must stay terminal" list in this issue — became eleven attempts. It also contradicted the "reuse the classifier rather than re-derive a list" requirement. Measured before the fix: {bad400: 11, notFound: 11, auth: 1, quota: 1, credits: 1}.

    Replaced with a table of only the codes that name a specific transient condition (rate limited, TPM rate limited, cluster overloaded). This also fixes a reporting lie: the fallback labelled every code overloaded in the logs.

    2. Output could be streamed to the user twice. The "do not duplicate visible output" guard read the safety writer's approved-partial record. With streamUnreviewedOutput the writer forwards events through a fast path that never populates it, so the guard reported "nothing shown" while the answer was streaming. turn-execution-policy.ts turns that on for every BYOK turn when tuiProductPolicy is set, so this was the production default. Measured before the fix: calls: 3, assistant chunks ["THE ANSWER TEXT","THE ANSWER TEXT"].

    The gate now records that any non-terminal event reached the client, which covers both writer modes. It is deliberately conservative: any streamed event, including tool results, blocks the retry, because the retry re-runs the whole tool loop.

    Two smaller fixes came out of the same pass: a held failure is now released from a finally block (it was dropped if an attempt threw while one was still held), and a retry is counted and logged only once it actually starts, so a cancelled backoff no longer reports a retry that never ran.

    Verification

    pnpm typecheck and pnpm check:source pass. 30 tests across the two new files pass, including the two regression tests.

    Full pnpm test:capabilities: 3 failures, all pre-existing. The same three fail on a clean main checkout — update-runtime.test.ts (Homebrew libnode dylib), skill-directory-symlinks.test.ts and tui/theme/runtime.test.ts (both filesystem-watcher timing, they flip between runs). local-native-tools.test.ts also flakes under parallel load but passes 3/3 in isolation. None import the turn runner.

    NOT RUN: full pnpm verify, the Windows profile, the perf:full profile, and any live-provider or desktop acceptance.

    Known limitation worth deciding on

    The auto-retry path does not call recallAttempt, so a failed attempt's already-committed assistant messages and tool calls stay in canonical history while the re-run appends a second pass. The output guard blocks retries once anything is streamed, which covers the visible case, but a turn that failed before streaming anything can still leave committed tool side effects in history. Whether that needs retracting is a design question I would rather the maintainers answer than guess at.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions