fix(state): restore SQLite state files via staged atomic replace (#7312)#7344
Conversation
The Hermes gateway process creates runtime/state.db and kanban.db as the gateway user with no group-write bit (SQLite creates databases at 0644 & ~umask, so umask 0007 yields 0640). Snapshot restore runs as the sandbox user, whose group read covers backup but not the in-place sqlite3 backup write, so every restore failed with 'attempt to write a readonly database' and 'Failed files: runtime/state.db, kanban.db'. Validate the backup into a staged database the restoring user owns, then replace the target atomically like the copy strategy — the swap needs only directory write permission, which the group-writable setgid state directories grant. Drop the replaced database's stale WAL/SHM sidecars after the swap, and recommend a gateway restart after a Hermes restore so the running gateway reopens the restored databases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughSnapshot restore now stages SQLite databases for permissioned atomic replacement and removes stale WAL/SHM files. Successful Hermes restores conditionally print gateway restart guidance, with unit, integration, and documentation updates. ChangesSnapshot restore behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SnapshotRestore
participant SQLiteRestore
participant HermesHint
participant TargetDirectory
SnapshotRestore->>SQLiteRestore: restore SQLite state file
SQLiteRestore->>TargetDirectory: stage, chmod, and atomically replace database
SQLiteRestore->>TargetDirectory: remove stale WAL/SHM sidecars
SnapshotRestore->>HermesHint: provide restored files and agent
HermesHint-->>SnapshotRestore: print gateway restart hint for Hermes SQLite restore
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/state-file-restore-command.test.ts (1)
67-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer behavioral assertions over generated-shell text.
These substring and ordering checks lock the test to the current shell implementation. Keep coverage through command execution—like the integration test below—and assert readonly-target replacement, permissions, sidecar cleanup, and symlink refusal as outcomes instead.
As per path instructions, “Prefer observable outcomes through the public boundary over source-text, private-shape, or mock-call assertions.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/state-file-restore-command.test.ts` around lines 67 - 85, Replace the generated-shell substring and ordering assertions in the state-file restore test with command-execution coverage through the public restore boundary. Assert the observable outcomes: readonly-target replacement succeeds with the expected permissions, stale WAL/SHM sidecars are removed, and symlinked state parents or SQLite targets are refused; avoid inspecting shell text or implementation-specific command ordering.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/state-file-restore-command.test.ts`:
- Around line 67-85: Replace the generated-shell substring and ordering
assertions in the state-file restore test with command-execution coverage
through the public restore boundary. Assert the observable outcomes:
readonly-target replacement succeeds with the expected permissions, stale
WAL/SHM sidecars are removed, and symlinked state parents or SQLite targets are
refused; avoid inspecting shell text or implementation-specific command
ordering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 029e1fdf-6191-4cda-a122-40e72d380951
📒 Files selected for processing (5)
src/lib/actions/sandbox/snapshot-hermes-gateway-hint.test.tssrc/lib/actions/sandbox/snapshot-hermes-gateway-hint.tssrc/lib/actions/sandbox/snapshot.tssrc/lib/state/state-file-restore.tstest/state-file-restore-command.test.ts
|
🌿 Preview your docs: https://nvidia-preview-pr-7344.docs.buildwithfern.com/nemoclaw |
PR Review Advisor — InformationalAdvisor assessment: Informational / medium confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. Since last review: 0 prior items resolved · 0 still apply · 0 new items found E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 2 optional E2E recommendations
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit ea9a66d in the TypeScript / code-coverage/cliThe overall coverage in commit ea9a66d in the Show a code coverage summary of the most impacted files.
Updated |
…tore-sqlite-staged-replace # Conflicts: # src/lib/state/state-file-restore.ts
|
✨ Thanks for the fix. This restores snapshot restore for Hermes sandboxes and unblocks the cloned sandbox lifecycle tests. Related open issues: |
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
prekshivyas
left a comment
There was a problem hiding this comment.
Reviewed exact head ea9a66de2. The staged SQLite restore now fails closed, preserves the live database and sidecars on validation failure, atomically replaces valid restores, and limits the Hermes restart hint to restored manifest-declared SQLite state. Focused tests, broad CI, CodeRabbit, both advisors, and the authenticated exact-head E2E retry are green; no unresolved review threads remain.
<!-- markdownlint-disable MD041 --> ## Summary Adds the canonical pre-tag `## v0.0.95` release entry to `docs/changelog/2026-07-24.mdx`, before the existing v0.0.94 entry. The entry summarizes approved user-visible changes merged since v0.0.94 and excludes internal-only prerequisites. ## Changes - Adds the v0.0.95 summary and detailed bullets for gateway lifecycle, recovery, state transfer, inference compatibility, sandbox security, Discord policy, and E2E evidence. - Links each user-facing theme to the most specific published documentation. - Records the release entry in the shared native changelog used by the OpenClaw, Hermes, and Deep Agents guides. Source summary: - [#7246](#7246), [#7228](#7228), [#7267](#7267), [#7489](#7489), [#7509](#7509), [#7351](#7351), and [#7290](#7290) -> `docs/changelog/2026-07-24.mdx`: Gateway authority, forward teardown and retry, managed recovery, Hermes restart recovery, scoped uninstall, and orphan-aware backup behavior. - [#7344](#7344) and [#7416](#7416) -> `docs/changelog/2026-07-24.mdx`: Atomic SQLite restore and host download verification. - [#7476](#7476), [#7347](#7347), [#7281](#7281), [#7485](#7485), [#7491](#7491), and [#7422](#7422) -> `docs/changelog/2026-07-24.mdx`: Windows Ollama reuse, CDI fallback, bounded OpenRouter connection setup, Nemotron-3 request compatibility, and managed Deep Agents retry and provider-error behavior. - [#6884](#6884), [#7481](#7481), [#6878](#6878), [#7467](#7467), [#7502](#7502), [#7503](#7503), [#7504](#7504), and [#7486](#7486) -> `docs/changelog/2026-07-24.mdx`: Trusted base-image overrides, local rebuild images, runtime validation, config preservation, reviewed package updates, and fewer final-image payload layers. - [#7303](#7303) -> `docs/changelog/2026-07-24.mdx`: Scoped Discord application-command management. - [#7488](#7488), [#7465](#7465), [#7497](#7497), [#7464](#7464), [#7501](#7501), [#7494](#7494), and [#7493](#7493) -> `docs/changelog/2026-07-24.mdx`: Selected-test risk signals, retry cleanup, full root-image validation, direct-main Hermes setup, executed PR-gate evidence, nightly history, and runner wait reporting. - [#7447](#7447) is an internal pinned-runtime prerequisite and is intentionally excluded from canonical supported-integration documentation. - [#7370](#7370) adds maintainer-only advisory reconciliation tooling and does not change supported user behavior. - [#7495](#7495) updates existing documentation and does not add a new v0.0.95 behavior claim. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the dated changelog structure, heading uniqueness, and published links. - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `docs/changelog/2026-07-24.mdx`; writing rules, documentation style, factual release meaning, and published links reviewed at exact head `58b02f2bf`. - Agent: Codex documentation writer reviewer <!-- docs-review-head-sha: 58b02f2 --> <!-- docs-review-agents-blob-sha: 9c9b36d --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npx vitest run test/changelog-docs.test.ts` passed 6 tests. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — the build passed with 0 errors and 2 Fern warnings. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a new v0.0.95 changelog entry above v0.0.94. * Documented improved externally supervised gateway lifecycle ownership. * Improved snapshot restore reliability and SQLite state handling. * Tightened CLI `backup-all` behavior and host artifact verification. * Updated Windows onboarding guidance (including Ollama service reuse and CDI directory fallback). * Noted inference compatibility fixes, deeper agent failure classification, stricter base-image validation, updated Discord bot command permissions, and refined E2E release automation evidence handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Summary
nemoclaw <sandbox> snapshot restorefailed on every Hermes sandbox withFailed files: runtime/state.db, kanban.db, which breaks all QA tests that useuseClonedSandboxLifecycle(~40 sanity tasks). The restore now validates each SQLite backup into a staged database owned by the restoring user and replaces the live database atomically, so the restore succeeds and the restored databases keep working for the gateway user. Failed validation leaves the live database and its sidecars unchanged.Related Issue
Fixes #7312
Root cause
The Hermes gateway runs as the
gatewayuser withumask 0007and createsruntime/state.dbandkanban.dbat mode0640(SQLite creates database files at0644 & ~umask), ownergateway, groupsandbox. Snapshot restore runs over SSH as thesandboxuser: group read letssnapshot createsucceed, but the restore script opened the live database for an in-placesqlite3online backup write, which fails deterministically withsqlite3.OperationalError: attempt to write a readonly database. The trailingos.chmod(dst, 0o660)would also fail for a non-owner. Thecopystrategy never hit this because it stages a temp file and renames over the target, which needs only directory write permission.Reproduced mechanically in a container with the production ownership layout (
gateway:sandbox 0640database, setgid2770state directory, restore assandbox): the old command fails with the exact error; the staged replace succeeds, restores the expected content, and thegatewayuser can still open the restored database read-write afterward.Changes
src/lib/state/state-file-restore.ts: thesqlite_backuprestore command validates the backup into amktempstaged database in the destination's parent directory (keeping thePRAGMA quick_checkgate and both symlink guards), sets0660on the staged copy this user owns, renames it over the destination, and drops the replaced database's stale-wal/-shmsidecars. The Python step no longer chmods or writes the gateway-owned live file.src/lib/actions/sandbox/snapshot-hermes-gateway-hint.ts(+ co-located test): after a successful Hermes restore that actually restored a manifest-declaredsqlite_backupfile, printrun \nemoclaw gateway restart`— the running gateway holds the replaced inode open and serves pre-restore state until it reopens the database. Consumer:runSnapshotRestoresuccess path insnapshot.ts`.docs/manage-sandboxes/backup-restore.mdx: Hermes-variant note that a running gateway serves pre-restore state databases until it is restarted.test/state-file-restore-command.test.ts: executable Linux coverage proves successful replacement of a read-only database, mode0660, stale-sidecar removal, failure preservation of the live database and sidecars, and refusal of symlinked parents and targets.Type of Change
Quality Gates
docs/manage-sandboxes/backup-restore.mdxgains a Hermes-variant note about the post-restore gateway restartea9a66de2passed all nine security categories; independent review remains requested from @cv and @ericksoaDocumentation Writer Review
docs-updateddocs/manage-sandboxes/backup-restore.mdxaccurately limits the Hermes restart instruction to restores containing state databases. The CLI hint matches the manifest-declaredsqlite_backupcondition.npm run docspassed.DGX Station Hardware Evidence
Not applicable —
scripts/prepare-dgx-station-host.shis unchanged.Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm run test:changedpassed 89/89; focused snapshot tests passed 7/7; local restore-contract tests passed 3 with 4 Linux-only executions selected for CI;npm run typecheck:clipassednpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: not applicable to this narrow restore fix;npm run check:diffpassed the complete path-scoped gatenpm run docsbuilds without warnings (doc changes only) — passed with 0 errors and 2 existing Fern warningsSigned-off-by: Shawn Xie shaxie@nvidia.com
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
-wal/-shmsidecars and keep the database file permissions as intended.Documentation
Tests