fix: make the Phase 4 reverse-lint actually run (v1.17.0) - #13
Merged
Conversation
Phase 4 step 24 invoked doc-freshness-reverse-lint's reverse_lint.py through a hardcoded ~/.claude/skills/doc-freshness-reverse-lint/scripts/ path. That root only exists for personal-scope installs. When the skill is installed as a plugin it lives at ~/.claude/plugins/cache/<marketplace>/<plugin>/<version>/scripts/, so the path missed and the fallback logged "doc-freshness-reverse-lint: not installed" and continued. The step therefore did nothing while the handoff still read as clean, and the log line was read by a human as proof the skill was absent — it was installed at v1.3.0 the whole time. Fixes: - Resolve reverse_lint.py at both roots, plugin cache version-sorted so 1.10.0 beats 1.9.0. CLAUDE_PLUGIN_ROOT cannot be used here: it points at session-handoff's own root, not at another plugin's. - The not-found branch now says "not found" (a claim about lookup) instead of "not installed" (a claim about install state), and prints the paths tried. - Report the step as skipped, never as clean, when the script does not resolve. - Document both roots in the Dependencies section. - New tests/dependency-path-resolution.test.mjs guards the whole bug class: no interpreter may be invoked directly on a hardcoded ~/.claude/skills/ path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review of the first attempt (1.16.1) found the fix too shallow and its regression test unable to detect the bug it claimed to guard. Both are addressed here. TWO INDEPENDENT REASONS STEP 24 NEVER RAN 1. Wrong root on plugin installs. reverse_lint.py was reached through a hardcoded ~/.claude/skills/... path, which only exists for personal-scope installs. On a plugin install the lookup missed and the fallback logged "doc-freshness-reverse-lint: not installed". A user read that as proof the skill was absent; it had been installed at v1.3.0 the whole time. 2. Literal HEAD~N. `git diff --name-only HEAD~N..HEAD` shipped with N unsubstituted, so git exited 128 and the loop ran zero times — on EVERY install, plugin-scope or not. Fixing (1) alone would not have made the step do anything. Both ended at the same place: a summary row offering only "Clean / N candidates", so a step that never ran was written up as clean. CHANGES - New bundled scripts/resolve_dep.sh resolves a sibling plugin's script at both install roots. CLAUDE_PLUGIN_ROOT cannot do this — it points at this plugin's own root. Ranks on the version segment alone, so a second marketplace cannot make an older copy win (sort -V over whole paths ranks by marketplace name); uses find, not a glob, so zsh's nomatch cannot leak a raw error past 2>/dev/null; survives spaces in $HOME. - BASE must be a real SHA; the step reports skipped when it is not. The scan now includes uncommitted worktree edits, which a lessons.md written this session usually is. - The reverse-lint summary row became the fourth non-blankable row and enumerates "skipped: <reason>". "Clean" now asserts the lint ran. - skill_freshness_audit.py had the identical single-root bug: it scanned only ~/.claude/skills, so plugin-scope skills were never audited and the result was written up as clean. It now scans both roots, dedupes per skill (personal wins, else highest version), and treats a missing root as skipped rather than fatal. On this machine it goes from personal-scope only to 87 skills. - Every "<step>: not installed" log line became "not found — tried <paths>". A failed lookup is not evidence about install state. - 24b's audit invocation is guarded; previously its documented fallback was unreachable and python3 exited 2 on a non-existent path. TESTS The previous guard only regex-matched SKILL.md text. It stayed green through a one-character ||->&& edit that restored the original bug, and its "resolves at BOTH roots" assertion was satisfied by the Dependencies prose alone. Tests now EXECUTE resolve_dep.sh and skill_freshness_audit.py against fixture HOMEs (personal-only, plugin-only, both, neither, multi-version, multi-marketplace, name collision, spaces in HOME, zsh), bash -n the step-24 fence, and scope the text lint to that fence so prose cannot satisfy it. Verified against a 13-mutation battery covering every escape the review found; all 13 fail the suite. 49 tests, 47 pass, 0 fail (was 30/28 before this work). Also adds a manifest-consistency assertion that README's newest Version History entry matches plugin.json, so a bump cannot ship without the human-facing note. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
wan-huiyan
changed the title
The zsh check covers a real zsh-only failure mode — nomatch fires before 2>/dev/null applies — so it must keep running on macOS, where this skill is mostly used. ubuntu-latest has no zsh, so spawnSync returned a non-zero status and reddened the matrix. Skip on absence rather than deleting the coverage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round-two review found the previous commit still did not make step 24 run, and
that its own commit had been contaminated.
1. THE BOOTSTRAP CARRIED THE IDENTICAL DEFECT.
resolve_dep.sh correctly found a sibling plugin at either root — but the two
lines that located resolve_dep.sh checked only $CLAUDE_PLUGIN_ROOT and
$HOME/.claude/skills/session-handoff/. On a plugin-scope install
CLAUDE_PLUGIN_ROOT is frequently unset and ~/.claude/skills/session-handoff
does not exist, so the resolver never ran and step 24 reported SKIPPED — on
the very machine whose report started this. Fixing how we find a sibling's
script accomplished nothing while the code that finds our own had the same
bug one level up. New find_own_script.sh checks all three roots; the inline
bootstrap that locates IT also checks all three.
2. THE STEP IS NOW AN EXECUTABLE, NOT A SNIPPET.
The fenced block was still only bash -n'd and regex-matched, so every logic
mutation inside it survived — the previous round's exact complaint, relocated.
scripts/reverse_lint_step.sh holds the logic and the suite runs it against
fixture HOMEs, asserting the sentinel actually appears. Writing that test
immediately found a further defect: a brand-new lessons.md is untracked, and
`git diff` does not list untracked files, so the step scanned nothing on
exactly the session with the most to check. It now reads all three sources.
3. skill_freshness_audit.py MISSED ~93% OF INSTALLED SKILLS.
Its cache glob matched only <mkt>/<plugin>/<ver>/SKILL.md. The common layout
is <mkt>/<plugin>/<ver>/skills/<skill>/SKILL.md. It also applied that
cache-shaped glob to the personal root, so a git-clone install collapsed all
its skills onto the key "skills" and dropped all but the first. Both layouts
are handled per-root now, and the personal-over-cache precedence — previously
asserted by no test, and silently broken during this fix — is pinned.
On one machine: 87 skills before, 244 after.
4. resolve_dep.sh hardening: find -L so a symlinked cache entry is visible; a
non-semver directory ("main", "dev") can no longer outrank a release, and a
v-prefix sorts with its peers; an explicit exit 2 for unset HOME rather than
a shell-chosen status that differs under dash.
5. Accuracy: the non-blankable row count was four, not five; the README's
bundled-scripts list omitted the new scripts and is now pinned by a test;
step 24b gained step 24's BASE guard and a SKILL.md match that works for the
plugins/<name>/SKILL.md layout.
Also fixes a `case` pattern that bash refuses to parse inside $( ) — verified
under bash, sh, zsh and dash.
67 tests, 65 pass, 0 fail.
PROCESS NOTE: the previous commit, d9f67a7, silently included a review agent's
mutation because it was staged with `git add -A` while background agents were
editing the worktree. It reverted step 24 to the original hardcoded path and was
pushed. Both the branch and the lesson are corrected here; this commit's staged
diff was inspected before it was made.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A mutation battery found four assertions that stayed green while a real defect was injected, because each checked that something skipped without checking the reason. Deleting the BASE revision guard still produced a SKIPPED line — via a different branch — and passed. Removing the bootstrap's plugin-name equality check, so a session-handoff-fork directory would satisfy the lookup, passed too. Now pinned: the bad-BASE reason, the missing-dependency reason plus the roots it carries, the not-found-never-not-installed wording, lookalike-plugin rejection, and that a file listed by git but absent on disk is skipped rather than counted as a clean scan. 18/18 mutations caught. 72 tests, 70 pass, 0 fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1. 24c wrote a zero-byte usage record when session_metrics.py was missing.
`> "$OUT.json"` creates and truncates before python3 is exec'd, so the failure
path left a 0-byte file that reads as a written record to anything scanning
~/.claude/usage-tracking/ later. Guarded before the redirect, and given the
third (plugin-cache) root the other steps got — self-contained, since $FOS
belongs to step 24's fence and fences do not reliably share a shell.
2. The HEAD~N lint required `..` to follow immediately, so `"HEAD~N"..HEAD` —
one pair of quotes — passed. It now rejects HEAD~N anywhere in live code,
with comment lines stripped so a fence may still warn about what it must not
use.
3. Both text lints only ever looked at step 24's fence. Step 24b used the same
diff idiom and was outside them, so the plainest possible reintroduction one
step later was invisible. They now iterate every bash fence. The cross-plugin
lint also rejects a VARIABLE in the plugin-name position: ${DEP} is how this
defect came back during round two and it defeated a literal-name pattern
completely.
4. skill_freshness_audit.py resolved duplicate installs silently. A skill
installed from two marketplaces is exactly the ambiguity a freshness audit
exists to raise, so the losing copy is now reported in both JSON and human
output. On one machine that is 142 of 244 skills.
Two of my own lint bugs surfaced while verifying these: `[^>]*` broke on the
`--project=<slug>` placeholder, and `[ -f "$SM" ] ||` fallback ASSIGNMENTS were
being counted as guards. Both fixed; the mutation that motivated the lint is now
caught.
76 tests, 74 pass, 0 fail.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 6, 2026
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
1 participant
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.
Step 24 — the doc-freshness reverse-lint — had never run, and reported itself clean either way. Two adversarial review rounds found three separate causes; all three are fixed and verified by execution.
Why it never ran
1. Wrong root for the dependency.
reverse_lint.pywas reached through a hardcoded~/.claude/skills/...path. That root only exists for personal-scope installs; a plugin install lives at~/.claude/plugins/cache/<marketplace>/<plugin>/<version>/. The lookup missed and the fallback loggednot installed— which a user read as proof the skill was absent. It had been installed at v1.3.0 the whole time.2. The bootstrap had the identical bug. Fixing (1) changed nothing on a real machine, because the code that locates the resolver checked only
$CLAUDE_PLUGIN_ROOTand~/.claude/skills/session-handoff/. On a plugin-scope installCLAUDE_PLUGIN_ROOTis frequently unset and that directory does not exist, so step 24 still reported SKIPPED. Round one missed this entirely.3. A literal
HEAD~N.git diff --name-only HEAD~N..HEADshipped withNnever substituted — git exits 128 and the loop runs zero times, on every install regardless of scope.All three ended at the same place: a summary row offering only
Clean / N candidates.The shape of the fix
Three new bundled scripts, so the logic can be executed rather than only read:
find_own_script.sh— locates this plugin's own scripts across all three roots.resolve_dep.sh— the same for a sibling plugin.CLAUDE_PLUGIN_ROOTcannot do this; it points at this plugin's root. Ranks on the version segment alone (sort -Vover whole paths ranks by marketplace name, lettingaaa-mkt/2.5.0lose tozzz-mkt/1.0.0), usesfind -L, and a non-semver directory likemaincan no longer outrank a release.reverse_lint_step.sh— the step itself. It emits exactly one status line:clean (N file(s) scanned),N candidate(s), orSKIPPED — <reason>. "Clean" now asserts the lint ran.Writing the executable test immediately found a further defect: a brand-new
lessons.mdis untracked, andgit diffdoes not list untracked files — so the step scanned nothing on exactly the session with the most to check. It now reads committed, modified, and untracked.skill_freshness_audit.pyhad the same class of bug and was missing roughly 93% of installed skills: its cache glob matched only<mkt>/<plugin>/<ver>/SKILL.md, never the common<ver>/skills/<skill>/SKILL.md, and it applied that cache-shaped glob to the personal root so a git-clone install collapsed every skill onto the key"skills". On one machine: 87 skills before, 244 after. Duplicate installs are now reported rather than resolved silently (142 of those 244).Also: 24c no longer leaves a 0-byte usage record when its script is missing (
>truncates before the command runs); every<step>: not installedbecamenot found — tried <paths>; the non-blankable row count was wrong (four, not five).Testing
Round one's guard only regex-matched SKILL.md. It stayed green through a one-character
||to&&edit that restored the original bug. Round two found the replacement still only text-checked the fence that actually ships to the model.Tests now execute everything: the resolver, the bootstrap, the audit, and the step-24 fence itself — extracted from SKILL.md and run against fixture
$HOMEs withCLAUDE_PLUGIN_ROOTunset, asserting a sentinel from a stubreverse_lint.pyactually appears. The text lints iterate every bash fence (step 24b was outside them) and reject a variable in the plugin-name position, since${DEP}is how this defect came back mid-review.Verified against a mutation battery covering every escape both rounds found — operator flips, sort-key changes, dropped roots, quoted
HEAD~N, variable indirection, precedence inversion, unguarded redirects. All caught.76 tests, 74 pass, 0 fail (baseline before this work: 30/28). CI green on Node 20 and 22.
Version
1.16.0 to 1.17.0 — three new bundled scripts, changed audit scan defaults, and a dormant step starts running.
Review record
Round one: 2 blockers, 9 majors confirmed. Round two: 7 blockers, 14 majors confirmed. Everything blocking is fixed; the four majors left open after round two are closed in the final commit.
One process note worth recording: commit
d9f67a7was pushed containing a review agent's mutation, because it was staged withgit add -Awhile background agents were editing the worktree. It reverted step 24 to the original hardcoded path. The branch was force-corrected, and every subsequent commit had its staged diff inspected first.🤖 Generated with Claude Code