diff --git a/docs/public/agents/skills.mdx b/docs/public/agents/skills.mdx index cbc300b67..05dd4d09e 100644 --- a/docs/public/agents/skills.mdx +++ b/docs/public/agents/skills.mdx @@ -21,8 +21,28 @@ Within each directory, Fabro looks for `*/SKILL.md` files (one level of nesting) When the same skill name appears in multiple directories, **later directories override earlier ones**. This means project-level skills take precedence over global skills, letting you customize behavior per-repo. +Fabro follows a symlinked search directory, but it does not recurse through symlinks below it. Symlinking `.fabro/skills/` at a skill directory elsewhere in the repo therefore does not register that skill; add the real directory with `[run.agent] skill_dirs` instead. + +### Extra directories for workflow runs + +Repositories that keep skills somewhere else — `.agents/skills/` is a common choice — add those directories with `[run.agent] skill_dirs`: + +```toml title="workflow.toml" +[run.agent] +skill_dirs = [".agents/skills"] +``` + +The key layers like every other run setting, so it can live in `workflow.toml`, `.fabro/project.toml`, or `settings.toml`. Configured directories are **added to** the convention directories and searched after them, so a skill defined there overrides a same-named convention skill. Relative entries resolve against the repository root, which keeps them stable regardless of a stage's working directory; absolute entries are used verbatim. Agent stages and their subagents both see the extra directories. + +To add a directory while keeping the ones a lower layer configured, splice them in with `"..."`: + +```toml title="workflow.toml" +[run.agent] +skill_dirs = ["...", ".agents/skills"] +``` + -You can override the default directories by setting `skill_dirs` in the session configuration. When set, only the specified directories are searched. +The Rust SDK's `SessionOptions.skill_dirs` **replaces** the convention directories, and `fabro exec --skills-dir` does the same for a single ad-hoc session. `[run.agent] skill_dirs` is the additive, configuration-driven equivalent for workflow and automation runs. ## Invocation diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index fc19316bf..5dc2c21e3 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -15030,10 +15030,17 @@ components: RunAgentSettings: type: object - required: [fabro_tools, mcps] + required: [fabro_tools, skill_dirs, mcps] properties: fabro_tools: type: boolean + skill_dirs: + description: >- + Extra skill discovery directories, searched after the convention + directories. Relative entries resolve against the repository root. + type: array + items: + type: string mcps: type: object additionalProperties: diff --git a/docs/public/execution/run-configuration.mdx b/docs/public/execution/run-configuration.mdx index 0d99b89bc..690711096 100644 --- a/docs/public/execution/run-configuration.mdx +++ b/docs/public/execution/run-configuration.mdx @@ -507,13 +507,21 @@ Configure workflow agent behavior that is not tied to a single stage. ```toml title="run.toml" [run.agent] fabro_tools = true +skill_dirs = [".agents/skills"] ``` +| Field | Description | +|---|---| +| `fabro_tools` | Allow the run's agents to use the Fabro run-management tools. Defaults to `false`. | +| `skill_dirs` | Extra [skill](/agents/skills) discovery directories, added to the convention directories. Defaults to none. | + `fabro_tools` defaults to `false`. Set it to `true` only for runs whose agents should be able to use the same Fabro run-management MCP tool catalog exposed to human MCP clients: create, search, get, interact, gather, events, and pair. One workflow-agent exception is intentional: `fabro_run_create` always creates [child runs](/execution/child-runs) parented to the current run. If an agent supplies `parent_id`, it must match the current run ID. -This setting is independent of `[run.agent.mcps]`, which configures external MCP servers available to the agent. +`skill_dirs` is searched **after** the convention directories (`~/.fabro/skills`, `{git_root}/.fabro/skills`, `{git_root}/skills`), so a skill defined in a configured directory overrides a same-named convention skill. Relative entries resolve against the repository root; absolute entries are used verbatim. Agent stages and their subagents both see the extra directories. As with other list settings, a higher layer's list replaces a lower layer's unless it splices the inherited entries in with `"..."`. + +These settings are independent of `[run.agent.mcps]`, which configures external MCP servers available to the agent. ### `[run.agent.mcps]` diff --git a/docs/public/reference/user-configuration.mdx b/docs/public/reference/user-configuration.mdx index e68f36896..7f9d2bb1d 100644 --- a/docs/public/reference/user-configuration.mdx +++ b/docs/public/reference/user-configuration.mdx @@ -436,17 +436,19 @@ enabled = true ## `[run.agent]` -`[run.agent]` — agent knobs only (Fabro tools and MCPs) +`[run.agent]` — agent knobs only (Fabro tools, skill directories, and MCPs) ```toml title="settings.toml" [run.agent] fabro_tools = true +skill_dirs = [".agents/skills"] ``` | Key | Type / values | Default | Description | |---|---|---|---| | `fabro_tools` | boolean | false | Allow workflow agents to use Fabro run-management tools. | | `mcps` | table | None | Agent-scoped MCP server entries, keyed by name. | +| `skill_dirs` | array | None | Extra skill discovery directories for workflow agents, searched after
the convention directories (`~/.fabro/skills`,
`{git_root}/.fabro/skills`, `{git_root}/skills`), so a skill defined
here overrides a same-named convention skill. Relative entries resolve
against the repository root. Splice marker supported at layering time. | ## `[run.agent.mcps.]` diff --git a/lib/apps/fabro-cli/tests/it/cmd/attach.rs b/lib/apps/fabro-cli/tests/it/cmd/attach.rs index 93cd58c83..c15900bc1 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/attach.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/attach.rs @@ -913,7 +913,8 @@ fn attach_json_errors_without_prompting_for_human_input() { "run": { "agent": { "fabro_tools": false, - "mcps": {} + "mcps": {}, + "skill_dirs": [] }, "artifacts": { "include": [] diff --git a/lib/apps/fabro-cli/tests/it/cmd/inspect.rs b/lib/apps/fabro-cli/tests/it/cmd/inspect.rs index 56821fe45..e34aeeeb4 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/inspect.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/inspect.rs @@ -194,6 +194,7 @@ fn inspect_resolves_selector_via_server_endpoint() { }, "agent": { "fabro_tools": false, + "skill_dirs": [], "mcps": {} }, "hooks": [], diff --git a/lib/components/fabro-agent/src/config.rs b/lib/components/fabro-agent/src/config.rs index cc207c3f4..34d1e6597 100644 --- a/lib/components/fabro-agent/src/config.rs +++ b/lib/components/fabro-agent/src/config.rs @@ -189,6 +189,14 @@ pub struct SessionOptions { /// Skill directories. `None` = use convention defaults, `Some(dirs)` = use /// these instead. pub skill_dirs: Option>, + /// Extra skill directories appended to `skill_dirs` (or, when that is + /// `None`, to the convention defaults). Searched last, so a skill defined + /// here wins over a same-named skill from an earlier directory. Relative + /// entries resolve against the git root, falling back to the sandbox + /// working directory. This is what `[run.agent] skill_dirs` configures for + /// workflow and automation runs, which need to *add* a discovery directory + /// without restating the conventions. + pub extra_skill_dirs: Vec, /// MCP server configurations to connect to on session startup. pub mcp_servers: Vec, /// Wall-clock timeout for the entire `process_input` call. @@ -226,6 +234,7 @@ impl std::fmt::Debug for SessionOptions { ) .field("compaction_preserve_turns", &self.compaction_preserve_turns) .field("skill_dirs", &self.skill_dirs) + .field("extra_skill_dirs", &self.extra_skill_dirs) .field("mcp_servers", &self.mcp_servers.len()) .field("wall_clock_timeout", &self.wall_clock_timeout) .finish() @@ -253,6 +262,7 @@ impl Default for SessionOptions { compaction_threshold_percent: 80, compaction_preserve_turns: 6, skill_dirs: None, + extra_skill_dirs: Vec::new(), mcp_servers: Vec::new(), wall_clock_timeout: None, } diff --git a/lib/components/fabro-agent/src/session.rs b/lib/components/fabro-agent/src/session.rs index 3d0cf6ef0..998511e8a 100644 --- a/lib/components/fabro-agent/src/session.rs +++ b/lib/components/fabro-agent/src/session.rs @@ -44,7 +44,7 @@ use crate::profiles::EnvContext; use crate::question_tools::AgentToolRuntime; use crate::sandbox::Sandbox; use crate::skills::{ - ExpandedInput, Skill, default_skill_dirs, discover_skills, expand_skill, + ExpandedInput, Skill, default_skill_dirs, discover_skills, expand_skill, extend_skill_dirs, make_use_skill_tool_for_vocabulary, }; use crate::subagent::{SubAgentCallbackEvent, SubAgentEventCallback, SubAgentSupervisor}; @@ -655,13 +655,14 @@ impl Session { }); // Discover skills - let skill_dirs = if let Some(dirs) = &self.config.skill_dirs { + let mut skill_dirs = if let Some(dirs) = &self.config.skill_dirs { dirs.clone() } else { let skills_dir = fabro_util::Home::from_env().skills_dir(); let skills_str = skills_dir.to_string_lossy().to_string(); default_skill_dirs(Some(&skills_str), Some(&doc_root)) }; + extend_skill_dirs(&mut skill_dirs, &self.config.extra_skill_dirs, &doc_root); self.skills = discover_skills(self.sandbox.as_ref(), &skill_dirs, &cancel_token).await?; debug!(skill_count = self.skills.len(), "Skills discovered"); @@ -5970,6 +5971,75 @@ mod tests { assert_eq!(skills[0].description, "Make a commit"); } + /// `extra_skill_dirs` (what `[run.agent] skill_dirs` feeds) adds to the + /// convention directories instead of replacing them, resolves a relative + /// entry against the git root, and is searched last so its skills win. + #[tokio::test] + async fn initialize_appends_extra_skill_dirs_to_the_convention_dirs() { + let sandbox = Arc::new(MockSandbox::linux()); + let config = SessionOptions { + git_root: Some("/home/test".into()), + extra_skill_dirs: vec![".agents/skills".into(), "/opt/shared/skills".into()], + ..Default::default() + }; + let mut session = build_initialized_session(sandbox, config).await; + let mut rx = session.subscribe(); + session.initialize().await.unwrap(); + + let mut source_dirs = None; + while let Ok(envelope) = rx.try_recv() { + if let AgentEvent::SkillsDiscovered { + source_dirs: dirs, .. + } = envelope.event + { + source_dirs = Some(dirs); + break; + } + } + let source_dirs = source_dirs.expect("SkillsDiscovered must be emitted"); + assert_eq!(&source_dirs[source_dirs.len() - 4..], [ + "/home/test/.fabro/skills".to_string(), + "/home/test/skills".to_string(), + "/home/test/.agents/skills".to_string(), + "/opt/shared/skills".to_string(), + ]); + } + + /// An explicit `skill_dirs` override still gets the extra directories + /// appended, so the two knobs compose rather than shadowing each other. + #[tokio::test] + async fn initialize_appends_extra_skill_dirs_to_an_explicit_override() { + let sandbox = Arc::new(MockSandbox::linux()); + let config = SessionOptions { + git_root: Some("/home/test".into()), + skill_dirs: Some(vec!["/skills".into()]), + extra_skill_dirs: vec!["/skills".into(), ".agents/skills".into()], + ..Default::default() + }; + let mut session = build_initialized_session(sandbox, config).await; + let mut rx = session.subscribe(); + session.initialize().await.unwrap(); + + let mut source_dirs = None; + while let Ok(envelope) = rx.try_recv() { + if let AgentEvent::SkillsDiscovered { + source_dirs: dirs, .. + } = envelope.event + { + source_dirs = Some(dirs); + break; + } + } + // `/skills` is already in the list, so it is not globbed twice. + assert_eq!( + source_dirs.expect("SkillsDiscovered must be emitted"), + vec![ + "/skills".to_string(), + "/home/test/.agents/skills".to_string() + ] + ); + } + #[tokio::test] async fn initialize_emits_skills_discovered_event_when_no_skills() { let sandbox = Arc::new(MockSandbox::linux()); diff --git a/lib/components/fabro-agent/src/skills.rs b/lib/components/fabro-agent/src/skills.rs index f1aff7031..17f074b7c 100644 --- a/lib/components/fabro-agent/src/skills.rs +++ b/lib/components/fabro-agent/src/skills.rs @@ -299,6 +299,33 @@ pub fn default_skill_dirs(fabro_skills_dir: Option<&str>, git_root: Option<&str> dirs } +/// Append configured extra discovery directories to `dirs`. +/// +/// Extra directories are searched last, so a skill they define wins over a +/// same-named skill from a convention directory (`discover_skills` keys skills +/// by name and later directories overwrite earlier ones). A relative entry +/// resolves against `doc_root` — the git root when known, otherwise the +/// sandbox working directory — so a repo-relative path such as +/// `.agents/skills` means the same thing no matter which subdirectory the +/// agent runs in. Duplicate directories are dropped so the same path is not +/// globbed twice. +pub fn extend_skill_dirs(dirs: &mut Vec, extra: &[String], doc_root: &str) { + for dir in extra { + let dir = dir.trim(); + if dir.is_empty() { + continue; + } + let resolved = if std::path::Path::new(dir).is_absolute() { + dir.to_string() + } else { + format!("{}/{dir}", doc_root.trim_end_matches('/')) + }; + if !dirs.contains(&resolved) { + dirs.push(resolved); + } + } +} + pub async fn discover_skills( env: &dyn Sandbox, dirs: &[String], @@ -642,6 +669,58 @@ name: trimmed assert_eq!(dirs, vec!["/home/user/.fabro/skills"]); } + // --- extend_skill_dirs tests --- + + #[test] + fn extra_dirs_are_appended_after_the_convention_dirs() { + let mut dirs = default_skill_dirs(Some("/home/user/.fabro/skills"), Some("/repo")); + extend_skill_dirs(&mut dirs, &[".agents/skills".to_string()], "/repo"); + assert_eq!(dirs, vec![ + "/home/user/.fabro/skills", + "/repo/.fabro/skills", + "/repo/skills", + "/repo/.agents/skills", + ]); + } + + #[test] + fn extra_dirs_keep_absolute_paths_verbatim() { + let mut dirs = Vec::new(); + extend_skill_dirs(&mut dirs, &["/opt/shared/skills".to_string()], "/repo"); + assert_eq!(dirs, vec!["/opt/shared/skills"]); + } + + #[test] + fn extra_dirs_resolve_against_a_doc_root_with_a_trailing_slash() { + let mut dirs = Vec::new(); + extend_skill_dirs(&mut dirs, &[".agents/skills".to_string()], "/repo/"); + assert_eq!(dirs, vec!["/repo/.agents/skills"]); + } + + #[test] + fn extra_dirs_skip_duplicates_and_blank_entries() { + let mut dirs = vec!["/repo/skills".to_string()]; + extend_skill_dirs( + &mut dirs, + &[ + "skills".to_string(), + " ".to_string(), + String::new(), + ".agents/skills".to_string(), + ".agents/skills".to_string(), + ], + "/repo", + ); + assert_eq!(dirs, vec!["/repo/skills", "/repo/.agents/skills"]); + } + + #[test] + fn extend_skill_dirs_is_a_no_op_without_extra_dirs() { + let mut dirs = vec!["/repo/skills".to_string()]; + extend_skill_dirs(&mut dirs, &[], "/repo"); + assert_eq!(dirs, vec!["/repo/skills"]); + } + // --- make_use_skill_tool tests --- #[tokio::test] diff --git a/lib/components/fabro-workflow/src/handler/llm/api.rs b/lib/components/fabro-workflow/src/handler/llm/api.rs index 30b765f4e..55bce83d2 100644 --- a/lib/components/fabro-workflow/src/handler/llm/api.rs +++ b/lib/components/fabro-workflow/src/handler/llm/api.rs @@ -641,6 +641,9 @@ pub struct AgentApiBackend { emitted_plan_notices: Mutex>, tool_env: Option>, mcp_servers: Vec, + /// Extra skill discovery directories from `[run.agent] skill_dirs`, added + /// to the convention directories every agent session already searches. + skill_dirs: Vec, tool_secrets: ToolSecrets, run_model_controls: RunModelControls, source: Arc, @@ -820,6 +823,7 @@ impl AgentApiBackend { emitted_plan_notices: Mutex::new(HashSet::new()), tool_env: None, mcp_servers: Vec::new(), + skill_dirs: Vec::new(), tool_secrets: ToolSecrets::default(), run_model_controls: RunModelControls::default(), source, @@ -847,6 +851,12 @@ impl AgentApiBackend { self } + #[must_use] + pub fn with_skill_dirs(mut self, dirs: Vec) -> Self { + self.skill_dirs = dirs; + self + } + #[must_use] pub fn with_tool_secrets(mut self, tool_secrets: ToolSecrets) -> Self { self.tool_secrets = tool_secrets; @@ -1026,6 +1036,7 @@ impl AgentApiBackend { self.tool_env.as_ref(), tool_hooks, self.mcp_servers.clone(), + self.skill_dirs.clone(), self.tool_secrets.clone(), self.fabro_run_tools.clone(), ) @@ -1050,6 +1061,7 @@ impl AgentApiBackend { tool_env: Option<&Arc>, tool_hooks: Option>, mcp_servers: Vec, + skill_dirs: Vec, tool_secrets: ToolSecrets, fabro_run_tools: Option, ) -> Result { @@ -1083,6 +1095,11 @@ impl AgentApiBackend { speed: controls.speed, tool_hooks, mcp_servers, + // `[run.agent] skill_dirs` adds to the convention directories + // rather than replacing them, so it lands in `extra_skill_dirs` + // and leaves `skill_dirs` (the SDK's replace-the-defaults knob) + // alone. + extra_skill_dirs: skill_dirs, // Workflow agents run with no `tool_access_policy`, which exposes // the entire tool registry (read, write, shell, subagent, MCP) and // skips approval gating. Report that truthfully so the UI doesn't @@ -1106,6 +1123,9 @@ impl AgentApiBackend { let factory_fabro_run_tools = fabro_run_tools.clone(); let factory_permission_level = config.permission_level; let factory_tool_hooks = config.tool_hooks.clone(); + // Subagents run the same repository's skills, so they inherit the + // run's extra discovery directories. + let factory_skill_dirs = config.extra_skill_dirs.clone(); let factory: SessionFactory = Arc::new(move || { let mut child_profile = factory_profile_builder.build(); if let Some(services) = factory_fabro_run_tools.clone() { @@ -1121,6 +1141,7 @@ impl AgentApiBackend { speed: controls.speed, tool_hooks: factory_tool_hooks.clone(), permission_level: factory_permission_level, + extra_skill_dirs: factory_skill_dirs.clone(), ..SessionOptions::default() }, None, @@ -1241,6 +1262,7 @@ impl AgentApiBackend { self.tool_env.as_ref(), request.tool_hooks.clone(), self.mcp_servers.clone(), + self.skill_dirs.clone(), self.tool_secrets.clone(), self.fabro_run_tools.clone(), ) @@ -3905,6 +3927,81 @@ enabled = true assert_eq!(text, r#"{"passed":true}"#); } + /// `[run.agent] skill_dirs` reaches the agent session: a skill living in a + /// configured directory is discovered even though that directory is not one + /// of the conventions. + #[tokio::test] + async fn agent_run_discovers_skills_from_configured_skill_dirs() { + let server = MockServer::start(); + let completion = server.mock(|when, then| { + when.method(POST).path("/chat/completions"); + then.status(200) + .header("content-type", "text/event-stream") + .body(chat_completion_stream("Done", 10, 1)); + }); + let workspace = tempfile::tempdir().unwrap(); + let skill_dir = workspace.path().join(".agents/skills/greet"); + tokio::fs::create_dir_all(&skill_dir).await.unwrap(); + tokio::fs::write( + skill_dir.join("SKILL.md"), + "---\nname: greet\ndescription: Greet the user\n---\nSay hello.", + ) + .await + .unwrap(); + + let backend = mock_api_backend(&server).with_skill_dirs(vec![".agents/skills".to_string()]); + let node = Node::new("greeter"); + let context = Context::new(); + let emitter = Arc::new(Emitter::new(fabro_types::RunId::new())); + let discovered = Arc::new(Mutex::new(Vec::new())); + let discovered_for_listener = Arc::clone(&discovered); + emitter.on_event(move |event| { + if let fabro_types::EventBody::AgentSkillsDiscovered(props) = &event.body { + discovered_for_listener.lock().unwrap().push(( + props.source_dirs.clone(), + props + .skills + .iter() + .map(|skill| skill.name.clone()) + .collect::>(), + )); + } + }); + let sandbox: Arc = + Arc::new(LocalSandbox::new(workspace.path().to_path_buf())); + + backend + .run(CodergenRunRequest { + node: &node, + prompt: "Greet", + context: &context, + thread_id: None, + emitter: &emitter, + sandbox: &sandbox, + tool_hooks: None, + cancel_token: CancellationToken::new(), + agent_tool_runtime: fabro_agent::AgentToolRuntime::default(), + }) + .await + .unwrap(); + + completion.assert_calls(1); + let discovered = discovered.lock().unwrap(); + let (source_dirs, skills) = discovered + .first() + .expect("agent.skills.discovered should be emitted"); + let configured = workspace + .path() + .join(".agents/skills") + .to_string_lossy() + .into_owned(); + assert_eq!(source_dirs.last(), Some(&configured)); + assert!( + skills.iter().any(|name| name == "greet"), + "configured skill dir should contribute its skills, got {skills:?}" + ); + } + #[tokio::test] async fn agent_run_web_search_uses_configured_brave_search_key() { let server = MockServer::start(); diff --git a/lib/components/fabro-workflow/src/operations/start.rs b/lib/components/fabro-workflow/src/operations/start.rs index 20edc4800..bdb144aa6 100644 --- a/lib/components/fabro-workflow/src/operations/start.rs +++ b/lib/components/fabro-workflow/src/operations/start.rs @@ -563,6 +563,7 @@ impl RunSession { provider_id: llm.provider_id.clone(), fallbacks: llm.fallbacks.policy, mcp_servers, + skill_dirs: resolved.agent.skill_dirs.clone(), model_controls: resolved.model.controls.clone(), dry_run: resolved.execution.mode == RunMode::DryRun, }, @@ -1822,6 +1823,41 @@ reasoning = false ); } + /// `[run.agent] skill_dirs` has to reach the LLM spec, which is what hands + /// the directories to every agent session the run creates. + #[tokio::test] + async fn run_session_new_carries_run_agent_skill_dirs_into_the_llm_spec() { + let temp = tempfile::tempdir().unwrap(); + let (storage_root, _run_dir) = storage_root_and_run_dir(&temp); + let mut settings = settings_from_run_layer(RunLayer { + execution: Some(RunExecutionLayer { + mode: Some(RunMode::DryRun), + ..RunExecutionLayer::default() + }), + ..RunLayer::default() + }); + settings.run.agent.skill_dirs = vec![ + ".agents/skills".to_string(), + "/opt/shared/skills".to_string(), + ]; + let (persisted, store) = + persisted_workflow_with_settings(MINIMAL_DOT, &storage_root, settings).await; + let emitter = Arc::new(Emitter::new(fixtures::RUN_1)); + let registry = Arc::new(test_registry()); + + let session = RunSession::new( + &persisted, + test_start_services(&store, &storage_root, emitter, registry).await, + ) + .await + .unwrap(); + + assert_eq!(session.llm.skill_dirs, vec![ + ".agents/skills".to_string(), + "/opt/shared/skills".to_string(), + ]); + } + #[tokio::test] async fn run_session_new_missing_secret_fails_startup() { let temp = tempfile::tempdir().unwrap(); diff --git a/lib/components/fabro-workflow/src/pipeline/execute/tests.rs b/lib/components/fabro-workflow/src/pipeline/execute/tests.rs index 7e8647f6c..67f6722ab 100644 --- a/lib/components/fabro-workflow/src/pipeline/execute/tests.rs +++ b/lib/components/fabro-workflow/src/pipeline/execute/tests.rs @@ -273,6 +273,7 @@ async fn execute_test_run_with_options( provider_id: fabro_model::ProviderId::anthropic(), fallbacks: ModelFallbackPolicy::default(), mcp_servers: Vec::new(), + skill_dirs: Vec::new(), model_controls: RunModelControls::default(), dry_run: true, }, @@ -333,6 +334,7 @@ async fn execute_runs_start_to_exit_and_returns_final_context() { provider_id: fabro_model::ProviderId::anthropic(), fallbacks: ModelFallbackPolicy::default(), mcp_servers: Vec::new(), + skill_dirs: Vec::new(), model_controls: RunModelControls::default(), dry_run: true, }, @@ -474,6 +476,7 @@ async fn resumed_in_flight_node_starts_a_new_stage_execution() { provider_id: fabro_model::ProviderId::anthropic(), fallbacks: ModelFallbackPolicy::default(), mcp_servers: Vec::new(), + skill_dirs: Vec::new(), model_controls: RunModelControls::default(), dry_run: true, }, @@ -588,6 +591,7 @@ async fn run_with_lifecycle( provider_id: fabro_model::ProviderId::anthropic(), fallbacks: ModelFallbackPolicy::default(), mcp_servers: Vec::new(), + skill_dirs: Vec::new(), model_controls: RunModelControls::default(), dry_run: true, }, diff --git a/lib/components/fabro-workflow/src/pipeline/initialize.rs b/lib/components/fabro-workflow/src/pipeline/initialize.rs index d05d9acd9..716175226 100644 --- a/lib/components/fabro-workflow/src/pipeline/initialize.rs +++ b/lib/components/fabro-workflow/src/pipeline/initialize.rs @@ -243,6 +243,7 @@ async fn build_registry( let provider_id = spec.provider_id.clone(); let fallbacks = spec.fallbacks.clone(); let mcp_servers = spec.mcp_servers.clone(); + let skill_dirs = spec.skill_dirs.clone(); let model_controls = spec.model_controls.clone(); let tool_secrets_for_api = tool_secrets.clone(); let llm_source_for_api = Arc::clone(&llm_source); @@ -263,7 +264,8 @@ async fn build_registry( .with_run_model_controls(model_controls.clone()) .with_tool_env_provider(tool_env_provider.clone()) .with_tool_secrets(tool_secrets_for_api.clone()) - .with_mcp_servers(mcp_servers.clone()); + .with_mcp_servers(mcp_servers.clone()) + .with_skill_dirs(skill_dirs.clone()); if let Some(services) = fabro_run_tools_for_api.clone() { api = api.with_fabro_run_tools(services); } @@ -914,6 +916,7 @@ mod tests { provider_id: fabro_model::ProviderId::anthropic(), fallbacks: ModelFallbackPolicy::default(), mcp_servers: Vec::new(), + skill_dirs: Vec::new(), model_controls: RunModelControls::default(), dry_run: true, }, @@ -1232,6 +1235,7 @@ mod tests { provider_id: fabro_model::ProviderId::anthropic(), fallbacks: ModelFallbackPolicy::default(), mcp_servers: Vec::new(), + skill_dirs: Vec::new(), model_controls: RunModelControls::default(), dry_run: false, }, @@ -1354,6 +1358,7 @@ mod tests { provider_id: fabro_model::ProviderId::openai(), fallbacks: ModelFallbackPolicy::default(), mcp_servers: Vec::new(), + skill_dirs: Vec::new(), model_controls: RunModelControls::default(), dry_run: false, }, @@ -1457,6 +1462,7 @@ mod tests { provider_id: fabro_model::ProviderId::anthropic(), fallbacks: ModelFallbackPolicy::default(), mcp_servers: Vec::new(), + skill_dirs: Vec::new(), model_controls: RunModelControls::default(), dry_run: true, }, @@ -1599,6 +1605,7 @@ mod tests { provider_id: fabro_model::ProviderId::anthropic(), fallbacks: ModelFallbackPolicy::default(), mcp_servers: Vec::new(), + skill_dirs: Vec::new(), model_controls: RunModelControls::default(), dry_run: true, }, diff --git a/lib/components/fabro-workflow/src/pipeline/types.rs b/lib/components/fabro-workflow/src/pipeline/types.rs index c65f1fdf6..c07299659 100644 --- a/lib/components/fabro-workflow/src/pipeline/types.rs +++ b/lib/components/fabro-workflow/src/pipeline/types.rs @@ -241,6 +241,9 @@ pub struct LlmSpec { pub provider_id: ProviderId, pub fallbacks: ModelFallbackPolicy, pub mcp_servers: Vec, + /// Extra skill discovery directories from `[run.agent] skill_dirs`, + /// searched in addition to the agent's convention directories. + pub skill_dirs: Vec, pub model_controls: RunModelControls, pub dry_run: bool, } diff --git a/lib/foundation/fabro-config/src/layers/run.rs b/lib/foundation/fabro-config/src/layers/run.rs index c18b72015..1b490c37d 100644 --- a/lib/foundation/fabro-config/src/layers/run.rs +++ b/lib/foundation/fabro-config/src/layers/run.rs @@ -405,7 +405,7 @@ pub struct InterviewProviderLayer { pub channel: Option, } -/// `[run.agent]` — agent knobs only (Fabro tools and MCPs). +/// `[run.agent]` — agent knobs only (Fabro tools, skill directories, and MCPs). #[derive( Debug, Clone, @@ -423,6 +423,15 @@ pub struct RunAgentLayer { #[option(default = "false", value_type = "boolean")] pub fabro_tools: Option, + /// Extra skill discovery directories for workflow agents, searched after + /// the convention directories (`~/.fabro/skills`, + /// `{git_root}/.fabro/skills`, `{git_root}/skills`), so a skill defined + /// here overrides a same-named convention skill. Relative entries resolve + /// against the repository root. Splice marker supported at layering time. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[option(value_type = "array")] + pub skill_dirs: Vec, + /// Agent-scoped MCP server entries, keyed by name. #[serde(default, skip_serializing_if = "StickyMap::is_empty")] #[option(value_type = "table")] diff --git a/lib/foundation/fabro-config/src/resolve/run.rs b/lib/foundation/fabro-config/src/resolve/run.rs index ae529224e..73609352d 100644 --- a/lib/foundation/fabro-config/src/resolve/run.rs +++ b/lib/foundation/fabro-config/src/resolve/run.rs @@ -458,6 +458,14 @@ fn resolve_agent( RunAgentSettings { fabro_tools: agent.fabro_tools.unwrap_or(false), + skill_dirs: agent + .skill_dirs + .iter() + .filter_map(|dir| match dir { + StringOrSplice::Value(value) => Some(value.clone()), + StringOrSplice::Splice => None, + }) + .collect(), mcps: resolve_mcp_entries(&agent.mcps, mcp_server_catalog, errors), } } diff --git a/lib/foundation/fabro-config/src/tests/resolve_run.rs b/lib/foundation/fabro-config/src/tests/resolve_run.rs index bc6fe8bc4..5f0c2b15b 100644 --- a/lib/foundation/fabro-config/src/tests/resolve_run.rs +++ b/lib/foundation/fabro-config/src/tests/resolve_run.rs @@ -1454,6 +1454,96 @@ fabro_tools = true assert!(!settings.agent.fabro_tools); } + + #[test] + fn skill_dirs_default_to_empty() { + let settings = super::workflow_settings_from_layer(SettingsLayer::default()) + .expect("empty settings should resolve") + .run; + + assert!(settings.agent.skill_dirs.is_empty()); + } + + #[test] + fn resolves_skill_dirs_in_declared_order() { + let settings = super::workflow_settings_from_toml( + r#" +_version = 1 + +[run.agent] +skill_dirs = [".agents/skills", "/opt/shared/skills"] +"#, + ) + .expect("run.agent.skill_dirs should resolve"); + + assert_eq!(settings.run.agent.skill_dirs, vec![ + ".agents/skills".to_string(), + "/opt/shared/skills".to_string(), + ]); + } + + /// A higher layer's list replaces the lower layer's, matching every other + /// splice-aware list setting. + #[test] + fn higher_layer_skill_dirs_replace_lower_layer() { + let workflow = parse_settings( + r#" +_version = 1 + +[run.agent] +skill_dirs = [".agents/skills"] +"#, + ); + let user = parse_settings( + r#" +_version = 1 + +[run.agent] +skill_dirs = ["/home/user/skills"] +"#, + ); + let merged = workflow.combine(user); + + let settings = super::workflow_settings_from_layer(merged) + .expect("merged settings should resolve") + .run; + + assert_eq!(settings.agent.skill_dirs, vec![ + ".agents/skills".to_string() + ]); + } + + /// `"..."` splices the lower layer's entries in place, so a project can add + /// a directory without restating the ones it inherits. + #[test] + fn splice_marker_expands_the_lower_layer_skill_dirs() { + let workflow = parse_settings( + r#" +_version = 1 + +[run.agent] +skill_dirs = ["...", ".agents/skills"] +"#, + ); + let user = parse_settings( + r#" +_version = 1 + +[run.agent] +skill_dirs = ["/home/user/skills"] +"#, + ); + let merged = workflow.combine(user); + + let settings = super::workflow_settings_from_layer(merged) + .expect("merged settings should resolve") + .run; + + assert_eq!(settings.agent.skill_dirs, vec![ + "/home/user/skills".to_string(), + ".agents/skills".to_string(), + ]); + } } mod run_checkpoint { diff --git a/lib/foundation/fabro-dev/src/commands/docs_options_reference.rs b/lib/foundation/fabro-dev/src/commands/docs_options_reference.rs index 406ec0b49..b1debac20 100644 --- a/lib/foundation/fabro-dev/src/commands/docs_options_reference.rs +++ b/lib/foundation/fabro-dev/src/commands/docs_options_reference.rs @@ -131,8 +131,9 @@ enabled = true", ), Section::of::( "[run.agent]", - r"[run.agent] -fabro_tools = true", + r#"[run.agent] +fabro_tools = true +skill_dirs = [".agents/skills"]"#, ), ] } diff --git a/lib/foundation/fabro-types/src/settings/run.rs b/lib/foundation/fabro-types/src/settings/run.rs index 6d2b3ebeb..080368be6 100644 --- a/lib/foundation/fabro-types/src/settings/run.rs +++ b/lib/foundation/fabro-types/src/settings/run.rs @@ -1477,6 +1477,16 @@ pub struct InterviewProviderSettings { pub struct RunAgentSettings { #[serde(default)] pub fabro_tools: bool, + /// Extra skill discovery directories, searched in addition to the + /// convention directories (`~/.fabro/skills`, `{git_root}/.fabro/skills`, + /// `{git_root}/skills`) and after them, so a skill defined here wins over a + /// same-named convention skill. Relative entries resolve against the + /// repository root. + /// + /// `#[serde(default)]`: run specs persisted before this field existed have + /// no `skill_dirs` key, and they must stay loadable. + #[serde(default)] + pub skill_dirs: Vec, pub mcps: HashMap, } diff --git a/lib/packages/fabro-api-client/src/models/run-agent-settings.ts b/lib/packages/fabro-api-client/src/models/run-agent-settings.ts index be5774084..9e289f06b 100644 --- a/lib/packages/fabro-api-client/src/models/run-agent-settings.ts +++ b/lib/packages/fabro-api-client/src/models/run-agent-settings.ts @@ -19,5 +19,9 @@ import type { McpServerSettings } from './mcp-server-settings'; export interface RunAgentSettings { 'fabro_tools': boolean; + /** + * Extra skill discovery directories, searched after the convention directories. Relative entries resolve against the repository root. + */ + 'skill_dirs': Array; 'mcps': { [key: string]: McpServerSettings; }; }