🤖 feat: manage installed plugin component selections - #4213
Conversation
Use canonical plugins.json selection snapshots at discovery, startup, and call authorization boundaries. Preserve retained server connections and retire only removed components, deferring leased clients until release. Remember managed installation provenance so missing registry rows cannot reclassify hidden sibling components as unmanaged. Validation: 583 unit tests passed (1 Windows-only skip); typecheck, touched ESLint, Prettier, and git diff --check passed. No UI or public API changes. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `coder:openai/gpt-6-astra` • Thinking: `xhigh` • Cost: `$4.69`_ <!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=xhigh costs=4.69 -->
Replace additive imports with full, consent-bound selections for installed plugins. Preserve plugin trees, data, identities and workspace MCP preferences while reporting post-save cleanup failures as warnings. Add reversible inline management, conflict/lost-response recovery, full-App stories and documentation. Validation: 591 unit/sibling tests passed (1 platform skip), 10 IPC/UI tests, 6 Storybook interactions, make static-check and make static-check-full passed. Recorded desktop/mobile sandbox UAT remains with the integrating parent. --- _Generated with `xum` • Model: `coder:openai/gpt-6-astra` • Thinking: `xhigh`_ <!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=xhigh -->
Skip runtime reconciliation when component selection is effectively unchanged. Keep failed retired clients owned for retry, report cleanup failures from explicit saves, and leave automatic MCP boundaries non-disruptive. Validation: 589 tests passed (1 Windows-only skip), typecheck, touched ESLint, formatting, and diff checks passed. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `coder:openai/gpt-6-astra` • Thinking: `xhigh` • Cost: `$24.47`_ <!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=xhigh costs=24.47 -->
Keep card counts in sync with the refreshed editor after a stale selection is rejected. Cover both skill and MCP counts with the real-IPC UI regression. Validation: 716 unit tests (1 platform skip), 10 IPC/UI tests, 6 warmed Storybook interactions, static-check, and static-check-full passed. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `coder:openai/gpt-6-astra` • Thinking: `xhigh` • Cost: `$76.33`_ <!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=xhigh costs=76.33 -->
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
This comment has been minimized.
This comment has been minimized.
|
@codex review |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6deceb6213
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🛡️ Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 6deceb6213
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
Refresh availability consumers and outer plugin counts when the selection write loses its response and the confirming inventory read also fails. Retain the uncertain status and pending draft, and do not invalidate definite failed saves. Add real-IPC full-App regressions for persisted removals and definite failures, including availability, counts, reopen behavior and no automatic resubmission. Validation: 11 plugin UI tests passed; make typecheck, scoped ESLint, Prettier and git diff --check passed. Full integration/static gates remain parent-owned. --- _Generated with `xum` • Model: `coder:openai/gpt-6-astra` • Thinking: `xhigh` • Cost: `$24.35`_ <!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=xhigh costs=24.35 -->
Extend the installed-plugin regression to prove that updates preserve raw selections for absent components, unavailable desired names remain invalid, and explicit management replaces those selections. A later update restoring the component must not silently re-import it. Document the Clear/empty-consent rationale beside effectiveImports without changing selection semantics. Validation: 134 installer tests and 11 full-App plugin UI tests passed; make typecheck, scoped ESLint, Prettier and git diff --check passed. --- _Generated with `xum` • Model: `coder:openai/gpt-6-astra` • Thinking: `xhigh` • Cost: `$36.19`_ <!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=xhigh costs=36.19 -->
Stop retired leased clients during matching plugin tree invalidations without adding deselected restart candidates. Use existing idle ticks to retry failed retirement before evicting its ownership. Recheck managed component policy after named-test preparation and before admitting the connection test. Validation: manager and plugin/config/DI sibling suites, repository typecheck, touched ESLint, formatting, and diff checks passed. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `coder:openai/gpt-6-astra` • Thinking: `xhigh` • Cost: `$37.78`_ <!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=xhigh costs=37.78 -->
Hold the shared plugin mutation lock from a fresh policy read through synchronous tool, prompt, and named-test dispatch. Try it under the override fence so a plugin writer can still prune overrides; deny managed admission when the fence is missing or contended. Cover real old-inode reads against component setters, override-prune progress, and exactly-once cleanup after read failure, timeout, abort, and late lock acquisition. --- _Generated with `xum` • Model: `coder:openai/gpt-6-astra` • Thinking: `xhigh` • Cost: `$73.96`_ <!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=xhigh costs=73.96 -->
|
@codex review |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5cc72a292
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🛡️ Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: f5cc72a292
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
Status: merge blocked; review loop pausedCurrent head: Functional CI and the final-head local/UAT gates passed, but the latest code/security reviews found four verified defects. This reaches the six-round review limit, with further findings in the same runtime area. No merge-queue submission, new fixes, thread resolutions, or additional review requests were made after this cycle. Finding disposition
All six new threads remain open while this pause is resolved. None of these deferred defects is claimed fixed. Proposed bounded continuation
The independent readiness pass found no blocker from the earlier evidence. The later external findings supersede that recommendation. The strategic advisor recommends this pause, not an unreviewed late security patch. Verified evidence on this exact head (not a readiness claim)
Final-head UAT screenshots and recordingsDesktop removal and mixed re-add recording: runtime-desktop.webmMobile stale-save conflict recording: mobile-stale-conflict.webmGenerated with |
Reuse the component writer try-lock after the override fence so queued stdio and remote startups cannot launch a removed managed component. Retain the bounded stdio abort and remote initiation lock lifetimes. Validation: 674 tests passed, one Windows-only skip; typecheck, targeted ESLint, Prettier and diff checks passed. --- _Generated with `xum` • Model: `coder:openai/gpt-6-astra` • Thinking: `xhigh`_ <!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=xhigh -->
Bracket component inventory with the existing discovery journal/epoch gate instead of holding the plugin writer lock across full-tree hashes. Reject installer transactions overlapping the scan, including complete reinstall with an unchanged final content receipt. Keep component updates under the existing SHA, content-hash and selection compare-and-swap lock. Validation: 678 tests passed, one Windows-only skip; final make static-check passed with one ESLint worker. Regression tests reproduce both contention and concurrent-installer receipt invalidation. --- _Generated with `xum` • Model: `coder:openai/gpt-6-astra` • Thinking: `xhigh` • Cost: `$103.63`_ <!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=xhigh costs=103.63 -->
Refresh outer plugin counts when an acknowledged selection write cannot be confirmed by the inventory read. Confirm the current tree and raw persisted selection before showing success; if another writer supersedes the save, show its current selection and counts without automatically resubmitting. Add deterministic full-App regressions for failed confirmation and subsequent writers, including restoration of the original baseline and explicit retry. Validation: 15 plugin UI/IPC tests passed; make typecheck, scoped ESLint, Prettier and git diff --check passed. Both defects were reproduced test-first. --- _Generated with `xum` • Model: `coder:openai/gpt-6-astra` • Thinking: `xhigh` • Cost: `$50.51`_ <!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=xhigh costs=50.51 -->
|
@codex review |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6680946720
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6680946720
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Transfer deselected startup clients whose close fails into the published workspace entry's existing retired-client set. Cover initial, additive, timed-out retry and leased restart publication without exposing retired clients or reusing them when a component is re-added. Validation: six deterministic regressions failed before the fix; 684 manager/sibling tests passed with one Windows-only skip. Sequential make typecheck and make static-check passed. --- _Generated with `xum` • Model: `coder:openai/gpt-6-astra` • Thinking: `xhigh` • Cost: `$119.29`_ <!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=xhigh costs=119.29 -->
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
The Codex aggregate summary still includes a historical advisory appendix. Both linked findings are fixed and their actual threads are resolved: selection-read fence and normal MCP launch fence. The latter’s summary badge is stale. For unchanged commit I am marking only that stale aggregate comment as resolved (minimized). The repository’s comment gate otherwise treats any historical findings appendix as an unresolved regular comment. No finding or clean verdict is being removed, and no gate code or policy is being changed. The original links above preserve the review record. I will rerun the local review gates and required CI before queueing the merge. Generated with |


