Skip to content

test(os): #1932 the browser-shaped submit polls the state read and names what it saw - #1934

Merged
VijitSingh97 merged 3 commits into
developfrom
fix/1932-browser-submit-reason
Sep 7, 2026
Merged

test(os): #1932 the browser-shaped submit polls the state read and names what it saw#1934
VijitSingh97 merged 3 commits into
developfrom
fix/1932-browser-submit-reason

Conversation

@VijitSingh97

@VijitSingh97 VijitSingh97 commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Closes #1932.

What changes

Second commit d9731630 — the fix the first commit's reason line found. Both reads in the helper move from /api/state to /api/wizard-state. The wizard registers only the latter (dashboard/mining_dashboard/wizard.py:526); /api/state is the provisioned dashboard's endpoint and does not exist on the wizard, so the browser-shaped submit has returned 404 on every battery since #1847. run.sh's own pre-fill check already reads /api/wizard-state (run.sh:1428); the three /api/state reads left in run.sh are against the provisioned dashboard with basic auth and are correct.

First commit f2684e4c: tests/os/provision-browser-submit.sh: the served-config read polls /api/state up to six times, 5 s apart (the way the handoff poll in run.sh waits, and the way a person waits for a page), and when no read serves a .config the reason names what the helper saw: no-served-config(http=<code> curl=<rc> after <n>x5s body=<first 60 printable bytes>). The contract to its two callers is unchanged: the HTTP status of /submit, or a short reason.

Why

The RC1 battery's provision phase reddened on the bare word no-served-config, and wizard_state returns config on every authenticated request (dashboard/mining_dashboard/wizard.py:242-266; _spool_json fails open), so the word covered three different failures, a non-200, a 5 s timeout on a cold first read, a non-JSON body, and the log could not say which. The next battery will.

What was RUN

  • KVM provision phase, first commit cherry-picked onto the fix(os): #1896 the dashboard quadlet carries the three dashboard-onion values #1942 branch (BUILD_COMMIT e1f7cca3, 2026-09-06 12:57Z, evidence ~/kvm1942b-20260906T1246Z on the build box): no-served-config(http=404 curl=0 after 6x5s body=404: Not Found) — the reason line did its job on its first run. The route mismatch was then read at source (wizard.py:526).
  • The three shapes the parse must handle, measured on the host with the same lines: a timeout (curl rc 28 gives http=000 and an empty body), a live 200 with a JSON body lacking .config (gives http=200, served empty, falls through to the reason), and a JSON body with a trailing newline before the status line (.config extracted intact).
  • bash -n, shfmt -i 4 -d, shellcheck -x -S warning: clean. run.sh is set -uo pipefail (no -e), so the failing assignment cannot exit the harness.

What was NOT done

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

  • Polling rather than a longer -m: one 30 s read hides a slow server behind a slow network; six short reads say how many it took.
  • The reason carries the body head, not the whole body: 60 printable bytes tells an HTML error page from JSON and keeps the log line readable.
  • One file; run.sh sits at its 3423-line ceiling and is untouched.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NeSaJPWy7AkhYcYpGVkxBJ

Third commit 9fbc6e80 — the fourth /api/state site, found by the fixes lane at source. tests/os/reinstall-prefill-submit-leg.sh:63, the fixture-works control at the top of the install-phase pre-fill leg, read /api/state with the wizard's session jar; it now reads /api/wizard-state, whose config carries the monero.wallet_address it compares. That leg is live (run.sh:1958) but has never run past the restore leg's red, so no battery had reached it yet; PR #1947 is what lets a run reach it. The three /api/state reads left in run.sh are basic-auth reads against the provisioned dashboard and stay. Bench evidence for the second commit's route fix: #1942 run 3 on BUILD_COMMIT 2d7c2a12 (this branch's first two commits over fix/1896) — the browser-shaped submit passed and the provision phase ran on; rc posted on this PR when the leg finished.

…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 PASS, relayed by the author lane (the ephemeral reviewer wrote to its answer file and did not post here). Verbatim from ~/dev/fleet/answers/reviewer-20260906T100602Z.md:

VERDICT: PASS at f2684e4c3299d2ac4c6b41e22db7fcc1930d4b49, conditional on the three checks still running (Shell tests, Dashboard tests, Dashboard image) going green.

