fix(wifi): retry an inconclusive backend probe instead of latching "no backend" (#613)

[vibechoom]

Closes #613
Refs #607

Phase 1 only, per the design settled on the #613 thread. Switching backends after one is already running is not here — that's #633, and it's blocked on a cancellation primitive that doesn't exist.

The bug

wifi/mod.rs's select_backend turned Err(ProbeError) — "I could not ask the bus" — into BackendChoice::None — "the bus answered and neither daemon is present". Service::start called it exactly once, so a single transient D-Bus failure at startup latched "no wireless backend" for the whole process lifetime: no watcher spawned, no in-UI hint anything had failed, and only systemctl --user restart trollshell brought Wi-Fi back.

Does this make a restart unnecessary for what Mara hit in #607? Yes, for that failure mode. Mara's case was a transient failure at startup that a restart cured — i.e. the bus was reachable moments later. That is exactly what the retry now covers: the probe keeps asking until it gets a real answer, and Wi-Fi comes up on its own. What it does not cover is a Wi-Fi daemon that starts after the probe has already reached a conclusive None verdict, or a runtime iwd↔NM switch — that's #633.

What changed

probe_backend itself is untouched (#609 already made it honest); the collapse was one layer up.

The obvious fix — retry inside select_backend — would have been worse than the bug: that function ran rt.block_on(...) on the calling thread during Service::start, i.e. the GTK main thread, so a retry loop there freezes shell startup. Instead the probe and the three-arm branch moved inside a single spawn_supervised closure, the shape networkd::NetworkdService::start (networkd.rs:305-368) already uses: one task owns the lifecycle and decides internally, off the main thread, so start() returns immediately.

Retry only while the probe is inconclusive (Err). Commit immediately on any Ok(_) — including Ok(BackendChoice::None), which is a real answer. Retrying "nobody's there" would be a bug of its own (an endless poll on a host that genuinely has no Wi-Fi daemon).

Two mechanical consequences of start() now returning before the backend is known:

• WifiHandles.backend is a Mutable<WifiBackend> set once the probe concludes, rather than a field fixed in the struct literal; get_backend() reads its current value. Command dispatch (scan, connect_network, …) sees None until then — indistinguishable from the pre-discovery startup window that already exists today.
• _ownership / _nm_agent became Arc<OnceLock<…>> slots the probe task parks its handles in, since neither exists when the struct is built.

The per-backend watchers keep their own independent spawn_supervised. The decide-task returns once it has committed, and spawn_supervised doesn't restart a clean completion, so a backend is never started twice.

Incidental second fix, not claimed in the title

Pre-PR, select_backend ran rt.block_on(probe_backend()) on the GTK main thread. That means a slow bus didn't just break Wi-Fi — it froze the entire shell for the duration of the probe: roughly 10s with the socket down (two sequential hytte_bus::calls, each fast-failing then waiting up to 5s in wait_for_reconnect), and up to ~50s against a wedged peer (two full 25s timeouts, which is_transient_zbus_error doesn't classify as transient so there's no retry). Moving the probe off the main thread removes that freeze as a side effect. Worth having in the merge note.

The diagnostic is not silenced

This was the requirement that held the fix, so it's treated as one. #609's error! exists to measure how often this fires, and a retry that healed quietly would trade a visible permanent bug for an invisible intermittent one — worse for exactly Mara's situation, where nothing in the UI hinted anything had failed.

• Every inconclusive attempt logs at error!, with attempt and retry_in_secs. Deliberately every attempt rather than every Nth, because the backoff caps at 8s — so a permanently dead bus stays loud in the journal instead of degrading into a silent poll.
• A retry that finally succeeds logs wifi: backend probe RECOVERED at warn! with attempts=N.
• The recovery wording is gated on the verdict, not just on having retried. A host that genuinely runs no Wi-Fi daemon recovers into None, and telling that user "Wi-Fi is coming up" would be a false line in the very diagnostic this PR exists to keep trustworthy — so that case gets its own wording ("the bus is answering again, and the verdict is that NO Wi-Fi daemon is present").
• The retry line names the backend probe RECOVERED token to grep for, so an error-level-only filter still points at the resolution.

Retry policy — unbounded

Unbounded retry with capped backoff. Attempt 1 is immediate; each retry waits 0.5s doubled per elapsed attempt, clamped at 8s, settling into one probe every 8s for as long as it takes.

I originally shipped a bounded policy (8 attempts, give up at error!). Review killed it, correctly, and the counter-example was in the file I copied the shape from — networkd.rs:283-289 already decided this exact question the other way:

Picking None here is what latched #607: it leaves the link list permanently empty even once the bus recovers. The NM watcher re-polls every 5s, so it self-heals if NM does turn up; the cost if NM genuinely never appears is a periodic GetDevices failed warn, which beats an inert service.

That transfers wholesale, because both Wi-Fi watchers self-heal the same way — wifi_nm.rs's discovery retries every 5s and watcher.rs has its 'discovery loop. A bounded give-up would leave the user in precisely #607's state (Wi-Fi gone, restart the only cure) whenever the bus takes longer than the bound: the bug this PR closes, reintroduced on a longer fuse. And the justification was circular — "giving up is logged at error! so a wrong bound is visible" argues for a bound from the visibility of the bound being wrong. Unbounded retry logging every attempt gets identical visibility with no latch.

GiveUp and its tests stay in the code, so a bounded policy remains one field away (max_attempts: Some(n)); only the shipped default is unbounded, and shipped_policy_is_unbounded_and_starts_promptly asserts that. The same question is open on #621 for networkd's startup refresh — @annikahannig may answer it either way, and the two subsystems should end up consistent.

I also dropped an earlier "~31.5s of wall clock" figure from the comment: that was the sum of the sleeps, assuming a probe attempt costs zero, which it doesn't in exactly the failure mode being retried (see the per-attempt costs in the incidental-fix note above). The comment now describes the schedule in attempts and backoff and states the real per-attempt range instead of asserting a false total.

Tests + falsification

The retry/commit/give-up decision is a pure function, RetryPolicy::step(&outcome, attempt) -> ProbeStep, so it's testable with no bus. 8 new tests, all in the hermetic bucket (plain #[cfg(test)], not system-tests):

ok_commits_on_the_first_attempt, ok_none_commits_and_does_not_retry, err_retries_while_attempts_remain, err_past_the_bound_gives_up, inconclusive_probe_never_commits_to_no_backend, backoff_doubles_and_clamps_to_the_ceiling, unbounded_policy_never_gives_up, shipped_policy_is_unbounded_and_starts_promptly.

Falsification 1 — the latch. With the fix in place: cargo test -p hytte-services → 671 passed, 0 failed. Reintroducing the original bug (Err(_) => ProbeStep::Commit(BackendChoice::None)):

test wifi::tests::err_past_the_bound_gives_up ... FAILED
test wifi::tests::err_retries_while_attempts_remain ... FAILED
test wifi::tests::inconclusive_probe_never_commits_to_no_backend ... FAILED
test wifi::tests::unbounded_policy_never_gives_up ... FAILED

test result: FAILED. 4 passed; 4 failed

e.g. err_retries_while_attempts_remain: left: Commit(None), right: Retry { after: 10ms }; inconclusive_probe_never_commits_to_no_backend: "attempt 1: an inconclusive probe was committed as 'no backend'".

Falsification 2 — the unbounded guard. Re-bounding the shipped policy to max_attempts: Some(8):

test wifi::tests::shipped_policy_is_unbounded_and_starts_promptly ... FAILED
  assertion `left == right` failed: the shipped probe policy must never give up; see PROBE_RETRY's docs and #621

test result: FAILED. 7 passed; 1 failed

Both edits restored afterwards and verified byte-identical with diff against saved copies, then re-run clean.

Gates

• cargo clippy -p hytte-services --all-targets --features system-tests -- -D warnings — clean (--all-targets so the new #[cfg(test)] code is linted; RUSTC_WRAPPER unset so sccache can't skip lint emission).
• cargo test -p hytte-services — 671 passed, 0 failed.
• nix fmt — 0 changed on both runs.

Docs this makes stale — not edited here

docs/live-verify.md goes false on merge, in two places. PR #628 owns that file and is green awaiting merge, so nothing here touches it; flagging for whoever sequences the merges:

• :434-437 currently tells a verifier "the backend probe still runs only once at startup … that re-probe gap is deliberately untouched and is tracked as feat(wifi): make backend selection re-entrant so a transient probe failure recovers without a restart (#609 follow-up) #613". After this merge the probe retries, and feat(wifi): make backend selection re-entrant so a transient probe failure recovers without a restart (#609 follow-up) #613 is closed.
• :463-472's fix(wifi): a failed backend probe is no longer indistinguishable from "no backend" (#607) #609 item now expects the wrong outcome: it directs the verifier to look for an error! pointing at systemctl restart, when the expected result is now recovery.

Live-verify

CI cannot exercise a transient bus failure — nothing in nix flake check can make ListNames fail and then succeed. This needs a manual check on a real session:

1. Start the shell while the system bus / NetworkManager is still coming up (e.g. systemctl --user restart trollshell immediately after systemctl restart NetworkManager, or start it early in a fresh boot's session).
2. Confirm the Wi-Fi card in the network panel populates on its own, without a shell restart — the network list appears once the probe recovers.
3. Confirm the journal shows both halves:

   journalctl --user -u trollshell | grep -E 'backend probe (was INCONCLUSIVE|RECOVERED)'

   Expect at least one INCONCLUSIVE line (with attempt= and retry_in_secs=) followed by one RECOVERED line carrying attempts=.
4. Sanity check the negative case is unchanged: on a host with neither daemon, wifi: no Wi-Fi backend present — service is inactive should still appear once, promptly, with no retry lines — Ok(None) must commit immediately.
5. Sanity check the shell no longer freezes on a slow bus: startup should stay responsive while the probe is retrying, since it no longer runs on the GTK main thread.

🤖 Generated with Claude Code