Skip to content

fix(sync): import an SVG into the open project instead of silently corrupting it - #7

Open
eetu wants to merge 1 commit into
mainfrom
fix/import-project-sync
Open

fix(sync): import an SVG into the open project instead of silently corrupting it#7
eetu wants to merge 1 commit into
mainfrom
fix/import-project-sync

Conversation

@eetu

@eetu eetu commented Aug 4, 2026

Copy link
Copy Markdown
Owner

You reported drag-and-drop as "does nothing visible when a project is selected". The drop does load — but only locally. The project never receives it, sync stays connected, and later edits are ops built against the imported document applied to the server's old one.

What it actually did

Drop a circle into an open blank project, then draw a rectangle:

canvas:  circle + rectangle
project: <path d="M 4.03 10.645 L 4.03 10.645 L 4.03 10.645 Z"/>   ← degenerate
         …and no circle

The project ends up holding something matching neither what you see nor what you imported. Silent, and it's the artwork. putProject had existed since the client was written and was called by nobody — there was no import path at all.

Client: a whole-document swap is its own event

Ops describe edits to a document, so replacing the document wholesale can't be expressed as one. editor.importDocument (drop / open file / paste / source-drawer apply) now announces the swap through a replace sink — the twin of the existing setSyncSink, so the standalone build still imports zero backend code.

Connected mode then:

  • importPUT the SVG to the open project, then re-load the server's model. That second step matters: the server's parse mints the node uids every client and the LLM address, so adopting it is what keeps identity shared. Skip it and ops start referring to nodes the backend has never heard of.
  • New → detach. A blank local document isn't the project any more, and pretending otherwise is the same bug.
  • push failed → disconnect and surface it in the panel, rather than quietly dropping every subsequent edit.

Server: PUT has to reach the live session

The other half. PUT /api/projects/{id} wrote the row while the resident Editor kept the old document — so an attached browser or MCP client carried on editing the previous drawing and overwrote the import on its next op. (This was on the known-issues list from the OIDC PR; the import path made it load-bearing.)

It now hands the import to the live session and broadcasts reload. Peers re-fetch rather than replay: a whole-document swap mints new uids, so there is nothing to replay against. SyncMsg gains one optional reload flag, skipped in serialization when false.

After

project stores: circle + <path d="M 4.03 10.645 L 22.931 10.645 L 22.931 26.846 L 4.03 26.846 Z"/>

Canvas and project agree. An MCP client attached across the import sees it land live:

mcp before import: project 5 · viewBox 0 0 100 100 · 0 paths
mcp after import:  project 5 · viewBox 0 0 100 100 · 1 paths | #0 circle-0 [20 20 60×60] fill #e11
sync after New:    DISCONNECTED

One nice side effect: the header now keeps showing the project name after an import, instead of silently becoming desktop-art.svg.

Verification

Driven against the real UI + a real backend + a live MCP connection, since this is precisely the class of bug that unit tests miss. The session/broadcast half is pinned by a backend test (importing_into_an_open_project_updates_the_live_session): the resident editor takes the import, the row agrees, and peers get reload with no ops.

cargo test --workspace green (14 backend), clippy -D warnings clean, yarn validate clean, e2e 67 passing.

Version stays 0.1.0.

Worth knowing

The connected-mode paths still aren't in CI — e2e builds without VITE_NIB_BACKEND, so the projects panel never mounts there. That's now two features verified by hand-driven probes. If this area keeps growing, teaching CI to run a connected-mode build against a live backend is probably the next infrastructure worth having.

…rrupting it

Reported as "drag and drop does nothing visible when a project is selected". The
drop did load — but only locally. The project never received it, and sync stayed
connected, so later edits were ops built against the imported document and applied
to the server's *old* one.

Measured before: drop a circle into an open blank project, then draw a rectangle.

  canvas:  circle + rectangle
  project: <path d="M 4.03 10.645 L 4.03 10.645 L 4.03 10.645 Z"/>   (degenerate)
           and no circle

The project ended up holding something that matched neither side. Silent, and it's
the artwork.

Ops describe edits *to* a document, so replacing the document wholesale can't be one
— it needs to be its own event. `editor.importDocument` (drop / open file / paste /
source-drawer apply) now announces the swap through a replace sink, the twin of the
existing `setSyncSink`. Connected mode pushes the SVG to the open project, then
re-loads the server's model so node uids stay shared — skipping that is how ops start
addressing nodes the backend has never heard of. A **New** document detaches from the
project rather than pretending to still be it, and a failed push disconnects and says
so instead of quietly dropping edits.

The other half was server-side: `PUT /api/projects/{id}` wrote the row while the
resident Editor kept the old document, so an attached browser or MCP client carried on
editing the previous drawing and overwrote the import on its next op. It now hands the
import to the live session and broadcasts a `reload` — peers re-fetch, because a
whole-document swap mints new uids and there is nothing to replay.

After: the project stores the circle *and* a correctly-sized rectangle, and an MCP
client attached throughout sees the import appear (`#0 circle-0 [20 20 60x60]`).

Verified by driving the real UI + a real backend + a live MCP connection; the
session/broadcast half is pinned by a backend test.
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