Access control, warnings that count, an installer that can update (3.5.0)

.claude-plugin/marketplace.json

@@ -1,7 +1,7 @@
 {
   "$schema": "https://json.schemastore.org/claude-code-marketplace.json",
   "name": "clean-code-toolkit",
-  "version": "3.4.0",
+  "version": "3.5.0",
   "description": "Clean-code and product-handoff tools for AI-assisted builders.",
   "owner": {
     "name": "Tarik Moody"
@@ -10,7 +10,7 @@
     {
       "name": "clean-code-toolkit",
       "description": "Review code, assess product readiness, refactor safely, and prepare a developer handoff.",
-      "version": "3.4.0",
+      "version": "3.5.0",
       "author": {
         "name": "Tarik Moody"
       },

.claude-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "clean-code-toolkit",
-  "version": "3.4.0",
+  "version": "3.5.0",
   "description": "Practical clean-code, product-readiness, and developer-handoff workflows for AI-assisted projects.",
   "author": {
     "name": "Tarik Moody"

CHANGELOG.md

@@ -1,5 +1,31 @@
 # Changelog
 
+## 3.5.0
+
+3.4.0 made the engine honest about what it found. This release is about what it was never looking at, and about the toolkit holding together as one thing.
+
+**Access Control, a new category. The tool shipped three versions with no authentication check at all.**
+- `auth-1`: is there an authentication mechanism, and which one.
+- `auth-2`: do request handlers consult an identity, or serve anyone who reaches the URL. Reported as a text match, because mentioning a guard is not the same as being guarded.
+- `auth-3`: does a permission check grant access when its own environment variable is unset. Real code rarely tests the variable inline, so the check learns the local names first, then looks for the fail-open branch on them.
+- Why it matters: this was written the same afternoon an independent review of a live application found a send endpoint reachable by any signed-in user and an owner check returning true when its variable was unset. The audit had graded that repository "A, strong evidence of controls". It now quotes the exact line and grades it D.
+
+**Warnings cost points.** Eight checks reported as `warn` with a severity badge and subtracted nothing, so a repository could display three MEDIUM findings and still score 100 in those categories: 55 points shown but not counted. A warning now costs half of the same finding failing. Less certain, never free.
+
+**The installer can deliver a correction.** `/add-clean-code` printed "already installed" and exited 0 without comparing anything, so the going-live rule corrected in 3.2.1 could never reach anyone who installed before it. The managed block now carries a version; a re-run diffs it and exits 3 when an update is available; `--update` replaces only the block between the markers; `--check` reports without touching anything. The dangling "that day" left in the template by the 3.2.1 edit is fixed.
+
+**The editing skill now says what its verification is worth.** `boy-scout-cleanup` claimed "behavior-preserving" while `/refactor`, its sibling with the same risk, carried the discipline that makes such a claim meaningful. A mutation test showed three of four real behavior changes passing a green suite. The skill now requires an undo to exist, requires checking that the suite covers the behavior being touched, and requires saying which evidence was used.
+
+**Every skill was run once against a real repository by an independent tester, and every one came back "partly works".**
+- `clean-code-scaffold` invented a stack on an empty directory without saying it was guessing, and produced a scaffold that failed on first run. It now asks when there is nothing to detect, and runs what it built the way its own README says to.
+- `developer-handoff` scoped verification to setup commands, and its template's empty sections pulled toward filling them. It now marks each statement as ran it, read it, or told to me, and deletes sections with no referent.
+- `product-readiness-review` promised in its description to run the coach first and never mentioned it in the workflow, called builds "safe" when they write into the repository, and judged against a milestone it never asked for. All three fixed.
+- `clean-code-review` never said where the review goes.
+
+**House style.** Em dashes are gone from every skill, command and template, so the report linter no longer rejects text this toolkit ships.
+
+Tests: 68.
+
 ## 3.4.0
 
 An audit of the auditor. A deliberately hollow repo, every control a costume, scored 78 out of 100 with nine passing checks, five of them false. Fixing that exposed more.

README.md

@@ -29,7 +29,7 @@ You do not need to remember tool names. Ask what you want and Claude picks the t
 
 | You want to... | Say this | What happens |
 |---|---|---|
-| Know what is missing before going live | "Audit this repo for production readiness" | A script scans for the things that bite you in production: no automatic test runner, no error alerts, secrets in the code, no undo plan. You get a plain-English report that explains each gap, and a step-by-step fix list you can hand back to Claude. Writes two report files to a scratch folder, not your repo, unless you ask. |
+| Know what is missing before going live | "Audit this repo for production readiness" | A script scans for the things that bite you in production: no automatic test runner, no error alerts, secrets in the code, a page anyone can reach that should be locked, no undo plan. You get a plain-English report that explains each gap, and a step-by-step fix list you can hand back to Claude. Writes two report files to a scratch folder, not your repo, unless you ask. |
 | Know if the product actually works for users | "Is this product ready to launch?" | Claude reads the app as a whole, not file by file. Does it do what it claims? What would a real user hit first? Changes nothing. |
 | Understand the quality of your code | "Review this code in plain English" | A report on what is unclear, untested, or fragile. Changes nothing. |
 | Get a quick cleanup list | `/code-smells` | A short list of small things worth a look. Changes nothing. |
@@ -78,6 +78,10 @@ So the audit takes waivers. Add `.prod-audit-waivers.json` to your repo:
 
 Every field is required. A waived check stops counting toward the score and the exit code, but it appears in every report with its reason, its evidence, and the name of the person who accepted it. Waivers expire after 180 days, and then the finding comes back. Claude will never write this file for you. It shows you the entry and you decide.
 
+## What it does not check
+
+It reads files, so it can tell you a control is missing or that a guard lets everyone through when a setting is absent. It cannot tell you your permissions model is correct, that your business rules are right, or that the app works. The access-control checks are deliberately shallow and say so in their own wording. A clean run is a starting point for a human, never a security review.
+
 ## The two readiness tools, and when to use which
 
 Both ask "is it ready?" They answer different halves.

commands/code-smells.md

@@ -7,7 +7,7 @@ Perform a read-only smell scan on the file, directory, or pasted code the user i
 1. Read repository guidance and detect the language, framework, and configured linter or analyzer.
 2. Run the narrowest safe configured tool when practical. Do not install dependencies or rewrite files.
 3. Inspect for duplicated business rules, unrelated responsibilities, confusing interfaces, deep nesting, misleading names, dead code, unexplained domain values, hidden side effects, and speculative abstractions.
-4. Treat counts such as 20 lines, four parameters, or three nesting levels as prompts to inspect context—not automatic failures.
+4. Treat counts such as 20 lines, four parameters, or three nesting levels as prompts to inspect context, not automatic failures.
 5. Report only actionable findings with file, line, evidence, impact, and a concrete recommendation.
 6. Separate deterministic tool output from judgment-based findings and avoid duplicating the same issue.
 7. If no linter is configured, note it as a tooling opportunity only when a linter would materially help this project. Do not automatically rank it above code defects.

scripts/add-clean-code.sh

@@ -1,36 +1,92 @@
 #!/usr/bin/env bash
+#
+# Install or update the Clean Code Standards block in a project's CLAUDE.md.
+#
+#   add-clean-code.sh [target-dir] [--update] [--check]
+#
+# The block is delimited by BEGIN/END markers and carries a version, so an
+# installed copy can be compared with the shipped one. Without that, a
+# correction shipped in a later release could never reach anyone who had
+# already installed, which is exactly what happened between v3.1.0 and v3.2.1.
+#
+#   (no flag)  install if absent; if present but out of date, say so and exit 3
+#   --check    report only, change nothing (exit 3 if an update is available)
+#   --update   replace the managed block in place, leaving everything else alone
 
 set -euo pipefail
 
-target_dir="${1:-.}"
+target_dir="."
+mode="install"
+for arg in "$@"; do
+  case "${arg}" in
+    --update) mode="update" ;;
+    --check)  mode="check" ;;
+    --*) echo "Unknown option: ${arg}" >&2; exit 64 ;;
+    *) target_dir="${arg}" ;;
+  esac
+done
+
 script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 template_file="${script_dir}/../templates/CLAUDE.md"
 target_file="${target_dir}/CLAUDE.md"
