Auto-mount active plugin routes and export core plugins via @sonicjs-cms/core/plugins#1
Auto-mount active plugin routes and export core plugins via @sonicjs-cms/core/plugins#1Copilot wants to merge 16 commits into
Conversation
Signed-off-by: Johan Furelid <johan@furelid.com>
Signed-off-by: Johan Furelid <johan@furelid.com>
furelid
left a comment
There was a problem hiding this comment.
We already have the canonical manifest source in code: packages/core/src/plugins/plugin-registry.ts (auto-generated) includes is_core: boolean, so we can enforce the policy without hardcoded lists.
Essential request: core plugins (is_core: true) must always be enabled (never 404 due to DB/config gating).
Proposed approach (minimal + flexible):
- Extend
mountPluginManagerRoutes()options with anisCorePlugin?: (pluginName: string) => boolean. - In
shouldEnablePlugin(), short-circuit:if (isCorePlugin(pluginName)) return true
- In
createSonicJSApp(), provide:isCorePlugin: (name) => getPlugin(name)?.is_core === true(fromplugin-registry.ts)
This uses the generated registry as the single source of truth and still allows plugins shipped in packages/core with is_core: false (e.g. global-variables, security-audit) to remain gated by DB/config as intended.
There was a problem hiding this comment.
Pull request overview
This PR fixes a core integration gap where plugin routes collected via PluginBuilder.addRoute() were not being mounted into the main Hono app, causing active/enabled plugins (e.g. global-variables) to return 404. It also expands the public @sonicjs-cms/core/plugins barrel exports and simplifies the sample app to rely on core auto-mounting.
Changes:
- Add
mountPluginManagerRoutes()and use it increateSonicJSApp()to mount/guard core plugin routes before core/admin catch-all routing. - Expand
packages/core/src/plugins/index.tsto publicly export core plugins and helpers via@sonicjs-cms/core/plugins. - Add a regression unit test for enabled vs inactive plugin route mounting, and clean up a few internal/self-imports.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/plugins/plugin-manager.ts | Extracts route registration into registerPluginRoutes() and reuses it during extension registration. |
| packages/core/src/plugins/index.ts | Adds public re-exports for core plugins and related helpers/services. |
| packages/core/src/plugins/core-plugins/otp-login-plugin/index.ts | Avoids unused destructured field by renaming to _isActive. |
| packages/core/src/plugins/available/email-templates-plugin/services/email-renderer.ts | Switches renderTemplate import to a source-relative utils import. |
| packages/core/src/app.ts | Implements mountPluginManagerRoutes() and uses it to auto-mount core plugin routes with enablement gating. |
| packages/core/src/tests/utils/utils.template-renderer.test.ts | Updates imports to source-relative utils. |
| packages/core/src/tests/plugins/plugin-route-mounting.test.ts | Adds regression coverage for enabled vs inactive plugin route mounting behavior. |
| packages/core/src/tests/middleware/middleware.permissions.test.ts | Updates imports to source-relative middleware exports. |
| my-sonicjs-app/src/index.ts | Removes comments/structure implying manual core plugin route wiring; keeps manual mounting for app-local plugins only. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| isPluginEnabled: async (pluginName, c) => { | ||
| if (config.plugins?.disableAll) { | ||
| return false | ||
| } | ||
|
|
||
| return isPluginActive(c.env.DB, pluginName) | ||
| }, |
There was a problem hiding this comment.
What is the best approach as you are the most senior and experienced developer with years of knowledge and experience
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Fix SVG D1 inserts and add commenting guidelines
| return false | ||
| } | ||
|
|
||
| return isPluginActive(c.env.DB, pluginName) |
There was a problem hiding this comment.
Response to https://github.com/furelid/sonicjs/pull/1/changes#r3208714130:
In the current implementation enabledPlugins is ignored when isPluginEnabled is provided, so disableAll isn’t bypassable today. To make this precedence explicit and prevent future regressions, I’ll add disableAll?: boolean to PluginRouteMountOptions and short-circuit it at the top of shouldEnablePlugin().
| if (options.isPluginEnabled !== undefined) { | ||
| return await isPluginEnabled(pluginName, c) | ||
| } | ||
|
|
||
| return enabledPlugins.has(pluginName) |
| if (config.plugins?.disableAll) { | ||
| return false | ||
| } | ||
|
|
||
| return isPluginActive(c.env.DB, pluginName) |
| class="inline-flex items-center gap-x-1.5 px-3.5 py-2.5 bg-zinc-950 dark:bg-white text-white dark:text-zinc-950 font-semibold text-sm rounded-lg hover:bg-zinc-800 dark:hover:bg-zinc-100 transition-colors shadow-sm" | ||
| > | ||
| <svg class="h-5 w-5" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"> | ||
| <path d="M7 8L3 11.6923L7 16M17 8L21 11.6923L17 16M14 4L10 20" stroke="#000000" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"/> |
| <p class="text-xs text-zinc-500 dark:text-zinc-400"> | ||
| This is a code-based representation of ${data.display_name ? `the ${data.display_name}` : 'this'} collection. | ||
| Learn more about | ||
| <a href="https://sonicjs.com/collections" target="_blank" class="text-blue-600 dark:text-blue-400 hover:underline"> |
| * Code Export Modal Logic | ||
| */ | ||
|
|
||
| let collectionName = '${data.name || ''}'; |
| * Selects the textarea content and attempts to copy it to the clipboard, providing user feedback on success or failure. | ||
| */ | ||
| codeExportModalContainer.querySelector('#copy-options-btn') | ||
| .addEventListener('click', function() { | ||
| const textarea = codeExportModalContainer.querySelector('#code-export-options'); | ||
| textarea.select(); | ||
| textarea.setSelectionRange(0, 99999); // For mobile devices | ||
|
|
||
| try { | ||
| const successful = document.execCommand('copy'); | ||
| if (successful) { | ||
| const feedback = codeExportModalContainer.querySelector('#copy-options-feedback'); | ||
| feedback.classList.remove('invisible'); | ||
| setTimeout(() => feedback.classList.add('invisible'), 2000); | ||
| } else { | ||
| throw new Error('Copy command was unsuccessful'); | ||
| } | ||
| } catch (err) { | ||
| console.error('Error copying to clipboard:', err); |
…e plugin active status - Add `isCorePlugin?: (pluginName) => boolean` to PluginRouteMountOptions so core plugins (is_core === true in manifest-registry) are always enabled and bypass disableAll - Add `disableAll?: boolean` to PluginRouteMountOptions with explicit precedence: core plugins skip it, others are blocked - Use `getPlugin(name)?.is_core === true` from manifest-registry as the isCorePlugin implementation in createSonicJSApp() - Add module-level TTL cache (60s) for isPluginActive DB queries to avoid a D1 read on every request in the hot path - All 1475 unit tests pass Agent-Logs-Url: https://github.com/furelid/sonicjs/sessions/6a6dbe7b-ef8b-4d96-89b1-1359dcd3b85c
- Document SonicJSConfig.plugins.disableAll and enabled fields with JSDoc explaining their interaction with isCorePlugin gating - Document the TTL cache invalidation window and memory boundedness Agent-Logs-Url: https://github.com/furelid/sonicjs/sessions/6a6dbe7b-ef8b-4d96-89b1-1359dcd3b85c
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| if (options.isPluginEnabled !== undefined) { | ||
| return await isPluginEnabled(pluginName, c) | ||
| } | ||
|
|
||
| return enabledPlugins.has(pluginName) |
There was a problem hiding this comment.
Consider the argument and adjust the code. https://github.com/furelid/sonicjs/pull/1/changes#r3209181998
… subpath exports Combines this branch's user-plugin work with the changes from furelid#1 (furelid#1). The two PRs solved different problems and are complementary; this commit takes both. From furelid #1: - mountPluginManagerRoutes(app, plugins[], options) replaces the 9 duplicated route-mount loops in createSonicJSApp. Each core plugin's routes are now gated by a per-request guard that checks `is_active` in the plugins table — flipping the toggle in the admin UI now actually affects route resolution. - 60s TTL cache (pluginStatusCache) so the gate is one Map lookup on the hot path; one D1 read per plugin per minute amortized. - isCorePlugin callback bypasses gating for plugins with is_core === true in their manifest (database-tools, seed-data, core-cache, etc.). - plugins.enabled?: string[] for explicit allowlists when DB gating is disabled. - @sonicjs-cms/core/plugins subpath now exports aiSearchPlugin, analyticsPlugin, globalVariablesPlugin, oauthProvidersPlugin, securityAuditPlugin, OAuthService, BUILT_IN_PROVIDERS, stripePlugin, shortcodesPlugin, etc. so advanced users can wrap or compose them. - Tightens a few imports that were going through the public package re-export back to the source modules. Kept from this branch: - plugins.register?: Plugin[] for user plugins, mounted via mountPlugin() (no DB gating — if the user imported and registered it, they want it on). - @deprecated marks on plugins.directory and plugins.autoLoad. - Doc + blog post fixes. User plugins still bypass the new gating system. Adding them to mountPluginManagerRoutes with isCorePlugin: () => false is a small follow-up so the admin "disable plugin" toggle applies to user plugins too. All 1486 unit tests pass (1479 mine + furelid + 2 plugin-route-mounting + 5 createSonicJSApp integration smoke tests). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
mountPluginManagerRoutes()@sonicjs-cms/core/pluginsisCorePluginoption toPluginRouteMountOptions(owner request — core plugins always enabled, bypass disableAll)disableAlloption toPluginRouteMountOptions(owner request — explicit precedence)isCorePlugin: (name) => getPlugin(name)?.is_core === trueincreateSonicJSApp()isPluginActiveresult (60s TTL) to avoid DB read per request (reviewer concern)