fix(claude): SessionStart hook SKILL.md path broken after src/ restructure#498
Open
ousamabenyounes wants to merge 3 commits into
Open
Conversation
…cture After the src/ directory restructure, caveman-activate.js moved from hooks/ to src/hooks/, but the relative path to skills/caveman/SKILL.md was not updated. The hook resolved __dirname/../skills/... which maps to src/skills/... — a path that doesn't exist. The readFileSync threw, the catch swallowed it, and every session silently served the weak hardcoded fallback ruleset (no intensity table, no examples). Models drifted back to verbose prose within a few turns. Fix: probe both post-restructure and legacy layouts before falling back: - src/hooks/ → ../../skills/caveman/SKILL.md (current) - hooks/ → ../skills/caveman/SKILL.md (legacy) Updated stale __dirname comment from hooks/ to src/hooks/. Closes JuliusBrussee#467
The fix shipped with no automated test. Adds tests/test_activate_skillmd_path.js spawning the real hook and asserting it loads the full SKILL.md ruleset, not the weak hardcoded fallback. - activation loads full SKILL.md: asserts `## Intensity` (SKILL.md-only) is in the output. RED when the candidate path is reverted to the stale `../skills` (src/hooks → src/skills, absent) — the hook drops to the fallback. - intensity table filtered to active level: `| **full** |` kept, `| **lite** |` dropped, proving the loaded file is actually parsed. node tests/test_activate_skillmd_path.js → 2 passed (revert: 2 failed, fallback). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes the Claude Code SessionStart activation hook silently falling back to the hardcoded rules because skills/caveman/SKILL.md was no longer found after the src/ restructure (hook moved under src/hooks/).
Changes:
- Update
src/hooks/caveman-activate.jsto probe multiple candidate SKILL.md paths (current + legacy layout) before falling back. - Add a regression test that executes the real hook and asserts the output contains SKILL.md-only content (the
## Intensitysection + correct row filtering).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/hooks/caveman-activate.js |
Tries multiple SKILL.md locations to avoid silent fallback after hook path changes. |
tests/test_activate_skillmd_path.js |
Adds an executable Node-based test verifying SKILL.md is actually loaded and intensity table filtering works. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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.
Fixes #467
Problem
After the
src/restructure,caveman-activate.jsmoved fromhooks/tosrc/hooks/, but the relative path toskills/caveman/SKILL.mdwas not updated. The hook resolved__dirname/../skills/...→src/skills/...— a path that doesn't exist.readFileSyncthrew, thecatchswallowed it, and every session silently served the weak hardcoded fallback ruleset (no intensity table, no examples). Models drifted back to verbose prose within a few turns.Fix
Probe both layouts before falling back:
src/hooks/ → ../../skills/caveman/SKILL.md(current layout)hooks/ → ../skills/caveman/SKILL.md(legacy layout)Test — real RED → GREEN
tests/test_activate_skillmd_path.jsspawns the real hook and asserts the full SKILL.md is loaded (the## Intensitytable is SKILL.md-only — never in the fallback).GREEN (fix in place):
RED (candidate path reverted to the stale
../skills):The reverted hook emits the weak fallback — the exact silent-degradation this issue reports. The test proves the SKILL.md path is actually resolved.
Run:
node tests/test_activate_skillmd_path.js