Store a streamed background suspension's exchange exactly once - #89
Merged
Conversation
A streamed run interrupted by a background suspension persisted the input and the partial updates itself, and the resumed run — whose continuation token replays the input and every update already produced — persisted the whole exchange again, so the next turn replayed the question and the partial answer twice. This broke the HistoryProvider contract that saveMessages only ever receives messages new to the turn. A streaming run that ends suspended now skips history persistence: its token carries the whole exchange, so the run that finally completes stores it in one append, with partial messages merged by the fold rather than split across store entries. The invariant is that a run whose outgoing token replays everything it saw stores nothing. An awaited run's token carries nothing, so an awaited suspension keeps storing its own half and the resumed run appends only its tail; mixed awaited/streaming chains store each fragment exactly once under the same rule. The fold takes the continuation token from the latest update, so a completed background response reliably clears it. Context providers other than history are still notified of a suspended run, unchanged. A discarded token now leaves nothing in history rather than an unanswered question, which is what the provider contract already documented as the desired shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR prevents duplicate history persistence when streamed background runs are suspended and resumed.
Changes:
- Defers history persistence for suspended streamed runs.
- Adds regression tests for single and chained suspensions.
- Documents suspension persistence behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Summary |
|---|---|
packages/core/src/context/context-provider.ts |
Documents history behavior across suspensions. |
packages/core/src/agent/continuation.test.ts |
Adds streamed suspension persistence regression tests. |
packages/core/src/agent/agent.ts |
Suppresses history persistence for suspended streams. Moderate issue (3 votes): injected context messages may be skipped and cannot be saved by the completing run. |
Suppressed comments (1)
packages/core/src/agent/agent.ts:710
- This condition also matches an early
breakfrom a stream after a provider has emitted a continuation token. In that pathResponseStreamstill invokesonResultwith the partial response, and the existingAgentRunStreamcontract says an abandoned stream persists the exchange as far as it got; this now skips the history append and loses the input/partial response. Pass the stream result'sabandonedstate through the teardown path and apply this suppression only when the stream ran to the suspension, not when the caller abandoned it.
const suspendedStream = streamed && response?.continuationToken !== undefined;
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A suspended streaming run defers persistence to the run that completes the exchange, but that run re-enters after context was applied and injects nothing itself — so with storeContextMessages enabled, the suspended run's injected context could never be stored. The streaming token now carries the injected messages, minus replayed history (the store already holds it, and a long transcript would dwarf the token), and a resumed run hands them to its providers as the run's context messages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/core/src/agent/agent.ts:715
- This gate conflates a stream that naturally suspended with one the caller abandoned. If a caller breaks after consuming a token-bearing update,
response.continuationTokenis still set, so history is skipped even though theAgentRunStreamcontract says abreakpersists the exchange as far as it got (agent.ts:60-67). The onResult hook already receivesresultCtx.abandoned; carry that state into this decision, or explicitly revise the abandonment/resume contract and its tests.
// A streaming run that ended suspended does not persist: its continuation token replays the
// caller's input and every update already produced, so the run that finally completes stores
// the whole exchange in one append — storing the suspended half here too would hand the next
// turn the question and the partial answer twice. An awaited run's token carries nothing, so
// an awaited suspension still stores its own half and the resumed run appends only its tail.
const suspendedStream = streamed && response?.continuationToken !== undefined;
packages/core/src/context/context-provider.ts:102
- The new suspension rule conflicts with the existing contract immediately below: breaking from a stream after receiving a continuation-token update is both an abandoned run and a suspended stream, but the latter now skips history while the docs still say every abandoned response is stored. Clarify this exception so callers know that discarding a streamed token intentionally leaves the partial exchange out of history.
* A background turn split across a suspension and a resume still appends once: a streaming run
* that ends suspended stores nothing (its continuation token replays the whole exchange, and
* the run that completes stores it), while an awaited suspension stores its own half and the
* resumed run appends only its tail.
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
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.
Problem
Interrupting a streamed run with
allowBackgroundResponsesand resuming it with thecontinuationTokendouble-saved the exchange:afterRunhook (a suspended stream still finalizes).The next turn then replayed the question and the partial answer twice. This violated the documented
HistoryProvidercontract:saveMessagesis handed only the messages new to that turn, so implementations append without de-duplicating.Change
A streaming run that ends suspended (its merged response still carries a continuation token) skips history persistence. Its token replays the caller's input and every update already produced, so the run that finally completes stores the whole exchange in one append — with partial messages merged by the fold rather than split across store entries. This matches the resumed-run semantics the .NET test suite locks in (
RunStreamingAsync_WhenResumingStreaming_UsesUpdatesFromInitialRunForContextProviderAndChatHistoryProviderAsync: the resumed run notifies the history provider with the carried-plus-new exchange, exactly once).The invariant: a run whose outgoing token replays everything it saw stores nothing.
The fold takes the continuation token from the latest update (matching Go
Response.Update), so a completed background response reliably clears it — a background stream that runs to completion persists normally.Context providers other than history are still notified of a suspended run, unchanged. A discarded token now leaves nothing in history rather than an unanswered question — the shape the provider contract already documents as desired. The
HistoryProvidercontract docs now state the suspension behavior explicitly.Verification
pnpm checkpasses (exit code verified).🤖 Generated with Claude Code