From 8589f8d8a11e6f1c077f00d80125972cd733f5d8 Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Tue, 8 Sep 2026 05:23:32 +0000 Subject: [PATCH 1/2] 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. `--agents` is rejected outside `--mcp` since downloaded skills use shared directory families. The `--mcp --agents` path bootstraps only agents that are not configured for MCP yet, so re-targeting an already-configured agent no longer re-runs the agent setup (re-login, binary reinstall, re-validate). It computes the not-yet-ready subset and passes just those to `_configure_agents_for_mcp`, while still scoping the skills update to every named agent. Co-authored-by: Arthur Jenoudet Co-authored-by: Isaac --- src/ucode/cli.py | 34 ++++++++++++++++++++++++++- src/ucode/mcp.py | 10 +++++--- tests/test_cli.py | 58 ++++++++++++++++++++++++++++++++++++++++++++++ tests/test_mcp.py | 59 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 157 insertions(+), 4 deletions(-) diff --git a/src/ucode/cli.py b/src/ucode/cli.py index 91ecb9ac..0641b1e5 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -105,8 +105,10 @@ add_mcp_command, add_skills_command, apply_managed_mcp_servers, + available_mcp_clients, configure_mcp_command, configure_skills_mcp_command, + configured_mcp_clients, purge_cross_workspace_mcp_residue, remove_mcp_command, revert_mcp_configs, @@ -1236,6 +1238,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. @@ -1251,6 +1262,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: @@ -1271,6 +1292,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) @@ -1305,7 +1329,15 @@ def skills_add( None if requested_skills is None else {s.split(".")[-1] for s in requested_skills} ) if mcp: - add_skills_command(locations) + if requested_agents: + scope = {a if a == "cursor" else normalize_tool(a) for a in requested_agents} + ready = set(configured_mcp_clients(load_state(), available_mcp_clients())) + to_bootstrap = sorted(scope - ready) + if to_bootstrap: + _configure_agents_for_mcp(to_bootstrap) + add_skills_command(locations, agents=scope) + else: + add_skills_command(locations) 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 84a4caf2..6ce85485 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1467,6 +1467,64 @@ def test_malformed_location_exit_1(self): assert "--location" in _strip_ansi(result.output) mock_add.assert_not_called() + def test_agents_scope_bootstraps_only_unconfigured_and_forwards_full_scope(self): + with ( + patch("ucode.cli.load_state", return_value={"workspace": "https://ws"}), + patch("ucode.cli.available_mcp_clients", return_value=["claude", "codex"]), + patch("ucode.cli.configured_mcp_clients", return_value=["claude"]), + patch("ucode.cli._configure_agents_for_mcp", return_value={"codex"}) as configure, + patch("ucode.cli.add_skills_command") as mock_add, + ): + result = runner.invoke( + app, + ["skill", "add", "--location", "a.b", "--mcp", "--agents", "claude,codex"], + ) + + assert result.exit_code == 0, result.output + # Only the not-yet-configured agent is bootstrapped; the scope keeps both. + configure.assert_called_once_with(["codex"]) + mock_add.assert_called_once_with(["a.b"], agents={"claude", "codex"}) + + def test_already_configured_agent_skips_bootstrap(self): + with ( + patch("ucode.cli.load_state", return_value={"workspace": "https://ws"}), + patch("ucode.cli.available_mcp_clients", return_value=["claude", "codex"]), + patch("ucode.cli.configured_mcp_clients", return_value=["claude"]), + 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", "claude"], + ) + + assert result.exit_code == 0, result.output + configure.assert_not_called() + 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): From 879247f3a29a4f669796c7a2e80726d7450abc54 Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Tue, 8 Sep 2026 23:08:08 +0000 Subject: [PATCH 2/2] Fold empty skill add --agents to None `skill add --agents ""` (or `,`) raised a hard "No agents provided" error, while the other agent-scoped commands (`mcp add`, `mcp remove`) fold an empty --agents value to None and act globally. Match them so `skill add --mcp` mirrors `ucode mcp add` exactly: an empty --agents now targets every configured agent instead of erroring. Co-authored-by: Arthur Jenoudet Co-authored-by: Isaac --- src/ucode/cli.py | 15 +++++---------- tests/test_cli.py | 7 +++---- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/ucode/cli.py b/src/ucode/cli.py index 0641b1e5..2da71f01 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -1262,16 +1262,11 @@ 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`." - ) + requested_agents = ( + None + if agents is None + else ({agent.strip().lower() for agent in agents.split(",") if agent.strip()} or None) + ) if mcp and path is not None: raise RuntimeError("--path is not supported when using --mcp") if mcp and requested_skills is not None: diff --git a/tests/test_cli.py b/tests/test_cli.py index 6ce85485..6513b90d 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1502,7 +1502,7 @@ def test_already_configured_agent_skips_bootstrap(self): configure.assert_not_called() mock_add.assert_called_once_with(["a.b"], agents={"claude"}) - def test_empty_agents_scope_is_rejected(self): + def test_empty_agents_folds_to_global_scope(self): with ( patch("ucode.cli._configure_agents_for_mcp") as configure, patch("ucode.cli.add_skills_command") as mock_add, @@ -1512,10 +1512,9 @@ def test_empty_agents_scope_is_rejected(self): ["skill", "add", "--location", "a.b", "--mcp", "--agents", ","], ) - assert result.exit_code == 1 - assert "No agents provided for --agents" in _strip_ansi(result.output) + assert result.exit_code == 0, result.output configure.assert_not_called() - mock_add.assert_not_called() + mock_add.assert_called_once_with(["a.b"]) def test_agents_is_rejected_for_download_mode(self): with patch("ucode.cli.configure_skills_download_command") as mock_download: