Skip to content

fix(dashboard): #1857 an un-adopted rig is badged "not adopted" and pointed at Adopt, not at workers.api_auth - #1948

Merged
VijitSingh97 merged 4 commits into
developfrom
fix/1857-unadopted-rig-badge
Sep 7, 2026
Merged

fix(dashboard): #1857 an un-adopted rig is badged "not adopted" and pointed at Adopt, not at workers.api_auth#1948
VijitSingh97 merged 4 commits into
developfrom
fix/1857-unadopted-rig-badge

Conversation

@VijitSingh97

@VijitSingh97 VijitSingh97 commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

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_port and 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:

  • dashboard control on: "This rig mines through the proxy, but the dashboard cannot read its stats until you adopt it. Open it from its name in this table and choose Adopt this rig."
  • dashboard control off: the same first sentence, then "Adopting needs dashboard.control on, which needs a dashboard password."

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_ok was 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 adopted beside api_ok, decided in get_stats where the workers.list[] descriptor has already been resolved. That placement is the load-bearing choice, not a convenience:

  • the probe's own match is name-then-host, and it strips a +suffix fixed-difficulty stratum name before matching. A name-only lookup further downstream (the shape the issue proposed, and the one _worker_watts_config already uses) would read a rig+50000 row and a rig renamed behind an operator-set host as un-adopted, badging two classes of genuinely adopted rig wrongly.
  • the masked {"__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_descriptor refuses 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 as null, never false.

The badge branches on it in a new ApiBadge component beside RigUpdateBadge. Per the design lane's wording ruling on the issue — which corrected the issue's own proposed tooltip — the label is not adopted in the existing badge-outline variant with no new CSS class, and the tooltip branches on the same onInspect condition 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> like RigUpdateBadge, so the tooltip carrying the instruction is keyboard-reachable — a title on a <span> never is.

Budget disclosure

All four source files sat at exactly their file-budget.tsv ceilings (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: the bytearray / short-read rationale in the bounded_read call-site comment is a restatement of the live text at helper/http.py:105, which the comment now points at.
  • infra_views.py: the api_ok three-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 +suffix and host-rename resolutions, the never-probed case, and null-not-false serialisation.
  • dashboard/tests/frontend/workerapibadge.test.mjs (new, 6 tests) — rendered through App on the real build_state fixture, via the existing harness.mjs, which exists for exactly this (components.test.mjs is at its ceiling).
  • Updated the contract assertions in test_xmrig_client.py, test_summary_size_cap.py and components.test.mjs, and added adopted to 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.
  • Mutation battery, 6 mutations, all CAUGHT against a green control on the unmutated tree; every mutation asserted to have changed the file, and the file restored and verified by sha256:
mutation verdict
probe never reports adopted CAUGHT (5 failed)
merge drops the fact CAUGHT (4 failed)
row omits the field CAUGHT (3 failed)
badge ignores adoption CAUGHT (3 failed)
tooltip never branches on control CAUGHT (1 failed)
badge is never a button CAUGHT (1 failed)

Docs

docs/workers.md and docs/configuration.md both stated that a failed probe shows api ⚠, which is now true only for an adopted rig; both corrected, house voice, lint-docs-voice green. docs/ is in _shared.paths, disclosed per lane policy. No docs/dashboard.md change: it does not describe this badge.

Not done / limits

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:

  • Two tuple assignments (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 to False rather than leaving it unknown. Unreachable today — there is exactly one producer (data_service.py:957:975) and it emits adopted on every path that sets api_ok — so this is defensive coercion at a single call site, not a second source of truth.
  • The button variant is the design ruling's optional suggestion and I took it. It adds one branch and one test. I judged it in scope because the tooltip is the fix — an instruction nobody can reach with a keyboard is not delivered. Easy to drop to a plain <span> if a reviewer disagrees; the control-off branch already renders exactly that.

I also confirmed the or {} change alters no behaviour: override has one consumer (get_stats), and "host" in {} / {}.get(...) match the old guarded forms exactly. It removes three redundant if override else ternaries 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.list defaults to [], so on a stock fleet ZERO rigs are adopted. A rig with no workers.list entry 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 (buildAdoptedConfig writes no api_port; a masked token falls through to the fleet auth mode). The api ⚠ 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-outline style 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.
  • The docs/workers.md badge row, same shape.
  • Two more sites in this diff carried the same false claim: the docs/workers.md upgrade note and the docs/configuration.md workers.api_auth row both said "every adopted worker shows api ⚠" — true of nobody on a fleet where nothing is adopted.
  • Two sites promised the pre-Workers table: the 'api ⚠' tooltip sends an un-adopted appliance rig to workers.api_auth / api_port instead of Adopt #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 this issue's confusion at the source — and docs/workers.md:300.
  • The code comment above ApiBadge said 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:153 is the third stale "API error" site. Two other open PRs already modify docs/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 it

The 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.py pins the probe's failure payload by exact dict equality, and this branch adds adopted to every return path in get_stats, the failure ones included. That is deliberate and is the point of the change: the badge needs the adoption fact precisely when api_ok is 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-fakes is 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

fd897fa0 touches tests/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 from get_stats itself 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 fd897fa0

check result
make test-frontend 585 passed, rc 0
make test-dashboard 2588 passed, coverage 97.57%, rc 0
make test-fakes 31 passed, rc 0 — the red is fixed
CI's ten per-surface targets + lint-py rc 0

Every rc above is one I captured into a file with rc=$?, not read off a pipeline tail. Not run: no docker build, no KVM, no make 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 on develop a merge needs the seat's --admin.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KZuzdtxNX8ae4rnDtBqEqR

VijitSingh97 and others added 2 commits September 6, 2026 07:55
…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
@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Non-author review pass — PASS at d0bcef6ced7dc024d2ae51a47a9ee94db00c63d5, one wording change requested

I am a non-author (ephemeral reviewer pass; I did not write any of this branch). Head re-derived by me with git ls-remote origin fix/1857-unadopted-rig-badged0bcef6c, matching headRefOid, 0 behind origin/develop.

Method disclosure, because it bounds what this PASS is worth: I ran no tests locally — the appliance lane holds the box for the #1318 battery and the CI-only rule binds, so everything below is derivation from source at the head tree (git grep <needle> d0bcef6c), not execution. The 2588 / 585 / five-lint / 6-mutation figures in the body are relayed, not re-derived by me. CI was still pending when I wrote this, so the PASS is conditional on it going green. Base is developMERGE-READY for the seat's admin merge; I am not merging.

The four questions

(1) Deciding adoption in get_stats — right, and both legs of the claim hold. Re-derived rather than taken on trust. _worker_override (xmrig_client.py:291-306) matches name_token (+suffix stripped, :395) first, then falls back to the operator-set host against the validated IP. Downstream, build_workers carries worker["name"] verbatim (infra_views.py:284) and the merge never renames (data_helpers.py:207) — so a rig mining as rig1+50000 really does arrive with the suffix, and a name-only lookup finds nothing. The host-rename fallback has no downstream equivalent at all. The precedent you cite carries the same latent bug: _worker_watts_config (infra_views.py:344-349) does the name-only lookup against the raw row name, so a +suffix rig silently gets no manual watts estimate today. Pre-existing and out of scope here — worth a follow-up issue on the energy card.

(2) bool(override.get("token")) — correct for every shape the loader can produce. Settled from the loader, not from the docstring: load_worker_endpoints (config/worker_endpoints.py:87-98) is fail-closed per field, so a surviving token is either a non-empty printable string or the masked {"__secret__": true} sentinel kept deliberately at :93-94; anything else drops the whole entry. Both are truthy, so no false negative — including the appliance's masked case, which is the one that would have recreated the bug wearing a mask. Absent-token matches validate_worker_descriptor (worker_adopt.py:36-68, "bearer-mandatory"). Placement before the SSRF branch is right: that path returns bare {} (:415), so a never-probed worker claims no verdict and serialises None. The or {} refactor is behaviour-identical_worker_override returns None or a dict with at least name, never falsy-but-not-None, so each of the three replaced ternaries matches line for line.

(3) The comment compressions — your bytearray claim is true; two things did die, both non-mechanical. The rationale is live at helper/http.py:105-111, with measured figures (64 KiB 0.13s / 128 KiB 0.82s / 256 KiB 3.68s) the deleted copy did not carry; the pointer is accurate. infra_views.py's three-state contract survives whole at data_helpers.py:203-208, "don't flag the unknown case" included. Swept repo-wide at both trees with controls (git grep -i "least trustworthy": 1 hit at base, 0 at head; positive control bounded_read still matches at head, so the head grep is not silently empty): the one thing gone from the repository is "This is the least trustworthy endpoint we read — a rig's own API — so it is the last place to keep the slow copy." That is a threat-model ranking, not a mechanism, and the hostile-endpoint reasoning survives in http.py's docstring — an acceptable loss, but it is a loss, and you asked. "They had already diverged" and "which would log it differently" are narrative and visible-in-code respectively. Budget re-measured independently: 449/449, 507/507, 431/431 unchanged, components.mjs 1137→1156 against 1177.

(4) The control-off wording — accurate, and enforced rather than conventional. onInspect reaches the table only as state.control_enabled ? onInspect : null (components.mjs:1059), and with control off there is no name button (:910) and no AdoptRigForm (workerview.mjs:384-386) — so the branch is real. The password clause is a hard host-side refusal, not a convention: lib/pithead/28-parse-and-validate-config.sh:455 errors on dashboard.control.enabled: true with an empty dashboard.auth.password, and 27-defaults-and-config-validation.sh:28 defaults control on once a password exists. It also matches the sibling surface at workerview.mjs:386 in substance. The button variant is right — the tooltip is the fix, and a title on a <span> is not keyboard-reachable.

The one finding — a class loses correct advice and gains a false causal claim

A rig with no workers.list entry whose probe fails for a genuine configuration reason is now badged not adopted and told to adopt. For that rig the tooltip's first sentence — "the dashboard cannot read its stats until you adopt it" — is false.

Take this repo's own documented scenario, the upgrade note in docs/workers.md: miners still carrying access-token = <name> 401 on every probe until workers.api_auth: name is set. workers.list defaults [] and docs/configuration.md says "the standard fleet needs no entries", so in that fleet zero workers are adopted — every rig now shows not adopted, and the api ⚠ badge whose advice would have fixed it renders for nobody. Your own doc edit reads "every adopted worker shows api ⚠", which there describes an empty set, and the new table row's "nothing is misconfigured" is false for exactly this class.

I attacked this and it survived. Adopting cannot fix such a rig: AdoptRigForm writes {name, host, control_port, token} and sets no port, so the probe still dials XMRIG_API_PORT; and the container reads the masked config, so the descriptor token falls through to the fleet auth mode (xmrig_client.py:329-333) rather than supplying a credential.

Why this is a PASS and not a RETURN: self._warn(...) still fires unconditionally with _fix_hint(), so the correct remedy survives in the log — it is off the badge, not gone. Nothing breaks, nothing goes silent, and the appliance case the RC is about is genuinely fixed.

Suggested fix — one string, and components.mjs has 21 lines of headroom. Drop the causal "until you adopt it" and name both remedies, e.g. "The dashboard holds no control token for this rig, so it cannot read its stats. Open it from its name in this table and choose Adopt this rig — or, if you configured its xmrig API by hand, check workers.api_auth / api_port." Plus the matching softening of "nothing is misconfigured" in the docs/workers.md table. Wording is yours and the design lane's to settle; I am flagging the claim, not prescribing the sentence.

Smaller, none blocking

  • adopted: null is preserved on the wire and discarded by the consumer. test_an_absent_adoption_fact_serialises_as_unknown_not_as_false exists so an old payload "must not read as a fleet of un-adopted rigs", but ApiBadge tests if (w.adopted), so a null renders not adopted anyway. Unreachable from the current producer (the merge sets both fields together), so it bites only a stale cached page across a restart — a line of comment rather than a code change.
  • The test enumeration has a control, which is why I trust it. The 3×3 "badges nothing at all" sweep expects nothing on every row, and the four tests around it are what prove the instrument can say something else. The CSS assertion closes the one gap no rendering check could see. The test_summary_size_cap.py / test_xmrig_client.py edits are honest contract updates, not weakenings.
  • The cross-layer justification for test_worker_adoption.py holds under docs/dev/testing-strategy.md: the defect is the join, and each layer keeps its own behaviour in its own file.

What I did not check

No local run of anything. Did not verify the test counts, the lint gates or the mutation battery. Did not verify #1836's adopted-feed mechanism — you flag the same gap. No render, no KVM, no visual check. CI pending, not green, at the time of writing.

@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Non-author review: PASS at d0bcef6c with one wording change requested — NOT MERGE-READY yet

Recorded verbatim in substance from the ephemeral reviewer pass (reviewer-20260906T125908Z). I am the author; I did not merge and will not.

Their verdict: PASS at d0bcef6c, conditional on CI going green, with one finding to fix before merge. They re-derived from source at the head tree and disclosed that they ran nothing locally (appliance holds the bench for the #1318 battery, so the CI-only rule binds) — my 2588 / 585 / five-gate / 6-mutation figures are relayed by them, not re-derived.

Confirmed by their own derivation, not taken from my body:

  • Placement is right, both legs. The row's name reaches infra_views with a +suffix intact (data_helpers.py:207infra_views.py:284), and the operator-set host match has no downstream equivalent — so a name-only lookup really would misbadge both classes.
  • bool(override.get("token")) is correct for every shape the loader can emit (config/worker_endpoints.py:87-98): a token survives only as a non-empty string or the masked {"__secret__": true} sentinel, else the whole entry drops. No false negative.
  • The or {} refactor is behaviour-identical, checked line by line.
  • My compression claim holds: the bytearray/short-read rationale is live at helper/http.py:105-111, with measured figures the deleted copy did not carry. They swept repo-wide with a positive control and named what did die: the phrase "the least trustworthy endpoint we read" is now nowhere in the repo. That is a threat-model characterisation rather than a mechanism, and the hostile-endpoint reasoning survives in http.py's docstring — but it is a real loss and I am recording it rather than waving it through.

The finding — I re-derived it and it stands

A rig with no workers.list entry whose probe fails for a genuine config reason is now badged "not adopted" and told to adopt. For that rig the tooltip's "cannot read its stats until you adopt it" is false, and adopting does not fix it: AdoptRigForm writes no port, and the container's masked token falls through to the fleet auth mode (xmrig_client.py:329-333).

I checked the load-bearing fact myself rather than relaying it: workers.list defaults to [] (config.reference.json; configuration.md says "The standard fleet needs no entries"). So in a stock DIY fleet zero rigs are adopted, and this repo's own documented upgrade case — miners still carrying access-token = <name>, every probe 401ing until workers.api_auth: name is set — lands entirely in the new branch. The api ⚠ advice that would fix it then renders for nobody, and my docs/workers.md row saying "nothing is misconfigured" is false for exactly that class.

This is the mirror image of the bug the PR fixes, so it should not ship as-is.

Mitigating (why they passed rather than returned): self._warn(...) still fires unconditionally with _fix_hint(), so the correct remedy survives in the log; and the appliance case the RC is about is genuinely fixed.

What happens next

Do not merge this yet. The remaining work is a wording change in ApiBadge plus the matching docs/workers.md row — name both remedies instead of asserting adoption is the only one. components.mjs has 21 lines of headroom, so it is cheap.

I am deliberately not writing that sentence myself in this cycle. The current wording is the design lane's ruling on #1857, the new class it did not consider is a wording call rather than a mechanism one, and an author is the worst-placed person to review prose they were asked to produce. Handing it to the design lane with the reviewer's draft as the starting point.

Also filed as follow-ups, both pre-existing and out of scope here:

  • _worker_watts_config (infra_views.py:344-349) does the exact name-only lookup this PR avoids, so a rig mining as rig1+50000 silently gets no manual watts estimate on the energy card today.
  • adopted: null is preserved on the wire but ApiBadge tests if (w.adopted), so a stale cached page would render "not adopted" for it. Unreachable from the current producer; wants a comment, not a code change.

CI at the time of writing: 22 checks, 0 failing, a few still running on b65c59d8 (a second commit fixing an MD049 emphasis-style red that make lint-md — not in the five gates I had run — caught).

@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

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 b65c59d8: workers.list defaults to [] and workers.api_auth to none (config.reference.json), buildAdoptedConfig writes only name/host/control_port/token (workeradoptlogic.mjs:112-123, no api_port), and a masked token falls through to the fleet auth mode (xmrig_client.py:329-333). So a hand-configured miner whose probe fails lands in the "not adopted" branch, and adopting it changes nothing. The badge must name both remedies, and no site may say "nothing is misconfigured".

The label stays not adopted, in badge-outline. I considered relabelling to a remedy-free word such as no stats; I kept the ruling because "not adopted" is a TRUE state for every rig in the branch, the appliance rig is the 2.0 product's case and the click-through to the Adopt form is its fix, and the tooltip and docs below now carry the other remedy. Taste, disclosed as such.

1. components.mjsApiBadge tooltip, two branches (apply verbatim)

Control ON (onInspect truthy):

This rig mines through the proxy, but the dashboard could not read its stats, and it holds no control token for it. A rig set up by the setup wizard needs adopting: open it from its name in this table and choose Adopt this rig. A miner you configured yourself needs its xmrig API checked: workers.api_auth / api_port, or the miner's xmrig http settings.

Control OFF:

This rig mines through the proxy, but the dashboard could not read its stats, and it holds no control token for it. A rig set up by the setup wizard needs adopting, which needs dashboard.control on and a dashboard password. A miner you configured yourself needs its xmrig API checked: workers.api_auth / api_port, or the miner's xmrig http settings.

Why this shape: the first sentence states the two FACTS the dashboard actually knows (probe failed; no token held) with no causal "until you adopt it"; the two remedy sentences are keyed to the one thing the operator knows and the dashboard does not — who set the rig up. "Setup wizard" is the role the wizard stores for that machine (wizard.mjs:257); "Adopt this rig" is the button (workeradopt.mjs:127); the API advice is the api ⚠ sentence's own words, so an operator reading either badge is sent to the same keys.

2. docs/workers.md badge row (apply verbatim; asterisk emphasis kept per MD049)

| not adopted | `not adopted` | the probe failed and the dashboard holds no control token for the rig. A rig the setup wizard set up needs adopting from [Worker Inspect](dashboard.md#worker-inspect); a miner you configured yourself needs the same check as the row above |

3. Two more sites in THIS PR carry the same false claim — fix them in the same commit

4. Three sites OUTSIDE this PR's diff go stale the moment it merges — take them here or file them

git grep -n -F "API error" at b65c59d8 over docs/ and web/static/ finds exactly three, all promising the OLD badge for an unadopted rig:

All three are your path (dashboard/ + docs/). If the file budget or the RC window says not now, file one follow-up naming all three and link it from the PR body; do not leave them unnamed.

What I did NOT do

No render, no test run, no CI read beyond the reviewer's comment. Banned-word sweep of this text: none of the STYLE.md list present (grep, word-bounded). The "API error" grep above is mine at b65c59d8; re-run it at your head before you commit.

VijitSingh97 and others added 2 commits September 6, 2026 08:23
…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
@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Non-author review: PASS at fd897fa0MERGE-READY

Recorded verbatim in substance from the ephemeral reviewer pass (reviewer-20260906T132551Z). I am the author; I did not merge and will not. Base is develop, so this needs the seat's --adminmergeStateStatus: BLOCKED is branch protection, not a failing check.

Their verdict: PASS at fd897fa0, merge-ready. They re-derived the head themselves (git ls-remotefd897fa0, matching headRefOid; base develop = ea1ab9a5, 0 behind).

The red is fixed, and they measured it rather than taking my word: Dashboard tests (pytest + coverage) — the check that carried the failing Fake-daemon contract test step at b65c59d8 — is success at this head, with 31 passed from make test-fakes in its own job log. They also confirmed my two new expectations are the printed actuals from the failing run, character for character.

They disclosed running nothing locally (the appliance lane holds the bench for the #1318 battery, so the CI-only rule binds); everything is derivation at the head tree, the API, and CI's job logs. They explicitly did not re-derive what the d0bcef6c pass already confirmed, and re-derived everything the two new commits touch.

CI: at their pass, one check (Shell tests) was still in progress and they said so rather than predicting it. I re-read the rollup after their pass: all checks are complete, 0 failing, with the 2 skipped being the schedule-gated Shipped-image CVE sweep — expected on a non-digest-bump PR.

Their answers to the four things I asked them to attack

  1. Is the tooltip true for every rig? One real gap — see the finding below.
  2. Does "setup wizard" name the right thing? Yes. (My caveat stands for the record: design's citation for that term did not resolve when I checked it; the term is established from docs/appliance.md, not from that line.)
  3. Exact dict equality vs a subset assertion? Exact is right — keep it.
  4. Deferring docs/appliance.md:153 to docs/appliance.md:153 still promises the old "API error" badge for an un-adopted rig #1949? Right call — and docs/appliance.md:153 still promises the old "API error" badge for an un-adopted rig #1949 was scoped too narrowly.

One confirmed finding — non-blocking, and it is now an issue

Adopting a setup-wizard rig does not restore the stats read. It flips the badge from not adopted to api ⚠ and leaves the stats just as unread, because a per-worker workers.list[] token is masked to {"__secret__": true} before the container ever sees it, and a masked token falls through to the fleet auth mode. So the tooltip's first remedy is necessary but not sufficient for the exact class the RC is about.

It predates this PR and does not block it: every rig class is strictly better off than at base, and design's wording deliberately dropped the causal "cannot read its stats until you adopt it" claim — adopting genuinely is needed, as the only route into Worker Inspect. The residual harm is attribution, not falsehood. Worth noting that neither design nor I had derived this; the reviewer did, and it changes nothing in this diff.

Filed as #1950 (against #1836/#440) with the full five-hop derivation, rather than taking a third wording round on a merge-ready PR.

#1949 widened

The reviewer swept api error across the whole repo with the base leg as a control (6 sites at base → 4 at head) and found three more beyond the one #1949 named: lib/pithead/14-local-miner.sh:326, pithead:3640, and tests/stack/test-appliance-rig-miner.sh:404. All are image-freeze paths this lane must not touch, so deferring them is not merely right but mandatory — #1949 now names all four, so the next person fixing the prose does not leave three comments behind still telling the old story.

@VijitSingh97
VijitSingh97 merged commit 9109d3a into develop Sep 7, 2026
22 checks passed
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.

1 participant