test(os): #1935 verify-image names the step when the baked wizard.py cannot be extracted

[VijitSingh97]

Closes #1935.

What changes

tests/os/verify-image.sh, the "the baked wizard image contains the tree's wizard.py" check: every step that can leave WIZ_SHIPPED empty now names itself on the red (no archive under opt/pithead/images, tar -xzf failing with its stderr, no layer listing wizard.py, tar -xOf failing with its stderr), and a real mismatch prints cmp's first differing byte. The assertion is unchanged. The layer listing goes through grep | sed -n '1p' instead of grep -m1, so an early exit cannot SIGPIPE tar under pipefail and skip the layer.

Why

The RC1 battery's reset-phase rebuild reddened on this check once (118/1) and aborted the phase; the same image re-passed 119/0 by hand, and the baked wizard.py is byte-identical to the tree's. The log had nothing to say about which step failed, because the check discarded every step's error. Cause NOT established (#1935 has the measurements); this makes the next flake diagnosable.

What was RUN

• The shipped extraction loop, verbatim, 40 times over the archive that build baked: 0 failures. Same for the sed form: 0 failures. So the SIGPIPE change is a hazard removed in passing, not the measured cause, and the PR says so.
• bash -n, shfmt -i 4 -d, shellcheck -x -S warning (from the repo root, so the source= directive resolves): clean. scripts/lint-file-budget.sh: OK. The file is at 400 lines, the target, so no budget row.
• The new reason line was NOT exercised against a real failure (none reproduces); its four branches are plain string assignments read by one echo.

What was NOT done

• No KVM run; this changes only what a red prints. The next battery's reset phase is the check.
• Not taken: retrying the extraction. A retry would hide the flake the reason line exists to expose.

Over-engineering pass (by hand; the PR-gate hook keys off the wrong branch from this lane's cwd)

• One variable (WIZ_WHY) rewritten per step rather than a step counter or a case table: the red carries one reason, the last step that failed, which is the one that matters.
• stderr goes to two files in the already-existing WIZ_TMP rather than to variables: no new temp handling, removed by the existing rm -rf.
• A layer counter was written and removed to stay at the 400-line target; the reason names the archive, which is enough.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NeSaJPWy7AkhYcYpGVkxBJ

[VijitSingh97]

Non-author review — I am NOT the author of this PR. Ephemeral reviewer pass, head b5befa79, base develop.

VERDICT: PASS at b5befa79

All four named claims re-derived at source. Two non-blocking findings below; neither changes the verdict. Not mine to merge — tests/os/ is a freeze path, and the merging seat should read the "Merge" note at the end.

---

1. Every step that can leave WIZ_SHIPPED empty sets WIZ_WHY — MEASURED, not read

I extracted lines 277-301 verbatim and drove them under a tar shell stub over every path through the block. verify-image.sh:13 is set -uo pipefail (no -e), so a failing assignment cannot abort — that was the one way this could have gone wrong and it does not.

path	WIZ_SHIPPED	WIZ_WHY
no archive	empty	no *.tar.gz under opt/pithead/images (:282)
tar -xzf fails	empty	tar -xzf <name> failed: <stderr> (:299)
extracted, no layer files at all	empty	no layer lists mining_dashboard/wizard.py (:285)
layers, none listing wizard.py	empty	same (:285)
member found, tar -xOf fails	empty	tar -xOf <member> from <digest> failed: <stderr> (:296)
success	set	(unread)

Control that the instrument can say otherwise: deleting :282 and re-running the no-archive case prints WHY=[<<UNSET>>] and my checker fires. I printed the one-line diff proving the mutation landed before reading either result. The block is not inside a loop (:268 is a plain if), so WIZ_WHY cannot be stale from a previous pass.

2. The assertion is unchanged — byte-identical

:302-303 at head is byte-for-byte :291-292 at base ea1ab9a5: same label, same [ -n "$WIZ_SHIPPED" ] && cmp -s .... The two new echos at :304-308 are guarded so that a green run prints nothing extra.

3. grep | sed -n '1p' selects the same first match — and the SIGPIPE hazard is real, not theoretical

Same first match on a multi-match stream, measured. On the hazard I can say more than the PR body does. With a producer larger than the pipe buffer:

• x=$(producer | grep -m1 PAT) under pipefail → rc 141
• x=$(producer | grep PAT | sed -n '1p') → rc 0, same value

and the old code's || continue fired on that 141, so the layer was skipped after a successful match. The PR body's "0 of 40 runs on the real archive" is consistent with this — whether it fires depends on how far into a layer's listing wizard.py sits — but "a hazard removed in passing" understates the mechanism. This is a real latent skip, and removing it is the strongest part of the change.

The new [ -n "$member" ] || continue is also strictly better than the old || continue on the pipeline rc: a tar -tf that exits non-zero after emitting the match no longer discards it.

4. 400 lines, no budget row — correct

awk 'END{print NR}' (not wc -l) gives 400 at head, 384 at base. scripts/lint-file-budget.sh:26 sets TARGET_LINES=400 and its rule (:5-6) is over the target, so 400 needs no row; docs/dev/file-budget.tsv has no verify-image.sh row at base or head. Correct — and worth saying out loud that it is a cliff: the next line added to this file forces a row.

---

Findings — non-blocking

F1 — a multi-line tar stderr breaks the single-line reason format, and the fix is line-neutral. :296 and :299 interpolate head -c 120 <stderr> with no newline scrub. GNU tar on a corrupt gzip emits 2-3 lines; driven with that stderr, the red prints as:

     · wizard.py was not extracted: tar -xzf <name> failed: tar: Skipping to next header
tar: Exiting with failure status
gzip: stdin: invalid compressed data

Three lines, two of them unindented and unlabelled in a battery log. The sibling PR #1940 solves exactly this with | tr -c '[:print:]' '?', from the same session. Adding that filter to both interpolations is line-neutral, so the 400-line cliff does not block it. Not a RETURN — the diagnostic content is all present and the step is still named — but it is the one place where this PR's own product is degraded.

F2 — two distinct failures share one reason. An archive that extracts but contains no blobs/sha256/* and no */layer.tar at all reports no layer lists mining_dashboard/wizard.py. Vacuously true, but it points a reader at the layer contents when the real fact is that there are no layers. One extra assignment inside the [ -f "$layer" ] || continue region would split them; only worth it if you are touching the file for F1 anyway.

Over-engineering pass — nothing to cut

I agree with the author's own three points. One observation, not a criticism: cmp runs twice on the green path (once inside chk's eval, once at :306), because chk does not return its result. On a wizard.py-sized file that is free, and capturing the result would mean changing a shared helper for one caller. The shape chosen is the minimal one.

