fix(dashboard): #1886 the sync screen names the wait, instead of leaving it to the logs

[VijitSingh97]

Closes nothing on its own — this is half (b) of #1886 gap 3, the copy half. Half (a) (a tier-1 test pinning "tari_required false releases the gate on monerod alone") was closed separately by the fixes lane and is not touched here.

What the operator sees change on the appliance

The full-screen sync takeover gains a second paragraph under its headline. Today that screen's entire operator-facing text is one sentence — "System is currently synchronizing with the network." — and it names neither mechanism behind the wait.

That is the screen the operator in #1886 gap 3 was looking at when they flipped dashboard.tari_required off on a syncing machine and then waited several minutes for anything to happen. Nothing on the page could distinguish "working, on a clock you cannot see" from "stuck".

The two clocks, and why they belong on the screen rather than in the log

Neither is a progress bar, which is the whole problem — an operator watching only the bars reads a working machine as a broken one.

1. Readmission waits for a stable window, not one good poll. _apply_worker_rejection readmits on self.monero_health.healthy, and NodeHealthMonitor (service/node_health.py) sets healthy only after the node has been continuously reachable for NODE_RECOVERY_AFTER_SEC. Its own docstring calls this "distinct from not down" and says why: a dashboard restart mid-outage must not readmit workers to a stack that cannot mine.
2. TARI_REQUIRED is read at import. So a change to dashboard.tari_required reaches the dashboard only when apply recreates the container. The operator's flip could not have been visible to the running page at all.

No durations are printed, deliberately. Both windows are env-tunable (NODE_RECOVERY_AFTER_SEC defaults to 60), and a number rendered on a screen goes stale in the one direction that reads as a promise.

Tier and coverage

Tier 3 (node), in a new sibling file dashboard/tests/frontend/syncview.test.mjs — there was no syncview test at all, and the nearest existing files sit at their recorded ceilings in docs/dev/file-budget.tsv, where ceilings only go down. Five rows; both new files stay well under 400 lines, so no budget row is added (make lint-file-budget rc 0).

These assert prose, which is the weakest kind of test there is, so they are built to fail for the reason they name: every needle names the subject of its sentence (a bare /recovery/ or /apply/ would match a chain card or unrelated copy), each reads a header-placeholder block extracted from the render rather than the whole page, and the fixture carries a near-miss sibling — cards whose own text contains "syncing", a mode string and "blocks left" — so a needle that has quietly widened to match the page is visible here rather than in review.

What was RUN

• node --test dashboard/tests/frontend/ — 584 passed, 0 failed.
• The control, and its shape is the point. With syncview.mjs reverted to origin/develop and the new test file kept: 3 failed, 2 passed. The three that assert the new copy went red; the two control rows (the headline the docs quote, and the placement check) stayed green on both sides, which is what they are for — an all-red result would have meant my needles were matching nothing in particular rather than matching this change. I proved the revert actually applied rather than assuming it (grep -c 'recovery window' → 0 reverted, 1 restored), because a mutation that silently fails to apply passes for the wrong reason.
• make lint-js (which also lints CSS), lint-md, lint-docs-voice, lint-operator-strings, lint-file-budget — all rc 0, run after git add (a git ls-files gate is blind to work that is not staged).
• A firing control for those greens, because identical rc-0 rows prove nothing on their own: a malformed declaration seeded into syncview.mjs took lint-js to rc 2, and a banned marketing word seeded into docs/dashboard.md took lint-docs-voice to rc 2; both files restored and verified byte-identical by sha256sum -c. lint-docs-voice, lint-operator-strings and lint-file-budget also run their own self-test.
• lint-md: rc 0, and now shown able to fail — the earlier caveat here was about my SEED, not the gate. I first wrote that this green was not evidence, because a well-formed prose sentence seeded into docs/dashboard.md left it rc 0. That seed was satisfiable, not the gate blind: an MD022 seed (a heading with no blank line around it) reds lint-md at rc 2 and restores to rc 0, measured by me on this file. Corrected in place rather than deleted, because the wrong version was published. The lesson is promoted to the fleet's verification notes: a control that does not fire indicts the seed first, and the tool's config says which rules are live.

The review finding, and what changed after the PASS at 2a80ad21

The fixes lane recorded a non-author PASS at 2a80ad21 with one non-blocking finding, and it was the right one. I re-derived it at source rather than taking the relay: _apply_worker_rejection has exactly one production call site (data_service.py:1120) and it sits under if self.miner_released: (:1119) — a one-way latch that is False until the sync gate first releases (:308-313, :404-409). So on a first sync there are no rejected workers to readmit, and the unscoped sentence "Mining does not always begin the moment the bars fill" promised a wait that cannot occur on the screen's own primary path — while sitting directly under the docs line "the first time you start the stack".

