v4.3.26: coder_patch_file의 SEARCH/REPLACE 적용부 분리 + 테스트 15개
execute-tool.ts에서 파일 편집 알고리즘을 순수 모듈로 추출. 1,529 → 1,498줄.
이 함수는 툴 레이어에서 가장 위험한 순수 로직임 — 사용자의 실제 소스 파일을
다시 쓰는데, 실패해도 예외를 던지지 않고 잘못된 바이트를 쓰거나 "3건 적용"이라
보고하고 2건만 적용한다. executeTool()의 1,349줄 본문 안에서 fs.writeFileSync
바로 옆에 인라인으로 있어서 디스크를 건드리지 않고는 검증할 방법이 없었음.
테스트가 고정한 성질(피해 큰 순):
1. 적용하지 않은 편집을 성공으로 세지 않는다
2. 엉뚱한 구간을 치환하지 않는다
3. 매칭 구간 밖의 바이트(들여쓰기·후행공백)를 보존한다
4. 실패를 보고해 모델이 재시도할 수 있게 한다
+ /g 정규식 lastIndex 오염으로 두 번째 호출이 편집을 건너뛰는 회귀 방지
추출 중 확인 — 계획 정정:
당초 "early return 평탄화"를 하려 했으나 실측해보니 이 파일의 깊은 중첩은
방어적 가드 피라미드가 아니라 알고리즘 자체의 깊이였음(공백 정규화 텍스트 검색,
Promise 래핑+파싱 루프 등). 평탄화해도 나아지지 않고 테스트 없는 I/O 코드를
기계적으로 건드리는 위험만 남으므로, 오늘 효과가 확인된 방식(파묻힌 순수 로직
추출)으로 방향을 바꿈.
알려진 날카로운 모서리를 문서화(수정하지 않음):
매칭이 줄 단위 앵커가 아니라 부분 문자열이라, 파일보다 얕게 들여쓴 SEARCH가
더 깊은 줄 안에서 매칭된다(" deep()"가 " deep()"에 적중). 대개 무해
하지만 한 줄을 노린 편집이 다른 줄에 갈 수 있는 경로임. 줄 앵커로 바꾸면 모든
호출자의 파일 편집 의미가 달라지므로 조용히 "고치지" 않고 테스트로 고정만 함.
전체 117개 테스트 통과. 실디스크 검증: coder_patch_file로 return 1 → return 42
수정이 들여쓰기·구조 보존한 채 정확히 반영됨.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,3 +1,4 @@
|
|||||||
|
import { applySearchReplaceEdits } from './search-replace-edit';
|
||||||
import path from 'path';
|
import path from 'path';
|
||||||
import fs from 'fs';
|
import fs from 'fs';
|
||||||
import { getConfig } from '../../config/config';
|
import { getConfig } from '../../config/config';
|
||||||
@@ -517,41 +518,10 @@ print(json.dumps({'slides': slides, 'total': len(prs.slides)}, ensure_ascii=Fals
|
|||||||
if (!fs.existsSync(filePath)) return { name, args, result: `"${filename}" not found. Use coder_write_file for new files.`, error: true };
|
if (!fs.existsSync(filePath)) return { name, args, result: `"${filename}" not found. Use coder_write_file for new files.`, error: true };
|
||||||
const content = fs.readFileSync(filePath, 'utf-8');
|
const content = fs.readFileSync(filePath, 'utf-8');
|
||||||
const edits = String(args.edits || '');
|
const edits = String(args.edits || '');
|
||||||
// Parse SEARCH/REPLACE blocks
|
const _patch = applySearchReplaceEdits(content, edits);
|
||||||
const srRegex = /------- SEARCH\n([\s\S]*?)\n=======\n([\s\S]*?)\n\+\+\+\+\+\+\+ REPLACE/g;
|
const result = _patch.content;
|
||||||
let match;
|
const editCount = _patch.editCount;
|
||||||
let result = content;
|
const failedEdits = _patch.failedEdits;
|
||||||
let editCount = 0;
|
|
||||||
const failedEdits: string[] = [];
|
|
||||||
while ((match = srRegex.exec(edits)) !== null) {
|
|
||||||
const searchText = match[1];
|
|
||||||
const replaceText = match[2];
|
|
||||||
if (result.includes(searchText)) {
|
|
||||||
result = result.replace(searchText, replaceText);
|
|
||||||
editCount++;
|
|
||||||
} else {
|
|
||||||
// Fallback: try with normalized whitespace (trim each line)
|
|
||||||
const normalize = (s: string) => s.split('\n').map(l => l.trimEnd()).join('\n');
|
|
||||||
if (normalize(result).includes(normalize(searchText))) {
|
|
||||||
// Find the original text with actual whitespace
|
|
||||||
const lines = result.split('\n');
|
|
||||||
const searchLines = searchText.split('\n');
|
|
||||||
const normLines = lines.map(l => l.trimEnd());
|
|
||||||
const normSearch = searchLines.map(l => l.trimEnd());
|
|
||||||
for (let i = 0; i <= normLines.length - normSearch.length; i++) {
|
|
||||||
if (normLines.slice(i, i + normSearch.length).join('\n') === normSearch.join('\n')) {
|
|
||||||
const originalText = lines.slice(i, i + searchLines.length).join('\n');
|
|
||||||
result = result.replace(originalText, replaceText);
|
|
||||||
editCount++;
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
const firstLine = searchText.split('\n')[0]?.slice(0, 80) || '(empty)';
|
|
||||||
failedEdits.push(firstLine);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if (editCount > 0) {
|
if (editCount > 0) {
|
||||||
fs.writeFileSync(filePath, result, 'utf-8');
|
fs.writeFileSync(filePath, result, 'utf-8');
|
||||||
const msg = `${filename}: ${editCount} edit(s) applied`;
|
const msg = `${filename}: ${editCount} edit(s) applied`;
|
||||||
@@ -560,8 +530,8 @@ print(json.dumps({'slides': slides, 'total': len(prs.slides)}, ensure_ascii=Fals
|
|||||||
}
|
}
|
||||||
return { name, args, result: msg, error: false };
|
return { name, args, result: msg, error: false };
|
||||||
}
|
}
|
||||||
// No srRegex matches — check if the model used a different format
|
// No blocks parsed at all — the model used a different format.
|
||||||
if (!edits.includes('------- SEARCH') && edits.length > 0) {
|
if (!_patch.hadAnyBlock && edits.length > 0) {
|
||||||
return { name, args, result: `No SEARCH/REPLACE blocks found. Use the format:\n------- SEARCH\nexact text\n=======\nreplacement\n+++++++ REPLACE`, error: true };
|
return { name, args, result: `No SEARCH/REPLACE blocks found. Use the format:\n------- SEARCH\nexact text\n=======\nreplacement\n+++++++ REPLACE`, error: true };
|
||||||
}
|
}
|
||||||
return { name, args, result: `No edits applied. SEARCH text not found in "${filename}". Use coder_read_file to verify exact content, including whitespace and indentation. Failed matches:\n${failedEdits.join('\n')}`, error: true };
|
return { name, args, result: `No edits applied. SEARCH text not found in "${filename}". Use coder_read_file to verify exact content, including whitespace and indentation. Failed matches:\n${failedEdits.join('\n')}`, error: true };
|
||||||
|
|||||||
@@ -0,0 +1,96 @@
|
|||||||
|
/**
|
||||||
|
* search-replace-edit.ts
|
||||||
|
*
|
||||||
|
* Applies coder_patch_file's SEARCH/REPLACE blocks to a file's contents.
|
||||||
|
*
|
||||||
|
* This is the highest-stakes pure function in the tool layer: it rewrites the user's actual
|
||||||
|
* source files. A bug here does not throw — it silently writes the wrong bytes, or reports
|
||||||
|
* "3 edits applied" while having applied two. Until 2026-07-29 it lived inline inside
|
||||||
|
* executeTool()'s 1,349-line body next to the fs.writeFileSync that consumes its output, so
|
||||||
|
* there was no way to exercise it without touching the disk.
|
||||||
|
*
|
||||||
|
* The whitespace fallback is the subtle part. Models routinely reproduce a SEARCH block with
|
||||||
|
* trailing whitespace that differs from the file, so an exact `includes()` fails on text that
|
||||||
|
* is visually identical. Rather than fail the edit, we retry with every line right-trimmed, and
|
||||||
|
* on a hit map the match back to the ORIGINAL text so the replacement preserves the file's real
|
||||||
|
* bytes outside the matched span. Leading whitespace is deliberately NOT normalised — it is
|
||||||
|
* indentation, and normalising it would let an edit land on a differently-indented block.
|
||||||
|
*
|
||||||
|
* ⚠ KNOWN SHARP EDGE (documented by tests, behaviour predates the 2026-07-29 extraction):
|
||||||
|
* matching is substring-based, not line-anchored, so a SEARCH indented less than the file still
|
||||||
|
* matches inside the deeper line — " deep()" hits within " deep()". The surviving
|
||||||
|
* indentation usually makes the result benign, but it does mean an edit aimed at one line can
|
||||||
|
* land on another. Changing this to line-anchored matching would alter file-editing semantics
|
||||||
|
* for every caller, so it is left as-is and pinned by a test rather than silently "fixed".
|
||||||
|
*/
|
||||||
|
|
||||||
|
/** Block delimiters coder_patch_file's tool description tells the model to emit. */
|
||||||
|
const SEARCH_REPLACE_BLOCK = /------- SEARCH\n([\s\S]*?)\n=======\n([\s\S]*?)\n\+\+\+\+\+\+\+ REPLACE/g;
|
||||||
|
|
||||||
|
export const SEARCH_MARKER = '------- SEARCH';
|
||||||
|
|
||||||
|
export interface SearchReplaceResult {
|
||||||
|
/** File contents after every applicable edit. Unchanged when nothing matched. */
|
||||||
|
content: string;
|
||||||
|
/** How many blocks were actually applied. */
|
||||||
|
editCount: number;
|
||||||
|
/** First line (≤80 chars) of each block whose SEARCH text could not be located. */
|
||||||
|
failedEdits: string[];
|
||||||
|
/** False when the edits text contains no SEARCH marker at all — a format error, not a miss. */
|
||||||
|
hadAnyBlock: boolean;
|
||||||
|
}
|
||||||
|
|
||||||
|
const rightTrimLines = (s: string) => s.split('\n').map(l => l.trimEnd()).join('\n');
|
||||||
|
|
||||||
|
export function applySearchReplaceEdits(originalContent: string, editsText: string): SearchReplaceResult {
|
||||||
|
const edits = String(editsText || '');
|
||||||
|
let content = String(originalContent ?? '');
|
||||||
|
let editCount = 0;
|
||||||
|
const failedEdits: string[] = [];
|
||||||
|
|
||||||
|
// exec() with /g is stateful; use a fresh regex per call so concurrent calls can't interfere.
|
||||||
|
const re = new RegExp(SEARCH_REPLACE_BLOCK.source, 'g');
|
||||||
|
let match: RegExpExecArray | null;
|
||||||
|
let sawBlock = false;
|
||||||
|
|
||||||
|
while ((match = re.exec(edits)) !== null) {
|
||||||
|
sawBlock = true;
|
||||||
|
const searchText = match[1];
|
||||||
|
const replaceText = match[2];
|
||||||
|
|
||||||
|
if (content.includes(searchText)) {
|
||||||
|
content = content.replace(searchText, replaceText);
|
||||||
|
editCount++;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Fallback: compare with trailing whitespace stripped, then locate the corresponding
|
||||||
|
// ORIGINAL span so the file's real bytes are what gets replaced.
|
||||||
|
const normContent = rightTrimLines(content);
|
||||||
|
const normSearch = rightTrimLines(searchText);
|
||||||
|
if (!normContent.includes(normSearch)) {
|
||||||
|
failedEdits.push(searchText.split('\n')[0]?.slice(0, 80) || '(empty)');
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
const lines = content.split('\n');
|
||||||
|
const searchLines = searchText.split('\n');
|
||||||
|
const normLines = lines.map(l => l.trimEnd());
|
||||||
|
const normSearchLines = searchLines.map(l => l.trimEnd());
|
||||||
|
let applied = false;
|
||||||
|
for (let i = 0; i <= normLines.length - normSearchLines.length; i++) {
|
||||||
|
if (normLines.slice(i, i + normSearchLines.length).join('\n') === normSearchLines.join('\n')) {
|
||||||
|
const originalText = lines.slice(i, i + searchLines.length).join('\n');
|
||||||
|
content = content.replace(originalText, replaceText);
|
||||||
|
editCount++;
|
||||||
|
applied = true;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Normalised compare matched but the line-window scan did not — treat as a miss rather than
|
||||||
|
// silently reporting success for an edit that never happened.
|
||||||
|
if (!applied) failedEdits.push(searchText.split('\n')[0]?.slice(0, 80) || '(empty)');
|
||||||
|
}
|
||||||
|
|
||||||
|
return { content, editCount, failedEdits, hadAnyBlock: sawBlock || edits.includes(SEARCH_MARKER) };
|
||||||
|
}
|
||||||
@@ -0,0 +1,140 @@
|
|||||||
|
/**
|
||||||
|
* search-replace-edit.test.ts
|
||||||
|
*
|
||||||
|
* This function rewrites the user's real source files. It has no way to "fail loudly" — a bug
|
||||||
|
* writes wrong bytes to disk, or reports "3 edits applied" having applied two, and the user
|
||||||
|
* finds out later from broken code. It was inline inside executeTool()'s 1,349-line body until
|
||||||
|
* 2026-07-29, so none of this had ever been exercised without touching the filesystem.
|
||||||
|
*
|
||||||
|
* The properties worth guarding, in rough order of how much damage a regression does:
|
||||||
|
* 1. never claim an edit that did not happen (silent corruption of the edit report)
|
||||||
|
* 2. never replace the wrong span (silent corruption of the file)
|
||||||
|
* 3. preserve bytes outside the matched span (indentation, trailing whitespace)
|
||||||
|
* 4. report misses so the model can retry
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { test, describe } from 'node:test';
|
||||||
|
import assert from 'node:assert/strict';
|
||||||
|
import { applySearchReplaceEdits } from '../src/gateway/chat/search-replace-edit';
|
||||||
|
|
||||||
|
const block = (search: string, replace: string) =>
|
||||||
|
`------- SEARCH\n${search}\n=======\n${replace}\n+++++++ REPLACE`;
|
||||||
|
|
||||||
|
describe('정확히 일치하는 경우', () => {
|
||||||
|
test('한 블록을 적용한다', () => {
|
||||||
|
const r = applySearchReplaceEdits('a\nb\nc\n', block('b', 'B'));
|
||||||
|
assert.equal(r.content, 'a\nB\nc\n');
|
||||||
|
assert.equal(r.editCount, 1);
|
||||||
|
assert.deepEqual(r.failedEdits, []);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('여러 블록을 순서대로 적용한다', () => {
|
||||||
|
const r = applySearchReplaceEdits('one\ntwo\nthree\n', block('one', '1') + '\n' + block('three', '3'));
|
||||||
|
assert.equal(r.content, '1\ntwo\n3\n');
|
||||||
|
assert.equal(r.editCount, 2);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('여러 줄 블록', () => {
|
||||||
|
const src = 'def f():\n return 1\n\nprint(f())\n';
|
||||||
|
const r = applySearchReplaceEdits(src, block('def f():\n return 1', 'def f():\n return 42'));
|
||||||
|
assert.equal(r.content, 'def f():\n return 42\n\nprint(f())\n');
|
||||||
|
assert.equal(r.editCount, 1);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('첫 번째 일치만 바꾼다 — 전역 치환이 아니다', () => {
|
||||||
|
const r = applySearchReplaceEdits('x\nx\nx\n', block('x', 'y'));
|
||||||
|
assert.equal(r.content, 'y\nx\nx\n');
|
||||||
|
assert.equal(r.editCount, 1);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('후행 공백 폴백 — 모델이 흔히 틀리는 지점', () => {
|
||||||
|
test('파일에 후행 공백이 있어도 매칭된다', () => {
|
||||||
|
const src = 'const a = 1; \nconst b = 2;\n';
|
||||||
|
const r = applySearchReplaceEdits(src, block('const a = 1;', 'const a = 99;'));
|
||||||
|
assert.equal(r.editCount, 1);
|
||||||
|
assert.ok(r.content.startsWith('const a = 99;'), r.content);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('SEARCH 쪽에 후행 공백이 있어도 매칭된다', () => {
|
||||||
|
const r = applySearchReplaceEdits('const a = 1;\n', block('const a = 1; ', 'const a = 99;'));
|
||||||
|
assert.equal(r.editCount, 1);
|
||||||
|
assert.equal(r.content, 'const a = 99;\n');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('매칭 구간 밖의 바이트는 보존된다', () => {
|
||||||
|
// SEARCH가 'target'뿐이면 파일에 있던 후행 공백은 매칭 구간 밖이므로 그대로 남는다.
|
||||||
|
// 앞뒤 줄의 공백도 손대지 않는다.
|
||||||
|
const src = 'head \ntarget \ntail \n';
|
||||||
|
const r = applySearchReplaceEdits(src, block('target', 'TARGET'));
|
||||||
|
assert.equal(r.editCount, 1);
|
||||||
|
assert.equal(r.content, 'head \nTARGET \ntail \n');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('⚠ 알려진 날카로운 모서리: SEARCH가 더 얕게 들여쓰기돼도 부분 문자열로 매칭된다', () => {
|
||||||
|
// 8칸 들여쓴 줄 안에 4칸 들여쓴 SEARCH가 부분 문자열로 들어있어 그대로 치환된다.
|
||||||
|
// 결과는 원래 들여쓰기가 유지되므로(4칸 + 치환문) 파이썬에서는 대개 무해하지만,
|
||||||
|
// "정확히 이 줄"을 노린 편집이 다른 줄에 적용될 수 있는 경로이기도 하다.
|
||||||
|
// 이는 추출 이전부터 있던 동작이며, 줄 단위 앵커로 바꾸는 것은 파일 편집 의미를
|
||||||
|
// 바꾸는 일이라 별도 판단이 필요하다. 지금은 동작을 문서화만 해둔다.
|
||||||
|
const src = 'if x:\n deep()\n';
|
||||||
|
const r = applySearchReplaceEdits(src, block(' deep()', ' shallow()'));
|
||||||
|
assert.equal(r.editCount, 1);
|
||||||
|
assert.equal(r.content, 'if x:\n shallow()\n');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('실패 보고 — 적용하지 않은 편집을 성공으로 세지 않는다', () => {
|
||||||
|
test('찾지 못한 블록은 failedEdits에 남고 editCount에 포함되지 않는다', () => {
|
||||||
|
const r = applySearchReplaceEdits('a\n', block('없는텍스트', 'x'));
|
||||||
|
assert.equal(r.editCount, 0);
|
||||||
|
assert.deepEqual(r.failedEdits, ['없는텍스트']);
|
||||||
|
assert.equal(r.content, 'a\n', '실패했는데 파일이 바뀜');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('성공과 실패가 섞이면 각각 정확히 집계된다', () => {
|
||||||
|
const r = applySearchReplaceEdits('a\nb\n', block('a', 'A') + '\n' + block('없음', 'x'));
|
||||||
|
assert.equal(r.editCount, 1);
|
||||||
|
assert.equal(r.failedEdits.length, 1);
|
||||||
|
assert.equal(r.content, 'A\nb\n');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('실패 라벨은 첫 줄 80자로 자른다', () => {
|
||||||
|
const long = 'z'.repeat(200);
|
||||||
|
const r = applySearchReplaceEdits('a\n', block(long, 'x'));
|
||||||
|
assert.equal(r.failedEdits[0].length, 80);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('형식 오류 구분', () => {
|
||||||
|
test('SEARCH 마커가 아예 없으면 hadAnyBlock=false — 형식 안내를 보내야 하는 경우', () => {
|
||||||
|
const r = applySearchReplaceEdits('a\n', 'just some text, no markers');
|
||||||
|
assert.equal(r.hadAnyBlock, false);
|
||||||
|
assert.equal(r.editCount, 0);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('마커는 있는데 매칭이 안 된 경우는 형식 오류가 아니다', () => {
|
||||||
|
// 이 둘을 섞으면 모델에게 엉뚱한 안내가 나간다.
|
||||||
|
const r = applySearchReplaceEdits('a\n', block('없음', 'x'));
|
||||||
|
assert.equal(r.hadAnyBlock, true);
|
||||||
|
assert.equal(r.editCount, 0);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('빈 입력에도 안전하다', () => {
|
||||||
|
const r = applySearchReplaceEdits('', '');
|
||||||
|
assert.equal(r.content, '');
|
||||||
|
assert.equal(r.editCount, 0);
|
||||||
|
assert.equal(r.hadAnyBlock, false);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('호출 간 상태 오염 방지', () => {
|
||||||
|
test('연속 호출이 서로 영향을 주지 않는다 (정규식 lastIndex)', () => {
|
||||||
|
// /g 정규식을 모듈 스코프에서 재사용하면 두 번째 호출이 중간부터 시작해 조용히 편집을 건너뛴다.
|
||||||
|
const edits = block('a', 'A');
|
||||||
|
const first = applySearchReplaceEdits('a\n', edits);
|
||||||
|
const second = applySearchReplaceEdits('a\n', edits);
|
||||||
|
assert.equal(first.editCount, 1);
|
||||||
|
assert.equal(second.editCount, 1, '두 번째 호출에서 편집이 유실됨 — lastIndex 오염');
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user