From one defect to a standing invariant

SkillAI & models

Turn a defect class into a well-designed guard PROPOSAL (and, once a human promotes it, a well-built guard). Activate when writing a `## Proposed guards` entry after fixing a bug, and when a human has explicitly approved implementing a lint, pre-commit hook, CI check, assertion, or ratchet — never to ship a guard inside a fix PR. Covers naming the defect CLASS rather than the instance, checking whether an existing guard should have fired (a gap in it is itself a bug), preferring a check that iterates the SSOT over a per-case assertion, naming the guard's HOME (this repo, or the shared ci-truth-serum pack) and what belongs in each, the inert-feature class that only a behavior-driving smoke test can catch, writing a repo-local checks/*.py line lint and dogfooding it against the real tree before shipping, the WATCHED-SURFACE question (when this goes red, who actually sees it?), the vacuous-green failure mode, and why mutation testing runs on CI and never locally.

Available today. Use it from your connected AI after setup.

Connect ahel once, and every AI you use reads what you have installed.

Then ask your AI: use the From one defect to a standing invariant skill

What this skill tells your AI

The instructions your AI receives, as published by alexandermattturner/agent-glovebox in .claude/skills/defect-to-guard/SKILL.md and read by ahel’s review.

When a defect earns a guard proposal

A defect you fixed does not owe a guard. Propose one only when a GENERIC CI check would have caught the whole CLASS without anticipating this specific error, AND that class can recur. A one-off, a class the fix itself makes unreachable, and a class an existing check already covers each get no entry. When one does qualify, the output is a ## Proposed guards entry in the PR body, never a guard shipped in the same PR — root CLAUDE.md § Guard budget owns that rule and the cost/benefit arithmetic the entry must show, and it charges a substantial expansion of an existing check the same way. The steps:

  1. Name the class the bug belongs to — a documented-but-unrouted subcommand, a parsed-but-undeclared flag, a referenced-but-undocumented env var, a feature whose lib code is written but never wired onto the live path, a portability divergence, a swallowed error.
  2. Find whether a guard for that class already exists (cli-dispatch-sync, check-cli-flags-documented, check-env-documented, the layer-engagement self-test, check-exit-suppression) and, if so, why it didn't fire — a gap in the existing guard is itself the bug.
  3. If none exists, prefer a check that iterates the SSOT and asserts the second copy, so it covers every future member for free and never names the instance (the anti-lockstep doctrine's forward form — a generic guard is an SSOT-completeness check, not a per-case assertion).
  4. Pick the cheapest mechanism that can express the class, in this order: a type the compiler already tracks, then an AST rule, then a line or regex lint. The order tracks the false-positive rate. A type follows the value, so it never guesses at a shape; an AST rule sees structure, so it skips a match inside a string literal, a comment or a fixture; a text lint matches characters, so every legitimate use of those characters earns a disable comment, and each one hides the next real finding on its line. tsc --noEmit (pnpm check) covers the non-test *.mjs under scripts/, .claude/hooks/, .github/actions/, bin/lib/github-app/ and sbx-kit/, and nothing else there — tsconfig.json include names those globs and exclude drops **/*.test.mjs, so the Python and shell beside them get no type check at all. For a file it does cover, a provenance type needs no new machinery: brand the safe constructor's return value and make the dangerous sink accept only the brand. Reach for a line lint only when the class has no syntax to bind to — a banned word in prose, a path convention — and say in the proposal why. Show the mechanism, never just name it: the proposal carries the smallest snippet a reader can check — the signature that changes and the one line that now fails. "A provenance type" is a label; the three lines below are the thing, and a reviewer can judge them in ten seconds.
/** @typedef {string & {__checked: "rewrite"}} Checked */
/** Throws when the anchor misses. @returns {Checked} */
export function patchAnchored(src, anchor) {
  return /** @type {Checked} */ (applyAt(src, anchor));
}
export function writeGenerated(path, /** @type {Checked} */ content) {}
writeGenerated(p, src.replace(a, b)); // type error: .replace returns plain string

The hardest class is the inert-feature bug (lib code written and unit-tested but never reached on the launch path): a source-text or unit check passes green while the feature is dead, so the only honest catch is a behavior-driving smoke test that exercises the real path and asserts the observable outcome (see the writing-tests skill).

When a class genuinely resists a cheap generic guard, say so — don't fabricate a hollow per-instance check that only re-tests the one bug you already fixed.

A guard is an economic object — show the arithmetic in the proposal

Weigh both sides before proposing one, and state both.

  • Benefit: (defects of that class actually reaching this repo per year) × (cost of one escaping to main) × (fraction the guard catches).
  • Cost: latency on every commit/push/PR forever, false positives (each trains sessions to route around the check), maintenance when the code it greps moves, and one more entry in the overseer's working set.

Most proposed guards fail this arithmetic — the class fires once, the guard runs 10,000 times. A guard is worth shipping when the defect class is recurring, severe, and cheaply detectable (secret leaks, fail-open enforcement, broken required-check wiring); not to commemorate a one-off bug the fix's own regression test already pins. A ## Proposed guards entry must show the arithmetic, not just the mechanism.

A term in that arithmetic you have not measured is MEASURED, not handed back. The proposal is where "I could not price it, so the call is yours" enters, and it reads as diligence while leaving the human to run what you could have run. Price it on whichever surface answers: read the code that pays the cost, time the operation locally, or add the measurement to a live check under bin/checks/ that already boots the thing. Then write the number in the entry.

No budget line caps the blocking surface. A blocking check is admitted on its own cost-benefit case and kept by guard rent, never by a count. Do not propose restoring a count: it prices a fast per-file lint the same as a slow tree-wide sweep. The overseer's working-set size is still the real constraint, because a blocking check set is an oversight instrument only while the human knows every entry and what it certifies. Eligibility tracks what a check certifies: product/security behavior qualifies; process idiom stays advisory.

Guard rent. Every blocking check must be able to cite a real defect it caught pre-merge within the last 8 weeks. A citation is a COMMENT on the CI-health report issue (<!-- ci-aggregate-sensor -->), naming the check, the PR it blocked, and what the block prevented — never a registry file with a completeness test, and never the issue BODY, which the weekly sensor run regenerates in place and would erase. That issue must stay OPEN: the sensor scans open issues only, so closing it strands every citation on the old thread, which an audit reads as "uncited". Writing one is the job of the session whose PR the check blocked, at the moment it blocks — you are the only one who will ever know. Collection is a human-initiated audit, not something anybody polls: uncited → demote to advisory in a prune PR; advisory and uncited 8 more weeks → delete; citations older than 8 weeks are spent and get deleted at that audit. Security-boundary checks are exempt from deletion, not from citation.

False reds are the cost side of the same cycle, in the same unit and on the same thread. When a blocking check fails on a change that was CORRECT — you changed the check, waived it, or re-ran it unmodified — post a <!-- false-red --> comment carrying check:, pr:, what it blocked: and resolution:, one line each (docs/false-reds-per-week.md § Collection). One instance per occurrence. Changing the code under test instead makes it a true red and a rent citation. The weekly report tables rent against false reds per check, which is what turns keep-or-demote into arithmetic: rent 0 and false reds 6 is a demotion, rent 3 and false reds 0 is a keep. The report measures each cited check's runs and PR share beside those counts, so 6 blocks out of 38 runs and 6 out of 4,000 do not read alike; you never type those two numbers, because no session can count them. A false-red citation is spent after 8 weeks and deleted at the same audit.

A guard that ships with a grandfathered baseline owes a paydown PR — a separate one, opened with it. A ratchet's grandfathered counts are debt the guard took on, not a fact about the tree: the check blocks new growth and is satisfied forever while every pre-existing site stays put, so a lint can run 10,000 times, stay green, and never once cause a violation to be fixed. Promoting a guard that lands with a non-empty baseline therefore obliges a second PR that reduces that baseline — separate because the two diffs want opposite review: the guard is a few lines read closely, the paydown a wide mechanical sweep skimmed for shape. It need not empty the baseline; it must strictly shrink it and state the count before and after, plus what remains and why. A baseline that cannot be reduced at all is evidence the guard is mis-specified — it is flagging an idiom the tree deliberately uses, and the honest response is to narrow the detector or drop it.

Every proposal names its HOME — this repo, the merge workflow, or ci-truth-serum

The ## Proposed guards entry carries a Home: line: glovebox, auto-resolve or ci-truth-serum. Decide it in the proposal, because the home changes the arithmetic. A CTS hook runs on every consumer repo, so its benefit multiplies and one false positive costs far more than a local check.

auto-resolve is the home for a check only a MACHINE-authored merge can fail. The workflow's post-merge-check-command input runs one command on the merged tree before the push. A non-zero exit refuses the resolution instead of pushing it. The check runs only when the resolver runs, so it charges no per-commit tax. A refusal leaves the conflict standing until a human resolves it. A false refusal stops every conflicted PR at once. Pick it when the defect needs the WHOLE tree read at once — a type check, an import check, a build. A merge that keeps both parents' definition of one name raises no conflict, so every per-path check passes it. .github/workflows/auto-resolve-conflicts.yaml names the command. A defect a per-commit lint catches earlier still belongs in glovebox.

ci-truth-serum is a pack of offline pre-commit lints against CI that reports a FALSE RESULT. This repo consumes it at the commit config/ci-truth-serum-version pins. It reads workflows, composite actions, Dockerfiles and shell — files only, with no network and no GitHub API. Its hook groups:

  • Honesty (Tier 1, default-on) — the pipeline says success while the real work failed: check-workflow-pipefail, check-exit-suppression, check-stderr-suppression, check-pipefail-grep-pipe, check-frozen-head-sha, check-soft-timeout.
  • Identity (Tier 1) — a base image, a download or an install names a MUTABLE target, so the bytes that run are not the bytes reviewed: check-pinned-base-images, check-pinned-downloads, check-versionless-install, check-provenance-repo-url.
  • Security (Tier 1)check-trusted-base, check-untrusted-exec, check-unscoped-tool-grant.
  • Opinionated (Tier 2, opt-in) — a required check that never reports, or one a concurrency group strands: check-always-reporter, check-required-reporter, check-required-event-closure, check-static-concurrency, check-cancellable-required-check, check-job-timeout.
  • Extras — line lints with no CI theme: check-drift-guards, check-historical-comments, check-claude-model, check-cron-alert-coverage.

That repo's README lists every hook with the failure it prevents.

Read the hook's SOURCE before you claim what it covers — pre-commit usually already has the pinned pack on disk. Check ~/.cache/pre-commit/repo*/ci_truth_serum/ first; git rev-parse HEAD in a hit directory confirms the commit against config/ci-truth-serum-version. .claude/hooks/install-toolchain.bash's _install_precommit_toolchain clones it detached and best-effort, so a cold session, or a prewarm whose retries failed, can leave the cache without it — an empty search there is not proof the pack is unreachable. Wait for the prewarm rather than firing your own pre-commit install-hooks: outside the prewarm's HOME-bypass that clone takes the web proxy's repo-scoping 403 and hangs (.hooks/lib-prewarm.sh owns that wait). .pre-commit-config.yaml's pin names the commit either way. A proposal that says a hook's shape is unknown because the pack "is not in this checkout" is usually one ls short of the answer.

The home is ci-truth-serum when all three hold:

  1. The subject is a workflow, an action, a Dockerfile or a shell script, and reading the file answers the question.
  2. The defect is a fact about GitHub Actions or supply-chain identity, so ANY repo with that shape is wrong. "A draft-skipping workflow omits ready_for_review from its trigger types" is such a fact.
  3. Stating the rule needs no glovebox path, label, workflow name or config file. "A bot may not arm auto-merge before a human consents" fails this, because that consent model is this repo's policy.

Fail any one and the home is glovebox, or auto-resolve when the check reads a whole merged tree. When a CTS hook already covers the class but misses this arm, the proposal is to WIDEN that hook, and the Check: line names it — never a sibling hook beside it.

A CTS lint that asks a STRUCTURAL question about shell, Python, JS or comments uses that repo's grammar helpers (_cts_bash_ast, _cts_py_ast, _cts_js_ast, _cts_comments), never a regex. Its shell-lint-parsing rule owns that, so a proposal reaching for a text scan says why the input has no grammar to parse.

Write the lint — only in a human-promoted implementation PR

A guard homed here is a repo-local pre-commit hook: a .github/scripts/checks/*.py line-lint (share the _linecheck.py harness — expose violations(text) -> list[int], register under repo: local in .pre-commit-config.yaml, scope with files:/types:, and give a # allow-<slug>: <reason> opt-out). One homed in ci-truth-serum is a hook and its tests in that repo, plus a pin bump here. Write the lint only after a human has promoted the proposal, in its own implementation PR — in the fix PR itself, the deliverable is the proposal entry, not the lint.

Two hard gates on doing it well:

  1. Dogfood against the real tree before committing — a lint that fires on hundreds of legitimate existing call sites is flagging an idiom, not a defect, so narrow the scope (subcommand/context/directory) until the only hits are genuine, then bring each into compliance (bound it, or annotate with a reason).
  2. Scope to where the failure actually bites — the same construct can be a bug in runtime tooling and a non-issue under a CI job's timeout-minutes backstop, so a directory/files: scope is often what turns a noisy check into a precise one.

If after honest dogfooding the class can't be separated from legitimate use with acceptable false positives, say so and DON'T ship the lint — a hollow or noisy guard gets disabled and teaches nothing.

A guard that lands in CI machinery — a workflow, a landing or queue script, the scheduler — answers the TLA+ spec question in its PR body; .claude/rules/tla-plus.md owns it.

Every guard needs a WATCHED SURFACE

Before calling a check done, ask "when this goes red, WHO SEES IT?" A required PR check blocks a merge and gets read; a schedule, workflow_dispatch, or post-merge run has no PR surface at all — its red lands in an Actions tab nobody opens, so the check is decorative. Route the failure to a human (./.github/actions/notify-ntfy), or say why nobody needs to know. Ask it of the fix you just shipped too, not only of new checks.

Then ask it about DURATION: "if this stayed broken for a month, what would tell me?" Nothing in this tree watches for a scheduled workflow that quietly stops producing successful runs, so a cron whose failures notify nobody is silent for exactly as long as it stays broken.

The strictly worse twin is the vacuous green: a check that fails to fetch its input, degrades to a placeholder, and reports success — no surface saves that, so pair the question with "what does this do when its input is missing?" and make the answer fail closed.

Two honesty mechanisms exist — extend, don't rebuild

CI exit-code honesty: the check-workflow-pipefail hook bans a meaningful command piped into tee/a filter without set -o pipefail (the PR #899 fix — don't add a parallel CI-honesty lint). Layer-engagement honesty: config/trace-events.json is the SSOT, and each required: true event is a STARTUP-deterministic engagement the self-test asserts fired on every launch. A new startup-deterministic defense layer ⇒ add its event with required: true AND a producer that emits it; don't invent a separate check. required is reserved for events that fire every launch regardless of agent activity, so the self-test never flakes.

Mutation testing: CI only

Mutation testing proves new tests assert behaviour rather than merely execute lines (docs/mutation-testing.md) — but run it on CI, never locally. The per-PR gate (.github/workflows/mutation-testing.yaml, decide-gated on changed .mjs/Python) is authoritative. Don't kick off Stryker/cosmic-ray locally: a full run takes minutes, and cosmic-ray mutates the source in place, breaking this sandbox's PostToolUse redaction hook (it shells out to the file being mutated) and corrupting your tree if interrupted. Instead, for a PR adding substantial testable code, write tests to kill mutants up front (exact-equality assertions, a case per boundary/comparison/arithmetic step) then let the CI gate verify; fold killing tests (or justified # pragma: no mutate / Stryker-disable) into a follow-up commit if CI reports survivors.

cosmic-ray exec mutates in place (reverting after each mutant), so never run it in the background against a file you're about to git add/commit — the rewrite races lint-staged and the commit, corrupting staged content and aborting the push (the file lands back clean, but the commit is mangled). Let the run finish first, or point it at a throwaway copy. Stryker reads inputs without rewriting, so the JS path is race-free.

The bash lane is the carve-out. tools/mutation/bash_mutation_run.py scores each mutant against a private git snapshot rather than your checkout, so a mutated rm cannot reach the tree and no rewrite races a commit. One module at a time is safe locally: uv run python tools/mutation/bash_mutation_run.py tools/mutation/bash-<module>.toml.

More generally, super-heavy suites — Stryker, cosmic-ray, full pytest tests/, kcov, coverage gates — run on CI, not in this sandbox.

Signals

GitHub stars
63
Forks
11
Last commit
Sep 2026
Advanced
Catalog kind
skill
Gateway key
defect-to-guard
Source
github.com/alexandermattturner/agent-glovebox