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

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.