From a39a675ba9410656e7aa7965fb2a58a65833eaf8 Mon Sep 17 00:00:00 2001 From: jaberjaber23 Date: Sun, 19 Apr 2026 22:27:23 +0300 Subject: [PATCH] skill config ui --- crates/openfang-api/src/middleware.rs | 1 + crates/openfang-api/src/rate_limiter.rs | 9 + crates/openfang-api/src/routes.rs | 2 +- crates/openfang-api/static/js/pages/skills.js | 168 +++++++- .../tests/skill_config_api_test.rs | 390 ++++++++++++++++++ crates/openfang-cli/src/tui/event.rs | 96 +++++ crates/openfang-cli/src/tui/mod.rs | 9 + crates/openfang-cli/src/tui/screens/skills.rs | 176 +++++++- crates/openfang-kernel/src/kernel.rs | 41 +- .../openfang-skills/src/config_injection.rs | 2 +- 10 files changed, 874 insertions(+), 20 deletions(-) create mode 100644 crates/openfang-api/tests/skill_config_api_test.rs diff --git a/crates/openfang-api/src/middleware.rs b/crates/openfang-api/src/middleware.rs index bc2e313d..702a66be 100644 --- a/crates/openfang-api/src/middleware.rs +++ b/crates/openfang-api/src/middleware.rs @@ -126,6 +126,7 @@ pub async fn auth( || (path == "/api/hands/active" && is_get) || (path.starts_with("/api/hands/") && is_get) || (path == "/api/skills" && is_get) + || (path.starts_with("/api/skills/") && path.ends_with("/config") && is_get) || (path == "/api/sessions" && is_get) || (path == "/api/integrations" && is_get) || (path == "/api/integrations/available" && is_get) diff --git a/crates/openfang-api/src/rate_limiter.rs b/crates/openfang-api/src/rate_limiter.rs index e2c1980f..2edad894 100644 --- a/crates/openfang-api/src/rate_limiter.rs +++ b/crates/openfang-api/src/rate_limiter.rs @@ -30,6 +30,15 @@ pub fn operation_cost(method: &str, path: &str) -> NonZeroU32 { ("POST", "/api/skills/install") => NonZeroU32::new(50).unwrap(), ("POST", "/api/skills/uninstall") => NonZeroU32::new(10).unwrap(), ("POST", "/api/skills/reload") => NonZeroU32::new(5).unwrap(), + ("GET", p) if p.starts_with("/api/skills/") && p.ends_with("/config") => { + NonZeroU32::new(3).unwrap() + } + ("PUT", p) if p.starts_with("/api/skills/") && p.ends_with("/config") => { + NonZeroU32::new(10).unwrap() + } + ("DELETE", p) if p.starts_with("/api/skills/") && p.contains("/config/") => { + NonZeroU32::new(10).unwrap() + } ("POST", "/api/migrate") => NonZeroU32::new(100).unwrap(), ("PUT", p) if p.contains("/update") => NonZeroU32::new(10).unwrap(), _ => NonZeroU32::new(5).unwrap(), diff --git a/crates/openfang-api/src/routes.rs b/crates/openfang-api/src/routes.rs index c46bb7a3..14a2debf 100644 --- a/crates/openfang-api/src/routes.rs +++ b/crates/openfang-api/src/routes.rs @@ -9316,7 +9316,7 @@ pub async fn schedule_delivery_log( let targets: Vec = job .delivery_targets .iter() - .map(|t| serde_json::to_value(t).unwrap_or_else(|_| serde_json::Value::Null)) + .map(|t| serde_json::to_value(t).unwrap_or(serde_json::Value::Null)) .collect(); ( StatusCode::OK, diff --git a/crates/openfang-api/static/js/pages/skills.js b/crates/openfang-api/static/js/pages/skills.js index 22c7478b..8a4ddf79 100644 --- a/crates/openfang-api/static/js/pages/skills.js +++ b/crates/openfang-api/static/js/pages/skills.js @@ -30,6 +30,16 @@ function skillsPage() { skillCodeFilename: '', skillCodeLoading: false, + // Skill config modal (local skill configuration from SKILL.md frontmatter) + configSkill: null, // skill object whose config is being edited + configDeclared: {}, // { var_name: { description, env, default, required } } + configResolved: {}, // { var_name: { value, source, is_secret } } + configDraft: {}, // { var_name: user-edited string value } + configRevealed: {}, // { var_name: bool } — toggle password reveal per row + configLoading: false, + configSaving: false, + configError: '', + // MCP servers mcpServers: [], mcpLoading: false, @@ -101,7 +111,8 @@ function skillsPage() { tags: s.tags || [], enabled: s.enabled !== false, source: s.source || { type: 'local' }, - has_prompt_context: !!s.has_prompt_context + has_prompt_context: !!s.has_prompt_context, + config_declared_count: s.config_declared_count || 0 }; }); } catch(e) { @@ -111,6 +122,161 @@ function skillsPage() { this.loading = false; }, + // ── Skill config editing ──────────────────────────────────────────── + async openSkillConfig(skill) { + this.configSkill = skill; + this.configDeclared = {}; + this.configResolved = {}; + this.configDraft = {}; + this.configRevealed = {}; + this.configError = ''; + this.configLoading = true; + try { + var data = await OpenFangAPI.get('/api/skills/' + encodeURIComponent(skill.name) + '/config'); + this.configDeclared = data.declared || {}; + this.configResolved = data.resolved || {}; + // Pre-populate draft values only for vars the user has already + // overridden — never copy redacted responses back into inputs or + // they'd be re-saved as "****redacted****" strings. + var names = Object.keys(this.configDeclared); + for (var i = 0; i < names.length; i++) { + var n = names[i]; + var res = this.configResolved[n] || {}; + if (res.source === 'user' && !res.is_secret) { + this.configDraft[n] = res.value == null ? '' : String(res.value); + } else { + this.configDraft[n] = ''; + } + } + } catch(e) { + this.configError = e.message || 'Failed to load skill config.'; + } + this.configLoading = false; + }, + + closeSkillConfig() { + this.configSkill = null; + this.configDeclared = {}; + this.configResolved = {}; + this.configDraft = {}; + this.configRevealed = {}; + this.configError = ''; + }, + + configRowInvalid(name) { + // A required var is invalid iff the user hasn't entered anything AND + // no env/default resolves it. Source from the server tells us where + // the current value came from; if it's "unresolved" and the draft is + // blank, the save would write an empty string over the required var. + var decl = this.configDeclared[name] || {}; + if (!decl.required) return false; + var draft = (this.configDraft[name] || '').trim(); + if (draft) return false; + var res = this.configResolved[name] || {}; + if (res.source === 'env' || res.source === 'default') return false; + // If the user has an existing secret override we don't want to force + // them to re-type it — treat that as "currently resolved". + if (res.source === 'user') return false; + return true; + }, + + hasInvalidConfig() { + var names = Object.keys(this.configDeclared); + for (var i = 0; i < names.length; i++) { + if (this.configRowInvalid(names[i])) return true; + } + return false; + }, + + toggleReveal(name) { + this.configRevealed[name] = !this.configRevealed[name]; + }, + + sourceBadgeClass(source) { + switch (source) { + case 'user': return 'badge-success'; + case 'env': return 'badge-info'; + case 'default': return 'badge-dim'; + default: return 'badge-danger'; + } + }, + + sourceBadgeLabel(res) { + if (!res) return 'unresolved'; + switch (res.source) { + case 'user': return 'user override'; + case 'env': return 'env' + ((this.configDeclared[res.__name] && this.configDeclared[res.__name].env) ? ':' + this.configDeclared[res.__name].env : ''); + case 'default': return 'default'; + default: return 'unresolved'; + } + }, + + async saveSkillConfig() { + if (!this.configSkill) return; + if (this.hasInvalidConfig()) { + OpenFangToast.error('Fill in all required variables before saving.'); + return; + } + this.configSaving = true; + this.configError = ''; + // Only PUT values the user actually typed. Empty strings are dropped + // so we don't silently clobber an env/default with "". + var payload = {}; + var names = Object.keys(this.configDeclared); + for (var i = 0; i < names.length; i++) { + var n = names[i]; + var v = (this.configDraft[n] || '').trim(); + if (v.length > 0) payload[n] = v; + } + try { + await OpenFangAPI.put('/api/skills/' + encodeURIComponent(this.configSkill.name) + '/config', { values: payload }); + OpenFangToast.success('Saved, reloading agents\u2026'); + // Refresh the modal contents so the new source/value shows up. + var refreshed = this.configSkill; + await this.loadSkills(); + this.closeSkillConfig(); + // Find the possibly-refreshed skill object and reopen. + var self = this; + var updated = this.skills.find(function(s) { return s.name === refreshed.name; }); + if (updated) await self.openSkillConfig(updated); + } catch(e) { + this.configError = e.message || 'Save failed.'; + OpenFangToast.error('Save failed: ' + (e.message || 'unknown error')); + } + this.configSaving = false; + }, + + async resetSkillConfigVar(name) { + if (!this.configSkill) return; + var decl = this.configDeclared[name] || {}; + var res = this.configResolved[name] || {}; + // If the server is already reporting a non-user source there's nothing + // to remove; just clear the draft so the input disappears. + if (res.source !== 'user') { + this.configDraft[name] = ''; + return; + } + try { + await OpenFangAPI.del('/api/skills/' + encodeURIComponent(this.configSkill.name) + '/config/' + encodeURIComponent(name)); + OpenFangToast.success('Reset ' + name); + // Refresh modal state from server. + var data = await OpenFangAPI.get('/api/skills/' + encodeURIComponent(this.configSkill.name) + '/config'); + this.configResolved = data.resolved || {}; + this.configDraft[name] = ''; + } catch(e) { + var msg = e.message || 'Reset failed'; + if (msg.indexOf('required') !== -1 || msg.indexOf('409') !== -1) { + OpenFangToast.error('Cannot reset: ' + decl.description + ' is required with no fallback.'); + } else { + OpenFangToast.error('Reset failed: ' + msg); + } + } + }, + + get configDeclaredNames() { + return Object.keys(this.configDeclared).sort(); + }, + async loadData() { await this.loadSkills(); }, diff --git a/crates/openfang-api/tests/skill_config_api_test.rs b/crates/openfang-api/tests/skill_config_api_test.rs new file mode 100644 index 00000000..f44bd186 --- /dev/null +++ b/crates/openfang-api/tests/skill_config_api_test.rs @@ -0,0 +1,390 @@ +//! Integration tests for the `/api/skills/{id}/config` surface. +//! +//! These boot a real kernel, start a real axum server on a random port, plant +//! a synthetic skill on disk whose SKILL.md declares a `config:` section, and +//! exercise GET / PUT / DELETE end to end. Bundled skills currently declare +//! no runtime config, so the synthetic skill fixture is what lets us prove +//! the wire contract. +//! +//! Run: cargo test -p openfang-api --test skill_config_api_test -- --nocapture + +use axum::Router; +use openfang_api::middleware; +use openfang_api::routes::{self, AppState}; +use openfang_kernel::OpenFangKernel; +use openfang_types::config::{DefaultModelConfig, KernelConfig}; +use std::sync::Arc; +use std::time::Instant; +use tower_http::cors::CorsLayer; + +// --------------------------------------------------------------------------- +// Test server harness +// --------------------------------------------------------------------------- + +struct TestServer { + base_url: String, + home_dir: std::path::PathBuf, + #[allow(dead_code)] + state: Arc, + _tmp: tempfile::TempDir, +} + +impl Drop for TestServer { + fn drop(&mut self) { + self.state.kernel.shutdown(); + } +} + +/// Write a skill fixture under `/skills//SKILL.md` that declares +/// a `config:` section. This matches the on-disk format that OpenClaw skills +/// use, so the loader's real `parse_skillmd_str` path is exercised. +fn plant_skill_with_config(home: &std::path::Path, skill_name: &str) { + let skill_dir = home.join("skills").join(skill_name); + std::fs::create_dir_all(&skill_dir).unwrap(); + // Leading four spaces inside YAML lists matter — keep them. + let skillmd = format!( + "--- +name: {skill_name} +description: Synthetic skill for config endpoint tests +config: + github_token: + description: GitHub personal access token + env: OPENFANG_TEST_SKILLCFG_GH_TOKEN + required: true + default_branch: + description: Default branch name + default: main + required: false +--- +# Test Skill + +Placeholder body so the parser accepts this as a valid prompt-only skill. +" + ); + std::fs::write(skill_dir.join("SKILL.md"), skillmd).unwrap(); +} + +async fn start_test_server() -> TestServer { + let tmp = tempfile::tempdir().expect("tempdir"); + let home = tmp.path().to_path_buf(); + + let config = KernelConfig { + home_dir: home.clone(), + data_dir: home.join("data"), + default_model: DefaultModelConfig { + provider: "ollama".to_string(), + model: "test-model".to_string(), + api_key_env: "OLLAMA_API_KEY".to_string(), + base_url: None, + }, + ..KernelConfig::default() + }; + + // Plant synthetic skill BEFORE booting so the initial skill load picks it up. + plant_skill_with_config(&home, "test-config-skill"); + + let kernel = OpenFangKernel::boot_with_config(config).expect("kernel boot"); + let kernel = Arc::new(kernel); + kernel.set_self_handle(); + + let state = Arc::new(AppState { + kernel, + started_at: Instant::now(), + peer_registry: None, + bridge_manager: tokio::sync::Mutex::new(None), + channels_config: tokio::sync::RwLock::new(Default::default()), + shutdown_notify: Arc::new(tokio::sync::Notify::new()), + clawhub_cache: dashmap::DashMap::new(), + provider_probe_cache: openfang_runtime::provider_health::ProbeCache::new(), + budget_config: Arc::new(tokio::sync::RwLock::new(Default::default())), + }); + + let app = Router::new() + .route("/api/skills", axum::routing::get(routes::list_skills)) + .route( + "/api/skills/{id}/config", + axum::routing::get(routes::get_skill_config).put(routes::put_skill_config), + ) + .route( + "/api/skills/{id}/config/{var_name}", + axum::routing::delete(routes::delete_skill_config_var), + ) + .layer(axum::middleware::from_fn(middleware::request_logging)) + .layer(CorsLayer::permissive()) + .with_state(state.clone()); + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind test port"); + let addr = listener.local_addr().unwrap(); + + tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + + TestServer { + base_url: format!("http://{}", addr), + home_dir: home, + state, + _tmp: tmp, + } +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn get_config_returns_declared_and_resolved() { + // Make sure no host leak from prior tests interferes. + // SAFETY: single-threaded test, env var is unique to this test suite. + unsafe { std::env::remove_var("OPENFANG_TEST_SKILLCFG_GH_TOKEN") }; + + let server = start_test_server().await; + let client = reqwest::Client::new(); + + let resp = client + .get(format!( + "{}/api/skills/test-config-skill/config", + server.base_url + )) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200); + let body: serde_json::Value = resp.json().await.unwrap(); + + assert_eq!(body["skill"], "test-config-skill"); + // Both vars declared + assert!(body["declared"]["github_token"].is_object()); + assert!(body["declared"]["default_branch"].is_object()); + assert_eq!(body["declared"]["github_token"]["required"], true); + assert_eq!(body["declared"]["default_branch"]["required"], false); + + // github_token has no user override, no env, no default -> unresolved. + assert_eq!( + body["resolved"]["github_token"]["source"], "unresolved", + "github_token should be unresolved without env" + ); + assert!(body["resolved"]["github_token"]["is_secret"].as_bool().unwrap()); + + // default_branch falls back to default "main". + assert_eq!(body["resolved"]["default_branch"]["source"], "default"); + assert_eq!(body["resolved"]["default_branch"]["value"], "main"); +} + +#[tokio::test] +async fn get_config_redacts_secret_values_after_put() { + let server = start_test_server().await; + let client = reqwest::Client::new(); + + // Write a real-looking token via PUT. + let payload = serde_json::json!({ + "values": { + "github_token": "ghp_realsecretvalue_DO_NOT_LEAK", + "default_branch": "develop" + } + }); + let resp = client + .put(format!( + "{}/api/skills/test-config-skill/config", + server.base_url + )) + .json(&payload) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200, "PUT should succeed"); + + // GET back and confirm the secret is redacted and non-secret is visible. + let resp = client + .get(format!( + "{}/api/skills/test-config-skill/config", + server.base_url + )) + .send() + .await + .unwrap(); + let body: serde_json::Value = resp.json().await.unwrap(); + + let returned_token = body["resolved"]["github_token"]["value"] + .as_str() + .unwrap() + .to_string(); + assert!( + !returned_token.contains("realsecretvalue"), + "secret leaked on the wire: {returned_token}" + ); + assert!( + returned_token.contains("redacted"), + "expected redaction marker, got: {returned_token}" + ); + assert_eq!(body["resolved"]["github_token"]["source"], "user"); + + // Non-secret var kept as-is. + assert_eq!(body["resolved"]["default_branch"]["value"], "develop"); + assert_eq!(body["resolved"]["default_branch"]["source"], "user"); + + // config.toml persisted the change, including the full secret value + // (redaction is only on the wire — disk is the source of truth). + let cfg = std::fs::read_to_string(server.home_dir.join("config.toml")).unwrap(); + assert!( + cfg.contains("[skills.test-config-skill]"), + "skills section missing: {cfg}" + ); + assert!(cfg.contains("realsecretvalue")); + assert!(cfg.contains("develop")); +} + +#[tokio::test] +async fn put_rejects_unknown_variable() { + let server = start_test_server().await; + let client = reqwest::Client::new(); + + let payload = serde_json::json!({ + "values": { "nonexistent_var": "value" } + }); + let resp = client + .put(format!( + "{}/api/skills/test-config-skill/config", + server.base_url + )) + .json(&payload) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 400); + let body: serde_json::Value = resp.json().await.unwrap(); + assert!(body["error"] + .as_str() + .unwrap() + .contains("nonexistent_var")); +} + +#[tokio::test] +async fn delete_override_reverts_to_default() { + let server = start_test_server().await; + let client = reqwest::Client::new(); + + // Set override first. + client + .put(format!( + "{}/api/skills/test-config-skill/config", + server.base_url + )) + .json(&serde_json::json!({ + "values": { "default_branch": "develop" } + })) + .send() + .await + .unwrap(); + + // Remove it. + let resp = client + .delete(format!( + "{}/api/skills/test-config-skill/config/default_branch", + server.base_url + )) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200); + + // Now source should be "default" again with value "main". + let body: serde_json::Value = client + .get(format!( + "{}/api/skills/test-config-skill/config", + server.base_url + )) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + assert_eq!(body["resolved"]["default_branch"]["source"], "default"); + assert_eq!(body["resolved"]["default_branch"]["value"], "main"); +} + +#[tokio::test] +async fn delete_refuses_to_strand_required_var() { + // github_token is required, has no default, and no env — so removing an + // override would leave it unresolvable. The endpoint must refuse. + // SAFETY: single-threaded test. + unsafe { std::env::remove_var("OPENFANG_TEST_SKILLCFG_GH_TOKEN") }; + + let server = start_test_server().await; + let client = reqwest::Client::new(); + + // Set an override. + client + .put(format!( + "{}/api/skills/test-config-skill/config", + server.base_url + )) + .json(&serde_json::json!({ + "values": { "github_token": "ghp_value" } + })) + .send() + .await + .unwrap(); + + // Try to delete — should 409. + let resp = client + .delete(format!( + "{}/api/skills/test-config-skill/config/github_token", + server.base_url + )) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 409); +} + +#[tokio::test] +async fn get_unknown_skill_returns_404() { + let server = start_test_server().await; + let client = reqwest::Client::new(); + let resp = client + .get(format!( + "{}/api/skills/this-skill-does-not-exist/config", + server.base_url + )) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 404); +} + +#[tokio::test] +async fn put_reloads_registry_so_agents_see_change() { + let server = start_test_server().await; + let client = reqwest::Client::new(); + + client + .put(format!( + "{}/api/skills/test-config-skill/config", + server.base_url + )) + .json(&serde_json::json!({ + "values": { + "github_token": "ghp_new", + "default_branch": "release" + } + })) + .send() + .await + .unwrap(); + + // The kernel's live override map must now hold the new values. + let guard = server + .state + .kernel + .skill_config_overrides + .read() + .unwrap(); + let overrides = guard.as_ref().expect("override map set after PUT"); + let skill_cfg = overrides.get("test-config-skill").expect("skill present"); + assert_eq!(skill_cfg.get("github_token").unwrap(), "ghp_new"); + assert_eq!(skill_cfg.get("default_branch").unwrap(), "release"); +} diff --git a/crates/openfang-cli/src/tui/event.rs b/crates/openfang-cli/src/tui/event.rs index ae62d1f6..c0befe06 100644 --- a/crates/openfang-cli/src/tui/event.rs +++ b/crates/openfang-cli/src/tui/event.rs @@ -121,6 +121,11 @@ pub enum AppEvent { SkillUninstalled(String), /// MCP servers loaded. McpServersLoaded(Vec), + /// Skill config details loaded (installed skill `c` key). + SkillConfigLoaded { + skill: String, + rows: Vec, + }, /// Templates providers loaded (auth status). TemplateProvidersLoaded(Vec), /// Security features loaded. @@ -1439,6 +1444,14 @@ pub fn spawn_fetch_skills(backend: BackendRef, tx: mpsc::Sender) { .as_str() .unwrap_or("") .to_string(), + config_declared: s["config_declared_count"] + .as_u64() + .unwrap_or(0) + as usize, + config_resolved: s["config_resolved_count"] + .as_u64() + .unwrap_or(0) + as usize, }) .collect() }) @@ -1604,6 +1617,89 @@ pub fn spawn_fetch_mcp_servers(backend: BackendRef, tx: mpsc::Sender) }); } +/// Fetch declared + resolved config for a specific installed skill. +/// +/// Pulls `GET /api/skills/{id}/config` and flattens the response into the +/// `SkillConfigVarDetail` rows that the TUI details pane renders. Secret +/// values are already redacted by the daemon so nothing sensitive crosses +/// the wire here. +pub fn spawn_fetch_skill_config( + backend: BackendRef, + skill_name: String, + tx: mpsc::Sender, +) { + use crate::tui::screens::skills::SkillConfigVarDetail; + std::thread::spawn(move || match backend { + BackendRef::Daemon(base_url) => { + let client = daemon_client(); + let encoded: String = skill_name + .chars() + .map(|c| { + if c.is_alphanumeric() || c == '-' || c == '_' || c == '.' || c == '~' { + c.to_string() + } else { + format!("%{:02X}", c as u32) + } + }) + .collect(); + let url = format!("{base_url}/api/skills/{encoded}/config"); + if let Ok(resp) = client.get(&url).send() { + if let Ok(body) = resp.json::() { + let declared = body.get("declared").cloned().unwrap_or_default(); + let resolved = body.get("resolved").cloned().unwrap_or_default(); + let mut rows: Vec = Vec::new(); + if let Some(obj) = declared.as_object() { + // Sort keys for deterministic output. + let mut keys: Vec<&String> = obj.keys().collect(); + keys.sort(); + for k in keys { + let d = &obj[k]; + let r = resolved.get(k).cloned().unwrap_or_default(); + let value_hint = r + .get("value") + .and_then(|v| v.as_str()) + .map(String::from) + .unwrap_or_default(); + rows.push(SkillConfigVarDetail { + name: k.clone(), + description: d + .get("description") + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string(), + required: d + .get("required") + .and_then(|v| v.as_bool()) + .unwrap_or(false), + source: r + .get("source") + .and_then(|v| v.as_str()) + .unwrap_or("unresolved") + .to_string(), + value_hint, + is_secret: r + .get("is_secret") + .and_then(|v| v.as_bool()) + .unwrap_or(false), + }); + } + } + let _ = tx.send(AppEvent::SkillConfigLoaded { + skill: skill_name, + rows, + }); + } + } + } + BackendRef::InProcess(_) => { + let _ = tx.send(AppEvent::SkillConfigLoaded { + skill: skill_name, + rows: Vec::new(), + }); + } + }); +} + /// Fetch provider auth status for templates screen. pub fn spawn_fetch_template_providers(backend: BackendRef, tx: mpsc::Sender) { std::thread::spawn(move || match backend { diff --git a/crates/openfang-cli/src/tui/mod.rs b/crates/openfang-cli/src/tui/mod.rs index 27b7fb19..12ae3a96 100644 --- a/crates/openfang-cli/src/tui/mod.rs +++ b/crates/openfang-cli/src/tui/mod.rs @@ -435,6 +435,10 @@ impl App { } self.skills.loading = false; } + AppEvent::SkillConfigLoaded { skill, rows } => { + self.skills.selected_config_details = Some((skill.clone(), rows)); + self.skills.status_msg = format!("Loaded config for '{skill}'"); + } AppEvent::TemplateProvidersLoaded(providers) => { self.templates.providers = providers; } @@ -1563,6 +1567,11 @@ impl App { event::spawn_fetch_mcp_servers(backend, self.event_tx.clone()); } } + skills::SkillsAction::LoadSkillConfig(name) => { + if let Some(backend) = self.backend.to_ref() { + event::spawn_fetch_skill_config(backend, name, self.event_tx.clone()); + } + } } } diff --git a/crates/openfang-cli/src/tui/screens/skills.rs b/crates/openfang-cli/src/tui/screens/skills.rs index 5494ebaf..8832ed5f 100644 --- a/crates/openfang-cli/src/tui/screens/skills.rs +++ b/crates/openfang-cli/src/tui/screens/skills.rs @@ -16,6 +16,23 @@ pub struct SkillInfo { pub runtime: String, pub source: String, pub description: String, + /// Number of config vars the skill's SKILL.md declares. Zero = no badge. + pub config_declared: usize, + /// Number of declared vars that are currently resolved (user override, + /// env, or default). When `< config_declared` the badge shows a warning. + pub config_resolved: usize, +} + +/// Declared + resolved config for a single variable, shown on the Skills +/// detail pane when a skill with a non-empty `config:` section is selected. +#[derive(Clone, Default)] +pub struct SkillConfigVarDetail { + pub name: String, + pub description: String, + pub required: bool, + pub source: String, // "user" | "env" | "default" | "unresolved" + pub value_hint: String, + pub is_secret: bool, } #[derive(Clone, Default)] @@ -82,6 +99,9 @@ pub struct SkillsState { pub tick: usize, pub confirm_uninstall: bool, pub status_msg: String, + /// Config variable details for the currently selected installed skill. + /// Keyed by skill name so refreshing the list doesn't strand stale data. + pub selected_config_details: Option<(String, Vec)>, } pub enum SkillsAction { @@ -92,6 +112,10 @@ pub enum SkillsAction { InstallSkill(String), UninstallSkill(String), RefreshMcp, + /// Fetch declared + resolved config for the named skill and display it + /// in the details pane. Consumed by the caller which drives the API + /// request. + LoadSkillConfig(String), } impl SkillsState { @@ -111,6 +135,7 @@ impl SkillsState { tick: 0, confirm_uninstall: false, status_msg: String::new(), + selected_config_details: None, } } @@ -186,6 +211,20 @@ impl SkillsState { self.confirm_uninstall = true; } } + KeyCode::Char('c') => { + if let Some(sel) = self.installed_list.selected() { + if sel < self.installed.len() { + let name = self.installed[sel].name.clone(); + // Only skills that declare config are worth fetching. + if self.installed[sel].config_declared > 0 { + return SkillsAction::LoadSkillConfig(name); + } else { + self.status_msg = + format!("'{}' declares no runtime config.", name); + } + } + } + } KeyCode::Char('r') => return SkillsAction::RefreshInstalled, _ => {} } @@ -336,9 +375,11 @@ fn draw_sub_tabs(f: &mut Frame, area: Rect, active: SkillsSub) { } fn draw_installed(f: &mut Frame, area: Rect, state: &mut SkillsState) { - let chunks = Layout::vertical([ + // Two panes: list on the left, config details on the right, with a + // single-line hint bar underneath. + let outer = Layout::vertical([ Constraint::Length(1), // header - Constraint::Min(3), // list + Constraint::Min(3), // list + details Constraint::Length(1), // hints ]) .split(area); @@ -346,14 +387,26 @@ fn draw_installed(f: &mut Frame, area: Rect, state: &mut SkillsState) { f.render_widget( Paragraph::new(Line::from(vec![Span::styled( format!( - " {:<20} {:<8} {:<12} {}", - "Name", "Runtime", "Source", "Description" + " {:<18} {:<7} {:<10} {:<11} {}", + "Name", "Runtime", "Source", "Config", "Description" ), theme::table_header(), )])), - chunks[0], + outer[0], ); + // Only show the details pane when there's room and a selection with + // declared config exists. Below ~80 cols we collapse to list-only. + let has_details = state.selected_config_details.is_some(); + let body_chunks: Vec = if has_details && outer[1].width >= 70 { + Layout::horizontal([Constraint::Percentage(60), Constraint::Percentage(40)]) + .split(outer[1]) + .to_vec() + } else { + vec![outer[1]] + }; + + // Skills list (left pane or full width) if state.loading { let spinner = theme::SPINNER_FRAMES[state.tick % theme::SPINNER_FRAMES.len()]; f.render_widget( @@ -361,7 +414,7 @@ fn draw_installed(f: &mut Frame, area: Rect, state: &mut SkillsState) { Span::styled(format!(" {spinner} "), Style::default().fg(theme::CYAN)), Span::styled("Loading skills\u{2026}", theme::dim_style()), ])), - chunks[1], + body_chunks[0], ); } else if state.installed.is_empty() { f.render_widget( @@ -369,7 +422,7 @@ fn draw_installed(f: &mut Frame, area: Rect, state: &mut SkillsState) { " No skills installed. Press [2] to browse ClawHub.", theme::dim_style(), )), - chunks[1], + body_chunks[0], ); } else { let items: Vec = state @@ -394,15 +447,29 @@ fn draw_installed(f: &mut Frame, area: Rect, state: &mut SkillsState) { "builtin" | "built-in" => Style::default().fg(theme::GREEN), _ => theme::dim_style(), }; + let (config_text, config_style) = if s.config_declared == 0 { + (String::from("-"), theme::dim_style()) + } else if s.config_resolved >= s.config_declared { + ( + format!("[{}/{}]", s.config_resolved, s.config_declared), + Style::default().fg(theme::GREEN), + ) + } else { + ( + format!("[{}/{} \u{26A0}]", s.config_resolved, s.config_declared), + Style::default().fg(theme::YELLOW), + ) + }; ListItem::new(Line::from(vec![ Span::styled( - format!(" {:<20}", truncate(&s.name, 19)), + format!(" {:<18}", truncate(&s.name, 17)), Style::default().fg(theme::CYAN), ), - Span::styled(format!(" {:<8}", runtime_badge), runtime_style), - Span::styled(format!(" {:<12}", &s.source), source_style), + Span::styled(format!(" {:<7}", runtime_badge), runtime_style), + Span::styled(format!(" {:<10}", &s.source), source_style), + Span::styled(format!(" {:<11}", config_text), config_style), Span::styled( - format!(" {}", truncate(&s.description, 30)), + format!(" {}", truncate(&s.description, 24)), theme::dim_style(), ), ])) @@ -412,7 +479,12 @@ fn draw_installed(f: &mut Frame, area: Rect, state: &mut SkillsState) { let list = List::new(items) .highlight_style(theme::selected_style()) .highlight_symbol("> "); - f.render_stateful_widget(list, chunks[1], &mut state.installed_list); + f.render_stateful_widget(list, body_chunks[0], &mut state.installed_list); + } + + // Config details pane (right side) + if has_details && body_chunks.len() > 1 { + draw_skill_config_details(f, body_chunks[1], state); } if state.confirm_uninstall { @@ -421,7 +493,7 @@ fn draw_installed(f: &mut Frame, area: Rect, state: &mut SkillsState) { " Uninstall this skill? [y] Yes [any] Cancel", Style::default().fg(theme::YELLOW), )])), - chunks[2], + outer[2], ); } else if !state.status_msg.is_empty() { f.render_widget( @@ -429,19 +501,91 @@ fn draw_installed(f: &mut Frame, area: Rect, state: &mut SkillsState) { format!(" {}", state.status_msg), Style::default().fg(theme::GREEN), )])), - chunks[2], + outer[2], ); } else { f.render_widget( Paragraph::new(Line::from(vec![Span::styled( - " [\u{2191}\u{2193}] Navigate [u] Uninstall [r] Refresh", + " [\u{2191}\u{2193}] Navigate [c] View config [u] Uninstall [r] Refresh", theme::hint_style(), )])), - chunks[2], + outer[2], ); } } +fn draw_skill_config_details(f: &mut Frame, area: Rect, state: &SkillsState) { + let Some((name, rows)) = state.selected_config_details.as_ref() else { + return; + }; + let block = Block::default() + .title(Line::from(vec![Span::styled( + format!(" Config: {name} "), + Style::default().fg(theme::ACCENT), + )])) + .borders(Borders::LEFT) + .border_style(theme::dim_style()) + .padding(Padding::horizontal(1)); + let inner = block.inner(area); + f.render_widget(block, area); + + if rows.is_empty() { + f.render_widget( + Paragraph::new(Span::styled( + "No config declared.", + theme::dim_style(), + )), + inner, + ); + return; + } + + let mut lines: Vec = Vec::new(); + for row in rows { + let src_style = match row.source.as_str() { + "user" => Style::default().fg(theme::GREEN), + "env" => Style::default().fg(theme::CYAN), + "default" => theme::dim_style(), + _ => Style::default().fg(theme::RED), + }; + let required_marker = if row.required { "*" } else { " " }; + let mut header = vec![ + Span::styled( + format!("{required_marker} {}", truncate(&row.name, 22)), + Style::default() + .fg(theme::CYAN) + .add_modifier(Modifier::BOLD), + ), + Span::raw(" "), + Span::styled(format!("[{}]", row.source), src_style), + ]; + if row.is_secret { + header.push(Span::raw(" ")); + header.push(Span::styled( + "(secret)", + Style::default() + .fg(theme::YELLOW) + .add_modifier(Modifier::ITALIC), + )); + } + lines.push(Line::from(header)); + if !row.value_hint.is_empty() { + lines.push(Line::from(vec![Span::styled( + format!(" = {}", truncate(&row.value_hint, 32)), + theme::dim_style(), + )])); + } + if !row.description.is_empty() { + lines.push(Line::from(vec![Span::styled( + format!(" {}", truncate(&row.description, 36)), + theme::dim_style(), + )])); + } + lines.push(Line::from(vec![Span::raw("")])); + } + f.render_widget(Paragraph::new(lines), inner); +} + fn draw_clawhub(f: &mut Frame, area: Rect, state: &mut SkillsState) { let chunks = Layout::vertical([ Constraint::Length(1), // search / sort diff --git a/crates/openfang-kernel/src/kernel.rs b/crates/openfang-kernel/src/kernel.rs index 3fd9cd85..7a39e7ba 100644 --- a/crates/openfang-kernel/src/kernel.rs +++ b/crates/openfang-kernel/src/kernel.rs @@ -92,6 +92,15 @@ pub struct OpenFangKernel { pub model_catalog: std::sync::RwLock, /// Skill registry for plugin skills (RwLock for hot-reload on install/uninstall). pub skill_registry: std::sync::RwLock, + /// Per-skill config overrides applied on top of `self.config.skills`. + /// + /// Written by the API (`PUT /api/skills/{id}/config`) so the user's edits + /// take effect on the next `reload_skills()` without having to mutate the + /// immutable boot-time `KernelConfig`. `None` means "fall back to + /// `self.config.skills`"; `Some(map)` means "this is the live override". + pub skill_config_overrides: std::sync::RwLock< + Option>>, + >, /// Tracks running agent tasks for cancellation support. pub running_tasks: dashmap::DashMap, /// MCP server connections (lazily initialized at start_background_agents). @@ -1129,6 +1138,7 @@ impl OpenFangKernel { auth, model_catalog: std::sync::RwLock::new(model_catalog), skill_registry: std::sync::RwLock::new(skill_registry), + skill_config_overrides: std::sync::RwLock::new(None), running_tasks: dashmap::DashMap::new(), mcp_connections: tokio::sync::Mutex::new(Vec::new()), mcp_tools: std::sync::Mutex::new(Vec::new()), @@ -5687,13 +5697,42 @@ impl OpenFangKernel { } let skills_dir = self.config.home_dir.join("skills"); let mut fresh = openfang_skills::registry::SkillRegistry::new(skills_dir); - fresh.set_skill_configs(self.config.skills.clone()); + // Prefer the live override (from `PUT /api/skills/{id}/config`) so + // dashboard edits survive hot-reloads without restarting the kernel. + // Fall back to the boot-time config. + let configs = self + .skill_config_overrides + .read() + .unwrap_or_else(|e| e.into_inner()) + .clone() + .unwrap_or_else(|| self.config.skills.clone()); + fresh.set_skill_configs(configs); let bundled = fresh.load_bundled(); let user = fresh.load_all().unwrap_or(0); info!(bundled, user, "Skill registry hot-reloaded"); *registry = fresh; } + /// Update the live per-skill config override map and reload skills. + /// + /// Used by `PUT /api/skills/{id}/config` / `DELETE + /// /api/skills/{id}/config/{var}`. The caller is also expected to have + /// persisted the same change to `config.toml` so the override survives a + /// full restart; this method only refreshes the in-memory skill registry. + pub fn reload_skills_with_configs( + &self, + configs: std::collections::HashMap>, + ) { + { + let mut guard = self + .skill_config_overrides + .write() + .unwrap_or_else(|e| e.into_inner()); + *guard = Some(configs); + } + self.reload_skills(); + } + /// Build a compact skill summary for the system prompt so the agent knows /// what extra capabilities are installed. /// diff --git a/crates/openfang-skills/src/config_injection.rs b/crates/openfang-skills/src/config_injection.rs index 277c7f13..83888bc7 100644 --- a/crates/openfang-skills/src/config_injection.rs +++ b/crates/openfang-skills/src/config_injection.rs @@ -257,7 +257,7 @@ mod tests { let user = HashMap::new(); let resolved = resolve_skill_config(&vars, &user).unwrap(); - assert!(resolved.get("optional").is_none()); + assert!(!resolved.contains_key("optional")); } #[test]