feat(skills): #140 wire /skill install/update/uninstall/trust + [skills] config
Slash-command surface for the community-skill installer: - `/skill install <github:owner/repo|https://...|<registry-name>>` parses the spec via `InstallSource::parse`, calls `install_with_registry`, and surfaces `NeedsApproval`/`NetworkDenied` with actionable messages pointing at `[network]` config (we deliberately don't dispatch a modal from the sync slash-command path; the underlying installer returns the outcome so a future approval wiring can reuse it). - `/skill update <name>` re-fetches and prints "no upstream change" when the checksum matches. - `/skill uninstall <name>` and `/skill trust <name>` both refuse to touch system skills (no `.installed-from` marker). - `/skills --remote` (or `/skills remote`) fetches the curated registry through the same network gate and prints `name — description (source)`. Internals: - Sub-command dispatch happens in `run_skill` before activation lookup, so a user can't accidentally activate a skill literally named `install`. Async install/update/uninstall plumbed through `tokio::task::block_in_place` + `Handle::current().block_on`, matching the existing pattern in `commands/cycle.rs`. - `installer_settings` loads `Config` on demand — `App` doesn't carry a `Config` reference, and the cost of a single TOML parse is negligible next to the network round-trip the install will make. Config: - New `[skills]` section in both `crates/tui/src/config.rs::Config` and the workspace `crates/config/src/lib.rs::ConfigToml` with `registry_url` (default: bundled raw GitHub index) and `max_install_size_bytes` (default: 5 MiB). - `merge_config` propagates the new field, default impls cover the unset case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -128,10 +128,29 @@ pub struct ConfigToml {
|
||||
/// to a permissive default that mirrors pre-v0.7.0 behavior.
|
||||
#[serde(default)]
|
||||
pub network: Option<NetworkPolicyToml>,
|
||||
/// Community skill installer settings (#140). Mirrors
|
||||
/// [`SkillsToml`] from the TUI side; the dispatcher consults
|
||||
/// `registry_url` when running `deepseek skill install`.
|
||||
#[serde(default)]
|
||||
pub skills: Option<SkillsToml>,
|
||||
#[serde(flatten)]
|
||||
pub extras: BTreeMap<String, toml::Value>,
|
||||
}
|
||||
|
||||
/// On-disk schema for the `[skills]` table (#140). See `config.example.toml`
|
||||
/// for documentation.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
|
||||
pub struct SkillsToml {
|
||||
/// Curated registry index URL. When unset, the TUI falls back to the
|
||||
/// bundled default (community-curated GitHub raw).
|
||||
#[serde(default)]
|
||||
pub registry_url: Option<String>,
|
||||
/// Per-skill maximum *uncompressed* size in bytes. When unset, the TUI
|
||||
/// uses 5 MiB.
|
||||
#[serde(default)]
|
||||
pub max_install_size_bytes: Option<u64>,
|
||||
}
|
||||
|
||||
/// On-disk schema for the `[network]` table (#135). See `config.example.toml`
|
||||
/// for documentation.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
|
||||
@@ -322,14 +322,14 @@ pub const COMMANDS: &[CommandInfo] = &[
|
||||
CommandInfo {
|
||||
name: "skills",
|
||||
aliases: &[],
|
||||
description: "List available skills",
|
||||
usage: "/skills",
|
||||
description: "List local skills (or --remote to browse the curated registry)",
|
||||
usage: "/skills [--remote]",
|
||||
},
|
||||
CommandInfo {
|
||||
name: "skill",
|
||||
aliases: &[],
|
||||
description: "Activate a skill for next message",
|
||||
usage: "/skill <name>",
|
||||
description: "Activate a skill, or install/update/uninstall/trust a community skill",
|
||||
usage: "/skill <name|install <spec>|update <name>|uninstall <name>|trust <name>>",
|
||||
},
|
||||
CommandInfo {
|
||||
name: "review",
|
||||
@@ -409,7 +409,7 @@ pub fn execute(cmd: &str, app: &mut App) -> CommandResult {
|
||||
"init" => init::init(app),
|
||||
|
||||
// Skills commands
|
||||
"skills" => skills::list_skills(app),
|
||||
"skills" => skills::list_skills(app, arg),
|
||||
"skill" => skills::run_skill(app, arg),
|
||||
"review" => review::review(app, arg),
|
||||
|
||||
|
||||
@@ -2,7 +2,12 @@
|
||||
|
||||
use std::fmt::Write;
|
||||
|
||||
use crate::network_policy::NetworkPolicy;
|
||||
use crate::skills::SkillRegistry;
|
||||
use crate::skills::install::{
|
||||
self, DEFAULT_MAX_SIZE_BYTES, DEFAULT_REGISTRY_URL, InstallOutcome, InstallSource,
|
||||
RegistryFetchResult, UpdateResult,
|
||||
};
|
||||
use crate::tui::app::App;
|
||||
use crate::tui::history::HistoryCell;
|
||||
|
||||
@@ -21,8 +26,18 @@ fn render_skill_warnings(registry: &SkillRegistry) -> String {
|
||||
out
|
||||
}
|
||||
|
||||
/// List all available skills
|
||||
pub fn list_skills(app: &mut App) -> CommandResult {
|
||||
/// List all available skills. Pass `--remote` (or `remote`) to fetch the
|
||||
/// curated registry instead of scanning the local skills directory.
|
||||
pub fn list_skills(app: &mut App, arg: Option<&str>) -> CommandResult {
|
||||
if let Some(arg) = arg {
|
||||
let trimmed = arg.trim();
|
||||
if trimmed == "--remote" || trimmed == "remote" {
|
||||
return list_remote_skills(app);
|
||||
}
|
||||
if !trimmed.is_empty() {
|
||||
return CommandResult::error("Usage: /skills [--remote]");
|
||||
}
|
||||
}
|
||||
let skills_dir = app.skills_dir.clone();
|
||||
let registry = SkillRegistry::discover(&skills_dir);
|
||||
let warnings = render_skill_warnings(®istry);
|
||||
@@ -61,15 +76,35 @@ pub fn list_skills(app: &mut App) -> CommandResult {
|
||||
CommandResult::message(output)
|
||||
}
|
||||
|
||||
/// Run a specific skill - activates skill for next user message
|
||||
/// Run a specific skill — activates skill for next user message, or
|
||||
/// dispatches a sub-command (`install`, `update`, `uninstall`, `trust`).
|
||||
pub fn run_skill(app: &mut App, name: Option<&str>) -> CommandResult {
|
||||
let name = match name {
|
||||
let raw = match name {
|
||||
Some(n) => n.trim(),
|
||||
None => {
|
||||
return CommandResult::error("Usage: /skill <name>");
|
||||
return CommandResult::error(
|
||||
"Usage: /skill <name>\n\nSubcommands:\n /skill install <github:owner/repo|https://…|<registry-name>>\n /skill update <name>\n /skill uninstall <name>\n /skill trust <name>",
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
// Sub-command dispatch happens before the activation path so users can't
|
||||
// accidentally activate a skill literally named "install".
|
||||
let mut iter = raw.splitn(2, char::is_whitespace);
|
||||
let head = iter.next().unwrap_or("").trim();
|
||||
let rest = iter.next().unwrap_or("").trim();
|
||||
match head {
|
||||
"install" => return install_skill(app, rest),
|
||||
"update" => return update_skill(app, rest),
|
||||
"uninstall" => return uninstall_skill(app, rest),
|
||||
"trust" => return trust_skill(app, rest),
|
||||
_ => {}
|
||||
}
|
||||
|
||||
activate_skill(app, raw)
|
||||
}
|
||||
|
||||
fn activate_skill(app: &mut App, name: &str) -> CommandResult {
|
||||
// `/skill new` is a friendly alias for `/skill skill-creator`.
|
||||
let name = if name == "new" { "skill-creator" } else { name };
|
||||
|
||||
@@ -111,6 +146,214 @@ pub fn run_skill(app: &mut App, name: Option<&str>) -> CommandResult {
|
||||
}
|
||||
}
|
||||
|
||||
// ─── /skill install ────────────────────────────────────────────────────────
|
||||
|
||||
fn install_skill(app: &mut App, spec: &str) -> CommandResult {
|
||||
if spec.is_empty() {
|
||||
return CommandResult::error(
|
||||
"Usage: /skill install <github:owner/repo|https://…|<registry-name>>",
|
||||
);
|
||||
}
|
||||
let source = match InstallSource::parse(spec) {
|
||||
Ok(s) => s,
|
||||
Err(err) => return CommandResult::error(format!("Invalid install source: {err}")),
|
||||
};
|
||||
let skills_dir = app.skills_dir.clone();
|
||||
let (network, max_size, registry_url) = installer_settings(app);
|
||||
|
||||
let outcome = run_async(async move {
|
||||
install::install_with_registry(
|
||||
source,
|
||||
&skills_dir,
|
||||
max_size,
|
||||
&network,
|
||||
false,
|
||||
®istry_url,
|
||||
)
|
||||
.await
|
||||
});
|
||||
|
||||
match outcome {
|
||||
Ok(InstallOutcome::Installed(installed)) => {
|
||||
let path_str = path_or_default(&installed.path);
|
||||
CommandResult::message(format!(
|
||||
"Installed skill '{}' from {}.\nLocation: {}\n\nRun /skills to see it in the list.",
|
||||
installed.name, spec, path_str
|
||||
))
|
||||
}
|
||||
Ok(InstallOutcome::NeedsApproval(host)) => {
|
||||
CommandResult::error(needs_approval_message(&host))
|
||||
}
|
||||
Ok(InstallOutcome::NetworkDenied(host)) => {
|
||||
CommandResult::error(network_denied_message(&host))
|
||||
}
|
||||
Err(err) => CommandResult::error(format!("Install failed: {err:#}")),
|
||||
}
|
||||
}
|
||||
|
||||
// ─── /skill update ─────────────────────────────────────────────────────────
|
||||
|
||||
fn update_skill(app: &mut App, name: &str) -> CommandResult {
|
||||
if name.is_empty() {
|
||||
return CommandResult::error("Usage: /skill update <name>");
|
||||
}
|
||||
let skills_dir = app.skills_dir.clone();
|
||||
let (network, max_size, registry_url) = installer_settings(app);
|
||||
let owned_name = name.to_string();
|
||||
let outcome = run_async(async move {
|
||||
install::update_with_registry(&owned_name, &skills_dir, max_size, &network, ®istry_url)
|
||||
.await
|
||||
});
|
||||
|
||||
match outcome {
|
||||
Ok(UpdateResult::NoChange) => {
|
||||
CommandResult::message(format!("Skill '{name}': no upstream change."))
|
||||
}
|
||||
Ok(UpdateResult::Updated(installed)) => CommandResult::message(format!(
|
||||
"Skill '{}' updated. Location: {}",
|
||||
installed.name,
|
||||
path_or_default(&installed.path)
|
||||
)),
|
||||
Ok(UpdateResult::NeedsApproval(host)) => {
|
||||
CommandResult::error(needs_approval_message(&host))
|
||||
}
|
||||
Ok(UpdateResult::NetworkDenied(host)) => {
|
||||
CommandResult::error(network_denied_message(&host))
|
||||
}
|
||||
Err(err) => CommandResult::error(format!("Update failed: {err:#}")),
|
||||
}
|
||||
}
|
||||
|
||||
// ─── /skill uninstall ──────────────────────────────────────────────────────
|
||||
|
||||
fn uninstall_skill(app: &mut App, name: &str) -> CommandResult {
|
||||
if name.is_empty() {
|
||||
return CommandResult::error("Usage: /skill uninstall <name>");
|
||||
}
|
||||
match install::uninstall(name, &app.skills_dir) {
|
||||
Ok(()) => CommandResult::message(format!("Removed skill '{name}'.")),
|
||||
Err(err) => CommandResult::error(format!("Uninstall failed: {err:#}")),
|
||||
}
|
||||
}
|
||||
|
||||
// ─── /skill trust ──────────────────────────────────────────────────────────
|
||||
|
||||
fn trust_skill(app: &mut App, name: &str) -> CommandResult {
|
||||
if name.is_empty() {
|
||||
return CommandResult::error("Usage: /skill trust <name>");
|
||||
}
|
||||
match install::trust(name, &app.skills_dir) {
|
||||
Ok(()) => CommandResult::message(format!(
|
||||
"Marked skill '{name}' as trusted. Tools that consult the .trusted marker may now invoke its scripts/."
|
||||
)),
|
||||
Err(err) => CommandResult::error(format!("Trust failed: {err:#}")),
|
||||
}
|
||||
}
|
||||
|
||||
// ─── /skills --remote ──────────────────────────────────────────────────────
|
||||
|
||||
/// List skills available in the configured curated registry.
|
||||
pub fn list_remote_skills(app: &mut App) -> CommandResult {
|
||||
let (network, _max_size, registry_url) = installer_settings(app);
|
||||
let registry = run_async(async move { install::fetch_registry(&network, ®istry_url).await });
|
||||
match registry {
|
||||
Ok(RegistryFetchResult::Loaded(doc)) => {
|
||||
if doc.skills.is_empty() {
|
||||
return CommandResult::message("Registry is empty.");
|
||||
}
|
||||
let mut out = format!("Available remote skills ({}):\n", doc.skills.len());
|
||||
out.push_str("─────────────────────────────\n");
|
||||
for (name, entry) in &doc.skills {
|
||||
let _ = writeln!(
|
||||
out,
|
||||
" {name} — {} (source: {})",
|
||||
entry.description.clone().unwrap_or_default(),
|
||||
entry.source
|
||||
);
|
||||
}
|
||||
let _ = write!(out, "\nInstall with: /skill install <name>");
|
||||
CommandResult::message(out)
|
||||
}
|
||||
Ok(RegistryFetchResult::NeedsApproval(host)) => {
|
||||
CommandResult::error(needs_approval_message(&host))
|
||||
}
|
||||
Ok(RegistryFetchResult::Denied(host)) => {
|
||||
CommandResult::error(network_denied_message(&host))
|
||||
}
|
||||
Err(err) => CommandResult::error(format!("Failed to fetch registry: {err:#}")),
|
||||
}
|
||||
}
|
||||
|
||||
// ─── helpers ───────────────────────────────────────────────────────────────
|
||||
|
||||
/// Read the active config knobs for the installer.
|
||||
///
|
||||
/// We load `Config::load` on demand because [`App`] does not carry a `Config`
|
||||
/// field — and loading is cheap (small TOML file) compared to the network
|
||||
/// round-trip the install/update operation will incur next. If the config
|
||||
/// fails to parse, we fall back to defaults so the user still gets a
|
||||
/// network-gated install rather than a silent crash.
|
||||
fn installer_settings(_app: &App) -> (NetworkPolicy, u64, String) {
|
||||
let cfg = crate::config::Config::load(None, None).unwrap_or_default();
|
||||
let network = cfg
|
||||
.network
|
||||
.clone()
|
||||
.map(|policy| policy.into_runtime())
|
||||
.unwrap_or_default();
|
||||
let skills_cfg = cfg.skills.as_ref();
|
||||
let max_size = skills_cfg
|
||||
.and_then(|s| s.max_install_size_bytes)
|
||||
.unwrap_or(DEFAULT_MAX_SIZE_BYTES);
|
||||
let registry_url = skills_cfg
|
||||
.and_then(|s| s.registry_url.clone())
|
||||
.unwrap_or_else(|| DEFAULT_REGISTRY_URL.to_string());
|
||||
(network, max_size, registry_url)
|
||||
}
|
||||
|
||||
fn run_async<F, T>(future: F) -> T
|
||||
where
|
||||
F: std::future::Future<Output = T>,
|
||||
{
|
||||
// We're on the TUI's thread, which is part of the multi-threaded runtime.
|
||||
// `block_in_place` + `Handle::current().block_on` is the pattern used by
|
||||
// `commands/cycle.rs` to bridge sync slash-command handlers back into the
|
||||
// async ecosystem.
|
||||
tokio::task::block_in_place(|| tokio::runtime::Handle::current().block_on(future))
|
||||
}
|
||||
|
||||
fn path_or_default(path: &std::path::Path) -> String {
|
||||
path.file_name()
|
||||
.map(|n| {
|
||||
// Display with parent so the user sees the full skill location.
|
||||
// We intentionally use `display()` here because it's just for
|
||||
// user-facing output, not for path comparisons.
|
||||
let parent = path
|
||||
.parent()
|
||||
.map(|p| p.display().to_string())
|
||||
.unwrap_or_default();
|
||||
if parent.is_empty() {
|
||||
n.to_string_lossy().to_string()
|
||||
} else {
|
||||
format!("{parent}/{}", n.to_string_lossy())
|
||||
}
|
||||
})
|
||||
.unwrap_or_else(|| path.display().to_string())
|
||||
}
|
||||
|
||||
fn needs_approval_message(host: &str) -> String {
|
||||
format!(
|
||||
"Network policy requires approval for {host}.\n\
|
||||
Add it to your allow list with `/network allow {host}` (or set [network].default = \"allow\" in ~/.deepseek/config.toml), then retry."
|
||||
)
|
||||
}
|
||||
|
||||
fn network_denied_message(host: &str) -> String {
|
||||
format!(
|
||||
"Network policy denied access to {host}.\n\
|
||||
Remove the deny entry from ~/.deepseek/config.toml under [network] or contact your administrator."
|
||||
)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -150,7 +393,7 @@ mod tests {
|
||||
fn test_list_skills_empty_directory() {
|
||||
let tmpdir = TempDir::new().unwrap();
|
||||
let mut app = create_test_app_with_tmpdir(&tmpdir);
|
||||
let result = list_skills(&mut app);
|
||||
let result = list_skills(&mut app, None);
|
||||
assert!(result.message.is_some());
|
||||
let msg = result.message.unwrap();
|
||||
assert!(msg.contains("No skills found"));
|
||||
@@ -166,13 +409,32 @@ mod tests {
|
||||
"---\nname: test-skill\ndescription: A test skill\n---\nDo something",
|
||||
);
|
||||
let mut app = create_test_app_with_tmpdir(&tmpdir);
|
||||
let result = list_skills(&mut app);
|
||||
let result = list_skills(&mut app, None);
|
||||
assert!(result.message.is_some());
|
||||
let msg = result.message.unwrap();
|
||||
assert!(msg.contains("Available skills"));
|
||||
assert!(msg.contains("/test-skill"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_skill_subcommand_dispatch_install_usage() {
|
||||
let tmpdir = TempDir::new().unwrap();
|
||||
let mut app = create_test_app_with_tmpdir(&tmpdir);
|
||||
// Empty install spec → usage hint, not invalid-source error.
|
||||
let result = run_skill(&mut app, Some("install"));
|
||||
let msg = result.message.unwrap();
|
||||
assert!(msg.contains("/skill install"), "got: {msg}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_skill_subcommand_dispatch_uninstall_missing() {
|
||||
let tmpdir = TempDir::new().unwrap();
|
||||
let mut app = create_test_app_with_tmpdir(&tmpdir);
|
||||
let result = run_skill(&mut app, Some("uninstall absent-skill"));
|
||||
let msg = result.message.unwrap();
|
||||
assert!(msg.contains("not installed"), "got: {msg}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_run_skill_without_name() {
|
||||
let tmpdir = TempDir::new().unwrap();
|
||||
|
||||
@@ -429,6 +429,43 @@ pub struct Config {
|
||||
/// to a permissive default that mirrors pre-v0.7.0 behavior.
|
||||
#[serde(default)]
|
||||
pub network: Option<NetworkPolicyToml>,
|
||||
|
||||
/// Community skill installer settings (#140). When absent, installer
|
||||
/// commands fall back to the bundled defaults
|
||||
/// ([`crate::skills::install::DEFAULT_REGISTRY_URL`] +
|
||||
/// [`crate::skills::install::DEFAULT_MAX_SIZE_BYTES`]).
|
||||
#[serde(default)]
|
||||
pub skills: Option<SkillsConfig>,
|
||||
}
|
||||
|
||||
/// `[skills]` table — knobs for the community-skill installer.
|
||||
#[derive(Debug, Clone, Deserialize, Default)]
|
||||
pub struct SkillsConfig {
|
||||
/// Curated registry index. `/skill install <name>` looks up the spec here.
|
||||
/// Defaults to [`crate::skills::install::DEFAULT_REGISTRY_URL`].
|
||||
#[serde(default)]
|
||||
pub registry_url: Option<String>,
|
||||
/// Per-skill maximum *uncompressed* size in bytes. Tarballs that exceed
|
||||
/// this limit are rejected during validation. Defaults to 5 MiB.
|
||||
#[serde(default)]
|
||||
pub max_install_size_bytes: Option<u64>,
|
||||
}
|
||||
|
||||
impl SkillsConfig {
|
||||
/// Resolve the registry URL with the bundled default.
|
||||
#[must_use]
|
||||
pub fn registry_url(&self) -> String {
|
||||
self.registry_url
|
||||
.clone()
|
||||
.unwrap_or_else(|| crate::skills::install::DEFAULT_REGISTRY_URL.to_string())
|
||||
}
|
||||
|
||||
/// Resolve the max install size with the bundled default.
|
||||
#[must_use]
|
||||
pub fn max_install_size_bytes(&self) -> u64 {
|
||||
self.max_install_size_bytes
|
||||
.unwrap_or(crate::skills::install::DEFAULT_MAX_SIZE_BYTES)
|
||||
}
|
||||
}
|
||||
|
||||
/// `[network]` table — mirrors `deepseek_config::NetworkPolicyToml` so the live
|
||||
@@ -1384,6 +1421,7 @@ fn merge_config(base: Config, override_cfg: Config) -> Config {
|
||||
features: merge_features(base.features, override_cfg.features),
|
||||
notifications: override_cfg.notifications.or(base.notifications),
|
||||
network: override_cfg.network.or(base.network),
|
||||
skills: override_cfg.skills.or(base.skills),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user