tkmind/tkmind_go
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

test(acp): add terminal delegation fixtures and fix shell singleton (#7923)

Browse Source 
Signed-off-by: Adrian Cole <adrian@tetrate.io>
This commit is contained in:
Adrian Cole
2026-03-16 22:02:08 +08:00
committed by GitHub
parent 5df2995695
commit 94eaacc06f
12 changed files with 727 additions and 142 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/goose/src/acp/provider.rs
+5 -1
View File
@@ -28,7 +28,6 @@ use crate::permission::{Permission, PermissionConfirmation};
use crate::providers::base::{MessageStream, PermissionRouting, Provider};
use crate::providers::errors::ProviderError;
#[derive(Clone, Debug)]
pub struct AcpProviderConfig {
pub command: PathBuf,
pub args: Vec<String>,
@@ -38,6 +37,7 @@ pub struct AcpProviderConfig {
pub mcp_servers: Vec<McpServer>,
pub session_mode_id: Option<String>,
pub permission_mapping: PermissionMapping,
pub notification_callback: Option<Arc<dyn Fn(SessionNotification) + Send + Sync>>,
}
enum ClientRequest {
@@ -522,12 +522,16 @@ impl AcpClientLoop {
goose_mode,
prompt_response_tx,
} = self;
let notification_callback = config.notification_callback.clone();
ClientToAgent::builder()
.on_receive_notification(
{
let prompt_response_tx = prompt_response_tx.clone();
async move |notification: SessionNotification, _cx| {
if let Some(ref cb) = notification_callback {
cb(notification.clone());
}
// stream() reads goose_mode at call time, so it must
// reflect any prior set_mode before the next prompt.
if let SessionUpdate::CurrentModeUpdate(update) = &notification.update {
crates/goose/src/agents/platform_extensions/developer/shell.rs
+69 -32
View File
@@ -1,7 +1,7 @@
use std::path::PathBuf;
use std::process::Stdio;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::OnceLock;
use std::sync::LazyLock;
use std::time::Duration;
use rmcp::model::{CallToolResult, Content};
@@ -59,38 +59,51 @@ fn resolve_login_shell_path() -> Option<String> {
std::env::var("SHELL").unwrap_or_else(|_| "sh".to_string())
};
std::process::Command::new(&shell)
let mut child = std::process::Command::new(&shell)
.args(["-l", "-i", "-c", "echo $PATH"])
.stdin(Stdio::null())
.stdout(Stdio::piped())
.stderr(Stdio::null())
.output()
.ok()
.and_then(|output| {
if output.status.success() {
// Take the last non-empty line — interactive shells may emit
// extra output from profile scripts before our echo.
String::from_utf8_lossy(&output.stdout)
.lines()
.rev()
.find(|line| !line.trim().is_empty())
.map(|line| line.trim().to_string())
.filter(|path| !path.is_empty())
} else {
None
}
})
.spawn()
.ok()?;
let mut stdout = child.stdout.take()?;
let (tx, rx) = std::sync::mpsc::channel();
std::thread::spawn(move || {
let mut buf = Vec::new();
use std::io::Read;
if stdout.read_to_end(&mut buf).is_ok() {
let _ = tx.send(buf);
}
});
match rx.recv_timeout(Duration::from_secs(5)) {
Ok(buf) if child.wait().is_ok_and(|s| s.success()) => {
// Take the last non-empty line — interactive shells may emit
// extra output from profile scripts before our echo.
String::from_utf8_lossy(&buf)
.lines()
.rev()
.find(|line| !line.trim().is_empty())
.map(|line| line.trim().to_string())
.filter(|path| !path.is_empty())
}
_ => {
let _ = child.kill();
let _ = child.wait();
None
}
}
}
/// Returns the user's full login shell PATH, resolved once and cached.
#[cfg(not(windows))]
fn user_login_path() -> Option<&'static str> {
static CACHED: OnceLock<Option<String>> = OnceLock::new();
CACHED.get_or_init(resolve_login_shell_path).as_deref()
}
static LOGIN_PATH: LazyLock<Option<String>> = LazyLock::new(resolve_login_shell_path);
pub struct ShellTool {
output_dir: tempfile::TempDir,
call_index: AtomicUsize,
#[cfg(not(windows))]
login_path: Option<String>,
}
impl ShellTool {
@@ -98,6 +111,18 @@ impl ShellTool {
Ok(Self {
output_dir: tempfile::tempdir()?,
call_index: AtomicUsize::new(0),
#[cfg(not(windows))]
login_path: LOGIN_PATH.clone(),
})
}
#[cfg(test)]
pub fn new_for_test() -> std::io::Result<Self> {
Ok(Self {
output_dir: tempfile::tempdir()?,
call_index: AtomicUsize::new(0),
#[cfg(not(windows))]
login_path: None,
})
}
@@ -114,7 +139,19 @@ impl ShellTool {
return Self::error_result("Command cannot be empty.", None);
}
let execution = match run_command(&params.command, params.timeout_secs, working_dir).await {
#[cfg(not(windows))]
let login_path = self.login_path.as_deref();
#[cfg(windows)]
let login_path: Option<&str> = None;
let execution = match run_command(
&params.command,
params.timeout_secs,
working_dir,
login_path,
)
.await
{
Ok(execution) => execution,
Err(error) => return Self::error_result(&error, None),
};
@@ -226,14 +263,14 @@ async fn run_command(
command_line: &str,
timeout_secs: Option<u64>,
working_dir: Option<&std::path::Path>,
login_path: Option<&str>,
) -> Result<ExecutionOutput, String> {
let mut command = build_shell_command(command_line);
if let Some(path) = working_dir {
command.current_dir(path);
}
#[cfg(not(windows))]
if let Some(path) = user_login_path() {
if let Some(path) = login_path {
command.env("PATH", path);
}
@@ -468,7 +505,7 @@ mod tests {
#[tokio::test]
async fn shell_executes_command() {
let tool = ShellTool::new().unwrap();
let tool = ShellTool::new_for_test().unwrap();
let result = tool
.shell(ShellParams {
command: "echo hello".to_string(),
@@ -483,7 +520,7 @@ mod tests {
#[cfg(not(windows))]
#[tokio::test]
async fn shell_returns_error_for_non_zero_exit() {
let tool = ShellTool::new().unwrap();
let tool = ShellTool::new_for_test().unwrap();
let result = tool
.shell(ShellParams {
command: "echo fail && exit 7".to_string(),
@@ -499,7 +536,7 @@ mod tests {
#[tokio::test]
async fn shell_uses_working_dir_for_relative_execution() {
let dir = tempfile::tempdir().unwrap();
let tool = ShellTool::new().unwrap();
let tool = ShellTool::new_for_test().unwrap();
let result = tool
.shell_with_cwd(
ShellParams {
@@ -598,7 +635,7 @@ mod tests {
#[test]
fn call_index_cycles_through_slots() {
let tool = ShellTool::new().unwrap();
let tool = ShellTool::new_for_test().unwrap();
for _cycle in 0..3 {
for expected in 0..OUTPUT_SLOTS {
let slot = tool.call_index.fetch_add(1, Ordering::Relaxed) % OUTPUT_SLOTS;
@@ -609,7 +646,7 @@ mod tests {
#[test]
fn concurrent_calls_get_distinct_slots() {
let tool = ShellTool::new().unwrap();
let tool = ShellTool::new_for_test().unwrap();
let mut slots: Vec<usize> = (0..OUTPUT_SLOTS)
.map(|_| tool.call_index.fetch_add(1, Ordering::Relaxed) % OUTPUT_SLOTS)
.collect();
@@ -630,7 +667,7 @@ mod tests {
}
}
let tool = ShellTool::new().unwrap();
let tool = ShellTool::new_for_test().unwrap();
let start = std::time::Instant::now();
let result = tool
.shell(ShellParams {
crates/goose/src/providers/claude_acp.rs
+1
View File
@@ -61,6 +61,7 @@ impl ProviderDef for ClaudeAcpProvider {
mcp_servers: extension_configs_to_mcp_servers(&extensions),
session_mode_id: Some(map_goose_mode(goose_mode)),
permission_mapping,
notification_callback: None,
};
let metadata = Self::metadata();
crates/goose/src/providers/codex_acp.rs
+1
View File
@@ -84,6 +84,7 @@ impl ProviderDef for CodexAcpProvider {
// Disabled until https://github.com/zed-industries/codex-acp/issues/179 is fixed.
session_mode_id: None,
permission_mapping,
notification_callback: None,
};
let metadata = Self::metadata();