diff --git a/kicad b/kicad index 39da876..d0afbc4 160000 --- a/kicad +++ b/kicad @@ -1 +1 @@ -Subproject commit 39da876c2df6326392b240453a485f28f91231f8 +Subproject commit d0afbc4bd35e19bb98685ceecd53f848914cff9d diff --git a/tests/web/findings-p1.spec.ts b/tests/web/findings-p1.spec.ts index 56a2486..1cd56fa 100644 --- a/tests/web/findings-p1.spec.ts +++ b/tests/web/findings-p1.spec.ts @@ -79,16 +79,24 @@ test('P-1 standalone: fit lands after a keyboard rotate of a clicked footprint', await bootBoard(page, 'alice'); const fp = await firstFootprint(page); expect(await fitLands(page, { cx: fp.x, cy: fp.y, hw: 15e6, hh: 10e6 }), 'fit before edit').toBe(true); - const box = await glBox(page); - const vp = await viewport(page); - const sx = box.x + (fp.x - vp.cx) * vp.scale + vp.w / 2; - const sy = box.y + (fp.y - vp.cy) * vp.scale + vp.h / 2; - await page.mouse.move(sx, sy); - await page.waitForTimeout(300); // eslint-disable-line -- pointer dwell - await page.mouse.down(); - await page.mouse.up(); - await page.waitForTimeout(400); // eslint-disable-line -- selection dwell - const sel = await page.evaluate(() => JSON.parse((window as unknown as W).Module.kicadCollabGetSelection())); + // Click-select, re-projected from the LIVE viewport each attempt: on CI the + // first click after the fit can land before the canvas has re-rendered at + // the new zoom (selection = []), so poll the real selection, not one click. + const selection = (): Promise => + page.evaluate(() => JSON.parse((window as unknown as W).Module.kicadCollabGetSelection())); + await expect.poll(async () => { + const box = await glBox(page); + const vp = await viewport(page); + const sx = box.x + (fp.x - vp.cx) * vp.scale + vp.w / 2; + const sy = box.y + (fp.y - vp.cy) * vp.scale + vp.h / 2; + await page.mouse.move(sx, sy); + await page.waitForTimeout(300); // eslint-disable-line -- documented interaction dwell (pointer hover before press) + await page.mouse.down(); + await page.mouse.up(); + await page.waitForTimeout(400); // eslint-disable-line -- documented interaction dwell (selection tool commit) + return (await selection()).includes(fp.id); + }, { timeout: 20000, intervals: [500], message: 'click-select of the first footprint' }).toBe(true); + const sel = await selection(); const r0 = await rotation(page, fp.id); await page.keyboard.press('r'); const rotated = await expect.poll(() => rotation(page, fp.id), { timeout: 10000 }).not.toBe(r0).then(() => true, () => false); diff --git a/tests/web/items-wire-batch-loss.spec.ts b/tests/web/items-wire-batch-loss.spec.ts index 4656a97..85d43f6 100644 --- a/tests/web/items-wire-batch-loss.spec.ts +++ b/tests/web/items-wire-batch-loss.spec.ts @@ -13,15 +13,20 @@ import { test, expect, type Page } from '@playwright/test'; * lost with it — silently: the throw lands in the C++ caller and surfaces only as * a bare pageerror. * - * The unresolvable entry is real, and is not fabricated here. The emit side - * serializes a non-footprint item as `Format(item)` wrapped in a - * `(kicad_pcb … (layers …) )` envelope, and KiCad's board writer emits - * NOTHING for a footprint field — `case PCB_FIELD_T: break;` - * (pcb_io_kicad_sexpr.cpp:411), correctly, because a field is written by its - * footprint's own writer and is never standalone board content. The result is an - * envelope with no item in it. This test reads that blob from the real serializer - * through `kicadCollabTestItemBlob` — the same function `blobForItem` calls — and - * asserts its shape before using it. + * The unresolvable entry was real. The emit side serializes a non-footprint + * item as `Format(item)` wrapped in a `(kicad_pcb … (layers …) )` + * envelope, and KiCad's board writer emits NOTHING for a footprint field — + * `case PCB_FIELD_T: break;` (pcb_io_kicad_sexpr.cpp:411), correctly, because a + * field is written by its footprint's own writer and is never standalone board + * content. The result was an envelope with no item in it. + * + * Findings P-5 closed the emit side: `blobForItem` now refuses to blob a field + * on its own (returns "") and `liftBlob` skips empty blobs, so the editor no + * longer produces the hollow envelope. That contract is asserted below through + * `kicadCollabTestItemBlob` (the same function `blobForItem` backs). The + * receive-side guard is independent of it — any sender, any version, any + * transient — so the poisoned entry is now built here in the exact shape the + * writer used to emit: a board envelope with a layer table and no item. * * Seen in the field: "Update PCB from Schematic" produced a 67-entry `changed` * batch in which one entry was that empty envelope. All 67 were dropped, so two @@ -135,7 +140,7 @@ test('a single un-unwrappable entry must not discard the rest of the batch', asy await bootBoard(bob, 'bob'); // ── the payloads, all straight out of the editor ────────────────────────── - // Two footprints; the second one's field supplies the poisoned entry. + // Two footprints; the second one's field pins the P-5 emit contract. const picked = await alice.evaluate(() => { const M = (window as unknown as W).Module; const snap = JSON.parse(M.kicadCollabSnapshotItems()) as { added: WireItem[] }; @@ -150,17 +155,33 @@ test('a single un-unwrappable entry must not discard the rest of the batch', asy const fp1 = fps[0]; const fp2 = fps.find((f, i) => i > 0 && !!f.field); if (!fp1 || !fp2) return null; - return { fp1, fp2, fieldBlob: M.kicadCollabTestItemBlob(fp2.field!) }; + return { + fp1, + fp2, + fieldBlob: M.kicadCollabTestItemBlob(fp2.field!), + fpBlob: M.kicadCollabTestItemBlob(fp1.uuid), + }; }); expect(picked, 'demo board should have two footprints, the second with a field').toBeTruthy(); - const { fp1, fp2, fieldBlob } = picked!; + const { fp1, fp2, fieldBlob, fpBlob } = picked!; - // The emit-side serializer, asked for a field on its own, yields an envelope - // with no item in it. This is the entry `unwrapWireItem` cannot resolve. - expect(fieldBlob, 'field blob is not empty text').toBeTruthy(); - expect(fieldBlob, 'field blob is a board envelope').toContain('(kicad_pcb'); - expect(fieldBlob, 'field blob carries the layer table').toContain('(layers'); - expect(fieldBlob, 'field blob contains NO item — this is the defect').not.toContain('(uuid'); + // P-5 contract (emit side): a field asked for on its own is NOT blobbed — the + // serializer yields empty text, never the hollow envelope. A footprint asked + // for the same way still serializes (bare, CTL_FOR_BOARD) with its uuid. + expect(fieldBlob, 'P-5: a standalone field blob is empty text').toBe(''); + expect(fpBlob, 'footprint blob is the footprint form').toMatch(/^\s*\(footprint\b/); + expect(fpBlob, 'footprint blob carries the item').toContain(`(uuid "${fp1.uuid}")`); + + // The poisoned entry, in the exact shape the writer used to emit for a field: + // a board envelope with a layer table and no uuid-bearing item in it. This is + // the entry `unwrapWireItem` cannot resolve (0 candidates). + const hollowBlob = + '(kicad_pcb (version 20240108) (generator "pcbnew") (generator_version "9.0")\n' + + ' (general (thickness 1.6) (legacy_teardrops no))\n' + + ' (paper "A4")\n' + + ' (layers (0 "F.Cu" signal) (31 "B.Cu" signal) (44 "Edge.Cuts" user))\n' + + ')'; + expect(hollowBlob, 'hollow blob contains NO item — the defect shape').not.toContain('(uuid'); const fp2Before = await positionOf(bob, fp2.uuid); expect(fp2Before, 'bob should already hold fp2').not.toBe('(absent)'); @@ -181,7 +202,7 @@ test('a single un-unwrappable entry must not discard the rest of the batch', asy .not.toBe(fp1Before); const fp1AfterControl = await positionOf(bob, fp1.uuid); - // ── 2. the defect: a good entry batched with the field entry ────────────── + // ── 2. the defect: a good entry batched with the hollow entry ───────────── // Deliberately NOT asserted on: today this returns the `unwrapWireItem: … // found 0` throw, and once the conversion skips an entry it cannot convert it // will return 'no throw'. Requiring either would pin the test to one side of @@ -189,7 +210,7 @@ test('a single un-unwrappable entry must not discard the rest of the batch', asy // this batch must reach the peer — so the outcome is recorded, not enforced. const reproEmit = await emit(alice, [ { sexpr: moveTo(fp2.sexpr, bumped(fp2.sexpr, 3.3, 4.4)), parent: null }, - { sexpr: fieldBlob, parent: null }, + { sexpr: hollowBlob, parent: null }, ]); test.info().annotations.push({ type: 'repro emit', description: reproEmit }); diff --git a/tests/web/presence-ghost-peer.spec.ts b/tests/web/presence-ghost-peer.spec.ts index 405cc1a..aec7ea7 100644 --- a/tests/web/presence-ghost-peer.spec.ts +++ b/tests/web/presence-ghost-peer.spec.ts @@ -89,6 +89,13 @@ for (const mode of ['half-open', 'unclean-close'] as const) { const ctxA = await browser.newContext(); const ctxB = await browser.newContext(); const alice = await bootAs(ctxA, 'alice'); + // Gate BEFORE the roster assertion: on the BroadcastChannel provider (CI's + // standalone preview has no apps/sync) two browser CONTEXTS never see each + // other, so bob would wait 30s for an alice that cannot arrive. + const liveWs = await alice.evaluate( + () => (window as unknown as { __wsList: Array<{ ws: WebSocket; url: string }> }).__wsList.filter((w) => w.ws.readyState === 1 && /\/parties\//.test(w.url)).length, + ); + test.skip(liveWs === 0, 'no live websocket on alice — BC stack (no apps/sync), nothing to lose'); const bob = await bootAs(ctxB, 'bob'); await openOverlayMenu(bob); await expect(bob.locator('[data-presence-user="alice"]')).toBeVisible({ timeout: 30000 }); diff --git a/tests/wizard-04-finish-headless.png b/tests/wizard-04-finish-headless.png index 0b15aa0..afd5951 100644 Binary files a/tests/wizard-04-finish-headless.png and b/tests/wizard-04-finish-headless.png differ diff --git a/wasm/bindings/collab_common.h b/wasm/bindings/collab_common.h index 9e2138b..246e80b 100644 --- a/wasm/bindings/collab_common.h +++ b/wasm/bindings/collab_common.h @@ -107,9 +107,18 @@ inline void drainApplies() if( applyBusy() ) { if( !slot.done ) - return; // suspended body still in flight — its tail re-drains + { + if( slot.cor && slot.cor->CanResume() ) + return; // suspended body still in flight — its tail re-drains - reapApply(); // completed since the last drain + // P-1: the body's activation died without finishing (its promising entry + // REJECTED — a JS exception out of a wire callback, or a trap — and libcontext + // flagged the record dead). Its tail never runs, so nothing else would ever + // reap it: release the slot here and keep draining. + EM_ASM( { console.error( '[pcbjam collab] apply body died mid-flight — slot released' ); } ); + } + + reapApply(); // completed (or died) since the last drain } auto& q = applyQueue(); @@ -126,11 +135,13 @@ inline void drainApplies() ApplySlot& sl = activeApplySlot(); // P-1 hardening (findings group P): `done` is the ONLY completion signal, so a - // body that unwinds — a C++ exception, or a foreign (JS) exception thrown out of - // an EM_ASM wire callback inside flushDiff and surfacing here as `...` under - // wasm EH — would leave applyBusy() true for the page's life and silently - // queue every later job behind it: fit/pan from JS, remote applies AND the - // local-edit flushDiff (local edits stop syncing). Always mark the slot done. + // body that unwinds on a C++ exception would leave applyBusy() true for the + // page's life and silently queue every later job behind it: fit/pan from JS, + // remote applies AND the local-edit flushDiff (local edits stop syncing). + // Always mark the slot done. NOTE: a JS exception thrown out of an EM_ASM wire + // callback does NOT surface here — under JSPI it rejects the promising entry + // and this frame is simply abandoned; that case is covered by the wire + // emitters' own try/catch plus the dead-body reap in drainApplies(). try { ( *sl.body )(); @@ -154,7 +165,12 @@ inline void drainApplies() slot.cor->Call( 0 ); if( !slot.done ) - return; // suspended — cor/body stay pinned for the resume + { + if( slot.cor->CanResume() ) + return; // suspended — cor/body stay pinned for the resume + + EM_ASM( { console.error( '[pcbjam collab] apply body died at entry — slot released' ); } ); + } reapApply(); } @@ -183,7 +199,12 @@ inline void emitDelta( const nlohmann::json& aDelta ) std::string s = aDelta.dump(); EM_ASM( { if( window.kicadCollab && window.kicadCollab.onDelta ) - window.kicadCollab.onDelta( UTF8ToString( $0 ) ); + { + // A throwing listener must never unwind the wasm frame that called it: under + // JSPI that rejects the running coroutine's entry (findings P-1). + try { window.kicadCollab.onDelta( UTF8ToString( $0 ) ); } + catch( e ) { console.error( '[pcbjam collab] onDelta listener threw', e ); } + } }, s.c_str() ); } @@ -193,7 +214,12 @@ inline void emitItemsWire( const nlohmann::json& aWire ) std::string s = aWire.dump(); EM_ASM( { if( window.kicadCollab && window.kicadCollab.onItems ) - window.kicadCollab.onItems( UTF8ToString( $0 ) ); + { + // A throwing listener must never unwind the wasm frame that called it: under + // JSPI that rejects the running coroutine's entry (findings P-1). + try { window.kicadCollab.onItems( UTF8ToString( $0 ) ); } + catch( e ) { console.error( '[pcbjam collab] onItems listener threw', e ); } + } }, s.c_str() ); } @@ -202,7 +228,12 @@ inline void emitCursor( double aX, double aY, bool aActive ) { EM_ASM( { if( window.kicadCollab && window.kicadCollab.onCursor ) - window.kicadCollab.onCursor( $0, $1, $2 ); + { + // A throwing listener must never unwind the wasm frame that called it: under + // JSPI that rejects the running coroutine's entry (findings P-1). + try { window.kicadCollab.onCursor( $0, $1, $2 ); } + catch( e ) { console.error( '[pcbjam collab] onCursor listener threw', e ); } + } }, aX, aY, aActive ? 1 : 0 ); } @@ -211,7 +242,12 @@ inline void emitSelection( const std::string& aJson ) { EM_ASM( { if( window.kicadCollab && window.kicadCollab.onSelection ) - window.kicadCollab.onSelection( UTF8ToString( $0 ) ); + { + // A throwing listener must never unwind the wasm frame that called it: under + // JSPI that rejects the running coroutine's entry (findings P-1). + try { window.kicadCollab.onSelection( UTF8ToString( $0 ) ); } + catch( e ) { console.error( '[pcbjam collab] onSelection listener threw', e ); } + } }, aJson.c_str() ); } @@ -220,7 +256,12 @@ inline void emitViewport( double aCx, double aCy, double aPxPerIu, int aW, int a { EM_ASM( { if( window.kicadCollab && window.kicadCollab.onViewport ) - window.kicadCollab.onViewport( $0, $1, $2, $3, $4 ); + { + // A throwing listener must never unwind the wasm frame that called it: under + // JSPI that rejects the running coroutine's entry (findings P-1). + try { window.kicadCollab.onViewport( $0, $1, $2, $3, $4 ); } + catch( e ) { console.error( '[pcbjam collab] onViewport listener threw', e ); } + } }, aCx, aCy, aPxPerIu, aW, aH ); } diff --git a/wasm/bindings/eeschema_embind.cpp b/wasm/bindings/eeschema_embind.cpp index 33690f5..9d82c40 100644 --- a/wasm/bindings/eeschema_embind.cpp +++ b/wasm/bindings/eeschema_embind.cpp @@ -381,7 +381,12 @@ void emitSheetChanged() std::string s = toUtf8( screen->GetFileName() ); EM_ASM( { if( window.kicadCollab && window.kicadCollab.onSheetChanged ) - window.kicadCollab.onSheetChanged( UTF8ToString( $0 ) ); + { + // A throwing listener must never unwind the wasm frame that called it: under + // JSPI that rejects the running coroutine's entry (findings P-1). + try { window.kicadCollab.onSheetChanged( UTF8ToString( $0 ) ); } + catch( e ) { console.error( '[pcbjam collab] onSheetChanged listener threw', e ); } + } }, s.c_str() ); } @@ -637,7 +642,12 @@ void scheduleSheetSave( SCH_SHEET* aSheet ) EM_ASM( { if( window.kicadCollab && window.kicadCollab.onSheetCreated ) - window.kicadCollab.onSheetCreated( UTF8ToString( $0 ) ); + { + // A throwing listener must never unwind the wasm frame that called it: under + // JSPI that rejects the running coroutine's entry (findings P-1). + try { window.kicadCollab.onSheetCreated( UTF8ToString( $0 ) ); } + catch( e ) { console.error( '[pcbjam collab] onSheetCreated listener threw', e ); } + } }, childAbs.c_str() ); } ); } @@ -1876,7 +1886,12 @@ extern "C" void kicadCollabOnSave( const char* aPath ) { EM_ASM( { if( window.kicadCollab && window.kicadCollab.onSave ) - window.kicadCollab.onSave( UTF8ToString( $0 ) ); + { + // A throwing listener must never unwind the wasm frame that called it: under + // JSPI that rejects the running coroutine's entry (findings P-1). + try { window.kicadCollab.onSave( UTF8ToString( $0 ) ); } + catch( e ) { console.error( '[pcbjam collab] onSave listener threw', e ); } + } }, aPath ); } #endif // !KICAD_MERGED_EMBIND diff --git a/wasm/bindings/kicad_editor_embind.cpp b/wasm/bindings/kicad_editor_embind.cpp index 92c7af0..0316c15 100644 --- a/wasm/bindings/kicad_editor_embind.cpp +++ b/wasm/bindings/kicad_editor_embind.cpp @@ -353,7 +353,12 @@ extern "C" void kicadCollabOnSave( const char* aPath ) { EM_ASM( { if( window.kicadCollab && window.kicadCollab.onSave ) - window.kicadCollab.onSave( UTF8ToString( $0 ) ); + { + // A throwing listener must never unwind the wasm frame that called it: under + // JSPI that rejects the running coroutine's entry (findings P-1). + try { window.kicadCollab.onSave( UTF8ToString( $0 ) ); } + catch( e ) { console.error( '[pcbjam collab] onSave listener threw', e ); } + } }, aPath ); } diff --git a/wasm/bindings/pl_editor_embind.cpp b/wasm/bindings/pl_editor_embind.cpp index d267ee2..ae679ae 100644 --- a/wasm/bindings/pl_editor_embind.cpp +++ b/wasm/bindings/pl_editor_embind.cpp @@ -227,7 +227,12 @@ void emit( const json& aDelta ) std::string s = aDelta.dump(); EM_ASM( { if( window.kicadCollab && window.kicadCollab.onDelta ) - window.kicadCollab.onDelta( UTF8ToString( $0 ) ); + { + // A throwing listener must never unwind the wasm frame that called it: under + // JSPI that rejects the running coroutine's entry (findings P-1). + try { window.kicadCollab.onDelta( UTF8ToString( $0 ) ); } + catch( e ) { console.error( '[pcbjam collab] onDelta listener threw', e ); } + } }, s.c_str() ); } @@ -239,7 +244,12 @@ void emitItems( const json& aWire ) std::string s = aWire.dump(); EM_ASM( { if( window.kicadCollab && window.kicadCollab.onItems ) - window.kicadCollab.onItems( UTF8ToString( $0 ) ); + { + // A throwing listener must never unwind the wasm frame that called it: under + // JSPI that rejects the running coroutine's entry (findings P-1). + try { window.kicadCollab.onItems( UTF8ToString( $0 ) ); } + catch( e ) { console.error( '[pcbjam collab] onItems listener threw', e ); } + } }, s.c_str() ); } @@ -455,7 +465,12 @@ extern "C" void kicadCollabOnSave( const char* aPath ) { EM_ASM( { if( window.kicadCollab && window.kicadCollab.onSave ) - window.kicadCollab.onSave( UTF8ToString( $0 ) ); + { + // A throwing listener must never unwind the wasm frame that called it: under + // JSPI that rejects the running coroutine's entry (findings P-1). + try { window.kicadCollab.onSave( UTF8ToString( $0 ) ); } + catch( e ) { console.error( '[pcbjam collab] onSave listener threw', e ); } + } }, aPath ); } diff --git a/web/pcbjam-shared b/web/pcbjam-shared index ff8835c..8dabf2e 160000 --- a/web/pcbjam-shared +++ b/web/pcbjam-shared @@ -1 +1 @@ -Subproject commit ff8835ccc173b5ff9b35ade95440793244ef27b4 +Subproject commit 8dabf2e8232f259a52eb2e2382c6a5a866bcbb1d diff --git a/wxwidgets b/wxwidgets index 9666a74..a7436d1 160000 --- a/wxwidgets +++ b/wxwidgets @@ -1 +1 @@ -Subproject commit 9666a743bcd236cd33034c4cf75ecf3c6684c541 +Subproject commit a7436d105271815592943aebfc91f42295a74b48