Skip to content

fix(agent-runtime): ignore a stream delta that omits its string field - #888

Merged
vastsa merged 1 commit into
mainfrom
fix/pi-ai-frame-missing-string-field
Sep 22, 2026
Merged

vastsa merged 1 commit into
mainfrom
fix/pi-ai-frame-missing-string-field

Conversation

@vastsa

@vastsa vastsa commented Sep 22, 2026

Copy link
Copy Markdown
Owner

Summary

A gateway that re-serializes provider events with omitempty (Go) or drops
undefined (JS) sends a text / thinking / tool-call delta without its string
field
. AssistantMessageFrameEncoder read .length off that missing value
and threw a bare TypeError: Cannot read properties of undefined (reading 'length'). The runtime had no HTTP status and no network cause to classify, so
it fell through to a retriable PROVIDER_ERROR: the turn burned the shared
ten-attempt budget and then showed the V8 text to the user, and the UI kept
offering "continue" for something that could never succeed.

Because the trigger is a property of one relay plus one conversation, the turn
failed identically every time, a fork of the session inherited it, and a new
session was clean — exactly the shape reported in #883.

Fix

  • Encoder (root cause). A non-string payload now reads as the empty string,
    which is the only value a dropped field can have been. This covers the
    text_start / thinking_start coverage counters, toolcall_delta,
    encodeTextDelta, and the two clone*Content helpers. The degradation is
    lossless and the frame protocol is unchanged.
  • Decoders (the values themselves). anthropic-messages (text, thinking,
    input_json, signature) and openai-responses-shared (text, thinking,
    reasoning summary, tool arguments) stop passing a missing field through.
    Without this, block.text += undefined corrupts accumulated text into the
    literal "undefined" — a wrong answer instead of an error.
  • openai-completions already guarded the same field; verified, not changed.

Both live in patches/@earendil-works__pi-ai@0.86.1.patch, regenerated through
pnpm patch / pnpm patch-commit. Upstream 0.87.0 still has the bare
.length, so the guards have to be carried forward on the next bump.

Correction to the report

The issue also claims the retry counter accumulates across turns and is only
cleared by a clean turn. That is not the mechanism: resetRunRecoveryState()
(runtime.ts:7790 → 5602) resets it at the start of every user turn, so a
repeated retryAttempt: 10 means each turn exhausted its own budget. The turn
fails every time because the trigger is deterministic, not because the budget
stays spent.

The report also says an OpenAI Chat Completions style relay reproduced it. It
does not: every omitted/empty shape is guarded on that path. The OpenAI-family
decoder that does share the defect is openai-responses, which this PR fixes.

Verification

  • packages/agent-runtime/src/provider-stream-frame-fields.test.ts (new):
    drives the real decoders through a stubbed transport and feeds every event
    through the real encoder. On the unpatched dependency 4 of 6 cases fail
    with the reported TypeError; both control cases pass. After the patch: 6/6.
  • Patch round-trip: the regenerated patch applies cleanly to a pristine
    pi-ai@0.86.1 tarball and reproduces the installed package byte-for-byte, so
    the pre-existing hosted-search hunks are intact.
  • pnpm build:js — pass (all packages, includes tsc typecheck).
  • pnpm -r --if-present test — pass: agent-runtime 1014, desktop 2659,
    shared 986, plugin-sdk 328, plugin-devkit 48, agent-host 47, host-runtime 46,
    i18n 25, racp 21, pi-host 6.
  • pnpm lint — pass.
  • node scripts/e2e-hosted-search.mjs — PASS 7/7 (search-next-prompt,
    search-read, search-read-instruction-change, search-task-delegation,
    search-persist-restore, invalid-search-container, invalid-search-phase)
    against the rebuilt production sidecar, whose source/patch/lock fingerprint
    stayed stable across the run. This is the relevant E2E suite: hosted-search
    frames are the frames this encoder produces.
  • pnpm check:pr-base — pass.

Impact

  • Specs/ADRs: none affected. No public interface, schema, IPC or persisted
    format changes; the frame types keep their strings.
  • Compatibility: behavior at the wire level is unchanged for well-formed
    providers. A relay that drops empty string fields now streams a no-op delta
    instead of failing the turn.
  • Migration: consumers need pnpm install for the patch to apply.
  • Remaining risk / follow-up: openai-responses-shared reads
    event.arguments.startsWith(...) on response.function_call_arguments.done;
    a relay that drops an empty arguments would throw
    reading 'startsWith' there. Same upstream behavior, different symptom, not
    in the report — left out to keep this diff to the reported defect.

Refs #883

A gateway that re-serializes provider events with `omitempty` (Go) or
drops `undefined` (JS) sends a text, thinking, or tool-call delta
without its string field. The assistant message frame encoder read
`.length` off that missing value and threw a bare TypeError, which the
runtime classified as a retriable `PROVIDER_ERROR` — no HTTP status and
no network cause — so it consumed the shared ten-attempt budget and then
showed the V8 text to the user. The trigger is a property of one relay
plus one conversation, so the turn failed identically every time, a fork
inherited it, and a new session was clean (issue #883).

The encoder now reads a non-string payload as the empty string, the only
value a dropped field can have been. The two stream decoders that leaked
the missing value — `anthropic-messages` (text, thinking, input_json,
signature) and `openai-responses-shared` (text, thinking, reasoning
summary, tool arguments) — stop passing it through, so a dropped field
can no longer corrupt accumulated text into the literal "undefined".
`openai-completions` already guarded the same field.

The regression test drives the real decoders through a stubbed transport
and feeds every event through the real encoder; on the unpatched
dependency it fails with the reported TypeError.

fixes #883
Copilot AI lite review requested due to automatic review settings September 22, 2026 16:23

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@vastsa
vastsa merged commit 3310830 into main Sep 22, 2026
3 checks passed
@vastsa
vastsa deleted the fix/pi-ai-frame-missing-string-field branch September 22, 2026 16:27
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