feat(stack-cli): auto-install emulator deps on pull#1384
Merged
Conversation
`stack emulator pull` now detects missing QEMU/VM dependencies (and aarch64 UEFI firmware on arm64), prints the list, and — with user confirmation — installs them via brew (bootstrapping Homebrew first if absent) or apt. Skipped under --skip-snapshot.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe emulator pull flow adds an interactive ensureDepsForPull(arch) step that detects missing qemu/utility binaries and optional aarch64 firmware, formats install instructions via a new installHint helper, and—on interactive macOS/Homebrew or apt-get Linux—proposes and can auto-install packages (bootstrapping Homebrew if needed); skip with --skip-snapshot. Non-interactive or unsupported platforms fall back to preflightForVmStart. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Pull as "emulator pull"
participant Deps as "ensureDepsForPull"
participant Detect as "Dependency Detector"
participant PkgMgr as "Package Manager (brew/apt-get)"
participant System as "Host System"
User->>Pull: Run `emulator pull` (with/without --skip-snapshot)
alt --skip-snapshot
Pull->>Pull: Skip ensureDepsForPull
else
Pull->>Deps: ensureDepsForPull(arch)
Deps->>Detect: Check missing qemu/utilities + optional firmware
Detect->>System: Query installed tools/packages
System-->>Detect: Installed state
Detect-->>Deps: Missing/optional list
Deps->>Deps: Build platform-specific install plan (installHint)
Deps->>User: Display plan (TTY-only on macOS/Linux)
alt Interactive TTY on supported OS
Deps->>User: Prompt for confirmation
User-->>Deps: Confirm / Decline
alt Confirm
Deps->>PkgMgr: Bootstrap if needed (e.g., install brew)
PkgMgr->>System: Install packages
System-->>PkgMgr: Install results
PkgMgr-->>Deps: Success
Deps-->>Pull: Dependencies satisfied
else Decline
Deps-->>Pull: Fail / abort
end
else Non-interactive or unsupported
Deps-->>Pull: Fallback to preflightForVmStart
end
end
Pull->>Pull: Continue pull workflow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
Greptile SummaryThis PR adds a dependency preflight step to
Confidence Score: 3/5Not safe to merge as-is: the Apple Silicon PATH bug will cause One P1 defect (brew PATH not updated for the current process on Apple Silicon after bootstrap) that reliably triggers on the machines most likely to need the auto-install path. The security concern around curl-pipe-bash without integrity verification is P2 but worth addressing. packages/stack-cli/src/commands/emulator.ts — specifically the
|
| Filename | Overview |
|---|---|
| packages/stack-cli/src/commands/emulator.ts | Adds ensureDepsForPull to auto-install QEMU/tool deps before emulator pull; contains a P1 PATH bug where brew is unreachable on Apple Silicon after a fresh Homebrew bootstrap, and a P2 non-TTY fallback gap. |
Prompt To Fix All With AI
This is a comment left during a code review.
Path: packages/stack-cli/src/commands/emulator.ts
Line: 494-501
Comment:
**`brew` not in PATH after bootstrap on Apple Silicon**
After the Homebrew install script runs, the newly created `/opt/homebrew/bin/brew` binary is **not** in the current process's PATH — environment variables are inherited at process start and the install script only updates shell profile files (e.g. `~/.zprofile`). On Apple Silicon Macs the default system `PATH` does not include `/opt/homebrew/bin`, so the immediately following `execFileSync("brew", ["install", ...])` call will throw a "command not found" error, leaving the user with a half-installed state.
The fix is to resolve the `brew` binary by its known absolute path after a fresh install:
```typescript
// After Homebrew bootstrap, resolve brew's absolute path since the
// current process PATH has not been updated.
const brewBin = existsSync("/opt/homebrew/bin/brew")
? "/opt/homebrew/bin/brew"
: "/usr/local/bin/brew"; // Intel fallback
console.log("\nInstalling packages...");
execFileSync(brewBin, ["install", ...pkgList], { stdio: "inherit" });
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/stack-cli/src/commands/emulator.ts
Line: 492-496
Comment:
**Homebrew bootstrap executes remote code without integrity verification**
The Homebrew install script is fetched over HTTPS and piped directly to `bash` without any checksum verification. If `raw.githubusercontent.com` is intercepted (e.g. a corporate TLS-inspecting proxy) or GitHub's CDN is compromised, this would silently execute malicious code on the user's machine.
Consider verifying the script's SHA-256 against a known-good hash before executing, or at minimum document this risk in the user-facing prompt so users can make an informed decision.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/stack-cli/src/commands/emulator.ts
Line: 460-465
Comment:
**Non-TTY path on darwin/linux produces unhelpful error**
`ensureDepsForPull` is only reached when `missingBins.length > 0 || firmwareMissing`. In non-TTY environments (CI, piped input) on macOS or Linux, `confirmPrompt` immediately throws "stdin is not a TTY" with no install hints. Consider falling back to `requireBinaries` (which produces actionable install hints) when stdin is not a TTY, rather than only throwing the TTY error.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(stack-cli): auto-install emulator d..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- Resolve `brew` by absolute path after a fresh Homebrew bootstrap, since the installer only updates shell profiles and not the current process's PATH (breaks on clean Apple Silicon installs). - Fall back to `preflightForVmStart` when stdin is not a TTY so CI/piped runs get the standard per-binary install hints instead of a generic TTY error. - Disclose in the confirmation prompt that Homebrew bootstrap executes remote code from raw.githubusercontent.com.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stack-cli/src/commands/emulator.ts (1)
513-522:⚠️ Potential issue | 🟠 Major
ncLinux package mapping installs the wrong binary.The entry
bin("nc", "ncat", "netcat")at line 518 has itslinuxPkgvalue passed directly tosudo apt-get installvia the pkgList at line 502. On Debian/Ubuntu, thencatpackage provides/usr/bin/ncat(from the nmap suite), not/usr/bin/nc. Thencbinary is provided bynetcat-openbsd(ornetcat-traditional). After auto-install completes,commandExists("nc")will still return false, and the subsequent operation will fail with the same "missing binaries" error — the exact failure this new flow attempts to prevent.Note that
netcatis correct for the macOS Homebrew formula name, butnetcat-openbsdis required on Debian.Proposed fix
- bin("nc", "ncat", "netcat"), + bin("nc", "netcat-openbsd", "netcat"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stack-cli/src/commands/emulator.ts` around lines 513 - 522, The Linux package mapping for the "nc" binary in commonVmBins() is incorrect: update the BinarySpec entry created via bin("nc", ...) so its linuxPkg uses the Debian package that provides /usr/bin/nc (e.g., "netcat-openbsd") instead of "ncat" or "netcat"; leave the macPkg (Homebrew) as "netcat" and keep the binary name "nc" unchanged so commandExists("nc") succeeds after auto-install. Locate the bin call for "nc" in commonVmBins() and change only the linux package string to "netcat-openbsd".
🧹 Nitpick comments (1)
packages/stack-cli/src/commands/emulator.ts (1)
453-507: Consider re-verifying after install and de-duplicating the zstd spec.A couple of quality-of-life follow-ups for
ensureDepsForPull:
- After
brew install/apt-get installsucceeds, no re-check is performed. If the install ran cleanly but didn't actually provide a required binary (see thenc/ncatissue), the user sees a "still missing" error several seconds later fromcaptureLocalSnapshot. CallingpreflightForVmStart("pull", arch)(or re-running the samecommandExistsfilter) at the end would surface that immediately with context.bin("zstd", "zstd", "zstd")is repeated on line 454 and line 533. Consider hoisting it to azstdBin()helper to keep the spec authoritative in one place.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stack-cli/src/commands/emulator.ts` around lines 453 - 507, ensureDepsForPull currently doesn't re-verify that installed packages provided the required binaries and repeats the zstd bin spec; after the install steps (after brew/apt-get execFileSync) re-run the same check (e.g., call preflightForVmStart("pull", arch) or re-evaluate missingBins via commandExists and firmware check) and throw or surface the same contextual error if things are still missing so users see immediate feedback; also deduplicate the zstd spec by centralizing it (create a zstdBin() helper or a shared constant used where bin("zstd", "zstd", "zstd") appears) and use that symbol in the allBins array and any other place to keep the spec authoritative.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/stack-cli/src/commands/emulator.ts`:
- Around line 459-465: The Linux auto-install currently assumes apt-get for all
distros; update the platform check in emulator.ts so that after confirming
process.platform === "linux" you also verify commandExists("apt-get") before
performing the apt-get install path—if commandExists("apt-get") is false, call
preflightForVmStart("pull", arch) and return (mirroring the existing non-Linux
fallback). Apply the same guard where the other apt-get auto-install block
occurs (the second occurrence around the related install logic) to avoid running
apt-get on non-Debian systems.
- Around line 472-505: The install plan can end up calling `brew install` with
an empty `pkgList` when `firmwareMissing` is true on macOS but `missingBins`
yields no packages; update the flow around `pkgs`/`pkgList` to avoid invoking
`brew install` with no args: detect the case where `pkgList.length === 0 &&
firmwareMissing && platform === "darwin"` and either (A) add `"qemu"` to `pkgs`
so `pkgList` includes a package to reinstall firmware, or (B) short-circuit and
call `preflightForVmStart()` / throw a clear `CliError` to surface the firmware
issue instead of attempting an empty install; ensure the change touches the
variables `pkgs`, `firmwareMissing`, `platform`, and the code paths that call
`execFileSync("brew", ...)` so you never call `brew install` with an empty
argument list.
---
Outside diff comments:
In `@packages/stack-cli/src/commands/emulator.ts`:
- Around line 513-522: The Linux package mapping for the "nc" binary in
commonVmBins() is incorrect: update the BinarySpec entry created via bin("nc",
...) so its linuxPkg uses the Debian package that provides /usr/bin/nc (e.g.,
"netcat-openbsd") instead of "ncat" or "netcat"; leave the macPkg (Homebrew) as
"netcat" and keep the binary name "nc" unchanged so commandExists("nc") succeeds
after auto-install. Locate the bin call for "nc" in commonVmBins() and change
only the linux package string to "netcat-openbsd".
---
Nitpick comments:
In `@packages/stack-cli/src/commands/emulator.ts`:
- Around line 453-507: ensureDepsForPull currently doesn't re-verify that
installed packages provided the required binaries and repeats the zstd bin spec;
after the install steps (after brew/apt-get execFileSync) re-run the same check
(e.g., call preflightForVmStart("pull", arch) or re-evaluate missingBins via
commandExists and firmware check) and throw or surface the same contextual error
if things are still missing so users see immediate feedback; also deduplicate
the zstd spec by centralizing it (create a zstdBin() helper or a shared constant
used where bin("zstd", "zstd", "zstd") appears) and use that symbol in the
allBins array and any other place to keep the spec authoritative.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2d806fdb-de58-4b06-8c79-73a1c3516b2c
📒 Files selected for processing (1)
packages/stack-cli/src/commands/emulator.ts
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
…nly miss on macOS - Only take the apt-get path when `apt-get` is actually available; otherwise fall through to the standard per-binary install hints so Fedora/Arch/Alpine/etc. users get something actionable. - On macOS arm64 when all binaries are present but aarch64 firmware is missing, add `qemu` to the brew install set so a reinstall recreates the bundled firmware; guard against an empty package list by falling back to preflight.
Contributor
|
Promptless prepared a documentation update related to this change. Triggered by PR #1384 Adds documentation for the stack emulator CLI, covering VM prerequisites, auto-install of dependencies on macOS and Linux, and all emulator commands (pull, start, stop, reset, status, run, list-releases). |
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.
Summary
stack emulator pullnow preflights VM dependencies (QEMU binaries, socat/curl/nc/lsof/openssl/zstd, and aarch64 UEFI firmware on arm64) before downloading.brewon macOS (bootstrapping Homebrew itself if absent) orsudo apt-geton Linux.--skip-snapshotis passed, since that path never boots the VM.gh/GITHUB_TOKENare intentionally excluded from the auto-install set.Test plan
node packages/stack-cli/dist/index.js emulator pullon a machine with all deps present → no prompt, proceeds as before.brew unlink zstd) and rerun → missing dep listed, decline prompt → exits with a clear error; accept prompt → brew install runs and pull continues.emulator pull --skip-snapshotstill bypasses the dep check.sudo apt-get update && sudo apt-get install -y ….Summary by CodeRabbit
New Features
Behavior