refactor(skills): unify auth/oauth handshake on start({validate}) and drop enabled flag (#484)

* refactor(preferences): streamline skill preference management by removing the enabled toggle

- Removed the `enabled` field from `SkillPreference`, simplifying the preference model to focus solely on `setup_complete`.
- Updated related methods and RPC handlers to reflect this change, ensuring that skills are automatically started based on the completion of their setup process.
- Adjusted tests to validate the new behavior, ensuring consistent functionality without the `enabled` toggle.
- Enhanced documentation to clarify the new preference management approach.

* refactor(auth): simplify authentication flow by removing onAuthComplete hook

- Updated the `handle_auth_complete` function to eliminate the separate `onAuthComplete` JavaScript hook, streamlining the authentication process.
- Revised the flow to directly inject new credentials and validate them using the `start` function, which now handles both validation and activation.
- Enhanced rollback logic to ensure temporary credentials are cleared if validation fails, preventing persistence on disk.
- Improved documentation to clarify the new authentication steps and their implications for credential management.

* refactor(oauth): streamline OAuth credential handling in `handle_oauth_complete`

- Removed the `build_start_credentials_arg` function and integrated its logic directly into `handle_oauth_complete`, simplifying the flow.
- Updated the OAuth handling to validate credentials before persisting them, ensuring that only successful validations are saved.
- Enhanced rollback logic to clear temporary credentials if validation fails, preventing incorrect state persistence.
- Improved documentation to clarify the new steps in the OAuth process and their implications for credential management.

src/openhuman/skills/preferences.rs

@@ -1,8 +1,10 @@
 //! Persistent skill preferences management.
 //!
-//! This module manages user-defined preferences for skills, such as whether a skill
-//! is enabled and whether its initial setup has been completed. Preferences are
-//! persisted to a JSON file on disk, ensuring they survive application restarts.
+//! This module manages user-defined preferences for skills. The only thing it
+//! tracks today is whether a skill's setup process (e.g. OAuth) has been
+//! completed. Setup completion is the single source of truth for "should this
+//! skill be running": there is no separate `enabled` toggle. Preferences are
+//! persisted to a JSON file on disk so they survive application restarts.
 
 use parking_lot::RwLock;
 use serde::{Deserialize, Serialize};
@@ -10,11 +12,10 @@ use std::collections::HashMap;
 use std::path::PathBuf;
 
 /// Represents the user's persistent preferences for a single skill.
-#[derive(Debug, Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize, Default)]
 pub struct SkillPreference {
-    /// Whether the skill is currently enabled by the user.
-    pub enabled: bool,
     /// Whether the skill's required setup process (e.g., OAuth) has been completed.
+    /// A skill with `setup_complete = true` is automatically started by the runtime.
     #[serde(default)]
     pub setup_complete: bool,
 }
@@ -44,6 +45,8 @@ impl PreferencesStore {
     /// Loads preferences from the persistent file on disk into the in-memory cache.
     ///
     /// If the file is missing or contains invalid JSON, the cache is initialized as empty.
+    /// Legacy entries that still carry an `enabled` field are silently ignored — only
+    /// `setup_complete` is read.
     fn load(&self) {
         if let Ok(content) = std::fs::read_to_string(&self.path) {
             if let Ok(prefs) = serde_json::from_str::<HashMap<String, SkillPreference>>(&content) {
@@ -67,58 +70,30 @@ impl PreferencesStore {
         }
     }
 
-    /// Retrieves the preference for a skill, returning a default object if no preference exists.
-    fn get_or_default(&self, skill_id: &str) -> SkillPreference {
-        self.cache
-            .read()
-            .get(skill_id)
-            .cloned()
-            .unwrap_or(SkillPreference {
-                enabled: false,
-                setup_complete: false,
-            })
-    }
-
     /// Updates the preference for a skill using the provided closure and persists the change.
     fn update<F: FnOnce(&mut SkillPreference)>(&self, skill_id: &str, f: F) {
         let mut cache = self.cache.write();
         let pref = cache
             .entry(skill_id.to_string())
-            .or_insert(SkillPreference {
-                enabled: false,
-                setup_complete: false,
-            });
+            .or_insert(SkillPreference::default());
         f(pref);
         drop(cache); // Explicitly release lock before saving to avoid potential deadlocks
         self.save();
     }
 
-    /// Returns whether a skill is explicitly enabled by the user.
-    ///
-    /// Returns `None` if no preference has been set for this skill.
-    pub fn is_enabled(&self, skill_id: &str) -> Option<bool> {
-        self.cache.read().get(skill_id).map(|p| p.enabled)
-    }
-
-    /// Sets the enabled preference for a skill and persists it to disk.
-    pub fn set_enabled(&self, skill_id: &str, enabled: bool) {
-        self.update(skill_id, |p| p.enabled = enabled);
-    }
-
     /// Checks if a skill's setup process has been recorded as complete.
     pub fn is_setup_complete(&self, skill_id: &str) -> bool {
-        self.get_or_default(skill_id).setup_complete
+        self.cache
+            .read()
+            .get(skill_id)
+            .map(|p| p.setup_complete)
+            .unwrap_or(false)
     }
 
     /// Marks a skill's setup as complete (or incomplete) and persists the state.
-    ///
-    /// If marked as complete, the skill is also automatically marked as `enabled`.
     pub fn set_setup_complete(&self, skill_id: &str, complete: bool) {
         self.update(skill_id, |p| {
             p.setup_complete = complete;
-            if complete {
-                p.enabled = true;
-            }
         });
         log::info!(
             "[preferences] setup_complete for '{}' set to {}",
@@ -136,15 +111,12 @@ impl PreferencesStore {
     ///
     /// Priority order:
     /// 1. If `setup_complete` is true, the skill should start.
-    /// 2. If an explicit `enabled` preference exists, use it.
-    /// 3. Otherwise, fall back to the `auto_start` value from the skill's manifest.
+    /// 2. Otherwise, fall back to the `auto_start` value from the skill's manifest.
     pub fn resolve_should_start(&self, skill_id: &str, manifest_auto_start: bool) -> bool {
-        let pref = self.cache.read().get(skill_id).cloned();
-        match pref {
-            Some(p) if p.setup_complete => true,
-            Some(p) => p.enabled,
-            None => manifest_auto_start,
+        if self.is_setup_complete(skill_id) {
+            return true;
         }
+        manifest_auto_start
     }
 }
 
@@ -159,21 +131,13 @@ mod tests {
     }
 
     #[test]
-    fn enable_disable_roundtrip() {
-        let (store, _dir) = temp_store();
-        assert_eq!(store.is_enabled("my-skill"), None);
-        store.set_enabled("my-skill", true);
-        assert_eq!(store.is_enabled("my-skill"), Some(true));
-        store.set_enabled("my-skill", false);
-        assert_eq!(store.is_enabled("my-skill"), Some(false));
-    }
-
-    #[test]
-    fn setup_complete_also_enables() {
+    fn setup_complete_roundtrip() {
         let (store, _dir) = temp_store();
+        assert!(!store.is_setup_complete("s1"));
         store.set_setup_complete("s1", true);
         assert!(store.is_setup_complete("s1"));
-        assert_eq!(store.is_enabled("s1"), Some(true));
+        store.set_setup_complete("s1", false);
+        assert!(!store.is_setup_complete("s1"));
     }
 
     #[test]
@@ -182,12 +146,10 @@ mod tests {
         let path = dir.path().to_path_buf();
         {
             let store = PreferencesStore::new(&path);
-            store.set_enabled("x", true);
             store.set_setup_complete("x", true);
         }
         // Reload from disk
         let store2 = PreferencesStore::new(&path);
-        assert_eq!(store2.is_enabled("x"), Some(true));
         assert!(store2.is_setup_complete("x"));
     }
 
@@ -195,7 +157,6 @@ mod tests {
     fn missing_file_returns_defaults() {
         let dir = tempfile::tempdir().unwrap();
         let store = PreferencesStore::new(&dir.path().to_path_buf());
-        assert_eq!(store.is_enabled("nonexistent"), None);
         assert!(!store.is_setup_complete("nonexistent"));
     }
 
@@ -205,26 +166,17 @@ mod tests {
         let file = dir.path().join("skill-preferences.json");
         std::fs::write(&file, "not valid json {{{}").unwrap();
         let store = PreferencesStore::new(&dir.path().to_path_buf());
-        assert_eq!(store.is_enabled("any"), None);
+        assert!(!store.is_setup_complete("any"));
     }
 
     #[test]
-    fn resolve_should_start_setup_complete_overrides() {
+    fn resolve_should_start_setup_complete_overrides_manifest() {
         let (store, _dir) = temp_store();
         store.set_setup_complete("s1", true);
         // Even if manifest says false, setup_complete wins
         assert!(store.resolve_should_start("s1", false));
     }
 
-    #[test]
-    fn resolve_should_start_falls_back_to_enabled() {
-        let (store, _dir) = temp_store();
-        store.set_enabled("s1", true);
-        assert!(store.resolve_should_start("s1", false));
-        store.set_enabled("s1", false);
-        assert!(!store.resolve_should_start("s1", true));
-    }
-
     #[test]
     fn resolve_should_start_falls_back_to_manifest() {
         let (store, _dir) = temp_store();
@@ -235,11 +187,11 @@ mod tests {
     #[test]
     fn get_all_returns_complete_map() {
         let (store, _dir) = temp_store();
-        store.set_enabled("a", true);
-        store.set_enabled("b", false);
+        store.set_setup_complete("a", true);
+        store.set_setup_complete("b", false);
         let all = store.get_all();
         assert_eq!(all.len(), 2);
-        assert!(all["a"].enabled);
-        assert!(!all["b"].enabled);
+        assert!(all["a"].setup_complete);
+        assert!(!all["b"].setup_complete);
     }
 }

src/openhuman/skills/qjs_engine.rs

@@ -675,27 +675,6 @@ impl RuntimeEngine {
         }
     }
 
-    /// Enable a skill and start it.
-    pub async fn enable_skill(&self, skill_id: &str) -> Result<(), String> {
-        self.preferences.set_enabled(skill_id, true);
-        self.start_skill(skill_id).await?;
-        Ok(())
-    }
-
-    /// Disable a skill and stop it if it's running.
-    pub async fn disable_skill(&self, skill_id: &str) -> Result<(), String> {
-        self.preferences.set_enabled(skill_id, false);
-        if self.registry.has_skill(skill_id) {
-            self.stop_skill(skill_id).await?;
-        }
-        Ok(())
-    }
-
-    /// Check whether a skill is enabled in user preferences.
-    pub fn is_skill_enabled(&self, skill_id: &str) -> bool {
-        self.preferences.is_enabled(skill_id).unwrap_or(false)
-    }
-
     /// Get all stored skill preferences.
     pub fn get_preferences(
         &self,

src/openhuman/skills/qjs_skill_instance/event_loop/mod.rs

@@ -636,7 +636,7 @@ async fn handle_message(
         }
         SkillMessage::Stop { reply } => {
             // Clean up lifecycle and clear credentials from the runtime
-            let _ = call_lifecycle(rt, ctx, "stop").await;
+            let _ = call_lifecycle(rt, ctx, "stop", None).await;
 
             let clear_code = r#"(function() {
                 if (typeof globalThis.oauth !== 'undefined' && globalThis.oauth.__setCredential) {

src/openhuman/skills/qjs_skill_instance/event_loop/rpc_handlers.rs

@@ -11,17 +11,16 @@ use tokio::sync::mpsc;
 
 use crate::openhuman::{memory::MemoryClientRef, skills::quickjs_libs::qjs_ops};
 
+use super::super::js_handlers::{handle_js_call, handle_js_void_call};
 use super::{persist_state_to_memory, MemoryWriteJob};
-use crate::openhuman::skills::qjs_skill_instance::js_handlers::{
-    handle_js_call, handle_js_void_call,
-};
 
 /// Handle `oauth/complete` RPC.
 ///
-/// 1. Injects the new OAuth credential into the JS runtime.
-/// 2. Persists the credential to `{data_dir}/oauth_credential.json`.
-/// 3. Injects and persists the `clientKeyShare` if present.
-/// 4. Invokes the `onOAuthComplete` lifecycle handler in JS.
+/// Mirrors `handle_auth_complete`: temp-inject the new credentials into the
+/// JS bridges, ask `start({ oauth, validate: true })` to validate them
+/// against the upstream API, and only persist to disk if start() returns
+/// `{ status: "complete" }`. There is no separate `onOAuthComplete` JS hook
+/// — `start()` is the single entry point for "this skill is now connected".
 pub(crate) async fn handle_oauth_complete(
     rt: &rquickjs::AsyncRuntime,
     ctx: &rquickjs::AsyncContext,
@@ -31,7 +30,9 @@ pub(crate) async fn handle_oauth_complete(
 ) -> Result<serde_json::Value, String> {
     let cred_json = serde_json::to_string(&params).unwrap_or_else(|_| "null".to_string());
 
-    // Extract client key share (required for encrypted OAuth proxy requests)
+    // Extract client key share (required for encrypted OAuth proxy requests).
+    // We have to inject it before start() runs because the proxy validation
+    // call inside start() needs it to encrypt the outgoing request.
     let client_key = params
         .get("clientKeyShare")
         .and_then(|v| v.as_str())
@@ -46,8 +47,9 @@ pub(crate) async fn handle_oauth_complete(
         String::new()
     };
 
-    // Inject credentials into both the bridge-level `oauth` object and the general `state` object
-    let code = format!(
+    // Step 1: Temporary injection so the JS `oauth` bridge sees the new
+    // credential while start() runs validation through the proxy.
+    let inject_code = format!(
         r#"(function() {{
             if (typeof globalThis.oauth !== 'undefined' && globalThis.oauth.__setCredential) {{
                 globalThis.oauth.__setCredential({cred});
@@ -61,11 +63,60 @@ pub(crate) async fn handle_oauth_complete(
         client_key_js = client_key_js,
     );
     ctx.with(|js_ctx| {
-        let _ = js_ctx.eval::<rquickjs::Value, _>(code.as_bytes());
+        let _ = js_ctx.eval::<rquickjs::Value, _>(inject_code.as_bytes());
     })
     .await;
 
-    // Persist to disk for restoration after skill/app restart
+    // Step 2: Build a `{ oauth, auth, validate: true }` arg bag and call
+    // start(). Auth comes from disk so the skill still has the full picture
+    // if it's already connected via auth/complete; oauth is the freshly
+    // submitted credential we want validated.
+    let auth_on_disk = match std::fs::read_to_string(data_dir.join("auth_credential.json")) {
+        Ok(s) if !s.trim().is_empty() => {
+            serde_json::from_str::<serde_json::Value>(&s).unwrap_or(serde_json::Value::Null)
+        }
+        _ => serde_json::Value::Null,
+    };
+    let start_args_value = serde_json::json!({
+        "oauth": params,
+        "auth": auth_on_disk,
+        "validate": true,
+    });
+    let start_args =
+        serde_json::to_string(&start_args_value).unwrap_or_else(|_| "null".to_string());
+    let result = handle_js_call(rt, ctx, "start", &start_args).await;
+
+    // Evaluate validation result
+    let validation_failed = match &result {
+        Err(_) => true,
+        Ok(val) => val.get("status").and_then(|s| s.as_str()) == Some("error"),
+    };
+
+    if validation_failed {
+        // Rollback temporary injection — credential never made it to disk so
+        // a follow-up restart will see the skill as disconnected again.
+        let clear_code = r#"(function() {
+            if (typeof globalThis.oauth !== 'undefined' && globalThis.oauth.__setCredential) {
+                globalThis.oauth.__setCredential(null);
+            }
+            if (typeof globalThis.state !== 'undefined' && globalThis.state.set) {
+                globalThis.state.set('__oauth_credential', '');
+            }
+            globalThis.__oauthClientKey = null;
+        })()"#;
+        ctx.with(|js_ctx| {
+            let _ = js_ctx.eval::<rquickjs::Value, _>(clear_code.as_bytes());
+        })
+        .await;
+        log::info!(
+            "[skill:{}] oauth/complete validation failed, credential not persisted",
+            skill_id
+        );
+        return result;
+    }
+
+    // Step 3: Validation passed — persist credentials and client key to disk
+    // so they survive restarts.
     let cred_path = data_dir.join("oauth_credential.json");
     if let Err(e) = std::fs::write(&cred_path, &cred_json) {
         log::error!(
@@ -97,8 +148,7 @@ pub(crate) async fn handle_oauth_complete(
         }
     }
 
-    let params_str = serde_json::to_string(&params).unwrap_or_else(|_| "{}".to_string());
-    handle_js_call(rt, ctx, "onOAuthComplete", &params_str).await
+    result
 }
 
 /// Handle `oauth/revoked` RPC.
@@ -163,10 +213,14 @@ pub(crate) async fn handle_oauth_revoked(
 
 /// Handle `auth/complete` RPC.
 ///
-/// Performs a 2-step process:
-/// 1. Temporarily injects credentials and calls `onAuthComplete` for validation.
-/// 2. If validation succeeds (status != "error"), persists credentials to disk
-///    and permanently injects them into the runtime.
+/// There is no separate `onAuthComplete` JS hook anymore. The flow is:
+///   1. Temporarily inject the new credentials into the JS bridges.
+///   2. Call `start({ oauth, auth, validate: true })`. start() owns both
+///      validation (when `validate` is set) and activation (cron, etc.) — it
+///      returns `{ status: "complete" }` or `{ status: "error", errors: [...] }`.
+///   3. If start() returned an error, roll back the temporary injection and
+///      surface the result to the caller (no persistence on disk).
+///   4. Otherwise persist the credentials to disk so they survive restarts.
 pub(crate) async fn handle_auth_complete(
     rt: &rquickjs::AsyncRuntime,
     ctx: &rquickjs::AsyncContext,
@@ -181,7 +235,8 @@ pub(crate) async fn handle_auth_complete(
         .map(|m| m == "managed")
         .unwrap_or(false);
 
-    // Step 1: Temporary injection for validation
+    // Step 1: Temporary injection so the JS bridges (`auth`, and `oauth` for
+    // managed mode) see the new credentials while start() runs validation.
     let temp_code = format!(
         r#"(function() {{
             if (typeof globalThis.auth !== 'undefined' && globalThis.auth.__setCredential) {{
@@ -195,8 +250,24 @@ pub(crate) async fn handle_auth_complete(
     })
     .await;
 
-    let params_str = serde_json::to_string(&params).unwrap_or_else(|_| "{}".to_string());
-    let result = handle_js_call(rt, ctx, "onAuthComplete", &params_str).await;
+    // Step 2: Build a `{ oauth, auth, validate: true }` arg bag and call
+    // start(). We pass the freshly-submitted auth params (not what's on disk)
+    // so the validation step inside start() inspects the *new* credentials.
+    // OAuth comes from disk so the skill still has the full picture.
+    let oauth_on_disk = match std::fs::read_to_string(data_dir.join("oauth_credential.json")) {
+        Ok(s) if !s.trim().is_empty() => {
+            serde_json::from_str::<serde_json::Value>(&s).unwrap_or(serde_json::Value::Null)
+        }
+        _ => serde_json::Value::Null,
+    };
+    let start_args_value = serde_json::json!({
+        "oauth": oauth_on_disk,
+        "auth": params,
+        "validate": true,
+    });
+    let start_args =
+        serde_json::to_string(&start_args_value).unwrap_or_else(|_| "null".to_string());
+    let result = handle_js_call(rt, ctx, "start", &start_args).await;
 
     // Evaluate validation result
     let validation_failed = match &result {
@@ -205,7 +276,8 @@ pub(crate) async fn handle_auth_complete(
     };
 
     if validation_failed {
-        // Rollback temporary injection
+        // Rollback temporary injection — credentials never made it to disk so
+        // a follow-up restart will see the skill as disconnected again.
         let clear_code = r#"(function() {
             if (typeof globalThis.auth !== 'undefined' && globalThis.auth.__setCredential) {
                 globalThis.auth.__setCredential(null);
@@ -222,7 +294,9 @@ pub(crate) async fn handle_auth_complete(
         return result;
     }
 
-    // Step 2: Permanent injection and persistence
+    // Step 3: Permanent injection and persistence. start() already activated
+    // cron etc. above; this just makes sure the bridges and disk reflect the
+    // new credentials so a future restart restores the same state.
     let managed_bridge = if is_managed {
         let creds_json = serde_json::to_string(
             params

src/openhuman/skills/qjs_skill_instance/instance.rs

@@ -22,6 +22,32 @@ use super::js_helpers::{
 };
 use super::types::{BridgeDeps, QjsSkillInstance, SkillState};
 
+/// Read persisted oauth/auth credentials from a skill's data directory and
+/// produce a JS expression suitable for `start({ oauth, auth })`.
+///
+/// This is the canonical shape passed to `start()`: a single object with
+/// `oauth` and `auth` keys, each either an object or `null`. Skills can read
+/// either field directly or rely on the runtime bridges that have already
+/// been populated by the `restore_*_credential` helpers.
+pub(crate) fn build_start_credentials_arg(data_dir: &std::path::Path) -> String {
+    fn read_json(path: &std::path::Path) -> serde_json::Value {
+        match std::fs::read_to_string(path) {
+            Ok(s) if !s.trim().is_empty() => {
+                serde_json::from_str(&s).unwrap_or(serde_json::Value::Null)
+            }
+            _ => serde_json::Value::Null,
+        }
+    }
+
+    let oauth = read_json(&data_dir.join("oauth_credential.json"));
+    let auth = read_json(&data_dir.join("auth_credential.json"));
+    let arg = serde_json::json!({
+        "oauth": oauth,
+        "auth": auth,
+    });
+    serde_json::to_string(&arg).unwrap_or_else(|_| "{\"oauth\":null,\"auth\":null}".to_string())
+}
+
 impl QjsSkillInstance {
     /// Create a new QuickJS skill instance.
     ///
@@ -212,7 +238,7 @@ impl QjsSkillInstance {
             restore_client_key(&ctx, &config.skill_id, &data_dir).await;
 
             // Trigger the `init()` lifecycle callback in the JS skill
-            if let Err(e) = call_lifecycle(&rt, &ctx, "init").await {
+            if let Err(e) = call_lifecycle(&rt, &ctx, "init", None).await {
                 let mut s = state.write();
                 s.status = SkillStatus::Error;
                 s.error = Some(format!("init() failed: {e}"));
@@ -223,8 +249,22 @@ impl QjsSkillInstance {
             // Execute any microtasks or pending promises scheduled during init()
             drive_jobs(&rt).await;
 
+            // Build the credential bag passed to `start(creds)`. We read the
+            // persisted credential files from disk so the skill receives the
+            // canonical view that matches what the bridges (`oauth`, `auth`)
+            // already see in JS land. If no creds are stored we still pass an
+            // explicit `{ oauth: null, auth: null }` so start() always has a
+            // well-defined shape — that is the whole point of this contract.
+            let start_args = build_start_credentials_arg(&data_dir);
+            log::info!(
+                "[skill:{}] Calling start() with credentials (oauth={}, auth={})",
+                config.skill_id,
+                !start_args.contains("\"oauth\":null"),
+                !start_args.contains("\"auth\":null"),
+            );
+
             // Trigger the `start()` lifecycle callback
-            if let Err(e) = call_lifecycle(&rt, &ctx, "start").await {
+            if let Err(e) = call_lifecycle(&rt, &ctx, "start", Some(&start_args)).await {
                 let mut s = state.write();
                 s.status = SkillStatus::Error;
                 s.error = Some(format!("start() failed: {e}"));

src/openhuman/skills/qjs_skill_instance/js_handlers.rs

@@ -16,12 +16,21 @@ use super::js_helpers::{drive_jobs, format_js_exception};
 /// Handles both synchronous and asynchronous (Promise-returning) lifecycle
 /// methods like `init`, `start`, and `stop`. If the JS function returns a Promise,
 /// this handler will poll for completion for up to 30 seconds.
+///
+/// `args_json` is a JavaScript expression injected as the first argument when
+/// calling the lifecycle function. Pass `None` to call the function with no
+/// arguments. Use this to forward credentials into `start(creds)`.
 pub(crate) async fn call_lifecycle(
     rt: &rquickjs::AsyncRuntime,
     ctx: &rquickjs::AsyncContext,
     name: &str,
+    args_json: Option<&str>,
 ) -> Result<(), String> {
     let name = name.to_string();
+    // Default to no argument so existing call sites (init, stop) keep their
+    // current behavior. When args_json is supplied it must already be a valid
+    // JS expression (e.g. JSON or `null`).
+    let args_expr = args_json.unwrap_or("").to_string();
 
     // First, try to initiate the call in the JS context
     let is_promise = ctx
@@ -35,7 +44,7 @@ pub(crate) async fn call_lifecycle(
                     : (globalThis.__skill || globalThis);
                 var fn = skill.{name} || globalThis.{name};
                 if (typeof fn === 'function') {{
-                    var result = fn.call(skill);
+                    var result = fn.call(skill, {args_expr});
                     if (result && typeof result.then === 'function') {{
                         // Function returned a Promise, set up global tracking flags
                         globalThis.__pendingLifecycleDone = false;

src/openhuman/skills/schemas.rs

@@ -893,35 +893,47 @@ fn handle_skills_data_stats(params: Map<String, Value>) -> ControllerFuture {
     })
 }
 
-/// RPC handler to enable a skill in user preferences and start it.
+/// RPC handler to enable a skill: marks setup_complete and starts it.
+///
+/// Skills no longer have a separate `enabled` toggle — the only state is
+/// "setup is complete" (driven by oauth/auth completion). This handler exists
+/// as a back-compat shim for the frontend; it sets `setup_complete = true`
+/// and starts the skill in one shot.
 fn handle_skills_enable(params: Map<String, Value>) -> ControllerFuture {
     Box::pin(async move {
         let p: SkillIdParams =
             serde_json::from_value(Value::Object(params)).map_err(|e| e.to_string())?;
         let engine = require_engine()?;
-        engine.enable_skill(&p.skill_id).await?;
+        engine.preferences().set_setup_complete(&p.skill_id, true);
+        engine.start_skill(&p.skill_id).await?;
         Ok(serde_json::json!({ "ok": true }))
     })
 }
 
-/// RPC handler to disable a skill in user preferences and stop it.
+/// RPC handler to disable a skill: clears setup_complete and stops it.
 fn handle_skills_disable(params: Map<String, Value>) -> ControllerFuture {
     Box::pin(async move {
         let p: SkillIdParams =
             serde_json::from_value(Value::Object(params)).map_err(|e| e.to_string())?;
         let engine = require_engine()?;
-        engine.disable_skill(&p.skill_id).await?;
+        engine.preferences().set_setup_complete(&p.skill_id, false);
+        if engine.get_skill_state(&p.skill_id).is_some() {
+            engine.stop_skill(&p.skill_id).await?;
+        }
         Ok(serde_json::json!({ "ok": true }))
     })
 }
 
-/// RPC handler to check if a skill is currently enabled in user preferences.
+/// RPC handler to check if a skill should currently be running.
+///
+/// Reports `setup_complete` so the frontend's existing "enabled" UI keeps
+/// working without learning a new term.
 fn handle_skills_is_enabled(params: Map<String, Value>) -> ControllerFuture {
     Box::pin(async move {
         let p: SkillIdParams =
             serde_json::from_value(Value::Object(params)).map_err(|e| e.to_string())?;
         let engine = require_engine()?;
-        let enabled = engine.is_skill_enabled(&p.skill_id);
+        let enabled = engine.preferences().is_setup_complete(&p.skill_id);
         Ok(serde_json::json!({ "enabled": enabled }))
     })
 }