-managed_marker="<!-- BEGIN CLEAN CODE STANDARDS -->"
+begin_prefix="<!-- BEGIN CLEAN CODE STANDARDS"
+end_marker="<!-- END CLEAN CODE STANDARDS -->"
 legacy_heading="# Clean Code Standards"
 temporary_file=""
 
-cleanup() {
-  if [[ -n "${temporary_file}" && -f "${temporary_file}" ]]; then
-    rm -f "${temporary_file}"
-  fi
-}
-
+cleanup() { [[ -n "${temporary_file}" && -f "${temporary_file}" ]] && rm -f "${temporary_file}"; return 0; }
 trap cleanup EXIT
 
-if [[ ! -d "${target_dir}" ]]; then
-  echo "Directory not found: ${target_dir}" >&2
-  exit 1
-fi
+[[ -d "${target_dir}" ]]   || { echo "Directory not found: ${target_dir}" >&2; exit 1; }
+[[ -f "${template_file}" ]] || { echo "Template not found: ${template_file}" >&2; exit 1; }
 
-if [[ ! -f "${template_file}" ]]; then
-  echo "Template not found: ${template_file}" >&2
-  exit 1
-fi
+shipped_version="$(grep -m1 -o "${begin_prefix}[^>]*" "${template_file}" | sed "s|${begin_prefix} *||; s| *-*$||")"
+shipped_version="${shipped_version:-unversioned}"
+
+write_atomically() {  # $1 = content-producing command writing to stdout
+  temporary_file="$(mktemp "${target_file}.tmp.XXXXXX")"
+  "$@" > "${temporary_file}"
+  mv "${temporary_file}" "${target_file}"
+  temporary_file=""
+}
+
+strip_leading_blank() { awk 'NR == 1 && $0 == "" { next } { print }' "${template_file}"; }
 
 if [[ -f "${target_file}" ]]; then