Summary
Replace installed-plugin Add components with Manage components. Add or remove imported skills and MCP servers, including clearing both groups, without uninstalling the plugin or deleting its files, data, or saved workspace MCP preferences.
Implementation
setComponentsoperation. Validate the reviewed Git SHA, full-tree content receipt, and nullable raw selection baseline before writing the desired set. Preserve legacy import-all behavior on no-op.Validation
Verified on
f2609e183f50f3a11af1215ec4342f9b45a74667:make static-check,make static-check-full, andgit diff --checkpassed on this head. The independent final code review found no concrete blocker. Current-head external CI and code/security reviews remain separate merge gates.668094672; UI code is unchanged on this head. Recorded desktop 240.3s and mobile 121.0s final frames were checked. Deterministic failure-injection races are automated-test evidence, not claimed as manual reproductions.Current-head startup-cleanup regression evidence
Recorded the actual six-case failure-injection test run on
f2609e183: 6 passed, 0 failed, exit 0. The accelerated recording is 5.0s; its final frame was checked. This is recorded automated regression evidence, not a manually induced process-close failure.Recorded startup-cleanup regression run:
regression.webm
Desktop/mobile UAT (
668094672; UI unchanged)Desktop removal, mixed re-add, and open-panel runtime observations:
desktop-runtime.webm
Mobile stale-save rejection and refresh:
mobile-conflict.webm
Earlier implementation evidence
Earlier validation
make static-check,make static-check-full, andgit diff --checkpassed again on the rebased publication head before push.52046f230build. Its feature commits were rebased without conflicts onto current main before publication; the feature patch is unchanged.Screenshots
Desktop and mobile conflict recovery
The attached videos cover selective removal/re-add, Cancel/empty selection, and corrected mobile stale-save recovery.
Earlier recordings
final-mixed-selection.webm
final-cancel-empty.webm
corrected-mobile-conflict.webm
Runtime boundary
Only skills and MCP component selection changes. Initial install, update, uninstall, agents, hooks, workflows, and slash commands retain their existing behavior. Calls already admitted before the selection commit may finish normally. Idle sibling processes reconcile at their next MCP boundary; immediate cross-process process termination is not promised.
Regression risk: Managed MCP admission and cleanup are the sensitive paths. Real mutations can temporarily deny admission; read-only inventory must not. Deterministic transport/installer races and retained-process UAT cover those boundaries. No new persisted fields, journal files, or locking subsystem are introduced by these review fixes.
📋 Implementation Plan
The plan below is the accepted implementation snapshot. Publication and conditional merge-queue submission were authorized afterward.
Reversible installed-plugin component management
Goal and scope
Replace the installed-plugin Add components panel with Manage components. Users can add components or deselect previously imported skills and MCP servers, then explicitly save the new selection.
Assumption: removing a component means removing it from Xum's available plugin components, not deleting the plugin's installed source files. Keeping the plugin installed makes re-adding possible without fetching again. Empty selections are valid. Initial installation remains unchanged. Scope matches the existing chooser: skills and MCP servers only; agents, hooks, workflows, and slash commands are unchanged.
Preserve workspace MCP enablement, tool allowlists, identity, and data. New MCP imports remain disabled without a workspace opt-in. Re-adding a previously enabled server can restore that saved opt-in; the panel must say so.
Verified foundations
PluginsSettingsSection.tsx:ComponentChooserlocks imported checkboxes;AddComponentsPanelmodels additions only.agentPlugins/installService.ts:addComponentsunions selections underrunExclusive; the registry already supports explicit empty allowlists.agentPluginInstalls.ts: absentimportedComponentsmeans legacy import-all. Cancel/no-op must preserve that distinction.getComponentsreturns the installed SHA, full-tree content hash, inventory, and current imported selection. SHA/hash do not change when another panel changes only the registry selection.loadPluginMcpServersalready filter by the selected allowlist. Active MCP connections require additional targeted reconciliation; blanket plugin teardown would disrupt retained servers.User experience
Implementation sequence
Deliver two dependent, reviewable increments. Stabilize increment 1 before exposing removal in increment 2. No PR creation or publishing is authorized by this plan.
1. Make MCP component removal safe at runtime
Estimated net product code: +180–300 LoC. No UI changes in this prerequisite.
Files:
src/node/services/mcpServerManager.ts,src/node/services/agentPlugins/registry.ts,src/node/services/agentPlugins/mcpConfig.ts, andCoreServicesRootLiveinsrc/node/services/di/layers/core.ts(the existingpluginInvalidationwiring).plugins.jsonas the cross-process authority. Include managed installation identity and selected MCP names. Extend the existing stable-scan/call-time invalidation seam; do not create another epoch file, use mtime-only checks, or bump the global tree epoch.info.plugin.serverName, not the composite key, for allowlist membership. Reuse existing managed-home/alias identity rules. A missing registry entry must not reclassify an already managed server as unmanaged. Unreadable managed policy fails closed without restricting genuinely unmanaged plugins. Cover those boundaries in tests.computeSignatureEntries; do not manually patch signature strings. Leave existing behavior for unrelated configuration changes intact.Runtime contract: An invocation whose call-time authorization check occurs after the selection commit must reject a deselected server, even through an old tool object in another process. “Admitted” means an actual invocation passed that gate—not that its tool definition was advertised. Already-admitted calls may finish or fail normally. Existing prompts cannot be retroactively edited. Idle sibling processes reconcile at their next MCP boundary; immediate cross-process process termination is not promised. Keep registry reads within the existing bounded gate and check their latency in the targeted runtime tests.
Gate: The targeted MCP/registry/discovery tests pass. Prove retained connection identity, not merely that a new connection works. No API for MCP removal ships before these checks are green.
2. Ship replacement selection and the management panel
Estimated net product code: +90–170 LoC. Depends on increment 1. Includes backend, UI, tests, and updated user docs.
agentPlugins.setComponents. Updatesrc/common/orpc/schemas/api.ts,src/node/orpc/router.ts, andsrc/node/services/agentPlugins/installService.ts. Frontend/backend are version-synchronized, so do not retain an unused additive compatibility endpoint.expectedLockedSha,expectedContentHash, required nullableexpectedImportedComponents, and desiredimportedComponents.nullbaseline means the legacy absent field. Compare selections as normalized sets; preserve absent versus explicit empty.runExclusive, read strict registry state, check all three baselines, validate desired names against the verified installed inventory, and atomically replace onlyimportedComponents.PluginsSettingsSection.tsx. RenameAddComponentsPanel, use a full-selection draft, unlock imported rows, and implement the interaction above. Keep the shared initial-install chooser behavior intact. Capture the raw baseline separately from the displayed effective selection.src/browser/utils/commands/sources.ts,pluginsSectionIntents.ts, and their command-ID definition/callsites. Preserve Tab/Space/Enter operation. Updatesrc/browser/stories/mocks/orpc.tsandsrc/browser/stories/App.pluginImports.stories.tsxto use replacement semantics.docs/agents/agent-skills.mdx, replace the one-way-import limitation. Explain deselection, preserved data/preferences, empty selections, and re-add behavior. Do not add a new documentation page.Gate: Backend, real-IPC UI tests, full-App Storybook interactions, and static checks pass. Opening the panel without edits must not mutate the registry. Initial installation, update, and uninstall regression tests remain green.
Acceptance criteria
Validation and dogfooding
Automated gates
Run
src/**suites with Bun. Runtests/**suites with Jest. Include sibling suites for every touched production module.Extend full-App stories only. Cover saved removals, empty selection, and conflict recovery. Pin the phone story with
parameters.pixel.matrix.viewports: ["phone"]and matchingglobals.viewport. Keep breakpoint assertions valid in the desktop-sized Storybook test runner by using the existing fixed-width/guarded pattern.Hands-on walkthrough
installService.test.tsfixture patterns. Include two skills and two harmless local MCP servers with observable process identities. Add a separate temporary workspace/project. No provider credentials are needed.make dev-server-sandbox \ DEV_SERVER_SANDBOX_ARGS="--clean-providers --clean-projects"agent-browser skills get core, open the sandbox, and start a video recording before interaction. Usesnapshot -ifor functional checks. Import a subset; enable one imported MCP server in the test workspace.attach_filefor screenshots and video.gh ... --attach; otherwise keep delivery local.Final gate: Run the complete relevant tests and static checks on the final code. Get one clean-context independent readiness recommendation from the diff, tests, and dogfood evidence. Record the verdict and any concrete blocker. Do not claim implementation success if required checks or evidence are missing.
Planning review disposition
One independent review accepted the UI/API design and recommended reducing runtime scope. The plan limits reconciliation to plugin-selection changes and makes runtime tests a prerequisite. Retain cross-process checks: process-local invalidation alone misses held tools in sibling processes. Verified existing call gates already await disk-backed epoch/override readers; extend that bounded seam rather than introducing a separate synchronization system. Planning left the source tree clean. Implementation and test execution remain pending.
Total size and exclusions
Recommended approach: approximately +270–470 net product LoC, excluding tests, stories, and docs. This includes the runtime protections; merely unlocking checkboxes is not sufficient. Re-estimate at the first gate if the existing reconciliation seam cannot support the bounded change.
No new component types, plugin-file deletion, workspace-setting cleanup, generic plugin lifecycle framework, or unrelated MCP refactor. Planning is read-only; the commands above are implementation-time gates, not tests already run.
Generated with
xum• Model:coder:openai/gpt-6-astra• Thinking:xhigh• Cost:$264.74