Skip to content

feat(claude): the skill picker lists skills from enabled plugins - #175

Merged
ImZoomBoy merged 1 commit into
mainfrom
feat/claude-plugin-skills
Oct 9, 2026
Merged

ImZoomBoy merged 1 commit into
mainfrom
feat/claude-plugin-skills

Conversation

@ImZoomBoy

@ImZoomBoy ImZoomBoy commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Problem

The Claude skill picker listed only skills from <claude config dir>/skills and <cwd>/.claude/skills. A skill that ships in a Claude Code plugin never appeared, so it could not be picked.

Reproduction

On this PC the plugin frontend-design@claude-plugins-official is installed and enabled. The CLI lists its skill as frontend-design:frontend-design. The picker did not.

The new test lists the skills of an installed, enabled plugin under plugin:skill builds the same layout in a temp folder. On main it fails: discovery returns only the ordinary skill.

AssertionError: expected [ { name: 'mine', …(4) } ] to deeply equal [ { name: 'alpha:hello', …(4) }, …(2) ]

How the CLI resolves plugin skills

Read from Claude Code 2.1.296. I let the CLI install made-up plugins into a temp config folder, then read the skills and plugins lists in its stream-json init message for each case. That needs no signed-in session.

Case What the CLI did
enabledPlugins has true for the id Loaded
enabledPlugins has false Not loaded
Installed, id absent from enabledPlugins Not loaded
User false, project .claude/settings.json true Loaded
Project true, .claude/settings.local.json false Not loaded
Repository root settings, read from a nested cwd Ignored, both files
Skill folder dirname, frontmatter name: frontmatter-name Listed as plugin:dirname
plugin.json name differs from the id Prefix is the plugin.json name
No plugin.json Prefix is the id name
plugin.json does not parse Plugin not loaded
installPath folder missing Plugin not loaded
plugin.json skills names extra folders Read in addition to skills/
Skill folder nested two levels deep Not listed
installed_plugins.json malformed or missing No plugins, ordinary skills still listed
Project-scoped install, cwd in another project Not loaded
skillOverrides set to off for plugin:skill or the bare name Skill stays on
Two enabled plugins with the same name Both load, in enabledPlugins order

The invoked form, checked against the real CLI with the real plugin and a one-word prompt on a small model:

Prompt starts with Skill instructions loaded
/frontend-design:frontend-design Yes
/frontend-design No
$frontend-design:frontend-design No

So a pick has to go out as /plugin:skill.

Fix

  • New ClaudePluginSkills.ts finds the skill folders of installed, enabled plugins by the rules above.
  • ClaudeSkills.ts adds those folders after the user and project ones. Each skill is named plugin:skill with scope plugin. skillOverrides is not applied to them.
  • The outgoing rewrite needed no change. It already accepts a colon, so $alpha:hello goes out as /alpha:hello once the name is in the discovered list.
  • User and project skills are scanned first and the first hit for a name wins, as before. Among plugins the enabledPlugins order decides.
  • Every plugin file is read best-effort. A broken one drops only its plugin.

Upstream has no plugin skill support yet, checked on upstream/main at a1db449fe4. This is a fork change, so fork-features.json has a new entry, claude-plugin-skills.

Clients

  • Web $ picker: the row reads Frontend Design, with a Plugin Skill badge.
  • Web / menu: /skill:Frontend Design, with a Plugin badge.
  • Mobile: the row shows the raw name frontend-design:frontend-design and the label Plugin. Changed by code and typecheck only. I did not run the mobile app.
  • Command palette: it does not list skills, so nothing to change.
  • A plugin whose one skill repeats its name is shown once. Other plugin skills read as plugin then skill, for example Pdf Viewer Annotate.

Tests

ClaudePluginSkills.test.ts has 12 tests. They cover: enabled plugin listed, disabled and unnamed plugins hidden, settings precedence, malformed installed_plugins.json, malformed install entries and manifests, a settings file that does not parse, plugin.json naming and extra folders, project-scoped installs, same-name plugins, skillOverrides, and the /plugin:skill rewrite. providerSkills.test.ts gains two tests for the label. All use made-up plugin folders in a temp directory.

Each fails on main. Proof: I put main's ClaudeSkills.ts and providerSkills.ts back in place and ran the new tests.

$ vp test run src/provider/Drivers/ClaudePluginSkills.test.ts
     × lists the skills of an installed, enabled plugin under plugin:skill 42ms
     × hides a disabled plugin and one enabledPlugins does not name 24ms
     × lets project settings outrank user settings, and local settings outrank both 28ms
     × keeps ordinary skills when installed_plugins.json is malformed or missing 19ms
     × skips only the broken plugin when one install entry or manifest is malformed 36ms
     × ignores enabledPlugins in a settings file that does not parse 26ms
     × names the skills after plugin.json and reads the folders it adds 27ms
     × loads a project-scoped install only inside its project 20ms
     × uses the first install entry that applies to the workspace 28ms
     × keeps the first enabled plugin when two share a name 22ms
     × does not apply skillOverrides to a plugin skill 19ms
     × sends a picked plugin skill as the /plugin:skill command 17ms
 Test Files  1 failed (1)
      Tests  12 failed (12)
$ vp test run src/providerSkills.test.ts
     × marks Claude plugin skills as plugin installs 1ms
     × names a plugin skill once when the plugin and the skill share a name 3ms
AssertionError: expected 'Frontend Design Frontend Design' to be 'Frontend Design' // Object.is equality
AssertionError: expected 'other' to be 'plugin' // Object.is equality
 Test Files  1 failed (1)
      Tests  2 failed | 20 passed (22)

Real run

Dev server on a new empty home under the temp folder, web app in a headless browser, a small Claude model.

  1. Typed $front in the composer. The picker listed Frontend Design with the Plugin Skill badge.
  2. Picked it and sent: "If this message contains skill instructions about frontend design, reply LOADED. Otherwise reply NONE."
  3. The reply was LOADED.

The three disabled enabledPlugins entries on this PC did not appear. The server and the browser are stopped.

No before and after images are attached. I had no way to upload images to GitHub from this session.

Left out

  • Plugins synced from the Claude account. The CLI loads them from a synced folder and installed_plugins.json does not list them. On this PC that is pdf-viewer, cowork-plugin-management, modern-web-guidance and the anthropic-skills:* set. They still do not show in the picker.
  • A plugin whose marketplace the CLI no longer knows. The CLI refuses it. The picker would still list it.
  • A plugin from a local directory marketplace. The CLI reads the live directory. The picker reads the install copy.
  • A SKILL.md at the plugin root.
  • Which copy the CLI runs when two enabled plugins share a name. The picker keeps the first in enabledPlugins order. I could not check this without a signed-in session on made-up plugins.
  • Managed settings as a source of enabledPlugins. The path is read, but I could not test it on this PC.

Found on the way, not changed here: the CLI lists a user skill whose frontmatter does not parse, and the picker hides it.

Checks

Run on Windows, on the final commit.

$ vpr typecheck   (in apps/server, packages/client-runtime, apps/web, apps/mobile)
typecheck apps/server exit=0 errors=0
typecheck packages/client-runtime exit=0 errors=0
typecheck apps/web exit=0 errors=0
typecheck apps/mobile exit=0 errors=0

$ vpr lint
lint exit=0   (warnings only, none in the files this change touches)

$ vpr fmt:check
Finished in 2133ms on 4162 files using 24 threads.
fmt:check exit=0

$ vp run knip:check
knip:check exit=0

$ vpr test   (packages/client-runtime)
 Test Files  81 passed (81)
      Tests  1602 passed (1602)

$ vpr test   (apps/web)
web test exit=0

$ vpr test   (apps/mobile)
 Test Files  188 passed | 2 skipped (190)
      Tests  1706 passed | 5 skipped (1711)

