[skills] Support ug skill remove --mcp --location

`ug skill remove --mcp` was interactive-only; the non-interactive counterpart to
`ug skill add --mcp --location` was missing. Passing `--location` now drops the named
`<catalog>.<schema>` schemas from the skills MCP scope without a picker, and `--mcp`
with no `--location` on a non-interactive terminal errors asking for it (matching the
add path) instead of falling into an unusable picker.

The removal core moves into `remove_skill_locations_from_mcp`, shared by the interactive
`remove_skills_command` and the new `remove_skills_locations_command`, mirroring the
`add_skill_locations_to_mcp` split. `--agents` scopes removal to the named clients as
before; `--path` and `--skills` stay rejected with `--mcp`.

Co-authored-by: Isaac <no-reply@databricks.com>

Author: xsh310

README.md

@@ -279,15 +279,19 @@ 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`:
+Remove schemas from the skills MCP connection with `ug skill remove --mcp`. `--location` drops the
+named schemas; with no `--location` on an interactive terminal a picker lists the scoped schemas.
 
 ```bash
-# Pick schemas to remove; each is removed from every agent it's on.
-ug skill remove --mcp
+# 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
 
 # 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 --mcp --agents claude
+ug skill remove --location main.default --mcp --agents claude
+
+# No --location launches a picker of the scoped schemas to remove.
+ug skill remove --mcp
 ```
 
 #### Remove downloaded skills
@@ -374,8 +378,8 @@ The output looks like:
 | `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 --mcp` | Remove skill schemas from the skills MCP connection (every agent) |
-| `ug skill remove --mcp --agents claude` | Remove skill schemas from specific agents only, keeping them on the rest |
+| `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 <dir>]` | Delete every skill downloaded from a schema (all bases, or one under `<dir>`) |
 | `ug skill remove --skills main.default.my-skill [--path <dir>]` | Delete named downloaded skills by fully-qualified name (comma-separated; may span schemas; `--path` limits to one base) |

src/ucode/cli.py

@@ -103,6 +103,7 @@
     purge_cross_workspace_mcp_residue,
     remove_mcp_command,
     remove_skills_command,
+    remove_skills_locations_command,
     revert_mcp_configs,
     skill_locations_for_client,
 )
@@ -1311,8 +1312,8 @@ def skills_remove(
         str | None,
         typer.Option(
             "--location",
-            help="(download) Comma-separated `<catalog>.<schema>` schemas whose downloaded "
-            "skills to remove.",
+            help="Comma-separated `<catalog>.<schema>` schemas to remove (from the skills MCP "
+            "scope with --mcp, else their downloaded skills).",
         ),
     ] = None,
     mcp: Annotated[
@@ -1351,26 +1352,33 @@ def skills_remove(
 ) -> None:
     """Remove Skills previously added to your coding tools.
 
-    With ``--mcp``, interactively drops skill schemas from the skills MCP connection.
-    Otherwise removes downloaded skill directories: ``--location`` removes every skill
-    downloaded from a ``<catalog>.<schema>``, ``--skills`` 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.
+    With ``--mcp``, drops skill schemas from the skills MCP connection: ``--location`` removes the
+    named ``<catalog>.<schema>`` 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 ``<catalog>.<schema>``, ``--skills`` 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.
     """
     try:
         requested_skills = (
             None if skills is None else {s.strip() for s in skills.split(",") if s.strip()}
         )
         if mcp:
-            if location is not None or path is not None or requested_skills is not None:
-                raise RuntimeError("--location, --path, and --skills are not supported with --mcp.")
+            if path is not None or requested_skills is not None:
+                raise RuntimeError("--path and --skills are not supported with --mcp.")
             requested_agents = (
                 None
                 if agents is None
                 else ({a.strip().lower() for a in agents.split(",") if a.strip()} or None)
             )
-            remove_skills_command(agents=requested_agents)
+            locations = _parse_skill_locations(location)
+            if locations:
+                remove_skills_locations_command(locations, agents=requested_agents)
+            elif _stdin_is_interactive():
+                remove_skills_command(agents=requested_agents)
+            else:
+                raise RuntimeError("--location is required for `ug skill remove --mcp`.")
             return
         if agents is not None:
             raise RuntimeError("--agents is only supported when using --mcp.")

src/ucode/mcp.py

@@ -1990,6 +1990,33 @@ def add_skill_locations_to_mcp(
     _update_skills_mcp(state, workspace, profile, clients, locations_by_client)
 
 
+def remove_skill_locations_from_mcp(
+    state: dict,
+    workspace: str,
+    profile: str | None,
+    clients: list[str],
+    locations: set[str],
+) -> list[str]:
+    """Drop ``locations`` from each targeted client's skill MCP scope, returning the schemas removed."""
+    locations_by_client = _skill_locations_by_client_from_state(state)
+    removed = sorted(
+        {
+            location
+            for client in clients
+            for location in locations_by_client.get(client, [])
+            if location in locations
+        }
+    )
+    for client in clients:
+        locations_by_client[client] = [
+            location
+            for location in locations_by_client.get(client, [])
+            if location not in locations
+        ]
+    _update_skills_mcp(state, workspace, profile, clients, locations_by_client, print_summary=False)
+    return removed
+
+
 def configured_skill_locations(state: dict, clients: list[str]) -> set[str]:
     """The union of skill schemas already in the MCP scope across ``clients``."""
     locations_by_client = _skill_locations_by_client_from_state(state)
@@ -2113,6 +2140,10 @@ def _prompt_for_skill_removal(locations_by_client: dict[str, list[str]]) -> list
     return [str(value) for value in selection]
 
 
+def _removed_schemas_summary(count: int) -> str:
+    return f"Removed {count} skill schema{'s' if count != 1 else ''}."
+
+
 def remove_skills_command(agents: set[str] | None = None) -> int:
     """`ucode skill remove --mcp`: interactively drop skill schemas from clients' skills scopes.
 
@@ -2142,15 +2173,28 @@ def remove_skills_command(agents: set[str] | None = None) -> int:
         print_note("No skill schemas selected.")
         return 0
 
-    remove_locations = set(selection)
-    for client in clients:
-        locations_by_client[client] = [
-            location
-            for location in locations_by_client.get(client, [])
-            if location not in remove_locations
-        ]
-    _update_skills_mcp(state, workspace, profile, clients, locations_by_client, print_summary=False)
-    print_success(
-        f"Removed {len(remove_locations)} skill schema{'s' if len(remove_locations) != 1 else ''}."
+    removed = remove_skill_locations_from_mcp(state, workspace, profile, clients, set(selection))
+    print_success(_removed_schemas_summary(len(removed)))
+    return 0
+
+
+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.
+
+    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
+    schema not in scope is a no-op. Needs no Databricks auth."""
+    state = load_state()
+    workspace, profile, clients = setup_mcp_clients(
+        state,
+        "Remove Skills MCP",
+        require_auth=False,
+        action_note="Removing from",
+        agents=agents,
     )
+    removed = remove_skill_locations_from_mcp(state, workspace, profile, clients, set(locations))
+    if removed:
+        print_success(_removed_schemas_summary(len(removed)))
+    else:
+        print_note("None of the given schemas were in the skills MCP scope.")
     return 0

tests/test_cli.py

@@ -1727,15 +1727,21 @@ def test_all_configured_skips_bootstrap(self):
 class TestSkillsRemoveCommand:
     """`ug skill remove`: `--mcp` drops MCP scopes, the default mode deletes downloads."""
 
-    def test_mcp_remove_dispatches_global_removal(self):
-        with patch("ucode.cli.remove_skills_command") as remove:
+    def test_mcp_remove_no_location_interactive_opens_picker(self):
+        with (
+            patch("ucode.cli._stdin_is_interactive", return_value=True),
+            patch("ucode.cli.remove_skills_command") as remove,
+        ):
             result = runner.invoke(app, ["skill", "remove", "--mcp"])
 
         assert result.exit_code == 0, result.output
         remove.assert_called_once_with(agents=None)
 
     def test_mcp_remove_forwards_agent_scope(self):
-        with patch("ucode.cli.remove_skills_command") as remove:
+        with (
+            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"])
 
         assert result.exit_code == 0, result.output
@@ -1815,11 +1821,52 @@ def test_agents_without_mcp_exit_1(self):
         assert "--agents is only supported when using --mcp" in _strip_ansi(result.output)
         mock_remove.assert_not_called()
 
-    def test_mcp_with_location_exit_1(self):
-        with patch("ucode.cli.remove_skills_command") as remove:
-            result = runner.invoke(app, ["skill", "remove", "--mcp", "--location", "a.b"])
+    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"])
+        assert result.exit_code == 0, result.output
+        remove.assert_called_once_with(["a.b", "c.d"], agents=None)
+
+    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"],
+            )
+        assert result.exit_code == 0, result.output
+        remove.assert_called_once_with(["a.b"], agents={"claude", "codex"})
+
+    def test_mcp_no_location_non_interactive_exit_1(self):
+        with (
+            patch("ucode.cli._stdin_is_interactive", return_value=False),
+            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"])
+        assert result.exit_code == 1
+        assert "--location is required" in _strip_ansi(result.output)
+        remove.assert_not_called()
+        remove_locations.assert_not_called()
+
+    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"])
         assert result.exit_code == 1
-        assert "not supported with --mcp" in _strip_ansi(result.output)
+        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"])
+        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"])
+        assert result.exit_code == 1
+        assert "--skills" in _strip_ansi(result.output)
         remove.assert_not_called()
 
 

tests/test_mcp.py

@@ -2512,6 +2512,78 @@ def test_agent_scope_with_empty_scope_is_a_noop(self, monkeypatch):
         assert "called" not in captured
 
 
+class TestRemoveSkillsLocationsCommand:
+    """`ug skill 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"])
+        return {
+            "workspace": WS,
+            "available_tools": ["claude", "codex"],
+            "mcp_servers": mcp._resolve_skills_mcp_servers(WS, list(by_client), by_client, []),
+        }
+
+    def _stub(self, monkeypatch, state):
+        configured: list[tuple[str, str]] = []
+        _stub_location_base(monkeypatch, state)
+        monkeypatch.setattr(mcp, "available_mcp_clients", lambda: ["claude", "codex"])
+        monkeypatch.setattr(
+            mcp,
+            "configure_client_mcp_server",
+            lambda client, name, url, *a, **kw: configured.append((client, url)) or [],
+        )
+        monkeypatch.setattr(mcp, "save_state", lambda s: None)
+        return configured
+
+    def test_removes_named_schema_from_every_client(self, monkeypatch):
+        state = self._state()
+        configured = self._stub(monkeypatch, state)
+
+        assert mcp.remove_skills_locations_command(["A.a"]) == 0
+
+        entry = _find_skills(state["mcp_servers"])[0]
+        assert mcp.skill_locations_for_client(entry, "claude") == ["B.b"]
+        assert mcp.skill_locations_for_client(entry, "codex") == ["B.b"]
+        assert sorted(configured) == [
+            ("claude", f"{WS}/ai-gateway/skills/?schema=B.b"),
+            ("codex", f"{WS}/ai-gateway/skills/?schema=B.b"),
+        ]
+
+    def test_schema_not_in_scope_is_a_noop(self, monkeypatch):
+        state = self._state()
+        configured = self._stub(monkeypatch, state)
+
+        assert mcp.remove_skills_locations_command(["Z.z"]) == 0
+
+        entry = _find_skills(state["mcp_servers"])[0]
+        assert mcp.skill_locations_for_client(entry, "claude") == ["A.a", "B.b"]
+        assert configured == []
+
+    def test_agents_removes_from_only_named_client(self, monkeypatch):
+        state = self._state()
+        configured = self._stub(monkeypatch, state)
+
+        assert mcp.remove_skills_locations_command(["A.a"], agents={"claude"}) == 0
+
+        entry = _find_skills(state["mcp_servers"])[0]
+        assert mcp.skill_locations_for_client(entry, "claude") == ["B.b"]
+        assert mcp.skill_locations_for_client(entry, "codex") == ["A.a", "B.b"]
+        assert configured == [("claude", f"{WS}/ai-gateway/skills/?schema=B.b")]
+
+    def test_removing_all_schemas_keeps_schemaless_connection(self, monkeypatch):
+        state = self._state()
+        configured = self._stub(monkeypatch, state)
+
+        assert mcp.remove_skills_locations_command(["A.a", "B.b"]) == 0
+
+        entry = _find_skills(state["mcp_servers"])[0]
+        assert entry["skill_locations"] == []
+        assert sorted(configured) == [
+            ("claude", f"{WS}/ai-gateway/skills/"),
+            ("codex", f"{WS}/ai-gateway/skills/"),
+        ]
+
+
 class TestRegisterSchemalessSkillsConnection:
     def _stub(self, monkeypatch):
         saved_states: list[dict] = []