Skip to content

refactor: route plugin SDK session entries through seam#89204

Closed
jalehman wants to merge 23 commits into
mainfrom
clawdbot-55f/31b-plugin-sdk-compat-seam
Closed

refactor: route plugin SDK session entries through seam#89204
jalehman wants to merge 23 commits into
mainfrom
clawdbot-55f/31b-plugin-sdk-compat-seam

Conversation

@jalehman

@jalehman jalehman commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

What

Routes the Plugin SDK and plugin runtime session entry helpers through the 3.1a session accessor seam for entry-level read/list/patch/update/upsert behavior, while preserving the existing public object-parameter API shape.

Refs #88838. Depends on #88840.

Why

This is one Path 3 3.1b caller-adoption slice: plugin-facing session entry APIs need to stop depending directly on file-backed store helpers before the session storage implementation can be validated and later flipped behind the seam.

Changes

  • Route SDK entry reads
  • Route runtime entry reads
  • Route patch/update/upsert
  • Preserve preserveActivity
  • Refresh SDK API baseline

Scope

Included: entry-level SDK/runtime getSessionEntry, listSessionEntries, patchSessionEntry, updateSessionStoreEntry, and upsertSessionEntry, plus 3.1a seam support for preserveActivity.

Excluded: transcript identity, sessionFile identity, public whole-store compatibility modernization, SQLite validation branch wiring, storage flip, doctor migration, and runtime dual-read/fallback behavior.

Testing

  • node scripts/run-vitest.mjs src/plugin-sdk/session-store-runtime.test.ts src/plugins/runtime/index.test.ts extensions/discord/src/monitor/native-command.plugin-dispatch.test.ts extensions/voice-call/src/response-generator.test.ts
  • node scripts/run-vitest.mjs src/config/sessions/session-accessor.test.ts src/plugin-sdk/session-store-runtime.test.ts src/plugins/runtime/index.test.ts extensions/discord/src/monitor/native-command.plugin-dispatch.test.ts extensions/voice-call/src/response-generator.test.ts
  • node scripts/run-vitest.mjs src/plugin-sdk/session-store-runtime.test.ts src/plugins/runtime/index.test.ts
  • node scripts/run-oxlint.mjs src/config/sessions/session-accessor.ts src/config/sessions/session-accessor.test.ts src/plugin-sdk/config-runtime.ts src/plugin-sdk/session-store-runtime.ts src/plugin-sdk/session-store-runtime.test.ts src/plugins/runtime/runtime-agent.ts src/plugins/runtime/types-core.ts src/plugins/runtime/index.test.ts
  • pnpm tsgo:core
  • pnpm tsgo:core:test
  • pnpm plugin-sdk:api:check
  • git diff --check upstream/clawdbot-9c3/session-accessor-seam...HEAD
  • Source branch autoreview passed after the last code change; disposable SQLite integration autoreview remains blocked and is intentionally out of this PR.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation size: M maintainer Maintainer-authored PR labels Jun 1, 2026
@jalehman jalehman mentioned this pull request Jun 1, 2026
Closed
@clawsweeper

clawsweeper Bot commented Jun 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 14, 2026, 12:48 AM ET / 04:48 UTC.

Summary
The PR reroutes plugin SDK and plugin runtime session entry read/list/patch/update/upsert helpers through the file-backed session accessor seam and adds accessor/boundary tests plus an SDK API checksum update.

PR surface: Source +1072, Tests +761, Config +6, Generated 0, Other +146. Total +1985 across 22 files.

Reproducibility: not applicable. This is a refactor/migration slice rather than a bug report; the review proof path is compatibility validation for plugin/runtime session helper behavior.

Review metrics: 1 noteworthy metric.

  • Plugin SDK session helper surface: 5 existing helpers rerouted, 0 removed. These helpers are plugin-facing session contracts, so compatibility and upgrade behavior need explicit proof before merge.

Stored data model
Persistent data-model change detected: serialized state: src/config/sessions/combined-store-gateway.ts, serialized state: src/config/sessions/session-accessor.test.ts, serialized state: src/config/sessions/session-accessor.ts, serialized state: src/config/sessions/store.ts, serialized state: src/config/sessions/transcript-append.ts, serialized state: src/gateway/server-methods/sessions.ts, and 15 more. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof is added.

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

Rank-up moves:

  • Carry requireWriteSuccess and any other current-main updateSessionStoreEntry options through the seam-backed SDK/runtime wrappers or document an explicit compatibility decision.
  • Regenerate the plugin SDK API baseline and rerun pnpm plugin-sdk:api:check.
  • [P1] Add redacted real terminal/log proof for a plugin/runtime session entry read and write path.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body provides tests/type/lint/API checks but no after-fix real setup output; add redacted terminal/log proof for a plugin/runtime session path and update the PR body so ClawSweeper can re-review.

