mirror of
https://github.com/RightNow-AI/openfang.git
synced 2026-07-30 06:32:17 +00:00
skill config ui
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -9316,7 +9316,7 @@ pub async fn schedule_delivery_log(
|
||||
let targets: Vec<serde_json::Value> = 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,
|
||||
|
||||
@@ -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();
|
||||
},
|
||||
|
||||
@@ -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<AppState>,
|
||||
_tmp: tempfile::TempDir,
|
||||
}
|
||||
|
||||
impl Drop for TestServer {
|
||||
fn drop(&mut self) {
|
||||
self.state.kernel.shutdown();
|
||||
}
|
||||
}
|
||||
|
||||
/// Write a skill fixture under `<home>/skills/<name>/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");
|
||||
}
|
||||
@@ -121,6 +121,11 @@ pub enum AppEvent {
|
||||
SkillUninstalled(String),
|
||||
/// MCP servers loaded.
|
||||
McpServersLoaded(Vec<McpServerInfo>),
|
||||
/// Skill config details loaded (installed skill `c` key).
|
||||
SkillConfigLoaded {
|
||||
skill: String,
|
||||
rows: Vec<crate::tui::screens::skills::SkillConfigVarDetail>,
|
||||
},
|
||||
/// Templates providers loaded (auth status).
|
||||
TemplateProvidersLoaded(Vec<ProviderAuth>),
|
||||
/// Security features loaded.
|
||||
@@ -1439,6 +1444,14 @@ pub fn spawn_fetch_skills(backend: BackendRef, tx: mpsc::Sender<AppEvent>) {
|
||||
.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<AppEvent>)
|
||||
});
|
||||
}
|
||||
|
||||
/// 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<AppEvent>,
|
||||
) {
|
||||
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::<serde_json::Value>() {
|
||||
let declared = body.get("declared").cloned().unwrap_or_default();
|
||||
let resolved = body.get("resolved").cloned().unwrap_or_default();
|
||||
let mut rows: Vec<SkillConfigVarDetail> = 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<AppEvent>) {
|
||||
std::thread::spawn(move || match backend {
|
||||
|
||||
@@ -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());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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<SkillConfigVarDetail>)>,
|
||||
}
|
||||
|
||||
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<Rect> = 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<ListItem> = 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<Line> = 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
|
||||
|
||||
@@ -92,6 +92,15 @@ pub struct OpenFangKernel {
|
||||
pub model_catalog: std::sync::RwLock<openfang_runtime::model_catalog::ModelCatalog>,
|
||||
/// Skill registry for plugin skills (RwLock for hot-reload on install/uninstall).
|
||||
pub skill_registry: std::sync::RwLock<openfang_skills::registry::SkillRegistry>,
|
||||
/// 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<std::collections::HashMap<String, std::collections::HashMap<String, String>>>,
|
||||
>,
|
||||
/// Tracks running agent tasks for cancellation support.
|
||||
pub running_tasks: dashmap::DashMap<AgentId, tokio::task::AbortHandle>,
|
||||
/// 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<String, std::collections::HashMap<String, String>>,
|
||||
) {
|
||||
{
|
||||
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.
|
||||
///
|
||||
|
||||
@@ -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]
|
||||
|
||||
Reference in New Issue
Block a user