Skip to content

refactor: route checkpoint mutations through accessor#96222

Merged
jalehman merged 2 commits into
openclaw:mainfrom
jalehman:josh/path3-compaction-checkpoint-store-transaction
Jun 24, 2026
Merged

refactor: route checkpoint mutations through accessor#96222
jalehman merged 2 commits into
openclaw:mainfrom
jalehman:josh/path3-compaction-checkpoint-store-transaction

Conversation

@jalehman

@jalehman jalehman commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Path 3 is moving session and transcript state toward SQLite-backed storage, but compaction checkpoint branch/restore still had storage mutation logic coupled directly to file-backed store helpers. This PR narrows that boundary so checkpoint branch/restore owns transcript fork/copy plus session-entry metadata mutation as one domain store operation.

Why This Change Was Made

The future SQLite implementation needs to copy transcript rows and update session_entries.entry_json inside one write transaction. Routing the current file-backed implementation through CompactionCheckpointStore and session-accessor operations preserves today's behavior while making the intended transaction boundary explicit for the SQLite-backed store.

User Impact

No user-visible behavior change is intended. Existing compaction checkpoint list/get/branch/restore behavior continues to fork from the stored checkpoint boundary and persist branch/restore session metadata; the change is internal migration preparation for #88838.

Evidence

  • pnpm lint:tmp:session-accessor-boundary passed.
  • node scripts/run-vitest.mjs src/config/sessions/session-accessor.test.ts src/gateway/session-compaction-checkpoints.test.ts test/scripts/check-session-accessor-boundary.test.ts passed: 28 unit-fast tests, 64 gateway tests, 49 runtime-config tests.
  • pnpm tsgo:test:src passed.
  • Autoreview clean against upstream/main: no accepted/actionable findings.
  • Real behavior proof completed on live local gateway from /Users/phaedrus/Projects/clawdbot using head 2f40aabd3e5 before the non-live rebase follow-up.
  • Non-live mergeability follow-up completed after upstream/main advanced to c70accc86f8: branch rebased to 4c3d1f83ce0, local merge-base HEAD upstream/main equals c70accc86f8, and local git merge-tree reports no conflicts.
  • Rebase conflicts were limited to accessor ratchet/test overlap from landed Path 3 slices; resolution kept both current-main entries and this compaction checkpoint slice.
  • Post-rebase proof passed: pnpm lint:tmp:session-accessor-boundary, node scripts/run-vitest.mjs src/config/sessions/session-accessor.test.ts src/gateway/session-compaction-checkpoints.test.ts test/scripts/check-session-accessor-boundary.test.ts, pnpm tsgo:test:src, and autoreview against upstream/main.

Real behavior proof

Behavior addressed: compaction checkpoint branch/restore storage boundary, including transcript fork/copy from a stored checkpoint boundary and session-entry metadata persistence through the accessor-backed store operation.

Real environment tested: live local OpenClaw gateway in /Users/phaedrus/Projects/clawdbot, loopback gateway on port 18789, agent main, synthetic temporary proof session only.

Exact steps or command run after this patch:

# Preflight and exact build
pnpm openclaw tasks list --status running --json
# -> { "count": 0, "tasks": [] }
git checkout --detach 2f40aabd3e5ab63763691adc2c4c54dbaed01d62
pnpm build
pnpm openclaw gateway restart
pnpm openclaw --version
# -> OpenClaw 2026.6.9 (2f40aab)
curl -fsS http://127.0.0.1:18789/readyz
curl -fsS http://127.0.0.1:18789/healthz

# Synthetic proof fixture, created through SessionManager, not raw JSONL message writes
# source key: agent:main:path3-checkpoint-proof-rebased-20260623164953-30482
# checkpoint: c9b07ac6-eece-4a07-9e9d-dc56193a6cfd
# pre-compaction leaf: 8a38d197
# post-compaction checkpoint leaf: 3d251843

pnpm openclaw gateway call sessions.compaction.list --json \
  --params '{"key":"agent:main:path3-checkpoint-proof-rebased-20260623164953-30482","agentId":"main"}' \
  --timeout 20000

pnpm openclaw gateway call sessions.compaction.branch --json \
  --params '{"key":"agent:main:path3-checkpoint-proof-rebased-20260623164953-30482","checkpointId":"c9b07ac6-eece-4a07-9e9d-dc56193a6cfd","agentId":"main"}' \
  --timeout 20000

