Repository navigation
os/v3/puppy: a row that runs the selftest the appliance ships, and three rows that could skip themselves - #1218
Conversation
…ree rows that could skip themselves c29 ships /usr/local/bin/1bit-selftest (engine #1166) - the one-command proof the install sheet asks the operator to run - and no row ever executed it. The file was proved present and byte-identical to os/mini/selftest.sh, and proved by nobody that it runs in the appliance: the same class the U4 release-check row was added for in cycle 21. The new row does not demand 'RESULT: PASS'. The image boots with an empty /data/models, where the tool's own documented answer is 'RESULT: FAIL - no model to test', so demanding PASS would red a healthy image - and a row that reds a healthy image gets deleted. It requires a VERDICT: PASS with a model, SKIP naming the no-model reason without one, FAIL when the binary is absent or dies without a RESULT line. Its first version printed selftest-broken-missing and came back SKIP, and that is how the second half of this commit was found: Puppy's shell echoes the typed command into the row's own output, vm_check_verdict.skipping() matches a *-skip-* token anywhere in the body, and the command carried the marker literally - so the row was skipped by the echo of itself. Two rows already in the tree carry a literal marker and are exposed to the same thing. U1 shipped shells (#1186, merged tonight) was measured self-skipping, i.e. the row that exists to catch an unparsable script had never run; U2 console heals (#1093) is latent. All three now assemble the marker from m/sk, so the token exists only in the output. No row's semantics change. Proved: four arms under bash and sh on fixtures (absent / no-model / pass / dies silently); the module's own verdict mapping on the echoed command before and after (SKIP with token m-skip-no-model -> no token, so the output decides); and end to end on two shipped artifacts, c28 0c371992 (no such file -> FAIL selftest-broken-missing, and U1 now RUNS: 'shells-parse: 45 scripts') and c29 8266aff9 (SKIP selftest-skip-no-model on its blank models dir, 77 PASS / 0 FAIL / 4 SKIP).
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 6ee37ee7 — the row is right, and the harness defect it found is real: I reproduced it from two real boots' artifacts and then watched the fix clear it.
The self-skip, from the c28 run's own output (~/wt/os3i/vmcheck-rows-c28/rows.txt:16) — the row did run and did answer correctly, and it was still SKIP:
SKIP U1 shipped shells n=0; bad=''; forroot@1bit-os:/$ f in /usr/local/bin/1bit-* … echo shells-skip-no-scripts; …
| bash: warning: command substitution: ignored null byte in input
| shells-parse: 45 scripts under bash and sh (shells-skip-no-scripts)
feeding that body to this tree's vm_check_verdict.verdict("U1 shipped shells", "shells-parse:", body) gives SKIP, skipping() -> 'shells-skip-no-scripts' — the leaked echo, not the answer. Note why the harness's own echo-stripping missed it: the line is n=0; bad=''; for immediately followed by the serial prompt root@1bit-os:/$ , so the echo is spliced and does not contain the command's first 24 characters, which is the only thing vm-check.sh drops. So the mechanism in the comment is right, and the strip is no defence.
And the same echo on this head's text is a PASS (rowproof-selftest-green.log, c29):
PASS U1 shipped shells m=shells; sk=skip; n=0root@1bit-os:/$ ; bad=''; for … echo $m-$sk-no-scripts; …
| shells-parse: 46 scripts under bash and sh
prompt splice included, skipping() -> None, however skipping() is what decides. That is the fix, measured on two boots rather than argued.
The new row's five arms, run here under both shells (the command lifted from vm-check.sh with the binary path parameterised, bash and dash):
absent binary -> selftest-broken-missing -> FAIL (want "selftest-ok" absent)
doc'd no-model FAIL -> RESULT: FAIL - no model to test |
selftest-skip-no-model -> SKIP
RESULT: PASS -> RESULT: PASS … | selftest-ok -> PASS
dies with no RESULT -> selftest-broken-no-verdict -> FAIL
a REAL failure -> RESULT: FAIL - the NPU route … |
selftest-broken-no-verdict -> FAIL (my extra arm)
Identical verdicts under dash. And the two real artifacts agree: FAIL U4 selftest selftest-broken-missing on c28 (no such file) and SKIP U4 selftest RESULT: FAIL - no model to test | selftest-skip-no-model on c29 — i.e. it does not red a healthy image for the tool's own documented answer, which is the thing that gets rows deleted.
Completeness: with all three markers assembled, no row command in the file still carries a literal *-skip-* token (grep -- -skip- vm-check.sh outside comments returns only the U0 verdict line, whose text is a *-skip-* marker, which cannot match the token regex — the character before -skip- is *). The other -skip- hits are comments.
Nits, neither blocking:
selftest-broken-no-verdictis the label the row prints when the tool did emit a verdict — a realRESULT: FAIL - <reason>lands there, and the token says the opposite.$m-failed(or$m-failed-$reason) would read true, and a person scanning the table would not have to open the log to learn the tool answered.- the row's 300 s is the only bound on a tool that waits with a bare
wget(os/mini/selftest.sh:90-92, no-T): a wedged Lemonade or NPU turns this row into the harness's ownFAIL … no answerrather than the tool's named verdict. A-T 120in the tool, ortimeout 240inside the row, keeps the failure informative. Not introduced by this PR; it is what the new row exposes.
Not verified by me: a board run of the row with a model staged (--device npu chat inside 1bit-selftest) — that needs the board/NPU window; the VM arms above and the two c28/c29 artifacts are the evidence here.
PR Reviewer Guide 🔍(Review updated until commit 2eed8eb)Here are some key observations to aid the review process:
|
|
Persistent review updated to latest commit 2eed8eb |
os/v3/puppy: a row that runs the selftest the appliance now ships, and three rows that could skip themselves
The gap this closes
Cycle 29's image ships
/usr/local/bin/1bit-selftest(engine #1166) — the one-command proof the install sheet asks theoperator to run — and nothing in the harness ever executed it. I found that handing c29 off: the file was proved present
and byte-identical to
os/mini/selftest.sh, and proved by nobody that it runs in the appliance. Same class theU4 release-checkrow was added for in cycle 21 ("the c20 image reached the owner with release-check sayingFAIL modelswhile this row printed it as INFO"), and the same class a shipped-but-dead tool reaches a board as: a line in a document.
The new row, and what it deliberately does not require
It does not demand
RESULT: PASS. The image boots with an empty/data/models, where the tool's own documented answeris
RESULT: FAIL - no model to test; demanding PASS would red for correct behaviour, and a row that reds a healthy imageis a row that gets deleted. What it requires is that the tool runs to a verdict — PASS with a model, SKIP naming the
no-model reason without one, FAIL when the binary is missing or dies without a
RESULTline.The defect the row found in the harness itself (the part worth reading)
Its first version printed
selftest-broken-missing— and the row came back SKIP. Reason: Puppy's console shell echoesthe typed command into the row's own output, the command contained the literal marker
selftest-skip-no-model, andvm_check_verdict.skipping()matches a*-skip-*token anywhere in the body. So the row was classified SKIP by theecho of itself. Measured, not argued:
verdict()… echo $m-skip-no-model …)m-skip-no-model… echo $m-$sk-no-model …)selftest-broken-missing… no model to test … selftest-skip-no-modelselftest-skip-no-model… RESULT: PASS … selftest-okTwo rows already in the tree carry a literal skip marker and are exposed to the same thing:
U1 shipped shells(#1186 —and it was measured self-skipping:
SKIP U1 shipped shells n=0; bad=''; for…on c28, i.e. the row that exists to catchunparseable scripts has never actually run) and
U2 console heals(#1093). All three now assemble the marker fromm/sk,so the token exists only in the output, never in the command. No row's semantics change: with a model staged the new row
PASSes, and both older rows keep their own verdicts (proved in the green run below).
Proof
Shell logic, four arms, under
bashandsh(dash), fixture binaries: absent →selftest-broken-missing; documentedno-model FAIL → SKIP;
RESULT: PASS→ PASS; dies silently →selftest-broken-no-verdict.End to end, one vm-check each (UEFI + Secure Boot ON), on shipped artifacts:
U4 selftest0c371992…selftest-broken-missing~/wt/os3i/rowproof-selftest-red.log8266aff9…0b4468b6…, 0755)selftest-skip-no-model(blank/data/models)~/wt/os3i/rowproof-selftest-green.logRow sets, stated because they differ between runs and a reader should not have to guess: #1186 (
U1 shipped shells) mergedwhile this was being measured, so the c28 red run has one more row than the c29 run that preceded it. Red run: 81 rows,
76 PASS / 2 FAIL / 3 SKIP— the two reds areU3 models dir(c28 predates #1151, the row c29 turns green) andU4 selftest(this change's red arm). Green run: 81 rows,
77 PASS / 0 FAIL / 4 SKIP, the four skips being the ssh key, the two boardrows, and the new selftest row with its reason.
U1 shipped shells— the row #1186 landed three hours ago — never ran once. It was measured self-skipping (SKIP U1 shipped shells n=0; bad=''; for…), i.e. the row that exists to catch a script that does not parse was itself inert. With themarker assembled it now runs and reports
shells-parse: 45 scripts under bash and shon c28 and on c29. That is why this PRtouches a row it did not add: the class is the same one, in the file it lives in, and leaving it would mean shipping a second
inert row knowingly.
Scope
os/v3/puppy/vm-check.shonly: one new row, three marker strings assembled instead of written, and the reasons in comments.No image change, no release-check change, no
/datachecker change. c29's frozen bytes (8266aff9…) ship without it.tools/copyright.py --checkrc 0,bash -nclean. No merge, nothing tagged.