fix(nodes): keep active observers as nodes and explain inactive ones (#199)

.github/workflows/deploy.yml

@@ -369,6 +369,11 @@ jobs:
         # (#89). See the file header for the rows and expected classes.
         run: sqlite3 test-fixtures/e2e-fixture.db < test-fixtures/seed-2073-route-adverts.sql
 
+      - name: Seed an inactive observer node for #199 E2E
+        # Also after the migration: the fixture has no inactive_nodes table
+        # until corescope-migrate creates it.
+        run: sqlite3 test-fixtures/e2e-fixture.db < test-fixtures/seed-199-inactive-observer.sql
+
       - name: Start Go server with fixture DB
         run: |
           fuser -k 13581/tcp 2>/dev/null || true
@@ -439,6 +444,7 @@ jobs:
           BASE_URL=http://localhost:13581 node test-live-multibyte-only-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-issue-125-live-toggles-early-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-issue-1150-404-state-e2e.js 2>&1 | tee -a e2e-output.txt
+          BASE_URL=http://localhost:13581 node test-issue-199-inactive-observer-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-issue-1146-path-link-contrast-e2e.js 2>&1 | tee -a e2e-output.txt
           CHROMIUM_REQUIRE=1 node test-issue-1705-subpath-contrast-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-issue-1147-section-order-e2e.js 2>&1 | tee -a e2e-output.txt

cmd/ingestor/db.go

@@ -1895,6 +1895,16 @@ func (s *Store) LogStats() {
 	)
 }
 
