AI_Assistant/pcbjam
Watch
1
0
Fork
You've already forked pcbjam
 0
Code Issues Pull requests Projects Releases 7 Packages Wiki Activity Actions

findings(E-10..E-22): fix the defects a code review found in the E-1..E-9 work

Browse source 
A review of the group-E fixes found 13 further defects; ten were introduced by
those fixes, two pre-existed and were merely relocated, one is deferred.

Services / transport
  E-10  retireWorker synthesized no bg/exit frame, so sharedspice's s_bgRunning
        mirror stayed latched true after a mid-run worker death: Run stayed
        disabled and the promised fresh-worker restart was unreachable for the
        whole session. Retirement now dispatches a synthetic controlled-exit
        straight to the installed handler (never through dispatchEvt — a
        fabricated frame must not touch the credit ledger). Driving the repro
        exposed two further defects, both fixed here: a replacement worker
        trapped on pre-init engine reads, and the rerun's cm_input_path/circ hit
        that uninitialized engine before KiCad's validate() re-init (the native
        flow assumes a crashed engine survives in-process — true for the dll,
        false for a dead worker). Reads now answer their empty shapes pre-init,
        writes lazy-init, and init is idempotent per worker engine.
  E-19  dispatchEvt acked only AFTER handler(evt) returned, and the sharedspice
        client deliberately rethrows non-trap errors — so each throw leaked one
        unit of the 64-frame credit window until the stream died with a
        misattributed "transport exceeded". The ack moves to a finally in both
        service copies; the throw still propagates (the trap machinery needs it).
  E-20  the oversize-line path promises to transfer the accepted prefix, but
        with the window full that flush only DEFERS, and stopEventStream wiped
        the deferred queue — losing the diagnostics that explain the failure.
        The terminal notice now carries them as pendingEvents; both hosts
        deliver them in order, unacked (the fatal frame is outside the credit
        protocol).
  E-21  the 30s prefetch deadline discarded every model already collected and
        reported nothing. A caller-owned progress sink ships the partials and
        the omission reaches the export report. (Awaiting the aborted collection
        was rejected: an in-flight source fetch is not abortable — E-4's
        original disease.) Plus a serving-candidate memo, so a .wrl ref served
        by its .step fallback stops re-probing the miss on every export.

Scheduler
  E-14  _terminalizeNativeTrap classified by message substring, so any plain JS
        error QUOTING 'Aborted(' or 'out of bounds' permanently bricked a
        healthy instance. Now structural only: instanceof RuntimeError plus a
        duck-typed name check (verified in this build's glue that abort() throws
        a genuine RuntimeError both pre- and post-runtime-init). Module.onAbort
        now latches the gate — the authoritative notification, previously
        ignored.
  E-15  the shim half: _pumpResume gates on terminal (catching wakes already
        queued at latch time) and resolveWait refuses on terminal WITHOUT
        consuming the entry, so a frame stays visibly parked rather than
        resuming inside a trapped module.
  E-16  the E-5 handler read the realm-global scheduler at dispatch instead of
        its installing module's; also frees the per-line buffer on the non-trap
        rethrow path.
  E-11  get_vec trusted the worker's res.length over the transferred arrays.
        Observed death shape: a 4 GiB std::vector threw an unhandled
        std::length_error that exited the editor's main loop. Now clamped, with
        the buffers freed on every failure path.

Guardrails (replacing two deferred refactors: e2e→production-code injection and
collapsing the four copies of the worker-lifecycle machinery)
  E-18  the source contract asserted comment-string counts — rewording failed
        CI while moving a guard outside its #ifdef passed. It now parses the
        #ifdef regions and asserts on code.
        service-stub-parity.ts pins what the four lifecycle copies must share:
        credit-window equality parsed from source, the finally-ack, boot
        deadlines, terminal-notice consumption. The transport numbers are now
        single-sourced from the worker.
        CI actually runs the gates: the web/standalone vitest suites (which had
        NEVER run in CI), the reducer, the source contract and the parity tool —
        with a NON_PLAYWRIGHT_GATES check so deleting a step re-fails the lint.
  E-22  the e2e occ stub's 60s boot watchdog, deleted in a66e109, is restored in
        the ngspice-stub shape with a wedgeNextBoot() repro hook.

Every behavioral fix has red-then-green evidence (the reds were captured first).
E-17 (a stale RUNNING cross-stamping the next run's generation under E-6's
transport deferral) is DEFERRED with its analysis recorded — a real fix needs
run identity on the bg frames.

Test hygiene: the dwell lint now requires the mandated ": <why>" and all 47 bare
markers carry their reason; three export-report dwells became modal-lease polls;
exact-ledger assertions became relative deltas; the dead data-wx-dom-id branch,
an unused fault hook and unused receipt plumbing are gone; abort scans, wx
dialog drivers, the sim harness and the vitest FakeWorker are each one copy now.

Bumps kicad and wxwidgets to their findings-group-e tips.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Istvan Matejcsok 2026-08-27 12:27:12 +02:00
commit c421d724b0
48 changed files with 2071 additions and 694 deletions
Download patch file Download diff file Expand all files Collapse all files

