fix(frontend): relay-aware staleness for infra nodes + dim-not-delete (#1598, PR A)

[Saarlandpower]

Implements PR A from the #1598 triage fix path (r6 — all three operator decisions locked: confidence ≥0.75 [PR B], dim-not-delete, in-place getNodeStatus signature extension).

Changes

public/roles.js — getNodeStatus()

• Now accepts a full node object (preferred); the legacy (role, lastSeenMs) signature keeps working unchanged.
• For infra roles (repeater/room), freshness = max(advert-based timestamp, last_relayed). Freshness precedence mirrors existing call sites: _liveSeen > _lastHeard > last_heard > last_seen.
• last_relayed is only consulted for infra — companions keep pure advert/heard-based staleness (per @liquidraver's collision caveat; the ≥0.75-confidence _liveSeen refresh is PR B).

public/live.js — pruneStaleNodes()

• Repeater/room markers are dimmed, never deleted, regardless of _fromAPI origin. WS-only non-infra nodes are still removed to prevent unbounded memory growth.

Call sites — all seven (nodes.js ×3, map.js ×3, live.js ×1) now pass the node object. The Nodes-page status explanation shows "Last relayed …" when relay participation is the fresher signal, so an Active badge next to an old "last heard" isn't confusing.

Tests — 16 new getNodeStatus unit tests incl. the triage's backbone-repeater fixture (last_seen=25h, last_relayed=5min → active); pruneStaleNodes tests updated for dim-not-delete plus a new relay-aware prune test.

Test results

node test-frontend-helpers.js: 641 passed, 2 failed — the 2 failures (favStar ★ assertions) are pre-existing on current master (verified on a clean upstream clone).

Validation offer

live.saarmesh.de currently has 160 infra nodes past infraSilentMs=72h while actively relaying (incl. KatS disaster-relief repeaters) — a ready-made test population. Happy to run this branch there and report before/after node visibility.

Refs #1598 (PR A of two; PR B = _liveSeen refresh on resolved_path ≥0.75 confidence).

[Kpa-clawbot]

Bot polish review

Verdict: MERGE-READY

Findings

• [MINOR] public/nodes.js:207 — the "Last relayed" branch fires only when relayedMs > lastHeardMs, but lastHeardMs here is _lastHeard || last_heard || last_seen (no _liveSeen). Callers on the live view will therefore keep showing "Last heard" even when _liveSeen is fresher. In practice this only matters for infra nodes whose live-view marker survives after your dim-not-delete change; the ordering is still safe (never reports a value older than truth), just occasionally suboptimal wording. Consider mirroring the _liveSeen precedence you added inside getNodeStatus.
• [MINOR] public/nodes.js:192 — getStatusTooltip(role, status) receives only the role; the tooltip copy therefore doesn't mention "actively relaying" for the case where last_relayed is what pulled the node back to active. Not blocking (the details panel at nodes.js:634 already covers this), but a follow-up could enrich the tooltip.
• [MINOR] Commit shape — 415ee37e bundles the production change AND the new getNodeStatus test cases in the same commit; the follow-up 8e1c537b only updates the pre-existing pruneStaleNodes test. Strict TDD would want the failing tests as a separate red commit. Since this is a signature change to an existing helper (not a pure net-new surface), a red-then-green history would be preferable next time. Not a blocker — the tests DO assert real behavior (they'd fail if the relay-aware branch were reverted), which is what actually matters.
• [NIT] public/roles.js:462 — String(role).toLowerCase() is called twice for the infra check. Extract to var normRole = String(role || '').toLowerCase(); once. Cosmetic.
• [NIT] public/live.js:3095 — comment says "WS-only non-infra nodes are removed" but the condition is n._fromAPI || isInfra → else. The current comment is correct; just verifying the reviewer trace. No change needed.

Persona votes

• carmack: correctness is tight — signature is backward-compatible (accepts both (role, ms) and (node)), !(effectiveMs >= relayedMs) correctly handles the NaN/undefined case, infra gate uses lowercased role, thresholds unchanged. Ship it.
• tufte: dim-not-delete is the right call — a 0.35-opacity marker preserves "infrastructure exists here, advert stale" as data-ink instead of silently rewriting the map. Consistent with the existing API-loaded dim path. No visual clutter concern at typical mesh sizes.
• kent-beck: TDD verdict — tests are assertive (13 new getNodeStatus cases + 1 new relay-aware pruneStaleNodes case, all assert.strictEqual on real outcomes), cover legacy signature, node-object signature, _liveSeen precedence, _lastHeard precedence, companion exclusion, missing-role default, and no-timestamps. Would fail if the relay-aware branch were reverted. Not a textbook red→green history (see MINOR above), but the tests DO gate the behavior, which is the point.
• meshcore: last_relayed semantics honored correctly — only repeater/room roles get the max(advert, last_relayed) treatment; companions are explicitly excluded, matching liquidraver's collision caveat on last_relayed being a repeater-hop marker. Threshold (infraSilentMs) unchanged. Correct.

Merge gate

• mergeable: MERGEABLE
• mergeState: BLOCKED
• CI: action_required (community first-timer — needs maintainer to approve workflow runs)
• Tests: present, assert behavior — test-frontend-helpers.js covers the new signature and the relay-aware branch end-to-end

---

Automated polish review. Human maintainer makes the final call. This is a clean, well-tested change; the MINORs are follow-up material, not merge blockers.

[SaarMesh-Bot]

Cross-referencing today's triage rounds on #1611, since they land on this PR.

The #1611 re-triage this morning concluded that the cadence-adaptive threshold should be split out as a separate follow-up, and that the #1598 r6 fix is the critical path to ship first — assessed at effort S, described as a Math.max on a field /api/nodes already returns.

That implementation is this PR. Open since 2026-07-01, bot review MERGE-READY since 2026-07-02, implementing all three locked r6 decisions: max(advert, last_relayed) for infra roles, dim-not-delete in pruneStaleNodes, in-place getNodeStatus signature extension across all seven call sites, plus 16 unit tests including the backbone-repeater fixture from the triage.

mergeable_state is blocked — the CI run sits at action_required pending workflow approval for a fork PR. Nothing else is outstanding on it as far as I can tell.

No rush intended; flagging it only because the design discussion has now converged on this change three times today across #1611, #1845 and #1598, and it may not be obvious that the code is already sitting here. Happy to rebase if it has drifted against master.

[Kpa-clawbot]

Delta re-review — 37b6a1ee

Only new commit is a clean merge of origin/master into the branch. No author code change since prior review (2026-07-02, verdict: MERGE-READY, 0 BLOCKER / 0 MAJOR, 3 MINOR follow-ups only).

Verdict: still merge-ready. Prior findings unchanged.

Merge gate: mergeable=MERGEABLE · mergeStateStatus=BLOCKED · CI = action_required (first-time contributor — workflow runs need maintainer approval). Not auto-mergeable; operator action required to release CI.

[efiten]

Closing and immediately reopening to get this a CI run against current master. Nothing is wrong with your PR and no action is needed from you.

A plain re-run would reuse the merge commit from the original run and so would not include the #1923 fix, which was failing PRs that never touched the affected page. Context and the full list in #1922. Apologies for the notification noise.

[efiten]

Review from the queue triage. Written 2026-08-30 against the tree at that time; posting now that the maintenance window in #1922 has opened.

Verdict: approve the staleness half. One question on the pruning half.

The premise checks out. last_relayed is a real, populated field, not something the PR assumes
into existence: it is set at cmd/server/routes.go:1413 and :1610, documented in
openapi.go:191, computed by relayMetrics at store.go:4218, and already consumed by
analytics.js:2518 and nodes.js:643. So making getNodeStatus consider it for infra roles is
building on something that works. Note this is independent of the #1855 problem: that one is about
nodes.last_seen, this uses last_relayed, which is computed in memory and was never affected.

The dual signature (getNodeStatus(node) preferred, getNodeStatus(role, ms) still accepted) keeps
every existing caller working, and fifteen tests cover both forms including the "companion does not
get relay-based freshness" case. The odd-looking if (!(effectiveMs >= relayedMs)) is deliberate,
not a bug: the negated form assigns when effectiveMs is NaN, which a plain < would not.

The question is the pruning change, which is a bigger behaviour change than the title suggests.
pruneStaleNodes now exempts every infra node from deletion, not just API-loaded ones. The
existing comment says pruning exists "to prevent unbounded memory growth", and the PR's own
replacement comment concedes it by narrowing to "remaining WS-only non-infra nodes are
removed". On a large mesh, a long-lived Live tab accumulates one permanently-retained entry per
repeater or room ever seen via ADVERT, with nothing bounding it.

Dimming rather than deleting is the right call for the user-visible behaviour, so ask for a bound
rather than a revert: evict infra after a much longer window than the staleness threshold, or cap
the retained marker count.