Consolidate mutex poison-recovery boilerplate in transcription engine

[BumpyClock]

Problem

The .unwrap_or_else(|e| { log::warn!(...); e.into_inner() }) poison-recovery pattern is copy-pasted across 19 lock sites in two files. The canonical wrapper already exists — EngineCell::lock (engine.rs:60-65) — but is only applied to one mutex:

// engine.rs:60-65  — the one site that uses a helper
fn lock(&self) -> std::sync::MutexGuard<'_, EngineSlot> {
    self.slot.lock().unwrap_or_else(|e| {
        log::warn!("engine slot mutex poisoned, recovering: {e}");
        e.into_inner()
    })
}

The remaining 18 call sites repeat the pattern inline, each with its own log string:

Location	Mutex	Lines
IdleUnloadCoordinator::touch	state	180-183
IdleUnloadCoordinator::cancel	state	193-196
IdleUnloadCoordinator::spawn_worker (initial wait)	state	212-215
spawn_worker condvar wait	state (via condvar)	224-229
spawn_worker pre-timeout lock	state	236-239
spawn_worker condvar wait_timeout	state (via condvar)	240-245
spawn_worker post-fire relock	state	282-285
IdleUnloadCoordinator::drop — state	state	317-320
IdleUnloadCoordinator::drop — worker	worker	326-329
TranscriptionEngineState::set_accelerator	accelerator	390-393
TranscriptionEngineState::set_model	model	409-412
TranscriptionEngineState::clear_model	model	427-430
checkout condvar wait (silent)	engine.slot	451
warm_up condvar wait (silent)	engine.slot	565
model_supports_streaming	model	605-608
streaming_chunk_ms	model	621-624
EngineRegistry::register	factories (RwLock write)	engine_trait.rs:112-115
EngineRegistry::factory_for	factories (RwLock read)	engine_trait.rs:121-124

Two of the condvar-wait sites (lines 451, 565) silently swallow the error without logging — an inconsistency introduced by the repetition.

Why it matters

• Lock-ordering audits are painful. Each inline recovery block is syntactic noise in the already-dense concurrency paths (spawn_worker inner loop, checkout, warm_up). Reviewers must read past the boilerplate to trace actual flow.
• Inconsistency hides bugs. Lines 451 and 565 silently recover (e.into_inner() with no log) while the others warn — different behaviors for the same condition, with no documented rationale.
• Hot path inflation. Several sites sit on the transcription hot path (checkout, model_supports_streaming, streaming_chunk_ms). Dead-code boilerplate makes the happy path harder to read and profile.

Proposed change

1. Introduce one generic helper (or a macro if the condvar variant needs covering) analogous to EngineCell::lock, e.g.:

   fn lock_recovering<T>(mutex: &Mutex<T>, label: &str) -> MutexGuard<'_, T> {
       mutex.lock().unwrap_or_else(|e| {
           log::warn!("{label} mutex poisoned, recovering: {e}");
           e.into_inner()
       })
   }

   The condvar .wait/.wait_timeout variants also need coverage — either a second helper or a macro capturing the label.

2. Replace all 18 inline sites with calls to the helper. EngineCell::lock (the existing wrapper) can delegate to the same helper internally or stay as-is since it already encapsulates the pattern.

3. Fix the two silent condvar sites (lines 451, 565): add the warn log to match the policy of every other site, or deliberately expect if poisoning there is considered unreachable.

4. No behavior change — this is a pure refactor. All recovery semantics stay identical; only the duplication is removed.

Tests to check: No test currently injects a poisoned mutex, so the test suite will pass unchanged. If a poison-recovery test is added in future, it should target the helper, not individual sites.

Recommendation / triage

DO — mechanical, behavior-identical, removes boilerplate from the trickiest code. Generalize the EngineCell::lock-style wrapper into one helper reused at every site. Broad but low-risk; keep it a pure refactor (no behavior change) in its own PR.

Acceptance criteria

• A single poison-recovery helper (function or macro) exists; all 18 inline .unwrap_or_else lock sites in engine.rs and engine_trait.rs delegate to it.
• The two silent condvar-wait sites (engine.rs:451, 565) either log a warning consistent with the policy or carry a comment explaining why silent recovery is intentional.
• cargo test -p ansible-core passes with no changes to test logic or assertions.
• cargo clippy -p ansible-core -- -D warnings is clean.
• No log::warn! message wording changes that would break any log-scraping in CI.
• Diff is behavior-identical: no changes to lock acquisition order, condvar usage, or error propagation paths.

Traceability

clawpatch finding ID: fnd_sig-feat-service-25552574c9-8c3e_f8b3f1b6ef

Revalidate command:

clawpatch revalidate --finding fnd_sig-feat-service-25552574c9-8c3e_f8b3f1b6ef --reasoning-effort high

No coupled issues at time of filing.

[coderabbitai]

🔗 Related PRs

BumpyClock/roci#2 - [BREAKING] Align agent runtime contracts [merged]
BumpyClock/gpui#1 - [BREAKING] Sync GPUI with upstream web support [merged]
BumpyClock/tasque#1 - [Feature] Add readable task identity and spec update flows [closed]
BumpyClock/Digests-Swift#94 - fix: preserve SwiftData startup actor isolation [merged]
BumpyClock/Digests-Swift#112 - Refactor reader content prefetch pipeline [merged]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

• Create Plan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!