fix(init): install gitignore for runtime artifacts#14
fix(init): install gitignore for runtime artifacts#14bcokdilli wants to merge 4 commits intokomunite:mainfrom
Conversation
📝 WalkthroughWalkthroughIntroduces a new Changes
Poem
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8688573845
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "CLAUDE.md", | ||
| { source: ".claude", destination: ".claude" }, | ||
| { source: "CLAUDE.md", destination: "CLAUDE.md" }, | ||
| { source: "templates/gitignore", destination: ".gitignore" }, |
There was a problem hiding this comment.
Avoid overwriting project .gitignore on forced init
Adding templates/gitignore to TEMPLATE_ITEMS means kalfa init --force now replaces an existing project .gitignore with the small Kalfa template, deleting all pre-existing ignore rules for that repository. This is a destructive regression introduced by this change, because users often run --force to refresh Kalfa files and will unintentionally lose their repo-specific patterns (e.g., build artifacts, env files), immediately polluting git status and risking accidental commits of sensitive files.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/cli.integration.test.js (1)
83-85: Consider asserting the full managed ignore set, not only two lines.This test can silently pass even if other runtime ignore entries regress. Expanding assertions will keep template and behavior in sync.
Suggested patch
const gitignore = fs.readFileSync(path.join(targetDir, ".gitignore"), "utf8"); assert.match(gitignore, /^\.claude\/logs\/$/m); assert.match(gitignore, /^\.claude\/agent-memory\/$/m); +assert.match(gitignore, /^\.claude\/backups\/$/m); +assert.match(gitignore, /^\.claude\/hooks\/__counter$/m);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli.integration.test.js` around lines 83 - 85, The test currently only checks two lines in the .gitignore via the gitignore variable and two assert.match calls; update it to assert the full managed ignore set instead of partial matches by building the expected ignore block (as an exact multiline string or array of expected entries) and comparing it against gitignore (e.g., split and compare arrays or use strict equality on trimmed multiline text) so any missing/regressed runtime ignore entries fail the test; update the assertions around the gitignore variable and replace the two assert.match calls with a single deterministic equality check against the expected ignore content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/commands/status.md:
- Around line 184-190: Unify the output-length rule in
.claude/commands/status.md by deciding whether the target is a fixed 6 lines or
a range of 6–8 lines and making both the short-description sentence ("Çıktı
kısa, sabit ve deterministik olmalı. Hedef 6-8 satır; hiçbir durumda 15 satırı
geçme.") and the strict rule block ("Sert kural: - Normal durumda yalnızca bu 6
satırı yaz - Fallback durumda da yalnızca bu 6 satırı yaz") consistent: either
change the short-description to target 6 lines and update the range mention to
"Hedef 6 satır" and adjust the strict bullets to match "6 satır", or change the
strict bullets to allow 6–8 lines in both normal and fallback modes; ensure the
same numeric constraint appears verbatim in both places so implementations
cannot diverge.
In `@bin/kalfa.js`:
- Line 10: For the .gitignore template entry (the mapping with source
"templates/gitignore" and destination ".gitignore") change the copy logic so
that if a destination ".gitignore" already exists you read both the existing
.gitignore and the template, compute which non-empty unique lines from the
template are missing, append only those missing lines (with a single newline
separator) to the existing file, and write the merged result back; keep the
original full-file copy behavior for other destinations. Apply the same merge
behavior to the similar handling referenced around lines 129-130 so existing
.gitignore files get augmented instead of skipped or overwritten.
In `@templates/gitignore`:
- Line 6: Replace the broad ignore token __NEEDS_ONBOARD with a path-scoped
pattern so it only ignores files under the .claude tree; update the entry in
templates/gitignore to /.claude/__NEEDS_ONBOARD (or /.claude/__NEEDS_ONBOARD* if
variants/extensions exist) so the ignore applies only to the .claude/ directory
and does not hide similarly named files elsewhere.
---
Nitpick comments:
In `@tests/cli.integration.test.js`:
- Around line 83-85: The test currently only checks two lines in the .gitignore
via the gitignore variable and two assert.match calls; update it to assert the
full managed ignore set instead of partial matches by building the expected
ignore block (as an exact multiline string or array of expected entries) and
comparing it against gitignore (e.g., split and compare arrays or use strict
equality on trimmed multiline text) so any missing/regressed runtime ignore
entries fail the test; update the assertions around the gitignore variable and
replace the two assert.match calls with a single deterministic equality check
against the expected ignore content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a0971d85-499b-446c-ac92-fc2335d92c31
📒 Files selected for processing (8)
.claude/command-index.md.claude/commands/status.mdCLAUDE.mdREADME.mdbin/kalfa.jskomunite-kalfa-1.0.0.tgztemplates/gitignoretests/cli.integration.test.js
| Çıktı kısa, sabit ve deterministik olmalı. Hedef 6-8 satır; hiçbir durumda 15 satırı geçme. | ||
|
|
||
| Sert kural: | ||
|
|
||
| - Normal durumda yalnızca bu 6 satırı yaz | ||
| - Fallback durumda da yalnızca bu 6 satırı yaz | ||
| - Ek açıklama, ikinci blok, dipnot veya uzun liste ekleme |
There was a problem hiding this comment.
Output length rule is internally inconsistent.
Line 184 permits 6–8 lines, while Lines 188–190 require exactly 6 lines in all modes. This should be unified to avoid divergent implementations.
Suggested patch
-Çıktı kısa, sabit ve deterministik olmalı. Hedef 6-8 satır; hiçbir durumda 15 satırı geçme.
+Çıktı kısa, sabit ve deterministik olmalı. Her durumda tam 6 satır üret.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Çıktı kısa, sabit ve deterministik olmalı. Hedef 6-8 satır; hiçbir durumda 15 satırı geçme. | |
| Sert kural: | |
| - Normal durumda yalnızca bu 6 satırı yaz | |
| - Fallback durumda da yalnızca bu 6 satırı yaz | |
| - Ek açıklama, ikinci blok, dipnot veya uzun liste ekleme | |
| Çıktı kısa, sabit ve deterministik olmalı. Her durumda tam 6 satır üret. | |
| Sert kural: | |
| - Normal durumda yalnızca bu 6 satırı yaz | |
| - Fallback durumda da yalnızca bu 6 satırı yaz | |
| - Ek açıklama, ikinci blok, dipnot veya uzun liste ekleme |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/commands/status.md around lines 184 - 190, Unify the output-length
rule in .claude/commands/status.md by deciding whether the target is a fixed 6
lines or a range of 6–8 lines and making both the short-description sentence
("Çıktı kısa, sabit ve deterministik olmalı. Hedef 6-8 satır; hiçbir durumda 15
satırı geçme.") and the strict rule block ("Sert kural: - Normal durumda
yalnızca bu 6 satırı yaz - Fallback durumda da yalnızca bu 6 satırı yaz")
consistent: either change the short-description to target 6 lines and update the
range mention to "Hedef 6 satır" and adjust the strict bullets to match "6
satır", or change the strict bullets to allow 6–8 lines in both normal and
fallback modes; ensure the same numeric constraint appears verbatim in both
places so implementations cannot diverge.
| "CLAUDE.md", | ||
| { source: ".claude", destination: ".claude" }, | ||
| { source: "CLAUDE.md", destination: "CLAUDE.md" }, | ||
| { source: "templates/gitignore", destination: ".gitignore" }, |
There was a problem hiding this comment.
init won’t add Kalfa ignore rules when .gitignore already exists.
Because existing files are skipped by default, the new .gitignore template is not applied in the common case of initialized repos. This leaves runtime artifacts unignored unless users force-overwrite their whole .gitignore.
Suggested approach: merge missing template lines instead of full-file copy for `.gitignore`
+function mergeGitignoreTemplate(sourcePath, destinationPath, options, results) {
+ const templateLines = fs
+ .readFileSync(sourcePath, "utf8")
+ .split(/\r?\n/)
+ .filter((line) => line.trim().length > 0);
+
+ const existingLines = fs.existsSync(destinationPath)
+ ? fs.readFileSync(destinationPath, "utf8").split(/\r?\n/)
+ : [];
+
+ const missingLines = templateLines.filter((line) => !existingLines.includes(line));
+ if (missingLines.length === 0) return;
+
+ results.copied.push(destinationPath);
+ if (options.dryRun) return;
+
+ ensureDir(path.dirname(destinationPath), false);
+ const needsNewline = existingLines.length > 0 && existingLines[existingLines.length - 1] !== "";
+ const prefix = needsNewline ? "\n" : "";
+ fs.appendFileSync(destinationPath, `${prefix}${missingLines.join("\n")}\n`, "utf8");
+}
+
for (const item of TEMPLATE_ITEMS) {
const sourcePath = path.join(packageRoot, item.source);
const destinationPath = path.join(targetRoot, item.destination);
+ if (item.destination === ".gitignore") {
+ mergeGitignoreTemplate(sourcePath, destinationPath, options, results);
+ continue;
+ }
copyRecursive(sourcePath, destinationPath, options, results);
}Also applies to: 129-130
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bin/kalfa.js` at line 10, For the .gitignore template entry (the mapping with
source "templates/gitignore" and destination ".gitignore") change the copy logic
so that if a destination ".gitignore" already exists you read both the existing
.gitignore and the template, compute which non-empty unique lines from the
template are missing, append only those missing lines (with a single newline
separator) to the existing file, and write the merged result back; keep the
original full-file copy behavior for other destinations. Apply the same merge
behavior to the similar handling referenced around lines 129-130 so existing
.gitignore files get augmented instead of skipped or overwritten.
| .claude/agent-memory/ | ||
| .claude/backups/ | ||
| .claude/hooks/__counter | ||
| __NEEDS_ONBOARD |
There was a problem hiding this comment.
Scope __NEEDS_ONBOARD to the .claude tree.
__NEEDS_ONBOARD is currently a broad ignore token and may hide unrelated files with the same name outside .claude/.
Suggested patch
-__NEEDS_ONBOARD
+.claude/__NEEDS_ONBOARD📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| __NEEDS_ONBOARD | |
| .claude/__NEEDS_ONBOARD |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@templates/gitignore` at line 6, Replace the broad ignore token
__NEEDS_ONBOARD with a path-scoped pattern so it only ignores files under the
.claude tree; update the entry in templates/gitignore to
/.claude/__NEEDS_ONBOARD (or /.claude/__NEEDS_ONBOARD* if variants/extensions
exist) so the ignore applies only to the .claude/ directory and does not hide
similarly named files elsewhere.
Summary
.gitignoreduringkalfa init.claude/logs/,.claude/agent-memory/, and.claude/backups/.gitignoreWhy
Fresh installs were leaving Kalfa runtime files as untracked changes in the target project. This surfaced during
/statussmoke testing and makes a clean baseline harder to get afterinit.Validation
.claude/logs/*after init and confirmedgit statusstayed cleanSummary by CodeRabbit
New Features
/statuscommand providing quick git-based status snapshots with fallback support.Documentation
Chores
Tests