collab: paste-collision sync-delete fix — lenient sibling restage, forced removals for deleted dirty roots, eeschema child-blob lifting
Field bug (2026-08-31): copy-pasting a symbol whose pins kept their source uuids corrupted the sheet's ydoc on delete (pcbjam-shared: cross-parent re-key fix, bumped here); the pcbnew tab's sibling mirror then silently froze on the dangling refs, so "Update PCB from Schematic" with delete enabled never removed the deleted symbols' footprints — until any later edit resynced the sheet. - standalone sibling-restage: render tolerantly past dangling item refs (docToFile onMissingItem) and console.warn on drops/failures instead of only the gated debug log — a frozen mirror is no longer silent. - pcbnew_embind flushDiff: dirty roots that are already STRUCT_DELETED emit forced removals on both wires instead of being P-5-skipped (observed: 8 footprints deleted on the board stayed in the board room forever); they are erased from the next baseline so a redo re-adds them. - eeschema_embind blobFor: lift an unlifted child (field/pin/sheet-pin) to its screen root before serializing — the selection writer emits nothing for such a child standalone, so the entry used to reach JS as an empty envelope and be skipped, silently dropping the edit; residual empty blobs now warn instead of shipping hollow envelopes. SCH_MARKER_T (ERC artifacts, never file content) stays out of the snapshot and seed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLPKYptXFrxHj5Gu7rhToz
This commit is contained in:
parent
f3119e690f
commit
8a3fc914e5
5 changed files with 143 additions and 9 deletions
|
|
@ -507,7 +507,16 @@ std::map<std::string, json> snapshotByUuid( SCH_EDIT_FRAME* aFrame )
|
||||||
if( SCH_SCREEN* screen = currentScreen( aFrame ) )
|
if( SCH_SCREEN* screen = currentScreen( aFrame ) )
|
||||||
{
|
{
|
||||||
for( SCH_ITEM* item : screen->Items() )
|
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 );
|
m[toUtf8( item->m_Uuid.AsString() )] = itemToJson( item );
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return m;
|
return m;
|
||||||
|
|
@ -595,13 +604,48 @@ void flushDiff()
|
||||||
|
|
||||||
auto blobFor = [&]( const std::string& id, json& aArr )
|
auto blobFor = [&]( const std::string& id, json& aArr )
|
||||||
{
|
{
|
||||||
if( !wDone.insert( id ).second )
|
|
||||||
return;
|
|
||||||
|
|
||||||
KIID kid( wxString::FromUTF8( id.c_str() ) );
|
KIID kid( wxString::FromUTF8( id.c_str() ) );
|
||||||
|
|
||||||
if( SCH_ITEM* item = fr->Schematic().ResolveItem( kid, nullptr, /*allowNull*/ true ) )
|
SCH_ITEM* item = fr->Schematic().ResolveItem( kid, nullptr, /*allowNull*/ true );
|
||||||
aArr.push_back( json{ { "sexpr", itemBlob( fr, item ) }, { "parent", nullptr } } );
|
|
||||||
|
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<SCH_ITEM*>( 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 )
|
for( const auto& [id, j] : cur )
|
||||||
|
|
@ -1357,7 +1401,20 @@ std::string schCollabSnapshotItems()
|
||||||
if( SCH_SCREEN* screen = currentScreen( fr ) )
|
if( SCH_SCREEN* screen = currentScreen( fr ) )
|
||||||
{
|
{
|
||||||
for( SCH_ITEM* item : screen->Items() )
|
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();
|
rebaseline();
|
||||||
|
|
|
||||||
|
|
@ -854,6 +854,13 @@ void flushDiff()
|
||||||
json wAdded = json::array(), wChanged = json::array();
|
json wAdded = json::array(), wChanged = json::array();
|
||||||
std::set<std::string> wDone;
|
std::set<std::string> 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<std::string> deletedDirtyRoots;
|
||||||
|
|
||||||
auto liftBlob = [&]( const std::string& id, json& aArr )
|
auto liftBlob = [&]( const std::string& id, json& aArr )
|
||||||
{
|
{
|
||||||
BOARD_ITEM* live =
|
BOARD_ITEM* live =
|
||||||
|
|
@ -892,6 +899,12 @@ void flushDiff()
|
||||||
{ uuid: UTF8ToString( $0 ), type: UTF8ToString( $1 ),
|
{ uuid: UTF8ToString( $0 ), type: UTF8ToString( $1 ),
|
||||||
why: UTF8ToString( $2 ), parentNull: !!$3 } );
|
why: UTF8ToString( $2 ), parentNull: !!$3 } );
|
||||||
}, rootId.c_str(), type.c_str(), why.c_str(), parentNull );
|
}, 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;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -973,6 +986,23 @@ void flushDiff()
|
||||||
|
|
||||||
g_dirty.clear();
|
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 );
|
g_baseline = std::move( cur );
|
||||||
|
|
||||||
if( !added.empty() || !changed.empty() || !removed.empty() )
|
if( !added.empty() || !changed.empty() || !removed.empty() )
|
||||||
|
|
|
||||||
|
|
@ -1 +1 @@
|
||||||
Subproject commit 8dabf2e8232f259a52eb2e2382c6a5a866bcbb1d
|
Subproject commit a4984c626d847bd07ce66148d6ec96fde8ee8e42
|
||||||
|
|
@ -2,10 +2,14 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
|
|
||||||
// Exercise ONLY the subscribe/debounce/restage orchestration: the room connect,
|
// Exercise ONLY the subscribe/debounce/restage orchestration: the room connect,
|
||||||
// the ydoc materialization, and the MEMFS write are all collaborators.
|
// 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(),
|
connectKicadDoc: vi.fn(),
|
||||||
restageFile: vi.fn(),
|
restageFile: vi.fn(),
|
||||||
ydocHasState: vi.fn(),
|
ydocHasState: vi.fn(),
|
||||||
|
docToFile: vi.fn(
|
||||||
|
(_doc: unknown, _opts?: { onMissingItem?: (u: string) => void }) =>
|
||||||
|
"(kicad_sch materialized)",
|
||||||
|
),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
vi.mock("./index", () => ({ connectKicadDoc }));
|
vi.mock("./index", () => ({ connectKicadDoc }));
|
||||||
|
|
@ -15,7 +19,7 @@ vi.mock("@pcbjam/shared", () => ({
|
||||||
ydocHasState,
|
ydocHasState,
|
||||||
ydocIsHollow: () => false,
|
ydocIsHollow: () => false,
|
||||||
yToDoc: (doc: unknown) => doc,
|
yToDoc: (doc: unknown) => doc,
|
||||||
docToFile: () => "(kicad_sch materialized)",
|
docToFile,
|
||||||
}));
|
}));
|
||||||
|
|
||||||
import { startSiblingRestage, type SiblingPresence } from "./sibling-restage";
|
import { startSiblingRestage, type SiblingPresence } from "./sibling-restage";
|
||||||
|
|
@ -99,6 +103,7 @@ beforeEach(() => {
|
||||||
});
|
});
|
||||||
restageFile.mockReset();
|
restageFile.mockReset();
|
||||||
ydocHasState.mockReset().mockReturnValue(true);
|
ydocHasState.mockReset().mockReturnValue(true);
|
||||||
|
docToFile.mockReset().mockReturnValue("(kicad_sch materialized)");
|
||||||
});
|
});
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
|
|
@ -130,6 +135,34 @@ describe("startSiblingRestage", () => {
|
||||||
expect(restageFile.mock.calls[0]![2]).toBe("main.kicad_sch");
|
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 () => {
|
it("leaves the boot snapshot alone when the room is empty", async () => {
|
||||||
ydocHasState.mockReturnValue(false);
|
ydocHasState.mockReturnValue(false);
|
||||||
await start(["main.kicad_sch"]);
|
await start(["main.kicad_sch"]);
|
||||||
|
|
|
||||||
|
|
@ -112,9 +112,23 @@ export async function startSiblingRestage(opts: {
|
||||||
// A hollow doc (layout only, never seeded) would restage a title-block-only
|
// 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.
|
// file over the real one — the staged copy is the freshest there is.
|
||||||
if (ydocIsHollow(doc)) return;
|
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);
|
restageFile(win, slug, sheetPath, new TextEncoder().encode(text), log);
|
||||||
} catch (err) {
|
} 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)}`);
|
log(`[sibling] restage failed for ${sheetPath}: ${String(err)}`);
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue