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