review-repo: harden scanner, apply safe fixes, record first review

First /review-repo run on boma. Hardened repo-scan.py (no TODO.md/prose false
positives). Applied 7 safe fixes (DNS staleness x2, STATUS factual correction,
hosts.yml path generalisation, trunk-based wording x2, scripts/README). Recorded
the run and 17 open findings in docs/reviews/2026-05-30-*.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

CLAUDE.md

@@ -146,7 +146,7 @@ Single-contributor, trunk-based (no merge requests / approval gates):
 
 - Run `make deploy` — always run `make check` first and show output
 - Run `make tf-apply` — always run `make tf-plan` first and show output
-- Modify `inventories/production/hosts.yml` directly — regenerate via `make tf-inventory`
+- Modify `inventories/<env>/hosts.yml` directly — regenerate via `make tf-inventory`
 - Edit vault-encrypted files directly — decrypt first, re-encrypt after
 - Push to `main` branch
 - Add a collection to `requirements.yml` without a specific module need in existing role tasks

CONTRIBUTING.md

@@ -48,7 +48,7 @@ Exception: the control node is added to `hosts.yml` by hand — see
 
 ## Testing
 
-Before opening a merge request:
+Before committing or merging to main:
 
 ```bash
 make lint

README.md

@@ -71,7 +71,7 @@ See `Makefile` for the full list of targets.
 │   ├── base/               # OS baseline applied to all hosts
 │   └── docker_host/        # Docker runtime setup
 │
-├── terraform/              # VM provisioning + infra DNS (see ADR-006/009)
+├── terraform/              # VM provisioning only — no DNS (see ADR-006/009)
 │   ├── modules/            # Reusable modules (proxmox_vm)
 │   └── environments/       # Per-env state: staging/, production/
 │

STATUS.md

@@ -28,7 +28,7 @@ _Last reviewed: 2026-05-30._
 |---|---|
 | `roles/base/` | Empty directory. `site.yml` references it, but it applies nothing. |
 | `roles/docker_host/` | Empty directory. Same. |
-| `inventories/*/hosts.yml` | Placeholder stubs (commented examples); regenerated by `make tf-inventory` once Terraform has hosts |
+| `inventories/*/hosts.yml` | Structured stubs with empty host maps (`hosts: {}`); regenerated by `make tf-inventory` once Terraform has hosts |
 | `inventories/production/group_vars/{docker_hosts,proxmox_hosts}/` | Empty dirs |
 
 So `make deploy PLAYBOOK=site` currently does effectively nothing — the roles it

docs/decisions/003-toolchain.md

@@ -134,4 +134,4 @@ are removed. Each entry in `requirements.yml` must justify its presence.
 | Kubernetes/Swarm | Out of scope — Docker Compose is the right complexity level |
 | NixOS targets    | Poor Ansible fit; all hosts standardised on Debian 13       |
 
-Terraform is **adopted** for VM provisioning and infrastructure DNS — see `docs/decisions/006-terraform.md`.
+Terraform is **adopted** for VM provisioning only (no DNS) — see `docs/decisions/006-terraform.md`.

docs/reviews/2026-05-30-findings.json

@@ -0,0 +1,34 @@
+{
+  "date": "2026-05-30",
+  "reviewed_commit": "de38d1c",
+  "mode": "on-demand",
+  "counts": {"auto_fixed": 7, "open": 17},
+  "auto_fixed": [
+    {"id": "F-dns-readme", "dimension": "consistency", "severity": "high", "location": "README.md:74", "description": "terraform/ tree comment claimed 'infra DNS'", "fix": "changed to 'no DNS'"},
+    {"id": "F-dns-adr003", "dimension": "consistency", "severity": "high", "location": "docs/decisions/003-toolchain.md:137", "description": "closing line claimed Terraform does 'infrastructure DNS'", "fix": "changed to 'only (no DNS)'"},
+    {"id": "F-status-hosts", "dimension": "drift", "severity": "medium", "location": "STATUS.md:31", "description": "hosts.yml described as 'commented examples'", "fix": "now 'structured stubs with empty host maps'"},
+    {"id": "F-claude-hosts", "dimension": "consistency", "severity": "medium", "location": "CLAUDE.md:149", "description": "don't-modify bullet named only production", "fix": "generalised to inventories/<env>"},
+    {"id": "F-newrole-pr", "dimension": "drift", "severity": "low", "location": "docs/runbooks/new-role.md:80", "description": "PR/merge-request comment", "fix": "trunk-based merge note"},
+    {"id": "F-contrib-mr", "dimension": "drift", "severity": "low", "location": "CONTRIBUTING.md:51", "description": "'before opening a merge request'", "fix": "'before committing or merging to main'"},
+    {"id": "F-scripts-readme", "dimension": "cruft", "severity": "medium", "location": "scripts/README.md", "description": "3 scripts undocumented", "fix": "added entries for vault-pass-client.sh, check-vault-encrypted.sh, repo-scan.py"}
+  ],
+  "open": [
+    {"id": "R1", "dimension": "consistency", "severity": "high", "location": "docs/decisions/005-bootstrapping.md:64-65", "description": "control-node setup still uses cp .vault_pass", "status": "new", "auto_fixable": false},
+    {"id": "R2", "dimension": "drift", "severity": "high", "location": "docs/runbooks/new-host.md:6", "description": "prerequisite references .vault_pass file", "status": "new", "auto_fixable": false},
+    {"id": "R3", "dimension": "drift", "severity": "high", "location": "docs/runbooks/new-host.md:129-130", "description": "'place .vault_pass' step", "status": "new", "auto_fixable": false},
+    {"id": "R4", "dimension": "drift", "severity": "medium", "location": "CONTRIBUTING.md:32-33", "description": "secrets section describes .vault_pass sharing", "status": "new", "auto_fixable": false},
+    {"id": "R5", "dimension": "drift", "severity": "medium", "location": "AGENTS.md:16", "description": "'never commit .vault_pass' names a removed file", "status": "new", "auto_fixable": false},
+    {"id": "R6", "dimension": "consistency", "severity": "high", "location": "docs/decisions/003-toolchain.md, docs/decisions/008-testing.md", "description": "CI pipelines assume PR-to-main + approval gates vs trunk-based docs", "status": "new", "auto_fixable": false},
+    {"id": "R7", "dimension": "consistency", "severity": "medium", "location": "CLAUDE.md:151", "description": "'push to main' forbidden contradicts trunk-based policy above", "status": "new", "auto_fixable": false},
+    {"id": "R8", "dimension": "conformance", "severity": "high", "location": "roles/", "description": "base/docker_host empty dirs not tracked by git; site.yml would error on a clean clone", "status": "new", "auto_fixable": false},
+    {"id": "R9", "dimension": "conformance", "severity": "medium", "location": "terraform/environments/*/main.tf", "description": "vlan_tag never passed; VMs land untagged vs ADR-007 srv VLAN", "status": "new", "auto_fixable": false},
+    {"id": "R10", "dimension": "conformance", "severity": "low", "location": "backend.tf, Makefile, .scaffold/molecule.yml", "description": "<owner>/<repo> placeholders unresolved", "status": "new", "auto_fixable": false},
+    {"id": "R11", "dimension": "cruft", "severity": "medium", "location": "Makefile new-role + .scaffold/converge.yml", "description": "ROLE_NAME_PLACEHOLDER never substituted", "status": "new", "auto_fixable": false},
+    {"id": "R12", "dimension": "conformance", "severity": "low", "location": "Makefile:154", "description": "echo \\n not expanded in sh; use printf", "status": "new", "auto_fixable": false},
+    {"id": "R13", "dimension": "drift", "severity": "low", "location": "CLAUDE.md:93", "description": "group_vars tree lists control/ which does not exist", "status": "new", "auto_fixable": false},
+    {"id": "R14", "dimension": "consistency", "severity": "low", "location": "docs/decisions/006-terraform.md:40", "description": "'No Galaxy roles' reused in Terraform section", "status": "new", "auto_fixable": false},
+    {"id": "R15", "dimension": "consistency", "severity": "low", "location": "docs/decisions/001-architecture.md:22", "description": "DNS row reads present-tense though dns role unbuilt", "status": "new", "auto_fixable": false},
+    {"id": "R16", "dimension": "consistency", "severity": "low", "location": "docs/decisions/001-architecture.md:42", "description": "'monitoring agent' baseline not in ADR-002", "status": "new", "auto_fixable": false},
+    {"id": "R17", "dimension": "conformance", "severity": "low", "location": "playbooks/bootstrap.yml:12-16", "description": "raw python install hardcodes changed_when: false", "status": "new", "auto_fixable": false}
+  ]
+}

docs/reviews/2026-05-30-review.md

@@ -0,0 +1,98 @@
+# Repo review — 2026-05-30
+
+- **Reviewed commit:** `de38d1c`
+- **Mode:** on-demand (interactive); safe fixes applied, rest reported.
+- **Method:** `scripts/repo-scan.py` (Phase 0) + 3 fan-out reviewers (consistency,
+  drift/cruft, conformance) + cross-cutting synthesis.
+
+## Summary
+
+| | high | medium | low | total |
+|---|---|---|---|---|
+| Auto-fixed | 2 | 3 | 2 | 7 |
+| Open (reported) | 4 | 4 | 9 | 17 |
+
+Dominant theme: **drift created by this session's own rapid changes** — residual
+`.vault_pass` references after the Vaultwarden/rbw switch, and leftover
+PR/merge-request language after going trunk-based.
+
+## Auto-fixes applied (safe + obvious)
+
+| File | Fix |
+|---|---|
+| `README.md` | `terraform/` tree comment: "infra DNS" → "no DNS" |
+| `docs/decisions/003-toolchain.md` | closing line: "and infrastructure DNS" → "only (no DNS)" |
+| `STATUS.md` | hosts.yml row: "commented examples" → "structured stubs with empty host maps" |
+| `CLAUDE.md` | don't-modify bullet generalised `production` → `<env>` |
+| `docs/runbooks/new-role.md` | "open PR / merge request" → "merge to main once make test passes" |
+| `CONTRIBUTING.md` | "Before opening a merge request" → "Before committing or merging to main" |
+| `scripts/README.md` | documented the 3 undocumented scripts |
+
+Also hardened `scripts/repo-scan.py` itself: the marker check no longer matches
+filenames like `TODO.md`, and path-ref checks only fire inside backticks/links (no
+prose false positives).
+
+## Open findings
+
+### Secrets — residual `.vault_pass` references (reported, not auto-fixed: secrets-adjacent)
+- **[high] R1** `docs/decisions/005-bootstrapping.md:64-65` — control-node setup still
+  says `cp /secure/location/.vault_pass …`. Replace with the rbw setup (ADR-002,
+  rotate-secrets runbook).
+- **[high] R2** `docs/runbooks/new-host.md:6` — prerequisite "you have the vault
+  password (`.vault_pass`)". → "rbw installed and unlocked".
+- **[high] R3** `docs/runbooks/new-host.md:129-130` — "place `.vault_pass`" step →
+  rbw unlock.
+- **[medium] R4** `CONTRIBUTING.md:32-33` — secrets section describes `.vault_pass`
+  secure-channel sharing → point to rbw/Vaultwarden.
+- **[medium] R5** `AGENTS.md:16` — "Never read/print/commit `.vault_pass`" names a
+  file that no longer exists → reword to the rbw flow.
+
+### Workflow model — trunk-based vs PR/approval-gates (judgement)
+- **[high] R6** `docs/decisions/003-toolchain.md` (CI/CD) & `docs/decisions/008-testing.md`
+  (CI pipeline) describe pull-requests-to-main + manual approval gates, contradicting
+  the trunk-based, no-MR model in CLAUDE.md/CONTRIBUTING. Decide: keep *deploy*
+  approval gates (reasonable even solo) but drop the PR framing, or fully reconcile.
+- **[medium] R7** `CLAUDE.md:151` — "Push to `main` branch" listed as forbidden,
+  contradicting the trunk-based "commit straight to main" policy a few lines above.
+
+### Roles not in git (judgement)
+- **[high] R8** `roles/` — `base` and `docker_host` are empty dirs on disk and **not
+  tracked by git** (empty dirs aren't committed). A fresh clone has neither, so
+  `make deploy PLAYBOOK=site` would **error** (role-not-found), not "apply nothing"
+  as STATUS.md:29-30 says. Decide: scaffold them (`make new-role`) or correct the docs.
+
+### Terraform correctness
+- **[medium] R9** module `vlan_tag` is never passed by either environment → VMs land
+  on untagged `vmbr0`, contradicting ADR-007's srv VLAN. Wire `vlan_tag` through.
+- **[low] R10** `<owner>/<repo>` placeholders unresolved in `backend.tf` (×2),
+  Makefile `MOLECULE_IMAGE`, `.scaffold/molecule.yml` — non-functional as committed.
+
+### Scaffolding (new-role)
+- **[medium] R11** `make new-role` copies `.scaffold/converge.yml` with literal
+  `ROLE_NAME_PLACEHOLDER`, never substituted → scaffolded molecule references a
+  non-existent role. Sed-substitute `NAME` or document the manual step.
+- **[low] R12** `Makefile:154` writes the role README via `echo "…\n…"`; `\n` isn't
+  expanded in `sh` → literal `\n`. Use `printf`.
+
+### Other consistency / drift
+- **[low] R13** `CLAUDE.md:93` — group_vars tree lists `control/`, which doesn't exist
+  on disk. Create it or drop the line.
+- **[low] R14** `docs/decisions/006-terraform.md:40` — "No Galaxy roles." reused in the
+  Terraform section (an Ansible-collections concept). Reword.
+- **[low] R15** `docs/decisions/001-architecture.md:22` — DNS row reads as present-tense
+  reality though the `dns` role is unbuilt (per STATUS.md). Add a "planned" pointer.
+- **[low] R16** `docs/decisions/001-architecture.md:42` — "monitoring agent" in the
+  Proxmox baseline isn't reflected in ADR-002's baseline. Align.
+- **[low] R17** `playbooks/bootstrap.yml:12-16` — raw python-install task hardcodes
+  `changed_when: false` → never reports a change. Consider an honest `changed_when`.
+
+## Suggested follow-up prompt
+
+> Remediate the boma 2026-05-30 review (`docs/reviews/2026-05-30-review.md`):
+> 1. Purge the residual `.vault_pass` references R1–R5 → the rbw/Vaultwarden flow.
+> 2. Decide the workflow model R6–R7 — I lean "keep deploy approval gates, drop the
+>    PR/merge-request framing"; reconcile ADR-003/008 and CLAUDE.md to match.
+> 3. Resolve R8 — scaffold `base`/`docker_host` via `make new-role`, or correct
+>    STATUS.md/roles/README.md to say the roles don't exist yet.
+> 4. Fix the Terraform `vlan_tag` wiring (R9).
+> Report on the rest.

docs/reviews/latest.md

@@ -0,0 +1,23 @@
+# Latest repo review
+
+Most recent: **2026-05-30** → full report: `docs/reviews/2026-05-30-review.md`
+
+| | high | medium | low | total |
+|---|---|---|---|---|
+| Auto-fixed | 2 | 3 | 2 | 7 |
+| Open | 4 | 4 | 9 | 17 |
+
+Dominant theme: drift from this session's own changes — residual `.vault_pass`
+references after the Vaultwarden/rbw switch, and leftover PR/merge-request language
+after going trunk-based.
+
+## Suggested follow-up prompt
+
+> Remediate the boma 2026-05-30 review (`docs/reviews/2026-05-30-review.md`):
+> 1. Purge the residual `.vault_pass` references R1–R5 → the rbw/Vaultwarden flow.
+> 2. Decide the workflow model R6–R7 — I lean "keep deploy approval gates, drop the
+>    PR/merge-request framing"; reconcile ADR-003/008 and CLAUDE.md to match.
+> 3. Resolve R8 — scaffold `base`/`docker_host` via `make new-role`, or correct
+>    STATUS.md/roles/README.md to say the roles don't exist yet.
+> 4. Fix the Terraform `vlan_tag` wiring (R9).
+> Report on the rest.

docs/runbooks/new-role.md

@@ -77,5 +77,5 @@ to `inventories/staging/hosts.yml` for integration testing.
 git checkout -b role/<rolename>
 git add roles/<rolename>
 git commit -m "Add <rolename> role"
-# open PR / merge request in Forgejo
+# merge to main once make test passes, then delete the branch
 ```

