feat(traceroute): participation picker on all sources — brings the strip to MQTT (epic phase 2)

[Yeraze]

Summary

Phase 2 (final) of the Traceroute Strip Interactivity epic (plan: docs/internal/dev-notes/TRACEROUTE_STRIP_INTERACTIVITY_EPIC.md, spec: TRS_PHASE2_SPEC.md). Adds a per-node traceroute picker to the Node Details traceroute section on every Meshtastic-family source — which is what makes the strip appear on MQTT sources at all, since they have no origin node and therefore no own-request traceroute.

Backend

• tracerouteParticipationKind() (src/utils/tracerouteSegments.ts): pure predicate classifying a row as endpoint / hop / not-involved. Number()-coerced against PG/MySQL BIGINT strings; hop membership via parseHopArray, immune to the substring bug a LIKE '%num%' approach would ship (regression test included).
• TraceroutesRepository.getTraceroutesInvolvingNode(): one bounded (2000), newest-first, source-scoped query-builder scan + JS filter — no raw SQL. Overflow can only drop older participations.
• GET /api/traceroutes/participation/:nodeNum — requirePermission('traceroute','read', {sourceIdFrom:'query'}), sourceId required (400 MISSING_SOURCE_ID, no ALL_SOURCES fallback), envelope via ok/fail, maskTraceroutesByChannel applied ([BUG] MeshCore telemetry retrieval fails on every repeater (meshcore-only setup, v4.6.1) #3092), routePositions stripped from the projection, honest hopCount: number | null (no 999 sentinel).

Frontend

• useNodeTraceroutes (one-shot TanStack query, 7-day window) + TracerouteParticipationPicker (native <select>, renders only at ≥2 entries; labels = date · endpoints · hop count, relay-hop entries marked "relayed").
• MessagesTab coexistence rules: displayed row = explicit pick, else newest-of-both (poll row vs entries[0]), so the shipped TCP request-flow (pending/failed badges, "last traced") stays byte-identical — badges and age now describe the displayed row. Pick clears on node change and on a fresh traceroute response. The render guard moves from recentTrace to the displayed row — the MQTT unlock.
• Box gate widened write → read || write (the box is a display; the request button still requires write). 6 new en.json keys, flat with inline defaults.

Testing

• Full Vitest suite: 12,512 passed / 0 failed (success: true, JSON reporter). New: predicate tests (incl. substring trap), repository + perSource isolation tests, route tests via createRouteTestApp (permission, missing-sourceId, bounds), ApiService test, picker component tests, MessagesTab gate/coexistence tests.
• tsc --noEmit clean; lint ratchet passes.
• Live-validated in the dev container:
  • Florida MQTT source, node !04c5b85c: picker lists 100 (capped) dated entries; strip renders on an MQTT source for the first time; picking a 2-hop entry re-renders 2→7 glyphs, "last traced" follows the pick; Phase 1 edge tooltips work on picked rows.
  • Sandbox TCP source regression: BOCA G2 strip unchanged, picker shows 4 entries incl. a "relayed" hop-participation label.
  • No new console errors.

Closes out the epic (Phase 1 was #4424).

🤖 Generated with Claude Code

https://claude.ai/code/session_01BTfGTsPBskKYJUK8SSvYob

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: feat(traceroute): participation picker on all sources

Tasks

• Gather context and understand the PR
• Review backend changes (predicate, repository, route)
• Review frontend changes (hook, component, MessagesTab)
• Review test coverage
• Check security and performance
• Post final review

---

Overall Assessment

Solid, well-structured feature addition. The code is clean, follows project conventions, and the PR description accurately reflects the implementation. The test coverage is thorough. I have a few observations ranging from minor nits to one worth discussing.

---

Backend

tracerouteParticipationKind (src/utils/tracerouteSegments.ts:303)

Clean predicate. The Number() coercion for PG/MySQL BIGINTs is correct and well-documented, and reusing parseHopArray avoids the substring bug LIKE '%num%' would have. Endpoint-wins-over-hop ordering is a reasonable design choice.

One edge case worth documenting (not a bug): a node that appears in both route and routeBack of a traceroute it's not an endpoint of returns 'hop' — this is correct by the predicate's ordering, but the comment on the endpoint-wins rule only mentions the origin-and-relay case. If this comes up in a user bug report it may cause confusion.

getTraceroutesInvolvingNode (src/db/repositories/traceroutes.ts:182)

The scan-then-JS-filter approach is a reasonable workaround for the JSON-column-search problem across three backends; the 2000-row scanLimit is documented and the "drops oldest" property is made explicit. One observation: the method accepts opts.sourceId: SourceScope but the route handler always supplies a concrete string (never ALL_SOURCES). It would be marginally safer to type the parameter as string here to make the "always-scoped" invariant impossible to violate accidentally, but this is a minor style preference.

Route GET /api/traceroutes/participation/:nodeNum (src/server/routes/tracerouteRoutes.ts:113)

• The requirePermission('traceroute', 'read', { sourceIdFrom: 'query' }) middleware runs before the !sourceId check at line 118. When sourceId is absent the middleware resolves it from query to undefined, which may cause the permission check to fall back to a global permission rather than source-scoped. This is worth verifying with the harness — the test at line 85 passes only because harness.admin has global write, not because the 400 fires before auth. If a read-only user with no sourceId skips through the middleware and then hits the 400, the 400 is correct, but the ordering means a missing sourceId isn't caught as early as possible.

• The hopCount computation at line 166 calls parseHopArray which is already called inside tracerouteParticipationKind for every candidate row. For the happy path this means each matched row parses its route twice. At 100-row cap this is negligible, but a shared parse at the scan level would eliminate the duplication.

• routePositions is correctly excluded from the projection (security: [BUG] MeshCore telemetry retrieval fails on every repeater (meshcore-only setup, v4.6.1) #3092). Well done.

• The sentinel-free hopCount: number | null instead of the 999 used elsewhere is a deliberate improvement — the PR description calls this out, and it's the right call for a picker label.

---

Frontend

useNodeTraceroutes (src/hooks/useNodeTraceroutes.ts)

Using useResolvedSourceId() rather than useSource() to handle the outside-SourceProvider case is the correct approach. staleTime: 60_000 is reasonable. refetchOnWindowFocus: false prevents noisy refetches. No issues.

TracerouteParticipationPicker (src/components/traceroute/TracerouteParticipationPicker.tsx)

• entries.length < 2 early-return is the right call — a single-entry dropdown adds chrome with no choice.
• The eslint-disable-next-line react-refresh/only-export-components comment on buildOptionLabel is correctly scoped and issue-referenced ([FEAT] Color-code node roles in traceroute display #4381).
• CSS module is used correctly; no global sheet modifications.
• Minor: the select element has both value and onChange but when selectedId is null and entries.length >= 2, value="" will not match any option's value (which are stringified IDs), so the browser will show no selection, effectively showing whatever the browser picks as "uncontrolled". This is probably the intended UX (first option visible by default), but it means the value prop is a noop in the null-selectedId state. Consider whether value={selectedId != null ? String(selectedId) : String(entries[0]?.id)} is clearer intent, or document the current behaviour explicitly.

MessagesTab.tsx coexistence logic (src/components/MessagesTab.tsx:795-802)

The three-way priority logic (explicit pick → newest-of-both by timestamp → fallback) is well-reasoned and the comment explains all the cases. One subtle point: when pickedTracerouteId is set but the participation list is refetched and that ID no longer appears (e.g. entry aged out of the 7-day window), pickedEntry becomes null and newestAvailable silently takes over. This is correct per the spec and the comment at line 793, but no badge indicates to the user that their pick disappeared — this might be surprising if a user picks a ~7-day-old entry and the next refetch drops it. Low priority, but worth a follow-up issue if not already tracked.

isPending / isFailed logic (src/components/MessagesTab.tsx:2034-2038)

These badges fire when forwardFailed && returnFailed — i.e. both route and routeBack are null/absent. For a 'hop'-participation entry the route is never null (the node is in it), so this path is unreachable for a pure-hop pick. For an 'endpoint' pick that was seeded with a pending row (no route yet), the logic correctly shows "Pending". This is fine, but worth noting that the badge semantics become moot for hop picks since a hop can't exist in a null-route row.

Box gate widening (src/components/MessagesTab.tsx:2010)

hasPermission('traceroute', 'read') || hasPermission('traceroute', 'write') — the comment at line 2005-2009 explains the rationale (non-regressive for write-without-read users). This is sound. The spec reference (§6.4) is helpful.

---

Test Coverage

Tests are comprehensive and well-structured:

• tracerouteParticipationKind unit tests include the substring-trap regression, malformed JSON, and string-typed node numbers — all the right cases.
• Route tests use createRouteTestApp() per CLAUDE.md convention, with permission/source isolation, bounds checks, and the missing-sourceId 400.
• Repository perSource isolation test exists and follows the prescribed pattern.
• MessagesTab.tracerouteStrip.test.tsx correctly stubs useNodeTraceroutes to prevent real network calls under jsdom.

One gap: there's no test exercising the newestAvailable coexistence rule directly (poll row newer than entries[0] → shows poll row; entries[0] newer → shows entries row). The MessagesTab component test is complex to set up for this, but it's the most nuanced part of the feature.

---

Security

• requirePermission('traceroute', 'read', { sourceIdFrom: 'query' }) is present and correctly scoped.
• maskTraceroutesByChannel is applied before the projection (line 148), matching the existing channel gate on other endpoints.
• routePositions is excluded from the response projection — correct.
• No raw SQL introduced.
• sourceId is required (no silent ALL_SOURCES fallback) — good design.

---

Summary

The PR is in good shape. The main points worth a second look before merge:

1. isPending/isFailed on a hop-picked entry with null route: confirms the badge logic is unreachable (correct), but consider whether hop entries should suppress those badges explicitly for clarity.
2. select value="" when selectedId is null: document or fix the implicit "first option" UX.
3. scanLimit SourceScope type on getTraceroutesInvolvingNode: minor — narrowing to string would enforce the always-scoped invariant.
4. Missing coexistence rule test: low priority follow-up.

None of these are blockers. The feature is well-reasoned and the implementation correctly follows project conventions.

[Yeraze]

Review feedback triage — addressed in the latest commit:

• feat: traceroute highlighting and UI improvements #2 select value=\"\" implicit first-option — fixed: a hidden "Latest" placeholder now renders for the null-selection state, so the picker never implies a selection that wasn't made. The test that documented the old browser-fallback quirk now asserts the placeholder.
• "Update Claude PR Assistant workflow" #3 sourceId: SourceScope on the repository method — narrowed to string with a comment; an ALL_SOURCES caller is now unrepresentable.

Declined:

• "Claude PR Assistant workflow" #1 badge suppression for hop-picked entries — the review itself confirms the current logic is unreachable-therefore-correct; adding an explicit suppression path would be dead code guarding a state that can't occur.
• feat: traceroute request tracking and auth documentation #4 extra coexistence-rule test — agreed it's worthwhile, deferred as a low-priority follow-up per the review's own framing; the shipped tests cover the newest-of-both and pick-reset rules.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BTfGTsPBskKYJUK8SSvYob

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: feat(traceroute): participation picker on all sources

Tasks

• Gather context and understand the PR changes since last review
• Review fixes for previously flagged items (feat: traceroute highlighting and UI improvements #2 select value, "Update Claude PR Assistant workflow" #3 sourceId type)
• Check for any new issues introduced by the fixes
• Post final review

---

Follow-up Review

Both addressed items are clean.

Fix #2 — select value="" implicit first-option (TracerouteParticipationPicker.tsx:93-97)

The hidden "Latest" placeholder option is the correct solution. When selectedId is null, value="" now has an actual matching <option value="" hidden> to land on, so the browser displays "Latest" rather than silently promoting the first real entry's label. The placeholder is rendered conditionally — it vanishes the moment a real pick is active, so it can't be selected again. Well done.

The test at line 275-314 covers exactly the right cases: selectedId: 7 binds correctly, selectedId: null transitions without throwing, select.value is "", the placeholder is present and hidden, and its text is "Latest". No gaps.

Fix #3 — sourceId: string on getTraceroutesInvolvingNode (traceroutes.ts:184-188)

The inline comment explains the narrowing rationale clearly: "Narrowed to string (not SourceScope): the participation picker is per-source by definition, so an ALL_SOURCES caller is a bug this signature makes unrepresentable." This is exactly the right approach — the type system enforces the invariant rather than relying on call-site discipline.

No new issues introduced. The two changes are minimal and additive; the rest of the implementation is unchanged.

---

The two outstanding actionable items from the previous review are resolved. The declined items (#1 dead-code badge suppression, #4 deferred coexistence test) remain reasonable calls given the justifications provided. This PR is ready to merge.