Skip to content

[Bug]: edit with replace_all corrupts literal dollar sequences in replacement text #155

Description

@Ashwinhegde19

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 });
}
NODE

I 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_all and 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions