Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,8 @@ For each of the three, the command wins, then the [User config](#user-config), t

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-<skill>`. `codex exec` kills what the agent left running as it exits, so a session whose turn ends with a command or a sub-agent call still running (started, never completed) gets a Resume, as on Claude: `codex exec … resume <session id>`, with the Model, Effort, bypass flag and `CLAUDE.md` fallback given again, as Codex keeps none of them. An interrupt sends a Codex session SIGINT, the only signal Codex stops cleanly on, before the SIGTERM and SIGKILL every session gets, each 10 seconds apart. 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.

Interrupting a Command stops every session's process tree on every Harness, including commands that started their own process group or session. thirdshift snapshots the descendants with `ps` on Linux and macOS, sends each process group the Harness's stop signals with a 10-second grace period each, then takes another snapshot before SIGKILL to catch commands started in the meantime. If the CLI exits on an earlier signal, thirdshift kills the remaining commands immediately without another grace period.

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 <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.
Expand Down
1 change: 1 addition & 0 deletions src/harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use anyhow::{Result, bail};
mod adapter;
pub(crate) mod claude;
pub(crate) mod codex;
mod process_tree;

pub use adapter::{Adapter, Invocation};
pub use codex::Catalog;
Expand Down
7 changes: 7 additions & 0 deletions src/harness/adapter.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Everything that varies between Harnesses, behind one interface.

use std::path::Path;
use std::process::Child;

use anyhow::Result;

Expand Down Expand Up @@ -30,6 +31,12 @@ pub trait Adapter: Sync {
) -> Result<Option<ModelAndEffort>>;
fn stream(&self, worktree: &Path) -> Box<dyn Stream>;

/// Stop the session and its descendants, including commands that made
/// their own process group or session, with this Harness's stop signals.
fn stop(&self, child: &mut Child) {
super::process_tree::stop(child, self.stop_signals());
}

/// Fixed overrides, also used by the adapter's Model and Effort check.
fn environment(&self) -> &'static [(&'static str, &'static str)] {
&[]
Expand Down
110 changes: 110 additions & 0 deletions src/harness/process_tree.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
//! The shared stop path for every Harness, including escaped process groups.

use std::collections::{BTreeSet, HashMap};
use std::process::{Child, Command};
use std::thread;
use std::time::{Duration, Instant};

use anyhow::{Context, Result, bail};

use crate::progress;

const POLL: Duration = Duration::from_millis(100);
const STOP_GRACE: Duration = Duration::from_secs(10);

/// Give each of the Harness's stop signals the existing grace period. Once
/// the CLI exits, skip the remaining grace periods but still kill its work.
pub(super) fn stop(child: &mut Child, signals: &[libc::c_int]) {
let pid = child.id() as libc::pid_t;
let mut tree = ProcessTree {
pids: BTreeSet::from([pid]),
groups: BTreeSet::from([pid]),
};
tree.refresh();
'signals: for &signal in signals {
tree.signal(signal);
let deadline = Instant::now() + STOP_GRACE;
while Instant::now() < deadline {
if let Ok(Some(_)) = child.try_wait() {
break 'signals;
}
thread::sleep(POLL);
}
}
// Retain the first snapshot: the CLI may already have exited, leaving
// its descendants reparented. Known descendants also seed this lookup
// so their new children are found even when the CLI is gone.
tree.refresh();
tree.signal(libc::SIGKILL);
let _ = child.wait();
}

struct ProcessTree {
pids: BTreeSet<libc::pid_t>,
groups: BTreeSet<libc::pid_t>,
}

impl ProcessTree {
fn refresh(&mut self) {
if let Err(error) = self.snapshot() {
progress::step(format_args!(
"could not snapshot the session's process tree: {error:#}; \
stopping its known process groups"
));
}
}

fn snapshot(&mut self) -> Result<()> {
// Separate -o flags and empty headers work on Linux and macOS.
let output = Command::new("ps")
.args(["-A", "-o", "pid=", "-o", "ppid=", "-o", "pgid="])
.output()
.context("could not run ps")?;
if !output.status.success() {
bail!(
"ps exited {}: {}",
output.status,
String::from_utf8_lossy(&output.stderr).trim()
);
}
let mut children = HashMap::<_, Vec<_>>::new();
let mut groups = HashMap::new();
for line in String::from_utf8_lossy(&output.stdout).lines() {
let mut fields = line.split_whitespace();
let mut next = || -> Result<libc::pid_t> {
fields
.next()
.context("missing ps field")?
.parse()
.context("invalid ps field")
};
let (pid, parent, group) = (next()?, next()?, next()?);
children.entry(parent).or_default().push(pid);
groups.insert(pid, group);
}
let mut pending: Vec<_> = self.pids.iter().copied().collect();
let mut seen = BTreeSet::new();
while let Some(pid) = pending.pop() {
if !seen.insert(pid) {
continue;
}
self.pids.insert(pid);
if let Some(&group) = groups.get(&pid)
&& group > 0
{
self.groups.insert(group);
}
if let Some(children) = children.get(&pid) {
pending.extend(children);
}
}
Ok(())
}

fn signal(&self, signal: libc::c_int) {
for &group in &self.groups {
// SAFETY: kill has no memory-safety preconditions; groups are positive.
unsafe { libc::kill(-group, signal) };
}
}
}
32 changes: 4 additions & 28 deletions src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::fs::{self, File};
use std::io::{BufRead, BufReader, Read, Write};
use std::os::unix::process::CommandExt;
use std::path::{Path, PathBuf};
use std::process::{Child, Command, Stdio};
use std::process::{Command, Stdio};
use std::thread;
use std::time::{Duration, Instant};

Expand Down Expand Up @@ -272,7 +272,7 @@ fn ending_with(killed: &[&str]) -> String {
/// `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, as [`stop`] does,
/// stream gave, if any. An interrupt stops the session, as [`Adapter::stop`] does,
/// and fails with `interrupted`.
fn run(
kind: &str,
Expand Down Expand Up @@ -332,13 +332,13 @@ fn run(
&& let Some(mut stdin) = child.stdin.take()
&& let Err(error) = stdin.write_all(input.as_bytes())
{
stop(&mut child, adapter);
adapter.stop(&mut child);
let _ = follower.join();
return Err(error).with_context(|| format!("could not write the prompt to {cli}"));
}
let status = loop {
if interrupt::requested() {
stop(&mut child, adapter);
adapter.stop(&mut child);
bail!("interrupted");
}
if let Some(status) = child
Expand Down Expand Up @@ -383,30 +383,6 @@ fn run(

const POLL: Duration = Duration::from_millis(100);

/// How long a session gets to exit after each signal that asks it to stop.
const STOP_GRACE: Duration = Duration::from_secs(10);

/// Stop `child`, a session on `adapter`, by its process group: with the
/// signals that ask it to stop, in turn, each given `STOP_GRACE`,
/// then SIGKILL.
fn stop(child: &mut Child, adapter: &dyn Adapter) {
let group = -(child.id() as libc::pid_t);
for &signal in adapter.stop_signals() {
// SAFETY: kill has no memory-safety preconditions.
unsafe { libc::kill(group, signal) };
let deadline = Instant::now() + STOP_GRACE;
while Instant::now() < deadline {
if let Ok(Some(_)) = child.try_wait() {
return;
}
thread::sleep(POLL);
}
}
// SAFETY: as above.
unsafe { libc::kill(group, libc::SIGKILL) };
let _ = child.wait();
}

/// Copy every line of `stream` to `log_file` and print the progress lines it
/// condenses to, until the stream ends.
fn follow(
Expand Down
20 changes: 20 additions & 0 deletions tests/fakes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ mod json;

use std::fs::{self, File};
use std::io::Write;
use std::os::unix::process::CommandExt;
use std::path::{Path, PathBuf};
use std::process::{Command, Output};

Expand Down Expand Up @@ -110,6 +111,25 @@ fn main() {
"gh" => gh::main(args.collect()),
"claude" => claude::main(args.collect()),
"codex" => codex::main(args.collect()),
"detached-command" => {
let command = args.next().expect("no detached command");
let mut command = Command::new(command);
command.args(args);
// SAFETY: setsid is async-signal-safe and accesses no Rust state.
unsafe {
command.pre_exec(|| {
unsafe extern "C" {
fn setsid() -> i32;
}
if setsid() == -1 {
return Err(std::io::Error::last_os_error());
}
Ok(())
});
}
let error = command.exec();
die(&format!("could not start detached command: {error}"), 1);
}
_ => die(&format!("fake: no fake is called {name}"), 2),
}
exit(0);
Expand Down
110 changes: 110 additions & 0 deletions tests/process_tree.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
//! Interrupting a Run stops commands that escaped the session's process group.

mod support;

use std::fs;
use std::thread;
use std::time::{Duration, Instant};

use support::Scenario;

/// Clean up by the recorded pid even when the regression assertion fails.
struct DetachedCommand(libc::pid_t);

impl DetachedCommand {
fn exists(&self) -> bool {
// SAFETY: kill has no memory-safety preconditions; signal 0 only probes.
unsafe { libc::kill(self.0, 0) == 0 }
}

fn assert_stopped(&self, message: &str) {
let deadline = Instant::now() + Duration::from_secs(2);
while self.exists() && Instant::now() < deadline {
thread::sleep(Duration::from_millis(10));
}
assert!(!self.exists(), "{message}");
}
}

impl Drop for DetachedCommand {
fn drop(&mut self) {
if self.exists() {
// SAFETY: the recorded pid is this command's process group leader.
unsafe { libc::kill(-self.0, libc::SIGKILL) };
}
}
}

#[test]
fn interrupting_a_claude_run_stops_a_command_in_its_own_session() {
assert_detached_command_stopped("claude");
}

#[test]
fn interrupting_a_codex_run_stops_a_command_in_its_own_session() {
assert_detached_command_stopped("codex");
}

fn assert_detached_command_stopped(harness: &str) {
let scenario = Scenario::new();
let pid = scenario.path("detached-pid");
scenario.agent_does(&format!(
r#"detached-command bash -c 'trap "" INT TERM; echo $$ > "{pid}"; exec sleep 120' >/dev/null 2>&1 &
while ! test -s "{pid}"; do sleep 0.01; done
touch "{started}"
while :; do sleep 0.1; done
"#,
pid = pid.display(),
started = scenario.path("agent-started").display(),
));

let mut held = scenario.run_until(
&["harness", harness, &scenario.issue_url(7)],
&[],
"agent-started",
);
let command = DetachedCommand(fs::read_to_string(pid).unwrap().trim().parse().unwrap());
assert!(command.exists(), "the detached command never started");
let interrupted_at = Instant::now();
held.signal("INT");
let result = held.finish();

assert_eq!(result.code, Some(1), "stderr: {}", result.stderr);
assert!(
interrupted_at.elapsed() < Duration::from_secs(8),
"a session that exited on its first signal incurred another grace period"
);
command.assert_stopped("the detached command survived interruption");
}

#[test]
fn a_command_started_during_the_stop_grace_period_is_killed_too() {
let scenario = Scenario::new();
let pid = scenario.path("late-pid");
let signals = scenario.path("signals");
scenario.agent_does(&format!(
r#"on_int() {{
echo INT >> "{signals}"
detached-command bash -c 'trap "" INT TERM; echo $$ > "{pid}"; exec sleep 120' >/dev/null 2>&1 &
}}
trap on_int INT
trap 'echo TERM >> "{signals}"' TERM
touch "{started}"
while :; do sleep 0.1 || :; done
"#,
pid = pid.display(),
signals = signals.display(),
started = scenario.path("agent-started").display(),
));

let result = scenario.run_and_signal(
&["harness", "codex", &scenario.issue_url(7)],
"agent-started",
"INT",
);
let command = DetachedCommand(fs::read_to_string(pid).unwrap().trim().parse().unwrap());

assert_eq!(result.code, Some(1), "stderr: {}", result.stderr);
assert_eq!(fs::read_to_string(signals).unwrap(), "INT\nTERM\n");
command.assert_stopped("the command started during the grace period survived");
}
2 changes: 1 addition & 1 deletion tests/support/fakes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const SOURCES: [&str; 5] = [

/// Put the fake `gh`, `claude` and `codex` in `bin`.
pub fn install(bin: &Path) {
for name in ["gh", "claude", "codex"] {
for name in ["gh", "claude", "codex", "detached-command"] {
std::os::unix::fs::symlink(executable(), bin.join(name)).unwrap();
}
}
Expand Down
Loading