diff --git a/.claude/hooks/guard-bash.py b/.claude/hooks/guard-bash.py new file mode 100644 index 0000000..cf21e02 --- /dev/null +++ b/.claude/hooks/guard-bash.py @@ -0,0 +1,107 @@ +# Copyright © 2025-26 l5yth & contributors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""PreToolUse guard for Bash: deny destructive actions, confirm risky ones. + +Enforces the risky-action policy from the Phase 2 environment audit so that +destructive git, recursive deletion, publishing, secret reads, and external +network egress cannot run by accident. +""" + +import json +import re +import sys + + +def decision(kind, reason): + """Emit a PreToolUse permission decision (``deny`` or ``ask``) and exit.""" + print( + json.dumps( + { + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "permissionDecision": kind, + "permissionDecisionReason": reason, + } + } + ) + ) + sys.exit(0) + + +DENY = [ + ( + re.compile(r"\bgit\s+push\b[^\n]*(--force\b|--force-with-lease\b|(? + + +# PotatoMesh — Acceptance Criteria + +> **Purpose.** Precise, command-backed pass/fail criteria for the invariants and +> decisions in [`SPEC.md`](./SPEC.md). A reviewer with **zero context from the +> design session** can judge a result against this file alone: run the command, +> compare to the expected result, record PASS/FAIL. +> +> **Format sources (cited per the kickoff protocol).** The engineering-bar +> criteria (Layer B) restate [`CLAUDE.md`](./CLAUDE.md); the API/event-contract +> criteria (Layer C) restate +> [`data/mesh_ingestor/CONTRACTS.md`](./data/mesh_ingestor/CONTRACTS.md). Those +> two files are authoritative if any wording here drifts. + +## How to use this document + +1. Do the one-time **Setup** below. +2. Run each check in Layers **A–D**. Each check states a **command** and an + **Expected** result. Commands are written for a POSIX shell at the **repo + root** unless noted. +3. Record **PASS/FAIL** per check, pasting the command output. +4. Apply the **Verdict rule**. Pre-existing, tracked deviations are listed under + [§ Known gaps](#known-gaps); they remain FAIL until fixed. + +### Setup (one-time) + +```bash +# Web (Ruby + JS) +( cd web && bundle install && npm ci ) +# Python ingestor +python -m venv .venv && . .venv/bin/activate \ + && pip install -r data/requirements.txt black pytest pytest-cov +# Rust bridge: stable toolchain + cargo (rustup) # for Layer B/D +# Flutter app: flutter SDK on PATH # for Layer B/D +``` + +### Test server helpers + +Some checks need a running web app. Start it with the env the check specifies, +then `kill` it afterward. Examples: + +```bash +# Privacy checks (Layer A2): private mode, federation off +( cd web && API_TOKEN=acctest PRIVATE=1 FEDERATION=0 bundle exec rackup -p 41447 ) & SRV=$! +# Auth / contract checks (Layer C): public mode, known token +( cd web && API_TOKEN=acctest PRIVATE=0 FEDERATION=0 bundle exec rackup -p 41447 ) & SRV=$! +# ... run curl checks ... +kill "$SRV" +``` + +### Verdict rule + +A result **PASSES** acceptance only when **every** check in Layers A, B, and C +passes and **every** Layer-D check matches documented behavior. Any FAIL not +already listed in [§ Known gaps](#known-gaps) blocks acceptance. The apex check +**A1** is a hard gate: a FAIL there fails the whole review regardless of anything +else (SPEC §1). + +--- + +## Layer A — Invariant conformance + +Maps to SPEC §1–§2 and decisions **D2, D3, D4**. + +### A1 — Apex: no MQTT / cloud data path *(hard gate)* — SPEC Invariant I + +**A1a. No broker/cloud-bus dependency in any manifest.** +```bash +git grep -niE 'mqtt|mosquitto|paho|amqp|kafka|broker' -- \ + web/Gemfile web/Gemfile.lock data/requirements.txt \ + matrix/Cargo.toml matrix/Cargo.lock app/pubspec.yaml app/pubspec.lock +``` +**Expected:** no output. + +**A1b. No broker connection in code (provenance flag excepted).** +```bash +git grep -niE 'mqtt|mosquitto|paho|amqp|kafka|broker' -- \ + '*.rb' '*.py' '*.rs' '*.dart' '*.js' | grep -viE 'via_?mqtt' +``` +**Expected:** no output. The only legitimate matches are Meshtastic's +`via_mqtt` / `viaMqtt` **provenance flag** (`data/mesh_ingestor/handlers/nodeinfo.py`), +which is filtered out here and is explicitly permitted by SPEC §1 (it is metadata +about a *foreign* node, not PotatoMesh acting as an MQTT client). + +### A2 — Privacy & consent first — SPEC Invariant II + +*Run the server with `PRIVATE=1`.* + +**A2a. Message API is disabled in private mode.** +```bash +curl -s -o /dev/null -w 'GET %{http_code}\n' http://127.0.0.1:41447/api/messages +curl -s -o /dev/null -w 'POST %{http_code}\n' -X POST \ + -H 'Authorization: Bearer acctest' http://127.0.0.1:41447/api/messages -d '[]' +``` +**Expected:** both `404` (the `before "/api/messages*"` filter halts 404 in +private mode — `web/lib/potato_mesh/application/routes/api.rb:49`). + +**A2b. Private flag is advertised (the client uses it to hide chat).** +```bash +curl -s http://127.0.0.1:41447/version | grep -o '"privateMode":true' +``` +**Expected:** prints `"privateMode":true`. + +**A2c. Node opt-out marker is honored wherever data is listed/exported.** +```bash +git grep -lE 'opt_out_self_filter|opt_out_node_id_filter|NODE_OPT_OUT_MARKER' -- web/lib | sort +``` +**Expected:** the opt-out filter appears in the read/export paths — at minimum +`application/queries/chat_queries.rb`, `application/identity.rb`, and +`application/federation/instance_metrics.rb`. Behavior is covered by the Ruby +suite (Layer B1). + +### A3 — Decentralized, opt-in federation; `PRIVATE` > `FEDERATION` — SPEC Invariant III, D4 + +**A3a. `federation_enabled?` is opt-in and overridden by privacy.** Open both +definitions and confirm the predicate is true only when `FEDERATION` is on **and** +the instance is **not** private: +```bash +git grep -nA12 'def federation_enabled\?' -- \ + web/lib/potato_mesh/config.rb web/lib/potato_mesh/application/helpers/config_helpers.rb +``` +**Expected:** the logic requires federation enabled **and** `!private_mode?` +(concrete form of Privacy > Federation, SPEC §3.1). + +**A3b. No central authority / hardcoded directory host.** Peers are discovered by +crawl, not from a baked-in registry: +```bash +git grep -nhoE 'https?://[A-Za-z0-9.-]+' -- web/lib/potato_mesh/application/federation \ + | grep -viE 'apache\.org|w3\.org|schema|example|localhost|127\.0\.0\.1' | sort -u +``` +**Expected:** no hardcoded third-party "central" host (matches are only standards +URLs in comments, if any). + +**A3c. Federation behavior is covered by tests.** +```bash +( cd web && bundle exec rspec spec -e federation ) +``` +**Expected:** federation specs pass (opt-in, isolation when `FEDERATION=0`, +privacy override, staleness eviction). + +### A4 — Protocol parity & pluggability — SPEC Invariant IV + +**A4a. Both protocols are first-class, neither privileged.** +```bash +git grep -n 'KNOWN_PROTOCOLS' -- web/lib/potato_mesh/application/routes/api.rb +``` +**Expected:** the whitelist is exactly `meshcore` + `meshtastic` +(`KNOWN_PROTOCOLS = Set.new(%w[meshcore meshtastic])`); classification is +data-driven, not a per-protocol control-flow fork. + +**A4b. A protocol plugs in behind `MeshProtocol` without touching the read-side.** +```bash +. .venv/bin/activate && pytest -q tests/test_provider_unit.py +``` +**Expected:** pass (includes an `isinstance(..., MeshProtocol)` conformance check +and error/retry paths). The contract that new protocols must preserve — and the +fact that the Ruby/DB/UI read-side stays unchanged — is documented in +`CONTRACTS.md` and the *"Adding a New Ingestor Protocol"* section of `CLAUDE.md`. + +### A4c — Chat name resolution honors protocol (no cross-protocol quoting) +```bash +( cd web && node --test public/assets/js/app/__tests__/meshcore-chat-helpers.test.js \ + public/assets/js/app/__tests__/chat-entry-renderer.test.js ) +``` +**Expected:** pass. In the chat UI a MeshCore message resolves a sender/quote/ +mention name **only** to a MeshCore node — never to a same-named Meshtastic node +(names collide across protocols, so the lookup must filter by the message's +protocol instead of taking the first match). When no same-protocol node matches, +a synthetic node carrying the message's protocol is rendered rather than +borrowing a node from another protocol (`findNodeByLongName(longName, nodesById, +protocol)` + `chat-entry-renderer.js`). Concrete UI form of SPEC Invariant IV +(protocol parity; neither protocol privileged in the data model or UI). + +--- + +## Layer B — Engineering bar (restated from `CLAUDE.md`) + +Maps to decision **D9**. Commands mirror the CI workflows so local results match CI. + +### B1 — All test suites green +```bash +( cd web && bundle exec rspec ) # Ruby +( cd web && npm test ) # JavaScript +( . .venv/bin/activate && pytest -q tests/ ) # Python +( cd matrix && cargo test --all --all-features ) # Rust +( cd app && flutter test ) # Flutter +``` +**Expected:** every suite exits 0. + +### B2 — Coverage: 100% target, 10% threshold, on project **and** patch +```bash +grep -A14 '^coverage:' .codecov.yml +``` +**Expected:** `status.project.default` **and** `status.patch.default` each set +`target: 100%` and `threshold: 10%`. Per-language coverage is produced by the +suites in B1 (SimpleCov for Ruby, `pytest-cov`, `cargo llvm-cov`, `flutter +--coverage`, V8 for JS) and enforced server-side by Codecov. +> See [§ Known gaps](#known-gaps): the `patch` block is currently missing. + +### B3 — 100% API documentation (language standard) +```bash +( cd matrix && RUSTDOCFLAGS='-D warnings' cargo doc --no-deps ) # Rust: no doc warnings +``` +**Expected:** `cargo doc` builds with no warnings. For Ruby (RDoc), Python +(PDoc), JS (JSDoc), and Dart (dartdoc) there is no single gating command, so the +criterion is: **every public module/class/method/function carries a doc comment +in the language standard** (plus inline comments where logic is non-obvious). +A reviewer confirms by opening each file changed in the diff; existing files such +as `web/lib/potato_mesh/application/data_processing/request_helpers.rb` show the +expected `@param`/`@return` RDoc density. + +### B4 — Apache v2 notice on every file (exact string) + +**B4a. Source files carry the full header.** +```bash +git ls-files '*.rb' '*.py' '*.js' '*.rs' '*.dart' \ + | grep -vE '(^|/)(vendor|node_modules|build|\.dart_tool)/' \ + | xargs grep -L 'Copyright © 2025-26 l5yth & contributors' +``` +**Expected:** no output (every source file contains the exact notice +`Copyright © 2025-26 l5yth & contributors`). + +**B4b. Non-source text files carry the 2-line notice** (where the format allows +comments): +```bash +git ls-files '*.yml' '*.yaml' '*.toml' 'Dockerfile' '*/Dockerfile' '*.md' '*.sh' '*.nix' \ + | xargs grep -L 'Copyright © 2025-26 l5yth & contributors' +``` +**Expected:** no output, except the documented exemptions in +[§ Known gaps / exemptions](#known-gaps) (formats without comment syntax — e.g. +JSON fixtures, `*.lock` files — are exempt). + +### B5 — Formatters & linters clean +```bash +( . .venv/bin/activate && black --check ./ ) # Python +( cd web && bundle exec rufo --check . ) # Ruby +( cd matrix && cargo fmt --all -- --check \ + && cargo clippy --all-targets --all-features -- -D warnings ) # Rust +( cd app && dart format --set-exit-if-changed . && flutter analyze ) # Flutter +``` +**Expected:** every command exits 0. + +### B6 — CI runs on PRs to `main` and pushes to `main` +```bash +for w in python ruby rust mobile javascript; do + echo "== $w =="; grep -A8 '^on:' ".github/workflows/$w.yml" +done +``` +**Expected:** each workflow triggers on `pull_request` and on `push` to `main`, +and covers the relevant suite(s) for the component(s) it touches. + +### B7 — Weekly Dependabot for every ecosystem +```bash +grep -E 'package-ecosystem|directory|interval' .github/dependabot.yml +``` +**Expected:** entries for `ruby` (`/web`), `npm` (`/web`), `python` (`/data`), +`cargo` (`/matrix`), `pub` (`/app`), and `github-actions` (`/`) — **every +language in the repo present**, each with `interval: "weekly"`. + +--- + +## Layer C — API & event contracts (restated from `CONTRACTS.md`) + +Maps to decision **D8**. *Run the server with `PRIVATE=0` and `API_TOKEN=acctest`.* + +### C1 — POST routes require a valid bearer token +```bash +curl -s -o /dev/null -w 'no-token %{http_code}\n' \ + -X POST http://127.0.0.1:41447/api/nodes -d '{}' +curl -s -o /dev/null -w 'wrong-token %{http_code}\n' \ + -X POST -H 'Authorization: Bearer wrong' http://127.0.0.1:41447/api/nodes -d '{}' +curl -s -o /dev/null -w 'good-token %{http_code}\n' \ + -X POST -H 'Authorization: Bearer acctest' http://127.0.0.1:41447/api/nodes -d '{}' +``` +**Expected:** `403` for missing and wrong tokens (constant-time compare in +`require_token!`); the valid-token request is **not** `403` (it is accepted, or +`400` only if the body is malformed). + +### C2 — Canonical payload shapes validated by the integration suite +```bash +. .venv/bin/activate && pytest -q tests/test_mesh.py +``` +**Expected:** pass. `CONTRACTS.md` states the `POST` shapes +(`nodes`/`messages`/`positions`/`telemetry`/`neighbors`/`traces`/`ingestors`), +sentinel normalization (issue #782), protocol stamping/propagation, and dedup are +"validated by existing tests (notably `tests/test_mesh.py`)." + +### C3 — Canonical node id is `!%08x` on both sides +```bash +git grep -nE '_canonical_node_id' -- data/mesh_ingestor/serialization.py +git grep -nE 'canonical_node_parts' -- web/lib/potato_mesh/application/data_processing.rb +. .venv/bin/activate && pytest -q tests/test_node_identity_unit.py tests/test_serialization_unit.py +``` +**Expected:** both normalizers exist; the id unit tests pass (lowercase 8-hex +`!abcdef01` form; dual numeric/canonical addressing). + +### C4 — GET window floors cannot be widened by the caller +```bash +git grep -nE 'week_seconds|four_weeks_seconds' -- web/lib/potato_mesh/config.rb +``` +**Expected:** the 7-day / 28-day window constants exist. Per `CONTRACTS.md` +("GET endpoint time windows"), `?since=` is clamped to `MAX(since, floor)`; +this clamp is exercised by the Ruby suite (B1). + +### C5 — Cross-ingestor dedup by id +```bash +git grep -nE 'MESHCORE_CONTENT_DEDUP_WINDOW_SECONDS' -- web/lib +``` +**Expected:** the content-dedup window constant exists. `messages.id` PRIMARY-KEY +collapse and the MeshCore content-dedup (issue #756) are covered by +`tests/test_mesh.py` (C2). Ids must fit in 53 bits (JS-safe). + +### C6 — Per-record protocol stamp precedence +**Expected (covered by C2 + A4):** an explicit per-record `protocol` (in the +`{meshtastic, meshcore}` whitelist) wins over the ingestor-heartbeat default, +which wins over `meshtastic` as the final fallback — exactly as `CONTRACTS.md` +("Protocol propagation") specifies. Values outside the whitelist fall through. + +### C7 — Chat feed is fully paginable within the window (issue #796 regression) +```bash +( cd web && bundle exec rspec spec/app_spec.rb -e "backward pagination" ) +``` +**Expected:** pass. `GET /api/messages` accepts a `before=` upper-bound +cursor that only *narrows* the result set (the 7-day floor and the per-request +`MAX_QUERY_LIMIT` cap are unchanged, so C4 still holds). With more than +`MAX_QUERY_LIMIT` messages inside the seven-day window, paging backward by +`before` recovers **every** in-window message instead of stalling at the newest +1000 — the landing page and `/chat` subpage page until the window is exhausted. + +--- + +## Layer D — Operator-facing behavior + +Maps to decisions **D10, D11** and the README. *Server env per check.* + +### D1 — Documented config surfaces through `/version` +```bash +curl -s http://127.0.0.1:41447/version +``` +**Expected:** a JSON `config` block exposing `siteName`, `channel`, `frequency`, +`contactLink`, `mapCenter` (`lat`/`lon`), `maxDistanceKm`, `instanceDomain`, and +`privateMode`, reflecting the env vars set at boot (README "Web App" table). + +### D2 — `ALLOWED_CHANNELS` / `HIDDEN_CHANNELS` enforced (ingestor) +```bash +. .venv/bin/activate && pytest -q tests/test_channels_unit.py +``` +**Expected:** pass. The allow-list discards all other channels *before* the +hidden filter; hidden channels are dropped (`data/mesh_ingestor/channels.py`). + +### D3 — Opt-out marker excludes nodes from public listings +```bash +git grep -lE 'opt_out_self_filter|NODE_OPT_OUT_MARKER' -- web/lib | sort +``` +**Expected:** the opt-out filter is applied across listing/export/federation +queries (same artifact as A2c); behavior covered by the Ruby suite (B1). + +### D4 — Retention & staleness windows are wired in +```bash +git grep -nE 'start_retention_worker|retention_thread|def .*retention' -- \ + web/lib/potato_mesh/application/retention.rb web/lib/potato_mesh/application.rb +``` +**Expected:** a retention worker is started by the app. Combined with the GET +floors (C4) and the README's federation windows (8 h peer refresh, 72 h staleness +eviction), stale data is bounded. Federation freshness lives in +`application/federation/validation.rb`. + +### D5 — WIP components are read-only (no radio, no new ingest path) — D10 + +**D5a. Matrix bridge touches no radio and posts to no ingest route.** +```bash +git grep -niE 'serial|bluetooth|/dev/tty|meshtastic|meshcore' -- matrix/src +git grep -niE '/api/(nodes|messages|positions|telemetry|neighbors|traces|ingestors)' -- matrix/src +``` +**Expected:** first command: no output (no radio). Second: only **read** usage of +the public API (the bridge consumes messages); **no POST to ingest routes.** + +**D5b. Mobile app is a GET-only reader.** +```bash +git grep -niE '\.post\(|/dev/tty|serial|bluetooth' -- app/lib +``` +**Expected:** no ingest `POST`, no radio interface — the app only `GET`s from the +public API. + +### D6 — Stack frozen per component (SPEC §3.2) — D7 +```bash +grep -E 'gem "sinatra"' web/Gemfile # Ruby + Sinatra ~> 4 +grep -E 'meshtastic|meshcore' data/requirements.txt # Python: both libs +grep -E 'axum|reqwest|tokio' matrix/Cargo.toml # Rust bridge +grep -E '^\s*flutter:' app/pubspec.yaml # Flutter app +``` +**Expected:** each manifest matches the locked stack; no language/framework swap. + +--- + +## Known gaps (pre-existing, tracked — not introduced by work under review) + +These deviate from the bar above and are surfaced by the Phase 2 environment +audit. They are **FAIL** until fixed, but a reviewer should attribute them to the +existing codebase, not to the change under review. + +- **B2 — `.codecov.yml` has no `patch` block.** It defines only + `coverage.status.project.default` (target 100% / threshold 10%); `CLAUDE.md` + requires the same on **patch**. Fix tracked in the Phase 2 audit. +- **B4 — header-check exemptions are conventional, not codified.** Formats + without comment syntax (JSON fixtures under `tests/`, `*.lock` files, binary + assets) cannot carry the notice; there is no committed allow-list or CI check + asserting headers. The B4 commands above are the interim verification. + +--- + +## Feature: Chat channel test-deprioritization + +Maps to SPEC decisions **F1–F4**. The ordering logic lives in +`web/public/assets/js/app/chat-log-tabs.js` (`buildChatTabModel`); behavior is +verified by the JS unit suite. + +### F-A1 — Three-tier channel ordering (default → custom → test) — F1 +```bash +( cd web && node --test public/assets/js/app/__tests__/chat-log-tabs.test.js ) +``` +**Expected:** pass. Given a default/primary channel (index 0, e.g. "Public"), a +custom channel (index > 0, e.g. "#BerlinMesh"), and a test channel (index > 0, +e.g. "#test"), `buildChatTabModel(...).channels` returns them in the order +**[default, custom, test]** — every test channel sorts after every non-test +channel regardless of 7-day activity. Within each tier the prior ordering +(message-count descending, then label alphabetical) is unchanged. + +### F-A2 — Word-boundary test detection (ping/test/bot), no false positives — F2 +```bash +( cd web && node --test public/assets/js/app/__tests__/chat-log-tabs.test.js ) +``` +**Expected:** pass. A channel label is classified **test** iff it contains the +standalone word `ping`, `test`, or `bot` (case-insensitive, matched at word +boundaries). So "#test", "Ping", "my bot", "test channel" are test; **"Camping", +"Robotics", "Contest", "Botswana" are NOT** and keep their custom-tier position. + +### F-A3 — Primary/default channel is never demoted — F3 +**Expected (covered by the F-A1 suite):** an index-0 channel whose label matches a +keyword (e.g. a primary literally named "test") still sorts in the default tier +(first), never the test tier — the main community feed always leads. + +### F-A4 — Presentation-only, protocol-neutral — F4 +**Expected (covered by the F-A1 suite + A4c):** reordering changes only tab +order — each channel's `messageCount`, `entries`, and `id` are unchanged, and the +default-active tab stays the primary. Detection is by channel name, so a MeshCore +"#test" and a Meshtastic "#test" are demoted identically (no protocol privileged). + +### F-R1 — Regression: prior acceptance still holds +```bash +( cd web && npm test ) && ( cd web && bundle exec rspec ) +``` +**Expected:** every prior check still passes. At risk and explicitly required to +remain green: **A4c** (chat name resolution honors protocol — same render path) +and **B1** (all suites). The existing two-tier ordering assertions in +`chat-log-tabs.test.js` are **updated** to the three-tier order, not removed. diff --git a/CLAUDE.md b/CLAUDE.md index 0533370..36ef3e6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -3,6 +3,21 @@ # Repository Guidelines +## Project Charter (read first) + +This repository is governed by [`SPEC.md`](SPEC.md) (product & engineering charter) +and [`ACCEPTANCE.md`](ACCEPTANCE.md) (command-backed pass/fail criteria). Before +changing behavior, confirm the change honors the four hard invariants and re-verify +the numbered decisions in `SPEC.md §6`: + +1. **Local LoRa only — never MQTT/cloud (apex).** No code path, dependency, or + feature may connect to or ingest from an MQTT broker or cloud message bus. +2. **Privacy & consent first** (`PRIVATE`, node opt-out marker, retention). +3. **Decentralized, opt-in federation** (no central authority; `PRIVATE` > `FEDERATION`). +4. **Protocol parity & pluggability** (Meshtastic/MeshCore equal; new protocols via `MeshProtocol`). + +A change is not "done" until it passes `ACCEPTANCE.md`. + Keep code as modular as possible to reduce duplication and improve reusability and readability — this applies to tests as well as production code. If a module grows large, split it into a submodule structure. Prefer composing small, single-purpose units over monolithic files. Make sure all tests pass for Python (`pytest`), Ruby (`rspec`), and JavaScript (`npm test`). diff --git a/README.md b/README.md index 023cde6..77db9c2 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ [![Matrix Chat](https://img.shields.io/badge/matrix-%23potatomesh:dod.ngo-blue)](https://matrix.to/#/#potatomesh:dod.ngo) [![Meshtastic](https://img.shields.io/badge/Meshtastic-supported-67ea94)](https://meshtastic.org) -[![MeshCore](https://img.shields.io/badge/MeshCore-supported-000000)](https://meshcore.co.uk) +[![MeshCore](https://img.shields.io/badge/MeshCore-supported-1f2937)](https://meshcore.io) A federated, Meshtastic & Meshcore node dashboard for your local community. _No MQTT clutter, just local LoRa aether._ diff --git a/SPEC.md b/SPEC.md new file mode 100644 index 0000000..ef828ef --- /dev/null +++ b/SPEC.md @@ -0,0 +1,220 @@ + + + +# PotatoMesh — Product & Engineering Charter (SPEC) + +> **Status:** Draft for confirmation (Phase 0 of the kickoff protocol). +> **Nature:** This is a *retrofit guardrail charter* for a mature, shipping +> project (v0.7). It does not design new behavior — it codifies the intent and +> non-negotiable invariants that already hold, **judged against current shipping +> behavior**, so future work by Claude or contributors cannot drift from them. +> The numbered decisions in [§6](#6-key-decisions-confirmation-checklist) must be +> **re-verified at every later checkpoint** (each build bucket, the independent +> review) to prevent drift. + +Companion document: [`ACCEPTANCE.md`](./ACCEPTANCE.md) turns every invariant and +decision below into a command-backed, zero-context pass/fail check. + +--- + +## 1. Vision & Apex + +**PotatoMesh is a federated Meshtastic & MeshCore node dashboard for a local +community. No MQTT clutter — just local LoRa aether.** + +It lets a community stand up its own dashboard fed only by radios its members +actually operate, optionally federate as equals with other communities, and do +so while respecting the privacy of operators and node owners. + +### Apex invariant — the line in the sand + +> **Local LoRa only. PotatoMesh must never connect to, depend on, or ingest from +> an MQTT broker or any cloud message bus.** + +This is the project's identity and its differentiator: every other Meshtastic +dashboard leans on MQTT/cloud. "Local aether only" is what makes PotatoMesh +*PotatoMesh*. When any other invariant, feature, or convenience collides with +this rule, **this rule wins.** Its loss would mean the project is no longer +PotatoMesh. + +**Precision (so the rule is enforceable, not superstitious):** the apex bans +PotatoMesh *acting as* an MQTT/cloud client or carrying such a dependency. It +does **not** ban *recording Meshtastic's own `via_mqtt` / `viaMqtt` provenance +flag* (`data/mesh_ingestor/handlers/nodeinfo.py`). That field is metadata about +how a *foreign* node was heard; surfacing it actually serves the invariant by +letting operators identify and reason about MQTT-bridged nodes. The acceptance +check targets dependencies and broker connections, not the substring `mqtt`. + +--- + +## 2. The Four Hard Invariants (ranked) + +All four are non-negotiable. They are listed in **priority order**: when two +collide, the higher-ranked one wins. In practice they rarely conflict; the only +conflict that occurs in the running system today (privacy vs. federation) is +already resolved below and in code. + +### I. Local LoRa only — never MQTT/cloud *(apex)* +The dashboard is fed exclusively by ingestors attached to physical radios +(serial / TCP / BLE) that push data through the authenticated `POST /api/*` +routes. No component pulls from MQTT or a cloud broker, and no manifest carries +a broker client. See [Apex](#apex-invariant--the-line-in-the-sand). + +### II. Privacy & consent first +`PRIVATE=1` hides the chat UI, disables the message APIs, and excludes hidden +clients from public listings. Node opt-out markers +(`PotatoMesh::Config::NODE_OPT_OUT_MARKER`) and data-retention policies +(`web/lib/potato_mesh/application/retention.rb`) are honored everywhere data is +read or exported. **When privacy collides with federation, privacy wins** — +`PRIVATE=1` always disables federation regardless of `FEDERATION`. Any change +that increases exposure of operators or node owners loses to consent, retention, +and opt-out. + +### III. Decentralized, opt-in federation +Instances discover and crawl one another as peers (`FEDERATION` toggle, periodic +well-known refresh, staleness eviction). There is **no central authority, +registry, or gatekeeper**; any instance can run fully isolated (`FEDERATION=0`) +and remain fully functional. Federation publishes only signed, public metadata +and respects remote `isPrivate` peers (`application/federation/crawl.rb`). + +### IV. Protocol parity & pluggability +Meshtastic and MeshCore are both first-class; neither is privileged in the data +model or UI. New protocols (e.g. Reticulum) plug in behind the `MeshProtocol` +abstraction (`data/mesh_ingestor/mesh_protocol.py`) and the canonical wire +contract (`data/mesh_ingestor/CONTRACTS.md`) **without changing the Ruby / DB / +UI read-side.** + +--- + +## 3. Cross-cutting decisions + +### 3.1 Invariant priority / tie-break order +**Local-LoRa (apex) → Privacy & consent → Federation → Parity.** Higher wins on +collision. The documented `PRIVATE` > `FEDERATION` rule is the concrete instance +of Privacy > Federation and must remain true in code. + +### 3.2 Fixed technology stack (per component) +The stack is a guardrail, not an implementation detail. It is **fixed** per +component; a rewrite into another language/framework requires a fresh kickoff, +not an incremental PR. + +| Component | Stack (locked) | +| --- | --- | +| `web/` | Ruby + **Sinatra ~> 4**, **SQLite** (sqlite3), Puma, Rackup, kramdown, sanitize, ferrum (headless Chromium for OG image), prometheus-client | +| `data/` | **Python** ingestor — `meshtastic`, `meshcore`, `bleak` (BLE), `protobuf`; `black` + `pytest` | +| `matrix/` | **Rust** — tokio, reqwest (rustls-tls), axum, serde, clap, tracing | +| `app/` | **Flutter / Dart** — http, shared_preferences, flutter_local_notifications, workmanager | + +### 3.3 The web app is data-in-by-POST only +The Sinatra app is **never** run attached to a radio. Its only data intake is the +authenticated `POST /api/*` surface; this is what allows many community ingestors +to feed one dashboard with no duplication (dedup by id). SQLite is the system of +record. + +### 3.4 Stable data & API contract +- Canonical node id is `!%08x` (lowercase 8-hex), treated as canonical + system-wide; new protocols must map their native ids into this space. +- The `POST`/`GET` route shapes and event schemas in + [`data/mesh_ingestor/CONTRACTS.md`](./data/mesh_ingestor/CONTRACTS.md) are the + contract. They evolve **backward-compatibly**; a breaking change must be + versioned (as the MeshCore dedup fingerprint already is: `v1:` prefix). +- `POST` routes require `Authorization: Bearer `; `GET` collection + routes enforce server-side rolling-window floors that callers cannot widen. + +### 3.5 Engineering quality bar (from `CLAUDE.md`, non-negotiable for new code) +- **100% unit test coverage** — every line, branch, and path. Codecov target + **100%**, threshold **10%**, enforced on **both `project` and `patch`**. +- **100% API documentation** to the language standard (PDoc / RDoc / JSDoc / + rustdoc / dartdoc), plus inline comments where logic is not self-evident. +- **Apache v2 notice on every file**, exact string + `Copyright © 2025-26 l5yth & contributors` — full header block for source + files, 2-line notice for non-source files. +- **Formatters clean**: `black` (Python), `rufo` (Ruby). +- **All suites green**: `pytest` (data), `rspec` + `npm test` (web), `cargo test` + (matrix), `flutter test` (app). +- **CI on every PR to `main` and every push to `main`**, covering each touched + language; **weekly Dependabot** for every ecosystem. +- **Modularity**: prefer small, single-purpose units; split modules that grow + large. + +--- + +## 4. Per-component scope + +### 4.1 `web/` — Sinatra dashboard *(mature)* +The only public surface and the system of record. Serves the map + chat UI and +the read APIs; accepts ingest via authenticated `POST`; performs federation +(well-known doc, peer crawl, staleness eviction), Prometheus `/metrics`, +OG-image generation, and custom Markdown pages. Enforces invariants II & III. + +### 4.2 `data/mesh_ingestor` — Python ingestor *(mature)* +The **only** component that touches radios and the **only** data source. Connects +over serial / TCP / BLE, normalizes Meshtastic **and** MeshCore packets to the +canonical contract, and POSTs them. Multiple ingestors per instance are +supported. Embodies invariants I & IV; honors `ALLOWED_CHANNELS` / +`HIDDEN_CHANNELS` and sentinel-position normalization. + +### 4.3 `matrix/` — Matrix bridge *(WIP, read-only)* +A one-way reader bridge: it **reads** messages from a PotatoMesh instance's +public API and forwards them to a configured Matrix channel. No radio. It is a +consumer of the public API and **must not introduce any new ingest path**; it +respects `PRIVATE` (no messages to forward when message APIs are disabled). + +### 4.4 `app/` — Flutter mobile app *(WIP, read-only)* +A read-only mobile **reader** of messages on the local aether. `GET`-only client; +no posting, no radio. Respects `PRIVATE`. + +> **WIP boundary:** the Matrix bridge and mobile app are feature-bounded as +> *readers* above, but are held to the **same engineering bar** (§3.5) as the +> mature components — 100% test/doc/license/CI applies to all code regardless of +> maturity. + +--- + +## 5. Non-goals (explicit) + +- **No MQTT/cloud ingest path — ever.** (Apex.) +- **No central federation authority, registry, or gatekeeper.** Federation is + peer-to-peer and opt-in. +- **No analytics, tracking, or phone-home.** The only outbound traffic is opt-in + federation of signed public metadata. +- **The web app is never radio-attached** — data arrives only via authenticated + `POST`. +- **No privileging of one mesh protocol** over another in the data model or UI. + +--- + +## 6. Key decisions (confirmation checklist) + +Per the kickoff protocol, **every item below must be confirmed explicitly** +before I proceed to `ACCEPTANCE.md`. Confirm all, or call out any `D#` to change. + +| # | Decision | Source | +| --- | --- | --- | +| **D1** | This SPEC is a **retrofit guardrail charter**, judged against current shipping behavior — not a design for new features. | interview | +| **D2** | **Apex invariant = Local-LoRa-only / never MQTT or cloud**, and it wins every collision. The ban targets broker dependencies & connections, **not** recording Meshtastic's `via_mqtt` provenance flag. | interview + code | +| **D3** | The **four hard invariants** (all non-negotiable): I Local-LoRa-only, II Privacy & consent, III Decentralized opt-in federation, IV Protocol parity & pluggability. | interview | +| **D4** | **Priority / tie-break order:** Local-LoRa → Privacy → Federation → Parity. `PRIVATE` > `FEDERATION` is preserved as the concrete Privacy > Federation rule. | proposed | +| **D5** | **Doc layout:** two root files — `SPEC.md` + `ACCEPTANCE.md` — each opening with vision + ranked invariants, then per-component sections. | interview | +| **D6** | **`ACCEPTANCE.md` enforces four layers**, each as a command-backed, zero-context check: (a) invariant conformance, (b) the restated engineering bar, (c) API & event contracts, (d) operator-facing behavior. | interview | +| **D7** | **Stack is fixed per component** (web=Ruby/Sinatra 4+SQLite, data=Python, matrix=Rust, app=Flutter); a language/framework rewrite needs a new kickoff. | proposed | +| **D8** | **Data/API contract is stable & backward-compatible**: canonical `!%08x` ids, the `CONTRACTS.md` shapes, `POST` auth, `GET` window floors; breaking changes must be versioned. | proposed + code | +| **D9** | **Engineering quality bar** (§3.5) is part of acceptance and applies to all new code: 100% tests, 100% docs, Apache headers, linters, CI on PR+push, weekly Dependabot, Codecov 100%/10% on project **and** patch. | CLAUDE.md | +| **D10** | **Component scope/status:** web + ingestor are mature (full feature acceptance); matrix bridge = one-way reader, mobile app = read-only reader (both WIP, no radio, no new ingest path) — all held to the same engineering bar. | README + interview | +| **D11** | **Non-goals** (§5) are in force: no MQTT ingest, no central federation authority, no analytics/phone-home, web never radio-attached, no protocol privileging. | proposed | + +--- + +## Feature: Chat channel test-deprioritization + +Pushes throwaway "test"/"ping"/"bot" channels to the end of the chat channel +tabs so a community's real channels lead. Presentation-only; integrates solely +with the channel-ordering sort in +`web/public/assets/js/app/chat-log-tabs.js` (`buildChatTabModel`). + +| # | Decision | Source | +| --- | --- | --- | +| **F1** | **Three-tier channel-tab ordering** in the dashboard and `/chat`: (1) default/primary channels (channel index 0 — e.g. Public, MediumFast, "0"); (2) custom channels (index > 0, e.g. hashtag channels); (3) **test channels last**. Within each tier the existing ordering is preserved unchanged: 7-day message-count descending, then label alphabetical. | interview | +| **F2** | **Test-channel detection** is by the channel's resolved display **label**: the label contains the standalone word `ping`, `test`, or `bot`, case-insensitive, matched at **word boundaries**. So "Camping", "Robotics", "Contest", "Botswana" are **not** test channels; concatenated forms ("MyBot", "test2") are intentionally **not** matched either — the rule favors zero false positives over catching every variant. | interview | +| **F3** | **Default/primary channels are never demoted.** Test classification only reorders custom (index > 0) channels; an index-0 channel always leads even if its name matches a keyword, so the primary community feed is never hidden. | interview | +| **F4** | **Presentation-only & protocol-neutral.** Reorders tabs only — no change to channel membership, message contents/counts, the default-active tab (still the primary), or any data/API surface. Detection is by channel name and identical for MeshCore and Meshtastic, so the change **extends** Invariant IV (protocol parity) without privileging either protocol. | interview | diff --git a/app/analysis_options.yaml b/app/analysis_options.yaml index 0d29021..ff4ae7a 100644 --- a/app/analysis_options.yaml +++ b/app/analysis_options.yaml @@ -1,3 +1,5 @@ +# Copyright © 2025-26 l5yth & contributors +# Licensed under the Apache License, Version 2.0 (see LICENSE) # This file configures the analyzer, which statically analyzes Dart code to # check for errors, warnings, and lints. # diff --git a/app/pubspec.yaml b/app/pubspec.yaml index abe71b3..dffe906 100644 --- a/app/pubspec.yaml +++ b/app/pubspec.yaml @@ -1,3 +1,5 @@ +# Copyright © 2025-26 l5yth & contributors +# Licensed under the Apache License, Version 2.0 (see LICENSE) name: potato_mesh_reader description: Meshtastic Reader — read-only view for PotatoMesh messages. publish_to: "none" diff --git a/flake.nix b/flake.nix index e4caad8..a5f23b3 100644 --- a/flake.nix +++ b/flake.nix @@ -1,3 +1,5 @@ +# Copyright © 2025-26 l5yth & contributors +# Licensed under the Apache License, Version 2.0 (see LICENSE) { description = "PotatoMesh - A federated, Meshtastic-powered node dashboard"; diff --git a/matrix/Config.toml b/matrix/Config.toml index c0bb0f6..271f849 100644 --- a/matrix/Config.toml +++ b/matrix/Config.toml @@ -1,3 +1,5 @@ +# Copyright © 2025-26 l5yth & contributors +# Licensed under the Apache License, Version 2.0 (see LICENSE) [potatomesh] # Base domain (with or without trailing slash) base_url = "https://potatomesh.net" diff --git a/scripts/check-license-headers.sh b/scripts/check-license-headers.sh new file mode 100755 index 0000000..9e837ec --- /dev/null +++ b/scripts/check-license-headers.sh @@ -0,0 +1,22 @@ +#!/usr/bin/env bash +# Copyright © 2025-26 l5yth & contributors +# Licensed under the Apache License, Version 2.0 (see LICENSE) +# +# Fail if any tracked source or comment-capable config file is missing the +# exact Apache notice mandated by CLAUDE.md and ACCEPTANCE.md (check B4). +set -euo pipefail + +NOTICE='Copyright © 2025-26 l5yth & contributors' + +missing=$(git ls-files \ + '*.rb' '*.py' '*.js' '*.rs' '*.dart' \ + '*.yml' '*.yaml' '*.toml' '*.sh' '*.nix' 'Dockerfile' '*/Dockerfile' \ + | grep -vE '(^|/)(vendor|node_modules|build|\.dart_tool)/' \ + | xargs grep -L "$NOTICE" || true) + +if [ -n "$missing" ]; then + echo "Files missing the Apache notice ('${NOTICE}'):" + echo "$missing" + exit 1 +fi +echo "All checked files carry the Apache notice." diff --git a/web/lib/potato_mesh/application/queries/chat_queries.rb b/web/lib/potato_mesh/application/queries/chat_queries.rb index 9c9a000..32b021d 100644 --- a/web/lib/potato_mesh/application/queries/chat_queries.rb +++ b/web/lib/potato_mesh/application/queries/chat_queries.rb @@ -23,8 +23,10 @@ module PotatoMesh # @param node_ref [String, Integer, nil] optional node reference to scope results. # @param include_encrypted [Boolean] when true, include encrypted payloads in the response. # @param since [Integer] unix timestamp threshold; messages with rx_time older than this are excluded. + # @param before [Integer, nil] inclusive upper-bound rx_time cursor used for + # backward pagination (issue #796); messages newer than this are excluded. # @return [Array] compacted message rows safe for API responses. - def query_messages(limit, node_ref: nil, include_encrypted: false, since: 0, protocol: nil) + def query_messages(limit, node_ref: nil, include_encrypted: false, since: 0, before: nil, protocol: nil) limit = coerce_query_limit(limit) now = Time.now.to_i # Default the chat feed to the same seven-day window the dashboard uses @@ -42,6 +44,19 @@ module PotatoMesh where_clauses << "m.rx_time >= ?" params << since_threshold + # Upper-bound cursor for backward pagination (issue #796). When set, + # +before+ is an *inclusive* ceiling on +rx_time+: the client walks it + # backward (newest -> oldest), passing the oldest +rx_time+ of each page + # as the next cursor and de-duplicating by +id+ client-side, so rows that + # share the boundary second are never skipped. Because the cursor only + # ever *narrows* the result set, the seven-day floor above still bounds + # the window — callers cannot use +before+ to reach further back. + before_cursor = coerce_positive_or_nil(before) + if before_cursor + where_clauses << "m.rx_time <= ?" + params << before_cursor + end + unless include_encrypted where_clauses << "COALESCE(TRIM(m.encrypted), '') = ''" end diff --git a/web/lib/potato_mesh/application/routes/api.rb b/web/lib/potato_mesh/application/routes/api.rb index 0d25af8..f287bdc 100644 --- a/web/lib/potato_mesh/application/routes/api.rb +++ b/web/lib/potato_mesh/application/routes/api.rb @@ -168,11 +168,16 @@ module PotatoMesh include_encrypted = coerce_boolean(params["encrypted"]) || false since = coerce_integer(params["since"]) since = 0 if since.nil? || since.negative? + # Upper-bound cursor for backward pagination (issue #796). A request + # carrying +before+ is a history page, so it bypasses the shared + # response cache (which only memoises the default newest-page feed). + before = coerce_integer(params["before"]) + before = nil if before && before <= 0 protocol = sanitize_protocol(params["protocol"]) enc_key = include_encrypted ? "1" : "0" - if since > 0 - json_body = query_messages(limit, include_encrypted: include_encrypted, since: since, protocol: protocol).to_json + if since > 0 || before + json_body = query_messages(limit, include_encrypted: include_encrypted, since: since, before: before, protocol: protocol).to_json etag Digest::MD5.hexdigest(json_body), kind: :weak api_cache_control json_body diff --git a/web/public/assets/js/app/__tests__/chat-entry-renderer.test.js b/web/public/assets/js/app/__tests__/chat-entry-renderer.test.js index 0c0e60b..d488346 100644 --- a/web/public/assets/js/app/__tests__/chat-entry-renderer.test.js +++ b/web/public/assets/js/app/__tests__/chat-entry-renderer.test.js @@ -39,8 +39,8 @@ function makeNode(overrides = {}) { // --------------------------------------------------------------------------- test('renderChatEntryContent: MeshCore channel leading @[Name] becomes reply prefix', () => { - const alice = makeNode({ node_id: '!11111111', short_name: 'AL', long_name: 'Alice' }); - const bob = makeNode({ node_id: '!22222222', short_name: 'BO', long_name: 'Bob' }); + const alice = makeNode({ node_id: '!11111111', short_name: 'AL', long_name: 'Alice', protocol: 'meshcore' }); + const bob = makeNode({ node_id: '!22222222', short_name: 'BO', long_name: 'Bob', protocol: 'meshcore' }); const nodesById = new Map([ [alice.node_id, alice], [bob.node_id, bob], @@ -71,7 +71,7 @@ test('renderChatEntryContent: MeshCore channel leading @[Name] becomes reply pre }); test('renderChatEntryContent: MeshCore channel leading @[Name] handles name whitespace', () => { - const timo = makeNode({ node_id: '!6aee769f', short_name: 'TI', long_name: '\u{1F4FA} Timo +' }); + const timo = makeNode({ node_id: '!6aee769f', short_name: 'TI', long_name: '\u{1F4FA} Timo +', protocol: 'meshcore' }); const nodesById = new Map([[timo.node_id, timo]]); const message = { text: 'Bob: @[ Timo +] vielleicht hat jemand einen tip', @@ -95,8 +95,8 @@ test('renderChatEntryContent: MeshCore channel leading @[Name] handles name whit }); test('renderChatEntryContent: MeshCore multi-mention body does NOT emit reply prefix', () => { - const alice = makeNode({ node_id: '!11111111', short_name: 'AL', long_name: 'Alice' }); - const bob = makeNode({ node_id: '!22222222', short_name: 'BO', long_name: 'Bob' }); + const alice = makeNode({ node_id: '!11111111', short_name: 'AL', long_name: 'Alice', protocol: 'meshcore' }); + const bob = makeNode({ node_id: '!22222222', short_name: 'BO', long_name: 'Bob', protocol: 'meshcore' }); const nodesById = new Map([[alice.node_id, alice], [bob.node_id, bob]]); const message = { text: 'X: @[Alice] and @[Bob] both', @@ -119,12 +119,60 @@ test('renderChatEntryContent: MeshCore multi-mention body does NOT emit reply pr assert.ok(html.includes('SHORT(BO|CLIENT|Bob)')); }); -test('renderChatEntryContent: leading mention with unresolved node still surfaces a reply prefix using the raw name (#727)', () => { +test('renderChatEntryContent: MeshCore reply does not quote a same-named Meshtastic node (protocol collision)', () => { + // A Meshtastic and a MeshCore node share the long name "Timo". The + // Meshtastic node is inserted first, so the protocol-blind lookup returns it. + // A MeshCore message quoting @[Timo] must badge the MeshCore node, never the + // Meshtastic one. + const meshtastic = makeNode({ node_id: '!10000001', short_name: 'MTMT', long_name: 'Timo', role: 'ROUTER', protocol: 'meshtastic' }); + const meshcore = makeNode({ node_id: '!20000002', short_name: 'MCMC', long_name: 'Timo', role: 'CLIENT', protocol: 'meshcore' }); + const nodesById = new Map([ + [meshtastic.node_id, meshtastic], + [meshcore.node_id, meshcore], + ]); + const message = { text: 'X: @[Timo] thanks!', protocol: 'meshcore', to_id: '^all' }; + + const { html } = renderChatEntryContent({ + message, + nodesById, + messagesById: new Map(), + renderShortHtml, + escapeHtml: esc, + renderEmojiHtml: emoji, + }); + + assert.ok(html.includes('chat-entry-reply'), 'leading mention becomes a reply prefix'); + assert.ok(html.includes('SHORT(MCMC|CLIENT|Timo)'), 'reply target must be the MeshCore node'); + assert.ok(!html.includes('MTMT'), 'reply must NOT quote the same-named Meshtastic node'); +}); + +test('renderChatEntryContent: MeshCore mention synthesises a node when only a same-named Meshtastic node exists', () => { + // Only a Meshtastic "Timo" is in the registry. A MeshCore message must NOT + // quote it; instead a synthetic MeshCore-stamped badge carrying the name is + // rendered (issue: don't quote meshtastic nodes in a meshcore message). + const meshtastic = makeNode({ node_id: '!10000001', short_name: 'MTMT', long_name: 'Timo', role: 'ROUTER', protocol: 'meshtastic' }); + const nodesById = new Map([[meshtastic.node_id, meshtastic]]); + const message = { text: 'X: hi @[Timo] and @[Timo]', protocol: 'meshcore', to_id: '^all' }; + + const { html } = renderChatEntryContent({ + message, + nodesById, + messagesById: new Map(), + renderShortHtml, + escapeHtml: esc, + renderEmojiHtml: emoji, + }); + + assert.ok(html.includes('SHORT(Timo|-|Timo)'), 'mention renders a synthetic node badge carrying the name'); + assert.ok(!html.includes('MTMT'), 'mention must NOT resolve to the same-named Meshtastic node'); +}); + +test('renderChatEntryContent: leading mention with unresolved node surfaces a reply prefix using a synthetic node badge (#727)', () => { // Production deployments cap ``/api/nodes`` at 1000 entries, so the global // registry can be missing nodes that recent messages reference. In that - // case the leading-mention-as-reply detection must still emit a reply - // prefix using the bare mention name, otherwise the body would render as - // ``@[Name] body...`` and look like an unresolved mention link. + // case the leading-mention-as-reply detection still emits a reply prefix, now + // backed by a protocol-stamped synthetic node badge (never a bare + // ``@[Name] body...`` leak, and never a same-named node from another protocol). const nodesById = new Map(); const message = { text: 'X: @[DA6ML/p] ja, klingt sehr gut', @@ -143,15 +191,16 @@ test('renderChatEntryContent: leading mention with unresolved node still surface assert.ok(html.includes('chat-entry-reply'), 'should include a reply prefix even without a node match'); assert.ok(html.includes('ESC(in reply to)'), 'reply prefix label is escaped'); - assert.ok(html.includes('ESC(DA6ML/p)'), 'mention name is shown verbatim (escaped)'); + assert.ok(html.includes('SHORT(DA6ML/p|-|DA6ML/p)'), 'mention renders as a synthetic node badge carrying the name'); assert.ok(html.includes('ESC(ja, klingt sehr gut)'), 'remaining text rendered after the prefix'); // The bare ``@[Name]`` form must NOT survive into the body. assert.ok(!html.includes('@[ESC('), 'unresolved mention should not leak into the body'); }); -test('renderChatEntryContent: inline (non-leading) mentions still render as escaped literals when unresolved', () => { - // Mentions that are NOT at the start are left as escaped literals — the - // reply-prefix fallback only applies to leading-mention-as-reply. +test('renderChatEntryContent: inline (non-leading) unresolved mentions render as synthetic node badges', () => { + // Mentions that are NOT at the start no longer fall back to an escaped + // ``@[Name]`` literal; they render a protocol-stamped synthetic node badge so + // the mention is honored without borrowing a node from another protocol. const nodesById = new Map(); const message = { text: 'X: hello @[Unknown] there', @@ -169,7 +218,8 @@ test('renderChatEntryContent: inline (non-leading) mentions still render as esca }); assert.ok(!html.includes('chat-entry-reply'), 'mid-text mention must not become reply prefix'); - assert.ok(html.includes('@[ESC(Unknown)]'), 'unresolved inline mention falls back to escaped literal'); + assert.ok(html.includes('SHORT(Unknown|-|Unknown)'), 'unresolved inline mention renders a synthetic node badge'); + assert.ok(!html.includes('@[ESC(Unknown)]'), 'bare escaped literal must not survive'); }); test('renderChatEntryContent: MeshCore DM leading mention also becomes reply prefix', () => { @@ -287,7 +337,7 @@ test('renderChatEntryContent: encrypted message without notice formatter returns // --------------------------------------------------------------------------- test('renderChatEntryContent: returns meshcoreSenderNode when prefix resolves against registry', () => { - const sender = makeNode({ node_id: '!11111111', short_name: 'SN', long_name: 'Sender' }); + const sender = makeNode({ node_id: '!11111111', short_name: 'SN', long_name: 'Sender', protocol: 'meshcore' }); const nodesById = new Map([[sender.node_id, sender]]); const message = { text: 'Sender: hello everyone', diff --git a/web/public/assets/js/app/__tests__/chat-log-tabs.test.js b/web/public/assets/js/app/__tests__/chat-log-tabs.test.js index d7682e8..206a37f 100644 --- a/web/public/assets/js/app/__tests__/chat-log-tabs.test.js +++ b/web/public/assets/js/app/__tests__/chat-log-tabs.test.js @@ -20,6 +20,7 @@ import assert from 'node:assert/strict'; import { CHAT_LOG_ENTRY_TYPES, buildChatTabModel, + isTestChannelLabel, MAX_CHANNEL_INDEX, normaliseChannelIndex, normaliseChannelName, @@ -27,6 +28,42 @@ import { } from '../chat-log-tabs.js'; const NOW = 1_000_000; + +// --------------------------------------------------------------------------- +// isTestChannelLabel — word-boundary ping/test/bot detection (SPEC F2) +// --------------------------------------------------------------------------- + +test('isTestChannelLabel: matches standalone keywords case-insensitively', () => { + for (const label of ['test', 'TEST', 'Ping', 'bot', '#test', '#ping', '#bot']) { + assert.equal(isTestChannelLabel(label), true, `${label} should be a test channel`); + } +}); + +test('isTestChannelLabel: matches a keyword as one word among others', () => { + for (const label of ['test channel', 'my bot', 'ping pong', 'daily-test', 'bot 2', 'EU ping']) { + assert.equal(isTestChannelLabel(label), true, `${label} should be a test channel`); + } +}); + +test('isTestChannelLabel: does NOT match keywords embedded in larger words', () => { + // The false positives the word-boundary rule exists to avoid (SPEC F2). + for (const label of ['Camping', 'Robotics', 'RobotWars', 'Contest', 'Botswana', 'Testing', 'testbed', 'MyBot', 'test2', 'pingu']) { + assert.equal(isTestChannelLabel(label), false, `${label} should NOT be a test channel`); + } +}); + +test('isTestChannelLabel: real default/custom channel names are not test channels', () => { + for (const label of ['Public', 'MediumFast', 'LongFast', '0', '#BerlinMesh', 'MeshTown']) { + assert.equal(isTestChannelLabel(label), false, `${label} should NOT be a test channel`); + } +}); + +test('isTestChannelLabel: non-string input returns false', () => { + assert.equal(isTestChannelLabel(null), false); + assert.equal(isTestChannelLabel(undefined), false); + assert.equal(isTestChannelLabel(7), false); + assert.equal(isTestChannelLabel(''), false); +}); const WINDOW = 60 * 60; // one hour function fixtureNodes() { @@ -92,8 +129,10 @@ test('buildChatTabModel returns sorted nodes and channel buckets', () => { ); assert.equal(model.channels.length, 6); - // Primary channels (index 0) come first, secondary channels (index > 0) come last. - // Within each tier, ties on messageCount are broken alphabetically by label. + // Default/primary channels (index 0) lead, then custom channels (index > 0); + // these fixtures contain no test channels, so the third (test) tier is + // exercised by the dedicated three-tier ordering tests below. Within each + // tier, ties on messageCount are broken alphabetically by label. assert.deepEqual(model.channels.map(channel => channel.label), [ 'EnvDefault', 'Fallback', @@ -142,6 +181,65 @@ test('buildChatTabModel skips channel buckets when there are no messages', () => assert.equal(model.channels.length, 0); }); +// --------------------------------------------------------------------------- +// Three-tier channel ordering: default -> custom -> test (SPEC F1/F3/F4) +// --------------------------------------------------------------------------- + +test('buildChatTabModel sinks test channels below custom channels even with more activity (F1)', () => { + const model = buildChatTabModel({ + nodes: [], + messages: [ + // Custom channel, low activity (1 message). + { id: 'c1', rx_time: NOW - 5, channel: 1, channel_name: 'BerlinMesh' }, + // Test channel, HIGH activity (3 messages) — must still sort last. + { id: 't1', rx_time: NOW - 4, channel: 2, channel_name: 'test' }, + { id: 't2', rx_time: NOW - 3, channel: 2, channel_name: 'test' }, + { id: 't3', rx_time: NOW - 2, channel: 2, channel_name: 'test' }, + // Default/primary channel, low activity (1 message) — must lead. + { id: 'p1', rx_time: NOW - 6, channel: 0, channel_name: 'MediumFast' }, + ], + nowSeconds: NOW, + windowSeconds: WINDOW, + primaryChannelFallbackLabel: '', + }); + assert.deepEqual(model.channels.map(channel => channel.label), ['MediumFast', 'BerlinMesh', 'test']); + // Presentation-only (F4): the demoted test channel keeps all its messages. + assert.equal(findChannelByLabel(model, 'test').messageCount, 3); +}); + +test('buildChatTabModel never demotes an index-0 channel even if its name matches a keyword (F3)', () => { + const model = buildChatTabModel({ + nodes: [], + messages: [ + { id: 'cust', rx_time: NOW - 5, channel: 1, channel_name: 'BerlinMesh' }, + { id: 'prim', rx_time: NOW - 4, channel: 0, channel_name: 'test' }, // primary literally named "test" + ], + nowSeconds: NOW, + windowSeconds: WINDOW, + primaryChannelFallbackLabel: '', + }); + // The index-0 "test" channel still leads; it is NOT sunk to the test tier. + assert.deepEqual(model.channels.map(channel => channel.label), ['test', 'BerlinMesh']); + assert.equal(model.channels[0].index, 0); +}); + +test('buildChatTabModel orders channels within the test tier by activity then label (F1)', () => { + const model = buildChatTabModel({ + nodes: [], + messages: [ + { id: 'a1', rx_time: NOW - 5, channel: 1, channel_name: 'AlphaMesh' }, // custom (tier 1) + { id: 'pb1', rx_time: NOW - 4, channel: 2, channel_name: 'ping-bot' }, // test, 1 message + { id: 'tt1', rx_time: NOW - 3, channel: 3, channel_name: 'test' }, // test, 2 messages + { id: 'tt2', rx_time: NOW - 2, channel: 3, channel_name: 'test' }, + ], + nowSeconds: NOW, + windowSeconds: WINDOW, + primaryChannelFallbackLabel: '', + }); + // Custom first; then test channels, busier ('test', 2) before quieter ('ping-bot', 1). + assert.deepEqual(model.channels.map(channel => channel.label), ['AlphaMesh', 'test', 'ping-bot']); +}); + test('buildChatTabModel falls back to numeric label when no metadata provided', () => { const model = buildChatTabModel({ nodes: [], diff --git a/web/public/assets/js/app/__tests__/incremental-helpers.test.js b/web/public/assets/js/app/__tests__/incremental-helpers.test.js index b6c8d69..af3163f 100644 --- a/web/public/assets/js/app/__tests__/incremental-helpers.test.js +++ b/web/public/assets/js/app/__tests__/incremental-helpers.test.js @@ -16,7 +16,7 @@ import test from 'node:test'; import assert from 'node:assert/strict'; -import { maxRecordTimestamp, mergeById, mergeByCompositeKey, trimToLimit } from '../incremental-helpers.js'; +import { maxRecordTimestamp, mergeById, mergeByCompositeKey, trimToLimit, trimToWindow } from '../incremental-helpers.js'; // --------------------------------------------------------------------------- // maxRecordTimestamp @@ -210,3 +210,49 @@ test('trimToLimit handles records with missing timestamp fields', () => { assert.equal(result.length, 2); assert.equal(result[0].id, 3); }); + +// --------------------------------------------------------------------------- +// trimToWindow (issue #796) +// --------------------------------------------------------------------------- + +test('trimToWindow drops records older than the floor and keeps the boundary', () => { + const records = [ + { id: 1, rx_time: 90 }, + { id: 2, rx_time: 100 }, // exactly at the floor → kept + { id: 3, rx_time: 150 }, + ]; + const result = trimToWindow(records, 100); + assert.deepEqual(result.map(r => r.id), [2, 3]); +}); + +test('trimToWindow retains records with a missing or non-numeric timestamp', () => { + const records = [ + { id: 1 }, + { id: 2, rx_time: 'nope' }, + { id: 3, rx_time: 50 }, + { id: 4, rx_time: 500 }, + ]; + const result = trimToWindow(records, 100); + assert.deepEqual(result.map(r => r.id), [1, 2, 4]); +}); + +test('trimToWindow uses a custom timestamp field', () => { + const records = [ + { id: 1, last_heard: 10 }, + { id: 2, last_heard: 200 }, + ]; + const result = trimToWindow(records, 100, 'last_heard'); + assert.deepEqual(result.map(r => r.id), [2]); +}); + +test('trimToWindow returns the input unchanged for an unusable floor', () => { + const records = [{ id: 1, rx_time: 10 }]; + assert.equal(trimToWindow(records, 0), records); + assert.equal(trimToWindow(records, Number.NaN), records); + assert.equal(trimToWindow(records, -5), records); +}); + +test('trimToWindow returns input for non-array values', () => { + assert.equal(trimToWindow(null, 100), null); + assert.equal(trimToWindow(undefined, 100), undefined); +}); diff --git a/web/public/assets/js/app/__tests__/main-incremental-refresh.test.js b/web/public/assets/js/app/__tests__/main-incremental-refresh.test.js index a835871..d372abd 100644 --- a/web/public/assets/js/app/__tests__/main-incremental-refresh.test.js +++ b/web/public/assets/js/app/__tests__/main-incremental-refresh.test.js @@ -238,3 +238,59 @@ test('since parameter uses a 1-second overlap to avoid missing rows', async () = ); }); }); + +test('first load pages the chat window backward with a before cursor (issue #796)', async () => { + const now = Math.floor(Date.now() / 1000); + // Page 1 is a *full* page (1000 rows), so the pager must request another page. + const page1 = Array.from({ length: 1000 }, (_, i) => ({ + id: 5000 - i, rx_time: now - 60 - i, from_id: '!aabb', text: `m${i}`, + })); + const oldestPage1 = page1[page1.length - 1].rx_time; // inclusive cursor for page 2 + // Page 2 re-returns the boundary row (must be de-duplicated) plus older rows, + // then is short — ending the walk. + const page2 = [ + { id: 4001, rx_time: oldestPage1, from_id: '!aabb', text: 'boundary' }, + ...Array.from({ length: 200 }, (_, i) => ({ + id: 4000 - i, rx_time: oldestPage1 - 1 - i, from_id: '!aabb', text: `o${i}`, + })), + ]; + + const env = createDomEnvironment({ includeBody: true }); + const originalFetch = globalThis.fetch; + const calls = []; + globalThis.fetch = (url, options = {}) => { + calls.push({ url, options }); + let body = []; + if (url.includes('/api/messages') && !url.includes('encrypted=true')) { + body = url.includes('before=') ? page2 : page1; + } + return Promise.resolve({ ok: true, status: 200, json: () => Promise.resolve(body) }); + }; + try { + initializeApp(BASE_CONFIG); + await new Promise(r => setTimeout(r, 100)); + + const plaintextMsgCalls = calls.filter( + c => c.url.includes('/api/messages') && !c.url.includes('encrypted=true'), + ); + // A full first page must be followed by a backward page; the cursor is the + // oldest rx_time of page 1 (issue #796). + assert.ok( + plaintextMsgCalls.length >= 2, + `expected backward pagination, saw ${plaintextMsgCalls.length} message call(s)`, + ); + assert.ok(!plaintextMsgCalls[0].url.includes('before='), 'first page must not carry a cursor'); + assert.ok( + plaintextMsgCalls.some(c => c.url.includes(`before=${oldestPage1}`)), + `expected a backward page with before=${oldestPage1}`, + ); + // The initial load must not use the incremental `since` cursor. + assert.ok( + plaintextMsgCalls.every(c => !c.url.includes('since=')), + 'first load should paginate with before, never since', + ); + } finally { + globalThis.fetch = originalFetch; + env.cleanup(); + } +}); diff --git a/web/public/assets/js/app/__tests__/main-protocol.test.js b/web/public/assets/js/app/__tests__/main-protocol.test.js index 0b2fb4c..b0a2ec0 100644 --- a/web/public/assets/js/app/__tests__/main-protocol.test.js +++ b/web/public/assets/js/app/__tests__/main-protocol.test.js @@ -302,13 +302,16 @@ test('createMessageChatEntry: meshcore message with @[Name] mention resolved to }); }); -test('createMessageChatEntry: meshcore message with @[Name] mention, node not found — fallback', () => { +test('createMessageChatEntry: meshcore message with @[Name] mention, node not found — synthetic badge', () => { withApp((t) => { t.rebuildNodeIndex([]); const div = t.createMessageChatEntry(makeMeshcoreChannelMsg('EchoBot: Pong! @[Ghost]', { rx_time: 3000 })); const html = innerHtml(div); - // @[Ghost] mention with no matching node renders as escaped plain text - assert.ok(html.includes('@[Ghost]'), 'unresolved mention should render as escaped @[Name] text'); + // @[Ghost] mention with no matching node renders a synthetic protocol-stamped + // node badge carrying the name, never a bare ``@[Name]`` literal. + assert.ok(html.includes('Ghost'), 'unresolved mention should render a synthetic badge carrying the name'); + assert.ok(html.includes('Pong!'), 'body text should still render'); + assert.ok(!html.includes('@[Ghost]'), 'bare @[Name] literal must not survive'); }); }); diff --git a/web/public/assets/js/app/__tests__/meshcore-chat-helpers.test.js b/web/public/assets/js/app/__tests__/meshcore-chat-helpers.test.js index df879e6..2fc2d7e 100644 --- a/web/public/assets/js/app/__tests__/meshcore-chat-helpers.test.js +++ b/web/public/assets/js/app/__tests__/meshcore-chat-helpers.test.js @@ -20,6 +20,7 @@ import assert from 'node:assert/strict'; import { parseMeshcoreSenderPrefix, findNodeByLongName, + buildSyntheticChatNode, extractLeadingMentionAsReply, } from '../meshcore-chat-helpers.js'; @@ -217,6 +218,69 @@ test('findNodeByLongName: whitespace-only input returns null', () => { assert.equal(findNodeByLongName(' ', map), null); }); +// --------------------------------------------------------------------------- +// findNodeByLongName — protocol-aware resolution (no cross-protocol quoting) +// --------------------------------------------------------------------------- + +test('findNodeByLongName: honors protocol and never returns a different-protocol node', () => { + // A Meshtastic and a MeshCore node share the long name "Timo". The + // Meshtastic node is inserted first, so the protocol-blind scan returns it. + const meshtastic = { node_id: '!10000001', long_name: 'Timo', protocol: 'meshtastic' }; + const meshcore = { node_id: '!20000002', long_name: 'Timo', protocol: 'meshcore' }; + const map = new Map([ + ['!10000001', meshtastic], + ['!20000002', meshcore], + ]); + assert.equal(findNodeByLongName('Timo', map, 'meshcore'), meshcore); + assert.equal(findNodeByLongName('Timo', map, 'meshtastic'), meshtastic); +}); + +test('findNodeByLongName: returns null when only a different-protocol node matches', () => { + const meshtastic = { node_id: '!10000001', long_name: 'Timo', protocol: 'meshtastic' }; + const map = new Map([['!10000001', meshtastic]]); + // A MeshCore message must not borrow the Meshtastic node, even as a last resort. + assert.equal(findNodeByLongName('Timo', map, 'meshcore'), null); +}); + +test('findNodeByLongName: an unstamped node never matches a MeshCore request', () => { + // Absent protocol normalises to the Meshtastic default, so it is excluded + // from MeshCore resolution but eligible for Meshtastic resolution. + const node = { node_id: '!10000001', long_name: 'Timo' }; + const map = new Map([['!10000001', node]]); + assert.equal(findNodeByLongName('Timo', map, 'meshcore'), null); + assert.equal(findNodeByLongName('Timo', map, 'meshtastic'), node); +}); + +test('findNodeByLongName: protocol filter also applies to the emoji-prefix fallback pass', () => { + // Same emoji-stripping fallback name, one node per protocol; the MeshCore + // request must resolve the MeshCore node despite the Meshtastic one matching + // the same stripped name first. + const meshtastic = { node_id: '!10000001', long_name: '\u{1F4FA} Timo +', protocol: 'meshtastic' }; + const meshcore = { node_id: '!20000002', long_name: '\u{1F4FA} Timo +', protocol: 'meshcore' }; + const map = new Map([ + ['!10000001', meshtastic], + ['!20000002', meshcore], + ]); + assert.equal(findNodeByLongName('Timo +', map, 'meshcore'), meshcore); +}); + +// --------------------------------------------------------------------------- +// buildSyntheticChatNode — protocol-stamped stand-in for unmatched names +// --------------------------------------------------------------------------- + +test('buildSyntheticChatNode: carries the name as short/long name and stamps the protocol', () => { + assert.deepEqual(buildSyntheticChatNode('Bob', 'meshcore'), { + short_name: 'Bob', + long_name: 'Bob', + protocol: 'meshcore', + }); +}); + +test('buildSyntheticChatNode: omits the protocol key when none is supplied', () => { + assert.deepEqual(buildSyntheticChatNode('Bob', null), { short_name: 'Bob', long_name: 'Bob' }); + assert.deepEqual(buildSyntheticChatNode('Bob'), { short_name: 'Bob', long_name: 'Bob' }); +}); + // --------------------------------------------------------------------------- // extractLeadingMentionAsReply — MeshCore leading-mention detection (#727) // --------------------------------------------------------------------------- diff --git a/web/public/assets/js/app/chat-entry-renderer.js b/web/public/assets/js/app/chat-entry-renderer.js index 8df5239..587fd16 100644 --- a/web/public/assets/js/app/chat-entry-renderer.js +++ b/web/public/assets/js/app/chat-entry-renderer.js @@ -16,6 +16,7 @@ import { buildMessageBody, resolveReplyPrefix } from './message-replies.js'; import { + buildSyntheticChatNode, extractLeadingMentionAsReply, findNodeByLongName, parseMeshcoreSenderPrefix, @@ -85,8 +86,10 @@ function formatReplyPrefixHtml(label, badgeHtml, escapeHtml) { * 2. MeshCore ``"SenderName: body"`` prefix parsing for channel messages. * 3. MeshCore leading-``@[Name]`` detection, surfacing it as an ``[in reply * to BADGE]`` prefix when no structured reply is already present. - * 4. Mention rendering for MeshCore messages, mapping ``@[Name]`` to either - * a badge (when the named node is known) or an escaped literal fallback. + * 4. Mention rendering for MeshCore messages, mapping ``@[Name]`` to a badge. + * Name resolution is restricted to nodes of the message's own protocol so + * a MeshCore message never quotes a same-named Meshtastic node; when no + * same-protocol node matches, a synthetic protocol-stamped node is badged. * 5. ``buildMessageBody()`` invocation, which handles URL linkification, * emoji rendering, and reaction detection. * 6. Encrypted-message notices when available from the caller. @@ -145,7 +148,12 @@ export function renderChatEntryContent({ if (isMeshcoreChannelMsg && typeof message.text === 'string') { parsedMeshcorePrefix = parseMeshcoreSenderPrefix(message.text); if (parsedMeshcorePrefix && !message.node) { - meshcoreSenderNode = findNodeByLongName(parsedMeshcorePrefix.senderName, nodesById); + // Resolve the sender only among same-protocol nodes; when none matches, + // synthesise a protocol-stamped stand-in rather than borrowing a + // same-named node from another protocol (issue: honor protocol in chat). + meshcoreSenderNode = + findNodeByLongName(parsedMeshcorePrefix.senderName, nodesById, protocol) ?? + buildSyntheticChatNode(parsedMeshcorePrefix.senderName, protocol); } } @@ -189,20 +197,15 @@ export function renderChatEntryContent({ if (!replyPrefix && isMeshcore && effectiveBodyText) { const leading = extractLeadingMentionAsReply(effectiveBodyText); if (leading) { - const replyNode = findNodeByLongName(leading.mentionName, nodesById); - let badgeHtml = ''; - if (replyNode) { - badgeHtml = renderNodeBadge(renderShortHtml, replyNode); - } - // Graceful degradation: when the registry doesn't contain the - // mention target (common on large deployments where ``/api/nodes`` - // caps at 1000 entries by recency), still surface the leading - // mention as a reply prefix using the raw name. Without this - // fallback the body would render as bare ``@[Name] body...`` which - // looks like an unresolved mention link to the user. - if (typeof badgeHtml !== 'string' || badgeHtml.length === 0) { - badgeHtml = `${escapeHtml(leading.mentionName)}`; - } + // Resolve the quoted node among same-protocol nodes only. When none + // matches — whether because the registry lacks it (``/api/nodes`` caps by + // recency) or only a different-protocol node shares the name — synthesise + // a protocol-stamped stand-in so the reply badge is always rendered with + // the correct protocol and never quotes a node from another protocol. + const replyNode = + findNodeByLongName(leading.mentionName, nodesById, protocol) ?? + buildSyntheticChatNode(leading.mentionName, protocol); + const badgeHtml = renderNodeBadge(renderShortHtml, replyNode); meshcoreReplyPrefix = formatReplyPrefixHtml('in reply to', badgeHtml, escapeHtml); effectiveBodyText = leading.remainingText ?? ''; } @@ -213,11 +216,13 @@ export function renderChatEntryContent({ // ------------------------------------------------------------------ const renderMentionHtml = isMeshcore ? (mentionedName) => { - const mentionNode = findNodeByLongName(mentionedName, nodesById); - if (mentionNode) { - return renderNodeBadge(renderShortHtml, mentionNode); - } - return `@[${escapeHtml(mentionedName)}]`; + // Same-protocol resolution with a protocol-stamped synthetic fallback, + // so an unresolved mention renders a MeshCore badge instead of either a + // bare ``@[Name]`` literal or a same-named Meshtastic node. + const mentionNode = + findNodeByLongName(mentionedName, nodesById, protocol) ?? + buildSyntheticChatNode(mentionedName, protocol); + return renderNodeBadge(renderShortHtml, mentionNode); } : null; diff --git a/web/public/assets/js/app/chat-log-tabs.js b/web/public/assets/js/app/chat-log-tabs.js index 989c313..6dbbfed 100644 --- a/web/public/assets/js/app/chat-log-tabs.js +++ b/web/public/assets/js/app/chat-log-tabs.js @@ -22,6 +22,50 @@ import { extractModemMetadata } from './node-modem-metadata.js'; */ export const MAX_CHANNEL_INDEX = 255; +/** + * Matches a throwaway "test" channel by the presence of the standalone word + * ``ping``, ``test``, or ``bot`` (case-insensitive). The ``\b`` word boundaries + * are deliberate: they keep legitimate channels whose names merely *contain* + * those letters — "Camping", "Robotics", "Contest", "Botswana" — out of the test + * tier, trading the odd concatenated form ("MyBot", "test2") for zero false + * positives (SPEC F2). + * @type {RegExp} + */ +const TEST_CHANNEL_PATTERN = /\b(?:ping|test|bot)\b/i; + +/** + * Decide whether a channel label denotes a deprioritized "test" channel. + * + * Used by {@link buildChatTabModel} to sink ``#test`` / ``#ping`` / ``#bot`` + * style channels below the community's real channels (SPEC F1/F2). Matching is + * on the resolved display label the operator sees, case-insensitive and bounded + * to whole words so substrings never trigger a false positive. + * + * @param {string} label Resolved channel display label. + * @returns {boolean} ``true`` when the label contains a standalone test keyword. + */ +export function isTestChannelLabel(label) { + if (typeof label !== 'string') return false; + return TEST_CHANNEL_PATTERN.test(label); +} + +/** + * Classify a channel bucket into its display-ordering tier (SPEC F1/F3). + * Lower tiers sort first: + * + * 0 — default/primary channel (index 0). Always leads and is **never** demoted + * to the test tier, even if its label matches a keyword (SPEC F3). + * 2 — test channel: a non-primary channel whose label names ping/test/bot. + * 1 — any other custom (non-primary, non-test) channel. + * + * @param {{ index: number, label: string }} channel Channel bucket. + * @returns {number} Ordering tier (0, 1, or 2). + */ +function channelPriorityTier(channel) { + if (channel.index === 0) return 0; + return isTestChannelLabel(channel.label) ? 2 : 1; +} + /** * Discrete event types that can appear in the chat activity log. * @@ -311,14 +355,16 @@ export function buildChatTabModel({ channel.entries.sort((a, b) => a.ts - b.ts); channel.messageCount = channel.entries.length; } - // Sort channels into two tiers: - // 1. Primary channels (channel index 0 — LongFast, MediumFast, Public, etc.) - // ordered by activity desc so the most-active protocol leads within the tier. - // 2. Secondary channels (index > 0) ordered by activity desc, then alpha. - // Within each tier, ties on messageCount are broken alphabetically by label. + // Sort channels into three priority tiers (SPEC F1): + // 0. Default/primary channels (index 0 — LongFast, MediumFast, Public, …), + // never demoted even if the name matches a test keyword (SPEC F3). + // 1. Custom channels (index > 0) that are not test channels. + // 2. Test channels (index > 0 whose label names ping/test/bot) — sunk last. + // Within each tier the prior ordering is preserved unchanged: activity + // (7-day message count) descending, then label alphabetical. const channels = Array.from(channelBuckets.values()).sort((a, b) => { - const aTier = a.index === 0 ? 0 : 1; - const bTier = b.index === 0 ? 0 : 1; + const aTier = channelPriorityTier(a); + const bTier = channelPriorityTier(b); if (aTier !== bTier) return aTier - bTier; return b.messageCount - a.messageCount || a.label.localeCompare(b.label); }); diff --git a/web/public/assets/js/app/incremental-helpers.js b/web/public/assets/js/app/incremental-helpers.js index ccb1bb4..fa2fdaf 100644 --- a/web/public/assets/js/app/incremental-helpers.js +++ b/web/public/assets/js/app/incremental-helpers.js @@ -106,3 +106,29 @@ export function trimToLimit(records, limit, tsField = 'rx_time') { const sorted = records.slice().sort((a, b) => (b[tsField] || 0) - (a[tsField] || 0)); return sorted.slice(0, limit); } + +/** + * Drop records older than a timestamp floor, keeping the retained set aligned + * with a rolling window rather than a fixed row count. + * + * The chat feed pages the whole seven-day window (issue #796), so bounding the + * accumulated set by *count* would silently discard older-but-in-window + * messages on the next incremental merge. Bounding by the window floor instead + * keeps exactly what the renderer can display while still preventing unbounded + * growth over a long-running tab. Records whose timestamp is missing or + * non-numeric are retained so data is never lost to a malformed field. + * + * @param {Array} records Merged record array. + * @param {number} floorSeconds Minimum retained timestamp (unix seconds). + * @param {string} [tsField] Timestamp field name used for comparison. + * @returns {Array} Filtered array (same reference when nothing is + * dropped or the floor is unusable). + */ +export function trimToWindow(records, floorSeconds, tsField = 'rx_time') { + if (!Array.isArray(records)) return records; + if (!Number.isFinite(floorSeconds) || floorSeconds <= 0) return records; + return records.filter(record => { + const ts = Number(record && record[tsField]); + return !Number.isFinite(ts) || ts >= floorSeconds; + }); +} diff --git a/web/public/assets/js/app/main.js b/web/public/assets/js/app/main.js index 36b7dec..8849491 100644 --- a/web/public/assets/js/app/main.js +++ b/web/public/assets/js/app/main.js @@ -98,7 +98,7 @@ import { aggregateTelemetrySnapshots, } from './snapshot-aggregator.js'; import { normalizeNodeCollection } from './node-snapshot-normalizer.js'; -import { maxRecordTimestamp, mergeById, mergeByCompositeKey, trimToLimit } from './incremental-helpers.js'; +import { maxRecordTimestamp, mergeById, mergeByCompositeKey, trimToLimit, trimToWindow } from './incremental-helpers.js'; import { buildTraceSegments } from './trace-paths.js'; import { getRoleColor, @@ -169,6 +169,7 @@ import { filterRecentTraces, resolveSnapshotLimit, fetchMessages as fetchMessagesImpl, + fetchAllMessages as fetchAllMessagesImpl, } from './main/data-fetchers.js'; import { compareNumber, @@ -2918,10 +2919,14 @@ export function initializeApp(config) { : isMeshcoreProtocol(channel.protocol) ? MESHCORE_ICON_SRC : null, + // Channel tabs are the chat proper: render the entire window (issue #796) + // rather than only the newest CHAT_LIMIT. The entry set is already bounded + // by the seven-day window, so there is no count cap to apply here. content: buildChatFragment({ entries: channel.entries.map(e => ({ ts: e.ts, item: e.message })), renderEntry: entry => createMessageChatEntry(entry.item), - emptyLabel: 'No messages on this channel.' + emptyLabel: 'No messages on this channel.', + limit: Infinity }), index: channel.index, isPrimaryFallback: Boolean(channel.isPrimaryFallback) @@ -2955,11 +2960,14 @@ export function initializeApp(config) { * @param {{ * entries: Array<{ ts: number, item: Object }>, * renderEntry: Function, - * emptyLabel?: string - * }} params Fragment construction parameters. + * emptyLabel?: string, + * limit?: number + * }} params Fragment construction parameters. ``limit`` caps how many of the + * newest entries are rendered; pass ``Infinity`` to render them all (the Log + * firehose defaults to {@link CHAT_LIMIT}, chat channel tabs opt out). * @returns {DocumentFragment} Populated fragment. */ - function buildChatFragment({ entries = [], renderEntry, emptyLabel }) { + function buildChatFragment({ entries = [], renderEntry, emptyLabel, limit = CHAT_LIMIT }) { const fragment = document.createDocumentFragment(); if (!entries || entries.length === 0) { if (emptyLabel) { @@ -2971,7 +2979,9 @@ export function initializeApp(config) { return fragment; } const getDivider = createDateDividerFactory(); - const limitedEntries = entries.slice(Math.max(entries.length - CHAT_LIMIT, 0)); + const limitedEntries = Number.isFinite(limit) + ? entries.slice(Math.max(entries.length - limit, 0)) + : entries; let renderedEntries = 0; for (const entry of limitedEntries) { if (!entry || typeof entry.ts !== 'number') { @@ -3019,6 +3029,24 @@ export function initializeApp(config) { }); } + /** + * Closure-bound bridge to ``fetchAllMessagesImpl``. Pages the entire chat + * window (issue #796) so the initial load surfaces every in-window message + * instead of just the newest {@link MESSAGE_LIMIT}. Like {@link fetchMessages} + * it injects the dashboard's ``CHAT_ENABLED`` flag and limit normaliser so the + * underlying pager stays pure. + * + * @param {{ encrypted?: boolean }} [options] Optional retrieval flags. + * @returns {Promise>} Every message in the visibility window. + */ + function fetchAllMessages(options = {}) { + return fetchAllMessagesImpl(MESSAGE_LIMIT, { + ...options, + chatEnabled: CHAT_ENABLED, + normaliseMessageLimit, + }); + } + /** * Compute distance from the configured map center. * @@ -3972,7 +4000,12 @@ export function initializeApp(config) { positionsPromise, neighborPromise, tracesPromise, - fetchMessages(MESSAGE_LIMIT, { since: msgSince }), + // First load pages the whole window so chat is complete (issue #796); + // incremental refreshes only need the slice newer than the high-water + // mark, which always fits in a single page. + useSince + ? fetchMessages(MESSAGE_LIMIT, { since: msgSince }) + : fetchAllMessages({}), telemetryPromise, encryptedMessagesPromise ]); @@ -4011,9 +4044,15 @@ export function initializeApp(config) { const traceEntries = useSince ? trimToLimit(mergeById(allTraces, incomingTraces, 'id'), TRACE_LIMIT) : incomingTraces; + // Plaintext chat is shown for the full seven-day window (issue #796), so + // bound the retained set by that window rather than a row count — a count + // cap would silently drop older-but-in-window messages on the next merge. + const messageWindowFloor = Math.floor(Date.now() / 1000) - CHAT_RECENT_WINDOW_SECONDS; const messages = useSince - ? trimToLimit(mergeById(allMessages, incomingMessages, 'id'), MESSAGE_LIMIT) + ? trimToWindow(mergeById(allMessages, incomingMessages, 'id'), messageWindowFloor) : incomingMessages; + // Encrypted blobs only feed the mixed Log tab (itself capped), so a count + // cap is the right memory bound for them. const encryptedMessages = useSince ? trimToLimit(mergeById(allEncryptedMessages, incomingEncryptedMessages, 'id'), MESSAGE_LIMIT) : incomingEncryptedMessages; diff --git a/web/public/assets/js/app/main/__tests__/data-fetchers.test.js b/web/public/assets/js/app/main/__tests__/data-fetchers.test.js index 295d2bf..294ee31 100644 --- a/web/public/assets/js/app/main/__tests__/data-fetchers.test.js +++ b/web/public/assets/js/app/main/__tests__/data-fetchers.test.js @@ -18,6 +18,7 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { + fetchAllMessages, fetchMessages, fetchNeighbors, fetchNodeById, @@ -344,3 +345,107 @@ test('fetchMessages propagates HTTP errors', async () => { stub.restore(); } }); + +test('fetchMessages forwards a positive before cursor and omits a non-positive one', async () => { + const stub = withFetchStub({ ok: true, body: [] }); + try { + await fetchMessages(10, { before: 1234 }); + assert.ok(stub.calls[0].url.includes('before=1234')); + await fetchMessages(10, { before: 0 }); + assert.ok(!stub.calls[1].url.includes('before=')); + } finally { + stub.restore(); + } +}); + +// --------------------------------------------------------------------------- +// fetchAllMessages (issue #796 backward pagination) +// --------------------------------------------------------------------------- + +test('fetchAllMessages pages backward until a short page and de-duplicates by id', async () => { + // limit=2; the inclusive cursor re-returns the boundary row, which must be + // de-duplicated rather than counted twice. + const stub = withFetchStub((url) => { + if (url.includes('before=40')) { + return { ok: true, body: [{ id: 4, rx_time: 40 }, { id: 3, rx_time: 30 }] }; + } + if (url.includes('before=30')) { + return { ok: true, body: [{ id: 3, rx_time: 30 }] }; // short page → stop + } + return { ok: true, body: [{ id: 5, rx_time: 50 }, { id: 4, rx_time: 40 }] }; + }); + try { + const all = await fetchAllMessages(2, {}); + assert.deepEqual(all.map(m => m.id), [5, 4, 3]); + assert.equal(stub.calls.length, 3); + assert.ok(stub.calls[1].url.includes('before=40')); + assert.ok(stub.calls[2].url.includes('before=30')); + } finally { + stub.restore(); + } +}); + +test('fetchAllMessages stops when the server ignores the cursor (no progress)', async () => { + // The stub returns the same full page regardless of the cursor; without the + // no-progress guard this would loop forever. + const stub = withFetchStub({ ok: true, body: [{ id: 5, rx_time: 50 }, { id: 4, rx_time: 40 }] }); + try { + const all = await fetchAllMessages(2, {}); + assert.deepEqual(all.map(m => m.id), [5, 4]); + assert.equal(stub.calls.length, 2); // page 1 + one no-progress page, then stop + } finally { + stub.restore(); + } +}); + +test('fetchAllMessages returns [] and makes one call for an empty window', async () => { + const stub = withFetchStub({ ok: true, body: [] }); + try { + const all = await fetchAllMessages(2, {}); + assert.deepEqual(all, []); + assert.equal(stub.calls.length, 1); + } finally { + stub.restore(); + } +}); + +test('fetchAllMessages stops when no row carries a usable timestamp cursor', async () => { + // A full page whose rows lack rx_time cannot advance the cursor; the loop must + // still terminate (and keep the rows it found). + const stub = withFetchStub({ ok: true, body: [{ id: 7 }, { id: 8 }] }); + try { + const all = await fetchAllMessages(2, {}); + assert.deepEqual(all.map(m => m.id), [7, 8]); + assert.equal(stub.calls.length, 1); + } finally { + stub.restore(); + } +}); + +test('fetchAllMessages skips rows without an id and forwards retrieval flags', async () => { + const stub = withFetchStub({ ok: true, body: [{ rx_time: 40 }] }); // no id → skipped, short page + try { + const all = await fetchAllMessages(2, { encrypted: true }); + assert.deepEqual(all, []); + assert.equal(stub.calls.length, 1); + assert.ok(stub.calls[0].url.includes('encrypted=true')); + } finally { + stub.restore(); + } +}); + +test('fetchAllMessages honours the maxPages backstop against a runaway feed', async () => { + // Every page is full and strictly older, so only maxPages bounds the walk. + let n = 0; + const stub = withFetchStub(() => { + n += 1; + return { ok: true, body: [{ id: 100 - n, rx_time: 100 - n }] }; + }); + try { + const all = await fetchAllMessages(1, { maxPages: 3 }); + assert.equal(all.length, 3); + assert.equal(stub.calls.length, 3); + } finally { + stub.restore(); + } +}); diff --git a/web/public/assets/js/app/main/data-fetchers.js b/web/public/assets/js/app/main/data-fetchers.js index 38d5234..0cf8132 100644 --- a/web/public/assets/js/app/main/data-fetchers.js +++ b/web/public/assets/js/app/main/data-fetchers.js @@ -102,13 +102,15 @@ export async function fetchNodeById(nodeId) { * Fetch recent messages from the JSON API. * * @param {number} limit Maximum number of rows. - * @param {{ encrypted?: boolean, since?: number, chatEnabled?: boolean, normaliseMessageLimit?: Function }} options + * @param {{ encrypted?: boolean, since?: number, before?: number, chatEnabled?: boolean, normaliseMessageLimit?: Function }} options * Retrieval flags and dependency hooks. When ``chatEnabled`` is false the * function short-circuits to an empty array without contacting the API. + * ``before`` is an inclusive upper-bound ``rx_time`` cursor used for backward + * pagination (issue #796). * @returns {Promise>} Parsed message payloads. */ export async function fetchMessages(limit, options = {}) { - const { chatEnabled = true, normaliseMessageLimit, encrypted = false, since = 0 } = options; + const { chatEnabled = true, normaliseMessageLimit, encrypted = false, since = 0, before = 0 } = options; if (!chatEnabled) return []; const safeLimit = typeof normaliseMessageLimit === 'function' ? normaliseMessageLimit(limit) @@ -120,12 +122,65 @@ export async function fetchMessages(limit, options = {}) { if (since > 0) { params.set('since', String(since)); } + if (before > 0) { + params.set('before', String(before)); + } const query = params.toString(); const r = await fetch(`/api/messages?${query}`, { cache: 'default' }); if (!r.ok) throw new Error('HTTP ' + r.status); return r.json(); } +/** + * Fetch *every* message in the server's visibility window by paging backward. + * + * The API clamps each response to {@link MESSAGE_LIMIT} rows, so a single + * request can only ever surface the newest page. This helper walks the feed + * from newest to oldest: it pulls a page, then re-requests everything at or + * before the oldest ``rx_time`` it has seen (an inclusive cursor), de-duplicating + * by ``id``. It stops when a short page signals the window is exhausted, when a + * page yields no new rows (the server ignored the cursor, or every row was a + * boundary duplicate), or when ``maxPages`` is hit as a runaway backstop. + * + * Used for the initial chat load (issue #796) so the landing page and ``/chat`` + * subpage show the full window instead of only the newest page. + * + * @param {number} limit Page size (rows requested per API call). + * @param {{ encrypted?: boolean, chatEnabled?: boolean, normaliseMessageLimit?: Function, maxPages?: number }} [options] + * Retrieval flags, dependency hooks, and an optional page-count backstop. + * @returns {Promise>} All de-duplicated messages in the window. + */ +export async function fetchAllMessages(limit, options = {}) { + const { maxPages = 200, ...fetchOptions } = options; + const all = []; + const seen = new Set(); + let before = 0; + for (let page = 0; page < maxPages; page += 1) { + // eslint-disable-next-line no-await-in-loop -- pages are inherently sequential (cursor depends on the prior page). + const batch = await fetchMessages(limit, { ...fetchOptions, before }); + if (!Array.isArray(batch) || batch.length === 0) break; + let added = 0; + let oldest = 0; + for (const message of batch) { + const id = message && message.id; + if (id != null && !seen.has(id)) { + seen.add(id); + all.push(message); + added += 1; + } + const ts = Number(message && message.rx_time); + if (Number.isFinite(ts) && (oldest === 0 || ts < oldest)) { + oldest = ts; + } + } + // A short page means the window is exhausted; no new rows (or no usable + // cursor) means we cannot make further progress without looping forever. + if (batch.length < limit || added === 0 || oldest === 0) break; + before = oldest; + } + return all; +} + /** * Fetch neighbour information from the JSON API. * diff --git a/web/public/assets/js/app/meshcore-chat-helpers.js b/web/public/assets/js/app/meshcore-chat-helpers.js index a5e88a9..ac41fe7 100644 --- a/web/public/assets/js/app/meshcore-chat-helpers.js +++ b/web/public/assets/js/app/meshcore-chat-helpers.js @@ -14,6 +14,8 @@ * limitations under the License. */ +import { isMeshcoreProtocol } from './protocol-helpers.js'; + /** * Parse the ``"SenderName: body"`` prefix that MeshCore embeds in channel * message text. MeshCore channel messages do not carry a sender node ID, so @@ -37,6 +39,24 @@ export function parseMeshcoreSenderPrefix(text) { return { senderName, bodyText }; } +/** + * Whether a candidate node belongs to the same protocol as the message that + * referenced it. Node and message protocols are reduced to the two canonical + * values (anything not explicitly MeshCore — including absent/unknown — is the + * Meshtastic default), so a MeshCore message never matches a Meshtastic (or + * unstamped) node and vice-versa. A ``null`` requested protocol disables the + * filter, preserving the original protocol-agnostic behaviour for callers that + * have no protocol context. + * + * @param {Object} node Candidate node record. + * @param {string|null|undefined} protocol Protocol of the referencing message. + * @returns {boolean} Whether the node may be matched for this protocol. + */ +function nodeMatchesProtocol(node, protocol) { + if (protocol == null) return true; + return isMeshcoreProtocol(protocol) === isMeshcoreProtocol(node && node.protocol); +} + /** * Look up a node in the provided ``nodesById`` Map by its long name. * @@ -47,6 +67,13 @@ export function parseMeshcoreSenderPrefix(text) { * Both the snake_case (``long_name``) and camelCase (``longName``) property * variants are checked to accommodate different serialisation paths. * + * When ``protocol`` is supplied, only nodes of that protocol are eligible. + * Long names collide across protocols (a MeshCore and a Meshtastic node can + * both be called "Timo"), so without this filter the scan would return whichever + * node happens to come first in insertion order — letting a MeshCore message + * quote a Meshtastic node. Filtering by the referencing message's protocol is + * what keeps chat resolution protocol-correct. + * * This is an O(n) scan over all nodes. For the typical node counts seen in * practice (hundreds) this is negligible; a long-name index is not maintained * in the client-side Map because insertions and lookups occur at different @@ -54,9 +81,12 @@ export function parseMeshcoreSenderPrefix(text) { * * @param {string} longName Long name to search for. * @param {Map} nodesById Loaded node registry keyed by node ID. - * @returns {object|null} The first matching node, or ``null`` when not found. + * @param {string|null} [protocol] Protocol the matched node must belong to; + * ``null``/omitted matches any protocol (legacy behaviour). + * @returns {object|null} The first matching node of the requested protocol, or + * ``null`` when not found. */ -export function findNodeByLongName(longName, nodesById) { +export function findNodeByLongName(longName, nodesById, protocol = null) { if (!longName || typeof longName !== 'string') return null; if (!(nodesById instanceof Map)) return null; const trimmed = longName.trim(); @@ -69,6 +99,7 @@ export function findNodeByLongName(longName, nodesById) { // First pass: exact match on trimmed candidate long names. for (const node of nodesById.values()) { + if (!nodeMatchesProtocol(node, protocol)) continue; const raw = node.long_name ?? node.longName; if (typeof raw !== 'string') continue; if (raw.trim() === trimmed) return node; @@ -80,6 +111,7 @@ export function findNodeByLongName(longName, nodesById) { // prefix the node carries in the registry — e.g. @[Timo +] matching // a node whose long_name is "📺 Timo +". for (const node of nodesById.values()) { + if (!nodeMatchesProtocol(node, protocol)) continue; const raw = node.long_name ?? node.longName; if (typeof raw !== 'string') continue; const stripped = raw.replace(/^[^\p{L}\p{N}]+/u, '').trim(); @@ -89,6 +121,32 @@ export function findNodeByLongName(longName, nodesById) { return null; } +/** + * Build a synthetic stand-in node for a chat name reference that no + * same-protocol registry node matched. + * + * Carrying the referencing message's protocol keeps the rendered badge's colour + * palette and protocol icon correct and — crucially — guarantees a MeshCore + * message renders a MeshCore-stamped badge instead of borrowing a colliding + * node from another protocol. This mirrors the protocol-stamped placeholder + * the message hydrator builds for unknown senders + * (``message-node-hydrator``), but is keyed on the visible name rather than a + * node id because mentions/quotes reference nodes by name. + * + * The visible name is used as both the short and long name so the badge stays + * legible (a bare ``long_name`` would render as a ``?`` placeholder). + * + * @param {string} name Visible name parsed from the message (mention/sender). + * @param {string|null|undefined} protocol Protocol of the referencing message. + * @returns {{short_name: string, long_name: string, protocol?: string}} + * Synthetic node ready for badge rendering. + */ +export function buildSyntheticChatNode(name, protocol) { + const node = { short_name: name, long_name: name }; + if (protocol != null) node.protocol = protocol; + return node; +} + /** * Extract a leading ``@[Name]`` mention from text if it looks like a reply. * diff --git a/web/spec/app_spec.rb b/web/spec/app_spec.rb index 11896f9..50de755 100644 --- a/web/spec/app_spec.rb +++ b/web/spec/app_spec.rb @@ -6533,6 +6533,81 @@ RSpec.describe "Potato Mesh Sinatra app" do expect(scoped_since.map { |row| row["id"] }).to eq([2]) end + # Regression for issue #796: more than MAX_QUERY_LIMIT messages inside the + # seven-day window must all remain reachable. Before the fix the feed had + # no upper-bound cursor, so paging stalled at the newest 1000 rows and every + # older in-window message was invisible. + it "exposes every in-window message through backward pagination (issue #796)" do + clear_database + allow(Time).to receive(:now).and_return(reference_time) + now = reference_time.to_i + + cap = PotatoMesh::App::Queries::MAX_QUERY_LIMIT + total = cap + 500 + # Seed more than one page of messages, all comfortably inside the + # seven-day window, each with a distinct rx_time so the keyset cursor is + # unambiguous. + with_db do |db| + db.transaction do + total.times do |i| + rx = now - 60 - i * 30 + db.execute( + "INSERT INTO messages(id, rx_time, rx_iso, from_id, to_id, channel, portnum, text) VALUES(?,?,?,?,?,?,?,?)", + [1000 + i, rx, Time.at(rx).utc.iso8601, "!a", "!b", 0, "TEXT_MESSAGE_APP", "msg #{i}"], + ) + end + end + end + + # Walk the feed the way the dashboard client does: pull a page, then ask + # for everything at-or-before the oldest row already seen. Without an + # upper-bound cursor the server cannot return anything past the newest + # `cap` rows, so the loop stalls and never reaches the older messages. + seen = {} + cursor = nil + pages = 0 + loop do + url = "/api/messages?limit=#{cap}" + url += "&before=#{cursor}" if cursor + get url + expect(last_response).to be_ok + rows = JSON.parse(last_response.body) + # The per-request cap is unchanged: a single response never exceeds it. + expect(rows.size).to be <= cap + added = rows.reject { |row| seen.key?(row["id"]) } + added.each { |row| seen[row["id"]] = true } + pages += 1 + break if rows.size < cap # window exhausted + break if added.empty? # no progress (unfixed server ignores `before`) + break if pages >= 10 # hard safety bound against an infinite loop + cursor = rows.map { |row| row["rx_time"] }.min + end + + expect(seen.size).to eq(total) + end + + it "treats a non-positive before cursor as absent (issue #796)" do + clear_database + allow(Time).to receive(:now).and_return(reference_time) + now = reference_time.to_i + with_db do |db| + db.execute( + "INSERT INTO messages(id, rx_time, rx_iso, from_id, to_id, channel, text) VALUES(?,?,?,?,?,?,?)", + [42, now - 30, Time.at(now - 30).utc.iso8601, "!a", "!b", 0, "hi"], + ) + end + + # before=0 and before=-5 must be ignored (not used as a ceiling), so the + # row still comes back rather than being filtered out by a bogus cursor. + get "/api/messages?before=0" + expect(last_response).to be_ok + expect(JSON.parse(last_response.body).map { |r| r["id"] }).to eq([42]) + + get "/api/messages?before=-5" + expect(last_response).to be_ok + expect(JSON.parse(last_response.body).map { |r| r["id"] }).to eq([42]) + end + it "clamps an explicit since older than the seven-day floor up to the floor" do clear_database allow(Time).to receive(:now).and_return(reference_time) diff --git a/web/spec/queries_spec.rb b/web/spec/queries_spec.rb index 323da9e..4431bf9 100644 --- a/web/spec/queries_spec.rb +++ b/web/spec/queries_spec.rb @@ -677,6 +677,30 @@ RSpec.describe PotatoMesh::App::Queries do expect(scoped_ids).to include(101) expect(scoped_ids).to include(102) end + + it "applies an inclusive before cursor and ignores a non-positive one (issue #796)" do + with_db do |db| + [10, 20, 30].each do |offset| + rx = now - offset + db.execute( + "INSERT INTO messages(id, rx_time, rx_iso, from_id, to_id, channel, text) VALUES (?,?,?,?,?,?,?)", + [200 + offset, rx, Time.at(rx).utc.iso8601, "!aabbccdd", "!ffffffff", 0, "m#{offset}"], + ) + end + end + + # Inclusive ceiling: the row exactly at the cursor stays; newer rows drop. + # This is what lets the client page backward by feeding the oldest rx_time + # of each page as the next cursor without skipping boundary-second rows. + paged = queries.query_messages(10, before: now - 20).map { |r| r["id"] } + expect(paged).to include(220, 230) + expect(paged).not_to include(210) # newer than the cursor + expect(paged).not_to include(1) # base row at `now` is newer than the cursor + + # A non-positive cursor is treated as "no cursor" — the default window. + unbounded = queries.query_messages(10, before: 0).map { |r| r["id"] } + expect(unbounded).to include(1, 210, 220, 230) + end end describe "#query_telemetry" do