Risk before merge

  • [P1] The new SDK/runtime wrappers can drift from the current public plugin SDK contract; requireWriteSuccess is present on current main but not carried through the PR wrapper type or seam options.
  • [P1] Session entry reads and writes move behind a new seam, so alias resolution, cloning, updatedAt preservation, and replace-vs-merge behavior need upgrade proof to avoid stale or mis-associated session state.
  • [P1] The branch is draft, merge-conflicting, depends on refactor: add session accessor seam with gateway consumer #90463, and the author explicitly says the SDK API baseline must be regenerated before plugin-sdk:api:check can pass.
  • [P1] The PR body lists focused tests and checks but does not include after-fix output from a real plugin/runtime session entry path.

Maintainer options:

  1. Refresh and preserve the SDK contract (recommended)
    Rebase onto the viable seam base/current main, carry current updateSessionStoreEntry options such as requireWriteSuccess through the wrapper/seam, regenerate the SDK API baseline, and rerun the focused SDK/runtime checks.
  2. Require live plugin-session proof
    Before merge, add redacted terminal or log proof that a real plugin/runtime path reads, patches, updates, and replaces session entries without losing fields or activity timestamps.
  3. Pause behind the base seam
    If refactor: add session accessor seam with gateway consumer #90463 changes shape or stalls, keep this stack slice paused rather than carrying a conflicting SDK branch.

Next step before merge

  • [P1] This needs maintainer/author handling because the PR is draft, conflicting, protected by maintainer, depends on the unsettled seam stack, and needs proof plus SDK compatibility decisions before merge.

Security
Cleared: No concrete security or supply-chain regression was found; the CI change adds a repo-local AST guard and no new external action, permission, secret, dependency, or artifact download.

Review findings

  • [P1] Forward requireWriteSuccess through the session wrapper — src/plugin-sdk/session-store-runtime.ts:44-50
  • [P2] Regenerate the SDK API baseline — docs/.generated/plugin-sdk-api-baseline.sha256:1-2
Review details

Best possible solution:

Land this only after the base seam is settled, the branch is refreshed against current main, SDK API option parity is preserved, generated baselines are regenerated, and redacted real plugin/runtime session proof is added.

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

Not applicable. This is a refactor/migration slice rather than a bug report; the review proof path is compatibility validation for plugin/runtime session helper behavior.

Is this the best way to solve the issue?

No, not yet. The accessor seam is the right layer for the migration plan, but this PR needs branch refresh, SDK API parity, baseline regeneration, and real behavior proof before it is the best merge candidate.

Full review comments:

  • [P1] Forward requireWriteSuccess through the session wrapper — src/plugin-sdk/session-store-runtime.ts:44-50
    Current main exposes updateSessionStoreEntry through the plugin SDK with a requireWriteSuccess?: boolean option, but this new wrapper type omits that option and only forwards skipMaintenance/takeCacheOwnership to the seam. That would drop a current public SDK/session safety option after rebase unless the seam and wrapper carry it forward or the PR includes an explicit deprecation/compatibility decision.
    Confidence: 0.86
  • [P2] Regenerate the SDK API baseline — docs/.generated/plugin-sdk-api-baseline.sha256:1-2
    The author notes that the plugin SDK API baseline is stale after the wave-1 rebase and that plugin-sdk:api:check will fail until it is regenerated. Because this PR changes SDK session helper exports and the checksum file, the branch should not merge with a known-stale generated API baseline.
    Confidence: 0.83

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a normal-priority migration/refactor slice with limited direct user blast radius but meaningful plugin SDK and session-state review risk.
  • merge-risk: 🚨 compatibility: The diff changes behavior behind public plugin SDK/runtime session helpers and currently misses a current-main option on the wrapper surface.
  • merge-risk: 🚨 session-state: The diff reroutes session entry read/write helpers where alias resolution, cloning, activity timestamps, and replacement semantics affect persisted session state.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • 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 provides tests/type/lint/API checks but no after-fix real setup output; add redacted terminal/log proof for a plugin/runtime session path and update the PR body so ClawSweeper can re-review.
Evidence reviewed

PR surface:

Source +1072, Tests +761, Config +6, Generated 0, Other +146. Total +1985 across 22 files.

View PR surface stats
Area Files Added Removed Net
Source 12 1125 53 +1072
Tests 6 794 33 +761
Docs 0 0 0 0
Config 2 6 0 +6
Generated 1 2 2 0
Other 1 146 0 +146
Total 22 2073 88 +1985

