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.
The trust-boundary list names only remote channels, so local values that the
OS hands you — another process's command line or environment, a filename on a
shared volume, a path in a job payload — read as internal. The one rule about
destructive actions (LLM06) governs who may act, not what gets hit: a
confirmed call with a wrongly derived target deletes the root just the same.
A shape check ("absolute, at least one directory deep") proves well-formedness
and gets mistaken for authorization.
SKILL.md gets the extended trust-boundary item, a prose-only "Destructive
Operations on Derived Paths" next to File Upload Safety, and one line each in
the Input checklist, Red Flags and Verification. The worked example lives in
references/security-checklist.md, per skill-anatomy's rule to move detailed
material out of SKILL.md — the file is already over the 500-line budget, so
this keeps the per-activation cost to +11 lines.
Two limits are stated where the snippet is copied from, rather than implied
away: a marker inside the tree is self-attestation until its integrity is
protected and the expected owner comes from authenticated state, and
resolving a path and then operating on the name is a check/use race wherever
an untrusted process can swap an ancestor.
No new directory, frontmatter unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A correlation ID identifies a run, not the code path that started it. When a
scheduler, a replay endpoint and a manual CLI run all reach the same job and
write into one sink, the lines are interchangeable — attributing one falls
back to elimination against external records (scheduler history, process
table, deploy log) that only happen to still exist.
Adds a paragraph plus a short example to "3. Structured logging" — stamp the
entry point where the run starts, next to the correlation ID, and propagate
both across queue metadata and outbound calls — and one line each to Red
Flags, Verification, and references/observability-checklist.md.
The field is `entryPoint` rather than `source`: Elastic Common Schema reserves
`source.*` for network fields (source.ip, source.port, …), so a scalar
`source` collides with that mapping and can get documents rejected.
No new heading, no new directory, frontmatter unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addy Osmani