feat(devx): Architecture simplification and developer experience improvements#118
Merged
seanhanca merged 7 commits intoFeb 18, 2026
Merged
Conversation
…ples - Update scaffolder template: mount.tsx now delegates to App.tsx plugin instance instead of manual createRoot/ShellProvider boilerplate - Update App.tsx template: uses createPlugin() with MemoryRouter - Migrate examples/hello-world to createPlugin() pattern - Migrate examples/todo-list to createPlugin() pattern - Add mount.tsx and main.tsx to both examples - Update test assertions to validate new scaffold output (220 passing) - Update docs: plugin-developer-guide, quickstart, SDK README Co-authored-by: Cursor <cursoragent@cursor.com>
- Delete packages/plugin-build/vite.umd.base.ts (0 callers, not exported) - Deprecate createPluginViteConfig() in @naap/config with migration guide - Deprecate createUMDPluginMount() and createUMDPlugin() in SDK UMD module with clear migration path to createPlugin() - All 15 plugins use createPluginConfig() exclusively — no breakage Co-authored-by: Cursor <cursoragent@cursor.com>
- Create contract-validation.ts: validatePluginModule(), validateShellContext(), formatPluginError() with actionable fix suggestions - Wire validation into createPlugin() constructor: validates App, name, version - Wire context validation into mount() in development mode - Improve UMD loader error messages with plugin name, fix instructions, and createPlugin() migration guidance - Improve mountUMDPlugin error messages with troubleshooting hints - Export validation utilities from @naap/plugin-sdk - Add 22 comprehensive tests for all validation paths (242 total passing) Co-authored-by: Cursor <cursoragent@cursor.com>
…ab conversion - Remove hardcoded 14-entry PLUGIN_DIR_MAP from CDN route - Add toKebabCase() function for automatic name resolution - New plugins no longer require shell code changes for CDN serving - Update CDN tests: verify all 11 old map entries produce identical output - Add tests for unknown/new plugin names, single-word passthrough, already-kebab passthrough, and triple-word camelCase Co-authored-by: Cursor <cursoragent@cursor.com>
- Re-export HTTP header constants (HEADER_CSRF_TOKEN, etc.) from @naap/plugin-sdk - Re-export AuthUser type from @naap/types via plugin-sdk - Update hello-world and todo-list peerDependencies: @naap/types -> @naap/plugin-sdk - Document the 2-package model in SDK README: @naap/plugin-sdk (runtime) + @naap/plugin-build (build tooling) - Plugin developers no longer need @naap/types as a direct dependency Co-authored-by: Cursor <cursoragent@cursor.com>
- Full regression suite: 280 tests passing (242 SDK + 19 BDD + 19 CDN) - 6 plugin builds verified with UMD validation - regression-guard.sh: 8/8 checks pass - New review-pluginQA.md with 100-point DevX assessment: Before: 62/100 → After: 83/100 (+21 points) - Detailed before/after comparison for each of the 5 fixes - Remaining opportunities documented for future work Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
scope/shell
|
|
📝 WalkthroughWalkthroughThis PR refactors the NAAP plugin framework to use a deterministic camelCase-to-kebab-case converter for CDN plugin resolution, introduces a new createPlugin factory pattern for plugin scaffolding, adds runtime contract validation utilities for module integrity, removes the legacy vite.umd.base.ts configuration, and updates documentation and examples to reflect the new architecture. It also adds detailed error handling for plugin loading and mounting failures. Changes
Sequence DiagramsequenceDiagram
participant App as Plugin App
participant Mount as Mount Handler
participant Validate as Contract Validator
participant Shell as Shell Context
participant Error as Error Handler
App->>Mount: Initialize plugin with App, name, version
Mount->>Validate: validatePluginModule(module)
Validate-->>Validate: Check type, mount(), unmount, metadata
Validate-->>Mount: Return ValidationResult
Mount->>Validate: validateShellContext(context)
Validate-->>Validate: Check navigate, eventBus
Validate-->>Mount: Return ValidationResult with warnings
alt Module or Context Invalid
Mount->>Error: formatPluginError(name, phase, result)
Error-->>Mount: Formatted error message
Mount->>App: Log structured error & display UI
else All Valid
Mount->>Shell: Mount plugin with context
Shell-->>Mount: Mount successful
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
examples/hello-world/frontend/src/mount.tsx (1)
12-15: Avoidanyby narrowing the plugin shape locally.A small local type keeps the surface explicit and avoids
anywhile still tolerating optional fields.♻️ Suggested typing refinement
-import plugin from './App'; +import plugin from './App'; + +type UmdPluginExports = { + mount: typeof plugin.mount; + unmount?: () => void; + getContext?: () => unknown; + metadata?: { name: string; version: string }; +}; + +const typedPlugin = plugin as UmdPluginExports; -export const mount = plugin.mount; -export const unmount = (plugin as any).unmount; -export const getContext = (plugin as any).getContext; -export const metadata = (plugin as any).metadata || { name: 'helloWorld', version: '1.0.0' }; +export const mount = typedPlugin.mount; +export const unmount = typedPlugin.unmount; +export const getContext = typedPlugin.getContext; +export const metadata = typedPlugin.metadata || { name: 'helloWorld', version: '1.0.0' };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/hello-world/frontend/src/mount.tsx` around lines 12 - 15, The exports use (plugin as any) which loosens types—create a small local interface (e.g., PluginShape) that declares mount: typeof plugin.mount, and optional unmount?: ..., getContext?: ..., metadata?: { name: string; version?: string } and then cast plugin to that interface before exporting mount, unmount, getContext, and metadata so you avoid any while preserving optional fields; update references to plugin in the file to use this local type rather than any.examples/todo-list/frontend/src/mount.tsx (1)
12-15: Avoidanyby narrowing the plugin shape locally.A small local type keeps the surface explicit and avoids
anywhile still tolerating optional fields.♻️ Suggested typing refinement
-import plugin from './App'; +import plugin from './App'; + +type UmdPluginExports = { + mount: typeof plugin.mount; + unmount?: () => void; + getContext?: () => unknown; + metadata?: { name: string; version: string }; +}; + +const typedPlugin = plugin as UmdPluginExports; -export const mount = plugin.mount; -export const unmount = (plugin as any).unmount; -export const getContext = (plugin as any).getContext; -export const metadata = (plugin as any).metadata || { name: 'todoList', version: '1.0.0' }; +export const mount = typedPlugin.mount; +export const unmount = typedPlugin.unmount; +export const getContext = typedPlugin.getContext; +export const metadata = typedPlugin.metadata || { name: 'todoList', version: '1.0.0' };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/todo-list/frontend/src/mount.tsx` around lines 12 - 15, The current code uses (plugin as any) to access unmount, getContext, and metadata; define a small local interface (e.g., PluginShape) that declares mount: typeof plugin.mount, optional unmount?: ..., optional getContext?: ..., and optional metadata?: { name: string; version: string } (or appropriate types) and then cast plugin to that PluginShape when exporting unmount, getContext, and metadata; update the exports to use the typed plugin reference (e.g., const typedPlugin = plugin as PluginShape) so you avoid any while keeping optional fields tolerated.apps/web-next/src/app/cdn/plugins/__tests__/cdn-serve.test.ts (1)
84-130: LGTM! Comprehensive test coverage for toKebabCase.Good test coverage including:
- Backward compatibility with old PLUGIN_DIR_MAP entries
- Multi-word, single-word, and already-kebab-case inputs
- New/unknown plugin names
Minor nit: The test case
dashboardProviderMockon line 128 duplicates line 124.🧹 Remove duplicate test case
it('handles unknown/new plugin names without a map entry', () => { expect(toKebabCase('someNewPlugin')).toBe('some-new-plugin'); expect(toKebabCase('dashboardProviderMock')).toBe('dashboard-provider-mock'); }); - it('handles triple-word camelCase', () => { - expect(toKebabCase('dashboardProviderMock')).toBe('dashboard-provider-mock'); - }); + it('handles triple-word camelCase', () => { + expect(toKebabCase('myCustomWidget')).toBe('my-custom-widget'); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-next/src/app/cdn/plugins/__tests__/cdn-serve.test.ts` around lines 84 - 130, Tests contain a duplicated assertion for the same input "dashboardProviderMock" (the expect(toKebabCase('dashboardProviderMock')).toBe('dashboard-provider-mock') appears twice); remove the redundant assertion — e.g., delete the duplicate expect inside the "handles triple-word camelCase" test (or remove the earlier identical check in "handles unknown/new plugin names") so each input is only tested once while keeping coverage for toKebabCase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web-next/src/app/cdn/plugins/`[pluginName]/[version]/[...file]/route.ts:
- Around line 77-79: The code is missing validation of pluginName and version
before building pluginDir/versionDir, allowing path traversal; add strict
allowlist checks (e.g., kebab-case pattern for pluginName and a semver-like
pattern for version) and reject/throw (or return 400) if they fail before
calling toKebabCase or joining paths; ensure pluginDir and versionDir are
derived only after validation and consider normalizing with path.posix.join to
keep them inside the intended dist/plugins root (references: pluginName,
version, pluginDir, versionDir, toKebabCase).
In `@apps/web-next/src/lib/plugins/umd-loader.ts`:
- Around line 660-666: The code sets container.innerHTML with unsanitized
plugin.name and errMsg which allows XSS; update the error-rendering in the UMD
loader to avoid inserting raw HTML by either escaping HTML entities for
plugin.name and errMsg or, better, build the DOM using document.createElement
and set textContent on elements (e.g., create a wrapper div, h3, p elements)
instead of using container.innerHTML; ensure you update the code paths around
container, plugin.name and errMsg in the function that renders plugin errors so
all user-controlled strings are never inserted as raw HTML.
In `@docs/plugin-developer-guide.md`:
- Around line 66-68: The "File Structure Reference" is out of date: update its
description to reflect the split entry points by documenting App.tsx as the
plugin definition (createPlugin()), mount.tsx as the UMD entry point that
delegates to App.tsx, and main.tsx as the standalone dev entry point; edit the
section text to mention all three files and clarify their responsibilities
(plugin definition vs UMD mount vs dev bootstrap) so it matches the tree listing
(App.tsx, mount.tsx, main.tsx).
In `@docs/plugin-developer-quickstart.md`:
- Around line 46-48: The App.tsx example still shows the legacy mount() pattern;
update the snippet to use the canonical createPlugin() flow (or move the legacy
mount() example into mount.tsx) so examples match the docs. Replace the mount()
usage in the "frontend/src/App.tsx" snippet with a createPlugin() implementation
that returns the plugin object and exposes the expected lifecycle/hooks, ensure
any top-level export or registration matches createPlugin(), and update comments
to reference createPlugin() instead of mount(); if you choose to retain mount()
show it only in mount.tsx as the UMD entry delegating to App.tsx.
In `@packages/plugin-sdk/src/utils/contract-validation.ts`:
- Around line 142-147: The current guard in contract-validation.ts around the
ctx.eventBus check treats null as an object (so null skips the warning); update
the condition used before warnings.push(...) to explicitly detect null/undefined
(e.g. use ctx.eventBus == null or add ctx.eventBus === null) in addition to the
existing typeof check so a null eventBus triggers the warning; locate the check
that references ctx, eventBus and warnings and change the conditional to include
the null guard so plugins using useEvents() get the warning when eventBus is
null or missing.
---
Nitpick comments:
In `@apps/web-next/src/app/cdn/plugins/__tests__/cdn-serve.test.ts`:
- Around line 84-130: Tests contain a duplicated assertion for the same input
"dashboardProviderMock" (the
expect(toKebabCase('dashboardProviderMock')).toBe('dashboard-provider-mock')
appears twice); remove the redundant assertion — e.g., delete the duplicate
expect inside the "handles triple-word camelCase" test (or remove the earlier
identical check in "handles unknown/new plugin names") so each input is only
tested once while keeping coverage for toKebabCase.
In `@examples/hello-world/frontend/src/mount.tsx`:
- Around line 12-15: The exports use (plugin as any) which loosens types—create
a small local interface (e.g., PluginShape) that declares mount: typeof
plugin.mount, and optional unmount?: ..., getContext?: ..., metadata?: { name:
string; version?: string } and then cast plugin to that interface before
exporting mount, unmount, getContext, and metadata so you avoid any while
preserving optional fields; update references to plugin in the file to use this
local type rather than any.
In `@examples/todo-list/frontend/src/mount.tsx`:
- Around line 12-15: The current code uses (plugin as any) to access unmount,
getContext, and metadata; define a small local interface (e.g., PluginShape)
that declares mount: typeof plugin.mount, optional unmount?: ..., optional
getContext?: ..., and optional metadata?: { name: string; version: string } (or
appropriate types) and then cast plugin to that PluginShape when exporting
unmount, getContext, and metadata; update the exports to use the typed plugin
reference (e.g., const typedPlugin = plugin as PluginShape) so you avoid any
while keeping optional fields tolerated.
Comment on lines
+77
to
+79
| // Resolve plugin directory: deterministic camelCase → kebab-case conversion | ||
| // No hardcoded map needed — all plugin directories follow this convention | ||
| const pluginDir = toKebabCase(pluginName); |
There was a problem hiding this comment.
Validate pluginName and version before path joining to prevent traversal.
fileName is sanitized, but pluginName and version can still inject .. segments and escape dist/plugins. Please add a strict allowlist pattern (kebab-case + semver-like) before computing pluginDir or versionDir.
🔒 Suggested hardening
const { pluginName, version, file } = resolvedParams;
const fileName = file.join('/');
+ // Security: Validate pluginName and version segments
+ const isValidPlugin = /^[a-zA-Z0-9-]+$/.test(pluginName) && pluginName !== '.' && pluginName !== '..';
+ const isValidVersion = /^[0-9A-Za-z.-]+$/.test(version) && version !== '.' && version !== '..';
+ if (!isValidPlugin || !isValidVersion) {
+ return NextResponse.json(
+ { error: 'Invalid plugin name or version' },
+ { status: 400 }
+ );
+ }
+
// Resolve plugin directory: deterministic camelCase → kebab-case conversion
// No hardcoded map needed — all plugin directories follow this convention
const pluginDir = toKebabCase(pluginName);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Resolve plugin directory: deterministic camelCase → kebab-case conversion | |
| // No hardcoded map needed — all plugin directories follow this convention | |
| const pluginDir = toKebabCase(pluginName); | |
| const { pluginName, version, file } = resolvedParams; | |
| const fileName = file.join('/'); | |
| // Security: Validate pluginName and version segments | |
| const isValidPlugin = /^[a-zA-Z0-9-]+$/.test(pluginName) && pluginName !== '.' && pluginName !== '..'; | |
| const isValidVersion = /^[0-9A-Za-z.-]+$/.test(version) && version !== '.' && version !== '..'; | |
| if (!isValidPlugin || !isValidVersion) { | |
| return NextResponse.json( | |
| { error: 'Invalid plugin name or version' }, | |
| { status: 400 } | |
| ); | |
| } | |
| // Resolve plugin directory: deterministic camelCase → kebab-case conversion | |
| // No hardcoded map needed — all plugin directories follow this convention | |
| const pluginDir = toKebabCase(pluginName); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web-next/src/app/cdn/plugins/`[pluginName]/[version]/[...file]/route.ts
around lines 77 - 79, The code is missing validation of pluginName and version
before building pluginDir/versionDir, allowing path traversal; add strict
allowlist checks (e.g., kebab-case pattern for pluginName and a semver-like
pattern for version) and reject/throw (or return 400) if they fail before
calling toKebabCase or joining paths; ensure pluginDir and versionDir are
derived only after validation and consider normalizing with path.posix.join to
keep them inside the intended dist/plugins root (references: pluginName,
version, pluginDir, versionDir, toKebabCase).
Comment on lines
660
to
666
| container.innerHTML = ` | ||
| <div class="plugin-error p-4 bg-red-50 border border-red-200 rounded-lg"> | ||
| <h3 class="text-red-800 font-semibold">Plugin Error</h3> | ||
| <p class="text-red-600 text-sm mt-1">${err instanceof Error ? err.message : 'Failed to mount plugin'}</p> | ||
| <h3 class="text-red-800 font-semibold">Plugin Error: ${plugin.name}</h3> | ||
| <p class="text-red-600 text-sm mt-1">${errMsg}</p> | ||
| <p class="text-red-400 text-xs mt-2">Check browser console for details.</p> | ||
| </div> | ||
| `; |
There was a problem hiding this comment.
XSS vulnerability: Unsanitized content in innerHTML.
The plugin.name and errMsg values are interpolated directly into innerHTML without escaping. Both values can be influenced by external sources:
plugin.nameoriginates from URL parameters/CDN resolutionerrMsgcan contain arbitrary content from thrown errors
An attacker could craft a malicious plugin name or trigger an error with script injection.
🔒 Proposed fix: Escape HTML entities
+function escapeHtml(unsafe: string): string {
+ return unsafe
+ .replace(/&/g, '&')
+ .replace(/</g, '<')
+ .replace(/>/g, '>')
+ .replace(/"/g, '"')
+ .replace(/'/g, ''');
+}
+
// In mountUMDPlugin:
} catch (err) {
const errMsg = err instanceof Error ? err.message : String(err);
console.error(
`[NAAP Plugin Error] Plugin "${plugin.name}" threw during mount():\n` +
` ✗ ${errMsg}\n` +
` → This usually means the plugin's React component tree failed to render.\n` +
` → Common causes: missing dependencies, undefined props, or import errors.`
);
container.innerHTML = `
<div class="plugin-error p-4 bg-red-50 border border-red-200 rounded-lg">
- <h3 class="text-red-800 font-semibold">Plugin Error: ${plugin.name}</h3>
- <p class="text-red-600 text-sm mt-1">${errMsg}</p>
+ <h3 class="text-red-800 font-semibold">Plugin Error: ${escapeHtml(plugin.name)}</h3>
+ <p class="text-red-600 text-sm mt-1">${escapeHtml(errMsg)}</p>
<p class="text-red-400 text-xs mt-2">Check browser console for details.</p>
</div>
`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web-next/src/lib/plugins/umd-loader.ts` around lines 660 - 666, The code
sets container.innerHTML with unsanitized plugin.name and errMsg which allows
XSS; update the error-rendering in the UMD loader to avoid inserting raw HTML by
either escaping HTML entities for plugin.name and errMsg or, better, build the
DOM using document.createElement and set textContent on elements (e.g., create a
wrapper div, h3, p elements) instead of using container.innerHTML; ensure you
update the code paths around container, plugin.name and errMsg in the function
that renders plugin errors so all user-controlled strings are never inserted as
raw HTML.
Comment on lines
+66
to
+68
| │ │ ├── App.tsx # Plugin definition via createPlugin() | ||
| │ │ ├── mount.tsx # UMD entry point (delegates to App.tsx) | ||
| │ │ ├── main.tsx # Standalone dev entry point |
There was a problem hiding this comment.
Update the later “File Structure Reference” to match this new entry-point split.
The tree now shows mount.tsx and main.tsx, but the “File Structure Reference” section still describes App.tsx as the sole mount entry. Consider updating that section to avoid mixed guidance.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plugin-developer-guide.md` around lines 66 - 68, The "File Structure
Reference" is out of date: update its description to reflect the split entry
points by documenting App.tsx as the plugin definition (createPlugin()),
mount.tsx as the UMD entry point that delegates to App.tsx, and main.tsx as the
standalone dev entry point; edit the section text to mention all three files and
clarify their responsibilities (plugin definition vs UMD mount vs dev bootstrap)
so it matches the tree listing (App.tsx, mount.tsx, main.tsx).
Comment on lines
46
to
+48
| # Edit files: | ||
| # - frontend/src/App.tsx (must export mount()) | ||
| # - frontend/src/App.tsx (uses createPlugin() — the canonical mount pattern) | ||
| # - frontend/src/mount.tsx (UMD entry, delegates to App.tsx — rarely needs editing) |
There was a problem hiding this comment.
Update the App.tsx example below to match the new createPlugin pattern.
The “Essential Files → frontend/src/App.tsx” snippet still shows the legacy mount() pattern, which now contradicts these updated notes. Please update that example (or move it to mount.tsx) to reflect the canonical createPlugin flow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plugin-developer-quickstart.md` around lines 46 - 48, The App.tsx
example still shows the legacy mount() pattern; update the snippet to use the
canonical createPlugin() flow (or move the legacy mount() example into
mount.tsx) so examples match the docs. Replace the mount() usage in the
"frontend/src/App.tsx" snippet with a createPlugin() implementation that returns
the plugin object and exposes the expected lifecycle/hooks, ensure any top-level
export or registration matches createPlugin(), and update comments to reference
createPlugin() instead of mount(); if you choose to retain mount() show it only
in mount.tsx as the UMD entry delegating to App.tsx.
Comment on lines
+142
to
+147
| if (!('eventBus' in ctx) || typeof ctx.eventBus !== 'object') { | ||
| warnings.push( | ||
| `Shell context is missing eventBus object.` + | ||
| `\n → Plugins that use useEvents() may fail.` | ||
| ); | ||
| } |
There was a problem hiding this comment.
Handle eventBus: null explicitly.
typeof null === 'object', so a null eventBus currently skips the warning and can mask an invalid context. Consider a null guard here.
🛠️ Proposed fix
- if (!('eventBus' in ctx) || typeof ctx.eventBus !== 'object') {
+ if (!('eventBus' in ctx) || ctx.eventBus === null || typeof ctx.eventBus !== 'object') {
warnings.push(
`Shell context is missing eventBus object.` +
`\n → Plugins that use useEvents() may fail.`
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!('eventBus' in ctx) || typeof ctx.eventBus !== 'object') { | |
| warnings.push( | |
| `Shell context is missing eventBus object.` + | |
| `\n → Plugins that use useEvents() may fail.` | |
| ); | |
| } | |
| if (!('eventBus' in ctx) || ctx.eventBus === null || typeof ctx.eventBus !== 'object') { | |
| warnings.push( | |
| `Shell context is missing eventBus object.` + | |
| `\n → Plugins that use useEvents() may fail.` | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/plugin-sdk/src/utils/contract-validation.ts` around lines 142 - 147,
The current guard in contract-validation.ts around the ctx.eventBus check treats
null as an object (so null skips the warning); update the condition used before
warnings.push(...) to explicitly detect null/undefined (e.g. use ctx.eventBus ==
null or add ctx.eventBus === null) in addition to the existing typeof check so a
null eventBus triggers the warning; locate the check that references ctx,
eventBus and warnings and change the conditional to include the null guard so
plugins using useEvents() get the warning when eventBus is null or missing.
There was a problem hiding this comment.
Pull request overview
This PR streamlines the NAAP plugin developer experience by standardizing plugin mounting around createPlugin(), reducing legacy build/config surface area, and adding runtime validation with clearer error reporting across SDK, scaffolding, examples, and the shell’s UMD loader.
Changes:
- Introduces runtime contract validation utilities and wires them into plugin mounting and UMD loading paths.
- Updates the plugin SDK CLI scaffold + example plugins/docs to use the canonical
createPlugin()+ delegatedmount.tsxpattern (plus standalonemain.tsx). - Simplifies CDN plugin serving by replacing the hardcoded directory map with deterministic camelCase→kebab-case conversion; removes unused UMD Vite base config and deprecates legacy config factories.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| review-pluginQA.md | Adds a DevX/architecture quality assessment report for the work in this branch. |
| packages/plugin-sdk/src/utils/mount.ts | Adds runtime validation hooks and construction-time checks to createPluginMount/createPlugin. |
| packages/plugin-sdk/src/utils/contract-validation.ts | New runtime validators for plugin modules and shell context + formatter for actionable messages. |
| packages/plugin-sdk/src/utils/tests/contract-validation.test.ts | Unit tests covering validation behavior and formatted output. |
| packages/plugin-sdk/src/umd/index.ts | Deprecates UMD helpers and documents migration toward createPlugin(). |
| packages/plugin-sdk/src/index.ts | Re-exports new validation utilities and selected headers/types for the “2-package” dev model. |
| packages/plugin-sdk/cli/commands/create.ts | Updates scaffold to generate App.tsx using createPlugin() and a delegating UMD mount.tsx. |
| packages/plugin-sdk/cli/commands/tests/create.test.ts | Updates scaffold tests to assert the new delegate + createPlugin() pattern. |
| packages/plugin-sdk/README.md | Documents canonical createPlugin() usage and the 2-package model for plugin developers. |
| packages/plugin-build/vite.umd.base.ts | Deletes unused UMD Vite base config implementation. |
| packages/config/src/vite-plugin.ts | Deprecates legacy Vite config factory and provides migration guidance. |
| examples/todo-list/frontend/src/mount.tsx | Adds delegated UMD mount entry wiring to the plugin instance from App.tsx. |
| examples/todo-list/frontend/src/main.tsx | Adds standalone dev entry point for running the plugin outside the shell. |
| examples/todo-list/frontend/package.json | Updates peer dependency to @naap/plugin-sdk per the 2-package model. |
| examples/hello-world/frontend/src/mount.tsx | Adds delegated UMD mount entry wiring to the plugin instance from App.tsx. |
| examples/hello-world/frontend/src/main.tsx | Adds standalone dev entry point for running the plugin outside the shell. |
| examples/hello-world/frontend/src/App.tsx | Migrates hello-world example to createPlugin() and removes manual mount boilerplate. |
| examples/hello-world/frontend/package.json | Updates peer dependency to @naap/plugin-sdk per the 2-package model. |
| docs/plugin-developer-quickstart.md | Updates quickstart to reflect createPlugin() as the canonical mount pattern and new mount.tsx role. |
| docs/plugin-developer-guide.md | Updates recommended plugin project structure to include mount.tsx + main.tsx. |
| apps/web-next/src/lib/plugins/umd-loader.ts | Improves UMD load/mount error messages with actionable guidance and UI error rendering. |
| apps/web-next/src/app/cdn/plugins/tests/cdn-serve.test.ts | Updates CDN tests to validate deterministic camelCase→kebab-case conversion behavior. |
| apps/web-next/src/app/cdn/plugins/[pluginName]/[version]/[...file]/route.ts | Replaces hardcoded plugin directory mapping with deterministic camelCase→kebab-case conversion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
660
to
666
| container.innerHTML = ` | ||
| <div class="plugin-error p-4 bg-red-50 border border-red-200 rounded-lg"> | ||
| <h3 class="text-red-800 font-semibold">Plugin Error</h3> | ||
| <p class="text-red-600 text-sm mt-1">${err instanceof Error ? err.message : 'Failed to mount plugin'}</p> | ||
| <h3 class="text-red-800 font-semibold">Plugin Error: ${plugin.name}</h3> | ||
| <p class="text-red-600 text-sm mt-1">${errMsg}</p> | ||
| <p class="text-red-400 text-xs mt-2">Check browser console for details.</p> | ||
| </div> | ||
| `; |
There was a problem hiding this comment.
mountUMDPlugin interpolates plugin.name and especially errMsg into container.innerHTML. Since errMsg can include arbitrary text from a plugin (or from upstream data in an error message), this creates an XSS injection risk. Prefer rendering a safe error UI via DOM APIs/React, or at minimum HTML-escape interpolated values before assigning to innerHTML.
Comment on lines
133
to
+141
| const mount: PluginMountFn = (container: HTMLElement, context: ShellContext) => { | ||
| // Runtime context validation — emit warnings in development, never throw | ||
| if (process.env.NODE_ENV !== 'production') { | ||
| const ctxResult = validateShellContext(context); | ||
| if (!ctxResult.valid || ctxResult.warnings.length > 0) { | ||
| const pluginName = (options as unknown as Record<string, unknown>).name as string || 'unknown'; | ||
| console.warn(formatPluginError(pluginName, 'mount', ctxResult)); | ||
| } | ||
| } |
There was a problem hiding this comment.
In createPluginMount, the dev-only warning tries to read options.name, but CreatePluginMountOptions doesn't define a name field. This will always log unknown for plugins created via createPlugin() (since name is stripped before calling createPluginMount). Consider passing pluginName into createPluginMount (or adding an optional name to its options) so formatPluginError reports the real plugin name.
Comment on lines
+248
to
+254
| if (typeof mountOptions.App !== 'function') { | ||
| const got = mountOptions.App === null ? 'null' : typeof mountOptions.App; | ||
| throw new Error( | ||
| `[NAAP Plugin "${name}"] App must be a React component (function), got: ${got}.` + | ||
| `\n → Pass your root component: createPlugin({ ..., App: MyApp })` | ||
| ); | ||
| } |
There was a problem hiding this comment.
The runtime validation for App formats the error as [NAAP Plugin "${name}"] ..., but name isn't validated until later. If name is missing/invalid, this message will include undefined (or another non-string) and be less actionable. Validate name (and ideally version) before using them in error strings.
Comment on lines
242
to
245
| export function createPlugin( | ||
| options: CreatePluginMountOptions & PluginMetadata | ||
| ): PluginModule & PluginMetadata { | ||
| const { name, version, routes, ...mountOptions } = options; |
There was a problem hiding this comment.
createPlugin() returns an object that includes getContext, but the function signature is PluginModule & PluginMetadata which does not expose getContext to consumers. This forces downstream code to use as any casts (see several example mount.tsx files). Consider including getContext in the public return type (e.g., PluginModule & PluginMetadata & { getContext: () => ShellContext | null }) so callers can use it without type escapes.
- Remove unsupported --include flag from lifecycle BDD step (use --dir only) - CDN: validate pluginName and version before path join to prevent traversal - UMD loader: escape HTML in error UI to prevent XSS (plugin.name, errMsg) - contract-validation: handle eventBus: null explicitly (typeof null === 'object') - Add CDN validation unit tests for pluginName/version allowlist Co-authored-by: Cursor <cursoragent@cursor.com>
seanhanca
deleted the
feat/architecture-refactoring-simplification-devx
branch
3 tasks
This was referenced Feb 27, 2026
3 tasks
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.
Summary
Systematic architecture simplification addressing the 5 highest-ROI developer experience improvements identified in the 100-point DevX assessment. All changes are on a single feature branch with atomic, independently revertible commits.
Changes
createPlugin()is now the only mount pattern. Updated scaffolder, migrated hello-world and todo-list examples, addedmount.tsxdelegate andmain.tsxstandalone entry.vite.umd.base.ts, deprecatedcreatePluginViteConfig()and UMD helpers with migration guides.validatePluginModule(),validateShellContext(),formatPluginError()with human-readable, actionable error messages. Wired intocreatePlugin(),mount(), and UMD loader.toKebabCase(). New plugins no longer require shell code changes for CDN serving.@naap/plugin-sdk. Documented that plugin devs only need@naap/plugin-sdk+@naap/plugin-build.Quality Score
Before: 62/100 → After: 83/100 (+21 points)
Test plan
regression-guard.sh: 8/8 checks passMade with Cursor
Summary by CodeRabbit
New Features
Documentation
createPluginpattern and recommended project structure including standalone development and UMD entry points.Refactor