Skip to content

fix(#3025): scope frontend neighbor-info fetch to current source + defensive lastHeard refresh#3049

Merged
Yeraze merged 3 commits into
mainfrom
fix/3025-frontend-source-scope
May 16, 2026
Merged

fix(#3025): scope frontend neighbor-info fetch to current source + defensive lastHeard refresh#3049
Yeraze merged 3 commits into
mainfrom
fix/3025-frontend-source-scope

Conversation

@Yeraze

@Yeraze Yeraze commented May 16, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes #3025 — Meshtastic NeighborInfo packets are received and stored, but the Messages-tab UI shows nothing.

Two distinct bugs combined to produce the reported symptom; this branch addresses both:

  1. Frontend was calling the wrong endpoint (root cause, surfaced by SyselMitY in the latest issue comment). App.tsx#fetchNeighborInfo still pointed at the legacy /api/neighbor-info route. In 4.x that handler returns [] for non-default sources because neighbor_info rows are partitioned by sourceId, so the UI silently rendered an empty list (HTTP 200, 0 entries). Switched to /api/sources/:sourceId/neighbor-info — matching the pattern already used by useDashboardData — and added sourceId to the effect's dep array so it refetches on source switch.

  2. Defensive lastHeard refresh on the local node (commit da1223b1, already on this branch). Between MeshMonitor start and the first self-originated broadcast that echoes through the packet pipeline, the local node row can have lastHeard=NULL. The sourceRoutes.ts:712 filter treats NULL as "stale reporter" and drops every NeighborInfo row sourced from the local node. We now refresh lastHeard whenever we receive MyNodeInfo or configComplete — both are direct evidence the local device just talked to us.

Test plan

  • npx eslint src/App.tsx — no new errors/warnings introduced
  • npx vitest run src/server/routes/sourceRoutes.neighbor-info.test.ts src/hooks/useDashboardData.test.ts — 29/29 pass
  • CI to verify full suite + system tests
  • Manual verification by reporter (@SyselMitY) once a build is available — confirm neighbor entries appear in Messages-tab Neighbor Info block for both local and remote nodes

🤖 Generated with Claude Code

Yeraze and others added 2 commits May 16, 2026 08:59
@Yeraze @claude
fix(neighbor-info): refresh local node lastHeard on MyNodeInfo + conf…
da1223b 
…igComplete

Refs #3025.

PR #3026 addressed one cause of NeighborInfo links not appearing in the UI —
the read-side filter dropped rows whose `neighbor.lastHeard` was NULL
(intentional state for indirect neighbors per #2615). The reporter has since
confirmed the bug persists in v4.5.1, including for the local node's own
NeighborInfo.

This commit closes a second gap: between MeshMonitor startup and the first
self-originated broadcast that echoes back through `processMeshPacket`, the
local node's row has `lastHeard=NULL` (or whatever value was last persisted).
When the filter at `sourceRoutes.ts:712` evaluates `!ni.node?.lastHeard ||
ni.node.lastHeard < cutoffTime`, every NeighborInfo row where the local node
is the reporter gets silently dropped.

The fix stamps `lastHeard = Date.now() / 1000` at every site where we have
direct evidence the local device is alive:

1. `processMyNodeInfo` reboot-merge upsert (was missing lastHeard entirely)
2. `processMyNodeInfo` security-flag-clear upsert (same)
3. `processMyNodeInfo` rebootCount/hasRemoteAdmin update on existing node
   (same — receiving MyNodeInfo is itself liveness evidence)
4. `configComplete` handler — added a best-effort lastHeard refresh on the
   local node, since configComplete is a separate top-level protobuf type
   that doesn't flow through `processMeshPacket`.

The bug reporter (#3025 / @SyselMitY) also describes remote-node scenarios
where both endpoints have fresh `lastHeard` but rows are still dropped. That
points at a different mechanism (sourceId scoping, units mismatch, or
something else) which cannot be pinned down without the DB inspection they
were asked to provide. This commit is intentionally scoped to the
local-node liveness gap and does not attempt to address the remote-node
case.

Tests:
- `npm run typecheck` clean
- `npx vitest run` — full suite 4959/4959 passing

No new dedicated unit test added — the existing `meshtasticManager.test.ts`
exercises `processMyNodeInfo` via a simulated helper rather than the real
method, and adding a meaningful DB-mock harness for this specific assertion
would dwarf the four-line code change. Will revisit once the reporter
confirms whether this defensive fix improves their scenario.

Authored by Merlin 🪄

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Yeraze @claude
fix(neighbor-info): scope App.tsx fetchNeighborInfo to current source
434adfe 
The Messages-tab Neighbor Info block hits `fetchNeighborInfo` in App.tsx,
which still pointed at the legacy non-source-scoped `/api/neighbor-info`.
That route is mounted under `apiRouter` but in 4.x its handler returns []
for non-default sources because rows are partitioned by `sourceId`, so the
UI silently rendered an empty list (HTTP 200, 0 entries) — exactly the
symptom reported in #3025.

Switch the call to `/api/sources/:sourceId/neighbor-info` (matching the
pattern already used by `useDashboardData`) and gate the effect on
`sourceId` so it refetches when the active source changes.

Refs #3025

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

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

Copy link
Copy Markdown

Claude finished @Yeraze's task —— View job

Code Review Completed ✅

I've thoroughly reviewed PR #3049 that fixes issue #3025 regarding Meshtastic NeighborInfo packets not displaying in the Messages tab UI.

Review Checklist:

  • Read and analyze modified files
  • Review against project guidelines (CLAUDE.md)
  • Check code quality and best practices
  • Identify potential bugs or issues
  • Assess performance considerations
  • Review security concerns
  • Evaluate test coverage

✅ Overall Assessment: Approved

This is a well-executed fix that addresses two related issues causing the neighbor info display problem. The changes are minimal, targeted, and follow project conventions.

📋 Code Quality & Best Practices

✅ Excellent:

  • Clean, focused changes addressing the root cause
  • Follows project's multi-source architecture patterns correctly
  • Proper error handling with defensive programming (try/catch, early returns)
  • Comprehensive comments explaining the defensive lastHeard refresh logic
  • Consistent with existing codebase patterns (useDashboardData already uses source-scoped endpoints)

✅ TypeScript & React:

  • Proper dependency array update in useEffect ([connectionStatus, sourceId])
  • Correct use of encodeURIComponent for URL construction
  • Early return pattern in fetchNeighborInfo when sourceId is null

🔍 Changes Analysis

Frontend Fix (src/App.tsx)

// Before: Legacy endpoint (4.x incompatible)
const response = await authFetch(`${baseUrl}/api/neighbor-info`);

// After: Source-scoped endpoint (4.x compatible)  
const url = `${baseUrl}/api/sources/${encodeURIComponent(sourceId)}/neighbor-info`;
const response = await authFetch(url);

Impact: This correctly aligns with the 4.x multi-source architecture where neighbor info is partitioned by sourceId. The legacy endpoint returns empty arrays for non-default sources.

Backend Fix (src/server/meshtasticManager.ts)

Three strategic lastHeard refreshes added:

  1. configComplete event (lines 3246-3262)
  2. Ghost node replacement (line 3436)
  3. Local node security flag clearing (line 3524)
  4. Existing node info handling (line 3553)

Rationale: Without these refreshes, a quiet local node could have lastHeard=NULL, causing the freshness filter in sourceRoutes.ts:712 to drop all neighbor info from that node.

🚀 Performance Considerations

✅ Efficient:

  • Minimal database updates (only when necessary)
  • Single additional query per configComplete/MyNodeInfo event
  • No performance impact on the hot path (packet processing)
  • Source switching now properly triggers neighbor info refetch

🔐 Security Assessment

✅ Secure:

  • Proper input sanitization with encodeURIComponent(sourceId)
  • Source isolation maintained (prevents data leakage across sources)
  • No new attack vectors introduced
  • Defensive error handling prevents crashes

🧪 Test Coverage Assessment

✅ Good Coverage:

  • Existing comprehensive test suite in sourceRoutes.neighbor-info.test.ts (29 tests)
  • Tests cover the freshness filtering logic that was causing the bug
  • Critical test case: "filters out records where the reporter node has no lastHeard" (lines 202-216)
  • Author ran targeted tests: npx vitest run src/server/routes/sourceRoutes.neighbor-info.test.ts ✅

📝 Test Gap (Minor):
No explicit test for the frontend endpoint change, but this is covered by integration behavior and the existing API tests validate the new endpoint works correctly.

🐛 Potential Issues

⚠️ Minor Considerations:

  1. Database Load: Multiple lastHeard updates per connection event, but this is acceptable for the defensive benefits gained
  2. Race Conditions: Theoretically possible between configComplete and MyNodeInfo, but the duplicate updates are harmless (idempotent)

🎯 Root Cause Analysis Validation

The fix correctly addresses both issues identified:

  1. Frontend API mismatch: ✅ Fixed by switching to /api/sources/:sourceId/neighbor-info
  2. Stale lastHeard on local node: ✅ Fixed by defensive refresh on connection events

📊 Code Metrics

  • Lines changed: +36/-4 (efficient, minimal change)
  • Files modified: 2 (focused scope)
  • Complexity added: Low (simple defensive updates)

🚀 Recommendation: APPROVE & MERGE

This PR successfully resolves issue #3025 with a clean, well-architected solution that:

  • Follows project conventions and multi-source architecture
  • Includes comprehensive error handling
  • Maintains performance characteristics
  • Provides defensive programming for edge cases
  • Has appropriate test coverage

The changes are production-ready and improve system robustness.

@github-actions

github-actions Bot commented May 16, 2026

Copy link
Copy Markdown
System Test Results

MeshMonitor System Test Results

Test Run: 2026-05-16 16:51:48 EDT

Test Summary

Test Suite Result
Configuration Import ✅ PASSED
Quick Start Test ✅ PASSED
Security Test ✅ PASSED
V1 API Test ✅ PASSED
Reverse Proxy Test ❌ FAILED
Reverse Proxy + OIDC ✅ PASSED
Virtual Node CLI Test ✅ PASSED
Backup & Restore Test ✅ PASSED
Database Migration Test ✅ PASSED
DB Backing Consistency ❌ FAILED
API Exercise (3 DBs) ❌ FAILED

❌ Overall Result: FAILED

Some tests failed. Please review the failures above and fix before creating/updating PR.

Failed Tests

  • Reverse Proxy Test: Production HTTPS deployment test failed
  • DB Backing Consistency: Database backing consistency test failed

@github-actions

github-actions Bot commented May 16, 2026

Copy link
Copy Markdown
System Test Results

MeshMonitor System Test Results

Test Run: 2026-05-16 17:36:25 EDT

Test Summary

Test Suite Result
Configuration Import ✅ PASSED
Quick Start Test ✅ PASSED
Security Test ✅ PASSED
V1 API Test ✅ PASSED
Reverse Proxy Test ✅ PASSED
Reverse Proxy + OIDC ✅ PASSED
Virtual Node CLI Test ✅ PASSED
Backup & Restore Test ✅ PASSED
Database Migration Test ✅ PASSED
DB Backing Consistency ❌ FAILED
API Exercise (3 DBs) ❌ FAILED

❌ Overall Result: FAILED

Some tests failed. Please review the failures above and fix before creating/updating PR.

Failed Tests

  • DB Backing Consistency: Database backing consistency test failed

@Yeraze
test(system): self-heal orphan test containers in pre-flight
4dafad2 
If test-api-exercise-all-backends.sh is killed before its EXIT trap can
run `docker compose down -v` (e.g., CI step timeout, SIGKILL, daemon
restart), its named test containers — `meshmonitor-api-exercise-*-test`
— linger on the self-hosted hardware runner. On the next CI cycle the
"Database Backing Consistency" pre-check trips on the orphan and bails
out before any test code runs, and the api-exercise step itself can't
bind its port. This poisons every subsequent system-test run until
someone SSHs in and removes the container by hand (e.g., PR #3049 hit
this exact state).

Both scripts now remove their own — and known sibling — test-scoped
container names at script start, before any pre-check or compose-up.
The names hard-coded here belong exclusively to test scripts, so the
removal can never touch a developer's running dev stack.
@github-actions

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

Copy link
Copy Markdown

Claude finished @Yeraze's task —— View job

Code Review Completed ✅

I've thoroughly reviewed PR #3049 that fixes issue #3025 regarding Meshtastic NeighborInfo packets not displaying in the Messages tab UI.

Review Checklist:

  • Read and analyze modified files
  • Review against project guidelines (CLAUDE.md)
  • Check code quality and best practices
  • Identify potential bugs or issues
  • Assess performance considerations
  • Review security concerns
  • Evaluate test coverage

✅ Overall Assessment: Approved

This is a well-executed fix that addresses two related issues causing the neighbor info display problem. The changes are minimal, targeted, and follow project conventions.

📋 Code Quality & Best Practices

✅ Excellent:

  • Clean, focused changes addressing the root cause
  • Follows project's multi-source architecture patterns correctly
  • Proper error handling with defensive programming (try/catch, early returns)
  • Comprehensive comments explaining the defensive lastHeard refresh logic
  • Consistent with existing codebase patterns (useDashboardData already uses source-scoped endpoints)

✅ TypeScript & React:

  • Proper dependency array update in useEffect ([connectionStatus, sourceId])
  • Correct use of encodeURIComponent for URL construction
  • Early return pattern in fetchNeighborInfo when sourceId is null

🔍 Changes Analysis

Frontend Fix (src/App.tsx)

// Before: Legacy endpoint (4.x incompatible)
const response = await authFetch(`${baseUrl}/api/neighbor-info`);

// After: Source-scoped endpoint (4.x compatible)  
const url = `${baseUrl}/api/sources/${encodeURIComponent(sourceId)}/neighbor-info`;
const response = await authFetch(url);

Impact: This correctly aligns with the 4.x multi-source architecture where neighbor info is partitioned by sourceId. The legacy endpoint returns empty arrays for non-default sources.

Backend Fix (src/server/meshtasticManager.ts)

Three strategic lastHeard refreshes added:

  1. configComplete event (lines 3246-3262)
  2. Ghost node replacement (line 3436)
  3. Local node security flag clearing (line 3524)
  4. Existing node info handling (line 3553)

Rationale: Without these refreshes, a quiet local node could have lastHeard=NULL, causing the freshness filter in sourceRoutes.ts:712 to drop all neighbor info from that node.

🚀 Performance Considerations

✅ Efficient:

  • Minimal database updates (only when necessary)
  • Single additional query per configComplete/MyNodeInfo event
  • No performance impact on the hot path (packet processing)
  • Source switching now properly triggers neighbor info refetch

🔐 Security Assessment

✅ Secure:

  • Proper input sanitization with encodeURIComponent(sourceId)
  • Source isolation maintained (prevents data leakage across sources)
  • No new attack vectors introduced
  • Defensive error handling prevents crashes

🧪 Test Coverage Assessment

✅ Good Coverage:

  • Existing comprehensive test suite in sourceRoutes.neighbor-info.test.ts (29 tests)
  • Tests cover the freshness filtering logic that was causing the bug
  • Critical test case: "filters out records where the reporter node has no lastHeard" (lines 202-216)
  • Author ran targeted tests: npx vitest run src/server/routes/sourceRoutes.neighbor-info.test.ts ✅

📝 Test Gap (Minor):
No explicit test for the frontend endpoint change, but this is covered by integration behavior and the existing API tests validate the new endpoint works correctly.

🐛 Potential Issues

⚠️ Minor Considerations:

  1. Database Load: Multiple lastHeard updates per connection event, but this is acceptable for the defensive benefits gained
  2. Race Conditions: Theoretically possible between configComplete and MyNodeInfo, but the duplicate updates are harmless (idempotent)

🎯 Root Cause Analysis Validation

The fix correctly addresses both issues identified:

  1. Frontend API mismatch: ✅ Fixed by switching to /api/sources/:sourceId/neighbor-info
  2. Stale lastHeard on local node: ✅ Fixed by defensive refresh on connection events

📊 Code Metrics

  • Lines changed: +36/-4 (efficient, minimal change)
  • Files modified: 2 (focused scope)
  • Complexity added: Low (simple defensive updates)

🚀 Recommendation: APPROVE & MERGE

This PR successfully resolves issue #3025 with a clean, well-architected solution that:

  • Follows project conventions and multi-source architecture
  • Includes comprehensive error handling
  • Maintains performance characteristics
  • Provides defensive programming for edge cases
  • Has appropriate test coverage

The changes are production-ready and improve system robustness.

@github-actions

github-actions Bot commented May 16, 2026

Copy link
Copy Markdown
System Test Results

MeshMonitor System Test Results

Test Run: 2026-05-16 18:15:29 EDT

Test Summary

Test Suite Result
Configuration Import ✅ PASSED
Quick Start Test ✅ PASSED
Security Test ✅ PASSED
V1 API Test ✅ PASSED
Reverse Proxy Test ✅ PASSED
Reverse Proxy + OIDC ✅ PASSED
Virtual Node CLI Test ✅ PASSED
Backup & Restore Test ✅ PASSED
Database Migration Test ✅ PASSED
DB Backing Consistency ✅ PASSED
API Exercise (3 DBs) ✅ PASSED

✅ Overall Result: PASSED

All deployment configurations are working correctly!

Test Details

Configuration Import:

  • Tests configuration import and device reboot cycle
  • Verifies channel roles, PSKs, and LoRa configuration
  • Note: Channel name verification skipped due to architectural limitation

Quick Start Test:

  • Zero-config deployment (no SESSION_SECRET or COOKIE_SECURE required)
  • HTTP access without HSTS
  • Auto-generated admin user with default credentials
  • Session cookies work over HTTP
  • Meshtastic node connection and message exchange verified

Security Test:

  • Verifies Node IP address hidden from anonymous users in API responses
  • Verifies MQTT configuration hidden from anonymous users
  • Verifies Node IP address visible to authenticated users
  • Verifies MQTT configuration visible to authenticated users
  • Verifies protected endpoints require authentication

V1 API Test:

  • Tests v1 REST API endpoints with Bearer token authentication
  • Verifies Bearer token requests bypass CSRF protection
  • Verifies POST/PUT/DELETE work without CSRF token when using Bearer auth
  • Verifies session-based requests still require CSRF token

Reverse Proxy Test:

  • Production deployment with COOKIE_SECURE=true
  • HTTPS-ready configuration
  • Trust proxy enabled for reverse proxy compatibility
  • CORS configured for HTTPS domain
  • Meshtastic node connection and message exchange verified

Reverse Proxy + OIDC Test:

  • OIDC authentication integration
  • Mock OIDC provider health checks
  • Authorization flow and session creation
  • Hybrid mode (OIDC + local auth)
  • Meshtastic node connection verified

Virtual Node CLI Test:

  • Virtual Node Server enabled on TCP port 4404
  • Meshtastic Python client successfully connects
  • Node data download and synchronization verified
  • Test message sent on gauntlet channel (index 3)
  • Message delivery confirmed via Web UI API
  • Virtual Node Server connection logging verified

Backup & Restore Test:

  • System backup created from running dev container
  • New container spun up with RESTORE_FROM_BACKUP env var
  • Data integrity verified (node count, message count, settings)
  • Restore event logged in audit log
  • Dev container unaffected by restore test

Database Migration Test:

  • SQLite to PostgreSQL migration verified
  • SQLite to MySQL migration verified
  • Data integrity confirmed for both target databases
  • Row counts match between source and target

DB Backing Consistency Test:

  • SQLite, PostgreSQL, and MySQL backends tested with same device
  • Node counts within ±10 across all three backends
  • Favorite counts identical across all backends
  • Key station verified as favorite on all backends

@Yeraze Yeraze merged commit 65ad2c5 into main May 16, 2026
20 checks passed
@Yeraze Yeraze mentioned this pull request May 17, 2026
Merged
5 tasks
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.

[BUG] Meshtastic NeigborInfo: Response received but not shown in UI

1 participant

@Yeraze