fix(meshcore): anon auth banner, channel-sync data loss, and reply/trigger ordering

[Yeraze]

Three MeshCore fixes found by driving the running app. All pre-existing — none introduced by the #4473 nav work (verified against both of its merge commits).

Supersedes #4489, whose commit is included here (it was stacked on the pre-squash version of #4488 and had gone conflicting).

---

1. Spurious "Authentication required" over the channel view

An anonymous viewer with read grants got a page-level banner that reads as "you can't view this channel", on a page that was otherwise working.

Reproduced in an anonymous session — two of four failing calls were the cause:

call	status
config/default-scope	401 ← the banner
saved-regions	401 ← the banner

Both are requireAuth() + configuration: read and exist only to populate the scope-override datalist — a send-side control a read-only viewer cannot use. fetchSavedRegions surfaced the raw 401 body through setError, turning {"error":"Authentication required"} into a banner.

• Gate both lookups on canSend — never requested for a viewer who cannot send.
• Drop the setError, matching getDefaultScope directly above it. Optional autocomplete suggestions must not raise a page-level error.

Permissions UI: connection moves to the top of RESOURCES, reworded to "Required to open a source." MeshCoreSourcePage gates its entire surface on connection: read, so without it every other grant is inert and the operator sees only "You do not have permission to view this MeshCore source". Note that gate is MeshCore-only — no Meshtastic path checks it, despite the comment there claiming parity.

2. Channel rows deleted for slots the device never reported

syncChannelsFromDevice() reconciled by deleting rows absent from the scan. getChannels() enumerates until the firmware errors, so a partial scan returns success with a short list rather than failing — and every unlisted slot was treated as deleted.

Absent is unknown, not empty. Deletion now requires the firmware to have positively reported the slot as unconfigured; unreached slots are preserved and logged at warn. This mirrors the decision refreshContacts already makes at the adjacent call site ("deliberately won't wipe on empty").

Scope note, stated plainly: this is hardening, not the fix for the report that prompted it. Debug logging showed that device returned its full 40-slot table with 39 empty (Synced 1 configured channel(s) ... filtered out 39 empty slot(s)) — a complete scan of a device that genuinely no longer had the channel, so the reconcile was correct. The partial-scan hazard is real regardless: the sibling source was simultaneously failing get_channels with a native command timeout.

3. Auto-responder replies sorted above their own trigger

Captured from the running app:

trigger (ours)   id sent-1785604213050-…   timestamp 1785604213050
reply   (remote) id 1785604214478-…        timestamp 1785604213000

The reply was observed 1.4 s after the trigger but carried a timestamp 50 ms earlier, because the two directions share neither a clock nor a resolution:

received: timestamp = sender_timestamp * 1000   ← remote, WHOLE SECONDS
sent:     timestamp = Date.now()                ← ours, milliseconds

Sub-second precision on our own sends is precision we can never have for the other side, and trusting it is what produced the inversion.

Adds receivedAt — MeshMonitor's own clock, stamped identically at all six message-creation sites (3 received, 3 sent) — plus a shared comparator in messageOrder.ts: compare timestamp at second granularity (the wire's real resolution), tie-break on receivedAt, fall back to id so the order is total and never depends on input sequence. Display still uses timestamp.

No migration: meshcore_messages already persists createdAt with exactly this meaning, so rehydration maps createdAt → receivedAt and existing history reorders too. Legacy rows without it fall back to timestamp.

The DM view is unchanged — it doesn't sort, inheriting pool arrival order, which is already causally correct.

Verified on the running container

• Anonymous session: banner gone, neither auth-only endpoint requested at all.
• Ordering, on the exact reported pair — trigger now precedes its reply despite both rendering 1:10:13 PM. A second pair sent later has byte-identical timestamps across three messages; only receivedAt separates them, and they order correctly.
• Historical rows reordered with no re-sending, via the createdAt mapping.

Test plan

• 3 tests for the sync guard (truncated scan, empty scan, reported-empty still deletes).
• 7 tests for the comparator, built from the real captured values — including one asserting the premise, that a naive timestamp sort genuinely inverts them, so the fix can't be quietly neutered.
• 2 tests for the read-only gating.
• All confirmed real regressions: reverting each fix fails exactly its own tests (3, 2, 1).
• Full Vitest suite — success: true, 12966 passed, 0 failed.
• npx tsc --noEmit clean; npm run lint:ci no FAIL lines.

Follow-ups not addressed here

• channel_0..7 are offered on MeshCore sources but are inert — no MeshCore code path consults channel_N. Granting channel_0 there looks like it should grant channel access and does nothing; channel content needs messages: read (the server names it in its 403 body). Hiding them for MeshCore sources would be its own change.
• get_channels intermittently times out on these companions, so a source's channel list can silently never sync. Worth its own issue.

---

Generated by Claude Code

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: fix(meshcore): anon auth banner, channel-sync data loss, and reply/trigger ordering

• Read changed files
• Review Fix 1: Auth banner / read-only gating
• Review Fix 2: Channel sync data loss
• Review Fix 3: Message ordering
• Review tests
• Post final feedback

---

Overall this is a high-quality, well-reasoned set of fixes. The PR description is unusually thorough, tests directly encode the real captured failure values, and the behavioral scope is tightly limited. A few observations below.

---

Fix 1 — Spurious auth banner (MeshCoreChannelsView.tsx, useMeshCore.ts)

