feat: auto-install sdlc as dev dependency #5
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the codebase to extract a reusable syscall function and adds npm integration capabilities. The main purpose is to enable automatic installation of the sdlc package as a dev dependency during project initialization.
- Introduces a generic
syscallfunction to handle command-line executions - Refactors the
gitfunction to use the newsyscallutility - Adds npm wrapper functions (
npm,npmInit,npmInstall) using thesyscallutility - Updates the
initcommand to initialize npm and installsdlcas a dev dependency
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/syscall.ts | New generic utility function for executing system commands with unified error handling |
| src/git/git.ts | Refactored to use the new syscall function instead of custom spawn implementation |
| src/npm/npm.ts | New npm wrapper using syscall |
| src/npm/npm-init.ts | Implements npm initialization functionality |
| src/npm/npm-install.ts | Implements npm package installation functionality |
| src/npm/index.ts | Exports npm-related functions |
| src/commands/init.command.ts | Enhanced to initialize npm and install sdlc as dev dependency |
| test/spec/npm/npm.spec.ts | Tests for basic npm command execution |
| test/spec/npm/npm-init.spec.ts | Tests for npm init functionality |
| test/spec/npm/npm-install.spec.ts | Tests for npm install functionality |
| test/spec/index.spec.ts | Added timeout configuration for longer-running tests |
| test/spec/git/git.spec.ts | Updated error message assertion to match new format |
| test/spec/commands/init-command.spec.ts | Added assertions for package.json creation and sdlc dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ): Promise<string> { | ||
| try { | ||
| const cwd = options.cwd || process.cwd(); | ||
| args = args.map((arg) => `"${arg.replace(/"/g, '\\"')}"`); |
There was a problem hiding this comment.
Arguments are wrapped in double quotes and escaped, but this may not handle all edge cases for shell command injection. For example, backticks, dollar signs, and other shell metacharacters could still pose security risks. Consider using an array-based execution approach or a more robust escaping library to prevent command injection vulnerabilities.
| args = args.map((arg) => `"${arg.replace(/"/g, '\\"')}"`); | ||
| const cmd = [command, subcommand, ...args].join(' '); |
There was a problem hiding this comment.
Arguments are being joined into a single string for shell execution, but execSync with a string command uses shell parsing which can be problematic. The original git implementation used spawn with an array of arguments, which is safer. Consider using execFileSync(command, [subcommand, ...args], options) instead to avoid shell interpretation and maintain the safety of the original implementation.
| await createConfigFile(); | ||
| await npmInit(); | ||
| await npmInstall({ | ||
| package: 'sdlc@^1.0.0', |
There was a problem hiding this comment.
The version ^1.0.0 is hardcoded in the init command. This creates a maintenance burden as the version needs to be manually updated when new versions are released. Consider dynamically reading the version from the current package's package.json or using a constant that can be updated in a single location.
No description provided.