fix(core): surface sandboxed plugins in the admin plugin management API#1284
Open
eyupcanakman wants to merge 1 commit into
Open
fix(core): surface sandboxed plugins in the admin plugin management API#1284eyupcanakman wants to merge 1 commit into
eyupcanakman wants to merge 1 commit into
Conversation
Plugins registered statically via `sandboxed: []` in astro.config.mjs were absent from the admin Plugins screen. The four admin plugin handlers only read configuredPlugins, never the runtime's sandboxedPluginEntries, so trusted `plugins: []` and marketplace plugins appeared while sandboxed ones did not. Pass sandboxedPluginEntries to handlePluginList, handlePluginGet, handlePluginEnable, and handlePluginDisable, and add a buildSandboxedPluginInfo helper. Sandboxed plugins now list with source "config" and a sandboxed flag, and can be fetched, enabled, and disabled like trusted config plugins. The runtime already gates their hook execution on isPluginEnabled, so enable and disable were supported at the runtime level but unreachable from the admin API. Closes emdash-cms#773
🦋 Changeset detectedLatest commit: 59114a8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
review/needs-review
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes admin plugin management so statically-sandboxed plugins (from sandboxed: [] in astro.config.mjs) appear in the Plugins UI and can be managed via the existing admin API.
Changes:
- Add
sandboxedPluginEntriesto the Astro middleware/handler facade and thread it through admin plugin routes. - Extend plugin admin handlers to list/get/enable/disable statically-sandboxed plugin entries.
- Add integration tests covering listing and enable/disable behavior for sandboxed entries.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/tests/utils/mcp-runtime.ts | Test runtime handler facade now includes sandboxedPluginEntries. |
| packages/core/tests/integration/astro/admin-plugins-sandboxed.test.ts | New end-to-end Astro route integration tests for sandboxed plugin listing and toggling. |
| packages/core/tests/integration/api/plugins.test.ts | New handler-level test for listing sandboxed entries and configured-plugin shadowing. |
| packages/core/src/astro/types.ts | Extends EmDashHandlers with sandboxedPluginEntries. |
| packages/core/src/astro/routes/api/admin/plugins/index.ts | Passes sandboxed entries into handlePluginList. |
| packages/core/src/astro/routes/api/admin/plugins/[id]/index.ts | Passes sandboxed entries into handlePluginGet. |
| packages/core/src/astro/routes/api/admin/plugins/[id]/enable.ts | Passes sandboxed entries into handlePluginEnable. |
| packages/core/src/astro/routes/api/admin/plugins/[id]/disable.ts | Passes sandboxed entries into handlePluginDisable. |
| packages/core/src/astro/middleware.ts | Adds sandboxedPluginEntries to locals.emdash. |
| packages/core/src/api/handlers/plugins.ts | Implements sandboxed plugin info + handler support for list/get/enable/disable. |
| .changeset/admin-list-sandboxed-plugins.md | Patch changeset describing the fix and newly supported behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+60
to
+61
| // Role.ADMIN is 50 in @emdash-cms/auth; plugins:read / plugins:manage require it. | ||
| const admin = { id: "admin-1", role: 50 }; |
Comment on lines
+66
to
+74
| function facade(runtime: EmDashRuntime) { | ||
| return { | ||
| db: runtime.db, | ||
| configuredPlugins: runtime.configuredPlugins, | ||
| sandboxedPluginEntries: runtime.sandboxedPluginEntries, | ||
| config: runtime.config, | ||
| setPluginStatus: runtime.setPluginStatus.bind(runtime), | ||
| }; | ||
| } |
Comment on lines
+217
to
+224
| const sandboxed = sandboxedPluginEntries.find((e) => e.id === pluginId); | ||
| if (sandboxed) { | ||
| const state = await stateRepo.get(pluginId); | ||
| return { | ||
| success: true, | ||
| data: { item: buildSandboxedPluginInfo(sandboxed, state) }, | ||
| }; | ||
| } |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
This is a clean, well-scoped bugfix. The approach is sound: sandboxed: [] config entries were missing from the admin plugin management API because the four plugin handlers only looked at configuredPlugins (trusted plugins) and _plugin_state rows (marketplace/registry). The fix threads sandboxedPluginEntries through all four handlers, adds a matching buildSandboxedPluginInfo helper, exposes the field via locals.emdash, and updates EmDashHandlers so the contract is compile-checked.
I checked:
- Logic: All four handlers (list, get, enable, disable) are updated consistently. The deduplication logic in
handlePluginListcorrectly prioritizes configured plugins over sandboxed ones with the same ID, and both over marketplace/registry rows. The enable/disable paths mirror the existing configured-plugin branch. - Type safety:
EmDashHandlersnow requiressandboxedPluginEntries. The middleware (both authenticated and anonymous paths), the MCP test helper, and runtime mocks are all updated. The anonymous fast path’sas EmDashHandlerscast is a pre-existing type hole and unchanged. - Tests: A handler-level unit test covers sandboxed surfacing and ID-shadowing (configured wins). A route-level integration test drives a real
EmDashRuntimethrough the actual Astro routes for list/enable/disable. - Conventions: Changeset present, no raw SQL,
ApiResult<T>envelope preserved, no admin UI strings to localize, no new indexes or content-table queries. - Edge cases: Leftover marketplace state rows for sandboxed IDs will display
source: "config"(hardcoded) rather than the stale DB source. This is actually preferable to the configured-plugin behavior, which surfaces the stale source and can incorrectly render an Uninstall button.
The PR is genuinely clean — no findings.
Contributor
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
github-actions
Bot
added
review/approved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Plugins registered statically via
sandboxed: []inastro.config.mjsdid not appear on the admin Plugins screen (/_emdash/admin/plugins). Trustedplugins: []entries and marketplace installs showed up, but sandboxed plugins were invisible there even though their hooks, cron schedules, and sandbox isolation all kept working.The four admin plugin handlers in
packages/core/src/api/handlers/plugins.ts(handlePluginList,handlePluginGet,handlePluginEnable,handlePluginDisable) only readconfiguredPluginsand never the runtime'ssandboxedPluginEntries, so thesandboxed: []array fell through the gap between the configured list and the marketplace state rows.The fix passes
sandboxedPluginEntriesto all four handlers and adds abuildSandboxedPluginInfohelper. The routes read it fromlocals.emdash, so the middleware now exposessandboxedPluginEntriesthere and theEmDashHandlerstype declares it (required, so it can not be dropped). Sandboxed plugins now list withsource: "config"and a newsandboxed: trueflag, and can be fetched, enabled, and disabled the same way trustedplugins: []entries are. The runtime already gates sandboxed hook execution onisPluginEnabled, and the enable/disable routes propagate state throughemdash.setPluginStatus, so toggling worked at the runtime level but could not be reached from the admin API. A list-only change would have shown a toggle that returnsNOT_FOUND, so all four handlers move together.A few notes for reviewers:
stateRepo.enable/disablewith nosourceoverride). An id that already has a leftover marketplace state row behaves the same on both config paths, so I left that case as-is rather than special-casing the sandboxed path.sandboxedflag is added to the core API response, matching the runtime manifest which already marks these entries. I did not add an admin UI badge; the adminPluginInfotype has lagged core additions before, and the flag is available for a follow-up if a badge is wanted.handlePluginGetstill returnsNOT_FOUNDfor marketplace and registry plugins. That is pre-existing and outside the scope here.A route-level test drives a real
EmDashRuntimethrough the actual admin routes: the list route surfaces the sandboxed plugin and the enable/disable routes toggle it, and the same test returns 404 against the current code. A handler test covers a sandboxed entry surfacing in the list and the case where a configured plugin shadows one with the same id, where the configured entry wins and the row is listed once.Closes #773
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs.AI-generated code disclosure
Screenshots / test output