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.
Discover local commands and services in the Plugins UI and route their settings to the local config overlay. Upgrade GitHub Actions to Node 24-capable majors and migrate frontend linting to ESLint 10 flat config. Keep command and service discovery namespaces isolated and align duplicate-name handling with runtime loading.
Add a password type to settings_schema so plugin secrets render as masked inputs and never reach the browser. Stored values stay in config.ini; an empty box on save leaves an existing secret unchanged.
[Test_Command] distance_unit (auto/km/mi) controls {path_distance} and {firstlast_distance}. Auto follows the reply language (miles for en/en-US, kilometres otherwise, including en-GB). Elapsed times of 1s and above render as seconds to keep the ack short.
The published packet payload mixed clocks: "timestamp" was UTC ISO 8601
with a Z suffix, but the "time" and "date" fields beside it were rendered
from a naive datetime.now(), i.e. the host's local wall clock. A consumer
reading those two off a bot in a non-UTC zone sees a skew of exactly that
zone's UTC offset and flags the observer's clock as wrong.
Local time was never the intended reading. The original script took time
and date off the firmware log line, and mctomqtt sets the device clock
from calendar.timegm(), so upstream both fields are already UTC.
All three now render one aware UTC instant, so they cannot disagree with
each other or straddle a second boundary. _utc_iso_timestamp() takes an
optional instant to make that possible; its no-arg behavior is unchanged.
The local translation path defaulted to a bare `local/translations/`, resolved
against the process cwd and unrelated to `[Bot] local_dir_path`. Since
`local_dir_path` already selects where an operator's commands, service plugins
and config overlay live, the catalog belongs in that same tree. It now defaults
to `<local_dir_path>/translations`, resolved absolute against the bot root, so
relocating `local_dir_path` moves the catalog with it and the lookup no longer
depends on the working directory. An explicit `local_translation_path` still
wins.
Only the constructor at startup was passing the local path. `get_translator()`
builds and caches a translator per detected language, and `reload_config()`
builds a fresh one, and both were still constructing `Translator` with the
distributed path alone. So local overrides silently vanished from any reply that
used the sender's language, and did not survive a config reload. Both now pass
it, `reload_config()` republishes it alongside `translation_path`, and it is
snapshotted and restored on rollback. The DummyTranslator fallback sets it too,
since `get_translator()` would otherwise raise AttributeError there.
Collapsed `_deep_merge_translations` into the existing `_merge_translations`,
which already merged deeply with the primary winning. Rewrote that one to stop
mutating its input: it shallow-copied `fallback` and then recursed into the
shared sub-dicts, so merging a base language over English corrupted the cached
English catalog at every level below the first. Also replaced the two `print()`
calls on the error paths with logger calls.
Registered `local_translation_path` in the config schema next to
`translation_path`, and reverted a trailing-whitespace reflow that touched 84
lines of config.ini.example for a one-key addition.
Tests: local overlay (single-string override, added keys, local-only catalog,
missing directory), the merge no longer mutating its fallback, the default
following `local_dir_path`, an explicit setting overriding it, the overlay
reaching `bot.translator` end to end, and a reload picking up a moved path.
Addresses the review on #243. The original change interpolated the last
traceback frame into an `err_desc` string used in two places, and the second
one was the problem: `errors.execution_error` is sent back over the mesh, so
an absolute install path went out over RF and spent airtime on the error
path, where retries are most likely.
logger.exception() puts the full traceback in the log instead of just one
frame, which is strictly more than the original wanted, and the RF reply goes
back to str(e). That also removes the `list(tb.stack)[-1]` IndexError on an
empty stack, since the frame handling is gone entirely.
Tests cover both halves: the failure is logged via exception(), and the reply
carries only the exception text with no filesystem path.
CHANGELOG entry moves under the existing `## [Unreleased]` / `### Fixed`.
Add shlink support and rework response templates onto a state machine.
Merged via this branch rather than the PR head: the fork is org-owned, so
"allow edits by maintainers" does not grant push access and the review fixes
could not be pushed to #254 directly. Roger Fedor's four commits are included
with authorship intact, followed by two commits addressing the review.
Differentially tested the current parser against the pre-PR one over every template
shape shipped in config.ini.example and docs (31 shapes x 3 field sets x 4 message
states, 372 comparisons) plus 32 adversarial inputs. That found one regression I had
introduced: reading a quote immediately after ':' as the start of a quoted argument
voided `prefix_if_nonempty:"`, whose unterminated string rejected the whole
placeholder and emitted raw template text. An unterminated quote now falls back to
greedy parsing, so a literal quote prefix behaves as it always has. Both cases are
pinned by tests.
Also covered the shorten_url alias in feed formats, which had none: accepted as a
function, chains like shorten, falls back to the original on failure, is seen through
by shorten_feed_urls so a link is not shortened twice, and does not swallow an
unrelated name that merely starts with it.
Reset the warn-once flag in the async no-warning test. It asserted warning was never
called while a preceding test could already have tripped the flag, so it would have
passed vacuously.
Security: a shlink deployment with short_url_website unset POSTed the operator's
API key to v.gd. _normalize_base falls back to the public default and the shlink
branch guarded only on a missing key, never on a missing base. shlink now requires
an explicit base and refuses a v.gd/is.gd host outright rather than sending an
X-Api-Key there.
Correctness: _shorten_url_with_gd lost its response.ok guard when the backends were
split out, so a 502 whose body starts with http was returned as the short URL and
transmitted over RF. Restored, with a warning. The regression test that should have
caught this passed only because it left mock_resp.text as a MagicMock; it now uses
a realistic proxy maintenance body.
_shorten_url_with_shlink never checked the status either. Shlink reports failures as
RFC 7807 problem details, which parse as JSON and simply lack shortUrl, so a bad API
key was indistinguishable from an unshortenable URL at DEBUG. It now warns on a
non-OK status, a non-JSON body, and a missing shortUrl. Dropped the shortUrlSlug
fallback: that is a request field, not a response field, and returning it puts a
bare slug where a link belongs.
Timeouts and connection errors are back on their own DEBUG handler. They had fallen
through to the broad handler, whose level rose to ERROR in the same diff, so a
routine intermittent uplink logged "Unexpected error shortening URL" on every reply.
Parser: _GREEDY_ARG_FILTERS was tested before the quoted-argument branch, so
prefix_if_nonempty, the one filter already in shipped configs, could not use the
quoted syntax this PR adds. {path_distance|prefix_if_nonempty:"Dist {sender}: "}
rendered raw template text. A quote immediately after the ':' now selects the quoted
grammar; anything else stays greedy, so config.ini.example's
`prefix_if_nonempty: | Path Dist: ` keeps its pipe and its whitespace.
Blocking HTTP on the event loop: the path command rendered its reply prefix inline
from async code, so a slow shortener stalled radio RX, MQTT and every other handler
for the full 5s timeout. Added format_piped_template_async and switched the path
command to it. The test command still renders synchronously through the sync
check_keywords dispatcher; making that async is a separate change, so the filter now
warns once when it runs on the loop.
Naming: one operation should not have two names in an operator-facing DSL. shorten
and shorten_url are aliases in both response templates and feed formats, and
if_nonempty is canonical with if_notempty as an alias, so a filter chain copied
between a feed format and a response_format works either way.
Also: reduced _build_create_shlink_url to the one parameter it uses and dropped its
dead query/startswith lines, renamed the shlink POST callable from `get`, documented
the config argument on format_piped_template, stopped gating the render trace on an
unrelated parameter and logging field values (sender IDs, user phrases) with it, and
moved the changelog entry from Fixed to Added and Changed.
- Extracted `modules/alert_format.py` as the single NWS alert formatter, replacing four copies of the event-type abbreviation table and two of the time compactor. `!wx alerts` and the proactive `WeatherService` broadcasts now localize from one code path, so a Russian bot no longer answers `!wx alerts` in English while its proactive alerts are Russian.
- Stopped leaking translation key paths into mesh broadcasts. `Translator.translate` returns the dotted key when a lookup misses in both the locale and the English fallback, which is right for development but reached the air in production: an unclassifiable NWS title rendered as `⚪Hazardous services.weather_service.event_types.Unknown`, an unmapped WMO code as `services.weather_service.weather_descriptions.4`, and an oddly-cased `wind_speed_unit` as `services.weather_service.wind_speed_units.KMH`. `alert_format.translate_or()` carries an English default at each site, and `WeatherService` now normalizes and validates its three `[Weather]` unit settings the way `GlobalWxCommand` already did.
- Fixed alert expiry rendering in every locale. The formatter rendered a timestamp to a string and re-parsed its own output with `(\d+)(AM|PM)` against a hardcoded English month list, so translated months took the wrong branch and truncated mid-string. Times now carry parsed parts and render through a per-locale `common.alerts.time_12h` template — the space before AM/PM was correct (Russian writes "6 дня", not "6дня"); the downstream regex was the bug.
- Restored month abbreviation. `_compact_time` iterated over abbreviations and replaced them in the string instead of mapping full names, so English stopped shortening "June 28" and Russian replaced the "Jun" inside "June", leaving a stray Latin "e" (`июнe 28`). Reuses the existing `common.date_time.month_abbreviations` rather than the duplicate `services.weather_service.months` block.
- Made `!gwx` display units follow `[Weather]` config instead of the response language. Visibility switched on `base_language != 'en'`, so `language = ru` with the default `temperature_unit = fahrenheit` printed Fahrenheit beside kilometers, and `en-GB` was forced to miles. Pressure is a locale convention rather than a metric/imperial split, so each catalog names its own via `commands.gwx.pressure_unit` — previously every non-English locale inherited mmHg from the English catalog, whose `pressure_mmhg` string contained Russian text, giving German and French users Cyrillic pressure units.
- Let localized `H`/`L` labels reach a standard install. `config.ini.example` shipped the three `temperature_*_format` keys uncommented with literal `H:`/`L:`, and a config value always beats the new locale-aware default, so a Russian bot built from the documented example still rendered `H:47°C L:33°C`. The example now uses the `{high_label}`/`{low_label}` placeholders, which were documented in the docstring but not in the file.
- Routed high/low labels through the reply's translator. `_format_high_low` passed `bot.translator`, so with `auto_detect_language` on, an English-default bot answering a Russian sender localized the rest of the line but not `H:`/`L:`. Added `BaseCommand.response_translator` for this, replacing `wx_international`'s reach into the private `_response_translator` ContextVar.
- Fixed a byte-budget overrun in `!gwx`. The guard on the extra conditions block compared a character count against a byte-derived budget while the rest of the function used `_count_display_width`; Cyrillic is two bytes per character, so the block was appended after the budget was spent.
- Reverted nine `commands.gwx` English rewordings that were not localization work, including the configuration hint in `mqtt_weather_no_subscriber` — dropping it left a mis-configured operator with no pointer to the two keys they need.
- Fixed the Russian `visibility` string, which said "км" on the miles key — the same locale/config conflation as the code bug, in the data. Shortened the Russian event-type abbreviations, which were full words consuming a quarter of the 130-byte budget at two bytes per character.
- Added `commands.wx.hourly_not_available`, missing from every catalog so `!wx hourly` printed the raw key path. Predates this branch; found while auditing every translation key the weather modules reference.
- Moved alert strings to `common.alerts.*` and wind directions to `common.wind_directions.*`, since a command and a service both read them.
- Addresses #267
- Added `original_content` attribute to `MeshMessage` to store the on-air body of messages, ensuring it remains unchanged during command processing.
- Updated command matching methods across various commands to utilize `_cleaned_content_matches`, which restores original content when necessary, preventing mention stripping from altering the message context.
- Implemented tests to verify that original content is preserved and correctly utilized in command execution and message handling.
Add the `local_tranlation_path` to the `Localization` section of
the configuration file to allow one to specify a local translation
file outside the distributed translation files. This allows
translation files to be built for local commands without the
need of altering the distributed ones.
Signed-off-by: Gerard Hickey <hickey@kinetic-compute.com>
Render GWX pressure in mm Hg (rounded hPa->mmHg conversion) for
non-English locales instead of hPa, matching the metric handling used for
visibility. Also fix format_temperature_high_low so custom [Weather]
templates can use the documented {high_label}/{low_label} placeholders,
and add tests for the translator param.
Carto now requires an API key for basemaps.cartocdn.com and is retiring
raster tiles, so the mesh map's dark theme rendered an "API KEY REQUIRED"
watermark. OSM hosts no dark tiles of its own -- its Standard layer is
light only -- so switch the dark basemap to OpenFreeMap's vector 'dark'
style, rendered through MapLibre GL via maplibre-gl-leaflet. Leaflet and
the light OSM raster layer are unchanged, and the bridge renders into
tilePane so marker and overlay z-order is untouched.
Fall back to inverted OSM raster tiles when WebGL 2 or the bridge is
unavailable, so a failure degrades to a filtered map rather than a blank
one. The filter targets .leaflet-tile rather than .leaflet-tile-container:
the container is a 0x0 element that its absolutely-positioned tiles
overflow, so a filter there has an empty reference box and paints nothing.
CSP: drop cartocdn, allow tiles.openfreemap.org, and permit blob: workers.
MapLibre spawns its renderer in a worker from a blob: URL; without
worker-src it falls back to default-src 'self' and never starts.
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.
This commit addresses several issues related to MQTT connections. It prevents MQTT brokers from entering a reconnect storm by ensuring that the packet-capture watchdog only triggers a reconnect when necessary, avoiding duplicate CONNACKs and spurious disconnects. Additionally, it ensures that renewed MQTT auth tokens are properly enforced by implementing a clean reconnect process.
New configuration options are introduced: `mqttN_keepalive` for setting the PINGREQ interval per broker, and `mqttN_jwt_reconnect_on_renew` to control reconnect behavior after token renewal. Documentation has been updated to reflect these changes and to clarify the importance of unique client IDs for brokers in the same cluster.
- Add optional translator param to format_temperature_high_low()
- Translate temperature labels (H/L) via common.temp_high_label/low_label
- Translate wind directions via services.weather_service.wind_directions
- Fix wind format spacing (direction and speed were concatenated)
- Pass translator from Weather_Service, !wx, and !gwx callers
- Add _translate() helper to WeatherService using self.bot.translator
- Replace all hardcoded English strings with services.weather_service.* keys
- Add services.weather_service section to en.json (fallback for all locales)
- Add ru.json with full Russian translation (~450 keys)
format_keyword_response_with_placeholders kept the third copy of the hop-count
logic, and it was the weakest: it consulted message.hops and then went straight
to parsing the path display string, never looking at routing_info. So a keyword
reply and a command reply could describe the same packet differently.
Three cases disagreed, all now resolved:
case keyword command
routing path_length ? -> 2
routing path_nodes ? -> 3
routing beats path text 1 -> 2
The last is the one that was actually wrong rather than merely unhelpful: with
both present it took the display string over the packet's own path_length.
routing_info is the decoded packet, so it wins, which is what BaseCommand
already did.
All three formatters now call utils.message_hop_count. Behaviour is unchanged
wherever routing_info is absent, so a message carrying only a path string still
parses the same way and "?" still means the count cannot be determined at all.
{firstlast_distance|prefix_if_nonempty: | F/L Dist: } prints "| F/L Dist: N/A"
on a direct message: the distance helpers return the literal "N/A" when there
is no path to measure, prefix_if_nonempty only asks whether the value is
non-empty, and "N/A" is. Suppressing that needed a gate, and pathbytes_min was
the only one available—so it was being used as a stand-in for "did this
message take any hops", which is not what it asks. It asks how the path is
encoded, so pathbytes_min:2 also discards a one-byte multi-hop path whose
distance is real and measurable.
hops_min:N asks about the route itself. hops_min:1 drops a clause on a direct
message and nothing else:
hops_min:1 pathbytes_min:2
direct cleared cleared
1-byte 2 hops kept cleared <- the difference
2-byte 2 hops kept kept
unknown cleared cleared
An unknown hop count clears the value: a gate that cannot confirm the route
should suppress rather than guess, matching pathbytes_min.
The hop-count logic moves to utils.message_hop_count so the filter and
BaseCommand.get_hops_display_values share one implementation instead of two.
It returns None for unknown rather than 0, since a gate must not read
"cannot tell" as "direct".
{path_distance|pathbytes_min:2|prefix_if_nonempty: | Path Dist: } printed
"| Path Dist: N/A" on a direct message. bytes_per_hop_from_routing_and_nodes
returned routing_info's bytes_per_hop before it ever looked at whether there
were any hops, so a 0-hop packet whose format happens to use 2-byte hops
reported 2, the gate passed, and prefix_if_nonempty found "N/A" waiting for
it and dutifully attached the label.
bytes_per_hop describes how a path is encoded. A direct packet has no path
for it to describe, so the format field is not evidence of a wide path and
must not stand in for one. Both docstrings already promised 1 for the direct
case—"Returns 1 when no nodes (direct / unknown)"—so this is the code
catching up to its stated contract rather than a change of policy.
Latent until now: channel messages never had routing_info attached, so the
field was absent and the fallback returned 1 by accident. Direct DMs have
been hitting it all along.
{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.
Adds the 16-char MeshCore packet identity hash to the three formatters that
render user-facing templates: [Keywords] responses, the test command's
response_format, and the path command's reply_prefix. It is read only from
routing_info, which is attached solely from an RF packet correlated to the
message, so a hash from an unrelated transmission is never shown. Missing,
empty, and the 0000000000000000 error sentinel all render empty.
get_packet_hash_placeholder returns "" rather than a False sentinel. Empty
gives byte-identical output on every path—prefix_if_nonempty already tests
falsiness and str.format renders "" as nothing—whereas False would print
the literal string "False" into a mesh reply from any consumer of
get_standard_placeholder_fields that formatted it directly.
TestCommand.format_response now builds on get_standard_placeholder_fields
instead of duplicating it. That also honors the extra argument its docstring
already promised but silently ignored, and picks up the path-string hop
fallback, so {hops} reads "0" rather than "?" for a message whose hops and
routing_info are both unset but whose path says "Direct (0 hops)".
Documents the placeholder in config.ini.example, the default config that
core.py writes on a fresh install, the README and docs/path-command-config.md.
The generated config was also missing {elapsed}; added that too.
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.
Keep the disabled-command fallback in one place. A disabled command still
matches; can_execute is what holds it back, and the dispatcher continues past it
so another command's alias can claim the trigger. The per-command enabled guards
in matches_keyword were a second mechanism for a problem already solved once, and
one every command would have to repeat, so drop them along with PathCommand's
matches_keyword override entirely.
Restore control-character tolerance in the test command. Matching runs against
clean_content-normalized text again, so a garbled "test" still matches. A test is
exactly the message someone sends when their link is marginal. The cleaned text
only sticks when the test command claims the message, since matching runs early
in the command scan and collapsing whitespace on another command's free-form
argument is not its business.
Make enable_p_shortcut = false actually work. Appending "p" to the inherited
class list left it on PathCommand.keywords for every instance built afterwards,
so a reload with the shortcut off still answered "p".
Have split_trigger_and_args honour the configured command prefix instead of
stripping a leading "!" unconditionally, and match word by word rather than
slicing the lowered copy by keyword length, which cuts the args in the wrong
place when str.lower() changes length for a non-ASCII alias.
Mention gating now applies to the test command, which the bespoke matcher had
skipped: "test @[Bob]" in a channel no longer gets an ack.
Added a new method `split_trigger_and_args` to the `BaseCommand` class, which splits message content into a matched keyword and arguments. This enhancement allows for better handling of command triggers and arguments across various command classes, ensuring that multi-word triggers are prioritized and leading characters are stripped appropriately. Updated the `ChannelsCommand`, `DiceCommand`, `HackerCommand`, `MultitestCommand`, `RollCommand`, and `TraceCommand` classes to utilize this new method for cleaner and more consistent command execution logic.
Removed redundant logging for command execution and added detailed debug logging for cases where a command cannot execute due to soft rejections. This allows for better tracking of command flow and ensures that keyword matching continues for subsequent commands. Updated the test_command to include config aliases in keyword matching, enhancing its flexibility.
PUT verified original_schedule outside the lock, so a concurrent delete
between the check and the write resurrected the entry as a new one, and
two concurrent renames of the same original left both results present.
The existence check now runs inside _save_scheduled_message_locked
against the same snapshot the duplicate check uses, so create, update and
delete are each a single critical section.
Added a concurrency test: two simultaneous creates of one schedule now
produce exactly one 200 and one 409 with a single entry on disk.
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.
agessaman