-  if grep -qF "${managed_marker}" "${target_file}"; then
-    echo "Clean Code Standards are already installed in ${target_file}."
+  if grep -qF "${begin_prefix}" "${target_file}"; then
+    installed_version="$(grep -m1 -o "${begin_prefix}[^>]*" "${target_file}" | sed "s|${begin_prefix} *||; s| *-*$||")"
+    installed_version="${installed_version:-unversioned}"
+
+    if [[ "${installed_version}" == "${shipped_version}" ]] && \
+       diff -q <(sed -n "/${begin_prefix}/,/${end_marker}/p" "${target_file}") \
+               <(sed -n "/${begin_prefix}/,/${end_marker}/p" "${template_file}") >/dev/null; then
+      echo "Clean Code Standards are up to date in ${target_file} (${installed_version})."
+      exit 0
+    fi
+
+    if [[ "${mode}" != "update" ]]; then
+      echo "An update is available for ${target_file}."
+      echo "  installed: ${installed_version}"
+      echo "  shipped:   ${shipped_version}"
+      echo "What would change inside the managed block:"
+      diff <(sed -n "/${begin_prefix}/,/${end_marker}/p" "${target_file}") \
+           <(sed -n "/${begin_prefix}/,/${end_marker}/p" "${template_file}") || true
+      echo "Re-run with --update to replace the managed block. Nothing outside it is touched."
+      exit 3
+    fi
+
+    if ! grep -qF "${end_marker}" "${target_file}"; then
+      echo "The managed block in ${target_file} has no END marker, so its boundary is unclear." >&2
+      echo "Fix it by hand rather than risk overwriting your own instructions." >&2
+      exit 2
+    fi
+
+    write_atomically awk -v begin="${begin_prefix}" -v endm="${end_marker}" -v tpl="${template_file}" '
+      index($0, begin) == 1 { inblock = 1; while ((getline line < tpl) > 0) print line; close(tpl); next }
+      inblock && index($0, endm) == 1 { inblock = 0; next }
+      !inblock { print }
+    ' "${target_file}"
+    echo "Updated Clean Code Standards in ${target_file} (${installed_version} -> ${shipped_version})."
     exit 0
   fi
 
@@ -40,20 +96,19 @@ if [[ -f "${target_file}" ]]; then
     exit 2
   fi
 
+  [[ "${mode}" == "check" ]] && { echo "Not installed in ${target_file}."; exit 3; }
+
   temporary_file="$(mktemp "${target_file}.tmp.XXXXXX")"
   cp -p "${target_file}" "${temporary_file}"
   printf '\n\n' >> "${temporary_file}"
-  awk 'NR == 1 && $0 == "" { next } { print }' "${template_file}" >> "${temporary_file}"
+  strip_leading_blank >> "${temporary_file}"
   mv "${temporary_file}" "${target_file}"
   temporary_file=""
   action="Appended"
 else
-  temporary_file="$(mktemp "${target_file}.tmp.XXXXXX")"
-  awk 'NR == 1 && $0 == "" { next } { print }' "${template_file}" > "${temporary_file}"
-  mv "${temporary_file}" "${target_file}"
-  temporary_file=""
+  [[ "${mode}" == "check" ]] && { echo "No CLAUDE.md in ${target_dir}."; exit 3; }
+  write_atomically strip_leading_blank
   action="Created"
 fi
 
-line_count="$(wc -l < "${target_file}" | tr -d ' ')"
-echo "${action} Clean Code Standards in ${target_file} (${line_count} lines)."
+echo "${action} Clean Code Standards in ${target_file} (${shipped_version}, $(wc -l < "${target_file}" | tr -d ' ') lines)."