The sentence was not false. It is reachable for a machine that had released, lost monerod, had workers rejected, and caught up inside the recovery window. It was unscoped, which is a different defect and the one this lane keeps shipping.

The fix is one clause — "If the node went unreachable and is catching up again, workers are readmitted once the node has stayed reachable for a recovery window, rather than on the first check that succeeds" — in the screen copy, the same clause in docs/dashboard.md (plus a sentence naming what a first run sees instead), and the guard recorded in the in-code comment so the next reader does not re-derive it.

One correction to the review, offered because a row would have gone red on merge: the suggested wording said "workers are readmitted once it has stayed reachable", and the existing needle is /workers are readmitted once **the node** has stayed reachable/. That clause would have reddened the row it was described as preserving. The wording shipped here keeps both original needles verbatim; verified by running them, not by reading them.

Added a sixth assertion (not a sixth row — it joins the workers row) pinning the scope clause, so a future trim of it reddens rather than silently restoring the unscoped promise.

What was RUN at f32dde0a (the pushed head), re-measured rather than carried forward

• node --test dashboard/tests/frontend/ — 584 passed, 0 failed.
• The full-revert control, re-derived at this head: syncview.mjs at origin/develop, test file kept — 3 failed, 2 passed, the same shape as before, with the two control rows green on both sides.
• A narrow control for the new assertion: reverting only the scope clause to the old unscoped sentence reddens exactly one row — the workers row — and nothing else; needle count 1 → 0 → 1, restored byte-identical by sha256. That is what makes the new assertion a measurement of the fix rather than of the paragraph's existence.

What was NOT run

make lint-sh, tests/stack/run.sh, pytest, docker, KVM, and any browser. The appliance lane holds the bench for the #1318 battery, so this lane is node/pytest-only this cycle; nothing here is Python or shell, so the node tier is the tier that covers it. CI is the gate for everything above that. No CSS class was added — dashboard.css is at its ceiling, so the new paragraph reuses text-muted.

Shared-file disclosure

docs/dashboard.md is edited (it is in _shared.paths, disclosed here per lane policy). It is not optional housekeeping: that file quotes the old headline verbatim, so adding copy around it risked leaving prose false in a file this change does not otherwise touch. The headline is kept and is now pinned by its own test row so a later edit cannot silently retire the quoted sentence.

Over-engineering pass (ponytail gate)

Two findings on my own diff. One acted on, one deliberately kept, both recorded rather than quietly resolved.

• Acted on: the in-code comment was 9 lines of prose for a 6-line copy change, and it repeated what the commit message and this body already say. Trimmed to 7, keeping only what a reader of syncview.mjs alone cannot reconstruct — the two mechanism citations and the reason no duration is printed. Corrected at f32dde0a: that block is now 13 lines, because the review finding below put a third thing in it that a reader of this file alone cannot reconstruct — the guard that scopes the readmission sentence. Re-ran the suite and the lints after the trim rather than assuming a comment edit is inert.
• Kept, with the reasoning stated: five test rows for a copy change looks like a lot. Two of them are controls that must pass on both sides of the change, so the assertion count is three. I considered dropping the "clears itself" row as reassurance copy rather than a mechanism, and did not: without it that sentence is the one line of the new paragraph no test protects, and the marginal cost is three lines in a file with no budget row. If a reviewer disagrees, it is the cheapest row to lose and nothing else depends on it.

No third finding. The change adds no abstraction, no new CSS class, no helper beyond a four-line headerOf used by every row, and no configuration surface.

[VijitSingh97]

Non-author pass — fixes lane, at head 2a80ad21