pnpm openclaw gateway call sessions.compaction.restore --json \
  --params '{"key":"agent:main:path3-checkpoint-proof-rebased-20260623164953-30482","checkpointId":"c9b07ac6-eece-4a07-9e9d-dc56193a6cfd","agentId":"main"}' \
  --timeout 20000

Evidence after fix:

  • sessions.compaction.list returned ok: true with the synthetic checkpoint.
  • sessions.compaction.branch returned ok: true, branch key agent:main:dashboard:a0d9bba0-926c-42ec-8114-f877bd608359, branch session id 9ad7d9d2-1f06-4b36-b564-10731e7a7234, parent session key agent:main:path3-checkpoint-proof-rebased-20260623164953-30482, and totalTokens: 34 / totalTokensFresh: true.
  • sessions.compaction.restore returned ok: true, restored session id fe11b495-db40-4605-a2e6-fa81010475fb, preserved one checkpoint in the restored entry, and persisted totalTokens: 34 / totalTokensFresh: true.
  • Verification script read sessions.json and both generated transcript files. The branch and restore transcripts contained PATH3_CHECKPOINT_SOURCE path3-checkpoint-proof-rebased-20260623164953-30482, did not contain PATH3_CHECKPOINT_FUTURE path3-checkpoint-proof-rebased-20260623164953-30482, and the store entry matched the restored session id/file.
  • Cleanup removed the synthetic source key, branch key, and all generated proof transcript files; final scan found no path3-checkpoint-proof keys.
  • Live service was restored to main at 6f2869c296f, rebuilt, restarted, and /readyz settled to ready: true with eventLoop.degraded: false; /healthz returned { "ok": true, "status": "live" }.

Observed result after fix: branch/restore kept the existing file-backed behavior through the real gateway surface while proving the store/accessor mutation boundary persists session-entry metadata and forks transcripts from the stored checkpoint boundary. No synthetic proof sessions or transcript files remained afterward.

What was not tested: SQLite-backed session/transcript storage is not enabled yet, so this proof covers the current file-backed store implementation and the intended transaction/accessor boundary only. Live gateway proof was not rerun after the non-live rebase to 4c3d1f83ce0 because the serialized live lane is assigned to the boot-session mapping thread; exact-head proof for 4c3d1f83ce0 is focused local proof plus clean merge-tree. Fresh CI is running on the rebased head.

@jalehman
jalehman force-pushed the josh/path3-compaction-checkpoint-store-transaction branch from 2f40aab to 4c3d1f8 Compare June 24, 2026 00:04
@clawsweeper

clawsweeper Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 23, 2026, 11:02 PM ET / 03:02 UTC.

Summary
Routes compaction checkpoint branch, restore, and checkpoint persistence through session-accessor boundary helpers, with regression coverage and boundary-ratchet updates.

PR surface: Source +131, Tests +162, Other +1. Total +294 across 6 files.

Reproducibility: not applicable. as a bug reproduction; this is an internal refactor. The relevant behavior path is covered by gateway sessions.compaction.list/branch/restore proof and focused regression tests.

Review metrics: 1 noteworthy metric.

  • Persistent session mutations: 3 rerouted. Checkpoint branch, restore, and persist all write session metadata, so maintainers should notice the state-boundary change even without config/default changes.

Stored data model
Persistent data-model change detected: serialized state: src/config/sessions/session-accessor.test.ts, serialized state: src/config/sessions/session-accessor.ts, serialized state: src/gateway/session-compaction-checkpoints.test.ts, serialized state: src/gateway/session-compaction-checkpoints.ts, unknown-data-model-change: src/config/sessions/session-accessor.test.ts, unknown-data-model-change: src/config/sessions/session-accessor.ts, and 1 more. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: partial_overlap
Canonical: #88838
Summary: This PR is one implementation slice of the broader session/transcript SQLite accessor-seam tracker, not a duplicate or complete replacement for it.

Members:

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • none.

Risk before merge

  • [P1] SQLite-backed session/transcript storage is intentionally not enabled or proven by this PR; that remains future work under the tracker.

Maintainer options:

  1. Decide the mitigation before merge
    Review and land this bounded file-backed accessor slice, leaving SQLite backend and migration proof to Track core session/transcript SQLite migration via accessor seam #88838 and later PRs.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • No ClawSweeper repair is needed; the remaining action is ordinary maintainer review/merge handling for a protected-label PR.

