Skip to content

feat: complete /api/import and /api/export with asset support, tests, and docs#91

Draft
elucid wants to merge 6 commits into
mainfrom
elucid/import-export
Draft

feat: complete /api/import and /api/export with asset support, tests, and docs#91
elucid wants to merge 6 commits into
mainfrom
elucid/import-export

Conversation

@elucid

@elucid elucid commented Jun 21, 2026

Copy link
Copy Markdown
Member

Summary

Completes the /api/import and /api/export endpoints so they handle the full board shape: sessions, surfaces, comments, assets (base64-encoded), and all settings — not just the theme key.

The two endpoints now round-trip cleanly: exporting from one sideshow instance and importing into another produces an identical board.

Open TODOs

  • Handle comment seq collisions during import without silently dropping feedback. Imports should distinguish true idempotent duplicates from a different comment with an already-used sequence number, and should fail with a clear conflict or otherwise preserve all comments.
  • Avoid materializing the entire asset payload in memory during export, or add an explicit documented/tested export size limit. Current export loads raw asset bytes, base64 copies, and the final JSON response at once, which can OOM on large boards.

Changes

Store interface (server/types.ts)

  • importData is now mandatory on all Store implementations (was importData?). Every store must support bulk import.
  • listSettings() added to the Store interface — returns all key/value settings so export does not need to hard-code setting names.
  • ImportAsset type added — mirrors Asset but carries data as a base64 string for JSON transport.
  • ImportData extended with an optional assets array.

JsonFileStore (server/storage.ts)

  • importData now decodes and inserts base64 assets, skipping duplicates by id.
  • listSettings returns all persisted settings.
  • Comment dedup uses both id and seq to prevent double-insertion.

SqlStore (workers/sqlStore.ts)

  • importData inserts assets as BLOBs decoded from base64, using INSERT OR IGNORE for idempotency.
  • Comment dedup does an explicit SELECT by id before insert (SQLite AUTOINCREMENT on seq makes INSERT OR IGNORE on the PK insufficient for externally-assigned seq values).
  • listSettings queries the settings table.
  • Local decodeBase64 helper (runtime-agnostic, no Buffer).

API (server/app.ts)

  • GET /api/export now includes assets (base64-encoded via a chunked encodeBase64 that avoids stack overflow on large blobs) and uses store.listSettings() instead of hard-coding theme.
  • POST /api/import response now reports assets count alongside sessions/surfaces/comments. Removed the store.importData? optional check — the method is mandatory.

Tests

  • Store contract (test/storeContract.ts): three new tests run against both JsonFileStore and SqlStore:

    • Round-trip: create a full board → export shape → import into a fresh store → verify all entities match including asset bytes.
    • Idempotency: import the same data twice → no duplicates.
    • Partial import: import only sessions → no errors on missing keys.
  • API tests (test/importExport.test.ts): five new tests:

    • Auth gating on both endpoints.
    • Invalid JSON → 400.
    • Export shape includes all entity types and all settings.
    • Import preserves IDs, and imported assets are serveable at /a/:id.
    • Full round-trip: export from one server, import into another, re-export, verify equality.

Documentation

  • docs/deploying.md: new "Backup and migration" section with curl examples for export/import.
  • .changeset/import-export.md: minor changeset for release notes.

Validation

All four checks pass:

npm test             # 192 passing
npm run typecheck    # node + workers + viewer
npm run lint
npm run format:check

elucid added 3 commits June 21, 2026 09:38
Make importData mandatory on Store implementations and add listSettings for complete board exports. JsonFileStore and SqlStore now import base64-encoded assets idempotently while preserving IDs and metadata. Extend the shared store contract to cover round-trip imports, idempotency, and partial imports.
Add /api/import and /api/export coverage for auth gating, invalid JSON, ID preservation, and full round-trips. Export now includes all settings plus base64-encoded assets fetched from the store, and import responses report asset counts.
Add a release changeset for the new board import/export endpoints and document backup/migration curl examples for deployed boards. Leave the implementation handoff plan as an uncommitted local artifact.

@elucid elucid left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Reviewed: PR #91 import/export completion (assets, settings, store/API tests)
Verdict: NEEDS CHANGES

Summary

The PR substantially improves import/export and the happy-path tests pass, but it does not preserve the full board state yet. Session trace steps are omitted entirely, and one real asset lifecycle edge case still drops assets during export.

Findings

[P1] Session trace steps are not included in import/export

File: server/types.ts:267 / server/app.ts:622

Issue: The store persists session trace state (JsonFileStore's trace map and SqlStore's trace_steps table), and the plan/review criteria call out trace steps as part of the data that must round-trip. ImportData has no trace field, /api/export only returns sessions/surfaces/comments/assets/settings, and both importData implementations ignore traces. Exporting a board with synced trace data and importing it into another store loses every session's trace.

Suggested Fix: Add trace data to the import/export shape (for example trace?: Record<string, TraceStep[]> keyed by session id), export it with store.listTrace(session.id) for each session, and import it in both stores preserving step order. Add store-contract and API round-trip tests that set trace steps, export/import, and verify GET /api/sessions/:id/trace returns the original steps.

[P1] Export skips assets whose owning session was deleted but which are still referenced by surviving surfaces

File: server/app.ts:628

Issue: /api/export discovers assets by iterating current sessions and calling store.listAssets(session.id). That misses a valid store state this codebase explicitly supports: content-addressed assets can be referenced by surfaces in other sessions, and removeSession keeps a deleted session's asset if a surviving surface still references it. In that state, the surface is exported with an assetId, but the asset bytes are omitted, so an imported backup renders broken image/trace/file references.

