test(os): #1936 the pre-fill control polls the state read and names what it saw - #1940
test(os): #1936 the pre-fill control polls the state read and names what it saw#1940VijitSingh97 wants to merge 9 commits into
Conversation
…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
|
Non-author review — I am NOT the author of this PR. Ephemeral VERDICT: PASS at
|
| mutant | result |
|---|---|
M1 x5s→x4s 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 -rc→jq -c at :30 |
rc 1, 2 red (wallet, control) — the raw-string half is pinned |
M8 jq -rc→jq -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 60→head -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:2010callsprovision_browser_submitinside$(…), a subshell, so the globals never escape there; the leg calls the poll in the same shell, which is exactly what:66needs. Both call sites are right for opposite reasons.- The helper's
local … tries=0(:23) shadows the leg's owntries(:22, live at:45-48) and restores it on return. The leg also resets at:82. No leak either way. run.sh:41isset -uo pipefail; both globals are assigned unconditionally at:24, so${WIZ_STATE:-none; $WIZ_STATE_WHY}at leg:69cannot 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 (.error → x) 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
- test(os): #1932 the browser-shaped submit polls the state read and names what it saw #1934 is still OPEN at
f2684e4c, this PR carries two commits, andmergeStateStatusisBEHIND. Squash-merging test(os): #1936 the pre-fill control polls the state read and names what it saw #1940 today lands test(os): #1932 the browser-shaped submit polls the state read and names what it saw #1934's content under KVM pre-fill submit leg: the "pre-fill armed" control reads /api/state once and reds with one word (same shape as #1932) #1936's title. - Worse, it would land it without closing KVM provision phase: the browser-shaped submit (#1847) gets no served config from the wizard, and the harness cannot say why #1932:
closingIssuesReferencesfor this PR is[#1936]only (measured), and test(os): #1932 the browser-shaped submit polls the state read and names what it saw #1934'sCloses #1932lives in its body, which does not travel into a squash of this one.Closes #1936here WILL fire — base is the default branch. tests/os/is a freeze path. Not mine to merge either way.
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.
…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
|
Non-author review (ephemeral G1 — the 60-byte bound is now genuinely exercised, and I have the pair that proves itRun at head: Mutation, with the diff printed before the run so the mutation is proved applied — The leg that makes this evidence rather than a demonstration: the same mutant at 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
G3 — the body's line figures, re-derived at source
All four match. The delta is confined where the ask saidOne commit, one file, Nit, non-blockingThe reflow at On the body's over-engineering passI agree with it, and one item is stronger than the body states. "The shim's call counter is a file: Not merge-eligible, independent of this verdict
Verdict: PASS at |
…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
963f234 to
549904a
Compare
Rebased onto #1934's head
|
|
Non-author review — I am the 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
Your claim stands as stated. G2 — the swap is correct and load-bearing. At The finding — the evidence line is TRUE and narrower than it readsThe body cites
This is not a defect in the self-test. It is a poll-logic test and a good one — six-read retry, the exact If you want the gap closed cheaply, the shim could append its last argument to What I did NOT do
Merge state as measured at the time of writing
|
|
CI update to my pass above — that PR is now merge-ready, and the caveat in it is spent.
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 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
Third commit
|
|
Non-author review — I am the I ran the control, and my first attempt was wrong in the way this thread is aboutMy first mutation read green, and that was my error, not your fix's. I The correct differential, one code site at a time, assertion left alone:
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
One scope note, not a defectRow 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: Carried forward from the
|
|
What failed
Cause: The part that matters more than the redBy our own rule, an early Two fixes; I proved the second(a) Take shfmt's canonical output — it pads (b) Move the new comment onto its own line above the # the URL — the last argument — so a case can assert the route
echo "${!#}" >>"$calls"Measured by me on (b), all three legs:
(b) costs one line and that line is free: this file has no row in StateMy content re-pass at 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
Non-author review: PASS at
|
| mutation | result |
|---|---|
:26 poll route -> /api/state |
caught — FAIL 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
|
Independent integration review: PASS at 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. |
|
Superseded by reviewed integration PR #1972, merged as |
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
tests/os/provision-browser-submit.sh: the six-reads-5-s-apart loop test(os): #1932 the browser-shaped submit polls the state read and names what it saw #1934 added becomes one helper,wizard_state_poll <ip> <jar> <jq-filter>, that both consumers call. On a hitWIZ_STATEholds the value (raw string or compact JSON) and the return is 0; otherwiseWIZ_STATE_WHYnames what the last read saw:http=<code> curl=<rc> after <n>x5s body=<first 60 printable bytes>.provision_browser_submitcalls it for.config; its reason line and its contract torun.share unchanged.tests/os/reinstall-prefill-submit-leg.sh: the "pre-fill armed" fixture-works control calls the same helper for the served wallet. Its red now names the wallet it saw, or, when no read carried one, what the reads saw (served wallet: none; http=… curl=… after 6x5s body=…), instead ofserved wallet: nonefor a timeout, a 401 and a non-JSON page alike.--self-testin the helper file (the pithead backup fails on the appliance: tar cannot stat config.json, after the stack has already been stopped #1059 / tests/os: battery leg 4 cannot age a x.y.0 running version, so the release gate reds at every release-prep tip (seven reds for one cause) #1676 shape) and its driver stanza intests/stack/test-harness-tooling.sh(a_shared.pathsfile, disclosed here; it grows 369 → 379 lines, under the 400 target, no budget row).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 ofWIZ_STATE, andprovision_browser_submit's end-to-end reason. The last case is the control that the shim answered, not the real curl.x5s→x4sin the reason reddens three checks (timeout, noconfig, html) and the self-test returns 1.casepattern 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.run.sh(. tests/os/provision-browser-submit.shdefines both functions and runs nothing).bash -n,shfmt -i 4 -d,shellcheck -x -S warningfrom the repo root over all three files: clean.scripts/lint-file-budget.sh: OK (provision-browser-submit.sh: 39 lines atorigin/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
pitheadover SSH pulls from the org's registry and the stack cannot come back up #1931 positive control and the RC1 tree's reset leg. The next battery's install phase is the check of the leg's line; the self-test is the check of the reason.tests/stack/run.shnot run locally (no per-domain selector; the KVM leg holds the box). CI's Shell tests job runs the tooling domain, one row longer.Over-engineering pass (by hand; the PR-gate hook keys off the wrong branch from this lane's cwd)
provision_page_errorkeeps its single read: its two callers (run.shhandoff poll, the leg's) already poll it inside a 24×5 s loop, and an empty answer is a real answer there.$(…)cannot also print a reason.run.shalready hands the leg its state as globals (ip,SERIAL).tests/stackfile: the shapefailure-evidence.shandaged-version.shuse, and a new stack file is gated bytests/inventory.sh.curlruns 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