diff --git a/Cargo.lock b/Cargo.lock index 1a1024a5b..75b55d988 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1678,6 +1678,7 @@ dependencies = [ "fabro-interview", "fabro-llm", "fabro-macros", + "fabro-manifest", "fabro-mcp", "fabro-mcp-server", "fabro-model", @@ -2004,6 +2005,24 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "fabro-manifest" +version = "0.230.0-nightly.0" +dependencies = [ + "anyhow", + "fabro-api", + "fabro-config", + "fabro-github", + "fabro-graphviz", + "fabro-template", + "fabro-types", + "fabro-workflow", + "git2", + "temp-env", + "tempfile", + "toml 0.8.23", +] + [[package]] name = "fabro-mcp" version = "0.230.0-nightly.0" @@ -2029,7 +2048,10 @@ dependencies = [ "dirs", "fabro-api", "fabro-client", + "fabro-config", "fabro-http", + "fabro-manifest", + "fabro-server", "fabro-types", "fabro-util", "rmcp", diff --git a/lib/crates/fabro-cli/Cargo.toml b/lib/crates/fabro-cli/Cargo.toml index e05a5691d..9da42ddaf 100644 --- a/lib/crates/fabro-cli/Cargo.toml +++ b/lib/crates/fabro-cli/Cargo.toml @@ -32,6 +32,7 @@ fabro-install = { path = "../fabro-install" } fabro-interview = { path = "../fabro-interview" } fabro-mcp = { path = "../fabro-mcp" } fabro-mcp-server = { path = "../fabro-mcp-server" } +fabro-manifest = { path = "../fabro-manifest" } fabro-proc = { path = "../fabro-proc" } fabro-sandbox = { path = "../fabro-sandbox", features = ["daytona"] } fabro-checkpoint = { path = "../fabro-checkpoint" } diff --git a/lib/crates/fabro-cli/src/command_context.rs b/lib/crates/fabro-cli/src/command_context.rs index 34b7d3876..5403c57ce 100644 --- a/lib/crates/fabro-cli/src/command_context.rs +++ b/lib/crates/fabro-cli/src/command_context.rs @@ -102,6 +102,10 @@ impl CommandContext { &self.cwd } + pub(crate) fn storage_dir(&self) -> &Path { + &self.storage_dir + } + pub(crate) fn run_settings(&self) -> Result<&RunNamespace> { self.run_settings .as_ref() diff --git a/lib/crates/fabro-cli/src/commands/mcp/mod.rs b/lib/crates/fabro-cli/src/commands/mcp/mod.rs index d7a82a7cc..566a33c17 100644 --- a/lib/crates/fabro-cli/src/commands/mcp/mod.rs +++ b/lib/crates/fabro-cli/src/commands/mcp/mod.rs @@ -4,6 +4,7 @@ use anyhow::{Context as _, Result}; use crate::args::{McpAgent, McpCommand, McpNamespace, ServerConnectionArgs}; use crate::command_context::CommandContext; +use crate::user_config; pub(crate) async fn dispatch(ns: McpNamespace, base_ctx: &CommandContext) -> Result<()> { match ns.command { @@ -26,10 +27,23 @@ fn server_settings( base_ctx: &CommandContext, connection: &ServerConnectionArgs, ) -> Result { + let connection_ctx = base_ctx.with_connection(connection)?; + let server_target = user_config::resolve_nondefault_server_target( + &connection.target, + connection_ctx.user_settings(), + )? + .map(|target| { + target + .as_unix_socket_path() + .map_or_else(|| target.to_string(), |path| path.display().to_string()) + }); Ok(fabro_mcp_server::McpServerSettings { - config: config_settings(connection), + config: config_settings(connection), + server_target, + storage_dir: connection_ctx.storage_dir().to_path_buf(), + config_path: connection_ctx.base_config_path().to_path_buf(), home_dir: home_dir()?, - cwd: base_ctx.cwd().to_path_buf(), + cwd: base_ctx.cwd().to_path_buf(), }) } diff --git a/lib/crates/fabro-cli/src/manifest_builder.rs b/lib/crates/fabro-cli/src/manifest_builder.rs index b2436db77..fe03bd5b1 100644 --- a/lib/crates/fabro-cli/src/manifest_builder.rs +++ b/lib/crates/fabro-cli/src/manifest_builder.rs @@ -1,177 +1,9 @@ -#![expect( - clippy::disallowed_methods, - reason = "CLI manifest builder: sync file I/O building install manifests" -)] - -use std::collections::{HashMap, HashSet}; -use std::path::{Component, Path, PathBuf}; - -use anyhow::{Context, Result, anyhow}; use fabro_api::types; -use fabro_config::project::{self, discover_project_config, resolve_workflow_path}; -use fabro_config::run::{resolve_run_goal_from_layer, resolve_run_goal_from_namespace}; -use fabro_config::{CliLayer, DaytonaDockerfileLayer, RunLayer, WorkflowSettingsBuilder}; -use fabro_graphviz::graph::AttrValue; -use fabro_graphviz::parser; -use fabro_template::{TemplateContext, render as render_template}; -use fabro_types::settings::run::{ResolvedGoalSource, ResolvedRunGoal}; -use fabro_types::{DirtyStatus, GitContext, PreRunPushOutcome, RunId, WorkflowSettings}; -use fabro_workflow::ManifestPath; -use fabro_workflow::git::{ - GitSyncStatus, branch_needs_push, head_sha, push_branch_noninteractive, sync_status, -}; +#[allow(unused_imports, reason = "fabro-cli public lib re-exports this type")] +pub use fabro_manifest::{BuiltManifest, ManifestBuildInput, build_run_manifest}; use crate::args::{PreflightArgs, RunArgs}; -#[derive(Debug, Default)] -pub struct ManifestBuildInput { - pub workflow: PathBuf, - pub cwd: PathBuf, - pub run_overrides: Option, - pub cli_overrides: Option, - pub input_overrides: HashMap, - pub args: Option, - pub run_id: Option, - /// Path to the user settings file (for inclusion in - /// `RunManifest.configs`). `None` skips the user config entry. - pub user_settings_path: Option, -} - -#[derive(Debug)] -pub struct BuiltManifest { - pub manifest: types::RunManifest, - pub target_path: PathBuf, -} - -struct CollectContext<'a> { - cwd: &'a Path, - inputs: &'a HashMap, - workflows: HashMap, - visited_workflows: HashSet, -} - -#[derive(Clone)] -struct WorkflowScanInput { - absolute_dot_path: PathBuf, - dot_path: ManifestPath, - source: String, -} - -pub fn build_run_manifest(input: ManifestBuildInput) -> Result { - let root_resolution = resolve_workflow_path(&input.workflow, &input.cwd)?; - if root_resolution.workflow_toml_path.is_none() - && !root_resolution.resolved_workflow_path.is_file() - { - return Err(fabro_config::Error::WorkflowNotFound( - root_resolution.resolved_workflow_path.display().to_string(), - ) - .into()); - } - let workflow_parent = root_resolution - .resolved_workflow_path - .parent() - .unwrap_or_else(|| Path::new(".")); - let project_config = discover_project_config(workflow_parent)?; - let mut workflow_settings_builder = WorkflowSettingsBuilder::new(); - if let Some(run) = input.run_overrides.clone() { - workflow_settings_builder = workflow_settings_builder.run_overrides(run); - } - if let Some(cli) = input.cli_overrides.clone() { - workflow_settings_builder = workflow_settings_builder.cli_overrides(cli); - } - if let Some(path) = root_resolution.workflow_toml_path.as_ref() { - workflow_settings_builder = workflow_settings_builder.workflow_file(path)?; - } - if let Some(path) = project_config.as_ref() { - workflow_settings_builder = workflow_settings_builder.project_file(path)?; - } - if let Some(path) = input - .user_settings_path - .as_ref() - .filter(|path| path.is_file()) - { - workflow_settings_builder = workflow_settings_builder.user_file(path)?; - } - let mut workflow_settings = workflow_settings_builder - .build() - .context("failed to resolve manifest settings")?; - workflow_settings.run.inputs.extend(input.input_overrides); - let target_path = root_resolution.dot_path.clone(); - let target_manifest_path = manifest_path_from_absolute(&target_path, &input.cwd)?; - let target_key = target_manifest_path.to_string(); - - let mut context = CollectContext { - cwd: &input.cwd, - inputs: &workflow_settings.run.inputs, - workflows: HashMap::new(), - visited_workflows: HashSet::new(), - }; - collect_workflow_entry(&mut context, &input.workflow, &input.cwd)?; - - let root_source = context - .workflows - .get(&target_key) - .map(|workflow| workflow.source.clone()) - .ok_or_else(|| anyhow!("root workflow missing from manifest bundle"))?; - - let mut configs = Vec::new(); - if let Some(path) = project_config { - let source = std::fs::read_to_string(&path) - .with_context(|| format!("Failed to read {}", path.display()))?; - configs.push(types::ManifestConfig { - path: Some(path.display().to_string()), - source: Some(source), - type_: types::ManifestConfigType::Project, - }); - } - if let Some(path) = input.user_settings_path.filter(|p| p.is_file()) { - let source = std::fs::read_to_string(&path) - .with_context(|| format!("Failed to read {}", path.display()))?; - configs.push(types::ManifestConfig { - path: Some(path.display().to_string()), - source: Some(source), - type_: types::ManifestConfigType::User, - }); - } - - let working_directory = - project::resolve_working_directory_from_run(&workflow_settings.run, &input.cwd); - - let rendered_root_source = - render_workflow_scan_source(&root_source, &target_path, &workflow_settings.run.inputs)?; - - let goal = resolve_manifest_goal( - input.run_overrides.as_ref(), - &workflow_settings, - &rendered_root_source, - &target_path, - &working_directory, - )?; - - let configured_repo_origin_url = configured_repo_origin_url(&workflow_settings); - let git = build_git_context(&working_directory, configured_repo_origin_url.as_deref()); - let args = input.args.filter(|args| !manifest_args_is_empty(args)); - - Ok(BuiltManifest { - manifest: types::RunManifest { - args, - configs, - cwd: input.cwd.display().to_string(), - git, - goal, - run_id: input.run_id.map(|run_id| run_id.to_string()), - title: None, - target: types::ManifestTarget { - identifier: input.workflow.display().to_string(), - path: target_key, - }, - version: 1, - workflows: context.workflows, - }, - target_path, - }) -} - pub(crate) fn run_manifest_args(args: &RunArgs) -> Option { let payload = types::ManifestArgs { auto_approve: args.auto_approve.then_some(true), @@ -187,7 +19,7 @@ pub(crate) fn run_manifest_args(args: &RunArgs) -> Option { input: args.inputs.values.clone(), verbose: args.verbose.then_some(true), }; - (!manifest_args_is_empty(&payload)).then_some(payload) + (!fabro_manifest::manifest_args_is_empty(&payload)).then_some(payload) } pub(crate) fn preflight_manifest_args(args: &PreflightArgs) -> Option { @@ -205,1002 +37,5 @@ pub(crate) fn preflight_manifest_args(args: &PreflightArgs) -> Option, - workflow: &Path, - resolve_from: &Path, -) -> Result<()> { - let normalized_workflow = if workflow.extension().is_some() && workflow.is_relative() { - normalize_absolute_path(resolve_from, &workflow.to_string_lossy()).ok_or_else(|| { - anyhow!( - "unsupported manifest workflow reference: {}", - workflow.display() - ) - })? - } else { - workflow.to_path_buf() - }; - let resolution = resolve_workflow_path(&normalized_workflow, resolve_from)?; - let dot_path = manifest_path_from_absolute(&resolution.dot_path, context.cwd)?; - let dot_key = dot_path.to_string(); - if !context.visited_workflows.insert(dot_key.clone()) { - return Ok(()); - } - - let source = std::fs::read_to_string(&resolution.dot_path) - .with_context(|| format!("Failed to read {}", resolution.dot_path.display()))?; - let config = if let Some(workflow_toml_path) = resolution.workflow_toml_path.as_ref() { - Some(types::ManifestWorkflowConfig { - path: manifest_path_from_absolute(workflow_toml_path, context.cwd)?.to_string(), - source: std::fs::read_to_string(workflow_toml_path) - .with_context(|| format!("Failed to read {}", workflow_toml_path.display()))?, - }) - } else { - None - }; - - let scan = WorkflowScanInput { - absolute_dot_path: resolution.dot_path, - dot_path, - source: source.clone(), - }; - let mut files = HashMap::new(); - let mut visited_imports = HashSet::new(); - if let Some(config) = config.as_ref() { - collect_workflow_config_files(context, config, &mut files)?; - } - collect_workflow_files(context, &scan, &mut files, &mut visited_imports)?; - - context.workflows.insert(dot_key, types::ManifestWorkflow { - config, - files, - source, - }); - - Ok(()) -} - -fn collect_workflow_files( - context: &mut CollectContext<'_>, - workflow: &WorkflowScanInput, - files: &mut HashMap, - visited_imports: &mut HashSet, -) -> Result<()> { - let rendered_source = render_workflow_scan_source( - &workflow.source, - &workflow.absolute_dot_path, - context.inputs, - )?; - let graph = parser::parse(&rendered_source).map_err(|err| { - anyhow!( - "Failed to parse {}: {err}", - workflow.absolute_dot_path.display() - ) - })?; - - if let Some(goal_ref) = graph.attrs.get("goal").and_then(AttrValue::as_str) { - if goal_ref.starts_with('@') { - collect_bundled_file( - files, - workflow - .absolute_dot_path - .parent() - .unwrap_or_else(|| Path::new(".")), - context.cwd, - goal_ref.trim_start_matches('@'), - types::ManifestFileRefType::FileInline, - Some(workflow.dot_path.clone()), - )?; - } - } - - for node in graph.nodes.values() { - if let Some(prompt_ref) = node.attrs.get("prompt").and_then(AttrValue::as_str) { - if prompt_ref.starts_with('@') { - collect_bundled_file( - files, - workflow - .absolute_dot_path - .parent() - .unwrap_or_else(|| Path::new(".")), - context.cwd, - prompt_ref.trim_start_matches('@'), - types::ManifestFileRefType::FileInline, - Some(workflow.dot_path.clone()), - )?; - } - } - - if let Some(import_ref) = node.attrs.get("import").and_then(AttrValue::as_str) { - let imported = collect_bundled_file( - files, - workflow - .absolute_dot_path - .parent() - .unwrap_or_else(|| Path::new(".")), - context.cwd, - import_ref, - types::ManifestFileRefType::Import, - Some(workflow.dot_path.clone()), - )?; - let import_key = imported.path.to_string(); - if visited_imports.insert(import_key) { - let imported_source = std::fs::read_to_string(&imported.absolute_path) - .with_context(|| { - format!("Failed to read {}", imported.absolute_path.display()) - })?; - let imported_scan = WorkflowScanInput { - absolute_dot_path: imported.absolute_path, - dot_path: imported.path, - source: imported_source, - }; - collect_workflow_files(context, &imported_scan, files, visited_imports)?; - } - } - - if let Some(child_ref) = node - .attrs - .get("stack.child_workflow") - .or_else(|| node.attrs.get("stack.child_dotfile")) - .and_then(AttrValue::as_str) - { - collect_workflow_entry( - context, - Path::new(child_ref), - workflow - .absolute_dot_path - .parent() - .unwrap_or_else(|| Path::new(".")), - )?; - } - } - - Ok(()) -} - -fn render_workflow_scan_source( - source: &str, - path: &Path, - inputs: &HashMap, -) -> Result { - render_template(source, &TemplateContext::for_input_scan(inputs.clone())) - .with_context(|| format!("Failed to render {} for manifest scanning", path.display())) -} - -fn collect_workflow_config_files( - context: &CollectContext<'_>, - config: &types::ManifestWorkflowConfig, - files: &mut HashMap, -) -> Result<()> { - let mut document: toml::Table = config - .source - .parse() - .context("Failed to parse run config TOML")?; - let run = document - .remove("run") - .map(toml::Value::try_into::) - .transpose() - .context("Failed to parse run config TOML")? - .unwrap_or_default(); - let dockerfile = run - .sandbox - .as_ref() - .and_then(|sandbox| sandbox.daytona.as_ref()) - .and_then(|daytona| daytona.snapshot.as_ref()) - .and_then(|snapshot| snapshot.dockerfile.as_ref()); - - let Some(DaytonaDockerfileLayer::Path { path }) = dockerfile else { - return Ok(()); - }; - - let config_path = ManifestPath::from_wire(&config.path) - .ok_or_else(|| anyhow!("invalid manifest workflow config path: {}", config.path))?; - let absolute_config_path = context.cwd.join(config_path.as_path()); - collect_bundled_file( - files, - absolute_config_path - .parent() - .unwrap_or_else(|| Path::new(".")), - context.cwd, - path, - types::ManifestFileRefType::Dockerfile, - Some(config_path), - )?; - Ok(()) -} - -struct BundledFile { - absolute_path: PathBuf, - path: ManifestPath, -} - -fn collect_bundled_file( - files: &mut HashMap, - base_dir: &Path, - cwd: &Path, - reference: &str, - ref_type: types::ManifestFileRefType, - from: Option, -) -> Result { - let absolute_path = normalize_absolute_path(base_dir, reference) - .ok_or_else(|| anyhow!("unsupported manifest reference: {reference}"))?; - let path = manifest_path_from_absolute(&absolute_path, cwd)?; - let key = path.to_string(); - if !files.contains_key(&key) { - let content = std::fs::read_to_string(&absolute_path) - .with_context(|| format!("Failed to read {}", absolute_path.display()))?; - files.insert(key.clone(), types::ManifestFileEntry { - content, - ref_: types::ManifestFileRef { - from: from.map(|value| value.to_string()), - original: reference.to_string(), - type_: ref_type, - }, - }); - } - - Ok(BundledFile { - absolute_path, - path, - }) -} - -fn resolve_manifest_goal( - run_overrides: Option<&RunLayer>, - settings: &WorkflowSettings, - root_source: &str, - root_dot_path: &Path, - working_directory: &Path, -) -> Result> { - // Precedence 1: CLI args (`--goal` / `--goal-file`). These are already - // resolved to absolute paths by `overrides::goal_layer_from_args`. - if let Some(run_overrides) = run_overrides { - if let Some(resolved) = resolve_run_goal_from_layer(run_overrides, working_directory) - .context("failed to resolve --goal-file contents")? - { - return Ok(Some(resolved_goal_to_manifest(resolved))); - } - } - - // Precedence 2: merged config `run.goal`. Config-sourced `goal.file` - // paths were rewritten to absolute by `load_settings_path` at the - // directory of the config file that declared them. - if let Some(resolved) = resolve_run_goal_from_namespace(&settings.run, working_directory) - .context("failed to resolve run.goal.file contents")? - { - return Ok(Some(resolved_goal_to_manifest(resolved))); - } - - // Precedence 3: graph-level `goal` attribute in the DOT, with `@file` - // sugar for workflow-colocated goal files. - let graph = parser::parse(root_source) - .with_context(|| format!("Failed to parse {}", root_dot_path.display()))?; - let Some(goal) = graph.attrs.get("goal").and_then(AttrValue::as_str) else { - return Ok(None); - }; - if let Some(reference) = goal.strip_prefix('@') { - let goal_path = normalize_absolute_path( - root_dot_path.parent().unwrap_or_else(|| Path::new(".")), - reference, - ) - .ok_or_else(|| anyhow!("unsupported manifest goal reference: {reference}"))?; - return Ok(Some(types::ManifestGoal { - path: Some(reference.to_string()), - text: std::fs::read_to_string(&goal_path) - .with_context(|| format!("Failed to read {}", goal_path.display()))?, - type_: types::ManifestGoalType::Graph, - })); - } - - Ok(Some(types::ManifestGoal { - path: None, - text: goal.to_string(), - type_: types::ManifestGoalType::Graph, - })) -} - -/// Translate a [`ResolvedRunGoal`] into the wire-level `ManifestGoal` -/// shape. Inline goals get `type = Value`; file-sourced goals keep their -/// absolute path as the `path` field and use `type = File`. -fn resolved_goal_to_manifest(resolved: ResolvedRunGoal) -> types::ManifestGoal { - match resolved.source { - ResolvedGoalSource::Inline => types::ManifestGoal { - path: None, - text: resolved.text, - type_: types::ManifestGoalType::Value, - }, - ResolvedGoalSource::File { path } => types::ManifestGoal { - path: Some(path.to_string_lossy().into_owned()), - text: resolved.text, - type_: types::ManifestGoalType::File, - }, - } -} - -fn build_git_context( - repo_path: &Path, - configured_repo_origin_url: Option<&str>, -) -> Option { - let (origin_url, branch) = detect_manifest_repo_info(repo_path)?; - let sha = head_sha(repo_path).ok(); - let dirty = match sync_status(repo_path, "origin", Some(&branch)) { - GitSyncStatus::Dirty => DirtyStatus::Dirty, - GitSyncStatus::Synced | GitSyncStatus::Unsynced => DirtyStatus::Clean, - }; - let repo_origin_url = configured_repo_origin_url - .map(fabro_github::normalize_repo_origin_url) - .filter(|url| !url.is_empty()) - .or_else(|| { - origin_url - .as_deref() - .map(fabro_github::normalize_repo_origin_url) - .filter(|url| !url.is_empty()) - }) - .unwrap_or_default(); - let push_outcome = build_manifest_push_outcome( - repo_path, - &branch, - origin_url.as_deref(), - configured_repo_origin_url, - ); - Some(GitContext { - origin_url: repo_origin_url, - branch, - sha, - dirty, - push_outcome, - }) -} - -fn configured_repo_origin_url(settings: &WorkflowSettings) -> Option { - let scm = &settings.run.scm; - if !scm - .provider - .as_deref() - .is_none_or(|provider| provider.eq_ignore_ascii_case("github")) - { - return None; - } - let owner = scm.owner.as_ref()?.as_source(); - let repository = scm.repository.as_ref()?.as_source(); - if owner.trim().is_empty() || repository.trim().is_empty() { - return None; - } - let origin = format!("https://github.com/{owner}/{repository}"); - let normalized = fabro_github::normalize_repo_origin_url(&origin); - (!normalized.is_empty()).then_some(normalized) -} - -fn detect_manifest_repo_info(repo_path: &Path) -> Option<(Option, String)> { - let repo = git2::Repository::discover(repo_path).ok()?; - let branch = repo.head().ok()?.shorthand().map(ToOwned::to_owned)?; - let origin_url = repo - .find_remote("origin") - .ok() - .and_then(|remote| remote.url().map(ToOwned::to_owned)); - Some((origin_url, branch)) -} - -fn build_manifest_push_outcome( - repo_path: &Path, - branch: &str, - origin_url: Option<&str>, - configured_repo_origin_url: Option<&str>, -) -> PreRunPushOutcome { - let Some(origin_url) = origin_url else { - return PreRunPushOutcome::SkippedNoRemote; - }; - - if let Some(repo_origin_url) = configured_repo_origin_url - .map(fabro_github::normalize_repo_origin_url) - .filter(|url| !url.is_empty()) - { - let remote = fabro_github::normalize_repo_origin_url(origin_url); - if remote != repo_origin_url { - return PreRunPushOutcome::SkippedRemoteMismatch { - remote, - repo_origin_url, - }; - } - } - - if !branch_needs_push(repo_path, "origin", branch) { - return PreRunPushOutcome::NotAttempted; - } - - match push_branch_noninteractive(repo_path, "origin", branch) { - Ok(()) => PreRunPushOutcome::Succeeded { - remote: "origin".to_string(), - branch: branch.to_string(), - }, - Err(err) => PreRunPushOutcome::Failed { - remote: "origin".to_string(), - branch: branch.to_string(), - message: err.to_string(), - }, - } -} - -fn normalize_absolute_path(base_dir: &Path, reference: &str) -> Option { - let path = Path::new(reference); - if path.is_absolute() || reference.starts_with('~') { - return None; - } - - let mut normalized = PathBuf::new(); - for component in base_dir.join(path).components() { - match component { - Component::CurDir => {} - Component::Normal(part) => normalized.push(part), - Component::ParentDir => { - normalized.pop(); - } - Component::RootDir => normalized.push(Path::new("/")), - Component::Prefix(prefix) => normalized.push(prefix.as_os_str()), - } - } - Some(normalized) -} - -fn manifest_path_from_absolute(path: &Path, cwd: &Path) -> Result { - ManifestPath::from_absolute(path, cwd) - .ok_or_else(|| anyhow!("Failed to compute manifest path for {}", path.display())) -} - -fn manifest_args_is_empty(args: &types::ManifestArgs) -> bool { - args.auto_approve.is_none() - && args.dry_run.is_none() - && args.label.is_empty() - && args.model.is_none() - && args.preserve_sandbox.is_none() - && args.provider.is_none() - && args.sandbox.is_none() - && args.docker_image.is_none() - && args.input.is_empty() - && args.verbose.is_none() -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn build_manifest_bundles_imports_prompts_and_children() { - let temp = tempfile::tempdir().unwrap(); - let project = temp.path(); - let workflow_dir = project.join(".fabro/workflows/demo"); - let child_dir = project.join(".fabro/workflows/child"); - std::fs::create_dir_all(workflow_dir.join("prompts")).unwrap(); - std::fs::create_dir_all(workflow_dir.join("imports")).unwrap(); - std::fs::create_dir_all(&child_dir).unwrap(); - std::fs::write(project.join(".fabro/project.toml"), "_version = 1\n").unwrap(); - std::fs::write( - workflow_dir.join("workflow.toml"), - "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n", - ) - .unwrap(); - std::fs::write( - workflow_dir.join("workflow.fabro"), - r#"digraph Demo { - graph [goal="@prompts/goal.md"] - start [shape=Mdiamond] - exit [shape=Msquare] - plan [prompt="@prompts/plan.md"] - imported [import="./imports/checks.fabro"] - child [shape=house, stack.child_workflow="../child/workflow.fabro"] - start -> plan -> imported -> child -> exit - }"#, - ) - .unwrap(); - std::fs::write(workflow_dir.join("prompts/goal.md"), "ship it").unwrap(); - std::fs::write(workflow_dir.join("prompts/plan.md"), "plan it").unwrap(); - std::fs::write( - workflow_dir.join("imports/checks.fabro"), - r#"digraph Checks { - start [shape=Mdiamond] - exit [shape=Msquare] - lint [prompt="@../prompts/lint.md"] - start -> lint -> exit - }"#, - ) - .unwrap(); - std::fs::write(workflow_dir.join("prompts/lint.md"), "lint it").unwrap(); - std::fs::write( - child_dir.join("workflow.fabro"), - r"digraph Child { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", - ) - .unwrap(); - - let built = build_run_manifest(ManifestBuildInput { - workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"), - cwd: project.to_path_buf(), - ..Default::default() - }) - .unwrap(); - - assert_eq!( - built.manifest.target.path, - ".fabro/workflows/demo/workflow.fabro" - ); - assert_eq!(built.manifest.workflows.len(), 2); - let root = &built.manifest.workflows[".fabro/workflows/demo/workflow.fabro"]; - assert!( - root.files - .contains_key(".fabro/workflows/demo/prompts/goal.md") - ); - assert!( - root.files - .contains_key(".fabro/workflows/demo/prompts/plan.md") - ); - assert!( - root.files - .contains_key(".fabro/workflows/demo/imports/checks.fabro") - ); - assert!( - root.files - .contains_key(".fabro/workflows/demo/prompts/lint.md") - ); - assert_eq!(built.manifest.goal.unwrap().text, "ship it"); - assert!( - built - .manifest - .workflows - .contains_key(".fabro/workflows/child/workflow.fabro") - ); - } - - #[test] - fn build_manifest_uses_input_overrides_for_structural_file_scanning() { - let temp = tempfile::tempdir().unwrap(); - let project = temp.path(); - let workflow_dir = project.join(".fabro/workflows/demo"); - let child_dir = project.join(".fabro/workflows/child"); - std::fs::create_dir_all(workflow_dir.join("prompts")).unwrap(); - std::fs::create_dir_all(workflow_dir.join("imports")).unwrap(); - std::fs::create_dir_all(&child_dir).unwrap(); - std::fs::write(project.join(".fabro/project.toml"), "_version = 1\n").unwrap(); - std::fs::write( - workflow_dir.join("workflow.toml"), - "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n", - ) - .unwrap(); - std::fs::write( - workflow_dir.join("workflow.fabro"), - r#"digraph Demo { - graph [goal="Demo"] - start [shape=Mdiamond] - exit [shape=Msquare] - plan [prompt="@prompts/{{ inputs.prompt_file }}"] - imported [import="./imports/{{ inputs.import_file }}"] - child [shape=house, stack.child_workflow="../{{ inputs.child_workflow }}/workflow.fabro"] - start -> plan -> imported -> child -> exit - }"#, - ) - .unwrap(); - std::fs::write(workflow_dir.join("prompts/plan.md"), "plan it").unwrap(); - std::fs::write( - workflow_dir.join("imports/checks.fabro"), - r"digraph Checks { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", - ) - .unwrap(); - std::fs::write( - child_dir.join("workflow.fabro"), - r"digraph Child { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", - ) - .unwrap(); - - let built = build_run_manifest(ManifestBuildInput { - workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"), - cwd: project.to_path_buf(), - input_overrides: HashMap::from([ - ( - "prompt_file".to_string(), - toml::Value::String("plan.md".to_string()), - ), - ( - "import_file".to_string(), - toml::Value::String("checks.fabro".to_string()), - ), - ( - "child_workflow".to_string(), - toml::Value::String("child".to_string()), - ), - ]), - ..Default::default() - }) - .unwrap(); - - let root = &built.manifest.workflows[".fabro/workflows/demo/workflow.fabro"]; - assert!( - root.source.contains("{{ inputs.prompt_file }}"), - "manifest should store original workflow source" - ); - assert!( - root.files - .contains_key(".fabro/workflows/demo/prompts/plan.md") - ); - assert!( - root.files - .contains_key(".fabro/workflows/demo/imports/checks.fabro") - ); - assert!( - built - .manifest - .workflows - .contains_key(".fabro/workflows/child/workflow.fabro") - ); - } - - #[test] - fn build_manifest_uses_input_overrides_for_graph_goal_file_resolution() { - let temp = tempfile::tempdir().unwrap(); - let project = temp.path(); - let workflow_dir = project.join(".fabro/workflows/demo"); - std::fs::create_dir_all(workflow_dir.join("prompts")).unwrap(); - std::fs::write(project.join(".fabro/project.toml"), "_version = 1\n").unwrap(); - std::fs::write( - workflow_dir.join("workflow.toml"), - "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n", - ) - .unwrap(); - std::fs::write( - workflow_dir.join("workflow.fabro"), - r#"digraph Demo { - graph [goal="@prompts/{{ inputs.goal_file }}"] - start [shape=Mdiamond] - exit [shape=Msquare] - start -> exit - }"#, - ) - .unwrap(); - std::fs::write(workflow_dir.join("prompts/goal.md"), "ship it").unwrap(); - - let built = build_run_manifest(ManifestBuildInput { - workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"), - cwd: project.to_path_buf(), - input_overrides: HashMap::from([( - "goal_file".to_string(), - toml::Value::String("goal.md".to_string()), - )]), - ..Default::default() - }) - .unwrap(); - - let goal = built.manifest.goal.expect("manifest goal should resolve"); - assert_eq!(goal.path.as_deref(), Some("prompts/goal.md")); - assert_eq!(goal.text, "ship it"); - assert_eq!(goal.type_, types::ManifestGoalType::Graph); - let root = &built.manifest.workflows[".fabro/workflows/demo/workflow.fabro"]; - assert!( - root.source.contains("{{ inputs.goal_file }}"), - "manifest should store original workflow source" - ); - } - - /// A relative `[run.goal] file = "..."` declared in `.fabro/project.toml` - /// must resolve against the directory of `.fabro/project.toml`, not against - /// the invocation cwd. We exercise this by invoking from a subdirectory - /// below the project root. - #[test] - fn build_manifest_resolves_relative_goal_file_in_project_config() { - let temp = tempfile::tempdir().unwrap(); - let project = temp.path(); - let workflow_dir = project.join(".fabro/workflows/demo"); - std::fs::create_dir_all(&workflow_dir).unwrap(); - std::fs::create_dir_all(project.join(".fabro/prompts")).unwrap(); - - std::fs::write( - project.join(".fabro/project.toml"), - r#"_version = 1 - -[run.goal] -file = "prompts/goal.md" -"#, - ) - .unwrap(); - std::fs::write( - project.join(".fabro/prompts/goal.md"), - "ship from project root", - ) - .unwrap(); - - std::fs::write( - workflow_dir.join("workflow.toml"), - "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n", - ) - .unwrap(); - std::fs::write( - workflow_dir.join("workflow.fabro"), - r"digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", - ) - .unwrap(); - - let built = build_run_manifest(ManifestBuildInput { - workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"), - cwd: project.to_path_buf(), - ..Default::default() - }) - .unwrap(); - - let goal = built.manifest.goal.expect("manifest goal should be set"); - assert_eq!(goal.text, "ship from project root"); - assert_eq!(goal.type_, types::ManifestGoalType::File); - let resolved = goal.path.expect("file goal must carry a path"); - let expected = project.join(".fabro").join("prompts").join("goal.md"); - assert_eq!(PathBuf::from(resolved), expected); - } - - /// A relative `[run.goal] file = "..."` declared in `workflow.toml` - /// must resolve against the directory of `workflow.toml`, not against - /// the invocation cwd or project root. - #[test] - fn build_manifest_resolves_relative_goal_file_in_workflow_config() { - let temp = tempfile::tempdir().unwrap(); - let project = temp.path(); - let workflow_dir = project.join(".fabro/workflows/demo"); - std::fs::create_dir_all(workflow_dir.join("prompts")).unwrap(); - - std::fs::write(project.join(".fabro/project.toml"), "_version = 1\n").unwrap(); - std::fs::write( - workflow_dir.join("workflow.toml"), - r#"_version = 1 - -[workflow] -graph = "workflow.fabro" - -[run.goal] -file = "prompts/goal.md" -"#, - ) - .unwrap(); - std::fs::write( - workflow_dir.join("prompts/goal.md"), - "ship from workflow dir", - ) - .unwrap(); - std::fs::write( - workflow_dir.join("workflow.fabro"), - r"digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", - ) - .unwrap(); - - let built = build_run_manifest(ManifestBuildInput { - workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"), - cwd: project.to_path_buf(), - ..Default::default() - }) - .unwrap(); - - let goal = built.manifest.goal.expect("manifest goal should be set"); - assert_eq!(goal.text, "ship from workflow dir"); - assert_eq!(goal.type_, types::ManifestGoalType::File); - let resolved = goal.path.expect("file goal must carry a path"); - let expected = workflow_dir.join("prompts").join("goal.md"); - assert_eq!(PathBuf::from(resolved), expected); - } - - /// When `[run] working_dir` points to a nested git repo, the manifest's - /// `git.branch` and `git.origin_url` must come from that target repo, not - /// from an enclosing workspace repo that happens to be the CLI's cwd. - /// Regression test for https://github.com/fabro-sh/fabro/issues/159. - #[test] - fn build_manifest_git_follows_working_directory_into_nested_repo() { - let temp = tempfile::tempdir().unwrap(); - let workspace = temp.path(); - let target = workspace.join("repos").join("target"); - std::fs::create_dir_all(&target).unwrap(); - - init_git_repo( - workspace, - "workspace-branch", - "https://github.com/example/workspace.git", - ); - mark_origin_branch_synced(workspace, "workspace-branch"); - init_git_repo( - &target, - "target-branch", - "https://github.com/example/target.git", - ); - mark_origin_branch_synced(&target, "target-branch"); - - let workflow_dir = workspace.join(".fabro/workflows/demo"); - std::fs::create_dir_all(&workflow_dir).unwrap(); - std::fs::write( - workspace.join(".fabro/project.toml"), - r#"_version = 1 - -[run] -working_dir = "repos/target" -"#, - ) - .unwrap(); - std::fs::write( - workflow_dir.join("workflow.toml"), - "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n", - ) - .unwrap(); - std::fs::write( - workflow_dir.join("workflow.fabro"), - r"digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", - ) - .unwrap(); - - let built = build_run_manifest(ManifestBuildInput { - workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"), - cwd: workspace.to_path_buf(), - ..Default::default() - }) - .unwrap(); - - let git = built - .manifest - .git - .expect("manifest git info should be detected"); - assert_eq!(git.branch, "target-branch"); - assert_eq!(git.origin_url, "https://github.com/example/target"); - assert_eq!(git.push_outcome, PreRunPushOutcome::NotAttempted); - } - - #[test] - fn build_manifest_git_skips_push_when_configured_repository_differs_from_origin() { - let temp = tempfile::tempdir().unwrap(); - let workspace = temp.path(); - - init_git_repo( - workspace, - "feature", - "https://github.com/user/forked-target.git", - ); - - let workflow_dir = workspace.join(".fabro/workflows/demo"); - std::fs::create_dir_all(&workflow_dir).unwrap(); - std::fs::write( - workspace.join(".fabro/project.toml"), - r#"_version = 1 - -[run.scm] -provider = "github" -owner = "example" -repository = "target" -"#, - ) - .unwrap(); - std::fs::write( - workflow_dir.join("workflow.toml"), - "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n", - ) - .unwrap(); - std::fs::write( - workflow_dir.join("workflow.fabro"), - r"digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", - ) - .unwrap(); - - let built = build_run_manifest(ManifestBuildInput { - workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"), - cwd: workspace.to_path_buf(), - ..Default::default() - }) - .unwrap(); - - let git = built - .manifest - .git - .expect("manifest git info should be detected"); - assert_eq!(git.origin_url, "https://github.com/example/target"); - assert_eq!(git.push_outcome, PreRunPushOutcome::SkippedRemoteMismatch { - remote: "https://github.com/user/forked-target".to_string(), - repo_origin_url: "https://github.com/example/target".to_string(), - }); - } - - #[cfg(unix)] - #[test] - fn build_manifest_push_attempt_disables_terminal_prompts() { - use std::os::unix::fs::PermissionsExt; - - let temp = tempfile::tempdir().unwrap(); - let workspace = temp.path().join("workspace"); - std::fs::create_dir_all(&workspace).unwrap(); - init_git_repo(&workspace, "feature", "fabro-prompt-test::target"); - - let helper_dir = temp.path().join("bin"); - std::fs::create_dir_all(&helper_dir).unwrap(); - let helper_path = helper_dir.join("git-remote-fabro-prompt-test"); - std::fs::write( - &helper_path, - r#"#!/bin/sh -printf '%s\n' "${GIT_TERMINAL_PROMPT-unset}" > "$FABRO_PROMPT_ENV_LOG" -echo "helper saw GIT_TERMINAL_PROMPT=${GIT_TERMINAL_PROMPT-unset}" >&2 -exit 1 -"#, - ) - .unwrap(); - let mut permissions = std::fs::metadata(&helper_path).unwrap().permissions(); - permissions.set_mode(0o755); - std::fs::set_permissions(&helper_path, permissions).unwrap(); - - let workflow_dir = workspace.join(".fabro/workflows/demo"); - std::fs::create_dir_all(&workflow_dir).unwrap(); - std::fs::write(workspace.join(".fabro/project.toml"), "_version = 1\n").unwrap(); - std::fs::write( - workflow_dir.join("workflow.toml"), - "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n", - ) - .unwrap(); - std::fs::write( - workflow_dir.join("workflow.fabro"), - r"digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", - ) - .unwrap(); - - let helper_log = temp.path().join("prompt-env.txt"); - let mut path_entries = vec![helper_dir]; - if let Some(path) = std::env::var_os("PATH") { - path_entries.extend(std::env::split_paths(&path)); - } - let path = std::env::join_paths(path_entries).unwrap(); - temp_env::with_var("PATH", Some(path), || { - temp_env::with_var("FABRO_PROMPT_ENV_LOG", Some(helper_log.as_os_str()), || { - let built = build_run_manifest(ManifestBuildInput { - workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"), - cwd: workspace.clone(), - ..Default::default() - }) - .unwrap(); - - let git = built - .manifest - .git - .expect("manifest git info should be detected"); - assert!(matches!(git.push_outcome, PreRunPushOutcome::Failed { .. })); - }); - }); - - assert_eq!(std::fs::read_to_string(helper_log).unwrap(), "0\n"); - } - - fn init_git_repo(path: &Path, branch: &str, origin_url: &str) { - run_git(path, &[ - "-c", - &format!("init.defaultBranch={branch}"), - "init", - "--quiet", - ]); - run_git(path, &[ - "-c", - "user.name=test", - "-c", - "user.email=test@example.com", - "commit", - "--allow-empty", - "--quiet", - "-m", - "init", - ]); - run_git(path, &["remote", "add", "origin", origin_url]); - } - - fn mark_origin_branch_synced(path: &Path, branch: &str) { - let remote_ref = format!("refs/remotes/origin/{branch}"); - run_git(path, &["update-ref", &remote_ref, "HEAD"]); - } - - fn run_git(path: &Path, args: &[&str]) { - use std::process::Command; - let output = Command::new("git") - .args(args) - .current_dir(path) - .output() - .unwrap_or_else(|e| panic!("failed to spawn git {args:?}: {e}")); - assert!( - output.status.success(), - "git {args:?} failed: stdout={} stderr={}", - String::from_utf8_lossy(&output.stdout), - String::from_utf8_lossy(&output.stderr), - ); - } + (!fabro_manifest::manifest_args_is_empty(&payload)).then_some(payload) } diff --git a/lib/crates/fabro-cli/tests/it/cmd/mcp.rs b/lib/crates/fabro-cli/tests/it/cmd/mcp.rs index 1a2274f23..0693ba02d 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/mcp.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/mcp.rs @@ -15,8 +15,11 @@ use std::process::Stdio; use fabro_mcp::client::McpClient; use fabro_mcp::config::{McpServerSettings, McpTransport}; use fabro_test::{fabro_json_snapshot, fabro_snapshot, test_context}; +use httpmock::Method::{GET, POST}; +use httpmock::MockServer; -use crate::support::{RealAuthHarness, TEST_DEV_TOKEN, seed_dev_token_auth}; +use super::support::mock_resolved_run; +use crate::support::{RealAuthHarness, TEST_DEV_TOKEN, seed_dev_token_auth, unique_run_id}; #[test] fn help() { @@ -481,8 +484,8 @@ async fn mcp_create_and_search_manage_real_runs_with_cli_auth() { "source": "mcp-test" }, "source_directory": "[SOURCE_DIRECTORY]", - "repo_origin_url": null, - "goal": "Run the Fabro workflow." + "repo_origin_url": "[REPO_ORIGIN_URL]", + "goal": "Run tests and report results" } ], "next_cursor": null @@ -496,6 +499,100 @@ async fn mcp_create_and_search_manage_real_runs_with_cli_auth() { harness.shutdown().await; } +#[tokio::test(flavor = "multi_thread")] +async fn mcp_run_tools_use_default_local_server_without_server_flag() { + let context = test_context!(); + let workflow = context.install_fixture("simple.fabro"); + let client = spawn_mcp_client(&context, &[]).await; + + let create = call_tool_json( + &client, + "fabro_run_create", + serde_json::json!({ + "runs": [{ + "workflow": workflow, + "dry_run": true, + "auto_approve": true, + "labels": { "source": "mcp-default-server-test" }, + "start": false + }] + }), + ) + .await; + let run_id = create["runs"][0]["run_id"].as_str().unwrap(); + let search = call_tool_json( + &client, + "fabro_run_search", + serde_json::json!({ "run_ids": [run_id], "first": 1 }), + ) + .await; + + assert_eq!(search["runs"][0]["run_id"], run_id); + assert_eq!( + search["runs"][0]["labels"]["source"], + "mcp-default-server-test" + ); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_search_filters_status_dates_and_paginates() { + let context = test_context!(); + let harness = + RealAuthHarness::start_with_dev_token(fabro_test::GitHubAppState::default()).await; + let target_url = harness.api_target(); + let target: fabro_client::ServerTarget = target_url.parse().unwrap(); + seed_dev_token_auth(&context.home_dir, &target, TEST_DEV_TOKEN); + let workflow = context.install_fixture("simple.fabro"); + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + let first = create_mcp_run(&client, workflow.clone(), false).await; + let second = create_mcp_run(&client, workflow, false).await; + + let page_one = call_tool_json( + &client, + "fabro_run_search", + serde_json::json!({ + "labels": { "source": "mcp-test" }, + "status": ["submitted"], + "archived": false, + "created_after": "2000-01-01", + "created_before": "2100-01-01T00:00:00Z", + "first": 1 + }), + ) + .await; + let cursor = page_one["next_cursor"] + .as_str() + .expect("first page should have cursor"); + let page_two = call_tool_json( + &client, + "fabro_run_search", + serde_json::json!({ + "labels": { "source": "mcp-test" }, + "status": ["submitted"], + "archived": false, + "after": cursor, + "first": 1 + }), + ) + .await; + + let page_one_id = page_one["runs"][0]["run_id"].as_str().unwrap(); + let page_two_id = page_two["runs"][0]["run_id"].as_str().unwrap(); + assert_ne!(page_one_id, page_two_id); + assert!([first.as_str(), second.as_str()].contains(&page_one_id)); + assert!([first.as_str(), second.as_str()].contains(&page_two_id)); + + client + .shutdown() + .await + .expect("MCP client should shut down"); + harness.shutdown().await; +} + #[tokio::test(flavor = "multi_thread")] async fn mcp_lifecycle_tools_manage_real_run() { let context = test_context!(); @@ -584,8 +681,8 @@ async fn mcp_lifecycle_tools_manage_real_run() { "source": "mcp-test" }, "source_directory": "[SOURCE_DIRECTORY]", - "repo_origin_url": null, - "goal": "Run the Fabro workflow." + "repo_origin_url": "[REPO_ORIGIN_URL]", + "goal": "Run tests and report results" } ], "timed_out": false, @@ -686,6 +783,253 @@ async fn mcp_interact_error_does_not_stop_server() { .expect("MCP client should shut down"); } +#[tokio::test(flavor = "multi_thread")] +async fn mcp_create_validation_errors_happen_before_auth_or_network() { + let context = test_context!(); + let client = spawn_mcp_client(&context, &["--server", "http://127.0.0.1:9"]).await; + let too_many = (0..51) + .map(|index| serde_json::json!({ "workflow": format!("wf-{index}.fabro") })) + .collect::>(); + + let empty = call_tool_error_text( + &client, + "fabro_run_create", + serde_json::json!({ "runs": [] }), + ) + .await; + let many = call_tool_error_text( + &client, + "fabro_run_create", + serde_json::json!({ "runs": too_many }), + ) + .await; + let null = call_tool_error_text( + &client, + "fabro_run_create", + serde_json::json!({ + "runs": [{ + "workflow": "simple.fabro", + "inputs": { "decision": null } + }] + }), + ) + .await; + + assert!(empty.contains("runs"), "{empty}"); + assert!(many.contains("runs"), "{many}"); + assert!(null.contains("decision"), "{null}"); + assert_eq!(client.list_tools().await.unwrap().len(), 5); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_interact_questions_and_answers_use_api_wire_contract() { + let context = test_context!(); + let server = MockServer::start(); + let target_url = format!("{}/api/v1", server.base_url()); + let target: fabro_client::ServerTarget = target_url.parse().unwrap(); + seed_dev_token_auth(&context.home_dir, &target, TEST_DEV_TOKEN); + let run_id = unique_run_id(); + let selector = "nightly"; + let resolve = mock_resolved_run(&server, selector, &run_id); + let questions = server.mock(|when, then| { + when.method(GET) + .path(format!("/api/v1/runs/{run_id}/questions")) + .query_param("page[limit]", "100") + .query_param("page[offset]", "0"); + then.status(200) + .header("Content-Type", "application/json") + .json_body(serde_json::json!({ + "data": [{ + "id": "q-1", + "text": "Proceed?", + "stage": "gate", + "question_type": "yes_no", + "options": [], + "allow_freeform": false, + "timeout_seconds": null, + "context_display": null + }], + "meta": { "has_more": false } + })); + }); + let expected_answers = [ + ( + serde_json::json!(true), + serde_json::json!({ "kind": "yes" }), + ), + ( + serde_json::json!(false), + serde_json::json!({ "kind": "no" }), + ), + ( + serde_json::json!("Looks good"), + serde_json::json!({ "kind": "text", "text": "Looks good" }), + ), + ( + serde_json::json!({ "option": "approve" }), + serde_json::json!({ "kind": "selected", "option_key": "approve" }), + ), + ( + serde_json::json!({ "options": ["approve", "notify"] }), + serde_json::json!({ "kind": "multi_selected", "option_keys": ["approve", "notify"] }), + ), + ( + serde_json::json!({ "text": "Freeform" }), + serde_json::json!({ "kind": "text", "text": "Freeform" }), + ), + ]; + let answer_mocks = expected_answers + .iter() + .map(|(_, expected_body)| { + server.mock(|when, then| { + when.method(POST) + .path(format!("/api/v1/runs/{run_id}/questions/q-1/answer")) + .json_body(expected_body.clone()); + then.status(204); + }) + }) + .collect::>(); + + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + let question_result = call_tool_json( + &client, + "fabro_run_interact", + serde_json::json!({ "run_id": selector, "action": "get_questions" }), + ) + .await; + assert_eq!(question_result["result"]["questions"][0]["id"], "q-1"); + + for (answer, _) in expected_answers { + let result = call_tool_json( + &client, + "fabro_run_interact", + serde_json::json!({ + "run_id": selector, + "action": "answer", + "question_id": "q-1", + "answer": answer + }), + ) + .await; + assert_eq!(result["result"]["submitted"], true); + } + + resolve.assert_calls(7); + questions.assert(); + for answer in answer_mocks { + answer.assert(); + } + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_events_filters_find_matches_beyond_first_page() { + let context = test_context!(); + let server = MockServer::start(); + let target_url = format!("{}/api/v1", server.base_url()); + let target: fabro_client::ServerTarget = target_url.parse().unwrap(); + seed_dev_token_auth(&context.home_dir, &target, TEST_DEV_TOKEN); + let run_id = unique_run_id(); + let resolve = mock_resolved_run(&server, "nightly", &run_id); + let events = (1..=60) + .map(|sequence| { + let event_name = if sequence == 60 { + "stage.started" + } else { + "run.started" + }; + let properties = if sequence == 60 { + serde_json::json!({ + "index": 1, + "handler_type": "prompt", + "attempt": 1, + "max_attempts": 1 + }) + } else { + serde_json::json!({ + "name": "Simple", + "goal": format!("ordinary event {sequence}") + }) + }; + serde_json::json!({ + "seq": sequence, + "id": format!("evt-{sequence}"), + "ts": "2026-04-05T12:00:00Z", + "run_id": run_id, + "event": event_name, + "properties": properties, + "actor": null + }) + }) + .collect::>(); + let first_event = events[0].clone(); + let _limited_events = server.mock(|when, then| { + when.method(GET) + .path(format!("/api/v1/runs/{run_id}/events")) + .query_param("limit", "1"); + then.status(200) + .header("Content-Type", "application/json") + .json_body(serde_json::json!({ + "data": [first_event], + "meta": { "has_more": true } + })); + }); + let list_events = server.mock(|when, then| { + when.method(GET) + .path(format!("/api/v1/runs/{run_id}/events")) + .query_param_missing("limit"); + then.status(200) + .header("Content-Type", "application/json") + .json_body(serde_json::json!({ + "data": events, + "meta": { "has_more": false } + })); + }); + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + + let details = call_tool_json( + &client, + "fabro_run_events", + serde_json::json!({ + "run_id": "nightly", + "action": "details", + "event_ids": ["evt-60"], + "first": 1 + }), + ) + .await; + let filtered = call_tool_json( + &client, + "fabro_run_events", + serde_json::json!({ + "run_id": "nightly", + "action": "search", + "categories": ["stage"], + "query": "prompt", + "first": 1, + "max_content_length": 32 + }), + ) + .await; + + assert_eq!(details["events"][0]["event_id"], "evt-60"); + assert_eq!(filtered["events"][0]["event_id"], "evt-60"); + assert_eq!(filtered["events"][0]["truncated"], true); + resolve.assert_calls(2); + list_events.assert_calls(2); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + #[tokio::test(flavor = "multi_thread")] async fn mcp_tool_auth_error_mentions_login() { let context = test_context!(); @@ -865,6 +1209,9 @@ fn normalize_run_search(mut value: serde_json::Value) -> serde_json::Value { if run["source_directory"].is_string() { run["source_directory"] = serde_json::json!("[SOURCE_DIRECTORY]"); } + if run["repo_origin_url"].is_string() { + run["repo_origin_url"] = serde_json::json!("[REPO_ORIGIN_URL]"); + } } } value @@ -885,6 +1232,9 @@ fn normalize_gather(mut value: serde_json::Value) -> serde_json::Value { if run["source_directory"].is_string() { run["source_directory"] = serde_json::json!("[SOURCE_DIRECTORY]"); } + if run["repo_origin_url"].is_string() { + run["repo_origin_url"] = serde_json::json!("[REPO_ORIGIN_URL]"); + } } } value diff --git a/lib/crates/fabro-manifest/Cargo.toml b/lib/crates/fabro-manifest/Cargo.toml new file mode 100644 index 000000000..417dc9af0 --- /dev/null +++ b/lib/crates/fabro-manifest/Cargo.toml @@ -0,0 +1,29 @@ +[package] +name = "fabro-manifest" +edition.workspace = true +version.workspace = true +publish = false +license.workspace = true +description = "Fabro run manifest construction" + +[lib] +doctest = false + +[lints] +workspace = true + +[dependencies] +anyhow.workspace = true +fabro-api = { path = "../fabro-api" } +fabro-config = { path = "../fabro-config" } +fabro-github = { path = "../fabro-github" } +fabro-graphviz = { path = "../fabro-graphviz" } +fabro-template = { path = "../fabro-template" } +fabro-types = { path = "../fabro-types" } +fabro-workflow = { path = "../fabro-workflow" } +git2.workspace = true +toml.workspace = true + +[dev-dependencies] +tempfile = "3" +temp-env = "0.3" diff --git a/lib/crates/fabro-manifest/src/lib.rs b/lib/crates/fabro-manifest/src/lib.rs new file mode 100644 index 000000000..5757d4b5b --- /dev/null +++ b/lib/crates/fabro-manifest/src/lib.rs @@ -0,0 +1,1168 @@ +#![expect( + clippy::disallowed_methods, + reason = "CLI manifest builder: sync file I/O building install manifests" +)] + +use std::collections::{HashMap, HashSet}; +use std::path::{Component, Path, PathBuf}; + +use anyhow::{Context, Result, anyhow}; +use fabro_api::types; +use fabro_config::project::{self, discover_project_config, resolve_workflow_path}; +use fabro_config::run::{resolve_run_goal_from_layer, resolve_run_goal_from_namespace}; +use fabro_config::{CliLayer, DaytonaDockerfileLayer, RunLayer, WorkflowSettingsBuilder}; +use fabro_graphviz::graph::AttrValue; +use fabro_graphviz::parser; +use fabro_template::{TemplateContext, render as render_template}; +use fabro_types::settings::run::{ResolvedGoalSource, ResolvedRunGoal}; +use fabro_types::{DirtyStatus, GitContext, PreRunPushOutcome, RunId, WorkflowSettings}; +use fabro_workflow::ManifestPath; +use fabro_workflow::git::{ + GitSyncStatus, branch_needs_push, head_sha, push_branch_noninteractive, sync_status, +}; + +#[derive(Debug, Default)] +pub struct ManifestBuildInput { + pub workflow: PathBuf, + pub cwd: PathBuf, + pub run_overrides: Option, + pub cli_overrides: Option, + pub input_overrides: HashMap, + pub args: Option, + pub run_id: Option, + /// Path to the user settings file (for inclusion in + /// `RunManifest.configs`). `None` skips the user config entry. + pub user_settings_path: Option, +} + +#[derive(Debug)] +pub struct BuiltManifest { + pub manifest: types::RunManifest, + pub target_path: PathBuf, +} + +struct CollectContext<'a> { + cwd: &'a Path, + inputs: &'a HashMap, + workflows: HashMap, + visited_workflows: HashSet, +} + +#[derive(Clone)] +struct WorkflowScanInput { + absolute_dot_path: PathBuf, + dot_path: ManifestPath, + source: String, +} + +pub fn build_run_manifest(input: ManifestBuildInput) -> Result { + let root_resolution = resolve_workflow_path(&input.workflow, &input.cwd)?; + if root_resolution.workflow_toml_path.is_none() + && !root_resolution.resolved_workflow_path.is_file() + { + return Err(fabro_config::Error::WorkflowNotFound( + root_resolution.resolved_workflow_path.display().to_string(), + ) + .into()); + } + let workflow_parent = root_resolution + .resolved_workflow_path + .parent() + .unwrap_or_else(|| Path::new(".")); + let project_config = discover_project_config(workflow_parent)?; + let mut workflow_settings_builder = WorkflowSettingsBuilder::new(); + if let Some(run) = input.run_overrides.clone() { + workflow_settings_builder = workflow_settings_builder.run_overrides(run); + } + if let Some(cli) = input.cli_overrides.clone() { + workflow_settings_builder = workflow_settings_builder.cli_overrides(cli); + } + if let Some(path) = root_resolution.workflow_toml_path.as_ref() { + workflow_settings_builder = workflow_settings_builder.workflow_file(path)?; + } + if let Some(path) = project_config.as_ref() { + workflow_settings_builder = workflow_settings_builder.project_file(path)?; + } + if let Some(path) = input + .user_settings_path + .as_ref() + .filter(|path| path.is_file()) + { + workflow_settings_builder = workflow_settings_builder.user_file(path)?; + } + let mut workflow_settings = workflow_settings_builder + .build() + .context("failed to resolve manifest settings")?; + workflow_settings.run.inputs.extend(input.input_overrides); + let target_path = root_resolution.dot_path.clone(); + let target_manifest_path = manifest_path_from_absolute(&target_path, &input.cwd)?; + let target_key = target_manifest_path.to_string(); + + let mut context = CollectContext { + cwd: &input.cwd, + inputs: &workflow_settings.run.inputs, + workflows: HashMap::new(), + visited_workflows: HashSet::new(), + }; + collect_workflow_entry(&mut context, &input.workflow, &input.cwd)?; + + let root_source = context + .workflows + .get(&target_key) + .map(|workflow| workflow.source.clone()) + .ok_or_else(|| anyhow!("root workflow missing from manifest bundle"))?; + + let mut configs = Vec::new(); + if let Some(path) = project_config { + let source = std::fs::read_to_string(&path) + .with_context(|| format!("Failed to read {}", path.display()))?; + configs.push(types::ManifestConfig { + path: Some(path.display().to_string()), + source: Some(source), + type_: types::ManifestConfigType::Project, + }); + } + if let Some(path) = input.user_settings_path.filter(|p| p.is_file()) { + let source = std::fs::read_to_string(&path) + .with_context(|| format!("Failed to read {}", path.display()))?; + configs.push(types::ManifestConfig { + path: Some(path.display().to_string()), + source: Some(source), + type_: types::ManifestConfigType::User, + }); + } + + let working_directory = + project::resolve_working_directory_from_run(&workflow_settings.run, &input.cwd); + + let rendered_root_source = + render_workflow_scan_source(&root_source, &target_path, &workflow_settings.run.inputs)?; + + let goal = resolve_manifest_goal( + input.run_overrides.as_ref(), + &workflow_settings, + &rendered_root_source, + &target_path, + &working_directory, + )?; + + let configured_repo_origin_url = configured_repo_origin_url(&workflow_settings); + let git = build_git_context(&working_directory, configured_repo_origin_url.as_deref()); + let args = input.args.filter(|args| !manifest_args_is_empty(args)); + + Ok(BuiltManifest { + manifest: types::RunManifest { + args, + configs, + cwd: input.cwd.display().to_string(), + git, + goal, + run_id: input.run_id.map(|run_id| run_id.to_string()), + title: None, + target: types::ManifestTarget { + identifier: input.workflow.display().to_string(), + path: target_key, + }, + version: 1, + workflows: context.workflows, + }, + target_path, + }) +} + +fn collect_workflow_entry( + context: &mut CollectContext<'_>, + workflow: &Path, + resolve_from: &Path, +) -> Result<()> { + let normalized_workflow = if workflow.extension().is_some() && workflow.is_relative() { + normalize_absolute_path(resolve_from, &workflow.to_string_lossy()).ok_or_else(|| { + anyhow!( + "unsupported manifest workflow reference: {}", + workflow.display() + ) + })? + } else { + workflow.to_path_buf() + }; + let resolution = resolve_workflow_path(&normalized_workflow, resolve_from)?; + let dot_path = manifest_path_from_absolute(&resolution.dot_path, context.cwd)?; + let dot_key = dot_path.to_string(); + if !context.visited_workflows.insert(dot_key.clone()) { + return Ok(()); + } + + let source = std::fs::read_to_string(&resolution.dot_path) + .with_context(|| format!("Failed to read {}", resolution.dot_path.display()))?; + let config = if let Some(workflow_toml_path) = resolution.workflow_toml_path.as_ref() { + Some(types::ManifestWorkflowConfig { + path: manifest_path_from_absolute(workflow_toml_path, context.cwd)?.to_string(), + source: std::fs::read_to_string(workflow_toml_path) + .with_context(|| format!("Failed to read {}", workflow_toml_path.display()))?, + }) + } else { + None + }; + + let scan = WorkflowScanInput { + absolute_dot_path: resolution.dot_path, + dot_path, + source: source.clone(), + }; + let mut files = HashMap::new(); + let mut visited_imports = HashSet::new(); + if let Some(config) = config.as_ref() { + collect_workflow_config_files(context, config, &mut files)?; + } + collect_workflow_files(context, &scan, &mut files, &mut visited_imports)?; + + context.workflows.insert(dot_key, types::ManifestWorkflow { + config, + files, + source, + }); + + Ok(()) +} + +fn collect_workflow_files( + context: &mut CollectContext<'_>, + workflow: &WorkflowScanInput, + files: &mut HashMap, + visited_imports: &mut HashSet, +) -> Result<()> { + let rendered_source = render_workflow_scan_source( + &workflow.source, + &workflow.absolute_dot_path, + context.inputs, + )?; + let graph = parser::parse(&rendered_source).map_err(|err| { + anyhow!( + "Failed to parse {}: {err}", + workflow.absolute_dot_path.display() + ) + })?; + + if let Some(goal_ref) = graph.attrs.get("goal").and_then(AttrValue::as_str) { + if goal_ref.starts_with('@') { + collect_bundled_file( + files, + workflow + .absolute_dot_path + .parent() + .unwrap_or_else(|| Path::new(".")), + context.cwd, + goal_ref.trim_start_matches('@'), + types::ManifestFileRefType::FileInline, + Some(workflow.dot_path.clone()), + )?; + } + } + + for node in graph.nodes.values() { + if let Some(prompt_ref) = node.attrs.get("prompt").and_then(AttrValue::as_str) { + if prompt_ref.starts_with('@') { + collect_bundled_file( + files, + workflow + .absolute_dot_path + .parent() + .unwrap_or_else(|| Path::new(".")), + context.cwd, + prompt_ref.trim_start_matches('@'), + types::ManifestFileRefType::FileInline, + Some(workflow.dot_path.clone()), + )?; + } + } + + if let Some(import_ref) = node.attrs.get("import").and_then(AttrValue::as_str) { + let imported = collect_bundled_file( + files, + workflow + .absolute_dot_path + .parent() + .unwrap_or_else(|| Path::new(".")), + context.cwd, + import_ref, + types::ManifestFileRefType::Import, + Some(workflow.dot_path.clone()), + )?; + let import_key = imported.path.to_string(); + if visited_imports.insert(import_key) { + let imported_source = std::fs::read_to_string(&imported.absolute_path) + .with_context(|| { + format!("Failed to read {}", imported.absolute_path.display()) + })?; + let imported_scan = WorkflowScanInput { + absolute_dot_path: imported.absolute_path, + dot_path: imported.path, + source: imported_source, + }; + collect_workflow_files(context, &imported_scan, files, visited_imports)?; + } + } + + if let Some(child_ref) = node + .attrs + .get("stack.child_workflow") + .or_else(|| node.attrs.get("stack.child_dotfile")) + .and_then(AttrValue::as_str) + { + collect_workflow_entry( + context, + Path::new(child_ref), + workflow + .absolute_dot_path + .parent() + .unwrap_or_else(|| Path::new(".")), + )?; + } + } + + Ok(()) +} + +fn render_workflow_scan_source( + source: &str, + path: &Path, + inputs: &HashMap, +) -> Result { + render_template(source, &TemplateContext::for_input_scan(inputs.clone())) + .with_context(|| format!("Failed to render {} for manifest scanning", path.display())) +} + +fn collect_workflow_config_files( + context: &CollectContext<'_>, + config: &types::ManifestWorkflowConfig, + files: &mut HashMap, +) -> Result<()> { + let mut document: toml::Table = config + .source + .parse() + .context("Failed to parse run config TOML")?; + let run = document + .remove("run") + .map(toml::Value::try_into::) + .transpose() + .context("Failed to parse run config TOML")? + .unwrap_or_default(); + let dockerfile = run + .sandbox + .as_ref() + .and_then(|sandbox| sandbox.daytona.as_ref()) + .and_then(|daytona| daytona.snapshot.as_ref()) + .and_then(|snapshot| snapshot.dockerfile.as_ref()); + + let Some(DaytonaDockerfileLayer::Path { path }) = dockerfile else { + return Ok(()); + }; + + let config_path = ManifestPath::from_wire(&config.path) + .ok_or_else(|| anyhow!("invalid manifest workflow config path: {}", config.path))?; + let absolute_config_path = context.cwd.join(config_path.as_path()); + collect_bundled_file( + files, + absolute_config_path + .parent() + .unwrap_or_else(|| Path::new(".")), + context.cwd, + path, + types::ManifestFileRefType::Dockerfile, + Some(config_path), + )?; + Ok(()) +} + +struct BundledFile { + absolute_path: PathBuf, + path: ManifestPath, +} + +fn collect_bundled_file( + files: &mut HashMap, + base_dir: &Path, + cwd: &Path, + reference: &str, + ref_type: types::ManifestFileRefType, + from: Option, +) -> Result { + let absolute_path = normalize_absolute_path(base_dir, reference) + .ok_or_else(|| anyhow!("unsupported manifest reference: {reference}"))?; + let path = manifest_path_from_absolute(&absolute_path, cwd)?; + let key = path.to_string(); + if !files.contains_key(&key) { + let content = std::fs::read_to_string(&absolute_path) + .with_context(|| format!("Failed to read {}", absolute_path.display()))?; + files.insert(key.clone(), types::ManifestFileEntry { + content, + ref_: types::ManifestFileRef { + from: from.map(|value| value.to_string()), + original: reference.to_string(), + type_: ref_type, + }, + }); + } + + Ok(BundledFile { + absolute_path, + path, + }) +} + +fn resolve_manifest_goal( + run_overrides: Option<&RunLayer>, + settings: &WorkflowSettings, + root_source: &str, + root_dot_path: &Path, + working_directory: &Path, +) -> Result> { + // Precedence 1: CLI args (`--goal` / `--goal-file`). These are already + // resolved to absolute paths by `overrides::goal_layer_from_args`. + if let Some(run_overrides) = run_overrides { + if let Some(resolved) = resolve_run_goal_from_layer(run_overrides, working_directory) + .context("failed to resolve --goal-file contents")? + { + return Ok(Some(resolved_goal_to_manifest(resolved))); + } + } + + // Precedence 2: merged config `run.goal`. Config-sourced `goal.file` + // paths were rewritten to absolute by `load_settings_path` at the + // directory of the config file that declared them. + if let Some(resolved) = resolve_run_goal_from_namespace(&settings.run, working_directory) + .context("failed to resolve run.goal.file contents")? + { + return Ok(Some(resolved_goal_to_manifest(resolved))); + } + + // Precedence 3: graph-level `goal` attribute in the DOT, with `@file` + // sugar for workflow-colocated goal files. + let graph = parser::parse(root_source) + .with_context(|| format!("Failed to parse {}", root_dot_path.display()))?; + let Some(goal) = graph.attrs.get("goal").and_then(AttrValue::as_str) else { + return Ok(None); + }; + if let Some(reference) = goal.strip_prefix('@') { + let goal_path = normalize_absolute_path( + root_dot_path.parent().unwrap_or_else(|| Path::new(".")), + reference, + ) + .ok_or_else(|| anyhow!("unsupported manifest goal reference: {reference}"))?; + return Ok(Some(types::ManifestGoal { + path: Some(reference.to_string()), + text: std::fs::read_to_string(&goal_path) + .with_context(|| format!("Failed to read {}", goal_path.display()))?, + type_: types::ManifestGoalType::Graph, + })); + } + + Ok(Some(types::ManifestGoal { + path: None, + text: goal.to_string(), + type_: types::ManifestGoalType::Graph, + })) +} + +/// Translate a [`ResolvedRunGoal`] into the wire-level `ManifestGoal` +/// shape. Inline goals get `type = Value`; file-sourced goals keep their +/// absolute path as the `path` field and use `type = File`. +fn resolved_goal_to_manifest(resolved: ResolvedRunGoal) -> types::ManifestGoal { + match resolved.source { + ResolvedGoalSource::Inline => types::ManifestGoal { + path: None, + text: resolved.text, + type_: types::ManifestGoalType::Value, + }, + ResolvedGoalSource::File { path } => types::ManifestGoal { + path: Some(path.to_string_lossy().into_owned()), + text: resolved.text, + type_: types::ManifestGoalType::File, + }, + } +} + +fn build_git_context( + repo_path: &Path, + configured_repo_origin_url: Option<&str>, +) -> Option { + let (origin_url, branch) = detect_manifest_repo_info(repo_path)?; + let sha = head_sha(repo_path).ok(); + let dirty = match sync_status(repo_path, "origin", Some(&branch)) { + GitSyncStatus::Dirty => DirtyStatus::Dirty, + GitSyncStatus::Synced | GitSyncStatus::Unsynced => DirtyStatus::Clean, + }; + let repo_origin_url = configured_repo_origin_url + .map(fabro_github::normalize_repo_origin_url) + .filter(|url| !url.is_empty()) + .or_else(|| { + origin_url + .as_deref() + .map(fabro_github::normalize_repo_origin_url) + .filter(|url| !url.is_empty()) + }) + .unwrap_or_default(); + let push_outcome = build_manifest_push_outcome( + repo_path, + &branch, + origin_url.as_deref(), + configured_repo_origin_url, + ); + Some(GitContext { + origin_url: repo_origin_url, + branch, + sha, + dirty, + push_outcome, + }) +} + +fn configured_repo_origin_url(settings: &WorkflowSettings) -> Option { + let scm = &settings.run.scm; + if !scm + .provider + .as_deref() + .is_none_or(|provider| provider.eq_ignore_ascii_case("github")) + { + return None; + } + let owner = scm.owner.as_ref()?.as_source(); + let repository = scm.repository.as_ref()?.as_source(); + if owner.trim().is_empty() || repository.trim().is_empty() { + return None; + } + let origin = format!("https://github.com/{owner}/{repository}"); + let normalized = fabro_github::normalize_repo_origin_url(&origin); + (!normalized.is_empty()).then_some(normalized) +} + +fn detect_manifest_repo_info(repo_path: &Path) -> Option<(Option, String)> { + let repo = git2::Repository::discover(repo_path).ok()?; + let branch = repo.head().ok()?.shorthand().map(ToOwned::to_owned)?; + let origin_url = repo + .find_remote("origin") + .ok() + .and_then(|remote| remote.url().map(ToOwned::to_owned)); + Some((origin_url, branch)) +} + +fn build_manifest_push_outcome( + repo_path: &Path, + branch: &str, + origin_url: Option<&str>, + configured_repo_origin_url: Option<&str>, +) -> PreRunPushOutcome { + let Some(origin_url) = origin_url else { + return PreRunPushOutcome::SkippedNoRemote; + }; + + if let Some(repo_origin_url) = configured_repo_origin_url + .map(fabro_github::normalize_repo_origin_url) + .filter(|url| !url.is_empty()) + { + let remote = fabro_github::normalize_repo_origin_url(origin_url); + if remote != repo_origin_url { + return PreRunPushOutcome::SkippedRemoteMismatch { + remote, + repo_origin_url, + }; + } + } + + if !branch_needs_push(repo_path, "origin", branch) { + return PreRunPushOutcome::NotAttempted; + } + + match push_branch_noninteractive(repo_path, "origin", branch) { + Ok(()) => PreRunPushOutcome::Succeeded { + remote: "origin".to_string(), + branch: branch.to_string(), + }, + Err(err) => PreRunPushOutcome::Failed { + remote: "origin".to_string(), + branch: branch.to_string(), + message: err.to_string(), + }, + } +} + +fn normalize_absolute_path(base_dir: &Path, reference: &str) -> Option { + let path = Path::new(reference); + if path.is_absolute() || reference.starts_with('~') { + return None; + } + + let mut normalized = PathBuf::new(); + for component in base_dir.join(path).components() { + match component { + Component::CurDir => {} + Component::Normal(part) => normalized.push(part), + Component::ParentDir => { + normalized.pop(); + } + Component::RootDir => normalized.push(Path::new("/")), + Component::Prefix(prefix) => normalized.push(prefix.as_os_str()), + } + } + Some(normalized) +} + +fn manifest_path_from_absolute(path: &Path, cwd: &Path) -> Result { + ManifestPath::from_absolute(path, cwd) + .ok_or_else(|| anyhow!("Failed to compute manifest path for {}", path.display())) +} + +pub fn manifest_args_is_empty(args: &types::ManifestArgs) -> bool { + args.auto_approve.is_none() + && args.dry_run.is_none() + && args.label.is_empty() + && args.model.is_none() + && args.preserve_sandbox.is_none() + && args.provider.is_none() + && args.sandbox.is_none() + && args.docker_image.is_none() + && args.input.is_empty() + && args.verbose.is_none() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn build_manifest_bundles_imports_prompts_and_children() { + let temp = tempfile::tempdir().unwrap(); + let project = temp.path(); + let workflow_dir = project.join(".fabro/workflows/demo"); + let child_dir = project.join(".fabro/workflows/child"); + std::fs::create_dir_all(workflow_dir.join("prompts")).unwrap(); + std::fs::create_dir_all(workflow_dir.join("imports")).unwrap(); + std::fs::create_dir_all(&child_dir).unwrap(); + std::fs::write(project.join(".fabro/project.toml"), "_version = 1\n").unwrap(); + std::fs::write( + workflow_dir.join("workflow.toml"), + "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n", + ) + .unwrap(); + std::fs::write( + workflow_dir.join("workflow.fabro"), + r#"digraph Demo { + graph [goal="@prompts/goal.md"] + start [shape=Mdiamond] + exit [shape=Msquare] + plan [prompt="@prompts/plan.md"] + imported [import="./imports/checks.fabro"] + child [shape=house, stack.child_workflow="../child/workflow.fabro"] + start -> plan -> imported -> child -> exit + }"#, + ) + .unwrap(); + std::fs::write(workflow_dir.join("prompts/goal.md"), "ship it").unwrap(); + std::fs::write(workflow_dir.join("prompts/plan.md"), "plan it").unwrap(); + std::fs::write( + workflow_dir.join("imports/checks.fabro"), + r#"digraph Checks { + start [shape=Mdiamond] + exit [shape=Msquare] + lint [prompt="@../prompts/lint.md"] + start -> lint -> exit + }"#, + ) + .unwrap(); + std::fs::write(workflow_dir.join("prompts/lint.md"), "lint it").unwrap(); + std::fs::write( + child_dir.join("workflow.fabro"), + r"digraph Child { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", + ) + .unwrap(); + + let built = build_run_manifest(ManifestBuildInput { + workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"), + cwd: project.to_path_buf(), + ..Default::default() + }) + .unwrap(); + + assert_eq!( + built.manifest.target.path, + ".fabro/workflows/demo/workflow.fabro" + ); + assert_eq!(built.manifest.workflows.len(), 2); + let root = &built.manifest.workflows[".fabro/workflows/demo/workflow.fabro"]; + assert!( + root.files + .contains_key(".fabro/workflows/demo/prompts/goal.md") + ); + assert!( + root.files + .contains_key(".fabro/workflows/demo/prompts/plan.md") + ); + assert!( + root.files + .contains_key(".fabro/workflows/demo/imports/checks.fabro") + ); + assert!( + root.files + .contains_key(".fabro/workflows/demo/prompts/lint.md") + ); + assert_eq!(built.manifest.goal.unwrap().text, "ship it"); + assert!( + built + .manifest + .workflows + .contains_key(".fabro/workflows/child/workflow.fabro") + ); + } + + #[test] + fn build_manifest_uses_input_overrides_for_structural_file_scanning() { + let temp = tempfile::tempdir().unwrap(); + let project = temp.path(); + let workflow_dir = project.join(".fabro/workflows/demo"); + let child_dir = project.join(".fabro/workflows/child"); + std::fs::create_dir_all(workflow_dir.join("prompts")).unwrap(); + std::fs::create_dir_all(workflow_dir.join("imports")).unwrap(); + std::fs::create_dir_all(&child_dir).unwrap(); + std::fs::write(project.join(".fabro/project.toml"), "_version = 1\n").unwrap(); + std::fs::write( + workflow_dir.join("workflow.toml"), + "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n", + ) + .unwrap(); + std::fs::write( + workflow_dir.join("workflow.fabro"), + r#"digraph Demo { + graph [goal="Demo"] + start [shape=Mdiamond] + exit [shape=Msquare] + plan [prompt="@prompts/{{ inputs.prompt_file }}"] + imported [import="./imports/{{ inputs.import_file }}"] + child [shape=house, stack.child_workflow="../{{ inputs.child_workflow }}/workflow.fabro"] + start -> plan -> imported -> child -> exit + }"#, + ) + .unwrap(); + std::fs::write(workflow_dir.join("prompts/plan.md"), "plan it").unwrap(); + std::fs::write( + workflow_dir.join("imports/checks.fabro"), + r"digraph Checks { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", + ) + .unwrap(); + std::fs::write( + child_dir.join("workflow.fabro"), + r"digraph Child { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", + ) + .unwrap(); + + let built = build_run_manifest(ManifestBuildInput { + workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"), + cwd: project.to_path_buf(), + input_overrides: HashMap::from([ + ( + "prompt_file".to_string(), + toml::Value::String("plan.md".to_string()), + ), + ( + "import_file".to_string(), + toml::Value::String("checks.fabro".to_string()), + ), + ( + "child_workflow".to_string(), + toml::Value::String("child".to_string()), + ), + ]), + ..Default::default() + }) + .unwrap(); + + let root = &built.manifest.workflows[".fabro/workflows/demo/workflow.fabro"]; + assert!( + root.source.contains("{{ inputs.prompt_file }}"), + "manifest should store original workflow source" + ); + assert!( + root.files + .contains_key(".fabro/workflows/demo/prompts/plan.md") + ); + assert!( + root.files + .contains_key(".fabro/workflows/demo/imports/checks.fabro") + ); + assert!( + built + .manifest + .workflows + .contains_key(".fabro/workflows/child/workflow.fabro") + ); + } + + #[test] + fn build_manifest_uses_input_overrides_for_graph_goal_file_resolution() { + let temp = tempfile::tempdir().unwrap(); + let project = temp.path(); + let workflow_dir = project.join(".fabro/workflows/demo"); + std::fs::create_dir_all(workflow_dir.join("prompts")).unwrap(); + std::fs::write(project.join(".fabro/project.toml"), "_version = 1\n").unwrap(); + std::fs::write( + workflow_dir.join("workflow.toml"), + "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n", + ) + .unwrap(); + std::fs::write( + workflow_dir.join("workflow.fabro"), + r#"digraph Demo { + graph [goal="@prompts/{{ inputs.goal_file }}"] + start [shape=Mdiamond] + exit [shape=Msquare] + start -> exit + }"#, + ) + .unwrap(); + std::fs::write(workflow_dir.join("prompts/goal.md"), "ship it").unwrap(); + + let built = build_run_manifest(ManifestBuildInput { + workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"), + cwd: project.to_path_buf(), + input_overrides: HashMap::from([( + "goal_file".to_string(), + toml::Value::String("goal.md".to_string()), + )]), + ..Default::default() + }) + .unwrap(); + + let goal = built.manifest.goal.expect("manifest goal should resolve"); + assert_eq!(goal.path.as_deref(), Some("prompts/goal.md")); + assert_eq!(goal.text, "ship it"); + assert_eq!(goal.type_, types::ManifestGoalType::Graph); + let root = &built.manifest.workflows[".fabro/workflows/demo/workflow.fabro"]; + assert!( + root.source.contains("{{ inputs.goal_file }}"), + "manifest should store original workflow source" + ); + } + + /// A relative `[run.goal] file = "..."` declared in `.fabro/project.toml` + /// must resolve against the directory of `.fabro/project.toml`, not against + /// the invocation cwd. We exercise this by invoking from a subdirectory + /// below the project root. + #[test] + fn build_manifest_resolves_relative_goal_file_in_project_config() { + let temp = tempfile::tempdir().unwrap(); + let project = temp.path(); + let workflow_dir = project.join(".fabro/workflows/demo"); + std::fs::create_dir_all(&workflow_dir).unwrap(); + std::fs::create_dir_all(project.join(".fabro/prompts")).unwrap(); + + std::fs::write( + project.join(".fabro/project.toml"), + r#"_version = 1 + +[run.goal] +file = "prompts/goal.md" +"#, + ) + .unwrap(); + std::fs::write( + project.join(".fabro/prompts/goal.md"), + "ship from project root", + ) + .unwrap(); + + std::fs::write( + workflow_dir.join("workflow.toml"), + "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n", + ) + .unwrap(); + std::fs::write( + workflow_dir.join("workflow.fabro"), + r"digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", + ) + .unwrap(); + + let built = build_run_manifest(ManifestBuildInput { + workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"), + cwd: project.to_path_buf(), + ..Default::default() + }) + .unwrap(); + + let goal = built.manifest.goal.expect("manifest goal should be set"); + assert_eq!(goal.text, "ship from project root"); + assert_eq!(goal.type_, types::ManifestGoalType::File); + let resolved = goal.path.expect("file goal must carry a path"); + let expected = project.join(".fabro").join("prompts").join("goal.md"); + assert_eq!(PathBuf::from(resolved), expected); + } + + /// A relative `[run.goal] file = "..."` declared in `workflow.toml` + /// must resolve against the directory of `workflow.toml`, not against + /// the invocation cwd or project root. + #[test] + fn build_manifest_resolves_relative_goal_file_in_workflow_config() { + let temp = tempfile::tempdir().unwrap(); + let project = temp.path(); + let workflow_dir = project.join(".fabro/workflows/demo"); + std::fs::create_dir_all(workflow_dir.join("prompts")).unwrap(); + + std::fs::write(project.join(".fabro/project.toml"), "_version = 1\n").unwrap(); + std::fs::write( + workflow_dir.join("workflow.toml"), + r#"_version = 1 + +[workflow] +graph = "workflow.fabro" + +[run.goal] +file = "prompts/goal.md" +"#, + ) + .unwrap(); + std::fs::write( + workflow_dir.join("prompts/goal.md"), + "ship from workflow dir", + ) + .unwrap(); + std::fs::write( + workflow_dir.join("workflow.fabro"), + r"digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", + ) + .unwrap(); + + let built = build_run_manifest(ManifestBuildInput { + workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"), + cwd: project.to_path_buf(), + ..Default::default() + }) + .unwrap(); + + let goal = built.manifest.goal.expect("manifest goal should be set"); + assert_eq!(goal.text, "ship from workflow dir"); + assert_eq!(goal.type_, types::ManifestGoalType::File); + let resolved = goal.path.expect("file goal must carry a path"); + let expected = workflow_dir.join("prompts").join("goal.md"); + assert_eq!(PathBuf::from(resolved), expected); + } + + /// When `[run] working_dir` points to a nested git repo, the manifest's + /// `git.branch` and `git.origin_url` must come from that target repo, not + /// from an enclosing workspace repo that happens to be the CLI's cwd. + /// Regression test for https://github.com/fabro-sh/fabro/issues/159. + #[test] + fn build_manifest_git_follows_working_directory_into_nested_repo() { + let temp = tempfile::tempdir().unwrap(); + let workspace = temp.path(); + let target = workspace.join("repos").join("target"); + std::fs::create_dir_all(&target).unwrap(); + + init_git_repo( + workspace, + "workspace-branch", + "https://github.com/example/workspace.git", + ); + mark_origin_branch_synced(workspace, "workspace-branch"); + init_git_repo( + &target, + "target-branch", + "https://github.com/example/target.git", + ); + mark_origin_branch_synced(&target, "target-branch"); + + let workflow_dir = workspace.join(".fabro/workflows/demo"); + std::fs::create_dir_all(&workflow_dir).unwrap(); + std::fs::write( + workspace.join(".fabro/project.toml"), + r#"_version = 1 + +[run] +working_dir = "repos/target" +"#, + ) + .unwrap(); + std::fs::write( + workflow_dir.join("workflow.toml"), + "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n", + ) + .unwrap(); + std::fs::write( + workflow_dir.join("workflow.fabro"), + r"digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", + ) + .unwrap(); + + let built = build_run_manifest(ManifestBuildInput { + workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"), + cwd: workspace.to_path_buf(), + ..Default::default() + }) + .unwrap(); + + let git = built + .manifest + .git + .expect("manifest git info should be detected"); + assert_eq!(git.branch, "target-branch"); + assert_eq!(git.origin_url, "https://github.com/example/target"); + assert_eq!(git.push_outcome, PreRunPushOutcome::NotAttempted); + } + + #[test] + fn build_manifest_git_skips_push_when_configured_repository_differs_from_origin() { + let temp = tempfile::tempdir().unwrap(); + let workspace = temp.path(); + + init_git_repo( + workspace, + "feature", + "https://github.com/user/forked-target.git", + ); + + let workflow_dir = workspace.join(".fabro/workflows/demo"); + std::fs::create_dir_all(&workflow_dir).unwrap(); + std::fs::write( + workspace.join(".fabro/project.toml"), + r#"_version = 1 + +[run.scm] +provider = "github" +owner = "example" +repository = "target" +"#, + ) + .unwrap(); + std::fs::write( + workflow_dir.join("workflow.toml"), + "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n", + ) + .unwrap(); + std::fs::write( + workflow_dir.join("workflow.fabro"), + r"digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", + ) + .unwrap(); + + let built = build_run_manifest(ManifestBuildInput { + workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"), + cwd: workspace.to_path_buf(), + ..Default::default() + }) + .unwrap(); + + let git = built + .manifest + .git + .expect("manifest git info should be detected"); + assert_eq!(git.origin_url, "https://github.com/example/target"); + assert_eq!(git.push_outcome, PreRunPushOutcome::SkippedRemoteMismatch { + remote: "https://github.com/user/forked-target".to_string(), + repo_origin_url: "https://github.com/example/target".to_string(), + }); + } + + #[cfg(unix)] + #[test] + fn build_manifest_push_attempt_disables_terminal_prompts() { + use std::os::unix::fs::PermissionsExt; + + let temp = tempfile::tempdir().unwrap(); + let workspace = temp.path().join("workspace"); + std::fs::create_dir_all(&workspace).unwrap(); + init_git_repo(&workspace, "feature", "fabro-prompt-test::target"); + + let helper_dir = temp.path().join("bin"); + std::fs::create_dir_all(&helper_dir).unwrap(); + let helper_path = helper_dir.join("git-remote-fabro-prompt-test"); + std::fs::write( + &helper_path, + r#"#!/bin/sh +printf '%s\n' "${GIT_TERMINAL_PROMPT-unset}" > "$FABRO_PROMPT_ENV_LOG" +echo "helper saw GIT_TERMINAL_PROMPT=${GIT_TERMINAL_PROMPT-unset}" >&2 +exit 1 +"#, + ) + .unwrap(); + let mut permissions = std::fs::metadata(&helper_path).unwrap().permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&helper_path, permissions).unwrap(); + + let workflow_dir = workspace.join(".fabro/workflows/demo"); + std::fs::create_dir_all(&workflow_dir).unwrap(); + std::fs::write(workspace.join(".fabro/project.toml"), "_version = 1\n").unwrap(); + std::fs::write( + workflow_dir.join("workflow.toml"), + "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n", + ) + .unwrap(); + std::fs::write( + workflow_dir.join("workflow.fabro"), + r"digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", + ) + .unwrap(); + + let helper_log = temp.path().join("prompt-env.txt"); + let mut path_entries = vec![helper_dir]; + if let Some(path) = std::env::var_os("PATH") { + path_entries.extend(std::env::split_paths(&path)); + } + let path = std::env::join_paths(path_entries).unwrap(); + temp_env::with_var("PATH", Some(path), || { + temp_env::with_var("FABRO_PROMPT_ENV_LOG", Some(helper_log.as_os_str()), || { + let built = build_run_manifest(ManifestBuildInput { + workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"), + cwd: workspace.clone(), + ..Default::default() + }) + .unwrap(); + + let git = built + .manifest + .git + .expect("manifest git info should be detected"); + assert!(matches!(git.push_outcome, PreRunPushOutcome::Failed { .. })); + }); + }); + + assert_eq!(std::fs::read_to_string(helper_log).unwrap(), "0\n"); + } + + fn init_git_repo(path: &Path, branch: &str, origin_url: &str) { + run_git(path, &[ + "-c", + &format!("init.defaultBranch={branch}"), + "init", + "--quiet", + ]); + run_git(path, &[ + "-c", + "user.name=test", + "-c", + "user.email=test@example.com", + "commit", + "--allow-empty", + "--quiet", + "-m", + "init", + ]); + run_git(path, &["remote", "add", "origin", origin_url]); + } + + fn mark_origin_branch_synced(path: &Path, branch: &str) { + let remote_ref = format!("refs/remotes/origin/{branch}"); + run_git(path, &["update-ref", &remote_ref, "HEAD"]); + } + + fn run_git(path: &Path, args: &[&str]) { + use std::process::Command; + let output = Command::new("git") + .args(args) + .current_dir(path) + .output() + .unwrap_or_else(|e| panic!("failed to spawn git {args:?}: {e}")); + assert!( + output.status.success(), + "git {args:?} failed: stdout={} stderr={}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + } +} diff --git a/lib/crates/fabro-mcp-server/Cargo.toml b/lib/crates/fabro-mcp-server/Cargo.toml index 4c47e35d4..fc40b6047 100644 --- a/lib/crates/fabro-mcp-server/Cargo.toml +++ b/lib/crates/fabro-mcp-server/Cargo.toml @@ -19,6 +19,9 @@ dirs.workspace = true fabro-api = { path = "../fabro-api" } fabro-client = { path = "../fabro-client" } fabro-http.workspace = true +fabro-manifest = { path = "../fabro-manifest" } +fabro-config = { path = "../fabro-config" } +fabro-server = { path = "../fabro-server" } fabro-types = { path = "../fabro-types" } fabro-util = { path = "../fabro-util" } rmcp = { workspace = true, features = ["server", "macros", "schemars", "transport-io"] } diff --git a/lib/crates/fabro-mcp-server/src/lib.rs b/lib/crates/fabro-mcp-server/src/lib.rs index be53c34a0..a4291e8f7 100644 --- a/lib/crates/fabro-mcp-server/src/lib.rs +++ b/lib/crates/fabro-mcp-server/src/lib.rs @@ -9,9 +9,12 @@ pub use server::start; #[derive(Debug, Clone)] pub struct McpServerSettings { - pub config: McpConfigSettings, - pub home_dir: PathBuf, - pub cwd: PathBuf, + pub config: McpConfigSettings, + pub server_target: Option, + pub storage_dir: PathBuf, + pub config_path: PathBuf, + pub home_dir: PathBuf, + pub cwd: PathBuf, } #[derive(Debug, Clone, Default)] diff --git a/lib/crates/fabro-mcp-server/src/run_tools.rs b/lib/crates/fabro-mcp-server/src/run_tools.rs index ca9e30281..7902c0dac 100644 --- a/lib/crates/fabro-mcp-server/src/run_tools.rs +++ b/lib/crates/fabro-mcp-server/src/run_tools.rs @@ -11,13 +11,20 @@ use std::time::{Duration, Instant}; use chrono::{DateTime, NaiveDate, Utc}; use fabro_api::types; use fabro_client::Client; +use fabro_config::{ + CliLayer, ReplaceMap, RunExecutionLayer, RunGoalLayer, RunLayer, RunModelLayer, RunSandboxLayer, +}; +use fabro_manifest::{ManifestBuildInput, build_run_manifest as build_canonical_run_manifest}; +use fabro_server::manifest_validation; +use fabro_types::settings::InterpString; +use fabro_types::settings::run::{ApprovalMode, RunMode}; use fabro_types::{EventEnvelope, Run, RunId, RunStatus}; use fabro_util::exit::{self, ExitClass}; use rmcp::model::{CallToolResult, Content}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_json::{Value, json}; -use tokio::{fs, time}; +use tokio::time; #[derive(Debug)] pub(crate) struct ToolError { @@ -329,12 +336,13 @@ pub(crate) struct RunEventResult { 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 manifest = build_run_manifest(&spec, &cwd).await?; + let manifest = build_mcp_run_manifest(&spec, &cwd, user_settings_path)?; let run_id = client .create_run_from_manifest(manifest) .await @@ -565,7 +573,7 @@ pub(crate) async fn run_events( .map_err(|err| ToolError::from_anyhow(&err))? .id; let mut events = client - .list_run_events(&run_id, raw.after, Some(event_fetch_limit(&raw))) + .list_run_events(&run_id, raw.after, event_fetch_limit(&raw)) .await .map_err(|err| ToolError::from_anyhow(&err))?; filter_events(&mut events, &raw)?; @@ -715,13 +723,24 @@ fn answer_to_submit_request(answer: Value) -> ToolResult usize { - params - .first - .or(params.limit) - .unwrap_or(50) - .saturating_add(params.offset.unwrap_or(0)) - .clamp(1, 200) +fn event_fetch_limit(params: &FabroRunEventsParams) -> Option { + let needs_full_scan = params.event_ids.is_some() + || params.event_types.is_some() + || params.categories.is_some() + || params.created_after.is_some() + || params.created_before.is_some() + || matches!( + params.action, + RunEventsAction::Details | RunEventsAction::Search + ); + (!needs_full_scan).then(|| { + params + .first + .or(params.limit) + .unwrap_or(50) + .saturating_add(params.offset.unwrap_or(0)) + .clamp(1, 200) + }) } fn filter_events(events: &mut Vec, params: &FabroRunEventsParams) -> ToolResult<()> { @@ -786,71 +805,45 @@ fn run_event_result( }) } -async fn build_run_manifest(spec: &CreateRunSpec, cwd: &Path) -> ToolResult { +fn build_mcp_run_manifest( + spec: &CreateRunSpec, + cwd: &Path, + user_settings_path: &Path, +) -> ToolResult { if let Some(run_id) = spec.run_id.as_deref() { run_id.parse::().map_err(|err| { ToolError::message(format!("run_id must be a valid Fabro run id: {err}")) })?; } - let workflow_path = resolve_workflow_path(&spec.workflow, cwd); - let manifest_cwd = manifest_cwd_for_workflow(cwd, &workflow_path); - let workflow_key = workflow_path - .strip_prefix(&manifest_cwd) - .unwrap_or(&workflow_path) - .display() - .to_string(); - let source = fs::read_to_string(&workflow_path).await.map_err(|err| { - ToolError::message(format!( - "failed to read workflow {}: {err}", - workflow_path.display() - )) - })?; - let workflows = HashMap::from([(workflow_key.clone(), types::ManifestWorkflow { - config: None, - files: HashMap::new(), - source, - })]); - Ok(types::RunManifest { - args: mcp_manifest_args(spec), - configs: Vec::new(), - cwd: manifest_cwd.display().to_string(), - git: None, - goal: Some(types::ManifestGoal { - path: None, - text: spec - .goal - .clone() - .unwrap_or_else(|| "Run the Fabro workflow.".to_string()), - type_: types::ManifestGoalType::Value, - }), - run_id: spec.run_id.clone(), - target: types::ManifestTarget { - identifier: spec.workflow.clone(), - path: workflow_key, - }, - title: None, - version: 1, - workflows, + + let built = build_canonical_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 + .iter() + .map(|(key, value)| json_to_toml_value(key, value).map(|value| (key.clone(), value))) + .collect::>>()?, + args: mcp_manifest_args(spec), + 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}")) + })?, + user_settings_path: Some(user_settings_path.to_path_buf()), }) -} - -fn resolve_workflow_path(workflow: &str, cwd: &Path) -> PathBuf { - let path = PathBuf::from(workflow); - if path.is_absolute() { - path - } else { - cwd.join(path) - } -} - -fn manifest_cwd_for_workflow(cwd: &Path, workflow_path: &Path) -> PathBuf { - if workflow_path.strip_prefix(cwd).is_ok() { - cwd.to_path_buf() - } else { - workflow_path - .parent() - .map_or_else(|| cwd.to_path_buf(), Path::to_path_buf) + .map_err(|err| ToolError::from_anyhow(&err))?; + let validation = manifest_validation::validate_manifest(&RunLayer::default(), &built.manifest) + .map_err(|err| ToolError::from_anyhow(&err))?; + if !validation.ok { + return Err(ToolError::message("workflow manifest validation failed")); } + Ok(built.manifest) } fn mcp_manifest_args(spec: &CreateRunSpec) -> Option { @@ -859,16 +852,11 @@ fn mcp_manifest_args(spec: &CreateRunSpec) -> Option { .iter() .map(|(key, value)| format!("{key}={value}")) .collect::>(); - let input = spec - .inputs - .iter() - .map(|(key, value)| format!("{key}={value}")) - .collect::>(); let payload = types::ManifestArgs { auto_approve: spec.auto_approve.filter(|value| *value), docker_image: None, dry_run: spec.dry_run.filter(|value| *value), - input, + input: Vec::new(), label, model: spec.model.clone(), preserve_sandbox: spec.preserve_sandbox.filter(|value| *value), @@ -879,6 +867,55 @@ fn mcp_manifest_args(spec: &CreateRunSpec) -> Option { (!mcp_manifest_args_is_empty(&payload)).then_some(payload) } +fn mcp_run_overrides(spec: &CreateRunSpec) -> Option { + let goal = spec + .goal + .as_ref() + .map(|goal| RunGoalLayer::Inline(InterpString::parse(goal))); + let model = (spec.model.is_some() || spec.provider.is_some()).then(|| RunModelLayer { + provider: spec.provider.as_deref().map(InterpString::parse), + name: spec.model.as_deref().map(InterpString::parse), + fallbacks: Vec::new(), + }); + let sandbox = + (spec.sandbox.is_some() || spec.preserve_sandbox.is_some()).then(|| RunSandboxLayer { + provider: spec.sandbox.clone(), + preserve: spec.preserve_sandbox, + ..RunSandboxLayer::default() + }); + let execution = + (spec.dry_run.is_some() || spec.auto_approve.is_some()).then(|| RunExecutionLayer { + mode: spec.dry_run.map(|dry_run| { + if dry_run { + RunMode::DryRun + } else { + RunMode::Normal + } + }), + approval: spec.auto_approve.map(|auto_approve| { + if auto_approve { + ApprovalMode::Auto + } else { + ApprovalMode::Prompt + } + }), + }); + let run = RunLayer { + goal, + metadata: ReplaceMap::from(spec.labels.clone()), + model, + sandbox, + execution, + ..RunLayer::default() + }; + (run.goal.is_some() + || !run.metadata.is_empty() + || run.model.is_some() + || run.sandbox.is_some() + || run.execution.is_some()) + .then_some(run) +} + fn mcp_manifest_args_is_empty(args: &types::ManifestArgs) -> bool { args.auto_approve.is_none() && args.docker_image.is_none() diff --git a/lib/crates/fabro-mcp-server/src/server.rs b/lib/crates/fabro-mcp-server/src/server.rs index f4f6e66b9..1eead36a4 100644 --- a/lib/crates/fabro-mcp-server/src/server.rs +++ b/lib/crates/fabro-mcp-server/src/server.rs @@ -1,21 +1,31 @@ -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::Arc; +use std::time::Duration; use anyhow::{Context as _, Result, anyhow}; use fabro_client::{ AuthEntry, AuthStore, Client, Credential, ServerTarget, TransportConnector, apply_bearer_token_auth, }; +use fabro_config::bind::Bind; +use fabro_config::daemon::ServerDaemon; +use fabro_config::{RuntimeDirectory, Storage}; +use fabro_util::dev_token; use rmcp::handler::server::router::tool::ToolRouter; use rmcp::handler::server::wrapper::Parameters; use rmcp::model::{CallToolResult, ServerCapabilities, ServerInfo}; use rmcp::transport::stdio; use rmcp::{ErrorData, ServerHandler, serve_server, tool, tool_handler, tool_router}; +use tokio::process::Command as TokioCommand; use tokio::sync::OnceCell; use tokio::task::yield_now; +use tokio::time::sleep; use crate::{McpServerSettings, run_tools}; +const CLIENT_REQUEST_TIMEOUT: Duration = Duration::from_secs(30); +const SERVER_START_TIMEOUT: Duration = Duration::from_secs(8); + #[derive(Clone)] pub(crate) struct FabroMcpServer { settings: Arc, @@ -67,7 +77,7 @@ impl FabroMcpServer { Ok(client) => client, Err(err) => return Ok(run_tools::error_result(err)), }; - match run_tools::create_runs(client, &self.cwd, params).await { + 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)), } @@ -176,19 +186,27 @@ impl FabroMcpServer { async fn client_from_settings(settings: &McpServerSettings) -> Result { yield_now().await; - let Some(server) = settings.config.server.as_ref() else { - return Err(anyhow!( - "fabro mcp start requires --server for run tools in this release" - )); - }; + if let Some(server) = settings.server_target.as_ref() { + return connect_target(server, settings).await; + } + connect_local_server(settings).await +} + +async fn connect_target(server: &str, settings: &McpServerSettings) -> Result { let target: ServerTarget = server.parse()?; - let credential = AuthStore::new(settings.home_dir.join(".fabro").join("auth.json")) + let mut credential = AuthStore::new(settings.home_dir.join(".fabro").join("auth.json")) .get(&target)? .map(credential_from_auth_entry); + if credential.is_none() && target.is_unix_socket() { + let runtime_token_path = Storage::new(&settings.storage_dir) + .runtime_directory() + .dev_token_path(); + credential = dev_token::read_dev_token_file(&runtime_token_path).map(Credential::DevToken); + } let mut builder = Client::builder() .target(target.clone()) .transport_connector(target_transport_connector(target)) - .request_timeout(std::time::Duration::from_secs(30)); + .request_timeout(CLIENT_REQUEST_TIMEOUT); if let Some(credential) = credential { builder = builder.credential(credential); } @@ -198,6 +216,89 @@ async fn client_from_settings(settings: &McpServerSettings) -> Result { .context("failed to connect Fabro API") } +async fn connect_local_server(settings: &McpServerSettings) -> Result { + let bind = ensure_local_server_running(&settings.storage_dir, &settings.config_path).await?; + match bind { + Bind::Unix(path) => { + let token = wait_for_runtime_dev_token( + &Storage::new(&settings.storage_dir) + .runtime_directory() + .dev_token_path(), + ) + .await?; + let http_client = connect_bind_http_client(&Bind::Unix(path), Some(&token)).await?; + Client::builder() + .transport("http://fabro", http_client) + .request_timeout(CLIENT_REQUEST_TIMEOUT) + .connect() + .await + } + Bind::Tcp(addr) => { + let target = ServerTarget::http_url(format!("http://{addr}"))?; + let credential = AuthStore::new(settings.home_dir.join(".fabro").join("auth.json")) + .get(&target)? + .map(credential_from_auth_entry); + let mut builder = Client::builder() + .target(target.clone()) + .transport_connector(target_transport_connector(target)) + .request_timeout(CLIENT_REQUEST_TIMEOUT); + if let Some(credential) = credential { + builder = builder.credential(credential); + } + builder.connect().await + } + } +} + +async fn ensure_local_server_running(storage_dir: &Path, config_path: &Path) -> Result { + let runtime_directory = RuntimeDirectory::new(storage_dir); + if let Some(existing) = ServerDaemon::load_running(&runtime_directory)? { + return Ok(existing.bind); + } + + let exe = std::env::current_exe().context("resolving current fabro executable path")?; + let status = TokioCommand::new(exe) + .args(["server", "start", "--no-web", "--storage-dir"]) + .arg(storage_dir) + .arg("--config") + .arg(config_path) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .stdin(std::process::Stdio::null()) + .status() + .await + .context("starting local Fabro server")?; + if !status.success() { + return Err(anyhow!("fabro server start exited with status {status}")); + } + + let deadline = std::time::Instant::now() + SERVER_START_TIMEOUT; + while std::time::Instant::now() < deadline { + if let Some(running) = ServerDaemon::load_running(&runtime_directory)? { + return Ok(running.bind); + } + sleep(Duration::from_millis(50)).await; + } + Err(anyhow!( + "Fabro server started but no active record was found for {}", + storage_dir.display() + )) +} + +async fn wait_for_runtime_dev_token(path: &Path) -> Result { + let deadline = std::time::Instant::now() + SERVER_START_TIMEOUT; + while std::time::Instant::now() < deadline { + if let Some(token) = dev_token::read_dev_token_file(path) { + return Ok(token); + } + sleep(Duration::from_millis(50)).await; + } + Err(anyhow!( + "runtime dev token did not become available at {}", + path.display() + )) +} + fn credential_from_auth_entry(entry: AuthEntry) -> Credential { match entry { AuthEntry::OAuth(entry) => Credential::OAuth(entry), @@ -237,3 +338,38 @@ fn connect_target_transport( } Ok((builder.build()?, "http://fabro".to_string())) } + +async fn connect_bind_http_client( + bind: &Bind, + bearer_token: Option<&str>, +) -> Result { + let (client, health_url) = match bind { + Bind::Unix(path) => { + let mut builder = fabro_http::HttpClientBuilder::new() + .unix_socket(path) + .no_proxy(); + if let Some(token) = bearer_token { + builder = apply_bearer_token_auth(builder, token)?; + } + (builder.build()?, "http://fabro/health".to_string()) + } + Bind::Tcp(addr) => { + let mut builder = fabro_http::HttpClientBuilder::new().no_proxy(); + if let Some(token) = bearer_token { + builder = apply_bearer_token_auth(builder, token)?; + } + (builder.build()?, format!("http://{addr}/health")) + } + }; + let deadline = std::time::Instant::now() + SERVER_START_TIMEOUT; + let mut last_error = None; + while std::time::Instant::now() < deadline { + match client.get(&health_url).send().await { + Ok(response) if response.status().is_success() => return Ok(client), + Ok(response) => last_error = Some(anyhow!("health returned {}", response.status())), + Err(err) => last_error = Some(anyhow!(err)), + } + sleep(Duration::from_millis(50)).await; + } + Err(last_error.unwrap_or_else(|| anyhow!("Fabro server did not become ready in time"))) +}