From a3378e8ec9deba9e848d77dfa0aa0b70a8585329 Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Tue, 8 Sep 2026 05:23:32 +0000 Subject: [PATCH] Add per-agent scoping to skill add --mcp Give `ucode skill add --mcp` an `--agents` option so a schema can be added to a chosen subset of configured coding agents instead of all of them, mirroring `ucode mcp add --agents`. `add_skills_command` takes an optional `agents` set and forwards it to `setup_mcp_clients`, which scopes the client set; the per-client map is updated only for the targeted clients, and the CLI bootstraps any named agent that is not configured yet before scoping. `--agents` is rejected outside `--mcp` since downloaded skills use shared directory families. Co-authored-by: Arthur Jenoudet Co-authored-by: Isaac --- src/ucode/cli.py | 30 +++++++++++++++++++++++- src/ucode/mcp.py | 10 +++++--- tests/test_cli.py | 37 +++++++++++++++++++++++++++++ tests/test_mcp.py | 59 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 132 insertions(+), 4 deletions(-) diff --git a/src/ucode/cli.py b/src/ucode/cli.py index 8a2c6eff..71d700fc 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -1352,6 +1352,15 @@ def skills_add( "Not valid with --mcp.", ), ] = None, + agents: Annotated[ + str | None, + typer.Option( + "--agents", + help="(--mcp only) Comma-separated coding agents whose skills MCP scope should " + "be updated. Any that aren't configured yet are set up first. Without --agents, " + "every configured agent is updated.", + ), + ] = None, ) -> None: """Add Databricks Skills to your coding tools, keeping any already configured. @@ -1367,6 +1376,16 @@ def skills_add( requested_skills = ( None if skills is None else {s.strip() for s in skills.split(",") if s.strip()} ) + requested_agents = None + if agents is not None: + requested_agents = { + agent.strip().lower() for agent in agents.split(",") if agent.strip() + } + if not requested_agents: + raise RuntimeError( + "No agents provided for --agents. Use a comma-separated list like " + "`--agents claude,codex`." + ) if mcp and path is not None: raise RuntimeError("--path is not supported when using --mcp") if mcp and requested_skills is not None: @@ -1387,6 +1406,9 @@ def skills_add( "`..` values " f"(invalid: {', '.join(sorted(invalid_skills))})." ) + # 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") if requested_skills is not None and not locations: schemas = {".".join(parts[:2]) for parts in qualified_skill_parts.values()} bare = sorted(skill for skill in requested_skills if skill not in qualified_skill_parts) @@ -1421,7 +1443,13 @@ def skills_add( None if requested_skills is None else {s.split(".")[-1] for s in requested_skills} ) if mcp: - add_skills_command(locations) + scope = ( + _configure_agents_for_mcp(sorted(requested_agents)) if requested_agents else None + ) + if scope is None: + add_skills_command(locations) + else: + add_skills_command(locations, agents=scope) else: configure_skills_download_command(locations, path=path, skills=selected_skills) except (RuntimeError, ValueError) as exc: diff --git a/src/ucode/mcp.py b/src/ucode/mcp.py index cff0f108..210be279 100644 --- a/src/ucode/mcp.py +++ b/src/ucode/mcp.py @@ -2284,10 +2284,14 @@ def _union_locations(base: list[str], new: list[str]) -> list[str]: return merged -def add_skills_command(locations: list[str]) -> int: - """Add ``locations`` to every configured client's skill scope, keeping any already configured.""" +def add_skills_command(locations: list[str], agents: set[str] | None = None) -> int: + """Add ``locations`` to each targeted client's skill scope, keeping any already configured. + + ``agents`` (from ``--agents``) scopes the update to that subset of configured clients; omitting + it targets every configured client. This mirrors ``ucode mcp add`` exactly: the client set is + the only thing ``--agents`` changes.""" state = load_state() - workspace, profile, clients = setup_mcp_clients(state, "Add Skills MCP") + workspace, profile, clients = setup_mcp_clients(state, "Add Skills MCP", agents=agents) locations_by_client = _skill_locations_by_client_from_state(state) for client in clients: locations_by_client[client] = _union_locations( diff --git a/tests/test_cli.py b/tests/test_cli.py index 4123726d..aa1ba1ad 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1433,6 +1433,43 @@ def test_malformed_location_exit_1(self): assert "--location" in _strip_ansi(result.output) mock_add.assert_not_called() + def test_agents_scope_is_configured_and_forwarded_for_mcp(self): + with ( + patch("ucode.cli._configure_agents_for_mcp", return_value={"claude"}) as configure, + patch("ucode.cli.add_skills_command") as mock_add, + ): + result = runner.invoke( + app, + ["skill", "add", "--location", "a.b", "--mcp", "--agents", "claude"], + ) + + assert result.exit_code == 0, result.output + configure.assert_called_once_with(["claude"]) + mock_add.assert_called_once_with(["a.b"], agents={"claude"}) + + def test_empty_agents_scope_is_rejected(self): + with ( + patch("ucode.cli._configure_agents_for_mcp") as configure, + patch("ucode.cli.add_skills_command") as mock_add, + ): + result = runner.invoke( + app, + ["skill", "add", "--location", "a.b", "--mcp", "--agents", ","], + ) + + assert result.exit_code == 1 + assert "No agents provided for --agents" in _strip_ansi(result.output) + configure.assert_not_called() + mock_add.assert_not_called() + + def test_agents_is_rejected_for_download_mode(self): + with patch("ucode.cli.configure_skills_download_command") as mock_download: + result = runner.invoke(app, ["skill", "add", "--location", "a.b", "--agents", "claude"]) + + assert result.exit_code == 1 + assert "--agents is only supported when using --mcp" in _strip_ansi(result.output) + mock_download.assert_not_called() + class TestManagedSkillsOnLaunch: """Managed skills are delivered by download only: the launch path downloads them and never diff --git a/tests/test_mcp.py b/tests/test_mcp.py index c28f0327..7997e93b 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -2606,6 +2606,65 @@ def test_registers_scope_from_empty_state(self, monkeypatch): assert _find_skills(state["mcp_servers"])[0]["skill_locations"] == ["A.a"] + def test_agents_updates_only_selected_client_scope(self, monkeypatch): + configured: list[tuple[str, str]] = [] + prior = mcp._resolve_skills_mcp_servers( + WS, ["claude", "codex"], _by_client(["claude", "codex"], ["A.a"]), [] + ) + state = {"workspace": WS, "available_tools": ["claude", "codex"], "mcp_servers": prior} + _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) + + assert mcp.add_skills_command(["B.b"], agents={"claude"}) == 0 + + entry = _find_skills(state["mcp_servers"])[0] + assert mcp.skill_locations_for_client(entry, "claude") == ["A.a", "B.b"] + assert mcp.skill_locations_for_client(entry, "codex") == ["A.a"] + assert configured == [("claude", f"{WS}/ai-gateway/skills/?schema=A.a&schema=B.b")] + + def test_global_addition_reaches_every_client_and_keeps_divergence(self, monkeypatch): + prior = mcp._resolve_skills_mcp_servers( + WS, ["claude", "codex"], {"claude": ["A.a", "B.b"], "codex": ["A.a"]}, [] + ) + state = {"workspace": WS, "available_tools": ["claude", "codex"], "mcp_servers": prior} + _stub_location_base(monkeypatch, state) + monkeypatch.setattr(mcp, "available_mcp_clients", lambda: ["claude", "codex"]) + monkeypatch.setattr(mcp, "configure_client_mcp_server", lambda *a, **kw: []) + monkeypatch.setattr(mcp, "save_state", lambda s: None) + + assert mcp.add_skills_command(["C.c"]) == 0 + + entry = _find_skills(state["mcp_servers"])[0] + assert mcp.skill_locations_for_client(entry, "claude") == ["A.a", "B.b", "C.c"] + assert mcp.skill_locations_for_client(entry, "codex") == ["A.a", "C.c"] + + def test_agents_add_matching_existing_scope_is_a_noop(self, monkeypatch): + configured: list[tuple[str, str]] = [] + prior = mcp._resolve_skills_mcp_servers( + WS, ["claude", "codex"], _by_client(["claude", "codex"], ["A.a"]), [] + ) + state = {"workspace": WS, "available_tools": ["claude", "codex"], "mcp_servers": prior} + _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) + + assert mcp.add_skills_command(["A.a"], agents={"claude"}) == 0 + + entry = _find_skills(state["mcp_servers"])[0] + assert mcp.skill_locations_for_client(entry, "claude") == ["A.a"] + assert configured == [] + class TestRegisterSchemalessSkillsConnection: def _stub(self, monkeypatch):