Conversation
Adds glob-based roots, multiple preloads, per-root tsconfig/timeout/retries and globalSetup to the Bun-native test manifest and runner, and closes three Vitest-compatibility gaps (it/test/describe.runIf, accessor-safe automock, restoreAllMocks resetting spy state). Unblocks the settings and ide-integration workspaces under Bun.
@lydell/node-pty never delivers onData/onExit under Bun on POSIX, so the interactive test harness selects Bun.spawn's terminal backend there. Also preserves the default export when the script harness patches node:fs.
…ation roots Declares glob-based Bun-native roots for settings, ide-integration, vscode-ide-companion, policy, telemetry, test-utils, a2a-server, the script harness, evals and integration-tests, and points those workspaces' test scripts at the Bun-native runner. Adds a Vitest-compatible JSON report so the evals aggregation keeps working, and replaces the Vitest-internal @fast-check/vitest dependency in the token-tracking property suite with plain fast-check.
The 3-argument itProp form was dropped by @fast-check/vitest v0.2, so the arbitraries were swallowed as an options object and every predicate ran once with the Vitest context instead of generated values. Driving them through plain fast-check exposed three assertions that never held: total deliberately excludes cache tokens, formatSessionTokenUsage groups digits via toLocaleString, and a freshly recorded entry is inside the 60-second window rather than outside it.
…suite Root scripts, scripts/test.ts and the CI workflows now invoke the Bun-native runner for the script harness, evals and integration tests. Roots that call a real provider are marked credentialed so an unfiltered run stays offline and the parity job can execute the complete suite without burning quota.
packages/cli compiles pty-backend.ts through a tsconfig that does not list Bun's types, so process.versions.bun and the Bun global are read through explicit narrow shapes instead.
Deletes the ten now-unused vitest.config.ts files and re-expresses the invariants that guarded them against the manifest: the evals report path, the OCR workflow's discovery, the scripts shard's two invocations and the settings boundary's alias check. Also fixes two bugs the migration exposed — the eval log directory was cwd-relative (issue 2605 again under the new cwd) and the JUnit converter attributed a nested suite's cases to its parent, doubling every count the evals aggregation reads.
…-timer waitFor The compat shim had grown try/catch fallbacks that turned an unresolvable vi.mock target into a silently registered empty module, defeating the behavioural test that guards that fail-fast contract. Reverted. waitFor's fake-timer loop advanced the clock but never yielded before retrying, so a promise chain resumed by a timer could not make progress between attempts; the openai-responses abort suite deadlocked on main and failed here. It now yields to the microtask queue between attempts.
…ing it stop() resolving means the server relinquished the socket, but releasing the listening descriptor is the runtime's job and is not necessarily complete on the next turn. The assertion still requires a refused connection; it just gives the runtime a bounded budget to get there.
…path waitFor's fake-timer loop now attempts the callback at t=0 like the real-timer path and no longer advances the clock twice between retries after an async rejection. The process.exit shim interpolates the raw argument so a no-arg call reports "undefined" exactly as Vitest does. Adds the missing resource-safety test for a global setup that throws after an earlier one succeeded.
…dget The subprocess-integration blocks move to their own file rather than raising the max-lines threshold.
Contributor
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis pull request migrates test orchestration toward Bun-native execution. It adds manifest-driven discovery, asynchronous lifecycle handling, JUnit report conversion, PTY compatibility, workspace script updates, CI changes, and runner-portable test adjustments. ChangesBun test migration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
the
maintainer:e2e:ok
…ntract A review flagged the missing t=0 attempt by analogy with the real-timer path, but stub-helpers.bun.test.ts asserts the opposite for Bun: nothing else moves the fake clock, so advancing before the first callback is what lets a callback waiting on a scheduled effect ever succeed. The double-advance fix after an async rejection stands.
# Conflicts: # .github/workflows/ci.yml # scripts/bun-test-manifest.ts # scripts/tests/bun-test-manifest.bun.test.ts # scripts/tests/test-setup.ts # scripts/tests/vitest.config.ts # tsconfig.scripts.json
Contributor
WalkthroughBefore this PR, the repository had already moved install and some orchestration onto Bun, but many workspaces and script-harness tests still relied on Vitest or were only partially covered by the native parity job. That left multiple test runners, fallback scripts, and CI paths in place, so contributors could still run or validate suites through older npm/Vitest flows and some test files could be omitted without immediate failure. After this PR, the remaining workspaces are migrated to Bun-native test execution, the root orchestrator and CI use Bun-native as the primary path, and the manifest/tests/configs are updated so the complete suite is exercised and silently dropped test files are no longer possible. Release NotesNew Features
Bug Fixes
Tests
Documentation
Refactor
Chore
Changes
Sequence DiagramsequenceDiagram
participant CI as GitHub Actions
participant Selector as shard_selector
participant Shard as test_shard
participant Orchestrator as scripts/test.ts
participant Manifest as bun-test-manifest.ts
participant Runner as run_bun_tests.ts
participant Bun as Bun native test
CI->>Selector: changed files, event type
Selector->>Selector: resolve affected shards from import graph
Selector-->>CI: matrix of shards, has_tests flag
CI->>Shard: run for each selected shard
Shard->>Shard: bun install, restore bun.lock
Shard->>Shard: npm run build
Shard->>Orchestrator: bun scripts/test.ts --shard <shard>
Orchestrator->>Manifest: resolveBunNativeTestFiles(repoRoot, workspace?)
Manifest-->>Orchestrator: list of BunTestFile entries with cwd/preload
loop for each manifest file
Orchestrator->>Runner: runSingleTestFile(entry, baseArgs, deps)
Runner->>Bun: Bun.spawnSync([bun, test, --preload?, file])
Bun-->>Runner: exitCode, stdout, stderr
Runner->>Runner: isChildSuccess(child)
end
Runner-->>Orchestrator: passed/failed counts
Orchestrator-->>Shard: exit code
Shard->>Shard: write junit.xml, upload coverage if applicable
Shard-->>CI: job result
Magnitude🎯 5 (XXL) Related
Pre-merge Checks
Walkthrough generated by LLxprt PR Review. Planner issue: #2256 |
Contributor
OpenCodeReview — automatic reviews suspendedAutomatic OCR reviews are suspended for this PR after 2 of 2 automatic reviews. To get more reviews you can:
|
…ndbox The e2e workflow passes --exclude through to the integration-test command to skip individual specs per sandbox mode; the Bun-native runner rejected it as an unknown option. Both the '--exclude glob' and '--exclude=glob' spellings are accepted, matching what Vitest took.
Both vi.mock targets name modules that no longer exist, so the mocks were inert; Vitest tolerated the unresolvable ids, Bun does not. Neither test used what the mock supplied. The PNG fixture resolved its asset from the process cwd, which is integration-tests/ under the runner rather than the repo root.
The e2e workflow's second invocation narrows the run to one spec and a set of test names, both Vitest CLI features the Bun-native runner rejected. Bare path arguments now filter the resolved files by substring, as Vitest did, and the name pattern is forwarded to Bun as --test-name-pattern.
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-24.x-ubuntu-latest' artifact from the main CI run. |
…honestly The parity job now covers the script harness, several of whose tests execute the built CLI; they previously ran only in the built scripts shard, so the job needs the same build step. The LSP case asserts what import.meta.resolve reports when the package is installed, but only checked that the API exists. An install that never links the workspace has the API and not the package, so the precondition now covers both halves.
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Merging main pushed bun-test-manifest.ts past the 800-line budget. The providers entry moves out following the pattern main established for tools, mcp and storage, rather than raising the threshold. File selection is unchanged: 971 resolved files, no duplicates.
This was referenced Aug 4, 2026
acoliver
added a commit
that referenced
this pull request
Aug 6, 2026
The second branch dispatch (run 31039119968) turned E2E Windows, the CLI bundle and every other leg green, and narrowed the rest to two things. windows_ci: only getPty remained. The previous attempt was aimed at the wrong runner. packages/core runs exclusively under Bun, and getPty short-circuits to the Bun.Terminal adapter on Bun POSIX, so these three cases only ever EXECUTE on Windows — the one platform where substituting the backend in the module registry silently fails, because Bun's runner cannot reset the registry and evaluates a module-mock factory once, eagerly. That is why they passed everywhere a developer would look and failed only in CI. getPty now takes injectable backend loaders, so the selection logic is driven the same way on every runtime and the cases run locally under Vitest too. macos_ci: with the canary race fixed, the job reached a step that failure had been masking. "Run scripts-assignment Bun manifest" invokes a workspace that #3014 deleted from the Bun test manifest while leaving the step behind, so it could never pass. Both of its test files are already covered by the scripts-tests glob root and ran green in the same job's harness step, so the dead step goes rather than the workspace coming back. From review: - scripts/bun-build.config.ts rendered a diagnostic's source position only for instanceof Error values, but Bun attaches BuildMessage/ResolveMessage objects, which are not Errors — so the file/line/column was always thrown away. Confirmed by probing Bun 1.3.14 directly. - The CLI bundle test's artifact check was an opaque existsSync assertion even though the build can exit 0 having built nothing, and its process renderer printed the literal "null" for the streams spawnSync nulls out on a spawn failure — both precisely when the diagnostic matters most. - The nested-vitest launcher accepted node.cmd, which spawnSync cannot execute without shell:true; on a Windows box where a .cmd shim shadows node.exe the child would have failed to launch at all. - The credential-proxy shutdown comment claimed a Windows named-pipe half-close that libuv does not implement. What actually fixes it is ordering: server.close() waits for the connection count to reach zero, so stop() can no longer return before the peer observes the close. The socket is also paused before ending, since end() leaves the read side open and an in-flight frame would otherwise still mutate the token store mid-shutdown, and endAndDestroyAfter now guards writableEnded against a double end(). - The canonical start-time platform list is exported from the credential lock and imported by its tests, so the mirror cannot drift. - Windows gained the complement of the is_background gate: it now asserts the rejection rather than merely skipping the case. Fixes #3061
This was referenced Aug 6, 2026
Open
acoliver
added a commit
that referenced
this pull request
Aug 7, 2026
…3084) * Build the CLI bundle through the real publish path in CI The nightly cli_bundle_launch job has never passed. It called Bun.build in-process from bun test, where the bundler does not rewrite ./src/cli.js to src/cli.tsx, so it could not resolve the CLI entry own import. The same build run as the prepack subprocess resolves it, so build the way npm pack actually builds. Spawn the Bun running the test rather than node_modules/bun/bin/bun: npm 11.16 no longer runs install scripts, so the bun postinstall never materialises that binary in CI. Render the diagnostics Bun attaches to a failed build. Bun.build rejects with an AggregateError whose own message is the constant "Bundle failed"; reporting only that left the nightly failure undiagnosable. Refs #3061 * Make the nightly platform legs pass on their own terms The nightly has failed every night since 2026-08-01 across four independent jobs. This addresses the remaining three. windows_ci carried six portability defects, all of which asserted POSIX behaviour on a platform where the production code deliberately behaves differently: - the credential-write-lock suite shelled out to `ps -o lstart=` for every platform, though production gates that probe to darwin/linux/freebsd and returns null elsewhere. It now mirrors the gate and fabricates an approximate owner record where no canonical source exists, so the test still runs on Windows rather than being skipped. Its M4 sabotage also moved to replacing the lock file with a directory: the old "remove the dir, write a file" trick raises ENOTDIR on POSIX but ENOENT on Windows, where readOwner reads it as "absent" and logs nothing, silently hiding the release failure the test exists to catch. - toolRegistryFactory compared a path built from os.tmpdir() (an 8.3 short form on the runner) against the tool's resolved long path; the workspace root is now realpath'd so both sides speak the same path. - getPty's vi.doMock was bypassed on Windows, where node-pty is a real installed native addon, so the assertions received the genuine module. File-level vi.mock intercepts at module-graph build time instead. - process-run asserted an ENOENT message; Bun on Windows says "Executable not found in $PATH" for the same condition. - the quota-guard integration tests spawned their nested vitest run with process.execPath, which is Bun here. Vitest is a Node tool whose forks pool assumes a Node runtime, so the child died on Windows and the run published no sentinel. The child is now pinned to Node, and the assertions report the child's own output so a nested run that dies for an unrelated reason is no longer indistinguishable from one that failed for the intended reason. Dispatching the nightly on this branch also surfaced a regression that landed after the failing run: the shell timeout-ceiling suite builds a background invocation unconditionally, but ShellTool rejects is_background on win32 outright, so build() threw before the clamp behaviour was reached. credential-proxy-server.stop() destroyed each connection abruptly. A Windows named-pipe peer does not reliably observe that as a close, so the client kept believing it was connected and its next request hung to the timeout instead of reporting the loss. It now half-closes with the existing endAndDestroyAfter helper, which sends EOF before force-destroying. macos_ci failed on a race in the OCR canary: the monitor forwarded the upstream 200 correctly (every telemetry assertion passed), but the client's response callback and the request-level reset competed for one first-wins resolve. The status is now captured when the response object is delivered and a later reset cannot overwrite it. e2e_full (windows) failed because the rig workspace lives under .integration-tests/, which the repo root .gitignore excludes, so file discovery resolved the enclosing repo and hid every workspace file from the agent — "Listed 0 item(s). (5 ignored)". The model then refused to run a script it could not see. The workspace is now its own git root. Fixes #3061 * Fix the last two nightly legs and act on review The second branch dispatch (run 31039119968) turned E2E Windows, the CLI bundle and every other leg green, and narrowed the rest to two things. windows_ci: only getPty remained. The previous attempt was aimed at the wrong runner. packages/core runs exclusively under Bun, and getPty short-circuits to the Bun.Terminal adapter on Bun POSIX, so these three cases only ever EXECUTE on Windows — the one platform where substituting the backend in the module registry silently fails, because Bun's runner cannot reset the registry and evaluates a module-mock factory once, eagerly. That is why they passed everywhere a developer would look and failed only in CI. getPty now takes injectable backend loaders, so the selection logic is driven the same way on every runtime and the cases run locally under Vitest too. macos_ci: with the canary race fixed, the job reached a step that failure had been masking. "Run scripts-assignment Bun manifest" invokes a workspace that #3014 deleted from the Bun test manifest while leaving the step behind, so it could never pass. Both of its test files are already covered by the scripts-tests glob root and ran green in the same job's harness step, so the dead step goes rather than the workspace coming back. From review: - scripts/bun-build.config.ts rendered a diagnostic's source position only for instanceof Error values, but Bun attaches BuildMessage/ResolveMessage objects, which are not Errors — so the file/line/column was always thrown away. Confirmed by probing Bun 1.3.14 directly. - The CLI bundle test's artifact check was an opaque existsSync assertion even though the build can exit 0 having built nothing, and its process renderer printed the literal "null" for the streams spawnSync nulls out on a spawn failure — both precisely when the diagnostic matters most. - The nested-vitest launcher accepted node.cmd, which spawnSync cannot execute without shell:true; on a Windows box where a .cmd shim shadows node.exe the child would have failed to launch at all. - The credential-proxy shutdown comment claimed a Windows named-pipe half-close that libuv does not implement. What actually fixes it is ordering: server.close() waits for the connection count to reach zero, so stop() can no longer return before the peer observes the close. The socket is also paused before ending, since end() leaves the read side open and an in-flight frame would otherwise still mutate the token store mid-shutdown, and endAndDestroyAfter now guards writableEnded against a double end(). - The canonical start-time platform list is exported from the credential lock and imported by its tests, so the mirror cannot drift. - Windows gained the complement of the is_background gate: it now asserts the rejection rather than merely skipping the case. Fixes #3061 * Reconcile the nightly fixes with a moving main main moved substantially during the nightly repair and two of this branch's changes were affected directly: - #3078 implemented real Windows background shell jobs via Start-Process and removed ShellTool's win32 rejection. The timeout-clamp test therefore no longer needs a Windows gate (or the complementary rejection assertion added from review); it runs on every platform again. - #3080 independently fixed half of the OCR canary race by retaining the status observed at the response callback. Taking main's version is correct, but it is not sufficient: reverting this branch's 50ms upstream-crash delay on top of it still failed 4 of 8 runs because the reset can preempt the HTTP parser before any response object exists. Keeping the delay passed 10 of 10. The merge also confirms #3080's CLI asset-staging work composes with the publish-path bundle test and build diagnostics here. Address all three open review threads: - prefer Bun's explicit diagnostic.message when present instead of relying on a custom diagnostic object's toString(), retaining String() as fallback; - surface git-init status/stderr in the Windows E2E instead of silencing the command that establishes the workspace boundary; - strip syntactic quotes from Windows PATH entries before looking for node.exe. Refs #3061 * Close the remaining platform races exposed by nightly The merged-head nightly turned the last failures into deterministic evidence: - Windows PowerShell 5.1 parsed the BOM-less UTF-8 script as CP932, corrupting its Japanese filename literal before the shell command even ran. Give the script a UTF-8 BOM. - Config's missing client-factory contract was checked only after parser and registry startup, allowing a simple dependency error to exceed the Windows test budget. Validate the required factory before initialization side effects. - Windows log-cap enforcement killed the child before recording why. The exit event could therefore finalize the job first, as failed but without a reason. Claim a capping phase around taskkill so cap enforcement owns that transition. - Graceful proxy shutdown waited for the three-second force timer on every cooperative connection because a peer can keep its write half open after our FIN. Destroy immediately after the FIN is flushed and retain the timer solely as a fallback. These fixes preserve the behavioral assertions rather than widening timeouts or weakening platform coverage. Refs #3061 * Let same-process clients observe proxy shutdown before return Windows named-pipe close notification can arrive one event-loop turn after the server's close callback. Yield that turn before stop resolves so a client cannot start a request using stale connected state and wait for the request timeout. Refs #3061 * Match Windows filesystem behavior in the CLI Windows surfaced two more Unix assumptions in the full nightly suite: - realpath reports ENOENT when an existing parent component is a file. Walk the parent chain so the trust dialog still distinguishes a malformed path from a genuinely missing folder. - fchmod is not supported for these Windows file handles. The capability transport is a Docker/Podman path whose Windows host permissions are not Unix modes, so retain the restrictive create modes and explicit fchmod enforcement on POSIX only. Refs #3061 * Keep capability-directory validation on Windows (Refs #3061) * Stabilize constrained cross-platform nightly runners (Refs #3061) * Release bounded taskkill handles after Windows cleanup (Refs #3061) * Guarantee bounded cleanup progress on Windows (Refs #3061) * Make Windows process tests complete deterministically (Refs #3061) * Serialize Windows core test processes (Refs #3061) * Use Bun process completion for Windows shell jobs (Refs #3061) * Observe Windows shell completion before releasing handles (Refs #3061) * Wait for Windows log handles during test teardown (Refs #3061) * Own Windows shell processes directly (Refs #3061) * Stabilize Windows lifecycle assertions under load (Refs #3061) * Keep Windows nightly assertions deterministic (Refs #3061) * Reject invalid sandbox mount modes (Refs #3061) * Make Windows shell-job cleanup deterministic (Refs #3061) * Make cross-platform Nightly process lifecycle deterministic (Fixes #3061) The Nightly suite failed on Windows and macOS for reasons that were real lifecycle races rather than slow tests, so raising timeouts would only have hidden them. Windows shell jobs now terminate their whole process tree with bounded taskkill handling, and cancellation, cap enforcement and disposal agree on who owns a job's terminal transition instead of racing for it. Detached background spawning uses PowerShell job-object semantics so the spawner is not kept alive by its own child. The core Bun runner distinguishes a child that has exited from one whose pipes are still held by a descendant, reaps the POSIX process group rather than only the direct child, and reports the effective per-file timeout so a JUnit failure names the budget that actually applied. Provider model listing is bounded by an abort signal instead of hanging on a live fetch, the secure-store lock test observes acquisition deterministically, and the VS Code companion proves shutdown by rebinding the released port and reports a descriptive error when that deadline expires. Tests converted for this work run on Bun and bun:test. * Accept a read-only history in the turn error report context (Fixes #3061) Two changes landed on main independently: chat history became a read-only view, while the error-report context helper still required a mutable array. The helper only slices the history, so widening the parameter restores the agents declaration build without weakening the read-only guarantee. * Check Windows PID liveness without spawning tasklist (Fixes #3061) The Windows shell-job tests probed process liveness by spawning `tasklist` with a 5s timeout, inside loops that polled every 200ms. `tasklist` enumerates every process on the machine, so on a loaded CI runner it exceeded that timeout, the probe threw ETIMEDOUT, and tests whose subject had behaved correctly were reported as failures. The repeated spawns were also the slowest part of each poll, making the timeout more likely the longer a suite ran. Signal 0 asks the kernel the same question directly: it succeeds for a live pid, reports ESRCH for one that is gone, and EPERM for one owned by another security context. Any other errno is a real fault and is rethrown rather than reported as liveness. This is the approach the Windows spawn test already used. The probe existed in four near-identical copies, each with slightly different error handling; they now share the single implementation in the test-cleanup helper. Also widens a session-listing timestamp assertion. It allowed 1ms of clock skew between two reads, but the default Windows system timer ticks about every 15.6ms, so the two could legitimately disagree by nearly a full tick. * Remove POSIX assumptions from six Windows CI tests (Fixes #3061) These are the last failures in the Nightly Windows job. Each asserted something that is true only on POSIX, so each failed on Windows while the behaviour under test was correct. Trust rules are keyed by the resolved working directory, but the dialog test expected a POSIX literal; path.resolve('/test/dir') is drive-qualified on Windows. findEnvFile likewise resolves before walking, so the settings mock, which matched on an unresolved join, never matched the path production looked for. Keyfile permissions and the colon-sanitizing command loader assert behaviour Windows cannot express: NTFS reports 666 whatever the file was created with and chmod there only toggles the read-only flag, and ':' in a filename is reserved for alternate data streams, so such a file cannot exist for the loader to find. Both are now stated as the POSIX contracts they are, with the permission check split into its own test rather than an inline conditional. The security test compared a raw path against JSON-encoded content, which only matches when the path has no backslashes to escape; it now asserts on the parsed settings. The sandbox test used the host resolver as a stand-in for resolution inside the container, which is always Linux; on a Windows host that rewrites the container destination, so it is POSIX-only. * Treat an already-exited Windows process tree as reaped (Fixes #3061) When a per-file timeout fires, the tree often exits between the timer and taskkill running, and taskkill then reports a nonzero code because the pid no longer exists. The runner took that as a reap failure and aborted the whole Windows run, so one Bun process hanging at teardown ended the suite even though every test in the file had passed. That is the Windows analogue of POSIX ESRCH, which this code already ignores for exactly the same reason. Rather than trust the exit code either way, the reap now verifies the invariant it exists to establish — that nothing is left alive holding the child's pipes — by waiting for close, and reports the taskkill code only if the tree genuinely outlives the reap. The behavioral test asserted the old contract, so it asserted the bug; it now pins the corrected one.
This was referenced Aug 7, 2026
Open
7 tasks
This was referenced Aug 8, 2026
Merged
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.
Fixes #2847.
Migrates every test root named in #2847 to Bun's native test runner and moves CI's test execution onto it.
agentsandclistill run under Vitest and are tracked separately by #2578 — see "Remaining Vitest usage" below for the full, honest enumeration.Test roots replace curated file lists
scripts/bun-test-manifest.tspreviously listed every Bun-ready file by hand, because a partially migrated workspace could not distinguish a Bun-ready file from one still owned by Vitest. A root now selects its files in one of two ways:include/excludeglobs for fully migrated roots — the Bun-native equivalent of a Vitest config'sinclude. This is what makes "no test file can be silently dropped" mechanically true: a newly added test file runs without a manifest edit.filesfor roots still finishing their migration.A root may also declare
preload(one or more, the equivalent of VitestsetupFiles),tsconfig(a test-only--tsconfig-override),timeout,retries,globalSetup(setup()/teardown()run once in the runner process, so the env it mutates is inherited by every spawned test process), andcredentialed.credentialedmarks a root that calls a real provider. An unfiltered run covers every other root — the complete offline suite — so the PR gate never burns quota;evalsandintegration-testsare requested by name from their own workflows.What now runs under Bun
settingside-integrationvscode-ide-companionpolicytelemetrytest-utilsa2a-serverscripts/testsevalsintegration-testsEach migrated workspace's
test/test:ciinvokesscripts/run_bun_tests.tsand still emitsjunit.xmlfor the CI test reporter.scripts/test.ts, the rootpackage.jsonscripts,.github/workflows/ci.yml,dev-docs/bun.mdandCONTRIBUTING.mdall point at the Bun-native path, withbun run test:bunas the single canonical command.Integration tests need real provider credentials, so they cannot pass locally; what was verified is that all 31 files load and collect under Bun, with the residual failure being the same
assertProviderConfigerror Vitest reports.evalswas verified end to end: the report lands atevals/logs/report.jsonandscripts/aggregate_evals.tsparses it.Compatibility gaps closed in the shim
Each of these was a whole class of failures rather than a single file:
it/test/describe.runIf— Bun shipsskipIfbut notrunIf, so gated tests failed to even collect.automockValuenow mirrors accessors instead of reading them.node:fsexposes getters backed by private class fields that throw off-instance, which aborted the automock of the whole module.restoreAllMocksalso resets spy state, matching Vitest'smockRestore. Without it a spy installed over an automocked export kept its call history across tests.waitForunder Bun's fake timers: the loop advanced the clock but never yielded, so a promise chain resumed by a timer could not progress between attempts. It also attempts the callback at t=0 like the real-timer path, and no longer advances twice between retries after an async rejection.PTY
@lydell/node-ptynever deliversonData/onExitunder Bun on POSIX (oven-sh/bun#25822), so the interactive harness selectsBun.spawn's terminal backend (packages/test-utils/src/pty-backend.ts). It is not shared withcore's adapter becausecorealready dev-depends ontest-utils, and importingcorehere would close a dependency cycle.Bugs the migration exposed
waitFordeadlock. The openai-responses abort suite hangs onmainunder Bun; it now passes.describesuite inside the file-level suite; attributing the nested suite's cases to its parent doubled every count the evals aggregation reads.token-tracking-property.test.ts's 24 property tests were inert.@fast-check/vitestv0.2 dropped the 3-argumentitProp(name, [arbs], fn)form, so the arbitraries were swallowed as an options object and each predicate ran once with the Vitest context instead of generated values. Driving them through plainfast-checkexposed three assertions that never held —totaldeliberately excludes cache tokens,formatSessionTokenUsagegroups digits viatoLocaleString(), and a freshly recorded entry is inside the 60-second window. Same test count (24 pass, 1 skip); now with real generated values.Retired Vitest configuration
Ten
vitest.config.tsfiles are deleted. The invariants they guarded are re-expressed against the manifest rather than dropped: the evals report path (scripts/tests/evals-report-path.test.ts), the OCR workflow's test discovery, the scripts shard's two invocations, and the settings boundary alias check.Remaining Vitest usage
Vitest is no longer the runner for any root named in #2847, but it is not yet absent from the repository. Enumerated honestly:
Still executes Vitest (out of this issue's scope, tracked by #2578):
packages/agentstest/test:ciagentsshard, viascripts/test.tspackages/clitest/test:ci(+test:integration,test:ci:covered,test:ci:fast,test:legacy)clishard, viascripts/test.tspackages/storagetest:vitestsecure_store_backendjob inci.ymland its nightly twin, which need the two backend-specific configspackages/test-utils/src/quota-guard-vitest-integration.test.tsDoes not execute —
viteststays indevDependenciesbecause migrated test files still importdescribe/it/expectfrom thevitestspecifier, which Bun resolves through its own injected handler.test:vitestescape hatches remain onauth,lsp,mcp,providers,storageandtools; no workflow ortestscript invokes them.So the issue's "CI uses Bun-native execution as the primary path for all workspaces" holds for every root this PR owns, and does not yet hold for
agents,clior the SecureStore backend matrix.Two decisions worth a second opinion
bun_native_test_paritycost. It now runs the complete 798-file non-credentialed manifest at a 90-minute cap, which substantially duplicatestest_shard. Repurposing it as a manifest-completeness gate would give the same protection far more cheaply.Verification
npm run typecheck(all workspaces),npx eslint,npx prettier --check,npm run lint:eslint-guard,npm run build, and the CLI smoke (bun scripts/start.ts --profile-load stepfun-37) all pass. The complete non-credentialed manifest and every migrated workspace suite pass locally.Open Code Review found six issues, all remediated in this branch. No test was dropped, skipped or filtered; no lint rule, complexity threshold or type suppression was loosened — the runner test file was split rather than raising
max-lines.Summary by CodeRabbit
New Features
Bug Fixes
Documentation