fix: root-cause the CI flakes instead of rerunning them - #20
Conversation
An in-place write is truncate-then-append, which a loaded watcher observes as two change events and compiles twice; the duplicate attempt supersedes the generation that ordinal-pinned assertions expect, which is exactly how CI committed generation-3 where the equivalent-revision test pinned generation-2. The fixture's three source mutations now go through one same-directory rename so a change is one event and one compile on any machine.
The example suites self-repair their pinned manifests and README version table against the installed lockfile; running them locally surfaced the drift left by the merged dependency bumps (@rstest/core 0.11.10, Rsbuild 2.2.1, react-server-dom-rspack 0.1.0).
pnpm's install-time verification and the example self-repair moved the workspace manifests onto the already-merged toolchain line (@rstest/* 0.11.10, Rsbuild 2.2.1, Rspack 2.2.1, react-server-dom-rspack 0.1.0); every suite in this branch ran against these installed versions.
The parallelism audit inverted the oversubscription hypothesis: rstest already resolves to one worker on a two-core CI runner, so the flakes come from fixed deadlines tuned on many-core machines applied to tests that are inherently multi-process. The sharpest edge was the MCP session layer stamping a five-second timeout on every request - an rsbuild compile or Chrome startup saturates both CI cores for longer than that, tripping -32001 - so the default rises to thirty seconds, still half the MCP SDK's own default. Fixed browser and per-test budgets across the heavy suites now scale four-fold under CI, which costs nothing on green runs since polling assertions return on success, and plugin-bundle.test.ts moves to the serialized integration pool per that list's own admission rule (it runs real builds and spawns node children).
The unit harness had no @inspector aliases, and rstest silently downgrades an unresolvable specifier to a runtime CJS require - so every unit-pool bundle of the runtime playground carried a require of '@inspector/core/json/xMcpHeader.js' that threw on each execution. React's Suspense boundary absorbed the throw on green runs and let it escape as an unhandled rejection under CI timing, which is the 'Cannot find module' flake. Instead of copying alias blocks into a fourth config, the vendored inspector core is now a private workspace package whose exports map serves the .js specifiers from the .ts sources, so every resolver - rsbuild, every rstest config, the browser test harnesses, and plain Node - finds it through node_modules; all five alias declarations are deleted. The runtime contract-compile test also stops racing the worker's post-file unhandled-rejection check: its stub answers every route with the status wrapper, so the deliberately rejecting bootstrap fan-out is now awaited and asserted instead of left dangling.
An aborted test run left its copied fixture workspace in the repo root and the previous commit swept it in; the fixture prefix is now ignored so test debris cannot enter history again.
🦋 Changeset detectedLatest commit: 7c699e8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f312ae8c03
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Keeps both sides: the CI time-scale multiplier on runtime waits and main's shared workbenchUrl and packed-release harness helpers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EMWczsXAkj7fC5ssSxGK43
commit: |
…closure The vendored inspector core now links as a workspace package through a package.json inside the snapshot, and the sync verifier counted that workspace-owned file as vendored source, failing every CI run with a closure mismatch. It joins package-manager state outside the closure walk, survives a resync, and the fixture test covers both. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EMWczsXAkj7fC5ssSxGK43
The runtime-playground HMR e2e test still stamped a fixed 30s Playwright budget tuned on many-core machines onto waits that sit behind rsbuild compiles and Chrome sharing a two-core runner - the same shape that tripped the capture script on the Node 22.19 Verify job. Both surfaces now read the shared timeScale helper instead of an inline CI multiplier, so the two-core rationale lives in one documented place. Scaling costs nothing on green runs since every wait returns on success.
… and budget the retention test for CI
The eviction-window sibling settles twelve real sessions through the store's fsync-per-append durability path plus replay, export, and promotion, so it starves on the same two-core default budget that already killed the subscribed-retention test. Give it the same timeScale treatment the script-playground suite carries.
The equivalent-prepared-revision test pinned generation-2 by ordinal, which assumes one source change burns exactly one generation. The dev runtime is a multi-compiler, and watch delivery can skew across the rsc and widget children under load: the last-done child fires the compile observer, so a slow sibling watcher yields an intermediate mixed cohort first, and its supersession burns an ordinal - the micro-eval job saw generation-3 commit. Staging temp files outside the project removed one duplicate-compile source, but the skew is inherent to the platform. The invariant this test owns is that an equivalent revision does not supersede the in-flight compile - the session leaves the first generation and settles active with clean diagnostics - so assert that relative shape the way the superseding-revision sibling already does.
…default The open form still seeded its timeout field with 5000 and always passed that explicit value, so the raised thirty-second dev-server default never reached desktop sessions. The field now starts empty and the form omits the timeout unless the user supplies one, letting the server default apply and still validating any explicit entry.
Summary
Root-cause fixes for all four recurring CI flake mechanisms, each investigated to a verified mechanism before touching code:
writeFileis truncate-then-append, which a loaded watcher observes as two change events → two compiles; the duplicate superseded the generation the ordinal-pinned assertions expect. Fixture source mutations now go through a same-directory atomic rename — one event, one compile.Cannot find module '@inspector/core/json/xMcpHeader.js': the unit harness had no@inspectoraliases, and rstest silently downgrades unresolvable specifiers into runtime CJS requires — every unit bundle of the runtime playground carried a require that threw on each execution, absorbed by React Suspense on green runs and escaping as an unhandled rejection under CI timing. Rather than copying alias blocks into a fourth config, the vendored inspector core is now a private workspace package whose exports map serves the.jsspecifiers from the.tssources — all five alias declarations across build/test configs are deleted and every resolver (rsbuild, rstest, browser harnesses, plain Node) finds it through node_modules.Runtime route returned an invalid surfaces wrapper.: the contract-compile test deliberately creates a rejectingbootstrap()fan-out and left it unhandled; whether the rejection landed before rstest's post-file unhandled-error check was a tick race (both sides demonstrated in the harness). The rejection is now awaited and asserted.-32001timeouts / budget blowouts: the parallelism audit disproved runner oversubscription (rstest already resolves to 1 worker on 2 cores). The causes were fixed deadlines tuned on many-core machines — foremost a 5s default stamped on every workbench MCP session request while a compile can pin both CI cores longer than that. The default rises to 30s (half the MCP SDK default; explicit short timeouts in timeout-path tests unaffected), heavy-suite budgets scale 4× underCI(free on green runs), andplugin-bundle.test.tsmoves to the serialized integration pool per that list's own admission rule.Also: workspace pins synced with the already-merged toolchain line (separate commit), and stray test-fixture debris removed + gitignored.
Test plan
pnpm test:unit— full suite green with the package-based resolution (zero runtime@inspectorrequires)mcp-session-service,script-playground-service,mcp-json-input(browser harness), workbench production build — all passpnpm typecheck/pnpm rslint— clean