Start dev server in container after copying repo #1375
Conversation
…\n\n- Add startDevServerInContainer helper to run install and dev in background based on lockfiles\n- Update copy-repo API route to trigger dev server startup post-copy\n\nThis enables previewing the Next.js app via HMR as soon as the code is copied into the container.
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the 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. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| { error: `Copied repo but failed to start dev server: ${err}` }, | ||
| { status: 202 } | ||
| ) | ||
| } |
There was a problem hiding this comment.
Bug: Exit code ignored, failures return success incorrectly
The try-catch wrapping startDevServerInContainer won't catch script execution failures because execInContainerWithDockerode returns { stdout, stderr, exitCode } instead of throwing exceptions on non-zero exit codes. The PR description states this endpoint should return HTTP 202 when dev server startup fails, but the returned exitCode is never checked. The endpoint will return HTTP 200 with { success: true } even when the shell script fails. Other callers of execInContainerWithDockerode in the codebase properly check exitCode === 0 to determine success.
| // - backgrounds the process with nohup and stores logs/pid | ||
| const script = [ | ||
| "set -e", | ||
| `cd ${mountPath}`, |
There was a problem hiding this comment.
Bug: Unquoted shell path breaks on paths with spaces
The mountPath parameter is interpolated directly into the shell command without quotes in cd ${mountPath}. While currently hardcoded to /workspace, the function interface accepts any string. If mountPath contained spaces (e.g., /my workspace), the cd command would receive multiple arguments and fail. Paths containing shell metacharacters like semicolons could also cause unexpected behavior. The path should be quoted like cd "${mountPath}" to handle paths safely.
Additional Locations (1)
| // Start dev in background after install completes | ||
| 'nohup sh -c "$INSTALL && "$PM" run -s dev" > .dev.log 2>&1 & echo $! > .dev.pid', | ||
| 'echo "Dev server starting with $PM. Logs: .dev.log, PID: $(cat .dev.pid)"', | ||
| ].join(" && ") |
There was a problem hiding this comment.
Bug: Broken quoting prevents dev server start
The nohup sh -c ... command string has mismatched quotes around $INSTALL and $PM, which makes the shell command syntactically invalid. This likely causes the nohup launch to fail, so the dev server never starts and .dev.pid/.dev.log may not reflect a running process.
Summary
Why
Implementation details
Notes
Checks
Closes #1374
Note
Automatically starts a background dev server in the container after copying a repo, with package manager auto-detect, install, and logging.
POST /api/playground/copy-repoto callstartDevServerInContaineraftercopyRepoToExistingContainerand return202if dev start fails (copy still succeeds).startDevServerInContainerinlib/utils/container.tsandshared/src/lib/utils/container.ts:pnpm/yarn/npmvia lockfiles.devin background withnohup..dev.logand PID to.dev.pidundermountPath(default/workspace).Written by Cursor Bugbot for commit 9b162c1. This will update automatically on new commits. Configure here.