Merge remote-tracking branch 'origin/main' into one-usage-type

# Conflicts:
#	Cargo.lock
#	Cargo.toml
This commit is contained in:
Bryan Helmkamp 2026-09-14 12:40:49 -06:00
commit e9ee0aaeaa
No known key found for this signature in database
6 changed files with 407 additions and 31 deletions

View file

@ -1198,7 +1198,8 @@ pub(crate) struct AgentArgs {
#[arg(long)]
pub(crate) debug: bool,
/// Print full LLM request/response JSON to stderr
/// Print tool results, the transcript, and full LLM request/response JSON
/// to stderr
#[arg(long)]
pub(crate) verbose: bool,

View file

@ -2,9 +2,12 @@
//!
//! The session is pebble's coding agent over a local sandbox, run through
//! pebble's own command-line session: its event renderer, closing summary,
//! and terminal approval prompt. What is fabro's here is the client (model
//! calls go either straight to the provider with the CLI's credentials or
//! through a Fabro server's completions endpoint when a server target is
//! and terminal approval prompt. `--verbose` asks that renderer for each
//! tool call's arguments and result in full and for the transcript, and adds
//! fabro's own dump of every model request and response; without it the
//! renderer prints what it always has. What is fabro's here is the client
//! (model calls go either straight to the provider with the CLI's credentials
//! or through a Fabro server's completions endpoint when a server target is
//! set), the sandbox, the MCP servers, skills, search, and redaction.
use std::collections::HashMap;
@ -31,8 +34,8 @@ use fabro_util::terminal::Styles;
use fabro_workflow::web_search::{self, SearchSecrets};
use lithos_llm::catalog::ProviderId;
use pebble_cli_core::approval::TerminalApproval;
use pebble_cli_core::render::{self, JsonStream, Style};
use pebble_cli_core::session::{SessionOptions, run_prompt};
use pebble_cli_core::render::{self, JsonStream, RenderOptions, Style};
use pebble_cli_core::session::{SessionOptions, run_prompt_with};
use pebble_coding_agent::environment::Environment;
use pebble_coding_agent::subagents::SubagentOptions;
use pebble_coding_agent::tools::{PermissionLevelPolicy, PermissionMiddleware};
@ -497,7 +500,16 @@ async fn run_session(
sigint_token.cancel();
});
let report = run_prompt(agent, args.prompt.as_str(), &cancel_token, session).await?;
// `--verbose` asks the renderer for everything it can say: each tool
// call's arguments and result in full, and the transcript. The answer on
// stdout is the session's and is not changed by it.
let render = if args.verbose {
RenderOptions::verbose()
} else {
RenderOptions::default()
};
let report =
run_prompt_with(agent, args.prompt.as_str(), &cancel_token, session, render).await?;
report
.result
.map(|_| ())

View file

