Skip to content

Add skill remove --mcp for global scope removal - #524

Merged
xsh310 merged 2 commits into
xshen/skill-per-agent-statusfrom
xshen/skill-remove
Sep 11, 2026
Merged

Add skill remove --mcp for global scope removal#524
xsh310 merged 2 commits into
xshen/skill-per-agent-statusfrom
xshen/skill-remove

Conversation

@xsh310

@xsh310 xsh310 commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

What

Adds ucode skill remove --mcp, the inverse of ucode skill add --mcp. It interactively removes skill schemas from the skills MCP connection across every configured agent. Removing downloaded skills is not supported yet, so the command requires --mcp.

How

  • remove_skills_command() reads each configured client's skills scope, offers the union of schemas via a picker (each annotated with the clients that carry it), and removes the selected schemas from every client that has them.
  • It reuses the per-client update path, so dropping the last schema leaves the schema-less connection registered rather than deleting it. It never adds or reconfigures anything and needs no Databricks auth.
  • cli.py gains the skill remove command and a status hint pointing at ug skill add / ug skill remove --mcp.

Tests

  • test_mcp.py: a selected schema is removed from every client and each is re-registered; removing all schemas keeps the schema-less connection; an empty scope is a no-op that never opens the picker; the picker is offered each client's real scope.
  • test_cli.py: skill remove without --mcp errors; skill remove --mcp dispatches the global removal.

uv run pytest tests/test_mcp.py tests/test_cli.py tests/test_lint.py is green.

Manual verification (installed build)

Ran against the installed build (0.1.0+91.g5c0dc1e) in the isolated sandbox (temp HOME, stub claude/codex binaries recording registrations, DATABRICKS_BEARER for offline auth), starting from state claude=[claude.only], codex=[shared.skills]. Invoked remove_skills_command() with the interactive picker stubbed to select shared.skills (the picker needs a TTY); all state I/O and per-client registration ran for real.

Check Result
Picker input Offered both clients' scopes: {claude:[claude.only], codex:[shared.skills]}
State after shared.skills dropped from codex; result {claude:[claude.only]} with codex's now-empty entry removed
Registrations Only codex re-registered, with the schema-less URL; claude untouched
Guard ucode skill remove (no --mcp) exits with ERROR Removing downloaded skills is not supported yet ...

Stacking

Third in the stacked per-agent skills series, based on xshen/skill-per-agent-status (#523). Per-agent remove builds on this next. Reviewing the diff against that base shows just this change. It rebuilds behavior originally designed by Arthur Jenoudet on the current per-client-map state model.

This pull request and its description were written by Isaac.

@xsh310
xsh310 force-pushed the xshen/skill-remove branch from fb94ba5 to 59d2fd5 Compare September 8, 2026 05:43
@xsh310
xsh310 force-pushed the xshen/skill-remove branch from 59d2fd5 to 0aa87df Compare September 8, 2026 18:25
@xsh310
xsh310 force-pushed the xshen/skill-remove branch from 0aa87df to 98934ea Compare September 8, 2026 19:09
@xsh310
xsh310 marked this pull request as ready for review September 8, 2026 21:09
@xsh310
xsh310 force-pushed the xshen/skill-remove branch from 98934ea to 4921ec5 Compare September 8, 2026 22:12
@xsh310
xsh310 force-pushed the xshen/skill-remove branch from 4921ec5 to 333fc68 Compare September 8, 2026 22:16
@xsh310
xsh310 force-pushed the xshen/skill-remove branch from 14e8dcd to bda1dc8 Compare September 8, 2026 23:09
@xsh310
xsh310 removed this pull request from stack #521 September 10, 2026 21:22
@xsh310
xsh310 added this pull request to stack #556 September 10, 2026 21:22
@xsh310
xsh310 force-pushed the xshen/skill-remove branch 2 times, most recently from 915b95c to 86e4fc9 Compare September 11, 2026 04:25
xsh310 and others added 2 commits September 11, 2026 05:27
Introduce `ucode skill remove --mcp`, the inverse of `ucode skill add --mcp`. It
shows the schemas in each configured client's skills scope, annotated with the
clients that carry them, and removes the selected schemas from every client that
has them. `remove_skills_command` reuses the per-client update path, so dropping
the last schema leaves the schema-less connection in place. It never adds or
reconfigures anything and needs no Databricks auth. Removing downloaded skills is
not supported yet, so the command requires `--mcp`. A status hint points at the
new add/remove commands.

Co-authored-by: Arthur Jenoudet <arthur.jenoudet@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
remove_skills_command called _update_skills_mcp with the default
print_summary=True, so a removal printed the full "Skills MCP registered"
block (Server / URL / Configured / Tools) before "Removed N skill schemas",
which reads wrong for a removal. Pass print_summary=False so a removal reports
only what it removed. The print_summary hook already existed for this case.

Co-authored-by: Arthur Jenoudet <arthur.jenoudet@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
@xsh310
xsh310 merged commit 73812bc into main Sep 11, 2026
4 of 6 checks passed
@xsh310
xsh310 deleted the xshen/skill-remove branch September 11, 2026 16:42
xsh310 added a commit that referenced this pull request Sep 11, 2026
## What

Adds an `--agents` option to `ucode skill remove --mcp` so a schema can
be removed from a chosen subset of configured agents and kept on the
rest, mirroring `ucode mcp remove --agents`. This is the core bug fix in
the series.

## The bug

Before per-agent scopes, an all-agent add wrote a shared layer while a
per-agent remove edited a separate one, so they did not compose: `skill
add --mcp X` (all) followed by `skill remove --mcp --agents claude` was
a silent no-op, and often printed a misleading "nothing to remove". Now
that developer scope is a single per-agent map, the two operations act
on the same layer.

## How

- `remove_skills_command(agents=None)` forwards `agents` to
`setup_mcp_clients`, which scopes the client set. The picker offers only
those clients' schemas and removal edits only their maps; other agents
keep the schema and are not re-registered.
- The "nothing to remove" note names the scope when `--agents` was
given.
- `cli.py`: `skill remove` gains the `--agents` option; without it,
removal stays global.

## Tests

- `test_mcp.py`: add-all then `remove --agents claude` removes the
schema for claude only and re-registers only claude; the picker is
offered only the named clients' scope; an empty named scope is a no-op.
- `test_cli.py`: `--mcp` forwards `agents=None`; `--mcp --agents
claude,codex` forwards the parsed set.

`uv run pytest tests/test_mcp.py tests/test_cli.py tests/test_lint.py`
is green.

### Manual verification (installed build)

This is the headline fix, so I verified the exact previously-failing
sequence end to end. In the sandbox (installed build
`0.1.0+91.g5c0dc1e`, stub `claude`/`codex` binaries recording
registrations, offline auth via `DATABRICKS_BEARER`), `shared.skills`
had been added to both agents. Then I invoked
`remove_skills_command(agents={"claude"})` with the picker stubbed to
select `shared.skills`:

| Check | Result |
|---|---|
| Picker input | Offered only the named client's scope:
`{claude:[shared.skills, claude.only]}` |
| State after | `shared.skills` removed from claude only:
`{claude:[claude.only], codex:[shared.skills]}` |
| Registrations | Only claude re-registered; codex untouched |

Before this change the same sequence was a silent no-op (add wrote a
shared layer, per-agent remove edited a different one). It now removes
for the named agent while the other keeps the schema.

## Stacking

Fourth in the stacked per-agent skills series, based on
`xshen/skill-remove` (#524). Reviewing the diff against that base shows
just this change. It rebuilds behavior originally designed by Arthur
Jenoudet on the current per-client-map state model.

This pull request and its description were written by Isaac.

Co-authored-by: Arthur Jenoudet <arthur.jenoudet@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
xsh310 added a commit that referenced this pull request Sep 11, 2026
## What

Documents the per-agent skills behavior this stack introduced but the
README did not yet cover:

- `--agents` scoping on `ug skill add --mcp` (add schemas to specific
agents; any not set up yet are configured first).
- `ug skill remove --mcp`, with `--agents` to scope removal to named
agents.
- Each agent's skill MCP scope now shown by `ug status`.

## How

- Added a "Remove skill scopes" subsection, plus an `--agents` example
and note in "Add skill scopes".
- Added quick-reference rows for the per-agent `skill add` and both
`skill remove` forms.
- Updated the `ug status` row to mention per-agent skill MCP scope.
- Aligned the existing `skill add` examples from `ucode` to `ug` for
consistency with the rest of the README.

Docs only; no code changes.

## Stacking

Stacked on `xshen/skill-per-agent-remove` (#525), the top of the
per-agent skills series. It documents the user-facing surface added
across #520, #522, #523, #524, and #525.

This pull request and its description were written by Isaac.

Co-authored-by: Isaac <no-reply@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants