Repository navigation
Fix the flaky Composio boot test: record only the vendor's own requests - #1035
Merged
Merged
Conversation
…test The "key it was given is the key the vendor sends" test replaced the process-wide fetch and process.on for the second and a half the raw client spends retrying a refused connection. bun test runs every file in one process, and the browser extension's tests leave timers running (its activity heartbeat, connection check and streamer reconnect) that keep calling fetch long after their files finish. When one of them fell inside that window it was recorded as a keyless attempt to 127.0.0.1, and the test failed with "Received: null" while Composio's client was behaving. The recorders now apply only inside an AsyncLocalStorage scope around the test body; calls from anywhere else pass through to the real globals. The test also waits for the refusal with a plain await instead of expect().rejects, because Bun waits on that by running the event loop inside the call, which runs other files' timers nested in the scope.
davidmckayv
requested review from
MikeRyanDev,
dustywusty,
guidovizoso,
mxmzb,
samjulien and
tylerslaton
as code owners
November 5, 2026 22:46
davidmckayv
added a commit
that referenced
this pull request
Nov 5, 2026
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
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.
The flake
server/tests/composio-adapter.test.ts, "what constructing the vendor is allowed to do on boot > the key it was given is the key the vendor sends, to Composio's own host", failed withExpected: "never-dialled"/Received: nullon PR #1011's first CI attempt and on main at af42cc6, and passed on rerun both times. It takes about 1.3 seconds.Cause
The test replaces
globalThis.fetchandprocess.onwith recorders, then lets Composio's raw client trytoolkits.listagainst a transport that refuses. The client retries a refused connection twice with backoff, so the recorders stay installed for about 1.3 seconds.bun testruns every test file in one process, and the browser extension's tests (which run earlier in CI's file order) leave timers running after their files finish: the Recorder's activity heartbeat, its connection check, and the offscreen streamer's reconnect. Those keep callingfetchon127.0.0.1for minutes. When one fired inside the window, the recorder logged it as the vendor's attempt: nox-api-key, sonull. Composio's client was sending its key every time.A probe that held the recorder open for 30 seconds at the same point in CI's file order caught three of these calls, with stacks through
extension/src/background/activity.ts,extension/src/background/recording.ts(checkConnection) andextension/src/offscreen/streamer.ts.There was a second way in. The test waited with
await expect(broker.listApps()).rejects.toThrow(). In Bun 1.4.3,expect(promise).rejectswaits by running the event loop inside the call, so another file's timer that falls due during that wait runs nested inside the test's own stack. Scoping the recorders alone still failed 2 of 40 runs, with the extension streamer's call recorded and its stack running through the test body.Fix (test harness only, no product change)
AsyncLocalStoragescope around the test body. The client the body builds, its retry timers and the SDK's deferred telemetry all inherit that scope. Calls from anywhere else pass through to the realfetchandprocess.on, as if the test were not running.awaitand the test asserts it afterwards. A comment on the helper says whyexpect(...).rejectsmust not be used inside it.Nothing the describe guards was weakened. Each product mutation was run against the old and new test in a Linux bun 1.4.3 container, and both fail the same way:
createComposioClientallowTracking: falsedeleteddisableVersionCheck: truedeletedapiKeyreplaced by another keybaseURLaddedEvidence
Reproduction: the 66 extension test files in CI's order, then 40 copies of
composio-adapter.test.ts, all in one bun 1.4.3 process the waybun testruns them in CI. Each copy gives the race one chance.oven/bun:1.4.3)expect().rejects(macOS)The Linux container also has about 47 extension test failures caused by the container environment (IndexedDB, git work tree checks). They are the same before and after, and they all come from extension files that run before any copy of this test.
bun run lint,bun run format:check,bun run typecheck,bun run typecheck:testsandbun test server/tests/composio-adapter.test.tspass.Not changed here
The extension tests still leave those timers running after their files finish. With this change they can no longer fail this test.