mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-30 23:14:37 +00:00
This commit is contained in:
@@ -91,6 +91,25 @@ export interface SubagentActivity {
|
||||
* tool sequence. Absent on legacy/test rows that predate streaming.
|
||||
*/
|
||||
transcript?: SubagentTranscriptItem[];
|
||||
/**
|
||||
* Absolute path to this worker's isolated `git worktree` checkout, when it
|
||||
* ran with `isolation = "worktree"` (#3376). `undefined` for non-isolated
|
||||
* (read-only or shared-workspace) workers. Scaffold-only: the open/diff/
|
||||
* remove action buttons that consume this land in a follow-up PR.
|
||||
*/
|
||||
worktreePath?: string;
|
||||
/**
|
||||
* Files (relative to the worktree root) this worker changed, collected from
|
||||
* `git status` after the run. Drives the future diff/overlap UI. Absent or
|
||||
* empty for non-isolated workers and clean worktrees.
|
||||
*/
|
||||
changedFiles?: string[];
|
||||
/**
|
||||
* `true` when the worker's worktree had uncommitted changes after the run.
|
||||
* A dirty worktree must not be auto-removed — the cleanup UI will require an
|
||||
* explicit user choice. `undefined` for non-isolated workers.
|
||||
*/
|
||||
isDirty?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -44,6 +44,7 @@ mod token_budget;
|
||||
pub(crate) mod tool_filter;
|
||||
mod tool_loop;
|
||||
pub(crate) mod tool_result_artifacts;
|
||||
pub mod worktree_context;
|
||||
|
||||
pub use definition::{
|
||||
AgentDefinition, AgentDefinitionRegistry, DefinitionSource, ModelSpec, PromptSource,
|
||||
@@ -55,6 +56,7 @@ pub use model_vision_context::{current_model_vision, with_current_model_vision};
|
||||
pub use sandbox_context::{current_sandbox_mode, with_current_sandbox_mode};
|
||||
pub(crate) use spawn_depth_context::{current_spawn_depth, with_spawn_depth, MAX_SPAWN_DEPTH};
|
||||
pub use subagent_runner::{run_subagent, SubagentRunError, SubagentRunOptions};
|
||||
pub use worktree_context::{current_action_dir_override, with_action_dir_override};
|
||||
|
||||
pub(crate) use instructions::build_tool_instructions_filtered;
|
||||
pub(crate) use parse::parse_tool_calls;
|
||||
|
||||
@@ -102,17 +102,36 @@ pub async fn run_subagent(
|
||||
|
||||
// Install the sub-agent's declared `sandbox_mode` as the active
|
||||
// task-local for every tool invocation inside this run.
|
||||
//
|
||||
// When the worker opted into git-worktree isolation, also install
|
||||
// its isolated checkout path as the `action_dir` override so acting
|
||||
// tools (shell, git) operate inside that worktree instead of the
|
||||
// shared `Config.action_dir`. When `worktree_action_dir` is `None`
|
||||
// (the default / non-isolated path), no override scope is entered and
|
||||
// behaviour is unchanged.
|
||||
let worktree_action_dir = options.worktree_action_dir.clone();
|
||||
if let Some(ref wt_dir) = worktree_action_dir {
|
||||
tracing::debug!(
|
||||
agent_id = %definition.id,
|
||||
task_id = %task_id,
|
||||
worktree = %wt_dir.display(),
|
||||
"[subagent_runner] installing worktree action_dir override"
|
||||
);
|
||||
}
|
||||
let mut outcome = with_spawn_depth(attempted_depth, async {
|
||||
with_file_state_agent_id(task_id.clone(), async {
|
||||
with_current_sandbox_mode(definition.sandbox_mode, async {
|
||||
Box::pin(run_typed_mode(
|
||||
definition,
|
||||
task_prompt,
|
||||
&options,
|
||||
&parent,
|
||||
&task_id,
|
||||
))
|
||||
.await
|
||||
let run = run_typed_mode(definition, task_prompt, &options, &parent, &task_id);
|
||||
match worktree_action_dir {
|
||||
Some(wt_dir) => {
|
||||
crate::openhuman::agent::harness::with_action_dir_override(
|
||||
wt_dir,
|
||||
Box::pin(run),
|
||||
)
|
||||
.await
|
||||
}
|
||||
None => Box::pin(run).await,
|
||||
}
|
||||
})
|
||||
.await
|
||||
})
|
||||
|
||||
@@ -479,6 +479,7 @@ async fn typed_mode_returns_text_through_runner() {
|
||||
worker_thread_id: None,
|
||||
initial_history: None,
|
||||
checkpoint_dir: None,
|
||||
worktree_action_dir: None,
|
||||
},
|
||||
)
|
||||
.await
|
||||
@@ -590,6 +591,7 @@ async fn typed_mode_filters_tools_by_skill_filter() {
|
||||
worker_thread_id: None,
|
||||
initial_history: None,
|
||||
checkpoint_dir: None,
|
||||
worktree_action_dir: None,
|
||||
},
|
||||
)
|
||||
.await
|
||||
|
||||
@@ -56,6 +56,16 @@ pub struct SubagentRunOptions {
|
||||
/// sub-agent pauses for user input. Defaults to
|
||||
/// `{workspace_dir}/.openhuman/subagent_checkpoints/`.
|
||||
pub checkpoint_dir: Option<PathBuf>,
|
||||
|
||||
/// Per-worker `action_dir` override for git-worktree isolation.
|
||||
///
|
||||
/// When `Some`, the runner installs this path as the
|
||||
/// `current_action_dir_override` task-local around the inner tool-call
|
||||
/// loop, so acting tools (shell, git) operate inside the worker's
|
||||
/// isolated worktree checkout instead of the shared `Config.action_dir`.
|
||||
/// When `None` (the default), behaviour is unchanged — tools fall through
|
||||
/// to `security.action_dir`.
|
||||
pub worktree_action_dir: Option<PathBuf>,
|
||||
}
|
||||
|
||||
/// Terminal status of a sub-agent run.
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
//! Task-local carrier for a **per-worker `action_dir` override**.
|
||||
//!
|
||||
//! Sibling of [`super::sandbox_context`] and [`super::fork_context`]. When an
|
||||
//! edit-capable worker opts into git-worktree isolation
|
||||
//! (`isolation = "worktree"`), the subagent runner installs the worktree's
|
||||
//! checkout path here for the duration of that worker's run. Acting tools
|
||||
//! (shell, git) read it via [`current_action_dir_override`] and, when present,
|
||||
//! redirect their working directory to the isolated worktree instead of the
|
||||
//! shared `Config.action_dir`.
|
||||
//!
|
||||
//! Why a task-local instead of rebuilding every tool with a new `action_dir`:
|
||||
//! the subagent runner reuses the parent's already-constructed tool instances
|
||||
//! (`parent.all_tools`), each of which captured `security.action_dir` at build
|
||||
//! time. Threading a fresh `action_dir` into those instances would mean
|
||||
//! reconstructing the entire tool set per worker. A task-local keeps the change
|
||||
//! additive and scoped to exactly the worker turn that needs it — and the scope
|
||||
//! does not leak into detached tasks (standard [`tokio::task_local!`]
|
||||
//! semantics). When unset, tools fall through to `security.action_dir`, so the
|
||||
//! non-isolated path is byte-for-byte unchanged.
|
||||
|
||||
use std::path::PathBuf;
|
||||
|
||||
tokio::task_local! {
|
||||
/// Absolute path to the isolated worktree checkout for the currently
|
||||
/// running worker. `None`-equivalent: the scope is simply not active.
|
||||
pub static CURRENT_ACTION_DIR_OVERRIDE: PathBuf;
|
||||
}
|
||||
|
||||
/// Returns the active per-worker `action_dir` override, if one is installed.
|
||||
///
|
||||
/// Returns `None` when called outside [`with_action_dir_override`] — e.g. the
|
||||
/// non-isolated parallel path, the main agent turn, CLI / JSON-RPC tool
|
||||
/// dispatch, or unit tests that invoke a [`crate::openhuman::tools::Tool`]
|
||||
/// directly.
|
||||
pub fn current_action_dir_override() -> Option<PathBuf> {
|
||||
CURRENT_ACTION_DIR_OVERRIDE.try_with(|p| p.clone()).ok()
|
||||
}
|
||||
|
||||
/// Run `future` with `action_dir` installed as the worker's action-dir
|
||||
/// override. Intended call site is the subagent runner, wrapping the inner
|
||||
/// tool-call loop for a worktree-isolated worker.
|
||||
pub async fn with_action_dir_override<F, R>(action_dir: PathBuf, future: F) -> R
|
||||
where
|
||||
F: std::future::Future<Output = R>,
|
||||
{
|
||||
CURRENT_ACTION_DIR_OVERRIDE.scope(action_dir, future).await
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[tokio::test]
|
||||
async fn override_absent_outside_scope() {
|
||||
assert_eq!(current_action_dir_override(), None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn override_visible_inside_scope() {
|
||||
let dir = PathBuf::from("/tmp/worker-xyz");
|
||||
let seen =
|
||||
with_action_dir_override(dir.clone(), async { current_action_dir_override() }).await;
|
||||
assert_eq!(seen, Some(dir));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn override_does_not_leak() {
|
||||
with_action_dir_override(PathBuf::from("/tmp/a"), async {
|
||||
assert_eq!(current_action_dir_override(), Some(PathBuf::from("/tmp/a")));
|
||||
})
|
||||
.await;
|
||||
assert_eq!(current_action_dir_override(), None);
|
||||
}
|
||||
}
|
||||
@@ -217,6 +217,7 @@ impl Tool for DelegateToPersonalityTool {
|
||||
worker_thread_id: None,
|
||||
initial_history: None,
|
||||
checkpoint_dir: None,
|
||||
worktree_action_dir: None,
|
||||
};
|
||||
|
||||
match run_subagent(&definition, &prompt, options).await {
|
||||
|
||||
@@ -11,6 +11,7 @@ mod ops;
|
||||
pub mod tools;
|
||||
pub mod types;
|
||||
pub mod workflow_runs;
|
||||
pub mod worktree;
|
||||
|
||||
#[cfg(test)]
|
||||
mod ops_tests;
|
||||
@@ -28,3 +29,4 @@ pub use types::{
|
||||
pub use workflow_runs::{
|
||||
all_workflow_run_controller_schemas, all_workflow_run_registered_controllers,
|
||||
};
|
||||
pub use worktree::{BaseRef, WorktreeError, WorktreeStatus};
|
||||
|
||||
@@ -464,6 +464,7 @@ impl AgentOrchestrationSession {
|
||||
worker_thread_id: None,
|
||||
initial_history: None,
|
||||
checkpoint_dir: None,
|
||||
worktree_action_dir: None,
|
||||
};
|
||||
|
||||
let task_session = self.clone();
|
||||
|
||||
@@ -228,6 +228,7 @@ impl Tool for ContinueSubagentTool {
|
||||
worker_thread_id: checkpoint.worker_thread_id.clone(),
|
||||
initial_history: Some(history),
|
||||
checkpoint_dir: Some(checkpoint_dir.clone()),
|
||||
worktree_action_dir: None,
|
||||
};
|
||||
|
||||
// Run the sub-agent from its checkpoint
|
||||
|
||||
@@ -108,6 +108,7 @@ pub(crate) async fn dispatch_subagent(
|
||||
worker_thread_id: None,
|
||||
initial_history: None,
|
||||
checkpoint_dir: None,
|
||||
worktree_action_dir: None,
|
||||
};
|
||||
|
||||
match run_subagent(definition, prompt, options).await {
|
||||
|
||||
@@ -244,6 +244,7 @@ impl Tool for SpawnAsyncSubagentTool {
|
||||
worker_thread_id: background_worker_thread_id.clone(),
|
||||
initial_history: None,
|
||||
checkpoint_dir: None,
|
||||
worktree_action_dir: None,
|
||||
};
|
||||
|
||||
let result = with_parent_context(background_parent, async move {
|
||||
|
||||
@@ -36,6 +36,19 @@ struct ParallelAgentTask {
|
||||
toolkit: Option<String>,
|
||||
#[serde(default)]
|
||||
ownership: Option<String>,
|
||||
/// File-isolation strategy for this worker: `"none"` (default — share the
|
||||
/// parent's `action_dir`) or `"worktree"` (run inside a dedicated
|
||||
/// `git worktree` checkout). Read-only workers should stay `"none"`;
|
||||
/// edit-capable workers opt into `"worktree"` explicitly. We never
|
||||
/// auto-promote a worker to worktree isolation without this flag — the
|
||||
/// approval UX for auto-isolation lands in a later PR.
|
||||
#[serde(default)]
|
||||
isolation: Option<String>,
|
||||
/// When `isolation = "worktree"`, which ref the worktree branches from:
|
||||
/// `"head"` (default — continue the parent's in-progress state) or
|
||||
/// `"fresh"` (start from the repo's default branch).
|
||||
#[serde(default)]
|
||||
base_ref: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Serialize)]
|
||||
@@ -54,6 +67,20 @@ struct ParallelAgentResult {
|
||||
iterations: u32,
|
||||
#[serde(skip_serializing_if = "Vec::is_empty")]
|
||||
stale_parent_reads: Vec<String>,
|
||||
/// Absolute path to the worker's isolated `git worktree` checkout, when
|
||||
/// it ran with `isolation = "worktree"`. `None` for non-isolated workers.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
worktree_path: Option<String>,
|
||||
/// Files (relative to the worktree root) the worker changed, collected
|
||||
/// from `git status` after the run. Empty for non-isolated workers or a
|
||||
/// clean worktree.
|
||||
#[serde(skip_serializing_if = "Vec::is_empty")]
|
||||
changed_files: Vec<String>,
|
||||
/// Whether the worker's worktree had uncommitted changes after the run.
|
||||
/// A dirty worktree must not be auto-removed (surfaced to the UI so the
|
||||
/// user can choose). `None` for non-isolated workers.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
dirty_status: Option<bool>,
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
@@ -96,6 +123,16 @@ impl Tool for SpawnParallelAgentsTool {
|
||||
"ownership": {
|
||||
"type": "string",
|
||||
"description": "Disjoint file/module/responsibility boundary for this worker."
|
||||
},
|
||||
"isolation": {
|
||||
"type": "string",
|
||||
"enum": ["none", "worktree"],
|
||||
"description": "File-isolation strategy. `none` (default) shares the workspace; `worktree` gives this edit-capable worker its own git worktree checkout so parallel edits never collide. Use `worktree` only for edit-capable coding workers, not read-only ones."
|
||||
},
|
||||
"base_ref": {
|
||||
"type": "string",
|
||||
"enum": ["head", "fresh"],
|
||||
"description": "For `isolation = worktree`: branch the worktree from current HEAD (`head`, default) or the repo's default branch (`fresh`)."
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -174,6 +211,16 @@ impl Tool for SpawnParallelAgentsTool {
|
||||
let mut immediate_results = Vec::new();
|
||||
let mut prepared = Vec::new();
|
||||
|
||||
// Resolve the agent sandbox root once — used as the repo root when a
|
||||
// task opts into git-worktree isolation. This is `Config.action_dir`
|
||||
// (the user's project repo the coding agent edits), NOT openhuman's
|
||||
// own tree. Loaded lazily; only consulted for worktree-isolated tasks.
|
||||
let action_root: Option<std::path::PathBuf> =
|
||||
crate::openhuman::config::Config::load_or_init()
|
||||
.await
|
||||
.ok()
|
||||
.map(|cfg| cfg.action_dir.clone());
|
||||
|
||||
for task in tasks {
|
||||
let agent_id = task.agent_id.trim().to_string();
|
||||
let prompt = task.prompt.trim().to_string();
|
||||
@@ -195,6 +242,9 @@ impl Tool for SpawnParallelAgentsTool {
|
||||
elapsed_ms: 0,
|
||||
iterations: 0,
|
||||
stale_parent_reads: Vec::new(),
|
||||
worktree_path: None,
|
||||
changed_files: Vec::new(),
|
||||
dirty_status: None,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
@@ -216,6 +266,9 @@ impl Tool for SpawnParallelAgentsTool {
|
||||
elapsed_ms: 0,
|
||||
iterations: 0,
|
||||
stale_parent_reads: Vec::new(),
|
||||
worktree_path: None,
|
||||
changed_files: Vec::new(),
|
||||
dirty_status: None,
|
||||
});
|
||||
continue;
|
||||
};
|
||||
@@ -242,6 +295,9 @@ impl Tool for SpawnParallelAgentsTool {
|
||||
elapsed_ms: 0,
|
||||
iterations: 0,
|
||||
stale_parent_reads: Vec::new(),
|
||||
worktree_path: None,
|
||||
changed_files: Vec::new(),
|
||||
dirty_status: None,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
@@ -269,6 +325,9 @@ impl Tool for SpawnParallelAgentsTool {
|
||||
elapsed_ms: 0,
|
||||
iterations: 0,
|
||||
stale_parent_reads: Vec::new(),
|
||||
worktree_path: None,
|
||||
changed_files: Vec::new(),
|
||||
dirty_status: None,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
@@ -311,7 +370,88 @@ impl Tool for SpawnParallelAgentsTool {
|
||||
);
|
||||
}
|
||||
}
|
||||
prepared.push((definition, prompt, task, task_id));
|
||||
// ── Optional git-worktree isolation ────────────────────────────
|
||||
// When the task requests `isolation = "worktree"`, create a
|
||||
// dedicated worktree under the user's project repo and run this
|
||||
// worker with its `action_dir` pointed there. On any failure we
|
||||
// surface an immediate error result rather than silently falling
|
||||
// back to the shared workspace (which is the exact collision this
|
||||
// feature prevents).
|
||||
let wants_worktree = task
|
||||
.isolation
|
||||
.as_deref()
|
||||
.map(str::trim)
|
||||
.map(|s| s.eq_ignore_ascii_case("worktree"))
|
||||
.unwrap_or(false);
|
||||
let worktree_path = if wants_worktree {
|
||||
use crate::openhuman::agent_orchestration::worktree;
|
||||
let base_ref = worktree::BaseRef::parse(task.base_ref.as_deref());
|
||||
match action_root.as_ref() {
|
||||
Some(repo_root) => match worktree::create(repo_root, &task_id, base_ref) {
|
||||
Ok(status) => {
|
||||
tracing::debug!(
|
||||
parent_session = %parent_session,
|
||||
task_id = %task_id,
|
||||
worktree = %status.path.display(),
|
||||
base_ref = base_ref.as_str(),
|
||||
"[spawn_parallel_agents] created isolated worktree"
|
||||
);
|
||||
Some(status.path)
|
||||
}
|
||||
Err(err) => {
|
||||
tracing::warn!(
|
||||
parent_session = %parent_session,
|
||||
task_id = %task_id,
|
||||
error = %err,
|
||||
"[spawn_parallel_agents] worktree_create_failed"
|
||||
);
|
||||
immediate_results.push(ParallelAgentResult {
|
||||
task_id,
|
||||
agent_id: definition.id.clone(),
|
||||
success: false,
|
||||
output: None,
|
||||
error: Some(format!("worktree isolation failed: {err}")),
|
||||
ownership: task.ownership,
|
||||
elapsed_ms: 0,
|
||||
iterations: 0,
|
||||
stale_parent_reads: Vec::new(),
|
||||
worktree_path: None,
|
||||
changed_files: Vec::new(),
|
||||
dirty_status: None,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
},
|
||||
None => {
|
||||
tracing::warn!(
|
||||
parent_session = %parent_session,
|
||||
task_id = %task_id,
|
||||
"[spawn_parallel_agents] worktree_requested_but_no_action_dir"
|
||||
);
|
||||
immediate_results.push(ParallelAgentResult {
|
||||
task_id,
|
||||
agent_id: definition.id.clone(),
|
||||
success: false,
|
||||
output: None,
|
||||
error: Some(
|
||||
"worktree isolation requested but action_dir is unavailable"
|
||||
.to_string(),
|
||||
),
|
||||
ownership: task.ownership,
|
||||
elapsed_ms: 0,
|
||||
iterations: 0,
|
||||
stale_parent_reads: Vec::new(),
|
||||
worktree_path: None,
|
||||
changed_files: Vec::new(),
|
||||
dirty_status: None,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
None
|
||||
};
|
||||
prepared.push((definition, prompt, task, task_id, worktree_path));
|
||||
}
|
||||
tracing::debug!(
|
||||
parent_session = %parent_session,
|
||||
@@ -320,11 +460,23 @@ impl Tool for SpawnParallelAgentsTool {
|
||||
"[spawn_parallel_agents] prepared_tasks"
|
||||
);
|
||||
|
||||
let futures = prepared
|
||||
.into_iter()
|
||||
.map(|(definition, prompt, task, task_id)| async move {
|
||||
run_one_parallel_task(definition, prompt, task, task_id).await
|
||||
});
|
||||
let futures =
|
||||
prepared
|
||||
.into_iter()
|
||||
.map(|(definition, prompt, task, task_id, worktree_path)| {
|
||||
let repo_root = action_root.clone();
|
||||
async move {
|
||||
run_one_parallel_task(
|
||||
definition,
|
||||
prompt,
|
||||
task,
|
||||
task_id,
|
||||
worktree_path,
|
||||
repo_root,
|
||||
)
|
||||
.await
|
||||
}
|
||||
});
|
||||
let mut results = immediate_results;
|
||||
for result in join_all(futures).await {
|
||||
match &result {
|
||||
@@ -442,12 +594,49 @@ impl Tool for SpawnParallelAgentsTool {
|
||||
}
|
||||
}
|
||||
|
||||
// Cross-worker overlap detection: when two isolated workers changed
|
||||
// the SAME file, surface a warning so the parent reconciles before
|
||||
// synthesis/merge instead of silently clobbering. Keyed on the
|
||||
// changed-file snapshot collected from each worker's worktree.
|
||||
let per_worker: Vec<(String, Vec<std::path::PathBuf>)> = results
|
||||
.iter()
|
||||
.filter(|r| !r.changed_files.is_empty())
|
||||
.map(|r| {
|
||||
(
|
||||
r.task_id.clone(),
|
||||
r.changed_files
|
||||
.iter()
|
||||
.map(std::path::PathBuf::from)
|
||||
.collect(),
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
let overlaps =
|
||||
crate::openhuman::agent_orchestration::worktree::detect_overlaps(&per_worker);
|
||||
let overlap_warnings: Vec<serde_json::Value> = overlaps
|
||||
.iter()
|
||||
.map(|(file, workers)| {
|
||||
json!({
|
||||
"file": file.to_string_lossy(),
|
||||
"workers": workers,
|
||||
})
|
||||
})
|
||||
.collect();
|
||||
if !overlap_warnings.is_empty() {
|
||||
tracing::warn!(
|
||||
parent_session = %parent_session,
|
||||
overlap_count = overlap_warnings.len(),
|
||||
"[spawn_parallel_agents] detected overlapping changed files across workers"
|
||||
);
|
||||
}
|
||||
|
||||
let failures = results.iter().filter(|r| !r.success).count();
|
||||
tracing::debug!(
|
||||
parent_session = %parent_session,
|
||||
total = results.len(),
|
||||
succeeded = results.len().saturating_sub(failures),
|
||||
failed = failures,
|
||||
overlaps = overlap_warnings.len(),
|
||||
"[spawn_parallel_agents] execute exit"
|
||||
);
|
||||
Ok(ToolResult::success(
|
||||
@@ -457,6 +646,7 @@ impl Tool for SpawnParallelAgentsTool {
|
||||
"succeeded": results.len() - failures,
|
||||
"failed": failures,
|
||||
"results": results,
|
||||
"overlap_warnings": overlap_warnings,
|
||||
}
|
||||
}))
|
||||
.unwrap_or_else(|_| "{}".to_string()),
|
||||
@@ -469,6 +659,8 @@ async fn run_one_parallel_task(
|
||||
prompt: String,
|
||||
task: ParallelAgentTask,
|
||||
task_id: String,
|
||||
worktree_path: Option<std::path::PathBuf>,
|
||||
repo_root: Option<std::path::PathBuf>,
|
||||
) -> ParallelAgentResult {
|
||||
let started = std::time::Instant::now();
|
||||
tracing::debug!(
|
||||
@@ -477,6 +669,7 @@ async fn run_one_parallel_task(
|
||||
toolkit = task.toolkit.as_deref().unwrap_or(""),
|
||||
context_chars = task.context.as_ref().map(|s| s.chars().count()).unwrap_or(0),
|
||||
prompt_chars = prompt.chars().count(),
|
||||
isolated = worktree_path.is_some(),
|
||||
"[spawn_parallel_agents] task_start"
|
||||
);
|
||||
let options = SubagentRunOptions {
|
||||
@@ -488,8 +681,51 @@ async fn run_one_parallel_task(
|
||||
worker_thread_id: None,
|
||||
initial_history: None,
|
||||
checkpoint_dir: None,
|
||||
worktree_action_dir: worktree_path.clone(),
|
||||
};
|
||||
match run_subagent(&definition, &prompt, options).await {
|
||||
let run_result = run_subagent(&definition, &prompt, options).await;
|
||||
|
||||
// After the worker finishes, snapshot the worktree's changed files +
|
||||
// dirty status so the parent can detect cross-worker overlaps and the UI
|
||||
// can surface diff/cleanup actions. Best-effort: a status error degrades
|
||||
// to "no changes recorded" rather than failing the task.
|
||||
let worktree_str = worktree_path
|
||||
.as_ref()
|
||||
.map(|p| p.to_string_lossy().to_string());
|
||||
let (changed_files, dirty_status) = match (&worktree_path, &repo_root) {
|
||||
(Some(wt), Some(root)) => {
|
||||
use crate::openhuman::agent_orchestration::worktree;
|
||||
match worktree::status(root, wt) {
|
||||
Ok(st) => {
|
||||
tracing::debug!(
|
||||
task_id = %task_id,
|
||||
worktree = %wt.display(),
|
||||
is_dirty = st.is_dirty,
|
||||
changed = st.changed_files.len(),
|
||||
"[spawn_parallel_agents] worktree_post_run_status"
|
||||
);
|
||||
let files = st
|
||||
.changed_files
|
||||
.iter()
|
||||
.map(|p| p.to_string_lossy().to_string())
|
||||
.collect();
|
||||
(files, Some(st.is_dirty))
|
||||
}
|
||||
Err(err) => {
|
||||
tracing::warn!(
|
||||
task_id = %task_id,
|
||||
worktree = %wt.display(),
|
||||
error = %err,
|
||||
"[spawn_parallel_agents] worktree_status_failed"
|
||||
);
|
||||
(Vec::new(), None)
|
||||
}
|
||||
}
|
||||
}
|
||||
_ => (Vec::new(), None),
|
||||
};
|
||||
|
||||
match run_result {
|
||||
Ok(outcome) => {
|
||||
tracing::debug!(
|
||||
task_id = %outcome.task_id,
|
||||
@@ -509,6 +745,9 @@ async fn run_one_parallel_task(
|
||||
elapsed_ms: outcome.elapsed.as_millis() as u64,
|
||||
iterations: outcome.iterations as u32,
|
||||
stale_parent_reads: Vec::new(),
|
||||
worktree_path: worktree_str,
|
||||
changed_files,
|
||||
dirty_status,
|
||||
}
|
||||
}
|
||||
Err(err) => {
|
||||
@@ -529,6 +768,9 @@ async fn run_one_parallel_task(
|
||||
elapsed_ms: started.elapsed().as_millis() as u64,
|
||||
iterations: 0,
|
||||
stale_parent_reads: Vec::new(),
|
||||
worktree_path: worktree_str,
|
||||
changed_files,
|
||||
dirty_status,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -46,6 +46,85 @@ fn ownership_boundary_is_prepended_when_present() {
|
||||
assert!(prompt.contains("[Task]\nimplement tests"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn schema_advertises_isolation_and_base_ref() {
|
||||
let tool = SpawnParallelAgentsTool::default();
|
||||
let schema = tool.parameters_schema();
|
||||
let props = &schema["properties"]["tasks"]["items"]["properties"];
|
||||
assert_eq!(props["isolation"]["enum"][0], "none");
|
||||
assert_eq!(props["isolation"]["enum"][1], "worktree");
|
||||
assert_eq!(props["base_ref"]["enum"][0], "head");
|
||||
assert_eq!(props["base_ref"]["enum"][1], "fresh");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn task_deserializes_isolation_and_base_ref() {
|
||||
let task: ParallelAgentTask = serde_json::from_value(json!({
|
||||
"agent_id": "coder",
|
||||
"prompt": "do it",
|
||||
"isolation": "worktree",
|
||||
"base_ref": "fresh"
|
||||
}))
|
||||
.expect("deserialize task");
|
||||
assert_eq!(task.isolation.as_deref(), Some("worktree"));
|
||||
assert_eq!(task.base_ref.as_deref(), Some("fresh"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn task_isolation_defaults_to_none() {
|
||||
let task: ParallelAgentTask = serde_json::from_value(json!({
|
||||
"agent_id": "researcher",
|
||||
"prompt": "read it"
|
||||
}))
|
||||
.expect("deserialize task");
|
||||
assert!(task.isolation.is_none());
|
||||
assert!(task.base_ref.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn result_omits_worktree_fields_when_absent() {
|
||||
let result = ParallelAgentResult {
|
||||
task_id: "t1".into(),
|
||||
agent_id: "a".into(),
|
||||
success: true,
|
||||
output: Some("ok".into()),
|
||||
error: None,
|
||||
ownership: None,
|
||||
elapsed_ms: 5,
|
||||
iterations: 1,
|
||||
stale_parent_reads: Vec::new(),
|
||||
worktree_path: None,
|
||||
changed_files: Vec::new(),
|
||||
dirty_status: None,
|
||||
};
|
||||
let v = serde_json::to_value(&result).unwrap();
|
||||
assert!(v.get("worktreePath").is_none());
|
||||
assert!(v.get("changedFiles").is_none());
|
||||
assert!(v.get("dirtyStatus").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn result_serializes_worktree_fields_when_present() {
|
||||
let result = ParallelAgentResult {
|
||||
task_id: "t2".into(),
|
||||
agent_id: "coder".into(),
|
||||
success: true,
|
||||
output: None,
|
||||
error: None,
|
||||
ownership: None,
|
||||
elapsed_ms: 9,
|
||||
iterations: 2,
|
||||
stale_parent_reads: Vec::new(),
|
||||
worktree_path: Some("/repo/.claude/worktrees/t2".into()),
|
||||
changed_files: vec!["src/a.rs".into()],
|
||||
dirty_status: Some(true),
|
||||
};
|
||||
let v = serde_json::to_value(&result).unwrap();
|
||||
assert_eq!(v["worktreePath"], "/repo/.claude/worktrees/t2");
|
||||
assert_eq!(v["changedFiles"][0], "src/a.rs");
|
||||
assert_eq!(v["dirtyStatus"], true);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn rejects_single_task() {
|
||||
let tool = SpawnParallelAgentsTool::new();
|
||||
|
||||
@@ -458,6 +458,7 @@ impl Tool for SpawnSubagentTool {
|
||||
worker_thread_id: worker_thread_id.clone(),
|
||||
initial_history: None,
|
||||
checkpoint_dir: None,
|
||||
worktree_action_dir: None,
|
||||
};
|
||||
|
||||
let progress_sink = current_parent().and_then(|p| p.on_progress.clone());
|
||||
|
||||
@@ -227,6 +227,7 @@ impl Tool for SpawnWorkerThreadTool {
|
||||
worker_thread_id: Some(worker_thread_id.clone()),
|
||||
initial_history: None,
|
||||
checkpoint_dir: None,
|
||||
worktree_action_dir: None,
|
||||
};
|
||||
|
||||
tracing::debug!(
|
||||
|
||||
@@ -0,0 +1,445 @@
|
||||
//! Git-worktree isolation for parallel, edit-capable agent workers.
|
||||
//!
|
||||
//! Parallel coding workers spawned via [`super::tools::spawn_parallel_agents`]
|
||||
//! historically shared one workspace (`Config.action_dir`). Two workers editing
|
||||
//! overlapping files left the parent with stale assumptions and silent
|
||||
//! clobbers. This module gives each isolated worker its own `git worktree`
|
||||
//! checkout of the **user's project repo** (the directory the coding agent
|
||||
//! edits), so file edits never collide.
|
||||
//!
|
||||
//! ## Scope and safety
|
||||
//!
|
||||
//! - This targets the **user's project repository** rooted at the agent's
|
||||
//! `action_dir` — it never operates on OpenHuman's own source tree.
|
||||
//! - Every operation validates that `repo_root` is a real git repository
|
||||
//! first (via `git rev-parse --is-inside-work-tree`), so a stray path can
|
||||
//! never be mutated.
|
||||
//! - [`remove`] refuses to delete a **dirty** worktree unless `force = true`.
|
||||
//! Clean worktrees can be auto-reclaimed; dirty ones require an explicit
|
||||
//! user decision (acceptance criterion of #3376).
|
||||
//!
|
||||
//! The wrapper shells out to `git` through [`std::process::Command`] with an
|
||||
//! explicit, validated working directory. It does not inherit ambient git
|
||||
//! configuration that could redirect operations elsewhere.
|
||||
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::process::Command;
|
||||
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
/// Directory (relative to the repo root) under which isolated worker
|
||||
/// worktrees are created — mirrors Claude Code's `.claude/worktrees/`
|
||||
/// convention so the layout is familiar and easy to `.gitignore`.
|
||||
pub const WORKTREE_SUBDIR: &str = ".claude/worktrees";
|
||||
|
||||
/// Which ref a new worktree branches from.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum BaseRef {
|
||||
/// Branch off the repo's current `HEAD` — the worker sees the parent's
|
||||
/// in-progress state.
|
||||
Head,
|
||||
/// Branch off the repository's default branch (origin/HEAD, else the
|
||||
/// local default) — the worker starts from a clean, known baseline.
|
||||
Fresh,
|
||||
}
|
||||
|
||||
impl BaseRef {
|
||||
/// Parse the spawn-request string form. Unknown / empty values default
|
||||
/// to [`BaseRef::Head`] (the least-surprising baseline for a worker that
|
||||
/// continues the parent's work).
|
||||
pub fn parse(value: Option<&str>) -> Self {
|
||||
match value.map(str::trim).map(str::to_ascii_lowercase).as_deref() {
|
||||
Some("fresh") => Self::Fresh,
|
||||
_ => Self::Head,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn as_str(self) -> &'static str {
|
||||
match self {
|
||||
Self::Head => "head",
|
||||
Self::Fresh => "fresh",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Snapshot of a single worktree's state.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct WorktreeStatus {
|
||||
/// Absolute path to the worktree checkout.
|
||||
pub path: PathBuf,
|
||||
/// Branch the worktree currently has checked out (or a detached-HEAD
|
||||
/// label like `(detached HEAD)`), if resolvable.
|
||||
pub branch: Option<String>,
|
||||
/// Whether the worktree has uncommitted changes (staged, unstaged, or
|
||||
/// untracked). A dirty worktree must not be auto-removed.
|
||||
pub is_dirty: bool,
|
||||
/// Paths (relative to the worktree root) that differ from HEAD.
|
||||
pub changed_files: Vec<PathBuf>,
|
||||
}
|
||||
|
||||
/// Errors surfaced by the worktree manager. Stringified at the RPC / tool
|
||||
/// boundary.
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
pub enum WorktreeError {
|
||||
#[error("path is not inside a git repository: {0}")]
|
||||
NotAGitRepo(PathBuf),
|
||||
|
||||
#[error("worktree is dirty and force=false; refusing to remove: {0}")]
|
||||
DirtyRefused(PathBuf),
|
||||
|
||||
#[error("git command `{command}` failed: {stderr}")]
|
||||
GitFailed { command: String, stderr: String },
|
||||
|
||||
#[error("io error running git: {0}")]
|
||||
Io(#[from] std::io::Error),
|
||||
}
|
||||
|
||||
type Result<T> = std::result::Result<T, WorktreeError>;
|
||||
|
||||
/// Run `git <args>` in `cwd`, returning trimmed stdout on success.
|
||||
fn git(cwd: &Path, args: &[&str]) -> Result<String> {
|
||||
tracing::debug!(
|
||||
cwd = %cwd.display(),
|
||||
args = ?args,
|
||||
"[worktree] git_invoke"
|
||||
);
|
||||
let output = Command::new("git").current_dir(cwd).args(args).output()?;
|
||||
if !output.status.success() {
|
||||
let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
|
||||
tracing::debug!(
|
||||
cwd = %cwd.display(),
|
||||
args = ?args,
|
||||
stderr = %stderr,
|
||||
"[worktree] git_failed"
|
||||
);
|
||||
return Err(WorktreeError::GitFailed {
|
||||
command: format!("git {}", args.join(" ")),
|
||||
stderr,
|
||||
});
|
||||
}
|
||||
Ok(String::from_utf8_lossy(&output.stdout).trim().to_string())
|
||||
}
|
||||
|
||||
/// Like [`git`] but returns stdout **without** trimming, preserving the
|
||||
/// leading whitespace that porcelain v1 status lines depend on for column
|
||||
/// alignment. Trailing newline is left intact; callers iterate `.lines()`.
|
||||
fn git_raw(cwd: &Path, args: &[&str]) -> Result<String> {
|
||||
let output = Command::new("git").current_dir(cwd).args(args).output()?;
|
||||
if !output.status.success() {
|
||||
let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
|
||||
return Err(WorktreeError::GitFailed {
|
||||
command: format!("git {}", args.join(" ")),
|
||||
stderr,
|
||||
});
|
||||
}
|
||||
Ok(String::from_utf8_lossy(&output.stdout).to_string())
|
||||
}
|
||||
|
||||
/// Validate that `repo_root` is inside a real git work tree. Returns the
|
||||
/// repository's top-level directory on success.
|
||||
fn validate_repo_root(repo_root: &Path) -> Result<PathBuf> {
|
||||
if !repo_root.exists() {
|
||||
return Err(WorktreeError::NotAGitRepo(repo_root.to_path_buf()));
|
||||
}
|
||||
// `--is-inside-work-tree` prints "true" when inside a work tree. We then
|
||||
// resolve the canonical top level so all later operations anchor on it.
|
||||
let inside = git(repo_root, &["rev-parse", "--is-inside-work-tree"])
|
||||
.map_err(|_| WorktreeError::NotAGitRepo(repo_root.to_path_buf()))?;
|
||||
if inside.trim() != "true" {
|
||||
return Err(WorktreeError::NotAGitRepo(repo_root.to_path_buf()));
|
||||
}
|
||||
let top = git(repo_root, &["rev-parse", "--show-toplevel"])
|
||||
.map_err(|_| WorktreeError::NotAGitRepo(repo_root.to_path_buf()))?;
|
||||
Ok(PathBuf::from(top.trim()))
|
||||
}
|
||||
|
||||
/// Resolve the repo's default branch ref for `BaseRef::Fresh`. Prefers
|
||||
/// `origin/HEAD`; falls back to the local `HEAD` symbolic ref, then `main`.
|
||||
fn resolve_fresh_base(repo_top: &Path) -> String {
|
||||
// origin/HEAD → e.g. "origin/main"
|
||||
if let Ok(sym) = git(
|
||||
repo_top,
|
||||
&["symbolic-ref", "--short", "refs/remotes/origin/HEAD"],
|
||||
) {
|
||||
if !sym.is_empty() {
|
||||
return sym;
|
||||
}
|
||||
}
|
||||
// Local default branch name.
|
||||
if let Ok(head) = git(repo_top, &["symbolic-ref", "--short", "HEAD"]) {
|
||||
if !head.is_empty() {
|
||||
return head;
|
||||
}
|
||||
}
|
||||
"main".to_string()
|
||||
}
|
||||
|
||||
/// Create an isolated worktree for `run_id` under
|
||||
/// `<repo>/.claude/worktrees/<run_id>`, branching off `base_ref`.
|
||||
///
|
||||
/// The new branch is named `worker/<run_id>`. Returns the worktree's status
|
||||
/// snapshot (freshly created, so `is_dirty = false`).
|
||||
pub fn create(repo_root: &Path, run_id: &str, base_ref: BaseRef) -> Result<WorktreeStatus> {
|
||||
let repo_top = validate_repo_root(repo_root)?;
|
||||
let run_slug = sanitize_run_id(run_id);
|
||||
let worktree_path = repo_top.join(WORKTREE_SUBDIR).join(&run_slug);
|
||||
let branch = format!("worker/{run_slug}");
|
||||
|
||||
let base = match base_ref {
|
||||
BaseRef::Head => "HEAD".to_string(),
|
||||
BaseRef::Fresh => resolve_fresh_base(&repo_top),
|
||||
};
|
||||
|
||||
tracing::debug!(
|
||||
repo = %repo_top.display(),
|
||||
worktree = %worktree_path.display(),
|
||||
branch = %branch,
|
||||
base = %base,
|
||||
base_ref = base_ref.as_str(),
|
||||
"[worktree] create_start"
|
||||
);
|
||||
|
||||
// Ensure the parent dir exists; `git worktree add` creates the leaf.
|
||||
if let Some(parent) = worktree_path.parent() {
|
||||
std::fs::create_dir_all(parent)?;
|
||||
}
|
||||
|
||||
let worktree_str = worktree_path.to_string_lossy().to_string();
|
||||
git(
|
||||
&repo_top,
|
||||
&["worktree", "add", "-b", &branch, &worktree_str, &base],
|
||||
)?;
|
||||
|
||||
tracing::debug!(
|
||||
worktree = %worktree_path.display(),
|
||||
branch = %branch,
|
||||
"[worktree] create_done"
|
||||
);
|
||||
|
||||
status(&repo_top, &worktree_path)
|
||||
}
|
||||
|
||||
/// List all worktrees registered on the repo at `repo_root`, parsed from
|
||||
/// `git worktree list --porcelain`.
|
||||
pub fn list(repo_root: &Path) -> Result<Vec<WorktreeStatus>> {
|
||||
let repo_top = validate_repo_root(repo_root)?;
|
||||
let porcelain = git(&repo_top, &["worktree", "list", "--porcelain"])?;
|
||||
let mut out = Vec::new();
|
||||
let mut cur_path: Option<PathBuf> = None;
|
||||
let mut cur_branch: Option<String> = None;
|
||||
|
||||
let mut flush = |path: &mut Option<PathBuf>, branch: &mut Option<String>| {
|
||||
if let Some(p) = path.take() {
|
||||
// status() re-derives dirty + changed files for the worktree.
|
||||
let (is_dirty, changed_files) = dirty_state(&p).unwrap_or((false, Vec::new()));
|
||||
out.push(WorktreeStatus {
|
||||
path: p,
|
||||
branch: branch.take(),
|
||||
is_dirty,
|
||||
changed_files,
|
||||
});
|
||||
} else {
|
||||
*branch = None;
|
||||
}
|
||||
};
|
||||
|
||||
for line in porcelain.lines() {
|
||||
if let Some(rest) = line.strip_prefix("worktree ") {
|
||||
// New record begins — flush the previous one.
|
||||
flush(&mut cur_path, &mut cur_branch);
|
||||
cur_path = Some(PathBuf::from(rest.trim()));
|
||||
} else if let Some(rest) = line.strip_prefix("branch ") {
|
||||
// "branch refs/heads/foo" → "foo"
|
||||
cur_branch = Some(
|
||||
rest.trim()
|
||||
.strip_prefix("refs/heads/")
|
||||
.unwrap_or(rest.trim())
|
||||
.to_string(),
|
||||
);
|
||||
} else if line.trim() == "detached" {
|
||||
cur_branch = Some("(detached HEAD)".to_string());
|
||||
}
|
||||
}
|
||||
flush(&mut cur_path, &mut cur_branch);
|
||||
|
||||
tracing::debug!(
|
||||
repo = %repo_top.display(),
|
||||
count = out.len(),
|
||||
"[worktree] list_done"
|
||||
);
|
||||
Ok(out)
|
||||
}
|
||||
|
||||
/// Branch + dirty + changed-file snapshot for a single worktree.
|
||||
pub fn status(repo_root: &Path, worktree_path: &Path) -> Result<WorktreeStatus> {
|
||||
validate_repo_root(repo_root)?;
|
||||
if !worktree_path.exists() {
|
||||
return Err(WorktreeError::NotAGitRepo(worktree_path.to_path_buf()));
|
||||
}
|
||||
let branch = git(worktree_path, &["rev-parse", "--abbrev-ref", "HEAD"])
|
||||
.ok()
|
||||
.map(|b| {
|
||||
if b == "HEAD" {
|
||||
"(detached HEAD)".to_string()
|
||||
} else {
|
||||
b
|
||||
}
|
||||
});
|
||||
let (is_dirty, changed_files) = dirty_state(worktree_path)?;
|
||||
Ok(WorktreeStatus {
|
||||
path: worktree_path.to_path_buf(),
|
||||
branch,
|
||||
is_dirty,
|
||||
changed_files,
|
||||
})
|
||||
}
|
||||
|
||||
/// Human-readable diff stat of a worktree's working changes vs HEAD,
|
||||
/// including untracked files. Returns an empty string for a clean worktree.
|
||||
pub fn diff_summary(repo_root: &Path, worktree_path: &Path) -> Result<String> {
|
||||
validate_repo_root(repo_root)?;
|
||||
// `--stat` over both staged and unstaged changes vs HEAD.
|
||||
let stat = git(worktree_path, &["diff", "HEAD", "--stat"])?;
|
||||
let untracked = git(
|
||||
worktree_path,
|
||||
&["ls-files", "--others", "--exclude-standard"],
|
||||
)?;
|
||||
let mut parts = Vec::new();
|
||||
if !stat.is_empty() {
|
||||
parts.push(stat);
|
||||
}
|
||||
if !untracked.is_empty() {
|
||||
let lines: Vec<String> = untracked
|
||||
.lines()
|
||||
.map(|l| format!(" {l} (untracked)"))
|
||||
.collect();
|
||||
parts.push(lines.join("\n"));
|
||||
}
|
||||
Ok(parts.join("\n"))
|
||||
}
|
||||
|
||||
/// Remove a worktree. **Refuses to remove a dirty worktree unless
|
||||
/// `force = true`** — the core safety guarantee of #3376.
|
||||
///
|
||||
/// On `force = true`, a dirty worktree is removed with `git worktree remove
|
||||
/// --force`. The associated `worker/<run_id>` branch is left intact so the
|
||||
/// caller can still inspect or merge the work.
|
||||
pub fn remove(repo_root: &Path, worktree_path: &Path, force: bool) -> Result<()> {
|
||||
let repo_top = validate_repo_root(repo_root)?;
|
||||
let (is_dirty, _changed) = dirty_state(worktree_path).unwrap_or((false, Vec::new()));
|
||||
|
||||
tracing::debug!(
|
||||
repo = %repo_top.display(),
|
||||
worktree = %worktree_path.display(),
|
||||
is_dirty,
|
||||
force,
|
||||
"[worktree] remove_start"
|
||||
);
|
||||
|
||||
if is_dirty && !force {
|
||||
tracing::warn!(
|
||||
worktree = %worktree_path.display(),
|
||||
"[worktree] remove_refused_dirty"
|
||||
);
|
||||
return Err(WorktreeError::DirtyRefused(worktree_path.to_path_buf()));
|
||||
}
|
||||
|
||||
let worktree_str = worktree_path.to_string_lossy().to_string();
|
||||
let mut args = vec!["worktree", "remove", &worktree_str];
|
||||
if force {
|
||||
args.push("--force");
|
||||
}
|
||||
git(&repo_top, &args)?;
|
||||
tracing::debug!(
|
||||
worktree = %worktree_path.display(),
|
||||
"[worktree] remove_done"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Compute `(is_dirty, changed_files)` for a worktree via
|
||||
/// `git status --porcelain`. A worktree is dirty when any tracked file is
|
||||
/// modified/staged or any untracked (non-ignored) file is present.
|
||||
///
|
||||
/// Uses the raw (un-trimmed) command output: porcelain v1 lines are
|
||||
/// column-aligned (`XY␠PATH`, status = 2 chars), so a global `.trim()` would
|
||||
/// corrupt the leading space of a `␠M file` line and shift the path. We parse
|
||||
/// the raw bytes and slice the fixed 3-char `XY␠` prefix per line.
|
||||
fn dirty_state(worktree_path: &Path) -> Result<(bool, Vec<PathBuf>)> {
|
||||
let porcelain = git_raw(worktree_path, &["status", "--porcelain"])?;
|
||||
let mut changed = Vec::new();
|
||||
for line in porcelain.lines() {
|
||||
// Porcelain v1 line: "XY <path>" — status is exactly 2 chars, then a
|
||||
// single separator space, so the path always starts at byte index 3.
|
||||
if line.len() > 3 {
|
||||
let path = line[3..].trim_end();
|
||||
// Rename lines look like "old -> new"; record the new path.
|
||||
let path = path.rsplit(" -> ").next().unwrap_or(path);
|
||||
changed.push(PathBuf::from(path));
|
||||
}
|
||||
}
|
||||
changed.sort();
|
||||
changed.dedup();
|
||||
Ok((!changed.is_empty(), changed))
|
||||
}
|
||||
|
||||
/// Sanitize a `run_id` into a filesystem-safe single path segment.
|
||||
fn sanitize_run_id(run_id: &str) -> String {
|
||||
let cleaned: String = run_id
|
||||
.chars()
|
||||
.map(|c| {
|
||||
if c.is_ascii_alphanumeric() || c == '_' || c == '-' {
|
||||
c
|
||||
} else {
|
||||
'-'
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
let trimmed = cleaned.trim_matches('-');
|
||||
if trimmed.is_empty() {
|
||||
"worker".to_string()
|
||||
} else {
|
||||
trimmed.to_string()
|
||||
}
|
||||
}
|
||||
|
||||
/// Detect changed files touched by more than one sibling worker.
|
||||
///
|
||||
/// Input maps each worker's id to the set of files it changed (relative
|
||||
/// paths, as produced by [`WorktreeStatus::changed_files`]). Output maps each
|
||||
/// overlapping file to the sorted list of worker ids that touched it — empty
|
||||
/// when there is no overlap. Used to surface a pre-merge conflict warning.
|
||||
pub fn detect_overlaps(
|
||||
per_worker: &[(String, Vec<PathBuf>)],
|
||||
) -> std::collections::BTreeMap<PathBuf, Vec<String>> {
|
||||
use std::collections::BTreeMap;
|
||||
let mut by_file: BTreeMap<PathBuf, Vec<String>> = BTreeMap::new();
|
||||
for (worker_id, files) in per_worker {
|
||||
let mut seen = std::collections::BTreeSet::new();
|
||||
for f in files {
|
||||
if seen.insert(f.clone()) {
|
||||
by_file
|
||||
.entry(f.clone())
|
||||
.or_default()
|
||||
.push(worker_id.clone());
|
||||
}
|
||||
}
|
||||
}
|
||||
by_file
|
||||
.into_iter()
|
||||
.filter_map(|(file, mut workers)| {
|
||||
workers.sort();
|
||||
workers.dedup();
|
||||
if workers.len() > 1 {
|
||||
Some((file, workers))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "worktree_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,235 @@
|
||||
//! Tests for the git-worktree isolation manager.
|
||||
//!
|
||||
//! Each test stands up a real temporary git repository (`git init`) so the
|
||||
//! `git worktree` plumbing is exercised end-to-end. Tests are skipped (pass
|
||||
//! trivially) when `git` is not on PATH, so CI without git doesn't hard-fail.
|
||||
|
||||
use super::*;
|
||||
use std::path::Path;
|
||||
use std::process::Command;
|
||||
|
||||
/// `true` when `git` is invokable on this host.
|
||||
fn git_available() -> bool {
|
||||
Command::new("git")
|
||||
.arg("--version")
|
||||
.output()
|
||||
.map(|o| o.status.success())
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
fn run(dir: &Path, args: &[&str]) {
|
||||
let status = Command::new("git")
|
||||
.current_dir(dir)
|
||||
.args(args)
|
||||
.output()
|
||||
.expect("git invocation");
|
||||
assert!(
|
||||
status.status.success(),
|
||||
"git {:?} failed: {}",
|
||||
args,
|
||||
String::from_utf8_lossy(&status.stderr)
|
||||
);
|
||||
}
|
||||
|
||||
/// Initialise a temp git repo with one committed file. Returns the tempdir
|
||||
/// guard (kept alive by the caller) and the repo root path.
|
||||
fn init_repo() -> (tempfile::TempDir, std::path::PathBuf) {
|
||||
let tmp = tempfile::tempdir().expect("tempdir");
|
||||
let root = tmp.path().to_path_buf();
|
||||
run(&root, &["init", "-b", "main"]);
|
||||
run(&root, &["config", "user.email", "test@example.com"]);
|
||||
run(&root, &["config", "user.name", "Test User"]);
|
||||
std::fs::write(root.join("README.md"), "hello\n").unwrap();
|
||||
run(&root, &["add", "README.md"]);
|
||||
run(&root, &["commit", "-m", "initial"]);
|
||||
(tmp, root)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_repo_root_rejects_non_repo() {
|
||||
if !git_available() {
|
||||
return;
|
||||
}
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let err = create(tmp.path(), "run-1", BaseRef::Head).unwrap_err();
|
||||
assert!(matches!(err, WorktreeError::NotAGitRepo(_)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn create_then_status_reports_clean_worktree() {
|
||||
if !git_available() {
|
||||
return;
|
||||
}
|
||||
let (_tmp, root) = init_repo();
|
||||
let st = create(&root, "run-1", BaseRef::Head).expect("create");
|
||||
assert!(st.path.exists(), "worktree dir should exist");
|
||||
assert_eq!(st.branch.as_deref(), Some("worker/run-1"));
|
||||
assert!(!st.is_dirty, "fresh worktree is clean");
|
||||
assert!(st.changed_files.is_empty());
|
||||
assert!(
|
||||
st.path.ends_with(Path::new(".claude/worktrees/run-1")),
|
||||
"worktree under .claude/worktrees/<run_id>, got {}",
|
||||
st.path.display()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn list_includes_created_worktree() {
|
||||
if !git_available() {
|
||||
return;
|
||||
}
|
||||
let (_tmp, root) = init_repo();
|
||||
create(&root, "run-a", BaseRef::Head).expect("create a");
|
||||
create(&root, "run-b", BaseRef::Fresh).expect("create b");
|
||||
let all = list(&root).expect("list");
|
||||
// main worktree + the two we created
|
||||
assert!(all.len() >= 3, "expected >=3 worktrees, got {}", all.len());
|
||||
let branches: Vec<_> = all.iter().filter_map(|w| w.branch.clone()).collect();
|
||||
assert!(branches.iter().any(|b| b == "worker/run-a"));
|
||||
assert!(branches.iter().any(|b| b == "worker/run-b"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn status_detects_dirty_changes() {
|
||||
if !git_available() {
|
||||
return;
|
||||
}
|
||||
let (_tmp, root) = init_repo();
|
||||
let st = create(&root, "run-dirty", BaseRef::Head).expect("create");
|
||||
// Touch a tracked file + add an untracked one.
|
||||
std::fs::write(st.path.join("README.md"), "changed\n").unwrap();
|
||||
std::fs::write(st.path.join("new.txt"), "fresh\n").unwrap();
|
||||
|
||||
let st2 = status(&root, &st.path).expect("status");
|
||||
assert!(st2.is_dirty, "worktree with edits must be dirty");
|
||||
let names: Vec<String> = st2
|
||||
.changed_files
|
||||
.iter()
|
||||
.map(|p| p.to_string_lossy().to_string())
|
||||
.collect();
|
||||
assert!(names.iter().any(|n| n.contains("README.md")));
|
||||
assert!(names.iter().any(|n| n.contains("new.txt")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn diff_summary_lists_tracked_and_untracked() {
|
||||
if !git_available() {
|
||||
return;
|
||||
}
|
||||
let (_tmp, root) = init_repo();
|
||||
let st = create(&root, "run-diff", BaseRef::Head).expect("create");
|
||||
std::fs::write(st.path.join("README.md"), "changed body\n").unwrap();
|
||||
std::fs::write(st.path.join("brand_new.txt"), "x\n").unwrap();
|
||||
|
||||
let summary = diff_summary(&root, &st.path).expect("diff");
|
||||
assert!(
|
||||
summary.contains("README.md"),
|
||||
"diff should mention tracked change: {summary}"
|
||||
);
|
||||
assert!(
|
||||
summary.contains("brand_new.txt") && summary.contains("untracked"),
|
||||
"diff should list untracked file: {summary}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remove_refuses_dirty_without_force() {
|
||||
if !git_available() {
|
||||
return;
|
||||
}
|
||||
let (_tmp, root) = init_repo();
|
||||
let st = create(&root, "run-keep", BaseRef::Head).expect("create");
|
||||
std::fs::write(st.path.join("README.md"), "dirty\n").unwrap();
|
||||
|
||||
let err = remove(&root, &st.path, false).expect_err("must refuse dirty");
|
||||
assert!(matches!(err, WorktreeError::DirtyRefused(_)));
|
||||
assert!(st.path.exists(), "dirty worktree must NOT be deleted");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remove_force_deletes_dirty_worktree() {
|
||||
if !git_available() {
|
||||
return;
|
||||
}
|
||||
let (_tmp, root) = init_repo();
|
||||
let st = create(&root, "run-force", BaseRef::Head).expect("create");
|
||||
std::fs::write(st.path.join("README.md"), "dirty\n").unwrap();
|
||||
|
||||
remove(&root, &st.path, true).expect("force remove");
|
||||
assert!(!st.path.exists(), "force remove deletes the worktree dir");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remove_clean_worktree_succeeds() {
|
||||
if !git_available() {
|
||||
return;
|
||||
}
|
||||
let (_tmp, root) = init_repo();
|
||||
let st = create(&root, "run-clean", BaseRef::Head).expect("create");
|
||||
remove(&root, &st.path, false).expect("clean remove");
|
||||
assert!(!st.path.exists(), "clean worktree removed without force");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn base_ref_parse_defaults_to_head() {
|
||||
assert_eq!(BaseRef::parse(None), BaseRef::Head);
|
||||
assert_eq!(BaseRef::parse(Some("head")), BaseRef::Head);
|
||||
assert_eq!(BaseRef::parse(Some("HEAD")), BaseRef::Head);
|
||||
assert_eq!(BaseRef::parse(Some("fresh")), BaseRef::Fresh);
|
||||
assert_eq!(BaseRef::parse(Some(" Fresh ")), BaseRef::Fresh);
|
||||
assert_eq!(BaseRef::parse(Some("garbage")), BaseRef::Head);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sanitize_run_id_strips_unsafe_chars() {
|
||||
assert_eq!(sanitize_run_id("sub-1234"), "sub-1234");
|
||||
assert_eq!(sanitize_run_id("a/b\\c"), "a-b-c");
|
||||
assert_eq!(sanitize_run_id("///"), "worker");
|
||||
assert_eq!(sanitize_run_id(""), "worker");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn detect_overlaps_flags_shared_files() {
|
||||
let per_worker = vec![
|
||||
(
|
||||
"w1".to_string(),
|
||||
vec![PathBuf::from("src/a.rs"), PathBuf::from("src/b.rs")],
|
||||
),
|
||||
(
|
||||
"w2".to_string(),
|
||||
vec![PathBuf::from("src/b.rs"), PathBuf::from("src/c.rs")],
|
||||
),
|
||||
("w3".to_string(), vec![PathBuf::from("src/c.rs")]),
|
||||
];
|
||||
let overlaps = detect_overlaps(&per_worker);
|
||||
// b.rs touched by w1+w2; c.rs touched by w2+w3; a.rs only w1 (no overlap).
|
||||
assert_eq!(overlaps.len(), 2);
|
||||
assert_eq!(
|
||||
overlaps.get(&PathBuf::from("src/b.rs")).unwrap(),
|
||||
&vec!["w1".to_string(), "w2".to_string()]
|
||||
);
|
||||
assert_eq!(
|
||||
overlaps.get(&PathBuf::from("src/c.rs")).unwrap(),
|
||||
&vec!["w2".to_string(), "w3".to_string()]
|
||||
);
|
||||
assert!(!overlaps.contains_key(&PathBuf::from("src/a.rs")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn detect_overlaps_empty_when_disjoint() {
|
||||
let per_worker = vec![
|
||||
("w1".to_string(), vec![PathBuf::from("a.rs")]),
|
||||
("w2".to_string(), vec![PathBuf::from("b.rs")]),
|
||||
];
|
||||
assert!(detect_overlaps(&per_worker).is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn detect_overlaps_ignores_intra_worker_duplicates() {
|
||||
// A single worker listing the same file twice must not self-overlap.
|
||||
let per_worker = vec![(
|
||||
"w1".to_string(),
|
||||
vec![PathBuf::from("a.rs"), PathBuf::from("a.rs")],
|
||||
)];
|
||||
assert!(detect_overlaps(&per_worker).is_empty());
|
||||
}
|
||||
@@ -19,6 +19,18 @@ impl GitOperationsTool {
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve the working directory for git operations.
|
||||
///
|
||||
/// Returns the per-worker git-worktree override when one is installed via
|
||||
/// [`crate::openhuman::agent::harness::with_action_dir_override`] (an
|
||||
/// edit-capable worker running with `isolation = "worktree"`), otherwise
|
||||
/// the tool's configured `action_dir`. Keeping `self.action_dir` as the
|
||||
/// fallback preserves the non-isolated behaviour exactly. See #3376.
|
||||
fn effective_action_dir(&self) -> std::path::PathBuf {
|
||||
crate::openhuman::agent::harness::current_action_dir_override()
|
||||
.unwrap_or_else(|| self.action_dir.clone())
|
||||
}
|
||||
|
||||
/// Sanitize git arguments to prevent injection attacks
|
||||
fn sanitize_git_args(&self, args: &str) -> anyhow::Result<Vec<String>> {
|
||||
let mut result = Vec::new();
|
||||
@@ -68,7 +80,7 @@ impl GitOperationsTool {
|
||||
async fn run_git_command(&self, args: &[&str]) -> anyhow::Result<String> {
|
||||
let output = tokio::process::Command::new("git")
|
||||
.args(args)
|
||||
.current_dir(&self.action_dir)
|
||||
.current_dir(self.effective_action_dir())
|
||||
.output()
|
||||
.await?;
|
||||
|
||||
@@ -479,10 +491,12 @@ impl Tool for GitOperationsTool {
|
||||
}
|
||||
};
|
||||
|
||||
// Check if we're in a git repository
|
||||
if !self.action_dir.join(".git").exists() {
|
||||
// Check if we're in a git repository. A linked worktree's `.git` is a
|
||||
// file (a gitdir pointer), not a directory — `exists()` covers both.
|
||||
let effective_dir = self.effective_action_dir();
|
||||
if !effective_dir.join(".git").exists() {
|
||||
// Try to find .git in parent directories
|
||||
let mut current_dir = self.action_dir.as_path();
|
||||
let mut current_dir = effective_dir.as_path();
|
||||
let mut found_git = false;
|
||||
while current_dir.parent().is_some() {
|
||||
if current_dir.join(".git").exists() {
|
||||
|
||||
@@ -140,6 +140,18 @@ impl ShellTool {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve the working directory for this shell invocation.
|
||||
///
|
||||
/// Returns the per-worker git-worktree override when one is installed via
|
||||
/// [`crate::openhuman::agent::harness::with_action_dir_override`] (an
|
||||
/// edit-capable worker running with `isolation = "worktree"`), otherwise
|
||||
/// the shared `self.security.action_dir`. Keeping `security.action_dir` as
|
||||
/// the fallback preserves the non-isolated behaviour exactly. See #3376.
|
||||
fn effective_action_dir(&self) -> std::path::PathBuf {
|
||||
crate::openhuman::agent::harness::current_action_dir_override()
|
||||
.unwrap_or_else(|| self.security.action_dir.clone())
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
@@ -279,7 +291,7 @@ impl ShellTool {
|
||||
// (CWE-200), then re-add only safe, functional variables.
|
||||
let mut cmd = match self
|
||||
.runtime
|
||||
.build_shell_command(command, &self.security.action_dir)
|
||||
.build_shell_command(command, &self.effective_action_dir())
|
||||
{
|
||||
Ok(cmd) => cmd,
|
||||
Err(e) => {
|
||||
@@ -383,7 +395,7 @@ impl ShellTool {
|
||||
let config = crate::openhuman::config::RuntimeConfig::default();
|
||||
let policy = sandbox::resolve_sandbox_policy(
|
||||
crate::openhuman::agent::harness::definition::SandboxMode::Sandboxed,
|
||||
&self.security.action_dir,
|
||||
&self.effective_action_dir(),
|
||||
&config,
|
||||
false,
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user