Suggested Fix: Export assets independently of current sessions. The most complete fix is adding a listAllAssets()/equivalent Store method and using it in /api/export; alternatively, at minimum collect asset ids from all current and historical surface parts and getAsset() any referenced assets not found through listAssets(session.id). Add a regression test that uploads an asset in session A, references it from a surface in session B, deletes session A, exports/imports, and verifies the referenced asset is present and serveable.

What's Good

  • Asset data is encoded/decoded in a cross-runtime-compatible way, with chunked export to avoid spread stack issues on larger blobs.
  • Settings export now uses a generic listSettings() instead of hard-coding theme.
  • The new tests cover auth, malformed JSON, idempotent store imports, partial imports, and JSON/SQL store round-trips for the entities currently in the import shape.

Validation Run

  • npm test — passed (192 tests)
  • npm run typecheck — passed
  • npm run lint — passed
  • npm run format:check — passed

@elucid elucid left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Reviewed: PR #91 import/export implementation against docs/import-export-plan.md
Verdict: NEEDS CHANGES

Summary

The PR substantially implements sessions, surfaces, comments, assets, settings, and trace import/export, and the new tests/typecheck pass locally. However it omits the persisted comment sequence high-water mark, which can make migrated sessions silently stop delivering new user feedback to agents.

Findings

[P1] Export/import drops comment sequence high-water state

File: server/app.ts:635, server/storage.ts:483, workers/sqlStore.ts:590

Issue: The export shape includes existing comments but not the store's comment sequence high-water mark (JsonFileStore.lastSeq, and SQLite's AUTOINCREMENT sequence). On import, both stores only advance their next comment seq from rows that still exist. If a source board has had comments deleted after being delivered to the agent, the exported session can still have agentSeq greater than the max exported comment seq. After import, the next new comment can reuse a seq <= agentSeq, so listComments({ afterSeq: session.agentSeq }) / agent waits never deliver that new feedback.

Concrete repro from the code path:

  1. create a session + surface + comment with seq = 1
  2. markAgentSeen(session, 1) so session.agentSeq = 1
  3. delete the surface, which removes the comment but leaves the session and cursor
  4. export/import into a fresh store
  5. create a new comment: it gets seq = 1, and agent reads after agentSeq = 1, so it is filtered out forever

This violates the feedback delivery invariant and means the board is not preserved identically across migration.

Suggested Fix: Include the comment sequence high-water mark in ImportData//api/export (or otherwise expose it via the Store interface), and restore it in both stores. At minimum, imported stores must set their next comment seq above max(imported comments.seq, imported sessions.agentSeq), but exporting/restoring the actual high-water mark is the exact preservation fix. For SQLite, also update sqlite_sequence for the comments table. Add a store/API test that exports a session whose delivered comments were deleted, imports it, creates a new user comment, and verifies its seq is greater than the imported agentSeq and is returned by an afterSeq read.

What's Good

  • Asset bytes are included and base64 encoded/decoded for both store implementations.
  • Settings are exported generically via listSettings() instead of hard-coding theme.
  • Trace steps are included in API/store round-trip coverage.
  • Local validation: npm test passed (193 tests); npm run typecheck passed.

@elucid elucid left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Reviewed: PR #91 import/export implementation (Store interface, JsonFileStore, SqlStore, API endpoints, tests/docs)
Verdict: NEEDS CHANGES

Summary

This substantially implements the planned import/export shape for sessions, surfaces, comments, assets, settings, and trace steps, and the happy-path coverage is strong across both store implementations. I found two material edge cases that can either silently lose imported comments or make export unusable for boards with larger asset sets.

Findings

[P1] Comment sequence collisions are silently treated as success, dropping imported feedback

File: server/storage.ts:490 and workers/sqlStore.ts:608

Issue: Import preserves each comment's seq, but both stores silently skip a comment when its sequence number already exists in the target with a different comment id. In JsonFileStore, the dedupe condition is x.id === c.id || x.seq === c.seq; in SqlStore, INSERT OR IGNORE ignores the row because seq is the primary key. This is idempotent for importing the same backup twice, but it loses data for any partial/non-empty import where the target has already allocated the same seq (very likely, since every board starts comments at 1). /api/import still reports the incoming comment count, so the operator gets a successful migration response with missing comments.

Suggested Fix: Distinguish idempotency from conflict. If the same comment id already exists, skip it only if it is the same record. If a seq exists for a different id, fail the import with a conflict (or require/validate an empty target for full-board imports). Add store-contract and API tests for a seq collision so the behavior is consistent across JsonFileStore and SqlStore.

[P1] Export materializes all raw assets and their base64 copies in memory

File: server/app.ts:622

Issue: GET /api/export calls store.listAllAssets() (which returns every asset including bytes), then maps those assets to base64 strings, then c.json() serializes the whole export. With the current board asset budget (MAX_BOARD_ASSET_BYTES is 2GB), this can hold raw bytes + base64 strings + the final JSON string at once. A board with many screenshots/trace files can OOM the Node process or Cloudflare Worker during backup, so the import/export feature does not handle the large-asset edge case called out in the plan/review criteria.

Suggested Fix: Stream the export JSON and encode each asset incrementally, or introduce an explicit export size limit/error before loading all bytes. If streaming is too large a change for this PR, lower/scope the supported export asset size and document/test the failure mode rather than crashing.

Tests Run

  • npm test — passed (196 tests)
  • npm run typecheck — passed
  • npm run lint — passed
  • npm run format:check — passed

What's Good

  • The exported shape now covers the important store entities: sessions, surfaces (including history/version), comments, retained assets, settings, trace steps, and comment seq high-water mark.
  • Store contract tests exercise both JsonFileStore and SqlStore for round-trip, idempotency, partial import, assets, settings, and comment cursor behavior.
  • API tests cover auth, invalid JSON, served imported assets, orphaned-but-referenced assets, and full re-export equality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant