Repository navigation
[Bug]: edit with replace_all corrupts literal dollar sequences in replacement text #155
Description
Product or interface
Source build or repository tooling — native edit tool used by the CLI.
Version
Source version 0.4.12, commit 17b8ded9c16da54224c2a05e54185cbee4f15021.
Node.js 24.21.0; pnpm 9.12.0.
Platform
Linux
OS version and architecture
Ubuntu 26.04.1 LTS, x86_64.
Issue area
File / workspace / path / encoding
Steps to reproduce
Bulk edits interpret JavaScript replacement-string sequences inside new_string instead of inserting the requested text literally.
The following self-contained reproduction runs from the source checkout with Node.js 24.21.0. It imports the repository's actual buildReplaceAllEdit implementation, using Node's TypeScript transformation to avoid an additional loader. It only creates and removes a synthetic temporary file; no account or model is needed.
node --input-type=module <<'NODE'
import { readFile, writeFile, mkdtemp, rm } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { stripTypeScriptTypes } from 'node:module';
const source = await readFile(
'packages/agent-tools/src/shared/replace-all-edit.ts', 'utf8',
);
const js = stripTypeScriptTypes(source, { mode: 'transform' });
const { buildReplaceAllEdit } = await import(
'data:text/javascript;base64,' + Buffer.from(js).toString('base64')
);
const dir = await mkdtemp(join(tmpdir(), 'mcode-replace-all-'));
try {
const file = join(dir, 'example.txt');
await writeFile(file, 'before TOKEN after\nTOKEN\n');
const result = await buildReplaceAllEdit(file, 'TOKEN', '$$');
console.log(JSON.stringify({
expected: 'before $$ after\n$$\n',
actual: result.edits[0].newText,
replacedCount: result.replacedCount,
}, null, 2));
} finally {
await rm(dir, { recursive: true, force: true });
}
NODEI also reproduced the final on-disk corruption through the actual LocalEditTool.execute implementation in a temporary Vitest test using the repository's Vitest aliases:
await writeFile(file, 'before TOKEN after\nTOKEN\n');
const result = await new LocalEditTool(directory).execute({} as never, {
file_path: file,
old_string: 'TOKEN',
new_string: '$$',
replace_all: true,
});
expect(await readFile(file, 'utf8')).toBe('before $$ after\n$$\n');That expected-behavior assertion fails, and the tool reports success. A single-occurrence control with replace_all: false correctly preserves $$.
Provider type / permission mode: not applicable to these direct tool reproductions. No model, permission service, real user files, or credentials are involved.
Expected and actual behavior
Expected: new_string is inserted literally. The resulting content should be before $$ after\n$$\n.
Actual: The resulting content is before $ after\n$\n, with a successful edit result.
Additional cases confirmed through LocalEditTool:
$&produces a false “No changes made” error because the matched text is reinserted.$'inserts the text following each match.- Dollar-backtick inserts the text preceding each match.
The latter two can duplicate unrelated surrounding file contents while reporting success. This affects legitimate replacement text in shell scripts, Makefiles, and JavaScript replacement expressions.
Redacted error summary
Helper output:
{
"expected": "before $$ after\n$$\n",
"actual": "before $ after\n$\n",
"replacedCount": 2
}
Actual edit-tool result:
Successfully replaced 1 block(s) in <temporary-directory>/example.txt.
Root cause and proposed fix
rawStringReplaceAll passes a replacement string directly to String.prototype.replaceAll, enabling substitution sequences. Its split().join() fallback already has literal semantics. The first-party LocalEditTool wrapper passes this transformed content to the actual file-edit pipeline.
A focused fix would use haystack.replaceAll(needle, () => replacement) and add regression tests for all four sequences, including final file-content assertions through the actual tool. This proposal has not yet been committed or submitted as a PR.
Validation: four expected-behavior assertions failed for the four bulk-replacement sequences; the single-edit dollar-sequence control passed. The self-contained helper reproduction above was also executed successfully and produced the shown mismatch. These are source-level, offline tests, not live-model or published-package acceptance. No full pnpm verify, native Windows/macOS validation, or published npm-package validation was performed for this report.
Contribution proposal
I would like to contribute the focused fix and regression tests. I read CONTRIBUTING.md and understand that code PRs are currently accepted only from repository collaborators. I am opening this issue first as requested. Would the maintainers accept a PR from me for this fix, and is collaborator access or another contribution arrangement required? I will follow the test registration, public-source inventory, and pnpm verify requirements for an accepted PR.
Screenshots / Desktop log upload ID
Not applicable; deterministic source-level reproduction provided above.
Before submitting
- I have searched existing issues, including
replace_alland edit-related reports, and found no matching report. - I have included my version and removed sensitive information. All files and inputs above are synthetic.
Activity
dvd233 commentedon Sep 18, 2026
The deterministic
replace_allreproduction looks actionable, and I verified the currentCONTRIBUTING.mdsays code/documentation contributions are currently limited to repository collaborators. Before preparing a branch, could a maintainer confirm whether an external contributordvd233may submit a focused fix for #155, or whether collaborator access is required? I will wait for that policy decision before creating a patch or PR.hetaoBackend commentedon Sep 18, 2026
Thanks for the precise reproduction and proposed fix. The fix is implemented in #165: the shared helper now uses a replacement callback so
new_stringis inserted literally.Validation on commit
9a35236fdc2ca1944c8e3b6eddbd7a50fee8ac22:$$,$&, dollar-backtick, and dollar-apostrophe through both the helper and actual file edits, including multiple matches and single-edit controls.pnpm verifypassed all 14 gates on macOS arm64 / Node 26.4.0.The merge attempt is currently blocked by the repository's required approving review. The PR has not merged and this fix has not been released; keeping this issue open until the merge completes.
hetaoBackend commentedon Sep 18, 2026
Update: #165 has now merged into
mainas f3160cd, and this issue is closed as completed. The review blocker mentioned above no longer applies.The fix and regression tests are included in the merged source. Validation results are recorded above; this confirms the source merge, not a published-package release. Thanks again for the detailed reproduction and proposed fix.