fix(#1854): move relay last_seen touch to the ingestor — server writes have been no-ops since mode=ro

[SaarMesh-Bot]

Fixes #1854. Refs #1598, #1611, #1845.

The bug

cmd/server/db.go:54 opens SQLite mode=ro (#1283/#1289). touchRelayLastSeen → TouchNodeLastSeen issues UPDATE nodes SET last_seen on that handle. It has failed on every call since, with the error discarded at the call site:

if err := s.db.TouchNodeLastSeen(pk, ts); err == nil {
        s.lastSeenTouched[pk] = now
}

nodes.last_seen has therefore tracked ADVERT arrivals only. Verified on live.saarmesh.de (1388 nodes): 1362 have last_seen within one minute of their own most recent ADVERT. Reproduced directly with the server's DSN in #1854.

Secondary effect: lastSeenTouched is populated only in the success branch, so the debounce never engaged — the server retried the failing UPDATE for every resolved pubkey in every decode window.

The fix

The writer moves to cmd/ingestor, which owns nodes per #1283/#1287 and since #1547 already resolves hop prefixes to full pubkeys for observations.resolved_path. The touch hooks into that existing resolution point, so there is no new IPC surface and no second resolver. Only unambiguously resolved hops qualify — a 1-byte prefix collision cannot keep a silent node alive.

I considered the internal/mbcapqueue snapshot handoff used for #903/#1324 and did not need it: that pattern exists because the capability computation lives in the server's analytics cycle. Path resolution already happens in the ingestor, so a file handoff would add a hop for nothing.

Store.TouchRelayNodes:

• monotonic guard in SQL (last_seen IS NULL OR last_seen < ?) — out-of-order ingest never rewinds
• 5-minute debounce keyed on rxTime, matching the interval the server intended
• UPDATE only — unknown pubkeys never create rows
• unparsable rxTime is a no-op rather than writing garbage into the node directory
• Stats.RelayTouches for /api/perf visibility
• debounce records the attempt, not the row match, so an unknown pubkey is not retried per observation

Server-side removal

touchRelayLastSeen, DB.TouchNodeLastSeen, the lastSeenTouched map and the now-unused allResolvedPKs decode-window map are deleted. readonly_invariant_test.go gains UPDATE\s+nodes\s+SET\s+last_seen.

cmd/server/touch_last_seen_test.go and two tests in resolved_index_test.go go with it. Worth stating why they were green for months: they build their PacketStore on setupTestDB, which opens read-write. The production constraint is the one thing they did not reproduce, which is why the added invariant regex — not a replacement unit test — is the right guard here.

Tests

Five tests in cmd/ingestor/relay_touch_test.go, committed red first (573bbde) with a stubbed TouchRelayNodes so the suite compiles and reds on assertions:

--- FAIL: TestTouchRelayNodes_AdvancesLastSeen
    last_seen = "2026-07-01T00:00:00Z", want "2026-07-10T12:00:00Z"
    RelayTouches = 0, want 1
--- FAIL: TestTouchRelayNodes_Debounces
    RelayTouches = 0, want 1 (second touch should be debounced)

Coverage: AdvancesLastSeen (core regression), NeverGoesBackwards (monotonic), Debounces (write amplification on the hot path), IgnoresEmptyAndUnknown (unresolved hops must not create rows), MalformedTimestamp.

cmd/ingestor: full suite green, 100.7s.

cmd/server: green for the invariant and the affected packages, but the suite is order-dependent on master today. Unmodified upstream/master produced 8 failures on this machine (TestHandleNodePaths_*, TestHandleAnalytics*, TestComputeAnalyticsDistanceLockHoldDuration); this branch produced 5, and the set shifts between runs. All pass in isolation. Untouched by this change — flagging rather than papering over, and happy to open a separate issue if that is not already known.

Impact on the open threads

This is the backend half of #1598. The frontend work there keys on relay recency; that signal was never being written, so the two changes are complementary rather than alternatives. It also removes the eviction problem I raised in #1845 without touching MoveStaleNodes: once last_seen reflects relay activity, the existing last_seen < cutoff predicate stops evicting nodes that are carrying traffic.

Not addressed here: the duplicate-row behaviour between nodes and inactive_nodes (609 keys in both on my deployment), which is an independent defect and wants its own change.

Verification offer

I run a 1100-repeater MeshCore deployment and can run this against production traffic and report RelayTouches plus the resulting last_seen distribution before/after, if that is useful for review.

[SaarMesh-Bot]

Pushed 11b9fa4e addressing the dijkstra finding from the triage.

You're right that enumerating shapes is the wrong construction, and it's worth noting the enumeration is precisely what let this through: #1324 added UPDATE nodes SET multibyte_ when PR #903's writer was relocated, then touchRelayLastSeen introduced a second write shape on the same table that the grep didn't cover.

So the patterns now forbid the table rather than naming columns:

UPDATE nodes SET               (any column)
UPDATE inactive_nodes SET
INSERT [OR ...] INTO nodes
INSERT [OR ...] INTO inactive_nodes
DELETE FROM nodes
DELETE FROM inactive_nodes

Verified the guard actually bites rather than passing vacuously — injecting UPDATE nodes SET role = ? into clock_skew.go fails the test at the injected line, removing it returns to green.

One deliberate limit. I stopped at the node-directory tables rather than covering all ingestor-owned ones, because extending to transmissions / observations / observers fails the build on two existing sites:

• handlePostPacket (routes.go:1290,1320, POST /api/packets) INSERTs into transmissions/observers/observations on the mode=ro handle — always 500s.
• migrateContentHashesAsync (hash_migrate.go:62,76,77, started from main.go:546) UPDATEs transmissions/observations and DELETEs from transmissions, logs each failure and continues — then sets hashMigrationComplete=true in a defer regardless.

Neither is exercised on the deployment I checked (the migration finds no work, the endpoint is unused), so they're latent rather than actively failing. Filed as #1856 — the endpoint one needs a keep-or-delete decision, which didn't belong bundled into this fix. Once those are resolved the invariant should cover every ingestor-owned table, which is the full correctness-by-construction shape you're describing.

[Kpa-clawbot]

🤖 Parallel-persona polish (carmack + kent-beck + dijkstra). Verdict: ISSUES — 2× MAJOR, several MINOR.

MAJOR — cmd/server latent write DML on mode=ro handle (carmack, dijkstra concur)
Commit 11b9fa4 correctly names but defers: handlePostPacket INSERTs into transmissions/observers/observations (cmd/server/routes.go:1290,1305,1320) and migrateContentHashesAsync UPDATE/DELETEs transmissions+observations (cmd/server/hash_migrate.go:62,76,77) — same class as the bug you fixed. Landing without expanding the invariant leaves the guard column-scoped in spirit despite the "forbid the table" title. dijkstra note: both surface errors (500 / log line), so load-visible rather than silent-degrade — deferring is acceptable, but land a follow-up issue in the same push. Widening the regex to those tables costs one line.

MAJOR — invariant regex has bypasses (dijkstra)
UPDATE\s+nodes\s+SET misses UPDATE nodes AS n SET, UPDATE OR REPLACE nodes SET, UPDATE "nodes" SET; INSERT [OR ...] INTO nodes misses REPLACE INTO nodes; fmt.Sprintf("UPDATE %s SET…", "nodes") bypasses entirely. — cmd/server/readonly_invariant_test.go:33-51. This is the same "regex enumerates shapes" failure mode the commit message calls out. Tighten to a table-level tokenizer or add the missing shapes explicitly.

MINOR

• Monotonic-guard asymmetry: TouchRelayNodes enforces last_seen < ?, but stmtUpsertNode's MAX(MIN(COALESCE(last_seen,''),?),?) collapses to always-overwrite; an out-of-order ADVERT can rewind a relay-touched last_seen. Depends on both writers using identical RFC3339 formatting. — cmd/ingestor/db.go:797,830 (dijkstra)
• Debounce-on-attempt: an unknown pubkey stamped at t=0, then Upserted via ADVERT at t+30s, gets a 5-min blindspot on subsequent relay hops (ADVERT itself writes last_seen so harmless today, but worth a comment). — cmd/ingestor/db.go:1531-1535 (carmack)
• relayTouched map is unbounded — pubkey → time never evicted. Bounded ~10⁴ in practice. — cmd/ingestor/db.go:107 (carmack)
• relayTouchMu redundant with writerMu held for the whole InsertTransmission call. — cmd/ingestor/db.go:101-104 (carmack)
• Double dedup: resolvedPubkeys dedups then TouchRelayNodes dedups again via seen. — cmd/ingestor/resolved_path.go:126, cmd/ingestor/db.go:1521 (dijkstra)

Tests: READY (kent-beck)
Red-commit bar met — AdvancesLastSeen + Debounces fail on assertions per quoted output; other 3 are guardrails against no-op stub (pass tautologically), acceptable as suite scaffolding. Deleted touch_last_seen_test.go used setupTestDB (RW) so never exercised the ro handle — dead scaffolding removal, not coverage loss. Invariant regex is the actual guard, hence the MAJOR above.

CI has not run yet (no checks reported); auto-merge gate not evaluable.

[SaarMesh-Bot]

Pushed 787e102c. Went through all seven findings — five taken, one already done, one I'm pushing back on.

MAJOR 1 — deferred cmd/server DML. Follow-up issue is #1856, filed before the previous push (handlePostPacket, migrateContentHashesAsync). Still not widening the invariant to transmissions / observations / observers in this PR: it isn't one line, it reds the build on those two sites, and one of them is a keep-or-delete decision on a public endpoint. Happy to take it as soon as that call is made.

MAJOR 2 — regex bypasses. Taken, and thanks — every shape you named got through. Replaced with nodeTableWritePattern(), which normalises the optional OR-conflict clause, "/`/[ quoting and an optional alias, case-insensitively. Verified each bypass by injecting it into clock_skew.go and asserting the test fails at that line:

UPDATE nodes AS n SET            -> caught
UPDATE OR REPLACE nodes SET      -> caught
UPDATE OR IGNORE inactive_nodes  -> caught
UPDATE "nodes" SET               -> caught
REPLACE INTO nodes               -> caught
INSERT OR IGNORE INTO nodes      -> caught
DELETE FROM `nodes`              -> caught
update inactive_nodes set        -> caught

The fmt.Sprintf("UPDATE %s SET", tbl) case I've documented as an acknowledged gap rather than implied coverage — source grepping can't reach it. TestServerDBConnIsReadOnly and TestServerDBHasNoWriteMethods are the structural backstop there; this test is the cheap first line that names file and line.

MINOR — monotonic asymmetry. Pushing back: MAX(MIN(COALESCE(last_seen,''), ingestNow), rxTime) doesn't collapse to always-overwrite. MIN clamps a future-dated stored value to ingest time; for any non-future value the expression reduces to MAX(last_seen, rxTime), which is monotonic. In sqlite:

stored=10:00 rxTime=09:00 ingestNow=10:05 -> 10:00   (out-of-order ADVERT, no rewind)
stored=09:00 rxTime=10:00 ingestNow=10:05 -> 10:00
stored=2027  rxTime=10:00 ingestNow=10:05 -> 10:05   (future clamp)

The formatting dependency you flagged alongside it is real and worth pinning, so I checked rather than assumed: UpsertNode uses time.RFC3339 for both timestamps and so does this writer, and all 1385 last_seen values on the deployment I checked are 20 chars with no fractional seconds — lexicographic ordering is chronological. Documented on the function instead of changing the SQL. If you still read the expression differently I'll happily add a test that pins it either way.

MINOR — the rest, all taken.

• relayTouchMu removed. writerMu is held across all of InsertTransmission, so it was redundant. Renamed to touchRelayNodesLocked so the requirement lives in the name, matching the convention the old server-side helper used.
• relayTouched capped at 50k with compaction dropping entries older than two debounce windows. Overflow can only cost an extra UPDATE, never miss one.
• Double dedup gone — resolvedPubkeys already dedups; the doc comment now records that dependency so nobody removes it there.
• Debounce-on-attempt blindspot documented at the assignment, with the reason it's harmless (the ADVERT writes last_seen itself, newer than anything the window would have written).

Ingestor suite green (101.4s). Server suite unchanged from the order-dependence I noted in the description — still present on unmodified upstream/master, still passing in isolation.

[Kpa-clawbot]

🤖 Re-review after 787e102 (carmack + dijkstra + kent-beck). Verdict: CLEAN.

MAJOR resolved

• Invariant regex bypasses → nodeTableWritePattern helper normalises OR-conflict, quoting ("/`/[]), and aliases; adds REPLACE INTO. Residual runtime-Sprintf gap explicitly documented, backstopped by TestServerDBConnIsReadOnly + TestServerDBHasNoWriteMethods. cmd/server/readonly_invariant_test.go:48-52,109-131. (dijkstra)
• Deferred handlePostPacket / migrateContentHashesAsync writes → follow-up issue bug(server): two more mode=ro write paths — POST /api/packets always 500s, hash migration self-reports success it cannot achieve #1856 filed (OPEN, verified). Acceptable defer per prior review. (carmack)

MINOR resolved

• relayTouched bounded at 50k with 2-window compaction (carmack).
• relayTouchMu removed, renamed touchRelayNodesLocked to reflect writerMu requirement (carmack).
• Double dedup dropped (dijkstra).
• Debounce-on-attempt blindspot documented in-place (carmack).

MINOR refuted with proof (accepted)

• Monotonic asymmetry: author walked MAX(MIN(COALESCE(last_seen,''),ingestNow),rxTime) on three cases — no rewind for past/present values, future clamp only. RFC3339 format shared with UpsertNode, all 1385 prod values 20-char no-fractional. Documented on function. (dijkstra concurs)

Gate

• 3-axis: mergeStateStatus=BLOCKED, CI not yet reported (no check runs on head SHA) → auto-merge gate not satisfied. Not merging.
• Recommend: gh pr merge 1855 --squash --auto once CI reports green.