feat: support per-target / per-feature gitignore destination routing ($gitignoreDestination)#1503
feat: support per-target / per-feature gitignore destination routing ($gitignoreDestination)#1503dyoshikawa wants to merge 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds configuration-driven routing for rulesync gitignore output so entries can be written per tool / per feature into either .gitignore (default) or .gitattributes, enabling workflows like “don’t ignore generated tool files, but mark them via attributes”.
Changes:
- Introduces
$gitignoreDestinationandConfig#getGitignoreDestination()to resolve destination with feature-level precedence. - Enhances gitignore entry resolution to return richer metadata (entry + target(s) + feature) and updates the
gitignorecommand to write to.gitignoreand/or.gitattributes. - Adds unit tests and updates docs (and synced skill docs) describing the new routing behavior.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/features.ts | Adds GitignoreDestination schema/type and expands feature value schema. |
| src/config/config.ts | Defines $gitignoreDestination key constant and adds destination resolution API. |
| src/config/config.test.ts | Adds tests for destination defaulting and precedence. |
| src/cli/commands/gitignore-entries.ts | Adds resolveGitignoreEntries() returning entry metadata; adjusts validation warnings. |
| src/cli/commands/gitignore.ts | Groups entries by destination and writes Rulesync blocks to .gitignore and .gitattributes. |
| src/cli/commands/gitignore.test.ts | Updates mocks and adds a test for .gitattributes writing behavior. |
| docs/guide/configuration.md | Documents $gitignoreDestination usage and precedence. |
| docs/reference/cli-commands.md | Documents destination routing behavior for rulesync gitignore. |
| skills/rulesync/configuration.md | Synced doc update describing $gitignoreDestination and options table. |
| skills/rulesync/cli-commands.md | Synced doc update describing destination routing behavior. |
| const gitattributesResult = await updateRulesyncFile({ | ||
| filePath: gitattributesPath, | ||
| entries: gitattributesEntries, | ||
| }); |
There was a problem hiding this comment.
Entries routed to .gitattributes are currently written as the same bare ignore-pattern strings used for .gitignore. In .gitattributes, a line without attributes has no effect, so this won’t achieve the intended behavior (e.g., marking paths as linguist-generated). Consider transforming entries for the gitattributes destination to include the desired attributes (and updating removeExistingRulesyncEntries / isRulesyncEntry so the command can reliably update/clean prior Rulesync-managed .gitattributes blocks).
| export const FeatureValueSchema = z.union([ | ||
| z.boolean(), | ||
| FeatureOptionsSchema, | ||
| GitignoreDestinationSchema, | ||
| ]); |
There was a problem hiding this comment.
FeatureValueSchema now allows the string literals "gitignore" / "gitattributes" for any per-feature key (e.g. targets.<tool>.rules: "gitattributes"), which previously would have been rejected at parse time. This will silently disable the feature (since isFeatureValueEnabled treats strings as disabled) and can hide config mistakes. Consider keeping GitignoreDestination out of generic FeatureValueSchema and instead validating $gitignoreDestination explicitly (e.g., as an optional reserved key in a dedicated schema/refinement), or add a runtime refinement that only permits these string values when the key is $gitignoreDestination.
| const perFeature: PerFeatureConfig = targetValue; | ||
| const toolLevel = Config.parseGitignoreDestination( | ||
| perFeature[TARGET_GITIGNORE_DESTINATION_KEY], | ||
| ); | ||
| if (feature) { |
There was a problem hiding this comment.
With the new reserved $gitignoreDestination key living alongside feature keys inside the per-feature object, invalid types like true or {...} can still pass schema validation (since per-feature values accept booleans/objects) and may then be treated as an enabled feature key elsewhere (e.g., feature normalization iterates object entries). It would be safer to validate $gitignoreDestination’s type/value at config load time and ensure this reserved key is excluded from feature-key enumeration/normalization paths.
| gitattributes.add(entry.entry); | ||
| } |
There was a problem hiding this comment.
When a single registry entry applies to multiple tool targets, and those targets resolve to mixed destinations, the current logic will add the same pattern to both .gitattributes and .gitignore (because it checks destinations.has("gitattributes") and also destinations.has("gitignore")). This can unintentionally ignore files even if some targets were configured for .gitattributes specifically to avoid ignoring. Consider defining a deterministic precedence for mixed-target entries (e.g., prefer .gitattributes as the less destructive option, or emit a warning and require consistent destination across all targets for a shared entry).
| gitattributes.add(entry.entry); | |
| } | |
| gitattributes.add(entry.entry); | |
| continue; | |
| } |
Motivation
rulesync gitignoreentries per tool or per tool×feature to either.gitignoreor.gitattributesas requested in issue Mark AI-generated files as linguist-generated using .gitattributes to reduce PR noise #1224, withgitignoreas the default and feature-level values taking precedence over tool-level ones.Description
"$gitignoreDestination"for object-formtargetsand support two values:"gitignore"(default) and"gitattributes", exposed asTARGET_GITIGNORE_DESTINATION_KEYand typed viaGitignoreDestinationinsrc/types/features.ts.Config#getGitignoreDestination(target, feature?)and parsing logic so tool-level and tool×feature-level values are resolved with feature-level winning when present (changes insrc/config/config.ts).entry + target(s) + feature) viaresolveGitignoreEntriesand updatefilterGitignoreEntriesto use it (changes insrc/cli/commands/gitignore-entries.ts).gitignorecommand to group resolved entries by destination and write Rulesync blocks into both.gitignoreand.gitattributesas appropriate, preserving previous cleanup / up-to-date behavior and JSON capture (changes insrc/cli/commands/gitignore.ts).$gitignoreDestinationusage and precedence (tests insrc/config/config.test.tsandsrc/cli/commands/gitignore.test.ts; docs indocs/guide/configuration.md,docs/reference/cli-commands.md, and syncedskills/rulesync/*).Testing
pnpm -s vitest run src/config/config.test.ts src/cli/commands/gitignore.test.ts src/cli/commands/gitignore-entries.test.tsand they passed locally.pnpm run cicheck:code(format/lint/typecheck/tests) which passed after formatting adjustments, and the full test suite completed (vitestall tests passed).pnpm run cicheck(content checks) failed oncspelldue to transient/generated cache files (tsx-0/*) being picked up by the spell-checker in this environment; this is an environment artifact and not caused by the code changes..gitattributeswhen configured; these tests succeeded in the runs above.Codex Task