Skip to content

🤖 feat: Agent Plugins install/update UX (managed installs, v1) - #3820

Open
ThomasK33 wants to merge 21 commits into
mainfrom
agent-plugin-install-ux
Open

🤖 feat: Agent Plugins install/update UX (managed installs, v1)#3820
ThomasK33 wants to merge 21 commits into
mainfrom
agent-plugin-install-ux

Conversation

@ThomasK33

@ThomasK33 ThomasK33 commented Aug 8, 2026

Copy link
Copy Markdown
Member

Summary

v1 of the Agent Plugins install/update UX ("Option B: managed installs"): paste a git URL or owner/repo[@ref] into Settings → Plugins, get a consent preview of everything the plugin contributes (manifest, every skill, every MCP server command line), and install into ~/.mux/plugins with provenance recorded in a managed-install registry. Update badge + manual update, uninstall with override pruning, all behind the existing agent-plugins experiment.

Background

PR #3815 shipped Agent Plugins 1.0.0 as discovery-only: users had to git clone into container dirs by hand, with no provenance, no update signal, no uninstall, and no list surface. The design doc (docs/research/agent-plugin-integration-options.md on branch research-agent-plugin-ux) compared five options; Thomas signed off on Option B (managed installs) with the §6 proposed decisions ratified.

