[adminopenclaw8-sketch]

Relates to #289

Implements M2 of the plan on #289 (comment "Plan — CS-pve-agent3 #289", section 4). M1 — switching the two batch readers to each observation's own route type — shipped in #308. This PR repairs the observations.resolved_path rows those readers had already written with the wrong route type. No plan is posted separately; it is below.

Round 2 (review "Review — CS-MacBook PR#590 — head 7f7126d"): the write rule now also requires the stored value to be what the pre-#308 rule produces, the pass is behind an opt-in config switch, and the rollback section is corrected. Details in each section.

Problem

DIRECT and FLOOD receptions of the same payload share one transmissions row, because the content hash does not cover the header route bits (MeshCore src/Packet.cpp calculatePacketHash). t.route_type is the route type of the first reception. Until #308 the resolved-path backfill used it for every observation under that row, so a reception carrying the other route family was resolved with the wrong rule, in both directions:

direction	what the pre-#308 backfill stored
DIRECT reception under a FLOOD transmission	anchored on the observer → a relay the observer never heard the packet from
FLOOD reception under a DIRECT transmission	no observer anchor → resolved_path left NULL

The second direction is the reason this cannot be left to the corrected backfill: the #188 backfill moves its watermark past every row it scans, resolved or not, so it never revisits those NULL rows. This pass therefore scans observations.id from 0, not from the backfill watermark.

Plan / change

One new file, cmd/ingestor/resolved_path_route_repair.go, plus eight lines of wiring in main.go, one config field and its documentation. No schema change to existing tables, no server change, no frontend change.

1. Opt-in. resolvedPathRouteRepair.enabled, default false. The pass overwrites resolved_path in place and keeps no copy of the previous value, so it must not run on the first boot of any build that happens to contain it; an operator turns it on deliberately, after taking a snapshot. With the switch off nothing is scheduled and nothing is recorded in _async_migrations, so turning it on later still runs the pass. Documented in config.example.json and docs/user-guide/configuration.md, both of which say to snapshot first. The switch and the three bounds below are customizer candidates later (AGENTS.md rule 8).

