diff --git a/docs/public/execution/run-configuration.mdx b/docs/public/execution/run-configuration.mdx index e3331e9fa..3f1a348d5 100644 --- a/docs/public/execution/run-configuration.mdx +++ b/docs/public/execution/run-configuration.mdx @@ -403,6 +403,17 @@ include = ["test-results/**", "playwright-report/**", "*.trace.zip"] Artifact collection is opt-in — when no `[run.artifacts]` section is present, no file scanning occurs. +### `[run.agent]` + +Configure workflow agent behavior that is not tied to a single stage. + +```toml title="run.toml" +[run.agent] +fabro_tools = true +``` + +`fabro_tools` defaults to `false`. Set it to `true` only for runs whose agents should be able to create, search, inspect, and interact with Fabro runs through the built-in Fabro run tools. This setting is separate from normal agent `permissions` and from MCP server configuration. + ### `[run.agent.mcps]` Configure [MCP servers](/agents/mcp) available to agent stages during the workflow run. Each server is a named TOML table under `[run.agent.mcps]`. All three transport types are supported: `stdio`, `http`, and `sandbox`. diff --git a/docs/public/reference/user-configuration.mdx b/docs/public/reference/user-configuration.mdx index abe17f584..80eb65123 100644 --- a/docs/public/reference/user-configuration.mdx +++ b/docs/public/reference/user-configuration.mdx @@ -430,15 +430,17 @@ enabled = true ## `[run.agent]` -`[run.agent]` — agent knobs only (permissions, MCPs) +`[run.agent]` — agent knobs only (Fabro tools, permissions, MCPs) ```toml title="settings.toml" [run.agent] +fabro_tools = true permissions = "read-write" ``` | Key | Type / values | Default | Description | |---|---|---|---| +| `fabro_tools` | boolean | false | Allow workflow agents to use Fabro run-management tools. | | `mcps` | table | None | Agent-scoped MCP server entries, keyed by name. | | `permissions` | "read-only" \| "read-write" \| "full" | "read-write" | Default tool permission level for workflow agents. | diff --git a/lib/crates/fabro-cli/src/commands/run/runner.rs b/lib/crates/fabro-cli/src/commands/run/runner.rs index 7cccfe674..d3394e3b0 100644 --- a/lib/crates/fabro-cli/src/commands/run/runner.rs +++ b/lib/crates/fabro-cli/src/commands/run/runner.rs @@ -93,14 +93,18 @@ pub(crate) async fn execute( 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 fabro_run_tools = if run_spec.settings.run.agent.fabro_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), + ) + } else { + None + }; let interviewer = Arc::new(ControlInterviewer::new()); let cancel_token = CancellationToken::new(); let emitter = Arc::new(Emitter::new(run_id)); diff --git a/lib/crates/fabro-cli/tests/it/cmd/attach.rs b/lib/crates/fabro-cli/tests/it/cmd/attach.rs index 285793b9d..5b649d36a 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/attach.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/attach.rs @@ -917,6 +917,7 @@ fn attach_json_errors_without_prompting_for_human_input() { }, "run": { "agent": { + "fabro_tools": false, "mcps": {}, "permissions": null }, diff --git a/lib/crates/fabro-cli/tests/it/cmd/inspect.rs b/lib/crates/fabro-cli/tests/it/cmd/inspect.rs index f060a2979..d947ce16b 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/inspect.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/inspect.rs @@ -175,6 +175,7 @@ fn inspect_resolves_selector_via_server_endpoint() { "slack": null }, "agent": { + "fabro_tools": false, "permissions": null, "mcps": {} }, diff --git a/lib/crates/fabro-config/src/layers/run.rs b/lib/crates/fabro-config/src/layers/run.rs index fd7e994e9..60ae6e54a 100644 --- a/lib/crates/fabro-config/src/layers/run.rs +++ b/lib/crates/fabro-config/src/layers/run.rs @@ -464,7 +464,7 @@ pub struct InterviewProviderLayer { pub channel: Option, } -/// `[run.agent]` — agent knobs only (permissions, MCPs). +/// `[run.agent]` — agent knobs only (Fabro tools, permissions, MCPs). #[derive( Debug, Clone, @@ -477,6 +477,11 @@ pub struct InterviewProviderLayer { )] #[serde(deny_unknown_fields)] pub struct RunAgentLayer { + /// Allow workflow agents to use Fabro run-management tools. + #[serde(default, skip_serializing_if = "Option::is_none")] + #[option(default = "false", value_type = "boolean")] + pub fabro_tools: Option, + /// Default tool permission level for workflow agents. #[serde(default, skip_serializing_if = "Option::is_none")] #[option( @@ -484,10 +489,11 @@ pub struct RunAgentLayer { value_type = "\"read-only\" | \"read-write\" | \"full\"" )] pub permissions: Option, + /// Agent-scoped MCP server entries, keyed by name. #[serde(default, skip_serializing_if = "StickyMap::is_empty")] #[option(value_type = "table")] - pub mcps: StickyMap, + pub mcps: StickyMap, } /// A single MCP entry. `type` selects the transport; `script`/`command` are diff --git a/lib/crates/fabro-config/src/resolve/run.rs b/lib/crates/fabro-config/src/resolve/run.rs index 15d818b6c..d360857b0 100644 --- a/lib/crates/fabro-config/src/resolve/run.rs +++ b/lib/crates/fabro-config/src/resolve/run.rs @@ -350,6 +350,7 @@ fn resolve_agent(agent: Option<&RunAgentLayer>) -> RunAgentSettings { }; RunAgentSettings { + fabro_tools: agent.fabro_tools.unwrap_or(false), permissions: agent.permissions, mcps: agent .mcps diff --git a/lib/crates/fabro-config/src/tests/resolve_run.rs b/lib/crates/fabro-config/src/tests/resolve_run.rs index d81656dbe..e35bcd73c 100644 --- a/lib/crates/fabro-config/src/tests/resolve_run.rs +++ b/lib/crates/fabro-config/src/tests/resolve_run.rs @@ -532,3 +532,80 @@ issues = "{{ env.GH_PERM_LEVEL }}" assert_eq!(issues.as_source(), "{{ env.GH_PERM_LEVEL }}"); } } + +mod run_agent_fabro_tools { + use crate::layers::Combine; + use crate::{SettingsLayer, WorkflowSettingsBuilder}; + + fn parse_settings(source: &str) -> SettingsLayer { + source + .parse::() + .expect("fixture should parse via SettingsLayer") + } + + #[test] + fn defaults_to_false_when_run_agent_is_absent() { + let settings = WorkflowSettingsBuilder::from_layer(&SettingsLayer::default()) + .expect("empty settings should resolve") + .run; + + assert!(!settings.agent.fabro_tools); + } + + #[test] + fn resolves_true_from_run_agent_table() { + let settings = WorkflowSettingsBuilder::from_toml( + r" +_version = 1 + +[run.agent] +fabro_tools = true +", + ) + .expect("run.agent.fabro_tools should resolve"); + + assert!(settings.run.agent.fabro_tools); + } + + #[test] + fn resolves_explicit_false_from_run_agent_table() { + let settings = WorkflowSettingsBuilder::from_toml( + r" +_version = 1 + +[run.agent] +fabro_tools = false +", + ) + .expect("run.agent.fabro_tools false should resolve"); + + assert!(!settings.run.agent.fabro_tools); + } + + #[test] + fn higher_layer_false_overrides_lower_true() { + let workflow = parse_settings( + r" +_version = 1 + +[run.agent] +fabro_tools = false +", + ); + let user = parse_settings( + r" +_version = 1 + +[run.agent] +fabro_tools = true +", + ); + let merged = workflow.combine(user); + + let settings = WorkflowSettingsBuilder::from_layer(&merged) + .expect("merged settings should resolve") + .run; + + assert!(!settings.agent.fabro_tools); + } +} diff --git a/lib/crates/fabro-dev/src/commands/docs_options_reference.rs b/lib/crates/fabro-dev/src/commands/docs_options_reference.rs index 2eca08d75..7507e431f 100644 --- a/lib/crates/fabro-dev/src/commands/docs_options_reference.rs +++ b/lib/crates/fabro-dev/src/commands/docs_options_reference.rs @@ -130,6 +130,7 @@ enabled = true", Section::of::( "[run.agent]", r#"[run.agent] +fabro_tools = true permissions = "read-write""#, ), ] diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 6fff21ae1..4e0ad1a9b 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -2811,6 +2811,7 @@ fn worker_command( run_id: RunId, mode: RunExecutionMode, run_dir: &std::path::Path, + agent_fabro_tools_enabled: bool, ) -> anyhow::Result { let current_exe = std::env::current_exe().context("reading current executable path")?; let exe = std::env::var_os(EnvVars::CARGO_BIN_EXE_FABRO).map_or(current_exe, PathBuf::from); @@ -2823,12 +2824,13 @@ fn worker_command( ) })?; let server_target = daemon.bind.to_target(); - 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 scopes = if agent_fabro_tools_enabled { + WorkerScopeSet::run_worker_with_agent_run_tools() + } else { + WorkerScopeSet::run_worker() + }; + let worker_token = issue_worker_token_with_scopes(state.worker_token_keys(), &run_id, scopes) + .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(), @@ -3103,7 +3105,7 @@ async fn execute_run(state: Arc, run_id: RunId) { return; } - execute_run_subprocess(state, run_id).await; + Box::pin(execute_run_subprocess(state, run_id)).await; } async fn execute_run_in_process(state: Arc, run_id: RunId) { @@ -3432,6 +3434,22 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { run_store.subscribe(), )); + let run_state = match run_store.state().await { + Ok(run_state) => run_state, + Err(err) => { + tracing::error!(run_id = %run_id, error = %err, "Failed to load run state"); + fail_managed_run( + &state, + run_id, + FailureReason::WorkflowError, + format!("Failed to load run state: {err}"), + ); + state.scheduler_notify.notify_one(); + return; + } + }; + let agent_fabro_tools_enabled = run_state.spec.settings.run.agent.fabro_tools; + let state_for_build = Arc::clone(&state); let run_dir_for_build = run_dir.clone(); let build_cmd_result = spawn_blocking(move || { @@ -3440,6 +3458,7 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { run_id, execution_mode, &run_dir_for_build, + agent_fabro_tools_enabled, ) }) .await; diff --git a/lib/crates/fabro-server/src/server/tests.rs b/lib/crates/fabro-server/src/server/tests.rs index 51ce36cbc..96e97d4bf 100644 --- a/lib/crates/fabro-server/src/server/tests.rs +++ b/lib/crates/fabro-server/src/server/tests.rs @@ -1516,92 +1516,53 @@ fn server_secrets_resolve_process_env_before_server_env() { #[cfg(unix)] #[test] -fn worker_command_always_sets_worker_token_env() { - let github_only = tempfile::tempdir().unwrap(); - let github_state = - worker_command_test_state(github_only.path(), &["github"], Some(TEST_DEV_TOKEN)); - let github_run_id = RunId::new(); - let github_cmd = worker_command( - github_state.as_ref(), - github_run_id, - RunExecutionMode::Start, - github_only.path(), - ) - .unwrap(); - assert!(matches!( - command_env_value(&github_cmd, "FABRO_WORKER_TOKEN"), - EnvOverride::Set(_) - )); - assert_eq!( - command_env_value(&github_cmd, "FABRO_DEV_TOKEN"), - EnvOverride::Unchanged - ); - let github_args = github_cmd - .as_std() - .get_args() - .map(|arg| arg.to_string_lossy().into_owned()) - .collect::>(); - assert!( - !github_args - .iter() - .any(|arg| arg == "--artifact-upload-token") - ); - assert!(!github_args.iter().any(|arg| arg == "--worker-token")); - let EnvOverride::Set(github_token) = command_env_value(&github_cmd, "FABRO_WORKER_TOKEN") - else { - panic!("worker token should be set"); - }; - let github_keys = WorkerTokenKeys::from_master_secret(TEST_SESSION_SECRET.as_bytes()) - .expect("worker keys should derive"); - let github_claims = jsonwebtoken::decode::( - &github_token, - github_keys.decoding_key(), - github_keys.validation(), - ) - .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"] - ); +fn worker_command_default_token_omits_agent_run_tools_scope() { + let storage_dir = tempfile::tempdir().unwrap(); + let state = worker_command_test_state(storage_dir.path(), &["dev-token"], Some(TEST_DEV_TOKEN)); + let run_id = RunId::new(); - let dev_token = tempfile::tempdir().unwrap(); - let dev_token_state = - worker_command_test_state(dev_token.path(), &["dev-token"], Some(TEST_DEV_TOKEN)); - let dev_token_run_id = RunId::new(); - let dev_token_cmd = worker_command( - dev_token_state.as_ref(), - dev_token_run_id, + let cmd = worker_command( + state.as_ref(), + run_id, RunExecutionMode::Start, - dev_token.path(), + storage_dir.path(), + false, ) .unwrap(); - assert!(matches!( - command_env_value(&dev_token_cmd, "FABRO_WORKER_TOKEN"), - EnvOverride::Set(_) - )); - assert_eq!( - command_env_value(&dev_token_cmd, "FABRO_DEV_TOKEN"), - EnvOverride::Unchanged - ); - let EnvOverride::Set(dev_worker_token) = - command_env_value(&dev_token_cmd, "FABRO_WORKER_TOKEN") - else { - panic!("worker token should be set"); - }; - let dev_claims = jsonwebtoken::decode::( - &dev_worker_token, - github_keys.decoding_key(), - github_keys.validation(), + + assert_worker_command_passes_token_only_by_env(&cmd); + let claims = worker_token_claims(&cmd, state.as_ref()); + + assert_eq!(claims.run_id, run_id.to_string()); + assert_eq!(claims.scope.split_whitespace().collect::>(), vec![ + "run:worker" + ]); +} + +#[cfg(unix)] +#[test] +fn worker_command_opt_in_token_includes_agent_run_tools_scope() { + let storage_dir = tempfile::tempdir().unwrap(); + let state = worker_command_test_state(storage_dir.path(), &["dev-token"], Some(TEST_DEV_TOKEN)); + let run_id = RunId::new(); + + let cmd = worker_command( + state.as_ref(), + run_id, + RunExecutionMode::Start, + storage_dir.path(), + true, ) - .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"] - ); + .unwrap(); + + assert_worker_command_passes_token_only_by_env(&cmd); + let claims = worker_token_claims(&cmd, state.as_ref()); + + assert_eq!(claims.run_id, run_id.to_string()); + assert_eq!(claims.scope.split_whitespace().collect::>(), vec![ + "run:worker", + "agent:run_tools" + ]); } #[cfg(unix)] @@ -1621,6 +1582,7 @@ fn worker_command_forwards_github_app_private_key_from_server_secrets() { RunId::new(), RunExecutionMode::Start, storage_dir.path(), + false, ) .unwrap(); @@ -1640,6 +1602,7 @@ fn worker_command_omits_github_app_private_key_when_unset() { RunId::new(), RunExecutionMode::Start, storage_dir.path(), + false, ) .unwrap(); @@ -1669,6 +1632,7 @@ level = "debug" run_id, RunExecutionMode::Start, storage_dir.path(), + false, ) .unwrap(); @@ -1698,6 +1662,7 @@ destination = "stdout" run_id, RunExecutionMode::Start, storage_dir.path(), + false, ) .unwrap(); @@ -1726,6 +1691,7 @@ fn worker_command_sets_fabro_config_to_active_absolute_config_path() { run_id, RunExecutionMode::Start, storage_dir.path(), + false, ) .unwrap(); @@ -1767,6 +1733,7 @@ destination = "file" run_id, RunExecutionMode::Start, storage_dir.path(), + false, ) .unwrap(); @@ -1798,6 +1765,7 @@ destination = "file" run_id, RunExecutionMode::Start, storage_dir.path(), + false, ) else { panic!("invalid env destination should fail"); }; @@ -2106,6 +2074,40 @@ fn command_env_value(cmd: &Command, key: &str) -> EnvOverride { .unwrap_or(EnvOverride::Unchanged) } +#[cfg(unix)] +fn assert_worker_command_passes_token_only_by_env(cmd: &Command) { + assert!(matches!( + command_env_value(cmd, EnvVars::FABRO_WORKER_TOKEN), + EnvOverride::Set(_) + )); + assert_eq!( + command_env_value(cmd, EnvVars::FABRO_DEV_TOKEN), + EnvOverride::Unchanged + ); + let args = cmd + .as_std() + .get_args() + .map(|arg| arg.to_string_lossy()) + .collect::>(); + assert!(!args.iter().any(|arg| arg == "--artifact-upload-token")); + assert!(!args.iter().any(|arg| arg == "--worker-token")); +} + +#[cfg(unix)] +fn worker_token_claims(cmd: &Command, state: &AppState) -> crate::worker_token::WorkerTokenClaims { + let EnvOverride::Set(token) = command_env_value(cmd, EnvVars::FABRO_WORKER_TOKEN) else { + panic!("worker token env should be set"); + }; + + jsonwebtoken::decode::( + &token, + state.worker_token_keys().decoding_key(), + state.worker_token_keys().validation(), + ) + .expect("worker token should decode") + .claims +} + #[tokio::test] async fn subprocess_answer_transport_cancel_run_enqueues_cancel_message() { let (control_tx, mut control_rx) = tokio::sync::mpsc::channel(1); diff --git a/lib/crates/fabro-server/src/worker_token.rs b/lib/crates/fabro-server/src/worker_token.rs index f64765801..9b7f688cf 100644 --- a/lib/crates/fabro-server/src/worker_token.rs +++ b/lib/crates/fabro-server/src/worker_token.rs @@ -66,7 +66,6 @@ pub(crate) struct WorkerScopeSet { } impl WorkerScopeSet { - #[cfg(test)] #[must_use] pub(crate) const fn run_worker() -> Self { Self { diff --git a/lib/crates/fabro-types/src/settings/run.rs b/lib/crates/fabro-types/src/settings/run.rs index 8a10fe9dc..a716c7827 100644 --- a/lib/crates/fabro-types/src/settings/run.rs +++ b/lib/crates/fabro-types/src/settings/run.rs @@ -417,10 +417,28 @@ pub struct InterviewProviderSettings { #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] pub struct RunAgentSettings { + #[serde(default)] + pub fabro_tools: bool, pub permissions: Option, pub mcps: HashMap, } +#[cfg(test)] +mod run_agent_settings_tests { + use super::RunAgentSettings; + + #[test] + fn deserializes_missing_fabro_tools_as_false() { + let settings: RunAgentSettings = serde_json::from_value(serde_json::json!({ + "permissions": null, + "mcps": {} + })) + .expect("legacy run agent settings should deserialize"); + + assert!(!settings.fabro_tools); + } +} + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct McpServerSettings { pub name: String,