fix(drift): declare metadata on the base event, and read canonical optionality correctly - #387
Merged
Conversation
Canonical AG-UI declares `metadata` once on BaseEventSchema, so every event type carries it. AGUIBaseEvent did not, which the AG-UI schema drift test reported against all 33 event types.
… defs
The canonical parser decided optionality by substring-matching
`.optional()` in a field's own text, on a line-by-line basis. Two
upstream spellings defeat that:
- a named alias (`metadata: OptionalMetadataSchema`, which resolves to
`MetadataSchema.optional()` in a sibling module) carries no literal
`.optional()`, so it read as required and every event type was
reported CRITICAL;
- a definition split across lines (`parentMessageId`, `outcome`)
matched only its first line, `z`, and read as required, producing
false optionality-mismatch warnings.
Resolve bare identifiers against the exported schemas of the canonical
core package, bounded against alias cycles, and cut fields on top-level
commas so a chain spanning lines is read whole.
commit: |
contextablemark
added a commit
that referenced
this pull request
Aug 30, 2026
…tRunId (#391) `Drift Tests` has been red on `main` since Aug 26. Of the criticals, **26 are AG-UI schema drift** — upstream ag-ui added the subagent lifecycle and aimock's types never followed. (The Gemini model-family criticals in the same run are a separate concern and are not touched here.) ## What upstream added Three new members on `EventType` — `SUBAGENT_STARTED`, `SUBAGENT_FINISHED`, `SUBAGENT_ERROR` — plus a `subagentRunId` correlation field threaded through every event a subagent can emit. ## `subagentRunId` is per-event, not a base field Worth stating explicitly, because the cheap fix is wrong. The drift report lists `subagentRunId` against 23 event types, which reads like a base-event field — and declaring it once on `AGUIBaseEvent` would have cleared all 23 at once, the way `metadata` was cleared in #387. But canonical does not put it on `BaseEventSchema`. Reading `events.ts` schema by schema: - **24 events** declare `subagentRunId: z.string().optional()` - **3 subagent events** declare it required, `z.string()` - **7 events deliberately omit it**: `RUN_STARTED`, `RUN_FINISHED`, `RUN_ERROR`, `MESSAGES_SNAPSHOT`, and the four deprecated `THINKING_*` events A base-event declaration would have gone green while putting the field on seven events canonical does not give it. So it is mirrored per event. (24 optional, not the 23 reported — `STATE_DELTA` was missing from the report for a separate reason, below.) ## Changes - **`src/agui-types.ts`** — `subagentRunId?: string` on the 24 events canonical marks optional; three new event interfaces (`AGUISubagentStartedEvent` / `Finished` / `Error`) with it required; `AGUISubagentFinishedOutcome` mirroring `AGUIRunFinishedOutcome` one level down; union and `AGUIEventType` members. - **`src/__tests__/drift/agui-schema.drift.ts`** — strip trailing comments in the canonical parser, plus a regression test. ## The parser bug this surfaced The canonical parser stripped whole-line comments only. A trailing comment survives, and since entries are cut on top-level commas it then *leads the next entry*, whose field-name match fails — the field is dropped silently. Upstream writes exactly that on `STATE_DELTA`: ```ts delta: z.array(z.any()), // JSON Patch (RFC 6902) subagentRunId: z.string().optional(), ``` So canonical `STATE_DELTA.subagentRunId` was invisible: it never appeared in the 23 criticals, and once declared it flipped to a false `exists in aimock but not in canonical` warning. Same class as the two parser mis-readings fixed in #387. Stripping is safe here — no canonical schema literal contains `//`. ## Red-green proof⚠️ **The local `../ag-ui` sibling this test resolves was stale (Aug 22, no subagent events at all), and against it the suite passes 15/15 — a false green.** Both runs below use an isolated, freshly-cloned canonical checkout, the same thing CI clones. - aimock: `5e3b500` (red) → `6ecdcce` (green) - **ag-ui: `363d3878e30887e88c1fd5ca1916ec3a5962b6be`** - Positive control: `grep -c SUBAGENT` on the fresh clone's `events.ts` = 9; on the stale sibling = **0**. Command, identical for both: ``` npx vitest run --config vitest.config.drift.ts ``` **RED** — at `5e3b500`, unmodified: ``` EXIT=1 Test Files 1 failed | 18 passed | 7 skipped (26) Tests 2 failed | 132 passed | 57 skipped (191) [CRITICAL] Event type "SUBAGENT_STARTED" exists in canonical @ag-ui/core but is missing from aimock AGUIEventType [CRITICAL] Event type "SUBAGENT_FINISHED" ... [CRITICAL] Event type "SUBAGENT_ERROR" ... [CRITICAL] TEXT_MESSAGE_START: field "subagentRunId" (optional) exists in canonical but missing from aimock ... × 23 event types ``` 26 criticals: 3 event types + `subagentRunId` × 23. **GREEN** — this branch: ``` EXIT=0 Test Files 19 passed | 7 skipped (26) Tests 135 passed | 57 skipped (192) ``` 0 criticals. One warning remains and is pre-existing and genuine — canonical `TEXT_MESSAGE_START.role` is `.default("assistant")` where aimock requires it, same one #387 left as-is. The false `STATE_DELTA` warning is gone. ## Guards mutation-tested | Mutation | Result | | --- | --- | | Drop `subagentRunId` from `agui-types.ts` | EXIT=1, criticals return | | Drop the 3 `SUBAGENT_*` members from `AGUIEventType` | EXIT=1, all 3 event-type criticals return | | Revert the trailing-comment strip | EXIT=1, the new regression test fails | ## Other checks `tsc --noEmit` clean · `eslint .` clean · `prettier --check` clean · `tsdown` build clean · full unit suite **5280 passed | 46 skipped (5326)**, EXIT=0. ## Note on CI The `drift` job is gated `if: github.event_name != 'pull_request'`, so **a green PR here does not exercise it** — hence the local proof above. `agui-schema-drift` does run on PRs and covers the change in this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Drift Tests have failed on
mainevery night since Aug 19 (green through Aug 18). Both failing jobs share one root cause —driftis red only because it aggregatesagui-schema-drift.What happened
Upstream ag-ui added
metadatatoBaseEventSchema(feat(core): declare metadata and the merge primitive), which landed on ag-uimainbetween the Aug 18 green run and the Aug 19 red run.Two separate defects fell out of that:
1. Real drift.
AGUIBaseEventhadtype/timestamp/rawEventand nometadata, so aimock was genuinely behind canonical. The test was right to complain.2. The severity was wrong. It reported
field "metadata" (required)and stamped 33 × CRITICAL. Upstream declares it optional:The canonical parser decided optionality by substring-matching
.optional()in a field's own text, line by line. A named alias carries no literal.optional(), so it read as required — turning "we're missing an optional field" into 33 CRITICALs, which is what forced quarantine and the hardexit 1.The same line-by-line reading also mis-read definitions split across lines (
parentMessageId,outcome), emitting false optionality-mismatch warnings.Changes
src/agui-types.ts— declaremetadata?: Record<string, unknown>onAGUIBaseEvent. The aimock-side parser folds base fields into every event interface, so this clears all 33 findings at once.src/__tests__/drift/agui-schema.drift.ts— resolve bare identifiers against the exported schemas of the canonical core package (bounded against alias cycles), and cut fields on top-level commas so a chain spanning lines is read whole. Plus 7 regression tests, which run without the ag-ui checkout.Red-green proof
Run locally against a fresh
ag-uiclone atmain, using the exact command CI runs:RED — at
62caaa2, the commit CI failed on:GREEN — same command, this branch:
The three false warnings (
parentMessageId×2,outcome) are also gone. One warning remains and is genuine: canonicalroleis.default("assistant")where aimock requires it — a real, non-critical difference, left as-is.Guards mutation-tested
Each new test was confirmed to fail when its fix is reverted, so none are vacuous:
metadataonAGUIBaseEventOther checks
eslintclean ·tsc --noEmitclean ·tsdownbuild clean · full unit suite 5326/5326.One thing worth flagging separately
On one full-suite run, two tests failed —
aimock-cli.test.ts > applies config auth and lets AIMOCK_API_KEYS override itandcli.test.ts > handles a SIGTERM delivered at or after the instant readiness is announced. They pass isolated, and pass on two subsequent full-suite runs on this branch; a clean-tree full run was also green. They look load-sensitive rather than related to this change, but they are flaky under parallel load and worth a separate look.