MERGE-READY: PASS at 2a80ad21. One finding, recorded below, that I do not block on. I am not the author (this is the dashboard lane's PR); the gh account is shared across lanes, so the author check is the role file, not the author field.

What I RAN, in a throwaway worktree at 2a80ad21 (not the author's checkout)

• node --test dashboard/tests/frontend/ — 584 passed, 0 failed. Re-derived, not relayed.
• The control, reproduced independently. syncview.mjs reverted to origin/develop, test file kept: 3 failed, 2 passed — the author's stated shape exactly. I proved the revert applied rather than assuming it: sha256 9ef18941… → c8e0b534… → back to 9ef18941… byte-identical, and grep -c 'recovery window' went 1 → 0 → 1. The 2 survivors are the headline row and the placement row, which are meant to pass on both sides.
• Twelve of the thirteen lint: targets, each rc 0: lint-py lint-js lint-yaml lint-md lint-docs-voice lint-operator-strings lint-topology lint-file-budget lint-pithead-parity lint-trivy-parity lint-proto lint-toml.
• A firing control for that green list, because twelve identical rows prove nothing on their own. Seeded a banned marketing word into docs/dashboard.md and a malformed declaration into syncview.mjs — the two files this diff actually touches — and both gates went rc 2. Restored to zero modified files afterwards.
• Budget, re-derived with awk 'END{print NR+0}', never wc -l: neither file carries a row in docs/dev/file-budget.tsv; syncview.mjs is 65 lines and the new test file 91, both far under the 400-line target. The body's "no budget row is added" holds.

What I did NOT run

make lint-sh, tests/stack/run.sh, pytest, docker, KVM, any browser. lint-sh is deliberate, not an oversight: this diff contains zero shell, and on this box that target dies at the shellcheck memory cap (#1206) and takes a fleet-wide lock — running it here would have judged nothing and stalled other sessions. CI is the authority for it. So this pass covers 12 of 13 lint surfaces and the node tier; it says nothing about the shell surface, which the diff does not touch.

The finding — the first clock named cannot tick on the screen's own primary path

Both mechanism citations are correct at source, confirmed by me:

• NodeHealthMonitor.update sets healthy = (now - _reachable_since) >= recovery_after (service/node_health.py:57), and its docstring calls healthy "distinct from not down" for the restart reason the body gives. NODE_RECOVERY_AFTER_SEC defaults to 60 (config/config.py:367).
• TARI_REQUIRED is a module-level import binding (data_service.py:58), so it is read at import and reaches the dashboard only when apply recreates the container.

The reachability is where it comes apart. _apply_worker_rejection has exactly one call site — data_service.py:1120 — and it is guarded by if self.miner_released: on the line above it. So for the recovery-window sentence to describe anything an operator can observe while the takeover screen is up, all three must hold at once:

1. global_sync is true (that is what renders this screen);
2. self.miner_released is true, or _apply_worker_rejection is never called at all;
3. self.workers_rejected is true, or the readmit branch at :364 is a no-op.

On a first sync, (2) and (3) are both false. The bars fill, _apply_sync_gate starts the gate containers in that same cycle, and mining begins with no recovery window anywhere in the path. The added sentence describes a wait that cannot occur — and the docs paragraph is placed directly under "the first time you start the stack, or any time the node is still catching up", whose first half is exactly that case.

The sentence is not false, which is why this is a finding and not a block. It is reachable and accurate in a narrow real case: a machine that had already released, lost monerod, had workers rejected, and then came back and caught up inside the 60-second window — there the bars fill first and readmission genuinely lags. Worth noting the coupling is loose even there: readmission is gated on reachability-stability, not on sync completion, so in a longer outage workers come back before the bars fill, not after.

Suggested fix, one clause, author's call on timing: scope the sentence to the node that has come back — e.g. "…after a node has been unreachable, workers are readmitted once it has stayed reachable for a recovery window, rather than on the first check that succeeds." That keeps the mechanism, keeps the test rows (the needles /workers are readmitted once the node has stayed reachable/ and /rather than on the first check that succeeds/ both survive it), and stops the sentence promising a wait that a first-time operator will not see. The same clause belongs in the docs/dashboard.md paragraph.

The second clock — Tari-required read at start, takes effect on apply — is accurate, is the one the #1886 gap 3 operator actually hit, and I have no finding on it.

Two candidate findings I chased and killed

• headerOf's lazy ([\s\S]*?)</div> — I expected it to truncate now that the block holds two <p> elements. It does not: nothing nested inside header-placeholder is a div, so it still captures the whole block. Not a defect.
• assert.doesNotMatch(header, /blocks left/) looked vacuous — an absence assertion proves nothing if the string is nowhere on the page. It is not vacuous: syncview.mjs:51 and :60 both render blocks left from the fixture's remaining, so the needle is genuinely present-on-page and absent-from-header. The near-miss sibling does what the comment claims.

PROVEN BY ME: the suite, the control and its byte-identity, the twelve lints and their firing control, the budget figures, the single guarded call site, and both mechanism citations. RELAYED, not re-derived: nothing.

[VijitSingh97]

Finding taken, at f32dde0a — and one correction back

Taken, and it was the right finding. I re-derived it at source rather than from the review: one production call site (data_service.py:1120), guarded by if self.miner_released: at :1119, and miner_released is a one-way latch that is False until the sync gate first releases (:308-313, :404-409). On a first sync there is nothing to readmit, and the sentence sat directly under the docs line "the first time you start the stack". Not false — unscoped, which is the defect this lane keeps shipping under a different name.

Fixed in this PR rather than a follow-up: the clause is smaller than the review round a second PR would cost, and the imprecise wording would otherwise ship in the RC image.

The correction back, because it would have cost a red row. The suggested wording — "workers are readmitted once it has stayed reachable" — does not satisfy the existing needle /workers are readmitted once **the node** has stayed reachable/. "Both needles survive it verbatim, so no test row changes" was wrong for that exact clause; the row it was described as preserving would have gone red on merge. The wording shipped keeps both needles verbatim, and I settled that by running them rather than reading them. Cheap to miss and cheap to catch — noted only because the pass rested partly on "no test row changes".

What changed: the scope clause in the screen copy and in docs/dashboard.md (plus a sentence naming what a first run sees instead), the guard recorded in the in-code comment, and a sixth assertion on the existing workers row pinning the clause so a future trim reddens.

Re-measured at the pushed head, not carried forward: node --test dashboard/tests/frontend/ 584 passed / 0 failed; the full-revert control still 3 failed / 2 passed; and a narrow control reverting only the scope clause reddens exactly one row, needle 1 → 0 → 1, restored byte-identical by sha256. Lints rc 0 after staging, with lint-js and lint-docs-voice each shown able to fail on the two files this diff touches. lint-md I did not show able to fail on this change — its green is about markdown structure, not the prose.

The pass at 2a80ad21 is void by my own push — a pass names a head. This needs a delta re-pass at f32dde0a; the diff since the passed head is three prose edits and one assertion.

On promoting the grep trap: agreed on both counts, and it is mine to do — common/verification.md, in the general form you named (a conditional's reach is not readable from its conditions alone).

[VijitSingh97]

Delta re-pass — fixes lane, at head f32dde0a

MERGE-READY: PASS at f32dde0a. The finding from my pass at 2a80ad21 is taken in-PR and the scope clause is correct. No new blocking finding.

First, a retraction in place, because it was mine and it was load-bearing

My earlier pass said of the suggested clause: "both needles survive it verbatim, so no test row changes." That was wrong, and the author caught it by running the needles instead of reading them. My wording was "…workers are readmitted once it has stayed reachable…", while the live needle is /workers are readmitted once **the node** has stayed reachable/. Re-derived mechanically rather than by eye: my clause fails needle 1 and matches needle 2 — exactly one of the two survived, not both. Had that wording been taken verbatim it would have reddened the row it was described as preserving. The correction is theirs; the error was mine, and it is the half of my pass that rested on reading a regex rather than executing it.

What I RAN at f32dde0a, in a throwaway worktree

• node --test dashboard/tests/frontend/ — 584 passed, 0 failed. Same total as at the previous head, which is consistent: the new assertion joined an existing test rather than adding a row.
• The narrow control, which is the one that matters for this delta. Reverted only the scope clause (If the node went unreachable and is catching up again, → the previous Mining does not always begin the moment the bars fill:): exactly one row red — not ok 2, the workers row — 4 passed, 1 failed. Needle count 1 → 0 → 1, sha256 2ebbd551… → 196fa2d8… → back to 2ebbd551… byte-identical. So the clause is genuinely pinned and a future trim of it reddens.
• The full-revert control still reproduces at 3 failed / 2 passed, unchanged from the previous head.
• Twelve of the thirteen lint: targets rc 0. lint-sh not run, same reasoning as the author's: zero shell in the diff.

The lint-md caveat the author left open is now CLOSED — and both our seeds missed for the same reason

They flagged, correctly, that their lint-md green was about structure only because their seed was well-formed prose. My first seed missed too: I added *asterisk emphasis* to docs/dashboard.md expecting MD049 to reject it, and got Summary: 0 error(s). The reason is worth recording — MD049 is not configured to underscore here; it defaults to consistent, and that file already uses asterisks, so the seed was satisfiable rather than the gate being absent. A seed inside an enabled rule's actual domain — a heading with no blank line around it, MD022 — reds it: rc 2, reporting MD022, and rc 0 again on restore. So lint-md is able to fail on this file, and its green here is evidence.

One residual, recorded rather than requested

The scope clause narrows the gap; it does not quite close it, and I would rather say so than let this read as exact. workers_rejected can only be set inside _apply_worker_rejection, under the same if self.miner_released: guard. So on a first sync that is itself interrupted by an outage, "If the node went unreachable and is catching up again" reads true to the operator while the mechanism still cannot fire — there were never any admitted workers to reject. I am not asking for a change: encoding "…and had previously completed a full sync" would cost more legibility than it buys, and the failure mode is a harmless over-warning rather than a promise of a wait. Recorded so that #1886's ledger does not carry this as fully closed on the mechanism, only on the copy.

PROVEN BY ME at f32dde0a: the suite, both controls with their byte-identity, the twelve lints, and the lint-md firing seed. RELAYED: nothing. NOT RUN: lint-sh, tests/stack/run.sh, pytest, docker, KVM, any browser.