fix(masthead): restart_needed means a restart HELPS — behind boards get their own cue (#1722)

The filing guessed at "an mtime-vs-process-start comparison that trips on file
touches that aren't builds". There is no such comparison. The flag was one
expression:

    "restart_needed": bool(server.get("stale")) or bool(behind)

Observed state had server_stale false, so `behind` was the trigger — a logical
certainty from the code, not an inference: nothing else writes this flag.

Two conditions were sharing one flag and one sentence, and they have DIFFERENT
REMEDIES. A stale server is fixed by restarting the Monitor. A board on older
firmware is fixed by FLASHING THE BOARD — restarting does nothing for it. The
operator was told to do something that cannot work.

The sharper half: the masthead ALREADY has the correct cue for behind boards —

    N boards on older fw (latest X)     [tooltip: device @ fw, ...]

— and it sits in an `else if` AFTER restart_needed. So while `behind` fed that
flag, the accurate message was UNREACHABLE. Every behind board rendered "newer
build - restart the Monitor" instead. The right words were written, shipped, and
could never appear.

Fix: restart_needed = server_stale. `behind` is untouched and still published in
the firmware block; it simply no longer claims a restart is the answer, which
lets the correct cue render for the first time.

The tooltip loses "or a board is behind" — that clause was the conflation stated
out loud — and now says restarting does not reflash a board.

One test asserted `restart_needed is True` for a behind board. That was the
defect written down as an expectation, so it is amended rather than deleted, with
the reason in place: a reader hitting that diff should see a corrected model, not
a fixture tweak.

Deliberately NOT changed: when the server is stale AND boards are behind, the
restart cue still wins the `else if`. That precedence predates this and showing
both is a masthead-density question, not an honesty one — the remaining cue is
true in that state.

AC1's intent is pinned by a test asserting restart_needed tracks server_stale
exactly, so a third condition cannot be folded in later without bringing its own
cue and its own remedy.

Refs #1712

Lane: DX

Author: OrangePeachPink

tools/analytics/card_context.py

@@ -377,7 +377,18 @@ def _versions_block(device_groups: list, server: dict) -> dict:
         },
         # running server predates the checked-out code -> a restart shows newer app
         "server_stale": bool(server.get("stale")),
-        "restart_needed": bool(server.get("stale")) or bool(behind),
+        # #1712: RESTART_NEEDED MEANS A RESTART HELPS. It used to be
+        # `stale or behind`, which conflated two conditions with different remedies:
+        # a stale server is fixed by restarting the Monitor, a board on older
+        # firmware is fixed by FLASHING THE BOARD, and restarting does nothing for it.
+        #
+        # The masthead already carries the correct, specific cue for behind boards
+        # ("N boards on older fw (latest X)") — but it sits in an `else if` after this
+        # flag, so while `behind` fed this flag that accurate message was UNREACHABLE.
+        # Every behind board rendered "newer build - restart the Monitor" instead, an
+        # instruction that cannot work. `behind` is untouched and still published in
+        # the firmware block; it simply no longer claims a restart is the answer.
+        "restart_needed": bool(server.get("stale")),
     }
 
 

tools/analytics/dashboard_template.html

@@ -1000,7 +1000,7 @@ <h2>Experiment Capture <span class="proposed" title="operator-driven; isolated f
   const V = DASH.versions || {};
   const appBit = V.app ? ` · app <span class="mono">${V.app}</span>` : '';
   let cue = '';
-  if(V.restart_needed) cue = ` · <span class="vercue warn" title="the running server predates the checked-out code, or a board is behind — restart the Monitor to pick it up">↻ newer build — restart the Monitor</span>`;
+  if(V.restart_needed) cue = ` · <span class="vercue warn" title="the running server predates the checked-out code — restart the Monitor to pick it up (a board on older firmware is a separate cue; restarting does not reflash a board)">↻ newer build — restart the Monitor</span>`;
   else if(V.firmware && V.firmware.behind && V.firmware.behind.length)
     cue = ` · <span class="vercue warn" title="${V.firmware.behind.map(b=>b.device+' @ fw '+b.fw).join(', ')}">${V.firmware.behind.length} board${V.firmware.behind.length===1?'':'s'} on older fw${V.firmware.latest?' (latest '+V.firmware.latest+')':''}</span>`;
   document.getElementById('subline').innerHTML =

tools/analytics/test_versions_masthead.py

