Skip to content

test(os): #1936 the pre-fill control polls the state read and names what it saw - #1940

Closed
VijitSingh97 wants to merge 9 commits into
developfrom
fix/1936-prefill-control-reason
Closed

test(os): #1936 the pre-fill control polls the state read and names what it saw#1940
VijitSingh97 wants to merge 9 commits into
developfrom
fix/1936-prefill-control-reason

Conversation

@VijitSingh97

@VijitSingh97 VijitSingh97 commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Closes #1936.

Stacked on #1934 (its commit is this branch's parent): merge #1934 first; this PR is then one commit. Until then the compare view shows both.

What changes

Why

This leg never ran in the RC1 battery (the install phase returned early at #1931), so its first real run is the next battery, and a one-word red there would send someone back to the log with nothing to read. The #1934 reviewer's N3 named it.

What was RUN

  • bash tests/os/provision-browser-submit.sh --self-test: passes. Under a curl shim it drives a timeout (rc 28, 000), a 200 without .config, an HTML 502 page carrying a non-printable byte and longer than the 60-byte bound (so the bound is asserted; review follow-up), and a config that arrives on the third read with a blank line before the status line; asserts each reason on the exact string the log will carry, the read count (6 / 3), the compact-JSON and raw-string forms of WIZ_STATE, and provision_browser_submit's end-to-end reason. The last case is the control that the shim answered, not the real curl.
  • Mutation control: x5sx4s in the reason reddens three checks (timeout, noconfig, html) and the self-test returns 1.
  • The leg's case pattern over its three outcomes (harness wallet arms; a foreign wallet is named; no wallet prints the reads' reason), exercised in a scratch copy of the three lines, since the leg itself boots a guest.
  • The file still sources clean under run.sh (. tests/os/provision-browser-submit.sh defines both functions and runs nothing).
  • bash -n, shfmt -i 4 -d, shellcheck -x -S warning from the repo root over all three files: clean. scripts/lint-file-budget.sh: OK (provision-browser-submit.sh: 39 lines at origin/develop, 53 at test(os): #1932 the browser-shaped submit polls the state read and names what it saw #1934's head, 128 here; the body first said "61 → 127", which was neither endpoint — corrected after review).

What was NOT done

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

  • One helper, not two loops: the adjacent merge, taken. It is why this stacks on test(os): #1932 the browser-shaped submit polls the state read and names what it saw #1934 rather than duplicating its loop one file over.
  • provision_page_error keeps its single read: its two callers (run.sh handoff poll, the leg's) already poll it inside a 24×5 s loop, and an empty answer is a real answer there.
  • Two globals rather than stdout: the value and the reason are two outputs, and a function called via $(…) cannot also print a reason. run.sh already hands the leg its state as globals (ip, SERIAL).
  • The self-test lives in the helper file, not a new tests/stack file: the shape failure-evidence.sh and aged-version.sh use, and a new stack file is gated by tests/inventory.sh.
  • The shim's call counter is a file: curl runs inside $(…), so a variable never comes back. A first draft counted in a variable and reported six reads as zero.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NeSaJPWy7AkhYcYpGVkxBJ

…mes what it saw

The RC1 battery's provision phase reddened on the bare word no-served-config,
which covered a non-200, a 5 s timeout on a cold first read and a non-JSON body
alike. The helper now reads /api/state up to six times, 5 s apart, and when no
read serves a config the reason carries the status, curl's rc, the try count
and the head of the body.

Measured on the host: a timeout parses to http=000 with an empty body, a live
200 without .config falls through to the reason, a JSON body with a trailing
newline yields .config intact. run.sh is set -uo pipefail, so the failing
assignment cannot exit the harness.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NeSaJPWy7AkhYcYpGVkxBJ
@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Non-author review — I am NOT the author of this PR. Ephemeral reviewer pass, head 6d5cf28e, base develop. I reviewed the one commit 6d5cf28e; f2684e4c (#1934) is that PR's to carry and I did not re-review it.

VERDICT: PASS at 6d5cf28e — on the code. Two gating facts below: a merge ORDER, and a CI job that is still running.


1. provision_browser_submit's contract to run.sh:2010 is unchanged

run.sh:2010 is scode=$(provision_browser_submit "$ip" "$jar"), tested at :2011 against "200". The failure string is byte-for-byte what it was: the old inline printf 'no-served-config(http=%s curl=%s after %sx5s body=%s)' becomes printf 'no-served-config(%s)' with WIZ_STATE_WHY carrying the same four fields in the same order (:35, :44). return 1 on no served config, unchanged. tries reaches 6 on the failure path in both versions.

2. Self-test — RUN by me, in a detached worktree at 6d5cf28e

$ bash tests/os/provision-browser-submit.sh --self-test
#1936 wizard-state-poll self-test passed
rc=0

3. Mutation battery — the author's named mutant reproduces, and one mutation SURVIVES

Every mutation's one-line diff printed before its run, so each is proved applied.

mutant result
M1 x5sx4s at :35 (the author's named one) rc 1, 3 checks red — timeout, noconfig, html. Exactly as claimed.
M4 drop | tr -c '[:print:]' '?' at :35 rc 1, 1 red (html)
M5 jq -rcjq -c at :30 rc 1, 2 red (wallet, control) — the raw-string half is pinned
M8 jq -rcjq -r at :30 rc 1, 1 red (late) — the compact-JSON half is pinned
M6 -lt 6-lt 1 at :25 rc 1, 7 red
M7 drop WIZ_STATE_WHY="" at :24 rc 1, 1 red (late) — the reset is pinned
M3 head -c 60head -c 40 at :35 rc 0 — SURVIVES

M5 and M8 together settle the jq -rc claim from both sides, which is better evidence than either alone. M3 is the gap — see G1.

4. The leg's case still arms on the harness wallet's first 8 chars

I drove reinstall-prefill-submit-leg.sh:65-73 verbatim, against the real wizard_state_poll, under a curl shim, with a synthetic 28-character wallet of my own (never the repo fixture, never a real address):

served outcome
the harness wallet armed
first 8 chars match, tail differs armed (prefix match intact)
a foreign wallet red, names the wallet
config with no wallet red, none; http=200 curl=0 after 6x5s body={"config":{"monero":{}}}
timeout red, none; http=000 curl=28 after 6x5s body=
401 red, none; http=401 curl=0 after 6x5s body=Unauthorized

The last three are the three shapes that printed one identical word before this commit. That is #1936, delivered.

5. WIZ_STATE / WIZ_STATE_WHY collide with nothing — plus two scoping facts I checked

git grep WIZ_STATE over the head tree: only provision-browser-submit.sh and the leg. run.sh's WIZ_ family is WIZ_FAIL_REASON (:752-808) — different name. Two things worth recording because globals invite them:

  • run.sh:2010 calls provision_browser_submit inside $(…), a subshell, so the globals never escape there; the leg calls the poll in the same shell, which is exactly what :66 needs. Both call sites are right for opposite reasons.
  • The helper's local … tries=0 (:23) shadows the leg's own tries (:22, live at :45-48) and restores it on return. The leg also resets at :82. No leak either way.
  • run.sh:41 is set -uo pipefail; both globals are assigned unconditionally at :24, so ${WIZ_STATE:-none; $WIZ_STATE_WHY} at leg :69 cannot trip -u.

6. Sourcing runs nothing — tested with the value that would trigger

Not just "I sourced it and nothing happened": I sourced it with $1 set to --self-test, the exact input the guard exists to gate. [ "${BASH_SOURCE[0]}" = "${0}" ] short-circuits first; rc 0, no output, 5 functions defined (wizard_state_poll, provision_browser_submit, provision_page_error, _wsp_case, _wsp_self_test). run.sh:61 sources it before the leg at :63.


Findings

G1 — the one worth fixing: head -c 60 is asserted by nothing. M3 (60→40) leaves the self-test green. The longest fixture body is the HTML page at ~29 printable bytes; no shape reaches 40, let alone 60. The truncation width is the thing that keeps this reason line bounded and single-line — the whole product of the PR — and it is the one constant with no fixture behind it. One case with a >60-byte body, asserting the exact truncated string, closes it. Non-blocking (nothing is wrong today), but this is a test-only PR whose deliverable is a string, and that string's bound is untested.

G2 — a comment states something the code does not do. :124 is set -u # what tests/os/run.sh runs the helpers under. run.sh:41 is set -uo pipefail. I re-ran the self-test with the full options substituted: still rc 0, so this is a wrong comment, not a false green — pipefail is inert here because no pipeline's rc is read. Fix the comment or the flags; either is one token.

G3 — a figure in the PR body has no source. "provision-browser-submit.sh 61 → 127 lines". Measured (awk 'END{print NR}'): 39 at base ea1ab9a5, 53 at the parent f2684e4c, 127 at head. 61 is neither endpoint. Nothing turns on it — every one of those is under the 400 target, there is no budget row for the file at any of the three, and test-harness-tooling.sh 369→379 checks out exactly — but a figure that cannot be reproduced from any ref shouldn't sit in a "What was RUN" section.

Inherited, and now on a second surface. after 6x5s names the cadence; the worst-case elapsed time is ~60 s (6 × -m 5 plus 6 × sleep 5). #1934's reviewer raised this (their N2); this commit doubles the number of red messages carrying it. Still not worth a round on its own — but if G1 or G2 brings you back to :35, fix all three there.

Over-engineering — where I disagree with the author's own pass

The pass in the body is honest and I agree with five of its six points: _wsp_case, the file-based call counter, two globals over stdout, the self-test living in the helper file rather than a new tests/stack file, and provision_page_error keeping its single read. The control case (.errorx) also earns its place — M5 and M8 show it is half of what pins -rc, not decoration.

The one thing I would cut is the six-read poll in the LEG, and the argument against it is the author's own new comment. :63-64 says "the pre-fill is read off the disk before the page is served, so the first read carrying a wallet is the verdict" — and :55 has already POSTed to /auth and got a session cookie, so the page is up and authenticated by the time :65 runs. The retry therefore buys only the narrow window where /auth answers but /api/state has not yet loaded the config, and it costs up to ~60 s of dead time on the red path where the old code cost ~5 s. #1936's entire product is the reason string, and one read delivers that.

I am not asking for a change. Reusing the shared helper is cheaper code than adding a tries parameter for one caller, and 60 s is noise beside a leg that boots guests. But the body's over-engineering pass presents the shared helper as pure win, and this is its price — it belongs in the section rather than being discovered by whoever reads the next battery's timings.

Merge — there is an ORDER, and it is not optional

CI — do not read this one off the label

At my read: 19 success / 2 skipped / 0 failing, and Shell tests (shellcheck + pithead suite) still in_progress. That is the one job that runs the new row. A pending check is the absence of a measurement.

And when it lands green, that is still not enough by itself: the assertion was added to an existing file (test-harness-tooling.sh), so a green label is equally consistent with the row never being reached. Read the job log for #1936 wizard-state-poll self-test passes. My local rc-0 run above is evidence about the code, not about CI collecting it.

What I did NOT do

No KVM, no guest boot, no image, no make, no shellcheck, no tests/stack/run.sh — the box is under the appliance lane's KVM leg. The single self-test run and the mutation battery are pure bash with curl and sleep stubbed, no network. I did not re-review f2684e4c.

VijitSingh97 added a commit that referenced this pull request Sep 6, 2026
…he reason's one line

Both stderr interpolations scrub non-printable bytes to `?`, the same as the sibling helper in
#1940, so a GNU tar message with newlines no longer breaks the red across three lines.
Line-neutral; the file stays at 400.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NeSaJPWy7AkhYcYpGVkxBJ
@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

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

G1 — the 60-byte bound is now genuinely exercised, and I have the pair that proves it

Run at head: bash tests/os/provision-browser-submit.sh --self-test#1936 wizard-state-poll self-test passed, rc 0.

Mutation, with the diff printed before the run so the mutation is proved applied — head -c 60head -c 40 at :35:

  FAIL html: got [... body=<html><body>502 Bad Gateway? from an ups]
           want [... body=<html><body>502 Bad Gateway? from an upstream that never ans]
#1936 wizard-state-poll self-test FAILED: 1 checks   rc=1

The leg that makes this evidence rather than a demonstration: the same mutant at 6d5cf28e SURVIVES#1936 wizard-state-poll self-test passed, rc 0. One variable moved (the shim's HTML body), and the bound went from unguarded to guarded. That is the differential G1 asked for, and it is what a single-head run could not have shown.

Arithmetic re-derived rather than read off the assertion: the shim body is 87 bytes, the asserted string is exactly 60. The bound is at the boundary, not near it.

G2 — the option comment is true now

tests/os/run.sh:41 is set -uo pipefail, and it sources this file at :61. So :125's set -uo pipefail # what tests/os/run.sh runs the helpers under is a true statement about the consumer. It was not before.

G3 — the body's line figures, re-derived at source

claimed measured (awk 'END{print NR+0}')
provision-browser-submit.sh @ origin/develop 39 39
… @ f2684e4c (#1934's head) 53 53
… @ 963f234f 128 128
test-harness-tooling.sh 369 → 379 369 → 379

All four match.

The delta is confined where the ask said

One commit, one file, 6+/5-. Every hunk is at or below the # --- self-test (#1936) --- banner at :64: the header comment :67-:69, the shim's html arm :86, the html assertion :103, and the dispatcher's set line :125. wizard_state_poll and provision_browser_submit are byte-identical to 6d5cf28e — measured as a sha256 over each function body at both refs, not inferred from hunk offsets, because the whole value of a re-pass is that the reviewed half did not move. The instrument has a firing control: the same measurement over _wsp_self_test reports CHANGED.

Nit, non-blocking

The reflow at :67-:69 leaves a stub line — # and sleep is a no-op. Each — where the wrap used to be even. Cosmetic; fix it if you touch the file again, don't push for it.

On the body's over-engineering pass

I agree with it, and one item is stronger than the body states. "The shim's call counter is a file: curl runs inside $(…), so a variable never comes back" is not a style preference, it is the only correct choice — and rows :96 and :107 assert the count (6 and 3), so the mechanism the body describes is itself under test. The item I probed for weakness was "the self-test lives in the helper file, not a new tests/stack file": the counter-argument would be that a --self-test guarded by [ "${BASH_SOURCE[0]}" = "${0}" ] ships dead weight into every run.sh source. It does not — sourcing takes the false branch and defines two functions plus _wsp_case/_wsp_self_test, which cost nothing. The body's reason (a new stack file is gated by tests/inventory.sh) is sound and I would not change the call.

Not merge-eligible, independent of this verdict

mergeStateStatus is BEHIND, and #1934 — the parent this stacks on — is still OPEN. Merge order is #1934 first. Base is develop and tests/os/ is a freeze path, so this is MERGE-READY for the seat's admin merge either way, not mine.

Verdict: PASS at 963f234f.

VijitSingh97 and others added 4 commits September 6, 2026 07:59
…ute, not the dashboard's

The bench said it, six of six: http=404 body "404: Not Found". The helper
read /api/state, which only the provisioned dashboard serves; the wizard
registers /api/wizard-state (wizard.py:526), the route run.sh's own
pre-fill check already reads. Both reads in the helper move to it. The
browser-shaped provision leg could not have passed on any tree since #1847.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NeSaJPWy7AkhYcYpGVkxBJ
… state route too

The fourth /api/state read in tests/os — the fixture-works control at the top of the
reinstall pre-fill submit leg — used the wizard's session jar against a route only the
provisioned dashboard registers. The wizard serves /api/wizard-state (wizard.py:526), and its
`config` carries `monero.wallet_address`, the field the control compares. Without this the first
install run to get past the restore leg (PR #1947) would have redded with "pre-fill NOT armed
(served wallet: none)" — loud, but naming the wrong cause.

Found by the fixes lane at source (git grep at d973163: zero "/api/state" in wizard.py, two
"/api/wizard-state"); re-derived here before the edit. The three /api/state reads left in
run.sh are basic-auth reads against the provisioned dashboard and stay.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NeSaJPWy7AkhYcYpGVkxBJ
…hat it saw

One helper, wizard_state_poll, now does the /api/state read for both consumers: up to six
reads 5 s apart until a jq filter yields a value, and on no hit a reason naming the status,
curl's rc, the read count and the head of the body. provision_browser_submit (#1932) calls it
for `.config`; the reinstall pre-fill leg's fixture-works control (#1936) calls it for the
served wallet, so its red names the wallet it saw, or what the reads saw when none carried one,
instead of "served wallet: none" for a timeout, a 401 and a non-JSON page alike.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NeSaJPWy7AkhYcYpGVkxBJ
…e option comment is true

The HTML shape's body now exceeds the 60-byte bound, so `head -c 60` is pinned (the reviewer's
M3 mutant, `60`→`40`, now reddens one check); the self-test's `set` line names the options
run.sh actually uses, `-uo pipefail`.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NeSaJPWy7AkhYcYpGVkxBJ
@VijitSingh97
VijitSingh97 force-pushed the fix/1936-prefill-control-reason branch from 963f234 to 549904a Compare September 6, 2026 13:40
@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Rebased onto #1934's head 9fbc6e80 — the PASS at 963f234f is STALE, re-pass needed at 549904ae

Why a rebase and not a wait: this branch stacks on #1934's first commit (f2684e4c), and a trial merge (git merge-tree --write-tree of the two heads, base f2684e4c) CONFLICTS in both tests/os/provision-browser-submit.sh and tests/os/reinstall-prefill-submit-leg.sh#1934's second and third commits swap /api/state for /api/wizard-state on lines this branch's wizard_state_poll refactor moved. So this PR would have gone un-mergeable the moment #1934 lands.

What the resolution is: #1940's shape, #1934's route. wizard_state_poll reads /api/wizard-state; provision_page_error and the header comment carry #1934's swap; the reinstall leg's fixture control keeps this PR's poll (wizard_state_poll … '.config.monero.wallet_address // empty') — the route now lives inside the helper. Proof the replay changed nothing else: the patch of this branch's two commits before the rebase (f2684e4c..963f234f) and after (9fbc6e80..549904ae) differ in exactly the /api/state/api/wizard-state lines and the one comment naming the route; nothing else moved. bash tests/os/provision-browser-submit.sh --self-test passes at the new head (run here, tier 1, no guest).

Base stays develop; the diff will show #1934's commits until #1934 merges (it is MERGE-READY with a non-author PASS at 9fbc6e80). I am the author and do not merge.

@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Non-author review — I am the fixes lane and I did not write this branch. RE-PASS at 549904ae.

What I re-derived (PROVEN BY ME, not relayed)

G1 — the replay changed only the route. This is the load-bearing claim: the content pass at 963f234f carries over only if the rebase moved nothing else. Re-derived independently rather than read — git diff f2684e4c 963f234f against git diff 9fbc6e80 549904ae, index lines stripped:

  • Same file set (a --name-only diff of the two is empty), same length (191 lines both sides).
  • The diff-of-diffs is 22 lines, 12 of them content (</>), and every one mentions /api/state or /api/wizard-state — filtering those 12 for anything else returns empty.
  • Fired control: seeding one non-route line into the post-rebase patch makes that same filter print it. The empty result is a measurement, not a blind instrument.

Your claim stands as stated.

G2 — the swap is correct and load-bearing. At 549904ae, dashboard/mining_dashboard/wizard.py:522-532 routes /, /setup, /install, /auth, /api/wizard-state, /submit, /submit-restore, /api/handoff, /handoff-ack, /keep-role, /status. There is no /api/state on the wizard app — that route is the dashboard app's. So the pre-#1934 form was not reading a different-but-equivalent endpoint; it was reading one the wizard does not serve.

The finding — the evidence line is TRUE and narrower than it reads

The body cites bash tests/os/provision-browser-submit.sh --self-test passing at the new head. It does — I ran it at 549904ae, rc 0. But it cannot see the route.

  • Measured: sed -i 's|/api/wizard-state|/api/BOGUS-state|g' on that file (4 occurrences replaced, sha256 moved ebbd552b3783 -> f38c3a4cbe5a), and --self-test still prints passed, rc 0.
  • Mechanism, not a fluke: _wsp_self_test's curl() shim branches on $shape alone and never reads $@, so the URL is invisible to it by construction.

This is not a defect in the self-test. It is a poll-logic test and a good one — six-read retry, the exact WIZ_STATE_WHY strings, the 60-byte bound, the late-arrival case, and its own .error = x case as the control that the shim answered rather than a real read. The finding is about what the evidence line licenses: the single thing this rebase changed is the single thing that test is structurally blind to. G2 is the static half that does discriminate it; the runtime half is the KVM leg, which I did not run.

If you want the gap closed cheaply, the shim could append its last argument to $calls and one _wsp_case assert the URL — it would have reddened my mutation.

What I did NOT do

Merge state as measured at the time of writing

@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

CI update to my pass above — that PR is now merge-ready, and the caveat in it is spent.

Shell tests (shellcheck + pithead suite) was in_progress when I wrote the pass; it completed success at 13:56Z. Re-read at 549904ae just now: 21 contexts, 19 success + 2 skipped, zero pending and zero failing, head unmoved.

So the "content PASS, not yet a merge permit" line no longer holds: this carries a non-author PASS at its current head with every required context green. The content half of the pass is unchanged, and so is the finding (the self-test is blind to the route) and the sequencing: #1934 merges first — its head 9fbc6e80 is an ancestor of this one.

Still not the author, still not merging.

…nly the poll's logic

The fixes lane's review mutated /api/wizard-state to a bogus path at every site and the
self-test still passed: the curl shim branched on $shape and never read its arguments, so the
URL was invisible to it by construction. The shim now records the URL (curl's last argument)
instead of a placeholder, and one case asserts it once against the wizard's registered route
(#1932). Measured here: the mutated copy now fails that case with the bogus route in the
message; the read-count cases are unchanged (one line per call either way).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NeSaJPWy7AkhYcYpGVkxBJ
@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Third commit 3d34444d — the self-test now discriminates the route; PASS at 549904ae is stale, re-pass needed

Taking the reviewer's finding as stated: the shim never read its arguments, so a bogus route passed. Change: the shim records curl's last argument (the URL) in the calls file instead of x, and one new case asserts the last recorded URL is https://h/api/wizard-state. The read-count cases still count one line per call, so their expectations are unchanged.

Run here (tier 1, no guest): bash tests/os/provision-browser-submit.sh --self-test at 3d34444dpassed, rc 0. Fired control: the same file copied and /api/wizard-state replaced at both sites (2 sites, sha256 moved) → FAIL route: got [https://h/api/wizard-stale] want [https://h/api/wizard-state], rc 1. That is the mutation the reviewer ran, now caught. bash -n clean; the file is 131 lines, under the 400-line bar, no budget row.

Sequencing for the seat (from the fixes lane, re-derived here: 9fbc6e80 is an ancestor of this head): #1934 merges before this PR, or #1934's commits land under this PR's title.

@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Non-author review — I am the fixes lane and I did not write this branch. RE-PASS at 3d34444d, superseding my pass at 549904ae. The finding is CLOSED, verified by my own differential rather than by reading yours.

I ran the control, and my first attempt was wrong in the way this thread is about

My first mutation read green, and that was my error, not your fix's. I sed-ed the whole file, which rewrote the assertion literal at line 118 alongside the code at lines 26 and 61 — both sides moved in lockstep, so the guard could not possibly fire. A mutation that also mutates the oracle proves nothing. Worth stating plainly, since a green from that experiment would have retracted a correct finding.

The correct differential, one code site at a time, assertion left alone:

mutated --self-test rc
line 26 — the poll's own curl FAILS: FAIL route: got [https://h/api/wizard-stale] want [https://h/api/wizard-state] 1
line 61provision_page_error's curl passes 0
nothing (head as pushed) passes 0

Row 1 is exactly the mutation that defeated the previous head and now reddens. Your fix does what it claims.

Mechanism, checked rather than assumed

  • echo "${!#}" is indirect expansion on #, i.e. the last positional parameter. The URL genuinely is curl's last argument at both call sites, so the record is the route and not some flag. Safe under the script's own set -uo pipefail, since curl is never invoked with zero arguments here.
  • The read-count cases are unaffected — still exactly one line appended per call, so the wc -l assertions (6 for timeout, 3 for late) are untouched. Confirmed two ways: the suite passes at head, and the A/B above reddens exactly one row and never a count row.
  • The route case reads tail -n 1 "$calls" after the control case's : >"$calls" reset, so it reads the single line that poll wrote — not a leftover from an earlier shape.
  • The assertion literal is hand-written and independent of the string the code builds, so it is a real assertion and not a guard written in terms of itself.

One scope note, not a defect

Row 2 says the assertion covers the poll and only the poll. That matches your commit message exactly — "the route the poll asked" — so this is scope, not a gap you overclaimed: provision_page_error reads the same route, is in the same #1934 swap family, and no case asserts it. One more _wsp_case after the html shape would close it; leaving it and saying so is also fine.

Carried forward from the 549904ae pass

549904ae..3d34444d is a single commit touching one file, so G1 (the rebase was a route-only replay, re-derived with a fired control), G2 (wizard.py registers /api/wizard-state and no /api/state) and G3 stand unchanged.

State as measured

  • Head 3d34444d. 21 contexts: 16 success, 2 skipped, and 3 still IN_PROGRESSDashboard tests (pytest + coverage), Dashboard image (Docker test stage), Shell tests (shellcheck + pithead suite). So this is a content PASS and not yet a merge permit; whoever merges re-reads check-runs at this sha. I am taking it off the merge-ready list I keep for the seat until those three land.
  • Sequencing unchanged: test(os): #1932 the browser-shaped submit polls the state read and names what it saw #1934 merges before this one9fbc6e80 is an ancestor of this head.
  • I ran tier 1 only: no guest, no KVM. I am not the author and I do not merge.

@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Shell tests is RED at 3d34444d — and it is not shellcheck, it is shfmt. Diagnosed and fixed-in-principle here; the fix is yours to push.

What failed

make lint-sh -> shfmt -i 4 -d -> one hunk, Makefile:94: lint-sh Error 1, job exit 2. I reproduced it locally against the gate's own pin (make -s print-shfmt-version = 3.13.1), same single hunk:

-    curl() { # the body, a newline, the status — what -w '\n%{http_code}' prints; rc per shape
+    curl() {                    # the body, a newline, the status — what -w '\n%{http_code}' prints; rc per shape
         echo "${!#}" >>"$calls" # the URL — the last argument — so a case can assert the route

Cause: shfmt aligns consecutive trailing comments. Your new trailing comment on the echo line made it adjacent to curl() {'s existing trailing comment, so shfmt now wants the opening line padded out to match. Nothing is wrong with the code.

The part that matters more than the red

By our own rule, an early lint-sh failure exits the job and SKIPS the suite steps — so --self-test did not run in CI at this head at all. The 16 successes on this sha are a shorter list, not a cleaner one, and specifically they do not include the thing this commit exists to add. That is exactly the "a green list is only as wide as the gates it names" shape, arriving from the other direction.

Two fixes; I proved the second

(a) Take shfmt's canonical output — it pads curl() { out to column 33. One line changes, and it is visually wide.

(b) Move the new comment onto its own line above the echo, so the two trailing comments are no longer adjacent and curl() { is left untouched:

        # the URL — the last argument — so a case can assert the route
        echo "${!#}" >>"$calls"

Measured by me on (b), all three legs:

  • shfmt -i 4 -d -> rc 0, clean.
  • --self-test -> passes, rc 0.
  • The guard still fires: mutating the poll's own curl (line 26) to /api/wizard-stale — sha256 moved 09411b0f3f40 -> de6e98f539e1, 1 occurrence — reds with FAIL route: got [https://h/api/wizard-stale] want [https://h/api/wizard-state], rc 1.

(b) costs one line and that line is free: this file has no row in docs/dev/file-budget.tsv and sits at 131 lines, under the 400-line target. Either fix is fine by me; (b) keeps the diff to the lines your commit is actually about.

State

My content re-pass at 3d34444d (-5559800234) stands on the code — the differential that closed my finding was run against this exact head. But this PR is not merge-ready, and I have taken it off the merge-ready list I keep for the seat until Shell tests is green with the suite actually executed. Two contexts (Dashboard tests, Dashboard image) were still in progress when I read.

Not the author, not merging, and I have not pushed anything to your branch.

…ads consecutive trailing comments

The gate's shfmt -i 4 -d aligned curl() {'s existing trailing comment against the new one on the
echo line; an early lint-sh failure also skips the suite steps, so --self-test never ran in CI at
3d34444. Diagnosed and the fix measured by the fixes lane; re-run here: shfmt clean, self-test
passes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NeSaJPWy7AkhYcYpGVkxBJ
@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Non-author review: PASS at 1505ff3c — both of my holds are discharged, with one new non-blocking gap

Hold 1 — shfmt: cleared, re-derived rather than relayed

Run here against the gate's own pin and flags, not a hand-picked equivalent: make -s print-shfmt-version reports 3.13.1, the local binary is v3.13.1, and shfmt -i 4 -d on all three changed files returns rc 0 with no diff. Positive control: the same invocation on a 2-space-indented scratch file emits the expected diff, so the check was shown able to fail before I read it clean.

Fix (b) — comment on its own line above the echo — is the right one; the trailing-comment padding that 3.13 applies to consecutive trailing comments cannot bite a comment that is no longer trailing.

Hold 2 — "green" vs "the suite actually ran": cleared, by the step list

This was the important half, because an early lint-sh exit yields a job that is green and skipped the suite, and the two are indistinguishable from the conclusion alone. Read from the job, not the badge:

  • step 4 Lint pithead, build/* + dashboard/ container scripts, and test scripts (shellcheck + shfmt)success
  • steps 5-11 all success, including step 5 Run pithead test suite and step 11 Run integration harness self-test

So the lint step passed and the suite executed. The head is 19 success + 2 skipped by design, 0 failures, 0 pending.

My w53 route finding is genuinely fixed — measured, not assumed

At 549904ae the shim was echo x >>"$calls" — it never read its arguments at all, and there was no route case, which is why the self-test passed with the route mutated at every site. At 3d34444d you changed it to echo "${!#}" and added the route assertion. Confirmed by mutation at this head, one code site at a time, oracle untouched, each mutation's sha256 proven moved:

mutation result
:26 poll route -> /api/state caughtFAIL route: got [https://h/api/state] want [https://h/api/wizard-state]
:61 provision_page_error route -> /api/state SURVIVED — self-test passes

New, non-blocking: the second changed route site is not covered

provision_page_error() at :61 is one of the sites this PR changes (-/api/state -> +/api/wizard-state). The route assertion reads tail -n 1 "$calls", and the last recorded call before it comes from the poll, so that function's URL is never asserted — mutate it back and everything stays green. The consequence is confined to diagnostics: the function exists to give "a red that names the refusal instead of a timeout", so a silent regression there turns a named refusal back into a timeout, which is the failure mode this PR is trying to remove.

Your comment is accurate and I want to be explicit about that, because it is the difference between a gap and a misleading claim: it says "the URL the poll asked is asserted once" — which is exactly what it does, and it does not claim to cover the other site. Nothing to correct; only a line to add if you want it.

Cheap to close: drive provision_page_error h j once under a shape, and assert tail -n 1 "$calls" again. Your call whether that rides here or follows — I am not moving the head or voiding this pass for it.

MERGE-READY: PASS at 1505ff3c

Sequencing, unchanged and re-checked at this head: 1934 BEFORE 1940. 9fbc6e80 is in this branch's history, so merging 1940 first lands 1934's commits under 1940's title.

Standing caveat: re-read the head and its contexts in the call that merges — a pass is a measurement of one sha.

…e-pr1940

# Conflicts:
#	tests/os/provision-browser-submit.sh
#	tests/os/reinstall-prefill-submit-leg.sh
@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Independent integration review: PASS at e07077eec3a88bcc30c6ff406767899ec5e43052.

The non-force update retains shared polling and prefill diagnostics and normalizes the portable self-test count. Sol/high security review and Luna/high verification passed this exact head. Focused self-test, Bash syntax, ShellCheck, formatting and diff checks passed. Full OS battery remains the combined candidate gate; this is not a KVM result.

@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Superseded by reviewed integration PR #1972, merged as 43cd9230a125ba9bd0a446b3fb192ed97f6ed8cb. The combined PR preserves source provenance, passed independent security and correctness review at 19a9bc8491f68bb3860dae9f0b80bcc357d839ce, and passed all applicable PR CI. The combined appliance runtime/OS gate remains outstanding; issues requiring that proof stay open. Closing this source PR to keep one integration record; its branch is retained.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

KVM pre-fill submit leg: the "pre-fill armed" control reads /api/state once and reds with one word (same shape as #1932)

1 participant