151 lines
6.0 KiB
Markdown
151 lines
6.0 KiB
Markdown
# Guards — how to write a check that actually checks
|
|
|
|
```
|
|
Status: Current
|
|
Owner: _null
|
|
Last reviewed: 2026-08-18
|
|
Governs: scripts/verify.d/**, .githooks/** — structural tests, source-grep
|
|
assertions, probes, and any check whose passing is taken as evidence
|
|
Review trigger: A guard is found to have been passing while the thing it guards
|
|
was broken; a new class of check is added to the suite.
|
|
```
|
|
|
|
> **prove-guard.sh is not in this repository.** It lives in the template at
|
|
> `~/.openclaw/Projects/Template/docs/architecture/scripts/`, and this project
|
|
> declined it on adoption — its guards are three shell scripts in
|
|
> `scripts/verify.d/` that fail visibly on their own. It is named without
|
|
> backticks throughout for that reason. §1 below still applies and was performed
|
|
> by hand on every guard here; `docs/history/DEVELOPMENT_LOG.md` for 2026-08-18
|
|
> records how each was broken and what it did.
|
|
|
|
A guard that cannot fail is worse than no guard, because it is trusted. Every
|
|
rule here was learned by finding one that had been green for months over
|
|
something broken.
|
|
|
|
## 1. Prove the guard fails before you believe it passes
|
|
|
|
The one discipline that matters most, and it takes thirty seconds:
|
|
|
|
```bash
|
|
cp src/lib/thing.ts /tmp/thing.bak
|
|
# break exactly the thing the test protects
|
|
sed -i 's/if (body.error)/if (false)/' src/lib/thing.ts
|
|
npx vitest run tests/thing.test.ts # expect: exactly one failure
|
|
cp /tmp/thing.bak src/lib/thing.ts
|
|
npx vitest run tests/thing.test.ts # expect: green again
|
|
```
|
|
|
|
**Exactly one** is the part people skip. If breaking the guard's target fails
|
|
three tests, two of them are coincidental and will mask a real regression later.
|
|
If it fails none, the guard is decoration — and you have just learned that for
|
|
the price of one `sed`.
|
|
|
|
prove-guard.sh performs exactly this, which removes the two ways it
|
|
gets skipped: the restore is a `trap`, so an interrupted run cannot leave the
|
|
code broken, and the failure count comes from the runner's own summary rather
|
|
than from eyeballing red — one failing test is routinely reported on half a
|
|
dozen lines, and counting those calls a clean result six coincidental
|
|
failures.
|
|
|
|
Do this when you write a guard, and again when you change what it guards. A
|
|
test written alongside the code it tests has never been observed failing.
|
|
|
|
## 2. A source-grep guard must tell code from the comment about code
|
|
|
|
Structural tests that assert a file does *not* contain some pattern will match
|
|
the docblock explaining why that pattern is forbidden. So the clearest possible
|
|
comment breaks the test, and the obvious fix is to delete the explanation.
|
|
|
|
Strip comments first:
|
|
|
|
```ts
|
|
const codeOf = (path: string) =>
|
|
readFileSync(path, "utf8")
|
|
.split("\n")
|
|
.filter((line) => !/^\s*(\*|\/\/|\{\/\*)/.test(line))
|
|
.join("\n");
|
|
|
|
expect(codeOf("src/lib/thing.ts")).not.toContain("dangerouslySetInnerHTML");
|
|
```
|
|
|
|
Otherwise the guard quietly punishes documenting the rule it exists to enforce —
|
|
which is exactly backwards, because the comment is how the next person learns
|
|
the rule at all.
|
|
|
|
## 3. Pin the behaviour, not the spelling
|
|
|
|
A guard should fail when the protected behaviour breaks and stay quiet
|
|
otherwise. One that asserts on a variable name fails on a rename that changed
|
|
nothing.
|
|
|
|
```ts
|
|
// Brittle: breaks when the variable is renamed, while the fallback it protects
|
|
// is untouched.
|
|
expect(route).toContain("readAsset(project.forgejoRepo");
|
|
|
|
// Pins the behaviour: the route fetches through the wrapper that tries both
|
|
// spellings, and never through the raw reader.
|
|
expect(route).toMatch(/readAsset\(\s*\w+,\s*ASSETS\[which\]\s*\)/);
|
|
expect(body).not.toContain("readFileBytes(");
|
|
```
|
|
|
|
A guard that fails on changes it does not care about is one people learn to edit
|
|
rather than heed, and the edit is usually deletion.
|
|
|
|
## 4. A negative result is only as good as the probe that produced it
|
|
|
|
"The check found nothing" and "the check did not run" are different facts, and
|
|
they look identical from the outside. Before reporting an absence, prove the
|
|
instrument worked:
|
|
|
|
```bash
|
|
# Not this alone — an unreadable file produces the same silence as an unset key
|
|
grep -c '^WANTED=' /proc/$PID/environ
|
|
|
|
# Establish the read succeeded first
|
|
tr '\0' '\n' < /proc/$PID/environ | grep -c . # 0 here means "could not read"
|
|
```
|
|
|
|
This is the confident-absence failure one level up: the same trap as a screen
|
|
rendering a failed query as a count of zero, applied to your own diagnosis.
|
|
|
|
## 5. A guard that is often wrong is worse than none
|
|
|
|
A check with a high false-positive rate trains everybody to skip its output,
|
|
including on the day it is right.
|
|
|
|
One written for this template flagged **684 of 1142** candidates on its first
|
|
run. That was not 684 findings, it was a broken heuristic — and shipping it
|
|
would have taught its readers that the check is noise. Two rounds of narrowing
|
|
brought it to 17 of 363, all of them real.
|
|
|
|
If a new guard's first run is loud, tune it until it is quiet before anybody
|
|
relies on it. Report the false-positive rate you settled at, so the next person
|
|
knows what silence is worth.
|
|
|
|
## 6. Guards belong before the artifact exists
|
|
|
|
A check that runs after publication catches the problem once it is somewhere it
|
|
cannot be taken back from: the tag is in the registry, and refusing the commit
|
|
afterwards leaves git with no record of it.
|
|
|
|
Order the gates so the expensive, irreversible step is last — preconditions,
|
|
guards, build, verify the built thing is what was asked for, publish, and record
|
|
it last of all.
|
|
|
|
## 7. When the gate finds something that invalidates the operation, stop
|
|
|
|
Printing a warning and continuing produces the worst outcome available: the bad
|
|
thing happens *and* a reassuring summary appears above it.
|
|
|
|
The question is not how bad the finding is. It is **whether it invalidates what
|
|
the operation claims**:
|
|
|
|
- A release whose test gate skipped half the suite — a release claims to be
|
|
tested. **Refuse.**
|
|
- A backup written to a group-readable directory — the backup is still a
|
|
backup. **Warn.**
|
|
|
|
Escape hatches are fine, and they have to be asked for by name, never be the
|
|
default, and say plainly what is being given up.
|