Skip to content

feat(loop-drill): fire drills that prove guardrails actually fire - #600

Open
THRISHAL12345 wants to merge 2 commits into
cobusgreyling:mainfrom
THRISHAL12345:feat/loop-drill
Open

feat(loop-drill): fire drills that prove guardrails actually fire#600
THRISHAL12345 wants to merge 2 commits into
cobusgreyling:mainfrom
THRISHAL12345:feat/loop-drill

Conversation

@THRISHAL12345

Copy link
Copy Markdown
Contributor

The problem

docs/failure-modes.md names ten ways loops fail. Several have mechanical counterparts — loop-gate for path scope, loop-context's circuit breaker for runaway retries — and loop-audit awards points for having them.

Nothing checked whether they fire.

The sharpest case is the verifier. loop-audit awards its joint-largest signal (14 points) for a verifier, and gates L3 on it, via a filename check:

if (base.includes('verifier') || base === 'loop-verifier')   // auditor.ts:181

An empty file passes. On a scratch repo:

State Score Level
No verifier 34 L0
Zero-byte .claude/agents/verifier.md 55 L1

touch verifier.md is worth 21 points and a readiness level.

docs/failure-modes.md calls the resulting failure Verifier Theater (S2). docs/primitives.md calls maker/checker "the single most important structural pattern for reliable loops." The scorer meant to certify that pattern was performing it.

What this adds

loop-drill injects a known fault and asserts the guardrail responds. 19 drills across three guardrails, each tagged with the docs/failure-modes.md entry it exercises, so the report reads as coverage of that document rather than a list of anonymous assertions.

