Skip to main content

borg/
review_fix.rs

1//! `borg review-fix`: the bugs a review step found and a fix step fixed, inside a session, before
2//! anything was committed. Most bugs are caught there (a build → review → verify → fix workflow), and a
3//! commit-based signal (regress links: a later commit from another session) never sees them.
4//!
5//! Source: each harness's runs of sub-agents (`Harness::sub_runs`: a Claude Code workflow run or a
6//! session's Agent-tool sub-agents, an omp session's task agents, an opencode session's subagents, a
7//! Codex session's spawned agents), each agent with its label, phase, kind,
8//! start, prompt, work and output. Workflow scripts and prompts are the engineer's own, so output
9//! shapes vary; this reads them tolerantly from each agent's output (Claude's last `StructuredOutput`,
10//! omp's yields, else a JSON object in its text, [`output_of`]):
11//! - **finding**: an object in a `findings` or `issues` array with a severity (or a `priority`) and a
12//!   text (`title`, `claim`, `problem`, `description`, `issue` or `body`), and no verdict of its own;
13//! - **verdict**: an output with a boolean `real`, or items of a `verdicts`/`results` array with `real`
14//!   or a `verdict` word (confirmed, refuted, …), linked to a finding of the same run by title (or, for
15//!   a one-finding verifier, by its label or prompt naming the title);
16//! - **fixed**: a fix agent of the run ([`is_fix`]), started after the finding, whose tool
17//!   calls or final text name the finding's file. Agents edit mostly through Bash, so tool inputs of
18//!   every kind count, not only Edit/Write.
19//!
20//! The builder (the episode's task and proposal) is the run's first agent that is none of these; a
21//! loose run (`Run::loose`: a plain session's own sub-agents) has none, since its session's own agent
22//! built.
23//!
24//! A finding is **strong** when it was fixed and no verifier rejected it. The negative control asks the
25//! same "fixed" question of a fix agent from another session's run: its rate is how often the file rule
26//! matches by chance.
27//!
28//! 0 tokens: nothing calls a model. Every finding is routed by its agent's cwd (`collective::route`); an
29//! off repo is only counted.
30
31use std::collections::BTreeMap;
32use std::path::{Path, PathBuf};
33
34use serde::Serialize;
35use serde_json::Value;
36
37use crate::collective::{self, Config, Route};
38use crate::harness::{self, Harness, Run, SubAgent};
39use crate::learn::{self, Episode, regret};
40
41/// Its flags (completions.rs).
42pub const FLAGS: &[&str] = &["--home", "--since", "--json", "--apply", "--only"];
43
44const USAGE: &str = "usage: borg review-fix [--home DIR] [--since YYYY-MM-DD] [--json] [--apply [--only ID,…]]";
45
46#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)]
47#[serde(rename_all = "snake_case")]
48pub enum Verdict {
49    Confirmed,
50    Rejected,
51    /// No verifier's verdict links to it.
52    Unchecked,
53}
54
55#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)]
56#[serde(rename_all = "snake_case")]
57pub enum Fixed {
58    Yes,
59    /// The run had a fix step after the finding, and it never named the finding's file.
60    No,
61    /// The run had no fix step after the finding.
62    NoFixStep,
63    /// The finding names no file to look for.
64    Unlocated,
65}
66
67#[derive(Debug, Clone, Serialize)]
68pub struct Finding {
69    /// `<runId>#<n>`, n counting the run's findings in agent start order: stable across replays.
70    pub id: String,
71    pub session: String,
72    pub run: String,
73    pub workflow: String,
74    /// The reviewing agent's label (`review:rust`).
75    pub agent: String,
76    pub ms: i64,
77    pub cwd: String,
78    pub file: Option<String>,
79    pub title: String,
80    /// The reviewer's explanation (`detail`, `evidence` or `reason`), at most `DETAIL_CHARS`.
81    pub detail: String,
82    pub severity: String,
83    pub verdict: Verdict,
84    pub fixed: Fixed,
85    /// The prompt of the run's builder (its first agent that neither reviews, verifies nor fixes),
86    /// at most `DETAIL_CHARS` × 2; empty without one.
87    pub task: String,
88    /// The end of the builder's own account of its work, at most `DETAIL_CHARS` × 2.
89    pub built: String,
90}
91
92impl Finding {
93    /// Fixed, and no verifier said it was not real.
94    pub fn strong(&self) -> bool {
95        self.fixed == Fixed::Yes && self.verdict != Verdict::Rejected
96    }
97
98    /// The episode `borg learn` generalises with the review regret prompt (learn/regret.rs). Its id is
99    /// `V` + the finding's id, the dedupe key; its engineer is whoever works in the finding's repo.
100    /// Every text is redacted before it is written.
101    pub fn episode(&self, engineer: &str) -> Episode {
102        let red = |t: &str| crate::notes::redact(t);
103        let verdict = match self.verdict {
104            Verdict::Confirmed => "confirmed",
105            _ => "unchecked",
106        };
107        let detail = match &self.file {
108            Some(f) => format!("{f}: {}", self.detail),
109            None => self.detail.clone(),
110        };
111        let r = regret::Regret {
112            decision: self.run.clone(),
113            decision_session: self.session.clone(),
114            decision_task: red(&self.task),
115            decision_proposal: red(&self.built),
116            fix: self.id.clone(),
117            fix_subject: red(&self.title),
118            diff: red(&detail),
119            signals: vec![regret::REVIEW.into(), verdict.into()],
120            // The builder decided before the reviewer found it.
121            decision_ms: self.ms,
122            ..regret::Regret::default()
123        };
124        Episode {
125            id: format!("V{}", self.id),
126            ms: self.ms,
127            repo: borg_trace::repo_key(&self.cwd).rsplit('/').next().unwrap_or_default().to_string(),
128            session: self.session.clone(),
129            task: r.decision_task.clone(),
130            agent: r.decision_proposal.clone(),
131            rejected_tool: None,
132            correction: format!("Review finding ({}, {verdict}), fixed before any commit: {}\n{}", self.severity, r.fix_subject, r.diff),
133            engineer: engineer.to_string(),
134            kind: regret::KIND.into(),
135            regret: Some(r),
136        }
137    }
138}
139
140/// The lowercase words of a label, phase or kind: split at non-alphanumerics and at CamelCase humps
141/// (`MappingValidityFix` → mapping, validity, fix).
142fn words(s: &str) -> Vec<String> {
143    let mut out: Vec<String> = Vec::new();
144    let mut prev_lower = false;
145    for c in s.chars() {
146        if !c.is_alphanumeric() {
147            prev_lower = false;
148            out.push(String::new());
149            continue;
150        }
151        if c.is_uppercase() && prev_lower || out.is_empty() {
152            out.push(String::new());
153        }
154        prev_lower = c.is_lowercase() || c.is_ascii_digit();
155        out.last_mut().expect("pushed").extend(c.to_lowercase());
156    }
157    out.retain(|w| !w.is_empty());
158    out
159}
160
161/// Says fix or repair as a whole word (`fix`, `harness-fix`, `DeriveFacetsRepairs`), and neither
162/// review, verify nor implement.
163fn says_fix(s: &str) -> bool {
164    let w = words(s);
165    w.iter().any(|w| ["fix", "fixes", "fixer", "fixing", "repair", "repairs", "repairer"].contains(&w.as_str()))
166        && !w.iter().any(|w| ["review", "verif", "impl"].iter().any(|n| w.starts_with(n)))
167}
168
169/// A fix step: its phase, its label's first segment (`fix:rust`, not `verify:fix-correctness`) or its
170/// kind says fix (or repair), it is in no review, verify or implement phase, and its output has no
171/// findings (not even an empty list) and no verdicts.
172fn is_fix(a: &SubAgent) -> bool {
173    let phase = words(&a.phase);
174    if phase.iter().any(|w| ["review", "verif", "impl"].iter().any(|n| w.starts_with(n))) {
175        return false;
176    }
177    let first = a.label.split([':', '#']).next().unwrap_or_default();
178    let listed = a.output.as_ref().is_some_and(|o| ["findings", "issues"].iter().any(|k| o.get(*k).is_some_and(Value::is_array)));
179    (says_fix(&a.phase) || says_fix(first) || says_fix(&a.kind)) && !listed && verdicts_of(a.output.as_ref()).is_empty()
180}
181
182/// An agent's output: its structured one, else the last JSON object in its final text that has a
183/// `findings` or `issues` array and starts a line (a fenced block's too). A harness without structured
184/// output (opencode) yields findings only from a reviewer told to answer in JSON; tool inputs are never
185/// read, so a fix agent repeating the findings stays a fix agent.
186fn output_of(a: &SubAgent) -> Option<Value> {
187    if a.output.is_some() {
188        return a.output.clone();
189    }
190    let w = a.final_text.as_str();
191    let listed = |v: &Value| ["findings", "issues"].iter().any(|k| v.get(*k).is_some_and(Value::is_array));
192    let starts = w.match_indices('{').map(|(i, _)| i).filter(|&i| w[w[..i].rfind('\n').map_or(0, |n| n + 1)..i].trim().is_empty());
193    starts.rev().find_map(|i| serde_json::Deserializer::from_str(&w[i..]).into_iter::<Value>().next()?.ok().filter(listed))
194}
195
196/// A finding's detail is cut to this many characters.
197pub const DETAIL_CHARS: usize = 800;
198
199const TEXT_KEYS: &[&str] = &["title", "claim", "problem", "description", "issue", "body"];
200const DETAIL_KEYS: &[&str] = &["detail", "evidence", "reason", "body"];
201const FILE_KEYS: &[&str] = &["file", "path", "file_path", "fileLines", "where"];
202
203fn field<'a>(o: &'a Value, keys: &[&str]) -> Option<&'a str> {
204    keys.iter().find_map(|k| o.get(*k).and_then(Value::as_str)).map(str::trim).filter(|s| !s.is_empty())
205}
206
207/// A verdict word or flag (`real`, `keep`, `refuted`), when `o` carries one.
208fn verdict_of(o: &Value) -> Option<Verdict> {
209    let flag = |k: &str| o.get(k).and_then(Value::as_bool);
210    if let Some(r) = flag("real").or_else(|| flag("keep")).or_else(|| flag("refuted").map(|r| !r)) {
211        return Some(if r { Verdict::Confirmed } else { Verdict::Rejected });
212    }
213    let w = o.get("verdict").and_then(Value::as_str)?.to_lowercase();
214    // A plausible finding survived the attempt to refute it.
215    if ["confirm", "real", "valid", "holds", "true", "plausible"].iter().any(|k| w.starts_with(k)) {
216        Some(Verdict::Confirmed)
217    } else if ["refut", "reject", "false", "invalid", "not"].iter().any(|k| w.starts_with(k)) {
218        Some(Verdict::Rejected)
219    } else {
220        None
221    }
222}
223
224/// A finding as a reviewer's output states it.
225#[derive(Debug, PartialEq)]
226struct Raw {
227    id: Option<String>,
228    title: String,
229    detail: String,
230    severity: String,
231    file: Option<String>,
232}
233
234/// The findings in an agent's output.
235fn findings_of(output: Option<&Value>) -> Vec<Raw> {
236    let Some(o) = output else { return Vec::new() };
237    ["findings", "issues"]
238        .iter()
239        .filter_map(|k| o.get(*k).and_then(Value::as_array))
240        .flatten()
241        .filter(|f| f.is_object() && verdict_of(f).is_none())
242        .filter_map(|f| {
243            let title = field(f, TEXT_KEYS)?;
244            let severity = severity_of(f)?;
245            let detail = field(f, DETAIL_KEYS).filter(|d| *d != title).unwrap_or_default().chars().take(DETAIL_CHARS).collect();
246            Some(Raw { id: field(f, &["id"]).map(String::from), title: title.to_string(), detail, severity, file: field(f, FILE_KEYS).and_then(path_in) })
247        })
248        .collect()
249}
250
251/// A finding's severity, lowercase: its `severity`, else its `priority` (0 to 3, `P1`) as `p0` to `p3`.
252fn severity_of(f: &Value) -> Option<String> {
253    if let Some(s) = field(f, &["severity"]) {
254        return Some(s.to_lowercase());
255    }
256    match f.get("priority")? {
257        Value::Number(n) => n.as_u64().map(|n| format!("p{n}")),
258        Value::String(s) => {
259            let s = s.trim().to_lowercase();
260            let n = s.strip_prefix('p').unwrap_or(&s);
261            Some(if n.parse::<u8>().is_ok() { format!("p{n}") } else { s }).filter(|s| !s.is_empty())
262        }
263        _ => None,
264    }
265}
266
267/// A verdict and what it names: a finding's id or title, or nothing (one verifier per finding).
268#[derive(Debug)]
269struct Said {
270    id: Option<String>,
271    title: Option<String>,
272    verdict: Verdict,
273}
274
275/// The verdicts in an agent's output.
276fn verdicts_of(output: Option<&Value>) -> Vec<Said> {
277    let Some(o) = output else { return Vec::new() };
278    if o.get("findings").is_none()
279        && let Some(verdict) = verdict_of(o)
280    {
281        return vec![Said { id: None, title: None, verdict }];
282    }
283    ["verdicts", "results"]
284        .iter()
285        .filter_map(|k| o.get(*k).and_then(Value::as_array))
286        .flatten()
287        .filter_map(|x| Some(Said { id: field(x, &["id"]).map(String::from), title: field(x, TEXT_KEYS).map(String::from), verdict: verdict_of(x)? }))
288        .collect()
289}
290
291/// The part of a label after its last `:` (`verify:rust` → `rust`), the dimension both sides share.
292fn dim(label: &str) -> &str {
293    label.rsplit(':').next().unwrap_or_default()
294}
295
296/// The first path-like token of `s` (`src/a.rs:12-14 (…)` → `src/a.rs`).
297fn path_in(s: &str) -> Option<String> {
298    s.split(|c: char| c.is_whitespace() || "`'\"(),;".contains(c))
299        .map(|t| t.split(':').next().unwrap_or(t).trim_end_matches(['.', '§']))
300        .find(|t| t.contains('.') && t.len() > 2 && !t.starts_with("http") && t.chars().any(char::is_alphabetic))
301        .map(String::from)
302}
303
304fn norm(s: &str) -> String {
305    s.chars().filter(|c| c.is_alphanumeric()).flat_map(char::to_lowercase).collect()
306}
307
308/// `a` and `b` name the same finding: one's normalised text starts with the other's (at least 12
309/// characters, so a label's truncated title still links).
310fn same_title(a: &str, b: &str) -> bool {
311    let (a, b) = (norm(a), norm(b));
312    let n = a.len().min(b.len());
313    n >= 12 && a[..n] == b[..n]
314}
315
316/// Does `work` name `file`? The path as written, or its last two components.
317fn names(work: &str, file: &str) -> bool {
318    let tail: Vec<&str> = file.rsplit('/').take(2).collect();
319    let short = tail.iter().rev().copied().collect::<Vec<_>>().join("/");
320    work.contains(file) || (short.len() > 3 && work.contains(&short))
321}
322
323/// The findings of one run, with their verdicts and whether a fix step fixed them.
324fn run_findings(run: &Run) -> Vec<Finding> {
325    let agents = &run.agents;
326    let verdicts: Vec<(&SubAgent, Said)> = agents.iter().flat_map(|a| verdicts_of(a.output.as_ref()).into_iter().map(move |v| (a, v))).collect();
327    let raws: Vec<(&SubAgent, Raw)> = agents.iter().flat_map(|a| findings_of(a.output.as_ref()).into_iter().map(move |r| (a, r))).collect();
328    // The builder: the first agent that produced neither findings nor verdicts and is no fix step; none
329    // in a loose run, where the session's own agent built and its sub-agents only searched or checked.
330    let builder =
331        agents.iter().filter(|_| !run.loose).find(|b| !is_fix(b) && findings_of(b.output.as_ref()).is_empty() && verdicts_of(b.output.as_ref()).is_empty());
332    let n = DETAIL_CHARS * 2;
333    let builder_repo = builder.map(|b| borg_trace::repo_key(&b.cwd));
334    let (task, built) = builder.map_or_else(Default::default, |b| {
335        let w: Vec<char> = b.work.chars().collect();
336        (b.prompt.chars().take(n).collect::<String>(), w[w.len().saturating_sub(n)..].iter().collect::<String>())
337    });
338    let mut out = Vec::new();
339    for (k, (a, Raw { id, title, detail, severity, file })) in raws.iter().enumerate() {
340        let head: String = title.chars().take(60).collect();
341        // An id links when it names one finding of the run, or one of the verifier's own dimension
342        // (reviewers number their findings F1, F2, … each); a title by its text; a verifier that
343        // names neither checks one finding, named by its label or prompt.
344        let same_id = |v: &str| raws.iter().filter(|(_, r)| r.id.as_deref() == Some(v)).count();
345        let linked = verdicts.iter().filter(|(va, said)| match (&said.id, &said.title) {
346            (Some(v), _) if id.as_deref() == Some(v.as_str()) => same_id(v) == 1 || dim(&va.label) == dim(&a.label),
347            (_, Some(t)) => same_title(t, title),
348            (Some(_), None) => false,
349            (None, None) => same_title(dim(&va.label), title) || va.prompt.contains(&head),
350        });
351        let verdict =
352            linked.fold(
353                Verdict::Unchecked,
354                |acc, (_, said)| if said.verdict == Verdict::Rejected || acc == Verdict::Rejected { Verdict::Rejected } else { said.verdict },
355            );
356        let fixers: Vec<&SubAgent> = agents.iter().filter(|f| is_fix(f) && f.started >= a.started).collect();
357        let fixed = match file {
358            _ if fixers.is_empty() => Fixed::NoFixStep,
359            None => Fixed::Unlocated,
360            Some(f) if fixers.iter().any(|x| names(&x.work, f)) => Fixed::Yes,
361            Some(_) => Fixed::No,
362        };
363        // The builder's text only when it worked in the reviewer's repo: the episode is routed by
364        // the reviewer's, and another repo's text must not land in its collective.
365        let same_repo = builder_repo.as_deref() == Some(borg_trace::repo_key(&a.cwd).as_str());
366        out.push(Finding {
367            // A loose run's agents finish in any order, so a finding there is named by its agent: a
368            // later finisher that started earlier must not renumber findings already recorded.
369            id: if run.loose {
370                format!("{}#{}.{}", run.id, a.id, raws[..k].iter().filter(|(b, _)| std::ptr::eq(*b, *a)).count())
371            } else {
372                format!("{}#{k}", run.id)
373            },
374            session: run.session.clone(),
375            run: run.id.clone(),
376            workflow: run.name.clone(),
377            agent: a.label.clone(),
378            ms: a.started,
379            cwd: a.cwd.clone(),
380            file: file.clone(),
381            title: title.clone(),
382            detail: detail.clone(),
383            severity: severity.clone(),
384            verdict,
385            fixed,
386            task: if same_repo { task.clone() } else { String::new() },
387            built: if same_repo { built.clone() } else { String::new() },
388        });
389    }
390    out
391}
392
393/// The runs of `harnesses` under `home` (of `session` only, when given), oldest first, each agent's
394/// output read from its text where it has no structured one ([`output_of`]).
395fn runs_of<'h>(harnesses: impl Iterator<Item = &'h dyn Harness>, home: &Path, since: Option<i64>, session: Option<&str>) -> Vec<Run> {
396    prepared(harnesses.flat_map(|h| h.sub_runs(home, session, since)).collect())
397}
398
399fn prepared(mut runs: Vec<Run>) -> Vec<Run> {
400    for a in runs.iter_mut().flat_map(|r| r.agents.iter_mut()) {
401        a.output = output_of(a);
402    }
403    runs.sort_by_key(|r| r.started);
404    runs
405}
406
407/// The findings of `runs`, and every run with a fix step, for the control.
408fn scan(runs: &[Run]) -> (Vec<Finding>, Vec<Fixers>) {
409    let mut findings = Vec::new();
410    let mut fixers = Vec::new();
411    for run in runs {
412        findings.extend(run_findings(run));
413        let fix: Vec<&SubAgent> = run.agents.iter().filter(|a| is_fix(a)).collect();
414        if let Some(first) = fix.first() {
415            let repo = borg_trace::repo_key(&first.cwd);
416            fixers.push(Fixers { session: run.session.clone(), run: run.id.clone(), repo, work: fix.iter().map(|a| a.work.clone()).collect() });
417        }
418    }
419    (findings, fixers)
420}
421
422/// A run's fix steps: what they did, and the repo they did it in.
423#[derive(Debug)]
424struct Fixers {
425    session: String,
426    run: String,
427    repo: String,
428    work: Vec<String>,
429}
430
431/// Counts for one collective, or for the off repos (only counted).
432#[derive(Debug, Default, Clone, Serialize)]
433pub struct Tally {
434    pub runs: usize,
435    pub findings: usize,
436    pub confirmed: usize,
437    pub rejected: usize,
438    pub unchecked: usize,
439    pub fixed: usize,
440    pub not_fixed: usize,
441    pub no_fix_step: usize,
442    pub unlocated: usize,
443    pub strong: usize,
444}
445
446impl Tally {
447    fn add(&mut self, f: &Finding) {
448        self.findings += 1;
449        match f.verdict {
450            Verdict::Confirmed => self.confirmed += 1,
451            Verdict::Rejected => self.rejected += 1,
452            Verdict::Unchecked => self.unchecked += 1,
453        }
454        match f.fixed {
455            Fixed::Yes => self.fixed += 1,
456            Fixed::No => self.not_fixed += 1,
457            Fixed::NoFixStep => self.no_fix_step += 1,
458            Fixed::Unlocated => self.unlocated += 1,
459        }
460        self.strong += usize::from(f.strong());
461    }
462}
463
464/// The negative control: of the located findings, how often the fix step of another session's run in
465/// the same repo names the finding's file (chance: the same files keep being worked on), against how
466/// often the run's own fix step does. Findings with no such other run are `unpaired`.
467#[derive(Debug, Default, Clone, Serialize)]
468pub struct Control {
469    pub unpaired: usize,
470    pub located: usize,
471    pub own_named: usize,
472    pub other_named: usize,
473}
474
475#[derive(Debug, Default, Serialize)]
476pub struct Report {
477    pub collectives: BTreeMap<String, Tally>,
478    pub off: Tally,
479    pub control: Control,
480    /// The findings in repos that route to a collective (never off ones).
481    pub findings: Vec<Finding>,
482}
483
484/// Route, count and control the findings of every harness's runs under `home`.
485pub fn run(home: &Path, since: Option<i64>, cfg: Option<&Config>, base: &Path) -> Report {
486    report(&runs_of(harness::ALL.iter().copied(), home, since, None), cfg, base)
487}
488
489/// Route, count and control the findings of `runs`.
490fn report(runs: &[Run], cfg: Option<&Config>, base: &Path) -> Report {
491    let (findings, fixers) = scan(runs);
492    let mut r = Report::default();
493    let mut runs: BTreeMap<String, std::collections::BTreeSet<String>> = BTreeMap::new();
494    for f in findings {
495        let name = match collective::route(cfg, &f.cwd, base) {
496            Route::Off => None,
497            Route::Collective { name, .. } => Some(name),
498        };
499        let Some(name) = name else {
500            r.off.add(&f);
501            continue;
502        };
503        if let Some(file) = f.file.as_deref().filter(|_| f.fixed != Fixed::NoFixStep) {
504            // The control pairs each finding with the next run, in time order (wrapping), of another
505            // session in the same repo that has a fix step.
506            let repo = borg_trace::repo_key(&f.cwd);
507            let at = fixers.iter().position(|x| x.run == f.run).map_or(0, |i| i + 1);
508            match fixers.iter().cycle().skip(at).take(fixers.len()).find(|x| x.session != f.session && x.repo == repo) {
509                Some(other) => {
510                    r.control.located += 1;
511                    r.control.own_named += usize::from(f.fixed == Fixed::Yes);
512                    r.control.other_named += usize::from(other.work.iter().any(|w| names(w, file)));
513                }
514                None => r.control.unpaired += 1,
515            }
516        }
517        runs.entry(name.clone()).or_default().insert(f.run.clone());
518        r.collectives.entry(name).or_default().add(&f);
519        r.findings.push(f);
520    }
521    for (name, set) in runs {
522        r.collectives.entry(name).or_default().runs = set.len();
523    }
524    r
525}
526
527fn pct(n: usize, d: usize) -> String {
528    if d == 0 { "-".into() } else { format!("{:.0}%", 100.0 * n as f64 / d as f64) }
529}
530
531pub fn render(r: &Report) -> String {
532    let mut s = String::new();
533    for (name, t) in &r.collectives {
534        s.push_str(&format!(
535            "{name}: {} findings in {} workflow runs; verdict: {} confirmed, {} rejected, {} unchecked; fix: {} fixed, {} not named by the fix step, {} without a fix step, {} without a file; {} strong\n",
536            t.findings, t.runs, t.confirmed, t.rejected, t.unchecked, t.fixed, t.not_fixed, t.no_fix_step, t.unlocated, t.strong
537        ));
538    }
539    if r.collectives.is_empty() {
540        s.push_str("no workflow findings in repos of a collective\n");
541    }
542    let c = &r.control;
543    s.push_str(&format!(
544        "control: of {} located findings, the run's own fix step names the file {} ({}), another session's fix step in the same repo {} ({}); {} without such a session\n",
545        c.located,
546        c.own_named,
547        pct(c.own_named, c.located),
548        c.other_named,
549        pct(c.other_named, c.located),
550        c.unpaired
551    ));
552    s.push_str(&format!("off: {} findings skipped (not shown)\n", r.off.findings));
553    s
554}
555
556/// Record the strong findings (those in `only`, when given) as episodes in their collectives' stores,
557/// skipping known ones, then learn each store that got new episodes. Returns (recorded, learned
558/// lessons, merged).
559pub fn apply(report: &Report, only: Option<&[String]>, cfg: Option<&Config>, base: &Path, ask: &dyn Fn(&str) -> Option<Value>) -> (usize, usize, usize) {
560    let (recorded, homes) = record(report.findings.iter().filter(|f| only.is_none_or(|o| o.contains(&f.id))), cfg, base);
561    let (mut new, mut merged) = (0, 0);
562    for home in &homes {
563        let (n, m, _) = learn::learn_in_reporting(home, None, ask, learn::learning(), learn::now_ms(), &mut learn::print_step);
564        new += n;
565        merged += m;
566    }
567    (recorded, new, merged)
568}
569
570/// Record the strong ones of `findings` as episodes in the stores their repos route to, skipping
571/// known ones and off repos. Returns how many were recorded and the stores that got any.
572fn record<'a>(findings: impl Iterator<Item = &'a Finding>, cfg: Option<&Config>, base: &Path) -> (usize, Vec<PathBuf>) {
573    let mut homes: Vec<PathBuf> = Vec::new();
574    let mut recorded = 0;
575    let mut engineers: BTreeMap<String, String> = BTreeMap::new();
576    for f in findings.filter(|f| f.strong()) {
577        let Route::Collective { home, .. } = collective::route(cfg, &f.cwd, base) else { continue };
578        let engineer = engineers.entry(f.cwd.clone()).or_insert_with(|| learn::engineer(&f.cwd)).clone();
579        let e = f.episode(&engineer);
580        if crate::notes::regret::known(&home, &e.id) {
581            continue;
582        }
583        if learn::record_episode_in(&home, &e).is_ok() {
584            recorded += 1;
585            crate::stats::count(&home, crate::stats::REVIEW_FINDING, &f.session, |ev| {
586                ev.engineer = engineer.clone();
587                ev.repo = e.repo.clone();
588                ev.why = f.severity.clone();
589                ev.text = e.regret.as_ref().map(|r| r.fix_subject.clone()).unwrap_or_default();
590            });
591            if !homes.contains(&home) {
592                homes.push(home);
593            }
594        }
595    }
596    (recorded, homes)
597}
598
599/// Session end records the session's strong review findings as episodes before `borg learn`
600/// generalises them: on by default (the value eval, docs/evidence.md), `BORG_REVIEW_FIX=off` turns it off.
601pub fn on() -> bool {
602    std::env::var("BORG_REVIEW_FIX").as_deref() != Ok("off")
603}
604
605/// At the end of `session`: record the strong findings of its runs under `home`, as the drone's harness
606/// writes them (`BORG_HARNESS`; every harness when unset). Returns how many were recorded and the
607/// stores that got any, so the caller learns each.
608pub fn record_session(home: &Path, session: &str, cfg: Option<&Config>, base: &Path) -> (usize, Vec<PathBuf>) {
609    // A session id names a directory or a file, never a path.
610    if session.is_empty() || session.contains(['/', '\\']) || session.starts_with('.') {
611        return (0, Vec::new());
612    }
613    let runs = match harness::named_current() {
614        Some(h) => runs_of(std::iter::once(h), home, None, Some(session)),
615        None => runs_of(harness::ALL.iter().copied(), home, None, Some(session)),
616    };
617    record(scan(&runs).0.iter(), cfg, base)
618}
619
620/// The engineer's home, where every harness keeps its data.
621pub fn home_dir() -> PathBuf {
622    PathBuf::from(std::env::var("HOME").unwrap_or_default())
623}
624
625/// `borg review-fix [--home DIR] [--since YYYY-MM-DD] [--json] [--apply [--only ID,…]]`; the exit code.
626pub fn main(args: &[&str], ask: &dyn Fn(&str) -> Option<Value>) -> i32 {
627    let mut home = None;
628    let mut since = None;
629    let mut json = false;
630    let mut apply_it = false;
631    let mut only: Option<Vec<String>> = None;
632    let mut it = args.iter();
633    while let Some(a) = it.next() {
634        match *a {
635            "--json" => json = true,
636            "--apply" => apply_it = true,
637            "--only" => only = it.next().map(|v| v.split(',').map(str::to_string).collect()),
638            "--home" => home = it.next().map(PathBuf::from),
639            "--since" => match it.next().map(|d| borg_trace::parse_ts(&format!("{d}T00:00:00Z"))) {
640                Some(ms) if ms > 0 => since = Some(ms),
641                _ => {
642                    eprintln!("borg review-fix: --since wants YYYY-MM-DD\n{USAGE}");
643                    return 2;
644                }
645            },
646            _ => {
647                eprintln!("{USAGE}");
648                return 2;
649            }
650        }
651    }
652    let home = home.unwrap_or_else(home_dir);
653    // A broken config routes everything off, exactly as in the hook.
654    let cfg = match collective::load() {
655        Ok(c) => c,
656        Err(e) => {
657            eprintln!("borg review-fix: invalid collectives config ({e}): every repo is off");
658            return 1;
659        }
660    };
661    let report = run(&home, since, cfg.as_ref(), &learn::home());
662    if apply_it {
663        let (recorded, new, merged) = apply(&report, only.as_deref(), cfg.as_ref(), &learn::home(), ask);
664        println!("{recorded} review findings recorded as episodes; {new} new lessons, {merged} merged");
665        return 0;
666    }
667    if json {
668        println!("{}", serde_json::to_string(&report).unwrap_or_default());
669    } else {
670        print!("{}", render(&report));
671    }
672    0
673}
674
675#[cfg(test)]
676mod tests {
677    use super::*;
678    use serde_json::json;
679
680    /// The Claude Code workflow runs under `projects`, as `borg review-fix` reads them.
681    fn claude_runs(projects: &Path, session: Option<&str>) -> Vec<Run> {
682        prepared(harness::claude::workflow_runs(projects, session, None))
683    }
684
685    fn found(projects: &Path) -> Vec<Finding> {
686        scan(&claude_runs(projects, None)).0
687    }
688
689    fn temp(name: &str) -> PathBuf {
690        let d = std::env::temp_dir().join(format!("borg-review-fix-{name}-{}", std::process::id()));
691        let _ = std::fs::remove_dir_all(&d);
692        d
693    }
694
695    /// A transcript: the prompt, then tool calls (name, input), then the structured output, if any.
696    fn transcript(prompt: &str, tools: &[(&str, Value)], output: Option<Value>) -> String {
697        let mut lines = vec![json!({"type": "user", "cwd": "/w/app", "message": {"role": "user", "content": prompt}})];
698        for (name, input) in tools {
699            lines.push(json!({"type": "assistant", "message": {"content": [{"type": "tool_use", "name": name, "input": input}]}}));
700        }
701        if let Some(o) = output {
702            lines.push(json!({"type": "assistant", "message": {"content": [{"type": "tool_use", "name": "StructuredOutput", "input": o}]}}));
703        }
704        lines.iter().map(Value::to_string).collect::<Vec<_>>().join("\n")
705    }
706
707    /// One run in `projects/<slug>/<session>` with agents (id, label, phase, started, transcript).
708    fn write_run(projects: &Path, session: &str, run: &str, agents: &[(&str, &str, &str, i64, String)]) {
709        let dir = projects.join("-w-app").join(session);
710        let ad = dir.join("subagents/workflows").join(run);
711        std::fs::create_dir_all(&ad).unwrap();
712        std::fs::create_dir_all(dir.join("workflows")).unwrap();
713        let progress: Vec<Value> = agents
714            .iter()
715            .map(|(id, label, phase, at, t)| {
716                std::fs::write(ad.join(format!("agent-{id}.jsonl")), t).unwrap();
717                json!({"type": "workflow_agent", "label": label, "phaseTitle": phase, "agentId": id, "startedAt": at.to_string()})
718            })
719            .collect();
720        let r = json!({"runId": run, "workflowName": "build", "startTime": agents[0].3, "workflowProgress": progress});
721        std::fs::write(dir.join("workflows").join(format!("{run}.json")), r.to_string()).unwrap();
722    }
723
724    const TITLE: &str = "Pre-aborted signal returns before the error listener is attached";
725
726    fn review() -> String {
727        transcript("review the diff", &[], Some(json!({"findings": [{"file": "adapters/omp/borg.ts", "line": 153, "severity": "Medium", "title": TITLE}]})))
728    }
729
730    #[test]
731    fn a_confirmed_finding_the_fix_step_touches_is_strong() {
732        let p = temp("strong");
733        let verify = transcript(&format!("Verify: {TITLE}"), &[], Some(json!({"real": true, "reason": "probe crashed"})));
734        let fix = transcript("apply the findings", &[("Bash", json!({"command": "sed -i 's/a/b/' adapters/omp/borg.ts"}))], None);
735        write_run(
736            &p,
737            "s1",
738            "wf_1",
739            &[
740                ("a1", "review:concurrency", "Review", 10, review()),
741                ("a2", "verify:concurrency:Pre-aborted signal ret", "Verify", 20, verify),
742                ("a3", "fix", "Fix", 30, fix),
743            ],
744        );
745        let f = found(&p);
746        assert_eq!(f.len(), 1);
747        assert_eq!((f[0].verdict, f[0].fixed, f[0].severity.as_str()), (Verdict::Confirmed, Fixed::Yes, "medium"));
748        assert!(f[0].strong());
749        assert_eq!(f[0].cwd, "/w/app");
750        let _ = std::fs::remove_dir_all(p);
751    }
752
753    #[test]
754    fn a_rejected_or_unfixed_finding_is_not_strong() {
755        let p = temp("weak");
756        let verify = transcript("check it", &[], Some(json!({"verdicts": [{"title": TITLE, "verdict": "refuted"}]})));
757        let fix = transcript("apply the findings", &[("Edit", json!({"file_path": "src/other.rs"}))], None);
758        write_run(&p, "s1", "wf_1", &[("a1", "review", "Review", 10, review()), ("a2", "verify", "Verify", 20, verify), ("a3", "fix", "Fix", 30, fix)]);
759        let f = found(&p);
760        assert_eq!((f[0].verdict, f[0].fixed), (Verdict::Rejected, Fixed::No));
761        assert!(!f[0].strong());
762        let _ = std::fs::remove_dir_all(p);
763    }
764
765    #[test]
766    fn a_review_without_a_later_fix_step_or_a_file_is_counted_apart() {
767        let p = temp("nofix");
768        write_run(&p, "s1", "wf_1", &[("a1", "review", "Review", 10, review())]);
769        let unlocated = transcript("review", &[], Some(json!({"issues": [{"severity": "high", "problem": "the budget is spent on noise"}]})));
770        write_run(&p, "s2", "wf_2", &[("b1", "review:queen", "Review", 10, unlocated), ("b2", "fix", "Fix", 20, transcript("fix", &[], None))]);
771        let f = found(&p);
772        assert_eq!(f.iter().map(|x| x.fixed).collect::<Vec<_>>(), [Fixed::NoFixStep, Fixed::Unlocated]);
773        assert_eq!(f[1].verdict, Verdict::Unchecked);
774        let _ = std::fs::remove_dir_all(p);
775    }
776
777    #[test]
778    fn string_findings_and_gate_results_are_not_findings() {
779        let out = json!({"findings": ["X1 the voice speaks the commentary 38/38 times"], "results": [{"command": "npm run check", "passed": true}]});
780        assert!(findings_of(Some(&out)).is_empty());
781        assert!(verdicts_of(Some(&out)).is_empty());
782        // A verify output's items carry a verdict, so they are verdicts, not new findings.
783        let v = json!({"findings": [{"title": TITLE, "severity": "low", "real": false}]});
784        assert!(findings_of(Some(&v)).is_empty());
785    }
786
787    #[test]
788    fn verdicts_link_by_id_within_the_verifiers_dimension() {
789        let p = temp("ids");
790        let f =
791            |id: &str, file: &str| json!({"id": id, "file": file, "severity": "minor", "claim": format!("{file} drops the queued lessons on a late answer")});
792        let rust = transcript("review", &[], Some(json!({"findings": [f("F1", "src/a.rs")]})));
793        let docs = transcript("review", &[], Some(json!({"findings": [f("F1", "docs/b.md")]})));
794        let verify = transcript("verify", &[], Some(json!({"verdicts": [{"id": "F1", "refuted": true}]})));
795        let keep = transcript("verify", &[], Some(json!({"results": [{"id": "F1", "keep": true}]})));
796        write_run(
797            &p,
798            "s1",
799            "wf_1",
800            &[
801                ("a1", "review:rust", "Review", 10, rust),
802                ("a2", "review:docs", "Review", 11, docs),
803                ("a3", "verify:rust", "Verify", 20, verify),
804                ("a4", "verify:docs", "Verify", 21, keep),
805            ],
806        );
807        let got = found(&p);
808        assert_eq!(
809            got.iter().map(|x| (x.agent.as_str(), x.verdict)).collect::<Vec<_>>(),
810            [("review:rust", Verdict::Rejected), ("review:docs", Verdict::Confirmed)]
811        );
812        assert_eq!(verdict_of(&json!({"verdict": "PLAUSIBLE"})), Some(Verdict::Confirmed));
813        assert_eq!(verdict_of(&json!({"verdict": "needs more info"})), None);
814        let _ = std::fs::remove_dir_all(p);
815    }
816
817    #[test]
818    fn a_strong_finding_becomes_a_review_regret_episode() {
819        let p = temp("episode");
820        let build = transcript("Port the voice lane to gpt-live.", &[], None);
821        let fix = transcript("apply", &[("Bash", json!({"command": "sed -i x adapters/omp/borg.ts"}))], None);
822        write_run(&p, "s1", "wf_1", &[("a0", "build", "Build", 5, build), ("a1", "review", "Review", 10, review()), ("a2", "fix", "Fix", 20, fix)]);
823        let f = found(&p);
824        assert_eq!((f[0].id.as_str(), f[0].task.as_str()), ("wf_1#0", "Port the voice lane to gpt-live."));
825        let e = f[0].episode("dev@x");
826        assert_eq!((e.id.as_str(), e.kind.as_str(), e.session.as_str(), e.engineer.as_str()), ("Vwf_1#0", regret::KIND, "s1", "dev@x"));
827        let r = e.regret.unwrap();
828        assert!(regret::is_review(&r) && r.signals == ["review", "unchecked"]);
829        assert_eq!(r.decision_ms, e.ms, "decided before the finding: asks up to it are blamed");
830        assert!(r.diff.starts_with("adapters/omp/borg.ts: ") && r.fix_subject == TITLE, "{r:?}");
831        let _ = std::fs::remove_dir_all(p);
832    }
833
834    #[test]
835    fn paths_come_out_of_location_text() {
836        assert_eq!(path_in("src/main/java/Tq.java:72-79 (resets at L74)").as_deref(), Some("src/main/java/Tq.java"));
837        assert_eq!(path_in("sliceA-spec.md §G.3/§G.8 (ii)").as_deref(), Some("sliceA-spec.md"));
838        assert_eq!(path_in("the whole design"), None);
839        assert!(names("cat > crates/borg/src/live.rs <<EOF", "/home/x/crates/borg/src/live.rs") || names("x src/live.rs", "crates/borg/src/live.rs"));
840        assert!(!names("edits main.rs", "src/live.rs"));
841    }
842
843    #[test]
844    fn the_control_pairs_a_finding_with_another_sessions_fix_step() {
845        let p = temp("control");
846        let fix = |file: &str| transcript("fix", &[("Bash", json!({"command": format!("vim {file}")}))], None);
847        write_run(&p, "s1", "wf_1", &[("a1", "review", "Review", 10, review()), ("a2", "fix", "Fix", 20, fix("adapters/omp/borg.ts"))]);
848        write_run(&p, "s2", "wf_2", &[("b1", "build", "Build", 30, transcript("build", &[], None)), ("b2", "fix", "Fix", 40, fix("src/other.rs"))]);
849        let r = report(&claude_runs(&p, None), None, &p.join("store"));
850        assert_eq!((r.control.located, r.control.own_named, r.control.other_named), (1, 1, 0));
851        assert_eq!(r.collectives.values().map(|t| (t.findings, t.strong, t.runs)).collect::<Vec<_>>(), [(1, 1, 1)]);
852        let _ = std::fs::remove_dir_all(p);
853    }
854
855    #[test]
856    fn session_end_records_only_that_sessions_strong_findings_once() {
857        let p = temp("session");
858        let fix = transcript("apply the findings", &[("Bash", json!({"command": "sed -i 's/a/b/' adapters/omp/borg.ts"}))], None);
859        for (s, run) in [("s1", "wf_1"), ("s2", "wf_2")] {
860            write_run(&p, s, run, &[("a1", "review:concurrency", "Review", 10, review()), ("a3", "fix", "Fix", 30, fix.clone())]);
861        }
862        let store = p.join("store");
863        let session = |s: &str| record(scan(&claude_runs(&p, Some(s))).0.iter(), None, &store);
864        let (n, homes) = session("s1");
865        assert_eq!((n, homes), (1, vec![store.clone()]));
866        let episodes = std::fs::read_to_string(store.join("episodes.jsonl")).unwrap();
867        assert!(episodes.contains("Vwf_1#") && !episodes.contains("wf_2"), "{episodes}");
868        // Each recorded finding is counted for the page (stats.rs), with its severity; a skipped one is not.
869        let evs = crate::stats::events(&store);
870        assert_eq!(
871            evs.iter().filter(|e| e.kind == crate::stats::REVIEW_FINDING).map(|e| (e.session.as_str(), e.why.as_str())).collect::<Vec<_>>(),
872            [("s1", "medium")]
873        );
874        // Known episodes are skipped; a session id is never a path.
875        assert_eq!(session("s1").0, 0);
876        assert_eq!(record_session(&p, "../-w-app/s2", None, &store).0, 0);
877        assert_eq!(record_session(&p, "", None, &store).0, 0);
878        let _ = std::fs::remove_dir_all(p);
879    }
880
881    fn fixture(p: &str) -> PathBuf {
882        Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/review_fix").join(p)
883    }
884
885    /// (title, severity, fixed, strong) of each finding.
886    fn rows(f: &[Finding]) -> Vec<(&str, &str, Fixed, bool)> {
887        f.iter().map(|x| (x.title.as_str(), x.severity.as_str(), x.fixed, x.strong())).collect()
888    }
889
890    #[test]
891    fn a_claude_workflow_run_gives_its_strong_finding() {
892        let f = found(&fixture("claude/projects"));
893        assert_eq!(rows(&f), [("The budget place is reset by an arc instead of drained", "high", Fixed::Yes, true)]);
894        assert_eq!((f[0].session.as_str(), f[0].file.as_deref(), f[0].workflow.as_str()), ("s1", Some("src/net.rs"), "build"));
895    }
896
897    #[test]
898    fn omp_findings_yielded_one_by_one_and_a_fix_task_give_one_strong_finding() {
899        let runs = prepared(harness::omp::session_runs(&fixture("omp/sessions"), Some("01a0a00d-0000-7000-8000-000000000001"), None));
900        let f = scan(&runs).0;
901        // The array the schema rejected is not counted: only the accepted yields, one finding each.
902        assert_eq!(
903            rows(&f),
904            [
905                ("Facet mapping drops the unit of a numeric range", "p1", Fixed::Yes, true),
906                ("Empty facet list renders a stray separator", "p3", Fixed::No, false)
907            ]
908        );
909        let x = &f[0];
910        assert_eq!(
911            (x.agent.as_str(), x.session.as_str(), x.cwd.as_str(), x.workflow.as_str()),
912            ("MappingReview", "01a0a00d-0000-7000-8000-000000000001", "/w/app", "Map the facets")
913        );
914        assert_eq!(x.file.as_deref(), Some("/w/app/src/facets/mapping.ts"));
915        assert!(x.detail.starts_with("`mapRange` in src/facets/mapping.ts") && x.task == "Implement the facet mapping in src/facets/mapping.ts.", "{x:?}");
916    }
917
918    #[test]
919    fn omp_findings_in_one_yielded_object_are_all_read() {
920        let runs = prepared(harness::omp::session_runs(&fixture("omp/sessions"), Some("01a0a00d-0000-7000-8000-000000000002"), None));
921        let f = scan(&runs).0;
922        assert_eq!(
923            rows(&f),
924            [
925                ("Retry loop never backs off", "p0", Fixed::NoFixStep, false),
926                ("Timeout is read in seconds and used as milliseconds", "p2", Fixed::NoFixStep, false)
927            ]
928        );
929        // Every session, oldest first.
930        let all = harness::omp::session_runs(&fixture("omp/sessions"), None, None);
931        assert_eq!(all.iter().map(|r| r.agents.len()).collect::<Vec<_>>(), [3, 2]);
932    }
933
934    /// opencode exports by id from the fixture's v1 and v2 directories; the query lists both roots.
935    fn opencode_runs(session: Option<&str>) -> Vec<Run> {
936        let export = |id: &str, _: Option<&Path>| {
937            ["v1", "v2"]
938                .iter()
939                .find_map(|v| std::fs::read(fixture("opencode").join(v).join(format!("{id}.json"))).ok())
940                .and_then(|b| serde_json::from_slice(&b).ok())
941        };
942        let query = |_: &str| Some(vec![json!({"id": "ses_1root0000000000000000000"}), json!({"id": "ses_2root0000000000000000000"})]);
943        prepared(harness::opencode::session_runs(session, None, &export, &query))
944    }
945
946    #[test]
947    fn opencode_findings_come_from_a_reviewers_json_text_and_a_fix_child_fixes_them() {
948        let f = scan(&opencode_runs(Some("ses_1root0000000000000000000"))).0;
949        assert_eq!(rows(&f), [("Cache key ignores the tenant id", "high", Fixed::Yes, true)]);
950        assert_eq!((f[0].agent.as_str(), f[0].session.as_str(), f[0].cwd.as_str()), ("review cache", "ses_1root0000000000000000000", "/w/app"));
951        // 2.x: a bare object with `issues`; the repair child's export is missing, so its final text
952        // as the parent saw it names the file.
953        let all = scan(&opencode_runs(None)).0;
954        assert_eq!(rows(&all)[1], ("Queue bound is checked after the push", "medium", Fixed::Yes, true));
955        // A session id that is no opencode one asks nothing.
956        assert!(opencode_runs(Some("01a0a00d-0000-7000-8000-000000000001")).is_empty());
957    }
958
959    #[test]
960    fn harnesses_without_sub_agent_transcripts_give_no_runs() {
961        let home = fixture("omp");
962        let none: Vec<&str> = harness::ALL.iter().filter(|h| h.sub_runs(&home, None, None).is_empty()).map(|h| h.name()).collect();
963        assert!(none.contains(&"sim"), "{none:?}");
964    }
965
966    #[test]
967    fn severity_comes_from_a_priority_and_text_from_a_body() {
968        let f = |v: Value| findings_of(Some(&json!({"findings": [v]}))).into_iter().map(|r| (r.title, r.severity, r.detail, r.file)).next();
969        assert_eq!(f(json!({"body": "src/a.rs drops it", "priority": "P2"})), Some(("src/a.rs drops it".into(), "p2".into(), String::new(), None)));
970        assert_eq!(
971            f(json!({"title": "t", "body": "b", "priority": 0, "file_path": "src/a.rs"})).map(|x| (x.1, x.2, x.3)),
972            Some(("p0".into(), "b".into(), Some("src/a.rs".into())))
973        );
974        assert_eq!(f(json!({"title": "no severity"})), None);
975        let text = |w: &str| output_of(&SubAgent { final_text: w.into(), ..Default::default() });
976        assert_eq!(text("{\"command\": \"x\"}\nDone: {\"findings\": []} inline"), None, "only an object that starts a line");
977        assert!(text("Report\n```json\n{\"issues\": [{\"problem\": \"p\"}]}\n```").is_some());
978    }
979
980    #[test]
981    fn only_a_fix_step_is_a_fix_step_whatever_its_label_mentions() {
982        let a = |label: &str, phase: &str, kind: &str, output: Option<Value>| SubAgent {
983            label: label.into(),
984            phase: phase.into(),
985            kind: kind.into(),
986            output,
987            ..Default::default()
988        };
989        let fix = |label: &str, phase: &str| is_fix(&a(label, phase, "", None));
990        // Labels of real workflow runs: fix in a later segment is what is checked, built or reviewed.
991        assert!(!fix("verify:fix-correctness", "Verify"));
992        assert!(!fix("review:harness-fix#0", "Review"));
993        assert!(!fix("impl:harness-fix", "Implement"));
994        assert!(!fix("verify:fix-correctness", ""), "the label's first segment decides");
995        assert!(fix("fix", "Fix") && fix("fix:rust", "") && fix("fix#1", "") && fix("anything", "Repair"));
996        assert!(fix("MappingValidityFix", "") && fix("DeriveFacetsRepairs", "") && !fix("Prefixed", "") && !fix("MappingReviewFix", ""));
997        assert!(fix("fix", "") && !fix("fix", "Review"), "a review phase is never a fix step");
998        assert!(is_fix(&a("ses", "", "fixer", None)), "an omp kind");
999        // An output with a findings list, even an empty one, or a verdict is a reviewer's or a verifier's.
1000        assert!(!is_fix(&a("fix", "Fix", "", Some(json!({"findings": []})))));
1001        assert!(!is_fix(&a("fix", "Fix", "", Some(json!({"real": true})))));
1002        // The implementer is the builder, not a fix step.
1003        let p = temp("labels");
1004        let build = transcript("Build the harness fix.", &[], None);
1005        let fix_it = transcript("apply", &[("Bash", json!({"command": "sed -i x adapters/omp/borg.ts"}))], None);
1006        let verify = transcript("verify", &[("Bash", json!({"command": "cat adapters/omp/borg.ts"}))], Some(json!({"real": true})));
1007        write_run(
1008            &p,
1009            "s1",
1010            "wf_1",
1011            &[
1012                ("a0", "impl:harness-fix", "Implement", 5, build),
1013                ("a1", "review:harness-fix#0", "Review", 10, review()),
1014                ("a2", "verify:fix-correctness", "Verify", 20, verify),
1015            ],
1016        );
1017        let f = found(&p);
1018        assert_eq!((f[0].fixed, f[0].task.as_str()), (Fixed::NoFixStep, "Build the harness fix."));
1019        write_run(&p, "s2", "wf_2", &[("b1", "review:harness-fix#0", "Review", 10, review()), ("b2", "fix:rust", "", 30, fix_it)]);
1020        assert_eq!(found(&p)[1].fixed, Fixed::Yes);
1021        let _ = std::fs::remove_dir_all(p);
1022    }
1023
1024    #[test]
1025    fn findings_come_only_from_the_final_text_never_from_a_tool_input() {
1026        let echo = json!({"findings": [{"file": "src/a.rs", "severity": "high", "title": "the findings this fix step works through"}]});
1027        // A fix step that passes the findings to a tool stays a fix step, whatever its text says after.
1028        for last in ["", "Fixed src/a.rs."] {
1029            let a = SubAgent {
1030                label: "fix".into(),
1031                work: format!("{{\"command\":\"cargo test\"}}\n{echo}\n{last}\n"),
1032                final_text: last.into(),
1033                ..Default::default()
1034            };
1035            let a = SubAgent { output: output_of(&a), ..a };
1036            assert!(a.output.is_none() && is_fix(&a), "{last:?}");
1037        }
1038        // A reviewer's answer is read, fenced or compact (keys in any order).
1039        let fenced = format!("Done.\n```json\n{}\n```\n", serde_json::to_string_pretty(&echo).unwrap());
1040        assert_eq!(output_of(&SubAgent { final_text: fenced, ..Default::default() }), Some(echo.clone()));
1041        assert_eq!(output_of(&SubAgent { final_text: echo.to_string(), ..Default::default() }), Some(echo));
1042    }
1043
1044    #[test]
1045    fn a_builder_in_another_repo_gives_no_task_text() {
1046        let p = temp("builder-repo");
1047        let fix = transcript("apply the findings", &[("Bash", json!({"command": "sed -i 's/a/b/' adapters/omp/borg.ts"}))], None);
1048        let build = transcript("build the secret feature", &[], None).replace("/w/app", "/w/private");
1049        write_run(&p, "s1", "wf_1", &[("a0", "build", "Build", 5, build), ("a1", "review:concurrency", "Review", 10, review()), ("a3", "fix", "Fix", 30, fix)]);
1050        let f = found(&p);
1051        assert_eq!(f.len(), 1);
1052        assert!(f[0].task.is_empty() && f[0].built.is_empty(), "{:?}", f[0].task);
1053        let _ = std::fs::remove_dir_all(p);
1054    }
1055}