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.
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.
materializeWorkspace() creates directories in os.tmpdir() but
runBehavioral() never cleans them up on success or failure. If
execFileSync throws (timeout, signal, OOM), the workspace persists
in /tmp indefinitely with fixture data. The test file
run-evals-test.js uses try/finally cleanup (line 234), proving
the pattern was known but omitted in production code.
Fix: Wrap eval execution in try/finally with best-effort
fs.rmSync cleanup.
The skillName argument from --behavioral has zero validation and is
used directly in path.join() for reading .json files, reading
SKILL.md, and WRITING .grading.json results. A value like
'../../etc/passwd' traverses outside the project directory for
both reads and writes.
Fix: Validate skillName against /^[a-z0-9]+(-[a-z0-9]+)*$/ before
any filesystem operations. This matches the existing KEBAB_CASE
validation in skill-lint.js.
Two latent Tier-3 bugs caught in review before the path was ever exercised:
- The grader prompt embeds the full stream-json trace (up to megabytes) and
was passed as an argv entry, which would fail with E2BIG on any real run.
It now goes to `claude -p` over stdin; the executor prompt moves to stdin
for the same reason.
- The executor ran headless with no permission mode, so file edits and
command runs could be denied, forcing the narrate-instead-of-perform
failure mode that trace grading exists to catch. It now runs with
--permission-mode acceptEdits and a pre-approved tool list
(Read,Glob,Grep,Edit,Write,Bash), documented in the README.
Per review from @federicobartoli and @nucliweb on #342:
Tier 3 (behavioral):
- Grade the execution trace, not the final output: executor runs with
--output-format stream-json --verbose so the grader judges tool calls
and file edits rather than the model's self-reporting.
- Run each eval in a throwaway workspace; files[] fixtures materialize
from evals/fixtures/ so evals can operate on real code.
- Node-level timeouts on executor and grader calls; grader output parsed
and shape-validated before writing (raw saved on failure); the trace is
fenced as untrusted data in the grader prompt.
- All 24 behavioral evals flagged trust_level: "provisional" until they
gain fixtures; the runner surfaces this and exits nonzero on failed
expectations.
Tier 2 (deterministic):
- Negative triggers accept an "owner" skill that must outrank this one,
turning them into pairwise routing tests that cannot pass vacuously;
37 of 48 negatives now declare owners (the rest are tracked in #351).
- Warn when a case file is below the documented minimums (3 positive /
2 negative / 1 behavioral); promotion to error tracked in #352.
- Stemmer: cluster trailing y/i ("simplify"/"simplifies").
Baseline holds: 120 checks, 0 errors, 85% trigger rank-1 rate.
There was no way to measure whether skills trigger correctly, stay
distinct, or change agent behavior. This adds evals, aligned with what
the community has converged on, with a deterministic CI tier on top:
- evals/cases/<skill>.json for all 24 skills. The evals[] block uses
Anthropic skill-creator's evals.json schema verbatim (id, prompt,
expected_output, expectations[]) so its runner, benchmarks, and eval
viewer work against our files unmodified. A trigger block (this
repo's extension) adds positive/negative routing prompts per skill.
- scripts/run-evals.js, zero-dependency runner:
Tier 2 (CI): trigger evals via stemmed TF-IDF ranking over skill
descriptions (positive prompts must rank top-k, negative prompts
must not rank first), catalog collision detection between skill
descriptions, schema and coverage checks.
Tier 3 (opt-in): --behavioral <skill> executes each eval through
headless claude -p and grades the transcript against expectations[]
(superpowers-style); --dry-run previews without spending tokens.
- CI: run the deterministic tier in the validate-skills job.
- Docs: evals/README.md defines the framework and prior art;
CONTRIBUTING requires an eval file for new skills (warning-level in
the runner until in-flight skill PRs clear); CLAUDE.md pointers.
Current baseline: 120 checks pass, 85% trigger rank-1 rate across 72
positive prompts, zero catalog collisions.
Addy Osmani