refactor(sessions): move parent-session forking behind the accessor boundary by jalehman · Pull Request #101699 · openclaw/openclaw

jalehman · 2026-07-07T13:47:26Z

Related: #88838
Related: #98236
Related: #101179

What Problem This Solves

After #101179 moved the raw fork-transcript write behind the accessor, the rest of parent-session forking still bypassed the session accessor seam (#88838): session-fork.runtime.ts resolved parent transcript paths directly (resolveSessionFilePath) and owned branch-selection reads, and session-fork.ts persisted the forked child entry through a raw whole-store updateSessionStore mutation with hand-rolled alias cleanup. Callers passed raw sessionsDir/targetSessionsDir paths across the boundary. This is prep for the SQLite storage flip (#98236), which makes the whole fork operation storage-owned.

Why This Change Was Made

New accessor boundary ops: forkSessionFromParentTranscript (parent-branch transcript fork composing createForkedSessionTranscript) and forkSessionEntryFromParentTarget (fork + child-entry persist + alias cleanup as one store operation). Pure branch-selection logic moves to src/config/sessions/session-fork-transcript.runtime.ts, loaded lazily so accessor consumers do not pull the agents session-manager runtime (and its module-init package metadata reads) at import time.
session-fork.ts becomes a thin adapter; session-fork.runtime.ts shrinks to the lazy token-estimation seam (its transcript-tail usage reads flow through gateway fs helpers, so the fork decision stays caller-supplied via a documented resolveDecision callback; the SQLite flip resolves the decision inside the storage boundary).
targetStorePath carries the cross-agent worktree-session fork (session-create-service.ts) as an explicit contract instead of a directory-path hack.
Callers (subagent-spawn, session-create-service, session-parent-fork-prepare) pass session keys and store paths, not directories.

User Impact

No user-visible behavior change. Thread/subagent/operator session forks keep the same branch selection, oversized-parent skip decision, fork placement (beside the mutated store), and entry/alias persistence semantics.

Evidence

End-to-end behavior proof — ad-hoc uncommitted harness (isolated state dir, real gateway process booted from this branch via test/helpers/openclaw-test-instance.ts, two configured agents to exercise the cross-agent path, public RPCs only, then direct sessions.json/transcript-JSONL inspection; macOS, Node v24.18.0). Run on origin/main baseline (c8d95da14c) and this branch at head 003c51975c — 17/17 checkpoints pass on both (behavior parity plus the new contract). Checkpoints on this PR's surface:

PASS  B1 forked child entry persisted with fork markers        {"forkedFromParent":true,"sessionId":"..."}
PASS  B2 child transcript artifact exists beside parent store  {"dirMatchesStore":true}
PASS  B3 child header records parent lineage                   parentSession -> parent transcript path
PASS  B4 child transcript copies the parent's active branch    {"records":3}
PASS  B5 cross-agent fork lands transcript beside the child
        agent's store (targetStorePath)                        agents/beta/sessions/..._<childId>.jsonl
PASS  B6 cross-agent child copies parent branch content
PASS  B7 oversized parent fork rejected via real byte-estimate {"transcriptBytes":525029,
        decision                                                "error":"parent session is too large to fork (131258/100000 tokens)"}
PASS  C1 no orphan transcript artifacts in sessions dir        {"orphans":[]}

Static/test gates

node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.test.src.json clean; oxfmt + type-aware oxlint clean on changed files
node scripts/check-session-accessor-boundary.mjs green; debt baseline regenerated (session-fork.ts write debt -> 0)
Branch-semantics tests relocated to session-accessor.parent-fork.test.ts against the accessor API; store-dir boundary test exercises real transcript files instead of runtime mocks
Green: session-fork suites, subagent-spawn.* family, agent-consult-runtime, server.sessions.create, agent-runner-session-reset, accessor suites, plus the command/channel isolation suites the earlier CI run flagged (agents.delete, channels.list, channel-setup/*, session.test.ts)
pnpm build green with no [INEFFECTIVE_DYNAMIC_IMPORT] for the new lazy boundary
.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main clean (two rounds; latest on 003c51975c)

clawsweeper · 2026-07-07T13:53:47Z

Codex review: needs maintainer review before merge. Reviewed July 7, 2026, 12:36 PM ET / 16:36 UTC.

Summary
The branch moves parent-session transcript forking and child entry persistence behind session accessor operations, updates callers/tests, and ratchets the accessor debt baseline.

PR surface: Source +149, Tests +50, Other -1. Total +198 across 13 files.

Reproducibility: not applicable. as a behavior-preserving refactor. The review relies on source comparison plus the PR's live-output parity proof rather than a current-main failure reproduction.

Review metrics: 1 noteworthy metric.

Accessor debt movement: 1 write-debt entry removed; session-accessor count +1. This quantifies that the parent-fork store write is moving from session-fork into the accessor boundary that protects the session-state migration.

Stored data model
Persistent data-model change detected: serialized state: scripts/lib/session-accessor-debt-baseline.json, serialized state: src/auto-reply/reply/session-fork.runtime.test.ts, serialized state: src/auto-reply/reply/session-fork.runtime.ts, serialized state: src/auto-reply/reply/session-fork.test.ts, serialized state: src/auto-reply/reply/session-fork.ts, serialized state: src/auto-reply/reply/session-parent-fork-prepare.ts, and 12 more. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #88838
Summary: This PR is a focused candidate slice for the broader session/transcript accessor and SQLite migration tracker, with adjacent merged and open PRs covering neighboring storage seams.

Members:

canonical: Track core session/transcript SQLite migration via accessor seam #88838 - Open tracker for moving session/transcript runtime state behind the accessor seam before the SQLite flip.
partial_overlap: refactor: flip sessions and transcripts to sqlite storage #98236 - The active SQLite storage flip depends on the same accessor seam but owns the larger runtime storage migration.
same_root_cause: refactor(sessions): route new session store bypasses through the accessor #101179 - Merged predecessor moved adjacent session store and fork-transcript writes behind the accessor.
same_root_cause: refactor(gateway): route chat transcript injection through the session accessor #101688 - Merged sibling moved gateway chat transcript injection through the same accessor boundary.
adjacent_distinct: fix(session-fork): skip fork when parent size is unresolved instead of forking (#101718) #101767 - Open timeout fix touches parent-fork token-count resolution, but it addresses availability rather than the accessor boundary refactor.

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
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:

[P2] Let queued exact-head CI/preflight checks finish before merge.

Risk before merge

[P1] The diff changes how parent-fork transcript identity and child session entries are created and persisted, so maintainers should explicitly accept the session-state upgrade risk before landing even with green tests and sufficient proof.

Maintainer options:

Land with session-state signoff (recommended)
Merge after exact-head checks complete and maintainers accept the proof covering same-agent, cross-agent, oversized-parent, lineage, and alias-cleanup behavior.
Pause into the storage flip
Keep this PR open or close it only if the same parent-fork seam is intentionally absorbed into the broader SQLite storage flip branch.

Next step before merge

No automated repair remains; maintainers should review the session-state boundary proof and decide whether to land this seam slice before the SQLite flip.

Maintainer decision needed

Question: Should maintainers land this parent-fork accessor-boundary slice as a standalone step before the broader SQLite storage flip?
Rationale: The implementation is a cleanup/refactor with sufficient proof and no current blocking finding, but it changes session-state ownership and is protected by the maintainer/session-state review surface.
Likely owner: jalehman — They own the active session/transcript accessor migration history and authored the adjacent merged seam work.
Options:
- Land after exact-head checks (recommended): Accept the standalone seam refactor once queued checks on c8a2214 finish and the session-state proof is accepted.
- Fold into SQLite flip: Pause this PR and carry the same parent-fork boundary movement in refactor: flip sessions and transcripts to sqlite storage #98236 if maintainers want one storage-flip review lane.

Security
Cleared: The diff changes session state code/tests and a JSON debt baseline only; no dependency, workflow, package, secret, permission, or supply-chain surface changed.

Review details

Best possible solution:

Land this standalone accessor-boundary slice after exact-head CI and maintainer session-state review, while leaving the broader SQLite runtime flip in #98236.

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

Not applicable as a behavior-preserving refactor. The review relies on source comparison plus the PR's live-output parity proof rather than a current-main failure reproduction.

Is this the best way to solve the issue?

Yes. Moving the remaining parent-fork transcript and child-entry persistence behind the session accessor is the clean owner-boundary step; leaving raw runtime/store writes or folding every seam into the giant SQLite flip would be harder to review.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body/comment provides copied live output from a real gateway harness with 17/17 relevant checkpoints, and the latest commit is test-only while GitHub's proof check is green for the current head.
remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

P2: This is a normal-priority session-state refactor with limited blast radius and no current user-facing outage.
merge-risk: 🚨 session-state: The diff changes parent-fork transcript lineage, placement, child session entry persistence, and alias cleanup.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body/comment provides copied live output from a real gateway harness with 17/17 relevant checkpoints, and the latest commit is test-only while GitHub's proof check is green for the current head.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body/comment provides copied live output from a real gateway harness with 17/17 relevant checkpoints, and the latest commit is test-only while GitHub's proof check is green for the current head.

Evidence reviewed

PR surface:

Source +149, Tests +50, Other -1. Total +198 across 13 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	7	537	388	+149
Tests	5	677	627	+50
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	1	1	2	-1
Total	13	1215	1017	+198

What I checked:

Repository policy read: Root AGENTS.md plus scoped src/agents, src/gateway, src/gateway/server-methods, and scripts guidance were read; the session-state/accessor-boundary and lazy-loading guidance shaped the review. (AGENTS.md:22, 453f5968bbca)
Current main gap: Current main still routes parent forking through session-fork.runtime.ts and raw store mutation, so the central accessor-boundary refactor is not already implemented on main. (src/auto-reply/reply/session-fork.ts:147, 453f5968bbca)
PR accessor entry point: The merge result routes forkSessionFromParent through forkSessionFromParentTranscript and routes forkSessionEntryFromParent through forkSessionEntryFromParentTarget. (src/auto-reply/reply/session-fork.ts:131, ea766da19129)
Storage-owned fork implementation: The accessor now owns parent transcript resolution, branch copy creation, child entry persistence, and alias cleanup for parent forks. (src/config/sessions/session-accessor.ts:1518, ea766da19129)
Cross-agent placement contract: Gateway session creation passes targetStorePath so cross-agent fork transcripts land beside the child store while the decision still reads the parent store. (src/gateway/session-create-service.ts:448, ea766da19129)
Prior finding fixed: The latest head changes only src/auto-reply/reply/session.test.ts and the mock now forwards storePath into the fork test double. (src/auto-reply/reply/session.test.ts:116, c8a22142cd19)

Likely related people:

jalehman: Authored the open session/transcript accessor tracker and the merged predecessor PRs that moved adjacent session persistence paths behind the accessor. (role: feature owner and recent area contributor; confidence: high; commits: 4e2a80cac34d, 46144a3bf37e, 453f5968bbca; files: src/config/sessions/session-accessor.ts, src/auto-reply/reply/session-fork.ts, src/auto-reply/reply/session-fork.runtime.ts)
vincentkoc: Local current-main blame/log points at the recent PR that introduced the current session-fork.ts file shape in available history, though the PR was a broader runtime facade cleanup rather than this session migration lane. (role: recent file contributor; confidence: low; commits: 098b471143c1; files: src/auto-reply/reply/session-fork.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.

Review history (3 earlier review cycles)

reviewed 2026-07-07T13:59:33.651Z sha 4b8a8b4 :: needs real behavior proof before merge. :: none
reviewed 2026-07-07T14:37:39.233Z sha 75f2d08 :: needs real behavior proof before merge. :: [P2] Update the thread-fork test double for the new call shape
reviewed 2026-07-07T15:54:39.671Z sha 003c519 :: needs changes before merge. :: [P2] Forward storePath in the fork test double

…oundary

jalehman · 2026-07-07T15:14:50Z

Superseded by the Evidence section in the PR body (refreshed for head 003c51975c, which adds the lazy fork-transcript runtime boundary; harness re-run 17/17 on that head).

E2E behavior proof (ad-hoc harness per the live-proof pattern: isolated state dir, real gateway process, public RPCs only, then direct store/transcript artifact inspection — harness is not committed).

Environment: macOS, Node v24.18.0, gateway booted from this branch's source via the repo test-instance machinery (test/helpers/openclaw-test-instance.ts), config with two agents (main, beta) to exercise the cross-agent path. Three runs: origin/main baseline (c8d95da14c), this branch (75f2d08c4a), and the #101688 branch — 17/17 checkpoints pass on all three, i.e. behavior parity with main plus the new seams.

Checkpoints covering this PR's surface (branch run, verbatim):

PASS  B1 forked child entry persisted with fork markers :: {"forkedFromParent":true,"sessionId":"43f5d7ba-..."}
PASS  B2 child transcript artifact exists beside parent store :: {"dirMatchesStore":true}
PASS  B3 child header records parent lineage :: {"parentSession":"<state>/agents/main/sessions/012ecd2c-....jsonl"}
PASS  B4 child transcript copies the parent's active branch :: {"records":3}
PASS  B5 cross-agent fork lands transcript beside the child agent's store (targetStorePath) :: {"sessionFile":"<state>/agents/beta/sessions/2026-07-07T15-13-11-242Z_383e53eb-....jsonl","forkedFromParent":true}
PASS  B6 cross-agent child copies parent branch content
PASS  B7 oversized parent fork rejected via real byte-estimate decision :: {"transcriptBytes":525029,"error":"parent session is too large to fork (131258/100000 tokens)"}
PASS  C1 no orphan transcript artifacts in sessions dir :: {"orphans":[]}

What this proves for the accessor boundary ops: sessions.create {fork: true} runs forkSessionFromParentTranscript + forkSessionEntryFromParentTarget end to end — child entry persisted in one store operation with forkedFromParent, fresh sessionId, and a real transcript artifact whose header records the parent lineage and whose records copy the parent's active branch. The new targetStorePath contract lands cross-agent fork transcripts beside the child agent's store (B5). The oversized-parent skip decision flows through the real file byte-estimate path (525 KB transcript → 131k token estimate → rejected against the 100k cap), i.e. the caller-supplied resolveDecision seam preserves the shipped decision semantics.

…orts lean

Squash-and-replay of sqlite/sessions-transcripts-storage-flip (361 commits, old head 7766516) onto main @ 21f2512, per the prep-series plan: Phase 0/1 accessor-prep PRs (#101178-#101182, #101688, #101699) landed the seam on main first, so this rebase reduces to the SQLite storage implementation behind the session accessor. Beyond the flip branch content, this merge: - implements findTranscriptEvent (reverse-seq), recordInboundSessionMeta and updateSessionLastRoute over SQLite (deriveLastRoutePatch shared with the file-era store for doctor paths) - adds cross-agent fork support (targetStorePath) so worktree/cross-agent sessions.create forks land child transcript rows in the target agent DB - keeps main's newer behavior across the conflict surface (work admission, archive-gated deletes, preflight overflow budgeting, model fallback, replyOptions dispatch shape, lifecycle-header ordering) - adopts the storage-neutral ContextEngineSessionTarget while preserving the deprecated CompactResult.sessionFile for shipped plugin readers Known remainder (tracked for follow-up commits on this PR): ~415 test-case failures across 43 files, dominated by stale vi.mock factories missing main-side exports; prod typecheck, full build, and boundary ratchet are green. Co-Authored-By: Claude Fable 5 <[email protected]>

…oundary (openclaw#101699) * refactor(sessions): move parent-session forking behind the accessor boundary * fix(sessions): lazy-load fork transcript runtime to keep accessor imports lean * test(sessions): align parent fork mock shape

Squash-and-replay of sqlite/sessions-transcripts-storage-flip (361 commits, old head 7766516) onto main @ 21f2512, per the prep-series plan: Phase 0/1 accessor-prep PRs (#101178-#101182, #101688, #101699) landed the seam on main first, so this rebase reduces to the SQLite storage implementation behind the session accessor. Beyond the flip branch content, this merge: - implements findTranscriptEvent (reverse-seq), recordInboundSessionMeta and updateSessionLastRoute over SQLite (deriveLastRoutePatch shared with the file-era store for doctor paths) - adds cross-agent fork support (targetStorePath) so worktree/cross-agent sessions.create forks land child transcript rows in the target agent DB - keeps main's newer behavior across the conflict surface (work admission, archive-gated deletes, preflight overflow budgeting, model fallback, replyOptions dispatch shape, lifecycle-header ordering) - adopts the storage-neutral ContextEngineSessionTarget while preserving the deprecated CompactResult.sessionFile for shipped plugin readers Known remainder (tracked for follow-up commits on this PR): ~415 test-case failures across 43 files, dominated by stale vi.mock factories missing main-side exports; prod typecheck, full build, and boundary ratchet are green. Co-Authored-By: Claude Fable 5 <[email protected]>

…oundary (openclaw#101699) * refactor(sessions): move parent-session forking behind the accessor boundary * fix(sessions): lazy-load fork transcript runtime to keep accessor imports lean * test(sessions): align parent fork mock shape

Squash-and-replay of sqlite/sessions-transcripts-storage-flip (361 commits, old head 7766516) onto main @ 21f2512, per the prep-series plan: Phase 0/1 accessor-prep PRs (#101178-#101182, #101688, #101699) landed the seam on main first, so this rebase reduces to the SQLite storage implementation behind the session accessor. Beyond the flip branch content, this merge: - implements findTranscriptEvent (reverse-seq), recordInboundSessionMeta and updateSessionLastRoute over SQLite (deriveLastRoutePatch shared with the file-era store for doctor paths) - adds cross-agent fork support (targetStorePath) so worktree/cross-agent sessions.create forks land child transcript rows in the target agent DB - keeps main's newer behavior across the conflict surface (work admission, archive-gated deletes, preflight overflow budgeting, model fallback, replyOptions dispatch shape, lifecycle-header ordering) - adopts the storage-neutral ContextEngineSessionTarget while preserving the deprecated CompactResult.sessionFile for shipped plugin readers Known remainder (tracked for follow-up commits on this PR): ~415 test-case failures across 43 files, dominated by stale vi.mock factories missing main-side exports; prod typecheck, full build, and boundary ratchet are green. Co-Authored-By: Claude Fable 5 <[email protected]>

openclaw-barnacle Bot added gateway Gateway runtime scripts Repository scripts agents Agent runtime and tooling size: XL maintainer Maintainer-authored PR labels Jul 7, 2026

refactor(sessions): move parent-session forking behind the accessor b…

75f2d08

…oundary

jalehman force-pushed the b62.4/session-fork-accessor branch from 4b8a8b4 to 75f2d08 Compare July 7, 2026 14:23

jalehman mentioned this pull request Jul 7, 2026

refactor(gateway): route chat transcript injection through the session accessor #101688

Merged

fix(sessions): lazy-load fork transcript runtime to keep accessor imp…

003c519

…orts lean

clawsweeper Bot mentioned this pull request Jul 7, 2026

fix(session-fork): skip fork when parent size is unresolved instead of forking (#101718) #101767

Closed

jalehman self-assigned this Jul 7, 2026

test(sessions): align parent fork mock shape

c8a2214

clawsweeper Bot added the status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. label Jul 7, 2026

clawsweeper Bot mentioned this pull request Jul 7, 2026

fix: add timeout to parent-fork token count resolution (fixes #101718) #101766

Closed

jalehman merged commit a68caa1 into main Jul 7, 2026
120 of 125 checks passed

jalehman deleted the b62.4/session-fork-accessor branch July 7, 2026 17:47

github-actions Bot mentioned this pull request Jul 7, 2026

📡 Upstream Digest — 2026-07-07 17:52 UTC curtismercier/openclaw-mods#1231

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

refactor(sessions): move parent-session forking behind the accessor boundary#101699

refactor(sessions): move parent-session forking behind the accessor boundary#101699
jalehman merged 3 commits into
mainfrom
b62.4/session-fork-accessor

jalehman commented Jul 7, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

jalehman commented Jul 7, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

jalehman commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jalehman commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jalehman commented Jul 7, 2026 •

edited

Loading

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

jalehman commented Jul 7, 2026 •

edited

Loading