Skip to content

refactor(channel-db): drop dead sourceId column and move UI to Global Settings#3091

Merged
Yeraze merged 4 commits into
mainfrom
worktree-channel-db-investigation
May 19, 2026
Merged

refactor(channel-db): drop dead sourceId column and move UI to Global Settings#3091
Yeraze merged 4 commits into
mainfrom
worktree-channel-db-investigation

Conversation

@Yeraze

@Yeraze Yeraze commented May 19, 2026

Copy link
Copy Markdown
Owner

Summary

The channel_database.sourceId column added by migration 021 was never wired up:

  • ChannelDatabaseRepository reads (getAllAsync, getEnabledAsync, getByIdAsync) never filtered by sourceId.
  • HTTP routes (channelDatabaseRoutes.ts, v1/channelDatabase.ts) never passed sourceId to createAsync, so every API-created row landed with sourceId = NULL.
  • channelDecryptionService is a singleton with one flat cache populated from getEnabledAsync() — the union of every enabled row.
  • After feat(mqtt): full source-scoped ingest with channel decryption and unified-view fixes #3089 merged, MQTT ingestion decrypts against that same global cache, making the per-source illusion even more misleading.

Permissions (channel_database_permissions) are already keyed only on (userId × channelDatabaseId) with no source dimension, so dropping the column is consistent with the existing permission model.

What's in here

Migration 063 (063_drop_source_id_from_channel_database.ts)
Drops the column across SQLite (ALTER TABLE DROP COLUMN), PostgreSQL (DROP COLUMN IF EXISTS), and MySQL (information_schema guard + drop). Idempotent.

Schema / repository / types

  • src/db/schema/channelDatabase.ts: drop sourceId from all three dialect tables.
  • src/db/repositories/channelDatabase.ts: drop sourceId arg on createAsync and the field on mapToDbChannelDatabase.
  • src/db/types.ts: drop sourceId from DbChannelDatabase.
  • src/db/repositories/channelDatabase.test.ts: drop sourceId from the SQLite/PG/MySQL test schemas.

Unified routes

  • src/server/routes/unifiedRoutes.ts: drop the vc.sourceId === source.id filters in both /channels and /messages. Virtual channels are global — every readable channel-db entry is surfaced under every source the user can read. Comments updated. The matching test was rewritten to assert the new global semantics.

