# Code Quality > Load for any coding or development task in any repository: writing, changing, fixing, refactoring, or testing code or scripts in any language. - Skill: `vanillagreencom/code-quality-2` (Agent Skill, multi-file: 2 files) - Install (CLI): `npx skillmds@latest add vanillagreencom/code-quality-2` - Raw SKILL.md: https://api.skillmd.com/api/skills/vanillagreencom/code-quality-2/raw - Safety review: pending - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: Productivity - License: MIT - Author: vanillagreencom (https://skillmd.com/u/vanillagreencom) - Updated: 2026-09-22 - Page: https://skillmd.com/skills/vanillagreencom/code-quality-2 --- ## Project Instructions Problems with a kendex-owned skill go through `kendex report`; check ownership in the file first. # 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 ` 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](../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`](../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.