Repository navigation
fix(agent-core): refuse multi-line empty Edit deletions - #2511
mangeshraut712 wants to merge 26 commits into
Conversation
🦋 Changeset detectedLatest commit: 55e1940 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 097b1774d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
commit: |
097b177 to
eb224a2
Compare
|
Rebased onto latest CI is currently stuck in |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 380b0df96d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a17e3b9f8c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
2f08cd7 to
51cc962
Compare
|
Rebased onto latest |
c6652fd to
1a45ffa
Compare
|
Rebased onto latest |
|
rebase it |
744fcd0 to
1ee01a3
Compare
1ee01a3 to
4aeab95
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 403a94d376
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
8cc1aee to
bf501f5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf501f5b79
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Following up on the earlier rebases. This is mergeable and still needs a maintainer review: multi-line empty Edit deletions are refused. I will leave the branch as it is unless you want a change. |
b1c9995 to
b2db743
Compare
Guard Edit against accidental +0/−N wipeouts when new_string is empty across 3+ lines unless allow_large_delete is set. Strengthen the old_string-not-found recovery guidance so the model rereads a large enough region instead of looping short Reads (fixes MoonshotAI#2427).
Count matched occurrences when guarding empty replace_all edits so a short old_string cannot delete 3+ lines without allow_large_delete.
Match and uniqueness checks run before refusing an empty deletion. replace_all still accumulates multi-line matches, but a one-line token is not treated as a multi-line span.
Re-record loop and tool inline snapshots so they include the post-rebase tool list and allow_large_delete hashes.
b2db743 to
2ba98cb
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87672fd030
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1cf039374
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Name the 3-or-more-line cutoff in the changelog entry, and split the English and Chinese Edit references so the large-delete rules are a separate paragraph.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac19e02b9f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d314b22030
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…use-large-delete-7813 # Conflicts: # packages/agent-core-v2/test/agent/loop/loop.test.ts # packages/agent-core-v2/test/tool/tool.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d2282dc2f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…use-large-delete-7813 # Conflicts: # packages/agent-core-v2/test/agent/loop/loop.test.ts # packages/agent-core-v2/test/tool/tool.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f75505a976
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…use-large-delete-7813 # Conflicts: # packages/agent-core-v2/test/agent/loop/loop.test.ts # packages/agent-core-v2/test/tool/tool.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3fe661cef7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…rge-delete-7813 # Conflicts: # packages/agent-core-v2/test/agent/loop/loop.test.ts # packages/agent-core-v2/test/tool/tool.test.ts
Signed-off-by: Mangesh Raut <mbr63@drexel.edu>
Signed-off-by: Mangesh Raut <mbr63@drexel.edu>
…use-large-delete-7813 Signed-off-by: Mangesh Raut <mbr63@drexel.edu>
Signed-off-by: Mangesh Raut <mbr63@drexel.edu>
Signed-off-by: Mangesh Raut <mbr63@drexel.edu>
Keep the 3-line threshold and multi-line replace_all qualifier without the extra not-found retry clause.
…use-large-delete-7813
Inline snapshots pick up allow_large_delete in the Edit schema and updated tools_snapshot hashes from main.
|
CI is green on this. It refuses multi-line empty Edit deletions in agent-core. Happy to take any feedback — if it looks good, could you please review and merge when you get a chance? Thanks. |
Requirement or Bug
Fixes #2427.
Bug Reproduction Steps
Editwith that section asold_stringand an empty or whitespace-onlynew_string.replace_all: truematches multiple multi-line sections whose total is at least three lines.old_string, its error only asks the model to reload content, which permits a short-read retry loop.Root Cause
EditServicetreated an empty replacement like any other exact replacement, so it had no size-based refusal before writing the result. Its stale-content error did not tell the model how much context to reread. This fixes both at the shared service boundary and in the Edit tool guidance rather than relying only on prompt behavior.Code Changes
allow_large_deleteto the Edit input schema and forward it to the service.replace_allmatches, before writing the file.Behavior Changes and Affected Users
replace_allmatchesEditrefuses before writing.Edit.allow_large_delete: truefor an intentional deletion.replace_alldeletionsold_string not founderror guidanceEdittool.edit.mddeletion-guidance sentenceallow_large_delete=trueonly for intentional 3+ line empty or whitespace-only deletions.Edittool.old_stringat the file-edit service boundaryAffected modules and coverage:
packages/agent-core-v2/src/app/edit/implements the refusal before file writes.packages/agent-core-v2/src/agent/tools/edit/exposes and explains the opt-in flag.packages/agent-core-v2/test/app/edit/tools/edit.test.tscovers direct refusal, the explicit override, cumulativereplace_all, whitespace-only input, LF/CRLF/lone-CR handling, allowed small deletions, stale/non-unique precedence, no-write behavior, and the public schema.docs/en/reference/tools.mdanddocs/zh/reference/tools.mddescribe the same threshold and opt-in.Checklist
/approve).#2427is linked; a maintainer approval is still required.allow_large_deleteescape hatch.gen-changesetsreview; the existing user-facing changeset covers the fix, and no additional entry is needed for the test and wording hardening.gen-docsreview; the English and Chinese reference pages are mirrored and the docs build passes. The skill's expecteddocs/scripts/sync-changelog.mjsprerequisite is absent, so no changelog-sync step was run.Prompt change
Population: every agent session that exposes the built-in Edit tool (CLI and other hosts that load the default tool set).
Before: after
old_string not found, the model guidance only said to reread; a short 30–50 line window often satisfied that and the retry loop continued.After: Edit tool guidance plus the
notFoundMessagetell the model to reread the whole file or a large enough region before retrying, and not to loop Edit → short Read → Edit. This is prompt/guidance only; the oversized empty-delete refusal is enforced in code viaallow_large_delete.