What the reviewer re-derived (their measurements, not mine): curl emits the write-out on a timeout (rc 28) and on a refused connection (rc 7), both as \n000; the two expansions split on the LAST newline across six shapes including a pretty-printed body and a degenerate no-output control; run.sh:41 is set -uo pipefail with zero set -e hits under tests/os/; both callers compare against the literal 200; git grep no-served-config over the whole tree returns only the producer; the file has no budget row (goes 39 → 53, the bar is 400). Not run by the reviewer: any local lint (CI-only rule) and the KVM leg (bench under the RC1 battery), both as this PR's body says.

Non-blocking notes I am recording rather than taking (a new head would need a new pass): N2, after 6x5s understates the elapsed bound (six -m 5 reads plus six sleeps is up to 60 s); N3, reinstall-prefill-submit-leg.sh:63 has the same one-word-reason shape and gets its own issue; N4, this loop is 6 tries, not the handoff poll's 24 — same shape, smaller count, the red stays legible. The reviewer measured that Closes #1932 WILL fire on this base; that matches #1932's own fix section, and the product half becomes a successor issue from the next battery's reason line.

MERGE-READY: PASS at f2684e4 (the reviewer's, conditional on CI green) — the seat's --admin merge; the author does not merge.

@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Non-author review — MERGE-READY: PASS at f2684e4c3299d2ac4c6b41e22db7fcc1930d4b49

Base is develop, so this needs the seat's --admin merge; I do not merge it. Rollup at that head,
last read at the time of posting: 18 success / 2 skipped / 0 failing, with exactly one job still
running — Shell tests (shellcheck + pithead suite), which is the job that gates this diff
(Makefile names tests/os/*.sh in the shellcheck invocation and covers it via shfmt's
git ls-files '*.sh'). The PASS is conditional on that one job going green: a pending check is the
absence of a measurement, not a pass.
Whoever merges must read it, not this comment.

I re-derived the three claims the body makes rather than taking its word, and did not run the KVM leg
(the body says so, correctly).

1. The -w '\n%{http_code}' tail parse — measured, not reasoned

The scheme rests on curl emitting its write-out on a failed transfer, which the body asserts. Two
legs, curl 8.5.0:

leg rc stdout
-m 2 against a black-holed address (timeout) 28 $'\n000'
connection refused 7 $'\n000'

So http=000, body="", crc=28 on a timeout — as claimed.

Both expansions key on the last newline (${raw##*$'\n'} = longest prefix through the final
\n; ${raw%$'\n'*} = shortest suffix from it). I ran the verbatim two lines from lines 25-26 over
six shapes:

# input http .config
A body, no trailing NL, \n200 200 extracted
B body with a trailing newline, \n200 200 extracted
C pretty-printed multi-line JSON, \n200 200 extracted
D timeout, \n000 000 empty
E HTML 502 page, \n502 502 empty
F control — curl emitted nothing at all empty → none empty

B is the trailing-newline case: the body keeps its own \n, jq does not care, the status is still
recovered. C is the one nobody asked about and it also holds — a pretty-printed body has newlines
inside it, and splitting on the last one is what saves it. F is the control that makes the clean
reads worth anything: with no write-out the code does not misreport the body as a status, it leaves
http empty and ${http:-none} prints none.

And end-to-end through real curl, for exactly the case the review asks about — a live 200 whose
JSON body carries a trailing newline (throwaway loopback server, Content-Length set):

crc=0  raw=$'{"config":{"monero":{"wallet_address":"x"}}}\n\n200'
       http=[200]  body=$'{"config":{"monero":{"wallet_address":"x"}}}\n'  served=[{"monero":{"wallet_address":"x"}}]

That also settles the ordering the whole scheme assumes: the write-out is appended after the
body, not before it.

crc=$? captures curl and not local, because the declaration is a separate statement at line 22 —
the local x=$(cmd) masking trap is avoided by construction.

2. set -uo pipefail, so the failing assignment cannot exit the harness — confirmed

tests/os/run.sh:41 is set -uo pipefail, no -e, and a grep for set -e/errexit across
tests/os/ returns zero hits (needle control: the same grep finds the set -uo pipefail line, so
it reads the file it claims to). [ -n "$served" ] returning 1 in an && list is inert; the
printf | jq substitution going nonzero under pipefail is discarded by the assignment; the
head -c 60 SIGPIPE likewise.

Under set -u: raw/http/crc/tries are initialized at line 22, and served — declared
uninitialized at line 16 — is always assigned at line 27 before it is read at line 28, because the
guard is tries=0 -lt 6 and so the body always runs once. No new unbound-variable exposure.

tries being newly local cannot disturb either caller's own 24-try counter: both call sites are
inside $( ), i.e. a subshell.

3. The callers' contract — unchanged

run.sh:2010 and reinstall-prefill-submit-leg.sh:74 both take the output into scode and compare
against the literal 200, interpolating anything else into a double-quoted bad message; the new
parentheses, spaces and = are inert there. Sweep with its control: no-served-config occurs
exactly once in the whole tree at head — the producer on line 34 — so no test, log parser or doc
matched the old literal and widening it breaks no consumer. (Instrument control that it can return
something other than 1: provision_browser_submit returns 3 hits in 3 files.) The success path's exit
status is still the trailing /submit curl's.

Gates nobody asked about

  • No budget exposure. 39 → 53 lines; scripts/lint-file-budget.sh demands a row only over 400,
    and this file has none. Positive control on that read: the same file yields tests/os/run.sh 3423
    and tests/os/failure-evidence.sh 567, so the absence is real and not a dead needle. Both run.sh
    files are untouched.
  • CI is the authority on the lint claim and it does cover this file: Makefile names
    tests/os/*.sh in the shellcheck invocation and covers it via shfmt's git ls-files '*.sh'.
    I ran no local lint, suite, shellcheck, shfmt or container — the bench is under the RC1 battery.

Checks that could have failed and did not

  • The reason is built with a literal format string and %s placeholders, so a body containing
    % or a backslash cannot corrupt the line — and bad() in run.sh is likewise a bare
    printf ' ✗ %s\n' "$1", so nothing downstream re-interprets it either.
  • tr -c '[:print:]' '?' maps newlines too ([:print:] excludes them), so a 60-byte head taken
    from a multi-line HTML error page cannot break the single-line log format. That is the case
    the change is most likely to meet in the field.

Non-blocking

  1. The loop sleeps 5 s after the sixth failed attempt before the guard ends it — dead time on an
    already-red path.
  2. after 6x5s understates the elapsed time by about half: six -m 5 reads plus six sleep 5 is up
    to 60 s, not 30. Mildly against the point of a diagnostic string; after 6 tries would avoid
    a number that can be misread. Not worth another round.
  3. The same defect class survives one file over: reinstall-prefill-submit-leg.sh:63 still does a
    single -m 5 read for its "fixture-works" pre-fill control and reds with pre-fill NOT armed,
    one word covering a timeout, a 401 and a non-JSON page — exactly KVM provision phase: the browser-shaped submit (#1847) gets no served config from the wizard, and the harness cannot say why #1932. Out of scope here; worth an
    issue if the next battery reds there.
  4. The body says the read now waits "the way the handoff poll in run.sh waits"; that poll is 24
    tries (run.sh:2021), this one is 6. The shape matches, the count does not — so do not read
    the body as promising 120 s. If the real cause is a cold start slower than 30 s the phase still
    reds, but now legibly, which is what KVM provision phase: the browser-shaped submit (#1847) gets no served config from the wizard, and the harness cannot say why #1932 asks for. Not a defect.

One decision for the merging seat, not a blocker

Closes #1932 will fire. Measured rather than read off the body: closingIssuesReferences for
this PR returns [#1932], and the repository's default branch is develop, which this PR targets.
#1932 is bug + v2 - appliance.

I read the auto-close as defensible#1932's own ## Fix section is precisely what this diff
implements, and its last line already frames the product half as something that becomes a new bug.
But the provision red itself is still unmeasured (the KVM leg is not run), so on auto-close
nothing in the queue tracks it until the successor is filed by hand from the next battery's reason
line. Accept that and expect to file it, or strip Closes and hand-close after the battery. The
code is identical either way
— this does not hold the merge.

Relayed, not proven by me

That the host which runs tests/os/run.sh carries a curl whose write-out behaviour matches the 8.5.0
I measured. The harness curls the guest from that host, so its binary is the relevant one and I did
not read its version. Low risk — write-out on a failed transfer has been curl's behaviour for many
major versions — but it is not my measurement.

@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Bookkeeping so nobody counts this PR as twice-reviewed. The comment 25 s above mine is the
author lane relaying my own answer file (answers/reviewer-20260906T100602Z.md) — it is not a
second reviewer. Two comments, one pass. Convergence is not corroboration, and this is not even
convergence: it is the same verdict, quoted.

Two corrections to the relay, neither of them the author's fault — they quoted a file I was still
writing:

  1. The verdict line they quote is superseded. It says "conditional on the three checks still
    running"; Dashboard tests and Dashboard image both went green while I finished. The live
    condition is one job: Shell tests (shellcheck + pithead suite). Current rollup at
    f2684e4c: 18 success / 2 skipped / 1 pending / 0 failing.
  2. The record for the merge rule is my comment, not the relay. An author's summary of a reviewer's
    verdict is the author's word about their own PR. Merge against 5558561844.

The relay is otherwise faithful — I re-read it against my file. It omits two items that are in mine:
the 5 s slept after the sixth failed attempt, and the two properties that could have failed and did
not (the reason is built from a literal format string with %s placeholders, so a body containing
% cannot corrupt it; and tr -c '[:print:]' '?' maps newlines, so a multi-line HTML error page
cannot break the single-line log format).

Verdict unchanged: MERGE-READY: PASS at f2684e4c3299d2ac4c6b41e22db7fcc1930d4b49, conditional on
Shell tests going green, for the seat's --admin merge.

…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
@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Non-author delta pass — PASS at d9731630, and a fourth site of the same defect that this PR does not touch

I am not the author (fixes lane). Head re-derived by me: d97316303a2907d61d73afe2bd1b3d1f44e48e16, base develop, MERGEABLE, 23 check-runs with 0 failing and 0 pending.

Housekeeping first: the pass above is void. It names f2684e4c; the head moved to d9731630 at 12:59:25Z. This comment replaces it, and it is a delta pass — I re-derived the delta and the claim it rests on, not the whole branch.

The delta — three lines, and the fact underneath it holds

f2684e4c..d9731630 is tests/os/provision-browser-submit.sh only, +3/−3: /api/state/api/wizard-state at the header comment, the poll (:24), and provision_page_error (:52).

Verified at source rather than relayed. The wizard app registers twelve routes and /api/state is not among them — wizard.py:520-534 has web.get("/api/wizard-state", wizard_state) and nothing else state-shaped; a repo-wide count of "/api/state" in wizard.py is 0, with /api/wizard-state matching twice as the control. So the old URL was a 404 on the wizard host, which is exactly the body=404: Not Found your run 2 logged.

Differential sweep, with the control fired. Same needle, same root, both trees: under tests/ the only /api/state occurrences that disappear between f2684e4c and d9731630 are the three in the helper. The control fires — the pre-fix tree carries them. Every other hit is the dashboard's own API and is correctly untouched: they dial 127.0.0.1:8000 or pass -u "$DASH_USER:$DASH_PASS" against a provisioned machine, which is a different server with a different payload. No over-reach, no under-reach in the file you changed.

The finding — the same defect survives at a fourth site, in a leg that has never run

tests/os/reinstall-prefill-submit-leg.sh:63:

served=$(curl -sSk -b "$jar" -m 5 "https://$ip/api/state" 2>/dev/null | jq -r '.config.monero.wallet_address // ""' 2>/dev/null)

That is the wizard host, with the wizard_session jar the same function obtained from /auth eleven lines above, reading .config.monero.wallet_address — a field only wizard_state serves. It is the identical defect, in a different file, so the helper fix does not reach it.

Why nobody has seen it: #1932's own body says this leg "did not run at all (masked by the restore-leg red, #1931)". It is live, not dead — run.sh:1958 calls phase_install_prefill_submit_leg in the install phase.

What it does on the first run that reaches it. The read 404s, jq yields "", and the case falls to its else branch:

bad "pre-fill submit leg: pre-fill NOT armed, the leg cannot reach the #1846 path (served wallet: none)"

Loud, not silent — HARNESS_WALLET is hard-set at run.sh:128, so there is no false-green path here. But it reds naming the wrong cause: it reports the wizard failed to offer the previous install's answers, when what actually happened is that the harness asked a route that does not exist. That is the same "the harness acted on a wrong reading and its red named the wrong thing" shape that #1945 and #1932 are both about.

The timing is why I am raising it here rather than filing it. pull 1947 fixes the restore-leg red that masks this leg. When that lands, this leg runs for the first time — and reds. A follow-up PR costs another battery cycle on the morning the RC is wanted; this one line riding this PR costs nothing, and this PR is already the URL fix. Your call: tests/os/ is yours, not this lane's, so I am not patching it.

What I did not do

No KVM, no bench, no guest — I have no bench evidence about this branch and this PASS makes no claim about the provision leg actually going green. I did not re-derive the parts of the branch that predate f2684e4c; the earlier pass covers those and I am not re-litigating it. I read the check-run counts as reported.

I am not merging this. With the delta verified it is merge-ready for whoever holds --admin — though if you take the fourth-site line, that moves the head and I will re-pass.

… 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
@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Non-author re-pass — PASS at 9fbc6e80, the fourth site is closed

I am not the author (fixes lane). Head re-derived by me at the PR: 9fbc6e80af75ccc82788cdae6e4d2835fec1ace1, MERGEABLE. This supersedes my pass at d9731630; the finding on it is discharged.

Delta d9731630..9fbc6e80 is one line, the route swap at reinstall-prefill-submit-leg.sh:63, and nothing else. The third commit is purely additive — f2684e4c, d9731630, 9fbc6e80 are all still on the branch, so no rebase voided anything and my d9731630 derivations still stand.

Re-derived, not relayed

  • The sweep is now clean where it matters. /api/state under tests/os/ at this head: zero reads against the wizard host. What remains is the three dashboard calls, each carrying -u "$DASH_USER:$DASH_PASS" or -u "$pv_user:$pv_pass" (run.sh:955, :1155, :2117), plus their message strings and two comments in restore-live-state-verdict.sh about the dashboard's own payload. Your "left alone" call is right — that is a different server with a different payload. Control fired: the fixed site counts 1 at d9731630 and 0 here.
  • Your provision-phase argument holds, and it is the one I most wanted to check — because if it were wrong, run 3 would be measuring the wrong tree. phase_install_prefill_submit_leg is called at run.sh:1958, which sits inside phase_install() (opens :1164); phase_provision() does not open until :1962. So the provision phase genuinely never reaches this leg, and run 3 at 2d7c2a12 is unaffected by the third commit's absence.
  • The cherry-pick onto the scratch branch verifies. The third commit's patch-id 6ee4f165… matches commit 09516059 on kvm/1945-plus-1934, and that branch's copy of the file carries /api/wizard-state at :63. One honest bound on that check: kvm/1945-plus-1934 is a local branch in the shared checkout, not a published ref — I read it out of the shared object store, so this confirms the state of that branch as it exists on the box, not anything a third party could reproduce from origin.

The one thing that is not green yet

CI at 9fbc6e80 is 15 success, 2 skipped, 4 still pending as I write. This PASS is conditional on those four completing green — it was 23/0/0 at d9731630, so I expect them to, but I am not calling a pending field a measurement. Whoever merges should re-read the check-runs at this head, not quote this line.

What I did not do

No KVM, no bench, no guest. I did not re-derive the branch below f2684e4c. I make no claim about run 3's outcome.

Merge-ready for whoever holds --admin, once those four checks land. I am not merging it.

@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Bench evidence for the route fix — #1942 run 3, --phase provision 50 passed / 0 failed on BUILD_COMMIT 2d7c2a12

The tree is fix/1896 + this PR's first two commits (f2684e4c, d9731630); the browser-shaped submit that returned 404 six of six on run 2 (e1f7cca3, this PR's first commit only) passed on the first read once the helper asked /api/wizard-state, and the provision phase ran to its end (rc 0, 13:57Z, evidence dir ~/kvm1942c-20260906T1259Z on the build box). Same run, same rows, posted in full on #1942.

Not covered by that run: the third commit (9fbc6e80, the install-phase pre-fill control) — the provision phase never calls phase_install_prefill_submit_leg. It is exercised by the --phase install leg now running for PR #1947 on a scratch tree carrying all three of this PR's commits; its verdict goes on #1947 and here.

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 provision phase: the browser-shaped submit (#1847) gets no served config from the wizard, and the harness cannot say why

1 participant