fix: handle companion nodes without adverts in My Mesh health cards (#696)

## Summary

Fixes #665 — companion nodes claimed in "My Mesh" showed "Could not load
data" because they never sent an advert, so they had no `nodes` table
entry, causing the health API to return 404.

## Three-Layer Fix

### 1. API Resilience (`cmd/server/store.go`)
`GetNodeHealth()` now falls back to building a partial response from the
in-memory packet store when `GetNodeByPubkey()` returns nil. Returns a
synthetic node stub (`role: "unknown"`, `name: "Unknown"`) with whatever
stats exist from packets, instead of returning nil → 404.

### 2. Ingestor Cleanup (`cmd/ingestor/main.go`)
Removed phantom sender node creation that used `"sender-" + name` as the
pubkey. Channel messages don't carry the sender's real pubkey, so these
synthetic entries were unreachable from the claiming/health flow — they
just polluted the nodes table with unmatchable keys.

### 3. Frontend UX (`public/home.js`)
The catch block in `loadMyNodes()` now distinguishes 404 (node not in DB
yet) from other errors:
- **404**: Shows 📡 "Waiting for first advert — this node has been seen
in channel messages but hasn't advertised yet"
- **Other errors**: Shows  "Could not load data" (unchanged)

## Tests
- Added `TestNodeHealthPartialFromPackets` — verifies a node with
packets but no DB entry returns 200 with synthetic node stub and stats
- Updated `TestHandleMessageChannelMessage` — verifies channel messages
no longer create phantom sender nodes
- All existing tests pass (`cmd/server`, `cmd/ingestor`)

Co-authored-by: you <you@example.com>
This commit is contained in:
Kpa-clawbot
2026-04-09 20:03:52 -07:00
committed by GitHub
parent fcad49594b
commit 2e1a4a2e0d
5 changed files with 95 additions and 23 deletions
+4 -12
View File
@@ -203,21 +203,13 @@ func TestHandleMessageChannelMessage(t *testing.T) {
t.Errorf("direction=%v, want rx", direction)
}
// Should create sender node
// Sender node should NOT be created (see issue #665: synthetic "sender-" keys
// are unreachable from the claiming/health flow)
if err := store.db.QueryRow("SELECT COUNT(*) FROM nodes").Scan(&count); err != nil {
t.Fatal(err)
}
if count != 1 {
t.Errorf("nodes count=%d, want 1 (sender node)", count)
}
// Verify sender node name
var nodeName string
if err := store.db.QueryRow("SELECT name FROM nodes LIMIT 1").Scan(&nodeName); err != nil {
t.Fatal(err)
}
if nodeName != "Alice" {
t.Errorf("node name=%s, want Alice", nodeName)
if count != 0 {
t.Errorf("nodes count=%d, want 0 (no phantom sender node)", count)
}
}
+5 -7
View File
@@ -446,13 +446,11 @@ func handleMessage(store *Store, tag string, source MQTTSource, m mqtt.Message,
log.Printf("MQTT [%s] channel insert error: %v", tag, err)
}
// Upsert sender as a companion node
if sender != "" {
senderKey := "sender-" + strings.ToLower(sender)
if err := store.UpsertNode(senderKey, sender, "companion", nil, nil, now); err != nil {
log.Printf("MQTT [%s] sender node upsert error: %v", tag, err)
}
}
// Note: we intentionally do NOT create a node entry for channel message senders.
// Channel messages don't carry the sender's real pubkey, so any entry we create
// would use a synthetic key ("sender-<name>") that doesn't match the real pubkey
// used for claiming/health lookups. The node will get a proper entry when it
// sends an advert. See issue #665.
log.Printf("MQTT [%s] channel message: ch%s from %s", tag, channelIdx, firstNonEmpty(sender, "unknown"))
return
+61
View File
@@ -774,6 +774,67 @@ func TestNodeHealthNotFound(t *testing.T) {
}
}
// TestNodeHealthPartialFromPackets verifies that a node with packets in the
// in-memory store but no DB entry returns a partial 200 response instead of 404.
// This is the fix for issue #665 (companion nodes without adverts).
func TestNodeHealthPartialFromPackets(t *testing.T) {
srv, router := setupTestServer(t)
// Inject a packet into byNode for a pubkey that doesn't exist in the nodes table
ghostPubkey := "ghost_companion_no_advert"
now := time.Now().UTC().Format(time.RFC3339)
snr := 5.0
srv.store.mu.Lock()
if srv.store.byNode == nil {
srv.store.byNode = make(map[string][]*StoreTx)
}
if srv.store.nodeHashes == nil {
srv.store.nodeHashes = make(map[string]map[string]bool)
}
srv.store.byNode[ghostPubkey] = []*StoreTx{
{Hash: "abc123", FirstSeen: now, SNR: &snr, ObservationCount: 1},
}
srv.store.nodeHashes[ghostPubkey] = map[string]bool{"abc123": true}
srv.store.mu.Unlock()
req := httptest.NewRequest("GET", "/api/nodes/"+ghostPubkey+"/health", nil)
w := httptest.NewRecorder()
router.ServeHTTP(w, req)
if w.Code != 200 {
t.Fatalf("expected 200 for ghost companion, got %d (body: %s)", w.Code, w.Body.String())
}
var body map[string]interface{}
if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil {
t.Fatalf("json unmarshal: %v", err)
}
// Should have a synthetic node stub
node, ok := body["node"].(map[string]interface{})
if !ok || node == nil {
t.Fatal("expected node in response")
}
if node["role"] != "unknown" {
t.Errorf("expected role=unknown, got %v", node["role"])
}
if node["public_key"] != ghostPubkey {
t.Errorf("expected public_key=%s, got %v", ghostPubkey, node["public_key"])
}
// Should have stats from the packet
stats, ok := body["stats"].(map[string]interface{})
if !ok || stats == nil {
t.Fatal("expected stats in response")
}
if stats["totalPackets"] != 1.0 { // JSON numbers are float64
t.Errorf("expected totalPackets=1, got %v", stats["totalPackets"])
}
if stats["lastHeard"] == nil {
t.Error("expected lastHeard to be set")
}
}
func TestBulkHealthEndpoint(t *testing.T) {
_, router := setupTestServer(t)
req := httptest.NewRequest("GET", "/api/nodes/bulk-health?limit=10", nil)
+17 -1
View File
@@ -5629,9 +5629,25 @@ func (s *PacketStore) GetBulkHealth(limit int, region string) []map[string]inter
func (s *PacketStore) GetNodeHealth(pubkey string) (map[string]interface{}, error) {
// Fetch node info from DB (fast single-row lookup)
node, err := s.db.GetNodeByPubkey(pubkey)
if err != nil || node == nil {
if err != nil {
return nil, err
}
// If the node isn't in the DB (e.g. companion that never advertised),
// check if we have any packet data for it. If so, build a partial response.
if node == nil {
s.mu.RLock()
hasPackets := len(s.byNode[pubkey]) > 0
s.mu.RUnlock()
if !hasPackets {
return nil, nil
}
// Build a synthetic node stub so the rest of the function works
node = map[string]interface{}{
"public_key": pubkey,
"name": "Unknown",
"role": "unknown",
}
}
s.mu.RLock()
defer s.mu.RUnlock()
+8 -3
View File
@@ -302,14 +302,19 @@
<button class="mnc-btn" data-action="packets" data-key="${mn.pubkey}">View packets </button>
</div>
</div>`;
} catch {
} catch (err) {
const is404 = err && err.message && err.message.includes('404');
const statusIcon = is404 ? '📡' : '❓';
const statusMsg = is404
? 'Waiting for first advert — this node has been seen in channel messages but hasn\u2019t advertised yet'
: 'Could not load data';
return `<div class="my-node-card silent" data-key="${mn.pubkey}" tabindex="0" role="button">
<div class="mnc-header">
<div class="mnc-status"></div>
<div class="mnc-status">${statusIcon}</div>
<div class="mnc-name">${escapeHtml(mn.name || truncate(mn.pubkey, 12))}</div>
<button class="mnc-remove" data-key="${mn.pubkey}" title="Remove" aria-label="Remove ${escapeAttr(mn.name || truncate(mn.pubkey, 12))} from My Mesh"></button>
</div>
<div class="mnc-status-text">Could not load data</div>
<div class="mnc-status-text">${statusMsg}</div>
</div>`;
}
}));