Fix(notebook edit): Fix edit_notebook_file silently failing and repeated "file already exists" errors

[George Zhao (CreatorZZY)]

Fixes #327473

Problem

The built-in Copilot agent tool edit_notebook_file often reports success but applies no changes when editing an existing notebook, while creating a brand-new notebook works fine. Two distinct root causes sit behind this behavior:

1. _createModifiedFileEntry misclassifies failures and produces repeated "file already exists" errors

In ChatEditingSession._createModifiedFileEntry, the catch block assumes any failure from doCreate(ChatEditKind.Modified) means the file does not exist, then falls back to creating it via bulkEditService.apply({ edits: [{ newResource: resource }] }) — without overwrite: true. When the file does exist (e.g. the original failure was a notebook model resolution error over SSH Remote), this throws FILE_MODIFIED_SINCE, which repeats on every streaming chunk iteration (roughly every ~10 s), flooding the chat output and logs.

Bug 2: Stale cell IDs produce non-actionable errors

When the model obtains cell IDs via copilot_getNotebookSummary, then the notebook is modified before edit_notebook_file is invoked, the old cell IDs are stale and no longer present in the cell-ID map. The tool correctly rejects the request, but the error message does not include the currently valid cell IDs, forcing the model to take an extra tool call before it can self-correct.

Fix

src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingSession.ts

• Before the create-fallback in the catch block, check await this._fileService.exists(notebookUri).
  • If the file already exists, rethrow the original error instead of assuming the file needs to be created — this stops the repeated FILE_MODIFIED_SINCE "file already exists" loop.
  • Only run the create-fallback when the file genuinely does not exist.

extensions/copilot/src/extension/tools/node/editNotebookTool.tsx

• getInvalidCellErrorMessage now accepts an optional NotebookDocument parameter. When a notebook is supplied, the error message includes the list of currently valid cell IDs, so the model can retry immediately with a correct ID instead of doing an extra tool call.

Impact

• Fixes the confusing silent no-op (success reported, notebook unchanged) that erodes trust in the built-in agent tool.
• Stops the repeated error spam that appears roughly every 10 seconds in streaming sessions.
• Reduces agent tool calls and latency by handing the model the exact valid cell IDs on the first rejection.

Known limitations

• The valid cell-ID list is best-effort; if the notebook document is unavailable, the error falls back to the previous message (no ID list is shown).
• If the notebook is modified concurrently between reading the summary and the check, a subsequent retry may still race until the model refreshs its understanding.
• The exists-check is a guard on the error path only; it does not change the normal create flow.

Version history

• v1 (this PR): initial fix for both root cause (a) and root cause (b).

Testing

Area	Test
Manual	Edit an existing notebook after it was modified (stale cell IDs) — verify the error lists currently valid IDs.
Manual	Repro the SSH Remote notebook-model resolution failure — verify no repeated "file already exists" errors and the original error is re-thrown.
Unit	Extend chatEditingService.test.ts to cover the new exists-branch in _createModifiedFileEntry.
CI	Run the affected test suites; keep all checks green.

Checklist

• Title references Fixes #327473 (auto-links and closes the issue on merge).
• Follows CONTRIBUTING.md conventions.
• CLA already agreed (@microsoft-github-policy-service agree).
• Regression test added for the new exists-branch (pending SME guidance on placement).

[Copilot]

Pull request overview

Attempts to improve notebook edit reliability and stale cell-ID recovery.

Changes:

• Restricts missing-file fallback to FILE_NOT_FOUND errors.
• Includes current notebook cell IDs in invalid-ID errors.
• Adds invalid cell-ID tests.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
chatEditingSession.ts	Changes file-creation fallback handling.
editNotebookTool.tsx	Enriches invalid cell-ID errors.
editNotebookTool.spec.tsx	Tests enriched error messages.

[George Zhao (CreatorZZY)]

Benjamin Christopher Simmonds (@benibenj)
Eleanor Boyd (@eleanorjboyd)

could we please get some attention on this one?
I'm flagging this because it's affecting real users in a pretty visible way. Bug #327473 describes a scenario where the built-in Copilot agent's edit_notebook_file tool silently fails — it executes against an existing notebook but doesn't actually apply any changes, while an entirely new notebook works without issue. This is one of the more confusing failure modes for Copilot in VS Code, because for a user it looks like the agent doesn't do anything, and the agent itself is effectively flying blind since there's no actionable feedback loop.
PR #327585 addresses two related root causes, and it's actually a small, well-scoped fix:
Repeated "file already exists" errors in ChatEditingSession._createModifiedFileEntry — the catch path assumes any failure means the file doesn't exist and falls back to creating it, but without overwrite: true. When the real cause is something else (e.g., a notebook model resolution failure over SSH Remote), that falls back into the create-existing path and throws FILE_MODIFIED_SINCE repeatedly — roughly once per streaming chunk (~every 10 seconds), flooding the logs. The fix adds an explicit fileService.exists(notebookUri) check and re-throws the original error when the file already exists, instead of misclassifying the failure.
Stale cell IDs producing unhelpful errors in editNotebookTool — if the notebook is modified between the model reading copilot_getNotebookSummary and the edit_notebook_file call, the model's cell IDs go stale. The tool correctly rejects the request, but the error message doesn't tell the model what the current valid cell IDs are, so the agent can't self-correct without an extra round trip. The PR wires the active NotebookDocument into getInvalidCellErrorMessage so the message now lists valid cell IDs and the model can immediately retry.
The PR (#327585) has been open since July 27 and the author has already been waiting without a response since July 30 for someone to take a look. It's a two-file change, the fix direction has been validated by bot review, and the author has generously already identified the exact behavior to guard against. There's also a regression-test gap the reviewer flagged (chatEditingService.test.ts would be the natural home) — if a SME could advise on that, the author can land it quickly rather than the PR stalling.
This is exactly the kind of silent degradation that erodes trust in the built-in agent tools, and it's blocking real users doing notebook work. Could we get a review, some guidance on the test coverage expectation, and hopefully a merge so it ships in an upcoming milestone? Happy to help with anything additional — repro steps, logs, or test additions — if that would help unblock.
Thanks for your time on this.