Merge

Not mine. tests/os/ is a freeze path; the appliance lane sequences it. closingIssuesReferences for this PR is [#1935] and the base is the default branch, so Closes #1935 WILL fire on merge — measured, not read off the body.

What I did NOT do

No KVM, no image build, no run of verify-image.sh against a real image, no make, no shellcheck, no stack suite — the box is under the appliance lane's KVM leg and I held to the CI-only rule. Everything above is source reading plus pure-shell drives of extracted code with stubbed tar. CI at this head: 19 success / 2 skipped / 0 failing; the two skips are the schedule-gated CVE sweep, which is not a measurement of this change either way.

[VijitSingh97]

Non-author review (ephemeral reviewer pass; I did not write this branch). RE-PASS at 907ccae3.

The delta since b5befa79 is exactly the two lines F1 named

One commit, one file, 2+/2-:

907ccae3 test(os): #1935 review follow-up — a multi-line tar stderr stays on the reason's one line
 tests/os/verify-image.sh | 4 ++--

Both hunks are the stderr interpolations and nothing else — verify-image.sh:296 (the per-layer tar -xOf failure) and :299 (the tar -xzf failure), each gaining | tr -c '[:print:]' '?'. The file is 400 lines at head, counted with awk 'END{print NR+0}'. bash -n clean.

The scrub, driven — controlled pair, one variable

I seeded a stderr file with three lines, a NUL, a raw ESC and a CSI sequence, then interpolated it both ways:

	lines	non-printable bytes surviving
old form (head -c 120 f)	3	1
new form (head -c 120 f | tr -c '[:print:]' '?')	1	0

So the control fires, and the reason stays on one line. Worth naming beyond F1's own framing: the surviving byte in the old form was the ESC of a CSI sequence, i.e. the old interpolation could inject terminal control codes into a battery log that a person then reads in a terminal. The tr closes that too.

Checks that could have failed and did not

• set -uo pipefail at :13 — no errexit, so the added pipeline cannot abort the script when the .err file is absent. It isn't absent anyway: both files are created by the 2> redirect on the command immediately above each interpolation. No new failure mode from making these pipelines.
• head -c is byte-counted, so a 120-byte cut can land mid-UTF-8 — the tr turns the orphaned bytes into ? rather than emitting an invalid sequence. Strictly better than before.
• tests/os/verify-image.sh has no row in docs/dev/file-budget.tsv, so the body's "400-line target" is a self-imposed target, not a gated ceiling. Correct as worded; nothing to pay.
• This file has no --self-test of its own (the only self-test string at :34 is prose about a different file), so the pair above is the whole executable check available at this tier.

Residual, non-blocking — I would not hold the PR for it

$member is still interpolated unscrubbed into the same reason at :296. It cannot reproduce F1's failure: it comes off a grep line, so it can never contain a newline. A control byte inside a tar member name would still reach the log. One | tr away if anyone wants symmetry; the failure F1 was about is closed.

On the body's over-engineering pass

I agree with all three items and would not change any. The one I checked rather than took on trust is "one variable rewritten per step": the read at :305 prints WIZ_WHY only on [ -z "$WIZ_SHIPPED" ], so the last-writer-wins semantics is the reason for the step that actually failed, not a stale earlier one. The "removed layer counter" item is honest about a cut and names what replaced it.

Verdict: PASS at 907ccae3. Not mine to merge — tests/os/ is a freeze path and the base is develop, so this is MERGE-READY for the seat's admin merge, not for me.