Skip to content

feat(plugins): plugins, MCP, and skills management on the provider-plugin model - #344

Merged
AprilNEA merged 33 commits into
masterfrom
chenyu/code-487
Aug 1, 2026
Merged

feat(plugins): plugins, MCP, and skills management on the provider-plugin model#344
AprilNEA merged 33 commits into
masterfrom
chenyu/code-487

Conversation

@lucas77778

@lucas77778 lucas77778 commented Jul 30, 2026

Copy link
Copy Markdown
Member

Builds the client-facing management surface for provider plugins, user-owned custom MCP servers, and skills on top of the provider-plugin model that landed with CODE-23/403/432 (which was backend-only: no wire contract, no UI, managementCapabilities hardcoded all-false).

Supersedes the closed #261 / CODE-382~387 batch — that design invented a parallel "capability unit + connector" concept in the same paths. Design doc: Plugins / MCP / Skills Management.

What ships

Wire (protocol 67; minimum compatible 67)

  • plugin.list.get/result carrying Plugin[], standalone skills, and providerStatus[] — so the UI can tell "this agent has no plugins" from "its CLI failed".
  • plugin.set-enabled/plugin.updated and skill.set-enabled/skill.updated, both replying with the re-read entity so clients patch one cache entry instead of re-running discovery.
  • config.get/config.set gain custom MCP servers: masked reads (envKeys/headerKeys, never values) and per-key patch ops.
  • session.started gains optional mcpWarnings.

Provider management, honestly capability-gated

  • Claude plugin enable/disable via claude plugin enable|disable -s <scope>; Codex enable/disable via config/value/write on plugins."<id>".enabled.
  • Codex plugin install/uninstall via app-server plugin/install and plugin/uninstall; Claude installation remains unimplemented by this adapter.
  • Per-skill toggles: Claude writes skillOverrides in settings.json (what the TUI /skills dialog writes; read-modify-write preserving every other key and any finer name-only/user-invocable-only tier the user set), Codex calls skills/config/write.
  • Both providers blind-write — a nonexistent plugin/skill still reports success — so every write is followed by a re-read, and that readback is the only success check. Covered by tests.

Custom (BYO) MCP servers

  • Persisted in ~/.linkcode/config.json (0600, per-entry tolerant parse), injected into MCP-capable sessions' StartOptions.mcpServers alongside the existing simulator endpoint, with agent-unsupported/name-conflict warnings instead of silent drops.
  • Writes are patch ops with per-key secret semantics (blank = keep, typed = replace, removed = delete). A whole-array replace would clobber real secrets with mask placeholders, so it is structurally impossible here. buildCustomMcpPatch is the highest-value unit test in the batch.

Settings page — one "Plugins & Skills" category on desktop and webview, four tabs: Plugins (installed only, provider-grouped, capability-gated switches), Market (uninstalled marketplace listings), MCP (custom servers + read-only plugin-provided ones), Skills (plugin + standalone, per-skill switches). Manual refresh only — discovery is a real CLI shell-out.

Verification

  • pnpm check:ci + pnpm test green on the current head (2341 tests).
  • Dev-mock Playwright pass, 17/17: card rendering, toggle round-trip, secret never reaching the DOM, blank-save preserving stored secrets, add/edit/remove, empty states.
  • Live against a real daemon before the latest rebase (then wire 64): 469 real plugins + 70 skills discovered; claude plugin disable/enable round-trips on the real CLI; a real skill toggle writes skillOverrides and restoring it removes the key; ~/.claude/settings.json diffed byte-identical to a pre-test backup afterwards.

Two real bugs were found by that live pass and are fixed here rather than left for later:

  • Discovery was broken for both providers on any current install (CODE-505). claude's available[].source became a union (bare path string in 52 of 275 entries, object with its own source discriminator in the rest) and its version is null for 262 of 275; codex omits version entirely while the schema required the key present. One bad entry failed the whole array, so the page was empty everywhere. Regression fixtures now cover every observed shape.
  • Two secret-row inputs inside one base-ui Field both rendered name="secrets.0.value" (Field owns a single control), silently dropping the key field from the form.

Deliberately out of scope

Managed/HQ connectors and OAuth (CODE-94/96/340 untouched); plugin update and Claude install/uninstall (Codex install/uninstall ships here); Claude's name-only/user-invocable-only skill tiers have no wire representation yet (preserved, not coarsened); retrofitting the same masking onto accounts, which still round-trips plaintext.

The Market list renders a bounded 60 entries per provider with a visible "showing 60 of 273 — narrow it down with search" note rather than truncating silently; virtualizing it is a follow-up if browsing the full catalog matters.

Closes CODE-487, CODE-488, CODE-490, CODE-491, CODE-492, CODE-493, CODE-494, CODE-495, CODE-496, CODE-497, CODE-502, CODE-503, CODE-504, CODE-505.

