Skip to content

Truncate oversized run-event payloads at the durable append boundary - #2774

Merged
kwakayama merged 3 commits into
mainfrom
fix/durable-run-event-oversize
Jul 4, 2026
Merged

kwakayama merged 3 commits into
mainfrom
fix/durable-run-event-oversize

Conversation

@kwakayama

Copy link
Copy Markdown
Contributor

What & why

An agent run could silently drop an oversized tool result and still report completed, leaving the durable event log with a hole and the UI waiting on a tool result that never arrived (observed on staging as a run stuck on "Fetching from the web… / Continuing…"). Tracked in veryfront/veryfront-studio#5552.

Root cause: a large tool result (e.g. a big web_fetch) serializes past the API's 256 KB per-event limit on the durable append endpoint. A truncation layer already existed (run-event-normalization.ts) but was bypassable (only wired into some producer paths, not the append chokepoint) and not size-correct (it truncated by raw UTF-8 bytes while the API measures JSON-escaped bytes, never re-checked the total, and ignored the redundant input field). The rejected event was then retried ~34× with no give-up, and completion is decided on a separate lifecycle path, so the run reported success with the result missing.

Changes

  • Enforce the size limit at the append chokepoint. appendConversationRunEvents now normalizes every batch immediately before POST, so no producer path (hosted lifecycle, child-run progress, or a direct enqueue) can bypass it. Idempotent on already-normalized events.
  • Make normalization total and correct. Measure JSON-serialized bytes (the unit the API enforces), drop the redundant tool input, truncate via a whole-event binary search, and add a backstop that guarantees every normalized event fits — regardless of event type, which field holds the bulk, or JSON escaping.
  • No more retry storm. A payload-too-large rejection is now classified as permanent, so the durable mirror stops with an ERROR-level log instead of retrying the same rejected bytes forever.

Why runtime-only (no API change)

Layers above make an oversized event unreachable at the source, and the API already degrades gracefully: normalizeTerminalToolCallStates flips any orphaned (result-less) tool call to a terminal state at finalize, so there is no permanent-pending. A schema flag / compensating-event layer / append-validation reorder would be over-engineering a now-near-impossible state — deliberately omitted.

Testing

  • TDD throughout (RED → GREEN). New tests: escape-heavy / non-content / generic / split-delta all stay within the byte limit; the append chokepoint clamps a raw oversized event; the oversize classifier + failure-handler routing to a permanent stop.
  • Full src/agent/conversation unit suite green (18 files / 129 steps); deno check clean on all changed files.

Follow-up

After this ships to npm, bump the veryfront dependency in veryfront-agent to pick it up on the hosted runtime.

kwakayama added 2 commits July 5, 2026 01:51
Oversized tool results (e.g. a large web_fetch) exceeded the API's 256 KB
per-event limit, were rejected, retried indefinitely, and silently dropped —
leaving the run's durable event log with a hole while the run still reported
completed and the UI waited on a tool result that never arrived.

- Normalize every batch at the appendConversationRunEvents chokepoint, so no
  producer path (hosted lifecycle, child-run progress, direct enqueue) can
  bypass the per-event size limit. Idempotent on already-normalized events.
- Make run-event normalization total and measured in JSON-serialized bytes —
  the same unit the API enforces. Previously truncation measured raw UTF-8
  bytes, so escape-heavy content slipped past the limit; it also never
  re-checked the total and ignored the redundant tool `input` field. Now it
  drops `input`, truncates via a whole-event binary search, and a backstop
  guarantees every normalized event fits regardless of type, field, or escaping.
- Classify a payload-too-large rejection as permanent so the durable mirror
  stops with an ERROR log instead of retry-storming the API (~34 retries in the
  observed incident).
@kwakayama
kwakayama requested a review from kojiwakayama as a code owner July 4, 2026 23:52
Copilot AI review requested due to automatic review settings July 4, 2026 23:52

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0728263f71

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +195 to +198
type,
truncated: true,
note: "Conversation-run event payload was summarized to stay within storage limits.",
summary: summarizeValue(rest),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve custom event fields during summarization

When a CUSTOM event exceeds the limit, this moves name and value under summary instead of preserving the top-level contract: the encoder emits CUSTOM as top-level name/value (src/agent/conversation/run-events.ts), and AG-UI validation requires payload.name/payload.value (src/chat/ag-ui.ts). Large data-* events persisted through this path therefore cannot be replayed or validated as custom events; preserve the required top-level fields and summarize only the oversized value.

Useful? React with 👍 / 👎.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens durable conversation-run event mirroring against oversized per-event payloads by enforcing normalization at the durable append boundary, guaranteeing every event is under the API byte limit and preventing retry storms on permanent “payload too large” rejections.

Changes:

  • Enforce run-event normalization in appendConversationRunEvents (append chokepoint) so no producer path can bypass size limits.
  • Make normalization byte-limit-correct (JSON-serialized bytes, redundant tool input dropped on oversize paths, whole-event truncation/backstop guard).
  • Classify payload-too-large append failures as permanent and stop mirroring instead of repeatedly retrying.
  • Add targeted tests covering escape-heavy content, non-content oversize, split delta parts, chokepoint clamping, and permanent-stop routing.
  • Bump package version to 0.1.1009.

