mysticalsoap/aqomui
Watch
1
0
Fork
You've already forked aqomui
 0
Code Issues 36 Pull requests 1 Releases 2 Packages Wiki Activity Actions

one typed tunnel-state object between service and GUI #79

Merged
mysticalsoap merged 1 commit from change/tunnel-state into trunk 2026-08-20 00:37:33 -04:00
Conversation 0 Commits 1 Files changed 6 +211 -108
mysticalsoap commented 2026-08-19 23:49:32 -04:00
Owner
Copy link

Closes #24.

Problem

The tunnel state (device names, DNS servers) lived twice: the service held it as instance attributes set via two thread signals, addressed by string concatenation (getattr(self, "tun{}".format(add))), and the GUI pulled each value back one string at a time with return_tun_device. State in two processes with no reconciliation after a service restart — and the string-concat addressing is how the DNS-to-the-wrong-variable bug happened.

Fix

  • New aqomui/state.py: a frozen TunnelRole (dev/dns/dns_2) per tunnel role (main/hop/bypass), collected in one TunnelState owned by the service.
  • TunnelThread keeps a per-role copy and pushes (role, TunnelRole) over a single role_update signal, replacing the dev + dnsserver tuple signals. Role names replace the add-suffix addressing in mgmt_events/tunnel_up/dns_updown_disabled; suffixes survive only where status names and socket paths need them.
  • The service pushes the whole state as JSON over one D-Bus signal, tunnel_state_changed, and answers get_tunnel_state with a snapshot (used at GUI startup — and the re-sync hook a #73 fix will need). return_tun_device, set_tun and set_dns are deleted.
  • The GUI keeps the latest pushed snapshot and reads self.tunnel_state.<role>.dev where it used to issue pulls.

Related fixes

The refactor removes several string-concat casualties in passing:

  • disconnect("main") contained self.tun is None — a no-op comparison where an assignment was meant. The main role's device reset now actually happens (previously a WireGuard disconnect left the stale device behind forever).
  • Bypass teardown wrote dns{add}_2 → dns_bypass_2, a junk attribute; the real dns_2_bypass kept its stale value.
  • tunnel_up ran the provider's custom "up" script on hop connects too: h != 1 compared a string to an int, so it was always true. Only the main tunnel runs it now. (The same h != 1 pattern still guards the "pre"/"down" scripts in ovpn() — deliberately untouched here.)
  • Dead interface_bypass assignment in cgroup_vpn and the dead connect_status service attribute removed.

Verification

  • pytest: 95 passed — new tests/test_state.py (JSON round trip, role-name validation, frozen roles), test_tunnel.py updated to the role-based API.
  • ruff check --select E9,F63,F7,F82 and compileall -W error::SyntaxWarning clean.
  • Needs live verification before merge (per process rule): on the installed build, connect main + bypass — active-connection widget shows the right devices, bypass egress IP still correct, disconnect/teardown resets the state. The wg_aqomui path and double-hop display are the least-covered spots.

🤖 Generated with Claude Code

Closes #24. ## Problem The tunnel state (device names, DNS servers) lived twice: the service held it as instance attributes set via two thread signals, addressed by string concatenation (`getattr(self, "tun{}".format(add))`), and the GUI pulled each value back one string at a time with `return_tun_device`. State in two processes with no reconciliation after a service restart — and the string-concat addressing is how the DNS-to-the-wrong-variable bug happened. ## Fix - New `aqomui/state.py`: a frozen `TunnelRole` (dev/dns/dns_2) per tunnel role (main/hop/bypass), collected in one `TunnelState` owned by the service. - `TunnelThread` keeps a per-role copy and pushes `(role, TunnelRole)` over a single `role_update` signal, replacing the `dev` + `dnsserver` tuple signals. Role names replace the `add`-suffix addressing in `mgmt_events`/`tunnel_up`/`dns_updown_disabled`; suffixes survive only where status names and socket paths need them. - The service pushes the whole state as JSON over one D-Bus signal, `tunnel_state_changed`, and answers `get_tunnel_state` with a snapshot (used at GUI startup — and the re-sync hook a #73 fix will need). `return_tun_device`, `set_tun` and `set_dns` are deleted. - The GUI keeps the latest pushed snapshot and reads `self.tunnel_state.<role>.dev` where it used to issue pulls. ## Related fixes The refactor removes several string-concat casualties in passing: - `disconnect("main")` contained `self.tun is None` — a no-op comparison where an assignment was meant. The main role's device reset now actually happens (previously a WireGuard disconnect left the stale device behind forever). - Bypass teardown wrote `dns{add}_2` → `dns_bypass_2`, a junk attribute; the real `dns_2_bypass` kept its stale value. - `tunnel_up` ran the provider's custom "up" script on hop connects too: `h != 1` compared a string to an int, so it was always true. Only the main tunnel runs it now. (The same `h != 1` pattern still guards the "pre"/"down" scripts in `ovpn()` — deliberately untouched here.) - Dead `interface_bypass` assignment in `cgroup_vpn` and the dead `connect_status` service attribute removed. ## Verification - pytest: 95 passed — new `tests/test_state.py` (JSON round trip, role-name validation, frozen roles), `test_tunnel.py` updated to the role-based API. - `ruff check --select E9,F63,F7,F82` and `compileall -W error::SyntaxWarning` clean. - **Needs live verification before merge** (per process rule): on the installed build, connect main + bypass — active-connection widget shows the right devices, bypass egress IP still correct, disconnect/teardown resets the state. The `wg_aqomui` path and double-hop display are the least-covered spots. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
change: one typed tunnel-state object owned by the service, pushed over a single signal
All checks were successful
ci / test (pull_request) Successful in 28s
Details
9ff0dddb16
 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mysticalsoap force-pushed change/tunnel-state from 9ff0dddb16
All checks were successful
ci / test (pull_request) Successful in 28s
Details
to da4f67c373
Some checks failed
ci / test (pull_request) Successful in 31s
Details
ci / test (push) Has been cancelled
Details
2026-08-20 00:20:31 -04:00 Compare
mysticalsoap deleted branch change/tunnel-state 2026-08-20 00:37:33 -04:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
architecture
Structural change, not a local fix

  
bug
Defect in existing behaviour

  
cleanup
Tidying with no behaviour change

  
docs
Documentation and repo standards

  
packaging
distro packages and their build tooling

  
security
Vulnerability or hardening

  
testing
Test coverage or infrastructure

  
upstream
Needs action on someone else's project

  
ux
Interface behaviour and clarity

No labels
architecture
bug
cleanup
docs
packaging
security
testing
upstream
ux
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
mysticalsoap/aqomui!79
No description provided.