Codex review: needs changes before merge. Reviewed July 6, 2026, 11:59 PM ET / 03:59 UTC.

Summary
The branch adds a local SQLite snapshot provider plus openclaw backup sqlite snapshot create|verify|restore|list CLI/docs/tests, sharing the existing backup SQLite VACUUM INTO path.

PR surface: Source +767, Tests +885, Docs +95, Other +27. Total +1774 across 16 files.

Reproducibility: not applicable. this is a new CLI/artifact feature rather than a current-main bug. The review used source inspection plus the PR body's after-fix Windows source-harness proof instead of a failing reproduction.

Review metrics: 2 noteworthy metrics.

• New backup CLI surface: 4 subcommands added under backup sqlite snapshot. Public backup commands are operator-facing and compatibility-sensitive because scripts and hosted workflows can copy them.
• Unrelated lockfile churn: 533 lockfile lines changed without package manifest changes. The PR changes transitive install inputs outside the snapshot feature, which needs security review before merge.

Stored data model
Persistent data-model change detected: database schema: packages/memory-host-sdk/src/host/sqlite-snapshot.ts, database schema: src/commands/snapshot.test.ts, database schema: src/snapshot/manifest.ts, database schema: src/snapshot/snapshot-provider.test.ts, database schema: src/snapshot/snapshot-provider.ts, serialized state: src/snapshot/manifest.ts, and 2 more. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: canonical
Canonical: #94805
Summary: This PR is the central implementation candidate for the SQLite snapshot artifact stack; broader backup/database issues and the stress harness overlap but do not supersede it.

Members:

• superseded: Add core snapshot provider proof #94694 - Earlier provider-proof slice is closed and folded into this PR's combined implementation.
• superseded: Add core snapshot CLI #94717 - Earlier top-level CLI slice is closed and superseded by this PR's nested backup sqlite snapshot surface.
• superseded: Add named snapshot database targets #94799 - Earlier named-target slice is closed and folded into this PR.
• adjacent_distinct: Add snapshot SQLite stress harness #94967 - Open stress harness stacked on this PR provides validation plumbing, not the core artifact command itself.
• partial_overlap: [Feature]: Define a database abstraction layer for a dual-backend (SQLite + Postgres) #94675 - The issue covers broader database abstraction and managed-service durability; this PR addresses only the SQLite artifact portion.
• 1 more in the report.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦞 diamond lobster
Patch quality: 🦪 silver shellfish
Result: blocked by patch quality or review findings.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• Restore pnpm-lock.yaml to the current base unless a deliberate manifest change requires it.
• Remove the top-level snapshot startup catalog entry or intentionally restore and document that root command.
• Get maintainer direction on whether the cleaned core command should land before or after the Scout/Lobster pilot.

Risk before merge

• [P1] Unrelated lockfile churn changes transitive install inputs and deprecation metadata on a security-owned file, so supply-chain review cannot be limited to the SQLite snapshot feature.
• [P1] The new snapshot artifacts can contain auth profiles, credentials-adjacent plugin state, sessions, and per-agent state, so merge needs explicit security and upgrade review around storage, restore, retention, and operator handling.
• [P1] The PR body says the Scout/Lobster pilot should decide whether the long-term surface is core, host-owned plugin, or both; landing the core command before that decision is a product-direction risk.
• [P1] The stacked stress harness remains a separate open/conflicting PR, so concurrent-write stress and broader operational proof are not part of this PR head.

Maintainer options:

1. Clean supply-chain and stale policy first (recommended)
   Restore pnpm-lock.yaml to the current base unless a matching manifest change is intentional, and remove the unused top-level snapshot startup catalog entry before another merge-readiness review.
2. Accept core artifact risk after review
   Maintainers may intentionally accept the new sensitive artifact contract once the lockfile is clean and they are satisfied with docs, restore behavior, and follow-up stress validation.
3. Pause until pilot resolves surface
   Keep the PR draft or close it if Scout/Lobster evidence shows the host-owned plugin path should remain the canonical implementation.

@clawsweeper automerge

Special instructions:
Restore pnpm-lock.yaml to the current base unless an intentional package manifest change requires it, remove the retired top-level `snapshot` entry from `src/cli/command-catalog.ts`, and keep the nested `backup sqlite snapshot` command surface and focused tests intact.

Next step before merge

• [P2] A repair worker can address the two concrete blockers, but maintainer product/security review is still required before merge.

Maintainer decision needed

• Question: Should OpenClaw core ship openclaw backup sqlite snapshot now, or wait for the Scout/Lobster pilot and RFC to settle whether the long-term surface belongs in core, a host-owned plugin, or both?
• Rationale: The PR adds a new sensitive backup artifact contract, and the PR body explicitly names the external pilot as the operational evidence checkpoint before greenlighting the core path.
• Likely owner: galiniliev — galiniliev is the current assignee and already reviewed the snapshot invariant on this PR, making them the best visible owner for the next product-direction call.
• Options:
  • Keep draft pending pilot (recommended): Hold the core command after mechanical cleanup until the pilot/RFC confirms that a built-in CLI artifact is the right durable surface.
  • Sponsor core command after cleanup: Accept the core command as the Phase 1 local artifact contract once lockfile and startup-policy blockers are fixed and security review is satisfied.
  • Move the surface to a host plugin: Close or redirect the core PR if the pilot shows the host-owned plugin path is sufficient without new core CLI surface.

Security
Needs attention: The snapshot code/docs handle sensitive state intentionally, but the unexplained root lockfile dependency churn is a concrete supply-chain blocker.

Review findings

• [P1] Remove the unrelated lockfile dependency churn — pnpm-lock.yaml:162
• [P3] Drop the retired snapshot startup policy — src/cli/command-catalog.ts:269

@clawsweeper re-review

I updated the PR body with packaged CLI proof for node openclaw.mjs snapshot create, snapshot verify, and snapshot restore, including redacted JSON output, restored DB readback, and the final artifact file list.

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Codex maintainer review: one blocker remains before this snapshot artifact contract should ship.

Findings:

• [P2] Record database.userVersion from the generated artifact, not the live source before VACUUM INTO — packages/memory-host-sdk/src/host/sqlite-snapshot.ts:23

  createVacuumedSqliteSnapshot() currently reads PRAGMA user_version from the source connection before loading sqlite-vec and before running VACUUM INTO at packages/memory-host-sdk/src/host/sqlite-snapshot.ts:23-27. The manifest is then written from that value in src/snapshot/local-repository.ts:55-69, while database.sqlite is the actual sync/restore artifact. Without wrapping those operations in a single read transaction or reading the metadata back from params.targetPath after VACUUM INTO, a migration or operator write that commits between the PRAGMA read and the snapshot statement can leave manifest.json describing a different schema version than the restored database. Please derive userVersion from the generated artifact, or otherwise tie the metadata read to the same SQLite snapshot used for database.sqlite, and add a regression that proves the manifest value matches the restored artifact.

Notes from independent review:

• I did not carry forward the earlier symlinked-global finding. An isolated Node/SQLite probe with a symlinked canonical path, active target -wal/-shm, and WAL-only rows preserved all rows when VACUUM INTO opened the symlink path, so I do not have reproduction-quality proof for that blocker.
• The overall feature boundary looks right for a draft MVP: core owns the local SQLite artifact command, docs distinguish snapshot from backup, and stress/object-store/failover work remains out of scope or stacked separately.

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Superseded by the hardened canonical implementation in #105718, landed as 2f25e9c.

Thank you, @giodl73-repo, for establishing the SQLite-safe artifact direction and the original backup-surface design. This PR could not be updated because maintainerCanModify=false, and the landed diff needed to differ materially: strict OpenClaw database roles, canonical schema/index/owner validation, fresh-only restore, content-pinned verification, durable publication, POSIX/macOS ownership and ACL checks, protected Windows DACL creation, path-race defenses, and native Windows/macOS/Linux proof.

The concurrent stress concept remains open in #94967 and will be adapted onto the landed API with author credit rather than closing that validation work here.