Skip to content

Commit edc7762

Browse files
authored
skills: add --skill subset filter to the download path (#252)
* skills: add --skill subset filter to the download path `ucode configure skills --location cat.sch --skill my_skill` downloads only the named skill(s) from each schema instead of every skill, mirroring the `--services` subset filter on `configure mcp`. Matched by leaf name; unknown names warn and are skipped. Download-only — rejected with `--mcp` (the skills MCP connection is schema-scoped) and without `--location`. Co-authored-by: Isaac * skills: require a single --location when --skill is present A single skill-name set can't be split across schemas, so reject --skill combined with more than one --location, mirroring `configure mcp` (whose --service filter also takes a single --location). Co-authored-by: Isaac * skills: tighten --skill comments and docstrings Trim the multi-location and download_skills comments to why-only, and delegate the configure_skills_download_command docstring to download_skills instead of restating the filter semantics. Co-authored-by: Isaac * skills: address PR nits - Drop the redundant single-location comment (the error message is clear). - Reword the unknown-skill warning to "Skipping requested skill(s) not found", matching the file's other "Skipping ..." warnings. Co-authored-by: Isaac * skills: fix --skill example name and over-filtered message - Use `my-skill` (hyphen) in all --skill examples/help; underscores don't match SKILL_NAME_PATTERN (^[a-z0-9-]+$), so `my_skill` could never match a real skill. - When a --skill filter selects none of a non-empty schema's skills, print "No requested skills to download from ..." instead of the misleading "No skills found in ...". Co-authored-by: Isaac * skills: drop unnecessary comment on the over-filtered branch The message string is self-explanatory; the two branches don't need a comment distinguishing them. Co-authored-by: Isaac
1 parent 41ec3f9 commit edc7762

5 files changed

Lines changed: 180 additions & 12 deletions

File tree

README.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,9 @@ ucode configure skills
124124
# Download mode: fetch every skill in the schema to disk (and register the connection).
125125
ucode configure skills --location main.default --path /abs/project/dir
126126

127+
# Download a named subset of the schema's skills instead of all of them.
128+
ucode configure skills --location main.default --skill my-skill
129+
127130
# MCP mode: expose the schema's skills as MCP tools instead of downloading.
128131
ucode configure skills --location main.default,ml.prod --mcp
129132
```
@@ -135,7 +138,10 @@ ucode configure skills --location main.default,ml.prod --mcp
135138
(plus its bundled files) into both `.claude/skills/` and `.agents/skills/`. `--path` (an existing
136139
absolute directory) is optional; when omitted, skills are written under your home directory. Any
137140
pre-existing skill dir prompts before it's overwritten. It then registers a schema-less skills
138-
MCP connection, leaving any prior `--mcp` scope untouched.
141+
MCP connection, leaving any prior `--mcp` scope untouched. `--skill <name>[,<name>…]` narrows the
142+
download to the named skills (by leaf name) from the schema instead of all of them; requested
143+
names not found in the schema warn and are skipped. `--skill` requires a single `--location`, is
144+
download-only, and is rejected with `--mcp`.
139145
- **MCP mode** (`--location … --mcp`) sets the connection's location set to exactly `<list>`
140146
(override-only) and rebuilds its `?schema=` URL; no files are downloaded and `--path` is rejected.
141147

@@ -160,6 +166,7 @@ you to run `ucode <agent>` (existing agent sessions need a restart before the MC
160166
| `ucode configure --agents claude --mcp system.ai.slack` | Configure an agent and register its Databricks MCP server(s) in one command |
161167
| `ucode configure skills` | Register the skills MCP connection (utility tools only); no skills download |
162168
| `ucode configure skills --location main.default [--path <dir>]` | Download a schema's skills to disk (under `<dir>`, or your home dir) and register a schema-less skills MCP connection |
169+
| `ucode configure skills --location main.default --skill my-skill` | Download only the named skill(s) from a schema (comma-separated for several) |
163170
| `ucode configure skills --location main.default --mcp` | Expose a schema's skills as MCP tools (override-only) instead of downloading |
164171

165172
## Managed Local Files

src/ucode/cli.py

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1545,6 +1545,15 @@ def configure_skills(
15451545
help="(download) Existing absolute dir to download into; defaults to your home dir.",
15461546
),
15471547
] = None,
1548+
skill: Annotated[
1549+
str | None,
1550+
typer.Option(
1551+
"--skill",
1552+
help="(download) Download only this comma-separated subset of skills (by leaf "
1553+
"name, e.g. `my-skill`) from the schema, instead of every skill. Requires a "
1554+
"single --location; not valid with --mcp.",
1555+
),
1556+
] = None,
15481557
) -> None:
15491558
"""Configure Databricks Skills for your coding tools.
15501559
@@ -1554,18 +1563,33 @@ def configure_skills(
15541563
When ``--location`` is provided: with ``--mcp``, sets the connection's scope to
15551564
exactly the listed schemas (no download); otherwise, downloads every skill in
15561565
each schema to disk (under ``--path``, or your home dir when omitted) and
1557-
registers the MCP connection with utility tools only.
1566+
registers the MCP connection with utility tools only. ``--skill`` narrows a
1567+
download to a named subset of a single schema's skills (requires exactly one
1568+
``--location``).
15581569
"""
15591570
try:
15601571
locations = _parse_skill_locations(location)
1572+
# `--skill` absent -> None (whole schema); present (even empty) -> the
1573+
# explicit subset, so `--skill ""` downloads nothing.
1574+
selected_skills = (
1575+
None if skill is None else {s.strip() for s in skill.split(",") if s.strip()}
1576+
)
15611577
if mcp and path is not None:
15621578
raise RuntimeError("--path is not valid with --mcp.")
1579+
if mcp and selected_skills is not None:
1580+
raise RuntimeError("--skill is not valid with --mcp; it only applies when downloading.")
15631581
if path is not None and not locations:
15641582
raise RuntimeError("--path only applies when downloading with --location.")
1583+
if selected_skills is not None and not locations:
1584+
raise RuntimeError("--skill only applies when downloading with --location.")
1585+
if selected_skills is not None and len(locations) != 1:
1586+
raise RuntimeError(
1587+
f"--skill requires a single --location (got: {', '.join(locations)})."
1588+
)
15651589
if mcp or not locations:
15661590
configure_skills_mcp_command(locations)
15671591
else:
1568-
configure_skills_download_command(locations, path=path)
1592+
configure_skills_download_command(locations, path=path, skills=selected_skills)
15691593
except (RuntimeError, ValueError) as exc:
15701594
print_err(str(exc))
15711595
raise typer.Exit(1) from None

src/ucode/skills_download.py

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -247,12 +247,21 @@ def _fetch_bundles(
247247
return results
248248

249249

250-
def download_skills(workspace: str, token: str, locations: list[str], path: str | None) -> None:
250+
def download_skills(
251+
workspace: str,
252+
token: str,
253+
locations: list[str],
254+
path: str | None,
255+
skills: set[str] | None = None,
256+
) -> None:
251257
"""Download every skill in each ``<catalog>.<schema>`` location to disk.
252258
253259
Bundles are fetched concurrently (with a progress bar) per schema, then
254260
written sequentially so overwrite prompts don't interleave. A failure on one
255261
skill warns and skips it without aborting the batch.
262+
263+
When ``skills`` is given, only those leaf names are downloaded; names absent
264+
from a schema warn and are skipped. ``None`` downloads the whole schema.
256265
"""
257266
roots = skill_dir_roots(path)
258267
roots_display = " and ".join(str(root) for root in roots)
@@ -262,6 +271,17 @@ def download_skills(workspace: str, token: str, locations: list[str], path: str
262271
if reason:
263272
print_warning(f"Skipping `{location}`: {reason}.")
264273
continue
274+
if skills is not None:
275+
unknown = skills - set(leaves)
276+
if unknown:
277+
print_warning(
278+
f"Skipping requested skill(s) not found in `{location}`: "
279+
f"{', '.join(sorted(unknown))}."
280+
)
281+
leaves = [leaf for leaf in leaves if leaf in skills]
282+
if not leaves:
283+
print_note(f"No requested skills to download from `{location}`.")
284+
continue
265285
if not leaves:
266286
print_note(f"No skills found in `{location}`.")
267287
continue
@@ -281,17 +301,20 @@ def download_skills(workspace: str, token: str, locations: list[str], path: str
281301
)
282302

283303

284-
def configure_skills_download_command(locations: list[str], *, path: str | None) -> int:
304+
def configure_skills_download_command(
305+
locations: list[str], *, path: str | None, skills: set[str] | None = None
306+
) -> int:
285307
"""Download every skill in each schema to disk and register the skills connection.
286308
287309
Downloads to ``path`` (or the home dir when None), then registers/keeps the
288310
schema-less MCP connection. ``skill_locations`` is never touched, so a prior
289-
``--mcp`` set survives a download run."""
311+
``--mcp`` set survives a download run. ``skills`` narrows the download (see
312+
``download_skills``)."""
290313
state = load_state()
291314
workspace, profile, clients = setup_mcp_clients(state, "Skills")
292315
token = get_databricks_token(workspace, profile)
293316

294-
download_skills(workspace, token, locations, path)
317+
download_skills(workspace, token, locations, path, skills)
295318

296319
register_schemaless_skills_connection(state, workspace, profile, clients)
297320
return 0

tests/test_cli.py

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -407,13 +407,63 @@ def test_default_mode_dispatches_download_with_path(self):
407407
app, ["configure", "skills", "--location", "a.b", "--path", "/tmp/skills"]
408408
)
409409
assert result.exit_code == 0, result.output
410-
mock_download.assert_called_once_with(["a.b"], path="/tmp/skills")
410+
mock_download.assert_called_once_with(["a.b"], path="/tmp/skills", skills=None)
411411

412412
def test_default_mode_without_path_dispatches_download(self):
413413
with patch("ucode.cli.configure_skills_download_command") as mock_download:
414414
result = runner.invoke(app, ["configure", "skills", "--location", "a.b"])
415415
assert result.exit_code == 0, result.output
416-
mock_download.assert_called_once_with(["a.b"], path=None)
416+
mock_download.assert_called_once_with(["a.b"], path=None, skills=None)
417+
418+
def test_skill_filter_dispatches_download_with_subset(self):
419+
with patch("ucode.cli.configure_skills_download_command") as mock_download:
420+
result = runner.invoke(
421+
app, ["configure", "skills", "--location", "a.b", "--skill", "my_skill"]
422+
)
423+
assert result.exit_code == 0, result.output
424+
mock_download.assert_called_once_with(["a.b"], path=None, skills={"my_skill"})
425+
426+
def test_skill_filter_parses_comma_list(self):
427+
with patch("ucode.cli.configure_skills_download_command") as mock_download:
428+
result = runner.invoke(
429+
app, ["configure", "skills", "--location", "a.b", "--skill", "s1, s2"]
430+
)
431+
assert result.exit_code == 0, result.output
432+
mock_download.assert_called_once_with(["a.b"], path=None, skills={"s1", "s2"})
433+
434+
def test_skill_with_mcp_exit_1(self):
435+
with (
436+
patch("ucode.cli.configure_skills_mcp_command") as mock_mcp,
437+
patch("ucode.cli.configure_skills_download_command") as mock_download,
438+
):
439+
result = runner.invoke(
440+
app, ["configure", "skills", "--location", "a.b", "--mcp", "--skill", "my_skill"]
441+
)
442+
assert result.exit_code == 1
443+
assert "--skill" in _strip_ansi(result.output)
444+
mock_mcp.assert_not_called()
445+
mock_download.assert_not_called()
446+
447+
def test_skill_without_location_exit_1(self):
448+
with (
449+
patch("ucode.cli.configure_skills_mcp_command") as mock_mcp,
450+
patch("ucode.cli.configure_skills_download_command") as mock_download,
451+
):
452+
result = runner.invoke(app, ["configure", "skills", "--skill", "my_skill"])
453+
assert result.exit_code == 1
454+
assert "--skill" in _strip_ansi(result.output)
455+
mock_mcp.assert_not_called()
456+
mock_download.assert_not_called()
457+
458+
def test_skill_with_multiple_locations_exit_1(self):
459+
with patch("ucode.cli.configure_skills_download_command") as mock_download:
460+
result = runner.invoke(
461+
app, ["configure", "skills", "--location", "a.b, c.d", "--skill", "my_skill"]
462+
)
463+
assert result.exit_code == 1
464+
output = _strip_ansi(result.output)
465+
assert "--skill requires a single --location" in output
466+
mock_download.assert_not_called()
417467

418468
def test_path_with_mcp_exit_1(self):
419469
with (

tests/test_skills_download.py

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,60 @@ def test_summary_counts_only_written_skills(self, tmp_path, monkeypatch, capsys)
363363

364364
assert "Downloaded 1/2 skill(s) from `main.default` in" in capsys.readouterr().out
365365

366+
def test_skill_filter_downloads_only_matching_leaves(self, tmp_path, monkeypatch):
367+
monkeypatch.setattr(
368+
sd, "list_schema_skills", lambda *a, **k: (["pii-handling", "triage"], None)
369+
)
370+
monkeypatch.setattr(sd, "fetch_skill_bundle", lambda *a, **k: ({"SKILL.md": b"x"}, None))
371+
372+
sd.download_skills(WS, "token", ["main.default"], str(tmp_path), {"triage"})
373+
374+
assert (tmp_path / ".claude/skills/triage/SKILL.md").read_bytes() == b"x"
375+
assert not (tmp_path / ".claude/skills/pii-handling").exists()
376+
377+
def test_skill_filter_warns_on_unknown_and_downloads_rest(self, tmp_path, monkeypatch, capsys):
378+
monkeypatch.setattr(sd, "list_schema_skills", lambda *a, **k: (["triage"], None))
379+
monkeypatch.setattr(sd, "fetch_skill_bundle", lambda *a, **k: ({"SKILL.md": b"x"}, None))
380+
381+
sd.download_skills(WS, "token", ["main.default"], str(tmp_path), {"triage", "ghost"})
382+
383+
out = capsys.readouterr().out
384+
assert "Skipping requested skill(s) not found in `main.default`: ghost" in out
385+
assert (tmp_path / ".claude/skills/triage/SKILL.md").read_bytes() == b"x"
386+
387+
def test_empty_skill_filter_downloads_nothing(self, tmp_path, monkeypatch, capsys):
388+
monkeypatch.setattr(sd, "list_schema_skills", lambda *a, **k: (["triage"], None))
389+
called = []
390+
monkeypatch.setattr(
391+
sd, "fetch_skill_bundle", lambda *a, **k: called.append(1) or ({"SKILL.md": b"x"}, None)
392+
)
393+
394+
sd.download_skills(WS, "token", ["main.default"], str(tmp_path), set())
395+
396+
assert called == []
397+
assert not (tmp_path / ".claude/skills/triage").exists()
398+
# The schema has skills; the filter selected none — distinct from the
399+
# empty-schema note.
400+
out = capsys.readouterr().out
401+
assert "No requested skills to download from `main.default`." in out
402+
assert "No skills found" not in out
403+
404+
def test_empty_schema_reports_no_skills_found(self, tmp_path, monkeypatch, capsys):
405+
monkeypatch.setattr(sd, "list_schema_skills", lambda *a, **k: ([], None))
406+
407+
sd.download_skills(WS, "token", ["main.default"], str(tmp_path), None)
408+
409+
assert "No skills found in `main.default`." in capsys.readouterr().out
410+
411+
def test_none_skill_filter_downloads_everything(self, tmp_path, monkeypatch):
412+
monkeypatch.setattr(sd, "list_schema_skills", lambda *a, **k: (["a", "b"], None))
413+
monkeypatch.setattr(sd, "fetch_skill_bundle", lambda *a, **k: ({"SKILL.md": b"x"}, None))
414+
415+
sd.download_skills(WS, "token", ["main.default"], str(tmp_path), None)
416+
417+
assert (tmp_path / ".claude/skills/a/SKILL.md").exists()
418+
assert (tmp_path / ".claude/skills/b/SKILL.md").exists()
419+
366420

367421
class TestConfigureSkillsDownloadCommand:
368422
def _stub(self, monkeypatch):
@@ -375,7 +429,9 @@ def _stub(self, monkeypatch):
375429
monkeypatch.setattr(
376430
sd,
377431
"download_skills",
378-
lambda ws, tok, locations, path: calls.update(download=(ws, tok, locations, path)),
432+
lambda ws, tok, locations, path, skills=None: calls.update(
433+
download=(ws, tok, locations, path, skills)
434+
),
379435
)
380436
monkeypatch.setattr(
381437
sd,
@@ -389,13 +445,21 @@ def test_downloads_then_registers_connection(self, monkeypatch):
389445

390446
assert sd.configure_skills_download_command(["a.b"], path="/tmp/skills") == 0
391447

392-
assert calls["download"] == (WS, "token", ["a.b"], "/tmp/skills")
448+
assert calls["download"] == (WS, "token", ["a.b"], "/tmp/skills", None)
393449
assert calls["register"] == (WS, "profile", ["claude"])
394450

395451
def test_none_path_threads_through(self, monkeypatch):
396452
calls = self._stub(monkeypatch)
397453

398454
assert sd.configure_skills_download_command(["a.b"], path=None) == 0
399455

400-
assert calls["download"] == (WS, "token", ["a.b"], None)
456+
assert calls["download"] == (WS, "token", ["a.b"], None, None)
457+
assert calls["register"] == (WS, "profile", ["claude"])
458+
459+
def test_skills_filter_threads_through(self, monkeypatch):
460+
calls = self._stub(monkeypatch)
461+
462+
assert sd.configure_skills_download_command(["a.b"], path=None, skills={"triage"}) == 0
463+
464+
assert calls["download"] == (WS, "token", ["a.b"], None, {"triage"})
401465
assert calls["register"] == (WS, "profile", ["claude"])

0 commit comments

Comments
 (0)