Skip to content

fix(agent-core): cap images per request and recover sessions over the provider image limit - #431

Merged
hetaoBackend merged 2 commits into
mainfrom
fix/425-image-history-cap
Oct 5, 2026
Merged

hetaoBackend merged 2 commits into
mainfrom
fix/425-image-history-cap

Conversation

@hetaoBackend

@hetaoBackend hetaoBackend commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Change

Refs #425

A session that has accumulated more images than the provider accepts in one request (the reporter's gateway rejects Too many images in request: 31 > 30) becomes permanently stuck: every following request fails, including a plain text turn, /retry, /compact and a session restarted with -c. Each failure takes the full BYOK retry window (six requests, about 20 s), and the TUI says "The model provider is temporarily unavailable … Run /retry". Only /new escapes.

After this PR:

  • requests carry at most N images (default 20, configurable per model). Older images are replaced in the request copy by a short note naming the original file. Stored history is unchanged;
  • an image-count rejection, or an HTTP 400 invalid_request_error, fails once with the upstream reason and points to /compact or /new;
  • /compact on a session the checkpoint provider rejects for too many images falls back to the image-free candidate and recovers the session.

Root cause

  • Inline images are stored as base64 in the session history (messages.jsonl). Each request re-sends the whole provider-visible history through projectAgentMessagesForModel (packages/agent-core/src/pi-turn-runner/outbound-message-normalizer.ts). That function had a per-message attachment cap (max_attachments_count: 4) but no request-level image cap. In the repro, the image count rose 4, 8 … 30, 31, and every request after that point was rejected.
  • packages/agent-core/src/pi-turn-runner/llm-retry.ts (failureResult): BYOK providers retry every pre-output failure (the retry-all policy). Only aborts and safety refusals were exempt, so a deterministic 400 was sent six times.
  • packages/tui/src/tui/controller/runtime/runtime-error-presentation.ts: BYOK upstream failures arrive with code 50113 (LLM_UPSTREAM_ERROR). That code maps to "temporarily unavailable / Run /retry" and hides the upstream reason.
  • The compaction checkpoint request (packages/local-runtime-v2/src/service/turn-system/compaction/execution/checkpoint-provider.ts) carried the same images. A rejection was neither a typed overflow nor a success, so compactContext failed with "Checkpoint Provider request failed after one retry" and never reached the image-free candidate (Hvideo).

Changes

  1. Request-time image cap
    • New packages/agent-core/src/pi-turn-runner/request-image-limit.ts. limitRequestImages keeps the newest N image blocks across the whole request, counting both user messages and tool results. Each older image becomes [Earlier image omitted: this request keeps only the N most recent images. Original file: <path>].
      • The path comes from the user message's <attachment … path=… kind="image" inline="true"> reminders, used only when their count matches the image blocks.
      • For a tool result, the path comes from the tool call's path / file_path argument when the result holds a single image.
      • Otherwise the note carries no path.
      • Inputs are never mutated.
      • Attachment tags are parsed with a linear scan instead of a regex, because the message text is user-controlled. This addresses the CodeQL js/polynomial-redos alert on the first push; a test covers adversarial input.
    • projectAgentMessagesForModel applies the cap last, after video and undersized-image projection, so the count matches what is actually sent. The agent turn, the checkpoint request and footprint measurement all use this projection with the target model. agent.ts logs outbound_images_limited when images are omitted.
    • N comes from the new model capability max_images_per_request (IModelCapabilities in packages/protocol/src/runtime.ts and ModelCapabilitiesConfig in packages/config/src/config.ts). It reaches the resolved model through model-ref.ts and local-model-resolver.ts.
      • Without a declaration, N is DEFAULT_MAX_IMAGES_PER_REQUEST = 20.
      • api.mistral.ai (exact host) gets 8, the limit given in Mistral's vision FAQ.
    • Why 20: it sits below the smallest general-purpose limit seen in practice (30 at the [Bug]: Session remains permanently stuck after temporary provider failure during /goal #425 gateway), with headroom for gateways that count images differently. It is far below Anthropic (100 per request) and OpenAI (500). It still keeps the attachments of the last five turns (up to 4 per message) visible to the model.
    • docs/examples.md documents the behavior and the override.
  2. Terminal rejection and a clear error
    • packages/shared/src/llm-error-classifier.ts: new image_limit and invalid_request signals, plus three exports:
      • isLLMImageLimitMessage matches the common phrasings: "too many images", "maximum (number) of N images", "number of images … exceeds", "at most N images".
      • classifyLLMRequestRejectionMessage, for UI layers.
      • isLLMDeterministicRequestRejection is true for an image-limit signal (status unknown, 400, 413 or 422) or for HTTP 400 together with invalid_request_error. It is always false for aborts, timeouts, network errors and 408/429/5xx.
      • The raw text is also checked, because payload extraction drops a JSON type field.
    • llm-retry.ts: the BYOK retry-all branch now also excludes isLLMDeterministicRequestRejection. Those errors fall through to the normal decision, which returns non-retryable bad_request. The narrow scope is deliberate: retry-all exists because custom gateways report transient errors inconsistently. A plain 400 Bad Request without the error type is still retried six times, as are 401, 429 and 5xx.
    • runtime-error-presentation.ts: an image-limit or invalid-request failure shows "The model provider rejected the request: the conversation carries too many images" (or "…as invalid"). It adds a Reason: line with the upstream text (the BYOK prefix is stripped), the provider and code, and "Resending fails the same way. Run /compact … or /new …", with retryable: false. Other 50113 failures are unchanged.
  3. Compaction fallback
    • compaction/contracts.ts: new CheckpointCandidateMediaRejectedError. checkpoint-provider.ts throws it when a checkpoint result or thrown error matches isLLMImageLimitMessage.
    • compaction/algorithm/compact-context.ts: on that error, recoverAfterMediaRejection retries the rejected candidate with media replaced by text (buildAttachmentFreeCandidate, reported as hvideo).
      • If that candidate overflows, it continues down the existing attachment-free ladder from Hall (Hvideo, Hmid, Hmin) and never resends an identical attachment-free history.
      • If there was no media to strip, or the image-free request is rejected again, it fails with CHECKPOINT_PROVIDER_FAILED ("…rejected the request for carrying too many images").

Tests (all registered in the capability suite):

  • New packages/agent-core/test/unit/pi-turn-runner/request-image-limit.test.ts:
    • placeholder projection, including attachment paths, tool-result images with and without a path, mismatched reminders and no input mutation;
    • default and per-model ceilings, and undersized images not counted;
    • the reporter's regression over a real HTTP transport: a text-only turn after 44 historical images completes with one request of 20 images against a server that rejects more than 30;
    • with a model ceiling of 100, the rejection fails after exactly one request.
  • llm-retry.test.ts:
    • deterministic rejections (image limit and 400 invalid_request_error, BYOK and non-BYOK) make one attempt and no sleep;
    • plain 400, 503 and 429 mentioning images still make six attempts;
    • classifier cases, including the BYOK-attributed message form the retry loop actually sees.
  • New packages/tui/test/unit/runtime-error-presentation-rejection.test.ts: the new message, reason, /compact and /new guidance and retryable: false; the BYOK prefix stripped; the invalid-request variant; generic 50113 unchanged.
  • compact-context.test.ts: image rejection falls back to an image-free Hvideo; the overflow continues the ladder; no duplicate attachment-free request; failure without media; failure on a second rejection.
  • checkpoint-provider.test.ts: an error result and a thrown error both map to the media rejection; the checkpoint request is capped at 20 images.
  • local-model-resolver.test.ts: a configured value (number or string) reaches the executor model; invalid or missing values leave the default; the Mistral host gets 8 unless overridden; a gateway path containing api.mistral.ai does not.

Validation

  • Checks run and results (head 6ca1a8f, base 56221c1, Linux, Node 24.21.0):
    • The key new tests fail on main behavior: 21 failures across the 6 touched test files. Examples:
      • the regression turn ends FAILED;
      • the rejection is sent 6 times ([31,31,31,31,31,31]);
      • BYOK deterministic rejections take 6 attempts;
      • the TUI still says "temporarily unavailable / Run /retry";
      • compaction propagates the media rejection;
      • checkpoint requests are not capped;
      • the resolver drops max_images_per_request.
    • pnpm typecheck, pnpm lint, pnpm check:source: pass.
    • node scripts/run-vitest-suite.mjs capability: 210 files, 5285 passed, 18 skipped.
    • pnpm verify: passed, 14 gates on Linux (test:windows, test:sandbox and test:release-package skipped as not applicable).
    • Local end-to-end replay of the real TUI (built dist) in node-pty against an OpenAI-compatible mock. The mock counts image_url parts and returns the reporter's HTTP 400 invalid_request_error body above its limit:
      • Gateway limit 30, fresh session: 31 images over 9 turns, then hi, /compact and continue. Every request carried at most 20 images and returned 200. The stored pre-compaction history still held all 31 images and no placeholders.
      • The session left stuck by the main repro (31 images, failed /retry, /compact and restart) was resumed with the fix. A plain hi sent one request with 20 images and 11 path placeholders and returned 200. /compact and the next turn succeeded. The old stored error now renders with the new text.
      • Gateway limit 10 (below the default): the 12-image request was rejected once, with no retries, and the TUI showed the new error with Reason: 400 Upstream [invalid_request_error] Too many images in request: 12 > 10 and the /compact or /new guidance. The prompt was preserved. /compact then sent a checkpoint (12 images, 400) and the image-free fallback (0 images, 200); the next turn succeeded.
      • max_images_per_request: 8 in config.yaml with a gateway limit of 10: every request carried 8 images, all 200 (re-run on the final head).
  • Performance: basic. This changes outbound history projection, so CONTRIBUTING asks for perf:full. I did not add labels; maintainers may want to add perf:full.
  • NOT RUN, platform limitations and live-service boundaries:
    • no live provider or gateway runs: the reporter's gateway, MiniMax, OpenRouter and Mistral were not called, and all evidence is from the local mock;
    • native Windows and macOS not run;
    • the 200k-window auto-compaction path was not re-run end to end (covered by unit tests);
    • the browser/screenshot tool path is covered only by unit tests (tool-result images), not driven end to end.

Risks / tradeoffs

  • The model no longer sees images older than the newest N in a request. The placeholder tells it the file path so it can re-read the image with a tool if needed. Very image-heavy sessions lose older visual context earlier than before.
  • Once a session holds more than N images, the omitted window moves each turn. This changes an early part of the request and lowers provider prompt-cache hits for image-heavy sessions; the alternative was a permanently failing session.
  • 20 is a heuristic default. Gateways with a lower limit still fail until the model sets max_images_per_request; with this PR they fail once with a clear message and /compact recovers. Mistral's current docs also say the limit depends on the model; 8 is the documented API limit and a conservative choice.
  • The BYOK retry-all exemption depends on message text: "too many images"-style phrases, or invalid_request_error together with an HTTP 400. A gateway that sends a transient failure as HTTP 400 invalid_request_error will no longer be retried. Bare 400s and anything that looks transient keep the old behavior.
  • Messages from failed turns still stay in history, as on main, so their images count toward the cap.

Publication and contribution checks

  • I have permission to contribute these changes under the existing licenses applicable to the changed files/packages; imported material and its provenance are identified and existing notices are preserved.
  • No credentials, account data, real user content, internal source history or private review material is included.
  • Added/removed source files were reviewed before regenerating release/public-source.json; new tests are declared in test/vitest-suites.json where applicable.
  • Shared English/Chinese documentation and capability/verification records are updated where applicable. Mock/offline results are not described as live-service acceptance. (docs/examples.md has no Chinese counterpart; README_ZH.md does not cover image configuration.)

Maintainer handoff

Publication scope or license changes (if any): none.

Shared-source port: pending.

…vider limit

Inline images stay in persisted history and every request re-sends the
whole provider-visible history, so a long session eventually exceeded the
provider's per-request image limit ("Too many images in request: 31 > 30").
Every later request, /retry, /compact and a restarted session then failed
the same way, each after the full BYOK retry window, behind a generic
"temporarily unavailable / Run /retry" message.

- Request-time image ceiling: projectAgentMessagesForModel keeps the newest
  N images across user messages and tool results and replaces older ones
  with a short placeholder naming the original file. Stored history is
  never rewritten. N comes from capabilities.max_images_per_request
  (default 20; api.mistral.ai defaults to its documented 8). Agent turns,
  checkpoint requests and footprint estimates share the projection.
- Deterministic rejections are terminal: an image-count rejection, or an
  HTTP 400 invalid_request_error, is no longer retried by the BYOK
  retry-all path (timeouts, network, 408/429/5xx stay retryable). The TUI
  shows the upstream reason and suggests /compact or /new instead of /retry.
- Compaction fallback: when the checkpoint Provider rejects a candidate for
  too many images, compaction retries the same history with media replaced
  by text (Hvideo) and continues down the attachment-free ladder.

Refs #425
Comment thread packages/agent-core/src/pi-turn-runner/request-image-limit.ts Fixed
CodeQL flagged the attribute regular expression as polynomial on
user-controlled message text (js/polynomial-redos). Parse the attachment
tags with indexOf instead and cover escaped paths and adversarial input.

Refs #425

@hetaoBackend hetaoBackend left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked at the request-side image cap, the classifier, and the BYOK retry-all exemption on 6ca1a8f. Cannot formally APPROVE (PR is under this GitHub account), so this is a review comment.

Image cap — Looks right. Applied last in projectAgentMessagesForModel after video/undersized projection, so the count matches what is actually sent. Newest N kept, older replaced with path placeholders, stored history untouched. Default 20 with headroom under the #425 gateway's 30, Mistral host exact-match → 8, override via max_images_per_request. Linear attachment parse on this head addresses the earlier CodeQL alert.

Error classification / TUI — image_limit and invalid_request are recognized, and the rejection branch runs before the generic 50113 "temporarily unavailable" path, so the real reason shows and retryable: false.

BYOK retry exemption — Narrow enough. Only isLLMDeterministicRequestRejection exits retry-all: image-limit with status undefined/400/413/422, or HTTP 400 + invalid_request_error. Abort/timeout/network/408/429/5xx stay retryable. Plain 400 Bad Request and "too many images queued" on 503 still take the full 6 attempts, matching the tests.

Non-blocking note: the exemption covers any HTTP 400 invalid_request_error, not only image-count failures (empty text blocks etc.). That is deliberate and fine for #425; just calling out the tradeoff already in the PR description (a gateway that mislabels a transient failure as 400 invalid_request_error will no longer retry).

From my side this is good to leave draft and merge once Jack is happy with /compact + defaults.

@hetaoBackend
hetaoBackend marked this pull request as ready for review October 5, 2026 06:52
@hetaoBackend
hetaoBackend merged commit 1852771 into main Oct 5, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants