fix: PG packet monitor names and key mismatch flag not clearing#2406
fix: PG packet monitor names and key mismatch flag not clearing#2406
Conversation
Two PostgreSQL-specific bugs: 1. Packet Monitor showing hex IDs instead of node names on PG: PostgreSQL lowercases unquoted column aliases, so `from_node_longName` became `from_node_longname` in results. Added normalization in normalizePacketLogRow to handle both cases. 2. Key mismatch warning never clearing on any backend: Setting `nodeData.keySecurityIssueDetails = undefined` to clear it was ineffective because the upsert used `??` which treats undefined as "not provided" and falls back to the existing value. Changed to `null` and updated upsert to use `!== undefined` checks so null explicitly clears the field. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Claude finished @Yeraze's task —— View job PR Code ReviewReviewing this pull request for code quality, potential bugs, performance considerations, security concerns, and test coverage. Todo List:
Review Findings✅ Overall Assessment: APPROVEDBoth fixes are well-implemented and address real issues with clear root cause analysis. The changes are minimal, focused, and follow project conventions. 🔧 Bug Fix 1: PostgreSQL Packet Monitor NamesProblem: PostgreSQL lowercases unquoted column aliases, causing Fix: Added fallback logic in from_node_longName: row.from_node_longName ?? row.from_node_longname ?? null,
to_node_longName: row.to_node_longName ?? row.to_node_longname ?? null,✅ Quality: Excellent defensive programming - handles both camelCase and lowercase variants without breaking other databases. 🔧 Bug Fix 2: Key Mismatch Warnings Never ClearingProblem: Code set Root Cause: Fix:
✅ Quality:
📊 Code Quality AssessmentStrengths:
Performance Considerations:
Security Considerations:
Test Coverage:
🎯 Specific Technical NotesPostgreSQL Column Aliasing:The PostgreSQL behavior is standard SQL - unquoted identifiers are lowercased. This fix is more robust than quoting aliases because it maintains compatibility with existing queries. Null vs Undefined Semantics:The distinction is crucial in JavaScript/TypeScript database operations:
Database Architecture Compliance:Changes follow the repository's multi-database architecture patterns correctly, using the base repository's normalization methods. 📝 Minor Suggestions
✅ Recommendation: APPROVE AND MERGEThis PR addresses two legitimate bugs with clean, targeted fixes. The code follows project conventions, maintains backward compatibility, and includes proper testing validation. No blocking issues found. |
Summary
Two bugs affecting PostgreSQL (and key clearing on all backends):
1. Packet Monitor showing hex IDs instead of node names on PostgreSQL
SELECT ... as from_node_longNamereturnsfrom_node_longnamein PG results, but frontend expectsfrom_node_longName.normalizePacketLogRownow checks both cases:row.from_node_longName ?? row.from_node_longname2. Key mismatch warning never clearing
nodeData.keySecurityIssueDetails = undefinedto clear the field, but the upsert used??(nullish coalescing) which treatsundefinedas "not provided" and falls back to the existing DB value — so the warning persisted forever.null, and updated upsert to use!== undefinedchecks sonullexplicitly clears the field.Changes
src/db/repositories/misc.tsnormalizePacketLogRowsrc/db/repositories/nodes.ts!== undefinedfor security flag fields in upsertsrc/server/meshtasticManager.tsnullinstead ofundefinedto clear security fieldssrc/server/routes/securityRoutes.tsnullfix for manual security flag clearTest plan
npx vitest run— 3070 tests passnpm run build— no TypeScript errors🤖 Generated with Claude Code