feat: add per-project dev-shell skills placement (task 0003) #3

alexion · 2026-07-22T22:23:10-04:00

alexion commented

2026-07-22 22:23:10 -04:00

Task: .claude/tasks/0003-per-project-placement-dev-shell.md

Summary

Adds the per-project placement output: a dev-shell helper that drops a project's selected skills into its own .claude/skills/, plus the check that proves it.

lib.mkSkillsShellHook (lib/mk-skills-shell-hook.nix) takes a list of selected skill derivations and returns a shellHook string. Selection is by derivation; the symlink name comes from each derivation's eval-time skillName. The hook is plain POSIX shell needing no home-manager context.
On shell entry the hook symlinks each selected skill as a direct child of .claude/skills/, pointing into the store. It is stateless: each entry first removes only store-pointing symlinks (deselected skills included), then relinks the current selection, so stale skills disappear with no manifest.
Real (non-symlink) skill directories are left untouched — including the collision case, where a hand-authored directory sharing a selected skill's name wins and the skill is skipped.
A self-ignoring .claude/skills/.gitignore is regenerated each entry, listing only the names the hook actually linked, so store symlinks stay out of git while hand-authored skills remain tracked.
The whole hook runs in a subshell so it leaks no names into the interactive shell.
A new nix flake check (checks/shell-hook.nix) sources the produced hook against a fixture project across two selections and asserts the full lifecycle.

Deviations

The check executes the hook, beyond the task's literal testing decision (which only requires asserting the hook string is non-empty and references the expected store paths/names). It does that, then additionally runs the hook against a fixture and asserts placement, stateless stale-removal, the collision guard, non-store-symlink survival, .gitignore contents, and no state file. This is a strict superset that proves the spec-mandated lifecycle (criteria 3–7) rather than trusting a string match.
Collision .gitignore correctness was tightened during implementation: a skill skipped due to a name collision is kept out of the managed set, so it is never gitignored and the hand-authored directory stays tracked. Only actually-linked skills are listed.

Review

Risk

Overall: Medium

Blast radius — Low: two new files plus a two-line wiring change in flake.nix's lib output and checks; no existing behavior altered.
Reversibility — Low: purely additive; deleting the two files and reverting the wiring fully undoes it, and the hook is stateless/self-regenerating with no persisted state.
Test coverage — Low: the diff ships a thorough nix flake check fixture exercising linking, stale-removal, collision-skip, hand-authored safety, and .gitignore regeneration across two successive selections.
Sensitive domain — Medium: not auth/payments, but it is generated shell that runs rm -f on symlinks under a developer's .claude/skills/ on every shell entry; deletion logic near real files warrants care, though it is guarded to store-pointing symlinks only.
Size & complexity — Low: ~180 lines, small and readable; hook logic is a single guarded loop plus a link function, no tangled control flow.
Runtime criticality — Medium: it is dev-only tooling (never production), but it executes automatically on every developer's shell entry and mutates their working tree, so a bug reaches many local checkouts silently.

Unaddressed findings

Standards

Single-quoted Nix interpolation of skillName into the shell string would break on a name containing a single quote (judgement, latent). Left as-is: skill names are kebab-case by convention, and hardening the charset belongs to the content tier, not this helper.
readlink is used to classify symlinks and is not strictly POSIX (judgement). Left as-is: it is present on every Nix-based machine this targets and has no clean substitute; the non-POSIX ln -n flag was removed in response to review.
Duplicated real-directory assertion in the check (Fowler, judgement). Left as-is: test explicitness is preferable to a shared helper here.

Spec

None. All eight acceptance criteria implemented; the extra execution-based testing was judged faithful, not overreaching.

