Skip to content

refactor: add SQLite transcript identity adapter#90437

Closed
jalehman wants to merge 12 commits into
mainfrom
clawdbot-d02.1.9.1.21/32-sqlite-transcript-identity-state-adapter
Closed

refactor: add SQLite transcript identity adapter#90437
jalehman wants to merge 12 commits into
mainfrom
clawdbot-d02.1.9.1.21/32-sqlite-transcript-identity-state-adapter

Conversation

@jalehman

@jalehman jalehman commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

What

Adds the SQLite-side transcript runtime identity/state adapter for the canonical transcript identity contract. This keeps the storage-neutral transcript runtime API shape while adding temp-DB backed SQLite coverage for the future storage flip.

Why

Path 3 needs the file-backed transcript identity contract and SQLite implementation to agree before transcript readers, writers, and plugin/memory consumers can be validated under SQLite without depending on filesystem sessionFile identity.

Changes

  • Adds SQLite transcript runtime state adapter
  • Adds session accessor conformance coverage
  • Extends agent DB transcript schema
  • Preserves file-backed transcript behavior

Testing

  • node scripts/run-vitest.mjs src/agents/embedded-agent-runner/transcript-runtime-state.test.ts src/config/sessions/session-accessor.conformance.test.ts src/config/sessions/session-accessor.test.ts
  • .agents/skills/autoreview/scripts/autoreview --mode branch --base clawdbot-d02.1.9.1.15.1/31b-transcript-identity-state-contract --no-web-search --thinking codex=low

Refs #88838

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XL maintainer Maintainer-authored PR labels Jun 4, 2026
@jalehman jalehman mentioned this pull request Jun 4, 2026
Closed
@clawsweeper

clawsweeper Bot commented Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 21, 2026, 8:55 AM ET / 12:55 UTC.

Summary
The PR adds a SQLite-backed session/transcript accessor and runtime transcript state adapter, agent DB transcript/session tables, runtime helper seams, and file/SQLite conformance tests for the Path 3 storage migration.

PR surface: Source +2392, Tests +1652, Generated +91. Total +4135 across 17 files.

Reproducibility: yes. for the review findings at source level. Current main rejects raw message events and uses a non-mutating runtime read target, while the PR head accepts raw SQLite/file-backed message events and routes read probes through the write target.

Review metrics: 2 noteworthy metrics.

  • Persistent schema surfaces: 4 agent DB tables added. The PR adds persisted session/transcript tables, so upgrade, migration, and rollback behavior matter before merge.
  • Current-main schema drift: 7 current-main agent DB tables absent from PR-head schema. The stale branch must rebase and preserve current auth-profile and memory-index schema before transcript schema review is safe.

Stored data model
Persistent data-model change detected: database schema: src/state/openclaw-agent-schema.generated.ts, database schema: src/state/openclaw-agent-schema.sql, migration/backfill/repair: src/agents/embedded-agent-runner/transcript-runtime-state.sqlite.ts, serialized state: src/agents/embedded-agent-runner/transcript-file-state.ts, serialized state: src/config/sessions/session-accessor.conformance.test.ts, serialized state: src/config/sessions/session-accessor.sqlite.ts, and 14 more. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #88838
Summary: This PR is an active implementation slice under the canonical session/transcript SQLite migration tracker; related merged slices cover adjacent seams, while the SQLite foundation remains a separate open overlapping PR.

Members:

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
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:

  • Rebase or restack onto current main and the active SQLite foundation while preserving current agent DB schema.
  • Restore the raw-message guard and non-mutating read-target behavior with focused regression coverage.
  • [P1] Add redacted real behavior or upgrade proof from an actual SQLite adapter setup.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body lists focused tests and autoreview only; it needs redacted terminal output, logs, live output, or a linked artifact from a real SQLite adapter setup before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] The PR is draft, protected by the maintainer label, and currently conflicting, so it needs maintainer-owned sequencing and a current-main rebase before any merge path.
  • [P1] The branch changes persistent agent DB schema and transcript identity semantics; upgrade, migration, and current-main schema preservation need explicit proof before merge.
  • [P1] The SQLite raw event path can persist message records outside the current redaction, idempotency, and parent-link append contract.
  • [P1] The PR body lists focused tests and autoreview, but no after-fix real environment proof for the SQLite adapter path.

