Repository navigation
feat(core): opt-in blob writes for session media behind experimental.media_blob - #58741
Conversation
a2690cb to
e024cce
Compare
3449c4e to
6bcb56a
Compare
kitlangton
changed the title
6bcb56a to
7fe11c2
Compare
…media_blob With experimental.media_blob on, inline user files and tool data: files are stored once in the blob table and rows keep media:<hash> refs. blob_ref rows track which message or pending inbox row keeps each blob alive; cancel, revert and session delete release them and drop blobs nothing references. Off by default; reads already hydrate refs.
… only when they can change Session.prompt and slash commands now admit inside the session's location services, so experimental.media_blob applies at admission for direct callers too, as it already did over HTTP. Message updates only record refs when externalize extracts new media; a content rewrite also releases refs it no longer names and drops orphaned blobs.
e024cce to
27249de
Compare
kitlangton
changed the title
Preparation, the staged revert commit and admission now run in one instance scope instead of acquiring the location twice.
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
The opt-in write path works as described. I ran the TUI from source with experimental.media_blob: true and attached a file. On the base, the bytes were stored inline in session_message. With this PR, the message keeps only a media:<sha256> ref, with one blob row and one blob_ref row. The projector, prompt, normalization and revert tests pass, and tsc is clean for packages/core.
Public API: adds the optional config key experimental.media_blob (boolean, default false) to the config schema, the OpenAPI spec and the generated client types. This is backwards compatible; existing configs and callers are unaffected.
The ref bookkeeping is sound: delivery reuses the inbox ID, revert drops refs at the same point it drops inbox rows, and deleting a session releases its child sessions' refs too. No blocking concerns. One minor gap: if updateMessage matches no row, the blob it already saved gets no ref and is never deleted. This only happens with the flag on and on an unusual path.
Related: #58436
There was a problem hiding this comment.
The new commit only merges dev. The PR's own media code and tests are unchanged, so my earlier review still applies. I re-ran the TUI check with experimental.media_blob: true at the new head: the attachment is still stored once in blob, with a blob_ref and a media: reference in the message. The projector, prompt, normalization and revert tests pass, and tsc is clean for packages/core.
Public API: unchanged from before. The PR adds only the optional experimental.media_blob config key, which is backwards compatible. No new concerns.
There was a problem hiding this comment.
The new commit is a sound simplification. Removing the dimensions parser leaves blob.width and blob.height empty, and nothing reads them. SessionPrompt.prepare now relies on its only caller, Session.prompt, to set up the session's location services, and the compiler checks this. Slash commands now set up those services once instead of twice.
I re-ran the TUI check with the flag on: the attachment is still stored once with a blob_ref and a media: reference in the message. The projector, prompt, owned-session, command, normalization and revert tests pass (131), and tsc is clean for packages/core.
Public API: unchanged from before. The PR only adds the optional experimental.media_blob config key, which is backwards compatible. No new concerns.
What it does
Adds an experimental write path for session media, off by default. With
experimental.media_blob: true, new inline user attachments and tooldata:files are stored once in theblobtable, and message and inbox rows keep onlymedia:<hash>refs. With the flag off, nothing changes: bytes stay inline.Why
Today every message row that carries an image or file holds its full base64 bytes, and the same screenshot is copied into the inbox row, the delivered message and every update of a tool result. #58656 already reads
media:<hash>refs back; this is the matching opt-in write side, so we can try it before turning it on for everyone.How it works
SessionBlob.externalize, which moves inline bytes intobloband swaps in aSessionStoredref. The flag is read from the session's config on every write.blob_refrows record which message or pending inbox row keeps each blob alive. They are written only for blobs that exist, and only when a write added new refs or rewrote message content.blob_refrows, then delete any released blob that nothing else references. A forked session keeps a shared blob alive until the last session is gone.Session.promptand slash commands now admit inside the session's location services, so the flag applies at admit for direct callers the same way it already did over HTTP.Follow-ups, left out to keep this small: session import (
transfer.ts) still writes media inline, and projections replayed without config in scope also stay inline. Both are safe, since reads handle inline and ref rows alike.Risk
Core session write path, but only with the flag on. With it off, the one change is that prompt admission and slash commands run with the session's location services, which matches the HTTP path, so events from direct callers now carry the session's location. With it on, a bug in ref tracking could drop a blob early (reads show the file as unavailable) or keep one around too long. No schema change and no migration.
Proof
bun test(packages/core):session-projector,session-prompt,config/normalization,command,session-compaction,session-execution,session-revert,session-runner,session-runner-tool-events,session-runner-message,session-store,session-title: 483 pass.config/command-subagentandsession-activation: 6 pass.server/session-instances: 1 pass.Session.Service(not HTTP) stores a ref at admit; this test fails without the admit change.bun run typecheck(core and the full pre-push turbo typecheck): clean.oxlinton changed files: 0 errors.bun run migration --check: no schema changes.Added by the opencode bot (AI-generated).
Related: #58436
Public API: adds the optional experimental.media_blob config key (backwards compatible)