MQTT bootstrap (post-rebase reconcile with #3089)

  • src/server/mqttIngestion.ts: bootstrapMqttChannelDatabase no longer passes sourceId to createAsync. The auto-seeded LongFast row is unowned, matching the global model.

Frontend — UI relocation

  • src/components/ConfigurationTab.tsx: drop the ChannelDatabaseSection import, the config-channel-database menu entry, and its render block. The now-unused useAuth import was removed too.
  • src/components/SettingsTab.tsx: import ChannelDatabaseSection, add 'settings-channel-database' to GLOBAL_SECTIONS, register an admin-gated menu entry between System Backup and Database Maintenance, and mount the section. The device-specific rebroadcastMode prop is dropped; the component already handles undefined by hiding the rebroadcast warning block — appropriate for a global context.

Registry / tests

  • src/db/migrations.ts: register migration 063 with the standard idempotent pattern.
  • src/db/migrations.test.ts: bump count 62 → 63 and update the "last migration" assertion.

Migration numbering

Latest migration on main post-#3089 is 62. PR #3089 did not add any migrations, so 063 is the next free number.

Test plan

  • npx tsc --noEmit — 0 errors
  • npx vitest run — 5090/5090 passing (5066 baseline + 24 added by feat(mqtt): full source-scoped ingest with channel decryption and unified-view fixes #3089)
  • ESLint — 0 errors on touched files (only inherited no-explicit-any warnings)
  • Verify in dev container that the moved UI appears in Global Settings → Channel Database (admin only) and is gone from Device Configuration
  • Verify that the unified /channels picker shows virtual channels under every source
  • Confirm migration 063 runs cleanly on a fresh SQLite DB and on an upgrade from a populated DB

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Claude finished @Yeraze's task —— View job

Code Review Complete ✅

Reviewing PR #3091 - refactor to drop unused sourceId column from channel_database and move UI to Global Settings.

Review Checklist:

  • Analyze migration 063 implementation across all database backends
  • Review schema and repository changes for consistency
  • Check UI relocation from Device Configuration to Global Settings
  • Verify test coverage and updates
  • Look for potential bugs or security issues
  • Assess performance implications
  • Provide comprehensive feedback

Review Summary

This is a well-executed refactor that properly removes dead code and aligns UI with runtime behavior. The implementation is thorough and follows the codebase's multi-database patterns correctly.

✅ Strengths

Migration Quality

  • Migration 063 properly implements idempotent column drops for all three database backends
  • Excellent error handling: SQLite catches "no such column", PostgreSQL uses IF EXISTS, MySQL uses information_schema checks
  • Proper logging and follows established migration patterns

Code Consistency

  • Repository methods were already global (no source filtering) - this change makes the schema match reality
  • DbChannelDatabase type properly removes sourceId field
  • All three dialect schemas consistently remove the column
  • Repository createAsync correctly removes unused sourceId parameter

UI Relocation Logic

  • Moving from per-device Configuration to Global Settings makes sense given global runtime behavior
  • Admin-only gating is appropriate for global channel database management
  • Removal of rebroadcastMode prop is correct (device-specific prop not needed in global context)

Test Coverage

  • Migration tests updated correctly (count 62→63, name assertion)
  • Unified routes test properly updated to reflect global virtual channel behavior
  • Channel database repository tests cleaned up to remove unused sourceId references

🔍 Key Architectural Insights

Global Channel Database is Correct Design

Proper Implementation Details

  • MQTT bootstrap no longer passes sourceId to createAsync (lines 46-54 in mqttIngestion.ts)
  • Unified routes correctly surface virtual channels under all sources without source filtering
  • Tests properly validate that virtual channels appear globally across sources

🟡 Minor Observations

Documentation Gaps

  • Migration comment excellently explains the rationale (lines 3-11 in migration 063)
  • Code comments in unifiedRoutes.ts clearly explain virtual channel global behavior

Security & Performance

  • No security concerns - permissions model remains intact
  • Performance impact minimal - removes unused column and parameter
  • Migration is safe and idempotent across all backends

✅ Conclusion

This refactor correctly removes technical debt by eliminating the unused sourceId column that was never properly implemented. The UI move to Global Settings accurately reflects the runtime behavior where channel database entries are global resources used for decryption across all sources.

Recommendation: Approve - This is a clean, well-tested refactor that improves code clarity and removes dead weight.

--- | Branch: worktree-channel-db-investigation

Yeraze and others added 2 commits May 19, 2026 16:31
@Yeraze @claude
refactor(channel-db): drop dead sourceId column and move UI to Global…
0a28b8e 
… Settings

The channel_database.sourceId column added by migration 021 was never wired
up. Repository reads ignored it, HTTP routes never passed it on create, and
channelDecryptionService pooled every enabled row into a single global cache
consumed by every source. After PR 3089, MQTT also decrypts against that
same global cache, making the per-source illusion even more misleading.

Migration 063 drops the column across SQLite/PostgreSQL/MySQL (idempotent).
Schema, types, repository, and tests are stripped of the field. Unified
channels/messages routes drop the `vc.sourceId === source.id` filter so
virtual channels are surfaced under every source (their actual semantics).

The ChannelDatabaseSection UI moves from the per-source Meshtastic Device
Configuration menu to the Global Settings menu, matching runtime behaviour.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Yeraze @claude
refactor(channel-db): drop sourceId arg from MQTT bootstrap createAsy…
ddefe90 
…nc call

PR 3089's bootstrapMqttChannelDatabase passed sourceId as a second argument
to channel_database.createAsync. With the channel database now global, the
parameter is gone and the auto-seeded LongFast row is owned by no source.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Yeraze Yeraze force-pushed the worktree-channel-db-investigation branch from 23fea93 to ddefe90 Compare May 19, 2026 20:31
@github-actions

github-actions Bot commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Claude finished @Yeraze's task —— View job

Code Review Complete ✅

Reviewing PR #3091 - refactor to drop unused sourceId column from channel_database and move UI to Global Settings.

Review Checklist:

  • Analyze migration 063 implementation across all database backends
  • Review schema and repository changes for consistency
  • Check UI relocation from Device Configuration to Global Settings
  • Verify test coverage and updates
  • Look for potential bugs or security issues
  • Assess performance implications
  • Provide comprehensive feedback

Review Summary

This is a well-executed refactor that properly removes dead code and aligns UI with runtime behavior. The implementation is thorough and follows the codebase's multi-database patterns correctly.

✅ Strengths

Migration Quality

  • Migration 063 properly implements idempotent column drops for all three database backends
  • Excellent error handling: SQLite catches "no such column", PostgreSQL uses IF EXISTS, MySQL uses information_schema checks
  • Proper logging and follows established migration patterns

Code Consistency

  • Repository methods were already global (no source filtering) - this change makes the schema match reality
  • DbChannelDatabase type properly removes sourceId field
  • All three dialect schemas consistently remove the column
  • Repository createAsync correctly removes unused sourceId parameter

UI Relocation Logic

  • Moving from per-device Configuration to Global Settings makes sense given global runtime behavior
  • Admin-only gating is appropriate for global channel database management
  • Removal of rebroadcastMode prop is correct (device-specific prop not needed in global context)

Test Coverage

  • Migration tests updated correctly (count 62→63, name assertion)
  • Unified routes test properly updated to reflect global virtual channel behavior
  • Channel database repository tests cleaned up to remove unused sourceId references

🔍 Key Architectural Insights

Global Channel Database is Correct Design

Proper Implementation Details

  • MQTT bootstrap no longer passes sourceId to createAsync (lines 46-54 in mqttIngestion.ts)
  • Unified routes correctly surface virtual channels under all sources without source filtering
  • Tests properly validate that virtual channels appear globally across sources

🟡 Minor Observations

Documentation Gaps

  • Migration comment excellently explains the rationale (lines 3-11 in migration 063)
  • Code comments in unifiedRoutes.ts clearly explain virtual channel global behavior

Security & Performance

  • No security concerns - permissions model remains intact
  • Performance impact minimal - removes unused column and parameter
  • Migration is safe and idempotent across all backends

✅ Conclusion

This refactor correctly removes technical debt by eliminating the unused sourceId column that was never properly implemented. The UI move to Global Settings accurately reflects the runtime behavior where channel database entries are global resources used for decryption across all sources.

Recommendation: Approve - This is a clean, well-tested refactor that improves code clarity and removes dead weight.

--- | Branch: worktree-channel-db-investigation

@Yeraze @claude
fix(channel-db): drop channel_database from assignNullSourceIds start…
030c63e 
…up pass

server.ts boots a routine that backfills NULL sourceId rows on every data
table. Migration 063 removed sourceId from channel_database, so the runtime
UPDATE now produces a noisy 'no such column' warning on every boot. The
caught-error branch swallows the failure but it's cruft — channel_database
is global and doesn't belong in the per-source backfill list.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Claude finished @Yeraze's task —— View job

Code Review Complete ✅

Reviewing PR #3091 - refactor to drop unused sourceId column from channel_database and move UI to Global Settings.

Review Checklist:

  • Analyze migration 063 implementation across all database backends
  • Review schema and repository changes for consistency
  • Check UI relocation from Device Configuration to Global Settings
  • Verify test coverage and updates
  • Look for potential bugs or security issues
  • Assess performance implications
  • Provide comprehensive feedback

Review Summary

This is a well-executed refactor that properly removes dead code and aligns UI with runtime behavior. The implementation is thorough and follows the codebase's multi-database patterns correctly.

✅ Strengths

Migration Quality

  • Migration 063 properly implements idempotent column drops for all three database backends
  • Excellent error handling: SQLite catches "no such column", PostgreSQL uses IF EXISTS, MySQL uses information_schema checks
  • Proper logging and follows established migration patterns

Code Consistency

  • Repository methods were already global (no source filtering) - this change makes the schema match reality
  • DbChannelDatabase type properly removes sourceId field
  • All three dialect schemas consistently remove the column
  • Repository createAsync correctly removes unused sourceId parameter

UI Relocation Logic

  • Moving from per-device Configuration to Global Settings makes sense given global runtime behavior
  • Admin-only gating is appropriate for global channel database management
  • Removal of rebroadcastMode prop is correct (device-specific prop not needed in global context)

Test Coverage

  • Migration tests updated correctly (count 62→63, name assertion)
  • Unified routes test properly updated to reflect global virtual channel behavior
  • Channel database repository tests cleaned up to remove unused sourceId references

🔍 Key Architectural Insights

Global Channel Database is Correct Design

Proper Implementation Details

  • MQTT bootstrap no longer passes sourceId to createAsync (lines 46-54 in mqttIngestion.ts)
  • Unified routes correctly surface virtual channels under all sources without source filtering
  • Tests properly validate that virtual channels appear globally across sources

🟡 Minor Observations

Documentation Gaps

  • Migration comment excellently explains the rationale (lines 3-11 in migration 063)
  • Code comments in unifiedRoutes.ts clearly explain virtual channel global behavior

Security & Performance

  • No security concerns - permissions model remains intact
  • Performance impact minimal - removes unused column and parameter
  • Migration is safe and idempotent across all backends

✅ Conclusion

This refactor correctly removes technical debt by eliminating the unused sourceId column that was never properly implemented. The UI move to Global Settings accurately reflects the runtime behavior where channel database entries are global resources used for decryption across all sources.

Recommendation: Approve - This is a clean, well-tested refactor that improves code clarity and removes dead weight.

--- | Branch: worktree-channel-db-investigation

@Yeraze @claude
fix(channel-db): preserve shorthand PSK input instead of expanding on…
071509b 
… save

The editor used to detect a 1-byte PSK (e.g. AQ== → 0x01 → "default key")
and replace it with the expanded 16-byte LongFast key before sending the
update. To the user this looked like the system was silently rewriting
their key. The MQTT bootstrap already stores AQ== as-is, and
channelDecryptionService.refreshChannelCache calls expandShorthandPsk at
cache build time, so the round-trip works end-to-end without UI expansion.

Validation kept: 0 / 16 / 32 byte lengths accepted, byte 0 still rejected
as "no crypto". The local expandShorthandPsk / uint8ArrayToBase64 helpers
and the MESHTASTIC_DEFAULT_KEY constant are dead now and removed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Claude finished @Yeraze's task —— View job

Code Review Complete ✅

Reviewing PR #3091 - refactor to drop unused sourceId column from channel_database and move UI to Global Settings.

Review Checklist:

  • Analyze migration 063 implementation across all database backends
  • Review schema and repository changes for consistency
  • Check UI relocation from Device Configuration to Global Settings
  • Verify test coverage and updates
  • Look for potential bugs or security issues
  • Assess performance implications
  • Provide comprehensive feedback

Review Summary

This is a well-executed refactor that properly removes dead code and aligns UI with runtime behavior. The implementation is thorough and follows the codebase's multi-database patterns correctly.

✅ Strengths

Migration Quality

  • Migration 063 properly implements idempotent column drops for all three database backends
  • Excellent error handling: SQLite catches "no such column", PostgreSQL uses IF EXISTS, MySQL uses information_schema checks
  • Proper logging and follows established migration patterns

Code Consistency

  • Repository methods were already global (no source filtering) - this change makes the schema match reality
  • DbChannelDatabase type properly removes sourceId field
  • All three dialect schemas consistently remove the column
  • Repository createAsync correctly removes unused sourceId parameter

UI Relocation Logic

  • Moving from per-device Configuration to Global Settings makes sense given global runtime behavior
  • Admin-only gating is appropriate for global channel database management
  • Removal of rebroadcastMode prop is correct (device-specific prop not needed in global context)

Test Coverage

  • Migration tests updated correctly (count 62→63, name assertion)
  • Unified routes test properly updated to reflect global virtual channel behavior
  • Channel database repository tests cleaned up to remove unused sourceId references

🔍 Key Architectural Insights

Global Channel Database is Correct Design

Proper Implementation Details

  • MQTT bootstrap no longer passes sourceId to createAsync (lines 46-54 in mqttIngestion.ts)
  • Unified routes correctly surface virtual channels under all sources without source filtering
  • Tests properly validate that virtual channels appear globally across sources

🟡 Minor Observations

Documentation Gaps

  • Migration comment excellently explains the rationale (lines 3-11 in migration 063)
  • Code comments in unifiedRoutes.ts clearly explain virtual channel global behavior

Security & Performance

  • No security concerns - permissions model remains intact
  • Performance impact minimal - removes unused column and parameter
  • Migration is safe and idempotent across all backends

✅ Conclusion

This refactor correctly removes technical debt by eliminating the unused sourceId column that was never properly implemented. The UI move to Global Settings accurately reflects the runtime behavior where channel database entries are global resources used for decryption across all sources.

Recommendation: Approve - This is a clean, well-tested refactor that improves code clarity and removes dead weight.

--- | Branch: worktree-channel-db-investigation

@Yeraze Yeraze merged commit cfb6dc8 into main May 19, 2026
17 checks passed
@Yeraze Yeraze deleted the worktree-channel-db-investigation branch May 19, 2026 21:12
@Yeraze Yeraze mentioned this pull request May 19, 2026
Merged
6 tasks
Yeraze added a commit that referenced this pull request May 19, 2026
@Yeraze @claude
chore(release): bump version to 4.6.2 (#3095)
3e6818f 
Patch release covering the MQTT-ingest rework (#3089) and follow-on UX
fixes (#3093 channel display-name, #3091 Channel Database move to Global)
plus the usual dependency bumps and minor fixes.

Version bumps across all five canonical files: package.json,
package-lock.json (regenerated), helm/meshmonitor/Chart.yaml,
desktop/package.json, desktop/src-tauri/tauri.conf.json.

CHANGELOG: organized under [4.6.2] with the user-visible MQTT/channel
items called out as the headline changes.

CLAUDE.md:
- Migration count 62+ → 63+ (latest: 063_drop_source_id_from_channel_database)
- "Every row carries a sourceId" rule gains an exception note for
  channel_database — it's intentionally global (decryption service tries
  every enabled row regardless of source) and migration 063 dropped the
  dead sourceId column.

Blog post + news.json: "MeshMonitor v4.6.2 — MQTT in the Unified views & cleaner channel names" — three things users will notice immediately
(slot-0 channel renamed from "Primary" to the modem-preset label,
Channel Database moved to Global Settings, MQTT sources now participate
in Unified Messages and Unified Telemetry) plus action items after
upgrade.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Yeraze