From 6d166ed865545981de9ac2db79d2ed94018eae6f Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Tue, 8 Sep 2026 05:30:52 +0000 Subject: [PATCH] Scope skill remove --mcp to named agents Give `ucode skill remove --mcp` an `--agents` option so a schema can be removed from a chosen subset of configured agents and kept on the rest, mirroring `ucode mcp remove --agents`. `remove_skills_command` takes an optional `agents` set and forwards it to `setup_mcp_clients`, which scopes the client set; the picker then offers only those clients' schemas and removal edits only their maps. This closes the compose gap: `skill add --mcp X` (all agents) followed by `skill remove --mcp --agents claude` now removes X from claude while codex keeps it, and the schema is actually offered instead of the false "nothing to remove". Co-authored-by: Arthur Jenoudet Co-authored-by: Isaac --- src/ucode/cli.py | 22 ++++++++++++++++++++-- src/ucode/mcp.py | 15 +++++++++------ tests/test_cli.py | 9 ++++++++- tests/test_mcp.py | 38 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 9 deletions(-) diff --git a/src/ucode/cli.py b/src/ucode/cli.py index d73a2035..a6f3a849 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -1487,15 +1487,33 @@ def skills_remove( help="Remove schemas from the skills MCP connection instead of downloaded files.", ), ] = False, + agents: Annotated[ + str | None, + typer.Option( + "--agents", + help="Comma-separated coding agents to remove the schemas from (e.g. claude,codex). " + "A schema scoped to several agents is removed only from the named ones and kept on " + "the rest. Without --agents, a selected schema is removed from every agent it's on.", + ), + ] = None, ) -> None: - """Interactively remove Skill schemas from the skills MCP connection.""" + """Interactively remove Skill schemas from the skills MCP connection. + + Without ``--agents`` a selected schema is removed from every configured agent; ``--agents`` + scopes the removal to the named agents and keeps the schema on the rest. + """ 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 b2bbff80..4fa83a5b 100644 --- a/src/ucode/mcp.py +++ b/src/ucode/mcp.py @@ -2336,23 +2336,26 @@ def _prompt_for_skill_removal(locations_by_client: dict[str, list[str]]) -> list return [str(value) for value in selection] -def remove_skills_command() -> int: - """`ucode skill remove --mcp`: interactively drop skill schemas from every configured client. +def remove_skills_command(agents: set[str] | None = None) -> int: + """`ucode skill remove --mcp`: interactively drop skill schemas from clients' skills scopes. - Shows the schemas currently in each configured client's skills scope and removes the ones you - select from every client that has them. It never adds or reconfigures anything, and needs no - Databricks auth.""" + 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; + with ``agents`` (from ``--agents``) removal is scoped to the named clients and kept on the rest. + It never adds or reconfigures anything, and 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, ) locations_by_client = _skill_locations_by_client_from_state(state) offered = {client: locations_by_client.get(client, []) for client in clients} if not any(offered.values()): - print_note("No skill schemas are configured to remove.") + scope = "" if agents is None else f" for {', '.join(sorted(agents))}" + print_note(f"No skill schemas are configured to remove{scope}.") return 0 selection = _prompt_for_skill_removal(offered) diff --git a/tests/test_cli.py b/tests/test_cli.py index 88eed167..daf19d1a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1485,7 +1485,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 TestManagedSkillsOnLaunch: diff --git a/tests/test_mcp.py b/tests/test_mcp.py index c5a1cb9f..ba5dd9aa 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -2745,6 +2745,44 @@ def test_offers_each_client_scope_to_the_picker(self, monkeypatch): assert mcp.remove_skills_command() == 0 assert captured["scopes"] == {"claude": ["A.a", "B.b"], "codex": ["A.a"]} + def test_agent_scope_removes_from_only_named_client(self, monkeypatch): + # The headline fix: add-all then remove --agents claude drops the schema for claude only. + state = self._state() + configured = self._stub(monkeypatch, state, ["A.a"]) + + assert mcp.remove_skills_command(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_agent_scope_offers_only_named_clients_scope(self, monkeypatch): + state = self._state({"claude": ["A.a", "B.b"], "codex": ["A.a"]}) + 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": ["A.a", "B.b"]} + + def test_agent_scope_with_empty_scope_is_a_noop(self, monkeypatch): + state = self._state({"claude": ["A.a"], "codex": []}) + captured: dict[str, bool] = {} + _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("called", True) + ) + + assert mcp.remove_skills_command(agents={"codex"}) == 0 + assert "called" not in captured + class TestRegisterSchemalessSkillsConnection: def _stub(self, monkeypatch):