fix(core): prevent omission placeholder deletions in replace/write_file#19870
fix(core): prevent omission placeholder deletions in replace/write_file#19870bdmorgan merged 4 commits intogoogle-gemini:mainfrom
Conversation
Add guardrails that reject standalone omission placeholders (for example `(rest of methods ...)`) in `replace.new_string` and `write_file.content`, while allowing pass-through when the same placeholder already exists in `replace.old_string`. Also tighten model-facing tool descriptions and add regression coverage. Regression proof: - Before this fix (tested in a detached temp worktree at the pre-change commit), new regression assertions failed: - `EditTool.validateToolParams` regression expected omission-placeholder error, got `null` - `WriteFileTool.build` regression expected throw, but no error was thrown - After this fix, the same regression scenarios now pass. Tests: - `npm run test --workspace @google/gemini-cli-core -- src/tools/omissionPlaceholderDetector.test.ts` - `npm run test --workspace @google/gemini-cli-core -- src/tools/edit.test.ts` - `npm run test --workspace @google/gemini-cli-core -- src/tools/write-file.test.ts` - `npm run test --workspace @google/gemini-cli-core -- src/tools/definitions/coreToolsModelSnapshots.test.ts` Fixes google-gemini#19858
Summary of ChangesHello @nsalerni, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical issue where AI models inadvertently introduced large, misleading deletions in code by replacing unchanged sections with omission placeholders. The changes introduce robust validation mechanisms within the core Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces validation to prevent the model from using omission placeholders like (rest of methods ...) in file edit and write operations, which was causing unintended code deletions. The approach of adding validation guards in replace and write_file, strengthening tool descriptions, and adding regression tests is solid. However, I've identified a critical flaw in the placeholder detection logic for the edit tool that could allow the original bug to persist in certain scenarios. My review includes a detailed explanation and suggestions for a more robust fix.
Summary
Prevents a reported failure mode where edit/write proposals replace unchanged code with omission placeholders like
(rest of methods ...), which shows up as large unintended deletions in diffs.This PR adds validation guardrails in
replaceandwrite_file, strengthens model-facing tool descriptions to forbid omission placeholders, and adds regression coverage for the issue pattern.Details
The issue described model edits that replace unchanged code with placeholder lines like (rest of methods ...), causing large red deletions in diffs. This fix now blocks those payloads at tool validation time in both:
So those placeholder-based edits no longer execute, which prevents that class of misleading diff/deletion.
It’s targeted to standalone omission-placeholder lines (the problematic pattern), not arbitrary ... inside normal code/string literals.
packages/core/src/tools/omissionPlaceholderDetector.ts(rest of methods ...)(rest of code ...)(unchanged code ...)// rest of methods ...packages/core/src/tools/edit.tsnew_stringomission placeholders unless the same placeholder is already present inold_string.packages/core/src/tools/write-file.tscontent.packages/core/src/tools/definitions/model-family-sets/gemini-3.tspackages/core/src/tools/definitions/model-family-sets/default-legacy.tspackages/core/src/tools/omissionPlaceholderDetector.test.tspackages/core/src/tools/edit.test.tspackages/core/src/tools/write-file.test.tspackages/core/src/tools/definitions/__snapshots__/coreToolsModelSnapshots.test.ts.snapRegression proof:
EditTool.validateToolParamsreturnednull(no rejection) for placeholder-containingnew_string.WriteFileTool.builddid not throw for placeholder-containingcontent.Related Issues
Fixes #19858
How to Validate
Run detector tests:
npm run test --workspace @google/gemini-cli-core -- src/tools/omissionPlaceholderDetector.test.tsRun edit tool tests:
npm run test --workspace @google/gemini-cli-core -- src/tools/edit.test.tsRun write-file tool tests:
npm run test --workspace @google/gemini-cli-core -- src/tools/write-file.test.tsRun tool-definition snapshot test:
npm run test --workspace @google/gemini-cli-core -- src/tools/definitions/coreToolsModelSnapshots.test.tsEdge cases covered by tests:
replaceallows pass-through only when the same placeholder is already inold_string.Pre-Merge Checklist
Breaking changes:
replace/write_filenow reject omission-placeholder payloads during validation.