From fb2174c7d0b6ab1fcad2a2463f40c8d8f6e5d59a Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp <19+brynary@users.noreply.github.com> Date: Thu, 21 May 2026 19:48:54 -0400 Subject: [PATCH] feat(agent): expose Fabro run tools in sessions (#339) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary API-backed Fabro agent sessions can now use the same five run-control tools that were previously MCP-only, while the server uses a single scoped `FABRO_WORKER_TOKEN` path for worker authorization. This lets server-dispatched API agents create, gather, inspect, and interact with runs without reintroducing a separate delegated run-agent token or leaking credentials into sandboxed child environments. ## Changes - Moved the reusable run-tool implementation into the new `fabro-tool` crate so MCP and API agent backends share the same tool schemas and client behavior. - Registered the five `fabro_run_*` tools for API-mode agents, with ACP sessions continuing to omit those tools. - Replaced `FABRO_RUN_AGENT_TOKEN` with scoped worker-token auth: base worker tokens keep same-run access, and `run:worker agent:run_tools` tokens can call the run-control API across runs. - Added server auth guards for run-tool actors and run-scoped-or-run-tools routes, then applied them only to the routes used by the run-tool client backend. - Kept `FABRO_WORKER_TOKEN` scrubbed from sandbox commands, hooks, ACP subprocesses, MCP env providers, and other child tool environments. ## Testing - `cargo nextest run -p fabro-server worker_token principal_middleware spawn_env --no-fail-fast` - `cargo nextest run -p fabro-cli runner --no-fail-fast` - `cargo nextest run -p fabro-workflow agent_run --no-fail-fast` - `cargo nextest run -p fabro-static --no-fail-fast` - `cargo +nightly-2026-04-14 fmt --check --all` - `cargo +nightly-2026-04-14 clippy -p fabro-server -p fabro-cli -p fabro-static --all-targets -- -D warnings` - `git diff --check` --- [![Compound Engineering](https://img.shields.io/badge/Compound_Engineering-6366f1)](https://github.com/EveryInc/compound-engineering-plugin) 🤖 Generated with GPT-5 via [Codex](https://openai.com/codex) --- Cargo.lock | 25 + lib/crates/fabro-cli/Cargo.toml | 1 + .../fabro-cli/src/commands/run/runner.rs | 67 +- lib/crates/fabro-cli/src/main.rs | 8 +- lib/crates/fabro-mcp-server/Cargo.toml | 1 + lib/crates/fabro-mcp-server/src/lib.rs | 2 +- .../fabro-mcp-server/src/manifest_builder.rs | 36 + lib/crates/fabro-mcp-server/src/run_tools.rs | 21 - .../fabro-mcp-server/src/run_tools/common.rs | 215 ------ .../fabro-mcp-server/src/run_tools/create.rs | 411 ----------- .../src/run_tools/manifest.rs | 188 ----- lib/crates/fabro-mcp-server/src/server.rs | 113 +-- lib/crates/fabro-server/Cargo.toml | 1 + lib/crates/fabro-server/src/lib.rs | 1 + .../fabro-server/src/principal_middleware.rs | 158 ++++- .../fabro-server/src/run_tool_manifest.rs | 119 ++++ lib/crates/fabro-server/src/server.rs | 14 +- .../fabro-server/src/server/handler/events.rs | 10 +- .../src/server/handler/lifecycle.rs | 55 +- .../fabro-server/src/server/handler/runs.rs | 66 +- .../fabro-server/src/server/handler/steer.rs | 27 +- lib/crates/fabro-server/src/server/tests.rs | 257 ++++++- lib/crates/fabro-server/src/spawn_env.rs | 2 + lib/crates/fabro-server/src/worker_token.rs | 149 +++- lib/crates/fabro-tool/Cargo.toml | 31 + lib/crates/fabro-tool/src/common.rs | 319 +++++++++ lib/crates/fabro-tool/src/create.rs | 646 ++++++++++++++++++ .../run_tools => fabro-tool/src}/events.rs | 79 ++- lib/crates/fabro-tool/src/fabro_client.rs | 141 ++++ .../run_tools => fabro-tool/src}/gather.rs | 41 +- .../run_tools => fabro-tool/src}/interact.rs | 79 ++- lib/crates/fabro-tool/src/lib.rs | 41 ++ lib/crates/fabro-tool/src/manifest.rs | 76 +++ .../run_tools => fabro-tool/src}/search.rs | 94 +-- lib/crates/fabro-workflow/Cargo.toml | 2 + .../fabro-workflow/src/handler/llm/api.rs | 524 +++++++++++++- .../fabro-workflow/src/operations/start.rs | 6 + .../src/pipeline/execute/tests.rs | 3 + .../fabro-workflow/src/pipeline/initialize.rs | 19 +- .../fabro-workflow/src/pipeline/types.rs | 3 +- lib/crates/fabro-workflow/src/services.rs | 10 +- 41 files changed, 2881 insertions(+), 1180 deletions(-) create mode 100644 lib/crates/fabro-mcp-server/src/manifest_builder.rs delete mode 100644 lib/crates/fabro-mcp-server/src/run_tools.rs delete mode 100644 lib/crates/fabro-mcp-server/src/run_tools/common.rs delete mode 100644 lib/crates/fabro-mcp-server/src/run_tools/create.rs delete mode 100644 lib/crates/fabro-mcp-server/src/run_tools/manifest.rs create mode 100644 lib/crates/fabro-server/src/run_tool_manifest.rs create mode 100644 lib/crates/fabro-tool/Cargo.toml create mode 100644 lib/crates/fabro-tool/src/common.rs create mode 100644 lib/crates/fabro-tool/src/create.rs rename lib/crates/{fabro-mcp-server/src/run_tools => fabro-tool/src}/events.rs (83%) create mode 100644 lib/crates/fabro-tool/src/fabro_client.rs rename lib/crates/{fabro-mcp-server/src/run_tools => fabro-tool/src}/gather.rs (74%) rename lib/crates/{fabro-mcp-server/src/run_tools => fabro-tool/src}/interact.rs (91%) create mode 100644 lib/crates/fabro-tool/src/lib.rs create mode 100644 lib/crates/fabro-tool/src/manifest.rs rename lib/crates/{fabro-mcp-server/src/run_tools => fabro-tool/src}/search.rs (87%) diff --git a/Cargo.lock b/Cargo.lock index 6ae477e5b..d5c0e49db 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1753,6 +1753,7 @@ dependencies = [ "fabro-telemetry", "fabro-template", "fabro-test", + "fabro-tool", "fabro-types", "fabro-util", "fabro-validate", @@ -2114,6 +2115,7 @@ dependencies = [ "fabro-manifest", "fabro-model", "fabro-server", + "fabro-tool", "fabro-types", "fabro-util", "futures", @@ -2278,6 +2280,7 @@ dependencies = [ "fabro-static", "fabro-store", "fabro-test", + "fabro-tool", "fabro-types", "fabro-util", "fabro-validate", @@ -2446,6 +2449,26 @@ dependencies = [ "twin-openai", ] +[[package]] +name = "fabro-tool" +version = "0.240.0-nightly.1" +dependencies = [ + "anyhow", + "async-trait", + "chrono", + "fabro-api", + "fabro-client", + "fabro-types", + "fabro-util", + "futures", + "schemars 1.2.1", + "serde", + "serde_json", + "tempfile", + "tokio", + "toml 0.8.23", +] + [[package]] name = "fabro-tracker" version = "0.240.0-nightly.1" @@ -2539,6 +2562,7 @@ dependencies = [ "dirs", "fabro-acp", "fabro-agent", + "fabro-api", "fabro-auth", "fabro-checkpoint", "fabro-config", @@ -2560,6 +2584,7 @@ dependencies = [ "fabro-store", "fabro-template", "fabro-test", + "fabro-tool", "fabro-types", "fabro-util", "fabro-validate", diff --git a/lib/crates/fabro-cli/Cargo.toml b/lib/crates/fabro-cli/Cargo.toml index 4d3b66d4d..4bff98f8d 100644 --- a/lib/crates/fabro-cli/Cargo.toml +++ b/lib/crates/fabro-cli/Cargo.toml @@ -51,6 +51,7 @@ fabro-util = { path = "../fabro-util" } fabro-http.workspace = true fabro-static.workspace = true fabro-template = { path = "../fabro-template" } +fabro-tool = { path = "../fabro-tool" } clap.workspace = true clap_complete.workspace = true cli-table.workspace = true diff --git a/lib/crates/fabro-cli/src/commands/run/runner.rs b/lib/crates/fabro-cli/src/commands/run/runner.rs index adc6e8c46..775731075 100644 --- a/lib/crates/fabro-cli/src/commands/run/runner.rs +++ b/lib/crates/fabro-cli/src/commands/run/runner.rs @@ -11,12 +11,16 @@ use std::time::Duration; use anyhow::{Context, Result, anyhow}; use async_trait::async_trait; +use fabro_api::types::RunManifest; +use fabro_config::user::active_settings_path; use fabro_config::{ServerSettingsBuilder, Storage, load_llm_catalog_settings}; use fabro_interview::{ AnswerSubmission, ControlInterviewer, WorkerControlEnvelope, WorkerControlMessage, }; use fabro_model::Catalog; +use fabro_server::run_tool_manifest; use fabro_store::{EventEnvelope, RunProjection, RunProjectionReducer}; +use fabro_tool::fabro_client::ClientBackend; use fabro_types::settings::InterpString; use fabro_types::settings::run::{RunMode, RunNamespace}; use fabro_types::{ @@ -29,6 +33,7 @@ use fabro_workflow::event::{Emitter, RunEventSink}; use fabro_workflow::operations::{self, StartServices}; use fabro_workflow::run_control::RunControlState; use fabro_workflow::runtime_store::{RunStoreBackend, RunStoreHandle}; +use fabro_workflow::services::FabroRunToolServices; #[cfg(unix)] use tokio::signal::unix::{SignalKind, signal}; use tokio::sync::{Mutex, RwLock as AsyncRwLock, mpsc}; @@ -77,11 +82,25 @@ pub(crate) async fn execute( .await .with_context(|| format!("failed to load run state for {run_id}"))?; let run_spec = &run_state.spec; + let llm_catalog_settings = + load_llm_catalog_settings(None).context("failed to load worker LLM catalog settings")?; + let catalog = Arc::new( + Catalog::from_builtin_with_overrides(&llm_catalog_settings) + .context("failed to build worker LLM catalog")?, + ); let artifact_sink = Some(ArtifactSink::Uploader(build_artifact_uploader( run_id, client.clone_for_reuse(), worker_token.to_owned(), ))); + let fabro_run_tools = build_fabro_run_tool_services( + worker_token, + client.clone_for_reuse(), + run_id, + run_spec.source_directory.as_deref(), + &run_dir, + Arc::clone(&catalog), + ); let interviewer = Arc::new(ControlInterviewer::new()); let cancel_token = CancellationToken::new(); let emitter = Arc::new(Emitter::new(run_id)); @@ -94,12 +113,6 @@ pub(crate) async fn execute( let run_control = RunControlState::new(); install_signal_handlers(Arc::clone(&run_control), cancel_token.clone())?; let vault = load_worker_vault(storage_dir.as_deref())?; - let llm_catalog_settings = - load_llm_catalog_settings(None).context("failed to load worker LLM catalog settings")?; - let catalog = Arc::new( - Catalog::from_builtin_with_overrides(&llm_catalog_settings) - .context("failed to build worker LLM catalog")?, - ); let github_app = { let vault_guard = match &vault { Some(arc) => Some(arc.read().await), @@ -137,6 +150,7 @@ pub(crate) async fn execute( catalog, on_node: None, registry_override: None, + fabro_run_tools, }; match mode { @@ -151,6 +165,47 @@ pub(crate) async fn execute( Ok(()) } +fn build_fabro_run_tool_services( + worker_token: &str, + client: fabro_client::Client, + current_run_id: RunId, + source_directory: Option<&str>, + run_dir: &Path, + catalog: Arc, +) -> Option { + if worker_token.trim().is_empty() { + return None; + } + let backend = ClientBackend::new(Arc::new(client)) + .with_manifest_builder(Arc::new(WorkerRunManifestBuilder { catalog })); + Some(FabroRunToolServices { + backend: Arc::new(backend), + current_run_id, + base_cwd: source_directory.map_or_else(|| run_dir.to_path_buf(), PathBuf::from), + user_settings_path: active_settings_path(None), + }) +} + +struct WorkerRunManifestBuilder { + catalog: Arc, +} + +impl fabro_tool::RunManifestBuilder for WorkerRunManifestBuilder { + fn build_run_manifest( + &self, + spec: &fabro_tool::ValidatedCreateRunSpec, + cwd: &Path, + user_settings_path: &Path, + ) -> fabro_tool::ToolResult { + run_tool_manifest::build_run_tool_manifest( + spec, + cwd, + user_settings_path, + Arc::clone(&self.catalog), + ) + } +} + fn load_worker_vault(storage_dir: Option<&Path>) -> Result>>> { let Some(storage_dir) = storage_dir else { return Ok(None); diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index 7cf2cfa98..c3402f1a7 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -54,11 +54,11 @@ async fn main() { // Capture the worker bearer token immediately and scrub it from the process // env before any subprocess can be spawned. Every descendant of the worker // (hooks, sandbox commands, devcontainer setup, MCP stdio, etc.) therefore - // inherits a process env that no longer contains FABRO_WORKER_TOKEN, so an + // inherits a process env that no longer contains this credential, so an // unscrubbed spawn site cannot leak it. The token flows to `runner::execute` - // through an explicit function argument instead of the environment. + // through explicit function arguments instead of the environment. let worker_token = if subcommand == Some("__run-worker") { - let token = process_env_var(EnvVars::FABRO_WORKER_TOKEN); + let worker_token = process_env_var(EnvVars::FABRO_WORKER_TOKEN); #[expect( clippy::disallowed_methods, reason = "Scrub the worker bearer from this process's env before any \ @@ -67,7 +67,7 @@ async fn main() { { std::env::remove_var(EnvVars::FABRO_WORKER_TOKEN); } - token + worker_token } else { None }; diff --git a/lib/crates/fabro-mcp-server/Cargo.toml b/lib/crates/fabro-mcp-server/Cargo.toml index 8a7f57f22..118fa1ddc 100644 --- a/lib/crates/fabro-mcp-server/Cargo.toml +++ b/lib/crates/fabro-mcp-server/Cargo.toml @@ -21,6 +21,7 @@ fabro-manifest = { path = "../fabro-manifest" } fabro-config = { path = "../fabro-config" } fabro-model = { path = "../fabro-model" } fabro-server = { path = "../fabro-server" } +fabro-tool = { path = "../fabro-tool" } fabro-types = { path = "../fabro-types" } fabro-util = { path = "../fabro-util" } futures.workspace = true diff --git a/lib/crates/fabro-mcp-server/src/lib.rs b/lib/crates/fabro-mcp-server/src/lib.rs index b8351bab5..ad6264dcb 100644 --- a/lib/crates/fabro-mcp-server/src/lib.rs +++ b/lib/crates/fabro-mcp-server/src/lib.rs @@ -1,5 +1,5 @@ mod config; -mod run_tools; +mod manifest_builder; mod server; use std::future::Future; diff --git a/lib/crates/fabro-mcp-server/src/manifest_builder.rs b/lib/crates/fabro-mcp-server/src/manifest_builder.rs new file mode 100644 index 000000000..53e899f1e --- /dev/null +++ b/lib/crates/fabro-mcp-server/src/manifest_builder.rs @@ -0,0 +1,36 @@ +use std::path::Path; +use std::sync::Arc; + +use fabro_api::types; +use fabro_config::load_llm_catalog_settings; +use fabro_model::Catalog; +use fabro_server::run_tool_manifest; +use fabro_tool::{RunManifestBuilder, ToolError, ToolResult, ValidatedCreateRunSpec}; + +#[derive(Default)] +pub(crate) struct McpRunManifestBuilder; + +impl RunManifestBuilder for McpRunManifestBuilder { + fn build_run_manifest( + &self, + spec: &ValidatedCreateRunSpec, + cwd: &Path, + user_settings_path: &Path, + ) -> ToolResult { + build_mcp_run_manifest(spec, cwd, user_settings_path) + } +} + +fn build_mcp_run_manifest( + spec: &ValidatedCreateRunSpec, + cwd: &Path, + user_settings_path: &Path, +) -> ToolResult { + let llm_catalog_settings = load_llm_catalog_settings(Some(user_settings_path)) + .map_err(|err| ToolError::message(err.to_string()))?; + let catalog = Arc::new( + Catalog::from_builtin_with_overrides(&llm_catalog_settings) + .map_err(|err| ToolError::message(err.to_string()))?, + ); + run_tool_manifest::build_run_tool_manifest(spec, cwd, user_settings_path, catalog) +} diff --git a/lib/crates/fabro-mcp-server/src/run_tools.rs b/lib/crates/fabro-mcp-server/src/run_tools.rs deleted file mode 100644 index 78c5ed14f..000000000 --- a/lib/crates/fabro-mcp-server/src/run_tools.rs +++ /dev/null @@ -1,21 +0,0 @@ -#![allow( - dead_code, - reason = "MCP DTO fields are consumed by serde and schema generation even when not read directly." -)] - -mod common; -mod create; -mod events; -mod gather; -mod interact; -mod manifest; -mod search; - -pub(crate) use common::{ToolError, error_result, success_result}; -pub(crate) use create::{FabroRunCreateParams, ValidatedCreateRuns, create_runs, create_runs_text}; -pub(crate) use events::{FabroRunEventsParams, ValidatedRunEvents, run_events, run_events_text}; -pub(crate) use gather::{FabroRunGatherParams, ValidatedGatherRuns, gather_runs, gather_runs_text}; -pub(crate) use interact::{ - FabroRunInteractParams, ValidatedInteractRun, interact_run, interact_run_text, -}; -pub(crate) use search::{FabroRunSearchParams, ValidatedSearchRuns, search_runs, search_runs_text}; diff --git a/lib/crates/fabro-mcp-server/src/run_tools/common.rs b/lib/crates/fabro-mcp-server/src/run_tools/common.rs deleted file mode 100644 index 42cbcbae8..000000000 --- a/lib/crates/fabro-mcp-server/src/run_tools/common.rs +++ /dev/null @@ -1,215 +0,0 @@ -use std::collections::HashMap; - -use chrono::{DateTime, NaiveDate, Utc}; -use fabro_client::Client; -use fabro_types::{Run, RunId, RunStatus}; -use fabro_util::exit::{self, ExitClass}; -use rmcp::model::{CallToolResult, Content}; -use schemars::JsonSchema; -use serde::Serialize; - -#[derive(Debug)] -pub(crate) struct ToolError { - message: String, -} - -impl ToolError { - pub(crate) fn message(message: impl Into) -> Self { - Self { - message: message.into(), - } - } - - pub(crate) fn from_anyhow(err: &anyhow::Error) -> Self { - Self::message(format_tool_error(err)) - } - - pub(crate) fn as_str(&self) -> &str { - &self.message - } -} - -pub(super) type ToolResult = Result; - -#[derive(Debug, Serialize, JsonSchema)] -pub(crate) struct RunSummaryResult { - pub(crate) run_id: String, - pub(crate) parent_id: Option, - pub(crate) children_count: u64, - pub(crate) workflow_name: Option, - pub(crate) workflow_graph_name: Option, - pub(crate) workflow_slug: Option, - pub(crate) status: String, - pub(crate) archived: bool, - pub(crate) created_at: String, - pub(crate) started_at: Option, - pub(crate) completed_at: Option, - pub(crate) labels: HashMap, - pub(crate) source_directory: Option, - pub(crate) repo_origin_url: Option, - pub(crate) goal: String, -} - -pub(crate) fn success_result( - value: &T, - text: impl Into, -) -> Result { - let structured_content = serde_json::to_value(value).map_err(|err| { - rmcp::ErrorData::internal_error( - format!("failed to serialize Fabro MCP tool result: {err}"), - None, - ) - })?; - let mut result = CallToolResult::structured(structured_content); - result.content = vec![Content::text(text.into())]; - Ok(result) -} - -pub(crate) fn error_result(err: ToolError) -> CallToolResult { - CallToolResult::error(vec![Content::text(err.message)]) -} - -pub(super) fn validate_len(name: &str, len: usize, min: usize, max: usize) -> ToolResult<()> { - if len < min { - return Err(ToolError::message(format!( - "{name} must contain at least {min} item(s)" - ))); - } - if len > max { - return Err(ToolError::message(format!( - "{name} must contain no more than {max} item(s)" - ))); - } - Ok(()) -} - -pub(super) async fn retrieve_run(client: &Client, run_id: &RunId) -> ToolResult { - client - .retrieve_run(run_id) - .await - .map_err(|err| ToolError::from_anyhow(&err)) -} - -pub(super) fn run_summary_result(run: &Run) -> RunSummaryResult { - RunSummaryResult { - run_id: run.id.to_string(), - parent_id: run.parent_id.map(|parent_id| parent_id.to_string()), - children_count: run.children_count, - workflow_name: run.workflow.name.clone(), - workflow_graph_name: run.workflow.graph_name.clone(), - workflow_slug: run.workflow.slug.clone(), - status: run_status_kind(run.lifecycle.status).to_string(), - archived: run.lifecycle.archived, - created_at: run.timestamps.created_at.to_rfc3339(), - started_at: run - .timestamps - .started_at - .map(|timestamp| timestamp.to_rfc3339()), - completed_at: run - .timestamps - .completed_at - .map(|timestamp| timestamp.to_rfc3339()), - labels: run.labels.clone(), - source_directory: run.source_directory.clone(), - repo_origin_url: run - .repository - .as_ref() - .and_then(|repository| repository.origin_url.clone()), - goal: run.goal.clone(), - } -} - -pub(super) fn parse_datetime_filter(name: &str, raw: &str) -> ToolResult> { - if let Ok(timestamp) = DateTime::parse_from_rfc3339(raw) { - return Ok(timestamp.with_timezone(&Utc)); - } - let date = NaiveDate::parse_from_str(raw, "%Y-%m-%d").map_err(|err| { - ToolError::message(format!("{name} must be RFC3339 or YYYY-MM-DD: {err}")) - })?; - let datetime = date - .and_hms_opt(0, 0, 0) - .ok_or_else(|| ToolError::message(format!("{name} contains an invalid date")))?; - Ok(DateTime::from_naive_utc_and_offset(datetime, Utc)) -} - -pub(super) fn run_status_kind(status: RunStatus) -> &'static str { - status.kind().into() -} - -fn format_tool_error(err: &anyhow::Error) -> String { - let mut rendered = format!("{err:#}"); - if exit::exit_class_for(err) == Some(ExitClass::AuthRequired) - && !rendered.contains("fabro auth login") - { - rendered.push_str("\nRun `fabro auth login` to authenticate."); - } - rendered -} - -#[cfg(test)] -mod tests { - use chrono::{TimeZone, Utc}; - use fabro_types::{RunLifecycle, RunLinks, RunOrigin, RunTimestamps, WorkflowRef}; - - use super::*; - - #[test] - fn run_summary_result_includes_parent_metadata() { - let parent_id = run_id("01KRBZW4DW0000000000000002"); - let run = Run { - id: run_id("01KRBZW5C00000000000000001"), - parent_id: Some(parent_id), - children_count: 3, - title: "test".to_string(), - goal: "test".to_string(), - workflow: WorkflowRef { - slug: Some("simple".to_string()), - name: Some("Simple".to_string()), - graph_name: Some("GraphName".to_string()), - node_count: 0, - edge_count: 0, - }, - automation: None, - repository: None, - created_by: None, - origin: RunOrigin::default(), - labels: HashMap::new(), - lifecycle: RunLifecycle { - status: RunStatus::Submitted, - pending_control: None, - queue_position: None, - error: None, - archived: false, - archived_at: None, - }, - sandbox: None, - models: Vec::new(), - source_directory: None, - timestamps: RunTimestamps { - created_at: Utc.with_ymd_and_hms(2026, 5, 11, 12, 0, 0).unwrap(), - started_at: None, - last_event_at: None, - completed_at: None, - duration_ms: None, - elapsed_secs: None, - }, - billing: None, - diff: None, - pull_request: None, - current_question: None, - superseded_by: None, - links: RunLinks { web: None }, - }; - - let summary = run_summary_result(&run); - - assert_eq!(summary.parent_id, Some(parent_id.to_string())); - assert_eq!(summary.children_count, 3); - assert_eq!(summary.workflow_name.as_deref(), Some("Simple")); - assert_eq!(summary.workflow_graph_name.as_deref(), Some("GraphName")); - } - - fn run_id(raw: &str) -> RunId { - raw.parse().expect("test run id should parse") - } -} diff --git a/lib/crates/fabro-mcp-server/src/run_tools/create.rs b/lib/crates/fabro-mcp-server/src/run_tools/create.rs deleted file mode 100644 index 77d67615e..000000000 --- a/lib/crates/fabro-mcp-server/src/run_tools/create.rs +++ /dev/null @@ -1,411 +0,0 @@ -use std::borrow::Cow; -use std::collections::HashMap; -use std::path::{Path, PathBuf}; -use std::sync::Arc; - -use fabro_client::Client; -use fabro_types::RunId; -use schemars::{JsonSchema, Schema, SchemaGenerator, json_schema}; -use serde::{Deserialize, Serialize}; -use serde_json::Value; - -use super::common::{ToolError, ToolResult}; -use super::{common, manifest}; - -#[derive(Debug, Deserialize, JsonSchema)] -pub(crate) struct FabroRunCreateParams { - pub(crate) runs: Vec, -} - -#[derive(Debug, Deserialize, JsonSchema)] -pub(crate) struct CreateRunSpec { - pub(crate) workflow: String, - pub(crate) cwd: Option, - pub(crate) run_id: Option, - pub(crate) parent_id: Option, - pub(crate) goal: Option, - #[serde(default)] - pub(crate) inputs: HashMap, - #[serde(default)] - pub(crate) labels: HashMap, - pub(crate) dry_run: Option, - pub(crate) auto_approve: Option, - pub(crate) model: Option, - pub(crate) provider: Option, - pub(crate) sandbox: Option, - pub(crate) preserve_sandbox: Option, - pub(crate) start: Option, -} - -#[derive(Debug, Deserialize)] -#[serde(transparent)] -pub(crate) struct RunInputValue(Value); - -impl From for RunInputValue { - fn from(value: Value) -> Self { - Self(value) - } -} - -impl RunInputValue { - fn into_inner(self) -> Value { - self.0 - } -} - -impl JsonSchema for RunInputValue { - fn inline_schema() -> bool { - true - } - - fn schema_name() -> Cow<'static, str> { - "RunInputValue".into() - } - - fn json_schema(_: &mut SchemaGenerator) -> Schema { - json_schema!({ - "description": "Run input override value. Inputs are TOML-compatible scalar values: string, boolean, integer, or float.", - "anyOf": [ - { "type": "string" }, - { "type": "boolean" }, - { "type": "integer" }, - { "type": "number" } - ] - }) - } -} - -#[derive(Debug)] -pub(crate) struct ValidatedCreateRuns { - pub(crate) runs: Vec, -} - -#[derive(Debug)] -pub(crate) struct ValidatedCreateRunSpec { - pub(crate) workflow: String, - pub(crate) cwd: Option, - pub(crate) run_id: Option, - pub(crate) parent_id: Option, - pub(crate) goal: Option, - pub(crate) inputs: HashMap, - pub(crate) labels: HashMap, - pub(crate) dry_run: Option, - pub(crate) auto_approve: Option, - pub(crate) model: Option, - pub(crate) provider: Option, - pub(crate) sandbox: Option, - pub(crate) preserve_sandbox: Option, - pub(crate) start: Option, -} - -impl TryFrom for ValidatedCreateRuns { - type Error = ToolError; - - fn try_from(params: FabroRunCreateParams) -> Result { - common::validate_len("runs", params.runs.len(), 1, 50)?; - let runs = params - .runs - .into_iter() - .map(ValidatedCreateRunSpec::try_from) - .collect::, _>>()?; - Ok(Self { runs }) - } -} - -impl TryFrom for ValidatedCreateRunSpec { - type Error = ToolError; - - fn try_from(spec: CreateRunSpec) -> Result { - let run_id = spec - .run_id - .as_deref() - .map(str::parse::) - .transpose() - .map_err(|err| { - ToolError::message(format!("run_id must be a valid Fabro run id: {err}")) - })?; - let parent_id = spec - .parent_id - .as_deref() - .map(str::trim) - .filter(|parent_id| !parent_id.is_empty()) - .map(ToOwned::to_owned); - if spec.parent_id.is_some() && parent_id.is_none() { - return Err(ToolError::message("parent_id must not be blank")); - } - let inputs = spec - .inputs - .into_iter() - .map(|(key, value)| { - let value = value.into_inner(); - manifest::json_to_toml_value(&key, &value).map(|value| (key, value)) - }) - .collect::>>()?; - Ok(Self { - workflow: spec.workflow, - cwd: spec.cwd, - run_id, - parent_id, - goal: spec.goal, - inputs, - labels: spec.labels, - dry_run: spec.dry_run, - auto_approve: spec.auto_approve, - model: spec.model, - provider: spec.provider, - sandbox: spec.sandbox, - preserve_sandbox: spec.preserve_sandbox, - start: spec.start, - }) - } -} - -#[derive(Debug, Serialize, JsonSchema)] -pub(crate) struct CreateRunsResult { - pub(crate) runs: Vec, -} - -#[derive(Debug, Serialize, JsonSchema)] -pub(crate) struct CreatedRunResult { - pub(crate) run_id: String, - pub(crate) parent_id: Option, - pub(crate) children_count: u64, - pub(crate) workflow: String, - pub(crate) started: bool, - pub(crate) status: String, -} - -pub(crate) async fn create_runs( - client: Arc, - base_cwd: &Path, - user_settings_path: &Path, - params: ValidatedCreateRuns, -) -> ToolResult { - let mut created = Vec::with_capacity(params.runs.len()); - for spec in params.runs { - let cwd = spec.cwd.clone().unwrap_or_else(|| base_cwd.to_path_buf()); - let mut manifest = manifest::build_mcp_run_manifest(&spec, &cwd, user_settings_path)?; - if let Some(parent_selector) = spec.parent_id.as_deref() { - let parent_id = client - .resolve_run(parent_selector) - .await - .map_err(|err| ToolError::from_anyhow(&err))? - .id; - manifest.parent_id = Some(parent_id.to_string()); - } - let run_id = client - .create_run_from_manifest(manifest) - .await - .map_err(|err| ToolError::from_anyhow(&err))?; - let started = spec.start.unwrap_or(true); - let summary = if started { - client - .start_run(&run_id, false) - .await - .map_err(|err| ToolError::from_anyhow(&err))? - } else { - client - .retrieve_run(&run_id) - .await - .map_err(|err| ToolError::from_anyhow(&err))? - }; - created.push(CreatedRunResult { - run_id: summary.id.to_string(), - parent_id: summary.parent_id.map(|parent_id| parent_id.to_string()), - children_count: summary.children_count, - workflow: spec.workflow, - started, - status: common::run_status_kind(summary.lifecycle.status).to_string(), - }); - } - Ok(CreateRunsResult { runs: created }) -} - -pub(crate) fn create_runs_text(result: &CreateRunsResult) -> String { - let started = result.runs.iter().filter(|run| run.started).count(); - format!( - "created {} Fabro run(s), started {started}", - result.runs.len() - ) -} - -#[cfg(test)] -mod tests { - use schemars::SchemaGenerator; - use serde_json::json; - use tokio::fs; - - use super::*; - - #[test] - fn run_input_value_schema_allows_only_json_scalars() { - let mut generator = SchemaGenerator::default(); - let schema = RunInputValue::json_schema(&mut generator); - let schema = serde_json::to_value(schema).expect("schema should serialize"); - - assert_eq!( - schema["anyOf"], - json!([ - { "type": "string" }, - { "type": "boolean" }, - { "type": "integer" }, - { "type": "number" }, - ]) - ); - } - - #[test] - fn create_spec_accepts_parent_selector() { - let spec = ValidatedCreateRunSpec::try_from(CreateRunSpec { - workflow: "simple.fabro".to_string(), - cwd: None, - run_id: None, - parent_id: Some(" nightly-parent ".to_string()), - goal: None, - inputs: HashMap::new(), - labels: HashMap::new(), - dry_run: None, - auto_approve: None, - model: None, - provider: None, - sandbox: None, - preserve_sandbox: None, - start: None, - }) - .expect("parent selectors should validate without requiring exact run ids"); - - assert_eq!(spec.parent_id.as_deref(), Some("nightly-parent")); - } - - #[tokio::test] - async fn create_runs_resolves_parent_selector_and_sends_parent_id_in_manifest() { - let temp = tempfile::tempdir().expect("tempdir should be created"); - let workflow = temp.path().join("simple.fabro"); - fs::write( - &workflow, - r#"digraph Simple { - graph [goal="Run tests and report results"] - start [shape=Mdiamond, label="Start"] - exit [shape=Msquare, label="Exit"] - start -> exit -} -"#, - ) - .await - .expect("workflow should be written"); - let settings = temp.path().join("settings.toml"); - fs::write(&settings, "") - .await - .expect("settings should be written"); - - let server = httpmock::MockServer::start(); - let child_id = run_id("01KRBZW5C00000000000000001"); - let parent_id = run_id("01KRBZW4DW0000000000000002"); - let resolve_parent = server.mock(|when, then| { - when.method("GET") - .path("/api/v1/runs/resolve") - .query_param("selector", "nightly-parent"); - then.status(200) - .header("Content-Type", "application/json") - .json_body(run_summary_json(parent_id, None, 1)); - }); - let create = server.mock(|when, then| { - when.method("POST") - .path("/api/v1/runs") - .json_body_includes(format!(r#"{{"parent_id":"{parent_id}"}}"#)); - then.status(201) - .header("Content-Type", "application/json") - .json_body(run_summary_json(child_id, Some(parent_id), 0)); - }); - let retrieve = server.mock(|when, then| { - when.method("GET").path(format!("/api/v1/runs/{child_id}")); - then.status(200) - .header("Content-Type", "application/json") - .json_body(run_summary_json(child_id, Some(parent_id), 0)); - }); - let client = - Arc::new(Client::new_no_proxy(&server.base_url()).expect("client should build")); - let params = ValidatedCreateRuns::try_from(FabroRunCreateParams { - runs: vec![CreateRunSpec { - workflow: workflow.display().to_string(), - cwd: None, - run_id: None, - parent_id: Some("nightly-parent".to_string()), - goal: None, - inputs: HashMap::new(), - labels: HashMap::new(), - dry_run: Some(true), - auto_approve: Some(true), - model: None, - provider: None, - sandbox: None, - preserve_sandbox: None, - start: Some(false), - }], - }) - .expect("create params should validate"); - - let result = create_runs(client, temp.path(), &settings, params) - .await - .expect("run should be created"); - - assert_eq!(result.runs[0].parent_id, Some(parent_id.to_string())); - assert_eq!(result.runs[0].children_count, 0); - resolve_parent.assert(); - create.assert(); - retrieve.assert(); - } - - fn run_id(raw: &str) -> RunId { - raw.parse().expect("test run id should parse") - } - - fn run_summary_json( - run_id: RunId, - parent_id: Option, - children_count: u64, - ) -> serde_json::Value { - json!({ - "id": run_id, - "parent_id": parent_id, - "children_count": children_count, - "title": "Test run", - "goal": "Test run", - "workflow": { - "slug": "simple", - "name": "Simple" - }, - "repository": null, - "origin": { - "kind": "api" - }, - "labels": {}, - "lifecycle": { - "status": { "kind": "submitted" }, - "pending_control": null, - "queue_position": null, - "error": null, - "archived": false, - "archived_at": null - }, - "models": [], - "source_directory": "/srv/repo", - "timestamps": { - "created_at": "2026-04-05T12:00:00Z", - "started_at": null, - "last_event_at": null, - "completed_at": null, - "duration_ms": null, - "elapsed_secs": null - }, - "billing": null, - "diff": null, - "pull_request": null, - "current_question": null, - "superseded_by": null, - "links": { - "web": null - } - }) - } -} diff --git a/lib/crates/fabro-mcp-server/src/run_tools/manifest.rs b/lib/crates/fabro-mcp-server/src/run_tools/manifest.rs deleted file mode 100644 index 576320694..000000000 --- a/lib/crates/fabro-mcp-server/src/run_tools/manifest.rs +++ /dev/null @@ -1,188 +0,0 @@ -use std::path::{Path, PathBuf}; - -use fabro_api::types; -use fabro_config::{CliLayer, RunLayer, load_llm_catalog_settings}; -use fabro_manifest::{self, ManifestBuildInput, RunOverrideInput}; -use fabro_model::Catalog; -use fabro_server::manifest_validation; -use serde_json::Value; - -use super::common::{ToolError, ToolResult}; -use super::create::ValidatedCreateRunSpec; - -pub(super) fn build_mcp_run_manifest( - spec: &ValidatedCreateRunSpec, - cwd: &Path, - user_settings_path: &Path, -) -> ToolResult { - let built = fabro_manifest::build_run_manifest(ManifestBuildInput { - workflow: PathBuf::from(&spec.workflow), - cwd: cwd.to_path_buf(), - run_overrides: mcp_run_overrides(spec), - cli_overrides: Some(CliLayer::default()), - input_overrides: spec.inputs.clone(), - args: mcp_manifest_args(spec), - run_id: spec.run_id, - user_settings_path: Some(user_settings_path.to_path_buf()), - }) - .map_err(|err| ToolError::from_anyhow(&err))?; - let llm_catalog_settings = load_llm_catalog_settings(Some(user_settings_path)) - .map_err(|err| ToolError::message(err.to_string()))?; - let catalog = std::sync::Arc::new( - Catalog::from_builtin_with_overrides(&llm_catalog_settings) - .map_err(|err| ToolError::message(err.to_string()))?, - ); - let mut validation = - manifest_validation::validate_manifest(&RunLayer::default(), &built.manifest, catalog) - .map_err(|err| ToolError::from_anyhow(&err))?; - manifest_validation::promote_template_undefined_variables_to_errors(&mut validation); - if !validation.ok { - return Err(ToolError::message("workflow manifest validation failed")); - } - Ok(built.manifest) -} - -pub(super) fn json_to_toml_value(key: &str, value: &Value) -> ToolResult { - match value { - Value::Null => Err(ToolError::message(format!( - "input `{key}` cannot be null; use a string, boolean, or number" - ))), - Value::Bool(value) => Ok(toml::Value::Boolean(*value)), - Value::Number(value) => { - if let Some(integer) = value.as_i64() { - Ok(toml::Value::Integer(integer)) - } else if let Some(float) = value.as_f64() { - Ok(toml::Value::Float(float)) - } else { - Err(ToolError::message(format!( - "input `{key}` contains a number outside TOML's supported range" - ))) - } - } - Value::String(value) => Ok(toml::Value::String(value.clone())), - Value::Array(_) => Err(ToolError::message(format!( - "input `{key}` does not support array values; use a string, boolean, or number", - ))), - Value::Object(_) => Err(ToolError::message(format!( - "input `{key}` does not support object values; use a string, boolean, or number", - ))), - } -} - -fn mcp_manifest_args(spec: &ValidatedCreateRunSpec) -> Option { - let mut input = spec - .inputs - .iter() - .map(|(key, value)| format!("{key}={value}")) - .collect::>(); - input.sort(); - let mut label = spec - .labels - .iter() - .map(|(key, value)| format!("{key}={value}")) - .collect::>(); - label.sort(); - let payload = types::ManifestArgs { - auto_approve: spec.auto_approve.filter(|value| *value), - docker_image: None, - dry_run: spec.dry_run.filter(|value| *value), - input, - label, - model: spec.model.clone(), - preserve_sandbox: spec.preserve_sandbox.filter(|value| *value), - provider: spec.provider.clone(), - sandbox: spec.sandbox.clone(), - verbose: None, - }; - (!fabro_manifest::manifest_args_is_empty(&payload)).then_some(payload) -} - -fn mcp_run_overrides(spec: &ValidatedCreateRunSpec) -> Option { - fabro_manifest::build_sparse_run_overrides(RunOverrideInput { - goal: spec.goal.as_deref(), - model: spec.model.as_deref(), - provider: spec.provider.as_deref(), - sandbox: spec.sandbox.as_deref(), - docker_image: None, - preserve_sandbox: spec.preserve_sandbox, - dry_run: spec.dry_run, - auto_approve: spec.auto_approve, - labels: spec.labels.clone(), - }) -} - -#[cfg(test)] -mod tests { - use std::collections::HashMap; - - use serde_json::{Value, json}; - - use super::super::create::CreateRunSpec; - use super::*; - - #[test] - fn json_inputs_convert_scalar_values_to_toml_values() { - let cases = [ - (json!("hello"), toml::Value::String("hello".to_string())), - (json!(true), toml::Value::Boolean(true)), - (json!(42), toml::Value::Integer(42)), - (json!(0.5), toml::Value::Float(0.5)), - ]; - - for (json, expected) in cases { - assert_eq!(json_to_toml_value("input", &json).unwrap(), expected); - } - } - - #[test] - fn json_input_arrays_and_objects_are_rejected() { - let array_err = json_to_toml_value("matrix", &json!(["a", 1])).unwrap_err(); - assert_eq!( - array_err.as_str(), - "input `matrix` does not support array values; use a string, boolean, or number", - ); - - let object_err = json_to_toml_value("settings", &json!({ "enabled": true })).unwrap_err(); - assert_eq!( - object_err.as_str(), - "input `settings` does not support object values; use a string, boolean, or number", - ); - } - - #[test] - fn json_input_null_is_rejected_with_key_name() { - let err = json_to_toml_value("goal", &Value::Null).unwrap_err(); - - assert_eq!( - err.as_str(), - "input `goal` cannot be null; use a string, boolean, or number", - ); - } - - #[test] - fn mcp_manifest_args_preserve_input_provenance() { - let spec = ValidatedCreateRunSpec::try_from(CreateRunSpec { - workflow: "simple".to_string(), - run_id: None, - parent_id: None, - cwd: None, - goal: None, - inputs: HashMap::from([ - ("count".to_string(), json!(3).into()), - ("decision".to_string(), json!("approve").into()), - ]), - labels: HashMap::new(), - model: None, - provider: None, - sandbox: None, - dry_run: None, - auto_approve: None, - preserve_sandbox: None, - start: None, - }) - .expect("create spec should validate"); - let args = mcp_manifest_args(&spec).expect("input args should be present"); - - assert_eq!(args.input, vec![r"count=3", r#"decision="approve""#]); - } -} diff --git a/lib/crates/fabro-mcp-server/src/server.rs b/lib/crates/fabro-mcp-server/src/server.rs index d2315f0d7..5a02c9a17 100644 --- a/lib/crates/fabro-mcp-server/src/server.rs +++ b/lib/crates/fabro-mcp-server/src/server.rs @@ -2,20 +2,23 @@ use std::path::PathBuf; use std::sync::Arc; use anyhow::Result; -use fabro_client::Client; +use fabro_tool::fabro_client::ClientBackend; +use fabro_tool::{self as run_tools, FabroToolBackend}; use rmcp::handler::server::router::tool::ToolRouter; use rmcp::handler::server::wrapper::Parameters; -use rmcp::model::{CallToolResult, ServerCapabilities, ServerInfo}; +use rmcp::model::{CallToolResult, Content, ServerCapabilities, ServerInfo}; use rmcp::transport::stdio; use rmcp::{ErrorData, ServerHandler, serve_server, tool, tool_handler, tool_router}; +use serde::Serialize; use tokio::sync::OnceCell; -use crate::{FabroMcpServerSettings, run_tools}; +use crate::FabroMcpServerSettings; +use crate::manifest_builder::McpRunManifestBuilder; #[derive(Clone)] pub(crate) struct FabroMcpServer { settings: Arc, - client: Arc>>, + backend: Arc>>, cwd: PathBuf, tool_router: ToolRouter, } @@ -41,7 +44,7 @@ impl FabroMcpServer { let cwd = settings.cwd.clone(); Self { settings, - client: Arc::new(OnceCell::new()), + backend: Arc::new(OnceCell::new()), cwd, tool_router: Self::tool_router(), } @@ -57,15 +60,15 @@ impl FabroMcpServer { ) -> Result { let params = match run_tools::ValidatedCreateRuns::try_from(params.0) { Ok(params) => params, - Err(err) => return Ok(run_tools::error_result(err)), + Err(err) => return Ok(error_result(&err)), }; - let client = match self.client().await { - Ok(client) => client, - Err(err) => return Ok(run_tools::error_result(err)), + let backend = match self.backend().await { + Ok(backend) => backend, + Err(err) => return Ok(error_result(&err)), }; - match run_tools::create_runs(client, &self.cwd, &self.settings.config_path, params).await { - Ok(result) => run_tools::success_result(&result, run_tools::create_runs_text(&result)), - Err(err) => Ok(run_tools::error_result(err)), + match run_tools::create_runs(backend, &self.cwd, &self.settings.config_path, params).await { + Ok(result) => success_result(&result, run_tools::create_runs_text(&result)), + Err(err) => Ok(error_result(&err)), } } @@ -79,15 +82,15 @@ impl FabroMcpServer { ) -> Result { let params = match run_tools::ValidatedSearchRuns::try_from(params.0) { Ok(params) => params, - Err(err) => return Ok(run_tools::error_result(err)), + Err(err) => return Ok(error_result(&err)), }; - let client = match self.client().await { - Ok(client) => client, - Err(err) => return Ok(run_tools::error_result(err)), + let backend = match self.backend().await { + Ok(backend) => backend, + Err(err) => return Ok(error_result(&err)), }; - match run_tools::search_runs(client, params).await { - Ok(result) => run_tools::success_result(&result, run_tools::search_runs_text(&result)), - Err(err) => Ok(run_tools::error_result(err)), + match run_tools::search_runs(backend, params).await { + Ok(result) => success_result(&result, run_tools::search_runs_text(&result)), + Err(err) => Ok(error_result(&err)), } } @@ -101,15 +104,15 @@ impl FabroMcpServer { ) -> Result { let params = match run_tools::ValidatedInteractRun::try_from(params.0) { Ok(params) => params, - Err(err) => return Ok(run_tools::error_result(err)), + Err(err) => return Ok(error_result(&err)), }; - let client = match self.client().await { - Ok(client) => client, - Err(err) => return Ok(run_tools::error_result(err)), + let backend = match self.backend().await { + Ok(backend) => backend, + Err(err) => return Ok(error_result(&err)), }; - match run_tools::interact_run(client, params).await { - Ok(result) => run_tools::success_result(&result, run_tools::interact_run_text(&result)), - Err(err) => Ok(run_tools::error_result(err)), + match run_tools::interact_run(backend, params).await { + Ok(result) => success_result(&result, run_tools::interact_run_text(&result)), + Err(err) => Ok(error_result(&err)), } } @@ -123,15 +126,15 @@ impl FabroMcpServer { ) -> Result { let params = match run_tools::ValidatedGatherRuns::try_from(params.0) { Ok(params) => params, - Err(err) => return Ok(run_tools::error_result(err)), + Err(err) => return Ok(error_result(&err)), }; - let client = match self.client().await { - Ok(client) => client, - Err(err) => return Ok(run_tools::error_result(err)), + let backend = match self.backend().await { + Ok(backend) => backend, + Err(err) => return Ok(error_result(&err)), }; - match run_tools::gather_runs(client, params).await { - Ok(result) => run_tools::success_result(&result, run_tools::gather_runs_text(&result)), - Err(err) => Ok(run_tools::error_result(err)), + match run_tools::gather_runs(backend, params).await { + Ok(result) => success_result(&result, run_tools::gather_runs_text(&result)), + Err(err) => Ok(error_result(&err)), } } @@ -145,27 +148,51 @@ impl FabroMcpServer { ) -> Result { let params = match run_tools::ValidatedRunEvents::try_from(params.0) { Ok(params) => params, - Err(err) => return Ok(run_tools::error_result(err)), + Err(err) => return Ok(error_result(&err)), }; - let client = match self.client().await { - Ok(client) => client, - Err(err) => return Ok(run_tools::error_result(err)), + let backend = match self.backend().await { + Ok(backend) => backend, + Err(err) => return Ok(error_result(&err)), }; - match run_tools::run_events(client, params).await { - Ok(result) => run_tools::success_result(&result, run_tools::run_events_text(&result)), - Err(err) => Ok(run_tools::error_result(err)), + match run_tools::run_events(backend, params).await { + Ok(result) => success_result(&result, run_tools::run_events_text(&result)), + Err(err) => Ok(error_result(&err)), } } - async fn client(&self) -> Result, run_tools::ToolError> { - self.client + async fn backend(&self) -> Result, run_tools::ToolError> { + self.backend .get_or_try_init(|| async { (self.settings.client_factory)() .await - .map(Arc::new) + .map(|client| { + Arc::new( + ClientBackend::new(Arc::new(client)) + .with_manifest_builder(Arc::new(McpRunManifestBuilder)), + ) as Arc + }) .map_err(|err| run_tools::ToolError::from_anyhow(&err)) }) .await .map(Arc::clone) } } + +fn success_result( + value: &T, + text: impl Into, +) -> Result { + let structured_content = serde_json::to_value(value).map_err(|err| { + rmcp::ErrorData::internal_error( + format!("failed to serialize Fabro MCP tool result: {err}"), + None, + ) + })?; + let mut result = CallToolResult::structured(structured_content); + result.content = vec![Content::text(text.into())]; + Ok(result) +} + +fn error_result(err: &run_tools::ToolError) -> CallToolResult { + CallToolResult::error(vec![Content::text(err.to_string())]) +} diff --git a/lib/crates/fabro-server/Cargo.toml b/lib/crates/fabro-server/Cargo.toml index 1ade0e182..ffc0a9af9 100644 --- a/lib/crates/fabro-server/Cargo.toml +++ b/lib/crates/fabro-server/Cargo.toml @@ -38,6 +38,7 @@ fabro-llm = { path = "../fabro-llm" } fabro-manifest = { path = "../fabro-manifest" } fabro-model = { path = "../fabro-model" } fabro-proc = { path = "../fabro-proc" } +fabro-tool = { path = "../fabro-tool" } fabro-types = { path = "../fabro-types" } fabro-util = { path = "../fabro-util" } fabro-api = { path = "../fabro-api" } diff --git a/lib/crates/fabro-server/src/lib.rs b/lib/crates/fabro-server/src/lib.rs index 917ae7a23..873482712 100644 --- a/lib/crates/fabro-server/src/lib.rs +++ b/lib/crates/fabro-server/src/lib.rs @@ -31,6 +31,7 @@ mod run_files; mod run_files_security; mod run_manifest; mod run_selector; +pub mod run_tool_manifest; pub mod security_headers; pub mod serve; pub mod server; diff --git a/lib/crates/fabro-server/src/principal_middleware.rs b/lib/crates/fabro-server/src/principal_middleware.rs index cbd9b58de..8200dc81b 100644 --- a/lib/crates/fabro-server/src/principal_middleware.rs +++ b/lib/crates/fabro-server/src/principal_middleware.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::convert::Infallible; use std::sync::{Arc, Mutex}; @@ -14,7 +15,7 @@ use crate::auth::{AuthErrorCode, JwtError, REFRESH_TOKEN_PREFIX}; use crate::error::ApiError; use crate::jwt_auth::{self, AuthMode, ConfiguredAuth}; use crate::server::{AppState, parse_blob_id_path, parse_run_id_path, parse_stage_id_path}; -use crate::worker_token::{self, WORKER_TOKEN_KID}; +use crate::worker_token::{self, WORKER_TOKEN_KID, WorkerScopeSet}; #[derive(Clone, Debug)] pub(crate) struct RequestAuthContext { @@ -22,6 +23,7 @@ pub(crate) struct RequestAuthContext { pub auth_status: AuthStatus, pub auth_error_code: Option, pub user_profile: Option, + pub worker_scopes: WorkerScopeSet, } #[derive(Clone, Debug)] @@ -54,7 +56,9 @@ pub(crate) struct AuthContextSlot(pub(crate) Arc>); pub(crate) struct RequestAuth(pub(crate) AuthContextSlot); pub(crate) struct RequiredUser(pub(crate) UserPrincipal); +pub(crate) struct RequiredRunToolActor(pub(crate) Principal); pub(crate) struct RequireRunScoped(pub(crate) RunId); +pub(crate) struct RequireRunScopedOrRunTools(pub(crate) RunId, pub(crate) Principal); pub(crate) struct RequireRunBlob(pub(crate) RunId, pub(crate) RunBlobId); pub(crate) struct RequireRunStageScoped(pub(crate) RunId, pub(crate) String); pub(crate) struct RequireStageArtifact(pub(crate) RunId, pub(crate) StageId); @@ -74,6 +78,7 @@ impl RequestAuthContext { auth_status: AuthStatus::Missing, auth_error_code: None, user_profile: None, + worker_scopes: WorkerScopeSet::default(), } } @@ -84,6 +89,18 @@ impl RequestAuthContext { auth_status: AuthStatus::Authenticated, auth_error_code: None, user_profile, + worker_scopes: WorkerScopeSet::default(), + } + } + + #[must_use] + pub(crate) fn authenticated_worker(run_id: RunId, scopes: WorkerScopeSet) -> Self { + Self { + principal: Principal::Worker { run_id }, + auth_status: AuthStatus::Authenticated, + auth_error_code: None, + user_profile: None, + worker_scopes: scopes, } } @@ -110,6 +127,7 @@ impl RequestAuthContext { auth_status: status, auth_error_code: code, user_profile: None, + worker_scopes: WorkerScopeSet::default(), } } @@ -197,6 +215,19 @@ impl FromRequestParts for RequiredUser { } } +impl FromRequestParts for RequiredRunToolActor { + type Rejection = ApiError; + + async fn from_request_parts(parts: &mut Parts, _: &S) -> Result { + let slot = parts + .extensions + .get::() + .cloned() + .unwrap_or_else(AuthContextSlot::initial); + require_run_tool_actor(&slot).map(Self) + } +} + impl FromRequestParts> for RequireRunScoped { type Rejection = Response; @@ -214,6 +245,30 @@ impl FromRequestParts> for RequireRunScoped { } } +impl FromRequestParts> for RequireRunScopedOrRunTools { + type Rejection = Response; + + async fn from_request_parts( + parts: &mut Parts, + state: &Arc, + ) -> Result { + let Path(params): Path> = Path::from_request_parts(parts, state) + .await + .map_err(IntoResponse::into_response)?; + let Some(id) = params.get("id") else { + return Err( + ApiError::new(StatusCode::BAD_REQUEST, "Run ID path parameter missing.") + .into_response(), + ); + }; + let run_id = parse_run_id_path(id)?; + let actor = + require_worker_or_user_for_run_or_run_tools(&auth_slot_from_parts(parts), &run_id) + .map_err(IntoResponse::into_response)?; + Ok(Self(run_id, actor)) + } +} + impl FromRequestParts> for RequireRunBlob { type Rejection = Response; @@ -339,6 +394,18 @@ pub(crate) fn require_authenticated_user( } } +pub(crate) fn require_run_tool_actor(slot: &AuthContextSlot) -> Result { + let context = slot.0.lock().expect("auth context lock poisoned"); + match &context.principal { + Principal::User(user) => Ok(Principal::User(user.clone())), + Principal::Worker { run_id } if context.worker_scopes.has_agent_run_tools() => { + Ok(Principal::Worker { run_id: *run_id }) + } + Principal::Worker { .. } => Err(ApiError::forbidden()), + _ => Err(auth_rejection(context.auth_status, context.auth_error_code)), + } +} + fn require_worker_or_user_for_run( slot: &AuthContextSlot, route_run_id: &RunId, @@ -352,6 +419,23 @@ fn require_worker_or_user_for_run( } } +fn require_worker_or_user_for_run_or_run_tools( + slot: &AuthContextSlot, + route_run_id: &RunId, +) -> Result { + let context = slot.0.lock().expect("auth context lock poisoned"); + match &context.principal { + Principal::User(user) => Ok(Principal::User(user.clone())), + Principal::Worker { run_id } + if run_id == route_run_id || context.worker_scopes.has_agent_run_tools() => + { + Ok(Principal::Worker { run_id: *run_id }) + } + Principal::Worker { .. } => Err(ApiError::forbidden()), + _ => Err(auth_rejection(context.auth_status, context.auth_error_code)), + } +} + fn classify_request(req: &Request, state: &AppState) -> RequestAuthContext { let AuthMode::Enabled(config) = req .extensions() @@ -391,7 +475,7 @@ fn classify_request(req: &Request, state: &AppState) -> RequestAuthContext { if header.kid.as_deref() == Some(WORKER_TOKEN_KID) { return match worker_token::decode_worker_token(token, state.worker_token_keys()) { - Ok(run_id) => RequestAuthContext::authenticated(Principal::Worker { run_id }, None), + Ok(decoded) => RequestAuthContext::authenticated_worker(decoded.run_id, decoded.scopes), Err(JwtError::AccessTokenExpired) => RequestAuthContext::rejected( AuthStatus::Expired, Some(AuthErrorCode::AccessTokenExpired), @@ -460,8 +544,8 @@ mod tests { use super::*; use crate::auth::{self, AuthErrorCode}; use crate::worker_token::{ - WORKER_TOKEN_ISSUER, WORKER_TOKEN_SCOPE, WorkerTokenClaims, issue_worker_token, - worker_token_header, + WORKER_RUN_TOOLS_SCOPE, WORKER_TOKEN_ISSUER, WORKER_TOKEN_SCOPE, WorkerScopeSet, + WorkerTokenClaims, issue_worker_token, worker_token_header, }; const TEST_JWT_ISSUER: &str = "https://fabro.example"; @@ -604,6 +688,26 @@ mod tests { assert_eq!(context.auth_status, AuthStatus::Authenticated); assert_eq!(context.principal, Principal::Worker { run_id }); + assert!(!context.worker_scopes.has_agent_run_tools()); + } + + #[test] + fn classifies_run_tools_worker_scope() { + let state = crate::test_support::test_app_state(); + let run_id = RunId::new(); + let token = issue_worker_claims( + state.as_ref(), + run_id, + u64::MAX / 2, + &format!("{WORKER_TOKEN_SCOPE} {WORKER_RUN_TOOLS_SCOPE}"), + ); + let request = request_with_bearer(Some(&token), auth_mode_for_state(state.as_ref())); + + let context = classify_request(&request, state.as_ref()); + + assert_eq!(context.auth_status, AuthStatus::Authenticated); + assert_eq!(context.principal, Principal::Worker { run_id }); + assert!(context.worker_scopes.has_agent_run_tools()); } #[test] @@ -713,4 +817,50 @@ mod tests { assert_eq!(err.status(), StatusCode::UNAUTHORIZED); assert_eq!(err.code(), Some("access_token_invalid")); } + + #[test] + fn run_tool_actor_rejects_base_worker_scope() { + let run_id = RunId::new(); + let slot = AuthContextSlot::initial(); + slot.replace(RequestAuthContext::authenticated( + Principal::Worker { run_id }, + None, + )); + + let err = require_run_tool_actor(&slot).unwrap_err(); + + assert_eq!(err.status(), StatusCode::FORBIDDEN); + } + + #[test] + fn run_tool_actor_accepts_worker_with_run_tools_scope() { + let run_id = RunId::new(); + let slot = AuthContextSlot::initial(); + slot.replace(RequestAuthContext::authenticated_worker( + run_id, + WorkerScopeSet::run_worker_with_agent_run_tools(), + )); + + assert_eq!(require_run_tool_actor(&slot).unwrap(), Principal::Worker { + run_id + },); + } + + #[test] + fn run_scoped_or_run_tools_accepts_cross_run_with_run_tools_scope() { + let token_run_id = RunId::new(); + let route_run_id = RunId::new(); + let slot = AuthContextSlot::initial(); + slot.replace(RequestAuthContext::authenticated_worker( + token_run_id, + WorkerScopeSet::run_worker_with_agent_run_tools(), + )); + + assert_eq!( + require_worker_or_user_for_run_or_run_tools(&slot, &route_run_id).unwrap(), + Principal::Worker { + run_id: token_run_id, + }, + ); + } } diff --git a/lib/crates/fabro-server/src/run_tool_manifest.rs b/lib/crates/fabro-server/src/run_tool_manifest.rs new file mode 100644 index 000000000..c1239c3bf --- /dev/null +++ b/lib/crates/fabro-server/src/run_tool_manifest.rs @@ -0,0 +1,119 @@ +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +use fabro_api::types; +use fabro_config::{CliLayer, RunLayer}; +use fabro_manifest::{ManifestBuildInput, RunOverrideInput}; +use fabro_model::Catalog; +use fabro_tool::{ToolError, ToolResult, ValidatedCreateRunSpec}; + +use crate::manifest_validation; + +pub fn build_run_tool_manifest( + spec: &ValidatedCreateRunSpec, + cwd: &Path, + user_settings_path: &Path, + catalog: Arc, +) -> ToolResult { + let built = fabro_manifest::build_run_manifest(ManifestBuildInput { + workflow: PathBuf::from(&spec.workflow), + cwd: cwd.to_path_buf(), + run_overrides: run_tool_run_overrides(spec), + cli_overrides: Some(CliLayer::default()), + input_overrides: spec.inputs.clone(), + args: run_tool_manifest_args(spec), + run_id: spec.run_id, + user_settings_path: Some(user_settings_path.to_path_buf()), + }) + .map_err(|err| ToolError::from_anyhow(&err))?; + + let mut validation = + manifest_validation::validate_manifest(&RunLayer::default(), &built.manifest, catalog) + .map_err(|err| ToolError::from_anyhow(&err))?; + manifest_validation::promote_template_undefined_variables_to_errors(&mut validation); + if !validation.ok { + return Err(ToolError::message("workflow manifest validation failed")); + } + + Ok(built.manifest) +} + +pub fn run_tool_manifest_args(spec: &ValidatedCreateRunSpec) -> Option { + let mut input = spec + .inputs + .iter() + .map(|(key, value)| format!("{key}={value}")) + .collect::>(); + input.sort(); + let mut label = spec + .labels + .iter() + .map(|(key, value)| format!("{key}={value}")) + .collect::>(); + label.sort(); + + let payload = types::ManifestArgs { + auto_approve: spec.auto_approve.filter(|value| *value), + docker_image: None, + dry_run: spec.dry_run.filter(|value| *value), + input, + label, + model: spec.model.clone(), + preserve_sandbox: spec.preserve_sandbox.filter(|value| *value), + provider: spec.provider.clone(), + sandbox: spec.sandbox.clone(), + verbose: None, + }; + (!fabro_manifest::manifest_args_is_empty(&payload)).then_some(payload) +} + +pub fn run_tool_run_overrides(spec: &ValidatedCreateRunSpec) -> Option { + fabro_manifest::build_sparse_run_overrides(RunOverrideInput { + goal: spec.goal.as_deref(), + model: spec.model.as_deref(), + provider: spec.provider.as_deref(), + sandbox: spec.sandbox.as_deref(), + docker_image: None, + preserve_sandbox: spec.preserve_sandbox, + dry_run: spec.dry_run, + auto_approve: spec.auto_approve, + labels: spec.labels.clone(), + }) +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use fabro_tool::CreateRunSpec; + use serde_json::json; + + use super::*; + + #[test] + fn manifest_args_preserve_input_provenance() { + let spec = ValidatedCreateRunSpec::try_from(CreateRunSpec { + workflow: "simple".to_string(), + run_id: None, + parent_id: None, + cwd: None, + goal: None, + inputs: HashMap::from([ + ("count".to_string(), json!(3).into()), + ("decision".to_string(), json!("approve").into()), + ]), + labels: HashMap::new(), + model: None, + provider: None, + sandbox: None, + dry_run: None, + auto_approve: None, + preserve_sandbox: None, + start: None, + }) + .expect("create spec should validate"); + let args = run_tool_manifest_args(&spec).expect("input args should be present"); + + assert_eq!(args.input, vec![r"count=3", r#"decision="approve""#]); + } +} diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 731c3f926..7f2fb6a3a 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -132,13 +132,14 @@ use crate::ip_allowlist::{IpAllowlistConfig, ip_allowlist_middleware}; use crate::jwt_auth::{self, AuthMode}; use crate::principal_middleware::{ AuthContextSlot, RequestAuth, RequestAuthContext, RequireRunBlob, RequireRunScoped, - RequireRunStageScoped, RequireStageArtifact, RequiredUser, principal_middleware, + RequireRunScopedOrRunTools, RequireRunStageScoped, RequireStageArtifact, RequiredUser, + principal_middleware, }; use crate::request_id::{self, RequestId}; use crate::run_files::{FilesInFlight, new_files_in_flight}; use crate::server_secrets::{LlmClientResult, ServerSecrets}; use crate::spawn_env::{apply_render_graph_env, apply_worker_env}; -use crate::worker_token::{WorkerTokenKeys, issue_worker_token}; +use crate::worker_token::{WorkerScopeSet, WorkerTokenKeys, issue_worker_token_with_scopes}; use crate::{ canonical_host, demo, diagnostics, run_manifest, security_headers, static_files, vault_legacy_migration, web_auth, @@ -2732,8 +2733,12 @@ fn worker_command( ) })?; let server_target = daemon.bind.to_target(); - let worker_token = issue_worker_token(state.worker_token_keys(), &run_id) - .map_err(|_| anyhow::anyhow!("failed to sign worker token"))?; + let worker_token = issue_worker_token_with_scopes( + state.worker_token_keys(), + &run_id, + WorkerScopeSet::run_worker_with_agent_run_tools(), + ) + .map_err(|_| anyhow::anyhow!("failed to sign worker token"))?; let server_destination = resolved_log_destination(state)?; let worker_stdout = match server_destination { LogDestination::Stdout => Stdio::inherit(), @@ -3197,6 +3202,7 @@ async fn execute_run_in_process(state: Arc, run_id: RunId) { catalog: state.catalog(), on_node: None, registry_override, + fabro_run_tools: None, }; let execution = async { diff --git a/lib/crates/fabro-server/src/server/handler/events.rs b/lib/crates/fabro-server/src/server/handler/events.rs index d5b4d7109..c34d07a25 100644 --- a/lib/crates/fabro-server/src/server/handler/events.rs +++ b/lib/crates/fabro-server/src/server/handler/events.rs @@ -9,10 +9,10 @@ use fabro_workflow::event::build_redacted_event_payload; use super::super::{ ApiError, AppState, AppendEventResponse, BroadcastStream, Event, EventBody, EventEnvelope, EventPayload, HashSet, IntoResponse, Json, KeepAlive, PaginatedEventList, PaginationMeta, Path, - Query, RequireRunScoped, RequireRunStageScoped, RequiredUser, Response, Router, RunEvent, - RunId, Sse, State, StatusCode, StreamExt, UnboundedReceiverStream, broadcast, get, mpsc, - parse_run_id_path, parse_stage_id_path, redact_jsonl_line, reject_if_archived, - update_live_run_from_event, + Query, RequireRunScoped, RequireRunScopedOrRunTools, RequireRunStageScoped, RequiredUser, + Response, Router, RunEvent, RunId, Sse, State, StatusCode, StreamExt, UnboundedReceiverStream, + broadcast, get, mpsc, parse_run_id_path, parse_stage_id_path, redact_jsonl_line, + reject_if_archived, update_live_run_from_event, }; pub(super) fn routes() -> Router> { @@ -198,7 +198,7 @@ async fn append_run_event( } async fn list_run_events( - RequireRunScoped(id): RequireRunScoped, + RequireRunScopedOrRunTools(id, _actor): RequireRunScopedOrRunTools, State(state): State>, Query(params): Query, ) -> Response { diff --git a/lib/crates/fabro-server/src/server/handler/lifecycle.rs b/lib/crates/fabro-server/src/server/handler/lifecycle.rs index 80e5d2918..4bb1b4bb9 100644 --- a/lib/crates/fabro-server/src/server/handler/lifecycle.rs +++ b/lib/crates/fabro-server/src/server/handler/lifecycle.rs @@ -2,12 +2,12 @@ use std::sync::Arc; use super::super::{ ApiError, AppState, FailureReason, ForkRequest, ForkResponse, IntoResponse, Json, Path, - Principal, RequiredUser, Response, RewindRequest, RewindResponse, Router, RunAnswerTransport, - RunControlAction, RunExecutionMode, RunId, RunStatus, StartRunRequest, State, StatusCode, - Storage, TimelineEntryResponse, WORKER_CANCEL_GRACE, WorkflowError, append_control_request, - durable_run_status, get, load_pending_control, managed_run, operations, parse_run_id_path, - persist_cancelled_run_status, post, reject_if_archived, sleep, update_live_run_from_event, - workflow_event, + Principal, RequireRunScopedOrRunTools, RequiredUser, Response, RewindRequest, RewindResponse, + Router, RunAnswerTransport, RunControlAction, RunExecutionMode, RunId, RunStatus, + StartRunRequest, State, StatusCode, Storage, TimelineEntryResponse, WORKER_CANCEL_GRACE, + WorkflowError, append_control_request, durable_run_status, get, load_pending_control, + managed_run, operations, parse_run_id_path, persist_cancelled_run_status, post, + reject_if_archived, sleep, update_live_run_from_event, workflow_event, }; pub(super) fn routes() -> Router> { @@ -34,15 +34,10 @@ async fn run_response(state: &AppState, id: RunId, status: StatusCode) -> Respon } async fn start_run( - _auth: RequiredUser, + RequireRunScopedOrRunTools(id, _actor): RequireRunScopedOrRunTools, State(state): State>, - Path(id): Path, body: Option>, ) -> Response { - let id = match parse_run_id_path(&id) { - Ok(id) => id, - Err(response) => return response, - }; if let Some(response) = reject_if_archived(state.as_ref(), &id).await { return response; } @@ -153,14 +148,9 @@ fn schedule_worker_kill(state: Arc, run_id: RunId, worker_pid: u32) { } async fn cancel_run( - subject: RequiredUser, + RequireRunScopedOrRunTools(id, actor): RequireRunScopedOrRunTools, State(state): State>, - Path(id): Path, ) -> Response { - let id = match parse_run_id_path(&id) { - Ok(id) => id, - Err(response) => return response, - }; if let Some(response) = reject_if_archived(state.as_ref(), &id).await { return response; } @@ -217,13 +207,8 @@ async fn cancel_run( }; if pending_control != Some(RunControlAction::Cancel) { - if let Err(err) = append_control_request( - state.as_ref(), - id, - RunControlAction::Cancel, - Some(Principal::User(subject.0.clone())), - ) - .await + if let Err(err) = + append_control_request(state.as_ref(), id, RunControlAction::Cancel, Some(actor)).await { return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()) .into_response(); @@ -456,19 +441,17 @@ async fn unpause_run( } async fn archive_run( - subject: RequiredUser, + RequireRunScopedOrRunTools(id, actor): RequireRunScopedOrRunTools, State(state): State>, - Path(id): Path, ) -> Response { - run_archive_action(state, subject, id, ArchiveAction::Archive).await + run_archive_action(state, actor, id, ArchiveAction::Archive).await } async fn unarchive_run( - subject: RequiredUser, + RequireRunScopedOrRunTools(id, actor): RequireRunScopedOrRunTools, State(state): State>, - Path(id): Path, ) -> Response { - run_archive_action(state, subject, id, ArchiveAction::Unarchive).await + run_archive_action(state, actor, id, ArchiveAction::Unarchive).await } async fn rewind_run( @@ -635,15 +618,11 @@ enum ArchiveAction { async fn run_archive_action( state: Arc, - subject: RequiredUser, - id: String, + actor: Principal, + id: RunId, action: ArchiveAction, ) -> Response { - let id = match parse_run_id_path(&id) { - Ok(id) => id, - Err(response) => return response, - }; - let actor = Some(Principal::User(subject.0.clone())); + let actor = Some(actor); let result = match action { ArchiveAction::Archive => operations::archive(&state.store, &id, actor) .await diff --git a/lib/crates/fabro-server/src/server/handler/runs.rs b/lib/crates/fabro-server/src/server/handler/runs.rs index 7fc1a3fd9..8fc6b15ec 100644 --- a/lib/crates/fabro-server/src/server/handler/runs.rs +++ b/lib/crates/fabro-server/src/server/handler/runs.rs @@ -18,8 +18,7 @@ use fabro_api::types::{ use fabro_config::Storage; use fabro_interview::AnswerSubmission; use fabro_types::{ - Principal, RunClientProvenance, RunId, RunProvenance, RunServerProvenance, UserPrincipal, - parse_blob_ref, + Principal, RunClientProvenance, RunId, RunProvenance, RunServerProvenance, parse_blob_ref, }; use fabro_util::version::FABRO_VERSION; use fabro_workflow::command_log::{command_log_path, read_json_string_blob, read_log_slice}; @@ -36,7 +35,8 @@ use super::super::{ }; use crate::error::ApiError; use crate::principal_middleware::{ - RequestAuth, RequireCommandLog, RequireRunScoped, RequiredUser, require_user, + RequireCommandLog, RequireRunScoped, RequireRunScopedOrRunTools, RequiredRunToolActor, + RequiredUser, }; use crate::run_files::{list_run_commits, list_run_files}; use crate::run_manifest; @@ -206,15 +206,10 @@ async fn list_board_runs( } async fn link_run_parent( - subject: RequiredUser, + RequireRunScopedOrRunTools(child_id, actor): RequireRunScopedOrRunTools, State(state): State>, - Path(id): Path, Json(req): Json, ) -> Response { - let child_id = match parse_run_id_path(&id) { - Ok(id) => id, - Err(err) => return err.into_response(), - }; let parent_id = match req.parent_id.parse::() { Ok(parent_id) => parent_id, Err(err) => { @@ -249,7 +244,7 @@ async fn link_run_parent( &workflow_event::Event::RunParentLinked { previous_parent_id: child.parent_id, parent_id, - actor: Some(Principal::User(subject.0)), + actor: Some(actor), }, ) .await @@ -260,14 +255,9 @@ async fn link_run_parent( } async fn unlink_run_parent( - subject: RequiredUser, + RequireRunScopedOrRunTools(child_id, actor): RequireRunScopedOrRunTools, State(state): State>, - Path(id): Path, ) -> Response { - let child_id = match parse_run_id_path(&id) { - Ok(id) => id, - Err(err) => return err.into_response(), - }; let _parent_link_guard = state.parent_link_lock.lock().await; let child = match state.store.get_cached_summary(&child_id).await { Ok(Some(summary)) => summary, @@ -289,7 +279,7 @@ async fn unlink_run_parent( &child_id, &workflow_event::Event::RunParentUnlinked { previous_parent_id, - actor: Some(Principal::User(subject.0)), + actor: Some(actor), }, ) .await @@ -340,7 +330,7 @@ async fn updated_run_response(state: &AppState, run_id: &RunId) -> Response { } async fn list_runs( - _auth: RequiredUser, + _auth: RequiredRunToolActor, State(state): State>, Query(params): Query, ) -> Response { @@ -410,7 +400,7 @@ struct CommandLogResponseBody { } async fn resolve_run( - _auth: RequiredUser, + _auth: RequiredRunToolActor, State(state): State>, Query(query): Query, ) -> Response { @@ -527,15 +517,11 @@ async fn update_run( } async fn create_run( - RequestAuth(auth_slot): RequestAuth, + RequiredRunToolActor(actor): RequiredRunToolActor, State(state): State>, headers: HeaderMap, body: Bytes, ) -> Response { - let subject = match require_user(&auth_slot) { - Ok(subject) => subject, - Err(err) => return err.into_response(), - }; let req = match serde_json::from_slice::(&body) { Ok(req) => req, Err(err) => return ApiError::bad_request(err.to_string()).into_response(), @@ -562,7 +548,7 @@ async fn create_run( let mut create_input = run_manifest::create_run_input(prepared.clone(), configured_providers, web_url.clone()); create_input.run_id = Some(run_id); - create_input.provenance = Some(run_provenance(&headers, &subject)); + create_input.provenance = Some(run_provenance(&headers, &actor)); create_input.submitted_manifest_bytes = Some(body.to_vec()); let storage_root = match resolve_interp_string(&state.server_settings().server.storage.root) { @@ -622,13 +608,13 @@ async fn create_run( (StatusCode::CREATED, Json(summary)).into_response() } -fn run_provenance(headers: &HeaderMap, subject: &UserPrincipal) -> RunProvenance { +fn run_provenance(headers: &HeaderMap, subject: &Principal) -> RunProvenance { RunProvenance { server: Some(RunServerProvenance { version: FABRO_VERSION.to_string(), }), client: run_client_provenance(headers), - subject: Some(Principal::User(subject.clone())), + subject: Some(subject.clone()), } } @@ -713,14 +699,9 @@ async fn validate_run_manifest( } async fn get_run_status( - _auth: RequiredUser, + RequireRunScopedOrRunTools(id, _actor): RequireRunScopedOrRunTools, State(state): State>, - Path(id): Path, ) -> Response { - let id = match parse_run_id_path(&id) { - Ok(id) => id, - Err(response) => return response, - }; match state.store.get_cached_summary(&id).await { Ok(Some(run)) => (StatusCode::OK, Json(run)).into_response(), Ok(None) => ApiError::not_found("Run not found.").into_response(), @@ -755,14 +736,9 @@ async fn get_run_settings( } async fn get_questions( - _auth: RequiredUser, + RequireRunScopedOrRunTools(id, _actor): RequireRunScopedOrRunTools, State(state): State>, - Path(id): Path, ) -> Response { - let id = match parse_run_id_path(&id) { - Ok(id) => id, - Err(response) => return response, - }; match state.store.get_cached_run(&id).await { Ok(Some(cached)) => { let questions = cached @@ -781,15 +757,11 @@ async fn get_questions( } async fn submit_answer( - auth: RequiredUser, + RequireRunScopedOrRunTools(id, actor): RequireRunScopedOrRunTools, State(state): State>, - Path((id, qid)): Path<(String, String)>, + Path((_id, qid)): Path<(String, String)>, Json(req): Json, ) -> Response { - let id = match parse_run_id_path(&id) { - Ok(id) => id, - Err(response) => return response, - }; if let Some(response) = reject_if_archived(state.as_ref(), &id).await { return response; } @@ -801,7 +773,7 @@ async fn submit_answer( Ok(answer) => answer, Err(response) => return response, }; - let submission = AnswerSubmission::new(answer, Principal::User(auth.0)); + let submission = AnswerSubmission::new(answer, actor); match submit_pending_interview_answer(state.as_ref(), &pending, submission).await { Ok(()) => StatusCode::NO_CONTENT.into_response(), Err(response) => response, @@ -809,7 +781,7 @@ async fn submit_answer( } async fn get_run_state( - RequireRunScoped(id): RequireRunScoped, + RequireRunScopedOrRunTools(id, _actor): RequireRunScopedOrRunTools, State(state): State>, ) -> Response { match state.store.get_cached_run(&id).await { diff --git a/lib/crates/fabro-server/src/server/handler/steer.rs b/lib/crates/fabro-server/src/server/handler/steer.rs index e69292cee..03591b21e 100644 --- a/lib/crates/fabro-server/src/server/handler/steer.rs +++ b/lib/crates/fabro-server/src/server/handler/steer.rs @@ -1,7 +1,7 @@ use std::sync::Arc; use axum::Json; -use axum::extract::{Path, State}; +use axum::extract::State; use axum::http::StatusCode; use axum::response::{IntoResponse, Response}; use axum::routing::post; @@ -9,11 +9,9 @@ use fabro_api::types::SteerRunRequest; use fabro_types::Principal; use fabro_workflow::run_status::RunStatus; -use super::super::{ - AnswerTransportError, AppState, durable_run_status, parse_run_id_path, reject_if_archived, -}; +use super::super::{AnswerTransportError, AppState, durable_run_status, reject_if_archived}; use crate::error::ApiError; -use crate::principal_middleware::RequiredUser; +use crate::principal_middleware::RequireRunScopedOrRunTools; pub(super) fn routes() -> axum::Router> { axum::Router::new() @@ -34,9 +32,8 @@ impl RunControlRequest { } async fn steer_run( - auth: RequiredUser, + RequireRunScopedOrRunTools(id, actor): RequireRunScopedOrRunTools, State(state): State>, - Path(id): Path, Json(req): Json, ) -> Response { // OpenAPI enforces minLength=1/maxLength=8192 already; only whitespace-only @@ -52,27 +49,22 @@ async fn steer_run( RunControlRequest::Steer { text } }; - control_run(auth, state, id, control).await + control_run(actor, state, id, control).await } async fn interrupt_run( - auth: RequiredUser, + RequireRunScopedOrRunTools(id, actor): RequireRunScopedOrRunTools, State(state): State>, - Path(id): Path, ) -> Response { - control_run(auth, state, id, RunControlRequest::Interrupt).await + control_run(actor, state, id, RunControlRequest::Interrupt).await } async fn control_run( - auth: RequiredUser, + actor: Principal, state: Arc, - id: String, + id: fabro_types::RunId, control: RunControlRequest, ) -> Response { - let id = match parse_run_id_path(&id) { - Ok(id) => id, - Err(response) => return response, - }; if let Some(response) = reject_if_archived(state.as_ref(), &id).await { return response; } @@ -155,7 +147,6 @@ async fn control_run( .into_response(); }; - let actor = Principal::User(auth.0); let result = match control { RunControlRequest::Steer { text } => answer_transport.steer(text, actor).await, RunControlRequest::Interrupt => answer_transport.interrupt(actor).await, diff --git a/lib/crates/fabro-server/src/server/tests.rs b/lib/crates/fabro-server/src/server/tests.rs index 35da8ff60..52affb51e 100644 --- a/lib/crates/fabro-server/src/server/tests.rs +++ b/lib/crates/fabro-server/src/server/tests.rs @@ -573,7 +573,18 @@ fn issue_test_user_jwt() -> String { fn issue_test_worker_token(run_id: &RunId) -> String { let keys = WorkerTokenKeys::from_master_secret(TEST_SESSION_SECRET.as_bytes()) .expect("worker keys should derive"); - issue_worker_token(&keys, run_id).expect("worker token should issue") + crate::worker_token::issue_worker_token(&keys, run_id).expect("worker token should issue") +} + +fn issue_test_run_tools_worker_token(run_id: &RunId) -> String { + let keys = WorkerTokenKeys::from_master_secret(TEST_SESSION_SECRET.as_bytes()) + .expect("worker keys should derive"); + crate::worker_token::issue_worker_token_with_scopes( + &keys, + run_id, + crate::worker_token::WorkerScopeSet::run_worker_with_agent_run_tools(), + ) + .expect("worker token should issue") } async fn create_run_with_bearer(app: &Router, bearer: &str) -> RunId { @@ -603,6 +614,21 @@ fn bearer_request(method: Method, path: &str, bearer: &str, body: Body) -> Reque .unwrap() } +fn json_bearer_request( + method: Method, + path: &str, + bearer: &str, + body: &serde_json::Value, +) -> Request { + Request::builder() + .method(method) + .uri(api(path)) + .header(header::AUTHORIZATION, format!("Bearer {bearer}")) + .header(header::CONTENT_TYPE, "application/json") + .body(Body::from(serde_json::to_vec(&body).unwrap())) + .unwrap() +} + fn canonical_origin_settings(url: &str) -> ServerSettings { server_settings_from_toml(&format!( r#" @@ -1535,6 +1561,10 @@ fn worker_command_always_sets_worker_token_env() { .expect("github worker token should decode") .claims; assert_eq!(github_claims.run_id, github_run_id.to_string()); + assert_eq!( + github_claims.scope.split_whitespace().collect::>(), + vec!["run:worker", "agent:run_tools"] + ); let dev_token = tempfile::tempdir().unwrap(); let dev_token_state = @@ -1568,6 +1598,10 @@ fn worker_command_always_sets_worker_token_env() { .expect("dev-token worker token should decode") .claims; assert_eq!(dev_claims.run_id, dev_token_run_id.to_string()); + assert_eq!( + dev_claims.scope.split_whitespace().collect::>(), + vec!["run:worker", "agent:run_tools"] + ); } #[cfg(unix)] @@ -6940,6 +6974,23 @@ async fn worker_token_accepts_run_scoped_routes_and_falls_back_to_user_jwt() { .unwrap(); assert_status!(response, StatusCode::OK).await; + for path in [ + format!("/runs/{run_id}"), + format!("/runs/{run_id}/questions"), + ] { + let response = app + .clone() + .oneshot(bearer_request( + Method::GET, + &path, + &worker_token, + Body::empty(), + )) + .await + .unwrap(); + assert_status!(response, StatusCode::OK).await; + } + let append_body = serde_json::to_vec(&serde_json::json!({ "id": "evt-run-notice", "ts": "2026-04-23T12:00:00Z", @@ -7028,6 +7079,203 @@ async fn worker_token_accepts_run_scoped_routes_and_falls_back_to_user_jwt() { assert_status!(response, StatusCode::FORBIDDEN).await; } +#[tokio::test] +async fn run_tool_worker_token_can_use_client_backend_routes_across_runs() { + let (state, app) = jwt_auth_app(); + let user_jwt = issue_test_user_jwt(); + let parent_run_id = create_run_with_bearer(&app, &user_jwt).await; + let target_run_id = create_run_with_bearer(&app, &user_jwt).await; + let run_tool_worker_token = issue_test_run_tools_worker_token(&parent_run_id); + + let response = app + .clone() + .oneshot(bearer_request( + Method::GET, + "/runs", + &run_tool_worker_token, + Body::empty(), + )) + .await + .unwrap(); + assert_status!(response, StatusCode::OK).await; + + let response = app + .clone() + .oneshot(bearer_request( + Method::GET, + &format!("/runs/resolve?selector={target_run_id}"), + &run_tool_worker_token, + Body::empty(), + )) + .await + .unwrap(); + assert_status!(response, StatusCode::OK).await; + + for path in [ + format!("/runs/{target_run_id}"), + format!("/runs/{target_run_id}/state"), + format!("/runs/{target_run_id}/events"), + format!("/runs/{target_run_id}/questions"), + ] { + let response = app + .clone() + .oneshot(bearer_request( + Method::GET, + &path, + &run_tool_worker_token, + Body::empty(), + )) + .await + .unwrap(); + assert_status!(response, StatusCode::OK).await; + } + + let response = app + .clone() + .oneshot(json_bearer_request( + Method::POST, + &format!("/runs/{target_run_id}/start"), + &run_tool_worker_token, + &json!({ "resume": false }), + )) + .await + .unwrap(); + assert_status!(response, StatusCode::OK).await; + + let response = app + .clone() + .oneshot(bearer_request( + Method::POST, + &format!("/runs/{target_run_id}/cancel"), + &run_tool_worker_token, + Body::empty(), + )) + .await + .unwrap(); + assert_status!(response, StatusCode::OK).await; + + for path in [ + format!("/runs/{target_run_id}/archive"), + format!("/runs/{target_run_id}/unarchive"), + format!("/runs/{target_run_id}/interrupt"), + ] { + let response = app + .clone() + .oneshot(bearer_request( + Method::POST, + &path, + &run_tool_worker_token, + Body::empty(), + )) + .await + .unwrap(); + assert_ne!(response.status(), StatusCode::UNAUTHORIZED, "{path}"); + assert_ne!(response.status(), StatusCode::FORBIDDEN, "{path}"); + } + + let response = app + .clone() + .oneshot(json_bearer_request( + Method::POST, + &format!("/runs/{target_run_id}/steer"), + &run_tool_worker_token, + &json!({ "text": "continue", "interrupt": false }), + )) + .await + .unwrap(); + assert_ne!(response.status(), StatusCode::UNAUTHORIZED); + assert_ne!(response.status(), StatusCode::FORBIDDEN); + + let response = app + .clone() + .oneshot(json_bearer_request( + Method::POST, + &format!("/runs/{target_run_id}/questions/q-1/answer"), + &run_tool_worker_token, + &json!({ "kind": "yes" }), + )) + .await + .unwrap(); + assert_ne!(response.status(), StatusCode::UNAUTHORIZED); + assert_ne!(response.status(), StatusCode::FORBIDDEN); + + let created_child = create_run_with_bearer(&app, &run_tool_worker_token).await; + let cached = state + .store + .get_cached_run(&created_child) + .await + .unwrap() + .expect("created run should be cached"); + assert_eq!( + cached + .projection + .spec + .provenance + .as_ref() + .and_then(|provenance| provenance.subject.as_ref()), + Some(&Principal::Worker { + run_id: parent_run_id, + }), + ); + + let response = app + .clone() + .oneshot(json_bearer_request( + Method::PUT, + &format!("/runs/{created_child}/parent"), + &run_tool_worker_token, + &json!({ "parent_id": target_run_id.to_string() }), + )) + .await + .unwrap(); + assert_status!(response, StatusCode::OK).await; + + let response = app + .clone() + .oneshot(bearer_request( + Method::DELETE, + &format!("/runs/{created_child}/parent"), + &run_tool_worker_token, + Body::empty(), + )) + .await + .unwrap(); + assert_status!(response, StatusCode::OK).await; +} + +#[tokio::test] +async fn base_worker_token_is_rejected_by_run_tool_only_routes() { + let (_state, app) = jwt_auth_app(); + let user_jwt = issue_test_user_jwt(); + let run_id = create_run_with_bearer(&app, &user_jwt).await; + let worker_token = issue_test_worker_token(&run_id); + + for (method, path) in [ + (Method::GET, "/runs".to_string()), + (Method::POST, "/runs".to_string()), + (Method::GET, "/runs/resolve?selector=latest".to_string()), + ] { + let response = app + .clone() + .oneshot(bearer_request( + method.clone(), + &path, + &worker_token, + Body::empty(), + )) + .await + .unwrap(); + assert!( + matches!( + response.status(), + StatusCode::UNAUTHORIZED | StatusCode::FORBIDDEN + ), + "{method} {path} unexpectedly accepted base worker token with status {}", + response.status() + ); + } +} + #[tokio::test] async fn worker_token_controls_stage_artifact_route() { let (_state, app) = jwt_auth_app(); @@ -7195,18 +7443,11 @@ async fn worker_token_is_rejected_on_user_only_routes() { (Method::POST, "/graph/render".to_string()), (Method::GET, "/attach".to_string()), (Method::GET, "/boards/runs".to_string()), - (Method::GET, format!("/runs/{run_id}")), (Method::DELETE, format!("/runs/{run_id}")), - (Method::GET, format!("/runs/{run_id}/questions")), - (Method::POST, format!("/runs/{run_id}/questions/q-1/answer")), (Method::GET, format!("/runs/{run_id}/attach")), (Method::GET, format!("/runs/{run_id}/checkpoint")), - (Method::POST, format!("/runs/{run_id}/cancel")), - (Method::POST, format!("/runs/{run_id}/start")), (Method::POST, format!("/runs/{run_id}/pause")), (Method::POST, format!("/runs/{run_id}/unpause")), - (Method::POST, format!("/runs/{run_id}/archive")), - (Method::POST, format!("/runs/{run_id}/unarchive")), (Method::GET, format!("/runs/{run_id}/graph")), (Method::GET, format!("/runs/{run_id}/graph/source")), (Method::GET, format!("/runs/{run_id}/stages")), diff --git a/lib/crates/fabro-server/src/spawn_env.rs b/lib/crates/fabro-server/src/spawn_env.rs index 2dd7100b4..5c1ed8140 100644 --- a/lib/crates/fabro-server/src/spawn_env.rs +++ b/lib/crates/fabro-server/src/spawn_env.rs @@ -98,6 +98,7 @@ mod tests { ("GITHUB_APP_CLIENT_SECRET".to_string(), "leak".to_string()), ("GITHUB_APP_WEBHOOK_SECRET".to_string(), "leak".to_string()), ("FABRO_DEV_TOKEN".to_string(), "garbage".to_string()), + ("FABRO_WORKER_TOKEN".to_string(), "leak".to_string()), ("MY_API_KEY".to_string(), "blocked".to_string()), ]); let mut cmd = env_command(); @@ -132,6 +133,7 @@ mod tests { assert!(!actual.contains_key("GITHUB_APP_PRIVATE_KEY")); assert!(!actual.contains_key("GITHUB_APP_CLIENT_SECRET")); assert!(!actual.contains_key("GITHUB_APP_WEBHOOK_SECRET")); + assert!(!actual.contains_key("FABRO_WORKER_TOKEN")); assert!(!actual.contains_key("MY_API_KEY")); } diff --git a/lib/crates/fabro-server/src/worker_token.rs b/lib/crates/fabro-server/src/worker_token.rs index cc950d548..f64765801 100644 --- a/lib/crates/fabro-server/src/worker_token.rs +++ b/lib/crates/fabro-server/src/worker_token.rs @@ -14,6 +14,7 @@ use crate::auth::{self, JwtError, KeyDeriveError}; pub(crate) const WORKER_TOKEN_ISSUER: &str = "fabro-server-worker"; pub(crate) const WORKER_TOKEN_KID: &str = "fabro-worker"; pub(crate) const WORKER_TOKEN_SCOPE: &str = "run:worker"; +pub(crate) const WORKER_RUN_TOOLS_SCOPE: &str = "agent:run_tools"; pub(crate) const WORKER_TOKEN_TTL_SECS: u64 = 72 * 60 * 60; #[derive(Clone)] @@ -59,9 +60,59 @@ pub(crate) struct WorkerTokenClaims { pub(crate) jti: String, } +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) struct WorkerScopeSet { + agent_run_tools: bool, +} + +impl WorkerScopeSet { + #[cfg(test)] + #[must_use] + pub(crate) const fn run_worker() -> Self { + Self { + agent_run_tools: false, + } + } + + #[must_use] + pub(crate) const fn run_worker_with_agent_run_tools() -> Self { + Self { + agent_run_tools: true, + } + } + + #[must_use] + pub(crate) const fn has_agent_run_tools(self) -> bool { + self.agent_run_tools + } + + fn claim(self) -> String { + if self.agent_run_tools { + [WORKER_TOKEN_SCOPE, WORKER_RUN_TOOLS_SCOPE].join(" ") + } else { + WORKER_TOKEN_SCOPE.to_string() + } + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct DecodedWorkerToken { + pub(crate) run_id: RunId, + pub(crate) scopes: WorkerScopeSet, +} + +#[cfg(test)] pub(crate) fn issue_worker_token( keys: &WorkerTokenKeys, run_id: &RunId, +) -> Result { + issue_worker_token_with_scopes(keys, run_id, WorkerScopeSet::run_worker()) +} + +pub(crate) fn issue_worker_token_with_scopes( + keys: &WorkerTokenKeys, + run_id: &RunId, + scopes: WorkerScopeSet, ) -> Result { let now = SystemTime::now() .duration_since(UNIX_EPOCH) @@ -71,7 +122,7 @@ pub(crate) fn issue_worker_token( iat: now, exp: now + WORKER_TOKEN_TTL_SECS, run_id: run_id.to_string(), - scope: WORKER_TOKEN_SCOPE.to_string(), + scope: scopes.claim(), jti: Uuid::new_v4().simple().to_string(), }; jsonwebtoken::encode(&worker_token_header(), &claims, &keys.encoding).map_err(|err| { @@ -88,7 +139,10 @@ pub(crate) fn worker_token_header() -> Header { header } -pub(crate) fn decode_worker_token(token: &str, keys: &WorkerTokenKeys) -> Result { +pub(crate) fn decode_worker_token( + token: &str, + keys: &WorkerTokenKeys, +) -> Result { let claims = jsonwebtoken::decode::(token, &keys.decoding, &keys.validation) .map_err(|err| match err.kind() { ErrorKind::ExpiredSignature => JwtError::AccessTokenExpired, @@ -96,20 +150,39 @@ pub(crate) fn decode_worker_token(token: &str, keys: &WorkerTokenKeys) -> Result })? .claims; - if claims.scope != WORKER_TOKEN_SCOPE { + let scopes = parse_worker_scopes(&claims.scope).map_err(|()| { warn!( target: "worker_auth", jti = %claims.jti, reason = "wrong_scope", "worker token rejected" ); - return Err(JwtError::AccessTokenInvalid); - } + JwtError::AccessTokenInvalid + })?; - claims + let run_id = claims .run_id .parse() - .map_err(|_| JwtError::AccessTokenInvalid) + .map_err(|_| JwtError::AccessTokenInvalid)?; + Ok(DecodedWorkerToken { run_id, scopes }) +} + +fn parse_worker_scopes(scope: &str) -> Result { + let mut has_run_worker = false; + let mut has_agent_run_tools = false; + for scope in scope.split_whitespace() { + match scope { + WORKER_TOKEN_SCOPE => has_run_worker = true, + WORKER_RUN_TOOLS_SCOPE => has_agent_run_tools = true, + _ => return Err(()), + } + } + if !has_run_worker { + return Err(()); + } + Ok(WorkerScopeSet { + agent_run_tools: has_agent_run_tools, + }) } #[cfg(test)] @@ -121,8 +194,9 @@ mod tests { use uuid::Uuid; use super::{ - WORKER_TOKEN_ISSUER, WORKER_TOKEN_KID, WORKER_TOKEN_SCOPE, WorkerTokenClaims, - WorkerTokenKeys, decode_worker_token, issue_worker_token, worker_token_header, + WORKER_RUN_TOOLS_SCOPE, WORKER_TOKEN_ISSUER, WORKER_TOKEN_KID, WORKER_TOKEN_SCOPE, + WorkerScopeSet, WorkerTokenClaims, WorkerTokenKeys, decode_worker_token, + issue_worker_token, issue_worker_token_with_scopes, worker_token_header, }; use crate::auth::{self, JwtError}; @@ -138,12 +212,20 @@ mod tests { } fn wrong_scope_token(keys: &WorkerTokenKeys, run_id: &fabro_types::RunId) -> String { + worker_token_with_scope(keys, run_id, "wrong:scope") + } + + fn worker_token_with_scope( + keys: &WorkerTokenKeys, + run_id: &fabro_types::RunId, + scope: &str, + ) -> String { let claims = WorkerTokenClaims { iss: WORKER_TOKEN_ISSUER.to_string(), iat: 1, exp: u64::MAX / 2, run_id: run_id.to_string(), - scope: "wrong:scope".to_string(), + scope: scope.to_string(), jti: Uuid::new_v4().simple().to_string(), }; jsonwebtoken::encode(&worker_token_header(), &claims, &keys.encoding) @@ -249,8 +331,53 @@ mod tests { let run_id = run_id(); let keys = keys(TEST_SECRET); let token = issue_worker_token(&keys, &run_id).expect("worker token should issue"); + let decoded = decode_worker_token(&token, &keys).unwrap(); - assert_eq!(decode_worker_token(&token, &keys).unwrap(), run_id); + assert_eq!(decoded.run_id, run_id); + assert!(!decoded.scopes.has_agent_run_tools()); + } + + #[test] + fn decode_worker_token_exposes_run_tool_scope() { + let run_id = run_id(); + let keys = keys(TEST_SECRET); + let token = issue_worker_token_with_scopes( + &keys, + &run_id, + WorkerScopeSet::run_worker_with_agent_run_tools(), + ) + .expect("worker token should issue"); + let decoded = decode_worker_token(&token, &keys).unwrap(); + + assert_eq!(decoded.run_id, run_id); + assert!(decoded.scopes.has_agent_run_tools()); + } + + #[test] + fn decode_worker_token_accepts_space_delimited_run_tool_scope_claim() { + let run_id = run_id(); + let keys = keys(TEST_SECRET); + let token = worker_token_with_scope( + &keys, + &run_id, + &format!("{WORKER_TOKEN_SCOPE} {WORKER_RUN_TOOLS_SCOPE}"), + ); + let decoded = decode_worker_token(&token, &keys).unwrap(); + + assert_eq!(decoded.run_id, run_id); + assert!(decoded.scopes.has_agent_run_tools()); + } + + #[test] + fn decode_worker_token_rejects_missing_base_scope() { + let run_id = run_id(); + let keys = keys(TEST_SECRET); + let token = worker_token_with_scope(&keys, &run_id, WORKER_RUN_TOOLS_SCOPE); + + assert_eq!( + decode_worker_token(&token, &keys).expect_err("base worker scope should be required"), + JwtError::AccessTokenInvalid, + ); } #[test] diff --git a/lib/crates/fabro-tool/Cargo.toml b/lib/crates/fabro-tool/Cargo.toml new file mode 100644 index 000000000..2457bacc8 --- /dev/null +++ b/lib/crates/fabro-tool/Cargo.toml @@ -0,0 +1,31 @@ +[package] +name = "fabro-tool" +edition.workspace = true +version.workspace = true +publish = false +license.workspace = true +description = "Shared Fabro run-control tool behavior" + +[lib] +doctest = false + +[lints] +workspace = true + +[dependencies] +anyhow.workspace = true +async-trait.workspace = true +chrono = { workspace = true, features = ["serde"] } +fabro-api = { path = "../fabro-api" } +fabro-client = { path = "../fabro-client" } +fabro-types = { path = "../fabro-types" } +fabro-util = { path = "../fabro-util" } +futures.workspace = true +schemars = "1.2.1" +serde.workspace = true +serde_json.workspace = true +tokio.workspace = true +toml.workspace = true + +[dev-dependencies] +tempfile = "3" diff --git a/lib/crates/fabro-tool/src/common.rs b/lib/crates/fabro-tool/src/common.rs new file mode 100644 index 000000000..492d83ee5 --- /dev/null +++ b/lib/crates/fabro-tool/src/common.rs @@ -0,0 +1,319 @@ +use std::collections::HashMap; +use std::path::Path; +use std::sync::LazyLock; + +use async_trait::async_trait; +use chrono::{DateTime, NaiveDate, Utc}; +use fabro_api::types; +use fabro_types::{Run, RunId, RunStatus}; +use fabro_util::exit::{self, ExitClass}; +use schemars::JsonSchema; +use serde::Serialize; +use serde_json::Value; + +#[derive(Debug)] +pub struct ToolError { + message: String, +} + +impl ToolError { + pub fn message(message: impl Into) -> Self { + Self { + message: message.into(), + } + } + + pub fn from_anyhow(err: &anyhow::Error) -> Self { + Self::message(format_tool_error(err)) + } + + #[must_use] + pub fn as_str(&self) -> &str { + &self.message + } +} + +impl std::fmt::Display for ToolError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.message) + } +} + +impl std::error::Error for ToolError {} + +pub type ToolResult = Result; + +#[async_trait] +pub trait FabroToolBackend: Send + Sync { + async fn create_run_from_spec( + &self, + spec: &crate::ValidatedCreateRunSpec, + cwd: &Path, + user_settings_path: &Path, + parent_id: Option, + ) -> anyhow::Result; + + async fn resolve_run(&self, selector: &str) -> anyhow::Result; + async fn retrieve_run(&self, run_id: &RunId) -> anyhow::Result; + async fn start_run(&self, run_id: &RunId, resume: bool) -> anyhow::Result; + async fn cancel_run(&self, run_id: &RunId) -> anyhow::Result; + async fn interrupt_run(&self, run_id: &RunId) -> anyhow::Result<()>; + async fn steer_run(&self, run_id: &RunId, text: String, interrupt: bool) -> anyhow::Result<()>; + async fn archive_run(&self, run_id: &RunId) -> anyhow::Result; + async fn unarchive_run(&self, run_id: &RunId) -> anyhow::Result; + async fn list_store_runs(&self) -> anyhow::Result>; + async fn list_store_runs_by_parent(&self, parent_id: RunId) -> anyhow::Result>; + async fn link_run_parent(&self, child_id: &RunId, parent_id: &RunId) -> anyhow::Result; + async fn unlink_run_parent(&self, child_id: &RunId) -> anyhow::Result; + async fn get_run_state(&self, run_id: &RunId) -> anyhow::Result; + async fn list_run_events( + &self, + run_id: &RunId, + after: Option, + limit: Option, + ) -> anyhow::Result>; + async fn list_run_events_until( + &self, + run_id: &RunId, + after: Option, + limit: usize, + ) -> anyhow::Result>; + async fn list_run_questions(&self, run_id: &RunId) -> anyhow::Result>; + async fn submit_run_answer( + &self, + run_id: &RunId, + question_id: &str, + body: types::SubmitAnswerRequest, + ) -> anyhow::Result<()>; +} + +pub trait RunManifestBuilder: Send + Sync { + fn build_run_manifest( + &self, + spec: &crate::ValidatedCreateRunSpec, + cwd: &Path, + user_settings_path: &Path, + ) -> ToolResult; +} + +#[derive(Debug, Serialize, JsonSchema)] +pub struct RunSummaryResult { + pub run_id: String, + pub parent_id: Option, + pub children_count: u64, + pub workflow_name: Option, + pub workflow_graph_name: Option, + pub workflow_slug: Option, + pub status: String, + pub archived: bool, + pub created_at: String, + pub started_at: Option, + pub completed_at: Option, + pub labels: HashMap, + pub source_directory: Option, + pub repo_origin_url: Option, + pub goal: String, +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +pub struct ToolDefinition { + pub name: &'static str, + pub description: &'static str, + pub parameters: Value, +} + +pub const FABRO_RUN_CREATE_TOOL_NAME: &str = "fabro_run_create"; +pub const FABRO_RUN_SEARCH_TOOL_NAME: &str = "fabro_run_search"; +pub const FABRO_RUN_INTERACT_TOOL_NAME: &str = "fabro_run_interact"; +pub const FABRO_RUN_GATHER_TOOL_NAME: &str = "fabro_run_gather"; +pub const FABRO_RUN_EVENTS_TOOL_NAME: &str = "fabro_run_events"; + +static TOOL_DEFINITIONS: LazyLock> = LazyLock::new(|| { + vec![ + tool_definition::( + FABRO_RUN_CREATE_TOOL_NAME, + "Create one or more Fabro workflow runs, optionally under a parent run, starting them by default.", + ), + tool_definition::( + FABRO_RUN_SEARCH_TOOL_NAME, + "Search Fabro workflow runs by id, parent, workflow, labels, status, archival state, and creation time.", + ), + tool_definition::( + FABRO_RUN_INTERACT_TOOL_NAME, + "Get, start, message, interrupt, cancel, archive, unarchive, link or unlink a parent, inspect questions, or answer a Fabro run.", + ), + tool_definition::( + FABRO_RUN_GATHER_TOOL_NAME, + "Wait for Fabro runs to reach terminal states, returning current state on timeout.", + ), + tool_definition::( + FABRO_RUN_EVENTS_TOOL_NAME, + "List, inspect, or search stored events for a Fabro workflow run.", + ), + ] +}); + +#[must_use] +pub fn tool_definitions() -> &'static [ToolDefinition] { + TOOL_DEFINITIONS.as_slice() +} + +fn tool_definition(name: &'static str, description: &'static str) -> ToolDefinition +where + T: JsonSchema, +{ + ToolDefinition { + name, + description, + parameters: serde_json::to_value(schemars::schema_for!(T)) + .expect("tool parameter schema should serialize"), + } +} + +pub(super) fn validate_len(name: &str, len: usize, min: usize, max: usize) -> ToolResult<()> { + if len < min { + return Err(ToolError::message(format!( + "{name} must contain at least {min} item(s)" + ))); + } + if len > max { + return Err(ToolError::message(format!( + "{name} must contain no more than {max} item(s)" + ))); + } + Ok(()) +} + +pub(super) async fn retrieve_run( + backend: &dyn FabroToolBackend, + run_id: &RunId, +) -> ToolResult { + backend + .retrieve_run(run_id) + .await + .map_err(|err| ToolError::from_anyhow(&err)) +} + +pub(crate) fn run_summary_result(run: &Run) -> RunSummaryResult { + RunSummaryResult { + run_id: run.id.to_string(), + parent_id: run.parent_id.map(|parent_id| parent_id.to_string()), + children_count: run.children_count, + workflow_name: run.workflow.name.clone(), + workflow_graph_name: run.workflow.graph_name.clone(), + workflow_slug: run.workflow.slug.clone(), + status: run_status_kind(run.lifecycle.status).to_string(), + archived: run.lifecycle.archived, + created_at: run.timestamps.created_at.to_rfc3339(), + started_at: run + .timestamps + .started_at + .map(|timestamp| timestamp.to_rfc3339()), + completed_at: run + .timestamps + .completed_at + .map(|timestamp| timestamp.to_rfc3339()), + labels: run.labels.clone(), + source_directory: run.source_directory.clone(), + repo_origin_url: run + .repository + .as_ref() + .and_then(|repository| repository.origin_url.clone()), + goal: run.goal.clone(), + } +} + +pub(crate) fn parse_datetime_filter(name: &str, raw: &str) -> ToolResult> { + if let Ok(timestamp) = DateTime::parse_from_rfc3339(raw) { + return Ok(timestamp.with_timezone(&Utc)); + } + let date = NaiveDate::parse_from_str(raw, "%Y-%m-%d").map_err(|err| { + ToolError::message(format!("{name} must be RFC3339 or YYYY-MM-DD: {err}")) + })?; + let datetime = date + .and_hms_opt(0, 0, 0) + .ok_or_else(|| ToolError::message(format!("{name} contains an invalid date")))?; + Ok(DateTime::from_naive_utc_and_offset(datetime, Utc)) +} + +pub(crate) fn run_status_kind(status: RunStatus) -> &'static str { + status.kind().into() +} + +fn format_tool_error(err: &anyhow::Error) -> String { + let mut rendered = format!("{err:#}"); + if exit::exit_class_for(err) == Some(ExitClass::AuthRequired) + && !rendered.contains("fabro auth login") + { + rendered.push_str("\nRun `fabro auth login` to authenticate."); + } + rendered +} + +#[cfg(test)] +mod tests { + use chrono::{TimeZone, Utc}; + use fabro_types::{RunLifecycle, RunLinks, RunOrigin, RunTimestamps, WorkflowRef}; + + use super::*; + + #[test] + fn run_summary_result_includes_parent_metadata() { + let parent_id = run_id("01KRBZW4DW0000000000000002"); + let run = Run { + id: run_id("01KRBZW5C00000000000000001"), + parent_id: Some(parent_id), + children_count: 3, + title: "test".to_string(), + goal: "test".to_string(), + workflow: WorkflowRef { + slug: Some("simple".to_string()), + name: Some("Simple".to_string()), + graph_name: Some("GraphName".to_string()), + node_count: 0, + edge_count: 0, + }, + automation: None, + repository: None, + created_by: None, + origin: RunOrigin::default(), + labels: HashMap::new(), + lifecycle: RunLifecycle { + status: RunStatus::Submitted, + pending_control: None, + queue_position: None, + error: None, + archived: false, + archived_at: None, + }, + sandbox: None, + models: Vec::new(), + source_directory: None, + timestamps: RunTimestamps { + created_at: Utc.with_ymd_and_hms(2026, 5, 11, 12, 0, 0).unwrap(), + started_at: None, + last_event_at: None, + completed_at: None, + duration_ms: None, + elapsed_secs: None, + }, + billing: None, + diff: None, + pull_request: None, + current_question: None, + superseded_by: None, + links: RunLinks { web: None }, + }; + + let summary = run_summary_result(&run); + + assert_eq!(summary.parent_id, Some(parent_id.to_string())); + assert_eq!(summary.children_count, 3); + assert_eq!(summary.workflow_name.as_deref(), Some("Simple")); + assert_eq!(summary.workflow_graph_name.as_deref(), Some("GraphName")); + } + + fn run_id(raw: &str) -> RunId { + raw.parse().expect("test run id should parse") + } +} diff --git a/lib/crates/fabro-tool/src/create.rs b/lib/crates/fabro-tool/src/create.rs new file mode 100644 index 000000000..b69e95ae7 --- /dev/null +++ b/lib/crates/fabro-tool/src/create.rs @@ -0,0 +1,646 @@ +use std::borrow::Cow; +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +use fabro_types::RunId; +use schemars::{JsonSchema, Schema, SchemaGenerator, json_schema}; +use serde::{Deserialize, Serialize}; +use serde_json::Value; + +use super::common::{self, FabroToolBackend, ToolError, ToolResult}; +use super::manifest; + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct FabroRunCreateParams { + pub runs: Vec, +} + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct CreateRunSpec { + pub workflow: String, + pub cwd: Option, + pub run_id: Option, + pub parent_id: Option, + pub goal: Option, + #[serde(default)] + pub inputs: HashMap, + #[serde(default)] + pub labels: HashMap, + pub dry_run: Option, + pub auto_approve: Option, + pub model: Option, + pub provider: Option, + pub sandbox: Option, + pub preserve_sandbox: Option, + pub start: Option, +} + +#[derive(Debug, Deserialize)] +#[serde(transparent)] +pub struct RunInputValue(Value); + +impl From for RunInputValue { + fn from(value: Value) -> Self { + Self(value) + } +} + +impl RunInputValue { + pub(crate) fn into_inner(self) -> Value { + self.0 + } +} + +impl JsonSchema for RunInputValue { + fn inline_schema() -> bool { + true + } + + fn schema_name() -> Cow<'static, str> { + "RunInputValue".into() + } + + fn json_schema(_: &mut SchemaGenerator) -> Schema { + json_schema!({ + "description": "Run input override value. Inputs are TOML-compatible scalar values: string, boolean, integer, or float.", + "anyOf": [ + { "type": "string" }, + { "type": "boolean" }, + { "type": "integer" }, + { "type": "number" } + ] + }) + } +} + +#[derive(Debug)] +pub struct ValidatedCreateRuns { + pub runs: Vec, +} + +#[derive(Debug)] +pub struct ValidatedCreateRunSpec { + pub workflow: String, + pub cwd: Option, + pub run_id: Option, + pub parent_id: Option, + pub goal: Option, + pub inputs: HashMap, + pub labels: HashMap, + pub dry_run: Option, + pub auto_approve: Option, + pub model: Option, + pub provider: Option, + pub sandbox: Option, + pub preserve_sandbox: Option, + pub start: Option, +} + +impl TryFrom for ValidatedCreateRuns { + type Error = ToolError; + + fn try_from(params: FabroRunCreateParams) -> Result { + common::validate_len("runs", params.runs.len(), 1, 50)?; + let runs = params + .runs + .into_iter() + .map(ValidatedCreateRunSpec::try_from) + .collect::, _>>()?; + Ok(Self { runs }) + } +} + +impl TryFrom for ValidatedCreateRunSpec { + type Error = ToolError; + + fn try_from(spec: CreateRunSpec) -> Result { + let run_id = spec + .run_id + .as_deref() + .map(str::parse::) + .transpose() + .map_err(|err| { + ToolError::message(format!("run_id must be a valid Fabro run id: {err}")) + })?; + let parent_id = spec + .parent_id + .as_deref() + .map(str::trim) + .filter(|parent_id| !parent_id.is_empty()) + .map(ToOwned::to_owned); + if spec.parent_id.is_some() && parent_id.is_none() { + return Err(ToolError::message("parent_id must not be blank")); + } + let inputs = spec + .inputs + .into_iter() + .map(|(key, value)| { + let value = value.into_inner(); + manifest::json_to_toml_value(&key, &value).map(|value| (key, value)) + }) + .collect::>>()?; + Ok(Self { + workflow: spec.workflow, + cwd: spec.cwd, + run_id, + parent_id, + goal: spec.goal, + inputs, + labels: spec.labels, + dry_run: spec.dry_run, + auto_approve: spec.auto_approve, + model: spec.model, + provider: spec.provider, + sandbox: spec.sandbox, + preserve_sandbox: spec.preserve_sandbox, + start: spec.start, + }) + } +} + +#[derive(Debug, Serialize, JsonSchema)] +pub struct CreateRunsResult { + pub runs: Vec, +} + +#[derive(Debug, Serialize, JsonSchema)] +pub struct CreatedRunResult { + pub run_id: String, + pub parent_id: Option, + pub children_count: u64, + pub workflow: String, + pub started: bool, + pub status: String, +} + +#[derive(Debug, Clone, Copy, Default)] +pub struct CreateRunOptions { + pub forced_parent_id: Option, +} + +pub async fn create_runs( + backend: Arc, + base_cwd: &Path, + user_settings_path: &Path, + params: ValidatedCreateRuns, +) -> ToolResult { + create_runs_with_options( + backend, + base_cwd, + user_settings_path, + params, + CreateRunOptions::default(), + ) + .await +} + +pub async fn create_runs_with_options( + backend: Arc, + base_cwd: &Path, + user_settings_path: &Path, + params: ValidatedCreateRuns, + options: CreateRunOptions, +) -> ToolResult { + let mut created = Vec::with_capacity(params.runs.len()); + let mut parent_id_cache = HashMap::::new(); + for spec in params.runs { + let cwd = spec.cwd.clone().unwrap_or_else(|| base_cwd.to_path_buf()); + let parent_id = if let Some(forced_parent_id) = options.forced_parent_id { + Some(forced_parent_id) + } else if let Some(parent_selector) = spec.parent_id.as_deref() { + Some( + resolve_parent_run_id(backend.as_ref(), &mut parent_id_cache, parent_selector) + .await?, + ) + } else { + None + }; + let run_id = backend + .create_run_from_spec(&spec, &cwd, user_settings_path, parent_id) + .await + .map_err(|err| ToolError::from_anyhow(&err))?; + let started = spec.start.unwrap_or(true); + let summary = if started { + backend + .start_run(&run_id, false) + .await + .map_err(|err| ToolError::from_anyhow(&err))? + } else { + backend + .retrieve_run(&run_id) + .await + .map_err(|err| ToolError::from_anyhow(&err))? + }; + created.push(CreatedRunResult { + run_id: summary.id.to_string(), + parent_id: summary.parent_id.map(|parent_id| parent_id.to_string()), + children_count: summary.children_count, + workflow: spec.workflow, + started, + status: common::run_status_kind(summary.lifecycle.status).to_string(), + }); + } + Ok(CreateRunsResult { runs: created }) +} + +async fn resolve_parent_run_id( + backend: &dyn FabroToolBackend, + parent_id_cache: &mut HashMap, + parent_selector: &str, +) -> ToolResult { + if let Ok(parent_id) = parent_selector.parse::() { + return Ok(parent_id); + } + if let Some(parent_id) = parent_id_cache.get(parent_selector) { + return Ok(*parent_id); + } + + let parent_id = backend + .resolve_run(parent_selector) + .await + .map_err(|err| ToolError::from_anyhow(&err))? + .id; + parent_id_cache.insert(parent_selector.to_string(), parent_id); + Ok(parent_id) +} + +pub fn create_runs_text(result: &CreateRunsResult) -> String { + let started = result.runs.iter().filter(|run| run.started).count(); + format!( + "created {} Fabro run(s), started {started}", + result.runs.len() + ) +} + +#[cfg(test)] +mod tests { + use std::sync::Mutex; + + use async_trait::async_trait; + use chrono::{TimeZone, Utc}; + use fabro_api::types; + use fabro_types::{ + EventEnvelope, Run, RunLifecycle, RunLinks, RunOrigin, RunProjection, RunStatus, + RunTimestamps, WorkflowRef, + }; + use schemars::SchemaGenerator; + use serde_json::json; + + use super::*; + + #[test] + fn run_input_value_schema_allows_only_json_scalars() { + let mut generator = SchemaGenerator::default(); + let schema = RunInputValue::json_schema(&mut generator); + let schema = serde_json::to_value(schema).expect("schema should serialize"); + + assert_eq!( + schema["anyOf"], + json!([ + { "type": "string" }, + { "type": "boolean" }, + { "type": "integer" }, + { "type": "number" }, + ]) + ); + } + + #[test] + fn create_spec_accepts_parent_selector() { + let spec = ValidatedCreateRunSpec::try_from(CreateRunSpec { + workflow: "simple.fabro".to_string(), + cwd: None, + run_id: None, + parent_id: Some(" nightly-parent ".to_string()), + goal: None, + inputs: HashMap::new(), + labels: HashMap::new(), + dry_run: None, + auto_approve: None, + model: None, + provider: None, + sandbox: None, + preserve_sandbox: None, + start: None, + }) + .expect("parent selectors should validate without requiring exact run ids"); + + assert_eq!(spec.parent_id.as_deref(), Some("nightly-parent")); + } + + #[tokio::test] + async fn create_runs_resolves_parent_selector_and_sends_parent_id_to_backend() { + let temp = tempfile::tempdir().expect("tempdir should be created"); + let settings = temp.path().join("settings.toml"); + let child_id = run_id("01KRBZW5C00000000000000001"); + let parent_id = run_id("01KRBZW4DW0000000000000002"); + let backend = Arc::new(MockCreateBackend { + child_id, + parent_id, + created_parent_ids: Mutex::new(Vec::new()), + resolved_selectors: Mutex::new(Vec::new()), + }); + let params = ValidatedCreateRuns::try_from(FabroRunCreateParams { + runs: vec![CreateRunSpec { + workflow: "simple.fabro".to_string(), + cwd: None, + run_id: None, + parent_id: Some("nightly-parent".to_string()), + goal: None, + inputs: HashMap::new(), + labels: HashMap::new(), + dry_run: Some(true), + auto_approve: Some(true), + model: None, + provider: None, + sandbox: None, + preserve_sandbox: None, + start: Some(false), + }], + }) + .expect("create params should validate"); + + let result = create_runs(backend.clone(), temp.path(), &settings, params) + .await + .expect("run should be created"); + + assert_eq!(result.runs[0].parent_id, Some(parent_id.to_string())); + assert_eq!(result.runs[0].children_count, 0); + assert_eq!(backend.created_parent_ids.lock().unwrap().as_slice(), &[ + Some(parent_id) + ]); + assert_eq!(backend.resolved_selectors.lock().unwrap().as_slice(), &[ + "nightly-parent".to_string() + ]); + } + + #[tokio::test] + async fn create_runs_reuses_parent_selector_resolution_within_batch() { + let temp = tempfile::tempdir().expect("tempdir should be created"); + let settings = temp.path().join("settings.toml"); + let child_id = run_id("01KRBZW5C00000000000000001"); + let parent_id = run_id("01KRBZW4DW0000000000000002"); + let backend = Arc::new(MockCreateBackend { + child_id, + parent_id, + created_parent_ids: Mutex::new(Vec::new()), + resolved_selectors: Mutex::new(Vec::new()), + }); + let runs = (0..2) + .map(|_| CreateRunSpec { + workflow: "simple.fabro".to_string(), + cwd: None, + run_id: None, + parent_id: Some("nightly-parent".to_string()), + goal: None, + inputs: HashMap::new(), + labels: HashMap::new(), + dry_run: Some(true), + auto_approve: Some(true), + model: None, + provider: None, + sandbox: None, + preserve_sandbox: None, + start: Some(false), + }) + .collect(); + let params = ValidatedCreateRuns::try_from(FabroRunCreateParams { runs }) + .expect("create params should validate"); + + create_runs(backend.clone(), temp.path(), &settings, params) + .await + .expect("runs should be created"); + + assert_eq!(backend.created_parent_ids.lock().unwrap().as_slice(), &[ + Some(parent_id), + Some(parent_id), + ]); + assert_eq!(backend.resolved_selectors.lock().unwrap().as_slice(), &[ + "nightly-parent".to_string() + ]); + } + + #[tokio::test] + async fn create_runs_forced_parent_id_skips_selector_resolution() { + let temp = tempfile::tempdir().expect("tempdir should be created"); + let settings = temp.path().join("settings.toml"); + let child_id = run_id("01KRBZW5C00000000000000001"); + let parent_id = run_id("01KRBZW4DW0000000000000002"); + let backend = Arc::new(MockCreateBackend { + child_id, + parent_id, + created_parent_ids: Mutex::new(Vec::new()), + resolved_selectors: Mutex::new(Vec::new()), + }); + let params = ValidatedCreateRuns::try_from(FabroRunCreateParams { + runs: vec![CreateRunSpec { + workflow: "simple.fabro".to_string(), + cwd: None, + run_id: None, + parent_id: Some(parent_id.to_string()), + goal: None, + inputs: HashMap::new(), + labels: HashMap::new(), + dry_run: Some(true), + auto_approve: Some(true), + model: None, + provider: None, + sandbox: None, + preserve_sandbox: None, + start: Some(false), + }], + }) + .expect("create params should validate"); + + create_runs_with_options( + backend.clone(), + temp.path(), + &settings, + params, + CreateRunOptions { + forced_parent_id: Some(parent_id), + }, + ) + .await + .expect("run should be created"); + + assert_eq!(backend.created_parent_ids.lock().unwrap().as_slice(), &[ + Some(parent_id) + ]); + assert!(backend.resolved_selectors.lock().unwrap().is_empty()); + } + + fn run_id(raw: &str) -> RunId { + raw.parse().expect("test run id should parse") + } + + fn run(run_id: RunId, parent_id: Option, children_count: u64) -> Run { + Run { + id: run_id, + parent_id, + children_count, + title: "Test run".to_string(), + goal: "Test run".to_string(), + workflow: WorkflowRef { + slug: Some("simple".to_string()), + name: Some("Simple".to_string()), + graph_name: None, + node_count: 0, + edge_count: 0, + }, + automation: None, + repository: None, + created_by: None, + origin: RunOrigin::default(), + labels: HashMap::new(), + lifecycle: RunLifecycle { + status: RunStatus::Submitted, + pending_control: None, + queue_position: None, + error: None, + archived: false, + archived_at: None, + }, + sandbox: None, + models: Vec::new(), + source_directory: Some("/srv/repo".to_string()), + timestamps: RunTimestamps { + created_at: Utc.with_ymd_and_hms(2026, 4, 5, 12, 0, 0).unwrap(), + started_at: None, + last_event_at: None, + completed_at: None, + duration_ms: None, + elapsed_secs: None, + }, + billing: None, + diff: None, + pull_request: None, + current_question: None, + superseded_by: None, + links: RunLinks { web: None }, + } + } + + struct MockCreateBackend { + child_id: RunId, + parent_id: RunId, + created_parent_ids: Mutex>>, + resolved_selectors: Mutex>, + } + + #[async_trait] + impl FabroToolBackend for MockCreateBackend { + async fn create_run_from_spec( + &self, + _spec: &ValidatedCreateRunSpec, + _cwd: &Path, + _user_settings_path: &Path, + parent_id: Option, + ) -> anyhow::Result { + self.created_parent_ids.lock().unwrap().push(parent_id); + Ok(self.child_id) + } + + async fn resolve_run(&self, selector: &str) -> anyhow::Result { + assert_eq!(selector, "nightly-parent"); + self.resolved_selectors + .lock() + .unwrap() + .push(selector.to_string()); + Ok(run(self.parent_id, None, 1)) + } + + async fn retrieve_run(&self, run_id: &RunId) -> anyhow::Result { + assert_eq!(*run_id, self.child_id); + Ok(run(self.child_id, Some(self.parent_id), 0)) + } + + async fn start_run(&self, _run_id: &RunId, _resume: bool) -> anyhow::Result { + unreachable!("test uses start=false") + } + + async fn cancel_run(&self, _run_id: &RunId) -> anyhow::Result { + unreachable!() + } + + async fn interrupt_run(&self, _run_id: &RunId) -> anyhow::Result<()> { + unreachable!() + } + + async fn steer_run( + &self, + _run_id: &RunId, + _text: String, + _interrupt: bool, + ) -> anyhow::Result<()> { + unreachable!() + } + + async fn archive_run(&self, _run_id: &RunId) -> anyhow::Result { + unreachable!() + } + + async fn unarchive_run(&self, _run_id: &RunId) -> anyhow::Result { + unreachable!() + } + + async fn list_store_runs(&self) -> anyhow::Result> { + unreachable!() + } + + async fn list_store_runs_by_parent(&self, _parent_id: RunId) -> anyhow::Result> { + unreachable!() + } + + async fn link_run_parent( + &self, + _child_id: &RunId, + _parent_id: &RunId, + ) -> anyhow::Result { + unreachable!() + } + + async fn unlink_run_parent(&self, _child_id: &RunId) -> anyhow::Result { + unreachable!() + } + + async fn get_run_state(&self, _run_id: &RunId) -> anyhow::Result { + unreachable!() + } + + async fn list_run_events( + &self, + _run_id: &RunId, + _after: Option, + _limit: Option, + ) -> anyhow::Result> { + unreachable!() + } + + async fn list_run_events_until( + &self, + _run_id: &RunId, + _after: Option, + _limit: usize, + ) -> anyhow::Result> { + unreachable!() + } + + async fn list_run_questions( + &self, + _run_id: &RunId, + ) -> anyhow::Result> { + unreachable!() + } + + async fn submit_run_answer( + &self, + _run_id: &RunId, + _question_id: &str, + _body: types::SubmitAnswerRequest, + ) -> anyhow::Result<()> { + unreachable!() + } + } +} diff --git a/lib/crates/fabro-mcp-server/src/run_tools/events.rs b/lib/crates/fabro-tool/src/events.rs similarity index 83% rename from lib/crates/fabro-mcp-server/src/run_tools/events.rs rename to lib/crates/fabro-tool/src/events.rs index 42b7c44cc..130dc40ca 100644 --- a/lib/crates/fabro-mcp-server/src/run_tools/events.rs +++ b/lib/crates/fabro-tool/src/events.rs @@ -1,48 +1,47 @@ use std::sync::Arc; use chrono::{DateTime, Utc}; -use fabro_client::Client; use fabro_types::EventEnvelope; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_json::Value; use super::common; -use super::common::{ToolError, ToolResult}; +use super::common::{FabroToolBackend, ToolError, ToolResult}; #[derive(Debug, Clone, Copy, Deserialize, Serialize, JsonSchema)] #[serde(rename_all = "snake_case")] -pub(crate) enum RunEventsAction { +pub enum RunEventsAction { List, Details, Search, } #[derive(Debug, Deserialize, JsonSchema)] -pub(crate) struct FabroRunEventsParams { - pub(crate) action: RunEventsAction, - pub(crate) run_id: String, - pub(crate) event_types: Option>, - pub(crate) categories: Option>, - pub(crate) direction: Option, - pub(crate) created_after: Option, - pub(crate) created_before: Option, - pub(crate) first: Option, - pub(crate) after: Option, - pub(crate) event_ids: Option>, - pub(crate) offset: Option, - pub(crate) limit: Option, - pub(crate) max_content_length: Option, - pub(crate) query: Option, +pub struct FabroRunEventsParams { + pub action: RunEventsAction, + pub run_id: String, + pub event_types: Option>, + pub categories: Option>, + pub direction: Option, + pub created_after: Option, + pub created_before: Option, + pub first: Option, + pub after: Option, + pub event_ids: Option>, + pub offset: Option, + pub limit: Option, + pub max_content_length: Option, + pub query: Option, } #[derive(Debug)] -pub(crate) struct ValidatedRunEvents { - pub(crate) raw: FabroRunEventsParams, - pub(crate) descending: bool, - pub(crate) first: usize, - pub(crate) created_after: Option>, - pub(crate) created_before: Option>, +pub struct ValidatedRunEvents { + pub raw: FabroRunEventsParams, + pub descending: bool, + pub first: usize, + pub created_after: Option>, + pub created_before: Option>, } impl TryFrom for ValidatedRunEvents { @@ -97,23 +96,23 @@ impl TryFrom for ValidatedRunEvents { } #[derive(Debug, Serialize, JsonSchema)] -pub(crate) struct RunEventsResult { - pub(crate) run_id: String, - pub(crate) action: RunEventsAction, - pub(crate) events: Vec, - pub(crate) next_cursor: Option, +pub struct RunEventsResult { + pub run_id: String, + pub action: RunEventsAction, + pub events: Vec, + pub next_cursor: Option, } #[derive(Debug, Serialize, JsonSchema)] -pub(crate) struct RunEventResult { - pub(crate) event_id: String, - pub(crate) sequence: u32, - pub(crate) event: Value, - pub(crate) truncated: bool, +pub struct RunEventResult { + pub event_id: String, + pub sequence: u32, + pub event: Value, + pub truncated: bool, } -pub(crate) async fn run_events( - client: Arc, +pub async fn run_events( + backend: Arc, params: ValidatedRunEvents, ) -> ToolResult { let descending = params.descending; @@ -121,18 +120,18 @@ pub(crate) async fn run_events( let created_after = params.created_after; let created_before = params.created_before; let raw = params.raw; - let run_id = client + let run_id = backend .resolve_run(&raw.run_id) .await .map_err(|err| ToolError::from_anyhow(&err))? .id; let fetch_after = if descending { None } else { raw.after }; let mut events = if let Some(limit) = event_fetch_limit(&raw, first) { - client + backend .list_run_events_until(&run_id, fetch_after, limit) .await } else { - client.list_run_events(&run_id, fetch_after, None).await + backend.list_run_events(&run_id, fetch_after, None).await } .map_err(|err| ToolError::from_anyhow(&err))?; if descending { @@ -171,7 +170,7 @@ pub(crate) async fn run_events( }) } -pub(crate) fn run_events_text(result: &RunEventsResult) -> String { +pub fn run_events_text(result: &RunEventsResult) -> String { format!("returned {} Fabro event(s)", result.events.len()) } diff --git a/lib/crates/fabro-tool/src/fabro_client.rs b/lib/crates/fabro-tool/src/fabro_client.rs new file mode 100644 index 000000000..7c364de29 --- /dev/null +++ b/lib/crates/fabro-tool/src/fabro_client.rs @@ -0,0 +1,141 @@ +use std::path::Path; +use std::sync::Arc; + +use async_trait::async_trait; +use fabro_api::types; +use fabro_types::{EventEnvelope, Run, RunId, RunProjection}; + +use crate::{FabroToolBackend, RunManifestBuilder, ToolError}; + +#[derive(Clone)] +pub struct ClientBackend { + client: Arc<::fabro_client::Client>, + manifest_builder: Option>, +} + +impl ClientBackend { + #[must_use] + pub fn new(client: Arc<::fabro_client::Client>) -> Self { + Self { + client, + manifest_builder: None, + } + } + + #[must_use] + pub fn with_manifest_builder(mut self, builder: Arc) -> Self { + self.manifest_builder = Some(builder); + self + } +} + +#[async_trait] +impl FabroToolBackend for ClientBackend { + async fn create_run_from_spec( + &self, + spec: &crate::ValidatedCreateRunSpec, + cwd: &Path, + user_settings_path: &Path, + parent_id: Option, + ) -> anyhow::Result { + let Some(builder) = self.manifest_builder.as_ref() else { + return Err(ToolError::message(format!( + "{} is not available", + crate::FABRO_RUN_CREATE_TOOL_NAME + )) + .into()); + }; + let mut manifest = builder + .build_run_manifest(spec, cwd, user_settings_path) + .map_err(anyhow::Error::new)?; + manifest.parent_id = parent_id.map(|run_id| run_id.to_string()); + self.client.create_run_from_manifest(manifest).await + } + + async fn resolve_run(&self, selector: &str) -> anyhow::Result { + self.client.resolve_run(selector).await + } + + async fn retrieve_run(&self, run_id: &RunId) -> anyhow::Result { + self.client.retrieve_run(run_id).await + } + + async fn start_run(&self, run_id: &RunId, resume: bool) -> anyhow::Result { + self.client.start_run(run_id, resume).await + } + + async fn cancel_run(&self, run_id: &RunId) -> anyhow::Result { + self.client.cancel_run(run_id).await + } + + async fn interrupt_run(&self, run_id: &RunId) -> anyhow::Result<()> { + self.client.interrupt_run(run_id).await + } + + async fn steer_run(&self, run_id: &RunId, text: String, interrupt: bool) -> anyhow::Result<()> { + self.client.steer_run(run_id, text, interrupt).await + } + + async fn archive_run(&self, run_id: &RunId) -> anyhow::Result { + self.client.archive_run(run_id).await + } + + async fn unarchive_run(&self, run_id: &RunId) -> anyhow::Result { + self.client.unarchive_run(run_id).await + } + + async fn list_store_runs(&self) -> anyhow::Result> { + self.client.list_store_runs().await + } + + async fn list_store_runs_by_parent(&self, parent_id: RunId) -> anyhow::Result> { + self.client.list_store_runs_by_parent(parent_id).await + } + + async fn link_run_parent(&self, child_id: &RunId, parent_id: &RunId) -> anyhow::Result { + self.client.link_run_parent(child_id, parent_id).await + } + + async fn unlink_run_parent(&self, child_id: &RunId) -> anyhow::Result { + self.client.unlink_run_parent(child_id).await + } + + async fn get_run_state(&self, run_id: &RunId) -> anyhow::Result { + self.client.get_run_state(run_id).await + } + + async fn list_run_events( + &self, + run_id: &RunId, + after: Option, + limit: Option, + ) -> anyhow::Result> { + self.client.list_run_events(run_id, after, limit).await + } + + async fn list_run_events_until( + &self, + run_id: &RunId, + after: Option, + limit: usize, + ) -> anyhow::Result> { + self.client + .list_run_events_until(run_id, after, limit) + .await + } + + async fn list_run_questions(&self, run_id: &RunId) -> anyhow::Result> { + self.client.list_run_questions(run_id).await + } + + async fn submit_run_answer( + &self, + run_id: &RunId, + question_id: &str, + body: types::SubmitAnswerRequest, + ) -> anyhow::Result<()> { + self.client + .submit_run_answer(run_id, question_id, body) + .await + } +} diff --git a/lib/crates/fabro-mcp-server/src/run_tools/gather.rs b/lib/crates/fabro-tool/src/gather.rs similarity index 74% rename from lib/crates/fabro-mcp-server/src/run_tools/gather.rs rename to lib/crates/fabro-tool/src/gather.rs index 48e59b33f..4c9dc68fb 100644 --- a/lib/crates/fabro-mcp-server/src/run_tools/gather.rs +++ b/lib/crates/fabro-tool/src/gather.rs @@ -1,27 +1,26 @@ use std::sync::Arc; use std::time::{Duration, Instant}; -use fabro_client::Client; use futures::future::try_join_all; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use tokio::time; use super::common; -use super::common::{RunSummaryResult, ToolError, ToolResult}; +use super::common::{FabroToolBackend, RunSummaryResult, ToolError, ToolResult}; #[derive(Debug, Deserialize, JsonSchema)] -pub(crate) struct FabroRunGatherParams { - pub(crate) run_ids: Vec, - pub(crate) timeout_seconds: Option, - pub(crate) poll_interval_seconds: Option, +pub struct FabroRunGatherParams { + pub run_ids: Vec, + pub timeout_seconds: Option, + pub poll_interval_seconds: Option, } #[derive(Debug)] -pub(crate) struct ValidatedGatherRuns { - pub(crate) run_ids: Vec, - pub(crate) timeout_seconds: u64, - pub(crate) poll_interval_seconds: u64, +pub struct ValidatedGatherRuns { + pub run_ids: Vec, + pub timeout_seconds: u64, + pub poll_interval_seconds: u64, } impl TryFrom for ValidatedGatherRuns { @@ -47,22 +46,22 @@ impl TryFrom for ValidatedGatherRuns { } #[derive(Debug, Serialize, JsonSchema)] -pub(crate) struct GatherRunsResult { - pub(crate) runs: Vec, - pub(crate) timed_out: bool, - pub(crate) elapsed_seconds: u64, +pub struct GatherRunsResult { + pub runs: Vec, + pub timed_out: bool, + pub elapsed_seconds: u64, } -pub(crate) async fn gather_runs( - client: Arc, +pub async fn gather_runs( + backend: Arc, params: ValidatedGatherRuns, ) -> ToolResult { let start = Instant::now(); let deadline = start + Duration::from_secs(params.timeout_seconds); let run_ids = try_join_all(params.run_ids.into_iter().map(|selector| { - let client = Arc::clone(&client); + let backend = Arc::clone(&backend); async move { - client + backend .resolve_run(&selector) .await .map(|run| run.id) @@ -73,8 +72,8 @@ pub(crate) async fn gather_runs( loop { let summaries = try_join_all(run_ids.iter().map(|run_id| { - let client = Arc::clone(&client); - async move { common::retrieve_run(&client, run_id).await } + let backend = Arc::clone(&backend); + async move { common::retrieve_run(backend.as_ref(), run_id).await } })) .await?; if summaries @@ -100,7 +99,7 @@ pub(crate) async fn gather_runs( } } -pub(crate) fn gather_runs_text(result: &GatherRunsResult) -> String { +pub fn gather_runs_text(result: &GatherRunsResult) -> String { format!( "gathered {} Fabro run(s), timed_out={}", result.runs.len(), diff --git a/lib/crates/fabro-mcp-server/src/run_tools/interact.rs b/lib/crates/fabro-tool/src/interact.rs similarity index 91% rename from lib/crates/fabro-mcp-server/src/run_tools/interact.rs rename to lib/crates/fabro-tool/src/interact.rs index 920b49ff3..79606ac8d 100644 --- a/lib/crates/fabro-mcp-server/src/run_tools/interact.rs +++ b/lib/crates/fabro-tool/src/interact.rs @@ -2,18 +2,17 @@ use std::borrow::Cow; use std::sync::Arc; use fabro_api::types; -use fabro_client::Client; use fabro_types::RunId; use schemars::{JsonSchema, Schema, SchemaGenerator, json_schema}; use serde::{Deserialize, Serialize}; use serde_json::{Value, json}; use super::common; -use super::common::{ToolError, ToolResult}; +use super::common::{FabroToolBackend, ToolError, ToolResult}; #[derive(Debug, Clone, Copy, Deserialize, Serialize, JsonSchema)] #[serde(rename_all = "snake_case")] -pub(crate) enum RunInteractAction { +pub enum RunInteractAction { Get, Start, Message, @@ -32,19 +31,19 @@ pub(crate) enum RunInteractAction { } #[derive(Debug, Deserialize, JsonSchema)] -pub(crate) struct FabroRunInteractParams { - pub(crate) action: RunInteractAction, - pub(crate) run_id: String, - pub(crate) parent_id: Option, - pub(crate) message: Option, - pub(crate) interrupt: Option, - pub(crate) question_id: Option, - pub(crate) answer: Option, +pub struct FabroRunInteractParams { + pub action: RunInteractAction, + pub run_id: String, + pub parent_id: Option, + pub message: Option, + pub interrupt: Option, + pub question_id: Option, + pub answer: Option, } #[derive(Debug, Deserialize)] #[serde(transparent)] -pub(crate) struct AnswerValue(Value); +pub struct AnswerValue(Value); impl From for AnswerValue { fn from(value: Value) -> Self { @@ -53,7 +52,7 @@ impl From for AnswerValue { } impl AnswerValue { - fn into_inner(self) -> Value { + pub(crate) fn into_inner(self) -> Value { self.0 } } @@ -106,13 +105,13 @@ impl JsonSchema for AnswerValue { } #[derive(Debug)] -pub(crate) struct ValidatedInteractRun { - pub(crate) run_id: String, - pub(crate) action: ValidatedInteractAction, +pub struct ValidatedInteractRun { + pub run_id: String, + pub action: ValidatedInteractAction, } #[derive(Debug)] -pub(crate) enum ValidatedInteractAction { +pub enum ValidatedInteractAction { Get, Start, Message { @@ -225,94 +224,94 @@ impl TryFrom for ValidatedInteractRun { } #[derive(Debug, Serialize, JsonSchema)] -pub(crate) struct InteractRunResult { - pub(crate) run_id: String, - pub(crate) action: RunInteractAction, - pub(crate) result: Value, +pub struct InteractRunResult { + pub run_id: String, + pub action: RunInteractAction, + pub result: Value, } -pub(crate) async fn interact_run( - client: Arc, +pub async fn interact_run( + backend: Arc, params: ValidatedInteractRun, ) -> ToolResult { - let run_id = client + let run_id = backend .resolve_run(¶ms.run_id) .await .map_err(|err| ToolError::from_anyhow(&err))? .id; let action = params.action.action(); let result = match params.action { - ValidatedInteractAction::Get => interact_get(&client, &run_id).await?, + ValidatedInteractAction::Get => interact_get(backend.as_ref(), &run_id).await?, ValidatedInteractAction::Start => { - let summary = client + let summary = backend .start_run(&run_id, false) .await .map_err(|err| ToolError::from_anyhow(&err))?; json!({ "summary": common::run_summary_result(&summary) }) } ValidatedInteractAction::Message { message, interrupt } => { - client + backend .steer_run(&run_id, message.clone(), interrupt) .await .map_err(|err| ToolError::from_anyhow(&err))?; json!({ "message": message, "interrupt": interrupt }) } ValidatedInteractAction::Interrupt => { - client + backend .interrupt_run(&run_id) .await .map_err(|err| ToolError::from_anyhow(&err))?; json!({ "interrupted": true }) } ValidatedInteractAction::Cancel => { - let summary = client + let summary = backend .cancel_run(&run_id) .await .map_err(|err| ToolError::from_anyhow(&err))?; json!({ "summary": common::run_summary_result(&summary) }) } ValidatedInteractAction::Archive => { - let summary = client + let summary = backend .archive_run(&run_id) .await .map_err(|err| ToolError::from_anyhow(&err))?; json!({ "summary": common::run_summary_result(&summary) }) } ValidatedInteractAction::Unarchive => { - let summary = client + let summary = backend .unarchive_run(&run_id) .await .map_err(|err| ToolError::from_anyhow(&err))?; json!({ "summary": common::run_summary_result(&summary) }) } ValidatedInteractAction::LinkParent { parent_id } => { - let parent_id = client + let parent_id = backend .resolve_run(&parent_id) .await .map_err(|err| ToolError::from_anyhow(&err))? .id; - let summary = client + let summary = backend .link_run_parent(&run_id, &parent_id) .await .map_err(|err| ToolError::from_anyhow(&err))?; json!({ "summary": common::run_summary_result(&summary) }) } ValidatedInteractAction::UnlinkParent => { - let summary = client + let summary = backend .unlink_run_parent(&run_id) .await .map_err(|err| ToolError::from_anyhow(&err))?; json!({ "summary": common::run_summary_result(&summary) }) } ValidatedInteractAction::GetQuestions => { - let questions = client + let questions = backend .list_run_questions(&run_id) .await .map_err(|err| ToolError::from_anyhow(&err))?; json!({ "questions": questions }) } ValidatedInteractAction::Answer { question_id, body } => { - client + backend .submit_run_answer(&run_id, &question_id, body) .await .map_err(|err| ToolError::from_anyhow(&err))?; @@ -327,16 +326,16 @@ pub(crate) async fn interact_run( }) } -pub(crate) fn interact_run_text(result: &InteractRunResult) -> String { +pub fn interact_run_text(result: &InteractRunResult) -> String { format!( "completed {:?} for Fabro run {}", result.action, result.run_id ) } -async fn interact_get(client: &Client, run_id: &RunId) -> ToolResult { - let summary = common::retrieve_run(client, run_id).await?; - let projection = client +async fn interact_get(backend: &dyn FabroToolBackend, run_id: &RunId) -> ToolResult { + let summary = common::retrieve_run(backend, run_id).await?; + let projection = backend .get_run_state(run_id) .await .map_err(|err| ToolError::from_anyhow(&err))?; diff --git a/lib/crates/fabro-tool/src/lib.rs b/lib/crates/fabro-tool/src/lib.rs new file mode 100644 index 000000000..02b71389c --- /dev/null +++ b/lib/crates/fabro-tool/src/lib.rs @@ -0,0 +1,41 @@ +#![allow( + dead_code, + reason = "Tool DTO fields are consumed by serde and schema generation even when not read \ + directly." +)] + +mod common; +mod create; +mod events; +pub mod fabro_client; +mod gather; +mod interact; +mod manifest; +mod search; + +pub use common::{ + FABRO_RUN_CREATE_TOOL_NAME, FABRO_RUN_EVENTS_TOOL_NAME, FABRO_RUN_GATHER_TOOL_NAME, + FABRO_RUN_INTERACT_TOOL_NAME, FABRO_RUN_SEARCH_TOOL_NAME, FabroToolBackend, RunManifestBuilder, + RunSummaryResult, ToolDefinition, ToolError, ToolResult, tool_definitions, +}; +pub use create::{ + CreateRunOptions, CreateRunSpec, CreateRunsResult, CreatedRunResult, FabroRunCreateParams, + RunInputValue, ValidatedCreateRunSpec, ValidatedCreateRuns, create_runs, create_runs_text, + create_runs_with_options, +}; +pub use events::{ + FabroRunEventsParams, RunEventResult, RunEventsAction, RunEventsResult, ValidatedRunEvents, + run_events, run_events_text, +}; +pub use gather::{ + FabroRunGatherParams, GatherRunsResult, ValidatedGatherRuns, gather_runs, gather_runs_text, +}; +pub use interact::{ + AnswerValue, FabroRunInteractParams, InteractRunResult, RunInteractAction, + ValidatedInteractAction, ValidatedInteractRun, interact_run, interact_run_text, +}; +pub use manifest::json_to_toml_value; +pub use search::{ + FabroRunSearchParams, SearchRunSummaryResult, SearchRunsResult, ValidatedSearchRuns, + search_runs, search_runs_text, +}; diff --git a/lib/crates/fabro-tool/src/manifest.rs b/lib/crates/fabro-tool/src/manifest.rs new file mode 100644 index 000000000..01473602c --- /dev/null +++ b/lib/crates/fabro-tool/src/manifest.rs @@ -0,0 +1,76 @@ +use serde_json::Value; + +use super::common::{ToolError, ToolResult}; + +pub fn json_to_toml_value(key: &str, value: &Value) -> ToolResult { + match value { + Value::Null => Err(ToolError::message(format!( + "input `{key}` cannot be null; use a string, boolean, or number" + ))), + Value::Bool(value) => Ok(toml::Value::Boolean(*value)), + Value::Number(value) => { + if let Some(integer) = value.as_i64() { + Ok(toml::Value::Integer(integer)) + } else if let Some(float) = value.as_f64() { + Ok(toml::Value::Float(float)) + } else { + Err(ToolError::message(format!( + "input `{key}` contains a number outside TOML's supported range" + ))) + } + } + Value::String(value) => Ok(toml::Value::String(value.clone())), + Value::Array(_) => Err(ToolError::message(format!( + "input `{key}` does not support array values; use a string, boolean, or number", + ))), + Value::Object(_) => Err(ToolError::message(format!( + "input `{key}` does not support object values; use a string, boolean, or number", + ))), + } +} + +#[cfg(test)] +mod tests { + use serde_json::{Value, json}; + + use super::*; + + #[test] + fn json_inputs_convert_scalar_values_to_toml_values() { + let cases = [ + (json!("hello"), toml::Value::String("hello".to_string())), + (json!(true), toml::Value::Boolean(true)), + (json!(42), toml::Value::Integer(42)), + (json!(0.5), toml::Value::Float(0.5)), + ]; + + for (json, expected) in cases { + assert_eq!(json_to_toml_value("input", &json).unwrap(), expected); + } + } + + #[test] + fn json_input_arrays_and_objects_are_rejected() { + let array_err = json_to_toml_value("matrix", &json!(["a", 1])).unwrap_err(); + assert_eq!( + array_err.as_str(), + "input `matrix` does not support array values; use a string, boolean, or number", + ); + + let object_err = json_to_toml_value("settings", &json!({ "enabled": true })).unwrap_err(); + assert_eq!( + object_err.as_str(), + "input `settings` does not support object values; use a string, boolean, or number", + ); + } + + #[test] + fn json_input_null_is_rejected_with_key_name() { + let err = json_to_toml_value("goal", &Value::Null).unwrap_err(); + + assert_eq!( + err.as_str(), + "input `goal` cannot be null; use a string, boolean, or number", + ); + } +} diff --git a/lib/crates/fabro-mcp-server/src/run_tools/search.rs b/lib/crates/fabro-tool/src/search.rs similarity index 87% rename from lib/crates/fabro-mcp-server/src/run_tools/search.rs rename to lib/crates/fabro-tool/src/search.rs index 377581be1..ee0d72064 100644 --- a/lib/crates/fabro-mcp-server/src/run_tools/search.rs +++ b/lib/crates/fabro-tool/src/search.rs @@ -1,35 +1,34 @@ use std::collections::HashMap; use std::sync::Arc; -use fabro_client::Client; use fabro_types::{Run, RunId, RunStatusKind}; use futures::future::try_join_all; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use super::common; -use super::common::{RunSummaryResult, ToolError, ToolResult}; +use super::common::{FabroToolBackend, RunSummaryResult, ToolError, ToolResult}; const SEARCH_GOAL_PREVIEW_CHARS: usize = 240; #[derive(Debug, Deserialize, JsonSchema)] -pub(crate) struct FabroRunSearchParams { - pub(crate) run_ids: Option>, - pub(crate) workflow: Option, - pub(crate) labels: Option>, - pub(crate) status: Option>, - pub(crate) archived: Option, - pub(crate) created_after: Option, - pub(crate) created_before: Option, - pub(crate) first: Option, - pub(crate) after: Option, - pub(crate) parent_id: Option, +pub struct FabroRunSearchParams { + pub run_ids: Option>, + pub workflow: Option, + pub labels: Option>, + pub status: Option>, + pub archived: Option, + pub created_after: Option, + pub created_before: Option, + pub first: Option, + pub after: Option, + pub parent_id: Option, } #[derive(Debug)] -pub(crate) struct ValidatedSearchRuns { - pub(crate) raw: FabroRunSearchParams, - pub(crate) status: Option>, +pub struct ValidatedSearchRuns { + pub raw: FabroRunSearchParams, + pub status: Option>, } impl TryFrom for ValidatedSearchRuns { @@ -77,40 +76,40 @@ impl TryFrom for ValidatedSearchRuns { } #[derive(Debug, Serialize, JsonSchema)] -pub(crate) struct SearchRunsResult { - pub(crate) runs: Vec, - pub(crate) next_cursor: Option, +pub struct SearchRunsResult { + pub runs: Vec, + pub next_cursor: Option, } #[derive(Debug, Serialize, JsonSchema)] -pub(crate) struct SearchRunSummaryResult { - pub(crate) run_id: String, - pub(crate) parent_id: Option, - pub(crate) children_count: u64, - pub(crate) workflow_name: Option, - pub(crate) workflow_graph_name: Option, - pub(crate) workflow_slug: Option, - pub(crate) status: String, - pub(crate) archived: bool, - pub(crate) created_at: String, - pub(crate) started_at: Option, - pub(crate) completed_at: Option, - pub(crate) labels: HashMap, - pub(crate) source_directory: Option, - pub(crate) repo_origin_url: Option, - pub(crate) goal_preview: String, - pub(crate) goal_truncated: bool, +pub struct SearchRunSummaryResult { + pub run_id: String, + pub parent_id: Option, + pub children_count: u64, + pub workflow_name: Option, + pub workflow_graph_name: Option, + pub workflow_slug: Option, + pub status: String, + pub archived: bool, + pub created_at: String, + pub started_at: Option, + pub completed_at: Option, + pub labels: HashMap, + pub source_directory: Option, + pub repo_origin_url: Option, + pub goal_preview: String, + pub goal_truncated: bool, } -pub(crate) async fn search_runs( - client: Arc, +pub async fn search_runs( + backend: Arc, params: ValidatedSearchRuns, ) -> ToolResult { let status = params.status; let raw = params.raw; let parent_id = if let Some(parent_selector) = raw.parent_id.as_deref() { Some( - client + backend .resolve_run(parent_selector) .await .map_err(|err| ToolError::from_anyhow(&err))? @@ -120,14 +119,14 @@ pub(crate) async fn search_runs( None }; let runs = if let Some(run_ids) = raw.run_ids.as_ref() { - resolve_requested_runs(&client, run_ids).await? + resolve_requested_runs(&backend, run_ids).await? } else if let Some(parent_id) = parent_id { - client + backend .list_store_runs_by_parent(parent_id) .await .map_err(|err| ToolError::from_anyhow(&err))? } else { - client + backend .list_store_runs() .await .map_err(|err| ToolError::from_anyhow(&err))? @@ -263,15 +262,18 @@ fn filter_sort_and_page_runs( }) } -pub(crate) fn search_runs_text(result: &SearchRunsResult) -> String { +pub fn search_runs_text(result: &SearchRunsResult) -> String { format!("found {} Fabro run(s)", result.runs.len()) } -async fn resolve_requested_runs(client: &Arc, run_ids: &[String]) -> ToolResult> { +async fn resolve_requested_runs( + backend: &Arc, + run_ids: &[String], +) -> ToolResult> { let runs = try_join_all(run_ids.iter().map(|run_id| { - let client = Arc::clone(client); + let backend = Arc::clone(backend); async move { - client + backend .resolve_run(run_id) .await .map_err(|err| ToolError::from_anyhow(&err)) diff --git a/lib/crates/fabro-workflow/Cargo.toml b/lib/crates/fabro-workflow/Cargo.toml index 716bf6660..d5025f1bd 100644 --- a/lib/crates/fabro-workflow/Cargo.toml +++ b/lib/crates/fabro-workflow/Cargo.toml @@ -32,6 +32,7 @@ fabro-mcp = { path = "../fabro-mcp" } fabro-github = { path = "../fabro-github" } fabro-interview = { path = "../fabro-interview" } fabro-template = { path = "../fabro-template" } +fabro-tool = { path = "../fabro-tool" } fabro-util = { path = "../fabro-util" } fabro-redact.workspace = true fabro-checkpoint = { path = "../fabro-checkpoint" } @@ -73,6 +74,7 @@ fabro-vault = { path = "../fabro-vault" } [dev-dependencies] base64.workspace = true fabro-acp = { path = "../fabro-acp", features = ["test-support"] } +fabro-api = { path = "../fabro-api" } fabro-sandbox = { path = "../fabro-sandbox", features = ["daytona", "docker", "test-support"] } fabro-mcp = { path = "../fabro-mcp" } tokio = { workspace = true, features = ["test-util", "macros"] } diff --git a/lib/crates/fabro-workflow/src/handler/llm/api.rs b/lib/crates/fabro-workflow/src/handler/llm/api.rs index ebea22546..01bc9fe8a 100644 --- a/lib/crates/fabro-workflow/src/handler/llm/api.rs +++ b/lib/crates/fabro-workflow/src/handler/llm/api.rs @@ -3,6 +3,7 @@ use std::sync::{Arc, Mutex}; use async_trait::async_trait; use fabro_agent::subagent::{SessionFactory, SubAgentManager}; +use fabro_agent::tool_registry::{RegisteredTool, ToolContext, ToolRegistry}; use fabro_agent::{ AgentEvent, AgentProfile, AnthropicProfile, CompletionCoordinator, GeminiProfile, Message as AgentMessage, OpenAiProfile, Sandbox, Session, SessionOptions, StaticEnvProvider, @@ -11,13 +12,16 @@ use fabro_agent::{ use fabro_auth::{CredentialSource, EnvCredentialSource}; use fabro_graphviz::graph::{AttrValue, Node}; use fabro_llm::client::Client; -use fabro_llm::types::{Message, ReasoningEffort, Request, Speed, TokenCounts}; +use fabro_llm::types::{ + Message, ReasoningEffort, Request, Speed, TokenCounts, ToolDefinition as LlmToolDefinition, +}; use fabro_mcp::config::McpServerSettings; #[cfg(test)] use fabro_model::catalog::LlmCatalogSettings; use fabro_model::{AgentProfileKind, Catalog, FallbackTarget, ModelRef, ProviderId}; use fabro_types::settings::run::RunModelControls; -use fabro_types::{SessionCapability, StageId}; +use fabro_types::{RunId, SessionCapability, StageId}; +use serde::de::DeserializeOwned; use tokio::sync::Mutex as TokioMutex; use tokio::task::JoinHandle; use tokio_util::sync::CancellationToken; @@ -31,6 +35,7 @@ use crate::context::keys::Fidelity; use crate::error::Error; use crate::event::{Emitter, Event, StageScope}; use crate::outcome::billed_model_usage_from_llm; +use crate::services::FabroRunToolServices; use crate::steering_hub::{ActiveControlHandle, SteeringHub}; /// Spawn a task that, when the run-level token cancels, sets the agent @@ -186,6 +191,149 @@ fn build_profile( } } +pub(crate) fn register_fabro_run_tools( + registry: &mut ToolRegistry, + services: &FabroRunToolServices, +) { + for definition in fabro_tool::tool_definitions() { + registry.register(fabro_run_tool(definition, services.clone())); + } +} + +fn fabro_run_tool( + definition: &fabro_tool::ToolDefinition, + services: FabroRunToolServices, +) -> RegisteredTool { + let name = definition.name.to_string(); + RegisteredTool { + definition: LlmToolDefinition { + name: name.clone(), + description: definition.description.to_string(), + parameters: definition.parameters.clone(), + }, + executor: Arc::new(move |args, _context: ToolContext| { + let name = name.clone(); + let services = services.clone(); + Box::pin(async move { + execute_fabro_run_tool(&name, args, services) + .await + .map_err(|err| err.to_string()) + }) + }), + } +} + +async fn execute_fabro_run_tool( + name: &str, + args: serde_json::Value, + services: FabroRunToolServices, +) -> fabro_tool::ToolResult { + match name { + fabro_tool::FABRO_RUN_CREATE_TOOL_NAME => { + let params = parse_fabro_tool_args::(name, args)?; + ensure_current_run_parent(¶ms, services.current_run_id)?; + let validated = fabro_tool::ValidatedCreateRuns::try_from(params)?; + let result = fabro_tool::create_runs_with_options( + Arc::clone(&services.backend), + &services.base_cwd, + &services.user_settings_path, + validated, + fabro_tool::CreateRunOptions { + forced_parent_id: Some(services.current_run_id), + }, + ) + .await?; + let summary = fabro_tool::create_runs_text(&result); + render_fabro_tool_result(&summary, &result) + } + fabro_tool::FABRO_RUN_SEARCH_TOOL_NAME => { + let params = parse_fabro_tool_args::(name, args)?; + let result = fabro_tool::search_runs( + Arc::clone(&services.backend), + fabro_tool::ValidatedSearchRuns::try_from(params)?, + ) + .await?; + let summary = fabro_tool::search_runs_text(&result); + render_fabro_tool_result(&summary, &result) + } + fabro_tool::FABRO_RUN_INTERACT_TOOL_NAME => { + let params = parse_fabro_tool_args::(name, args)?; + let result = fabro_tool::interact_run( + Arc::clone(&services.backend), + fabro_tool::ValidatedInteractRun::try_from(params)?, + ) + .await?; + let summary = fabro_tool::interact_run_text(&result); + render_fabro_tool_result(&summary, &result) + } + fabro_tool::FABRO_RUN_GATHER_TOOL_NAME => { + let params = parse_fabro_tool_args::(name, args)?; + let result = fabro_tool::gather_runs( + Arc::clone(&services.backend), + fabro_tool::ValidatedGatherRuns::try_from(params)?, + ) + .await?; + let summary = fabro_tool::gather_runs_text(&result); + render_fabro_tool_result(&summary, &result) + } + fabro_tool::FABRO_RUN_EVENTS_TOOL_NAME => { + let params = parse_fabro_tool_args::(name, args)?; + let result = fabro_tool::run_events( + Arc::clone(&services.backend), + fabro_tool::ValidatedRunEvents::try_from(params)?, + ) + .await?; + let summary = fabro_tool::run_events_text(&result); + render_fabro_tool_result(&summary, &result) + } + _ => Err(fabro_tool::ToolError::message(format!( + "unknown Fabro run tool `{name}`" + ))), + } +} + +fn parse_fabro_tool_args(name: &str, args: serde_json::Value) -> fabro_tool::ToolResult +where + T: DeserializeOwned, +{ + serde_json::from_value(args) + .map_err(|err| fabro_tool::ToolError::message(format!("invalid {name} arguments: {err}"))) +} + +fn ensure_current_run_parent( + params: &fabro_tool::FabroRunCreateParams, + current_run_id: RunId, +) -> fabro_tool::ToolResult<()> { + let current_parent = current_run_id.to_string(); + for run in ¶ms.runs { + match run.parent_id.as_deref().map(str::trim) { + None => {} + Some("") => { + return Err(fabro_tool::ToolError::message( + "parent_id must be omitted or match the current run; blank parent_id is invalid", + )); + } + Some(parent_id) if parent_id == current_parent => {} + Some(parent_id) => { + return Err(fabro_tool::ToolError::message(format!( + "parent_id must be omitted or match the current run {current_parent}; got {parent_id}" + ))); + } + } + } + Ok(()) +} + +fn render_fabro_tool_result(summary: &str, result: &T) -> fabro_tool::ToolResult +where + T: serde::Serialize, +{ + let json = serde_json::to_string_pretty(result).map_err(|err| { + fabro_tool::ToolError::message(format!("failed to serialize tool result: {err}")) + })?; + Ok(format!("{summary}\n{json}")) +} + pub(super) fn effective_request_controls( run_model_controls: &RunModelControls, node: &Node, @@ -342,6 +490,7 @@ pub struct AgentApiBackend { source: Arc, steering_hub: Arc, catalog: Arc, + fabro_run_tools: Option, } impl AgentApiBackend { @@ -384,6 +533,7 @@ impl AgentApiBackend { source, steering_hub, catalog, + fabro_run_tools: None, } } @@ -427,6 +577,12 @@ impl AgentApiBackend { self } + #[must_use] + pub fn with_fabro_run_tools(mut self, services: FabroRunToolServices) -> Self { + self.fabro_run_tools = Some(services); + self + } + fn effective_request_controls(&self, node: &Node) -> Result { effective_request_controls(&self.run_model_controls, node) } @@ -468,6 +624,7 @@ impl AgentApiBackend { self.tool_env.as_ref(), tool_hooks, self.mcp_servers.clone(), + self.fabro_run_tools.clone(), ) .await } @@ -483,6 +640,7 @@ impl AgentApiBackend { tool_env: Option<&Arc>, tool_hooks: Option>, mcp_servers: Vec, + fabro_run_tools: Option, ) -> Result { let controls = effective_request_controls(run_model_controls, node)?; let client = Client::from_source(source, Arc::clone(&catalog)) @@ -517,13 +675,18 @@ impl AgentApiBackend { let factory_catalog = Arc::clone(&catalog); let factory_env = Arc::clone(sandbox); let factory_tool_env = tool_env.cloned(); + let factory_fabro_run_tools = fabro_run_tools.clone(); let factory: SessionFactory = Arc::new(move || { - let child_profile: Arc = Arc::from(build_profile( + let mut child_profile = build_profile( &factory_model, factory_provider.provider_id.clone(), factory_provider.profile_kind, Arc::clone(&factory_catalog), - )); + ); + if let Some(services) = factory_fabro_run_tools.clone() { + register_fabro_run_tools(child_profile.tool_registry_mut(), &services); + } + let child_profile: Arc = Arc::from(child_profile); let mut session = Session::new( factory_client.clone(), child_profile, @@ -542,6 +705,9 @@ impl AgentApiBackend { }); profile.register_subagent_tools(manager, factory, 0); + if let Some(services) = fabro_run_tools { + register_fabro_run_tools(profile.tool_registry_mut(), &services); + } let profile: Arc = Arc::from(profile); let mut session = Session::new( @@ -950,6 +1116,7 @@ impl CodergenBackend for AgentApiBackend { self.tool_env.as_ref(), tool_hooks.clone(), self.mcp_servers.clone(), + self.fabro_run_tools.clone(), ) .await; if cancel_token.is_cancelled() { @@ -1150,16 +1317,27 @@ impl CompletionCoordinator for SteeringCompletionCoordinator { #[cfg(test)] mod tests { + use std::path::{Path, PathBuf}; + + use chrono::TimeZone; use fabro_agent::subagent::SessionFactory; - use fabro_agent::{AgentProfile, ToolRegistry}; + use fabro_agent::{AgentProfile, LocalSandbox, ToolRegistry}; + use fabro_api::types; use fabro_auth::{EnvCredentialSource, VaultCredentialSource}; use fabro_llm::provider::{ProviderAdapter, StreamEventStream}; use fabro_llm::{Error as LlmError, ProviderErrorDetail, ProviderErrorKind}; + use fabro_tool::FabroToolBackend; + use fabro_types::{ + EventEnvelope, Run, RunId, RunLifecycle, RunLinks, RunOrigin, RunProjection, RunStatus, + RunTimestamps, SuccessReason, WorkflowRef, + }; use fabro_vault::{SecretType, Vault}; use futures::stream; use tokio::sync::RwLock as AsyncRwLock; + use tokio_util::sync::CancellationToken; use super::*; + use crate::services::FabroRunToolServices; struct ShutdownTestProfile { registry: ToolRegistry, @@ -1249,6 +1427,342 @@ mod tests { assert!(backend.sessions.lock().unwrap().is_empty()); } + #[test] + fn agent_run_tools_register_exact_shared_definitions() { + let mut registry = ToolRegistry::new(); + let (services, _backend) = fabro_run_tool_services(); + register_fabro_run_tools(&mut registry, &services); + + let mut registered = registry + .names() + .into_iter() + .filter(|name| name.starts_with("fabro_run_")) + .collect::>(); + registered.sort(); + assert_eq!(registered, vec![ + fabro_tool::FABRO_RUN_CREATE_TOOL_NAME, + fabro_tool::FABRO_RUN_EVENTS_TOOL_NAME, + fabro_tool::FABRO_RUN_GATHER_TOOL_NAME, + fabro_tool::FABRO_RUN_INTERACT_TOOL_NAME, + fabro_tool::FABRO_RUN_SEARCH_TOOL_NAME, + ]); + + for definition in fabro_tool::tool_definitions() { + let registered = registry + .get(definition.name) + .expect("shared Fabro run tool should be registered"); + assert_eq!(registered.definition.description, definition.description); + assert_eq!(registered.definition.parameters, definition.parameters); + } + } + + #[tokio::test] + async fn agent_run_create_injects_current_run_as_parent() { + let (services, backend) = fabro_run_tool_services(); + let mut registry = ToolRegistry::new(); + register_fabro_run_tools(&mut registry, &services); + let tool = registry + .get(fabro_tool::FABRO_RUN_CREATE_TOOL_NAME) + .expect("create tool should be registered"); + + let output = (tool.executor)( + serde_json::json!({ + "runs": [{ + "workflow": "child.fabro", + "start": false + }] + }), + tool_context(), + ) + .await + .expect("create tool should succeed"); + + assert!(output.contains("created 1 Fabro run(s)")); + assert_eq!(backend.created_parent_ids.lock().unwrap().as_slice(), &[ + Some(current_run_id()) + ]); + } + + #[tokio::test] + async fn agent_run_create_rejects_conflicting_parent_id() { + let mut registry = ToolRegistry::new(); + let (services, _backend) = fabro_run_tool_services(); + register_fabro_run_tools(&mut registry, &services); + let tool = registry + .get(fabro_tool::FABRO_RUN_CREATE_TOOL_NAME) + .expect("create tool should be registered"); + + let err = (tool.executor)( + serde_json::json!({ + "runs": [{ + "workflow": "child.fabro", + "parent_id": "01KRBZW4DW0000000000000002", + "start": false + }] + }), + tool_context(), + ) + .await + .expect_err("conflicting parent should be rejected"); + + assert!(err.contains("parent_id")); + assert!(err.contains("current run")); + } + + #[tokio::test] + async fn agent_run_tools_share_create_gather_and_events_backend() { + let (services, backend) = fabro_run_tool_services(); + let mut registry = ToolRegistry::new(); + register_fabro_run_tools(&mut registry, &services); + + let create = registry + .get(fabro_tool::FABRO_RUN_CREATE_TOOL_NAME) + .unwrap(); + (create.executor)( + serde_json::json!({ + "runs": [{ + "workflow": "child.fabro", + "start": false + }] + }), + tool_context(), + ) + .await + .expect("create should succeed"); + + let gather = registry + .get(fabro_tool::FABRO_RUN_GATHER_TOOL_NAME) + .unwrap(); + let gathered = (gather.executor)( + serde_json::json!({ + "run_ids": [child_run_id().to_string()], + "timeout_seconds": 0 + }), + tool_context(), + ) + .await + .expect("gather should succeed"); + + let events = registry + .get(fabro_tool::FABRO_RUN_EVENTS_TOOL_NAME) + .unwrap(); + let listed = (events.executor)( + serde_json::json!({ + "action": "list", + "run_id": child_run_id().to_string(), + "first": 5 + }), + tool_context(), + ) + .await + .expect("events should succeed"); + + assert!(gathered.contains("gathered 1 Fabro run(s)")); + assert!(listed.contains("returned 0 Fabro event(s)")); + assert_eq!(backend.created_parent_ids.lock().unwrap().as_slice(), &[ + Some(current_run_id()) + ]); + } + + fn fabro_run_tool_services() -> (FabroRunToolServices, Arc) { + let backend = Arc::new(MockRunToolBackend { + child_id: child_run_id(), + created_parent_ids: Mutex::new(Vec::new()), + }); + let services = FabroRunToolServices { + backend: backend.clone(), + current_run_id: current_run_id(), + base_cwd: PathBuf::from("/tmp/fabro-test"), + user_settings_path: PathBuf::from("/tmp/fabro-test/settings.toml"), + }; + (services, backend) + } + + fn tool_context() -> ToolContext { + ToolContext { + env: Arc::new(LocalSandbox::new(PathBuf::from("."))), + cancel: CancellationToken::new(), + tool_env_provider: None, + } + } + + fn current_run_id() -> RunId { + run_id("01KRBZW5C00000000000000001") + } + + fn child_run_id() -> RunId { + run_id("01KRBZW5C00000000000000002") + } + + fn run_id(raw: &str) -> RunId { + raw.parse().expect("test run id should parse") + } + + fn run(run_id: RunId, parent_id: Option, children_count: u64) -> Run { + Run { + id: run_id, + parent_id, + children_count, + title: "Test run".to_string(), + goal: "Test run".to_string(), + workflow: WorkflowRef { + slug: Some("simple".to_string()), + name: Some("Simple".to_string()), + graph_name: None, + node_count: 0, + edge_count: 0, + }, + automation: None, + repository: None, + created_by: None, + origin: RunOrigin::default(), + labels: HashMap::new(), + lifecycle: RunLifecycle { + status: RunStatus::Succeeded { + reason: SuccessReason::Completed, + }, + pending_control: None, + queue_position: None, + error: None, + archived: false, + archived_at: None, + }, + sandbox: None, + models: Vec::new(), + source_directory: None, + timestamps: RunTimestamps { + created_at: chrono::Utc.with_ymd_and_hms(2026, 5, 21, 12, 0, 0).unwrap(), + started_at: None, + last_event_at: None, + completed_at: None, + duration_ms: None, + elapsed_secs: None, + }, + billing: None, + diff: None, + pull_request: None, + current_question: None, + superseded_by: None, + links: RunLinks { web: None }, + } + } + + struct MockRunToolBackend { + child_id: RunId, + created_parent_ids: Mutex>>, + } + + #[async_trait] + impl FabroToolBackend for MockRunToolBackend { + async fn create_run_from_spec( + &self, + _spec: &fabro_tool::ValidatedCreateRunSpec, + _cwd: &Path, + _user_settings_path: &Path, + parent_id: Option, + ) -> anyhow::Result { + self.created_parent_ids.lock().unwrap().push(parent_id); + Ok(self.child_id) + } + + async fn resolve_run(&self, selector: &str) -> anyhow::Result { + let run_id = selector.parse::()?; + Ok(run(run_id, None, 0)) + } + + async fn retrieve_run(&self, run_id: &RunId) -> anyhow::Result { + assert_eq!(*run_id, self.child_id); + Ok(run(self.child_id, Some(current_run_id()), 0)) + } + + async fn start_run(&self, _run_id: &RunId, _resume: bool) -> anyhow::Result { + unreachable!("agent create test uses start=false") + } + + async fn cancel_run(&self, _run_id: &RunId) -> anyhow::Result { + unreachable!() + } + + async fn interrupt_run(&self, _run_id: &RunId) -> anyhow::Result<()> { + unreachable!() + } + + async fn steer_run( + &self, + _run_id: &RunId, + _text: String, + _interrupt: bool, + ) -> anyhow::Result<()> { + unreachable!() + } + + async fn archive_run(&self, _run_id: &RunId) -> anyhow::Result { + unreachable!() + } + + async fn unarchive_run(&self, _run_id: &RunId) -> anyhow::Result { + unreachable!() + } + + async fn list_store_runs(&self) -> anyhow::Result> { + unreachable!() + } + + async fn list_store_runs_by_parent(&self, _parent_id: RunId) -> anyhow::Result> { + unreachable!() + } + + async fn link_run_parent( + &self, + _child_id: &RunId, + _parent_id: &RunId, + ) -> anyhow::Result { + unreachable!() + } + + async fn unlink_run_parent(&self, _child_id: &RunId) -> anyhow::Result { + unreachable!() + } + + async fn get_run_state(&self, _run_id: &RunId) -> anyhow::Result { + unreachable!() + } + + async fn list_run_events( + &self, + _run_id: &RunId, + _after: Option, + _limit: Option, + ) -> anyhow::Result> { + Ok(Vec::new()) + } + + async fn list_run_events_until( + &self, + _run_id: &RunId, + _after: Option, + _limit: usize, + ) -> anyhow::Result> { + Ok(Vec::new()) + } + + async fn list_run_questions( + &self, + _run_id: &RunId, + ) -> anyhow::Result> { + unreachable!() + } + + async fn submit_run_answer( + &self, + _run_id: &RunId, + _question_id: &str, + _body: types::SubmitAnswerRequest, + ) -> anyhow::Result<()> { + unreachable!() + } + } + fn new_file_tracking() -> FileTracking { FileTracking { pending: HashMap::new(), diff --git a/lib/crates/fabro-workflow/src/operations/start.rs b/lib/crates/fabro-workflow/src/operations/start.rs index 8dedc67df..cd59dced0 100644 --- a/lib/crates/fabro-workflow/src/operations/start.rs +++ b/lib/crates/fabro-workflow/src/operations/start.rs @@ -49,6 +49,7 @@ use crate::run_metadata::metadata_branch_name; use crate::run_options::{GitCheckpointOptions, LifecycleOptions, RunOptions}; use crate::run_status::{FailureReason, RunStatus}; use crate::runtime_store::RunStoreHandle; +use crate::services::FabroRunToolServices; use crate::steering_hub::SteeringHub; use crate::workflow_bundle::{RunDefinition, WorkflowBundle}; @@ -82,6 +83,7 @@ struct RunSession { run_control: Option>, vault: Option>>, catalog: Arc, + fabro_run_tools: Option, } struct ResolvedStartLlm { @@ -108,6 +110,7 @@ pub struct StartServices { pub catalog: Arc, pub on_node: crate::OnNodeCallback, pub registry_override: Option>, + pub fabro_run_tools: Option, } pub struct Started { @@ -434,6 +437,7 @@ impl RunSession { workflow_bundle, vault: services.vault, catalog, + fabro_run_tools: services.fabro_run_tools, }) } } @@ -887,6 +891,7 @@ impl RunSession { run_control: self.run_control, checkpoint, seed_context: self.seed_context, + fabro_run_tools: self.fabro_run_tools, }; let mut initialized = Box::pin(pipeline::initialize(persisted, init_options)).await?; initialized.on_node = on_node; @@ -1402,6 +1407,7 @@ reasoning = false catalog: test_catalog(), on_node: None, registry_override: Some(registry), + fabro_run_tools: None, } } diff --git a/lib/crates/fabro-workflow/src/pipeline/execute/tests.rs b/lib/crates/fabro-workflow/src/pipeline/execute/tests.rs index 11b2562c7..f74c131cb 100644 --- a/lib/crates/fabro-workflow/src/pipeline/execute/tests.rs +++ b/lib/crates/fabro-workflow/src/pipeline/execute/tests.rs @@ -281,6 +281,7 @@ async fn execute_test_run_with_options( artifact_sink: None, checkpoint: None, seed_context: None, + fabro_run_tools: None, }, ) .await @@ -346,6 +347,7 @@ async fn execute_runs_start_to_exit_and_returns_final_context() { artifact_sink: None, checkpoint: None, seed_context: None, + fabro_run_tools: None, }, ) .await @@ -418,6 +420,7 @@ async fn run_with_lifecycle( artifact_sink: None, checkpoint: None, seed_context: None, + fabro_run_tools: None, }, ) .await?; diff --git a/lib/crates/fabro-workflow/src/pipeline/initialize.rs b/lib/crates/fabro-workflow/src/pipeline/initialize.rs index c3af87988..953f39d2d 100644 --- a/lib/crates/fabro-workflow/src/pipeline/initialize.rs +++ b/lib/crates/fabro-workflow/src/pipeline/initialize.rs @@ -33,7 +33,9 @@ use crate::handler::{HandlerRegistry, default_registry}; use crate::run_metadata::{RunMetadataRuntime, build_metadata_writer, metadata_branch_name}; use crate::run_options::{GitCheckpointOptions, RunOptions}; use crate::sandbox_git_runtime::SandboxGitRuntime; -use crate::services::{EngineServices, RunLocations, RunServices, WorkflowToolEnvProvider}; +use crate::services::{ + EngineServices, FabroRunToolServices, RunLocations, RunServices, WorkflowToolEnvProvider, +}; use crate::steering_hub::SteeringHub; type BuiltSandboxEnv = (HashMap, Option>); @@ -123,6 +125,7 @@ async fn build_registry( graph: &graph::Graph, llm_source: Arc, catalog: Arc, + fabro_run_tools: Option, ) -> Result<(Arc, bool), Error> { let no_backend_interviewer = Arc::clone(&interviewer); let build_no_backend = move || { @@ -155,9 +158,10 @@ async fn build_registry( let catalog_for_api = Arc::clone(&catalog); let steering_hub_for_api = Arc::clone(&steering_hub); let tool_env_provider_for_backend = Arc::clone(&tool_env_provider); + let fabro_run_tools_for_api = fabro_run_tools.clone(); Arc::new(default_registry(interviewer, move || { let tool_env_provider = Arc::clone(&tool_env_provider_for_backend); - let api = AgentApiBackend::new_with_catalog( + let mut api = AgentApiBackend::new_with_catalog( model.clone(), provider_id.clone(), fallback_chain.clone(), @@ -168,6 +172,9 @@ async fn build_registry( .with_run_model_controls(model_controls.clone()) .with_tool_env_provider(tool_env_provider.clone()) .with_mcp_servers(mcp_servers.clone()); + if let Some(services) = fabro_run_tools_for_api.clone() { + api = api.with_fabro_run_tools(services); + } let acp = AgentAcpBackend::new() .with_tool_env_provider(tool_env_provider.clone(), github_token_refresh_managed) .with_steering_hub(Arc::clone(&steering_hub)); @@ -502,6 +509,7 @@ pub async fn initialize( &graph, Arc::clone(&llm_source), Arc::clone(&catalog), + options.fabro_run_tools.clone(), ) .await? }; @@ -899,6 +907,7 @@ mod tests { artifact_sink: None, checkpoint: None, seed_context: None, + fabro_run_tools: None, }) .await; let events = seen.lock().unwrap().clone(); @@ -960,6 +969,7 @@ mod tests { artifact_sink: None, checkpoint: None, seed_context: None, + fabro_run_tools: None, }) .await .unwrap(); @@ -1042,6 +1052,7 @@ mod tests { &graph, Arc::new(VaultCredentialSource::new(Arc::clone(&vault))), test_catalog(), + None, ) .await .unwrap(); @@ -1158,6 +1169,7 @@ mod tests { artifact_sink: None, checkpoint: None, seed_context: None, + fabro_run_tools: None, }) .await .unwrap(); @@ -1256,6 +1268,7 @@ mod tests { artifact_sink: None, checkpoint: None, seed_context: None, + fabro_run_tools: None, }) .await .unwrap(); @@ -1372,6 +1385,7 @@ mod tests { artifact_sink: None, checkpoint: None, seed_context: None, + fabro_run_tools: None, }) .await; @@ -1439,6 +1453,7 @@ mod tests { artifact_sink: None, checkpoint: None, seed_context: None, + fabro_run_tools: None, }) .await; diff --git a/lib/crates/fabro-workflow/src/pipeline/types.rs b/lib/crates/fabro-workflow/src/pipeline/types.rs index 745924037..2398441f1 100644 --- a/lib/crates/fabro-workflow/src/pipeline/types.rs +++ b/lib/crates/fabro-workflow/src/pipeline/types.rs @@ -24,7 +24,7 @@ use crate::records::{Checkpoint, Conclusion, RunSpec}; use crate::run_control::RunControlState; use crate::run_options::{GitCheckpointOptions, LifecycleOptions, RunOptions}; use crate::runtime_store::RunStoreHandle; -use crate::services::{EngineServices, RunServices}; +use crate::services::{EngineServices, FabroRunToolServices, RunServices}; use crate::steering_hub::SteeringHub; use crate::transforms::{RenderMode, Transform}; use crate::workflow_bundle::WorkflowBundle; @@ -275,6 +275,7 @@ pub struct InitOptions { pub run_control: Option>, pub checkpoint: Option, pub seed_context: Option, + pub fabro_run_tools: Option, } /// Output of the INITIALIZE phase. diff --git a/lib/crates/fabro-workflow/src/services.rs b/lib/crates/fabro-workflow/src/services.rs index 76f988770..72c5499f9 100644 --- a/lib/crates/fabro-workflow/src/services.rs +++ b/lib/crates/fabro-workflow/src/services.rs @@ -10,7 +10,7 @@ use fabro_auth::CredentialSource; use fabro_auth::ResolvedCredentials; use fabro_hooks::{HookContext, HookDecision, HookExecutionContext, HookRunner}; use fabro_model::{Catalog, ProviderId}; -use fabro_types::ManifestPath; +use fabro_types::{ManifestPath, RunId}; use tokio_util::sync::CancellationToken; use crate::event::Emitter; @@ -73,6 +73,14 @@ impl RunLocations { } } +#[derive(Clone)] +pub struct FabroRunToolServices { + pub backend: Arc, + pub current_run_id: RunId, + pub base_cwd: PathBuf, + pub user_settings_path: PathBuf, +} + /// Services shared across workflow phases. /// /// Production construction is expected to happen from pipeline initialization