Conversation
FileMutation.StaleContentError is raised by writeIfUnchanged purely on a byte comparison under the per-path lock, and carries only a path. It fires whenever anything writes the file inside the edit's read to write window, from any session or process. The edit tool was the only place mapping it, and the mapping invented a cause the error never knew about: "File changed after permission approval." That misdirects anyone debugging a concurrent write, which is the common case. The existing regression test shows this directly. It is named "rejects an in-place content change after matching but before conditional commit" and simulates a plain concurrent write, yet asserted the permission wording. Name the actual condition instead and keep the same remedy.
Contributor
|
The following comment was made by an LLM, it may be inaccurate: Based on my search results, I found one potentially related PR that addresses a similar issue: Related PR:
This PR also deals with stale content/state handling in the context of approval workflows and patching operations, which is related to the stale content error handling being fixed in PR #48904. However, this is addressing a different aspect (stale patch sources vs. stale file content in edits), so it's not a direct duplicate. Conclusion: No direct duplicate PRs found. PR #48904 appears to be addressing a specific bug fix for improving error messaging around stale content failures in the edit tool. |
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.
Issue for this PR
Closes #48707
Type of change
What does this PR do?
editreportedFile changed after permission approval. Read it again before editing.for every stale-content failure, which names a cause the error has no knowledge of.FileMutation.StaleContentErroris raised inwriteIfUnchangedpurely from a byte comparison under the per-path lock:It carries only a
path, and fires whenever anything writes the file inside the edit's read → write window — another session, another process, the user's editor. The permission wording is introduced entirely by the mapping intool/edit.ts, which is the only place that maps this error.The repo's own regression test makes the mismatch plain.
rejects an in-place content change after matching but before conditional commitsimulates a plain concurrent write with no permission flow involved, and asserted the permission wording anyway.So this renames the condition to what actually happened and keeps the same remedy:
File changed since it was read. Read it again before editing.Behaviour is unchanged — same error, same failure path, same instruction to re-read. Only the sentence is different.
I checked
apply-patchfor the same problem; it does not mapStaleContentError, soedit.tswas the only occurrence.How did you verify your code works?
The string is pinned by the existing concurrent-write test, so updating it is what proves the change is scoped to this one message and nothing else reads it:
Screenshots / recordings
Not a UI change.
Checklist