fix(generate): stop creating empty shared config files - #2431
Merged
Conversation
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(...) ?? <default>` 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) <noreply@anthropic.com>
…onfig files 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Owner
Author
|
@dyoshikawa Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
rulesync generateleft untracked, empty files behind:These are shared config files rulesync merges into rather than owns, so they are deliberately excluded from
rulesync gitignoreviaDERIVED_PATHS_NOT_GITIGNORED. The result was that every generate run handed the user a file to manage without putting anything in it.This PR takes Option B from the issue (do not emit an empty merge-target file), which preserves the documented ownership model and needs no
.gitignorechange.Changes
Two causes, both fixed:
1.
readOrInitializeFileContentcreated the file as a side effectreadOrInitializeFileContent(filePath, initial)wroteinitialto disk when the file was missing, so a merge-target file was materialized beforewriteAiFilesever decided whether it had anything to write.Every generator now reads with
(await readFileContentOrNull(filePath)) ?? <default>instead — the pattern the copilot / zed / qwencode permissions generators had already adopted for exactly this reason. Three call sites that discarded the result entirely (antigravity/devin/grokcli hooks) are simply dropped. The util itself is removed so the eager-write path cannot come back.Side benefit:
--dry-run/--checkno longer touch the filesystem in ~30 generators.2.
writeAiFilescreated files for empty payloadsFeatureProcessor#writeAiFilesnow skips a file that does not exist yet when:AiFile#shouldSkipCreationWhenPayloadEmpty().An already-existing file is still rewritten, so user-authored content is never dropped, and every other file is still created regardless.
The path list moves to
src/constants/shared-config-paths.tsasSHARED_USER_MANAGED_CONFIG_PATHS, andDERIVED_PATHS_NOT_GITIGNOREDis now derived from it — one source of truth for "rulesync merges into this file but does not own it", so the gitignore exclusions and the creation rule cannot drift apart. (The predicate deliberately does not key offisDeletable(): that flag also protects global-scope files and files the orphan sweep cannot rediscover, which are unrelated concerns.)fileContentIsEmptyPayloadinsrc/utils/content-equivalence.tsdecides emptiness on the parsed document (JSON/JSONC/YAML/TOML), so{},{"permissions":{}}and{"mcpServers":{}}all count as empty while any scalar counts as content. It is deliberately conservative — a non-empty array, an object with a non-plain prototype (Date/TomlDate, or a__proto__entry that jsonc-parser resolves by replacing the prototype), a value already on the current path, and anything unparseable all count as content, because "not empty" only ever means "write the file".Verification
Generating every target/feature from this repo's
.rulesync/into a clean directory, before vs. after — exactly the three paths from the issue, and nothing else:.devin/config.jsonis still written in that particular run because it genuinely carriesmcpServerscontent; with an empty MCP payload it is skipped too.Tests added:
src/e2e/e2e-permissions.spec.ts— the regression test the issue asked for: an empty permissions payload leaves.antigravity/settings.json,.factory/settings.jsonand.vscode/settings.jsonabsent.src/constants/shared-config-paths.test.ts— path matching, including any-depth matches and partial-segment non-matches.src/utils/content-equivalence.test.ts— JSON/JSONC/YAML/TOML, comment-only YAML,null, arrays,__proto__, a self-referential YAML anchor, and a date-only TOML document.src/types/feature-processor.test.ts— the fourwriteAiFilesbranches, delegating to the realAiFile.prototypepredicate rather than a stub.pnpm cicheckpasses.Closes #2430