Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
},
Expand Down Expand Up @@ -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: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
},
Expand Down Expand Up @@ -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: {
Expand Down
58 changes: 58 additions & 0 deletions packages/core/src/tools/edit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,64 @@ 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 the same 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 allow omission placeholder when the same placeholder already exists 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 ...)',
};
expect(tool.validateToolParams(params)).toBeNull();
});

it('should allow normal code that contains ellipsis 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();
});

it('should reject multiline omission placeholder edits that hide unchanged methods (regression #19858)', () => {
const params: EditToolParams = {
file_path: path.join(rootDir, 'service.ts'),
instruction: 'Rename one method and keep rest unchanged',
old_string: `class Service {
run() {
return "run";
}

helper() {
return "helper";
}
}`,
new_string: `class Service {
execute() {
return "run";
}

(rest of methods ...)
}`,
};
expect(tool.validateToolParams(params)).toBe(
"`new_string` contains an omission placeholder (for example 'rest of methods ...'). Provide exact literal replacement text.",
);
});
});

describe('execute', () => {
Expand Down
19 changes: 19 additions & 0 deletions packages/core/src/tools/edit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 { detectOmissionPlaceholder } from './omissionPlaceholderDetector.js';

const ENABLE_FUZZY_MATCH_RECOVERY = true;
const FUZZY_MATCH_THRESHOLD = 0.1; // Allow up to 10% weighted difference
Expand Down Expand Up @@ -973,6 +974,24 @@ export class EditTool
}
params.file_path = filePath;

const newPlaceholderMatch = detectOmissionPlaceholder(params.new_string);
if (newPlaceholderMatch.found) {
const oldPlaceholderMatch = detectOmissionPlaceholder(params.old_string);
const normalizedNewPlaceholder = newPlaceholderMatch.match
?.trim()
.toLowerCase();
const normalizedOldPlaceholder = oldPlaceholderMatch.match
?.trim()
.toLowerCase();

if (
!oldPlaceholderMatch.found ||
normalizedOldPlaceholder !== normalizedNewPlaceholder
) {
return "`new_string` contains an omission placeholder (for example 'rest of methods ...'). Provide exact literal replacement text.";
}
}
Comment thread
nsalerni marked this conversation as resolved.
Outdated

return this.config.validatePathAccess(params.file_path);
}

Expand Down
74 changes: 74 additions & 0 deletions packages/core/src/tools/omissionPlaceholderDetector.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

import { describe, expect, it } from 'vitest';
import { detectOmissionPlaceholder } from './omissionPlaceholderDetector.js';

describe('detectOmissionPlaceholder', () => {
it('detects standalone placeholder lines', () => {
expect(detectOmissionPlaceholder('(rest of methods ...)')).toEqual({
found: true,
match: '(rest of methods ...)',
});
expect(detectOmissionPlaceholder('(rest of code ...)')).toEqual({
found: true,
match: '(rest of code ...)',
});
expect(detectOmissionPlaceholder('(unchanged code ...)')).toEqual({
found: true,
match: '(unchanged code ...)',
});
expect(detectOmissionPlaceholder('// rest of methods ...')).toEqual({
found: true,
match: '// rest of methods ...',
});
});

it('detects case-insensitive placeholders', () => {
expect(detectOmissionPlaceholder('(Rest Of Methods ...)')).toEqual({
found: true,
match: '(Rest Of Methods ...)',
});
});

it('detects placeholder lines inside larger multiline text', () => {
const text = `class Example {\n methodA() {}\n (rest of methods ...)\n}`;
expect(detectOmissionPlaceholder(text)).toEqual({
found: true,
match: '(rest of methods ...)',
});
});

it('does not detect placeholders embedded in normal code', () => {
expect(
detectOmissionPlaceholder(
'const note = "(rest of methods ...)";\nconsole.log(note);',
),
).toEqual({
found: false,
});
});

it('does not detect text without ellipsis marker', () => {
expect(detectOmissionPlaceholder('(rest of methods)')).toEqual({
found: false,
});
});

it('does not detect unrelated ellipsis text', () => {
expect(detectOmissionPlaceholder('const message = "loading...";')).toEqual({
found: false,
});
});

it('does not detect omission phrase when it is inline code/comment context', () => {
expect(
detectOmissionPlaceholder('return value; // rest of methods ...'),
).toEqual({
found: false,
});
});
});
50 changes: 50 additions & 0 deletions packages/core/src/tools/omissionPlaceholderDetector.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

export interface OmissionPlaceholderDetectionResult {
found: boolean;
match?: string;
}

const STANDALONE_OMISSION_LINE_PATTERN =
/^(?:\/\/\s*)?\(?\s*(?:rest\s+of(?:\s+(?:methods?|code))?|unchanged\s+(?:code|methods?))\s*\.{3,}\s*\)?$/i;

/**
* Detects shorthand omission placeholders such as:
* - (rest of methods ...)
* - (rest of code ...)
* - (unchanged code ...)
* - // rest of methods ...
*/
export function detectOmissionPlaceholder(
text: string,
): OmissionPlaceholderDetectionResult {
const lines = text.split(/\r?\n/);

for (const rawLine of lines) {
const line = rawLine.trim();
if (!line) {
continue;
}

if (!line.includes('...')) {
continue;
}

if (
!/rest\s+of/i.test(line) &&
!/unchanged\s+(?:code|methods?)/i.test(line)
) {
continue;
}

if (STANDALONE_OMISSION_LINE_PATTERN.test(line)) {
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
return { found: true, match: line };
}
}

return { found: false };
}
Comment thread
nsalerni marked this conversation as resolved.
Outdated
36 changes: 36 additions & 0 deletions packages/core/src/tools/write-file.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 allow content with ellipsis 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);
});

it('should reject omission placeholder within multiline file content (regression #19858)', () => {
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.",
);
});
});

describe('getCorrectedFileContent', () => {
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/tools/write-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 { detectOmissionPlaceholder } from './omissionPlaceholderDetector.js';

/**
* Parameters for the WriteFile tool
Expand Down Expand Up @@ -486,6 +487,11 @@ export class WriteFileTool
}`;
}

const omissionPlaceholderMatch = detectOmissionPlaceholder(params.content);
if (omissionPlaceholderMatch.found) {
return "`content` contains an omission placeholder (for example 'rest of methods ...'). Provide complete file content.";
}
Comment thread
nsalerni marked this conversation as resolved.
Outdated

return null;
}

Expand Down
Loading