# Conflicts:
#	apps/daemon/src/index.ts
#	packages/foundation/schema/src/wire/message.ts
Copilot AI review requested due to automatic review settings July 30, 2026 12:29
@linear-code

linear-code Bot commented Jul 30, 2026

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Codex has no plugin enable/disable RPC and no such CLI subcommand — enablement is
the config value plugins."<id>".enabled, which config/value/write persists into
config.toml. Install and uninstall ride plugin/install and plugin/uninstall, which
share plugin/read's marketplace addressing rule.

Toggling resolves its target through plugin/installed first, so a caller-supplied
id never reaches the TOML key path and an uninstalled plugin fails here instead of
leaving a stray config entry.
Bumps the wire protocol: two new requests plus an optional pendingAuthApps on
plugin.updated, which carries the provider apps an install left unauthorized —
codex reports them for most of its catalog and LinkCode runs no OAuth flow.

The router had no case for the new kinds and its default arm is a silent
Effect.void, so an unrouted request would hang the client with no error.

Every mutation now ends in one shared readback: the providers blind-write, so
re-listing is the only proof the change landed. An uninstalled plugin stays in
the marketplace catalog with no installations, so both directions reply with the
same shape.
The three plugin mutations share one reply kind, so they share one pending tag;
setPluginEnabled now resolves with the mutation result rather than a bare plugin.

Market gains an Install action and Plugins a confirmed Uninstall, both gated on
the plugin's own managementCapabilities so claude cards stay read-only. Neither
needs its own cache handling: the Plugins/Market split is derived from
installations, so patching the one replaced entry moves it between tabs.

An install that leaves apps unauthorized says so in a toast instead of reading as
finished.
The remote curated catalog listed `metabase` twice among 2321 live entries. Ids
are the normalized model's identity, so a duplicate reaches the client as two
cards sharing one key; keep the installed copy.
The live remote catalog is 2300+ entries and plugin/read costs ~160ms each, so
detailing the whole catalog blew the 30s discovery deadline and the Plugins page
reported codex discovery as failed. Measured on 0.144.1: plugin/list 3.3s,
plugin/installed 14ms, one plugin/read ~160ms.

Market cards now carry no component list; their identity, description, category
and keywords come from the catalog summary, which already has them.
Copilot AI review requested due to automatic review settings July 30, 2026 13:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@lucas77778
lucas77778 requested review from AprilNEA and Zerlight July 30, 2026 14:01

@Zerlight Zerlight left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found three high-priority safety issues and seven correctness or UX issues. The most serious problems are that custom MCP credentials bypass the existing secret vault, while Claude skill toggles target the wrong settings layer and can weaken settings-file permissions. Please address the inline comments before merging.

Verification: 136 focused tests passed. In the full suite, 2,312 tests passed and one unrelated timeout passed immediately when rerun in isolation. The wire-version change from 66/66 to 67/67 is consistent with the new required config.get.result.customMcpServers field.

Comment thread apps/daemon/src/config.ts Outdated
Comment thread packages/host/agent-adapter/src/plugins/claude-code.ts Outdated
Comment thread packages/host/agent-adapter/src/plugins/claude-code.ts Outdated
Comment thread packages/foundation/schema/src/model/custom-mcp.ts Outdated
Comment thread packages/host/engine/src/agent/request-handler.ts
Comment thread packages/host/agent-adapter/src/plugins/codex.ts Outdated
Comment thread packages/presentation/ui/src/shell/plugins/plugins-tab.tsx Outdated
Comment thread packages/client/workbench/src/settings/plugins/view.ts Outdated
Comment thread packages/client/workbench/src/settings/plugins/plugins-settings.tsx Outdated
Comment thread packages/client/workbench/src/settings/plugins/mcp-settings.tsx Outdated
Copilot AI review requested due to automatic review settings August 1, 2026 02:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Human review recommended

It introduces a wide cross-layer wire/schema + daemon persistence/secret-handling + client/UI surface area change where small mismatches can cause hard-to-debug compatibility and data-loss issues.

Review details

Suppressed comments (2)

packages/foundation/schema/src/wire/message.ts:16

  • The wire-version bump here (66 → 67) doesn’t match the PR description (“Wire (62 → 64)”). Either the PR description needs updating (likely after rebases), or these constants need to reflect the intended version bump so reviewers/users aren’t misled about required lockstep rebuilds.
    packages/presentation/ui/src/shell/plugins/plugins-shell.tsx:20
  • This doc comment says “three tabs”, but the component renders four tabs (Plugins/Market/MCP/Skills).
  • Files reviewed: 79/79 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@lucas77778
lucas77778 requested a review from Zerlight August 1, 2026 02:33

@Zerlight Zerlight left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the first review. The earlier UI, schema, and persistence issues are largely resolved, and the focused verification is green. Three correctness issues remain: custom MCP persistence is not crash-consistent across the vault/config pair, Codex install and uninstall have no shutdown cancellation path, and the Claude settings rewrite still narrows some existing file modes through the process umask. Please address the inline comments before merging.

