Skip to content

feat(core): recover sessions from rejected attachments - #53014

Draft
kitlangton wants to merge 1 commit into
v2from
media-rejection-recovery
Draft

kitlangton wants to merge 1 commit into
v2from
media-rejection-recovery

Conversation

@kitlangton

Copy link
Copy Markdown
Contributor

Why

A session became permanently unusable after the read tool stored a truncated PDF from an interrupted download as a successful tool result. Every later request resent it, and the provider rejected each one with HTTP 400 invalid_file ("The file you uploaded is badly formatted or corrupted"). Retrying, continuing, and compacting all resend the same history, so the only escape was abandoning the session. No validator can catch every bad file, and a valid file can still be rejected by a particular provider, so this adds a general, user-confirmed recovery path.

What Changes

Before: Error: The file you uploaded is badly formatted or corrupted…, with no way forward. Every retry or new prompt fails the same way.

After:

  1. Rejected. The request fails with a typed provider.media-rejected error that names the suspect attachments:
    Error: The model provider rejected an attachment: The file you uploaded is badly formatted or corrupted…
    Attachments the model has not accepted yet: /repo/report.pdf (application/pdf)
    Run /continue-without-attachments to stop sending them and continue.
    
  2. Confirm. /continue-without-attachments (also in the command palette while the latest failure is a rejection) lists exactly what will be excluded:
    Continue without these attachments?
    The model provider rejected an attachment. These attachments were added since its last
    accepted request and will no longer be sent to the model. Each is replaced by a note;
    your history keeps them.
    
    • /repo/report.pdf (application/pdf)
                                                   cancel  [Continue]
    
  3. Continue. The session resumes from the existing history without running any tool again. In that request and every later one, including compaction requests and after a restart, the excluded file is replaced in place by:
    [Attachment omitted: "/repo/report.pdf" (application/pdf). The model provider rejected a request
    that included it, so the user chose to continue without it. Its contents are not available;
    tell the user if you need them.]
    
    Other files and text in the same message or tool result are still sent. Stored message and tool-result content does not change.
sequenceDiagram
  participant TUI
  participant Server
  participant Runner
  participant Provider
  Runner->>Provider: request (tool result has report.pdf)
  Provider-->>Runner: 400 invalid_file
  Runner-->>TUI: Step failed: provider.media-rejected (names suspects)
  TUI->>Server: GET /session/:id/attachment/candidates
  Server-->>TUI: [report.pdf]
  TUI->>Server: POST /session/:id/attachment/exclude [ref]
  Server->>Server: publish session.attachments.excluded
  Server->>Runner: resume
  Runner->>Provider: request (report.pdf replaced by a note)
  Provider-->>Runner: 200
Loading

Classification

classifyProviderFailure adds an InvalidRequestError classification, media-rejected. It applies to client errors (4xx, or no status for stream errors) only, after the context-overflow, payload-size, and content-policy checks, so those keep their existing handling. Signals:

  • OpenAI-style codes: invalid_file, invalid_image, invalid_image_format, invalid_image_url, image_parse_error, plus their messages ("badly formatted or corrupted", "unsupported image", "Invalid image data.").
  • Anthropic and Bedrock: a block path such as messages.N.content.M.image|document.source…, plus "Could not process image", "does not appear to be a valid png image", and "The PDF specified was not valid".
  • Gemini: "Unable to process input image", "Provided image is not valid", and "The document has no pages".

InvalidRequest stays non-retryable, so nothing retries automatically.

The Anthropic and Gemini phrasings come from public docs and reported errors. The repo had no recorded fixtures for them, so all tests use synthetic payloads.

Attachment Exclusion

Attribution. A request the provider accepted contained every attachment before the assistant message it produced. So when model M rejects an attachment, the suspects are the attachments M has not accepted yet. That means media in tool results of M's latest accepted assistant message (one with output or usage) and media in any later message. In the incident, that is exactly the PDF the last successful step read. If M never accepted a request (a new session, or right after a model switch), every attachment in the active history is a suspect. The candidate endpoint and the TUI dialog list exactly the set that will be excluded, so nothing is guessed silently. A healthy file added in the same window is also a suspect, because a provider does not say which file failed. The API accepts any subset of refs if a client wants per-file choice.

Durable fact. One new durable event, session.attachments.excluded { attachments: [{ messageID, callID?, index }] }, names user files (files[index]) or tool-result files (content[index] of callID).

Projection. The projector records the excluded positions on the referenced item as read-model fields: User.excludedFiles and AssistantTool.excludedContent. Content is never touched. Because the marks live on the message data, forks copy them even though forks remap message IDs. Every history subset also carries them, including compaction's older slice and recentUserMessages. The Solid client store mirrors the event.

Filtering. toLLMMessages swaps each excluded media part or tool-result file for the note. Every model request is built through it: primary steps, summary and native compaction, generate, and prompt estimation. The existing replaceMedia, unsupportedParts, and boundImages seams in model-request.ts were not reused. They operate on lowered LLMRequest messages, where tool results no longer carry the session message ID or the content index, so they cannot target one specific attachment.

Action. POST /api/session/:id/attachment/exclude returns SessionBusyError while the session is running. It rejects refs that do not name media in the active history (InvalidRequestError), publishes the event, and resumes unless resume: false. GET /api/session/:id/attachment/candidates returns the suspects with mime and name. The client SDK was regenerated with bun run generate.

Demo

No TUI recording was made. The change adds a hint line under the failed assistant's error, a /continue-without-attachments command gated on the latest failure being a rejection, and a confirm dialog built with the existing DialogConfirm. I verified it only with the type checker, not by running the TUI against a rejecting provider. An opencode-drive before/after clip with a simulated invalid_file provider is still needed before this leaves draft.

Scope

This PR owns classification, the durable exclusion, request filtering, the API, and the TUI affordance. Follow-ups:

  • Web app: the app (packages/app) shows the descriptive error but has no button yet.
  • Exclusion markers: the transcript does not yet mark excluded files.
  • Compaction transcript text: the flattened text still says [Attached application/pdf: …] for excluded files. The payload itself is never sent.
  • Per-file picker: the dialog does not offer per-file choice when several suspects exist.
  • Unsupported-modality filter: candidates do not yet skip media that the model's capabilities already strip, which can over-list.
  • More provider codes: add codes as real fixtures are recorded.

Verification

cd packages/ai && bun test test/provider-error.test.ts          # 41 pass
cd packages/core && bun test test/session-attachment.test.ts    # 8 pass
cd packages/core && bun test test/session-runner.test.ts        # 224 pass
cd packages/core && bun run test                                # 5605 pass, 6 fail (provider-digitalocean OAuth listener; fails identically on origin/v2)
cd packages/ai && bun run test                                  # 1733 pass
cd packages/schema && bun test                                  # 65 pass
cd packages/server && bun run test                              # 68 pass
cd packages/client && bun run test                              # 175 pass, 4 fail (fail identically on origin/v2)
cd packages/tui && bun run test                                 # 1429 pass, 4 fail (app-lifecycle plugin-failure widths; fail identically on origin/v2)
bun typecheck   # schema, ai, core, protocol, server, client, tui: clean
bun run check   # exit 0

The new end-to-end runner scenario:

  1. A tool returns [text, corrupt report.pdf, healthy notes.pdf], and the next request fails with media-rejected.
  2. The error names both PDFs, and the candidates are exactly those two tool-result refs. Earlier history is not suspect.
  3. Excluding report.pdf and resuming sends [text, note, notes.pdf], with no tool re-execution.
  4. Stored tool content still holds the original PDF, read back from the projection as a restart would.
  5. A later manual compaction request carries the same filtered tool result.

Unit tests cover:

  • the attribution rules: the acceptance boundary, usage-only acceptance, model switches, and already-excluded files;
  • user-file exclusion with healthy files and text attachments kept;
  • error-state tool results;
  • fork inheritance;
  • rejection of unknown refs.

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant