mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(workflows): Automations list shows only task templates, not capability skills (#3581)
This commit is contained in:
Cyrus Gray
@@ -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,
|
||||
|
||||
@@ -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 `<workspace>/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<Workflow> {
|
||||
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
|
||||
/// `<workspace>/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<Workflow> {
|
||||
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<Workflow> {
|
||||
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<String, Workflow> = 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 `<workspace>/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 `<workspace>/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<Workflow> = 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<PathBuf> {
|
||||
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<PathBuf> {
|
||||
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,
|
||||
),
|
||||
]
|
||||
}
|
||||
|
||||
|
||||
@@ -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 <workspace>/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:?}");
|
||||
}
|
||||
|
||||
@@ -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<String, Value>) -> ControllerFuture {
|
||||
Box::pin(async move {
|
||||
let _ = deserialize_params::<WorkflowsListParams>(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,
|
||||
|
||||
Reference in New Issue
Block a user