From e0ede4243a87822f364734cb2037469ffd21fe07 Mon Sep 17 00:00:00 2001 From: Arthur Jenoudet Date: Thu, 3 Sep 2026 19:18:27 +0000 Subject: [PATCH] Add agent-specific skill removal --- README.md | 8 ++++--- src/ucode/cli.py | 16 ++++++++++++-- src/ucode/mcp.py | 43 ++++++++++++++++++++++++++++---------- tests/test_cli.py | 9 +++++++- tests/test_mcp.py | 53 +++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 112 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 9b390cae..4e7442fe 100644 --- a/README.md +++ b/README.md @@ -251,11 +251,13 @@ ucode skill add --skills main.default.my-skill,main.default.other-skill #### Remove shared skill MCP scopes `ucode skill remove --mcp` interactively removes developer-configured schemas from the shared -skills MCP scope. Administrator-managed schemas are not offered, and the schema-less utility -connection remains registered after its last schema is removed. +skills MCP scope. With `--agents`, only those agents' additions are offered and removed; shared +schemas remain inherited. Administrator-managed schemas are not offered, and the schema-less +utility connection remains registered after its last schema is removed. ```bash ucode skill remove --mcp +ucode skill remove --mcp --agents codex ``` ### Managed config for a workspace (admins) @@ -396,7 +398,7 @@ The output looks like: | `ucode skill add --location main.default --mcp --agents claude` | Set up selected agents if needed and add schemas only to their MCP scopes | | `ucode skill add --location main.default` | Download a schema's skills to disk without removing existing downloads | | `ucode skill add --skills main.default.my-skill` | Download a named subset of skills (bare names need `--location`; fully-qualified names stand alone) | -| `ucode skill remove --mcp` | Interactively remove developer schemas from the shared skills MCP scope | +| `ucode skill remove --mcp [--agents codex]` | Interactively remove developer schemas from shared or selected agent scopes | | `ucode setup` | Author the managed config's agents and models (workspace admins only) | | `ucode setup mcps` | Add or change the managed config's MCP servers | | `ucode setup skills [--location a.b,c.d]` | Add or change the managed config's skills | diff --git a/src/ucode/cli.py b/src/ucode/cli.py index 95e73c6b..a321bfd7 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -1483,15 +1483,27 @@ def skills_remove( help="Remove schemas from the skills MCP connection instead of downloaded files.", ), ] = False, + agents: Annotated[ + str | None, + typer.Option( + "--agents", + help="Limit removal to additions for these comma-separated coding agents.", + ), + ] = None, ) -> None: - """Interactively remove shared Skill schemas from the skills MCP connection.""" + """Interactively remove shared schemas or selected agents' additions.""" try: if not mcp: raise RuntimeError( "Removing downloaded skills is not supported yet. Pass --mcp to remove " "schemas from the skills MCP connection." ) - remove_skills_command() + requested_agents = ( + None + if agents is None + else ({agent.strip().lower() for agent in agents.split(",") if agent.strip()} or None) + ) + remove_skills_command(agents=requested_agents) except RuntimeError as exc: print_err(str(exc)) raise typer.Exit(1) from None diff --git a/src/ucode/mcp.py b/src/ucode/mcp.py index 18a4bd67..056187ce 100644 --- a/src/ucode/mcp.py +++ b/src/ucode/mcp.py @@ -2423,14 +2423,15 @@ def _prompt_for_skill_removal( return [str(value) for value in selection] -def remove_skills_command() -> int: - """Interactively remove developer schemas from every client's effective scope.""" +def remove_skills_command(agents: set[str] | None = None) -> int: + """Interactively remove shared schemas or selected clients' additions.""" state = load_state() workspace, profile, clients = setup_mcp_clients( state, "Remove Skills MCP", require_auth=False, action_note="Removing from", + agents=agents, ) entry = _skills_entry(list(state.get("mcp_servers") or [])) managed = { @@ -2443,7 +2444,11 @@ def remove_skills_command() -> int: locations_by_client = { client: [ location - for location in skill_locations_for_client(entry, client) + for location in ( + skill_locations_for_client(entry, client) + if agents is None + else overrides.get(client, []) + ) if location not in managed ] for client in clients @@ -2459,14 +2464,30 @@ def remove_skills_command() -> int: return 0 remove_locations = set(selection) - new_default = [location for location in default if location not in remove_locations] - remaining_overrides: dict[str, list[str]] = {} - for client, client_locations in overrides.items(): - remaining = [ - location for location in client_locations if location not in remove_locations - ] - if remaining: - remaining_overrides[client] = remaining + if agents is None: + new_default = [location for location in default if location not in remove_locations] + remaining_overrides: dict[str, list[str]] = {} + for client, client_locations in overrides.items(): + remaining = [ + location for location in client_locations if location not in remove_locations + ] + if remaining: + remaining_overrides[client] = remaining + else: + new_default = default + remaining_overrides = overrides + for client in clients: + if client not in remaining_overrides: + continue + _set_skill_location_override( + remaining_overrides, + client, + [ + location + for location in remaining_overrides.get(client, []) + if location not in remove_locations + ], + ) _update_skills_mcp( state, diff --git a/tests/test_cli.py b/tests/test_cli.py index 78d1036f..d62994aa 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1265,7 +1265,14 @@ def test_mcp_remove_dispatches_global_removal(self): result = runner.invoke(app, ["skill", "remove", "--mcp"]) assert result.exit_code == 0, result.output - remove.assert_called_once_with() + remove.assert_called_once_with(agents=None) + + def test_mcp_remove_forwards_agent_scope(self): + with 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 + remove.assert_called_once_with(agents={"claude", "codex"}) class TestApplyManagedSkills: diff --git a/tests/test_mcp.py b/tests/test_mcp.py index a347b31b..eb42c695 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -2707,6 +2707,59 @@ def _stub(self, monkeypatch, state, selection): monkeypatch.setattr(mcp, "save_state", lambda s: None) return configured + def test_agent_scope_removes_from_only_selected_client(self, monkeypatch): + state = self._state() + entry = _find_skills(state["mcp_servers"])[0] + entry[mcp.SKILL_LOCATION_OVERRIDES_KEY] = {"claude": ["C.c"]} + configured = self._stub(monkeypatch, state, ["C.c"]) + + assert mcp.remove_skills_command(agents={"claude"}) == 0 + + entry = _find_skills(state["mcp_servers"])[0] + assert entry["skill_locations"] == ["A.a", "B.b"] + assert entry[mcp.SKILL_LOCATION_OVERRIDES_KEY] == {"claude": []} + assert mcp.skill_locations_for_client(entry, "claude") == ["A.a", "B.b"] + assert mcp.skill_locations_for_client(entry, "codex") == ["A.a", "B.b"] + assert configured == [("claude", f"{WS}/ai-gateway/skills/?schema=A.a&schema=B.b")] + + def test_agent_scope_does_not_add_empty_overrides_to_other_selected_clients(self, monkeypatch): + state = self._state() + entry = _find_skills(state["mcp_servers"])[0] + entry[mcp.SKILL_LOCATION_OVERRIDES_KEY] = {"claude": ["C.c"]} + self._stub(monkeypatch, state, ["C.c"]) + + assert mcp.remove_skills_command(agents={"claude", "codex"}) == 0 + + entry = _find_skills(state["mcp_servers"])[0] + assert entry[mcp.SKILL_LOCATION_OVERRIDES_KEY] == {"claude": []} + + def test_agent_scope_offers_only_agent_specific_additions(self, monkeypatch): + state = self._state() + entry = _find_skills(state["mcp_servers"])[0] + entry[mcp.SKILL_LOCATION_OVERRIDES_KEY] = {"claude": ["B.b", "C.c"]} + captured: dict[str, dict[str, list[str]]] = {} + _stub_location_base(monkeypatch, state) + monkeypatch.setattr(mcp, "available_mcp_clients", lambda: ["claude", "codex"]) + monkeypatch.setattr( + mcp, + "_prompt_for_skill_removal", + lambda scopes: captured.setdefault("scopes", scopes) and None, + ) + + assert mcp.remove_skills_command(agents={"claude"}) == 0 + assert captured["scopes"] == {"claude": ["B.b", "C.c"]} + + def test_agent_scope_remains_explicit_when_removal_matches_default(self, monkeypatch): + state = self._state() + entry = _find_skills(state["mcp_servers"])[0] + entry[mcp.SKILL_LOCATION_OVERRIDES_KEY] = {"claude": ["A.a", "B.b", "C.c"]} + self._stub(monkeypatch, state, ["C.c"]) + + assert mcp.remove_skills_command(agents={"claude"}) == 0 + + entry = _find_skills(state["mcp_servers"])[0] + assert entry[mcp.SKILL_LOCATION_OVERRIDES_KEY] == {"claude": ["A.a", "B.b"]} + def test_unscoped_remove_keeps_schemaless_connection(self, monkeypatch): state = self._state() configured = self._stub(monkeypatch, state, ["A.a", "B.b"])