feat(cli): run app dev candidates through execa - #117
Conversation
Migrates the local dev-server launcher ladder from raw node:child_process spawn to execa (already a CLI dependency). execa spawns through cross-spawn, so on Windows the npx rung resolves its .cmd shim via PATHEXT instead of always failing with a false ENOENT, which made the Next.js ladder bunx-or-bust there. The hand-rolled win32 next.cmd special case goes away for the same reason. Behavior preserved: inherited stdio, ENOENT falls through to the next candidate, exit code and termination signal are reported (SIGTERM on abort still maps to the controller's cancel path), and the child env is passed exactly (extendEnv: false). Spawn failures other than ENOENT now surface with execa's message instead of a raw errno throw. Drops the unused spawnImpl injection seam; tests mock runLocalApp itself. Verified live: bun --watch runs and terminates cleanly on abort, and an all-ENOENT ladder still produces the friendly install message.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughLocal development command execution was standardized on 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
|
Closing as superseded by the platform port. This PR improves |
Companion to project-compute#106 (approved): the SDK's spawn layer moved to execa there; this migrates the one remaining Windows-broken spawn site in the CLI, the
app devlauncher ladder inlib/app/local-dev.ts.Why
The ladder spawned candidates with raw
spawnand no shell. On Windows,npxis a.cmdshim, which rawspawncannot launch (only.exe/.comresolve), so the npx rung always failed with a false ENOENT and Next.js dev degraded to bunx-or-bust. There was even a hand-rollednext.cmdspecial case for the local-bin rung; execa (via cross-spawn, PATHEXT) makes that unnecessary, so it's gone.execa is already a CLI dependency (
controllers/init.ts,controllers/agent.ts), so no new ecosystem choice.Survey of remaining spawn sites (checked all of
packages/)lib/app/local-dev.ts: migrated here.shell/update-check.ts: spawnsprocess.execPath(an absolute path to the running Node binary) detached and unref'd. No PATH or shim resolution involved, no output handling, and execa adds nothing but risk aroundunrefsemantics. Left as is, deliberately.adapters/git.ts,lib/git/local-status.ts:execFile("git", ...);git.exeresolves fine on Windows. Left as is.Behavior preserved
AbortError) is unchanged.extendEnv: false).spawnImplinjection seam; nothing consumed it (tests mockrunLocalAppitself).Testing
bun --watchapp spawns, streams output, and terminates cleanly on abort (SIGTERMreported); an all-ENOENT ladder (empty PATH) produces the friendly message.