mirror of
https://github.com/l5yth/potato-mesh.git
synced 2026-08-07 17:33:26 +02:00
web: fix MeshCore ghost nodes (stale contact enrichment discarded) (#841)
* web: fix MeshCore ghost nodes (stale contact enrichment discarded) * web: guard short_name gap-fill against empty strings; codify Known gaps
This commit is contained in:
l5y
@@ -435,6 +435,21 @@ existing codebase, not to the change under review.
|
||||
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.
|
||||
- **A1b — two benign textual matches in the broker grep.** The repo-wide A1b
|
||||
command matches `.claude/hooks/guard-edits.py` (the anti-broker edit guard's
|
||||
own pattern list) and the "no broker" documentation comment in
|
||||
`web/lib/potato_mesh/application/pubsub.rb` (PS1). Both are descriptive or
|
||||
defensive text *about* the apex ban — neither is a broker dependency or
|
||||
connection — but they sit outside A1b's `via_mqtt` exemption wording. Treat
|
||||
these two files as documented exemptions until the A1b filter codifies them.
|
||||
- **B1 — sandbox DNS breaks the `POST /api/instances` spec block.** In
|
||||
sandboxed environments whose resolver maps the suite's test domains
|
||||
(`mesh.example`, …) into the SSRF guard's restricted address ranges, 17
|
||||
`spec/app_spec.rb` "POST /api/instances" examples fail with
|
||||
`{"error":"restricted domain"}` (400 instead of 201). Environmental only:
|
||||
the failures reproduce identically with and without any change under review
|
||||
and do not occur where the test domains resolve normally (CI). Attribute to
|
||||
the environment, not the codebase or the change.
|
||||
|
||||
---
|
||||
|
||||
@@ -2521,3 +2536,52 @@ federation wire, and Flutter app are behaviorally untouched by this batch —
|
||||
the only edits outside the four fixes are the lockstep 0.7.2 version-bump
|
||||
stamps (manifests, lockfiles, iOS plist, README pinned tags, S-A1), verified
|
||||
by `tests/test_version_sync.py`.
|
||||
|
||||
---
|
||||
|
||||
## Bugfix: MeshCore ghost nodes (stale contact enrichment discarded)
|
||||
|
||||
A MeshCore node first seen via a bare `ADVERTISEMENT` push was upserted as a
|
||||
minimal placeholder stamped `lastHeard = now` (receiver wall clock). The
|
||||
follow-up roster contact record — carrying the real name/role/public key — is
|
||||
stamped `lastHeard = last_advert`, the **sender-side** advert-creation time,
|
||||
which is always older than the placeholder's receive time (seconds for healthy
|
||||
clocks, years for broken ones). `upsert_node`'s row-level freshness guard
|
||||
(`WHERE excluded.last_heard >= nodes.last_heard`) therefore discarded the
|
||||
entire named update, permanently: every later contact re-post (auto-update,
|
||||
periodic snapshot, restart) is also sender-stamped and also lost, while each
|
||||
advertised-position ingest re-bumps the row's `last_heard`. Result: nameless
|
||||
"ghost" nodes with a hex `short_name`, NULL role (displayed as the CLIENT
|
||||
default), and an advert-stamped `position_time` — violating the reconciliation
|
||||
promise in `CONTRACTS.md` ("a later full contact advertisement reconciles it",
|
||||
SPEC A4e). Fixed web-side (Ruby): after the guarded upsert, a non-synthetic
|
||||
record additionally **fills identity columns that are still NULL** (`num`,
|
||||
`short_name`, `long_name`, `macaddr`, `hw_model`, `role`, `public_key`,
|
||||
`is_unmessagable`) regardless of staleness — stale data can fill gaps but can
|
||||
never overwrite fresher values, and synthetic placeholders remain barred from
|
||||
real rows. No ingestor/API/DB-schema change; protocol-neutral (Invariant IV).
|
||||
|
||||
### GH-A1 — Stale contact records name advert-placeholder ghosts
|
||||
```bash
|
||||
( cd web && bundle exec rspec spec/data_processing_spec.rb -e "stale contact record enrichment" )
|
||||
```
|
||||
**Expected:** pass. Replaying the ingestor's wire sequence — bare-advert
|
||||
placeholder (`lastHeard = now`, no name) followed by the roster contact record
|
||||
(`lastHeard = last_advert`, older by 17 s and by ~2 years in a second example) —
|
||||
leaves the node **named** with its real role and public key. The stale record
|
||||
never regresses `last_heard`, never overwrites an existing name/role, empty
|
||||
strings never fill `long_name` / `short_name` (the other identity fields of the
|
||||
same record still fill), and a stale `synthetic=1` chat placeholder still
|
||||
cannot touch a real row.
|
||||
|
||||
### GH-R1 — Regression: prior acceptance still holds
|
||||
```bash
|
||||
( cd web && bundle exec rspec ) && ( cd web && npm test ) && ( cd web && bundle exec rufo --check . )
|
||||
( . .venv/bin/activate && pytest -q tests/ )
|
||||
```
|
||||
**Expected:** all green. At risk and explicitly required to stay green: the
|
||||
pre-existing `upsert_node` guard specs (`data_processing_spec.rb` — role/
|
||||
identity preservation, generic-name fallback, synthetic flag + merge #755/#803)
|
||||
and `database_spec.rb`'s node-merge suites, since the fix appends a second
|
||||
NULL-fill statement inside the same `upsert_node` transaction; the Python
|
||||
ingestor is untouched (A4e's advert-capture suite unchanged).
|
||||
|
||||
@@ -66,7 +66,7 @@ The web application applies the same normalisation as a safety net so legacy ing
|
||||
|
||||
- *Contact roster (rich).* The startup `ensure_contacts()` fetch plus live `NEW_CONTACT` / `NEXT_CONTACT` pushes carry the full advert (name, role, position) and upsert complete node rows. This covers every node the radio has added to its contact book.
|
||||
- *Auto-update re-fetch (freshness).* The provider sets `mc.auto_update_contacts = True`, so the meshcore library re-fetches **changed** contacts (incrementally, by `lastmod`) whenever an `ADVERTISEMENT` / `PATH_UPDATE` push arrives. A re-advert from a known node therefore refreshes its `last_advert` / position without waiting for a reconnect.
|
||||
- *Bare advert (reach).* The `ADVERTISEMENT` (pubkey-only) push is also handled directly: for a public key **not** in the contact roster it upserts a minimal "heard now" node (`lastHeard`, `protocol`, `user.shortName`/`publicKey` only — no name/type/position), so radios running with auto-add off still register the advertiser. Known keys are skipped (the auto-update path keeps them fresh). The Ruby web app preserves an existing long name on conflict, so this placeholder never clobbers a richer record, and a later full contact advertisement reconciles it.
|
||||
- *Bare advert (reach).* The `ADVERTISEMENT` (pubkey-only) push is also handled directly: for a public key **not** in the contact roster it upserts a minimal "heard now" node (`lastHeard`, `protocol`, `user.shortName`/`publicKey` only — no name/type/position), so radios running with auto-add off still register the advertiser. Known keys are skipped (the auto-update path keeps them fresh). The Ruby web app preserves an existing long name on conflict, so this placeholder never clobbers a richer record, and a later full contact advertisement reconciles it. Reconciliation does not depend on timestamp ordering: the contact record carries `lastHeard = last_advert` (the **sender-stamped** advert-creation time), which is always older than the placeholder's wall-clock stamp — the web app's node upsert therefore fills identity fields (name, role, public key, …) that are still NULL even from an older-stamped record, while timestamps/telemetry stay freshness-guarded (ACCEPTANCE GH-A1).
|
||||
|
||||
New protocols SHOULD likewise treat "node was heard" as a first-class, name-optional upsert so peer discovery does not hinge on a roster being populated.
|
||||
|
||||
|
||||
@@ -148,13 +148,6 @@ module PotatoMesh
|
||||
updated
|
||||
end
|
||||
|
||||
# Insert or update a node row from an inbound NodeInfo-style payload.
|
||||
#
|
||||
# @param db [SQLite3::Database] open database handle.
|
||||
# @param node_id [String] canonical node identifier.
|
||||
# @param n [Hash] node payload extracted from the ingestor.
|
||||
# @param protocol [String] protocol identifier (default +meshtastic+).
|
||||
# @return [void]
|
||||
# Read +hash[primary]+, falling back to the first present alias key. Lets the
|
||||
# node ingest contract accept snake_case fields in addition to the Meshtastic
|
||||
# camelCase the collector emits today; nil-aware so a boolean +false+ from the
|
||||
@@ -171,6 +164,26 @@ module PotatoMesh
|
||||
nil
|
||||
end
|
||||
|
||||
# Insert or update a node row from an inbound NodeInfo-style payload.
|
||||
#
|
||||
# Two-phase write. Phase one is the freshness-guarded upsert: a record
|
||||
# whose +lastHeard+ is older than the stored row cannot change
|
||||
# timestamps, telemetry, or position. Phase two fills identity columns
|
||||
# (+num+, +short_name+, +long_name+, +macaddr+, +hw_model+, +role+,
|
||||
# +public_key+, +is_unmessagable+) that are still NULL, regardless of the
|
||||
# record's staleness — a stale-but-richer record (e.g. a MeshCore roster
|
||||
# contact stamped with the sender-side +last_advert+, which is always
|
||||
# older than the wall-clock +lastHeard+ of the bare-advert placeholder
|
||||
# that created the row) still names the node instead of being discarded
|
||||
# wholesale, which produced permanently unnamed "ghost" nodes
|
||||
# (ACCEPTANCE GH-A1). Synthetic chat placeholders never touch real rows
|
||||
# in either phase.
|
||||
#
|
||||
# @param db [SQLite3::Database] open database handle.
|
||||
# @param node_id [String] canonical node identifier.
|
||||
# @param n [Hash] node payload extracted from the ingestor.
|
||||
# @param protocol [String] protocol identifier (default +meshtastic+).
|
||||
# @return [void]
|
||||
def upsert_node(db, node_id, n, protocol: "meshtastic")
|
||||
user = n["user"] || {}
|
||||
met = pick_alias(n, "deviceMetrics", "device_metrics") || {}
|
||||
@@ -219,6 +232,11 @@ module PotatoMesh
|
||||
# sender names before the real contact advertisement is received.
|
||||
synthetic = user["synthetic"] ? 1 : 0
|
||||
long_name = pick_alias(user, "longName", "long_name")
|
||||
short_name = pick_alias(user, "shortName", "short_name")
|
||||
macaddr = user["macaddr"]
|
||||
hw_model = pick_alias(user, "hwModel", "hw_model") || pick_alias(n, "hwModel", "hw_model")
|
||||
public_key = pick_alias(user, "publicKey", "public_key")
|
||||
is_unmessagable = coerce_bool(pick_alias(user, "isUnmessagable", "is_unmessagable"))
|
||||
|
||||
# If the incoming long name is a generic placeholder, prefer any real
|
||||
# name already on record so we never stomp known data with fallback
|
||||
@@ -240,13 +258,13 @@ module PotatoMesh
|
||||
row = [
|
||||
node_id,
|
||||
node_num,
|
||||
pick_alias(user, "shortName", "short_name"),
|
||||
short_name,
|
||||
long_name,
|
||||
user["macaddr"],
|
||||
pick_alias(user, "hwModel", "hw_model") || pick_alias(n, "hwModel", "hw_model"),
|
||||
macaddr,
|
||||
hw_model,
|
||||
role,
|
||||
pick_alias(user, "publicKey", "public_key"),
|
||||
coerce_bool(pick_alias(user, "isUnmessagable", "is_unmessagable")),
|
||||
public_key,
|
||||
is_unmessagable,
|
||||
coerce_bool(pick_alias(n, "isFavorite", "is_favorite")),
|
||||
pick_alias(n, "hopsAway", "hops_away"),
|
||||
n["snr"],
|
||||
@@ -305,6 +323,35 @@ module PotatoMesh
|
||||
AND NOT (COALESCE(nodes.synthetic,0) = 0 AND excluded.synthetic = 1)
|
||||
SQL
|
||||
|
||||
# Ghost-node repair (GH-A1): the guard above skips records whose
|
||||
# last_heard is older than the stored row — correct for timestamps,
|
||||
# telemetry, and position, but it also starved identity data. A
|
||||
# MeshCore roster contact is stamped with the sender-side
|
||||
# last_advert, which is always older than the wall-clock lastHeard
|
||||
# of the bare-advert placeholder that created the row, so the
|
||||
# name/role/public key never landed and the node stayed a
|
||||
# permanently unnamed ghost. Fill identity columns that are still
|
||||
# NULL from any non-synthetic record regardless of staleness: gaps
|
||||
# get filled, fresher values are never regressed, and synthetic
|
||||
# chat placeholders remain barred from real rows. NULLIF keeps
|
||||
# empty strings — a MeshCore contact may carry an empty adv_name,
|
||||
# and shortName is guarded the same way — from filling
|
||||
# long_name/short_name with blank text.
|
||||
if synthetic.zero?
|
||||
db.execute(<<~SQL, [node_num, short_name, long_name, macaddr, hw_model, role, public_key, is_unmessagable, node_id])
|
||||
UPDATE nodes SET
|
||||
num=COALESCE(num, ?),
|
||||
short_name=COALESCE(short_name, NULLIF(?, '')),
|
||||
long_name=COALESCE(long_name, NULLIF(?, '')),
|
||||
macaddr=COALESCE(macaddr, ?),
|
||||
hw_model=COALESCE(hw_model, ?),
|
||||
role=COALESCE(role, ?),
|
||||
public_key=COALESCE(public_key, ?),
|
||||
is_unmessagable=COALESCE(is_unmessagable, ?)
|
||||
WHERE node_id = ?
|
||||
SQL
|
||||
end
|
||||
|
||||
# Reconcile synthetic placeholder rows with their real counterparts
|
||||
# whenever a MeshCore node is upserted. Both directions must fire —
|
||||
# the arrival order of chat messages vs contact advertisements is
|
||||
|
||||
@@ -744,6 +744,129 @@ RSpec.describe PotatoMesh::App::DataProcessing do
|
||||
end
|
||||
end
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# upsert_node — MeshCore ghost nodes: a bare-advert placeholder row (stamped
|
||||
# with the receiver's wall clock) must not starve the follow-up roster
|
||||
# contact record, whose lastHeard is the sender-stamped last_advert and is
|
||||
# therefore always older (seconds for healthy clocks, years for broken
|
||||
# ones). The freshness guard may skip the stale record's timestamps, but
|
||||
# its identity fields (name, role, public key, …) must still fill columns
|
||||
# that are NULL, or the node stays a permanently nameless "ghost".
|
||||
# ---------------------------------------------------------------------------
|
||||
describe "#upsert_node — stale contact record enrichment (ghost nodes)" do
|
||||
include_context "with isolated db"
|
||||
|
||||
let(:pub_key) { "60e53d9b#{"ab" * 28}" }
|
||||
|
||||
# Mirror the ingestor's bare-advert placeholder (_advert_to_node_dict +
|
||||
# radio metadata): wall-clock lastHeard, shortName/publicKey, no name/role.
|
||||
def upsert_advert_placeholder(db, heard_at)
|
||||
dp.upsert_node(db, "!60e53d9b", {
|
||||
"lastHeard" => heard_at,
|
||||
"lora_freq" => 869,
|
||||
"modem_preset" => "SF8/BW62/CR8",
|
||||
"user" => { "shortName" => "60e5", "publicKey" => pub_key },
|
||||
}, protocol: "meshcore")
|
||||
end
|
||||
|
||||
# Mirror the follow-up roster contact (_contact_to_node_dict): the full
|
||||
# record named from the advert, stamped with the sender-side last_advert.
|
||||
def upsert_contact_record(db, last_advert, long_name: "Alpha Repeater")
|
||||
dp.upsert_node(db, "!60e53d9b", {
|
||||
"lastHeard" => last_advert,
|
||||
"lora_freq" => 869,
|
||||
"modem_preset" => "SF8/BW62/CR8",
|
||||
"user" => {
|
||||
"longName" => long_name,
|
||||
"shortName" => "60e5",
|
||||
"publicKey" => pub_key,
|
||||
"role" => "REPEATER",
|
||||
},
|
||||
}, protocol: "meshcore")
|
||||
end
|
||||
|
||||
def node_row(db)
|
||||
db.execute("SELECT * FROM nodes WHERE node_id = '!60e53d9b'").first
|
||||
end
|
||||
|
||||
it "fills name and role from a contact record stamped seconds older (healthy clock)" do
|
||||
db = open_db
|
||||
upsert_advert_placeholder(db, now)
|
||||
upsert_contact_record(db, now - 17)
|
||||
row = node_row(db)
|
||||
db.close
|
||||
expect(row["long_name"]).to eq("Alpha Repeater")
|
||||
expect(row["role"]).to eq("REPEATER")
|
||||
expect(row["public_key"]).to eq(pub_key)
|
||||
end
|
||||
|
||||
it "fills name and role from a contact record stamped years older (broken node clock)" do
|
||||
db = open_db
|
||||
upsert_advert_placeholder(db, now)
|
||||
upsert_contact_record(db, now - 66_988_974)
|
||||
row = node_row(db)
|
||||
db.close
|
||||
expect(row["long_name"]).to eq("Alpha Repeater")
|
||||
expect(row["role"]).to eq("REPEATER")
|
||||
end
|
||||
|
||||
it "keeps the newer last_heard when a stale contact record enriches the row" do
|
||||
db = open_db
|
||||
upsert_advert_placeholder(db, now)
|
||||
upsert_contact_record(db, now - 17)
|
||||
row = node_row(db)
|
||||
db.close
|
||||
expect(row["last_heard"]).to eq(now)
|
||||
end
|
||||
|
||||
it "does not overwrite an existing name or role with stale data" do
|
||||
db = open_db
|
||||
upsert_contact_record(db, now, long_name: "Fresh Name")
|
||||
dp.upsert_node(db, "!60e53d9b", {
|
||||
"lastHeard" => now - 3600,
|
||||
"user" => { "longName" => "Old Name", "shortName" => "OLD", "role" => "COMPANION" },
|
||||
}, protocol: "meshcore")
|
||||
row = node_row(db)
|
||||
db.close
|
||||
expect(row["long_name"]).to eq("Fresh Name")
|
||||
expect(row["role"]).to eq("REPEATER")
|
||||
end
|
||||
|
||||
it "does not fill a missing name with a stale empty adv_name" do
|
||||
db = open_db
|
||||
upsert_advert_placeholder(db, now)
|
||||
upsert_contact_record(db, now - 17, long_name: "")
|
||||
row = node_row(db)
|
||||
db.close
|
||||
expect(row["long_name"]).to be_nil
|
||||
end
|
||||
|
||||
it "does not fill a missing short_name with a stale empty shortName" do
|
||||
db = open_db
|
||||
dp.upsert_node(db, "!60e53d9b", { "lastHeard" => now }, protocol: "meshcore")
|
||||
dp.upsert_node(db, "!60e53d9b", {
|
||||
"lastHeard" => now - 17,
|
||||
"user" => { "shortName" => "", "longName" => "Alpha Repeater" },
|
||||
}, protocol: "meshcore")
|
||||
row = node_row(db)
|
||||
db.close
|
||||
expect(row["short_name"]).to be_nil
|
||||
expect(row["long_name"]).to eq("Alpha Repeater")
|
||||
end
|
||||
|
||||
it "still ignores stale synthetic placeholders for real rows" do
|
||||
db = open_db
|
||||
upsert_advert_placeholder(db, now)
|
||||
dp.upsert_node(db, "!60e53d9b", {
|
||||
"lastHeard" => now - 17,
|
||||
"user" => { "longName" => "Chat Alias", "synthetic" => true },
|
||||
}, protocol: "meshcore")
|
||||
row = node_row(db)
|
||||
db.close
|
||||
expect(row["long_name"]).to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# upsert_node — synthetic flag + merge
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user