fix(masthead): restart_needed means a restart HELPS — behind boards get their own cue - #1722
Merged
Merged
Conversation
…et their own cue
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
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1712. All three ACs, though the diagnosis in the filing was wrong and the real one is sharper.
There is no mtime comparison
The filing guessed at "an mtime-vs-process-start comparison that trips on file touches that aren't builds." There isn't one. The flag was a single expression:
Observed state had
server_stale: false— sobehindwas the trigger. That's a logical certainty from the code, not an inference: nothing else writes this flag.Two conditions, one sentence, different remedies
server_stalebehind(board on older fw)The operator was told to do something that cannot work.
The part I didn't expect
The masthead already has the right cue for behind boards:
It's an
else ifafterrestart_needed. So whilebehindfed that flag, the accurate message was unreachable — every behind board rendered the restart prompt instead. The right words were written, shipped, and could never appear.Fixing the flag is what lets that cue render for the first time.
The change
restart_needed = server_stale.behindis untouched and still published in the firmware block — it just stops claiming a restart is the answer.One test asserted the bug
test_device_behind_latest_firmware_is_flaggedassertedrestart_needed is Truefor a behind board — the defect written down as an expectation. Amended rather than deleted, with the reason in place, so a reader hitting that diff sees a corrected model rather than a fixture tweak.ACs
restart_neededtracksserver_staleexactly, so a third condition can't be folded in later without bringing its own cue and remedy.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 rather than an honesty one — the cue that remains is true in that state. Flagging it rather than quietly widening scope.just checkgreen; 16 masthead tests.Lane note:
card_context.pyis analytics. The board assigns this to DX and the ACs are precise, so I took it — but if Data would rather own changes in that module, say so and I'll route it instead of assuming the board settles it.— DX