Fix/recover extraction path#32071
Conversation
…inux-docs Fix/cli api key validation and linux docs
The tar extraction target in `recover` was hardcoded to homedir(), which is wrong for any instance whose instanceDir differs from \~/. When a named instance (instanceDir !== homedir()) was retired, its data was archived under $(retiredDir)/$(name).tar.gz.staging. Extracting with `-C homedir()` would silently unpack those files into the home directory instead of the original location, making the restore unusable. Fix: mirror the logic in retireLocal — - named instance → targetDir = instanceDir - default instance → targetDir = join(instanceDir, '.vellum') Extract with `-C retiredDir` (the directory where the archive lives), then rename the extracted staging entry to targetDir. Also create parent directories with mkdirSync so custom paths like /opt/vellum/… are recoverable even when the parent did not previously exist. Also improves the `--help` output with a description of where archives are stored and two concrete example invocations, per CLI AGENTS.md. Adds recover.test.ts covering: - help text output - missing name / missing archive / missing resources error paths - target-already-exists collision guard - correct extraction path for default instances (instanceDir === homedir()) - correct extraction path for named instances (instanceDir !== homedir()) - parent directory creation for deep custom paths
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e22308176
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const targetDir = isNamedInstance | ||
| ? entry.resources.instanceDir | ||
| : join(entry.resources.instanceDir, ".vellum"); |
There was a problem hiding this comment.
Check named-instance restore path before moving staging dir
When instanceDir !== homedir(), recovery now restores into entry.resources.instanceDir, but the preflight collision check still validates only instanceDir/.vellum. In that case, if instanceDir already exists (for example as a leftover directory) but .vellum does not, the command passes validation and then renameSync(extractedPath, targetDir) fails with EEXIST/ENOTEMPTY after extraction, leaving recovery half-complete and requiring manual cleanup.
Useful? React with 👍 / 👎.
dvargasfuertes
left a comment
There was a problem hiding this comment.
thanks for the contribution!
|
@dvargasfuertes you are welcome |
Prompt / plan
Test plan
CLI verb checklist