ag-ui-protocol / ag-ui-protocol/ag-ui
[Bug]: a stream that ends without RUN_FINISHED/RUN_ERROR resolves as success (truncated run is indistinguishable from a completed one)
- 主要語言
- Python
- 星號
- 15.9k
- 分支
- 1.4k
- 平均合併
- 1 天 17 小時
- 30 天內合併 PR
- 163
描述
### Pre-flight Checklist
- [x] I have searched existing issues and this hasn't been reported yet.
- [x] I am using the **latest** version AG-UI (`@ag-ui/client` 0.0.57).
### Summary
The spec says a run's terminal event is mandatory:
> The `RunStarted` and either `RunFinished` or `RunError` events are mandatory, forming the boundaries of an agent run.
`verifyEvents` enforces a lot around those boundaries, but it never checks that a terminator arrived at all. When a stream ends mid-run (connection drop, LB idle timeout, server OOM, pod eviction), `runAgent()` **resolves successfully**, no lifecycle hook fires, and the partial assistant message is committed to `agent.messages` as if complete.
So a truncated run is byte-for-byte indistinguishable from a successful one, and the truncation point can land mid-token.
### Repro
Real `HttpAgent`, real SSE bytes, injected `fetch` so there is no server to run:
```js
import { HttpAgent } from '@ag-ui/client'
const FRAMES = [
{ type: 'RUN_STARTED', threadId: 'thread_1', runId: 'run_1' },
{ type: 'TEXT_MESSAGE_START', messageId: 'msg_1', role: 'assistant' },
{ type: 'TEXT_MESSAGE_CONTENT', messageId: 'msg_1', delta: 'Transferring $50,0' },
// socket dies here: no RUN_FINISHED, no RUN_ERROR
]
const fetchImpl = async () =>
new Response(
new ReadableStream({
start(c) {
const enc = new TextEncoder()
for (const f of FRAMES) c.enqueue(enc.encode(`data: ${JSON.stringify(f)}\n\n`))
c.close()
},
}),
{ status: 200, headers: { 'content-type': 'text/event-stream' } },
)
const agent = new HttpAgent({ url: 'http://example.test/agent', fetch: fetchImpl })
let flagged = false
const result = await agent.runAgent({}, {
onRunFailed: () => { flagged = true },
onRunErrorEvent: () => { flagged = true },
})
console.log('resolved: ', JSON.stringify(result))
console.log('flagged: ', flagged)
console.log('messages: ', JSON.stringify(agent.messages))
```
Output:
```
resolved: {"newMessages":[{"id":"msg_1","role":"assistant","content":"Transferring $50,0"}]}
flagged: false
messages: [{"id":"msg_1","role":"assistant","content":"Transferring $50,0"}]
```
### The asymmetry
Same harness, six streams. Only the middle two are rejected, and they are the *stricter* violations:
| stream | valid? | result |
|---|---|---|
| `RUN_STARTED, MSG_START, MSG_CONTENT, MSG_END, RUN_FINISHED` | yes | resolves |
| `RUN_STARTED, MSG_START, MSG_CONTENT, RUN_FINISHED` (msg left open) | no | **throws** `Cannot send 'RUN_FINISHED' while text messages are still active: m` |
| `... RUN_FINISHED, RUN_FINISHED` (duplicate terminator) | no | **throws** `The run has already finished with 'RUN_FINISHED'` |
| `RUN_STARTED, MSG_START, MSG_CONTENT` (truncated) | no | resolves, nothing flagged |
| `RUN_STARTED, MSG_START, MSG_CONTENT, MSG_END` (no terminator) | no | resolves, nothing flagged |
| `RUN_STARTED` (nothing else) | no | resolves, nothing flagged |
A run with *two* terminators is caught. A run with *zero* is not. That looks like an oversight rather than a deliberate tolerance, especially since zero is the case the network produces on its own.
### Cause
`verifyEvents` is a `mergeMap` state machine over arriving events. It tracks `runStarted` / `runFinished` / `runError` and rejects bad transitions, but the pipeline has no `finalize`/completion handler, so stream completion is never inspected. `lastValueFrom` then resolves normally and `runAgent()` returns `{ result, newMessages }`.
Related but distinct: #1892 was the mirror of this (an ADK integration emitting `RUN_ERROR` **then** `RUN_FINISHED`, correctly rejected by the client). That direction is guarded; this one isn't.
Worth noting for cross-SDK parity: #1327 (`validate_sequence` for the Python SDK) ports the same design, so it inherits the same blind spot. Cheaper to settle the semantics once, here, before both SDKs ship it.
### Suggested fix
Assert the invariant on stream completion in `verifyEvents`: if the source completes while `runStarted && !runFinished && !runError`, error the stream (an `AGUIError` naming the incomplete run, mirroring the existing message style). That surfaces truncation through the channel apps already handle, needs no wire change, and is roughly the same shape as the existing active-message/active-step checks that already run at `RUN_FINISHED`.
Two details worth deciding explicitly:
1. **Should this be opt-out?** A strict-by-default throw is a behaviour change for anyone currently (unknowingly) relying on partial results. If that is a concern, a config flag defaulting to strict, or routing it through `onRunFailed` rather than a rejection, both work. I would lean strict-by-default since the current behaviour silently loses data.
2. **Streams with no `RUN_STARTED` at all** (an immediately closed 200) currently resolve too. Same check covers it if the condition is "started but unterminated"; a separate rule is needed if an empty stream should also fail.
Happy to open a PR with the `finalize` check plus tests for the six cases above if you'd like it in that form.
### Environment
- `@ag-ui/client` 0.0.57 (latest on npm at time of filing)
- `@ag-ui/core` 0.0.57
- Bun 1.3.14, macOS arm64 (nothing runtime-specific; the pipeline is the same under Node)
貢獻指南
評估
這個 Issue 還沒有評估資料。