feat(e2b): add sandbox templates for common workflows (v1.1) - #48
Conversation
Add a comprehensive template management system for E2B sandboxes that enables pre-configured development environments. New features: - TemplateManager class for CRUD operations on templates - Built-in templates: node-20-typescript, python-3.12-fastapi, full-stack-nextjs - CLI commands: templates-list, templates-show, templates-create, templates-delete, templates-export, templates-import - --use-template option for sandbox-run command - Project type auto-detection for template suggestions - Template validation and reserved name protection Implementation details: - Templates stored as JSON in templates/ (built-in) and ~/.parallel-cc/templates/ (custom) - SandboxManager.applyTemplate() executes setup commands and sets environment variables - Non-blocking template application (warns but continues on failure) - ES module compatible with import.meta.url for path resolution Test coverage: - 50 new template tests (TemplateManager, validation, detection) - 6 new applyTemplate tests in sandbox-manager - All 864 tests passing Documentation: - Updated CLAUDE.md with new CLI commands and template usage - Added version history entry for v1.1
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds TemplateManager and template types, three built-in JSON templates, CLI template commands and a --use-template option, SandboxManager.applyTemplate to run setupCommands and set environment vars, and tests for template management and application. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant TemplateManager
participant FileSystem
participant SandboxManager
participant Sandbox
CLI->>TemplateManager: list/get/create/delete/import/export
TemplateManager->>FileSystem: read/write template JSON (built-in/custom)
FileSystem-->>TemplateManager: JSON / result
TemplateManager-->>CLI: template list / object / status
CLI->>SandboxManager: create sandbox / sandbox-run (--use-template)
SandboxManager->>Sandbox: create/upload/check sandbox
Sandbox-->>SandboxManager: sandbox ready
SandboxManager->>TemplateManager: getTemplate(name)
TemplateManager-->>SandboxManager: SandboxTemplate
SandboxManager->>Sandbox: set environment variables
loop setupCommands
SandboxManager->>Sandbox: execute setupCommand
Sandbox-->>SandboxManager: command exit status
end
SandboxManager-->>CLI: TemplateApplicationResult (success|failure, details)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)src/**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/cli.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (7)📓 Common learnings📚 Learning: 2026-01-13T09:16:25.976ZApplied to files:
📚 Learning: 2026-01-13T09:16:25.976ZApplied to files:
📚 Learning: 2026-01-13T09:16:25.976ZApplied to files:
📚 Learning: 2026-01-13T09:16:25.976ZApplied to files:
📚 Learning: 2026-01-13T09:16:25.976ZApplied to files:
📚 Learning: 2026-01-13T09:16:25.976ZApplied to files:
🧬 Code graph analysis (1)src/cli.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (10)
✏️ Tip: You can disable this entire section by setting Comment |
Add sandbox templates for common workflows and integrate
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
metadata is never validated. Consider adding shape checks (e.g., author/version strings, tags as array of strings, createdAt/updatedAt strings), or document why metadata is intentionally free-form.
🚀 Want me to fix this? Reply ex: "fix it for me".
| const filePath = path.join(this.config.customDir, `${name}.json`); | ||
| await fs.unlink(filePath); | ||
|
|
There was a problem hiding this comment.
deleteTemplate derives ${name}.json, but files may not match the internal name. Delete the actual matched file or catch ENOENT and return not‑found.
| const filePath = path.join(this.config.customDir, `${name}.json`); | |
| await fs.unlink(filePath); | |
| const filePath = path.join(this.config.customDir, `${name}.json`); | |
| try { | |
| await fs.unlink(filePath); | |
| } catch (error: any) { | |
| if (error.code === 'ENOENT') { | |
| return { | |
| success: false, | |
| message: 'Template deletion failed', | |
| error: `Template file for "${name}" not found` | |
| }; | |
| } | |
| throw error; | |
| } |
🚀 Want me to fix this? Reply ex: "fix it for me".
| } else if (cmd.length > MAX_COMMAND_LENGTH) { | ||
| errors.push(`setupCommands[${i}] exceeds maximum length of ${MAX_COMMAND_LENGTH} characters`); | ||
| } |
There was a problem hiding this comment.
Empty setupCommands strings pass validation. Consider rejecting blanks (e.g., cmd.trim() === '') to avoid executing empty commands.
| } else if (cmd.length > MAX_COMMAND_LENGTH) { | |
| errors.push(`setupCommands[${i}] exceeds maximum length of ${MAX_COMMAND_LENGTH} characters`); | |
| } | |
| } else if (cmd.trim() === '') { | |
| errors.push(`setupCommands[${i}] cannot be empty`); | |
| } else if (cmd.length > MAX_COMMAND_LENGTH) { | |
| errors.push(`setupCommands[${i}] exceeds maximum length of ${MAX_COMMAND_LENGTH} characters`); | |
| } |
🚀 Want me to fix this? Reply ex: "fix it for me".
| await fs.access(path.join(repoPath, 'package.json')); | ||
| detectedFiles.push('package.json'); | ||
|
|
||
| // Read package.json to check for specific frameworks |
There was a problem hiding this comment.
Only record package.json after a successful parse and object check, and handle parse errors explicitly (e.g., report Node with a reason instead of falling through) to keep detection consistent.
🚀 Want me to fix this? Reply ex: "fix it for me".
Review: feat(e2b): add sandbox templates for common workflows (v1.1)OverviewThis PR adds a comprehensive template management system for E2B sandboxes with built-in templates for common workflows. The implementation is well-designed with solid test coverage (914 tests passing). ✅ StrengthsCode Quality
Validation & Safety
Testing
Documentation
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/cli.ts`:
- Around line 1802-1827: The block handling managedTemplate (around the
sandboxManager.applyTemplate call and using templateResult and options.json)
must stop using console methods: replace console.log(...) with logger.info(...)
and replace console.warn(...) with logger.warn(...), keeping the same messages
and conditionals (including the chalk formatting and checks for
templateResult.commandsExecuted/environmentVarsSet) so all output uses the
existing logger utility.
In `@src/e2b/sandbox-manager.ts`:
- Around line 711-741: The code currently builds envExports by interpolating
template.environment values directly, which allows shell injection; update the
env export construction used before calling sandbox.commands.run to safely
escape values: for each [key,value] in template.environment produce export
KEY='escaped' where escaped is the value with all single quotes replaced by '\''
(i.e. close-quote, escaped single-quote, reopen-quote) so embedded single quotes
are handled and the whole value is single-quoted to prevent $, `, and other
expansions; keep the same join(' && ') and the sandbox.commands.run call (and
timeoutMs) but replace the envExports generation code to use this safe escaping
and then set environmentVarsSet as before on success.
- Around line 719-732: The template's environment variables are being exported
at applyTemplate() runtime (via sandbox.commands.run exports) which won't
persist; instead modify createSandbox() to accept and pass template.environment
to Sandbox.create() using its envs parameter (or add an envs arg to
createSandbox() if missing), remove the shell export logic and any assignment to
environmentVarsSet inside applyTemplate(), and ensure subsequent calls reuse the
created sandbox so those envs are available for all later sandbox.commands.run
invocations.
In `@templates/node-20-typescript.json`:
- Around line 2-8: The "description" field claims "Jest/Vitest" but the
"setupCommands" array doesn't install either and the generic "npm install" will
fail without a package.json; update the JSON so the description matches the
actual setup or add installs for testing frameworks and ensure package.json
exists: either remove "Jest/Vitest" from the "description" key, or add
appropriate install commands (e.g., add entries to "setupCommands" to install
jest or vitest and their types/configs) and/or ensure a package.json is created
before running "npm install" so the sandbox setup succeeds.
🧹 Nitpick comments (3)
templates/full-stack-nextjs.json (1)
5-8: Consider removing global Next.js installation.Installing
next@latestglobally (line 6) is non-standard. Next.js is typically a local project dependency, andnpx nextworks without a global install. The global version may also conflict with project-specific versions.Additionally,
npm install(line 7) will fail if nopackage.jsonexists yet, which may be the case for new sandboxes.♻️ Suggested fix
"setupCommands": [ - "npm install -g next@latest", - "npm install", + "npm install || true", "npx playwright install --with-deps chromium" ],Or if the intent is to scaffold a new Next.js project, consider:
"setupCommands": [ "npx create-next-app@latest --typescript --tailwind --eslint --app --src-dir --import-alias '@/*' my-app || npm install", "npx playwright install --with-deps chromium" ]CLAUDE.md (1)
544-550: Add blank line before table for markdown compliance.The static analysis tool flagged MD058 (tables should be surrounded by blank lines). Add a blank line before the "Built-in Templates" table.
📝 Suggested fix
**Built-in Templates:** + | Template | Description | Setup | |----------|-------------|-------| | `node-20-typescript` | Node.js 20 with TypeScript tooling | TypeScript, ESLint, Prettier + npm install |src/cli.ts (1)
1414-1433: Template loading handles errors correctly.The error handling provides helpful feedback in both JSON and human-readable formats, including a hint to run
templates-list.Minor style note: Line 1415 uses dynamic type import
import('./types.js').SandboxTemplateinstead of importing the type at the top of the file. Consider addingSandboxTemplateto the existing type imports at line 43 for consistency.📝 Suggested simplification
-import { SandboxStatus, type E2BSession, type StatusResult, type SessionInfo } from './types.js'; +import { SandboxStatus, type E2BSession, type StatusResult, type SessionInfo, type SandboxTemplate } from './types.js';Then at line 1415:
- let managedTemplate: import('./types.js').SandboxTemplate | null = null; + let managedTemplate: SandboxTemplate | null = null;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
CLAUDE.mdsrc/cli.tssrc/e2b/sandbox-manager.tssrc/e2b/templates.tssrc/types.tstemplates/full-stack-nextjs.jsontemplates/node-20-typescript.jsontemplates/python-3.12-fastapi.jsontests/e2b/sandbox-manager.test.tstests/e2b/templates.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
tests/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.test.ts: Use Vitest as the testing framework for unit and integration tests
Write unit and integration tests for all new features and bug fixes
Files:
tests/e2b/templates.test.tstests/e2b/sandbox-manager.test.ts
src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.ts: Use TypeScript strict mode for all source files
Use ES modules (type: "module") for all TypeScript source files
Use async/await over callbacks for asynchronous operations in TypeScript source files
Implement explicit error handling for all async/await operations in TypeScript source files
Use meaningful and descriptive variable names throughout the codebase
Maintain >85% test coverage across all source files
Use better-sqlite3 via the SessionDB class in db.ts for all database operations
Validate all database inputs using db-validators.ts functions before database operations
Use the logger utility from logger.ts for all console output and logging
Wrap gtr CLI commands through GtrWrapper class in gtr.ts instead of direct subprocess calls
Automatically redact sensitive data (API keys, credentials, SSH keys) from all logs
Files:
src/e2b/sandbox-manager.tssrc/types.tssrc/e2b/templates.tssrc/cli.ts
src/e2b/sandbox-manager.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Implement E2B sandbox lifecycle management in src/e2b/sandbox-manager.ts
Files:
src/e2b/sandbox-manager.ts
src/types.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Export type definitions from types.ts for all TypeScript interfaces and types
Files:
src/types.ts
src/cli.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use Commander.js for CLI command definition and argument parsing in cli.ts
Files:
src/cli.ts
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/e2b/sandbox-manager.ts : Implement E2B sandbox lifecycle management in src/e2b/sandbox-manager.ts
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to tests/**/*.test.ts : Write unit and integration tests for all new features and bug fixes
Applied to files:
tests/e2b/templates.test.tstests/e2b/sandbox-manager.test.tsCLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/**/*.ts : Maintain >85% test coverage across all source files
Applied to files:
tests/e2b/templates.test.tsCLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to tests/**/*.test.ts : Use Vitest as the testing framework for unit and integration tests
Applied to files:
tests/e2b/templates.test.tsCLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/e2b/sandbox-manager.ts : Implement E2B sandbox lifecycle management in src/e2b/sandbox-manager.ts
Applied to files:
tests/e2b/sandbox-manager.test.tssrc/e2b/sandbox-manager.tssrc/types.tssrc/e2b/templates.tssrc/cli.tsCLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/cli.ts : Use Commander.js for CLI command definition and argument parsing in cli.ts
Applied to files:
src/cli.ts
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/e2b/claude-runner.ts : Implement autonomous Claude Code execution in src/e2b/claude-runner.ts
Applied to files:
src/cli.tsCLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/e2b/ssh-key-injector.ts : Implement SSH key injection for private repository access in src/e2b/ssh-key-injector.ts
Applied to files:
src/cli.ts
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Document all new CLI commands in the CLI Commands section of CLAUDE.md
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Update version history in CLAUDE.md when releasing new versions
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Document all new MCP tools in the MCP Server Tools section of CLAUDE.md
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/mcp/tools.ts : Implement tool logic in mcp/tools.ts with corresponding Zod schema validation
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/mcp/schemas.ts : Implement Zod schemas in mcp/schemas.ts for all MCP tool input/output validation
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/mcp/index.ts : Register all MCP tools in mcp/index.ts with proper error handling and input validation
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/hooks-installer.ts : Install shell hooks, aliases, and MCP configuration using hooks-installer.ts
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Test all E2E workflows locally before committing using npm test with --coverage flag
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Ensure all end-to-end integration tests pass and coverage remains >85% before merging
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/merge-strategies.ts : Implement merge conflict resolution strategies in merge-strategies.ts
Applied to files:
CLAUDE.md
🧬 Code graph analysis (4)
tests/e2b/templates.test.ts (2)
src/types.ts (1)
SandboxTemplate(584-591)src/e2b/templates.ts (3)
TemplateManager(134-534)validateTemplateName(60-72)validateTemplate(77-129)
src/e2b/sandbox-manager.ts (2)
src/types.ts (1)
SandboxTemplate(584-591)src/logger.ts (1)
error(140-157)
src/e2b/templates.ts (2)
src/types.ts (5)
SandboxTemplate(584-591)TemplateValidationResult(616-619)TemplateListEntry(596-601)TemplateOperationResult(606-611)ProjectTypeDetection(624-629)src/logger.ts (1)
error(140-157)
src/cli.ts (1)
src/e2b/templates.ts (2)
TemplateManager(134-534)validateTemplateName(60-72)
🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md
545-545: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Macroscope - Correctness Check
- GitHub Check: opencode-review
🔇 Additional comments (25)
templates/full-stack-nextjs.json (1)
10-20: LGTM!The environment variables and metadata are well-structured and appropriate for a Next.js development environment. Disabling telemetry and increasing memory allocation are sensible defaults.
src/e2b/sandbox-manager.ts (3)
21-30: LGTM!The
TemplateApplicationResultinterface is well-designed with appropriate fields for success status, message, counts, and error handling.
777-798: LGTM!The success logging and return value provide good observability. The outer catch block ensures unexpected errors are handled gracefully and logged appropriately.
743-775: LGTM on sequential command execution with proper error handling and automatic sensitive data redaction.The setup commands are executed sequentially with appropriate timeouts (5 minutes) and error handling. Non-zero exit codes are properly detected and reported with the failing command and stderr. The
logger.info()method appliesredactSensitive()to all logged messages, ensuring setup commands containing API keys, SSH keys, or other sensitive data are automatically redacted.templates/node-20-typescript.json (1)
9-18: LGTM!The environment variables and metadata are well-structured and appropriate for a Node.js/TypeScript development environment.
tests/e2b/sandbox-manager.test.ts (4)
905-911: LGTM!The
beforeEachproperly sets up thesandbox.commands.runmock for testing template application. The mock returns a successful result by default, which is appropriate for positive test cases.
913-929: LGTM!The test correctly verifies that setup commands are executed in order and that the success result is returned. The assertion for exactly 2 calls matches the template's 2 setup commands.
931-954: LGTM!The test correctly verifies that environment variables are exported via shell commands. The assertion properly checks for the presence of
export NODE_ENVin the command string.
956-1019: LGTM!The remaining tests provide comprehensive coverage:
- Error handling for nonexistent sandbox
- Command failure detection and error propagation
- Edge case of templates with no setup commands or environment variables
- Logging verification for observability
Good test coverage for the
applyTemplatemethod. As per coding guidelines, unit tests are written using Vitest for new features.templates/python-3.12-fastapi.json (1)
1-19: LGTM!This template is well-structured:
- Upgrading pip before installing packages is a best practice
- All dependencies mentioned in the description are installed
PYTHONUNBUFFERED=1ensures real-time output in containerized environmentsPYTHONDONTWRITEBYTECODE=1prevents.pycfile cluttersrc/types.ts (1)
560-629: Well-structured type definitions for the template system.The new types follow consistent patterns established elsewhere in the file. Key observations:
SandboxTemplatecorrectly marks optional fields (setupCommands,environment,metadata)TemplateOperationResultprovides proper success/failure handling with optional error detailsProjectTypeDetectionsupports both detected and undetected cases with appropriate optional fieldsAs per coding guidelines, these type definitions are properly exported from
types.tsfor use across the codebase.tests/e2b/templates.test.ts (4)
14-59: Test setup follows Vitest best practices.Good use of:
vi.mock('fs/promises')for mocking the fs modulevi.resetAllMocks()inbeforeEach/afterEachfor test isolation- A helper factory
createMockTemplate()for consistent test dataThe mock defaults (empty directories, access denied) provide a clean baseline that individual tests override as needed.
76-118: Good coverage of template loading edge cases.The tests properly verify:
- Successful template loading from directory
- Graceful handling of missing directories (ENOENT)
- Filtering of non-JSON files
- Graceful skipping of malformed JSON files
427-468: Thorough validation tests for template names.Good coverage of the
validateTemplateNamefunction including edge cases for length constraints, special characters, and reserved names. The test at line 436 correctly validates that dots are allowed (e.g.,python-3.12-fastapi).
573-631: Project type detection tests look correct.The tests cover the main detection scenarios (Node.js/TypeScript, Python FastAPI, Next.js, unknown).
Minor observation: Line 586 uses
mockResolvedValueinstead ofmockResolvedValueOnce, which means the same package.json content will be returned for any subsequentreadFilecalls in that test. This works for the current test but could cause subtle issues if the test logic changes.CLAUDE.md (2)
115-131: Documentation updates are comprehensive and well-organized.The source files section now properly documents:
src/e2b/templates.tsfor template managementtemplates/directory with built-in template JSON filesThis aligns with the PR's goal of adding sandbox template functionality. Based on learnings, this follows the pattern of documenting new CLI commands in CLAUDE.md.
351-356: CLI commands properly documented in the commands table.All six new template commands are documented with descriptions, maintaining consistency with the existing table format.
src/e2b/templates.ts (4)
24-55: ES module path resolution is correctly implemented.The use of
fileURLToPath(import.meta.url)andpath.dirnameis the correct pattern for ES modules. ThegetDefaultConfigfunction properly resolves the project root from the compiled location (dist/e2b/templates.js→ project root).The
TEMPLATE_NAME_REGEXcorrectly allows dots (e.g.,python-3.12-fastapi) while enforcing the 3-50 character limit.
158-191: Template loading implementation is solid.The method correctly:
- Filters to
.jsonfiles only- Validates templates before adding them
- Handles ENOENT gracefully while propagating other errors
Consider adding debug-level logging for skipped invalid templates to aid troubleshooting, but this is optional since silent skipping is the documented behavior.
283-314: Template creation logic is well-structured.The validation order (name → structure → uniqueness → write) is correct. Timestamps are properly added to metadata.
Minor note: The exists check (line 284) and file write (line 308) are not atomic, so concurrent
createTemplatecalls with the same name could both succeed. This is low risk for CLI usage but could be a consideration if this API is used programmatically in parallel.
468-533: Project type detection covers common use cases.The detection flow prioritizes Node.js projects (via
package.json) before checking Python projects. The FastAPI detection at line 479 uses simple string matching (content.includes('fastapi')), which could produce false positives if "fastapi" appears in comments, but this is acceptable for a heuristic suggestion system.The
detectedFilesarray provides useful context for why a particular template was suggested.src/cli.ts (4)
1435-1442: Template precedence logic is correctly implemented.The precedence chain
--use-template > --template > E2B_TEMPLATE env var > defaultis clear and correctly uses optional chaining to handle the null case when no managed template is provided.
2558-2612: Well-implementedtemplates-listcommand.The command follows established CLI patterns with:
- JSON output support
- Helpful grouping by template type (built-in vs custom)
- Informative message when no templates exist
- Consistent use of chalk for colorization
2710-2719: Environment variable parsing handles edge cases correctly.The parsing at lines 2714-2718 correctly handles environment values containing
=signs by joining the remaining parts:const [key, ...valueParts] = envVar.split('='); environment[key] = valueParts.join('=');This allows values like
DATABASE_URL=postgres://user:pass@host/dbto be parsed correctly.
2852-2906:templates-importcommand handles both file and stdin input.Good implementation that:
- Supports
--filefor file input or stdin for piped input- Validates that input is not empty before attempting import
- Provides clear error messages in both JSON and human-readable formats
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Changed templates CLI from hyphenated commands to proper subcommands: - templates-list → templates list - templates-show → templates show <name> - templates-create → templates create <name> - templates-delete → templates delete <name> - templates-export → templates export <name> - templates-import → templates import Added CLI naming convention note to CLAUDE.md explaining preference for subcommands over hyphenated command names.
Review SummaryThis PR adds a comprehensive E2B sandbox template management system with built-in templates for Node.js, Python, and Next.js workflows. The implementation is well-designed with excellent test coverage (914 tests passing). ✅ Strengths
🔴 Critical Issues (Must Fix Before Merge)1. Environment Variable Persistence (src/e2b/sandbox-manager.ts:732) Recommended Fix:
2. Shell Injection Risk (src/e2b/sandbox-manager.ts:741) Recommended Fix: 🟡 Medium Priority Issues3. Use Logger Instead of Console (src/cli.ts:1804-1825) Recommended Fix: Replace all console methods with /. 4. Template Description Mismatch (templates/node-20-typescript.json:8) Recommended Fix: Either remove Jest/Vitest from description or add vitest to setupCommands. 🟢 Low Priority / Nice to Have5. Template Loading Performance (src/e2b/templates.ts:158-191) 6. Project Type Detection (src/e2b/templates.ts:479-486) 📊 Test Coverage
🎯 RecommendationConditionally Approve - Address the two critical issues (env var persistence and shell injection) before merging. These are functional bugs that will impact user experience. The medium priority issues (logger usage, description accuracy) should also be fixed for consistency and documentation accuracy. The implementation is solid overall and the template system adds significant value to the parallel-cc project. With the critical issues resolved, this is ready for v1.1 release. |
|
Review comment posted to PR #48 at #48 (comment) |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/cli.ts`:
- Line 1399: The .option('--use-template <name>', ...) help text references the
old command name "templates-list"; update the description string for the
'--use-template' option to mention the new command name "templates list" (e.g.,
"Use managed template from templates list (runs setup commands and sets
environment)"), editing the .option('--use-template <name>') call to reflect
this exact wording change.
- Around line 1414-1433: The hint text for the missing template uses the old
subcommand name "templates-list"; update both occurrences where the template not
found message is emitted (the JSON error object and the human-readable branch)
to reference the new subcommand "templates list" instead; look for the block
that creates TemplateManager, checks options.useTemplate, sets managedTemplate,
and emits the error (the JSON output with hint and the chalk.dim hint line) and
replace "templates-list" with "templates list".
- Around line 2883-2894: When running templateManager.importTemplate, the CLI
prints JSON when options.json is true but doesn't set a non-zero exit code on
failure; update the CLI logic around templateManager.importTemplate and
options.json so that after console.log(JSON.stringify(result...)) you check if
result.success is false and call process.exit(1) (or otherwise set
process.exitCode = 1) so both JSON and human-readable branches exit non-zero on
import failure.
♻️ Duplicate comments (3)
src/cli.ts (3)
1802-1827: Use logger utility for internal warnings; console for user output.This was flagged in a previous review. The template application block at lines 1823-1824 uses
console.warn()for internal warnings. Per coding guidelines, uselogger.warn()for logging. However, lines 1804-1819 usingconsole.log()for user-facing output with chalk formatting appears consistent with the rest of cli.ts.📝 Suggested fix for warning messages
} else { // Template application failure is non-blocking - warn but continue - if (!options.json) { - console.warn(chalk.yellow(`⚠ Template application failed: ${templateResult.error}`)); - console.warn(chalk.dim(' Continuing without template setup')); - } + if (!options.json) { + logger.warn(`Template application failed: ${templateResult.error}`); + console.log(chalk.yellow(`⚠ Template application failed: ${templateResult.error}`)); + console.log(chalk.dim(' Continuing without template setup')); + } }
2798-2809:--jsonmode should exit non-zero when operation fails.This was flagged in a previous review. When
result.successis false, the JSON output is printed but the process exits with code 0, making it difficult for scripts to detect failures.📝 Suggested fix
if (options.json) { console.log(JSON.stringify(result, null, 2)); + if (!result.success) process.exit(1); } else {
2748-2757:--jsonmode should exit non-zero when creation fails.Same pattern issue - when
result.successis false, the process should exit with a non-zero code.📝 Suggested fix
if (options.json) { console.log(JSON.stringify(result, null, 2)); + if (!result.success) process.exit(1); } else {
🧹 Nitpick comments (1)
CLAUDE.md (1)
540-568: Add blank line before the table for markdown compliance.The static analysis tool flagged missing blank lines around the table at line 545.
📝 Suggested fix
Pre-configured development environments that automatically set up tools and dependencies: **Built-in Templates:** + | Template | Description | Setup | |----------|-------------|-------| | `node-20-typescript` | Node.js 20 with TypeScript tooling | TypeScript, ESLint, Prettier + npm install |
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CLAUDE.mdsrc/cli.ts
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.ts: Use TypeScript strict mode for all source files
Use ES modules (type: "module") for all TypeScript source files
Use async/await over callbacks for asynchronous operations in TypeScript source files
Implement explicit error handling for all async/await operations in TypeScript source files
Use meaningful and descriptive variable names throughout the codebase
Maintain >85% test coverage across all source files
Use better-sqlite3 via the SessionDB class in db.ts for all database operations
Validate all database inputs using db-validators.ts functions before database operations
Use the logger utility from logger.ts for all console output and logging
Wrap gtr CLI commands through GtrWrapper class in gtr.ts instead of direct subprocess calls
Automatically redact sensitive data (API keys, credentials, SSH keys) from all logs
Files:
src/cli.ts
src/cli.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use Commander.js for CLI command definition and argument parsing in cli.ts
Files:
src/cli.ts
🧠 Learnings (19)
📓 Common learnings
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/e2b/sandbox-manager.ts : Implement E2B sandbox lifecycle management in src/e2b/sandbox-manager.ts
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/e2b/sandbox-manager.ts : Implement E2B sandbox lifecycle management in src/e2b/sandbox-manager.ts
Applied to files:
src/cli.tsCLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/**/*.ts : Use the logger utility from logger.ts for all console output and logging
Applied to files:
src/cli.ts
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/e2b/claude-runner.ts : Implement autonomous Claude Code execution in src/e2b/claude-runner.ts
Applied to files:
src/cli.tsCLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/e2b/ssh-key-injector.ts : Implement SSH key injection for private repository access in src/e2b/ssh-key-injector.ts
Applied to files:
src/cli.ts
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/cli.ts : Use Commander.js for CLI command definition and argument parsing in cli.ts
Applied to files:
src/cli.ts
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Document all new CLI commands in the CLI Commands section of CLAUDE.md
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Update version history in CLAUDE.md when releasing new versions
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Document all new MCP tools in the MCP Server Tools section of CLAUDE.md
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/mcp/tools.ts : Implement tool logic in mcp/tools.ts with corresponding Zod schema validation
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/mcp/schemas.ts : Implement Zod schemas in mcp/schemas.ts for all MCP tool input/output validation
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/mcp/index.ts : Register all MCP tools in mcp/index.ts with proper error handling and input validation
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/hooks-installer.ts : Install shell hooks, aliases, and MCP configuration using hooks-installer.ts
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to tests/**/*.test.ts : Use Vitest as the testing framework for unit and integration tests
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to tests/**/*.test.ts : Write unit and integration tests for all new features and bug fixes
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/**/*.ts : Maintain >85% test coverage across all source files
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Test all E2E workflows locally before committing using npm test with --coverage flag
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Ensure all end-to-end integration tests pass and coverage remains >85% before merging
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/merge-strategies.ts : Implement merge conflict resolution strategies in merge-strategies.ts
Applied to files:
CLAUDE.md
🧬 Code graph analysis (1)
src/cli.ts (1)
src/e2b/templates.ts (2)
TemplateManager(134-534)validateTemplateName(60-72)
🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md
545-545: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: opencode-review
- GitHub Check: Macroscope - Correctness Check
🔇 Additional comments (10)
CLAUDE.md (3)
115-131: LGTM!The source file structure documentation accurately reflects the new e2b/templates.ts module and the built-in templates directory structure.
351-356: LGTM!The CLI commands table correctly documents the new template subcommands with proper descriptions matching the v1.1 implementation.
945-951: LGTM!The CLI naming convention note is helpful for maintaining consistency. It clarifies when to use subcommands vs hyphenated names, which aligns with the v1.1 refactor from
templates-listtotemplates list.src/cli.ts (7)
36-36: LGTM!The import correctly includes all required exports from the templates module.
2554-2560: LGTM!The templates subcommand group is properly structured using Commander.js's
.command()pattern for hierarchical commands.
2562-2615: LGTM!The
templates listcommand implementation is clean with proper error handling and consistent JSON/human-readable output patterns.
2617-2682: LGTM!The
templates showcommand provides comprehensive template details with proper null checks for optional fields like metadata, setupCommands, and environment variables.
2684-2767: LGTM!The
templates createcommand properly validates template names, parses environment variables (correctly handling values containing=), and supports project-type auto-detection via--from-repo.
2821-2849: LGTM!The
templates exportcommand correctly outputs JSON directly (no--jsonflag needed) and supports writing to a file via--output.
1435-1439: LGTM!The template precedence chain (
--use-template>--template>E2B_TEMPLATEenv var > default) is correctly implemented and well-documented in the comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Critical fixes: - Pass environment variables via E2B SDK envs parameter instead of shell exports, ensuring persistence across commands and preventing shell injection - Remove shell export logic that didn't persist between commands.run() calls Medium priority fixes: - Replace console methods with logger in template application output - Fix template description in node-20-typescript.json (remove Jest/Vitest claim) - Update invalid name error message to include dots in allowed characters - Check template existence before prompting for delete confirmation - Update CLI help text and hints to reference 'templates list' subcommand - Add exit code on JSON import failure Test updates: - Update sandbox-manager test to verify envs parameter usage instead of shell exports
Review: feat(e2b): add sandbox templates for common workflows (v1.1)This PR introduces a comprehensive template management system with well-architected code and excellent test coverage. However, there are several issues identified in previous reviews that need attention before merging. Critical Issues (Must Fix)1. Environment Variable Persistence (src/e2b/sandbox-manager.ts:736) 2. Shell Injection Risk (src/e2b/sandbox-manager.ts:714) Medium Priority Issues3. Logger Usage (src/cli.ts:1804-1825) 4. JSON Exit Codes (src/cli.ts:2748, 2798, 2908) 5. Template Description Mismatch (templates/node-20-typescript.json:8) Minor Issues6. Outdated Command References (src/cli.ts:1399, 1425, 1429) 7. Markdown Formatting (CLAUDE.md:545) Strengths
RecommendationRequest Changes - Please address the two critical issues (env var persistence and shell injection) before merging. These are functional bugs that will impact user experience and security. The medium priority issues should also be fixed for consistency. The implementation is solid overall and the template system adds significant value to the project. With critical issues resolved, this is ready for v1.1 release. |
|
Review comment posted successfully: #48 (comment) |
- Revert logger.info/warn back to console.log/warn for template output (logger.info is hidden at default LogLevel.WARN, making user feedback invisible) - Add process.exit(1) for create/delete commands in JSON mode on failure (consistent with import command, enables scripts to detect errors)
Review: feat(e2b): add sandbox templates for common workflows (v1.1)Excellent work on the template management system! The implementation is well-designed with solid architecture. Most critical issues from previous reviews have been addressed. ✅ StrengthsArchitecture & Design
Critical Fixes Applied
🟡 Minor Issues Remaining1. Markdown formatting (Low Priority) 2. Console vs Logger usage (Informational) 🎯 Overall AssessmentThe template system is production-ready for v1.1. The implementation demonstrates:
The two critical functional bugs (env var persistence and shell injection) have been properly fixed using the correct E2B SDK patterns. The remaining issues are minor documentation formatting points. Recommendation: ✅ Approve with optional follow-up
Great work on this feature! |
|
Review comment posted to PR #48 at #48 (comment) |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli.ts (1)
1399-1439: Move template loading and SandboxManager initialization inside the try block.Template loading (line 1419) runs before the try/catch and if
await templateManager.getTemplate()throws, the finally block containingcoordinator.close()(line 2135) won't execute, causing a coordinator resource leak. Move lines 1417–1440 inside the try block to ensure proper cleanup.Fix
.action(async (options) => { const coordinator = new Coordinator(); const templateManager = new TemplateManager(); let managedTemplate: import('./types.js').SandboxTemplate | null = null; - // Load managed template if --use-template is specified - if (options.useTemplate) { - managedTemplate = await templateManager.getTemplate(options.useTemplate); - if (!managedTemplate) { - if (options.json) { - console.log(JSON.stringify({ - success: false, - error: `Template "${options.useTemplate}" not found`, - hint: 'Run "parallel-cc templates list" to see available templates' - })); - } else { - console.error(chalk.red(`✗ Template "${options.useTemplate}" not found`)); - console.log(chalk.dim('Run "parallel-cc templates list" to see available templates')); - } - process.exit(1); - } - } - - // Precedence: --use-template > --template > E2B_TEMPLATE env var > default - const sandboxImage = managedTemplate?.e2bTemplate || - options.template || - (process.env.E2B_TEMPLATE?.trim() || '') || - 'anthropic-claude-code'; - const sandboxManager = new SandboxManager(logger, { - sandboxImage - }); - let sandboxId: string | null = null; - try { + // Load managed template if --use-template is specified + if (options.useTemplate) { + managedTemplate = await templateManager.getTemplate(options.useTemplate); + if (!managedTemplate) { + if (options.json) { + console.log(JSON.stringify({ + success: false, + error: `Template "${options.useTemplate}" not found`, + hint: 'Run "parallel-cc templates list" to see available templates' + })); + } else { + console.error(chalk.red(`✗ Template "${options.useTemplate}" not found`)); + console.log(chalk.dim('Run "parallel-cc templates list" to see available templates')); + } + process.exit(1); + } + } + + // Precedence: --use-template > --template > E2B_TEMPLATE env var > default + const sandboxImage = managedTemplate?.e2bTemplate || + options.template || + (process.env.E2B_TEMPLATE?.trim() || '') || + 'anthropic-claude-code'; + const sandboxManager = new SandboxManager(logger, { sandboxImage }); + let sandboxId: string | null = null; + // Check schema version - E2B features require v1.0.0 migration const db = coordinator['db']; const currentVersion = db.getSchemaVersion();
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/cli.ts
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.ts: Use TypeScript strict mode for all source files
Use ES modules (type: "module") for all TypeScript source files
Use async/await over callbacks for asynchronous operations in TypeScript source files
Implement explicit error handling for all async/await operations in TypeScript source files
Use meaningful and descriptive variable names throughout the codebase
Maintain >85% test coverage across all source files
Use better-sqlite3 via the SessionDB class in db.ts for all database operations
Validate all database inputs using db-validators.ts functions before database operations
Use the logger utility from logger.ts for all console output and logging
Wrap gtr CLI commands through GtrWrapper class in gtr.ts instead of direct subprocess calls
Automatically redact sensitive data (API keys, credentials, SSH keys) from all logs
Files:
src/cli.ts
src/cli.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use Commander.js for CLI command definition and argument parsing in cli.ts
Files:
src/cli.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/e2b/sandbox-manager.ts : Implement E2B sandbox lifecycle management in src/e2b/sandbox-manager.ts
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/e2b/sandbox-manager.ts : Implement E2B sandbox lifecycle management in src/e2b/sandbox-manager.ts
Applied to files:
src/cli.ts
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/cli.ts : Use Commander.js for CLI command definition and argument parsing in cli.ts
Applied to files:
src/cli.ts
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/**/*.ts : Use the logger utility from logger.ts for all console output and logging
Applied to files:
src/cli.ts
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/e2b/claude-runner.ts : Implement autonomous Claude Code execution in src/e2b/claude-runner.ts
Applied to files:
src/cli.ts
📚 Learning: 2026-01-13T09:16:25.976Z
Learnt from: CR
Repo: frankbria/parallel-cc PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:16:25.976Z
Learning: Applies to src/e2b/ssh-key-injector.ts : Implement SSH key injection for private repository access in src/e2b/ssh-key-injector.ts
Applied to files:
src/cli.ts
🧬 Code graph analysis (1)
src/cli.ts (1)
src/e2b/templates.ts (2)
TemplateManager(134-534)validateTemplateName(60-72)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: opencode-review
- GitHub Check: Macroscope - Correctness Check
🔇 Additional comments (3)
src/cli.ts (3)
36-36: Guard against import/export contract drift for templates module.
This new import is a CLI hard dependency; if./e2b/templates.jsthrows on import (e.g., path resolution / fs access at module init), it will fail CLI startup globally. Ensure the templates module doesn’t do eager filesystem work at import-time (keep it insideTemplateManagermethods).
1804-1825: Logging policy decision needed: console output vsloggeroutput.
This PR usesconsole.log/warnfor user-facing output (including template flows), while repo guidelines say “use logger utility from logger.ts for all console output”. Given the known UX issue wherelogger.infois hidden at default WARN, either (a) codify “CLI uses console; internal libs use logger” in guidelines, or (b) adjust logger to have a dedicated always-on “ui” channel.
As per coding guidelines, please confirm the intended standard and align accordingly.Also applies to: 2558-2605
1802-1827: No changes required—template application is already properly non-blocking.The
applyTemplate()method is comprehensively designed to never throw exceptions. It wraps its entire execution in a try-catch block (outer) and all error paths—whether from command failures or unexpected errors—return a{success: false, error: message}result object rather than throwing. The calling code in cli.ts correctly handles this by checkingtemplateResult.successand logging warnings without aborting execution. The current implementation already fulfills the non-blocking requirement.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
1. Move template loading inside try block (sandbox-run command) - Prevents coordinator resource leak if getTemplate() throws - SandboxManager declared outside try, initialized inside 2. Redact sensitive env var values in templates show - Pattern: /token|key|secret|password|credential|auth/i - Redacted by default, use --show-secrets to reveal - Applies to both human-readable and JSON output - Shows hint when sensitive values are hidden 3. Fail fast for malformed --env entries (templates create) - Validates KEY=value format - Rejects missing '=' or empty key - Provides clear error messages in both human and JSON modes
Review SummaryExcellent work on the template management system! All critical issues from previous reviews have been properly addressed in the recent commits (21fa5a4, 41fc4a2, 35400f7). ✅ Critical Issues - All Resolved1. Environment Variable Persistence (src/e2b/sandbox-manager.ts:712-717)
2. Shell Injection Risk (src/e2b/sandbox-manager.ts:711-728)
3. Template Description Mismatch (templates/node-20-typescript.json:3)
4. JSON Exit Codes (src/cli.ts:2750-2752, 2816-2818, 2904-2906)
5. Command References (src/cli.ts:1399, 1425, 1429)
6. Resource Leak (src/cli.ts:1417-1440)
7. Secrets Redaction (src/cli.ts:2652-2684)
📊 Code QualityArchitecture:
Validation:
Testing:
|
|
Perfect! The review has been posted successfully. SummaryI've reviewed PR #48 and posted a comprehensive review comment. The review finds that all critical issues from previous reviews have been properly addressed in the recent commits: ✅ All Critical Issues Resolved:
📊 Overall Assessment:
|
| // Read from stdin | ||
| const chunks: string[] = []; | ||
| for await (const chunk of process.stdin) { | ||
| chunks.push(chunk.toString()); | ||
| } | ||
| jsonInput = chunks.join(''); | ||
| } |
There was a problem hiding this comment.
templates import silently waits on stdin when run in a TTY, which looks hung. Consider checking process.stdin.isTTY and immediately show a helpful message (or JSON) and exit, guiding users to use --file or pipe input.
| // Read from stdin | |
| const chunks: string[] = []; | |
| for await (const chunk of process.stdin) { | |
| chunks.push(chunk.toString()); | |
| } | |
| jsonInput = chunks.join(''); | |
| } | |
| // Read from stdin | |
| if (process.stdin.isTTY) { | |
| if (options.json) { | |
| console.log(JSON.stringify({ success: false, error: 'No input provided; use --file or pipe JSON to stdin' })); | |
| } else { | |
| console.error(chalk.red('✗ No input provided')); | |
| console.log(chalk.dim('Provide JSON via --file or pipe to stdin')); | |
| } | |
| process.exit(1); | |
| } | |
| const chunks: string[] = []; | |
| for await (const chunk of process.stdin) { | |
| chunks.push(chunk.toString()); | |
| } | |
| jsonInput = chunks.join(''); |
🚀 Want me to fix this? Reply ex: "fix it for me".
1 participant
Summary
Add a comprehensive template management system for E2B sandboxes that enables pre-configured development environments for common workflows.
Implementation Details
New Files
src/e2b/templates.ts- TemplateManager with validation, loading, and project detectiontemplates/*.json- Built-in template definitionstests/e2b/templates.test.ts- 50 comprehensive testsModified Files
src/types.ts- Template type definitions (SandboxTemplate, TemplateListEntry, etc.)src/e2b/sandbox-manager.ts- applyTemplate() method for executing setup commandssrc/cli.ts- Template CLI commands and --use-template support for sandbox-runCLAUDE.md- Documentation updatesKey Design Decisions
templates/directory (version controlled)~/.parallel-cc/templates/(user-specific)Test Plan
templates-list,templates-show node-20-typescriptsandbox-run --use-template node-20-typescript(requires E2B API key)Usage Examples
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.