Skip to content

Add named snapshot database targets#94799

Closed
giodl73-repo wants to merge 1 commit into
giodl/snapshot-cli-pr2from
giodl/snapshot-named-targets-pr3
Closed

Add named snapshot database targets#94799
giodl73-repo wants to merge 1 commit into
giodl/snapshot-cli-pr2from
giodl/snapshot-named-targets-pr3

Conversation

@giodl73-repo

@giodl73-repo giodl73-repo commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Summary

This is PR 3 in the snapshot stack. It makes the core snapshot command understand OpenClaw's named SQLite databases instead of requiring operators to know internal file paths.

Adds:

  • openclaw snapshot create --target global --repository <dir> for the shared state database
  • openclaw snapshot create --agent <id> --repository <dir> for a per-agent database
  • selector validation so --db, --target, and --agent remain mutually exclusive

Keeps:

  • explicit --db <path> from PR 2 for non-OpenClaw or advanced caller paths
  • local snapshot repository semantics from the provider proof

Why

Ryan's issue (#94675) called out that live SQLite files and their WAL/SHM companions are not safe sync artifacts for hosted deployments. PR #94646 made the database layout more explicit by separating shared state from per-agent state. This PR uses those core path helpers so snapshotting can target the logical OpenClaw database units directly:

  • global control-plane state: state/openclaw.sqlite
  • per-agent data-plane state: agents/<agent>/agent/openclaw-agent.sqlite

That keeps the operator-facing contract small: ask OpenClaw for a syncable snapshot artifact, not for a copy of whatever live SQLite files happen to exist on disk.

Stack

Scope

This PR does not add object storage, scheduling, restore-on-boot, leases, or WAL bundle deltas. Those stay in later phases. This only proves the CLI can name OpenClaw's own SQLite targets and resolve them through core-owned path helpers.

Real behavior proof

Behavior or issue addressed:
openclaw snapshot create can now create snapshots from named OpenClaw database targets (--target global, --agent <id>) without requiring users to locate and pass live SQLite paths manually.

Real environment tested:
Windows checkout at C:\src\openclaw-snapshot-pr1, source test harness with temporary SQLite databases and local snapshot repositories.

Exact steps or command run after this patch:

  1. node scripts/run-vitest.mjs run src/commands/snapshot.test.ts src/cli/program/register.snapshot.test.ts src/snapshot/snapshot-provider.test.ts
  2. node_modules\.bin\oxlint src\commands\snapshot.ts src\commands\snapshot.test.ts src\cli\program\register.snapshot.ts src\cli\program\register.snapshot.test.ts
  3. git diff --check

Evidence after fix:
Vitest passed 3 shards: src/commands/snapshot.test.ts (6 tests), src/cli/program/register.snapshot.test.ts (7 tests), and src/snapshot/snapshot-provider.test.ts (5 tests). oxlint and git diff --check completed with no output.

Observed result after fix:
The command resolves --target global to the shared OpenClaw state database, resolves --agent "Ops Team" to the normalized per-agent database path, records default manifest ids/kinds for both, preserves explicit --db, and rejects ambiguous selector combinations.

What was not tested:
No live gateway runtime, object storage upload, scheduled backup loop, restore-on-boot path, lease/failover path, or WAL bundle/delta behavior was tested in this PR.

@openclaw-barnacle openclaw-barnacle Bot added cli CLI command changes commands Command implementations size: S maintainer Maintainer-authored PR labels Jun 19, 2026
@clawsweeper

clawsweeper Bot commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 19, 2026, 1:08 AM ET / 05:08 UTC.

Summary
The PR adds openclaw snapshot create selectors for the shared global database and per-agent databases, changes --db from required to one of several mutually exclusive sources, and adds command-handler and registration tests.

PR surface: Source +57, Tests +111. Total +168 across 4 files.

Reproducibility: not applicable. as a bug reproduction; this is a feature PR. Source inspection confirms the proposed handler and tests exercise named target resolution, but real packaged CLI execution has not been proven.

Review metrics: 1 noteworthy metric.

  • Snapshot create selector surface: 2 options added, 1 option requirement changed. The PR adds --target and --agent and changes --db from Commander-required to handler-validated, which becomes public CLI contract once the stack lands.

Stored data model
Persistent data-model change detected: serialized state: src/commands/snapshot.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: canonical
Canonical: #94799
Summary: This PR is the specific named-target layer in the snapshot stack; related items are prerequisites or broader database/durability discussions, not duplicates.

Members:

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

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P1] Add redacted terminal or copied live output from real openclaw snapshot create --target global and openclaw snapshot create --agent <id> runs.
  • Resolve or retarget after the provider and base CLI draft PRs are accepted.
  • [P1] Add user docs for the named target selector contract.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body reports Vitest, oxlint, and git diff --check from a source harness, but does not include redacted real packaged openclaw snapshot create terminal/live output for the new selectors. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Merging the stack would create a new public root CLI selector contract for snapshotting OpenClaw databases, so maintainers need to accept the command and flag semantics before users rely on them.
  • [P1] This PR depends on two open draft snapshot PRs, so the provider and base CLI behavior are not yet accepted on current main.
  • [P1] The contributor proof is source-harness tests and lint only; external PR policy still needs redacted real command output or a maintainer override before merge.
  • [P1] The new --target and --agent selectors are not documented, leaving users without a stable explanation of the target names, path resolution, and agent-id normalization.

