feat(devx): Simplify full-stack plugin onboarding with frontend-first defaults and add commands#119
Conversation
…nd mode - Reorder TEMPLATES to place frontend-only first as recommended default - Change fallback template from full-stack to frontend-only - Add --simple flag for full-stack without Prisma/Docker dependencies - Scaffold Express + in-memory CRUD backend for simple mode - Add next-steps messaging guiding frontend-only users to upgrade paths - Add 10 new tests covering default template behavior and simple mode Co-authored-by: Cursor <cursoragent@cursor.com>
- New `naap-plugin add model <Name> [fields...]` inserts Prisma model into unified schema with @@Schema annotation and datasource expansion - New `naap-plugin add endpoint <name> [--crud]` scaffolds route file and wires it into the route aggregator idempotently - Both commands enforce idempotency: duplicate model/route detection prevents accidental overwrites (unless --force) - Monorepo context detection with clear failure messages for standalone - 20 new tests covering helpers, integration, and CLI discoverability Co-authored-by: Cursor <cursoragent@cursor.com>
- Add routes/README.md to both full-stack and simple backend scaffolds with step-by-step "add route + register" instructions - Improve dev command tips: surface `add endpoint` and `add model` commands in the dev server status output - Clarify --with-shell is for monorepo development path Co-authored-by: Cursor <cursoragent@cursor.com>
- Update CLI reference: add `naap-plugin add model` and `add endpoint` docs, update `create` to show frontend-first default and --simple flag - Update developer guide: frontend-first messaging, createPlugin() pattern, file structure with routes/README.md - Update quickstart: frontend-first and simple full-stack paths - Update "Your First Plugin" guide: add model CLI shortcut, --simple tip - New: docs/simplified-plugin-development.md (progressive disclosure overview) - New: docs/how-to-add-backend-endpoint.md (step-by-step tutorial) Co-authored-by: Cursor <cursoragent@cursor.com>
… and add command - New BDD scenarios: default create => frontend-only (2 tests) - New BDD scenario: full-stack simple => no Prisma scaffold (1 test) - New BDD scenarios: add command discoverability and idempotency (5 tests) - Full regression matrix: SDK 272/272, BDD 27/27, CDN 24/24, Guard 8/8 Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (CLI)
participant CLI as naap-plugin add
participant FS as FileSystem (plugin files)
participant Aggregator as routes aggregator
participant Prisma as Prisma schema
User->>CLI: run `naap-plugin add model <Name> [fields]` / `add endpoint <name> [--crud]`
CLI->>FS: resolve plugin context (read plugin.json, detect monorepo, locate schema/paths)
alt add model
CLI->>Prisma: read unified schema.prisma
CLI->>CLI: buildModelBlock(name, schemaName, fields)
CLI->>Prisma: check modelExistsInSchema
alt model missing or --force
CLI->>Prisma: insert model block and ensure datasource entry
CLI->>FS: write updated schema.prisma
CLI->>User: print next steps (prisma generate/push)
else model exists
CLI->>User: skip or overwrite depending on --force
end
else add endpoint
CLI->>FS: generate route file (generateEndpointFile)
CLI->>Aggregator: read aggregator index, registrationExists?
alt registration missing
CLI->>Aggregator: append import/use lines
CLI->>FS: write updated aggregator index
else already registered
CLI->>User: report idempotent skip or --force override
end
end
CLI->>User: success/follow-up guidance
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
There was a problem hiding this comment.
Pull request overview
This PR implements a progressive disclosure approach to plugin development, making NAAP more accessible to new developers by defaulting to frontend-only scaffolding and providing simpler full-stack options without requiring Docker or Prisma.
Changes:
- Changed default plugin template from
full-stacktofrontend-onlywith--simpleflag for in-memory backends - Added
naap-plugin add modelandnaap-plugin add endpointcommands for incremental scaffolding with idempotency - Enhanced developer experience with route discoverability guides and contextual CLI tips
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
packages/plugin-sdk/cli/index.ts |
Registers new add command |
packages/plugin-sdk/cli/commands/create.ts |
Changes default template to frontend-only, adds --simple flag and createBackendSimple() function |
packages/plugin-sdk/cli/commands/add.ts |
New command implementing add model and add endpoint with idempotency guards |
packages/plugin-sdk/cli/commands/dev.ts |
Updates tips to mention new add commands with monorepo clarification |
packages/plugin-sdk/cli/commands/__tests__/create.test.ts |
Adds tests for frontend-first defaults and simple backend mode |
packages/plugin-sdk/cli/commands/__tests__/add.test.ts |
New test suite for add command helpers and integration scenarios |
tests/lifecycle-bdd/plugin-lifecycle.feature.test.ts |
Adds BDD scenarios for new features |
docs/simplified-plugin-development.md |
New guide explaining progressive disclosure approach |
docs/plugin-developer-quickstart.md |
Updated with new commands and workflow |
docs/plugin-developer-guide.md |
Updated with frontend-first guidance and new scaffolding options |
docs/how-to-add-backend-endpoint.md |
New tutorial for adding endpoints |
apps/web-next/src/content/docs/guides/your-first-plugin.mdx |
Updated with tip about simpler start options |
apps/web-next/src/content/docs/api-reference/cli.mdx |
Updated CLI reference with new commands and options |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await fs.writeFile(path.join(backendDir, 'src', 'routes', 'README.md'), `# Adding Routes | ||
|
|
||
| ## Quick steps | ||
|
|
||
| 1. Create a new file, e.g. \`users.ts\`: | ||
| \`\`\`ts | ||
| import { Router } from 'express'; | ||
| export const usersRouter = Router(); | ||
| usersRouter.get('/', (_req, res) => res.json({ users: [] })); | ||
| \`\`\` | ||
|
|
||
| 2. Register it in \`index.ts\`: | ||
| \`\`\`ts | ||
| import { usersRouter } from './users.js'; | ||
| router.use('/users', usersRouter); | ||
| \`\`\` | ||
|
|
||
| Or run: \`naap-plugin add endpoint users --crud\` | ||
| `); |
There was a problem hiding this comment.
The routes/README.md content is duplicated between the createBackend function (lines 616-634) and the createBackendSimple function (lines 838-856). Consider extracting this to a shared constant or helper function to avoid maintaining identical content in two places.
| if (existing.includes(`"${schemaName}"`)) return schemaContent; | ||
| const newSchemas = existing.trimEnd() + `, "${schemaName}"`; |
There was a problem hiding this comment.
The ensureSchemaInDatasource function uses a simple string includes check to detect if a schema name already exists. This could lead to false positives if a schema name is a substring of another (e.g., "plugin_a" would match "plugin_abc"). Consider using a more robust check that ensures the schema name is a complete match within the array, such as parsing the JSON array or using word boundaries in the regex.
| if (existing.includes(`"${schemaName}"`)) return schemaContent; | |
| const newSchemas = existing.trimEnd() + `, "${schemaName}"`; | |
| // Parse existing schemas into individual entries and compare by exact name. | |
| const existingSchemas = existing | |
| .split(',') | |
| .map(entry => entry.trim()) | |
| .filter(entry => entry.length > 0) | |
| .map(entry => { | |
| const m = /^"(.*)"$/.exec(entry); | |
| return m ? m[1] : entry; | |
| }); | |
| if (existingSchemas.includes(schemaName)) { | |
| return schemaContent; | |
| } | |
| const trimmedExisting = existing.trim(); | |
| const newSchemas = | |
| trimmedExisting.length === 0 | |
| ? `"${schemaName}"` | |
| : existing.trimEnd() + `, "${schemaName}"`; |
| for (const field of fields) { | ||
| const parts = field.split(':'); | ||
| const fname = parts[0]; | ||
| const ftype = parts.length > 1 ? parts[1] : 'String'; |
There was a problem hiding this comment.
The field type capitalization logic simply capitalizes the first character (ftype.charAt(0).toUpperCase() + ftype.slice(1)), which works for standard Prisma types like 'String', 'Int', 'Boolean', etc. However, this doesn't validate whether the type is a valid Prisma type. If a user provides an invalid type like 'string' (lowercase), it will be capitalized to 'String', which is correct. But if they provide something like 'myCustomType', it will become 'MyCustomType', which may not be a valid Prisma type. Consider adding validation or a comment explaining valid Prisma types.
| const ftype = parts.length > 1 ? parts[1] : 'String'; | |
| const ftype = parts.length > 1 ? parts[1] : 'String'; | |
| // Note: This only normalizes the field type's first character for convenience | |
| // (e.g. "string" -> "String", "int" -> "Int"). It does NOT validate that | |
| // the resulting type is a valid Prisma type. Callers should ensure that | |
| // `prismaType` is a valid scalar (String, Int, Boolean, DateTime, etc.) or | |
| // a model/enum name that actually exists in the Prisma schema. |
| it('ensureSchemaInDatasource adds missing schema name', () => { | ||
| const schema = 'schemas = ["public", "plugin_a"]'; | ||
| const result = ensureSchemaInDatasource(schema, 'plugin_b'); | ||
| expect(result).toContain('"plugin_b"'); | ||
| }); | ||
|
|
||
| it('ensureSchemaInDatasource is idempotent when schema already present', () => { | ||
| const schema = 'schemas = ["public", "plugin_a"]'; | ||
| const result = ensureSchemaInDatasource(schema, 'plugin_a'); | ||
| expect(result).toBe(schema); | ||
| }); | ||
|
|
||
| it('ensureSchemaInDatasource returns unchanged if no schemas array found', () => { | ||
| const schema = 'generator client { }'; | ||
| const result = ensureSchemaInDatasource(schema, 'plugin_x'); | ||
| expect(result).toBe(schema); | ||
| }); |
There was a problem hiding this comment.
The test suite for ensureSchemaInDatasource doesn't cover the edge case where a schema name is a substring of another schema name (e.g., testing with 'plugin_a' when 'plugin_abc' exists). This would help catch the substring matching bug in the implementation. Consider adding a test case like: ensureSchemaInDatasource('schemas = ["plugin_abc"]', 'plugin_a') to ensure it correctly adds 'plugin_a' instead of incorrectly detecting it as already present.
| lines.push(` // TODO: create ${name}`); | ||
| lines.push(` res.status(201).json({ ...req.body });`); | ||
| lines.push('});'); | ||
| lines.push(''); | ||
| lines.push(`${routerName}.put('/:id', async (req, res) => {`); | ||
| lines.push(` // TODO: update ${name}`); |
There was a problem hiding this comment.
The generated CRUD endpoint skeleton returns the entire request body in POST and PUT operations using the spread operator ({ ...req.body }). This could inadvertently expose or persist unintended fields sent by the client. While this is a TODO scaffold, consider adding a comment reminding developers to validate and whitelist specific fields before saving, e.g., // TODO: Validate and extract only allowed fields from req.body.
| lines.push(` // TODO: create ${name}`); | |
| lines.push(` res.status(201).json({ ...req.body });`); | |
| lines.push('});'); | |
| lines.push(''); | |
| lines.push(`${routerName}.put('/:id', async (req, res) => {`); | |
| lines.push(` // TODO: update ${name}`); | |
| lines.push(` // TODO: create ${name}`); | |
| lines.push(` // TODO: Validate and extract only allowed fields from req.body before saving or returning`); | |
| lines.push(` res.status(201).json({ ...req.body });`); | |
| lines.push('});'); | |
| lines.push(''); | |
| lines.push(`${routerName}.put('/:id', async (req, res) => {`); | |
| lines.push(` // TODO: update ${name}`); | |
| lines.push(` // TODO: Validate and extract only allowed fields from req.body before saving or returning`); |
| } | ||
|
|
||
| function toSnakeCase(str: string): string { | ||
| return str.replace(/[-]+/g, '_').toLowerCase(); |
There was a problem hiding this comment.
The toSnakeCase function only handles hyphens but not underscores in the input. If a plugin name already contains underscores (e.g., "my_test_plugin"), consecutive underscores would not be collapsed to a single underscore. Consider using the same pattern as toPascalCase for consistency: str.replace(/[-_]+/g, '_').toLowerCase() would become str.replace(/[-_]+/g, '_').replace(/_+/g, '_').toLowerCase() to normalize consecutive underscores, or simply ensure the replace pattern handles both cases properly.
| return str.replace(/[-]+/g, '_').toLowerCase(); | |
| return str.replace(/[-_]+/g, '_').toLowerCase(); |
| const lastImportIdx = aggregator.lastIndexOf('import '); | ||
| if (lastImportIdx !== -1) { | ||
| const lineEnd = aggregator.indexOf('\n', lastImportIdx); | ||
| aggregator = aggregator.slice(0, lineEnd + 1) + importLine + '\n' + aggregator.slice(lineEnd + 1); |
There was a problem hiding this comment.
The import line insertion logic uses lastIndexOf to find the last import statement, but then inserts after it. This could result in the new import being placed after comments or blank lines that follow the last import, rather than being grouped with other imports. Consider finding the end of the import block more robustly, or inserting the import immediately after the last import line without additional blank lines in between.
| const lastImportIdx = aggregator.lastIndexOf('import '); | |
| if (lastImportIdx !== -1) { | |
| const lineEnd = aggregator.indexOf('\n', lastImportIdx); | |
| aggregator = aggregator.slice(0, lineEnd + 1) + importLine + '\n' + aggregator.slice(lineEnd + 1); | |
| const lines = aggregator.split('\n'); | |
| let lastImportLineIndex = -1; | |
| for (let i = 0; i < lines.length; i++) { | |
| if (/^\s*import\s/.test(lines[i])) { | |
| lastImportLineIndex = i; | |
| } | |
| } | |
| if (lastImportLineIndex !== -1) { | |
| lines.splice(lastImportLineIndex + 1, 0, importLine); | |
| aggregator = lines.join('\n'); |
| const app = express(); | ||
| const PORT = process.env.PORT || ${backendPort}; | ||
|
|
||
| app.use(cors()); |
There was a problem hiding this comment.
The CORS configuration uses cors() without any options, allowing requests from any origin. While this is acceptable for development and the scaffold is marked as "in-memory" (implying it's for development), consider adding a comment to remind developers to configure CORS properly for production, or use environment-based configuration such as cors({ origin: process.env.ALLOWED_ORIGINS?.split(',') || '*' }).
| app.use(cors()); | |
| const allowedOrigins = process.env.ALLOWED_ORIGINS | |
| ? process.env.ALLOWED_ORIGINS.split(',') | |
| : ['*']; | |
| app.use(cors({ | |
| origin: allowedOrigins.includes('*') ? '*' : allowedOrigins, | |
| })); |
| router.post('/', (req, res) => { | ||
| const { name } = req.body; | ||
| if (!name || typeof name !== 'string') { | ||
| res.status(400).json({ error: 'name is required' }); | ||
| return; | ||
| } | ||
| const item = { id: String(nextId++), name, createdAt: new Date().toISOString() }; | ||
| items.push(item); | ||
| res.status(201).json(item); | ||
| }); |
There was a problem hiding this comment.
The POST endpoint lacks validation for the maximum length of the name field. An attacker could send an extremely long string that could cause memory issues or denial of service. Consider adding a length validation, e.g., if (!name || typeof name !== 'string' || name.length > 1000) to protect against potential abuse.
- Extract ROUTES_README_CONTENT to shared constant in create.ts - Fix ensureSchemaInDatasource substring false-positive (plugin_a vs plugin_abc) - Add Prisma type normalization comment in buildModelBlock - Add substring edge-case test for ensureSchemaInDatasource Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
tests/lifecycle-bdd/plugin-lifecycle.feature.test.ts (1)
249-266: Consider reducing brittleness of source-string assertions.
The tests couple to exact source text/formatting. A higher-level check (e.g., invoking the create command or checking exported defaults/constants) would be more resilient.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/lifecycle-bdd/plugin-lifecycle.feature.test.ts` around lines 249 - 266, The tests are brittle because they assert on raw source strings; instead import/require the create command module and assert on exported values or behavior: load the module that defines the create command (the create command export in create.ts) and check the exported TEMPLATES array ordering (ensure the index of 'frontend-only' is less than 'full-stack') and/or check the exported default template or DEFAULT_TEMPLATE/PluginTemplate fallback used by the command rather than matching source text; update the two tests to import the create module and assert on those exported constants or call the create command's function to observe the default template selection.
🤖 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/content/docs/api-reference/cli.mdx`:
- Around line 124-171: Add a short note to the "naap-plugin add model" section
clarifying that the command writes to the unified Prisma schema file
(packages/database/prisma/schema.prisma) and therefore only works in monorepo
setups that include the unified-schema; mention that standalone plugin
repositories without that path should not use this command or must create the
same schema location first, and include this constraint near the description or
examples for "naap-plugin add model".
In `@docs/how-to-add-backend-endpoint.md`:
- Around line 112-118: Update the docs to avoid a hardcoded change-directory:
replace the lone "cd packages/database" instruction with a short note offering
two options—either run the shown Prisma commands from the monorepo root, or, if
starting from inside a plugin directory, change into the repository's database
package using the correct relative path from your current location—then run "npx
prisma db push" and "npx prisma generate"; ensure the text explicitly tells
readers which starting location each option assumes.
In `@packages/plugin-sdk/cli/commands/add.ts`:
- Around line 89-96: The function ensureSchemaInDatasource currently builds
newSchemas by blindly appending `, "schemaName"` which yields `schemas = [,
"name"]` when the matched existing group is empty; update the logic in
ensureSchemaInDatasource to detect an empty existing string (e.g., check if
existing.trim() is empty) and in that case set newSchemas to just
`"<schemaName>"`, otherwise keep the current `existing.trimEnd() + ',
"<schemaName>"' behavior, then perform the replacement using the same dsRegex;
this fixes empty `schemas = []` without changing the regex or overall flow.
- Around line 119-131: The current logic appends a duplicate model when
opts.force is true; change it to remove/replace the existing model block instead
of appending: detect an existing model via modelExistsInSchema(pascalModel) and
when opts.force is true, strip the existing "model <pascalModel> { ... }" block
from schema (e.g. with a safe regex or dedicated helper like
removeModelBlock(schema, pascalModel)) before calling ensureSchemaInDatasource
and buildModelBlock, then insert the new block in place (or replace the removed
span) so the schema contains only the updated model; keep spinner.info for the
forced overwrite path and ensure spacing/trim is preserved.
- Around line 200-233: Validate and sanitize the incoming name in
handleAddEndpoint before using it in file paths or identifiers: reject values
containing path separators or characters outside a safe pattern (e.g. only
lowercase letters, digits and hyphens) and throw/exit with a clear error if
validation fails to prevent path traversal; derive a routeName (kebab-case, safe
for filenames and URLs) to use for routeFile and router.use paths and derive
routerName by passing routeName through the existing toPascalCase helper and
appending "Router" for use as the TypeScript identifier; update
buildRegistrationLines to accept routeName and routerName (or compute routerName
there via toPascalCase) so importLine uses `./${routeName}.js` and useLine uses
`router.use('/${routeName}', ${routerName});`; ensure generateEndpointFile and
any other usages get the correct routerName (PascalCase) vs routeName
(kebab-case).
- Around line 84-86: The regex in modelExistsInSchema interpolates raw modelName
into new RegExp, which can misbehave for names containing regex metacharacters;
update modelExistsInSchema to escape modelName before building the pattern
(e.g., implement an escapeRegExp helper that replaces /[.*+?^${}()|[\]\\]/g with
"\\$&" and use the escaped value when constructing the regex), then continue to
use the same pattern and 'm' flag so regex logic (the regex variable and
regex.test(schemaContent)) is unchanged except for using the escapedModelName.
In `@packages/plugin-sdk/cli/commands/create.ts`:
- Around line 128-130: The template resolution can silently ignore --simple;
update the logic after computing pluginName/template/targetDir so that if
options.simple is true and neither options.template nor answers.template was
explicitly provided (i.e., the template came from the default), promote the
template to 'full-stack' (set template = 'full-stack') so a backend is created;
additionally, add a guard to throw or print an error if the user explicitly
provided a conflicting template (options.template or answers.template ===
'frontend-only') while also passing --simple, referencing the template variable
and the options.simple flag to locate where to implement this.
---
Nitpick comments:
In `@tests/lifecycle-bdd/plugin-lifecycle.feature.test.ts`:
- Around line 249-266: The tests are brittle because they assert on raw source
strings; instead import/require the create command module and assert on exported
values or behavior: load the module that defines the create command (the create
command export in create.ts) and check the exported TEMPLATES array ordering
(ensure the index of 'frontend-only' is less than 'full-stack') and/or check the
exported default template or DEFAULT_TEMPLATE/PluginTemplate fallback used by
the command rather than matching source text; update the two tests to import the
create module and assert on those exported constants or call the create
command's function to observe the default template selection.
| ### naap-plugin add | ||
|
|
||
| Incrementally add models or endpoints to an existing plugin. | ||
|
|
||
| #### naap-plugin add model | ||
|
|
||
| ```bash | ||
| naap-plugin add model <ModelName> [fields...] [options] | ||
| ``` | ||
|
|
||
| | Option | Description | Default | | ||
| |---|---|---| | ||
| | `--force` | Overwrite if model already exists | `false` | | ||
|
|
||
| Inserts a Prisma model into the unified schema at `packages/database/prisma/schema.prisma` with correct `@@schema()` annotation. Idempotent: skips if the model already exists. | ||
|
|
||
| **Examples:** | ||
|
|
||
| ```bash | ||
| # Add a model with fields | ||
| naap-plugin add model Todo title:String done:Boolean | ||
|
|
||
| # Add a model with default String type | ||
| naap-plugin add model Note content | ||
| ``` | ||
|
|
||
| #### naap-plugin add endpoint | ||
|
|
||
| ```bash | ||
| naap-plugin add endpoint <name> [options] | ||
| ``` | ||
|
|
||
| | Option | Description | Default | | ||
| |---|---|---| | ||
| | `--crud` | Generate full CRUD skeleton (GET, POST, PUT, DELETE) | `false` | | ||
| | `--force` | Overwrite existing route file | `false` | | ||
|
|
||
| Creates a route file and registers it in the route aggregator. Idempotent: skips if the route already exists. | ||
|
|
||
| **Examples:** | ||
|
|
||
| ```bash | ||
| # Add a basic endpoint | ||
| naap-plugin add endpoint users | ||
|
|
||
| # Add a CRUD endpoint | ||
| naap-plugin add endpoint products --crud | ||
| ``` |
There was a problem hiding this comment.
Add a brief monorepo/unified-schema constraint for add model.
The command writes to packages/database/prisma/schema.prisma, which only exists in monorepo setups. Calling this out here prevents confusion in standalone plugin repos.
📚 Suggested doc tweak
- Inserts a Prisma model into the unified schema at `packages/database/prisma/schema.prisma` with correct `@@schema()` annotation. Idempotent: skips if the model already exists.
+ Inserts a Prisma model into the unified schema at `packages/database/prisma/schema.prisma` with correct `@@schema()` annotation (monorepo/unified DB only). Idempotent: skips if the model already exists.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web-next/src/content/docs/api-reference/cli.mdx` around lines 124 - 171,
Add a short note to the "naap-plugin add model" section clarifying that the
command writes to the unified Prisma schema file
(packages/database/prisma/schema.prisma) and therefore only works in monorepo
setups that include the unified-schema; mention that standalone plugin
repositories without that path should not use this command or must create the
same schema location first, and include this constraint near the description or
examples for "naap-plugin add model".
| Then push the schema: | ||
|
|
||
| ```bash | ||
| cd packages/database | ||
| npx prisma db push | ||
| npx prisma generate | ||
| ``` |
There was a problem hiding this comment.
Clarify the monorepo path for Prisma commands.
Line 115’s cd packages/database will fail if the reader is in plugins/<name>. Consider instructing them to run from the monorepo root (or provide a relative path from the plugin directory).
📝 Suggested doc tweak
- cd packages/database
+ # From the monorepo root:
+ cd packages/database📝 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.
| Then push the schema: | |
| ```bash | |
| cd packages/database | |
| npx prisma db push | |
| npx prisma generate | |
| ``` | |
| Then push the schema: | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/how-to-add-backend-endpoint.md` around lines 112 - 118, Update the docs
to avoid a hardcoded change-directory: replace the lone "cd packages/database"
instruction with a short note offering two options—either run the shown Prisma
commands from the monorepo root, or, if starting from inside a plugin directory,
change into the repository's database package using the correct relative path
from your current location—then run "npx prisma db push" and "npx prisma
generate"; ensure the text explicitly tells readers which starting location each
option assumes.
| export function modelExistsInSchema(schemaContent: string, modelName: string): boolean { | ||
| const regex = new RegExp(`^model\\s+${modelName}\\s*\\{`, 'm'); | ||
| return regex.test(schemaContent); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files packages/plugin-sdk/cli/commands/ | head -20Repository: livepeer/naap
Length of output: 959
🏁 Script executed:
cat -n packages/plugin-sdk/cli/commands/add.ts | head -120Repository: livepeer/naap
Length of output: 4841
🏁 Script executed:
rg -n "modelExistsInSchema|handleAddModel|escapeRegExp" -A 5 -B 2Repository: livepeer/naap
Length of output: 10088
🏁 Script executed:
rg -n "modelName" packages/plugin-sdk/cli/commands/add.ts -B 2 -A 2Repository: livepeer/naap
Length of output: 1017
🏁 Script executed:
cat -n packages/plugin-sdk/cli/commands/add.ts | sed -n '260,290p'Repository: livepeer/naap
Length of output: 1302
🏁 Script executed:
rg -n "command.*model|\.arguments|\.argument|parse.*model" packages/plugin-sdk/cli/commands/add.ts -B 2 -A 3Repository: livepeer/naap
Length of output: 324
🏁 Script executed:
cat -n packages/plugin-sdk/cli/commands/add.ts | tail -50Repository: livepeer/naap
Length of output: 2369
🏁 Script executed:
rg -n "validate.*model|sanitize|valid.*name|identifier" packages/plugin-sdk/ -iRepository: livepeer/naap
Length of output: 3466
🏁 Script executed:
rg -n "Prisma|schema" packages/plugin-sdk/cli/commands/add.ts | head -20Repository: livepeer/naap
Length of output: 1350
🏁 Script executed:
cat -n packages/plugin-sdk/cli/commands/__tests__/add.test.ts | sed -n '45,70p'Repository: livepeer/naap
Length of output: 1401
🏁 Script executed:
web_search
Prisma model name constraints valid identifiers requirementsRepository: livepeer/naap
Length of output: 152
🌐 Web query:
Prisma model name identifier constraints valid characters
💡 Result:
In the Prisma schema (PSL), model names and field names are identifiers with these constraints:
- Valid characters / format (models & fields): must match
[A-Za-z][A-Za-z0-9_]*
i.e., start with a letter, then only letters, digits,_(no hyphens, no spaces, cannot start with a number). [1] - Reserved words: some identifiers are reserved by Prisma and can’t be used as model names. [1]
- Per-model identifier requirement (uniqueness): every model must define at least one unique identifier, using one of:
@id,@@id,@unique,@@unique. [1] - If your DB uses “invalid” names (snake_case, hyphens, etc.): keep Prisma identifiers valid and use
@@map(model/table) and@map(field/column) to map to the underlying database names. [1][2]
Sources:
[1] Prisma Schema Reference (naming conventions + uniqueness requirements) (prisma.nodejs.cn)
[2] Prisma docs example of @map / @@map for custom underlying table/column names (prisma.io)
Citations:
- 1: https://prisma.nodejs.cn/orm/reference/prisma-schema-reference?utm_source=openai
- 2: https://www.prisma.io/docs/getting-started/setup-prisma/add-to-existing-project/relational-databases/introspection-typescript-planetscale?utm_source=openai
🏁 Script executed:
rg -n "modelExistsInSchema|regex|escape|metachar" packages/plugin-sdk/cli/commands/__tests__/add.test.ts -B 2 -A 3Repository: livepeer/naap
Length of output: 1909
🏁 Script executed:
cat -n packages/plugin-sdk/cli/commands/add.ts | sed -n '99,130p'Repository: livepeer/naap
Length of output: 1491
🏁 Script executed:
cat -n packages/plugin-sdk/cli/commands/add.ts | sed -n '1,120p' | grep -n "validate\|sanitize\|check"Repository: livepeer/naap
Length of output: 39
🏁 Script executed:
rg -n "handleAddModel|\.action\(" packages/plugin-sdk/cli/commands/add.ts -B 3 -A 10 | head -40Repository: livepeer/naap
Length of output: 1271
🏁 Script executed:
cat -n packages/plugin-sdk/cli/commands/add.ts | sed -n '275,280p'Repository: livepeer/naap
Length of output: 342
🏁 Script executed:
rg -n "command.*model|\.action\(" packages/plugin-sdk/cli/commands/add.ts -A 2Repository: livepeer/naap
Length of output: 361
Escape user input before constructing the model regex.
Line 85 interpolates raw modelName into a RegExp without escaping. Although Prisma schema validation will reject invalid model names at write-time (identifiers must match [A-Za-z][A-Za-z0-9_]*), the function should defensively escape input rather than relying on downstream validation. This follows secure coding practices and prevents unexpected regex behavior.
🛠️ Proposed fix
+function escapeRegExp(value: string): string {
+ return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+}
+
export function modelExistsInSchema(schemaContent: string, modelName: string): boolean {
- const regex = new RegExp(`^model\\s+${modelName}\\s*\\{`, 'm');
+ const safeName = escapeRegExp(modelName);
+ const regex = new RegExp(`^model\\s+${safeName}\\s*\\{`, 'm');
return regex.test(schemaContent);
}📝 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.
| export function modelExistsInSchema(schemaContent: string, modelName: string): boolean { | |
| const regex = new RegExp(`^model\\s+${modelName}\\s*\\{`, 'm'); | |
| return regex.test(schemaContent); | |
| function escapeRegExp(value: string): string { | |
| return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); | |
| } | |
| export function modelExistsInSchema(schemaContent: string, modelName: string): boolean { | |
| const safeName = escapeRegExp(modelName); | |
| const regex = new RegExp(`^model\\s+${safeName}\\s*\\{`, 'm'); | |
| return regex.test(schemaContent); | |
| } |
🧰 Tools
🪛 ast-grep (0.40.5)
[warning] 84-84: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^model\\s+${modelName}\\s*\\{, 'm')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/plugin-sdk/cli/commands/add.ts` around lines 84 - 86, The regex in
modelExistsInSchema interpolates raw modelName into new RegExp, which can
misbehave for names containing regex metacharacters; update modelExistsInSchema
to escape modelName before building the pattern (e.g., implement an escapeRegExp
helper that replaces /[.*+?^${}()|[\]\\]/g with "\\$&" and use the escaped value
when constructing the regex), then continue to use the same pattern and 'm' flag
so regex logic (the regex variable and regex.test(schemaContent)) is unchanged
except for using the escapedModelName.
| if (modelExistsInSchema(schema, pascalModel)) { | ||
| if (!opts.force) { | ||
| spinner.warn(`Model ${pascalModel} already exists in schema — skipping`); | ||
| return; | ||
| } | ||
| spinner.info(`Model ${pascalModel} exists but --force used, appending duplicate`); | ||
| } | ||
|
|
||
| schema = ensureSchemaInDatasource(schema, schemaName); | ||
|
|
||
| const block = buildModelBlock(pascalModel, schemaName, fields); | ||
| schema = schema.trimEnd() + '\n\n' + block + '\n'; | ||
|
|
There was a problem hiding this comment.
--force should overwrite the model, not append a duplicate.
Line 124 explicitly appends a duplicate model block when --force is used. That leaves multiple model <Name> blocks in the unified schema, which Prisma will reject. Overwrite (remove the existing block first) or fail loudly.
🛠️ Proposed fix
if (modelExistsInSchema(schema, pascalModel)) {
if (!opts.force) {
spinner.warn(`Model ${pascalModel} already exists in schema — skipping`);
return;
}
- spinner.info(`Model ${pascalModel} exists but --force used, appending duplicate`);
+ spinner.info(`Model ${pascalModel} exists — overwriting due to --force`);
+ const modelBlockRegex = new RegExp(
+ `^model\\s+${escapeRegExp(pascalModel)}\\s*\\{[\\s\\S]*?^\\}`,
+ 'm'
+ );
+ schema = schema.replace(modelBlockRegex, '').trimEnd();
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/plugin-sdk/cli/commands/add.ts` around lines 119 - 131, The current
logic appends a duplicate model when opts.force is true; change it to
remove/replace the existing model block instead of appending: detect an existing
model via modelExistsInSchema(pascalModel) and when opts.force is true, strip
the existing "model <pascalModel> { ... }" block from schema (e.g. with a safe
regex or dedicated helper like removeModelBlock(schema, pascalModel)) before
calling ensureSchemaInDatasource and buildModelBlock, then insert the new block
in place (or replace the removed span) so the schema contains only the updated
model; keep spinner.info for the forced overwrite path and ensure spacing/trim
is preserved.
| function buildRegistrationLines(name: string): { importLine: string; useLine: string } { | ||
| const routerName = `${name}Router`; | ||
| return { | ||
| importLine: `import { ${routerName} } from './${name}.js';`, | ||
| useLine: `router.use('/${name}', ${routerName});`, | ||
| }; | ||
| } | ||
|
|
||
| async function handleAddEndpoint(name: string, opts: { crud?: boolean; force?: boolean }): Promise<void> { | ||
| const spinner = ora('Adding endpoint...').start(); | ||
| try { | ||
| const ctx = await resolvePluginContext(process.cwd()); | ||
|
|
||
| const routesDir = path.join(ctx.backendDir, 'src', 'routes'); | ||
| if (!(await fs.pathExists(routesDir))) { | ||
| spinner.fail('No backend/src/routes directory found'); | ||
| console.log(chalk.yellow( | ||
| 'Your plugin does not appear to have a backend.\n' + | ||
| 'Create one first with: naap-plugin create <name> --template full-stack' | ||
| )); | ||
| return; | ||
| } | ||
|
|
||
| const routeFile = path.join(routesDir, `${name}.ts`); | ||
| if (await fs.pathExists(routeFile)) { | ||
| if (!opts.force) { | ||
| spinner.warn(`Route file ${name}.ts already exists — skipping`); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| await fs.writeFile(routeFile, generateEndpointFile(name, !!opts.crud)); | ||
|
|
||
| const aggregatorPath = path.join(routesDir, 'index.ts'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, explore the file structure
find . -type f -name "add.ts" | grep -E "(cli|command)" | head -10Repository: livepeer/naap
Length of output: 98
🏁 Script executed:
# Get the full content of the target file
wc -l packages/plugin-sdk/cli/commands/add.tsRepository: livepeer/naap
Length of output: 100
🏁 Script executed:
# Read the file to see the full implementation
cat -n packages/plugin-sdk/cli/commands/add.tsRepository: livepeer/naap
Length of output: 12299
Add validation for endpoint names to prevent path traversal and invalid identifiers.
name is passed unsanitized to file paths (line 223) and used directly as a TypeScript identifier (line 154). A malicious or accidental value like ../../../etc could escape the routes directory, and hyphenated names like my-endpoint produce invalid identifiers (my-endpointRouter).
Add validation to reject invalid characters, then separate routeName (kebab-case for files/URLs) from routerName (PascalCase for the identifier). The toPascalCase helper already exists in this file and can be reused.
Suggested changes
-function generateEndpointFile(name: string, crud: boolean): string {
- const routerName = `${name}Router`;
+function generateEndpointFile(routeName: string, routerName: string, crud: boolean): string {
const lines: string[] = [];
lines.push(`import { Router } from 'express';`);
lines.push('');
lines.push(`export const ${routerName} = Router();`);
lines.push('');
if (crud) {
lines.push(`${routerName}.get('/', async (_req, res) => {`);
- lines.push(` // TODO: list ${name}`);
+ lines.push(` // TODO: list ${routeName}`);-function buildRegistrationLines(name: string): { importLine: string; useLine: string } {
- const routerName = `${name}Router`;
+function buildRegistrationLines(routeName: string, routerName: string): { importLine: string; useLine: string } {
return {
- importLine: `import { ${routerName} } from './${name}.js';`,
- useLine: `router.use('/${name}', ${routerName});`,
+ importLine: `import { ${routerName} } from './${routeName}.js';`,
+ useLine: `router.use('/${routeName}', ${routerName});`,
};
}
async function handleAddEndpoint(name: string, opts: { crud?: boolean; force?: boolean }): Promise<void> {
const spinner = ora('Adding endpoint...').start();
try {
+ const routeName = name.trim().toLowerCase();
+ if (!/^[a-z0-9-]+$/.test(routeName)) {
+ spinner.fail('Endpoint name must contain only alphanumeric characters and dashes');
+ return;
+ }
+ const routerName = `${toPascalCase(routeName)}Router`;- const routeFile = path.join(routesDir, `${name}.ts`);
+ const routeFile = path.join(routesDir, `${routeName}.ts`);- await fs.writeFile(routeFile, generateEndpointFile(name, !!opts.crud));
+ await fs.writeFile(routeFile, generateEndpointFile(routeName, routerName, !!opts.crud));- const routerName = `${name}Router`;
- const { importLine, useLine } = buildRegistrationLines(name);
+ const { importLine, useLine } = buildRegistrationLines(routeName, routerName);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/plugin-sdk/cli/commands/add.ts` around lines 200 - 233, Validate and
sanitize the incoming name in handleAddEndpoint before using it in file paths or
identifiers: reject values containing path separators or characters outside a
safe pattern (e.g. only lowercase letters, digits and hyphens) and throw/exit
with a clear error if validation fails to prevent path traversal; derive a
routeName (kebab-case, safe for filenames and URLs) to use for routeFile and
router.use paths and derive routerName by passing routeName through the existing
toPascalCase helper and appending "Router" for use as the TypeScript identifier;
update buildRegistrationLines to accept routeName and routerName (or compute
routerName there via toPascalCase) so importLine uses `./${routeName}.js` and
useLine uses `router.use('/${routeName}', ${routerName});`; ensure
generateEndpointFile and any other usages get the correct routerName
(PascalCase) vs routeName (kebab-case).
| const pluginName = name || answers.name || 'my-plugin'; | ||
| const template = (options?.template || answers.template || 'full-stack') as PluginTemplate; | ||
| const template = (options?.template || answers.template || 'frontend-only') as PluginTemplate; | ||
| const targetDir = options?.directory || path.join(process.cwd(), pluginName); |
There was a problem hiding this comment.
--simple can be silently ignored when no template is provided.
If a user runs naap-plugin create my-plugin --simple, the template defaults to frontend-only, so no backend is created despite requesting “simple full-stack.” Consider auto-promoting to full-stack or erroring when --simple is incompatible.
✅ Suggested fix (auto-promote + guard)
- const template = (options?.template || answers.template || 'frontend-only') as PluginTemplate;
+ const template = (options?.template || answers.template || (options?.simple ? 'full-stack' : 'frontend-only')) as PluginTemplate;
+ if (options?.simple && template === 'frontend-only') {
+ throw new Error('--simple cannot be used with the frontend-only template. Use --template full-stack.');
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/plugin-sdk/cli/commands/create.ts` around lines 128 - 130, The
template resolution can silently ignore --simple; update the logic after
computing pluginName/template/targetDir so that if options.simple is true and
neither options.template nor answers.template was explicitly provided (i.e., the
template came from the default), promote the template to 'full-stack' (set
template = 'full-stack') so a backend is created; additionally, add a guard to
throw or print an error if the user explicitly provided a conflicting template
(options.template or answers.template === 'frontend-only') while also passing
--simple, referencing the template variable and the options.simple flag to
locate where to implement this.
Summary
Addresses the challenge: "Full-stack is intimidating. Docker, Prisma multi-schema, Express route registration, and port management are a lot of concepts before a contributor can add a backend endpoint."
naap-plugin createnow scaffolds frontend-only plugins as the default template, lowering the entry barrier--simpleflag creates an Express backend with in-memory CRUD routes — no Docker, Prisma, or database setup requirednaap-plugin addcommands: Newadd modelandadd endpointsubcommands for incremental scaffolding with idempotency guardsroutes/README.mdwith step-by-step instructions; dev command surfacesaddcommands in tipssimplified-plugin-development.mdandhow-to-add-backend-endpoint.mddocsTest Evidence
Commit Groups (reviewer-friendly)
feat(cli): frontend-first create defaults and simple full-stack backend mode— P0 changesfeat(cli): add model/endpoint commands with idempotent updates and tests— P1 changesdocs(devx): scaffold route discoverability and dev command guidance— P2 changesdocs(devx): CLI/tutorial updates for simplified backend onboarding— Documentationtest(bdd): add scenarios for frontend-first defaults, simple backend, and add command— BDD hardeningReview Checklist
add modelandadd endpointRisk Mitigations
--template full-stackstill works; migration note in docsMade with Cursor
Summary by CodeRabbit
New Features
Documentation
Tests