Comment thread apps/daemon/src/config.ts Outdated
Comment thread packages/host/agent-adapter/src/plugins/codex.ts Outdated
Comment thread packages/host/agent-adapter/src/plugins/claude-code.ts
Copilot AI review requested due to automatic review settings August 1, 2026 10:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

There are user-visible text/grammar and documentation mismatches (and an apparent PR-description wire-version mismatch) that should be corrected before approval.

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

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (5)

packages/presentation/ui/src/shell/plugins/plugins-shell.tsx:20

  • The doc comment says this page has "three tabs", but the component renders four (plugins/market/mcp/skills). This can confuse future edits and reviewers.
    packages/presentation/ui/src/shell/plugins/skills-tab.tsx:22
  • This doc comment says standalone skills are "display-only" and that skills are grouped by plugin, but the UI renders standalone skills with toggles (when supported) and does not actually group by plugin in the list.
    packages/presentation/ui/src/shell/plugins/custom-server-list.tsx:21
  • The comment says "LinkCode-owned" custom MCP servers, but these are user-owned (BYO) servers; LinkCode-owned servers are the simulator/browser tool endpoints and should stay reserved internally. The current wording conflicts with the UI copy about "servers you add here are yours".
    packages/presentation/i18n/src/locales/en.ts:757
  • The English strings use plural component kinds ("skills", "commands", …) with a naive "{count} {kind}" template, which will render ungrammatical text for singular counts (e.g. "1 skills"). Consider switching to singular kind labels + ICU pluralization.
    packages/foundation/schema/src/wire/message.ts:16
  • PR description claims a wire bump "62 → 64", but this diff sets WIRE_PROTOCOL_VERSION / MIN_COMPATIBLE_WIRE_VERSION to 67. Please align the PR description with the actual wire version being shipped (or adjust the constants if 67 is unintended), since this affects upgrade sequencing across daemon/clients.
  • Files reviewed: 79/79 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Copilot AI review requested due to automatic review settings August 1, 2026 10:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

It introduces a few user-visible correctness issues (missing i18n keys referenced by settings search, and a semantic change that forces mcpServers: [] even when no MCP config was provided) that should be resolved before merging.

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

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (3)

packages/client/workbench/src/settings/search.ts:102

  • Settings search keywords reference plugins.componentKind.*, but those message keys don’t exist in the i18n locales added in this PR, so the settings sidebar search will show raw keys or throw depending on use-intl config. Use existing, defined strings (tab titles / section titles) for plugin-related keywords, or add the missing messages to every locale.
    packages/host/engine/src/session/start-options-resolver.ts:120
  • withCustomMcpServers always writes mcpServers: servers even when options.mcpServers was undefined and nothing was actually injected (e.g. all custom servers skipped due to conflicts/unsupported). This changes the semantic from “no MCP config” to an explicit empty list and can leak into provider adapters; preserve undefined when the merged list is empty and the caller didn’t provide one.
    packages/presentation/ui/src/shell/plugins/custom-server-list.tsx:47
  • The custom MCP section shows the “empty” state while rows is still undefined (initial load). This conflates loading with a true empty list and can flash misleading UI; treat rows === undefined as a loading state (render nothing/skeletons) and only show empty when rows is defined and length is 0.
  • Files reviewed: 79/79 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Copilot AI review requested due to automatic review settings August 1, 2026 10:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings August 1, 2026 10:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copy link
Copy Markdown
Member Author

Addressed all findings from Copilot’s two completed reviews:

  • 2f84c5e7: corrected stale comments/copy, ICU component plurals, and the PR wire-version description.
  • 2bdf2100: restored the singular componentKind locale keys used by settings search while retaining plural-aware counts.
  • 9320b3e8: preserves an absent mcpServers value when every custom server is skipped, and renders a skeleton rather than the empty state while custom MCP data is loading. Added regression coverage for both semantics.

Validation: pnpm check:ci (the final lint pass used the equivalent serial pnpm lint:ci after an auto-concurrency worker OOM), pnpm test (2342 passed), and the webview production/mock browser smoke all pass.

The latest requested Copilot re-review did not run because the requester had reached the Copilot review quota.

@lucas77778
lucas77778 requested a review from Zerlight August 1, 2026 10:59
Amp-Thread-ID: https://ampcode.com/threads/T-019fbc9e-9054-709c-ae76-594060e29be2

# Conflicts:
#	apps/daemon/AGENTS.md
#	apps/daemon/src/__tests__/config.test.ts
#	apps/daemon/src/config.ts
#	apps/daemon/src/provider-store.ts
#	packages/host/engine/src/agent/provider-config.ts
Copilot AI review requested due to automatic review settings August 1, 2026 11:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@AprilNEA
AprilNEA merged commit cc7ca6c into master Aug 1, 2026
10 checks passed
@AprilNEA
AprilNEA deleted the chenyu/code-487 branch August 1, 2026 11:36
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.

4 participants