Greptile Summary

This PR fixes two bugs in the model fallback retry path: (1) the original user prompt is now forwarded verbatim to the fallback model instead of being replaced with a hardcoded continuation string, and (2) images are only stripped when the previous failure reason is "format" (indicating the prior model couldn't handle the modality) rather than on every fallback retry.

Key changes:

• resolveFallbackRetryPrompt is deleted; params.body is forwarded as-is — correct because fallback retries only fire on thrown errors, not partial streams.
• New resolveRetryImages helper gates image stripping on previousFailureReason === "format", preserving images on rate_limit, timeout, auth, and unknown failures.
• ModelFallbackRunOptions gains previousFailureReason?: FailoverReason; the fallback loop classifies each thrown error and injects the reason into the next attempt's options.
• _testInternals export exposes resolveRetryImages for direct unit testing.

Notable observations:

• "format" errors are not exclusively image-related (they can also stem from unsupported tool schemas, parameter shape mismatches, etc.). When a non-image format error triggers the branch, images will be stripped for the fallback model even if it supports them and even though the root cause is unrelated — this is a known limitation acknowledged in the PR description.
• The most impactful behavioral change (prompt body forwarding) has no dedicated regression test; only the image-handling helper is directly tested.

Confidence Score: 4/5

• Safe to merge — the core logic is correct and well-tested; the two noted concerns are pre-existing edge cases or missing-test gaps, not regressions.
• The prompt-preservation fix is clearly correct (fallback retries only fire on thrown errors, so the original body is always safe to re-send). The failure-mode-aware image handling is a strict improvement over unconditional stripping. The previousFailureReason plumbing is well-tested with 10+ updated call-site assertions plus a dedicated describe block. Score is 4 rather than 5 because: (a) the "format" → strip-images heuristic can fire on non-image format errors and silently discard valid visual context on the fallback model, and (b) there is no explicit test asserting that the fallback call receives the original body verbatim, leaving the most behaviorally significant change without a regression guard.
• src/commands/agent.ts — the resolveRetryImages format-error heuristic and the absence of a prompt-forwarding regression test are the areas most worth a second look before merge.

_{Last reviewed commit: 2935806}

🔒 Aisle Security Analysis

We found 2 potential security issue(s) in this PR:

1. 🟠 User-supplied images are forwarded on fallback retries (including cross-provider), enabling unintended third-party disclosure

Description

The agent fallback retry logic no longer strips user-supplied images except for format failures.

• resolveRetryImages() returns the original images for fallback retries unless previousFailureReason === "format".
• runWithModelFallback() can retry with a different provider/model based on configured fallbacks.
• As a result, if the first provider fails due to rate_limit, timeout, auth, or unknown, the same user image payloads can be sent to a different external provider on the next attempt.

This is a privacy/data-exfiltration regression vs. the prior behavior (where images were stripped on any fallback retry) and may violate user expectations/consent boundaries when fallbacks cross providers.

Vulnerable code:

function resolveRetryImages(images, isFallbackRetry, previousFailureReason?) {
  if (isFallbackRetry && previousFailureReason === "format") {
    return undefined;
  }
  return images;
}

Recommendation

Treat image forwarding on fallback as a privacy-sensitive operation.

Mitigations (prefer safest default):

1. Default to stripping images on any fallback retry, unless explicitly opted-in.
2. If images must be preserved, only forward images when staying on the same provider (or same configured “trust zone”).
3. Add an explicit config flag (opt-in) such as agents.defaults.model.forwardImagesOnCrossProviderFallback: true.

Example safer implementation (strip on cross-provider retries):

function resolveRetryImages(
  images: AgentCommandOpts["images"],
  isFallbackRetry: boolean,
  previousFailureReason: FailoverReason | undefined,
  prevProvider: string | undefined,
  nextProvider: string | undefined,
): AgentCommandOpts["images"] {
  if (!isFallbackRetry) return images;

  const isCrossProvider = Boolean(prevProvider && nextProvider && prevProvider !== nextProvider);

  ​// Always strip on cross-provider retry unless explicit opt-in.
  if (isCrossProvider) return undefined;

  ​// Optionally still strip on format.
  if (previousFailureReason === "format") return undefined;

  return images;
}

Additionally, document this behavior prominently so users understand that enabling cross-provider fallbacks can cause attachments to be shared with other providers.

2. 🟡 Model fallback retries resend original prompt, risking duplicate non-idempotent side effects

Description

runAgentAttempt() now always reuses the original user prompt (params.body) when a model fallback retry occurs.

This is risky because fallback retries are triggered by thrown errors (via runWithModelFallback()), and the embedded runner can throw after partial progress within the turn, including after tool calls or other side effects:

• runEmbeddedPiAgent() can throw FailoverError to trigger model fallback on assistant-side failures/timeouts when fallbacks are configured (e.g. timeout aborts, later-call rate limits, etc.).
• Those errors can happen after the agent has already executed tool calls (e.g. messaging send, cron add, filesystem/shell tools), so repeating the full original instruction can cause the fallback attempt to repeat the same side effects.
• The referenced “orphaned-user-message repair” in attempt.ts only removes a trailing orphaned user message to maintain role ordering; it does not provide idempotency for tool executions or external side effects.

Security/impact scenarios:

• Duplicate outbound messages/notifications (spam or sensitive info sent twice)
• Duplicate external API calls/actions triggered by tools (potential cost, integrity issues)
• Repeating destructive operations requested by the prompt (e.g., deleting/modifying files, re-running shell commands)

Vulnerable code:

​// Fallback retries only fire on thrown errors (not partial streaming results),
​// so the original prompt is always safe to re-send. The orphaned-user-message
​// repair in attempt.ts handles any residual duplicate user turns.
const effectivePrompt = params.body;

Recommendation

Treat fallback retries as potentially non-idempotent.

Recommended mitigations (choose one or combine):

1. Reintroduce a “resume” prompt on fallback retries (or include a strict no-repeat instruction):

const effectivePrompt = params.isFallbackRetry
  ? "Continue where you left off. Do NOT repeat any actions already completed; inspect the transcript/tool results first."
  : params.body;

2. Make the retry conditional: only resend the original prompt when you can guarantee no side effects occurred (e.g., failures that happen before any model output/tool calls). Otherwise, use a resume prompt.

3. Add idempotency/deduplication at the tool layer (preferred for strong guarantees):

• Include an idempotency key (e.g., runId + tool call id) in tool executions
• Have tools reject or no-op repeated calls with the same idempotency key

These changes reduce the chance that fallback attempts will duplicate external side effects when recovering from timeouts or mid-turn failures.

Analyzed PR: #43331 at commit 5e98007

_{Last updated on: 2026-03-11T19:18:03Z}

Re: Aisle Security Analysis

Both flagged issues are pre-existing architectural gaps in the fallback logic, not regressions introduced by this PR. That said, the security review surfaced real improvement opportunities worth tracking:

1. Cross-provider image forwarding (High)

Pre-existing: the old behavior stripped images as a compatibility hack, not a privacy boundary. Users who configure cross-provider fallbacks have already opted into sending their prompts (text included) to multiple providers. This PR doesn't change the trust model.

However, there's a real gap: users may not expect image data to follow text to a different provider. Filed #43481 to add provider-level trust boundaries for images/attachments on cross-provider fallback.

2. Non-idempotent prompt replay (Medium)

Pre-existing: the old "Continue where you left off" prompt also triggered a new model turn that could repeat side effects, just with less context about what was originally requested. This PR doesn't make it worse.

However, there's a real gap: fallback retries can fire after partial tool execution, and neither the old nor new behavior accounts for that. Filed #43482 to track detecting partial tool execution before deciding replay strategy.

Converting to draft — rethinking the approach based on reviewer feedback across this PR and its follow-ups (#43485, #43507).

The current approach works around the orphaned user message repair in attempt.ts by re-injecting params.body on every retry. Review feedback (human and bot) consistently pointed out that this creates seams: user-turn injection instead of system context, image stripping at the wrong layer, and prompt-detected images bypassing the guard.

Planning a unified PR that coordinates fallback retries with the session manager's existing retry/branching mechanisms instead of working around them. Issues #43481 and #43492 remain valid and will be addressed in the new approach.

Superseded by #44188 (unified fallback retry safety PR).