chore(services): three hand-rolled exponential backoffs, and three fixed-2s reconnect loops with no ceiling (#645 follow-up)

[vibechoom]

Follow-up from PR #645 (#621). That PR added a third hand-rolled exponential backoff to the workspace, which is the right call for its own lane — sharing the type would have meant editing wifi/mod.rs, outside the issue's file lane — but it makes the duplication worth filing rather than absorbing silently.

Filing as code-climate, not a bug. Nothing here is user-visible today.

Three independent exponential-backoff implementations

Site	Shape	Schedule
crates/hytte-bus/src/connection.rs:63-81	Backoff { next_ms }, mutable cursor with reset()/next()	250 ms, doubling, 30 s cap
crates/hytte-services/src/wifi/mod.rs:213-260	RetryPolicy { max_attempts, initial, max_backoff } + pure step()	500 ms, doubling, 8 s cap
crates/hytte-services/src/networkd.rs (new in #645)	RefreshRetry { max_attempts, initial, max_backoff } + pure step()	500 ms, doubling, 8 s cap

The latter two are near-identical: same three fields with the same meanings, and a backoff() body that is character-for-character the same checked_shl / saturating_mul / min sequence. They differ only in the type their step() weighs — wifi's is typed to Result<BackendChoice, ProbeError> because an inconclusive probe is a distinct outcome from a failed one, while networkd's is generic over &Result<T, E> because a refresh either read the links or did not.

That difference is real, and it is the reason #645 did not just call the existing type. But it is also small: the generic version in networkd.rs subsumes the wifi one if wifi keeps its verdict-weighing in the caller rather than inside step.

hytte-bus's Backoff is a different enough shape (a stateful cursor, no attempt budget, no give-up) that folding all three into one type is probably over-unification. Two is the interesting number here.

The doc comments on the two hytte-services copies currently cross-reference each other by hand, which is what keeps them from drifting. That works until someone edits one and not the other.

Three fixed-2s reconnect loops with no backoff and no log ceiling

Same family, different symptom — a listen()-style stream that ends or errors, retried at a flat 2 s forever, logging a warn! every time:

• crates/hytte-services/src/networkd.rs:346-353 — Ok(()) => warn!("networkd stream ended, retrying in 2s") / Err(e) => warn!("networkd error, retrying in 2s"), then sleep(2s). Deliberately left untouched by fix(networkd): retry a transient startup refresh failure instead of going inert (#621) #645, which was scoped to the startup seed path.
• crates/hytte-services/src/mpris/mod.rs:172-182 — identical shape, "mpris watcher stream closed/error, reconnecting in 2s".
• crates/hytte-services/src/bluetooth/mod.rs:~190-201 — resets the adapter-path store and sleeps 2 s inside its watch loop.

If any of these daemons is down for an extended period, the journal gets a line every 2 s indefinitely. This is the same concern the STARTUP_REFRESH_RETRY docs in #645 reason about explicitly, and the same one hytte-bus already solved for connections with its 30 s cap.

Ruled out, so nobody re-adds it: crates/hytte-services/src/wifi/watcher.rs:131 matches the sleep(Duration::from_secs(2)) grep but is not a retry loop — it is a one-shot debounce before return on the iwd station-removed path, where the caller re-establishes the watch. Leave it alone.

Severity

Low. No wrong behaviour and no user-facing symptom. The cost is maintenance: a policy change (or a bug fix) in one copy silently fails to reach the other, and the two hytte-services copies are similar enough that a reader can easily believe they have already read the second one. The journal-cadence half is a mild operational annoyance on a host with a persistently dead daemon, not a defect.

Recommendation

Two independent pieces of work; the first is worth doing, the second is worth doing only if someone is in the file anyway.

1. Promote one retry-policy type within hytte-services. Keep networkd.rs's generic step<T, E>(&Result<T, E>, attempt) as the surviving shape, move it somewhere shared in the crate, and have wifi weigh its inconclusive-vs-failed verdict in the caller before handing a Result to step. This keeps both call sites' current behaviour byte-identical — the constants already match — so it is a pure refactor with the existing tests on both sides as the gate. Do not pull hytte-bus's Backoff into it.

2. Give the three listen-loop retries a capped backoff. The reconnect cadence matters much less than the startup one, so the simple move is to reuse whatever type falls out of (1) with a longer ceiling, and drop the "retrying in 2s" wording from the log lines, which becomes false the moment a backoff exists.

Neither is urgent. If only one gets done, do (1) — the drift risk is the actual cost here.

Refs #645, #621, #634, #613.

[vibechoom]

Half of this shipped in PR #675 (Refs, not Closes): wifi's RetryPolicy/ProbeStep and networkd's RefreshRetry/RefreshStep are now one retry::Policy / retry::Step in a new private crates/hytte-services/src/retry.rs, with no behaviour change.

Two details from that PR worth carrying here rather than leaving in a merged body:

• It is named retry::Policy, not RetryPolicy — because hytte_bus::RetryPolicy already exists and is a different concept (a per-call Never/Once/Backoff selector). PROBE_RETRY's docs referenced that type two paragraphs from where they described their own, which is exactly the collision this issue was about. The module doc now links it explicitly.
• hytte_bus's Backoff and wifi/watcher.rs:131's debounce are both named in retry.rs's module docs as deliberately excluded, so the next person grepping from_secs(2) finds the reasoning before they find the sites. That was the one thing most likely to be "fixed" wrongly later.

The second half — and the design question nobody had asked

The three fixed-2 s reconnect loops (networkd's listen loop, mpris, bluetooth) are deliberately untouched, and the reason is better than "out of scope":

Those loops have no clean success signal. listen returning is not success — it is the stream ending, which is the same event whether the daemon restarted cleanly or the bus is thrashing. So there is nothing to reset an attempt counter on.

Bolt a naive counter onto them and a healthy-but-flappy daemon — one that reconnects fine every few minutes — backs off to the 8 s ceiling and stays there, because nothing ever resets it. That is strictly worse than the current flat 2 s: same log volume, worse recovery.

So the real work here is choosing a reset predicate, not applying retry::Policy. The obvious candidate is "reset if the previous listen ran longer than N seconds" — treat a connection that survived a while as evidence the daemon is healthy — but N is a judgement call, and getting it wrong in either direction is a live-verify problem rather than something a unit test settles.

That is a self-contained follow-up. It should also fix the log wording: every one of those sites says "retrying in 2s", which becomes false the moment a backoff exists.

Still open here

• the three reconnect loops, per the above
• their uncapped warn! cadence, which is the observability half of the same change

retry.rs now carries a SHIPPED table of every policy the crate ships, swept by two tests, so whatever lands for the loops cannot ship without the ceiling-reached and nonzero-floor assertions that #665 added.

Refs #675, #665.