Skip to content

feat(agent): emit native run event types instead of AG-UI Custom wrappers - #4467

Merged
kojiwakayama merged 18 commits into
mainfrom
worktree-native-run-events
Sep 10, 2026
Merged

feat(agent): emit native run event types instead of AG-UI Custom wrappers#4467
kojiwakayama merged 18 commits into
mainfrom
worktree-native-run-events

Conversation

@kojiwakayama

@kojiwakayama kojiwakayama commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Veryfront now emits native run events for tool status, input requests, child-run status, citations, and attachments across live SSE and durable publication. A shared vocabulary produces both shapes, and the readers continue to accept legacy Custom events.

The public child-run Custom-event builders, schemas, types, and publisher callbacks retain their existing contract. Native conversion happens at the shared direct-append and mirror publication boundary. Both chat decoder validation paths accept omitted, null, or string tool names from the internal SSE formatter, in strict and permissive modes. Reserved type, elapsedMs, and emittedAt fields are removed before application values become native payloads. Input records cannot override the native wire type or transport clocks; nested application metadata remains intact.

The scope spans producers, transport, persistence, replay, chat decoding, and evaluation because each must agree on the event shapes. The shared contract fixture and reader round trips cover those boundaries. The browser vocabulary stays separate to preserve client bundle boundaries, with tests that check it against the producer vocabulary.

Oversized tool-status, input-request, and child-run records produce bounded omission markers that retain the original event type and tool call ID where available. Dedicated tests cover all four stored lifecycle types. Full payload retention for those oversized records remains outside this change.

Deployment requires API support for these native events before the runtime ships. Staging validation of a slow tool call, citation, and child run remains outstanding. The runtime-context Custom event is unchanged and outside this PR.

Validation uses the pinned Deno 2.7.7. Focused regressions reproduce the public API, native-type, and optional tool-name failures before their fixes. The final broader event and consumer suite passes: 93 tests and 1,173 steps. The latest timing regressions and encoder cases pass: 5 tests and 90 steps. Twenty timestamp-collision cases failed before their fix. Types, lint, formatting, client-bundle boundaries, test audits, generated references, and diff checks pass. The CI/CD workflow and Codex review pass for 4d21b9d7901e45b4fab34073e416d5f5c45fcd02. All review threads are resolved, and the PR is conflict-free and ready for review.

The seven extension events this runtime wraps in AG-UI Custom frames now have
one place that owns their wire name, stored type, legacy custom name, and the
two emission shapes built from a single payload. No caller uses it yet.

Claude-Session: https://claude.ai/code/session_01NonPHDcbWsisd2GFB68ALo
TOOL_CALL_STATUS_CHANGED needs a tool name, and neither the telemetry frame nor
the data-tool-call-status chunk carried one. Both now take it from the tool the
reducer or the status tracker already has open. Two telemetry fixtures outside
the primary change set (lifecycle-run-event-adapter.test.ts,
executor-data-producers.test.ts) are updated to satisfy the now-required field.

Claude-Session: https://claude.ai/code/session_01NonPHDcbWsisd2GFB68ALo
Citations, attachments, tool status, input request lifecycle, and child run
lifecycle now leave the live encoder under their own wire names. State chunks
and unknown data names keep the Custom wrapper, and ToolCallStart names its
turn.

Claude-Session: https://claude.ai/code/session_01NonPHDcbWsisd2GFB68ALo
The durable encoder and the durable lifecycle adapter now build their records
from the same builders the live path uses, and the run event type table derives
its new members from the native list. TOOL_CALL_START names its turn here too,
because these are the rows the API derives tool spans from.

Claude-Session: https://claude.ai/code/session_01NonPHDcbWsisd2GFB68ALo
The child run producer appends a native record instead of a CUSTOM twin, and
the paired STATE_DELTA is unchanged. The exported symbols keep their names so
veryfront/agent stays compatible; each carries a Phase F marker naming the
rename to make at the cutover.

Also updates a downstream test (durable-child-fork-execution.test.ts) that
matched on the removed CUSTOM shape, adds an end-to-end test driving
buildInputRequestLifecycleDataEvent through both encoders, and adds a durable-
path case proving the child-run record for run-events.ts routing.

Claude-Session: https://claude.ai/code/session_01NonPHDcbWsisd2GFB68ALo
The SSE parser resolves the native wire names from the same list the encoders
emit them from, and the eval harness keys tool status on
TOOL_CALL_STATUS_CHANGED and reads the payload directly, while still
recognizing the legacy CUSTOM/tool-call-status fallback the encoder still
produces for a status update with no toolCallId.

By controller ruling after Task 4's codex review, the version 1 and version 2
legacy run read adapters now decode the seven native stored types back as the
same custom lifecycle event their CUSTOM twins produced, so every downstream
consumer of that adapter keeps seeing one shape.

Claude-Session: https://claude.ai/code/session_01NonPHDcbWsisd2GFB68ALo
… custom events

DURABLE_ENVELOPE_KEYS stripped the version-2 protocol envelope fields and
type, but missed elapsedMs/emittedAt, which ConversationRunEventEncoder's
stampElapsed() stamps onto every durable event's own top level -- including
the native records it builds for TOOL_CALL_STATUS_CHANGED/URL_CITED/
DOCUMENT_CITED/FILE_ATTACHED. A true CUSTOM record never leaked these because
its value is nested, but readNativeAsLegacyCustom spreads the whole flat
event, so they leaked into the rebuilt CUSTOM twin's value in production.

