Extract the two fs.rmSync calls into clearGradingSlot(base), export it,
and call it from runBehavioral. Rewrite both slot-clearing tests to call
clearGradingSlot instead of inlining fs.rmSync, so each test exercises
the production cleanup path and fails if the call is removed.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move stale-file cleanup from persistGradingOutcome (which only ran on
rejected grading) to the top of each eval iteration, before the executor
is invoked. Both ${base}.grading.json and ${base}.grading.raw.txt are
removed with fs.rmSync(p, { force: true }), which ignores a missing file
but still throws on a permissions error.
This closes three gaps at the source:
- The bare catch around unlinkSync swallowed EACCES alongside ENOENT.
- An executor or grader crash skipped persistGradingOutcome entirely,
leaving the previous run's .grading.json intact.
- A successful grading left a stale .grading.raw.txt from a prior
rejected run beside the new .grading.json.
persistGradingOutcome now only writes, never deletes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Each .grading.json now carries a run object with executor_model,
grader_model, and timestamp so results can be attributed to a
specific model version and point in time.
The executor model is read from the init event of the stream-json
output that the executor already produces (type "system",
subtype "init", top-level model field). The grader model is recorded
as "unknown": the grader is a bare `claude -p` whose output
parseGrading extracts by regex, so switching it to stream-json would
break parsing, and adding --model is a behaviour change that does
not belong in a recording fix.
RESULTS_DIR is only ever created and never cleared, and the output
path is derived from skill name and eval id alone. When a run's
grading is rejected (parseGrading returns null), the runner writes
.grading.raw.txt but leaves any .grading.json from a previous
successful run in place. A collector reading that directory gets the
old score beside the raw output of the run that actually happened.
Extract persistGradingOutcome so the write-and-cleanup is a testable
unit, and unlink the stale .grading.json before writing the raw file.
When a Stop leaves no BLOCK_ placeholder to expand, the hook has to restore
the backup to recover the protected block — which overwrote whatever had
replaced the file. Copy that content into the cache as <id>.recovered first
and name both paths in the warning, so the rewrite is merged back by hand
rather than lost.
Adds a test that the fallback keeps the rewritten content in the cache, and
a minimal lifecycle test (Read, a direct write with no event, Stop) asserting
the write survives.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Stop branch restores every cached file from its backup, but that
backup is written on PreToolUse Read and refreshed only by the
PostToolUse Edit|Write branch. A change made to a filtered file through
any other route between Read and Stop — a Bash command, a formatter, an
external editor — is overwritten at session end. Reproduced in #295:
Bash wrote const editable = 2 and read it back, Stop left const
editable = 1 on disk, with no crash and no failed tool call.
The hook protects the content of an annotated block, not the file: the
header documents the backup as holding the model's changes, and Edit and
Write already preserve them. So Stop now runs the same placeholder
expansion the Edit|Write branch runs, against the file as it is on disk.
Whatever wrote the file wrote its filtered form with the placeholders
intact, so expanding it restores the protected blocks and keeps the
change, whichever route it came from.
The expansion body moves into an expand_file function that both branches
call, so the fuzzy-match and deleted-block warnings apply at Stop too.
When no cached BLOCK_<hash> is left, the file was rewritten wholesale and
the hook cannot place the block; it falls back to the backup as before,
now with a warning instead of in silence. That residual is documented in
SIMPLIFY-IGNORE.md. With no outside write, expansion reproduces the
backup exactly, so Read→Stop and Read→Edit→Stop are byte-identical.
Test 11 drives the real hook through Read, an Edit event, a write with no
event after it, and Stop; it fails at be4e44a on the two assertions for
the lost write. Suite: 28 passed, 0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two hardening changes to parseGrading:
- Once a result's id is validated, overwrite its text with the declared
expectation at that index. The id is the binding, so a grader that
paraphrases an expectation no longer leaks its own wording into the
saved grading report.
- Stop rejecting a grading whose summary.pass_rate disagrees with the
counters. pass_rate is derived, not an independent observation, so a
rounded or mis-divided value is now recomputed from the validated
results instead of discarding the run. The integer counters
(passed/failed/total) remain exact-match checks.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
parseGrading accepted any result set whose length matched the expected count, without verifying that each result corresponded to a declared expectation. A grader could return duplicates, omit an expectation, or include results for undeclared expectations and still pass validation.
parseGrading now takes the full expectations array, requires each result to carry an integer id in 1..n matching the numbered expectations in the prompt, rejects duplicates and out-of-range ids, and derives pass_rate from the validated counters. The grader prompt requests the id field and the call site in runBehavioral is updated.
Five tests added: reordered-but-complete results, duplicate ids, out-of-range ids, omitted expectations, untrusted pass_rate.
The plugin's SessionStart hook injected using-agent-skills/SKILL.md as
additionalContext on every session. Claude Code already discovers and
routes skills from their descriptions, so the hook stacks a second
router on the native one — the pattern docs/getting-started.md warns
against ("That creates two routers for the same task"), measured at
~3.5k wasted cache-creation tokens per session in #569.
Delete hooks/hooks.json (the registration; the plugin now ships no
hooks) and keep hooks/session-start.sh as a standalone script for hosts
without native skill routing, matching the fix shape suggested in the
issue. Update the hook header, CONTRIBUTING.md, codex-setup.md, and
gemini-cli-setup.md to match.
`## All 24 Skills` is contradicted by the sentence directly beneath it —
"The pack includes 25 skills total" — and by the section's own contents:
skills listed under the heading 25
directories under skills/ 25
validate-skills --strict 25 skills checked
difference (none)
The section does list every skill, meta-skill included, in its own `Meta`
subsection. So 25 is the honest number for the heading.
The nearby "24 lifecycle skills plus the `using-agent-skills` meta-skill"
phrasing is correct and is left alone — 24 is a real count, just not of
what the heading is counting.
Also retargets the one inbound anchor, which the rename would otherwise
break:
docs/adoption-guide.md:5 ...README.md#all-24-skills -> #all-25-skills
That link is the only reference to the anchor anywhere in the repo
(checked across md/json/ts/js), so the rename is complete.
Gates: validate-skills --strict 25 skills, 0 errors, 0 warnings; the JS
suite 44 pass / 0 fail.
stripFencedCodeBlocks matched fences with a single whole-document regex that
only recognised a column-zero backtick fence closed by a run of exactly the
same length. Three CommonMark-legal forms slipped through and their contents
were linted as prose: tilde fences, fences indented one to three spaces, and
closing fences longer than their opener. This is live on main today —
spec-driven-development's list-nested examples use indented fences, so the
linter has been reading their contents as skill text.
Replace the regex with a line scanner that tracks the opener's marker and
length, closes only on a same-marker run at least as long, treats an
unterminated fence as running to end of file (so sections after it fail
loud), and follows the CommonMark rule that a backtick fence's info string
may not contain backticks. Stripped lines become empty lines so positions
are preserved.
Add unit coverage for each form plus CRLF input, and run
scripts/lib/skill-lint-test.js in CI — it was the only test file the
workflow did not execute.
Revives the approach from #444 by @ujjwalks, which was approved and then
closed after a rebase conflict.
Closes#437
Eleven positive trigger prompts ranked their skill #2-#3 because the
descriptions lacked vocabulary users actually say (follow-up to #351):
- debugging-and-error-recovery: regression vocabulary (worked yesterday,
what broke)
- documentation-and-adrs: 'document an architecture decision', design
reasoning ('documentation' and 'document' don't stem together)
- doubt-driven-development: cross-examining assumptions, stress-testing
a plan for hidden failure modes, high-stakes changes
- git-workflow-and-versioning: splitting a messy working tree into
atomic commits
- incremental-implementation: thin/verifiable slices, next task from a
plan, feature flags
- shipping-and-launch: what needs to be in place before shipping
- spec-driven-development: PRD, objectives and scope
- test-driven-development: red-green-refactor
- using-agent-skills: deciding which workflow applies to the work
- source-driven-development: verify against the official docs
('docs'/'documentation' don't stem together)
Trigger rank-1 rate: 86% -> 100% (76/76), no new collision warnings.
CI floor raised from 80 to 95 per the ratchet policy; README baseline
updated and the stale #351 tracking sentence rephrased as guidance.
Addy Osmani