test(1788): house-rules.sh gets a suite, a mutation tier, and an adversarial pass

[LukasWodka]

Closes backend#1788.

852 lines of quote-aware, heredoc-aware, command-position-aware shell lexing, with quality / house-rules a required status check on develop, staging and prod across 16 repos — and no test of any kind. A false positive here doesn't make noise; it blocks every merge in the org, including the release train's own promotion PRs, so the fix would have to travel through the pipeline it just blocked.

The adversarial pass — the ticket's central question

#1788's argument is that this checker matches arbitrary source content, including comments that discuss the patterns being matched. Its evidence was a five-pattern matcher over 588 real PR titles producing a false positive on a domain term — and that was on titles, the most uniform text we have.

Run over 103 real shell files in five repos, including release-train's 19 prose-dense scripts that discuss set -o pipefail and SIGPIPE at length, and client's 44 installers with real curl/helm:

release-train 19 -> 0 findings client 45 -> 0 findings
e2e-test-agent 25 -> 0 findings cli 11 -> 0 findings
client-runtime 3 -> 0 findings

And a control, because "zero findings" and "the matcher stopped working" print the same thing. A violation injected into that same corpus fires all four rules. Both are cases; the control is what makes the clean result mean anything.

41 cases

Four rules × fires / silent — neither half alone is worth having: assert only fires and a matcher that flags everything passes; assert only silent and one that flags nothing does. Plus the lexer's six documented precision promises (each a reason a rule does not fire, and therefore a way it could start crying wolf), all three pragma forms, both repo-wide stand-downs, six config directives, the exit contract including --soft-fail, and --base over a diff with a deleted file.

Every case pins the rule ID, not just a non-zero exit — four rules share an exit code.

A mutation tier, because .github had none

It applies each mutation to the real script and runs the real suite — no re-implemented rule inline, which reads as satisfied while breaking the real thing reddens nothing. An anchor matching other than exactly once, or changing no text, is STALE and fails the run.

It earned its keep on the first run. Two mutations passed the entire suite:

mutation	why the suite couldn't see it
--tlsv1\.[23] → --tlsv1	every case used a compliant flag, so the version floor — the actual rule — was never exercised
lmask → lraw	re-introduces a documented, already-fixed bug where set -o pipefail inside a string marks a whole file safe. Nothing pinned the fix

Both are cases now. The second took three attempts: echo "set -o pipefail" does not reproduce it, because the detector anchors on (^|[[:space:];])set and there set follows a quote. I applied the mutation and enumerated which forms do. A case written for a bug it cannot construct is worth less than no case, because it reads like coverage.

Two near-misses, recorded because both were one commit from being wrong

• The flag-var case failed and I read it for a minute as "the documented feature is inert". It isn't — the table is built with git grep, so a /tmp fixture can never reach it. The harness now runs inside a throwaway git repo with fixtures git added.
• Three fixtures appeared to show a live fail-open in the pipefail detector. Re-run on the pristine script, all three fire correctly; the "finding" was an artefact of a badly-escaped sed that had corrupted the awk program. Verifying on a clean file is the only reason it didn't become a wrong ticket.

One behaviour pinned rather than fixed

A named path that doesn't exist reports no shell files to check and exits 0. That looks like the fail-open this ticket is about, and it is not reachable from CI — code-quality.yml only ever builds --all or --base "$BASE_SHA" (:755-758). Asserted as current behaviour with the reason, per the rule that a fix for an unreachable path costs more than filing nothing. The reachable neighbour — --base over a diff that deleted a shell file — is asserted to still check the survivors.

Wiring

selftest-house-rules joins the already-requiredselftests context, and the mutation run gets its own step there rather than a new context that would sit unrequired until someone armed it. make check resolves anchors only (~ms); the full ~58s run is in check-all and CI. The numbers are in the Makefile next to the choice.

And the coverage guard learned a second family.selftests-cover rejected the mutation runner for not matching *-selftest.{py,sh} — correctly, since a file no wildcard sees makes its assertion pass vacuously. Renaming it would have been a lie that also nested the 58s tier inside the 3s one, so *-mutations.py is its own family with its own coverage assertion.

Item 4 of the ticket, not done here

