From 74c3a178201d3c1685aacf5d765e01104115ded0 Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Wed, 16 Sep 2026 23:48:40 +0000 Subject: [PATCH 1/2] ucode: rename `skill` command to `skills` and `--skills` flag to `--skill` Renames the `ug skill` command group to `ug skills` (`ug skills add`, `ug skills remove`) and the download subset flag from `--skills` to `--skill` on both subcommands. This aligns the sibling commands: the group now matches `ug configure skills`, and the flag matches the existing `ug configure skills --skill`. Updates the CLI wiring, user facing help and error strings, docstrings, README, and tests. No behavior change beyond the renamed names. Co-authored-by: Isaac --- README.md | 54 +++++++++---------- src/ucode/cli.py | 36 ++++++------- src/ucode/mcp.py | 6 +-- src/ucode/skills_download.py | 2 +- tests/test_cli.py | 100 ++++++++++++++++++----------------- tests/test_mcp.py | 4 +- 6 files changed, 102 insertions(+), 100 deletions(-) diff --git a/README.md b/README.md index 606e2097..70278ff2 100644 --- a/README.md +++ b/README.md @@ -292,27 +292,27 @@ you to run `ug ` (existing agent sessions need a restart before the MCP t #### Add skill scopes without replacing existing ones -`ug skill add` registers skills additively, keeping anything already configured. With `--mcp` it +`ug skills add` registers skills additively, keeping anything already configured. With `--mcp` it adds the schemas to the connection's scope, otherwise it downloads their skills to disk. `--location` -downloads whole schemas; `--skills` downloads a named set of fully-qualified skills that may span +downloads whole schemas; `--skill` downloads a named set of fully-qualified skills that may span schemas. ```bash # Add schemas to the skills MCP scope, keeping any already configured. -ug skill add --location main.default,ml.prod --mcp +ug skills add --location main.default,ml.prod --mcp # Scope the schemas to specific agents. Any not set up yet are configured first. -ug skill add --location main.default --mcp --agents claude,codex +ug skills add --location main.default --mcp --agents claude,codex # Download a schema's skills to disk, keeping existing downloads. -ug skill add --location main.default +ug skills add --location main.default # Download a named set of skills by fully-qualified name (may span schemas). -ug skill add --skills main.default.my-skill,ml.prod.other-skill +ug skills add --skill main.default.my-skill,ml.prod.other-skill -# No --location (or --skills) launches an interactive picker of the workspace's +# No --location (or --skill) launches an interactive picker of the workspace's # skills to download; it opens immediately and streams skills in as they're found. -ug skill add +ug skills add ``` With `--mcp`, `--agents` limits the change to the named agents; without it the schemas go to every @@ -320,39 +320,39 @@ configured agent. It applies only to `--mcp`, since downloaded skills are shared #### Remove skill scopes -Remove schemas from the skills MCP connection with `ug skill remove --mcp`. `--location` drops the +Remove schemas from the skills MCP connection with `ug skills remove --mcp`. `--location` drops the named schemas; with no `--location` on an interactive terminal a picker lists the scoped schemas. ```bash # Remove specific schemas from the MCP scope; each is removed from every agent it's on. -ug skill remove --location main.default,ml.prod --mcp +ug skills remove --location main.default,ml.prod --mcp # Remove from specific agents only. A schema scoped to several agents is # removed from the named ones and kept on the rest. -ug skill remove --location main.default --mcp --agents claude +ug skills remove --location main.default --mcp --agents claude # No --location launches a picker of the scoped schemas to remove. -ug skill remove --mcp +ug skills remove --mcp ``` #### Remove downloaded skills -Without `--mcp`, `ug skill remove` deletes downloaded skill directories. Only skills +Without `--mcp`, `ug skills remove` deletes downloaded skill directories. Only skills `ug` downloaded are removed, so a same-named skill you authored is left alone. ```bash # Pick from every skill downloaded to disk, across all download bases. -ug skill remove +ug skills remove # Remove every skill downloaded from a schema (all bases, or one with --path). -ug skill remove --location main.default -ug skill remove --location main.default --path /abs/project/dir +ug skills remove --location main.default +ug skills remove --location main.default --path /abs/project/dir # Remove named skills by fully-qualified name (may span schemas). -ug skill remove --skills main.default.my-skill,ml.prod.other-skill +ug skills remove --skill main.default.my-skill,ml.prod.other-skill ``` -`--location` and `--skills` each accept `--path` to limit removal to one download base, and are +`--location` and `--skill` each accept `--path` to limit removal to one download base, and are mutually exclusive with each other. ### Exporting the config @@ -417,15 +417,15 @@ The output looks like: | `ug configure skills --location main.default [--path ]` | Download a schema's skills to disk (under ``, or your home dir) and register a schema-less skills MCP connection | | `ug configure skills --skill main.default.my-skill` | Download named skills by fully-qualified name (comma-separated; may span schemas) | | `ug configure skills --location main.default --mcp` | Expose a schema's skills as MCP tools (override-only) instead of downloading | -| `ug skill add --location main.default --mcp` | Add schemas to the skills MCP scope, keeping any already configured (additive; never replaces) | -| `ug skill add --location main.default --mcp --agents claude,codex` | Add schemas to specific agents' skills MCP scope (sets up any not yet configured) | -| `ug skill add --location main.default` | Download a schema's skills to disk without removing existing downloads | -| `ug skill add --skills main.default.my-skill` | Download named skills by fully-qualified name (comma-separated; may span schemas) | -| `ug skill remove --location main.default --mcp` | Remove specific schemas from the skills MCP scope, or omit `--location` on a TTY for a picker (every agent) | -| `ug skill remove --location main.default --mcp --agents claude` | Remove schemas from specific agents' skills MCP scope, keeping them on the rest | -| `ug skill remove` | Pick from every downloaded skill (across all bases) and delete it from disk | -| `ug skill remove --location main.default [--path ]` | Delete every skill downloaded from a schema (all bases, or one under ``) | -| `ug skill remove --skills main.default.my-skill [--path ]` | Delete named downloaded skills by fully-qualified name (comma-separated; may span schemas; `--path` limits to one base) | +| `ug skills add --location main.default --mcp` | Add schemas to the skills MCP scope, keeping any already configured (additive; never replaces) | +| `ug skills add --location main.default --mcp --agents claude,codex` | Add schemas to specific agents' skills MCP scope (sets up any not yet configured) | +| `ug skills add --location main.default` | Download a schema's skills to disk without removing existing downloads | +| `ug skills add --skill main.default.my-skill` | Download named skills by fully-qualified name (comma-separated; may span schemas) | +| `ug skills remove --location main.default --mcp` | Remove specific schemas from the skills MCP scope, or omit `--location` on a TTY for a picker (every agent) | +| `ug skills remove --location main.default --mcp --agents claude` | Remove schemas from specific agents' skills MCP scope, keeping them on the rest | +| `ug skills remove` | Pick from every downloaded skill (across all bases) and delete it from disk | +| `ug skills remove --location main.default [--path ]` | Delete every skill downloaded from a schema (all bases, or one under ``) | +| `ug skills remove --skill main.default.my-skill [--path ]` | Delete named downloaded skills by fully-qualified name (comma-separated; may span schemas; `--path` limits to one base) | Databricks AI Tools are installed only by `ug configure`, never by `ug ` launches. Use `--enable-databricks-ai-tools` or `--disable-databricks-ai-tools` with `ug configure` to diff --git a/src/ucode/cli.py b/src/ucode/cli.py index 8dc024e8..bd3bb3ba 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -986,7 +986,7 @@ def status() -> int: print_note( "Use `ug configure skills` to set up Unity Catalog Skills for configured coding tools." ) - print_note("Use `ug skill add` and `ug skill remove --mcp` to manage UC Skills.") + print_note("Use `ug skills add` and `ug skills remove --mcp` to manage UC Skills.") print_note("Use `ug revert` to clear managed configs and restore prior files.") return 0 @@ -1045,7 +1045,7 @@ def revert() -> int: mcp_app = typer.Typer(add_completion=False, no_args_is_help=True) app.add_typer(mcp_app, name="mcp", help="MCP servers exposed by ug.") skill_app = typer.Typer(add_completion=False, no_args_is_help=True) -app.add_typer(skill_app, name="skill", help="Databricks Skills for your coding tools.") +app.add_typer(skill_app, name="skills", help="Databricks Skills for your coding tools.") def _version_callback(value: bool) -> None: @@ -1058,7 +1058,7 @@ def _version_callback(value: bool) -> None: def _configure_agents_for_mcp(requested: list[str]) -> set[str]: """Ensure the named coding agents are set up (workspace + models) so a - subsequent `ug mcp add` / `ug skill add --mcp` has them as targets, and + subsequent `ug mcp add` / `ug skills add --mcp` has them as targets, and return the full canonical name set. Agents already configured are left as-is; only the rest are bootstrapped. Model agents go through configure_workspace_command (which installs binaries and configures models); @@ -1220,7 +1220,7 @@ def skills_add( skills: Annotated[ str | None, typer.Option( - "--skills", + "--skill", help="(download) Download exactly these comma-separated fully-qualified " "`..` skills, spanning any number of schemas. Not valid " "with --mcp or --location.", @@ -1241,9 +1241,9 @@ def skills_add( With ``--mcp``, adds the given schemas to the skills MCP connection's scope. Otherwise downloads skills to project-level skill directories under ``--path``, or to user-level skill directories when omitted, keeping already-downloaded skills. - ``--location`` downloads whole ``.`` schemas; ``--skills`` + ``--location`` downloads whole ``.`` schemas; ``--skill`` downloads a named set of fully-qualified skills that may span schemas (and takes - no ``--location``). With no ``--location``/``--skills`` on an interactive terminal, + no ``--location``). With no ``--location``/``--skill`` on an interactive terminal, opens a picker of the workspace's schemas to scope (``--mcp``) or skills to download. """ try: @@ -1259,9 +1259,9 @@ def skills_add( if mcp and path is not None: raise RuntimeError("--path is not supported when using --mcp") if mcp and requested_skills is not None: - raise RuntimeError("--skills is not supported when using --mcp") + raise RuntimeError("--skill is not supported when using --mcp") if requested_skills is not None and location is not None: - raise RuntimeError("--skills takes fully-qualified names; drop --location.") + raise RuntimeError("--skill takes fully-qualified names; drop --location.") # Downloaded skills use shared directory families, so only MCP scopes can be agent-scoped. if not mcp and agents is not None: raise RuntimeError("--agents is only supported when using --mcp") @@ -1269,7 +1269,7 @@ def skills_add( invalid = sorted(s for s in requested_skills if not _is_qualified_skill_name(s)) if invalid: raise RuntimeError( - "--skills entries must be fully-qualified `..` names " + "--skill entries must be fully-qualified `..` names " f"(invalid: {', '.join(invalid)})." ) configure_selected_skills_download_command(sorted(requested_skills), path) @@ -1287,7 +1287,7 @@ def skills_add( else: configure_skills_download_picker_command(path=path) return - raise RuntimeError("--location is required for `ucode skill add`.") + raise RuntimeError("--location is required for `ucode skills add`.") if mcp: configured_agents = ( _configure_agents_for_mcp(sorted(requested_agents)) if requested_agents else None @@ -1331,7 +1331,7 @@ def skills_remove( skills: Annotated[ str | None, typer.Option( - "--skills", + "--skill", help="(download) Remove exactly these comma-separated fully-qualified " "`..` skills, spanning any number of schemas. Not valid " "with --mcp or --location.", @@ -1352,7 +1352,7 @@ def skills_remove( With ``--mcp``, drops skill schemas from the skills MCP connection: ``--location`` removes the named ``.`` schemas, and with none on an interactive terminal a picker lists the scoped schemas. Otherwise removes downloaded skill directories: ``--location`` removes every - skill downloaded from a ``.``, ``--skills`` removes named fully-qualified skills + skill downloaded from a ``.``, ``--skill`` removes named fully-qualified skills that may span schemas, and with none of them a picker lists every downloaded skill. ``--path`` limits either to one download base. Only skills ucode downloaded are removed; a same-named skill you authored is left alone. @@ -1364,7 +1364,7 @@ def skills_remove( ) if mcp: if path is not None or requested_skills is not None: - raise RuntimeError("--path and --skills are not supported with --mcp.") + raise RuntimeError("--path and --skill are not supported with --mcp.") requested_agents = ( None if agents is None @@ -1376,26 +1376,26 @@ def skills_remove( elif _stdin_is_interactive(): remove_skills_command(agents=requested_agents) else: - raise RuntimeError("--location is required for `ug skill remove --mcp`.") + raise RuntimeError("--location is required for `ug skills remove --mcp`.") return if agents is not None: raise RuntimeError("--agents is only supported when using --mcp.") if requested_skills is not None and location is not None: - raise RuntimeError("--skills takes fully-qualified names; drop --location.") + raise RuntimeError("--skill takes fully-qualified names; drop --location.") if requested_skills is not None: invalid = sorted(s for s in requested_skills if not _is_qualified_skill_name(s)) if invalid: raise RuntimeError( - "--skills entries must be fully-qualified `..` names " + "--skill entries must be fully-qualified `..` names " f"(invalid: {', '.join(invalid)})." ) remove_downloaded_skills_command([], sorted(requested_skills), path=path) return locations = _parse_skill_locations(location) if path is not None and not locations: - raise RuntimeError("--path is only supported with --location or --skills.") + raise RuntimeError("--path is only supported with --location or --skill.") if not locations and not _stdin_is_interactive(): - raise RuntimeError("--location or --skills is required for `ug skill remove`.") + raise RuntimeError("--location or --skill is required for `ug skills remove`.") remove_downloaded_skills_command(locations, path=path) except (RuntimeError, ValueError) as exc: print_err(str(exc)) diff --git a/src/ucode/mcp.py b/src/ucode/mcp.py index 9771b583..15ad291a 100644 --- a/src/ucode/mcp.py +++ b/src/ucode/mcp.py @@ -2036,7 +2036,7 @@ def _skill_schema_choice(location: str, skill_count: int, in_scope: bool) -> que """Picker row for one schema: value is ``.``, title carries the skill count. An already-scoped schema is flagged and stays selectable; re-selecting it is a no-op, since - adding to the MCP scope is additive (removal is ``ug skill remove --mcp``). + adding to the MCP scope is additive (removal is ``ug skills remove --mcp``). """ noun = "skill" if skill_count == 1 else "skills" scope_flag = " (already in skill MCP)" if in_scope else "" @@ -2149,7 +2149,7 @@ def _removed_schemas_summary(count: int) -> str: def remove_skills_command(agents: set[str] | None = None) -> int: - """`ucode skill remove --mcp`: interactively drop skill schemas from clients' skills scopes. + """`ucode skills remove --mcp`: interactively drop skill schemas from clients' skills scopes. Shows the schemas in each targeted client's skills scope and removes the ones you select from those clients. Without ``agents`` a selected schema is removed from every configured client; @@ -2183,7 +2183,7 @@ def remove_skills_command(agents: set[str] | None = None) -> int: def remove_skills_locations_command(locations: list[str], agents: set[str] | None = None) -> int: - """`ucode skill remove --mcp --location`: drop the named schemas from clients' skills scopes. + """`ucode skills remove --mcp --location`: drop the named schemas from clients' skills scopes. Non-interactive counterpart to ``remove_skills_command``. ``agents`` (from ``--agents``) scopes removal to that subset of configured clients; omitting it targets every configured client. A diff --git a/src/ucode/skills_download.py b/src/ucode/skills_download.py index 27c56936..7633519e 100644 --- a/src/ucode/skills_download.py +++ b/src/ucode/skills_download.py @@ -483,7 +483,7 @@ def _prompt_for_downloaded_skill_removal(records: list[dict]) -> list[dict] | No def remove_downloaded_skills_command( locations: list[str], fqns: list[str] | None = None, *, path: str | None ) -> int: - """`ug skill remove` (download side): delete downloaded skills and forget them. + """`ug skills remove` (download side): delete downloaded skills and forget them. With ``fqns``, removes those fully-qualified skills; with ``locations``, every skill downloaded from those ``.`` schemas; with neither, opens a picker over diff --git a/tests/test_cli.py b/tests/test_cli.py index cbb5afd2..815afa72 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1563,7 +1563,7 @@ def test_path_without_location_exit_1(self): class TestSkillsAddCommand: - """`ucode skill add` is the additive sibling of `configure skills`: `--mcp` + """`ucode skills add` is the additive sibling of `configure skills`: `--mcp` unions schemas into the connection scope, the default mode downloads.""" @pytest.fixture(autouse=True) @@ -1575,51 +1575,51 @@ def test_requires_skills_mcp_cli_floor(self, _stub_install_cli): from ucode.databricks import SKILLS_MCP_MIN_DATABRICKS_CLI_VERSION with patch("ucode.cli.add_skills_command"): - result = runner.invoke(app, ["skill", "add", "--location", "a.b", "--mcp"]) + result = runner.invoke(app, ["skills", "add", "--location", "a.b", "--mcp"]) assert result.exit_code == 0, result.output _stub_install_cli.assert_called_once_with(minimum=SKILLS_MCP_MIN_DATABRICKS_CLI_VERSION) def test_mcp_flag_unions_locations(self): with patch("ucode.cli.add_skills_command") as mock_add: - result = runner.invoke(app, ["skill", "add", "--location", "a.b", "--mcp"]) + result = runner.invoke(app, ["skills", "add", "--location", "a.b", "--mcp"]) assert result.exit_code == 0, result.output mock_add.assert_called_once_with(["a.b"], agents=None) def test_comma_location_yields_multiple_schemas(self): with patch("ucode.cli.add_skills_command") as mock_add: - result = runner.invoke(app, ["skill", "add", "--location", "a.b, c.d", "--mcp"]) + result = runner.invoke(app, ["skills", "add", "--location", "a.b, c.d", "--mcp"]) assert result.exit_code == 0, result.output mock_add.assert_called_once_with(["a.b", "c.d"], agents=None) def test_default_mode_dispatches_download(self): with patch("ucode.cli.configure_location_skills_download_command") as mock_download: - result = runner.invoke(app, ["skill", "add", "--location", "a.b", "--path", "/tmp/s"]) + result = runner.invoke(app, ["skills", "add", "--location", "a.b", "--path", "/tmp/s"]) assert result.exit_code == 0, result.output mock_download.assert_called_once_with(["a.b"], path="/tmp/s") def test_skills_download_fully_qualified_across_schemas(self): with patch("ucode.cli.configure_selected_skills_download_command") as mock_download: - result = runner.invoke(app, ["skill", "add", "--skills", "a.b.s1, c.d.s2"]) + result = runner.invoke(app, ["skills", "add", "--skill", "a.b.s1, c.d.s2"]) assert result.exit_code == 0, result.output mock_download.assert_called_once_with(["a.b.s1", "c.d.s2"], None) def test_skills_thread_path_through(self): with patch("ucode.cli.configure_selected_skills_download_command") as mock_download: - result = runner.invoke(app, ["skill", "add", "--skills", "a.b.s1", "--path", "/tmp/s"]) + result = runner.invoke(app, ["skills", "add", "--skill", "a.b.s1", "--path", "/tmp/s"]) assert result.exit_code == 0, result.output mock_download.assert_called_once_with(["a.b.s1"], "/tmp/s") def test_skills_with_location_exit_1(self): with patch("ucode.cli.configure_selected_skills_download_command") as mock_download: - result = runner.invoke(app, ["skill", "add", "--location", "a.b", "--skills", "a.b.s1"]) + result = runner.invoke(app, ["skills", "add", "--location", "a.b", "--skill", "a.b.s1"]) assert result.exit_code == 1 - assert "--skills takes fully-qualified names; drop --location" in _strip_ansi(result.output) + assert "--skill takes fully-qualified names; drop --location" in _strip_ansi(result.output) mock_download.assert_not_called() @pytest.mark.parametrize("skill", ["a.b", "a..s1", "a.b.c.d", "leaf"]) def test_non_fully_qualified_skill_exit_1(self, skill): with patch("ucode.cli.configure_selected_skills_download_command") as mock_download: - result = runner.invoke(app, ["skill", "add", "--skills", skill]) + result = runner.invoke(app, ["skills", "add", "--skill", skill]) assert result.exit_code == 1 assert "must be fully-qualified" in _strip_ansi(result.output) mock_download.assert_not_called() @@ -1631,7 +1631,7 @@ def test_without_location_non_interactive_exit_1(self): patch("ucode.cli.configure_location_skills_download_command") as mock_download, patch("ucode.cli.configure_skills_download_picker_command") as mock_picker, ): - result = runner.invoke(app, ["skill", "add"]) + result = runner.invoke(app, ["skills", "add"]) assert result.exit_code == 1 assert "--location is required" in _strip_ansi(result.output) mock_add.assert_not_called() @@ -1643,7 +1643,7 @@ def test_no_args_interactive_opens_picker(self): patch("ucode.cli._stdin_is_interactive", return_value=True), patch("ucode.cli.configure_skills_download_picker_command") as mock_picker, ): - result = runner.invoke(app, ["skill", "add"]) + result = runner.invoke(app, ["skills", "add"]) assert result.exit_code == 0, result.output mock_picker.assert_called_once_with(path=None) @@ -1652,7 +1652,7 @@ def test_interactive_picker_passes_path(self): patch("ucode.cli._stdin_is_interactive", return_value=True), patch("ucode.cli.configure_skills_download_picker_command") as mock_picker, ): - result = runner.invoke(app, ["skill", "add", "--path", "/tmp/s"]) + result = runner.invoke(app, ["skills", "add", "--path", "/tmp/s"]) assert result.exit_code == 0, result.output mock_picker.assert_called_once_with(path="/tmp/s") @@ -1662,7 +1662,7 @@ def test_mcp_no_location_interactive_opens_schema_picker(self): patch("ucode.cli.configure_skills_mcp_picker_command") as mock_picker, patch("ucode.cli.configure_skills_download_picker_command") as mock_download, ): - result = runner.invoke(app, ["skill", "add", "--mcp"]) + result = runner.invoke(app, ["skills", "add", "--mcp"]) assert result.exit_code == 0, result.output mock_picker.assert_called_once_with(agents=None) mock_download.assert_not_called() @@ -1672,7 +1672,7 @@ def test_mcp_no_location_non_interactive_exit_1(self): patch("ucode.cli._stdin_is_interactive", return_value=False), patch("ucode.cli.configure_skills_mcp_picker_command") as mock_picker, ): - result = runner.invoke(app, ["skill", "add", "--mcp"]) + result = runner.invoke(app, ["skills", "add", "--mcp"]) assert result.exit_code == 1 assert "--location is required" in _strip_ansi(result.output) mock_picker.assert_not_called() @@ -1685,7 +1685,7 @@ def test_mcp_picker_with_agents_forwards_scope(self): ) as configure, patch("ucode.cli.configure_skills_mcp_picker_command") as mock_picker, ): - result = runner.invoke(app, ["skill", "add", "--mcp", "--agents", "codex,claude"]) + result = runner.invoke(app, ["skills", "add", "--mcp", "--agents", "codex,claude"]) assert result.exit_code == 0, result.output configure.assert_called_once_with(["claude", "codex"]) mock_picker.assert_called_once_with(agents={"claude", "codex"}) @@ -1696,7 +1696,7 @@ def test_skills_bypass_picker_even_when_interactive(self): patch("ucode.cli.configure_skills_download_picker_command") as mock_picker, patch("ucode.cli.configure_selected_skills_download_command") as mock_download, ): - result = runner.invoke(app, ["skill", "add", "--skills", "a.b.s1"]) + result = runner.invoke(app, ["skills", "add", "--skill", "a.b.s1"]) assert result.exit_code == 0, result.output mock_picker.assert_not_called() mock_download.assert_called_once_with(["a.b.s1"], None) @@ -1706,16 +1706,16 @@ def test_skill_with_mcp_exit_1(self): patch("ucode.cli.add_skills_command") as mock_add, patch("ucode.cli.configure_selected_skills_download_command") as mock_download, ): - result = runner.invoke(app, ["skill", "add", "--mcp", "--skills", "a.b.s1"]) + result = runner.invoke(app, ["skills", "add", "--mcp", "--skill", "a.b.s1"]) assert result.exit_code == 1 - assert "--skills" in _strip_ansi(result.output) + assert "--skill" in _strip_ansi(result.output) mock_add.assert_not_called() mock_download.assert_not_called() def test_path_with_mcp_exit_1(self): with patch("ucode.cli.add_skills_command") as mock_add: result = runner.invoke( - app, ["skill", "add", "--location", "a.b", "--mcp", "--path", "/tmp/s"] + app, ["skills", "add", "--location", "a.b", "--mcp", "--path", "/tmp/s"] ) assert result.exit_code == 1 assert "--path" in _strip_ansi(result.output) @@ -1723,7 +1723,7 @@ def test_path_with_mcp_exit_1(self): def test_malformed_location_exit_1(self): with patch("ucode.cli.add_skills_command") as mock_add: - result = runner.invoke(app, ["skill", "add", "--location", "a.b.c", "--mcp"]) + result = runner.invoke(app, ["skills", "add", "--location", "a.b.c", "--mcp"]) assert result.exit_code == 1 assert "--location" in _strip_ansi(result.output) mock_add.assert_not_called() @@ -1737,7 +1737,7 @@ def test_agents_scope_delegates_to_helper_and_forwards_returned_scope(self): ): result = runner.invoke( app, - ["skill", "add", "--location", "a.b", "--mcp", "--agents", "codex,claude"], + ["skills", "add", "--location", "a.b", "--mcp", "--agents", "codex,claude"], ) assert result.exit_code == 0, result.output @@ -1751,7 +1751,7 @@ def test_empty_agents_folds_to_global_scope(self): ): result = runner.invoke( app, - ["skill", "add", "--location", "a.b", "--mcp", "--agents", ","], + ["skills", "add", "--location", "a.b", "--mcp", "--agents", ","], ) assert result.exit_code == 0, result.output @@ -1760,7 +1760,9 @@ def test_empty_agents_folds_to_global_scope(self): def test_agents_is_rejected_for_download_mode(self): with patch("ucode.cli.configure_location_skills_download_command") as mock_download: - result = runner.invoke(app, ["skill", "add", "--location", "a.b", "--agents", "claude"]) + result = runner.invoke( + app, ["skills", "add", "--location", "a.b", "--agents", "claude"] + ) assert result.exit_code == 1 assert "--agents is only supported when using --mcp" in _strip_ansi(result.output) @@ -1794,7 +1796,7 @@ def test_all_configured_skips_bootstrap(self): class TestSkillsRemoveCommand: - """`ug skill remove`: `--mcp` drops MCP scopes, the default mode deletes downloads.""" + """`ug skills remove`: `--mcp` drops MCP scopes, the default mode deletes downloads.""" @pytest.fixture(autouse=True) def _stub_install_cli(self): @@ -1805,7 +1807,7 @@ def test_requires_skills_mcp_cli_floor(self, _stub_install_cli): from ucode.databricks import SKILLS_MCP_MIN_DATABRICKS_CLI_VERSION with patch("ucode.cli.remove_downloaded_skills_command"): - result = runner.invoke(app, ["skill", "remove", "--location", "a.b"]) + result = runner.invoke(app, ["skills", "remove", "--location", "a.b"]) assert result.exit_code == 0, result.output _stub_install_cli.assert_called_once_with(minimum=SKILLS_MCP_MIN_DATABRICKS_CLI_VERSION) @@ -1814,7 +1816,7 @@ def test_mcp_remove_no_location_interactive_opens_picker(self): patch("ucode.cli._stdin_is_interactive", return_value=True), patch("ucode.cli.remove_skills_command") as remove, ): - result = runner.invoke(app, ["skill", "remove", "--mcp"]) + result = runner.invoke(app, ["skills", "remove", "--mcp"]) assert result.exit_code == 0, result.output remove.assert_called_once_with(agents=None) @@ -1824,48 +1826,48 @@ def test_mcp_remove_forwards_agent_scope(self): patch("ucode.cli._stdin_is_interactive", return_value=True), patch("ucode.cli.remove_skills_command") as remove, ): - result = runner.invoke(app, ["skill", "remove", "--mcp", "--agents", "claude, codex"]) + result = runner.invoke(app, ["skills", "remove", "--mcp", "--agents", "claude, codex"]) assert result.exit_code == 0, result.output remove.assert_called_once_with(agents={"claude", "codex"}) def test_location_routes_to_download_remove(self): with patch("ucode.cli.remove_downloaded_skills_command") as mock_remove: - result = runner.invoke(app, ["skill", "remove", "--location", "a.b, c.d"]) + result = runner.invoke(app, ["skills", "remove", "--location", "a.b, c.d"]) assert result.exit_code == 0, result.output mock_remove.assert_called_once_with(["a.b", "c.d"], path=None) def test_location_with_path_narrows_base(self): with patch("ucode.cli.remove_downloaded_skills_command") as mock_remove: - result = runner.invoke(app, ["skill", "remove", "--location", "a.b", "--path", "/abs"]) + result = runner.invoke(app, ["skills", "remove", "--location", "a.b", "--path", "/abs"]) assert result.exit_code == 0, result.output mock_remove.assert_called_once_with(["a.b"], path="/abs") def test_skills_routes_to_download_remove_by_name(self): with patch("ucode.cli.remove_downloaded_skills_command") as mock_remove: - result = runner.invoke(app, ["skill", "remove", "--skills", "a.b.s1, c.d.s2"]) + result = runner.invoke(app, ["skills", "remove", "--skill", "a.b.s1, c.d.s2"]) assert result.exit_code == 0, result.output mock_remove.assert_called_once_with([], ["a.b.s1", "c.d.s2"], path=None) def test_skills_with_path(self): with patch("ucode.cli.remove_downloaded_skills_command") as mock_remove: - result = runner.invoke(app, ["skill", "remove", "--skills", "a.b.s1", "--path", "/abs"]) + result = runner.invoke(app, ["skills", "remove", "--skill", "a.b.s1", "--path", "/abs"]) assert result.exit_code == 0, result.output mock_remove.assert_called_once_with([], ["a.b.s1"], path="/abs") def test_skills_with_location_exit_1(self): with patch("ucode.cli.remove_downloaded_skills_command") as mock_remove: result = runner.invoke( - app, ["skill", "remove", "--skills", "a.b.s1", "--location", "a.b"] + app, ["skills", "remove", "--skill", "a.b.s1", "--location", "a.b"] ) assert result.exit_code == 1 - assert "--skills takes fully-qualified names; drop --location" in _strip_ansi(result.output) + assert "--skill takes fully-qualified names; drop --location" in _strip_ansi(result.output) mock_remove.assert_not_called() @pytest.mark.parametrize("skill", ["a.b", "a..s1", "a.b.c.d", "leaf"]) def test_non_fully_qualified_skill_exit_1(self, skill): with patch("ucode.cli.remove_downloaded_skills_command") as mock_remove: - result = runner.invoke(app, ["skill", "remove", "--skills", skill]) + result = runner.invoke(app, ["skills", "remove", "--skill", skill]) assert result.exit_code == 1 assert "must be fully-qualified" in _strip_ansi(result.output) mock_remove.assert_not_called() @@ -1875,7 +1877,7 @@ def test_no_args_interactive_opens_picker(self): patch("ucode.cli._stdin_is_interactive", return_value=True), patch("ucode.cli.remove_downloaded_skills_command") as mock_remove, ): - result = runner.invoke(app, ["skill", "remove"]) + result = runner.invoke(app, ["skills", "remove"]) assert result.exit_code == 0, result.output mock_remove.assert_called_once_with([], path=None) @@ -1884,28 +1886,28 @@ def test_no_args_non_interactive_exit_1(self): patch("ucode.cli._stdin_is_interactive", return_value=False), patch("ucode.cli.remove_downloaded_skills_command") as mock_remove, ): - result = runner.invoke(app, ["skill", "remove"]) + result = runner.invoke(app, ["skills", "remove"]) assert result.exit_code == 1 - assert "--location or --skills is required" in _strip_ansi(result.output) + assert "--location or --skill is required" in _strip_ansi(result.output) mock_remove.assert_not_called() def test_path_without_location_exit_1(self): with patch("ucode.cli.remove_downloaded_skills_command") as mock_remove: - result = runner.invoke(app, ["skill", "remove", "--path", "/abs"]) + result = runner.invoke(app, ["skills", "remove", "--path", "/abs"]) assert result.exit_code == 1 - assert "--path is only supported with --location or --skills" in _strip_ansi(result.output) + assert "--path is only supported with --location or --skill" in _strip_ansi(result.output) mock_remove.assert_not_called() def test_agents_without_mcp_exit_1(self): with patch("ucode.cli.remove_downloaded_skills_command") as mock_remove: - result = runner.invoke(app, ["skill", "remove", "--agents", "claude"]) + result = runner.invoke(app, ["skills", "remove", "--agents", "claude"]) assert result.exit_code == 1 assert "--agents is only supported when using --mcp" in _strip_ansi(result.output) mock_remove.assert_not_called() def test_mcp_with_location_routes_to_location_removal(self): with patch("ucode.cli.remove_skills_locations_command") as remove: - result = runner.invoke(app, ["skill", "remove", "--mcp", "--location", "a.b, c.d"]) + result = runner.invoke(app, ["skills", "remove", "--mcp", "--location", "a.b, c.d"]) assert result.exit_code == 0, result.output remove.assert_called_once_with(["a.b", "c.d"], agents=None) @@ -1913,7 +1915,7 @@ def test_mcp_with_location_forwards_agent_scope(self): with patch("ucode.cli.remove_skills_locations_command") as remove: result = runner.invoke( app, - ["skill", "remove", "--mcp", "--location", "a.b", "--agents", "claude, codex"], + ["skills", "remove", "--mcp", "--location", "a.b", "--agents", "claude, codex"], ) assert result.exit_code == 0, result.output remove.assert_called_once_with(["a.b"], agents={"claude", "codex"}) @@ -1924,7 +1926,7 @@ def test_mcp_no_location_non_interactive_exit_1(self): patch("ucode.cli.remove_skills_command") as remove, patch("ucode.cli.remove_skills_locations_command") as remove_locations, ): - result = runner.invoke(app, ["skill", "remove", "--mcp"]) + result = runner.invoke(app, ["skills", "remove", "--mcp"]) assert result.exit_code == 1 assert "--location is required" in _strip_ansi(result.output) remove.assert_not_called() @@ -1932,29 +1934,29 @@ def test_mcp_no_location_non_interactive_exit_1(self): def test_mcp_malformed_location_exit_1(self): with patch("ucode.cli.remove_skills_locations_command") as remove: - result = runner.invoke(app, ["skill", "remove", "--mcp", "--location", "a.b.c"]) + result = runner.invoke(app, ["skills", "remove", "--mcp", "--location", "a.b.c"]) assert result.exit_code == 1 assert "--location" in _strip_ansi(result.output) remove.assert_not_called() def test_mcp_with_path_exit_1(self): with patch("ucode.cli.remove_skills_locations_command") as remove: - result = runner.invoke(app, ["skill", "remove", "--mcp", "--path", "/abs"]) + result = runner.invoke(app, ["skills", "remove", "--mcp", "--path", "/abs"]) assert result.exit_code == 1 assert "--path" in _strip_ansi(result.output) remove.assert_not_called() def test_mcp_with_skills_exit_1(self): with patch("ucode.cli.remove_skills_locations_command") as remove: - result = runner.invoke(app, ["skill", "remove", "--mcp", "--skills", "a.b.s1"]) + result = runner.invoke(app, ["skills", "remove", "--mcp", "--skill", "a.b.s1"]) assert result.exit_code == 1 - assert "--skills" in _strip_ansi(result.output) + assert "--skill" in _strip_ansi(result.output) remove.assert_not_called() class TestManagedSkillsOnLaunch: """Managed skills are delivered by download only: the launch path downloads them and never - registers them on the skills MCP connection (only a developer's own `skill add --mcp` schemas + registers them on the skills MCP connection (only a developer's own `skills add --mcp` schemas live there).""" def _state(self): diff --git a/tests/test_mcp.py b/tests/test_mcp.py index c163e930..033d3bb6 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -2184,7 +2184,7 @@ def test_drops_duplicate_new_locations(self): class TestAddSkillsCommand: - """`ucode skill add --mcp` unions schemas into the connection scope rather + """`ucode skills add --mcp` unions schemas into the connection scope rather than replacing it (unlike `configure_skills_mcp_command`).""" def test_unions_into_existing_scope(self, monkeypatch): @@ -2530,7 +2530,7 @@ def test_agent_scope_with_empty_scope_is_a_noop(self, monkeypatch): class TestRemoveSkillsLocationsCommand: - """`ug skill remove --mcp --location`: non-interactive schema removal from the skills scope.""" + """`ug skills remove --mcp --location`: non-interactive schema removal from the skills scope.""" def _state(self, by_client=None): by_client = by_client or _by_client(["claude", "codex"], ["A.a", "B.b"]) From 3e4034127cac8e5ac2ac2ce7f272b17f513fc441 Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Wed, 16 Sep 2026 23:59:54 +0000 Subject: [PATCH 2/2] ucode: document `--path` for `ug skills add` in README `--path` was documented for `ug configure skills` and `ug skills remove` but not for `ug skills add`, though it is a valid download destination flag there. Adds a matching example and extends the quick-reference row. Co-authored-by: Isaac --- README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 70278ff2..fb1a7abc 100644 --- a/README.md +++ b/README.md @@ -307,6 +307,9 @@ ug skills add --location main.default --mcp --agents claude,codex # Download a schema's skills to disk, keeping existing downloads. ug skills add --location main.default +# Download into a specific project directory instead of your home dir. +ug skills add --location main.default --path /abs/project/dir + # Download a named set of skills by fully-qualified name (may span schemas). ug skills add --skill main.default.my-skill,ml.prod.other-skill @@ -419,7 +422,7 @@ The output looks like: | `ug configure skills --location main.default --mcp` | Expose a schema's skills as MCP tools (override-only) instead of downloading | | `ug skills add --location main.default --mcp` | Add schemas to the skills MCP scope, keeping any already configured (additive; never replaces) | | `ug skills add --location main.default --mcp --agents claude,codex` | Add schemas to specific agents' skills MCP scope (sets up any not yet configured) | -| `ug skills add --location main.default` | Download a schema's skills to disk without removing existing downloads | +| `ug skills add --location main.default [--path ]` | Download a schema's skills to disk (under ``, or your home dir) without removing existing downloads | | `ug skills add --skill main.default.my-skill` | Download named skills by fully-qualified name (comma-separated; may span schemas) | | `ug skills remove --location main.default --mcp` | Remove specific schemas from the skills MCP scope, or omit `--location` on a TTY for a picker (every agent) | | `ug skills remove --location main.default --mcp --agents claude` | Remove schemas from specific agents' skills MCP scope, keeping them on the rest |