AgentKit Option C: AG-UI wire format with a typed extension profile - #4612
AgentKit Option C: AG-UI wire format with a typed extension profile#4612builder-io-integration[bot] wants to merge 1 commit into
Conversation
![builder-io-integration[bot]](https://avatars.githubusercontent.com/in/353721?s=60&v=4){kind=small}
There was a problem hiding this comment.
Builder reviewed your changes and found 6 potential issues 🔴
Review Details
Code Review Summary
PR #4612 replaces AgentKit's SSE envelope with AG-UI frames plus a Builder-owned metadata/profile extension, adds codec conformance checks, and translates approval continuation to AG-UI-style resume entries. The overall direction is coherent: the 1:1 frame rule preserves replay cursors, the profile keeps the domain event union typed, and the conformance coverage catches several mapping classes. However, this is high risk because it changes a public wire/API contract and approval lifecycle while retaining the existing protocol version.
Key Findings
🔴 HIGH
- Existing v1 peers can negotiate successfully but cannot parse the new incompatible SSE encoding.
- Removing
resolveApprovaland its route is a breaking public transport change shipped as a patch. - Cancelled resume entries can be interpreted inconsistently, including an approval payload being accepted as approval.
🟡 MEDIUM
- Formatted message deltas lose their optional format.
- Foreign AG-UI frames with opaque SSE IDs abort the stream before they can be skipped.
- Malformed profile/native combinations and residual payloads bypass domain validation.
The implementation has good explicit-failure behavior for unreadable Builder frames and strong round-trip/conformance tests, but the compatibility and validation gaps should be addressed before merge.
🧪 Browser testing: Will run after this review (PR touches shared client/chat protocol code)
| ); | ||
| } | ||
| const event = parseAgentEvent(envelope.payload, "envelope.payload"); | ||
| const event = decodeAgUiEvent(JSON.parse(data) as unknown, context); |
There was a problem hiding this comment.
🔴 Negotiate a wire version for the AG-UI SSE encoding
The SSE payload changed from AgentKit v1 envelopes to AG-UI frames, but the negotiated AgentKit protocol version remains v1. A new client accepts an old v1 server with no profile header and then fails decoding its legacy envelope; an old client likewise cannot parse the new frames. Introduce a new negotiated version or retain the legacy encoding for v1 instead of advertising compatibility for incompatible peers.
| /** | ||
| * Answers the interrupts that ended a run and starts the run that carries the | ||
| * work forward. An interrupt terminates its run under AG-UI semantics, so | ||
| * resolution produces a new run to subscribe to rather than resuming a | ||
| * stream that is already closed. | ||
| */ | ||
| resumeRun?( |
There was a problem hiding this comment.
🔴 Preserve the legacy approval transport during migration
This removes the public optional resolveApproval operation and replaces it with a different request/response contract, while the old HTTP route is removed too. Existing transport implementations and independently deployed clients will fail on a patch upgrade; retain a deprecated bridge/route or ship this as an explicitly breaking, coordinated release.
| return requireRecord(entry.payload, "resume.payload"); | ||
| } | ||
|
|
||
| export function approvalResponseFromResume( |
There was a problem hiding this comment.
🔴 Honor cancellation status when converting resume entries
approvalResponseFromResume ignores entry.status and derives the decision only from payload.decision. A cancelled entry without a payload is rejected, while a contradictory cancelled entry containing decision: "approve" can continue the Core turn as approved. Handle cancellation as an explicit denial/cancel path and reject status/decision conflicts before continuing the protected operation.
| id: event.id, | ||
| at: event.occurredAt, | ||
| meta: event.metadata, | ||
| residual: isLosslessEventType(type) ? undefined : payloadOf(event), |
There was a problem hiding this comment.
🟡 Preserve formatted message deltas in the residual
message.delta is unconditionally treated as lossless, but the domain event permits an optional format and the native AG-UI projection carries only messageId and delta. The decoder never restores format, so formatted streaming responses silently round-trip without it. Preserve a residual when format is present or encode/decode the field explicitly.
| ); | ||
| } | ||
| const event = parseAgentEvent(envelope.payload, "envelope.payload"); | ||
| const event = decodeAgUiEvent(JSON.parse(data) as unknown, context); |
There was a problem hiding this comment.
🟡 Skip foreign frames before validating AgentKit cursor IDs
The parser validates every SSE id: as a numeric AgentKit cursor before determining whether the AG-UI frame belongs to this profile. Foreign producers may use opaque IDs, so a valid foreign frame aborts the stream instead of being skipped. Decode/classify first and enforce numeric ID and sequence checks only for frames carrying the Builder profile.
| } | ||
|
|
||
| if (extension.residual !== undefined) { | ||
| return { ...base, type: extension.t, ...extension.residual } as AgentEvent; |
There was a problem hiding this comment.
🟡 Validate reconstructed profile events before yielding them
Residual native/profile payloads are returned with a type assertion after only AG-UI schema validation. A syntactically valid frame can declare one Builder event type while carrying incompatible fields, and direct transport consumers receive malformed AgentEvent values; the prior HTTP boundary parsed each domain event. Verify the native frame type matches the declared profile type and run domain validation on every reconstructed event before advancing the cursor.
cba7e22 to
7cdf4f0
Compare
… profile Implements Option C from the AgentKit protocol evaluation: AG-UI (@ag-ui/core 0.0.59) becomes the wire format, and the Builder events it has no counterpart for travel as a versioned typed profile over CUSTOM. The domain AgentEvent survives in memory and is rebuilt by a Builder-side parser, so the client reducer, its exhaustive switch, and the React layer are untouched. Encoding holds one domain event to one AG-UI frame, which keeps the sequence contract, gap rejection, and reconnect cursor intact. Sequencing, event ids, and profile version are defined as explicit extensions because AG-UI specifies none of them; they ride in one owned metadata key, since @ag-ui/core reserves "ag-ui" for itself. Approvals move to AG-UI's interrupt model: the transport exposes resumeRun with resume entries, replacing resolveApproval and its bespoke route. Conformance now asserts that every event from Core's adapter round-trips through AG-UI unchanged and that each frame parses under AG-UI's own schemas.
7cdf4f0 to
6c6a2b6
Compare
dbf2c2d to
ed19335
Compare
There was a problem hiding this comment.
Builder reviewed your changes and found 6 potential issues 🔴
Review Details
Incremental Code Review Summary
The latest commit updates the repository layout and continues the AG-UI codec, resumeRun migration, and Core adapter integration. The previously reported issues remain open in the latest code, so they were not reposted or resolved. This incremental review found additional lifecycle, release, and interoperability problems. Risk remains high because the change spans a public wire protocol, approval authorization flow, runtime package boundaries, and reconnect/run state.
New Findings
🔴 HIGH
- The changeset publishes only
@agent-native/core, although the new AgentKit package exports and dependency contract are required at runtime. A Core patch can therefore load against an older AgentKit package and fail on missing exports.
🟡 MEDIUM
- A distinct resumed run leaves the interrupted run permanently active in client state.
- AG-UI interrupt frames decode as approval events without preserving the terminal lifecycle, causing standards-conforming closed streams to fail with
run_missing_terminal. - The conformance scenario discards the resumed run ID and cannot validate transports that create a new run.
- Missing profile headers are accepted and can produce an apparently successful empty stream.
- Explicit approval denials are encoded as AG-UI cancellation rather than a resolved negative decision.
The existing conformance and focused tests passing is useful, but several cases are masked by the Core adapter's same-run suspension behavior and do not exercise an independent AG-UI runtime creating a new run.
🧪 Browser testing: Will run after this review (PR touches shared client/chat protocol code)
| @@ -0,0 +1,12 @@ | |||
| --- | |||
| "@agent-native/core": patch | |||
There was a problem hiding this comment.
🔴 Release the changed AgentKit package with the Core update
This changeset releases only @agent-native/core, although the implementation adds runtime imports (approvalResponseFromResume and resumeOptionId) and the new @ag-ui/core dependency to the publishable AgentKit package. Core declares AgentKit as workspace:^, so a Core patch can resolve an already-published AgentKit version whose protocol entrypoint does not export these symbols, causing ESM loading to fail. Add an AgentKit release entry with the appropriate versioning so Changesets publishes the required package together with Core.
| const key = this.runKey(input.threadId, result.runId); | ||
| if (this.consumers.has(key)) return; | ||
| this.markRunStarted(input.threadId, result.runId); |
There was a problem hiding this comment.
🟡 Retire the interrupted run before adopting a new resumed run
When resumeRun returns a different run ID, this path starts consuming only the new run while the interrupted run remains in thread.activeRunIds with status running. That stale active run blocks queued-message promotion and leaves incorrect state after the resumed run completes. Retire/remove the interrupted run when adopting a distinct returned ID, while preserving the existing same-run behavior.
| `disagrees with the profile event type ${JSON.stringify(extension.t)}`, | ||
| ); | ||
| } | ||
| return { |
There was a problem hiding this comment.
🟡 Preserve the terminal lifecycle of AG-UI interrupt frames
An AG-UI approval interrupt is encoded as RUN_FINISHED, but decoding returns only approval.requested. If a conforming producer closes the interrupted stream after that frame, the client consumer sees no run.completed/run.failed/run.cancelled event and reports run_missing_terminal before resume can complete. Preserve the interrupt terminal boundary or teach the consumer to accept approval interruption as an intentional terminal state.
| if (event.type === "approval.requested") requested = event; | ||
| } | ||
| await input.transport.resolveApproval({ | ||
| await input.transport.resumeRun({ |
There was a problem hiding this comment.
🟡 Validate the run returned by resumeRun
resumeRun returns StartRunResult because AG-UI-compatible runtimes may create a distinct resumed run, but this conformance scenario discards the result and continues consuming started.runId. A correct transport returning a new run ID will leave the check on the interrupted run and fail to observe the continuation events. Capture the returned ID and collect the resumed run separately.
| ?.split(";", 1)[0] | ||
| ?.trim() | ||
| .toLowerCase(); | ||
| const profile = response.headers.get(AGENTKIT_PROFILE_HEADER); |
There was a problem hiding this comment.
🟡 Reject streams without the AgentKit profile header
The parser rejects an unknown profile but accepts a missing x-agentkit-profile header even though the wire format is no longer the legacy envelope. A stock AG-UI or misconfigured response will have every frame skipped as foreign and can finish as an apparently successful empty run. Require the expected profile header before consuming the stream.
| return { | ||
| interruptId: input.approvalId, | ||
| status: input.response.decision === "approve" ? "resolved" : "cancelled", |
There was a problem hiding this comment.
🟡 Encode approval denials as resolved resume entries
resumeEntryFromApproval maps an explicit decision: "deny" to AG-UI status: "cancelled". AG-UI distinguishes cancelling an interrupt from resolving it with a negative decision, so a compatible runtime may abort the interrupted run instead of delivering the denial payload and continuing with approved: false. Use status: "resolved" for approve and deny decisions, reserving cancelled for explicit cancellation.
1 participant
Implements Option C from AgentKit Protocol Evaluation — AG-UI: AG-UI core plus a typed extension profile, instead of AgentKit's own wire protocol.
Based on
agent-kit(#4037), so this diff is exactly the cost of Option C.Approach
AG-UI (
@ag-ui/core@0.0.59) becomes the wire format. The domainAgentEventsurvives in memory and is rebuilt by a Builder-side typed parser — the spec's "type safety back only via a Builder-side parser".CUSTOMnamedbuilder.agentkit.v1/<type>.afterSequence, and the dedupe key unchanged.@ag-ui/corereserves"ag-ui".resolveApprovalis replaced byresumeRunwithresume[]entries; the bespokePOST /runs/:id/approvals/:idroute is gone.Both falsification steps pass
The evaluation said Option C becomes correct if step 2 and step 3 hold.
CUSTOM: the 29 profile events decode back into exhaustively-switchable domain events.Conformance now asserts, on every event produced by Core's real Agent-Native adapter, that the frame parses under AG-UI's own
EventSchemasand round-trips unchanged. That check caught a real defect while this was being written.What it actually costs
Three findings that bear on the recommendation.
1. The LOC case does not hold. Measured +1,345 / −169. The spec estimated ~1,200 added (accurate) and ~2,000–3,000 removed (off by an order of magnitude).
validation.tsvalidates the domain model; AG-UI validates the wire encoding. Option C replaces the encoding, which was never the expensive part. No variant of this deletes 2,000 lines while keeping the typing step 3 requires.2. "12 direct mappings" describes names, not payloads. Only 2 survive on native fields alone.
RUN_ERRORdropsAgentError,TEXT_MESSAGE_STARTdrops the whole message, andACTIVITY_DELTAis a JSON Patch against prior state that a stateless encoder cannot produce. The rest duplicate their payload into the profile residual to stay lossless — fidelity bought with wire duplication.3. Two costs the evaluation does not name.
@ag-ui/corerequires zod ^3 while this repo is on zod v4;zod@3.25.76now sits alongside it. Andagentkit-protocolgoes from zero runtime dependencies to a dependency chain.Net: implementing Option C strengthened the case for Option B, on evidence rather than argument.
Not addressed
mapAgUiEventinpackages/core/src/client/chat/connectors.tsstill flattens lossily. Option C does not fix it:decodeAgUiEventonly reconstructs frames carrying the Builder profile, and foreign AG-UI agents — the actual consumers ofcreateAgUiChatRuntime— carry none. The interop win is not automatic.Verification
packages/core/src/client/chattsc --noEmitclean in every touched package includingpackages/coreoxlintclean,oxfmtappliedpnpm guardsfailures (plan-skills,plan-marketplace,i18n-catalogs) are unbuilt-distenvironment issues that fail identically without this changeSteps to test
pnpm installpnpm --filter "@agent-native/agentkit-*" testpackages/agentkit-protocol/src/agui-codec.spec.tsholds the round-trip, AG-UI-validity, and reconnect proofs.