refactor(gateway): route chat transcript injection through the session accessor by jalehman · Pull Request #101688 · openclaw/openclaw

jalehman · 2026-07-07T13:19:33Z

Related: #88838
Related: #98236

What Problem This Solves

src/gateway/server-methods/chat.ts and chat-transcript-inject.ts were among the last gateway runtime surfaces doing direct durable session/transcript persistence outside the session accessor seam (#88838): raw resolveSessionFilePath path resolution, three direct readSessionTranscriptIndex JSONL reads, a direct updateSessionStoreEntry write, and ad-hoc transcript file creation. This keeps rebase conflicts for the SQLite storage flip (#98236) entangled with gateway behavior.

Why This Change Was Made

Prep work for the storage flip: move persistence behind the accessor on main so the flip branch's chat.ts becomes a storage-implementation swap instead of a behavior-plus-storage rewrite.

New storage-neutral accessor reads: loadTranscriptEvents(scope) and findTranscriptEvent(scope, match) (newest-first, early-exit reverse scan) — the hot idempotent append path never materializes the whole transcript.
chat-transcript-inject.ts persists through persistSessionTranscriptTurn with scope identity, in-lock assistant-scoped idempotency (shouldAppend + idempotencyLookup: "caller-checked"), and the session marker touch folded into touchSessionEntry.
The file-specific source-reply mirror rewrite stays contained in one helper (rewriteSourceReplyTranscriptMirrors) around the existing branch-aware rewriteTranscriptEntriesInRuntimeTranscript.
chat.ts joins migratedSessionAccessorWriteFiles; its debt-baseline entries drop to zero.
Kept on main intentionally: legacy transcripts with id-less assistant rows still dedupe by idempotency key (real shipped file state).

User Impact

No user-visible behavior change. Gateway-injected assistant messages (webchat media, abort partials, chat.inject) persist through the accessor with the same idempotency, dedupe, and session-marker 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, public RPCs only, then direct sessions.json/transcript-JSONL inspection; macOS, Node v24.18.0). Run on origin/main baseline (c8d95da14c) and this branch — 17/17 checkpoints pass on both (behavior parity). Checkpoints on this PR's surface:

PASS  A0 sessions.create persisted parent entry
PASS  A1 chat.inject returned message ids
PASS  A2 session entry names transcript artifact
PASS  A3 transcript has header + both injected rows            {"records":3,"messages":2}
PASS  A4 injected rows carry gateway-injected provenance       {"provider":"openclaw","model":"gateway-injected"}
PASS  A5 label prefix rendered into first row content          "[proof]\n\nfirst injected assistant message"
PASS  A6 second row parent-links to first (persist turn chain) secondParentId === firstId
PASS  A7 session marker touched through accessor               updatedAt 1783437157423 -> 1783437157450
PASS  A8 chat.history replays both injected messages           {"messages":2}
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 (chat.ts write debt -> 0)
node scripts/run-vitest.mjs src/gateway/server-methods — 2760 passed (pre-existing models.test.ts failures reproduce identically on pristine origin/main; local ambient auth state)
Focused: chat.directive-tags (268), chat.abort-persistence, chat.inject.parentid, session-accessor, boundary-script tests
.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main clean after two accepted findings were fixed (map-spread lint; full-parse idempotency lookup replaced with the reverse early-exit scan)
PR CI green on the current head

clawsweeper · 2026-07-07T13:33:15Z

Codex review: needs maintainer review before merge. Reviewed July 7, 2026, 11:55 AM ET / 15:55 UTC.

Summary
The PR routes gateway chat transcript injection, source-reply mirror rewrites, and related transcript reads through session accessor helpers while ratcheting the session-accessor boundary baseline.

PR surface: Source +173, Tests +91, Other 0. Total +264 across 9 files.

Reproducibility: not applicable. this is a behavior-preserving refactor PR, not a bug report. The contributor supplied a live-output gateway proof showing current-main parity for the changed chat.inject persistence path.

Review metrics: 1 noteworthy metric.

