mirror of
https://github.com/aaif-goose/goose.git
synced 2026-07-03 14:10:03 +02:00
feat(summon): add working_dir override for delegate tasks (#9520)
Signed-off-by: Douwe M Osinga <douwe@sidewalklabs.com> Co-authored-by: Douwe M Osinga <douwe@sidewalklabs.com>
This commit is contained in:
@@ -58,6 +58,7 @@ pub struct DelegateParams {
|
||||
pub model: Option<String>,
|
||||
pub temperature: Option<f32>,
|
||||
pub max_turns: Option<usize>,
|
||||
pub working_dir: Option<String>,
|
||||
#[serde(default)]
|
||||
pub r#async: bool,
|
||||
}
|
||||
@@ -534,6 +535,10 @@ impl SummonClient {
|
||||
"minimum": 1,
|
||||
"description": "Maximum turns for this delegate. Overrides recipe settings.max_turns and GOOSE_SUBAGENT_MAX_TURNS."
|
||||
},
|
||||
"working_dir": {
|
||||
"type": "string",
|
||||
"description": "Working directory for the delegate. Must be within the parent session's working directory. Defaults to the parent's working directory."
|
||||
},
|
||||
"async": {
|
||||
"type": "boolean",
|
||||
"default": false,
|
||||
@@ -1096,7 +1101,7 @@ impl SummonClient {
|
||||
.context
|
||||
.session_manager
|
||||
.create_session(
|
||||
working_dir,
|
||||
task_config.parent_working_dir.clone(),
|
||||
"Delegated task".to_string(),
|
||||
SessionType::SubAgent,
|
||||
GooseMode::Auto,
|
||||
@@ -1212,7 +1217,7 @@ impl SummonClient {
|
||||
return Err(format!(
|
||||
"Source '{}' has kind '{}' which cannot be delegated from summon",
|
||||
source_name, source.source_type
|
||||
))
|
||||
));
|
||||
}
|
||||
};
|
||||
|
||||
@@ -1400,8 +1405,14 @@ impl SummonClient {
|
||||
);
|
||||
}
|
||||
|
||||
let task_config = TaskConfig::new(provider, &session.id, &session.working_dir, extensions)
|
||||
.with_max_turns(Some(max_turns));
|
||||
let effective_working_dir = match ¶ms.working_dir {
|
||||
Some(dir) => resolve_working_dir(&session.working_dir, dir)?,
|
||||
None => session.working_dir.clone(),
|
||||
};
|
||||
|
||||
let task_config =
|
||||
TaskConfig::new(provider, &session.id, &effective_working_dir, extensions)
|
||||
.with_max_turns(Some(max_turns));
|
||||
|
||||
Ok(task_config)
|
||||
}
|
||||
@@ -1612,7 +1623,7 @@ impl SummonClient {
|
||||
.context
|
||||
.session_manager
|
||||
.create_session(
|
||||
working_dir,
|
||||
task_config.parent_working_dir.clone(),
|
||||
description.clone(),
|
||||
SessionType::SubAgent,
|
||||
GooseMode::Auto,
|
||||
@@ -1830,6 +1841,34 @@ impl McpClientTrait for SummonClient {
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve a requested `working_dir` override against the parent session
|
||||
/// directory. Relative paths are joined to the parent dir; the result must
|
||||
/// canonicalize to an existing directory contained within the parent dir.
|
||||
fn resolve_working_dir(parent_dir: &Path, requested: &str) -> Result<PathBuf, anyhow::Error> {
|
||||
let requested_path = PathBuf::from(requested);
|
||||
let resolved = if requested_path.is_absolute() {
|
||||
requested_path
|
||||
} else {
|
||||
parent_dir.join(&requested_path)
|
||||
};
|
||||
let canonical = resolved
|
||||
.canonicalize()
|
||||
.map_err(|e| anyhow::anyhow!("working_dir '{}' could not be resolved: {}", requested, e))?;
|
||||
let parent_canonical = parent_dir
|
||||
.canonicalize()
|
||||
.unwrap_or_else(|_| parent_dir.to_path_buf());
|
||||
if !canonical.starts_with(&parent_canonical) {
|
||||
anyhow::bail!(
|
||||
"working_dir '{}' is outside the parent session directory",
|
||||
requested
|
||||
);
|
||||
}
|
||||
if !canonical.is_dir() {
|
||||
anyhow::bail!("working_dir '{}' is not a directory", requested);
|
||||
}
|
||||
Ok(canonical)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -1860,6 +1899,58 @@ You review code."#;
|
||||
assert!(source.description.contains("sonnet"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_resolve_working_dir_relative_subdir() {
|
||||
let temp_dir = TempDir::new().unwrap();
|
||||
let parent = temp_dir.path().canonicalize().unwrap();
|
||||
let subdir = parent.join("sub");
|
||||
fs::create_dir(&subdir).unwrap();
|
||||
|
||||
let resolved = resolve_working_dir(&parent, "sub").unwrap();
|
||||
assert_eq!(resolved, subdir.canonicalize().unwrap());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_resolve_working_dir_rejects_traversal_outside_parent() {
|
||||
let temp_dir = TempDir::new().unwrap();
|
||||
let parent = temp_dir.path().join("parent");
|
||||
let sibling = temp_dir.path().join("sibling");
|
||||
fs::create_dir(&parent).unwrap();
|
||||
fs::create_dir(&sibling).unwrap();
|
||||
|
||||
let err = resolve_working_dir(&parent, "../sibling").unwrap_err();
|
||||
assert!(
|
||||
err.to_string()
|
||||
.contains("outside the parent session directory"),
|
||||
"unexpected error: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_resolve_working_dir_rejects_file_path() {
|
||||
let temp_dir = TempDir::new().unwrap();
|
||||
let parent = temp_dir.path().canonicalize().unwrap();
|
||||
let file = parent.join("a.txt");
|
||||
fs::write(&file, "hello").unwrap();
|
||||
|
||||
let err = resolve_working_dir(&parent, "a.txt").unwrap_err();
|
||||
assert!(
|
||||
err.to_string().contains("is not a directory"),
|
||||
"unexpected error: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_resolve_working_dir_rejects_nonexistent_path() {
|
||||
let temp_dir = TempDir::new().unwrap();
|
||||
let parent = temp_dir.path().canonicalize().unwrap();
|
||||
|
||||
let err = resolve_working_dir(&parent, "does-not-exist").unwrap_err();
|
||||
assert!(
|
||||
err.to_string().contains("could not be resolved"),
|
||||
"unexpected error: {err}"
|
||||
);
|
||||
}
|
||||
#[test]
|
||||
fn test_agent_scan_skips_non_agent_markdown() {
|
||||
let temp_dir = TempDir::new().unwrap();
|
||||
|
||||
Reference in New Issue
Block a user