scripts/README.md

@@ -5,3 +5,9 @@ dependencies (keeps them runnable anywhere without a venv).
 
 - `tf_to_inventory.py` — reads `terraform output -json` on stdin and writes an
   Ansible `hosts.yml`. Invoked by `make tf-inventory`. Data contract: **ADR-009**.
+- `vault-pass-client.sh` — fetches the master vault password from Vaultwarden via
+  `rbw`. Wired as `vault_password_file` (ADR-002).
+- `check-vault-encrypted.sh` — pre-commit guard: fails if a `vault.yml` holds
+  plaintext secrets.
+- `repo-scan.py` — Phase-0 deterministic scan for `/review-repo` (markers, broken
+  refs, unencrypted vaults, inventory).

scripts/repo-scan.py

@@ -24,7 +24,9 @@ PRUNE = {".git", ".venv", ".collections", ".ansible", ".worktrees",
 SKIP_PREFIX = os.path.join("docs", "reviews")  # don't scan our own reports
 SOURCE_EXTS = {".yml", ".yaml", ".j2", ".py", ".sh", ".md", ".tf", ".cfg", ".ini"}
 
-MARKER_RE = re.compile(r"(?<![(|])\b(TODO|FIXME|XXX|HACK)\b(?![|)])")
+# Marker words, but NOT when part of a regex alternation `(TODO|...)` or a filename
+# like `TODO.md` / `docs/TODO.md`.
+MARKER_RE = re.compile(r"(?<![(|/])\b(TODO|FIXME|XXX|HACK)\b(?![|)]|\.\w)")
 ADR_REF_RE = re.compile(r"\bADR-(\d{3})\b")
 PATH_REF_RE = re.compile(r"(?:docs|scripts|roles|inventories|terraform|playbooks)/[\w./-]+")
 PLACEHOLDER = set("<>*${}")
@@ -115,13 +117,15 @@ def scan():
                 if m.group(1) not in adrs:
                     findings.append({"check": "broken-adr-ref", "severity": "medium", "path": rpath,
                                      "line": i, "detail": f"references ADR-{m.group(1)} (no such file)"})
-            for m in PATH_REF_RE.finditer(line):
+            # Only check path-like references that appear inside backticks or a
-                # Skip paths that are part of a URL (e.g. a backend API endpoint).
+            # markdown link target — bare prose ("roles/docs") is not a real path.
-                token_prefix = line[max(line.rfind(" ", 0, m.start()),
+            for cand in re.findall(r"`([^`]+)`", line) + re.findall(r"\]\(([^)]+)\)", line):
-                                        line.rfind("\t", 0, m.start())) + 1:m.start()]
+                if "://" in cand:  # skip URLs
-                if "://" in token_prefix:
                     continue
-                ref = m.group(0).rstrip(".,);:`'\"")
+                pm = PATH_REF_RE.search(cand)
+                if not pm:
+                    continue
+                ref = pm.group(0).rstrip(".,);:`'\"")
                 if any(c in ref for c in PLACEHOLDER):
                     continue
                 if not os.path.exists(os.path.join(ROOT, ref)):