skill: add additive skill configuration

[artjen]

Summary

• Add the ucode skill add command namespace.
• Add schemas to the Skills MCP connection without replacing existing schemas.
• Support additive disk downloads with named and fully-qualified skill selection.
• Preserve all existing ucode configure skills behavior.

Part 1 of the skill CLI series.

Testing

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

---

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

[Copilot]

🟢 Approval recommended

The implementation and validation logic align with the documented behavior, and the PR adds focused test coverage for both the MCP union path and CLI parsing/dispatch.

Pull request overview

This PR adds a new ucode skill add CLI command to support additive Skills configuration: it can either (1) union new UC skill schema scopes into the existing Skills MCP connection (--mcp), or (2) download skills to disk without removing existing downloads, including support for selecting named skills via bare or fully-qualified identifiers.

Changes:

• Added ucode skill add CLI surface area with validation for --location, --mcp, --path, and --skills (including fully-qualified <catalog>.<schema>.<name> handling).
• Implemented additive Skills MCP scope updates via _union_locations(...) and add_skills_command(...).
• Added test coverage for both the unioning behavior and CLI argument parsing/dispatch, plus README documentation for the new command.

File summaries

File	Description
tests/test_mcp.py	Adds unit tests for _union_locations and add_skills_command additive scope behavior.
tests/test_cli.py	Adds CLI tests ensuring ucode skill add validates flags correctly and dispatches to MCP union vs download modes.
src/ucode/mcp.py	Introduces _union_locations and add_skills_command to merge new schema scopes into the existing Skills MCP connection.
src/ucode/cli.py	Adds the skill Typer sub-app and skill add command, including --skills parsing for named/fully-qualified selection.
README.md	Documents ucode skill add usage and adds it to the command summary table.

Review details

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

---

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

[xsh310]

Looks great!

[Copilot]

🟡 Changes recommended

_union_locations does not normalize duplicates already present in the existing base scope, which can preserve repeated locations and propagate them into persisted state/URL generation.

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

Review details

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

[Copilot]

🟡 Changes recommended

There is a confirmed edge-case where --skills parsing to an empty set yields a misleading error when --location is omitted, and it should be adjusted for clearer user-facing behavior.

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

Review details

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

[Copilot]

🔵 Needs a closer look

_union_locations should de-duplicate persisted base locations (not just new additions) and skill add should avoid a confusing error path when --skills is provided but empty and --location is omitted.

Review details

Suppressed comments (2)

src/ucode/mcp.py:2219

• _union_locations preserves duplicates that already exist in base, so the returned "union" can still contain repeated schema locations. Since base comes from persisted state, this can lead to duplicated skill_locations (and redundant schema entries in the generated URL/tools list). Consider normalizing both base and new while preserving the first-seen order.

def _union_locations(base: list[str], new: list[str]) -> list[str]:
    have = set(base)
    merged = list(base)
    for location in new:
        if location not in have:

src/ucode/cli.py:1391

• When --skills is provided but parses to an empty set (e.g. --skills "") and --location is omitted, the current branch tries to derive a schema and errors with "must all share one ... (got: none)", which is confusing for an empty selection. Consider only attempting schema derivation when requested_skills is non-empty so this falls through to the clearer --location is required error.

        if requested_skills is not None and not locations:

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