fix(agui): mirror the canonical subagent lifecycle events and subagentRunId - #391
Merged
Conversation
…tRunId Upstream ag-ui added SUBAGENT_STARTED / SUBAGENT_FINISHED / SUBAGENT_ERROR and threaded an optional `subagentRunId` through the events a subagent can emit, so aimock's AG-UI types were behind canonical. `subagentRunId` is declared per event, not on the base event: canonical puts it on 24 event schemas as optional and on the three subagent events as required, and deliberately omits it from RUN_STARTED / RUN_FINISHED / RUN_ERROR / MESSAGES_SNAPSHOT and the four deprecated THINKING_* events. Declaring it once on AGUIBaseEvent would have cleared the drift report but put the field on seven events that canonical does not give it.
…pped 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 `delta: z.array(z.any()), // JSON Patch (RFC 6902)` on STATE_DELTA, which hid canonical STATE_DELTA.subagentRunId: it went unreported as missing, then read as an aimock-only field once declared.
commit: |
contextablemark
approved these changes
Aug 30, 2026
contextablemark
left a comment
contextablemark
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the scoped AG-UI event-schema parity and trailing-comment parser regression against canonical upstream. The reviewed head is 6ecdcce; all active checks are green.
contextablemark
added a commit
that referenced
this pull request
Aug 31, 2026
…ges and interrupts, and stop the drift collector reporting clean when it cannot see (#393) 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 `main` currently has the three `SUBAGENT_*` 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 #391 is unreachable from outside the package. - `AGUIMessage` and `AGUIInterrupt` never got `subagentRunId`, though canonical declares it on both — `types.ts:28` (`BaseMessageSchema`) and `types.ts:224` (`InterruptSchema`). - The AG-UI drift CI lane ran a single hardcoded filename, so any drift guard added later would silently never run. - The report collector **dropped any AG-UI failure it could not pattern-match**, with no counter and no quarantine — a real assertion in the existing `agui-schema.drift.ts` produced exit 0 and `conclusion: "clean"`. That is pre-existing and independent of #391. ## Changes **Public surface** - Export `AGUISubagentStartedEvent`, `AGUISubagentFinishedEvent`, `AGUISubagentErrorEvent`, `AGUISubagentFinishedOutcome` from **both** barrels — `src/index.ts` and `src/agui-stub.ts` (the `./agui` subpath). The second barrel was missing them too. - Mirror canonical optional `subagentRunId` onto `AGUIMessage` and `AGUIInterrupt`. **Guard against recurrence** - `src/__tests__/agui-barrel-exports.test.ts` asserts every type declared in `agui-types.ts` is 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** - The AG-UI drift lane and the collector now select by path prefix `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-burning `drift` leg is untouched. **Collector fail-closed** - An AG-UI failure the collector cannot structurally interpret now **quarantines** (existing exit-5 lane, `CollectResult {entries, quarantine}` — the same shape the HTTP leg already returns) instead of vanishing. Per-assertion and ungated. - A failed assertion with empty `failureMessages` no longer reads as clean. - Unparseable AG-UI stdout on a zero-exit run now throws, matching the HTTP twin, instead of returning `{testResults: []}`. - `classifyAgUiCheckout` verifies the canonical `types.ts` actually exists rather than trusting a directory named `ag-ui`, so a stale clone reports as stale instead of as a git/network failure. ## Evidence Barrel exports — before: ``` error TS2724: '"./src/index.js"' has no exported member named 'AGUISubagentStartedEvent' ``` 8 such errors across both barrels; clean after. CI lane — with `subagentRunId` deleted from `AGUIMessage`, the **old** lane command still passed, exit 0. The guard was provably inert on the server. After: exit 1. Collector — mutating `agui-schema.drift.ts` to produce an uninterpretable real failure: ``` before: entries 0 / quarantined 0 / exit 0 / conclusion "clean" after: QUARANTINE= 1 ... EXITCODE= 5 CONCLUSION= quarantine ``` Gates: `tsc --noEmit`, `eslint`, `prettier --check`, `tsdown` all exit 0; full suite 179 files / 5367 tests. ## Known gaps — deliberate, not oversights - **`subagentRunId` on `AGUIMessage`/`AGUIInterrupt` ships 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 legacy `agui-schema.drift.ts`, so one differ gets built once. The field is correct as merged; nothing will catch its removal until then. - **The `SUBAGENT_*` types are declarative only.** No builder in `agui-handler.ts` emits them and no build option accepts `subagentRunId`, 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) - Rewrite `agui-schema.drift.ts` onto the TypeScript AST — same root cause as everything above; the barrel guard's readers are liftable. - The drift **reporting** layer: the base-report reuse path is dead for three independent reproduced reasons (bare `node` importing a `.ts` module, missing `actions: read`, and a `--status=success` filter 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-level `env:`, exposed to `pnpm install` lifecycle scripts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01AvkmhXVLqSSEW6FvPQHSu5
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.
Drift Testshas been red onmainsince 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 asubagentRunIdcorrelation field threaded through every event a subagent can emit.subagentRunIdis per-event, not a base fieldWorth stating explicitly, because the cheap fix is wrong. The drift report lists
subagentRunIdagainst 23 event types, which reads like a base-event field — and declaring it once onAGUIBaseEventwould have cleared all 23 at once, the waymetadatawas cleared in #387.But canonical does not put it on
BaseEventSchema. Readingevents.tsschema by schema:subagentRunId: z.string().optional()z.string()RUN_STARTED,RUN_FINISHED,RUN_ERROR,MESSAGES_SNAPSHOT, and the four deprecatedTHINKING_*eventsA 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_DELTAwas missing from the report for a separate reason, below.)Changes
src/agui-types.ts—subagentRunId?: stringon the 24 events canonical marks optional; three new event interfaces (AGUISubagentStartedEvent/Finished/Error) with it required;AGUISubagentFinishedOutcomemirroringAGUIRunFinishedOutcomeone level down; union andAGUIEventTypemembers.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:So canonical
STATE_DELTA.subagentRunIdwas invisible: it never appeared in the 23 criticals, and once declared it flipped to a falseexists in aimock but not in canonicalwarning. Same class as the two parser mis-readings fixed in #387. Stripping is safe here — no canonical schema literal contains//.Red-green proof
../ag-uisibling 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.5e3b500(red) →6ecdcce(green)363d3878e30887e88c1fd5ca1916ec3a5962b6begrep -c SUBAGENTon the fresh clone'sevents.ts= 9; on the stale sibling = 0.Command, identical for both:
RED — at
5e3b500, unmodified:26 criticals: 3 event types +
subagentRunId× 23.GREEN — this branch:
0 criticals. One warning remains and is pre-existing and genuine — canonical
TEXT_MESSAGE_START.roleis.default("assistant")where aimock requires it, same one #387 left as-is. The falseSTATE_DELTAwarning is gone.Guards mutation-tested
subagentRunIdfromagui-types.tsSUBAGENT_*members fromAGUIEventTypeOther checks
tsc --noEmitclean ·eslint .clean ·prettier --checkclean ·tsdownbuild clean · full unit suite 5280 passed | 46 skipped (5326), EXIT=0.Note on CI
The
driftjob is gatedif: github.event_name != 'pull_request', so a green PR here does not exercise it — hence the local proof above.agui-schema-driftdoes run on PRs and covers the change in this PR.