Open
Conversation
Adds downloadGitHubTemplate() that clones InsForge/insforge-templates repo, copies the selected template to the project dir, writes .env.local with credentials, and runs migrations/db_int.sql if present. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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)
📝 Coding Plan
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: 2
🧹 Nitpick comments (2)
src/commands/create.ts (2)
311-314: Consider handling missinggitmore gracefully.The shallow clone requires
gitto be installed. If it's not available, the error message fromexecAsyncmay not clearly indicate the root cause. A pre-check or clearer error would improve UX.💡 Suggested enhancement
await fs.mkdir(tempDir, { recursive: true }); // Shallow clone the templates repo - await execAsync( - 'git clone --depth 1 https://github.com/InsForge/insforge-templates.git .', - { cwd: tempDir, maxBuffer: 10 * 1024 * 1024, timeout: 60_000 }, - ); + try { + await execAsync( + 'git clone --depth 1 https://github.com/InsForge/insforge-templates.git .', + { cwd: tempDir, maxBuffer: 10 * 1024 * 1024, timeout: 60_000 }, + ); + } catch (err) { + const msg = (err as Error).message; + if (msg.includes('ENOENT') || msg.includes('not found') || msg.includes('not recognized')) { + throw new Error('Git is not installed or not in PATH. Please install git and try again.'); + } + throw err; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/create.ts` around lines 311 - 314, The git shallow-clone call using execAsync('git clone ...', { cwd: tempDir, ... }) can fail if git is not installed; add a pre-check before the execAsync call to detect git availability (e.g., run 'git --version' or check for ENOENT) and throw or log a user-friendly error that clearly states "git is required" including detected error details; alternatively wrap the execAsync call in a try/catch that inspects the thrown error (ENOENT or message including 'git') and replaces it with a clearer message. Update the logic around execAsync and tempDir in src/commands/create.ts so failures explicitly surface missing git vs other clone errors.
338-358: Database migration runs after spinner stops, which may confuse timing.The main spinner stops at line 336 ("template downloaded"), but database migration has its own spinner. This is acceptable, but note that if
getAnonKey()(line 328) fails, the error is caught at line 359 and the template download is reported as failed, even though files were already copied to cwd. Consider moving the anonKey fetch earlier or clarifying partial success state.Also, the migration endpoint
/api/database/advance/rawsql/unrestrictedexecutes arbitrary SQL from the template repository. This is by design for trusted templates, but ensure theinsforge-templatesrepository has appropriate access controls.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/create.ts` around lines 338 - 358, The code reports the template download as failed if getAnonKey() (function getAnonKey) fails after files are copied and the template spinner is stopped, and the DB migration spinner timing is confusing; to fix, move the getAnonKey() call to before starting/copying the template (so failures occur before the "template downloaded" spinner stop) or, alternatively, change the catch that logs the template failure (the catch around the template download/copy) to report a partial success/partial failure (mention files were copied) instead of a full download failure; also ensure the database migration spinner lifecycle (dbSpinner created in the migration block and its start/stop around ossFetch to /api/database/advance/rawsql/unrestricted) is independent of the template spinner so timing is clear, and add a code comment near the migration call to confirm that templates in insforge-templates are trusted/controlled before executing arbitrary SQL.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/create.ts`:
- Line 62: Add explicit validation for the --template option value where it's
parsed (the option declared as .option('--template <template>', 'Template to
use: react, nextjs, chatbot, or empty') and any code that assigns to the
framework variable), ensuring only the allowed values
['react','nextjs','chatbot','empty'] are accepted; if the provided value is
invalid, print a clear error and exit (or throw) instead of silently falling
back to react (the current fallback logic using framework === 'nextjs' ?
'nextjs' : 'react'). Implement this once as a small helper/validator used in all
parsing locations (references: the option declaration and the framework variable
usage) so the same validation is applied at the other spots noted in the review
(around the areas where template parsing occurs).
- Around line 327-334: The current write that uses fs.writeFile(path.join(cwd,
'.env.local'), envContent) will silently overwrite an existing .env.local;
change create logic to detect and merge instead: check for file existence using
fs.existsSync or fs.stat, read and parse existing .env.local into key/value
pairs, update or add only the NEXT_PUBLIC_INSFORGE_URL and
NEXT_PUBLIC_INSFORGE_ANON_KEY entries (leave all other keys untouched), then
write the merged content back (or warn and abort if merge fails). Update the
section around getAnonKey(), projectConfig, and the fs.writeFile call to perform
this read-merge-write flow so existing env vars are preserved.
---
Nitpick comments:
In `@src/commands/create.ts`:
- Around line 311-314: The git shallow-clone call using execAsync('git clone
...', { cwd: tempDir, ... }) can fail if git is not installed; add a pre-check
before the execAsync call to detect git availability (e.g., run 'git --version'
or check for ENOENT) and throw or log a user-friendly error that clearly states
"git is required" including detected error details; alternatively wrap the
execAsync call in a try/catch that inspects the thrown error (ENOENT or message
including 'git') and replaces it with a clearer message. Update the logic around
execAsync and tempDir in src/commands/create.ts so failures explicitly surface
missing git vs other clone errors.
- Around line 338-358: The code reports the template download as failed if
getAnonKey() (function getAnonKey) fails after files are copied and the template
spinner is stopped, and the DB migration spinner timing is confusing; to fix,
move the getAnonKey() call to before starting/copying the template (so failures
occur before the "template downloaded" spinner stop) or, alternatively, change
the catch that logs the template failure (the catch around the template
download/copy) to report a partial success/partial failure (mention files were
copied) instead of a full download failure; also ensure the database migration
spinner lifecycle (dbSpinner created in the migration block and its start/stop
around ossFetch to /api/database/advance/rawsql/unrestricted) is independent of
the template spinner so timing is clear, and add a code comment near the
migration call to confirm that templates in insforge-templates are
trusted/controlled before executing arbitrary SQL.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f5b99976-6b0d-42ab-96b8-bd6c730a5e1a
📒 Files selected for processing (1)
src/commands/create.ts
…ates Instead of writing hardcoded NEXT_PUBLIC_ vars to .env.local, the CLI now reads the template's .env.example, pattern-matches INSFORGE URL and ANON_KEY vars (regardless of prefix), and fills in the real values. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/create.ts`:
- Around line 333-341: The regex in the envExample.replace call (used to build
envContent) only matches letters and underscores and therefore skips env var
names containing digits; update the pattern used in envExample.replace from
/^([A-Z_]+=)(.*)$/gm to include digits in the character class (e.g., use
[A-Z0-9_]+) so variable names like API_KEY_V2 or REGION_1 are matched, keep the
same two capture groups and replacement logic in the envContent computation
(referencing envExample.replace and envContent).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eb0d2ad6-2f3f-4d91-b533-ca2dd45f8245
📒 Files selected for processing (1)
src/commands/create.ts
During `insforge create`, the CLI now reads .env.local / .env.production / .env and passes the variables to the deployment API so the deployed app has the correct InsForge credentials. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
tested my running cli to crate project |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/commands/create.ts (1)
156-160: Make template routing data-driven.Hard-coding
chatbotas the only GitHub-backed template means the next external template still needs branching changes here. A small registry mapping template → source/handler would keep template additions declarative.♻️ Example direction
+const templateSources: Record<string, 'github' | 'builtin'> = { + chatbot: 'github', + react: 'builtin', + nextjs: 'builtin', +}; + const hasTemplate = template !== 'empty'; -if (template === 'chatbot') { - await downloadGitHubTemplate('chatbot', projectConfig, json); -} else if (hasTemplate) { - await downloadTemplate(template as Framework, projectConfig, projectName, json, apiUrl); +if (hasTemplate) { + const selectedTemplate = template as string; + if (templateSources[selectedTemplate] === 'github') { + await downloadGitHubTemplate(selectedTemplate, projectConfig, json); + } else { + await downloadTemplate(selectedTemplate as Framework, projectConfig, projectName, json, apiUrl); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/create.ts` around lines 156 - 160, The template routing is hard-coded around the template variable and downloadGitHubTemplate/downloadTemplate calls; replace this branching with a small registry mapping template names to handler functions or source descriptors and dispatch to the appropriate handler instead of using if (template === 'chatbot') {...} else if (hasTemplate) {...}. Create a TemplateRegistry (map from template string to a handler function or an object describing {type, handler}) and in the create flow use templateRegistry.get(template) to invoke the correct handler (passing projectConfig, projectName, json, apiUrl as needed) with a sensible default/fallback to the existing downloadTemplate behavior for non-GitHub or unknown templates; update places that reference template, hasTemplate, downloadGitHubTemplate, and downloadTemplate to use the registry dispatch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/create.ts`:
- Around line 369-382: The migration and outer template-download catch blocks
currently only log warnings (in the catch handling around dbSpinner and s in
src/commands/create.ts) which allows execution to continue; change both catch
handlers to rethrow the caught error after logging (i.e., after
dbSpinner?.stop(...)/s?.stop(...) and clack.log.warn/info calls) so the error
bubbles up to registerCreateCommand and stops further steps (npm install /
deploy prompt) instead of proceeding with a partial project.
- Around line 320-323: Summary: The code currently clones the templates repo's
default branch via execAsync and immediately runs migrations (e.g.,
migrations/db_int.sql), which allows upstream changes to execute arbitrary SQL;
fix by pinning the template source or requiring explicit user confirmation.
Change the git clone in create.ts (the execAsync call referencing tempDir) to
clone a specific commit or tag supplied by a new
--template-version/--template-commit option (or default to a reviewed,
hard-coded tag), verify the ref was checked out successfully, and refuse to
proceed if no pinned ref is provided; alternatively, if dynamic checkouts are
allowed, require an explicit interactive confirmation before reading/posting
migrations/db_int.sql to the SQL endpoint and log the template ref being used.
Apply the same protection (pin-or-confirm) to the duplicate logic referenced
around the other migration execution (the code block at lines 363-367) so both
clone-and-migrate flows are gated.
In `@src/lib/env.ts`:
- Around line 8-28: readEnvFile currently returns after finding the first env
file and also preserves surrounding quotes verbatim; change it to read all
candidate files ('.env', '.env.production', '.env.local' order or similar where
later files override earlier) and merge entries so missing defaults are retained
while later files overwrite keys, and update the parsing logic inside
readEnvFile to strip surrounding single/double quotes from values and unescape
common escape sequences so deployed values match the intended contents; ensure
you update references to candidates, filePath checks, and the vars accumulation
logic so you accumulate across files and apply overrides by key rather than
returning early.
---
Nitpick comments:
In `@src/commands/create.ts`:
- Around line 156-160: The template routing is hard-coded around the template
variable and downloadGitHubTemplate/downloadTemplate calls; replace this
branching with a small registry mapping template names to handler functions or
source descriptors and dispatch to the appropriate handler instead of using if
(template === 'chatbot') {...} else if (hasTemplate) {...}. Create a
TemplateRegistry (map from template string to a handler function or an object
describing {type, handler}) and in the create flow use
templateRegistry.get(template) to invoke the correct handler (passing
projectConfig, projectName, json, apiUrl as needed) with a sensible
default/fallback to the existing downloadTemplate behavior for non-GitHub or
unknown templates; update places that reference template, hasTemplate,
downloadGitHubTemplate, and downloadTemplate to use the registry dispatch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1f3f4d9e-c89a-4833-98e0-847cc712d8a1
📒 Files selected for processing (3)
package.jsonsrc/commands/create.tssrc/lib/env.ts
✅ Files skipped from review due to trivial changes (1)
- package.json
src/commands/create.ts
Outdated
Comment on lines
+369
to
+382
| } catch (err) { | ||
| dbSpinner?.stop('Database migration failed'); | ||
| if (!json) { | ||
| clack.log.warn(`Migration failed: ${(err as Error).message}`); | ||
| clack.log.info('You can run the migration manually: insforge db query --unrestricted "$(cat migrations/db_int.sql)"'); | ||
| } | ||
| } | ||
| } | ||
| } catch (err) { | ||
| s?.stop(`${templateName} template download failed`); | ||
| if (!json) { | ||
| clack.log.warn(`Failed to download ${templateName} template: ${(err as Error).message}`); | ||
| clack.log.info('You can manually clone from: https://github.com/InsForge/insforge-templates'); | ||
| } |
There was a problem hiding this comment.
Bubble template bootstrap failures back to create.
Both the migration block and the outer template-setup catch only warn, so the command keeps going into npm install and the deploy prompt with a partial project. Return a failure here so registerCreateCommand can stop before presenting the setup as successful.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/create.ts` around lines 369 - 382, The migration and outer
template-download catch blocks currently only log warnings (in the catch
handling around dbSpinner and s in src/commands/create.ts) which allows
execution to continue; change both catch handlers to rethrow the caught error
after logging (i.e., after dbSpinner?.stop(...)/s?.stop(...) and
clack.log.warn/info calls) so the error bubbles up to registerCreateCommand and
stops further steps (npm install / deploy prompt) instead of proceeding with a
partial project.
Contributor
Author
There was a problem hiding this comment.
Intentional design — migration failure is non-fatal because the project, env file, and template files are already set up successfully. Rethrowing would kill the entire create flow over a migration hiccup. The user gets a warning with a manual command to retry.
There was a problem hiding this comment.
@tonychang04, that's a fair point — since the scaffolding (project, env, and template files) is already complete at that stage, treating the migration as a non-fatal step with a manual retry path is a reasonable UX choice. Acknowledged!
✏️ Learnings added
Learnt from: tonychang04
Repo: InsForge/CLI PR: 27
File: src/commands/create.ts:0-0
Timestamp: 2026-03-19T22:19:06.362Z
Learning: In `src/commands/create.ts` (InsForge CLI), migration failures inside `downloadGitHubTemplate` are intentionally non-fatal. The project creation, env file generation, and template file copy are all complete before migrations run, so a migration hiccup should only warn the user (with a manual retry command) rather than abort the entire `create` flow.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
- Reject invalid --template values with a clear error - Env var regex now matches names containing digits (e.g. API_KEY_V2) - Strip surrounding quotes from env file values before deploying 0.1.25 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
0.1.26 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Prompt user for confirmation before executing SQL from downloaded templates, addressing CodeRabbit review feedback. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jwfing
reviewed
Mar 19, 2026
|
|
||
| if (!clack.isCancel(runMigration) && runMigration) { | ||
| const dbSpinner = clack.spinner(); | ||
| dbSpinner.start('Running database migrations...'); |
Member
There was a problem hiding this comment.
Why not call insforge db query {sql} instead of REST API?
jwfing
reviewed
Mar 19, 2026
| // Run database migrations if db_int.sql exists | ||
| const migrationPath = path.join(cwd, 'migrations', 'db_int.sql'); | ||
| const migrationExists = await fs.stat(migrationPath).catch(() => null); | ||
| if (migrationExists && !json) { |
Member
There was a problem hiding this comment.
With insforge create --json --template chatbot, the scaffold still includes migrations/db_int.sql, but this block skips it entirely because it is gated on !json. That means the non-interactive/automation path reports success while leaving the chatbot template with an uninitialized schema, so the generated app is broken until the caller notices and runs the SQL manually.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
downloadGitHubTemplate()function that clones fromInsForge/insforge-templatesrepoinsforge createtemplate selector.env.localwith InsForge credentialsmigrations/db_int.sqlif present in the templateTest plan
insforge create, select "AI Chatbot with Next.js".env.localis written with correct credentialsdb_int.sqlmigration runs against the databasenpm installandnpm run devwork--template chatbotflag in non-interactive mode🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores