mirror of
https://github.com/zernie/vigiles.git
synced 2026-09-14 20:53:57 +08:00
zernie
e78841174d
* feat: sweep the disaster battery across every hook a repo declares `verifyGuardrail` takes a command string. #211 taught it to honour a hook's `if` condition, but only when the caller passes it — its own comment has to ask. That left "forgot to pass the condition" reachable, and reaching it reproduces the exact false 7/7 #211 existed to kill: a narrow force-push guard certified as stopping `rm -rf /` and `cat ~/.ssh/id_rsa`. `experimental_verifyPluginGuards(dir, opts?)` takes a DIRECTORY instead. It reads the command, event, matcher and condition off the same registration (loadPlugin -> normalizeHooks), so the pairing cannot be made wrongly. Per hook it returns a discriminated union, not a score with nullable fields: measured the battery reached it; results/blocked/allowed/notRun not-applicable another event, or a matcher selecting no battery tool unresolved the command names a variable nothing sets Reading the score of a hook that never ran is a tsc error rather than a 0/7 someone quotes. A repo with no hooks — or none reachable — says so in `notes`, because `hooks: []` on its own reads as a clean bill of health, which is the same false confidence this battery exists to remove. It reports and does not judge: no throwing sibling, because a repo's config never declares which of its hooks is meant to be a guard. `assertBlocksDisasters` stays the gate once you have said so yourself. Harness-agnostic with Claude Code as the default, the runHarnessTest shape — every piece it composes already takes its harness by injection, and a shellHooks:false adapter reports n/a in words rather than an empty pass. Also exports `hookMatcherSelects` from core/hook-matcher.ts so the measured matcher semantics have one home; it fails open in the same direction `decideHookCondition` does, so a wrong answer can only ever say "ran and did not block", never invent a skip. Tested in both directions over a fixture plugin carrying all five shapes, the real vendored davila7 guard, a Codex-shaped repo, and the opencode prototype. Two mutations verified: dropping each hook's own condition brings the 7/7 back (3 assertions red), and ignoring the hook's event turns a Stop hook into a measured 0/7 (4 red). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: render a plugin-guard sweep, without giving an unmeasured hook a number `experimental_verifyPluginGuards` returned a discriminated union and nothing to print it with, so the one use case it exists for — "point the battery at YOUR hooks" — was a six-line fold over that union at every call site. That is a worse code block than the two-liner it replaces, and `cohesive-feature-delivery` row 9 (output parity) could not be claimed. `experimental_formatPluginGuardReport(report)` renders it as terminal text, and its contract is that the union's honesty survives becoming a string: measured prints `blocks n/7` and one row per battery event not-applicable prints its REASON under a ⊘ heading, and no count unresolved likewise notes printed FIRST and in full A rendered `0/7` beside a hook the battery never reached would reintroduce, one layer above the type system, exactly the false confidence the union was built to make unrepresentable. So it is asserted directly and in both directions: for a sweep that measured nothing the output must match no `n/m` at all, and must carry the sentence saying nothing was measured. Mutating the formatter to print `blocks 0/7` for a not-applicable hook fails that assertion and three others. MANY HOOKS stay readable by grouping the unmeasured half BY REASON — a repo with thirty hooks usually has two or three — and naming at most three hooks per reason before counting the rest. That is the only lossy step, and it is confined to the half whose payload is the reason rather than the hook; the measured half is never collapsed, since those are the hooks you came for. The per-event rows come from a shared `guardrailRow`, so this renderer and `formatGuardrailReport` cannot drift into two vocabularies for the same three outcomes; `formatGuardrailReport`'s output is byte-identical. The prefix is on the name because the report SHAPE is documented as likely to move, and a stable formatter pinned to an unstable shape is a lie. It comes off with the same change that takes it off the sweep. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: four ways the guard sweep answered for a hook it could not run An automated review of #212 found four, and all four move the same way: toward measuring less. Each has a test that failed before the fix. 1. An UNCOMPILABLE matcher scored 7/7. `hookMatcherSelects` failed open on every uncertain input, reasoning that imprecision can only add a run the harness makes. That reasoning holds where the harness fails open, and this is where it fails closed: the same module already reports `Bash(` as invalid-regex, "the harness can't compile it, so the hook never fires". So the sweep ran the battery against a hook the harness never spawns, and an unconditional-deny body came back blocking all seven disasters — the exact false 7/7 #211 removed, arriving through the matcher instead of the condition. The predicate is now `hookMatcherReach`, three-valued, and an uncompilable matcher is `not-applicable` with the matcher named. 2. Codex matcher semantics were ignored. `HookProtocol.matcherStyle` already says all Codex matchers are regexes; the helper applied Claude Code's literal-equality rule globally, so `ash` — which regex-matches Bash — was classified n/a and never tested. The style is threaded through. 3. Variables the command sets for itself read as unresolved. A `$NAME` regex over shell called two self-contained commands unresolvable: one that assigns the name before expanding it, and one written inside single quotes, where the shell expands nothing. Both are real guards that went unmeasured for a reason untrue of them. Reads now come from the shell's own parser (core/shell-vars.ts, the mvdan-sh parse bash-effects uses); a parse failure falls back to the regex and says so. 4. Adapter-declared event variables were not supplied. `eventEnvVars` exists to say what a synthesized hook event carries, and the sweep synthesizes one, so a Codex hook reading `$hook_event_name` or `$cwd` was unresolvable for lack of a variable the harness would have set. Now populated from the declaration. Deliberately NOT populated: `permission_mode`, `session_id`, `model` — the sweep does not know those, and a guard branching on an invented permission mode is a measurement of a program we made up. The direction is the same in the ambiguous places: refuse to score rather than score optimistically. `*` and `**` on a regex-matcher harness come back uncompilable rather than assumed to be special-cased; an expansion with a default counts as a dependency; a parse failure over-reports. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: five ways the guard sweep answered for a hook it never ran Two came from pointing the instrument at a real OSS corpus; three more from an automated review of #212. All five are one class — we run a DIFFERENT program than the harness runs, and record a score anyway — and all five move the same way: refuse rather than answer. Each has a test that failed first. 1. A hook whose SCRIPT IS MISSING scored 7/7. `python3 <missing>.py` exits 2, and 2 is Claude Code's deny code, so a guard that does not exist was certified as stopping every disaster in the battery. Worse than the uncompilable-matcher door fixed last round, which at least needs a malformed config: a relative script path is the commonest hook shape in the wild, and the campaign hit this three times in one run. THE EXIT CODE CANNOT DECIDE IT, and this repo already knew: run-script.ts says so in prose for its own coverage attribution ("`sh <missing>` exits 2 ... indistinguishable from a gate legitimately denying, so it cannot be encoded"). The fact was written down one module away and the sweep scored on it regardless. So the question is asked in two halves that no single signal covers. BEFORE the run, core/command-files.ts names the files the command hands a program to execute, and an absent one is `unresolved` -- no score at all, which is the assertion the mutation targets. AFTER it, a shell exit of 126/127 means the program never launched (missing interpreter, no execute bit, bad shebang); that is the shell's own code, not a language's convention and not a guess about stderr text, and `shellNeverLaunched` is now shared with run-script.ts rather than copied. The pre-flight narrowing is MEASURED, not taste. Over davila7's 107 real hook registrations, "any path-shaped operand" hits 31 with 8 wrong -- files a hook writes or later reads, plus `echo N/A`, which contains a slash -- while "interpreter head OR script extension" hits 23 with 0 wrong. 2. `**` was in MATCH_ALL on no evidence, and that direction MANUFACTURES AN ACCUSATION: a guard registered under it came back "allows 0/7", reported as letting seven disasters through, about a hook the harness never spawned -- the exact mirror of scoring one that could not start. Measured here against claude 2.1.263 with a marker file as the oracle: `*`, `.*` and `""` fire on a Bash call and `**` does not, with `.*` as the in-run control and a hand-run `*` swap ruling out "only the first hook per event fires". Pinned by src/hook-matcher-delivery.test.ts, the shape subagent-delivery already uses: prose cannot notice that someone else's product moved. 3. Variables were checked against `process.env`, which a CONFINED run does not have. `trusted: false` starts from --clearenv and restores only HOME/TMPDIR/PATH plus the caller's env, so a foreign hook reading an ambient variable was measured with it missing. The availability set now follows the execution MODE, decided by the same `decideSandbox` policy the runner uses, and `hookEnvironment` returns three fields because they answer three different questions: what is passed, what the hook will find set, and the subset whose value is known here (a confined HOME points at a sandbox temp dir, so it is present without being resolvable). 4. `layout.projectRootTokens` were never supplied, so `$CLAUDE_PROJECT_DIR` -- what an ordinary project hook reads, including this repo's own -- read as unresolvable while the harness supplies exactly the root being swept. 5. Assignment subtraction ignored execution order, and this one is worth naming plainly: the parser rewrite an hour earlier removed two ways to be wrong and added a third, in the worse direction. Subtracting every assigned name globally excused a read the assignment never reached. Measured against /bin/sh with the name exported first, `echo "$X"; X=1` and `FOO=1 sh -c "echo $FOO"` both print the AMBIENT value -- so both really do read the environment. Two existing tests asserted the opposite; they were the defect written down, and are corrected with the measurement in place of the argument. The rule is now dominance, not membership, and a prefix, subshell or function-body assignment excuses nothing. Where a claim about someone else's product is load-bearing it is pinned by a test against the real binary, not restated: #2 was re-measured here rather than taken from the report that found it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: three more doors to a score for a hook that never ran Review found five; four were real and one measurement had to be redone before the fifth could be. All four are the same class the branch exists to close: `sh` exits 2 both for a hook that legitimately denied and for a hook that never ran, so anything reaching the spawn that should not have comes back as a perfect 7/7. SYNTAX ERROR SCORED AS A BLOCK. `commandFileRefs` returns `{ parsed: false, refs: [] }` for a command the shell parser rejects, and the caller read only `refs` — so an unparseable command looked exactly like a clean one. Measured on the build before this commit: hook `echo "unterminated` → MEASURED blocks=7/7 exits=2,2,2,2,2,2,2 The parse verdict is now decided first, ahead of the variable check too: `shellVarReads` falls back to a regex when the parse fails, so it still named `$FOO` in `echo "$FOO` and the sweep blamed an unset variable for a syntax error, telling the caller to pass `env` for a command no `env` could have saved. PRE-FLIGHT AND RUNNER LOOKED IN DIFFERENT DIRECTORIES. `sandboxedSpawn` chdirs into a `work/` directory it has just created and left empty when no `cwd` is given, while the missing-file check resolved against `process.cwd()`. Same guard, same command, two directories: project dir → exit 0 (the guard's real verdict: allow) fresh empty dir → exit 2 (cannot open its script = DENY = a block) So a guard that allows was reported as blocking, out of nothing but the mismatch. `effectiveCwd` now derives the run's directory from the same policy the runner uses; a relative ref under confinement-without-`cwd` is unresolvable by construction, and an absolute one is still tested on disk because `/` stays ro-bound in there. THE PROJECT IS NOT THE PLUGIN. `$CLAUDE_PROJECT_DIR` and `$CLAUDE_PROJECT` were bound to the swept dir, which is right only while the two coincide. Sweeping an installed plugin against a host project — `dir` the plugin, `cwd` the project — pointed an ordinary project hook at the wrong tree: measured before this commit, a hook reading `$CLAUDE_PROJECT_DIR` came back `unresolved — not on disk here` for a script sitting in the project all along. The plugin-root token still follows `dir`; the project tokens and Codex's `$cwd` follow the run. SOURCE ORDER IS NOT CONTROL FLOW. An assignment excused a later read whenever its offset came first, so `false && MODE=safe; [ "$MODE" = safe ]` suppressed `MODE` although the shell goes on reading the environment — and under confinement the sweep clears it, so it would have measured the other arm of the guard. The denylist of non-persisting assignments is now an allowlist of unconditional ones (a top-level `NAME=value`, or an `export`/`readonly`/`declare`), which makes the unlisted construct default to excusing nothing rather than to excusing everything. The old rule was wrong on four of five measured shapes: `&&`, `if`, `&`, and a pipeline. AN EXTENSION IS NOT A LICENCE TO EXECUTE. `rm -f /tmp/stale.sh` — an ordinary cleanup hook whose file is meant to be absent — was reported as running a missing script, because the extension test ran independently of the head. It is evidence about the FILE, not about the command, so it now requires a head that could execute: a known data-only head silences it, an unknown head still carries it. Re-measured before narrowing, since narrowing that loses a true positive would be worse than the false positive it fixes. The pinned corpus behind the original table (107 registrations, 23 hits, 0 wrong) has moved, so this is a second sample rather than a reproduction: today's davila7/claude-code-templates gives 134 registrations / 120 distinct commands and 5 hits, all true positives reached by an interpreter head or a head that is itself a path — 5 before the clause, 5 after. Both numbers are recorded in the module header, labelled as different corpora. Each fix is pinned by a test proven with a mutation: dropping the parse gate, ignoring confinement in the pre-flight, collapsing project onto plugin, restoring source-order dominance, and removing the head check each fail on their own assertion, with the patch grepped in source and dist. Two existing tests caught the allowlist being too narrow while it was being written — `export NAME=value` is a DeclClause, not a CallExpr — and that gap is now handled and covered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: three hand-kept copies of a fact that already had one source Review's second round left five findings. Three are the same defect wearing different clothes — a fact with ONE authoritative source, restated by hand somewhere else, where the copy fell behind — so they are closed as classes rather than patched as places. Two are their own bugs. DERIVE, DON'T RESTATE CONFINEMENT. The sweep kept its own copy of the runner's mode expression and did not know `recordEgress` confines. So a `recordEgress` sweep pre-flighted a relative script against the host's cwd and env, accepted it, and handed it to a run that starts in a fresh empty directory with a cleared environment — where `sh` cannot open the script and exits 2, this harness's DENY code, scored as a block. `routeScriptRun` is now the one decision: `runScriptWith` dispatches on it and owns no policy of its own, and the pre-flight asks it. There is no list here left to fall behind — a future option that selects confinement is added once, in the runner, and both readers inherit it. WRAPPERS. `commandFileRefs` read `args[0]` as the head, so `env python3 guard` named no file at all and the same exit-2 door opened. `stripWrappers` already owns the wrapper list for the effect classifier, so it is exported and ASKED, not copied: env / command / timeout / sudo / nice / xargs / nohup, recursively, skipping each wrapper's own options and `NAME=value` words. NOT fixed, and said out loud in the code and pinned by a test so it is recorded rather than rediscovered: `exec` is a pass-through that list does not carry, and `stripWrappers` has its own recorded remainder (an escaped head behind a wrapper, `sudo g\it push`). Both belong in that module, where the safety matcher reads the same list and a change deserves the measuring a matcher change deserves — not in a private copy here. EXECUTION CWD. The pre-flight tested files against the CALLER's directory while the project-root variables named the swept root: two trees, one run. A guard on disk exactly where its config says was reported `unresolved`; and where the caller happened to hold the same relative path, the sweep ran and SCORED the caller's file under the swept repo's name. `effectiveCwd(opts, root)` answers once, defaults to the swept repository, and the runner is handed that same answer. An explicit `cwd` still overrides — the installed-plugin case, where hooks are READ from the plugin and RUN against a project elsewhere. TWO ORDERING BUGS, THEIR OWN A read inside an assignment's own initializer is never dominated by that assignment: the shell expands the RHS and only then binds the name. Keyed on the assignment's START offset it was, because the walk is preorder — the `Assign` was recorded before its own `ParamExp` was visited. So `MODE="$MODE"; [ "$MODE" = strict ]` reported NO dependency, and under confinement the sweep cleared an ambient value without reporting it unresolved and scored whichever branch the empty value took. Keyed on the END, the containment falls out of the arithmetic instead of needing a special case. `prompt` / `http` / `mcp_tool` / `agent` are real hook actions. `normalizeHooks` drops them correctly — nothing that spawns a shell can drive one — but the sweep read the empty list as "No hooks are declared", which is an accusation about the repository rather than a limit of the tier, written in the one sentence this report exists to get right. They are carried now as `report.unmeasurable` and reported declared-and-NOT-measured in the notes, the census and the rendering; the mixed case is counted too, because a reader who sees six measured hooks cannot otherwise know a seventh went unexamined. MUTATIONS Six, each reverting a fix to its old shape, each failing on its own assertion. Two passed GREEN first, and both were findings about the tests: - `routeScriptRun`'s unit tests pinned the ROUTER, but nothing pinned that the SWEEP asks it, so restoring the hand-kept copy went green. Added. - the cwd fixture's guard DENIED — and `sh missing.sh` also exits 2, so a correct measurement and a total failure to find the file both read 7/7. The fixture now ALLOWS, making the right answer 0/7: the one thing the false 7/7 cannot be confused with. That is this branch's own bug class, found inside its own test. No published number moves. `WRAPPER_HEADS` is untouched (the bash-effects diff is a doc comment and an `export` keyword): 7 seeds, 136 alternate spellings and the 2/7 -> 7/7 dogfood all unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: name the one class five review findings belong to `blocked` is decided by exit code 2, and an interpreter that cannot start its script also exits 2 — so a guard that never ran and a guard that denied are the same observation. The preflight that resolves a hook's script is a proxy for "did it run", and review has defeated that proxy five times. Two are fixed on this branch; three are not, and patching them individually would buy one spelling each while leaving the class open — extracting a script path from an arbitrary shell command is the same undecidable problem bash-effects.ts already documents for itself. Names the three open spellings and points at the fix that closes the class: a control probe, one benign command the guard must allow, run beside the battery. Blocked too means the program is failing to start, not blocking, and the hook is unmeasurable rather than scored. Comment only; no behaviour change. Follow-up: #213 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: a hook rejected by its own condition was scored, not excused Matcher `Bash` with `if: "Bash(terraform apply*)"` reaches the battery by matcher and matches none of it by condition. Every entry in `ran` came back `ran: false`, and the hook was still returned `measured` — so the renderer printed `blocks 0/7` and `sweepNotes`, seeing a measured hook, stayed silent. A guard the harness never spawned, reported as a guard that blocks nothing. That is the same false verdict as the other three non-verdicts and in the unsafe direction, so it gets the same shape: `not-applicable` with its own reason naming the condition, and no score anywhere. The module header said THREE things are not a verdict; the condition is the fourth and it now says so. Decided AFTER the run rather than before, deliberately: `ran: false` IS the runner's own condition verdict, so this reads the decision the harness would make instead of re-deciding it beside `decideHookCondition`. It costs nothing, because a rejected condition never spawns a process either. `ran` is non-empty (an empty `reachable` returned earlier) and a launch failure already returned `unresolved`, so every `ran: false` left is a condition rejection. The sole-hook path falls out: a repo whose only hook was that one now measures nothing, and `sweepNotes` reaches its "none of them reachable / Nothing was measured" branch instead of rendering an empty score. Four tests, and the boundary is pinned in both directions — a condition matching PART of the battery must still score. Mutations: removing the classification reddens three assertions (`expected 'measured' to be 'not-applicable'`); widening `every` to `some` reddens seven, including the partial-condition guard and the vendored davila7 hooks. Also records two review findings as KNOWN REMAINDER (#213) rather than patching them. Both fail toward UNDER-reporting — a real guard reads `unresolved`, nothing is falsely scored — and both were reproduced: an unexpanded `~` probes `<cwd>/~/.claude/…` so a guard present at $HOME is called missing, and `$PWD` is absent from the confined name set though /bin/sh sets it after bubblewrap's --chdir (same command: 7/7 unconfined, `unresolved` confined). Fixing either by resolving harder is the reflex the block already argues against. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
7.0 KiB
7.0 KiB
What experimental_ means
If a name you import from vigiles starts with experimental_, we ship it but do
not promise to keep it. It can change shape, be renamed, or disappear in any
release, including a patch release. Renaming it is not a breaking change and
does not bump the major version.
Everything so named has a stable alternative. The feature's own docs page says what it is.
Contents
The rule
If we export it and it is not stable, its name starts with experimental_.
Why the name, and not a note in the docs or the changelog: the name is in every call site, every diff, every review and every autocomplete for as long as the thing is unstable. An import line scrolls out of view. A changelog is read once. A doc note is read never.
Two checks keep the rule honest:
- The tag and the name cannot disagree. Every experimental export is declared
once with the
@experimentalTSDoc tag. An ESLint rule (local/experimental-name) fails the build if a tagged export lacks the prefix. - One experimental root per feature. The prefix says whether this name is
stable. It cannot say whether the names it depends on are. So a feature's whole
vocabulary hangs off one prefixed root:
experimental_agent.result(), never a plainresult()that only works withexperimental_agent(). The root's name also survives destructuring —const { result } = experimental_agentstill names the root at the binding site — which an import path does not.
What is experimental today
| Symbol | Since | Why it is not stable yet |
|---|---|---|
experimental_defineHook and its five siblings (experimental_defineFileGate, experimental_definePromptGate, experimental_defineStopGate, experimental_defineInject, experimental_defineReact) |
2026-08-20 | The named-state API (record / state) has one author and no outside user. There is no supported way to seed that state in a test other than runHook. vigiles compile appends a duplicate wiring block when re-run. Details: compiled hooks. |
experimental_alternateSpellings |
2026-09-02 | Days old. Its only user is this repo's own test. The set of rewrite rules and the id format of the generated cases are the parts most likely to change. Details: the disaster battery. |
experimental_verifyPluginGuards |
2026-09-07 | Days old, with no user outside this repository. The REPORT SHAPE is what is most likely to move — whether not-applicable stays one status or splits by cause, and whether the per-hook counts stay arrays of event ids. The prefix comes off when that shape survives sweeping several real third-party repos unchanged. Details: sweep every hook a repo declares. |
experimental_formatPluginGuardReport |
2026-09-07 | It renders PluginGuardReport, so it inherits that shape's instability exactly — a stable name here would promise more than its only input can. The prefix comes off with the same change that takes it off the sweep. Details: sweep every hook a repo declares. |
When the prefix comes off
All four of these must hold. They are about other people on purpose: everything else we could judge ourselves, and we would be generous.
- Someone outside this repository uses it, and we know what for. Our own in-repo test does not count: same author, same assumptions.
- Nothing about its shape has moved for a full release cycle. No signature, no return type, no option name.
- Every gap in the table above is closed, or written down as a permanent limitation with its reason.
- Testing it needs no private knowledge. If the only way to test it is to hard-code a path the runtime computes internally, it is not finished: a user cannot test their own use of it.
Dropping the prefix is a rename, so it ships as a feat! with a major bump.
How we learned this
skill() shipped under a stable name while docs/skills.md opened with
"skill() is experimental". Nothing caught the contradiction, because the rule
was a habit rather than a check.
We also tried a vigiles/experimental import path. It marked the import line,
which is out of view by the time anyone reads the call. It was retired.
See also
- Compiled hooks — the largest experimental surface, and the guide that explains what each entry point does.
- Stability — the stability promise for everything that is not prefixed.