Skip to main content

car_server_core/coder/
external_loop.rs

1//! Delegation to an external agentic CLI (Claude Code, Codex, Gemini).
2//!
3//! The CLI does the coding inside the worktree; **CAR keeps the verdict**:
4//! after every invocation the runtime re-runs the outcome contract itself
5//! through the policy-gated shell tool. A CLI claiming success doesn't matter
6//! — the checks do.
7//!
8//! When the daemon's MCP listener is bound, its URL is threaded into
9//! [`InvokeOptions::mcp_endpoint`] so the CLI's **CAR-namespace** tool calls
10//! (`memory_*`, `verify`, `skill_*`) route back through car-server's policy +
11//! memgine — gated and audited. The CLI's own **built-in** tools (Edit, Bash)
12//! still run with the CLI's permissions inside the worktree: that is the
13//! residual Phase 2 stage-4b upstream limitation. The pinned `cwd`, the
14//! contract re-evaluation, and the merge approval gate remain the containment
15//! for those built-ins until tool round-trip governance lands in
16//! `car-external-agents`.
17
18use std::collections::HashMap;
19use std::sync::atomic::Ordering;
20use std::sync::Arc;
21
22use async_trait::async_trait;
23use car_external_agents::{InvokeError, InvokeOptions, InvokeResult, StreamEventEmitter};
24
25use super::budget::SessionDeadline;
26use super::contract::{evaluate_contract_with_baselines, CheckResult, OutcomeContract};
27use super::native_loop::{
28    primary_failure, record_recurrence, recurrence_notice, LoopFailure, LoopOutcome,
29};
30use super::session::{CancelFlag, CoderEventKind, EventSink};
31use super::shell_tool::WorktreeExecutor;
32
33/// Tuning for external delegation.
34#[derive(Debug, Clone)]
35pub struct ExternalLoopConfig {
36    /// Per-invocation model-turn cap (maps to the CLI's `--max-turns`).
37    pub max_turns: Option<u32>,
38    /// Per-invocation wall-clock budget (runner clamps to 1h).
39    pub timeout_secs: Option<u64>,
40    /// Fresh repair invocations after a red first pass (the stream-json
41    /// protocol has no session resume yet, so repairs re-state the task plus
42    /// the failing-check output).
43    ///
44    /// This is the **hypothesis** budget: each one buys another attempt at
45    /// being right. It is deliberately not spent on transport failures — see
46    /// `transient_retries`.
47    ///
48    /// **Defaults to 2 because recurrence escalation needs it.** Round 1
49    /// establishes a failure signature, round 2 is the first that can repeat it,
50    /// and only round 3 can be told it did. At 1 the loop still *detects* the
51    /// repeat, but the session ends before the feedback carrying that news
52    /// reaches anyone — and since `rpc` is the only construction site and takes
53    /// `..Default::default()`, a default of 1 made the escalation unreachable in
54    /// every shipped configuration.
55    ///
56    /// The cost is smaller than it looks: worst-case invocations are
57    /// `max_hypotheses + transient_retries`, so this moves 3 -> 4, about +33%,
58    /// and only on sessions that are already failing.
59    pub repair_invokes: u32,
60    /// Re-invocations after the CLI process itself died mid-run (timeout or
61    /// I/O) with the contract still red.
62    ///
63    /// Separate from `repair_invokes` because it buys a different thing: an
64    /// **availability** retry, not a new hypothesis. Sharing one counter means a
65    /// single flaky timeout eats a replan the coder needed for an actual
66    /// hypothesis — the difference between a session that recovers and one that
67    /// silently gives up on a task it was about to finish.
68    pub transient_retries: u32,
69    /// Pin the external CLI's backbone (`coder.start`'s `model`). `None` = the
70    /// CLI's own default.
71    ///
72    /// The paired A/B only measures the *harness* when both arms share a
73    /// backbone; this is the external half of that invariant (the native half is
74    /// `NativeLoopConfig.model`). Before this existed the pin reached only the
75    /// native loop, so "both arms on gpt-5.5" was an unverified assumption.
76    pub model: Option<String>,
77    /// The session's absolute deadline, SHARED with every other rung of the
78    /// fallback ladder. See [`super::budget`] for why this is a handle and not
79    /// a value.
80    pub deadline: Arc<SessionDeadline>,
81    /// The session-start baseline captures differential checks compare against
82    /// (car#1067). Empty when the contract declares none; differential checks
83    /// fail closed under an empty map.
84    pub baseline_captures: super::contract::BaselineCaptures,
85}
86
87impl Default for ExternalLoopConfig {
88    fn default() -> Self {
89        Self {
90            max_turns: Some(50),
91            timeout_secs: Some(1800),
92            repair_invokes: 2,
93            transient_retries: 1,
94            model: None,
95            deadline: SessionDeadline::shared_default(),
96            baseline_captures: super::contract::BaselineCaptures::new(),
97        }
98    }
99}
100
101/// The CLI seam: one invocation of an external agent.
102///
103/// Exists for the same reason `native_loop` takes a `&dyn TurnGenerator` — the
104/// loop's interesting behavior (budget accounting, retry-vs-replan, round
105/// counting) is decided by what comes back from here, and none of it is
106/// testable while the call is hard-wired to a real subprocess.
107#[async_trait]
108pub trait CliInvoker: Send + Sync {
109    async fn invoke(
110        &self,
111        agent_id: &str,
112        task: &str,
113        opts: InvokeOptions,
114        emitter: StreamEventEmitter,
115    ) -> Result<InvokeResult, InvokeError>;
116}
117
118/// The production invoker: a real external CLI subprocess.
119pub struct LiveInvoker;
120
121#[async_trait]
122impl CliInvoker for LiveInvoker {
123    async fn invoke(
124        &self,
125        agent_id: &str,
126        task: &str,
127        opts: InvokeOptions,
128        emitter: StreamEventEmitter,
129    ) -> Result<InvokeResult, InvokeError> {
130        car_external_agents::invoke_with_emitter(agent_id, task, opts, Some(emitter)).await
131    }
132}
133
134/// The task text handed to the CLI: intent + contract + ground rules.
135fn build_task(intent: &str, contract: &OutcomeContract, feedback: Option<&str>) -> String {
136    let mut task = format!(
137        "{intent}\n\n\
138         OUTCOME CONTRACT — your work is verified by running these checks at the repository \
139         root; all must pass:\n{}\n\
140         Ground rules:\n\
141         - Work only inside the current directory (an isolated git worktree).\n\
142         - Do NOT git commit, push, or touch remotes; the runtime owns version control.\n\
143         - Run the checks yourself before finishing.\n",
144        contract.render()
145    );
146    if let Some(fb) = feedback {
147        task.push_str(&format!(
148            "\nA previous attempt left these checks FAILING — fix the code so they pass:\n{fb}"
149        ));
150    }
151    task
152}
153
154/// The per-invocation options handed to the runner. The `mcp_endpoint`, when
155/// set, routes the CLI's CAR-namespace tool calls through the daemon's policy +
156/// memgine; `allowed_tools: None` leaves the CLI's own built-in tools on their
157/// default (ungoverned) policy — see the module docs.
158fn build_invoke_opts(
159    executor: &WorktreeExecutor,
160    cfg: &ExternalLoopConfig,
161    mcp_endpoint: Option<&str>,
162) -> InvokeOptions {
163    InvokeOptions {
164        cwd: Some(executor.worktree().to_path_buf()),
165        allowed_tools: None, // the CLI's default policy; see module docs
166        max_turns: cfg.max_turns,
167        // Clamped to what the SESSION has left, not just this invocation's own
168        // budget. Admission alone grants a whole round, so a hypothesis let in
169        // just under the ceiling could otherwise run its full 1800s past it —
170        // a ceiling exceeded by 50% is not a ceiling. This is not interruption:
171        // the round simply starts with a shorter clock, and a CLI that hits its
172        // own timeout already flows through `Infrastructure` ->
173        // `evaluate_contract`, so nothing goes unjudged.
174        timeout_secs: match (cfg.timeout_secs, cfg.deadline.remaining_secs()) {
175            (Some(own), Some(left)) => Some(own.min(left)),
176            (own, None) => own,
177            (None, left) => left,
178        },
179        // The external half of the A/B's same-backbone invariant.
180        model: cfg.model.clone(),
181        // Gate + audit the CLI's CAR-namespace tool calls through the daemon
182        // when its MCP listener is bound; None degrades cleanly.
183        mcp_endpoint: mcp_endpoint.map(String::from),
184        ..Default::default()
185    }
186}
187
188/// Run the external engine to completion, cancellation, or exhaustion.
189///
190/// The `cancel` flag is checked between invocations only, but an in-flight CLI
191/// is NOT left running: `rpc::cancel_session` aborts the task handle, which
192/// drops this future along with the `Child`, and every adapter sets
193/// `kill_on_drop(true)` (with a Windows `JobObject` for the Node grandchildren).
194/// Enforcement lives one level up; the flag here is belt-and-braces, which is
195/// why threading `invoke_with_emitter_and_cancel` through [`CliInvoker`] would
196/// be tidier rather than more correct. The classification
197/// below already handles [`LoopFailure::Cancelled`] as its own terminal so that
198/// change does not need to revisit the control flow.
199///
200/// ## Why the contract is evaluated before a failure is classified
201///
202/// Every path that got as far as launching the CLI evaluates the contract,
203/// including one that ended in a timeout or a broken stream. A 30-minute
204/// timeout that fires after the CLI has already edited fifteen files says
205/// nothing about whether those edits satisfy the contract — and a loop that
206/// returns terminal without asking has let the *transport* pronounce the
207/// verdict, which is precisely what this module exists to prevent. The state
208/// under judgement is the worktree, not the process that was writing to it.
209///
210/// Only two conditions skip evaluation, both because no work can exist yet:
211/// the engine never started ([`LoopFailure::EngineUnavailable`]) and the user
212/// cancelled ([`LoopFailure::Cancelled`]).
213pub async fn run_external_loop(
214    invoker: &dyn CliInvoker,
215    agent_id: &str,
216    intent: &str,
217    contract: &OutcomeContract,
218    executor: &WorktreeExecutor,
219    sink: &Arc<EventSink>,
220    cancel: &CancelFlag,
221    cfg: &ExternalLoopConfig,
222    // Daemon MCP URL, when bound. Routes the CLI's CAR-namespace tool calls
223    // through the daemon's policy + memgine. `None` degrades cleanly.
224    mcp_endpoint: Option<&str>,
225) -> LoopOutcome {
226    let max_hypotheses = 1 + cfg.repair_invokes;
227    let mut feedback: Option<String> = None;
228    let mut last_results = Vec::new();
229    // Hypotheses spent. Bumped only by a replan, so a transport retry buys
230    // another invocation without costing an attempt at being right.
231    let mut hypothesis = 1u32;
232    let mut transient_budget = cfg.transient_retries;
233    // Contract-evaluation rounds — what `LoopOutcome.iterations` means to its
234    // real consumer, `session.iterations` as reported by `coder.get`. A retry
235    // evaluates the contract, so it counts here even though it costs no
236    // hypothesis. (`ab::ArmOutcome.iterations` documents the same meaning but
237    // `coder_ab` hardcodes 0 today — `car code` emits no machine-readable count.)
238    let mut rounds = 0u32;
239    // Set when the previous pass was a transport retry: the hypothesis banner
240    // must not fire twice for one hypothesis.
241    let mut retrying = false;
242    // Failure signatures seen across hypotheses, so a repair that lands the
243    // identical failure is told so rather than handed the same text again.
244    let mut seen_sigs: HashMap<String, u32> = HashMap::new();
245    // This loop's clock starts here, before the first invocation.
246
247    // Metered spend across every invocation. `None` until something reports a
248    // figure, so "unmetered" stays distinguishable from "$0.00".
249    let mut spent_usd: Option<f64> = None;
250
251    loop {
252        if cancel.load(Ordering::SeqCst) {
253            return LoopOutcome::lost(
254                LoopFailure::Cancelled,
255                Some("cancelled".into()),
256                rounds,
257                last_results,
258            )
259            .with_cost(spent_usd);
260        }
261        // Admission, not interruption. A retry is admitted too — it is the same
262        // hypothesis re-run, and denying only fresh hypotheses would let a
263        // flaky CLI run past the ceiling indefinitely.
264        if let Some(reason) = cfg.deadline.admit() {
265            sink.emit(CoderEventKind::BudgetExhausted {
266                reason: reason.clone(),
267                elapsed_secs: cfg.deadline.elapsed_secs(),
268                iterations: rounds,
269            });
270            return LoopOutcome::lost(
271                LoopFailure::BudgetExhausted,
272                Some(reason),
273                rounds,
274                last_results,
275            )
276            .with_cost(spent_usd);
277        }
278        if !retrying {
279            sink.emit(CoderEventKind::IterationStarted {
280                n: hypothesis,
281                max: max_hypotheses,
282            });
283        }
284        retrying = false;
285
286        let task = build_task(intent, contract, feedback.as_deref());
287        let opts = build_invoke_opts(executor, cfg, mcp_endpoint);
288
289        let emitter_sink = sink.clone();
290        let emitter: StreamEventEmitter = Arc::new(move |event| {
291            if let Ok(raw) = serde_json::to_value(&event) {
292                emitter_sink.emit(CoderEventKind::ExternalEvent { raw });
293            }
294        });
295
296        // What the invocation itself reported, before the contract has spoken.
297        // `Ok(None)` = the CLI ran clean; `Ok(Some(msg))` = it ran and reported
298        // its own error; `Err(class)` = it died, and how.
299        let invocation: Result<Option<String>, (LoopFailure, String)> =
300            match invoker.invoke(agent_id, &task, opts, emitter).await {
301                Ok(result) if result.is_error => {
302                    record_spend(&mut spent_usd, result.total_cost_usd);
303                    let msg = result.error.unwrap_or_else(|| "unknown".into());
304                    sink.emit(CoderEventKind::Error {
305                        message: format!("external agent '{agent_id}' reported an error: {msg}"),
306                    });
307                    Ok(Some(msg))
308                }
309                Ok(result) => {
310                    record_spend(&mut spent_usd, result.total_cost_usd);
311                    Ok(None)
312                }
313                Err(e) => Err((classify_invoke_error(&e), e.to_string())),
314            };
315
316        // Two failures make evaluation meaningless rather than merely
317        // unnecessary: nothing ran, so no work can exist to judge. Matched
318        // variant-by-variant — a wildcard here would silently hand a future
319        // variant the string `"cancelled"` and the wrong terminal state.
320        let terminal = match &invocation {
321            // Prefix kept verbatim: `rpc` branches on the typed variant now, but
322            // `car-cli`'s A/B still scrapes this text out-of-process.
323            Err((LoopFailure::EngineUnavailable, msg)) => Some((
324                LoopFailure::EngineUnavailable,
325                format!("external agent '{agent_id}' failed: {msg}"),
326            )),
327            Err((LoopFailure::Cancelled, _)) => {
328                Some((LoopFailure::Cancelled, "cancelled".to_string()))
329            }
330            // `classify_invoke_error` produces none of these, but naming them
331            // keeps the match wildcard-free so a new variant fails to compile
332            // rather than silently acquiring a terminal it does not mean.
333            // `NeedsAuth` is native-loop-only: the external arm's credentials
334            // belong to the CLI it shells, so CAR has nothing to re-authenticate
335            // on its behalf and no standing to pause the run waiting for it.
336            Err((LoopFailure::Infrastructure, _))
337            | Err((LoopFailure::Configuration, _))
338            | Err((LoopFailure::NeedsAuth, _))
339            | Err((LoopFailure::Execution, _))
340            | Err((LoopFailure::Verification, _))
341            | Err((LoopFailure::BudgetExhausted, _))
342            | Ok(_) => None,
343        };
344        if let Some((failure, error)) = terminal {
345            return LoopOutcome::lost(failure, Some(error), rounds, last_results)
346                .with_cost(spent_usd);
347        }
348
349        // CAR's verdict, not the CLI's — and not the transport's.
350        last_results =
351            evaluate_contract_with_baselines(contract, executor, sink, &cfg.baseline_captures)
352                .await;
353        rounds += 1;
354        if last_results.iter().all(|r| r.passed) {
355            return LoopOutcome::green(rounds, last_results).with_cost(spent_usd);
356        }
357
358        // Red. Now — and only now — the failure has a class.
359        let failure = match &invocation {
360            Err((class, _)) => *class,
361            Ok(Some(_)) => LoopFailure::Execution,
362            Ok(None) => LoopFailure::Verification,
363        };
364        if failure == LoopFailure::Infrastructure && transient_budget > 0 {
365            // Availability, not correctness: re-invoke against the worktree as
366            // it now stands, carrying the failing checks so the retry is
367            // better-informed than the attempt it replaces.
368            transient_budget -= 1;
369            retrying = true;
370            sink.emit(CoderEventKind::InvocationRetried {
371                hypothesis,
372                reason: match &invocation {
373                    Err((_, msg)) => msg.clone(),
374                    Ok(_) => String::new(),
375                },
376                retries_remaining: transient_budget,
377            });
378            continue;
379        }
380
381        if hypothesis >= max_hypotheses {
382            // An exhausted Infrastructure failure must still LOOK like one.
383            // `car-cli`'s A/B splits infra out of the scored denominator by
384            // scraping this string (`coder_ab::INFRA_MARKERS`); leaving it
385            // `None` let `rpc` substitute "contract not satisfied after N
386            // iteration(s)", which scores a dead transport as a genuine task
387            // loss and quietly biases the arm it belongs to.
388            let error = match (failure, &invocation) {
389                (LoopFailure::Infrastructure, Err((_, msg))) => {
390                    Some(format!("external agent '{agent_id}' failed: {msg}"))
391                }
392                _ => None,
393            };
394            // Returns BEFORE the feedback below is built: nothing will read it,
395            // and rendering it means formatting every failing check's 4KB tail
396            // on the last round of every failing session. Ordering carries the
397            // invariant so a reader need not hold it: feedback is only built for
398            // a round that will actually happen.
399            return LoopOutcome::lost(failure, error, rounds, last_results).with_cost(spent_usd);
400        }
401
402        // Another round WILL happen, so build its handoff.
403        let check_feedback = render_check_failures(&last_results);
404        feedback = Some(match &invocation {
405            // The CLI's own error is context the checks cannot supply.
406            Ok(Some(msg)) => {
407                format!("A previous attempt reported this error:\n{msg}\n\n{check_feedback}")
408            }
409            Err((_, msg)) => format!(
410                "A previous attempt was cut short ({msg}); its work may be partially applied.\n\n\
411                 {check_feedback}"
412            ),
413            // Only a clean run earns a recurrence. NOT because the other cases
414            // were "cut short" — an `is_error` CLI may well have run to
415            // completion — but because `InvokeResult.is_error` is a
416            // heterogeneous bucket: it covers a non-zero exit, an empty answer,
417            // and "produced no agent_message" alike, so it cannot distinguish
418            // "hit a real wall" from "never produced anything". Counting the
419            // latter would inflate the tally against an attempt that did not
420            // happen. Accepted cost: a signature first seen on an `Execution`
421            // round is never recorded, so its count stays one low all session.
422            //
423            // The count is computed here, in the one arm that consumes it, so
424            // the policy is stated once. Hoisting it into a separate `if`
425            // duplicates this condition and silently zeroes any escalation a
426            // future arm might add.
427            Ok(None) => {
428                match record_recurrence(&mut seen_sigs, primary_failure(&last_results).as_ref()) {
429                    0 => check_feedback,
430                    n => format!("{check_feedback}\n\n{}", recurrence_notice(n)),
431                }
432            }
433        });
434
435        hypothesis += 1;
436    }
437}
438
439/// Map a transport-level error onto the outcome it implies.
440///
441/// The dividing question is whether the agent ever received the task, because
442/// that is what decides if any work can exist to judge:
443/// - `Spawn` / `Setup` — the process never started, or started and never got
444///   the prompt (pipes, stdin, MCP config). No work exists; another engine may
445///   be tried.
446/// - `Timeout` / `Io` — the agent had the task and the run died underneath it.
447///   Edits may be on disk, so the contract gets consulted and the same
448///   hypothesis may be retried.
449/// - `Cancelled` — the human stopped it. Never substitute another engine.
450///
451/// `Setup` exists because most of what used to be `Io` was this case: writing
452/// the MCP tempfile, acquiring stdout, delivering the prompt. Calling those
453/// retryable meant re-running a full contract evaluation against an untouched
454/// worktree and then declining the fallback that used to fire.
455fn classify_invoke_error(e: &car_external_agents::InvokeError) -> LoopFailure {
456    use car_external_agents::InvokeError as E;
457    match e {
458        E::Spawn(_) | E::Setup(_) => LoopFailure::EngineUnavailable,
459        E::Timeout(_) | E::Io(_) => LoopFailure::Infrastructure,
460        E::Cancelled => LoopFailure::Cancelled,
461    }
462}
463
464/// Fold one invocation's reported spend into the session total.
465///
466/// Stays `None` until a provider actually reports a figure, so a native run (or
467/// a CLI that reports nothing) is recorded as *unknown* rather than as $0.00 —
468/// the conflation that made `ab::ArmOutcome.cost_usd` a published zero.
469/// Non-finite or negative figures are ignored rather than allowed to poison the
470/// total.
471fn record_spend(total: &mut Option<f64>, reported: Option<f64>) {
472    let Some(usd) = reported else { return };
473    if !usd.is_finite() || usd < 0.0 {
474        return;
475    }
476    *total = Some(total.unwrap_or(0.0) + usd);
477}
478
479/// The failing half of a contract evaluation, rendered for a model to act on.
480fn render_check_failures(results: &[CheckResult]) -> String {
481    results
482        .iter()
483        .filter(|r| !r.passed)
484        .map(|r| {
485            format!(
486                "FAILED {} (exit {:?}):\n{}",
487                r.name, r.exit_code, r.output_tail
488            )
489        })
490        .collect::<Vec<_>>()
491        .join("\n\n")
492}
493
494#[cfg(test)]
495mod tests {
496    use std::collections::VecDeque;
497    use std::sync::atomic::AtomicU32;
498    use std::sync::Mutex;
499
500    use super::*;
501    use crate::coder::contract::ContractCheck;
502    use crate::coder::session::CoderEvent;
503    // Checks run through the coder's shell — `sh -lc` on Unix, `cmd /C` on
504    // Windows — so fixtures use the portable builders rather than POSIX
505    // literals. `true` is not a program on Windows (car#760).
506    use crate::coder::test_cmds::PASS;
507
508    fn contract() -> OutcomeContract {
509        OutcomeContract {
510            description: "x".into(),
511            checks: vec![ContractCheck {
512                name: "tests".into(),
513                command: "cargo test".into(),
514                expect_exit_zero: true,
515                output_contains: None,
516                timeout_secs: 300,
517                baseline: false,
518                differential: None,
519            }],
520        }
521    }
522
523    #[test]
524    fn task_carries_intent_contract_and_ground_rules() {
525        let t = build_task("add a CLI flag", &contract(), None);
526        assert!(t.contains("add a CLI flag"));
527        assert!(t.contains("cargo test"));
528        assert!(t.contains("Do NOT git commit"));
529        assert!(!t.contains("FAILING"));
530    }
531
532    #[test]
533    fn repair_task_carries_failure_feedback() {
534        let t = build_task("x", &contract(), Some("FAILED tests (exit Some(1)):\nboom"));
535        assert!(t.contains("previous attempt"));
536        assert!(t.contains("boom"));
537    }
538
539    #[test]
540    fn mcp_endpoint_is_threaded_into_invoke_opts() {
541        let dir = tempfile::tempdir().unwrap();
542        let executor = WorktreeExecutor::new(dir.path());
543        let cfg = ExternalLoopConfig::default();
544        let opts = build_invoke_opts(&executor, &cfg, Some("http://127.0.0.1:9102/mcp"));
545        assert_eq!(
546            opts.mcp_endpoint.as_deref(),
547            Some("http://127.0.0.1:9102/mcp")
548        );
549        // The CLI's own built-in tools stay on the default policy.
550        assert!(opts.allowed_tools.is_none());
551    }
552
553    #[test]
554    fn absent_mcp_endpoint_degrades_to_none() {
555        let dir = tempfile::tempdir().unwrap();
556        let executor = WorktreeExecutor::new(dir.path());
557        let cfg = ExternalLoopConfig::default();
558        let opts = build_invoke_opts(&executor, &cfg, None);
559        assert!(opts.mcp_endpoint.is_none());
560    }
561
562    // --- Failure classification ------------------------------------------
563
564    /// Pins the mapping. Note this is an array of literals, NOT the exhaustive
565    /// guard — a new upstream variant would compile straight past it. What
566    /// actually forces the decision is the wildcard-free match in
567    /// [`classify_invoke_error`], which fails to compile instead.
568    #[test]
569    fn every_invoke_error_maps_to_its_outcome() {
570        use car_external_agents::InvokeError as E;
571        let cases = [
572            (E::Spawn("no binary".into()), LoopFailure::EngineUnavailable),
573            // Pre-handoff I/O: the agent never received the task, so this is
574            // `Spawn`'s neighbour, not a retryable mid-run fault.
575            (
576                E::Setup("stdin closed".into()),
577                LoopFailure::EngineUnavailable,
578            ),
579            (E::Timeout(1800), LoopFailure::Infrastructure),
580            (E::Io("stdout read".into()), LoopFailure::Infrastructure),
581            (E::Cancelled, LoopFailure::Cancelled),
582        ];
583        for (err, want) in cases {
584            assert_eq!(classify_invoke_error(&err), want, "{err}");
585        }
586    }
587
588    /// The distinction the `Setup` split exists for: both are I/O, but one left
589    /// a worktree worth evaluating and the other could not have.
590    #[test]
591    fn setup_and_midrun_io_are_not_the_same_outcome() {
592        use car_external_agents::InvokeError as E;
593        assert_ne!(
594            classify_invoke_error(&E::Setup("stdin closed".into())),
595            classify_invoke_error(&E::Io("stdout read".into())),
596        );
597    }
598
599    // --- Loop behavior, against a scripted CLI ----------------------------
600
601    /// A CLI whose every invocation is scripted, so the loop's budgets, round
602    /// counting and retry policy are observable without a subprocess.
603    struct ScriptedInvoker {
604        script: Mutex<VecDeque<Result<InvokeResult, InvokeError>>>,
605        calls: AtomicU32,
606        /// Every task text handed over, in order — the only place the repair
607        /// feedback is observable from outside the loop.
608        tasks: Mutex<Vec<String>>,
609    }
610
611    impl ScriptedInvoker {
612        fn new(script: Vec<Result<InvokeResult, InvokeError>>) -> Self {
613            Self {
614                script: Mutex::new(script.into()),
615                calls: AtomicU32::new(0),
616                tasks: Mutex::new(Vec::new()),
617            }
618        }
619        fn calls(&self) -> u32 {
620            self.calls.load(Ordering::SeqCst)
621        }
622        fn task(&self, n: usize) -> String {
623            self.tasks.lock().expect("tasks poisoned")[n].clone()
624        }
625    }
626
627    #[async_trait]
628    impl CliInvoker for ScriptedInvoker {
629        async fn invoke(
630            &self,
631            _agent_id: &str,
632            task: &str,
633            _opts: InvokeOptions,
634            _emitter: StreamEventEmitter,
635        ) -> Result<InvokeResult, InvokeError> {
636            self.calls.fetch_add(1, Ordering::SeqCst);
637            self.tasks
638                .lock()
639                .expect("tasks poisoned")
640                .push(task.to_string());
641            self.script
642                .lock()
643                .expect("script poisoned")
644                .pop_front()
645                .unwrap_or_else(|| Err(InvokeError::Spawn("script exhausted".into())))
646        }
647    }
648
649    /// A contract whose single check always fails / always passes, cheaply.
650    fn contract_with(command: &str) -> OutcomeContract {
651        OutcomeContract {
652            description: "x".into(),
653            checks: vec![ContractCheck {
654                name: "gate".into(),
655                command: command.into(),
656                expect_exit_zero: true,
657                output_contains: None,
658                timeout_secs: 30,
659                baseline: false,
660                differential: None,
661            }],
662        }
663    }
664
665    fn clean_run() -> Result<InvokeResult, InvokeError> {
666        Ok(InvokeResult::default())
667    }
668
669    fn errored_run(msg: &str) -> Result<InvokeResult, InvokeError> {
670        Ok(InvokeResult {
671            is_error: true,
672            error: Some(msg.into()),
673            ..Default::default()
674        })
675    }
676
677    async fn run(
678        invoker: &dyn CliInvoker,
679        contract: &OutcomeContract,
680        cfg: &ExternalLoopConfig,
681    ) -> (LoopOutcome, Vec<CoderEvent>) {
682        let dir = tempfile::tempdir().unwrap();
683        let executor = WorktreeExecutor::new(dir.path());
684        let (sink, collected) = EventSink::collecting("t");
685        let sink = Arc::new(sink);
686        let cancel: CancelFlag = Arc::new(std::sync::atomic::AtomicBool::new(false));
687        let outcome = run_external_loop(
688            invoker, "codex", "x", contract, &executor, &sink, &cancel, cfg, None,
689        )
690        .await;
691        let events = collected.lock().unwrap().clone();
692        (outcome, events)
693    }
694
695    /// **The regression test for the core defect.** A dead transport must not
696    /// be able to fail a session whose worktree already satisfies the contract.
697    /// Before classification moved after evaluation, this returned an error.
698    #[tokio::test]
699    async fn a_timeout_over_green_checks_still_passes() {
700        let invoker = ScriptedInvoker::new(vec![Err(InvokeError::Timeout(1800))]);
701        let (outcome, _) = run(
702            &invoker,
703            &contract_with(PASS),
704            &ExternalLoopConfig::default(),
705        )
706        .await;
707        assert!(outcome.passed, "the contract, not the transport, decides");
708        assert_eq!(outcome.failure, None);
709        assert_eq!(outcome.iterations, 1);
710    }
711
712    /// A transient retry buys an invocation without spending a hypothesis.
713    /// Budgets: 1 + repair_invokes(1) hypotheses, transient_retries(1) retries
714    /// => exactly 3 invocations, and 3 contract-evaluation rounds.
715    #[tokio::test]
716    async fn a_transient_retry_does_not_spend_a_hypothesis() {
717        let invoker = ScriptedInvoker::new(vec![
718            Err(InvokeError::Timeout(1)),
719            Err(InvokeError::Timeout(1)),
720            Err(InvokeError::Timeout(1)),
721            Err(InvokeError::Timeout(1)),
722        ]);
723        let (outcome, events) = run(
724            &invoker,
725            &contract_with("exit 1"),
726            &ExternalLoopConfig::default(),
727        )
728        .await;
729        assert_eq!(invoker.calls(), 4, "3 hypotheses + 1 transient retry");
730        assert_eq!(outcome.iterations, 4, "every invocation evaluated");
731        // The retry gets its own event, and does NOT re-fire the hypothesis
732        // banner — otherwise `iteration 1/2` would print twice for one attempt.
733        let started = events
734            .iter()
735            .filter(|e| matches!(e.kind, CoderEventKind::IterationStarted { .. }))
736            .count();
737        let retried = events
738            .iter()
739            .filter(|e| matches!(e.kind, CoderEventKind::InvocationRetried { .. }))
740            .count();
741        assert_eq!(started, 3, "one banner per hypothesis");
742        assert_eq!(retried, 1);
743    }
744
745    /// Exhausting the transient budget must still LOOK infrastructural, or
746    /// `car-cli`'s A/B scores a dead transport as a genuine task loss and
747    /// biases the arm's pass rate.
748    #[tokio::test]
749    async fn exhausted_infrastructure_keeps_the_scraped_error_prefix() {
750        let invoker = ScriptedInvoker::new(vec![
751            Err(InvokeError::Timeout(1)),
752            Err(InvokeError::Timeout(1)),
753            Err(InvokeError::Timeout(1)),
754            Err(InvokeError::Timeout(1)),
755        ]);
756        let (outcome, _) = run(
757            &invoker,
758            &contract_with("exit 1"),
759            &ExternalLoopConfig::default(),
760        )
761        .await;
762        assert_eq!(outcome.failure, Some(LoopFailure::Infrastructure));
763        let err = outcome
764            .error
765            .expect("an exhausted infra failure must still surface as infra");
766        assert!(
767            err.starts_with("external agent '"),
768            "car-cli INFRA_MARKERS depends on this prefix: {err}"
769        );
770    }
771
772    /// Setup failures never reach the contract: nothing ran, so there is
773    /// nothing to evaluate, and the caller gets its fallback immediately.
774    #[tokio::test]
775    async fn a_setup_failure_does_not_retry_or_evaluate() {
776        let invoker = ScriptedInvoker::new(vec![Err(InvokeError::Setup("stdout missing".into()))]);
777        let (outcome, _) = run(
778            &invoker,
779            &contract_with("exit 1"),
780            &ExternalLoopConfig::default(),
781        )
782        .await;
783        assert_eq!(invoker.calls(), 1, "no retry: nothing ran");
784        assert_eq!(outcome.iterations, 0, "the contract was never consulted");
785        assert_eq!(outcome.failure, Some(LoopFailure::EngineUnavailable));
786        assert!(outcome.error.unwrap().starts_with("external agent '"));
787    }
788
789    /// A CLI that ran and reported its own error is `Execution`; one that ran
790    /// clean and simply got it wrong is `Verification`. Same action today, but
791    /// the repair feedback differs — only `Execution` has the CLI's own error
792    /// to pass back.
793    #[tokio::test]
794    async fn execution_and_verification_are_distinguished() {
795        let cfg = ExternalLoopConfig {
796            repair_invokes: 0,
797            ..Default::default()
798        };
799        let (errored, _) = run(
800            &ScriptedInvoker::new(vec![errored_run("tool denied")]),
801            &contract_with("exit 1"),
802            &cfg,
803        )
804        .await;
805        assert_eq!(errored.failure, Some(LoopFailure::Execution));
806        assert!(errored.error.is_none(), "the CLI ran; this is a task loss");
807
808        let (clean, _) = run(
809            &ScriptedInvoker::new(vec![clean_run()]),
810            &contract_with("exit 1"),
811            &cfg,
812        )
813        .await;
814        assert_eq!(clean.failure, Some(LoopFailure::Verification));
815    }
816
817    /// A repair that lands the IDENTICAL failure is told so, rather than handed
818    /// the same feedback text a second time. Before this, every repair round
819    /// re-sent the failing checks verbatim with no signal that the previous
820    /// attempt had changed nothing.
821    #[tokio::test]
822    async fn a_repeated_failure_escalates_the_repair_feedback() {
823        // Needs three hypotheses, not the default two: round 1 establishes the
824        // signature, round 2 is the first that can REPEAT it, and only round 3
825        // can be told. See `repair_invokes` on why the default cannot escalate.
826        let cfg = ExternalLoopConfig {
827            repair_invokes: 2,
828            ..Default::default()
829        };
830        let invoker = ScriptedInvoker::new(vec![clean_run(), clean_run(), clean_run()]);
831        let (outcome, _) = run(&invoker, &contract_with("exit 1"), &cfg).await;
832        assert_eq!(invoker.calls(), 3);
833        assert_eq!(outcome.failure, Some(LoopFailure::Verification));
834
835        // Rounds 1 and 2: nothing has repeated yet from the model's side.
836        assert!(!invoker.task(0).contains("failed the same way"));
837        assert!(!invoker.task(1).contains("failed the same way"));
838        // Round 3: round 2 reproduced round 1's signature exactly. Say so.
839        let repair = invoker.task(2);
840        assert!(repair.contains("failed the same way 2 times"), "{repair}");
841        assert!(repair.contains("DIFFERENT hypothesis"));
842    }
843
844    /// A transport failure must NOT escalate: the attempt was cut short before
845    /// it could have changed the outcome, so a repeated check result says
846    /// nothing about the hypothesis.
847    #[tokio::test]
848    async fn a_cut_short_attempt_does_not_escalate() {
849        let invoker = ScriptedInvoker::new(vec![
850            Err(InvokeError::Timeout(1)),
851            Err(InvokeError::Timeout(1)),
852            Err(InvokeError::Timeout(1)),
853            Err(InvokeError::Timeout(1)),
854        ]);
855        let (_, _) = run(
856            &invoker,
857            &contract_with("exit 1"),
858            &ExternalLoopConfig::default(),
859        )
860        .await;
861        for n in 0..invoker.calls() as usize {
862            assert!(
863                !invoker.task(n).contains("failed the same way"),
864                "a timeout is not evidence about the hypothesis (task {n})"
865            );
866        }
867    }
868
869    /// An exhausted session budget denies the FIRST admission — before any CLI
870    /// is invoked — and is its own terminal, not a task loss. Conflating it with
871    /// `Verification` would teach the recurrence machinery that an approach
872    /// failed when it was merely cut off.
873    #[tokio::test]
874    async fn an_exhausted_budget_denies_admission_before_invoking() {
875        let cfg = ExternalLoopConfig {
876            // A deadline that is already spent.
877            deadline: std::sync::Arc::new(SessionDeadline::new(Some(0))),
878            ..Default::default()
879        };
880        let invoker = ScriptedInvoker::new(vec![clean_run()]);
881        let (outcome, events) = run(&invoker, &contract_with("exit 1"), &cfg).await;
882        assert_eq!(invoker.calls(), 0, "the budget gates before any work");
883        assert_eq!(outcome.failure, Some(LoopFailure::BudgetExhausted));
884        assert_ne!(outcome.failure, Some(LoopFailure::Verification));
885        assert!(outcome
886            .error
887            .expect("the reason must surface")
888            .contains("session budget exhausted"));
889        assert!(events
890            .iter()
891            .any(|e| matches!(e.kind, CoderEventKind::BudgetExhausted { .. })));
892    }
893
894    /// **The clamp.** Admission grants a whole round, so without this a
895    /// hypothesis admitted just under the ceiling could run its full 1800s past
896    /// it — a ceiling exceeded by 50% is not a ceiling. The invocation's own
897    /// timeout is reduced to what the session has left.
898    #[test]
899    fn an_invocation_timeout_is_clamped_to_the_session_remainder() {
900        let dir = tempfile::tempdir().unwrap();
901        let executor = WorktreeExecutor::new(dir.path());
902
903        // 10s left on the session, 1800s asked for by the invocation.
904        let tight = ExternalLoopConfig {
905            timeout_secs: Some(1800),
906            deadline: std::sync::Arc::new(SessionDeadline::new(Some(10))),
907            ..Default::default()
908        };
909        let opts = build_invoke_opts(&executor, &tight, None);
910        assert_eq!(
911            opts.timeout_secs,
912            Some(10),
913            "the round must not outlive the session"
914        );
915
916        // Plenty of session left: the invocation keeps its own, smaller bound.
917        let roomy = ExternalLoopConfig {
918            timeout_secs: Some(60),
919            ..Default::default()
920        };
921        assert_eq!(
922            build_invoke_opts(&executor, &roomy, None).timeout_secs,
923            Some(60)
924        );
925
926        // No session ceiling: the invocation's own bound stands unchanged.
927        let unbounded = ExternalLoopConfig {
928            timeout_secs: Some(60),
929            deadline: SessionDeadline::unlimited(),
930            ..Default::default()
931        };
932        assert_eq!(
933            build_invoke_opts(&executor, &unbounded, None).timeout_secs,
934            Some(60)
935        );
936    }
937
938    /// The whole point of the `Arc`: a second rung of the fallback ladder gets
939    /// the SAME clock, not a fresh one. Before this, `external -> native` and
940    /// `foreman -> native` each restarted the ceiling.
941    #[test]
942    fn a_second_rung_shares_the_first_rungs_clock() {
943        let first = ExternalLoopConfig {
944            deadline: std::sync::Arc::new(SessionDeadline::new(Some(0))),
945            ..Default::default()
946        };
947        // How `rpc` builds the fallback rung: clone the handle, not the value.
948        let second = ExternalLoopConfig {
949            deadline: std::sync::Arc::clone(&first.deadline),
950            ..Default::default()
951        };
952        assert!(
953            std::sync::Arc::ptr_eq(&first.deadline, &second.deadline),
954            "the fallback must not buy the session another full ceiling"
955        );
956        assert!(
957            second.deadline.admit().is_some(),
958            "an already-spent session must stay spent across the ladder"
959        );
960    }
961
962    /// The default budget must not interfere with an ordinary session.
963    #[tokio::test]
964    async fn the_default_budget_does_not_gate_a_normal_run() {
965        let invoker = ScriptedInvoker::new(vec![clean_run()]);
966        let (outcome, _) = run(
967            &invoker,
968            &contract_with(PASS),
969            &ExternalLoopConfig::default(),
970        )
971        .await;
972        assert!(outcome.passed);
973        assert_eq!(invoker.calls(), 1);
974    }
975
976    /// A clean run that loses on the checks is a task loss, not an infra one —
977    /// it must NOT carry the scraped infra prefix.
978    #[tokio::test]
979    async fn a_verification_loss_carries_no_infra_marker() {
980        let cfg = ExternalLoopConfig {
981            repair_invokes: 0,
982            ..Default::default()
983        };
984        let (outcome, _) = run(
985            &ScriptedInvoker::new(vec![clean_run()]),
986            &contract_with("exit 1"),
987            &cfg,
988        )
989        .await;
990        assert_eq!(outcome.failure, Some(LoopFailure::Verification));
991        assert!(
992            outcome.error.is_none(),
993            "a genuine task loss must stay in the scored denominator"
994        );
995    }
996
997    /// The one test that exercises the REAL classification path end-to-end.
998    /// Every other loop test scripts the invoker, so without this nothing
999    /// verifies that a genuinely absent CLI still produces `Spawn` ->
1000    /// `EngineUnavailable` -> the scraped prefix. That is the standing cost of
1001    /// introducing a seam, and it is worth paying once.
1002    #[tokio::test]
1003    async fn a_missing_cli_is_engine_unavailable_through_the_live_invoker() {
1004        let dir = tempfile::tempdir().unwrap();
1005        let executor = WorktreeExecutor::new(dir.path());
1006        let sink = Arc::new(EventSink::test_sink());
1007        let cancel: CancelFlag = Arc::new(std::sync::atomic::AtomicBool::new(false));
1008        let outcome = run_external_loop(
1009            &LiveInvoker,
1010            "no-such-cli",
1011            "x",
1012            &contract(),
1013            &executor,
1014            &sink,
1015            &cancel,
1016            &ExternalLoopConfig::default(),
1017            None,
1018        )
1019        .await;
1020        assert!(!outcome.passed);
1021        assert_eq!(outcome.failure, Some(LoopFailure::EngineUnavailable));
1022        let err = outcome.error.expect("spawn failure must surface");
1023        assert!(err.starts_with("external agent '"), "{err}");
1024        assert!(err.contains("no-such-cli"), "{err}");
1025    }
1026
1027    /// Cancellation must stay distinguishable from "the engine could not run",
1028    /// because `rpc` starts a native loop on the latter and moves to a
1029    /// different terminal state. Conflating them starts work the user stopped.
1030    #[tokio::test]
1031    async fn cancellation_is_not_engine_unavailable() {
1032        let dir = tempfile::tempdir().unwrap();
1033        let executor = WorktreeExecutor::new(dir.path());
1034        let sink = Arc::new(EventSink::test_sink());
1035        let cancel: CancelFlag = Arc::new(std::sync::atomic::AtomicBool::new(true));
1036        let invoker = ScriptedInvoker::new(vec![]);
1037        let outcome = run_external_loop(
1038            &invoker,
1039            "claude-code",
1040            "x",
1041            &contract(),
1042            &executor,
1043            &sink,
1044            &cancel,
1045            &ExternalLoopConfig::default(),
1046            None,
1047        )
1048        .await;
1049        assert_eq!(invoker.calls(), 0, "pre-cancelled must not invoke");
1050        assert_eq!(outcome.failure, Some(LoopFailure::Cancelled));
1051        assert_ne!(outcome.failure, Some(LoopFailure::EngineUnavailable));
1052        // The bare spelling `rpc` and the A/B both still read.
1053        assert_eq!(outcome.error.as_deref(), Some("cancelled"));
1054    }
1055
1056    #[test]
1057    fn rendered_feedback_carries_only_failing_checks() {
1058        let results = vec![
1059            CheckResult {
1060                name: "build".into(),
1061                passed: true,
1062                exit_code: Some(0),
1063                output_tail: "ok".into(),
1064                duration_ms: 1,
1065                timed_out: false,
1066                deadline_clamped: false,
1067            },
1068            CheckResult {
1069                name: "tests".into(),
1070                passed: false,
1071                exit_code: Some(1),
1072                output_tail: "assertion failed".into(),
1073                duration_ms: 2,
1074                timed_out: false,
1075                deadline_clamped: false,
1076            },
1077        ];
1078        let rendered = render_check_failures(&results);
1079        assert!(rendered.contains("FAILED tests"));
1080        assert!(rendered.contains("assertion failed"));
1081        assert!(!rendered.contains("build"), "passing checks are noise");
1082    }
1083}