refactor: route SDK session compatibility through seam#89203
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 18, 2026, 8:04 PM ET / 00:04 UTC. Summary PR surface: Source +314, Tests +273, Docs +2, Generated 0. Total +589 across 11 files. Reproducibility: not applicable. this is a refactor/compatibility PR, not a bug report with a failing reproduction path. The PR body includes live Gateway proof, and the diff adds focused regression tests for the compatibility helpers. Review metrics: 2 noteworthy metrics.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land this compatibility slice only after the session/transcript migration owner confirms the deprecation window and accessor-backed write semantics match the SQLite flip sequence. Do we have a high-confidence way to reproduce the issue? Not applicable: this is a refactor/compatibility PR, not a bug report with a failing reproduction path. The PR body includes live Gateway proof, and the diff adds focused regression tests for the compatibility helpers. Is this the best way to solve the issue? Yes, with maintainer sequencing review: routing through the existing accessor seam is the narrow owner-boundary fix, and keeping the SQLite flip, doctor migration, and transcript identity APIs out of this PR avoids the monolithic migration failure mode tracked in the canonical issue. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 2a0e63d12bde. Label changesLabel justifications:
Evidence reviewedPR surface: Source +314, Tests +273, Docs +2, Generated 0. Total +589 across 11 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
f18f99c to
b39023f
Compare
7409398 to
dfb367a
Compare
890d5e3 to
b496b4b
Compare
dfb367a to
4b52864
Compare
|
Follow-up: the plugin SDK API baseline needs regeneration on this branch ( |
1e31abd to
094f9b5
Compare
fad01c4 to
278c1f9
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
What
Routes the public plugin SDK and plugin runtime session-entry helpers through the file-backed session accessor seam while keeping legacy whole-store and session-file helpers as explicit deprecated compatibility exports.
Why
Path 3 needs SDK-facing session entry operations to stop depending on the mutable
sessions.jsonwhole-store shape before the SQLite storage flip, without breaking existing plugin contracts that still import whole-store or transcript-file helpers.Changes
Testing
node scripts/run-vitest.mjs src/plugin-sdk/session-transcript-hit.test.ts src/plugin-sdk/session-store-runtime.test.ts src/plugin-sdk/config-runtime.test.ts src/plugins/runtime/index.test.ts src/config/sessions/session-accessor.test.tspnpm tsgo:corepnpm tsgo:core:testpnpm plugin-sdk:api:genpnpm plugin-sdk:api:checknode scripts/run-oxlint.mjs <touched SDK/runtime files>git diff --checkpnpm build/Users/phaedrus/Projects/prompts/skills/autoreview/scripts/autoreview --mode localTracker: #88838
Scope notes:
clawdbot-d02.1.9.1.20/31b-public-sdk-transcript-identity-apishould define the future public SDK transcript identity API beforesession-transcript-hitor plugin transcript helpers move off legacy basename/session-file matching.Real behavior proof
Behavior addressed: SDK/runtime session-entry compatibility helpers still persist plugin-owned session entry updates after routing through the session accessor seam, while the live Gateway uses the rebased build.
Real environment tested: Local LaunchAgent Gateway from this checkout, OpenClaw 2026.6.8 (278c1f9), loopback port 18789,
active-memorytemporarily enabled for agentmainand then restored from backup.Exact steps or command run after this patch:
pnpm build;pnpm openclaw gateway restart;curl -fsS http://127.0.0.1:18789/readyz;curl -fsS http://127.0.0.1:18789/healthz;pnpm openclaw agent --session-key agent:main:telegram:default:direct:path3-89203-compat-main --channel telegram --to path3-89203-compat-main --message "Reply exactly: path3 sdk channel proof" --model openai/gpt-5.4-mini --timeout 180 --json; targeted session-store read for that one key.Evidence after fix: Gateway returned
ready=trueandhealthzreturned{"ok":true,"status":"live"}. The live agent turn completed withstatus: ok,runId: b3edc3c5-1868-4755-85e6-03b84e535bb0,sessionId: b6c4bc32-76ac-473a-83f7-c8be1c70894a, provider/modelopenai/gpt-5.4-mini, and assistant textpath3 sdk channel proof.Observed result after fix: The persisted proof session row existed with
channel: telegram, delivery context{ channel: telegram, to: path3-89203-compat-main }, and exactly oneactive-memoryplugin debug entry:🧩 Active Memory: status=unavailable elapsed=319ms query=recent. This entry is written byactive-memorythroughapi.runtime.agent.session.patchSessionEntry, proving the deprecated runtime session compatibility wrapper still writes through the accessor-backed path in a live Gateway turn.What was not tested: A real external Telegram delivery was intentionally not sent (
--deliverwas omitted), and the future SQLite storage flip/doctor migration remains out of scope for this compatibility slice. Temporary proof config/session rows were cleaned up and the Gateway was restarted back to the original active-memory config.