Session Accessor Debt Ratchet: 1 writer file migrated, 1 baseline entry removed. The PR tightens the storage-neutral session boundary that the SQLite flip depends on, so maintainers should verify the ratchet is correct before merge.

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, unknown-data-model-change: src/config/sessions/session-accessor.test.ts, unknown-data-model-change: src/config/sessions/session-accessor.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: partial_overlap
Canonical: #88838
Summary: The canonical session/transcript SQLite migration tracker is open; this PR is a focused preparatory gateway accessor slice, while the broader storage-flip PR remains the larger implementation candidate.

Members:

canonical: Track core session/transcript SQLite migration via accessor seam #88838 - The tracker explicitly owns the core session/transcript SQLite migration and remaining accessor-to-storage flip acceptance criteria.
fixed_by_candidate: refactor: flip sessions and transcripts to sqlite storage #98236 - The open storage-flip PR is the broader candidate for the SQLite migration tracked by the canonical issue, but it is currently conflicting and marked do-not-merge.
adjacent_distinct: refactor(sessions): move parent-session forking behind the accessor boundary #101699 - That PR moves parent-session forking behind the accessor boundary, adjacent to but distinct from this PR's chat transcript injection slice.
adjacent_distinct: gateway: support user-role chat.inject with idempotent replays #81513 - That draft PR changes chat.inject API behavior for user-role and idempotent replay, while this PR is an internal 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: 🐚 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:

Refresh or confirm exact-head focused gateway/session-accessor proof after resolving the branch-behind-current-main state.

Risk before merge

[P1] This PR rewires transcript injection, idempotency lookup, source-reply mirror rewrites, and session updatedAt touches, so a regression could stale or mis-associate session transcript state.
[P1] The changed WebChat media/source-reply and chat.inject paths are message-delivery visible even though the intended behavior is unchanged.
[P1] The PR is mergeable but behind current main, so maintainers should verify the actual merge head or refreshed head before landing.

Maintainer options:

Refresh and land after exact-head proof (recommended)
Rebase or validate the actual merge head, rerun or confirm the focused gateway/server-methods, session-accessor, and boundary-guard checks, then merge if maintainers accept this as the right prep slice.
Defer to the storage flip
Keep this PR open or later close it if the broader SQLite storage-flip branch absorbs the same gateway accessor work cleanly.
Accept the accessor refactor risk
Maintainers can intentionally merge based on the supplied proof and green checks if they accept the transcript and delivery-path risk.

Next step before merge

[P2] Protected maintainer labeling plus session-state/message-delivery merge risk make this a human merge-review item rather than an automated repair candidate.

Maintainer decision needed

Question: Should maintainers land this gateway transcript accessor refactor now as prep for the SQLite storage flip after exact-head validation on the current main merge result?
Rationale: The patch appears to move persistence to the intended accessor boundary, but it touches session-state and message-delivery-sensitive runtime paths under a protected maintainer label.
Likely owner: jalehman — jalehman has the strongest recent history on the session-accessor migration slices and authored the current prep branch.
Options:
- Refresh and land after exact-head proof (recommended): Rebase or validate the actual merge head, rerun or confirm the focused gateway/server-methods, session-accessor, and boundary-guard checks, then merge if maintainers accept this as the right prep slice.
- Defer to the storage flip: Keep this PR open or later close it if the broader SQLite storage-flip branch absorbs the same gateway accessor work cleanly.
- Accept the refactor risk now: Maintainers may merge based on the PR-body proof, green selected CI, and source review if they intentionally accept the session transcript and delivery-path blast radius.

Security
Cleared: The diff touches TypeScript source, tests, and a local boundary/debt script only; I found no concrete security or supply-chain regression.

Review details

Best possible solution:

Land this accessor slice only after maintainer exact-head review confirms chat.inject, abort partials, source-reply media mirrors, updatedAt touches, and the boundary ratchet still preserve current behavior; otherwise deliberately defer it to the broader storage-flip lane.

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

Not applicable: this is a behavior-preserving refactor PR, not a bug report. The contributor supplied a live-output gateway proof showing current-main parity for the changed chat.inject persistence path.

Is this the best way to solve the issue?

Yes: routing transcript injection and mirror rewrites through the session accessor is the maintainable boundary for the SQLite migration prep. The safer alternative is not another custom gateway writer, but exact-head validation or deliberate deferral to the broader storage-flip branch.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. The contributor added copied live-output proof from a real gateway process with isolated state, public RPCs, and transcript/store inspection showing 17/17 parity checkpoints after the change.

Label justifications:

P2: This is a normal-priority internal gateway/session refactor with important correctness surface but no reported active user-facing outage.
merge-risk: 🚨 session-state: The diff rewires transcript injection, idempotency checks, source-reply mirror rewrites, and session updatedAt touches through the accessor.
merge-risk: 🚨 message-delivery: The changed WebChat media/source-reply and chat.inject paths can affect whether assistant messages are persisted, mirrored, or broadcast correctly.
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 (live_output): The contributor added copied live-output proof from a real gateway process with isolated state, public RPCs, and transcript/store inspection showing 17/17 parity checkpoints after the change.
proof: sufficient: Contributor real behavior proof is sufficient. The contributor added copied live-output proof from a real gateway process with isolated state, public RPCs, and transcript/store inspection showing 17/17 parity checkpoints after the change.

Evidence reviewed

PR surface:

Source +173, Tests +91, Other 0. Total +264 across 9 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	3	441	268	+173
Tests	4	217	126	+91
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	2	1	1	0
Total	9	659	395	+264

What I checked:

Repository policy applied: Root AGENTS.md and scoped scripts, gateway, gateway/server-methods, and test guides were read; the server-methods guide requires transcript message writes to preserve parentId chains through SessionManager.appendMessage or a wrapper that uses it. (src/gateway/server-methods/AGENTS.md:3, 3d206140f3f0)
Current main still has direct gateway debt: Current main still resolves transcript paths in chat.ts and reads transcript indexes directly before appending gateway assistant messages, so the central refactor is not already implemented on main. (src/gateway/server-methods/chat.ts:1725, 3d206140f3f0)
PR routes injected appends through accessor turn writer: The PR head persists injected assistant messages through persistSessionTranscriptTurn with store/session identity, caller-checked idempotency inside the write path, and touchSessionEntry instead of a separate direct store update. (src/gateway/server-methods/chat-transcript-inject.ts:202, 0dfff95be199)
PR adds storage-neutral read helpers: The PR head adds findTranscriptEvent for newest-first early-exit reads and loadTranscriptEvents for parsed transcript records through resolveSessionTranscriptReadTarget, with missing transcript files streaming as empty. (src/config/sessions/session-accessor.ts:2101, 0dfff95be199)
Source-reply mirror rewrites stay accessor-scoped: The PR head loads transcript events through the accessor, guards rewrite suffixes using visible transcript records, calls rewriteTranscriptEntriesInRuntimeTranscript with session scope, and touches the session marker after a changed rewrite. (src/gateway/server-methods/chat.ts:1968, 0dfff95be199)
Changed chat paths still call the accessor append helper: The PR head continues to call appendAssistantTranscriptMessage for WebChat media persistence and chat.inject, preserving the existing gateway entry points while changing the persistence boundary. (src/gateway/server-methods/chat.ts:4697, 0dfff95be199)

Likely related people:

