#54 loader: publish lastQmatmulKernel/lastThreadReport on error paths

[heskew]

Plan (issue #54 — before / with the code)

Sherlock's #53 note: lastQmatmulKernel and lastThreadReport were published only after a successful instantiate / worker pool. A missing wasm, probe throw, instantiate failure, worker spawn failure, Q4_K policy throw, or GGUF load failure left the previous value (null on first load, or a stale success after a later failed load()). Compare-verdict treats a missing receipt as RED, so the gate was already fail-closed; a consumer reading the reports was not.

Which load-failure sites publish

Site	Where	Attempted artifact (wasm)
resolveQmatmulKernel throw (MILTON_RELAXED_SIMD=1 + probe reject)	ensureWasm sync	intended .wasm basename
Missing prebuilt wasm (existsSync / read)	ensureWasm sync	that basename
Glue import, WebAssembly.Module / instantiate, worker spawn, applyQ4kPolicy	ensureWasm async IIFE	that basename
Missing GGUF / new Milton(bytes) throw	load()	GGUF basename

One helper: publishLoadError. Success still goes through publishThreadReport + lastQmatmulKernel = qk with the same keys as today — no error field.

Error-field shape (Sherlock / Flint ASK)

• error: string from errorText — absolute paths rewritten to basename. GGUF-not-found is built with basename(path).
• Attempted artifact: wasm (basename)
• Success grain unchanged (artifact / workers / availableParallelism / sabAvailable / wasm; kernel forced / kernel / probe)
• Stale-success test asserts published error does not contain tmpdir()

Why reports are never stale

Every throw out of ensureWasm / load overwrites both reports before the throw. A later successful load() republishes the last success grain (Bugbot: no leftover error / failed basename).

Fails-first (Flint) — same child against main 2b5305d

Exit 1 on main: after is the prior success (staleThread=true, staleKernel=true, no error).

Scope

Refs #54 #43 #53. Not Closes. Off merge. Stay draft. Do not request tps-flint / tps-kern / tps-sherlock. Parent pings Cos.

Assign heskew. Token 403'd on assignees. Author is heskew.

No wasm rebuild, no .wasm blob changes, no held-number / bench / kernel / calibration / threads rewrite.

Verification

HEAD de9c68f087ef8d45ba1c6098c85bb78d59ecd671 is green. Failure-then-stale test passes. Epsilon / goldens / expected.json untouched.

node --test harness/test/wasm-thread-report.test.js harness/test/wasm-relaxed-kernel.test.js
# tests 21  # pass 21  # fail 0
#   ok  success then a failed load … (error has no tmpdir path, no '/')
#   ok  successful load after a failed load republishes success
#   ok  probe throw publishes error + attempted artifact

cargo test --manifest-path crate/Cargo.toml
# crate lib ok

Named CI 33819240864 all GREEN: test, dequant, embed-native, wasm, wasm-compare, wasm-compare-node26. must-fail stays RED where it should.

Flint PLAN ACCEPTED. Kern APPROVED. Sherlock APPROVED (path leak closed). Parent pings Cos.

  

[tps-flint]

PLAN ACCEPTED on head 8507e9c — undraft and request tps-kern / tps-sherlock once the lanes are green; K&S briefs go out from my side at the same time. Do not wait on me further.

Matches #54: every load-failure site (probe throw, missing wasm, glue import / instantiate / worker spawn / Q4_K policy, missing GGUF) publishes error plus the attempted wasm basename through one helper; the success shape is unchanged, so no consumer of today's fields moves. The second test — success, then a failed load, and the reports must not read as a stale success — is the case the issue was about and fails on main by construction.

One condition for the PR body, not a code change: paste that stale-success test failing against main before this change (the fails-first record), the way #53 carried its held table. Everything else is fine as it stands.

[tps-sherlock]

SECURITY review — REQUEST CHANGES (one finding; the rest holds).

Finding — the error field leaks the full filesystem path, not just the basename. The prompt's Sherlock check is "confirm no path can leak a filesystem path beyond the wasm basename or env values into the published report." It does.

publishLoadError sets error = errorText(err) = err.message (src/index.js:145, 154), unsanitized. Two load() sites embed the FULL path in that message:

1. GGUF-not-found: fail-closed: GGUF not found at ${path} — ... (src/index.js:274) — path is the full ggufPath (or resolveGguf()), not the basename. The wasm field is correctly basename(path) (src/index.js:271), but error carries the full path.
2. readFileSync(path) (src/index.js:285) — Node's ENOENT message is ENOENT: no such file or directory, open '/full/path/to/file', which also embeds the full path.

So a consumer reading lastThreadReport.error / lastQmatmulKernel.error after a failed load() sees the full filesystem path (home dir, username, project layout) — exactly what the wasm-basename discipline was meant to avoid. The wasm field is clean; error is not.

The wasm-missing path is fine (missingName is a basename, src/index.js:253/259), and the probe-throw path is fine (the message is the fixed MILTON_RELAXED_SIMD=1 string). The leak is specific to load()'s GGUF path.

Fix: sanitize the error before publishing — e.g. build the GGUF-not-found message with basename(path) instead of path, and/or strip the path from err.message in errorText (or pass a sanitized string to publishLoadError). The wasm field already demonstrates the right pattern.

Everything else HOLDS:

1. Coverage — HOLDS. Every throw site reaches publishLoadError: the sync resolveQmatmulKernel throw and existsSync/read throw are caught by the outer try/catch (src/index.js:258-260); the async IIFE's glue-import / instantiate / worker-spawn / applyQ4kPolicy throws are caught by the inner try/catch (src/index.js:252-254); load()'s GGUF-not-found and new Milton throws are caught (src/index.js:273-293). The one residual: a worker that dies after startWorkerPool resolves and publishThreadReport runs is not a throw out of ensureWasm, so it is not re-published — but that is a post-success runtime failure, not a load-failure path, and the report correctly reflects the successful pool start. No stale-success path remains: a rejection after publishThreadReport (e.g. applyQ4kPolicy throws) is caught by the inner catch and overwrites both reports with error.

2. Shape — HOLDS. Success objects are byte-for-byte #53 (the test asserts Object.keys(success.thread) is exactly [artifact, availableParallelism, sabAvailable, wasm, workers] and success.kernel is [forced, kernel, probe] — no error). On failure error is a string and wasm is the attempted basename. lastQmatmulKernel on failure carries no kernel key when the pick didn't complete (probe-throw test asserts got.kernel.kernel === undefined), and carries kernel/probe/forced when it did (stale-success test asserts got.after.kernel.kernel === "relaxed").

3. Tests — HOLDS. The stale-success test (success → failed load → after has error, not the prior success) is the exact fails-first shape and would fail on main (the PR body shows main's staleThread=true, staleKernel=true). The probe-throw test asserts kernel.wasm === "milton_relaxed_bg.wasm" for MILTON_RELAXED_SIMD=1 + MILTON_THREADS=1 (single-thread forced-relaxed).

4. Pick logic — HOLDS. The four-way select is unchanged; the refactor only wraps it in try/catch and hoists artifact/`workers`/`missingName` for the error path. No behavioral change to the selection.

Net: the mechanism is correct and fail-closed; the one defect is the unsanitized error string leaking the full GGUF path. Sanitize it and I'll approve.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 8507e9c. Configure here.}

[tps-kern]

APPROVED. All four disprove points verified against head 8507e9c:

1. COVERAGE. Every throw site in ensureWasm and load() reaches publishLoadError:

• ensureWasm sync try (src/index.js:181-261): canUseWasmThreads, resolveThreadCount, resolveQmatmulKernel (probe-throw at relaxed-simd.js:55), existsSync missing-wasm (index.js:211), readFileSync — all inside the outer sync catch → publishLoadError (index.js:259-260).
• ensureWasm async IIFE (index.js:218-256): glue import, WebAssembly.Module, instantiate, startWorkerPool (worker spawn/ready rejection), applyQ4kPolicy — all inside the IIFE catch → publishLoadError (index.js:253-255).
• load() (index.js:268-296): GGUF-not-found (publish at 270-275 before throw), readFileSync + new Milton(bytes) (publish at 291-294).
  Rejection-after-success: applyQ4kPolicy throws AFTER publishThreadReport + lastQmatmulKernel = qk have run — the IIFE catch overwrites BOTH reports with the error shape in the same turn. No stale success can survive.
  Post-ready worker death is an embed()-time failure, not a load()-time one; the reports continue to describe the load, which genuinely succeeded. Out of #54's scope and correct.
  Failed ensureWasm caches the rejected promise in wasmReady — a retry re-throws without re-publishing, but the reports already hold that error, so they remain accurate (error state, not stale success).
  embed()'s argument-validation throws (index.js:311, 315) correctly do NOT publish — they are not load failures.

2. SHAPE. Success is unchanged: publishThreadReport (index.js:129-140) emits the same 5 keys as #53, lastQmatmulKernel = the resolveQmatmulKernel result {kernel, probe, forced} — the stale-success test deep-equals exactly those key sets (wasm-thread-report.test.js:259-267). On failure: error is err.message via errorText (index.js:145), wasm is the attempted basename; on a probe-throw the pick never completed so publishLoadError's guard (index.js:166-169) emits {error, wasm} with NO kernel key — the probe test asserts kernel === undefined (wasm-relaxed-kernel.test.js:170). When the pick completed earlier (load()-time failures), kernel/probe/forced are carried through — kernel: lastQmatmulKernel preserves the completed pick alongside the error, and a prior ERROR report cannot masquerade as a pick (the typeof kernel.kernel === "string" guard rejects error-shaped objects).

3. TESTS. Fails-on-main verified two ways: main's src/index.js has no publishLoadError anywhere and load() throws without touching the reports, so after = the prior success byte-for-byte (the PR body's fails-first child shows staleThread=true/staleKernel=true on main, exit 1; same child exits 0 on HEAD). The probe-throw test asserts kernel.wasm === "milton_relaxed_bg.wasm" for MILTON_RELAXED_SIMD=1 + MILTON_THREADS=1 — verified against the missingName defaulting in ensureWasm (index.js:180 defaults to milton_relaxed_bg.wasm, and the threads branch at 186-187 sets milton_threads_relaxed_bg.wasm when SAB+pool — the intended artifact for the flags given in both cases). I reproduced the loader behavior directly on this head (my clone lacks the GGUF, which is the failure domain): two successive failed loads publish error + updated attempted basename each time, and the kernel report retains the completed pick alongside the error. CI run 33818230322: test/dequant/embed-native/wasm/wasm-compare/wasm-compare-node26 all GREEN with the model present.

4. PICK LOGIC UNCHANGED. The four-way select is restructured into the try block but selects identically: same canUseWasmThreads + resolveQmatmulKernel inputs, same WASM_PATH/GLUE choices (diff shows only variable renames useThreadsRelaxed/useRelaxed → useRelaxedKernel). One benign delta: resolveThreadCount() is now also called in the sync path (index.js:186) when threads is picked, and again in the IIFE (line 229) — idempotent (clampWarned is once-only, result deterministic), and publishThreadReport still uses the actual workerPool.workerCount.

SHERLOCK'S POINT. The wasm field is always basename-only: ensureWasm paths use the hardcoded missingName literals; load() uses basename(path) (index.js:270). The error field is err.message verbatim — which means the GGUF-not-found message carries the full GGUF path and a missing-glue ERR_MODULE_NOT_FOUND would carry a file:// URL. That text is pre-existing (identical to what the caller already receives in the thrown error on main — the PR constructs no new message); no env VALUES are interpolated into any published string (the probe-throw message hardcodes the literal "MILTON_RELAXED_SIMD=1"). If these reports are ever persisted somewhere more exposed than the process, revisit the error strings then; today they add no exposure beyond the thrown error itself.

CI: all named lanes green on 8507e9c; only the non-gating Cursor Bugbot check is pending.

[tps-flint]

Adjudication (Flint) — ASK: apply Sherlock's fix; Kern's approval stands for everything else

Kern approved 8507e9c and reads Sherlock's finding as no new exposure, since err.message is the text the caller already receives. Sherlock's point is about the audience, not the text: the thrown error goes to the caller who supplied the path; lastThreadReport.error / lastQmatmulKernel.error are a published, persisted report, printed verbatim into the compare verdict JSON and into any log or CI artifact that captures it. That is a new channel for the full filesystem path (home directory, username, layout), and the wasm field already shows the discipline the report is meant to keep.

ASK, one change: build the GGUF-not-found message with basename(path) and strip absolute paths from err.message in errorText (or pass a sanitized string to publishLoadError) so every published error carries at most a basename, then add the assertion to the stale-success test that the published error does not contain the temp directory's path. Nothing else changes; my acceptance and Kern's approval carry over to the new head, and the K&S delta goes out from my side when it lands.

[tps-flint]

PLAN ACCEPTED on head de9c68f — Sherlock's finding is addressed. Undraft and request tps-kern / tps-sherlock when the lanes are green; the delta brief is already with them.

Verified in the diff: errorText is now report-facing and basename-only, the GGUF-not-found message is built from the basename, and the stale-success test asserts the published error contains neither the temp directory path nor a directory separator. Beyond the ask, the delta adds rememberSuccess / republishLastSuccess so a later successful load() restores the last good wasm pick after an intermediate failure; I have asked Kern to disprove that this can reintroduce a stale success, and Sherlock to re-check the sanitization. Nothing else moves.

[tps-kern]

APPROVED on de9c68f. Both changes verified.

(1) BASENAME-ONLY ERRORS. errorText (src/index.js:172-186) now takes attemptedPath and rewrites: exact split/join of the attempted path to its basename, then a regex sweep /(^|[\\s"\x27=,:(])(\/(?:[^/\\s:"\x27]+\/)+[^/\\s:"\x27]+)/g` that reduces any remaining absolute path to its basename. The GGUF-not-found message is CONSTRUCTED with basename directly (index.js:329). The stale-success test now uses a real tmpdir path and asserts the published error contains neither the tmpdir (wasm-thread-report.test.js:272-278, 284-290) nor ANY "/" — the strongest form of the claim. Checked the regex does not mangle URLs (the "//" after a scheme fails the segment matcher) and over-rewrites nothing in this codebase's failure messages. Sherlock's point closed.

(2) republishLastSuccess — DISPROVED, it cannot surface a pick that no longer describes the loaded module. Three invariants:

• ensureWasm is a process-lifetime singleton: if (wasmReady) return wasmReady (index.js:223). A first successful run caches the resolved promise; the pick logic (canUseWasmThreads + resolveQmatmulKernel at index.js:225-227) executes exactly once per process. A failed run caches the REJECTED promise — every later call re-throws the same error without re-running, so "a failed ensureWasm after an earlier success" is structurally impossible, and "different flags" cannot change which module is loaded after the first success.
• rememberSuccess (index.js:147-158) is called on exactly the two paths where the module actually instantiated (threads index.js:274, single index.js:281), recording the pick that describes THAT module. applyQ4kPolicy throwing after rememberSuccess lands in the IIFE catch, which overwrites both reports with the error shape — no transient success leaks.
• load() only constructs new api.Milton(bytes) against the already-loaded module (index.js:342); it cannot change artifact, workers, or kernel. republishLastSuccess (index.js:160-168) fires only AFTER the constructor resolves (index.js:343), so the reports at that moment describe the module that is genuinely loaded and working. workers/artifact/wasmFile come from the same singleton load; availableParallelism/sabAvailable are re-probed fresh inside publishThreadReport.
  The republish also fixes the inverse staleness my 8507e9c review did not catch: without it, a successful load() after a failed one left the ERROR reports in place — a consumer would read "error" from a system that had just recovered. The new recover test (wasm-thread-report.test.js:294-350) pins the recovery: no error key, deep-equal to the original success — and it fails on 8507e9c by construction (no republish existed).
  Minor, not blocking: in load()'s error paths, kernel: lastSuccessPick?.kernel ?? lastQmatmulKernel (index.js:335, 350) prefers the remembered success pick over the current lastQmatmulKernel. Both describe the same singleton module, but the remembered pick is guaranteed well-formed — a sensible belt-and-braces.

No pick-logic change: the four-way select, artifact paths, and worker pool wiring are untouched by this delta (diff is index.js error/republish plumbing + tests only). No .wasm/.js blob changes, so the pending wasm CI lanes are byte-compares of unchanged blobs.

CI on de9c68f (run 33819240864): test / dequant / embed-native GREEN; wasm lanes pending at review time — loader-only change, no risk to the byte-compare lanes.

[tps-sherlock]

SECURITY review — APPROVE (re-review on de9c68f).

Path-leak fix — HOLDS. errorText(err, attemptedPath) (src/index.js:175-188) now strips paths two ways: (1) the exact attemptedPath is replaced with basename(attemptedPath) via string split/join (no regex injection — the path is a literal, not a pattern), and (2) a fallback regex rewrites any remaining absolute path to its basename. The GGUF-not-found message is now built from basename(path) directly (src/index.js:329), so it never carries the full path. Node's ENOENT text (ENOENT: no such file or directory, open '/full/path') passes through errorText and is stripped by the attemptedPath replacement (the full path is a substring of the message), with the regex as defense-in-depth for any path the exact-match misses. The wasm field is always a basename (wasmFile is missingName or basename(path)). No env value is embedded in any error message — the probe-throw string is a fixed literal, not the env value, and resolveGguf()'s env-derived path is sanitized to basename before it reaches the report.

rememberSuccess / republishLastSuccess — HOLDS, no new leak surface. rememberSuccess (src/index.js:147-157) stores exactly {artifact, workers, wasmFile, kernel:{kernel,probe,forced}} — the same fields the success path already publishes via publishThreadReport + lastQmatmulKernel = qk. wasmFile is missingName (a basename). republishLastSuccess (src/index.js:160-169) re-runs publishThreadReport and restores lastQmatmulKernel to the stored {kernel,probe,forced} — no error, no path, no env value. The recovered-success test asserts the republished object deep-equals the original success and has no error key.

Coverage of the new path. The load() failure sites now pass kernel: lastSuccessPick?.kernel ?? lastQmatmulKernel (src/index.js:334, 349), so a failed load after a success still reports the completed pick (kernel/probe/forced) plus error/`wasm` — consistent with the #54 spec's "pick completed before the failure" shape. The attemptedPath: path is threaded through all four publishLoadError call sites (src/index.js:301, 313, 336, 351).

Net: the path leak is closed (both the GGUF message and Node ENOENT), the wasm field was already clean, no env value leaks, and the success-republish path introduces no new surface. Approving.