refactor: zero-friction install — remove .claude/commands/ git tracking#23
refactor: zero-friction install — remove .claude/commands/ git tracking#23emredursun merged 4 commits intomainfrom
Conversation
…tall kit init no longer modifies existing .gitignore entries. Bridge files (.claude/commands/, .cursor/commands/, etc.) are now local-only artifacts regenerated by kit init/update instead of being tracked in git. - Remove ensureClaudeCommandsTracked() and checkBridgeGitignoreWarnings() - Add cleanupLegacyClaudeTracking() to revert v5.2.0 patterns - Detect and suggest untracking .claude/commands/ committed during v5.2.0 - Replace test suites with 7 new cleanup migration tests (13 total) - Update docs: bridge files are local-only, not git-tracked
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request transitions bridge files to be local-only and untracked by git, removing previous logic that ensured tracking. It introduces a cleanup mechanism for legacy .gitignore patterns and adds detection for previously tracked bridge files. Feedback focuses on making the .gitignore cleanup logic more robust by using anchored regular expressions to avoid matching commented-out patterns and adding corresponding test cases.
lib/io.js
Outdated
| if (!content.includes('.claude/*') || !content.includes('!.claude/commands/')) { | ||
| return { cleaned: false, reason: 'not-kit-pattern' }; | ||
| } |
There was a problem hiding this comment.
The guard condition using content.includes() is not precise enough. It can be triggered if the patterns .claude/* or !.claude/commands/ appear inside comments, leading to incorrect modifications of the .gitignore file. For example, if .claude/* is only in a comment but !.claude/commands/ is an active rule, the function will incorrectly remove the !.claude/commands/ line.
The check should use regular expressions anchored to the start of the line to ensure it's matching active gitignore patterns, consistent with the replace calls.
| if (!content.includes('.claude/*') || !content.includes('!.claude/commands/')) { | |
| return { cleaned: false, reason: 'not-kit-pattern' }; | |
| } | |
| const legacyPattern = /^\.claude\/\*/m; | |
| const legacyNegation = /^!\.claude\/commands\//m; | |
| if (!legacyPattern.test(content) || !legacyNegation.test(content)) { | |
| return { cleaned: false, reason: 'not-kit-pattern' }; | |
| } |
| expect(content).not.toContain('.claude/*'); | ||
| expect(content).not.toContain('!.claude/commands/'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
It would be beneficial to add a test case to ensure that cleanupLegacyClaudeTracking does not act when one of the legacy patterns is present only as a comment. This would have caught the potential bug in the guard condition and would prevent regressions.
it('does not act if one pattern is commented out', () => {
const { cleanupLegacyClaudeTracking } = loadModule();
fs.writeFileSync(
path.join(tmpDir, '.gitignore'),
'# .claude/*\n!.claude/commands/\n',
'utf-8'
);
const result = cleanupLegacyClaudeTracking(tmpDir);
expect(result.cleaned).toBe(false);
const content = fs.readFileSync(path.join(tmpDir, '.gitignore'), 'utf-8');
expect(content).toContain('!.claude/commands/');
expect(content).not.toContain('.claude/');
});Addresses @gemini-code-assist's finding at lib/io.js:226 — content.includes() could match patterns inside comments, causing incorrect .gitignore modifications. Replaced with start-of-line anchored regex to only match active gitignore patterns.
Addresses @gemini-code-assist's finding at tests/unit/gitignore.test.js:193 — ensures cleanupLegacyClaudeTracking does not act when legacy patterns appear only in comments, preventing regressions on the anchored regex guard.
🔧 ✅ PR Fix CompleteFindings addressed: 2 (0 human · 2 bot) Fix Summary
DiffsFix #1 — lib/io.js:226 (@gemini-code-assist: "guard condition using - // Guard: only act when BOTH patterns are present (Kit's unique fingerprint)
- if (!content.includes('.claude/*') || !content.includes('!.claude/commands/')) {
- return { cleaned: false, reason: 'not-kit-pattern' };
- }
+ // Guard: only act when BOTH patterns are active (not commented out)
+ const legacyPattern = /^\.claude\/\*/m;
+ const legacyNegation = /^!\.claude\/commands\//m;
+ if (!legacyPattern.test(content) || !legacyNegation.test(content)) {
+ return { cleaned: false, reason: 'not-kit-pattern' };
+ }Fix #2 — tests/unit/gitignore.test.js (@gemini-code-assist: "add a test case to ensure + it('does not act if one pattern is commented out', () => {
+ const { cleanupLegacyClaudeTracking } = loadModule();
+ fs.writeFileSync(
+ path.join(tmpDir, '.gitignore'),
+ '# .claude/*\n!.claude/commands/\n',
+ 'utf-8'
+ );
+
+ const result = cleanupLegacyClaudeTracking(tmpDir);
+ expect(result.cleaned).toBe(false);
+ const content = fs.readFileSync(path.join(tmpDir, '.gitignore'), 'utf-8');
+ expect(content).toContain('!.claude/commands/');
+ });Verification
@gemini-code-assist — fixes applied, ready for re-review. |
There was a problem hiding this comment.
Code Review
This pull request transitions bridge files, such as those in .claude/commands/, to be local-only artifacts that are no longer tracked by git. It replaces the previous git-tracking logic with a cleanup utility that reverts legacy .gitignore patterns and adds a diagnostic check in kit init to warn users if these files remain tracked. Feedback was provided to suppress stderr during the git status check to avoid displaying confusing error messages in directories that are not git repositories.
| const trackedClaude = execSync('git ls-files .claude/commands/', { | ||
| cwd: targetDir, encoding: 'utf-8', | ||
| }).trim(); |
There was a problem hiding this comment.
When running kit init in a directory that is not a git repository, execSync will output a 'fatal: not a git repository' error to stderr. Since this is a non-critical diagnostic check, it's better to suppress stderr to avoid confusing the user with unexpected error messages in the console.
| const trackedClaude = execSync('git ls-files .claude/commands/', { | |
| cwd: targetDir, encoding: 'utf-8', | |
| }).trim(); | |
| const trackedClaude = execSync('git ls-files .claude/commands/', { | |
| cwd: targetDir, encoding: 'utf-8', stdio: ['ignore', 'pipe', 'ignore'] | |
| }).trim(); |
Addresses @gemini-code-assist's finding at lib/commands/init.js:362 — execSync outputs 'fatal: not a git repository' to stderr when running kit init in a non-git directory. Suppress stderr with stdio config since this is a non-critical diagnostic check.
🔧 ✅ PR Fix Complete (Round 2)Findings addressed: 1 (0 human · 1 bot) Fix Summary
DiffFix #1 — lib/commands/init.js:360 (@gemini-code-assist: "suppress stderr to avoid displaying confusing error messages") const trackedClaude = execSync('git ls-files .claude/commands/', {
- cwd: targetDir, encoding: 'utf-8',
+ cwd: targetDir, encoding: 'utf-8', stdio: ['ignore', 'pipe', 'ignore'],
}).trim();Verification
@gemini-code-assist — fix applied, ready for re-review. |
|
Thank you for the update. I will proceed with a re-review of the pull request. |
1 participant
Summary
kit initno longer modifies existing.gitignoreentries — only adds the.agent/block.claude/commands/,.cursor/commands/, etc.) are now local-only artifacts, regenerated bykit init/kit updateensureClaudeCommandsTracked()andcheckBridgeGitignoreWarnings()(S7), replaced withcleanupLegacyClaudeTracking()that reverts legacy patterns from v5.2.0.claude/commands/files still tracked from v5.2.0 window and suggestsgit rm --cachedWhy
Users running
kit initsaw unexpected.gitignoremodifications (.claude/→.claude/*+!.claude/commands/) and untracked directories they didn't understand. This created confusion about what to commit. The bridge files are auto-generated from.agent/workflows/*.mdand don't need git tracking — they can be regenerated locally in < 100ms.Files changed (6)
lib/io.jscleanupLegacyClaudeTracking()lib/commands/init.jslib/updater.jsensureClaudeCommandsTracked→cleanupLegacyClaudeTrackingtests/unit/gitignore.test.jsdocs/ide-support.mdCHANGELOG.mdTest plan
kit initin fresh directory — only.agent/added to.gitignorekit initwith legacy.claude/*+!.claude/commands/— patterns cleaned upkit update— legacy patterns cleaned, bridges regenerated