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.
GlobalWxCommand and WeatherService had identical _load_weather_model methods. The logic is now weather_common.load_open_meteo_model(config, logger); both methods delegate, so the existing tests that call them are unchanged.
Five methods were the same in WxCommand and GlobalWxCommand: _format_high_low, _get_custom_mqtt_weather_topic, _mqtt_weather_line (only the catalog namespace differed), _count_display_width and _send_multiday_forecast. They now live once in modules/weather_common.WeatherCommandMixin, which both commands list before BaseCommand; each sets translation_ns ('commands.wx' / 'commands.gwx'). The mixin is outside modules/commands, so the plugin loader does not treat it as a command.
A new characterization test pins how a multi-day forecast is packed into messages (packing, rate-limit skips, pauses); it passes on the code before and after.
wx, gwx, rain and aurora each carried their own _get_companion_location (the same contact-tracking query) and _get_bot_location (the same [Bot] lat/lon read and range check), while location.py's get_companion_lat_lon and get_bot_lat_lon had no callers. The command methods keep their names, since tests patch them, and now delegate. The helpers take an optional logger so lookup errors are still logged at debug. wx and gwx previously returned the database values without float(); on a REAL column that is the same value.
reload_config named the same thirteen attributes four times: building the candidates, snapshotting the old values, publishing, and rolling back. The candidates now go in one dict whose keys are the attribute names; the snapshot, the publish loop and the rollback loop all come from it. Command config state, max_channels, solar config, the region-warning monitor and the scheduler keep their explicit rollback steps, since a reference swap cannot undo them. The reload failure-injection tests pass unchanged.
setup_logging's formatter, handler and library-logger wiring is now configure_bot_logging(config, bot_root); MeshCoreBot.setup_logging calls it and still installs the signal handlers. The meshcore logger list that setup_logging and _configure_meshcore_debug_logging each spelled out is one MESHCORE_LOGGER_NAMES, and both use configure_meshcore_loggers. _JsonFormatter moves along as JsonFormatter. Statement order inside setup is unchanged, so the startup log lines come out as before.
Also fixes mypy on the admin_server module from the previous commit: as a new module it is type-checked, and its Flask view annotations needed loosening.
create_default_config held a 340-line config.ini as a string literal in core.py. The text now lives in a packaged template (added to package-data, which the packaging test checks) and the method writes it the same way. The file written on first run is byte-identical to before, checked against the literal's value from the previous commit.
_BotAdminServer (the loopback Flask thread behind [Admin] with the reload and health endpoints) is unrelated to the rest of core.py and moves unchanged to admin_server.BotAdminServer.
process_rss_feed and process_api_feed ended with the same 50 lines: apply the sort config (or reverse to oldest first), collect already-posted and still-queued item ids, drop those, and record the newest item as last_item_id. That tail is now FeedManager._select_new_items, called from inside each poller's existing try block so errors are still logged as RSS or API failures.
The bot's poller and the web viewer's preview each carried the same API timestamp parser, API item field extraction, RSS published-date parse, and the allow_private_urls / max_response_bytes / max_parsed_items config reads. modules/feed_parse.py now holds those; the default size limits move there and feed_manager re-exports them.
Where the two sides differ they still do: the bot skips non-dict and id-less API items while the preview turns string items into dicts, and the preview unwraps items/data/results from a bare dict response. Those divergences are a separate decision.
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.
get_config_value converted values in two parallel if/elif ladders (current/old/legacy sections, then legacy enabled aliases). Both now call _read_typed_option, which picks getboolean/getint/getfloat or splits a comma list. The 'value != fallback or has_option' test it replaced was always true, since has_option had just been checked. The unknown-type warning still fires only on the section path, as before.
HelpCommand and CmdCommand carried identical _is_command_valid_for_channel methods; it now lives once on BaseCommand under the same name.
BaseCommand.translated_or(key, fallback, kind=list) returns a catalog value when it is a non-empty list (or dict) and the built-in fallback otherwise. hello's six getters and catfact's use it. The built-in fallback data stays: a configured or local catalog can leave a key out or empty, and then these lists are what the bot says.
hacker's get_hacker_error was a 23-branch if/elif with an inline list per branch; it is now an ordered _HACKER_ERRORS table of (prefixes, key, fallback) generated from that chain, with the generic list last. For every probed command it selects the same translation key and the same candidate list as before, checked by running both versions.
The two commands carried near-identical copies of fetch-until-short, split-at-a-natural-break and two-part send. modules/joke_text.py now holds them as plain functions (a mixin module under modules/commands would be loaded as a plugin and recorded as a failure). Each command keeps its public method names, its emoji, its own split points (joke also splits on newlines) and its own pause: joke still waits 2 s between the two parts, dadjoke does not. Lengths stay counted in characters against 130, as before.
reload, channelpause, status and feed overrode requires_admin_access() to return True, then checked 'if not self.requires_admin_access(): return False' in their own can_execute, which could never fire. BaseCommand gains an admin_only class attribute that requires_admin_access() honors before consulting [Admin_ACL] admin_commands; the four commands set it and lose both overrides. status and feed move their enable switch onto enabled_attr. requires_admin_access() is unchanged for every other caller, including local plugins and the website generator.
32 commands overrode can_execute only to return False while their own <name>_enabled flag was off and otherwise defer to the base. BaseCommand now has an enabled_attr class attribute naming that flag, which can_execute checks first. Each command keeps reading its flag exactly as before (same section, default and legacy aliases), and code and tests that set help_enabled, webviewer_enabled and the like still work, since the flag is read at call time.
Only overrides that were exactly 'if not self.<flag>: return False; return super().can_execute(...)' were converted, checked by AST. Commands with extra conditions (wx's delegate, joke's dark-joke DM rule, admin-only and DM-only checks, dadjoke's reversed order) keep their overrides. Several of the removed overrides called super().can_execute(message) without passing skip_channel_check; nothing calls them with it set, so honoring it now changes nothing.
_radio_zombie_detected, _radio_fail_count, _tcp_probe_fail_count, _radio_offline, _send_consecutive_failures, _last_radio_probe and _last_health_update were created on first write, so every read went through getattr(self, name, default) or hasattr. They are now set in __init__ and read directly. The probe timestamp starts as None and is still stamped on the first health-loop pass, so the first probe waits a full interval as before.
Five copies read meshcore.self_info as a dict or an object with slightly different rules. self_info_name(self_info, fields, by_presence=False) returns the first truthy field, or with by_presence the first field an object has at all, which is how BaseCommand has always read objects (so name='' on an object still falls back to config there, but not in CommandManager). Each caller passes its own field order and keeps its own fallback: budgets read name/user_name, mention matching reads name/adv_name. The device-name sync in core is left alone, since it logs the raw value. The identity golden file is unchanged.
MeshMessage.effective_outgoing_flood_scope, CommandManager's per-channel and override readers and its config_section branch, and BaseServicePlugin.get_mesh_flood_scope each scanned config.ini themselves. modules/flood_scope.py now owns those reads: channel_scope_entry (stripped flood_scope.<channel>, None with no entry), outgoing_override, and section_flood_scope (normalized, None when empty). Callers keep their own contracts on top: MeshMessage still reports raw values, CommandManager still normalizes, and None still means 'apply the override later' in resolve_channel_send_scope. MeshMessage.is_global_flood_scope delegates to is_global_marker, the identical predicate. The scope matrix golden file is unchanged.
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 :=.
The notifications, log-rotation and maintenance settings routes each looped over '<prefix>.<field>' keys in bot_metadata, filled unset values with '' and then applied defaults to empty ones, and the matching POST routes (plus zombie-alert's) each looped over an allowed set. _read_meta_settings and _write_meta_settings do both. The zombie-alert GET and the radio probe/offline routes are left as they are: they treat non-string values differently or merge config.ini fallbacks. A new test pins defaults, stored-value precedence, empty-string fallback and allowed-field saves; it passes on the code before and after.
wx, rain and the weather service each built the same requests session (two retries on connection errors and 5xx for GET, 0.3 s backoff, status codes not raised, 10x20 connection pool). It now lives in modules/http_retry.py; each class keeps its _create_retry_session as a one-line delegate, and each keeps its own session lifetime (rain still builds one per call).
Ten routes inserted a channel_operations row by hand (radio reboot, connect/disconnect, firmware and radio-parameter reads and writes, advert, radio-debug reconnect, and two config-reload paths). They now call _queue_operation(type, payload=None), which writes the same columns, JSON-encodes payload_data only when a payload is given, and returns the row id. Errors still propagate to each route's existing handler. The two channel add/remove writes keep their own inserts, since they fill dedicated columns.
Four helpers replace copy-pasted blocks, each keeping the original order of side effects:
- _record_successful_send(rate_limit_key): the global/TX/per-user limiter charges, pasted three times in _handle_send_result
- _record_transmission(content, target, type, command_id): repeat-tracking registration, identical in send_dm and send_channel_message apart from target and type, returning the (possibly generated) command id
- record_command_stat(message, name, response_sent): the guarded stats write, seven copies in this module
- _chunk_spacing_seconds(): the pause between parts of a multi-part send, three copies
message_handler's stats writes go through bot.command_manager.commands directly and tests assert on that, so they are left as they are.
Found by scanning every __init__ assignment for reads across modules, tests and generate_website.py. Removed: wx's forecast_duration, num_wx_alerts, use_metric and zulu_time; the geolocator that wx, gwx, aqi and solarforecast each built and never used (all geocoding goes through the rate-limited helpers); and the web viewer's _db_connection/_db_lock/_db_last_used/_db_timeout pooling fields left over from before connections moved to DBManager. Two test fixtures patched get_nominatim_geocoder only to stub that unused construction.
Left alone although unread: BaseCommand._command_prefix (plugin-facing base class), MeshGraph._batch_task (holds a task reference), and config reads that feed known bugs (trace timeout_per_hop) or documented keys.
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.
meshcore >= 2.3.14 is required and installed, and these guards protected against its absence or against first-party modules failing to import:
- channel_manager: the meshcore_cli get_channel fallback beside commands.get_channel
- core: the set_name hasattr check (an absent method still lands in the existing AttributeError handler and returns False), the wait_for_contacts polling loop after an always-true hasattr(meshcore, 'contacts'), and the hasattr check before _start_queue_processor
- wx and gwx: try/except ImportError around wx_international and wxsim_parser, which import only required dependencies, and the 'parser not available' branches that could never run
Branches that also catch runtime errors raised inside an existing API stay, as do the DM retry and set_flood_scope paths, which open PRs are changing. The inspect.signature checks in execute_commands become one _call_with_optional_user_id helper, keeping zero-argument cooldown hooks working for local plugin commands.
Ten leftovers from earlier unused-variable cleanups: dict lookups, .get() calls and one config read evaluated for nothing. None can raise where it sits: the subscripts read keys a SELECT * row and a fixed four-tuple always carry, and row[0] precedes reads of row[1] and row[2].
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.
The packaging test checks pyproject.toml against git ls-files: every directory under modules/ with Python files must be a regular package, and every tracked runtime data file must match a package-data glob, since include-package-data is off and a new subpackage or data file would otherwise silently miss the wheel. The endpoint inventory records all 104 routes with their methods and endpoint names, plus the status, content type, redirect target, script-src CSP (nonce normalized) and framing headers of anonymous requests with and without a password, and of anonymous mutating requests when locked. Endpoint names feed the CSP nonce allowlist and url_for, so a helper extraction that renames or reroutes anything shows up here.
Execution accounting runs execute_commands with an advancing clock: when the cooldown is recorded, how a second record_execution inside execute moves the boundary, which stats rows each outcome writes, and that zero-argument cooldown methods from local plugins still work. The stale _last_response defect (a silent command after any reply counts as responded) is pinned as current behavior so the refactor cannot change it by accident. Reload rollback injects a failure at each stage (command construction, introduced plugin failures, the region-warning monitor, the scheduler) and asserts every attribute the rollback owns is restored.
First slice of the maintainability refactor: golden-file characterization of behavior that later commits consolidate. The scope matrix runs every resolver (MeshMessage, resolve/effective_channel_send_scope, channel_body_budget, the scheduler's budget resolver, BaseServicePlugin.get_mesh_flood_scope, and the scopes a real send_channel_message hands to set_flood_scope) against 420 combinations of override, per-channel and section scope. The identity matrix runs each node-name lookup against every self_info shape. Both record today's divergences rather than fixing them, so a refactor commit that changes either golden file is a behavior change by definition.
Builds on #303. Channel text is capped at 155 bytes of "<name>: <text>" instead of 124. The firmware encrypts a 4-byte timestamp and a 1-byte text type ahead of the text in 16-byte AES blocks, so 155 fills the 160-byte block exactly (163-byte payload), and the last 5 bytes up to the firmware's 160 would spill into another block (179-byte payload).
124 was set on the premise that repeaters drop 165-byte frames. They don't: allowPacketForward has no size check and MAX_PACKET_PAYLOAD is 184, and 163-byte GRP_TXT payloads are routinely seen at 3+ hops. The frame-budget tests now pin the limit within the firmware's 160 and to a whole cipher block instead of to a 131-byte relay ceiling.
Also corrects the payload model's plaintext header from 4 bytes to 5, and moves a test import back above the helper that had split the import block.
* feat(web_viewer): split anonymous read access from an admin session
When web_viewer_password is set, Dashboard, Contacts, and Mesh Graph are readable without logging in, along with the read-only APIs they need. Radio, Config, Logs, every mutating API, /api/channels, and the live Socket.IO streams require an admin session, since those can expose channel keys and decrypted traffic. The Realtime page still renders publicly but shows "Admin login required" instead of connecting. Anything not on the public allowlist fails closed.
Login now compares HMAC digests with hmac.compare_digest and clears the session before setting authenticated_admin. Logout clears the whole session and returns to the dashboard.
Co-authored-by: Gerard Hickey <hickey@kinetic-compute.com>
* fix(web_viewer): stop Contacts redeclaring IS_ADMIN and hide banner admin buttons
contacts.html declared its own top-level const IS_ADMIN, but base.html already declares one in an earlier script on the same page. Classic scripts share one global lexical scope, so the redeclaration threw a SyntaxError and the whole Contacts script never ran, leaving the table on its loading placeholder in every mode. Contacts now uses the global from base.html.
The zombie-recover and clear-offline banner buttons were still rendered for anonymous viewers even though their /api/admin/* endpoints now return 401. They are gated on the same auth_enabled/is_admin check as the other admin controls.
* fix(web_viewer): drop sockets on logout, block off-site login redirects, allow HEAD on public paths
Logging out cleared the HTTP session but left that login's Socket.IO connections open, and a socket keeps the session it connected with, so a /realtime tab kept streaming packets, decrypted messages, and logs after logout. Each login now gets an admin_login_id, sockets record it on connect, and logout disconnects the sockets opened under that login. Other admin logins stay connected.
The login next check passed ///host and /\host because urlparse sees no netloc in either, but browsers resolve both as another origin. Next targets are now rejected if they start with //, contain a backslash, or contain a tab or newline. This check predates the admin split.
Public pages and GET APIs only accepted literal GET, so HEAD (and OPTIONS) on them returned 401 and made uptime checks report the viewer as down. They now accept HEAD and OPTIONS too; admin paths are unchanged.
* docs(changelog): note the login redirect fix
* fix(web_viewer): pass IS_ADMIN through a body data attribute so ESLint can parse base.html
The frontend lint job failed with "Parsing error: Unexpected token {" because base.html's inline script assigned IS_ADMIN from a raw Jinja expression, which isn't valid JS until rendered. The flag now lives in data-is-admin on <body> and the script reads it from document.body.dataset. I also dropped the unused AUTH_ENABLED constant and declared IS_ADMIN as a readonly ESLint global, since contacts.html reads it from base.html.
---------
Co-authored-by: Gerard Hickey <hickey@kinetic-compute.com>
Decoded packet_stream rows had no way back to the original packet, so alternate routes could not be joined. The hash is already on routing_info; keep it in the existing JSON blob.
An oversized webhook or channel body was handed to the firmware whole. The device dropped it without an ack, retries stalled the transport, and the radio looked dead. Split to the frame budget, keep links intact, and tag multi-part sends so they can be reassembled.
Reserve the wx location prefix from that same budget, and let NOAA alert polling be turned off without disabling the rest of the weather service.
Commands are bare by default; `command_prefix` is empty unless configured, so
`!wx` misrepresents a stock install. Shipped release sections are left alone.
Fold the Unreleased section into 1.1.0 and date it. The 1.1.0 heading was
written on 2026-09-13 but never tagged, so everything merged since belongs in
it; the section also carried two separate "### Added" headings from earlier
merges, now consolidated. All 79 entries preserved.
Extend the v1.0.0 -> v1.1.0 upgrade guide with the meshcore 2.3.14 requirement,
migration 24, and the configuration and behavior changes that landed after the
heading was first written.
The report used /n and got two literal characters. The escape works the same in
every keyword path — I checked the test command's template engine too — so the
defect is that the instructions never said "backslash" and used `test` as their
example, which is the one keyword a built-in command owns. That command renders
through a different engine and silently drops the mesh-info placeholders listed
in the same block.
Also documents the configparser continuation form, which needs no escape, and
corrects two packet-capture examples that still showed a naive local timestamp
where the bot has published UTC with a Z suffix since v1.0.0 (#276).
agessaman