fix(dashboard): #1857 an un-adopted rig is badged "not adopted" and pointed at Adopt, not at workers.api_auth - #1948
Conversation
…ointed at Adopt, not at workers.api_auth
One probe verdict was answering two questions. A rig whose configured xmrig
feed broke and a rig the dashboard was never given a control token for both
reached the Workers table as `api_ok: False`, so a freshly provisioned
appliance rig — mining fine, simply not adopted yet — got the red `api ⚠`
badge telling its operator to go and check `workers.api_auth` / `api_port`.
The probe now emits `adopted` alongside `api_ok`, decided where the
`workers.list[]` descriptor has already been resolved. That placement is the
point: the probe's own name-then-host match already handles a `+suffix`
stratum name and a rig renamed behind an operator-set `host`, both of which a
name-only lookup further downstream would read as un-adopted. The masked
`{"__secret__": true}` token the container actually sees (#440) still counts
as adopted. The field rides both verdicts, so it cannot flip between polls.
The badge branches on it: adopted keeps the red `api ⚠` and its config advice;
un-adopted gets a neutral `badge-outline` "not adopted" — no new CSS class —
whose tooltip branches on the same `onInspect` condition the row's name button
does, because with dashboard control off there is no Adopt form to open. With
control on it renders as a button, like RigUpdateBadge, so the tooltip that
carries the instruction is keyboard-reachable. Wording per the design lane's
ruling on the issue, which corrected the issue's own proposed tooltip.
All four source files sat at their file-budget ceilings, so every edit is
line-neutral; the `bytearray`/short-read rationale trimmed from
`xmrig_client.py` is a duplicate of the live copy in `helper/http.py:105`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KZuzdtxNX8ae4rnDtBqEqR
…n style (MD049) `make lint-md` was not in the five gates I ran; CI's per-surface job runs ten targets, and MD049 reddened on the `*adopted*` I added to configuration.md. The rule is `consistent` — per FILE, not repo-wide — so configuration.md takes underscore and workers.md keeps asterisk. Correcting both to one style is the wrong fix and reddens the other file; measured both ways. Ran CI's exact command afterwards: make lint-docs-voice lint-operator-strings lint-topology lint-file-budget lint-trivy-parity lint-js lint-yaml lint-md lint-proto lint-toml -> rc 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KZuzdtxNX8ae4rnDtBqEqR
Non-author review pass — PASS at
|
Non-author review: PASS at
|
Wording ruling 2 for #1857 / PR #1948 — the unlisted-rig-with-a-real-fault class (design lane, Fable 5.1)The reviewer's finding stands; I re-derived it at the PR head The label stays 1.
|
…doption alone The design lane's second wording ruling on #1857. The first shipped a tooltip and a docs row promising that adopting the rig is what fixes it. `workers.list` defaults to `[]`, so on a stock fleet no rig is adopted and a hand-configured miner with a genuine API fault lands in the same branch — for that rig adopting changes nothing (`buildAdoptedConfig` writes no `api_port`, and a masked token falls through to the fleet auth mode), and the `api ⚠` advice that would fix it rendered for nobody. That is this issue's own bug mirrored, so it does not ship. The tooltip and the docs row now state the two facts the dashboard actually knows — the probe failed, and it holds no control token — with no causal "until you adopt it", then key each remedy to the one thing the operator knows and the dashboard cannot: who set the rig up. Two more sites carried the same false claim and are corrected here: the `docs/workers.md` upgrade note and the `docs/configuration.md` `workers.api_auth` row both said "every adopted worker shows `api ⚠`", which is true of nobody on a fleet where nothing is adopted. Two sites promised the pre-#1857 "API error" badge for an un-adopted rig and go stale the moment this merges: `rigcardlogic.mjs` (the rig card is the operator's first briefing, so leaving it re-plants the confusion at the source) and `docs/workers.md:300`. `docs/appliance.md:153` is the third and is deferred to #1949 — two other open PRs already modify that file and one is merge-ready on a recorded pass, which a rebase would void. The badge label and its neutral `badge-outline` style are unchanged: the design lane considered a remedy-free label and kept "not adopted" as a true state for every rig in the branch. Tier 2, frontend component tests: the control-ON and control-OFF tooltip tests now both pin the second remedy, which is the whole point of the ruling. What the operator sees change on the appliance: a rig whose xmrig API cannot be read still shows the neutral "not adopted" badge, but its tooltip now also tells a hand-configured miner's owner to check `workers.api_auth` / `api_port` instead of sending them to an Adopt form that cannot help them. RAN: make test-frontend (585 passed), make test-dashboard (2588 passed, coverage 97.57%), make test-fakes (31 passed), and CI's ten per-surface lint targets plus lint-py — all rc 0, read from the rc I captured rather than a pipeline tail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KZuzdtxNX8ae4rnDtBqEqR
…he failure payload
CI's "Fake-daemon contract test" job went red on this branch: `2 failed, 29
passed`. Two contract assertions pin the probe's failure payload by exact dict
equality (`bad == {"api_ok": False}`), and this branch adds an `adopted` key to
every return path in `get_stats` — including the failure ones, which is the
point: the badge needs the adoption fact precisely when `api_ok` is False.
The product is right and the expectation was stale, so the expectations move.
Exact equality is kept rather than loosened to a subset assertion: pinning the
contract is this file's whole job, and a subset assertion would stop it noticing
the next payload change.
`adopted: True` on the second one is not a typo. Adoption is
`bool(override.get("token"))` on the worker's descriptor, and that fixture is
the per-worker-descriptor-token case, so the rig genuinely is adopted — the
assertion now also proves the adoption fact survives the real client against a
real fake, which is what this suite exists for.
LANE OVERRIDE, disclosed: `tests/integration/fakes/test_contract.py` is owned by
the e2e lane (`roles/e2e.paths`), not this one. Granted by that lane after it
re-derived both dict values from `get_stats` and confirmed with its own grep
that these are the only two sites, so nothing here is taken on relay. The commit
is this file alone.
The file is at its file-budget ceiling (451/451) with no headroom, so the edit is
line-neutral by construction: both changes are in-place dict literals and the
line count is unchanged.
Tier 3. RAN: make test-fakes — 31 passed, rc 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KZuzdtxNX8ae4rnDtBqEqR
Non-author review: PASS at
|
Fixes the manual-test finding in #1857.
What the operator will see change on the appliance
A rig provisioned from the wizard mines at once but is not adopted yet, so the dashboard cannot read its xmrig API. Today its Workers-table row carries a red api ⚠ badge whose tooltip sends the operator to
workers.api_auth/api_portand the miner's xmrig http settings. Nothing is misconfigured, so that advice cannot help.After this, that row carries a neutral not adopted badge instead, and its tooltip says what to do:
An adopted rig whose configured feed then fails is unchanged: it keeps the red api ⚠ badge and its config advice, which for that rig is correct.
Mechanism
api_okwas one boolean answering two questions — "the configured feed broke" and "there is no feed to read" — so the row reported the first for both.The probe now emits
adoptedbesideapi_ok, decided inget_statswhere theworkers.list[]descriptor has already been resolved. That placement is the load-bearing choice, not a convenience:+suffixfixed-difficulty stratum name before matching. A name-only lookup further downstream (the shape the issue proposed, and the one_worker_watts_configalready uses) would read arig+50000row and a rig renamed behind an operator-sethostas un-adopted, badging two classes of genuinely adopted rig wrongly.{"__secret__": true}token the dashboard container actually reads (Control channel: move the config merge host-side so the container never mounts plaintext config.json #440) is still a token, so an adopted appliance rig reads as adopted.Adoption is the control token, not mere presence in
workers.list:validate_worker_descriptorrefuses an adopted entry without one. The field rides the success payload too, so it cannot contradict itself between two polls; a worker we deliberately never probe (SSRF guard) claims no adoption verdict at all and serialises asnull, neverfalse.The badge branches on it in a new
ApiBadgecomponent besideRigUpdateBadge. Per the design lane's wording ruling on the issue — which corrected the issue's own proposed tooltip — the label isnot adoptedin the existingbadge-outlinevariant with no new CSS class, and the tooltip branches on the sameonInspectcondition the row's name button does, because with control off there is no name button and no Adopt form, making "open the rig and adopt it" an impossible instruction. I also took that ruling's optional, non-blocking suggestion: with control on the badge renders as a<button>likeRigUpdateBadge, so the tooltip carrying the instruction is keyboard-reachable — atitleon a<span>never is.Budget disclosure
All four source files sat at exactly their
file-budget.tsvceilings (449/449, 431/431, 507/507 and, for the client, 1137/1177). Per the standing rule that the line-neutral question comes first, every server edit is line-neutral and no ceiling is raised. Two of those lines were paid for by compression, both of which I checked are duplicates rather than the only copy:xmrig_client.py: thebytearray/ short-read rationale in thebounded_readcall-site comment is a restatement of the live text athelper/http.py:105, which the comment now points at.infra_views.py: theapi_okthree-state comment is stated at the producer and at the merge.The three natural test files are at their ceilings too (504, 618, 768), so the new tests are a new file rather than a ceiling raise.
Tests
Tier 1 (unit) per
docs/dev/testing-strategy.md.dashboard/tests/test_worker_adoption.py(new, 16 tests) — deliberately cross-layer: the defect is the verdict's journey from probe to row, and splitting it across the three layers' files would leave the joins covered nowhere. Covers both verdicts, the token-less descriptor, the masked-token sentinel, the+suffixand host-rename resolutions, the never-probed case, andnull-not-falseserialisation.dashboard/tests/frontend/workerapibadge.test.mjs(new, 6 tests) — rendered throughAppon the realbuild_statefixture, via the existingharness.mjs, which exists for exactly this (components.test.mjsis at its ceiling).test_xmrig_client.py,test_summary_size_cap.pyandcomponents.test.mjs, and addedadoptedto the render fixture, since the payload shape changed.What was RUN
make test-dashboard: 2588 passed, coverage 97.57% (gate 80%).make test-frontend: 585 passed, 0 failed; all six new tests confirmed present by name in the TAP output.make lint-py,make lint-js,make lint-docs-voice,make lint-operator-strings,make lint-file-budget: all rc 0, run after staging so the new untracked files were in scope.adoptedDocs
docs/workers.mdanddocs/configuration.mdboth stated that a failed probe showsapi ⚠, which is now true only for an adopted rig; both corrected, house voice,lint-docs-voicegreen.docs/is in_shared.paths, disclosed per lane policy. Nodocs/dashboard.mdchange: it does not describe this badge.Not done / limits
applianceholds the bench; per this lane's test discipline this is pytest + node only. The wizard-screen tier-4 leg does not render this surface either, so the visual check is the operator's on the RC image.api ⚠wording itself. The design ruling flagged, as inference it had not checked, that if Appliance rig role: generate an access token at provision, enable RigForge's feed + control pinned to the coordinator, and show the token once on the rig card — today every appliance rig is an 'API error' row and its XMRig API is open on the LAN #1836's adopted feed is read through the control port rather than xmrig's HTTP API thenworkers.api_auth/api_portmay name the wrong keys for an appliance rig. That is Appliance rig role: generate an access token at provision, enable RigForge's feed + control pinned to the coordinator, and show the token once on the rig card — today every appliance rig is an 'API error' row and its XMRig API is open on the LAN #1836's mechanism, not this PR's, and I did not verify it either.origin/developat its current tip and re-checked the ref twice; nofixesPR is in this diff's files.make lint-shnot run (shellcheck OOMs the box, and this diff touches no shell).The issue is hand-closed after merge; this body uses no closing keyword.
Over-engineering pass (done on merits, findings noted)
Nothing found that warranted a code change; three things I decided rather than defaulted, so a reviewer can overrule any of them:
w["api_ok"], w["adopted"] = ..., and the same on the success payload) exist for line economy, not clarity. Separate statements would read better and cost one line each in files with zero headroom. This is the ratchet's cost, and it is the one place in the diff where the budget shaped style rather than design.bool(extra_stats.get("adopted"))in the merge normalises a missing field toFalserather than leaving it unknown. Unreachable today — there is exactly one producer (data_service.py:957→:975) and it emitsadoptedon every path that setsapi_ok— so this is defensive coercion at a single call site, not a second source of truth.<span>if a reviewer disagrees; the control-off branch already renders exactly that.I also confirmed the
or {}change alters no behaviour:overridehas one consumer (get_stats), and"host" in {}/{}.get(...)match the old guarded forms exactly. It removes three redundantif override elseternaries and is what pays for the new line.UPDATE — design ruling 2 applied, and a CI red fixed (head
fd897fa0)The non-author reviewer passed the mechanism but returned one finding, and it stands. I re-derived it rather than relaying it:
workers.listdefaults to[], so on a stock fleet ZERO rigs are adopted. A rig with noworkers.listentry whose probe fails for a genuine config reason therefore landed in the new "not adopted" branch and was told to adopt — which cannot fix it (buildAdoptedConfigwrites noapi_port; a masked token falls through to the fleet auth mode). Theapi ⚠advice that would fix it rendered for nobody. That is this issue's own bug mirrored, so it could not ship as it was.The design lane owns this wording and ruled on it (their comment below). I applied their text verbatim rather than writing the sentence I had just been told was wrong — the author is the worst-placed person to review prose they were asked to produce. The badge label and its neutral
badge-outlinestyle are unchanged: they considered a remedy-free label and kept "not adopted" as a true state for every rig in the branch.Fixed in
ba7fa699:ApiBadge's tooltip, both branches. It now states the two facts the dashboard actually knows — the probe failed, and it holds no control token — with no causal "until you adopt it", then keys each remedy to the one thing the operator knows and the dashboard cannot: who set the rig up.docs/workers.mdbadge row, same shape.docs/workers.mdupgrade note and thedocs/configuration.mdworkers.api_authrow both said "every adopted worker showsapi ⚠" — true of nobody on a fleet where nothing is adopted.rigcardlogic.mjs— the rig card is the operator's first briefing, so leaving it re-plants this issue's confusion at the source — anddocs/workers.md:300.ApiBadgesaid an un-adopted rig "is not broken at all". That is the defect stated as a safety property beside the code, which is how it survives being read, so it is corrected too.Deferred, filed as #1949:
docs/appliance.md:153is the third stale "API error" site. Two other open PRs already modifydocs/appliance.md, and one of them is merge-ready on a recorded non-author pass that a rebase would void. A one-clause doc fix is not worth costing that PR its readiness.CI was RED on
b65c59d8, and this fixes itThe Fake-daemon contract test job failed —
2 failed, 29 passed— after my previous status recorded 0 failing; it flipped at 13:10:10Z.tests/integration/fakes/test_contract.pypins the probe's failure payload by exact dict equality, and this branch addsadoptedto every return path inget_stats, the failure ones included. That is deliberate and is the point of the change: the badge needs the adoption fact precisely whenapi_okis False. The product is right and the expectation was stale, so the expectations moved — and they stay exact rather than becoming subset assertions, because pinning the contract is that file's whole job.None of the gates I habitually run covers that suite.
make test-fakesis a separate target: 2588 pytest, 585 node and CI's ten per-surface lint targets were all green while it was red.⚠ LANE OVERRIDE — disclosed, as the guard requires
fd897fa0touchestests/integration/fakes/test_contract.py, owned by the e2e lane, not this one. I did not reach in: I asked that lane, and it granted the edit after re-deriving both dict values fromget_statsitself and confirming with its own grep that these are the only two affected sites — so nothing in that commit rests on my say-so. The override commit is that one file alone, and the marker was consumed on use. The file sits at its file-budget ceiling (451/451), so the edit is line-neutral by construction: both changes are in-place dict literals.RAN at
fd897fa0make test-frontendmake test-dashboardmake test-fakeslint-pyEvery rc above is one I captured into a file with
rc=$?, not read off a pipeline tail. Not run: no docker build, no KVM, nomake lint-sh— the appliance lane holds the bench, so CI is the gate for those.NOT MERGE-READY BY ME. The recorded pass named
d0bcef6c; this is a new head and needs a fresh non-author pass. I never merge my own, and ondevelopa merge needs the seat's--admin.🤖 Generated with Claude Code
https://claude.ai/code/session_01KZuzdtxNX8ae4rnDtBqEqR