diff --git a/wasm/bindings/eeschema_embind.cpp b/wasm/bindings/eeschema_embind.cpp index 6c52737..a259ec6 100644 --- a/wasm/bindings/eeschema_embind.cpp +++ b/wasm/bindings/eeschema_embind.cpp @@ -507,7 +507,16 @@ std::map snapshotByUuid( SCH_EDIT_FRAME* aFrame ) if( SCH_SCREEN* screen = currentScreen( aFrame ) ) { for( SCH_ITEM* item : screen->Items() ) + { + // ERC markers are screen items but not document content: the file + // writer never saves them and the selection writer cannot format + // one (empty blob → the JS side skips the entry with a warning). + // Keep them out of the collab model entirely. + if( item->Type() == SCH_MARKER_T ) + continue; + m[toUtf8( item->m_Uuid.AsString() )] = itemToJson( item ); + } } return m; @@ -595,13 +604,48 @@ void flushDiff() auto blobFor = [&]( const std::string& id, json& aArr ) { - if( !wDone.insert( id ).second ) - return; - KIID kid( wxString::FromUTF8( id.c_str() ) ); - if( SCH_ITEM* item = fr->Schematic().ResolveItem( kid, nullptr, /*allowNull*/ true ) ) - aArr.push_back( json{ { "sexpr", itemBlob( fr, item ) }, { "parent", nullptr } } ); + SCH_ITEM* item = fr->Schematic().ResolveItem( kid, nullptr, /*allowNull*/ true ); + + if( !item ) + return; + + // A child that reached the dirty set unlifted (field/pin/sheet-pin — + // ResolveItem walks direct children too) serializes to an EMPTY blob: + // the selection writer has no case for it, so the entry used to reach + // JS as envelope furniture and be skipped, silently losing the edit. + // Lift to the screen item the differ tracks (same promotion as + // noteDirty) so the parent's re-blob carries the child's content. + while( EDA_ITEM* p = item->GetParent() ) + { + if( !p->IsType( { SCH_SYMBOL_T, SCH_TABLE_T, SCH_SHEET_T, SCH_LABEL_LOCATE_ANY_T } ) ) + break; + + item = static_cast( p ); + } + + std::string rootId = toUtf8( item->m_Uuid.AsString() ); + + if( !wDone.insert( rootId ).second ) + return; + + std::string sexpr = itemBlob( fr, item ); + + // Still empty after lifting (a marker that slipped in, an unformattable + // type): never put a hollow envelope on the wire — mirror pcbnew's P-5 + // guard and leave a breadcrumb instead. + if( sexpr.empty() ) + { + std::string type = toUtf8( item->GetClass() ); + EM_ASM( { + console.warn( '[pcbjam collab] eeschema: skipped un-serializable dirty root', + { uuid: UTF8ToString( $0 ), type: UTF8ToString( $1 ) } ); + }, rootId.c_str(), type.c_str() ); + return; + } + + aArr.push_back( json{ { "sexpr", sexpr }, { "parent", nullptr } } ); }; for( const auto& [id, j] : cur ) @@ -1357,7 +1401,20 @@ std::string schCollabSnapshotItems() if( SCH_SCREEN* screen = currentScreen( fr ) ) { for( SCH_ITEM* item : screen->Items() ) - added.push_back( json{ { "sexpr", itemBlob( fr, item ) }, { "parent", nullptr } } ); + { + // Same exclusions as snapshotByUuid/blobFor: markers are not + // document content, and an empty blob (unformattable type) + // must never seed a hollow envelope. + if( item->Type() == SCH_MARKER_T ) + continue; + + std::string sexpr = itemBlob( fr, item ); + + if( sexpr.empty() ) + continue; + + added.push_back( json{ { "sexpr", sexpr }, { "parent", nullptr } } ); + } } rebaseline(); diff --git a/wasm/bindings/pcbnew_embind.cpp b/wasm/bindings/pcbnew_embind.cpp index 976ab9f..f2fba4d 100644 --- a/wasm/bindings/pcbnew_embind.cpp +++ b/wasm/bindings/pcbnew_embind.cpp @@ -854,6 +854,13 @@ void flushDiff() json wAdded = json::array(), wChanged = json::array(); std::set wDone; + // Roots that reached the dirty set already STRUCT_DELETED. The baseline + // diff can miss such a deletion (observed 2026-08-31: 8 footprints deleted + // by a netlist-update/undo churn were P-5-skipped with removed:0 and + // survived in the board room forever) — collect them here and emit the + // removals explicitly below instead of dropping the event. + std::set deletedDirtyRoots; + auto liftBlob = [&]( const std::string& id, json& aArr ) { BOARD_ITEM* live = @@ -892,6 +899,12 @@ void flushDiff() { uuid: UTF8ToString( $0 ), type: UTF8ToString( $1 ), why: UTF8ToString( $2 ), parentNull: !!$3 } ); }, rootId.c_str(), type.c_str(), why.c_str(), parentNull ); + + // A DELETED root must still leave the shared doc: skipping it here + // while the baseline diff misses it strands the item in the room. + if( live->GetFlags() & STRUCT_DELETED ) + deletedDirtyRoots.insert( rootId ); + return; } @@ -973,6 +986,23 @@ void flushDiff() g_dirty.clear(); + // Deleted dirty roots the baseline diff did not catch (the id cache still + // resolves them, and an apply/boot rebaseline can race the commit): emit + // their removal on both wires, and keep them out of the next baseline so + // a redo re-emits them as adds. Duplicate removals are no-ops downstream. + for( const std::string& id : deletedDirtyRoots ) + { + bool alreadyDiffed = g_baseline.count( id ) && !cur.count( id ); + + cur.erase( id ); + + if( alreadyDiffed ) + continue; + + removed.push_back( id ); + wRemoved.push_back( id ); + } + g_baseline = std::move( cur ); if( !added.empty() || !changed.empty() || !removed.empty() ) diff --git a/web/pcbjam-shared b/web/pcbjam-shared index 8dabf2e..a4984c6 160000 --- a/web/pcbjam-shared +++ b/web/pcbjam-shared @@ -1 +1 @@ -Subproject commit 8dabf2e8232f259a52eb2e2382c6a5a866bcbb1d +Subproject commit a4984c626d847bd07ce66148d6ec96fde8ee8e42 diff --git a/web/standalone/src/wasm/collab/sibling-restage.test.ts b/web/standalone/src/wasm/collab/sibling-restage.test.ts index cc39f6c..4e1f6d5 100644 --- a/web/standalone/src/wasm/collab/sibling-restage.test.ts +++ b/web/standalone/src/wasm/collab/sibling-restage.test.ts @@ -2,10 +2,14 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; // Exercise ONLY the subscribe/debounce/restage orchestration: the room connect, // the ydoc materialization, and the MEMFS write are all collaborators. -const { connectKicadDoc, restageFile, ydocHasState } = vi.hoisted(() => ({ +const { connectKicadDoc, restageFile, ydocHasState, docToFile } = vi.hoisted(() => ({ connectKicadDoc: vi.fn(), restageFile: vi.fn(), ydocHasState: vi.fn(), + docToFile: vi.fn( + (_doc: unknown, _opts?: { onMissingItem?: (u: string) => void }) => + "(kicad_sch materialized)", + ), })); vi.mock("./index", () => ({ connectKicadDoc })); @@ -15,7 +19,7 @@ vi.mock("@pcbjam/shared", () => ({ ydocHasState, ydocIsHollow: () => false, yToDoc: (doc: unknown) => doc, - docToFile: () => "(kicad_sch materialized)", + docToFile, })); import { startSiblingRestage, type SiblingPresence } from "./sibling-restage"; @@ -99,6 +103,7 @@ beforeEach(() => { }); restageFile.mockReset(); ydocHasState.mockReset().mockReturnValue(true); + docToFile.mockReset().mockReturnValue("(kicad_sch materialized)"); }); afterEach(() => { @@ -130,6 +135,34 @@ describe("startSiblingRestage", () => { expect(restageFile.mock.calls[0]![2]).toBe("main.kicad_sch"); }); + it("restages leniently past dangling item refs, and warns (2026-08-31 corruption)", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + docToFile.mockImplementation( + (_doc: unknown, opts?: { onMissingItem?: (u: string) => void }) => { + opts?.onMissingItem?.("ghost-1"); + return "(kicad_sch healed)"; + }, + ); + await start(["main.kicad_sch"]); + expect(restageFile).toHaveBeenCalledTimes(1); + expect(warn).toHaveBeenCalledWith(expect.stringContaining("dangling item ref")); + warn.mockRestore(); + }); + + it("a restage failure is loud, not just debug-logged", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + docToFile.mockImplementation(() => { + throw new Error("renderItem: cycle through item x"); + }); + await start(["main.kicad_sch"]); + expect(restageFile).not.toHaveBeenCalled(); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining("restage failed"), + expect.any(Error), + ); + warn.mockRestore(); + }); + it("leaves the boot snapshot alone when the room is empty", async () => { ydocHasState.mockReturnValue(false); await start(["main.kicad_sch"]); diff --git a/web/standalone/src/wasm/collab/sibling-restage.ts b/web/standalone/src/wasm/collab/sibling-restage.ts index a47d1fc..907353b 100644 --- a/web/standalone/src/wasm/collab/sibling-restage.ts +++ b/web/standalone/src/wasm/collab/sibling-restage.ts @@ -112,9 +112,23 @@ export async function startSiblingRestage(opts: { // A hollow doc (layout only, never seeded) would restage a title-block-only // file over the real one — the staged copy is the freshest there is. if (ydocIsHollow(doc)) return; - const text = docToFile(yToDoc(doc)); + // Render tolerantly: a doc carrying a dangling item ref (pre-fix + // paste-collision corruption, 2026-08-31) must still restage — a strict + // throw here silently froze the MEMFS copy at its last good state, so + // "update PCB from schematic" kept seeing deleted symbols forever. + const missing: string[] = []; + const text = docToFile(yToDoc(doc), { onMissingItem: (u) => missing.push(u) }); + if (missing.length) { + console.warn( + `[sibling] ${sheetPath}: dropped ${missing.length} dangling item ref(s): ` + + missing.slice(0, 5).join(", "), + ); + } restageFile(win, slug, sheetPath, new TextEncoder().encode(text), log); } catch (err) { + // Loud on purpose: a swallowed failure here leaves the sibling mirror + // permanently stale with no visible symptom. + console.warn(`[sibling] restage failed for ${sheetPath}:`, err); log(`[sibling] restage failed for ${sheetPath}: ${String(err)}`); } };