Verification

  • Not run here (no command execution available in this review environment).
  • Safest next step: run the repo’s unit tests and check on the changed modules:
    • deno check src/agent/conversation/{durable.ts,run-event-normalization.ts,durable-append-errors.ts}
    • deno test --no-check --allow-all --parallel src/agent/conversation/

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/utils/version-constant.ts Bumps exported VERSION to align with release.
deno.json Bumps package version to 0.1.1009.
src/agent/conversation/run-mirror.ts Adds payload_too_large as a durable-mirror stop reason.
src/agent/conversation/run-event-normalization.ts Ensures normalization is JSON-byte-correct and enforces a final per-event size invariant.
src/agent/conversation/run-event-normalization.test.ts Adds tests for escape-heavy, non-content, generic, and split-delta normalization staying within the limit.
src/agent/conversation/run-chunk-mirror.ts Logs and handles the new payload_too_large disable reason distinctly.
src/agent/conversation/durable.ts Normalizes events at append chokepoint and treats payload-too-large append failures as permanent stops.
src/agent/conversation/durable.test.ts Adds coverage for chokepoint clamping and permanent-stop behavior on oversize rejections.
src/agent/conversation/durable-contracts.ts Extends controller contract union to include payload_too_large.
src/agent/conversation/durable-append-errors.ts Adds classifier for payload-too-large append errors.
src/agent/conversation/durable-append-errors.test.ts Tests payload-too-large classification and ensures it is not treated as ignorable.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@kwakayama

Copy link
Copy Markdown
Contributor Author

Critical review — 92/100 ✅

Reviewed the full diff against the source, ran the touched unit suites, and typechecked the changed files.

Verification

  • deno test src/agent/conversation/{run-event-normalization,durable,durable-append-errors}.test.ts → 3 files / 44 steps, all green
  • deno check on all four changed .ts files → clean
  • Branch is 0 commits behind main, mergeable, version + version-constant.ts bumped together (0.1.1009).

What's strong

  • Right chokepoint. Normalizing inside appendConversationRunEvents means no producer path (hosted lifecycle, child-run progress, direct enqueue) can bypass the size guard — the fix holds even for callers that don't pre-normalize. Idempotent on already-normalized events, so the mirror's double-normalization doesn't change event counts or break external-sequence accounting.
  • Correct measurement. Switching from raw-UTF-8 to JSON-serialized byte length matches the unit the API actually enforces, and the whole-event binary search + dropping the redundant input field close the two real gaps in the old normalizer.
  • enforceEventSizeLimit backstop guarantees every emitted event fits regardless of type/branch — verified it catches the pathological generic-summary case that can still exceed the budget after summarization.
  • Retry-storm killed. isPayloadTooLargeConversationRunAppendError → permanent stopped outcome with an ERROR-level log is exactly the ops fix this needed (ref veryfront/veryfront-studio#5552).

Minor gaps (non-blocking, worth a follow-up)

  1. Escape-heavy split parts lose data silently. For TEXT_MESSAGE_CONTENT/REASONING_*/TOOL_CALL_ARGS, splitStringFieldEvent splits by raw bytes; an all-" delta doubles under JSON escaping so each part re-enters enforceEventSizeLimit, which truncates (drops the tail) rather than splitting into more parts. Strictly better than the prior reject→retry-storm, and the new test confirms parts fit — but it does not assert preservation. TOOL_CALL_ARGS (JSON, quote-heavy) is the case where a truncated tail could corrupt reassembled args. Low probability (deltas stream in small chunks), acceptable as a follow-up: make the split escape-aware, or drop TOOL_CALL_ARGS to the summarize path.
  2. Perf: every append now serializes each event once for the size check and again for the POST body (~2× on the mirror hot path). Fast-path early-return keeps it cheap, but worth noting for high-volume streaming.
  3. 240KB vs the API's 256KB relies on an assumption not verifiable in this repo — reasonable 16KB headroom for the batch envelope/escaping, just flagging the coupling.

The "why runtime-only" reasoning (API already flips orphaned tool calls terminal at finalize) is sound and correctly avoids over-engineering a now-near-impossible state.

Merging at 92. Don't forget the noted follow-up: bump the veryfront dep in veryfront-agent so the hosted runtime picks this up.

@kwakayama
kwakayama merged commit ff72c81 into main Jul 4, 2026
30 checks passed
@kwakayama
kwakayama deleted the fix/durable-run-event-oversize branch July 4, 2026 23:59
@kwakayama

Copy link
Copy Markdown
Contributor Author

Addressed review finding #1 (escape-heavy split data loss) — commit c70f5a6

The split path (splitStringFieldEvent) budgeted by raw UTF-8 bytes, which undershoots the JSON-escaped byte limit the API actually enforces. For escape-heavy deltas (e.g. quote-dense TOOL_CALL_ARGS) each part came out oversized once serialized and was then truncated by the enforceEventSizeLimit backstop — silently dropping the tail.

Fix: split by measuring the whole serialized event per candidate prefix (binary search on the same JSON-byte unit truncateEventStringFieldToLimit uses), so every part fits and the parts reconstruct the original delta losslessly. Removed the now-dead raw-byte budget helper. enforceEventSizeLimit stays as the invariant guard but no longer has to truncate these.

Tests (TDD, RED→GREEN): added lossless-reconstruction assertions for escape-heavy TEXT_MESSAGE_CONTENT and TOOL_CALL_ARGS deltas (parts.map(p => p.delta).join("") === original) plus per-part size checks.

Verification: full src/agent/conversation unit suite green (18 files / 131 steps); deno check + deno lint + deno fmt clean on both changed files. No version bump (0.1.1009 unpublished).

The other two review notes were informational, not defects: the ~2× serialization at the chokepoint is inherent to a size-enforcing guard (fast-path early-return keeps it cheap), and the 240KB-vs-256KB margin is intentional envelope headroom.

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.

3 participants