Repository navigation
dasweb-verify: emscripten's run-dependency watchdog is not a page error - #4097
Conversation
…or - a slow artifact download printed it as console.error and failed rows whose programs ran Both pages collect their page errors through one runner.mjs helper, collectPageErrors, which asks one pure predicate, isConsolePageError: console errors only, minus failed requests, program stderr echoes, and the watchdog's three lines. The browser leg's node:test suite now also runs per PR (dasweb-verify-browser.yml), path-filtered to the leg and the playground manifest, so a sample added without an expectations row fails its PR instead of the nightly. browser/REVIEW.md: the probe.mjs rule is a ban, and the placement lines name console-message classification and what runner.test.mjs may not assert. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It changes the failure-detection/error-suppression logic that gates the nightly production verifier, where an over-broad filter could silently turn the nightly green, warranting human confirmation of the watchdog matchers' scope.
Review effort: Balanced
Findings: None
What changed in this PR
This PR fixes a false-positive in the nightly playground verifier (dasweb-verify): Emscripten's run-dependency watchdog (still waiting on run dependencies:, printed to console.error when an artifact load exceeds ~10 s) was being counted as a page error, turning the nightly red for programs that actually rendered. It centralizes console-message classification into protocol.mjs, deduplicates the two page-listener setups into one collectPageErrors helper, and adds a per-PR CI lane so the browser leg's node:test suite runs before nightly, not only in it.
Changes:
- Add
isRunDependencyWait,isFailedRequest, andisConsolePageErrortoprotocol.mjs, dropping the watchdog lines (plus already-dropped failed requests and program-stderr echoes) from the page-error channel. - Replace the two duplicated
pageerror/consolelistener blocks inrunner.mjswith a single exportedcollectPageErrorshelper, and cover it plus the new classifiers with tests. - Add the
dasweb-verify-browser.ymlper-PR lane and document it inpreflight.md; updatebrowser/REVIEW.mdplacement/ban wording.
| File | Description |
|---|---|
utils/internal/dasweb-verify/browser/protocol.mjs |
Adds pure console-message classifiers, including the emscripten watchdog filter that fixes the false FAIL. |
utils/internal/dasweb-verify/browser/runner.mjs |
Extracts the two duplicated page-error listener blocks into one collectPageErrors helper. |
utils/internal/dasweb-verify/browser/protocol.test.mjs |
Tests the new watchdog / failed-request / console-error classification. |
utils/internal/dasweb-verify/browser/runner.test.mjs |
Tests collectPageErrors wiring and the --console echo via a stubbed page. |
.github/workflows/dasweb-verify-browser.yml |
New per-PR lane running node --test for the browser leg. |
skills/internal/preflight.md |
Documents the new lane, its triggers, and local mirror command. |
utils/internal/dasweb-verify/browser/REVIEW.md |
Turns the probe.mjs rule into a ban and updates placement lines for console-message classification. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Why. The nightly playground verifier failed wasm rows whose programs ran. When an artifact download takes more than 10 s, Emscripten's run-dependency watchdog prints
still waiting on run dependencies:toconsole.error, and the verifier counted every console error as a page error. This turned the nightly red on 9/17 (5 rows) and 9/20 (1 row); the failure screenshots show the programs rendering.What changes.
isConsolePageErrorinprotocol.mjsdecides which console messages are page errors; it drops the watchdog's three lines, as well as failed requests and program stderr echoes, which were already dropped.runner.mjshelper,collectPageErrors, instead of two copies of the same listeners.dasweb-verify-browser.yml, runs the browser leg'snode:testsuite; until now only the nightly ran it.browser/REVIEW.md: theprobe.mjsrule is now a ban, and the placement lines name console-message classification and whatrunner.test.mjsmay not assert.Observable behavior.
page error: still waiting on run dependencies:-> PASS, or FAILrafs 0/Nif the load never finishes.data.jsonwithout anexpectations.jsonrow: red nightly -> red PR.Where to look.
protocol.mjsisConsolePageError, andrunner.mjscollectPageErrors.Validation, claims, ledger
Validation
npm ci && node --testinutils/internal/dasweb-verify/browser, on Linux x86_64 (WSL Ubuntu 24.04, node v22.23.2, agit archiveof the head commit): 35 pass, 0 fail. The same on Windows 11: 35 pass, 0 fail. Negative controls: removing the watchdog filter, the type filter, the failed-request anchor, or the--consoleecho each fails its named test.node runner.mjs --mode wasm -f "River Run"- PASS (rafs 803, draws 27720).Claims - stated, not tested
Playground.openthat callscollectPageErrorshas no unit test:Playgroundis not exported. TheArtifacts.opencall is tested, and so is the helper itself. If thePlaygroundcall were dropped, a playground page error would stop failing interpreter rows; the nightly is the only check that would notice.Not done
utils/internal/dasweb-verify/REVIEW.md,utils/REVIEW.mdand.github/workflows/REVIEW.mdfound rule defects whose fixes delete, split, move, or re-scope rules. They wait for the owner's ruling, and this PR does not touch those three files.