Security
Cleared: No concrete security or supply-chain concern found; the diff touches session/gateway code and a local boundary script, with no dependency, workflow, permission, secret, or package-resolution changes.

Review details

Best possible solution:

Review and land this bounded file-backed accessor slice, leaving SQLite backend and migration proof to #88838 and later PRs.

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

Not applicable as a bug reproduction; this is an internal refactor. The relevant behavior path is covered by gateway sessions.compaction.list/branch/restore proof and focused regression tests.

Is this the best way to solve the issue?

Yes, this is the best current shape for the tracker slice: it moves the checkpoint write boundary to the session accessor without flipping storage or adding runtime dual-read behavior. Leaving the direct gateway updateSessionStore mutation in place would keep the file-backed coupling the tracker is trying to remove.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P2: This is a normal-priority internal session-state hardening/refactor slice with limited current-runtime blast radius and good focused proof.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal live-gateway list/branch/restore proof for the changed file-backed path plus exact-head focused tests and green CI after rebase.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal live-gateway list/branch/restore proof for the changed file-backed path plus exact-head focused tests and green CI after rebase.

Label justifications:

  • P2: This is a normal-priority internal session-state hardening/refactor slice with limited current-runtime blast radius and good focused proof.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal live-gateway list/branch/restore proof for the changed file-backed path plus exact-head focused tests and green CI after rebase.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal live-gateway list/branch/restore proof for the changed file-backed path plus exact-head focused tests and green CI after rebase.
Evidence reviewed

PR surface:

Source +131, Tests +162, Other +1. Total +294 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 2 227 96 +131
Tests 3 162 0 +162
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 1 1 0 +1
Total 6 390 96 +294

What I checked:

  • Repository policy read: Read the full root AGENTS.md plus the scoped scripts, gateway, and test guides; the storage/accessor and gateway-test guidance shaped the review. (AGENTS.md:1)
  • Live PR state: GitHub reports the PR open, non-draft, mergeable, CLEAN, with maintainer/gateway/scripts labels and successful CI/status checks on head 4c3d1f8. (4c3d1f83ce04)
  • Current main behavior: Current main still keeps checkpoint branch/restore mutations directly inside session-compaction-checkpoints via updateSessionStore, so the PR is not already implemented on main. (src/gateway/session-compaction-checkpoints.ts:583, d15e89a83e61)
  • PR accessor boundary: The PR adds typed branch/restore checkpoint mutation helpers that fork transcript content and write session entry metadata through one storage-sized accessor operation. (src/config/sessions/session-accessor.ts:1264, 4c3d1f83ce04)
  • Gateway caller update: The PR changes gateway checkpoint branch/restore to call the accessor helpers and changes checkpoint persistence to use updateSessionEntry while preserving the malformed-row no-op guard. (src/gateway/session-compaction-checkpoints.ts:578, 4c3d1f83ce04)
  • Regression coverage: The PR adds accessor-level tests for branch/restore mutation behavior and gateway-level coverage for malformed checkpoint persistence rows. (src/config/sessions/session-accessor.test.ts:892, 4c3d1f83ce04)

Likely related people:

  • jalehman: Authored and updated the canonical session/transcript SQLite seam tracker and multiple recent session-accessor slices on current main. (role: feature owner and recent area contributor; confidence: high; commits: 96cee6cb6442, ae433525f0b1, f8ed4de460f7; files: src/config/sessions/session-accessor.ts, src/gateway/session-compaction-checkpoints.ts)
  • vincentkoc: Recent commits touched checkpoint and session utility paths, including bounded file reads and checkpoint wrapper cleanup. (role: recent adjacent contributor; confidence: medium; commits: f7d6a059a4b8, 6b25ccc4b176; files: src/gateway/session-compaction-checkpoints.ts, src/config/sessions/session-accessor.ts)
  • steipete: Recent history includes broad agent/runtime refactor work and compaction checkpoint fixes in the same runtime/session area. (role: adjacent owner; confidence: medium; commits: bb46b79d3c14, c59635ae970c; files: src/gateway/session-compaction-checkpoints.ts, src/agents/embedded-agent-runner/compact.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 proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. labels Jun 24, 2026
@jalehman
jalehman merged commit c588606 into openclaw:main Jun 24, 2026
128 of 133 checks passed
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 25, 2026
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gateway Gateway runtime maintainer Maintainer-authored PR P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. scripts Repository scripts size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant