Cut /screen's cold boot ~40%, and stop a signature failure reporting itself as a tab conflict - #92
Merged
Conversation
A bundle that fails ed25519 verification told the visitor to close a tab.
Two defects stacked, and each one alone was enough:
1. `worker.ts` posted only `error.message`. postMessage structured-clones its
payload, and structured clone drops prototypes — so a `SignatureError`
arrived on the main thread as a plain `Error` with `.name === "Error"`.
The app's registry classifies by `.name` (deliberately duck-typed "so it
survives the Worker boundary without instanceof coupling"), but nothing
ever carried `.name` across. Every type-based branch was dead in
production.
2. Even with the type intact, the registry had NO `SignatureError` branch. A
signature failure fell through to `internal.unknown` and rendered "Local
screening engine unavailable — Close another AML-Filter tab."
Fix: a shared `{ error, errorName }` envelope (errorEnvelope.ts) built in the
Worker's catch and rebuilt on the client, and a VERIFICATION_FAILURES set that
maps the whole fail-closed family — SignatureError, IntegrityError,
RollbackError, the zstd decode bounds, FetchLimitError — onto
`bundle.integrity_failed`. A discriminant, not serialize/rehydrate: the
classification contract is exactly `.name` + `.message`, and a name→constructor
registry would re-introduce the coupling the duck-typing exists to avoid, plus
let a Worker-supplied string choose which class the main thread instantiates.
The embedder Worker had the identical defect (`new Error(response.error)`) and
is now on the same envelope. The workstation DB worker already did this right
via encodeWorkerError/decodeWorkerError — the pattern existed, unadopted.
WHY THE TESTS MISSED IT, and the part actually fixed here: every existing test
hands the registry a freshly-built `new IntegrityError(...)` in-process.
`bundleSource.test.ts:86` says so — "An in-process BundleEngineClient mirroring
worker.ts". They assert the registry's behaviour, not the system's, in a
context where the type structurally cannot be lost. Four hand-rolled `{ok:false,
id, error}` literals in client/transfer/embedderClient tests now go through the
production serializer instead, so the envelope has one definition.
New tests are at the boundary, not beside it:
- client.boundary.test.ts drives EngineClient with replies that cross a real
structuredClone (what postMessage applies).
- bootErrorMessage.workerBoundary.test.ts asserts the rendered copy after that
trip, and pins that the engine-unavailable fallback stays REACHABLE.
- e2e-bundle/signature-failure-copy.spec.ts flips one base64 char of the
detached signature and drives a real dedicated Worker in Chromium.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186xrrT9mYfCZTVkk8HN12p
/screen downloaded four sanctions lists to read one. `ScreenPage` has passed `enabledLists: ["OFAC_SDN"]` since it shipped, and the runtime honoured it — but only in `#loadEnabledLists` / `#loadStreamingSources`, which run AFTER `openBundleSource` has already delta-synced the whole bundle. `syncIndex` walked `manifest.files` unconditionally. Measured against the live 2026-08-01 origin (1,296 chunks / 46,714,573 bytes): 527 chunks and 18.4 MB belonged to EU, UK and UN, which the screening page never reads. `syncIndex` gains `wantedPaths`: a path is in scope if it equals an entry, or starts with one ending in "/". Omitted means every file, so every other caller is byte-identical to before. The load-bearing detail is that all FIVE walks over `manifest.files` — the missing-chunk diff, the reused-chunk verification, the quota preflight, the offline reassembly check, and the cached-result count — now go through one `scopedFiles()` helper. Five walks each deciding membership for themselves would be five chances for "what I fetched" to drift from "what I verified", and that drift is how an unverified chunk gets promoted. Resolving a selection needs `catalog.json`, which lives inside the bundle, so `openBundleSource` is two-phase: sync the catalog alone (one chunk), resolve ids to `slug/` prefixes, sync those. Both phases are complete independent syncs — each re-fetches the no-store pointer, re-checks its ed25519 signature, re-checks the manifest content-address, and re-runs the anti-rollback gate. Splitting the FETCH did not split the VERIFY. Cost: one extra 256-byte pointer fetch. Enabling a list later is a top-up, not a re-sync: `#reload` sets the selection then drops the memoized source, the re-open carries the wider scope, and the content-addressed store reports everything already local as `chunksReused`. Also scoped the version poll to `[]` — it reads catalog.json and nothing else, yet was re-running a whole-bundle sync every time. Guards, each shown failing: - scope ignored in the diff -> 3 of 7 sync tests red - reused-chunk verification skipped for scoped files -> exactly the poison test red, which is how I know that test targets `verifyReusedChunks` and not some unrelated whole-store check - runtime stops passing the selection (the original bug) -> 2 runtime tests red - `wantedPathsFor` returns undefined (pre-fix) -> 4 of 5 bundleSource tests red The poison test also asserts the control: with EU out of scope the same poisoned chunk is legitimately NOT caught. Without that asymmetry the test could have been passing for the wrong reason. `bundleSource.test.ts`'s in-process client ignored `wantedPaths` entirely, which would have made every scoping test in the package vacuous; it now honours the scope exactly as worker.ts does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186xrrT9mYfCZTVkk8HN12p
The 23 MB model warmup was wrapped in `withTimeout(..., 120_000)`. 23 MB in 120 s is about 1.6 Mbps, so that constant was a bandwidth floor in disguise: every visitor slower than it was shown a Retry banner for a download that was working perfectly. Measured live on 2026-08-01, a sub-3 Mbps link failed the boot at 356 s with the model as the remaining cause. Same bug class as the sync ceiling fixed in 52c96fb. A bound on a download has to key on the GAP between progress ticks, never on their sum. `MODEL_LOAD_IDLE_TIMEOUT_MS = 90_000` is now the longest the warmup may stay SILENT. The window has to exceed the longest legitimate gap, not the longest legitimate duration, and there are only two gaps: - between network chunks: the transport meter emits per stream chunk, so even a 0.5 Mbps link ticks about every 4 s. Never close. - after the last byte, while ONNX Runtime compiles the graph. Nothing emits during that stretch, so it is what sets the floor. Measured ~6 s here; 90 s keeps an order of magnitude of headroom for a cold or low-core device. What a genuinely stalled visitor pays: 90 s to the error banner — better than the 120 s the wall clock charged them. Strictly better on both sides. The old wall clock is REMOVED, not kept alongside: `MODEL_LOAD_TIMEOUT_MS`, `parseTimeoutMs` and `modelLoadTimeoutMs` are gone, and the three Playwright configs move to `VITE_MODEL_LOAD_IDLE_TIMEOUT_MS`. They were setting a variable nothing read any more — a silent no-op, and two ceilings on one operation is how the next drift starts. The idle timer is fed from the RAW progress sink, deliberately. The banner throttle drops every tick that would not change the rendered value, so keying proof-of-life off it would manufacture silence on a moving download. `startIdleTimer` / `withIdleTimeout` are extracted into idleTimeout.ts and `EngineClient` is refactored onto them — it had this same shape open-coded in a private `Pending.rearm` closure. Two production callers, one definition. Guards, each shown failing: - tick() made a no-op (i.e. a wall clock again) -> the slow-but-moving case red - expiry suppressed (a "fix" that just deletes the guard) -> the stall case red - idle timer fed from the THROTTLED sink -> 2 runtime tests red, including the one built for exactly that mutant Kept the phrase "timed out" in the rejection, and pinned it with a test: the error registry classifies bundle.timeout on that text, and losing it would silently downgrade a stalled model to "Local screening engine unavailable — Close another AML-Filter tab." Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186xrrT9mYfCZTVkk8HN12p
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Several comments justified their bounds with "~1,296 chunks / ~48 MB". That is still the full four-list bundle, but it is no longer what /screen fetches — the default OFAC-only selection is ~769 chunks / ~28 MB. Both numbers now appear where the distinction matters, so a future reader sizing a timeout against these figures picks the right one. BOOT_TIMEOUT_MS keeps sizing against the four-list worst case, which is what a backstop is for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186xrrT9mYfCZTVkk8HN12p
The real-browser e2e caught what the unit boundary tests could not. With the Worker envelope and the registry both fixed, a tampered signature STILL rendered "Local screening engine unavailable — Close another AML-Filter tab", with the technical detail underneath reading "signature verification failed". `ScreenPage`'s error phase held only `bootErrorMessage(error)` — a STRING — and the banner re-derived its copy by calling `userFacingBootError(phase.message)` on that string. A string has no `.name`, so every `.name`-matched branch in the registry was unreachable and EVERY boot failure rendered the internal.unknown fallback regardless of cause. It is the same defect as the Worker boundary, one layer further in: React state is another place a typed error gets flattened. The fix is the same shape too — classify while the error is still an error, and carry the result. This is exactly why the e2e was written to drive a real dedicated Worker rather than to assert the registry in-process. Two unit-level fixes were both correct and the product was still broken; only the rendered page showed it. Evidence: - e2e-bundle/signature-failure-copy.spec.ts, red before this commit with "Local screening engine unavailableClose another AML-Filter tab, then retry.", green after. - a unit regression pinning the constraint directly: classifyBundleError on the error is "integrity_failed", on its flattened string it is "unknown". Also finishes the scoped-sync e2e (e2e-bundle/scoped-sync-topup.spec.ts): the /screen cold boot fetches the catalog + OFAC only, and enabling EU later re-fetches not one chunk that was already local (asserted as an empty intersection, so it cannot pass by coincidence). The workstation behind /settings still defaults to every catalog list when no selection is stored, and that is deliberately left alone — narrowing what a KYC workspace screens would silently drop matches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186xrrT9mYfCZTVkk8HN12p
CI caught a regression from the previous commit. Classifying the boot error early fixed the banner TITLE, but the detail line then rendered the raw cause instead of the app's one user-facing wrapper, so screen-cold-blocked.spec.ts:213 went red: Expected pattern: /could not load the screening bundle/i Received: "Local screening engine unavailableClose another AML-Filter tab, then retry.Technical details`local_files_only=true` ... Retry" Both are contracts and both now hold: the title/recovery come from the classified error object, the technical detail keeps the "Could not load the screening bundle: <cause>" framing that bootErrorMessage.ts names as the single wrapper for a load failure. Also retunes the bundle e2e lane for the scoped sync. boot-ceiling.spec.ts proves the whole-boot ceiling binds a sync that is moving but never arriving, and it does that by feeding one chunk per tick to keep the 30 s no-progress watchdog re-armed. A scoped /screen boot fetches 4 chunks, not the fixture's 13, so there were no longer enough ticks to reach a 200 s ceiling and the watchdog fired instead — the test would have silently started measuring the wrong bound. The lane's ceiling drops to 75 s (3 ticks x 25 s) with the fourth chunk deliberately never released, so the sync cannot complete out from under the ceiling, and a new assertion pins that. The model idle window drops to 45 s so the backstop still sits above every tighter bound. Corrects an unevidenced number I wrote in the previous commit: the ONNX compile gap is 359-419 ms measured over four cold boots, not the "~6 s" I claimed. It is a floor, not a worst case — localhost, warm cache — and the comments now say so, and say why the window is set far above it anyway. Local evidence: e2e-bundle 11/11, e2e-c1 4/4, including the spec CI failed on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186xrrT9mYfCZTVkk8HN12p
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three defects found against the live production bundle (2026-08-01: 1,296 chunks / 46,714,573 bytes). Each commit stands alone.
The claims this PR touches
1.
/screendownloaded four watchlists to read oneScreenPagehas passedenabledLists: ["OFAC_SDN"]since it shipped, and the runtime honoured it — but only afteropenBundleSourcehad already synced the whole bundle. 527 chunks / 18.4 MB went to EU, UK and UN, which the page never reads.syncIndexgainswantedPaths. The load-bearing detail: all five walks overmanifest.filesnow go through onescopedFiles()helper, so "what I fetched" cannot drift from "what I verified".Resolving a selection needs
catalog.json, which lives inside the bundle, soopenBundleSourceis two-phase: sync the catalog alone (one chunk), resolve ids toslug/prefixes, sync those. Both phases are complete, independent, fail-closed syncs — each re-fetches the no-store pointer, re-checks its ed25519 signature, re-checks the manifest content-address, re-runs the anti-rollback gate. Splitting the fetch did not split the verify.Enabling a list later is a top-up, not a re-sync. Also scoped the version poll, which was re-running a whole-bundle sync to read one 700-byte file.
2. Every type-based branch in the error registry was dead in production
Two defects stacked:
worker.tsposted onlyerror.message. Structured clone drops prototypes, so aSignatureErrorarrived as a plainError.SignatureErrorbranch at all — it fell through tointernal.unknownand rendered "Local screening engine unavailable — Close another AML-Filter tab."Fixed with a shared
{ error, errorName }envelope and aVERIFICATION_FAILURESset covering the whole fail-closed family. The embedder Worker had the identical defect. (The workstation DB worker already did this correctly — the pattern existed, unadopted.)Why the tests missed it: they hand the registry a fresh
new IntegrityError(...)in-process.bundleSource.test.ts:86says so in its own comment. They asserted the registry's behaviour, not the system's, in a context where the type structurally could not be lost. The new tests are at the boundary — a realstructuredClone, plus a Chromium spec that flips one base64 character of the detached signature and drives a real dedicated Worker.3. The model load was bounded by a wall clock
23 MB in 120 s is ~1.6 Mbps, so
MODEL_LOAD_TIMEOUT_MSwas a bandwidth floor in disguise. Replaced withMODEL_LOAD_IDLE_TIMEOUT_MS = 90_000— the longest the warmup may stay silent. A stalled visitor now waits 90 s, not 120 s: strictly better on both sides.The old wall clock is removed, not kept alongside — three Playwright configs were setting a variable nothing read.
startIdleTimer/withIdleTimeoutare extracted andEngineClientrefactored onto them, so the primitive has two production callers and one definition.Guards, each shown failing
wantedPathsForreturns undefined (pre-fix)new Error(msg).nametick()made a no-op (wall clock again)The poison test also asserts the control: with EU out of scope the same poisoned chunk is legitimately not caught. Without that asymmetry it could have been passing for the wrong reason.
bundleSource.test.ts's in-process client ignoredwantedPaths, which would have made every scoping test vacuous; it now honours the scope exactly asworker.tsdoes. Four hand-rolled error envelopes in other tests now go through the production serializer.🤖 Generated with Claude Code
https://claude.ai/code/session_0186xrrT9mYfCZTVkk8HN12p
Measured: cold boot against the real production bundle
Mirrored the live origin (
aml-filter.com/bundle/origin, version 2026-08-01, 1,296 chunks / 46,714,573 B, verified through the publisher's own mirror path), served it frompublic/bundle/origin, and drove/screenin Chromium to "search box enabled". Two git worktrees pinned at the pre-fix and post-fix SHAs, identical script, fresh context per run.06989c4592431fBundle bytes −18,471,248 (−39.6%). Total cold transfer −21%. The 769 is the right 769:
catalog.json(1) +ofac/*(768). EU (272), UK (207) and UN (48) are exactly what stops being fetched. The two-phase sync's cost shows up as expected and is negligible: 2 pointer fetches (512 B vs 256 B) and a second manifest request that returns 0 body bytes.Time-to-ready is not a headline number here — this is localhost, unthrottled. The byte count is the honest measure; on a 3 Mbps link those 18.5 MB are about 50 seconds.
A methodology trap worth recording
The first BEFORE run reported 769 chunks — a plausible-looking, completely wrong result.
frontend/app/node_modules/@amlfilter/browseris a relative symlink, so symlinkingnode_modulesinto a worktree resolves back through the main checkout and both arms silently compiled the same engine. Fixed by mirroringnode_modulesper arm with the workspace scopes retargeted, verified withgrep -con the built engine (before: 0 hits for the scope code, after: 11) and distinct dist asset hashes.Job 3: the bound, and what it costs
MODEL_LOAD_IDLE_TIMEOUT_MS = 90_000— the longest the warmup may stay silent.Real stalls stay bounded:
e2e-c1/screen-cold-blocked.spec.tsblocks every weight source, so nothing ever reports progress, the window expires once, and the banner + Retry appear.Local e2e evidence
e2e-bundlee2e-c1e2e-kycA third instance of the same defect, found by the e2e
With the Worker envelope and the registry both fixed, the tampered-signature spec was still red:
ScreenPage's error phase held only a string, and the banner re-classified that string at render. A string has no.name, so every branch was unreachable — the same flattening as the Worker boundary, one layer further in, in React state. Two unit-level fixes were both correct and the product was still broken. Only the rendered page showed it.