From 22fdaeae2c3a5df97f146617ccdfc01172714b47 Mon Sep 17 00:00:00 2001 From: bellman Date: Thu, 4 Jun 2026 03:58:35 +0900 Subject: [PATCH] fix: keep skills lifecycle local --- ROADMAP.md | 4 +- USAGE.md | 14 +- rust/README.md | 6 +- rust/crates/commands/src/lib.rs | 570 ++++++++++++++++-- rust/crates/rusty-claude-cli/src/main.rs | 47 +- .../tests/output_format_contract.rs | 294 +++++++-- 6 files changed, 805 insertions(+), 130 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index ab652332..ae9e0021 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -6368,7 +6368,7 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed) 430. **DONE — `dump-manifests` emits the self-contained Rust resolver inventory instead of requiring upstream Claude Code TypeScript source files** — fixed 2026-06-03 in `fix: make dump-manifests self-contained`. `claw dump-manifests --output-format json` now succeeds from an installed workspace with `source:"rust-resolver"`, command/tool/agent/skill/bootstrap manifests, no `CLAUDE_CODE_UPSTREAM` hint, and no `src/commands.ts` dependency. Explicit `--manifests-dir` scopes resolver discovery to another directory and missing/not-directory roots emit typed `missing_manifests` JSON. Sibling export diagnostics now validate explicit positional/`--output` paths before session discovery and return typed `invalid_output_path` JSON with `path` and `reason`. Regression coverage: `dump_manifests_defaults_to_rust_resolver_inventory`, `dump_manifests_scopes_explicit_manifest_dir_without_upstream_ts`, `dump_manifests_missing_explicit_dir_has_typed_kind`, `dump_manifests_and_init_emit_json_when_requested`, `local_json_surfaces_have_non_empty_action_contract_714`, and `export_invalid_output_path_reports_typed_json_430`. -431. **`skills uninstall ` requires Anthropic credentials despite being a local filesystem operation — `claw skills uninstall nonexistent-skill-xyz --output-format json` returns `kind:"missing_credentials"` instead of resolving locally that the skill doesn't exist** — dogfooded 2026-05-11 by Jobdori on `328fd114` in response to Clawhip pinpoint nudge at `1503275502046023690` (sibling probe to #430). Reproduction (no creds, isolated `CLAW_CONFIG_HOME`): `claw skills uninstall nonexistent-skill-xyz --output-format json` returns `{"error":"missing Anthropic credentials; export ANTHROPIC_AUTH_TOKEN or ANTHROPIC_API_KEY...","kind":"missing_credentials"}`. Uninstalling a skill is a pure local filesystem operation: read the skills directory, find the named skill, remove its files. There is no semantic reason to require API credentials. Same class of bug as #357 (`session list` requires creds), #369 (`session help/fork` require creds), and #427 (`resume ` requires creds). **Three sibling findings in same probe:** (a) `claw skills install ` returns `{"error":"No such file or directory (os error 2)","kind":"unknown"}` — leaks raw OS error string with no hint about expected install source format (path vs name vs URL?), and the catch-all `kind:"unknown"` again instead of typed `kind:"skill_install_source_not_found"`. (b) `claw skills install` (no args) returns `action:"help"` with `unexpected:"install"` — but `install` IS a documented subcommand. The handler treats it as "unknown action" instead of "missing required argument". Should emit `kind:"missing_argument"` with `argument:"install_source"`. (c) `claw agents create my-agent` returns `action:"help"` with `unexpected:"create my-agent"` — there is no agent-creation surface at all. Users must hand-craft `.claw/agents/.md` files with no scaffolding command, while `claw init` only creates the top-level `.claw/` skeleton. **Required fix shape:** (a) `skills uninstall ` must be local-first: enumerate the local skills dir, return `kind:"skill_not_found"` (with `skills_dir:` and `available_names:[]` fields) for missing, or remove the files and return `kind:"skills"` with `action:"uninstall", removed:` for present skills; (b) `skills install ` must distinguish source forms (`path:`, `name:`, `url:`) and emit `kind:"invalid_install_source"` with the parsed-and-failed reason; (c) `skills install` (no args) emits `kind:"missing_argument"` with `argument:"install_source"`; (d) add `claw agents create ` (or `claw init agent `) that scaffolds `.claw/agents/.md` with a stub frontmatter; or document explicitly that agents are user-authored only. **Why this matters:** lifecycle commands (`uninstall`, `install`, `create`) are the primary surface for managing claw's extension surface area. If `uninstall` requires API creds, an offline user who fat-fingered an install can't undo it. If `install` returns a raw OS error, automation can't programmatically recover. If `agents create` doesn't exist, agent authoring is undocumented file-touching only. Cross-references #357, #369, #427 (auth-gate-on-local-ops cluster), and #422/#423/#428/#430 (`kind:"unknown"` catch-all cluster). Source: Jobdori live dogfood, `328fd114`, 2026-05-11. +431. **DONE — `skills uninstall ` resolves locally instead of requiring Anthropic credentials** — fixed 2026-06-03 in `fix: keep skills lifecycle local`. `claw skills uninstall nonexistent-skill-xyz --output-format json` now stays on the local skills lifecycle surface and emits `kind:"skills"`, `action:"uninstall"`, `error_kind:"skill_not_found"`, `skills_dir`, `available_names`, and a hint without provider credentials. `claw skills install` no-arg emits typed `missing_argument` with `argument:"install_source"`; `claw skills install ` emits typed `invalid_install_source` with `source`, `source_kind`, `reason`, and a recovery hint. Installed skill roundtrips remove the installed files through the shared local lifecycle helper. `claw agents create ` now scaffolds `.claw/agents/.toml` and lists through the existing TOML agent discovery surface. Regression coverage: `skills_lifecycle_errors_have_typed_local_json_795_431`, `skills_install_uninstall_roundtrip_stays_local_431`, `agents_create_scaffolds_toml_and_lists_locally_431`, local command routing tests, parser discriminant tests, and command help/docs assertions. 432. **`--allowedTools` validator inconsistency: tool name list is half snake_case (`bash`, `read_file`, `write_file`, `edit_file`, `glob_search`, `grep_search`) and half PascalCase (`WebFetch`, `WebSearch`, `TodoWrite`, `Skill`, `Agent`, `Sleep`) with three UPPERCASE entries (`REPL`, `LSP`, `MCP`); accepts undocumented CamelCase aliases (`Read`, `Write`, `Edit`) and silently translates them to snake_case; argument parsing consumes the next positional when value is missing** — dogfooded 2026-05-11 by Jobdori on `fad53e2d` in response to Clawhip pinpoint nudge at `1503283046856655029`. Reproduction: `claw --allowedTools status --output-format json` → `{"error":"unsupported tool in --allowedTools: status (expected one of: bash, read_file, write_file, edit_file, glob_search, grep_search, WebFetch, WebSearch, TodoWrite, Skill, Agent, ToolSearch, NotebookEdit, Sleep, SendUserMessage, Config, EnterPlanMode, ExitPlanMode, StructuredOutput, REPL, PowerShell, AskUserQuestion, TaskCreate, RunTaskPacket, TaskGet, TaskList, TaskStop, TaskUpdate, TaskOutput, WorkerCreate, WorkerGet, WorkerObserve, WorkerResolveTrust, WorkerAwaitReady, WorkerSendPrompt, WorkerRestart, WorkerTerminate, WorkerObserveCompletion, TeamCreate, TeamDelete, CronCreate, CronDelete, CronList, LSP, ListMcpResources, ReadMcpResource, McpAuth, RemoteTrigger, MCP, TestingPermission)","kind":"unknown"}`. The `status` subcommand was consumed as the `--allowedTools` value because the flag parser doesn't distinguish missing-value from end-of-flag-args. The error reveals **the supported tool list mixes naming conventions inconsistently within a single error message**: snake_case (`bash`, `read_file`, `write_file`, `edit_file`, `glob_search`, `grep_search`), PascalCase (`WebFetch`, `WebSearch`, `TodoWrite`, `Skill`, `Agent`, `Sleep`, `Config`, `PowerShell`, `AskUserQuestion`, `TaskCreate`, `WorkerCreate`, `TeamCreate`, `CronCreate`), UPPERCASE (`REPL`, `LSP`, `MCP`), and CamelCase compounds (`McpAuth`, `RemoteTrigger`). **Hidden alias mapping**: `claw --allowedTools Read,Write,Edit status --output-format json` is accepted and returns `allowed_tools.entries:["edit_file","read_file","write_file"]` — proving the validator has an undocumented CamelCase→snake_case alias map (`Read`→`read_file`, `Write`→`write_file`, `Edit`→`edit_file`) that is not surfaced in the error message. Users who copy-paste tool names from Claude Code documentation work, users who copy from the validator error don't. **Sibling missing-value bug:** `claw --allowedTools status` with `status` as a positional subcommand is interpreted as `--allowedTools=status`, swallowing the subcommand. The flag parser must require a value for `--allowedTools` and emit `kind:"missing_argument"` when followed by a recognized subcommand or `--`-prefixed flag instead of silently treating the next arg as a tool name. **Sibling typed-kind bug:** both errors use `kind:"unknown"` instead of typed `kind:"invalid_tool_name"` / `kind:"missing_argument"` — the catch-all keeps appearing (#422/#423/#424/#428/#430/#431/#432). **Required fix shape:** (a) standardize the canonical tool-name registry on one casing convention (snake_case is most CLI-ergonomic) and update both the registry and all CamelCase aliases; (b) document and expose the alias map (`tool_aliases:{Read:"read_file",...}`) in `claw doctor`/`status` and in the validator error; (c) flag parser must require a value for `--allowedTools` and refuse to consume a recognized subcommand or `-`/`--`-prefixed token as the value, emit `kind:"missing_argument"` with `argument:"--allowedTools"`; (d) emit `kind:"invalid_tool_name"` with `tool_name:` and `available:[]` fields instead of `kind:"unknown"`; (e) regression test that `claw --allowedTools ` rejects with `missing_argument`, and that the canonical name list in errors uses the same casing as the alias map. **Why this matters:** `--allowedTools` is the primary surface for restricting claw's tool surface area (security-relevant). Inconsistent naming between the validator error and the alias map means users following the error message guidance pick names that work in some places and fail in others. The missing-value bug silently swallows a subcommand, leading to confusing "unsupported tool: status" errors when the user actually wanted to run `claw status`. Cross-references #94/#97/#101/#106/#115/#123 (permission-rule audit), #428 (default permission_mode), #422/#423/#424/#428/#430/#431 (`kind:"unknown"` catch-all). Source: Jobdori live dogfood, `fad53e2d`, 2026-05-11. @@ -7755,7 +7755,7 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed) 794. **`claw plugins install /nonexistent/path` returned `error_kind:"unknown"` + `hint:null`** — dogfooded 2026-05-27 on `57a57ef7`. The error message `"plugin source '/path' was not found"` had no classifier arm, falling to `"unknown"`. Fix: added `plugin_source_not_found` classifier arm (`message.contains("plugin source") && message.contains("was not found")`); added `"plugin_source_not_found"` → `"Check that the path or URL is correct..."` to `fallback_hint_for_error_kind`. Unit test assertion added to `test_classify_error_kind`; integration test `plugins_install_not_found_path_returns_typed_kind_794` added. 56 CLI contract tests pass. [SCOPE: claw-code] Source: Jobdori plugins install probe on `57a57ef7`, 2026-05-27. -795. **`claw skills install /nonexistent` returned `skill_not_found + hint:null` and `claw skills uninstall x` returned `unsupported_skills_action + hint:null`** — dogfooded 2026-05-27 on `491f179a`. Both error kinds were missing from `fallback_hint_for_error_kind` table, so even though classify returned a typed kind, the hint field was always null. Fix: added `"skill_not_found"` → hint suggesting `claw skills list` / `claw skills install`; added `"unsupported_skills_action"` → hint listing supported actions. Integration test `skills_install_not_found_and_unsupported_action_have_hints_795` covers both paths. 57 CLI contract tests pass. [SCOPE: claw-code] Source: Jobdori skills lifecycle probe on `491f179a`, 2026-05-27. +795. **`claw skills install /nonexistent` returned `skill_not_found + hint:null` and `claw skills uninstall x` returned `unsupported_skills_action + hint:null`** — dogfooded 2026-05-27 on `491f179a`. Both error kinds were missing from `fallback_hint_for_error_kind` table, so even though classify returned a typed kind, the hint field was always null. Fix: added `skill_not_found` and `unsupported_skills_action` fallback hints. ROADMAP #431 later moved the lifecycle surface fully local: install failures now emit typed `invalid_install_source`, uninstall failures emit local `skill_not_found` with `skills_dir` and `available_names`, and the combined regression is covered by `skills_lifecycle_errors_have_typed_local_json_795_431` plus the install/uninstall roundtrip test. [SCOPE: claw-code] Source: Jobdori skills lifecycle probe on `491f179a`, 2026-05-27. 796. **`claw agents show ` and `claw skills show ` returned confusing `agent_not_found`/`skill_not_found` for the concatenated "name extra" string** — dogfooded 2026-05-27 on `18b4cee5`. `join_optional_args` passes all tokens as a space-joined string; both `show` handlers called `split_once(' ')` to extract the name but did not check if the remainder (after the first split) contained additional tokens. Extra positional args (including `--flags`) became part of the "name", silently mangling the lookup. Fix: added second `split_once(' ')` on the extracted name; if the result has two parts, return `unexpected_extra_args` with a usage hint. Valid single-name lookups are unaffected. Two new integration tests `agents_show_extra_positional_arg_returns_unexpected_extra_796`, `skills_show_extra_positional_arg_returns_unexpected_extra_796`. 59 CLI contract tests pass. [SCOPE: claw-code] Source: Jobdori agents/skills show extra-arg probe on `18b4cee5`, 2026-05-27. 797. **DONE — Installed `claw version --output-format json` reports `git_sha:null` / `Git SHA unknown`, so dogfood cannot tie the binary under test to a source revision** — dogfooded 2026-05-27 from `#clawcode-building-in-public` using an installed binary in a clean `ultraworkers/claw-code` checkout. The gap was that version/status/doctor did not provide a structured executable-vs-workspace provenance object when build metadata was missing or stale. [SCOPE: claw-code] diff --git a/USAGE.md b/USAGE.md index c58c913e..2a9a827f 100644 --- a/USAGE.md +++ b/USAGE.md @@ -477,11 +477,12 @@ let client = build_http_client_with(&config).expect("proxy client"); ## Skills -Use `/skills list` in the interactive REPL or `claw skills --output-format json` from the direct CLI to inspect installed skills. For offline/local installs, install the directory that contains `SKILL.md`, then verify the discovered name before invoking it: +Use `/skills list` in the interactive REPL or `claw skills --output-format json` from the direct CLI to inspect installed skills. For offline/local installs, install the directory that contains `SKILL.md`, then verify the discovered name before invoking it. `skills install`, `skills uninstall`, and `agents create` are local filesystem lifecycle commands; they do not require provider credentials. ```text /skills install /absolute/path/to/my-skill /skills list +/skills uninstall my-skill /skills my-skill ``` @@ -494,6 +495,7 @@ cd rust ./target/debug/claw status ./target/debug/claw sandbox ./target/debug/claw agents +./target/debug/claw agents create my-agent ./target/debug/claw mcp ./target/debug/claw skills ./target/debug/claw system-prompt --cwd .. --date 2026-04-04 @@ -513,6 +515,7 @@ git clone https://github.com/Xquik-dev/tweetclaw cd claw-code/rust ./target/debug/claw skills install ../../tweetclaw/skills/tweetclaw ./target/debug/claw skills show tweetclaw +./target/debug/claw skills uninstall tweetclaw ``` TweetClaw gives `claw` users a local skill guide for OpenClaw/Xquik workflows @@ -520,6 +523,15 @@ such as tweet search, reply search, follower export, monitors, webhooks, and approval-gated posting. Configure any Xquik credentials outside the prompt and avoid pasting API keys into chat. +## Author a local agent + +`claw agents create ` scaffolds a local `.claw/agents/.toml` file for the current workspace. The scaffold is intentionally small so you can edit the description, model, and reasoning effort before listing or invoking agents: + +```bash +./target/debug/claw agents create release-checker +./target/debug/claw agents list +``` + ## Session management REPL turns are persisted under `.claw/sessions/` in the current workspace. diff --git a/rust/README.md b/rust/README.md index 4150ad2a..80d0033c 100644 --- a/rust/README.md +++ b/rust/README.md @@ -100,7 +100,7 @@ Primary artifacts: | Slash commands (including `/skills`, `/agents`, `/mcp`, `/doctor`, `/plugin`, `/subagent`) | ✅ | | Hooks (`/hooks`, config-backed lifecycle hooks) | ✅ | | Plugin management surfaces | ✅ | -| Skills inventory / install surfaces | ✅ | +| Skills inventory / install / uninstall surfaces | ✅ | | Machine-readable JSON output across core CLI surfaces | ✅ | ## Model Aliases @@ -168,8 +168,8 @@ The REPL now exposes a much broader surface than the original minimal shell: - plugin management: `/plugin` (with aliases `/plugins`, `/marketplace`) Notable claw-first surfaces now available directly in slash form: -- `/skills [list|install |help]` -- `/agents [list|help]` +- `/skills [list|show |install |uninstall |help]` +- `/agents [list|show |create |help]` - `/mcp [list|show |help]` - `/doctor` - `/plugin [list|install |enable |disable |uninstall |update ]` diff --git a/rust/crates/commands/src/lib.rs b/rust/crates/commands/src/lib.rs index 79d401be..314a3598 100644 --- a/rust/crates/commands/src/lib.rs +++ b/rust/crates/commands/src/lib.rs @@ -239,15 +239,15 @@ const SLASH_COMMAND_SPECS: &[SlashCommandSpec] = &[ SlashCommandSpec { name: "agents", aliases: &[], - summary: "List configured agents", - argument_hint: Some("[list|help]"), + summary: "List, show, or create configured agents", + argument_hint: Some("[list|show |create |help]"), resume_supported: true, }, SlashCommandSpec { name: "skills", aliases: &["skill"], - summary: "List, install, or invoke available skills", - argument_hint: Some("[list|install |help| [args]]"), + summary: "List, install, uninstall, or invoke available skills", + argument_hint: Some("[list|show |install |uninstall |help| [args]]"), resume_supported: true, }, SlashCommandSpec { @@ -1767,13 +1767,25 @@ fn parse_list_or_help_args( args: Option, ) -> Result, SlashCommandParseError> { match normalize_optional_args(args.as_deref()) { - None | Some("list" | "help" | "-h" | "--help") => Ok(args), + None + | Some( + "list" | "help" | "-h" | "--help" | "show" | "info" | "describe" | "create", + ) => Ok(args), + Some(value) + if value.starts_with("list ") + || value.starts_with("show ") + || value.starts_with("info ") + || value.starts_with("describe ") + || value.starts_with("create ") => + { + Ok(args) + } Some(unexpected) => Err(command_error( &format!( - "Unexpected arguments for /{command}: {unexpected}. Use /{command}, /{command} list, or /{command} help." + "Unexpected arguments for /{command}: {unexpected}. Use /{command}, /{command} list, /{command} show , /{command} create , or /{command} help." ), command, - &format!("/{command} [list|help]"), + &format!("/{command} [list|show |create |help]"), )), } } @@ -1787,14 +1799,6 @@ fn parse_skills_args(args: Option<&str>) -> Result, SlashCommandP return Ok(Some(args.to_string())); } - if args == "install" { - return Err(command_error( - "Usage: /skills install ", - "skills", - "/skills install ", - )); - } - if let Some(target) = args.strip_prefix("install").map(str::trim) { if !target.is_empty() { return Ok(Some(format!("install {target}"))); @@ -2195,6 +2199,30 @@ struct InstalledSkill { installed_path: PathBuf, } +#[derive(Debug, Clone, PartialEq, Eq)] +struct UninstalledSkill { + invocation_name: String, + registry_root: PathBuf, + removed_path: PathBuf, + available_names: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +enum SkillUninstallOutcome { + Removed(UninstalledSkill), + Missing { + requested: String, + registry_root: PathBuf, + available_names: Vec, + }, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct CreatedAgent { + name: String, + path: PathBuf, +} + #[derive(Debug, Clone, PartialEq, Eq)] enum SkillInstallSource { Directory { root: PathBuf, prompt_path: PathBuf }, @@ -2422,10 +2450,32 @@ pub fn handle_agents_slash_command(args: Option<&str>, cwd: &Path) -> std::io::R } Ok(render_agents_report(&matched)) } + Some("create") => Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "missing_argument: agents create requires an agent name.\nUsage: claw agents create ", + )), + Some(args) if args.starts_with("create ") => { + let mut parts = args.split_whitespace(); + let _ = parts.next(); + let Some(name) = parts.next() else { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "missing_argument: agents create requires an agent name.\nUsage: claw agents create ", + )); + }; + if let Some(extra) = parts.next() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("unexpected extra arguments after agent name\nUsage: claw agents create \nUnexpected extra: '{extra}'"), + )); + } + let agent = create_agent(name, cwd)?; + Ok(render_agent_create_report(&agent)) + } Some(args) if is_help_arg(args) => Ok(render_agents_usage(None)), Some(args) => Err(std::io::Error::new( std::io::ErrorKind::InvalidInput, - format!("unknown agents subcommand: {args}.\nSupported: list, show, help"), + format!("unknown agents subcommand: {args}.\nSupported: list, show, create, help"), )), } } @@ -2522,10 +2572,32 @@ pub fn handle_agents_slash_command_json(args: Option<&str>, cwd: &Path) -> std:: } Ok(render_agents_report_json_with_action(cwd, &matched, "show")) } + Some("create") => Ok(render_agents_missing_argument_json("create", "agent_name")), + Some(args) if args.starts_with("create ") => { + let mut parts = args.split_whitespace(); + let _ = parts.next(); + let Some(name) = parts.next() else { + return Ok(render_agents_missing_argument_json("create", "agent_name")); + }; + if let Some(extra) = parts.next() { + return Ok(json!({ + "kind": "agents", + "action": "create", + "status": "error", + "error_kind": "unexpected_extra_args", + "unexpected": extra, + "hint": format!("Usage: claw agents create \nUnexpected extra: '{extra}'"), + })); + } + match create_agent(name, cwd) { + Ok(agent) => Ok(render_agent_create_report_json(&agent)), + Err(error) => Ok(render_agent_create_error_json(name, &error)), + } + } Some(args) if is_help_arg(args) => Ok(render_agents_usage_json(None)), Some(args) => Err(std::io::Error::new( std::io::ErrorKind::InvalidInput, - format!("unknown agents subcommand: {args}.\nSupported: list, show, help"), + format!("unknown agents subcommand: {args}.\nSupported: list, show, create, help"), )), } } @@ -2627,15 +2699,53 @@ pub fn handle_skills_slash_command(args: Option<&str>, cwd: &Path) -> std::io::R } Ok(render_skills_report(&matched)) } - Some("install") => Ok(render_skills_usage(Some("install"))), + Some("install") => Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "missing_argument: skills install requires an install source.\nUsage: claw skills install ", + )), Some(args) if args.starts_with("install ") => { let target = args["install ".len()..].trim(); if target.is_empty() { - return Ok(render_skills_usage(Some("install"))); + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "missing_argument: skills install requires an install source.\nUsage: claw skills install ", + )); } let install = install_skill(target, cwd)?; Ok(render_skill_install_report(&install)) } + Some("uninstall" | "remove" | "delete") => Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "missing_argument: skills uninstall requires a skill name.\nUsage: claw skills uninstall ", + )), + Some(args) + if args.starts_with("uninstall ") + || args.starts_with("remove ") + || args.starts_with("delete ") => + { + let (_, target) = args.split_once(' ').unwrap_or_default(); + let target = target.trim(); + if target.is_empty() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "missing_argument: skills uninstall requires a skill name.\nUsage: claw skills uninstall ", + )); + } + match uninstall_skill(target)? { + SkillUninstallOutcome::Removed(skill) => Ok(render_skill_uninstall_report(&skill)), + SkillUninstallOutcome::Missing { + requested, + available_names, + .. + } => Err(std::io::Error::new( + std::io::ErrorKind::NotFound, + format!( + "skill '{requested}' not found\nAvailable skills: {}\nRun `claw skills list` to see available skills.", + format_optional_list(&available_names) + ), + )), + } + } Some(args) if is_help_arg(args) => Ok(render_skills_usage(None)), Some(args) => Ok(render_skills_usage(Some(args))), } @@ -2734,14 +2844,58 @@ pub fn handle_skills_slash_command_json(args: Option<&str>, cwd: &Path) -> std:: } Ok(render_skills_report_json_with_action(&matched, "show")) } - Some("install") => Ok(render_skills_usage_json(Some("install"))), + Some("install") => Ok(render_skills_missing_argument_json( + "install", + "install_source", + "Usage: claw skills install ", + )), Some(args) if args.starts_with("install ") => { let target = args["install ".len()..].trim(); if target.is_empty() { - return Ok(render_skills_usage_json(Some("install"))); + return Ok(render_skills_missing_argument_json( + "install", + "install_source", + "Usage: claw skills install ", + )); + } + match install_skill(target, cwd) { + Ok(install) => Ok(render_skill_install_report_json(&install)), + Err(error) => Ok(render_skill_install_error_json(target, &error)), + } + } + Some("uninstall" | "remove" | "delete") => Ok(render_skills_missing_argument_json( + "uninstall", + "skill_name", + "Usage: claw skills uninstall ", + )), + Some(args) + if args.starts_with("uninstall ") + || args.starts_with("remove ") + || args.starts_with("delete ") => + { + let (_, target) = args.split_once(' ').unwrap_or_default(); + let target = target.trim(); + if target.is_empty() { + return Ok(render_skills_missing_argument_json( + "uninstall", + "skill_name", + "Usage: claw skills uninstall ", + )); + } + match uninstall_skill(target)? { + SkillUninstallOutcome::Removed(skill) => { + Ok(render_skill_uninstall_report_json(&skill)) + } + SkillUninstallOutcome::Missing { + requested, + registry_root, + available_names, + } => Ok(render_skill_uninstall_missing_json( + &requested, + ®istry_root, + &available_names, + )), } - let install = install_skill(target, cwd)?; - Ok(render_skill_install_report_json(&install)) } Some(args) if is_help_arg(args) => Ok(render_skills_usage_json(None)), Some(args) => Ok(render_skills_usage_json(Some(args))), @@ -2751,9 +2905,11 @@ pub fn handle_skills_slash_command_json(args: Option<&str>, cwd: &Path) -> std:: #[must_use] pub fn classify_skills_slash_command(args: Option<&str>) -> SkillSlashDispatch { match normalize_optional_args(args) { - None | Some("list" | "help" | "-h" | "--help" | "show" | "info" | "describe") => { - SkillSlashDispatch::Local - } + None + | Some( + "list" | "help" | "-h" | "--help" | "show" | "info" | "describe" | "install" + | "uninstall" | "remove" | "delete", + ) => SkillSlashDispatch::Local, Some(args) if args .split_whitespace() @@ -2761,7 +2917,12 @@ pub fn classify_skills_slash_command(args: Option<&str>) -> SkillSlashDispatch { { SkillSlashDispatch::Local } - Some(args) if args == "install" || args.starts_with("install ") => { + Some(args) + if args.starts_with("install ") + || args.starts_with("uninstall ") + || args.starts_with("remove ") + || args.starts_with("delete ") => + { SkillSlashDispatch::Local } Some(args) @@ -2806,7 +2967,7 @@ pub fn resolve_skill_invocation( message.push_str(&names.join(", ")); } } - message.push_str("\n Usage: /skills [list|install |help| [args]]"); + message.push_str("\n Usage: /skills [list|show |install |uninstall |help| [args]]"); return Err(message); } } @@ -3461,6 +3622,103 @@ fn install_skill_into( }) } +fn uninstall_skill(target: &str) -> std::io::Result { + let registry_root = default_skill_install_root()?; + let requested = sanitize_skill_invocation_name(target).unwrap_or_else(|| { + target + .trim() + .trim_start_matches('/') + .trim_start_matches('$') + .to_ascii_lowercase() + }); + let available_names = installed_skill_names(®istry_root)?; + let matched_name = available_names + .iter() + .find(|name| name.eq_ignore_ascii_case(&requested)) + .cloned(); + + let Some(invocation_name) = matched_name else { + return Ok(SkillUninstallOutcome::Missing { + requested, + registry_root, + available_names, + }); + }; + + let removed_path = registry_root.join(&invocation_name); + if removed_path.is_dir() { + fs::remove_dir_all(&removed_path)?; + } else { + fs::remove_file(&removed_path)?; + } + let available_names = available_names + .into_iter() + .filter(|name| !name.eq_ignore_ascii_case(&invocation_name)) + .collect(); + + Ok(SkillUninstallOutcome::Removed(UninstalledSkill { + invocation_name, + registry_root, + removed_path, + available_names, + })) +} + +fn installed_skill_names(registry_root: &Path) -> std::io::Result> { + let entries = match fs::read_dir(registry_root) { + Ok(entries) => entries, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(error) => return Err(error), + }; + let mut names = Vec::new(); + for entry in entries { + let entry = entry?; + let path = entry.path(); + if path.is_dir() && path.join("SKILL.md").is_file() { + names.push(entry.file_name().to_string_lossy().to_string()); + } else if path + .extension() + .is_some_and(|extension| extension.to_string_lossy().eq_ignore_ascii_case("md")) + { + if let Some(stem) = path.file_stem() { + names.push(stem.to_string_lossy().to_string()); + } + } + } + names.sort(); + Ok(names) +} + +fn create_agent(name: &str, cwd: &Path) -> std::io::Result { + let Some(name) = sanitize_skill_invocation_name(name) else { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "invalid_agent_name: agent name must contain at least one alphanumeric character", + )); + }; + let root = cwd.join(".claw").join("agents"); + let path = root.join(format!("{name}.toml")); + if path.exists() { + return Err(std::io::Error::new( + std::io::ErrorKind::AlreadyExists, + format!( + "agent_already_exists: agent '{name}' already exists at {}", + path.display() + ), + )); + } + + fs::create_dir_all(&root)?; + fs::write( + &path, + format!( + "name = \"{name}\"\ndescription = \"Describe when to use this agent.\"\nmodel_reasoning_effort = \"medium\"\n" + ), + )?; + + Ok(CreatedAgent { name, path }) +} + fn default_skill_install_root() -> std::io::Result { if let Ok(claw_config_home) = env::var("CLAW_CONFIG_HOME") { return Ok(PathBuf::from(claw_config_home).join("skills")); @@ -3902,6 +4160,59 @@ fn render_agents_report_json_with_action( }) } +fn render_agents_missing_argument_json(action: &str, argument: &str) -> Value { + json!({ + "kind": "agents", + "action": action, + "status": "error", + "error_kind": "missing_argument", + "argument": argument, + "hint": "Usage: claw agents create ", + }) +} + +fn render_agent_create_report(agent: &CreatedAgent) -> String { + format!( + "Agents\n Result created {}\n Path {}\n Format TOML", + agent.name, + agent.path.display() + ) +} + +fn render_agent_create_report_json(agent: &CreatedAgent) -> Value { + json!({ + "kind": "agents", + "status": "ok", + "action": "create", + "result": "created", + "name": &agent.name, + "path": agent.path.display().to_string(), + "format": "toml", + }) +} + +fn render_agent_create_error_json(name: &str, error: &std::io::Error) -> Value { + let message = error.to_string(); + let error_kind = if message.starts_with("invalid_agent_name:") { + "invalid_agent_name" + } else if message.starts_with("agent_already_exists:") + || error.kind() == std::io::ErrorKind::AlreadyExists + { + "agent_already_exists" + } else { + "agent_create_failed" + }; + json!({ + "kind": "agents", + "status": "error", + "action": "create", + "error_kind": error_kind, + "name": name, + "message": message, + "hint": "Use `claw agents create ` with a simple alphanumeric, dash, underscore, or dot name.", + }) +} + fn agent_detail(agent: &AgentSummary) -> String { let mut parts = vec![agent.name.clone()]; if let Some(description) = &agent.description { @@ -4019,6 +4330,102 @@ fn render_skill_install_report_json(skill: &InstalledSkill) -> Value { }) } +fn render_skills_missing_argument_json(action: &str, argument: &str, hint: &str) -> Value { + json!({ + "kind": "skills", + "action": action, + "status": "error", + "error_kind": "missing_argument", + "argument": argument, + "hint": hint, + }) +} + +fn render_skill_install_error_json(target: &str, error: &std::io::Error) -> Value { + let source_kind = skill_install_source_kind(target); + json!({ + "kind": "skills", + "action": "install", + "status": "error", + "error_kind": "invalid_install_source", + "source": target, + "source_kind": source_kind, + "reason": io_error_reason(error), + "message": format!("invalid install source: {error}"), + "hint": match source_kind { + "url" => "Remote skill install is not supported yet; pass a local directory containing SKILL.md or a markdown file.", + "name" => "Skill install expects a local path, not a registry name. Pass a directory containing SKILL.md or a markdown file.", + _ => "Check that the path exists and is a directory containing SKILL.md or a markdown file.", + }, + }) +} + +fn render_skill_uninstall_report(skill: &UninstalledSkill) -> String { + format!( + "Skills\n Result uninstalled {}\n Registry {}\n Removed path {}\n Remaining {}", + skill.invocation_name, + skill.registry_root.display(), + skill.removed_path.display(), + format_optional_list(&skill.available_names) + ) +} + +fn render_skill_uninstall_report_json(skill: &UninstalledSkill) -> Value { + json!({ + "kind": "skills", + "status": "ok", + "action": "uninstall", + "result": "removed", + "removed": &skill.invocation_name, + "skills_dir": skill.registry_root.display().to_string(), + "removed_path": skill.removed_path.display().to_string(), + "available_names": &skill.available_names, + }) +} + +fn render_skill_uninstall_missing_json( + requested: &str, + registry_root: &Path, + available_names: &[String], +) -> Value { + json!({ + "kind": "skills", + "status": "error", + "action": "uninstall", + "error_kind": "skill_not_found", + "requested": requested, + "skills_dir": registry_root.display().to_string(), + "available_names": available_names, + "message": format!("skill '{requested}' not found"), + "hint": "Run `claw skills list` to see available skills.", + }) +} + +fn skill_install_source_kind(source: &str) -> &'static str { + let trimmed = source.trim(); + if trimmed.contains("://") { + "url" + } else if Path::new(trimmed).is_absolute() + || trimmed.starts_with('.') + || trimmed.contains('/') + || trimmed.contains('\\') + { + "path" + } else { + "name" + } +} + +fn io_error_reason(error: &std::io::Error) -> &'static str { + match error.kind() { + std::io::ErrorKind::NotFound => "not_found", + std::io::ErrorKind::AlreadyExists => "already_exists", + std::io::ErrorKind::PermissionDenied => "permission_denied", + std::io::ErrorKind::InvalidInput => "invalid", + _ => "io_error", + } +} + fn render_mcp_summary_report( cwd: &Path, servers: &BTreeMap, @@ -4188,8 +4595,10 @@ fn help_path_from_args(args: &str) -> Option> { fn render_agents_usage(unexpected: Option<&str>) -> String { let mut lines = vec![ "Agents".to_string(), - " Usage /agents [list|help]".to_string(), - " Direct CLI claw agents".to_string(), + " Usage /agents [list|show |create |help]".to_string(), + " Direct CLI claw agents [list|show |create |help]".to_string(), + " Format TOML files (.toml); create scaffolds .claw/agents/.toml" + .to_string(), " Sources .claw/agents, ~/.claw/agents, $CLAW_CONFIG_HOME/agents".to_string(), ]; if let Some(args) = unexpected { @@ -4205,8 +4614,10 @@ fn render_agents_usage_json(unexpected: Option<&str>) -> Value { "ok": unexpected.is_none(), "status": if unexpected.is_some() { "error" } else { "ok" }, "usage": { - "slash_command": "/agents [list|help]", - "direct_cli": "claw agents [list|help]", + "slash_command": "/agents [list|show |create |help]", + "direct_cli": "claw agents [list|show |create |help]", + "format": "toml", + "create": "claw agents create ", "sources": [".claw/agents", "~/.claw/agents", "$CLAW_CONFIG_HOME/agents"], }, "unexpected": unexpected, @@ -4216,9 +4627,10 @@ fn render_agents_usage_json(unexpected: Option<&str>) -> Value { fn render_skills_usage(unexpected: Option<&str>) -> String { let mut lines = vec![ "Skills".to_string(), - " Usage /skills [list|install |help| [args]]".to_string(), + " Usage /skills [list|show |install |uninstall |help| [args]]".to_string(), " Alias /skill".to_string(), - " Direct CLI claw skills [list|install |help| [args]]".to_string(), + " Direct CLI claw skills [list|show |install |uninstall |help| [args]]".to_string(), + " Lifecycle install , uninstall ".to_string(), " Invoke /skills help overview -> $help overview".to_string(), " Install root $CLAW_CONFIG_HOME/skills or ~/.claw/skills".to_string(), " Sources .claw/skills, .omc/skills, .agents/skills, .codex/skills, .claude/skills, ~/.claw/skills, ~/.omc/skills, ~/.claude/skills/omc-learned, ~/.codex/skills, ~/.claude/skills, legacy /commands".to_string(), @@ -4236,9 +4648,10 @@ fn render_skills_usage_json(unexpected: Option<&str>) -> Value { "ok": unexpected.is_none(), "status": if unexpected.is_some() { "error" } else { "ok" }, "usage": { - "slash_command": "/skills [list|install |help| [args]]", + "slash_command": "/skills [list|show |install |uninstall |help| [args]]", "aliases": ["/skill"], - "direct_cli": "claw skills [list|install |help| [args]]", + "direct_cli": "claw skills [list|show |install |uninstall |help| [args]]", + "lifecycle": ["install ", "uninstall "], "invoke": "/skills help overview -> $help overview", "install_root": "$CLAW_CONFIG_HOME/skills or ~/.claw/skills", "sources": [ @@ -5113,16 +5526,17 @@ mod tests { #[test] fn rejects_invalid_agents_arguments() { // given - let agents_input = "/agents show planner"; + let agents_input = "/agents frobnicate"; // when let agents_error = parse_error_message(agents_input); // then assert!(agents_error.contains( - "Unexpected arguments for /agents: show planner. Use /agents, /agents list, or /agents help." + "Unexpected arguments for /agents: frobnicate. Use /agents, /agents list, /agents show , /agents create , or /agents help." )); - assert!(agents_error.contains(" Usage /agents [list|help]")); + assert!(agents_error + .contains(" Usage /agents [list|show |create |help]")); } #[test] @@ -5144,6 +5558,13 @@ mod tests { "`skills {arg}` must be Local, not Invoke" ); } + for arg in ["uninstall", "uninstall plan", "remove plan", "delete plan"] { + assert_eq!( + classify_skills_slash_command(Some(arg)), + SkillSlashDispatch::Local, + "`skills {arg}` must be Local, not Invoke" + ); + } // Bare invocable tokens still dispatch to Invoke. assert_eq!( classify_skills_slash_command(Some("plan")), @@ -5171,6 +5592,10 @@ mod tests { classify_skills_slash_command(Some("install ./skill-pack")), SkillSlashDispatch::Local ); + assert_eq!( + classify_skills_slash_command(Some("uninstall help")), + SkillSlashDispatch::Local + ); } #[test] @@ -5263,8 +5688,10 @@ mod tests { "/plugin [list|install |enable |disable |uninstall |update ]" )); assert!(help.contains("aliases: /plugins, /marketplace")); - assert!(help.contains("/agents [list|help]")); - assert!(help.contains("/skills [list|install |help| [args]]")); + assert!(help.contains("/agents [list|show |create |help]")); + assert!(help.contains( + "/skills [list|show |install |uninstall |help| [args]]" + )); assert!(help.contains("aliases: /skill")); assert!(!help.contains("/login")); assert!(!help.contains("/logout")); @@ -5609,10 +6036,27 @@ mod tests { #[test] fn renders_agents_reports_as_json() { + let _guard = env_guard(); let workspace = temp_dir("agents-json-workspace"); let project_agents = workspace.join(".codex").join("agents"); let user_home = temp_dir("agents-json-home"); let user_agents = user_home.join(".codex").join("agents"); + let isolated_home = temp_dir("agents-json-isolated-home"); + let config_home = temp_dir("agents-json-config-home"); + let codex_home = temp_dir("agents-json-codex-home"); + let claude_config = temp_dir("agents-json-claude-config"); + fs::create_dir_all(&isolated_home).expect("isolated home"); + fs::create_dir_all(&config_home).expect("config home"); + fs::create_dir_all(&codex_home).expect("codex home"); + fs::create_dir_all(&claude_config).expect("claude config"); + let original_home = std::env::var_os("HOME"); + let original_claw_config_home = std::env::var_os("CLAW_CONFIG_HOME"); + let original_codex_home = std::env::var_os("CODEX_HOME"); + let original_claude_config_dir = std::env::var_os("CLAUDE_CONFIG_DIR"); + std::env::set_var("HOME", &isolated_home); + std::env::set_var("CLAW_CONFIG_HOME", &config_home); + std::env::set_var("CODEX_HOME", &codex_home); + std::env::set_var("CLAUDE_CONFIG_DIR", &claude_config); write_agent( &project_agents, @@ -5664,7 +6108,10 @@ mod tests { assert_eq!(help["kind"], "agents"); assert_eq!(help["action"], "help"); assert_eq!(help["status"], "ok"); - assert_eq!(help["usage"]["direct_cli"], "claw agents [list|help]"); + assert_eq!( + help["usage"]["direct_cli"], + "claw agents [list|show |create |help]" + ); // `show ` is now valid. Known agent returns ok with matching entry. let show_planner = handle_agents_slash_command_json(Some("show planner"), &workspace) @@ -5686,6 +6133,14 @@ mod tests { let _ = fs::remove_dir_all(workspace); let _ = fs::remove_dir_all(user_home); + restore_env_var("HOME", original_home); + restore_env_var("CLAW_CONFIG_HOME", original_claw_config_home); + restore_env_var("CODEX_HOME", original_codex_home); + restore_env_var("CLAUDE_CONFIG_DIR", original_claude_config_dir); + let _ = fs::remove_dir_all(isolated_home); + let _ = fs::remove_dir_all(config_home); + let _ = fs::remove_dir_all(codex_home); + let _ = fs::remove_dir_all(claude_config); } #[test] @@ -5816,7 +6271,7 @@ mod tests { assert_eq!(help["usage"]["aliases"][0], "/skill"); assert_eq!( help["usage"]["direct_cli"], - "claw skills [list|install |help| [args]]" + "claw skills [list|show |install |uninstall |help| [args]]" ); let _ = fs::remove_dir_all(workspace); @@ -5829,13 +6284,20 @@ mod tests { let agents_help = super::handle_agents_slash_command(Some("help"), &cwd).expect("agents help"); - assert!(agents_help.contains("Usage /agents [list|help]")); - assert!(agents_help.contains("Direct CLI claw agents")); + assert!( + agents_help.contains("Usage /agents [list|show |create |help]") + ); + assert!(agents_help + .contains("Direct CLI claw agents [list|show |create |help]")); + assert!(agents_help.contains( + "Format TOML files (.toml); create scaffolds .claw/agents/.toml" + )); assert!(agents_help .contains("Sources .claw/agents, ~/.claw/agents, $CLAW_CONFIG_HOME/agents")); // `show ` is now valid. For an agent that doesn't exist it returns Err(NotFound). - let agents_show_missing = super::handle_agents_slash_command(Some("show planner"), &cwd); + let agents_show_missing = + super::handle_agents_slash_command(Some("show definitely-missing-agent-431"), &cwd); assert!( agents_show_missing.is_err(), "show of a missing agent should Err" @@ -5854,9 +6316,11 @@ mod tests { let skills_help = super::handle_skills_slash_command(Some("--help"), &cwd).expect("skills help"); - assert!(skills_help - .contains("Usage /skills [list|install |help| [args]]")); + assert!(skills_help.contains( + "Usage /skills [list|show |install |uninstall |help| [args]]" + )); assert!(skills_help.contains("Alias /skill")); + assert!(skills_help.contains("Lifecycle install , uninstall ")); assert!(skills_help.contains("Invoke /skills help overview -> $help overview")); assert!(skills_help.contains("Install root $CLAW_CONFIG_HOME/skills or ~/.claw/skills")); assert!(skills_help.contains(".omc/skills")); @@ -5870,15 +6334,17 @@ mod tests { let skills_install_help = super::handle_skills_slash_command(Some("install --help"), &cwd) .expect("nested skills help"); - assert!(skills_install_help - .contains("Usage /skills [list|install |help| [args]]")); + assert!(skills_install_help.contains( + "Usage /skills [list|show |install |uninstall |help| [args]]" + )); assert!(skills_install_help.contains("Alias /skill")); assert!(skills_install_help.contains("Unexpected install")); let skills_unknown_help = super::handle_skills_slash_command(Some("show --help"), &cwd).expect("skills help"); - assert!(skills_unknown_help - .contains("Usage /skills [list|install |help| [args]]")); + assert!(skills_unknown_help.contains( + "Usage /skills [list|show |install |uninstall |help| [args]]" + )); assert!(skills_unknown_help.contains("Unexpected show")); let skills_help_json = diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index cf6b9927..594547e6 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -422,6 +422,8 @@ fn classify_error_kind(message: &str) -> &'static str { "missing_argument" } else if message.contains("unsupported skills action") { "unsupported_skills_action" + } else if message.starts_with("invalid_install_source:") { + "invalid_install_source" } else if message.starts_with("invalid_cwd:") { "invalid_cwd" } else if message.starts_with("invalid_output_path:") { @@ -567,9 +569,12 @@ fn fallback_hint_for_error_kind(kind: &str) -> Option<&'static str> { "skill_not_found" => Some( "Run `claw skills list` to see available skills, or `claw skills install ` to install a new one.", ), - // #795: unsupported action on skills (e.g. /skills uninstall) with no \n hint + // #795/#431: unsupported/invalid skills lifecycle input should include actionable local guidance. "unsupported_skills_action" => Some( - "Supported: list, install , show , help. Run `claw skills help` for details.", + "Supported: list, show , install , uninstall , help. Run `claw skills help` for details.", + ), + "invalid_install_source" => Some( + "Pass a local skill directory containing SKILL.md or a standalone markdown file.", ), _ => None, } @@ -1711,9 +1716,9 @@ fn parse_args(args: &[String]) -> Result { let args = join_optional_args(&rest[1..]); if let Some(action) = args.as_deref() { let first_word = action.split_whitespace().next().unwrap_or(action); - if matches!(first_word, "remove" | "add" | "uninstall" | "delete") { + if matches!(first_word, "add") { return Err(format!( - "unsupported skills action: {first_word}. Supported actions: list, install , help, or [args]" + "unsupported skills action: {first_word}. Supported actions: list, show , install , uninstall , help, or [args]" )); } } @@ -14408,6 +14413,10 @@ mod tests { classify_error_kind("unsupported skills action: bogus. Supported actions: list"), "unsupported_skills_action" ); + assert_eq!( + classify_error_kind("invalid_install_source: bogus"), + "invalid_install_source" + ); assert_eq!( classify_error_kind( "missing_flag_value: missing value for --model.\nUsage: --model " @@ -15056,17 +15065,27 @@ mod tests { #[test] fn unsupported_skills_actions_return_typed_error_683() { - for action in ["remove", "add", "uninstall", "delete"] { - let error = parse_args(&["skills".to_string(), action.to_string()]) - .expect_err(&format!("skills {action} should error")); - assert!( - error.contains("unsupported skills action"), - "skills {action} should contain 'unsupported skills action', got: {error}" - ); + let error = parse_args(&["skills".to_string(), "add".to_string()]) + .expect_err("skills add should error"); + assert!( + error.contains("unsupported skills action"), + "skills add should contain 'unsupported skills action', got: {error}" + ); + assert_eq!( + classify_error_kind(&error), + "unsupported_skills_action", + "skills add should classify as unsupported_skills_action, got: {error}" + ); + + for action in ["remove", "uninstall", "delete"] { assert_eq!( - classify_error_kind(&error), - "unsupported_skills_action", - "skills {action} should classify as unsupported_skills_action, got: {error}" + parse_args(&["skills".to_string(), action.to_string()]) + .expect(&format!("skills {action} should parse")), + CliAction::Skills { + args: Some(action.to_string()), + output_format: CliOutputFormat::Text, + }, + "skills {action} should route locally so missing targets are handled without credentials" ); } } diff --git a/rust/crates/rusty-claude-cli/tests/output_format_contract.rs b/rust/crates/rusty-claude-cli/tests/output_format_contract.rs index 0854b175..35d98c19 100644 --- a/rust/crates/rusty-claude-cli/tests/output_format_contract.rs +++ b/rust/crates/rusty-claude-cli/tests/output_format_contract.rs @@ -2346,6 +2346,16 @@ fn run_claw(current_dir: &Path, args: &[&str], envs: &[(&str, &str)]) -> Output command.output().expect("claw should launch") } +fn parse_json_stdout(output: &Output, context: &str) -> Value { + serde_json::from_slice(&output.stdout).unwrap_or_else(|_| { + panic!( + "{context} should emit valid stdout JSON; stdout:\n{}\n\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ) + }) +} + fn strings(items: &[&str]) -> Vec { items.iter().map(|item| (*item).to_string()).collect() } @@ -4531,86 +4541,254 @@ fn plugins_install_not_found_path_returns_typed_kind_794() { } #[test] -fn skills_install_not_found_and_unsupported_action_have_hints_795() { - // #795: `claw skills install /nonexistent` returned skill_not_found + hint:null, and - // `claw skills uninstall x` returned unsupported_skills_action + hint:null. Both error - // kinds were missing from fallback_hint_for_error_kind table. Fix: added both entries. - let root = unique_temp_dir("skills-install-795"); - fs::create_dir_all(&root).expect("temp dir"); +fn skills_lifecycle_errors_have_typed_local_json_795_431() { + // #431: skills install/uninstall lifecycle paths are local JSON surfaces and must not + // fall through to provider credential checks. #795: every error envelope needs a hint. + let root = unique_temp_dir("skills-lifecycle-431"); + let config_home = root.join("config-home"); + let home = root.join("home"); + fs::create_dir_all(&config_home).expect("config home"); + fs::create_dir_all(&home).expect("home"); std::process::Command::new("git") .args(["init", "-q"]) .current_dir(&root) .output() .ok(); + let envs = [ + ( + "CLAW_CONFIG_HOME", + config_home.to_str().expect("utf8 config home"), + ), + ("HOME", home.to_str().expect("utf8 home")), + ("ANTHROPIC_API_KEY", ""), + ("ANTHROPIC_AUTH_TOKEN", ""), + ("OPENAI_API_KEY", ""), + ]; - // skills install with nonexistent local path - let out1 = run_claw( + let missing_arg = run_claw( &root, - &[ - "--output-format", - "json", - "skills", - "install", - "/nonexistent-xyz-795", - ], - &[], + &["skills", "install", "--output-format", "json"], + &envs, ); + assert_eq!(missing_arg.status.code(), Some(1)); assert!( - !out1.status.success(), - "skills install not-found must exit non-zero (#795)" + missing_arg.stderr.is_empty(), + "stderr: {}", + String::from_utf8_lossy(&missing_arg.stderr) ); - let stderr1 = String::from_utf8_lossy(&out1.stderr); - let stdout1 = String::from_utf8_lossy(&out1.stdout); - let j1: serde_json::Value = stdout1 - .lines() - .find(|l| l.trim_start().starts_with('{')) - .and_then(|l| serde_json::from_str(l).ok()) - .expect("skills install not-found should emit JSON error"); - assert_eq!( - j1["error_kind"], "skill_not_found", - "skills install not-found should be skill_not_found, got {:?}", - j1["error_kind"] - ); - let h1 = j1["hint"] + let missing_arg_json = parse_json_stdout(&missing_arg, "skills install missing source"); + assert_eq!(missing_arg_json["kind"], "skills"); + assert_eq!(missing_arg_json["action"], "install"); + assert_eq!(missing_arg_json["error_kind"], "missing_argument"); + assert_eq!(missing_arg_json["argument"], "install_source"); + assert!(missing_arg_json["hint"] .as_str() - .expect("skill_not_found must have non-null hint (#795)"); - assert!( - h1.contains("skills list") || h1.contains("skills install"), - "hint should reference skills commands, got: {h1:?}" - ); + .is_some_and(|hint| !hint.is_empty())); - // skills uninstall (unsupported action) - let out2 = run_claw( + let invalid_source = run_claw( + &root, + &["skills", "install", "bogus-name", "--output-format", "json"], + &envs, + ); + assert_eq!(invalid_source.status.code(), Some(1)); + assert!( + invalid_source.stderr.is_empty(), + "stderr: {}", + String::from_utf8_lossy(&invalid_source.stderr) + ); + let invalid_source_json = parse_json_stdout(&invalid_source, "skills install invalid source"); + assert_eq!(invalid_source_json["kind"], "skills"); + assert_eq!(invalid_source_json["action"], "install"); + assert_eq!(invalid_source_json["error_kind"], "invalid_install_source"); + assert_eq!(invalid_source_json["source"], "bogus-name"); + assert_eq!(invalid_source_json["source_kind"], "name"); + assert_eq!(invalid_source_json["reason"], "not_found"); + assert!(invalid_source_json["hint"] + .as_str() + .is_some_and(|hint| { hint.contains("local path") || hint.contains("SKILL.md") })); + + let missing_uninstall = run_claw( &root, &[ - "--output-format", - "json", "skills", "uninstall", - "some-skill", + "nonexistent-skill-xyz", + "--output-format", + "json", ], - &[], + &envs, + ); + assert_eq!(missing_uninstall.status.code(), Some(1)); + assert!( + missing_uninstall.stderr.is_empty(), + "stderr: {}", + String::from_utf8_lossy(&missing_uninstall.stderr) + ); + let missing_uninstall_json = + parse_json_stdout(&missing_uninstall, "skills uninstall missing skill"); + assert_eq!(missing_uninstall_json["kind"], "skills"); + assert_eq!(missing_uninstall_json["action"], "uninstall"); + assert_eq!(missing_uninstall_json["error_kind"], "skill_not_found"); + assert_eq!(missing_uninstall_json["requested"], "nonexistent-skill-xyz"); + assert_eq!( + missing_uninstall_json["skills_dir"], + config_home.join("skills").display().to_string() + ); + assert_eq!( + missing_uninstall_json["available_names"] + .as_array() + .expect("available_names") + .len(), + 0 + ); + assert!(missing_uninstall_json["hint"] + .as_str() + .is_some_and(|hint| !hint.is_empty())); +} + +#[test] +fn skills_install_uninstall_roundtrip_stays_local_431() { + let root = unique_temp_dir("skills-roundtrip-431"); + let config_home = root.join("config-home"); + let home = root.join("home"); + let source_root = root.join("fixtures"); + fs::create_dir_all(&config_home).expect("config home"); + fs::create_dir_all(&home).expect("home"); + write_skill(&source_root, "roundtrip", "Roundtrip skill"); + let skill_source = source_root.join("roundtrip"); + let envs = [ + ( + "CLAW_CONFIG_HOME", + config_home.to_str().expect("utf8 config home"), + ), + ("HOME", home.to_str().expect("utf8 home")), + ("ANTHROPIC_API_KEY", ""), + ("ANTHROPIC_AUTH_TOKEN", ""), + ("OPENAI_API_KEY", ""), + ]; + + let install = run_claw( + &root, + &[ + "skills", + "install", + skill_source.to_str().expect("utf8 skill source"), + "--output-format", + "json", + ], + &envs, ); assert!( - !out2.status.success(), - "skills uninstall must exit non-zero (#795)" + install.status.success(), + "stdout:\n{}\n\nstderr:\n{}", + String::from_utf8_lossy(&install.stdout), + String::from_utf8_lossy(&install.stderr) ); - let stderr2 = String::from_utf8_lossy(&out2.stderr); - let stdout2 = String::from_utf8_lossy(&out2.stdout); - let j2: serde_json::Value = stdout2 - .lines() - .find(|l| l.trim_start().starts_with('{')) - .and_then(|l| serde_json::from_str(l).ok()) - .expect("skills uninstall should emit JSON error"); + let install_json = parse_json_stdout(&install, "skills install roundtrip"); + assert_eq!(install_json["kind"], "skills"); + assert_eq!(install_json["action"], "install"); + assert_eq!(install_json["status"], "ok"); + assert_eq!(install_json["invocation_name"], "roundtrip"); + let installed_path = config_home.join("skills").join("roundtrip"); assert_eq!( - j2["error_kind"], "unsupported_skills_action", - "skills uninstall should be unsupported_skills_action, got {:?}", - j2["error_kind"] + install_json["installed_path"], + installed_path.display().to_string() ); - let h2 = j2["hint"] - .as_str() - .expect("unsupported_skills_action must have non-null hint (#795)"); - assert!(!h2.is_empty(), "hint must be non-empty"); + assert!(installed_path.join("SKILL.md").is_file()); + + let uninstall = run_claw( + &root, + &[ + "skills", + "uninstall", + "roundtrip", + "--output-format", + "json", + ], + &envs, + ); + assert!( + uninstall.status.success(), + "stdout:\n{}\n\nstderr:\n{}", + String::from_utf8_lossy(&uninstall.stdout), + String::from_utf8_lossy(&uninstall.stderr) + ); + let uninstall_json = parse_json_stdout(&uninstall, "skills uninstall roundtrip"); + assert_eq!(uninstall_json["kind"], "skills"); + assert_eq!(uninstall_json["action"], "uninstall"); + assert_eq!(uninstall_json["status"], "ok"); + assert_eq!(uninstall_json["removed"], "roundtrip"); + assert_eq!( + uninstall_json["removed_path"], + installed_path.display().to_string() + ); + assert!( + !installed_path.exists(), + "uninstall should remove installed skill files" + ); +} + +#[test] +fn agents_create_scaffolds_toml_and_lists_locally_431() { + let root = unique_temp_dir("agents-create-431"); + let config_home = root.join("config-home"); + let home = root.join("home"); + fs::create_dir_all(&config_home).expect("config home"); + fs::create_dir_all(&home).expect("home"); + let envs = [ + ( + "CLAW_CONFIG_HOME", + config_home.to_str().expect("utf8 config home"), + ), + ("HOME", home.to_str().expect("utf8 home")), + ("ANTHROPIC_API_KEY", ""), + ("ANTHROPIC_AUTH_TOKEN", ""), + ("OPENAI_API_KEY", ""), + ]; + + let create = run_claw( + &root, + &["agents", "create", "my-agent", "--output-format", "json"], + &envs, + ); + assert!( + create.status.success(), + "stdout:\n{}\n\nstderr:\n{}", + String::from_utf8_lossy(&create.stdout), + String::from_utf8_lossy(&create.stderr) + ); + let create_json = parse_json_stdout(&create, "agents create my-agent"); + let agent_path = root.join(".claw").join("agents").join("my-agent.toml"); + let reported_agent_path = PathBuf::from( + create_json["path"] + .as_str() + .expect("agents create should report path"), + ); + assert_eq!(create_json["kind"], "agents"); + assert_eq!(create_json["action"], "create"); + assert_eq!(create_json["status"], "ok"); + assert_eq!(create_json["format"], "toml"); + assert_eq!( + reported_agent_path, + fs::canonicalize(&agent_path).expect("canonical agent path") + ); + assert!(agent_path.is_file()); + let agent_contents = fs::read_to_string(&agent_path).expect("agent scaffold should read"); + assert!(agent_contents.contains("name = \"my-agent\"")); + + let list = + assert_json_command_with_env(&root, &["--output-format", "json", "agents", "list"], &envs); + assert_eq!(list["kind"], "agents"); + assert_eq!(list["action"], "list"); + assert!(list["agents"] + .as_array() + .expect("agents array") + .iter() + .any(|agent| { + agent["name"] == "my-agent" + && PathBuf::from(agent["path"].as_str().expect("listed agent path")) + == fs::canonicalize(&agent_path).expect("canonical listed agent path") + })); } #[test]