@ -260,7 +260,7 @@ fn help() {
--quiet Suppress non-essential output [env: FABRO_QUIET=]
--auto-approve Skip interactive prompts; deny tools outside permission level
--debug Print LLM request/response debug info to stderr
--verbose Print full LLM request/response JSON to stderr
--verbose Print tool results, the transcript, and full LLM request/response JSON to stderr
--skills-dir <SKILLS_DIR> Directory containing skill files (overrides default discovery)
--output-format <OUTPUT_FORMAT> Output format (text for human-readable, json for NDJSON event stream) [possible values: text, json]
-h, --help Print help
@ -914,7 +914,10 @@ async fn twin_exec_shell_command() {
.input_contains(
"Run the shell command `echo hello_from_shell` and tell me what it printed",
)
.tool_call(fabro_test::TwinToolCall::shell("echo hello_from_shell"))
.tool_call(fabro_test::TwinToolCall::shell("echo hello_from_shell")),
)
.scenario(
fabro_test::TwinScenario::responses("gpt-5.4-mini")
.text("It printed hello_from_shell."),
)
.load(twin)
@ -934,9 +937,80 @@ async fn twin_exec_shell_command() {
]);
let output = run_success_output(cmd).await;
let stdout = String::from_utf8(output.stdout).expect("valid utf8");
let stderr = String::from_utf8(output.stderr).expect("valid utf8");
let stderr = console::strip_ansi_codes(&stderr);
assert_eq!(
stdout, "It printed hello_from_shell.\n",
"without --verbose, stdout carries the final answer only; stderr:\n{stderr}"
);
for absent in ["[result]", "[reasoning]", "[verbose]"] {
assert!(
!stderr.contains(absent),
"without --verbose, stderr should not carry {absent}:\n{stderr}"
);
}
}
#[fabro_macros::e2e_test(twin)]
async fn twin_exec_verbose_prints_tool_calls_and_results() {
let context = test_context!();
let twin = fabro_test::twin_openai().await;
let namespace = format!("{}::{}", module_path!(), line!());
fabro_test::TwinScenarios::new(namespace.clone())
.scenario(
fabro_test::TwinScenario::responses("gpt-5.4-mini")
.input_contains(
"Run the shell command `echo verbose_marker` and tell me what it printed",
)
.tool_call(fabro_test::TwinToolCall::shell("echo verbose_marker")),
)
.scenario(
fabro_test::TwinScenario::responses("gpt-5.4-mini").text("It printed verbose_marker."),
)
.load(twin)
.await;
let mut cmd = context.exec_cmd();
twin.configure_command(&mut cmd, &namespace);
cmd.args([
"--auto-approve",
"--permissions",
"full",
"--verbose",
"--provider",
"openai",
"--model",
"gpt-5.4-mini",
"Run the shell command `echo verbose_marker` and tell me what it printed",
]);
let output = run_success_output(cmd).await;
let stdout = String::from_utf8(output.stdout).expect("valid utf8");
let stderr = String::from_utf8(output.stderr).expect("valid utf8");
let stderr = console::strip_ansi_codes(&stderr);
assert_eq!(
stdout, "It printed verbose_marker.\n",
"--verbose should leave the answer on stdout unchanged; stderr:\n{stderr}"
);
// Pebble prints the call's arguments in full under its `[tool]` line and
// what the call answered under a `[result]` line; fabro's middleware
// still dumps each model request. (A streamed response is not dumped.)
for expected in [
"[tool] shell\n",
"\"command\": \"echo verbose_marker\"",
"[result] shell\n",
"[verbose] request:",
] {
assert!(
stderr.contains(expected),
"--verbose should print {expected:?} on stderr, got:\n{stderr}"
);
}
let (_, result) = stderr
.split_once("[result] shell\n")
.expect("the result block should follow the tool line");
assert!(
stdout.contains("hello_from_shell"),
"expected shell marker in output, got: {stdout}"
result.contains("verbose_marker"),
"the [result] block should carry what the shell printed, got:\n{result}"
);
}

View file

@ -29,6 +29,10 @@ use crate::driver_sandbox::RunSandbox;
use crate::managed_labels::{MANAGED_LABEL, MANAGED_LABEL_VALUE};
use crate::sandbox::SandboxFile;
mod deleted_on_drop;
pub use deleted_on_drop::DeletedOnDrop;
/// The id a run record carries for a local sandbox at `working_directory`,
/// as the Host provider derives it from the canonical path. A record a test
/// writes by hand reconnects the way one fabro wrote would. The directory

View file

