Codex's final review found that moving code out of modules dropped imports that local plugins could import from them, for example `from modules.service_plugins.discord_bridge_service import EventType`, which would make such a plugin fail to load. Comparing every top-level name bound on dev against the branch (both trees imported, so names under TYPE_CHECKING are excluded) found 24 in 11 modules: EventType, copy and contextlib in both bridges; sha256, Iterable and hmac_mod in message_handler; RotatingFileHandler, colorlog, contextlib, contextvars and functools in core; hashlib and re in utils; contextlib in feed_manager, earthquake_service and weather_service; Any in cmd and help; time in path. Each is re-imported exactly as dev imported it.
The public-import guard only covered `from modules... import` statements, so it missed these. It now records every unconditional top-level import and assignment, regenerated from dev's tree (461 → 1,597 names), and fails if the discord bridge stops exposing EventType.
handle_contact_message's copy of the payload/metadata/signal-cache lookup becomes a _message_signal call with its own SNR keys (it also accepts signal_to_noise and signal_noise_ratio), so DMs and channel messages share one implementation. The helper loses a comment that only made sense inline.
A DM replay (1,500 corpus packets as RF rows, 1,500 DMs with varied SNR/RSSI key layouts, metadata, cached signal values keyed by pubkey prefix, with and without radio contacts) gives byte-identical processed messages and logger calls; passing the channel keys instead changes them. The channel replay is unchanged.
handle_channel_message stays on MessageHandler (the plan keeps the handle_* entry points there) and its blocks move to modules/channel_message.py, unchanged apart from returning what they used to assign: _split_channel_sender, _message_signal (SNR/RSSI from payload, metadata or the signal cache; the DM handler can share it), _channel_route_from_rf (signal values, decoded packet, hops and path from the correlated RF row, still refusing a fallback row's route, #80), _decode_scope_packet, _channel_reply_scope, _channel_scope_allowed (the flood_scopes allowlist, returning False where it returned) and _full_sender_pubkey. Two copies of a stale comment are gone.
CHANNEL_SENDER_FALLBACK and _signal_value move with them, since channel_message can't import from message_handler; message_handler imports both back, so existing imports keep working.
Checked with a new replay: the 4,009-packet corpus plus TRACE and truncated packets as RF log rows, interleaved with channel messages (120 decrypted with the public channel key, the rest synthetic, some arriving with an empty RF cache) in four configurations (no allowlist, flood_scopes keys, '*' only, keys plus '*' with old cached messages). Processed messages, flood-scope observations, every logger, mesh graph and web viewer call and the caches are byte-identical, as is the RF log replay. Mutations in six of the helpers each change the output; replacing the raw-hex path guess with None and dropping int() from cached RSSI don't, because the replay's undecodable packets never yield a guessed path and its cached RSSI values are already ints.
handle_rf_log_data and _transmission_evidence (only it calls that) move verbatim (AST-identical) to modules/rf_log.py, which MessageHandler mixes in; core's RX_LOG_DATA subscription and the tests call them on the handler as before. message_handler keeps importing calculate_packet_hash as a re-export.
A replay of the 4,009-packet corpus plus TRACE packets from the dev bot and truncated packets, in three setups (no tracker or viewer; a transmission tracker; tracker, web viewer and a capturing mesh graph with perturbed decodes so all three path-consistency warnings fire), gives byte-identical caches, logger, tracker, viewer, mesh graph and bot calls before and after.
The by-name and by-pubkey-prefix loops over meshcore.contacts become two _find_contact calls with the same predicates, in the same order, and the pubkey lookup still runs only when the name lookup found nothing and a prefix is set. An old-vs-new fuzz (20,000 random contact sets, senders and RF prefixes, comparing the result and every logger call) found no differences; two mutations of the predicates each produced over 1,000.
handle_rf_log_data no longer updates the caches itself. _remember_signal replaces its two copies of the LRU-bounded SNR/RSSI cache update, and _insert_rf_row takes the block that files a built row: the authenticated channel cache, the TC_FLOOD debug line, recent_rf_data, the timestamp and prefix indexes, and eviction. Log text and levels are unchanged.
Checked by replaying the 4,009-packet decode corpus through handle_rf_log_data on the old and new code (frozen clock, small cache limits so eviction runs, with and without a transmission tracker, varied SNR/RSSI/metadata/scope fields): the caches, indexes, every logger and tracker call and 400 lookups afterwards are byte-identical, and three mutations of the moved code each change the output.
find_recent_rf_data, _correlate_channel_message_rf_data, the channel payload check (_find_rf_row_matching_chan_payload, _rf_data_matches_chan_payload), the authenticated channel cache (_channel_message_identity, _channel_secrets, _decode_authenticated_channel_identity, _cache_authenticated_channel_rf_data, _find_authenticated_channel_rf_data) and _cleanup_stale_cache_entries move verbatim (AST-identical) to modules/rf_correlation.py. MessageHandler mixes it in, so names, callers (rf_match, scope_gate, multitest) and tests are unchanged, and the cache state stays on the handler, where about 130 test references set or read it, many on object.__new__ instances.
message_handler keeps importing channel_hash_for_key and decrypt_group_text as re-exports.
#342 removed the pending-message correlation list. message_handler keeps this branch's mixin layout without PendingMessageEntry; the default config comment for message_correlation_timeout moves into modules/templates/default_config.ini (core.py keeps reading the template, which matches dev's inline text otherwise); both sides' new tests at the end of test_message_handler.py stay.
The public-API golden drops exactly the five names #342 removed on purpose.
When a channel message had no RF log row yet, the handler stored it in pending_messages, slept 100 ms, then looked up find_recent_rf_data(payload["pubkey_prefix"]) through correlate_message_with_rf_data. Nothing else read the list: try_correlate_pending_messages only set a "processed" flag nobody checked, and cleanup_old_messages pruned it, both on every received packet.
The wait and the second lookup stay, now written out directly. Results only differ where the list misbehaved (found by codex review): two messages entering the wait in the same millisecond with the same packet prefix shared one entry, so both got the second one's row, and now each gets its own; with message_correlation_timeout under 0.1 s the entry could be pruned during the wait, skipping the second lookup; and a non-string pubkey_prefix made the per-packet scan raise, logged as "Error handling RF log data". The list's DEBUG lines are gone with it.
The list, its four methods and PendingMessageEntry were public names, so this goes in as its own change rather than a refactor.
message_correlation_timeout is still read: it bounds how old an RF row can be and still match a channel message by its contents (#255). The example configs described it as a wait time; the comment now says what it does.
New tests cover the wait (a row logged during it is found; no wait without enhanced correlation or for scope lookups). They pass against the previous code and one fails without the wait.
_mesh_graph_capturing() replaces the three copies of `hasattr(self.bot, "mesh_graph") and self.bot.mesh_graph and self.bot.mesh_graph.capture_enabled`, evaluated in the same order after the same preceding conditions.
_manage_contact_capacity(contact_name, after_add) replaces the two copies of the near-limit check in handle_new_contact (device mode, and bot mode after an add attempt). The WARNING and INFO lines keep their exact text, including the " after add" and " after add attempt" suffixes in bot mode; new tests pin all four lines and the cleanup call, and pass against the code before this commit.
handle_new_contact, _process_advertisement_packet, _ensure_contact_meshcore_path_encoding and the NEW_CONTACT helpers from #335 (_remember_advert_rf, _find_advert_rf_data, _claim_new_contact_add, _release_new_contact_add) move verbatim (AST-identical) to modules/contact_events.py. MessageHandler mixes it in, so names, callers (core's NEW_CONTACT subscription, the prefix collision service) and tests are unchanged. message_handler keeps importing upsert_zero_hop_observed_path_via_manager and encode_path_len_byte as re-exports.
The one test that patched modules.message_handler.upsert_zero_hop_observed_path_via_manager now patches it in modules.contact_events, where the moved code looks it up.
_update_mesh_graph, _store_observed_path, _update_mesh_graph_from_advert, _update_mesh_graph_from_trace and the two location lookups they use move verbatim (AST-identical) to modules/mesh_graph_recorder.py. MessageHandler mixes it in, so every method keeps its name and callers and tests are unchanged. message_handler keeps importing update_mesh_graph_from_trace_data as a re-export; unique_recent_repeater_key, which only this branch had imported there, moves with its user.
command_manager: _record_transmission passes extra keyword arguments through, so #336's recipient_key (DMs) and channel_idx (channels) reach record_transmission as on dev.
graph_trace_helper: dev's rewrite from #336, with its two repeater-key lookups (immediate neighbor and _unique_repeater_key) going through contacts_repo.unique_recent_repeater_key, whose queries are identical.
Codex re-review follow-up. A cancelled add (reconnect cancelling callbacks) kept its claim, so a later NEW_CONTACT for the same advert never added; it now releases and re-raises. A last_advert of 0 is a real advert timestamp, not a missing one. The expiry test now fills the advert cache it is meant to exercise.
Codex review follow-up. A channel message from another node with the same text claimed our record, and names the sender-prefix heuristic cannot split ("Bot [RF]", "Bot: RF") never matched; the match now compares the full decrypted message with "<radio name>: <text>", falling back to the text alone before the name is known. Bot shutdown closes the tracker after disconnecting, which waits for queued repeat-count writes; any later write runs inline.
Codex review follow-up. The RX handler cached an advert's RF entry only after awaiting _process_advertisement_packet, which can yield (geocoding), so a NEW_CONTACT handled meanwhile found nothing or an older advert. Adverts are now recorded in their own short cache (_advert_rf) before that await. The match also requires the advert timestamp to equal NEW_CONTACT's last_advert, so a delayed copy of an older advert no longer stands in for the newer one; without last_advert the most recent advert is used.
A failed or raising add releases its claim, so a later NEW_CONTACT for the same advert tries again.
match_packet_hash gave a pending transmission the hash of the first packet heard within its 30-second window, whatever the packet was. On a busy mesh that was usually someone else's message or advert, so its repeats were counted as ours (the web viewer's repeat counts, DARC MoWaS's ACK check, graph_trace_helper's "our trace" test) and our real repeats were not.
A transmission now records what identifies its packet (the channel index for a channel message, the recipient's key for a DM, the tag for a trace) and match_packet_hash claims it only for a packet carrying the same: a GRP_TXT that decrypts on that channel to the same text, a TXT_MSG addressed to the recipient's hash and, when the radio's key is known, from ours, or a TRACE with the same tag. The RX handler passes that evidence through _transmission_evidence, reusing the authenticated channel decode. Without evidence, match_packet_hash only looks up hashes already matched, so record_repeat and graph_trace_helper no longer claim pending transmissions.
Repeat-count writes go to a single worker thread instead of running sqlite on the event loop, and _update_command_in_database no longer inserts into sys.path on every repeat.
bot_prefix still reads meshcore.device and stays None; fixing it turns on graph_trace_helper's trace updates, which have never run, so it is a separate change.
handle_new_contact walked the last ten RF cache entries and used the first with routing info, whatever its type or sender. That packet's route went into the contact's out_path (and from there to add_contact in bot mode), the mesh graph and observed_paths, and its hash and signal data into tracking.
It now uses _find_advert_rf_data: the cached ADVERT whose payload starts with the contact's public key, newest advert first, and among the copies of that packet the first one heard, within rf_data_timeout. The mesh graph and observed_paths writes are gone: _process_advertisement_packet already records them for the same packet, so a correct match was counted twice.
With the correct hash, tracking reports the packet as a duplicate (the advert packet was tracked first), and bot mode skipped add_companion on that. The add gate now dedupes on the (public key, packet hash) pairs NEW_CONTACT itself added, so repeated events for one advert still add once.
A codex review found that moving dashboard code out of app.py dropped `modules.web_viewer.app.DashboardStatsService`. The public-API golden only recorded names a module defines, so I added a second one, `public_imports.json`, recording the public names each module imports from another project module, generated from dev's tree as the baseline. It found 20 more names earlier refactor commits had stopped importing (the Nominatim and MQTT-weather helpers in wx, gwx, aqi and solarforecast; `PayloadVersion`, the region-warning verdicts and `decode_path_len_byte` in message_handler; two feed helpers and two feed limits in the web viewer). All are imported again as re-exports, so `modules.<module>.<name>` and patches of it resolve as they did on dev.
Seven places in message_handler and graph_trace_helper ran the same pair of queries, byte for byte: count the repeaters and room servers whose key starts with the prefix and were heard within the recency window, and if exactly one was, fetch its key (starred first, then most recent). contacts_repo.unique_recent_repeater_key now runs it and returns (count, key); each caller keeps its own logging, including the two that report the match count. Same queries, same order, same handling of execute_query's empty result on a database error.
#315 changed populate_missing_geolocation_data, which S1 had deleted as dead. It is public, and local plugins may call public manager and command methods, so it comes back in dev's form, along with 11 other public methods S1 removed: CommandManager.get_available_commands_list, GreeterCommand.get_recent_greeted_users, WxCommand.abbreviate_alert_title and get_current_conditions, MessageHandler.discover_message_path, and RepeaterManager.add_discovered_contact, get_daily_advertisement_stats, get_nodes_per_day_stats, get_purging_stats, restore_repeater, test_meshcore_cli_commands and toggle_auto_add. All are verbatim from dev; every self. attribute they use still exists. A scan of every public name in modules/ on dev against this branch now finds nothing missing.
A codex review of the branch found seven places where earlier refactor commits changed behavior. Each is restored here with a test:
- base_service imported flood_scope relatively; the local-service loader also runs that file as local_services.base_service, where the import cannot resolve. Now absolute.
- Radio-health fields were initialized after commands and services were built, so a plugin constructor reading is_radio_offline raised. They are now set before any plugin.
- Removing the per-command can_execute overrides made 31 commands honor skip_channel_check, which their overrides had dropped. BaseCommand.honors_skip_channel_check (False on exactly those commands) keeps them enforcing their channel list.
- CommandManager.get_plugin_metadata, WxCommand.get_weather_for_zipcode and PluginLoader.validate_plugin were public compatibility entry points; they are back.
- feed_manager stopped exporting DEFAULT_MAX_FEED_RESPONSE_BYTES and DEFAULT_MAX_PARSED_FEED_ITEMS despite the commit saying it re-exported them; it does now.
- wx and gwx logged companion-location lookup failures at WARNING; the shared helper logged at debug. It now takes the level.
- The INFO 'MESSAGE ROUTING' / 'DIRECT MESSAGE' lines for pubkey-correlated DMs were removed with the dead path_info computation; they are logged again by a log-only helper.
decode_meshcore_packet, the path processor, the path chunkers, format_path_string and the route/payload type-name tables are now plain functions in packet_decode.py. MessageHandler keeps every old method as a delegate that supplies the configured prefix width and its logger, so callers and tests that patch decode_meshcore_packet on a handler still work. The existing quirk is preserved: path_info uses the configured width while path/path_hex follow the packet's own bytes-per-hop.
Parity: decoding a sample of 4,009 real packets from the dev database (plus malformed inputs) at prefix widths 2 and 4, with the path and format helpers, produces byte-identical output before and after. A handler built with no bot attribute now decodes path_info at width 2 instead of falling into the error branch; only bare test objects can hit that.
The eight scope helpers (transport-code HMAC match, decode field extraction, reply-scope resolution, route-type and eligibility checks, the confirmed-global test and the scoped/global/unknown classifier) used no handler state beyond the logger. They are now plain functions in scope_gate.py; MessageHandler keeps every old private name as a staticmethod alias or a one-line delegate passing its logger, so the tests that call them on handler instances, including object.__new__ ones, are untouched. The RF-correlation markers and rf_data_is_correlated move to rf_match.py, which both modules import; message_handler re-exports them.
Bodies moved verbatim apart from dropping self. scope_gate is a new module, so mypy checks it.
Four if/elif ladders (SNR and RSSI, for DMs and channel messages) checked a list of payload keys, then a list of metadata keys, converting the first present value. _signal_value takes the key lists and converter, so each site keeps its own keys: DMs still accept signal_to_noise and signal_noise_ratio, channel messages do not. A present-but-None key still ends the search, metadata is still read only when no payload key is present, and the cache and RF fallbacks after each call are untouched. A test checks it against the original ladder for every combination of absent, None and valued keys.
Five loops scanned meshcore.contacts for the first entry matching a public-key prefix or adv_name. _find_contact(predicate) does the scan; each call site keeps its own predicate and guard. In handle_contact_message the display-name and full-key lookups used the same predicate, so they now share one scan; the display-name lookup still runs with an empty prefix (matching the first contact, as before) while the full-key lookup still requires a non-empty one.
Five copies split a hex path into node IDs of N characters and fell back to one-byte nodes when the path did not divide evenly. They now call split_path_hex; the one caller that wanted uppercase nodes upper-cases the result. The observed-path parser that drops an incomplete trailing node instead of falling back keeps its own loop.
Six places in message_handler and command_manager spelled out hasattr(bot, 'web_viewer_integration') and ... and ....bot_integration, then reached through the same chain again to call it. A _viewer_bridge() method on each class returns the bridge or None, and the call sites bind it once with :=.
handle_contact_message built path_info from the contact table, the payload's path_len and a correlated RF packet's routing_info, then unconditionally replaced it (and path_len) before constructing the MeshMessage; the RF lookup it did was repeated after the message was built. The only effect of the removed block was a pair of INFO log lines reporting a route the DM never carried. The helpers it called (find_recent_rf_data, decode_meshcore_packet) mutate nothing.
handle_new_contact's 'unknown contact type' fallback could not run: the branch above returns on both of its paths whenever the bot has a repeater_manager, and every statement in the fallback was guarded by that same hasattr. RepeaterManager.add_discovered_contact was called only from there.
Every name removed here has zero references across modules, tests, docs, local plugins, scripts and generate_website.py, and none is part of the documented plugin surface (db_manager, command_manager send methods, channel_manager stay untouched). About 1,160 lines:
- multitest: four formatter helpers that only referenced each other (the flat and nested variants are the ones in use)
- path: _get_api_cache_data, _filter_recent_repeaters
- repeater command: _handle_contact_stats, _get_geocoding_status
- airplanes: _send_split_response; greeter: get_recent_greeted_users; plugin_loader: validate_plugin
- wx: get_weather_for_zipcode, abbreviate_alert_title, get_current_conditions
- message_handler: discover_message_path, and an ADVERT debug line behind a self.debug attribute that is never set
- utils: _get_node_location_and_key_from_db
- repeater_manager: test_meshcore_cli_commands, toggle_auto_add, restore_repeater, get_purging_stats, populate_missing_geolocation_data, get_daily_advertisement_stats, get_nodes_per_day_stats
- command_manager: get_available_commands_list and get_plugin_metadata (HelpCommand and PluginLoader have the live versions)
- base_command: _load_command_prefix, and a no-op .replace('_', '_')
- scheduler: the first of two _run_data_retention definitions, shadowed by the second since it was added
- core: the write-only last_clock_sync_time, and _setup_routing_capture, which only logged a line
Full suite, ruff and mypy pass, and the characterization golden files are unchanged.
_channel_secrets() assigned dict_items and enumerate to the same
variable, and mypy inferred Any | None for the channel_idx .get() with
a mixed Any | int default. Declare items as Iterable[tuple[int, Any]]
and hold the raw index as Any. Runtime behavior is unchanged.
Also cover the list layout meshcore actually uses, where entries
without channel_idx fall back to their position.
**A `global` verdict now requires RF correlated to the message.** It was also
reachable through `_is_confirmed_global_flood`'s argument-from-absence route
("no scope-eligible packet in the window, therefore unscoped"), so a row that
literally said TC_FLOOD, or a scoped ADVERT that the GRP_TXT filter excluded,
came back as "no region code". That inference is fine for deciding whether a
`*` in flood_scopes authorizes a reply; it is not fine for accusing someone of
a misconfiguration. Channel messages still correlate through the payload match
(#255), so the ordinary case is unaffected.
**A `%` in the warning message no longer wedges config reload.** `_get` read
without `raw=True`, so configparser's interpolation raised, the error was
swallowed, and the bot transmitted the default wording instead. Worse, the
bot's own `_validate_config_snapshot` iterates `config.items(section)` and
would reject every hot reload until the file was hand-edited. The save endpoint
now rejects `%` outright and caps the message at 500 characters, and the read
is raw so a hand-edited value is still honored.
**One unreachable sender no longer eats the daily cap forever.** The cap counted
failed attempts but the per-sender cooldown did not, so a node the radio cannot
reach was retried every `min_unscoped_messages` messages indefinitely and no
real offender was ever warned. Both count attempts now. The regression test
fails with three sends against the old filter.
**The sender is a display name, not an identity.** MeshCore's CHANNEL_MSG_RECV
carries no public key, so `sender_pubkey` was always empty on this path and the
only identity is a prefix anyone with the channel key can forge. DM delivery
now requires a contact the radio already holds, which bounds the bot to nodes
it knows and stops failed sends spending cap slots. Two tests asserted the
opposite because the fixture supplied a pubkey the radio never sends; they now
run with what the call site actually passes, and the docs no longer claim
pubkey identity.
Also: a negative `max_warnings_per_day` fell back instead of clamping to the
"unlimited" sentinel; tallies group case-insensitively so a `#` or case change
does not split a channel; retention uses the same clock the rows are written
in; the status figure shows delivered with attempts beside it, rather than a
count of four next to "last warning: none yet"; and the channel bars are scaled
over classified traffic so their width equals the percentage printed beside
them (33.7% was drawn at 28.7%).
A channel message with no "Name: " prefix falls back to a stand-in name.
That is not a node: every such message shares the identity, so they would
accumulate a run together and then get a DM addressed to a contact that
does not exist. The hook now passes no sender for those, so they are still
counted but can never earn a warning. The literal is a named constant and a
test pins the hook's guard to it.
Also folds the channel body-budget formula into models.channel_body_limit
rather than letting the web viewer keep a fourth copy of
"max(130, 160 - len(name) - 2)"; BaseCommand and CommandManager now call it
too, and 158 becomes models.DM_BODY_LIMIT.
Closes#279.
A MeshCore client with no region configured sends every channel message as
a plain FLOOD, which every repeater on the mesh rebroadcasts. This adds two
things: free observation of how much of that the bot hears, and an opt-in
warning to the senders.
Observation classifies each channel message as scoped, global or unknown and
tallies it per channel per local day. It costs one upsert and no airtime, and
it is what lets an operator see the size of the problem before deciding to
spend airtime on it. The classification runs ahead of the flood_scopes
allowlist, because an unscoped message is exactly what that allowlist drops.
Warnings only ever fire on positive RF evidence of an unscoped FLOOD. Absence
of correlation is not proof that a sender omitted a region, so it classifies
as unknown and stays quiet. They are off by default, start in dry run, and are
fenced by min_unscoped_messages, a per-sender cooldown, a mesh-wide cooldown
and a daily cap on attempts. Dry run consumes the same budget it previews, so
the log is what going live would put on the air, not an upper bound. All three
limits read from the database, so a restart cannot release a burst.
Channel-delivered warnings go out at global scope on purpose: the recipient is
by definition outside any region the bot replies under.
New Settings -> Region Warnings page in the web viewer covers all of it. Its
dark-mode striped rows exposed a pre-existing base.html bug where Bootstrap's
light-theme text color survived on a dark row background (~1.3:1); fixed there
for every table in the app.
Verifying a channel message against the RF cache only ever checked the row
strategy 4 returned — the most recent packet heard. That assumes the RF log row
and the decoded CHAN event for one reception arrive back to back with nothing in
between. On a dense mesh they do not: a repeater's echo of the very same packet
is routinely logged in the gap.
The reporter's message was heard directly (SNR 13.25, 0 hops) and again via
repeater f0 185 ms later (SNR 12.0, 1 hop). Both rows carry packet hash
392926C85DCB87D0, but the check saw only the echo, disagreed on path length and
SNR, and left the route unresolved — so the bot withheld a path it had decoded
correctly and answered "No path information available in current message". The
reporter's own observation that MultiTest still reported paths is the tell:
multitest reads recent_rf_data directly and never consults the match tag, so the
radio data was there the whole time.
The cache is now searched for the row the payload matches rather than testing
just the newest one. #80's guarantee is unchanged — a route is still only ever
attributed on a positive match, never on recency — so this widens where the
check looks, not what it accepts. When more than one row matches they must
resolve to a single non-empty packet hash, which is only true of receptions of
one packet; two unrelated packets that happen to agree on all three fields stay
a fallback. A debug line now names the case where the newest row was not the
message's packet, so this failure mode is visible in logs rather than silent.
Side effect worth knowing: SNR and RSSI now come from the message's own
reception too. The reporter's message was logged at SNR 12.0 / RSSI -10, the
echo's figures, when its actual reception was 13.25 / -32.
Five tests cover it, including a reproduction built from the issue's log. Three
fail on the current code; the other two pin behaviour the fix must not break —
newest-wins among receptions of one packet, and scope_eligible_only still
filtering the search so the scope correlation cannot be handed an ineligible row.
{packet_hash} came back empty on a channel, and so did {path} and
{connection_info}. message.routing_info is only attached when the RF row is
correlated, and a channel message can never correlate: MeshCore's CHAN event
carries neither raw_hex nor a pubkey prefix, so every prefix strategy in
find_recent_rf_data is skipped and the message lands on the most-recent-packet
fallback, which #80 rightly refuses to take a route from.
The fallback is in fact almost always the right packet—the firmware emits the
RF log row and the decoded CHAN event for one reception back to back—but
"almost always" is a guess, and #80 is what guessing costs. The CHAN payload
does restate three things the RF row records independently: payload type,
path length and SNR. Requiring all three to agree, on a row no older than the
correlation window, turns the fallback into a checked hypothesis and earns a
real match kind rather than a fallback tag.
SNR carries the weight: it is one reception's measured value, quantised to
0.25 dB, so an unrelated packet agreeing on all three is improbable rather
than merely unlikely. Across the 55 channel messages in my logs the
immediately preceding RF row agreed on all three every time, always within a
second, while a window-wide search on the same fields was ambiguous for half
of them—so this verifies the ordering the firmware already gives us instead
of searching for a match.
Because these messages now correlate, they also satisfy the '*' gate by the
primary route, resolve their scope properly when flood_scopes is regional,
and contribute their path to the mesh graph, which they never did before.
MeshCore's CHAN payload carries neither raw_hex nor a pubkey prefix, so a
channel message has no correlation key and find_recent_rf_data always lands
on the most-recent-packet fallback. rf_data_is_correlated() is therefore
never true for one, and _is_confirmed_global_flood could never pass: across
three log files every one of 45 received channel messages was rejected.
I raised this when the gate went in and accepted the counter that the
handler already has the general RF correlation and decoded packet info for
this message's own packet. That holds for DMs and is false for channel
messages.
'*' asks one question—was this a scoped regional flood?—and there is a
second way to answer it that the handler already computes and then discards.
A scoped message travels as TRANSPORT_FLOOD GRP_TXT, so if no such packet is
anywhere in the RF window, the message cannot have been scoped. That is a
window-wide fact, so unlike a route type read off a fallback row it does not
depend on having picked the right cached packet.
Correlated rows stay authoritative: a correlated TRANSPORT_FLOOD is still
blocked even when the scope window is empty, so an empty window cannot
launder a message the radio positively identified as scoped. Replaying the
real packets from the log, the 43 plain-FLOOD messages now get a reply and
the 2 that had a TC_FLOOD GRP_TXT alongside them still fail closed.
A flood_scopes of "*" alone leaves scope_keys empty while setting
flood_scope_allow_global, and the loader already logs that as an active
allowlist. The handler gated on scope_keys alone, so that configuration
skipped authorisation entirely and admitted absent, uncorrelated and
TRANSPORT_FLOOD traffic. The gate now fires when either is set, so "*"
means global-only rather than everything.
The scheduled-message duplicate check ran outside the write lock inside
update_ini_values, so two concurrent creates for the same schedule could
both pass and the second silently replace the first instead of getting
the 409. Check and write are now one critical section, and delete is too.
Repaired the delete path's error handling while moving it, so an OSError
during the write is still a 500 rather than escaping.
Accepting the reviewer's rejection of my narrower version. I had argued
that requiring positive proof would silence legitimate global replies
whenever correlation is unavailable, but that objection does not hold:
the handler already has the general RF correlation and decoded packet
info for this message's own packet, so the normal global case can be
proven rather than assumed. Only genuinely uncorrelated traffic is
affected, and for an allowlist that should fail closed.
_is_confirmed_global_flood() now requires RF data correlated to this
message showing RouteType.FLOOD. Absent, uncorrelated, or
TRANSPORT_FLOOD data means the scope is unknown, and '*' no longer
admits it.
The channel-message tests never set flood_scope_keys, so it was a Mock
and read as truthy, meaning they were unintentionally exercising the
allowlist and only passed because `not allow_global` on a Mock is False.
Set explicitly to the unconfigured default they meant to test.
Path distance was still leaking between concurrent requests three ways,
all of which Codex reproduced:
- The formatter fell back to shared instance state whenever the request's
own value was None, so a request with no measurable distance rendered
another request's. It now consults instance state only when there is no
request to read from.
- One extraction branch still called _decode_path without the request, so
that route stored its distance on shared state alone.
- _get_sender_location read the shared _current_message, so an
interleaved command could measure from the wrong sender. It takes the
request's message, falling back to the shared value only for direct
calls that pass nothing.
Scope authorisation: '*' permits unscoped global traffic, not traffic of
unknown scope. When a scope-eligible packet was heard but could not be
tied to this message, its scope is unknown and '*' no longer admits it.
Deliberately narrower than the reviewer suggested: when no scope-eligible
packet was heard at all, '*' still applies, because nothing scoped being
heard is what a genuinely global message looks like. Requiring positive
proof of FLOOD in that case would silence legitimate global replies
whenever correlation is unavailable, which is too much availability to
trade for the residual risk.
Two fixes from the previous round were incomplete:
- The scheduled-message chunk budget used the schedule's explicit scope,
but send_channel_message resolves an unset scope from
flood_scope.<channel> and then outgoing_flood_scope_override. A
schedule with an implicit regional scope was therefore sized for a
global send and every chunk could overshoot once the sender added the
regional header. The budget now resolves the effective scope, and
assumes regional if that resolution fails, since guessing regional only
ever makes chunks smaller.
- Resetting _last_path_distance_km per request fixed sequential reuse but
not concurrency: the decode awaits a database lookup, and the
dispatcher runs handlers as independent tasks, so two path commands can
interleave and render each other's distance. The distance now rides on
the request's own message, with the instance attribute kept only as a
fallback for direct calls.
Two new findings:
- rf_data_is_correlated() treated pubkey and partial-prefix matches as
packet-unique, but a sender prefix identifies a sender, not one
transmission. With several cached packets from the same sender, the
first (usually oldest) was returned and allowed to supply a route.
Those strategies now take the newest match and are authoritative only
when the match is unambiguous; otherwise the entry is marked fallback,
so it still provides SNR/RSSI but never a route.
- The flood_scopes allowlist accepted a scope resolved from an
uncorrelated fallback packet. The HMAC proves the cached packet is in
an allowed scope, not that this message is, so a recent allowed-scope
packet could admit an unrelated message. Scope authorisation now
requires packet-bound correlation and logs plainly when it does not
have it.
That last one is a deliberate fail-closed change to an authorisation
path that predates tonight. Channel messages normally carry raw_hex and
correlate exactly, so the fallback is the exception rather than the rule,
but a deployment using flood_scopes will now stay quiet in cases where it
previously replied on an assumed scope.
Correctness:
- {path_distance} was always blank in production. The resolution code that
builds repeater_info dropped latitude/longitude in every branch, so the
calculator could never find a coordinate. My tests passed hand-built
dicts straight to the calculator and never exercised the builder, which
is why they stayed green. Coordinates are now carried through all four
construction sites, and the new tests drive _lookup_repeater_names via
its lookup_func hook so the real builder runs.
- The #80 route guard was defeated two ways in the channel handler. When
the RF data was an uncorrelated fallback, control fell through to the
raw-hex and routing_info fallbacks below, which took the route from the
unrelated packet anyway; the guard had actually made that path
reachable. message.routing_info was also assigned unconditionally, and
the path command reads it. "Not attributable" is now a terminal branch
and the routing_info hand-off checks provenance.
- Same fix was incomplete for DMs: routing_info was captured and turned
into path_info before the provenance check ran, so the later check only
declined to overwrite an already-wrong value. Guarded at the source.
- Rendering could transmit for real. Capture only intercepts
send_response, but advert calls send_advert() directly and
send_response_chunked never checked capture_sink. Chunked sends are now
captured, and rendering is opt-in via BaseCommand.render_safe (default
False) instead of a denylist that cannot be complete. This also closes
the DM-only leak: schedule is not marked safe, so {cmd:schedule} can no
longer broadcast configuration to a channel.
- Multi-part rendered output was rejoined into one oversized send.
Scheduled messages are now split to the RF body budget and sent through
send_channel_messages_chunked, on character boundaries so multi-byte
text is not corrupted.
- _last_path_distance_km is instance state that was only set on success,
so an invalid path request could show the previous request's distance.
Reset at the start of every execute().
- Stale-contact retries were only counted on non-OK results, so timeouts
and exceptions left a contact eligible forever and could recreate the
storm. All failed attempts count now.
Web viewer:
- A failed config reload was reported as a successful save. The API and
UI now distinguish "saved and active" from "saved, restart needed".
- Preview count was unbounded; clamped to 1-20.
- update_ini_values is a read-modify-replace with no locking, so two
concurrent viewer saves could lose one. Serialised behind a lock.
find_recent_rf_data has four strategies, and the fourth returns the most
recent packet in the cache when the first three fail to correlate. That
fallback exists for SNR/RSSI timing issues, but callers were also taking
its route. So when correlation missed, a message was recorded with some
other transmission's path and hop count — the reporter's four-hop message
stored as a single direct hop via 79, which is the last hop of an
unrelated packet. Rare, because it only fires when correlation fails.
Results are now tagged with how they were matched (exact, pubkey, partial
or fallback) and rf_data_is_correlated() gates the route. The tag rides on
a shallow copy so the cache entry is never marked, and it fails closed: an
untagged dict is treated as uncorrelated.
Guarded in both handlers. The DM path was worse than the channel one — it
falls back to find_recent_rf_data() with no correlation key at all, which
can only ever return a fallback, and then overwrote message.path and
message.routing_info from it. routing_info is what the path command reads,
so a wrong route reached the user directly.
The route is now left unresolved rather than fabricated, which also stops
a bogus edge being written to the mesh graph from path_nodes that belong
to a different packet. SNR and RSSI still use the fallback as before;
mis-correlated signal figures are approximate rather than structurally
wrong, and changing them is a separate call.
Existing tests asserted the returned dict was the cache entry itself.
Identity was incidental, so they now compare contents and additionally
assert the provenance tag.
This commit enhances the handling of zero-hop neighbors in the dashboard and database. It ensures that the **One-hop neighbours** section accurately reflects radios heard directly (MeshCore hop count 0) rather than originators of relayed adverts. The `observed_paths` table now includes nullable `snr` and `rssi` columns for zero-hop advert rows, allowing for better signal reporting. Additionally, a one-time backfill process copies recent zero-hop ADVERTs from the `packet_stream` to `observed_paths`. Documentation and tests have been updated to reflect these changes.
Changed the default setting for `auto_manage_contacts` from `false` to `device` across configuration files and updated related documentation. This ensures that the device handles auto-addition of contacts while the bot manages capacity. Adjusted comments and documentation to reflect this change for clarity and consistency.
- Updated logging statements in the `MessageHandler` class to use `debug` level instead of `info` for `RAW_DATA` and `NEW_CONTACT` events, reducing log verbosity while maintaining essential information.
- This change enhances the clarity of logs by reserving `info` level for more significant events.
- Resolves issue #214
NEW_CONTACT adverts were always logged as newly discovered, even for
nodes heard many times before. Check the tracking DB (get_tracked_contact_row)
and the live device contact list (is_contact_on_device) to classify the
advert, log "known" vs "new" accordingly, and skip the
new_contact_discovered audit entry for contacts that are already known.
Also dedup the is_currently_tracked device-contact lookups onto the shared
is_contact_on_device() helper.
Behavior preserved: repeaters remain database-only, device-mode companion
adds are still left to the firmware, and no per-advert device round-trip is
introduced.