The ticket asks whether house-rules needs to be required on staging/prod at all, since the content was already linted into develop. That's a branch-protection change across 16 repos × 2 roles and a judgement call, not a test — written up as a comment on #1788 rather than actioned unilaterally.

make check exit 0 (captured, not piped). 41 cases, 5 mutations, 0 stale, 0 uncaught.

---

Note

Low Risk
Changes add and wire tests and CI/Makefile targets only; they do not modify house-rules.sh matcher behavior or production gate logic.

Overview
Adds test coverage for house-rules.sh, a required org-wide gate that previously had no tests.

New suites:house-rules-selftest.sh (~41 cases) exercises the four rules (fires/silent pairs), lexer precision, pragmas, repo-wide stand-downs, config directives, exit/--soft-fail/--base behavior, and an adversarial --all pass over this repo’s tracked scripts with a staged violation control. Cases assert exact rule IDs, not just non-zero exit.

Mutation tier:house-rules-mutations.py mutates the real script and re-runs the real suite; stale anchors and uncaught breaks fail the run. Full mutations (~58s) run in CI (selftests.yml) and make check-all; mutation-house-rules-dry (~ms) is on make lint for fast anchor checks.

Makefile / coverage:selftest-house-rules joins SELFTEST_TARGETS; selftests-cover also tracks *-mutations.py via MUTATION_FILES / MUTATION_TARGETS so unwired mutation runners cannot pass vacuously.

^{Reviewed by Cursor Bugbot for commit 400b9e0. Bugbot is set up for automated code reviews on this repo. Configure here.}

[LukasWodka]

bugbot run

[LukasWodka]

bugbot run

[LukasWodka]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 400b9e0. Configure here.}

[saadqbal]

Ran both tiers rather than reading the tables. Selftest 41 passed, 0 failed; mutation tier 7 mutations, 0 stale, 0 uncaught — including the two the description says initially sailed through the whole suite:

caught curl-tls accepts ANY --tlsv1, so the version floor is gone
by: curl-tls still fires on a DOWNGRADED --tlsv1.0, curl-tls still fires on --tlsv1.1
caught pipefail reads the RAW line, so a quoted string marks a file safe
by: a quoted "set -o pipefail" does NOT mark the file safe, ...nor does one in a variable assignment

Those two are the whole argument for the mutation tier existing. "Every case used a compliant flag, so the version floor was never exercised" is the classic shape — full green on a rule whose actual content nothing touched. And the note that echo "set -o pipefail" does not reproduce the lraw bug, because the detector anchors on (^|[[:space:];])set and there set follows a quote, is the detail that makes the second case real: a case written for a bug it cannot construct is worse than none, because it reads as coverage. Worth the three attempts.

Applying mutations to the real script and running the real suite — no inline re-implementation — is the right construction, and treating an anchor that matches other than exactly once as stale is what stops the tier rotting into a no-op after a refactor. That's the failure mode a mutation harness normally acquires within a year.

Also good, and both apparently Bugbot-driven:

• the trap cleaning the checkout and not just $WORK. .hr-control.sh is itself a house-rules hit, so an interrupt between the git add and cleanup would leave it staged and the next make check would fail pointing at the wrong thing.
• here-strings over printf … | grep -q. grep -q exiting on first match SIGPIPEs the writer, pipefail turns that into 141, and the negated form reads 141 as "absent" — so a disable: directive that did nothing would have recorded a PASS. That's a test harness that lies in the safe-looking direction, which is the worst kind.

Two notes, neither blocking:

1. The headline corpus number isn't what CI reproduces. The description leads with 103 real shell files across five repos; the standing check is --all over this repo's tracked files, which reported 5 in my run, plus the injected control. Both are worth having — the 103-file sweep is the evidence for #1788's central question, the 5-file pass is the regression guard — but they read as one thing. Might be worth a line in the suite saying the cross-repo sweep was one-time evidence, so nobody later assumes CI is still doing it.
2. Pinning the named-missing-path behaviour (no shell files to check, rc 0) as current-and-CI-unreachable rather than fixing it is the right call, and I'm glad the reachability argument is written next to it (code-quality.yml:755-758 only ever builds --all or --base). That reasoning is the thing that would go stale if the workflow changed, so it being cited by line is what makes it checkable later.

Good PR. 852 lines of quote-aware lexing gating every merge in the org with no test of any kind was a real exposure.

[LukasWodka]

/fr-pass