Repository navigation
[CI] Run Vitest on Bun with Node runtime contracts - #25840
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughVitest now runs through Bun by default and assigns selected runtime-contract tests to Node workers. A separate command runs Vitest entirely on Node. CI workflows, runtime setup, and timing reports now account for the Bun and Node versions. Tests were also updated for runtime-specific behavior, asynchronous setup, and child-process IPC serialization. Priority: ⬇️ Low Merge Risk: 🔵 Low · up to The shutdown tests should exercise Node workers before merge. The identified gap affects test confidence rather than demonstrated application behavior. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
88bd27a7-a1f7-469f-9dcb-a8cb4c3a1b80
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (34)
.github/CONTRIBUTING.md.github/workflows/ssh-hostile-hosts.yml.github/workflows/unit-tests.ymlconfig/bun-versionconfig/oxlint-anti-slop.jsonconfig/scripts/ci-unit-timing-import.mjsconfig/scripts/ci-unit-timing-reporter.mjsconfig/scripts/ci-unit-timing-reporter.test.mjsconfig/scripts/hang-watchdog-process-metrics.test.mjsconfig/scripts/run-vitest.mjsconfig/scripts/ssh-hostile-hosts-workflow.test.mjsconfig/scripts/vitest-bun-node-builtins.tsconfig/scripts/vitest-node-runtime-boundary.test.tsconfig/scripts/vitest-node-runtime-files.mjsconfig/scripts/vitest-node-runtime-pool.tsconfig/scripts/vitest-node-runtime-worker.mjsconfig/scripts/vitest-runtime-boundary.test.tsconfig/tsconfig.node.jsonconfig/vitest.config.tspackage.jsonsrc/main/daemon/daemon-scope-death-watch.test.tssrc/main/native-chat/agent-session-wire/structured-conversation-compaction.test.tssrc/main/runtime/runtime-remote-fetch-ref-maintenance.test.tssrc/relay/fs-handler-list-files-diagnosis-cancel.test.tssrc/relay/fs-search-cancellation.test.tssrc/relay/fs-search-diagnosis-cancellation.test.tssrc/renderer/src/components/UpdateCard.error-card.test.tsxsrc/renderer/src/components/activity/activity-auto-mark-read-loop.react185.test.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/ssh-routed-browser-page-gate.test.tsxsrc/renderer/src/components/right-sidebar/file-explorer-drag-scroll-marker.test.tsxsrc/renderer/src/components/terminal-pane/remote-runtime-pty-transport-stream-reconnect.test.tssrc/shared/child-process/process-spec.tssrc/shared/child-process/run-process.test.tssrc/shared/child-process/spawn-resolution.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
615f7fe7-a54e-41a7-8615-568e7fcbc324
📒 Files selected for processing (1)
src/main/ipc/terminal-render-desync-evidence.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
config/scripts/vitest-node-runtime-pool.test.ts (1)
32-32: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse the launcher’s Node executable for pool workers.
The test launcher runs Vitest under Bun and passes the Node executable in
ORCA_TEST_NODE_EXECUTABLE. Thenode-runtimeproject selectsnodeRuntimePool, butoptions()replaces that value with Bun’sprocess.execPath. The pool then spawns Bun, so the shutdown tests do not exercise a Node worker. Keep the explicit missing-executable argument for the startup-failure test.🐛 Suggested fix
-function options(executable = process.execPath): PoolOptions { +function options( + executable = process.env.ORCA_TEST_NODE_EXECUTABLE ?? process.execPath +): PoolOptions {
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
ef563b14-5324-40d6-8ff9-a1570c23a525
📒 Files selected for processing (5)
config/scripts/vitest-node-runtime-files.mjsconfig/scripts/vitest-node-runtime-pool.test.tsconfig/scripts/vitest-node-runtime-pool.tssrc/main/ipc/terminal-render-desync-evidence.test.tssrc/main/runtime/runtime-remote-fetch-ref-maintenance.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/ipc/terminal-render-desync-evidence.test.ts
- config/scripts/vitest-node-runtime-files.mjs
- config/scripts/vitest-node-runtime-pool.ts
- src/main/runtime/runtime-remote-fetch-ref-maintenance.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.
7 of 9 tasks
brennanb2025
added a commit
that referenced
this pull request
Oct 6, 2026
Bun formats a method's stack frame without its class ("at step"), so the quit
test's caller check never matched, the step was never held, and both cases timed
out once CI ran Vitest on Bun (#25840). Only the drain step heals owed queue
bookkeeping, so the hold needs no caller check.
brennanb2025
added a commit
that referenced
this pull request
Oct 6, 2026
… held card (#24660) * fix(native-chat): keep a message accepted before a quit or crash as a held card A send the host accepted while the agent was still starting, and never handed over, was rejected unseen at quit or at the next open after a crash. The next open now keeps a person's message (typed, or a launch's first prompt) as a waiting card at the head of the queue, held until Resume, Send now, Edit or Delete; quit no longer rejects it. Each submission records its source so a restart knows which leftovers to keep. A direct send's replay answers from its own record, never the queued arm. Cards shown without the queue capability hide Turn off queueing and the steer chord. * fix(native-chat): keep an unsent message as a held card at every close, not only a restart The host now keeps a person's message it accepted and never handed over with one rule wherever it can no longer hand it over: a quit or crash (settled at the next open) and a close of the chat (tab close, worktree teardown, orchestration stop). - The hold is card state: a new per-card hold_reason 'kept', published as the existing pausedReason, instead of a fake host_instance value. host_instance means the owner again, and the pause clause, adoption filter and /clear carry special cases are gone. A kept card holds the cards behind it until the person sends, edits or deletes it; a send_failed card still does not. - A card hand-off rejected by a restart or a close returns as a kept card (rejectedDraftSettlement), so a person's next message can never release it. - One hold function, parameterized by cause (hostRestarted / chatClosed), replaces the close path's plain rejection. - The phone shows published cards and per-card holds whatever the queue capability says; only queueing a new send stays gated. - source gains 'dispatch' for the orchestration preamble (still rejected); an unknown source is kept as written and never takes the legacy rule. - Kept cards from earlier settlements stay ahead of a batch's new ones. * test(native-chat): the dispatch preamble records its source * fix(native-chat): skip a kept card like a failed one, and make a quit leave the queue as a crash does - A kept card is held on its own, as a send_failed one is: the queue sends the cards behind it, and the "a message ahead needs attention" caption no longer appears behind it (desktop and phone). - Quit disposes the queue's drain together with delivery, so it mints no hand-off that only the next process could settle. - Only a Send the person asked for (origin client) that a restart or close cut short returns kept; the queue's own hand-off returns where it stood, under the restart's pause, as on main. - Comments that said only a capable host gets cards or the card actions now say the capability gates only queueing a new send. * refactor(native-chat): one dispose gate in the queue drain's step * test(native-chat): the downgrade test names the older build's /clear exception * fix(native-chat): re-check the drain's quit gate right before it appends a hand-off A drain step already past its first check when quit begins no longer makes a hand-off. Adds regression tests for that, for Send now on an ordinary card cut short by a quit, and for the open-time repair deriving kept from the hand-off's origin; fixes the pause and settlement comments that called a kept card one the queue never passes. * fix(native-chat): a card held on its own starts no restart pause for the others After a second restart a kept (or send_failed) card is another process's, but it waits for its own Send, so it no longer pauses every other card under "Queue paused because Orca restarted". * fix(native-chat): record on a kept send which card holds it, so no trace survives an Edit or Delete The rejection row of a send the host kept as a card now names that card (`keptAsQueuedMessageId`), in the same transaction that writes the card, and the fold publishes it on the submission. The shared projection draws such a send only as its card: once the card is sent, edited or deleted, neither the send nor the sending desktop's local copy of it shows. * chore(native-chat): leave the unused submission schema as main has it No client parses published submissions with it (they arrive as typed frames, and the host's history pages carry the field, as the Edit/Delete test reads); listing the field put the file over its line budget. * fix(native-chat): retire a kept send's local copy instead of only hiding it The outbox reconcile and the send disposition drop an entry whose submission the host rejected as kept as a card, as they already do for a Stop's withdrawal, so the copy never comes back as "Not sent / Retry" once the submission falls out of the loaded page. The projection reads the reconciled outbox, so its separate filter goes. * test(native-chat): move the queued-message rig's scripted provider into its own fixture The rig fixture grew past the 300-line limit once main's changes merged in. * fix(native-chat): hide a kept send by its own record, not by its card still existing The transcript hid a rejected send while a queued card held it under its id, so the card's Edit or Delete brought back a "Not sent" row. It now reads the send's own keptAsQueuedMessageId, which the host records with the rejection, and the live card list is no longer threaded to the transcript or the delivery notices. * refactor(native-chat): move a sent message's row writes into their own journal collaborator The journal store went past its line limit once main's ledger receipt joined this branch's transaction hook. The submission and dispatch-transition writes, and what commits in their transaction, now live in JournalSubmissionWriter; the store's methods delegate to it unchanged. * fix(native-chat): list a kept send's card id in the submission schema The schema drops keys it does not list, so a reader that kept a parsed submission would lose keptAsQueuedMessageId and source, both persisted with the row. The submission schema and the failure fact it shares with item bodies move to their own modules, with room for both fields. * test(native-chat): match the transcript and outbox hook signatures main and the swap changed * test(native-chat): import the journal types once in the queue-delivery test * fix(mobile): a resend the host kept as a card shows no error and returns no text The host answers a resend of a message it kept as a card with that message's rejected submission, marked keptAsQueuedMessageId. The phone read it as any rejection: "Message not sent" and the text back in the composer, while the card showed the same text. It now answers like a queued send, as the desktop's send disposition does: the id is spent, no error, and the card holds the text. * test(native-chat): pin that quit's first step stops the queue's hand-off Quit now stops delivery, the queue drain included, at its first step (stopDelivery), before teardown drains recovery. The drain-step quit test runs from that step as well as from the flush. * test(native-chat): give cards their source and store unknown sources as another build would #25078 made a card's source required, so the tests that insert a card pass the person's. The hold's unknown-kind and unreadable-source cases now rewrite the stored row the way a newer build would leave it, instead of casting a type. * refactor(native-chat): stop delivery and the queue drain in one line at quit Main's #25159 left the host at its line limit; quit's stop now disposes both in one expression instead of a block. * test(native-chat): hold the drain step without reading the call stack Bun formats a method's stack frame without its class ("at step"), so the quit test's caller check never matched, the step was never held, and both cases timed out once CI ran Vitest on Bun (#25840). Only the drain step heals owed queue bookkeeping, so the hold needs no caller check.
7 of 8 tasks
This was referenced Oct 6, 2026
nwparker
changed the title
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.
ELI5
pnpm testnow runs ordinary Vitest suites on Bun. Tests that depend on the application's Node runtime still run in Node, within the same invocation and report.What Changed
pnpm test:nodeas the full Node fallback. SSH host contracts continue using Node.Why
This gives a smaller first step than rewriting thousands of tests for Bun's native runner. It preserves runtime contracts where Bun currently differs, including a SQLite close/checkpoint problem.
Benchmarks
Measured on an Apple M4 Max, macOS arm64, Node 24.20.0 and Bun 1.4.2. The merged implementation uses Vitest 4.1.11.
Previous Node configuration versus this PR: same source files and test identities, four workers, one warmup and five measured runs per runner, alternating runner order. Times are median process wall time, including startup, setup, reporting and exit. All 20 measured runs passed with identical case identities and outcomes.
DOM timings were noisy: Bun ranged from 7.867–14.905s, versus Node's 9.282–11.957s. Background TypeScript compilation was observed during this comparison. These samples do not establish a whole-suite or CI speedup.
A separate comparison using this PR's current Node fallback configuration, with the same samples and measurement method, recorded 5.188s → 4.412s for shared code (15.0%) and 9.916s → 7.742s for DOM (21.9%). Those numbers compare the two current runtime configurations, rather than the previous configuration, so the table above is the relevant before/after result.
Full-suite measurements:
These full runs differ in source, test count and timing boundaries; they are verification measurements, not a controlled full-suite before/after speedup. The merged run had 107,508 passing cases, one expected failure and 849 existing skips. The later Node baseline had 107,513 passing cases, one expected failure and the same 849 skips.
The five merged Linux ARM CI unit-test steps took 464s, 579s, 477s, 507s and 505s, excluding installation. Their artifacts covered all 11,110 files discovered at the newer merge base exactly once. No comparable pre-migration CI timing was established, so these are not a CI speedup claim.
Further optimization experiments are not included in this PR. The expanded goal is twice the full-suite speed: at most 271.248s against the later 542.496s passing baseline. Faster VM/native-runner experiments were rejected for failed assertions, worker crashes or incomplete execution. The latest interleaved full run collected all 11,048 files / 108,363 cases but took 578.475s and had one failed assertion. The 2× goal has not been achieved; none of those experimental runners has shipped.
Raw sample reports were retained locally in
notes/bun-migration/benchmark/before-after/andnotes/bun-migration/benchmark/current-runners/; full-run reports are undernotes/bun-migration/andnotes/bun-migration/performance/. These artifacts are not committed in this PR.Linked Issue
N/A — maintainer-requested test-runner work.
Visual Proof
N/A — test tooling and fixtures only; no application UI or interaction change.
Testing
Verified on macOS arm64 with Node 24.20.0, Bun 1.4.2 and Vitest 4.1.11:
pnpm tc, changed-code quality gate, and commit-time lint/format checks pass.ORCA_BACKGROUND_LAUNCH=1 ORCA_BALANCE_UNIT_SHARDS=1 pnpm test --maxWorkers=16passes: 10,921 passing suites plus 125 existing skipped suites; 107,508 passing tests, one expected failure and 849 existing skipped cases. No unhandled errors. Test and timing reports cover all 11,046 files.Review
Reviewed worker cleanup, IPC serialization, builtin test guards, project exclusions, shard discovery, and failure reporting. Follow-up review adds a bounded shutdown deadline, explicit spawn/kill failure handling, and four real-child lifecycle tests; all CI checks passed again on commit
096b5ea17ade. The child-process wrapper preserves hidden Windows launches and disables shell execution. Native dependencies remain installed for Node.Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Production runtimes and remote protocols are unchanged. SSH host, relay integration, mobile, cloud, and specialized runtime jobs retain Node. CI validation passed on Linux, macOS and Windows. The unchanged Node mobile browser job passed after one retry of a Chromium navigation timeout.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)