feat(meshcore): delete/purge messages — per-message, per-conversation, purge-all (#3981)

[Yeraze]

Closes #3981.

Summary

MeshCore messages (channel and DM) had no way to be deleted or purged from the UI, unlike Meshtastic sources. This PR adds the full stack — per-message delete, per-conversation/channel clear, and "purge all MeshCore messages for this source" — mirroring the existing Meshtastic purge patterns and honouring per-source scoping end to end.

Storage model note

All MeshCore messages live in one meshcore_messages table; channel identity is synthesised into channel-N pubkey columns and DMs use real pubkeys. Inbound DMs store the sender as a pubkey prefix while outbound store the full key, so DM conversation matching needs the same prefix semantics the frontend keysMatch uses — resolved in the manager, then deleted by id.

Backend

• Repository (src/db/repositories/meshcore.ts): source-scoped deletes — deleteMessageForSource, deleteMessagesByIds (chunked to stay under SQLite's bind-variable ceiling), deleteChannelMessagesForSource (reuses the existing channelWhereClause), deleteAllMessagesForSource, plus a lightweight getMessageEndpointsForSource. Every method requires a sourceId.
• Manager (src/server/meshcoreManager.ts): deleteStoredMessage, purgeConversation, purgeChannelMessages, purgeAllMessages. Each deletes scoped to its own source, prunes the in-memory message pool, and broadcasts a new meshcore:messages:deleted event.
• Routes (src/server/routes/meshcoreRoutes.ts): DELETE /messages, /messages/channel/:idx, /messages/conversation/:publicKey, /messages/:messageId — all requireAuth() + requirePermission('messages','write',{ sourceIdFrom: 'params.id' }) and audit-logged. (The single-message route param is :messageId, not :id, so it doesn't shadow the mount's source id.)

Frontend / UI approach

• Per-message delete: an optional onDeleteMessage callback on the shared MeshCoreMessageStream, rendering a hover-revealed 🗑️ next to the existing reply affordance — wired into both the DM view and the channel view (one component, both surfaces).
• Per-conversation / per-channel clear: a small "Clear conversation" / "Clear channel messages" toolbar button above the stream in each view.
• Purge all: a "Message data" danger section in MeshCoreSettingsView with a "Purge all messages" button. Chosen because the settings view is the natural home for a source-wide destructive action and already receives actions (no sourceId threading needed).
• The useMeshCore hook gains the four actions and a meshcore:messages:deleted socket handler that prunes local state; all destructive UI paths window.confirm first, matching the Meshtastic idiom.

Tests

• meshcore.messagePurge.perSource.test.ts — repository source-isolation (incl. chunking + refusing another source's id).
• meshcoreManager.messagePurge.test.ts — DM prefix matching, pool prune, event emit.
• meshcoreRoutes.test.ts — auth (401), permission-scoped routing, validation (400), 404s, and no cross-source leak.

Verification

• tsc --noEmit: clean.
• Full Vitest suite: 8080 passed, 0 failed (2 skipped files, 607 skipped, 21 todo).
• Lint: no new errors introduced (pre-existing NodeJS/no-undef noise from eslint . is unrelated to this change).

🤖 Generated with Claude Code

https://claude.ai/code/session_015AFBA76hsjqhsXe1BdnYub

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: feat(meshcore): delete/purge messages (#3981)

• Read changed files
• Review code quality and best practices
• Check for potential bugs
• Review security concerns
• Review test coverage
• Post final review

---

Overall Assessment

This is a well-structured, thorough implementation that follows existing project patterns carefully. The per-source scoping is correct throughout, the select-then-delete pattern ensures accurate counts, and the layered test coverage is solid. A few items worth addressing:

---

Bugs / Correctness Issues

1. deleteMessageForSource — unnecessary double-query on the hot path (src/db/repositories/meshcore.ts:1028)

The method does a SELECT to check existence, then a DELETE. This is a TOCTOU race (another source could delete the row between the two queries) and adds a round-trip. The existing deleteNode method already shows the idiomatic pattern for this repo: use getAffectedRows(result) on the delete directly.

// Current (two queries):
const existing = await this.db.select({...}).where(condition).limit(1);
if (existing.length === 0) return false;
await this.db.delete(meshcoreMessages).where(condition);
return true;

// Preferred (one query, matches deleteNode pattern):
const result = await this.db.delete(meshcoreMessages).where(condition);
return this.getAffectedRows(result) > 0;

2. deleteMessagesByIds — same double-query pattern repeated (src/db/repositories/meshcore.ts:1057-1063)

Same issue — SELECT before DELETE to count rows removed. The getAffectedRows helper exists for exactly this. With the chunked batch this also means 2N queries instead of N.

3. deleteChannelMessagesForSource and deleteAllMessagesForSource — unnecessary SELECT (src/db/repositories/meshcore.ts:1105-1111, 1126-1132)

Both pre-select to count, then delete. getAffectedRows would be cleaner and atomic. (Minor note: these are the only new methods where this is important — the existing deleteMessagesOlderThan has the same pattern pre-existing, so this is a consistency call rather than a new flaw.)

4. purgeConversation — potential false-positive when selfKey is undefined (src/server/meshcoreManager.ts:4982)

const selfKey = this.localNode?.publicKey;

If this.localNode is null (device not yet queried), selfKey is undefined. Then keysMatch(r.fromPublicKey, selfKey) returns false because of the falsy guard at line 4955. The exclusion of selfKey then silently fails, and a message where fromPublicKey equals the peer key could be matched incorrectly. This is a subtle edge case, but worth a defensive check or a log warning when selfKey is absent.

---

Security

5. Route registration order is correct but relies on implicit Express ordering (src/server/routes/meshcoreRoutes.ts:1462)

The comment on line 1462 ("Registered before /messages/:id so the two-segment path wins") is accurate and Express does what you expect here, but this is fragile — a future reorder of routes in this file could silently break the routing. Consider a comment in a test to assert which route handles DELETE /messages/channel/3 vs DELETE /messages/abc, or at minimum the existing test coverage of each path is sufficient to catch a regression.

6. conversationPublicKey in the socket payload is unvalidated on the client side (src/components/MeshCore/hooks/useMeshCore.ts:638)

In onMessagesDeleted, the client does:

const matches = (a?: string) => !!a && (a === pk || a.startsWith(pk) || pk.startsWith(a));

The pk comes directly from a socket event payload without any sanitization. Since the server controls what goes into emitMeshCoreMessagesDeleted, this is low-risk — but if the event payload were ever malformed (e.g. pk = ''), pk.startsWith(a) would be true for any a, wiping the entire message list. A pk.length > 0 guard would be defensive.

---

Code Quality / Minor Issues

7. Duplicated channel-filter logic in three places

The same channel filter appears in:

• meshcoreManager.ts:purgeChannelMessages (in-memory prune)
• useMeshCore.ts:onMessagesDeleted (client socket handler)
• useMeshCore.ts:clearChannelMessages (optimistic local update)

The clearChannelMessages optimistic update and the onMessagesDeleted handler apply nearly identical logic independently. The server broadcast → client prune double-apply is intentional and documented, but the filter logic itself is duplicated (three places on the client, one on the server). Extracting a filterChannelMessages(messages, channelIdx) helper would reduce drift risk.

8. deleteMessage in useMeshCore.ts does optimistic local prune AND the server broadcasts a deletion event (src/components/MeshCore/hooks/useMeshCore.ts:1370-1372)

The comment at line 1360-1362 acknowledges the double-apply is "idempotent" and "harmless" — that's correct. Minor cosmetic note only.

9. Clear channel button visibility condition (src/components/MeshCore/MeshCoreChannelsView.tsx:632)

{canSend && filtered.length > 0 && (
  <div className="meshcore-conversation-toolbar">

The "Clear channel messages" button only appears when there are visible messages (filtered.length > 0). This hides it after a successful clear (since filtered becomes []), which is fine UX-wise but could confuse a user who thinks nothing happened when no messages are shown. Consider showing it whenever canSend is true, or adding a brief "Cleared" toast — but this is a UX preference, not a bug.

10. publicKey validation allows 2-char minimum in the route (src/server/routes/meshcoreRoutes.ts:1493)

if (!/^[0-9a-f]{2,64}$/.test(publicKey)) {

The PR description mentions "pubkey prefix" semantics, so a 2-char minimum makes sense. However, a 2-char prefix would match an extremely large number of conversations (most contacts share the first byte). Consider documenting the minimum practical prefix length or enforcing a higher floor (e.g. 8 chars = 4 bytes for reasonable uniqueness).

---

Test Coverage

The test coverage is genuinely good:

• ✅ Repository source-isolation tests with chunking boundary (1200 rows → 2 chunks of 500+500+200)
• ✅ Manager unit tests for DM prefix matching, channel filter, pool prune, event broadcast
• ✅ Route tests for 401/403/400/404 flows and no cross-source leak via unregistered source
• ⚠️ No test for purgeConversation when selfKey is null/undefined (the edge case in item feat: traceroute request tracking and auth documentation #4 above)
• ⚠️ No test for the socket onMessagesDeleted client-side filter (empty pk edge case in item feat: telemetry parsing, direct messages, danger zone, and UX improvements #6)
• ⚠️ No test that the "Clear channel" button is absent when filtered is empty (item feat: Add GitHub Container Registry publishing and pre-built images #9) — minor

---

Summary

Priority	Item
Medium	Items #1–3: double-query pattern (correctness + performance)
Low	Item #4: selfKey undefined edge case in purgeConversation
Low	Item #6: unvalidated socket payload pk in client filter
Suggestion	Item #10: document/tighten min prefix length
Cosmetic	Items #7, #8, #9

The core implementation is sound, permissions are correctly scoped, the event pipeline is clean, and the existing tests are comprehensive. The most actionable fixes are the double-query patterns in the repository (#1–3) which can be made atomic with getAffectedRows.

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: feat(meshcore): delete/purge messages (#3981)

• Read changed files
• Review code quality and best practices
• Check for potential bugs
• Review security concerns
• Review test coverage
• Post final review

---

Overall Assessment

This is a solid, well-structured implementation that closely mirrors the existing Meshtastic purge patterns. Per-source scoping is correctly enforced end-to-end, auth + permission middleware is consistently applied, and the layered tests provide good coverage. A few areas worth addressing before merge:

---

Bugs / Correctness

1. Double-query pattern in all four delete methods (src/db/repositories/meshcore.ts:1028–1131)

Every new delete method does a SELECT to count/check existence, then a separate DELETE. This creates a TOCTOU race (another request could remove the row between queries) and doubles the DB round-trips. The existing deleteNode pattern in this repo uses getAffectedRows(result) instead:

// Current (two queries — racy + slow):
const existing = await this.db.select({id: ...}).from(...).where(condition).limit(1);
if (existing.length === 0) return false;
await this.db.delete(meshcoreMessages).where(condition);
return true;

// Preferred (one atomic query):
const result = await this.db.delete(meshcoreMessages).where(condition);
return this.getAffectedRows(result) > 0;

This affects deleteMessageForSource (line 1028), deleteMessagesByIds (lines 1057–1063), deleteChannelMessagesForSource (lines 1105–1111), and deleteAllMessagesForSource (lines 1126–1132). For deleteMessagesByIds specifically, the double-query means 2N DB round-trips instead of N for large batches.

2. purgeConversation — silent exclusion failure when selfKey is undefined (src/server/meshcoreManager.ts:5237)

const selfKey = this.localNode?.publicKey;
// ...
const fromPeer = !MeshCoreManager.keysMatch(r.fromPublicKey, selfKey)
  && MeshCoreManager.keysMatch(r.fromPublicKey, publicKey);

If this.localNode is null (device hasn't responded to SelfInfo yet), selfKey is undefined. keysMatch(r.fromPublicKey, undefined) returns false (due to the falsy guard), so the self-key exclusion silently becomes a no-op. In the worst case where publicKey happens to equal part of our own key prefix, we'd match our own outbound messages as "other peer" rows. A defensive guard or log warning when selfKey is absent would be safer:

if (!selfKey) {
  logger.warn(`[MeshCore:${this.sourceId}] purgeConversation: localNode has no publicKey; self-exclusion skipped`);
}

---

Security

3. Unvalidated socket payload pk in client-side filter (src/components/MeshCore/hooks/useMeshCore.ts:684)

const matches = (a?: string) => !!a && (a === pk || a.startsWith(pk) || pk.startsWith(a));

The pk comes from a socket event payload directly. If pk were '' (empty string), pk.startsWith(a) evaluates to true only for empty a, so that specific case is actually safe. But if pk were undefined (malformed event), the expression would throw. Since the server controls what's emitted via emitMeshCoreMessagesDeleted this is low-risk in practice, but a typeof pk === 'string' && pk.length > 0 guard would be defensive.

4. Route registration relies on implicit Express ordering (src/server/routes/meshcoreRoutes.ts:1462)

The comment correctly notes that /messages/channel/:idx must be registered before /messages/:messageId. This is fragile — a future reorder silently breaks routing. The existing test suite covers each path (good!), but consider an inline assertion or explicit comment block to document the ordering constraint.

5. publicKey validation allows 2-char minimum (src/server/routes/meshcoreRoutes.ts:1493)

if (!/^[0-9a-f]{2,64}$/.test(publicKey)) {

A 2-hex-character prefix matches ~1/256 of all contacts — far too broad for practical use. The PR description mentions prefix semantics, so document the minimum useful prefix length (8+ chars = 4 bytes for reasonable uniqueness) or enforce a higher floor. At minimum, a comment explaining why 2 is acceptable would prevent future confusion.

---

Code Quality

6. Channel-filter logic duplicated in three places

The same filter expression appears in:

• meshcoreManager.ts:5270–5274 (in-memory pool prune)
• useMeshCore.ts:674–680 (socket onMessagesDeleted handler)
• useMeshCore.ts:1454–1458 (clearChannelMessages optimistic update)

The double-apply (optimistic + server broadcast) is intentional and documented. However, the filter logic itself appears in three independent locations — if channelWhereClause logic changes, all three need updating. Extracting a shared isChannelMessage(msg, channelIdx) predicate would reduce drift risk.

---

Test Coverage

The test coverage is genuinely comprehensive:

• ✅ Repository source-isolation with chunking boundary (1200 rows → multiple 500-row batches)
• ✅ deleteMessageForSource cross-source rejection
• ✅ Manager unit tests: DM prefix matching, pool prune, event broadcast
• ✅ Route tests: 401/403/400/404 flows and no cross-source leak via unregistered source
• ⚠️ No test for purgeConversation when selfKey is undefined (item feat: traceroute highlighting and UI improvements #2 above)
• ⚠️ No test for the onMessagesDeleted socket handler with an empty/malformed pk payload (item "Update Claude PR Assistant workflow" #3)
• ⚠️ The deleteMessagesByIds mock in meshcoreManager.messagePurge.test.ts:42 ignores the sourceId arg — this is correct for a manager unit test (SQL scoping is tested separately) but worth a note

---

Summary

Priority	Item
Medium	Items #1: double-query → use getAffectedRows for atomicity + performance
Low	Item #2: selfKey undefined edge case — add defensive log
Low	Item #3: add typeof pk === 'string' && pk.length > 0 guard
Suggestion	Item #5: document/tighten min prefix length
Cosmetic	Items #4, #6

The core implementation is sound, permissions are correctly scoped, the event pipeline is clean, and the test suite is thorough. The most impactful fix is the double-query pattern (#1) — it's both a correctness issue (TOCTOU) and a performance issue for bulk operations.

---