From bfc5214b5dd410449e48fbb6217fc4d1391b1481 Mon Sep 17 00:00:00 2001 From: Cyrus Gray <144336577+graycyrus@users.noreply.github.com> Date: Wed, 10 Jun 2026 19:16:12 +0530 Subject: [PATCH] fix(workflows): Automations list shows only task templates, not capability skills (#3581) --- src/openhuman/workflows/ops.rs | 4 +- src/openhuman/workflows/ops_discover.rs | 130 +++++++++++++++++--- src/openhuman/workflows/ops_tests.rs | 115 +++++++++++++++++ src/openhuman/workflows/schemas/handlers.rs | 15 ++- 4 files changed, 237 insertions(+), 27 deletions(-) diff --git a/src/openhuman/workflows/ops.rs b/src/openhuman/workflows/ops.rs index 31ac446dd..ec4601ab6 100644 --- a/src/openhuman/workflows/ops.rs +++ b/src/openhuman/workflows/ops.rs @@ -31,8 +31,8 @@ // callers are unaffected. pub use super::ops_create::{create_workflow, CreateWorkflowParams, WorkflowCreateInputDef}; pub use super::ops_discover::{ - discover_workflows, init_workflows_dir, is_workspace_trusted, load_workflow_metadata, - read_workflow_resource, + discover_automations, discover_workflows, init_workflows_dir, is_workspace_trusted, + load_workflow_metadata, read_workflow_resource, }; pub use super::ops_install::{ install_workflow_from_url, uninstall_workflow, validate_install_url, validate_resolved_host, diff --git a/src/openhuman/workflows/ops_discover.rs b/src/openhuman/workflows/ops_discover.rs index f76336e2f..4ccdebe38 100644 --- a/src/openhuman/workflows/ops_discover.rs +++ b/src/openhuman/workflows/ops_discover.rs @@ -95,58 +95,150 @@ pub fn is_workspace_trusted(workspace_dir: &Path) -> bool { workspace_dir.join(".openhuman").join(TRUST_MARKER).exists() } +/// Which on-disk root category a bundle was discovered under. +/// +/// `Workflow` roots (`.openhuman/workflows/`) hold task *automations* authored +/// via "New workflow". `Skill` roots (`.openhuman/skills/`, `.agents/skills/`, +/// and the legacy `/skills/`) hold capability *skills*. Both are the +/// same on-disk primitive (SKILL.md / WORKFLOW.md bundles) and the agent +/// harness loads both — but the Automations UI lists only `Workflow`-root +/// bundles (see [`discover_automations`]) so capability skills don't masquerade +/// as task templates. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub(crate) enum RootKind { + Skill, + Workflow, +} + +const ALL_ROOT_KINDS: &[RootKind] = &[RootKind::Skill, RootKind::Workflow]; +const WORKFLOW_ROOT_KINDS: &[RootKind] = &[RootKind::Workflow]; + pub(crate) fn discover_workflows_inner( home_dir: Option<&Path>, workspace_dir: Option<&Path>, trusted: bool, ) -> Vec { + discover_filtered(home_dir, workspace_dir, trusted, ALL_ROOT_KINDS) +} + +/// Discover only *automation* bundles — those under the `workflows/` roots — +/// for the Automations UI list (`openhuman.workflows_list`). +/// +/// Capability skills (under the `skills/` / `.agents/skills/` / legacy +/// `/skills/` roots) are deliberately excluded so they don't show up +/// as task templates. They remain fully available to the agent harness and the +/// run/describe paths via [`discover_workflows`] / [`load_workflow_metadata`]. +/// +/// Note: bundles authored *before* the skills→workflows rename live under the +/// `skills/` roots and will therefore not appear in this automations-only view; +/// new automations created via "New workflow" land in `~/.openhuman/workflows/`. +pub fn discover_automations( + home_dir: Option<&Path>, + workspace_dir: Option<&Path>, + trusted: bool, +) -> Vec { + tracing::debug!( + trusted, + has_home = home_dir.is_some(), + has_workspace = workspace_dir.is_some(), + "[workflows] discover:automations:enter" + ); + discover_filtered(home_dir, workspace_dir, trusted, WORKFLOW_ROOT_KINDS) +} + +/// Shared discovery core. `kinds` selects which root categories to scan, +/// letting the full surface ([`discover_workflows_inner`]) and the +/// automations-only list ([`discover_automations`]) share collision handling. +fn discover_filtered( + home_dir: Option<&Path>, + workspace_dir: Option<&Path>, + trusted: bool, + kinds: &[RootKind], +) -> Vec { + tracing::debug!( + trusted, + has_home = home_dir.is_some(), + has_workspace = workspace_dir.is_some(), + include_skills = kinds.contains(&RootKind::Skill), + include_workflows = kinds.contains(&RootKind::Workflow), + "[workflows] discover:enter" + ); // Scan order matters for collision resolution: the last scope to register // a name wins, so we scan user first, then project, then legacy. let mut by_name: HashMap = HashMap::new(); if let Some(home) = home_dir { - for root in user_roots(home) { - absorb(&mut by_name, scan_root(&root, WorkflowScope::User)); + for (root, kind) in user_roots(home) { + if kinds.contains(&kind) { + tracing::trace!( + root = %root.display(), + ?kind, + scope = ?WorkflowScope::User, + "[workflows] discover:branch:user" + ); + absorb(&mut by_name, scan_root(&root, WorkflowScope::User)); + } } } if let Some(ws) = workspace_dir { if trusted { - for root in project_roots(ws) { - absorb(&mut by_name, scan_root(&root, WorkflowScope::Project)); + for (root, kind) in project_roots(ws) { + if kinds.contains(&kind) { + tracing::trace!( + root = %root.display(), + ?kind, + scope = ?WorkflowScope::Project, + "[workflows] discover:branch:project" + ); + absorb(&mut by_name, scan_root(&root, WorkflowScope::Project)); + } } } - // Legacy `/skills/` is always scanned so existing setups - // keep working without requiring users to move files or add the trust - // marker. Flagged with `legacy = true` so the UI can nudge migration. - absorb( - &mut by_name, - scan_root(&ws.join("skills"), WorkflowScope::Legacy), - ); + // Legacy `/skills/` is a skill root: scanned for the full + // surface (back-compat, no trust marker required) but excluded from the + // automations-only view. Flagged with `legacy = true` so the UI can + // nudge migration. + if kinds.contains(&RootKind::Skill) { + let legacy_root = ws.join("skills"); + tracing::trace!( + root = %legacy_root.display(), + scope = ?WorkflowScope::Legacy, + "[workflows] discover:branch:legacy" + ); + absorb(&mut by_name, scan_root(&legacy_root, WorkflowScope::Legacy)); + } } let mut out: Vec = by_name.into_values().collect(); out.sort_by(|a, b| a.name.cmp(&b.name)); + tracing::debug!(discovered_count = out.len(), "[workflows] discover:exit"); out } -fn user_roots(home: &Path) -> Vec { +fn user_roots(home: &Path) -> Vec<(PathBuf, RootKind)> { // `workflows/` is the current layout (create writes here); the `skills/` // roots are still scanned for back-compat with installs created before the // skills→workflows rename. Order matters: `workflows/` is scanned last so a // same-named entry there wins over a legacy `skills/` one. vec![ - home.join(".openhuman").join("skills"), - home.join(".agents").join("skills"), - home.join(".openhuman").join("workflows"), + (home.join(".openhuman").join("skills"), RootKind::Skill), + (home.join(".agents").join("skills"), RootKind::Skill), + ( + home.join(".openhuman").join("workflows"), + RootKind::Workflow, + ), ] } -fn project_roots(workspace: &Path) -> Vec { +fn project_roots(workspace: &Path) -> Vec<(PathBuf, RootKind)> { vec![ - workspace.join(".openhuman").join("skills"), - workspace.join(".agents").join("skills"), - workspace.join(".openhuman").join("workflows"), + (workspace.join(".openhuman").join("skills"), RootKind::Skill), + (workspace.join(".agents").join("skills"), RootKind::Skill), + ( + workspace.join(".openhuman").join("workflows"), + RootKind::Workflow, + ), ] } diff --git a/src/openhuman/workflows/ops_tests.rs b/src/openhuman/workflows/ops_tests.rs index 494ebf038..b0e1dc228 100644 --- a/src/openhuman/workflows/ops_tests.rs +++ b/src/openhuman/workflows/ops_tests.rs @@ -1664,3 +1664,118 @@ fn symlinked_manifest_file_is_rejected() { "a symlinked manifest must not be loaded; got {skills:?}" ); } + +// --------------------------------------------------------------------------- +// discover_automations: the Automations UI list shows only `workflows/`-root +// task templates, never capability skills under `skills/` roots. The full +// surface (`discover_workflows`) still includes both for the agent harness. +// --------------------------------------------------------------------------- + +#[test] +fn discover_automations_excludes_user_skill_root_but_keeps_workflows() { + let home = tempfile::tempdir().unwrap(); + // A capability skill installed under ~/.openhuman/skills/. + write( + &home + .path() + .join(".openhuman") + .join("skills") + .join("ascii-art") + .join(SKILL_MD), + "---\nname: ascii-art\ndescription: ASCII art\n---\n", + ); + // A real automation authored under ~/.openhuman/workflows/. + write( + &home + .path() + .join(".openhuman") + .join("workflows") + .join("deploy") + .join(WORKFLOW_MD), + "---\nname: deploy\ndescription: Ship a release\n---\n", + ); + + let automations = discover_automations(Some(home.path()), None, false); + let names: Vec<&str> = automations.iter().map(|w| w.name.as_str()).collect(); + assert_eq!( + names, + vec!["deploy"], + "automations must exclude the skills/ root; got {names:?}" + ); + + // The full surface still sees both (agent harness / run paths rely on this). + let all = discover_workflows(Some(home.path()), None, false); + let all_names: Vec<&str> = all.iter().map(|w| w.name.as_str()).collect(); + assert!(all_names.contains(&"ascii-art"), "got {all_names:?}"); + assert!(all_names.contains(&"deploy"), "got {all_names:?}"); +} + +#[test] +fn discover_automations_excludes_agents_skills_root() { + let home = tempfile::tempdir().unwrap(); + write( + &home + .path() + .join(".agents") + .join("skills") + .join("polymarket") + .join(SKILL_MD), + "---\nname: polymarket\ndescription: Query Polymarket\n---\n", + ); + + let automations = discover_automations(Some(home.path()), None, false); + assert!( + automations.is_empty(), + ".agents/skills bundles are skills, not automations; got {automations:?}" + ); + let all = discover_workflows(Some(home.path()), None, false); + assert_eq!(all.len(), 1); + assert_eq!(all[0].name, "polymarket"); +} + +#[test] +fn discover_automations_excludes_legacy_workspace_skills_root() { + let ws = tempfile::tempdir().unwrap(); + write( + &ws.path().join("skills").join("sketch").join(SKILL_MD), + "---\nname: sketch\ndescription: HTML mockups\n---\n", + ); + + let automations = discover_automations(None, Some(ws.path()), false); + assert!( + automations.is_empty(), + "legacy /skills is a skill root; got {automations:?}" + ); + // Full surface still scans legacy for back-compat. + let all = discover_workflows(None, Some(ws.path()), false); + assert_eq!(all.len(), 1); + assert_eq!(all[0].name, "sketch"); + assert_eq!(all[0].scope, WorkflowScope::Legacy); +} + +#[test] +fn discover_automations_includes_project_workflows_when_trusted() { + let ws = tempfile::tempdir().unwrap(); + write(&ws.path().join(".openhuman").join(TRUST_MARKER), ""); + write( + &ws.path() + .join(".openhuman") + .join("workflows") + .join("proj-flow") + .join(WORKFLOW_MD), + "---\nname: proj-flow\ndescription: Project automation\n---\n", + ); + // A sibling project skill must NOT leak into the automations list. + write( + &ws.path() + .join(".openhuman") + .join("skills") + .join("proj-skill") + .join(SKILL_MD), + "---\nname: proj-skill\ndescription: Project skill\n---\n", + ); + + let automations = discover_automations(None, Some(ws.path()), true); + let names: Vec<&str> = automations.iter().map(|w| w.name.as_str()).collect(); + assert_eq!(names, vec!["proj-flow"], "got {names:?}"); +} diff --git a/src/openhuman/workflows/schemas/handlers.rs b/src/openhuman/workflows/schemas/handlers.rs index 134d78f49..9da8473e6 100644 --- a/src/openhuman/workflows/schemas/handlers.rs +++ b/src/openhuman/workflows/schemas/handlers.rs @@ -12,7 +12,7 @@ use serde_json::{Map, Value}; use crate::core::all::ControllerFuture; use crate::openhuman::skill_runtime::spawn_workflow_run_background; use crate::openhuman::workflows::ops::{ - create_workflow, discover_workflows, install_workflow_from_url, is_workspace_trusted, + create_workflow, discover_automations, install_workflow_from_url, is_workspace_trusted, read_workflow_resource, uninstall_workflow, CreateWorkflowParams, UninstallWorkflowParams, }; use crate::openhuman::workflows::{registry, run_log}; @@ -31,18 +31,21 @@ use super::wire_types::{ pub(super) fn handle_workflows_list(params: Map) -> ControllerFuture { Box::pin(async move { let _ = deserialize_params::(params)?; - tracing::debug!("[skills][rpc] list skills"); + tracing::debug!("[workflows][rpc] list automations"); let workspace = resolve_workspace_dir().await; let trusted = is_workspace_trusted(&workspace); let home = dirs::home_dir(); - let skills = discover_workflows(home.as_deref(), Some(workspace.as_path()), trusted); + // Automations list shows only `workflows/`-root task templates — not the + // capability skills under `skills/` roots, which the agent harness still + // loads via `discover_workflows` / `load_workflow_metadata`. + let automations = discover_automations(home.as_deref(), Some(workspace.as_path()), trusted); tracing::debug!( - count = skills.len(), + count = automations.len(), workspace = %workspace.display(), trusted, - "[skills][rpc] list result" + "[workflows][rpc] list result" ); - let summaries = skills.into_iter().map(WorkflowSummary::from).collect(); + let summaries = automations.into_iter().map(WorkflowSummary::from).collect(); to_json(RpcOutcome::new( WorkflowsListResult { workflows: summaries,