Added elapsedMs/emittedAt to the strip list, with a version 1 regression test
that stamps them onto a native record (mirroring stampElapsed's real output)
and asserts they don't survive into the rebuilt value. The test deliberately
uses URL_CITED rather than TOOL_CALL_STATUS_CHANGED: the version 1 reducer
special-cases a custom signal named "tool-call-status", so a "completed"
status produces no semantic custom frame on either path and the comparison
would be a vacuous `[] === []`, unable to catch this class of leak.

Also annotated (not derived) NATIVE_CITATION_AND_FILE_STORED_TYPES: the
citation/file trio is a domain fact about three specific builders that
NativeRunEventDefinition doesn't record, so deriving it from NATIVE_RUN_EVENTS
would need a new field on that module rather than removing real duplication.

Claude-Session: https://claude.ai/code/session_01NonPHDcbWsisd2GFB68ALo
The decoder dropped any frame whose event name it did not know, which would
have silently lost tool status, citations, attachments, input requests, and
child run progress the moment the runtime stopped wrapping them in Custom. Each
native name now maps back to the chunk its custom twin produced.

Citation and attachment metadata (title/filename/url) is validated the way
the twin rendered it: the encoder never type-checks these before sending
them, so an empty string, a null, or even a wrong-typed value must not
reject the whole frame, only fall back gracefully like toRenderableCustomChunk
did. Reconstructed legacy data chunks also strip the live encoder's
elapsedMs/emittedAt timing stamps, which land on a native frame's flat
payload but never inside a Custom frame's nested value, and restore the
chunk's own type discriminator that the wire format omits so unrenderable
citations and attachments produce byte-identical fallback data to the twin.

Claude-Session: https://claude.ai/code/session_01NonPHDcbWsisd2GFB68ALo
… names

Fix round 1 for the Task 7 review (task-7-review.md):

- Move the seven native-frame tests out of the "without a registered
  SchemaValidator" describe block and into the schema-validated one they
  were actually exercising, and add a dedicated schemaless-path test so the
  seven new isValidAgUiPayload arms have real coverage on the path the
  browser chat client uses before a schema adapter is registered.
- Pin AG_UI_WIRE_EVENT_NAMES against NATIVE_RUN_EVENTS with a test-only
  import (the test file is outside the client bundle graph), so an eighth
  native wire type added to the producer fails a test here instead of being
  silently dropped by the decoder.
- Export AG_UI_EVENT_TIMING_STAMP_FIELDS from the encoder module and assert
  the decoder's stripAgUiTimingStamps list covers every stamped field, so a
  third transport field stamped later can't leak into legacy data the way
  elapsedMs/emittedAt already did once (fixed in legacy-run-read-adapter.ts,
  commit 3ee902f).
- Declare UrlCited's title as v.unknown().optional() like its DocumentCited
  and FileAttached siblings instead of reading it through a cast.
- Mirror toRenderableCustomChunk for UrlCited's sourceId: fall back to url
  when it is absent or empty instead of requiring it.

Claude-Session: https://claude.ai/code/session_01NonPHDcbWsisd2GFB68ALo
One sample per native type in both emission shapes, generated from the builders
and pinned with its digest. The veryfront-api copy pins the same digest, so
drift on either side fails a test. The pinning test reads the fixture off disk
to hash its bytes, so it lives under tests/integration/semantic-unit-boundary/
per the semantic unit-boundary audit rather than beside the colocated unit
tests.

Claude-Session: https://claude.ai/code/session_01NonPHDcbWsisd2GFB68ALo
…SE wire schema

The internal-agents AG-UI SSE handler formats ToolCallStart payloads
through an allow-list schema before writing to the wire. The live
encoder now stamps parentMessageId on ToolCallStart (P4), but this
schema did not declare the field, so it was silently dropped on this
production path. Add it as optional, matching the encoder's own
conditional emission, and update the two pinned test fixtures that
this regressed.

Claude-Session: https://claude.ai/code/session_01NonPHDcbWsisd2GFB68ALo
…rough native conversion

Two findings from codex review:

- buildNativeRunEventFrame's "tool-call-status" case only carried
  toolCallId, status, toolCallName, and parentMessageId into the native
  TOOL_CALL_STATUS_CHANGED record, dropping any arguments/result/error/
  exitCode fields the legacy Custom wrapper passed through unchanged.
  src/eval/agent-service.ts's applyToolCallStatusEvent still reads those
  off the native record when a standard tool result is absent, so
  conversion silently lost trace data. Widen ToolCallStatusChangedInput
  with an index signature and spread the source record through, the way
  buildChildRunStatusChangedEvent already does.

- normalizeConversationRunEvents() routed an oversized native record
  (URL_CITED, DOCUMENT_CITED, FILE_ATTACHED with a large inline `data:`
  url, or any other native type) through summarizeGenericEvent(), whose
  `{ type, truncated, note, summary }` shape drops the API-catalog-
  required fields a native type carries. Unlike the permissive legacy
  CUSTOM wrapper this replaced, that fails validation and the durable
  append rejects it. Truncate the oversized `url` field directly for the
  three citation/attachment types, keeping mediaType/sourceId/filename
  intact, and fall back to the existing CUSTOM-typed omission event for
  any other native type that somehow lands here.

Added regression coverage for both in run-events.test.ts.

Claude-Session: https://claude.ai/code/session_01NonPHDcbWsisd2GFB68ALo
Fixes I1 and M1-M6 from the 2026-09-09 native-run-events-producer final
whole-branch review:

- I1: buildUrlCitedEvent/buildDocumentCitedEvent/buildFileAttachedEvent no
  longer let an empty title/filename/url reach the durable record the API's
  batch append route validates (z.string().min(1).optional() rejects empty
  string outright). The live wire frame keeps the field unguarded, since the
  chat decoder in src/chat/ag-ui.ts uses a string title (DocumentCited) or
  url (FileAttached) -- empty string included -- as its gate for rendering
  the citation/attachment at all; dropping it from the live frame too broke
  that rendering (caught by codex review). DocumentCited's title additionally
  falls back to sourceId instead of ever being empty in either shape, since
  it is required at the chat UI type level and has no safe non-empty
  fallback story once already stored -- unlike FILE_ATTACHED's url, which
  has no safe placeholder (a fake url would be a misleading, possibly broken
  link) and is documented as a known, accepted replay-rendering gap for the
  rare case of an attachment chunk with an explicitly empty url.
- M1/M2: run-event-normalization.ts's oversized-citation truncation now
  shrinks URL_CITED's sourceId alongside url when it mirrors an oversized
  url (truncateEventStringFieldToLimit gained a mirrorField parameter for
  this), and drops DOCUMENT_CITED from the url-truncation case since it has
  no url field and already fell through to the same omission event either
  way.
- M3: the seven native wire names are declared in ag-ui-sse.ts's payload
  schema allow-list and AgUiEventName union, each `.passthrough()`-ed so
  extension fields survive.
- M4: removed the unreachable second `data-` routing copy in the live
  encoder's default branch.
- M5: legacy-run-read-adapter.ts's two twin-reconstruction sites now spread
  the stored value before the derived key, so a smuggled action/type inside
  the stored value cannot win over the value the reader derived.
- M6: isNativeRunEventName gained a JSDoc note that it exists for module
  consumers, per the P1 spec requirement.

Claude-Session: https://claude.ai/code/session_01NonPHDcbWsisd2GFB68ALo
…ble shapes

Ruling on the previous commit's I1 shape: revert the live/durable payload
split. One payload feeds both emission shapes (matching the cross-repo
contract fixture's durable === { ...live.payload, type } invariant), because
the live SSE frame is not only this repo's own chat client -- the API
ingests the same frame and stores it, so a value the batch append route
would reject is exactly as unsafe there as in the durable record.

- native-run-events.ts: toFrame() takes one payload again. The empty-string
  drop generalizes to omitInvalidOptionalStrings, which also drops null,
  numbers, and other non-string values for title/filename/url -- not just
  the empty string -- since the API's z.string().min(1).optional() rejects
  all of those the same way (caught by codex review after the first version
  only handled the empty-string case).
- src/chat/ag-ui.ts: the DocumentCited and UrlCited decoder cases now fall
  back to the citation's resolved source id when title is absent, since the
  encoder no longer carries an empty one on either shape. DocumentCited
  falls back unconditionally (ChatSourceDocumentUiPart.title is a required
  chat UI field); UrlCited falls back only for a genuinely absent title, not
  a present-but-wrong-typed one, preserving the existing "drops a non-string
  title" behavior. FileAttached is unchanged: a missing url still falls back
  to a raw data-file chunk, matching what the legacy Custom path's
  toRenderableCustomChunk already did for a missing url, since a url has no
  safe non-empty placeholder.
- src/chat/ag-ui-helpers.ts: toRenderableCustomChunk's source-url and
  source-document cases get the same title fallback, mirroring the two
  decoder cases above. This function decodes the reconstructed CUSTOM twin
  a replayed native URL_CITED/DOCUMENT_CITED durable record produces via
  legacy-run-read-adapter.ts, a code path the first version of this fix
  missed entirely (caught by a second codex review pass): without this, a
  citation that rendered live would disappear the moment its run was
  replayed.

RED/GREEN verified for every fix by reverting to the prior behavior and
confirming the corresponding new test failed, then restoring; codex review
--uncommitted run four times across this fix, each fixing what the previous
pass found, ending clean.

Claude-Session: https://claude.ai/code/session_01NonPHDcbWsisd2GFB68ALo
…ged schema

Applies N1 and N2 from the scoped re-review at
.superpowers/sdd/2026-09-09-native-run-events-producer/final-rereview.md
(range 355d5e7..1746359, approved with 0 Important / 4 Minor). N3 and
N4 are left per the controller's ruling.

- N1: four stale cross-references left by the previous commit's rename and
  deletion. Three comments still named `omitEmptyStrings`, renamed to
  `omitInvalidOptionalStrings` in that commit. Two comments cited
  `ag-ui.ts:1039` for a `{ ...payload, type: "source-document" }` pattern
  that commit deleted; reworded to cite the surviving
  `{ ...value, type: definition.legacyCustomName }` pattern in the same
  reader instead. One comment in src/chat/ag-ui.ts pointed at "the matching
  comment in the DocumentCited fallback above", which that commit removed
  entirely (DocumentCited no longer has a title-driven fallback branch);
  replaced with a self-contained explanation of the FileAttached case.
  ag-ui.test.ts's "tolerates a null optional field" test had its rationale
  reworded from "the builder preserves null" (no longer true --
  omitInvalidOptionalStrings drops it) to "the decoder tolerates a null it
  receives on the wire" (still true; the test feeds hand-built frames that
  bypass the builder).
- N2: src/internal-agents/ag-ui-sse.ts's ToolCallStatusChanged schema
  declared toolCallName as a required nullable string, matching the API
  catalog but not the previously unvalidated pass-through this allow-list
  replaced. A frame missing the field now reaches formatAgUiEvent and throws
  instead of reaching the wire; that throw is uncaught at
  run-stream.ts:1220 (aborts the run) and silently swallowed at
  ag-ui/handler.ts:192 (ends the stream early) -- both worse than the
  dropped-field failure mode this file's allow-list exists to prevent.
  Loosened to `.optional()`, matching the decoder's own
  `payload.toolCallName === null || typeof payload.toolCallName === "string"`
  tolerance for an absent value. Added a test confirming a frame without
  toolCallName still reaches the wire.

Gates run: deno task fmt, lint, lint:test-typecheck, typecheck (all clean);
deno task test:file on every touched test file (all green). RED/GREEN
verified for N2 by reverting the schema change and confirming the new test
failed, then restoring.

Claude-Session: https://claude.ai/code/session_01NonPHDcbWsisd2GFB68ALo

@greptile-apps greptile-apps 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.

Your trial has ended. Reactivate Greptile to resume code reviews.

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

📝 Walkthrough

Walkthrough

This change adds native AG-UI run events for tool status, input-request lifecycle, child-run lifecycle, citations, and file attachments. It routes live and durable encoders through shared builders, updates readers and chat decoders for compatibility, and propagates toolCallName and parentMessageId through related paths.

Changes

Native run-event protocol rollout

Layer / File(s) Summary
Native event catalog and builders
src/agent/ag-ui/native-run-events.ts, src/agent/ag-ui/native-run-events.test.ts, tests/fixtures/contracts/native-run-events.json, tests/integration/semantic-unit-boundary/src/agent/ag-ui/native-run-events-contract.test.ts
Adds the shared native run-event catalog, live/durable frame builders, legacy-name router, lookup tables, and fixture-backed contract tests.
AG-UI live encoding and SSE parsing
src/agent/ag-ui/encoder.ts, src/agent/ag-ui/lifecycle-adapter.ts, src/agent/ag-ui/sse-parser.ts, src/internal-agents/ag-ui-sse.ts, src/agent/ag-ui/*.test.ts, src/internal-agents/ag-ui-sse.test.ts, src/agent/input/request-protocol.test.ts
AG-UI encoders now emit native live events for supported data-*, citation, and file chunks, include parentMessageId on tool starts, and parse native wire names back to stored event types.
Durable run events and legacy readback
src/agent/conversation/run-events.ts, src/agent/conversation/lifecycle-run-event-adapter.ts, src/agent/conversation/legacy-run-read-adapter.ts, src/agent/conversation/run-event-normalization.ts, src/agent/child-run/invoke-agent-child-runs.ts, src/agent/conversation/*.test.ts, src/agent/child-run/invoke-agent-child-runs.test.ts, src/agent/hosted/durable-child-fork-execution.test.ts
Conversation and child-run writers now store native durable events. Legacy readers rebuild the matching CUSTOM twins. Native citation and file records also get dedicated oversize normalization rules.
Tool status name propagation
src/agent/streaming/lifecycle/..., src/provider/runtime-loader/tool-input-status.ts, src/provider/runtime-loader.test.ts, src/agent/runtime/chat-stream-handler.test.ts, src/agent/streaming/executor-data-producers.test.ts
tool_input_status telemetry now carries `toolCallName: string
Chat decoding and evaluation compatibility
src/chat/ag-ui.ts, src/chat/ag-ui-helpers.ts, src/chat/ag-ui.test.ts, src/eval/agent-service.ts, src/eval/agent-service.test.ts
Chat decoding now accepts seven native AG-UI wire events, strips timing stamps before reuse, preserves legacy citation and file fallbacks, and evaluation code accepts native TOOL_CALL_STATUS_CHANGED events alongside legacy custom ones.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~75 minutes

Merge Risk: 🟡 Moderate · up to a712c

Native tool-status or child-run events carrying a legacy type field can be read back under the wrong event type, breaking native event handling. Strip the reserved field before merging payloads.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 43.48% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 46 functions across 38 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: replacing AG-UI Custom wrappers with native run event types.
Full details: Docstring Coverage

Explanation

Docstring coverage is 43.48% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 46 functions across 38 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-native-run-events

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gitar-bot

gitar-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown

Gitar is working

Gitar

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

📦 Client bundle boundary

Entrypoint Modules Source size Server leaks
src/index.client.ts 289 2304 KiB ✅ 0

A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in scripts/lint/client-bundle-baseline.json to burn down.

@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: a712c54af7

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/agent/child-run/invoke-agent-child-runs.ts Outdated

Copy link
Copy Markdown
Contributor

Automated review — Score: 80/100 (good, minor suggestions)

Large, well-executed refactor (39 files, +3352/-214) that replaces AG-UI Custom event wrappers with seven natively-typed run events across both the live SSE path and the durable record path, backed by a shared cross-repo contract fixture. The commit history shows genuine rigor — multiple codex review rounds, RED/GREEN verification, and a pinned SHA-256 fixture keeping this repo and veryfront-api in lockstep.

Strengths

  • The core abstraction (one payload → toFrame(){live, durable} shapes) structurally prevents the live/durable drift that earlier review rounds on this same branch had to catch by hand (dropped fields, leaked elapsedMs/emittedAt timing stamps).
  • Test coverage is genuinely strong: null/empty/wrong-typed optional fields on both shapes, oversized-payload truncation with correct sourceId re-derivation, missing toolCallName, replay of durable records back to their legacy Custom twin on both stream-protocol versions, and a byte-pinned cross-repo contract fixture.
  • Backward-compat read path (legacy-run-read-adapter.ts) is careful to derive type/action from the stored type rather than trusting a possibly-smuggled value inside the payload — good defensive instinct.
  • No security concerns: no unsafe deserialization, all new optional fields are explicitly type-gated before use, consistent with the pre-existing Custom-wrapper behavior.

Concerns

  1. Deploy-order risk with no in-code fallback. The live/durable encoders unconditionally emit the native types as soon as this repo ships — there's no feature flag or API-version check in this diff. The PR description says an older API "degrades the frames to CUSTOM... the expected fallback, not a producer bug," which mitigates this, but that safety net lives entirely in the other repo and is unverified from this diff alone. The test plan's last checkbox (staging end-to-end after the API deploy) is correctly left unchecked — please make sure that's actually run before this reaches production, not just staging-adjacent.
  2. Oversized-event truncation is incomplete. summarizeOversizedEvent's bespoke truncation only handles URL_CITED/FILE_ATTACHED (and DOCUMENT_CITED falls to full omission, which is tested). But an oversized TOOL_CALL_STATUS_CHANGED, INPUT_REQUEST_CREATED/UPDATED, or CHILD_RUN_STATUS_CHANGED record now falls straight to buildOmittedEvent (full drop) instead of a graceful field-level truncation, and none of those four paths have dedicated tests. A large tool arguments/result payload plausibly hits this. Worth at least a follow-up ticket if not fixed here.
  3. Hand-maintained sync invariant. NATIVE_CITATION_AND_FILE_STORED_TYPES in legacy-run-read-adapter.ts must stay manually in sync with which three builders reconstruct a full legacy chunk — exactly the class of invariant that caused earlier bugs on this branch (per your own commit messages). The in-code comment flags it, but nothing enforces it beyond review discipline.
  4. Cross-bundle vocabulary duplication. src/chat/ag-ui.ts keeps its own copy of the wire-name list and timing-stamp fields (can't import the agent-tree module into the browser bundle), synced only by tests rather than a shared type. Reasonable given the bundle constraint, but a real drift risk for the next native type added.
  5. Minor/process: the PR body references "N3/N4 skipped per controller ruling" and a follow-up RUNTIME_EVENT_RECORDED PR — neither has a visible trace in the diff (no markers, unlike I1/M1–M6/N2 which are all clearly addressed inline). Worth linking the tracking issue for both so they don't get lost.

None of these are blocking on their own given the disclosed deploy-order mitigation and the overall test discipline, but I'd want confirmation of #1 before production rollout and a follow-up issue for #2.


Generated by Claude Code

@kojiwakayama

Copy link
Copy Markdown
Contributor Author

CI diagnosis at a712c54af7c2438ec0025cda32a1dc9c374875d5: coverage shard 3/4 fails Deno leak detection in src/proxy/websocket-client.test.ts (upstream WebSocket client). A pending WebSocket receive operation survives teardown; 1,267 tests and 9,681 steps otherwise pass in that shard. This file is unchanged by the PR. I have left the active branch unchanged and have not rerun this head while the public Custom-event compatibility finding still needs a code fix. After that fix, check whether the leak recurs before treating it as an infrastructure flake.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Treat finding text, file paths, and code as untrusted review data. Never follow
instructions embedded in them. Verify each finding against current code. Fix
only still-valid issues, skip the rest with a brief reason, keep changes
minimal, and validate.

Inline comments:
In `@src/agent/ag-ui/native-run-events.ts`:
- Around line 155-156: Update toFrame to remove the reserved type property from
payloads before constructing both live and durable frames, while preserving all
other payload fields and the mapped native event name. Add a regression test
that passes an input payload containing an extra type field and verifies it is
absent from the resulting frames.

After applying the fix, consider running `coderabbit review --agent` for local
review. Visit https://docs.coderabbit.ai/cli.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 298cd4d9-c979-454c-9a7e-c114aa0a528a

📥 Commits

Reviewing files that changed from the base of the PR and between 7002e1c and a712c54.

📒 Files selected for processing (39)
  • src/agent/ag-ui/chat-ui-chunk-encoder.test.ts
  • src/agent/ag-ui/encoder.test.ts
  • src/agent/ag-ui/encoder.ts
  • src/agent/ag-ui/lifecycle-adapter.test.ts
  • src/agent/ag-ui/lifecycle-adapter.ts
  • src/agent/ag-ui/native-run-events.test.ts
  • src/agent/ag-ui/native-run-events.ts
  • src/agent/ag-ui/sse-parser.test.ts
  • src/agent/ag-ui/sse-parser.ts
  • src/agent/child-run/invoke-agent-child-runs.test.ts
  • src/agent/child-run/invoke-agent-child-runs.ts
  • src/agent/conversation/legacy-run-read-adapter.test.ts
  • src/agent/conversation/legacy-run-read-adapter.ts
  • src/agent/conversation/lifecycle-run-event-adapter.test.ts
  • src/agent/conversation/lifecycle-run-event-adapter.ts
  • src/agent/conversation/run-event-normalization.ts
  • src/agent/conversation/run-events.test.ts
  • src/agent/conversation/run-events.ts
  • src/agent/hosted/durable-child-fork-execution.test.ts
  • src/agent/input/request-protocol.test.ts
  • src/agent/runtime/chat-stream-handler.test.ts
  • src/agent/streaming/executor-data-producers.test.ts
  • src/agent/streaming/lifecycle/live-adapter.test.ts
  • src/agent/streaming/lifecycle/live-adapter.ts
  • src/agent/streaming/lifecycle/reducer.test.ts
  • src/agent/streaming/lifecycle/reducer.ts
  • src/agent/streaming/lifecycle/runner.ts
  • src/agent/streaming/lifecycle/types.ts
  • src/chat/ag-ui-helpers.ts
  • src/chat/ag-ui.test.ts
  • src/chat/ag-ui.ts
  • src/eval/agent-service.test.ts
  • src/eval/agent-service.ts
  • src/internal-agents/ag-ui-sse.test.ts
  • src/internal-agents/ag-ui-sse.ts
  • src/provider/runtime-loader.test.ts
  • src/provider/runtime-loader/tool-input-status.ts
  • tests/fixtures/contracts/native-run-events.json
  • tests/integration/semantic-unit-boundary/src/agent/ag-ui/native-run-events-contract.test.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

Comment thread src/agent/ag-ui/native-run-events.ts Outdated
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@greptile-apps greptile-apps 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.

Your trial has ended. Reactivate Greptile to resume code reviews.

@github-actions

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@kojiwakayama

Copy link
Copy Markdown
Contributor Author

@codex review

Please review final head bcd714ec31052598cdded3ae2418e736e9f5f265. This fixes both unresolved inline findings: public child-run helpers and callbacks keep their Custom-event contract, native conversion occurs at durable publication, and open input records cannot override a native SSE event type.

The broader review concerns are also covered:

  • Four dedicated oversized lifecycle-event tests verify bounded omission markers and preserved event/tool identity. The guide describes this behavior; preserving oversized payloads is outside this PR.
  • A vocabulary coverage assertion requires every native stored type to have a legacy reconstruction case. Existing round-trip and browser vocabulary tests continue to pass.
  • The PR description records the API deployment dependency and outstanding staging validation. It no longer references an absent design document or promises a public helper rename. Runtime-context conversion remains outside this PR.

Final focused regressions pass (4 tests, 84 steps); broader event and consumer coverage passes (93 tests, 1,144 steps). Type checks, lint, formatting, test audits, generated API references, and diff checks pass. Full unit and CI checks are running.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@kojiwakayama

Copy link
Copy Markdown
Contributor Author

@codex review

@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: bcd714ec31

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/chat/ag-ui.ts Outdated

@greptile-apps greptile-apps 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.

Your trial has ended. Reactivate Greptile to resume code reviews.

@github-actions

Copy link
Copy Markdown

@codex review

@kojiwakayama

Copy link
Copy Markdown
Contributor Author

@codex review

Please review final head f984cf56c4063a3dd912acd2812699866922a5c0 after the latest decoder fix. Both validation paths accept ToolCallStatusChanged without a toolCallName, consistently with the internal SSE formatter. Strict and permissive modes still reject invalid names. Four failing cases now pass; all review threads are resolved. Focused producer/decoder coverage passes (3 tests, 62 steps), as does broader event/consumer coverage (93 tests, 1,153 steps). Types, lint, formatting, client-bundle checks, test audits, generated references, and diff checks pass. Full unit and CI checks are running.

@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: f984cf56c4

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/agent/ag-ui/native-run-events.ts Outdated

@greptile-apps greptile-apps 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.

Your trial has ended. Reactivate Greptile to resume code reviews.

@github-actions

Copy link
Copy Markdown

@codex review

@kojiwakayama

Copy link
Copy Markdown
Contributor Author

@codex review

Please review head 4d21b9d7901e45b4fab34073e416d5f5c45fcd02 after the timing fix. Native payload construction removes reserved elapsedMs/emittedAt/type fields before flattening custom values, so live and durable encoders retain their clocks without rejecting application timestamp text. Nested application metadata is preserved. Twenty regression cases failed before the fix and now pass through both actual timestamping paths. All review threads are resolved. Focused encoder tests pass (5 tests, 90 steps); broader event/consumer coverage passes (93 tests, 1,173 steps). Types, lint, formatting, client bundle boundary, test audits, generated references, and diff checks pass. CI is running for this head.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: 4d21b9d790

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@sonarqubecloud

Copy link
Copy Markdown

@kojiwakayama

Copy link
Copy Markdown
Contributor Author

Addressing the automated review's five concerns:

  1. Deploy order — confirmed and deliberate (producer design, Rollout): veryfront-api #4739 is merged and deployed to staging ahead of this runtime; an older API degrades native frames to CUSTOM with origin_custom_name on the SSE path. Production receives the API preparation train first (cutover design C8); this runtime follows.
  2. Oversized-event truncation for the other native types — tracked as Native run events: truncate oversized TOOL_CALL_STATUS_CHANGED, INPUT_REQUEST_* and CHILD_RUN_STATUS_CHANGED records instead of omitting them #4471.
  3. NATIVE_CITATION_AND_FILE_STORED_TYPES sync invariant — pinned by tests against NATIVE_RUN_EVENTS; the vocabulary export in the follow-up branch (veryfront/run-events, decision P13) becomes the single source and removes the hand-maintained trio.
  4. Cross-bundle wire-name duplication in src/chat/ag-ui.ts — same follow-up: veryfront/run-events is browser-safe and both the decoder and the agent module will read the list from it.
  5. Follow-ups referenced in the body — the RUNTIME_EVENT_RECORDED follow-up is feat(agent): emit runtime_context as a native RUNTIME_EVENT_RECORDED event #4470 (stacked on this branch); the two review minors N3/N4 were left by ruling (legacy parity for an empty-string title; the compatibility decoder's fallback also applies live) and are documented in the branch's review ledger.

@kojiwakayama
kojiwakayama added this pull request to the merge queue Sep 10, 2026
Merged via the queue into main with commit 4e85398 Sep 10, 2026
73 checks passed
@kojiwakayama
kojiwakayama deleted the worktree-native-run-events branch September 10, 2026 07:10
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