test(web): fix flaky CI from api/client mock.module pollution#297
Merged
Conversation
…r hacks
Several web test files replaced the entire `../api/client` module with a
partial `mock.module` stub containing only the exports that file needed.
Bun's `mock.module` registry is process-global, so under CI's parallel test
execution one of those partial stubs could be the active version of
`../api/client` at the moment a bridge test's module graph first evaluated —
and the bridge imports `getActiveWorkspaceId`, which the partial stubs omit.
The result was the intermittent, CI-only failure:
SyntaxError: Export named 'getActiveWorkspaceId' not found in module
.../web/src/api/client.ts
Not a circular import and not a bun-version regression (CI's bun 1.3.14 runs
the suite green locally). The fix is to make every whole-module client mock
complete: spread the real module, then override only the handful of functions
the test stubs. A complete mock is inert when it leaks across files, so no
export can ever go missing regardless of load order.
That also removes the reason `a-t009-acceptance` and `b-workspace-section`
carried filename prefixes to win alphabetical load order — an ineffective hack
under concurrent execution anyway. Renamed both to drop the prefixes.
mgoldsborough
added a commit
that referenced
this pull request
May 27, 2026
…ke) (#298) * test(web): snapshot real api/client in preload instead of racy await import PR #297's whole-module api/client mocks spread `await import("../api/client")` to stay complete. That dynamic import reads the same process-global mock registry it's defending against, so under CI's file-scheduling it can resolve to an incomplete mock and propagate the gap — the flaky "Export named 'getActiveWorkspaceId' / 'ApiClientError' not found" link errors still fired on main (intermittently; passed on re-run). Capture the real module once in the test preload (web/test/setup.ts), before any test file can register a mock.module replacement, and freeze it as a plain object. Each whole-module mock spreads that snapshot instead. Because the snapshot is taken before any mock exists and is immune to later swaps, every registered mock is complete by construction — there is no longer a code path that registers an incomplete api/client. Web suite: 385 pass / 0 fail, 10x stable on bun 1.3.14. tsc + format clean. * test(web): migrate remaining api/client mocks to preload snapshot Spread realClient (the web/test/setup.ts snapshot) into the five test/* sites that still registered incomplete or registry-derived api/client mocks: streamingState, inlineError, useWorkspaceBriefing, useShell, and WorkspaceOverviewPage (dropping its racy `import * as actualClient`). All 10 mock.module("api/client") sites now register complete mocks, so no code path can leak an incomplete replacement into the process-global registry and crash bridge.ts/format-error.ts at link time. Completes the guarantee the snapshot was introduced to provide. 385/0, tsc clean. --------- Co-authored-by: Mathew Goldsborough <1759329+mgoldsborough@users.noreply.github.com>
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.
Problem
Web unit tests fail intermittently on CI (never reproducibly local) with:
Surfaced on #291 CI, but it is pre-existing and unrelated to that PR — the failing commit there was an empty commit, byte-identical to a commit that had passed CI three minutes earlier.
Root cause
Not a circular import, and not a bun-version regression (CI's
bun-version: latest= 1.3.14 runs the suite 30/30 green locally).Several web test files replace the entire
../api/clientmodule with a partialmock.modulestub containing only the exports that file needs:InstallConnectorDialog→{ installConnector }connector-sections→ 5 functionsResourceLinkView→{ readResource, ApiClientError }workspace-section→{ setActiveWorkspaceId, getActiveWorkspaceId, callTool }Bun's
mock.moduleregistry is process-global. Under CI's parallel execution, one partial stub can be the active version of../api/clientat the moment a bridge test's module graph first evaluatesbridge.ts/mcp-bridge-client.ts, which statically importgetActiveWorkspaceId— an export those partial stubs omit → the crash. Timing/concurrency dependent, hence CI-only and non-deterministic. The codebase already half-knew this (thea-/b-filename prefixes and the comment inclient.ts).Fix
Make every whole-module
../api/clientmock complete: spread the real module, then override only the functions the test stubs.A complete mock is inert when it leaks — no export can go missing regardless of load order. The overrides the components actually call still win; the existing passing tests prove the components never invoke the spread-in real functions.
Because completeness removes the load-order dependency, the
a-/b-filename-ordering hacks are no longer needed (and were ineffective under concurrent execution anyway). Renamed:a-t009-acceptance.test.ts→t009-acceptance.test.tsb-workspace-section.test.tsx→workspace-section.test.tsxVerification
format:check,lint, webtsc: cleanNote: the original flake does not reproduce locally even pre-fix (the race needs CI's concurrency), so the guarantee here is logical — a complete mock cannot drop an export — not a failing-test-now-passing demonstration.