mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-09 22:33:37 +00:00
fix: cover ACP prompt workflows and exit diagnostics
This commit is contained in:
parent
a9ef7ffd77
commit
e1d5b925e4
9 changed files with 138 additions and 23 deletions
|
|
@ -129,8 +129,12 @@ impl ConnectTo<Client> for SandboxAcpTransport {
|
|||
termination = handle.wait() => {
|
||||
let termination = termination.map_err(ProtocolError::into_internal_error)?;
|
||||
let stderr = stderr.tail_string().await;
|
||||
let exit_code = termination
|
||||
.exit_code
|
||||
.map_or_else(|| "unknown".to_string(), |code| code.to_string());
|
||||
Err(internal_error(format!(
|
||||
"ACP process exited before protocol completed: termination={termination:?}, stderr={stderr}"
|
||||
"ACP process exited before protocol completed: termination={}, exit_code={exit_code}, stderr={stderr}",
|
||||
termination.termination,
|
||||
)))
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -309,7 +309,18 @@ async fn early_exit_returns_protocol_error_with_stderr() {
|
|||
.await
|
||||
.expect_err("early exit should error");
|
||||
|
||||
assert!(matches!(err, AcpError::Protocol(_)));
|
||||
let AcpError::Protocol(error) = err else {
|
||||
panic!("expected protocol error");
|
||||
};
|
||||
let message = error.to_string();
|
||||
assert!(
|
||||
message.contains("exit_code=2"),
|
||||
"early exit should include exit code in diagnostic: {message}"
|
||||
);
|
||||
assert!(
|
||||
message.contains("early boom"),
|
||||
"early exit should include stderr tail in diagnostic: {message}"
|
||||
);
|
||||
}
|
||||
|
||||
async fn run_fake_agent(
|
||||
|
|
|
|||
|
|
@ -4,6 +4,6 @@
|
|||
pub use fabro_sandbox::{
|
||||
CommandOutputCallback, DirEntry, ExecResult, ExecStreamingResult, GrepOptions, Sandbox,
|
||||
SandboxEvent, SandboxEventCallback, StderrCollector, StdioProcess, StdioProcessHandle,
|
||||
WorktreeEvent, WorktreeEventCallback, WorktreeOptions, WorktreeSandbox, delegate_sandbox,
|
||||
format_lines_numbered, shell_quote,
|
||||
StdioProcessTermination, WorktreeEvent, WorktreeEventCallback, WorktreeOptions,
|
||||
WorktreeSandbox, delegate_sandbox, format_lines_numbered, shell_quote,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -85,6 +85,77 @@ fn acp_backend_workflow() {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn acp_prompt_workflow_uses_acp_backend() {
|
||||
let mut context = test_context!();
|
||||
context.write_home(
|
||||
".fabro/settings.toml",
|
||||
"[server.auth]\nmethods = [\"dev-token\"]\n",
|
||||
);
|
||||
context.isolated_server();
|
||||
seed_openai_vault(&context.storage_dir);
|
||||
let fake_agent = fixture("fake_acp_agent.py");
|
||||
let workflow = context.temp_dir.join("acp_prompt_backend.fabro");
|
||||
context.write_temp(
|
||||
"acp_prompt_backend.fabro",
|
||||
format!(
|
||||
r#"digraph ACP {{
|
||||
graph [goal="Exercise ACP prompt backend"]
|
||||
start [shape=Mdiamond]
|
||||
prompt [type="prompt", backend="acp", provider="openai", model="fake-acp", project_memory=false, prompt="write hello.txt", acp_command="python3 {}"]
|
||||
exit [shape=Msquare]
|
||||
start -> prompt
|
||||
prompt -> exit
|
||||
}}"#,
|
||||
fake_agent.display()
|
||||
),
|
||||
);
|
||||
init_git_repo(&context.temp_dir);
|
||||
|
||||
context
|
||||
.run_cmd()
|
||||
.args(["--auto-approve", "--sandbox", "local"])
|
||||
.arg(&workflow)
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
let run_dir = find_run_dir(&context);
|
||||
let conclusion = read_conclusion(&run_dir);
|
||||
assert_eq!(conclusion["status"].as_str(), Some("succeeded"));
|
||||
|
||||
let events = run_events(&run_dir);
|
||||
assert!(has_event(&run_dir, "agent.acp.started"));
|
||||
assert!(has_event(&run_dir, "agent.acp.completed"));
|
||||
assert!(
|
||||
!has_event(&run_dir, "agent.session.activated"),
|
||||
"ACP prompt should not activate an API-mode agent session"
|
||||
);
|
||||
let completed = events
|
||||
.iter()
|
||||
.find_map(|event| match &event.event.body {
|
||||
EventBody::StageCompleted(props)
|
||||
if event.event.node_id.as_deref() == Some("prompt") =>
|
||||
{
|
||||
Some(props)
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.expect("prompt stage should complete");
|
||||
assert_eq!(completed.response.as_deref(), Some("hello from acp"));
|
||||
|
||||
let state = serde_json::to_value(run_state(&run_dir)).expect("run state should serialize");
|
||||
let stages = state["stages"]
|
||||
.as_object()
|
||||
.expect("run state should contain stages");
|
||||
assert!(
|
||||
stages.values().any(|stage| {
|
||||
stage["provider_used"]["mode"] == "acp"
|
||||
&& stage["provider_used"]["provider"] == "openai"
|
||||
}),
|
||||
"run projection should include ACP provider metadata: {stages:?}"
|
||||
);
|
||||
}
|
||||
|
||||
fn seed_openai_vault(storage_dir: &std::path::Path) {
|
||||
let mut vault =
|
||||
Vault::load(Storage::new(storage_dir).secrets_path()).expect("test vault should load");
|
||||
|
|
|
|||
|
|
@ -30,7 +30,7 @@ use crate::sandbox::{StdioProcessControl, optional_timeout, resolve_path};
|
|||
use crate::{
|
||||
CommandOutputCallback, DEFAULT_EXEC_OUTPUT_TAIL_BYTES, DirEntry, ExecResult,
|
||||
ExecStreamingResult, GrepOptions, Sandbox, SandboxEvent, SandboxEventCallback, StderrCollector,
|
||||
StdioProcess, StdioProcessHandle, format_lines_numbered, shell_quote,
|
||||
StdioProcess, StdioProcessHandle, StdioProcessTermination, format_lines_numbered, shell_quote,
|
||||
};
|
||||
|
||||
const WORKING_DIRECTORY: &str = "/workspace";
|
||||
|
|
@ -882,7 +882,7 @@ struct DockerStdioProcessControl {
|
|||
container_id: String,
|
||||
exec_id: String,
|
||||
stop_file: String,
|
||||
termination: TokioMutex<Option<CommandTermination>>,
|
||||
termination: TokioMutex<Option<StdioProcessTermination>>,
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
|
|
@ -892,11 +892,11 @@ impl StdioProcessControl for DockerStdioProcessControl {
|
|||
return Ok(());
|
||||
}
|
||||
request_docker_exec_stop_with(&self.docker, &self.container_id, &self.stop_file).await?;
|
||||
*self.termination.lock().await = Some(CommandTermination::Cancelled);
|
||||
*self.termination.lock().await = Some(StdioProcessTermination::cancelled());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn wait(&self) -> crate::Result<CommandTermination> {
|
||||
async fn wait(&self) -> crate::Result<StdioProcessTermination> {
|
||||
if let Some(termination) = *self.termination.lock().await {
|
||||
return Ok(termination);
|
||||
}
|
||||
|
|
@ -908,8 +908,10 @@ impl StdioProcessControl for DockerStdioProcessControl {
|
|||
.await
|
||||
.map_err(|e| crate::Error::context("Failed to inspect Docker stdio exec", e))?;
|
||||
if inspect.running != Some(true) {
|
||||
*self.termination.lock().await = Some(CommandTermination::Exited);
|
||||
return Ok(CommandTermination::Exited);
|
||||
let exit_code = inspect.exit_code.and_then(|code| i32::try_from(code).ok());
|
||||
let termination = StdioProcessTermination::exited(exit_code);
|
||||
*self.termination.lock().await = Some(termination);
|
||||
return Ok(termination);
|
||||
}
|
||||
time::sleep(std::time::Duration::from_millis(50)).await;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -41,8 +41,9 @@ pub use reconnect::{reconnect, reconnect_for_run, reconnect_for_run_with_callbac
|
|||
pub use sandbox::{
|
||||
CommandOutputCallback, DEFAULT_EXEC_OUTPUT_TAIL_BYTES, DirEntry, ExecResult,
|
||||
ExecStreamingResult, GitRunInfo, GitSetupIntent, GrepOptions, Sandbox, SandboxEvent,
|
||||
SandboxEventCallback, StderrCollector, StdioProcess, StdioProcessHandle, format_lines_numbered,
|
||||
git_push_via_exec, redacted_output_tail, setup_git_via_exec, shell_quote,
|
||||
SandboxEventCallback, StderrCollector, StdioProcess, StdioProcessHandle,
|
||||
StdioProcessTermination, format_lines_numbered, git_push_via_exec, redacted_output_tail,
|
||||
setup_git_via_exec, shell_quote,
|
||||
};
|
||||
pub use sandbox_spec::SandboxSpec;
|
||||
pub use terminal::{TerminalSession, TerminalSize, open_terminal_for_run};
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ use crate::sandbox::{StdioProcessControl, optional_timeout};
|
|||
use crate::{
|
||||
CommandOutputCallback, DEFAULT_EXEC_OUTPUT_TAIL_BYTES, DirEntry, ExecResult,
|
||||
ExecStreamingResult, GrepOptions, Sandbox, SandboxEvent, SandboxEventCallback, StderrCollector,
|
||||
StdioProcess, StdioProcessHandle, format_lines_numbered,
|
||||
StdioProcess, StdioProcessHandle, StdioProcessTermination, format_lines_numbered,
|
||||
};
|
||||
|
||||
pub struct LocalSandbox {
|
||||
|
|
@ -145,7 +145,7 @@ where
|
|||
|
||||
struct LocalStdioProcessControl {
|
||||
child: TokioMutex<Child>,
|
||||
termination: TokioMutex<Option<CommandTermination>>,
|
||||
termination: TokioMutex<Option<StdioProcessTermination>>,
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
|
|
@ -157,22 +157,23 @@ impl StdioProcessControl for LocalStdioProcessControl {
|
|||
|
||||
let mut child = self.child.lock().await;
|
||||
sigterm_then_kill(&mut child).await;
|
||||
*self.termination.lock().await = Some(CommandTermination::Cancelled);
|
||||
*self.termination.lock().await = Some(StdioProcessTermination::cancelled());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn wait(&self) -> crate::Result<CommandTermination> {
|
||||
async fn wait(&self) -> crate::Result<StdioProcessTermination> {
|
||||
if let Some(termination) = *self.termination.lock().await {
|
||||
return Ok(termination);
|
||||
}
|
||||
|
||||
let mut child = self.child.lock().await;
|
||||
child
|
||||
let status = child
|
||||
.wait()
|
||||
.await
|
||||
.map_err(|e| crate::Error::context("Failed to wait for stdio process", e))?;
|
||||
*self.termination.lock().await = Some(CommandTermination::Exited);
|
||||
Ok(CommandTermination::Exited)
|
||||
let termination = StdioProcessTermination::exited(status.code());
|
||||
*self.termination.lock().await = Some(termination);
|
||||
Ok(termination)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -754,15 +754,39 @@ impl StdioProcessHandle {
|
|||
self.control.terminate().await
|
||||
}
|
||||
|
||||
pub async fn wait(&self) -> crate::Result<CommandTermination> {
|
||||
pub async fn wait(&self) -> crate::Result<StdioProcessTermination> {
|
||||
self.control.wait().await
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct StdioProcessTermination {
|
||||
pub termination: CommandTermination,
|
||||
pub exit_code: Option<i32>,
|
||||
}
|
||||
|
||||
impl StdioProcessTermination {
|
||||
#[must_use]
|
||||
pub fn exited(exit_code: Option<i32>) -> Self {
|
||||
Self {
|
||||
termination: CommandTermination::Exited,
|
||||
exit_code,
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn cancelled() -> Self {
|
||||
Self {
|
||||
termination: CommandTermination::Cancelled,
|
||||
exit_code: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
pub(crate) trait StdioProcessControl: Send + Sync {
|
||||
async fn terminate(&self) -> crate::Result<()>;
|
||||
async fn wait(&self) -> crate::Result<CommandTermination>;
|
||||
async fn wait(&self) -> crate::Result<StdioProcessTermination>;
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ use crate::sandbox::StdioProcessControl;
|
|||
use crate::{
|
||||
DEFAULT_EXEC_OUTPUT_TAIL_BYTES, DirEntry, ExecResult, GrepOptions, Sandbox, SandboxEvent,
|
||||
SandboxEventCallback, StderrCollector, StdioProcess, StdioProcessHandle,
|
||||
StdioProcessTermination,
|
||||
};
|
||||
|
||||
// --- MockSandbox ---
|
||||
|
|
@ -117,8 +118,8 @@ impl StdioProcessControl for MockStdioProcessControl {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
async fn wait(&self) -> crate::Result<CommandTermination> {
|
||||
Ok(CommandTermination::Exited)
|
||||
async fn wait(&self) -> crate::Result<StdioProcessTermination> {
|
||||
Ok(StdioProcessTermination::exited(Some(0)))
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue