`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>
`RepeaterDaemon._shutdown` arms an exit watchdog -- a `threading.Timer` that
calls `os._exit(0)` after `SHUTDOWN_EXIT_GRACE_S` (5 s). That is right in
production: it guarantees SIGTERM is honoured.
`test_run_starts_http_and_handles_dispatcher_cancelled_gracefully` awaits
`daemon.run()`, whose `finally` reaches `_shutdown()`, so it armed a real
watchdog and returned. Five seconds later the timer fired and took the whole
pytest process down.
The failure mode is what makes this expensive: `os._exit(0)` exits with **status
zero**. pytest produces no summary line and a truncated progress bar, so the run
looks like it simply ended -- CI reads exit 0 as green on a run that never
finished, and any failure after that point is invisible. Locally the suite
appeared to hang at ~77% with no failures reported; it was actually being killed
there.
It also misattributes itself. The test blamed is whichever happened to be
running five seconds later, so it moves between runs
(`test_route_login_server_companion_only_skips_login_helper` and
`test_route_login_server_hash_collision_offers_to_both` on two runs here) and
never points at the cause. Finding it needed wrapping `_arm_exit_watchdog` to
log the arming test.
Two changes:
- Patch `_arm_exit_watchdog` for that test, matching the guard the shutdown
tests in `test_main_py_coverage.py` already use.
- Add `tests/conftest.py` with an autouse fixture that fails any test leaving a
live `shutdown-watchdog` timer, and cancels it. This blames the responsible
test at the moment it leaks, rather than an unrelated one later. Verified it
catches the original bug: reverting the one-line fix turns the silent
process-kill into a teardown error naming the right test.
`test_exit_watchdog_is_a_daemon_timer_that_forces_exit` is unaffected -- it sets
a 0.05 s grace and asserts on a patched `os._exit` before returning, so it
leaves nothing behind.
With this the suite runs to completion: 3 failed, 1580 passed. Those 3 are
pre-existing on `dev` and unrelated.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
