feat(nodes): per-node free-text notes field (#3921)

[Yeraze]

Summary

Closes #3921. Adds an optional free-text notes annotation per node, editable from the node detail view and stored server-side so it persists across devices/browsers.

The official Meshtastic mobile clients (Android/Apple) expose a local per-node "notes" field that is purely client-side app state — no protobuf/admin message involved. This brings the same capability to MeshMonitor, scoped per source like other per-node local metadata (favorites / ignored / hide-from-map). It is never synced to or from the mesh.

Changes

Backend

• Migration 112 (112_add_notes_to_nodes) adds a nullable nodes.notes column (TEXT on SQLite/PostgreSQL, VARCHAR(2000) on MySQL), idempotent across all three backends. Existing rows keep NULL and behave exactly as before.
• Schema definitions (3 backends), DbNode / NodeData / DeviceInfo types, and dbNodeMapper.
• NodesRepository.setNodeNotes(nodeNum, notes, sourceId) — Drizzle, source-scoped, works on all backends (no raw SQL).
• DatabaseService.setNodeNotesAsync — repository-backed, updates the in-memory cache.
• upsertNode preserves notes across packet-driven updates and, like mobile, intentionally omits notes from the conflict DO UPDATE SET clause so an incoming packet can never clobber a user-authored note.
• POST /api/nodes/:nodeId/notes route — requirePermission('nodes', 'write'), sourceId from body, with string / 2000-char / hex-nodeId validation.

Frontend

• Editable notes <textarea> in NodeDetailsBlock (the node detail panel in Messages), gated on nodes:write permission; read-only display when the user lacks write. The draft tracks a local baseline so the Save button settles immediately after a successful save, before the node poll catches up.
• apiService.setNodeNotes client method + styling.

Tests

• Repository per-source isolation + notes-preservation-across-upsert tests (nodes.test.ts).
• Route validation-contract tests: set / clear (empty string) / non-string / over-length / missing sourceId / invalid nodeId (server.test.ts).
• Migration registry count + last-name assertions bumped to 112.

Verification

• Full Vitest suite: 7944 passed, 0 failures (556 files).
• Server project typecheck (tsconfig.server.json): 0 errors.
• Client vite build: success.
• No new ESLint errors (pre-existing count unchanged).

🤖 Generated with Claude Code

https://claude.ai/code/session_01AphLfgUJWaFNAgU5UXdJ4n

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: feat(nodes): per-node free-text notes field (#3921)

Tasks:

• Gather context and understand the PR
• Review migration and schema changes
• Review backend (repository, service, routes)
• Review frontend components
• Review tests
• Post final feedback

---

Overall this is a well-structured implementation that follows existing project patterns closely. The notes field integrates cleanly with the multi-source architecture and multi-database support. Here's my analysis:

Migration & Schema

Migration 112 (112_add_notes_to_nodes.ts) — Clean and idempotent across all three backends.

• SQLite uses try/catch on duplicate column — consistent with other migrations.
• PostgreSQL uses IF NOT EXISTS — correct.
• MySQL queries information_schema before altering — correct.
• The down migration is a no-op (intentionally), which is consistent with project convention.

Schema (nodes.ts) — Correctly uses TEXT for SQLite/PostgreSQL and VARCHAR(2000) for MySQL. All three definitions match. The field is correctly nullable with no default, so existing rows are unaffected.

Minor observation: There's a subtle inconsistency between backends in the type system — SQLite/Postgres use unbounded TEXT while MySQL caps at VARCHAR(2000). This matches the route-level 2000-char limit and is validated server-side, so it's fine in practice, but could be surprising if someone queries the DB directly with a longer value bypassing the API.

Backend (Repository, Service, Routes)

nodes.ts (repository):

The setNodeNotes method (line 917) correctly:

• Updates only the target (nodeNum, sourceId) row — source-scoped.
• Calls syncCacheNode to keep PG/MySQL caches coherent.
• Works on all three Drizzle backends without raw SQL.

Notes preservation in upsertNode:

The PR uses two distinct code paths:

1. UPDATE existing node (line 416): notes: nodeData.notes ?? existingNode.notes — preserves existing notes if the incoming packet has none.
2. INSERT new node (line 469): notes: nodeData.notes ?? null — starts with null, which is correct since there's no pre-existing note.
3. Upsert conflict SET (lines 531–533): Notes is deliberately omitted from the DO UPDATE SET clause, with a clear comment.

This triple-layered protection is thorough and correct.

database.ts (setNodeNotesAsync):

The cache update at line 7621–7624 is correct for PG/MySQL users, but there's a subtle issue worth noting: the now timestamp computed at the top of the function is captured before await this.nodesRepo.setNodeNotes(...), then used to update cached.updatedAt afterward. If the repo write takes time, now will be slightly stale compared to the actual updatedAt written to the DB. This is unlikely to cause observable bugs since updatedAt is informational, but it's worth being aware of.

Route (server.ts line 2116):

Validation order and input checking looks solid:

• Checks typeof notes !== 'string' before length — correct.
• Validates hex nodeId format with regex before parseInt.
• Checks sourceId presence.
• Uses requirePermission('nodes', 'write', { sourceIdFrom: 'body' }) — consistent with other write operations.

Potential issue: no node existence check. If someone calls POST /api/nodes/!deadbeef/notes for a nodeNum that doesn't exist in the database (for the given sourceId), the setNodeNotes Drizzle .update() will silently succeed with 0 rows affected, and the API returns { success: true }. This is the same behavior as hideFromMap and isFavorite endpoints, so it's consistent — but it means the API doesn't tell the caller whether the node actually existed. Not a bug per se, but worth noting if you want to surface a 404 for non-existent nodes.

api.ts (setNodeNotes):

The sourceId parameter is typed as string | null | undefined and only included in the body if truthy (line 1144). However, the server-side validation (line 2141) rejects requests where sourceId is missing or empty string. If sourceId is null/undefined at the call site, the frontend will silently omit it from the body and the server will return a 400. This path is guarded by the MessagesTab.tsx call site passing sourceId from useSource(), but the type signature implies callers could omit it — consider making sourceId required or documenting when it can be null.

Frontend

NodeDetailsBlock.tsx:

• The dual-prop pattern (canEditNotes + onSaveNotes) is clean — the component doesn't know about apiService directly.
• The notesBaseline / notesDraft dual-state approach is well-reasoned: it lets the Save button disable immediately after a successful save, even before the server poll updates node.notes. Good UX.
• The useEffect (line 60–64) correctly resets draft and baseline when nodeNum or storedNotes changes, preventing stale state when switching nodes.
• The read-only fallback (line 514–518) only renders if storedNotes is truthy — no empty Notes section shown to read-only users. This is good.

MessagesTab.tsx (lines 2166–2172):

The onSaveNotes callback correctly passes selectedNode.user.id (the hex !xxxxxxxx nodeId) to apiService.setNodeNotes. The guard if (!selectedNode.user?.id) throw new Error(...) is appropriate.

NodeDetailsBlock.css: Styles follow existing project conventions with CSS variables. No issues.

Tests

nodes.test.ts:

• setNodeNotes - sets and clears: Covers basic set and empty-string clear. ✓
• setNodeNotes - is per-source isolated: Verifies cross-source isolation. ✓
• upsertNode - preserves notes across packet-driven updates: Confirms notes survive a packet-driven upsert. ✓

server.test.ts:

• Covers set, clear (empty string), non-string type, over-length, missing sourceId, invalid nodeId. ✓
• Tests use databaseMock.setNodeNotesAsync mocked as vi.fn(async ...). ✓

migrations.test.ts: Count and last-name updated correctly to 112. ✓

Gap: There's no test for calling setNodeNotes on a non-existent nodeNum (silently succeeds). Given the consistent behavior with other endpoints, this is acceptable but could be added for clarity.

Security

No concerns. Notes are stored server-side only, never returned to the mesh. Input is validated for type and length. Permission check (nodes:write) is properly applied. CSRF token is included via getHeadersWithCsrf().

The field is surfaced in API responses as part of the node object (via mapDbNodeToDeviceInfo) — the content is user-authored text and isn't rendered as HTML anywhere, only as {storedNotes} in JSX and in a <textarea> value, so XSS risk is minimal.

Summary

Verdict: Looks good to merge. The implementation is clean, thorough, and consistent with established project patterns. The two minor items worth tracking:

1. Silent no-op on non-existent node — POST /api/nodes/:nodeId/notes returns 200 even if the node doesn't exist. Consistent with other endpoints but may be unexpected.
2. sourceId type in api.ts — Optional sourceId could lead to a 400 if callers pass null/undefined, since the server requires it. The current call site is safe, but the signature is a bit loose.

Neither is a blocker.