Skip to content

Commit da6de33

Browse files
LTSCommerceclaude
andcommitted
Plan 00067: QA gates scanned ZERO files in a nested checkout and reported a pass
`./scripts/qa-all.bash` — the gate CLAUDE.md makes mandatory before every Bash/Python commit — printed `✓ bash: 0 files OK` while checking nothing, in any checkout living under a path segment it excludes. lts-infra vendors this repo at untracked/repos/fedora-desktop, so that is not hypothetical. Cause: the exclusions are repo-root-relative in intent (`untracked/`, `roles/vendor/`, `.claude/ccy/`, …) but were written as unanchored globs. `find -path` matches the WHOLE path it prints, so `! -path "*/untracked/*"` excluded the entire repository. 112 bash files, all JS, and — masked behind `ruff not installed` — all Python went unscanned, exit 0 throughout. A control silently degraded to a no-op: the same outcome the ban on `|| true` exists to prevent, reached without using any banned token. Fix, in two halves: - Anchor every root-relative exclusion to `$REPO_ROOT`. `.git`, `node_modules`, `__pycache__`, `.venv` and `venv` stay unanchored — they legitimately occur at any depth. This stops today's instance. - Add a zero-file guard to each gate: finding no files of its language exits 2 with the likely cause named, instead of reporting a pass. This stops the class — the next mis-scoped exclusion, wrong REPO_ROOT, or rename. Proven by measurement in the affected checkout, not by reasoning about the patch: qa-bash exit 0, 0 files -> exit 0, 112 files (105 shellcheck findings, 0 error-level) qa-js exit 0, 0 files -> exit 0, 6 files qa-python exit 2 (masked) -> exit 0, 38 files (discovery proof, stub analyser) The 105 shellcheck findings are warning/info/style on files never analysed here before — results, not regressions. They must not be re-hidden. Two things stated rather than smoothed over: - The plan predicted 86 bash files. The real number is 112: the first estimate counted only name-matched *.sh/*.bash and missed the second find block, which picks up shebang'd executables and was equally disabled. The defect was bigger than the plan said. Corrected in the doc rather than left as an overshoot. - `qa-python.bash` checks for ruff BEFORE discovery, so neither its anchoring fix nor its guard is reachable while ruff is absent. Both were proven with a stub ruff that lints nothing; the 38 files did pass a real py_compile, but the stub's empty result set is not evidence of clean Python. Also recorded in CLAUDE/QA.md: a `0 files` report is a failure; never remove the guard to make a gate work somewhere. Plus a follow-up finding of the same class, deliberately NOT fixed here — qa-bash treats shellcheck as optional while qa-python treats ruff as required, so a box without shellcheck gets a green qa-all with no static analysis of its bash at all. That is an IaC question about declared dependencies with a far wider blast radius than a path-matching bug. The `ruff not installed` exit 2 is NOT this repo's gap, contrary to the plan's original Non-Goal: ruff is declared in both play-python.yml and this repo's own .claude/ccy/Dockerfile. The gate was run from lts-infra's container, which installs only ansible-lint and yamllint. Corrected in the plan; the fix belongs in lts-infra's IaC. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 73396b3 commit da6de33

7 files changed

Lines changed: 547 additions & 28 deletions

