Code Quality

Code Quality

Repo-specific standards live in each repo's ## Project Instructions section and add to these rules.

Core Principle

A loud failure beats a silent wrong answer. Handle every error, check invariants, and never continue in a state the code does not understand.

Correctness

• No workarounds or quick hacks. If the correct fix is larger than expected, say so.
• Never fail open. A dependency failure (command, file, network, parse) must not leave the caller in a passing or default state: no validator degrading to "no findings", no probe failure read as "not applicable". An absent or unknown input is not a dependency failure: refusing on it disables a working path, which needs its own justification stated where the choice is made.
• Make illegal states unrepresentable: a state with several cases is one tagged value each site matches exhaustively, never independent flags each site conjoins. § Language Discipline holds each language's spelling.
• A gate, guard or scanner change adds no enumerated exemption list; a refusal is one rule at the point the code cannot judge.
• A branch that "shouldn't happen" is never an empty or silently-ignored else: assert it, return an explicit internal error, or mark it unreachable, with a message naming the violated invariant. Use plain conditionals only when both branches are expected paths.
• An error path must name the actual cause, not a neighbouring dependency.
• Handle edge cases: empty input, boundary values, junk prefixes/suffixes, interrupted-then-retried flows.

Prove Your Guards

A new or modified production gate or guard ships with one must-fail control per rule it enforces. Plant one defect that the rule catches and see it go red before its green counts. This per-rule control is inside the validation gate. Every other changed check, assertion, or test follows § Tests: one control per changed surface. The control that counts keeps the matched text and removes the behavior; one that deletes the code under test only proves the assertion runs. Reject assertions loose enough to match a skip note, fixtures that never reach the guarded bound, and harness code that keeps alive what the implementation should.

• A scripted text substitution asserts its match, or it is not an edit. Assert the pattern's occurrence count and that the file changed, or use an edit tool that errors on no match. Neither assertion holds on a symlink, which sed -i replaces with a new file while its target stands: resolve the path first, or refuse a symlink.
• A floor alone is not a control. Derive the expected set from the artifact under test (the flag's own regex, the function's own body), never from a second list in a test file. Floor it, with a message naming the extractor as broken rather than the subject as sparse. Under-inclusion needs the floor plus a required member; over-inclusion needs a forbidden member. State which direction stays open.

Instruments you did not write

• A check narrower than the claim can only confirm it, never establish it. Match the instrument's reach to the assertion's reach before running it, and prefer one that fails visibly on a planted counterexample. A grep over one directory supports no claim about the tree.
• Behaviour measured at an interactive prompt is not what scripts get. type <cmd> names the shadow, which differs per shell. Resolve the command in the script's own shell and PATH, and name the shell and implementation it resolves to.
• A guard's failure message is an instrument. It is what an author acts on. Unescaped backticks inside a double-quoted diagnostic execute their contents, so the intended text is altered or gone while the surrounding command still succeeds.

Tests

• One must-fail control per changed behavioral surface with a test is the whole mutation requirement. A workflow sentence has no test and adds no control. Do not run mutant batteries beyond the one control. The control plants one defect that turns the surface's test red; use the existing HOOK_UNDER_TEST and mutant-hook patterns as the shape. Each surface also tests its inverse. Where no edit to the production code reddens a surface's test, state that, why, and what the test holds, in place of its control; a production gate or guard keeps its per-rule control. A test that asserts the mechanism implementing a guarantee has no control, since planting the mechanism moves the test with it: assert the guarantee.
• N planted defects means N asserted rows. A fixture that plants several defects under one verdict passes while any one of them is caught, and is never allowed.
• Shaped input (positions, settings keys, tamper classes) is one table-driven case: one loop, one assertion per row, the row list visible in the file.
• Every hook or script refusal and notice starts with a stable first line: a short key and the relevant path, count, exit code or other value. Put the English explanation on following lines. Keep message text in one place per hook or script.
• Assert codes, enums and exit status. For refusals and notices, assert the key, value and exit status, never an English sentence. When a downstream consumer parses a text protocol, state that contract in the script header and pin the whole protocol. Change the message mechanism and its tests in the same package PR.
• A row pins the clause only its own guard emits: an expectation a neighbouring gate or the production helper on both sides can also produce is not a pin, a value read as a truthiness bit is not a pin, and a fold keeps every assertion of its former cases.
• No test of the test harness: a pin on a manifest script string or a runner configuration proves nothing about behaviour.
• A shared fixture is a neutral world (a seeded repository, a fake SDK). A fixture that carries a planted defect is private to its case.
• A test that spawns a real process passes the child's environment explicitly instead of inheriting the developer's live environment.
• One file per surface, beside the code, named for the surface.
• A file past about 64 KB or about 60 cases holds more than one surface. Split it at a surface seam and move cases whole: count occurrences per test name rather than keying by name, compare bodies in both directions, and re-target every document citing the file. The seam is the author's judgement; no check measures it.

