fix(agui): export the subagent types, mirror subagentRunId onto messages and interrupts, and stop the drift collector reporting clean when it cannot see - #393
Merged
Conversation
The subagent lifecycle types added alongside SUBAGENT_STARTED / SUBAGENT_FINISHED / SUBAGENT_ERROR were declared in src/agui-types.ts but never re-exported, so no consumer of the package root or of the "./agui" subpath could name them. Export AGUISubagentStartedEvent, AGUISubagentFinishedEvent, AGUISubagentErrorEvent and AGUISubagentFinishedOutcome from both barrels, and add a public-surface test that fails whenever any agui-types.ts type is missing from either.
`AGUIMessage` and `AGUIInterrupt` dropped the optional `subagentRunId` that `@ag-ui/core` declares on `BaseMessageSchema`, the three standalone message schemas, and `InterruptSchema`. A replayed message or approval request lost the attribution that lets a client group it under the subagent that produced it, and read as root-raised instead. The existing AG-UI drift suite only walks `*EventSchema` against `AGUI*Event` interfaces, so it could not see this. Adds `agui-nonevent-schema.drift.ts`, which compares the canonical non-event schemas that carry `subagentRunId` against the aimock interfaces that mirror them, plus a type-level guard. Scoped to `subagentRunId`; broader non-event field drift is tracked separately. Note: tsconfig.json excludes src/__tests__, so the type-level guard is not covered by `pnpm build` and must be typechecked explicitly.
The barrel guard split export blocks on commas before stripping comments, so a comma inside a comment dropped a real exported name and injected a bogus one; and it recorded `X as Y` under `X`, so renaming a re-export defeated the reachability check the guard exists to perform. Read src/index.ts, src/agui-stub.ts and src/agui-types.ts with the TypeScript parser instead. Comments are trivia and a rename is a distinct propertyName/name pair, so both failure modes are handled by construction. A name is counted as reachable only when its local and exported names match. Star re-exports are recognised as publishing everything. The reader itself is now unit-tested on the shapes that defeat a regex, and the non-empty positive control against a total parse failure is retained.
…rroring The branch adds four public type exports and mirrors canonical's optional subagentRunId onto 24 events, AGUIMessage and AGUIInterrupt, with no [Unreleased] entry — against the repo's own convention. Adds one Added bullet matching the depth of the 1.39.0 AGUITokenUsage entry (the same kind of change), and one Fixed bullet for the drift parser's trailing- comment bug that made the report undercount. States plainly that the new types are declarative only: agui-handler.ts gained no builder, so aimock can type a subagent event but not emit one.
The `agui-schema-drift` job and `runAgUiDriftTests()` in the drift report collector both ran a single hardcoded path, `src/__tests__/drift/agui-schema.drift.ts`. The default vitest config only includes `*.test.ts`, so `pnpm test` never picks up `*.drift.ts` either — meaning `agui-nonevent-schema.drift.ts` was executed by no CI job at all: a guard that could never fail server-side. Both consumers now pass the vitest filename filter `src/__tests__/drift/agui-` instead, so every current and future `agui-*.drift.ts` guard runs. `vitest.config.drift.ts` already scopes `include` to the drift-suffixed files, so the filter cannot pull in plain `.test.ts` helpers from the same directory. The lane still needs no provider API keys — the AG-UI drift files compare `src/agui-types.ts` against the cloned canonical ag-ui repo only. The `paths:` trigger entry is generalised to `agui-*.drift.ts` to match.
…pping The availability check lived inside the `describe.skipIf` it was meant to guard, so a missing ag-ui checkout skipped the assertion that would have reported it and the file exited 0 while comparing nothing. The gate now lives outside the skipped block: an absent or unreadable canonical checkout fails, and the local escape hatch (AIMOCK_ALLOW_MISSING_AGUI_CHECKOUT=1) is inert whenever CI is set, so no automated lane can reach a silent pass. The mirror loop also `continue`d past any canonical schema that did not declare `subagentRunId`, so an upstream rename -- the drift this file exists to catch -- reported clean for the four message mirrors. The canonical side is now asserted and a missing declaration is reported as drift. Drop the type-level companion block: it asserted nothing at runtime and the `tsc` command its comment documented was wired into no script and no CI lane. Run the file in test-drift.yml's AG-UI job, which previously named only the older agui-schema.drift.ts and so never executed this one.
# Conflicts: # .github/workflows/test-drift.yml
…, and record drift lane change
The non-event subagentRunId guard hand-parsed TypeScript with regexes and a brace counter, and every way that reader failed was a fail-open failure: - the brace counter ignored comments and strings, so a brace inside a comment ran the reader past the end of the schema it was reading; - the canonical-side check matched subagentRunId against the whole nested body, so a nested occurrence masked a top-level removal; - MIRRORS was a hardcoded list with no completeness check, so a new canonical schema carrying the field went silently uncompared. It also pinned the aimock side to the literal text subagentRunId?: string;, reporting false drift on a reformat, a readonly, or a widened type; and gated its local escape hatch on CI === "true", so a runner setting CI=1 reached an opt-out meant to be unreachable in CI. Both sides are now read with ts.createSourceFile (syntax-only, no program), following the pattern already established in agui-barrel-exports.test.ts. Members are read structurally, so comments and strings are trivia and a nested member is distinguishable from a top-level one by construction; the aimock mirror is compared by name, optionality and type under a stated widening rule (must admit absence/undefined and a bare string; extra union members are an acceptable widening, narrowing is drift). The canonical side of MIRRORS is derived and asserted equal to the hand-written list, so a new or renamed canonical schema fails loudly. The CI gate now treats any non-negating value as truthy, and the readers have direct unit tests.
…ailure The AG-UI leg of the drift collector recognized exactly three failure-message shapes and DROPPED everything else — no entry, no counter, no warning. A genuinely failing AG-UI guard whose message matched none of them therefore produced zero entries, exit 0 and conclusion "clean": the collector reported a clean baseline for a run it could not see. Every unrecognized FAILED AG-UI assertion is now returned as a QuarantineEntry, reusing the collector's existing quarantine / exit-5 lane rather than inventing a parallel scheme. Unlike the HTTP leg's entries.length === 0 gated rescue, this is per-assertion and unconditional, so an uninterpretable failure survives a mixed run in which other failures did parse. ensureAgUiRepo also accepted any directory merely NAMED ag-ui. A stale or partial checkout passed that check, the drift suites' describe.skipIf gates then skipped everything, and the run was certified clean. classifyAgUiCheckout now requires the canonical sdks/typescript/packages/core/src/types.ts and reports a stale/incomplete checkout as exactly that — not as a git/network failure — which routes to the AG-UI-skipped lane (exit 1) instead of clean.
…r fail-opens
The hand-rolled schema differ in `src/__tests__/drift/agui-nonevent-schema.drift.ts`
produced a fail-open bug in each of three consecutive review rounds. It is
withdrawn rather than patched a fourth time; the follow-up that rewrites the
legacy drift harness will build one differ properly, once. Its
`AGUI_NONEVENT_FAILURE` collector fixture goes with it — it was documented as a
verbatim vitest capture but carried 2 of 4 drift lines and the wrong frame, so
it could not be trusted in any form. The fail-closed test it anchored now runs
on `AGUI_UNPARSEABLE_SCHEMA_FAILURE`, a real capture from the surviving
`agui-schema.drift.ts`, which is the guard the invariant actually protects.
Two fail-opens that outlive the withdrawal are fixed:
- `collectAgUiDriftEntries` skipped a FAILED assertion whose `failureMessages`
was empty, before it was even looked at — zero entries, exit 0,
`conclusion: "clean"`. A failure with no message is the least interpretable
failure there is, so it now takes the same quarantine lane (exit 5) as an
unrecognized message. The test that PINNED the old behavior pinned a
fail-open; it now pins the quarantine.
- A ZERO-exit AG-UI run whose stdout is not vitest JSON returned
`{ testResults: [] }`, which the collector reads as "no failures", while the
HTTP twin `runDriftTests()` throws on the identical condition. The AG-UI leg
now throws too. The run/parse half is extracted as the exported
`runAgUiVitest(exec)` so that contract is testable without spawning vitest.
The barrel-export guard, `subagentRunId` on `AGUIMessage`/`AGUIInterrupt`, the
CI lane's `agui-` path-prefix filter and naming contract, and the collector's
fail-closed invariant are all untouched. `subagentRunId` on the two non-event
types now ships with NO regression test — the CHANGELOG says so plainly rather
than implying a guard exists.
commit: |
contextablemark
approved these changes
Aug 31, 2026
contextablemark
left a comment
contextablemark
left a comment
Contributor
There was a problem hiding this comment.
I dug through the collector edge cases and checked them against the documented gaps and CI paths. Nothing here should hold the merge — approved.
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
2 participants
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.
Follow-up to #391, which merged at its pre-review head. The type mirror landed, but the review that was running against it had not reported yet — so
maincurrently has the threeSUBAGENT_*event types and no way for a consumer to import them. This carries the review fixes.What was actually broken on main
git show origin/main:src/index.ts | grep -c Subagent→ 0. The new event types are exported from neither barrel, so the headline benefit of fix(agui): mirror the canonical subagent lifecycle events and subagentRunId #391 is unreachable from outside the package.AGUIMessageandAGUIInterruptnever gotsubagentRunId, though canonical declares it on both —types.ts:28(BaseMessageSchema) andtypes.ts:224(InterruptSchema).agui-schema.drift.tsproduced exit 0 andconclusion: "clean". That is pre-existing and independent of fix(agui): mirror the canonical subagent lifecycle events and subagentRunId #391.Changes
Public surface
AGUISubagentStartedEvent,AGUISubagentFinishedEvent,AGUISubagentErrorEvent,AGUISubagentFinishedOutcomefrom both barrels —src/index.tsandsrc/agui-stub.ts(the./aguisubpath). The second barrel was missing them too.subagentRunIdontoAGUIMessageandAGUIInterrupt.Guard against recurrence
src/__tests__/agui-barrel-exports.test.tsasserts every type declared inagui-types.tsis re-exported from both barrels. It parses with the TypeScript compiler API, not regexes, and records a name only when it is reachable under its declared name — so a renamed re-export (X as Y) cannot pass, by construction.CI lane
src/__tests__/drift/agui-with a documented naming contract, instead of one hardcoded filename. A future AG-UI guard is picked up automatically rather than orphaned. The lane still requires no provider API keys; the credit-burningdriftleg is untouched.Collector fail-closed
CollectResult {entries, quarantine}— the same shape the HTTP leg already returns) instead of vanishing. Per-assertion and ungated.failureMessagesno longer reads as clean.{testResults: []}.classifyAgUiCheckoutverifies the canonicaltypes.tsactually exists rather than trusting a directory namedag-ui, so a stale clone reports as stale instead of as a git/network failure.Evidence
Barrel exports — before:
8 such errors across both barrels; clean after.
CI lane — with
subagentRunIddeleted fromAGUIMessage, the old lane command still passed, exit 0. The guard was provably inert on the server. After: exit 1.Collector — mutating
agui-schema.drift.tsto produce an uninterpretable real failure:Gates:
tsc --noEmit,eslint,prettier --check,tsdownall exit 0; full suite 179 files / 5367 tests.Known gaps — deliberate, not oversights
subagentRunIdonAGUIMessage/AGUIInterruptships without a regression test. A non-event drift guard was written for this PR and withdrawn: three review rounds found fail-open defects in it (most seriously, it read.omit({subagentRunId: true})— a schema removing the field — as declaring it, an idiom canonical already uses). Rather than a fourth patching round on a hand-rolled schema differ, it is deferred to the follow-up that rewrites the legacyagui-schema.drift.ts, so one differ gets built once. The field is correct as merged; nothing will catch its removal until then.SUBAGENT_*types are declarative only. No builder inagui-handler.tsemits them and no build option acceptssubagentRunId, so aimock can type a subagent event but cannot emit one. The CHANGELOG says so explicitly rather than implying reach.Follow-ups filed (not in scope here)
agui-schema.drift.tsonto the TypeScript AST — same root cause as everything above; the barrel guard's readers are liftable.nodeimporting a.tsmodule, missingactions: read, and a--status=successfilter that cannot fire on days main drifted), so every PR pays a full fresh live provider run; infrastructure failures are Slacked as "providers changed response formats"; and seven provider secrets sit in job-levelenv:, exposed topnpm installlifecycle scripts.🤖 Generated with Claude Code
https://claude.ai/code/session_01AvkmhXVLqSSEW6FvPQHSu5