fix(daemon): a repeater that is not running is no longer reported as running (#1298)

CLAUDE.md

@@ -238,6 +238,7 @@ Each requirement below is done when the linked test passes. Add new links as tes
 | No relay-path error leaves rig_b keyed **while the daemon is alive**: every path is an RAII guard under a watchdog, the §97.119 ID rides the frame's key instead of asserting a second one underneath it, and a full-duplex hold is bounded by **silence** (each relayed frame re-stamps the deadline) rather than held from session start. The watchdog is in-process, so it bounds a hang, NOT a dead daemon — on rigctld/CM108/GPIO nothing releases rig_b if the process dies, which is why the hold is no longer eager. The ID gate decodes `DE N0CALL` off the transmit side; the edge count it replaced was a proxy that passed while the double-key shipped. **Loopback tier only — the repeater cannot receive a frame on hardware at all (#1297)** | `cargo test -p openpulse-repeater --no-default-features --no-fail-fast` + `cargo test -p openpulse-radio --no-default-features --lib shared_ptt` |
 | `[radio.rig_b]` cannot alias the rigctld the main rig already uses — the daemon refuses to start **when the repeater is enabled at startup** (otherwise it warns and builds no repeater). Two controllers over one transmitter key and release each other, and #1263's refusal rule reaches only *within* one `SharedPtt`. Not exotic: both `RigConfig::default()` and `RadioConfig::default()` carry `127.0.0.1:4532`, so an empty `[radio.rig_b]` header IS the collision — and the shared endpoint is rigctld itself, so `cat_backend = "rigctld"` alone collides even with a non-rigctld `ptt_backend`. Scoped to string equality: it catches the shipped defaults, not `localhost` vs `127.0.0.1` | `cargo test -p openpulse-daemon --no-default-features --lib repeater_rig_b_tests` |
 | A **cap-flushed** burst is not evidence about the rate ladder (#1255) — the cap exceeds the longest candidate frame, so hitting it means the carrier was still up and the slab is not one transmission. A failed decode of one must not key an ACK or move `recommended_level`. The decode itself still runs: when the squelch sits below the band floor EVERY burst is a cap flush (#1254's regime), so skipping it would make the daemon deaf on a hot band — pinned by a control that decodes a frame at the head of a capped slab. Runs ~70 s, dominated by one `ota_decode_burst` over the candidate rungs | `cargo test -p openpulse-modem --no-default-features --test cap_flush_is_not_ladder_evidence` |
+| A repeater that is not running is not reported as running (#1298) — enabling with nothing to run FAILS with a reason instead of emitting `RepeaterChanged { enabled: true }`, and a thread that exited is reaped so the next command sees the truth rather than "already enabled" forever. The thread OWNS the `CrossBandRepeater`, so its exit means the repeater is gone | `cargo test -p openpulse-daemon --no-default-features --lib command_apply_tests` |
 | `openpulse-kiss`'s `SharedPtt` has a **watchdog thread**, so its deadline is enforced — the crate built one and called `spawn_watchdog` nowhere, leaving `force_release_if_expired` with no caller in the crate. The guard covers an early return and an unwind; it cannot reach a transmit that BLOCKS, which is the case the watchdog exists for. Driven through the real constructor, since the defect was the wiring | `cargo test -p openpulse-kiss --no-default-features --test ptt_keys_every_transmit` |
 | `openpulse-mesh` has no route to a sound card — it beacons and relays automatically with no PTT controller, no carrier sense and no station-ID timer, and its beacon carries no callsign field, so the capability was REMOVED rather than guarded (a fourth hand-rolled keying path on a crate with no §97.221 mapping, no control point and no on-air record). Each check is validated against a planted input | `cargo test -p openpulse-mesh --no-default-features --test no_real_audio` |
 | A CONACK cannot select a signing mode the CONREQ never offered (F-1147-05 — v1 checked local policy only) | `cargo test -p openpulse-core --no-default-features --test handshake_integration conack_rejected_when_mode_not_offered` |

crates/openpulse-daemon/src/lib.rs

@@ -1609,6 +1609,39 @@ pub(crate) enum KeyedTxError {
     Transmit(openpulse_core::error::ModemError),
 }
 
+/// Clear repeater state whose thread has already exited (#1298).
+///
+/// The `EnableRepeater` thread OWNS the `CrossBandRepeater` — it is `take()`n out of the runtime
+/// state — so when that thread exits the repeater is gone for good. Nothing observed that:
+/// `repeater_enabled` stayed `true`, `EnableRepeater` answered "already enabled", and a
+/// `DisableRepeater` + `EnableRepeater` pair reported success while starting nothing.
+///
+/// Called at the top of both repeater commands rather than from a periodic tick: the events emitted
+/// by the thread itself are what notify clients promptly, and this only has to make the *next*
+/// command see the truth. A tick poll would need `runtime_state` on the tick path for no added
+/// signal.
+fn reap_finished_repeater(
+    runtime_state: &mut RuntimeControlState,
+    event_tx: &Arc<broadcast::Sender<ControlEvent>>,
+) {
+    let finished = runtime_state
+        .repeater_thread
+        .as_ref()
+        .is_some_and(|t| t.is_finished());
+    if !finished {
+        return;
+    }
+    if let Some(t) = runtime_state.repeater_thread.take() {
+        let _ = t.join();
+    }
+    runtime_state.repeater_stop = None;
+    if runtime_state.repeater_enabled {
+        runtime_state.repeater_enabled = false;
+        tracing::warn!("cross-band repeater thread has exited; marking the repeater disabled");
+        let _ = event_tx.send(ControlEvent::RepeaterChanged { enabled: false });
+    }
+}
+
 pub(crate) fn keyed_transmit<T>(
     ptt: &crate::ptt::SharedPtt,
     event_tx: Option<&broadcast::Sender<ControlEvent>>,
@@ -2567,6 +2600,7 @@ pub async fn apply_command_to_engine(
             }
         }
         ControlCommand::EnableRepeater => {
+            reap_finished_repeater(runtime_state, event_tx);
             if runtime_state.repeater_enabled {
                 let _ = event_tx.send(ControlEvent::CommandError {
                     command: "enable_repeater".to_string(),
@@ -2575,24 +2609,46 @@ pub async fn apply_command_to_engine(
                 return;
             }
 
-            if let Some(mut repeater) = runtime_state.repeater.take() {
-                let stop = Arc::new(AtomicBool::new(false));
-                let stop_clone = Arc::clone(&stop);
-                let thread = std::thread::spawn(move || {
-                    if let Err(e) = repeater.run_full_duplex(stop_clone) {
-                        tracing::warn!(error = %e, "cross-band repeater exited with error");
-                    }
+            // No repeater to run is a FAILED enable, not a quiet one (#1298). This arm used to warn
+            // at `tracing::warn!` and then set `repeater_enabled = true` and emit
+            // `RepeaterChanged { enabled: true }` anyway — so a daemon with no repeater reported one
+            // as running, and no command sequence could get back to a truthful state.
+            let Some(mut repeater) = runtime_state.repeater.take() else {
+                let _ = event_tx.send(ControlEvent::CommandError {
+                    command: "enable_repeater".to_string(),
+                    reason: "no repeater is available — it was not built at startup (see the \
+                             startup log for why), or a previous session ended and consumed it"
+                        .to_string(),
                 });
-                runtime_state.repeater_stop = Some(stop);
-                runtime_state.repeater_thread = Some(thread);
-            } else {
-                tracing::warn!("enable_repeater: no pre-built repeater in runtime state; audio routing not started");
-            }
+                return;
+            };
+
+            let stop = Arc::new(AtomicBool::new(false));
+            let stop_clone = Arc::clone(&stop);
+            // The thread owns the repeater, so when it exits the repeater is GONE. Report that:
+            // without it, "relaying nothing because the band is quiet" and "the thread died" are
+            // the same observation from outside (#1298).
+            let thread_tx = Arc::clone(event_tx);
+            let thread = std::thread::spawn(move || {
+                if let Err(e) = repeater.run_full_duplex(stop_clone) {
+                    tracing::warn!(error = %e, "cross-band repeater exited with error");
+                    let _ = thread_tx.send(ControlEvent::CommandError {
+                        command: "repeater".to_string(),
+                        reason: format!("cross-band repeater stopped: {e}"),
+                    });
+                    // Only on the error path: a clean stop is already reported by DisableRepeater,
+                    // and emitting there too would put two `false` edges on one transition.
+                    let _ = thread_tx.send(ControlEvent::RepeaterChanged { enabled: false });
+                }
+            });
+            runtime_state.repeater_stop = Some(stop);
+            runtime_state.repeater_thread = Some(thread);
 
             runtime_state.repeater_enabled = true;
             let _ = event_tx.send(ControlEvent::RepeaterChanged { enabled: true });
         }
         ControlCommand::DisableRepeater => {
+            reap_finished_repeater(runtime_state, event_tx);
             if !runtime_state.repeater_enabled {
                 let _ = event_tx.send(ControlEvent::CommandError {
                     command: "disable_repeater".to_string(),
@@ -4334,13 +4390,34 @@ mod command_apply_tests {
         }
     }
 
+    /// Build a real, runnable repeater so `EnableRepeater` has something to start.
+    fn test_repeater(enabled: bool) -> openpulse_repeater::CrossBandRepeater {
+        let mk = || {
+            let mut e = ModemEngine::new(Box::new(openpulse_audio::LoopbackBackend::new()));
+            let _ = e.register_plugin(Box::new(bpsk_plugin::BpskPlugin::new()));
+            e
+        };
+        openpulse_repeater::CrossBandRepeater::new(
+            Box::new(openpulse_radio::NoOpPtt::new()),
+            mk(),
+            mk(),
+            openpulse_repeater::RepeaterConfig {
+                enabled,
+                ..Default::default()
+            },
+        )
+    }
+
     #[tokio::test]
     async fn apply_repeater_enable_disable_emits_state_changes() {
         let mut engine = test_engine();
         let active_mode: SharedMode = Arc::new(Mutex::new("BPSK250".to_string()));
         let (tx, mut rx) = broadcast::channel::<ControlEvent>(16);
         let ev_tx = Arc::new(tx);
         let mut runtime_state = RuntimeControlState::default();
+        // #1298: this test used to run with `repeater: None` and assert that enabling SUCCEEDED —
+        // it was pinning the defect. A daemon with nothing to run must not report a running repeater.
+        runtime_state.repeater = Some(test_repeater(true));
 
         apply_command_to_engine(
             &ControlCommand::EnableRepeater,
@@ -4373,6 +4450,121 @@ mod command_apply_tests {
         }
     }
 
+    /// THE #1298 GATE (enable half): with nothing to run, enabling must FAIL and say so.
+    ///
+    /// The old arm logged a `warn!`, then set `repeater_enabled = true` and emitted
+    /// `RepeaterChanged { enabled: true }` regardless — so a daemon with no repeater reported one as
+    /// running, and no command sequence could get back to a truthful state.
+    #[tokio::test]
+    async fn enabling_a_repeater_that_does_not_exist_fails_instead_of_claiming_success() {
+        let mut engine = test_engine();
+        let active_mode: SharedMode = Arc::new(Mutex::new("BPSK250".to_string()));
+        let (tx, mut rx) = broadcast::channel::<ControlEvent>(16);
+        let ev_tx = Arc::new(tx);
+        let mut runtime_state = RuntimeControlState::default();
+        assert!(runtime_state.repeater.is_none(), "premise: nothing to run");
+
+        apply_command_to_engine(
+            &ControlCommand::EnableRepeater,
+            &mut engine,
+            &active_mode,
+            &ev_tx,
+            None,
+            &mut runtime_state,
+        )
+        .await;
+
+        assert!(
+            !runtime_state.repeater_enabled,
+            "the daemon reports a repeater as enabled while none exists"
+        );
+        match rx.recv().await.expect("expected an event") {
+            ControlEvent::CommandError { command, reason } => {
+                assert_eq!(command, "enable_repeater");
+                assert!(
+                    reason.contains("no repeater is available"),
+                    "unhelpful reason: {reason}"
+                );
+            }
+            other => {
+                panic!("expected CommandError, got {other:?} — a RepeaterChanged here is the bug")
+            }
+        }
+    }
+
+    /// THE #1298 GATE (reap half): a thread that has exited must not leave the repeater "enabled".
+    ///
+    /// The thread OWNS the `CrossBandRepeater`, so its exit means the repeater is gone. Nothing
+    /// observed that: `EnableRepeater` answered "already enabled" forever after.
+    #[tokio::test]
+    async fn a_repeater_thread_that_exited_is_reaped_rather_than_reported_enabled() {
+        let mut engine = test_engine();
+        let active_mode: SharedMode = Arc::new(Mutex::new("BPSK250".to_string()));
+        let (tx, mut rx) = broadcast::channel::<ControlEvent>(64);
+        let ev_tx = Arc::new(tx);
+        let mut runtime_state = RuntimeControlState::default();
+        // A DISABLED repeater's `run_full_duplex` returns Ok(0) at once, so the thread exits
+        // immediately — standing in for any exit the daemon did not ask for.
+        runtime_state.repeater = Some(test_repeater(false));
+
+        apply_command_to_engine(
+            &ControlCommand::EnableRepeater,
+            &mut engine,
+            &active_mode,
+            &ev_tx,
+            None,
+            &mut runtime_state,
+        )
+        .await;
+        assert!(runtime_state.repeater_enabled, "enable succeeded");
+
+        // Let the thread finish.
+        for _ in 0..200 {
+            if runtime_state
+                .repeater_thread
+                .as_ref()
+                .is_some_and(|t| t.is_finished())
+            {
+                break;
+            }
+            tokio::time::sleep(std::time::Duration::from_millis(5)).await;
+        }
+
+        // The next command must see the truth rather than "already enabled".
+        apply_command_to_engine(
+            &ControlCommand::EnableRepeater,
+            &mut engine,
+            &active_mode,
+            &ev_tx,
+            None,
+            &mut runtime_state,
+        )
+        .await;
+
+        let mut saw_disabled_edge = false;
+        let mut saw_already_enabled = false;
+        while let Ok(ev) = rx.try_recv() {
+            match ev {
+                ControlEvent::RepeaterChanged { enabled: false } => saw_disabled_edge = true,
+                ControlEvent::CommandError { ref reason, .. }
+                    if reason.contains("already enabled") =>
+                {
+                    saw_already_enabled = true
+                }
+                _ => {}
+            }
+        }
+        assert!(
+            saw_disabled_edge,
+            "the dead thread was never reported: clients still believe a repeater is running"
+        );
+        assert!(
+            !saw_already_enabled,
+            "the daemon answered 'repeater already enabled' about a thread that had exited — this \
+             is the state no command sequence could escape"
+        );
+    }
+
     #[tokio::test]
     async fn apply_qsy_accept_reject_record_and_emit_decisions() {
         let mut engine = test_engine();

docs/dev/project/traceability.md

@@ -9,6 +9,45 @@ and the actually-observed results per change.
 
 ---
 
+## 2026-09-08 — A repeater that is not running was reported as running (#1298)
+
+- **Requirement/change:** the `EnableRepeater` thread `take()`s the `CrossBandRepeater` out of the
+  runtime state, so its exit means the repeater is gone — and nothing observed that.
+  `repeater_enabled` stayed `true`, `EnableRepeater` answered "already enabled" forever, and the
+  no-repeater arm logged a `warn!` and then set `repeater_enabled = true` and emitted
+  `RepeaterChanged { enabled: true }` anyway.
+
+- **Design decision (reviewed by Fable as part of #1297's design; it corrected the issue's premise).**
+  #1298 said "the loop exits on the first capture that does not decode, which on hardware is the
+  first capture", making enabled-forever the *normal* state on a rig. **That went stale when #1300
+  merged** — a non-decoding capture is now `Ok(None)` and the loop continues. What kills the thread
+  now is a PTT fault or a transmit error. So the normal state on a rig is not a dead thread reported
+  as enabled; it is **a live thread that never relays and never says so above DEBUG**. Both defects
+  in the issue survive that correction; its severity framing did not, and the issue has been amended.
+  Reaping at the top of the two repeater commands rather than from a periodic tick: the thread's own
+  events are what notify clients promptly, and the reap only has to make the *next* command truthful
+  — a tick poll would need `runtime_state` on the tick path for no added signal.
+
+- **Implementation:** `crates/openpulse-daemon/src/lib.rs` — `reap_finished_repeater`; the
+  no-repeater arm returns a `CommandError`; the thread emits `CommandError` + a `false` edge on the
+  **error path only**, since a clean stop is already reported by `DisableRepeater` and emitting there
+  too would put two `false` edges on one transition.
+
+- **Tests:** `enabling_a_repeater_that_does_not_exist_fails_instead_of_claiming_success` and
+  `a_repeater_thread_that_exited_is_reaped_rather_than_reported_enabled`. And
+  `apply_repeater_enable_disable_emits_state_changes` **was pinning the defect**: it ran with
+  `RuntimeControlState::default()` (`repeater: None`) and asserted that enabling SUCCEEDED. It now
+  builds a real `CrossBandRepeater` so the success path is the one it tests.
+
+- **Test results:** 7 passed. Sabotage-verified in both directions: removing the reap calls fails the
+  reap gate with "the dead thread was never reported"; reinstating the claim-success arm fails the
+  enable gate with "the daemon reports a repeater as enabled while none exists". Full workspace gate
+  below.
+
+- **Filed alongside:** #1308 — the repeater has **no audio device configuration at all**; both its
+  engines use the OS default input, and no config field anywhere could name a second card. #1297's
+  accumulation fix cannot be exercised on a real two-rig station without it.
+
 ## 2026-09-08 — A cap-flushed burst is not ladder evidence (#1255)
 
 - **Requirement/change:** `accumulate_routed` returned `Ok(Some(burst))` for two opposite events —