Approach is correct. Gating both getDefaultScope and fetchSavedRegions on canSend is the right call: both routes require requireAuth() + configuration: read, and both exist exclusively to populate the send-side scope control. A viewer who cannot send has no use for either.

One minor note: canSend is derived from hasPermission('messages', 'write') (MeshCoreChannelsView.tsx:104). This conflates channel send with general message write. The PR description notes the scope-control endpoints need configuration: read, but the guard chosen is messages: write. These are effectively the same population in practice (a read-only viewer has neither), but documenting the intent in a comment near the canSend derivation would help a future reader understand why configuration: read isn't checked directly here.

fetchSavedRegions error handling (useMeshCore.ts:1036-1045): the new implementation silently returns null on failure instead of surfacing the error. The doc-comment explains the reason clearly. This matches the comment in getDefaultScope above it. No issue here — just noting the pattern is intentional per the comment.

---

Fix 2 — Channel sync data loss (meshcoreManager.ts, syncChannelsFromDevice)

Logic is correct. The reconcile now requires reportedIdxSet.has(row.id) before issuing a delete (meshcoreManager.ts:2492-2497). The distinction "reported AND empty" vs "not reported at all" is the right invariant, and the warn log at line 2503 surfaces the partial-scan case for operators without being fatal.

One concern: warn log is suppressed per-message. The counter is incremented in a loop over existing rows (preserved++), but the log fires once with the total count. That's actually good — a single warn per sync rather than N warnings. No issue.

listChannels throws on firmware error (meshcoreManager.ts): syncChannelsFromDevice calls this.listChannels() without a try/catch. The caller at meshcoreManager.ts:1195-1197 does wrap it, so a firmware error propagates cleanly and produces the [MeshCore:…] syncChannelsFromDevice failed: log. No problem — the design is intentional.

Test coverage for the "partial scan" scenario is excellent and directly represents the described failure mode. The four distinct cases (truncated, empty, positive-empty, mixed) are all tested.

---

Fix 3 — Message ordering (messageOrder.ts)

Design is sound. Using per-second granularity for the primary sort key correctly normalises the clock mismatch, and receivedAt as the tie-break gives a causal total order. The id-based final fallback ensures the sort is stable even for legacy rows.

Graceful fallback for legacy rows (messageOrder.ts:42-44):

function observedAt(m: ...): number {
  return typeof m.receivedAt === 'number' ? m.receivedAt : (m.timestamp ?? 0);
}

When receivedAt is absent, this falls back to timestamp. For two legacy messages in the same second-bucket whose timestamps differ by sub-second, the tie-break will use those ms-precision timestamp values — which re-introduces the original clock mismatch, but only for same-second legacy pairs. The impact is minimal (legacy rows from before this fix, same second bucket). The id fallback below it handles identical timestamps. The test 'falls back to timestamp when receivedAt is absent (legacy rows)' verifies stability but not ordering for this case — not a bug, just noting the fallback isn't wrong, it's just the best available approximation.

compareMeshCoreMessages is not exposed on the type interface (useMeshCore.ts): MeshCoreMessage is defined in the hook file; compareMeshCoreMessages takes Pick<MeshCoreMessage, 'id' | 'timestamp' | 'receivedAt'>. The receivedAt field is defined both on the backend's internal message type (meshcoreManager.ts:573) and on MeshCoreMessage in useMeshCore.ts:133. These are parallel type definitions — no structural drift visible here, though it's worth watching over time.

receivedAt stamped at 6 sites (meshcoreManager.ts: 1127, 1623, 1663, 1702, 2605, 3257, 4935, 5992). The rehydration from createdAt at lines 1127 and 5992 is the clever part — existing history gets the benefit of the new ordering with no migration. Verified this is mapped from dbMsg.createdAt, which is the DB's insertion time and therefore the correct semantic.

---

Test coverage

• 12966 tests passing, 0 failing per the PR description — no concern.
• Tests encode the real captured values from the bug report. The guard test ("naive sort genuinely inverts them") at messageOrder.test.ts:38-41 is particularly strong — it documents the premise, not just the fix.
• The "read-only gating" tests (MeshCoreChannelsView.test.tsx) test that getDefaultScope and fetchSavedRegions are not called for a non-sender, which is the right assertion for a guard that works by not making the request.

---

Minor nits (non-blocking)

1. src/types/permission.ts connection description change — now reads "Required to open a source. Also controls connect/disconnect." This is accurate and more informative than before. The ordering move (first in RESOURCES) with the rationale comment is helpful for admins reading the permissions UI.

2. The RESOURCES comment (permission.ts:139-149) clarifies that the MeshCore-only connection: read gate has no Meshtastic parallel. The PR description calls this out as a follow-up concern but doesn't address it — that's appropriate scope discipline. The comment makes the asymmetry visible to the next reader.

3. fetchSavedRegions effect dependency (MeshCoreChannelsView.tsx:357): sourceId is in the dep array for fetchSavedRegions but not for getDefaultScope (line 340), which only has canSend and status?.connected. This means switching sources re-fetches saved regions but not the default scope until the connection state changes. Low-impact (the scope will refresh on reconnect), but the inconsistency is worth noting.

---

Summary

All three fixes are correct, well-tested, and appropriately scoped. The channel-sync hardening in particular addresses a real data-loss class (transient firmware timeout → permanent channel deletion) with a principled "absence ≠ deletion" invariant. The ordering fix is elegant in its use of createdAt → receivedAt mapping to retro-fix history without a migration. Approve.