diff --git a/README.md b/README.md index 0c0079daf..8373ae56d 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,7 @@ One statically linked Linux binary covers the first three, so it doesn't depend The user account that runs thirdshift needs: - **`claude`** (Claude Code), logged in. +- **`codex`** (Codex CLI), logged in, only for `harness codex`. - **`gh`** (GitHub CLI), logged in. - **`git`** with a global `user.name` and `user.email`, and credentials that can push to the repository (`gh auth setup-git` makes git use `gh`'s login). The agents commit as this identity; without it, an agent may borrow the author of the last commit. @@ -108,7 +109,7 @@ Sessions run headless in Claude Code's auto mode (`claude -p --permission-mode a ### What sessions leave behind -Cleanup removes only thirdshift's own worktree, local Issue branch and the temporary directory it wrote the Factory skills out to. The `.git/info/exclude` entry that keeps the linked skills out of git, `/.claude/skills/thirdshift-*`, is added once and left in place, as every worktree of the repository shares the file. Anything else an agent does as your user stays. For example, one Run downloaded a JDK to `~/.local/jdk/` because the server had no Java, installed Playwright in `/tmp/pw`, and left a pull request body draft in `/tmp/`. This is by design, but worth knowing: +Cleanup removes only thirdshift's own worktree, local Issue branch and the temporary directory it wrote the Factory skills out to. The `.git/info/exclude` entry that keeps the linked skills out of git, `/.claude/skills/thirdshift-*`, or `/.agents/skills/thirdshift-*` on Codex, is added once and left in place, as every worktree of the repository shares the file. Anything else an agent does as your user stays. For example, one Run downloaded a JDK to `~/.local/jdk/` because the server had no Java, installed Playwright in `/tmp/pw`, and left a pull request body draft in `/tmp/`. This is by design, but worth knowing: - **Install the target repository's toolchains up front** (language runtimes, package managers, test tools), so the agents don't download their own on every Run. - **For UI work, install the headless browser dependencies**, e.g. `libgbm1` on Ubuntu server, which headless Chromium needs. Without it, agents can't take screenshots to check a UI change and fall back to reading the code. @@ -257,7 +258,7 @@ effort = "" # how hard that Model reasons; default blank, for Codex's own Every key holds its real value, so a Run reading it does exactly what it does with no file. `email.to` has no default, so `setup` suggests one: the public email of your GitHub profile (from `gh api user`), else your global git `user.email`, unless that is a `@users.noreply.github.com` address, which can't receive mail. With neither, `email.to` is the only line written commented out, as above. `setup` never asks `gh` for more scopes, so a private GitHub email is not read, and a Run never looks the suggestion up: `--email` with no address and no `email.to` still stops the Run. From a terminal (stdin and stderr both terminals), `setup` first asks, on stderr: -1. The Harness for every Run's sessions (`harness.default`), listing both, each marked where sessions can't run on it here: `claude (not installed)` when `claude` isn't on `PATH`, and `codex (not supported yet)` until Codex sessions are. A marked Harness, or a name that isn't one, is refused and asked again. The default is the one sessions can run on, `claude` when both can. Then the Model (`harness..model`) and the Effort (`harness..effort`) for the Harness chosen only, each defaulting to the User config's, where Enter on none keeps the Harness's own default and `-` clears one. A named Model gets the test call a Run makes (see [Harness, Model and Effort](#harness-model-and-effort)); if Claude refuses it, Setup shows what Claude said and asks the Model and Effort again. The other Harness's section is left as it is. With no Harness sessions can run on here, Setup says so, asks none of this, and leaves the harness settings as they are. +1. The Harness for every Run's sessions (`harness.default`), listing both, each marked where sessions can't run on it here: `claude (not installed)` when `claude` isn't on `PATH`, and `codex (not supported yet)` until Setup offers Codex. A marked Harness, or a name that isn't one, is refused and asked again. The default is the one sessions can run on, `claude` when both can. Then the Model (`harness..model`) and the Effort (`harness..effort`) for the Harness chosen only, each defaulting to the User config's, where Enter on none keeps the Harness's own default and `-` clears one. A named Model gets the test call a Run makes (see [Harness, Model and Effort](#harness-model-and-effort)); if Claude refuses it, Setup shows what Claude said and asks the Model and Effort again. The other Harness's section is left as it is. With no Harness sessions can run on here, Setup says so, asks none of this, and leaves the harness settings as they are. 2. Every Run a Merge run? (`merge.always`) 3. With that on, every Run may start a Base fix when the Base branch's CI is red? (`base.fix`). With it off, this is not asked, and `base.fix` is written at its default, `false`. 4. Every Run first fast-forwards your checkout of the Base branch? (`launch.pull`) @@ -294,9 +295,11 @@ Every session of a command, its implement session, each Repair and Resume, a Spe thirdshift https://github.com///issues/ model claude-opus-5-5 effort high ``` -For each of the three, the command wins, then the [User config](#user-config), then the default. The Harness's default is `claude`; the Model's and the Effort's are nothing, which leaves them to the Harness, so with no flag and no `[harness]` section sessions run exactly as `claude -p` would. A Model and Effort set in the User config come from the chosen Harness's own section, so `harness claude` over a `codex` default takes `[harness.claude]`, while `model` and `effort` on the command apply to whichever Harness is chosen. Names are passed to Claude Code as given: `--model` and `--effort` when set. Only `claude` works for now: `harness codex` fails with `not supported yet`. +For each of the three, the command wins, then the [User config](#user-config), then the default. The Harness's default is `claude`; the Model's and the Effort's are nothing, which leaves them to the Harness, so with no flag and no `[harness]` section sessions run exactly as `claude -p` would. A Model and Effort set in the User config come from the chosen Harness's own section, so `harness claude` over a `codex` default takes `[harness.claude]`, while `model` and `effort` on the command apply to whichever Harness is chosen. Names are passed to Claude Code as given: `--model` and `--effort` when set. Codex takes them as it names them, which the check below settles: `-m ` and `-c model_reasoning_effort=""` when set. -Before any work, where an Origin match failure would stop it, and so before the Claim, the worktree and any Command log, a command checks what it chose: the Harness's CLI must be on `PATH`, else it fails naming the CLI and the setting that chose it; and a named Model gets a minimal test call, `claude -p --model ` with the Effort, if any, which must succeed, else the command fails with what Claude said. No test call is made when no Model is named. A Pickup run or an Architect run makes these checks only once it has decided not to skip, and a Spec run makes them once for all its Tickets. +With `harness codex`, every session runs `codex exec --json --dangerously-bypass-approvals-and-sandbox` in the Run's worktree, with stdin set to null, and with `-c project_doc_fallback_filenames=["CLAUDE.md"]`, so Codex reads `CLAUDE.md` where a repository has no `AGENTS.md`. Codex's sandbox is off: on a stock Ubuntu machine it can't start, and commits and pushes from a worktree write outside it, so the worktree is the only boundary, as with Claude's auto mode ([ADR-0012](docs/adr/0012-factory-skills-linked-into-the-worktree-codex-unsandboxed.md)). The Factory skills are linked into the worktree's `.agents/skills/`, kept out of git with `/.agents/skills/thirdshift-*` in `.git/info/exclude`, and each prompt's first line loads its skill as `$thirdshift-`. A Resume runs `codex exec … resume `, with every flag given again. Progress lines come from Codex's `item.*` events, the session-ended line gives its token totals, as Codex reports no turns or cost, and the final message is the last `agent_message`. A session whose turn fails (`turn.failed`), or that exits non-zero, fails the Run with Codex's error in the cause; an `error` event Codex retries does not. Each Codex session adds a `trust_level = "trusted"` entry for the repository to `~/.codex/config.toml`, and your `~/.claude/` instructions and hooks don't reach it. + +Before any work, where an Origin match failure would stop it, and so before the Claim, the worktree and any Command log, a command checks what it chose: the Harness's CLI must be on `PATH`, else it fails naming the CLI and the setting that chose it; and on Claude a named Model gets a minimal test call, `claude -p --model ` with the Effort, if any, which must succeed, else the command fails with what Claude said. No test call is made when no Model is named. On Codex a named Model and Effort are checked against `codex debug models`, which costs no tokens: each matches regardless of case, a Model by its slug or its display name, and becomes the name Codex takes, so `model GPT-6.1-Sol effort Max` reaches Codex as `gpt-6.1-sol` and `max`. An unknown Model, or an Effort that Model doesn't support (with no Model, one no Model supports), fails naming the valid choices. What the command records is what the check settled on. A Pickup run or an Architect run makes these checks only once it has decided not to skip, and a Spec run makes them once for all its Tickets. The Command log's opening lines (`sessions run on claude · claude-opus-5-5 · high`), the Activity log's start line, the pull request's body and the Run notification each name the Harness, Model and Effort, `default model` or `default effort` where the Harness's own is used. In the pull request's body, the Spec PR's included, thirdshift writes the line itself once the opening session has ended, such as `Built with claude · claude-opus-5-5 · high`, replacing any it wrote there before. diff --git a/prompts/README.md b/prompts/README.md index b1012d114..6b0f38373 100644 --- a/prompts/README.md +++ b/prompts/README.md @@ -1,4 +1,4 @@ - + # Session prompts @@ -16,6 +16,8 @@ A [Resume](resume.md) continues the session that ended: claude -p --permission-mode auto --output-format stream-json --verbose --resume ``` +With `harness codex`, each session runs `codex exec --json --dangerously-bypass-approvals-and-sandbox -m -c model_reasoning_effort="" -c project_doc_fallback_filenames=["CLAUDE.md"] ` instead, without `-m ` or the Effort's `-c` where they are not set, and with `resume ` before the prompt for a Resume. The Factory skills are linked into `.agents/skills/`, and a prompt's first line loads its skill as `$thirdshift-`. + | Prompt | When it is sent | File | | --- | --- | --- | | Fresh | Starts the implement session, which goes on to review and open the pull request, when the Run starts a new Issue branch. | [fresh.md](fresh.md) | diff --git a/site/prompts/index.html b/site/prompts/index.html index 7607c4f82..31c82a603 100644 --- a/site/prompts/index.html +++ b/site/prompts/index.html @@ -38,6 +38,7 @@

Prompts and skills

claude -p --permission-mode auto --output-format stream-json --verbose <prompt>

A Resume continues the session that ended:

claude -p --permission-mode auto --output-format stream-json --verbose --resume <session id> <prompt>
+

With harness codex, each session runs codex exec --json --dangerously-bypass-approvals-and-sandbox -m <model> -c model_reasoning_effort="<effort>" -c project_doc_fallback_filenames=["CLAUDE.md"] <prompt> instead, without -m <model> or the Effort's -c where they are not set, and with resume <session id> before the prompt for a Resume. The Factory skills are linked into .agents/skills/, and a prompt's first line loads its skill as $thirdshift-<skill>.

