Catcrafts/Crafter.Review
Watch
2
0
Fork
You've already forked Crafter.Review
 0
Code Issues 7 Pull requests Projects Releases 3 Packages Wiki Activity Actions

fix(init): the skill it writes ignores itself, as a generated directory

Browse source 
A repository that ignores .claude wholesale, as Crafter.Build does,
never sees in git the prepush skill init writes, and the question was
whether it should: it should not. The skill is generated
from the shipped text with init's marker and refreshed by every init, a
tracked copy lags the installed binary whenever the shipped skill
changes, and every refresh would be a diff in the consumer's own ranges.
Jorijn's ruling: every consumer leaves it untracked, one init per
checkout.

When init owns the skill - wrote or refreshed it - it writes a
.gitignore of `*` beside it, as Python's venv and pytest's cache do for
theirs: the directory ignores itself, that file included, on every
checkout that runs init, whatever the repository's own rules or one
machine's say, and nothing of the repository's is edited for it. A copy
init wrote that the repository tracks gets the step that untracks it, in
the output and in the README's migration. A skill the repository wrote
itself is kept and not ignored - the ignore file an earlier init wrote
beside it goes - and an ignore file init did not write is kept unless
--force; a link there, dangling or not, is refused like the skill's
other links unless --force, which replaces it (the old setup's exemption
- a link that resolves into the review-gates submodule at the toplevel,
not one that merely names it - is the skill directory's and SKILL.md's
alone), and nothing is written through it: the links go before anything
is written, and one init cannot remove refuses. The procedure's log
records the ruling.
This commit is contained in:
Jorijn van der Graaf 2026-11-01 17:24:03 +01:00
commit ddf32a8c02
Signed by: jorijnvdgraaf
GPG key ID: 2937E59CDCC1BCFB
4 changed files with 185 additions and 20 deletions
Download patch file Download diff file Expand all files Collapse all files

124
tests/Commands/main.cpp
View file