Language Discipline

• Rust: exhaustive matches (no _ => over enums you own); enums over strings/sentinels/booleans-with-meaning. A test that hands a temporary path to code that may resolve symlinks binds its canonical root at creation and passes that binding, never the raw path; platform-only test APIs carry a cfg and, when the property is portable, a portable twin.
• Bash: check the result of every effectful substitution, in test position too; -- before path arguments sourced from configuration, argv, or the environment (not paths the script built itself, e.g. mktemp -d); no [A-Za-z]-class assumptions under arbitrary locales. The set -euo pipefail preamble, an unchecked or untrapped mktemp and a declaration masking a status are preflight's fail-open, mktemp-trap and masked-returns lanes.
• Bash scripts prove jq filters with jq 1.7.1; use f? | .r because f? .r does not parse there.
• In any pipefail script, never pipe a shell writer into an early-closing reader — head, grep -q, grep -m N — which stops reading while its producer still writes: the 141 SIGPIPE status aborts the run where errexit fires, and in condition position, where it does not, reads as a plain false that drops the result with no error. Capture whole and window in-shell, or give the reader a here-string. An added line of that shape is preflight's early-close-pipe lane. A must-fail control for one writes its input from the shell, never cat reading a file, which pushes several hundred KB before it blocks and passes a buffer-sized fixture either way.
• Measure a commit header with commit-guards' locale-stable gg_chars, never raw awk length or wc -c.
• TypeScript/JS: discriminated unions switched with a never default over strings and booleans-with-meaning; distinguish missing from present-but-falsy ("", 0) at every guard; no any at module boundaries. A store selector returns a stable reference: never mint an array, object or Set inside one (a fresh value re-renders forever and blanks the page). An effect that positions or measures rendered content takes what is drawn as its trigger, not a list of state names; the exhaustive-dependencies lint checks only what the effect reads.

Comments and Prose

Do:

• Document the constraint or invariant the code cannot show, not what the line does.
• Document public functions, structs, enums, and variants.

Don't:

• Comments that repeat the code.
• History: a temporal marker, a date, an issue id, a review round or a conversation. For an optional audit, see commit-guards CHECKS.md § comments.
• Claims broader than what the adjacent code or assertion actually enforces.
• A numeral counting things outside the sentence. State the property and the command that enumerates it. A numeral bound to something adjacent stays: a list in the same paragraph, a constant a check compares against, one a ratchet owns.

Markdown is ../docs-writing/SKILL.md: the writing standard, and what each file type holds and excludes.

Commit bodies explain intent, never narrate the diff.

Over-Engineering

Build only what was asked. No speculative abstractions, no error handling for impossible scenarios, no generalization before a third caller exists; a decision re-derived at N sites already has N callers. Delete wrappers that only forward. A new dependency needs a one-line justification in its commit message.

One judge per question: never re-implement a decision (classify, validate, parse, detect state) another component or language already owns; delegate. A decision re-derived at each use site in one file is the same defect: compute it once and let each site match on the result. A second spelling is a defect even when both copies agree. Package behavior lives in the package's shipped scripts; a host binary only locates, execs, and surfaces results.

Cleanup

Remove unused code completely: no backwards-compatibility shims, no renamed _vars, no commented-out blocks, no // removed markers, no re-exports without callers. Breaking removals get a CHANGELOG note, not a compat layer.