diff --git a/src/architect.rs b/src/architect.rs index 14be9be77..aea25565c 100644 --- a/src/architect.rs +++ b/src/architect.rs @@ -220,7 +220,7 @@ pub fn run( focus: Option<&str>, base: Option<&str>, launch_pull: bool, - harness: &Choice, + harness: &mut Choice, ) -> Result { let started = Utc::now(); let Launch { @@ -242,6 +242,7 @@ pub fn run( return Ok(skip(&repo, skipped)); } harness.check()?; + let harness = &*harness; logs::started(Work::ArchitectRun(&repo), harness); if launch_pull { directory.pull(&base); diff --git a/src/base_fix.rs b/src/base_fix.rs index 1ab90a3f1..d3716d53b 100644 --- a/src/base_fix.rs +++ b/src/base_fix.rs @@ -157,6 +157,12 @@ impl BaseFix { } } + /// Have any Base fix it starts run its sessions on `harness`: the Run's, + /// once the Run has checked it. + pub fn runs_on(&mut self, harness: Choice) { + self.harness = harness; + } + /// Whether the Run compares its red checks with the Base branch's, so /// that some may be Inherited failures. pub fn sees_inherited_failures(&self) -> bool { diff --git a/src/harness.rs b/src/harness.rs index cedd7c603..42cc3a500 100644 --- a/src/harness.rs +++ b/src/harness.rs @@ -6,9 +6,10 @@ use std::fmt; use std::io::Write; use std::path::Path; -use std::process::{Command, Stdio}; +use std::process::{Command, Output, Stdio}; use anyhow::{Context, Result, bail}; +use serde_json::Value; use crate::progress; @@ -41,8 +42,8 @@ impl Harness { /// Every Harness, in the order Setup lists them. pub const ALL: [Harness; 2] = [Harness::Claude, Harness::Codex]; - /// Why sessions can't run on it on this machine, if they can't: Codex is - /// not supported yet, and otherwise its CLI must be on `PATH`. + /// Why Setup can't offer it on this machine, if it can't: Setup doesn't + /// offer Codex yet, and otherwise its CLI must be on `PATH`. pub fn unavailable(self) -> Option { if self == Harness::Codex { Some(Unavailable::NotSupportedYet) @@ -57,7 +58,7 @@ impl Harness { /// Why sessions can't run on a Harness. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Unavailable { - /// thirdshift can't run its sessions yet. + /// Setup doesn't offer it yet. NotSupportedYet, /// Its CLI isn't on `PATH`. NotInstalled, @@ -198,31 +199,46 @@ impl Choice { args } - /// Check, before any work, that sessions can run on this: Codex is not - /// supported yet; the Harness's CLI must be on `PATH`; and a Model named - /// for Claude must take a minimal test call, with the Effort, if any. - /// Each failure says what to change. - pub fn check(&self) -> Result<()> { + /// The arguments that run a Codex session on its Model and Effort, where + /// they are set. + pub fn codex_args(&self) -> Vec { + let mut args = Vec::new(); + if let Some(model) = &self.model { + args.extend(["-m".to_string(), model.clone()]); + } + if let Some(effort) = &self.effort { + args.extend([ + "-c".to_string(), + format!("model_reasoning_effort=\"{effort}\""), + ]); + } + args + } + + /// Check, before any work, that sessions can run on this, and settle the + /// Model and Effort on the names the Harness knows them by. The + /// Harness's CLI must be on `PATH`. A Model named for Claude must take a + /// minimal test call, with the Effort, if any. A Model or Effort named + /// for Codex must be in its catalog, which costs no tokens to read: each + /// is matched regardless of case, a Model by its slug or display name, + /// and becomes the name Codex takes. Each failure says what to change. + pub fn check(&mut self) -> Result<()> { let cli = self.harness.name(); - match self.harness.unavailable() { - Some(Unavailable::NotSupportedYet) => bail!( - "harness {cli}, chosen by {}, is not supported yet: Codex sessions come in a \ - later thirdshift; choose harness claude", - self.chosen_by - ), - Some(Unavailable::NotInstalled) => bail!( + if !on_path(cli) { + bail!( "{cli} is not on PATH, and the Harness {cli} is chosen by {}: install it, \ or choose another Harness", self.chosen_by - ), - None => {} + ); + } + match self.harness { + Harness::Claude => self.test_call(), + Harness::Codex => self.check_codex(), } - self.test_call() } - /// Check that the Harness's CLI takes a minimal test call on the Model, - /// if one is named, with the Effort, if any. A refusal says what the CLI - /// said. Only Claude reaches it while Codex is not supported. + /// Check that Claude takes a minimal test call on the Model, if one is + /// named, with the Effort, if any. A refusal says what Claude said. pub fn test_call(&self) -> Result<()> { let cli = self.harness.name(); let Some(model) = &self.model else { @@ -249,17 +265,165 @@ impl Choice { if output.status.success() { return Ok(()); } - let said = [&output.stderr, &output.stdout] - .map(|said| String::from_utf8_lossy(said).trim().to_string()) - .into_iter() - .filter(|said| !said.is_empty()) - .collect::>() - .join("\n"); let effort = match &self.effort { Some(effort) => format!(" with the Effort {effort}"), None => String::new(), }; - bail!("{cli} refused a test call on the Model {model}{effort}: {said}") + bail!( + "{cli} refused a test call on the Model {model}{effort}: {}", + said(&output) + ) + } + + /// Settle the Model and Effort, if either is named, on Codex's names for + /// them, from its catalog. + fn check_codex(&mut self) -> Result<()> { + if self.model.is_none() && self.effort.is_none() { + return Ok(()); + } + progress::step("checking the Model and Effort against codex debug models"); + let output = Command::new("codex") + .args(["debug", "models"]) + .stdin(Stdio::null()) + .output() + .context("could not run codex debug models to check the Model and Effort")?; + if !output.status.success() { + bail!( + "codex debug models failed, so the Model and Effort can't be checked: {}", + said(&output) + ); + } + let catalog = Catalog::parse(&String::from_utf8_lossy(&output.stdout))?; + let settled = catalog.settle(self.model.as_deref(), self.effort.as_deref())?; + self.model = settled.model; + self.effort = settled.effort; + Ok(()) + } +} + +/// What a CLI said on stderr and stdout, trimmed, each on its own lines. +fn said(output: &Output) -> String { + [&output.stderr, &output.stdout] + .map(|said| String::from_utf8_lossy(said).trim().to_string()) + .into_iter() + .filter(|said| !said.is_empty()) + .collect::>() + .join("\n") +} + +/// One Model in Codex's catalog. +#[derive(Debug, Clone, PartialEq, Eq)] +struct CatalogModel { + /// The name Codex takes. + slug: String, + display_name: Option, + /// The Efforts it supports. + efforts: Vec, + /// Whether the catalog lists it to choose from. + listed: bool, +} + +/// The Models Codex can run, as `codex debug models` prints them. +#[derive(Debug)] +struct Catalog { + models: Vec, +} + +impl Catalog { + /// The catalog `codex debug models` printed as `json`. + fn parse(json: &str) -> Result { + let parsed: Value = serde_json::from_str(json) + .context("codex debug models printed no JSON catalog of Models")?; + let models = parsed["models"] + .as_array() + .context("codex debug models printed no list of Models")? + .iter() + .filter_map(|model| { + Some(CatalogModel { + slug: model["slug"].as_str()?.to_string(), + display_name: model["display_name"].as_str().map(String::from), + efforts: model["supported_reasoning_levels"] + .as_array() + .into_iter() + .flatten() + .filter_map(|level| level["effort"].as_str().map(String::from)) + .collect(), + listed: model["visibility"] != "hide", + }) + }) + .collect(); + Ok(Catalog { models }) + } + + /// `model` and `effort` as Codex names them: the slug of the Model whose + /// slug or display name is `model`, regardless of case, and the Effort, + /// of those that Model supports, or with no Model of those any Model + /// supports, that is `effort`, regardless of case. Fails naming the + /// valid choices for a Model or Effort the catalog doesn't have. + fn settle(&self, model: Option<&str>, effort: Option<&str>) -> Result { + let found = match model { + Some(model) => Some(self.model(model)?), + None => None, + }; + let Some(effort) = effort else { + return Ok(ModelAndEffort { + model: found.map(|found| found.slug.clone()), + effort: None, + }); + }; + let efforts: Vec<&str> = match found { + Some(found) => found.efforts.iter().map(String::as_str).collect(), + None => { + let mut efforts = Vec::new(); + for each in self.models.iter().flat_map(|model| &model.efforts) { + if !efforts.contains(&each.as_str()) { + efforts.push(each.as_str()); + } + } + efforts + } + }; + let Some(settled) = efforts + .iter() + .find(|supported| supported.eq_ignore_ascii_case(effort)) + else { + let of = match found { + Some(found) => format!("the Codex Model {}", found.slug), + None => "any Codex Model".to_string(), + }; + bail!( + "the Effort {effort} is not one {of} supports: choose one of {}", + efforts.join(", ") + ); + }; + Ok(ModelAndEffort { + model: found.map(|found| found.slug.clone()), + effort: Some(settled.to_string()), + }) + } + + /// The Model whose slug or display name is `name`, regardless of case. + fn model(&self, name: &str) -> Result<&CatalogModel> { + let named = |candidate: &&CatalogModel| { + candidate.slug.eq_ignore_ascii_case(name) + || candidate + .display_name + .as_deref() + .is_some_and(|display| display.eq_ignore_ascii_case(name)) + }; + if let Some(found) = self.models.iter().find(named) { + return Ok(found); + } + let listed: Vec<&str> = self + .models + .iter() + .filter(|model| model.listed) + .map(|model| model.slug.as_str()) + .collect(); + bail!( + "the Model {name} is not in Codex's catalog: choose one of {}", + listed.join(", ") + ) } } @@ -458,4 +622,104 @@ mod tests { assert_eq!(chosen.built_with(), line); } } + + #[test] + fn codex_is_given_the_model_and_the_effort_in_its_config() { + for (model, effort, args) in [ + ("", "", vec![]), + ("gpt-6.1-sol", "", vec!["-m", "gpt-6.1-sol"]), + ( + "gpt-6.1-sol", + "max", + vec!["-m", "gpt-6.1-sol", "-c", "model_reasoning_effort=\"max\""], + ), + ] { + let chosen = choice(Harness::Codex, model, effort, ChosenBy::Command); + + assert_eq!(chosen.codex_args(), args); + } + } + + /// A catalog as `codex debug models` prints one, trimmed to what is read. + fn catalog() -> Catalog { + Catalog::parse( + r#"{"models": [ + {"slug": "gpt-6.1-sol", "display_name": "GPT-6.1-Sol", "visibility": "list", + "supported_reasoning_levels": [{"effort": "low"}, {"effort": "high"}, {"effort": "max"}]}, + {"slug": "gpt-5.5", "display_name": "GPT-5.5 Classic", "visibility": "list", + "supported_reasoning_levels": [{"effort": "low"}, {"effort": "xhigh"}]}, + {"slug": "codex-auto-review", "display_name": "Auto review", "visibility": "hide", + "supported_reasoning_levels": [{"effort": "minimal"}]} + ]}"#, + ) + .unwrap() + } + + fn settled(model: Option<&str>, effort: Option<&str>) -> Result { + catalog().settle(model, effort) + } + + #[test] + fn a_codex_model_and_effort_match_regardless_of_case_and_become_codexs_names() { + for (model, effort, settled_as) in [ + (Some("GPT-6.1-Sol"), Some("Max"), set("gpt-6.1-sol", "max")), + (Some("gpt-6.1-sol"), None, set("gpt-6.1-sol", "")), + ( + Some("gpt-5.5 classic"), + Some("XHIGH"), + set("gpt-5.5", "xhigh"), + ), + ( + Some("codex-auto-review"), + None, + set("codex-auto-review", ""), + ), + (None, Some("High"), set("", "high")), + (None, None, set("", "")), + ] { + assert_eq!( + settled(model, effort).unwrap(), + settled_as, + "{model:?} {effort:?}" + ); + } + } + + #[test] + fn an_unknown_codex_model_fails_naming_the_listed_models() { + let error = settled(Some("gpt-7"), Some("max")).unwrap_err(); + + assert_eq!( + error.to_string(), + "the Model gpt-7 is not in Codex's catalog: choose one of gpt-6.1-sol, gpt-5.5" + ); + } + + #[test] + fn an_effort_the_codex_model_does_not_support_fails_naming_those_it_does() { + let error = settled(Some("GPT-5.5"), Some("max")).unwrap_err(); + + assert_eq!( + error.to_string(), + "the Effort max is not one the Codex Model gpt-5.5 supports: choose one of low, xhigh" + ); + } + + #[test] + fn with_no_codex_model_the_effort_must_be_one_some_model_supports() { + let error = settled(None, Some("ultra")).unwrap_err(); + + assert_eq!( + error.to_string(), + "the Effort ultra is not one any Codex Model supports: choose one of low, high, max, \ + xhigh, minimal" + ); + } + + #[test] + fn output_that_is_no_catalog_fails() { + for output in ["", "not json", r#"{"models": 3}"#] { + assert!(Catalog::parse(output).is_err(), "{output}"); + } + } } diff --git a/src/main.rs b/src/main.rs index 3392fd5d1..e0fc0020a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -85,11 +85,14 @@ default takes [harness.claude]: model = \"opus\" effort = \"high\" -Before any work, the Harness's CLI must be on PATH, and a named Model gets a minimal test call -with its Effort, which must succeed: a failure stops the command naming what to fix, before -the Claim, the worktree and any Command log. Only claude works for now: harness codex fails as -not supported yet. The Command log, the Activity log's start line, the pull request's body, -as in Built with claude · opus · high, and the Run notification each name all three. +Before any work, the Harness's CLI must be on PATH. On claude, a named Model gets a minimal +test call with its Effort, which must succeed. On codex, a named Model and Effort must be in +codex debug models, matched regardless of case, a Model by slug or display name, and are +passed on as Codex names them. A failure stops the command naming what to fix, before the +Claim, the worktree and any Command log. Codex sessions run codex exec --json +--dangerously-bypass-approvals-and-sandbox, with the skills in .agents/skills/. The Command +log, the Activity log's start line, the pull request's body, as in Built with claude · opus · +high, and the Run notification each name all three. --email sends one Run notification when the Run ends, whatever the outcome: ready for review, merged, failed or interrupted. --email
sends it to
; a word @@ -383,12 +386,13 @@ fn main() -> ExitCode { Ok(started) => started, Err(failure) => return failure, }; - let asks = asks_of(&config); - started.built_with(&asks.harness); + let mut asks = asks_of(&config); let started_by = given .as_ref() .map_or(StartedBy::Command, |given| StartedBy::Child(&given.kind)); - started.finish(Ending::Run(run::run_to_end(&issue, &asks, started_by))) + let ended = run::run_to_end(&issue, &mut asks, started_by); + started.built_with(&asks.harness); + started.finish(Ending::Run(ended)) } /// An Architect run: the Architecture review and its plan marked ready, then, @@ -420,14 +424,15 @@ fn architect(args: ArchitectArgs) -> ExitCode { Ok(started) => started, Err(failure) => return failure, }; - let harness = args.flags.harness(&config); - started.built_with(&harness); - let review = match architect::run( + let mut harness = args.flags.harness(&config); + let review = architect::run( args.focus.as_deref(), args.base.as_deref(), config.launch_pull, - &harness, - ) { + &mut harness, + ); + started.built_with(&harness); + let review = match review { Ok(Outcome::Skipped(skipped)) => { return started.finish(Ending::Skipped(skipped.into())); } @@ -440,9 +445,12 @@ fn architect(args: ArchitectArgs) -> ExitCode { "dispatching the plan {url}, as thirdshift {url} would", url = plan.url )); - let asks = Asks::of_architect_plan(plan, &args.flags, &config); + let mut asks = Asks { + harness: harness.clone(), + ..Asks::of_architect_plan(plan, &args.flags, &config) + }; let started_by = StartedBy::Dispatch { base }; - Some(run::run_to_end(plan, &asks, started_by)) + Some(run::run_to_end(plan, &mut asks, started_by)) } _ => None, }; @@ -470,9 +478,10 @@ fn pickup(args: PickupArgs) -> ExitCode { Ok(started) => started, Err(failure) => return failure, }; - let harness = args.flags.harness(&config); + let mut harness = args.flags.harness(&config); + let taken = pickup::run(args.base.as_deref(), config.pickup_limit, &mut harness); started.built_with(&harness); - let taken = match pickup::run(args.base.as_deref(), config.pickup_limit, &harness) { + let taken = match taken { Ok(pickup::Outcome::Taken(taken)) => taken, Ok(pickup::Outcome::Skipped(skipped)) => { return started.finish(Ending::Skipped(skipped.into())); @@ -480,11 +489,14 @@ fn pickup(args: PickupArgs) -> ExitCode { Err(error) => return failure(&error), }; started.took(&taken.issue, taken.title); - let asks = Asks::of_ready_issue(&taken.issue, taken.is_spec, &args.flags, &config); + let mut asks = Asks { + harness, + ..Asks::of_ready_issue(&taken.issue, taken.is_spec, &args.flags, &config) + }; let started_by = StartedBy::Dispatch { base: &taken.base }; started.finish(Ending::Run(run::run_to_end( &taken.issue, - &asks, + &mut asks, started_by, ))) } diff --git a/src/pickup.rs b/src/pickup.rs index e67bc7569..f899d39be 100644 --- a/src/pickup.rs +++ b/src/pickup.rs @@ -112,7 +112,7 @@ impl fmt::Display for Skipped { /// it dispatches will run its sessions on, failing before any work if they /// can't run; then it records that it started work on the issue, which /// keeps its Command log. -pub fn run(base: Option<&str>, limit: NonZeroUsize, harness: &Choice) -> Result { +pub fn run(base: Option<&str>, limit: NonZeroUsize, harness: &mut Choice) -> Result { let Launch { directory, repo, diff --git a/src/progress.rs b/src/progress.rs index 8433d0fa2..7733170c0 100644 --- a/src/progress.rs +++ b/src/progress.rs @@ -1,15 +1,19 @@ //! Progress lines on stderr: thirdshift's own steps, and a session's -//! stream-json output condensed to one short line per notable event. Each -//! line is stamped with the local time it was printed, so a stalled Run can -//! be told from a busy one. Each is kept in the Command log too, if the -//! command keeps one. +//! stream, Claude's stream-json or Codex's JSONL, condensed to one short +//! line per notable event. Each line is stamped with the local time it was +//! printed, so a stalled Run can be told from a busy one. Each is kept in the +//! Command log too, if the command keeps one. + +mod codex; use std::collections::HashMap; use std::fmt::Display; +use std::path::Path; use chrono::Local; use serde_json::Value; +use crate::harness::Harness; use crate::logs; /// The longest detail a session line shows before it is cut short. @@ -108,21 +112,67 @@ fn split_stamp(line: &str) -> Option<(&str, &str)> { is_stamp.then_some((time, message)) } -/// Condenses one session's stream, a line at a time. Unknown and malformed -/// lines are skipped, never an error. +/// What a session's stream shows, read a line at a time: the progress lines +/// it condenses to, and once it has ended, how the session went. Each +/// Harness streams its own events: see [`for_harness`]. +pub trait Stream: Send { + /// The stderr lines, without the `thirdshift: ` prefix, for one line of + /// the stream: one per notable event in it, often none. Unknown and + /// malformed lines give none, never an error. + fn condense(&mut self, raw: &str) -> Vec; + + /// What the session-ended line says the session took, if the stream + /// told. + fn summary(&self) -> Option; + + /// The session's final message: what the agent said as it ended its + /// last turn, if anything. + fn final_message(&self) -> Option<&str>; + + /// The session's id, which a Resume continues. + fn session_id(&self) -> Option<&str>; + + /// Descriptions of the background work killed as the session ended: the + /// work still running when it ended its last turn, which it may have + /// been waiting on. None if that turn failed: the session ended for + /// another reason. + fn killed_background_work(&self) -> Vec<&str>; + + /// Whether the stream says the session failed, whatever its exit code. + fn failed(&self) -> bool { + false + } + + /// The error the stream gave for the session, for a failure's cause: + /// what failed its turn, else the last error it reported, if any. + fn error(&self) -> Option<&str> { + None + } +} + +/// The [`Stream`] of a session on `harness` in `worktree`. +pub fn for_harness(harness: Harness, worktree: &Path) -> Box { + match harness { + Harness::Claude => Box::new(ClaudeProgress::default()), + Harness::Codex => Box::new(codex::CodexProgress::in_worktree(worktree)), + } +} + +/// Condenses one Claude session's stream-json, a line at a time. Unknown and +/// malformed lines are skipped, never an error. /// /// A `result` event doesn't mean the session is over: a session that waits on /// background sub-agents resumes, with another `system`/`init` event, and ends /// each resumption with another `result`. So only the first `init` gives a -/// line, `result` events give none, and [`Progress::summary`] reports the last +/// line, `result` events give none, and [`ClaudeProgress::summary`] reports the last /// one once the process has exited. /// /// A background task still running when the session ends its turn for good is /// killed as the process exits, so a task killed after the last `result` is /// work the session may have been waiting on, or work it had given up on and -/// left running: see [`Progress::killed_background_work`]. +/// left running: see [`ClaudeProgress::killed_background_work`]. #[derive(Default)] -pub struct Progress { +pub struct ClaudeProgress { started: bool, cwd: Option, session_id: Option, @@ -140,10 +190,8 @@ pub struct Progress { final_message: Option, } -impl Progress { - /// The stderr lines, without the `thirdshift: ` prefix, for one line of - /// the stream: one per notable event in it, often none. - pub fn condense(&mut self, raw: &str) -> Vec { +impl Stream for ClaudeProgress { + fn condense(&mut self, raw: &str) -> Vec { let Ok(event) = serde_json::from_str::(raw) else { return Vec::new(); }; @@ -186,7 +234,7 @@ impl Progress { /// Turns and cost from the last `result` event that reported them. On a /// claude.ai subscription the cost is the API-price equivalent, not a /// charge, and says so. - pub fn summary(&self) -> Option { + fn summary(&self) -> Option { let (turns, cost) = self.turns_and_cost?; let basis = if self.subscription { " at API prices" @@ -196,24 +244,21 @@ impl Progress { Some(format!("{turns} turns, ${cost:.2}{basis}")) } - /// The session's final message: the text of its last `result` event, - /// what the agent said as it ended its last turn. None if that `result` - /// had no text, as one that is an error may not. - pub fn final_message(&self) -> Option<&str> { + /// The text of the last `result` event. None if that `result` had no + /// text, as one that is an error may not. + fn final_message(&self) -> Option<&str> { self.final_message.as_deref() } /// The session's id, from its first `init` event. - pub fn session_id(&self) -> Option<&str> { + fn session_id(&self) -> Option<&str> { self.session_id.as_deref() } - /// Descriptions of the background tasks killed after the last `result`: - /// the work still running when the session ended, which it may have been - /// waiting on. The stream doesn't say whether it was, or had been - /// abandoned. None if that `result` was an error: the session ended for - /// another reason. - pub fn killed_background_work(&self) -> Vec<&str> { + /// Descriptions of the background tasks killed after the last `result`. + /// The stream doesn't say whether the session was waiting on them, or + /// had abandoned them. None if that `result` was an error. + fn killed_background_work(&self) -> Vec<&str> { if self.failed { return Vec::new(); } @@ -222,7 +267,9 @@ impl Progress { .map(|(_, description)| description.as_str()) .collect() } +} +impl ClaudeProgress { /// Track a background task's description and whether it was killed. fn track_task(&mut self, event: &Value) { let Some(id) = event["task_id"].as_str() else { @@ -310,8 +357,8 @@ mod tests { use super::*; use serde_json::json; - fn lines(events: &[Value]) -> (Progress, Vec>) { - let mut progress = Progress::default(); + fn lines(events: &[Value]) -> (ClaudeProgress, Vec>) { + let mut progress = ClaudeProgress::default(); let lines = events .iter() .map(|event| progress.condense(&event.to_string())) @@ -496,7 +543,7 @@ mod tests { #[test] fn no_result_means_no_summary() { - assert_eq!(Progress::default().summary(), None); + assert_eq!(ClaudeProgress::default().summary(), None); } fn task_started(id: &str, description: &str) -> Value { @@ -678,7 +725,7 @@ mod tests { #[test] fn unknown_and_malformed_lines_are_skipped() { - let mut progress = Progress::default(); + let mut progress = ClaudeProgress::default(); for raw in [ "", "not json", diff --git a/src/progress/codex.rs b/src/progress/codex.rs new file mode 100644 index 000000000..66168ba83 --- /dev/null +++ b/src/progress/codex.rs @@ -0,0 +1,492 @@ +//! Codex's `exec --json` stream, condensed: `thread.started` gives the +//! session id, `item.*` events give a progress line per command, file +//! change and tool call, `turn.completed` the token totals and +//! `turn.failed` the error the session failed with. See +//! `docs/research/codex-headless-harness.md`. + +use std::collections::HashSet; +use std::path::Path; + +use serde_json::Value; + +use super::{Stream, bash, shorten}; + +/// Condenses one Codex session's JSONL stream, a line at a time. +/// +/// `codex exec` runs one turn per process. A command the agent left running +/// shows as a `command_execution` item still `in_progress` when the turn +/// completes, and it is killed as Codex exits: see +/// [`Stream::killed_background_work`]. +#[derive(Default)] +pub struct CodexProgress { + /// The worktree the session runs in, which Codex's stream doesn't name. + cwd: String, + session_id: Option, + /// The ids of the items that have had their line. + seen: HashSet, + /// The commands still running, by item id, in the order they started. + running: Vec<(String, String)>, + /// The commands still running when the turn completed. + killed: Vec, + /// Token totals from `turn.completed`. + tokens: Option, + final_message: Option, + /// What failed the turn, from `turn.failed`. + failure: Option, + /// The last error the stream reported, which may have been retried. + last_error: Option, +} + +/// A session's token totals. +struct Tokens { + input: u64, + /// Of the input, how many were cached. + cached: u64, + output: u64, +} + +impl CodexProgress { + /// The stream of a session running in `worktree`. + pub fn in_worktree(worktree: &Path) -> Self { + CodexProgress { + cwd: worktree.display().to_string(), + ..CodexProgress::default() + } + } + + /// Keep the message of an `error`, an event or an item, as the last + /// error reported. + fn reported(&mut self, error: &Value) { + if let Some(message) = error["message"].as_str() { + self.last_error = Some(message.to_string()); + } + } + + /// Track an item's state, and give its line the first time it is seen. + fn item(&mut self, item: &Value) -> Vec { + let id = item["id"].as_str().unwrap_or("").to_string(); + match item["type"].as_str() { + Some("command_execution") => { + self.running.retain(|(running, _)| *running != id); + if item["status"] == "in_progress" { + let command = unwrapped(item["command"].as_str().unwrap_or("")); + self.running.push((id.clone(), command)); + } + } + Some("agent_message") => { + if let Some(text) = item["text"].as_str() { + self.final_message = Some(text.to_string()); + } + } + Some("error") => { + self.reported(item); + } + _ => {} + } + if !self.seen.insert(id) { + return Vec::new(); + } + self.lines_for(item) + } + + /// The progress lines an item gives. + fn lines_for(&self, item: &Value) -> Vec { + let text = |key: &str| item[key].as_str().unwrap_or("?"); + match item["type"].as_str() { + Some("command_execution") => vec![bash(&unwrapped(text("command")))], + Some("file_change") => item["changes"] + .as_array() + .into_iter() + .flatten() + .map(|change| { + let path = change["path"].as_str().unwrap_or("?"); + let kind = match change["kind"].as_str() { + Some("add") => "Add", + Some("delete") => "Delete", + _ => "Edit", + }; + format!("{kind} {}", shorten(self.relative(path))) + }) + .collect(), + Some("mcp_tool_call") => vec![format!("{} {}", text("server"), text("tool"))], + Some("web_search") => vec![format!("web search {}", shorten(text("query")))], + Some("collab_tool_call") => vec![format!("agent {}", text("tool"))], + _ => Vec::new(), + } + } + + /// `path` relative to the worktree, if it is inside it. + fn relative<'a>(&self, path: &'a str) -> &'a str { + path.strip_prefix(&self.cwd) + .and_then(|rest| rest.strip_prefix('/')) + .unwrap_or(path) + } +} + +impl Stream for CodexProgress { + fn condense(&mut self, raw: &str) -> Vec { + let Ok(event) = serde_json::from_str::(raw) else { + return Vec::new(); + }; + match event["type"].as_str() { + Some("thread.started") if self.session_id.is_none() => { + self.session_id = event["thread_id"].as_str().map(String::from); + vec!["session started".to_string()] + } + Some("item.started" | "item.updated" | "item.completed") => self.item(&event["item"]), + Some("turn.completed") => { + let usage = &event["usage"]; + let count = |key: &str| usage[key].as_u64(); + if let (Some(input), Some(output)) = (count("input_tokens"), count("output_tokens")) + { + let cached = count("cached_input_tokens").unwrap_or(0); + self.tokens = Some(Tokens { + input, + cached, + output, + }); + } + self.killed = self + .running + .iter() + .map(|(_, command)| command.clone()) + .collect(); + Vec::new() + } + Some("turn.failed") => { + if self.failure.is_none() { + let message = event["error"]["message"].as_str().unwrap_or("turn failed"); + self.failure = Some(message.to_string()); + } + Vec::new() + } + Some("error") => { + self.reported(&event); + Vec::new() + } + _ => Vec::new(), + } + } + + /// The token totals from `turn.completed`: Codex reports no turns and no + /// cost. + fn summary(&self) -> Option { + let Tokens { + input, + cached, + output, + } = self.tokens.as_ref()?; + Some(format!( + "{input} input tokens ({cached} cached), {output} output tokens" + )) + } + + /// The text of the last `agent_message` item. + fn final_message(&self) -> Option<&str> { + self.final_message.as_deref() + } + + /// The thread id from `thread.started`, which `codex exec resume` takes. + fn session_id(&self) -> Option<&str> { + self.session_id.as_deref() + } + + /// The commands still running when the turn completed. + fn killed_background_work(&self) -> Vec<&str> { + if self.failure.is_some() { + return Vec::new(); + } + self.killed.iter().map(String::as_str).collect() + } + + fn failed(&self) -> bool { + self.failure.is_some() + } + + fn error(&self) -> Option<&str> { + self.failure.as_deref().or(self.last_error.as_deref()) + } +} + +/// `command` without the shell Codex wraps it in, as in `/bin/bash -lc 'git +/// push'`, if it is wrapped so. +fn unwrapped(command: &str) -> String { + let mut words = command.splitn(3, ' '); + let (Some(shell), Some(flag), Some(script)) = (words.next(), words.next(), words.next()) else { + return command.to_string(); + }; + let is_shell = ["bash", "sh", "zsh"] + .iter() + .any(|name| shell == *name || shell.ends_with(&format!("/{name}"))); + if !is_shell || !matches!(flag, "-c" | "-lc") { + return command.to_string(); + } + if let Some(quoted) = script + .strip_prefix('\'') + .and_then(|rest| rest.strip_suffix('\'')) + { + return quoted.replace("'\\''", "'"); + } + if let Some(quoted) = script + .strip_prefix('"') + .and_then(|rest| rest.strip_suffix('"')) + { + return quoted.replace("\\\"", "\""); + } + script.to_string() +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + fn lines(events: &[Value]) -> (CodexProgress, Vec>) { + let mut progress = CodexProgress::in_worktree(Path::new("/work/widgets-issue-7")); + let lines = events + .iter() + .map(|event| progress.condense(&event.to_string())) + .collect(); + (progress, lines) + } + + fn item(kind: &str, id: &str, item: Value) -> Value { + let mut item = item; + item["id"] = json!(id); + json!({ "type": kind, "item": item }) + } + + fn command(kind: &str, id: &str, command: &str, status: &str) -> Value { + item( + kind, + id, + json!({ "type": "command_execution", "command": command, "status": status }), + ) + } + + fn completed(input: u64, output: u64) -> Value { + json!({ "type": "turn.completed", "usage": { + "input_tokens": input, "cached_input_tokens": 100, "output_tokens": output + } }) + } + + #[test] + fn the_first_thread_started_starts_the_session_and_gives_its_id() { + let (progress, lines) = lines(&[ + json!({ "type": "thread.started", "thread_id": "019a-thread" }), + json!({ "type": "turn.started" }), + ]); + + assert_eq!(lines, [vec!["session started".to_string()], vec![]]); + assert_eq!(progress.session_id(), Some("019a-thread")); + } + + #[test] + fn a_command_gets_one_line_without_its_shell_naming_commits_and_pushes() { + let (_, lines) = lines(&[ + command( + "item.started", + "item_1", + "/bin/bash -lc 'cargo test'", + "in_progress", + ), + command( + "item.completed", + "item_1", + "/bin/bash -lc 'cargo test'", + "completed", + ), + command( + "item.started", + "item_2", + "bash -lc 'git add -A && git commit -m '\\''x'\\'' && git push'", + "in_progress", + ), + command("item.completed", "item_3", "git push", "completed"), + ]); + + assert_eq!( + lines, + [ + vec!["$ cargo test".to_string()], + vec![], + vec!["commit and push".to_string()], + vec!["push".to_string()], + ] + ); + } + + #[test] + fn file_changes_tool_calls_and_searches_get_a_line_each() { + let (_, lines) = lines(&[ + item( + "item.completed", + "item_1", + json!({ "type": "file_change", "status": "completed", "changes": [ + { "path": "/work/widgets-issue-7/src/run.rs", "kind": "update" }, + { "path": "/work/widgets-issue-7/NOTES.md", "kind": "add" }, + { "path": "/etc/hosts", "kind": "delete" } + ] }), + ), + item( + "item.started", + "item_2", + json!({ "type": "mcp_tool_call", "server": "github", "tool": "get_issue" }), + ), + item( + "item.started", + "item_3", + json!({ "type": "web_search", "query": "codex exec json" }), + ), + item( + "item.started", + "item_4", + json!({ "type": "collab_tool_call", "tool": "spawn_agent" }), + ), + ]); + + assert_eq!( + lines, + [ + vec![ + "Edit src/run.rs".to_string(), + "Add NOTES.md".to_string(), + "Delete /etc/hosts".to_string() + ], + vec!["github get_issue".to_string()], + vec!["web search codex exec json".to_string()], + vec!["agent spawn_agent".to_string()], + ] + ); + } + + #[test] + fn messages_reasoning_and_turn_events_give_no_line() { + let (_, lines) = lines(&[ + item( + "item.completed", + "item_1", + json!({ "type": "reasoning", "text": "Thinking." }), + ), + item( + "item.completed", + "item_2", + json!({ "type": "agent_message", "text": "Done." }), + ), + json!({ "type": "turn.started" }), + completed(10, 2), + ]); + + assert_eq!(lines, [vec![], vec![], vec![], vec![]] as [Vec; 4]); + } + + #[test] + fn the_final_message_is_the_last_agent_message() { + let said = |id: &str, text: &str| { + item( + "item.completed", + id, + json!({ "type": "agent_message", "text": text }), + ) + }; + let (progress, _) = lines(&[said("item_1", "Running tests."), said("item_5", "Done.")]); + + assert_eq!(progress.final_message(), Some("Done.")); + } + + #[test] + fn the_summary_is_the_token_totals() { + let (progress, _) = lines(&[completed(12000, 345)]); + + assert_eq!( + progress.summary().as_deref(), + Some("12000 input tokens (100 cached), 345 output tokens") + ); + assert_eq!(lines(&[]).0.summary(), None); + } + + #[test] + fn a_command_still_running_when_the_turn_completes_is_killed_background_work() { + let (progress, _) = lines(&[ + command( + "item.started", + "item_1", + "/bin/bash -lc 'sleep 188'", + "in_progress", + ), + command("item.started", "item_2", "cargo build", "in_progress"), + command("item.completed", "item_2", "cargo build", "completed"), + completed(10, 2), + ]); + + assert_eq!(progress.killed_background_work(), ["sleep 188"]); + assert!(!progress.failed()); + } + + #[test] + fn a_command_reconciled_as_in_progress_at_turn_end_is_killed_background_work() { + let (progress, _) = lines(&[ + command("item.started", "item_1", "npm run dev", "in_progress"), + command("item.completed", "item_1", "npm run dev", "in_progress"), + completed(10, 2), + ]); + + assert_eq!(progress.killed_background_work(), ["npm run dev"]); + } + + #[test] + fn a_failed_turn_fails_with_its_error_and_kills_no_background_work() { + let (progress, _) = lines(&[ + command("item.started", "item_1", "sleep 188", "in_progress"), + json!({ "type": "error", "message": "Reconnecting... 1/5" }), + json!({ "type": "turn.failed", "error": { "message": "The 'GPT-6.1-Sol' model is not supported." } }), + ]); + + assert!(progress.failed()); + assert_eq!( + progress.error(), + Some("The 'GPT-6.1-Sol' model is not supported.") + ); + assert!(progress.killed_background_work().is_empty()); + } + + #[test] + fn an_error_event_alone_does_not_fail_the_session_but_is_its_last_error() { + let (progress, _) = lines(&[ + json!({ "type": "error", "message": "Reconnecting... 1/5" }), + completed(10, 2), + ]); + + assert!(!progress.failed()); + assert_eq!(progress.error(), Some("Reconnecting... 1/5")); + } + + #[test] + fn unknown_and_malformed_lines_are_skipped() { + let mut progress = CodexProgress::default(); + for raw in [ + "", + "not json", + "[1, 2]", + r#"{"type": "item.started""#, + r#"{"type": "item.started", "item": 3}"#, + r#"{"type": "item.started", "item": {"type": "command_execution"}}"#, + r#"{"type": "turn.completed", "usage": {"input_tokens": "many"}}"#, + ] { + let lines = progress.condense(raw); + assert!(lines.len() <= 1, "{raw}: {lines:?}"); + } + assert_eq!(progress.summary(), None); + } + + #[test] + fn a_command_not_wrapped_in_a_shell_is_kept_as_it_is() { + for command in ["cargo test", "bash script.sh", "bash -lc", "python -c 'x'"] { + assert_eq!(unwrapped(command), command); + } + assert_eq!( + unwrapped("/usr/bin/zsh -c \"echo \\\"hi\\\"\""), + "echo \"hi\"" + ); + } +} diff --git a/src/prompts_page.rs b/src/prompts_page.rs index 39a2421e9..aa22c1540 100644 --- a/src/prompts_page.rs +++ b/src/prompts_page.rs @@ -13,10 +13,10 @@ use include_dir::{Dir, File}; use crate::ci::FailedChecks; use crate::github::{Check, CheckState}; -use crate::harness::Choice; +use crate::harness::{Choice, Harness}; use crate::issue::IssueUrl; use crate::prompt; -use crate::session::claude_args; +use crate::session::{claude_args, codex_args}; use crate::skills::SKILLS; use Sender::{ArchitectRun, Units}; @@ -58,7 +58,9 @@ const BACKGROUND_WORK: &str = ""; const FOCUS: &str = ""; const SESSION_ID: &str = ""; const PROMPT: &str = ""; -const PLACEHOLDERS: [&str; 15] = [ +const MODEL: &str = ""; +const EFFORT: &str = ""; +const PLACEHOLDERS: [&str; 17] = [ ISSUE_URL, SPEC_URL, NUMBER, @@ -74,6 +76,8 @@ const PLACEHOLDERS: [&str; 15] = [ FOCUS, SESSION_ID, PROMPT, + MODEL, + EFFORT, ]; /// Who sends a prompt. @@ -259,7 +263,7 @@ fn readme(prompts: &[Prompt]) -> String { .find(|prompt| prompt.id == "prompt-resume") .expect("there is a Resume prompt"); let mut readme = format!( - " + " # Session prompts @@ -277,12 +281,15 @@ A [Resume]({resume_file}) continues the session that ended: {resume} ``` +{codex} + | Prompt | When it is sent | File | | --- | --- | --- | ", new = command(None), resume = command(Some(SESSION_ID)), resume_file = file_name(resume), + codex = codex_note(|code| format!("`{code}`")), ); for prompt in prompts { let _ = writeln!( @@ -354,6 +361,33 @@ fn command(resume: Option<&str>) -> String { format!("claude {}", claude_args(&harness, resume, PROMPT).join(" ")) } +/// The `codex` command line a session on a Model and Effort starts with, +/// with the page's placeholders. +fn codex_command() -> String { + let harness = Choice { + harness: Harness::Codex, + model: Some(MODEL.to_string()), + effort: Some(EFFORT.to_string()), + ..Choice::default() + }; + format!("codex {}", codex_args(&harness, None, PROMPT).join(" ")) +} + +/// The note on how a session runs on Codex instead, with `code` marking up +/// each piece of code in it. +fn codex_note(code: impl Fn(&str) -> String) -> String { + format!( + "With {harness}, each session runs {command} instead, without {model} or the Effort's {c} where they are not set, and with {resume} before the prompt for a Resume. The Factory skills are linked into {dir}, and a prompt's first line loads its skill as {sigil}.", + harness = code("harness codex"), + command = code(&codex_command()), + model = code(&format!("-m {MODEL}")), + c = code("-c"), + resume = code(&format!("resume {SESSION_ID}")), + dir = code(".agents/skills/"), + sigil = code("$thirdshift-"), + ) +} + fn command_line(html: &mut String) { let _ = write!( html, @@ -364,11 +398,13 @@ fn command_line(html: &mut String) { 
{new}

A Resume continues the session that ended:

{resume}
+

{codex}

"##, new = highlighted(&command(None)), resume = highlighted(&command(Some(SESSION_ID))), + codex = codex_note(|code| format!("{}", highlighted(code))), ); } @@ -577,7 +613,7 @@ mod tests { } assert!( checked_in == rendered, - "{} is out of date with the prompts, the claude arguments or the skills:\n{}\nRegenerate it with: {UPDATE}", + "{} is out of date with the prompts, the claude and codex arguments or the skills:\n{}\nRegenerate it with: {UPDATE}", page_path.display(), diff(checked_in, &rendered, before.lines().count() + 2), ); diff --git a/src/run.rs b/src/run.rs index d35bcbe00..fab341e20 100644 --- a/src/run.rs +++ b/src/run.rs @@ -111,9 +111,10 @@ impl<'a> StartedBy<'a> { } /// [`run`] the Run on `issue` that `started_by` started and that is asked -/// `asks`, to its end. Its Run notification, if `asks` has it send one, is -/// for what started the Run to send, once it has ended. -pub fn run_to_end(issue: &IssueUrl, asks: &Asks, started_by: StartedBy) -> Ended { +/// `asks`, to its end, with the Model and Effort in `asks` settled on the +/// Harness's names for them once checked. Its Run notification, if `asks` +/// has it send one, is for what started the Run to send, once it has ended. +pub fn run_to_end(issue: &IssueUrl, asks: &mut Asks, started_by: StartedBy) -> Ended { let mut base_fix = BaseFix::new( started_by.child(), asks.base_fix.clone(), @@ -139,8 +140,9 @@ pub fn run_to_end(issue: &IssueUrl, asks: &Asks, started_by: StartedBy) -> Ended /// The Launch directory is opened first, with its Origin match, the check /// that `issue` is open and the git identity check. A Run started by its /// command then checks the Harness, Model and Effort its sessions run on, as -/// `asks` say: one started by another thirdshift, or dispatched by a pass, -/// runs on what that command checked. The rest is [`start`], through the +/// `asks` say, settling the Model and Effort in them, and in `base_fix`, on +/// the Harness's names: one started by another thirdshift, or dispatched by +/// a pass, runs on what that command checked. The rest is [`start`], through the /// Launch directory, GitHub and the logs. /// /// `base_fix` is the one Base fix the Run, or a Spec run for its Spec PR, may @@ -148,14 +150,16 @@ pub fn run_to_end(issue: &IssueUrl, asks: &Asks, started_by: StartedBy) -> Ended /// `asks` ask about one is already in it. fn run( issue: &IssueUrl, - asks: &Asks, + asks: &mut Asks, started_by: StartedBy, base_fix: &mut BaseFix, ) -> Result { let directory = LaunchDirectory::open_for_run(issue)?; if let StartedBy::Command = started_by { asks.harness.check()?; + base_fix.runs_on(asks.harness.clone()); } + let asks = &*asks; let mut outside = LaunchAndGitHub { directory: &directory, issue, diff --git a/src/session.rs b/src/session.rs index 5497338bd..d3185ae7f 100644 --- a/src/session.rs +++ b/src/session.rs @@ -1,4 +1,4 @@ -//! Headless Claude Code sessions and their logs. +//! Headless agent sessions, on Claude Code or Codex, and their logs. use std::cell::RefCell; use std::fs::{self, File}; @@ -11,11 +11,11 @@ use std::time::{Duration, Instant}; use anyhow::{Context, Result, anyhow, bail}; -use crate::harness::Choice; +use crate::harness::{Choice, Harness}; use crate::interrupt; use crate::issue::{IssueUrl, Repo}; use crate::logs; -use crate::progress::{self, Progress}; +use crate::progress::{self, Stream}; use crate::prompt; use crate::skills; @@ -86,7 +86,7 @@ impl<'a> Sessions<'a> { harness: &'a Choice, steps: impl FnOnce(&Self) -> Result, ) -> (Result, Option) { - if let Err(error) = skills::link_into(worktree) { + if let Err(error) = skills::link_into(worktree, harness.harness) { return (Err(error), None); } let sessions = Sessions { @@ -149,12 +149,14 @@ impl<'a> Sessions<'a> { /// Run one session as `kind`, logged under its own path, which becomes /// the last log. - fn start(&self, kind: &str, resume: Option<&str>, prompt: &str) -> Result { + fn start(&self, kind: &str, resume: Option<&str>, prompt: &str) -> Result> { let log = self.logs.path(kind); progress::step(format_args!("logging the session to {}", log.display())); *self.last_log.borrow_mut() = Some(log.clone()); - let args = claude_args(self.harness, resume, prompt); - run(kind, self.worktree, &args, &log) + let harness = self.harness.harness; + let args = session_args(self.harness, resume, prompt); + let stream = progress::for_harness(harness, self.worktree); + run(kind, harness, self.worktree, &args, &log, stream) } } @@ -170,19 +172,29 @@ fn ending_with(killed: &[&str]) -> String { } } -/// Run `claude` with `args`, as [`claude_args`] gives them, in `worktree`, -/// where it finds the Factory skills, streaming its output to `log` and -/// condensing it to progress lines on stderr, each labelled `kind`. Returns -/// what the stream showed once `claude` has exited cleanly. An interrupt -/// stops the session and fails with `interrupted`. -fn run(kind: &str, worktree: &Path, args: &[&str], log: &Path) -> Result { +/// Run `harness`'s CLI with `args`, as [`session_args`] gives them, in +/// `worktree`, where it finds the Factory skills, streaming its output to +/// `log` and condensing it through `stream` to progress lines on stderr, +/// each labelled `kind`. Returns what the stream showed once the CLI has +/// exited cleanly, its turn not failed. Otherwise fails with the error the +/// stream gave, if any. An interrupt stops the session and fails with +/// `interrupted`. +fn run( + kind: &str, + harness: Harness, + worktree: &Path, + args: &[String], + log: &Path, + mut stream: Box, +) -> Result> { + let cli = harness.name(); if let Some(dir) = log.parent() { fs::create_dir_all(dir).with_context(|| format!("could not create {}", dir.display()))?; } let mut log_file = File::create(log).with_context(|| format!("could not create {}", log.display()))?; let started = Instant::now(); - let mut child = Command::new("claude") + let mut child = Command::new(cli) .args(args) .current_dir(worktree) .stdin(Stdio::null()) @@ -191,60 +203,96 @@ fn run(kind: &str, worktree: &Path, args: &[&str], log: &Path) -> Result bail!("{cli}{ended}: {error}"), + None => bail!("{cli}{ended}"), + } +} + +/// The arguments a session runs `harness`'s CLI with, as [`claude_args`] or +/// [`codex_args`] give them, its prompt loading its skill with that +/// Harness's sigil. +fn session_args(harness: &Choice, resume: Option<&str>, prompt: &str) -> Vec { + match harness.harness { + Harness::Claude => claude_args(harness, resume, prompt) + .into_iter() + .map(String::from) + .collect(), + Harness::Codex => codex_args(harness, resume, &codex_prompt(prompt)), + } +} + +/// `prompt` as Codex takes it: a first line that loads a Factory skill, +/// `/thirdshift-` as Claude's prompts write it, loads it as +/// `$thirdshift-`. +pub fn codex_prompt(prompt: &str) -> String { + match prompt.strip_prefix("/thirdshift-") { + Some(rest) => format!("$thirdshift-{rest}"), + None => prompt.to_string(), } - Ok(progress) } /// The arguments every session runs `claude` with: headless in auto mode, @@ -265,6 +313,33 @@ pub fn claude_args<'a>( args } +/// The arguments every session runs `codex` with: `exec`, streaming JSONL, +/// with no approvals and no sandbox (ADR-0012), on the Model and Effort +/// `harness` sets, if any, reading `CLAUDE.md` where a directory has no +/// `AGENTS.md`. With `resume`, the session with that id continues, given +/// every one of those again, as Codex keeps none of them. The prompt comes +/// last. +pub fn codex_args(harness: &Choice, resume: Option<&str>, prompt: &str) -> Vec { + let mut args: Vec = [ + "exec", + "--json", + "--dangerously-bypass-approvals-and-sandbox", + ] + .map(String::from) + .to_vec(); + args.extend(harness.codex_args()); + args.extend(["-c".to_string(), CLAUDE_MD_FALLBACK.to_string()]); + if let Some(session_id) = resume { + args.extend(["resume".to_string(), session_id.to_string()]); + } + args.push(prompt.to_string()); + args +} + +/// The config setting that has Codex read `CLAUDE.md` where a directory has +/// no `AGENTS.md`. +const CLAUDE_MD_FALLBACK: &str = r#"project_doc_fallback_filenames=["CLAUDE.md"]"#; + const POLL: Duration = Duration::from_millis(100); /// How long a session gets to exit after SIGTERM before it is killed. @@ -295,7 +370,7 @@ fn follow( stream: impl Read, log_file: &mut File, log: &Path, - progress: &mut Progress, + progress: &mut dyn Stream, ) -> Result<()> { let mut stream = BufReader::new(stream); let mut line = Vec::new(); @@ -325,3 +400,36 @@ fn minutes_and_seconds(duration: Duration) -> String { minutes => format!("{minutes}m {}s", seconds % 60), } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_prompt_that_loads_a_skill_loads_it_with_codexs_sigil_on_codex() { + let issue = IssueUrl::parse("https://github.com/acme/widgets/issues/7").unwrap(); + for (prompt, first_line) in [ + ( + prompt::fresh(&issue, "main", "issue-7"), + "$thirdshift-implement https://github.com/acme/widgets/issues/7", + ), + ( + prompt::conflict_repair(&issue, "main", "issue-7", ""), + "$thirdshift-resolving-merge-conflicts", + ), + ( + prompt::review_repair(&issue, "issue-7", "", "abc123"), + "$thirdshift-code-review abc123", + ), + ( + prompt::resume(&["cargo test"]), + "Your background work (cargo test) was killed when your turn ended, because \ + ending the turn ends the session.", + ), + ] { + let codex_prompt = codex_prompt(&prompt); + assert_eq!(codex_prompt.lines().next(), Some(first_line)); + assert_eq!(codex_prompt[1..], prompt[1..]); + } + } +} diff --git a/src/skills.rs b/src/skills.rs index 3043887ec..7a6d646fb 100644 --- a/src/skills.rs +++ b/src/skills.rs @@ -1,6 +1,6 @@ //! The Factory skills, embedded at compile time (ADR-0001), written out once -//! per Command and linked into each worktree its sessions run in, where -//! Claude Code finds project skills (ADR-0012). +//! per Command and linked into each worktree its sessions run in, where the +//! Harness finds project skills (ADR-0012). use std::fs::{self, OpenOptions}; use std::io::{ErrorKind, Write}; @@ -13,29 +13,34 @@ use include_dir::{Dir, include_dir}; use tempfile::TempDir; use crate::git::Git; +use crate::harness::Harness; /// The Factory skills, as they are written out and the Prompts and skills /// page shows them, each in its `thirdshift-` directory. pub static SKILLS: Dir = include_dir!("$CARGO_MANIFEST_DIR/skills"); -/// Where in a worktree Claude Code finds its project skills. -const PROJECT_SKILLS: &str = ".claude/skills"; - -/// The `.git/info/exclude` pattern that keeps the linked skills out of git. -const EXCLUDE: &str = "/.claude/skills/thirdshift-*"; +/// Where in a worktree `harness` finds its project skills. +fn project_skills(harness: Harness) -> &'static str { + match harness { + Harness::Claude => ".claude/skills", + Harness::Codex => ".agents/skills", + } +} /// The temp directory the Command wrote the Factory skills out to, once it /// has. static WRITTEN: Mutex> = Mutex::new(None); -/// Link every Factory skill into `worktree`'s `.claude/skills/`, writing them -/// out first if the Command has not yet, and make sure the repository's -/// `.git/info/exclude` keeps them out of git. The links are left in place: -/// they go with the worktree. -pub fn link_into(worktree: &Path) -> Result<()> { +/// Link every Factory skill into `worktree`'s project skills for `harness`, +/// `.claude/skills/` or `.agents/skills/`, writing them out first if the +/// Command has not yet, and make sure the repository's `.git/info/exclude` +/// keeps them out of git. The links are left in place: they go with the +/// worktree. +pub fn link_into(worktree: &Path, harness: Harness) -> Result<()> { let written = written_out()?; - exclude(&Git::new(worktree))?; - let project_skills = worktree.join(PROJECT_SKILLS); + let dir = project_skills(harness); + exclude(&Git::new(worktree), &format!("/{dir}/thirdshift-*"))?; + let project_skills = worktree.join(dir); fs::create_dir_all(&project_skills) .with_context(|| format!("could not create {}", project_skills.display()))?; for skill in SKILLS.dirs() { @@ -80,11 +85,11 @@ fn written_out() -> Result { Ok(path) } -/// Add [`EXCLUDE`] to the `.git/info/exclude` of the repository `git` works -/// in, unless it is there already. The file is shared by every worktree, so -/// the entry is left there, under a lock other Runs from the same Launch -/// directory take too. -fn exclude(git: &Git) -> Result<()> { +/// Add the pattern `exclude` to the `.git/info/exclude` of the repository +/// `git` works in, unless it is there already. The file is shared by every +/// worktree, so the entry is left there, under a lock other Runs from the +/// same Launch directory take too. +fn exclude(git: &Git, exclude: &str) -> Result<()> { let _lock = git.lock("thirdshift-exclude.lock")?; let info = git.common_dir()?.join("info"); let path = info.join("exclude"); @@ -95,7 +100,7 @@ fn exclude(git: &Git) -> Result<()> { return Err(error).with_context(|| format!("could not read {}", path.display())); } }; - if existing.lines().any(|line| line == EXCLUDE) { + if existing.lines().any(|line| line == exclude) { return Ok(()); } fs::create_dir_all(&info).with_context(|| format!("could not create {}", info.display()))?; @@ -108,6 +113,6 @@ fn exclude(git: &Git) -> Result<()> { .create(true) .append(true) .open(&path) - .and_then(|mut file| writeln!(file, "{separator}{EXCLUDE}")) + .and_then(|mut file| writeln!(file, "{separator}{exclude}")) .with_context(|| format!("could not add the Factory skills to {}", path.display())) } diff --git a/tests/codex_sessions.rs b/tests/codex_sessions.rs new file mode 100644 index 000000000..eb4f75eba --- /dev/null +++ b/tests/codex_sessions.rs @@ -0,0 +1,439 @@ +//! A Command's sessions on Codex, with `harness codex`: each runs `codex exec +//! --json` unsandboxed in the Run's worktree, with stdin set to null, on the +//! Model and Effort as Codex names them, reading `CLAUDE.md` where there is +//! no `AGENTS.md`, with the Factory skills linked into the worktree's +//! `.agents/skills/` and its prompt loading its skill as +//! `$thirdshift-`. Codex's stream gives the progress lines, the +//! Session log, the session id a Resume continues and the final message, and +//! a failed turn fails the Run with Codex's error. The Model and Effort are +//! checked against `codex debug models` before any work. + +mod support; + +use std::fs; + +use serde_json::Value; +use support::{RunResult, Scenario}; + +/// The agent commits its work and opens a PR that closes issue #7. +const AGENT_OPENS_PR: &str = r#" +echo "feature" > feature.txt +git add feature.txt +git commit -q -m "Add feature" +gh pr create --base main --head issue-7 --title "Add feature" --body "Closes #7" +"#; + +/// The scenario's repository's logs, relative to the scenario. +const LOGS: &str = "home/.thirdshift/logs/acme/widgets"; + +/// Bash that has the agent run `command` as Codex streams it, wrapped in its +/// shell, still running when its turn ends. +fn leaves_running(command: &str) -> String { + format!( + r#"echo '{{"type": "item.started", "item": {{"id": "item_9", "type": "command_execution", "command": "/bin/bash -lc '\''{command}'\''", "status": "in_progress"}}}}' +"# + ) +} + +/// Bash that has the agent run `command` to completion, as Codex streams it. +fn runs(command: &str) -> String { + format!( + r#"echo '{{"type": "item.started", "item": {{"id": "item_1", "type": "command_execution", "command": "{command}", "status": "in_progress"}}}}' +echo '{{"type": "item.completed", "item": {{"id": "item_1", "type": "command_execution", "command": "{command}", "status": "completed", "exit_code": 0}}}}' +"# + ) +} + +/// Bash that sets the check runs on the worktree's HEAD to `checks`. +fn checks_on_head(checks: &str) -> String { + format!("gh fake checks \"$(git rev-parse HEAD)\" '{checks}'\n") +} + +const RED: &str = + r#"[{"name": "test", "conclusion": "failure", "url": "https://ci.example/test"}]"#; +const GREEN: &str = + r#"[{"name": "test", "conclusion": "success", "url": "https://ci.example/test"}]"#; + +fn argv(call: &Value) -> Vec { + call["argv"] + .as_array() + .unwrap() + .iter() + .map(|arg| arg.as_str().unwrap().to_string()) + .collect() +} + +/// The arguments a Codex session on `model` and `effort` starts with, before +/// any `resume` and its prompt. +fn codex_args(model: &str, effort: &str) -> Vec { + [ + "exec", + "--json", + "--dangerously-bypass-approvals-and-sandbox", + "-m", + model, + "-c", + &format!("model_reasoning_effort=\"{effort}\""), + "-c", + "project_doc_fallback_filenames=[\"CLAUDE.md\"]", + ] + .map(String::from) + .to_vec() +} + +/// Assert every session ran on `codex exec` with the bypass flag, `model` +/// and `effort`, the `CLAUDE.md` fallback and stdin set to null, its prompt +/// last, and found every Factory skill in `.agents/skills/` with none of +/// them in `git status`; that its prompt's first line loads its skill with +/// Codex's sigil, if it loads one; and that none ran on `claude`. +fn assert_sessions_on_codex(scenario: &Scenario, model: &str, effort: &str) { + let calls = scenario.codex_calls(); + assert!(!calls.is_empty(), "no Codex session ran"); + assert!( + scenario.claude_calls().is_empty(), + "a session ran on claude" + ); + for call in &calls { + let args = argv(call); + let prompt = call["prompt"].as_str().unwrap(); + assert_eq!(args.last().map(String::as_str), Some(prompt)); + assert_eq!(args[..9], codex_args(model, effort), "{args:?}"); + assert_eq!(call["stdin_null"], true, "{args:?}"); + assert_eq!(call["git_status"], "", "{args:?}"); + assert!(!prompt.starts_with("/thirdshift-"), "{prompt}"); + } + scenario.assert_every_codex_session_found_the_factory_skills(); +} + +/// The first line of each Codex session's prompt. +fn first_lines(scenario: &Scenario) -> Vec { + scenario + .codex_calls() + .iter() + .map(|call| { + let prompt = call["prompt"].as_str().unwrap(); + prompt.lines().next().unwrap().to_string() + }) + .collect() +} + +#[test] +fn a_runs_implement_session_and_its_repair_run_on_codex_with_the_model_and_effort_as_codex_names_them() + { + let scenario = Scenario::new(); + scenario.agent_does_for_in_session( + 7, + 1, + &format!( + "{}{AGENT_OPENS_PR}{}", + runs("/bin/bash -lc 'cargo test'"), + checks_on_head(RED) + ), + ); + scenario.agent_does_for_in_session( + 7, + 2, + &format!( + "echo fix > fix.txt\ngit add fix.txt\ngit commit -q -m Fix\n{}", + checks_on_head(GREEN) + ), + ); + let url = scenario.issue_url(7); + + let result = scenario.run(&[ + "harness", + "codex", + "model", + "GPT-6.1-Sol", + &url, + "effort", + "Max", + ]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_sessions_on_codex(&scenario, "gpt-6.1-sol", "max"); + assert_eq!(scenario.codex_calls().len(), 2); + assert_eq!( + first_lines(&scenario)[0], + format!("$thirdshift-implement {url}") + ); + for line in [ + "thirdshift: checking the Model and Effort against codex debug models\n", + "thirdshift: sessions run on codex · gpt-6.1-sol · max\n", + "thirdshift: implement: session started\n", + "thirdshift: implement: $ cargo test\n", + ": 1200 input tokens (200 cached), 300 output tokens\n", + ] { + assert!( + result.stderr.contains(line), + "{line:?} in {}", + result.stderr + ); + } + let body = scenario.gh_state()["prs"][0]["body"] + .as_str() + .unwrap() + .to_string(); + assert!( + body.ends_with("Built with codex · gpt-6.1-sol · max \n"), + "{body}" + ); + let logs = scenario.entries(&format!("{LOGS}/sessions")); + let implement = logs + .iter() + .find(|log| log.ends_with("-implement.jsonl")) + .unwrap_or_else(|| panic!("no implement session log: {logs:?}")); + let log = fs::read_to_string(scenario.path(&format!("{LOGS}/sessions/{implement}"))).unwrap(); + assert!( + log.starts_with(r#"{"type": "thread.started", "thread_id": "fake-thread-1"}"#), + "{log}" + ); + assert!(log.contains(r#""type": "turn.completed""#), "{log}"); +} + +#[test] +fn a_session_left_with_a_command_running_is_resumed_on_its_thread_with_every_flag_again() { + let scenario = Scenario::new(); + scenario.agent_does_for_in_session( + 7, + 1, + &format!("{AGENT_OPENS_PR}{}", leaves_running("sleep 188")), + ); + let url = scenario.issue_url(7); + + let result = scenario.run(&[ + "harness", + "codex", + "model", + "gpt-6-luna", + "effort", + "high", + &url, + ]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_sessions_on_codex(&scenario, "gpt-6-luna", "high"); + let calls = scenario.codex_calls(); + assert_eq!(calls.len(), 2); + let resume = argv(&calls[1]); + assert_eq!(resume[9..11], ["resume", "fake-thread-1"], "{resume:?}"); + assert!( + calls[1]["prompt"] + .as_str() + .unwrap() + .starts_with("Your background work (sleep 188) was killed"), + "{resume:?}" + ); + assert!( + result.stderr.contains( + "implement: background work was killed as the session ended; resuming it once\n" + ), + "stderr: {}", + result.stderr + ); +} + +#[test] +fn a_failed_turn_or_a_non_zero_exit_fails_the_run_with_codexs_error_in_the_cause() { + for (script, cause) in [ + ( + "echo \"The 'gpt-6.1-sol' model is not supported.\" > \"$FAKE_CODEX_ERROR\"\nexit 1", + "codex exited 1: The 'gpt-6.1-sol' model is not supported.", + ), + ( + r#"echo '{"type": "turn.failed", "error": {"message": "stream disconnected"}}'"#, + "codex's turn failed: stream disconnected", + ), + ( + "echo '{\"type\": \"error\", \"message\": \"Reconnecting... 1/5\"}'\nkill -KILL $PPID", + "codex exited by signal: Reconnecting... 1/5", + ), + ] { + let scenario = Scenario::new(); + scenario.agent_does_for(7, script); + + let result = scenario.run(&["harness", "codex", &scenario.issue_url(7)]); + + assert_eq!(result.code, Some(1), "stderr: {}", result.stderr); + assert!( + result.stderr.contains(cause), + "{cause:?} in {}", + result.stderr + ); + assert_eq!(scenario.codex_calls().len(), 1); + } +} + +#[test] +fn a_retried_error_alone_does_not_fail_the_session() { + let scenario = Scenario::new(); + scenario.agent_does_for( + 7, + &format!( + "echo '{{\"type\": \"error\", \"message\": \"Reconnecting... 1/5\"}}'\n{AGENT_OPENS_PR}" + ), + ); + + let result = scenario.run(&["harness", "codex", &scenario.issue_url(7)]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert!( + !result.stderr.contains("checking the Model"), + "a Model was checked with none named: {}", + result.stderr + ); +} + +#[test] +fn a_merge_run_on_codex_merges_the_pr() { + let scenario = Scenario::new(); + scenario.agent_does_for(7, AGENT_OPENS_PR); + + let result = scenario.run(&["merge", "harness", "codex", &scenario.issue_url(7)]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_eq!(scenario.gh_state()["prs"][0]["state"], "MERGED"); + assert_eq!(scenario.codex_calls().len(), 1); + assert!(scenario.claude_calls().is_empty()); +} + +/// Assert the Run failed with `error` before any work: no Claim, no +/// worktree, no session and no Command log. +fn assert_failed_before_any_work(scenario: &Scenario, result: &RunResult, error: &str) { + assert_eq!(result.code, Some(1), "stderr: {}", result.stderr); + assert!( + result.stderr.contains(error), + "expected {error:?} in stderr: {}", + result.stderr + ); + assert!(scenario.codex_calls().is_empty(), "a session ran"); + assert_eq!(scenario.entries("work"), vec!["widgets"]); + assert_eq!( + scenario + .launch_git(&["worktree", "list", "--porcelain"]) + .matches("worktree ") + .count(), + 1 + ); + assert!(scenario.issue_labels(7).is_empty(), "the Claim was made"); + assert!( + !scenario.path(&format!("{LOGS}/commands")).exists(), + "a Command log was kept" + ); +} + +#[test] +fn a_model_or_effort_codex_lacks_fails_before_any_work_naming_the_valid_choices() { + for (model, effort, error) in [ + ( + "gpt-7", + "max", + "thirdshift: the Model gpt-7 is not in Codex's catalog: choose one of gpt-6.1-sol, \ + gpt-6-luna, gpt-5.5\n", + ), + ( + "GPT-5.5", + "Max", + "thirdshift: the Effort Max is not one the Codex Model gpt-5.5 supports: choose one \ + of low, medium, high, xhigh\n", + ), + ] { + let scenario = Scenario::new(); + scenario.agent_does_for(7, AGENT_OPENS_PR); + + let result = scenario.run(&[ + &scenario.issue_url(7), + "harness", + "codex", + "model", + model, + "effort", + effort, + ]); + + assert_failed_before_any_work(&scenario, &result, error); + } +} + +/// A script in which the agent for issue `issue` commits its work and opens +/// its PR into `base`. +fn agent_opens_pr(issue: u32, base: &str) -> String { + format!( + r#" +echo "{issue}" > issue-{issue}.txt +git add issue-{issue}.txt +git commit -q -m "Work on {issue}" +gh pr create --base {base} --head issue-{issue} --title "Work on {issue}" --body "Closes #{issue}" +"# + ) +} + +#[test] +fn an_architecture_review_and_the_spec_run_it_dispatches_run_every_session_on_codex() { + let scenario = Scenario::new(); + scenario.repo_has_labels(&["needs-triage", "ready-for-agent", "architecture"]); + scenario.agent_does_in_session( + 1, + r#" +spec=$(gh issue create --title "Deepen the session module" --body "The Spec" --label needs-triage) +gh issue create --title "Move the logs" --body "A Ticket" --label ready-for-agent +gh issue create --title "Move the sessions" --body "A Ticket" --label ready-for-agent +gh fake sub-issues 8 '[9, 10]' +printf 'Architecture review plan: %s\n' "$spec" > "$FAKE_CLAUDE_FINAL_MESSAGE" +"#, + ); + scenario.agent_does_for(9, &agent_opens_pr(9, "issue-8")); + scenario.agent_does_for(10, &agent_opens_pr(10, "issue-8")); + + let result = scenario.run(&["architect", "harness", "codex", "model", "GPT-6-Luna"]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + let gh = scenario.gh_state(); + let spec_pr = gh["prs"] + .as_array() + .unwrap() + .iter() + .find(|pr| pr["head"] == "issue-8") + .unwrap_or_else(|| panic!("no Spec PR: {gh}")); + assert_eq!(spec_pr["isDraft"], false); + let calls = scenario.codex_calls(); + assert_eq!(calls.len(), 4, "{calls:?}"); + for call in &calls { + let args = argv(call); + assert_eq!( + args[..7], + [ + "exec", + "--json", + "--dangerously-bypass-approvals-and-sandbox", + "-m", + "gpt-6-luna", + "-c", + "project_doc_fallback_filenames=[\"CLAUDE.md\"]", + ], + "{args:?}" + ); + assert_eq!(call["stdin_null"], true, "{args:?}"); + assert_eq!(call["git_status"], "", "{args:?}"); + } + assert!(scenario.claude_calls().is_empty()); + scenario.assert_every_codex_session_found_the_factory_skills(); + let mut first_lines = first_lines(&scenario); + assert_eq!( + first_lines.remove(0), + "$thirdshift-improve-codebase-architecture" + ); + assert!( + first_lines[2].starts_with("$thirdshift-code-review main, with the Spec "), + "{first_lines:?}" + ); + first_lines.pop(); + first_lines.sort(); + assert_eq!( + first_lines, + [ + "$thirdshift-implement https://github.com/acme/widgets/issues/10", + "$thirdshift-implement https://github.com/acme/widgets/issues/9", + ] + ); +} diff --git a/tests/fakes.rs b/tests/fakes.rs index eaa999dcc..0b197d18a 100644 --- a/tests/fakes.rs +++ b/tests/fakes.rs @@ -1,7 +1,7 @@ -//! The fake `gh` and `claude` the end-to-end tests put first on PATH: one -//! executable, which acts as whichever its name says. See `fakes/gh.rs` and -//! `fakes/claude.rs` for what each does, and `support/fakes.rs` for how the -//! harness builds it. +//! The fake `gh`, `claude` and `codex` the end-to-end tests put first on +//! PATH: one executable, which acts as whichever its name says. See +//! `fakes/gh.rs`, `fakes/claude.rs` and `fakes/codex.rs` for what each does, +//! and `support/fakes.rs` for how the harness builds it. //! //! It uses only the standard library, so the harness can build it with plain //! `rustc`. It started out as two Python scripts, whose startup cost was about @@ -15,6 +15,8 @@ #[path = "fakes/claude.rs"] mod claude; +#[path = "fakes/codex.rs"] +mod codex; #[path = "fakes/gh.rs"] mod gh; #[path = "fakes/json.rs"] @@ -107,6 +109,7 @@ fn main() { match name { "gh" => gh::main(args.collect()), "claude" => claude::main(args.collect()), + "codex" => codex::main(args.collect()), _ => die(&format!("fake: no fake is called {name}"), 2), } exit(0); diff --git a/tests/fakes/claude.rs b/tests/fakes/claude.rs index 684741aa5..a0b2d3d00 100644 --- a/tests/fakes/claude.rs +++ b/tests/fakes/claude.rs @@ -66,9 +66,10 @@ fn snapshot_into(files: &mut Json, root: &Path, dir: &Path) { } } -/// The `thirdshift-*` entries of the working directory's `.claude/skills/`. -fn linked_skills() -> Vec { - let Ok(entries) = fs::read_dir(SKILLS) else { +/// The `thirdshift-*` entries of the working directory's `skills`, its +/// project skills. +fn linked_skills(skills: &str) -> Vec { + let Ok(entries) = fs::read_dir(skills) else { return Vec::new(); }; entries @@ -82,22 +83,22 @@ fn linked_skills() -> Vec { } /// Every file of the `thirdshift-*` skills in the working directory's -/// `.claude/skills/`, through their links, by its path relative to -/// `.claude/skills/`, with its contents. -fn skills_snapshot() -> Json { +/// `skills`, through their links, by its path relative to `skills`, with its +/// contents. +pub fn skills_snapshot(skills: &str) -> Json { let mut files = object([]); - for skill in linked_skills() { - snapshot_into(&mut files, Path::new(SKILLS), &skill); + for skill in linked_skills(skills) { + snapshot_into(&mut files, Path::new(skills), &skill); } files } /// The files written out beside the skills the `thirdshift-*` links in the -/// working directory's `.claude/skills/` point to, such as their licence, by -/// name, with their contents. -fn beside_skills_snapshot() -> Json { +/// working directory's `skills` point to, such as their licence, by name, +/// with their contents. +pub fn beside_skills_snapshot(skills: &str) -> Json { let mut files = object([]); - let written = linked_skills() + let written = linked_skills(skills) .first() .and_then(|link| fs::read_link(link).ok()) .and_then(|target| target.parent().map(Path::to_owned)); @@ -135,7 +136,7 @@ fn prompt_of(record: &Json) -> &str { } /// The script for the session just recorded last in `records`. -fn script_for(records: &[Json]) -> PathBuf { +pub fn script_for(records: &[Json]) -> PathBuf { let script = crate::env_path("FAKE_CLAUDE_SCRIPT").display().to_string(); let mut candidates = Vec::new(); if let Some(issue) = issue_of(prompt_of(records.last().unwrap())) { @@ -156,7 +157,7 @@ fn script_for(records: &[Json]) -> PathBuf { /// `git` in the working directory, with its stdout trimmed, and whether it /// succeeded. -fn git_here(args: &[&str]) -> (String, bool) { +pub fn git_here(args: &[&str]) -> (String, bool) { let output = crate::git(Path::new("."), args); let stdout = String::from_utf8_lossy(&output.stdout).trim().to_owned(); (stdout, output.status.success()) @@ -164,7 +165,7 @@ fn git_here(args: &[&str]) -> (String, bool) { /// The exit code a Python fake gave for `status`, which is negative for a /// signal and wraps as an exit code. -fn exit_code(status: ExitStatus) -> i32 { +pub fn exit_code(status: ExitStatus) -> i32 { status .code() .unwrap_or_else(|| -status.signal().unwrap() & 0xff) @@ -195,8 +196,8 @@ pub fn main(argv: Vec) { ("cwd", string(cwd)), ("branch", string(branch)), ("merging", Bool(merging)), - ("skill_files", skills_snapshot()), - ("beside_skills", beside_skills_snapshot()), + ("skill_files", skills_snapshot(SKILLS)), + ("beside_skills", beside_skills_snapshot(SKILLS)), ("stdin", stdin), ]), ); diff --git a/tests/fakes/codex.rs b/tests/fakes/codex.rs new file mode 100644 index 000000000..7d317fd88 --- /dev/null +++ b/tests/fakes/codex.rs @@ -0,0 +1,158 @@ +//! Fake `codex` for the thirdshift test harness. +//! +//! `codex debug models` prints [`CATALOG`], a fixed catalog of Models. +//! +//! `codex exec --json …` is an agent session, scripted as the fake `claude`'s +//! are: it runs the same per-test bash script, chosen the same way from +//! $FAKE_CLAUDE_SCRIPT by the session's issue and count, with the sessions +//! counted from its own record. Each call appends a record to the JSON list +//! in $FAKE_CODEX_RECORD: its argv, prompt, working directory, the branch +//! checked out there, whether its stdin was `/dev/null`, `git status +//! --porcelain` there, a snapshot of the `thirdshift-*` skills in its +//! `.agents/skills/`, through their links, and of the files written out +//! beside them. +//! +//! It emits `thread.started`, with thread id `fake-thread-` for the n-th +//! call, or none if $FAKE_CLAUDE_NO_SESSION_ID is set, then `turn.started`, +//! then runs the script, whose stdout joins the stream. What the script +//! writes to $FAKE_CLAUDE_FINAL_MESSAGE becomes an `agent_message` item. A +//! script that succeeds ends the turn with `turn.completed`, and one that +//! fails with `turn.failed`, whose error is what it wrote to +//! $FAKE_CODEX_ERROR, else a default, and exits 1, as Codex does. + +use std::fs; +use std::io::Write; +use std::os::fd::AsFd; +use std::os::unix::fs::MetadataExt; +use std::process::Command; + +use crate::claude::{beside_skills_snapshot, exit_code, git_here, script_for, skills_snapshot}; +use crate::json::{Array, Bool, Json, Null, object, string}; + +/// Where Codex finds a worktree's project skills. +const SKILLS: &str = ".agents/skills"; + +/// What `codex debug models` prints: a few Models, each with its display +/// name and the Efforts it supports. +const CATALOG: &str = r#"{"models": [ + {"slug": "gpt-6.1-sol", "display_name": "GPT-6.1-Sol", "visibility": "list", "default_reasoning_level": "low", + "supported_reasoning_levels": [{"effort": "low"}, {"effort": "medium"}, {"effort": "high"}, {"effort": "xhigh"}, {"effort": "max"}, {"effort": "ultra"}]}, + {"slug": "gpt-6-luna", "display_name": "GPT-6-Luna", "visibility": "list", "default_reasoning_level": "medium", + "supported_reasoning_levels": [{"effort": "low"}, {"effort": "medium"}, {"effort": "high"}, {"effort": "xhigh"}, {"effort": "max"}]}, + {"slug": "gpt-5.5", "display_name": "GPT-5.5", "visibility": "list", "default_reasoning_level": "medium", + "supported_reasoning_levels": [{"effort": "low"}, {"effort": "medium"}, {"effort": "high"}, {"effort": "xhigh"}]} +]}"#; + +/// Whether stdin is `/dev/null`, by `fstat` on it, as macOS has no `/proc`. +fn stdin_is_null() -> bool { + let stdin = std::io::stdin() + .as_fd() + .try_clone_to_owned() + .map(fs::File::from) + .and_then(|file| file.metadata()); + let (Ok(stdin), Ok(null)) = (stdin, fs::metadata("/dev/null")) else { + return false; + }; + stdin.rdev() == null.rdev() && stdin.ino() == null.ino() +} + +/// Print one line of the stream. +fn emit(event: Json) { + println!("{event}"); + std::io::stdout().flush().unwrap(); +} + +pub fn main(argv: Vec) { + match argv.first().map(String::as_str) { + Some("debug") if argv.get(1).map(String::as_str) == Some("models") => { + println!("{CATALOG}"); + crate::exit(0); + } + Some("exec") if argv.iter().any(|arg| arg == "--json") => {} + _ => crate::die( + &format!( + "fake codex: unexpected arguments {}", + crate::python_list(&argv) + ), + 2, + ), + } + let (branch, _) = git_here(&["branch", "--show-current"]); + let (status, _) = git_here(&["status", "--porcelain"]); + let record_path = &crate::env_path("FAKE_CODEX_RECORD"); + + let lock = crate::lock_beside(record_path); + let cwd = std::env::current_dir().unwrap(); + let cwd = cwd.to_str().unwrap(); + let records = crate::append_record( + record_path, + object([ + ("argv", Array(argv.iter().map(string).collect())), + ("prompt", argv.last().map(string).unwrap_or(Null)), + ("cwd", string(cwd)), + ("branch", string(branch)), + ("stdin_null", Bool(stdin_is_null())), + ("git_status", string(status)), + ("skill_files", skills_snapshot(SKILLS)), + ("beside_skills", beside_skills_snapshot(SKILLS)), + ]), + ); + let records = records.items(); + let script = script_for(records); + drop(lock); + + let session = records.len(); + if std::env::var_os("FAKE_CLAUDE_NO_SESSION_ID").is_none() { + emit(object([ + ("type", string("thread.started")), + ("thread_id", string(format!("fake-thread-{session}"))), + ])); + } + emit(object([("type", string("turn.started"))])); + // Beside the record, not in $TMPDIR, which tests expect to be left empty. + let final_message = format!("{}.final-message.{session}", record_path.display()); + let error = format!("{}.error.{session}", record_path.display()); + let status = Command::new("bash") + .arg("-e") + .arg(&script) + .env("FAKE_CLAUDE_FINAL_MESSAGE", &final_message) + .env("FAKE_CODEX_ERROR", &error) + .status() + .unwrap(); + let code = exit_code(status); + if let Ok(message) = fs::read_to_string(&final_message) { + emit(object([ + ("type", string("item.completed")), + ( + "item", + object([ + ("id", string("item_final")), + ("type", string("agent_message")), + ("text", string(message)), + ]), + ), + ])); + } + if code == 0 { + emit(object([ + ("type", string("turn.completed")), + ( + "usage", + object([ + ("input_tokens", crate::json::number(1200)), + ("cached_input_tokens", crate::json::number(200)), + ("output_tokens", crate::json::number(300)), + ]), + ), + ])); + crate::exit(0); + } + let message = fs::read_to_string(&error) + .map(|message| message.trim_end().to_string()) + .unwrap_or_else(|_| format!("the agent's script exited {code}")); + emit(object([ + ("type", string("turn.failed")), + ("error", object([("message", string(message))])), + ])); + crate::exit(1); +} diff --git a/tests/harness_choice.rs b/tests/harness_choice.rs index 3ddc4677b..60cdca151 100644 --- a/tests/harness_choice.rs +++ b/tests/harness_choice.rs @@ -278,35 +278,6 @@ fn a_harness_missing_from_path_fails_the_run_before_any_work_naming_what_chose_i } } -#[test] -fn harness_codex_fails_the_run_before_any_work_as_not_supported_yet() { - for (args, config, chosen_by) in [ - (vec!["harness", "codex"], None, "the harness flag"), - ( - vec![], - Some("[harness]\ndefault = \"codex\"\n"), - "harness.default in the User config", - ), - ] { - let scenario = scenario(); - if let Some(config) = config { - scenario.user_config_is(config); - } - let url = scenario.issue_url(7); - let mut args = args; - args.push(&url); - - let result = scenario.run(&args); - - assert_failed_before_any_work( - &scenario, - &result, - &format!("harness codex, chosen by {chosen_by}, is not supported yet"), - ); - assert!(scenario.claude_calls().is_empty()); - } -} - #[test] fn the_command_log_the_activity_log_and_the_pr_body_name_the_harness_model_and_effort() { let scenario = scenario(); diff --git a/tests/support/fakes.rs b/tests/support/fakes.rs index 6a506d892..b851c6b75 100644 --- a/tests/support/fakes.rs +++ b/tests/support/fakes.rs @@ -1,4 +1,4 @@ -//! The fake `gh` and `claude`, built from `tests/fakes.rs` with plain +//! The fake `gh`, `claude` and `codex`, built from `tests/fakes.rs` with plain //! `rustc` the first time a test needs them and kept in Cargo's scratch //! directory for tests. They aren't a binary target, so neither the crate nor //! a release ships them. @@ -11,16 +11,17 @@ use std::sync::OnceLock; /// The fakes' sources. They name the built executable, and including them /// makes Cargo rebuild the tests when they change. -const SOURCES: [&str; 4] = [ +const SOURCES: [&str; 5] = [ include_str!("../fakes.rs"), include_str!("../fakes/gh.rs"), include_str!("../fakes/claude.rs"), + include_str!("../fakes/codex.rs"), include_str!("../fakes/json.rs"), ]; -/// Put the fake `gh` and `claude` in `bin`. +/// Put the fake `gh`, `claude` and `codex` in `bin`. pub fn install(bin: &Path) { - for name in ["gh", "claude"] { + for name in ["gh", "claude", "codex"] { std::os::unix::fs::symlink(executable(), bin.join(name)).unwrap(); } } diff --git a/tests/support/mod.rs b/tests/support/mod.rs index d3afd916b..f593337b0 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -1,5 +1,5 @@ //! Test harness: runs the compiled `thirdshift` binary against real git and -//! fake `gh` and `claude` executables, and, for email, a stand-in for Resend +//! fake `gh`, `claude` and `codex` executables, and, for email, a stand-in for Resend //! ([`resend::ResendStandIn`]). //! //! Layout of a scenario's temp root: @@ -13,7 +13,7 @@ //! home/.thirdshift/ the User config, config.toml, if the test writes one //! home/.config/ $XDG_CONFIG_HOME, where an install receipt would be //! installed/ a copy of thirdshift, if the test runs one to replace it -//! bin/ fake gh and claude, first on PATH +//! bin/ fake gh, claude and codex, first on PATH //! tmp/ $TMPDIR, so leftover temp directories are visible //! work// the launch clone, origin https://github.com//.git //! gh-state.json fake GitHub state @@ -27,6 +27,11 @@ //! claude-calls.json.after-result. stream lines the n-th session emits //! after its closing result //! claude-calls.json.final-message. the n-th session's final message +//! codex-calls.json what the fake agent was asked to do on Codex, whose +//! sessions take the same scripts +//! codex-calls.json.final-message., codex-calls.json.error. the n-th +//! Codex session's final message, and the error it +//! failed with //! gh-calls.json every gh command run, by thirdshift or the fake agent //! ``` @@ -597,6 +602,7 @@ test -f {root}/{COPY_REPLACED} .env("FAKE_GH_STATE", self.path("gh-state.json")) .env("FAKE_CLAUDE_SCRIPT", self.path("claude-script.sh")) .env("FAKE_CLAUDE_RECORD", self.path("claude-calls.json")) + .env("FAKE_CODEX_RECORD", self.path("codex-calls.json")) .env("FAKE_GH_RECORD", self.path("gh-calls.json")) // Seconds of waiting for CI become milliseconds. Each poll starts // the fake gh; at 100ms the grace period holds about three reads, @@ -801,22 +807,24 @@ test -f {root}/{COPY_REPLACED} } } + /// Every call the fake agent received on Codex, in order. + pub fn codex_calls(&self) -> Vec { + match fs::read_to_string(self.path("codex-calls.json")) { + Ok(text) => serde_json::from_str(&text).unwrap(), + Err(_) => Vec::new(), + } + } + /// Assert every `claude` call found every Factory skill, by its /// `thirdshift-` name, in its worktree's `.claude/skills/`. pub fn assert_every_session_found_the_factory_skills(&self) { - let calls = self.claude_calls(); - assert!(!calls.is_empty(), "no session ran"); - for call in calls { - for skill in FACTORY_SKILLS { - let skill_md = call["skill_files"][format!("{skill}/SKILL.md")].as_str(); - assert!( - skill_md.is_some_and(|text| text.contains(&format!("name: {skill}\n"))), - "the session in {} with the prompt {} found no {skill}", - call["cwd"], - call["prompt"] - ); - } - } + assert_found_the_factory_skills(self.claude_calls()); + } + + /// Assert every `codex` call found every Factory skill, by its + /// `thirdshift-` name, in its worktree's `.agents/skills/`. + pub fn assert_every_codex_session_found_the_factory_skills(&self) { + assert_found_the_factory_skills(self.codex_calls()); } /// The prompt of the first `claude` call. @@ -1080,3 +1088,20 @@ fn git_output(dir: &Path, args: &[&str]) -> Output { .output() .unwrap() } + +/// Assert each of `calls` found every Factory skill, by its +/// `thirdshift-` name, in its worktree's project skills. +fn assert_found_the_factory_skills(calls: Vec) { + assert!(!calls.is_empty(), "no session ran"); + for call in calls { + for skill in FACTORY_SKILLS { + let skill_md = call["skill_files"][format!("{skill}/SKILL.md")].as_str(); + assert!( + skill_md.is_some_and(|text| text.contains(&format!("name: {skill}\n"))), + "the session in {} with the prompt {} found no {skill}", + call["cwd"], + call["prompt"] + ); + } + } +}