From ff231e890b281ff287ad98953240092d5c80c678 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 9 Sep 2026 22:19:26 -0600 Subject: [PATCH] Pin the driver PR stack and run its own plugin executables The plugin scenarios launched two executables fabro built itself, `fabro-sandbox-host` and `fabro-sandbox-docker`, that only wrapped the driver's providers in a stdio server the driver already ships as `sandbox-driver-host` and `sandbox-driver-docker`. Fabro now finds the driver's executables on PATH: CI installs them at the rev the workspace pins, read from Cargo.toml so the plugins and the in-process providers are one build, and a developer installs them the same way. The plugin proof in fabro-sandbox skips without the executable unless the CI environment forbids skipping; it was also never running in CI, which ran it under `--run-ignored only` although it is not ignored, so the job now runs it on its own. The pin moves to the head of the sandbox-driver PR stack #9 through #15: tag pins, classified git failures, the stop grace ladder, snapshot ensure, the ownership scope, and the testing doubles, which the next commits adopt. The `sandbox-driver-testing` crate joins the workspace dependencies for them. Co-Authored-By: Claude Fable 5.1 --- .github/workflows/rust.yml | 18 ++++-- Cargo.lock | 15 +++-- Cargo.toml | 20 ++++--- .../fabro-cli/tests/it/workflow/plugin.rs | 31 ++++++----- lib/components/fabro-sandbox/Cargo.toml | 9 --- .../src/bin/fabro-sandbox-docker.rs | 43 --------------- .../src/bin/fabro-sandbox-host.rs | 55 ------------------- .../fabro-sandbox/tests/plugin_provider.rs | 46 ++++++++++++++-- 8 files changed, 91 insertions(+), 146 deletions(-) delete mode 100644 lib/components/fabro-sandbox/src/bin/fabro-sandbox-docker.rs delete mode 100644 lib/components/fabro-sandbox/src/bin/fabro-sandbox-host.rs diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index e17f5c5fd..2983369fa 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -154,15 +154,23 @@ jobs: cache-on-failure: true - uses: taiki-e/install-action@773334c0e05d7e699e4d78234494308223f3a2cf # nextest - run: docker pull buildpack-deps:noble - # The driver's Host and Docker executables fabro ships as - # `fabro-sandbox-`; the CLI scenarios launch them over stdio. - - run: cargo build --locked -p fabro-sandbox --bins + # The driver's own Host and Docker executables, installed at the rev the + # workspace pins so the plugins and the in-process providers are one + # build; the CLI scenarios find them on PATH and launch them over stdio. + - name: Install the sandbox-driver plugin executables + run: | + rev="$(sed -n 's/^sandbox-driver = { git = "[^"]*", rev = "\([0-9a-f]*\)" }$/\1/p' Cargo.toml)" + test -n "$rev" + cargo install --locked --git https://github.com/lithoscomputer/sandbox-driver --rev "$rev" sandbox-driver-host sandbox-driver-docker # Host and Docker served as plugins through the workflow scenarios. The # scenarios are e2e tests (ignored by default); the key-free ones run # here, the LLM-backed ones self-skip without credentials. - run: cargo nextest run --locked --profile ci --status-level slow --run-ignored only -p fabro-cli --test it -E 'test(/host_plugin_|docker_plugin_/)' - # The driver-backed Docker integration tests and the stdio plugin proof. - - run: cargo nextest run --locked --profile ci --status-level slow --run-ignored only -p fabro-sandbox --test docker_streaming --test plugin_provider + # The stdio plugin proof (not ignored: it skips without the executable, + # which the environment above forbids) and the driver-backed Docker + # integration tests. + - run: cargo nextest run --locked --profile ci --status-level slow -p fabro-sandbox --test plugin_provider + - run: cargo nextest run --locked --profile ci --status-level slow --run-ignored only -p fabro-sandbox --test docker_streaming - run: cargo nextest run --locked --profile ci --status-level slow --run-ignored only -p fabro-agent --test it -E 'test(docker_shell)' - run: cargo nextest run --locked --profile ci --status-level slow --run-ignored only -p fabro-workflow --test it -E 'test(asset_collection_docker_sandbox)' diff --git a/Cargo.lock b/Cargo.lock index 7a1060669..bde521116 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3012,7 +3012,6 @@ dependencies = [ "tokio-util", "toml 0.8.23", "tracing", - "tracing-subscriber", "uuid", ] @@ -7008,7 +7007,7 @@ dependencies = [ [[package]] name = "sandbox-driver" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=e01d786a26a62291bb3716eca968d9c8838ad551#e01d786a26a62291bb3716eca968d9c8838ad551" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d#d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" dependencies = [ "async-trait", "globset", @@ -7024,7 +7023,7 @@ dependencies = [ [[package]] name = "sandbox-driver-daytona" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=e01d786a26a62291bb3716eca968d9c8838ad551#e01d786a26a62291bb3716eca968d9c8838ad551" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d#d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" dependencies = [ "anyhow", "async-trait", @@ -7049,7 +7048,7 @@ dependencies = [ [[package]] name = "sandbox-driver-daytona-config" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=e01d786a26a62291bb3716eca968d9c8838ad551#e01d786a26a62291bb3716eca968d9c8838ad551" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d#d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" dependencies = [ "sandbox-driver-docker-config", "serde", @@ -7059,7 +7058,7 @@ dependencies = [ [[package]] name = "sandbox-driver-docker" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=e01d786a26a62291bb3716eca968d9c8838ad551#e01d786a26a62291bb3716eca968d9c8838ad551" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d#d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" dependencies = [ "anyhow", "async-trait", @@ -7080,7 +7079,7 @@ dependencies = [ [[package]] name = "sandbox-driver-docker-config" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=e01d786a26a62291bb3716eca968d9c8838ad551#e01d786a26a62291bb3716eca968d9c8838ad551" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d#d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" dependencies = [ "serde", "serde_json", @@ -7089,7 +7088,7 @@ dependencies = [ [[package]] name = "sandbox-driver-host" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=e01d786a26a62291bb3716eca968d9c8838ad551#e01d786a26a62291bb3716eca968d9c8838ad551" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d#d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" dependencies = [ "anyhow", "async-trait", @@ -7107,7 +7106,7 @@ dependencies = [ [[package]] name = "sandbox-driver-protocol" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=e01d786a26a62291bb3716eca968d9c8838ad551#e01d786a26a62291bb3716eca968d9c8838ad551" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d#d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" dependencies = [ "async-trait", "base64", diff --git a/Cargo.toml b/Cargo.toml index ccb2cc771..fd2118feb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -98,14 +98,18 @@ futures-util = "0.3" # sandbox-driver: the sandbox provider layer. Bundled Host, Docker, and # Daytona providers link in-process; third-party providers run as stdio # plugins through sandbox-driver-protocol. Pinned by rev; currently the head of -# sandbox-driver PR #9 (trust the configured plugin kind), to move to main on merge. -sandbox-driver = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "e01d786a26a62291bb3716eca968d9c8838ad551" } -sandbox-driver-protocol = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "e01d786a26a62291bb3716eca968d9c8838ad551" } -sandbox-driver-host = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "e01d786a26a62291bb3716eca968d9c8838ad551" } -sandbox-driver-docker = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "e01d786a26a62291bb3716eca968d9c8838ad551" } -sandbox-driver-docker-config = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "e01d786a26a62291bb3716eca968d9c8838ad551" } -sandbox-driver-daytona = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "e01d786a26a62291bb3716eca968d9c8838ad551" } -sandbox-driver-daytona-config = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "e01d786a26a62291bb3716eca968d9c8838ad551" } +# the sandbox-driver PR stack #9-#15 (configured plugin kind, tag pins, classified +# git failures, stop grace, snapshot ensure, ownership scope, testing doubles), to +# move to main on merge. The CI plugin job installs the driver executables at the +# same rev, read from this file. +sandbox-driver = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" } +sandbox-driver-protocol = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" } +sandbox-driver-host = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" } +sandbox-driver-docker = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" } +sandbox-driver-docker-config = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" } +sandbox-driver-daytona = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" } +sandbox-driver-daytona-config = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" } +sandbox-driver-testing = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" } sentry = { version = "0.35", default-features = false, features = ["backtrace", "contexts", "ureq", "rustls"] } fork = "0.2" exec = "0.3" diff --git a/lib/apps/fabro-cli/tests/it/workflow/plugin.rs b/lib/apps/fabro-cli/tests/it/workflow/plugin.rs index 907b35c9e..a30f476e8 100644 --- a/lib/apps/fabro-cli/tests/it/workflow/plugin.rs +++ b/lib/apps/fabro-cli/tests/it/workflow/plugin.rs @@ -1,12 +1,15 @@ //! Sandbox providers served by sandbox-driver plugin executables, for the //! workflow scenarios. //! -//! The executables come from the `fabro-sandbox` package's `[[bin]]` targets, -//! which `cargo` places beside the `fabro` binary under test. Each runs under -//! a kind of the scenario's choosing (`host`, `docker-plugin`): the configured -//! kind names the plugin, whatever the executable declares. A scenario -//! configured here runs against its own server so the plugin settings and the -//! environment it creates never leak into the shared session server. +//! The executables are the driver's own `sandbox-driver-host` and +//! `sandbox-driver-docker`, found on `PATH`; CI installs them at the rev the +//! workspace pins, and a developer installs them with +//! `cargo install --locked --git https://github.com/lithoscomputer/sandbox-driver --rev sandbox-driver-host sandbox-driver-docker`. +//! Each runs under a kind of the scenario's choosing (`host`, +//! `docker-plugin`): the configured kind names the plugin, whatever the +//! executable declares. A scenario configured here runs against its own +//! server so the plugin settings and the environment it creates never leak +//! into the shared session server. #![expect( clippy::disallowed_methods, @@ -49,8 +52,8 @@ impl Plugin { fn executable(self) -> &'static str { match self { - Self::Host => "fabro-sandbox-host", - Self::Docker => "fabro-sandbox-docker", + Self::Host => "sandbox-driver-host", + Self::Docker => "sandbox-driver-docker", } } @@ -75,7 +78,8 @@ pub(crate) fn configure(context: &mut TestContext, plugin: Plugin) -> Option<&'s plugin.executable() ); eprintln!( - "skipping: {} is not built; run `cargo build -p fabro-sandbox --bins`", + "skipping: {} is not on PATH; install the sandbox-driver executables at the rev \ + Cargo.toml pins", plugin.executable() ); return None; @@ -134,11 +138,12 @@ inherit_env = ["PATH", "HOME", "DOCKER_HOST", "DOCKER_CERT_PATH", "DOCKER_TLS_VE Some(plugin.environment()) } -/// The plugin executable `cargo` built beside the `fabro` binary under test. +/// The driver executable on `PATH`, when installed. fn plugin_executable(plugin: Plugin) -> Option { - let fabro = Path::new(env!("CARGO_BIN_EXE_fabro")); - let candidate = fabro.with_file_name(plugin.executable()); - candidate.is_file().then_some(candidate) + let path = std::env::var_os("PATH")?; + std::env::split_paths(&path) + .map(|dir| dir.join(plugin.executable())) + .find(|candidate| candidate.is_file()) } fn docker_image_available() -> bool { diff --git a/lib/components/fabro-sandbox/Cargo.toml b/lib/components/fabro-sandbox/Cargo.toml index ef2869795..dbeaf4fd8 100644 --- a/lib/components/fabro-sandbox/Cargo.toml +++ b/lib/components/fabro-sandbox/Cargo.toml @@ -14,14 +14,6 @@ test-support = [] [lib] doctest = false -[[bin]] -name = "fabro-sandbox-host" -path = "src/bin/fabro-sandbox-host.rs" - -[[bin]] -name = "fabro-sandbox-docker" -path = "src/bin/fabro-sandbox-docker.rs" - [lints] workspace = true @@ -42,7 +34,6 @@ serde.workspace = true serde_json.workspace = true strum.workspace = true tracing.workspace = true -tracing-subscriber.workspace = true reqwest.workspace = true base64.workspace = true hmac.workspace = true diff --git a/lib/components/fabro-sandbox/src/bin/fabro-sandbox-docker.rs b/lib/components/fabro-sandbox/src/bin/fabro-sandbox-docker.rs deleted file mode 100644 index dc9577d71..000000000 --- a/lib/components/fabro-sandbox/src/bin/fabro-sandbox-docker.rs +++ /dev/null @@ -1,43 +0,0 @@ -//! The bundled Docker provider served as a sandbox-driver plugin over stdio. -//! -//! Fabro links Docker in-process for normal runs. This executable exists so -//! CI can run the same provider through the plugin protocol and so a -//! deployment can move it out of process. The daemon named by `DOCKER_HOST` -//! (or the local default) is not required to answer at launch: an -//! unreachable daemon is reported through `provider/health`. Stdout belongs -//! to the protocol; logs go to stderr. - -use std::io::stderr; -use std::sync::Arc; - -use anyhow::Context as _; -use sandbox_driver_docker::DockerProvider; -use sandbox_driver_protocol::serve_stdio; -use tracing_subscriber::filter::LevelFilter; -use tracing_subscriber::prelude::*; -use tracing_subscriber::{EnvFilter, fmt}; - -#[tokio::main] -async fn main() -> anyhow::Result<()> { - let filter = EnvFilter::builder() - .with_default_directive(LevelFilter::INFO.into()) - .from_env_lossy(); - // Stdout carries the protocol, so diagnostics must go to the process - // stderr handle; tracing's writer contract is synchronous. - #[expect( - clippy::disallowed_methods, - reason = "tracing-subscriber requires a synchronous writer and stdout is reserved for \ - the plugin protocol" - )] - let diagnostics = fmt::layer().with_writer(stderr); - tracing_subscriber::registry() - .with(filter) - .with(diagnostics) - .try_init() - .context("configuring docker plugin diagnostics")?; - - let provider = DockerProvider::connect_unverified().context("configuring the docker client")?; - serve_stdio(Arc::new(provider)) - .await - .context("serving the docker provider plugin") -} diff --git a/lib/components/fabro-sandbox/src/bin/fabro-sandbox-host.rs b/lib/components/fabro-sandbox/src/bin/fabro-sandbox-host.rs deleted file mode 100644 index b444eb9b1..000000000 --- a/lib/components/fabro-sandbox/src/bin/fabro-sandbox-host.rs +++ /dev/null @@ -1,55 +0,0 @@ -//! The bundled Host provider served as a sandbox-driver plugin over stdio. -//! -//! Fabro links Host in-process for normal runs. This executable exists so -//! CI can run the same provider through the plugin protocol and so a -//! deployment can move it out of process by configuring -//! `[server.sandbox.providers.host]` instead of `local`. Stdout belongs to -//! the protocol; logs go to stderr. `SANDBOX_DRIVER_HOST_REGISTRY` names a -//! persistent registry directory; without it sandboxes live in a fresh -//! temporary registry. - -use std::io::stderr; -use std::sync::Arc; - -use anyhow::Context as _; -use sandbox_driver_host::HostProvider; -use sandbox_driver_protocol::serve_stdio; -use tracing_subscriber::filter::LevelFilter; -use tracing_subscriber::prelude::*; -use tracing_subscriber::{EnvFilter, fmt}; - -#[tokio::main] -async fn main() -> anyhow::Result<()> { - let filter = EnvFilter::builder() - .with_default_directive(LevelFilter::INFO.into()) - .from_env_lossy(); - // Stdout carries the protocol, so diagnostics must go to the process - // stderr handle; tracing's writer contract is synchronous. - #[expect( - clippy::disallowed_methods, - reason = "tracing-subscriber requires a synchronous writer and stdout is reserved for \ - the plugin protocol" - )] - let diagnostics = fmt::layer().with_writer(stderr); - tracing_subscriber::registry() - .with(filter) - .with(diagnostics) - .try_init() - .context("configuring host plugin diagnostics")?; - - #[expect( - clippy::disallowed_methods, - reason = "a plugin executable starts from the scrubbed environment its host declared; \ - reading it here is the configured channel" - )] - let registry = std::env::var_os("SANDBOX_DRIVER_HOST_REGISTRY"); - let provider = match registry { - Some(root) => HostProvider::with_registry(root) - .await - .context("opening the host registry")?, - None => HostProvider::new(), - }; - serve_stdio(Arc::new(provider)) - .await - .context("serving the host provider plugin") -} diff --git a/lib/components/fabro-sandbox/tests/plugin_provider.rs b/lib/components/fabro-sandbox/tests/plugin_provider.rs index ddabc2e02..9fd74e21f 100644 --- a/lib/components/fabro-sandbox/tests/plugin_provider.rs +++ b/lib/components/fabro-sandbox/tests/plugin_provider.rs @@ -1,21 +1,51 @@ //! The construction function serves a non-bundled kind through a plugin //! executable, and a sandbox created through one plugin generation is //! reachable by persisted id from a fresh connection. +//! +//! The executable is the driver's own `sandbox-driver-host`, found on `PATH` +//! (CI installs it at the rev the workspace pins). Without it the tests skip, +//! unless `FABRO_REQUIRE_SANDBOX_PLUGINS` is set. + +#![expect( + clippy::disallowed_methods, + reason = "the test locates the plugin executable through the process PATH" +)] +#![expect(clippy::print_stderr, reason = "a skipped test says why on its stderr")] use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; use fabro_sandbox::driver::{ProviderConnectOptions, connect_provider}; use fabro_types::SandboxProviderKind; use fabro_types::settings::server::{SandboxPluginSettings, ServerSandboxProviderSettings}; use sandbox_driver::{ExecSpec, SandboxId, SandboxSource, SandboxSpec}; -const HOST_PLUGIN: &str = env!("CARGO_BIN_EXE_fabro-sandbox-host"); +const HOST_PLUGIN: &str = "sandbox-driver-host"; +const REQUIRE_ENV: &str = "FABRO_REQUIRE_SANDBOX_PLUGINS"; -fn host_plugin_settings(registry: &std::path::Path) -> ServerSandboxProviderSettings { +/// The driver's Host executable on `PATH`, or `None` (after saying so) when +/// the test should skip. +fn host_plugin() -> Option { + let found = std::env::var_os("PATH").and_then(|path| { + std::env::split_paths(&path) + .map(|dir| dir.join(HOST_PLUGIN)) + .find(|candidate| candidate.is_file()) + }); + if found.is_none() { + assert!( + std::env::var_os(REQUIRE_ENV).is_none(), + "{REQUIRE_ENV} is set but {HOST_PLUGIN} is not on PATH" + ); + eprintln!("skipping: {HOST_PLUGIN} is not on PATH"); + } + found +} + +fn host_plugin_settings(executable: &Path, registry: &Path) -> ServerSandboxProviderSettings { ServerSandboxProviderSettings { enabled: true, plugin: Some(SandboxPluginSettings { - path: Some(HOST_PLUGIN.to_string()), + path: Some(executable.display().to_string()), sha256: None, dev: true, args: Vec::new(), @@ -30,6 +60,9 @@ fn host_plugin_settings(registry: &std::path::Path) -> ServerSandboxProviderSett #[tokio::test] async fn host_plugin_under_a_non_bundled_kind_creates_and_reattaches_by_persisted_id() { + let Some(executable) = host_plugin() else { + return; + }; let registry = tempfile::tempdir().expect("registry tempdir"); let workspace = tempfile::tempdir().expect("workspace tempdir"); let kind = SandboxProviderKind::try_new("host").expect("host is a valid kind"); @@ -38,7 +71,7 @@ async fn host_plugin_under_a_non_bundled_kind_creates_and_reattaches_by_persiste None, "host is not one of fabro's bundled kinds" ); - let settings = host_plugin_settings(registry.path()); + let settings = host_plugin_settings(&executable, registry.path()); let persisted_id: SandboxId = { let connected = connect_provider(&kind, &settings, &ProviderConnectOptions::default()) @@ -94,11 +127,14 @@ async fn host_plugin_under_a_non_bundled_kind_creates_and_reattaches_by_persiste /// plugin's own declared kind is information, not a gate. #[tokio::test] async fn the_configured_kind_names_the_plugin_whatever_it_declares() { + let Some(executable) = host_plugin() else { + return; + }; let registry = tempfile::tempdir().expect("registry tempdir"); let kind = SandboxProviderKind::try_new("host-alias").expect("valid kind"); let connected = connect_provider( &kind, - &host_plugin_settings(registry.path()), + &host_plugin_settings(&executable, registry.path()), &ProviderConnectOptions::default(), ) .await