fix(subagent): panic-proof overlap guards and update stale docs

RunningGuard resets TEST_GEN_RUNNING/ARCH_REVIEW_RUNNING/SECURITY_REVIEW_RUNNING
via Drop so a subagent panic can no longer wedge that review kind disabled
for the rest of the process. Doc comments on run_subagent_with_retry and the
three spawn_background_* / spawn_all_background functions now describe the
abort_flag and overlap-guard behavior added in Task 7.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

src/app/subagent/auto.rs

@@ -47,6 +47,19 @@ static TEST_GEN_RUNNING: AtomicBool = AtomicBool::new(false);
 static ARCH_REVIEW_RUNNING: AtomicBool = AtomicBool::new(false);
 static SECURITY_REVIEW_RUNNING: AtomicBool = AtomicBool::new(false);
 
+/// RAII guard that resets a per-kind overlap flag back to `false` on drop —
+/// including during a panic-triggered unwind inside the spawned thread — so
+/// a background review can never wedge itself permanently disabled for the
+/// rest of the process if the subagent run panics before reaching its
+/// normal completion path.
+struct RunningGuard(&'static AtomicBool);
+
+impl Drop for RunningGuard {
+    fn drop(&mut self) {
+        self.0.store(false, Ordering::SeqCst);
+    }
+}
+
 /// ─── Helpers ───
 ///
 /// Check whether a file path is worth auto-reviewing (not config/lock/data).
@@ -194,8 +207,14 @@ pub fn spawn_quick_review(
 /// silently swallowing the error into a note string, so a single transient
 /// LLM/tool failure doesn't just disappear.
 ///
+/// `abort_flag` is checked before every attempt (including the first) and
+/// forwarded into the subagent's own context, so a cancelled turn stops
+/// retrying immediately instead of burning a second attempt.
+///
 /// Return: `Ok(output)` if either attempt succeeded, `Err(message)`
-/// describing the final failure if both attempts failed.
+/// describing the final failure if both attempts failed, or the literal
+/// message `"aborted by user"` if `abort_flag` was already set before an
+/// attempt could start.
 fn run_subagent_with_retry(
     def: &AgentDefinition,
     session_dir: &Path,
@@ -239,6 +258,11 @@ fn run_subagent_with_retry(
 /// Uses the test-generator prompt and has read-write access so it can
 /// create test files. Runs in a separate OS thread and reports completion
 /// via `TurnEvent::SystemNote { kind: "bg-test-gen" }`.
+///
+/// Skipped (no-op) if a test-gen run is already in flight (guarded by
+/// `TEST_GEN_RUNNING`) — prevents a chatty multi-turn edit session from
+/// stacking overlapping runs. `abort_flag` is forwarded to
+/// `run_subagent_with_retry` so the run can be cancelled if the turn aborts.
 pub fn spawn_background_test_gen(
     file_paths: &[String],
     session_dir: &Path,
@@ -260,6 +284,7 @@ pub fn spawn_background_test_gen(
     let events = turn_events.clone();
 
     std::thread::spawn(move || {
+        let _running_guard = RunningGuard(&TEST_GEN_RUNNING);
         tracing::info!(
             "[bg-test-gen] spawning for {} file(s): {:?}",
             paths.len(),
@@ -296,7 +321,6 @@ pub fn spawn_background_test_gen(
                 message,
             });
         }
-        TEST_GEN_RUNNING.store(false, Ordering::SeqCst);
     });
 }
 
@@ -305,6 +329,10 @@ pub fn spawn_background_test_gen(
 /// Inspects the modified files for architectural consistency (layering,
 /// coupling, module boundaries). Reports via
 /// `TurnEvent::SystemNote { kind: "bg-arch-review" }`.
+///
+/// Skipped (no-op) if an arch-review run is already in flight (guarded by
+/// `ARCH_REVIEW_RUNNING`). `abort_flag` is forwarded to
+/// `run_subagent_with_retry` so the run can be cancelled if the turn aborts.
 pub fn spawn_background_arch_review(
     file_paths: &[String],
     session_dir: &Path,
@@ -326,6 +354,7 @@ pub fn spawn_background_arch_review(
     let events = turn_events.clone();
 
     std::thread::spawn(move || {
+        let _running_guard = RunningGuard(&ARCH_REVIEW_RUNNING);
         let file_list = paths.join("\n");
         let prompt = format!(
             "{}\n\nModified files for architecture review:\n{}",
@@ -356,7 +385,6 @@ pub fn spawn_background_arch_review(
                 message,
             });
         }
-        ARCH_REVIEW_RUNNING.store(false, Ordering::SeqCst);
     });
 }
 
@@ -364,6 +392,10 @@ pub fn spawn_background_arch_review(
 ///
 /// Checks modified files for security vulnerabilities. Reports via
 /// `TurnEvent::SystemNote { kind: "bg-security-review" }`.
+///
+/// Skipped (no-op) if a security-review run is already in flight (guarded by
+/// `SECURITY_REVIEW_RUNNING`). `abort_flag` is forwarded to
+/// `run_subagent_with_retry` so the run can be cancelled if the turn aborts.
 pub fn spawn_background_security_review(
     file_paths: &[String],
     session_dir: &Path,
@@ -397,6 +429,7 @@ pub fn spawn_background_security_review(
     let events = turn_events.clone();
 
     std::thread::spawn(move || {
+        let _running_guard = RunningGuard(&SECURITY_REVIEW_RUNNING);
         let file_list = paths.join("\n");
         let prompt = format!(
             "{}\n\nModified files for security review:\n{}",
@@ -427,7 +460,6 @@ pub fn spawn_background_security_review(
                 message,
             });
         }
-        SECURITY_REVIEW_RUNNING.store(false, Ordering::SeqCst);
     });
 }
 
@@ -437,6 +469,9 @@ pub fn spawn_background_security_review(
 /// Flow: always spawns arch-review and security-review if there are
 /// reviewable production files → spawns test-gen only if there are source
 /// files that aren't already tests.
+///
+/// `abort_flag` is cloned and forwarded to all three spawn calls so a
+/// single cancellation source stops every kind of background review.
 pub fn spawn_all_background(
     file_paths: &[String],
     session_dir: &Path,
@@ -516,4 +551,19 @@ mod tests {
         assert!(!is_production_code("README.md"));
         assert!(is_production_code("src/main.rs"));
     }
+
+    #[test]
+    fn running_guard_resets_flag_on_drop_even_after_panic() {
+        static TEST_FLAG: AtomicBool = AtomicBool::new(false);
+        TEST_FLAG.store(true, Ordering::SeqCst);
+        let result = std::panic::catch_unwind(|| {
+            let _guard = RunningGuard(&TEST_FLAG);
+            panic!("simulated failure inside guarded region");
+        });
+        assert!(result.is_err());
+        assert!(
+            !TEST_FLAG.load(Ordering::SeqCst),
+            "guard must reset the flag even when the guarded closure panics"
+        );
+    }
 }