From f6150b895729e5f3d62ae265197ea4e264a79cae Mon Sep 17 00:00:00 2001 From: pm-agent Date: Sat, 31 Oct 2026 17:24:23 +0000 Subject: [PATCH] docs: Massive amounts of historical data From Gitea issue #97. Authored by the PM agent's worker; the merge gate is the approver. Refs #97. Agent: pm-agent (issue #97, mode implement) --- decisions/2026-10-31-source-comment-audit.md | 441 +++++++++++++++++++ 1 file changed, 441 insertions(+) create mode 100644 decisions/2026-10-31-source-comment-audit.md diff --git a/decisions/2026-10-31-source-comment-audit.md b/decisions/2026-10-31-source-comment-audit.md new file mode 100644 index 0000000..2971d62 --- /dev/null +++ b/decisions/2026-10-31-source-comment-audit.md @@ -0,0 +1,441 @@ +--- +date: 2026-10-31 +branch: docs/source-comment-audit +title: Audit — historical narrative in source comments (read-only inventory, sample not exhaustive) +--- + +## What changed + +**Nothing but this file.** This is the read-only audit asked for in issue #97. No source +file was edited; the pruning is to be briefed afterwards, per directory, from this list. + +### What was asked, and the calibration applied + +From the reporter (issue #97, 2026-10-31T16:42:30Z): *"'historical' talks about previous +issues or intent that have changed and why/how it is used today within comments of source +files rather than keeping them in the decisions and documentation. Source file comments +should only talk about the current files and sections of code/yaml purposes, uses, +expectations."* + +Each row below is classified: + +- **`historical`** — narrative about a superseded state, a past incident, a corrected + earlier belief, or a closed issue. Says nothing about what the line does today. +- **`load-bearing`** — phrased as history, but the sentence is the only record of a + **current** non-obvious constraint. Per the brief these get **rewrite**, never delete. +- **`ambiguous`** — I could not rule without the operator. Left for a human, deliberately + not resolved here. + +### THIS IS A SAMPLE, NOT AN EXHAUSTIVE INVENTORY + +The brief caps the audit at roughly 40–50 findings and asks for honesty over padding. +Measured candidate volume, tree-wide, with the keyword sweep below: + +``` +git ls-files -z | xargs -0 grep -nEi '#.*(used to|previously|formerly|no longer|we had| + before we|historically|in the past|originally|superseded|turned out|was wrong|predates| + legacy|issue #[0-9]+|PR #[0-9]+|measured 20|as of 20[0-9][0-9]-|see decisions/|DECISIONS-LOG)' +``` + +| Directory | candidate hits | files | coverage here | findings listed | +|---|---|---|---|---| +| `platform/` | 107 | 56 | **sampled** | 9 | +| `infra/` | 75 | 44 | **sampled** | 8 | +| `conformance/` (excl. `runs/`) | 72 | ~31 | **sampled** | 5 | +| `clusters/` | 43 | 19 | **sampled** | 7 | +| `ci/` | 42 | 15 | **sampled** | 6 | +| `catalog/` | 39 | 9 | **sampled** | 7 | +| `.gitea/` | 10 | 5 | **full** | 5 | +| `bootstrap/` | 4 | 3 | **full** | 3 | +| `platform-config/` | 3 | 1 | **full** | 3 | +| `apps/vms/` (hand-authored) | 3 | 2 | **full** | 2 | +| `apps/wave-*` (generated) | 15 | — | **excluded — see below** | 0 | +| `conformance/runs/` (archived logs) | 8 | 18 files | **excluded — see below** | 0 | +| `runbooks/` | 11 | — | **not audited** (prose docs, not source comments) | 0 | +| **total** | **~420** | **~190** | — | **55** | + +Covered **fully**: `.gitea/`, `bootstrap/`, `platform-config/`, `apps/vms/`. +Covered **by sample only**: `platform/`, `infra/`, `conformance/`, `clusters/`, `ci/`, +`catalog/`. Roughly **365 candidate hits remain unexamined**, and candidate ≠ finding — +the keyword sweep's precision on the parts I did read was well under half, so the true +residual is smaller than 365 but is certainly in the low hundreds. + +**The issue's premise holds**, with one qualification the operator should price in: of the +55 rows below, **22 are `load-bearing`** and get a rewrite rather than a deletion. The +narrative is genuinely excessive, but a mechanical "delete the history" sweep would destroy +real constraints. That ratio is the single most important number in this report. + +### Two structural warnings that change how the follow-up PRs must be written + +**1. `apps/wave-0/`, `apps/wave-1/`, `apps/wave-2/` are GENERATED. Do not edit comments there.** +They are goldens produced by one canonical invocation of `catalog/render.py` +(`ci/render-golden.sh:21`), and `ci/render-golden.sh:136-150` byte-compares every rendered +file against its committed golden — **any diff is a hard `exit 1` that blocks the merge**. +So the 15 candidate hits under `apps/wave-*` are *copies* of comments whose source is +`catalog/*.yaml` or `catalog/render.py`; e.g. `apps/wave-0/gitea.yaml:130` is the rendered +form of `catalog/gitea.yaml:100`. Edit the catalog source, then re-render with the canonical +command and commit both. A pruning PR that hand-edits `apps/wave-*` will sit red. + +**2. `conformance/runs/*/run.log` are archived evidence artifacts, not source.** 8 candidate +hits sit in 18 committed run logs. They are the recorded output of past conformance runs — +the comments inside them are copies of the test bodies as they stood at run time. Rewriting +them would falsify evidence. Excluded from scope entirely; recommend the operator confirm. + +### A dangling pointer, replicated across 18 files — fix this before any "replace with a pointer" work + +**18 files cite `DECISIONS-LOG: "Headlamp gating stall"` as their rationale pointer, and no +such entry exists.** `grep -ci "gating stall" DECISIONS-LOG.md` → **0**; the phrase appears +nowhere in `decisions/` either. Citers: + +``` +clusters/base/crafty.yaml clusters/base/headlamp.yaml +clusters/base/homepage.yaml clusters/base/kustomization.yaml +clusters/base/runbooks.yaml clusters/base/vpn-ingress.yaml +platform-config/kustomization.yaml platform/crafty/deployment.yaml +platform/crafty/kustomization.yaml platform/headlamp/kustomization.yaml +platform/homepage/kustomization.yaml platform/homepage/README.md +runbooks/kustomization.yaml runbooks/flux-reconcile.md +conformance/tests/l0-static/alert-runbooks-resolvable/check.sh (+3 more) +``` + +This is the audit's clearest warning about its own remedy: "replace the narrative with a +pointer" is only an improvement if the target exists and keeps existing. The archive is +frozen and has no index, so a title-based citation into it cannot be checked by anything. +**Recommendation:** when the operator takes the pruning PRs, either write a real +`decisions/` entry for the leaf-gating lesson and repoint all 18 citations at its filename, +or drop the citation. Pointer targets of the form `decisions/.md` are checkable; +`DECISIONS-LOG: ""` is not. Nothing below recommends a pointer target I did not +confirm exists. + +--- + +### `platform/` — 107 candidates, 9 listed (sampled). NOT sensitive, except `platform/kyverno/**` + +| # | path:lines | comment (gist) | class | action | sensitive | +|---|---|---|---|---|---| +| 1 | `platform/crafty/service-l4.yaml:33-91` | "CORRECTED 2026-09-14 after an 8-DAY LAN OUTAGE … This file used to say `Local`" — 58 lines of outage narrative | load-bearing | **rewrite.** Keep the current constraints (Cilium's L2 announcer is not backend-aware; SNAT mode loses client IP; `/ban-ip` degrades so use `/ban`; `remote-node` is subsumed by `cluster` so do NOT add it; no `loadBalancerSourceRanges` deliberately). Drop the outage story, the lease-age detail and the per-protocol tell | no | +| 2 | `platform/longhorn/helmrelease.yaml:191-200` | "`orphanAutoDeletion: true` USED TO BE HERE AND WAS INERT. Removed 2026-09-13" | load-bearing | **rewrite** to the forward-looking rule: Longhorn ≥1.9 replaced it with `orphanResourceAutoDeletion` and the chart silently drops unknown `defaultSettings` keys — so do not set it | no | +| 3 | `platform/longhorn/helmrelease.yaml:249-254` | "CORRECTION, re-measured 2026-09-13 later the same day: the line originally recorded here read …" — corrects a superseded measurement | historical | **delete.** The correct reading is already stated at `:246-248` | no | +| 4 | `platform/monitoring/alerts.yaml:625-636` | "Both rules below used to read `== 2` / `== 3`, with the comment 0=unknown 1=healthy…" | load-bearing | **rewrite.** Keep "`longhorn_volume_robustness` is a one-hot `state`-labelled gauge, value 0 or 1" — that is why the expressions look as they do. Drop the old-enum story | no | +| 5 | `platform/monitoring/alerts.yaml:869-875` | "This alert originally used `severity: info`. That was WRONG, and wrong in the silent direction" | load-bearing | **rewrite** to the current rule: carries no `severity` deliberately, because `general.rules`' `InfoInhibitor` would swallow an info-severity deploy notification | no | +| 6 | `platform/monitoring/pvc-capacity-alerts.yaml:12-27` | "`LonghornVolumeCapacityHigh` … never did, for two different reasons in sequence, and it is now GONE" — dated 2026-09-06/2026-09-13 timeline | load-bearing | **rewrite.** Keep only "these rules key on `kubelet_volume_stats_*` because `longhorn_volume_actual_size_bytes` measures allocation, not filesystem use". Delete the sequence | no | +| 7 | `platform/headlamp/helmrelease.yaml:77-85` | "These two used to be keys INSIDE headlamp-oidc-credentials.sops.yaml. That was the bug…" + the dev-Keycloak `i/o timeout` story | load-bearing | **rewrite** to the current rule: issuer and callback URLs are public and MUST be cleartext, because Flux's `${BASE_DOMAIN}` postBuild substitution cannot reach inside an encrypted Secret. Keep the `[parity]` note at `:87-92` as-is | no | +| 8 | `platform/vpn-egress/egress-steering.yaml:79` | "CORRECTION 2026-09-12 — this comment used to say the host nftables kill-switch 'is the…'" | load-bearing | **rewrite** to state the actual mechanism (Cilium's eBPF egress-gateway drop) without the correction framing | no | +| 9 | `platform/keycloak/passkey-reconciler.yaml:18` | "It used to be duplicated here almost verbatim" | historical | **delete**, or replace with pointer `decisions/2026-10-18-keycloak-passkey-reconciler.md` (confirmed present) | no | + +Also seen in `platform/` and left unlisted for the cap, same shapes: `platform/cilium/helmrelease.yaml:57-65` +(load-bearing — the `k3s_api_host`/`k3s_apiserver_endpoint` split is the current parity contract), +`platform/kubevirt/kubevirt-cr.yaml:7` (`useEmulation was previously true`), `platform/external-snapshotter/kustomization.yaml:35` +(`Previously these were REMOTE`), `platform/kustomization.yaml:7` (`the same pattern by which kyverno was previously deferred`), +`platform/pm-agent/configmap.yaml:22`, `platform/monitoring/tests/run.sh:61` and `:129`, +`platform/cnpg/alerts.promtool-test.yaml:20`, `platform/oauth2-proxy/auth-visibility-alerts.yaml:36-42` +and `:98-101`, `platform/vpn-ingress/daemonset.yaml:165`. + +### `infra/` — 75 candidates, 8 listed (sampled). NOT sensitive, except `infra/gitea/**` + +| # | path:lines | comment (gist) | class | action | sensitive | +|---|---|---|---|---|---| +| 10 | `infra/plex/claim-ship.sh:29-32` | "Branch prefix is now cosmetic. It used to matter: gate.yml's push trigger covered feature/** … Those per-branch push triggers were deleted" | historical | **delete.** The live trigger set is already documented at `.gitea/workflows/gate.yml:48-56` and `CLAUDE.md` §4 | no | +| 11 | `infra/ansible/inventory/hosts.ini:105` | "GROUP-NAME GAP: CLOSED 2026-08-31. The roles and playbooks used to hardcode the…" | historical | **delete** — the gap is closed and the current variable is named at `playbooks/node-health.yml:12` | no | +| 12 | `infra/ansible/inventory/hosts.ini:147-153` | "NOTE (CORRECTED 2026-09-12): this used to read 'node 3 also has wlp1s0 up on 192.168.2.195 (wifi)'. It does not" | load-bearing | **rewrite** as a positive present-tense assertion: node 3 has no `wl*` netdev and a single IPv4 uplink on `eno1`. That fact is load-bearing for the kill-switch datapath; only the correction framing should go | no | +| 13 | `infra/ansible/roles/node_health/defaults/main.yml:110-121` | "The first real run of this role FAILED k3s-prod-1's Crucial/Micron C400 on `reallocated=61440` … That verdict was WRONG" | load-bearing | **rewrite.** Keep the current rule (SMART raw values are vendor-defined; Micron TN-FD-20 scales ID 5 by `Block_Sector_Count`, so ID 170/196 are the trustworthy corroborators). Drop "the first real run failed" | no | +| 14 | `infra/ansible/roles/unattended_upgrades/tasks/main.yml:304-309` | "This was originally a `find` over /etc/kernel/postinst.d … That theory was wrong" | load-bearing | **rewrite** to the current justification: the sentinel writer is ours at two known paths, and two exact `stat`s are strictly stronger than a glob that would match Debian's own conffile | no | +| 15 | `infra/ansible/roles/node_health/tasks/assess.yml:398` | "measurement was right; the inference from the log line was wrong — the memattr block below…" | historical | **delete** | no | +| 16 | `infra/devcontainer/dev.sh:103-108` | "SUPERSEDED but still honoured: `DEV_KUBECONFIG` used to be the only way to reach a cluster other…" | load-bearing | **rewrite.** "Still honoured" is current behaviour and `CLAUDE.md` §3 tells every agent to set `DEV_KUBECONFIG=/dev/null`, so this must keep documenting the variable — just not as history | no | +| 17 | `infra/vpn-ingress/new-peer.sh:136` | "COMMENT LINES ARE NOT ALLOCATIONS. This used to grep the whole file and so matched the…" | load-bearing | **rewrite**, replacing the history with pointer `decisions/2026-10-06-new-peer-ignores-template-comments.md` (confirmed present) | no | + +Unlisted, same shapes: `infra/ansible/inventory/group_vars/all.yml:31`, +`infra/ansible/inventory/group_vars/k3s_prod/vars.yml:86` ("CORRECTION 2026-09-06: an earlier +version of this comment claimed…"), `infra/ansible/roles/node_baseline/tasks/disks.yml:559`, +`infra/ansible/roles/node_baseline/tasks/main.yml:801`, `infra/ansible/roles/kube_vip/tasks/main.yml:68`, +`infra/ansible/roles/cilium/defaults/main.yml:30`, `infra/ansible/playbooks/node-health.yml:12`, +`infra/ansible/roles/vpn_egress_gateway/templates/vpn-egress-up.sh.j2:39` and +`vpn-egress-down.sh.j2:15`, `infra/devcontainer/kubeconfig.sh:26`, `infra/agent/Containerfile:216`, +`infra/agent/build-image.sh:30`, `infra/agent/pm-worker:97`, `infra/agent/probe-validation.sh:98`. + +### `catalog/` — 39 candidates, 7 listed (sampled). NOT sensitive — but every edit here re-renders `apps/wave-*` + +| # | path:lines | comment (gist) | class | action | sensitive | +|---|---|---|---|---|---| +| 18 | `catalog/qbittorrent.yaml:7-12` | "BULK DOWNLOADS TIER — DELIVERED 2026-09-12 (the deferral below is CLOSED; kept for the record). This entry used to ship config-only, with a comment promising … 'once OQ-2 is resolved'" | historical | **delete.** Self-describes as "kept for the record", which is precisely the issue's complaint | no | +| 19 | `catalog/qbittorrent.yaml:141-151` | "It USED to be a doubled-dollar default-operator expansion of a WEBUI_PORT env var … That MISDIAGNOSIS is preserved in this service's own marker log at 2026-09-13T05:35Z" | load-bearing | **rewrite.** Keep "PORT is a literal matching `ingressRoutes[0].port`; a literal cannot be mangled by envsubst". Drop the misdiagnosis story (it is already preserved in the marker log the comment itself cites) | no | +| 20 | `catalog/plex.yaml:43-46` | "(This comment previously asserted '~4 minutes' as established fact. It was … Measured 2026-09-29: the full loop is ~2m45s and fits.)" | historical | **rewrite** to the measured figure alone; delete the self-correction parenthesis | no | +| 21 | `catalog/plex.yaml:65` | "So the earlier 'a placeholder claim Secret would crashloop the pod' note was WRONG about the…" | historical | **delete** | no | +| 22 | `catalog/plex.yaml:318` | "actually granted access (device-cgroup EPERM, measured 2026-10-01), so there was nothing to keep" | historical | **rewrite** with pointer `decisions/2026-10-01-quicksync-via-device-plugin.md` and/or `decisions/2026-10-03-cilium-cgroup-hostroot.md` (both confirmed present) | no | +| 23 | `catalog/gitea.yaml:14` | "Headless-install bootstrap (formerly `apps/gitea-bootstrap-patch.yaml`; now first-class via the…)" | historical | **delete** the "formerly" clause; keep the description of what the block does | no | +| 24 | `catalog/render.py:159-166` | "THE SCRAPE WORKS AS OF 2026-10-05 — but it took THREE independent pieces … (PR #28) … (PR #49)" | load-bearing | **rewrite.** The three-part requirement and "do not read this flag alone as 'metrics are working'" are current and genuinely non-obvious; drop the PR numbers and the "63.7h" incident, pointing at `decisions/2026-10-05-oauth2-sidecar-scrape-verified.md` (confirmed present) | no | + +`catalog/pihole.yaml:210` and `:218` are the **exemplar to copy**: narrative already reduced to +`see decisions/2026-10-04-pihole-multi-replica.md` / `-pihole-declare-state.md`, both of which +exist. Unlisted: `catalog/gitea.yaml:100-130`, `catalog/nextcloud.yaml:211` and `:226-240`, +`catalog/semaphore.yaml:84`, `catalog/pihole.yaml:179`, `catalog/qbittorrent.yaml:389-417`, +`catalog/sonarr.yaml:120`, `catalog/render.py:672`, `:1371`, `:2161`. +`catalog/jackett.yaml:73` ("floor only, no ceiling; 0.24.x runs as of 2026-10-30") is +**load-bearing, leave alone** — a dated version constraint, exactly the carve-out the brief names. + +### `.gitea/` — 10 candidates, 5 listed (FULL coverage). **SENSITIVE** (`.gitea/**`) + +| # | path:lines | comment (gist) | class | action | sensitive | +|---|---|---|---|---|---| +| 25 | `.gitea/workflows/gate.yml:14-16` | "(it used to print '— not green' and then 'MERGE GATE: PASS', exit 0 — fail-open in the gate itself)" | historical | **delete** the parenthesis; the surrounding `GATE_STRICT_SKIPS` rationale is current and stays | **yes** | +| 26 | `.gitea/workflows/gate.yml:48-56` | "There is NO per-branch push trigger. There used to be a prefix list (feature/** fix/** …) and it was a net negative here" + the 2026-09-29 plex-claim cost | load-bearing | **rewrite.** "Do not re-add a push trigger; `act_runner` is capacity 1 and the duplicate run serialises ahead of the PR run" is a live constraint a future editor needs. Drop the plex-claim anecdote | **yes** | +| 27 | `.gitea/workflows/gate.yml:118-123` | "WIDENED TOLERANCE, DECLARED: `\|\| [ "$?" -eq 3 ]` used to cover exactly one skip (spec-immutability)" | load-bearing | **rewrite.** This documents a deliberate *current* weakening plus its exit condition (`REQUIRE_GITEA_SETTINGS=true` once the secret is seen green). Keep the instruction, drop "used to" | **yes** | +| 28 | `.gitea/workflows/renovate.yaml:72-76` | "CORRECTED 2026-10-30. This list used to name only write:repository + read:user … Run 350 then failed at `GET /api/v1/orgs/… 403`" | load-bearing | **rewrite** to the required scope list as a plain present-tense requirement; point at `decisions/2026-10-30-renovate-auth.md` (confirmed present) for the run-350 story | **yes** | +| 29 | `.gitea/workflows/merge-gate.yaml:15` | "The text that used to sit here…" | historical | **delete** | **yes** | + +Remaining `.gitea/` candidates examined and judged **not findings** (current-state +measurements, correctly phrased): `renovate.yaml:6`, `:34`, `:259`, `images.yaml:44`, +`scripts/images.sh:11`. + +### `ci/` — 42 candidates, 6 listed (sampled). **SENSITIVE** (`ci/**`) + +| # | path:lines | comment (gist) | class | action | sensitive | +|---|---|---|---|---|---| +| 30 | `ci/decisions-log.sh:5-23` | 19-line "WHY THIS STAGE EXISTS" block reproducing the four-PR conflict table verbatim | historical | **replace with a pointer.** It is a near-verbatim duplicate of `decisions/README.md:9-30`, which the comment itself already cites at `:23`. Cut to the assertion list + that pointer (and `decisions/2026-09-29-decisions-log-conflict-free.md`, confirmed present) | **yes** | +| 31 | `ci/gitea-settings.sh:7-23` | "On 2026-09-29 that blind spot was measured … PR #1 … sat `pending` for 30.6 minutes … PRs #5-#9 merged with zero approvals" | load-bearing | **rewrite.** "No other check can see Gitea's own database; assert-scripts do not detect drift from their own target" is the current reason the stage exists. Drop the PR-by-PR history | **yes** | +| 32 | `ci/l0.sh:15-23` | "That union replaces what used to be a hand-written 'UNCONDITIONAL' stage per whole-repo check (3c subst-safety / 3d keycloak-client-secrets / 3e bulk-tier)" | historical | **rewrite** to describe the present selection rule only (touched-requirement subset ∪ every test declaring `scope: whole-repo`) | **yes** | +| 33 | `ci/lib.sh:138-142` | "WHY THIS EXISTS: `ci/l0.sh` used to carry a hand-written stage per whole-repo check, each with its own `summary …` line" | historical | **rewrite** to "names every offline check that ran and its verdict" | **yes** | +| 34 | `ci/approval-policy.sh:107-112` | "Blocking on that turned 'CI cannot read your override' into a message that said 'you did not post one', and cost a real debugging session on PR #38" | load-bearing | **rewrite.** The token-scope fact and the fail-direction guarantee ("an unreadable source can only cause a false BLOCK, never a false pass") are current; the debugging-session cost is not | **yes** | +| 35 | `ci/touched-requirements.sh:288-293` | "R-TEST-06 added 2026-09-12. The comment in `changed_diff()`'s rule 3 above records why it was ABSENT: R-TEST-06 used to be enforced by the spec-immutability STAGE" | load-bearing | **rewrite.** Keep why a `ci/` change must attribute R-TEST-06 today; drop the absence history | **yes** | + +Unlisted: `ci/approval-policy.sh:30-38` and `:182`, `ci/conformance.sh:73`, `ci/gate.sh:133`, +`:233-234`, `ci/l0.sh:86`, `ci/l1.sh:33`, `ci/lib.sh:65`, `ci/render-golden.sh:5-6` and `:24`, +`ci/touched-requirements.sh:39`, `:316`, plus `ci/tests/*.test.sh` (5 files — note these are +test *rationale* headers, where a "this used to fail on main" line arguably earns its keep; +I would send the whole `ci/tests/` group to `ambiguous`). + +### `clusters/` — 43 candidates, 7 listed (sampled). **SENSITIVE** (`clusters/**`) + +| # | path:lines | comment (gist) | class | action | sensitive | +|---|---|---|---|---|---| +| 36 | `clusters/base/cluster-vars.yaml:244-251` | "IT IS NO LONGER A CROSS-NETWORK KNOB (2026-10-07). This used to read 'the two clusters are on OPPOSITE SIDES of pfSense'…" | load-bearing | **rewrite.** Keep "exactly one storage host, 192.168.2.30; still a knob only because the renderer reads it per-environment" and the immutability warning at `:253+`. Replace the pfSense/DNAT history with pointer `decisions/2026-10-07-bulk-nfs-truenas.md` (confirmed present) | **yes** | +| 37 | `clusters/prod/kustomization.yaml:335-351` | "APEX AS OF 2026-09-13 … Prerequisites completed and VERIFIED before this flip, in this order: 1. dev's 18 A records released … 2. dev's 18 LEGACY UNPREFIXED TXT registry entries deleted by hand … 3. dev's Certificate deleted" | historical | **rewrite.** The three completed prerequisites are done and unrepeatable. Keep only the still-true `NOTE` at `:348-351` (the zone's two wildcards point at the legacy stack's public IP and prod's specific records shadow them) | **yes** | +| 38 | `clusters/prod/kustomization.yaml:472-479` | "*** DECIDED 2026-09-06 by the operator: the VIP, as intended. *** (This block previously read 'Not yet decided' while shipping .10 …)" | load-bearing | **rewrite.** Keep "this is the control-plane VIP, NOT .20 the ingress VIP and NOT .10 the single node address" and why a node-pinned CNI endpoint fails silently. Drop the previous-wording parenthesis | **yes** | +| 39 | `clusters/base/apps-vms.yaml:51-55` | "postBuild.substituteFrom ENABLED 2026-08-31 (R-ENV-02). It was previously omitted because Flux's envsubst runs over the ENTIRE manifest…" | load-bearing | **rewrite.** The envsubst hazard over VM cloud-init and the `$${...}` escape obligation are live constraints for anyone adding a VM manifest; the enablement date and prior omission are not | **yes** | +| 40 | `clusters/dev/kustomization.yaml:282-287` | "THIS VALUE IS INERT — dev is decommissioned, its nodes no longer exist, and the Proxmox host this used to name (192.168.1.9) went with the pools it held" | load-bearing | **keep, trim.** "Inert, restated per this overlay's supply-every-knob convention, unproven because there is no dev node" is honest current state and the kind of comment the issue wants | **yes** | +| 41 | `clusters/base/apps-wave-0.yaml:4`, `apps-wave-1.yaml:4`, `apps-wave-2.yaml:4` | "wait:true health-gates ONLY this wave's resources, so a later wave can no longer stall an earlier one" (×3, and the same sentence is emitted by `catalog/render.py:2385`) | load-bearing | **rewrite** the "no longer" to plain present tense. Note the renderer emits this string, so fix `render.py:2385` and re-render rather than editing the three files by hand | **yes** | +| 42 | `clusters/base/headlamp.yaml:9-12`, `homepage.yaml:10`, `crafty.yaml:11`, `kustomization.yaml:53`, `runbooks.yaml:15`, `vpn-ingress.yaml:9` | six files citing `DECISIONS-LOG: "Headlamp gating stall"` — **target does not exist** (see above) | historical | **rewrite** to the present-tense rule (a leaf UI belongs in its own post-`platform-config` Kustomization so its crashloop cannot stall the chain) and drop or repoint the dead citation | **yes** | + +Unlisted: `clusters/base/cilium.yaml:52`, `cluster-vars.yaml:29`, `:51`, `:264`, +`platform.yaml:17`, `:56`, `platform-config.yaml:6`, `platform-config-cdi.yaml:13`, +`oauth2-proxy.yaml:10`, `clusters/dev/kustomization.yaml:67`, `:81`, `:222`, +`clusters/dev/sync.yaml:8`, `clusters/prod/kustomization.yaml:277-283`, `:317`, `:363`, `:546`, `:554`. + +### `conformance/` — 72 candidates (excl. `runs/`), 5 listed (sampled). **SENSITIVE** (`conformance/**`) + +| # | path:lines | comment (gist) | class | action | sensitive | +|---|---|---|---|---|---| +| 43 | `conformance/tests/l3-fault-injection/vpn-killswitch/chainsaw-test.yaml:16-24` | "MECHANISM CORRECTED 2026-09-12: this comment used to credit 'the host nftables kill-switch' … The ASSERTION below is unchanged and still the right one — only the explanation was wrong" | load-bearing | **rewrite** to state the real mechanism (Cilium's eBPF egress-gateway drop, netfilter never invoked) in the present tense. Pairs with finding #8 — same correction duplicated in `platform/vpn-egress/` | **yes** | +| 44 | `conformance/tests/l3-fault-injection/vm-data-survival/chainsaw-test.yaml:18-27` | "(Feature-gate note, revalidated 2026-08: an earlier revision of this comment also claimed the containerDisk BOOT path was broken … That premise is now STALE: the cluster is k3s v1.36.4, where ImageVolume is ENABLED)" | historical | **rewrite** to the one current sentence that matters — DataVolume is used because only it survives a rebuild — and drop the whole stale-premise parenthesis | **yes** | +| 45 | `conformance/tests/l0-static/ha-control-plane-as-code/chainsaw-test.yaml:51-60` | "TWO GAPS CLOSED 2026-09-06 (they used to be listed above; recorded so the change is legible)" | historical | **rewrite.** Both gaps are closed and now asserted; state the assertions, drop the gap history. Same in `check.sh:98` and `:500` ("This section used to be an ESCALATION in the header's gap list") | **yes** | +| 46 | `conformance/lib/status.py:303-310` | "It used to be, and that mis-reported a SAFETY-CRITICAL requirement … A previous engineer worked around it by rewording prose" | load-bearing | **rewrite.** "Scaffold detection must name the idioms it hunts for, because a placeholder-detection test legitimately contains the words" is the current design constraint; the R-STATE-03 misreport and the previous engineer are not | **yes** | +| 47 | `conformance/tests/l0-static/bulk-tier-distinct-from-replicated/check.sh:447-455` | "It used to assert `… in L0`: ci/l0.sh carried a hand-written 'UNCONDITIONAL' stage per whole-repo check" — **the same paragraph is duplicated verbatim in 3 files** (also `flux-postbuild-substitution/check.sh:604`, `keycloak-client-secret-reconciler/check.sh:678`) | historical | **rewrite** all three to the present mechanism (the test declares `scope: whole-repo`; `ci/l0.sh` unions declaring tests via `--include-scope`; both halves checked). Good candidate for one small PR — identical edit ×3 | **yes** | + +Unlisted: `conformance/incluster/chainsaw-job.yaml:44`, `conformance/incluster/rbac.yaml:105` +(**also sensitive as `**/rbac.yaml`** — "superseded design" for the gluetun Deployment; +load-bearing, since it explains why the fault Job needs no RBAC), `conformance/lib/common.sh:33`, +`conformance/lib/status.py:114`, and ~10 further `tests/l0-static/*/check.sh` headers of the +same shape (`alert-metrics-have-scrape-targets`, `alert-rules-unit-tested`, +`alert-runbooks-resolvable`, `auditability-proofs-producible`, `keycloak-passkey-reconciler`). + +### `bootstrap/` — 4 candidates, 3 listed (FULL coverage). **SENSITIVE** (`bootstrap/**`) + +| # | path:lines | comment (gist) | class | action | sensitive | +|---|---|---|---|---|---| +| 48 | `bootstrap/flux/components/kustomization.yaml:36-38` | "On 2026-09-01 that shipped `k8sServiceHost: ""` into the Cilium HelmRelease and put 2 of 3 agents into Init:Error (DECISIONS-LOG, INCIDENT entry)" | load-bearing | **rewrite.** The fail-open default and what the feature gate changes are the current justification; the incident is one clause. Pointer is vague ("INCIDENT entry") — `grep` finds 2026-09-01 material in the archive, so if a citation is kept it should name a locatable heading, not "INCIDENT entry" | **yes** | +| 49 | `bootstrap/flux/bootstrap.sh:84-86` | "see ./components/kustomization.yaml and the 2026-09-01 INCIDENT entry in DECISIONS-LOG.md" | historical | **rewrite** to keep only the cross-reference to `./components/kustomization.yaml`, which is checkable; drop the archive citation (duplicate of #48) | **yes** | +| 50 | `bootstrap/sops/secret-templates/pia-custom-ovpn.yaml.example:6` | `See DECISIONS-LOG "Phase 3 VPN — custom .ovpn"` | ambiguous | **operator call.** The target *does* exist (10 hits for "Phase 3 VPN" in the archive), so this is a working pointer of the discouraged form. Keep, or repoint if a `decisions/` entry is written. Cited by path+line only; no content quoted (secret-adjacent `.example` template) | no (`*.sops.yaml.example` is not matched by `**/*.sops.yaml`) | + +Remaining candidate `bootstrap/flux/bootstrap.sh:122` ("a patch target that no longer exists +after a Flux…") examined — **not a finding**, it describes current failure behaviour. + +### `platform-config/` — 3 candidates, 3 listed (FULL coverage). **SENSITIVE** (`platform-config/kustomization.yaml`) + +| # | path:lines | comment (gist) | class | action | sensitive | +|---|---|---|---|---|---| +| 51 | `platform-config/kustomization.yaml:8` | "See `clusters/base/platform-config.yaml` + DECISIONS-LOG." | historical | **rewrite** to keep the checkable half (`clusters/base/platform-config.yaml`) and drop the bare, untargeted `DECISIONS-LOG` reference — it names no entry and cannot be resolved | **yes** | +| 52 | `platform-config/kustomization.yaml:93` | "Invisible on dev, whose CDI CRDs have existed for months. See DECISIONS-LOG 2026-09-05." | load-bearing | **rewrite.** "THE RULE" at `:95-96` is the keeper; the dev-invisibility observation is stale (dev is decommissioned — see finding #40) and should go | **yes** | +| 53 | `platform-config/kustomization.yaml:167` | "headlamp: httproute MOVED OUT … Keeping it here … meant a headlamp crashloop stalled platform-config. See DECISIONS-LOG 'Headlamp gating stall'." | historical | **rewrite** to the present-tense statement of where the HTTPRoute lives now; **dead pointer**, see the dangling-pointer section | **yes** | + +### `apps/vms/` — 3 candidates, 2 listed (FULL coverage). Hand-authored, NOT generated + +| # | path:lines | comment (gist) | class | action | sensitive | +|---|---|---|---|---|---| +| 54 | `apps/vms/unifi/50-service-l4.yaml:27-78` | "CORRECTED 2026-09-14 after an 8-DAY LAN OUTAGE … This file used to say `Local` … BOTH HALVES OF THAT WERE FALSE" — ~52 lines | load-bearing | **rewrite**, same treatment as #1 (its near-twin). Keep: L2 announcer is not backend-aware; KubeVirt `masquerade` already destroyed the client IP so `Cluster` costs nothing here; `cluster` subsumes `remote-node` so do NOT "fix" the CNP; no `loadBalancerSourceRanges` deliberately. Drop the outage story | no | +| 55 | `apps/vms/unifi/30-virtualmachine.yaml:119`, `40-networkpolicy.yaml:36` | `# legacy Network UI (still served)`, `# legacy UI` on port 8443 | load-bearing | **keep.** "legacy" here is the upstream product's own name for a port that is still served — it describes current behaviour, not history. Listed so a keyword-driven sweep does not delete it | no | + +--- + +### Per-directory counts (findings listed above) + +| Directory | historical | load-bearing | ambiguous | total | sensitive-path? | +|---|---|---|---|---|---| +| `platform/` | 2 | 7 | 0 | 9 | no (except `kyverno/**`) | +| `infra/` | 3 | 5 | 0 | 8 | no (except `gitea/**`) | +| `catalog/` | 4 | 3 | 0 | 7 | no | +| `clusters/` | 2 | 5 | 0 | 7 | **yes** | +| `ci/` | 3 | 3 | 0 | 6 | **yes** | +| `conformance/` | 3 | 2 | 0 | 5 | **yes** | +| `.gitea/` | 2 | 3 | 0 | 5 | **yes** | +| `bootstrap/` | 1 | 1 | 1 | 3 | **yes** (×2) | +| `platform-config/` | 2 | 1 | 0 | 3 | **yes** | +| `apps/vms/` | 0 | 2 | 0 | 2 | no | +| **total** | **22** | **32** | **1** | **55** | 29 of 55 sensitive | + +The `ambiguous` bucket is thin because I could rule on most rows against the reporter's +calibration. Two groups I am **deliberately not ruling on** and want the operator's call: + +1. **`ci/tests/*.test.sh` headers** (5 files). These say things like "this suite exists + because `ci/gate.sh` used to set BLOCK=1 for a FAILED stage but…" and "a stage was added + to gate.sh by PR #17, never added here, so 15 of 50 assertions failed on `main`". For a + *regression test*, the regression it guards is arguably the test's subject rather than + fluff. Ruling these `historical` would strip the only record of what each assertion is + for. **Operator to decide.** +2. **In-repo `README.md` prose** (`platform/vpn-egress/README.md:91`, + `platform/crafty/README.md:52`, `infra/devcontainer/README.md:126`, + `infra/gitea/README.md:253`). These are documentation, not source comments, so arguably + already in the place the issue wants narrative to live. I did not audit them as findings. + +### Suggested order for the follow-up PRs — cheapest and least sensitive first + +1. **`catalog/` + re-render `apps/wave-*`** (7 findings, not sensitive). Highest value per + unit of review: it also fixes the generated copies under `apps/wave-*`. Must re-run + `python3 catalog/render.py --all catalog -o apps --wave-layout --no-image-automation`, + drop the not-onboarded outputs, and commit the goldens, or `render-golden` fails. +2. **`platform/` excluding `kyverno/**`** (9 findings, not sensitive). Biggest residual too + (107 candidates) — worth splitting by component (`monitoring/`, `longhorn/`, `crafty/`, + `headlamp/`, `vpn-egress/`) into 2–3 PRs rather than one sweep. +3. **`infra/` excluding `gitea/**`** (8 findings, not sensitive). `ansible/roles/*` is most + of it; `infra/plex/claim-ship.sh:29-32` (#10) is a clean one-paragraph deletion and a + good first commit. +4. **`apps/vms/`** (2 findings, not sensitive). Self-contained; #54 is the twin of #1, so do + it right after `platform/crafty/service-l4.yaml` while the reasoning is fresh. +5. **The dangling-pointer fix** (18 files, crosses sensitive and non-sensitive). Either + write one `decisions/` entry for the leaf-gating lesson and repoint all 18, or drop the + citations. Doing this *before* steps 6–9 means those PRs do not have to re-litigate it. +6. **`platform-config/kustomization.yaml`** (3 findings, **sensitive**). One file, three + edits — the smallest possible sensitive-path PR, so it is the cheapest review. +7. **`bootstrap/`** (3 findings, **sensitive**). Two files. +8. **`.gitea/`** (5 findings, **sensitive**). Keep #27 (`gate.yml:118-123`) in its own commit + — it documents a live, deliberate tolerance-widening and is the one most likely to be + misread as a behaviour change. +9. **`ci/`** then **`clusters/`** then **`conformance/`** (6 + 7 + 5, all **sensitive**, + largest residuals). Last, because each needs a non-author approving review or an operator + `/approve-override`, and because `ci/` edits touch the thing doing the approving. + +A note on review cost: **29 of the 55 findings are on sensitive paths**, so roughly half +this work cannot merge on a green gate alone. Steps 1–4 (26 findings) are the portion that +can. That is the argument for the ordering above. + +## What I verified + +Everything below was run in this container, offline, against the tree at +`466f63e` on branch `docs/source-comment-audit`. + +- **The tree layout, against the brief's believed list.** `git ls-files` → **855** tracked + files. The brief's list was broadly right but differs in three ways worth recording: + (a) there is **no `scripts/` and no `tools/` directory**; (b) `docs/` **does exist in this + repo** (9 files: `docs/BULK-NFS-STORAGE.md` + `docs/plans/*.md`) and is *not* the sibling + spec repo's `docs/` — I audited neither, since both are prose, not source comments; + (c) `platform-config-cdi/` exists as a separate top-level directory (1 file, 0 candidate + hits). +- **Candidate volume, per directory**, with the keyword sweep quoted verbatim above: + `platform/` 107 hits / 56 files, `infra/` 75 / 44, `conformance/` 80 / 40 (**72 excluding + the 8 in `conformance/runs/`**), `clusters/` 43 / 19, `ci/` 42 / 15, `catalog/` 39 / 9, + `apps/` 18 (**15 under the generated `apps/wave-*`, 3 under hand-authored `apps/vms/`**), + `runbooks/` 11, `.gitea/` 10 / 5, `bootstrap/` 4 / 3, `platform-config/` 3 / 1, + `platform-config-cdi/` 0. Total ≈ **420 candidate hits in ≈190 files**. +- **`apps/wave-*` is generated and byte-enforced.** Read `ci/render-golden.sh`: the canonical + command is at `:21`/`:120`, and the comparison loop at `:136-150` iterates the *rendered* + output, requiring each golden to exist and to be byte-identical, `exit 1` on any diff. + Confirmed the duplication concretely: the "HARDENED (was a silent first-boot RACE)" + paragraph appears at both `catalog/gitea.yaml:100` and `apps/wave-0/gitea.yaml:130`. + Confirmed `apps/vms/` is *not* produced by that loop (the renderer emits `wave-N/` paths + only), so it is hand-authored and directly editable. +- **The dangling pointer.** `grep -ci "gating stall" DECISIONS-LOG.md` → **0**; + `grep -ci "Headlamp gating stall" DECISIONS-LOG.md` → **0**; + `grep -ril "gating stall" decisions/` → **no output**; `git grep -il "gating stall"` → + **18 files**. So 18 files cite a rationale entry that exists in neither the frozen archive + nor `decisions/`. +- **Every `decisions/...md` pointer I recommend exists.** Checked against `ls decisions/` + (71 entries + `README.md` + `ARCHIVE.sha256`): `2026-10-18-keycloak-passkey-reconciler.md`, + `2026-10-06-new-peer-ignores-template-comments.md`, `2026-10-01-quicksync-via-device-plugin.md`, + `2026-10-03-cilium-cgroup-hostroot.md`, `2026-10-05-oauth2-sidecar-scrape-verified.md`, + `2026-10-07-bulk-nfs-truenas.md`, `2026-10-30-renovate-auth.md`, + `2026-09-29-decisions-log-conflict-free.md`, `2026-10-04-pihole-multi-replica.md`, + `2026-10-04-pihole-declare-state.md` — all present. I recommend **no pointer target I did + not list in `ls` output**. +- **Archive-citation targets**, for the rows where I say a `DECISIONS-LOG` pointer does or + does not resolve: `"Remote Path"` 5 hits, `"Phase 3 VPN"` 10, `"Phase 4 image-automation"` 1, + `F-DNS1` 3, `F-OPS1` 1, `2026-09-01` 6 — all present in the 5,733-line archive. + `"Headlamp gating stall"` 0, as above. +- **The sensitive-path column is from the policy data, not the brief.** Read + `infra/gitea/approval-policy.json`: `sensitive_paths` = `ci/**`, `.gitea/**`, + `conformance/**`, `clusters/**`, `bootstrap/**`, `infra/gitea/**`, `platform/kyverno/**`, + `platform-config/kustomization.yaml`, `.sops.yaml`, `**/*.sops.yaml`, `**/rbac.yaml`, + `CLAUDE.md`. This matches the brief, and adds the root `.sops.yaml` and the explicit + `**/` glob forms. Note `**/*.sops.yaml` does **not** match `*.sops.yaml.example`, which is + why findings #50 and the `platform/headlamp/...sops.yaml.example` row are marked not + sensitive. +- **Every cited `path:line-range` was opened and read**, not matched and trusted. All 55 + rows quote text I read with `sed -n` at the line numbers given, at `466f63e`. Line numbers + are from that commit and this PR changes no source file, so they remain valid on merge. +- **Secret-adjacent files**: no `*.sops.yaml` content is quoted anywhere in this report, and + no `.data`/`.stringData` value or key name appears. The two `.sops.yaml.example` rows + (#50, and `platform/headlamp/headlamp-oidc-credentials.sops.yaml.example:15` in the + unlisted set) are cited by path and line only. I ran no `sops -d`. +- **The gate.** Result recorded in the PR; see "What I did NOT verify" for the stages that + cannot run in this container. +- **`git diff --stat` against `main`**: one added file, `decisions/2026-10-31-source-comment-audit.md`, + and nothing else — which is the brief's "done means" condition. + +## What I did NOT verify + +- **Anything about the live cluster. I hold no cluster credentials** — no kubeconfig and no + ServiceAccount token is mounted in this container, so `kubectl` cannot work here. This + audit is a pure text analysis of the tree and makes **no claim about the running + platform**. That is appropriate (no behaviour changes), but it also means every *factual* + assertion quoted inside the comments I audited — the measured Cilium lease holders, the + `longhorn_volume_robustness` series counts, the 192.168.2.30 NFS reachability, the k3s + version, the oauth2-proxy scrape targets — is **unverified by me**. I classified comments + by what they *talk about*, not by whether their measurements are still true. A rewrite PR + that restates one of those measurements as current fact should re-measure it first. +- **The two gate stages that cannot run in this container**, per the environment notes and + `CLAUDE.md` §3: `spec-immutability` (the sibling spec repo has no git remote and is not + reachable from the cluster) and `gitea-settings` (needs `SETTINGS_API_TOKEN`, which I + deliberately do not hold). So R-TEST-06 is **not** actually exercised on this run. +- **~365 of the ~420 candidate hits were not examined.** The per-directory table marks which + directories are full and which are sampled. For `platform/`, `infra/`, `conformance/`, + `clusters/`, `ci/` and `catalog/` the listed findings are **representative, not complete**, + and the `historical : load-bearing` ratio I report (22:32) is measured over the 55 rows I + judged, **not** over the whole corpus — do not project it onto the residual. +- **The keyword sweep is not a complete detector.** It finds English phrasings I thought of. + Historical narrative that avoids all of them — a dated war story with no "used to", a + stale rationale phrased entirely in the present tense, a comment that is simply wrong + about today — is **invisible to it**. The most dangerous category (a confidently + present-tense comment describing a superseded state) is precisely the one this method + cannot find. A complete audit needs a reader per directory, not a grep. +- **`runbooks/` (11 candidate hits), the repo-root markdown (`README.md`, `DESIGN-waves.md`), + and the in-repo `docs/` tree were not audited.** They are prose documentation rather than + source comments, so they fall outside the reporter's wording — but `CLAUDE.md` §1 already + records that `runbooks/` contains stale `git push origin main` instructions, so there is + known stale narrative there that this audit does not cover. +- **The sibling spec repo (`R-*` requirements, ADRs) was not read or assessed at all** — it + is not present in this container and is human-owned and immutable to agents (R-TEST-06). + Nothing here proposes a change to it. Where a comment cites a spec document + (e.g. `docs/06/R-STATE-01` in finding #44), I did **not** verify that the citation + resolves. +- **I did not verify that any recommended rewrite is correct, only that the current comment + is historical.** Each "rewrite" row names what I believe is the load-bearing residue; the + engineer doing the pruning must confirm that residue is still true before writing it as + present-tense fact. I wrote no replacement text. +- **Whether the operator agrees with my `historical` / `load-bearing` line.** I applied the + calibration from issue #97 literally plus the brief's one carve-out. The 22:32 split is a + judgement call, and the two groups in the `ambiguous` discussion above are explicitly + unresolved and need a human ruling before the `ci/tests/` and README rows are touched. +- **`conformance/runs/*/run.log` and `apps/wave-*` were excluded by my own judgement**, on + the structural grounds given (archived evidence; gate-enforced goldens). The operator has + not confirmed either exclusion. +- **Nothing here traces to an `R-*` requirement**, deliberately — this is housekeeping, per + the brief. I did not attempt to attribute one, and I did not check whether + `ci/touched-requirements.sh` expects an attribution for a `decisions/`-only change. -- 2.54.0