dandri/meshcore-analyzer
1
0
Fork 0
mirror of https://github.com/Kpa-clawbot/meshcore-analyzer.git synced 2026-10-12 19:37:23 +00:00
Code Issues Packages Projects Releases Wiki Activity
3,000 Commits 83 Branches 39 Tags
de364a8bb6cb26fb530890aec2dc1ffa2e7301d9
Commit Graph
409 Commits
Author SHA1 Message Date
efitenandClaude Opus 5.5 de364a8bb6 feat: mail notifications for watched nodes (user management part E) (#2140) 
Part E of #2128: a logged-in user watches nodes and gets one mail when a
watched node goes offline, comes back, or reports a low battery. Admins
can add instance events: a new foreign node, and an observer going
offline or back. This covers the per-user mail part of #775.

Stacked on #2139 (part D, which builds on #2138). Review the commits
after that branch.

## The situation

A sysop learns that a repeater went silent, or that its battery is
running down, only by opening CoreScope. #775 asks for notifications.
Accounts (part A) now give a verified address to mail and a mailer with
delivery status.

## What this PR adds

**Events.** They use the instance thresholds, so a mail does not
disagree with the node page:
- `node.offline`: silent for the role's `healthThresholds` window. Last
heard comes from the packet store, and repeaters and rooms count relayed
traffic (#1598).
- `node.battery`: advert telemetry below `batteryThresholds.lowMv`,
recovered at `lowMv + 100`.
- Admins only: `foreign.new` (once per node) and `observer.offline`.

**Store.** Schema v5 with `notification_prefs`, `notification_watches`
and `notification_state`.

**Server** (only with `userManagement.notifications.enabled`)
- A notifier next to the janitor, every `intervalMinutes` (5). The
evaluator is a pure function of watches, prefs, stored states and node
snapshots.
- No burst after a restart or a new watch: a subject's first evaluation
stores its state without mailing. The loop waits for the startup load.
While ingest is stale (newest packet older than 30 minutes) the offline
checks pause, and after recovery they wait one silent window.
- One mail per user per check. Limits: 20 per user and 100 per instance
per rolling 24 hours, 50 watches per user. A change over a limit is
recorded and never mailed later.
- Every mail has a one-click unsubscribe (`List-Unsubscribe` and
`List-Unsubscribe-Post`). The GET only redirects to a confirm page, so
link scanners cannot unsubscribe anyone. Node names are cleaned of
control and bidi characters before they go into a mail.
- Routes: `GET`/`PUT /api/account/notifications`, `PUT`/`DELETE
/api/account/notifications/watches/{pubkey}`, `POST
/api/account/notifications/watch-my-nodes` (copies the synced "my nodes"
list), `GET`/`POST /api/notifications/unsubscribe`. `mailer.Message`
gains `Headers`.

**Frontend.** A "Notify me" toggle on the node pages, a Notifications
section on the account page (`#/account?section=notifications`), an
unsubscribe page, and the notification mail count on the admin overview.

Spec:
[`docs/specs/2026-10-07-node-notifications-design.md`](https://github.com/efiten/CoreScope/blob/feat/notifications/docs/specs/2026-10-07-node-notifications-design.md).

## Performance

Each check reads watches, prefs and states from `users.db` (one query
each), the watched nodes from the analyzer DB in chunks of 500, and
last-heard times from the packet store.

| Measurement | Result |
|---|---|
| One check on our staging instance, 130,000 packets in memory, 1
watcher | 6.9 ms, of which 0.93 ms holding the store read lock |
| Evaluator benchmark, 100 users with 50 watches each, 2,000 nodes |
1.75 ms |

No work is added to ingest, broadcast or any request path.

## Verification

- `internal/users` and `internal/mailer` with `-race`; the `cmd/server`
suite plus `-race` on the notifier tests; 74 new Go tests. The same
Windows-only failure as noted in #2138 applies.
- `sh test-all.sh` exits 0; the XSS gate in diff mode passes.
- User-management E2E: 26 of 26 steps locally. With notifications off
the new steps fail, so they do test the flag.
- On our staging and production instance since 7 October 2026.

## Not in this PR

- Other channels (Discord, Telegram, webhooks). Detection is separate
from delivery, so one can be added.
- The topology, RF and anomaly alerts of #775.
- If the whole server starts after a feed outage that already ended, the
grace window is unknown and a watcher can get one wrong offline mail.
The user guide says so.

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-08 22:02:39 +02:00
efitenandClaude Opus 5.5 abbe6846ff feat: propose and approve hashtag channels (user management part D) (#2139) 
Part D of #2128: logged-in users propose a hashtag channel, an admin
approves, rejects or later revokes it, and the ingestor decrypts
approved channels without a restart. It is the first consumer of a
generic proposal table, and answers the request in #2092.

Stacked on #2138 (part C). Review the commits after that branch; GitHub
shows C's commits here until it is merged.

## The situation

- An instance decrypts only the hashtag channels listed in
`hashChannels`. Users who want a city or club channel shown have to ask
the operator, who edits `config.json` and restarts the ingestor.
- #2092 asked for a suggest-and-approve flow; a downstream fork
(dborup/CoreScope#99) runs one with anonymous suggestions and a file
queue.

## What this PR adds

**Name rules (`internal/channel`).** `ValidateHashtagName`, mirrored in
the browser:
- at most 31 UTF-8 bytes including `#` (firmware
`ChannelDetails.name[32]`), case preserved, `Public` refused in any
case;
- control, bidi, separator and format characters refused except ZWJ,
plus invisible fillers (`Other_Default_Ignorable_Code_Point`, U+2800,
non-ASCII spaces).

Go and JS give the same verdict for every code point.

**Store.** Schema v4, one generic `proposals` table (kind, subject,
status pending/approved/rejected/revoked, proposer, reviewer, note). One
row per (kind, subject); limits are checked in the same transaction as
the change.

**Server** (only with `userManagement.channelProposals.enabled`)
- `POST /api/proposals`, `GET /api/account/proposals`, `GET
/api/admin/proposals`, `POST
/api/admin/proposals/{id}/approve|reject|revoke`.
- 400 for an invalid name. 409 for a duplicate, a rejected name, or a
name already in `hashChannels`/`channelKeys` ("this channel is already
decrypted on this instance"). 429 over a limit (`maxPending` 100,
`maxApproved` 128, `perUserPerDay` 5).
- `GET /api/channels` gains `approvedChannels`, so approved channels are
listed before they have traffic. With the feature off the response is
byte-identical.

**Ingestor.** Opens `users.db` with `mode=ro` every 60 seconds,
re-validates the names, and swaps its key map through an
`atomic.Pointer`. Configured channels always win, and a failed read
keeps the last good set. The ingestor never writes `users.db`; AGENTS.md
now says so.

**Frontend.** "Propose for everyone" in the add-channel dialog, an
"approved" marker in the channel list, "My proposals" on the account
page, and an admin "Proposals" tab with a warning on approve. The packet
detail pane now escapes the channel name: until now only the operator
chose those names.

Spec:
[`docs/specs/2026-10-07-channel-proposals-design.md`](https://github.com/efiten/CoreScope/blob/feat/channel-proposals/docs/specs/2026-10-07-channel-proposals-design.md).

## Performance

Every key is tried on each GRP_TXT that no key opens (`decodeGrpTxt`).
Benchmark of that worst case:

| Keys | Time per packet |
|---|---|
| 320 configured | 204 to 216 µs |
| 320 configured + 128 approved | 272 to 320 µs |

That is about +33%, linear in the key count and capped by `maxApproved`.
With the feature off the decoder gets the identical map and no ticker
runs.

## Verification

- `internal/channel`, `internal/users` (with `-race`), `cmd/server` and
`cmd/ingestor` pass locally, 53 new Go tests. The ingestor's
`TestWriteStatsAtomic_SymlinkAtDestIsReplaced` needs symlink rights on
Windows and fails on `master` too.
- `sh test-all.sh` exits 0; the XSS gate in diff mode passes.
- User-management E2E: 21 of 21 steps locally (propose, approve, the
channel appears, revoke removes it).
- On our staging and production instance since 7 October 2026.

## Not in this PR

- Private (PSK) channels: their keys stay in the browser (#725).
- Anonymous proposals.
- Names that need ZWNJ (some Persian and Urdu spellings) and subdivision
flags are refused, because ZWNJ and tag characters are format
characters. Allowing them later only widens the rule, so no approved
name gets stranded.

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-08 21:51:42 +02:00
efitenandClaude Opus 5.5 18a634c115 feat: admin dashboard with overview and audit log (user management part C) (#2138) 
Part C of #2128: an admin area with an overview, the user table and a
global audit log. Off by default like the rest of user management: with
`userManagement` off nothing changes, and non-admins get no new UI.

This is the first of 4 stacked PRs (C, D, E, then a small export/backup
follow-up). Each later one contains this branch; review them in order.

## The situation

- An admin could manage users one by one in `#/admin/users`, but nothing
showed whether the instance needs attention: accounts stuck in
activation, bouncing mail, someone guessing a password, an MQTT source
that dropped.
- The audit log existed (`internal/users/audit.go`) but could only be
read per user, and logins were not recorded.

## What this PR adds

**Store (`internal/users`)**
- Schema v3: an index on `audit_log(at)`.
- `AuditList` with filters (action or group prefix like `user.login.*`,
user as actor or target, period) and keyset pagination; `PruneAudit`;
`Stats` for the user figures.

**Server**
- `GET /api/admin/stats` (typed struct): accounts by status, admins,
registrations and active users over 7 and 30 days, logins and failed
logins in 24 hours, mail by final status, and the attention items
computed server-side.
- `GET /api/admin/audit`: filtered, newest first, `next` cursor.
- `GET /api/admin/users` gains `bouncing=1`.
- Logins are audited as `user.login` and `user.login.failed` (reason
`wrong_password`, `pending` or `disabled`). An unknown address writes no
row. The writes are asynchronous, so the login response does not wait on
`users.db`. Login rows are pruned after 90 days.

**Frontend**
- `#/admin?tab=overview|users|audit`: `admin.js` (tab shell),
`admin-overview.js` ("Needs attention", Users card, System card from the
existing health, MQTT and observer endpoints), `admin-audit.js` (filters
in the URL, "Load more"). `admin-users.js` becomes the Users tab; the
old `#/admin/users` link rewrites to it.
- `/api/healthz` is read on open and on Refresh only, not on the
60-second timer, because it walks every packet under a read lock.

Spec:
[`docs/specs/2026-10-07-admin-dashboard-design.md`](https://github.com/efiten/CoreScope/blob/feat/admin-dashboard/docs/specs/2026-10-07-admin-dashboard-design.md).

## Verification

- `internal/users` and `cmd/server`: `go vet` and `go test` pass
locally, 26 new Go tests. One upstream test,
`TestSaveGeoFilterPreservesFileMode`, also fails on Windows on plain
`master` (file modes) and is unrelated.
- `sh test-all.sh` exits 0; the XSS gate in diff mode passes.
- User-management E2E with the `e2etest` build: 13 of 13 steps locally.
- Running on our staging and production instance since 7 October 2026.

## Not in this PR

- Restricting existing pages (perf, MQTT status) to logged-in users or
admins. The admin area adds no access mechanism of its own, so that
stays a later router rule plus endpoint check.
- Server-enforced customizer tab restrictions (#1508).
- The 24-hour, 5-attempt and 10-minute attention thresholds are
constants for now (AGENTS.md rule 8: customizer later).

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-08 21:38:30 +02:00
nullrouten0andClaude Mythos 5.1 caa66356bd fix(api): cap concurrent cold reach scans at 2, answer 429 beyond that (#2127) 
`GET /api/nodes/{pubkey}/reach` is unauthenticated and a cold-cache
request runs a full scan. `singleflight` only collapses identical keys,
so distinct `(pubkey, days)` requests each start a scan, and they share
the 4-connection SQLite pool with every other handler. A handful of
requests for different nodes can hold every reader and stall the whole
API.

**Fix:** a small semaphore allows two cold scans at once. A cold request
that finds both slots busy gets `429` with `Retry-After: 5` instead of
queueing. Cached answers are unaffected, and the two in-flight scans
still complete normally.

**Tests:** `node_reach_concurrency_test.go` — with both slots held a
cold request returns 429 with `Retry-After`; after release the same
request runs normally. Full server suite passes.

**Note:** our instance runs a fork that already bounds reach work
differently (an async job queue), so this exact change is not what we
run in production. The test suite is the verification here.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Mythos 5.1 <noreply@anthropic.com>
 2026-10-08 16:26:15 +02:00
nullrouten0andClaude Mythos 5.1 0a6870c508 fix(api): cap request bodies on /api/decode and /api/packets/observations (#2119) 
`POST /api/decode` and `POST /api/packets/observations` are
unauthenticated and decoded the request body with no size limit.
`json.Decoder` buffers the whole value in memory, and `/api/decode`
hex-decodes the string before the 184-byte payload check runs. On a test
instance one 100 MB request raised RSS by about 180 MB. Many requests in
parallel can push a container past its memory limit, and a restart
reloads the dataset for several minutes.

**Fix:** `http.MaxBytesReader` on both handlers, the same way the
geo-filter PUT and `/api/paths/inspect` already do it.

| endpoint | cap | why |
|---|---|---|
| `/api/decode` | 4 KB | a packet is at most ~512 hex chars |
| `/api/packets/observations` | 64 KB | 200 hashes of 64 chars is ~14 KB
|

Oversized bodies get HTTP 413 instead of a generic 400.

**Tests:** `body_limits_test.go` — normal bodies still return 200,
oversized bodies return 413, for both endpoints. Full `go test ./...` in
`cmd/server` passes.

**Note:** deployments behind a reverse proxy with its own body limit
were already safe. The shipped Caddy config has no body limit, so
default installs were not.

Running in production on our instance since 2026-10-07.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Mythos 5.1 <noreply@anthropic.com>
 2026-10-08 16:26:10 +02:00
nullrouten0andClaude Mythos 5.1 548e4cd4a4 fix(api): cap the nodes= list on /api/packets at 50 entries (#2120) 
Each entry in the comma-separated `nodes=` list on `GET /api/packets`
costs one SQLite lookup (`resolveNodePubkey`) while the packet store's
read lock is held. A 1 MB URL fits about 15,000 entries. On a test
instance 12,000 entries took 1.3 s per request, against 0.9 ms for one
entry, and the lock stalls the poller's writes for that long. A few
parallel clients can keep the site busy and the live feed stale.

**Fix:** lists longer than 50 entries get HTTP 400 with a clear message.
No UI page sends more than a handful.

**Tests:** `multi_node_cap_test.go` — 50 entries return 200, 51 return
400. Full `go test ./...` in `cmd/server` passes.

Running in production on our instance since 2026-10-07.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Mythos 5.1 <noreply@anthropic.com>
 2026-10-08 15:58:07 +02:00
nullrouten0andClaude Mythos 5.1 4ce6d9f6d4 fix(ui): pin CDN script versions and add integrity hashes (#2121) 
`leaflet.heat` and `chart.js` in `index.html`, and swagger-ui on
`/api/docs`, were loaded from unpkg with no `integrity` attribute.
`chart.js@4` and `swagger-ui-dist@5` also floated on a major version, so
a new release would load unreviewed. A compromised CDN or package would
run as our own code on every page. Leaflet itself already had a hash.

**Fix:** pin `chart.js@4.5.1`, `leaflet.heat@0.2.0`,
`swagger-ui-dist@5.33.1`, each with a sha384 hash and
`crossorigin="anonymous"`.

**How the hashes were made:** download each pinned file, `openssl dgst
-sha384 -binary | base64`, then download again and check the hash
matches.

**Note:** bumping chart.js or swagger-ui now means updating the hash
too. Vendoring them into `public/vendor/` (as markercluster already is)
would remove the CDN dependency entirely; happy to do that instead if
preferred.

Running in production on our instance since 2026-10-07.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Mythos 5.1 <noreply@anthropic.com>
 2026-10-08 15:56:48 +02:00
nullrouten0andClaude Mythos 5.1 8b64439635 fix(server): keep config.json's file mode when saving the geo-filter (#2126) 
`SaveGeoFilter` rewrote `config.json` through a temp file created with
mode 0644, so a config an operator had made 0600 (it holds the API key
and broker passwords) became world-readable after the first geo-filter
save.

**Fix:** stat the original and reuse its mode for the temp file. Falls
back to 0644 when the stat fails.

**Tests:** `config_mode_test.go` — a 0600 config stays 0600 after a
save; a 0644 config stays 0644.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Mythos 5.1 <noreply@anthropic.com>
 2026-10-08 15:56:42 +02:00
dborupandClaude Opus 5.5 7309bdb5a9 fix(store): index a transmission once per relay key in byPathHop (#2117) 
Relates to #2108

## Problem

`traffic_share_score` grows with server uptime until it is far above
reality, and many relays end up clamped at 1.0.

The score is the number of non-advert entries in `byPathHop[pubkey]`
divided by the number of non-advert transmissions.
`indexResolvedPathHops` runs once per **observation**, on the
live-ingest path (`IngestNewFromDB`) and on the late-observation path
(`IngestNewObservations`). `addResolvedPubkeysToPathHopIndex` only
de-duplicates within one call, so every further observation through the
same relay appends the transmission to that relay's bucket again. The
denominator counts each transmission once.

After a restart the values look right, because `buildPathHopIndex` →
`retainResolvedPathHops` de-duplicates by `*StoreTx`. They then drift
upwards as live observations arrive.

None of the affected functions has changed on `master` since the
diagnosis against `415362c`. This branch is based on `000d9ab`.

## Fix

1. **Idempotent insert, per (transmission, relay key).**
`addResolvedPubkeysToPathHopIndex` keeps a side map `pathHopResolved
map[*StoreTx][]string`. It holds the resolved keys each transmission is
already indexed under and skips those.
- Keys are compared **exactly**, so no collision can ever drop an entry.
- A later observation through a **new** relay still adds that relay,
once.
   - `StoreTx` is unchanged.
2. **Interned keys.** `pathHopKeys map[string]string` keeps one shared
copy of each resolved key. The record then holds a 16-byte string header
per entry, and not the string each observation's resolve allocated: a
fresh `json.Unmarshal` string per persisted observation on `Load`, and a
`strings.ToLower` result on the live paths. The `byPathHop` key is the
same shared copy.
3. **Eviction and rebuild.**
- `evictStaleInternal` drops the record of evicted transmissions, plus
any interned key whose bucket it deletes.
- `retainResolvedPathHops` keeps the records of live transmissions only
and drops interned keys whose bucket was not carried over. When a
rebuild starts from an empty index, it clears both.
4. **Defence in depth.** The single (`GetRepeaterUsefulnessScore`),
batch (`GetRepeaterNodeStatsBatch`) and bulk
(`computeRepeaterUsefulnessScoreMap`) scores count **distinct**
non-advert transmissions per bucket, via `countDistinctNonAdvert`.
   - IDs are collected in a reused slice.
- A bucket in ascending ID order is already distinct; only out-of-order
buckets are sorted.
- The bulk pass does this for full-pubkey keys only. Raw-hop buckets get
one entry per transmission from `addTxToPathHopIndex`.
5. **Cache side effect.** A repeated observation through known relays no
longer mutates `byPathHop`, so it no longer drops the batch relay-stats
cache, as the cache contract from `1164` intends. An observation through
a new relay still drops it.

The other `byPathHop` consumers already de-duplicate by `tx.ID` or read
only raw prefix keys: `GetNodeHopAnalytics`,
`computeMultiByteCapability`, `handleNodePaths`,
`computeRepeaterRelayInfoMap` and the relay info in
`GetRepeaterNodeStatsBatch`. Two tests pin that they ignore duplicate
entries.

## Exact keys vs. a 64-bit fingerprint

The first version of this fix stored a 64-bit FNV-1a hash per key. As
requested, I measured both, plus exact keys without interning, on the
real `addResolvedPubkeysToPathHopIndex`.

**Keys per transmission.** In the e2e fixture, the union of resolved
relay keys over all observations of one transmission has a mean of 5.4
and a maximum of 17. The protocol limit is 64 path bytes
(`MAX_PATH_SIZE`), i.e. 64 / hash size hops.

**Memory** retained by the record, per transmission.
`BenchmarkPathHopRecordMemory_2108`: 100K transmissions, keys from 4,000
relays, each transmission heard twice with freshly allocated keys. The
record map and the interned keys are included.

| keys per tx | exact, interned (this PR) | 64-bit fingerprint | exact,
not interned |
|---|---|---|---|
| 2 | 88 B | 68 B | 207 B |
| 5 | 136 B | 101 B | 447 B |
| 17 | 344 B | 197 B | 1,423 B |

- At realistic key counts, exact keys cost 20–35 B per transmission more
than the fingerprint. That is about 3.5 MB per 100K transmissions, and
small next to what the store already charges per transmission
(`storeTxBaseBytes` alone is 384 B).
- Without interning, exact keys would cost 3–4× more, and most of that
would be held during every `Load`. Interning is what makes exact keys
affordable.

**Time.** I ran the variants interleaved, 3 rounds each, median ns/op on
an Apple M4. The record step is not measurable inside the full call. The
isolated record step was faster with exact keys in a separate
micro-benchmark, because comparing a handful of 64-char strings is
cheaper than hashing each one.

| benchmark | exact, interned | fingerprint | exact, not interned |
|---|---|---|---|
| late observation, 20K txs | 312 | 358 | 331 |
| late observation, 100K txs | 760 | 836 | 758 |
| `IngestNewObservations` (SQL + resolve + index) | 531 µs | 497 µs |
527 µs |

**Collision probability of the fingerprint.** For k distinct keys in one
transmission it is about k(k−1)/2 / 2^64:

| k | per transmission | per 10^9 transmissions |
|---|---|---|
| 2 | 5.4e-20 | 5.4e-11 |
| 5 | 5.4e-19 | 5.4e-10 |
| 17 | 7.4e-18 | 7.4e-9 |
| 64 | 1.1e-16 | 1.1e-7 |

For random keys this is negligible. FNV-1a is unkeyed, though, so two
relay keys that collide could be found deliberately (a birthday search
over about 2^32 key pairs). A collision would leave a transmission out
of one relay's bucket.

**Decision.** Exact keys are cheap enough once interned: the same speed
within noise, and a few tens of bytes per transmission. They remove the
question of collision-driven omissions entirely, so this PR uses exact
keys and drops the fingerprint.

The two new maps (`pathHopResolved`, `pathHopKeys`) are not added to
`trackedBytes`, so the `maxMemoryMB` trigger undercounts by roughly the
per-transmission figures above. Both are bounded by live state (eviction
and rebuild prune them), so this is a steady proportional undercount
rather than a leak.

## Tests

**New: `cmd/server/pathhop_dedupe_2108_test.go`.** Behaviour tests that
use only existing API. Each one runs against the real SQLite schema
through `Load`, `IngestNewFromDB` and `IngestNewObservations` where
noted.

| test | covers | on `master` |
|---|---|---|
| `TestPathHopIndexOncePerTx_LateObservations_2108` | 1 + 10 late
observations through the same relays: one entry per relay | **fails**
(11 entries) |
| `TestPathHopIndexOncePerTx_LiveIngestBatch_2108` | 11 observations in
one `IngestNewFromDB` batch | **fails** (11) |
| `TestPathHopIndexOncePerTx_AfterLoad_2108` | `Load` + rebuild, then
one live observation | **fails** (2) |
| `TestPathHopIndexAddsNewRelayFromLaterObservation_2108` | a later
observation through a **new** relay adds it exactly once, and its share
becomes correct | **fails** (2) |
| `TestTrafficShareStableAcrossLateObservations_2108` | single, batch
and bulk scores agree, equal the definition, stay put over rounds of
late observations, and equal a fresh `Load` of the same data | **fails**
(all four relays at 1.0, want 0.4–0.5) |
| `TestPathHopIndexSizeBoundedByTransmissions_2108` | the index size
does not grow with observations | **fails** |
| `TestRelayStatsCacheAcrossRepeatedObservations_2108` | a repeated
observation keeps the relay-stats cache, and the kept cache equals a
fresh compute; a new relay drops it, and the next read sees the relay |
**fails** |
| `TestAddResolvedPubkeysToPathHopIndex_PerRelayIdempotent_2108` | the
helper's return value and cache invalidation per (tx, relay), in any key
order | **fails** |
| `TestTrafficShareCountsDistinctTransmissions_2108` | all three scores
count distinct transmissions in a duplicated bucket | **fails** |
| `TestPathHopConsumersIgnoreDuplicateEntries_2108` | pin of the
consumer audit | passes (by design) |
| `TestMultiByteCapabilityIgnoresDuplicateEntries_2108` | pin of the
consumer audit | passes (by design) |

**New: `cmd/server/pathhop_record_2108_test.go`.** These tests use the
new symbols, so they do not compile against `master`.
- `TestCountDistinctNonAdvert_2108`: ascending, descending, interleaved
duplicates, adverts, nils, untyped.
- `TestPathHopResolvedRecordBoundedAndEvicted_2108`: the record stays at
2 keys after 11 observations. Eviction removes the record and every
entry. A survivor's next observation adds nothing. Evicting everything
leaves no record, no interned key and no bucket.
- `TestPathHopResolvedRecordAcrossRebuild_2108`: a rebuild keeps the
records of live transmissions, so the next observation adds nothing. It
drops the record of a removed transmission and the interned key only
that transmission used.
- `TestPathHopResolvedRecordClearedWithEmptyIndex_2108`: a rebuild from
an empty index clears the record and the interned keys, and the next
observation puts the transmission back.
- `TestPathHopResolvedRecordInternsKeys_2108`: record entries and the
`byPathHop` key share one copy, even though every observation passes
freshly allocated keys.

**Changed: `pathhop_eviction_1908_test.go`.** It built its duplicate
entries by repeating `indexResolvedPathHops`, which no longer
duplicates. It now seeds the duplicates directly, so the `1908` sweep is
still tested against buckets that hold one transmission several times.
The expected buckets are unchanged.

**Changed: `db_test.go`.** `setupTestDB` takes `testing.TB`, so the SQL
benchmark can use it.

**Mutants.** I applied each mutant on its own to this branch. All 14 are
killed:

| mutant | killed by |
|---|---|
| record never consulted | the OncePerTx, stable-share, size, cache and
record tests |
| dedupe per transmission instead of per relay |
`AddsNewRelayFromLaterObservation`,
`RelayStatsCacheAcrossRepeatedObservations`, `PerRelayIdempotent` |
| eviction keeps the record | `RecordBoundedAndEvicted` |
| rebuild keeps records of removed transmissions | `RecordAcrossRebuild`
|
| rebuild from an empty index keeps the record |
`RecordClearedWithEmptyIndex` |
| cache dropped on every call |
`RelayStatsCacheAcrossRepeatedObservations`, `PerRelayIdempotent`,
existing `NoMutation_PreservesCache` |
| cache kept although `byPathHop` changed |
`RelayStatsCacheAcrossRepeatedObservations`, `PerRelayIdempotent`,
existing `InvalidatesRelayStatsCache` |
| single score counts entries |
`TrafficShareCountsDistinctTransmissions` |
| batch score counts entries | `TrafficShareCountsDistinctTransmissions`
|
| bulk score counts entries | `TrafficShareCountsDistinctTransmissions`
|
| distinct count trusts any bucket order |
`TrafficShareCountsDistinctTransmissions`, `CountDistinctNonAdvert` |
| keys not interned | `RecordInternsKeys` |
| eviction keeps interned keys | `RecordBoundedAndEvicted` |
| rebuild keeps interned keys of dropped buckets | `RecordAcrossRebuild`
|

**Commands run:**
- `gofmt -l` on all tracked Go files: clean.
- `go vet ./...` in all 14 modules: clean.
- `cd cmd/server && go test -race ./...`: pass.
- `cd cmd/ingestor && go test ./...`: pass.
- `sh test-all.sh`: 184 of 186 suites pass locally. The other two,
`test-issue-1956-release-routing.js` and `test-preflight-xss-gate.js`,
shell out to scripts that need bash ≥ 4 (`mapfile`). They fail under
macOS's bash 3.2 regardless of this change. This PR touches no frontend
or script files.

## Benchmark: `master` vs. this branch

I ran `master`'s sources (`000d9ab`) and this branch interleaved, 5
rounds, with the same benchmark files. Medians on an Apple M4.

| benchmark | `master` | this PR | change |
|---|---|---|---|
| late observation through known relays, 20K txs | 446 ns, 38 B/op | 441
ns, 0 B/op | within noise |
| late observation through known relays, 100K txs | 710 ns, 69 B/op |
755 ns, 0 B/op | within noise (runs overlap) |
| … index size afterwards, entries/tx (20K / 100K) | 24.99 / 8.99, still
growing | 4.99 / 4.99 | bounded |
| `IngestNewObservations`, one observation for each of 20 txs | 532 µs |
513 µs | within noise |
| bulk score pass, clean index, 20K, ingest order | 246 µs | 316 µs |
+28 % |
| bulk score pass, clean index, 100K, ingest order | 3.03 ms | 4.30 ms |
+42 % |
| bulk score pass, clean index, 20K, reversed buckets | 257 µs | 467 µs
| +82 % |
| bulk score pass, clean index, 100K, reversed buckets | 3.38 ms | 4.79
ms | +42 % |
| bulk score pass after 11 observations per tx, 20K | 663 µs | 284 µs |
−57 % |
| bulk score pass after 11 observations per tx, 100K | 4.72 ms | 3.19 ms
| −32 % |

- On an index that is clean on both builds, the distinct count makes the
bulk pass slower. That pass runs on the cache-miss path, which the
background recomputer refreshes every 5 minutes by default.
- On the index a live server actually holds without this fix, `master`
walks every accumulated duplicate, so the real-world pass is faster
after the fix. The gap grows with uptime.
- The late-observation step no longer allocates. On `master` its index
grows with every observation.

Benchmarks: `BenchmarkLateObservationIndex_2108`,
`BenchmarkIngestNewObservations_2108`,
`BenchmarkTrafficShareScoreMap_2108` and
`BenchmarkPathHopRecordMemory_2108`.

## Production motivation

We have run this fix on two production instances. Before the fix, the
summed `traffic_share_score` grew past 180 and many relays sat at the
1.0 clamp; with the fix no node reaches `≥ 0.999`.

A controlled 12 h A/B run makes the drift explicit. Two servers read one
identical database — one on this `master` base, one with the fix —
alongside a reference server that freshly loads the same database (a
fresh load is correct, because the rebuild dedupes). The unfixed server
drifted to 9 relays at the 1.0 clamp and up to 0.95 absolute error per
node against the reference; the fixed server stayed within 0.05 of the
reference for every node, with no relay at the clamp.

A smaller residual rise remains and is a separate cause (startup-vs-live
hop resolution); it is deliberately left for a follow-up so its effect
stays measurable.

## Out of scope

That remaining slow rise comes from a separate cause: live ingest
resolves some hops that the startup load does not. This PR deliberately
leaves that drift alone, so its effect stays measurable. The fix will
follow in a separate PR.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-08 14:34:31 +02:00
efitenandClaude Opus 5.5 f3ae8ac25a feat: sync a logged-in user's settings across devices (part B) (#2130) 
Part B of #2128: a logged-in user's settings follow them across devices.
Log in on a phone and your own nodes, favorites, customizer and filters
are there; a change on one device reaches the others within a minute or
when you return to the tab.

**This PR builds on #2129.** Until that one is merged, the diff here
includes it. The commits for this part start at `docs(specs): settings
sync for optional user management (sub-project B)`.

## The situation

Everything a visitor sets up lives in one browser's `localStorage`
(about 100 keys in `public/`). A second device or a cleared cache starts
from zero (#895).

## What this PR adds

**Storage.** `users.db` schema v2: one JSON document per user in
`user_settings`, with a revision number and a generation id. A write
succeeds only when the client's revision and generation match the stored
ones, so two devices cannot overwrite each other silently.

**Server.** `GET`, `PUT` and `DELETE /api/account/settings`, behind the
same session and CSRF checks as the account routes.
- The server owns the list of synced keys (61 keys,
[`settings_allowlist.go`](https://github.com/efiten/CoreScope/blob/feat/settings-sync/cmd/server/settings_allowlist.go))
and sends it to the client, so the two cannot drift.
- A hard denylist, checked first, refuses `meshcore-api-key`, every
`corescope_channel_*` key and `live-channel-colors` (#725). The colour
map is keyed by channel hash, and for a user-added channel that hash is
`user:<name>`, which would expose hashtag channel names.
- Documents are capped at 256 KiB, measured like `JSON.stringify`. PUT
is limited to 60 requests per hour per user. A stale revision gets 409
with the current document.

**Client**
([`settings-sync.js`](https://github.com/efiten/CoreScope/blob/feat/settings-sync/public/settings-sync.js)).
Inert unless the feature is on and someone is logged in.
- It wraps `localStorage.setItem` and `removeItem` for allowlisted keys
only and pushes 2 seconds after the last change.
- It pulls on login, page load, tab focus and every 60 seconds while the
tab is visible.
- **Merge:** three-way, against a per-device baseline that belongs to
one user and one document generation. Lists (own nodes, favorites, saved
filters) merge per item, so an item added anywhere is kept and an item
removed on one device does not come back from another. Single values:
the profile wins unless only this device changed it.
- Remote changes are written without a push, theme and colour-blind
preset are re-applied, and the current page re-renders (skipped on
account pages and while the geofilter editor is open).

**UI.**
- Logout asks: keep my settings on this device (default), remove them
from this device, or cancel. Channel keys are never removed: no copy
exists anywhere else.
- The account page gets a "Settings sync" section: last synced time,
"Sync now", what is and is not synced, and "Delete synced settings from
my account".

## Not synced

Layout and device state (panel and column widths, collapsed panels, map
positions, geofilter drafts), channel data (#725), the API key, and all
`sessionStorage`. The full list is in the
[spec](https://github.com/efiten/CoreScope/blob/feat/settings-sync/docs/specs/2026-10-06-user-settings-sync-design.md).

## Performance

- One GET per page load, tab focus and minute while visible; one
debounced PUT per burst of changes.
- The `setItem` wrapper costs one Set lookup per write for non-synced
keys. A synced write reads one small revision key, not the stored
document.
- The server reads or writes one row per request.

## Verification

- `internal/users` and `cmd/server`: `go vet` and `go test` pass locally
(22 new Go tests), including a test that every allowlisted key still
occurs in `public/`, and denylist tests.
- `tests/unit/test-settings-sync.js`: 79 passing (vm, real module). The
cases cover the merge table, two tabs sharing one storage, stale answers
after a push, delete while a push is in flight, and logout while the
final push fails.
- `sh test-all.sh` exits 0.
- `tests/e2e/test-user-management-e2e.js` (10 steps, 4 of them new)
passed locally with two browser contexts as two devices: a favorite and
the packet time window travel from device 1 to device 2, a removal does
not come back, and "remove from this device" clears the synced keys
while a channel key stays.
- Checked by hand on a staging instance with a desktop and a phone on
one account.

## Not in this PR

- On a shared browser where the previous user chose "keep", the next
user's first login merges those settings into their own account. The
user guide says to choose "remove" on shared computers.
- Saved filter expressions are synced as typed, including any channel
names written in them. The guide says so.
- Realtime push between devices; the minute pull is the sync interval.

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
 2026-10-07 11:20:44 +02:00
efitenandClaude Opus 5.5 d232072f85 feat: optional user accounts (part A: foundation) (#2129) 
Part A of #2128: optional, off-by-default user accounts. With the
feature off nothing changes; with it on, visitors can register and log
in, and admins manage users and use the operator actions without the API
key.

PR #2130 (settings sync) builds on this one. The two are meant to be
merged together.

## The situation

- Operator actions (geofilter save and prune, backup, perf reset) need
the shared `apiKey`. There is no per-person right.
- Nothing in CoreScope knows who a visitor is, so the requests in #2128
that need that (#1835, #2092, #1508, #730) have nothing to build on.

## What this PR adds

**Two new Go modules**
- `internal/users`: a separate `users.db` (SQLite through
`modernc.org/sqlite`) with users, sessions, single-use tokens, an audit
log and a mail log. Passwords use argon2id.
- `internal/mailer`: a `Mailer` interface with a Brevo client (send,
delivery events, webhook parsing) and an in-memory fake for tests.

**Server (`cmd/server`)**, active only with `userManagement.enabled`
- 24 routes, all documented in OpenAPI under the `users` tag
([`auth_routes.go`](https://github.com/efiten/CoreScope/blob/feat/user-management/cmd/server/auth_routes.go)):
  - auth: register, activate, login, logout, me, forgot, reset;
- account: profile, password, email change with confirmation, sessions,
self-delete;
- admin: list, detail, disable, enable, delete, role, resend activation,
manual activation, mail status refresh;
  - a Brevo webhook, registered only when `mail.webhookSecret` is set.
- `requireAdmin` replaces `requireAPIKey` at the 7 operator call sites:
the API key **or** an admin session. With the feature off it is the old
API-key gate (`TestRequireAdminWithoutUserManagementIsAPIKeyGate`).
- `/api/config/client` gets `userManagement: {enabled: true}` only when
the service started; with the feature off the response is
byte-identical.

**Frontend**
- `auth.js` (header account control, request helper that adds the CSRF
header), `account.js` (login, register, activate, forgot, reset, confirm
email, my account), `admin-users.js` (`#/admin/users`, deep-linked
filters), `account.css` (theme tokens only).
- On phones the top-bar control is hidden, so a conditional entry goes
into the bottom-nav "More" sheet and the nav drawer.
- The customizer geofilter tab and the Perf "Reset stats" button use the
admin session when there is one.

**Config.** A `userManagement` block (`config.example.json`,
[`docs/user-guide/accounts.md`](https://github.com/efiten/CoreScope/blob/feat/user-management/docs/user-guide/accounts.md)).
The Brevo key can come from `CORESCOPE_BREVO_API_KEY`. The server
refuses to start when the block is enabled but incomplete.

## Security choices

- Session cookie `cs_session`: HttpOnly, SameSite=Lax, Secure when
`publicBaseUrl` is https. Every cookie-authenticated state change needs
the `X-CS-CSRF` header and a matching Origin.
- Activation needs the token **and** the account password. Without the
password, an attacker who keeps re-registering a known address could get
the owner to activate an account that carries the attacker's password.
- Register, forgot and email change answer identically for known and
unknown addresses. A password reset ends all sessions, a password change
ends all other sessions, and both end outstanding email-change links.
- Rate limits: login 10 per 15 minutes, register and forgot 5 per hour,
per IP and per address. The bucket count is capped. `trustedProxies`
makes the per-IP limits see real client IPs behind a proxy.
- Server logs carry `#<user id>`, never addresses, tokens or passwords;
mail-provider error texts are redacted before logging.

## Performance

No change to an existing hot path with the feature off. With it on:
- One `users.db` lookup per authenticated request (session by token
hash).
- The admin user table rebuilds its `tbody` on each filter change.
`users.List` caps the result at 1000 rows (`internal/users/users.go`),
which bounds the rebuild.
- `map[string]interface{}` in `openapi.go`: 79 before, 78 after.

## Verification

- `internal/users`, `internal/mailer` and `cmd/server`: `go vet` and `go
test -race` pass locally. 121 new Go tests.
- `cmd/server` with `-tags e2etest`: vet and the e2e hook tests pass.
- `sh test-all.sh` exits 0. `tests/unit/test-user-management-ui.js`: 67
passing (vm, real modules).
- `tests/e2e/test-user-management-e2e.js` (6 steps) passed locally
against an `e2etest` build with the fake mailer and against a
feature-off build. CI builds the `e2etest` binary and runs the suite on
a second server (`deploy.yml`).
- On a staging instance with a real Brevo key: register, activation mail
delivered, activate, admin table, "Refresh status" showing sent,
deferred, delivered, opened and clicked.

## Not in this PR

- Settings sync (#2130), the admin dashboard, approval flows and
notifications (parts B to E of #2128).
- A `requireReadAuth` mode (#1835). Sessions from this PR are what such
a mode would accept.
- Binary size and build time with `modernc.org/sqlite` linked next to
`mattn/go-sqlite3` were not measured. Their driver names do not collide.
#1992 discusses the driver choice.
- No Brevo webhook was configured on staging; delivery status there came
from "Refresh status".

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
 2026-10-07 09:25:29 +02:00
efitenandClaude Opus 5.5 4363403495 fix(nodes): match the region node filter on observer ID, not ingest-time IATA (#2115) 
Follow-up to #2114, from its review.

`RegionNodePubkeys` compared the IATA copied onto each observation at
ingest (`StoreObs.ObserverIATA`). When an operator changes an observer's
code, those copies stay as they were until a restart or until the
observations age out. `/api/nodes?region=` then kept matching the old
region, while the other store region filters already used the new one.
Those filters resolve observers by ID through `resolveRegionObservers`,
for example the packets query and `computeNodeHomeRegions`. The SQL
subquery that #2114 replaced also read `observers.iata` at query time,
so this restores that behaviour.

## Also fixes a regression from #2114

Found in review: `loadChunk`, the background history loader
(`cmd/server/store.go`, the chunk SELECT and the `StoreObs` it builds),
sets `ObserverID` on each observation but never `ObserverIATA`. With
#2114 matching on that IATA, **every node whose adverts came only from
background-loaded history dropped out of `/api/nodes?region=`** on
current master. Matching on observer ID fixes it.
`TestRegionNodePubkeysMatchesChunkLoadedObservations` pins it (an
observation with the observer ID and an empty IATA) and fails on
master's `region_nodes.go`.

## Change

- Resolve the region to observer IDs with `resolveRegionObservers` (own
mutex, 30 s cache) and match observations by `ObserverID`. Lock order:
`regionNodesMu` is released before it, and `s.mu` is taken after it;
none of the three is held together.
- Without a database there is nothing to resolve, so `RegionNodePubkeys`
reports no set and the handler keeps the SQL path.

## Tests

- The region tests now seed an observers table and leave each
observation's IATA at a stale value, so they can only pass through the
table.
- New `TestRegionNodePubkeysFollowsObserverIATAChange`: an observer that
moved from SJC to SFO matches SFO and not SJC. It fails on the previous
code (`got [pk_moved]` for SJC).
- `TestRegionNodePubkeysMatchesChunkLoadedObservations` (second commit),
see above.
- `setupTestDB` takes `testing.TB` so the benchmark can use it; every
existing caller passes `*testing.T` unchanged.
- Full `cmd/server` suite passes locally, the region tests also with
`-race`.

## Performance

`BenchmarkRegionNodePubkeys` (220k adverts × 8 observations): 37 ms to
30 ms per uncached scan, a map lookup per observation instead of a
string normalisation. The observer lookup is one query on the small
`observers` table, cached for 30 s.

## Not done

- No singleflight on a cold cache, and the 64-entry cache still resets
when full. Both were non-blocking in the #2114 review.

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-05 23:42:07 +02:00
efitenandClaude Opus 5.5 d5e6d1b2a6 fix(nodes): resolve the /api/nodes region filter from the packet store (#2114) 
Fixes #2101.

`/api/nodes?region=` filtered nodes with a subquery that joins every
advert to all of its observations and observers, with no time bound
(`cmd/server/db.go`, `GetNodes`). It ran twice per request, once for
`COUNT(*)` and once for the page. A client paging through nodes repeats
both per page.

## Reproduced

On our staging database (10.3 GB, 16.5M observations, 219,750 adverts),
read-only `sqlite3`, region `BRU`:

| | real | user | sys |
|---|---|---|---|
| one regional `COUNT(*)`, as `GetNodes` builds it | **154.8 s** | 1.9 s
| 5.8 s |

Almost all of it is waiting on disk. The plan walks
`idx_transmissions_payload_type` for every advert and then
`idx_observations_dedup` for each one's observations. Two of those per
request, a few requests at once, and the reader pool is gone, which
matches the report of all four database workers sitting in `GetNodes`.

## Fix

- **`PacketStore.RegionNodePubkeys(region)`** (new
`cmd/server/region_nodes.go`) walks the store's in-memory adverts
(`byPayloadType[ADVERT]`) once. It keeps the pubkey of every node with
an advert heard by an observer in the region, using the `ObserverIATA`
each observation already carries. It is cached for 30 s per region, and
the cache is bounded to 64 entries because its keys come from the
client's parameter. Lock discipline follows `resolveAreaNodes`: the
cache mutex and `s.mu` are never held together, and the ordering note in
`store.go` lists it.
- **`GetNodes` takes a `NodeQuery` struct** with a `RegionPubkeys`
field, passed as one `json_each` parameter: `public_key IN (SELECT value
FROM json_each(?))`, a primary-key lookup. An empty set matches nothing.
The SQL subquery stays for a server without a store (tests, tooling).
- The advert-pubkey lookup that `trackAdvertPubkey`,
`untrackAdvertPubkey` and `computeNodeHomeRegions` each copied is now
one helper, `advertPubkey`.

The struct instead of a second `GetNodes` variant keeps the
`map[string]interface{}` count unchanged in `db.go` (75) and `routes.go`
(59).

## Behaviour change

The region filter now covers the adverts the store holds
(`packetStore.retentionHours`), which is the window the rest of the UI
shows, instead of all database history. A node heard in a region only
before that window no longer matches the filter. While the store is
still loading after a restart, the set grows as history loads.

## Performance

| | before | after |
|---|---|---|
| region set, uncached | 154.8 s (SQL count, staging) | 37 ms
(`BenchmarkRegionNodePubkeys`: 220k adverts × 8 observations, 4,000
nodes) |
| region set, within 30 s | same again | cache hit |
| node count over the set | (included above) | 2 ms on staging (1,200
keys) |
| 500-row page over the set | same scan again | 3 ms on staging |

The scan holds `s.mu` for reading for those 37 ms, at most once per
region every 30 s.

## Tests

- `TestRegionNodePubkeys`: the in-region advert, an advert heard in two
regions, case and whitespace in codes, a comma list, an unknown region
giving an empty set, a non-advert never counting, a blank region giving
no filter.
- `TestRegionNodePubkeysCached`, `TestRegionNodePubkeysCacheIsBounded`
(1,000 distinct regions stay within 64 entries).
- `TestGetNodesRegionPubkeys`: the set combines with the role filter and
counts correctly, and an empty set returns nothing even with `Region`
set.
- `TestHandleNodesRegionUsesStore`: an advert that only the store knows
about shows up through `/api/nodes?region=`, so the handler is proven
not to ask SQL.
- Existing region tests (`TestGetNodesRegionFilterV2` and the
`db_test.go` region cases) pass unchanged through the SQL fallback. Full
`cmd/server` suite passes locally; the new tests also pass with `-race`.

## Not done

- No request context on these queries, also raised in the issue. With
the scan gone they take milliseconds, so I left that out of this change.
- The region semantics stay "heard by an observer in the region". #1879
argues for the node's home region instead; that is a separate decision.
- No frontend change. I did not check this in a browser; the nodes page
and map call the same endpoint with the same parameters.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-05 21:45:02 +02:00
Sylvain Rabot b5b230e884 feat(channels): show the sender's path hash size on each message (#2089) 
Each channel message now shows the hash size its sender's path uses, read
from bits 7-6 of the path byte that the originator writes and repeaters keep.
The server sends it as path_hash_size (packetpath.HashSize); the frontend
helper pathHashSize() applies the same rule and returns 0 for unknown.

One rule on both pages: the packet detail Hash Size row and the hex
breakdown call pathHashSize() too, so a 0-hop flood message reports the
same size on Channels and on its packet page. A direct packet with no hops
left reports no size, matching cmd/server/decoder.go. The cases live in
test-fixtures/path-hash-size-cases.json, read by the Go and JS tests.
 2026-10-05 20:32:40 +02:00
efitenandClaude Opus 5.5 000d9ab030 feat(coverage): RF noise-floor layer on the Mobile RX coverage page (#2113) 
The ingestor already stores the noise floor that CoreDrive RX companions
report with each GPS fix (`client_rf_samples`, opt-in through
`clientRfSamples`, `cmd/ingestor/client_rf_sample.go:62`), but nothing
reads it back. An operator who enables it collects the data and cannot
see it. This adds the read side: `GET /api/rf-noise` and a Signal/Noise
toggle on the Mobile RX coverage page.

## What it does

- **`GET /api/rf-noise?bbox=&z=&days=`** returns a GeoJSON hex grid with
the median, quietest and noisiest noise floor per cell, in the same
shape as `/api/rx-coverage`. It is registered always and 404s unless
`clientRfSamples.enabled` is true, the same pattern as the coverage
routes.
- **Stationary samples are excluded.** A parked companion logs hundreds
of readings at one point, which would otherwise define its cell.
- **Coverage page:** a Signal/Noise toggle, rendered only when
`/api/config/client` reports `clientRfSamples: true`. The noise layer
reuses the coverage colour tokens with the axis inverted, because a
lower dBm is quieter. Tiers are at -115 and -108 dBm.
- **Deep link:** `#/rx-coverage?layer=noise` opens on the noise layer.
- **Empty and failed answers** are labelled on the map ("No RF samples
in this view yet", or a retry hint), so a blank map never reads as
"feature off".

No new configuration key: it reads the existing `clientRfSamples`
section that the ingestor already uses. Default off, so nothing changes
for an instance that has not opted in.

## Where it comes from

Ported from the efiten/CoreScope fork, where it has run on
analyzer.on8ar.eu since September (fork commits `42d09f0e`, `80581c43`,
`cde95078`). The cherry-picks conflicted with upstream's newer
`routes.go`, `types.go` and `rx-coverage.js`, so the final state was
ported by hand. The fork-only `/scopes` route that sat next to it in the
same hunk is deliberately left out.

## Performance

The query is bounded by `sampled_at` (indexed, `idx_crf_prune`) and the
bbox, aggregation is one pass plus a per-cell sort, and the response is
capped at 5000 cells. Measured on analyzer.on8ar.eu, all of Belgium
(`bbox=49.4,2.4,51.6,6.5&z=9`), from a client in Belgium, so network
time included:

| window | samples | cells | response time |
|---|---|---|---|
| 7 days | 10,835 | 241 | 0.37 s |
| 30 days | 43,303 | 429 | 0.39 s |

That table holds 45,270 rows in total. It is only read when someone
opens the noise layer, never on ingest or WebSocket paths.

## Tests

- `cmd/server/rf_noise_test.go`: 8 tests for the aggregation (median and
extremes, stationary exclusion, the cell cap, empty input) and the gate
(404 when off, even with data present). All pass, and the full
`cmd/server` suite passes locally.
- `tests/unit/test-rx-coverage-noise.js` (new, in `test-all.sh`): the
colour axis runs the right way, including both tier bounds and a string
median from the API. Slices the real function out of `rx-coverage.js`
and fails on master's copy, which has no thresholds.
- `tests/e2e/test-rx-coverage-noise-e2e.js` (new, wired in `deploy.yml`
and `scripts/non-unit-tests.json`): no toggle when the flag is off;
Noise fetches `/api/rf-noise`, draws the cells, swaps legend and
subtitle, puts `layer=noise` in the hash; a deep link opens on the noise
layer and an empty answer shows the message. 3/3 locally; against
master's `rx-coverage.js` and `roles.js`, 1/3 (only the "flag off" case
passes, as it should).
- `test-rx-coverage-viewport-e2e.js` still passes. ESLint 8 reports 0
errors on the changed files.

## Not done

- The thresholds (-115 / -108 dBm) are fitted to the fork's own data
(1,241 moving samples at the time) and are constants in
`rx-coverage.js`. Per AGENTS.md rule 8 they belong in the customizer
eventually; not in this PR.
- The E2E suite mocks the API. The real endpoint is covered by the Go
tests and by the measurements above, not by a fixture with seeded
samples.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-04 22:11:34 +02:00
efitenandClaude Opus 5 00ee4f9e23 fix(coverage): attribute node-discover replies to the responder (#2111) 
## Problem

A CoreDrive RX companion that runs node-discover gets no coverage on an
upstream instance. Reported today by an operator on v3.13.0: the app
logged 32 discover replies from one repeater in 16 minutes (`heard
751a49f0c5dadc70 (8B, discover)`), all published, and
`client_receptions` stayed at 0 rows for that companion while
`client_rx_observations` held 93.

Two gaps, both on the upstream side:

1. **Ingestor.** `deriveHeardKey` attributes a FLOOD `path[last]` and a
0-hop advert, and drops a 0-hop `CONTROL/DISCOVER_RESP` (firmware
`CTL_TYPE_NODE_DISCOVER_RESP`). The reply carries the responder's own
pubkey at offset 6 of the payload, which `decoder.go` already parses
into `CtrlPubKey` (#1802), but the coverage path never used it.
2. **Server.** CoreDrive RX asks for `DISCOVER_PREFIX_ONLY`, so the
firmware answers with an 8-byte prefix
(`simple_repeater/MyMesh.cpp:799-805`). `coverageHeardKeyCandidates`
only built the 64, 6 and 4 hex candidates, so a 16-hex `heard_key` would
be stored and then matched by no per-node coverage query.

## Change

- `cmd/ingestor/client_reception.go`: a third branch in `deriveHeardKey`
for a discover response with no hops. Accepts exactly 8 or 32 bytes,
nothing truncated, stored with `src='discover'`.
- `cmd/server/rx_coverage.go`: adds the 16-hex prefix to
`coverageHeardKeyCandidates`.
- `docs/client-rx-coverage.md`: documents the `discover` source, the
8-byte keylen and the four-candidate lookup.

The leaderboard and `/api/rx-coverage` read `client_receptions` without
a key filter, so they pick the rows up without a change. Name resolution
goes through `batchResolveHeardKeys`, which is a prefix lookup and
handles 16 hex as is.

## Evidence from a deployment that has had this since 2026-08-19

On analyzer.on8ar.eu, `client_receptions` over the last 7 days by `src`:
discover 4232, rxlog 4909, geo 548, advert 34. Discover replies are 44%
of all coverage rows there (4232 of 9723); on an upstream instance those
rows are not written. They cannot be backfilled afterwards either:
`client_rx_observations` keeps no raw bytes, so the responder pubkey is
gone.

## Tests

- `TestDeriveHeardKey` and `TestBuildClientReception` gain discover
cases: 8-byte and 32-byte keys accepted (32-byte uppercase input
lowercased), a 3-byte and an empty key rejected, a non-discover CONTROL
rejected, a discover response with hops not attributed.
- `TestHandleClientPacketDiscoverRespWritesReception`: end to end, a raw
0-hop DISCOVER_RESP on the client topic writes one `client_receptions`
row with `src='discover'`.
- `TestCoverageHeardKeyCandidatesIncludesDiscoverPrefix`: the 16-hex
prefix is among the per-node candidates.
- Ran locally on Windows: `go test ./...` in `cmd/server` passes; in
`cmd/ingestor` everything passes except
`TestWriteStatsAtomic_SymlinkAtDestIsReplaced`, which needs the symlink
privilege on Windows and fails on clean master too.

Not done: no browser validation, as the change is ingestor and server
only and the frontend reads the same endpoints. Not included: geographic
resolution of 1-byte hops (`src='geo'`) and the RF noise layer, which
are separate changes.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-10-04 17:15:50 +02:00
n30nex b9fb3d244c feat(analytics): split adverts using recorded route evidence (#2088) 
Red commits: `dc7dcbe`, `76e3217` ([assertion
failures](https://github.com/Kpa-clawbot/CoreScope/actions/runs/37059977385)).

Fixes #2041; supplies #2085's backend contract.

Records at most two route bits per retained advert. Node flood counts
now use the same evidence, including mixed and transport-flood adverts.
Analytics failures log/count errors while core observation, path and
liveness writes continue. The unrelated recent-advert limit clamp is
removed.

Direct adverts (empty path) describes the observed remaining path across
valid hash widths. Firmware can forward an advert down to an empty path;
this cannot establish original send mode or RF distance. The `zero_hop`
API spelling remains compatible.

Successful post-upgrade evidence writes survive observation replacement
and restart. Older overwritten frames and failed writes can leave gaps.
The UI explains automatic background backfill.

## Measured validation

Synthetic Windows workloads; timings are not production guarantees:

- 300K retained adverts, one-hour window: median 64.35→39.84 ms; 13.98
MB→145 KB allocated. Seven-day allocations unchanged; five paired
samples.
- 1M transmissions/11M observations: backfill 5m43s. Simple 10 Hz
writer: all 3,423 observations persisted; p99 28 ms, maximum 323 ms.
- Simulated pre-backfill cursor: retained evidence ready 4m21s; full
catch-up 6m41s, 122 cache invalidations. Normal completed-migration
startup bulk-loads current evidence.

Portable opt-in scale tests and window benchmarks are included. All 183
frontend suites, full server tests, targeted races, lint and real-API
desktop/mobile checks passed. Full [Linux
CI](https://github.com/Kpa-clawbot/CoreScope/actions/runs/37063210229)
passed at `36def9c`; three independent reviews are clear. Local Windows
testing hit the unchanged symlink-privilege test. No configuration
changes.

Preflight overrides: external script/OpenClaw profile unavailable;
repository checks and Chromium used.
 2026-10-03 12:16:55 +02:00
n30nex 248d2045fd test(server): await indexes before node-path regression requests (#2084) 
Fixes #2083.

Node-path regression tests could request `/paths` while background
indexes were still loading, intermittently receiving HTTP 503 instead of
exercising hop resolution or sorting. Seven fixture loads now await the
existing bounded `WaitIndexesReady` signal. The anchor-bias test uses
the same signal instead of polling.

This changes five test files only. Production readiness behavior and
every HTTP/content assertion are preserved. No dependencies,
configuration or customizer changes.

## Validation

- Unchanged baseline: 94 passed / 6 failed across 100 targeted
executions; failures were actual HTTP 503 assertions.
- Fixed setup: 200/200 targeted executions passed.
- Parent independently ran the entire server suite: exit 0, 83.4
seconds.
- All 183 standalone frontend suites and 20 real Chromium route-map
checks passed.
- Formatting, whitespace and PII checks passed.

TDD justification: test-fixture synchronization repair, with no
production logic change. The existing unchanged behavioral assertions
supplied the before/after failure proof; no fabricated failing test was
added.

Local browser checks used Chromium against the unchanged production
source; OpenClaw profile/external preflight were unavailable. The
unrelated ingestor symlink test requires a Windows privilege absent
locally; Linux CI validates that suite. Final CI must pass before merge.
 2026-09-30 11:39:25 +02:00
AdamanietoClaude Sonnet 5
 		6d3da77b67 perf(channels): coalesce concurrent GetChannels/GetEncryptedChannels cache misses (#2059) 
Fixes #2029.

## What was wrong

`GetChannels`/`GetEncryptedChannels` (`cmd/server/db.go`) cache their
region-scoped result for 60s but had no request coalescing on a cache
miss, so every request that arrived while the cache was cold or expired
ran the region-scoped `GROUP BY` scan itself. Measured on production: 5
concurrent requests for the same never-cached region each took ~8s, no
cheaper than 5 independent runs.

`statsSF`/`regionMembershipSF` already fix the identical bug class
elsewhere in this file (#1910), so this wraps both functions'
query-build/execute/cache-populate block in a `singleflight.Group` the
same way, keyed per region, double-checking the cache inside the flight
in case a previous winner already refreshed it.

## Tests

The review on #2029 pointed out that a timing-based check ("finish
within ~2ms of each other") doesn't actually prove coalescing happened —
it would pass on a fast machine even without singleflight.
`db_channels_singleflight_test.go` uses a call counter instead, same
pattern as `TestEnsureNeighborGraph_Singleflight` (#1203 Pair A):

- `TestGetChannels_SingleflightCoalescesQueries` /
`TestGetEncryptedChannels_SingleflightCoalescesQueries`: 10 concurrent
callers against a cold cache, asserting the real query runs exactly
once. A test-only hook (`channelsQueryHook`/`encChannelsQueryHook`, nil
in production, same contract as `bgLoaderEntryHook`) increments the
counter right where the query executes, since these functions hit
`db.conn.Query` directly rather than going through an injectable builder
function.
- `TestGetChannels_SingleflightPerRegion`: two regions queried
concurrently (5 callers each) assert 2 queries, not 1 — pins that the
flight is keyed per-region and a caller for one region can't receive
another region's coalesced result.

Anti-tautology: reverting `channelsSF.Do`/`encChannelsSF.Do` back to a
bare call makes the coalescing tests observe N instead of 1.

`go build ./...`, `go vet ./...`, `gofmt -l .` clean. Full `cmd/server`
suite (race-enabled for the new concurrency tests) run in a
`golang:1.22-alpine` container, mounted repo, workdir `cmd/server` so
the sibling `internal/*` replace directives resolve:

```
=== RUN   TestGetChannels_SingleflightCoalescesQueries
--- PASS: TestGetChannels_SingleflightCoalescesQueries (0.06s)
=== RUN   TestGetChannels_SingleflightPerRegion
--- PASS: TestGetChannels_SingleflightPerRegion (0.05s)
=== RUN   TestGetEncryptedChannels_SingleflightCoalescesQueries
--- PASS: TestGetEncryptedChannels_SingleflightCoalescesQueries (0.07s)
```
Full suite: `FAIL github.com/corescope/server 98.261s`, but the only
failures are `TestHandleNodePaths_PrefixCollision_1352`,
`TestHandleNodePaths_FallbackUniquePrefix_1352`, and
`TestHandleNodePaths_FallbackUnresolvableHop_1352`, all failing on a
`503 {"error":"index loading","retryAfter":5}` — an index-build race in
this container's timing, not this change. Confirmed by running the same
three against an unmodified, freshly-cloned `master` in the same
container: they fail there too (plus
`TestHandleNodePaths_PrefixCollision_1352_FallbackBranch`, which this
run happened not to hit). Nothing in this diff touches node-path
handling.

## Not done

The deeper query-plan issue flagged in #2029 (the outer scan is driven
by `payload_type`, not region, so a cold solo request still costs
several seconds regardless of concurrency) is filed separately as #2058,
with `EXPLAIN QUERY PLAN` output and row counts against production data.
Coalescing makes one slow query serve everybody; it doesn't make the
query itself fast.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: anieto <anieto@meshtexas.org>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
 2026-09-23 10:05:59 +02:00
liquidraverandClaude Opus 5 b695b979a2 fix(nodes): keep paginating past a page that post-LIMIT filtering shortened (#2061) 
## Problem

`handleNodes` runs the geo-filter, `nodeBlacklist`, `hiddenNamePrefixes`
and area
passes **after** the SQL `LIMIT/OFFSET`, and rewrites `total` to the
filtered
length. A page that loses a row is therefore short **without being the
last
page**, and neither the page length nor `total` can tell a client
whether to ask
for another page.

#1606 added the pagination loop and chose the page length as the
canonical stop.
That is correct only where nothing is ever filtered. Everywhere else the
list
truncates at the first filtered page boundary and strands every node
behind it —
the #1598 symptom reached by a different route: a node that is relaying
right
now simply stops being in the list.

The comment at `app.js:240` rejects `total` for exactly the right
reason, then
picks the signal the same code path also breaks.

## Measured on a live 2346-node deployment

Page sizes for the query the map issues:

```
offset=0     returned=500     ← full, loop continues
offset=500   returned=499     ← one row filtered AFTER the LIMIT → loop STOPS
offset=1000  returned=500     ← never requested
offset=1500  returned=500     ← never requested
offset=2000  returned=345     ← never requested
```

| stop rule | requests | nodes reached |
|---|---:|---:|
| short page (master) | 2 | **999** |
| `has_more`, else empty page | 6 | **2344** |

**1341 nodes, 57%, unreachable through the UI.**

### One hidden node truncates the whole list

The deployment this came from has no `geoFilter`
(`/api/config/geo-filter`
returns `polygon: null`) and no `nodeBlacklist`. It has a single
`hiddenNamePrefixes` entry — a deliberate operator choice — and exactly
one node
whose name starts with it:

```
public_key   d4a46ea2…1054      (64 clean hex chars)
name         🚫🔥☀️
role         repeater
last_seen    2026-09-22T10:09:38Z
```

`handleNodes` drops that row in the `IsNameHidden` pass, which runs
after the SQL
`LIMIT`. The row is counted by the `LIMIT` and by `COUNT(*)`, so the
page it lands
in comes back exactly one short — and stops every client that treats a
short page
as the end.

Isolated against SQL on the same database, seconds apart:

```
SELECT lower(public_key) FROM nodes ORDER BY last_seen DESC LIMIT 500 OFFSET 500
  -> 500 rows
GET /api/nodes?limit=500&offset=500
  -> 499 rows
comm -23 sql.txt api.txt
  -> d4a46ea2e99cab132a3286ef3d9cce9099318790af7f25671fe83de453721054
```

Deterministic — `offset=500` returned 499 on three consecutive requests.
Not a
CDN artifact either: `cf-cache-status: DYNAMIC`, origin `cache-control:
no-store`, no `age` header, and four requests with deliberately unique
cache keys
all returned 499.

So **one deliberately hidden node makes 1341 of 2344 nodes
unreachable.** The
hiding feature does exactly what it was asked to do for that one node,
and takes
57% of the network with it, silently. A single `hiddenNamePrefixes`
entry is
enough; no geo-filter, blacklist or area filter is needed to reach this
state.

### The cutoff moves, which is why this reads as intermittent

The visible set is the sum of the pages up to and including the first
short one,
so the boundary sits wherever the unreturnable row currently sorts by
`last_seen`, and jumps a whole page as ingest reorders the list. Same
deployment, same code, same config, ~2h apart:

| dropped row's rank | first short page | nodes visible |
|---|---|---:|
| inside 0–499 | page 1 | 499 |
| inside 500–999 | page 2 | 999 |

A node is visible or invisible purely by where it lands relative to that
moving
line, so affected nodes appear to vanish and return on their own. Two
operators
on this deployment reported exactly that, independently, while I was
measuring.

### A named reproduction

`HU-ZA-Lentihegy` (`5287a33f…`), reported missing from the map by an
operator
whose companion had logged its advert at 04:20 local the same morning.

Ingest was fine. The row is in `nodes` with `last_seen`
`2026-09-22T02:20:35Z` — the same advert, to the second — valid GPS,
role
`repeater`, 1033 adverts, and `/api/nodes/search?q=lentihegy` returns
it.

```
rank by last_seen : 1081
cutoff at the time:  999
```

It missed by 82 positions. Walking the same live endpoint, same moment:

| stop rule | requests | nodes reached | Lentihegy |
|---|---:|---:|---|
| short page (master) | 2 | 999 | **not reached** |
| `has_more`, else empty page | 6 | 2340 | reached |

The practical shape of this on a busy mesh: 1081 nodes had been heard
more
recently than 9.4 hours, so on that deployment **anything last heard
more than
~9 hours ago was invisible**, alive or not.

`#/nodes` compounds it — its search box filters client-side over the
truncated
set, so the server-side `?search=` never runs and an operator cannot
find the
node by searching for it either, even though the endpoint would return
it.

## Change

**Server** — `NodeListResponse` gains `has_more`, computed from the raw
SQL page
against the real `COUNT(*)` before the filter passes run, so it survives
them:

```go
hasMore := offset+len(nodes) < total
```

Always emitted (no `omitempty`) so a client can tell `false` from an old
server.
No extra request in the fixed path: `has_more` ends the loop exactly,
where the
old rule needed a probe page.

**Clients** — `app.js` `fetchAllNodes`, `nodes.js` `loadNodes` and
`area-map.html`'s inline helper stop on `has_more`, falling back to a
zero-length
page against a server that predates it. An empty page always ends the
loop, so a
`has_more` against a concurrently-shrinking table cannot spin to
`safetyCap`.

Left alone: the three loops are still three copies. Collapsing them onto
`fetchAllNodes` is a bigger change than this fix needs, and `nodes.js`
has its
own inter-page progress UI. Happy to do it separately if you want it.

## Testing

- **Unit** (`tests/unit/test-fetch-all-nodes-pagination.js`): the
fixture now
models the real handler — a row counted by the LIMIT and by `COUNT(*)`,
then
removed from the page. Three new cases. Fails on the old rule at 499 of
1199.
- **E2E** (`tests/e2e/test-map-nodes-pagination-e2e.js`, already wired
into
  `deploy.yml`): the mock drops a page-1 row and emits `has_more`.
  Mutation-checked — restoring master's stop rule fails 3 of its steps.
- **Go** (`cmd/server/nodes_pagination_has_more_test.go`): asserts
`has_more`
stays true on a page filtering shortened. Mutation-checked — recomputing
it
  after the filter block fails the test.
- Full server suite `go test -race`: ok, 41.4s. `gofmt` clean, `go vet`
passes.
- **Against a real binary**, not just mocks: fixture DB migrated with
`corescope-migrate`, `hiddenNamePrefixes: ["SKCE"]`, `limit=3`. Page 1
returns
2 of 3 with `total` rewritten to 2 and `has_more=true`. Walking the real
server
with master's rule reaches 2 nodes; with `has_more`, all 199 visible of
200,
the hidden one still hidden. The real frontend against that server loads
199
  with no JS errors.

Two existing expectations changed, both deliberate:

1. `surfaces ALL nodes past the 500 server cap` — 3 → 4 requests. That
mock emits
no `has_more`, so the 200-row final page can no longer end the loop (a
short
page is exactly what a filtered page looks like) and a zero-length probe
   follows. Against a current server `has_more` still ends it at 3.
2. `rows missing public_key are NOT collapsed into one` — its stub
returned a
constant body, which would now be paged to `safetyCap`. It serves one
page
   then empties.

Local `test-all.sh` exits 1 on two XSS-gate self-tests
(`good-2-tested.js`, `good-4-tested.js`) via a `UnicodeEncodeError`
printing an
emoji under Windows cp1252. Identical on clean `origin/master` in a
scratch
worktree, so it is pre-existing and platform-local, not this branch.

There is a second identical filter block further down `routes.go` on
another list
endpoint. Likely the same class; not touched here.

If you would rather land your own version of this, say so and I will
close mine.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
 2026-09-23 09:45:18 +02:00
efitenandClaude Opus 5 61f565c606 fix(node-health): credit zero-hop adverts as direct reception (#2064) 
Reported by @dborup on #2057. The mechanism is real; the suspected scale
is not. Both parts measured below.

## The defect

`directHeardNode` rejects every non-flood route type before it looks at
the path, so the `hop == ""` branch that credits an advert's originator
can never run for a direct route. Zero-hop adverts — the clearest
direct-RF evidence the network produces — are discarded and the node is
listed under "Seen via relay" instead.

## Firmware

Read at `0679dbef` rather than taken on trust:

- `Mesh::sendZeroHop` sets `ROUTE_TYPE_DIRECT` and `path_len = 0`,
commented there as "path_len of zero means Zero Hop". The transport
overload does the same with `ROUTE_TYPE_TRANSPORT_DIRECT`.
- `examples/simple_repeater/MyMesh.cpp` sends the periodic **local
advert** through it (the `next_local_advert` branch), as does
`sendSelfAdvertisement` when `flood` is false.
- `examples/companion_radio/MyMesh.cpp` does the same for a companion's
own advert.

An ADVERT arriving on a direct route with an empty path therefore cannot
have been forwarded: the observer received the advertiser's own
transmission, and the advert carries its pubkey in the clear.

Every other direct case keeps #2057's rule. A non-empty path on a direct
route is the **remaining** route, because the forwarder ran
`removeSelfFromPath` before retransmitting, and `advertOriginPubkey`
already returns `""` for any payload type other than ADVERT — so the
payload guard costs nothing.

## Measured, 7-day window on a production instance

| | |
|---|---|
| zero-hop advert observations currently dropped | 8,166 |
| distinct nodes they evidence | 139 |
| node-observer pairs they evidence | 185 |
| pairs **not** already credited via an empty-path flood advert | **52**
|
| pairs currently credited from flood adverts | 217 |

So the fix restores 52 node-observer pairs of direct evidence that are
invisible today, roughly a quarter more advert-based direct evidence.

## What it does not explain

The report suspected this accounts for #2057's low headline numbers
("NL-BXE-RP01 | 433 → 0", 234 of 1,860 nodes with any direct observer).
The measurement does not support that:

- Of 30 sampled nodes with zero-hop advert evidence, **29 already show
at least one direct observer**, because they also send flood adverts
which #2057 credits.
- **NL-BXE-RP01 | 433 has zero zero-hop adverts** in the window. Its
empty list is not caused by this rule.

So this mostly enriches lists that are already non-empty, and flips few
cards from empty to populated. Worth doing on correctness grounds, not
as a fix for the counts.

## Tests

Four cases added to the `TestDirectHeardNode` table, which previously
covered direct routes only with `PayloadTXT_MSG`:

- direct + ADVERT + empty path credits the advertiser
- transport-direct + ADVERT + empty path credits the advertiser
- direct + empty path + **not** an advert credits nobody
- direct + ADVERT + **non-empty** path credits nobody

The last two matter as much as the first two: they pin the exception to
exactly the shape the firmware guarantees.

`gofmt` clean. Go tests not run locally (no cgo toolchain on this
machine since #1992, and per AGENTS.md `CGO_ENABLED=0` builds a stub
that proves nothing), so CI is their first run.

## Related

#2063 fixes the empty state's wording on the same card, which asserts
the node is out of range when the data cannot establish that. The two
are independent: this one adds evidence, that one stops overclaiming
when there is none.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-22 23:55:55 +02:00
efitenandClaude Opus 5 b614badb85 fix(packets): give each observation its own wire bytes in the detail API (#2055) 
Closes #1999.

## The defect, measured on a production instance

Packet `96d716f18d885e78` on a live deployment, read from the deployed
build's own API:

| | |
|---|---|
| observations in the response | 60 |
| distinct `path_json` values | 50 |
| distinct `raw_hex` values returned | **1** |
| distinct frames actually stored in SQLite | **51** |

The contradiction the issue describes, from that same response:

```
obs 38410791  path ["58C0","1403","50C7"]  ->  hex 094258C01403AF37E39624E548FB7575F195A0BF
```

Three hops in the path, two path bytes in the frame. The bytes belong to
the 2-hop observation and are served for all 60.

Across the 3000 most recent transmissions on that database: 1977 have
more than one observation and **1844 of those (93%) hold genuinely
different frames**. 32659 of 36133 observations (90%) differ from their
transmission's canonical bytes. This is the normal case, not an edge
case.

## Cause

The store deliberately does not retain `obs.RawHex`. #881 dropped it as
a memory optimisation, ~98MB measured on a 1.7M-observation store, on
the assumption that one content hash implies one frame. The firmware
hashes payload and type independently of the relay path, so that
assumption is false.

Worth adding to the issue's diagnosis: all four load and ingest paths in
`cmd/server/store.go` still `SELECT o.raw_hex` and scan it into
`obsRawHex`, then use it nowhere — LoadAll, loadChunk, `IngestNewFromDB`
and `IngestNewObservations`. The bytes are read out of SQLite and
discarded, so a cold load pays the transfer for nothing.

## The fix

Keeps the memory saving and reads the bytes back only where a human is
looking at one packet.

- **`cmd/server/db.go`** gains `ObservationRawHexForHash`: one query
returning the stored frame per observation id. Two indexed lookups
regardless of observation count — `transmissions.hash` through the
prepared `stmtTxByHash` (`idx_transmissions_hash`), then
`observations.transmission_id` (`idx_observations_transmission_id`).
Guarded by `hasObsRawHex`, because #881 made the column optional and the
query would be a SQL error without it.
- **`cmd/server/routes.go`** backfills in `handlePacketDetail`: once per
request rather than once per observation, and after the store lock is
released. Bytes already present are never overwritten, and an
observation with no stored frame still falls back to the transmission's.

Against the acceptance list: observation bytes exposed with canonical as
fallback only ✓; the store's memory optimisation untouched ✓; bounded
indexed reads with no query per observation and no work under the store
lock ✓; startup-loaded, newly ingested and DB-fallback details all
covered, because both the store path and the DB path converge on this
one backfill and both key observations by an int `id` ✓.

**No frontend change is needed.** `public/packets.js` already spreads
the selected observation over the packet (`{...pkt, ...currentObs}`) and
already reasons about per-observation bytes: the comment there says
"post-#882 per-obs raw_hex with a different path length than the
top-level packet's raw_hex still gets accurate byte highlights". The
client was built for this and has been receiving 60 copies of one frame.

## Tests

`cmd/server/obs_raw_hex_test.go`:

- the per-id mapping, with three distinct frames and a fourth
observation storing none
- the `hasObsRawHex` guard, so a schema without the column is not
queried
- the handler regression: each observation carries its own frame, the
frameless one falls back to the canonical bytes, and at least three
distinct frames come back across four observations — the last assertion
so that a regression to repeating one frame fails, rather than passing
on shape

## Verification

`gofmt` clean. **Go tests were not run locally**: no cgo toolchain on
this machine since #1992, and per AGENTS.md `CGO_ENABLED=0` builds a
stub that proves nothing. CI is their first run.

Browser validation per AGENTS.md rule 2: I verified **the defect** in a
real browser and through the deployed API, with the numbers above. I
could **not** validate the fix in a browser, because the change is
server-side Go and is not deployed anywhere yet. Saying so rather than
claiming otherwise.

## Not done

- The four scan sites that fetch `o.raw_hex` and discard it are left
alone. Removing the column from those query builders would stop
transferring roughly ten frames per transmission on every cold load, but
it touches four builders and their `scanArgs` alignment and is not
needed for this defect.
- `fetchResolvedPathForObs`, immediately next to this code in
`enrichObsWithTx`, does run one query per observation. This change
deliberately does not copy that pattern, and does not fix it either.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-22 11:08:17 +02:00
efitenandClaude Opus 5 d1b615fc0d fix(node-health): list only observers that heard the node on air (#2057) 
Closes #2056.

## What changes

The node detail "Heard By" card now lists only observers that received
the node's **own transmission off the air**, and reports the rest as a
count.

```
HEARD BY — DIRECT (8 OBSERVERS)
OBSERVER                REGION  PACKETS  AVG SNR   AVG RSSI
BE-DUF-SiSCD-01         —        16276    7.9 dB   -108 dBm
BE-BRU-Moris  repeater  —        13775   -5.6 dB   -122 dBm
...
Seen via relay by 29 observers. Those observers heard a repeater that
forwarded this node's traffic, not this node.
```

and for a node nothing hears:

```
HEARD BY — DIRECT (0 OBSERVERS)
No observer is within radio range of this node.
Seen via relay by 2 observers. …
```

## The rule, and where it comes from

Read out of the firmware rather than assumed:

| | |
|---|---|
| `Packet.h:83` | `setPathHashSizeAndCount(sz,n) { path_len =
((sz-1)<<6) \| (n&63); }` — hash size rides in the packet's `path_len`
byte |
| `Mesh.cpp:649,678` | only `sendFlood()` sets it, so the **originator**
decides; `CommonCLI.h:69` defaults `path_hash_mode = 0`, i.e. one byte |
| `Mesh.cpp:349` | a forwarding repeater appends its hash with the
packet's size — it cannot upgrade a packet, and the **last hop is who
was heard** |
| `Mesh.cpp:89,103` | on a direct route a forwarder matches the head of
the path and calls `removeSelfFromPath` before retransmitting, so the
path is the **remaining** route and the transmitter is not in it |

So an observation credits exactly one node:

1. Route type must be `ROUTE_TYPE_FLOOD` or
`ROUTE_TYPE_TRANSPORT_FLOOD`. Direct routes never qualify (38% of
transmissions over 7 days).
2. Empty path → the originator, known only for ADVERTs.
3. Otherwise the last hop.
4. The hop must resolve to exactly one candidate. Same gate
`resolvePathForObsColdLoad` already applies: under-attribute rather than
guess. It drops 418,530 of 1,455,721 flood observations with a path over
7 days (28.8%), and it is what stops the wrong-band credits.

## Measured effect

| node | before | after |
|---|---|---|
| BE-BRU-Moris | 36 observers | 3 |
| BE-KRO-RP01 \| ON1KW | 40 | 3 |
| BE-BRE-ON8AR | 38 | 2 |
| NL-BXE-RP01 \| 433 | 35 | 0 |

Network-wide over 7 days, 234 of 1,860 nodes have at least one direct
observer (161 have exactly one, maximum 8). The direct list is therefore
empty for most nodes, with the relay count below it. That is the correct
reading: no observer is in radio range of them.

Independent corroboration on staging: for BE-WIL-3EIK-01 the eight
direct observers are exactly the top eight entries of its Neighbors
table by score and observation count.

## Perf justification

`GetNodeHealth` is fast today precisely because it never walks
observations — it uses one representative observation per transmission.
Direct-RF needs the per-observation path, and that cannot be a
per-request walk: the reference store holds **232,928 transmissions /
2,887,861 observations**, one node's `byNode` slice alone holds **55,458
transmissions / 1,450,544 observations**, and
`/api/nodes/bulk-health?limit=200` would multiply that.

So the aggregate is rebuilt by a background recomputer on the existing
`newAnalyticsRecomputer` pattern, published into an `atomic.Value`.
Reads are `O(direct observers)`, which is **cheaper than before** — the
old code built per-observer sums over every transmission in `byNode` on
every request.

Proof, `BenchmarkBuildDirectHeardIndex`:

```
BenchmarkBuildDirectHeardIndex-12    1    63067900 ns/op
```

3,000,000 observations (60,000 transmissions × 50 observations, 8-hop
paths, 64 candidate repeaters) in **63 ms**, once per recompute
interval.

Per observation the walk does one route-type check, one backward scan of
`PathJSON` for the last quoted token (no allocation, no
`json.Unmarshal`), one prefix-map lookup and one counter update.

Rebuilding wholesale also means eviction needs no bookkeeping: a pass
simply does not see evicted transmissions. The alternative — a field on
`StoreObs` updated incrementally — would have needed the call at five
construction sites (`store.go:942,1264,2854,3179`,
`chunked_load.go:609`), which is the duplication that caused #1558, plus
matching decrements at eviction.

## API

Both `GetNodeHealth` and `GetBulkHealth` carried a near-identical copy
of the observer loop; they now share one builder.

- `observers` — direct-RF only. Same field names, so no client
migration. Rows are a named `HealthObserverRow` instead of
`map[string]interface{}` (one fewer occurrence in a touched file, per
the AGENTS.md ratchet).
- `relayObserverCount` — new integer, observers that saw traffic through
the node without hearing it. `stats.totalPackets` and `stats.avgHops`
still count relayed traffic, so without this number the card would
contradict the figures printed beside it.

`docs/api-spec.md` is updated for both endpoints. It also documented an
`iata` field on these rows that the endpoint has never emitted; removed.

## Tests

- `cmd/server/direct_heard_test.go` — table test over the rule: flood
with empty path and known originator, flood whose last hop is the node,
flood whose last hop is another node, direct and transport-direct routes
(never credit), ambiguous last-hop prefix, listener-only candidate,
1-byte and 2-byte hop sizes; plus aggregation and row-building.
- `cmd/server/node_health_direct_rf_test.go` — end-to-end through the
handler: an observer that only saw relayed traffic must not appear in
`observers` but must be counted in `relayObserverCount`. Plus the
benchmark.
- `tests/unit/test-direct-rf-heard-by.js` — slices the card template out
of `public/nodes.js` and evaluates it, so it tests the shipped markup
rather than a copy: heading, empty state, relay line, singular/plural,
signal columns, listener/repeater badge tri-state.
- `cmd/server/node_health_can_relay_case_1290_test.go` — updated to seed
a genuinely direct reception, since a relay-only observer no longer
carries a badge.
- `cmd/server/analytics_recompute_after_load_test.go` — recomputer count
10 → 11.

Verified locally: `cmd/server` suite green, `sh test-all.sh` green (180
suites), `tests/e2e/test-e2e-playwright.js` 131/134 passed with 3
skipped and 0 failures against the seeded fixture, plus
`test-issue-1147-section-order-e2e.js`,
`test-issue-1151-orphan-separators-e2e.js` and
`test-issue-1281-location-row-e2e.js`, which all assert on this card.
`gofmt` clean, `vet` clean across all modules.

Browser-validated on staging: both the full detail page and the side
pane, on a node with 8 direct observers and on the 433 MHz node with
none. No console errors.

## What this does not do

`prefixMap.resolveWithContext` still guesses on ambiguous hops, so
paths, neighbor edges and analytics keep their current attribution.
Making it abstain is a much larger change and needs its own issue.

The "Regions" line and Region column on this card read `o.iata`, which
this endpoint has never emitted, so both have always been dead. Left as
found rather than widened into this change.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-22 07:55:37 +02:00
efitenandClaude Opus 5 7c6b95ea53 fix(store): merge background chunks in order instead of prepending them (#2050) 
Fixes #2024.

`s.packets` is declared "sorted by first_seen ASC (oldest first; newest
at tail)" (`cmd/server/store.go:177`), and retention eviction depends on
it: `evictStaleInternal` walks from the head and stops at the first
transmission inside the window. A slice out of order is therefore
**under-evicted silently** rather than failing loudly.

## What breaks it

The background chunk loader. Chunks are windowed on `last_seen` (#1690),
so a transmission first heard weeks ago and heard again recently arrives
in a *recent* chunk carrying its old `first_seen`. The chunk was then
put in front of the slice:

```go
s.packets = append(localPackets, s.packets...)
```

and never re-sorted, so the next chunk, which covers an older window,
was prepended in front of it and left that ancient row sitting behind
newer ones. `LoadChunked` re-sorts after its own load; the background
merge did not. That asymmetry is the whole bug.

It is not a corner case. On a production database, of the **236080**
transmissions in a 14 day window, **2071** have a `first_seen` more than
a day older than their `last_seen`, and **1848** more than a week.

This matters more since #2035: with the accounting fixed, `maxMemoryMB`
actually triggers, and a walk that stops early works against it.

## The fix

`mergeChunkIntoPackets` merges the two sorted runs linearly. Re-sorting
the whole slice was not an option: this runs under `s.mu` once per
chunk, so it would sort hundreds of thousands of packets while ingest
waits for the lock. The chunk already arrives sorted, since the chunk
query ends in `ORDER BY t.first_seen ASC`, so the `sort.SliceIsSorted`
guard is a contract check costing one linear pass that never sorts in
production.

## Covered

- `TestMergeChunkIntoPackets_KeepsFirstSeenOrder` pins the merge against
an interleaving, deliberately unsorted chunk.
- `BenchmarkMergeChunkIntoPackets` guards the linear cost, against a
future simplification back into a sort.

The server suite runs under `-race` in CI and is green.

## Not covered, and I would rather say it than let the PR imply
otherwise

There is **no integration test driving `loadChunk` end to end**. I wrote
one and dropped it: a faithful seed database for that path needs more of
the schema and more of the loader's preconditions than the fix itself is
worth. Two CI rounds in, the seed was still loading zero packets (the
first attempt failed at `OpenDB` on a missing `nodes` table, the second
on the window). Both attempts are in this branch's history rather than
rewritten away.

So the end-to-end claim rests on the code path quoted above and on the
production measurement, not on a test that exercises it. The unit test
covers the function where the logic now lives, which is the part that
can regress.

Also not verified locally: `cmd/server` needs cgo for the #1992 driver
and this machine has no C toolchain, so CI is the check.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-20 15:12:36 +02:00
efiten e01565737a feat(ingestor): store CoreDrive RX region answers, with position, clock and retention (#2047) 
The Scope Audit page said a repeater's declared region list can come from CoreDrive RX while nothing the app sends ever reached it: the client-topic switch handled packets and rf only, so /regions was dropped without a log line, and node_declared_regions is read by region_keys.go and config.go but created by nothing in this tree. #2044 found that gap and proved it against a live instance.

This lands the implementation that has been carrying the feature in production on the ON8AR fork since 2026-09-06, the instance CoreDrive RX publishes to. Measured there: 1840 answers about 275 repeaters from 51 collectors, 2026-08-18 to 2026-09-19.

Beyond storing the answer it keeps three things the first version did not: position (lat, lon, pos_acc_m, filled on 1263 of 1840 rows, with acc_m dropped when the fix it qualifies was rejected), repeater_clock (filled on all 1840, so a wrong repeater clock cannot make an answer look newer than it is), and retention with per-collector history (pruneOldClientDeclaredRegionsAt bounds by age instead of keeping one row per target). On that dataset 138 of 275 repeaters have answers from more than one collector and 40 have collectors that disagree about the region list, which is the signal the Scope Audit exists to surface and which only survives while more than one answer does.

It gates on its own clientRegions block rather than riding on clientRxCoverage, so region answers can be accepted without GPS-tagged reception uploads, and AES block padding is trimmed from region names on ingest.

Taken from #2044 with the author credited as co-author: declaredRegionsTablePresent() and its test, a real bug this version lacked (supervisord starts both processes together, so a server that probes first ignores every answer until its next restart), and the docs/client-rx-coverage.md section.

Ported by cherry-picking the fork's nine commits rather than retyping, so this is the code that has been running. CI run 35434151026 is green: server ok 80.177s, ingestor ok 99.413s, race detector ok 112.406s, no --- FAIL lines.

Merged by the interim maintainer without a second human reviewer: CI and the production figures above are the independent checks.
 2026-09-19 11:44:47 +02:00
efiten aabeda0f2c test(server): set the #1239 lock-hold threshold from measurement, 150µs to 5ms (#2039) 
TestComputeAnalyticsDistanceLockHoldDuration failed on two consecutive master commits (5430bc79 at 222µs, 89377333 at 156µs), both passing on a re-run of the identical tree, neither touching cmd/server runtime code. The flat 150µs limit sat inside the healthy band.

Measured, not assumed:

  healthy    156µs, 222µs, 402µs   three commits, 402µs from run 35252186369
  regressed  201203µs              fork run 35252430017, RLock deliberately
                                   held across the whole compute

A factor of 500 apart, so the limit only had to stop sitting inside the healthy band. 5ms is 12x above the worst healthy reading and 40x below the measured regression. The four numbers and the run IDs are in the doc comment.

The first attempt (a1767c77) calibrated against a control where readers churned a second store the writer never locks, on the assumption that their CPU load was slowing the writer. CI measured that control at 0µs: the readers cost the writer nothing, the variance is lock handoff, and the control could not see what it was meant to subtract. 5e457961 replaces it. Both commits are kept in this branch's history, and issue #2038 is corrected where it argued against raising the limit.

Methodology untouched: same eight readers, same 200 writer cycles, same 20000 hops and 200 paths.

Merged by the interim maintainer without a second human reviewer. Not run locally: cmd/server needs cgo for the #1992 driver and this machine has no C toolchain, so CI (run 35253209412) is the check, and the mutation run above is what proves the assertion still fails on a real regression.

Fixes #2038
 2026-09-17 21:58:34 +02:00
Alex B e6323ec587 fix(store): account path, decode-cache and dedup-key bytes so maxMemoryMB eviction triggers (#2035) 
trackedBytes undercounted the packet store by about 2.5x, so packetStore.maxMemoryMB never triggered: a tx was charged at creation, before pickBestObservation set its path, so the byPathHop and spTxIndex costs were never added, and eviction then re-estimated with the path known and subtracted more than had been added, drifting the total downwards. The ParsedDecoded cache, the obsKeys dedup key and several per-observation strings were not estimated at all.

StoreTx.accountedBytes now records what was charged, rechargeTx returns the delta after every pickBestObservation, and eviction subtracts accountedBytes instead of re-estimating. Measured by the author on a production database copy: trackedMB 151 against 402 MB of heap in use before, 351 against 396 MB after, with GC cycles dropping from ~0.71/s to ~0.011/s over 12 h on their instance.

Reviewed by auditing the accounting rather than the arithmetic: all six production pickBestObservation sites recharge, all three subtraction sites read accountedBytes, every recharge site holds s.mu (Load from :857, the two ingest paths at :2778 and :3142 with deferred unlocks), observations are charged only after acceptance, and no charged tx is discarded during the chunk merge. That lock audit is the independent check, because CI's race job covers the ingestor only.

Operator impact, both from the estimate growing rather than any limit moving: where maxMemoryMB is set, eviction now caps the real store size, and the cold load clamp drops about a third of the boot walk (124420 to 84374 packets at 650 MB). Where it is unset, which is the default, the change is inert. docs/go-migration.md claimed the Go server ignored the setting, which was never true, and is corrected here.

Merged by the interim maintainer without a second human reviewer: CI (run 35258839322) plus the review above are the independent checks.
 2026-09-17 21:58:16 +02:00
Sylvain Rabot a2ea18f778 perf(sqlite): swap modernc.org/sqlite for mattn/go-sqlite3, cross-built with zig (#1992) 
Swaps the SQLite driver from `modernc.org/sqlite` (pure Go, SQLite
3.46.0) to `github.com/mattn/go-sqlite3` (cgo, bundled SQLite 3.53.4),
and pays the resulting cross-compilation cost with `zig cc`.

Draft because the riskiest part of this deletes rows — see [Please
review this part first](#please-review-this-part-first) — and because
three things remain unverified at the bottom.

`modernc.org/sqlite` is a transpilation of the C amalgamation. This repo
is read-heavy: `cmd/server` chunk-loads a graph at startup and fans out
neighbour/topology/analytics queries per request, and it pays for that
transpilation on exactly those paths. Head-to-head on the same
120k-transmission / 240k-observation database, running our own hot-path
SQL under both drivers (Apple M4, `-count=5`, medians):

| workload | modernc | mattn | |
|---|---:|---:|---|
| chunk load (`chunked_load.go` v3 join, 20k tx) | 449ms | 196ms |
**2.3×** |
| aggregate scan (240k-row join + `GROUP BY`) | 276ms | 137ms | **2.0×**
|
| 1500 prepared-statement lookups | 512ms | 403ms | **1.3×** |

Allocations fall with it: 1.12M vs 1.64M allocs and 21MB vs 30MB on the
chunk load.

**Superseded by a production run.** @efiten measured both drivers on a
real instance — 11,077,038 observations, 9.7GB database, 4-core arm64 —
as server-only containers against the same live volume, one at a time,
with round 2 reversing the order so the page cache favours the old
driver:

| | audit 7d | audit 24h | background fill (13 chunks) | start →
/api/health |
|---|---:|---:|---:|---:|
| modernc, round 1 | 16.67s | 2.27s | 130.2s | 16.6s |
| mattn, round 1 | 7.87s | 1.34s | 93.8s | 13.5s |
| mattn, round 2 | 8.15s | 1.35s | 96.4s | 13.0s |
| modernc, round 2 | 13.46s | 2.29s | 137.8s | 15.5s |

Warm, the old driver improves to 13.46s on the 7d audit and still loses
by ~1.8×. Chunk load is ~1.4×. `/api/nodes?limit=500` is 0.039s against
0.037s — nothing.

**So the real gain is ~1.4–1.8× on the paths that matter, not 2–2.3×.**
The shape the harness predicted holds — scans and joins gain, small
lookups do not — which is more reassuring than the magnitude would have
been. Quote these numbers.

**The counterweight**, cold and native on that machine: a build goes
from **52s to 163s**. An instance that builds its own image pays that
per deploy.

## The build is cgo now, and one thing about that is a trap

**`CGO_ENABLED=0` still builds.** mattn links a stub, and the binary
dies on its first query with `go-sqlite3 requires cgo to work. This is a
stub`. A green build is not evidence of anything here, which is why
`AGENTS.md` now says so explicitly. `GOOS=linux go build` genuinely
cannot cross-compile any more.

A new root `Makefile` is the entry point. `make crossbuild` uses `zig cc
-target {x86_64,aarch64}-linux-musl` and links static, so each artifact
stays a single self-contained file and the `alpine:3.20` runtime no
longer depends on the base image's libc at all.

`-Wl,-s` is load-bearing: Go's own `-s -w` does not reach the musl
objects zig links in, and without it the server binary is 19.8MB instead
of 12.1MB.

The Dockerfile keeps its single `$BUILDPLATFORM` builder — still no QEMU
for compilation — and gains a checksum-pinned zig plus BuildKit cache
mounts. The mounts are not a nicety: without them an image build
recompiles the amalgamation from cold and takes over half an hour.

## Please review this part first

`internal/dbschema/dedup_index.go` **deletes observation rows**. It is
the one part of this change that can lose data, and it exists because
the migration exposed a real bug rather than causing one.

`stmtInsertObservation` resolves its `ON CONFLICT` against
`idx_observations_dedup`, which `cmd/ingestor/db.go` only ever created
inside the branch that creates the `observations` table for the first
time. Any database whose table predates that branch never got one, so
the UPSERT had no conflict target. modernc failed on the first insert;
mattn fails at `OpenStore`. Same bug, found earlier.

Creating the index unconditionally repairs it — but the index is what
was supposed to prevent duplicates, so a database that never had it can
already hold rows violating it. **`test-fixtures/e2e-fixture.db` in this
repo holds one.** So duplicates are collapsed first. Refusing is not the
safer option: without the index the ingestor cannot prepare its UPSERT,
so it cannot start at all.

Replaying that UPSERT faithfully is subtler than it looks, and a first
cut of this got it wrong twice:

- `COALESCE(excluded.x, x)` means the **incoming** value wins, so down a
group in id order the survivor keeps the **last** non-NULL value. Taking
the first silently discarded newer readings.
- The UPSERT names exactly five columns (`snr`, `rssi`, `score`,
`raw_hex`, `resolved_path`). Every other column must keep the surviving
row's own value; merging those too invents history the ingestor would
never have written.

Merge, delete and `CREATE UNIQUE INDEX` now share one transaction. Split
apart, a writer inserting a duplicate in the gap fails the index
creation while leaving the deletions committed — rows destroyed and no
index to show for it.

Cost, measured on 2.4M synthetic rows holding 5 duplicates: **4.1s**,
holding the write lock throughout, once, at ingestor startup before MQTT
subscribe. Materialising the duplicate-group scan once rather than per
column took that from 9.7s; the pathological case (400k of 600k rows
duplicated) is 5.7s, slightly worse than the 4.2s it was before that
change.

## Four more behavioural differences

Full detail in `docs/sqlite-driver-migration.md`. Briefly:

**Statement preparation is eager.** modernc's `newStmt` stored the SQL
and compiled lazily; mattn calls `sqlite3_prepare_v2` inside `Prepare`,
so SQL naming a missing table fails at *open*. 59 server tests failed on
this alone, all fixtures with partial schemas. `OpenDB` keeps failing
loudly (#1901; `main.go` gates on `dbschema.AssertReady` anyway) and the
fixtures now declare what they are prepared against via
`ensurePreparable`. This also exposed nine `nodes(pubkey …)`
declarations across seven files, where production has only ever had
`public_key` — lazy compilation had hidden the mismatch for as long as
it existed.

**`synchronous` silently dropped FULL → NORMAL.** mattn defaults it to
NORMAL and executes the pragma unconditionally, where SQLite's own
default (what modernc left alone) is FULL. In WAL mode that weakens
durability under power loss. Pinned in `dbschema.WriterDSN`, which both
writers now share — `cmd/migrate` kept a bare path at first and so
quietly wrote at NORMAL, which is what a second copy of a DSN buys you.

**The DSN dialects are mutually invisible.** modernc understood only
`_pragma=name(value)`, mattn only `_`-prefixed parameters, and neither
errors on the other's form — a driver-only rename would have dropped
every pragma in silence. `_journal_mode=WAL` is also gone from the
server's read handle: modernc ignored it, mattn honours it, and setting
`journal_mode` on a read-only connection is a write. Dropping
`_busy_timeout` with it costs nothing, since mattn already defaults to
5000ms — which means the read handle finally *gets* the busy timeout it
had silently lacked.

**`mode=ro` survives for a non-obvious reason.** mattn always passes
`READWRITE|CREATE` and its amalgamation has `SQLITE_USE_URI=0`; what
makes the URI work is its C wrapper ORing `SQLITE_OPEN_URI` in. So the
#1283/#1289 invariant holds with no build flags — but it depends on the
`file:` prefix. `cmd/decrypt` had been building its DSN without one, so
its `mode=ro` had never applied and a missing path was created
read-write. Fixed in passing; never a migration regression.

## What did not change

No modernc-specific API was in use: no `RegisterFunction`, no
`*sqlite.Conn`, no `sqlite/lib` error constants, no `sql.Register`. No
`time.Time` is ever bound as a query argument, so driver time handling
is not in play. Both drivers convert declared
`DATE`/`DATETIME`/`TIMESTAMP` columns to `time.Time`, so
`/api/dropped-packets` keeps emitting `dropped_at` as RFC3339 — an
earlier draft "fixed" that with a `CAST` and would have been the
regression.

## Tests and CI

New regression tests, each written because something got through without
it:

- `TestEnsureObservationsDedupIndexKeepsLatestValues` — the merge
ordering. The original test used complementary NULLs, which passes
whichever direction you pick, which is why the bug survived it.
- `TestCollapseDuplicatesAndIndexIsAtomic` — a failed index creation
must roll the deletions back.
- `TestOpenStorePragmas` / `TestWriterDSNPragmas` — every writer pragma,
read back through the store's own connection. A separate `sqlite3`
session or the startup log line would prove nothing.
- `TestOpenDBRefusesMissingDatabase` — the read-only invariant, which
now rests on a detail of the driver's C wrapper.
- `TestEnsurePreparableMatchesPrepareStatements` — fails when a new
prepared statement outgrows the fixture helper.

CI gains test execution for `cmd/migrate` and `internal/dbschema`, which
had none and both open the database. A PR-time two-arch build plus an
arm64 QEMU smoke gate is new: the GHCR push is push/tag-only, so without
it nothing on a PR would exercise zig, static musl linking or arm64, and
the first signal would arrive on master. `cache-dependency-path` widens
from 2 of the 5 tracked `go.sum` files to all of them.

`make test` passes across all 14 modules, `cmd/server` also under `-race
-count=2` with no failures and no races. `gofmt` and `go vet` clean.
Release-routing and Dockerfile COPY-invariant gates pass.

## Verified by running

- All 8 cross-builds static and correct-architecture; both arches of the
container image built, exported and run under QEMU, serving
`/api/health` and `/api/nodes` against a 2.9M-observation production
snapshot.
- The `migrate` binary repairing that snapshot's duplicate on bare
Alpine.
- `CGO_ENABLED=0` producing a binary that builds and then fails on first
query.

## Not verified

- ~~The 2–2.3× figures come from a standalone harness, not this load
under the old driver.~~ **Closed** by @efiten's production run above,
which also corrected the multiplier.
- SQLite 3.46.0 → 3.53.4 query-planner differences on queries with no
total `ORDER BY`.
- Sustained live ingest through the new writer DSN, and the duplicate
collapse against a database an ingestor is actively writing to. Verified
against a static snapshot only, and the collapse is measured at 4.1s on
2.4M synthetic rows with 5 duplicates — well short of an 11M-row
instance. @efiten has offered a staging instance taking real MQTT
traffic; **this is the item to close before the PR leaves draft.**

An earlier revision of this branch shipped the dedup merge in the wrong
direction with a green test suite, and review then found three more
things in the same file: the repair gated on an error string, a
non-atomic TEMP table drop aimed at the wrong connection, and a deletion
whose only record was a row count. All fixed in ac7e8d38. Passing tests
did not establish safety here, which is why the deletion path wanted a
second pair of eyes rather than a rubber stamp.
 2026-09-16 09:02:13 +02:00
efitenandClaude Opus 5 52b9474d7a feat(map): filter repeaters by region name (#1862) (#2022) 
Fixes #1862

## What

Adds a **Region Scope** picker to the map controls: pick `#be` and the
map keeps the nodes that declare `#be` or were seen carrying `#be`
traffic. It combines with the #2006 scope-state filter and persists in
localStorage the same way. While a region is picked, a small "Region:
#be · reset" chip sits on the map itself, so the filter stays visible
when the controls panel is collapsed (the default on phones) and can be
cleared from there.

## API

`/api/nodes` and `/api/nodes/{pubkey}` gain two fields on repeater/room
rows:

- `declared_regions`: named regions from the node's newest
declared-regions answer, split by the same function the Scope Audit now
uses for `declaredRegions` (`splitDeclaredRegions`,
`cmd/server/scope_config_state.go`), so both pages list a repeater under
the same names. `[]` means it answered and named no region. Absent means
it never answered, other roles, no declared-regions source, or the
declared-regions lookup failed.
- `declared_regions_truncated`: present, and `true`, only when that
answer was flagged as truncated, so the list is partial. Never `false`:
the `nodes.configured_scope` source does not record truncation, so
absence does not mean the list is complete.

The observed side reuses `transported_scopes`. Documented in
`docs/api-spec.md` and the served OpenAPI spec.

### Why no `?hashRegion=` query parameter
The observed side lives in the in-memory store. Filtering it after the
SQL `LIMIT`/`OFFSET` would corrupt `total` and paging, and the map pages
through `/api/nodes`. Same reasoning as the Data path section of #2001.

## Map behaviour

- Filtering is client-side over the nodes `fetchAllNodes` already
loaded: no new request. One pass over the loaded nodes to build the
picker counts, one Set lookup per node per render. The marker filter is
`nodePassesMapFilters` (`public/map.js:219`) and the observer stand-down
`observerLayerShown` (`:212`), both exported and tested.
- The picker and hint count only nodes with a map position, the same
test the marker filter applies first, so a count never promises markers
the map cannot draw.
- The observer layer stands down while a region is picked, for the same
reason it does for the scope-state filter.
- The popup lists declared and observed regions separately. A truncated
declared answer carries the same `truncated` badge the Scope Audit
shows.
- Absence is not read as a finding: the hint under the picker says a
node left off the map is not proof it lacks the region.

## Tests

- Go: `node_declared_regions_api_test.go` covers `declared_regions` on
list and detail endpoints, the no-source case, agreement with
`/api/scope-audit`, `declared_regions_truncated` (truncated,
truncated-empty, complete, configured_scope-only, newer untruncated
answer, companion) and the OpenAPI schema.
- JS: `test-issue-1862-map-region-filter.js` (30 tests) covers the pure
pieces (evidence, counts, options, hint, popup rows,
`nodePassesMapFilters`, `observerLayerShown`) and, at page level, runs
the registered map page through `init()` and `loadNodes()` in a vm
sandbox: picker built from loaded nodes, markers filtered by a stored
region, observer pins standing down, popup rows, the change handler
persisting, and the chip showing, resetting and rendering its text as
text.
- Mutation-checked: removing the region check, the observer stand-down,
the picker build in `loadNodes`, the popup rows, the persist on change,
the chip reset, or the truncated flag (Go or JS) each fails a test.
`test-issue-2001-map-scope-state.js` still passes.
- `go test ./...` in `cmd/server` passes; `check-css-vars` and
`check-xss-sinks --diff` are clean.

## Staging validation
Build `c646310f`, Chrome, no console errors:
- Picking `#be`: chip "Region: #be · reset" at the top of the map, hint
"221 nodes with a map position have evidence for #be: 129 declare it,
191 seen carrying its traffic. Absence here is not proof: ...".
- Clicking reset: picker back to "All regions", stored choice cleared,
chip hidden.
- First version, same instance: `declared_regions` on 136 of 500
`/api/nodes` rows; returning to "All regions" blocked the main thread
5.4 s against 3.2 s for the existing Status filter returning to "All".
The review measured the region filter's own added work at about 0.08 ms
per render plus popup rows that are already built for every marker, so
most of that time is the existing full re-render.

## Not verified
- The chip placement at phone width, and on narrow desktop widths where
it may sit under the expanded controls panel.
- The truncated badge with real truncated answers (staging has none
today).
- `test-map-clustering.js` has one failing test on `upstream/master`
too; untouched here.
- Marker badges from the original issue body are not implemented; the
popup rows are the per-node display.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-13 21:09:51 +02:00
efitenandClaude Opus 5 efdb3ea0b3 feat(analytics): retransmission pressure over time (#1699) (#2023) 
## Summary
Adds `GET /api/analytics/retransmissions` and a "Retransmission Pressure
(proxy)" chart on the Analytics Topology tab, implementing the metric
agreed in #1699: for each flood, the number of distinct repeaters in the
union of the paths of all its observations (`[A]`, `[A,B,C]`, `[A,D]`
gives 4), averaged per time bucket.

Topology is the tab that already shows hop counts and repeaters in
paths, so the chart sits there instead of in a new tab.

## Definition
- Flood routes only (`route_type` 0/1); TRACE excluded. Direct routes
carry the route still to travel (firmware
`src/Mesh.cpp:78-106,334-342`), zero-hop sends are direct
(`src/Mesh.cpp:717-737`), TRACE path bytes are SNR values
(`src/Mesh.cpp:59-61`, refused by `sendFlood` at
`src/Mesh.cpp:637-641`). Firmware commit 0679dbef.
- **Flood events, not hashes.** `transmissions.hash` is UNIQUE and the
packet hash excludes the path (`src/Packet.cpp:41-50`), so when the same
bytes flood again the observations land on the same transmission.
Observations are sorted by time and split into events wherever two
consecutive observations are more than 5 minutes apart. Each event is
counted on its own and bucketed by its first observation.
- Why 5 minutes: a node holds a flood for at most 32 s
(`src/Dispatcher.cpp:11,243-251`) plus a random retransmit delay. On
live over 7 days, 72,806 of 74,347 flood transmissions span 60 s or
less, and of 1,372,283 consecutive observation gaps, 52 fall between 60
s and 300 s against 1,823 above 300 s.
- Events that start before the store retention floor (now minus
`retentionHours`) are left out for every request shape. The store keeps
older observations only for hashes heard again recently, so they do not
represent that period. Eviction of those transmissions is tracked in
#2024.
- A flood event heard only with an empty path counts as 0 repeaters.
- **Prefixes are not resolved to nodes, and a prefix counts once per
event**, whether it repeats across observations or inside one path. On
live (7 days), a repeated 2-byte prefix inside one path occurs in 1.08%
of flood transmissions and 6,178 of 6,596 such repeats match exactly one
known node; for 3-byte it is 0.69% and 104 of 104. That is one node
forwarding again after its 160-slot cyclic duplicate filter dropped the
hash (`src/helpers/SimpleMeshTables.h:9,52-57`). A repeated 1-byte
prefix (44.9% of 1-byte transmissions) is mostly two nodes; counting it
once keeps the value a lower bound. `summary.one_byte_packets` reports
how many events that affects.
- Observations are stored once per observer and path per hash, so a
later event of the same hash only holds pairs not stored before; its
count is a lower bound too. On live these are 1,466 of 75,356 events
(1.9%), and they are kept in the average.
- Resolution was not used: on live, 1-byte observations nearly all have
`resolved_path` NULL, and cold load refuses context-based resolution of
history (`cmd/server/neighbor_persist.go:155-168`).
- Buckets `5m|15m|1h|6h|1d`. `region` filters on observers like
`/api/analytics/rf`, after the event split; a region with no known
observers is not filtered, the same as the other analytics endpoints.
`area` is not supported.

## Implementation
- `cmd/server/retransmission_pressure.go:255` `addPath`: scans path JSON
directly into a generation-stamped hash set, no allocation per
observation.
- `cmd/server/retransmission_pressure.go:367`
`computeRetransmissionPressure`: one pass under `s.mu.RLock`. Per flood
transmission it sorts the observations by cached parsed time into a
reused scratch slice, splits events and counts each in `addEvent`
(`:319`). O(T + O log k + H).
- `cmd/server/retransmission_pressure.go:470`
`GetRetransmissionPressure`: default shape from the recomputer (#1659
warm-up gate). Other shapes come from a typed TTL cache (max 64 entries)
cleared on new paths and eviction (`cmd/server/store.go:2289,2336`);
concurrent misses on one key share one compute through singleflight
(`store.go:204`).
- `cmd/server/retransmission_pressure.go:515` handler,
`cmd/server/routes.go:331`, `cmd/server/openapi.go:108`,
`docs/api-spec.md:1283`.
- `public/analytics.js:761` card, `:855` `renderRetransmissionChart`
(CSS variables only, lines break at missing buckets, caption states it
is a proxy, names the observer coverage bias, the once-per-flood prefix
rule and the 5 minute event split), `:829` loader with stale-response
guard.

## Performance
- `BenchmarkComputeRetransmissionPressure`, 50k transmissions x 20
observations, `-cpu 1`, i5-1335U: median 161 ms/op (132 ms/op before the
event split); first pass after startup with timestamps not yet parsed
192 ms/op. About 22 KB and 281 allocations per op.
- On staging the default shape is served from the recomputer in 0.3 s;
the post-load recompute of this recomputer took 994 ms on a
121k-transmission store (log line quoted in #2025). A 336h store would
be about twice that, every recompute interval, under the store read
lock.

## Tests
- `cmd/server/retransmission_pressure_test.go`: union counting (reporter
example, overlaps, once per event for 1/2/3-byte, width/case, growth);
route/TRACE/zero-hop filter, bucketing, window by event start; event
split and the 5 minute settle gap (boundary, chained steps, unsorted
input), retention floor; region filter, region applied after the split,
unknown region, 1-byte share; recomputer read, TTL cache invalidation on
new paths and on eviction, cache expiry, singleflight, recomputer gate
wiring, handler, warm-up gate.
- `test-issue-1699-retransmission-chart.js` (33 tests, registered in
`test-all.sh` and `deploy.yml`).
- Mutation-checked: 18 mutations of the event split, floor, prefix rule,
bucketing, region order, cache clears, expiry, gate wiring and
singleflight, all killed.
- `go test ./...` in cmd/server passes; `scripts/check-css-vars.js` OK.

## Staging validation
Build `c646310f` (this rework plus #2025 and the other review
follow-ups), after a container restart and full load: default shape
74,974 flood events, average 27.08 repeaters, 169 hourly buckets from
2026-09-06 16:00 (the 168h floor) to the current hour, highest hourly
average 53.3. Before the rework the same instance showed buckets back to
2026-07-18, averages up to 148, and for the first minutes after a
restart only 5,911 packets.

## Merge order with #2025
#2025 fixes the recomputer startup for all analytics endpoints (the
stale first snapshot seen here). Whichever of the two merges second has
to add `recompRetransmissions` to `analyticsRecomputersLocked`, wire it
to that PR's `loadedGate` instead of `LoadComplete`, bump the recomputer
count in `TestAnalyticsRecomputers_PostLoadOrder` from 9 to 10, and make
`TestStartAnalyticsRecomputers_RetransmissionsGatedOnLoadComplete` call
`signalStartupLoadDone()`. That resolution is what ran on staging.

## Not verified
- Recompute timing on a production-size (336h) store; only extrapolated.
- Phone-width layout and dark theme of the reworked chart.
- E2E Playwright suite.

Fixes #1699

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-13 20:37:14 +02:00
efitendborupClaude Opus 5
 		914bd4cf0e feat(channels): show each message's region (#1851) (#2018) 
## Show each channel message's region scope

Each message in the Channels view now shows the region scope it was sent
with, as a small chip in the meta line: the region name (for example
`#be`), `unknown scope`, or nothing.

This is the channel-message part of #1852 by @dborup, extracted as a
focused change. #1852 was closed unmerged because it had grown to the
whole fork diff. The implementation follows dborup's commits c686ae3f,
350bf7ee and a94d57ed on dborup/CoreScope, adapted to current master.
dborup is co-author on the commit.

### What changed
- `cmd/server/db.go:2099,2195`: `GetChannelMessages` selects
`t.scope_name` when the column exists and returns it as `scope_name`.
- `cmd/server/store.go:5654`: the in-memory `GetChannelMessages` returns
`scope_name`, so the field does not depend on which path serves the
endpoint.
- `cmd/server/store.go:2966,3244`: both WebSocket broadcast builders
carry `scope_name`, so a live message shows its region immediately.
- `public/channels.js:342,2278`: `messageScopeChipHtml` renders the chip
with the existing `.sa-chip-declared` / `.sa-chip-unmatched` styles from
`scope-audit.css`. The name goes through `escapeHtml`. No new CSS.
- `public/channels.js:678,695,1435,1487`: the decrypt path and the
WebSocket path keep `scope_name` on the message.

### Differences from #1852
- The field is `scope_name`, the name `/api/packets` already uses.
- No `routeType` field. `transmissions.scope_name` already tells the
states apart: NULL means no transport code, an empty string means a
transport code that no configured region key matched. The frontend uses
`??`, not `||`, so the empty string is kept.
- A chip instead of `scope: <name>` text. The area label from later
#1852 commits is not included.

### Perf
One extra column per observation row in the page query (at most `limit`
transmissions), and one extra map entry per broadcast observation. No
new queries, loops or API calls.

### Tests
- `cmd/server/channel_message_scope_name_test.go`: the three states
through the DB query, the store, `/api/channels/{hash}/messages` over
both paths, a schema without the column, and both broadcast builders. 5
of its 6 tests fail without the change; the sixth guards the
missing-column case and passes either way.
- `test-issue-1851-channel-message-scope.js`: the REST, WebSocket and
client-side decrypt paths, escaping, and the name / unknown / none
render. 4/4 fail without the change. Registered in `test-all.sh` and the
unit step of `deploy.yml`.
- Mutation checks: returning `nil` for `scope_name` in the DB path fails
the DB and endpoint tests; `||` instead of `??` in the WebSocket path
fails the WebSocket test.
- `go test ./...` in `cmd/server`: ok. gofmt and go vet clean.

### Browser validation
On a staging instance with live traffic (build `e84d2da6`), in Chrome:
- `/api/channels/{hash}/messages` carries the `scope_name` key on every
message in the 19 channels whose results I read. `#hamradio`, latest 50:
34 named, 1 empty string, 15 NULL.
- Opening `#hamradio` renders 104 chips: `#nl` 53, `#be` 32, `#de` 11,
`#eu` 6, `#bx` 1 and `unknown scope` 1, and no chip on unscoped
messages. Chip text `rgb(26, 26, 46)` on `rgb(238, 242, 255)` in the
light theme.

### Not verified
- Dark theme not checked.
- The real-decrypt branch of `decryptCandidates` has no test and was not
exercised in the browser; the already-decrypted branch is tested.
- Messages already in the client decrypt cache show no chip until they
are decrypted again.
- `go test -race` and the Playwright E2E suite were not run locally.

Fixes #1851



## Review follow-up (commit `50346589`)

An independent review found no correctness or XSS problem and confirmed
DB, store and WebSocket agree on the value. Changed:

- **Real decrypt branch tested.** A new test runs the real AES+HMAC
decrypt branch in `decryptCandidates` with one packet per scope state;
deleting `scope_name` there now fails 2 of 7 tests.
- **Tooltip wording.** The unknown-scope tooltip now says the scope
"could not be matched to a single region on this instance"
(`public/channels.js:338-347`). The ingestor stores an empty name both
when no key matches and when several match without exactly one
operator-configured key (`cmd/ingestor/region_keys.go:364-393`), so
"matches none of the configured keys" was wrong for the second case.
- **Old decrypt cache.** Decrypted messages cached before this change
had no `scope_name` key and stayed chipless as long as the candidate
count did not change. A cached message missing the key now forces one
full decrypt; a cache that has it still takes the delta path. A test
covers each case.
- **Docs.** `docs/api-spec.md` documents `scope_name` on the channel
messages response, with the null / empty string / name semantics.

Corrections to the description:

- **Test counts.** With the `db.go` and `store.go` changes reverted, 4
of the 5 top-level Go tests fail (6 of 7 counting subtests); only the
missing-column test passes.
- **Broadcast payload.** `scope_name` is added to `pkt`, which is copied
into `broadcastMap` and also nested as `packet` (`store.go` ~2974-2980,
~3252-3257), so the key appears twice per observation: 36 bytes for
`null`, 48 bytes for `"#belgium"`.
- **Side effect on the Packets page.** The live table reads
`m.data.packet` (`packets.js` ~1316-1318), so flat rows and expanded
group children now show Scope for live packets. In grouped mode a new
group copies a fixed field list without `scope_name` (~1384-1395) and
shows the empty placeholder until reload. Before this PR every live row
showed that placeholder, so this is not a regression.

The three copies of the three-state scope rendering (`app.js`,
`packets.js`, `channels.js`) are left as they are.

---------

Co-authored-by: dborup <3627142+dborup@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-13 20:36:34 +02:00
efitenandClaude Opus 5 a059299588 feat(node-analytics): hop-count statistics per node (#1812) (#2021) 
## Summary
Adds per-node hop-count statistics so repeater operators can choose
`flood.max`, `flood.max.unscoped` and `flood.max.advert` from what their
node actually sees.

- New endpoint `GET /api/nodes/{pubkey}/hop_analytics?days=N`
(`cmd/server/routes.go:299`, `cmd/server/node_hop_analytics.go:312`),
separate from `/analytics` as requested in the issue.
- New card "Hop Count at This Node" on the node analytics page
(`public/node-hop-analytics.js`, wired at
`public/node-analytics.js:130,174`): histogram of hop counts with a box
plot on the same x axis, filters `flood.max` (default),
`flood.max.advert`, `flood.max.unscoped`, driven by the existing range
picker.
- The existing "Hop Distribution" chart is unchanged: it shows path
length at the observer, a different quantity.
- No `direct` tag, although the issue lists one: for DIRECT packets the
path is the remaining route and no flood limit applies, so there is no
hop count to report.

## Hop count definition (firmware 0679dbef)
- `src/helpers/RoutingPolicy.h:15-21`: limits compare
`getPathHashCount()`; `.unscoped` applies to route type FLOOD, `.advert`
to adverts.
- `src/Mesh.cpp:344-350`: `routeRecvPacket` checks with n hashes in the
path, then writes its own hash at index n. So hops = the node's
zero-based index in the path, no +1.
- `src/Mesh.cpp:265-285`: a node forwards a flood once;
`src/Mesh.cpp:651,680`: an originator never forwards its own flood.
- DIRECT packets are excluded: their path is the remaining route
(`src/Mesh.cpp:78-103,334-341`).

Response: `{timeRange, packets: [{hash, timestamp, hops, tags}],
ambiguous}`. Tags: `flood`, `scoped` or `unscoped`, `advert`. Documented
in `docs/api-spec.md:679` and `cmd/server/openapi.go:90`.

## Attribution
`cmd/server/node_hop_analytics.go:198-309`. The result depends only on
the observed paths, the prefix map and the neighbor graph, so it is the
same after a restart as after live ingest.

- Every observation of every flood packet in the window is read.
`byNode` holds the server resolver's pick at ingest and other picks
after a cold load; `byPathHop` indexes only each packet's longest path,
which for a busy relay often runs through another branch of the flood.
- A packet counts when the node's prefix sits at exactly one index
across its observations, and either the node is the only relay candidate
for that prefix (`prefixMap.relayCandidates`,
`cmd/server/store.go:6795`), or the hop resolves to the node under the
ingestor's strict rule (`cmd/ingestor/path_resolver.go:143-214`) in at
least one observation and to another node in none. Strict rule: earlier
hops identified without a tiebreak, exactly one candidate adjacent in
`neighbor_edges` to the previous hop (the originator for hop 0 of an
advert), nodes already on the path excluded.
- The server resolver's tiebreaks (affinity, GPS distance, advert count,
pubkey order) are not used.
- Everything else with the node's prefix goes to `ambiguous`. In
practice that is most packets with a colliding 1-byte path hash.

On a read-only 7-day dump of a 1,669-node mesh DB, for one busy
repeater: 23,081 packets attributed, 11,437 ambiguous. Taking candidates
from `byPathHop` instead gave 9,995 attributed, with the histogram mode
moved from 2 to 3-5 hops.

## Performance
Scans `s.packets` under the read lock, no SQL per packet. Per
observation: one substring test for the node's first prefix byte; the
hop scan only for observations containing it; the strict walk only for
colliding prefixes, with per-request caches for candidates and
adjacency. `BenchmarkNodeHopPackets` models one 7-day request at that
scale (73,782 flood packets, 1,430,280 observations): 44-87 ms/op, 13.4
MB, 40 allocs on a throttling laptop.

Response size for that repeater over 7 days: about 23k entries, 2.3 MB
JSON, 375 KB gzipped. `hash` and `timestamp` are 61% of the raw and 91%
of the gzipped bytes; they stay because the issue asks for them so a
client can join entries to packets and bin by time.

## Tests
- Go: `cmd/server/node_hop_analytics_test.go`: 12 unit tests, a
live-ingest test through `IngestNewFromDB` (a colliding prefix without
independent attribution goes to `ambiguous`, not to the node the
resolver picked), live ingest versus cold load of the same DB, route
test, benchmark. 15 mutations of the attribution logic each fail a test.
- JS: `test-node-hop-analytics.js` (filters, histogram, quartiles and
whiskers with a fixture that separates 1.5 IQR from 3 IQR, render),
registered in `test-all.sh` and `.github/workflows/deploy.yml`.
- `gofmt`, `go vet ./...`, `go test ./...` in `cmd/server`,
`scripts/check-css-vars.js` pass.

## Staging validation
Build `c646310f` (this PR's review follow-up together with the other
open follow-ups), after a container restart and full load, on a busy
Belgian repeater:

- `hop_analytics?days=7`: 23,302 packets, 11,548 ambiguous, median 4,
adverts never above hop 7 (matching the firmware default
`flood_max_advert = 8`, `examples/simple_repeater/MyMesh.cpp:922`), 1.2
s. The first version reported 23,035 packets and 86 ambiguous in 534 ms,
because it trusted the resolver's pick for colliding prefixes.
- The card rendered on the first version with no console errors; the
rework does not touch the frontend beyond a test fixture.

## Not verified
- Response time and lock hold for 30 days on the busiest node on a
14-day store.
- Server relay candidates exclude companions and listeners while the
ingestor's prefix index does not, so a few strict attributions can
differ from the ingestor's persisted `resolved_path`.
- Identical numbers across a second container restart were shown in a Go
test, not repeated on staging.
- Dark theme, phone width, and switching the range picker in the
browser.
- Filter state is not reflected in the URL hash (the range picker is not
either).

Fixes #1812

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-13 19:59:56 +02:00
efitenandClaude Opus 5 2d4019f719 fix(analytics): recompute once the store has fully loaded (#2025) 
Refs #2023, #1659, #1724

### Problem
`main.go:258` waits only for the first load chunk, then `main.go:402`
starts the analytics recomputers. `Start()` computes immediately on that
chunk (`analytics_recomputer.go:86` on master) and the next compute
waits a full interval (`:93`, 5 min default). The chunk loader walks by
ascending id, so that chunk holds the oldest transmissions.

- RF, topology, channels: the #1659 gate checked `LoadComplete()` after
the compute (`analytics_warmup_1659.go:122`). `LoadComplete` flips at
the end of the hot window (`chunked_load.go:489`), before the background
fill (`store.go:1455`), so the gate could open on a snapshot without the
background fill, and the 60 s force timeout (`:73`, `:162`) opened it on
the first-chunk snapshot. In an end-to-end test on master,
`/api/analytics/rf` returned 200 with 8 of 100 packets before the
background fill ran.
- Distance, hash-collisions, hash-sizes, roles, observers-clock-skew,
nodes-clock-skew: no gate, partial snapshot served from the start.
- Distance additionally served a snapshot from the previous index for up
to one interval after each lazy index build.

On a staging instance, a default analytics request returned 5,911
packets with hours-old last buckets until the next recompute (about
74k).

### Change
- `StartupLoadDone()` (`chunked_load.go:108`): closed when
`RunStartupLoad` returns, on every path (`chunked_load.go:202`). Closing
it drops the hash-size info cache (15 s TTL) and the clock-skew engine
throttle (30 s, `clock_skew.go:225`), both read by the post-load
computes.
- `recomputeWhenLoaded` (`analytics_recomputer.go:172`): on that signal,
recompute each recomputer once, sequentially, via `RecomputeNow`
(`:154`), which runs on the recomputer's own loop and restarts its
ticker (`:106`). Order (`:255`): rf, topology, channels, distance,
hash-collisions, hash-sizes, observers-clock-skew, nodes-clock-skew,
roles (roles reads the nodes-clock-skew snapshot). Logs one line with
per-recomputer durations.
- Warm-up gate: now the same signal (`:343-354`), sampled before the
compute starts (`:129`), so a pass that began on partial data never
opens it. 503 + `Retry-After: 5` and the force timeout are unchanged; a
forced-open snapshot is replaced by the post-load recompute.
- Ungated endpoints: no new 503s (their API has none); snapshot replaced
right after the load.
- Distance: the lazy index build refreshes the distance recomputer
before reporting built (`store.go:4476`).
- Recompute intervals and config unchanged.

### Performance
One extra compute per recomputer per process start, run sequentially so
they do not all hold the store read lock at once. Ticker phases
afterwards are offset by the cumulative post-load compute durations
instead of all starting within the first-chunk compute window (relevant
to #1724; the effect on lock waves is not measured).

### Tests
`analytics_recompute_after_load_test.go`: signal open during background
fill, closed after success and failure; cache drops; immediate and
ordered post-load recompute; gate not opened by a pass started before
the load; forced-open snapshot replaced on load; ticker restart;
distance refresh before 202 ends; end to end with recomputers started
before the background fill (RF 503 until load, then `totalTransmissions`
equals the full store; six ungated endpoints 200 during load; all nine
recomputed after load). 9 of these failed on master with stubs; 8
single-line mutations each caught. `go test ./...` in `cmd/server`
passes.

### Staging validation
Deployed together with the review follow-ups of #2015-#2023 (build
`c646310f`), container restart:

```
16:35:20 [store] first chunk ready (chunkSize=10000)
16:35:25 [store] LoadChunked complete ... starting background fill loader
16:36:58 [store] background load complete: 121120/121282 packets in memory (coverage=99.9%)
16:37:03 [analytics-recompute] startup load done: recomputed 10 snapshots in 5.155s (rf=955ms topology=1.684s channels=43ms distance=49ms hash-collisions=30ms hash-sizes=338ms observers-clock-skew=369ms nodes-clock-skew=692ms roles=2ms retransmissions=994ms)
```

Right after that line, `/api/analytics/rf` reported `totalTransmissions`
121,121 against 120,700 packets in memory, and the retransmissions
default shape from #2023 covered the full 7 days. Before this change
both waited for the next 5 minute tick.

### Merge order with #2023
#2023 adds a tenth recomputer. Whichever of the two merges second has to
add `recompRetransmissions` to `analyticsRecomputersLocked`, wire it to
`loadedGate` instead of `LoadComplete`, and change 9 to 10 in
`TestAnalyticsRecomputers_PostLoadOrder`; the retransmissions gate test
then calls `signalStartupLoadDone()` instead of setting `loadComplete`.
That resolution is what ran on staging above.

### Not verified
- Repeater-enrich recomputer and the region/window TTL caches
(hash-collisions region results have a 1 h TTL) may also keep partial
results after the load; not changed here.
- Recompute order is tested structurally, not with roles/clock-skew
data.
- Whether this reduces the #1724 stalls; not measured.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-13 19:59:37 +02:00
efitenandClaude Opus 5 7f8f71e773 fix(live): reconnect when the websocket goes silent (#1074) (#2020) 
## Problem

#1074 reports that after a proxy dropped the WebSocket, live updates
only came back 8 to 10 minutes later.

The client only reconnects from `onclose` (`public/app.js:791` on
master). A half-open connection (a proxy or NAT dropping state without a
FIN reaching the browser, a laptop that slept) can keep a WebSocket OPEN
for minutes without `onclose`, and nothing retries in the meantime.

The server does ping every 30s (`cmd/server/websocket.go:252` on
master), but ping frames are answered by the browser below the page and
JS cannot observe them. The only app-level frames are packet broadcasts
(`websocket.go:337, 343, 366`), which stop on a quiet mesh. So the
client had no signal to tell a quiet mesh from a dead socket.

## Change

Server (`cmd/server/websocket.go`):
- On the existing ping tick, `writePump` also writes the text frame
`{"type":"heartbeat"}` (`:283`, bytes at `:80`). One 20-byte frame per
client per 30s, from the goroutine that already writes the ping, no hub
lock.
- The interval moves to `Hub.pingInterval` (default 30s, `:118`) so a
test can shorten it.

Client (`public/app.js`):
- Every frame refreshes `wsLastMessageAt` (`:841`). One timer
(`checkWSLiveness`, `:806`) fires at last message + `WS_STALE_MS` (75s,
one late or lost heartbeat of slack) and replaces the socket if it is
still silent. It is armed at socket creation, so a stuck handshake is
covered too.
- `dropWS` (`:796`) detaches the old socket's handlers before `close()`,
so a late close event cannot schedule a second connection.
- `connectWS` (`:818`) cancels a pending reconnect and drops the
previous socket, so the watchdog, resume checks, `onclose` and
pull-to-reconnect cannot stack sockets. Before this, `pullReconnect` on
a non-open socket left a third socket 3s later. The 3s `WS_RECONNECT_MS`
delay after `onclose` is unchanged (`:837`).
- `visibilitychange` (to visible) and `online` run the check immediately
(`:861`), because a hidden or sleeping tab's timers can run late.
- Heartbeat frames are matched by exact bytes (`:842`) and are not
pulsed or dispatched to `onWS` listeners.

Compatibility: tabs loaded before the deploy dispatch heartbeats to
their listeners until reloaded. Every current listener filters on
`msg.type`, so the visible effect is a logo pulse and a `/stats` cache
refresh every 30s.

Perf: one `Date.now()` and one string compare per WS message on the
client; one extra 20-byte write per client per 30s on the server.

## Tests

- `test-ws-stale-watchdog-1074.js`: real `app.js` in a vm with a fake
clock, timers and WebSocket. 12 tests: silence past the threshold
replaces the socket exactly once; a handshake that never opens is
replaced; heartbeats and packet traffic keep the socket; heartbeats are
not dispatched; resume and `online` after silence reconnect immediately,
with recent traffic they do not, and hiding does not trigger a check;
repeated resume events open one socket; after `onclose` only the
reconnect timer is pending; pull-to-reconnect leaves one socket. 9 of 12
fail on master. 12 of 12 source mutations (threshold, reconnect path,
detaching, timer cleanup, resume wiring, heartbeat filter) are caught.
Registered in `test-all.sh` and the deploy.yml unit step.
- `TestWritePumpSendsAppHeartbeat`: fails with a read timeout without
the heartbeat, even with pings every 20ms. `TestHubDefaultPingInterval`
pins the 30s interval that `WS_STALE_MS` assumes.
- `go test ./...` in `cmd/server` passes; gofmt and go vet are clean.

## Browser validation

On a staging instance (build `139e484e`, together with #1979's branch),
in Chrome, no console errors:

- A `{"type":"heartbeat"}` frame arrived on the open socket within the
observation window.
- Silent socket: after `ws.onmessage = null`, the page replaced the
socket after 76.2s (threshold 75s plus a 250ms poll); the old socket
ended in CLOSED, the new one OPEN.
- Normal close: `ws.close()` led to a new OPEN socket after 4.1s, and
exactly one new `WebSocket` was constructed.

## Not verified

- The reporter's proxy setup was not reproduced; that their delay was a
half-open socket is a hypothesis consistent with the symptom. Hence
`Refs`, not `Fixes`.
- Laptop sleep and the `visibilitychange` / `online` resume path were
only covered by the unit test, not in a browser.
- Behaviour under Chrome's intensive background-timer throttling and
mobile tab freezing was not measured; a frozen but healthy tab may do
one unnecessary reconnect on resume.
- Go tests were run without `-race`.

Refs #1074



## Review follow-up (commit `72e5e906`)

An independent review found no blocking bug: all data writes stay on the
write goroutine, pong-based dead-client detection still works, and no
ordering of onclose, watchdog, resume checks and pull ends with two live
sockets or none. It reproduced the silent-socket case in headless
Chromium through a blackholing TCP proxy (replacement 75.0 s after the
last frame). Changed:

1. **Startup wiring tested.** The first resume test now boots through
the page's real `DOMContentLoaded` listeners, so removing
`setupWSResumeCheck()` from startup makes it fail.
2. **Wall clock stepping back.** If the clock steps back after the last
message, the watchdog no longer re-arms for the size of the step (a 1 h
step used to delay detection by about an hour). A negative silence
reading is treated as stale, so the socket is replaced within
`WS_STALE_MS` of the step (`public/app.js:810-814`). A step in either
direction costs at most one extra reconnect on a healthy socket.
`Date.now()` stays the clock so a tab resumed after sleep is still
checked against real elapsed time.
3. **Pull-to-reconnect at once.** On an OPEN socket, pull-to-reconnect
now replaces it through `connectWS()` instead of closing it and waiting
for onclose, which took 63 s on a half-open connection in the review's
measurement (`public/app.js:927-934`). This was slow on master too; it
is safe now that `connectWS()` detaches the old socket.

Tests: 12 to 16 in `test-ws-stale-watchdog-1074.js`;
`test-pull-to-reconnect.js`, `test-pull-to-reconnect-1091.js` and
`test-live.js` pass.

Correction to the compatibility note: tabs opened before the deploy
treat the heartbeat like any other message. Besides the logo pulse,
`app.js` runs `updateNavStats` on every message and invalidates the
cached `/stats` and `/nodes` responses 5 s later; `packets.js` also
pushes every message into `pauseBuffer` unfiltered (~1310-1313), so an
old tab with Packets paused sees its counter rise by 2 per minute.
Cosmetic: heartbeats are filtered out on replay, and a reload ends it.

Not verified: real hidden-tab or mobile freeze behaviour, Firefox and
Safari, and the reporter's proxy setup.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-13 19:59:18 +02:00
efitenandClaude Opus 5 0fea3f2a75 feat(analytics): break scope adverts down by node role (#1979) (#2019) 
## Summary

Adds a breakdown of flood adverts by sender role to `/api/scope-stats`
and the Scopes tab, in the descriptive shape agreed in #1979: per node
role, how many flood adverts were unscoped, scoped with an unnamed
region, or scoped with a named region. It reports what was sent, not
why.

## Changes

- `cmd/server/db.go:3114-3144`: one grouped query in `GetScopeStats`.
ADVERT packets on flood routes (TRANSPORT_FLOOD 0, FLOOD 1) in the
window, `LEFT JOIN nodes` on `from_pubkey`, split by the three
`scope_name` states (NULL, empty string, name). A missing or empty role
becomes `"unknown"`. Ordered by total descending, then role. Zero-hop
adverts are excluded because firmware sends them as
DIRECT/TRANSPORT_DIRECT (`src/Mesh.cpp:717-730`, `Mesh::sendZeroHop`),
so they would inflate "unscoped".
- `cmd/server/types.go:116-133`: `ScopeAdvertRoleCount` and
`ScopeStatsResponse.AdvertsByRole` (`advertsByRole`, always an array).
- `public/analytics.js:4760`: `scopeAdvertsByRoleHtml` renders a table
under the time-series chart with the count per state and its share of
the row. Role text is escaped. It reuses the existing `/scope-stats`
response, so there is no extra request.
- `docs/api-spec.md:1763-1786`: documents the new field.
`/api/scope-stats` is in `openapi_known_gaps.json`, so there is no
`openapi.go` entry to update.

API addition (existing fields unchanged):

    "advertsByRole": [
{ "role": "repeater", "unscoped": 7741, "unknownScope": 7, "named": 1562
}
    ]

## Performance

The query runs inside `GetScopeStats`, so it shares the existing 30s
cache per window. The unary `+` on `payload_type` keeps SQLite on the
`first_seen` range index. Without it the planner picked the
`payload_type` index and walked every stored advert whatever the window.
Read-only timing on a production DB (1,063,345 transmissions, 161,634
adverts, sqlite3 CLI 3.45.1):

| Window | payload_type index | first_seen index (this PR) |
|---|---|---|
| 7d | 0.231s | 0.059s |
| 24h | 0.214s | 0.008s |
| 1h | 0.213s | 0.001s |

## Tests

- `TestGetScopeStatsAdvertsByRole` (`cmd/server/db_test.go:2295`): the
three states, flood-only routes, non-advert and out-of-window exclusion,
`unknown` for a missing node row, an empty role and a NULL
`from_pubkey`, and ordering. Mutation checked: widening to routes 0-3
and dropping the empty-role fallback both fail it.
- `TestGetScopeStatsAdvertsByRoleEmpty` (`:2372`): empty result is `[]`,
not null.
- `test-issue-1979-scope-adverts-by-role.js`: renders the real
`analytics.js` helper in a vm sandbox. Covers row order, totals and
shares, columns, escaping (mutation checked), the empty state, and the
non-causal caption. Registered in `test-all.sh` and the deploy.yml unit
step.
- `go test ./...` in `cmd/server` passes, gofmt and go vet are clean,
`check-css-vars.js` OK, `check-xss-sinks.sh --diff` exits 0.

## Browser validation

On a staging instance with live traffic (build `139e484e`, together with
#1074's branch), in Chrome, no console errors: `/#/analytics?tab=scopes`
shows "Flood adverts by node role" under the time-series chart with its
caption, and a table of 6 roles for the default window, for example
`repeater 1.361 | 1.109 (81.5%) | 2 (0.1%) | 250 (18.4%)`. Shares in
each row add up to 100%. `/api/scope-stats?window=7d` returns
`advertsByRole` with the same six roles.

## Not verified

- Timings are from the sqlite3 CLI, not the modernc driver in the server
process.
- Role is the sender's current `nodes.role`. A node whose advert type
changed within the window is counted under its latest role. The data
also contains a raw `type-13` role, shown as is.
- Switching the window in the browser was not exercised; the API was
checked for 7d.
- No Playwright E2E added.

Fixes #1979



## Review follow-up (commit `43af46b8`)

An independent review found no correctness, security or performance
problem: counts are per transmission, the window matches the rest of the
Scopes tab, zero-hop adverts are DIRECT per firmware, and the
`+t.payload_type` hint holds on modernc SQLite 3.46.0. It found two test
gaps and a docs gap. Changed:

- **Ordering.** The Go fixture gave companion, repeater and unknown 3
adverts each, so ordering by total was never checked (`ORDER BY COUNT(*)
ASC` still passed). The fixture now has repeater 4, unknown 3, companion
2, sensor 2, so the expected order differs from alphabetical and the
companion/sensor tie checks the role-name tie-break. Reversing the count
order, dropping it, or reversing the tie-break now fails the test.
- **Column positions.** The JS test only checked that each cell string
appeared somewhere in the row, so swapping two columns passed. It now
compares each row's cells and the header cells by position; both swaps
fail.
- **Docs.** `docs/api-spec.md` and the `ScopeAdvertRoleCount` comment
now name every source of `unknown`: a NULL `from_pubkey` (legacy rows
the #1143 backfill has not reached), a sender with no `nodes` row,
including one moved to `inactive_nodes` by node retention (inside the 7d
window only with `retention.nodeDays` below 7), and an empty role.

Not added: a query-plan test pinning the `+t.payload_type` hint. The SQL
is inline in `GetScopeStats`, so the test would have to copy it or the
query would have to move into a constant; left for a follow-up if
wanted. The raw `type-13` role in the table is the ingestor's
placeholder for reserved advert types 5-15
(`cmd/ingestor/decoder.go:1229`, #1279), shown as is.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-13 19:59:00 +02:00
efiten 296456f9c1 feat(map): colour and filter repeaters by scope-configuration state (#2006) 
Closes #2001. Two review rounds plus a re-review; findings and evidence on the PR. Verified on a deployment against live data: the field over 1653 nodes, marker tints per filter state, the marker title and popup Scope row reaching the DOM, and the colorblind-preset cascade. The audit and the map are held to the same classification by an end-to-end test that fails when either side's wildcard handling drifts.
 2026-09-11 15:07:31 +02:00
efiten 675c576fea fix(scope-audit): bound the verifier's payload, and fix two tests that proved less than they claimed (#1993) 
Three leftovers from reviewing the scope-audit series (#1986, #1987,
#1990). None is urgent; all three are the kind of thing that gets harder
to explain the longer it sits.

## `scopeHMACInputs` accepted a payload `DecodePacket` rejects

Its comment says it walks the same offsets as the decoder, and it does,
minus the `maxPacketPayload` bound the decoder enforces. Unreachable in
practice: such a packet never reaches the database with an empty
`scope_name` in the first place, so the verifier never sees one.

Worth closing anyway, because the comment claims the two agree. A
verifier that accepts what the decoder refuses is a small divergence
today and an hour of confusion on the day it matters.

## The corroboration test seeded the same packet twice

The threshold of two rests on `code1` being two bytes: one match is
1/65536 by chance, two on the same region is (1/65536)². That argument
needs two **independent** observations.

The test seeded `realTransportFloodPacket` twice. Identical payloads
derive identical codes, so it was one observation counted twice, and it
would have passed just as happily against an implementation that counted
rows rather than deriving anything.

It now seeds the real captured packet plus a second one built for a
different payload, both deriving to `#fm-112` on their own.

**The feature was never wrong here.** `transmissions.hash` is unique and
`ComputeContentHash` is path-independent, so two rows always mean two
distinct payloads in production. Only the test failed to demonstrate the
property it is named for.

## Naming at ingest and verifying at read time had no test together

They were built separately, in #1990 and #1989, and the interaction
between them is not exotic: with derived region keys enabled, a packet
that used to be stored unnameable now arrives with a name. The audit
must then report that region as observed by the ordinary route:

- present in `agg.scopes`
- absent from `notObserved`
- **not** claimed by `regionEvidence`, which exists to explain regions
that could only be established by verification

Getting that wrong is quiet. The chip stays green while the reason
underneath it is wrong, and a reader asking "how do we know this" gets
the wrong story.

## Verification

`cd cmd/server && go test ./...` passes (254s), `go vet` and `gofmt -l`
clean. No production behaviour changes beyond the payload bound, which
rejects input that cannot occur.
 2026-09-10 22:30:24 +02:00
efiten 8c164c5315 feat(scope-audit): verify a declared region against the repeater's own traffic (#1990) 
Follow-up to #1987, and the point of counting that traffic in the first
place.

A region this instance holds no `hashRegions` key for is **unnameable,
not absent**. #1987 says so with a caveat chip. This settles it wherever
the evidence allows: derive `SHA256("#region")[:16]` from the repeater's
own declaration and HMAC that repeater's own unmatched packets with it.
Same computation the ingestor performs at ingest, with the candidate set
narrowed from every configured key to this repeater's handful of
declarations.

Where it fires, a grey "declared but not observed" chip becomes a green
one and the caveat count shrinks by the packets it explained.

## Two packets, not one

`code1` is two bytes, so an unrelated name matches a given packet with
probability 1/65536. Across ~400 unmatched packets and ~124 declared
names, chance alone produces roughly one false match per refresh. Two
matches on the same region for the same repeater is (1/65536)², about
one in four billion.

Lowering the threshold to one would not make this noisy, it would make
it **unsound**, so the constant carries that arithmetic and a test
rather than a comment. A region with exactly one hit stays grey and
reports its single hit, so the page can say why it is still shown as not
observed instead of leaving the reader to wonder.

## What it deliberately does not do

**It writes nothing.** Read-time only. A wrong answer expires with the
window instead of sitting in `transmissions.scope_name` until someone
runs a repair, and `cmd/server` stays read-only per the invariant in
AGENTS.md.

**`notObserved` remains the single source of chip colour.**
`regionEvidence` says only HOW a region was established. Two fields that
can disagree about the same fact is how this column got confusing in the
first place.

## Rule 0, including the part that was wrong at first

The naive shape is `targets × names × packets` HMACs: 205 × 124 × 400 ≈
10M.

Caching per `(region, transmission)` pair cuts the HMACs to ~50k. **That
measured 501ms**, because the HMACs had become a rounding error while
the *iteration* stayed cubic at 10.2M map lookups. Re-keyed per region,
holding the set of matching transmissions, it is **36ms** at the same
worst-case shape: a region is HMACed over every packet once, and a
target then asks one question per declared region instead of one per
(region, packet). Most declared regions match nothing, so the common
case is a single map lookup and no packet loop at all.

`hmacCount` exists so a test can assert the first mistake cannot come
back; the benchmark exists because only it caught the second.

## Both axes are bounded, because neither is bounded by the data

The "~400 packets in a 7 day window" this was sized against is a
property of one instance's configuration, not of the feature: the
ingestor stores the unnameable state for every transport-scoped packet
no configured key names, so an instance with few or no `hashRegions`
entries — the stock state, and the one this helps most — has **every**
scoped packet in that set.

- the window query takes the 4096 most recent candidates and reports
truncation, which the handler logs, so a grey chip on a sampled refresh
is not read as "not forwarded"
- the declared list is capped at 32 names per repeater: it arrives from
a collector that validates each entry's shape but never how many entries
there are
- measured at the cap: **306ms** for 205 targets over 124 names, against
29ms for the shape a real network produces

Because both caps make the evidence a sample, the response carries
`observedUnmatchedSampled`. Without it a client subtracts a capped
numerator from an uncapped total and overstates the unexplained traffic
with no way to know it is doing so. The chip subtracts only evidence for
regions **absent** from `notObserved` — a single-hit region the server
refused to accept is not called explained either — and says "at most N"
when the count was sampled.

## Verified on live data

Six repeaters clear the threshold in a 7d window on a real instance. One
of them: `nl-nb` green with 3 corroborating packets and the tooltip
stating the count, `belml` still grey on 1, and the caveat chip reading
31 of 34 packets unexplained rather than 30.

## Tests

`scope_verify_test.go` covers the HMAC-input walk against a real
transport-flood packet captured from a live instance (a hand-built
fixture would only prove the parser agrees with itself), that
`regionCode` does not fold case, the threshold in both directions, the
memo's HMAC count, both bounds with their truncation flag, and the
benchmark at cap size. Handler-level tests cover a region verified into
green, a single hit left grey with its count reported, and the
sample-size field.

`cd cmd/server && go test ./...` passes (77s), frontend 723 assertions
pass, `go vet` and `gofmt -l` clean.
 2026-09-09 17:58:09 +02:00
efiten 0605b1703a feat(scope-audit): count and surface the traffic this instance cannot name (#1987) 
Follow-up to #1986, and the second half of the same problem.

`ScopeAuditForwarding` drops rows whose `scope_name` is the empty string
with a bare `continue`. That empty string is the ingestor's
"transport-scoped, but no configured region key matched `code1`" state
(`scopeNameForDB`), so those packets name no region and can never
satisfy a declared one. The consequence is on the page: **a repeater
forwarding a region this instance holds no `hashRegions` key for is
reported exactly like a repeater forwarding nothing at all.** The audit
presents a gap in the reader's own configuration as a finding about
someone else's hardware.

This counts them per target, exposes the count as
`observedUnmatchedPackets`, and renders it as a caveat chip beside the
scope chips.

## Why it is not a rare edge

Measured on a live instance before this landed: of 613 `notObserved`
entries across 205 repeaters, **260 named a region that never appeared
under any name in the whole 7-day window**. Two of them (`behss`,
`fm-112`) were hash-verified as genuinely forwarded traffic the instance
simply could not name: packet `0a065d41d51f1f77` decodes to
`code1=9209`, which is exactly the code `#fm-112` derives over that
packet's own payload.

That instance had 16 region keys configured against 124 distinct region
names its repeaters declare. A stock install has fewer.

## What the counter is not

It is deliberately **not** folded into `unscopedPackets`. The two are
opposites:

| | meaning | what governs it |
|---|---|---|
| `unscopedPackets` | the packet carried no scope at all (`scope_name`
SQL NULL) | the `*` wildcard |
| `observedUnmatchedPackets` | the packet IS scoped, this instance holds
no key for that region | nothing the repeater declares |

For the same reason the new count never feeds `wildcardContradiction`,
which counts only plain unscoped floods. `scopeNameForDB` in the
ingestor is the source of truth for that three-state encoding, and the
comments point there rather than restating it.

It is also distinct from `ambiguousHops`, and the distinction is the
point of the chip: that one is a pubkey-prefix collision between two
repeaters and is nobody's fault, this one is a missing entry in the
reader's own configuration and they can act on it. Saying which is which
is what stops someone investigating an innocent repeater.

## Frontend

The chip reuses the muted dashed treatment of `.sa-chip-ambiguous` on
purpose: both are caveats on the row's finding rather than findings
themselves, and neither may compete visually with the red/green scope
chips beside them.

It renders nothing for a non-numeric count. The value is
server-supplied, and a truthiness check would put the literal string
`NaN forwarded packets` on the page if that ever stopped holding.

## Docs

`docs/api-spec.md` had **no entry for `GET /api/scope-audit` at all**,
so this adds one: query parameter, full response shape, and the notes a
client needs (the three traps the per-node endpoint documents apply here
identically, `*` is never a scope, and "never asked" is not "declared
nothing"). The new field is documented there rather than in isolation.

## Tests

- the counter on a last-hop and on a mid-path hop
- an unmatched packet enters neither `agg.scopes` nor `unscopedPackets`,
which is the confusion this field exists to prevent
- the field on the API row
- six frontend cases: zero renders nothing, a missing field renders
nothing (older server), the chip carries its count and class, singular
and plural are both grammatical, the title names the cause and the fix,
and a non-numeric count renders nothing rather than `NaN`

`cd cmd/server && go test ./...` passes, frontend 712 assertions pass,
`go vet` and `gofmt -l` clean.

Rule 0: the counter is one increment on a branch that already existed as
a `continue`, inside a loop this PR does not change. No new query, no
new pass over the data.
 2026-09-09 17:05:29 +02:00
efiten 079e73aa4c fix(scope-audit): attribute forwarding to every hop, and pay for the wider scan (#1986) 
The Scope Audit credits a transmission to `path[last]` only. On a
flood-family route every forwarder appends its own hash to the END of
the path (`internal/packetpath/route.go`), so `path[last]` does not mean
"forwarded this packet", it means "was the transmission an uplinked
observer heard directly". Every earlier hop forwarded the same packet
and is discarded.

The last-hop rule is genuinely required for DIRECT routes, which consume
hops from the front, so their `path[last]` is the route’s far end rather
than the transmitter. But `scopeAuditForwarderScanQuery` already
restricts to `route_type IN (0, 1)` via
`scopeConformanceForwarderRouteTypesSQL`, where that hazard cannot
arise, so inside this query the restriction only throws evidence away.

## What it costs the page today

Measured on a live-shaped instance, 206 declared repeaters, 965k
transmissions, 7d window:

| | before | after |
|---|---|---|
| repeaters with no attributable evidence of any kind | 133 of 205 (65%)
| 30 of 206 (15%) |

On a 1000-packet flood sample the mean path length is 7.08 hops, so the
last-hop rule keeps 394 of 2789 hop observations (14%), and 85% of the
nodes seen forwarding never appear as a last hop at all. Those repeaters
have every region they declare reported as "declared, not observed",
which is the page presenting a gap in our own attribution as a finding
about someone else’s repeater.

## Rule 0: what widening it costs, and what pays for it

Reading every hop multiplies the rows the scan returns: a 7d window
yields **3,470,188 hop rows** from 1,368,761 observations carrying a
path. Cold cost before this change was 16.7s for 7d and 4.0s for 24h, of
which SQLite accounts for 2.7s. The rest was the Go side reading rows.

Three changes, in order of what they bought:

1. **The scan carried `scope_name` and `first_seen` on every hop row.**
Both are columns of `transmissions`, and at 43 hop rows per transmission
the same two values were re-read that many times. They now come from one
query over the same window keyed by transmission id, both inside one
read transaction so a transmission arriving between them cannot appear
in the hop scan with no metadata to attribute it by. The hop scan
carries two columns instead of four.
2. **The hop is lower-cased into a stack buffer** instead of through
`strings.ToLower`. 1,026,814 of the 1,284,897 hops in a 24h window are
stored uppercase, because `packetpath.DecodePathFromRawHex` writes them
that way, and the great majority match no declared target, so that
allocation was paid millions of times to answer "no". The `(target,
txID)` de-duplication key became a struct for the same reason.
3. **The compute ran outside the cache mutex**, so every request
arriving on a cold window ran its own full scan concurrently. It now
sits behind a singleflight, the same treatment `/api/observers` and
`/api/nodes/{pubkey}/reach` already have, and the 7d window gets a 5
minute TTL while 1h and 24h keep 30s. At 30s a single reader with 7d
open keeps the instance recomputing more than half the time, for an
aggregate that moves at the pace of a week of traffic.

Result, warm process:

| window | before | after |
|---|---|---|
| 1h | 0.155s | 0.140s |
| 24h | 4.04s | 2.79-2.89s across six samples |
| 7d | 16.7s | 11.6s |
| repeat inside TTL | ~1ms | ~1ms |

**Rejected alternatives, measured on the same database**, so the next
reader does not have to re-derive them:

| approach | rows returned | time in SQLite |
|---|---|---|
| the query as written | 3,470,188 | 2.7s |
| pre-filter on the declared targets’ first 4 hex chars | 1,971,126 |
20.9s |
| `GROUP BY t.id, hop` | 965,025 | 38.0s |
| `SELECT DISTINCT t.id, path_json` | 1,229,966 | 17.7s |

The query plan is already index-driven (`idx_transmissions_first_seen`,
then `idx_observations_tx_ts`), so there is no missing index behind
this: the rows are inherent to the data. Note for anyone attempting a
hop comparison in SQL: a case-sensitive comparison silently drops most
attributable hops, per the 80% figure above.

## Tests

- a mid-path hop is attributed (the case behind the 65% blind spot)
- a DIRECT transmission whose `path[last]` **is** the target is still
not attributed. With the last-hop rule gone this is the only thing
standing between the audit and misattribution, so it gets its own test
rather than relying on the route filter being obvious
- one transmission counted once per target even when it appears on
several hops of the same path, which the `(target, txID)` de-duplication
now carries alone
- a hop longer than the 4-char floor resolved by its own length, which
nothing pinned before: every other test seeds 4-char hops
- the per-window TTL, so collapsing it back to one constant has to
delete the reason
- a second request inside the TTL served from cache rather than
recomputed. The cache path had no test at all

`cd cmd/server && go test ./...` passes (168s), `go vet` and `gofmt -l`
clean. Server-side only, no API shape change, no frontend change.

Browser validation: run against a live instance carrying this change,
the Scope Audit renders 220 rows matching the API row for row, and the
per-node scopes page still answers with its route-type mix.
 2026-09-09 16:00:15 +02:00
efiten 2c8c1161b5 feat(#1975): network-wide Scope Audit page, fed by confirmed scopes (#1976) 
One row per repeater whose configured region list is known, answering a question
no other view answers: you declare these regions, but were you seen forwarding
them? default_scope says what a node's adverts were observed under and
transported_scopes (#1751) says what it carried, but nothing lined the declared
list up against observed forwarding.

The declared side merges every confirmed-scope source the database carries,
newest answer per node wins, rather than naming one. On a stock install only
nodes.configured_scope (#1865/#1971) exists and it degrades to the one-source
case; deployments that collect the same fact another way keep working. Reading a
single hard-coded source would have rendered an empty page on the very instance
the evidence came from.

Declared and observed are compared through normScope, so a leading "#" and a
bare region name are one region. Unobserved regions render neutral, not red:
absence over a short window is weak evidence, which the page header already
states in words.

Ported from a long-running fork deployment with its 17 server tests, rewired to
the upstream data source, plus 14 frontend cases asserting rendered markup.
 2026-09-06 23:04:42 +02:00
efitenandClaude Opus 5 1ffaad8eb1 feat(#1794): per-IP limits and a deny list on the /ws upgrade (#1974) 
Closes #1794. Follow-up to #1793, decided **before** the upgrade because
the handshake is the resource being protected.

- Deny list of addresses and CIDRs → 403
- Per-IP concurrent connection cap → 403
- Per-IP upgrade rate limit over a rolling minute → **429**, not 403: a
temporary refusal should not read as "never come back"
- Rejection counters split by cause in `/api/stats` under `websocket`

### The decision this feature lives or dies on

Most CoreScope installs sit behind nginx, Caddy, Traefik or an ingress.
`cdn_detection.go` says so in as many words: it deliberately excludes
`X-Forwarded-For` from its CDN signals precisely because *every*
reverse-proxied install sets it. For those deployments `r.RemoteAddr` is
the proxy, `127.0.0.1` for every visitor on earth. A per-IP cap keyed on
that address protects nobody and hands the sixth legitimate browser tab
a 403. That is a self-inflicted outage wearing the costume of hardening.

So:

- **`X-Forwarded-For` is believed only from an address listed in
`webSocket.trustedProxies`.** From anywhere else it is
attacker-supplied, and trusting it would let anyone mint a fresh source
IP per connection, which is strictly worse than having no limit at all.
- **When the peer looks like a local reverse proxy and no
`trustedProxies` is set, the per-IP limits are skipped**, and one
warning names the setting that fixes it. Silently refusing real users is
the worse failure.
- **The deny list still applies there**, because it is the operator's
explicit instruction rather than an inference.

That is the answer to @mcode6726's question on the thread: it is neither
"always the socket address" nor "always the header", and the operator
decides which by naming their proxy.

### Two deliberate departures from the issue body

**`maxConnsPerIP` ships as 0 (off), not 5.** Carrier-grade NAT puts
thousands of unrelated mobile subscribers behind a single public IPv4. A
cap of 5 refuses real visitors on phones while a scraper simply rents
more addresses: all of the cost, none of the benefit.
`upgradesPerMinPerIP` ships at **30 and on**, because that one *is* safe
under CGNAT: a real client upgrades a handful of times per minute even
while reconnecting, so 30 leaves ordinary traffic untouched while
flattening a reconnect loop. A pointer type distinguishes "unset" from
an explicit `0` that turns it off.

**The default deny list is not shipped.** The thread proposed seeding 44
CIDRs for one VPS provider after a single scraper was seen at
`23.111.177.6`. I have left it out: blanket-blocking a hosting provider
by default breaks legitimate operators who host there, is undiscoverable
by the person locked out (they see a bare 403), and ages badly as ranges
get reassigned. The mechanism is here and `config.example.json` shows
exactly how to configure it, so any operator who wants that list can
have it in one line. If you want it shipped as a default anyway, that is
your call as maintainer and it is a one-line change.

### Verification

19 tests, including all five the issue specifies as TDD requirements,
each marked with the issue's own wording. Beyond those five:

- a **bare address** in the deny list works, not just CIDR form.
Operators write `1.2.3.4`, and silently ignoring that would be the worst
possible failure for a deny list: it looks configured and blocks nothing
- an unparseable deny entry is skipped and logged, not fatal. One typo
must not take the server down
- one client behind a trusted proxy does **not** exhaust another
client's budget behind the same proxy, which is the entire point of
honouring XFF
- changing a forged XFF from an untrusted peer buys no fresh budget
- `release` frees a slot and is **idempotent**, because `Unregister` can
run twice for one client and double-crediting would leak slots
- a **rejected** upgrade does not consume rate budget, or a retrying
client could never recover once its window cleared
- limits skipped for loopback and private peers; deny list applies
anyway
- a nil limiter allows everything, so a `Hub` built without
`ConfigureLimits` behaves exactly as before
- idle per-IP state is collected, while a record with a live connection
never is

Full `cmd/server` suite green, `gofmt` clean.

### Not done

- No runtime config reload; restart required. Listed as optional in the
issue.
- No `WS_DENY_IPS` env override. Also listed as optional.
- From the OWASP expansion in the first comment: `maxPayload` and the
idle/read timeout are **already in master** (`SetReadLimit`,
`SetReadDeadline`). The ping/pong heartbeat is not, and is not in this
PR either; it is a separate change to the read/write pumps and belongs
in its own review.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-06 21:11:29 +02:00
efitenandSaarMesh-Bot 9d6f08c144 feat(#1865): ingest observer /neighbors as confirmed scope evidence (#1971) 
Carries SaarMesh-Bot's implementation from the closed #1867 forward onto current
master, 67 commits later, and surfaces the result on the per-node Reach report.

The declared region list a repeater answers with is now stored on the node as
configured_scope, normalised to the same leading-# syntax default_scope already
uses so the two are directly comparable. That normalisation is the point
@cwichura raised on #1865 and @dborup agreed with before the original PR closed.

Co-authored-by: SaarMesh-Bot <300107934+SaarMesh-Bot@users.noreply.github.com>
 2026-09-06 21:06:12 +02:00
n30nex cf67a5e5ec fix: remove evicted resolved path hops and preserve relay snapshots (#1966) 
Eviction removes raw wire hops but leaves resolved full-key entries in
`byPathHop`, retaining expired transmissions and stale relay
counts/scopes. Filter every hop bucket once per eviction batch using the
existing evicted-ID set, remove duplicate references and empty buckets,
and clear discarded pointer slots.

Bulk relay aggregation now owns its bucket snapshots before releasing
the read lock, so eviction and raw-path updates cannot mutate an
in-flight reader. Three existing handler test fixtures also wait for
index readiness or explicitly simulate not-ready state, preserving their
original 200/503 assertions.

Fixes #1908.

Validation:

- Regression commits fail before their corresponding fixes: resolved
keys/counts/scopes remain after eviction, and saved relay snapshots
change during eviction.
- Targeted eviction, relay, scope, cache and concurrent-reader checks
pass under `-race`; coverage includes time/cap eviction, missing
resolved-path prefetch, disabled membership indexing, duplicate
references, retained backing arrays and surviving entries.
- Local browser smoke: nodes, node details/path attribution and
analytics render using the fixture-backed Go server.
- The last full Windows server race run, before the final snapshot-copy
correction, had one remaining DB-only timing failure
(`TestGetChannelMessagesPerfLargeChannel`: 2.198s against a 1.5s
budget). The final correction was checked with focused race tests. The
unchanged ingestor suite also cannot create one symlink without Windows
privileges. These thresholds/assertions were preserved; full Linux
Go/E2E results still require upstream CI approval.

Performance tradeoff: cleanup is O(total indexed pointers) per nonempty
eviction batch, under the existing write lock. The minute-based ticker
pays for one sweep instead of repeated scans of shared raw buckets. No
per-transmission string index or dependency is added. Synthetic
benchmark medians (three single-iteration runs, shared Windows host):

| Transmissions | Evicted | Before | After |
|---:|---:|---:|---:|
| 30,000 | 1 | 1.07 ms | 14.19 ms |
| 30,000 | 3,000 | 56.27 ms | 61.68 ms |
| 30,000 | 7,500 | 83.58 ms | 65.54 ms |
| 100,000 | 1 | 0.30 ms | 56.95 ms |
| 100,000 | 10,000 | 949.93 ms | 190.71 ms |
| 100,000 | 25,000 | 1,834.48 ms | 320.38 ms |

Fixture: eight raw plus eight resolved hops per transmission, two
observations, 2,048 relays; 480,000/1,600,000 hop entries. Timing
includes acquiring the store lock and omits unrelated secondary indexes.
Small batches now pay for the complete sweep; shared-host timing is
noisy.

Owning the bulk reader's arrays also has a measured cost on cold/bulk
recomputation, rather than cached hits. Snapshot medians from three
samples of ten iterations:

| Transmissions / relay nodes | Before time / bytes per operation |
After time / bytes per operation |
|---|---:|---:|
| 30,000 / 50 | 0.0068 ms / 5,416 B | 23.65 ms / 4,101,435 B |
| 30,000 / 2,000 | 0.1374 ms / 196,768 B | 26.77 ms / 4,274,336 B |
| 100,000 / 2,000 | 0.1376 ms / 196,768 B | 27.89 ms / 13,959,337 B |

These are total snapshot costs, comparing the unsafe header-only
snapshot with owned pointer arrays. Cleanup guarantees here apply to
`byPathHop`; other indexes and existing periodic bulk-cache freshness
are outside this change.

Following #1922, this runtime fix is separate from the release-routing
and frontend-runner PRs. Current Go and E2E job results should be
assessed separately from workflow-approval or staging-runner state.
 2026-09-06 21:00:44 +02:00
efitenandClaude Opus 5 eb3d71f8f6 perf(#1910): collapse concurrent /stats work and serve the count cache stale (#1963) 
Addresses #1910. The Observers page hangs on "Loading..." for 10-20s;
the reporter measured `/stats` at 10-17s under the mixed load that page
produces, while the same endpoint stays under 70ms at 8x concurrency
when it is the only one being hit.

## Cause

Two cache layers guard the expensive work and **neither has
single-flight**:

| | | |
|---|---|---|
| `handleStats` | 10s cache | releases `statsMu` before rebuilding
(`routes.go:774`) |
| `GetStoreStats` | 30s cache | releases `statsCacheMu` before scanning
(`store.go:2048`) |

Both do check, release, then work. The moment either window expires,
**every in-flight request does the whole thing itself**.

The expensive part is a range scan over 24h of `observations` with two
`SUM(CASE...)` over it. The column is indexed
(`idx_observations_timestamp`), but the scan still visits every row in
the window, and at 18k observers that is millions. The pool is
`SetMaxOpenConns(4)` (`db.go:111`), and the page fires stats, observers,
nodes, channels and clock-skew at once, so one cache miss turns a single
scan into a queue of them.

That is exactly the reported profile: fast alone, slow only when mixed.

## Changes

1. **Single-flight both layers.** Concurrent callers that miss the cache
wait for the first one's result instead of each running the same
queries.
2. **Serve the observation counts stale while refreshing in the
background.** An expired cache answers from the previous value and kicks
off one refresh, so a miss is never a wait.

Single-flight alone would not have fixed the hang: the first caller
still waits for the full scan. The second change is what removes it.

## Contract change, stated plainly

`TestGetStoreStats_CacheExpiry` asserted that an expired cache returns
**fresh DB values on the same call**. It no longer does.

For `packetsLastHour` / `packetsLast24h` on a dashboard, answering with
a value up to ~30s older instead of blocking for seconds looks like the
right trade to me. But that is a judgement, not a bug fix, and **a
reviewer should be able to reject it**. I did not quietly delete the
test: it now asserts what still has to hold, that the refresh happens,
and the new behaviour is pinned separately by
`TestGetStoreStats_StaleCacheServedWithoutBlocking`.

If you would rather keep the old contract, drop change 2 and keep change
1; the diff separates cleanly.

## Verification

The new test **fails without the change**:

```
stale cache not served: got (0, 2), want (424242, 434343). An expired cache must
answer from the previous value and refresh in the background, not block the
request on the observations scan
```

`gofmt` clean, `go vet` clean, `cmd/server` suite ok in 267s.

## Two things I did not verify

**The race detector.** It needs cgo and there is no gcc on this machine,
so `go test -race` cannot run here. This change adds a background
goroutine writing the cache under `statsCacheMu`, so that check matters.
CI runs `go test -timeout 20m -race` for `cmd/server`
(`deploy.yml:134`), which covers it before merge.

**The 10-17s itself.** I have no database with 18k observers. The
mechanism above explains the reported profile, including why the
endpoint is fast in isolation, but I did not measure the figure.
@dborup, if you can run a build from this branch, the number to watch is
`/stats` under the same mixed-load command from your issue.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-05 09:57:44 +02:00
efitenandClaude Opus 5 71b892ba95 fix(#1899): scope the channel preview to the region filter (#1961) 
Closes #1899.

The Channels sidebar preview (`lastMessage` / `lastSender`) ignored the
active region filter, so an operator filtering on their own region saw a
preview line from a message their observers never heard.

## Cause

`GetChannels` scoped `msg_count` and `last_activity` correctly: the
outer query joins `observations` and `observers` and filters on IATA.
The `sample_json` subquery that feeds the preview joined neither, so it
always returned the globally newest message on the channel.

## Fix

Both region-filtered branches now scope the subquery the way the outer
query does: v3 through `observations`/`observers` on `observer_idx`, v2
through the `EXISTS` on `observer_id`. The unfiltered branch is
untouched, since there is no filter for it to respect.

**One thing that is easy to get wrong here:** the subquery sits in the
SELECT list, *ahead of* the WHERE, so its placeholders bind first. The
region codes are appended twice, subquery set first, or every filtered
call binds the wrong values.

**Scope checked rather than assumed:** `GetEncryptedChannels` has the
same shape and the same `regionPlaceholder` pattern, but selects no
`sample_json`, so it does not have this bug and is left alone.

## Verification

The regression test **fails on unmodified master**, with the reported
symptom:

```
db_test.go:2465: preview sender = Bob, want Alice: SJC must not be shown Bob's
                 message, which only SFO heard
db_test.go:2469: preview message = heard in SFO, want "heard in SJC"
```

It asserts both directions, so it cannot pass by always picking the
oldest row, and it asserts the unfiltered call still shows the globally
newest message, which was never in question.

`gofmt` clean, `go vet` clean, `cmd/server` suite ok in 98.5s.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-04 17:04:54 +02:00
efitenandClaude Opus 5 e7b3a2e77f chore(#1856): remove POST /api/packets, which has never worked (#1959) 
Part 1 of #1856. Part 2 (the hash migration reporting false success) is
#1958.

## It has never worked

`handlePostPacket` writes to the server's DB handle, and that handle is
read-only. `cmd/server/db.go:106`:

```go
dsn := fmt.Sprintf("file:%s?mode=ro&_journal_mode=WAL&_busy_timeout=5000", path)
```

Every call answered `500 attempt to write a readonly database`.

**This is the second report.** #1196 raised it on 2026-06-13, a fix was
merged that corrected v2 column names to v3, and the issue was closed.
That fix could not have worked, because the column names were never why
the write failed. Its comment is still sitting at `routes.go:1288`, next
to code that has never executed successfully in production.

## Why remove rather than build a handoff

**It cannot break a caller.** An endpoint that has only ever returned
500 has no working consumer. This is not a breaking API change, it is
documentation catching up with reality. Nothing in `public/` calls it.

**It was actively misleading.** `openapi.go` advertised it as "Ingest a
packet" and it sits behind `requireAPIKey`, which reads as a live,
protected write endpoint.

**Its test hid the breakage.** `TestPostPacketPersistsV3Schema` asserted
the observation row is written and passed for four months, because the
test DB is opened read-write while production is not. That is how #1196
came to be closed as fixed.

**Ingest is MQTT-only by design since #1283.** Re-adding an HTTP write
path re-opens the invariant that change established. If manual injection
is wanted later for testing or replay, it belongs on the ingestor side
and deserves its own issue. The repository already has the handoff shape
for that: the server writes `request-<id>.json` and the ingestor
consumes it (`cmd/ingestor/prune_geofilter.go`).

## What went

The route, `handlePostPacket` (103 lines), the now-unused
`PacketIngestResponse` type, the `openapi.go` entry, the round-trip
test, and the section plus table-of-contents line in `docs/api-spec.md`.
The `packetpath` import in `routes.go` became unused and went with it.

`+4/-225` across 6 files.

## The auth tests

The four `requireAPIKey` tests used `"/api/packets"` only as a request
path while building their own handler with `s.requireAPIKey(...)`, so
they never touched the route.

I checked that by **running them**, not by reading the code:

```
--- PASS: TestRequireAPIKey_RejectsWeakKey
--- PASS: TestRequireAPIKey_AcceptsStrongKey
--- PASS: TestRequireAPIKey_EmptyKeyDisablesEndpoints
--- PASS: TestRequireAPIKey_WrongKeyUnauthorized
```

Their paths now point at `/api/admin/prune-geo-filter`, which still
exists, so they no longer name a removed endpoint. Re-ran after that
change: still 4 of 4.

`/api/packets/observations` is a different endpoint and is untouched.

Verified: `gofmt` clean, `go vet` clean, `cmd/server` suite ok in 62.9s.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-04 16:02:59 +02:00
efitenandClaude Opus 5 56d6d4c722 fix(#1856): stop the hash migration reporting success it never achieved (#1958) 
Part 2 of #1856. **Part 1 is deliberately not fixed here** and the issue
stays open for it; reasoning at the end.

## The bug

`migrateContentHashesAsync` set `store.hashMigrationComplete` in a
deferred func that ran unconditionally. Every DB failure inside the loop
takes a `continue` (begin tx, prepare, commit), so the loop always
reaches that defer, **including when not a single batch was written**.

That is not hypothetical. The server has held a `mode=ro` handle since
#1283, so `Begin`, `Prepare` and `Commit` all fail, every batch is
skipped, and `/api/stats` then answers `hashMigrationComplete: true`
after migrating nothing. The migration is started unconditionally on
every boot at `main.go:546`.

## The fix

The three failure paths now count, and the defer only claims completion
when the count is zero. When it is not, it logs once, naming the
read-only handle as the expected cause and pointing at this issue, so an
operator can tell "no work to do" apart from "could not do the work".

**Nothing waits on the flag.** The only reader is `routes.go:828`, which
reports it in `/api/stats`. Leaving it false on failure blocks nothing;
it just stops the endpoint from lying.

The in-memory index is untouched on failure. That was already true,
because the index update runs only after a successful commit, and the
test now asserts it so memory and disk cannot drift apart.

## Verification

The regression test **fails on unmodified master**:

```
hash_migrate_test.go:115: hashMigrationComplete must stay false when no batch
could be written; reporting true here is what #1856 called self-reported success
```

It closes the DB handle to make writes fail. That is deterministic and
exercises the identical path as a read-only handle (`Begin` errors,
batch skipped); the in-memory test DB cannot be reopened read-only.

The existing happy-path test still passes, so the flag still turns true
on a real migration. `gofmt` clean, `go vet` clean, `cmd/server` suite
ok in 59.7s.

## Why part 1 is not in here

`handlePostPacket` writes to the same read-only handle and therefore
always answers 500. I checked the error path before assuming it was
misleading: it already returns `"transmission insert: attempt to write a
readonly database"`, so the message is accurate. The endpoint is not
confusing, it is simply dead.

The issue asks maintainers directly: *"is this endpoint still wanted? If
ingestion is MQTT-only now, deleting it is simpler than routing it
through a handoff."* That is a product decision, not a fix, and
inventing a middle answer would only add code without settling it. Worth
noting the repository already has a precedent for the handoff shape: the
server writes `request-<id>.json` and the ingestor consumes it
(`cmd/ingestor/prune_geofilter.go`).

Two things a decision should account for: the endpoint is documented in
`openapi.go:69` and guarded by `requireAPIKey`, and
`routes_test.go:4850` asserts it writes an observation row using the v3
schema, which passes only because the test DB is read-write.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-04 15:15:16 +02:00
efitenSaarMesh-BotClaude Opus 5
 		40f664c587 chore(#1859): gofmt sweep + gofmt/go vet CI gate (rebase of #1881) (#1941) 
Rebase of #1881 by @SaarMesh-Bot onto current master. Their three
commits are preserved, two of them cherry-picked with authorship intact;
the sweep itself had to be regenerated. Opened as a new PR rather than
force-pushing their branch.

Closes #1881 once merged. Addresses parts 1 and 3 of #1859; part 2
landed as #1937.

## Why regenerated rather than merged

The sweep in #1881 was cut on 2026-09-02 07:13 and roughly forty PRs
landed after it, so it went `CONFLICTING/DIRTY`. Re-running `gofmt` on
current master is cheaper and less error-prone than resolving 72
conflicts that are all whitespace. The drift it fixes also grew in the
meantime: 66 files now, against 72 then, but spread differently.

## The three commits

1. **`style(#1859)`** — `gofmt -w` across the 14 modules. 66 files.
2. **`test(#1859)`** — @SaarMesh-Bot's fix for the one `go vet`
copylocks finding, `cmd/ingestor/coverage_boost_test.go`: the range
variable copied a `Config` embedding `sync.Once`. Cherry-picked
unchanged.
3. **`ci(#1859)`** — @SaarMesh-Bot's CI step that fails on gofmt drift
or vet findings, plus `.git-blame-ignore-revs`. Cherry-picked with one
change, noted in the commit message: the ignore file pointed at
`04bc80ee`, the sweep commit on their branch, which does not exist on
this base and would make `git blame --ignore-revs-file` error. Repointed
at `d3a02599`, the sweep here.

## Verification

The claim "formatting only" is checked twice rather than asserted:

- Every changed file is byte-identical to `gofmt(previous content)`. 0
of 66 deviate.
- With line comments and all whitespace stripped, 0 of 66 files differ,
so no code outside comments changed.

14 of the 66 also show doc-comment reflow. Since Go 1.19 `gofmt`
re-indents indented comment blocks to tabs and inserts a blank comment
line before them; the behavior matrix above `resolveHopWithContext` in
`cmd/ingestor/path_resolver.go` is a clear example. That is gofmt's own
output, not an edit, but it is worth naming because it makes the diff
look larger than "whitespace" suggests.

The gate was run locally exactly as the workflow runs it: `gofmt` clean,
and `go vet` clean in all 14 modules, including `cmd/ingestor` which is
what commit 2 fixes.

Suites: `cmd/server` ok (80.7s), `internal/packetpath` ok (2.3s),
`cmd/ingestor` passes except
`TestWriteStatsAtomic_SymlinkAtDestIsReplaced`, which fails identically
on bare master with "A required privilege is not held by the client"
(Windows symlink privilege on my host, not code).

## Sequencing

This should go last in the queue. The sweep touches 66 files, so merging
it before the remaining open Go PRs gives each of them a conflict about
nothing but formatting. After it lands the gate is active, and any PR
with drift fails CI until it runs `gofmt -w`.

Excluded from the sweep: the misnamed `Dockerfile.go`, which is a
Dockerfile that gofmt cannot parse (the workflow excludes it too), and
`docs/DEPLOYMENT.md`, which a case-insensitive filesystem surfaces as a
spurious modification against `docs/deployment.md` and is unrelated.

---------

Co-authored-by: SaarMesh-Bot <300107934+SaarMesh-Bot@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-03 18:52:03 +02:00
Joel ClawandJoel Claw ac6fbaf9f3 perf: reuse ctx buffer in resolvePathForObs, cache ReadMemStats per store (#1873) 
## Problem

Three hot-path inefficiencies causing excess CPU and memory allocations:

### 1. `filterTxSlice` starts with nil slice
`filterTxSlice` is called on the full `s.packets` slice (50k+ packets)
for every query that doesn't hit a fast-path index. Starting with `var
result []*StoreTx` means Go's append does ~15 growth+copy cycles
(1→2→4→8→...→32768→65536) before reaching steady state.

### 2. `resolvePathForObs` allocates per hop
Each hop in the path resolution loop allocates a new `ctx` slice
(`make([]string, len(contextPKs), len(contextPKs)+2)`). For a 5-hop
path, that's 5 allocations per observation. With 500+ observations per
ingest batch, that's 2500+ small allocations.

### 3. `estimatedMemoryMB` calls `runtime.ReadMemStats` without caching
`runtime.ReadMemStats()` triggers a STW (stop-the-world) pause. It's
called from stats/debug endpoints (`GetStoreStats`, `GetPerfStoreStats`)
that may be polled frequently. The routes.go layer already caches this
with a 5s TTL, but the store layer doesn't.

## Fix

1. **Pre-allocate `filterTxSlice`**: `make([]*StoreTx, 0, n/2)` — the 2x
over-allocation is cheaper than repeated growth+copy.

2. **Reuse ctx buffer**: Allocate one `ctx` buffer before the hop loop,
reset to base length each iteration with `ctx = ctx[:ctxLen]`.

3. **Cache `ReadMemStats`**: 5-second TTL cache matching the routes.go
pattern. Uses a package-level mutex (not on `PacketStore`) to avoid
adding a field.

## Testing
- `go build` passes
- No behavior change — same results, fewer allocations

---------

Co-authored-by: Joel Claw <358739783+Joel-Claw@users.noreply.github.com>
 2026-09-02 23:20:39 +02:00