Maintainer options:

  1. Rebase and fix storage invariants (recommended)
    Refresh onto current main and the active foundation, preserve existing agent DB tables, reject raw message events, keep read probes non-mutating, and add focused tests plus proof.
  2. Keep as a draft stack slice
    Maintainers can leave this open as source work while the foundation and sibling Path 3 slices settle, without treating it as a landing candidate.
  3. Pause if the foundation absorbs it
    If the open foundation PR absorbs the useful SQLite adapter work, close or replace this branch after the replacement is proof-positive and viable.

Next step before merge

  • [P1] Protected draft storage-migration work has merge conflicts, schema drift, and P1 storage/security findings, so it needs maintainer sequencing rather than an automated repair lane.

Security
Needs attention: The diff introduces a concrete transcript security concern because SQLite raw message-event writes can bypass the redaction path used by normal message appends.

Review findings

  • [P1] Route raw message events through the message append seam — src/config/sessions/session-accessor.sqlite.ts:411-419
  • [P1] Keep read probes on the non-mutating resolver — src/agents/embedded-agent-runner/transcript-runtime-state.ts:39
Review details

Best possible solution:

Restack this adapter onto current main and the active SQLite foundation, preserve current schema and transcript read/write invariants, then merge only after focused tests plus real SQLite adapter and upgrade proof.

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

Yes for the review findings at source level. Current main rejects raw message events and uses a non-mutating runtime read target, while the PR head accepts raw SQLite/file-backed message events and routes read probes through the write target.

Is this the best way to solve the issue?

No. The branch-by-abstraction direction aligns with the tracker, but this branch is not the best merge shape until it is rebased, preserves current-main schema, fixes the raw-message and read-probe invariants, and supplies real proof.

Full review comments:

  • [P1] Route raw message events through the message append seam — src/config/sessions/session-accessor.sqlite.ts:411-419
    Current main rejects type: "message" in raw transcript-event appenders because message writes need parent-link generation, idempotency, and redaction. This SQLite adapter forwards arbitrary events into appendTranscriptEventInTransaction, creating a second message-write path that can persist unredacted or mis-parented messages.
    Confidence: 0.91
  • [P1] Keep read probes on the non-mutating resolver — src/agents/embedded-agent-runner/transcript-runtime-state.ts:39
    Runtime read/existence/delete helpers should not persist missing sessionFile metadata into the session store. Current main has a separate read-target resolver for that contract, but this branch calls the write target from read/probe helpers, so probing a missing transcript can mutate session state.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a normal-priority internal storage migration slice with high blast radius but no immediate user outage.
  • merge-risk: 🚨 compatibility: The PR changes persistent agent DB schema and is stale against current-main schema, so upgrades could break if merged incorrectly.
  • merge-risk: 🚨 session-state: The patch touches transcript identity, runtime read/probe behavior, session metadata, and compaction transcript state.
  • merge-risk: 🚨 security-boundary: The SQLite adapter exposes raw message-event storage that can bypass the current redacting message append path.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • 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 lists focused tests and autoreview only; it needs redacted terminal output, logs, live output, or a linked artifact from a real SQLite adapter setup before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +2392, Tests +1652, Generated +91. Total +4135 across 17 files.

View PR surface stats
Area Files Added Removed Net
Source 12 2398 6 +2392
Tests 3 1652 0 +1652
Docs 0 0 0 0
Config 0 0 0 0
Generated 2 91 0 +91
Other 0 0 0 0
Total 17 4141 6 +4135

Security concerns:

  • [medium] Raw SQLite message events bypass redaction — src/config/sessions/session-accessor.sqlite.ts:1244
    The SQLite event appender stores arbitrary message events with JSON.stringify(event) instead of routing message payloads through the redacting message append path, which can persist sensitive message content that the canonical path would redact.
    Confidence: 0.88

