Conversation
- Add interactive platform multi-select with scope choice (global/project) - Install OpenSpec + Superpowers via official CLI in batch mode - Copy Comet skills to all selected platforms - Add ASCII art banner for init command - Add .npmignore to prevent source/secrets from entering npm package - Add prepublish-check.js security scan (blocks publish if secrets detected) - Enhance .gitignore with comprehensive secret/IDE/credential patterns
- Comprehensive English README with ASCII banner, platform table, skills reference, workflow diagram, and project structure - Version bump to 0.1.1
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly enhances the Comet CLI by introducing an interactive initialization process and robust security measures. Key updates include a refactored init command that supports 28 AI coding platforms with configurable installation scopes, a new pre-publish security script to scan for secrets, and comprehensive documentation updates. Reviewer feedback focused on improving the reliability of the installation process, specifically recommending the use of development dependencies for local tool installations and switching execSync output handling from pipe to inherit to avoid potential buffer overflow issues and provide better progress visibility to the user.
| ? 'npm install -g @fission-ai/openspec@latest' | ||
| : 'npm install @fission-ai/openspec@latest'; | ||
| execSync(npmCmd, { cwd: projectPath, stdio: 'pipe', timeout: 120_000 }); |
There was a problem hiding this comment.
When installing @fission-ai/openspec at the project level, it is generally better to save it as a development dependency since it is a tool used during development.
| ? 'npm install -g @fission-ai/openspec@latest' | |
| : 'npm install @fission-ai/openspec@latest'; | |
| execSync(npmCmd, { cwd: projectPath, stdio: 'pipe', timeout: 120_000 }); | |
| const npmCmd = scope === 'global' | |
| ? 'npm install -g @fission-ai/openspec@latest' | |
| : 'npm install --save-dev @fission-ai/openspec@latest'; |
| ? 'npm install -g @fission-ai/openspec@latest' | ||
| : 'npm install @fission-ai/openspec@latest'; | ||
| execSync(npmCmd, { cwd: projectPath, stdio: 'pipe', timeout: 120_000 }); | ||
| return isCommandAvailable('openspec'); |
There was a problem hiding this comment.
Using stdio: 'pipe' with execSync for commands like npm install can be problematic. If the output exceeds the default buffer size (typically 1MB), the process will crash. Additionally, it hides progress from the user during a potentially long operation. Consider using stdio: 'inherit' to show progress and avoid buffer limits, or at least increase maxBuffer.
| return isCommandAvailable('openspec'); | |
| execSync(npmCmd, { cwd: projectPath, stdio: 'inherit', timeout: 120_000 }); |
|
|
||
| execSync(`openspec init ${flags}`, { | ||
| cwd: projectPath, | ||
| stdio: 'pipe', |
There was a problem hiding this comment.
Similar to the npm install command, using stdio: 'pipe' here risks a buffer overflow if openspec init produces significant output. Using stdio: 'inherit' would provide better visibility into the initialization process.
| stdio: 'pipe', | |
| stdio: 'inherit', |
|
|
||
| execSync(`npx skills add obra/superpowers ${flags}`, { | ||
| cwd: projectPath, | ||
| stdio: 'pipe', |
There was a problem hiding this comment.
Using stdio: 'pipe' for npx skills add may cause issues if the command produces a lot of output or if npx prompts for package installation. Consider using stdio: 'inherit'.
| stdio: 'pipe', | |
| stdio: 'inherit', |
fix: enable shell option for Windows in command execution
1 participant
No description provided.