feat(runtime): import a Session bundle into a workspace - #5139
Conversation
Opening the source for backup translated every acquisition failure into `schema_unsupported`, so an unreadable `runtime.sqlite` told the caller to upgrade when the real answer was that the file could not be opened. Only a blocked migration is a statement about the schema. Permission, busy and I/O failures are the environment talking, and the operational store preserves them on purpose.
The mirror of the export, and the asymmetry is the design. The export owns a private copy and deletes what is not the subtree; the import writes into a live workspace holding other people's Sessions and can only add. What keeps that from needing a table list is that the bundle's database already contains nothing else, so the merge copies every table it has. A table added to the schema later travels in both directions without anyone maintaining a list — the same property that made the export's deny-by-default filter right. Three kinds of table are not copied. The workspace-level ones belong to the target, which has its own. `session_catalog_projection` is derived, and `session_metadata` carries triggers that maintain it — inserting the bundle's copy and then the Session row makes the trigger collide with what was just inserted, and letting the target derive it is the only way it stays correct when the derivation changes. Write order is the safety argument. Artifact bytes land first and the database transaction commits last, so a failure between them leaves files nothing points at — reclaimable — rather than rows pointing at files that are not there. The context-offload closure is merged too: without it an imported Session arrives with its read-image references intact and none of the bytes behind them. A Session id already present refuses the whole bundle. Ids are generated rather than chosen, so one already here means this Session is already here, not that two of them collided. Importing over it would merge two histories that share ids and agree about nothing else. Copying a Session that already exists is a separate operation and belongs to `conversation-copy`. Unlike the export, this opens the target the ordinary way. The export refuses to migrate its source because it only reads; an import is a write the user asked for, and the target is often a workspace with no database yet — moving to a new machine is the point. The round-trip test is the acceptance criterion the export PR could not state: export, import into a different workspace, and compare the rows the model reads field for field, including the JSON columns whose bytes a re-encoding would change.
Three defects from reviewing the import against the workspace it writes into. **The merge drove a shared connection directly.** The operational store hands out one reference-counted connection per workspace with its own transaction depth, and this ran `BEGIN IMMEDIATE` on it and left `PRAGMA foreign_keys` off afterwards. Every later user of that workspace would have run without constraint checking, and nothing would have reported it. The merge now goes through the lease's transaction and puts the pragma back. **The bundle's schema was never compared with the target's.** Rows are copied with `INSERT ... SELECT *`, which maps by position: a bundle whose tables carry the same column count in a different order would be inserted transposed -- rows that read as data and are not. The export only writes a bundle at its own current schema, so a mismatch means two builds disagree, and saying so is the only honest answer. **A target this build cannot open reported as an IO failure.** Same shape as the export's own follow-up: a blocked migration is a schema verdict, and flattening it sends the caller after the wrong problem. The bundle is also attached read-only now, so reading one can never write it. The first regression I wrote for the pragma passed with the fix removed: it read `PRAGMA foreign_keys` on a fresh handle, and the pragma is per-connection. It now holds a lease across the import the way a running Runtime Host does, on the canonicalised path the import itself resolves -- without that it observes a different connection and proves nothing.
|
Reviewed at 1. [P1] A fresh target fails after the Session has already been committed
The caller receives failure, but the Session rows have already been committed; retrying hits Please acquire the required authority based on incoming context as well as target state, and prepare the context closure before publishing the Session. Failure/retry handling also needs to account for data left by earlier attempts. 2. [P1] Managed context payloads are skipped while import reports successThe copy loop only visits immediate children of the values directory and skips directories (lines 1182-1197). Actual managed payloads live under An isolated execution of this merge function returned one imported reference while its target payload file was absent. Please copy the validated managed-file paths with their directory structure, and test reading the imported payload rather than only counting references. 3. [P2] Existing context databases get stale usage accountingThe merge inserts only In an isolated SQLite fixture, importing one 3-byte blob and one reference produced:
These values drive quotas and cleanup consistency checks, so this is not just a display issue. Please maintain the accounting invariants in the same transaction. Minimal completion pathNo general migration framework or history-merging machinery seems necessary. Keep the current design, but complete its invariant: prepare all referenced data first, publish the Session last. Add regressions for a fresh target with context, nested managed-file payloads, merging into an existing context store with correct usage, and retry after an injected failure. Verification: complete diff and related storage code reviewed; the context merge function was extracted/transpiled from this commit and exercised against real temporary SQLite databases using the repository's initial context schema. This was an isolated helper-level reproduction, not a full project test run. |
Three gaps in the context half of the import, all of which let a Session arrive without the bytes it points at. **The Session was published before its context.** The rows committed, then the context merge ran — so a failure there left a Session visible whose payloads never arrived, and a retry hit `session_exists` forever because the ids were now taken. The order is inverted: artifacts and context land first, and the Session rows are the last thing written. A failure before that leaves data nothing points at, which the store reclaims. **The offline authority was decided by the wrong side.** It was taken only when the TARGET already had a context store, so a bundle carrying context into a fresh workspace failed the check with no Runtime Host anywhere — the common case, and the one where the store is about to be created. It is now taken for what is being written. **Managed payloads were never copied.** They live at `sha256/<prefix>/<hash>`, and the copy visited only immediate children and skipped directories: it saw one directory, skipped it, and reported a successful import whose referenced bytes were all absent. The tree is copied whole, and the regression reads an imported payload rather than counting the reference that names it — a reference whose bytes never arrived counts exactly the same. **Usage accounting went stale.** `context_store_usage` and `context_session_usage` are maintained by the context store explicitly; no trigger does it. They are recomputed in the same transaction, the way the export recomputes them when it filters. These numbers drive quotas and the cleanup consistency checks. One more, found while testing the above: the merge now asks the same question of a bundle table that the export asks before emptying one. A table with no Session column and no referential rule describes the workspace, and the target has its own. The export empties those, so this inserts nothing in practice — but an import that relies on the other side having tidied up is one bundle away from writing a workspace singleton into somebody else's workspace.
|
All three fixed. The reproductions were precise enough to confirm each one directly — thanks. The first is the one that mattered most, and your framing of it is the fix: prepare everything referenced, publish the Session last. Artifacts and context now land before the Session rows, so a failure leaves data nothing points at — which the store reclaims — rather than a Session already visible whose payloads never arrived and whose ids a retry can no longer claim. The authority follows from the same correction: it is taken for what is being written, not for what the target already has. A bundle carrying context into a fresh workspace is the common case and was the one running unprotected. Managed payloads. Copied with their tree now. The regression reads an imported payload rather than counting the reference that names it, since a reference whose bytes never arrived counts exactly the same — that distinction is what your repro turned on. Usage accounting. Recomputed in the same transaction, using the same recomputation the export does when it filters. Covered by merging into an existing store and asserting blob count, physical bytes and the per-Session rows. One more that surfaced while writing those tests: the merge now asks the same question of a bundle table that the export asks before emptying one — a table with no Session column and no referential rule describes the workspace, not a Session. The export empties those, so this inserts nothing in practice, but an import relying on the other side having tidied up is one bundle away from writing a workspace singleton into someone else's workspace. Regressions added for a fresh target with context, nested managed payloads read back, merging into an existing store with correct accounting, and retry after an injected failure. Each was checked by removing the fix it covers; the publish-order one was the one I had no coverage for until you named it. |
|
Follow-up review at The recursive managed-payload copy is fixed, and publishing the Session after its context is the right ordering correction. Per-Session usage is now rebuilt too. However, the failure/retry and fresh-target cases are not yet closed, and the new physical-byte recomputation introduces a GC accounting regression. 1. [P1] A failed import still prevents retry when the bundle contains artifactsArtifacts are copied before the context merge, but they are neither rolled back on failure nor recognized as identical leftovers on retry. The unchanged COPYFILE_EXCL handling rejects them. I reproduced this through
Moving publication last prevents a broken visible Session, but does not by itself make the operation retryable. Please clean up only files owned by the failed attempt, or safely recognize identical staged leftovers without overwriting unrelated data. The new test named “so a retry is still possible” neither includes an artifact nor actually retries; it only checks that Session metadata is absent. Please remove the injected failure and assert the second import succeeds with both payloads intact. Also cover failure at the final operational merge after context has already committed, since 2. [P2] A genuinely fresh, unmarked target still cannot import context
Using an existing empty directory as the target and a valid source containing context now fails with Please initialize/resolve the target through the existing root authority before taking its owner lease, or explicitly require and validate an initialized root at the public API/CLI boundary. The latter would narrow the currently described fresh-workspace behavior. 3. [P2] Recomputing physical_bytes from live blobs loses pending-file-deletion bytesThe new UPDATE replaces Reproduction using the actual migrated context schema:
The export can use the simpler sum because it explicitly empties the deletion queue on its private copy. The live import preserves that queue, so it cannot reuse the export formula unchanged. Please preserve pending-file accounting and reconcile any pending deletion whose payload the import makes live again. Verification / test coverageI bundled this commit's source with esbuild and ran the complete Please also consider replacing The main design still does not need a new framework. The remaining work is to complete the existing ownership, retry, and accounting invariants rather than add another abstraction. |
A failed import left three kinds of residue that turned the second attempt into a permanent conflict: - Artifacts were staged before the context merge and neither rolled back nor recognised on retry, so `COPYFILE_EXCL` refused the import's own leftover. The copy now reports what it created, the caller removes exactly those on failure, and a leftover from a crashed attempt is accepted only when its bytes are identical. - `context_refs` insertion was not idempotent, so a failure after the context transaction committed blocked the retry. Identical rows are now skipped; a row naming different content is still a conflict. - A genuinely fresh workspace could not import context at all: the snapshot lock discovered a marked Storage Root rather than resolving one. A writer that is about to create the store now initialises it. Also stop recomputing `physical_bytes` from live blobs alone. Bytes queued in `context_file_deletions` are on disk and still charged, and the store subtracts them when the drain completes -- so dropping them here made that later subtraction underflow. The export can use the simpler sum because it empties the queue on its private copy. The context fixture now uses the real v3 schema rather than an approximation of it.
|
All three fixed in Artifacts blocking the retry. The copy now reports the paths it created, and the caller removes exactly those when a later step fails — nothing else in
Fresh unmarked target. Pending deletions. Tests: the retry test now stages an artifact, fails, and actually retries, asserting both payloads and the artifact bytes survive; a second test fails at the operational merge after the context transaction has committed and retries from there; the identical-vs-different leftover is covered both ways; a bare directory with no marker imports context; and the pending-deletion total is asserted. Each fix was checked by reverting it: rejecting any leftover, plain |
Seven upstream commits: managed WSL recovery and handoff (apache#5175, apache#5161, apache#5160), Session bundle import (apache#5139), WorkHub model routing and delegated results (apache#5152, apache#5150), React dev performance measures (apache#5156). Resolution per the sync policy: every conflict under the old renderer's WorkHub, composition and platform trees, its stories and the main tests that import them stays deleted; upstream's new files in those trees (delegation-feedback.ts, react-performance-measures.ts and its test) are dropped; `components/ui/textarea.tsx` was a rename/rename against `features/workhub/index.ts` and keeps ours; `locales/conversation-copy.ts` auto-merged and is Biome-formatted. Nothing in this round is behaviour the new renderer has to re-implement. Gates: build:test + build:renderer, typecheck, ledger, biome, knip, locale hygiene, ASF headers, desktop dist tests (1575), renderer state (237), Electron smoke (44), core-dialogue smoke. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2 participants
Summary
Import a
.maka-sessionbundle into a workspace — the other half of #5113 — plus the one non-blocking follow-up from that PR's approving review.Why this is not simply the export run backwards
The export owns a private copy and can delete what is not the subtree. The import writes into a live workspace holding other people's Sessions, and can only add. That single difference drives the design.
What survives the asymmetry is the property that mattered most in #5113: no table list. The bundle's database has already been filtered down to its own Sessions, so the merge copies every table it has. A table added to the schema later travels in both directions without anyone maintaining a list — the same reason the export filters by deleting rather than selecting.
Three kinds of table are not copied:
operational_schema_migrations,session_catalog_state, …) belong to the target, which has its own.session_catalog_projectionis derived, andsession_metadatacarries triggers that maintain it. Inserting the bundle's copy and then the Session row makes the trigger collide with what was just inserted — found by the round-trip test, and letting the target derive it is the only way it stays correct when the derivation changes.Write order
Artifact bytes land first; the database transaction commits last. A failure between them leaves files nothing points at, which is reclaimable, rather than rows pointing at files that are not there, which is not.
The context-offload closure is merged too. Without it an imported Session arrives with its read-image references intact and none of the bytes behind them — the hole @likun666661 caught on the export side, in the other direction. Blobs are content-addressed, so an id already present is the same bytes and the insert is skipped rather than treated as a conflict.
Conflicts
A Session id already present refuses the whole bundle, before anything is written. Ids are generated rather than chosen, so one already here means this Session is already here — not that two of them collided. Importing over it would merge two histories that share ids and agree about nothing else.
Copying a Session that already exists is a different operation with a different meaning, and
conversation-copyalready does it. Not in this PR.One difference from the export, on purpose
The export refuses to migrate its source, because it only reads. This opens the target the ordinary way: an import is a write the user asked for, and the target is frequently a workspace with no database yet — moving to a new machine is the point of the feature.
Follow-up from #5113
backupOperationalState()translated every acquisition failure intoschema_unsupported, so an unreadableruntime.sqlitetold the caller to upgrade when the real answer was that the file could not be opened. Only a blocked migration is a statement about the schema now; permission, busy and I/O failures are the environment talking and are preserved, as the operational store intends. Covered by achmod 000regression.Tests
Five import tests, 18 export tests (one new), and the 20 existing storage bundle/context/lock tests.
The round-trip is the acceptance criterion #5113 could not state on its own: export a Session, import it into a different workspace that already holds someone else's Session, and compare the rows the model reads field for field — including the JSON columns whose bytes a re-encoding would change. It also asserts the receiving workspace kept what it had.
The subtree test carries a parent and its subagent child with the child's artifact bytes, and checks
subagent_spawnsarrives: without it the target holds two Sessions and nothing saying which tool call joined them.Each assertion was checked by removing what it covers — the conflict precheck, the derived-table skip, the artifact copy, the bundle-schema comparison, the pragma restore, and the narrowed error translation — and confirming the corresponding test fails.
Gates: storage and runtime build and typecheck clean,
biome checkon every changed file,check:asf-headers, and the published-entrypoints check.What this does not do
Desktop menus, IPC and the protocol epoch. Importing a Session that already exists as a copy. The
snapshot | portablemode collapse suggested on #5113 — I would still rather let a second real profile prove itself before naming one, and this PR is the first evidence about that, not the last.