Ignore a steering message that is re-delivered while still in flight

[Ryan Ewen (RyanEwen)]

IAgent.setPendingMessages is a state sync rather than a command. QueueDrainContribution._syncPendingMessages calls it with whatever state.steeringMessage currently is, on every ChatPendingMessageSet, on every ChatPendingMessageRemoved of any kind, and on ChatQueuedMessagesReordered. So while a steering message is pending, unrelated pending-message activity on that chat re-delivers the same PendingMessage.id to the provider.

ClaudeSdkPipeline.injectSteering queued it every time it arrived. The effect is that a single steering message can be yielded to the model twice, under two different turns, and the second copy can leave a turn open that no work will ever fill.

Observed in a dev container: the same id enqueued at two points about two and a half minutes apart, followed by a chat/turnStarted for that id with no chat/turnComplete and no deltas after it.

The other two providers already guard

This is not a new idea, and Codex names the cause exactly:

// node/codex/codexAgent.ts
// `_syncPendingMessages` re-sends the current steering message on every
// pending-state change; ignore a steering message already in flight.
if (session.pendingSteeringFlips.has(steeringMessage.id)) {
	return;
}

Copilot does the same in copilotAgentSession.sendSteering. The Claude path had no equivalent, so this brings it to parity rather than introducing a new mechanism.

What this changes

ClaudeSdkPipeline tracks the steering ids it has accepted into the queue and not yet seen consumed, and ignores a repeat of one already in flight. The set is cleared when the queue reports the steer consumed, and on abort, since an aborted queue consumes nothing.

Clearing on consumption rather than only on abort matters: it keeps a genuinely new steer with a recycled id working, and it keeps the set bounded.

Related: #333174 groups this with the other provider-parity gaps.

AI disclosure: this pull request and the related code were written with the assistance of AI.

[vs-code-engineering]

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

TylerLeonhardt

Matched files:

• src/vs/platform/agentHost/node/claude/claudeSdkPipeline.ts

[Copilot]

🟡 Changes recommended

Crash cleanup can retain steering IDs indefinitely, and the deduplication behavior lacks regression coverage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Prevents duplicate Claude steering messages during pending-state synchronization.

Changes:

• Tracks steering IDs until consumed.
• Clears tracking on consumption or abort.

File summaries

File	Description
claudeSdkPipeline.ts	Adds steering-message deduplication and lifecycle cleanup.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 2
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Ryan Ewen (RyanEwen)]

Correct. Fixed in 84870a0: _failQueue fails the queue and clears the in-flight set together, and the aborted-rebind and fatal-stream paths both go through it, so those ids are released without an abort.

AI disclosure: this comment and the related code were written with the assistance of AI.

[Ryan Ewen (RyanEwen)]

Added in 84870a0: a steering message re-delivered before it is consumed is only queued once delivers the same id three times before consumption and asserts exactly one priority: 'now' prompt reaches the SDK. It fails without the guard. A second test covers the same id being accepted again once the first delivery is consumed.

AI disclosure: this comment and the related code were written with the assistance of AI.

[Ryan Ewen (RyanEwen)]

Both points were correct and are fixed in 84870a0.

The retained-ids path

Correct, and the consequence is worse than the bug the guard was added for. Three paths fail the queue and only one aborts the controller first:

site	aborts first	ids released before
abort()	yes	yes
aborted rebind	no	no
fatal stream error in _processMessages	no	no

So after a crash or a failed rebind, a steer that was recorded and never consumed stayed recorded, and every later delivery of that still-pending message was ignored. The message was silently dropped rather than duplicated.

Fixed by routing all three through one helper that fails the queue and clears the tracking together:

private _failQueue(error: Error): void {
    this._queue.failAll(error);
    this._steeringInFlight.clear();
}

That leaves a single choke point, which is the same shape #330785 already uses for its own steering state.

The missing coverage

Also correct; there was none. Two tests added, both passing (290 passing, 0 failing in claudeAgent.test.ts):

• a steering message re-delivered before it is consumed is only queued once delivers the same id three times before consumption and asserts exactly one priority: 'now' prompt reaches the SDK. It fails without the guard.
• a steering id is accepted again once the first delivery has been consumed covers the reset semantics, so a later message carrying a reused id is not swallowed.

One gap worth stating rather than hiding: I could not build a reliable test for acceptance after a failure specifically. Driving abort-then-rebind through the existing harness, the second steer never reaches a live parent in the rebuilt queue, so the test failed for reasons unrelated to the guard. Instrumenting it showed the rebind happening and the second turn starting, with no now prompt queued against it. That looks like a sequencing limitation of the test harness rather than product behaviour, and I did not want to ship a test whose failure mode I could not explain. The failure path is covered by inspection through the single _failQueue choke point.

AI disclosure: this comment and the related code were written with the assistance of AI.