Maintainer options:

  1. Resolve Stack And Document Targets (recommended)
    Land or otherwise resolve the provider and base CLI PRs, add docs for --target global and --agent, and attach real packaged command proof before merge.
  2. Accept The Draft Contract Explicitly
    Maintainers can intentionally accept the new CLI selector contract now, with the remaining proof and docs handled before this leaves draft.
  3. Pause With Snapshot Stack
    Keep this PR paused if the snapshot provider or root command shape is still unsettled.

Next step before merge

  • [P1] Protected label, draft stack dependencies, missing real behavior proof, and public CLI/storage contract acceptance all require maintainer review rather than an automated fix lane.

Security
Cleared: No concrete security or supply-chain concern was found; the diff adds CLI option handling and tests without dependencies, workflows, lockfiles, permissions, credential handling, or downloaded code execution.

Review findings

  • [P3] Document the named snapshot target contract — src/cli/program/register.snapshot.ts:30-31
Review details

Best possible solution:

Land this only after the provider and base CLI PRs are accepted, docs describe the named targets, and redacted real openclaw snapshot create --target global and --agent command output proves the packaged path.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a bug reproduction; this is a feature PR. Source inspection confirms the proposed handler and tests exercise named target resolution, but real packaged CLI execution has not been proven.

Is this the best way to solve the issue?

Unclear as merge-ready. The implementation uses the right state path helpers and is a plausible narrow layer, but the best solution still requires maintainer acceptance of the stack, docs, and real CLI proof.

Full review comments:

  • [P3] Document the named snapshot target contract — src/cli/program/register.snapshot.ts:30-31
    The new --target and --agent selectors become user-facing CLI contract, but the PR only adds help strings and tests. Please add/update user docs so operators know that global resolves through the OpenClaw state directory, --agent is normalized, and explicit --db remains the advanced path.
    Confidence: 0.78

Overall correctness: patch is correct
Overall confidence: 0.78

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against af3c10626c4b.

Label changes

Label changes:

  • add P2: This is a normal-priority storage/CLI feature with bounded operator scope and no urgent live-user outage.
  • add merge-risk: 🚨 compatibility: The PR adds public CLI selector flags and changes snapshot create source validation semantics that users and operators can script once shipped.
  • add merge-risk: 🚨 other: Green CI cannot decide whether the snapshot provider, root command, and named database target contract should land in core while the base stack is still draft.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports Vitest, oxlint, and git diff --check from a source harness, but does not include redacted real packaged openclaw snapshot create terminal/live output for the new selectors. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: This is a normal-priority storage/CLI feature with bounded operator scope and no urgent live-user outage.
  • merge-risk: 🚨 compatibility: The PR adds public CLI selector flags and changes snapshot create source validation semantics that users and operators can script once shipped.
  • merge-risk: 🚨 other: Green CI cannot decide whether the snapshot provider, root command, and named database target contract should land in core while the base stack is still draft.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports Vitest, oxlint, and git diff --check from a source harness, but does not include redacted real packaged openclaw snapshot create terminal/live output for the new selectors. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +57, Tests +111. Total +168 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 63 6 +57
Tests 2 112 1 +111
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 175 7 +168

What I checked:

  • Live PR state: Live GitHub metadata shows the PR is open, draft, mergeable clean, labeled maintainer, and stacked on giodl/snapshot-cli-pr2 at base SHA a341086. (addd22047e2b)
  • Named target implementation: The proposed handler resolves exactly one source selector, maps --target global through the shared state DB path helper, and maps --agent through normalized per-agent DB path resolution. (src/commands/snapshot.ts:139, addd22047e2b)
  • CLI selector surface: The proposed Commander registration adds --target <target> and --agent <id> while making explicit --db <path> optional at the parser layer. (src/cli/program/register.snapshot.ts:29, addd22047e2b)
  • Current path contract: Current main resolves the shared state DB as <state root>/state/openclaw.sqlite and per-agent DBs as <state root>/agents/<agentId>/agent/openclaw-agent.sqlite, matching the intended named targets. (src/state/openclaw-agent-db.paths.ts:20, af3c10626c4b)
  • Database layout docs: The database-first refactor docs name the global and per-agent SQLite files that this PR exposes as friendly snapshot targets. Public docs: docs/refactor/database-first.md. (docs/refactor/database-first.md:103, af3c10626c4b)
  • Not already implemented: Current main and latest release v2026.6.8 have no src/snapshot, src/commands/snapshot, or register.snapshot files, so the named-target work is not obsolete on main or shipped release. (af3c10626c4b)

Likely related people:

  • vincentkoc: Current blame and recent history point to Vincent Koc for the database-first SQLite path helpers, command registry area, and backup snapshot path that this PR builds on. (role: recent area contributor; confidence: high; commits: a1b7118d0f2c, 65eb2ccd4ae2; files: src/state/openclaw-state-db.paths.ts, src/state/openclaw-agent-db.paths.ts, src/cli/program/command-registry-core.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels Jun 19, 2026
@giodl73-repo giodl73-repo mentioned this pull request Jun 19, 2026
Closed
@clawsweeper clawsweeper Bot mentioned this pull request Jun 19, 2026
Closed
@giodl73-repo giodl73-repo mentioned this pull request Jun 19, 2026
Closed
@giodl73-repo

giodl73-repo commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Closing this stack slice in favor of the consolidated core snapshot PR: #94805.

We collapsed the previous PR1-4 chain into one reviewable feature PR so reviewers can evaluate the provider, CLI, named targets, and safe-sync artifact proof together. The stress harness remains a separate follow-up in #94967.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

cli CLI command changes commands Command implementations maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@giodl73-repo