Skip to main content

orch/engine/runner/
response.rs

1//! Response collection + error classification.
2//!
3//! After the agent finishes (tmux session ends), this module:
4//! 1. Reads the output file
5//! 2. Parses the response JSON
6//! 3. Classifies errors (timeout, usage limit, auth, tooling)
7//! 4. Determines next action (success, reroute, needs_review)
8
9use crate::store;
10use crate::store::TaskStore;
11use std::collections::HashSet;
12use std::collections::hash_map::DefaultHasher;
13use std::fs;
14use std::fs::File;
15use std::hash::Hash;
16use std::hash::Hasher;
17use std::io::Read;
18use std::io::Seek;
19use std::io::SeekFrom;
20use std::path::{Path, PathBuf};
21use std::result;
22use std::sync::Arc;
23use std::thread;
24use std::time::Duration;
25use std::time::SystemTime;
26
27use tokio::fs as tokio_fs;
28use tokio::task;
29use tokio::time;
30// Cooldown tracking is implemented in `crate::engine::cooldown` (shared with the router).
31use crate::cli::ndjson;
32use crate::cmd_cache;
33use crate::config as app_config;
34use crate::engine;
35pub use crate::engine::cooldown::{
36    clear_expired_cooldowns, is_agent_in_cooldown, is_model_in_cooldown,
37    record_agent_failure_with_message, record_model_failure, set_agent_cooldown,
38};
39use crate::engine::router::config;
40use crate::home;
41use crate::parser::AgentResponse;
42use crate::store::MemoryEntry;
43use crate::store::Task;
44
45const JITTER_FACTOR: f64 = 0.3;
46
47/// Maximum output file size to read (10 MB).
48/// Prevents runaway outputs from consuming excessive memory.
49const MAX_OUTPUT_SIZE_BYTES: u64 = 10 * 1024 * 1024;
50
51/// Read a file with a size limit, returning truncated content if the file is too large.
52///
53/// This prevents runaway agent outputs from causing memory issues. If the file
54/// exceeds the limit, reads from the end of the file to capture the most recent
55/// output (which is typically where the result or error message is).
56fn read_file_with_limit<P: AsRef<Path>>(path: P, max_bytes: u64) -> anyhow::Result<String> {
57    let path = path.as_ref();
58    let metadata = fs::metadata(path)?;
59
60    if !metadata.is_file() {
61        anyhow::bail!("not a file: {}", path.display());
62    }
63
64    let file_size = metadata.len();
65
66    // If file is within limits, read the whole thing
67    if file_size <= max_bytes {
68        return fs::read_to_string(path).map_err(Into::into);
69    }
70
71    // File is too large - read from the end to get the most recent output
72    let mut file = File::open(path)?;
73    let start_pos = file_size.saturating_sub(max_bytes);
74    file.seek(SeekFrom::Start(start_pos))?;
75
76    let mut buffer = Vec::with_capacity(usize::try_from(max_bytes).unwrap_or(0));
77    file.read_to_end(&mut buffer)?;
78
79    // Try to convert to UTF-8, replacing invalid sequences
80    let content = String::from_utf8_lossy(&buffer);
81
82    // Log a warning about truncation
83    tracing::warn!(
84        path = %path.display(),
85        file_size = file_size,
86        max_bytes = max_bytes,
87        "output file truncated: exceeded size limit"
88    );
89
90    Ok(content.into_owned())
91}
92
93/// Calculate exponential backoff with jitter for fallback retries.
94/// Uses retry count from the reroute chain to determine the delay.
95///
96/// Delay = min(base * 2^retry_count, max) * (1 + jitter)
97pub(crate) fn calculate_backoff_delay(retry_count: usize) -> u64 {
98    let base_delay = config::retry_base_delay_ms();
99    let max_delay = config::retry_max_delay_ms();
100
101    // Exponential: base * 2^retry_count
102    let exponential =
103        base_delay * (2_u64.saturating_pow(u32::try_from(retry_count).unwrap_or(u32::MAX)));
104    let capped = exponential.min(max_delay);
105
106    // Add jitter: ±30% using a hash-based pseudo-random value.
107    // Mix system time (nanoseconds) with thread ID so concurrent retries on
108    // different threads get different delays even when called in the same
109    // microsecond, avoiding the thundering-herd problem.
110    #[expect(
111        clippy::cast_precision_loss,
112        clippy::cast_possible_truncation,
113        clippy::cast_sign_loss,
114        reason = "a jitter range in milliseconds, far below 2^52 and never negative"
115    )]
116    let jitter_range = (capped as f64 * JITTER_FACTOR) as u64;
117    let jitter = {
118        let mut h = DefaultHasher::new();
119        SystemTime::now().hash(&mut h);
120        thread::current().id().hash(&mut h);
121        h.finish() % (jitter_range * 2 + 1)
122    };
123
124    capped.saturating_sub(jitter_range) + jitter
125}
126
127fn fallback_retry_count(task: Option<&Task>) -> usize {
128    task.map_or(0, |t| {
129        let chain_retry_count = t
130            .limit_reroute_chain
131            .split(',')
132            .filter(|entry| !entry.is_empty())
133            .count();
134        let network_retry_count = usize::try_from(t.network_retries).unwrap_or(0);
135        chain_retry_count.max(network_retry_count)
136    })
137}
138
139/// Wait with exponential backoff before retrying with a fallback agent.
140/// Returns the delay applied in milliseconds.
141pub async fn wait_for_fallback_backoff(
142    task_id: &str,
143    store: &Option<Arc<TaskStore>>,
144    repo: &str,
145) -> u64 {
146    let task = store::opt_store_get_task(store, repo, task_id).await;
147
148    // Normal failover uses reroute chain length. NetworkError retries bypass
149    // failover, so track their own streak and use whichever retry source is larger.
150    let retry_count = fallback_retry_count(task.as_ref());
151
152    let delay = calculate_backoff_delay(retry_count);
153
154    if delay > 0 {
155        tracing::debug!(
156            task_id,
157            retry_count = retry_count,
158            delay_ms = delay,
159            "fallback backoff: waiting before retry"
160        );
161        time::sleep(Duration::from_millis(delay)).await;
162    }
163
164    delay
165}
166
167/// Outcome signal for the engine to update router weights.
168///
169/// Returned by the task runner so the engine can feed rate limit and
170/// success signals back to the router's weighted round-robin implementation.
171#[derive(Debug, Clone)]
172pub enum WeightSignal {
173    /// Agent completed a task successfully.
174    Success { agent: String },
175    /// Agent hit a rate limit / usage limit.
176    RateLimited { agent: String },
177    /// The run failed (timeout, no fallback, exhausted agents) and ended in `needs_review`.
178    /// Never read as a completed run, so the no-PR shortcut cannot mark the task done.
179    Failed { agent: String },
180    /// Task is blocked on child tasks (delegations). Do not trigger review agent.
181    Blocked,
182    /// Task was rerouted (silence detection, timeout, parse error) — reset to new for re-routing.
183    /// Distinct from None so the engine never converts a silent-reset task into needs_review.
184    Rerouted,
185    /// No weight-relevant signal (guard skipped task, unrecognised status, etc.)
186    None,
187}
188
189/// Read the agent's output file, trying multiple locations.
190///
191/// Checks per-task attempt directory first, then legacy flat paths.
192/// Output is truncated to `MAX_OUTPUT_SIZE_BYTES` to prevent runaway memory usage.
193pub async fn read_output_file(task_id: &str, primary_path: &Path, repo: &str) -> String {
194    // Primary: explicit output file (already points to attempt dir)
195    if let Some(content) = task::spawn_blocking({
196        let p = primary_path.to_path_buf();
197        move || read_file_with_limit(p, MAX_OUTPUT_SIZE_BYTES)
198    })
199    .await
200    .ok()
201    .and_then(result::Result::ok)
202        && !content.is_empty()
203    {
204        return content;
205    }
206
207    // Fallback: check all attempt dirs for this task (newest first)
208    if let Ok(task_dir) = home::task_dir_async(repo, task_id).await {
209        let attempts_dir = task_dir.join("attempts");
210        // Use async metadata for is_dir check
211        if tokio_fs::metadata(&attempts_dir)
212            .await
213            .is_ok_and(|m| m.is_dir())
214        {
215            let mut attempt_nums: Vec<u32> = task::spawn_blocking({
216                let a = attempts_dir.clone();
217                move || {
218                    match fs::read_dir(&a) {
219                        Ok(entries) => {
220                            let mut nums = Vec::new();
221                            for entry in entries {
222                                match entry {
223                                    Ok(e) => {
224                                        if let Some(n) =
225                                            e.file_name().to_str().and_then(|n| n.parse().ok())
226                                        {
227                                            nums.push(n);
228                                        }
229                                    }
230                                    Err(e) => tracing::warn!(
231                                        path = %a.display(),
232                                        err = %e,
233                                        "error reading attempt dir entry in output fallback"
234                                    ),
235                                }
236                            }
237                            nums
238                        }
239                        Err(e) => {
240                            tracing::warn!(path = %a.display(), err = %e, "failed to read attempts dir in output fallback");
241                            Vec::new()
242                        }
243                    }
244                }
245            })
246            .await
247            .unwrap_or_default();
248
249            attempt_nums.sort_unstable_by(|a, b| b.cmp(a)); // newest first
250            for n in attempt_nums {
251                let p = attempts_dir.join(n.to_string()).join("output.json");
252                if let Some(content) = task::spawn_blocking({
253                    let p = p.clone();
254                    move || read_file_with_limit(p, MAX_OUTPUT_SIZE_BYTES)
255                })
256                .await
257                .ok()
258                .and_then(result::Result::ok)
259                    && !content.is_empty()
260                {
261                    tracing::info!(task_id, path = %p.display(), "read output from attempt dir");
262                    return content;
263                }
264            }
265        }
266    }
267
268    // Legacy fallback locations
269    let state_dir = home::state_dir().unwrap_or_else(|_| PathBuf::from("/tmp"));
270
271    let mut fallbacks = vec![
272        PathBuf::from(format!("/tmp/output-{task_id}.json")),
273        state_dir.join(format!("output-{task_id}.json")),
274    ];
275
276    if let Ok(legacy_path) = home::state_file(&format!("output-{task_id}.json"))
277        && !fallbacks.contains(&legacy_path)
278    {
279        fallbacks.push(legacy_path);
280    }
281
282    for path in &fallbacks {
283        if let Some(content) = task::spawn_blocking({
284            let p = path.clone();
285            move || read_file_with_limit(p, MAX_OUTPUT_SIZE_BYTES)
286        })
287        .await
288        .ok()
289        .and_then(result::Result::ok)
290            && !content.is_empty()
291        {
292            tracing::info!(task_id, path = %path.display(), "read output from legacy fallback");
293            return content;
294        }
295    }
296
297    String::new()
298}
299
300/// Pick a fallback agent, avoiding agents already in the reroute chain and agents in cooldown.
301pub fn pick_fallback_agent(
302    current_agent: &str,
303    chain: &str,
304    available_agents: &[String],
305) -> Option<String> {
306    // Clear expired cooldowns first
307    clear_expired_cooldowns();
308
309    let chain_set: HashSet<&str> = if chain.is_empty() {
310        HashSet::new()
311    } else {
312        chain.split(',').collect()
313    };
314
315    // Prefer agents that are not in cooldown and are not heavily degraded
316    // from recent rate-limits. This prevents immediate failover to agents
317    // that have been heavily penalized and likely to fail as well.
318    for agent in available_agents {
319        if agent == current_agent || chain_set.contains(agent.as_str()) {
320            continue;
321        }
322        if is_agent_in_cooldown(agent) {
323            continue;
324        }
325        // Weight-based filtering was removed from this module because it used
326        // a default-constructed AgentWeights (dead code that always returned
327        // true). Keep failover behavior simple here: prefer the first agent
328        // that is not in cooldown and not in the reroute chain. If we later
329        // expose the router's AgentWeights snapshot to this module we can add
330        // weight-based avoidance back in a non-dead way.
331        return Some(agent.clone());
332    }
333
334    None
335}
336
337/// Retryable error types that should trigger agent failover.
338#[derive(Debug, Clone, Copy, PartialEq, Eq)]
339pub enum RetryableError {
340    /// Agent timed out.
341    Timeout,
342    /// Run exited without a result envelope
343    Interrupted,
344    /// Usage/rate limit hit.
345    UsageLimit,
346    /// Auth/billing error.
347    AuthError,
348    /// General failure (non-retryable but we still try fallback agents).
349    Failed,
350    /// Missing tooling - fallback might help if another agent has the tool.
351    MissingTooling,
352    /// A specific model was unavailable (e.g. "Model not found"). The agent
353    /// itself is fine — only the model is in cooldown. Failover may still pick
354    /// a different agent, but we must not bump the agent-level failure counter.
355    ModelUnavailable,
356}
357
358impl RetryableError {
359    /// Return a short classified string for this error type.
360    /// Used in DB rate-limit metrics and test assertions.
361    pub fn type_str(self) -> &'static str {
362        match self {
363            RetryableError::Timeout => "timeout",
364            RetryableError::Interrupted => "interrupted",
365            RetryableError::UsageLimit => "rate_limit",
366            RetryableError::AuthError => "auth_error",
367            RetryableError::ModelUnavailable => "model_unavailable",
368            RetryableError::Failed | RetryableError::MissingTooling => "failed",
369        }
370    }
371}
372
373/**
374 * Handle failover for any retryable error type. Returns "new" on reroute (or
375 * retryable exhaustion), "blocked" on hard exhaustion, "needs_review" when no
376 * fallback agents are available. `credit_recorded` means the caller already ran
377 * `record_credit_exhaustion`; the generic `failure_count:` counter must not be
378 * bumped here or the next ordinary failure starts at the extended backoff tier
379 * (issue #3652). Rate-limit event DB recording is done by the caller (mod.rs)
380 **/
381pub async fn handle_failover(
382    task_id: &str,
383    agent_name: &str,
384    error_type: RetryableError,
385    error_message: &str,
386    credit_recorded: bool,
387    store: &Option<Arc<TaskStore>>,
388    repo: &str,
389) -> String {
390    // Get the reroute chain
391    let chain = get_reroute_chain(task_id, store, repo).await;
392    let chain = update_reroute_chain(task_id, agent_name, &chain, store, repo).await;
393
394    if error_type == RetryableError::Timeout
395        && let Some(status) =
396            block_after_repeated_timeouts(task_id, error_message, store, repo).await
397    {
398        return status;
399    }
400
401    // Get all available agents
402    let available: Vec<String> = engine::configured_agents()
403        .into_iter()
404        .filter(|a| cmd_cache::command_exists(a))
405        .collect();
406
407    // Check if we've exhausted all agents (build chain_set once)
408    let chain_set: HashSet<&str> = if chain.is_empty() {
409        HashSet::new()
410    } else {
411        chain.split(',').collect()
412    };
413    let is_exhausted = available.iter().all(|a| chain_set.contains(a.as_str()));
414
415    if is_exhausted {
416        // Retryable failures (rate limits, quota, model unavailable) will resolve once
417        // cooldowns expire — reset to "new" so the router picks it up again then.
418        // Hard failures (generic agent crash, missing tooling) won't improve on retry;
419        // block immediately with a clear reason rather than wasting 63 minutes in the
420        // review-refire loop before getting a misleading "review agent rebroadcast" message.
421        let is_retryable = matches!(
422            error_type,
423            RetryableError::UsageLimit
424                | RetryableError::AuthError
425                | RetryableError::ModelUnavailable
426                | RetryableError::Timeout
427                | RetryableError::Interrupted
428        );
429        let next_status = if is_retryable { "new" } else { "blocked" };
430
431        tracing::warn!(
432            task_id,
433            agent = agent_name,
434            error_type = ?error_type,
435            next_status,
436            "all agents exhausted"
437        );
438
439        let mut store_fields: Vec<(&str, serde_json::Value)> = Vec::new();
440
441        if is_retryable {
442            // Clear reroute chain so the next dispatch builds a fresh one rather than
443            // immediately seeing "all agents in chain" and exhausting again.
444            let msg = format!("{error_message} (all agents exhausted, will retry when available)");
445            store_fields.push(("last_error", serde_json::json!(msg)));
446            store_fields.push(("limit_reroute_chain", serde_json::json!("")));
447        } else {
448            let msg = format!("{error_message} (all agents exhausted)");
449            let block_reason = format!(
450                "all agents exhausted: {} — {}",
451                error_type.type_str(),
452                error_message
453            );
454            store_fields.push(("last_error", serde_json::json!(msg)));
455            store_fields.push(("block_reason", serde_json::json!(block_reason)));
456        }
457
458        if let Err(e) = store::store_set_result(store, repo, task_id, &store_fields).await {
459            tracing::warn!(task_id, err = %e, "failed to write agents_exhausted fields to store");
460        }
461
462        // Log reroute activity for exhausted agents
463        let details = serde_json::json!({
464            "failure_reason": error_type.type_str(),
465            "error_message": error_message,
466            "chain": chain,
467            "agents_exhausted": true,
468            "is_retryable": is_retryable,
469        });
470        store::store_log_activity(
471            store,
472            repo,
473            task_id,
474            "rerouted",
475            Some("in_progress"),
476            Some(next_status),
477            None::<&str>,
478            None::<&str>,
479            Some(&details),
480        )
481        .await;
482
483        return next_status.to_string();
484    }
485
486    // Pick a fallback agent
487    if let Some(next) = pick_fallback_agent(agent_name, &chain, &available) {
488        tracing::info!(
489            task_id,
490            from = agent_name,
491            to = next,
492            error_type = ?error_type,
493            "failover: switching to fallback agent"
494        );
495
496        // Timeouts and interrupts are not agent faults, skip the agent cooldown
497        if !matches!(
498            error_type,
499            RetryableError::MissingTooling
500                | RetryableError::ModelUnavailable
501                | RetryableError::Timeout
502                | RetryableError::Interrupted
503        ) {
504            // Credit errors skip the generic counter (already counted via
505            // credit_failure_count); the 120s cooldown below still applies
506            if !credit_recorded {
507                record_agent_failure_with_message(agent_name, error_message).await;
508            }
509
510            // Apply brief 120s cooldown on the failed agent so the router skips it
511            // on the next routing attempt. This prevents the router from immediately
512            // re-selecting the same agent after clearing both agent and model (#1492).
513            // Also skipped for ModelUnavailable: the agent is healthy.
514            set_agent_cooldown(agent_name, 120).await;
515        }
516
517        let msg = format!("{error_message}, clearing agent/model for re-route");
518        if let Err(e) = store::store_set_result(
519            store,
520            repo,
521            task_id,
522            &[
523                ("agent", serde_json::json!("")),
524                ("model", serde_json::json!("")),
525                ("last_error", serde_json::json!(msg)),
526            ],
527        )
528        .await
529        {
530            tracing::warn!(task_id, err = %e, "failed to write agent/model clear to store");
531        }
532
533        // Apply exponential backoff with jitter before retry.
534        // Return "new" (not "routed") so the task goes through Phase 3a routing
535        // again. This ensures the router actually re-selects a compatible agent+model
536        // via model_for_complexity() instead of bypassing routing and defaulting to
537        // claude in the dispatch loop (#1604).
538        wait_for_fallback_backoff(task_id, store, repo).await;
539
540        // Log reroute activity for successful failover
541        let details = serde_json::json!({
542            "failure_reason": error_type.type_str(),
543            "error_message": error_message,
544            "from_agent": agent_name,
545            "to_agent": next,
546            "chain": chain,
547        });
548        store::store_log_activity(
549            store,
550            repo,
551            task_id,
552            "rerouted",
553            Some("in_progress"),
554            Some("new"),
555            Some(&next),
556            None::<&str>,
557            Some(&details),
558        )
559        .await;
560
561        return "new".to_string();
562    }
563
564    // No fallback available
565    tracing::warn!(task_id, agent = agent_name, "no fallback agents available");
566
567    let is_retryable = matches!(
568        error_type,
569        RetryableError::UsageLimit
570            | RetryableError::AuthError
571            | RetryableError::ModelUnavailable
572            | RetryableError::Timeout
573            | RetryableError::Interrupted
574    );
575
576    // Timeouts and interrupts are not agent faults, skip the agent cooldown
577    if !matches!(
578        error_type,
579        RetryableError::MissingTooling
580            | RetryableError::ModelUnavailable
581            | RetryableError::Timeout
582            | RetryableError::Interrupted
583    ) && !credit_recorded
584    {
585        record_agent_failure_with_message(agent_name, error_message).await;
586    }
587
588    if is_retryable {
589        // The other agents are in cooldown, so wait out the backoff and let the router
590        // pick the task up again. A fresh chain keeps the next run from seeing every agent as tried
591        wait_for_fallback_backoff(task_id, store, repo).await;
592
593        let msg = format!("{error_message} (no fallback agents, will retry)");
594        if let Err(e) = store::store_set_result(
595            store,
596            repo,
597            task_id,
598            &[
599                ("last_error", serde_json::json!(msg)),
600                ("limit_reroute_chain", serde_json::json!("")),
601            ],
602        )
603        .await
604        {
605            tracing::warn!(task_id, err = %e, "failed to write no_fallback retry fields to store");
606        }
607
608        let details = serde_json::json!({
609            "failure_reason": error_type.type_str(),
610            "error_message": error_message,
611            "chain": chain,
612            "no_fallback_available": true,
613            "is_retryable": true,
614        });
615        store::store_log_activity(
616            store,
617            repo,
618            task_id,
619            "rerouted",
620            Some("in_progress"),
621            Some("new"),
622            None::<&str>,
623            None::<&str>,
624            Some(&details),
625        )
626        .await;
627
628        return "new".to_string();
629    }
630
631    let msg = format!("{error_message}, no fallback agents");
632    if let Err(e) = store::store_set_result(
633        store,
634        repo,
635        task_id,
636        &[("last_error", serde_json::json!(msg))],
637    )
638    .await
639    {
640        tracing::warn!(task_id, err = %e, "failed to write no_fallback last_error to store");
641    }
642
643    // Log reroute activity for no fallback available
644    let details = serde_json::json!({
645        "failure_reason": error_type.type_str(),
646        "error_message": error_message,
647        "chain": chain,
648        "no_fallback_available": true,
649    });
650    store::store_log_activity(
651        store,
652        repo,
653        task_id,
654        "rerouted",
655        Some("in_progress"),
656        Some("needs_review"),
657        None::<&str>,
658        None::<&str>,
659        Some(&details),
660    )
661    .await;
662
663    "needs_review".to_string()
664}
665
666/// Count a timeout against the task. Returns "blocked" once the task has timed out
667/// `workflow.max_timeout_retries` times (default 3), so a task that always times out does not loop
668pub async fn block_after_repeated_timeouts(
669    task_id: &str,
670    error_message: &str,
671    store: &Option<Arc<TaskStore>>,
672    repo: &str,
673) -> Option<String> {
674    let max: i64 = app_config::get("workflow.max_timeout_retries")
675        .ok()
676        .and_then(|v| v.parse().ok())
677        .unwrap_or(3);
678    let count = match store::store_increment(store, repo, task_id, "timeout_retries").await {
679        Ok(count) => i64::try_from(count).unwrap_or(i64::MAX),
680        Err(e) => {
681            tracing::warn!(task_id, err = %e, "failed to increment timeout_retries");
682            return None;
683        }
684    };
685    if count < max {
686        return None;
687    }
688
689    let block_reason = format!("timed out {count} times");
690    tracing::warn!(
691        task_id,
692        count,
693        max,
694        "timeout retries exhausted, blocking task"
695    );
696    if let Err(e) = store::store_set_result(
697        store,
698        repo,
699        task_id,
700        &[
701            (
702                "last_error",
703                serde_json::json!(format!("{error_message} ({block_reason})")),
704            ),
705            ("block_reason", serde_json::json!(block_reason)),
706            ("limit_reroute_chain", serde_json::json!("")),
707        ],
708    )
709    .await
710    {
711        tracing::warn!(task_id, err = %e, "failed to write timeout block fields to store");
712    }
713    let details = serde_json::json!({
714        "failure_reason": "timeout",
715        "error_message": error_message,
716        "timeout_retries": count,
717    });
718    store::store_log_activity(
719        store,
720        repo,
721        task_id,
722        "rerouted",
723        Some("in_progress"),
724        Some("blocked"),
725        None::<&str>,
726        None::<&str>,
727        Some(&details),
728    )
729    .await;
730    Some("blocked".to_string())
731}
732
733/// Get the reroute chain from store.
734pub async fn get_reroute_chain(
735    task_id: &str,
736    store: &Option<Arc<TaskStore>>,
737    repo: &str,
738) -> String {
739    store::opt_store_get_task(store, repo, task_id)
740        .await
741        .map(|t| t.limit_reroute_chain)
742        .unwrap_or_default()
743        .trim()
744        .to_string()
745}
746
747/// Update the reroute chain in store.
748pub async fn update_reroute_chain(
749    task_id: &str,
750    current_agent: &str,
751    existing_chain: &str,
752    store: &Option<Arc<TaskStore>>,
753    repo: &str,
754) -> String {
755    let mut chain = existing_chain.to_string();
756    if chain.is_empty() {
757        chain = current_agent.to_string();
758    } else if !chain.split(',').any(|a| a == current_agent) {
759        chain = format!("{chain},{current_agent}");
760    }
761
762    if let Err(e) = store::store_set_result(
763        store,
764        repo,
765        task_id,
766        &[("limit_reroute_chain", serde_json::json!(chain))],
767    )
768    .await
769    {
770        tracing::warn!(task_id, err = %e, "failed to write limit_reroute_chain to store");
771    }
772    chain
773}
774
775/// Review response from the review agent.
776#[derive(Debug, Clone, serde::Deserialize, serde::Serialize)]
777pub struct ReviewResponse {
778    /// Decision: "approve" | "request_changes"
779    pub decision: String,
780    /// Detailed review feedback.
781    pub notes: String,
782    /// Test results: "pass" | "fail" | "skipped"
783    pub test_results: Option<String>,
784    /// List of issues found.
785    pub issues: Vec<ReviewIssue>,
786}
787
788/// A single issue found during review.
789#[derive(Debug, Clone, serde::Deserialize, serde::Serialize, PartialEq, Eq)]
790pub struct ReviewIssue {
791    /// File path.
792    pub file: String,
793    /// Line number (optional).
794    pub line: Option<u32>,
795    /// Severity: "error" | "warning"
796    pub severity: String,
797    /// Description of the issue.
798    pub description: String,
799}
800
801/// Parse a review response from already-unwrapped text.
802///
803/// Expects the raw result text (already extracted from the agent envelope
804/// by the agent-specific parser). Tries direct JSON parse, then markdown
805/// code block extraction.
806pub fn parse_review_response(text: &str) -> anyhow::Result<ReviewResponse> {
807    let resp = parse_review_json(text)?;
808    if matches!(resp.decision.as_str(), "approve" | "request_changes") {
809        return Ok(resp);
810    }
811
812    // Weak models echo the prompt template ("approve|request_changes"), strip it so it
813    // does not match the "request_changes" keyword
814    let stripped = text.replace("approve|request_changes", "");
815    infer_review_response_from_text(&stripped)
816        .ok_or_else(|| anyhow::anyhow!("invalid review decision: {}", resp.decision))
817}
818
819fn parse_review_json(text: &str) -> anyhow::Result<ReviewResponse> {
820    // Try direct JSON parse
821    if let Ok(resp) = serde_json::from_str::<ReviewResponse>(text) {
822        return Ok(resp);
823    }
824
825    // Try extracting JSON from markdown code blocks
826    if let Some(json_str) = extract_json_block(text)
827        && let Ok(resp) = serde_json::from_str::<ReviewResponse>(&json_str)
828    {
829        return Ok(resp);
830    }
831
832    if let Some(resp) = extract_review_response_object(text) {
833        return Ok(resp);
834    }
835
836    anyhow::bail!("failed to parse review response")
837}
838
839fn infer_review_response_from_text(text: &str) -> Option<ReviewResponse> {
840    let lower = text.to_ascii_lowercase();
841    let has_changes_requested =
842        lower.contains("changes requested") || lower.contains("request_changes");
843    if has_changes_requested {
844        return Some(ReviewResponse {
845            decision: "request_changes".to_string(),
846            notes: "Inferred decision from plain-text review output.".to_string(),
847            test_results: None,
848            issues: Vec::new(),
849        });
850    }
851
852    // Approval only from an explicit decision token. "approved" and CI-evidence
853    // phrases ("all checks passed") are not a decision, a negation list cannot cover them
854    let positive_approval = lower
855        .split(|c: char| !c.is_ascii_alphabetic())
856        .any(|word| word == "lgtm")
857        || lower.contains("decision is `approve`")
858        || lower.contains("decision: `approve`")
859        || lower.contains("decision is approve")
860        || lower.contains("decision: approve");
861    if positive_approval {
862        return Some(ReviewResponse {
863            decision: "approve".to_string(),
864            notes: "Inferred decision from plain-text review output.".to_string(),
865            test_results: None,
866            issues: Vec::new(),
867        });
868    }
869
870    None
871}
872
873/// Public entry point for keyword-based review response inference.
874///
875/// Used by `review.rs` when `parse_review_response` fails on the extracted text.
876pub fn infer_review_response(text: &str) -> Option<ReviewResponse> {
877    infer_review_response_from_text(text)
878}
879
880/// Extract the first valid JSON object from markdown code blocks.
881///
882/// Searches for ```json blocks and returns the first one whose content
883/// starts with `{`, skipping non-JSON code blocks.
884fn extract_json_block(text: &str) -> Option<String> {
885    let mut search_from = 0;
886    while let Some(start) = text[search_from..].find("```json") {
887        let abs_start = search_from + start;
888        let after_tag = abs_start + "```json".len();
889        let Some(rest) = text[after_tag..].strip_prefix('\n') else {
890            search_from = after_tag;
891            continue;
892        };
893        let content_start = text.len() - rest.len();
894        let Some(end_offset) = rest.find("```") else {
895            search_from = content_start;
896            continue;
897        };
898        let end = content_start + end_offset;
899        let content = text[content_start..end].trim();
900        if content.starts_with('{') {
901            return Some(content.to_string());
902        }
903        search_from = end + 3;
904    }
905    None
906}
907
908/// Extract a ReviewResponse JSON object from arbitrary text.
909///
910/// Scans for balanced JSON objects and tries to parse them as ReviewResponse.
911fn extract_review_response_object(text: &str) -> Option<ReviewResponse> {
912    let mut start: Option<usize> = None;
913    let mut depth: i32 = 0;
914    let mut in_string = false;
915    let mut escape = false;
916
917    for (idx, ch) in text.char_indices() {
918        if start.is_some() {
919            if in_string {
920                if escape {
921                    escape = false;
922                } else if ch == '\\' {
923                    escape = true;
924                } else if ch == '"' {
925                    in_string = false;
926                }
927                continue;
928            }
929
930            match ch {
931                '"' => in_string = true,
932                '{' => depth += 1,
933                '}' => {
934                    depth -= 1;
935                    if depth == 0 {
936                        debug_assert!(
937                            start.is_some(),
938                            "start must be Some when depth returns to 0"
939                        );
940                        let start_idx = start.unwrap_or(0);
941                        let candidate = &text[start_idx..=idx];
942                        if let Ok(resp) = serde_json::from_str::<ReviewResponse>(candidate) {
943                            return Some(resp);
944                        }
945                        start = None;
946                    }
947                }
948                _ => {}
949            }
950            continue;
951        }
952
953        if ch == '{' {
954            start = Some(idx);
955            depth = 1;
956            in_string = false;
957            escape = false;
958        }
959    }
960
961    None
962}
963
964/// Extract learnings and store as memory for future attempts.
965#[expect(
966    clippy::too_many_arguments,
967    reason = "a memory entry records the task, attempt, agent, model and outcome"
968)]
969pub async fn store_learnings_from_response(
970    task_id: &str,
971    attempt: u32,
972    agent: &str,
973    model: Option<&str>,
974    response: &AgentResponse,
975    error: Option<&str>,
976    store: &Option<Arc<TaskStore>>,
977    repo: &str,
978    raw_stdout: &str,
979) {
980    // Use agent self-reported files; fall back to NDJSON tool-call extraction when empty.
981    let files_modified = if response.files.is_empty() {
982        let extracted = ndjson::extract_files_changed(raw_stdout);
983        if !extracted.is_empty() {
984            tracing::debug!(
985                task_id,
986                count = extracted.len(),
987                "files_modified: fell back to NDJSON extraction (agent reported none)"
988            );
989        }
990        extracted
991    } else {
992        response.files.clone()
993    };
994
995    // Build the memory entry
996    let entry = MemoryEntry {
997        attempt,
998        agent: agent.to_string(),
999        model: model.map(String::from),
1000        learnings: response.learnings.clone(),
1001        error: error.map(String::from),
1002        files_modified,
1003        approach: response.summary.clone(),
1004        timestamp: chrono::Utc::now().format("%Y-%m-%dT%H:%M:%SZ").to_string(),
1005    };
1006
1007    if let Some(st) = store
1008        && let Ok(Some(store_id)) = st.resolve_task_id(repo, task_id).await
1009    {
1010        match st.append_memory(store_id, &entry).await {
1011            Err(e) => {
1012                tracing::warn!(task_id, error = ?e, "failed to store memory");
1013            }
1014            _ => {
1015                tracing::debug!(task_id, attempt, "stored memory for attempt");
1016            }
1017        }
1018    }
1019}
1020
1021/// Store a memory entry for a failed attempt (without a full AgentResponse).
1022pub async fn store_failure_memory(
1023    task_id: &str,
1024    attempt: u32,
1025    agent: &str,
1026    model: Option<&str>,
1027    error: &str,
1028    store: &Option<Arc<TaskStore>>,
1029    repo: &str,
1030) {
1031    let entry = MemoryEntry {
1032        attempt,
1033        agent: agent.to_string(),
1034        model: model.map(String::from),
1035        learnings: vec![],
1036        error: Some(error.to_string()),
1037        files_modified: vec![],
1038        approach: String::new(),
1039        timestamp: chrono::Utc::now().format("%Y-%m-%dT%H:%M:%SZ").to_string(),
1040    };
1041
1042    if let Some(st) = store
1043        && let Ok(Some(store_id)) = st.resolve_task_id(repo, task_id).await
1044    {
1045        match st.append_memory(store_id, &entry).await {
1046            Err(e) => {
1047                tracing::warn!(task_id, error = ?e, "failed to store failure memory");
1048            }
1049            _ => {
1050                tracing::debug!(task_id, attempt, "stored failure memory");
1051            }
1052        }
1053    }
1054}
1055
1056#[cfg(test)]
1057mod tests {
1058    use std::env;
1059    #[cfg(unix)]
1060    use std::os::unix::fs::PermissionsExt;
1061    use std::process;
1062
1063    use super::*;
1064    use crate::engine::cooldown;
1065    use crate::engine::runner::agents::{AgentError, find_agent_result, patterns};
1066    use crate::store::TaskStatus;
1067    use serial_test::serial;
1068
1069    #[serial(cooldown_state)]
1070    #[test]
1071    fn calculate_backoff_delay_jitter_is_centered() {
1072        // Run many samples and verify the range is [0.7*capped, 1.3*capped].
1073        // We can't control `now` directly, but calling the function many times
1074        // exercises different microsecond values and lets us check bounds.
1075        let base = config::retry_base_delay_ms();
1076        let max = config::retry_max_delay_ms();
1077
1078        // retry_count=0 → exponential = base, capped = base (assuming base < max)
1079        let capped = base.min(max);
1080        #[expect(
1081            clippy::cast_precision_loss,
1082            clippy::cast_possible_truncation,
1083            clippy::cast_sign_loss,
1084            reason = "mirrors the jitter range in calculate_backoff_delay"
1085        )]
1086        let jitter_range = (capped as f64 * JITTER_FACTOR) as u64;
1087        let lo = capped.saturating_sub(jitter_range);
1088        let hi = capped + jitter_range;
1089
1090        for _ in 0..200 {
1091            let delay = calculate_backoff_delay(0);
1092            assert!(
1093                delay >= lo && delay <= hi,
1094                "delay {delay} out of expected range [{lo}, {hi}]"
1095            );
1096        }
1097    }
1098
1099    #[serial(cooldown_state)]
1100    #[test]
1101    fn fallback_retry_count_uses_larger_network_retry_streak() {
1102        let task = Task {
1103            id: 1,
1104            external_id: None,
1105            repo: "owner/repo".to_string(),
1106            origin: "internal".to_string(),
1107            title: "test".to_string(),
1108            body: String::new(),
1109            status: TaskStatus::New,
1110            source: String::new(),
1111            source_id: String::new(),
1112            author: String::new(),
1113            url: String::new(),
1114            labels: vec![],
1115            agent: None,
1116            model: None,
1117            complexity: "simple".to_string(),
1118            estimate: 0,
1119            route_reason: String::new(),
1120            agent_profile: String::new(),
1121            selected_skills: String::new(),
1122            route_attempts: 0,
1123            attempts: 0,
1124            branch: String::new(),
1125            worktree: String::new(),
1126            worktree_cleaned: false,
1127            owner_node: None,
1128            summary: String::new(),
1129            last_error: String::new(),
1130            parent_id: None,
1131            block_reason: None,
1132            pr_number: None,
1133            pr_review_context: String::new(),
1134            last_review_ts: String::new(),
1135            review_ts_map: "{}".to_string(),
1136            last_comment_review_ts: String::new(),
1137            merge_conflict_retries: 0,
1138            ci_merge_failures: 0,
1139            pr_create_failures: 0,
1140            push_failures: 0,
1141            network_retries: 3,
1142            timeout_retries: 0,
1143            review_agent_failures: 0,
1144            review_resets: 0,
1145            review_cycles: 0,
1146            review_invocations: 0,
1147            needs_review_refires: 0,
1148            input_tokens: 0,
1149            output_tokens: 0,
1150            input_cost_usd: 0.0,
1151            output_cost_usd: 0.0,
1152            total_cost_usd: 0.0,
1153            model_reroute_chain: String::new(),
1154            limit_reroute_chain: "claude".to_string(),
1155            memory: vec![],
1156            delegations: vec![],
1157            auto_unblock_count: 0,
1158            auto_unblock_last_at: String::new(),
1159            auto_unblock_last_reason: String::new(),
1160            ci_recovery_count: 0,
1161            no_code_reroutes: 0,
1162            no_code_last_agent: String::new(),
1163            created_at: String::new(),
1164            updated_at: String::new(),
1165        };
1166
1167        assert_eq!(fallback_retry_count(Some(&task)), 3);
1168    }
1169
1170    #[serial(cooldown_state)]
1171    #[test]
1172    fn retryable_type_str_returns_correct_labels() {
1173        assert_eq!(RetryableError::Timeout.type_str(), "timeout");
1174        assert_eq!(RetryableError::Interrupted.type_str(), "interrupted");
1175        assert_eq!(RetryableError::UsageLimit.type_str(), "rate_limit");
1176        assert_eq!(RetryableError::AuthError.type_str(), "auth_error");
1177        assert_eq!(RetryableError::Failed.type_str(), "failed");
1178        assert_eq!(RetryableError::MissingTooling.type_str(), "failed");
1179    }
1180
1181    #[serial(cooldown_state)]
1182    #[test]
1183    fn is_usage_limit_detects_rate_limit() {
1184        assert!(patterns::detect_rate_limit("Error: rate limit exceeded").is_some());
1185        assert!(patterns::detect_rate_limit("HTTP 429 Too Many Requests").is_some());
1186        assert!(patterns::detect_rate_limit("quota exceeded for model").is_some());
1187        // context_length_exceeded is handled by detect_context_overflow, not detect_rate_limit
1188        assert!(patterns::detect_context_overflow("context_length_exceeded").is_some());
1189    }
1190
1191    #[serial(cooldown_state)]
1192    #[test]
1193    fn is_usage_limit_rejects_normal_text() {
1194        assert!(patterns::detect_rate_limit("task completed successfully").is_none());
1195        assert!(patterns::detect_rate_limit("").is_none());
1196    }
1197
1198    #[serial(cooldown_state)]
1199    #[test]
1200    fn is_auth_error_detects_common_patterns() {
1201        assert!(patterns::detect_auth_error("401 Unauthorized").is_some());
1202        assert!(patterns::detect_auth_error("invalid api key provided").is_some());
1203        assert!(patterns::detect_auth_error("Your billing plan has expired").is_some());
1204        assert!(patterns::detect_auth_error("Error: 403 Forbidden").is_some());
1205    }
1206
1207    #[serial(cooldown_state)]
1208    #[test]
1209    fn is_auth_error_rejects_normal_text() {
1210        assert!(patterns::detect_auth_error("task completed successfully").is_none());
1211        assert!(patterns::detect_auth_error("").is_none());
1212    }
1213
1214    #[serial(cooldown_state)]
1215    #[test]
1216    fn detect_missing_tooling_finds_known_tools() {
1217        let result = patterns::detect_missing_tool("bun: command not found");
1218        assert!(result.is_some());
1219        if let AgentError::MissingTool { tool } = result.unwrap() {
1220            assert_eq!(tool, "bun");
1221        } else {
1222            panic!("expected MissingTool error");
1223        }
1224
1225        let result = patterns::detect_missing_tool("env: anchor: no such file");
1226        assert!(result.is_some());
1227        if let AgentError::MissingTool { tool } = result.unwrap() {
1228            assert_eq!(tool, "anchor");
1229        } else {
1230            panic!("expected MissingTool error");
1231        }
1232
1233        let result = patterns::detect_missing_tool("spawn docker enoent");
1234        assert!(result.is_some());
1235        if let AgentError::MissingTool { tool } = result.unwrap() {
1236            assert_eq!(tool, "docker");
1237        } else {
1238            panic!("expected MissingTool error");
1239        }
1240    }
1241
1242    #[serial(cooldown_state)]
1243    #[test]
1244    fn detect_missing_tooling_returns_none_for_normal() {
1245        assert!(patterns::detect_missing_tool("everything works fine").is_none());
1246        assert!(patterns::detect_missing_tool("").is_none());
1247    }
1248
1249    // ── Cooldown tests ────────────────────────────────────────────
1250
1251    #[serial(cooldown_state)]
1252    #[tokio::test]
1253    async fn record_and_check_agent_cooldown() {
1254        cooldown::reset_global_state().await;
1255        // Use unique names to avoid interference from other tests
1256        let agent = "test_cooldown_agent_1";
1257        assert!(!is_agent_in_cooldown(agent));
1258        record_agent_failure_with_message(agent, "").await;
1259        assert!(is_agent_in_cooldown(agent));
1260    }
1261
1262    #[serial(cooldown_state)]
1263    #[tokio::test]
1264    async fn record_and_check_model_cooldown() {
1265        cooldown::reset_global_state().await;
1266        let agent = "test_cooldown_agent_2";
1267        let model = "test_model_x";
1268        assert!(!is_model_in_cooldown(agent, model));
1269        record_model_failure(agent, model).await;
1270        assert!(is_model_in_cooldown(agent, model));
1271        // Different model should not be in cooldown
1272        assert!(!is_model_in_cooldown(agent, "other_model"));
1273    }
1274
1275    #[serial(cooldown_state)]
1276    #[tokio::test]
1277    async fn clear_expired_does_not_remove_fresh_entries() {
1278        cooldown::reset_global_state().await;
1279        let agent = "test_cooldown_agent_3";
1280        record_agent_failure_with_message(agent, "").await;
1281        clear_expired_cooldowns();
1282        // Should still be in cooldown (just recorded)
1283        assert!(is_agent_in_cooldown(agent));
1284    }
1285
1286    // Use fake agent names so other tests don't interfere.
1287    #[serial(cooldown_state)]
1288    #[test]
1289    fn pick_fallback_skips_current_agent() {
1290        let available = vec!["test_agent_a".to_string(), "test_agent_b".to_string()];
1291        let result = pick_fallback_agent("test_agent_a", "", &available);
1292        assert_eq!(result, Some("test_agent_b".to_string()));
1293    }
1294
1295    #[serial(cooldown_state)]
1296    #[test]
1297    fn pick_fallback_skips_chain_agents() {
1298        let available = vec![
1299            "test_agent_a".to_string(),
1300            "test_agent_b".to_string(),
1301            "test_agent_c".to_string(),
1302        ];
1303        let result = pick_fallback_agent("test_agent_a", "test_agent_a,test_agent_b", &available);
1304        assert_eq!(result, Some("test_agent_c".to_string()));
1305    }
1306
1307    #[serial(cooldown_state)]
1308    #[test]
1309    fn pick_fallback_returns_none_when_exhausted() {
1310        let available = vec!["test_agent_a".to_string(), "test_agent_b".to_string()];
1311        let result = pick_fallback_agent("test_agent_a", "test_agent_a,test_agent_b", &available);
1312        assert!(result.is_none());
1313    }
1314
1315    #[serial(cooldown_state)]
1316    #[test]
1317    fn weight_signal_variants() {
1318        let success = WeightSignal::Success {
1319            agent: "claude".to_string(),
1320        };
1321        let limited = WeightSignal::RateLimited {
1322            agent: "codex".to_string(),
1323        };
1324        let none = WeightSignal::None;
1325
1326        // Verify Debug trait
1327        assert!(format!("{success:?}").contains("claude"));
1328        assert!(format!("{limited:?}").contains("codex"));
1329        assert!(format!("{none:?}").contains("None"));
1330    }
1331
1332    #[serial(cooldown_state)]
1333    #[test]
1334    fn parse_review_response_direct_json() {
1335        let json = r#"{"decision":"approve","notes":"LGTM","test_results":"pass","issues":[]}"#;
1336        let resp = parse_review_response(json).unwrap();
1337        assert_eq!(resp.decision, "approve");
1338        assert_eq!(resp.notes, "LGTM");
1339        assert!(resp.issues.is_empty(), "{:?}", resp.issues);
1340    }
1341
1342    #[serial(cooldown_state)]
1343    #[test]
1344    fn parse_review_response_from_markdown() {
1345        let md = r#"Here is my review:
1346
1347```json
1348{"decision":"request_changes","notes":"Fix the bug","issues":[{"file":"src/main.rs","line":10,"severity":"error","description":"null deref"}]}
1349```
1350
1351That's all."#;
1352        let resp = parse_review_response(md).unwrap();
1353        assert_eq!(resp.decision, "request_changes");
1354        assert_eq!(resp.issues.len(), 1);
1355        assert_eq!(resp.issues[0].file, "src/main.rs");
1356    }
1357
1358    #[serial(cooldown_state)]
1359    #[test]
1360    fn parse_review_response_embedded_json_object() {
1361        let text = "Review complete.\n\n{\"decision\":\"approve\",\"notes\":\"LGTM\",\"test_results\":\"pass\",\"issues\":[]}\nThanks!";
1362        let resp = parse_review_response(text).unwrap();
1363        assert_eq!(resp.decision, "approve");
1364        assert_eq!(resp.notes, "LGTM");
1365    }
1366
1367    #[serial(cooldown_state)]
1368    #[test]
1369    fn parse_review_response_template_echo_infers_from_notes() {
1370        let json = r#"{"decision":"approve|request_changes","notes":"LGTM","issues":[]}"#;
1371        assert_eq!(parse_review_response(json).unwrap().decision, "approve");
1372
1373        let no_signal = r#"{"decision":"approve|request_changes","notes":"hmm","issues":[]}"#;
1374        assert!(parse_review_response(no_signal).is_err());
1375    }
1376
1377    #[serial(cooldown_state)]
1378    #[test]
1379    fn parse_review_response_invalid() {
1380        let result = parse_review_response("not json at all");
1381        assert!(result.is_err());
1382    }
1383
1384    #[serial(cooldown_state)]
1385    #[test]
1386    fn extract_json_block_basic() {
1387        let md = "text\n```json\n{\"key\": \"value\"}\n```\nmore";
1388        let result = extract_json_block(md);
1389        assert!(result.is_some());
1390        assert!(result.unwrap().contains("\"key\""));
1391    }
1392
1393    #[serial(cooldown_state)]
1394    #[test]
1395    fn extract_json_block_skips_non_json_blocks() {
1396        let md = "```json\nnot-json-array\n```\n\n```json\n{\"real\": true}\n```";
1397        let result = extract_json_block(md);
1398        assert!(result.is_some());
1399        assert!(result.unwrap().contains("\"real\""));
1400    }
1401
1402    #[serial(cooldown_state)]
1403    #[test]
1404    fn extract_json_block_none_when_missing() {
1405        assert!(extract_json_block("no code blocks here").is_none());
1406    }
1407
1408    #[serial(cooldown_state)]
1409    #[test]
1410    fn extract_json_block_skips_malformed_intermediate_blocks() {
1411        let md = "prefix ```json{\"broken\": true}\n\n```json\n{\"real\": true}\n```";
1412        let result = extract_json_block(md);
1413        assert_eq!(result.as_deref(), Some("{\"real\": true}"));
1414    }
1415
1416    #[serial(cooldown_state)]
1417    #[test]
1418    fn extract_json_block_skips_multiple_malformed_intermediate_blocks() {
1419        let md = "prefix ```json{\"broken\": true}\nmid ```json[1, 2, 3]\n\n```json\n{\"real\": true}\n```";
1420        let result = extract_json_block(md);
1421        assert_eq!(result.as_deref(), Some("{\"real\": true}"));
1422    }
1423
1424    // ── parse_review_response + infer_review_response (production path) ─
1425
1426    /// Helper that mirrors the production plain-text path in review.rs:
1427    /// parse_review_response → infer_review_response.
1428    fn parse_review_plain(text: &str) -> anyhow::Result<ReviewResponse> {
1429        parse_review_response(text).or_else(|_| {
1430            infer_review_response(text)
1431                .ok_or_else(|| anyhow::anyhow!("failed to parse review response"))
1432        })
1433    }
1434
1435    #[serial(cooldown_state)]
1436    #[test]
1437    fn parse_review_from_output_direct_json() {
1438        let json = r#"{"decision":"approve","notes":"LGTM","test_results":"pass","issues":[]}"#;
1439        let resp = parse_review_response(json).unwrap();
1440        assert_eq!(resp.decision, "approve");
1441    }
1442
1443    #[serial(cooldown_state)]
1444    #[test]
1445    fn parse_review_from_output_markdown() {
1446        let md = "Review complete.\n\n```json\n{\"decision\":\"request_changes\",\"notes\":\"Fix it\",\"issues\":[]}\n```\n";
1447        let resp = parse_review_response(md).unwrap();
1448        assert_eq!(resp.decision, "request_changes");
1449    }
1450
1451    #[serial(cooldown_state)]
1452    #[test]
1453    fn parse_review_from_output_plain_text_fallback() {
1454        let text = "The review is done. LGTM.";
1455        let resp = parse_review_plain(text).unwrap();
1456        assert_eq!(resp.decision, "approve");
1457        assert!(resp.issues.is_empty(), "{:?}", resp.issues);
1458    }
1459
1460    /// opencode NDJSON stream — text events use the `part.text` format.
1461    #[serial(cooldown_state)]
1462    #[test]
1463    fn parse_review_from_output_ndjson_part_format() {
1464        let ndjson = concat!(
1465            r#"{"type":"step_start","timestamp":1000,"sessionID":"ses_abc"}"#,
1466            "\n",
1467            r#"{"type":"text","timestamp":1001,"part":{"type":"text","text":"Reviewing the PR...\n\n```json\n{\"decision\":\"approve\",\"notes\":\"All checks pass\",\"test_results\":\"pass\",\"issues\":[]}\n```\n"}}"#,
1468            "\n",
1469            r#"{"type":"step_finish","timestamp":1002,"part":{"type":"step-finish","reason":"stop"}}"#,
1470        );
1471        let resp = parse_via_agent_path("opencode", ndjson).unwrap();
1472        assert_eq!(resp.decision, "approve");
1473        assert_eq!(resp.notes, "All checks pass");
1474    }
1475
1476    /// opencode NDJSON stream — text events use the newer `text` directly-in-event format.
1477    #[serial(cooldown_state)]
1478    #[test]
1479    fn parse_review_from_output_ndjson_direct_text_format() {
1480        let ndjson = concat!(
1481            r#"{"type":"step_start","timestamp":1000,"sessionID":"ses_abc"}"#,
1482            "\n",
1483            r#"{"type":"text","timestamp":1001,"sessionID":"ses_abc","text":"Here is my review:\n\n```json\n{\"decision\":\"request_changes\",\"notes\":\"Fix the bug\",\"test_results\":\"fail\",\"issues\":[]}\n```\n"}"#,
1484            "\n",
1485            r#"{"type":"step_finish","timestamp":1002,"sessionID":"ses_abc"}"#,
1486        );
1487        let resp = parse_via_agent_path("opencode", ndjson).unwrap();
1488        assert_eq!(resp.decision, "request_changes");
1489        assert_eq!(resp.notes, "Fix the bug");
1490    }
1491
1492    /// opencode NDJSON with ReviewResponse JSON directly in a text event (no markdown wrapper).
1493    #[serial(cooldown_state)]
1494    #[test]
1495    fn parse_review_from_output_ndjson_direct_json_in_text_event() {
1496        let ndjson = concat!(
1497            r#"{"type":"step_start","timestamp":1000}"#,
1498            "\n",
1499            r#"{"type":"text","timestamp":1001,"part":{"type":"text","text":"{\"decision\":\"approve\",\"notes\":\"LGTM\",\"test_results\":\"pass\",\"issues\":[]}"}}"#,
1500            "\n",
1501            r#"{"type":"step_finish","timestamp":1002}"#,
1502        );
1503        let resp = parse_via_agent_path("opencode", ndjson).unwrap();
1504        assert_eq!(resp.decision, "approve");
1505    }
1506
1507    #[serial(cooldown_state)]
1508    #[test]
1509    fn parse_review_from_output_ndjson_prefers_final_json_text_event() {
1510        let ndjson = concat!(
1511            r#"{"type":"text","timestamp":1001,"part":{"type":"text","text":"Working on it..."}}"#,
1512            "\n",
1513            r#"{"type":"text","timestamp":1002,"part":{"type":"text","text":"{\"decision\":\"approve\",\"notes\":\"LGTM\",\"test_results\":\"pass\",\"issues\":[]}"}}"#,
1514        );
1515        let resp = parse_via_agent_path("opencode", ndjson).unwrap();
1516        assert_eq!(resp.decision, "approve");
1517        assert_eq!(resp.notes, "LGTM");
1518    }
1519
1520    /// Concatenated text events produce a valid ReviewResponse.
1521    #[serial(cooldown_state)]
1522    #[test]
1523    fn parse_review_from_output_ndjson_concatenated_text() {
1524        let ndjson = concat!(
1525            r#"{"type":"text","timestamp":1001,"part":{"type":"text","text":"Here is my\n"}}"#,
1526            "\n",
1527            r#"{"type":"text","timestamp":1002,"part":{"type":"text","text":"review:\n\n```json\n{\"decision\":\"approve\",\"notes\":\"OK\",\"test_results\":\"pass\",\"issues\":[]}\n```\n"}}"#,
1528        );
1529        let resp = parse_via_agent_path("opencode", ndjson).unwrap();
1530        assert_eq!(resp.decision, "approve");
1531    }
1532
1533    /// codex NDJSON stream — agent_message item carries the ReviewResponse JSON.
1534    ///
1535    /// Real format from codex `exec --json` output (observed 2026-03-06):
1536    /// thread.started / turn.started / item.completed(reasoning) / item.completed(agent_message) / turn.completed
1537    #[serial(cooldown_state)]
1538    #[test]
1539    fn parse_review_from_output_codex_agent_message() {
1540        let ndjson = concat!(
1541            r#"{"type":"thread.started","thread_id":"t1"}"#,
1542            "\n",
1543            r#"{"type":"turn.started"}"#,
1544            "\n",
1545            r#"{"type":"item.completed","item":{"id":"item_0","type":"reasoning","text":"Planning git commands..."}}"#,
1546            "\n",
1547            r#"{"type":"item.completed","item":{"id":"item_1","type":"command_execution","command":"git log --oneline -5","output":"abc123 fix: something"}}"#,
1548            "\n",
1549            r#"{"type":"item.completed","item":{"id":"item_2","type":"agent_message","text":"{\"decision\":\"approve\",\"notes\":\"All checks pass\",\"test_results\":\"pass\",\"issues\":[]}"}}"#,
1550            "\n",
1551            r#"{"type":"turn.completed"}"#,
1552        );
1553        let resp = parse_via_agent_path("codex", ndjson).unwrap();
1554        assert_eq!(resp.decision, "approve");
1555        assert_eq!(resp.notes, "All checks pass");
1556    }
1557
1558    /// codex NDJSON with ReviewResponse in a markdown code block inside agent_message.
1559    #[serial(cooldown_state)]
1560    #[test]
1561    fn parse_review_from_output_codex_agent_message_markdown() {
1562        let ndjson = concat!(
1563            r#"{"type":"thread.started","thread_id":"t1"}"#,
1564            "\n",
1565            r#"{"type":"turn.started"}"#,
1566            "\n",
1567            r#"{"type":"item.completed","item":{"id":"item_0","type":"reasoning","text":"Reviewing..."}}"#,
1568            "\n",
1569            r#"{"type":"item.completed","item":{"id":"item_1","type":"agent_message","text":"Review complete.\n\n```json\n{\"decision\":\"request_changes\",\"notes\":\"Fix the tests\",\"issues\":[]}\n```\n"}}"#,
1570            "\n",
1571            r#"{"type":"turn.completed"}"#,
1572        );
1573        let resp = parse_via_agent_path("codex", ndjson).unwrap();
1574        assert_eq!(resp.decision, "request_changes");
1575        assert_eq!(resp.notes, "Fix the tests");
1576    }
1577
1578    /// codex reasoning items must NOT be extracted as review text — only agent_message is used.
1579    #[serial(cooldown_state)]
1580    #[test]
1581    fn codex_skips_reasoning_items() {
1582        let ndjson = concat!(
1583            r#"{"type":"item.completed","item":{"type":"reasoning","text":"internal thoughts..."}}"#,
1584            "\n",
1585            r#"{"type":"item.completed","item":{"type":"agent_message","text":"{\"decision\":\"approve\",\"notes\":\"LGTM\",\"issues\":[]}"}}"#,
1586        );
1587        let resp = parse_via_agent_path("codex", ndjson).unwrap();
1588        assert_eq!(resp.decision, "approve");
1589        assert_eq!(resp.notes, "LGTM");
1590    }
1591
1592    /// Negation patterns must NOT trigger a false approval.
1593    #[serial(cooldown_state)]
1594    #[test]
1595    fn infer_review_response_negation_not_approved() {
1596        assert!(
1597            parse_review_plain("The changes were not approved.").is_err(),
1598            "\"not approved\" should not infer approval"
1599        );
1600    }
1601
1602    #[serial(cooldown_state)]
1603    #[test]
1604    fn infer_review_response_negation_unapproved() {
1605        assert!(
1606            parse_review_plain("Unapproved changes found in the diff.").is_err(),
1607            "\"unapproved\" should not infer approval"
1608        );
1609    }
1610
1611    #[serial(cooldown_state)]
1612    #[test]
1613    fn infer_review_response_negation_not_approving() {
1614        assert!(
1615            parse_review_plain("I am not approving this PR.").is_err(),
1616            "\"not approving\" should not infer approval"
1617        );
1618    }
1619
1620    #[serial(cooldown_state)]
1621    #[test]
1622    fn infer_review_response_negation_cannot_be_approved() {
1623        assert!(
1624            parse_review_plain("This PR cannot be approved until the race in tick.rs is fixed.")
1625                .is_err(),
1626            "\"cannot be approved\" should not infer approval"
1627        );
1628    }
1629
1630    #[serial(cooldown_state)]
1631    #[test]
1632    fn infer_review_response_negation_should_not_be_approved() {
1633        assert!(
1634            parse_review_plain("The diff breaks the build and should not be approved.").is_err(),
1635            "\"should not be approved\" should not infer approval"
1636        );
1637    }
1638
1639    #[serial(cooldown_state)]
1640    #[test]
1641    fn infer_review_response_negation_not_yet_approved() {
1642        assert!(
1643            parse_review_plain("This change is not yet approved.").is_err(),
1644            "\"not yet approved\" should not infer approval"
1645        );
1646    }
1647
1648    #[serial(cooldown_state)]
1649    #[test]
1650    fn infer_review_response_negation_wont_approve() {
1651        assert!(
1652            parse_review_plain("I won't approve this until tests cover the retry path.").is_err(),
1653            "\"won't approve\" should not infer approval"
1654        );
1655    }
1656
1657    #[serial(cooldown_state)]
1658    #[test]
1659    fn infer_review_response_bare_approved() {
1660        assert!(
1661            parse_review_plain("The PR was approved.").is_err(),
1662            "bare \"approved\" should not infer approval"
1663        );
1664    }
1665
1666    /// Positive plain-text approval signals must still work.
1667    #[serial(cooldown_state)]
1668    #[test]
1669    fn infer_review_response_lgtm() {
1670        let resp = parse_review_plain("LGTM, everything looks fine.").unwrap();
1671        assert_eq!(resp.decision, "approve");
1672    }
1673
1674    /// Evidence phrases are not a decision token
1675    #[serial(cooldown_state)]
1676    #[test]
1677    fn infer_review_response_looks_good() {
1678        assert!(
1679            parse_review_plain("Looks good to me!").is_err(),
1680            "\"looks good\" should not infer approval"
1681        );
1682    }
1683
1684    #[serial(cooldown_state)]
1685    #[test]
1686    fn infer_review_response_all_checks_passed() {
1687        assert!(
1688            parse_review_plain(
1689                "The background task completed but I already got the CI results directly. All checks passed — no action needed.",
1690            )
1691            .is_err(),
1692            "\"all checks passed\" should not infer approval"
1693        );
1694    }
1695
1696    #[serial(cooldown_state)]
1697    #[test]
1698    fn infer_review_response_checks_passed() {
1699        assert!(
1700            parse_review_plain("All checks passed (fmt ✓, clippy ✓, tests ✓).").is_err(),
1701            "\"all checks passed\" should not infer approval"
1702        );
1703    }
1704
1705    #[serial(cooldown_state)]
1706    #[test]
1707    fn infer_review_response_all_tests_passed() {
1708        assert!(
1709            parse_review_plain("All tests passed and the implementation looks correct.").is_err(),
1710            "\"all tests passed\" should not infer approval"
1711        );
1712    }
1713
1714    #[serial(cooldown_state)]
1715    #[test]
1716    fn infer_review_response_no_issues_found() {
1717        assert!(
1718            parse_review_plain("Review complete. No issues found in the diff.").is_err(),
1719            "\"no issues found\" should not infer approval"
1720        );
1721    }
1722
1723    /// Partial "tests passed" mid-sentence must NOT infer approval without "all" prefix.
1724    #[serial(cooldown_state)]
1725    #[test]
1726    fn infer_review_response_partial_tests_passed_no_approve() {
1727        let text = "The existing tests passed before this change but the new payment logic has a null-pointer risk on line 42.";
1728        assert!(parse_review_plain(text).is_err());
1729    }
1730
1731    /// Partial "checks passed" mid-sentence must NOT infer approval without "all" prefix.
1732    #[serial(cooldown_state)]
1733    #[test]
1734    fn infer_review_response_partial_checks_passed_no_approve() {
1735        let text = "CI checks passed for the base branch but this PR adds a broken path.";
1736        assert!(parse_review_plain(text).is_err());
1737    }
1738
1739    /// LLMs sometimes echo the JSON decision field value literally: "Decision is `approve`."
1740    #[serial(cooldown_state)]
1741    #[test]
1742    fn infer_review_response_decision_is_approve_backtick() {
1743        let text = "Already retrieved the output and completed the review above. The CI test check passed (3m11s) and all other checks are green. Decision is `approve`.";
1744        let resp = parse_review_plain(text).unwrap();
1745        assert_eq!(resp.decision, "approve");
1746    }
1747
1748    /// "Decision: approve" without backticks should also match.
1749    #[serial(cooldown_state)]
1750    #[test]
1751    fn infer_review_response_decision_colon_approve() {
1752        let text = "Reviewed the PR. No blocking issues found. Decision: approve";
1753        let resp = parse_review_plain(text).unwrap();
1754        assert_eq!(resp.decision, "approve");
1755    }
1756
1757    /// Bare backtick `approve` in a negative context must NOT infer approval.
1758    /// e.g. "do not `approve`" previously matched the removed `lower.contains("`approve`")` check.
1759    #[serial(cooldown_state)]
1760    #[test]
1761    fn infer_review_response_negation_do_not_approve_backtick() {
1762        let text = "There are unresolved issues — do not `approve` this PR until they are fixed.";
1763        assert!(
1764            parse_review_plain(text).is_err(),
1765            "negative context with backtick approve must not infer approval"
1766        );
1767    }
1768
1769    // ── review parsing via find_agent_result + parse_review_response ────
1770
1771    /// Helper that mirrors the production path in review.rs:
1772    /// find_agent_result → parse_review_response → infer_review_response.
1773    fn parse_via_agent_path(agent: &str, output: &str) -> anyhow::Result<ReviewResponse> {
1774        let text = find_agent_result(agent, output)
1775            .map(|r| r.result_text)
1776            .filter(|t| !t.is_empty())
1777            .unwrap_or_else(|| output.to_string());
1778        if let Ok(r) = parse_review_response(&text) {
1779            return Ok(r);
1780        }
1781        infer_review_response_from_text(&text)
1782            .ok_or_else(|| anyhow::anyhow!("failed to parse review response from {agent} output"))
1783    }
1784
1785    #[serial(cooldown_state)]
1786    #[test]
1787    fn agent_output_claude_ndjson_review() {
1788        let ndjson = concat!(
1789            r#"{"type":"system","subtype":"init","session_id":"s1"}"#,
1790            "\n",
1791            r#"{"type":"assistant","message":{"content":[{"type":"text","text":"Reviewing..."}]}}"#,
1792            "\n",
1793            r#"{"type":"result","subtype":"success","is_error":false,"result":"{\"decision\":\"approve\",\"notes\":\"LGTM\",\"test_results\":\"pass\",\"issues\":[]}","usage":{"input_tokens":100,"output_tokens":20}}"#,
1794        );
1795        let resp = parse_via_agent_path("claude", ndjson).unwrap();
1796        assert_eq!(resp.decision, "approve");
1797    }
1798
1799    #[serial(cooldown_state)]
1800    #[test]
1801    fn agent_output_opencode_ndjson_review() {
1802        let ndjson = concat!(
1803            r#"{"type":"step_start","timestamp":1000}"#,
1804            "\n",
1805            r#"{"type":"text","timestamp":1001,"part":{"type":"text","text":"{\"decision\":\"request_changes\",\"notes\":\"Fix it\",\"issues\":[]}"}}"#,
1806            "\n",
1807            r#"{"type":"step_finish","timestamp":1002,"part":{"type":"step-finish","reason":"stop","tokens":{"input":100,"output":10}}}"#,
1808        );
1809        let resp = parse_via_agent_path("opencode", ndjson).unwrap();
1810        assert_eq!(resp.decision, "request_changes");
1811    }
1812
1813    #[serial(cooldown_state)]
1814    #[test]
1815    fn agent_output_codex_ndjson_review() {
1816        let ndjson = concat!(
1817            r#"{"type":"thread.started","thread_id":"t1"}"#,
1818            "\n",
1819            r#"{"type":"item.completed","item":{"type":"agent_message","text":"{\"decision\":\"approve\",\"notes\":\"All good\",\"issues\":[]}"}}"#,
1820            "\n",
1821            r#"{"type":"turn.completed"}"#,
1822        );
1823        let resp = parse_via_agent_path("codex", ndjson).unwrap();
1824        assert_eq!(resp.decision, "approve");
1825    }
1826
1827    #[serial(cooldown_state)]
1828    #[test]
1829    fn agent_output_plain_text_keyword_fallback() {
1830        let text = "All checks passed, LGTM.";
1831        let resp = parse_via_agent_path("claude", text).unwrap();
1832        assert_eq!(resp.decision, "approve");
1833    }
1834
1835    #[serial(cooldown_state)]
1836    #[test]
1837    fn agent_output_direct_json() {
1838        let json = r#"{"decision":"approve","notes":"LGTM","test_results":"pass","issues":[]}"#;
1839        let resp = parse_via_agent_path("opencode", json).unwrap();
1840        assert_eq!(resp.decision, "approve");
1841    }
1842
1843    /// Regression: plain text containing JSON fragments should NOT trigger false
1844    /// positives. The per-agent extractor returns None for non-NDJSON text,
1845    /// so we fall through to keyword inference instead of generic NDJSON parsing.
1846    #[serial(cooldown_state)]
1847    #[test]
1848    fn agent_output_text_with_json_fragment_no_false_positive() {
1849        let text = r#"I checked the file and found {"type":"text"} in the output. No issues."#;
1850        // This should NOT extract a review from the JSON fragment.
1851        // With the generic parser, this would match as a "text" NDJSON event.
1852        // With per-agent extraction, it returns None (not valid agent NDJSON).
1853        let result = parse_via_agent_path("claude", text);
1854        // Should either fail or infer from keywords, NOT parse the JSON fragment
1855        assert!(
1856            result.is_err() || result.as_ref().is_ok_and(|r| r.decision != "text"),
1857            "should not extract review from embedded JSON fragment"
1858        );
1859    }
1860
1861    /// Prose containing the word "decision" and a JSON-like fragment must not
1862    /// be misidentified as a review response by `extract_review_response_object`.
1863    #[serial(cooldown_state)]
1864    #[test]
1865    fn parse_review_response_ignores_prose_with_decision_word() {
1866        let text = r#"The decision was made. Here is a debug trace: {"trace":123}"#;
1867        assert!(
1868            parse_review_response(text).is_err(),
1869            "prose with 'decision' word and unrelated JSON should not parse as ReviewResponse"
1870        );
1871    }
1872
1873    /// Plain text with an embedded JSON object that has unrelated keys should
1874    /// not be extracted as a review response.
1875    #[serial(cooldown_state)]
1876    #[test]
1877    fn parse_review_response_ignores_embedded_json_without_decision_key() {
1878        let text = r#"Analysis complete. Config: {"timeout":30,"retries":3}. Done."#;
1879        assert!(
1880            parse_review_response(text).is_err(),
1881            "embedded JSON without 'decision' key should not parse as ReviewResponse"
1882        );
1883    }
1884
1885    // ── read_file_with_limit tests ───────────────────────────────────────────
1886
1887    #[serial(cooldown_state)]
1888    #[test]
1889    fn read_file_with_limit_reads_small_file_completely() {
1890        let temp_dir = env::temp_dir();
1891        let file_path = temp_dir.join(format!("orch_test_small_{}.txt", process::id()));
1892        let content = "Hello, World!";
1893        fs::write(&file_path, content).unwrap();
1894
1895        let result = read_file_with_limit(&file_path, 1024);
1896        assert!(result.is_ok());
1897        assert_eq!(result.unwrap(), content);
1898
1899        let _ = fs::remove_file(&file_path);
1900    }
1901
1902    #[serial(cooldown_state)]
1903    #[test]
1904    fn read_file_with_limit_truncates_large_file() {
1905        let temp_dir = env::temp_dir();
1906        let file_path = temp_dir.join(format!("orch_test_large_{}.txt", process::id()));
1907
1908        // Create a file larger than the limit (100 bytes)
1909        let large_content = "x".repeat(200);
1910        fs::write(&file_path, &large_content).unwrap();
1911
1912        // Read with a 50-byte limit
1913        let result = read_file_with_limit(&file_path, 50);
1914        assert!(result.is_ok());
1915
1916        let read_content = result.unwrap();
1917        // Should only read 50 bytes (the tail of the file)
1918        assert_eq!(read_content.len(), 50);
1919        assert_eq!(read_content, "x".repeat(50));
1920
1921        let _ = fs::remove_file(&file_path);
1922    }
1923
1924    #[serial(cooldown_state)]
1925    #[test]
1926    fn read_file_with_limit_handles_empty_file() {
1927        let temp_dir = env::temp_dir();
1928        let file_path = temp_dir.join(format!("orch_test_empty_{}.txt", process::id()));
1929        fs::write(&file_path, "").unwrap();
1930
1931        let result = read_file_with_limit(&file_path, 1024);
1932        assert!(result.is_ok());
1933        assert_eq!(result.unwrap(), "");
1934
1935        let _ = fs::remove_file(&file_path);
1936    }
1937
1938    #[serial(cooldown_state)]
1939    #[test]
1940    fn read_file_with_limit_handles_nonexistent_file() {
1941        let temp_dir = env::temp_dir();
1942        let file_path = temp_dir.join("orch_test_nonexistent_xyz123.txt");
1943
1944        let result = read_file_with_limit(&file_path, 1024);
1945        assert!(result.is_err());
1946    }
1947
1948    #[serial(cooldown_state)]
1949    #[test]
1950    fn read_file_with_limit_reads_exact_limit() {
1951        let temp_dir = env::temp_dir();
1952        let file_path = temp_dir.join(format!("orch_test_exact_{}.txt", process::id()));
1953
1954        // Create exactly 100 bytes
1955        let content = "y".repeat(100);
1956        fs::write(&file_path, &content).unwrap();
1957
1958        // Read with exactly 100-byte limit (should read all)
1959        let result = read_file_with_limit(&file_path, 100);
1960        assert!(result.is_ok());
1961        assert_eq!(result.unwrap().len(), 100);
1962
1963        let _ = fs::remove_file(&file_path);
1964    }
1965
1966    #[serial(cooldown_state)]
1967    #[test]
1968    fn read_file_with_limit_handles_unicode_truncation() {
1969        let temp_dir = env::temp_dir();
1970        let file_path = temp_dir.join(format!("orch_test_unicode_{}.txt", process::id()));
1971
1972        // Create content with multi-byte UTF-8 characters
1973        let emoji = "🎉".repeat(100); // Each emoji is 4 bytes
1974        fs::write(&file_path, &emoji).unwrap();
1975
1976        // Read with a limit that cuts into the emoji sequence
1977        let result = read_file_with_limit(&file_path, 50);
1978        assert!(result.is_ok());
1979
1980        let read_content = result.unwrap();
1981        // from_utf8_lossy should handle invalid sequences
1982        assert_ne!(read_content, "");
1983
1984        let _ = fs::remove_file(&file_path);
1985    }
1986
1987    // ── Fixture-based review integration tests ──────────────────────────────────
1988
1989    /// Full review pipeline: find_agent_result → parse_review_response → infer_review_response.
1990    fn parse_review_via_agent(agent: &str, output: &str) -> anyhow::Result<ReviewResponse> {
1991        let text = find_agent_result(agent, output)
1992            .map(|r| r.result_text)
1993            .filter(|t| !t.is_empty())
1994            .unwrap_or_else(|| output.to_string());
1995        if let Ok(r) = parse_review_response(&text) {
1996            return Ok(r);
1997        }
1998        infer_review_response(&text)
1999            .ok_or_else(|| anyhow::anyhow!("failed to parse review from {agent} output"))
2000    }
2001
2002    // ── Claude review fixtures ─────────────────────────────────────────────────
2003
2004    #[serial(cooldown_state)]
2005    #[test]
2006    fn fixture_claude_review_approve() {
2007        let raw = include_str!("../../../tests/fixtures/review_claude_approve.jsonl");
2008        let resp = parse_review_via_agent("claude", raw).unwrap();
2009        assert_eq!(resp.decision, "approve");
2010        assert_eq!(resp.notes, "All checks pass");
2011        assert_eq!(resp.test_results.as_deref(), Some("pass"));
2012        assert!(resp.issues.is_empty(), "{:?}", resp.issues);
2013    }
2014
2015    #[serial(cooldown_state)]
2016    #[test]
2017    fn fixture_claude_review_request_changes() {
2018        let raw = include_str!("../../../tests/fixtures/review_claude_request_changes.jsonl");
2019        let resp = parse_review_via_agent("claude", raw).unwrap();
2020        assert_eq!(resp.decision, "request_changes");
2021        assert_eq!(resp.notes, "Fix the null deref on line 42");
2022        assert_eq!(resp.test_results.as_deref(), Some("fail"));
2023        assert_eq!(resp.issues.len(), 1);
2024        assert_eq!(resp.issues[0].file, "src/main.rs");
2025        assert_eq!(resp.issues[0].line, Some(42));
2026        assert_eq!(resp.issues[0].severity, "error");
2027    }
2028
2029    #[serial(cooldown_state)]
2030    #[test]
2031    fn fixture_claude_review_rate_limit_returns_error() {
2032        // find_claude_result on an is_error=true result returns Some with is_error=true.
2033        // The integration test verifies this is captured, not that it's a ReviewResponse.
2034        let raw = include_str!("../../../tests/fixtures/review_claude_rate_limit.jsonl");
2035        let result = find_agent_result("claude", raw);
2036        assert!(
2037            result.is_some(),
2038            "should find result even for error envelope"
2039        );
2040        let result = result.unwrap();
2041        assert!(result.is_error, "rate limit should be flagged as error");
2042        assert!(result.result_text.contains("rate limit"));
2043    }
2044
2045    // ── OpenCode review fixtures ────────────────────────────────────────────────
2046
2047    #[serial(cooldown_state)]
2048    #[test]
2049    fn fixture_opencode_review_approve() {
2050        let raw = include_str!("../../../tests/fixtures/review_opencode_approve.jsonl");
2051        let resp = parse_review_via_agent("opencode", raw).unwrap();
2052        assert_eq!(resp.decision, "approve");
2053        assert_eq!(resp.notes, "All checks pass");
2054        assert!(resp.issues.is_empty(), "{:?}", resp.issues);
2055    }
2056
2057    #[serial(cooldown_state)]
2058    #[test]
2059    fn fixture_opencode_review_request_changes() {
2060        let raw = include_str!("../../../tests/fixtures/review_opencode_request_changes.jsonl");
2061        let resp = parse_review_via_agent("opencode", raw).unwrap();
2062        assert_eq!(resp.decision, "request_changes");
2063        assert_eq!(resp.notes, "Fix the memory leak");
2064        assert_eq!(resp.test_results.as_deref(), Some("fail"));
2065        assert_eq!(resp.issues.len(), 1);
2066        assert_eq!(resp.issues[0].file, "src/lib.rs");
2067        assert_eq!(resp.issues[0].line, Some(100));
2068    }
2069
2070    #[serial(cooldown_state)]
2071    #[test]
2072    fn fixture_opencode_review_plain_text_approve() {
2073        // OpenCode with plain-text review (no JSON) — should infer approval via keywords.
2074        let raw = include_str!("../../../tests/fixtures/review_opencode_plain_text.jsonl");
2075        let resp = parse_review_via_agent("opencode", raw).unwrap();
2076        assert_eq!(resp.decision, "approve");
2077    }
2078
2079    // ── Codex review fixtures ────────────────────────────────────────────────────
2080
2081    #[serial(cooldown_state)]
2082    #[test]
2083    fn fixture_codex_review_approve() {
2084        let raw = include_str!("../../../tests/fixtures/review_codex_approve.jsonl");
2085        let resp = parse_review_via_agent("codex", raw).unwrap();
2086        assert_eq!(resp.decision, "approve");
2087        assert_eq!(resp.notes, "Clean implementation");
2088        assert!(resp.issues.is_empty(), "{:?}", resp.issues);
2089    }
2090
2091    #[serial(cooldown_state)]
2092    #[test]
2093    fn fixture_codex_review_request_changes() {
2094        let raw = include_str!("../../../tests/fixtures/review_codex_request_changes.jsonl");
2095        let resp = parse_review_via_agent("codex", raw).unwrap();
2096        assert_eq!(resp.decision, "request_changes");
2097        assert_eq!(resp.notes, "Race condition on line 15");
2098        assert_eq!(resp.issues.len(), 1);
2099        assert_eq!(resp.issues[0].file, "src/server.rs");
2100        assert_eq!(resp.issues[0].line, Some(15));
2101    }
2102
2103    #[serial(cooldown_state)]
2104    #[test]
2105    fn fixture_codex_review_plain_text_approve() {
2106        // Codex with plain-text review — should infer approval via "LGTM" keyword.
2107        let raw = include_str!("../../../tests/fixtures/review_codex_plain_text.jsonl");
2108        let resp = parse_review_via_agent("codex", raw).unwrap();
2109        assert_eq!(resp.decision, "approve");
2110    }
2111
2112    // ── Kimi review fixtures ────────────────────────────────────────────────────
2113
2114    #[serial(cooldown_state)]
2115    #[test]
2116    fn fixture_kimi_review_approve() {
2117        let raw = include_str!("../../../tests/fixtures/review_kimi_approve.jsonl");
2118        let resp = parse_review_via_agent("kimi", raw).unwrap();
2119        assert_eq!(resp.decision, "approve");
2120        assert_eq!(resp.notes, "LGTM");
2121        assert_eq!(resp.test_results.as_deref(), Some("pass"));
2122        assert!(resp.issues.is_empty(), "{:?}", resp.issues);
2123    }
2124
2125    // ── MiniMax review fixtures ────────────────────────────────────────────────
2126
2127    #[serial(cooldown_state)]
2128    #[test]
2129    fn fixture_minimax_review_request_changes() {
2130        let raw = include_str!("../../../tests/fixtures/review_minimax_request_changes.jsonl");
2131        let resp = parse_review_via_agent("minimax", raw).unwrap();
2132        assert_eq!(resp.decision, "request_changes");
2133        assert_eq!(resp.notes, "Fix the bug");
2134        assert_eq!(resp.test_results.as_deref(), Some("fail"));
2135        assert_eq!(resp.issues.len(), 1);
2136        assert_eq!(resp.issues[0].file, "src/foo.rs");
2137        assert_eq!(resp.issues[0].line, Some(42));
2138        assert_eq!(resp.issues[0].severity, "error");
2139    }
2140
2141    // ── Cross-agent token/metadata extraction ───────────────────────────────────
2142
2143    /// Verify that find_agent_result extracts token counts from each agent's format.
2144    #[serial(cooldown_state)]
2145    #[test]
2146    fn find_agent_result_extracts_tokens_claude() {
2147        let raw = include_str!("../../../tests/fixtures/review_claude_approve.jsonl");
2148        let result = find_agent_result("claude", raw).expect("should find result");
2149        assert_eq!(result.input_tokens, Some(200));
2150        assert_eq!(result.output_tokens, Some(30));
2151        assert!(result.cost_usd.is_none()); // review fixtures don't include cost
2152    }
2153
2154    #[serial(cooldown_state)]
2155    #[test]
2156    fn find_agent_result_extracts_tokens_opencode() {
2157        let raw = include_str!("../../../tests/fixtures/review_opencode_approve.jsonl");
2158        let result = find_agent_result("opencode", raw).expect("should find result");
2159        assert_eq!(result.input_tokens, Some(2900));
2160        assert_eq!(result.output_tokens, Some(100));
2161        assert_eq!(result.cost_usd, Some(0.002));
2162    }
2163
2164    /// Verify that find_agent_result returns None for empty/unparseable input.
2165    #[serial(cooldown_state)]
2166    #[test]
2167    fn find_agent_result_returns_none_for_empty() {
2168        assert!(find_agent_result("claude", "").is_none());
2169        assert!(find_agent_result("opencode", "").is_none());
2170        assert!(find_agent_result("codex", "").is_none());
2171        assert!(find_agent_result("kimi", "").is_none());
2172        assert!(find_agent_result("minimax", "").is_none());
2173    }
2174
2175    #[serial(cooldown_state)]
2176    #[test]
2177    fn find_agent_result_returns_none_for_plain_text() {
2178        assert!(find_agent_result("claude", "just some plain text").is_none());
2179        assert!(find_agent_result("codex", "just some plain text").is_none());
2180        // OpenCode is a special case — it can sometimes return a result for plain text.
2181        // This is acceptable because opencode's extract_ndjson_text falls back gracefully.
2182    }
2183
2184    // ── Error propagation via find_agent_result ─────────────────────────────────
2185
2186    /// Verify that find_agent_result surfaces rate-limit errors from Claude.
2187    #[serial(cooldown_state)]
2188    #[test]
2189    fn find_agent_result_propagates_claude_rate_limit() {
2190        let raw = include_str!("../../../tests/fixtures/review_claude_rate_limit.jsonl");
2191        let result = find_agent_result("claude", raw).expect("should find result");
2192        assert!(result.is_error, "should be flagged as error");
2193        assert!(result.result_text.contains("rate limit"));
2194    }
2195
2196    /// Two fake agents on PATH, a config that names them and a 1 ms backoff. Each nextest test
2197    /// runs in its own process, so the environment is not restored
2198    #[cfg(unix)]
2199    fn failover_env() -> tempfile::TempDir {
2200        let dir = tempfile::tempdir().unwrap();
2201        let bin = dir.path().join("bin");
2202        fs::create_dir(&bin).unwrap();
2203        for agent in ["claude", "codex"] {
2204            let path = bin.join(agent);
2205            fs::write(&path, "#!/bin/sh\n").unwrap();
2206            fs::set_permissions(&path, fs::Permissions::from_mode(0o755)).unwrap();
2207        }
2208        let home = dir.path().join("home");
2209        fs::create_dir(&home).unwrap();
2210        fs::write(
2211            home.join("config.yml"),
2212            "agents: [claude, codex]\ndispatch:\n  retry_base_delay_ms: 1\n  retry_max_delay_ms: 1\nworkflow:\n  max_timeout_retries: 3\n",
2213        )
2214        .unwrap();
2215        let mut path = bin.as_os_str().to_os_string();
2216        path.push(":");
2217        path.push(env::var_os("PATH").unwrap_or_default());
2218        // SAFETY: nextest runs each test in its own process, so no other test touches the environment
2219        unsafe { env::set_var("PATH", path) };
2220        // SAFETY: same as above
2221        unsafe { env::set_var("ORCH_HOME", &home) };
2222        cmd_cache::reset_for_test();
2223        dir
2224    }
2225
2226    async fn failover_task() -> (Arc<TaskStore>, String) {
2227        let store = Arc::new(TaskStore::open_memory().await.expect("in-memory store"));
2228        let id = store
2229            .create_internal("owner/repo", "t", "b", "test", "", None)
2230            .await
2231            .unwrap();
2232        (store, format!("internal:{id}"))
2233    }
2234
2235    /// Every other agent is in cooldown, so the timed-out task retries from `new` after the backoff
2236    #[cfg(unix)]
2237    #[serial(cooldown_state)]
2238    #[tokio::test]
2239    async fn timeout_with_every_other_agent_cooled_retries_as_new() {
2240        let _env = failover_env();
2241        cooldown::reset_global_state().await;
2242        set_agent_cooldown("codex", 600).await;
2243        let (store, task_id) = failover_task().await;
2244        let store = Some(store);
2245
2246        let status = handle_failover(
2247            &task_id,
2248            "claude",
2249            RetryableError::Timeout,
2250            "claude timed out after 1804s",
2251            false,
2252            &store,
2253            "owner/repo",
2254        )
2255        .await;
2256
2257        assert_eq!(status, "new");
2258        let task = store::opt_store_get_task(&store, "owner/repo", &task_id)
2259            .await
2260            .unwrap();
2261        assert!(task.limit_reroute_chain.is_empty(), "chain must be cleared");
2262        assert!(
2263            task.last_error.contains("will retry"),
2264            "{}",
2265            task.last_error
2266        );
2267        assert_eq!(task.timeout_retries, 1);
2268    }
2269
2270    /// Another agent is free, so the timeout still reroutes to it
2271    #[cfg(unix)]
2272    #[serial(cooldown_state)]
2273    #[tokio::test]
2274    async fn timeout_with_a_free_agent_reroutes_to_it() {
2275        let _env = failover_env();
2276        cooldown::reset_global_state().await;
2277        let (store, task_id) = failover_task().await;
2278        let store = Some(store);
2279
2280        let status = handle_failover(
2281            &task_id,
2282            "claude",
2283            RetryableError::Timeout,
2284            "claude timed out after 1804s",
2285            false,
2286            &store,
2287            "owner/repo",
2288        )
2289        .await;
2290
2291        assert_eq!(status, "new");
2292        let task = store::opt_store_get_task(&store, "owner/repo", &task_id)
2293            .await
2294            .unwrap();
2295        assert_eq!(task.limit_reroute_chain, "claude");
2296        assert!(task.last_error.contains("re-route"), "{}", task.last_error);
2297    }
2298
2299    /// A task that times out on every run blocks after `max_timeout_retries` and keeps its issue open
2300    #[cfg(unix)]
2301    #[serial(cooldown_state)]
2302    #[tokio::test]
2303    async fn repeated_timeouts_block_the_task() {
2304        let _env = failover_env();
2305        cooldown::reset_global_state().await;
2306        set_agent_cooldown("codex", 600).await;
2307        let (store, task_id) = failover_task().await;
2308        let store = Some(store);
2309
2310        let mut statuses = Vec::new();
2311        for _ in 0..3 {
2312            statuses.push(
2313                handle_failover(
2314                    &task_id,
2315                    "claude",
2316                    RetryableError::Timeout,
2317                    "claude timed out after 1804s",
2318                    false,
2319                    &store,
2320                    "owner/repo",
2321                )
2322                .await,
2323            );
2324        }
2325
2326        assert_eq!(statuses, ["new", "new", "blocked"]);
2327        let task = store::opt_store_get_task(&store, "owner/repo", &task_id)
2328            .await
2329            .unwrap();
2330        assert_eq!(task.block_reason.as_deref(), Some("timed out 3 times"));
2331        assert_eq!(task.timeout_retries, 3);
2332    }
2333}