@ -0,0 +1,281 @@
//! A sandbox a test deletes even when it fails.
//!
//! A live test that creates a provider sandbox and deletes it on its last
//! line leaks a running (and billed) sandbox whenever it panics or fails an
//! assertion before that line. [`DeletedOnDrop`] owns the sandbox for the
//! test: the happy path still calls `delete` explicitly, and any other exit
//! deletes it from `Drop`.
//!
//! `Drop` is synchronous and may run while the test's runtime is unwinding
//! a panic, so the cleanup never uses that runtime: it spawns a thread with
//! a small runtime of its own and blocks until the delete finishes or a
//! bounded timeout passes. A live provider's handle cannot be driven from
//! that thread either, because its pooled HTTP connections are tasks on the
//! test's runtime, which nobody polls while it unwinds. The guard therefore
//! connects the provider afresh through the [`ProviderAccess`] the test
//! built the sandbox with and deletes the sandbox by id over that new
//! connection.
use std::fmt;
use std::ops::Deref;
use std::sync::Arc;
use std::time::Duration;
use fabro_types::SandboxProviderKind;
use tokio::runtime::Builder as RuntimeBuilder;
use tokio::time;
use crate::driver::ProviderAccess;
use crate::driver_sandbox::RunSandbox;
use crate::error::display_for_log;
use crate::provider_sandbox;
/// How long a drop-time delete may take before the guard gives up and
/// reports the sandbox as possibly leaked. Daytona's driver bounds each
/// delete call at 10s and may wait out a state change once; a reconnect
/// adds a few seconds of its own.
const DROP_DELETE_TIMEOUT: Duration = Duration::from_secs(90);
/// A run sandbox that is deleted when the guard drops, unless the test
/// deleted it explicitly through [`DeletedOnDrop::delete`].
///
/// Derefs to the [`RunSandbox`] so a test reads the same as before; code
/// that needs a shared handle takes one from [`DeletedOnDrop::shared`].
pub struct DeletedOnDrop {
sandbox: Arc<RunSandbox>,
/// Access for a fresh provider connection at drop time. `None` deletes
/// through the handle the sandbox already holds.
access: Option<ProviderAccess>,
deleted: bool,
}
impl DeletedOnDrop {
/// Guards a sandbox built through `access`, as every live provider test
/// builds one. A drop-time delete reconnects the provider through
/// `access` and deletes the sandbox by id.
pub fn new(sandbox: impl Into<Arc<RunSandbox>>, access: &ProviderAccess) -> Self {
Self {
sandbox: sandbox.into(),
access: Some(access.clone()),
deleted: false,
}
}
/// Guards a sandbox whose own handle can finish a delete from any
/// thread: the scripted double, whose delete needs no live connection.
/// Not for a live provider, whose handle is bound to the test's runtime
/// (see the module docs).
pub fn through_handle(sandbox: impl Into<Arc<RunSandbox>>) -> Self {
Self {
sandbox: sandbox.into(),
access: None,
deleted: false,
}
}
/// A shared handle to the sandbox for code that takes an `Arc`, such as
/// a workflow runner or an agent environment. The guard keeps its own
/// and still deletes the sandbox when it drops.
#[must_use]
pub fn shared(&self) -> Arc<RunSandbox> {
Arc::clone(&self.sandbox)
}
/// Deletes the sandbox now, returning the driver's result. After a
/// successful delete the drop does nothing; after a failed one it tries
/// once more so a transient failure still leaves nothing behind.
pub async fn delete(mut self) -> crate::Result<()> {
let result = self.sandbox.delete().await;
self.deleted = result.is_ok();
result
}
}
impl Deref for DeletedOnDrop {
type Target = RunSandbox;
fn deref(&self) -> &RunSandbox {
&self.sandbox
}
}
impl fmt::Debug for DeletedOnDrop {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("DeletedOnDrop")
.field("kind", self.sandbox.kind())
.field("id", &self.sandbox.sandbox_info())
.field("deleted", &self.deleted)
.finish_non_exhaustive()
}
}
impl Drop for DeletedOnDrop {
#[expect(
clippy::print_stderr,
reason = "The guard runs during a failing test; its report has to reach the captured test output."
)]
fn drop(&mut self) {
if self.deleted {
return;
}
let id = self.sandbox.sandbox_info();
if id.is_empty() {
// Never created on the provider: nothing to delete.
return;
}
let kind = self.sandbox.kind().clone();
eprintln!("DeletedOnDrop: deleting the {kind} sandbox {id} the test left behind");
let outcome = delete_on_own_thread(
kind.clone(),
id.clone(),
Arc::clone(&self.sandbox),
self.access.clone(),
);
match outcome {
Ok(()) => eprintln!("DeletedOnDrop: deleted the {kind} sandbox {id}"),
Err(error) => {
eprintln!("DeletedOnDrop: the {kind} sandbox {id} may be leaked: {error}");
}
}
}
}
/// Runs the delete to completion on a dedicated thread with its own
/// runtime, bounded by [`DROP_DELETE_TIMEOUT`].
#[expect(
clippy::disallowed_methods,
reason = "Drop is synchronous and the test's runtime may be unwinding; the delete needs a thread and runtime of its own."
)]
fn delete_on_own_thread(
kind: SandboxProviderKind,
id: String,
sandbox: Arc<RunSandbox>,
access: Option<ProviderAccess>,
) -> Result<(), String> {
let thread = std::thread::Builder::new()
.name("sandbox-delete-on-drop".to_string())
.spawn(move || -> Result<(), String> {
let runtime = RuntimeBuilder::new_current_thread()
.enable_all()
.build()
.map_err(|error| format!("could not build a runtime for the delete: {error}"))?;
runtime.block_on(async {
time::timeout(
DROP_DELETE_TIMEOUT,
delete_afresh(&kind, &id, &sandbox, access.as_ref()),
)
.await
.map_err(|_| {
format!(
"the delete did not finish within {}s",
DROP_DELETE_TIMEOUT.as_secs()
)
})?
})
})
.map_err(|error| format!("could not spawn the delete thread: {error}"))?;
thread
.join()
.map_err(|_| "the delete thread panicked".to_string())?
}
/// Deletes sandbox `id` over a fresh provider connection when `access` is
/// given, through the sandbox's own handle otherwise.
async fn delete_afresh(
kind: &SandboxProviderKind,
id: &str,
sandbox: &RunSandbox,
access: Option<&ProviderAccess>,
) -> Result<(), String> {
let Some(access) = access else {
return sandbox
.delete()
.await
.map_err(|error| display_for_log(&error));
};
let fresh = provider_sandbox::attach_provider_sandbox(
kind.clone(),
access,
id,
false,
sandbox.working_directory().to_string(),
None,
None,
None,
)
.await
.map_err(|error| format!("could not reconnect: {}", display_for_log(&error)))?;
fresh
.delete()
.await
.map_err(|error| display_for_log(&error))
}
#[cfg(test)]
mod tests {
use std::panic::AssertUnwindSafe;
use super::*;
use crate::test_support::MockSandbox;
#[tokio::test]
async fn deletes_once_when_dropped_without_an_explicit_delete() {
let mock = MockSandbox::default();
let guard = DeletedOnDrop::through_handle(mock.sandbox());
assert_eq!(
guard.working_directory(),
"/work",
"reads through to the sandbox"
);
assert_eq!(mock.driver().delete_count(), 0);
drop(guard);
assert_eq!(mock.driver().delete_count(), 1);
}
#[tokio::test]
async fn an_explicit_delete_runs_once() {
let mock = MockSandbox::default();
let guard = DeletedOnDrop::through_handle(mock.sandbox());
let shared = guard.shared();
guard.delete().await.unwrap();
assert_eq!(mock.driver().delete_count(), 1);
drop(shared);
assert_eq!(
mock.driver().delete_count(),
1,
"a shared handle does not delete"
);
}
#[test]
fn a_panic_before_the_delete_still_deletes_once() {
let mock = MockSandbox::default();
let sandbox = mock.sandbox();
let outcome = std::panic::catch_unwind(AssertUnwindSafe(|| {
let _guard = DeletedOnDrop::through_handle(sandbox);
panic!("the test failed before its delete");
}));
assert!(outcome.is_err(), "the panic still propagates");
assert_eq!(mock.driver().delete_count(), 1);
}
#[tokio::test]
async fn a_panic_inside_a_runtime_still_deletes_once() {
let mock = MockSandbox::default();
let sandbox = mock.sandbox();
let outcome = std::panic::catch_unwind(AssertUnwindSafe(|| {
let _guard = DeletedOnDrop::through_handle(sandbox);
panic!("the test failed before its delete");
}));
assert!(outcome.is_err(), "the panic still propagates");
assert_eq!(mock.driver().delete_count(), 1);
}
}