What I checked:

  • Repository policy applied: Root AGENTS.md was read fully, and its storage/session-state compatibility rules shaped this review; scoped src/agents and src/gateway policies were also read because the PR touches those paths. (AGENTS.md:30, c85113e30e2a)
  • Live PR state: Live GitHub state shows this PR is open, draft, conflicting, and protected by the maintainer label, with compatibility, session-state, security-boundary, and needs-proof labels already present. (ab1fa5aa2c50)
  • Current main lacks this adapter: Current main has no session-accessor.sqlite.ts or transcript-runtime-state.sqlite.ts files, so the central SQLite adapter work is not implemented on main. (c85113e30e2a)
  • Current raw event contract: Current main requires message transcript records to use appendTranscriptMessage so parent links, idempotency, and redaction are preserved, and rejects raw message events. (src/config/sessions/session-accessor.ts:878, c85113e30e2a)
  • Current raw-event regression coverage: Current main has a test that raw message transcript events throw and do not create the transcript file. (src/config/sessions/session-accessor.test.ts:1341, c85113e30e2a)
  • PR raw SQLite event path: PR head exposes appendSqliteTranscriptEvent and appendSqliteTranscriptEvents that forward arbitrary events into the SQLite transaction path. (src/config/sessions/session-accessor.sqlite.ts:411, ab1fa5aa2c50)

Likely related people:

  • jalehman: Authored the canonical Path 3 tracker, this PR head, and merged seam slices that define the session/transcript accessor and reader/writer contracts this adapter builds on. (role: feature owner and recent area contributor; confidence: high; commits: ab1fa5aa2c50, ef47dd610c87, d216f7c876dd; files: src/config/sessions/session-accessor.ts, src/config/sessions/session-accessor.sqlite.ts, src/agents/embedded-agent-runner/transcript-runtime-state.ts)
  • steipete: Authored the earlier merged SQLite session metadata migration that the tracker names as the postmortem and salvage source for the staged reland. (role: adjacent storage migration contributor; confidence: medium; commits: 538d36eaaaa6; files: src/config/sessions/store-sqlite.ts, src/commands/session-state-migration.ts, src/state/openclaw-agent-db.ts)
  • vincentkoc: Local blame in the shallow checkout attributes the current checked non-message guard and runtime read-target implementation to recent current-main import commits; the shallow grafted history limits ownership confidence. (role: recent current-main contributor; confidence: low; commits: c85113e30e2a, 35d7cb0bffe7; files: src/config/sessions/session-accessor.ts, src/agents/embedded-agent-runner/transcript-runtime-state.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: 🧂 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. 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 4, 2026
@jalehman
jalehman force-pushed the clawdbot-d02.1.9.1.15.1/31b-transcript-identity-state-contract branch from a5012a3 to a0e06d4 Compare June 4, 2026 21:54
@jalehman jalehman mentioned this pull request Jun 8, 2026
Closed
@jalehman
jalehman force-pushed the clawdbot-d02.1.9.1.15.1/31b-transcript-identity-state-contract branch 2 times, most recently from 8432a1b to efbe88d Compare June 14, 2026 14:24
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. label Jun 14, 2026
@jalehman
jalehman force-pushed the clawdbot-d02.1.9.1.15.1/31b-transcript-identity-state-contract branch 3 times, most recently from cb112eb to 57aa52c Compare June 15, 2026 19:00
Base automatically changed from clawdbot-d02.1.9.1.15.1/31b-transcript-identity-state-contract to main June 15, 2026 20:04
@openclaw-barnacle openclaw-barnacle Bot added the gateway Gateway runtime label Jun 15, 2026
@clawsweeper clawsweeper Bot 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 16, 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. labels Jun 16, 2026
@jalehman jalehman mentioned this pull request Jun 16, 2026
Closed
This was referenced Jun 19, 2026
Merged
Merged
Closed
@jalehman

jalehman commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #96625. The SQLite storage flip PR now carries the remaining Path 3 implementation line, including the foundation/adapter work that this older 3.2 PR was meant to stage. Please use #96625 and tracker #88838 for remaining review/proof instead of rebasing this stale split PR.

@jalehman jalehman closed this Jun 28, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 30, 2026
Merged
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

agents Agent runtime and tooling gateway Gateway runtime maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. 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. 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