microsoft / microsoft/vscode

Chat paste attachment lifecycle diverges between the workbench and Agents window inputs

Open
#331,993 0 comments 0 reactions 1 assignee Claimed by @lramos15 View on GitHub
chat-input
Dominant language
TypeScript
Stars
193k
Forks
42.4k
PR merge metrics
PR metrics pending

Description

The chat input exists twice: `ChatInputPart` in the workbench and `NewChatInputWidget` in the Agents window. Paste **decision** logic is already shared, but everything after an attachment is created is not, and the two halves reconcile differently. That seam has now produced two bugs in one feature's lifetime.

### What is shared vs duplicated

Shared: [`chatPasteProviders.ts`](https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/chat/browser/widget/input/editor/chatPasteProviders.ts) decides whether a paste becomes an image, long text, a symbol, or code. Both windows register the same providers behind `IChatPasteTarget`, implemented by `ChatWidgetPasteTarget` and `NewChatInputPasteTarget`. This layer works well: the threshold fix in #331852 was one line and fixed both windows.

Duplicated, once an attachment exists:

| | Workbench | Agents window |
| --- | --- | --- |
| Pill rendering | `PasteAttachmentWidget` | inline DOM in `newChatContextAttachments.ts` |
| Inline reference tracking | `ChatDynamicVariableModel` | `AgentHostInputCompletionHandler._insertedReferences` |
| Reconciliation | against the **parsed input** | against the **raw text** |

Only `openPastedTextArtifact` is shared across that boundary.

### Why this matters

The reconciliation difference is not cosmetic, it changes what a caller must do to keep an attachment alive.

While prototyping an "Insert in Prompt" action for #331852, the Agents window implementation was correct because its reconciler re-derives references from raw text, so restoring the attachment was sufficient. The structurally identical workbench implementation was broken: its reconciler matches against the parsed input, and the text edit had destroyed the `ChatDynamicVariableModel` entry. Undo looked correct, then the pill disappeared on the next keystroke and the pasted content was silently dropped from the request.

The Agents window test passed throughout and could not have caught it. That is the cost: duplication stays invisible until behavior has to be transactional, then the two halves diverge in ways one side's tests cannot detect.

Note that #329455, "Improve copy and paste architecture across agent and editor window," is both the commit that shared the provider layer and the commit that shipped the threshold regression in #331852. Both bugs landed at the same seam.

### Churn

Last 6 months: `chatAttachmentWidgets.ts` 36 commits, `agentHostInputCompletions.ts` 17, `newChatContextAttachments.ts` 12, `chatPasteProviders.ts` 8. Five commits had to touch both surfaces together, roughly one a month. This is active code, not dormant.

### Proposal

Do **not** unify the two inputs. Reference tracking feeds undo, decorations, and the outgoing request payload in both windows; rewriting it risks silent wrong-payload bugs worse than the ones it prevents. The Agents window is also still evolving, so unifying now risks freezing the wrong abstraction.

Instead add a shared conformance suite, following the existing precedent in [`modelSelectionConformance.ts`](https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/chat/test/browser/widget/input/modelSelectionConformance.ts), which is a shared scenario table consumed by both `chatInputModelSelectionController.test.ts` and `sessionModelSelection.test.ts` for exactly this "two surfaces must agree, separate implementations" problem.

A `pasteAttachmentConformance.ts` would assert the same lifecycle against both implementations:

- paste over the threshold creates an attachment plus an inline reference
- undo and redo of the paste keep attachment and reference consistent
- deleting the attachment removes its reference text
- deleting the reference text removes the attachment
- after each step, the attachment set sent with the request matches what is shown

This makes divergence a test failure instead of a review catch, without touching correctness-critical code.

### Priority

Real but not urgent. One escaped bug in six months, caught before shipping. Worth doing before the next feature that touches attachment lifecycle, since that feature would otherwise pay the cost again.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.