Conversation
How to use the Graphite Merge QueueAdd the label merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Preview deployed successfully!
Built from commit f6cb4c7 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| name: readStringFlagInternal(parsed, "name") | ||
| ? String(readStringFlagInternal(parsed, "name")) | ||
| : allowPrompts | ||
| ? await promptTextInternal("Template name", defaultName) | ||
| : defaultName, | ||
| description: readStringFlagInternal(parsed, "description") | ||
| ? String(readStringFlagInternal(parsed, "description")) | ||
| : allowPrompts | ||
| ? await promptTextInternal("Description", defaultDescription) | ||
| : defaultDescription, | ||
| author: readStringFlagInternal(parsed, "author") | ||
| ? String(readStringFlagInternal(parsed, "author")) | ||
| : allowPrompts | ||
| ? await promptTextInternal("Author", defaultAuthor) | ||
| : defaultAuthor, | ||
| tags, | ||
| serviceName: readStringFlagInternal(parsed, "service") | ||
| ? String(readStringFlagInternal(parsed, "service")) | ||
| : allowPrompts | ||
| ? await promptTextInternal("Service name", defaultService) | ||
| : defaultService, | ||
| image: readStringFlagInternal(parsed, "image") | ||
| ? String(readStringFlagInternal(parsed, "image")) | ||
| : allowPrompts | ||
| ? await promptTextInternal("Container image", defaultImage) | ||
| : defaultImage, | ||
| websiteUrl: readStringFlagInternal(parsed, "website") | ||
| ? String(readStringFlagInternal(parsed, "website")) | ||
| : allowPrompts | ||
| ? await promptTextInternal("Website URL", defaultWebsite) | ||
| : defaultWebsite, | ||
| iconUrl: readStringFlagInternal(parsed, "icon") | ||
| ? String(readStringFlagInternal(parsed, "icon")) | ||
| : allowPrompts | ||
| ? await promptTextInternal("Icon URL", defaultIcon) | ||
| : defaultIcon, | ||
| composeFileName, | ||
| includeReadme, | ||
| }; |
There was a problem hiding this comment.
readStringFlagInternal is invoked twice for every optional flag — once to test truthiness, then again to retrieve the value. Since it already returns string | undefined, the truthiness check is implicit in the ternary. This pattern repeats for name, description, author, service, image, website, and icon.
| name: readStringFlagInternal(parsed, "name") | |
| ? String(readStringFlagInternal(parsed, "name")) | |
| : allowPrompts | |
| ? await promptTextInternal("Template name", defaultName) | |
| : defaultName, | |
| description: readStringFlagInternal(parsed, "description") | |
| ? String(readStringFlagInternal(parsed, "description")) | |
| : allowPrompts | |
| ? await promptTextInternal("Description", defaultDescription) | |
| : defaultDescription, | |
| author: readStringFlagInternal(parsed, "author") | |
| ? String(readStringFlagInternal(parsed, "author")) | |
| : allowPrompts | |
| ? await promptTextInternal("Author", defaultAuthor) | |
| : defaultAuthor, | |
| tags, | |
| serviceName: readStringFlagInternal(parsed, "service") | |
| ? String(readStringFlagInternal(parsed, "service")) | |
| : allowPrompts | |
| ? await promptTextInternal("Service name", defaultService) | |
| : defaultService, | |
| image: readStringFlagInternal(parsed, "image") | |
| ? String(readStringFlagInternal(parsed, "image")) | |
| : allowPrompts | |
| ? await promptTextInternal("Container image", defaultImage) | |
| : defaultImage, | |
| websiteUrl: readStringFlagInternal(parsed, "website") | |
| ? String(readStringFlagInternal(parsed, "website")) | |
| : allowPrompts | |
| ? await promptTextInternal("Website URL", defaultWebsite) | |
| : defaultWebsite, | |
| iconUrl: readStringFlagInternal(parsed, "icon") | |
| ? String(readStringFlagInternal(parsed, "icon")) | |
| : allowPrompts | |
| ? await promptTextInternal("Icon URL", defaultIcon) | |
| : defaultIcon, | |
| composeFileName, | |
| includeReadme, | |
| }; | |
| id, | |
| name: | |
| readStringFlagInternal(parsed, "name") ?? | |
| (allowPrompts ? await promptTextInternal("Template name", defaultName) : defaultName), | |
| description: | |
| readStringFlagInternal(parsed, "description") ?? | |
| (allowPrompts | |
| ? await promptTextInternal("Description", defaultDescription) | |
| : defaultDescription), | |
| author: | |
| readStringFlagInternal(parsed, "author") ?? | |
| (allowPrompts ? await promptTextInternal("Author", defaultAuthor) : defaultAuthor), | |
| tags, | |
| serviceName: | |
| readStringFlagInternal(parsed, "service") ?? | |
| (allowPrompts | |
| ? await promptTextInternal("Service name", defaultService) | |
| : defaultService), | |
| image: | |
| readStringFlagInternal(parsed, "image") ?? | |
| (allowPrompts ? await promptTextInternal("Container image", defaultImage) : defaultImage), | |
| websiteUrl: | |
| readStringFlagInternal(parsed, "website") ?? | |
| (allowPrompts ? await promptTextInternal("Website URL", defaultWebsite) : defaultWebsite), | |
| iconUrl: | |
| readStringFlagInternal(parsed, "icon") ?? | |
| (allowPrompts ? await promptTextInternal("Icon URL", defaultIcon) : defaultIcon), | |
| composeFileName, | |
| includeReadme, |
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/cli.ts
Line: 255-293
Comment:
**Redundant double-call pattern for flag reads**
`readStringFlagInternal` is invoked twice for every optional flag — once to test truthiness, then again to retrieve the value. Since it already returns `string | undefined`, the truthiness check is implicit in the ternary. This pattern repeats for `name`, `description`, `author`, `service`, `image`, `website`, and `icon`.
```suggestion
id,
name:
readStringFlagInternal(parsed, "name") ??
(allowPrompts ? await promptTextInternal("Template name", defaultName) : defaultName),
description:
readStringFlagInternal(parsed, "description") ??
(allowPrompts
? await promptTextInternal("Description", defaultDescription)
: defaultDescription),
author:
readStringFlagInternal(parsed, "author") ??
(allowPrompts ? await promptTextInternal("Author", defaultAuthor) : defaultAuthor),
tags,
serviceName:
readStringFlagInternal(parsed, "service") ??
(allowPrompts
? await promptTextInternal("Service name", defaultService)
: defaultService),
image:
readStringFlagInternal(parsed, "image") ??
(allowPrompts ? await promptTextInternal("Container image", defaultImage) : defaultImage),
websiteUrl:
readStringFlagInternal(parsed, "website") ??
(allowPrompts ? await promptTextInternal("Website URL", defaultWebsite) : defaultWebsite),
iconUrl:
readStringFlagInternal(parsed, "icon") ??
(allowPrompts ? await promptTextInternal("Icon URL", defaultIcon) : defaultIcon),
composeFileName,
includeReadme,
```
How can I resolve this? If you propose a fix, please make it concise.
| async function promptTextInternal( | ||
| question: string, | ||
| fallback: string, | ||
| ): Promise<string> { | ||
| const rl = createInterface({ | ||
| input: process.stdin, | ||
| output: process.stdout, | ||
| }); | ||
|
|
||
| try { | ||
| const response = await rl.question(`${question} [${fallback}]: `); | ||
| const trimmed = response.trim(); | ||
| return trimmed.length > 0 ? trimmed : fallback; | ||
| } finally { | ||
| rl.close(); | ||
| } | ||
| } | ||
|
|
||
| async function promptBooleanInternal( | ||
| question: string, | ||
| fallback: boolean, | ||
| ): Promise<boolean> { | ||
| const suffix = fallback ? "Y/n" : "y/N"; | ||
| const rl = createInterface({ | ||
| input: process.stdin, | ||
| output: process.stdout, | ||
| }); | ||
|
|
||
| try { | ||
| const response = await rl.question(`${question} [${suffix}]: `); | ||
| const normalized = response.trim().toLowerCase(); | ||
| if (normalized.length === 0) { | ||
| return fallback; | ||
| } | ||
| return normalized === "y" || normalized === "yes"; | ||
| } finally { | ||
| rl.close(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Both promptTextInternal and promptBooleanInternal construct and immediately close a readline.Interface for each question. When collectCreateOptionsInternal chains many prompts, stdin is repeatedly paused/resumed and the createInterface overhead is paid multiple times. A single shared interface should be opened at the start and closed after the last prompt.
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/cli.ts
Line: 159-197
Comment:
**New readline interface created per prompt**
Both `promptTextInternal` and `promptBooleanInternal` construct and immediately close a `readline.Interface` for each question. When `collectCreateOptionsInternal` chains many prompts, stdin is repeatedly paused/resumed and the `createInterface` overhead is paid multiple times. A single shared interface should be opened at the start and closed after the last prompt.
How can I resolve this? If you propose a fix, please make it concise.| "scripts": { | ||
| "preinstall": "npx only-allow pnpm", | ||
| "build": "tsc --project tsconfig.json", | ||
| "prepare": "pnpm run build", |
There was a problem hiding this comment.
prepare script hard-codes pnpm run build
Now that preinstall: only-allow pnpm was removed (intentionally for public publishing), contributors who npm install or yarn install will hit a cryptic error because prepare invokes pnpm run build. Consider using tsc --project tsconfig.json directly in prepare so it works regardless of the package manager:
| "prepare": "pnpm run build", | |
| "prepare": "tsc --project tsconfig.json", |
Prompt To Fix With AI
This is a comment left during a code review.
Path: package.json
Line: 18
Comment:
**`prepare` script hard-codes `pnpm run build`**
Now that `preinstall: only-allow pnpm` was removed (intentionally for public publishing), contributors who `npm install` or `yarn install` will hit a cryptic error because `prepare` invokes `pnpm run build`. Consider using `tsc --project tsconfig.json` directly in `prepare` so it works regardless of the package manager:
```suggestion
"prepare": "tsc --project tsconfig.json",
```
How can I resolve this? If you propose a fix, please make it concise.| for (const dir of templateDirs) { | ||
| const templateDir = path.join(TEMPLATES_DIR, dir); | ||
| const composeFile = await findComposeFile(templateDir); | ||
| const composePath = path.join(templateDir, composeFile); | ||
| const envPath = path.join(templateDir, ".env"); | ||
| const envExamplePath = path.join(templateDir, ".env.example"); | ||
| const hasEnvFile = await exists(envPath); | ||
|
|
||
| if (!hasEnvFile) { | ||
| const envExample = await fs.readFile(envExamplePath, "utf8"); | ||
| await fs.writeFile(envPath, envExample, "utf8"); | ||
| } | ||
|
|
||
| try { | ||
| run( | ||
| "docker", | ||
| [ | ||
| "compose", | ||
| "--project-directory", | ||
| templateDir, | ||
| "-f", | ||
| composePath, | ||
| "--env-file", | ||
| envPath, | ||
| "config", | ||
| "-q", | ||
| ], | ||
| templateDir, | ||
| ); | ||
| console.log( | ||
| `Compose validation passed for templates/${dir}/${composeFile}`, | ||
| ); | ||
| } finally { | ||
| if (!hasEnvFile && (await exists(envPath))) { | ||
| await fs.rm(envPath); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
run() throws on a non-zero exit code and the error propagates out of the for loop, so any single failing template short-circuits validation for all subsequent templates. Users would need to fix each template one at a time and re-run to find the next failure. Consider catching the error, collecting it, and re-throwing after all templates have been checked.
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/lib/validation.ts
Line: 90-127
Comment:
**First compose failure terminates all remaining template validation**
`run()` throws on a non-zero exit code and the error propagates out of the `for` loop, so any single failing template short-circuits validation for all subsequent templates. Users would need to fix each template one at a time and re-run to find the next failure. Consider catching the error, collecting it, and re-throwing after all templates have been checked.
How can I resolve this? If you propose a fix, please make it concise.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| "dependencies": { | ||
| "ajv": "^8.18.0", | ||
| "ajv-formats": "^3.0.1" | ||
| }, |
There was a problem hiding this comment.
🔴 Published CLI will crash because prettier is missing from dependencies
prettier is imported at the top level in both scripts/lib/registry.ts:4 and scripts/lib/template-scaffold.ts:3, but it is only listed in devDependencies, not in dependencies. Since the package is now public ("private": false) with bin entries pointing to dist/scripts/cli.js, anyone installing via npx arcane-templates or npm install -g arcane-templates will not get prettier installed (npm only installs dependencies, not devDependencies for published packages). Because cli.ts eagerly imports all modules at the top level, the import prettier from "prettier" statement will fail on module load, crashing every CLI command — not just generate and create which actually use prettier, but also help, validate, and stage.
| "dependencies": { | |
| "ajv": "^8.18.0", | |
| "ajv-formats": "^3.0.1" | |
| }, | |
| "dependencies": { | |
| "ajv": "^8.18.0", | |
| "ajv-formats": "^3.0.1", | |
| "prettier": "^3.8.3" | |
| }, |
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Disclaimer Greptiles Reviews use AI, make sure to check over its work.
To better help train Greptile on our codebase, if the comment is useful and valid Like the comment, if its not helpful or invalid Dislike
To have Greptile Re-Review the changes, mention
greptileai.Greptile Summary
This PR introduces the
arcanetmpl/arcane-templatesCLI tool by adding a newscripts/cli.tsentrypoint, extracting the registry generation, validation, staging, and template-scaffolding logic into dedicatedscripts/lib/modules, and wiring them up as named subcommands. The package is made publishable (private: false),binfields are added, and runtime dependencies (ajv,ajv-formats,prettier) are correctly moved fromdevDependenciestodependencies.Confidence Score: 5/5
Safe to merge — all findings are minor style/UX improvements with no correctness impact.
The refactoring cleanly separates concerns, the CLI routing is correct, error handling is preserved from the original scripts, and the publish configuration looks sound. All four comments are P2 (style/quality).
No files require special attention.
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat: arcanetmpl cli tool" | Re-trigger Greptile