skills/boy-scout-cleanup/SKILL.md

@@ -13,18 +13,21 @@ Before editing:
 
 1. Read repository guidance and inspect the working tree so user changes are preserved.
 2. Identify the file's callers, exports, tests, and configured checks.
-3. Decide what evidence can verify behavior: focused tests, type checks, lint, build, or careful call-site inspection.
-4. If a proposed change could affect behavior and verification is weak, either add a characterization test with the user's approval or leave the change as a recommendation.
+3. Confirm there is an undo. If the project is not under version control and has no backup, say so and stop: a cleanup you cannot reverse is not a cleanup.
+4. Decide what evidence can verify behavior: focused tests, type checks, lint, build, or careful call-site inspection.
+5. **A passing test suite is evidence only if it covers the behavior you are about to touch.** Check that it does. The cheap way: change the value you are about to extract, or invert the condition you are about to simplify, run the suite, and see whether anything fails. If the suite stays green while the behavior is different, it does not cover this code, and green afterwards will mean nothing.
+6. When the suite does not cover it, pick one: propose a characterization test first and get approval, verify by comparing real output before and after (same input, same bytes), or leave the change as a recommendation and say why.
+7. Say which of those you did. "Tests pass" without saying what they cover is the sentence this step exists to prevent.
 
-No edit is literally zero-risk. Unused imports may have side effects, comments may preserve important context, and renames may cross public boundaries. Inspect before removing or renaming.
+"Behavior-preserving" is a claim you are making, not a property the edits have. No edit is literally zero-risk. Unused imports may have side effects, comments may preserve important context, and renames may cross public boundaries. Inspect before removing or renaming.
 
 ## Good cleanup candidates
 
 - Clarify a local variable without changing an external name.
 - Remove proven unreachable code or a proven-unused import.
 - Reduce nesting while preserving the exact conditions and evaluation order.
 - Extract a domain value whose meaning is otherwise unclear.
-- Use the project's formatter or import organizer.
+- Use the project's formatter or import organizer. If none is configured, leave formatting alone: reflowing code by hand is a repository-wide style rewrite wearing a small diff.
 - Improve a misleading comment or delete one that demonstrably restates the code.
 
 ## Out of scope

skills/clean-code-review/SKILL.md

@@ -45,6 +45,10 @@ For a beginner or vibe coder, explain each important finding in three short part
 
 Avoid unexplained acronyms and pattern-name trivia. Teach the decision, not the vocabulary.
 
+## Where the review goes
+
+Report in chat by default. Write a file only if the user asks, and then to a path they name. Do not create files in their repository unannounced.
+
 ## Boundaries
 
 - For a product-level assessment, use `product-readiness-review`.

skills/clean-code-scaffold/SKILL.md

@@ -16,12 +16,14 @@ Do not impose `components/services/utils/tests` on every stack. Next.js, Django,
 ## Workflow
 
 1. Determine the product type, language, framework, runtime, package manager, and deployment target from repository evidence.
-2. Identify the product's primary domains or features and the code that changes together.
-3. Preserve an established, coherent repository pattern unless the user explicitly requests a migration.
-4. Propose the smallest structure that clarifies ownership and dependency direction.
-5. For a new project, create the agreed scaffold and starter files.
-6. For an existing project, show the proposed moves and risks before moving files or changing imports.
-7. Run framework checks after implementation.
+2. **When there is no evidence, say so and ask.** An empty directory has nothing to detect, so anything you pick comes from the user's sentence and your own habits, not from the repository. Name the stack you would choose, name the one real alternative, and get an answer before creating files. Never present a guess in the voice of a detection.
+3. Identify the product's primary domains or features and the code that changes together.
+4. Preserve an established, coherent repository pattern unless the user explicitly requests a migration.
+5. Propose the smallest structure that clarifies ownership and dependency direction.
+6. For a new project, propose the layout and the reason for it, wait for a yes, then create the scaffold and starter files. "Agreed" means the user said yes to this layout, not that they asked for a project.
+7. For an existing project, show the proposed moves and risks before moving files or changing imports.
+8. **Run it.** Not "checks": run the thing you just created, exactly as your own README tells the user to run it, from a clean shell. A scaffold that errors on first command is worse than no scaffold, and it is the most common way this step fails. Then run the tests you created, and the project's formatter, linter, or type checker if you configured one.
+9. If the first run needs anything the README does not say, either fix the layout so it does not, or put the missing step in the README. A Python `src/` layout, for example, is not importable until the package is installed or `PYTHONPATH` is set; say which one you chose and why.
 
 ## Principles