Skip to content

refactor: add scoped transcript target writers#89262

Closed
jalehman wants to merge 3 commits into
clawdbot-d02.1.9.1.20/31b-public-sdk-transcript-identity-apifrom
clawdbot-d02.1.9.1.24/31b-public-sdk-transcript-target-writer-api
Closed

refactor: add scoped transcript target writers#89262
jalehman wants to merge 3 commits into
clawdbot-d02.1.9.1.20/31b-public-sdk-transcript-identity-apifrom
clawdbot-d02.1.9.1.24/31b-public-sdk-transcript-target-writer-api

Conversation

@jalehman

@jalehman jalehman commented Jun 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What

Adds public SDK transcript target and writer helpers on top of the transcript identity API. The PR lets callers bind scoped transcript identity to an active sessionFile target and then read, append, publish, or run locked transcript work against that same target.

Refs #88838. Stacked on #89261.

Why

Plugin and memory transcript consumers cannot safely migrate with read-only scoped identity APIs. Several callers still lock and append to an active transcript file; if reads resolve a different target than writes, mirror writers and history/projector paths can miss existing events or duplicate messages. This PR defines the missing target/writer contract without flipping storage.

Changes

  • Add scoped transcript target resolver
  • Add active target binding
  • Add scoped append helper
  • Add scoped update publisher
  • Add scoped write-lock helper
  • Preserve malformed-line tolerance
  • Cover active writer regressions

Testing

  • node scripts/run-vitest.mjs src/plugin-sdk/session-transcript-runtime.test.ts - 9 passed
  • node scripts/run-tsgo.mjs -p tsconfig.core.json ... - passed
  • node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.json ... - passed
  • node scripts/sync-plugin-sdk-exports.mjs --check - passed
  • node scripts/check-plugin-sdk-subpath-exports.mjs - passed
  • node --max-old-space-size=8192 --import tsx scripts/generate-plugin-sdk-api-baseline.ts --check - passed
  • git diff --check - passed
  • local autoreview - clean

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

clawsweeper Bot commented Jun 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 19, 2026, 4:07 AM ET / 08:07 UTC.

Summary
Adds public plugin SDK transcript target, read/write, publish, and write-lock helpers with docs, generated API hash updates, and focused transcript runtime tests.

PR surface: Source +195, Tests +257, Docs 0, Generated 0. Total +452 across 7 files.

Reproducibility: not applicable. This is an additive public SDK/API migration slice rather than a bug; source inspection shows current main does not already provide the requested public transcript runtime helpers.

Review metrics: 1 noteworthy metric.

  • Public SDK transcript surface: 4 helpers added, 1 file-shaped parameter added. These become third-party plugin contracts once released, so deprecation and migration-boundary wording matters before merge.

Stored data model
Persistent data-model change detected: serialized state: src/config/sessions/session-accessor.ts, serialized state: src/config/sessions/transcript-append.ts, serialized state: src/plugin-sdk/session-transcript-runtime.test.ts, serialized state: src/plugin-sdk/session-transcript-runtime.ts, unknown-data-model-change: src/config/sessions/session-accessor.ts, unknown-data-model-change: src/config/sessions/transcript-append.ts, and 2 more. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: partial_overlap
Canonical: #88838
Summary: This PR is one SDK transcript writer slice in the broader Path 3 session/transcript SQLite migration stack tracked by #88838, with overlapping external transcript-write API requests and downstream stack PRs.

Members:

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

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🌊 off-meta tidepool
Patch quality: 🦐 gold shrimp
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] Add the @deprecated replacement and SQLite flip boundary to SessionTranscriptTargetParams.sessionFile.
  • Refresh SDK export/API-baseline proof after the stacked base is rebased and settled.

Risk before merge

  • [P1] If SessionTranscriptTargetParams.sessionFile ships as a normal public SDK parameter, third-party plugins can treat transcript file paths as a durable contract during the SQLite migration.
  • [P1] The branch targets the open draft base at refactor: add public transcript identity API #89261, so SDK export/API-baseline proof should be refreshed after the stack is rebased and settled.

Maintainer options:

  1. Mark the file-shaped parameter deprecated (recommended)
    Add replacement guidance and the SQLite storage-flip removal boundary before this public SDK surface ships.
  2. Hold for stacked SDK contract review
    Keep the draft paused until maintainers settle the public transcript identity base and review the final helper names, target semantics, docs, and API baseline together.
  3. Accept sessionFile as a durable API
    Maintainers may intentionally ship sessionFile as a normal active-target parameter, but that owns the compatibility risk for plugin authors during the storage flip.

Next step before merge

  • [P1] Manual review is appropriate because the remaining blocker is a public SDK contract decision on a protected draft stack, even though the immediate code fix is narrow.

Security
Cleared: No concrete security or supply-chain issue was found; the diff changes TypeScript SDK/runtime code, docs, generated API hashes, and tests without workflows, dependencies, lockfiles, install scripts, permissions, or secret handling.

Review findings

  • [P2] Mark sessionFile as a deprecated SDK target parameter — src/plugin-sdk/session-transcript-runtime.ts:52
Review details

Best possible solution:

Add the @deprecated marker with structured identity replacement guidance and the SQLite flip removal boundary, then review the final stacked SDK contract and refreshed proof before landing.

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

Not applicable. This is an additive public SDK/API migration slice rather than a bug; source inspection shows current main does not already provide the requested public transcript runtime helpers.