File tree

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
# Plan 00067 — Journal 26-07-29
2+
3+
> **Append-only activity log** for plan 00067. One file per day
4+
> (`00067-Journal-YY-MM-DD.md`). `PLAN.md` tracks the plan (what/why,
5+
> current state, tasks); this journal tracks what actually *happened* —
6+
> findings, decisions, dead-ends, hand-off state — the linear lifecycle a plan
7+
> document structurally cannot carry.
8+
>
9+
> **Entry grammar** — append new entries at the BOTTOM; NEVER edit earlier
10+
> entries (corrections are new entries):
11+
>
12+
> ```
13+
> ## HH:MM · category · REF — optional short title
14+
> ```
15+
>
16+
> - `HH:MM` local 24h (the date is in the filename); times increase down the file.
17+
> - `category` ∈ `action` | `finding` | `decision` | `thought` | `blocker` | `handoff`
18+
> - `REF` = optional task/phase ref (`T1.2`, `P1`) or `—`.
19+
> - Bodies may embed fenced logs/diffs/snippets — no size limit — with a
20+
> one-line takeaway above the fence.
21+
> - End a working session with a `handoff` entry so the next agent's entry
22+
> point is the last entry of the newest day-file.
23+
24+
## 22:08 · action · — — plan scaffolded
25+
26+
Plan 00067 created via `mkplan.bash`; `JOURNAL/` initialised. Next:
27+
fill in `PLAN.md`, then log progress here as it happens.
28+
29+
## 22:12 · action · T1.2,T1.3 — anchoring + zero-file guards applied
30+
31+
Anchored every repo-root-relative `find -path` exclusion to `"$REPO_ROOT"` in
32+
`scripts/qa-bash.bash` (both find blocks), `scripts/qa-python.bash` (both) and
33+
`scripts/qa-js.bash` (one). Left `*/.git/*`, `*/node_modules/*`,
34+
`*/__pycache__/*`, `*/.venv/*`, `*/venv/*` unanchored — those legitimately occur
35+
at any depth. Added a zero-file guard to each gate (exit 2, message naming the
36+
likely cause).
37+
38+
`bash -n` clean on all three; `shellcheck -x` exit 0, no new findings.
39+
40+
## 22:15 · finding · T2.1 — the predicted count was WRONG, and low
41+
42+
`PLAN.md` predicted bash would go 0 → 86. Measured after the fix: **112**.
43+
44+
The 86 came from my Task 1.1 measurement, which counted only the `*.sh`/`*.bash`
45+
**name-matched** files. `qa-bash.bash` has a *second* `find` block that picks up
46+
executables carrying a bash shebang with no such extension — and that block was
47+
disabled by the identical unanchored glob. So the defect was bigger than the plan
48+
said, not smaller, and the success criterion "scans 86 files" was itself wrong.
49+
50+
Corrected the Overview, Task 1.1 and the success criterion to 112 with the
51+
reason, rather than leaving 112 looking like an unexplained overshoot.
52+
53+
```
54+
=== AFTER, same nested checkout ===
55+
qa-bash exit=0 ⚠ shellcheck: 105 issues ✓ bash: 112 files OK
56+
qa-js exit=0 ✓ js: 6 files OK
57+
qa-python exit=2 ✗ python: ruff not installed (sudo dnf install ruff)
58+
```
59+
60+
The 105 shellcheck findings are all `warning`/`info`/`style` — 0 `error`-level, so
61+
exit 0 is correct. They are **results, not regressions**: files that had never
62+
been analysed here. Triage is separate work; they must not be re-hidden.
63+
64+
## 22:18 · finding · T2.1 — qa-python's own fix is unreachable in this container
65+
66+
`qa-python.bash` checks for `ruff` at line 22, **before** file discovery. With
67+
ruff absent it exits 2 there, so neither its anchoring fix nor its new zero-file
68+
guard is reached — the gate would report the same thing either way. Running it and
69+
seeing `exit 2` proves nothing about the fix.
70+
71+
Proved both with a **stub `ruff`** on `PATH` that lints nothing and prints `[]`:
72+
discovery went 0 → 38 and the guard fired `exit 2` on an empty tree. Stated
73+
explicitly in `PLAN.md` that the 38 files did pass a *real* `python3 -m py_compile`
74+
and that the stub's empty result set is **not** evidence of clean Python — that
75+
distinction is the entire subject of this plan, so blurring it here would be
76+
self-defeating.
77+
78+
Not fixing `ruff not installed`: it is a real IaC gap, `exit 2` is the correct
79+
fail-fast behaviour, and it is an explicit Non-Goal.
80+
81+
## 22:20 · action · T2.2 — negative control on the exclusions
82+
83+
Confirmed 0 of 112 scanned files come from an excluded tree — and, more usefully,
84+
that the exclusions are not *vacuously* satisfied. Each excluded tree genuinely
85+
contains bash that is genuinely being skipped:
86+
87+
```
88+
untracked 7 bash files present, 0 in scanned set
89+
roles/vendor 23 bash files present, 0 in scanned set
90+
.claude/hooks-daemon 88 bash files present, 0 in scanned set
91+
.claude/ccy 20 bash files present, 0 in scanned set
92+
.claude/skills 6 bash files present, 0 in scanned set
93+
.ansible/roles 23 bash files present, 0 in scanned set
94+
```
95+
96+
167 files skipped on purpose, 112 scanned. Asserting only "no excluded path
97+
appears" would have passed just as happily if the trees were empty.
98+
99+
## 22:22 · action · T2.3 — the guard fires
100+
101+
Copied each gate into an empty tree **without** a `.sh`/`.bash`/`.js` name and
102+
**without** the execute bit. That detail is load-bearing: a copy named
103+
`qa-bash.bash` is itself discovered, the gate reports `1 file OK`, and the test
104+
silently proves nothing. All three exit **2** with the diagnostic.
105+
106+
## 22:26 · finding · — — same class, found in our own docs, deliberately NOT fixed
107+
108+
While writing the `CLAUDE/QA.md` note I hit this in the existing gate table:
109+
110+
> `qa-bash.bash` … shellcheck (only when `shellcheck` is in PATH — silently
111+
> skipped if absent)
112+
113+
That is the same defect this plan is about: `qa-bash` treats its analyser as
114+
optional (absent ⇒ writes `[]`, still reports a pass) while `qa-python` treats
115+
its analyser as required (absent ⇒ exit 2). On a box without `shellcheck`,
116+
`qa-all.bash` goes green with **no static analysis of its bash at all**. The
117+
zero-file guard does not catch it — the file count is non-zero; it is the
118+
*analysis* that is missing.
119+
120+
Left unfixed on purpose, and said so out loud in both `PLAN.md` and `QA.md`
121+
rather than quietly folding it in: making `shellcheck` required is an IaC
122+
question about the gate's declared dependencies with a far wider blast radius
123+
than a path-matching bug, and it belongs with the `ruff` gap in its own plan.
124+
Recorded so it is tracked, not stranded in a session transcript.
125+
126+
## 22:28 · action · T3.1,P3 — recorded and completed
127+
128+
`CLAUDE/QA.md` gained a "A gate reporting `0 files` is a FAILURE, not a pass"
129+
section (the instance, the anchoring rule, "never remove the guard to make a gate
130+
work somewhere", and the shellcheck caveat above). Folder moved to `Completed/`,
131+
status flipped to Complete, README index row added in the same commit.
132+
133+
## 22:30 · handoff · — — 00067 closed; 00066 still needs the HOST
134+
135+
Plan 00067 is Complete: all tasks ✅, all success criteria met, proven by
136+
measurement in the affected checkout rather than by reasoning about the patch.
137+
138+
Still open elsewhere, unchanged by this plan:
139+
140+
- **Plan 00066 needs a HOST run** of
141+
`CLAUDE/Plan/00066-ccy-ci-runner-variant/triage.bash`. `plan_require_host`
142+
correctly refuses to run it in this container, so those facts cannot be
143+
self-served. E6 is already settled (`EXIT 125` — a missing `--device` is fatal
144+
to podman).
145+
- `ruff` is not installed here (IaC gap), and the `shellcheck`-optional finding
146+
above wants its own plan.
147+
- 105 shellcheck `warning`/`info`/`style` findings are now *visible* on 112 files
148+
that were never analysed before. Nothing regressed; they were always there.
149+
150+
## 22:34 · finding · — — CORRECTION: the `ruff` gap is not this repo's
151+
152+
Two corrections to entries above (appended, per the append-only rule — the
153+
originals are left as written).
154+
155+
**1. I edited an earlier entry in this file and had to undo it.** Reaching for
156+
`Edit` on the 22:30 handoff to point it at this correction was exactly the thing
157+
the rule forbids. Reverted byte-identical; the correction is here instead. Noting
158+
it because it is the second time this session I have walked into an append-only
159+
trap I already knew about.
160+
161+
**2. The `ruff not installed` gap is NOT fedora-desktop's.** The 22:18 entry and
162+
the plan's Non-Goal both called it "a real IaC gap" in this repo. It is not. This
163+
repo declares `ruff` in both places it needs it:
164+
165+
```
166+
playbooks/imports/play-python.yml:20 - ruff # host, via dnf
167+
.claude/ccy/Dockerfile:33 pipx install ruff # its own CCY image
168+
```
169+
170+
I am running in **lts-infra's** CCY container, not fedora-desktop's — lts-infra
171+
vendors this repo at `untracked/repos/fedora-desktop`, and its
172+
`.claude/ccy/Dockerfile` installs only `ansible-lint` and `yamllint`. So the
173+
missing dependency belongs to the surrounding container image, and the IaC fix
174+
belongs in lts-infra, not here.
175+
176+
This is the same shape of mistake as the one this whole plan is about: a true
177+
observation (`ruff` is absent, exit 2) attached to the wrong subject (*this repo's*
178+
IaC). Had I "fixed" it here I would have added a duplicate declaration to a repo
179+
that already had one, and left the actual gap in place. Corrected in `PLAN.md`'s
180+
Non-Goals too, since a stale Non-Goal would send the next reader hunting for a gap
181+
that does not exist.

0 commit comments

Comments
 (0)