@@ -110,7 +110,11 @@ def test_device_behind_latest_firmware_is_flagged(monkeypatch) -> None:
     assert v["firmware"]["latest"] == "0.7.1"
     behind = {b["device"] for b in v["firmware"]["behind"]}
     assert behind == {"old"}  # only the lower board, and only because it parses
-    assert v["restart_needed"] is True
+    # #1712: this used to assert restart_needed is True — the defect, written down as
+    # an expectation. A board on older firmware is fixed by FLASHING IT; restarting
+    # the Monitor cannot help, and saying so trains the operator to ignore the cue.
+    assert v["restart_needed"] is False
+    assert v["server_stale"] is False
 
 
 def test_stale_server_sets_restart_needed(monkeypatch) -> None:
@@ -127,3 +131,74 @@ def test_no_false_behind_when_latest_unknown(monkeypatch) -> None:
     assert v["firmware"]["latest"] is None
     assert v["firmware"]["behind"] == []
     assert v["restart_needed"] is False
+
+
+# --------------------------------------------------------------------------- #
+# #1712 — restart_needed means A RESTART HELPS
+# --------------------------------------------------------------------------- #
+# Observed live 2026-08-20: the masthead showed "newer build - restart the Monitor"
+# while app == server == 0.8.1, app_server_match true and server_stale false. The
+# working tree, origin/main and the running process were all 98f17f3. Nothing newer
+# existed to restart into.
+
+
+def test_a_behind_board_does_not_ask_for_a_restart(monkeypatch) -> None:
+    """THE bug. `restart_needed` was `stale or behind`, conflating two remedies.
+
+    A stale server is fixed by restarting the Monitor. A board on older firmware is
+    fixed by flashing the board. Telling the operator to restart for the second one is
+    an instruction that cannot work — and a prompt that fires when no restart helps
+    trains them to ignore it, which costs exactly when it is real.
+    """
+    monkeypatch.setattr(card_context, "_declared_fw_version", lambda: "0.8.1")
+    v = _versions_block(
+        [_dev("classic", "0.8.1"), _dev("c5", "0.7.3")],
+        {"version": "0.8.1", "stale": False},
+    )
+    assert v["firmware"]["behind"], "the fixture must actually have a behind board"
+    assert v["restart_needed"] is False
+
+
+def test_the_behind_fact_is_still_published(monkeypatch) -> None:
+    """The fix must not silence a true statement — only stop it asking for a restart.
+
+    The masthead's own accurate cue ("N boards on older fw") reads this. It sits in an
+    `else if` AFTER restart_needed, so while `behind` fed that flag the correct message
+    was unreachable: every behind board rendered the restart prompt instead.
+    """
+    monkeypatch.setattr(card_context, "_declared_fw_version", lambda: "0.8.1")
+    v = _versions_block([_dev("c5", "0.7.3")], {"version": "0.8.1", "stale": False})
+    assert [b["device"] for b in v["firmware"]["behind"]] == ["c5"]
+    assert v["firmware"]["latest"] == "0.8.1"
+
+
+def test_matching_tree_and_server_render_no_restart_cue(monkeypatch) -> None:
+    """AC2: with tree == running build, the banner does not render."""
+    monkeypatch.setattr(card_context, "_declared_fw_version", lambda: "0.8.1")
+    v = _versions_block(
+        [_dev("classic", "0.8.1")], {"version": "0.8.1", "stale": False}
+    )
+    assert v["app"] == v["server"] == "0.8.1"
+    assert v["app_server_match"] is True
+    assert v["restart_needed"] is False
+
+
+def test_a_genuinely_newer_checkout_still_triggers_it(monkeypatch) -> None:
+    """AC3, the regression that matters: the cue must still fire when it is true."""
+    monkeypatch.setattr(card_context, "_declared_fw_version", lambda: "0.8.1")
+    v = _versions_block([_dev("classic", "0.8.1")], {"version": "0.8.1", "stale": True})
+    assert v["restart_needed"] is True
+
+
+def test_restart_needed_tracks_server_stale_exactly(monkeypatch) -> None:
+    """AC1's intent: the flag derives from the versions block's own facts.
+
+    The filing guessed at an mtime-vs-process-start comparison. There is none — the
+    flag is one expression, and `behind` was the other half of it. Pinning the
+    equivalence keeps a third condition from being folded in later without a cue and a
+    remedy of its own.
+    """
+    monkeypatch.setattr(card_context, "_declared_fw_version", lambda: "0.8.1")
+    for stale in (True, False):
+        v = _versions_block([_dev("a", "0.7.0")], {"version": "0.8.1", "stale": stale})
+        assert v["restart_needed"] is v["server_stale"] is stale