Repository navigation
refactor(cli): port runtime to Node-compatible APIs - #29
Merged
Merged
Conversation
Replace the Bun-only APIs so funfa runs on Node as well as Bun: Bun.spawn -> node:child_process execFile (argv-only, preserves the ENOENT->127 contract), Bun.argv -> process.argv, @effect/platform-bun BunContext/BunRuntime -> @effect/platform-node NodeContext/NodeRuntime, BunPath -> NodePath in the CLI test, and StderrBun -> StderrNode (it was already runtime-neutral). Drop the now-unused @effect/platform-bun dependency; add @effect/platform-node@0.106.0 (matches @effect/platform 0.96 / effect 3.21 / platform-node-shared 0.59 already in the tree). pi itself uses no Bun globals, and skills inline at build time via the .md text loader. No distribution change: still builds via 'bun build --compile'. This unblocks shipping a small node bundle later. Verified: tsc + Ultracite clean, full suite 105 pass / 1 skip / 0 fail (incl. real makeNodeApplyRunner echo + ENOENT->127 tests), and a node-target bundle runs under node v25 (--help + auth).
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
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.
What & why
funfa's only Bun-specific code was a handful of spots in its own source — pi itself uses zero Bun globals. This swaps those for Node-compatible APIs so funfa runs on Node as well as Bun, unblocking a much smaller distribution later (a
bun build --target nodebundle is ~6.9 MB / 1.8 MB gzip, vs the 74 MB compiled binary — most of which is the embedded Bun runtime).No distribution change in this PR — it still builds via
bun build --compile. This is purely the API port; how we ship (single node bundle vs npm package vs keep the compiled binary) is a separate decision.Changes
Bun.spawn→node:child_processexecFile(argv-only — same no-shell contract — preserving theENOENT→127 "command absent" signal and the child's real exit code).apply-command.tsBun.argv→process.argv.index.ts@effect/platform-bunBunContext/BunRuntime→@effect/platform-nodeNodeContext/NodeRuntime;BunPath→NodePathin the CLI test.StderrBun→StderrNode(it only usedprocess.stderr.write— runtime-neutral, just misnamed).@effect/platform-bundep; added@effect/platform-node@0.106.0(matches@effect/platform0.96 /effect3.21 /@effect/platform-node-shared0.59 already resolved).Skills still inline at build time via the
.mdtext loader, so nothing reads them from disk at runtime.Verification
tsc --noEmit+ Ultracite clean.makeNodeApplyRunnertests (echoround-trip + missing-binary → exit 127) and the existingbun --compilesmoke test (the ported code still runs under Bun).--target nodebundle and ran it under node v25:--helprenders (exit 0, clean stderr) andauth bogushits the real provider registry.Follow-up (not in this PR)
Pick a small-distribution model — single self-contained
funfa.mjs(requires node) vs npm package vs keep the 74 MB zero-dep binary — then wire the launcher/install/CI/smoke-test accordingly.