fix(setup): symlink skill bin/ dirs so PreToolUse hooks resolve#1486
Open
genisis0x wants to merge 1 commit into
Open
fix(setup): symlink skill bin/ dirs so PreToolUse hooks resolve#1486genisis0x wants to merge 1 commit into
genisis0x wants to merge 1 commit into
Conversation
…ytan#1459) `link_claude_skill_dirs` previously only symlinked SKILL.md into the top-level skill directory, leaving `bin/` behind. PreToolUse hook frontmatter references \/bin/<script>; with no symlink the path resolves to a non-existent target and the hook silently no-ops. The same defect affects every skill that ships scripts - freeze, careful, browse, etc. Now creates a directory symlink for `bin/` alongside the existing SKILL.md symlink. Idempotent (rm + ln -snf) and tolerant of a prior real-directory entry at the target path. Source: issue garrytan#1459 §1. Defects §2 (registering PreToolUse hooks in user settings.json) and §3 (runtime path resolution) are out of scope; they need a separate design discussion about whether the setup script should mutate `~/.claude/settings.json` on user behalf.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves §1 of #1459 (`/freeze` enforcement chain). Companion defects §2 / §3 are intentionally out of scope.
`link_claude_skill_dirs` previously only symlinked `SKILL.md` into the top-level skill directory, leaving `bin/` behind. PreToolUse hook frontmatter references `\$CLAUDE_SKILL_DIR/bin/<script>`; with no symlink the path resolves to a non-existent target and the hook silently no-ops. The same defect affects every skill that ships scripts — `freeze`, `careful`, `browse`, etc.
What changes
In `setup`'s `link_claude_skill_dirs` function, after the existing `SKILL.md` symlink:
```bash
if [ -d "\$gstack_dir/\$dir_name/bin" ]; then
if [ -L "\$target/bin" ]; then rm "\$target/bin"; fi
if [ -e "\$target/bin" ] && [ ! -L "\$target/bin" ]; then rm -rf "\$target/bin"; fi
ln -snf "\$gstack_dir/\$dir_name/bin" "\$target/bin"
fi
```
Idempotent (rm + `ln -snf`) and tolerant of a prior real-directory entry at the target path so re-installs over an older copy upgrade cleanly.
Out of scope (explicitly)
Defects §2 (registering PreToolUse hooks in user `~/.claude/settings.json`) and §3 are out of scope. Both touch user-global state on behalf of `./setup` and deserve a separate design call — happy to follow up if you want me to take §2 next.
Test plan