+// staleNodesWhere selects the nodes MoveStaleNodes retires: no advert in
+// nodeDays (?1 = cutoff), unless the pubkey is an observer seen since the
+// cutoff (#199). An observer never hears its own adverts, so an online
+// observer can go nodeDays without one reaching another observer. last_seen
+// stays the last advert. Observer ids arrive upper-case from the MQTT topic
+// and node keys are lower-case, hence lower() on both sides; NULL ids are
+// skipped because a single NULL makes NOT IN false for every node.
+const staleNodesWhere = `last_seen < ?1 AND lower(public_key) NOT IN (
+	SELECT lower(id) FROM observers WHERE id IS NOT NULL AND last_seen >= ?1)`
+
 // MoveStaleNodes moves nodes not seen in nodeDays to the inactive_nodes table.
 // Returns the number of nodes moved.
 func (s *Store) MoveStaleNodes(nodeDays int) (int64, error) {
@@ -1905,11 +1915,11 @@ func (s *Store) MoveStaleNodes(nodeDays int) (int64, error) {
 	}
 	defer tx.Rollback()
 
-	_, err = tx.Exec(`INSERT OR REPLACE INTO inactive_nodes SELECT * FROM nodes WHERE last_seen < ?`, cutoff)
+	_, err = tx.Exec(`INSERT OR REPLACE INTO inactive_nodes SELECT * FROM nodes WHERE `+staleNodesWhere, cutoff)
 	if err != nil {
 		return 0, fmt.Errorf("insert inactive: %w", err)
 	}
-	result, err := tx.Exec(`DELETE FROM nodes WHERE last_seen < ?`, cutoff)
+	result, err := tx.Exec(`DELETE FROM nodes WHERE `+staleNodesWhere, cutoff)
 	if err != nil {
 		return 0, fmt.Errorf("delete stale: %w", err)
 	}

cmd/ingestor/issue199_test.go

@@ -0,0 +1,118 @@
+package main
+
+import (
+	"testing"
+	"time"
+)
+
+// #199: MoveStaleNodes must not retire a node whose pubkey is an observer
+// seen within nodeDays. An observer never hears its own adverts, so an
+// online observer can go a week without one reaching another observer.
+
+const (
+	issue199ObserverKey = "a199000000000000000000000000000000000000000000000000000000000001"
+	issue199QuietKey    = "a199000000000000000000000000000000000000000000000000000000000002"
+	issue199PlainKey    = "a199000000000000000000000000000000000000000000000000000000000003"
+	issue199OldAdvert   = "2020-01-01T00:00:00Z"
+)
+
+func issue199Upper(s string) string {
+	b := []byte(s)
+	for i, c := range b {
+		if c >= 'a' && c <= 'z' {
+			b[i] = c - 'a' + 'A'
+		}
+	}
+	return string(b)
+}
+
+func issue199NodeIn(t *testing.T, store *Store, table, pubkey string) (bool, string) {
+	t.Helper()
+	var lastSeen string
+	err := store.db.QueryRow(`SELECT last_seen FROM `+table+` WHERE public_key = ?`, pubkey).Scan(&lastSeen)
+	if err != nil {
+		return false, ""
+	}
+	return true, lastSeen
+}
+
+func issue199Observer(t *testing.T, store *Store, id, lastSeen string) {
+	t.Helper()
+	if _, err := store.db.Exec(`INSERT INTO observers (id, name, last_seen, first_seen) VALUES (?, 'obs', ?, ?)`, id, lastSeen, lastSeen); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestMoveStaleNodesKeepsActiveObserverNode(t *testing.T) {
+	store := newTestStore(t)
+	if err := store.UpsertNode(issue199ObserverKey, "ObsRepeater", "repeater", nil, nil, issue199OldAdvert); err != nil {
+		t.Fatal(err)
+	}
+	// Observer ids are upper-case pubkeys from the MQTT topic; node keys are lower-case.
+	issue199Observer(t, store, issue199Upper(issue199ObserverKey), time.Now().UTC().Format(time.RFC3339))
+
+	moved, err := store.MoveStaleNodes(7)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if moved != 0 {
+		t.Errorf("moved=%d, want 0: an observer seen within nodeDays must stay a node", moved)
+	}
+	in, lastSeen := issue199NodeIn(t, store, "nodes", issue199ObserverKey)
+	if !in {
+		t.Fatal("active observer's node was removed from nodes")
+	}
+	if lastSeen != issue199OldAdvert {
+		t.Errorf("last_seen=%q, want %q: last_seen is the last advert and must not be faked", lastSeen, issue199OldAdvert)
+	}
+	if in, _ := issue199NodeIn(t, store, "inactive_nodes", issue199ObserverKey); in {
+		t.Error("active observer's node was copied to inactive_nodes")
+	}
+}
+
+func TestMoveStaleNodesRetiresQuietObserverAndPlainNode(t *testing.T) {
+	store := newTestStore(t)
+	for _, pk := range []string{issue199QuietKey, issue199PlainKey} {
+		if err := store.UpsertNode(pk, "Stale", "repeater", nil, nil, issue199OldAdvert); err != nil {
+			t.Fatal(err)
+		}
+	}
+	// Last heard as an observer 8 days ago: outside the 7-day window.
+	issue199Observer(t, store, issue199Upper(issue199QuietKey), time.Now().UTC().AddDate(0, 0, -8).Format(time.RFC3339))
+
+	moved, err := store.MoveStaleNodes(7)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if moved != 2 {
+		t.Errorf("moved=%d, want 2", moved)
+	}
+	for _, pk := range []string{issue199QuietKey, issue199PlainKey} {
+		if in, _ := issue199NodeIn(t, store, "nodes", pk); in {
+			t.Errorf("%s still in nodes, want retired", pk[:8])
+		}
+		if in, _ := issue199NodeIn(t, store, "inactive_nodes", pk); !in {
+			t.Errorf("%s missing from inactive_nodes", pk[:8])
+		}
+	}
+}
+
+// A legacy observers row with a NULL id must not stop retention: with a bare
+// NOT IN, one NULL in the subquery makes the predicate NULL for every node.
+func TestMoveStaleNodesNullObserverIDDoesNotBlockRetention(t *testing.T) {
+	store := newTestStore(t)
+	if err := store.UpsertNode(issue199PlainKey, "Stale", "companion", nil, nil, issue199OldAdvert); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := store.db.Exec(`INSERT INTO observers (id, name, last_seen) VALUES (NULL, 'legacy', ?)`, time.Now().UTC().Format(time.RFC3339)); err != nil {
+		t.Fatal(err)
+	}
+
+	moved, err := store.MoveStaleNodes(7)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if moved != 1 {
+		t.Errorf("moved=%d, want 1", moved)
+	}
+}

cmd/server/issue199_node_not_found_test.go

@@ -0,0 +1,155 @@
+package main
+
+import (
+	"encoding/json"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/gorilla/mux"
+)
+
+// #199: a 404 from /api/nodes/{pubkey} carries what the instance still knows
+// about the key -- its inactive_nodes row and/or its observer row -- so the
+// node page can explain the state instead of dead-ending on "Node not found".
+
+const (
+	issue199InactiveObs = "b199000000000000000000000000000000000000000000000000000000000001"
+	issue199ObsOnly     = "b199000000000000000000000000000000000000000000000000000000000002"
+	issue199Unknown     = "b199000000000000000000000000000000000000000000000000000000000003"
+)
+
+// Decoded locally (not via the server's types) so the contract is pinned by
+// its JSON field names.
+type issue199Inactive struct {
+	PublicKey string `json:"public_key"`
+	Name      string `json:"name"`
+	Role      string `json:"role"`
+	LastSeen  string `json:"last_seen"`
+}
+
+type issue199Observer struct {
+	ID       string `json:"id"`
+	Name     string `json:"name"`
+	LastSeen string `json:"last_seen"`
+}
+
+func issue199Get(t *testing.T, router *mux.Router, pubkey string) (int, map[string]json.RawMessage) {
+	t.Helper()
+	req := httptest.NewRequest("GET", "/api/nodes/"+pubkey, nil)
+	w := httptest.NewRecorder()
+	router.ServeHTTP(w, req)
+	var body map[string]json.RawMessage
+	if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil {
+		t.Fatalf("decode %q: %v", w.Body.String(), err)
+	}
+	return w.Code, body
+}
+
+func issue199Seed(t *testing.T, srv *Server) {
+	t.Helper()
+	ensureInactiveNodesTable(t, srv)
+	if _, err := srv.db.conn.Exec(`INSERT INTO inactive_nodes (public_key, name, role, last_seen, first_seen) VALUES (?, 'Quiet Repeater', 'repeater', '2026-09-24T15:35:00Z', '2026-09-11T00:00:00Z')`, issue199InactiveObs); err != nil {
+		t.Fatal(err)
+	}
+	for _, o := range []struct{ id, name string }{
+		{strings.ToUpper(issue199InactiveObs), "Quiet Observer"},
+		{strings.ToUpper(issue199ObsOnly), "Listener Only"},
+	} {
+		if _, err := srv.db.conn.Exec(`INSERT INTO observers (id, name, last_seen, first_seen) VALUES (?, ?, '2026-10-04T04:23:00Z', '2026-09-11T00:00:00Z')`, o.id, o.name); err != nil {
+			t.Fatal(err)
+		}
+	}
+}
+
+func TestNodeDetail404CarriesInactiveNodeAndObserver(t *testing.T) {
+	srv, router := setupTestServer(t)
+	issue199Seed(t, srv)
+
+	code, body := issue199Get(t, router, issue199InactiveObs)
+	if code != 404 {
+		t.Fatalf("status=%d, want 404 (the node is not in nodes)", code)
+	}
+	var inactive issue199Inactive
+	if err := json.Unmarshal(body["inactive_node"], &inactive); err != nil {
+		t.Fatalf("inactive_node missing or malformed: %v (body %v)", err, body)
+	}
+	if inactive.PublicKey != issue199InactiveObs || inactive.Name != "Quiet Repeater" || inactive.Role != "repeater" || inactive.LastSeen != "2026-09-24T15:35:00Z" {
+		t.Errorf("inactive_node=%+v", inactive)
+	}
+	var obs issue199Observer
+	if err := json.Unmarshal(body["observer"], &obs); err != nil {
+		t.Fatalf("observer missing or malformed: %v (body %v)", err, body)
+	}
+	if obs.ID != strings.ToUpper(issue199InactiveObs) || obs.Name != "Quiet Observer" || obs.LastSeen != "2026-10-04T04:23:00Z" {
+		t.Errorf("observer=%+v", obs)
+	}
+
+	// Upper-case path (as an observer id) finds the same rows.
+	if _, body := issue199Get(t, router, strings.ToUpper(issue199InactiveObs)); body["inactive_node"] == nil {
+		t.Error("upper-case pubkey did not find the inactive_nodes row")
+	}
+}
+
+func TestNodeDetail404ObserverWithoutNodeRecord(t *testing.T) {
+	srv, router := setupTestServer(t)
+	issue199Seed(t, srv)
+
+	code, body := issue199Get(t, router, issue199ObsOnly)
+	if code != 404 {
+		t.Fatalf("status=%d, want 404", code)
+	}
+	if body["inactive_node"] != nil {
+		t.Errorf("unexpected inactive_node: %s", body["inactive_node"])
+	}
+	var obs issue199Observer
+	if err := json.Unmarshal(body["observer"], &obs); err != nil || obs.Name != "Listener Only" {
+		t.Errorf("observer=%+v err=%v", obs, err)
+	}
+}
+
+func TestNodeDetail404UnknownStaysBare(t *testing.T) {
+	srv, router := setupTestServer(t)
+	issue199Seed(t, srv)
+
+	code, body := issue199Get(t, router, issue199Unknown)
+	if code != 404 || len(body) != 1 || body["error"] == nil {
+		t.Errorf("status=%d body=%v, want 404 with only an error field", code, body)
+	}
+}
+
+// Without an inactive_nodes table (minimal schemas) the observer still counts.
+func TestNodeDetail404WithoutInactiveTable(t *testing.T) {
+	srv, router := setupTestServer(t)
+	if _, err := srv.db.conn.Exec(`INSERT INTO observers (id, name, last_seen) VALUES (?, 'Listener Only', '2026-10-04T04:23:00Z')`, strings.ToUpper(issue199ObsOnly)); err != nil {
+		t.Fatal(err)
+	}
+	code, body := issue199Get(t, router, issue199ObsOnly)
+	if code != 404 || body["observer"] == nil || body["inactive_node"] != nil {
+		t.Errorf("status=%d body=%v", code, body)
+	}
+}
+
+// Blacklisted or hidden identities get the bare 404: nothing about the rows leaks.
+func TestNodeDetail404HiddenIdentityStaysBare(t *testing.T) {
+	cases := []struct {
+		name  string
+		setup func(*Server)
+	}{
+		{"node blacklist", func(s *Server) { s.cfg.SetNodeBlacklist([]string{issue199InactiveObs}) }},
+		{"observer blacklist", func(s *Server) { s.cfg.ObserverBlacklist = []string{strings.ToUpper(issue199InactiveObs)} }},
+		{"hidden inactive name", func(s *Server) { s.cfg.SetHiddenNamePrefixes([]string{"Quiet Rep"}) }},
+		{"hidden observer name", func(s *Server) { s.cfg.SetHiddenNamePrefixes([]string{"Quiet Obs"}) }},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			srv, router := setupTestServer(t)
+			issue199Seed(t, srv)
+			tc.setup(srv)
+			code, body := issue199Get(t, router, issue199InactiveObs)
+			if code != 404 || len(body) != 1 || body["error"] == nil {
+				t.Errorf("status=%d body=%v, want bare 404", code, body)
+			}
+		})
+	}
+}