fix: don't stream chat progress into a completed response (fixes #326647)

[vs-code-engineering]

Summary

For contributed/streamed chat sessions (e.g. Copilot cloud sessions), ChatServiceImpl runs a single autorun that mirrors a provider-supplied progressObs into the model via model.acceptResponseProgress(...). When the provider's observable emits additional progress items after the response has already been completed (progress and completion delivered on separate ticks), the autorun forwards them into an already-completed ChatResponseModel, which throws acceptResponseProgress: Adding progress to a completed response. The throw surfaces as an unhandled error in the Agents (sessions) window. Impact is a noisy uncaught error (158 hits / 109 users in 1.129.1) with no user-facing recovery.

Fixes #326647
Recommended reviewer: @osortega

Culprit Commit

The bucket has non-zero hits in every tracked release (1.128.0, 1.128.1, 1.129.0, 1.129.1, 1.130.0-insider), first seen 2026-07-08. The underlying streaming autorun that forwards provider progress predates all of these, so this is a pre-existing consumer-side lifecycle bug rather than a single-commit regression.

Field	Value
Commit	not identified — pre-existing
Author	n/a
PR	n/a
Message	n/a
Why	Hits appear in every release the bucket-history tool returns; the throwing code path (streaming progressObs into the model) predates the regression window. The large 1.129.x spike correlates with broader cloud/agent-host session rollout — notably 3eb10e18 (mid-turn steering, #325368), which reset progress tracking mid-stream and increased the chance progress arrives around completion — but that commit post-dates the first sighting, so it is a contributor to volume, not the introducer.

Code Flow

sequenceDiagram
    participant Provider as Contributed session (progressObs)
    participant Autorun as ChatServiceImpl autorun
    participant Model as ChatModel
    participant Response as ChatResponseModel

    Provider->>Autorun: isComplete = true (tick A)
    Autorun->>Model: completeLastResponse()
    Model->>Response: complete() → isComplete = true
    Note over Provider: ⚠️ Root cause:<br/>progressObs appends more items after completion
    Provider->>Autorun: progressArray grows (tick B)
    Autorun->>Model: acceptResponseProgress(lastRequest, progress)
    Model->>Response: isComplete === true
    Note over Response: 💥 Error thrown:<br/>Adding progress to a completed response

Loading

Affected Files

File	Role	Evidence
src/vs/workbench/contrib/chat/common/model/chatModel.ts	crash site	L2992-2993: if (request.response.isComplete) { throw new Error('acceptResponseProgress: Adding progress to a completed response'); }
src/vs/workbench/contrib/chat/common/chatService/chatServiceImpl.ts	root cause (consumer)	L987-990: if (lastRequest && progressArray.length > lastProgressLength) { ... model?.acceptResponseProgress(lastRequest, progress); } — forwards new progress without checking whether the response was already completed at L999 (completeLastResponse()) on a prior tick

Repro Steps

1. Open the Agents window with a contributed streamed session (e.g. a Copilot cloud session) that exposes progressObs + isCompleteObs.
2. Have the provider signal completion (isCompleteObs → true) on one observable tick, so the autorun calls completeLastResponse() and the ChatResponseModel becomes complete.
3. Have the same provider append one more progress item to progressObs on a subsequent tick (late/trailing chunk after the completion signal).
4. The streaming autorun forwards the trailing item via acceptResponseProgress, which throws because the response is already complete.

How the Fix Works

Chosen approach — src/vs/workbench/contrib/chat/common/chatService/chatServiceImpl.ts: guard the progress-forwarding loop with !lastRequest.response?.isComplete so trailing progress emitted after the response has been completed is dropped instead of being pushed into a completed ChatResponseModel.

This is a consumer-side guard, which is the correct place here because the producer is an extension-contributed observable (providedSession.progressObs) — an external boundary whose emit ordering (progress after completion) the core cannot control or fix at its source. The guard does not mask a core-owned producer bug and does not use try/catch: the throwing telemetry path in chatModel.ts is preserved for every other caller. Within the same autorun tick where completion first occurs, response.isComplete is still false when the loop runs (completion happens later at L999), so legitimate final progress delivered alongside the completion signal is still accepted — only strictly-after-completion trailing items are skipped.

Alternatives considered:

• Wrap the acceptResponseProgress call in try/catch — rejected: hides the error from the telemetry pipeline instead of preventing the invalid state, and would swallow genuine misuse from other callers.
• Widen acceptResponseProgress in chatModel.ts to silently no-op when complete — rejected: that patches the shared crash-site utility for all ~30 callers and removes a useful invariant check; the offending caller is the streaming autorun, so the fix belongs there.

Recommended Owner

@osortega — most recent author of the contributed-session progress-streaming / mid-turn-steering code in chatServiceImpl.ts (#325368) and actively working in this area.

Generated by errors-fix · 1.6K AIC · ⌖ 34.8 AIC · ⊞ 71K · ◷

---

errors-fix-driver — cycle 1

Trigger: cron_review_comments · Head: 8baa98525235012264a6ddbf5bcaf19f3c6d7dca (8baa985) + review-fix commit pushed this cycle

Item	Action
Review on chatServiceImpl.ts:990 (in scope)	Fixed — made response-exists check explicit (lastRequest.response && !lastRequest.response.isComplete); replied + resolved

Push: yes (review-fix commit staged via push_to_pull_request_branch) · Copilot rerequested: ok

Ready gate: PR already non-draft; new commit pushed → CI re-running and Copilot re-review pending, so no ready-state change needed this cycle.

Generated by errors-fix-driver · 702.4 AIC · ⌖ 33.4 AIC · ⊞ 72.9K · ◷

[Copilot]

Pull request overview

Note

Copilot could not run the full agentic suite for this review because it was automatically requested on a bot-authored pull request. Request a review from Copilot under Reviewers to retry with the full agentic suite. Improved support for bot-authored pull requests is coming soon.

Guards chat progress processing to avoid exceptions when contributed sessions emit progress after a response has already completed.

Changes:

• Adds an isComplete check to skip processing progress once the response is completed.
• Expands the inline comment to document the race/tick scenario and why the guard is needed.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[vs-code-engineering]

Good catch — made the intent explicit by replacing !lastRequest.response?.isComplete with lastRequest.response && !lastRequest.response.isComplete, so trailing progress is only forwarded when the response exists and is not yet complete. Pushed to the PR branch.

Generated by errors-fix-driver · 702.4 AIC · ⌖ 33.4 AIC · ⊞ 72.9K

[vs-code-engineering]

Commit pushed: 66e6209

Generated by errors-fix-driver · 702.4 AIC · ⌖ 33.4 AIC · ⊞ 72.9K

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.