Over-Reach (Wrong Scope) — ✅ PROVEN
  ✅ denylist blocks **/.env
  ✅ denylist blocks **/payments/**
  ✅ file-count cap fires above 10 files
  ✅ ordinary change is still allowed
Infinite Fix Loop — ✅ PROVEN
  ✅ 3 identical failures trip the breaker
  ✅ a healthy run is not escalated
Token Burn — ⚠️ PARTIAL
  ⚠️ No tokenBudget configured — nothing caps spend mid-run.

Both directions, always

Every guardrail is drilled twice, because only one direction is easy:

  • sensitivity — the fault is caught. A guardrail that never fires is theater.
  • specificity — benign input is not caught. A guardrail that blocks everything passes every sensitivity drill while being useless.

A denylist: ["**"] catches every seeded fault and still fails the suite, because it blocks an ordinary docs change too.

The verifier canary

Borrows mutation testing: apply a small mechanical, behaviour-changing edit to real source and check whether the verifier notices. Deterministic and model-free — no agent invented the defect, so it costs only what your verifier costs.

control (repo)      no defect    -> must ACCEPT
control (worktree)  no defect    -> must ACCEPT
mutant × N          one defect   -> must REJECT

Three properties make the score trustworthy:

  1. Isolation — each run is an ephemeral git worktree, removed even on throw, so a verifier that writes or builds cannot touch the checkout.
  2. The worktree control — a fresh worktree has no node_modules, so npm test fails there for reasons unrelated to code quality. Without this second control every mutant scores as "caught" and a broken setup reports a perfect score. When it fails, mutants are skipped, not scored, and you are pointed at --setup.
  3. Bail on a failed control — a verifier that rejects a clean tree catches every mutant while being worthless, so the mutants never run.

Operators are limited to edits that change behaviour in any C-family language and read as bugs: ===!==, <=<, >=>, &&||, return truereturn false. <> and +- are deliberately excluded — they break TS generics and string concatenation, and would make a correct verifier look broken. Comments, tests, dist/ and node_modules/ are never mutated.

Verified end-to-end

Scenario Result
This repo's gate.yaml + breaker 18 passed, 1 skipped, exit 1
Real verifier (loop-gate's own test suite) 100% mutation score, exit 0
Rubber-stamp verifier (echo LGTM) 0%, named as Verifier Theater, exit 2
Reject-everything verifier Control fails, mutants correctly not run, exit 2

A rubber-stamp verifier, caught:

Verifier Theater — ❌ NOT PROVEN
  ✅ verifier accepts an unmodified tree
  ✅ verifier accepts a clean worktree
  ❌ verifier rejects strict-equality-flip in tools/loop-gate/src/gate.ts:34
      expected: non-zero exit (rejected)
      actual:   exit 0 (approved a seeded defect)
      flip === to !== (typeof v === 'string' -> typeof v !== 'string')

Mutation score: 0% of seeded defects rejected

52/52 tests pass. loop-gate (32) and loop-context (53) are unaffected. No worktrees leaked.

Design notes for review

  • Composes existing primitives, adds no new concepts. checkGate and checkCircuitBreaker are already pure functions, so the gate and breaker drills are offline and deterministic — no agent, no tokens. The tool is useful before anyone wires up a verifier.
  • Exit codes match loop-gate / loop-context — 0 proceed, 1 warnings, 2 escalate — so control scripts chain all three.
  • Dependencies are file: links to loop-gate and loop-context, following loop-swarm's existing dependency on loop-sandbox.

Two bugs I hit building it — both this tool's own failure mode

Worth surfacing because they are the argument for the tool:

  1. The canary would have reported 100% for a completely broken setup, until I added the worktree control. It would have manufactured exactly the false confidence it exists to prevent.
  2. The CI guard silently passed failing drills. I first wrote node cli.js . || [ $? -eq 1 ]. Under set -e, a failing command on the right of || does not exit the shell, so an exit-2 drill sailed through. Now an explicit if, verified against both exit 1 and exit 2.

Limits (also in the README)

  • Mutation is regex-based, not AST-based. It establishes a floor, not a ceiling — a verifier that catches every mutant is not proven against subtle logic errors.
  • Escalation Failure and Notification Fatigue have no drills yet — they need a notification sink to observe.
  • The canary costs whatever your verifier costs, once per mutant plus two controls. Start with --mutants 1 and --scope.

One call to flag

I wired the dogfood run into scripts/ci-validate-gates.sh, so this repo's CI now fails if its own guardrails stop firing. That is the strongest form of the idea, but it is also the most intrusive part of this PR — happy to drop it to a non-blocking step, or remove it entirely, if you would rather land the package first and adopt it separately.

docs/failure-modes.md names ten ways loops fail. loop-audit scores whether the
mechanical counterparts are *present*. Nothing checked whether they fire.

The sharpest case is the verifier. loop-audit awards its joint-largest signal
(14 points) for one, and gates L3 on it, via a filename check:

    if (base.includes('verifier') || base === 'loop-verifier')   // auditor.ts:181

An empty file passes. On a scratch repo, `touch .claude/agents/verifier.md`
moves the score 34 (L0) -> 55 (L1). docs/failure-modes.md calls the resulting
failure Verifier Theater and rates it S2; docs/primitives.md calls maker/checker
"the single most important structural pattern for reliable loops". The scorer
meant to certify that pattern was performing it.

loop-drill injects a known fault and asserts the guardrail responds. Each drill
is tagged with the failure mode it exercises, so the report reads as coverage of
docs/failure-modes.md rather than a list of anonymous assertions.

Every guardrail is drilled in both directions, because only one is easy:
sensitivity (the fault is caught) and specificity (benign input is not). A
denylist of ["**"] catches every seeded fault and still fails, because it blocks
an ordinary docs change too.

The verifier canary borrows mutation testing: apply a mechanical,
behaviour-changing edit to real source and see whether the verifier notices.
Deterministic and model-free, so it costs only what the verifier costs.

Three properties make the score trustworthy:
  - each run is an ephemeral git worktree, removed even on throw
  - a second control runs in a clean worktree; without it a missing
    node_modules fails every mutant for the wrong reason and a broken setup
    reports a perfect score, so mutants are skipped rather than scored
  - a verifier that rejects a clean tree bails before mutants run, so
    reject-everything cannot report 100%

Operators are limited to edits that change behaviour in any C-family language
and read as bugs. `<` -> `>` and `+` -> `-` are excluded: they break TS generics
and string concatenation, and would make a correct verifier look broken.

The gate and breaker drills are offline and deterministic -- no agent, no
tokens -- so the tool is useful before anyone wires up a verifier.

CI runs it against this repo, tolerating exit 1 (a skipped drill) and failing on
exit 2. The guard is an explicit if rather than `cmd || [ $? -eq 1 ]`, because
under set -e a failing command on the right of || does not exit the shell --
that shorter form silently passed an exit-2 drill when first written.
@github-actions

Copy link
Copy Markdown
Contributor

This PR changes paths that must run the real validate and audit workflows (tools, patterns, scripts, or CI).

Fork PRs from first-time contributors start with those workflows waiting for approval. A maintainer needs to open the Checks tab and click Approve and run workflows. Until that happens, branch protection will show the PR as blocked even after a review.

Content-only PRs (docs/, examples/, stories/, skills/, root markdown) skip this step — required checks are posted from this workflow instead.

— loop-engineering fork-pr-gate

@cobusgreyling cobusgreyling left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this — the gate/breaker drill design (sensitivity + specificity, exit 0/1/2 matching loop-gate/loop-context, worktree-isolated canary) is the right shape for proving guardrails actually fire.

Requesting changes on one test that currently cannot fail, plus a smaller typing issue in the canary runner.

Comment thread tools/loop-drill/test/drill.test.mjs Outdated
// maxIterations below stagnationThreshold means the synthetic ledger trips
// the iteration cap first; the stagnation rule itself stays unproven.
const results = runBreakerDrills({ ...DEFAULT_BREAKER, stagnationThreshold: 99, maxIterations: 1000 });
assert.equal(byId(results, 'breaker.stagnation').outcome, 'passed');

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test cannot fail and does not exercise the case the name/comment describe.

  • Name says the unreachable-threshold case is reported as failed.
  • Comment says maxIterations is below stagnationThreshold, so the iteration cap trips first and stagnation stays unproven.
  • Values are stagnationThreshold: 99, maxIterations: 1000 — so maxIterations is above the threshold, and a 99-identical-failure ledger can still trip stagnation.
  • Assertion is outcome === 'passed'.

Please make the three agree. If the intended case is “threshold not reachable because the iteration cap fires first,” use something like stagnationThreshold: 99, maxIterations: 10 (or whatever runBreakerDrills actually needs) and assert failed (or skipped, if that is the real outcome). If the intended case is a still-reachable higher threshold, rename the test and drop the comment about an unreachable threshold.

As written this is a passing no-op and would not catch a regression in the unproven-stagnation path.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 09a50bd — you were right, and it was hiding a real bug.

The test was a no-op, and the reason it could never fail was that the drill itself only asserted decision.escalate, not which rule fired. So I tightened the drills first: breaker.stagnation, breaker.no-progress and breaker.token-budget now assert their own trigger, matching the standard the gate drills already applied via trigger !== 'denylist'.

That immediately exposed breaker.no-progress passing for the wrong reason. It built its ledger from errors differing only by a number (module 1 not found, module 2 not found), and errorSignature() collapses every number to # — so all five normalized to one signature and tripped stagnation. The no-progress rule was never exercised. It now uses errors that stay distinct after normalization.

The no-op is replaced by four tests that each fail when a trigger check is weakened. I verified that by reverting each check in turn:

Config Trigger What it proves
similarityThreshold: 95 (percent, not fraction) none — never escalates Identical failures never stop the loop
noProgressThreshold: 1 no-progress Breaker fired, stagnation still unproven
maxIterations: 2 max-iterations Same, via the iteration cap
similarityThreshold: 0 stagnation Stagnation swallows the no-progress drill

The middle two are the ones that matter for your point — the breaker does escalate there, so a bare decision.escalate assertion passes while the rule under test was never touched. Weakening the stagnation check fails 2 tests; weakening no-progress fails 1.

One gap I left rather than faked: breaker.token-budget has no wrong-trigger test. Its ledger is a single successful attempt, so stagnation/no-progress (which need failures) and max-iterations (checked after the budget) are all unreachable — there's no config where it escalates for another reason. That's noted in a comment in the test file.

Comment thread tools/loop-drill/src/canary.ts Outdated
shell: true,
timeout: timeoutMs,
maxBuffer: 8 * 1024 * 1024,
} as never);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

promisify(execFile) is being called as exec(command, options as never). execFile’s second argument is args: string[], so this relies on Node treating a non-array as options and on as never to hide it from tsc.

Prefer exec from node:child_process (the shell form) or execFile(process.execPath, …) / execFile('sh', ['-c', command], options) without the cast. The canary runs caller-supplied --verifier-cmd / --setup strings; keeping the spawn API honest makes timeout/killed handling less surprising too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 09a50bd. Agreed the cast was hiding the real signature.

Split into two helpers so each spawn is honest about what it takes:

  • promisify(exec) (shell form) for caller-supplied strings — --verifier-cmd and --setup. Shell semantics are what those flags actually mean, so this drops both the shell: true option and the as never.
  • promisify(execFile) with an argv array for git, so repo paths never reach a shell for re-parsing.

No casts remain in the file.

On timeout/killed: exec sets killed: true on the error when it kills the child, which the existing check already handles. Re-verified after the change — a hanging verifier still reports timed out after 2000ms rather than surfacing as a spawn error, and is still treated as a rejection.

Also re-ran the canary end to end to be sure the spawn swap didn't change behaviour: 100% mutation score against loop-gate's real test suite, exit 2 for a rubber-stamp verifier (echo LGTM), control-failure bail still working.

…ary spawn

Review feedback from @cobusgreyling on cobusgreyling#600.

1. test/drill.test.mjs: the unreachable-threshold test was a passing no-op.

   Its name, comment, values and assertion disagreed, and no config could make
   it fail. Chasing that down surfaced a real bug behind it: the breaker drills
   asserted only `decision.escalate`, not *which* rule fired. Escalating for
   another reason leaves the rule under test unproven -- the same standard the
   gate drills already applied via `trigger !== 'denylist'`.

   breaker.stagnation, breaker.no-progress and breaker.token-budget now assert
   their own trigger.

   That exposed breaker.no-progress passing for the wrong reason. It built its
   ledger from errors differing only by a number ("module 1 not found", "module
   2 not found"), and errorSignature() collapses every number to '#', so all
   five normalized to one signature and tripped *stagnation*. The no-progress
   rule was never exercised. It now uses errors that stay distinct after
   normalization.

   The no-op is replaced by tests that fail when the assertion is weakened,
   verified by reverting each trigger check in turn:
     - similarityThreshold as a percentage (95 instead of 0.95) -> identical
       errors never match, the breaker never fires, stagnation unproven
     - noProgressThreshold 1 -> the breaker escalates via no-progress, so
       stagnation is unproven even though the loop did stop
     - maxIterations 2 -> escalates via max-iterations, same conclusion
     - similarityThreshold 0 -> every error counts as the same error, so
       stagnation swallows the no-progress drill

   breaker.token-budget has no wrong-trigger test: its ledger is one successful
   attempt, so no other trigger is reachable. Noted in the test file rather than
   faked.

2. src/canary.ts: promisify(execFile) was called with an options object in the
   `args` position behind an `as never` cast. It worked only because Node
   detects a non-array second argument, and the cast hid the real signature.

   Caller-supplied strings (--verifier-cmd, --setup) now go through
   promisify(exec), the shell form, which is what those flags mean. git keeps
   execFile with an argv array and no shell, so repo paths are never re-parsed
   by a shell. No casts remain.

Canary re-verified end to end after the spawn change: 100% mutation score
against loop-gate's real suite, exit 2 for a rubber-stamp verifier, and timeout
handling still reports "timed out" rather than a spawn error. 57 tests pass.
@THRISHAL12345

Copy link
Copy Markdown
Contributor Author

@cobusgreyling I have made the changes, please verify it now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants