feat(#149): Settings cancel button + IPC-level abort signal for DNS enable/disable by flyhigher139 · Pull Request #151 · igevin/mHost

flyhigher139 · 2026-08-07T08:48:39Z

Closes #149

Background

Epic #147 (DNS enable state desync) closed #148 (orphan proxy) and #140 (proxy bind race), but the user still had no way to actively cancel a hung enable — the osascript sudo prompt is system-modal and Tauri's IPC is stuck inside await spawn_blocking. This PR exposes a Settings Cancel button that drives a backend CancellationToken through every phase of set_dns_mode.

Changes

Backend (Rust)

mhost-core: new MhostError::Cancelled variant (serializes as { Cancelled: null }). The frontend's AbortError path treats this as a non-error; the catch block clears dnsErrorAtom and refetches backend truth to mirror the rolled-back state.
AppState: new dns_cancel: Mutex<Option<CancellationToken>> slot. Same swap-not-mutate pattern as ad_block_refresh_cancel (issue DNS refresh task: spawn_blocking closure is not cancellable, races with disable #138 follow-up) so a disable → re-enable cycle doesn't leak a cancelled token into the new operation.
commands::dns::set_dns_mode allocates a fresh CancellationToken, wraps the work future in tokio::select!, and clears the slot on completion (success or err).
New IPC command cancel_dns_mode — fires whatever token is in the slot. No-op when no operation is in flight.
set_dns_mode_enable checks cancel.is_cancelled() at three phase boundaries:
- Phase 1 (post server.start, pre-osascript): stop server only — no system DNS side effects yet.
- Phase 2 (post-osascript): stop server + call disable_dns_mode(cancel=None) to roll back the system DNS rewrite (proxy self-cleanup via signal-file, with osascript sudo fallback).
- Phase 3 (post-manifest-save): full set_dns_mode_disable rollback (clears in-memory state too).
set_dns_mode_disable takes cancel: Option<&CancellationToken>. The 5s proxy-exit wait loop checks cancel.is_cancelled() after every 100ms sleep tick and bails with Ok(()) on cancel.
platform::disable_dns_mode signature gained the same cancel param.

Frontend

src/lib/tauri.ts: setDnsMode accepts options.signal for forward compatibility. New cancelDnsMode() IPC wrapper.
src/stores/profiles/actions.ts: toggleDnsModeAtom creates an AbortController, registers an abort listener that flips a local cancelled flag and fires cancelDnsMode, and treats the eventual IPC rejection as a user cancel. New module-level helper cancelActiveDnsToggle.
src/pages/Settings.tsx: while isDnsLoading is true, the primary Enable/Disable button is replaced with a red Cancel button (data-testid="dns-cancel-button").

Acceptance criteria

dev mode enable → click Cancel → UI immediately reverts + proxy is killed (proxy trap from fix(dns): enable_dns_mode must kill orphan proxy via sudo + add AppleScript trap #148 + cancel triggers disable rollback)
dev mode enable → server.start deliberately blocked → click Cancel → IPC returns immediately + server.stop() runs
dev mode disable → click Cancel mid-disable → no error, proxy self-cleans
Frontend AbortError handled by toggleDnsModeAtom (no error toast)
Regression: regular enable / disable flow unaffected (135 mhost-lib + 110 mhost-dns + 313 frontend tests all pass)
cargo clippy --workspace -- -D warnings clean
pnpm test + cargo test --workspace --all-features clean

Tests added

Backend (commands::dns::tests):

test_cancel_dns_mode_fires_slot_token
test_cancel_dns_mode_noop_when_slot_empty
test_set_dns_mode_swap_cancellation_token_is_fresh (regression for issue DNS refresh task: spawn_blocking closure is not cancellable, races with disable #138 follow-up)

Backend (platform::tests):

test_disable_dns_mode_cancellable_bails_on_pre_cancelled_token
test_disable_dns_mode_cancellable_none_does_not_bail

Frontend (src/stores/__tests__/dns.test.ts):

cancelActiveDnsToggle fires cancelDnsMode IPC and aborts the controller
toggleDnsModeAtom does NOT throw when cancelled mid-flight
cancelActiveDnsToggle is a no-op when no toggle is in flight
Real backend error path: dnsErrorAtom is set and atom throws

Verification

cargo fmt --check: clean
cargo clippy --workspace --all-targets -- -D warnings: clean
cargo test --all-features --workspace: 463 passed, 0 failed
pnpm test: 313 passed, 0 failed
pnpm build: clean

🤖 Generated with Claude Code

…nable/disable Background ========== Epic #147 (DNS enable state desync) closed #148 (orphan proxy) and #140 (proxy bind race), but the user still had no way to actively cancel a hung enable — the osascript sudo prompt is system-modal and Tauri's IPC is stuck inside `await spawn_blocking`. This PR exposes a Settings Cancel button that drives a backend CancellationToken through every phase of set_dns_mode. Changes ======= Backend (Rust) - mhost-core: new `MhostError::Cancelled` variant (serializes as `{ Cancelled: null }`). The frontend's AbortError path treats this as a non-error; the catch block clears `dnsErrorAtom` and refetches backend truth to mirror the rolled-back state. - AppState: new `dns_cancel: Mutex<Option<CancellationToken>>` slot. Same swap-not-mutate pattern as `ad_block_refresh_cancel` (issue #138) so a disable -> re-enable cycle doesn't leak a cancelled token into the new operation. - commands::dns::set_dns_mode now allocates a fresh CancellationToken, wraps the work future in `tokio::select!`, and clears the slot on completion (success or err). - New IPC command `cancel_dns_mode` — fires whatever token is in the slot. No-op when no operation is in flight; safe to call from Settings' signal-abort handler. - set_dns_mode_enable checks `cancel.is_cancelled()` at three phase boundaries (after server.start, after spawn_blocking for osascript, after manifest save) and runs the appropriate rollback: * Phase 1 (post server.start, pre-osascript): stop server only — no system DNS side effects yet. * Phase 2 (post-osascript): stop server + call disable_dns_mode with cancel=None to roll back the system DNS rewrite (proxy self-cleanup via signal-file, with osascript sudo fallback). * Phase 3 (post-manifest-save): full set_dns_mode_disable rollback (clears in-memory state too). - set_dns_mode_disable takes `cancel: Option<&CancellationToken>`. The 5s proxy-exit wait loop checks `cancel.is_cancelled()` after every 100ms sleep tick and bails with Ok(()) on cancel; proxy self-cleanup continues in the background and the recovery marker preserves worst-case recovery for next launch. - platform::disable_dns_mode signature gained the same cancel param. Rollback/cleanup callers pass None (must not be interrupted); the user-initiated disable path passes Some. Frontend - src/lib/tauri.ts: setDnsMode accepts `options.signal` for forward compatibility (Tauri 2's `invoke` doesn't natively propagate signal to the backend, so cancellation is tracked locally and the separate cancelDnsMode IPC fires the backend token). - New cancelDnsMode() IPC wrapper. - src/stores/profiles/actions.ts: toggleDnsModeAtom now creates an AbortController, registers an abort listener that flips a local `cancelled` flag and fires cancelDnsMode, and treats the eventual IPC rejection as a user cancel (clears dnsErrorAtom, refetches truth, does not throw). A new module-level helper `cancelActiveDnsToggle` is exposed for the Settings button. - src/pages/Settings.tsx: while `isDnsLoading` is true, the primary Enable/Disable button is replaced with a red Cancel button that calls cancelActiveDnsToggle. data-testid="dns-cancel-button" added for testability. Tests ===== Backend (commands::dns): - test_cancel_dns_mode_fires_slot_token - test_cancel_dns_mode_noop_when_slot_empty - test_set_dns_mode_swap_cancellation_token_is_fresh (regression for issue #138 follow-up — fresh token on every operation) Backend (platform::disable_dns_mode): - test_disable_dns_mode_cancellable_bails_on_pre_cancelled_token - test_disable_dns_mode_cancellable_none_does_not_bail Frontend (src/stores/__tests__/dns.test.ts): - cancelActiveDnsToggle fires cancelDnsMode IPC and aborts the controller - toggleDnsModeAtom does NOT throw when cancelled mid-flight - cancelActiveDnsToggle is a no-op when no toggle is in flight - real backend error: dnsErrorAtom is set and atom throws Verification ============ - cargo fmt --check: clean - cargo clippy --workspace --all-targets -- -D warnings: clean - cargo test --all-features --workspace: 463 passed, 0 failed - pnpm test: 313 passed, 0 failed - pnpm build: clean Closes #149

… DNS enable hang) Root cause: build_enable_script_body backgrounded mhost-dns-proxy with '& disown' but did NOT redirect stdin/stdout/stderr. Because invoke_osascript uses Command::output(), osascript's captured pipes were inherited by the long-lived proxy. disown only removes the job from the shell's job table — it does NOT close FDs — so Command::output() on the Rust side never observed EOF. This matches the reported symptom exactly: no TCC password dialog visible, no error returned to the frontend, UI stuck on 'Loading' forever. PR #146 (a2fe6e4) had removed the v0.3.3 60 s tokio::time::timeout safety net; without any recovery path the user had no way out except force-quit. Fix: 1. platform.rs (build_enable_script_body): redirect </dev/null >/dev/null 2>&1 BEFORE the '&' that backgrounds the proxy. Order matters: '&' must come last or the redirects are ignored. Pinned by the new test_enable_script_redirects_backgrounded_proxy_fds. 2. platform.rs (new helpers): OsascriptRun + spawn_osascript + kill_osascript + run_with_privileges_timeout(Duration::from_secs(60)). Synchronous try_wait loop with SIGKILL on expiry. Switches the single call site in enable_dns_mode from run_with_privileges to run_with_privileges_timeout(60s). This restores the v0.3.3 60 s safety net without the JoinHandle-leak problem that motivated PR #149's removal — we hold the Child directly and SIGKILL on expiry. 3. platform.rs (enable_dns_mode): two tracing::info! breadcrumbs around the osascript call so the next time this hangs, logs show exactly where it stopped. 4. commands/dns.rs (set_dns_mode_enable): capture_dns_state moved into spawn_blocking; get_upstream_resolvers wrapped in tokio::time::timeout(10s) that falls back to tier3_fallback() on expiry. Defends the pre-prompt phase against wedged configd/scutil (same symptom, different root cause). 5. platform.rs (tests): test_enable_script_redirects_backgrounded_proxy_fds pins the redirect pattern AND its ordering. Manual verification on macOS (in plan): - Cause A (root): TCC prompt appears within 1-3 s of clicking Enable. - Cause C (timeout): with a temporary sleep(70) in enable_dns_mode, UI receives 'osascript timed out after 60s (killed pid=...)' within ~60 s and no osascript lingers. - Cause B (pre-prompt): with a temporary sleep(15) in capture_dns_state on a DHCP-empty system, after ~10 s a tracing::warn! fires and enable continues with public-DNS fallback. Verified: - cargo fmt --all -- --check: clean - cargo clippy --workspace --all-targets -- -D warnings: clean - cargo test --workspace: green - pnpm test: 313/313 passed Follow-up (out of scope, to file separately): impl Drop for DnsServer would let us safely re-introduce tokio::select! for instant cancel during the spawn_blocking phase. References dns.rs:84 and the test_set_dns_mode_no_outer_tokio_select_race_after_cancel_leak_fix sentinel. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…s lazy sh exec) Found via MHOST_AUTO_DNS=1 end-to-end test on macOS: prior run_with_privileges_timeout removed the temp script file immediately after spawn_osascript returned, racing against osascript's lazy 'sh <path>' exec. osascript then returned exit 256 with stderr: 'sh: /var/folders/.../mhost-dns-37800-0.sh: No such file or directory (127)'. This was the actual cause of the user-reported 'no prompt, no error, UI stuck forever' symptom. The 60s timeout killed the osascript process correctly, but the underlying bug (the file got deleted before sh could exec it) had already triggered an earlier silent failure on every call. Two extra early breadcrumbs in enable_dns_mode were added during diagnosis and are kept as permanent 0-cost diagnostic anchors: - enable_dns_mode: entered (dns_port=...) - enable_dns_mode: get_active_network_interface returned: <iface> Fix: defer std::fs::remove_file(&path) until AFTER osascript has definitively exited (Ok branch + timeout branch both funnel through the same 'outcome' variable, then we clean up). On every exit path the script file is removed exactly once. Verified via MHOST_AUTO_DNS=1 headless test: osascript now waits the full 60s timeout (no silent ENOENT), then returns the clean 'osascript timed out after 60s (killed pid=...)' error to the frontend instead of silently failing. cargo fmt / clippy -D warnings / 466 tests / pnpm test 313/313: all clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…from capture (fixes #152) Two related but independent bugs in the DNS mode that combine to leave the user's system DNS stuck at 127.0.0.1 after a partial-fail disable. Root cause 1 — try_recover_dns reads the WRONG path for the recovery marker - `disable_dns_mode` writes the marker to `runtime_dir()/mhost-dns-disable-recovery.marker` - `try_recover_dns` (state/mod.rs:267) reads it from a hard-coded `/tmp/mhost-dns-disable-recovery.marker` path that disable never wrote to - The `if` branch has been dead code since the runtime_dir migration in PR #90; force_dns_restore_if_needed was never called - Fix: read via `mhost_dns::platform::disable_recovery_marker_file()` — same helper the disable path writes through Root cause 2 — capture_dns_state does not filter mHost's loopback proxy - `networksetup_get_dns` returns whatever networksetup says, including `127.0.0.1` if a previous enable left it in place - That `127.0.0.1` is mHost's own proxy address, not the user's DNS; capturing it as "original DNS" silently corrupts future restores - Fix: filter loopback via the existing `is_local_resolver` helper (already used by `get_upstream_resolvers` since issue #103) - If after filtering the list is empty, `capture_dns_state`'s existing `if !servers.is_empty()` guard correctly falls through to `DhcpEmpty` Tests - `test_capture_dns_state_filters_mhost_loopback`: source-grep pins the filter expression - `test_try_recover_dns_reads_canonical_marker_path`: source-grep pins both that the bad hard-coded path is gone AND the canonical helper is called Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…restore verify Three independent changes plus tests + E2E recipe, all addressing the open follow-ups from commit 3873345 (which closed the marker path and capture-filter root causes). Step 1 — OriginalDns defense layer - models.rs:466-471: OriginalDns::restore_argv filters loopback; empty after filter → ["Empty"] (never silently write 127.0.0.1) - models.rs:524-534: Repr::Legacy deserializer filters loopback so legacy ['127.0.0.1'] manifest migrates cleanly to DhcpEmpty - platform.rs:578-586: enable_dns_mode original.txt writer does final filter before write_atomic_0600 (belt-and-suspenders) Step 2 — D3-2 pgrep race fix (disable→re-enable kills proxy mid-restore) - platform.rs:752-794: top-of-script inline block now reads pid_file, verifies ps -o comm= basename matches recorded binary path, then TERM. Broad pgrep sweep gated behind pid_file freshness check (>30s old or missing). Same safety pattern as cleanup_stale_proxy. Step 3 — post-restore verification fallback - platform.rs verify_dns_restored_against_loopback helper + new any_local_resolver pure function - disable_dns_mode success branch (line 965) now verifies DNS no longer contains loopback before clearing marker; if it does (proxy exited but networksetup failed silently), escalates to osascript sudo fallback and preserves marker for next-launch retry Tests - Replaced source-grep test_capture_dns_state_filters_mhost_loopback with test_networksetup_get_dns_filter_pipeline (real behavior) - Added test_post_restore_verify_helper_detects_loopback - Added test_disable_recovery_marker_file_path_is_canonical (real behavior, no source-grep) - Added test_enable_script_inline_orphan_kill_uses_pid_file + the negative pin test_enable_script_inline_orphan_kill_does_not_blind_pgrep - Added 3 tests in mhost-core covering the OriginalDns defense layer (restore_argv, legacy Vec deserializer, tagged Manual deserializer) Documentation - doc/tech/dns-mode-e2e-recipe.md: 4 macOS scenarios + log grep cheatsheet Verified - cargo fmt --all -- --check (clean) - cargo clippy --all-targets --all-features -- -D warnings (clean) - cargo test --all-features (137 mhost + 117 mhost-dns + 48 mhost-core + ...) - pnpm test (313 frontend tests pass) - pnpm build (clean) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…pagate cleanup Err Two structural bugs in commands/dns.rs that together produce the 'UI says Running while system DNS is stuck at 127.0.0.1' symptom: A. set_dns_mode_disable ordered its in-memory flag flip AFTER the privileged disable_dns_mode call. If disable_dns_mode returned Err (proxy hung past 5s, sudo rejected, networksetup hiccup), the IPC returned Err before the flip. Frontend catch path truth-fetched getDnsMode(), which returned the in-memory stale 'true', and set dnsEnabledAtom=true. UI showed 'Running' while system DNS was broken. Fix: flip dns_enabled BEFORE disable_dns_mode so it always matches user intent regardless of the privileged step's outcome. B. cleanup_dns_on_exit swallowed set_dns_mode_disable's Err into Ok + a single eprintln, claiming the recovery marker would handle next-launch restoration. The recovery IS handled, but the swallow hid the in-session degraded state from lib.rs at lib.rs:91, 273, 381. Tray Quit / Cmd-Q / SIGINT paths logged 'DNS cleanup ok' while system DNS was actually stuck at 127.0.0.1. Fix: propagate the Err. lib.rs already handles it correctly; the swallow was the source of the misleading log. Tests: - test_set_dns_mode_disable_clears_in_memory_flag_even_on_disable_failure (new): asserts dns_enabled=false after a disable where disable_dns_mode returns Err. Pre-fix would have left it true. - test_cleanup_dns_on_exit_propagates_disable_failure (new): asserts cleanup_dns_on_exit returns Err when disable actually failed. Pre-fix swallowed into Ok. - Updated test_cleanup_dns_on_exit_idempotent_across_calls and test_set_dns_mode_disable_succeeds_with_dhcp_empty_snapshot to match the new contract (Err propagated, not swallowed). Out of scope: B's separate OS-probing IPC idea (#153). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

macOS TCC caches authorization for ~5min; same AppleScript command string within that window silently returns Ok without showing the password prompt. This caused 'I clicked Enable but no prompt appeared, UI stuck' confusion. Fix: extract build_osascript_command(script_path, nonce) as a pure function; spawn_osascript / invoke_osascript now inject a unique nonce (nanos + pid + atomic counter) into the elevated shell command as a shell comment '#nonce<value>'. Different commands → different TCC cache key → fresh prompt every time. 5 new tests cover: nonce in command, uniqueness, escape rules for \ and ", uniqueness across rapid calls. Applies to both enable (spawn_osascript) and disable/recovery (invoke_osascript) paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

mHost Developer and others added 7 commits August 7, 2026 16:48

flyhigher139 closed this Aug 10, 2026

Navigation Menu

Uh oh!

feat(#149): Settings cancel button + IPC-level abort signal for DNS enable/disable - #151

feat(#149): Settings cancel button + IPC-level abort signal for DNS enable/disable#151
flyhigher139 wants to merge 7 commits into
masterfrom
fix/dns-cancel-149

flyhigher139 commented Aug 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

flyhigher139 commented Aug 7, 2026

Background

Changes

Backend (Rust)

Frontend

Acceptance criteria

Tests added

Verification

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant