extract the D-Bus service client from the gui #84

Merged
mysticalsoap merged 2 commits from change/service-client into trunk 2026-08-20 11:43:31 -04:00
Owner

First seam of #83.

Problem

AqomuiGui owns its D-Bus transport inline: proxy creation, signal subscriptions, dbus_call's restart-recovery machinery, and a getattr(self, reply)() status dispatch that aborts the GUI from inside a Qt slot if the service ever emits a status without a handler (the "fail" status did exactly that once).

Fix

  • New aqomui/service_client.py: ServiceClient owns the bus proxy (follow_name_owner_changes), idempotent signal subscription from a {signal: handler} mapping, call() with the one-silent-recovery-then-ask flow, and the owner-wait poll. UI policy stays in the GUI and arrives as callbacks: notify, on_recovered (re-register bypass net info, re-sync tunnel state), on_unavailable (the Quit/Restart dialog).
  • The GUI shrinks accordingly: dbus_call/wait_for_service_owner/create_dbus_object are gone, initialize_service no longer rebuilds a proxy (the unit is Type=dbus, so systemctl returns with the name owned, and the proxy follows the name anyway), and all 26 call sites go through self.service.call.
  • openvpn_log_monitor becomes dispatch_status with an explicit STATUSES whitelist (the 4 statuses × 3 role suffixes the service emits); an unknown status is logged instead of dispatched blind.

aqomui_cli.py still builds its own proxy — it can adopt ServiceClient when it gets attention (its -c is broken independently).

Verification

  • pytest: 126 passed — the #80 recovery tests moved to tests/test_service_client.py against the real client (with a fake clock, so the owner-wait poll and give-up paths are now tested for real instead of stubbed); new GUI tests cover known/unknown status dispatch and assert every whitelisted status has a handler.
  • ruff check --select E9,F63,F7,F82 and compileall -W error::SyntaxWarning clean.
  • Needs live verification before merge: normal GUI session (connect, disconnect, logs visible once), plus the #73 scenario again — systemctl restart aqomui under a running GUI recovers silently — since that flow now runs through the extracted client.

Regression fix (second commit): bypass died after a service restart

Live testing #73's merge found a gap in it: with the proxy following the name, calls against a restarted service just succeed — so the recovery hook that re-registers the bypass network info never fires (before #80, the dialog's Restart path did this). The fresh service then has no net, the next bypass-tunnel teardown logs "no network info registered - skipping bypass rebuild", and table 11 is left without a default route: bypass works at app start and while a bypass tunnel is up, and dies the moment it disconnects.

Fix: the client watches NameOwnerChanged (issue #73's original suggestion) — a new unique owner behind the well-known name means a service with a clean slate, so on_recovered runs proactively even though no call failed. The in-call recovery path records the new owner first so the queued watch notification doesn't resync twice.

Live verification for this now includes: restart the service under the running GUI, then connect and disconnect a bypass server — bypassed apps must still reach the internet through the physical link afterwards (ip route show table 11 keeps a default route).

🤖 Generated with Claude Code

First seam of #83. ## Problem `AqomuiGui` owns its D-Bus transport inline: proxy creation, signal subscriptions, `dbus_call`'s restart-recovery machinery, and a `getattr(self, reply)()` status dispatch that aborts the GUI from inside a Qt slot if the service ever emits a status without a handler (the `"fail"` status did exactly that once). ## Fix - New `aqomui/service_client.py`: `ServiceClient` owns the bus proxy (`follow_name_owner_changes`), idempotent signal subscription from a `{signal: handler}` mapping, `call()` with the one-silent-recovery-then-ask flow, and the owner-wait poll. UI policy stays in the GUI and arrives as callbacks: `notify`, `on_recovered` (re-register bypass net info, re-sync tunnel state), `on_unavailable` (the Quit/Restart dialog). - The GUI shrinks accordingly: `dbus_call`/`wait_for_service_owner`/`create_dbus_object` are gone, `initialize_service` no longer rebuilds a proxy (the unit is `Type=dbus`, so systemctl returns with the name owned, and the proxy follows the name anyway), and all 26 call sites go through `self.service.call`. - `openvpn_log_monitor` becomes `dispatch_status` with an explicit `STATUSES` whitelist (the 4 statuses × 3 role suffixes the service emits); an unknown status is logged instead of dispatched blind. `aqomui_cli.py` still builds its own proxy — it can adopt `ServiceClient` when it gets attention (its `-c` is broken independently). ## Verification - pytest: 126 passed — the #80 recovery tests moved to `tests/test_service_client.py` against the real client (with a fake clock, so the owner-wait poll and give-up paths are now tested for real instead of stubbed); new GUI tests cover known/unknown status dispatch and assert every whitelisted status has a handler. - `ruff check --select E9,F63,F7,F82` and `compileall -W error::SyntaxWarning` clean. - **Needs live verification before merge**: normal GUI session (connect, disconnect, logs visible once), plus the #73 scenario again — `systemctl restart aqomui` under a running GUI recovers silently — since that flow now runs through the extracted client. ## Regression fix (second commit): bypass died after a service restart Live testing #73's merge found a gap in it: with the proxy following the name, calls against a restarted service just succeed — so the recovery hook that re-registers the bypass network info never fires (before #80, the dialog's Restart path did this). The fresh service then has no `net`, the next bypass-tunnel teardown logs "no network info registered - skipping bypass rebuild", and table 11 is left without a default route: bypass works at app start and while a bypass tunnel is up, and dies the moment it disconnects. Fix: the client watches `NameOwnerChanged` (issue #73's original suggestion) — a new unique owner behind the well-known name means a service with a clean slate, so `on_recovered` runs proactively even though no call failed. The in-call recovery path records the new owner first so the queued watch notification doesn't resync twice. Live verification for this now includes: restart the service under the running GUI, then connect **and disconnect** a bypass server — bypassed apps must still reach the internet through the physical link afterwards (`ip route show table 11` keeps a default route). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
change: extract the D-Bus service client from the gui
All checks were successful
ci / test (pull_request) Successful in 35s
91f118f515
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fix: resync gui-registered state when the service's bus owner changes
All checks were successful
ci / test (pull_request) Successful in 30s
ci / test (push) Successful in 29s
d0a647a639
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mysticalsoap deleted branch change/service-client 2026-08-20 11:43:31 -04:00
Sign in to join this conversation.
No description provided.