diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ed9f08e47..f72e5c42c 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -61,6 +61,14 @@ jobs: sudo apt-get update sudo apt-get install -y build-essential pkg-config libssl-dev + - name: Install Docker for ARM Linux tests + if: matrix.target == 'aarch64-unknown-linux-gnu' + run: | + sudo apt-get install -y docker.io acl + sudo systemctl start docker + sudo setfacl -m "u:$(id -un):rw" /var/run/docker.sock + docker info + - name: Install musl toolchain for x86_64-musl tests if: matrix.target == 'x86_64-unknown-linux-musl' run: sudo apt-get install -y musl-tools @@ -104,7 +112,7 @@ jobs: env: CC_x86_64_unknown_linux_musl: musl-gcc CARGO_TARGET_X86_64_UNKNOWN_LINUX_MUSL_LINKER: musl-gcc - run: cargo nextest run --locked --workspace --target ${{ matrix.target }} --release --status-level slow --profile ci + run: cargo nextest run --locked --workspace --target ${{ matrix.target }} --release --status-level slow --profile ci --no-fail-fast - name: Test # aarch64-musl test runs have not been validated on the compile @@ -115,7 +123,7 @@ jobs: if [[ "$RUNNER_OS" == "macOS" ]]; then ulimit -n 4096 fi - cargo nextest run --locked --workspace --target ${{ matrix.target }} --release --status-level slow --profile ci + cargo nextest run --locked --workspace --target ${{ matrix.target }} --release --status-level slow --profile ci --no-fail-fast - name: Build (musl via cargo-zigbuild) if: matrix.musl diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 70ddb282d..84e65b52b 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -17,6 +17,7 @@ on: - "docs/public/reference/user-configuration.mdx" - "openapi/**" - ".github/workflows/rust.yml" + - ".github/workflows/release.yml" pull_request: branches: [main] paths: @@ -33,6 +34,7 @@ on: - "docs/public/reference/user-configuration.mdx" - "openapi/**" - ".github/workflows/rust.yml" + - ".github/workflows/release.yml" workflow_dispatch: concurrency: @@ -203,7 +205,6 @@ jobs: test-macos: name: Test (macOS) - if: github.event_name == 'workflow_dispatch' runs-on: macos-15 permissions: contents: read @@ -220,3 +221,25 @@ jobs: - uses: taiki-e/install-action@773334c0e05d7e699e4d78234494308223f3a2cf # nextest # No Docker daemon on the macOS runner: the Docker tests skip there. - run: cargo nextest run --locked --workspace --status-level slow --profile ci + + process-title-musl: + name: Process titles (musl) + runs-on: ubuntu-24.04-x86-32-cores + permissions: + contents: read + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + - run: sudo apt-get update && sudo apt-get install -y musl-tools + - uses: dtolnay/rust-toolchain@631a55b12751854ce901bb631d5902ceb48146f7 # stable + with: + toolchain: 1.97.1 + targets: x86_64-unknown-linux-musl + - uses: Swatinem/rust-cache@779680da715d629ac1d338a641029a2f4372abb5 # v2 + - uses: taiki-e/install-action@773334c0e05d7e699e4d78234494308223f3a2cf # nextest + - name: Test real process titles under musl + env: + CC_x86_64_unknown_linux_musl: musl-gcc + CARGO_TARGET_X86_64_UNKNOWN_LINUX_MUSL_LINKER: musl-gcc + run: cargo nextest run --locked --release -p fabro-proc --target x86_64-unknown-linux-musl --profile ci diff --git a/Cargo.lock b/Cargo.lock index 56e57e336..bfab50ec0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2566,7 +2566,6 @@ dependencies = [ name = "fabro-proc" version = "0.369.0-nightly.0" dependencies = [ - "cc", "libc", "tempfile", "thiserror 2.0.18", diff --git a/lib/components/fabro-petri/src/engine.rs b/lib/components/fabro-petri/src/engine.rs index b322cce1e..e4f63483b 100644 --- a/lib/components/fabro-petri/src/engine.rs +++ b/lib/components/fabro-petri/src/engine.rs @@ -170,9 +170,15 @@ pub enum Conclusion { /// Execute the run to its end and report what the record says. pub async fn run(request: RunRequest) -> Result { - let backend = backend(&request.provider).ok_or_else(|| RunError::UnsupportedProvider { - provider: request.provider.clone(), - })?; + let backend = if request.runtime.dry_run { + // The dry-run admission pass puts every scope on the host. Keep the + // real local workspace for Fabro's checkpoint and artifact hooks. + SandboxBackend::Host + } else { + backend(&request.provider).ok_or_else(|| RunError::UnsupportedProvider { + provider: request.provider.clone(), + })? + }; let key = RunKey::new(request.run_id.as_str()); let mut options = RunOptions::new(&request.run_dir); options.run_key = Some(key.clone()); @@ -196,7 +202,10 @@ pub async fn run(request: RunRequest) -> Result { if let Some(blobs) = &request.blobs { runtime = runtime.capability(RunBlobs::output_store(Arc::clone(blobs))); } - let fabro_hooks = request.hooks.map(|spec| { + let fabro_hooks = request.hooks.map(|mut spec| { + if request.runtime.dry_run { + spec.git.host_workspaces = true; + } let inner = runtime .installed_hooks() .unwrap_or_else(|| Arc::new(NoHooks)); diff --git a/lib/components/fabro-petri/src/runtime.rs b/lib/components/fabro-petri/src/runtime.rs index a19603f9b..da6a3b129 100644 --- a/lib/components/fabro-petri/src/runtime.rs +++ b/lib/components/fabro-petri/src/runtime.rs @@ -25,7 +25,9 @@ use lithos_llm::credentials::CredentialProvider; use petri_attractor_steps::pebble::PebbleClient; use petri_attractor_steps::skills::FabroHome; use petri_frontend_fabro::Fabro; -use petri_runtime::Runtime; +use petri_runtime::ir::{Graph, RuntimeTarget}; +use petri_runtime::steps::Capabilities; +use petri_runtime::{AdmissionPass, AdmissionProblem, Runtime}; use tracing::debug; use crate::host_tools; @@ -51,8 +53,8 @@ pub struct RuntimeSpec { /// catalog the admission pass resolves model selectors against. `None` /// leaves every LLM node unpinned and every model call unconfigured. pub model_client: Option, - /// Run the simulated step registry (Fabro's `--dry-run` handlers) - /// instead of the real one. + /// Simulate steps (Fabro's `--dry-run` handlers) in local workspaces, + /// without acquiring the configured Docker or Daytona sandboxes. pub dry_run: bool, /// The Fabro home the skills step reads; `None` leaves it to Petri's /// own lookup (`FABRO_HOME`, else `$HOME/.fabro`). @@ -89,6 +91,9 @@ impl RuntimeSpec { if let Some(services) = &self.run_tools { runtime = runtime.capability(host_tools::capability(services.clone())); } + if self.dry_run { + runtime = runtime.admission(LocalDryRun); + } if for_execution && self.dry_run { petri_attractor_steps::register_stubs(runtime) } else { @@ -97,6 +102,22 @@ impl RuntimeSpec { } } +/// Dry runs keep real local workspaces for checkpoint hooks, while simulated +/// stages need neither container images nor sidecars. Do this during admission +/// so Petri persists the effective scopes and re-digests nested graphs itself. +struct LocalDryRun; + +impl AdmissionPass for LocalDryRun { + fn admit(&self, graph: &mut Graph, _caps: &Capabilities) -> Vec { + for scope in &mut graph.body.scopes { + scope.runtime.target = RuntimeTarget::HostProcess; + scope.runtime.requirements.clear(); + scope.services.clear(); + } + Vec::new() + } +} + /// The model client Fabro hands Petri: the server's catalog, its credential /// provider, its HTTP client (so a test's loopback client and a server's /// proxy policy carry over), and only the providers whose credentials are diff --git a/lib/components/fabro-petri/tests/check.rs b/lib/components/fabro-petri/tests/check.rs index 3ce8a428e..502e4bdc3 100644 --- a/lib/components/fabro-petri/tests/check.rs +++ b/lib/components/fabro-petri/tests/check.rs @@ -21,6 +21,7 @@ use fabro_petri::check::{self, Bundle, CheckError, CheckRequest, DiagnosticSever use fabro_petri::runtime::{self, RuntimeSpec}; use fabro_store::{BlobStore, test_support}; use lithos_llm::catalog::ProviderId; +use petri_runtime::ir::RuntimeTarget; const COMMAND_WORKFLOW: &str = r#"digraph Command { graph [goal="Run one command"] @@ -420,3 +421,59 @@ async fn a_launch_goal_replaces_the_graphs_goal_on_the_admitted_graph_and_its_st // Without a launch goal, the bundle's `[run] goal` stands over the graph's. assert_eq!(goal_of(Launch::default()), "The bundle's goal"); } + +/// Dry-run admission must cover child workflows too, and its rewritten +/// graphs must survive the same digest-checked persistence as any other run. +#[tokio::test] +async fn dry_run_admission_makes_nested_scopes_local_and_preserves_graph_digests() { + let root = r#"digraph Root { + start [shape=Mdiamond] + exit [shape=Msquare] + child [shape=house, stack.child_workflow="child/workflow.fabro"] + start -> child -> exit + }"#; + let settings = format!( + "{SETTINGS}\n[run.environment]\nid = \"docker\"\n\n[environments.docker]\nprovider = \"docker\"\n[environments.docker.image]\ndocker = \"invalid.example/dry-run:must-not-pull\"\n" + ); + let files = [ + ("workflow.fabro", root), + ("workflow.toml", settings.as_str()), + ("child/workflow.fabro", COMMAND_WORKFLOW), + ("child/workflow.toml", settings.as_str()), + ]; + let real = check::check(&request(bundle(&files), RuntimeSpec::default())) + .expect("the real workflow admits"); + assert!(!real.children.is_empty()); + assert!( + real.graph + .body + .scopes + .iter() + .any(|scope| { matches!(scope.runtime.target, RuntimeTarget::Container { .. }) }) + ); + let dry_run = check::check(&request(bundle(&files), RuntimeSpec { + dry_run: true, + ..RuntimeSpec::default() + })) + .expect("the dry run admits"); + assert_eq!(dry_run.children.len(), real.children.len()); + for graph in std::iter::once(&dry_run.graph).chain(&dry_run.children) { + assert!(!graph.body.scopes.is_empty()); + for scope in &graph.body.scopes { + assert_eq!(scope.runtime.target, RuntimeTarget::HostProcess); + assert!(scope.services.is_empty()); + assert!(scope.runtime.requirements.is_empty()); + } + } + let blobs = BlobStore::new(test_support::in_memory_pool_with(&[ + fabro_db::BLOBS_MIGRATION_SQL, + ])); + let record = admission::persist(&blobs, &dry_run) + .await + .expect("the graphs persist"); + let loaded = admission::load(&blobs, &record) + .await + .expect("the graph digests match"); + assert_eq!(loaded.graph, dry_run.graph); + assert_eq!(loaded.children, dry_run.children); +} diff --git a/lib/components/fabro-petri/tests/hooks.rs b/lib/components/fabro-petri/tests/hooks.rs index 2ad46a608..7249590f4 100644 --- a/lib/components/fabro-petri/tests/hooks.rs +++ b/lib/components/fabro-petri/tests/hooks.rs @@ -302,6 +302,62 @@ fn stages(inspection: &RunInspection) -> Vec<(String, String)> { .collect() } +/// A dry run keeps checkpointable local workspaces even when the selected +/// environment would require Docker or Daytona. Its command is simulated. +#[tokio::test] +async fn dry_runs_use_local_workspaces_and_checkpoint_without_sandbox_credentials() { + for provider in [SandboxProviderKind::DOCKER, SandboxProviderKind::DAYTONA] { + let harness = Harness::new(); + let workflow = workflow( + r#" write [shape=parallelogram, script="touch should-not-exist; exit 1"]"#, + " start -> write -> exit", + ); + let settings = format!( + "{SETTINGS}\n[run.environment]\nid = \"remote\"\n\n[environments.remote]\nprovider = \"{provider}\"\n\n[environments.remote.image]\ndocker = \"invalid.example/dry-run:must-not-pull\"\n" + ); + let runtime = RuntimeSpec { + dry_run: true, + ..RuntimeSpec::default() + }; + let graphs = support::admit( + &[("workflow.fabro", &workflow), ("workflow.toml", &settings)], + Launch::default(), + &runtime, + ); + let mut request = support::run_request( + &harness.run_id.to_string(), + &harness.run_dir, + graphs, + harness.store.clone(), + runtime, + support::no_questions(Arc::new(support::Silent)), + ); + request.hooks = Some(harness.hooks(&provider)); + request.provider = provider.clone(); + request.blobs = Some(harness.blobs.clone()); + let outcome = engine::run(request).await.expect("the dry run executes"); + assert_eq!( + outcome.status, + RunStatus::Success, + "{provider}: {outcome:?}" + ); + assert!(outcome.complete, "{provider}: {:?}", outcome.incomplete); + let workspace = harness.workspace().await; + assert!( + !harness + .workspace_path(&workspace) + .join("should-not-exist") + .exists() + ); + assert_eq!( + harness.checkpoints().len(), + 3, + "{provider}: every stage checkpoints" + ); + assert_eq!(harness.snapshot_commits(&workspace).await.len(), 3); + } +} + /// Every finished stage is committed on the run branch with the identity /// trailers, its platform record names the commit, and the run-end hooks /// reached Petri's local service through Fabro's wrapper. diff --git a/lib/foundation/fabro-proc/Cargo.toml b/lib/foundation/fabro-proc/Cargo.toml index 47335363e..1cb5296ae 100644 --- a/lib/foundation/fabro-proc/Cargo.toml +++ b/lib/foundation/fabro-proc/Cargo.toml @@ -20,6 +20,3 @@ libc = "0.2" [dev-dependencies] tempfile = "3" - -[build-dependencies] -cc = "1" diff --git a/lib/foundation/fabro-proc/build.rs b/lib/foundation/fabro-proc/build.rs deleted file mode 100644 index 5f06d7639..000000000 --- a/lib/foundation/fabro-proc/build.rs +++ /dev/null @@ -1,14 +0,0 @@ -#![allow( - clippy::disallowed_methods, - reason = "Build scripts run at compile time and read Cargo-provided env vars." -)] - -fn main() { - println!("cargo:rerun-if-changed=c/capture_argv.c"); - - if std::env::var("CARGO_CFG_TARGET_OS").as_deref() == Ok("linux") { - cc::Build::new() - .file("c/capture_argv.c") - .compile("capture_argv"); - } -} diff --git a/lib/foundation/fabro-proc/c/capture_argv.c b/lib/foundation/fabro-proc/c/capture_argv.c deleted file mode 100644 index b719a78fc..000000000 --- a/lib/foundation/fabro-proc/c/capture_argv.c +++ /dev/null @@ -1,16 +0,0 @@ -#include - -static char *g_argv_start = 0; -static unsigned long g_argv_len = 0; - -__attribute__((constructor)) -static void capture_argv(int argc, char **argv, char **envp) { - (void)envp; - if (argc <= 0 || !argv || !argv[0]) return; - g_argv_start = argv[0]; - char *end = argv[argc - 1] + strlen(argv[argc - 1]) + 1; - g_argv_len = (unsigned long)(end - argv[0]); -} - -char *fabro_proctitle_argv_start(void) { return g_argv_start; } -unsigned long fabro_proctitle_argv_len(void) { return g_argv_len; } diff --git a/lib/foundation/fabro-proc/src/title.rs b/lib/foundation/fabro-proc/src/title.rs index f1f087636..29ada5849 100644 --- a/lib/foundation/fabro-proc/src/title.rs +++ b/lib/foundation/fabro-proc/src/title.rs @@ -13,12 +13,6 @@ unsafe impl Sync for Buffer {} static STATE: OnceLock> = OnceLock::new(); -#[cfg(target_os = "linux")] -unsafe extern "C" { - fn fabro_proctitle_argv_start() -> *mut libc::c_char; - fn fabro_proctitle_argv_len() -> libc::c_ulong; -} - #[cfg(target_os = "macos")] unsafe extern "C" { fn _NSGetArgv() -> *mut *mut *mut libc::c_char; @@ -73,23 +67,38 @@ fn write_title(dst: &mut [u8], title: &[u8]) { } #[cfg(target_os = "linux")] +#[expect( + clippy::disallowed_methods, + reason = "process-title initialization reads this process's kernel-owned argv bounds once" +)] fn platform_init() -> Option { - // SAFETY: these symbols are provided by the Linux-only C object compiled in - // build.rs. - let start = unsafe { fabro_proctitle_argv_start() }; - // SAFETY: paired with the symbol above. - let len = unsafe { fabro_proctitle_argv_len() }; - let len = usize::try_from(len).ok()?; - if start.is_null() || len == 0 { - return None; - } - + // Linux exposes the original argv span independently of libc. In particular, + // musl does not pass argc/argv to C constructors, unlike glibc. + let stat = std::fs::read_to_string("/proc/self/stat").ok()?; + let (start, len) = linux_argv_span(&stat)?; Some(Buffer { - start: start.cast(), + // The kernel reports this process's writable, initial argv allocation. + // No code in this crate relocates or frees that allocation. + start: std::ptr::with_exposed_provenance_mut(start), len, }) } +#[cfg(any(target_os = "linux", test))] +fn linux_argv_span(stat: &str) -> Option<(usize, usize)> { + // comm (field 2) can contain spaces and ')'; the final ')' ends it. + let (_, tail) = stat.rsplit_once(')')?; + let mut fields = tail.split_whitespace(); + // The tail starts at field 3; arg_start and arg_end are fields 48 and 49. + let start: usize = fields.nth(45)?.parse().ok()?; + let end: usize = fields.next()?.parse().ok()?; + let len = end.checked_sub(start)?; + if start == 0 || len == 0 || len > isize::MAX as usize { + return None; + } + Some((start, len)) +} + #[cfg(target_os = "macos")] fn platform_init() -> Option { // SAFETY: macOS exposes argc/argv through crt_externs for the current process. @@ -140,7 +149,23 @@ fn platform_init() -> Option { #[cfg(test)] mod tests { - use super::write_title; + use super::{linux_argv_span, write_title}; + + #[test] + fn linux_argv_bounds_handle_parentheses_in_the_process_name() { + let fields = vec!["0"; 45].join(" "); + let stat = format!("42 (a name ) with (parens)) {fields} 4096 4160 5000 5100 0"); + assert_eq!(linux_argv_span(&stat), Some((4096, 64))); + } + + #[test] + fn linux_argv_bounds_refuse_missing_or_invalid_addresses() { + let fields = vec!["0"; 45].join(" "); + for addresses in ["", "4096", "0 64", "4096 4096", "4160 4096", "x 4160"] { + let stat = format!("42 (probe) {fields} {addresses}"); + assert_eq!(linux_argv_span(&stat), None, "{stat}"); + } + } #[test] fn write_title_zero_fills_remainder() { diff --git a/lib/foundation/fabro-proc/tests/title.rs b/lib/foundation/fabro-proc/tests/title.rs new file mode 100644 index 000000000..7f6d4f1b3 --- /dev/null +++ b/lib/foundation/fabro-proc/tests/title.rs @@ -0,0 +1,40 @@ +//! Exercise the real argv allocation in a separate process, including musl. +#![cfg(target_os = "linux")] +#![expect( + clippy::disallowed_methods, + reason = "the test launches an isolated copy of itself and reads its kernel process title" +)] + +use std::process::Command; +use std::{env, fs}; + +const TITLE: &str = "fabro title-probe running"; + +#[test] +fn process_title_is_visible_in_proc() { + let output = Command::new(env::current_exe().unwrap()) + .args(["--exact", "title_child", "--ignored", "--nocapture"]) + .output() + .unwrap(); + assert!(output.status.success(), "{output:?}"); +} + +#[test] +#[ignore = "launched by process_title_is_visible_in_proc to isolate argv mutation"] +fn title_child() { + let len = fabro_proc::title_init(); + assert!(len > TITLE.len(), "no usable argv buffer: {len}"); + fabro_proc::title_set(TITLE); + let cmdline = fs::read("/proc/self/cmdline").unwrap(); + assert_eq!( + cmdline.split(|byte| *byte == 0).next().unwrap(), + TITLE.as_bytes() + ); + assert_eq!(fabro_proc::title_init(), len); + fabro_proc::title_set("fabro done"); + let cmdline = fs::read("/proc/self/cmdline").unwrap(); + assert_eq!( + cmdline.split(|byte| *byte == 0).next().unwrap(), + b"fabro done" + ); +}