feat(core): compile-time skills feature gate (#4798)

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<WorkflowDefinition>`, 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.

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 "<explicit list without skills>"`.
+# 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"]

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 }
 

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")]

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),
     ];
 

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};
 

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<WorkflowDefinition>`, 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),

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,
 };

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"),

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::*;

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<RegisteredController> {
+    Vec::new()
+}
+
+/// Always empty — see [`all_skill_registry_registered_controllers`].
+pub fn all_skill_registry_controller_schemas() -> Vec<ControllerSchema> {
+    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`.