Approved decisions implemented here

  1. Registry — a standalone ~/.mux/plugins.json owned by the install service (atomic, throwing writes; in-process serialized mutations). Lenient-on-read: invalid entries are dropped with a warning, and plugin names are pattern-validated so a malformed entry can never resolve a path outside the container. (Deviation from §6-Q2's letter, following its own contingency: Codex review demonstrated that .passthrough() only affects schema validation — older builds rebuild config.json from known fields on save, so a downgrade would drop an embedded registry section. Q2 priced exactly this: "Cost if wrong: a one-time migration to a separate file." A file older builds never rewrite is the only mechanism that actually survives downgrade round-trips, and owning the write path also makes registry-persistence failures observable for rollback.)
  2. Tracking semanticssource.ref is the tracking channel, lockedSha is what runs. No ref given ⇒ record the remote default branch + pin its current SHA. Tag/SHA refs are pinned (moved tags surface a tag moved warning badge). Nothing auto-applies, ever.
  3. Consent preview — temp shallow clone to ~/.mux/plugin-staging (never inside a discovery container), validated with the same validatePluginManifest + discovery code the runtime uses, listing manifest metadata, every skill name+description, and every MCP command line (rendered against the final install path, incl. PLUGIN_DATA expansion). Cancelling writes nothing — the preview is stateless; install re-fetches the exact consented SHA and fails loudly if the remote moved.
  4. Update — badge + manual only; checks run on Settings-section open and on the explicit button (git ls-remote vs lockedSha, no fetch, no timers). Applying = temp clone at the new SHA → re-validate → wholesale directory swap (rename-old → promote-new → delete-old, with rollback) → bump lockedSharecycle that plugin's running MCP servers via the new MCPServerManager.stopServersWithKeyPrefix (content can change behind an unchanged stdio command line, so the config-signature check cannot notice). Local edits to a managed plugin dir are discarded on update (documented).
  5. Uninstall — deletes plugin dir + registry entry + prunes that plugin's plugin:<instanceId>:* keys from every local workspace's MCP overrides (reinstall re-attaches the same instanceId, so stale overrides would silently re-enable servers). ~/.mux/plugin-data/<instanceId> is preserved behind an "also delete stored plugin data" checkbox, unchecked by default.
  6. Scope — global-only; the installer never writes into a project checkout.
  7. Human-only surfaces — Settings section + palette commands (Settings: Plugins, Install Agent Plugin…, Check for Plugin Updates, Update All Plugins — keyboard rule). No agent-facing installer tool.
  8. Name collisions — existing registry entry or target dir ⇒ clear error; the installer never overwrites.
  9. Subpath grammar, not subpath installsowner/repo/sub/path[@ref] parses and the subpath field is persisted in the source descriptor, but installs reject with "monorepo subpath installs land in v2". Claude Code plugin/marketplace repos fail with a clear message naming the limitation (source stays a discriminated union so an import adapter is additive).

Implementation

  • Step 1 — registry schema: src/common/config/schemas/agentPluginInstalls.ts (entry + tagged-union source + plugins.json file schema); name grammar shared with the manifest validator via src/common/utils/agentPluginName.ts.
  • Step 2 — service + oRPC: discoverAgentPluginAt (public single-root wrapper over the existing per-entry discovery, so staged clones get the exact runtime validation); normalizeRepoUrlForClone extracted to src/node/utils/gitUrls.ts (shared with the project clone flow); sourceInput.ts grammar; AgentPluginInstallService (preview/install/list/uninstall/checkUpdates/update, mutations serialized on an internal queue, staging under ~/.mux/plugin-staging with stale-dir reclamation, GIT_TERMINAL_PROMPT=0 + SSH BatchMode so private repos without auth fail fast instead of hanging); plugins.* oRPC namespace returning Result values; MCPServerManager.stopServersWithKeyPrefix recycle hook. Backend gating mirrors the MCP provider: the service is constructed with isEnabled: () => experimentsService.isExperimentEnabled(AGENT_PLUGINS).
  • Step 3 — UI: PluginsSettingsSection (list with unmanaged/missing/update available/tag moved/pinned badges, two-phase add flow, inline uninstall confirm), experiment-gated section registration + redirect + palette entry.
  • Step 4/5 — docs, stories, tests: docs additions in docs/config/mcp-servers.mdx + docs/agents/agent-skills.mdx; Storybook stories with play assertions (consent preview, update states, unchecked-by-default checkbox); unit tests for the input grammar, registry round-trip/self-heal, and the full service lifecycle against real local git remotes (hermetic — local-path remotes exercise the same clone/ls-remote plumbing).

Validation

  • make static-check green (typecheck, ESLint, prettier, docs links); targeted suites: 393 tests across the touched areas (agentPlugins, config, schemas, SettingsPage, palette sources, MCPServerManager, oRPC router, projectService) all pass; test-storybook passes for the new stories.
  • Live dogfooding in a make dev-server-sandbox instance (screenshots in the workspace transcript): enabled the experiment via Settings → Experiments (Plugins section appeared immediately), installed a local fixture repo through the full preview → consent → install flow, verified the on-disk registry entry + plugin tree (no .git), advanced the fixture remote → update available badge appeared on "Check for updates" → Update bumped lockedSha/version/updatedAt, uninstall (checkbox unchecked) removed dir + registry but preserved plugin-data, and the reinstalled plugin's MCP server surfaced in Settings → MCP as plugin · … default-disabled/read-only.

Risks

  • Config surface: none — the registry is a standalone ~/.mux/plugins.json; config.json load/save is untouched. Malformed registry entries degrade to "unmanaged dir" rather than errors; downgrade-safe because older builds never touch the file.
  • MCP recycle: stopServersWithKeyPrefix only stops matching workspaces' server sets; they restart lazily on next use, same as the idle-timeout path. No behavior change for non-plugin servers.
  • Everything is experiment-gated: with agent-plugins off, the service throws, the section/palette entry hide, and no new code paths run.

Judgement calls

  • install re-fetches the exact consented SHA (direct SHA fetch, falling back to branch clone + HEAD verification) rather than keeping the preview clone on disk between preview and confirm — a stateless preview means cancel/crash cannot leave partial state, at the cost of a second shallow clone on confirm.
  • The installed tree drops .git (plain content snapshot): the registry holds all provenance, updates replace the dir wholesale, and a live checkout would only invite in-place edits that updates discard.
  • Update refuses upstream renames (plugin.json#name changed): container-entry names are identity (instanceId → PLUGIN_DATA, workspace overrides), so renames require uninstall/reinstall.
  • Uninstall stops that plugin's running MCP servers before deleting the tree, mirroring the update-recycle rationale.
  • Update All Plugins applies only update-available entries; moved tags stay per-plugin manual (a mutated tag deserves the section's warning, not a bulk apply).

Deferred (per §5/§6 of the design)

  • v2: monorepo subpath installs (sparse checkout; grammar + schema already in place), content-addressed store + symlinked container entries, dev-mode/local-path installs, unmanaged-dir adoption ("convert to managed"), Pin row action, bun run debug plugin … CLI + /plugin slash command.
  • v3: repo-declared prompt-on-trust team plugins, archive+sha256 / seed dirs for air-gap, restore-from-lock, catalogs/marketplace (Claude marketplace import adapter only on demonstrated demand).
  • Explicit non-goals: background/auto-update (per-entry autoUpdate boolean reserved in the schema, unused), agent-facing install tool, Claude Code marketplace compatibility.

Post-review hardening (Codex rounds 1–20)

20 review rounds of fixes folded into this diff

Highlights beyond the original plan (full round-by-round history in the PR review threads):

  • Registry durability: standalone plugins.json with lossless raw-document writes (unknown envelope/entry/tombstone fields from newer builds survive rewrites), strict-mode reads for mutations vs lenient reads for list, raw-entry collision checks, non-ENOENT read errors refuse mutations.
  • Uninstall override pruning: pre-commit workspace enumeration, persisted pendingOverridePrunes tombstones (pessimistic commit, retry on list, reconciliation against deleted workspaces, reinstall gate) so a temporarily unreachable checkout can never let a reinstall silently re-enable a pruned server.
  • Workspace MCP overrides optimistic concurrency: workspace.mcp.get returns { overrides, revision }; set requires expectedRevision and rejects stale dialog snapshots (serialized check-and-set), and the uninstaller's prune retries on conflict — a stale open dialog can no longer resurrect pruned plugin: keys.
  • MCP recycle vs in-flight startups: monotonic prefix-invalidation epochs; closeInvalidatedInstancesThenPublish re-scans until the invalidation clock is stable and publishes synchronously in the same continuation, closing the microtask window where a mid-startup plugin swap could publish (and keep alive) a server from a deleted tree. Regression test interleaves the swap into the exact yield window.
  • Consent preview parity: symlinked skill dirs are disclosed (with containment warnings for escaping symlinks), matching runtime discovery.
  • Mobile: break-all on plugin name/location/source lines + pinned 390px story with a max-length-name overflow assertion.

Two P2 follow-ups are documented (not in this PR) in this comment: stale workspace-switch modal loads, and prefix-stop retry-marker publish ordering.

Rebase + experimental label

  • Rebased onto main (squashed to one feature commit): merged this branch's install contract with main's independently-landed agentPlugins.ts oRPC schemas (slash commands/composition inspector), re-ported the stable-clock publication onto the MCP SDK v2 mcpServerManager, and adopted main's SSH→HTTPS clone fallback into the extracted gitUrls.ts (installer records the primary URL only, documented).
  • The Plugins section is now marked experimental the same way as Backup: FlaskConical nav icon + in-section warning banner.
  • Re-dogfooded the full lifecycle post-rebase in a fresh dev-server-sandbox (install → consent preview incl. a deliberately-invalid mcp.json diagnostic → update badge → atomic update → read-only MCP row → uninstall with data-preservation default); screenshots in the workspace transcript.

Generated with mux • Model: anthropic:claude-fable-5 • Thinking: xhigh • Cost: $327.63

@mintlify

mintlify Bot commented Aug 8, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
Mux 🟢 Ready View Preview Aug 8, 2026, 6:37 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3b9d7245ce

ℹ️ 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".

Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread src/common/config/schemas/appConfigOnDisk.ts Outdated
Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread src/browser/features/Settings/Sections/PluginsSettingsSection.tsx
@ThomasK33

Copy link
Copy Markdown
Member Author

Addressed all five Codex findings in b9c9a10:

  • Traversal names (P1) — the plugin-name grammar (§5 pattern, now shared via src/common/utils/agentPluginName.ts) is enforced in the registry entry schema and asserted in targetPathFor before any filesystem mutation; entries named ./../a/../b are dropped on read and can never resolve outside the container. Test: "registry survives config.json rewrites and drops traversal names on read".
  • Downgrade preservation (P1) — correct: .passthrough() only affects schema validation; older builds rebuild config.json from known fields on save. Followed §6-Q2's own contingency ("migration to a separate file"): the registry now lives in a standalone ~/.mux/plugins.json that older builds never rewrite. Test: registry survives editConfig config.json rewrites.
  • Registry write observability (P2) — solved by the same move: the service owns the file and its atomic write throws, so install rolls back the promoted dir ("Failed to persist the plugin registry"), uninstall writes the registry before deleting the tree, and a failed update write keeps the stale lockedSha (badge stays, retry self-heals). Test: "install rolls back the promoted dir when the registry write fails".
  • Fallback clone into non-empty dir (P1) — the staging dir is reset before the branch-clone fallback. Test: "falls back to a branch clone when the remote refuses direct SHA fetches" (file:// remote with uploadpack.allowAnySHA1InWant=false).
  • Keyboard rule (P1) — added palette commands: Install Agent Plugin… (opens Settings → Plugins with the add form expanded), Check for Plugin Updates (toast + navigates when updates exist), Update All Plugins (applies update-available; moved tags intentionally stay per-plugin manual since mutated tags deserve the section's warning). Uninstall/per-plugin update remain reachable via standard focus navigation within the section.

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b9c9a1062a

ℹ️ 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".

Comment thread src/browser/features/Settings/Sections/PluginsSettingsSection.tsx Outdated
Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread src/browser/utils/commands/sources.ts Outdated
Comment thread src/browser/utils/commands/sources.ts Outdated
@ThomasK33

Copy link
Copy Markdown
Member Author

Addressed all four round-2 findings in edcdfa0:

  • Registry restore on removal failure — uninstall now stages the tree out of the container (rename into the staging root) before the registry write: a locked/undeletable tree fails the rename with the install fully intact, and a failed registry write renames the tree back. Deleting the staged tree is best-effort (stale-dir reclamation covers leftovers). Test: "uninstall restores the registry entry when the tree cannot be staged out" (read-only container forces the rename failure, then the retry succeeds).
  • Add panel with section already mounted — the intent module now supports subscription; the mounted section subscribes and expands the add panel immediately, while the useState initializer still covers the palette → fresh-mount path.
  • Mutation errors clobbered by refresh — update/uninstall re-assert the operation error after the refresh (whose success path clears error state); a failed uninstall also keeps the confirmation open instead of dismissing it.
  • Per-plugin check errors — both Check for Plugin Updates and Update All Plugins now distinguish status: "error" entries: unreachable remotes surface as "Update check failed for …" (with navigation to the section) instead of masquerading as "All plugins are up to date."

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: edcdfa05fe

ℹ️ 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".

Comment thread src/browser/features/Settings/Sections/PluginsSettingsSection.tsx
Comment thread src/browser/utils/commands/sources.ts
Comment thread src/node/services/agentPlugins/sourceInput.ts
@ThomasK33

Copy link
Copy Markdown
Member Author

Addressed all three round-3 findings:

  • Keyboard path for uninstall — new Uninstall Agent Plugin… palette command using the palette's select prompt (async getOptions over agentPlugins.list(), managed entries only). Submission publishes a confirm-uninstall intent and opens the section, landing the user in the existing confirmation flow with the plugin-data checkbox — the palette never deletes directly.
  • Mounted-section staleness after bulk updates — the intent module is now a typed bus (open-add-panel / confirm-uninstall / refresh); Update All Plugins publishes refresh after its mutations, so a mounted section re-queries list + update checks instead of showing stale versions/badges. Unmounted sections still consume the buffered intent on mount.
  • Tilde expansion~/~/… sources resolve against os.homedir() before git sees them (git is spawned via execFile, no shell). Grammar test added.

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a6503d2fe4

ℹ️ 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".

Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread src/node/services/agentPlugins/installService.ts
Comment thread src/browser/utils/commands/sources.ts Outdated
Comment thread src/browser/features/Settings/Sections/PluginsSettingsSection.tsx
Comment thread src/node/services/agentPlugins/sourceInput.ts Outdated
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bc68e771d7

ℹ️ 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".

Comment thread src/node/services/mcpServerManager.ts Outdated
Comment thread src/node/services/agentPlugins/installService.ts
Comment thread src/node/services/agentPlugins/installService.ts
Comment thread src/browser/utils/commands/sources.ts
Comment thread src/node/services/agentPlugins/installService.ts Outdated
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5a2bd105f4

ℹ️ 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".

Comment thread src/node/services/mcpServerManager.ts
Comment thread src/node/services/mcpServerManager.ts Outdated
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0443e1af3e

ℹ️ 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".

Comment thread src/node/services/agentPlugins/installService.ts
Comment thread src/browser/features/Settings/Sections/PluginsSettingsSection.tsx Outdated
Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread src/browser/features/Settings/Sections/PluginsSettingsSection.tsx
Comment thread src/node/services/agentPlugins/installService.ts Outdated
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dc141c4c7e

ℹ️ 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".

Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread src/node/services/agentPlugins/installService.ts Outdated
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b6daf6aec0

ℹ️ 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".

Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread src/node/services/agentPlugins/installService.ts
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f1fd47e8cd

ℹ️ 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".

Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread src/node/services/agentPlugins/installService.ts
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b6965bc298

ℹ️ 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".

Comment thread src/browser/utils/commands/sources.ts Outdated
Comment thread src/browser/utils/commands/sources.ts Outdated
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e576538423

ℹ️ 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".

Comment thread src/node/services/agentPlugins/installService.ts Outdated
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3b030057e1

ℹ️ 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".

Comment thread src/node/services/workspaceMcpOverridesService.ts Outdated
Comment thread src/browser/features/Settings/Sections/PluginsSettingsSection.tsx Outdated
Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread src/node/services/agentPlugins/installService.ts
Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread src/node/services/agentPlugins/installService.ts Outdated
- Security: updates reject capability increases/changes (new hooks.js,
  expanded hook tool grants, added or changed MCP servers) instead of
  silently applying them; uninstall + reinstall routes through the full
  install consent preview.
- Consent preview renders stdio argv shell-quoted per token exactly like
  the runtime, so argument boundaries cannot be concealed or faked.
- Reject Windows-reserved device names (con, prn, aux, nul, com1-9,
  lpt1-9, with or without extension) in the shared plugin-name validator
  on every platform.
- Bound untrusted git subprocess output (10 MiB) so a noisy remote
  cannot exhaust main-process memory before the timeout.
- checkUpdates reads the registry strictly (corrupted registry surfaces
  as the update-check error state, not a false all-clear) and bounds
  concurrent ls-remote lookups to 4.
- prunePluginOverrideKeys rejects opaque enabledServers/disabledServers/
  toolAllowlist shapes from newer builds so tombstones stay retryable
  instead of retiring against uninspectable content.
- Use shared warning color tokens (bg-warning/text-warning) instead of
  fixed yellow-500 utilities in the Plugins section.
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5c7136d8f2

ℹ️ 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".

Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread src/browser/features/Settings/Sections/PluginsSettingsSection.tsx Outdated
Comment thread src/node/services/agentPlugins/installService.ts
Comment thread src/node/services/agentPlugins/installService.ts
Comment thread src/browser/utils/commands/sources.ts
- Include stdio cwd in the update capability fingerprint: a cwd-only
  change (e.g. plugin root -> writable PLUGIN_DATA) silently moves
  relative module/config resolution, so it requires re-consent.
- Settings copy shows the backend-provided managed plugin container
  path (new agentPlugins.containerLocation route) instead of a
  hardcoded ~/.mux/plugins: the active root is config-derived.
- checkUpdates errors when the registry holds entries this build
  cannot parse (newer source kinds / corruption) instead of silently
  skipping them and reporting a false all-clear.
- Uninstall surfaces a failed user-requested plugin-data deletion
  (with the staged path for manual cleanup) instead of confirming
  success; staging reclamation may otherwise never run.
- New 'Update Agent Plugin…' palette command: a per-plugin selector so
  keyboard-only users can apply moved-tag updates after reviewing the
  warning (bulk update intentionally excludes them).
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fc62ab47ac

ℹ️ 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".

Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread src/node/services/workspaceMcpOverridesService.ts Outdated
- Install rollback retries the promoted-tree deletion after stopping
  the plugin's MCP servers (a running server can hold the lock), and
  quarantines a still-undeletable tree into the staging root so global
  discovery cannot load it as an unmanaged plugin after a failed
  install.
- prunePluginOverrideKeys rejects a non-object override document root
  (a newer build's representation) the same way it rejects opaque
  owned-field shapes, keeping the uninstall tombstone retryable
  instead of retiring it against uninspectable content.
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d18a2d3158

ℹ️ 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".

Comment thread src/node/services/mcpServerManager.ts
Comment thread src/node/services/agentPlugins/installService.ts
Comment thread src/node/services/agentPlugins/mcpConfig.ts Outdated
Comment thread src/browser/features/Settings/Sections/PluginsSettingsSection.stories.tsx Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Security Review

Here are some automated security review suggestions for this pull request.

Reviewed commit: d18a2d3158

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ 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.

Comment thread src/node/services/agentPlugins/installService.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d18a2d3158

ℹ️ 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".

Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread src/node/services/agentPlugins/installService.ts Outdated
- Recheck the workspace removal stop-epoch inside the synchronous MCP
  cache publication callback: a stopServers(workspaceId) landing while
  the awaited invalidation scan yielded found no cache entry to close,
  so publishing would resurrect processes for a removed workspace.
  Skip publication and close the late clients instead.
- Sort env entries in the update capability fingerprint: env is an
  unordered map, so an upstream property reordering must not be
  rejected as a capability change.
- Validate added plugin override keys PER FIELD: a stale key surviving
  only in toolAllowlist must not smuggle that key into enabledServers
  without discovery validation.
- Stories: accept a single props parameter instead of destructuring
  (repo rename-friendly convention).
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d8a0da25b9

ℹ️ 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".

Comment thread src/node/services/mcpServerManager.ts
- Security: enforce an aggregate quota (100 MiB / 10,000 files, .git
  excluded, symlinks not followed) on every staged clone checkout
  before validation reads it — --depth 1 and the subprocess output cap
  do not bound checkout bytes from an untrusted remote.
- Consent preview discloses full stdio env assignments (quoted
  KEY='value'), not just key names: values like
  NODE_OPTIONS=--require=./payload.js change what executes without
  appearing in the argv.
- Uninstall data rollback re-invalidates the plugin's MCP prefix and
  removes a recreated (empty) data dir before restoring the staged
  original, so a late server launch cannot make the restore
  EEXIST-fail and strand the user's data in staging.
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ffa0312519

ℹ️ 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".

Comment thread src/node/services/agentPlugins/installService.ts
Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread docs/config/mcp-servers.mdx Outdated
Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread src/node/services/agentPlugins/installService.ts
- MCPServerManager: recheck cache ownership inside the synchronous
  publication callbacks of the timed-out retry and active-lease restart
  paths; a removal/replacement landing during the awaited invalidation
  scan no longer merges new clients into a detached entry (ownerless
  processes). Removed workspaces return empty instead of recursing.
- Bound git DURING clone/fetch/checkout with a disk-quota watchdog
  (aborts the process tree when the staging dir outgrows 2x the
  checkout quota) — the post-clone check could only reject a tree git
  had already fully materialized.
- Stale staging reclamation ages trash dirs by their embedded Date.now
  stamp (renames preserve the tree's old mtime) and never touches paths
  owned by in-process operations, so an old install staged as a
  rollback copy cannot be reaped mid-uninstall/update.
- resolveRemoteRef prefers the tracked entry's stored ref kind when a
  branch and tag share a name: a remote ADDING a same-name branch no
  longer breaks a still-valid tracked tag.
- Duplicate registry names: strict (mutation) reads refuse, lenient
  views keep the first entry — raw rewrites match by name and would
  otherwise patch every duplicate from the first entry's source.
- Install writes a promotion journal before the promote rename;
  reconcileOrphanedPromotions (section open) quarantines trees orphaned
  by a crash between promote and registry write, restoring
  reinstallability without manual deletion.
- Docs: Settings->Plugins paragraph names the canonical ~/.shux plugin
  locations instead of legacy ~/.mux paths.
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7be3716930

ℹ️ 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".

Comment thread src/node/services/agentPlugins/installService.ts Outdated
Comment thread src/node/services/agentPlugins/installService.ts
Comment thread src/node/services/agentPlugins/installService.ts
Comment thread src/node/services/agentPlugins/installService.ts
Comment thread src/node/services/agentPlugins/installService.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Security Review

Here are some automated security review suggestions for this pull request.

Reviewed commit: 7be3716930

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ 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.

Comment thread src/node/services/agentPlugins/installService.ts Outdated
- Generalize promotion journals into a three-kind journal system
  (promotion/update/uninstall) with reconcileJournals crash recovery
  running at service startup (before any global discovery consumer)
  and on section open
- reconcileJournals uses a STRICT registry read: an unreadable registry
  retains every journal instead of treating committed installs as orphans
- update() journals the swap before moving the live tree; recovery
  restores the old tree so assertNoCapabilityIncrease can't self-reject
- uninstall() journals the staging transaction; recovery restores staged
  tree/data while the registry entry exists and finishes trash cleanup
  once the commit landed
- purgeStaleStaging never reclaims journal-referenced trash dirs
- install rollback re-invalidates the plugin's MCP prefix after the
  retry/quarantine removes the tree
- Disk quota watchdog enforces maxFiles during clone/checkout, not just
  bytes
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Round 38 fixes (f8019ee):

  • Reconcile before discovery: journal reconciliation now also runs at service startup (construction), so a session serving agent requests without opening Settings recovers orphaned promotions before global discovery can load them; list() awaits the startup pass and re-runs it.
  • Rollback re-invalidation: install rollback re-runs stopServersWithKeyPrefix after the retry/quarantine removes the tree (same second post-removal stop as update/uninstall); the orphan-promotion recovery path does the same.
  • Strict registry read in reconciliation: an unreadable/corrupted registry now throws and retains every journal instead of degrading to an empty list and deleting committed installs.
  • Update swap journal: update() journals {name, trashDir} before renaming the live tree; recovery restores the old tree when the target is missing, so a retried update no longer self-rejects on an empty capability surface.
  • Uninstall journal: uninstall() journals the staging transaction before anything moves; recovery restores staged tree/data while the registry entry still exists (clearing a recreated fresh data dir first) and finishes trash deletion once the commit landed. purgeStaleStaging never reclaims journal-referenced trash dirs.
  • File-count quota during checkout: the in-flight watchdog now enforces maxFiles (counting every non-directory entry) alongside bytes; cloneDiskQuota passes 2× the checkout file quota.

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.

1 participant