From faa1ec3044be0ef17511c150b0f9158588bb4461 Mon Sep 17 00:00:00 2001 From: Nick Salerni Date: Sun, 22 Feb 2026 11:58:31 -0800 Subject: [PATCH] fix(core): prevent omission placeholder deletions in replace/write_file (#19870) Co-authored-by: Bryan Morgan --- .../coreToolsModelSnapshots.test.ts.snap | 8 +- .../model-family-sets/default-legacy.ts | 5 +- .../definitions/model-family-sets/gemini-3.ts | 5 +- packages/core/src/tools/edit.test.ts | 47 ++++++++ packages/core/src/tools/edit.ts | 14 +++ .../tools/omissionPlaceholderDetector.test.ts | 63 +++++++++++ .../src/tools/omissionPlaceholderDetector.ts | 106 ++++++++++++++++++ packages/core/src/tools/write-file.test.ts | 36 ++++++ packages/core/src/tools/write-file.ts | 6 + 9 files changed, 282 insertions(+), 8 deletions(-) create mode 100644 packages/core/src/tools/omissionPlaceholderDetector.test.ts create mode 100644 packages/core/src/tools/omissionPlaceholderDetector.ts diff --git a/packages/core/src/tools/definitions/__snapshots__/coreToolsModelSnapshots.test.ts.snap b/packages/core/src/tools/definitions/__snapshots__/coreToolsModelSnapshots.test.ts.snap index 7e3e1dcf80..8ec768d843 100644 --- a/packages/core/src/tools/definitions/__snapshots__/coreToolsModelSnapshots.test.ts.snap +++ b/packages/core/src/tools/definitions/__snapshots__/coreToolsModelSnapshots.test.ts.snap @@ -547,7 +547,7 @@ A good instruction should concisely answer: "type": "string", }, "new_string": { - "description": "The exact literal text to replace \`old_string\` with, preferably unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic.", + "description": "The exact literal text to replace \`old_string\` with, preferably unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic. Do not use omission placeholders like '(rest of methods ...)', '...', or 'unchanged code'; provide exact literal code.", "type": "string", }, "old_string": { @@ -665,7 +665,7 @@ exports[`coreTools snapshots for specific models > Model: gemini-2.5-pro > snaps "parametersJsonSchema": { "properties": { "content": { - "description": "The content to write to the file.", + "description": "The content to write to the file. Do not use omission placeholders like '(rest of methods ...)', '...', or 'unchanged code'; provide complete literal content.", "type": "string", }, "file_path": { @@ -1312,7 +1312,7 @@ The user has the ability to modify the \`new_string\` content. If modified, this "type": "string", }, "new_string": { - "description": "The exact literal text to replace \`old_string\` with, unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic.", + "description": "The exact literal text to replace \`old_string\` with, unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic. Do not use omission placeholders like '(rest of methods ...)', '...', or 'unchanged code'; provide exact literal code.", "type": "string", }, "old_string": { @@ -1429,7 +1429,7 @@ The user has the ability to modify \`content\`. If modified, this will be stated "parametersJsonSchema": { "properties": { "content": { - "description": "The content to write to the file.", + "description": "The content to write to the file. Do not use omission placeholders like '(rest of methods ...)', '...', or 'unchanged code'; provide complete literal content.", "type": "string", }, "file_path": { diff --git a/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts b/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts index fad72047a9..ae9c4831ad 100644 --- a/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts +++ b/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts @@ -71,7 +71,8 @@ export const DEFAULT_LEGACY_SET: CoreToolSet = { type: 'string', }, content: { - description: 'The content to write to the file.', + description: + "The content to write to the file. Do not use omission placeholders like '(rest of methods ...)', '...', or 'unchanged code'; provide complete literal content.", type: 'string', }, }, @@ -332,7 +333,7 @@ A good instruction should concisely answer: }, new_string: { description: - 'The exact literal text to replace `old_string` with, preferably unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic.', + "The exact literal text to replace `old_string` with, preferably unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic. Do not use omission placeholders like '(rest of methods ...)', '...', or 'unchanged code'; provide exact literal code.", type: 'string', }, expected_replacements: { diff --git a/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts b/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts index 1ceca46d9f..6bb2809874 100644 --- a/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts +++ b/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts @@ -73,7 +73,8 @@ The user has the ability to modify \`content\`. If modified, this will be stated type: 'string', }, content: { - description: 'The content to write to the file.', + description: + "The content to write to the file. Do not use omission placeholders like '(rest of methods ...)', '...', or 'unchanged code'; provide complete literal content.", type: 'string', }, }, @@ -310,7 +311,7 @@ The user has the ability to modify the \`new_string\` content. If modified, this }, new_string: { description: - 'The exact literal text to replace `old_string` with, unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic.', + "The exact literal text to replace `old_string` with, unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic. Do not use omission placeholders like '(rest of methods ...)', '...', or 'unchanged code'; provide exact literal code.", type: 'string', }, expected_replacements: { diff --git a/packages/core/src/tools/edit.test.ts b/packages/core/src/tools/edit.test.ts index 3b8cbe9645..9c67515f38 100644 --- a/packages/core/src/tools/edit.test.ts +++ b/packages/core/src/tools/edit.test.ts @@ -607,6 +607,53 @@ function doIt() { }; expect(tool.validateToolParams(params)).toMatch(/Path not in workspace/); }); + + it('should reject omission placeholder in new_string when old_string does not contain that placeholder', () => { + const params: EditToolParams = { + file_path: path.join(rootDir, 'test.txt'), + instruction: 'An instruction', + old_string: 'old content', + new_string: '(rest of methods ...)', + }; + expect(tool.validateToolParams(params)).toBe( + "`new_string` contains an omission placeholder (for example 'rest of methods ...'). Provide exact literal replacement text.", + ); + }); + + it('should reject new_string when it contains an additional placeholder not present in old_string', () => { + const params: EditToolParams = { + file_path: path.join(rootDir, 'test.txt'), + instruction: 'An instruction', + old_string: '(rest of methods ...)', + new_string: `(rest of methods ...) +(unchanged code ...)`, + }; + expect(tool.validateToolParams(params)).toBe( + "`new_string` contains an omission placeholder (for example 'rest of methods ...'). Provide exact literal replacement text.", + ); + }); + + it('should allow omission placeholders when all are already present in old_string', () => { + const params: EditToolParams = { + file_path: path.join(rootDir, 'test.txt'), + instruction: 'An instruction', + old_string: `(rest of methods ...) +(unchanged code ...)`, + new_string: `(unchanged code ...) +(rest of methods ...)`, + }; + expect(tool.validateToolParams(params)).toBeNull(); + }); + + it('should allow normal code that contains placeholder text in a string literal', () => { + const params: EditToolParams = { + file_path: path.join(rootDir, 'test.ts'), + instruction: 'Update string literal', + old_string: 'const msg = "old";', + new_string: 'const msg = "(rest of methods ...)";', + }; + expect(tool.validateToolParams(params)).toBeNull(); + }); }); describe('execute', () => { diff --git a/packages/core/src/tools/edit.ts b/packages/core/src/tools/edit.ts index d758e03229..da230be95e 100644 --- a/packages/core/src/tools/edit.ts +++ b/packages/core/src/tools/edit.ts @@ -54,6 +54,7 @@ import { debugLogger } from '../utils/debugLogger.js'; import levenshtein from 'fast-levenshtein'; import { EDIT_DEFINITION } from './definitions/coreTools.js'; import { resolveToolDeclaration } from './definitions/resolver.js'; +import { detectOmissionPlaceholders } from './omissionPlaceholderDetector.js'; const ENABLE_FUZZY_MATCH_RECOVERY = true; const FUZZY_MATCH_THRESHOLD = 0.1; // Allow up to 10% weighted difference @@ -973,6 +974,19 @@ export class EditTool } params.file_path = filePath; + const newPlaceholders = detectOmissionPlaceholders(params.new_string); + if (newPlaceholders.length > 0) { + const oldPlaceholders = new Set( + detectOmissionPlaceholders(params.old_string), + ); + + for (const placeholder of newPlaceholders) { + if (!oldPlaceholders.has(placeholder)) { + return "`new_string` contains an omission placeholder (for example 'rest of methods ...'). Provide exact literal replacement text."; + } + } + } + return this.config.validatePathAccess(params.file_path); } diff --git a/packages/core/src/tools/omissionPlaceholderDetector.test.ts b/packages/core/src/tools/omissionPlaceholderDetector.test.ts new file mode 100644 index 0000000000..4e574d5e22 --- /dev/null +++ b/packages/core/src/tools/omissionPlaceholderDetector.test.ts @@ -0,0 +1,63 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { detectOmissionPlaceholders } from './omissionPlaceholderDetector.js'; + +describe('detectOmissionPlaceholders', () => { + it('detects standalone placeholder lines', () => { + expect(detectOmissionPlaceholders('(rest of methods ...)')).toEqual([ + 'rest of methods ...', + ]); + expect(detectOmissionPlaceholders('(rest of code ...)')).toEqual([ + 'rest of code ...', + ]); + expect(detectOmissionPlaceholders('(unchanged code ...)')).toEqual([ + 'unchanged code ...', + ]); + expect(detectOmissionPlaceholders('// rest of methods ...')).toEqual([ + 'rest of methods ...', + ]); + }); + + it('detects case-insensitive placeholders', () => { + expect(detectOmissionPlaceholders('(Rest Of Methods ...)')).toEqual([ + 'rest of methods ...', + ]); + }); + + it('detects multiple placeholder lines in one input', () => { + const text = `class Example { + run() {} + (rest of methods ...) + (unchanged code ...) +}`; + expect(detectOmissionPlaceholders(text)).toEqual([ + 'rest of methods ...', + 'unchanged code ...', + ]); + }); + + it('does not detect placeholders embedded in normal code', () => { + expect( + detectOmissionPlaceholders( + 'const note = "(rest of methods ...)";\nconsole.log(note);', + ), + ).toEqual([]); + }); + + it('does not detect omission phrase when inline in a comment', () => { + expect( + detectOmissionPlaceholders('return value; // rest of methods ...'), + ).toEqual([]); + }); + + it('does not detect unrelated ellipsis text', () => { + expect(detectOmissionPlaceholders('const message = "loading...";')).toEqual( + [], + ); + }); +}); diff --git a/packages/core/src/tools/omissionPlaceholderDetector.ts b/packages/core/src/tools/omissionPlaceholderDetector.ts new file mode 100644 index 0000000000..7057a7f09d --- /dev/null +++ b/packages/core/src/tools/omissionPlaceholderDetector.ts @@ -0,0 +1,106 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +const OMITTED_PREFIXES = new Set([ + 'rest of', + 'rest of method', + 'rest of methods', + 'rest of code', + 'unchanged code', + 'unchanged method', + 'unchanged methods', +]); + +function isAllDots(str: string): boolean { + if (str.length === 0) { + return false; + } + for (let i = 0; i < str.length; i++) { + if (str[i] !== '.') { + return false; + } + } + return true; +} + +function normalizeWhitespace(input: string): string { + const segments: string[] = []; + let current = ''; + + for (const char of input) { + if (char === ' ' || char === '\t' || char === '\n' || char === '\r') { + if (current.length > 0) { + segments.push(current); + current = ''; + } + continue; + } + current += char; + } + + if (current.length > 0) { + segments.push(current); + } + + return segments.join(' '); +} + +function normalizePlaceholder(line: string): string | null { + let text = line.trim(); + if (!text) { + return null; + } + + if (text.startsWith('//')) { + text = text.slice(2).trim(); + } + + if (text.startsWith('(') && text.endsWith(')')) { + text = text.slice(1, -1).trim(); + } + + const ellipsisStart = text.indexOf('...'); + if (ellipsisStart < 0) { + return null; + } + + const prefixRaw = text.slice(0, ellipsisStart).trim().toLowerCase(); + const suffixRaw = text.slice(ellipsisStart + 3).trim(); + const prefix = normalizeWhitespace(prefixRaw); + + if (!OMITTED_PREFIXES.has(prefix)) { + return null; + } + + if (suffixRaw.length > 0 && !isAllDots(suffixRaw)) { + return null; + } + + return `${prefix} ...`; +} + +/** + * Detects shorthand omission placeholders such as: + * - (rest of methods ...) + * - (rest of code ...) + * - (unchanged code ...) + * - // rest of methods ... + * + * Returns all placeholders found as normalized tokens. + */ +export function detectOmissionPlaceholders(text: string): string[] { + const lines = text.replaceAll('\r\n', '\n').split('\n'); + const matches: string[] = []; + + for (const rawLine of lines) { + const normalized = normalizePlaceholder(rawLine); + if (normalized) { + matches.push(normalized); + } + } + + return matches; +} diff --git a/packages/core/src/tools/write-file.test.ts b/packages/core/src/tools/write-file.test.ts index 3a0c8487b8..84fd4d93d7 100644 --- a/packages/core/src/tools/write-file.test.ts +++ b/packages/core/src/tools/write-file.test.ts @@ -310,6 +310,42 @@ describe('WriteFileTool', () => { }; expect(() => tool.build(params)).toThrow(`Missing or empty "file_path"`); }); + + it('should throw an error if content includes an omission placeholder', () => { + const params = { + file_path: path.join(rootDir, 'placeholder.txt'), + content: '(rest of methods ...)', + }; + expect(() => tool.build(params)).toThrow( + "`content` contains an omission placeholder (for example 'rest of methods ...'). Provide complete file content.", + ); + }); + + it('should throw an error when multiline content includes omission placeholders', () => { + const params = { + file_path: path.join(rootDir, 'service.ts'), + content: `class Service { + execute() { + return "run"; + } + + // rest of methods ... +}`, + }; + expect(() => tool.build(params)).toThrow( + "`content` contains an omission placeholder (for example 'rest of methods ...'). Provide complete file content.", + ); + }); + + it('should allow content with placeholder text in a normal string literal', () => { + const params = { + file_path: path.join(rootDir, 'valid-content.ts'), + content: 'const note = "(rest of methods ...)";', + }; + const invocation = tool.build(params); + expect(invocation).toBeDefined(); + expect(invocation.params).toEqual(params); + }); }); describe('getCorrectedFileContent', () => { diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index 3ad5838c95..d7708d767a 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -47,6 +47,7 @@ import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { debugLogger } from '../utils/debugLogger.js'; import { WRITE_FILE_DEFINITION } from './definitions/coreTools.js'; import { resolveToolDeclaration } from './definitions/resolver.js'; +import { detectOmissionPlaceholders } from './omissionPlaceholderDetector.js'; /** * Parameters for the WriteFile tool @@ -486,6 +487,11 @@ export class WriteFileTool }`; } + const omissionPlaceholders = detectOmissionPlaceholders(params.content); + if (omissionPlaceholders.length > 0) { + return "`content` contains an omission placeholder (for example 'rest of methods ...'). Provide complete file content."; + } + return null; }