139
tests/kicad/utils/occ-service.ts
View file

@ -39,10 +39,23 @@ const OCC_WORKER_SRC = fs.readFileSync(
'web', 'standalone', 'src', 'wasm', 'occ-worker.js'),
'utf8');
export async function installOccServiceStub(page: Page): Promise<void> {
await page.addInitScript((workerSrc: string) => {
export interface OccHarnessWatchdogs {
bootTimeoutMs?: number;
}
export async function installOccServiceStub(
page: Page,
watchdogs: OccHarnessWatchdogs = {},
): Promise<void> {
const bootTimeoutMs = watchdogs.bootTimeoutMs ?? 2 * 60_000;
if (!Number.isSafeInteger(bootTimeoutMs) || bootTimeoutMs < 1)
throw new Error('bootTimeoutMs must be a positive safe integer');
await page.addInitScript((options: { workerSrc: string; bootTimeoutMs: number }) => {
if ((globalThis as any).occService) return;
const { workerSrc, bootTimeoutMs } = options;
(window as any).__occExports = [];
interface WorkerSlot {
@ -50,6 +63,9 @@ export async function installOccServiceStub(page: Page): Promise<void> {
worker?: Worker;
failed: boolean;
ready: Promise<WorkerSlot>;
bootTimer?: ReturnType<typeof setTimeout>;
rejectBoot?: (reason?: unknown) => void;
removeBootListener?: () => void;
/** The exact lifecycle transition used by Worker.onmessageerror. */
failDecode: () => void;
}
@ -67,11 +83,9 @@ export async function installOccServiceStub(page: Page): Promise<void> {
let requestsStarted = 0;
let requestsPosted = 0;
const workerGenerationsStarted: number[] = [];
let armedFault: {
count: number;
kind: 'terminate' | 'messageerror';
report: string;
} | null = null;
let armedFault: { count: number; report: string } | null = null;
/** One-shot: the next boot uses a worker that never answers. */
let wedgeNextBootArmed: { bootTimeoutMs?: number } | null = null;
const retiredGenerations: number[] = [];
const pendingInGeneration = (generation: number): number => {
@ -94,6 +108,12 @@ export async function installOccServiceStub(page: Page): Promise<void> {
if (slot.failed) return;
slot.failed = true;
retiredGenerations.push(slot.generation);
if (slot.bootTimer !== undefined) {
clearTimeout(slot.bootTimer);
slot.bootTimer = undefined;
}
slot.removeBootListener?.();
slot.removeBootListener = undefined;
failPending(slot.generation, report);
if (workerSlot === slot) workerSlot = null;
try {
@ -101,21 +121,22 @@ export async function installOccServiceStub(page: Page): Promise<void> {
} catch {
/* already gone */
}
const reject = slot.rejectBoot;
slot.rejectBoot = undefined;
reject?.(new Error(report));
};
const maybeTriggerArmedFault = (slot: WorkerSlot): void => {
if (!armedFault || slot.failed) return;
if (pendingInGeneration(slot.generation) < armedFault.count) return;
const { kind, report } = armedFault;
const { report } = armedFault;
armedFault = null;
console.log(`[TEST-OCC] faulting generation ${slot.generation} (${kind}): ${report}`);
if (kind === 'messageerror' && slot.worker) {
console.log(`[TEST-OCC] faulting generation ${slot.generation} (messageerror): ${report}`);
if (slot.worker) {
// Synthetic dispatch on Worker is engine-dependent. Invoke
// the exact transition installed as the real event handler.
slot.failDecode();
} else {
// Worker.terminate() intentionally emits no error event, so
// the hook supplies the fatal lifecycle transition explicitly.
retireWorker(slot, report);
}
};
@ -127,36 +148,53 @@ export async function installOccServiceStub(page: Page): Promise<void> {
failed: false,
} as WorkerSlot;
workerGenerationsStarted.push(slot.generation);
workerSlot = slot;
slot.ready = (async () => {
const wedge = wedgeNextBootArmed;
wedgeNextBootArmed = null;
const bootDeadlineMs = wedge?.bootTimeoutMs ?? bootTimeoutMs;
// Legible boot (E-22): a worker DEATH shape that never posts
// ready OR bootError (importScripts hang, pthread spawn
// wedge, OOM-kill) used to hang every request until the
// spec's timeout with zero evidence. Bound the boot — same
// shape as the ngspice stub and the production service.
const bootDeadline = new Promise<never>((_resolve, reject) => {
slot.rejectBoot = reject;
slot.bootTimer = setTimeout(() => {
if (slot.failed || workerSlot !== slot) return;
const why = `occ_service boot timed out after ${bootDeadlineMs} ms`;
console.log(`[TEST-OCC] ${why} — resetting service`);
retireWorker(slot, why);
}, bootDeadlineMs);
});
const boot = (async () => {
const glue = new URL('occ_service.js', window.location.href).href;
console.log(`[TEST-OCC] booting occ_service from ${glue}`);
const worker = new Worker(URL.createObjectURL(new Blob(
[`self.OCC_GLUE_URL = ${JSON.stringify(glue)};\n`, workerSrc],
{ type: 'text/javascript' })));
// A wedged boot is a REAL silent Worker (an empty module:
// it boots, runs nothing, never posts ready/bootError) —
// the importScripts-hang / pthread-wedge shape, engine
// independent.
const worker = wedge
? new Worker('data:text/javascript,/* [TEST-OCC] wedged boot */')
: new Worker(URL.createObjectURL(new Blob(
[`self.OCC_GLUE_URL = ${JSON.stringify(glue)};\n`, workerSrc],
{ type: 'text/javascript' })));
slot.worker = worker;
let rejectBoot: ((reason?: unknown) => void) | undefined;
let removeBootListener: (() => void) | undefined;
// All fatal transitions settle the boot through the ONE
// retirement funnel (which clears the deadline, removes
// the boot listener, and rejects the raced promise).
worker.onerror = (e) => {
const report = `occ_service crashed: ${e.message || 'worker error'}`;
console.error(`[TEST-OCC] ${report}; resetting service`);
retireWorker(slot, report);
removeBootListener?.();
removeBootListener = undefined;
const reject = rejectBoot;
rejectBoot = undefined;
reject?.(new Error(report));
};
slot.failDecode = () => {
const report = 'occ_service transport failed: message decode failed';
console.error(`[TEST-OCC] ${report}; resetting service`);
retireWorker(slot, report);
removeBootListener?.();
removeBootListener = undefined;
const reject = rejectBoot;
rejectBoot = undefined;
reject?.(new Error(report));
};
worker.onmessageerror = slot.failDecode;
worker.onmessage = (e) => {
@ -169,38 +207,37 @@ export async function installOccServiceStub(page: Page): Promise<void> {
request.resolve(res);
}
};
await new Promise<void>((resolve, reject) => {
rejectBoot = reject;
await new Promise<void>((resolve) => {
const onFirst = (e: MessageEvent) => {
if (e.data?.ready) {
removeBootListener?.();
removeBootListener = undefined;
rejectBoot = undefined;
if (slot.bootTimer !== undefined) {
clearTimeout(slot.bootTimer);
slot.bootTimer = undefined;
}
slot.removeBootListener?.();
slot.removeBootListener = undefined;
slot.rejectBoot = undefined;
resolve();
} else if (e.data?.bootError) {
removeBootListener?.();
removeBootListener = undefined;
rejectBoot = undefined;
const report = `occ_service boot failed: ${String(e.data.bootError)}`;
retireWorker(slot, report);
reject(new Error(report));
retireWorker(slot,
`occ_service boot failed: ${String(e.data.bootError)}`);
}
};
worker.addEventListener('message', onFirst);
removeBootListener = () => worker.removeEventListener('message', onFirst);
slot.removeBootListener = () => worker.removeEventListener('message', onFirst);
});
if (slot.failed || workerSlot !== slot)
throw new Error('occ_service worker retired during boot');
console.log('[TEST-OCC] occ_service ready');
return slot;
})().catch((e) => {
})();
slot.ready = Promise.race([boot, bootDeadline]).catch((e) => {
// A late rejection from a retired generation cannot clear
// the replacement slot created by a new request.
retireWorker(slot, `occ_service unavailable: ${String(e)}`);
throw e;
});
workerSlot = slot;
}
return workerSlot.ready;
};
@ -291,12 +328,13 @@ export async function installOccServiceStub(page: Page): Promise<void> {
};
(globalThis as any).__occServiceTestHooks = {
/** Arm one deterministic host-side fault after N real posts. */
terminateWhenPendingAtLeast(count: number, report = 'occ_service test fault') {
if (!Number.isSafeInteger(count) || count < 1)
throw new Error('pending threshold must be a positive safe integer');
armedFault = { count, kind: 'terminate', report };
if (workerSlot) maybeTriggerArmedFault(workerSlot);
/** One-shot: wedge the next boot (silent worker, no ready and no
* bootError), optionally shortening that boot's deadline. */
wedgeNextBoot(bootTimeoutMs?: number) {
if (bootTimeoutMs !== undefined
&& (!Number.isSafeInteger(bootTimeoutMs) || bootTimeoutMs < 1))
throw new Error('bootTimeoutMs must be a positive safe integer');
wedgeNextBootArmed = { bootTimeoutMs };
},
/** Arm the real Worker's production-parity messageerror handler. */
messageErrorWhenPendingAtLeast(count: number) {
@ -304,7 +342,6 @@ export async function installOccServiceStub(page: Page): Promise<void> {
throw new Error('pending threshold must be a positive safe integer');
armedFault = {
count,
kind: 'messageerror',
report: 'occ_service transport failed: message decode failed',
};
if (workerSlot) maybeTriggerArmedFault(workerSlot);
@ -324,5 +361,5 @@ export async function installOccServiceStub(page: Page): Promise<void> {
};
(globalThis as any).occService = { request };
}, OCC_WORKER_SRC);
}, { workerSrc: OCC_WORKER_SRC, bootTimeoutMs });
}