2. Async migration resolved_path_route_type_repair_v1 (RunAsyncMigration, the Same advert seen on flood and zero-hop routes keeps the route of the first inserted observation #89 pattern), started next to the feat(ingestor): resolve the last hop from the observer, relay-only prefix index, backfill NULL resolved_path #188 backfill in main.go — after the ingest buffer is draining and the neighbour-edges builder is running — and cancelled on shutdown.

3. Readiness gate. The pass resolves against the live prefix index and neighbour graph, so it waits for resolvedPathBackfillReady (primed index, a post-build graph with at least one edge) before any batch, and re-checks per batch. Resolving against a cold graph would move the watermark past rows that cannot resolve yet. A batch that finds the snapshot unusable after the gate has passed writes nothing and the loop retries the same watermark.

4. Candidates. A row is a candidate when the fix(ingestor): use each observation's own route type in backfill and neighbor builder (#289 M1) #308 helper observationRouteType(header, txRoute, mask) disagrees with t.route_type about flood-ness — the only thing resolveObservationPath reads. That covers both directions above and the rows whose frame is gone but whose route_mask holds both families (where fix(ingestor): use each observation's own route type in backfill and neighbor builder (#289 M1) #308 now reports the route type as unknown and the pre-fix(ingestor): use each observation's own route type in backfill and neighbor builder (#289 M1) #308 backfill used t.route_type). A SQL prefilter over a bounded id window selects a superset; Go re-checks each row with the helper itself:

   (length(o.raw_hex) >= 2
     AND ((instr('0123456789abcdef', lower(substr(o.raw_hex,2,1))) - 1) & 2) != (COALESCE(t.route_type,-1) & 2))
   OR (COALESCE(length(o.raw_hex),0) < 2
     AND t.route_mask IS NOT NULL AND (t.route_mask & 3) != 0 AND (t.route_mask & 12) != 0
     AND COALESCE(t.route_type,-1) IN (0,1))

   (& 2 separates the families — the plan's count query. Bit 1 is the only bit that does: the transport types, 0 and 3, share bit 0 with the other family's non-transport type, which is why the tests now carry a route-0 and a route-3 case. An unparseable second character yields -1, whose bit 1 is set, so such a row is never dropped by the prefilter when t.route_type is a flood type.)

5. What it writes. Each candidate is resolved twice outside any transaction: want with the observation's own route type and had with t.route_type (what the pre-fix(ingestor): use each observation's own route type in backfill and neighbor builder (#289 M1) #308 reader would produce with today's graph). A row is written only when both:

   • want != had — the route type actually changes the answer. A candidate whose answer is route-type-independent is left alone even if its stored value is stale for some other reason; that is not Plan: per-observation route type — backfill and neighbor-builder use the transmission's first route type #289, and rewriting it would turn this into an unbounded re-resolve of history.
   • stored == had — the stored value is exactly what the pre-fix(ingestor): use each observation's own route type in backfill and neighbor builder (#289 M1) #308 rule produces, which is the signature of a row that rule wrote. A family mismatch on its own is not that signature (round 2, F1): live ingest has always resolved with the frame's own route type, so most mismatched rows hold a correct ingest value, which differs from today's had as soon as the graph or the prefix index has drifted. Those rows are read and left alone, and so is a row an earlier run already repaired (whose stored value is want).
   • want may be empty; the row is then set back to NULL. That is the repair for a stored wrong relay.

6. Bounds. Each batch scans at most idWindow ids (default 20,000) and reads at most batchSize candidates (default 500), so a window with no candidates cannot become one unbounded range scan on the single SQLite connection. Resolution runs outside the transaction (flaky: TestResolvedPathBackfill_WriteHoldUnderBudget_188 — wall-clock write-hold budget fails under CI load #267); one WriterTx per batch under writerMu, so a live insert queues behind a batch and not behind the run. 250 ms pause between batches. The three bounds are package vars — no new config block.

7. Resume and idempotence. Row rewrites, the watermark and the counters commit in one transaction (resolved_path_route_repair_state), so a crash or SIGTERM repeats at most the batch that did not commit; a cancel is returned as an error so _async_migrations records the run unfinished and the next start resumes it. Every UPDATE is guarded on COALESCE(resolved_path,'') = <the value the batch read>, so a repeat is a no-op and a row live ingest rewrote in between keeps ingest's value. The ceiling is pinned on the first run: rows above it come from live ingest, which has always used the frame's own route type.

8. Sizing script. scripts/resolved-path-route-repair-estimate.sh — read-only (immutable=1 URI), counts the candidates split by header family first (which is the bug direction), each direction then split into stored and NULL rows, with unparseable headers reported on their own; reports any recorded run and estimates the pass. One query, so one scan. Mirrors scripts/decode-backfill-estimate.sh (Backfill historical channel and region decoding when keys are added #380), with unit tests and ShellCheck wired into CI.

cmd/server is not touched and stays read-only. No new map[string]interface{}. No colours, no frontend, so the XSS gate has nothing to scan.

Sizing

BenchmarkResolvedPathRouteRepairPass_289 builds a synthetic database of the shape the plan describes and runs one full pass. Mix: 1.2M observations over 400K transmissions (3 receptions per transmission), every third transmission carrying both route families in route_mask, half of those with a DIRECT first reception so both repair directions are measured, 1 observation in 100 a #289 candidate (4,002 of 1.2M: 1,334 cleared and 2,668 that gain a path). Apple M4, 10 cores, warm page cache, 311 MB database.

Metric	Value
observations / transmissions	1,200,000 / 400,000
candidates found and repaired	4,002 (1,334 cleared, 2,668 resolved)
full pass, pause disabled	511 ms (426 ns per observation), 61 batches
writer hold per batch	p50 0.8 ms, p95 12.7 ms, p99 13.3 ms, max 13.3 ms
one 20,000-id window (BenchmarkResolvedPathRouteRepairBatch_289)	19.7 ms

The p50 is the windows with no candidate (state write only); the p95/p99 are the windows that rewrite rows. Both are far below the 500 ms slow-writer threshold and below the #89 backfill's measured p99 of 161 ms. Adding the rows that gain a path (round 2, F7) did not change the order of magnitude: a repaired row writes a 66-byte JSON array where a cleared row writes a NULL.

Extrapolation to ~6.5M observations (the count in the #89 sizing comment): 426 ns/observation × 6.5M ≈ 3 s of CPU, plus 325 windows × 250 ms ≈ 81 s of deliberate pause — about 1.5 min of wall clock on a warm cache. Two honest caveats: this is a 311 MB database with everything in page cache, where production at 6.5M observations is a multi-GB file, so the real pass is cold-cache I/O bound (the #89 comment measured 18–19.5 s cold for a single full scan of transmissions on a 4.95 GB copy); and the candidate rate is synthetic. Run the sizing script on a snapshot copy for the real candidate counts before scheduling this anywhere:

scripts/resolved-path-route-repair-estimate.sh /path/to/snapshot.db

Nothing was run against staging or production for this PR.

Tests

All in cmd/ingestor, file resolved_path_route_repair_289_test.go. "before" is a stub pass that reads and advances exactly as the real one but writes no row — master's behaviour, since no repair exists there.

Requirement	Test	before	after
Both bug directions repaired, plain and transport route types	TestResolvedPathRouteRepair_RepairsBothDirections_289 — 13 seeded rows in one table: DIRECT frame under FLOOD tx (wrong relay cleared), FLOOD frame under DIRECT tx (NULL resolved), TRANSPORT_DIRECT under FLOOD and TRANSPORT_FLOOD under DIRECT (the route-0/route-3 pair), no frame + mixed mask + FLOOD tx (cleared)	red	green
Correct live-ingest rows are not rewritten when the graph has drifted	TestResolvedPathRouteRepair_LeavesDriftedLiveIngestRowsUntouched_289 — the reviewer's two probe rows on the #188 fixture: a DIRECT and a FLOOD reception whose stored relay is not what the pre-#308 rule gives. 0 repaired, 0 cleared, 2 candidates read. The same two rows are in the shared 13-row table	red (1 repaired, 1 cleared)	green
No change to correct rows	RepairsBothDirections_289 — FLOOD/FLOOD, DIRECT/DIRECT, no frame + mixed mask + DIRECT tx, flood-only mask, NULL mask: all unchanged; plus a candidate whose answer is route-type-independent (["c355"]) keeps its stale stored value	green	green
NULL candidates below the backfill watermark	TestResolvedPathRouteRepair_RepairsNullRowBelowBackfillWatermark_289 — the watermark is set past the row, RunResolvedPathBackfill scans 0 rows, the repair fixes it	red	green
Idempotency	TestResolvedPathRouteRepair_Idempotent_289 — a second pass is a no-op, and a rescan from watermark 0 reads every candidate again and writes 0	red	green
Resume after a stop	TestResolvedPathRouteRepair_ResumesAfterStop_289 — cancelled mid-pass: context.Canceled is returned, the committed watermark is below the ceiling, the first row is repaired and a later one is not, and the resume finishes	red	green
Resolve outside the writer transaction, low hold	TestResolvedPathRouteRepair_ResolvesOutsideWriteTransaction_289 — 0 resolutions under writerMu, one WriterTx per batch	red	green
Concurrent live write wins	TestResolvedPathRouteRepair_DoesNotOverwriteAConcurrentWrite_289 — a row rewritten between read and write keeps its new value, 0 repaired/cleared	red	green
Readiness gate	TestResolvedPathRouteRepair_WaitsForIndexAndGraph_289 — on a cold index the pass writes nothing and the watermark stays 0; it completes once the graph is published	red	green
A batch that sees a cold snapshot after the gate is retried	TestResolvedPathRouteRepair_RetriesABatchThatSeesAColdSnapshot_289 — the batch's readiness check is a seam; the first batch reports not-ready, the loop retries the same watermark and finishes	n/a (new seam)	green
Opt-in, off by default	TestResolvedPathRouteRepairEnabled_OptIn_289 (nil config, no block, empty block, enabled: false → off; enabled: true → on) and TestResolvedPathRouteRepair_NotScheduledWhenDisabled_289 (nothing in _async_migrations, no state row, the defect row untouched; then the same row is repaired once enabled)	n/a (new switch)	green
Runs once, as an async migration	TestResolvedPathRouteRepair_RunsOnceAsAnAsyncMigration_289 — status done, and a later start does not re-run it	red	green
Sizing script is read-only and counts correctly	scripts/test-resolved-path-route-repair-estimate.sh — 21 checks: the split by header family (each direction holding both NULL and stored rows, so a NULL/non-NULL split cannot produce the numbers), the transport route types, the unparseable header, no -wal/-shm/-journal created, database byte-identical, recorded run reported, pre-Kpa-clawbot#881 degradation, argument handling	n/a	green

Mutants (all killed)

Mutant	Killed by
the pass writes nothing (master)	6 tests
re-resolves with t.route_type instead of the observation's route type	6 tests
the stored == had guard is gone (drifted live-ingest rows rewritten)	LeavesDriftedLiveIngestRowsUntouched, RepairsBothDirections, Idempotent, ResumesAfterStop
the prefilter's family bit is & 1 instead of & 2	RepairsBothDirections (the route-0 and route-3 rows)
the opt-in switch defaults to on	OptIn, NotScheduledWhenDisabled
a not-ready batch returns an error instead of being retried	RetriesABatchThatSeesAColdSnapshot
the script splits the candidates by NULL/stored instead of by header family	test-resolved-path-route-repair-estimate.sh
the benchmark mix has no row that gains a path	BenchmarkResolvedPathRouteRepairPass_289 (its both-directions assertion)
only non-NULL rows are candidates	RepairsNullRowBelowBackfillWatermark + 4
one resolve runs inside the write transaction	ResolvesOutsideWriteTransaction
the UPDATE is not guarded on the value the batch read	DoesNotOverwriteAConcurrentWrite
the route-type-independent guard is gone (correct rows rewritten)	RepairsBothDirections, Idempotent, ResumesAfterStop
no readiness gate (batches run against a cold index)	WaitsForIndexAndGraph
a window truncated by the LIMIT advances to the window end anyway (rows skipped)	RepairsBothDirections, Idempotent
a cancel ends the pass normally, so the migration is recorded done	ResumesAfterStop

One mutant was found equivalent and replaced: break instead of return err on the loop's ctx.Err() check — the select at the bottom of the same iteration still returns ctx.Err(), so the behaviour is identical. The version that mutates that select is the one in the table.

Rollback

The migration overwrites data, so this is the part to read before running it anywhere. It is off by default (resolvedPathRouteRepair.enabled), so a deployment that does nothing runs nothing.

What can be undone by code: setting resolvedPathRouteRepair.enabled back to false, or reverting this PR, stops any further repair. The _async_migrations marker and resolved_path_route_repair_state are inert once the code is gone; no down-migration is needed and no schema change to an existing table has to be reversed.

What cannot be undone: every row the pass wrote. The previous value is kept nowhere, and a database snapshot taken before the run is the only way back to it — so take one first. That applies to both directions, including the rows set back to NULL. The earlier text claimed a NULL row would be re-resolved by the #188 backfill and so was not a permanent loss; that was wrong (round 2, F2). The #188 backfill never revisits a row below its watermark — its own doc comment says so — and every row this pass clears sits below the pinned ceiling and normally below that watermark. A clear is permanent without a snapshot.

Mitigations that make the overwrite narrow: only rows where the route type demonstrably changes the resolve and whose stored value is exactly what the pre-#308 rule produces are touched, so a correct live-ingest value that has drifted from today's answer is read and left alone; the COALESCE(resolved_path,'') = <value read> guard means a row live ingest wrote in the meantime is never clobbered; resolved_path_route_repair_state records exactly how many rows were repaired and how many cleared, so the blast radius is auditable after the fact, and the sizing script reports it.

Re-arming after a rollback: a recorded migration is never re-run. If an ingestor is rolled back to a pre-#308 build, it writes new wrong rows that this pass will not revisit; clearing the _async_migrations row (or bumping the migration name) re-arms it. Unlike the #89 backfill, there is no cheap boot-time check for new candidates — that would need a full scan at every start — so this is deliberately manual and documented rather than automatic.

Server visibility is restart-only. The server indexes an observation's resolved_path when it first polls or loads the row, and a later rewrite reaches its indexes on the next Load. A change log like route_mask_changes is only worth adding if a production count shows the repaired volume is large. neighbor_edges is not repaired here: false DIRECT-derived observer edges age out with the 5-day prune unless refreshed, and dropped edges come back with new flood traffic — the same approach #190 took.

Residuals (outside this PR)

• A row whose stored value is both a correct live-ingest value and exactly what the pre-fix(ingestor): use each observation's own route type in backfill and neighbor builder (#289 M1) #308 rule produces today is rewritten. Nothing in the database distinguishes the two writers, so the pass cannot tell them apart. It is also the case where the rewrite is least harmful — the stored value is the one the buggy rule gives — but it is not free, and it is the reason the rollback section asks for a snapshot. Accepted residual (round 2, F1).
• The candidate counts the sizing script prints cannot tell a defect from a drifted live-ingest row, because the script does not resolve. A resolve-aware count (stored == had versus stored != had) on a snapshot copy is the honest pre-flight number, and is what should be run before this is enabled anywhere. Not done here; no staging or production access.
• neighbor_edges is not repaired, as above.
• Pre-schema: store per-observation raw_hex (each observer sees different bytes on air) Kpa-clawbot/CoreScope#881 rows with a single-family route_mask keep today's behaviour: the mask is a lower bound, and a variant whose frame no longer exists cannot be detected.
• The shared upsert row — one observer hearing DIRECT and FLOOD with an identical path shares one observation row and the last frame wins — is unchanged; fixing it means changing the upsert key.
• A stale resolved_path from graph drift (not a Plan: per-observation route type — backfill and neighbor-builder use the transmission's first route type #289 defect) is deliberately left alone.
• No customizer entry for resolvedPathRouteRepair.enabled or the three bounds; AGENTS.md rule 8, a follow-up.
• test-e2e-1267-mobile-vcr.js fails locally on the bug(live): VCR controls disappear once node markers fully populate on the map Kpa-clawbot/CoreScope#1267 bottom-nav overlap assertion. It is unrelated to this PR (git diff origin/master -- public cmd/server is empty) and is a macOS-vs-CI rendering difference; the suite also hardcodes /usr/bin/chromium and needs CHROMIUM_PATH to run here at all.

🤖 Generated with Claude Code

[adminopenclaw8-sketch]

Rapport — CS-Macmini PR#590 #289 — head 7f7126d

Status: #289 M2 implemented as the async ingestor migration resolved_path_route_type_repair_v1; 9 Go tests + 1 shell suite, all red before and green after, 10/10 mutants killed, sized on a synthetic 1.2M-observation database, every CI job green.

Evidence tags: [T] measured or tested here, [A] analysis or inference, [K] read in code, firmware source or a measurement someone else recorded.

Requirements

#	Requirement	Test (red before → green after)	Mutant	Evidence
1	Async ingestor migration (RunAsyncMigration, writerMu, bounded batches, the #89 pattern)	TestResolvedPathRouteRepair_RunsOnceAsAnAsyncMigration_289 — status done, repaired row, a later start does not re-run	the pass writes nothing (the stub that stands for master) → killed by 6 tests	[T]
2	Re-resolve with each observation's own route type from o.raw_hex, with the route_mask fallback as in #308	TestResolvedPathRouteRepair_RepairsBothDirections_289 — the route type comes from observationRouteType(header, txRoute, mask); one case has no frame and a mixed mask, so the fallback is exercised	re-resolve with t.route_type instead → killed by 6 tests	[T]
3	Both bug directions repaired: DIRECT treated as FLOOD, and the reverse	same test — DIRECT frame under a FLOOD tx: the stored relay is cleared; FLOOD frame under a DIRECT tx: the NULL row resolves to the observer-anchored relay	a window truncated by the LIMIT advances to the window end anyway (rows skipped) → killed by RepairsBothDirections, Idempotent	[T]
4	Candidates include NULL rows below the #188 backfill watermark	TestResolvedPathRouteRepair_RepairsNullRowBelowBackfillWatermark_289 — the watermark is set past the row, RunResolvedPathBackfill scans 0 rows and leaves it NULL, the repair (which scans from id 0) fixes it	only non-NULL rows are candidates → killed by that test + 4 more	[T]
5	No change to correct rows	RepairsBothDirections_289 — FLOOD/FLOOD, DIRECT/DIRECT, no frame + mixed mask + DIRECT tx, flood-only mask and NULL mask are all unchanged; a candidate whose answer is route-type-independent keeps its stale stored value	the route-type-independent guard removed → killed by RepairsBothDirections, Idempotent, ResumesAfterStop	[T]
6	Idempotent	TestResolvedPathRouteRepair_Idempotent_289 — a second pass is a no-op; a rescan from watermark 0 reads every candidate again and writes 0 rows	the already-correct check removed (a repeat rewrites every candidate) → killed by Idempotent	[T]
7	Resumable (progress marker)	TestResolvedPathRouteRepair_ResumesAfterStop_289 — cancelled mid-pass: context.Canceled is returned, the committed watermark is below the pinned ceiling, the first row is repaired and a later one is not, and the resume finishes the rest	a cancel ends the pass normally, so the migration is recorded done and never resumes → killed by ResumesAfterStop	[T]
8	Resolve outside the writer transaction (#267), low writer hold	TestResolvedPathRouteRepair_ResolvesOutsideWriteTransaction_289 — 0 of N resolutions ran while writerMu was held, one WriterTx per batch	one resolve moved inside the write transaction → killed by that test	[T]
9	Bounded per batch	idWindow 20,000 ids scanned and batchSize 500 candidates read per batch; the window bound is what keeps a sparse window from becoming one unbounded range scan on the single SQLite connection. Pinned by the batch count in requirement 8's test and by the benchmark's 60 batches over 1.2M rows	see requirement 3's mutant	[T]
10	Overwrite is safe against a concurrent live write	TestResolvedPathRouteRepair_DoesNotOverwriteAConcurrentWrite_289 — a row rewritten between the batch's read and its write keeps the new value; 0 repaired, 0 cleared	the COALESCE(resolved_path,'') = <value read> guard removed → killed by that test	[T]
11	Never resolve against a cold index or graph	TestResolvedPathRouteRepair_WaitsForIndexAndGraph_289 — on a cold index the pass writes nothing and the watermark stays 0; it completes once the post-build graph is published	the readiness gate removed → killed by that test	[T]
12	Sized first on a synthetic DB (1M+ observations), run time estimated at ~6.5M	BenchmarkResolvedPathRouteRepairPass_289 and BenchmarkResolvedPathRouteRepairBatch_289; numbers below	n/a (benchmark)	[T]
13	Read-only sizing for a production-like copy	scripts/resolved-path-route-repair-estimate.sh + scripts/test-resolved-path-route-repair-estimate.sh — 18 checks: candidate split by direction, no -wal/-shm/-journal created, database byte-identical, recorded run reported, pre-Kpa-clawbot#881 degradation, argument handling	n/a (asserted directly: a mutation that wrote would fail the byte-identical check)	[T]
14	All writes in cmd/ingestor (Kpa-clawbot#1283)	cmd/server is untouched: git diff origin/master -- cmd/server public is empty	n/a	[T]
15	Rollback plan documented in the PR	PR body, "Rollback" section: what a revert undoes, what the overwrite cannot undo, the mitigations that keep it narrow, and how to re-arm after a rollback to a pre-#308 build	n/a	[A]

Sizing

Synthetic database built by the benchmark: 1,200,000 observations over 400,000 transmissions (3 receptions each), every third transmission carrying both route families in route_mask, 1 observation in 100 a #289 candidate (4,002 in all). 325 MB file, Apple M4, 10 cores, warm page cache. [T]

Metric	Value
full pass, pause disabled	498 ms (415 ns per observation), 60 batches
candidates found / repaired	4,002 / 4,002 (all cleared — direction 1)
writer hold per batch	p50 0.5 ms, p95 14.4 ms, p99 17.0 ms, max 20.1 ms
one 20,000-id window	18.1 ms

p50 is the windows with no candidate (state write only); p95/p99 are the windows that rewrite rows. All well under the 500 ms slow-writer threshold, and under the #89 backfill's recorded p99 of 161 ms [K].

Extrapolated to ~6.5M observations (the count in the #89 sizing comment [K]): ~3 s of CPU plus 325 windows × 250 ms ≈ 81 s of deliberate pause, so about 1.5 min of wall clock warm [A]. Two caveats, both in the PR body: production at that size is a multi-GB file where the pass is cold-cache I/O bound (#89 measured 18–19.5 s cold for one full scan of transmissions on a 4.95 GB copy [K]), and the candidate rate here is synthetic. The sizing script gives the real counts on a snapshot copy. Nothing was run against staging or production.

Local verification

Check	Result
go test ./... in cmd/ingestor	ok, 110.7 s [T]
go test -race on the _289 tests	ok, 4.4 s [T]
go build + go test ./... in cmd/server, cmd/migrate, internal/dbschema, internal/packetpath	ok [T]
go vet in cmd/ingestor	clean [T]
sh test-all.sh	271 passed, 0 failed [T]
node test-frontend-helpers.js	725 passed, 0 failed [T]
node test-e2e-playwright.js (local server on a free high port, fixture prepared as in CI)	154/157 passed, 3 skipped [T]
node test-e2e-532-route-hop-count.js, test-e2e-563-route-titles.js	pass [T]
node test-e2e-1267-mobile-vcr.js	1 assertion fails locally, see Residuals [T]
scripts/check-xss-sinks.sh --diff origin/master	"no public/**/*.{js,html} changes to scan" [T]
bash scripts/test-resolved-path-route-repair-estimate.sh	18/18 checks passed [T]
new map[string]interface{} outside tests	none [T]
hardcoded colours	none (no frontend change) [T]
fork guards	9 in deploy.yml, 1 in release-fast-path.yml, both unchanged [T]

CI per job

All 15 jobs green; 3 skipped by their own guards (release/fork). Run 40178051032. [T]

Job	Result
Go Ingestor Tests	pass
Go Server Tests (1/3), (2/3), (3/3)	pass
Go Libraries & CLI	pass
Go Build & Test	pass
Verify Go Test Assignments	pass
JavaScript Unit Tests & Lint	pass
Script & Container Unit Checks	pass (runs the new test-resolved-path-route-repair-estimate.sh and its ShellCheck)
Playwright E2E Tests (1/3), (2/3)	pass
Playwright E2E Tests (3/3)	fail, then pass on one re-run (see below)
Verify E2E Shards & Coverage	fail, then pass on the same re-run (it aggregates shard 3)
Validate Docker Image	pass
Build & Publish Docker Image	pass
Release Artifacts and 2 other release/fork-gated jobs	skipping

The one re-run, as noted. Playwright shard 3 failed on test-issue-315-estimated-positions-e2e.js with a Chromium crash at browser launch, before the suite ran an assertion: Received signal 11 SI_KERNEL, Possibly a General Protection Fault, <process did exit: exitCode=null, signal=SIGSEGV> — browserType.launch: Target page, context or browser has been closed. That is the runner's browser segfaulting, not a product failure, and this PR touches no frontend or server code (git diff origin/master -- public cmd/server is empty). Re-run once per the policy for the known-unstable suites; shard 3 and the aggregate job both passed. Not the #271 flake and not a new one worth filing on this evidence — a launch-time SIGSEGV is runner infrastructure. [T]

Mutants

10 applied one at a time against the committed implementation, each reverted before the next; all killed. [T]

Mutant	Killed by
the pass writes nothing (stands for master)	6 tests
re-resolves with t.route_type	6 tests
only non-NULL rows are candidates	RepairsNullRowBelowBackfillWatermark + 4
one resolve inside the write transaction	ResolvesOutsideWriteTransaction
the UPDATE is not guarded on the value the batch read	DoesNotOverwriteAConcurrentWrite
the already-correct check removed	Idempotent
the route-type-independent guard removed	RepairsBothDirections, Idempotent, ResumesAfterStop
the readiness gate removed	WaitsForIndexAndGraph
a truncated window advances to the window end anyway	RepairsBothDirections, Idempotent
a cancel ends the pass normally	ResumesAfterStop

One further mutant was found equivalent and replaced: break instead of return err on the loop's ctx.Err() check — the select at the bottom of the same iteration still returns ctx.Err(), so the behaviour is identical. The version that mutates that select is the one in the table.

Residuals

• neighbor_edges is not repaired. False DIRECT-derived observer edges age out with the 5-day prune unless refreshed, and dropped edges come back with new flood traffic — the same approach feat(ingestor): resolve the last hop from the observer (#188) #190 took. [K/A]
• Server visibility is restart-only. A rewritten resolved_path reaches the server's indexes at its next Load. A change log like route_mask_changes is only worth adding if a production count shows the repaired volume is large. Not filed. [A]
• Re-arming after a rollback is manual. A recorded migration is never re-run, so an ingestor rolled back to a pre-fix(ingestor): use each observation's own route type in backfill and neighbor builder (#289 M1) #308 build writes new wrong rows that this pass will not revisit; the _async_migrations row has to be cleared, or the migration name bumped. Unlike the Same advert seen on flood and zero-hop routes keeps the route of the first inserted observation #89 backfill there is no cheap boot-time candidate check (it would need a full scan at every start), so this is deliberate and documented rather than automatic. Not filed. [A]
• A stale resolved_path from graph drift (not a Plan: per-observation route type — backfill and neighbor-builder use the transmission's first route type #289 defect) is deliberately left alone: the pass writes only where the route type changes the answer. [T]
• Pre-schema: store per-observation raw_hex (each observer sees different bytes on air) Kpa-clawbot/CoreScope#881 rows with a single-family route_mask keep today's behaviour — the mask is a lower bound, and a variant whose frame no longer exists cannot be detected. Unchanged from the plan's residuals. [K]
• The shared upsert row (one observer hearing DIRECT and FLOOD with an identical path shares one observation row, last frame wins) is unchanged; fixing it means changing the upsert key. Unchanged from the plan's residuals. [K]
• test-e2e-1267-mobile-vcr.js fails locally on its bottom-nav overlap assertion. Unrelated to this PR — git diff origin/master -- public cmd/server is empty — and a local rendering difference: the suite hardcodes /usr/bin/chromium and needs CHROMIUM_PATH to start at all on this host. Green in the CI Playwright shards. [T]
• Three bounds are hardcoded (idWindow 20,000, batchSize 500, pause 250 ms) as package vars, no config block. Customizer candidates later, AGENTS.md rule 8, the same treatment the Same advert seen on flood and zero-hop routes keeps the route of the first inserted observation #89 backfill's bounds got. Not filed. [A]

[dborup-agent]

Review — CS-MacBook PR#590 — head 7f7126d

Dom: REQUEST CHANGES

Reviewed head 7f7126da62ee2e82dc5625eca9d6375ce1fdb9a2 (unchanged before and after the review, checked with git ls-remote). Tests ran on the merged tree against origin/master 88f3be29 (git merge-tree --write-tree: clean merge, tree fe6b1ee6).

Evidence tags: [T] tested or measured here, [A] analysis or inference, [K] read in code or in the PR/issue.

The mechanics are solid: bounded windows, resolve outside the transaction, a guarded UPDATE, a resumable watermark, a readiness gate, and writer-hold numbers I reproduced. Two things block it. The pass overwrites or clears correct rows that live ingest wrote, and the rollback section says something about cleared rows that is not true. Both fixes are small.

Findings

#	Severity	Finding	Evidence
F1	High	The pass rewrites correct live-ingest rows when the graph or index has drifted. Most family-mismatch rows were written by live ingest, which used the frame's own route type, not by the pre-#308 backfill. The write rule want != had && stored != want never checks that the stored value is what the buggy rule would produce. So a correct row whose stored value differs from today's want gets rewritten, or cleared when today's want is empty. Repro on the #188 fixture: (a) a DIRECT frame under a FLOOD tx, path ["c3"], stored [c3b] (live ingest when c3b was the only c3). Today DIRECT gives nothing and FLOOD gives c3a, so the row is cleared to NULL. (b) A FLOOD frame under a DIRECT tx, stored [c3b], is overwritten with [c3a]. The pass reported 1 repaired, 1 cleared on those two rows. This breaks "correct rows are left untouched", and the PR's claim that want != had is "the protection against graph drift clobbering a correct row" only holds for route-type-independent answers. Suggested fix (tested): also require r.stored == had, i.e. if want == had || r.stored == want || r.stored != had { continue }. Then the pass only writes rows whose stored value is exactly what the pre-#308 rule produces today. All 9 PR _289 tests stay green with it, and the two probe rows are left alone. The residual case is stored == had == a value live ingest once wrote correctly. The pass cannot tell that apart from a defect, so it should be documented as an accepted residual.	[T] probe test, [A]
F2	Medium	The rollback section is wrong about cleared rows. It says: "Rows set back to NULL are candidates for the #188 backfill, which will re-resolve them … so a NULL is not a permanent loss of information." The #188 backfill never revisits rows below its watermark. Its own doc comment says so (resolved_path_backfill.go: "Rows that still do not resolve stay NULL and are not retried in a later pass (the watermark has passed them)"), and so does the PR body's Problem section. Every row this pass clears sits below the ceiling and normally below the backfill watermark, so a clear is permanent without a snapshot. Re-resolving would give want anyway, which is empty. Combined with F1, a clear can permanently remove a correct value. Please correct the text.	[K]
F3	Medium (owner decision)	No opt-out: the irreversible rewrite runs automatically on the first boot of any build that contains it. The #188 backfill sits behind cfg.ResolvedPathBackfillSettings(), but the repair in main.go is unconditional. The rollback plan says "take a snapshot first", so every deployment, forks included, has to snapshot before upgrading, and the only defence is the release notes. Consider a config switch, which would also cover AGENTS.md rule 8, or at least a release-note item that cannot be missed.	[K], [A]
F4	Low	Test gap: transport route types. My mutant M2, which changes the prefilter's family bit from & 2 to & 1, survives. The fixture only uses route types 1 and 2, which differ in both bits. With & 1, a TRANSPORT_DIRECT (3) frame under a FLOOD (1) tx, or TRANSPORT_FLOOD (0) under DIRECT (2), would silently drop out of the candidates. Add a case with route types 0 or 3.	[T]
F5	Low	Test gap: the not-ready retry inside the loop. Mutant M5 (errResolvedPathBackfillNotReady from a batch returns an error instead of continue) survives. The race between the gate and the batch is untested. That is acceptable if it is documented, or covered with a seam that flips readiness between the gate and the batch.	[T]
F6	Nit	The sizing script labels the header-mismatch split as "NULL = FLOOD read as DIRECT" and "stored = DIRECT read as FLOOD; may carry a wrong relay". That is a NULL/non-NULL split, not a direction split. A DIRECT-under-FLOOD row can be NULL, and per F1 most stored rows are probably correct live-ingest values. Splitting by header family first, then NULL/stored, would match the PR's claim of a "split by direction".	[K]
F7	Nit	Every benchmark candidate is direction 1 (all cleared), so the measured p95/p99 hold covers no row that gains a path. That should not change the order of magnitude.	[K]

The points in the brief

Point	Result
Candidates include NULL rows below the backfill watermark	Yes. The scan runs from id 0 to a pinned ceiling and has no resolved_path IS NOT NULL filter. My mutant M1 (adding that filter) is killed by 5 tests, including RepairsNullRowBelowBackfillWatermark. [T]
Both bug directions repaired	Yes for the defect rows: DIRECT-under-FLOOD wrong relay cleared, FLOOD-under-DIRECT NULL resolved, and no frame + mixed mask + FLOOD tx cleared. [T] But see F1: rows outside the defect are rewritten too.
Resumable / idempotent, bounded batches	Yes. The watermark, counters and row rewrites commit in one tx, and the ceiling is persisted. My mutant M4 (re-pin the ceiling/watermark on every start) is killed by Idempotent. The id window (20,000) and LIMIT (500) bound each batch. A cancel is recorded as unfinished, and RunAsyncMigration re-runs failed/pending_async. [T]/[K]
Low writer hold (measurement)	Reproduced with BenchmarkResolvedPathRouteRepairPass_289 (1.2M obs, Apple M-series, warm): 472 ms pass, writer hold p50 0.7 / p95 17.8 / p99 19.4 / max 19.4 ms over 61 transactions. One window takes 13.7 ms. This matches the PR's numbers. Hold is measured around WriterTx, so it includes any wait for writerMu, which makes it an upper bound. [T]
Resolve outside the writer transaction (#267)	Yes. Both resolves run before WriterTx. ResolvesOutsideWriteTransaction_289 passes, including under -race. [T]/[K]
Correct rows untouched	No, see F1. Rows the frame and tx agree on are never read, and route-type-independent rows are left alone [T]. But drifted live-ingest rows are rewritten or cleared [T].
All writes in cmd/ingestor (Kpa-clawbot#1283)	Yes. git diff origin/master...head -- cmd/server public is empty. [T]
Rollback plan honest about what cannot be undone	Partly. It correctly says the rewritten values are not kept and that a snapshot is the only way back. But the claim that NULL rows get re-resolved is false (F2), and the auto-run on deploy is not called out (F3). [K]
-race	go test -race -run _289 is green (6.3 s). [T]
CI, Playwright shard 3 failure on the first run	Unrelated. Attempt 1, job "Playwright E2E Tests (3/3)": test-issue-315-estimated-positions-e2e.js died at browserType.launch with Chromium Received signal 11 SI_KERNEL … General Protection Fault, signal=SIGSEGV, before any assertion ran. Every earlier suite in the shard passed. "Verify E2E Shards & Coverage" and "Build & Publish Docker Image" failed in attempt 1 only because they aggregate shard 3. Attempt 2: all 15 jobs green, 3 skipped by their release/fork guards. The PR touches no frontend or server code, and the same suite passes locally (below). It is neither #256 nor #267. [T]

Standing checks

Check	Result
Every acceptance criterion has a test that is red before and green after	Yes for the listed criteria. My "pass writes nothing" mutant (M9, i.e. master behaviour) fails 6 of the 9 tests. The other 3 are guards that are green before and after by design. F1 lacks a test because the behaviour is wrong today. [T]
No behaviour change beyond the purpose	No, see F1: correct rows are rewritten outside the #289 defect. [T]
cmd/server read-only, no new map[string]interface{} outside tests	Yes: 0 added. [T]
No hardcoded colours	Yes: 0. [T]
scripts/check-xss-sinks.sh --diff origin/master	Clean: "no public/**/*.{js,html} changes to scan", exit 0. Run in an isolated clone at head. [T]
Fork guards	9 in deploy.yml, 1 in release-fast-path.yml. [T]
Closing keywords	None in the PR body or commit ("Relates to #289"). [T]
Commit author	dborup <kontakt@meshview.dk> (author and committer). [T]

Tests run (merged tree, origin/master 88f3be2)

Suite	Result
cmd/ingestor: go vet ./... + go test -count=1 ./...	ok, 116.7 s [T]
cmd/ingestor: go test -race -run _289	ok [T]
cmd/server go test ./...	ok, 74.4 s [T]
cmd/migrate, internal/dbschema, internal/packetpath	ok [T]
sh test-all.sh	272 passed, 0 failed [T]
node test-frontend-helpers.js	725 passed, 0 failed [T]
bash scripts/test-resolved-path-route-repair-estimate.sh + ShellCheck on both scripts	all checks passed, ShellCheck clean [T]
E2E against a local Go server on a free high port, with e2e-fixture.db prepared as in CI (freshen, Kpa-clawbot#1486 seed SQL, corescope-migrate, seeds 2073, 199, 245, 122)	test-e2e-playwright.js 154/157 passed, 3 skipped; test-issue-165-grouped-hop-warn-e2e.js 12/12; test-e2e-532-route-hop-count.js pass; test-issue-315-estimated-positions-e2e.js (the CI crash suite, own server/migrate binaries and local Leaflet assets) PASS for default/enabled/disabled [T]
Server stopped by the pid of the port listener, after checking its command and cwd	done [T]

The PR has no E2E of its own, and no UI or server change, so the E2E runs above are regression checks only.

Mutants (my own, each applied alone to the PR code and reverted)

#	Mutant	Result
M1	prefilter adds o.resolved_path IS NOT NULL	killed: RepairsBothDirections, RepairsNullRowBelowBackfillWatermark, Idempotent, ResumesAfterStop, WaitsForIndexAndGraph
M2	prefilter family bit & 1 instead of & 2	survives (F4)
M3	no-frame + mixed-mask branch disabled (0 = 1 AND …)	killed: RepairsBothDirections, Idempotent, ResumesAfterStop
M4	persisted state ignored, ceiling/watermark re-pinned on every start (INSERT OR REPLACE)	killed: Idempotent
M5	a not-ready batch returns an error instead of retrying	survives (F5)
M9	the pass writes no rows (updates = nil, i.e. master behaviour)	killed: 6 tests

Probe for F1, as a reviewer-only test file not pushed: TestReviewProbe_DriftedLiveIngestRowUntouched_289 on backfillFixture188, two rows as described in F1. It fails on head (one row cleared, one overwritten, repaired=1 cleared=1) and passes with the suggested r.stored != had guard. With that guard, all _289 tests in the package still pass. [T]

Not verified

• The real-data impact of F1. I ran the pass against a copy of the raw test-fixtures/e2e-fixture.db (500 observations). It has 0 candidates, so it says nothing about how many rows on a real database are drifted live-ingest rows versus genuine pre-fix(ingestor): use each observation's own route type in backfill and neighbor builder (#289 M1) #308 defects. The sizing script cannot tell them apart either, because it does not resolve. Before this runs anywhere, run a resolve-aware count on a snapshot copy: candidates where stored == had versus stored != had. Not done here; no staging or production access. [A]
• I did not run the full ingestor suite with the suggested F1 guard applied, only the _289 tests and the probe.
• On macOS the Leaflet asset sha256 checks in prepare-e2e-leaflet.sh did not run (the BSD sha256sum rejects the --check usage). The files were downloaded and the Feature request: allow operators to disable estimated node positions via configuration #315 suite passed with them.
• Cold-cache I/O cost on a multi-GB database: not measured. The PR's extrapolation is warm-cache only, as the PR itself says. [K]
• Firmware: no protocol claim beyond the route-type bit layout (0/1 flood, 2/3 direct) was re-checked against the MeshCore source in this review. [K]

[adminopenclaw8-sketch]

Rapport — CS-Macmini PR#590 runde-2 — head 69e71c0

Review feedback addressed (commit 69e71c09e28e65490708d72c5083adfe1301b2c9). Merged origin/master 88f3be29 in as 2ee99401 first; the round-2 fixes are one commit on top.

Evidence tags: [T] measured or tested here, [A] analysis or inference, [K] read in code, in the PR/issue or in a measurement someone else recorded.

Findings

1. F1 (High) — the pass rewrote correct live-ingest rows after graph drift. Fixed.

The write rule now requires the stored value to be exactly what the pre-#308 rule produces today, not merely that it differs from the new answer:

had := marshalResolvedPath(resolvedPathRouteRepairResolve(hops, from, r.observerID, r.txRoute, graph, idx))
if want == had || r.stored != had {
    continue
}

r.stored == had is the signature of a row that rule wrote. A family mismatch on its own is not: live ingest has always resolved with the frame's own route type, so most mismatched rows hold a correct ingest value, which differs from today's had as soon as the neighbour graph or the prefix index has drifted. Those rows are read, counted as candidates and left alone. The old second condition (r.stored != want, "already repaired") is subsumed: if stored == want and want != had then stored != had.

The reviewer's repro is now TestResolvedPathRouteRepair_LeavesDriftedLiveIngestRowsUntouched_289 — the two rows described in F1 on backfillFixture188, a DIRECT and a FLOOD reception whose stored relay is c3b where today's had is c3a/empty. On the pre-fix code it reported exactly what the review did: 1 repaired, 1 cleared, one row cleared to NULL and one overwritten with c3a. After the fix: 0 repaired, 0 cleared, 2 candidates read, both rows byte-identical. [T] The same two rows are also in the shared 14-row case table, so RepairsBothDirections, Idempotent and ResumesAfterStop all cover them.

The residual the review named is documented as accepted, in the code and in the PR body: a row whose stored value is both a correct ingest value and exactly what the buggy rule produces today is still rewritten. Nothing in the database distinguishes the two writers. [A]

2. F2 (Medium) — the rollback section was wrong about cleared rows. Corrected.

The claim that a NULL row would be re-resolved by the #188 backfill is gone. The section now says that every row the pass writes, clears included, is irreversible without a snapshot, and says why: the #188 backfill never revisits a row below its watermark (its own doc comment in resolved_path_backfill.go says so [K]), and every row this pass clears sits below the pinned ceiling and normally below that watermark. The correction is called out as such rather than quietly swapped. The same warning is in config.example.json and in docs/user-guide/configuration.md.

3. F3 (Medium, owner decision) — the repair is now opt-in, default off.

New config block, the shape the review asked for:

"resolvedPathRouteRepair": { "enabled": false }

cfg.ResolvedPathRouteRepairEnabled() is false for a nil config, an absent block, an empty block and enabled: false. main.go passes it to StartResolvedPathRouteRepair(ctx, enabled), which logs one line and returns when off — nothing is scheduled and nothing is recorded in _async_migrations, so turning the switch on later still runs the pass. Documented in config.example.json (two comments: what it does, and what a rollback does not undo) and in a new docs/user-guide/configuration.md section, both of which say take a database snapshot first and point at the sizing script. Customizer entries for the switch and the three bounds are noted as a follow-up (AGENTS.md rule 8), not filed. [T]/[K]

4. F4 (Low) — transport route types tested; the reviewer's M2 now dies.

Two cases added to the shared table: a TRANSPORT_DIRECT frame (0x17, route 3) under a FLOOD transmission, and a TRANSPORT_FLOOD frame (0x14, route 0) under a DIRECT transmission. Both are repaired by the fixed pass, and both become invisible to the prefilter when its family bit is & 1 instead of & 2, because 3 shares bit 0 with 1 and 0 shares it with 2. The reviewer's M2 is killed by RepairsBothDirections, Idempotent and ResumesAfterStop. [T]

5. F5 (Low) — the not-ready retry inside the loop is covered.

The batch's readiness re-check is now a package var, resolvedPathRouteRepairBatchReady, alongside the existing resolvedPathRouteRepairResolve seam. TestResolvedPathRouteRepair_RetriesABatchThatSeesAColdSnapshot_289 makes the first check report not-ready — the snapshot going cold after the loop's gate passed, which is the race the retry exists for — then lets the real check through: the loop retries the same watermark, the migration does not fail and the row is repaired. The reviewer's M5 (return err instead of continue) now fails that test. [T]

Honest limit: this test is new code exercising a new seam, so on head 7f7126da it is a compile error rather than a behavioural red. Its job is killing M5, and it does. The real race is still not won for real — simulating it is the alternative the review offered.

6. F6 (Nit) — the sizing script splits by direction, then by NULL/stored.

The header split is now by the frame's own route family, which is the bug direction, and each direction reports its stored and NULL counts inside it. Rows whose second header character is not hex are reported on their own line instead of landing in the DIRECT bucket (instr()-1 is -1, whose bit 1 is set), since the pass re-checks those in Go. It is one SUM(CASE …) query now rather than two counts, so the script does one scan where it did two. The comment says a stored value may well be a correct ingest value, per F1.

7. F7 (Nit) — the benchmark measures rows that gain a path.

Half the mixed transmissions now have a DIRECT first reception, so their candidates are NULL rows that gain a path. The 1.2M-row mix is 4,002 candidates: 1,334 cleared and 2,668 resolved (the 2:1 split is where the every-100th-row counter lands). The benchmark asserts both are non-zero and both counters match the seed. It did not change the order of magnitude — p99 write hold 13.3 ms against 17.0 ms in round 1. [T]

Sizing, re-measured with the new mix

Apple M4, 10 cores, warm page cache, 311 MB synthetic database (1,200,000 observations / 400,000 transmissions). [T]

Metric	Round 1	Round 2 (both directions)
full pass, pause disabled	498 ms (415 ns/obs), 60 batches	511 ms (426 ns/obs), 61 batches
candidates	4,002 (all cleared)	4,002 (1,334 cleared, 2,668 resolved)
writer hold per batch	p50 0.5 / p95 14.4 / p99 17.0 / max 20.1 ms	p50 0.8 / p95 12.7 / p99 13.3 / max 13.3 ms
one 20,000-id window	18.1 ms	19.7 ms

Extrapolation is unchanged in substance: 426 ns × 6.5M ≈ 3 s of CPU plus 325 windows × 250 ms ≈ 81 s of deliberate pause, warm cache, with the same two caveats in the PR body (production at that size is a multi-GB file where the pass is cold-cache I/O bound, and the candidate rate is synthetic). Nothing was run against staging or production. [T]/[A]

Tests

Suite	Result
cmd/ingestor: go vet ./... + go test -count=1 ./...	ok, 111.5 s [T]
cmd/ingestor: go test -race -count=1 -run _289	ok, 8.0 s [T]
cmd/server go build ./... + go test -count=1 ./...	ok, 71.7 s [T]
cmd/migrate, cmd/decrypt, internal/dbschema, internal/packetpath	ok [T]
sh test-all.sh	272 passed, 0 failed [T]
node test-frontend-helpers.js	725 passed, 0 failed [T]
bash scripts/test-resolved-path-route-repair-estimate.sh	21/21 checks passed (18 before) [T]
node test-e2e-playwright.js	154/157 passed, 3 skipped [T]
node test-e2e-532-route-hop-count.js, test-e2e-563-route-titles.js, test-issue-165-grouped-hop-warn-e2e.js	pass; 12/12 for #165 [T]
node test-issue-315-estimated-positions-e2e.js (the suite that crashed in round 1 CI)	PASS for default / enabled / disabled [T]
gofmt -l on every touched Go file	clean [T]
python3 -c 'json.load(open("config.example.json"))'	valid [T]

E2E ran against a local Go server on port 13800 with test-fixtures/e2e-fixture.db prepared as in CI (freshen, the Kpa-clawbot#1486 and Kpa-clawbot#1791 seed SQL, corescope-migrate, then seeds 2073, 199, 245 and 122), public served unminified, and a .git-commit file in place. The server was stopped by the pid of the port listener after checking its command and cwd; the fixture, the built binaries and the snapshot the server wrote were removed afterwards. The PR has no E2E of its own and changes no frontend or server code, so these are regression checks. [T]

Mutants

Seven new mutants for the seven findings, each applied alone to the committed code and reverted before the next. All killed. [T]

#	Mutant	Result
R1	the stored == had guard removed (back to round-1 behaviour)	killed: LeavesDriftedLiveIngestRowsUntouched (1 repaired / 1 cleared, the review's own numbers), RepairsBothDirections, Idempotent, ResumesAfterStop
R2	prefilter family bit & 1 instead of & 2 (the review's M2)	killed: RepairsBothDirections (the route-3 row keeps its relay, the route-0 row stays NULL), Idempotent, ResumesAfterStop
R3	ResolvedPathRouteRepairEnabled() defaults to on	killed: ResolvedPathRouteRepairEnabled_OptIn (nil config and no-block cases)
R4	the if !enabled gate removed from StartResolvedPathRouteRepair	killed: NotScheduledWhenDisabled (migration status done where none is wanted)
R5	a not-ready batch returns an error instead of being retried (the review's M5)	killed: RetriesABatchThatSeesAColdSnapshot
R6	the script splits the candidates by NULL/stored instead of by header family	killed: test-resolved-path-route-repair-estimate.sh, 2 of 21 checks
R7	the benchmark mix has no row that gains a path	killed: BenchmarkResolvedPathRouteRepairPass_289 ("the mix must measure both directions: 102 to clear, 0 to repair"). Run at a reduced 30,000-row size so the mutant is cheap; the mix logic is size-independent

One round-1 mutant changed meaning and is re-reported honestly: "the route-type-independent guard (want == had) removed" survives the round-1 test set once the stored == had guard is in, because a drifted route-type-independent row fails the new guard anyway. A case was added for it — a ["c355"] candidate whose stored value already is the answer, so want == had == stored — and the mutant is killed by RepairsBothDirections (3 repaired instead of 2, a pointless rewrite of a row that already holds its value) and by Idempotent. [T] The round-1 mutant "the already-correct check (stored != want) removed" is now subsumed by R1 and is not listed separately.

Standing checks

Check	Result
cmd/server and public untouched	git diff origin/master...HEAD -- cmd/server public is empty [T]
New map[string]interface{} outside tests	none [T]
Hardcoded colours	none (no frontend change) [T]
scripts/check-xss-sinks.sh --diff origin/master	"no public/**/*.{js,html} changes to scan" [T]
Fork guards	9 in deploy.yml, 1 in release-fast-path.yml, unchanged [T]
Closing keywords in the body or the commit	none ("Relates to #289") [T]
Commit author and committer	dborup <kontakt@meshview.dk> [T]
PR state	still a draft [T]

CI per job

Run 40184604864. Attempt 1: 12 pass, 3 fail, 3 skipped by their release/fork guards. Attempt 2 (the one re-run the policy allows, reasons below): all 15 jobs pass, 3 skipped. [T]

Job	Attempt 1	Attempt 2
Go Ingestor Tests	pass (4m18s)	—
Go Server Tests (1/3) / (2/3) / (3/3)	pass (6m3s / 6m24s / 19m10s)	—
Go Build & Test	pass	—
Go Libraries & CLI	pass	—
Verify Go Test Assignments	pass	—
JavaScript Unit Tests & Lint	pass	—
Script & Container Unit Checks	pass (runs the new shell suite and ShellCheck on both scripts)	—
Playwright E2E Tests (1/3) / (3/3)	pass (10m6s / 12m21s)	—
Playwright E2E Tests (2/3)	fail (1m39s)	pass (14m0s)
Verify E2E Shards & Coverage	fail (aggregates shard 2)	pass
Build & Publish Docker Image	fail (aggregates shard 2)	pass
Validate Docker Image	pass	—
Release Artifacts, Deploy Staging, Publish Badges	skipping (release/fork guards)	skipping

The shard-2 failure, and why it is not this PR. test-health-status-e2e.js timed out at line 130 — locator.getAttribute: Timeout 15000ms exceeded, waiting for locator('#homeHealth .health-banner').locator('.ph-icon use').first() — one of the #304 F2 banner assertions. That line and the public/home.js code behind it reached this branch through the origin/master merge (#586/#304, merged about an hour before the run); this PR adds no frontend or server code at all and git diff origin/master...HEAD -- cmd/server public is empty, so the merged tree's frontend is byte-identical to master's. Master's own CI at 88f3be29 was green (run 40179700488) [K], the suite passes locally 3 of 3 runs (173 checks each) against a local Go server with the fixture prepared as in CI [T], and it passed on the re-run [T]. A timing flake in a freshly changed suite — not #271 and not #301, the two the brief names, and not worth filing on this evidence. The other two failures were only the known aggregation coupling to shard 2, the same one round 1 saw.

Not fixed, and why

• The real-data impact of F1 is still unmeasured. The review's point stands: the sizing script cannot tell a genuine pre-fix(ingestor): use each observation's own route type in backfill and neighbor builder (#289 M1) #308 defect from a drifted live-ingest row, because it does not resolve. The new stored == had guard is what makes that distinction at run time, and the state table records how many rows it actually wrote — but the honest pre-flight number is still a resolve-aware count on a snapshot copy. Recorded as a residual in the PR body. No staging or production access was used. [A]
• The three bounds and the new switch have no customizer entries. AGENTS.md rule 8, a follow-up, as the review suggested. Not filed. [A]
• neighbor_edges is not repaired, pre-schema: store per-observation raw_hex (each observer sees different bytes on air) Kpa-clawbot/CoreScope#881 single-family route_mask rows and the shared upsert row are unchanged residuals from round 1. [K]
• ShellCheck was not run locally — it is not installed on this host and no container was started for it. The "Script & Container Unit Checks" job runs both scripts through it. [T]

[dborup-agent]

Review — CS-MacBook PR#590 — head 69e71c0

Dom: APPROVE with nits

This re-review covers round 2. Head 69e71c09e28e65490708d72c5083adfe1301b2c9 was unchanged before and after the review, checked with git ls-remote, and the PR is still a draft. Tests ran on the merged tree against origin/master f7e13bc3. git merge-tree --write-tree gave a clean merge, tree f4526f82. The head itself is based on 88f3be29. The merge note from the brief still applies: do not merge before 2026-11-09, because of the stg anomaly rounds.

Evidence tags: [T] tested or measured here, [A] analysis or inference, [K] read in code or in the PR/issue.

All seven round-1 findings are addressed. My F1 repro now leaves the drifted live-ingest rows alone, the rollback text is honest, and the pass is opt-in. What remains is non-blocking. The new switch is tested only from a struct literal downwards, so three mutants on the JSON/config path survive. There are also two documentation nits.

Findings

#	Severity	Finding	Evidence
N1	Low	The opt-in switch is not tested end to end. TestResolvedPathRouteRepairEnabled_OptIn_289 builds Config from struct literals, and NotScheduledWhenDisabled passes a bool straight to StartResolvedPathRouteRepair. Three of my mutants survive the full cmd/ingestor suite: (M5) a typo in the JSON tag resolvedPathRouteRepair, (M6) main.go passing true instead of cfg.ResolvedPathRouteRepairEnabled(), and (M7) config.example.json shipping "enabled": true. M5 fails safe, because the switch silently does nothing. M6 and M7 fail unsafe: the irreversible rewrite runs without opt-in. M7 matters because the repo already guards other features with "config.example.json is copied as a live config by some deployments" tests (neighbor_edge_decay_test.go:647, client_regions_test.go:642, mqtt_client_id_118_test.go:145). Suggested fix: a test that LoadConfigs config.example.json and asserts ResolvedPathRouteRepairEnabled() == false, plus one that loads {"resolvedPathRouteRepair":{"enabled":true}} and asserts true. That kills M5 and M7. M6 is main() wiring and is acceptable untested.	[T]
N2	Nit	Wrong link in docs/user-guide/configuration.md. The text "before #308" points at the upstream repo, where #308 is "Fix #304: unblock /api/decode and consolidate decoder", an unrelated item. The M1 change is #308 in this repo ("fix(ingestor): use each observation's own route type…", merged 2026-10-06). Use a bare #308 or this repo's PR URL.	[T] (gh lookups of both)
N3	Nit	The docs overstate what the guard protects. configuration.md says live-ingest values "are read and left alone", and the config.example.json _comment says "rows live ingest wrote are left alone", with no caveat. The PR body and the code comment correctly name the residual (stored == had). My residual probe shows it in two forms: a live-ingest NULL FLOOD-under-DIRECT row is filled, and a live-ingest DIRECT-under-FLOOD row whose value equals today's FLOOD answer is cleared (repaired=1 cleared=1). The fill is benign, since it uses the correct rule. The clear is the accepted residual. Suggest "…left alone unless the stored value is exactly what the pre-#308 rule produces today; see the snapshot warning".	[T] probe, [K]

The round-1 points

Point	Result
F1: only rows whose stored value equals the old buggy rule's output are touched; correct live-ingest rows are not changed under graph/index drift	Fixed. The rule is now if want == had || r.stored != had { continue } (resolved_path_route_repair.go:471). I re-ran my round-1 probe, TestReviewProbe_DriftedLiveIngestRowUntouched_289, extended from 2 to 6 drifted rows: plain and transport route types in both directions, no frame + mixed mask, and a live-ingest NULL DIRECT-under-FLOOD row. On head and on the merged tree the result is 0 repaired, 0 cleared, 6 candidates read, and every row is byte-identical [T]. The author's LeavesDriftedLiveIngestRowsUntouched_289 turns red with the round-1 rule (my M1) [T]. The residual (stored == had) behaves as documented in the PR body (see N3) [T].
F2: honest rollback text	Fixed. The false "NULL is re-resolved by #188" claim is gone and explicitly retracted. The text now says every write, clears included, is irreversible without a snapshot, and gives the correct reason: the #188 watermark never revisits those rows. The same text is in config.example.json and configuration.md. [K]
F3: config switch, default OFF, documented with "take a snapshot first"	Fixed in code and docs. resolvedPathRouteRepair.enabled is off by default. When it is off, nothing is scheduled or recorded in _async_migrations, so enabling it later still runs the pass. "Take a database snapshot first" is in both the example config and the user guide [K]. The default-off logic and the !enabled gate are covered by tests: my M8 is killed by NotScheduledWhenDisabled [T]. The JSON/example/wiring path is not covered (N1).
F4: transport route types; my M2 (& 2 → & 1)	Dies. My M3 (& 1 in both operands of the prefilter's first branch) is killed by RepairsBothDirections, Idempotent and ResumesAfterStop. [T]
F5: not-ready retry inside the loop; my M5	Dies. My M4 (continue → return err at line 250) is killed by RetriesABatchThatSeesAColdSnapshot_289. The test exercises a new seam rather than the real race, and the author says so. That is acceptable. [T]
F6: sizing script splits by header family	Fixed. It now uses one SUM(CASE…) scan, splits by frame family and then by stored/NULL, and reports unparseable headers separately. The comment notes that a stored value may be a correct ingest value. test-resolved-path-route-repair-estimate.sh: all checks pass, ShellCheck is clean on both scripts. [T]/[K]
F7: benchmark measures rows that gain a path	Fixed. The benchmark asserts both directions are non-zero and match the seed. My run (Apple M-series, warm): 1.2M obs, 473 ms, 4,002 candidates (1,334 cleared, 2,668 repaired), 61 batches, writer hold p50 0.9 / p95 14.3 / p99 14.5 / max 16.0 ms. One window takes 17.0 ms. This matches the PR's numbers. [T]
CI: Playwright failure	Unrelated flake. Run 40184604864, attempt 1: only "Playwright E2E Tests (2/3)" failed. test-health-status-e2e.js:130 hit locator.getAttribute: Timeout 15000ms on #homeHealth .health-banner .ph-icon use (#304 F2 banner assertions) after the preceding suite passed 17/17. "Verify E2E Shards & Coverage" and "Build & Publish Docker Image" failed only because they aggregate shard 2. In attempt 2 all 15 jobs pass; 3 are skipped by their release/fork guards. The merged tree's public/, cmd/server/ and that suite are byte-identical to master (git diff origin/master f4526f82 -- public cmd/server test-health-status-e2e.js is empty). The suite passed 3 of 3 runs locally against the merged tree, 173 checks each. It is neither #256 nor #267. It is a timing flake in a suite that changed this morning (#304), and it is worth watching if it recurs. [T]

Standing checks

Check	Result
Every acceptance criterion has a test that is red before and green after	Yes. The new F1 test is red under the round-1 rule (M1). The criteria from round 1 still go red on a "writes nothing" pass, per the author's table and my round-1 M9. The opt-in and retry tests are new seams ("n/a" before), which is acceptable. [T]/[K]
No behaviour change beyond the purpose	Yes. The pass is off by default, so a deployment that sets nothing behaves as master. cmd/server and public are untouched. [T]
cmd/server read-only; no new map[string]interface{} outside tests	Yes: cmd/server diff is empty, 0 added. [T]
No hardcoded colours	Yes: no CSS, JS or HTML is changed. [T]
scripts/check-xss-sinks.sh --diff origin/master	"no public/**/*.{js,html} changes to scan", exit 0 (isolated clone at head). [T]
Fork guards	9 in deploy.yml, 1 in release-fast-path.yml, on head and on the merged tree. [T]
Closing keywords	None in the body or the 3 commits ("Relates to #289"). [T]
Commit author	dborup <kontakt@meshview.dk>, author and committer, on all 3 commits. [T]

Tests run (merged tree, origin/master f7e13bc)

Suite	Result
cmd/ingestor: go vet ./... + go test -count=1 ./...	ok, 116.7 s [T]
cmd/ingestor: go test -race -count=1 -run _289	ok, 7.7 s [T]
cmd/server go test -count=1 ./...	ok, 74.9 s [T]
cmd/migrate, internal/dbschema, internal/packetpath	ok [T]
sh test-all.sh	273 passed, 0 failed [T]
node test-frontend-helpers.js	725 passed, 0 failed [T]
bash scripts/test-resolved-path-route-repair-estimate.sh + ShellCheck	all checks passed; ShellCheck clean [T]
gofmt -l on the touched Go files	clean [T]
Benchmarks (-benchtime 1x)	see F7 above [T]
E2E against a local Go server on a free high port, e2e-fixture.db prepared as in CI (freshen, the Kpa-clawbot#1486/Kpa-clawbot#1791 seed SQL taken verbatim from deploy.yml, corescope-migrate, seeds 2073, 199, 245, 122)	test-e2e-playwright.js 154/157, 3 skipped; test-health-status-e2e.js 173 checks ×3 runs; test-issue-165-grouped-hop-warn-e2e.js 12/12; test-e2e-532-route-hop-count.js pass [T]
Server stopped by the pid of the port listener, after checking its command line and cwd	done [T]

The PR has no E2E of its own and makes no UI or server change, so the E2E runs are regression checks only.

Mutants (my own, each applied alone to a copy of head)

#	Mutant	Result
M1	write guard back to the round-1 rule (r.stored == want)	killed: LeavesDriftedLiveIngestRowsUntouched, RepairsBothDirections, Idempotent, ResumesAfterStop
M2	want == had guard removed (if r.stored != had)	killed: RepairsBothDirections, Idempotent
M3	prefilter family bit & 1 instead of & 2 (round-1 M2)	killed: RepairsBothDirections, Idempotent, ResumesAfterStop
M4	not-ready batch returns the error instead of continue (round-1 M5)	killed: RetriesABatchThatSeesAColdSnapshot
M8	!enabled gate in StartResolvedPathRouteRepair disabled	killed: NotScheduledWhenDisabled
M5	JSON tag resolvedPathRouteRepair → resolvedPathRouteRepairX	survives full suite (N1, fails safe)
M6	main.go passes true instead of the config value	survives full suite (N1, fails unsafe; main() wiring)
M7	config.example.json ships "enabled": true	survives full suite (N1, fails unsafe)

Probes (reviewer-only test file, not pushed): TestReviewProbe_DriftedLiveIngestRowUntouched_289 (6 rows, 0/0) and TestReviewProbe_Residual_289 (repaired=1 cleared=1, as documented). Both were run on head and on the merged tree. [T]

Not verified

• The real-data split between defects and drifted live-ingest rows is still unmeasured. The fixture has 0 candidates, the sizing script does not resolve, and I have no staging or production access. A resolve-aware count (stored == had vs stored != had) on a snapshot copy remains the honest pre-flight number before anyone enables the switch. This is already listed as a residual in the PR body. [A]
• Cold-cache cost on a multi-GB database: not measured. The benchmark is warm-cache only, as the PR says. [K]
• The CI flake was not reproduced locally (3/3 green). It was classified from the CI log and the diff only. [T]/[A]
• Firmware: no protocol claim beyond the route-type bit layout (0/1 flood, 2/3 direct; transport 0/3) was re-checked against the MeshCore source in this round. [K]
• Leaflet asset checksums in the E2E prep were not run (not needed for the suites above).