Support client-aware managed skill scopes

[artjen]

Why

The skills MCP state previously had one shared schema list and one derived URL. That cannot represent different scopes for different agents, and updating one client risks rewriting another client's configuration.

What changed

This PR adds the state and MCP-configuration layer for client-aware scopes. It intentionally does not add a new CLI flag; later PRs build on this representation.

How it works

• skill_locations remains the shared default.
• skill_location_overrides stores durable per-client additions independently from the default.
• A client's effective scope is the ordered, deduplicated union of those two lists.
• MCP updates compare per-client effective URLs and rewrite only clients whose URL changed.
• Managed schemas are added for the launching client without copying the shared default into its additions.

Testing

• uv run pytest tests/test_skills_download.py tests/test_mcp.py tests/test_cli.py -q — 495 passed
• uv run ruff check .
• uv run ty check src/

Tests cover persistence, durable additions, union deduplication, client-specific URL updates, and managed skill application.

---

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

[Copilot]

🟡 Changes recommended

There is a confirmed CLI behavior bug around empty --agents input plus a gap in test coverage for the new managed per-client override behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR extends ucode’s Skills MCP configuration to support per-agent skill schema scopes (via sparse per-client overrides) while keeping backward-compatible default behavior, and exposes that capability through ucode skill add --mcp --agents ... plus updated status/help output.

Changes:

• Added persisted per-client scope overrides (skill_location_overrides) and client-specific effective scope resolution in ucode.mcp.
• Updated Skills MCP registration/update flow to reconfigure only affected clients and to render per-agent scope details when scopes diverge.
• Extended CLI and docs to support skill add --agents (MCP-only) and to show per-agent scopes in ucode status, with corresponding test coverage updates.

File summaries

File	Description
src/ucode/mcp.py	Adds per-client skill scope overrides and updates managed/apply/add flows to be client-aware.
src/ucode/cli.py	Adds skill add --agents (MCP-only) and updates status output for divergent per-agent scopes.
tests/test_mcp.py	Adds tests for override precedence and agent-scoped MCP skill additions.
tests/test_cli.py	Adds CLI tests for --agents forwarding/rejection and per-agent status rendering.
README.md	Documents agent-scoped skill scope additions and clarifies download vs MCP behavior.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

The new state-reading helpers can mis-handle malformed persisted skill_locations values (e.g., a string becomes a list of characters), which is a correctness issue worth fixing before merge.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/ucode/mcp.py:2141

• skill_locations_for_client() builds the default scope via list((entry or {}).get("skill_locations") or []). If a persisted state is malformed (e.g., skill_locations is a string), this will split it into characters and produce an invalid scope (and invalid ?schema= URL generation downstream). Consider type-checking skill_locations and treating non-lists as empty, consistent with other state-reading helpers in this module.

This issue also appears on line 2325 of the same file.

src/ucode/mcp.py:2328

• _skill_mcp_locations() currently does list((entry or {}).get("skill_locations") or []), which will also split a string into characters if the persisted state is malformed. Since this helper is used as the default scope source for updates, it’s safer to validate the type and treat non-lists as empty.

def _skill_mcp_locations(state: dict) -> list[str]:
    """The skills MCP connection's ``skill_locations``, or ``[]`` if none exists."""
    entry = _skills_entry(list(state.get("mcp_servers") or []))
    return list((entry or {}).get("skill_locations") or [])

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The changes are cohesive and well-tested, with only a minor best-practice nit identified in the new skills MCP update helper.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/ucode/mcp.py:2284

• Avoid using assert for a required runtime invariant in production code, since assertions can be disabled with -O and then a missing skills entry would fail later with a less clear error. Prefer an explicit check that raises a RuntimeError with context.

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

There is a confirmed case where apply_managed_skills can skip registering the skills MCP server for a newly launching client if the effective schema list matches, and configure_skills_mcp_command currently clears per-client overrides unexpectedly.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The change is internally consistent, preserves backwards behavior where intended, and is supported by focused regression tests covering the new client-aware scope semantics.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite