Conversation
Adds a disaster-recovery CLI for the HybridClaw runtime home: - `hybridclaw backup [--output <path>]` writes a timestamped zip of `~/.hybridclaw` (or `$HYBRIDCLAW_DATA_DIR`). SQLite databases are snapshotted via the SQLite backup API so WAL-mode files produce a consistent copy. Ephemeral state (WAL/SHM sidecars, PID files, cache/, container-image-state/, evals/, migration-backups/, node_modules, .git) is excluded. - `hybridclaw backup restore <archive.zip> [--force]` validates the embedded manifest plus `config.json` / `credentials.json` markers, stages the extraction in a sibling temp dir, and atomically swaps the runtime home into place (with rollback on failure). Prompts before overwriting an existing installation; `--force` skips the prompt for scripted restores. Help text, the commands reference doc, and the CLI dispatcher in `src/cli.ts` are wired up. The handler imports `./help.js` lazily so tests that exercise `createBackupArchive` / `restoreBackupArchive` do not transitively trigger `runtime-config` schema migration on the test fixture. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds first-class CLI support for backing up and restoring the HybridClaw runtime home (~/.hybridclaw) in a way that avoids inconsistent SQLite WAL copies, and documents/tests the new workflow.
Changes:
- Introduces
hybridclaw backupandhybridclaw backup restoreCLI commands (zip-based archive with SQLite snapshots). - Wires the new command into CLI dispatch and
hybridclaw help, plus adds reference documentation. - Adds unit tests covering archive layout, exclusions, WAL-safe SQLite snapshot/restore, and restore overwrite confirmation behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/cli/backup-command.ts |
Implements backup/restore logic (file collection, SQLite snapshotting, zip writing, safe staged restore). |
src/cli.ts |
Registers backup in the main CLI dispatcher. |
src/cli/help.ts |
Adds backup to main usage, help topics, and a dedicated help page. |
docs/content/reference/commands.md |
Documents the new backup/restore commands and behavior. |
tests/unit/backup-command.test.ts |
Adds unit tests for zip layout, exclusions, WAL-safe SQLite snapshot round-trip, and restore conflicts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function openBackupZip(archivePath: string): Promise<yauzl.ZipFile> { | ||
| return new Promise((resolve, reject) => { | ||
| yauzl.open( | ||
| archivePath, | ||
| { lazyEntries: true, autoClose: false }, | ||
| (error, zipFile) => { | ||
| if (error) return reject(error); | ||
| if (!zipFile) { | ||
| return reject( | ||
| new Error(`Failed to open backup archive at ${archivePath}.`), | ||
| ); | ||
| } | ||
| resolve(zipFile); | ||
| }, | ||
| ); | ||
| }); | ||
| } | ||
|
|
||
| function readBackupManifestText(zipFile: yauzl.ZipFile): Promise<string> { | ||
| return new Promise<string>((resolve, reject) => { | ||
| let settled = false; | ||
| let found = false; | ||
|
|
||
| const finish = (value: string) => { | ||
| if (settled) return; | ||
| settled = true; | ||
| resolve(value); | ||
| }; | ||
| const fail = (error: unknown) => { | ||
| if (settled) return; | ||
| settled = true; | ||
| reject(error); | ||
| }; | ||
|
|
||
| zipFile.on('error', fail); | ||
| zipFile.on('end', () => { | ||
| if (!found) { | ||
| fail( | ||
| new Error( | ||
| `Archive is missing ${BACKUP_MANIFEST_FILE}; not a HybridClaw backup.`, | ||
| ), | ||
| ); | ||
| } | ||
| }); | ||
| zipFile.on('entry', (entry: yauzl.Entry) => { | ||
| if (entry.fileName !== BACKUP_MANIFEST_FILE) { | ||
| zipFile.readEntry(); | ||
| return; | ||
| } | ||
| found = true; | ||
| zipFile.openReadStream(entry, (error, stream) => { | ||
| if (error || !stream) { | ||
| fail(error ?? new Error('Failed to read backup manifest.')); | ||
| return; | ||
| } | ||
| const chunks: Buffer[] = []; | ||
| stream.on('data', (chunk: Buffer | string) => { | ||
| chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); | ||
| }); | ||
| stream.on('error', fail); | ||
| stream.on('end', () => { | ||
| try { | ||
| zipFile.close(); | ||
| } catch { | ||
| // best effort | ||
| } | ||
| finish(Buffer.concat(chunks).toString('utf-8')); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| zipFile.readEntry(); | ||
| }); |
There was a problem hiding this comment.
openBackupZip() uses autoClose: false, but readBackupManifestText() doesn’t reliably close the yauzl.ZipFile on failure paths (e.g., archive missing the manifest, openReadStream error). This can leak file descriptors and keep the archive handle open. Ensure the zip file is closed in fail() and in the end handler when the manifest isn’t found (best-effort zipFile.close() in a finally/try-catch), or switch to autoClose: true and still explicitly close on early success.
| const existingEntries = targetExists | ||
| ? fs.readdirSync(targetDir).filter((name) => !name.startsWith('.tmp-')) | ||
| .length | ||
| : 0; |
There was a problem hiding this comment.
When targetDir exists, existingEntries is computed via fs.readdirSync(targetDir), but there’s no check that targetDir is actually a directory. If a file exists at that path (or it’s a broken symlink), this will throw an ENOTDIR/ENOENT with an unhelpful message. Add an explicit statSync/lstatSync check and throw a clear error if the restore target exists but isn’t a directory.
| const existingEntries = targetExists | |
| ? fs.readdirSync(targetDir).filter((name) => !name.startsWith('.tmp-')) | |
| .length | |
| : 0; | |
| let existingEntries = 0; | |
| if (targetExists) { | |
| const targetStats = fs.lstatSync(targetDir); | |
| if (targetStats.isSymbolicLink()) { | |
| let resolvedTargetStats: fs.Stats; | |
| try { | |
| resolvedTargetStats = fs.statSync(targetDir); | |
| } catch { | |
| throw new Error( | |
| `Restore target exists but is a broken symlink, not a directory: ${targetDir}`, | |
| ); | |
| } | |
| if (!resolvedTargetStats.isDirectory()) { | |
| throw new Error( | |
| `Restore target exists but is not a directory: ${targetDir}`, | |
| ); | |
| } | |
| } else if (!targetStats.isDirectory()) { | |
| throw new Error( | |
| `Restore target exists but is not a directory: ${targetDir}`, | |
| ); | |
| } | |
| existingEntries = fs | |
| .readdirSync(targetDir) | |
| .filter((name) => !name.startsWith('.tmp-')).length; | |
| } |
| if (settled) return; | ||
| settled = true; | ||
| output.destroy(); | ||
| fs.rmSync(outputPath, { force: true }); |
There was a problem hiding this comment.
In writeZipArchive() the fail() handler calls fs.rmSync(outputPath, { force: true }) without guarding for non-file targets. If outputPath points at an existing directory (or another non-removable entry), rmSync can throw and mask the original stream/zip error. Wrap the cleanup rmSync in a try/catch (best-effort cleanup) or check with statSync/lstatSync before removing so the original failure is preserved.
| fs.rmSync(outputPath, { force: true }); | |
| try { | |
| fs.rmSync(outputPath, { force: true }); | |
| } catch { | |
| // Best-effort cleanup: preserve the original archive/stream failure. | |
| } |
…idation Address Copilot review feedback on PR #428: - writeZipArchive(): wrap the failure-path `fs.rmSync(outputPath)` in try/catch so an unwritable/non-file output path can no longer mask the original archive/stream failure with a cleanup error. - readBackupManifestText(): close the underlying yauzl ZipFile in both the success and failure paths via a shared `closeQuietly()` helper. Previously a missing manifest, an `openReadStream` error, or a stream error would leave the file descriptor open because we opened the archive with `autoClose: false`. - restoreBackupArchive(): when the target path exists, explicitly lstat/stat it and throw a clear error if it is a regular file or a broken symlink instead of letting `fs.readdirSync` surface a generic ENOTDIR/ENOENT. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Summary
~/.hybridclaw), so disaster-recovery required ad-hoccp -Rplus the risk of copying a WAL-mode SQLite mid-write.hybridclaw backup/hybridclaw backup restorecommands, wired into the CLI dispatcher, help text, and reference docs, with unit-test coverage for the zip layout, SQLite WAL-safety, and conflict-handling on restore.Change Type
Linked Context
Validation
--force/confirmation, confirmed overwrite, and rejection of non-backup archives.Docs And Config Impact
docs/content/reference/commands.mdgains a Backup And Restore section.src/cli/help.tsadds abackuptopic tohybridclaw helpand lists the command in the main usage banner.Risk Notes
Yes— restore extracts a user-supplied zip into the runtime home. Mitigations: reusessafeExtractZipandresolveArchiveEntryDestinationfromsrc/agents/claw-security.ts(zip-slip safe), validates the embeddedhybridclaw-backup.jsonmanifest plus requiredconfig.json/credentials.jsonmarkers before any swap, and stages extraction in a sibling temp dir so a failure rolls back to the previous runtime home.No.Evidence
tests/unit/backup-command.test.ts(6 tests).Backup archive layout:
Excluded from backups: WAL/SHM sidecars,
*-journal,.tmp-*, PID files (gateway.pid,cron.pid),cache/,container-image-state/,evals/,migration-backups/,node_modules/,.git/,.DS_Store,Thumbs.db.🤖 Generated with Claude Code