diff --git a/packages/cli/src/utils/commentJson.test.ts b/packages/cli/src/utils/commentJson.test.ts index e92b19cb63..c5752ffd35 100644 --- a/packages/cli/src/utils/commentJson.test.ts +++ b/packages/cli/src/utils/commentJson.test.ts @@ -366,5 +366,71 @@ describe('commentJson', () => { expect(updatedContent).toContain('// This should be preserved'); }); + + it('should skip write if logical content has not changed', async () => { + const originalContent = `{ + "context": { + "fileName": ["AGENTS.md", "GEMINI.md"] + } + }\n`; + + fs.writeFileSync(testFilePath, originalContent, 'utf-8'); + const originalMtime = fs.statSync(testFilePath).mtimeMs; + + // Wait a bit to ensure mtime would change if a write happened + await new Promise((resolve) => setTimeout(resolve, 10)); + + updateSettingsFilePreservingFormat(testFilePath, { + context: { + fileName: ['AGENTS.md', 'GEMINI.md'], + }, + }); + + const newMtime = fs.statSync(testFilePath).mtimeMs; + expect(newMtime).toBe(originalMtime); + + const updatedContent = fs.readFileSync(testFilePath, 'utf-8'); + expect(updatedContent).toBe(originalContent); + }); + + it('should preserve trailing newline on legitimate update', () => { + const originalContent = `{ + "model": "gemini-2.5-pro" + }\n`; + + fs.writeFileSync(testFilePath, originalContent, 'utf-8'); + + updateSettingsFilePreservingFormat(testFilePath, { + model: 'gemini-2.5-flash', + }); + + const updatedContent = fs.readFileSync(testFilePath, 'utf-8'); + expect(updatedContent).toMatch(/\n$/); + expect(updatedContent).toContain('"model": "gemini-2.5-flash"'); + }); + + it('should add trailing newline to new files', () => { + updateSettingsFilePreservingFormat(testFilePath, { + model: 'gemini-2.5-pro', + }); + + const content = fs.readFileSync(testFilePath, 'utf-8'); + expect(content).toMatch(/\n$/); + }); + + it('should NOT add trailing newline if original file did not have one', () => { + const originalContent = `{ + "model": "gemini-2.5-pro" + }`; + + fs.writeFileSync(testFilePath, originalContent, 'utf-8'); + + updateSettingsFilePreservingFormat(testFilePath, { + model: 'gemini-2.5-flash', + }); + + const updatedContent = fs.readFileSync(testFilePath, 'utf-8'); + expect(updatedContent).not.toMatch(/\n$/); + }); }); }); diff --git a/packages/cli/src/utils/commentJson.ts b/packages/cli/src/utils/commentJson.ts index c60011b81f..1a29df8aa2 100644 --- a/packages/cli/src/utils/commentJson.ts +++ b/packages/cli/src/utils/commentJson.ts @@ -7,6 +7,7 @@ import * as fs from 'node:fs'; import { parse, stringify } from 'comment-json'; import { coreEvents } from '@google/gemini-cli-core'; +import stripJsonComments from 'strip-json-comments'; /** * Type representing an object that may contain Symbol keys for comments. @@ -21,16 +22,23 @@ export function updateSettingsFilePreservingFormat( updates: Record, ): void { if (!fs.existsSync(filePath)) { - fs.writeFileSync(filePath, JSON.stringify(updates, null, 2), 'utf-8'); + fs.writeFileSync(filePath, JSON.stringify(updates, null, 2) + '\n', 'utf-8'); return; } const originalContent = fs.readFileSync(filePath, 'utf-8'); + const hasTrailingNewline = originalContent.endsWith('\n'); let parsed: Record; + let cleanParsed: Record; try { // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion parsed = parse(originalContent) as Record; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + cleanParsed = JSON.parse(stripJsonComments(originalContent)) as Record< + string, + unknown + >; } catch (error) { coreEvents.emitFeedback( 'error', @@ -40,10 +48,57 @@ export function updateSettingsFilePreservingFormat( return; } + // First, check if logical content would change using the clean parse + const updatedClean = applyUpdates(structuredClone(cleanParsed), updates); + if (deepEqual(cleanParsed, updatedClean)) { + return; + } + + // If content changed, apply to the version with comments and write const updatedStructure = applyUpdates(parsed, updates); const updatedContent = stringify(updatedStructure, null, 2); + const finalContent = hasTrailingNewline + ? updatedContent + '\n' + : updatedContent; - fs.writeFileSync(filePath, updatedContent, 'utf-8'); + fs.writeFileSync(filePath, finalContent, 'utf-8'); +} + +/** + * Performs a deep equality check on two objects, ignoring Symbol keys. + * This is used to compare comment-json parsed objects without their comment metadata. + */ +function deepEqual(a: unknown, b: unknown): boolean { + if (a === b) return true; + + if (a && b && typeof a === 'object' && typeof b === 'object') { + if (Array.isArray(a) !== Array.isArray(b)) return false; + + if (Array.isArray(a) && Array.isArray(b)) { + if (a.length !== b.length) return false; + for (let i = 0; i < a.length; i++) { + if (!deepEqual(a[i], b[i])) return false; + } + return true; + } + + const keysA = Object.getOwnPropertyNames(a); + const keysB = Object.getOwnPropertyNames(b); + + if (keysA.length !== keysB.length) return false; + + for (const key of keysA) { + if (!Object.prototype.hasOwnProperty.call(b, key)) return false; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + if (!deepEqual((a as Record)[key], (b as Record)[key])) { + return false; + } + } + + return true; + } + + return false; } /**