Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .claude-plugin/marketplace.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"$schema": "https://json.schemastore.org/claude-code-marketplace.json",
"name": "clean-code-toolkit",
"version": "3.5.2",
"version": "3.6.0",
"description": "Clean-code and product-handoff tools for AI-assisted builders.",
"owner": {
"name": "Tarik Moody"
Expand All @@ -10,7 +10,7 @@
{
"name": "clean-code-toolkit",
"description": "Review code, assess product readiness, refactor safely, and prepare a developer handoff.",
"version": "3.5.2",
"version": "3.6.0",
"author": {
"name": "Tarik Moody"
},
Expand Down
2 changes: 1 addition & 1 deletion .claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "clean-code-toolkit",
"version": "3.5.2",
"version": "3.6.0",
"description": "Practical clean-code, product-readiness, and developer-handoff workflows for AI-assisted projects.",
"author": {
"name": "Tarik Moody"
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/validate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,8 @@ jobs:
- name: Run prod-readiness-coach tests
run: python -m unittest discover skills/prod-readiness-coach/tests

- name: Coverage grid must stay full
run: python skills/prod-readiness-coach/scripts/coverage_grid.py --fail-under 100

- name: Coach audits this repository
run: python skills/prod-readiness-coach/scripts/prod_audit.py --repo . --profile library --fail-on critical --output /dev/null
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,23 @@
# Changelog

## 3.6.0

Three review rounds in a row each found new defects, and the question was asked plainly: is there an end to this. There was no way to answer, because nobody could say how much had been checked. This release makes that a number.

**The coverage grid.** Every check must now have a fixture that makes it fire and a fixture that makes it stay quiet. Checks a profile can skip must also have a fixture that skips them. That is 83 cells across 34 checks, and it stands at 83 of 83. `scripts/coverage_grid.py` prints it, and CI fails if it drops below 100 percent. The grid is computed by running the audit over the fixtures, so it cannot drift from the code: add a check with no fixture and the build fails with that check's id in the message.

Before this, all 68 tests asked only whether a check fires. None asked whether it stays quiet. That is the exact hole `auth-2` fell through, wrong about half the time it spoke, for three releases, with a green suite the whole way.

**Two false negatives the grid found in its first hour, both invisible to three rounds of human review.**
- **Sentry installed as `@sentry/nextjs` did not count as error tracking.** The dependency matcher knew `sentry` and `@sentry` but not the scope prefix, so a Next.js app with Sentry correctly wired up was told at CRITICAL that it had no error tracking at all. That is the most common error-tracker and framework pairing this tool audits. `@vercel/otel`, `@sentry/node` and every other scoped package were equally invisible. Type stubs like `@types/pino` still correctly do not count.
- **A plain `migrations/` folder was never read for destructive SQL.** The irreversible-migration check knew Prisma, Drizzle, Alembic and Rails, and nothing else. A `DROP TABLE` in the folder that dbmate, golang-migrate, node-pg-migrate, sqlx and Supabase all use went unreported.

**Five invariants, each with a test.** Each closes a whole class of defect so it cannot come back one instance at a time: never report about a file that is not there; never claim a control exists from a text match without saying the evidence is weak; never grade what it did not scan; never ship text its own linter rejects; every check has a firing case and a quiet case. Two more properties are now enforced across every fixture: a failing check always carries a recommendation, and no finding text uses an em or en dash.

**The stop rule.** The engine is done when the grid is full, the invariants have tests, and CI is green. After that a review hunts for a new class of defect, not a new instance of an old one, and a review that finds nothing counts as a pass. See `docs/decisions/009`.

Tests: 114.

## 3.5.2

A developer-experience audit of the toolkit itself: install it fresh, run the CLI, break it on purpose, read every doc link. Four things it found.
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ Repository layout:

The root `CLAUDE.md` holds contributor instructions for this repository. User-facing behavior lives in the skills, commands, and the installable template. Maintainers follow the [release checklist](docs/release-checklist.md) before tagging.

The audit engine keeps a coverage grid: every check has to prove it fires when a control is missing and stays quiet when the control is there. It sits at 83 of 83 and CI fails if it slips. Run `python3 skills/prod-readiness-coach/scripts/coverage_grid.py` to see it.

Design principles, in one line each: product intent first; read before editing; evidence over confidence; the framework's conventions beat generic advice; line counts are prompts to look, not failures; small focused diffs; unknowns stay unknown. [How It Works](docs/how-it-works.md) describes each tool's boundaries. [Start Here for Vibe Coders](docs/start-here-vibe-coders.md) is the longer plain-English guide.

## Scope
Expand Down
23 changes: 23 additions & 0 deletions docs/decisions/009-coverage-grid-and-stop-rule.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# 009: A coverage grid, and a rule for when to stop reviewing

**Decision.** Every check must have a fixture that makes it fire and a fixture that makes it stay quiet. Checks a profile can skip must also have a fixture that skips them. The count is computed by running the audit over the fixtures, printed by `scripts/coverage_grid.py`, and CI fails if it drops below 100 percent. Once the grid is full and the invariants have tests, only a real user's bug report changes the engine.

**Why this came up.** Three review rounds in a row each found new defects, and the question was asked plainly: is there any end to this. There was no way to answer, because nobody could say how much had been checked. Sixty-eight tests existed and every one asked only whether a check fires. None asked whether it stays quiet when it should. That is the exact hole `auth-2` fell through: it was wrong about half the time it spoke, for three releases, with a full green suite. Without a denominator, every finding looks like proof of infinite findings, and the honest answer to "are we done" is "nobody can tell".

**Options.**
1. Keep reviewing. Each round finds real things, and the severity has been dropping. Cost: no finish line, and no way to distinguish a review that found nothing because the code is good from one that found nothing because the reviewer looked in the wrong place.
2. Rules as data. Move every check into a YAML or Rego file carrying its own applicability and its own fixtures, the way Semgrep, KICS and Trivy do. Cost: those projects did that at thousands of rules. This engine has thirty-four. The machinery would cost more than the checks.
3. Keep the checks as Python. Add fixtures and count the grid mechanically. Borrow the contract from KICS, which refuses a query that ships without both a vulnerable and a safe fixture, and from Checkov, which requires both a PASSED and a FAILED test case. Cost: fixtures are work, and a fixture that is wrong teaches the wrong lesson confidently.

**What we chose and why.** Option 3. Tarik's call, after research into how OpenSSF Scorecard, Semgrep, Checkov, Trivy, KICS, SonarQube and Datree each handle it. The deciding detail is that the grid is derived, not maintained. Nobody writes down which checks have which coverage. The number comes from running the audit over the fixtures, so it cannot drift from the code, and adding a check with no fixture fails the build with that check's id in the message.

It paid immediately. Demanding a passing fixture for every check surfaced two false negatives that three rounds of human review had missed: a Sentry package installed as `@sentry/nextjs` was invisible to the error-tracking check, so a correctly instrumented Next.js app was told at CRITICAL that it had no error tracking, and a plain `migrations/` folder was never read for destructive SQL, so a `DROP TABLE` went unreported outside four specific ORMs.

**What we gave up.** Fixtures are a maintenance surface of their own. A fixture that misrepresents a real repository will make a check look covered when it is not, and nothing in the grid can detect that. The count says a behaviour is exercised; it does not say the behaviour is right.

**The stop rule.** The engine is done when the grid is at 100 percent, the five invariants have tests, and CI is green. After that, a review looks for a new class of defect, not a new instance of an old one, and a review that finds nothing counts as a pass. Only a bug report from a real user reopens the engine.

**How we'll know if this was right.** The next review round finds either nothing or something in a class nobody had named before. If it finds another instance of a class already on the invariant list, the invariant was written too narrowly and that is the thing to fix.

**What actually happened.**
(Tarik fills this in.)
26 changes: 24 additions & 2 deletions skills/prod-readiness-coach/scripts/audit/checks_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,25 @@ def check_structured_logging(repo: Repo) -> list[CheckResult]:
# A dependency named @types/pino is a type stub, not a logger. Match the package
# name (allowing a scope prefix), never a substring of a serialized blob.
def _installed(lib: str) -> bool:
"""True when a dependency is this library, under any packaging style.

Sentry ships as `@sentry/nextjs`, not `sentry`, and the scope prefix was
matched by nothing here: a Next.js app with Sentry correctly installed was
told at CRITICAL that it had no error tracking at all. That is the most
common error-tracker and framework pairing this tool audits.
"""
lib = lib.lower()
return any(n == lib or (n.endswith(f"/{lib}") and not n.startswith("@types/"))
for n in dep_names)
# A type stub is a description of a library, not the library.
real = [n for n in dep_names if not n.startswith("@types/")]
return any(
n == lib
# `@foo/pino` is pino.
or n.endswith(f"/{lib}")
# `@sentry` is `@sentry/nextjs`, `@sentry/node`, `@sentry/react`.
or (lib.startswith("@") and n.startswith(f"{lib}/"))
# `sentry` is also `@sentry/nextjs`; `otel` is also `@vercel/otel`.
or (n.startswith("@") and "/" in n and n.split("/", 1)[1] == lib)
for n in real)

matched_logging_libs = [lib for lib in LOGGING_LIBS if _installed(lib)]
if matched_logging_libs:
Expand Down Expand Up @@ -498,6 +514,12 @@ def check_multi_surface_deployment(repo: Repo, fp: StackFingerprint) -> list[Che
"drizzle/*.sql", "**/drizzle/*.sql", "drizzle/**/*.sql", "**/drizzle/**/*.sql",
"alembic/versions/*.py", "**/alembic/versions/*.py",
"db/migrate/*.rb", "**/db/migrate/*.rb",
# A plain migrations folder is what dbmate, golang-migrate, node-pg-migrate,
# sqlx, Supabase and hand-rolled setups all use. Knowing only the four ORMs
# above meant a `DROP TABLE` in `migrations/` was never read.
"migrations/*.sql", "**/migrations/*.sql", "migrations/**/*.sql",
"**/migrations/**/*.sql", "db/migrations/*.sql", "supabase/migrations/*.sql",
"migrations/*.js", "migrations/*.ts", "**/migrations/*.js", "**/migrations/*.ts",
])
destructive_hits = []
for f in migration_paths:
Expand Down
70 changes: 70 additions & 0 deletions skills/prod-readiness-coach/scripts/coverage_grid.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
#!/usr/bin/env python3
"""Print the coverage grid, so 'is there an end to this' has a number.

Every check must prove two things: that it fires when the control is missing,
and that it stays quiet when the control is there. Checks that a profile can
skip must also have a fixture that skips them. This counts those cells.

Run: python3 skills/prod-readiness-coach/scripts/coverage_grid.py
python3 skills/prod-readiness-coach/scripts/coverage_grid.py --fail-under 100
"""
import argparse
import sys
from pathlib import Path

HERE = Path(__file__).resolve().parent
sys.path.insert(0, str(HERE))
sys.path.insert(0, str(HERE.parent / "tests"))

import prod_audit # noqa: E402
from test_coverage_grid import ALL_CHECK_IDS, FIXTURES, FIRING, build_grid # noqa: E402

MARK = {True: " yes", False: " NO"}


def main() -> int:
parser = argparse.ArgumentParser(description=__doc__.splitlines()[0])
parser.add_argument("--fail-under", type=float, default=None,
help="Exit 1 if the filled percentage is below this number.")
parser.add_argument("--quiet", action="store_true", help="Print the summary only.")
args = parser.parse_args()

grid = build_grid()
skippable = set(prod_audit.CHECK_SKIPS_BY_PROFILE["library"])

filled = required = 0
rows = []
for cid in ALL_CHECK_IDS:
seen = grid.get(cid, set())
fires, quiet = bool(seen & FIRING), "pass" in seen
needs_na = cid in skippable
has_na = "n/a" in seen
required += 2 + (1 if needs_na else 0)
filled += int(fires) + int(quiet) + (int(has_na) if needs_na else 0)
rows.append((cid, fires, quiet, needs_na, has_na))

pct = 100.0 * filled / required if required else 100.0
if not args.quiet:
print(f"Coverage grid, {len(FIXTURES)} fixtures over {len(ALL_CHECK_IDS)} checks\n")
print(f"{'check':10}{'fires':>7}{'quiet':>7}{'skips':>7}")
print("-" * 31)
for cid, fires, quiet, needs_na, has_na in rows:
skip = MARK[has_na] if needs_na else " ."
print(f"{cid:10}{MARK[fires]:>7}{MARK[quiet]:>7}{skip:>7}")
print("-" * 31)

gaps = [cid for cid, fires, quiet, needs_na, has_na in rows
if not fires or not quiet or (needs_na and not has_na)]
print(f"\n{filled} of {required} cells filled ({pct:.0f}%). {len(gaps)} check(s) with a gap.")
if gaps:
print("Gaps: " + ", ".join(gaps))
print("A gap means no fixture proves that behaviour. Add one to tests/fixtures.py.")

if args.fail_under is not None and pct < args.fail_under:
print(f"\nBelow the floor of {args.fail_under}%.", file=sys.stderr)
return 1
return 0


if __name__ == "__main__":
sys.exit(main())
Loading
Loading