View file

@ -23,6 +23,7 @@ use std::path::Path;
use std::sync::Arc;
use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node};
use fabro_sandbox::test_support::DeletedOnDrop;
use fabro_sandbox::{
CloneRequest, DaytonaCredentials, ProviderAccess, RunSandbox, SandboxProviderKind,
provider_sandbox,
@ -197,7 +198,7 @@ fn live_daytona_credentials() -> DaytonaCredentials {
DaytonaCredentials::from_api_key(api_key, |name| std::env::var(name).ok())
}
async fn create_env() -> RunSandbox {
async fn create_env() -> DeletedOnDrop {
let creds = load_github_app_credentials();
create_env_with_github_app(Some(creds)).await
}
@ -212,17 +213,19 @@ fn test_artifact_store(run_dir: &Path) -> ArtifactStore {
async fn create_env_with_github_app(
github_app: Option<fabro_github::GitHubCredentials>,
) -> RunSandbox {
provider_sandbox(
) -> DeletedOnDrop {
let access = daytona_access(live_daytona_credentials());
let sandbox = provider_sandbox(
SandboxProviderKind::DAYTONA,
&daytona_access(live_daytona_credentials()),
&access,
SandboxSpec::new(SandboxSource::HostDirectory),
&CloneRequest::default(),
github_app.as_ref(),
None,
)
.await
.expect("Failed to create Daytona client — is DAYTONA_API_KEY set?")
.expect("Failed to create Daytona client — is DAYTONA_API_KEY set?");
DeletedOnDrop::new(sandbox, &access)
}
fn load_github_app_credentials() -> fabro_github::GitHubCredentials {
@ -425,9 +428,10 @@ async fn daytona_snapshot_sandbox() {
.timers(timers);
let creds = load_github_app_credentials();
let access = daytona_access(live_daytona_credentials());
let env = provider_sandbox(
SandboxProviderKind::DAYTONA,
&daytona_access(live_daytona_credentials()),
&access,
spec,
&CloneRequest::default(),
Some(&creds),
@ -435,6 +439,7 @@ async fn daytona_snapshot_sandbox() {
)
.await
.expect("Failed to create Daytona client — is DAYTONA_API_KEY set?");
let env = DeletedOnDrop::new(env, &access);
env.initialize().await.unwrap();
// Verify rg is available (installed by snapshot)
@ -532,7 +537,6 @@ impl Handler for LargeOutputHandler {
async fn daytona_pipeline_artifact_offload_and_sync() {
let env = create_env().await;
env.initialize().await.unwrap();
let env: Arc<RunSandbox> = Arc::new(env);
// Pipeline: start -> big_output -> exit
let mut graph = Graph::new("DaytonaArtifactPipeline");
@ -570,7 +574,7 @@ async fn daytona_pipeline_artifact_offload_and_sync() {
registry.register("start", Box::new(StartHandler));
registry.register("exit", Box::new(ExitHandler));
let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.clone());
let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.shared());
let run_options = RunOptions {
settings: WorkflowSettings::default(),
run_dir: dir.path().to_path_buf(),
@ -687,7 +691,6 @@ async fn setup_daytona_git(sandbox: &RunSandbox) -> (RunId, String, String) {
async fn daytona_git_checkpoint_remote_emits_events() {
let env = create_env().await;
env.initialize().await.unwrap();
let env: Arc<RunSandbox> = Arc::new(env);
// Install git if not available (the default ubuntu:22.04 image may not have it)
let git_check = env
@ -759,7 +762,7 @@ async fn daytona_git_checkpoint_remote_emits_events() {
registry.register("start", Box::new(StartHandler));
registry.register("exit", Box::new(ExitHandler));
let engine = WorkflowRunner::new(registry, Arc::new(emitter), env.clone());
let engine = WorkflowRunner::new(registry, Arc::new(emitter), env.shared());
let run_options = RunOptions {
settings: WorkflowSettings::default(),
run_dir: dir.path().to_path_buf(),
@ -836,7 +839,6 @@ async fn daytona_git_checkpoint_remote_emits_events() {
async fn daytona_git_checkpoint_without_metadata_branch() {
let env = create_env().await;
env.initialize().await.unwrap();
let env: Arc<RunSandbox> = Arc::new(env);
// Install git if not available
let git_check = env
@ -901,7 +903,7 @@ async fn daytona_git_checkpoint_without_metadata_branch() {
registry.register("start", Box::new(StartHandler));
registry.register("exit", Box::new(ExitHandler));
let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.clone());
let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.shared());
let run_options = RunOptions {
settings: WorkflowSettings::default(),
run_dir: dir.path().to_path_buf(),
@ -997,7 +999,6 @@ impl Handler for AssetCreatorHandler {
async fn daytona_asset_collection() {
let env = create_env().await;
env.initialize().await.unwrap();
let env: Arc<RunSandbox> = Arc::new(env);
let dir = tempfile::tempdir().unwrap();
@ -1005,7 +1006,7 @@ async fn daytona_asset_collection() {
registry.register("start", Box::new(StartHandler));
registry.register("exit", Box::new(ExitHandler));
let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.clone());
let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.shared());
let mut graph = Graph::new("DaytonaAssetTest");
graph.attrs.insert(
@ -1256,7 +1257,6 @@ async fn daytona_git_push_run_branch_to_origin() {
let creds = load_github_app_credentials();
let env = create_env_with_github_app(Some(creds)).await;
env.initialize().await.unwrap();
let env: Arc<RunSandbox> = Arc::new(env);
// Install git if not available
let git_check = env
@ -1319,7 +1319,7 @@ async fn daytona_git_push_run_branch_to_origin() {
registry.register("start", Box::new(StartHandler));
registry.register("exit", Box::new(ExitHandler));
let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.clone());
let engine = WorkflowRunner::new(registry, Arc::new(Emitter::default()), env.shared());
let run_options = RunOptions {
settings: WorkflowSettings::default(),
run_dir: dir.path().to_path_buf(),
@ -1623,9 +1623,10 @@ async fn daytona_cp_upload_download_round_trip() {
#[fabro_macros::e2e_test(live("DAYTONA_API_KEY"))]
async fn daytona_computer_use_browser_screenshot() {
let access = daytona_access(live_daytona_credentials());
let env = provider_sandbox(
SandboxProviderKind::DAYTONA,
&daytona_access(live_daytona_credentials()),
&access,
SandboxSpec::new(SandboxSource::HostDirectory),
&CloneRequest::none(),
None,
@ -1633,6 +1634,7 @@ async fn daytona_computer_use_browser_screenshot() {
)
.await
.expect("DAYTONA_API_KEY must be set");
let env = DeletedOnDrop::new(env, &access);
env.initialize().await.unwrap();
// 1. Start the computer use desktop environment (Xvfb, xfce4, etc.) through the
@ -1764,9 +1766,10 @@ async fn daytona_computer_use_browser_screenshot() {
#[fabro_macros::e2e_test(live("DAYTONA_API_KEY"))]
async fn daytona_playwright_mcp_sandbox_transport() {
// Create sandbox from daytona-medium (has Node.js + Chromium)
let access = daytona_access(live_daytona_credentials());
let sandbox = provider_sandbox(
SandboxProviderKind::DAYTONA,
&daytona_access(live_daytona_credentials()),
&access,
SandboxSpec::new(SandboxSource::HostDirectory),
&CloneRequest::none(),
None,
@ -1774,6 +1777,7 @@ async fn daytona_playwright_mcp_sandbox_transport() {
)
.await
.expect("DAYTONA_API_KEY must be set");
let sandbox = DeletedOnDrop::new(sandbox, &access);
sandbox.initialize().await.unwrap();
// 1. Install Playwright MCP server and its browser
@ -1858,13 +1862,13 @@ async fn daytona_playwright_mcp_sandbox_transport() {
),
]),
);
let sandbox = Arc::new(sandbox);
let routes = sandbox
.shared()
.port_routes()
.expect("Daytona forwards ports through preview URLs");
let mut agent = pebble_coding_agent::CodingAgent::builder(
client,
Arc::clone(&sandbox) as Arc<dyn pebble_coding_agent::environment::Environment>,
sandbox.shared() as Arc<dyn pebble_coding_agent::environment::Environment>,
)
.model("test/model")
.permission_level(pebble_coding_agent::events::PermissionLevel::Full)