diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index a4ccd84..326dff2 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -105,8 +105,12 @@ never stops the loop. ## Persistence and durable execution One database: `~/.bullpen/bullpen.db`, WAL mode, `busy_timeout` set, schema -versioned by `pragma user_version`. Session ids resolve by unique prefix. -`BULLPEN_HOME` overrides the directory (see README, "Where state lives"). +versioned by `pragma user_version` (v6). Session ids resolve by unique +prefix. `BULLPEN_HOME` overrides the directory (see README, "Where state +lives"). v6 added `sessions.worktree_path` / `worktree_branch`, both NULL +for a session that shares the caller's checkout; a session's `cwd` stays the +directory it was dispatched from, which is what still points at the +repository when the worktree itself is gone. The durability rule, the reduction idea, and the recovery discipline below are adapted from pi's `harness-v2.md` design spec — see @@ -265,6 +269,10 @@ Milestones, in order. Each lands as its own crate or a bounded extension: derived state (Working = running + live pid, Failed = running + dead pid i.e. crashed, Completed, Idle), dispatches from an input line, and peeks a session's latest output. `bullpen logs ` tails captured output. + `--bg --worktree` (opt-in) additionally gives a session its own git + worktree on a run-unique branch, so concurrent background sessions stop + editing each other's files; the location is recorded on the session row + and wins over the caller's cwd on resume (see "Worktree retention" below). Stage 2 (committed): interactive attach to and detach from a *live* process, leaving it running (needs a per-session control socket), needs-input state (needs the approvals feature), notifications. Stage 2+ @@ -336,3 +344,48 @@ that is still the default. Linux has no out-of-process confinement yet (Landlock is the intended mechanism; the in-process write check already works there). Do not point v0 at anything you wouldn't hand to a contractor's laptop. + +### Worktree retention (fail-closed) + +`--worktree` isolates a background session in its own git worktree. Nothing +in bullpen removes that worktree or its branch — not when the run completes, +not when it fails, not on any later invocation. The asymmetry is the whole +argument: a leftover directory costs disk, while an eager cleanup can +destroy the only copy of what an agent did — an uncommitted diff, an +interrupted rebase, a file it wrote but never mentioned. Uncertainty +retains. Even the resume path that restores a deleted worktree uses +`git worktree add --force` rather than pruning the stale entry, because +pruning is a removal. + +That constrains any future `bullpen prune`: it has to earn deletion from +proof that the work was published — the branch is merged or pushed, the +worktree is clean — rather than inherit an optimistic rule like age or +session status. A `completed` session is not evidence its diff was kept. + +The resume rule follows the same posture. The recorded location beats the +directory the command was typed in; a missing directory whose branch +survives is recreated from that branch with one stderr notice; a missing +directory *and* missing branch is an error naming both, never a silent run +somewhere else. A directory that exists but is not that worktree — an +ordinary one restored at the path, a worktree of another repository — is a +third error rather than a fourth place to run, so the recorded path is +checked against the session's repository, not merely stat'ed. +`--worktree` outside a git repository is likewise an error, because falling +back to the shared checkout would silently reintroduce exactly the +interference the flag exists to remove. For the same reason the path and +branch are written to the session row *before* `git worktree add` runs: a +row pointing at a directory that failed to appear is refused on resume, +while a row pointing at nothing would read as a plain shared-cwd session. + +`--sandbox` has to be widened to compose with this. A linked worktree's +`.git` is a file; its index, refs and objects all live under the *main* +repository, outside the worktree, so a sandbox confined to the worktree +alone leaves an agent able to edit files and unable to stage or commit +them — which under the rule above is fatal, since a commit is the only +evidence that would ever justify reclaiming its directory. The run therefore +adds the worktree's git dirs to the write roots. That is the whole shared +common directory, because git's ref store cannot be sliced by path — `git +gc` prunes `refs/heads/bullpen/` and moves the refs into a `packed-refs` +file at the top of it — so a sandboxed agent in a worktree can also write +the main repository's config and hooks. Confining that needs a mechanism +other than a path allowlist. diff --git a/Cargo.lock b/Cargo.lock index 97d8719..28624a1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -151,6 +151,7 @@ dependencies = [ "ratatui", "reqwest", "serde_json", + "tempfile", "tokio", "tracing-subscriber", ] diff --git a/README.md b/README.md index fba7287..661e766 100644 --- a/README.md +++ b/README.md @@ -83,6 +83,7 @@ bullpen run -r 6ee4acc9 "now write the fix" ```bash bullpen run --bg "audit the auth module" # detached, returns immediately +bullpen run --bg --worktree "refactor it" # …in its own git worktree bullpen agents # the dashboard in the GIF above bullpen logs 6ee4acc9 # tail a background session ``` @@ -92,6 +93,16 @@ bullpen logs 6ee4acc9 # tail a background session lets you dispatch from the input line, `Space` to peek at output, `Esc` to quit. Quitting stops nothing. +Plain `--bg` sessions share your checkout, so two of them edit the same +files. `--worktree` gives a session a git worktree of its own on a +run-unique `bullpen/` branch, under `$BULLPEN_HOME/worktrees/`; +the path shows up in `bullpen sessions`, in `bullpen sessions --json`, and +in the peek panel, and `bullpen run -r ` returns to it from anywhere. Outside a +git repository the flag fails rather than quietly sharing the checkout. +**Nothing removes a worktree or its branch** — not on success, not on +failure, not later. A worktree can hold the only copy of what an agent did, +so cleaning up is yours to decide. + ## The pen The model can delegate bounded work to child agents through the `agent` @@ -130,8 +141,9 @@ can't be opened read-only without their shared-memory file: sqlite3 "file:${BULLPEN_HOME:-$HOME/.bullpen}/bullpen.db?immutable=1" "select id, status from sessions" ``` -Set `BULLPEN_HOME` to move the whole directory — database, `auth.json`, and -background logs land directly in it, with no `.bullpen` segment appended. +Set `BULLPEN_HOME` to move the whole directory — database, `auth.json`, +background logs, and `--worktree` checkouts (`worktrees/`) land +directly in it, with no `.bullpen` segment appended. ## Status diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index 2d73399..8b02cec 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -25,3 +25,6 @@ crossterm.workspace = true libc.workspace = true tokio.workspace = true tracing-subscriber.workspace = true + +[dev-dependencies] +tempfile = "3" diff --git a/crates/cli/src/agents.rs b/crates/cli/src/agents.rs index 87dc196..1f79749 100644 --- a/crates/cli/src/agents.rs +++ b/crates/cli/src/agents.rs @@ -302,34 +302,19 @@ fn draw_peek(f: &mut Frame, app: &App) { let area = centered(70, 60, f.area()); f.render_widget(Clear, area); - let mut lines = vec![ - Line::from(Span::styled( - format!("{} ({})", &session.id[..8], session.provider), - Style::default().add_modifier(Modifier::BOLD), - )), - Line::from(""), - ]; // Latest assistant text from the durable transcript. - match Store::open(&Store::default_path()).and_then(|s| s.path_messages(&session.id)) { - Ok(messages) => { - let latest = messages + let latest = + match Store::open(&Store::default_path()).and_then(|s| s.path_messages(&session.id)) { + Ok(messages) => messages .iter() .rev() .find(|m| m.role == bullpen_llm::Role::Assistant) .map(|m| m.text()) .filter(|t| !t.is_empty()) - .unwrap_or_else(|| "(no output yet)".into()); - for line in latest.lines() { - lines.push(Line::from(line.to_string())); - } - } - Err(e) => lines.push(Line::from(format!("(could not read transcript: {e})"))), - } - lines.push(Line::from("")); - lines.push(Line::from(Span::styled( - format!("continue with: bullpen run -r {} \"…\"", &session.id[..8]), - Style::default().fg(Color::DarkGray), - ))); + .unwrap_or_else(|| "(no output yet)".into()), + Err(e) => format!("(could not read transcript: {e})"), + }; + let lines = peek_lines(session, &latest); let block = Block::default() .borders(Borders::ALL) @@ -342,6 +327,34 @@ fn draw_peek(f: &mut Frame, app: &App) { ); } +/// The peek panel's contents. Pure — the transcript read happens in the +/// caller — so what the panel says about a session can be tested without a +/// terminal or a store. +fn peek_lines(session: &Session, latest: &str) -> Vec> { + let mut lines = vec![Line::from(Span::styled( + format!("{} ({})", &session.id[..8], session.provider), + Style::default().add_modifier(Modifier::BOLD), + ))]; + // An isolated session's output is not in the directory the dashboard was + // started from, so the panel has to say where it is. + if let Some(path) = &session.worktree_path { + lines.push(Line::from(Span::styled( + format!("worktree {path}"), + Style::default().fg(Color::DarkGray), + ))); + } + lines.push(Line::from("")); + for line in latest.lines() { + lines.push(Line::from(line.to_string())); + } + lines.push(Line::from("")); + lines.push(Line::from(Span::styled( + format!("continue with: bullpen run -r {} \"…\"", &session.id[..8]), + Style::default().fg(Color::DarkGray), + ))); + lines +} + fn status_color(status: AgentStatus) -> Color { match status { AgentStatus::Working => Color::Cyan, @@ -389,11 +402,20 @@ mod tests { parent_session_id: None, status: "idle".into(), pid: None, + worktree_path: None, + worktree_branch: None, }, status, } } + fn rendered(lines: &[Line<'static>]) -> Vec { + lines + .iter() + .map(|l| l.spans.iter().map(|s| s.content.as_ref()).collect()) + .collect() + } + #[test] fn arrange_groups_working_first_then_newest() { let rows = vec![ @@ -414,4 +436,19 @@ mod tests { ] ); } + + #[test] + fn peek_lines_shows_the_worktree_for_an_isolated_session() { + let mut r = row("0123456789ab", AgentStatus::Working, "2026-08-07 10:00"); + r.session.worktree_path = Some("/h/.bullpen/worktrees/0123456789ab".into()); + let lines = rendered(&peek_lines(&r.session, "output")); + assert_eq!(lines[1], "worktree /h/.bullpen/worktrees/0123456789ab"); + } + + #[test] + fn peek_lines_says_nothing_about_worktrees_for_a_shared_cwd_session() { + let r = row("0123456789ab", AgentStatus::Working, "2026-08-07 10:00"); + let lines = rendered(&peek_lines(&r.session, "output")); + assert!(!lines.iter().any(|l| l.contains("worktree")), "{lines:?}"); + } } diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index 3b1e97f..e7e5b3a 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -7,6 +7,7 @@ mod agents; mod bg; mod json; +mod worktree; use std::sync::Arc; @@ -66,6 +67,12 @@ enum Command { /// Watch it with `bullpen agents`. #[arg(long)] bg: bool, + /// Give the background session its own git worktree on its own + /// branch, so concurrent runs cannot edit each other's files. + /// Requires --bg; not compatible with --resume. The worktree is + /// never removed automatically. + #[arg(long)] + worktree: bool, }, /// Dispatch and monitor background sessions from one screen. Agents, @@ -183,6 +190,7 @@ async fn main() -> anyhow::Result<()> { sandbox, sandbox_strict, bg, + worktree, } => { run( prompt, @@ -194,6 +202,7 @@ async fn main() -> anyhow::Result<()> { sandbox, sandbox_strict, bg, + worktree, ) .await } @@ -339,12 +348,31 @@ async fn run( sandbox: bool, sandbox_strict: bool, bg: bool, + worktree: bool, ) -> anyhow::Result<()> { + if worktree && !bg { + bail!("--worktree applies to background dispatch only; add --bg"); + } + if worktree && resume.is_some() { + bail!( + "--worktree creates a worktree for a new session; a resumed one \ + already has its location recorded" + ); + } let cwd = std::env::current_dir()?; // Background dispatch: create/resolve the session here so we have an id to // hand back and to spawn a detached child against, then return. if bg { + // Resolve the repository before any state exists. A directory that + // is not a repository must leave behind no session row, no log file + // and no detached process — not a session pointing at a worktree + // that was never created. + let repo_root = if worktree { + Some(worktree::repo_root(&cwd)?) + } else { + None + }; let store = Store::open(&Store::default_path())?; let session = match &resume { Some(prefix) => store.resolve_session(prefix)?, @@ -359,6 +387,21 @@ async fn run( } else if sandbox { extra.push("--sandbox".to_string()); } + let worktree_at = match &repo_root { + Some(root) => { + let path = worktree::worktree_path(&session.id); + let branch = worktree::branch_for(&session.id); + // Recorded before it is created, not after: if `git worktree + // add` fails the row then names a directory that does not + // exist, which resume refuses outright. A row naming nothing + // would instead resume into the shared checkout — the bug + // this flag exists to close. + store.set_worktree(&session.id, &path.display().to_string(), &branch)?; + worktree::create(root, &path, &branch)?; + Some(path) + } + None => None, + }; let pid = bg::spawn_detached(&session.id, &prompt, &extra)?; store.set_run_status(&session.id, "running", Some(pid as i64))?; if json { @@ -366,6 +409,12 @@ async fn run( } else { println!("dispatched {} (pid {pid})", &session.id[..8]); } + if let Some(path) = &worktree_at { + eprintln!( + "[worktree {} — never removed automatically]", + path.display() + ); + } eprintln!( " bullpen agents watch it\n bullpen logs {} tail its output\n bullpen run -r {} \"...\" continue it", &session.id[..8], @@ -374,20 +423,10 @@ async fn run( return Ok(()); } - // Build the write-confinement sandbox, if requested. - let sandbox = if sandbox_strict { - Some(Arc::new(bullpen_sandbox::Sandbox::strict(&cwd))) - } else if sandbox { - Some(Arc::new(bullpen_sandbox::Sandbox::workspace(&cwd))) - } else { - None - }; - if sandbox.is_some() && !bullpen_sandbox::Sandbox::os_enforced() { - eprintln!( - "[sandbox: OS-level shell confinement is macOS-only; on this platform \ - only the file-editing tools are confined, not arbitrary shell commands]" - ); - } + // The session is resolved before the sandbox is built: an isolated + // session's working directory is recorded on its row, and it is that + // directory the sandbox must confine — not the one the caller happened + // to type the command in. let mut store = Store::open(&Store::default_path())?; let (session, provider_kind, model) = match &resume { @@ -406,6 +445,56 @@ async fn run( } }; + // A recorded worktree wins over the caller's cwd. This is the only + // mechanism that places a run: the detached background child is a + // `run --resume` and arrives here too, as does a resume typed by hand + // from anywhere. + let cwd = match worktree::locate( + session.worktree_path.as_deref(), + session.worktree_branch.as_deref(), + std::path::Path::new(&session.cwd), + ) { + worktree::Location::Shared => cwd, + worktree::Location::Use(path) => path, + worktree::Location::Recreate { path, branch } => { + let root = worktree::repo_root(std::path::Path::new(&session.cwd))?; + worktree::recreate(&root, &path, &branch)?; + eprintln!("[recreated worktree {} from {branch}]", path.display()); + path + } + worktree::Location::Fail { path, branch } => bail!( + "session {} is recorded in worktree {} on branch {branch}, and \ + neither survives — nothing was removed by bullpen, so restore \ + them (or start a new session) rather than running somewhere else", + &session.id[..8], + path.display() + ), + worktree::Location::Occupied { path, branch } => bail!( + "session {} is recorded in worktree {}, but what stands there is \ + not that worktree — move it aside and restore the session's own \ + from branch {branch} (or start a new session) rather than \ + running somewhere else", + &session.id[..8], + path.display() + ), + }; + + // Build the write-confinement sandbox, if requested. + let sandbox = (sandbox || sandbox_strict).then(|| { + let base = if sandbox_strict { + bullpen_sandbox::Sandbox::strict(&cwd) + } else { + bullpen_sandbox::Sandbox::workspace(&cwd) + }; + Arc::new(base.allowing_writes(worktree::git_write_roots(&cwd))) + }); + if sandbox.is_some() && !bullpen_sandbox::Sandbox::os_enforced() { + eprintln!( + "[sandbox: OS-level shell confinement is macOS-only; on this platform \ + only the file-editing tools are confined, not arbitrary shell commands]" + ); + } + // Recover any run a previous process left open, then rebuild the // transcript from the durable entry tree. let (transcript, recovery) = bullpen_harness::prepare_session(&mut store, &session.id)?; @@ -598,6 +687,8 @@ fn sessions_json(sessions: &[Session]) -> serde_json::Value { "parent_session_id": s.parent_session_id, "status": s.status, "pid": s.pid, + "worktree_path": s.worktree_path, + "worktree_branch": s.worktree_branch, }) }) .collect() @@ -622,8 +713,12 @@ fn sessions(json: bool) -> anyhow::Result<()> { Some(parent) => format!(" └ child of {}", &parent[..8]), None => String::new(), }; + let worktree_marker = match &s.worktree_path { + Some(path) => format!(" [worktree {path}]"), + None => String::new(), + }; println!( - "{} {} {:<10} {:>6}/{:<6} {}{}", + "{} {} {:<10} {:>6}/{:<6} {}{}{}", &s.id[..8], s.updated_at, s.provider, @@ -635,6 +730,7 @@ fn sessions(json: bool) -> anyhow::Result<()> { &s.title }, child_marker, + worktree_marker, ); } Ok(()) @@ -671,6 +767,8 @@ mod tests { parent_session_id: parent.map(|p| p.to_string()), status: "idle".into(), pid, + worktree_path: None, + worktree_branch: None, } } @@ -696,10 +794,32 @@ mod tests { "parent_session_id": null, "status": "idle", "pid": 4242, + "worktree_path": null, + "worktree_branch": null, }]) ); } + #[test] + fn worktree_keys_are_null_for_shared_cwd_sessions() { + let rows = sessions_json(&[session("shared", None, None)]); + assert_eq!(rows[0]["worktree_path"], json!(null)); + assert_eq!(rows[0]["worktree_branch"], json!(null)); + } + + #[test] + fn worktree_keys_carry_the_location_for_an_isolated_session() { + let mut s = session("isolated", None, None); + s.worktree_path = Some("/h/.bullpen/worktrees/isolated".into()); + s.worktree_branch = Some("bullpen/isolated".into()); + let rows = sessions_json(&[s]); + assert_eq!( + rows[0]["worktree_path"], + json!("/h/.bullpen/worktrees/isolated") + ); + assert_eq!(rows[0]["worktree_branch"], json!("bullpen/isolated")); + } + #[test] fn parent_session_id_is_null_for_top_level_and_a_string_for_children() { let rows = sessions_json(&[ diff --git a/crates/cli/src/worktree.rs b/crates/cli/src/worktree.rs new file mode 100644 index 0000000..6fe6b83 --- /dev/null +++ b/crates/cli/src/worktree.rs @@ -0,0 +1,504 @@ +//! Per-session git worktrees for `--bg --worktree`. +//! +//! Two background sessions sharing one checkout edit each other's files. +//! Giving each its own worktree on its own branch removes that entirely, at +//! the cost of a directory that has to be found again later — which is why +//! the path and branch are recorded in the store rather than recomputed. +//! +//! **This module creates worktrees and never removes one.** No exit path +//! here deletes a directory or a branch, because a worktree may hold the +//! only copy of an agent's work: an uncommitted diff, a half-finished +//! rebase, a file the agent wrote but never mentioned. Uncertainty retains. +//! A leftover directory costs disk; an eager cleanup costs the work itself. +//! +//! Split like [`bullpen_store::home_dir`]: pure derivation (path, branch +//! name, and the resume decision table) separated from a thin `git` adapter, +//! so the decisions are testable without a repository. git is shelled out to +//! rather than linked, matching how `bullpen-tools` runs bash and grep. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +use anyhow::bail; + +/// Where a background session's isolated checkout lives. Under +/// `$BULLPEN_HOME` rather than inside the repository: the worktree is +/// bullpen's state, keyed to a session id, and moving the database without +/// it would split the two apart. +pub fn worktree_path(session_id: &str) -> PathBuf { + worktree_dir(&bullpen_store::home_dir(), session_id) +} + +fn worktree_dir(home: &Path, session_id: &str) -> PathBuf { + home.join("worktrees").join(session_id) +} + +/// The branch a session's worktree checks out. The whole session id, not the +/// short prefix the CLI prints: a truncated id is 32 bits, and two sessions +/// that collided would name one branch, which git refuses to check out in two +/// worktrees at once — the second dispatch would simply fail. +pub fn branch_for(session_id: &str) -> String { + format!("bullpen/{session_id}") +} + +/// The repository containing `cwd`. +/// +/// Fails when there isn't one. `--worktree` never degrades to the shared +/// working directory: silently doing that is exactly the behaviour the flag +/// exists to avoid. +pub fn repo_root(cwd: &Path) -> anyhow::Result { + let out = Command::new("git") + .arg("-C") + .arg(cwd) + .args(["rev-parse", "--show-toplevel"]) + .output(); + let out = match out { + Ok(out) => out, + Err(e) => bail!("--worktree needs the git binary, but running it failed: {e}"), + }; + if !out.status.success() { + bail!( + "{} is not inside a git repository, so --worktree has nothing to \ + branch from — it will not fall back to running in the shared \ + working directory", + cwd.display() + ); + } + Ok(PathBuf::from( + String::from_utf8_lossy(&out.stdout).trim().to_string(), + )) +} + +/// Check out a new worktree at `path` on a new `branch` off the current HEAD. +pub fn create(root: &Path, path: &Path, branch: &str) -> anyhow::Result<()> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + git_worktree_add(root, &["-b", branch], path, "HEAD") +} + +/// Restore a recorded worktree whose directory is gone but whose branch +/// survives. `--force` is required, not optional: git's administrative entry +/// for the deleted directory still claims both the path and the branch. +/// Pruning that entry would be a removal path, which this module does not have. +pub fn recreate(root: &Path, path: &Path, branch: &str) -> anyhow::Result<()> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + git_worktree_add(root, &["--force"], path, branch) +} + +fn git_worktree_add( + root: &Path, + flags: &[&str], + path: &Path, + commit_ish: &str, +) -> anyhow::Result<()> { + let out = Command::new("git") + .arg("-C") + .arg(root) + .args(["worktree", "add"]) + .args(flags) + .arg(path) + .arg(commit_ish) + .output(); + let out = match out { + Ok(out) => out, + Err(e) => bail!("could not run git worktree add: {e}"), + }; + if !out.status.success() { + bail!( + "git worktree add failed for {}:\n{}", + path.display(), + String::from_utf8_lossy(&out.stderr).trim() + ); + } + Ok(()) +} + +/// The git directories a run in `cwd` must be able to write to, beyond `cwd` +/// itself. Empty for an ordinary checkout, whose `.git` is already inside the +/// workspace; for a linked worktree the index, refs and objects all live under +/// the main repository, so a sandbox confined to the worktree alone would let +/// an agent edit files but never stage or commit them — and a commit is the +/// only evidence that would ever justify reclaiming its directory. +/// +/// The whole common directory, not the session's own ref and objects: git's +/// ref store is shared mutable state that a path allowlist cannot slice. Once +/// `git gc` packs refs it prunes `refs/heads/bullpen/`, so the next commit has +/// to recreate that directory under `refs/heads` — and `packed-refs` itself +/// sits at the top of the common directory. Granting either grants every +/// branch. The residual risk is real and unavoidable here: a sandboxed agent +/// in a worktree can write the main repository's config and hooks. +pub fn git_write_roots(cwd: &Path) -> Vec { + let (Some(git_dir), Some(common_dir)) = ( + rev_parse_dir(cwd, "--absolute-git-dir"), + rev_parse_dir(cwd, "--git-common-dir"), + ) else { + return Vec::new(); + }; + if git_dir == common_dir { + return Vec::new(); + } + vec![git_dir, common_dir] +} + +/// `git rev-parse `, resolved to an absolute path. `--git-common-dir` +/// answers relative to `cwd` in an ordinary checkout and absolutely in a +/// linked worktree, so both forms have to be accepted before the two can be +/// compared. +fn rev_parse_dir(cwd: &Path, flag: &str) -> Option { + let out = Command::new("git") + .arg("-C") + .arg(cwd) + .args(["rev-parse", flag]) + .output() + .ok()?; + if !out.status.success() { + return None; + } + let dir = PathBuf::from(String::from_utf8_lossy(&out.stdout).trim().to_string()); + let dir = if dir.is_absolute() { + dir + } else { + cwd.join(dir) + }; + Some(dir.canonicalize().unwrap_or(dir)) +} + +pub fn branch_exists(root: &Path, branch: &str) -> bool { + Command::new("git") + .arg("-C") + .arg(root) + .args(["rev-parse", "--verify", &format!("refs/heads/{branch}")]) + .output() + .is_ok_and(|out| out.status.success()) +} + +/// Where a run should happen, given what the session recorded. +#[derive(Debug, PartialEq, Eq)] +pub enum Location { + /// No worktree was ever recorded: the caller's working directory. + Shared, + Use(PathBuf), + Recreate { + path: PathBuf, + branch: String, + }, + Fail { + path: PathBuf, + branch: String, + }, + /// Something that is not this session's worktree sits at the recorded + /// path. + Occupied { + path: PathBuf, + branch: String, + }, +} + +/// What stands at a session's recorded worktree path. +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +pub enum Candidate { + /// A live worktree of the session's own repository. + Live, + /// Nothing — the directory is gone. + Gone, + /// A directory, but not that worktree: an ordinary one restored at the + /// path, or a worktree of a different repository. + Foreign, +} + +/// The resume decision, as a table. git and the filesystem supply only the +/// [`Candidate`] and the branch bit, so every row is testable without either. +/// +/// A recorded worktree always wins over the caller's working directory: a +/// `bullpen run -r ` typed from somewhere else must not quietly append +/// to the session in a different tree than the one it has been editing. By +/// the same rule a foreign directory is refused outright rather than run in +/// or overwritten — either would be the silent misplacement this avoids. +pub fn decide( + recorded: Option<(&str, &str)>, + candidate: Candidate, + branch_exists: bool, +) -> Location { + let Some((path, branch)) = recorded else { + return Location::Shared; + }; + let path = PathBuf::from(path); + let branch = branch.to_string(); + match (candidate, branch_exists) { + (Candidate::Live, _) => Location::Use(path), + (Candidate::Foreign, _) => Location::Occupied { path, branch }, + (Candidate::Gone, true) => Location::Recreate { path, branch }, + (Candidate::Gone, false) => Location::Fail { path, branch }, + } +} + +/// The impure half of [`decide`]: answers its two inputs. `anchor` is the +/// session's recorded cwd — the directory it was dispatched from, which is +/// what still points at the repository once the worktree itself is gone. +pub fn locate( + recorded_path: Option<&str>, + recorded_branch: Option<&str>, + anchor: &Path, +) -> Location { + let Some((path, branch)) = recorded_path.zip(recorded_branch) else { + return Location::Shared; + }; + let candidate = inspect(anchor, Path::new(path)); + let branch_lives = candidate == Candidate::Gone + && repo_root(anchor).is_ok_and(|root| branch_exists(&root, branch)); + decide(Some((path, branch)), candidate, branch_lives) +} + +/// Whether `path` really is a worktree of the repository `anchor` sits in. +/// `is_dir` alone would accept an ordinary directory left at the recorded +/// path — running there is the silent misplacement the recording exists to +/// prevent — so both the shared git directory and the worktree's own top +/// level have to agree. +fn inspect(anchor: &Path, path: &Path) -> Candidate { + if !path.is_dir() { + return Candidate::Gone; + } + let same_repo = rev_parse_dir(path, "--git-common-dir") + .zip(rev_parse_dir(anchor, "--git-common-dir")) + .is_some_and(|(candidate, anchor)| candidate == anchor); + let is_top_level = repo_root(path).is_ok_and(|top| canonical(&top) == canonical(path)); + if same_repo && is_top_level { + Candidate::Live + } else { + Candidate::Foreign + } +} + +fn canonical(path: &Path) -> PathBuf { + path.canonicalize().unwrap_or_else(|_| path.to_path_buf()) +} + +#[cfg(test)] +mod tests { + use super::*; + + const ID: &str = "0123456789abcdef0123456789abcdef"; + + /// A repository at `root` with one committed file. + fn init_repo(root: &Path) { + std::fs::create_dir_all(root).unwrap(); + let git = |args: &[&str]| { + let out = Command::new("git") + .arg("-C") + .arg(root) + // CI runners have no global identity, and the commit below + // needs one. + .args(["-c", "user.email=t@example.com", "-c", "user.name=t"]) + .args(args) + .output() + .unwrap(); + assert!(out.status.success(), "git {args:?}: {out:?}"); + }; + git(&["init"]); + std::fs::write(root.join("f.txt"), "x").unwrap(); + git(&["add", "f.txt"]); + git(&["commit", "-m", "init"]); + } + + #[test] + fn branch_name_carries_the_whole_session_id_not_the_short_prefix() { + assert_eq!(branch_for(ID), format!("bullpen/{ID}")); + assert_ne!( + branch_for("0123456789abcdef00000000deadbeef"), + branch_for("0123456789abcdef11111111deadbeef") + ); + } + + #[test] + fn worktree_dir_lives_under_bullpen_home_not_the_repo() { + assert_eq!( + worktree_dir(Path::new("/h/.bullpen"), ID), + PathBuf::from("/h/.bullpen/worktrees/0123456789abcdef0123456789abcdef") + ); + } + + #[test] + fn decide_falls_back_to_the_shared_cwd_only_when_nothing_was_recorded() { + assert_eq!(decide(None, Candidate::Gone, false), Location::Shared); + } + + #[test] + fn decide_uses_the_recorded_directory_when_it_is_still_that_worktree() { + assert_eq!( + decide(Some(("/w/a", "bullpen/a")), Candidate::Live, false), + Location::Use(PathBuf::from("/w/a")) + ); + } + + #[test] + fn decide_recreates_from_the_branch_when_only_the_directory_is_gone() { + assert_eq!( + decide(Some(("/w/a", "bullpen/a")), Candidate::Gone, true), + Location::Recreate { + path: PathBuf::from("/w/a"), + branch: "bullpen/a".into(), + } + ); + } + + #[test] + fn missing_branch_fails_rather_than_falling_back_to_the_callers_cwd() { + assert_eq!( + decide(Some(("/w/a", "bullpen/a")), Candidate::Gone, false), + Location::Fail { + path: PathBuf::from("/w/a"), + branch: "bullpen/a".into(), + } + ); + } + + #[test] + fn a_foreign_directory_at_the_recorded_path_is_refused_even_with_the_branch() { + assert_eq!( + decide(Some(("/w/a", "bullpen/a")), Candidate::Foreign, true), + Location::Occupied { + path: PathBuf::from("/w/a"), + branch: "bullpen/a".into(), + } + ); + } + + #[test] + fn non_repo_error_names_the_directory_and_refuses_to_degrade() { + let dir = tempfile::tempdir().unwrap(); + let err = repo_root(dir.path()).unwrap_err().to_string(); + assert!(err.contains(&dir.path().display().to_string()), "{err}"); + assert!(err.contains("not fall back"), "{err}"); + } + + /// The tests below really shell out: the `--force` recreate path and the + /// layout of a linked worktree's git dirs are claims about git's + /// behaviour, not about ours, and asserting them against a stub would + /// prove nothing. + #[test] + fn create_then_recreate_restores_a_deleted_worktree_on_its_branch() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("repo"); + init_repo(&root); + + let path = dir.path().join("worktrees").join("s1"); + create(&root, &path, "bullpen/s1").unwrap(); + assert!(path.join("f.txt").is_file()); + assert!(branch_exists(&root, "bullpen/s1")); + // macOS resolves TempDir under /private/var, so compare canonical forms. + assert_eq!( + repo_root(&path).unwrap().canonicalize().unwrap(), + path.canonicalize().unwrap() + ); + + std::fs::remove_dir_all(&path).unwrap(); + recreate(&root, &path, "bullpen/s1").unwrap(); + assert!(path.join("f.txt").is_file()); + } + + #[test] + fn locate_accepts_only_a_real_worktree_of_the_sessions_own_repository() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("repo"); + init_repo(&root); + let path = dir.path().join("worktrees").join("s1"); + create(&root, &path, "bullpen/s1").unwrap(); + let recorded = path.display().to_string(); + let at = |anchor: &Path| locate(Some(&recorded), Some("bullpen/s1"), anchor); + + assert_eq!(at(&root), Location::Use(path.clone())); + + // A worktree of some other repository, at the same recorded path. + let other = dir.path().join("other"); + init_repo(&other); + assert_eq!( + at(&other), + Location::Occupied { + path: path.clone(), + branch: "bullpen/s1".into(), + } + ); + + // An ordinary directory restored where the worktree used to be: the + // branch still exists, and it is still refused rather than run in. + std::fs::remove_dir_all(&path).unwrap(); + std::fs::create_dir_all(path.join("f.txt")).unwrap(); + assert_eq!( + at(&root), + Location::Occupied { + path: path.clone(), + branch: "bullpen/s1".into(), + } + ); + + std::fs::remove_dir_all(&path).unwrap(); + assert_eq!( + at(&root), + Location::Recreate { + path, + branch: "bullpen/s1".into(), + } + ); + } + + #[test] + fn an_ordinary_checkout_needs_no_write_roots_beyond_itself() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("repo"); + init_repo(&root); + assert!(git_write_roots(&root).is_empty()); + // Nor does a directory that is not a repository at all. + assert!(git_write_roots(dir.path()).is_empty()); + } + + #[test] + fn a_linked_worktree_needs_the_main_repositorys_git_dirs() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("repo"); + init_repo(&root); + let path = dir.path().join("worktrees").join("s1"); + create(&root, &path, "bullpen/s1").unwrap(); + + let common = root.join(".git").canonicalize().unwrap(); + assert_eq!( + git_write_roots(&path), + vec![common.join("worktrees").join("s1"), common] + ); + } + + /// Write roots alone do not prove an agent can publish its work; only + /// running git under the real generated profile does. macOS-only because + /// that is the platform where the profile is enforced. + #[test] + #[cfg(target_os = "macos")] + fn a_sandboxed_worktree_can_still_commit() { + // Under $HOME rather than a tempdir: the sandbox deliberately allows + // writes anywhere under the system temp roots, so a tempdir repo + // would pass whatever the write roots said. + let base = std::env::home_dir() + .unwrap() + .join(".bullpen-worktree-sandbox-test"); + let _ = std::fs::remove_dir_all(&base); + let root = base.join("repo"); + init_repo(&root); + let path = base.join("worktrees").join("s1"); + create(&root, &path, "bullpen/s1").unwrap(); + + let sandbox = + bullpen_sandbox::Sandbox::workspace(&path).allowing_writes(git_write_roots(&path)); + let (prog, args) = sandbox + .wrap_bash("git -c user.email=t@example.com -c user.name=t commit -qam sandboxed"); + std::fs::write(path.join("f.txt"), "edited").unwrap(); + let out = Command::new(prog) + .args(args) + .current_dir(&path) + .output() + .unwrap(); + assert!(out.status.success(), "{out:?}"); + + std::fs::remove_dir_all(&base).unwrap(); + } +} diff --git a/crates/sandbox/src/lib.rs b/crates/sandbox/src/lib.rs index 32ffdf6..7930d41 100644 --- a/crates/sandbox/src/lib.rs +++ b/crates/sandbox/src/lib.rs @@ -59,6 +59,16 @@ impl Sandbox { Self { caps } } + /// Widen the write confinement to further subtrees. Some workspaces have + /// machinery outside themselves that the agent legitimately writes + /// through — a linked git worktree's index and object store being the + /// case that forced this; the caller names those, since the sandbox has + /// no business knowing what a worktree is. + pub fn allowing_writes(mut self, roots: impl IntoIterator) -> Self { + self.caps.write_roots.extend(roots); + self + } + pub fn capabilities(&self) -> &Capabilities { &self.caps } diff --git a/crates/store/src/lib.rs b/crates/store/src/lib.rs index 60bd844..1f57893 100644 --- a/crates/store/src/lib.rs +++ b/crates/store/src/lib.rs @@ -1,6 +1,6 @@ //! Durable local state. //! -//! One SQLite database in WAL mode. Schema v3 implements the durable +//! One SQLite database in WAL mode. Schema v6 implements the durable //! execution model from ARCHITECTURE.md ("Persistence and durable //! execution"): //! @@ -62,6 +62,12 @@ pub struct Session { pub status: String, /// OS process id of the last run, for liveness checks. pub pid: Option, + /// Set when the session runs in its own git worktree; the directory the + /// run happens in, which outlives the process that created it. + pub worktree_path: Option, + /// Set alongside `worktree_path`; the branch that worktree checks out. + /// It is what a lost directory can be restored from. + pub worktree_branch: Option, } /// One conversation entry. `payload` holds a [`Message`] for kind @@ -227,6 +233,15 @@ impl Store { )?; tx.commit()?; } + if version < 6 { + let tx = self.conn.transaction()?; + tx.execute_batch( + "ALTER TABLE sessions ADD COLUMN worktree_path TEXT; + ALTER TABLE sessions ADD COLUMN worktree_branch TEXT; + PRAGMA user_version = 6;", + )?; + tx.commit()?; + } Ok(()) } @@ -321,11 +336,29 @@ impl Store { Ok(()) } + /// Record where an isolated session runs. Written before the worktree is + /// created, so a failed creation leaves a row naming a directory that is + /// not there — which resume refuses — rather than a row naming none, + /// which resume would read as an ordinary shared-cwd session. + pub fn set_worktree( + &self, + session_id: &str, + path: &str, + branch: &str, + ) -> Result<(), StoreError> { + self.conn.execute( + "UPDATE sessions SET worktree_path = ?2, worktree_branch = ?3 WHERE id = ?1", + params![session_id, path, branch], + )?; + Ok(()) + } + pub fn get_session(&self, id: &str) -> Result { self.conn .query_row( "SELECT id, title, cwd, provider, model, input_tokens, output_tokens, - created_at, updated_at, parent_session_id, status, pid + created_at, updated_at, parent_session_id, status, pid, + worktree_path, worktree_branch FROM sessions WHERE id = ?1", params![id], row_to_session, @@ -339,7 +372,8 @@ impl Store { pub fn resolve_session(&self, prefix: &str) -> Result { let mut stmt = self.conn.prepare( "SELECT id, title, cwd, provider, model, input_tokens, output_tokens, - created_at, updated_at, parent_session_id, status, pid + created_at, updated_at, parent_session_id, status, pid, + worktree_path, worktree_branch FROM sessions WHERE id LIKE ?1 || '%' LIMIT 2", )?; let mut matches: Vec = stmt @@ -355,7 +389,8 @@ impl Store { pub fn list_sessions(&self) -> Result, StoreError> { let mut stmt = self.conn.prepare( "SELECT id, title, cwd, provider, model, input_tokens, output_tokens, - created_at, updated_at, parent_session_id, status, pid + created_at, updated_at, parent_session_id, status, pid, + worktree_path, worktree_branch FROM sessions ORDER BY updated_at DESC", )?; Ok(stmt @@ -712,6 +747,8 @@ fn row_to_session(row: &rusqlite::Row<'_>) -> rusqlite::Result { parent_session_id: row.get(9)?, status: row.get(10)?, pid: row.get(11)?, + worktree_path: row.get(12)?, + worktree_branch: row.get(13)?, }) } @@ -914,4 +951,65 @@ mod tests { assert_eq!(messages[1].role, Role::Assistant); assert!(store.open_run("s1").unwrap().is_none()); } + + #[test] + fn migrates_v5_sessions_adding_worktree_columns() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("v5.db"); + // Build a v5 database by hand. Only `sessions` matters here: the v6 + // migration touches nothing else. + { + let conn = Connection::open(&path).unwrap(); + conn.execute_batch( + "CREATE TABLE sessions ( + id TEXT PRIMARY KEY, title TEXT NOT NULL DEFAULT '', + cwd TEXT NOT NULL, model TEXT NOT NULL, + input_tokens INTEGER NOT NULL DEFAULT 0, + output_tokens INTEGER NOT NULL DEFAULT 0, + created_at TEXT NOT NULL DEFAULT (datetime('now')), + updated_at TEXT NOT NULL DEFAULT (datetime('now')), + provider TEXT NOT NULL DEFAULT 'anthropic', + next_seq INTEGER NOT NULL DEFAULT 1, + parent_session_id TEXT, + status TEXT NOT NULL DEFAULT 'idle', + pid INTEGER + ); + INSERT INTO sessions (id, cwd, model) VALUES ('s1', '/tmp', 'm'); + PRAGMA user_version = 5;", + ) + .unwrap(); + } + + let store = Store::open(&path).unwrap(); + let session = store.get_session("s1").unwrap(); + // Sessions that predate the feature are shared-cwd sessions. + assert_eq!(session.worktree_path, None); + assert_eq!(session.worktree_branch, None); + } + + #[test] + fn set_worktree_round_trips_through_list_and_resolve() { + let (_dir, store) = store(); + let s = store.create_session("/repo", "anthropic", "m").unwrap(); + assert_eq!(s.worktree_path, None); + + store + .set_worktree(&s.id, "/h/.bullpen/worktrees/s1", "bullpen/abcd1234") + .unwrap(); + + for found in [ + store.get_session(&s.id).unwrap(), + store.resolve_session(&s.id[..8]).unwrap(), + store.list_sessions().unwrap().remove(0), + ] { + assert_eq!( + found.worktree_path.as_deref(), + Some("/h/.bullpen/worktrees/s1") + ); + assert_eq!(found.worktree_branch.as_deref(), Some("bullpen/abcd1234")); + // The dispatch anchor is untouched: it is what still points at + // the repository once the worktree directory is gone. + assert_eq!(found.cwd, "/repo"); + } + } }