What I checked:

  • Live PR state: Live GitHub metadata shows this PR is open, draft, merge-conflicting, based on clawdbot-9c3-ce2/session-accessor-gateway-entry, and labeled maintainer, merge-risk: 🚨 compatibility, merge-risk: 🚨 session-state, and status: 📣 needs proof. (51d8d95f7ab7)
  • Current main behavior: Current main still re-exports the plugin SDK session entry helpers directly from ../config/sessions/store.js, so the central accessor-seam routing is not already implemented on main. (src/plugin-sdk/session-store-runtime.ts:23, 231b5a14d5d7)
  • Current public option contract: Current main's updateSessionStoreEntry accepts requireWriteSuccess?: boolean, and the current SDK export exposes that type through the store function. (src/config/sessions/store.ts:972, 231b5a14d5d7)
  • PR implementation surface: The PR head adds SDK wrapper types for updateSessionStoreEntry, but the wrapper params include only skipMaintenance and takeCacheOwnership, so the current-main requireWriteSuccess option is not carried through this new public wrapper shape. (src/plugin-sdk/session-store-runtime.ts:44, 51d8d95f7ab7)
  • Runtime caller boundary: The PR head reroutes plugin runtime session helpers through local wrappers over session-accessor, which is the session-state-sensitive runtime path reviewers need to validate. (src/plugins/runtime/runtime-agent.ts:84, 51d8d95f7ab7)
  • Accessor callee: The PR head's seam delegates entry reads, patch/update, and replacement to file-backed session store helpers while presenting storage-neutral accessor functions. (src/config/sessions/session-accessor.ts:148, 51d8d95f7ab7)

Likely related people:

  • jalehman: Authored this PR, the linked session/transcript SQLite migration tracker, and the open base seam PR that this stack slice depends on. (role: migration stack coordinator and likely follow-up owner; confidence: high; commits: 094f9b57d6d2, 51d8d95f7ab7; files: src/config/sessions/session-accessor.ts, src/plugin-sdk/session-store-runtime.ts, src/plugins/runtime/runtime-agent.ts)
  • steipete: GitHub history shows the current plugin SDK session entry workflow helpers were added in cabb5538, touching src/config/sessions/store.ts, src/plugin-sdk/session-store-runtime.ts, and src/plugins/runtime/runtime-agent.ts. (role: current API introducer and recent area contributor; confidence: high; commits: cabb55380f84; files: src/config/sessions/store.ts, src/plugin-sdk/session-store-runtime.ts, src/plugins/runtime/runtime-agent.ts)
  • vincentkoc: GitHub history shows the recent revert that deferred session metadata SQLite and preserved file-backed session helper behavior touched central session store and SDK files. (role: recent session storage contributor; confidence: medium; commits: 7f1d82ab2518; files: src/config/sessions/store.ts, src/plugin-sdk/session-store-runtime.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: 🐚 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. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 1, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 1, 2026
Merged
@jalehman
jalehman force-pushed the clawdbot-9c3/session-accessor-seam branch from f18f99c to b39023f Compare June 4, 2026 20:31
@jalehman
jalehman changed the base branch from clawdbot-9c3/session-accessor-seam to clawdbot-9c3-ce2/session-accessor-gateway-entry June 4, 2026 21:51
@jalehman
jalehman force-pushed the clawdbot-55f/31b-plugin-sdk-compat-seam branch from a617bb7 to 3c7b433 Compare June 4, 2026 21:51
@openclaw-barnacle openclaw-barnacle Bot removed the docs Improvements or additions to documentation label Jun 4, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed 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. labels Jun 4, 2026
@jalehman
jalehman force-pushed the clawdbot-9c3-ce2/session-accessor-gateway-entry branch from a13e0eb to 890d5e3 Compare June 4, 2026 22:37
@jalehman
jalehman force-pushed the clawdbot-9c3-ce2/session-accessor-gateway-entry branch from 890d5e3 to b496b4b Compare June 10, 2026 22:16
@jalehman
jalehman force-pushed the clawdbot-55f/31b-plugin-sdk-compat-seam branch from 3c7b433 to 51d8d95 Compare June 11, 2026 05:04
@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 11, 2026
@jalehman

jalehman commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up: the plugin SDK API baseline needs regeneration on this branch (pnpm plugin-sdk:api:gen) after the wave-1 rebase — plugin-sdk:api:check CI will fail until then. Pending a pnpm-capable environment; tracked in the wave-1 status on #88838.

@jalehman jalehman mentioned this pull request Jun 11, 2026
Merged
@jalehman
jalehman force-pushed the clawdbot-9c3-ce2/session-accessor-gateway-entry branch from 1e31abd to 094f9b5 Compare June 13, 2026 23:20
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels Jun 14, 2026
@jalehman
jalehman force-pushed the clawdbot-9c3-ce2/session-accessor-gateway-entry branch 3 times, most recently from 9a55d1b to 58aa59e Compare June 14, 2026 13:13
@jalehman
jalehman deleted the branch main June 14, 2026 13:18
Base automatically changed from clawdbot-9c3-ce2/session-accessor-gateway-entry to main June 14, 2026 13:18
@jalehman jalehman closed this Jun 14, 2026
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation gateway Gateway runtime scripts Repository scripts size: XL and removed size: M labels Jun 14, 2026
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

docs Improvements or additions to documentation gateway Gateway runtime maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. scripts Repository scripts size: XL 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

@jalehman