Task: `.claude/tasks/0003-per-project-placement-dev-shell.md` ## Summary Adds the per-project placement output: a dev-shell helper that drops a project's selected skills into its own `.claude/skills/`, plus the check that proves it. - `lib.mkSkillsShellHook` (`lib/mk-skills-shell-hook.nix`) takes a list of selected skill derivations and returns a shellHook string. Selection is by derivation; the symlink name comes from each derivation's eval-time `skillName`. The hook is plain POSIX shell needing no home-manager context. - On shell entry the hook symlinks each selected skill as a direct child of `.claude/skills/`, pointing into the store. It is **stateless**: each entry first removes only store-pointing symlinks (deselected skills included), then relinks the current selection, so stale skills disappear with no manifest. - Real (non-symlink) skill directories are left untouched — including the collision case, where a hand-authored directory sharing a selected skill's name wins and the skill is skipped. - A self-ignoring `.claude/skills/.gitignore` is regenerated each entry, listing only the names the hook actually linked, so store symlinks stay out of git while hand-authored skills remain tracked. - The whole hook runs in a subshell so it leaks no names into the interactive shell. - A new `nix flake check` (`checks/shell-hook.nix`) sources the produced hook against a fixture project across two selections and asserts the full lifecycle. ### Deviations - **The check executes the hook**, beyond the task's literal testing decision (which only requires asserting the hook string is non-empty and references the expected store paths/names). It does that, then additionally runs the hook against a fixture and asserts placement, stateless stale-removal, the collision guard, non-store-symlink survival, `.gitignore` contents, and no state file. This is a strict superset that proves the spec-mandated lifecycle (criteria 3–7) rather than trusting a string match. - **Collision `.gitignore` correctness** was tightened during implementation: a skill skipped due to a name collision is kept out of the managed set, so it is never gitignored and the hand-authored directory stays tracked. Only actually-linked skills are listed. ## Review ### Risk **Overall: Medium** - Blast radius — Low: two new files plus a two-line wiring change in `flake.nix`'s `lib` output and checks; no existing behavior altered. - Reversibility — Low: purely additive; deleting the two files and reverting the wiring fully undoes it, and the hook is stateless/self-regenerating with no persisted state. - Test coverage — Low: the diff ships a thorough `nix flake check` fixture exercising linking, stale-removal, collision-skip, hand-authored safety, and `.gitignore` regeneration across two successive selections. - Sensitive domain — Medium: not auth/payments, but it is generated shell that runs `rm -f` on symlinks under a developer's `.claude/skills/` on every shell entry; deletion logic near real files warrants care, though it is guarded to store-pointing symlinks only. - Size & complexity — Low: ~180 lines, small and readable; hook logic is a single guarded loop plus a link function, no tangled control flow. - Runtime criticality — Medium: it is dev-only tooling (never production), but it executes automatically on every developer's shell entry and mutates their working tree, so a bug reaches many local checkouts silently. ### Unaddressed findings **Standards** - Single-quoted Nix interpolation of `skillName` into the shell string would break on a name containing a single quote (judgement, latent). Left as-is: skill names are kebab-case by convention, and hardening the charset belongs to the content tier, not this helper. - `readlink` is used to classify symlinks and is not strictly POSIX (judgement). Left as-is: it is present on every Nix-based machine this targets and has no clean substitute; the non-POSIX `ln -n` flag was removed in response to review. - Duplicated real-directory assertion in the check (Fowler, judgement). Left as-is: test explicitness is preferable to a shared helper here. **Spec** - None. All eight acceptance criteria implemented; the extra execution-based testing was judged faithful, not overreaching.

alexion added 1 commit 2026-07-22 22:23:10 -04:00

feat: add per-project dev-shell skills placement (task 0003) 49a73c30d1

Expose lib.mkSkillsShellHook, which takes a list of selected skill derivations
and returns a dev-shell shellHook string. On shell entry the hook symlinks each
selected skill as a direct child of the project's .claude/skills/, pointing into
the store. It is stateless: each entry first removes only store-pointing symlinks
(deselected skills included), then relinks the current selection, leaving
hand-authored real directories untouched and regenerating a self-ignoring
.gitignore of the managed names. Add a nix flake check that sources the produced
hook against a fixture project and asserts placement, stale-removal, the
collision guard, non-store-symlink survival, and the .gitignore contents.

alexion merged commit 49a73c30d1 into main

2026-07-22 22:32:34 -04:00

alexion deleted branch task-0003-per-project-placement-dev-shell

2026-07-22 22:32:34 -04:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: alexion/skills#3