From ac8e39409104607b160e86b712cfc6d4637247dd Mon Sep 17 00:00:00 2001 From: dyoshikawa Date: Sun, 26 Jul 2026 23:05:57 -0700 Subject: [PATCH 1/3] fix(generate): stop creating empty shared config files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Running `rulesync generate` left untracked, contentless files behind for shared config paths that rulesync merges into but does not own — e.g. `.antigravity/settings.json` (`{}`), `.factory/settings.json` (`{}`) and `.vscode/settings.json`. Those paths are deliberately excluded from `rulesync gitignore` (DERIVED_PATHS_NOT_GITIGNORED), so every generate run handed the user a file to manage with nothing in it. Two causes, both fixed: - `readOrInitializeFileContent` wrote the initial content to disk as a side effect, so a merge-target file was materialized before `writeAiFiles` ever decided whether it had anything to write. Every generator now reads with `readFileContentOrNull(...) ?? ` instead, matching the pattern already used by copilot/zed/qwencode permissions. This also removes a filesystem side effect from `--dry-run` / `--check` across ~30 generators. The util itself is dropped so the eager-write path cannot come back. - `writeAiFiles` now skips creating a file that does not exist yet when the generated payload is empty and the file is a shared config file rulesync does not own (`AiFile#shouldSkipCreationWhenPayloadEmpty()`, defaulting to `!isDeletable()`). An already-existing file is still rewritten, so user-authored content is never dropped, and files rulesync fully owns are still created. `fileContentIsEmptyPayload` decides emptiness on the parsed document (JSON/JSONC/YAML/TOML), so `{}`, `{"permissions":{}}` and `{"mcpServers":{}}` all count as empty while any scalar counts as content. Co-Authored-By: Claude Opus 5 (1M context) --- src/features/hooks/antigravity-hooks.ts | 7 +- src/features/hooks/augmentcode-hooks.ts | 7 +- src/features/hooks/claudecode-hooks.ts | 7 +- src/features/hooks/devin-hooks.ts | 13 ++- src/features/hooks/factorydroid-hooks.ts | 7 +- src/features/hooks/grokcli-hooks.ts | 8 +- src/features/hooks/junie-hooks.ts | 7 +- src/features/hooks/kiro-hooks.test.ts | 4 +- src/features/hooks/kiro-hooks.ts | 7 +- src/features/hooks/qwencode-hooks.test.ts | 4 +- src/features/hooks/qwencode-hooks.ts | 7 +- src/features/hooks/reasonix-hooks.ts | 7 +- src/features/mcp/antigravity-mcp.ts | 10 +-- src/features/mcp/augmentcode-mcp.ts | 7 +- src/features/mcp/claudecode-mcp.ts | 10 +-- src/features/mcp/cline-mcp.ts | 10 +-- src/features/mcp/copilotcli-mcp.ts | 10 +-- src/features/mcp/cursor-mcp.ts | 10 +-- src/features/mcp/deepagents-mcp.ts | 10 +-- src/features/mcp/devin-mcp.ts | 8 +- src/features/mcp/goose-mcp.ts | 10 +-- src/features/mcp/hermesagent-mcp.ts | 10 +-- src/features/mcp/qwencode-mcp.ts | 8 +- src/features/mcp/rovodev-mcp.ts | 10 +-- src/features/mcp/warp-mcp.ts | 10 +-- src/features/mcp/zed-mcp.ts | 4 +- .../antigravity-cli-permissions.ts | 7 +- .../antigravity-ide-permissions.ts | 7 +- .../permissions/claudecode-permissions.ts | 7 +- .../permissions/cursor-permissions.ts | 7 +- src/features/permissions/devin-permissions.ts | 7 +- .../permissions/factorydroid-permissions.ts | 7 +- src/features/permissions/junie-permissions.ts | 4 +- .../permissions/qwencode-permissions.ts | 5 +- src/types/ai-file.ts | 15 ++++ src/types/feature-processor.test.ts | 82 ++++++++++++++++++- src/types/feature-processor.ts | 14 +++- src/utils/content-equivalence.test.ts | 59 ++++++++++++- src/utils/content-equivalence.ts | 79 ++++++++++++++++++ src/utils/file.test.ts | 79 ------------------ src/utils/file.ts | 13 --- 41 files changed, 350 insertions(+), 254 deletions(-) diff --git a/src/features/hooks/antigravity-hooks.ts b/src/features/hooks/antigravity-hooks.ts index 574d26905..5fe9edcd8 100644 --- a/src/features/hooks/antigravity-hooks.ts +++ b/src/features/hooks/antigravity-hooks.ts @@ -12,7 +12,7 @@ import { CANONICAL_TO_ANTIGRAVITY_EVENT_NAMES, } from "../../types/hooks.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import type { Logger } from "../../utils/logger.js"; import { isPrototypePollutionKey } from "../../utils/prototype-pollution.js"; import type { RulesyncHooks } from "./rulesync-hooks.js"; @@ -161,11 +161,8 @@ class AntigravityHooks extends ToolHooks { logger?: Logger; }): Promise { const paths = this.getSettablePaths({ global }); - const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); // hooks.json is dedicated to hooks, so any existing content is fully - // replaced; reading it first keeps a stable round-trip when unchanged. - await readOrInitializeFileContent(filePath, JSON.stringify({}, null, 2)); - + // replaced; the write happens later in `writeAiFiles`. const config = rulesyncHooks.getJson(); const eventMap = canonicalToToolHooks({ config, diff --git a/src/features/hooks/augmentcode-hooks.ts b/src/features/hooks/augmentcode-hooks.ts index 6466c85d8..03dfab91c 100644 --- a/src/features/hooks/augmentcode-hooks.ts +++ b/src/features/hooks/augmentcode-hooks.ts @@ -13,7 +13,7 @@ import { } from "../../types/hooks.js"; import { readAugmentcodeSettingsWithLocalOverlay } from "../../utils/augmentcode-settings.js"; import { formatError } from "../../utils/error.js"; -import { readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import type { Logger } from "../../utils/logger.js"; import { applySharedConfigPatch, sharedConfigFileKey } from "../shared/shared-config-gateway.js"; import type { RulesyncHooks } from "./rulesync-hooks.js"; @@ -122,10 +122,7 @@ export class AugmentcodeHooks extends ToolHooks { }): Promise { const paths = AugmentcodeHooks.getSettablePaths({ global }); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - const existingContent = await readOrInitializeFileContent( - filePath, - JSON.stringify({}, null, 2), - ); + const existingContent = (await readFileContentOrNull(filePath)) ?? JSON.stringify({}, null, 2); const config = rulesyncHooks.getJson(); const augmentHooks = canonicalToToolHooks({ config, diff --git a/src/features/hooks/claudecode-hooks.ts b/src/features/hooks/claudecode-hooks.ts index 2e81cd78c..1f3e8d688 100644 --- a/src/features/hooks/claudecode-hooks.ts +++ b/src/features/hooks/claudecode-hooks.ts @@ -9,7 +9,7 @@ import { CANONICAL_TO_CLAUDE_EVENT_NAMES, } from "../../types/hooks.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import type { Logger } from "../../utils/logger.js"; import { applySharedConfigPatch, @@ -110,10 +110,7 @@ export class ClaudecodeHooks extends ToolHooks { }): Promise { const paths = this.getSettablePaths({ global }); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - const existingContent = await readOrInitializeFileContent( - filePath, - JSON.stringify({}, null, 2), - ); + const existingContent = (await readFileContentOrNull(filePath)) ?? JSON.stringify({}, null, 2); const config = rulesyncHooks.getJson(); const claudeHooks = canonicalToToolHooks({ config, diff --git a/src/features/hooks/devin-hooks.ts b/src/features/hooks/devin-hooks.ts index 339a58d85..f723c49d9 100644 --- a/src/features/hooks/devin-hooks.ts +++ b/src/features/hooks/devin-hooks.ts @@ -13,7 +13,7 @@ import { DEVIN_TO_CANONICAL_EVENT_NAMES, } from "../../types/hooks.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import type { Logger } from "../../utils/logger.js"; import { isRecord } from "../../utils/type-guards.js"; import { applySharedConfigPatch, sharedConfigFileKey } from "../shared/shared-config-gateway.js"; @@ -149,10 +149,8 @@ export class DevinHooks extends ToolHooks { // Global hooks live under the `hooks` key of the shared config.json, which // also carries `mcpServers` / `permissions` from the other features, so // read-modify-write and preserve the sibling keys. - const existingContent = await readOrInitializeFileContent( - filePath, - JSON.stringify({}, null, 2), - ); + const existingContent = + (await readFileContentOrNull(filePath)) ?? JSON.stringify({}, null, 2); fileContent = applySharedConfigPatch({ fileKey: sharedConfigFileKey(paths), feature: "hooks", @@ -161,9 +159,8 @@ export class DevinHooks extends ToolHooks { filePath, }); } else { - // The project hooks.v1.json is dedicated to hooks; reading it first keeps a - // stable round-trip when unchanged. - await readOrInitializeFileContent(filePath, JSON.stringify({}, null, 2)); + // The project hooks.v1.json is dedicated to hooks, so any existing content + // is fully replaced; the write happens later in `writeAiFiles`. fileContent = JSON.stringify(devinHooks, null, 2); } diff --git a/src/features/hooks/factorydroid-hooks.ts b/src/features/hooks/factorydroid-hooks.ts index 9f8bc9c45..8b39b3e1c 100644 --- a/src/features/hooks/factorydroid-hooks.ts +++ b/src/features/hooks/factorydroid-hooks.ts @@ -13,7 +13,7 @@ import { CANONICAL_TO_FACTORYDROID_EVENT_NAMES, } from "../../types/hooks.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import type { Logger } from "../../utils/logger.js"; import type { RulesyncHooks } from "./rulesync-hooks.js"; import type { ToolHooksConverterConfig } from "./tool-hooks-converter.js"; @@ -96,10 +96,7 @@ export class FactorydroidHooks extends ToolHooks { }): Promise { const paths = FactorydroidHooks.getSettablePaths({ global }); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - const existingContent = await readOrInitializeFileContent( - filePath, - JSON.stringify({}, null, 2), - ); + const existingContent = (await readFileContentOrNull(filePath)) ?? JSON.stringify({}, null, 2); let settings: Record; try { settings = JSON.parse(existingContent); diff --git a/src/features/hooks/grokcli-hooks.ts b/src/features/hooks/grokcli-hooks.ts index 7849d3068..9d48b8c0c 100644 --- a/src/features/hooks/grokcli-hooks.ts +++ b/src/features/hooks/grokcli-hooks.ts @@ -8,7 +8,7 @@ import { GROKCLI_TO_CANONICAL_EVENT_NAMES, } from "../../types/hooks.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import type { Logger } from "../../utils/logger.js"; import { isRecord } from "../../utils/type-guards.js"; import type { RulesyncHooks } from "./rulesync-hooks.js"; @@ -126,7 +126,6 @@ export class GrokcliHooks extends ToolHooks { logger?: Logger; }): Promise { const paths = GrokcliHooks.getSettablePaths({ global }); - const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); const config = rulesyncHooks.getJson(); const grokHooks = canonicalToToolHooks({ config, @@ -134,9 +133,8 @@ export class GrokcliHooks extends ToolHooks { converterConfig: GROKCLI_CONVERTER_CONFIG, logger, }); - // The standalone rulesync.json is dedicated to hooks; reading it first keeps - // a stable round-trip when unchanged. - await readOrInitializeFileContent(filePath, JSON.stringify({ hooks: {} }, null, 2)); + // The standalone rulesync.json is dedicated to hooks, so any existing + // content is fully replaced; the write happens later in `writeAiFiles`. const fileContent = JSON.stringify({ hooks: grokHooks }, null, 2); return new GrokcliHooks({ outputRoot, diff --git a/src/features/hooks/junie-hooks.ts b/src/features/hooks/junie-hooks.ts index d85e0f457..7806216aa 100644 --- a/src/features/hooks/junie-hooks.ts +++ b/src/features/hooks/junie-hooks.ts @@ -9,7 +9,7 @@ import { JUNIE_TO_CANONICAL_EVENT_NAMES, } from "../../types/hooks.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import type { Logger } from "../../utils/logger.js"; import type { RulesyncHooks } from "./rulesync-hooks.js"; import type { ToolHooksConverterConfig } from "./tool-hooks-converter.js"; @@ -101,10 +101,7 @@ export class JunieHooks extends ToolHooks { }): Promise { const paths = JunieHooks.getSettablePaths({ global }); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - const existingContent = await readOrInitializeFileContent( - filePath, - JSON.stringify({}, null, 2), - ); + const existingContent = (await readFileContentOrNull(filePath)) ?? JSON.stringify({}, null, 2); let settings: Record; try { settings = JSON.parse(existingContent); diff --git a/src/features/hooks/kiro-hooks.test.ts b/src/features/hooks/kiro-hooks.test.ts index 0550011c0..b57a2cda5 100644 --- a/src/features/hooks/kiro-hooks.test.ts +++ b/src/features/hooks/kiro-hooks.test.ts @@ -3,7 +3,7 @@ import { join } from "node:path"; import { describe, expect, it, beforeEach, afterEach } from "vitest"; import { setupTestDirectory } from "../../test-utils/test-directories.js"; -import { readOrInitializeFileContent, ensureDir, writeFileContent } from "../../utils/file.js"; +import { ensureDir, writeFileContent } from "../../utils/file.js"; import { KiroHooks } from "./kiro-hooks.js"; import { RulesyncHooks } from "./rulesync-hooks.js"; @@ -170,7 +170,7 @@ describe("KiroHooks", () => { const configPath = join(testDir, ".kiro", "agents", "default.json"); await ensureDir(join(testDir, ".kiro", "agents")); - await readOrInitializeFileContent(configPath, JSON.stringify(mockConfig)); + await writeFileContent(configPath, JSON.stringify(mockConfig)); const rulesyncHooks = new RulesyncHooks( createMockAiFileParams({ diff --git a/src/features/hooks/kiro-hooks.ts b/src/features/hooks/kiro-hooks.ts index 88b851ee2..cc3c2e2b9 100644 --- a/src/features/hooks/kiro-hooks.ts +++ b/src/features/hooks/kiro-hooks.ts @@ -13,7 +13,7 @@ import { safeString, } from "../../types/hooks.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import { applySharedConfigPatch, sharedConfigFileKey } from "../shared/shared-config-gateway.js"; import type { RulesyncHooks } from "./rulesync-hooks.js"; import { buildImportedHooksConfig } from "./tool-hooks-converter.js"; @@ -195,10 +195,7 @@ export class KiroHooks extends ToolHooks { }: ToolHooksFromRulesyncHooksParams & { global?: boolean }): Promise { const paths = KiroHooks.getSettablePaths({ global }); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - const existingContent = await readOrInitializeFileContent( - filePath, - JSON.stringify({}, null, 2), - ); + const existingContent = (await readFileContentOrNull(filePath)) ?? JSON.stringify({}, null, 2); const config = rulesyncHooks.getJson(); const kiroHooks = canonicalToKiroHooks(config, this.getOverrideKey()); const fileContent = applySharedConfigPatch({ diff --git a/src/features/hooks/qwencode-hooks.test.ts b/src/features/hooks/qwencode-hooks.test.ts index a22669238..31b402330 100644 --- a/src/features/hooks/qwencode-hooks.test.ts +++ b/src/features/hooks/qwencode-hooks.test.ts @@ -3,7 +3,7 @@ import { join } from "node:path"; import { describe, expect, it, beforeEach, afterEach, vi } from "vitest"; import { setupTestDirectory } from "../../test-utils/test-directories.js"; -import { readOrInitializeFileContent, ensureDir, writeFileContent } from "../../utils/file.js"; +import { ensureDir, writeFileContent } from "../../utils/file.js"; import { QwencodeHooks } from "./qwencode-hooks.js"; import { RulesyncHooks } from "./rulesync-hooks.js"; @@ -136,7 +136,7 @@ describe("QwencodeHooks", () => { const settingsPath = join(testDir, ".qwen", "settings.json"); await ensureDir(join(testDir, ".qwen")); - await readOrInitializeFileContent(settingsPath, JSON.stringify(mockSettings)); + await writeFileContent(settingsPath, JSON.stringify(mockSettings)); const rulesyncHooks = new RulesyncHooks( createMockAiFileParams({ diff --git a/src/features/hooks/qwencode-hooks.ts b/src/features/hooks/qwencode-hooks.ts index dc6863dc4..fab832f1f 100644 --- a/src/features/hooks/qwencode-hooks.ts +++ b/src/features/hooks/qwencode-hooks.ts @@ -12,7 +12,7 @@ import { CANONICAL_TO_QWENCODE_EVENT_NAMES, } from "../../types/hooks.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import { compact } from "../../utils/object.js"; import { applySharedConfigPatch, sharedConfigFileKey } from "../shared/shared-config-gateway.js"; import type { RulesyncHooks } from "./rulesync-hooks.js"; @@ -268,10 +268,7 @@ export class QwencodeHooks extends ToolHooks { }: ToolHooksFromRulesyncHooksParams & { global?: boolean }): Promise { const paths = QwencodeHooks.getSettablePaths({ global }); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - const existingContent = await readOrInitializeFileContent( - filePath, - JSON.stringify({}, null, 2), - ); + const existingContent = (await readFileContentOrNull(filePath)) ?? JSON.stringify({}, null, 2); const config = rulesyncHooks.getJson(); const patch: Record = { hooks: canonicalToQwencodeHooks(config) }; // Round-trip Qwen Code's top-level switch that disables every hook. diff --git a/src/features/hooks/reasonix-hooks.ts b/src/features/hooks/reasonix-hooks.ts index 6ec0b3fe9..d631503e9 100644 --- a/src/features/hooks/reasonix-hooks.ts +++ b/src/features/hooks/reasonix-hooks.ts @@ -10,7 +10,7 @@ import { REASONIX_TO_CANONICAL_EVENT_NAMES, } from "../../types/hooks.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import type { Logger } from "../../utils/logger.js"; import type { RulesyncHooks } from "./rulesync-hooks.js"; import { buildImportedHooksConfig } from "./tool-hooks-converter.js"; @@ -212,10 +212,7 @@ export class ReasonixHooks extends ToolHooks { }): Promise { const paths = ReasonixHooks.getSettablePaths({ global }); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - const existingContent = await readOrInitializeFileContent( - filePath, - JSON.stringify({}, null, 2), - ); + const existingContent = (await readFileContentOrNull(filePath)) ?? JSON.stringify({}, null, 2); let settings: Record; try { settings = JSON.parse(existingContent); diff --git a/src/features/mcp/antigravity-mcp.ts b/src/features/mcp/antigravity-mcp.ts index 1c3231b75..2ad24bce3 100644 --- a/src/features/mcp/antigravity-mcp.ts +++ b/src/features/mcp/antigravity-mcp.ts @@ -6,7 +6,7 @@ import { ANTIGRAVITY_MCP_FILE_NAME, } from "../../constants/antigravity-paths.js"; import { ValidationResult } from "../../types/ai-file.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import { RulesyncMcp } from "./rulesync-mcp.js"; import { ToolMcp, @@ -126,10 +126,10 @@ export class AntigravityMcp extends ToolMcp { }: ToolMcpFromRulesyncMcpParams): Promise { const paths = this.getSettablePaths({ global }); - const fileContent = await readOrInitializeFileContent( - join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), - JSON.stringify({ mcpServers: {} }, null, 2), - ); + const fileContent = + (await readFileContentOrNull( + join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), + )) ?? JSON.stringify({ mcpServers: {} }, null, 2); const json = JSON.parse(fileContent); const newJson = { ...json, mcpServers: toAntigravityMcpServers(rulesyncMcp.getMcpServers()) }; diff --git a/src/features/mcp/augmentcode-mcp.ts b/src/features/mcp/augmentcode-mcp.ts index 088aadbcc..ae8d2b6d4 100644 --- a/src/features/mcp/augmentcode-mcp.ts +++ b/src/features/mcp/augmentcode-mcp.ts @@ -8,7 +8,7 @@ import { ValidationResult } from "../../types/ai-file.js"; import { isMcpServers } from "../../types/mcp.js"; import { readAugmentcodeSettingsWithLocalOverlay } from "../../utils/augmentcode-settings.js"; import { formatError } from "../../utils/error.js"; -import { readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import { isPlainObject } from "../../utils/type-guards.js"; import { applySharedConfigPatch, sharedConfigFileKey } from "../shared/shared-config-gateway.js"; import { RulesyncMcp } from "./rulesync-mcp.js"; @@ -137,10 +137,7 @@ export class AugmentcodeMcp extends ToolMcp { const paths = this.getSettablePaths({ global }); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - const existingContent = await readOrInitializeFileContent( - filePath, - JSON.stringify({}, null, 2), - ); + const existingContent = (await readFileContentOrNull(filePath)) ?? JSON.stringify({}, null, 2); return new AugmentcodeMcp({ outputRoot, diff --git a/src/features/mcp/claudecode-mcp.ts b/src/features/mcp/claudecode-mcp.ts index d3102b59e..c01b38a3c 100644 --- a/src/features/mcp/claudecode-mcp.ts +++ b/src/features/mcp/claudecode-mcp.ts @@ -6,7 +6,7 @@ import { CLAUDECODE_MCP_FILE_NAME, } from "../../constants/claudecode-paths.js"; import { ValidationResult } from "../../types/ai-file.js"; -import { fileExists, readFileContent, readOrInitializeFileContent } from "../../utils/file.js"; +import { fileExists, readFileContent, readFileContentOrNull } from "../../utils/file.js"; import { RulesyncMcp } from "./rulesync-mcp.js"; import { ToolMcp, @@ -133,10 +133,10 @@ export class ClaudecodeMcp extends ToolMcp { }: ToolMcpFromRulesyncMcpParams): Promise { const paths = this.getSettablePaths({ global }); - const fileContent = await readOrInitializeFileContent( - join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), - JSON.stringify({ mcpServers: {} }, null, 2), - ); + const fileContent = + (await readFileContentOrNull( + join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), + )) ?? JSON.stringify({ mcpServers: {} }, null, 2); const json = JSON.parse(fileContent); const mcpJson = { ...json, mcpServers: rulesyncMcp.getMcpServers() }; diff --git a/src/features/mcp/cline-mcp.ts b/src/features/mcp/cline-mcp.ts index 5b660b9b7..d5d706b28 100644 --- a/src/features/mcp/cline-mcp.ts +++ b/src/features/mcp/cline-mcp.ts @@ -4,7 +4,7 @@ import { CLINE_MCP_DIR_PATH, CLINE_MCP_FILE_NAME } from "../../constants/cline-p import { ValidationResult } from "../../types/ai-file.js"; import { isMcpServers } from "../../types/mcp.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import { isPlainObject } from "../../utils/type-guards.js"; import { RulesyncMcp } from "./rulesync-mcp.js"; import { @@ -122,10 +122,10 @@ export class ClineMcp extends ToolMcp { } const paths = this.getSettablePaths({ global }); - const fileContent = await readOrInitializeFileContent( - join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), - JSON.stringify({}, null, 2), - ); + const fileContent = + (await readFileContentOrNull( + join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), + )) ?? JSON.stringify({}, null, 2); const json = parseClineSettings(fileContent, paths.relativeDirPath, paths.relativeFilePath); // Merge `mcpServers` into the existing settings, preserving other keys. diff --git a/src/features/mcp/copilotcli-mcp.ts b/src/features/mcp/copilotcli-mcp.ts index 16c8765d5..1afbc36f4 100644 --- a/src/features/mcp/copilotcli-mcp.ts +++ b/src/features/mcp/copilotcli-mcp.ts @@ -8,7 +8,7 @@ import { } from "../../constants/copilot-paths.js"; import { ValidationResult } from "../../types/ai-file.js"; import { McpServerSchema, type McpServer, type McpServers } from "../../types/mcp.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import { RulesyncMcp } from "./rulesync-mcp.js"; import { ToolMcp, @@ -195,10 +195,10 @@ export class CopilotcliMcp extends ToolMcp { }: ToolMcpFromRulesyncMcpParams): Promise { const paths = this.getSettablePaths({ global }); - const fileContent = await readOrInitializeFileContent( - join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), - JSON.stringify({ mcpServers: {} }, null, 2), - ); + const fileContent = + (await readFileContentOrNull( + join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), + )) ?? JSON.stringify({ mcpServers: {} }, null, 2); const json = JSON.parse(fileContent); // Convert rulesync format to Copilot CLI format (add "type": "stdio") diff --git a/src/features/mcp/cursor-mcp.ts b/src/features/mcp/cursor-mcp.ts index 117a79857..706150b06 100644 --- a/src/features/mcp/cursor-mcp.ts +++ b/src/features/mcp/cursor-mcp.ts @@ -4,7 +4,7 @@ import { CURSOR_DIR, CURSOR_MCP_FILE_NAME } from "../../constants/cursor-paths.j import { ValidationResult } from "../../types/ai-file.js"; import { isMcpServers } from "../../types/mcp.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import { convertEnvVarRefsFromToolFormat, convertEnvVarRefsToToolFormat, @@ -96,10 +96,10 @@ export class CursorMcp extends ToolMcp { }: ToolMcpFromRulesyncMcpParams): Promise { const paths = this.getSettablePaths({ global }); - const fileContent = await readOrInitializeFileContent( - join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), - JSON.stringify({ mcpServers: {} }, null, 2), - ); + const fileContent = + (await readFileContentOrNull( + join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), + )) ?? JSON.stringify({ mcpServers: {} }, null, 2); let json: Record; try { json = JSON.parse(fileContent); diff --git a/src/features/mcp/deepagents-mcp.ts b/src/features/mcp/deepagents-mcp.ts index be83eead0..dc02691c1 100644 --- a/src/features/mcp/deepagents-mcp.ts +++ b/src/features/mcp/deepagents-mcp.ts @@ -2,7 +2,7 @@ import { join } from "node:path"; import { DEEPAGENTS_DIR, DEEPAGENTS_MCP_FILE_NAME } from "../../constants/deepagents-paths.js"; import { ValidationResult } from "../../types/ai-file.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import { RulesyncMcp } from "./rulesync-mcp.js"; import { ToolMcp, @@ -66,10 +66,10 @@ export class DeepagentsMcp extends ToolMcp { }: ToolMcpFromRulesyncMcpParams): Promise { const paths = this.getSettablePaths({ global }); - const fileContent = await readOrInitializeFileContent( - join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), - JSON.stringify({ mcpServers: {} }, null, 2), - ); + const fileContent = + (await readFileContentOrNull( + join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), + )) ?? JSON.stringify({ mcpServers: {} }, null, 2); const json = JSON.parse(fileContent); const mcpJson = { ...json, mcpServers: rulesyncMcp.getMcpServers() }; diff --git a/src/features/mcp/devin-mcp.ts b/src/features/mcp/devin-mcp.ts index ce6a25fbe..04be59f60 100644 --- a/src/features/mcp/devin-mcp.ts +++ b/src/features/mcp/devin-mcp.ts @@ -7,7 +7,7 @@ import { } from "../../constants/devin-paths.js"; import { ValidationResult } from "../../types/ai-file.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import { applySharedConfigPatch, sharedConfigFileKey } from "../shared/shared-config-gateway.js"; import { RulesyncMcp } from "./rulesync-mcp.js"; import { @@ -120,10 +120,8 @@ export class DevinMcp extends ToolMcp { const paths = this.getSettablePaths({ global }); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - const existingContent = await readOrInitializeFileContent( - filePath, - JSON.stringify({ mcpServers: {} }, null, 2), - ); + const existingContent = + (await readFileContentOrNull(filePath)) ?? JSON.stringify({ mcpServers: {} }, null, 2); return new DevinMcp({ outputRoot, diff --git a/src/features/mcp/goose-mcp.ts b/src/features/mcp/goose-mcp.ts index b885f9d12..bd4a25f0f 100644 --- a/src/features/mcp/goose-mcp.ts +++ b/src/features/mcp/goose-mcp.ts @@ -11,7 +11,7 @@ import { import { ValidationResult } from "../../types/ai-file.js"; import { McpServers } from "../../types/mcp.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import type { Logger } from "../../utils/logger.js"; import { warnWithFallback } from "../../utils/logger.js"; import { @@ -384,10 +384,10 @@ export class GooseMcp extends ToolMcp { }); } - const fileContent = await readOrInitializeFileContent( - join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), - "", - ); + const fileContent = + (await readFileContentOrNull( + join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), + )) ?? ""; const config = parseGooseConfig(fileContent, paths.relativeDirPath, paths.relativeFilePath); // Merge the `extensions:` block into the shared config, preserving other diff --git a/src/features/mcp/hermesagent-mcp.ts b/src/features/mcp/hermesagent-mcp.ts index 632b61ac3..a2adc8a9b 100644 --- a/src/features/mcp/hermesagent-mcp.ts +++ b/src/features/mcp/hermesagent-mcp.ts @@ -6,7 +6,7 @@ import { } from "../../constants/hermesagent-paths.js"; import { ValidationResult } from "../../types/ai-file.js"; import { McpServers } from "../../types/mcp.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import { omitPrototypePollutionKeys, PROTOTYPE_POLLUTION_KEYS, @@ -366,10 +366,10 @@ export class HermesagentMcp extends ToolMcp { } const paths = this.getSettablePaths({ global }); - const fileContent = await readOrInitializeFileContent( - join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), - "", - ); + const fileContent = + (await readFileContentOrNull( + join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), + )) ?? ""; const config = parseSharedConfig({ format: "yaml", fileContent }); // Merge the `mcp_servers:` block into the shared config, preserving other diff --git a/src/features/mcp/qwencode-mcp.ts b/src/features/mcp/qwencode-mcp.ts index 63ea5f01d..87c390873 100644 --- a/src/features/mcp/qwencode-mcp.ts +++ b/src/features/mcp/qwencode-mcp.ts @@ -3,7 +3,7 @@ import { join } from "node:path"; import { QWENCODE_DIR, QWENCODE_SETTINGS_FILE_NAME } from "../../constants/qwencode-paths.js"; import { ValidationResult } from "../../types/ai-file.js"; import { McpServers } from "../../types/mcp.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import { applySharedConfigPatch, sharedConfigFileKey } from "../shared/shared-config-gateway.js"; import { RulesyncMcp } from "./rulesync-mcp.js"; import { @@ -116,10 +116,8 @@ export class QwencodeMcp extends ToolMcp { const paths = this.getSettablePaths({ global }); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - const existingContent = await readOrInitializeFileContent( - filePath, - JSON.stringify({ mcpServers: {} }, null, 2), - ); + const existingContent = + (await readFileContentOrNull(filePath)) ?? JSON.stringify({ mcpServers: {} }, null, 2); return new QwencodeMcp({ outputRoot, diff --git a/src/features/mcp/rovodev-mcp.ts b/src/features/mcp/rovodev-mcp.ts index 1aaa56151..30b14cbb4 100644 --- a/src/features/mcp/rovodev-mcp.ts +++ b/src/features/mcp/rovodev-mcp.ts @@ -4,7 +4,7 @@ import { ROVODEV_DIR, ROVODEV_MCP_FILE_NAME } from "../../constants/rovodev-path import { ValidationResult } from "../../types/ai-file.js"; import { isMcpServers } from "../../types/mcp.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import { isPlainObject } from "../../utils/type-guards.js"; import { RulesyncMcp } from "./rulesync-mcp.js"; import { @@ -110,10 +110,10 @@ export class RovodevMcp extends ToolMcp { } const paths = this.getSettablePaths({ global }); - const fileContent = await readOrInitializeFileContent( - join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), - JSON.stringify({ mcpServers: {} }, null, 2), - ); + const fileContent = + (await readFileContentOrNull( + join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), + )) ?? JSON.stringify({ mcpServers: {} }, null, 2); const json = parseRovodevMcpJson(fileContent, paths.relativeDirPath, paths.relativeFilePath); // Use getMcpServers() (not getJson()) so rulesync-only fields and diff --git a/src/features/mcp/warp-mcp.ts b/src/features/mcp/warp-mcp.ts index 52963a000..cef11183e 100644 --- a/src/features/mcp/warp-mcp.ts +++ b/src/features/mcp/warp-mcp.ts @@ -3,7 +3,7 @@ import { join } from "node:path"; import { WARP_DIR, WARP_MCP_FILE_NAME } from "../../constants/warp-paths.js"; import { ValidationResult } from "../../types/ai-file.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import { RulesyncMcp } from "./rulesync-mcp.js"; import { ToolMcp, @@ -90,10 +90,10 @@ export class WarpMcp extends ToolMcp { }: ToolMcpFromRulesyncMcpParams): Promise { const paths = this.getSettablePaths({ global }); - const fileContent = await readOrInitializeFileContent( - join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), - JSON.stringify({ mcpServers: {} }, null, 2), - ); + const fileContent = + (await readFileContentOrNull( + join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), + )) ?? JSON.stringify({ mcpServers: {} }, null, 2); const json = this.parseJsonOrThrow(fileContent, paths.relativeDirPath, paths.relativeFilePath); const warpConfig = { ...json, mcpServers: rulesyncMcp.getMcpServers() }; diff --git a/src/features/mcp/zed-mcp.ts b/src/features/mcp/zed-mcp.ts index de865a8fe..25c403f08 100644 --- a/src/features/mcp/zed-mcp.ts +++ b/src/features/mcp/zed-mcp.ts @@ -2,7 +2,7 @@ import { join } from "node:path"; import { ZED_DIR, ZED_GLOBAL_DIR, ZED_SETTINGS_FILE_NAME } from "../../constants/zed-paths.js"; import { ValidationResult } from "../../types/ai-file.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import { applySharedConfigPatch, sharedConfigFileKey } from "../shared/shared-config-gateway.js"; import { RulesyncMcp } from "./rulesync-mcp.js"; import { @@ -78,7 +78,7 @@ export class ZedMcp extends ToolMcp { const paths = this.getSettablePaths({ global }); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - const existingContent = await readOrInitializeFileContent(filePath, "{}"); + const existingContent = (await readFileContentOrNull(filePath)) ?? "{}"; return new ZedMcp({ outputRoot, diff --git a/src/features/permissions/antigravity-cli-permissions.ts b/src/features/permissions/antigravity-cli-permissions.ts index ee86f6431..edf98f63e 100644 --- a/src/features/permissions/antigravity-cli-permissions.ts +++ b/src/features/permissions/antigravity-cli-permissions.ts @@ -9,7 +9,7 @@ import { import type { AiFileParams, ValidationResult } from "../../types/ai-file.js"; import type { PermissionAction, PermissionsConfig } from "../../types/permissions.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import { RulesyncPermissions } from "./rulesync-permissions.js"; import { ToolPermissions, @@ -161,10 +161,7 @@ export class AntigravityCliPermissions extends ToolPermissions { }: ToolPermissionsFromRulesyncPermissionsParams): Promise { const paths = AntigravityCliPermissions.getSettablePaths(); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - const existingContent = await readOrInitializeFileContent( - filePath, - JSON.stringify({}, null, 2), - ); + const existingContent = (await readFileContentOrNull(filePath)) ?? JSON.stringify({}, null, 2); let settings: AntigravityCliSettingsJson; try { settings = JSON.parse(existingContent); diff --git a/src/features/permissions/antigravity-ide-permissions.ts b/src/features/permissions/antigravity-ide-permissions.ts index 41ab69a01..7f4a3f31d 100644 --- a/src/features/permissions/antigravity-ide-permissions.ts +++ b/src/features/permissions/antigravity-ide-permissions.ts @@ -9,7 +9,7 @@ import { import type { AiFileParams, ValidationResult } from "../../types/ai-file.js"; import type { PermissionAction, PermissionsConfig } from "../../types/permissions.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import { RulesyncPermissions } from "./rulesync-permissions.js"; import { ToolPermissions, @@ -152,10 +152,7 @@ export class AntigravityIdePermissions extends ToolPermissions { }: ToolPermissionsFromRulesyncPermissionsParams): Promise { const paths = AntigravityIdePermissions.getSettablePaths(); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - const existingContent = await readOrInitializeFileContent( - filePath, - JSON.stringify({}, null, 2), - ); + const existingContent = (await readFileContentOrNull(filePath)) ?? JSON.stringify({}, null, 2); let settings: AntigravityIdeSettingsJson; try { settings = JSON.parse(existingContent); diff --git a/src/features/permissions/claudecode-permissions.ts b/src/features/permissions/claudecode-permissions.ts index ff9c06648..5700afe75 100644 --- a/src/features/permissions/claudecode-permissions.ts +++ b/src/features/permissions/claudecode-permissions.ts @@ -5,7 +5,7 @@ import type { AiFileParams, ValidationResult } from "../../types/ai-file.js"; import type { ClaudeSettingsJson } from "../../types/claude-settings.js"; import type { PermissionAction, PermissionsConfig } from "../../types/permissions.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import { applyPermissions } from "../shared/shared-config-gateway.js"; import { RulesyncPermissions } from "./rulesync-permissions.js"; import { @@ -119,10 +119,7 @@ export class ClaudecodePermissions extends ToolPermissions { }: ToolPermissionsFromRulesyncPermissionsParams): Promise { const paths = ClaudecodePermissions.getSettablePaths(); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - const existingContent = await readOrInitializeFileContent( - filePath, - JSON.stringify({}, null, 2), - ); + const existingContent = (await readFileContentOrNull(filePath)) ?? JSON.stringify({}, null, 2); let settings: ClaudeSettingsJson; try { settings = JSON.parse(existingContent); diff --git a/src/features/permissions/cursor-permissions.ts b/src/features/permissions/cursor-permissions.ts index 27d972f6b..cd8e2d1eb 100644 --- a/src/features/permissions/cursor-permissions.ts +++ b/src/features/permissions/cursor-permissions.ts @@ -10,7 +10,7 @@ import { import type { AiFileParams, ValidationResult } from "../../types/ai-file.js"; import type { PermissionAction, PermissionsConfig } from "../../types/permissions.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import type { Logger } from "../../utils/logger.js"; import { RulesyncPermissions } from "./rulesync-permissions.js"; import { @@ -274,10 +274,7 @@ export class CursorPermissions extends ToolPermissions { }: ToolPermissionsFromRulesyncPermissionsParams): Promise { const paths = CursorPermissions.getSettablePaths({ global }); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - const existingContent = await readOrInitializeFileContent( - filePath, - JSON.stringify({}, null, 2), - ); + const existingContent = (await readFileContentOrNull(filePath)) ?? JSON.stringify({}, null, 2); let settings: CursorCliConfig; try { settings = asCursorCliConfig(JSON.parse(existingContent), logger, filePath); diff --git a/src/features/permissions/devin-permissions.ts b/src/features/permissions/devin-permissions.ts index 242ba5abb..c6d74a0b3 100644 --- a/src/features/permissions/devin-permissions.ts +++ b/src/features/permissions/devin-permissions.ts @@ -10,7 +10,7 @@ import { import type { AiFileParams, ValidationResult } from "../../types/ai-file.js"; import type { PermissionAction, PermissionsConfig } from "../../types/permissions.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import { isPrototypePollutionKey } from "../../utils/prototype-pollution.js"; import { isRecord } from "../../utils/type-guards.js"; import { applySharedConfigPatch, sharedConfigFileKey } from "../shared/shared-config-gateway.js"; @@ -172,10 +172,7 @@ export class DevinPermissions extends ToolPermissions { }: ToolPermissionsFromRulesyncPermissionsParams): Promise { const paths = DevinPermissions.getSettablePaths({ global }); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - const existingContent = await readOrInitializeFileContent( - filePath, - JSON.stringify({}, null, 2), - ); + const existingContent = (await readFileContentOrNull(filePath)) ?? JSON.stringify({}, null, 2); let settings: Record; try { diff --git a/src/features/permissions/factorydroid-permissions.ts b/src/features/permissions/factorydroid-permissions.ts index 2e3ab7b17..ad1788fd9 100644 --- a/src/features/permissions/factorydroid-permissions.ts +++ b/src/features/permissions/factorydroid-permissions.ts @@ -9,7 +9,7 @@ import { import type { AiFileParams, ValidationResult } from "../../types/ai-file.js"; import type { PermissionAction, PermissionsConfig } from "../../types/permissions.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import type { Logger } from "../../utils/logger.js"; import { RulesyncPermissions } from "./rulesync-permissions.js"; import { @@ -130,10 +130,7 @@ export class FactorydroidPermissions extends ToolPermissions { }: ToolPermissionsFromRulesyncPermissionsParams): Promise { const paths = FactorydroidPermissions.getSettablePaths({ global }); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - const existingContent = await readOrInitializeFileContent( - filePath, - JSON.stringify({}, null, 2), - ); + const existingContent = (await readFileContentOrNull(filePath)) ?? JSON.stringify({}, null, 2); let settings: FactorydroidSettingsJson; try { diff --git a/src/features/permissions/junie-permissions.ts b/src/features/permissions/junie-permissions.ts index 835c4c29f..6925dfbe5 100644 --- a/src/features/permissions/junie-permissions.ts +++ b/src/features/permissions/junie-permissions.ts @@ -8,7 +8,7 @@ import { type PermissionsConfig, } from "../../types/permissions.js"; import { formatError } from "../../utils/error.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; import type { Logger } from "../../utils/logger.js"; import { RulesyncPermissions } from "./rulesync-permissions.js"; import { @@ -158,7 +158,7 @@ export class JuniePermissions extends ToolPermissions { }: ToolPermissionsFromRulesyncPermissionsParams): Promise { const paths = JuniePermissions.getSettablePaths({ global }); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - const existingContent = await readOrInitializeFileContent(filePath, "{}"); + const existingContent = (await readFileContentOrNull(filePath)) ?? "{}"; let existing: JunieAllowlist; try { diff --git a/src/features/permissions/qwencode-permissions.ts b/src/features/permissions/qwencode-permissions.ts index 05ce1b13e..37d523a75 100644 --- a/src/features/permissions/qwencode-permissions.ts +++ b/src/features/permissions/qwencode-permissions.ts @@ -201,8 +201,9 @@ export class QwencodePermissions extends ToolPermissions { }: ToolPermissionsFromRulesyncPermissionsParams): Promise { const paths = QwencodePermissions.getSettablePaths({ global }); const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); - // Use null-fallback (instead of readOrInitializeFileContent) so generation has no filesystem - // side effects when the destination directory does not yet exist (important for dry-run). + // Read without initializing so generation has no filesystem side effects + // when the destination directory does not yet exist (important for dry-run); + // the actual write happens later in `writeAiFiles`. const existingContent = (await readFileContentOrNull(filePath)) ?? "{}"; let settings: QwenSettings; diff --git a/src/types/ai-file.ts b/src/types/ai-file.ts index 85dd3c517..425fab86c 100644 --- a/src/types/ai-file.ts +++ b/src/types/ai-file.ts @@ -130,5 +130,20 @@ export abstract class AiFile { return true; } + /** + * Returns whether rulesync should refrain from creating this file when it does + * not exist yet and the generated payload carries no content. + * + * Defaults to the shared/user-managed config files that rulesync merges into + * rather than owns (the same files that `isDeletable()` protects). Those paths + * are deliberately not gitignored, so materializing an empty `{}` there hands + * the user an untracked file to manage without giving them anything in return. + * Files rulesync fully owns are still written, because their mere existence is + * part of what rulesync generates. + */ + shouldSkipCreationWhenPayloadEmpty(): boolean { + return !this.isDeletable(); + } + abstract validate(): ValidationResult; } diff --git a/src/types/feature-processor.test.ts b/src/types/feature-processor.test.ts index 7b7fc811d..6976e017e 100644 --- a/src/types/feature-processor.test.ts +++ b/src/types/feature-processor.test.ts @@ -18,13 +18,22 @@ vi.mock("../utils/file.js", async () => { }; }); -function createMockFile(filePath: string): AiFile { +function createMockFile( + filePath: string, + { + fileContent = "content", + skipCreationWhenPayloadEmpty = false, + }: { fileContent?: string; skipCreationWhenPayloadEmpty?: boolean } = {}, +): AiFile { return { getFilePath: () => filePath, - getFileContent: () => "content", + getFileContent: () => fileContent, getRelativePathFromCwd: () => filePath, // Declared on the AiFile base class; defaults to false for non-merging files. shouldMergeExistingFileContent: () => false, + // Declared on the AiFile base class; defaults to `!isDeletable()`, i.e. true + // only for shared config files rulesync merges into but does not own. + shouldSkipCreationWhenPayloadEmpty: () => skipCreationWhenPayloadEmpty, } as AiFile; } @@ -204,6 +213,75 @@ describe("FeatureProcessor", () => { expect(result).toEqual({ count: 2, paths: ["/path/to/file1.md", "/path/to/file2.md"] }); expect(writeFileContent).not.toHaveBeenCalled(); }); + + it("should not create a missing merge-target file when the payload is empty", async () => { + vi.mocked(readFileContentOrNull).mockResolvedValue(null); + const processor = new TestProcessor({ logger: createMockLogger(), outputRoot: testDir }); + + const files = [ + createMockFile("/path/to/settings.json", { + fileContent: "{}", + skipCreationWhenPayloadEmpty: true, + }), + createMockFile("/path/to/config.json", { + fileContent: JSON.stringify({ mcpServers: {}, permissions: {} }), + skipCreationWhenPayloadEmpty: true, + }), + ]; + + const result = await processor.writeAiFiles(files); + + expect(result).toEqual({ count: 0, paths: [] }); + expect(writeFileContent).not.toHaveBeenCalled(); + }); + + it("should still create a missing merge-target file when the payload has content", async () => { + vi.mocked(readFileContentOrNull).mockResolvedValue(null); + const processor = new TestProcessor({ logger: createMockLogger(), outputRoot: testDir }); + + const files = [ + createMockFile("/path/to/settings.json", { + fileContent: JSON.stringify({ permissions: { allow: ["read"] } }), + skipCreationWhenPayloadEmpty: true, + }), + ]; + + const result = await processor.writeAiFiles(files); + + expect(result).toEqual({ count: 1, paths: ["/path/to/settings.json"] }); + expect(writeFileContent).toHaveBeenCalledTimes(1); + }); + + it("should still write an empty payload into a merge-target file that already exists", async () => { + vi.mocked(readFileContentOrNull).mockResolvedValue( + JSON.stringify({ permissions: { allow: ["read"] } }), + ); + const processor = new TestProcessor({ logger: createMockLogger(), outputRoot: testDir }); + + const files = [ + createMockFile("/path/to/settings.json", { + fileContent: "{}", + skipCreationWhenPayloadEmpty: true, + }), + ]; + + const result = await processor.writeAiFiles(files); + + expect(result).toEqual({ count: 1, paths: ["/path/to/settings.json"] }); + expect(writeFileContent).toHaveBeenCalledTimes(1); + }); + + it("should create a missing rulesync-owned file even when the payload is empty", async () => { + vi.mocked(readFileContentOrNull).mockResolvedValue(null); + const processor = new TestProcessor({ logger: createMockLogger(), outputRoot: testDir }); + + const files = [createMockFile("/path/to/hooks.json", { fileContent: "{}" })]; + + const result = await processor.writeAiFiles(files); + + expect(result).toEqual({ count: 1, paths: ["/path/to/hooks.json"] }); + expect(writeFileContent).toHaveBeenCalledTimes(1); + }); }); describe("removeAiFiles", () => { diff --git a/src/types/feature-processor.ts b/src/types/feature-processor.ts index 4dc459d29..7653079f7 100644 --- a/src/types/feature-processor.ts +++ b/src/types/feature-processor.ts @@ -1,4 +1,4 @@ -import { fileContentsEquivalent } from "../utils/content-equivalence.js"; +import { fileContentIsEmptyPayload, fileContentsEquivalent } from "../utils/content-equivalence.js"; import { addTrailingNewline, readFileContentOrNull, @@ -70,6 +70,18 @@ export abstract class FeatureProcessor { const contentWithNewline = addTrailingNewline(aiFile.getFileContent()); const existingContent = existingFileContent; + // Never bring a shared, user-managed config file into existence just to + // hold an empty payload — that is pure `git status` noise for paths + // rulesync merges into but does not own. An existing file is still + // rewritten, so user-authored content is never dropped. + if ( + existingContent === null && + aiFile.shouldSkipCreationWhenPayloadEmpty() && + fileContentIsEmptyPayload({ filePath, content: contentWithNewline }) + ) { + continue; + } + if ( fileContentsEquivalent({ filePath, diff --git a/src/utils/content-equivalence.test.ts b/src/utils/content-equivalence.test.ts index dd4cc3ed5..cad846994 100644 --- a/src/utils/content-equivalence.test.ts +++ b/src/utils/content-equivalence.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; -import { fileContentsEquivalent } from "./content-equivalence.js"; +import { fileContentIsEmptyPayload, fileContentsEquivalent } from "./content-equivalence.js"; import { addTrailingNewline } from "./file.js"; import { stringifyFrontmatter } from "./frontmatter.js"; @@ -173,3 +173,60 @@ Body ).toBe(true); }); }); + +describe("fileContentIsEmptyPayload", () => { + it("treats whitespace-only content as empty for any extension", () => { + expect(fileContentIsEmptyPayload({ filePath: "/x/settings.json", content: "" })).toBe(true); + expect(fileContentIsEmptyPayload({ filePath: "/x/config.toml", content: "\n \n" })).toBe(true); + expect(fileContentIsEmptyPayload({ filePath: "/x/notes.txt", content: " " })).toBe(true); + }); + + it("treats structurally empty JSON documents as empty", () => { + expect(fileContentIsEmptyPayload({ filePath: "/x/settings.json", content: "{}" })).toBe(true); + expect( + fileContentIsEmptyPayload({ filePath: "/x/settings.json", content: '{"permissions":{}}' }), + ).toBe(true); + expect( + fileContentIsEmptyPayload({ + filePath: "/x/config.json", + content: '{"mcpServers":{},"permissions":{"allow":[]}}', + }), + ).toBe(true); + }); + + it("treats any scalar value as content", () => { + expect( + fileContentIsEmptyPayload({ filePath: "/x/settings.json", content: '{"enabled":false}' }), + ).toBe(false); + expect( + fileContentIsEmptyPayload({ + filePath: "/x/settings.json", + content: '{"permissions":{"allow":["read"]}}', + }), + ).toBe(false); + }); + + it("handles YAML and TOML documents", () => { + expect( + fileContentIsEmptyPayload({ filePath: "/x/config.yaml", content: "extensions: {}" }), + ).toBe(true); + expect( + fileContentIsEmptyPayload({ filePath: "/x/config.yaml", content: "extensions:\n a: 1\n" }), + ).toBe(false); + expect(fileContentIsEmptyPayload({ filePath: "/x/config.toml", content: "# comment\n" })).toBe( + true, + ); + expect( + fileContentIsEmptyPayload({ filePath: "/x/config.toml", content: 'model = "x"\n' }), + ).toBe(false); + }); + + it("treats unparseable or unstructured content as non-empty", () => { + expect(fileContentIsEmptyPayload({ filePath: "/x/settings.json", content: "{not json" })).toBe( + false, + ); + expect(fileContentIsEmptyPayload({ filePath: "/x/AGENTS.md", content: "# Title\n" })).toBe( + false, + ); + }); +}); diff --git a/src/utils/content-equivalence.ts b/src/utils/content-equivalence.ts index bdca82451..f2d62b8f7 100644 --- a/src/utils/content-equivalence.ts +++ b/src/utils/content-equivalence.ts @@ -94,6 +94,85 @@ function tryFileContentsEquivalent( } } +/** + * Structured parse for known extensions. Returns `undefined` when the content + * cannot be parsed as that format (or the extension is not a structured one). + */ +function tryParseStructured(filePath: string, content: string): unknown | undefined { + const ext = extname(filePath).toLowerCase(); + + switch (ext) { + case ".json": + case ".jsonc": { + const errors: ParseError[] = []; + const parsed = parseJsonc(content, errors); + return errors.length > 0 ? undefined : parsed; + } + case ".yaml": + case ".yml": + try { + return loadYaml(content); + } catch { + return undefined; + } + case ".toml": + try { + return smolToml.parse(content); + } catch { + return undefined; + } + default: + return undefined; + } +} + +/** + * Whether a parsed structured value carries no information: `null`/`undefined`, + * an empty array, or an object whose every value is itself empty by this same + * rule. Any scalar (string, number, boolean) counts as content. + */ +function isEmptyStructuredValue(value: unknown): boolean { + if (value === null || value === undefined) { + return true; + } + if (Array.isArray(value)) { + return value.length === 0; + } + if (typeof value === "object") { + return Object.values(value).every(isEmptyStructuredValue); + } + return false; +} + +/** + * Whether generated content carries nothing worth writing — an empty structured + * document such as `{}`, `{"permissions":{}}` or `{"mcpServers":{}}`, or (for + * unstructured formats) whitespace-only text. + * + * Used to avoid creating shared, user-managed config files that rulesync merges + * into but does not own: writing an empty file there hands the user a file to + * manage without giving them anything in return. + */ +export function fileContentIsEmptyPayload({ + filePath, + content, +}: { + filePath: string; + content: string; +}): boolean { + if (content.trim() === "") { + return true; + } + + const parsed = tryParseStructured(filePath, content); + + if (parsed === undefined) { + return false; + } + + return isEmptyStructuredValue(parsed); +} + /** * Whether on-disk content is equivalent to generated content for --check / dry-run. * diff --git a/src/utils/file.test.ts b/src/utils/file.test.ts index a21fed382..29691ebab 100644 --- a/src/utils/file.test.ts +++ b/src/utils/file.test.ts @@ -25,7 +25,6 @@ import { listDirectoryFiles, readFileContent, readJsonFile, - readOrInitializeFileContent, removeDirectory, removeFile, removeTempDirectory, @@ -383,84 +382,6 @@ describe("file utilities", () => { }); }); - describe("readOrInitializeFileContent", () => { - it("should return existing file content if file exists", async () => { - const filePath = join(testDir, "existing.txt"); - const existingContent = "existing content"; - await writeFileContent(filePath, existingContent); - - const content = await readOrInitializeFileContent(filePath, "initial content"); - - expect(content).toBe(existingContent); - }); - - it("should return initial content if file does not exist", async () => { - const filePath = join(testDir, "nonexistent.txt"); - const initialContent = "initial content"; - - const content = await readOrInitializeFileContent(filePath, initialContent); - - expect(content).toBe(initialContent); - }); - - it("should create file with initial content when file does not exist", async () => { - const filePath = join(testDir, "nonexistent.txt"); - const initialContent = "initial content"; - - const content = await readOrInitializeFileContent(filePath, initialContent); - - expect(await fileExists(filePath)).toBe(true); - expect(content).toBe(initialContent); - expect(await readFileContent(filePath)).toBe(initialContent); - }); - - it("should create parent directories when file does not exist", async () => { - const filePath = join(testDir, "nested", "deep", "file.txt"); - const initialContent = "initial content"; - - await readOrInitializeFileContent(filePath, initialContent); - - expect(await directoryExists(join(testDir, "nested", "deep"))).toBe(true); - }); - - it("should handle empty existing file", async () => { - const filePath = join(testDir, "empty.txt"); - await writeFileContent(filePath, ""); - - const content = await readOrInitializeFileContent(filePath, "initial content"); - - expect(content).toBe(""); - }); - - it("should handle empty initial content", async () => { - const filePath = join(testDir, "new-empty.txt"); - - const content = await readOrInitializeFileContent(filePath, ""); - - expect(content).toBe(""); - }); - - it("should preserve multiline content from existing file", async () => { - const filePath = join(testDir, "multiline.txt"); - const multilineContent = "line1\nline2\nline3\n"; - await writeFileContent(filePath, multilineContent); - - const content = await readOrInitializeFileContent(filePath, "initial"); - - expect(content).toBe(multilineContent); - }); - - it("should handle special characters in file content", async () => { - const filePath = join(testDir, "special.txt"); - const specialContent = 'content with "quotes" and \n special chars: <>?*|'; - await writeFileContent(filePath, specialContent); - - const content = await readOrInitializeFileContent(filePath, "initial"); - - expect(content).toBe(specialContent); - }); - }); - describe("fileExists", () => { it("should return true for existing file", async () => { const filePath = join(testDir, "exists.txt"); diff --git a/src/utils/file.ts b/src/utils/file.ts index 09665b9b6..6afd1df9e 100644 --- a/src/utils/file.ts +++ b/src/utils/file.ts @@ -143,19 +143,6 @@ export async function ensureDir(dirPath: string): Promise { } } -export async function readOrInitializeFileContent( - filePath: string, - initialContent: string = "", -): Promise { - if (await fileExists(filePath)) { - return await readFileContent(filePath); - } else { - await ensureDir(dirname(filePath)); - await writeFileContent(filePath, initialContent); - return initialContent; - } -} - /** * Converts OS-native path separators to POSIX forward slashes. * Use this instead of `path.posix.join` when input segments may already From 7739d79ed1d03c94a638a0be4d83b3fcc26987e4 Mon Sep 17 00:00:00 2001 From: dyoshikawa Date: Sun, 26 Jul 2026 23:36:35 -0700 Subject: [PATCH 2/3] fix(generate): narrow the empty-payload skip to shared user-managed config files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on the initial commit. - The skip keyed off `!isDeletable()`, but that flag carries three unrelated meanings (shared config file, global-scope file that is unsafe to delete, file the orphan sweep cannot rediscover), so it also stopped creating files such as `~/.cursor/mcp.json` — outside the scope of #2430, and a silent foot-gun for any future rulesync-owned file marked non-deletable. The predicate is now path-driven: `SHARED_USER_MANAGED_CONFIG_PATHS` in `src/constants/shared-config-paths.ts` is the single source of truth, and `DERIVED_PATHS_NOT_GITIGNORED` is derived from it so the two can never drift. - Add the regression test #2430 asked for: an e2e case per shared config file asserting that an empty permissions payload leaves the path absent. - `feature-processor.test.ts` now delegates to the real `AiFile.prototype.shouldSkipCreationWhenPayloadEmpty` instead of stubbing it, so the default implementation is actually covered. - `isEmptyStructuredValue` no longer reports a value as empty when it cannot see the whole thing: an object with a non-plain prototype counts as content (covers Date/TomlDate and a `__proto__` entry, which jsonc-parser resolves by replacing the prototype), and a cycle guard keeps a self-referential YAML anchor from recursing forever. - Unify the extension dispatch: `tryParseStructured` returns a `{ ok, value }` result so a comment-only YAML document (a successful parse yielding `undefined`) is no longer confused with a parse failure, and `fileContentsEquivalent` reuses it instead of duplicating the switch. - Document the new contract under `generate` in docs/reference/cli-commands.md. Co-Authored-By: Claude Opus 5 (1M context) --- docs/reference/cli-commands.md | 6 + skills/rulesync/cli-commands.md | 6 + src/cli/commands/gitignore-derive.ts | 29 ++--- src/constants/shared-config-paths.test.ts | 45 +++++++ src/constants/shared-config-paths.ts | 52 ++++++++ src/e2e/e2e-permissions.spec.ts | 26 +++- src/types/ai-file.ts | 22 ++-- src/types/feature-processor.test.ts | 53 ++++---- src/utils/content-equivalence.test.ts | 48 +++++++ src/utils/content-equivalence.ts | 145 ++++++++++------------ 10 files changed, 297 insertions(+), 135 deletions(-) create mode 100644 src/constants/shared-config-paths.test.ts create mode 100644 src/constants/shared-config-paths.ts diff --git a/docs/reference/cli-commands.md b/docs/reference/cli-commands.md index e935eb790..e9ad72e1e 100644 --- a/docs/reference/cli-commands.md +++ b/docs/reference/cli-commands.md @@ -123,6 +123,12 @@ rulesync generate --dry-run --targets claudecode --features rules rulesync generate --check --targets "*" --features "*" ``` +### Shared config files are never created empty + +Some outputs are files Rulesync merges into rather than owns, because the tool (or you) keeps unrelated settings there — `.claude/settings.json`, `.vscode/settings.json`, `.antigravity/settings.json`, `.factory/settings.json`, `.devin/config.json`, `.codex/config.toml`, `.zed/settings.json`, `opencode.json`, `kilo.json(c)`, `reasonix.toml`, `.grok/config.toml`, `.vibe/config.toml`, `.amp/settings.json(c)`. These are deliberately **not** added to `.gitignore` by `rulesync gitignore`, so that settings you hand-author in them stay version-controlled. + +Because they stay committable, `generate` will not **create** one of them just to hold an empty payload: if Rulesync has nothing to contribute (e.g. no permissions map to that tool), the file is left absent instead of being written as `{}`. A file that already exists is always rewritten as usual, so nothing you authored is dropped. Every other generated file is written even when empty, since for a file Rulesync owns its existence is part of the output. + ## Gitignore Command The `gitignore` command adds generated AI tool configuration files to `.gitignore`. By default, it emits entries only for the tools listed in the `targets` of your `rulesync.jsonc` (controlled by the `gitignoreTargetsOnly` option, which defaults to `true`). Set `gitignoreTargetsOnly` to `false` to emit entries for all supported tools instead. You can also filter the output per-invocation with `--targets` / `--features`, which take precedence over the config. diff --git a/skills/rulesync/cli-commands.md b/skills/rulesync/cli-commands.md index e935eb790..e9ad72e1e 100644 --- a/skills/rulesync/cli-commands.md +++ b/skills/rulesync/cli-commands.md @@ -123,6 +123,12 @@ rulesync generate --dry-run --targets claudecode --features rules rulesync generate --check --targets "*" --features "*" ``` +### Shared config files are never created empty + +Some outputs are files Rulesync merges into rather than owns, because the tool (or you) keeps unrelated settings there — `.claude/settings.json`, `.vscode/settings.json`, `.antigravity/settings.json`, `.factory/settings.json`, `.devin/config.json`, `.codex/config.toml`, `.zed/settings.json`, `opencode.json`, `kilo.json(c)`, `reasonix.toml`, `.grok/config.toml`, `.vibe/config.toml`, `.amp/settings.json(c)`. These are deliberately **not** added to `.gitignore` by `rulesync gitignore`, so that settings you hand-author in them stay version-controlled. + +Because they stay committable, `generate` will not **create** one of them just to hold an empty payload: if Rulesync has nothing to contribute (e.g. no permissions map to that tool), the file is left absent instead of being written as `{}`. A file that already exists is always rewritten as usual, so nothing you authored is dropped. Every other generated file is written even when empty, since for a file Rulesync owns its existence is part of the output. + ## Gitignore Command The `gitignore` command adds generated AI tool configuration files to `.gitignore`. By default, it emits entries only for the tools listed in the `targets` of your `rulesync.jsonc` (controlled by the `gitignoreTargetsOnly` option, which defaults to `true`). Set `gitignoreTargetsOnly` to `false` to emit entries for all supported tools instead. You can also filter the output per-invocation with `--targets` / `--features`, which take precedence over the config. diff --git a/src/cli/commands/gitignore-derive.ts b/src/cli/commands/gitignore-derive.ts index 6df8e225f..c6ca5c759 100644 --- a/src/cli/commands/gitignore-derive.ts +++ b/src/cli/commands/gitignore-derive.ts @@ -1,3 +1,4 @@ +import { SHARED_USER_MANAGED_CONFIG_PATHS } from "../../constants/shared-config-paths.js"; import type { ToolRuleExtraFixedFile } from "../../features/rules/tool-rule.js"; import type { Feature } from "../../types/features.js"; import { getProcessorRegistryEntry } from "../../types/processor-registry.js"; @@ -22,28 +23,12 @@ const TARGETS_NOT_DERIVED: ReadonlySet = new Set([ // Project-scope outputs that rulesync merges into rather than fully owns // (user-managed settings files), so they are deliberately not gitignored even -// though a feature emits them. Most paths come straight from a tool's default -// getSettablePaths; `.amp/settings.jsonc` (runtime probe twin of -// `.amp/settings.json`) and `.claude/settings.local.json` (claudecode ignore -// `fileMode: "local"` variant) are emitted only under non-default options. -export const DERIVED_PATHS_NOT_GITIGNORED: ReadonlySet = new Set([ - "**/.amp/settings.json", - "**/.amp/settings.jsonc", - "**/.antigravity/settings.json", - "**/.claude/settings.json", - "**/.claude/settings.local.json", - "**/.codex/config.toml", - "**/.devin/config.json", - "**/.factory/settings.json", - "**/.grok/config.toml", - "**/.vibe/config.toml", - "**/reasonix.toml", - "**/.vscode/settings.json", - "**/.zed/settings.json", - "**/kilo.json", - "**/kilo.jsonc", - "**/opencode.json", -]); +// though a feature emits them. The list itself lives in +// `src/constants/shared-config-paths.ts` because the same set also decides +// which files must not be created just to hold an empty payload. +export const DERIVED_PATHS_NOT_GITIGNORED: ReadonlySet = new Set( + SHARED_USER_MANAGED_CONFIG_PATHS.map((path) => `**/${path}`), +); const toPosix = (path: string): string => path.replace(/\\/g, "/"); diff --git a/src/constants/shared-config-paths.test.ts b/src/constants/shared-config-paths.test.ts new file mode 100644 index 000000000..7680c9413 --- /dev/null +++ b/src/constants/shared-config-paths.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from "vitest"; + +import { + isSharedUserManagedConfigPath, + SHARED_USER_MANAGED_CONFIG_PATHS, +} from "./shared-config-paths.js"; + +describe("SHARED_USER_MANAGED_CONFIG_PATHS", () => { + it("stores paths without the glob prefix so the gitignore derivation can add it", () => { + for (const path of SHARED_USER_MANAGED_CONFIG_PATHS) { + expect(path.startsWith("*")).toBe(false); + expect(path.startsWith("/")).toBe(false); + } + }); +}); + +describe("isSharedUserManagedConfigPath", () => { + it("matches a listed path at the project root", () => { + expect(isSharedUserManagedConfigPath(".antigravity/settings.json")).toBe(true); + expect(isSharedUserManagedConfigPath(".factory/settings.json")).toBe(true); + expect(isSharedUserManagedConfigPath("opencode.json")).toBe(true); + }); + + it("matches at any depth, mirroring the `**` gitignore entries", () => { + expect(isSharedUserManagedConfigPath("packages/app/.vscode/settings.json")).toBe(true); + expect(isSharedUserManagedConfigPath("packages/app/reasonix.toml")).toBe(true); + }); + + it("normalizes a leading ./ and native separators", () => { + expect(isSharedUserManagedConfigPath("./.claude/settings.json")).toBe(true); + expect(isSharedUserManagedConfigPath(".claude\\settings.json")).toBe(true); + }); + + it("does not match files rulesync owns outright", () => { + expect(isSharedUserManagedConfigPath(".agents/hooks.json")).toBe(false); + expect(isSharedUserManagedConfigPath(".cursor/mcp.json")).toBe(false); + expect(isSharedUserManagedConfigPath(".junie/allowlist.json")).toBe(false); + expect(isSharedUserManagedConfigPath("AGENTS.md")).toBe(false); + }); + + it("does not match on a partial final segment", () => { + expect(isSharedUserManagedConfigPath("my-opencode.json")).toBe(false); + expect(isSharedUserManagedConfigPath("not-reasonix.toml")).toBe(false); + }); +}); diff --git a/src/constants/shared-config-paths.ts b/src/constants/shared-config-paths.ts new file mode 100644 index 000000000..3f08f439f --- /dev/null +++ b/src/constants/shared-config-paths.ts @@ -0,0 +1,52 @@ +import { toPosixPath } from "../utils/file.js"; + +/** + * Project-scope outputs that rulesync merges into rather than fully owns + * (user-managed settings files). Most paths come straight from a tool's default + * `getSettablePaths`; `.amp/settings.jsonc` (runtime probe twin of + * `.amp/settings.json`) and `.claude/settings.local.json` (claudecode ignore + * `fileMode: "local"` variant) are emitted only under non-default options. + * + * Two behaviors are derived from this single list: + * + * - They are deliberately **not** gitignored (`DERIVED_PATHS_NOT_GITIGNORED` in + * `src/cli/commands/gitignore-derive.ts`), because a user may hand-author + * settings in them that should stay version-controlled. + * - Because they are committable, rulesync must not **create** one just to hold + * an empty payload — that would be pure `git status` noise. See + * `AiFile#shouldSkipCreationWhenPayloadEmpty()`. + * + * Paths are stored without the leading "**" glob prefix; the gitignore + * derivation adds it. + */ +export const SHARED_USER_MANAGED_CONFIG_PATHS: readonly string[] = [ + ".amp/settings.json", + ".amp/settings.jsonc", + ".antigravity/settings.json", + ".claude/settings.json", + ".claude/settings.local.json", + ".codex/config.toml", + ".devin/config.json", + ".factory/settings.json", + ".grok/config.toml", + ".vibe/config.toml", + "reasonix.toml", + ".vscode/settings.json", + ".zed/settings.json", + "kilo.json", + "kilo.jsonc", + "opencode.json", +]; + +/** + * Whether a relative output path (POSIX or native separators) is one of the + * shared, user-managed config files above. Matches the same any-depth semantics + * as the derived gitignore entries: the path matches when it is exactly a listed + * path, or ends with a slash followed by one. + */ +export function isSharedUserManagedConfigPath(relativePath: string): boolean { + const normalized = toPosixPath(relativePath).replace(/^\.\//, ""); + return SHARED_USER_MANAGED_CONFIG_PATHS.some( + (path) => normalized === path || normalized.endsWith(`/${path}`), + ); +} diff --git a/src/e2e/e2e-permissions.spec.ts b/src/e2e/e2e-permissions.spec.ts index 55d5403f9..f3f612ebd 100644 --- a/src/e2e/e2e-permissions.spec.ts +++ b/src/e2e/e2e-permissions.spec.ts @@ -11,7 +11,7 @@ import { RULESYNC_PERMISSIONS_SCHEMA_URL, } from "../constants/rulesync-paths.js"; import { PermissionsProcessor } from "../features/permissions/permissions-processor.js"; -import { readFileContent, writeFileContent } from "../utils/file.js"; +import { fileExists, readFileContent, writeFileContent } from "../utils/file.js"; import { assertGenerateMatrixCoversTargets, runGenerate, @@ -88,6 +88,30 @@ describe("E2E: permissions", () => { }); }); + it.each([ + { target: "antigravity-ide", relativePath: [".antigravity", "settings.json"] }, + { target: "factorydroid", relativePath: [".factory", "settings.json"] }, + { target: "copilot", relativePath: [".vscode", "settings.json"] }, + ])( + "should not create the shared $target config file when the permissions payload is empty", + async ({ target, relativePath }) => { + const testDir = getTestDir(); + + // A permissions file whose categories map to nothing this tool models, so + // the merge payload for the shared config file comes out empty. These + // paths are deliberately not gitignored, so creating them would leave + // untracked files with no content behind after every generate. + await writeFileContent( + join(testDir, RULESYNC_PERMISSIONS_RELATIVE_FILE_PATH), + JSON.stringify({ permission: {} }, null, 2), + ); + + await runGenerate({ target, features: "permissions" }); + + expect(await fileExists(join(testDir, ...relativePath))).toBe(false); + }, + ); + it("should generate claudecode permissions into .claude/settings.json", async () => { const testDir = getTestDir(); diff --git a/src/types/ai-file.ts b/src/types/ai-file.ts index 425fab86c..340562aa2 100644 --- a/src/types/ai-file.ts +++ b/src/types/ai-file.ts @@ -1,5 +1,6 @@ import path, { relative, resolve } from "node:path"; +import { isSharedUserManagedConfigPath } from "../constants/shared-config-paths.js"; import { toPosixPath } from "../utils/file.js"; export type ValidationResult = @@ -134,15 +135,22 @@ export abstract class AiFile { * Returns whether rulesync should refrain from creating this file when it does * not exist yet and the generated payload carries no content. * - * Defaults to the shared/user-managed config files that rulesync merges into - * rather than owns (the same files that `isDeletable()` protects). Those paths - * are deliberately not gitignored, so materializing an empty `{}` there hands - * the user an untracked file to manage without giving them anything in return. - * Files rulesync fully owns are still written, because their mere existence is - * part of what rulesync generates. + * Defaults to the shared, user-managed config files listed in + * `SHARED_USER_MANAGED_CONFIG_PATHS` — the same set that is deliberately kept + * out of `rulesync gitignore`. Because those paths stay committable, + * materializing an empty `{}` there hands the user an untracked file to manage + * without giving them anything in return. + * + * Every other file is still written, because for a file rulesync owns its mere + * existence is part of what rulesync generates. This deliberately does NOT key + * off `isDeletable()`: that flag also protects global-scope files and files the + * orphan sweep cannot rediscover, which are unrelated concerns. + * + * Override and return `false` for any file whose emptiness is itself + * meaningful (e.g. an ignore format where an empty file blocks everything). */ shouldSkipCreationWhenPayloadEmpty(): boolean { - return !this.isDeletable(); + return isSharedUserManagedConfigPath(this.getRelativePathFromCwd()); } abstract validate(): ValidationResult; diff --git a/src/types/feature-processor.test.ts b/src/types/feature-processor.test.ts index 6976e017e..22e4a058f 100644 --- a/src/types/feature-processor.test.ts +++ b/src/types/feature-processor.test.ts @@ -18,23 +18,26 @@ vi.mock("../utils/file.js", async () => { }; }); +/** + * Minimal `AiFile` stand-in. `shouldSkipCreationWhenPayloadEmpty` deliberately + * delegates to the real `AiFile.prototype` implementation (which keys off the + * relative path), so these tests exercise the production predicate rather than a + * hand-written stub. + */ function createMockFile( filePath: string, - { - fileContent = "content", - skipCreationWhenPayloadEmpty = false, - }: { fileContent?: string; skipCreationWhenPayloadEmpty?: boolean } = {}, + { fileContent = "content" }: { fileContent?: string } = {}, ): AiFile { - return { + const file = { getFilePath: () => filePath, getFileContent: () => fileContent, getRelativePathFromCwd: () => filePath, // Declared on the AiFile base class; defaults to false for non-merging files. shouldMergeExistingFileContent: () => false, - // Declared on the AiFile base class; defaults to `!isDeletable()`, i.e. true - // only for shared config files rulesync merges into but does not own. - shouldSkipCreationWhenPayloadEmpty: () => skipCreationWhenPayloadEmpty, + shouldSkipCreationWhenPayloadEmpty: () => + AiFile.prototype.shouldSkipCreationWhenPayloadEmpty.call(file), } as AiFile; + return file; } class TestProcessor extends FeatureProcessor { @@ -214,18 +217,14 @@ describe("FeatureProcessor", () => { expect(writeFileContent).not.toHaveBeenCalled(); }); - it("should not create a missing merge-target file when the payload is empty", async () => { + it("should not create a missing shared config file when the payload is empty", async () => { vi.mocked(readFileContentOrNull).mockResolvedValue(null); const processor = new TestProcessor({ logger: createMockLogger(), outputRoot: testDir }); const files = [ - createMockFile("/path/to/settings.json", { - fileContent: "{}", - skipCreationWhenPayloadEmpty: true, - }), - createMockFile("/path/to/config.json", { + createMockFile(".antigravity/settings.json", { fileContent: "{}" }), + createMockFile(".devin/config.json", { fileContent: JSON.stringify({ mcpServers: {}, permissions: {} }), - skipCreationWhenPayloadEmpty: true, }), ]; @@ -235,39 +234,33 @@ describe("FeatureProcessor", () => { expect(writeFileContent).not.toHaveBeenCalled(); }); - it("should still create a missing merge-target file when the payload has content", async () => { + it("should still create a missing shared config file when the payload has content", async () => { vi.mocked(readFileContentOrNull).mockResolvedValue(null); const processor = new TestProcessor({ logger: createMockLogger(), outputRoot: testDir }); const files = [ - createMockFile("/path/to/settings.json", { + createMockFile(".antigravity/settings.json", { fileContent: JSON.stringify({ permissions: { allow: ["read"] } }), - skipCreationWhenPayloadEmpty: true, }), ]; const result = await processor.writeAiFiles(files); - expect(result).toEqual({ count: 1, paths: ["/path/to/settings.json"] }); + expect(result).toEqual({ count: 1, paths: [".antigravity/settings.json"] }); expect(writeFileContent).toHaveBeenCalledTimes(1); }); - it("should still write an empty payload into a merge-target file that already exists", async () => { + it("should still write an empty payload into a shared config file that already exists", async () => { vi.mocked(readFileContentOrNull).mockResolvedValue( JSON.stringify({ permissions: { allow: ["read"] } }), ); const processor = new TestProcessor({ logger: createMockLogger(), outputRoot: testDir }); - const files = [ - createMockFile("/path/to/settings.json", { - fileContent: "{}", - skipCreationWhenPayloadEmpty: true, - }), - ]; + const files = [createMockFile(".antigravity/settings.json", { fileContent: "{}" })]; const result = await processor.writeAiFiles(files); - expect(result).toEqual({ count: 1, paths: ["/path/to/settings.json"] }); + expect(result).toEqual({ count: 1, paths: [".antigravity/settings.json"] }); expect(writeFileContent).toHaveBeenCalledTimes(1); }); @@ -275,11 +268,13 @@ describe("FeatureProcessor", () => { vi.mocked(readFileContentOrNull).mockResolvedValue(null); const processor = new TestProcessor({ logger: createMockLogger(), outputRoot: testDir }); - const files = [createMockFile("/path/to/hooks.json", { fileContent: "{}" })]; + // `.agents/hooks.json` is owned wholesale by rulesync, so its existence is + // part of what generation produces even when it holds no hooks. + const files = [createMockFile(".agents/hooks.json", { fileContent: "{}" })]; const result = await processor.writeAiFiles(files); - expect(result).toEqual({ count: 1, paths: ["/path/to/hooks.json"] }); + expect(result).toEqual({ count: 1, paths: [".agents/hooks.json"] }); expect(writeFileContent).toHaveBeenCalledTimes(1); }); }); diff --git a/src/utils/content-equivalence.test.ts b/src/utils/content-equivalence.test.ts index cad846994..fa900118a 100644 --- a/src/utils/content-equivalence.test.ts +++ b/src/utils/content-equivalence.test.ts @@ -229,4 +229,52 @@ describe("fileContentIsEmptyPayload", () => { false, ); }); + + it("treats a comment-only YAML document as empty", () => { + // `loadYaml` legitimately returns `undefined` here, which must not be + // confused with a parse failure. + expect(fileContentIsEmptyPayload({ filePath: "/x/config.yaml", content: "# comment\n" })).toBe( + true, + ); + }); + + it("treats a null document and empty containers as empty", () => { + expect(fileContentIsEmptyPayload({ filePath: "/x/settings.json", content: "null" })).toBe(true); + expect(fileContentIsEmptyPayload({ filePath: "/x/settings.json", content: "[]" })).toBe(true); + expect( + fileContentIsEmptyPayload({ filePath: "/x/settings.jsonc", content: "// note\n{}\n" }), + ).toBe(true); + }); + + it("treats a non-empty array as content regardless of its elements", () => { + expect(fileContentIsEmptyPayload({ filePath: "/x/settings.json", content: '{"a":[{}]}' })).toBe( + false, + ); + expect(fileContentIsEmptyPayload({ filePath: "/x/settings.json", content: "[{}]" })).toBe( + false, + ); + }); + + it("treats a __proto__ entry as content instead of losing its payload", () => { + // jsonc-parser resolves `__proto__` by replacing the prototype, so the + // nested servers would be invisible to a plain own-property walk. + expect( + fileContentIsEmptyPayload({ + filePath: "/x/settings.json", + content: '{"mcpServers":{"__proto__":{"evil":1}}}', + }), + ).toBe(false); + }); + + it("does not recurse forever on a self-referential YAML anchor", () => { + expect( + fileContentIsEmptyPayload({ filePath: "/x/config.yaml", content: "&r\nfoo: *r\n" }), + ).toBe(false); + }); + + it("treats a date-only TOML document as content", () => { + expect( + fileContentIsEmptyPayload({ filePath: "/x/config.toml", content: "updated = 2026-01-01\n" }), + ).toBe(false); + }); }); diff --git a/src/utils/content-equivalence.ts b/src/utils/content-equivalence.ts index f2d62b8f7..6d5c52218 100644 --- a/src/utils/content-equivalence.ts +++ b/src/utils/content-equivalence.ts @@ -9,34 +9,44 @@ import { parseFrontmatter } from "./frontmatter.js"; import { loadYaml } from "./yaml.js"; /** - * Structural equality for JSON and JSONC using jsonc-parser (valid JSON parses the same as JSONC). + * Result of a structured parse. `ok: false` means "this content is not parseable + * as its extension's format" — distinct from a successful parse that happens to + * yield `undefined` (e.g. `loadYaml` on a comment-only document). */ -function tryJsonEquivalent(a: string, b: string): boolean | undefined { - const errorsA: ParseError[] = []; - const errorsB: ParseError[] = []; - const parsedA = parseJsonc(a, errorsA); - const parsedB = parseJsonc(b, errorsB); +type ParseResult = { readonly ok: true; readonly value: unknown } | { readonly ok: false }; - if (errorsA.length > 0 || errorsB.length > 0) { - return undefined; - } +const PARSE_FAILED: ParseResult = { ok: false }; - return isDeepStrictEqual(parsedA, parsedB); -} - -function tryYamlEquivalent(a: string, b: string): boolean | undefined { - try { - return isDeepStrictEqual(loadYaml(a), loadYaml(b)); - } catch { - return undefined; - } -} +/** + * Structured parse for known extensions. JSON and JSONC both go through + * jsonc-parser (valid JSON parses the same as JSONC). Returns `ok: false` for + * unknown extensions and for content that does not parse. + */ +function tryParseStructured(filePath: string, content: string): ParseResult { + const ext = extname(filePath).toLowerCase(); -function tryTomlEquivalent(a: string, b: string): boolean | undefined { - try { - return isDeepStrictEqual(smolToml.parse(a), smolToml.parse(b)); - } catch { - return undefined; + switch (ext) { + case ".json": + case ".jsonc": { + const errors: ParseError[] = []; + const value = parseJsonc(content, errors); + return errors.length > 0 ? PARSE_FAILED : { ok: true, value }; + } + case ".yaml": + case ".yml": + try { + return { ok: true, value: loadYaml(content) }; + } catch { + return PARSE_FAILED; + } + case ".toml": + try { + return { ok: true, value: smolToml.parse(content) }; + } catch { + return PARSE_FAILED; + } + default: + return PARSE_FAILED; } } @@ -77,61 +87,38 @@ function tryFileContentsEquivalent( ): boolean | undefined { const ext = extname(filePath).toLowerCase(); - switch (ext) { - case ".json": - case ".jsonc": - return tryJsonEquivalent(expected, existing); - case ".yaml": - case ".yml": - return tryYamlEquivalent(expected, existing); - case ".toml": - return tryTomlEquivalent(expected, existing); - case ".md": - case ".mdc": - return tryMarkdownEquivalent(expected, existing); - default: - return undefined; + if (ext === ".md" || ext === ".mdc") { + return tryMarkdownEquivalent(expected, existing); } -} -/** - * Structured parse for known extensions. Returns `undefined` when the content - * cannot be parsed as that format (or the extension is not a structured one). - */ -function tryParseStructured(filePath: string, content: string): unknown | undefined { - const ext = extname(filePath).toLowerCase(); + const parsedExpected = tryParseStructured(filePath, expected); + const parsedExisting = tryParseStructured(filePath, existing); - switch (ext) { - case ".json": - case ".jsonc": { - const errors: ParseError[] = []; - const parsed = parseJsonc(content, errors); - return errors.length > 0 ? undefined : parsed; - } - case ".yaml": - case ".yml": - try { - return loadYaml(content); - } catch { - return undefined; - } - case ".toml": - try { - return smolToml.parse(content); - } catch { - return undefined; - } - default: - return undefined; + if (!parsedExpected.ok || !parsedExisting.ok) { + return undefined; } + + return isDeepStrictEqual(parsedExpected.value, parsedExisting.value); } /** * Whether a parsed structured value carries no information: `null`/`undefined`, - * an empty array, or an object whose every value is itself empty by this same - * rule. Any scalar (string, number, boolean) counts as content. + * an empty array, or a plain object whose every value is itself empty by this + * same rule. Any scalar (string, number, boolean) counts as content. + * + * Deliberately conservative — anything it cannot prove empty counts as content, + * because the only consequence of "not empty" is that the file gets written: + * + * - A non-empty array is content regardless of what its elements hold, so + * `[{}]` counts as content while `{"a":{}}` does not. + * - An object with a non-plain prototype counts as content. That covers `Date` / + * `TomlDate` values (whose payload is invisible to `Object.values`) and a + * `{"__proto__": {...}}` entry, which jsonc-parser resolves by replacing the + * prototype rather than creating an own property. + * - A value already on the current path counts as content, so a self-referential + * YAML anchor cannot drive this into infinite recursion. */ -function isEmptyStructuredValue(value: unknown): boolean { +function isEmptyStructuredValue(value: unknown, seen: Set = new Set()): boolean { if (value === null || value === undefined) { return true; } @@ -139,7 +126,17 @@ function isEmptyStructuredValue(value: unknown): boolean { return value.length === 0; } if (typeof value === "object") { - return Object.values(value).every(isEmptyStructuredValue); + if (seen.has(value)) { + return false; + } + const prototype = Object.getPrototypeOf(value); + if (prototype !== Object.prototype && prototype !== null) { + return false; + } + seen.add(value); + const empty = Object.values(value).every((child) => isEmptyStructuredValue(child, seen)); + seen.delete(value); + return empty; } return false; } @@ -166,11 +163,7 @@ export function fileContentIsEmptyPayload({ const parsed = tryParseStructured(filePath, content); - if (parsed === undefined) { - return false; - } - - return isEmptyStructuredValue(parsed); + return parsed.ok && isEmptyStructuredValue(parsed.value); } /** From fe7ebd6ab68b42fc44ea91c80d6f5832905a6583 Mon Sep 17 00:00:00 2001 From: dyoshikawa Date: Sun, 26 Jul 2026 23:49:41 -0700 Subject: [PATCH 3/3] fix(generate): cover opencode.jsonc and compare the whole relative path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second round of review feedback. - `opencode.jsonc` was missing from SHARED_USER_MANAGED_CONFIG_PATHS. The opencode generators write the `.jsonc` twin when neither file exists yet, so `rulesync generate --targets opencode --features permissions` with an empty payload still left an untracked `opencode.jsonc` containing `{"permission": {}}` — the exact symptom #2430 reports, and the path is not gitignored either because it never comes from getSettablePaths. Added, along with the matching `conditionallyEmittedExclusions` entry the gitignore reverse guard needs, and an e2e case asserting both spellings stay absent. - `isSharedUserManagedConfigPath` matched any path *ending* in a listed path. Since the compared value is the tool-relative path (never the output root), a deeper path can only come from a tool that relocates the file, so the suffix match only ever fired by accident: in global mode `~/.config/kilo/kilo.jsonc` was skipped while `~/.config/zed/settings.json` was not. It now compares the whole path, so global scope is covered exactly when a tool keeps the same relative layout under the home directory and is left alone otherwise. - Complete the docs list (`.claude/settings.local.json` and `opencode.jsonc` were missing), and switch the two new content-equivalence helpers to object arguments per .claude/rules/coding-guidelines.md. Co-Authored-By: Claude Opus 5 (1M context) --- docs/reference/cli-commands.md | 2 +- skills/rulesync/cli-commands.md | 2 +- src/cli/commands/gitignore-entries.test.ts | 3 ++ src/constants/shared-config-paths.test.ts | 17 +++++++++-- src/constants/shared-config-paths.ts | 30 ++++++++++++-------- src/e2e/e2e-permissions.spec.ts | 15 ++++++---- src/utils/content-equivalence.ts | 33 ++++++++++++++++------ 7 files changed, 71 insertions(+), 31 deletions(-) diff --git a/docs/reference/cli-commands.md b/docs/reference/cli-commands.md index e9ad72e1e..94caa8859 100644 --- a/docs/reference/cli-commands.md +++ b/docs/reference/cli-commands.md @@ -125,7 +125,7 @@ rulesync generate --check --targets "*" --features "*" ### Shared config files are never created empty -Some outputs are files Rulesync merges into rather than owns, because the tool (or you) keeps unrelated settings there — `.claude/settings.json`, `.vscode/settings.json`, `.antigravity/settings.json`, `.factory/settings.json`, `.devin/config.json`, `.codex/config.toml`, `.zed/settings.json`, `opencode.json`, `kilo.json(c)`, `reasonix.toml`, `.grok/config.toml`, `.vibe/config.toml`, `.amp/settings.json(c)`. These are deliberately **not** added to `.gitignore` by `rulesync gitignore`, so that settings you hand-author in them stay version-controlled. +Some outputs are files Rulesync merges into rather than owns, because the tool (or you) keeps unrelated settings there: `.amp/settings.json(c)`, `.antigravity/settings.json`, `.claude/settings.json`, `.claude/settings.local.json`, `.codex/config.toml`, `.devin/config.json`, `.factory/settings.json`, `.grok/config.toml`, `.vibe/config.toml`, `.vscode/settings.json`, `.zed/settings.json`, `kilo.json(c)`, `opencode.json(c)`, and `reasonix.toml`. These are deliberately **not** added to `.gitignore` by `rulesync gitignore`, so that settings you hand-author in them stay version-controlled. Because they stay committable, `generate` will not **create** one of them just to hold an empty payload: if Rulesync has nothing to contribute (e.g. no permissions map to that tool), the file is left absent instead of being written as `{}`. A file that already exists is always rewritten as usual, so nothing you authored is dropped. Every other generated file is written even when empty, since for a file Rulesync owns its existence is part of the output. diff --git a/skills/rulesync/cli-commands.md b/skills/rulesync/cli-commands.md index e9ad72e1e..94caa8859 100644 --- a/skills/rulesync/cli-commands.md +++ b/skills/rulesync/cli-commands.md @@ -125,7 +125,7 @@ rulesync generate --check --targets "*" --features "*" ### Shared config files are never created empty -Some outputs are files Rulesync merges into rather than owns, because the tool (or you) keeps unrelated settings there — `.claude/settings.json`, `.vscode/settings.json`, `.antigravity/settings.json`, `.factory/settings.json`, `.devin/config.json`, `.codex/config.toml`, `.zed/settings.json`, `opencode.json`, `kilo.json(c)`, `reasonix.toml`, `.grok/config.toml`, `.vibe/config.toml`, `.amp/settings.json(c)`. These are deliberately **not** added to `.gitignore` by `rulesync gitignore`, so that settings you hand-author in them stay version-controlled. +Some outputs are files Rulesync merges into rather than owns, because the tool (or you) keeps unrelated settings there: `.amp/settings.json(c)`, `.antigravity/settings.json`, `.claude/settings.json`, `.claude/settings.local.json`, `.codex/config.toml`, `.devin/config.json`, `.factory/settings.json`, `.grok/config.toml`, `.vibe/config.toml`, `.vscode/settings.json`, `.zed/settings.json`, `kilo.json(c)`, `opencode.json(c)`, and `reasonix.toml`. These are deliberately **not** added to `.gitignore` by `rulesync gitignore`, so that settings you hand-author in them stay version-controlled. Because they stay committable, `generate` will not **create** one of them just to hold an empty payload: if Rulesync has nothing to contribute (e.g. no permissions map to that tool), the file is left absent instead of being written as `{}`. A file that already exists is always rewritten as usual, so nothing you authored is dropped. Every other generated file is written even when empty, since for a file Rulesync owns its existence is part of the output. diff --git a/src/cli/commands/gitignore-entries.test.ts b/src/cli/commands/gitignore-entries.test.ts index dfa71161e..ae6d873b6 100644 --- a/src/cli/commands/gitignore-entries.test.ts +++ b/src/cli/commands/gitignore-entries.test.ts @@ -100,6 +100,9 @@ describe("registry derivation", () => { "**/.amp/settings.jsonc", // claudecode ignore feature with `fileMode: "local"`. "**/.claude/settings.local.json", + // Preferred over `opencode.json` when neither file exists yet, so it is + // chosen at write time rather than declared by getSettablePaths. + "**/opencode.jsonc", ]); const rawEntries = new Set(deriveAllGitignoreEntriesUnfiltered().map((tag) => tag.entry)); const stale = [...DERIVED_PATHS_NOT_GITIGNORED].filter( diff --git a/src/constants/shared-config-paths.test.ts b/src/constants/shared-config-paths.test.ts index 7680c9413..1d2da6d28 100644 --- a/src/constants/shared-config-paths.test.ts +++ b/src/constants/shared-config-paths.test.ts @@ -21,9 +21,20 @@ describe("isSharedUserManagedConfigPath", () => { expect(isSharedUserManagedConfigPath("opencode.json")).toBe(true); }); - it("matches at any depth, mirroring the `**` gitignore entries", () => { - expect(isSharedUserManagedConfigPath("packages/app/.vscode/settings.json")).toBe(true); - expect(isSharedUserManagedConfigPath("packages/app/reasonix.toml")).toBe(true); + it("matches both twins of a path a generator picks at write time", () => { + expect(isSharedUserManagedConfigPath("opencode.json")).toBe(true); + expect(isSharedUserManagedConfigPath("opencode.jsonc")).toBe(true); + expect(isSharedUserManagedConfigPath("kilo.json")).toBe(true); + expect(isSharedUserManagedConfigPath("kilo.jsonc")).toBe(true); + expect(isSharedUserManagedConfigPath(".amp/settings.json")).toBe(true); + expect(isSharedUserManagedConfigPath(".amp/settings.jsonc")).toBe(true); + }); + + it("compares the whole tool-relative path, not a suffix of it", () => { + // The output root is never part of the compared path, so a deeper path can + // only come from a tool that relocates the file — a different file. + expect(isSharedUserManagedConfigPath("packages/app/.vscode/settings.json")).toBe(false); + expect(isSharedUserManagedConfigPath(".config/zed/settings.json")).toBe(false); }); it("normalizes a leading ./ and native separators", () => { diff --git a/src/constants/shared-config-paths.ts b/src/constants/shared-config-paths.ts index 3f08f439f..11a47714f 100644 --- a/src/constants/shared-config-paths.ts +++ b/src/constants/shared-config-paths.ts @@ -1,11 +1,13 @@ import { toPosixPath } from "../utils/file.js"; /** - * Project-scope outputs that rulesync merges into rather than fully owns - * (user-managed settings files). Most paths come straight from a tool's default - * `getSettablePaths`; `.amp/settings.jsonc` (runtime probe twin of - * `.amp/settings.json`) and `.claude/settings.local.json` (claudecode ignore - * `fileMode: "local"` variant) are emitted only under non-default options. + * Tool outputs that rulesync merges into rather than fully owns (user-managed + * settings files), as paths relative to the output root. Most come straight from + * a tool's default `getSettablePaths`; the rest are twins a generator only + * chooses at write time or under non-default options: `.amp/settings.jsonc` + * (runtime probe twin of `.amp/settings.json`), `opencode.jsonc` / `kilo.jsonc` + * (preferred over the `.json` twin when neither file exists yet), and + * `.claude/settings.local.json` (claudecode ignore `fileMode: "local"`). * * Two behaviors are derived from this single list: * @@ -36,17 +38,21 @@ export const SHARED_USER_MANAGED_CONFIG_PATHS: readonly string[] = [ "kilo.json", "kilo.jsonc", "opencode.json", + "opencode.jsonc", ]; /** - * Whether a relative output path (POSIX or native separators) is one of the - * shared, user-managed config files above. Matches the same any-depth semantics - * as the derived gitignore entries: the path matches when it is exactly a listed - * path, or ends with a slash followed by one. + * Whether an output path is one of the shared, user-managed config files above. + * + * `relativePath` is the tool-relative path (`relativeDirPath` + file name, POSIX + * or native separators), never the output root, so the comparison is exact + * rather than a suffix match. Global-scope outputs are covered exactly when the + * tool keeps the same relative layout under the home directory (e.g. + * `~/.claude/settings.json`); a tool that relocates its global file (e.g. Zed's + * `~/.config/zed/settings.json`) is not matched, which is harmless because the + * `git status` noise this guards against is project-scope by nature. */ export function isSharedUserManagedConfigPath(relativePath: string): boolean { const normalized = toPosixPath(relativePath).replace(/^\.\//, ""); - return SHARED_USER_MANAGED_CONFIG_PATHS.some( - (path) => normalized === path || normalized.endsWith(`/${path}`), - ); + return SHARED_USER_MANAGED_CONFIG_PATHS.includes(normalized); } diff --git a/src/e2e/e2e-permissions.spec.ts b/src/e2e/e2e-permissions.spec.ts index f3f612ebd..421c14de2 100644 --- a/src/e2e/e2e-permissions.spec.ts +++ b/src/e2e/e2e-permissions.spec.ts @@ -89,12 +89,15 @@ describe("E2E: permissions", () => { }); it.each([ - { target: "antigravity-ide", relativePath: [".antigravity", "settings.json"] }, - { target: "factorydroid", relativePath: [".factory", "settings.json"] }, - { target: "copilot", relativePath: [".vscode", "settings.json"] }, + { target: "antigravity-ide", relativePaths: [[".antigravity", "settings.json"]] }, + { target: "factorydroid", relativePaths: [[".factory", "settings.json"]] }, + { target: "copilot", relativePaths: [[".vscode", "settings.json"]] }, + // opencode writes the `.jsonc` twin when neither file exists yet, so both + // spellings must stay absent. + { target: "opencode", relativePaths: [["opencode.json"], ["opencode.jsonc"]] }, ])( "should not create the shared $target config file when the permissions payload is empty", - async ({ target, relativePath }) => { + async ({ target, relativePaths }) => { const testDir = getTestDir(); // A permissions file whose categories map to nothing this tool models, so @@ -108,7 +111,9 @@ describe("E2E: permissions", () => { await runGenerate({ target, features: "permissions" }); - expect(await fileExists(join(testDir, ...relativePath))).toBe(false); + for (const relativePath of relativePaths) { + expect(await fileExists(join(testDir, ...relativePath))).toBe(false); + } }, ); diff --git a/src/utils/content-equivalence.ts b/src/utils/content-equivalence.ts index 6d5c52218..ebb84d4c3 100644 --- a/src/utils/content-equivalence.ts +++ b/src/utils/content-equivalence.ts @@ -22,7 +22,13 @@ const PARSE_FAILED: ParseResult = { ok: false }; * jsonc-parser (valid JSON parses the same as JSONC). Returns `ok: false` for * unknown extensions and for content that does not parse. */ -function tryParseStructured(filePath: string, content: string): ParseResult { +function tryParseStructured({ + filePath, + content, +}: { + filePath: string; + content: string; +}): ParseResult { const ext = extname(filePath).toLowerCase(); switch (ext) { @@ -91,8 +97,8 @@ function tryFileContentsEquivalent( return tryMarkdownEquivalent(expected, existing); } - const parsedExpected = tryParseStructured(filePath, expected); - const parsedExisting = tryParseStructured(filePath, existing); + const parsedExpected = tryParseStructured({ filePath, content: expected }); + const parsedExisting = tryParseStructured({ filePath, content: existing }); if (!parsedExpected.ok || !parsedExisting.ok) { return undefined; @@ -113,12 +119,19 @@ function tryFileContentsEquivalent( * `[{}]` counts as content while `{"a":{}}` does not. * - An object with a non-plain prototype counts as content. That covers `Date` / * `TomlDate` values (whose payload is invisible to `Object.values`) and a - * `{"__proto__": {...}}` entry, which jsonc-parser resolves by replacing the - * prototype rather than creating an own property. + * `{"__proto__": {…}}` entry, which jsonc-parser resolves by replacing the + * prototype rather than creating an own property. (`{"__proto__": null}` still + * counts as empty — a null prototype hides nothing.) * - A value already on the current path counts as content, so a self-referential * YAML anchor cannot drive this into infinite recursion. */ -function isEmptyStructuredValue(value: unknown, seen: Set = new Set()): boolean { +function isEmptyStructuredValue({ + value, + seen = new Set(), +}: { + value: unknown; + seen?: Set; +}): boolean { if (value === null || value === undefined) { return true; } @@ -134,7 +147,9 @@ function isEmptyStructuredValue(value: unknown, seen: Set = new Set()): return false; } seen.add(value); - const empty = Object.values(value).every((child) => isEmptyStructuredValue(child, seen)); + const empty = Object.values(value).every((child) => + isEmptyStructuredValue({ value: child, seen }), + ); seen.delete(value); return empty; } @@ -161,9 +176,9 @@ export function fileContentIsEmptyPayload({ return true; } - const parsed = tryParseStructured(filePath, content); + const parsed = tryParseStructured({ filePath, content }); - return parsed.ok && isEmptyStructuredValue(parsed.value); + return parsed.ok && isEmptyStructuredValue({ value: parsed.value }); } /**