Red commit: 178617ca7b6a3b34597ae59d83e15801985be949 (CI run:
https://github.com/Kpa-clawbot/CoreScope/actions/runs/27191921487 —
red-state was verified locally; CI on this branch runs against green
HEAD per pull_request triggers)
Fixes#1629
## Summary
`/api/nodes/{pubkey}/reach` cached responses survived blacklist
mutations for up to the 5-minute TTL. A node added to `NodeBlacklist`
after a recent reach request was still served the cached non-blacklisted
payload until the entry expired.
## Fix (per triage)
Per @Kpa-clawbot's locked fix path on the issue:
1. Add a monotonic `BlacklistGeneration()` counter on `*Config`.
2. `SetNodeBlacklist` (new setter) atomically replaces the slice,
rebuilds the lookup set under an `RWMutex`, and bumps the generation via
`atomic.AddUint64`.
3. `cmd/server/node_reach.go` folds the generation into the cache key
(`"<pubkey>|<days>|g<gen>"`) so any mutation invalidates prior entries
on the next request — no callbacks bolted onto the setter, no
cache-layer surgery, no TTL change.
While here, the latent bug in `blacklistSet()` is also fixed:
`sync.Once` locked in the initial set, so a later `SetNodeBlacklist` was
invisible to `IsBlacklisted`. The `Once` still gates the lock-free
initial build; mutations rebuild under `RWMutex` and reads take an
`RLock` around the map handoff.
## Files
- `cmd/server/config.go` — `SetNodeBlacklist`, `BlacklistGeneration`,
`rebuildBlacklistSetLocked`, `RWMutex`. `IsBlacklisted` reads the
rebuilt set (no stale-slice short-circuit).
- `cmd/server/node_reach.go` — `cacheKey` includes `|g<gen>`.
- `cmd/server/node_reach_blacklist_cache_test.go` — new regression test
(the red commit).
- `cmd/server/node_reach_endpoint_test.go` — existing cache-hit
assertion updated to the generation-suffixed key.
## TDD evidence
- Red commit `178617ca` adds the test + a deliberate `SetNodeBlacklist`
stub that only reassigns the slice. The test fails on the post-blacklist
assertion: `status=200 want 404 (cached payload was served — #1629)`.
- Green commit `257c104f` replaces the stub with the real
implementation; full `go test ./...` and `go test -race -run
"TestNodeReach|TestNodeBlacklist|TestConfig"` pass locally.
## Scope
- One narrow PR. Backend only — no frontend or API response-shape
change.
- No public type signatures touched beyond the new exported
`SetNodeBlacklist` / `BlacklistGeneration` on `*Config`.
- Preflight: all hard gates pass (PII, branch scope, red commit, CSS,
LIKE/JSON, sync/async migration, XSS).
---------
Co-authored-by: corescope-bot <bot@corescope.local>
Co-authored-by: openclaw-bot <bot@openclaw.local>
Red commit: 03546923b4 (CI run: pending —
see Checks)
E2E assertion added: test-issue-1630-reach-mobile-e2e.js:97
## Summary
Adds narrow-viewport CSS to `public/node-reach.css` so the
`/nodes/{pubkey}/reach` page no longer overflows phone-class viewports.
Fixes#1630
## Approach (red → green)
1. **RED** (`03546923`): added `test-issue-1630-reach-mobile-e2e.js`
asserting at 393×800 and 360×740 that:
- `#nqMap` computed height ≤ 320px
- `.nq-table` scrollWidth ≤ clientWidth (no inner h-scroll)
- ≤ 4 visible TH columns (low-signal collapsed)
Desktop guard at 1440×900: map height stays ~420px and all 6 columns
remain visible — proves no desktop regression.
Wired into `.github/workflows/deploy.yml` Playwright job so CI is the
source of truth.
2. **GREEN**: added `@media (max-width: 480px)` block in
`public/node-reach.css` that shrinks `.nq-map` to 280px, hides the
`distance (km)` column, and stacks `we hear` / `they hear us` into a
single compact column.
## Out of scope (intentionally not touched)
- Backend `cmd/server/node_reach.go` (tracked in #1631 / #1629).
- Reach page re-theming.
- Per-column user toggles.
## Local verification
Screenshots at the three target viewports (393×800, 360×740, 1440×900)
attached below.
---------
Co-authored-by: openclaw-bot <bot@openclaw.local>
## TL;DR
Post-merge regression introduced by #1627 r3 (commit `e2212f50`):
`buildNodeInfoMap` in `cmd/server/neighbor_api.go` ran an uncached
`SELECT … FROM nodes` scan on every call. Folded `first_seen` into the
already-cached `getCachedNodesAndPM` (30s TTL) so the 4 hot handlers
that call `buildNodeInfoMap` no longer pay for a full table scan per
request.
## Before / After
`buildNodeInfoMap` is called by **4 hot handlers**:
- `cmd/server/neighbor_api.go:130`
- `cmd/server/neighbor_api.go:297`
- `cmd/server/neighbor_debug.go:83`
- `cmd/server/node_reach.go:421`
| | Before | After |
|---|---|---|
| `SELECT … FROM nodes` per call | 1 (uncached) | 0 (cache hit) |
| `SELECT … FROM observers` per call | 1 (uncached) | 1 (unchanged) |
| At Cascadia scale (~2600 nodes) | full scan × 4 handlers × N req/s |
one scan / 30s |
## How
- Extended the `getAllNodes` schema probe to also `COALESCE(first_seen,
'')`. Falls back through the existing richest → leanest ladder if the
column is missing.
- `nodeInfo.FirstSeen` is therefore populated for every cached entry in
`getCachedNodesAndPM`.
- `buildNodeInfoMap` drops its second `SELECT` entirely and just copies
`nodeInfo` values out of the cached map.
- Public signature of `buildNodeInfoMap` is unchanged.
`node_reach.go:421` still sees `nodeInfo.FirstSeen` populated, served
from cache.
`cmd/server/store.go` is touched because `getAllNodes` is the only
sensible owner of the `first_seen` SELECT — adding a parallel cache
would duplicate the 30s TTL machinery this fix is designed to leverage.
## Test (red → green)
- Commit 1 (`test:`): `TestBuildNodeInfoMap_FirstSeenIsCached` — calls
`buildNodeInfoMap`, mutates `first_seen` out-of-band via a separate rw
connection, calls it again, and asserts both calls return the same
(cached) value. Fails on `origin/master` (call 2 sees the mutated value,
proving the uncached scan).
- Commit 2 (`perf:`): the fold. Test now passes.
## Refs
Post-merge audit identified this as the only MAJOR finding from #1627;
recommendation was a follow-up hot-fix PR. This is that PR.
---------
Co-authored-by: openclaw-bot <bot@openclaw>
Co-authored-by: openclaw-bot <bot@openclaw.local>
Red commit: 67088342ec (CI run: pending)
## Summary
Fixes#1631 — `scanReachRows` swallowed `QueryContext` / `rows.Err()`
failures and returned `nil`. The handler treated that as "genuinely no
reach" and rendered a 200 with empty arrays (or 404 in some flows), so
transient SQLite failures surfaced to operators as "this node has no
reach" — misleading and undiagnosable without log access.
## Fix
`cmd/server/node_reach.go`:
- `scanReachRows` now returns `([]pathRow, error)`; propagates
`QueryContext` + `rows.Err()` failures.
- `computeNodeReach` signature gains an error return: non-nil error
means real backend failure (NOT "unknown node").
- `handleNodeReach` renders **500** on that error path and does **NOT**
cache the failure (next request retries cleanly). Genuinely-empty reach
still renders **200** with empty arrays; unknown/blacklisted nodes still
render 404.
## TDD
- Red commit `67088342`: adds `TestNodeReach_ScanDBErrorReturns500` —
warms the integration DB, drops the `observations` table, asserts
handler returns 500. Pre-fix this got 200 with empty arrays.
- Green commit `5408be3a`: the fix + caller updates. Adds
`TestScanReachRows_ErrorReturn` (unit-level: closed-DB → non-nil err).
- `TestNodeReach_ShapeAndClamp` had to be tightened: the v2 fixture's
`observations` table was missing `observer_idx`; the swallowed error
masked that schema gap. Now rebuilt with the right shape.
## Scope
- `cmd/server/node_reach.go` — fix.
- `cmd/server/node_reach_endpoint_test.go` — new red test +
ShapeAndClamp fixture fix.
- `cmd/server/node_reach_test.go`, `node_reach_bench_test.go` — caller
updates for new signature + one new unit assertion test.
No cache changes (#1629 is separate). No sibling refactors. No frontend.
## Verification
- `go test ./cmd/server/...` — green (48s, all tests).
- pr-preflight — clean (PII, scope, red-commit, CSS vars, LIKE-on-JSON,
async-migration, XSS).
---------
Co-authored-by: clawbot <bot@kpa-clawbot.local>
Re-submission of #1625 (which was merged early, then reverted in #1626)
— now with **all three round-1 reviews addressed** so it lands in one
hardened state instead of as post-merge follow-ups.
## What
Per-node **Reach** view: a standalone page (`#/nodes/{pubkey}/reach`) +
a node-detail section + `GET /api/nodes/{pubkey}/reach`. It shows which
nodes a node has a **stable two-way RF link** with, derived from raw
`path_json` adjacency (a path travels origin→observer, so `[A,B]` ⇒ B
heard A). A link is bidirectional when both directions have
observations; the **bottleneck** (weaker direction) rates two-way
reliability. Nodes are identified only by **unique 2–3 byte** path
prefixes (1-byte collides → excluded).
## Review fixes folded in vs #1625
**Performance (Carmack):** hard scan LIMIT (200k) + modest prealloc;
`json.Unmarshal` replaced by a single-pass `parsePathTokens` (100k-row
scan 2.2M→1.3M allocs, 344→203ms); memoized resolver; size-hinted maps
(attribution over 100k rows: 102 allocs); `context.Context` plumbed;
cache `RWMutex` + evict-oldest (no full wipe); singleflight dedup;
degree/rank from a 60s shared snapshot; bench rewritten (ReportAllocs,
1k/10k/100k, mixed-payload, isolated attribution).
**Correctness/safety + tests (Independent + Kent Beck):** pubkey
validation → 400; error logging instead of silent swallow (first_seen /
degree / marshal→500 / discarded rows); `public_key=?` index use;
canonical `PayloadADVERT`; `min()` builtin; documented cache-slice
immutability; mux ordering comment. New tests: scanReachRows decode,
3-byte token branch, non-advert first-hop guard, observer SNR
aggregation across rows, HTTP-level attribution (asserts non-zero
we_hear/they_hear), 400/404/blacklist/cache-hit.
**UI / a11y / Tufte:** in-map legend (tiers + thresholds); dropped the
colour+width double-encoding (constant width, colour-only); colour-blind
glyphs (●●●/●●/●) + tier title beside the bottleneck number; dark-theme
`--link-*`; lighter table (horizontal rules, sentence-case headers); map
built once + link layer updated in place on toggle (no flicker);
time-range no longer flashes a loader; `destroy()` generation guard;
statCard escaping; scoped `@media print` to `#nq-report`;
`fieldset/legend` + `for/id` toggles; `aria-pressed` / `aria-live` /
back-link `aria-label`; "distance (km)" + bottleneck tooltip + no-GPS
note; inline styles → CSS; decorative emoji removed.
**Docs:** api-spec documents the 5-min cache, 200k scan cap, and 400.
## Testing
- `cmd/server` full suite green; reach unit + endpoint + bench all pass.
- `eslint public/*.js` (no-undef) and the XSS-sink gate clean.
- E2E updated: request status checks + exact (non-tautological) toggle
assertions + hard map-render assert.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---
## TDD-history note (Kent Beck gate)
This branch carries production + tests together, not a fabricated
red→green sequence. That's deliberate: the branch was rebased onto
upstream and the intermediate SHAs were squashed, so reconstructing a
"failing-test-first" commit after the fact would be theatre, not
evidence — and rewriting history to stage it would be dishonest. The
behaviour is instead covered by a comprehensive, anti-tautological suite
(directional attribution edges, 3-byte token branch, non-advert
first-hop guard, observer SNR aggregation, HTTP-level attribution
asserting non-zero counts, scan-cap truncation, zero-reach 200-not-404,
companion mis-attribution, cache eviction). Requesting maintainer
acceptance of the work on test *substance* rather than commit
*choreography*; the net-new-UI exemption is not claimed for the server
endpoint.
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: meshcore-bot <bot@meshcore>
Reverts #1625.
#1625 was merged before the round-1 reviews (Independent / Kent Beck /
Tufte) were addressed. Reverting to land it cleanly: a fresh PR will
re-add the feature with the perf pass, the backend correctness/safety +
test-coverage fixes, and the UI/a11y (Tufte) batch folded in, so it goes
through review in a single hardened state rather than as a string of
post-merge follow-ups.
No functional loss — the feature returns in the replacement PR.
## What
Adds a per-node **Reach** view that answers "how well does this specific
node hear, and get heard by, its neighbours?" — both as a standalone
page (`#/nodes/{pubkey}/reach`) and as a section on the node detail
page.
New endpoint: **`GET /api/nodes/{pubkey}/reach`**.
## What it measures
For the target node it derives, from raw `path_json` adjacency (a path
travels origin→observer, so in `[A,B]` B received A directly):
- **Directional link counts** per neighbour: `we_hear` (how often we
received them) vs `they_hear` (how often they received us).
- **Bidirectional / bottleneck**: a link is two-way stable when both
directions > 0; the weaker direction is the bottleneck and rates real
two-way reliability.
- **Importance**: neighbour degree + rank, relay-observation volume,
bidirectional-link count, direct-observer count.
- **Direct observers**: who received the node at 0 hops, with SNR.
Reliability rule: a neighbour is only attributed when its pubkey
**prefix is unique** at the path's byte length (collisions are skipped,
never misattributed).
## UI
- Standalone Reach page + node-detail section.
- Reusable bidirectional link map (OSM) with links coloured by
bottleneck.
- Incoming/outgoing toggles to isolate each direction.
## Naming note (deliberate, no collision)
This is distinct from the existing **per-observer reachability** in
topology analytics (`ReachNode` / `ObserverReach` / `perObserverReach`).
This PR adds its own `NodeReach*` response structs in a new
`node_reach.go` and a new `/api/nodes/{pubkey}/reach` route — there are
no symbol or route collisions (verified: `go build ./...` clean). Happy
to rename to disambiguate further (e.g. "Link Quality") if you'd prefer
to reserve "Reach" for the per-observer feature.
## Testing
- `cmd/server`: endpoint shape/404/limit-clamp + unit tests for token
derivation and directional attribution, plus a scan benchmark — all
pass.
- Frontend: helper tests + Reach-page E2E (`test-node-reach-e2e.js`),
standalone route + incoming/outgoing toggles.
- `go build ./...` and `eslint public/*.js` (no-undef) clean.
## Docs
Design spec, implementation plan, and the `GET
/api/nodes/{pubkey}/reach` API contract are included under `docs/`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Closes#1290.
cross-stack: justified — backend persists firmware-side `repeat` hint to
a new observers column, frontend surfaces the listener/repeater status
as a badge on the observers list and node-detail Heard By table per the
issue's UI acceptance criterion.
## What
Firmware 1.16 publishes a `repeat: on|off` flag in the MQTT `/status`
JSON (confirmed by @cwichura on the issue thread — see
[`MQTTMessageBuilder.cpp:58`](https://github.com/agessaman/MeshCore/blob/b45373a31f111fb0de98bb3b168226d09ceadc47/src/helpers/MQTTMessageBuilder.cpp#L58)
in `agessaman/MeshCore mqtt-bridge-implementation-flex`). Listener-only
observers (`repeat:off`) by firmware contract never relay packets, so
they cannot legitimately be a hop in someone else's resolved path. This
PR plumbs the hint end-to-end so the disambiguator stops considering
them.
## How
* **`internal/dbschema`**: idempotent `can_relay INTEGER DEFAULT 1`
migration on `observers`, plus `AssertReady` probe (server fatal-logs if
absent). Mirrored in `cmd/ingestor/db.go` `CREATE TABLE` for fresh DBs.
Annotated `PREFLIGHT: async=true` — `DEFAULT 1` is constant so SQLite
does this as a metadata-only schema rewrite.
* **`cmd/ingestor`**: `extractObserverMeta` accepts `repeat` as bool,
case-insensitive string (`on|off|true|false|yes|no`), or numeric `0|1`.
Missing field → `nil` → `COALESCE` preserves the existing column value
(back-compat with legacy observers). Plumbed through `UpsertObserverAt`
and the prepared upsert statement.
* **`cmd/server`**: `GetNonRelayObserverPubkeys` + new
`prefixMap.markNonRelay` drop matching candidates inside
`pm.resolveWithContext` at the top of the resolver, so all 4 tiers see
the pruned candidate set. `ObserverResp.CanRelay` is surfaced on
`/api/observers` and `/api/observers/{id}`. `GetNodeHealth` enriches
per-observer rows with `can_relay` so the node-detail badge renders.
Probe-and-fall-back when the `can_relay` column is absent (legacy test
fixtures).
* **`public/`**: listener vs repeater pill on observers list, observer
detail `Relay` stat card, and node-detail `Heard By` table. CSS uses
existing theme vars.
## Test
Added `TestResolveWithContext_ExcludesNonRelayObservers_Issue1290` in
`cmd/server/resolve_non_relay_1290_test.go` covering all three required
cases:
* `repeat:off` pubkey → not a candidate (assertion failed in red commit
`5f7fdb96`, passes after green `f12911dc`)
* `repeat:on` pubkey → still a candidate (regression guard)
* legacy obs (no field) → still a candidate (back-compat)
Red→green proof:
```
$ git log --oneline origin/master..HEAD
f12911dc feat(#1290): exclude listener-only observers from path-hop disambiguator
5f7fdb96 test(#1290): red — assert listener-only observers excluded from path-hop candidates
```
Full server + ingestor + dbschema + migrate test suites pass locally.
## Acceptance checklist (from #1290)
* [x] Ingestor parses `repeat` field (boolean OR string `on|off`)
* [x] Field persisted on `observers` table (new `can_relay BOOLEAN`
column, idempotent migration via `internal/dbschema`)
* [x] Server's disambiguator (`pm.resolveWithContext`) excludes
`can_relay=false` observer-nodes from path-hop candidate set
* [x] UI badge on observers list + node detail page indicating
"listener" vs "repeater"
* [x] Backward compat: legacy observers default to `can_relay=true`
* [x] Test: `repeat:off` → NOT a candidate
* [x] Test: `repeat:on` → IS a candidate
* [x] Test: legacy → IS a candidate
## Out of scope (preserved per issue)
Backfilling already-resolved paths is left as a follow-up. No
firmware/broker changes.
---------
Co-authored-by: Kpa-clawbot <bot@kpa-clawbot.local>
Co-authored-by: openclaw-bot <bot@openclaw>
Kpa-clawbot