fix(agent-core): refuse multi-line empty Edit deletions - #2511
fix(agent-core): refuse multi-line empty Edit deletions#2511mangeshraut712 wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: eb224a2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
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".
| | { readonly ok: true; readonly rawContent: string; readonly count: number } | ||
| | { readonly ok: false; readonly error: string }; | ||
|
|
||
| /** Multi-line empty replacements without an explicit opt-in are refused (see #2427). */ |
There was a problem hiding this comment.
This new documentation comment sits beside a constant, but the scoped convention requires comments to live solely in the top-of-file block and explicitly prohibits comments beside statements. Incorporate this responsibility into the existing module header or remove the comment.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L13-L13
Useful? React with 👍 / 👎.
| for (let i = 0; i < text.length; i++) { | ||
| if (text.charCodeAt(i) === 10) lines++; | ||
| } |
There was a problem hiding this comment.
When editing a file with lone-CR line endings, such as an old_string of a\rb\rc, this counter returns one line because it recognizes only LF. The existing text-model path deliberately preserves mixed/lone-CR content, so that string can match and delete three logical lines without allow_large_delete, bypassing the safety check introduced here. Count lone \r separators as line breaks while avoiding double-counting \r\n; the mirrored v1 helper has the same defect.
Useful? React with 👍 / 👎.
commit: |
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).
Update inline tools_snapshot hashes for allow_large_delete on the Edit tool.
097b177 to
eb224a2
Compare
|
Rebased onto latest CI is currently stuck in |
1 participant
Summary
new_stringunlessallow_large_delete=true(fixes Edit 失败后陷入死循环：反复"Edit 失败 → 重读 → 再失败"，并逐步删空了被编辑文件的多个章节 #2427).old_string not foundrecovery guidance to reread a large enough region.Test plan
pnpm exec vitest run test/tools/edit.test.tsinpackages/agent-core(23 passed)pnpm exec vitest run test/app/edit/tools/edit.test.tsinpackages/agent-core-v2(26 passed)Fixes #2427