@ -85,18 +85,23 @@ echo '{"type":"result","subtype":"success","is_error":false,"result":"ok","total
};
const std::string shipped = Read(checkout / "process" / "skill" / "prepush" / "SKILL.md");
const std::string marker = "<!-- written by crafter-review init";
const std::string ignoreMarker = "# written by crafter-review init";
// init on a repository that has nothing yet, then again.
const fs::path r = repo("r");
const Process::Captured first = Self({"init"}, r);
const std::string skill = Read(r / ".claude" / "skills" / "prepush" / "SKILL.md");
const std::string skillIgnore = Read(r / ".claude" / "skills" / "prepush" / ".gitignore");
const Process::Captured firstStatus = Process::Run({"git", "-C", r.string(), "status", "--porcelain", "--untracked-files=all"}, std::nullopt, std::chrono::seconds(60), {}, Env);
const std::size_t frontmatter = shipped.find("\n---\n", 3) + 5;
Check("init: the config from the template (the three kinds as modes), the artifact and worktree dirs ignored, the skill with init's marker after its frontmatter, the next steps (trim each mode's lenses; the browser line, since the template lists the functional lens)",
Check("init: the config from the template (the three kinds as modes), the artifact and worktree dirs ignored, the skill with init's marker after its frontmatter and a .gitignore of `*` beside it - git sees nothing of the skill, by no rule of the repository's - the next steps (trim each mode's lenses; the browser line, since the template lists the functional lens)",
first.returnCode == 0 && Read(r / ".review-gates" / "config.json") == Read(checkout / "process" / "templates" / "config.json")
&& Read(r / ".gitignore") == ".review-gates/push/\n.review-gates/trees/\n" && skill.starts_with(shipped.substr(0, frontmatter))
&& skill.substr(frontmatter).starts_with(marker) && skill.ends_with(shipped.substr(frontmatter)) && first.out.contains(" config - until it reports no problem")
&& skill.substr(frontmatter).starts_with(marker) && skill.ends_with(shipped.substr(frontmatter)) && skillIgnore.starts_with(ignoreMarker) && skillIgnore.ends_with("\n*\n")
&& firstStatus.returnCode == 0 && firstStatus.out.contains(".review-gates/config.json") && !firstStatus.out.contains(".claude")
&& first.out.contains("wrote .claude/skills/prepush/.gitignore") && first.out.contains(" config - until it reports no problem")
&& first.out.contains("trim each to what this repository runs") && first.out.contains(" browser install - "),
first.out + first.err);
first.out + first.err + firstStatus.out);
// A kept config has no trim step (that line is the template's own,
// printed as init writes it); one that lists no lens is told to list
// them.
@ -112,18 +117,60 @@ echo '{"type":"result","subtype":"success","is_error":false,"result":"ok","total
Write(r / ".gitignore", "node_modules/\n/.review-gates/push\n");
Write(r / ".claude" / "skills" / "prepush" / "SKILL.md", std::format("---\nname: prepush\n---\n{} 0.0.1: old -->\nold text\n", marker));
const Process::Captured again = Self({"init"}, r);
Check("init again: the config and an entry already ignored are kept, the missing one added once, the skill it wrote refreshed",
Check("init again: the config and an entry already ignored are kept, the missing one added once, the skill it wrote refreshed, and the .gitignore beside it",
again.returnCode == 0 && Read(r / ".review-gates" / "config.json") == R"({"base": "origin/dev"})"
&& Read(r / ".gitignore") == "node_modules/\n/.review-gates/push\n.review-gates/trees/\n" && Read(r / ".claude" / "skills" / "prepush" / "SKILL.md") == skill
&& again.out.contains("refreshed .claude/skills/prepush/SKILL.md"),
&& again.out.contains("refreshed .claude/skills/prepush/SKILL.md") && again.out.contains("refreshed .claude/skills/prepush/.gitignore")
&& Read(r / ".claude" / "skills" / "prepush" / ".gitignore") == skillIgnore,
again.out + again.err);
// The repository's own .gitignore is never edited for the skill: one
// that ignores .claude wholesale (Crafter.Build) is kept as it is.
Write(r / ".gitignore", "node_modules/\n/.review-gates/push\n.review-gates/trees/\n.claude\n");
const Process::Captured wholesale = Self({"init"}, r);
Check("init: a repository that ignores .claude wholesale keeps its .gitignore as it is, and the skill its own .gitignore",
wholesale.returnCode == 0 && Read(r / ".gitignore") == "node_modules/\n/.review-gates/push\n.review-gates/trees/\n.claude\n" && wholesale.out.contains("kept .gitignore")
&& Read(r / ".claude" / "skills" / "prepush" / ".gitignore") == skillIgnore,
wholesale.out + wholesale.err);
// A copy init wrote that the repository tracks (a base-era init
// ignored nothing): the note says how to untrack it, on every init.
// A hand-written tracked skill is kept and not ignored - the ignore
// file the earlier init wrote beside it goes - with no note.
const fs::path tracked = repo("tracked");
Self({"init"}, tracked);
Process::Run({"git", "-C", tracked.string(), "add", "-f", ".claude/skills/prepush/SKILL.md", ".gitignore"}, std::nullopt, std::chrono::seconds(60), {}, Env);
Process::Run({"git", "-C", tracked.string(), "-c", "user.email=me@example.com", "-c", "user.name=Me", "commit", "-q", "-m", "track the skill"}, std::nullopt, std::chrono::seconds(60), {}, Env);
const Process::Captured trackedInit = Self({"init"}, tracked);
const Process::Captured trackedAgain = Self({"init"}, tracked);
Write(tracked / ".claude" / "skills" / "prepush" / "SKILL.md", "my own skill\n");
Process::Run({"git", "-C", tracked.string(), "-c", "user.email=me@example.com", "-c", "user.name=Me", "commit", "-q", "-am", "my own skill"}, std::nullopt, std::chrono::seconds(60), {}, Env);
const Process::Captured handWritten = Self({"init"}, tracked);
const bool stillIgnored =
Process::Run({"git", "-C", tracked.string(), "check-ignore", "--no-index", "-q", ".claude/skills/prepush/SKILL.md"}, std::nullopt, std::chrono::seconds(60), {}, Env).returnCode == 0;
Check("init on a tracked skill: one init wrote gets the note with `git rm --cached` on every init; one the repository wrote is kept and no rule ignores it any more - the earlier init's .gitignore beside it removed - and no note",
trackedInit.returnCode == 0 && trackedInit.out.contains("git rm --cached .claude/skills/prepush/SKILL.md") && trackedAgain.returnCode == 0 && trackedAgain.out.contains("git rm --cached")
&& handWritten.returnCode == 0 && handWritten.out.contains("kept .claude/skills/prepush/SKILL.md") && handWritten.out.contains("removed .claude/skills/prepush/.gitignore")
&& !fs::exists(tracked / ".claude" / "skills" / "prepush" / ".gitignore") && !stillIgnored && !handWritten.out.contains("git rm --cached"),
std::format("exits {} {} {}, ignored {}; {}{}{}{}", trackedInit.returnCode, trackedAgain.returnCode, handWritten.returnCode, stillIgnored, trackedInit.out, trackedAgain.out, handWritten.out,
handWritten.err));
Write(r / ".claude" / "skills" / "prepush" / "SKILL.md", "my own skill\n");
const Process::Captured kept = Self({"init"}, r);
const std::string mine = Read(r / ".claude" / "skills" / "prepush" / "SKILL.md");
const bool keptIgnoreFile = fs::exists(r / ".claude" / "skills" / "prepush" / ".gitignore");
const Process::Captured forced = Self({"init", "--force"}, r);
Check("init: a SKILL.md init did not write is kept; --force replaces it",
kept.returnCode == 0 && mine == "my own skill\n" && kept.out.contains("--force replaces it") && forced.returnCode == 0 && Read(r / ".claude" / "skills" / "prepush" / "SKILL.md") == skill,
Check("init: a SKILL.md init did not write is kept, and the .gitignore init wrote beside it goes; --force replaces the skill and writes the .gitignore again",
kept.returnCode == 0 && mine == "my own skill\n" && kept.out.contains("--force replaces it") && !keptIgnoreFile && forced.returnCode == 0
&& Read(r / ".claude" / "skills" / "prepush" / "SKILL.md") == skill && Read(r / ".claude" / "skills" / "prepush" / ".gitignore") == skillIgnore,
kept.out + forced.out);
// A .gitignore beside the skill that init did not write is kept;
// --force replaces it.
Write(r / ".claude" / "skills" / "prepush" / ".gitignore", "my own rules\n");
const Process::Captured foreignIgnore = Self({"init"}, r);
const std::string ownRules = Read(r / ".claude" / "skills" / "prepush" / ".gitignore");
const Process::Captured forcedIgnore = Self({"init", "--force"}, r);
Check("init: a .gitignore beside the skill that init did not write is kept, saying so; --force replaces it",
foreignIgnore.returnCode == 0 && ownRules == "my own rules\n" && foreignIgnore.out.contains("kept .claude/skills/prepush/.gitignore (init did not write it: --force replaces it)")
&& forcedIgnore.returnCode == 0 && Read(r / ".claude" / "skills" / "prepush" / ".gitignore") == skillIgnore,
foreignIgnore.out + forcedIgnore.out);
// The old setup: the skill dir a link into the submodule.
const fs::path old = repo("old");
@ -158,6 +205,69 @@ echo '{"type":"result","subtype":"success","is_error":false,"result":"ok","total
&& atHome.err.contains("is your home directory") && homeUntouched && atHomeForced.returnCode == 0 && plain.returnCode == 3
&& plain.err.contains("is not in a git checkout"),
refused.err + sub.err + atHome.err + plain.err);
// A .gitignore beside the skill that is a link - to a file outside
// the skill's directory, or dangling - is a link like the skill's
// others: refused before anything is written, never written through;
// --force replaces the link with init's file and leaves its target as
// it was.
const fs::path linked = repo("linked");
Write(linked / "outside.ignore", "keep me\n");
fs::create_directories(linked / ".claude" / "skills" / "prepush");
fs::create_symlink("../../../outside.ignore", linked / ".claude" / "skills" / "prepush" / ".gitignore");
const Process::Captured linkRefused = Self({"init"}, linked);
const bool linkUntouched = !fs::exists(linked / ".review-gates") && Read(linked / "outside.ignore") == "keep me\n";
const Process::Captured linkForced = Self({"init", "--force"}, linked);
std::error_code linkError;
const bool replaced = !fs::is_symlink(fs::symlink_status(linked / ".claude" / "skills" / "prepush" / ".gitignore", linkError))
&& Read(linked / ".claude" / "skills" / "prepush" / ".gitignore") == skillIgnore && Read(linked / "outside.ignore") == "keep me\n";
const fs::path dangling = repo("dangling");
fs::create_directories(dangling / "shared");
fs::create_directories(dangling / ".claude" / "skills" / "prepush");
fs::create_symlink("../../../shared/skill.ignore", dangling / ".claude" / "skills" / "prepush" / ".gitignore");
const Process::Captured danglingRefused = Self({"init"}, dangling);
Check("init: a .gitignore beside the skill that is a link is refused before anything is written and never written through, dangling or not; --force replaces the link with init's file, "
"its target untouched",
linkRefused.returnCode == 3 && linkRefused.err.contains(".claude/skills/prepush/.gitignore is a link to ../../../outside.ignore") && linkUntouched && linkForced.returnCode == 0
&& replaced && danglingRefused.returnCode == 3 && !fs::exists(dangling / "shared" / "skill.ignore") && !fs::exists(dangling / ".review-gates"),
linkRefused.err + linkForced.out + linkForced.err + danglingRefused.err);
// A .gitignore link whose target names review-gates is the
// repository's too - the old setup linked the skill's directory, never
// its .gitignore - so it is refused like any other. A link init cannot
// remove (its directory not writable, where permissions bind) refuses
// under --force before anything is written, nothing written through it.
const fs::path named = repo("named");
Write(named / ".review-gates" / "skill.gitignore", "mine\n");
fs::create_directories(named / ".claude" / "skills" / "prepush");
fs::create_symlink("../../../.review-gates/skill.gitignore", named / ".claude" / "skills" / "prepush" / ".gitignore");
const Process::Captured namedRefused = Self({"init"}, named);
// A SKILL.md link into the pipeline's own .review-gates/ names the
// submodule but does not resolve into it: refused and kept too.
const fs::path namedSkill = repo("named-skill");
Write(namedSkill / ".review-gates" / "skill.md", "mine\n");
fs::create_directories(namedSkill / ".claude" / "skills" / "prepush");
fs::create_symlink("../../../.review-gates/skill.md", namedSkill / ".claude" / "skills" / "prepush" / "SKILL.md");
const Process::Captured namedSkillRefused = Self({"init"}, namedSkill);
const bool namedSkillKept = fs::is_symlink(fs::symlink_status(namedSkill / ".claude" / "skills" / "prepush" / "SKILL.md", linkError))
&& Read(namedSkill / ".review-gates" / "skill.md") == "mine\n" && !fs::exists(namedSkill / ".review-gates" / "config.json");
const bool namedKept = fs::is_symlink(fs::symlink_status(named / ".claude" / "skills" / "prepush" / ".gitignore", linkError))
&& Read(named / ".review-gates" / "skill.gitignore") == "mine\n" && !fs::exists(named / ".review-gates" / "config.json");
const fs::path stuck = repo("stuck");
Write(stuck / "outside.ignore", "keep me\n");
fs::create_directories(stuck / ".claude" / "skills" / "prepush");
fs::create_symlink("../../../outside.ignore", stuck / ".claude" / "skills" / "prepush" / ".gitignore");
fs::permissions(stuck / ".claude" / "skills" / "prepush", fs::perms::owner_read | fs::perms::owner_exec);
const bool writable = static_cast<bool>(std::ofstream(stuck / ".claude" / "skills" / "prepush" / ".probe"));
fs::remove(stuck / ".claude" / "skills" / "prepush" / ".probe", linkError);
const Process::Captured stuckForced = Self({"init", "--force"}, stuck);
fs::permissions(stuck / ".claude" / "skills" / "prepush", fs::perms::owner_all);
Check("init: a .gitignore or SKILL.md link into a review-gates path that is not the submodule is refused and kept; a link init cannot remove refuses under --force, writing nothing (where "
"permissions bind)",
namedRefused.returnCode == 3 && namedRefused.err.contains(".claude/skills/prepush/.gitignore is a link to ../../../.review-gates/skill.gitignore") && namedKept
&& namedSkillRefused.returnCode == 3 && namedSkillRefused.err.contains(".claude/skills/prepush/SKILL.md is a link to ../../../.review-gates/skill.md") && namedSkillKept
&& (writable
|| (stuckForced.returnCode == 3 && stuckForced.err.contains("cannot remove the link .claude/skills/prepush/.gitignore") && Read(stuck / "outside.ignore") == "keep me\n"
&& !fs::exists(stuck / ".review-gates"))),
namedRefused.err + namedSkillRefused.err + stuckForced.out + stuckForced.err);
// doc: the texts, and where they are.
const Process::Captured prepush = Self({"doc", "prepush"}, base);