Is this the best way to solve the issue?

No as-is. The scoped writer seam is a plausible layer, but the public file-shaped parameter needs the requested deprecation marker and the stack needs refreshed proof before it is the best landable SDK contract.

Full review comments:

  • [P2] Mark sessionFile as a deprecated SDK target parameter — src/plugin-sdk/session-transcript-runtime.ts:52
    This PR adds SessionTranscriptTargetParams.sessionFile as a public SDK parameter, but its JSDoc presents the file path as a normal option. Please mark it @deprecated with the structured identity replacement and SQLite flip boundary, matching the existing session-store compatibility exports; otherwise plugins can treat file paths as the stable transcript contract.
    Confidence: 0.9

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 86b24ac2b2fc.

Label changes

Label changes:

  • add status: 🛠️ actively grinding: The PR author has acted after the latest ClawSweeper review and work remains. Not applicable: This protected maintainer-labeled draft is being reviewed as an internal stack slice; the PR body lists focused local checks, but final proof should still be refreshed before maintainer landing.
  • remove status: ⏳ waiting on author: Current PR status label is status: 🛠️ actively grinding.

Label justifications:

  • P2: The PR changes a public plugin SDK/session transcript migration surface with limited but real compatibility impact.
  • merge-risk: 🚨 compatibility: The new public transcript helpers and file-shaped sessionFile target parameter can become third-party plugin compatibility contracts once released.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: 🛠️ actively grinding: The PR author has acted after the latest ClawSweeper review and work remains. Not applicable: This protected maintainer-labeled draft is being reviewed as an internal stack slice; the PR body lists focused local checks, but final proof should still be refreshed before maintainer landing.
Evidence reviewed

PR surface:

Source +195, Tests +257, Docs 0, Generated 0. Total +452 across 7 files.

View PR surface stats
Area Files Added Removed Net
Source 3 205 10 +195
Tests 1 258 1 +257
Docs 2 2 2 0
Config 0 0 0 0
Generated 1 2 2 0
Other 0 0 0 0
Total 7 467 15 +452

What I checked:

  • Repository policy applied: Root AGENTS.md was read fully, and its ClawSweeper/plugin-SDK compatibility guidance treats public plugin APIs and file-backed session/transcript surfaces as upgrade-sensitive. (AGENTS.md:20, 86b24ac2b2fc)
  • Scoped plugin SDK policy applied: The scoped plugin SDK guide says this directory is the public plugin/core contract and requires SDK subpaths, docs, package/export metadata, API baselines, and focused tests to stay aligned. (src/plugin-sdk/AGENTS.md:1, 86b24ac2b2fc)
  • PR head still exposes neutral sessionFile docs: PR head d5f82f4 defines SessionTranscriptTargetParams.sessionFile with a normal optional-target JSDoc and no @deprecated replacement/removal-boundary wording. (src/plugin-sdk/session-transcript-runtime.ts:46, d5f82f4358df)
  • Existing SDK compatibility pattern: Current main marks whole-store and file-path session SDK helpers as deprecated, names identity-based replacements, and states they are kept only during the transition before SQLite migration. (src/plugin-sdk/session-store-runtime.ts:77, 86b24ac2b2fc)
  • Database-first transcript boundary: Current docs say active transcript identity is {agentId, sessionId} in SQLite and file paths are legacy doctor inputs only, supporting the deprecation requirement for new file-shaped public SDK parameters. Public docs: docs/refactor/database-first.md. (docs/refactor/database-first.md:219, 86b24ac2b2fc)
  • Live PR state and discussion: Live GitHub data shows the PR is open, draft, mergeable against its stacked base, carries the protected maintainer label, and has an author comment explicitly requesting the sessionFile deprecation fix before land. (d5f82f4358df)

Likely related people:

  • jalehman: Authored this PR, the linked Path 3 tracker/stack work, and the merged session SDK compatibility PR that establishes the deprecation pattern relevant to this review. (role: active migration stack owner and SDK contract author; confidence: high; commits: d5f82f4358df, f6d594959907, f1cab04966fa; files: src/plugin-sdk/session-transcript-runtime.ts, src/plugin-sdk/session-transcript-runtime.test.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. labels Jun 2, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 2, 2026
Closed
@jalehman jalehman mentioned this pull request Jun 2, 2026
Closed
@jalehman

jalehman commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up before land: SessionTranscriptTargetParams.sessionFile (src/plugin-sdk/session-transcript-runtime.ts) carries a neutral doc comment. RFC 0007 requires file-shaped public params to be marked @deprecated alongside the structured target, naming the replacement and the SQLite storage flip as the removal boundary — same treatment the session-store exports got in #89203.

@clawsweeper clawsweeper Bot mentioned this pull request Jun 14, 2026
Merged
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. 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 15, 2026
This was referenced Jun 18, 2026
Closed
Closed
Merged
@jalehman jalehman mentioned this pull request Jun 19, 2026
Merged
@jalehman

jalehman commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #95030.

I rebuilt this slice together with #89261 on current main as one replacement draft PR because the scoped transcript target writer helpers are part of the same public SDK transcript contract as storage-neutral identity. Please review #95030 instead.

@clawsweeper clawsweeper Bot added status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 19, 2026
@jalehman jalehman closed this Jun 19, 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 maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. plugin: file-transfer rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: M status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@jalehman