Claude finished @Yeraze's task —— View job

PR Review: fix(config): persist & surface local node isUnmessagable/isLicensed (#3684)

• Read changed files
• Review schema changes
• Review migration
• Review repository layer
• Review ingest (meshtasticManager.ts)
• Review server.ts get-owner fix
• Review test coverage
• Review types/service layer

Summary

This is a well-structured, minimal-footprint fix that follows existing codebase patterns closely. The root cause diagnosis is correct: the local-node branch of /admin/get-owner was hardcoding the flags to false, and the ingest path never persisted them. The fix addresses both sides properly.

Observations

Schema & Migration (src/db/schema/nodes.ts, 101_add_node_unmessagable.ts) ✅

Schema changes are consistent across all three backends (SQLite INTEGER mode:'boolean', PostgreSQL BOOLEAN, MySQL BOOLEAN) with DEFAULT false/0. The migration is idempotent in all three dialects: SQLite try/catch on duplicate column, PostgreSQL IF NOT EXISTS, MySQL information_schema check. This follows the established pattern correctly.

Repository (src/db/repositories/nodes.ts) ✅

Fields are correctly added to all three paths (update, insert, upsert conflict-set). The update path uses ?? existingNode.isUnmessagable which correctly preserves the prior value when the incoming nodeData doesn't include the field. Insert/upsert defaults to false.

Minor observation: upsertNodeSqlite (the legacy SQLite-sync path at line 1720) does NOT set isUnmessagable/isLicensed on update or insert. Since the callers of this legacy sync path appear to be the backup-restore import path (importNodeSqlite) and the shim facade, this could mean imported nodes lose these flags. It's a pre-existing pattern for this sync path (e.g. hideFromMap has the same gap), so this may be acceptable, but worth noting for completeness.

Ingest (src/server/meshtasticManager.ts lines 7929-7930, 8132-8134) ✅

Both the initial NodeInfo processing path and the local-node update path now populate the flags. The PR correctly moves the localNodeInfo update outside the nameChanged guard (isLocked check) so the flag can update without requiring a name change — this is the right call since the User proto's capability flags are independent of the identity fields.

get-owner (src/server/server.ts) ✅

The local-node branch now queries the DB for the persisted flags instead of hardcoding false. The guard for an absent nodeNum (if (localNodeInfo.nodeNum)) is correct — the local node row may not exist in the DB yet at the time of the request, and the fallback to false is a safe default.

Types (src/db/types.ts, src/services/database.ts, src/types/device.ts) ✅

All three type definitions are updated consistently. No breaking changes to existing fields.

Test coverage (src/db/repositories/nodes.test.ts, src/db/migrations.test.ts) ✅

The repository test covers the three key scenarios for the new columns: default-false on insert, stored-true round-trip, and update path. The migration count test is properly bumped to 101. PG and MySQL DDL strings in the test include the new columns.

Issues to Consider

1. upsertNodeSqlite gap (minor): As noted above, the legacy SQLite sync upsert path (used by backup/import) doesn't propagate isUnmessagable/isLicensed. If a backup is exported and re-imported, these flags will be reset to false. This is consistent with how hideFromMap was handled previously, so it may be a known/accepted limitation, but worth a follow-up issue if these flags need to survive backup/restore.

2. localNodeInfo in-memory vs. DB for getCurrentConfig(): The getCurrentConfig() method returns this.localNodeInfo for the in-memory snapshot. With this PR, localNodeInfo.isUnmessagable/isLicensed is set from NodeInfo during ingest, but the get-owner endpoint reads from the DB. There are now two code paths serving the same UI element — the Config tab uses get-owner (reads DB), but if getCurrentConfig() is used elsewhere to surface these flags, it would read localNodeInfo (in-memory). The in-memory and DB values should stay in sync as long as every ingest writes to both, which appears to be the case here. Just worth being aware of.

3. No frontend change needed claim: The PR states no frontend change is needed because ConfigurationTab.tsx already reads config.localNodeInfo.isUnmessagable. That's accurate only if localNodeInfo in getCurrentConfig() includes these fields. Looking at the code, getCurrentConfig() returns this.localNodeInfo which now has these fields. The get-owner endpoint is what the Config tab actually calls for the checkbox. This is correct as-implemented.

Verdict

LGTM. The fix is correct, well-scoped, and follows codebase conventions throughout. The upsertNodeSqlite gap is a minor pre-existing pattern (not introduced by this PR) and doesn't affect the primary bug fix. Tests are adequate for the changes made.