diff --git a/AGENTS.md b/AGENTS.md index e9bc252853..2840c0ac12 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -236,6 +236,7 @@ GGML_NATIVE=OFF cargo check --manifest-path Cargo.toml \ | `web3` | ON | `openhuman::wallet` + `openhuman::web3` + `openhuman::x402` domains — crypto wallet (multi-chain sign/broadcast), swaps/bridges/dapp calls, x402 machine payments | `bitcoin`, `curve25519-dalek` | | `media` | ON | `openhuman::media_generation` (the `media_generate_*` agent tools) + `openhuman::image` scaffold | none (surface-only) | | `meet` | ON | `openhuman::meet` (join-URL validation) + `openhuman::meet_agent` (live STT/LLM/TTS loop) + `openhuman::agent_meetings` (backend-delegated Meet bot over Socket.IO) | none — see note | +| `skills` | ON | `openhuman::skills` + `openhuman::skill_runtime` + `openhuman::skill_registry` domains — SKILL.md discovery/parse/install, workflow execution + run logs, remote catalogs, the `skill_setup` / `skill_executor` builtin agents, and the 16 skill agent tools | none (see below) | **Facade pattern (pathfinder for the other gates).** `pub mod voice;` is **always compiled** as a facade: the real submodules are `#[cfg(feature = "voice")]`, and a `#[cfg(not(feature = "voice"))] mod stub;` (`src/openhuman/voice/stub.rs`) re-exposes the same public surface that always-on / other-gated callers use (`server`, `dictation_listener`, `streaming`, `reply_speech`, `cloud_transcribe`, `cli`, `create_stt_provider`, `effective_stt_provider`, `publish_ptt_transcript_committed`) with no-op / `None` / disabled-error bodies. Callers therefore do **not** need per-call `#[cfg]`. When voice is off: the voice/audio controllers are unregistered (unknown-method over `/rpc`, absent from `/schema`), the `audio_generate_podcast` agent tools are absent, and `openhuman voice` returns a "voice disabled" error. Stub signatures must match the real ones exactly — the disabled build (`--no-default-features --features tokenjuice-treesitter`) is the **only** thing that catches drift, so run it before pushing any change to the voice surface. @@ -256,6 +257,21 @@ GGML_NATIVE=OFF cargo check --manifest-path Cargo.toml \ **Both-ways tests.** `src/core/all_tests.rs` pins the gate in both directions (`meet_controllers_registered_when_feature_on` / `meet_controllers_absent_when_feature_off`). The negative half is the one that proves the gate removes anything. Note CI's smoke lane runs `cargo check` only and never compiles test code, so a disabled-build **test** break is invisible to it — run `cargo test --lib --no-default-features --features tokenjuice-treesitter core::all::tests` locally after touching any gated surface. +**`skills` gate — the type carve-out (read before adding the next gate).** The three skill domains follow the same facade+stub shape as `voice`, with one important refinement: **`skills` is not a leaf — it is partly load-bearing infrastructure.** `src/openhuman/tools/traits.rs` re-exports the crate's unified `ToolResult` / `ToolContent` out of `skills::types`, and ~236 files consume them (`mcp_client`, `runtime_node`, every `Tool` impl). `Workflow` / `WorkflowFrontmatter` / `WorkflowScope` from `skills::ops_types` likewise appear in always-on agent-harness and prompt signatures. Gating `skills` wholesale would take down the entire tool trait system, MCP, and the Node runtime. + +So `skills::types` and `skills::ops_types` stay **compiled in both directions** — they are inert serde/std-only definitions with zero coupling to their gated siblings — and only *behaviour* is gated. `src/openhuman/skills/stub.rs` therefore mirrors **functions only** and re-exports the real types (`pub use super::ops_types::{Workflow, …}`), so there is **zero type duplication** — strictly less drift surface than the `voice` stub, which had to re-declare `SttResult` + the `SttProvider` trait because those live inside its gated tree. + +> **Generalizable rule for the remaining gates:** put a domain's inert types in a dep-free submodule and leave it **ungated**; stub only the behaviour. Reach for a stub type only when the type genuinely cannot be carved out. + +Two places the carve-out doesn't reach, and why they are `#[cfg]` at the call site instead of stubbed: + +- `agent_registry/agents/loader.rs` — the `skill_setup` / `skill_executor` `BuiltinAgent` entries. `include_str!` embeds the agent TOML from disk regardless of module gating, so the entry itself must disappear. +- `agent/task_dispatcher/executor.rs` — the workflow-resolution branch. `registry::get_workflow` returns `Option`, which flattens in `AgentDefinition` and is destructured at the call site; stubbing it would mean re-declaring that struct (exactly what the carve-out avoids). With the domain compiled out no handle can resolve to a skill, so falling through to the builtin-agent branch is correct, not degraded. + +**Dep note:** `skills = []` — the empty list is **intentional, do not "fix" it**. Unlike `voice` (`hound`/`lettre`), these domains have no exclusive dependencies: every crate they touch is shared with always-on domains, and `runtime_node` / `runtime_python` are used by Agent / Flows / Memory too. This gate's value is tool-surface + prompt-bloat + startup cost, **not** binary size. + +When skills are off: the `skills` / `skill_runtime` / `skill_registry` controllers are unregistered (unknown-method over `/rpc`, absent from `/schema`), the 16 skill agent tools (incl. `run_workflow` / `await_workflow`) are **absent** from the tool list rather than degraded to an error, the `skill_setup` / `skill_executor` builtin agents are gone, and the boot-time remote catalog refresh is skipped. Composes with the runtime `DomainSet::skills` flag (#4796) — that axis needed no change here; #4798 is compile-time only. + ### Event bus (`src/core/event_bus/`) Typed pub/sub + native request/response. Both singletons — use module-level functions. diff --git a/Cargo.toml b/Cargo.toml index ec5e3b80b4..000e22a42a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -341,7 +341,7 @@ tokio = { version = "1", features = ["test-util"] } proptest = "1" [features] -default = ["tokenjuice-treesitter", "voice", "web3", "media", "meet"] +default = ["tokenjuice-treesitter", "voice", "web3", "media", "meet", "skills"] # AST-aware code compression (tree-sitter Rust/TS/Python grammars; C build). # On by default; disable to fall back to the brace-depth heuristic. tokenjuice-treesitter = [ @@ -398,6 +398,27 @@ media = [] # CARVE-OUT: `meet_agent::wav` stays compiled in ALL builds — the always-on # `desktop_companion` STT path depends on it. See `meet_agent/mod.rs`. meet = [] +# Skills domains: `openhuman::skills` (metadata/discovery/install), +# `openhuman::skill_runtime` (SKILL.md execution + the skill_executor agent), +# and `openhuman::skill_registry` (remote catalogs + the skill_setup agent). +# Default-ON — the desktop app always ships with skills. Slim / headless builds +# opt out via `--no-default-features --features ""`. +# Composes with the runtime `DomainSet::skills` flag (#4796): the feature +# narrows the compile-time surface, `DomainSet` gates it at runtime. +# +# The dep list is INTENTIONALLY EMPTY — do not "fix" it. Unlike `voice` +# (`hound`/`lettre`), these domains have no exclusive dependencies: every crate +# they touch (serde, serde_json, tokio, anyhow, async-trait, …) is shared with +# always-on domains, and `runtime_node`/`runtime_python` are used by Agent / +# Flows / Memory too. This gate's value is tool-surface + prompt-bloat + +# startup cost, NOT binary size. +# +# NOTE: `openhuman::skills::types` and `::ops_types` stay compiled even when +# this feature is OFF — `tools::traits` re-exports `ToolResult`/`ToolContent` +# out of `skills::types` and ~236 files consume them, so those inert serde +# types are a type carve-out, not part of the gated behaviour. See +# `src/openhuman/skills/stub.rs`. +skills = [] sandbox-landlock = ["dep:landlock"] sandbox-bubblewrap = [] peripheral-rpi = ["dep:rppal"] diff --git a/app/src-tauri/Cargo.toml b/app/src-tauri/Cargo.toml index 7ed60465c3..6909d53d45 100644 --- a/app/src-tauri/Cargo.toml +++ b/app/src-tauri/Cargo.toml @@ -161,6 +161,7 @@ openhuman_core = { path = "../..", package = "openhuman", default-features = fal "tokenjuice-treesitter", "web3", "meet", + "skills", ] } tinyjuice = { version = "0.2.1", default-features = false } diff --git a/src/core/all_tests.rs b/src/core/all_tests.rs index c5fbf6af43..6049ffebbf 100644 --- a/src/core/all_tests.rs +++ b/src/core/all_tests.rs @@ -234,6 +234,42 @@ fn voice_and_audio_controllers_absent_when_feature_off() { ); } +/// With the `skills` feature on (the default), all three skill domains are +/// compiled in and registered — the desktop build is byte-identical. +#[test] +#[cfg(feature = "skills")] +fn skill_controllers_registered_when_feature_on() { + let schemas = all_controller_schemas(); + for ns in ["skills", "skill_runtime", "skill_registry"] { + assert!( + schemas.iter().any(|s| s.namespace == ns), + "`{ns}` controllers must be registered when the `skills` feature is on" + ); + } +} + +/// With the `skills` feature off, all three domains are compiled out: their +/// controllers never enter the registry, so skills RPC methods are +/// unknown-method and absent from `/schema`. This is the compile-time +/// stub-facade correctness gate (see `openhuman::skills::stub`). +/// +/// Note this does NOT cover `skills::types` / `skills::ops_types`: those stay +/// compiled in both directions (the type carve-out — `tools::traits` re-exports +/// `ToolResult`/`ToolContent` out of them), but they expose no controllers, so +/// the namespaces are absent either way. +#[test] +#[cfg(not(feature = "skills"))] +fn skill_controllers_absent_when_feature_off() { + let schemas = all_controller_schemas(); + assert!( + !schemas.iter().any(|s| s.namespace == "skills" + || s.namespace == "skill_runtime" + || s.namespace == "skill_registry"), + "skills/skill_runtime/skill_registry controllers must be compiled out \ + when the `skills` feature is off" + ); +} + /// With the `web3` feature on (the default), the wallet + web3 + x402 /// controllers are compiled in and registered, and the high-level web3 agent /// tools (swap/bridge/dapp) are present — the desktop build is byte-identical. @@ -905,6 +941,9 @@ fn group_mapping_smoke() { assert_eq!(group_for_namespace("agent"), Some(DomainGroup::Agent)); // …and a representative gated one maps to its gate group. assert_eq!(group_for_namespace("flows"), Some(DomainGroup::Flows)); + // `group_for_namespace` is registry-derived, so a compile-time-gated domain + // has no controller to map. Skip when its Cargo feature is off. + #[cfg(feature = "skills")] assert_eq!(group_for_namespace("skills"), Some(DomainGroup::Skills)); assert_eq!(group_for_namespace("voice"), Some(DomainGroup::Voice)); #[cfg(feature = "web3")] diff --git a/src/openhuman/agent/harness/definition_tests.rs b/src/openhuman/agent/harness/definition_tests.rs index ed905d26dc..d6d8ebc0f7 100644 --- a/src/openhuman/agent/harness/definition_tests.rs +++ b/src/openhuman/agent/harness/definition_tests.rs @@ -376,6 +376,8 @@ fn all_builtin_agent_definitions_have_expected_effective_max_iterations() { ("tools_agent", 50), ("flow_discovery", 50), ("workflow_builder", 50), + // Compiled out with the `skills` gate — see `openhuman::skills::stub`. + #[cfg(feature = "skills")] ("skill_executor", 50), ("tinyplace_agent", 50), ("subconscious", 30), @@ -404,6 +406,8 @@ fn all_builtin_agent_definitions_have_expected_effective_max_iterations() { ("vision_agent", 6), // Unchanged. ("presentation_agent", 10), + // Compiled out with the `skills` gate — see `openhuman::skills::stub`. + #[cfg(feature = "skills")] ("skill_setup", 10), ]; diff --git a/src/openhuman/agent/harness/session/tests.rs b/src/openhuman/agent/harness/session/tests.rs index 992c028d67..da62b719cf 100644 --- a/src/openhuman/agent/harness/session/tests.rs +++ b/src/openhuman/agent/harness/session/tests.rs @@ -485,7 +485,11 @@ fn skill_listener_closed_channel_nulls_rx_and_is_not_a_signal() { ); } +/// Exercises real SKILL.md discovery from disk, so it is meaningful only with +/// the `skills` domain compiled in — the disabled facade's +/// `load_workflow_metadata` always returns an empty catalog by design. #[test] +#[cfg(feature = "skills")] fn refresh_workflows_picks_up_skill_installed_on_disk() { use crate::openhuman::skills::ops_types::{SKILL_MD, TRUST_MARKER}; @@ -551,7 +555,10 @@ fn refresh_workflows_picks_up_skill_installed_on_disk() { ); } +/// See [`refresh_workflows_picks_up_skill_installed_on_disk`] — same +/// disk-discovery dependency, so same `skills` gate. #[test] +#[cfg(feature = "skills")] fn refresh_workflows_retracts_skill_removed_from_disk() { use crate::openhuman::skills::ops_types::{SKILL_MD, TRUST_MARKER}; diff --git a/src/openhuman/agent/task_dispatcher/executor.rs b/src/openhuman/agent/task_dispatcher/executor.rs index daabdc7d7a..d54571bfca 100644 --- a/src/openhuman/agent/task_dispatcher/executor.rs +++ b/src/openhuman/agent/task_dispatcher/executor.rs @@ -6,7 +6,10 @@ use std::path::Path; -use crate::openhuman::agent::harness::definition::{AgentDefinitionRegistry, PromptSource}; +use crate::openhuman::agent::harness::definition::AgentDefinitionRegistry; +// Only read by the `skills`-gated workflow-resolution branch below. +#[cfg(feature = "skills")] +use crate::openhuman::agent::harness::definition::PromptSource; use crate::openhuman::agent::harness::session::Agent; use crate::openhuman::agent::harness::subagent_runner::with_autonomous_iter_cap; use crate::openhuman::agent::task_board::TaskCardStatus; @@ -69,6 +72,15 @@ pub(super) fn resolve_executor(workspace_dir: &Path, assigned: Option<&str>) -> } // 2) Workflow (#2824): the same autonomous run, seeded with SKILL.md. + // + // `#[cfg]` rather than a stubbed `get_workflow`: the real one returns + // `Option`, which flattens in `AgentDefinition` and is + // destructured just below — stubbing it would mean re-declaring that + // struct in the disabled facade (exactly the type duplication the skills + // carve-out exists to avoid). With the domain compiled out no handle can + // ever resolve to a skill, so falling through to (3) is the correct + // behaviour, not a degradation. + #[cfg(feature = "skills")] if let Some(skill) = crate::openhuman::skills::registry::get_workflow(workspace_dir, handle) { let guidelines = match &skill.definition.system_prompt { PromptSource::Inline(s) => truncate_chars(s, EXECUTOR_PREAMBLE_MAX_CHARS), diff --git a/src/openhuman/agent/tools.rs b/src/openhuman/agent/tools.rs index dcba4c3d27..cc399a060e 100644 --- a/src/openhuman/agent/tools.rs +++ b/src/openhuman/agent/tools.rs @@ -3,6 +3,10 @@ mod delegate; mod delegate_to_personality; mod plan_exit; pub mod remember_preference; +// Pure `skill_runtime` client (spawn + await a workflow run) — compiled out +// with the `skills` gate so the tool list OMITS these rather than degrading +// them to a disabled-error. +#[cfg(feature = "skills")] mod run_workflow; pub mod save_preference; mod todo; @@ -13,6 +17,7 @@ pub use delegate::DelegateTool; pub use delegate_to_personality::DelegateToPersonalityTool; pub use plan_exit::{PlanExitTool, PLAN_EXIT_MARKER}; pub use remember_preference::RememberPreferenceTool; +#[cfg(feature = "skills")] pub use run_workflow::{ AwaitWorkflowTool, RunWorkflowTool, AWAIT_WORKFLOW_TOOL_NAME, RUN_WORKFLOW_TOOL_NAME, }; diff --git a/src/openhuman/agent_registry/agents/loader.rs b/src/openhuman/agent_registry/agents/loader.rs index 1a9111fa13..c8ea3415a6 100644 --- a/src/openhuman/agent_registry/agents/loader.rs +++ b/src/openhuman/agent_registry/agents/loader.rs @@ -263,12 +263,17 @@ pub const BUILTINS: &[BuiltinAgent] = &[ prompt_fn: super::mcp_agent::prompt::build, graph_fn: None, }, + // Skill agents — `#[cfg]` rather than stub: `include_str!` embeds the + // agent TOML from disk regardless of module gating, so the entry itself + // must disappear when the `skills` feature is off. + #[cfg(feature = "skills")] BuiltinAgent { id: "skill_setup", toml: include_str!("../../skill_registry/agent/skill_setup/agent.toml"), prompt_fn: crate::openhuman::skill_registry::agent::skill_setup::prompt::build, graph_fn: None, }, + #[cfg(feature = "skills")] BuiltinAgent { id: "skill_executor", toml: include_str!("../../skill_runtime/agent/skill_executor/agent.toml"), diff --git a/src/openhuman/skill_registry/mod.rs b/src/openhuman/skill_registry/mod.rs index 5cacac9850..6461dd6db9 100644 --- a/src/openhuman/skill_registry/mod.rs +++ b/src/openhuman/skill_registry/mod.rs @@ -1,19 +1,44 @@ //! Skill registry: browse, search, and install skills from the aggregated //! Hermes catalog (HermesHub, ClawHub, skills.sh, LobeHub, browse.sh) //! with local caching. +//! +//! ## Compile-time gate (`skills` feature) +//! +//! `pub mod skill_registry;` is ALWAYS compiled — it is a facade. The real +//! implementation is gated behind the default-ON `skills` Cargo feature (the +//! same gate as `openhuman::skills` and `openhuman::skill_runtime` — the three +//! domains ship as one unit). When the feature is off, [`stub`] takes its +//! place with no-op / empty bodies. See `src/openhuman/skills/mod.rs` for the +//! pattern and the type carve-out. +#[cfg(feature = "skills")] pub mod agent; +#[cfg(feature = "skills")] pub mod ops; +#[cfg(feature = "skills")] pub mod schemas; +#[cfg(feature = "skills")] pub mod store; +#[cfg(feature = "skills")] pub mod tools; +#[cfg(feature = "skills")] pub mod types; +#[cfg(feature = "skills")] pub use schemas::{ all_skill_registry_controller_schemas, all_skill_registry_registered_controllers, }; /// Serializes tests that mutate the process-global `OPENHUMAN_SKILL_REGISTRY_CACHE_DIR` /// env var, so cargo's parallel runner can't interleave their cache dirs. -#[cfg(test)] +#[cfg(all(test, feature = "skills"))] pub(crate) static TEST_ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + +// --------------------------------------------------------------------------- +// Disabled facade — compiled only when the `skills` feature is OFF. +// --------------------------------------------------------------------------- + +#[cfg(not(feature = "skills"))] +mod stub; +#[cfg(not(feature = "skills"))] +pub use stub::*; diff --git a/src/openhuman/skill_registry/stub.rs b/src/openhuman/skill_registry/stub.rs new file mode 100644 index 0000000000..44d72065eb --- /dev/null +++ b/src/openhuman/skill_registry/stub.rs @@ -0,0 +1,46 @@ +//! Disabled-skills facade for the skill-registry domain. +//! +//! Compiled only when the `skills` Cargo feature is OFF (see the gate in +//! [`super`]). Mirrors only what always-on code reaches: the boot catalog +//! refresh kicked off by `core::runtime::services`, the controller aggregators +//! (`src/core/all.rs`), and the `tools` module glob +//! (`src/openhuman/tools/mod.rs`). Everything else — the catalog store, wire +//! types, and the `skill_setup` agent — is only referenced from code gated by +//! the same feature, so it vanishes alongside. +//! +//! The signatures here MUST match the real ones exactly. The disabled build +//! (`cargo check --no-default-features --features tokenjuice-treesitter`) is +//! the only thing that catches drift. + +use crate::core::all::RegisteredController; +use crate::core::ControllerSchema; + +/// Always empty: the `openhuman.skill_registry_*` controllers are compiled +/// out, so they never enter the registry (unknown-method over `/rpc`, absent +/// from `/schema`). +pub fn all_skill_registry_registered_controllers() -> Vec { + Vec::new() +} + +/// Always empty — see [`all_skill_registry_registered_controllers`]. +pub fn all_skill_registry_controller_schemas() -> Vec { + Vec::new() +} + +// --------------------------------------------------------------------------- +// ops::start_boot_catalog_refresh — called unconditionally at core boot from +// `src/core/runtime/services.rs`, so it must stay callable without a `#[cfg]` +// at that always-on site. +// --------------------------------------------------------------------------- + +pub mod ops { + /// No-op: there is no remote skill catalog to warm when the skill domains + /// are compiled out. Skips the boot-time network fetch entirely. + pub fn start_boot_catalog_refresh() { + log::debug!("[skill-registry-stub] start_boot_catalog_refresh skipped (skills disabled)"); + } +} + +// NOTE: no `tools` module here — the `pub use skill_registry::tools::*` glob in +// `tools/mod.rs` is `#[cfg(feature = "skills")]` instead, mirroring the `voice` +// gate. See the note in `skills/stub.rs`. diff --git a/src/openhuman/skill_runtime/mod.rs b/src/openhuman/skill_runtime/mod.rs index ea903d99d6..4d0dfedb5a 100644 --- a/src/openhuman/skill_runtime/mod.rs +++ b/src/openhuman/skill_runtime/mod.rs @@ -5,15 +5,40 @@ //! owns remote catalogs and install sources. This module owns actually running //! a skill, regardless of whether the skill's instructions call Python, Node, //! shell tools, or another OpenHuman agent tool. +//! +//! ## Compile-time gate (`skills` feature) +//! +//! `pub mod skill_runtime;` is ALWAYS compiled — it is a facade. The real +//! implementation is gated behind the default-ON `skills` Cargo feature (the +//! same gate as `openhuman::skills` and `openhuman::skill_registry` — the +//! three domains ship as one unit). When the feature is off, [`stub`] takes +//! its place with disabled-error / empty bodies. See +//! `src/openhuman/skills/mod.rs` for the pattern and the type carve-out. +#[cfg(feature = "skills")] pub mod agent; +#[cfg(feature = "skills")] pub mod ops; +#[cfg(feature = "skills")] mod run_machinery; +#[cfg(feature = "skills")] pub mod schemas; +#[cfg(feature = "skills")] pub mod tools; +#[cfg(feature = "skills")] pub use run_machinery::{await_run_outcome, spawn_workflow_run_background, WorkflowRunStarted}; +#[cfg(feature = "skills")] pub use schemas::{ all_skill_runtime_controller_schemas, all_skill_runtime_registered_controllers, skill_runtime_schemas, }; + +// --------------------------------------------------------------------------- +// Disabled facade — compiled only when the `skills` feature is OFF. +// --------------------------------------------------------------------------- + +#[cfg(not(feature = "skills"))] +mod stub; +#[cfg(not(feature = "skills"))] +pub use stub::*; diff --git a/src/openhuman/skill_runtime/stub.rs b/src/openhuman/skill_runtime/stub.rs new file mode 100644 index 0000000000..aa1d1aa56b --- /dev/null +++ b/src/openhuman/skill_runtime/stub.rs @@ -0,0 +1,33 @@ +//! Disabled-skills facade for the skill-runtime domain. +//! +//! Compiled only when the `skills` Cargo feature is OFF (see the gate in +//! [`super`]). Deliberately tiny: every caller of the run machinery +//! (`spawn_workflow_run_background`, `await_run_outcome`, `WorkflowRunStarted`) +//! lives inside code gated by the *same* feature — the `run_workflow` agent +//! tool, the `openhuman.skills_*` handlers, and this domain's own tests — so +//! those vanish together and the stub owes only what always-on code reaches: +//! the controller aggregators (`src/core/all.rs`) and the `tools` module glob +//! (`src/openhuman/tools/mod.rs`). +//! +//! The signatures here MUST match the real ones exactly. The disabled build +//! (`cargo check --no-default-features --features tokenjuice-treesitter`) is +//! the only thing that catches drift. + +use crate::core::all::RegisteredController; +use crate::core::ControllerSchema; + +/// Always empty: the `openhuman.skill_runtime_*` controllers are compiled out, +/// so they never enter the registry (unknown-method over `/rpc`, absent from +/// `/schema`). +pub fn all_skill_runtime_registered_controllers() -> Vec { + Vec::new() +} + +/// Always empty — see [`all_skill_runtime_registered_controllers`]. +pub fn all_skill_runtime_controller_schemas() -> Vec { + Vec::new() +} + +// NOTE: no `tools` module here — the `pub use skill_runtime::tools::*` glob in +// `tools/mod.rs` is `#[cfg(feature = "skills")]` instead, mirroring the `voice` +// gate. See the note in `skills/stub.rs`. diff --git a/src/openhuman/skills/mod.rs b/src/openhuman/skills/mod.rs index ac6643f138..9d24a2923a 100644 --- a/src/openhuman/skills/mod.rs +++ b/src/openhuman/skills/mod.rs @@ -1,28 +1,86 @@ //! Skills metadata helpers (discovery, parse, install, run). +//! +//! ## Compile-time gate (`skills` feature) +//! +//! `pub mod skills;` is ALWAYS compiled — it is a facade. The behavioural +//! submodules below are gated behind the default-ON `skills` Cargo feature; +//! when it is off, [`stub`] takes their place and exposes the same public +//! surface that always-on callers depend on (`load_workflow_metadata`, +//! `init_workflows_dir`, `registry`, `bus`, the controller aggregators) with +//! no-op / empty bodies. Callers therefore do **not** need per-call `#[cfg]`. +//! (The `tools` glob is `#[cfg(feature = "skills")]` at its re-export site in +//! `tools/mod.rs`, so the stub omits it — mirroring the `voice` gate.) +//! +//! ### Type carve-out (why `types` + `ops_types` are NOT gated) +//! +//! [`types`] and [`ops_types`] stay compiled in **both** directions. They are +//! inert serde/std-only type definitions with zero coupling to the gated +//! siblings, and they are load-bearing far outside this domain: +//! `tools::traits` re-exports `ToolResult`/`ToolContent` out of [`types`] as +//! the crate's unified tool-result type (`mcp_client`, `runtime_node`, and +//! ~236 files consume it), and `Workflow`/`WorkflowFrontmatter`/ +//! `WorkflowScope` from [`ops_types`] appear in always-on agent-harness and +//! prompt signatures. Gating them would take down the entire tool trait +//! system, MCP, and the Node runtime. +//! +//! The generalizable rule: **put inert types in a dep-free submodule and leave +//! it ungated; stub only the behaviour.** The stub below therefore mirrors +//! FUNCTIONS ONLY and re-exports the real types — zero type duplication, so +//! strictly less drift surface than the `voice` stub (which had to re-declare +//! `SttResult` + the `SttProvider` trait because those live inside its gated +//! tree). +//! +//! Signatures in [`stub`] must match the real ones exactly — the disabled +//! build (`cargo check --no-default-features --features tokenjuice-treesitter`) +//! is the only thing that catches drift. +// Type carve-out: always compiled, both feature directions. See module docs. +pub mod ops_types; +pub mod types; + +#[cfg(feature = "skills")] pub mod bus; +#[cfg(feature = "skills")] pub mod ops; +#[cfg(feature = "skills")] pub mod ops_create; +#[cfg(feature = "skills")] pub mod ops_discover; +#[cfg(feature = "skills")] pub mod ops_install; +#[cfg(feature = "skills")] pub mod ops_parse; -pub mod ops_types; +#[cfg(feature = "skills")] pub mod preflight; +#[cfg(feature = "skills")] pub mod registry; +#[cfg(feature = "skills")] pub mod run_log; +#[cfg(feature = "skills")] pub mod schemas; +#[cfg(feature = "skills")] pub mod tools; -pub mod types; -#[cfg(test)] +#[cfg(all(test, feature = "skills"))] #[path = "e2e_plumbing_tests.rs"] mod e2e_plumbing_tests; -#[cfg(test)] +#[cfg(all(test, feature = "skills"))] #[path = "e2e_run_tests.rs"] mod e2e_run_tests; +#[cfg(feature = "skills")] pub use ops::*; +#[cfg(feature = "skills")] pub use schemas::{ all_skills_controller_schemas, all_skills_registered_controllers, skills_schemas, }; + +// --------------------------------------------------------------------------- +// Disabled facade — compiled only when the `skills` feature is OFF. +// --------------------------------------------------------------------------- + +#[cfg(not(feature = "skills"))] +mod stub; +#[cfg(not(feature = "skills"))] +pub use stub::*; diff --git a/src/openhuman/skills/stub.rs b/src/openhuman/skills/stub.rs new file mode 100644 index 0000000000..a250923198 --- /dev/null +++ b/src/openhuman/skills/stub.rs @@ -0,0 +1,101 @@ +//! Disabled-skills facade. +//! +//! Compiled only when the `skills` Cargo feature is OFF (see the gate in +//! [`super`]). It mirrors the subset of the real `skills` public surface that +//! always-on callers depend on, with no-op / empty bodies so the crate still +//! compiles, boots, and serves `/rpc` without the skills domains. +//! +//! Unlike the `voice` stub, this one mirrors **functions only**: the domain's +//! types live in the ungated [`super::types`] / [`super::ops_types`] carve-out +//! and are re-exported verbatim below, so there is zero type duplication and +//! correspondingly less drift surface. +//! +//! The signatures here MUST match the real ones exactly (return types +//! included). The disabled build +//! (`cargo check --no-default-features --features tokenjuice-treesitter`) is +//! the only thing that catches drift — if a real signature changes, update the +//! mirror below until that build is green again. + +use std::path::Path; + +use crate::core::all::RegisteredController; +use crate::core::ControllerSchema; + +// --------------------------------------------------------------------------- +// Type surface — re-exported from the ungated carve-out, NOT re-declared. +// Mirrors the `pub use ops::*` → `pub use super::ops_types::{…}` chain that the +// real build exposes at the `skills` root (`skills::Workflow`, etc.). +// --------------------------------------------------------------------------- + +pub use super::ops_types::{ + Workflow, WorkflowFrontmatter, WorkflowScope, MAX_WORKFLOW_RESOURCE_BYTES, +}; + +// --------------------------------------------------------------------------- +// Discovery surface (mirrors `ops_discover::*` re-exported at the skills root) +// --------------------------------------------------------------------------- + +/// Always empty: no skills are discovered when the domain is compiled out. +/// Callers (agent harness, channel startup, prompt rendering) treat an empty +/// catalog as "user has no skills installed", which is the correct degraded +/// behaviour. +pub fn load_workflow_metadata(_workspace_dir: &Path) -> Vec { + log::debug!("[skills-stub] load_workflow_metadata -> [] (skills disabled)"); + Vec::new() +} + +/// No-op success: with skills compiled out there is no skills directory to +/// provision, and workspace bootstrap must not fail because of it. +pub fn init_workflows_dir(_workspace_dir: &Path) -> Result<(), String> { + log::debug!("[skills-stub] init_workflows_dir skipped (skills disabled)"); + Ok(()) +} + +// --------------------------------------------------------------------------- +// Controller aggregators — empty so `src/core/all.rs` needs no `#[cfg]`. +// --------------------------------------------------------------------------- + +/// Always empty: the `openhuman.skills_*` controllers are compiled out, so +/// they never enter the registry (unknown-method over `/rpc`, absent from +/// `/schema`). +pub fn all_skills_registered_controllers() -> Vec { + Vec::new() +} + +/// Always empty — see [`all_skills_registered_controllers`]. +pub fn all_skills_controller_schemas() -> Vec { + Vec::new() +} + +// --------------------------------------------------------------------------- +// registry::prune_legacy_default_workflows +// --------------------------------------------------------------------------- + +pub mod registry { + use std::path::Path; + + /// No-op: the legacy bundled-skill prune only removes directories this + /// domain created, and it never ran when skills are compiled out. + pub fn prune_legacy_default_workflows(_workspace_dir: &Path) { + log::debug!("[skills-stub] prune_legacy_default_workflows skipped (skills disabled)"); + } +} + +// --------------------------------------------------------------------------- +// bus::{ensure_triggered_workflow_subscriber, register_workflow_cleanup_subscriber} +// --------------------------------------------------------------------------- + +pub mod bus { + /// No-op: there are no triggered skills to subscribe to the event bus for. + pub fn ensure_triggered_workflow_subscriber(_workspace: &std::path::Path) { + log::debug!("[skills-stub] ensure_triggered_workflow_subscriber skipped (skills disabled)"); + } + + /// No-op: no skill run directories exist to clean up. + pub fn register_workflow_cleanup_subscriber() {} +} + +// NOTE: no `tools` module here. The `pub use skills::tools::*` glob in +// `tools/mod.rs` is `#[cfg(feature = "skills")]` instead — mirroring the +// `voice` gate's handling of the `audio_toolkit::tools::*` glob. An empty stub +// module would re-export nothing and trip `unused_imports` at the glob site. diff --git a/src/openhuman/tools/mod.rs b/src/openhuman/tools/mod.rs index 1b18ff8b4b..074838b02a 100644 --- a/src/openhuman/tools/mod.rs +++ b/src/openhuman/tools/mod.rs @@ -46,8 +46,11 @@ pub use crate::openhuman::screen_intelligence::tools::*; pub use crate::openhuman::search::tools::*; pub use crate::openhuman::security::tools::*; pub use crate::openhuman::service::tools::*; +#[cfg(feature = "skills")] pub use crate::openhuman::skill_registry::tools::*; +#[cfg(feature = "skills")] pub use crate::openhuman::skill_runtime::tools::*; +#[cfg(feature = "skills")] pub use crate::openhuman::skills::tools::*; pub use crate::openhuman::task_sources::tools::*; pub use crate::openhuman::team::tools::*; diff --git a/src/openhuman/tools/ops.rs b/src/openhuman/tools/ops.rs index adb7b52617..7369298d41 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -101,6 +101,11 @@ pub fn all_tools_with_runtime( skill_allowlist: Option<&std::collections::HashSet>, mcp_allowlist: Option<&[String]>, ) -> Vec> { + // `skill_allowlist` scopes only the `skills`-gated tool registrations + // below, so it is genuinely unread when that feature is compiled out. + #[cfg(not(feature = "skills"))] + let _ = skill_allowlist; + // Build a session-scoped managed Node.js bootstrap once, so ShellTool, // NodeExecTool, and NpmExecTool all share the same memoised resolution // state. Disabled when `node.enabled = false` — in that case shell skips @@ -220,7 +225,9 @@ pub fn all_tools_with_runtime( // Both wrap `skill_runtime::spawn_workflow_run_background` + // `await_run_outcome` — the same spawn path `openhuman.skills_run` // JSON-RPC uses, so RPC and tool callers stay in sync. + #[cfg(feature = "skills")] Box::new(RunWorkflowTool::new().with_skill_allowlist(skill_allowlist.cloned())), + #[cfg(feature = "skills")] Box::new(AwaitWorkflowTool::new()), Box::new(CurrentTimeTool::new()), // Reversibility for native tool-output compaction (Stage 1a): when a @@ -472,9 +479,11 @@ pub fn all_tools_with_runtime( // above, so it is not duplicated. Reads ship default-ON; the // create/install/uninstall mutators ship default-OFF via // `tools::user_filter` (install also fetches remote content). + #[cfg(feature = "skills")] Box::new( WorkflowListTool::new(config.clone()).with_skill_allowlist(skill_allowlist.cloned()), ), + #[cfg(feature = "skills")] Box::new( WorkflowDescribeTool::new(config.clone()) .with_skill_allowlist(skill_allowlist.cloned()), @@ -482,22 +491,34 @@ pub fn all_tools_with_runtime( // Skill registry tools — browse/search/install from remote registries. // Browse and search are read-only (default-ON); install is a write // operation (fetches remote content and writes to disk). + #[cfg(feature = "skills")] Box::new(SkillRegistryBrowseTool), + #[cfg(feature = "skills")] Box::new(SkillRegistrySearchTool), + #[cfg(feature = "skills")] Box::new(SkillRegistryInstallTool::new(config.clone())), + #[cfg(feature = "skills")] Box::new(SkillRegistrySourcesTool), + #[cfg(feature = "skills")] Box::new(SkillRegistryUninstallTool), // Skill runtime probes — resolve the reusable Node/Python runtimes // that skill execution relies on before a script-backed skill runs. + #[cfg(feature = "skills")] Box::new(SkillRuntimeResolveRuntimesTool::new(config.clone())), + #[cfg(feature = "skills")] Box::new( WorkflowReadResourceTool::new(config.clone()) .with_skill_allowlist(skill_allowlist.cloned()), ), + #[cfg(feature = "skills")] Box::new(WorkflowRecentRunsTool::new(config.clone())), + #[cfg(feature = "skills")] Box::new(WorkflowReadRunLogTool::new(config.clone())), + #[cfg(feature = "skills")] Box::new(WorkflowCreateTool::new(config.clone())), + #[cfg(feature = "skills")] Box::new(WorkflowInstallFromUrlTool::new(config.clone())), + #[cfg(feature = "skills")] Box::new(WorkflowUninstallTool), // Threads (conversation) tools. Read/bounded-write ship default-ON; // the destructive thread_delete / thread_purge_all ship default-OFF