The companion frame proxy only accepted an operator JWT in `?token=`,
while `/ws/packets` also accepts a long-lived API token (X-API-Key
header, or the same query parameter). Apply the identical rule to the
frame proxy so a client that already holds a repeater API key can open
the companion link on the HTTP port without keeping a JWT session alive.
No behaviour change for JWT clients. Same fallback order, close codes
and log lines as PacketWebSocket.
After mesh.default_region was wired to the core dispatcher's send-time
scope resolver, a relayed plain flood was indistinguishable from a
locally originated one, so Dispatcher._apply_flood_scope() converted it
to the repeater's default transport scope and sent it out-of-region.
Successful flood forwards now mark their existing scope decision as
authoritative before entering Dispatcher.send_packet(): relayed plain
floods stay plain and relayed transport floods retain the originator's
codes. Locally originated floods and REPLY_SCOPE_DEFAULT fallback
replies still pick up the configured default scope.
Marking only on the flood_forward() success path leaves packets rejected
by validation, policy, duplicate, or loop detection untouched, and
leaves locally injected transmissions (which bypass forwarding) alone.
The SSE endpoints registered their bridge callbacks once behind a
_callbacks_registered latch. Anything that cleared the bridge's push
callbacks therefore killed the stream permanently — the latch stayed True
while the subscriptions were gone, so no later stream open repaired them.
Cache the callback objects instead and re-register them each time a stream
opens. Bridge registration is idempotent, so the repeat costs nothing while
a subscription that went missing is repaired rather than leaving the stream
silently dead until the daemon restarts.
The clear that made this reachable is fixed in openhop_core, where the
frame server now retracts only its own subscriptions instead of clearing
every callback on each companion client connection. This half is the
belt-and-braces: clear_push_callbacks() remains a legitimate full reset,
and the stream should survive one.
Firmware's ``simple_repeater`` resolves a ``default_scope`` TransportKey at boot
from ``region_map.getDefaultRegion()`` and answers the ``REPLY_SCOPE_DEFAULT``
row with ``sendFloodScoped(default_scope, ...)``. The comment on that branch is
the whole point of it::
// un-scoped would be dropped at hop 0 by repeaters running flood.max.unscoped=0
We had the config for it -- ``mesh.default_region``, which the web API already
auto-creates as an allow-flood region -- but nothing carried it to
``Dispatcher.default_flood_transport_key``, so it only ever reached adverts and
the neighbours publisher. Core's send-time resolver therefore found no default
and left the reply a plain flood.
The row is reached whenever the request's scope is *unknowable* rather than
un-scoped: a DIRECT request we hold no return path for, or a TRANSPORT_FLOOD
whose code matched no served region. Both are ordinary traffic, and on a mesh
running ``flood.max.unscoped = 0`` both replies died at hop 0 while firmware
would have delivered them.
``resolve_default_scope_key`` resolves through the built RegionMap rather than
hashing the configured name, for parity with ``getDefaultRegion()``, which can
only return a region the map holds: a ``$private`` default gets its stored key
material, and a default naming an unserved region resolves to nothing rather
than to a scope no local Region would match on the way back in. An unset default
and the ``*`` wildcard resolve to None, which is firmware's
``default_scope.isNull()`` row -- a plain flood.
Re-resolved from the three places that change the answer: boot, the
transport_keys change hook, and a runtime ``mesh.default_region`` edit. That last
one matters because the web API creates the region *before* writing the config,
so the storage hook has already run against the previous value.
Companion bridges need no separate wiring: a deferred reply from a bridge's
login server reaches the shared dispatcher unmarked, so it resolves here, which
is what firmware ``simple_room_server`` does with its own ``default_scope``.
The two end-to-end tests skip on a core that still resolves REPLY_SCOPE_DEFAULT
at RX and marks the reply final, since the assignment is correct but inert
there. They are gated on the behaviour rather than on a version, so they start
running by themselves once that core lands.
A local transmit retried after a flat 1s backoff. When the failure was the
link itself -- a KISS modem that dropped off USB and is re-enumerating, which
takes about 2s to come back and re-handshake -- that retry landed inside the
outage and failed immediately, turning one modem hiccup into a failed message
even though the companion client was still waiting.
A transient failure with the link up keeps its 1s backoff outside _tx_lock. A
dropped link is now waited for instead, polled at 100ms and bounded by
delays.local_tx_link_wait_seconds: default 5s, non-finite values rejected and
everything capped at 30s, because the client's own command timeout is already
part-spent on the TX delay by the time we get here. Relayed packets never
wait -- a repeat held back that long is worse than one dropped.
Link state is read from the radio the send will actually use. Attribute
access on a FabricRadio passes through to its default endpoint, so a send
carrying preferred_tx_radio_id would otherwise be judged by the wrong radio,
suppressing a healthy one or waving through a dead one. A multi-radio fabric
with no explicit target picks its endpoint at send time, so nothing is
inspected and the send proceeds as before. Radios that track no link state
(a directly driven SX1262) answer nothing and keep their exact timing.
pushPostToClient sizes the body with strlen(post.text) for the payload and
the expected ACK alike. The web API will store "a\0b", and pushing it whole
sends text the receiver cannot see -- it stops at the NUL -- while hashing
the pending ACK over a span the receiver never reproduces. The push would
then never be acknowledged and would retry into the failure backoff.
Same defect the core builder had; this is a second, hand-rolled builder.
Also proves MC-R4 end to end at last: a genuinely encrypted guest PLAIN
through a real registered room identity, watching the injector for an ACK
that must not appear. The seam was only covered in three separate halves
before -- core honours a veto, _may_ack computes one, a mocked constructor
receives one -- none of which would have caught the wiring being dropped.
The core text handler answers by BaseChatMesh's rule -- plain and signed
text, never a CLI type -- and it runs before this helper's gates. So a
signed post, a non-admin command or a guest's post was acknowledged and
only then thrown away: the sender saw delivery confirmed for a message
that went nowhere.
_may_ack is firmware's rule for a server, handed to the core handler as
should_ack_fn at register_identity. simple_repeater builds an ACK only
under `if (flags == TXT_TYPE_PLAIN)`, inside a branch already gated on
isAdmin(); simple_room_server sets send_ack only on the non-guest PLAIN
path. Neither answers a CLI type, an unsupported type or a replayed
timestamp -- but both still answer a retry, because firmware suppresses
the work, not the reply. It reads no state it does not also leave alone.
It predicts eligibility, not the outcome: a post that clears the role and
replay checks and is then refused by openHop's own rate limit or a
database error is still ACKed. Those refusals have no firmware
counterpart, so closing that gap needs the store result fed back here.
Guarded by a signature check: passing should_ack_fn to a core that does
not take it raises TypeError out of register_identity, which would leave
the node with no text handler at all. pyproject.toml tracks
openhop_core@dev with no version floor, so that is an ordinary state --
and in it the ACK goes back to being sent, availability over the
invariant.
Two things firmware does that openHop did not.
Dispatch by type, not by text. simple_room_server::onPeerDataRecv routes
CLI_DATA/CLI_COMMAND to the admin CLI and turns PLAIN into a post;
simple_repeater has no chat function, so it runs every accepted type
through handleCommand with no text test at all. openHop asked
_is_cli_command(message_text) for both, and so got both wrong: a PLAIN
"set ..." ran as a command instead of being posted, a command whose text
was not in the prefix list was published to the room, and an admin's
mistyped repeater command vanished silently instead of being answered.
Guard replays. Both servers open the branch with
`sender_timestamp >= client->last_timestamp`, treat equality as a retry
they accept but do not re-run, and drop anything older. openHop checked
neither, so replaying an admin command with the same timestamp and
different attempt bits re-executed it -- packet dedup does not catch that
because the bytes differ. Note ACL._is_replay is not reusable here: its
strict <= rejects the equal case this branch accepts.
Both read from packet.decrypted, which openhop_core now publishes. A core
too old to report them falls back to the previous text-prefix behaviour
and logs a warning rather than dropping every message.
simple_room_server::onPeerDataRecv takes the
`(client->permissions & PERM_ACL_ROLE_MASK) == PERM_ACL_GUEST` branch for a
PLAIN message and stores nothing. openHop called add_post for anyone who
authenticated, so knowing the guest password was enough to write to a room:
add_post caps length and rate-limits per client, but never looks at the role.
Also warns once per message when openhop_core does not publish txt_type,
so the open text-type gate is visible in the log rather than silent.
simple_repeater::onPeerDataRecv and simple_room_server::onPeerDataRecv
both open their TXT_MSG branch by rejecting any flags outside
{PLAIN, CLI_DATA, CLI_COMMAND}. openHop had no such gate: it decided what
a message was purely from whether the text looked like a command, so a
SIGNED_PLAIN -- a room post, whose 4-byte author prefix the core handler
has already stripped, leaving bare text -- ran as an admin CLI command
whenever it happened to start with a command prefix, and was stored as a
post on a room server that firmware would have dropped.
CLI_COMMAND (3) joined the accepted set when firmware split "a CLI
command" out of CLI_DATA (MeshCore 2c0ace25); CLI_DATA stays in it
because that is what every released client still sends.
Reads the type from packet.decrypted, which the core text handler now
publishes. A core too old to report it leaves the gate open rather than
dropping every message.
`pre-commit run --all-files` fails on `dev` at the `ruff-format` hook, so every
open pull request is red on a check that has nothing to do with its own diff.
Both files arrived with 72da7ee (pr-436) formatted by an earlier ruff.
Purely cosmetic, and exactly what `ruff format` produces:
- `sqlite_handler.py` — a generator expression and an f-string concatenation
that both now fit inside the line limit unsplit.
- `test_companion_import_repeater_contacts.py` — the blank line ruff requires
after a module docstring.
- Introduced tests for the plugin catalogue installation and update flows in `test_plugin_catalogue_install.py`.
- Added unit tests for the GitHub Releases plugin client in `test_plugin_github_releases.py`.
- Implemented IPC tests for the plugin manager Unix socket protocol in `test_plugin_ipc.py`.
- Created lifecycle and installation tests for the plugin manager in `test_plugin_manager_lifecycle.py`.
- Added tests for plugin manifest parsing in `test_plugin_manifest.py`.
- Developed tests for plugin filesystem layout and state management in `test_plugin_storage.py`.
"Import repeater contacts" wrote nothing whenever a contact-type filter was
supplied, and reported success having imported zero rows.
`adverts.contact_type` holds the *display* name written through
`handler_helpers.discovery.NODE_TYPE_NAMES` — "Chat Node", "Repeater",
"Room Server", "Sensor". The import API accepts MeshCore's names — "companion",
"repeater", "room_server", "sensor", validated in
`companion_endpoints.import_repeater_contacts`. `companion_import_repeater_contacts`
compared them directly:
query += f" AND contact_type IN ({placeholders})"
so "repeater" never matched "Repeater", "room_server" never matched
"Room Server", and "companion" never matched "Chat Node" at all. On a live
repeater with 285 adverts, every one of the four permitted values selected 0
rows, as did all four together; only an unfiltered import worked.
A second defect had the same cause. The adv_type lookup normalised the stored
name (`lower()`, spaces to underscores) but mapped it with the API-name table,
where "chat_node" is absent — so chat contacts imported as adv_type 0 rather
than 1, even on an unfiltered import. That is silently wrong rather than empty,
and needs a manual `UPDATE companion_contacts SET adv_type=1 WHERE adv_type=0`
to repair.
Both sites derived the mapping independently, which is how they drifted. They
now share one `_ADVERT_TYPE_BY_STORED` table keyed on the normalised stored
form: the filter reverse-maps the requested API names onto the stored keys and
compares `LOWER(REPLACE(TRIM(contact_type), ' ', '_'))`, and the adv_type comes
from the same table. Filtering and limiting stay in SQL. An unrecognised type
still selects nothing rather than silently widening the query.
Tested against the real stored forms: 6 of the 8 new tests fail on current dev
(each of the four type filters, all four combined, and the adv_type mapping)
and all 8 pass with this change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`SQLiteHandler.__init__` runs `_init_database()` and then `_run_migrations()`.
`_init_database` created `idx_packets_upstream_time`, which spans
`upstream_hash` and `upstream_hash_size` -- columns that, on a database
predating migration 13, only appear once `add_upstream_hash_to_packets` has
added them in the *later* migration step.
Fresh installs were fine, because the `CREATE TABLE` carries both columns. Any
existing database raised `no such column: upstream_hash` part-way through
`_init_database`, which caught and logged it. Every statement after the failing
index was therefore skipped -- including the `room_messages` and
`room_client_sync` tables -- and the daemon started with a partially-created
schema while reporting itself healthy: service active, radio initialised,
companions serving, HTTP up, and no packets recorded.
The `add_upstream_hash_to_packets` migration already creates this index, right
after adding the columns it depends on, so the copy in `_init_database` was
redundant as well as too early. Remove it and leave the index to the migration
that owns those columns. All three paths still end up with it: a fresh database
gets the columns from `CREATE TABLE` and the index when the migration runs, an
un-migrated database gets both from the migration, and an already-migrated one
kept the index from its earlier run.
Observed on a Raspberry Pi 4 upgrading a ~13 MB production database from 1.1.1;
the repeater was off-air for four minutes before the cause was found. It
self-heals on a second start, because the migration still runs after the failed
init, which makes it easy to misread as harmless.
`tests/test_sqlite_upgrade_existing_db.py` builds a database with the
pre-migration-13 schema and asserts `_init_database` runs to completion, keyed
on tables created after the old failure point. Asserting only on the upstream
columns is not sufficient -- the migration adds those either way.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Treehouse-00