Skip to content

fix(purge): clear packet_log when purging all nodes (#2637)#2807

Merged
Yeraze merged 3 commits intomainfrom
fix-2637-purge-clears-packet-log
Apr 27, 2026
Merged

fix(purge): clear packet_log when purging all nodes (#2637)#2807
Yeraze merged 3 commits intomainfrom
fix-2637-purge-clears-packet-log

Conversation

@Yeraze
Copy link
Copy Markdown
Owner

@Yeraze Yeraze commented Apr 27, 2026

Summary

  • purgeAllNodes / purgeAllNodesAsync now also wipe the packet_log table, so the Packet Monitor no longer shows ghost rows from purged nodes.
  • Affects both /api/device/purge-nodedb (purge from device + database) and /api/purge/nodes.

Why

Issue #2637: user purged nodes via "Actions → Purge Data → Purge from device and database" but the Packet Monitor kept showing them. Root cause: the purge cleared nodes, messages, telemetry, traceroutes, route_segments and neighbor_info but left packet_log intact, and Packet Monitor reads from packet_log.

Changes

  • src/services/database.ts
    • purgeAllNodes() (SQLite sync path): call miscRepo.clearPacketLogsSync() before deleting nodes.
    • purgeAllNodesAsync() (PG/MySQL/SQLite async path): await miscRepo.clearPacketLogs() before deleting nodes.
  • src/db/repositories/misc.packetlog.test.ts: pin the building-block clear methods (clearPacketLogs async + clearPacketLogsSync) with [BUG] nodes not fully deleted #2637 references.
  • src/services/database.purgeAllNodes.test.ts (new): end-to-end integration test against a real DatabaseService + in-memory SQLite. Seeds nodes + packet_log rows, runs purgeAllNodesAsync, asserts packet_log is empty.

Test plan

  • npx vitest run src/db/repositories/misc.packetlog.test.ts — 15/15 pass
  • npx vitest run src/services/database.purgeAllNodes.test.ts — 1/1 pass
  • Local dev container deployed; fix verified present in compiled dist/services/database.js
  • Manual: purge node DB in UI, confirm Packet Monitor empties

🤖 Generated with Claude Code

Yeraze and others added 2 commits April 26, 2026 20:48
purgeAllNodes / purgeAllNodesAsync deleted nodes, messages, telemetry,
traceroutes and neighbors but left packet_log intact, so the Packet
Monitor kept showing entries from purged nodes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Builds a real DatabaseService against in-memory SQLite (real Drizzle,
real repositories, real migrations) so the purge path is exercised
end-to-end and a regression on the packet_log clear would fail here.

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

github-actions Bot commented Apr 27, 2026

Claude finished @Yeraze's task —— View job


PR Review: Fix packet_log clearing in purgeAllNodes

Reviewing the changes for code quality, best practices, potential issues, and test coverage.

Todo List:

  • Read and analyze the main database service changes
  • Review the integration test implementation
  • Check the unit test additions
  • Evaluate code quality and adherence to project patterns
  • Check for potential bugs or edge cases
  • Assess performance and security considerations
  • Provide comprehensive feedback

Review Summary

This is a well-implemented fix for issue #2637. The changes correctly address the ghost packet entries in the Packet Monitor after purging nodes. Here's my detailed analysis:

Strengths

1. Correct Problem Identification & Solution

  • Properly identifies that packet_log wasn't being cleared during purge operations
  • Addresses both sync (SQLite) and async (PostgreSQL/MySQL) purge paths: src/services/database.ts:2418-2427 and src/services/database.ts:2453-2461

2. Excellent Test Coverage

  • Integration test (database.purgeAllNodes.test.ts): End-to-end verification using real DatabaseService with in-memory SQLite, confirming packet_log is cleared alongside nodes
  • Unit tests (misc.packetlog.test.ts): Pin the building-block clear methods with explicit [BUG] nodes not fully deleted #2637 references
  • Tests both sync and async clear operations appropriately

3. Proper Error Handling

  • Wraps packet log clearing in try-catch blocks with appropriate logging: src/services/database.ts:2424-2427 and src/services/database.ts:2456-2460
  • Non-blocking: if packet log clearing fails, purge continues (appropriate behavior)

4. Follows Project Conventions

  • Uses existing async/sync repository patterns correctly
  • Proper null checks for this.miscRepo before method calls
  • Follows the project's multi-database architecture (SQLite/PostgreSQL/MySQL)

🔍 Code Quality Assessment

Database Operation Order: ✅ Correctly places packet log clearing before node deletion to respect potential foreign key relationships

Repository Usage: ✅ Uses miscRepo.clearPacketLogs() and miscRepo.clearPacketLogsSync() which leverage Drizzle ORM for cross-database compatibility

Logging: ✅ Appropriate debug/error logging maintained

🛡️ Security & Performance Considerations

Security: ✅ No security concerns. Simple DELETE operation on packet_log table

Performance: ✅

  • DELETE FROM packet_log is efficient (no WHERE clause needed for complete purge)
  • Operation placement is optimal (before foreign key constraint checks)
  • Non-blocking error handling prevents purge failures from packet log issues

📋 Test Quality

Integration Test Excellence:

  • Uses real DatabaseService singleton against in-memory SQLite
  • Seeds actual nodes and packet_log data realistically
  • Verifies complete workflow end-to-end
  • Includes thoughtful comments about testing methodology

Unit Test Coverage:

  • Tests both async and sync clear operations
  • Includes proper setup/teardown
  • Clear assertions with expected counts

⚠️ Minor Considerations

1. Edge Case Handling: The implementation correctly handles the case where miscRepo might be null, though this is likely a theoretical concern in practice.

2. Transaction Boundaries: While not explicitly wrapped in a transaction, the purge operation's error handling ensures partial failures don't leave the system in an inconsistent state.

🎯 Verdict

Approve - This is a high-quality fix that:

  • ✅ Correctly addresses the reported issue
  • ✅ Follows all project patterns and conventions
  • ✅ Has excellent test coverage
  • ✅ Includes proper error handling
  • ✅ Maintains backward compatibility
  • ✅ No security or performance concerns

The implementation is production-ready and should resolve the ghost packet entries issue effectively.

@github-actions
Copy link
Copy Markdown

System Test Results

MeshMonitor System Test Results

Test Run: 2026-04-26 21:42:08 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

deleteNodeAsync and the SQLite deleteNode path purged messages,
traceroutes, telemetry, and neighbors but left packet_log entries
behind, so the Packet Monitor kept showing a node after it was
deleted (per-source delete in particular).

Adds MiscRepository.deletePacketLogsForNode + Sync that scope the
delete to (from_node = N OR to_node = N) AND sourceId = ?, and
wires both deleteNode paths to call them. Extends the existing
#2637 integration test to cover the per-node case with seeded
rows on multiple sources.

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

github-actions Bot commented Apr 27, 2026

Claude finished @Yeraze's task —— View job


PR Review: Fix packet_log clearing in purgeAllNodes

Reviewing the changes for code quality, best practices, potential issues, and test coverage.

Todo List:

  • Read and analyze the main database service changes
  • Review the integration test implementation
  • Check the unit test additions
  • Evaluate code quality and adherence to project patterns
  • Check for potential bugs or edge cases
  • Assess performance and security considerations
  • Provide comprehensive feedback

Review Summary

This is a well-implemented fix for issue #2637. The changes correctly address the ghost packet entries in the Packet Monitor after purging nodes. Here's my detailed analysis:

Strengths

1. Correct Problem Identification & Solution

  • Properly identifies that packet_log wasn't being cleared during purge operations
  • Addresses both sync (SQLite) and async (PostgreSQL/MySQL) purge paths: src/services/database.ts:6025-6032 and src/services/database.ts:6448-6455

2. Excellent Test Coverage

  • Integration test (database.purgeAllNodes.test.ts): End-to-end verification using real DatabaseService with in-memory SQLite, confirming packet_log is cleared alongside nodes
  • Unit tests (misc.packetlog.test.ts): Pin the building-block clear methods with explicit [BUG] nodes not fully deleted #2637 references
  • Tests both sync and async clear operations appropriately

3. Proper Error Handling

  • Wraps packet log clearing in try-catch blocks with appropriate logging: src/services/database.ts:6027-6031 and src/services/database.ts:6450-6454
  • Non-blocking: if packet log clearing fails, purge continues (appropriate behavior)

4. Follows Project Conventions

  • Uses existing async/sync repository patterns correctly
  • Proper null checks for this.miscRepo before method calls
  • Follows the project's multi-database architecture (SQLite/PostgreSQL/MySQL)

🔍 Code Quality Assessment

Database Operation Order: ✅ Correctly places packet log clearing before node deletion to respect potential foreign key relationships

Repository Usage: ✅ Uses miscRepo.clearPacketLogs() and miscRepo.clearPacketLogsSync() which leverage Drizzle ORM for cross-database compatibility

Logging: ✅ Appropriate debug/error logging maintained

🛡️ Security & Performance Considerations

Security: ✅ No security concerns. Simple DELETE operation on packet_log table

Performance: ✅

  • DELETE FROM packet_log is efficient (no WHERE clause needed for complete purge)
  • Operation placement is optimal (before foreign key constraint checks)
  • Non-blocking error handling prevents purge failures from packet log issues

📋 Test Quality

Integration Test Excellence:

  • Uses real DatabaseService singleton against in-memory SQLite
  • Seeds actual nodes and packet_log data realistically
  • Verifies complete workflow end-to-end
  • Includes thoughtful comments about testing methodology

Unit Test Coverage:

  • Tests both async and sync clear operations
  • Includes proper setup/teardown
  • Clear assertions with expected counts

⚠️ Minor Considerations

1. Edge Case Handling: The implementation correctly handles the case where miscRepo might be null, though this is likely a theoretical concern in practice.

2. Transaction Boundaries: While not explicitly wrapped in a transaction, the purge operation's error handling ensures partial failures don't leave the system in an inconsistent state.

🎯 Verdict

Approve - This is a high-quality fix that:

  • ✅ Correctly addresses the reported issue
  • ✅ Follows all project patterns and conventions
  • ✅ Has excellent test coverage
  • ✅ Includes proper error handling
  • ✅ Maintains backward compatibility
  • ✅ No security or performance concerns

The implementation is production-ready and should resolve the ghost packet entries issue effectively.

@github-actions
Copy link
Copy Markdown

System Test Results

MeshMonitor System Test Results

Test Run: 2026-04-26 22:47:23 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 d20f792 into main Apr 27, 2026
20 checks passed
@Yeraze Yeraze deleted the fix-2637-purge-clears-packet-log branch April 27, 2026 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant