-
Notifications
You must be signed in to change notification settings - Fork 77
fix: fail local hatch if base data directory already exists #9322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -538,6 +538,16 @@ async function hatchLocal(species: Species, name: string | null, daemonOnly: boo | |
| } | ||
| } | ||
|
|
||
| const baseDataDir = join(process.env.BASE_DATA_DIR?.trim() || (process.env.HOME ?? userInfo().homedir), ".vellum"); | ||
|
|
||
| if (existsSync(baseDataDir)) { | ||
| throw new Error( | ||
| `Base data directory already exists: ${baseDataDir}\n` + | ||
| " Another assistant may already be using this directory.\n" + | ||
| " To use a different directory, set the BASE_DATA_DIR environment variable.", | ||
| ); | ||
| } | ||
|
Comment on lines
+543
to
+549
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 existsSync check on baseDataDir blocks re-hatching after any failed hatch or incomplete cleanup The new Detailed failure scenarios and root causeScenario 1: Failed hatch recovery is impossible If a previous Scenario 2: Contradicts stale cleanup code above The stale process cleanup at lines 529-539 explicitly handles the case where Scenario 3: Desktop daemon creates the directory In desktop mode, Impact: Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| console.log(`🥚 Hatching local assistant: ${instanceName}`); | ||
| console.log(` Species: ${species}`); | ||
| console.log(""); | ||
|
|
@@ -555,8 +565,6 @@ async function hatchLocal(species: Species, name: string | null, daemonOnly: boo | |
| throw error; | ||
| } | ||
|
|
||
| const baseDataDir = join(process.env.BASE_DATA_DIR?.trim() || (process.env.HOME ?? userInfo().homedir), ".vellum"); | ||
|
|
||
| // Read the bearer token written by the daemon so the client can authenticate | ||
| // with the gateway (which requires auth by default). | ||
| let bearerToken: string | undefined; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check fails
hatch localwhenever~/.vellumalready exists, but that directory is also created by unrelated setup paths (for examplevellum config setcreates~/.vellum/workspace/config.jsonincli/src/commands/config.ts), so users can be blocked from hatching even when no local assistant is running. In practice, a documented pre-hatch config step now causes a hard failure, so the guard should detect active/conflicting local runtime state rather than treating any preexisting directory as an overwrite risk.Useful? React with 👍 / 👎.