feat: Add Baseplate Development Server with MCP Integration#646
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: cd86c61 The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 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 |
|
Caution Review failedThe pull request is closed. WalkthroughAdds a typed service-action framework with worker execution, a Fastify dev server (TRPC + MCP), new CLI commands (dev-server, mcp), many service actions (diff, sync, snapshots, template management), template utilities, plugin-discovery API change, structured evented logger, and assorted tooling/config updates (Zod bumps, exports, tests). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as CLI User
participant CLI as project-builder-cli
participant Ctx as ServiceActionContext
participant SA as ServiceAction
participant W as Worker
U->>CLI: bp diff --package foo --include "src/**"
CLI->>CLI: resolveProject() & createServiceActionContext()
CLI->>SA: invokeServiceActionAsCli(diffProjectAction, input, Ctx)
SA->>W: runActionInWorker(actionName, input, context)
W-->>SA: success/result
SA-->>CLI: action.writeCliOutput(result)
CLI-->>U: printed summary
sequenceDiagram
autonumber
participant Client as Browser/Tool
participant Dev as Fastify Dev Server
participant Auth as Auth Plugin
participant TRPC as TRPC Router
participant Act as ServiceAction
participant W as Worker
Client->>Dev: POST /dev/trpc (Bearer token)
Dev->>Auth: onRequest validation (host/origin/token)
Auth-->>Dev: allowed
Dev->>TRPC: route handler
TRPC->>Act: invoke corresponding action
Act->>W: runActionInWorker(...)
W-->>Act: result / error
Act-->>TRPC: return result
TRPC-->>Client: HTTP JSON response
sequenceDiagram
autonumber
participant Editor as MCP Client
participant Stdio as Stdio Transport
participant MCP as MCP Server
participant MCPsrv as MCP Server (actions)
participant W as Worker
Editor->>Stdio: initialize + requests
Stdio->>MCP: transport connect
MCP->>MCPsrv: invoke tool (e.g., sync-project)
MCPsrv->>W: runActionInWorker(action, input, context)
W-->>MCPsrv: action result
MCPsrv-->>MCP: tool result
MCP-->>Stdio: send response
Stdio-->>Editor: deliver result
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
kingston
deleted the
kingston/eng-858-implement-baseplate-development-server-with-mcp-integration
branch
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CLAUDE.md (1)
326-873: Duplicate MCP section — deduplicate to avoid conflicting guidance.There appear to be two “Baseplate Development Server MCP” sections. Keep a single canonical section.
♻️ Duplicate comments (2)
packages/project-builder-cli/src/utils/list-projects.ts (1)
126-135: Cross‑platform path detection: check isAbsolute on the tilde‑expanded path.Expand first, then run
path.isAbsolute(...)so Windows/posix home expansion is handled uniformly. Keeps the existing~/.checks.- if ( - projectNameOrDirectory.startsWith('.') || - path.isAbsolute(projectNameOrDirectory) || - projectNameOrDirectory.startsWith('~') - ) { - const resolvedPath = expandPathWithTilde(projectNameOrDirectory); - const projectInfo = await loadProjectFromDirectory(resolvedPath); + const expanded = expandPathWithTilde(projectNameOrDirectory); + if ( + projectNameOrDirectory.startsWith('.') || + projectNameOrDirectory.startsWith('~') || + path.isAbsolute(expanded) + ) { + const projectInfo = await loadProjectFromDirectory(expanded); return projectInfo; }packages/project-builder-server/src/actions/utils/run-in-worker.ts (1)
57-63: Validate log level before indexing logger.Guard against unexpected levels to avoid runtime errors.
- if (typedMessage.type === 'log') { - logger[typedMessage.level]( - typedMessage.metadata ?? {}, - typedMessage.message, - ); + if (typedMessage.type === 'log') { + const validLevels = ['info', 'warn', 'error', 'debug'] as const; + const level = validLevels.includes(typedMessage.level as any) + ? typedMessage.level + : 'info'; + logger[level](typedMessage.metadata ?? {}, typedMessage.message); return; }
🧹 Nitpick comments (47)
packages/project-builder-cli/src/utils/list-projects.ts (6)
28-40: Normalize/resolve directories before de‑duping to avoid duplicates and ensure consistency.Normalize env, additional, and example directories via
path.resolve(...)after tilde expansion so the Set truly de‑dupes and downstream discovery receives absolute paths.- const envDirectories = config.PROJECT_DIRECTORIES?.split(',') ?? []; + const envDirectories = config.PROJECT_DIRECTORIES?.split(',') ?? []; for (const dir of envDirectories) { - if (dir.trim()) { - allDirectories.add(expandPathWithTilde(dir.trim())); + const trimmed = dir.trim(); + if (trimmed) { + const expanded = expandPathWithTilde(trimmed); + allDirectories.add(path.resolve(expanded)); } } // Add explicitly provided directories for (const dir of additionalDirectories ?? []) { - allDirectories.add(expandPathWithTilde(dir)); + const expanded = expandPathWithTilde(dir); + allDirectories.add(path.resolve(expanded)); } // Add example directories if requested if (includeExamples) { const exampleDirs = await findExamplesDirectories(); for (const dir of exampleDirs) { - allDirectories.add(dir); + allDirectories.add(path.resolve(dir)); } }Also applies to: 41-47
54-55: Return directories in a stable order.Sorting improves determinism for logs/tests and makes behavior reproducible.
- return [...allDirectories]; + return [...allDirectories].sort();
15-15: Avoid reading env at import time; fetch per call for testability and late‑bound config.This prevents stale config when
process.envchanges during tests/CLI sessions.-const config = getEnvConfig(); +// Fetch env on demand within functions for freshness.- const envDirectories = config.PROJECT_DIRECTORIES?.split(',') ?? []; + const { PROJECT_DIRECTORIES } = getEnvConfig(); + const envDirectories = PROJECT_DIRECTORIES?.split(',') ?? [];- const searchDirectories = await getSearchDirectories({ - additionalDirectories, - includeExamples: config.INCLUDE_EXAMPLES ?? false, - defaultToCwd: true, - }); + const searchDirectories = await getSearchDirectories({ + additionalDirectories, + includeExamples: getEnvConfig().INCLUDE_EXAMPLES ?? false, + defaultToCwd: true, + });Also applies to: 28-30, 86-90
141-143: Improve the not‑found error with available options.Helps users discover valid names/IDs without re‑running another command.
- if (!project) { - throw new Error(`Project ${projectNameOrDirectory} not found`); - } + if (!project) { + const available = projects.map((p) => `${p.name} (${p.id})`).join(', '); + throw new Error( + `Project "${projectNameOrDirectory}" not found. Available: ${available}`, + ); + }
9-9: Import alias consistency with guidelines.Guidelines specify the
@src/alias; this file uses#src/. Verify the configured alias and align for consistency.-import { logger } from '#src/services/logger.js'; +import { logger } from '@src/services/logger.js';If
@srcisn’t configured in this package, keep#srcand update the guideline or add the alias to tsconfig/path exports for this package.
97-101: Doc nit: mention Windows/UNC and tilde‑expanded absolute detection.The comment still references leading
/. Clarify it now relies onpath.isAbsolute(expanded), which covers Windows and UNC paths.packages/project-builder-test/src/commands/serve.ts (2)
133-136: Fix incomplete CLI argument descriptionThe help text currently reads “Serve ”.
Apply:
- .argument('<project-name>', 'Serve ') + .argument('<project-name>', 'Project name to serve (will be generated if missing)')
89-96: Unify plugin discovery logging
- Move the
pinoLoggerinstantiation above the built-in plugin load and callawait getDefaultPlugins(pinoLogger)(instead oflogger) to consolidate all logs through Pino.- No change needed for
userConfig: passing{}is valid sincesyncis optional inBaseplateUserConfig.packages/utils/package.json (1)
52-52: Add to catalog or pin explicitly for consistency.Using a caret range here while other deps use exact versions or the repo’s “catalog:” indirection can lead to drift. Prefer moving fastest-levenshtein to the version catalog or pinning it exactly to keep builds reproducible.
- "fastest-levenshtein": "^1.0.16", + "fastest-levenshtein": "catalog:" // or pin "1.0.16"packages/sync/src/templates/extractor/utils/merge-extractor-template-entries.ts (1)
33-35: Error message consistency improved.Message now reflects the centralized filename; consider including the expected search location in a follow-up if users still struggle to locate configs.
packages/project-builder-server/src/templates/utils/infer-exports-from-ts-file.ts (2)
17-19: Optional: Reuse a ts-morph Project for performance.Creating a new
Project()per call is expensive. Consider accepting aProjectparameter (with a default) or caching one instance.
67-85: Optional: Include enums (and mark const enums as type-only).If you want parity with TS exportables, handle
enum/const enumas well.for (const enm of sourceFile.getEnums()) { if (enm.isExported()) { availableExports.set(enm.getName(), { name: enm.getName(), isTypeOnly: enm.isConstEnum(), // const enums are erased at runtime }); } }Also consider
sourceFile.getExportSymbols()for a single-pass, re-export-aware approach if you later need to resolveexport * from ....packages/project-builder-server/src/templates/utils/extractor-config.ts (1)
52-58: TightentemplateTypeto the allowed union.Type it as
TemplateConfig['type']to prevent invalid values at call sites.-export async function updateExtractorTemplate( - generatorDirectory: string, - templateName: string, - templateType = 'ts', +export async function updateExtractorTemplate( + generatorDirectory: string, + templateName: string, + templateType: TemplateConfig['type'] = 'ts',packages/sync/src/templates/extractor/utils/write-extractor-template-jsons.ts (1)
6-7: Good constant adoption; consider future-proofing the docs.Replacing hard-coded 'extractor.json' with
EXTRACTOR_CONFIG_FILENAMEin both the error and write path looks correct.
- Optional: Update the JSDoc to avoid hard-coding “extractor.json” so it won’t drift if the constant changes (e.g., “Writes extractor config files …”).
Apply this diff if you want to de-couple the doc from the filename:
- * Writes extractor.json files for the specified generators to the file system. - * This function reads the current configurations from the context and writes them - * to their respective extractor.json files. + * Writes extractor config files for the specified generators to the file system. + * This function reads the current configurations from the context and writes them + * to their respective config files.Also applies to: 24-26, 31-33
packages/utils/src/string/find-closest-match.unit.test.ts (1)
1-70: Comprehensive coverage; one minor robustness tweak.Great set of cases and proper Vitest imports. The “n greater than candidates length” test asserts exact ordering, which could become brittle if the distance function changes. Prefer checking membership and length instead of order for that case.
Apply:
- expect(result).toEqual(['apple', 'banana']); + expect(result).toHaveLength(2); + expect(new Set(result)).toEqual(new Set(['apple', 'banana']));packages/utils/src/string/find-closest-match.ts (1)
31-47: Micro-opt: fast path for n === 1 to avoid full sort.For large candidate sets, a single scan is cheaper than O(N log N) sort.
// Calculate distances for all candidates - const distances = candidates.map((candidate, index) => ({ + const distances = candidates.map((candidate, index) => ({ candidate, distance: distance(target, candidate), index, })); + // Fast path when only the single closest match is needed + if (n === 1) { + let best: { candidate: string; distance: number; index: number } | null = null; + for (let i = 0; i < candidates.length; i++) { + const d = distance(target, candidates[i]); + if (!best || d < best.distance) { + best = { candidate: candidates[i], distance: d, index: i }; + } + } + return best ? [best.candidate] : []; + } + // Sort by distance (ascending) and then by original index for stable sorting distances.sort((a, b) => { if (a.distance !== b.distance) { return a.distance - b.distance; } return a.index - b.index; });packages/project-builder-server/src/actions/snapshot/index.ts (1)
1-4: Prefer barrel re-exports for index filesAlign with our index.ts guideline by switching to star re-exports for consistency and easier maintenance.
-export { snapshotAddAction } from './snapshot-add.action.js'; -export { snapshotRemoveAction } from './snapshot-remove.action.js'; -export { snapshotSaveAction } from './snapshot-save.action.js'; -export { snapshotShowAction } from './snapshot-show.action.js'; +export * from './snapshot-add.action.js'; +export * from './snapshot-remove.action.js'; +export * from './snapshot-save.action.js'; +export * from './snapshot-show.action.js';packages/project-builder-server/src/templates/utils/resolve-generator.ts (2)
6-6: Use tsconfig path alias instead of relative importOur guideline prefers absolute alias imports. Switch to the
#src/alias for local modules.-import { discoverGenerators } from '../../template-extractor/discover-generators.js'; +import { discoverGenerators } from '#src/template-extractor/discover-generators.js';
26-37: Improve not-found logic: case-insensitive match and clearer suggestionsHandle case-insensitive names and produce a helpful message when no close matches exist.
- const generator = generators.find((gen) => gen.name === generatorName); + const target = generatorName.toLowerCase(); + const generator = generators.find((gen) => gen.name.toLowerCase() === target); if (!generator) { - const availableGenerators = generators.map((g) => g.name); - const closestGenerators = findClosestMatch( - generatorName, - availableGenerators, - 3, - ); - throw new Error( - `Generator '${generatorName}' not found. Did you mean one of these: ${closestGenerators.join(', ')}?`, - ); + const availableGenerators = generators.map((g) => g.name); + const closestGenerators = findClosestMatch(generatorName, availableGenerators, 3); + const suggestion = + closestGenerators.length > 0 + ? ` Did you mean: ${closestGenerators.join(', ')}?` + : ' No similarly named generators were found.'; + const available = + availableGenerators.length > 0 + ? ` Available generators: ${availableGenerators.join(', ')}.` + : ' No generators were discovered.'; + throw new Error(`Generator '${generatorName}' not found.${suggestion}${available}`); }packages/project-builder-server/src/actions/templates/configure-raw-template.action.ts (2)
5-15: Validate kebab-case for template names at the schemaEnforce the documented kebab-case requirement to catch mistakes early.
- templateName: z.string().describe('Template name in kebab-case format'), + templateName: z + .string() + .regex(/^[a-z0-9]+(?:-[a-z0-9]+)*$/, 'Use kebab-case, e.g. my-template-name') + .describe('Template name in kebab-case format'),
31-37: Optional: export schemas for reuseIf other packages (CLI, dev server validators) need these, consider exporting the schemas to avoid duplication.
packages/project-builder-server/src/templates/list/list-templates.ts (1)
43-49: Avoid unsafe property reads; guard optional fields with runtime checks.Use the in-operator and type narrowing instead of casting through Record<string, unknown>.
- group: (templateConfig as Record<string, unknown>).group as - | string - | undefined, - kind: (templateConfig as Record<string, unknown>).kind as - | string - | undefined, + group: + 'group' in templateConfig && typeof (templateConfig as any).group === 'string' + ? (templateConfig as any).group + : undefined, + kind: + 'kind' in templateConfig && typeof (templateConfig as any).kind === 'string' + ? (templateConfig as any).kind + : undefined,packages/project-builder-server/src/actions/templates/list-templates.action.ts (2)
5-10: Validate whethercreateServiceActionexpects Zod shapes or Zod objects.If Zod objects are required elsewhere, wrap these shapes with
z.object(...)for uniform validation.-const listTemplatesInputSchema = { +const listTemplatesInputSchema = z.object({ generatorDirectory: z .string() .describe('The directory path containing the generator'), -}; +}); -const listTemplatesOutputSchema = { +const listTemplatesOutputSchema = z.object({ message: z.string().describe('Success message'), generatorName: z.string().describe('The name of the generator'), generatorDirectory: z.string().describe('The generator directory'), templates: z.array(templateInfoSchema).describe('List of templates'), templateCount: z.number().describe('Total number of templates'), -}; +});Also applies to: 20-26
17-18: Avoidz.any(); preferz.unknown()for safer typing.Keeps unknown config opaque while still serializable.
- config: z.record(z.any()).describe('The full template configuration'), + config: z.record(z.unknown()).describe('The full template configuration'),packages/project-builder-server/src/actions/snapshot/snapshot-add.action.ts (2)
79-83: Return actual added count (if available) vs attempted.
filesAdded: files.lengthcounts inputs, not the net additions (unchanged files may be skipped). Consider plumbing a count fromaddFilesToSnapshot.
85-93: Include error stack in logs for diagnosis.Improve observability while keeping user-facing message clean.
- logger.error( - `Failed to add files to snapshot for project ${projectId}: ${String(error)}`, - ); + logger.error( + `Failed to add files to snapshot for project ${projectId}: ${ + error instanceof Error ? error.stack ?? error.message : String(error) + }`, + );packages/project-builder-server/src/templates/utils/resolve-file-path.ts (2)
70-72: Permit directory identifier in addition to name/id for parity withgetProjectByNameOrId.- const matchingProject = projects.find( - (proj) => proj.name === projectNameOrId || proj.id === projectNameOrId, - ); + const matchingProject = projects.find( + (proj) => + proj.name === projectNameOrId || + proj.id === projectNameOrId || + proj.directory === projectNameOrId, + );
75-78: Improve error message with more context (name/id/dir).- const availableProjects = projects.map((p) => p.name).join(', '); + const availableProjects = projects + .map((p) => `${p.name} (${p.id}) @ ${p.directory}`) + .join(', ');packages/project-builder-server/src/actions/snapshot/snapshot-show.action.ts (1)
146-181: CLI output LGTM; consider optionally showing diff filenames.Small UX improvement: include
diffFilealongside modified paths when present.- for (const entry of output.files.modified) { - console.info(` ${entry.path}`); - } + for (const entry of output.files.modified) { + const diffInfo = entry.diffFile ? ` (diff: ${entry.diffFile})` : ''; + console.info(` ${entry.path}${diffInfo}`); + }packages/project-builder-server/src/templates/utils/template-metadata.unit.test.ts (3)
9-11: Rely on project’s Vitest automocks for fs/memfs.You don’t need explicit vi.mock calls here; the suite is already configured to automock node:fs and node:fs/promises with memfs. This reduces noise across tests.
-vi.mock('node:fs'); -vi.mock('node:fs/promises');
37-49: Reduce repetition when reading metadata from memfs.Consider a small helper to read/parse .templates-info.json to DRY the Arrange/Assert sections and improve failure messages.
Example helper (add atop the file):
function readTemplatesInfo(dir = '/project/src/components'): Record<string, unknown> { const files = vol.toJSON(); const fileContent = files[`${dir}/.templates-info.json`]; return JSON.parse(String(fileContent || '{}')); }Also applies to: 80-96, 127-138, 155-166, 195-206, 273-279
141-166: Add coverage for preserving existing instanceData when updating without it.updateTemplateMetadata should keep prior instanceData for a file when called without instanceData and metadata already exists; add a test to lock this behavior.
Also applies to: 248-281
packages/project-builder-server/src/actions/templates/delete-template.action.ts (1)
39-41: Prefer tsconfig path alias for consistency.Most actions use the '#src/' alias; consider switching this import to '#src/templates/delete/delete-template.js' for consistency.
- const { deleteTemplate } = await import( - '../../templates/delete/delete-template.js' - ); + const { deleteTemplate } = await import( + '#src/templates/delete/delete-template.js' + );packages/project-builder-server/src/actions/registry.ts (1)
23-39: Make registry immutable.Mark the array as readonly to prevent accidental mutation at runtime.
-export const ALL_SERVICE_ACTIONS = [ +export const ALL_SERVICE_ACTIONS = [ diffProjectAction, syncProjectAction, discoverGeneratorsAction, extractTemplatesAction, generateTemplatesAction, configureTsTemplateAction, configureTextTemplateAction, configureRawTemplateAction, deleteTemplateAction, listTemplatesAction, showTemplateMetadataAction, snapshotAddAction, snapshotRemoveAction, snapshotSaveAction, snapshotShowAction, -]; +] as const;packages/project-builder-server/src/actions/utils/run-in-worker.ts (1)
47-53: Extract timeout to a constant for easier tuning.Small readability/maintainability win.
- const timeout = setTimeout(() => { + const ACTION_TIMEOUT_MS = 60_000; + const timeout = setTimeout(() => { worker.terminate().catch((err: unknown) => { context.logger.error(err); }); reject(new Error(`Action execution timed out for ${serviceAction.name}`)); - }, 60_000); // 60 second timeout + }, ACTION_TIMEOUT_MS);packages/project-builder-server/src/actions/templates/show-template-metadata.action.ts (1)
43-45: Use#srcpath alias in dynamic import for consistency and to avoid brittle relatives.- const { showTemplateMetadata } = await import( - '../../templates/show/show-template-metadata.js' - ); + const { showTemplateMetadata } = await import( + '#src/templates/show/show-template-metadata.js' + );packages/project-builder-server/src/dev-server/api/router.ts (1)
18-19: Prefer alias import for trpc to match the rest of the file.-import { devRouter, devTrpcActionBuilder } from './trpc.js'; +import { devRouter, devTrpcActionBuilder } from '#src/dev-server/api/trpc.js';packages/project-builder-server/src/actions/templates/configure-ts-template.action.ts (1)
21-25: DefaultprojectExportsat the schema level; drop handler default.
This simplifies handler typing and keeps defaults centralized.projectExports: z - .array(z.string()) - .optional() + .array(z.string()) + .default([]) .describe('Array of identifiers to expose as exports for other generators'),- const { + const { filePath, project, generator, templateName, - projectExports = [], + projectExports, group, } = input;Also applies to: 49-56
packages/project-builder-server/src/actions/snapshot/snapshot-remove.action.ts (2)
12-15: Require a non-empty files list to avoid no-op calls.files: z - .array(z.string()) - .describe('Array of file paths to remove from snapshot.'), + .array(z.string()) + .nonempty('Provide at least one file path') + .describe('Array of file paths to remove from snapshot.'),
7-7: Use#srcalias for internal imports to align with the rest of the file.-import { getProjectByNameOrId } from '../utils/projects.js'; +import { getProjectByNameOrId } from '#src/actions/utils/projects.js';packages/project-builder-server/src/actions/templates/configure-text-template.action.ts (2)
10-11: Trim variable values to avoid accepting whitespace-only stringsAdd .trim() before .min(1) so " " is rejected.
- value: z.string().min(1).describe('The value of the variable'), + value: z.string().trim().min(1).describe('The value of the variable'),
22-23: Enforce kebab-case for templateName at the schema levelThe description mentions kebab-case but it isn’t validated.
- templateName: z.string().describe('Template name in kebab-case format'), + templateName: z + .string() + .regex(/^[a-z0-9]+(?:-[a-z0-9]+)*$/, 'Use kebab-case, e.g. my-template-name') + .describe('Template name in kebab-case format'),packages/project-builder-server/src/templates/utils/template-metadata.ts (2)
92-95: Make delete resilient if file disappears between read and unlinkAvoid throwing on ENOENT by handling not-found on unlink.
- await (Object.keys(updatedMetadata).length === 0 - ? fs.unlink(metadataPath) - : writeStablePrettyJson(metadataPath, updatedMetadata)); + await (Object.keys(updatedMetadata).length === 0 + ? fs.unlink(metadataPath).catch(handleFileNotFoundError) + : writeStablePrettyJson(metadataPath, updatedMetadata));Alternatively, use fs.rm(metadataPath, { force: true }).
25-28: Minor DRY: factor repeated path computationsOptional helper to compute { metadataPath, fileName } to simplify maintenance.
Example:
function getPaths(filePath: string) { const dirPath = path.dirname(filePath); return { metadataPath: path.join(dirPath, TEMPLATES_INFO_FILENAME), fileName: path.basename(filePath), }; }Also applies to: 53-56, 75-78
packages/project-builder-server/src/templates/delete/delete-template.ts (1)
85-86: Optional: simplify not-found handling withfs.rm({ force: true })This avoids the
.catch(handleFileNotFoundError)pattern.- // Delete the generated file itself - await fs.unlink(absolutePath).catch(handleFileNotFoundError); + // Delete the generated file itself + await fs.rm(absolutePath, { force: true });Note: if you adopt
fs.rmhere (and above), drop the now-unusedhandleFileNotFoundErrorimport.packages/project-builder-server/src/templates/configure/configure-text-template.ts (1)
60-69: Trim and validate variable valuesReject whitespace-only values and use the trimmed value for presence checks to avoid false negatives.
- for (const [variable, { value }] of Object.entries(variables ?? {})) { - if (value === '') { - throw new Error(`Variable ${variable} must have a value`); - } - if (!templateContents.includes(value)) { - throw new Error( - `Variable ${variable} with value ${value} not found in template ${absolutePath}`, - ); - } - } + for (const [variable, { value }] of Object.entries(variables ?? {})) { + const v = value.trim(); + if (v.length === 0) { + throw new Error(`Variable '${variable}' must have a non-empty value`); + } + if (!templateContents.includes(v)) { + throw new Error( + `Variable '${variable}' with value '${v}' not found in template '${absolutePath}'`, + ); + } + }packages/project-builder-server/src/templates/configure/configure-ts-template.ts (1)
39-45: Optional: assert TS file extension earlyQuickly fail if a non-TS file is passed to the TS template configurator.
const { absolutePath, project, projectRelativePath } = resolveFilePath( filePath, projects, projectNameOrId, ); + if (!absolutePath.endsWith('.ts') && !absolutePath.endsWith('.tsx')) { + throw new Error(`File must be .ts or .tsx: '${absolutePath}'`); + }
| { | ||
| "mcpServers": { | ||
| "baseplate-dev-server": { | ||
| "command": "mise", | ||
| "args": ["exec", "--", "pnpm", "--silent", "start", "mcp"] | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the start script invocation; prefer a dedicated mcp script for clarity.
pnpm start mcp forwards “mcp” as an arg to the start script, which may be ignored depending on the script. Consider invoking a dedicated mcp script to avoid ambiguity.
- "args": ["exec", "--", "pnpm", "--silent", "start", "mcp"]
+ "args": ["exec", "--", "pnpm", "--silent", "run", "mcp"]Run to verify scripts exist and intended entrypoints are present:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Root start script:" && jq -r '.scripts.start // "MISSING"' package.json
echo "Root mcp script:" && jq -r '.scripts.mcp // "MISSING"' package.json
echo "CLI MCP command file:" && fd -a 'mcp.ts' -g 'packages/**/src/commands/mcp.ts' -x echo {}Length of output: 513
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Root package.json mcp script:"
jq -r '.scripts.mcp // "MISSING"' package.json
echo "All package.json mcp scripts in workspace:"
find . -name package.json | while read -r pkg; do
echo "– $pkg:"
jq -r '.scripts.mcp // "MISSING"' "$pkg"
done
echo "Finding all mcp.ts files in the repo:"
find . -type f -name 'mcp.ts'Length of output: 371055
Add a dedicated mcp npm script or keep the explicit filtered start — don’t switch to run mcp unless a script exists
Root package.json start = "pnpm run --filter @baseplate-dev/project-builder-cli start"; no mcp script was found in any package.json.
Action: either add a package.json script named mcp and update .mcp.json to call pnpm run mcp, or keep the current invocation but make the intended target explicit (e.g. run the filtered package start with the mcp arg).
File: .mcp.json (lines 1-8)
🤖 Prompt for AI Agents
.mcp.json lines 1-8: the current command uses "pnpm --silent start mcp" but
there is no package.json script named "mcp", so update either the package(s) or
the .mcp.json to make the target explicit: Option A — add an "mcp" script to the
appropriate package.json (or root package.json) that runs the intended start
command, then change .mcp.json to invoke "pnpm run mcp"; Option B — keep the
explicit filtered invocation and change .mcp.json to call the filtered start
directly (e.g. "pnpm run --filter @baseplate-dev/project-builder-cli start" with
the mcp arg preserved) so it targets the correct package without relying on a
missing "mcp" script. Ensure the command and args in .mcp.json match whichever
approach you choose.
| import { addFilesToSnapshot } from '#src/diff/snapshot/snapshot-management.js'; | ||
| import { createNodeSchemaParserContext } from '#src/plugins/node-plugin-store.js'; | ||
|
|
||
| import { getProjectByNameOrId } from '../utils/projects.js'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use alias import for internal utility.
Standardize on alias paths per guidelines.
-import { getProjectByNameOrId } from '../utils/projects.js';
+import { getProjectByNameOrId } from '#src/actions/utils/projects.js';📝 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.
| import { getProjectByNameOrId } from '../utils/projects.js'; | |
| import { getProjectByNameOrId } from '#src/actions/utils/projects.js'; |
🤖 Prompt for AI Agents
In packages/project-builder-server/src/actions/snapshot/snapshot-add.action.ts
around line 7, replace the relative import "import { getProjectByNameOrId } from
'../utils/projects.js';" with the project's configured alias import (e.g. import
{ getProjectByNameOrId } from '@project-builder-server/utils/projects';) to
conform to alias path guidelines; update the file extension to match the source
(omit .js if aliases point to TS sources) and ensure the alias is defined in
tsconfig/paths or build config if not already.
| return { | ||
| success: true, | ||
| message: `Snapshot saved successfully for ${project.name}/${app}`, | ||
| snapshotPath: `${project.directory}/${app}/${snapshotDirectory}`, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Returned snapshotPath is likely incorrect; avoid fabricating it.
createSnapshotForProject resolves appDirectory internally (not equal to /${app} in general). Returning ${project.directory}/${app}/${snapshotDirectory} can mislead users and tooling.
Minimal safe fix (omit the path until the source of truth returns it):
- return {
- success: true,
- message: `Snapshot saved successfully for ${project.name}/${app}`,
- snapshotPath: `${project.directory}/${app}/${snapshotDirectory}`,
- };
+ return {
+ success: true,
+ message: `Snapshot saved successfully for ${project.name}/${app}`,
+ };Follow-up (recommended): update createSnapshotForProject to return { snapshotPath, fileCount } and plumb it through to populate snapshotPath accurately. I can draft that change if you want.
📝 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.
| return { | |
| success: true, | |
| message: `Snapshot saved successfully for ${project.name}/${app}`, | |
| snapshotPath: `${project.directory}/${app}/${snapshotDirectory}`, | |
| }; | |
| return { | |
| success: true, | |
| message: `Snapshot saved successfully for ${project.name}/${app}`, | |
| }; |
🤖 Prompt for AI Agents
In packages/project-builder-server/src/actions/snapshot/snapshot-save.action.ts
around lines 83 to 87, the returned snapshotPath is being fabricated as
`${project.directory}/${app}/${snapshotDirectory}` but appDirectory is resolved
inside createSnapshotForProject and may differ; remove the snapshotPath property
from the returned object (or set it to undefined) so we don't return an
incorrect path, update the function's return type/interface accordingly, and
leave a TODO comment to follow up by changing createSnapshotForProject to return
{ snapshotPath, fileCount } and plumb that value through to this response in a
future change.
| import { createServiceAction } from '#src/actions/types.js'; | ||
| import { createNodeSchemaParserContext } from '#src/plugins/node-plugin-store.js'; | ||
|
|
||
| import { getProjectByNameOrId } from '../utils/projects.js'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use alias import for internal utility.
Align with project-wide import conventions.
-import { getProjectByNameOrId } from '../utils/projects.js';
+import { getProjectByNameOrId } from '#src/actions/utils/projects.js';Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/project-builder-server/src/actions/snapshot/snapshot-show.action.ts
around line 6, replace the relative import "import { getProjectByNameOrId } from
'../utils/projects.js';" with the project-wide alias import (e.g. import {
getProjectByNameOrId } from '@/utils/projects';) so the module follows the
repository's alias import convention; ensure the file extension and path match
your tsconfig/webpack alias settings.
| handler: async (input, context) => { | ||
| const { filePath, project } = input; | ||
| const { logger } = context; | ||
|
|
||
| const { deleteTemplate } = await import( | ||
| '../../templates/delete/delete-template.js' | ||
| ); | ||
|
|
||
| const result = await deleteTemplate( | ||
| { | ||
| filePath, | ||
| project, | ||
| }, | ||
| context, | ||
| ); | ||
|
|
||
| logger.info(result.message); | ||
|
|
||
| return { | ||
| success: true, | ||
| message: result.message, | ||
| templateName: result.templateName, | ||
| absolutePath: result.absolutePath, | ||
| generatorDirectory: result.generatorDirectory, | ||
| }; | ||
| }, | ||
| writeCliOutput: (output) => { | ||
| console.info(`✅ ${output.message}`); | ||
| }, |
There was a problem hiding this comment.
Return structured errors and align CLI output on failure.
Handler always returns success: true and doesn’t catch errors. Wrap in try/catch, return success: false on failures, and print via console.error accordingly.
handler: async (input, context) => {
const { filePath, project } = input;
const { logger } = context;
- const { deleteTemplate } = await import(
- '../../templates/delete/delete-template.js'
- );
-
- const result = await deleteTemplate(
- {
- filePath,
- project,
- },
- context,
- );
-
- logger.info(result.message);
-
- return {
- success: true,
- message: result.message,
- templateName: result.templateName,
- absolutePath: result.absolutePath,
- generatorDirectory: result.generatorDirectory,
- };
+ try {
+ const { deleteTemplate } = await import(
+ '../../templates/delete/delete-template.js'
+ );
+ const result = await deleteTemplate({ filePath, project }, context);
+ logger.info(result.message);
+ return {
+ success: true,
+ message: result.message,
+ templateName: result.templateName,
+ absolutePath: result.absolutePath,
+ generatorDirectory: result.generatorDirectory,
+ };
+ } catch (err) {
+ const message =
+ err instanceof Error ? err.message : `Unknown error: ${String(err)}`;
+ logger.error({ err }, `Failed to delete template for ${filePath}`);
+ return {
+ success: false,
+ message: `Failed to delete template: ${message}`,
+ templateName: '',
+ absolutePath: '',
+ generatorDirectory: '',
+ };
+ }
},
writeCliOutput: (output) => {
- console.info(`✅ ${output.message}`);
+ if (output.success) {
+ console.info(`✅ ${output.message}`);
+ } else {
+ console.error(`✗ ${output.message}`);
+ }
},📝 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.
| handler: async (input, context) => { | |
| const { filePath, project } = input; | |
| const { logger } = context; | |
| const { deleteTemplate } = await import( | |
| '../../templates/delete/delete-template.js' | |
| ); | |
| const result = await deleteTemplate( | |
| { | |
| filePath, | |
| project, | |
| }, | |
| context, | |
| ); | |
| logger.info(result.message); | |
| return { | |
| success: true, | |
| message: result.message, | |
| templateName: result.templateName, | |
| absolutePath: result.absolutePath, | |
| generatorDirectory: result.generatorDirectory, | |
| }; | |
| }, | |
| writeCliOutput: (output) => { | |
| console.info(`✅ ${output.message}`); | |
| }, | |
| handler: async (input, context) => { | |
| const { filePath, project } = input; | |
| const { logger } = context; | |
| try { | |
| const { deleteTemplate } = await import( | |
| '../../templates/delete/delete-template.js' | |
| ); | |
| const result = await deleteTemplate({ filePath, project }, context); | |
| logger.info(result.message); | |
| return { | |
| success: true, | |
| message: result.message, | |
| templateName: result.templateName, | |
| absolutePath: result.absolutePath, | |
| generatorDirectory: result.generatorDirectory, | |
| }; | |
| } catch (err) { | |
| const message = | |
| err instanceof Error ? err.message : `Unknown error: ${String(err)}`; | |
| logger.error({ err }, `Failed to delete template for ${filePath}`); | |
| return { | |
| success: false, | |
| message: `Failed to delete template: ${message}`, | |
| templateName: '', | |
| absolutePath: '', | |
| generatorDirectory: '', | |
| }; | |
| } | |
| }, | |
| writeCliOutput: (output) => { | |
| if (output.success) { | |
| console.info(`✅ ${output.message}`); | |
| } else { | |
| console.error(`✗ ${output.message}`); | |
| } | |
| }, |
🤖 Prompt for AI Agents
In
packages/project-builder-server/src/actions/templates/delete-template.action.ts
around lines 35 to 63, the handler currently always returns success: true and
doesn't handle thrown errors; wrap the main logic in a try/catch, call
deleteTemplate inside the try, and on success return success: true with the
existing fields; in the catch log the error via context.logger.error, return
success: false with a helpful message (and set templateName, absolutePath,
generatorDirectory to null or undefined), and update writeCliOutput to call
console.error when output.success is false (keeping console.info for success).
| // Check for default export | ||
| const defaultExportSymbol = sourceFile.getDefaultExportSymbol(); | ||
| if (defaultExportSymbol) { | ||
| const name = defaultExportSymbol.getName(); | ||
| availableExports.set(name, { | ||
| name, | ||
| isDefault: true, | ||
| isTypeOnly: false, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Normalize default export key to 'default'.
Using defaultExportSymbol.getName() can yield non-'default' values depending on declaration shapes. Downstream importers expect the key 'default'.
- const defaultExportSymbol = sourceFile.getDefaultExportSymbol();
- if (defaultExportSymbol) {
- const name = defaultExportSymbol.getName();
- availableExports.set(name, {
- name,
- isDefault: true,
- isTypeOnly: false,
- });
- }
+ if (sourceFile.getDefaultExportSymbol()) {
+ availableExports.set('default', {
+ name: 'default',
+ isDefault: true,
+ isTypeOnly: false,
+ });
+ }📝 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.
| // Check for default export | |
| const defaultExportSymbol = sourceFile.getDefaultExportSymbol(); | |
| if (defaultExportSymbol) { | |
| const name = defaultExportSymbol.getName(); | |
| availableExports.set(name, { | |
| name, | |
| isDefault: true, | |
| isTypeOnly: false, | |
| }); | |
| } | |
| // Check for default export | |
| if (sourceFile.getDefaultExportSymbol()) { | |
| availableExports.set('default', { | |
| name: 'default', | |
| isDefault: true, | |
| isTypeOnly: false, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In
packages/project-builder-server/src/templates/utils/infer-exports-from-ts-file.ts
around lines 87 to 96, the code uses defaultExportSymbol.getName() as the map
key which can produce non-'default' keys; change it to always use the literal
key 'default' (and set the export object's name to 'default' or otherwise ensure
the map key is 'default') so downstream importers receive the normalized
'default' key while keeping isDefault: true and isTypeOnly: false.
| @@ -0,0 +1,86 @@ | |||
| import path from 'node:path'; | |||
|
|
|||
| import type { ProjectInfo } from '../../api/projects.js'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Adopt alias import for internal types.
-import type { ProjectInfo } from '../../api/projects.js';
+import type { ProjectInfo } from '#src/api/projects.js';📝 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.
| import type { ProjectInfo } from '../../api/projects.js'; | |
| import type { ProjectInfo } from '#src/api/projects.js'; |
🤖 Prompt for AI Agents
In packages/project-builder-server/src/templates/utils/resolve-file-path.ts
around line 3, the import currently uses a relative path for the internal
ProjectInfo type; replace the relative import with the project's configured
alias import (e.g. use the internal alias defined in tsconfig/paths such as
@project/api/projects or #api/projects) so the module uses the canonical
internal alias for shared types; update the import statement accordingly and
ensure the project builds by adjusting any path mappings if needed.
| interface ResolvedFilePath { | ||
| /** The absolute file path */ | ||
| absolutePath: string; | ||
| /** The relative path from the project directory */ | ||
| projectRelativePath: string; | ||
| /** The project that contains this file */ | ||
| project: ProjectInfo; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Export the return type to make the function’s API consumable.
This type is part of the public contract of resolveFilePath.
-interface ResolvedFilePath {
+export interface ResolvedFilePath {📝 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.
| interface ResolvedFilePath { | |
| /** The absolute file path */ | |
| absolutePath: string; | |
| /** The relative path from the project directory */ | |
| projectRelativePath: string; | |
| /** The project that contains this file */ | |
| project: ProjectInfo; | |
| } | |
| export interface ResolvedFilePath { | |
| /** The absolute file path */ | |
| absolutePath: string; | |
| /** The relative path from the project directory */ | |
| projectRelativePath: string; | |
| /** The project that contains this file */ | |
| project: ProjectInfo; | |
| } |
🤖 Prompt for AI Agents
In packages/project-builder-server/src/templates/utils/resolve-file-path.ts
around lines 5 to 12, the ResolvedFilePath interface is declared but not
exported; export it so resolveFilePath's return type is part of the module's
public API. Change the interface declaration to an exported interface (export
interface ResolvedFilePath { ... }) and update any local usages/imports if
necessary so other modules can import and reference this type for the function's
return value.
| const matchingProject = projects.find((proj) => | ||
| absolutePath.startsWith(`${proj.directory}${path.sep}`), | ||
| ); | ||
|
|
There was a problem hiding this comment.
Fix absolute path containment check (root-equality bug, cross-platform robustness).
startsWith fails when absolutePath === proj.directory. Use path.relative containment logic.
- const matchingProject = projects.find((proj) =>
- absolutePath.startsWith(`${proj.directory}${path.sep}`),
- );
+ const matchingProject = projects.find((proj) => {
+ const rel = path.relative(proj.directory, absolutePath);
+ return rel === '' || (!rel.startsWith('..') && !path.isAbsolute(rel));
+ });📝 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.
| const matchingProject = projects.find((proj) => | |
| absolutePath.startsWith(`${proj.directory}${path.sep}`), | |
| ); | |
| const matchingProject = projects.find((proj) => { | |
| const rel = path.relative(proj.directory, absolutePath); | |
| return rel === '' || (!rel.startsWith('..') && !path.isAbsolute(rel)); | |
| }); |
🤖 Prompt for AI Agents
In packages/project-builder-server/src/templates/utils/resolve-file-path.ts
around lines 43-46, replace the startsWith containment check (which misses the
case where absolutePath === proj.directory and is fragile across platforms) with
a path.relative-based check: normalize/resolve both proj.directory and
absolutePath, compute path.relative(dir, absolutePath) and treat the path as
contained if the relative result is '' (same path) or does not start with '..'
(and not equal to '..'), taking care to use path.sep comparisons for
cross-platform correctness.
| const extractorJsonPath = path.join( | ||
| generatorDirectory, | ||
| EXTRACTOR_CONFIG_FILENAME, | ||
| ); |
There was a problem hiding this comment.
Path join is correct here, but there’s a critical glob bug above.
The extractorJsonPath construction via the constant looks good. However, Line 36 builds the glob with path.join('**', \/${generatorBasename}.generator.ts`). The leading slash makes the second segment absolute on POSIX, discarding the **` and likely breaking discovery. Fix the glob to avoid the leading slash.
- const searchPattern = path.join('**', `/${generatorBasename}.generator.ts`);
+ // Keep it a glob, not an absolute path
+ const searchPattern = `**/${generatorBasename}.generator.ts`;📝 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.
| const extractorJsonPath = path.join( | |
| generatorDirectory, | |
| EXTRACTOR_CONFIG_FILENAME, | |
| ); | |
| // Keep it a glob, not an absolute path | |
| const searchPattern = `**/${generatorBasename}.generator.ts`; |
1 participant
Summary by CodeRabbit