Add error handling for runInit in container entrypoint#35
Conversation
| try { | ||
| await runInit(); | ||
| } catch (error) { | ||
| console.log(`[entrypoint] Initialization failed (non-fatal): ${(error as Error).message}`); | ||
| console.log("[entrypoint] SSH will still start - connect to debug the issue"); | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Addresses Sentry review feedback on PR #34. If runInit() throws (e.g., git clone fails), the container would crash and restart infinitely, preventing SSH access for debugging. Now initialization failures are caught and logged as non-fatal, allowing the SSH daemon to start so users can connect and debug. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
f7b1b2a to
cd9d48f
Compare
| const repoName = repoUrl.replace(/\/+$/, "").split("/").pop() ?? "repo"; | ||
| const cleanRepoName = repoName.replace(/\.git$/, ""); | ||
| const repoPath = path.join(workspaceHome, cleanRepoName); | ||
| if (await pathExists(repoPath)) { | ||
| console.log(`Repository directory '${cleanRepoName}' already exists. Skipping clone.`); | ||
| await configureGitSshKey(workspaceHome, repoPath, runtimeConfig?.ssh?.selectedKey ?? ""); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Bug: A new check for an existing repository directory causes an infinite initialization loop if a post-clone step fails, preventing the workspace from ever successfully starting up.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
A new check for an existing repository directory can lead to an infinite initialization loop. If a post-clone step, such as runBootstrapScripts(), fails, the .workspace-initialized marker file is not created. On subsequent restarts, the new check at lines 86-93 will find the existing repository directory and cause cloneRepository() to return early. The initialization process will then re-run the subsequent steps, which will fail again at the same point. This leaves the workspace in a permanently broken state, unable to complete initialization without manual user intervention to remove the repository directory.
💡 Suggested Fix
The logic should be adjusted to handle this semi-initialized state. Instead of returning early if the repository directory exists, consider either deleting the directory to allow a fresh clone or making the subsequent initialization steps more robust to this condition. The previous behavior, which would fail on the git clone command itself, was more explicit.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: perry/internal/src/commands/init.ts#L86-L93
Potential issue: A new check for an existing repository directory can lead to an
infinite initialization loop. If a post-clone step, such as `runBootstrapScripts()`,
fails, the `.workspace-initialized` marker file is not created. On subsequent restarts,
the new check at lines 86-93 will find the existing repository directory and cause
`cloneRepository()` to return early. The initialization process will then re-run the
subsequent steps, which will fail again at the same point. This leaves the workspace in
a permanently broken state, unable to complete initialization without manual user
intervention to remove the repository directory.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8300857
|
Re: Sentry's second comment about "infinite initialization loop": The current behavior is intentional graceful degradation:
This is better than the alternatives:
The workspace isn't "broken" - it's in a recoverable state where users can SSH in to diagnose and fix the bootstrap issue. |
Summary
Addresses Sentry review feedback from PR #34.
The
runInit()call in the container entrypoint lacked error handling. If initialization failed (e.g., git clone failure, bootstrap script error), the container would crash and restart infinitely due to theunless-stoppedrestart policy, preventing SSH access for debugging.Changes
runInit()in try/catch blockTest plan
bun run checkpasses🤖 Generated with Claude Code