jalehman: Recent merged history includes multiple session-accessor seam migrations and the session-accessor boundary debt ratchet that this PR continues. (role: recent session-accessor area contributor; confidence: high; commits: cb0d8a1294ea, 4e2a80cac34d, c7295e417d5d; files: src/config/sessions/session-accessor.ts, scripts/check-session-accessor-boundary.mjs, src/gateway/server-methods/chat-transcript-inject.ts)
steipete: Recent main commits touched gateway chat routing, session lifecycle, and queued prompt/session behavior adjacent to the changed chat.ts paths. (role: recent gateway chat/session contributor; confidence: medium; commits: a2a68d154b1b, 6df0fb818d67, 1b8d83767445; files: src/gateway/server-methods/chat.ts, src/config/sessions/session-accessor.ts)
vincentkoc: Recent commits touched session accessor cleanup and scoped store-path resolution adjacent to the accessor boundary used by this PR. (role: recent session-state adjacent contributor; confidence: medium; commits: c8d95da14c5b, ab534919ff0e, 10d1af6867fc; files: src/config/sessions/session-accessor.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 (2 earlier review cycles)

reviewed 2026-07-07T13:37:22.758Z sha 6a64c8b :: needs real behavior proof before merge. :: none
reviewed 2026-07-07T14:39:28.027Z sha 0dfff95 :: needs real behavior proof before merge. :: none

…n accessor

…rly-exit scan

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

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), operator client over WebSocket. Three runs: origin/main baseline (c8d95da14c), this branch (0dfff95be1), and the #101699 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  A0 sessions.create persisted parent entry :: {"sessionId":"012ecd2c-..."}
PASS  A1 chat.inject returned message ids :: {"first":"34dcb4e0-...","second":"3a695e5f-..."}
PASS  A2 session entry names transcript artifact :: {"sessionFile":"<state>/agents/main/sessions/012ecd2c-....jsonl"}
PASS  A3 transcript has header + both injected rows :: {"records":3,"messages":2}
PASS  A4 injected rows carry gateway-injected provenance :: {"provider":"openclaw","model":"gateway-injected"}
PASS  A5 label prefix rendered into first row content :: {"content":"[{\"type\":\"text\",\"text\":\"[proof]\\n\\nfirst injected assistant message\"}]"}
PASS  A6 second row parent-links to first (persist turn chain) :: secondParentId === firstId
PASS  A7 session marker touched through accessor (updatedAt advanced) :: {"before":1783437191177,"after":1783437191203}
PASS  A8 chat.history replays both injected messages :: {"messages":2}
PASS  C1 no orphan transcript artifacts in sessions dir :: {"orphans":[]}

What this proves for the accessor-routed injection path: chat.inject persists real assistant rows through persistSessionTranscriptTurn with correct provenance markers, label rendering, and parentId chaining across consecutive injects; the session store marker (updatedAt) advances through the accessor touch instead of the removed direct updateSessionStoreEntry; chat.history replays the rows; and no stray transcript artifacts appear outside store-referenced files (the deleted local resolveTranscriptPath/ensureTranscriptFile helpers left no orphan-writer behind).

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]>

…n accessor (openclaw#101688) * refactor(gateway): route chat transcript injection through the session accessor * fix(gateway): avoid map-spread in source reply mirror rewrite results * perf(sessions): find injected transcript duplicates with a reverse early-exit scan

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]>

…n accessor (openclaw#101688) * refactor(gateway): route chat transcript injection through the session accessor * fix(gateway): avoid map-spread in source reply mirror rewrite results * perf(sessions): find injected transcript duplicates with a reverse early-exit scan

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 app: web-ui App: web-ui gateway Gateway runtime scripts Repository scripts size: XL maintainer Maintainer-authored PR labels Jul 7, 2026

jalehman added 3 commits July 7, 2026 07:22

refactor(gateway): route chat transcript injection through the sessio…

218847a

…n accessor

fix(gateway): avoid map-spread in source reply mirror rewrite results

643b563

perf(sessions): find injected transcript duplicates with a reverse ea…

0dfff95

…rly-exit scan

jalehman force-pushed the b62.4/chat-session-accessor branch from 6a64c8b to 0dfff95 Compare July 7, 2026 14:22

jalehman mentioned this pull request Jul 7, 2026

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

Merged

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jul 7, 2026

jalehman merged commit 453f596 into main Jul 7, 2026
117 checks passed

jalehman deleted the b62.4/chat-session-accessor branch July 7, 2026 16:10

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

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

Open

clawsweeper Bot mentioned this pull request Jul 7, 2026

Add reset family carryover summaries #90259

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

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

refactor(gateway): route chat transcript injection through the session accessor#101688
jalehman merged 3 commits into
mainfrom
b62.4/chat-session-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

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

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