$ vp test run --shard N/4   (apps/server, four batches)
1/4  Test Files  3 failed | 93 passed (96)              Tests  3 failed | 950 passed | 30 skipped (983)
2/4  Test Files  3 failed | 91 passed | 1 skipped (95)  Tests  3 failed | 1453 passed | 34 skipped (1490)
3/4  Test Files  3 failed | 90 passed | 2 skipped (95)  Tests  3 failed | 1443 passed | 24 skipped (1470)
4/4  Test Files  2 failed | 91 passed | 2 skipped (95)  Tests  8 failed | 1655 passed | 42 skipped (1705)

$ node scripts/check-fork-features.ts
 Test Files  1 failed | 33 passed (34)
      Tests  11 failed | 603 passed (614)

The server suite has 17 failures on this Windows PC and the fork features check has 11. None is in a file this change touches. Both new test files pass inside the fork features check.

16 of the 17 server failures also fail on main (8cd0f93d30), run from the same folder:

$ git checkout --detach origin/main
$ vp test run src/cli/update.test.ts src/provider/Layers/OpenCodeProvider.test.ts src/provider/userInputAttachments.test.ts src/git/GitManager.test.ts src/mcp/McpHttpServer.test.ts src/vcs/GitVcsDriverCore.test.ts src/device/LocalDeviceHost.test.ts src/project/AgentSessionScanner.test.ts src/vcs/GitVcsDriver.test.ts src/cloud/bootService.test.ts src/serviceLauncher.test.ts
 FAIL  src/cli/update.test.ts > t3 update launcher > finds the launcher a bare command name resolved to on PATH
 FAIL  src/cloud/bootService.test.ts > boot service install > install with start=false keeps the marker when a later write fails
 FAIL  src/cloud/bootService.test.ts > boot service install > install with start=false rewrites the files and marks a restart pending
 FAIL  src/cloud/bootService.test.ts > boot service install > installs, reports current state, and uninstalls
 FAIL  src/device/LocalDeviceHost.test.ts > puts detected Android tools on the helper PATH without losing existing commands
 FAIL  src/mcp/McpHttpServer.test.ts > saves the snapshot PNG on request and reports its path
 FAIL  src/project/AgentSessionScanner.test.ts > AgentSessionScanner > recentThreads > checks file identity and provider before skipping completed history
 FAIL  src/provider/Layers/OpenCodeProvider.test.ts > reads Go limits with the instance's XDG credentials and preserves reset times
 FAIL  src/provider/userInputAttachments.test.ts > question answer paths > keeps selected values intact and appends an actual readable server path
 FAIL  src/serviceLauncher.test.ts > service state persistence > a fresh launcher clears a restart deferred by t3 update
 FAIL  src/serviceLauncher.test.ts > service state persistence > commits only after the trial reports prepared
 FAIL  src/serviceLauncher.test.ts > service state persistence > restores the database when a migrating trial exits
 FAIL  src/serviceLauncher.test.ts > service state persistence > rolls back a trial that reports the wrong update ID
 FAIL  src/serviceLauncher.test.ts > service state persistence > serializes shutdown with launcher recovery
 FAIL  src/vcs/GitVcsDriver.test.ts > restores empty checkpoints without changing paths outside the workspace
 FAIL  src/vcs/GitVcsDriverCore.test.ts > GitVcsDriver core integration > review diff previews > keeps untracked filenames with pathspec magic in the review
 Test Files  10 failed | 1 passed (11)
      Tests  16 failed | 411 passed | 12 skipped (439)

The 17th is src/git/GitManager.test.ts > returns existing cross-repo PR metadata found under the bare branch name. It timed out at 12000ms inside batch 2. Run alone on this branch it passes:

$ vp test run src/git/GitManager.test.ts
 Test Files  1 passed (1)
      Tests  114 passed (114)

The 11 fork features failures are all in scripts/build-desktop-artifact.test.ts. The same 11 fail on main:

$ vp test run scripts/build-desktop-artifact.test.ts   (on origin/main)
 Test Files  1 failed (1)
      Tests  11 failed | 60 passed (71)

CI on this pull request runs the same suites on Linux.

The Claude skill picker scanned only the user skills folder and the
project's .claude/skills folder. A skill that ships in a Claude Code
plugin never appeared, so it could not be picked.

Discovery now also reads plugins/installed_plugins.json and the
enabledPlugins setting, and lists the skills of every plugin that is
installed and switched on. They are named plugin:skill, as the CLI
names them, and a pick goes out as /plugin:skill, the only form the CLI
runs. Web and mobile mark these rows Plugin.

A missing or malformed plugin file drops only that plugin. Ordinary
skills are listed as before.
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L labels Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ The exact PR base did not have a successful artifact. Baseline uses the latest successful main measurement shown below.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.5 KiB 13.5 KiB −4 B (−0.0%) 15.1 KiB ✅
Codex Thread snapshot wire 7.1 KiB 7.1 KiB +10 B (+0.1%) 7.3 KiB ✅
Codex Live turn WebSocket wire 6.4 KiB 6.4 KiB −14 B (−0.2%) 7.8 KiB ✅
Codex Live turn WebSocket decoded 56.1 KiB 56.1 KiB 0 B (0.0%) 66.4 KiB ✅
Codex Live turn messages 7 7 0 (0.0%) 21 ✅
Claude Total thread wire 13.5 KiB 13.5 KiB −33 B (−0.2%) 15.1 KiB ✅
Claude Thread snapshot wire 7.1 KiB 7.1 KiB −3 B (−0.0%) 7.3 KiB ✅
Claude Live turn WebSocket wire 6.4 KiB 6.4 KiB −30 B (−0.5%) 7.8 KiB ✅
Claude Live turn WebSocket decoded 57.0 KiB 56.9 KiB −44 B (−0.1%) 66.4 KiB ✅
Claude Live turn messages 8 7 −1 (−12.5%) 21 ✅

Baseline: d482468 · PR result: d6ab3a2 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 114.0 KiB
  • Claude decoded thread snapshot: 114.7 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@ImZoomBoy

Copy link
Copy Markdown
Owner Author

Code review, two axes. Each finding has a confidence and an estimated severity.

Standards

Standards review, PR 175: no code finding should block a merge. One hard breach sits in the pull request body, not the code.

Hard violations

  1. No before/after images. AGENTS.md "Pull requests" and CONTRIBUTING.md require them for UI changes. The body says none are attached. Confidence high, severity medium.
  2. Body does not end with the model and harness (AGENTS.md "Pull requests"). Confidence high, severity low.
  3. Module comment disagrees with the code (AGENTS.md "Taste", comments). The ClaudePluginSkills.ts header lists three settings files. skillOverrideSettingsPaths also returns the managed settings path, read last, so it wins. Confidence high, severity low.
  4. keep line is seven sentences. upstream-conflict-resolution.md calls it "one sentence". Other entries may do the same. Confidence medium, severity low.

Judgement calls on documented standards

  1. Claude naming rule in shared client code. formatProviderSkillDisplayName now splits on :. "Complexity belongs at the adapter boundary"; the adapter could set displayName. Confidence medium, severity low.
  2. Mobile differs from web ("Hit every surface"). Mobile shows the raw plugin:skill name. plugin and app share "square.grid.2x2". Confidence medium, severity low.
  3. Body length. The standard asks for the problem in a sentence or two. Confidence medium, severity low.
  4. Size. 731 lines against "Keep it small"; 423 are tests. Confidence low, severity low.
  5. Em dash in the new module header. It matches the ClaudeSkills.ts header. Confidence low, severity low.

Baseline smells

  1. Duplicated Code. readEnabledPluginIds repeats the read, decode, log, skip loop of readSkillOverrides. Each discovery now reads every settings file twice, which touches the performance rule. Confidence high, severity low.
  2. Duplicated Code. isInsideOrEqual repeats isInsideRoot in AntigravityAdapter.ts and the check in cli/update.ts. Confidence high, severity low.
  3. Mysterious Name. settingsPaths: skillOverrideSettingsPaths(...) now feeds enabledPlugins. The name no longer says what it holds. Confidence high, severity low.
  4. Primitive Obsession. root.namespace ? undefined : skillOverrides.get(name) uses a truthy string as the "is a plugin" flag, while scope: "plugin" already says it. Confidence medium, severity low.
  5. Mysterious Name. namespace is a new word for the plugin name. domain.md asks for glossary terms. Confidence low, severity low.
  6. Data Clumps. path, configDirPath, cwd, platform, environment travel together into skillOverrideSettingsPaths at two call sites. Confidence medium, severity low.
  7. Nested ternary for extraFolders. Confidence low, severity low.
  8. Test fixture. "uses the first install entry" rewrites JSON with .replace('"placeholder"', ...). manifest?: unknown carries three meanings. Confidence medium, severity low.

Clean

  • Tests assert behaviour and use no sleeps.
  • Effect.fn return annotations match the file's idiom.
  • test as an array is valid in fork-features.json.
  • The user doc keeps product voice.

Blocking: none of the code findings. Finding 1 is the only one a merge gate could hold on.

Notes: this fork leaves the model and harness line out of pull requests on purpose (finding 2). The screenshots from the real run were not uploaded (finding 1); the body describes them.

Spec

Spec review: PR 175

No finding blocks a merge. One needs a decision first (finding 1). I read the code and the PR body. I ran no tests.

(a) Missing or partial

  1. Synced plugins are still hidden. Spec: "The user asked why Anthropic skills are not showing". The PR leaves out plugins synced from the Claude account, which includes the anthropic-skills:* set. Only frontend-design appears. The worker did not declare needs-decision. Confidence high, severity medium.
  2. Same-name plugin precedence is unproven. Spec: "Name collisions follow the CLI's own precedence." The picker keeps the first id in enabledPlugins order. The PR says it could not check which copy the CLI runs. Confidence high, severity low.
  3. Checks do not all pass. Spec: "the fork features check, and the test suites ... pass." The server suite has 17 failures and the fork features check has 11. The PR shows 16 and 11 also fail on main. GitManager.test.ts timed out in the batch and passed alone, so it looks flaky. Confidence high, severity low.
  4. Some outputs are summaries. Spec: "pastes the real commands and what they printed". lint exit=0 and web test exit=0 are not printed output. Confidence high, severity low.
  5. Mobile shows the raw name. Spec: "the plugin label reads well in each, or say which you left". Mobile shows frontend-design:frontend-design with Plugin. The PR says so, and mobile was not run. Confidence high, severity low.
  6. Managed settings untested. enabledPlugins is read from the managed settings file, with no test and no CLI check. Confidence high, severity low.

(b) Not asked for

  1. formatProviderSkillDisplayName now collapses x:x to X for every provider, not only Claude. Confidence high, severity low.
  2. docs/user/providers-claude.md gained a paragraph. It fits the repository rules. Confidence high, severity low.

(c) Implemented, looks wrong

  1. Folder check is by path text only. isInsideOrEqual in ClaudePluginSkills.ts:93 blocks ../outside, and a test covers it. A symlink inside the plugin folder can still point outside. A folder named ..foo is wrongly rejected. Confidence medium, severity low.
  2. Scan cost. discoverClaudeSkills runs on every send (ClaudeAdapter.ts:5383) and every snapshot. Each enabled plugin adds a manifest read, a folder read, and a full read of each SKILL.md. Settings files are now read twice. There is no cache. Confidence high, severity low.
  3. Tests read one real file. Spec: "never the user's real files." Discovery reads the machine's managed settings path under PROGRAMDATA during tests. A real file there could change results. Confidence medium, severity low.
  4. Same display name twice. A user skill frontend-design and the plugin skill both show Frontend Design. Only the badge differs. Confidence high, severity low.
  5. Nested project install. A project-scoped install loads from a nested folder. The PR table only shows the other-project case checked. Confidence low, severity low.

Verified fine

  • Enabled and installed only. Absent and false ids stay hidden.
  • Merge order is user, project, local.
  • Malformed files never empty the ordinary list.
  • A pick goes out as /plugin:skill.
  • fork-features.json names the new files and tests.

Outcome: no finding blocks the merge. Spec finding 1 (plugins synced from the account stay hidden) goes forward as a follow-up, not a change to this pull request.

@ImZoomBoy
ImZoomBoy merged commit 9f064e0 into main Oct 9, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant