Skip to content

Allow use on x86_64 systems#46

Merged
mgabor3141 merged 2 commits into
mgabor3141:mainfrom
ian-ross:enclave-x86-vm
May 2, 2026
Merged

Allow use on x86_64 systems#46
mgabor3141 merged 2 commits into
mgabor3141:mainfrom
ian-ross:enclave-x86-vm

Conversation

@ian-ross
Copy link
Copy Markdown
Contributor

@ian-ross ian-ross commented May 1, 2026

This PR uses the system architecture to decide which QEMU binary to run. Allows for use on x86_64 machines.

@mgabor3141
Copy link
Copy Markdown
Owner

@greptile

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 2, 2026

Greptile Summary

This PR introduces qemuBinaryForHost() to select either qemu-system-aarch64 or qemu-system-x86_64 based on process.arch, then threads the resolved path through both checkQemuAvailable and the sandbox.qemuPath option in VM.create() — ensuring the preflight check and the actual VM launch always use the same binary. The previously noted gap where checkQemuAvailable could pass on an x86_64 host while VM.create() still hardcoded aarch64 is now fully addressed.

Confidence Score: 5/5

Safe to merge — the change is minimal, well-scoped, and correctly resolves the architecture mismatch for x86_64 hosts.

No P0 or P1 issues found. The previous P1 concern (VM.create() ignoring the architecture-specific binary) is addressed by passing qemuPath explicitly. Only a P2 documentation clarity nit remains.

No files require special attention.

Important Files Changed

Filename Overview
packages/enclave/src/vm.ts Adds qemuBinaryForHost() helper; threads the resolved binary into both checkQemuAvailable and VM.create() sandbox options — correctly closes the gap where the preflight check and VM launch could use different binaries.
packages/enclave/README.md Install instructions updated for x86_64; minor wording ambiguity around the qemu-system-x86 package vs qemu-system-x86_64 binary name.
.changeset/red-vms-sneeze.md Changeset entry for a patch bump with accurate description of the change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[EnclaveVM.start / checkQemuAvailable] --> B[qemuBinaryForHost]
    B --> C{process.arch}
    C -->|arm64| D[qemu-system-aarch64]
    C -->|x64| E[qemu-system-x86_64]
    C -->|other| F[undefined → error / unavailable]
    D --> G[execSync which …]
    E --> G
    G -->|found| H[VM.create sandbox.qemuPath]
    G -->|not found| I[return install hint]
    H --> J[Gondolin launches QEMU]
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
packages/enclave/src/vm.ts:358
**Debian package name inconsistent with the binary**

The install hint for x86_64 suggests `qemu-system-x86`, but the binary that is actually checked and launched is `qemu-system-x86_64`. While `qemu-system-x86` is the correct Debian/Ubuntu package name (it ships `qemu-system-x86_64`), the README on line 10 also says `sudo apt install qemu-system-x86` — a user who copies that verbatim and then looks for a `qemu-system-x86` _binary_ will be confused. The hint would be clearer if it noted that the binary inside the package is `qemu-system-x86_64`.

```suggestion
			const debianPackage = process.arch === "arm64" ? "qemu-system-aarch64" : "qemu-system-x86";
			installHint = `Install with: sudo apt install ${debianPackage} (provides the ${qemuBinary} binary on Debian/Ubuntu) or sudo pacman -S qemu-full (Arch)`;
```

Reviews (2): Last reviewed commit: "Pass QEMU binary to enclave VM" | Re-trigger Greptile

Co-authored-by: GPT-5.5 <openai-codex-gpt-5.5@pi.local>
Pi-Model: openai-codex/gpt-5.5
@mgabor3141
Copy link
Copy Markdown
Owner

Thank you!

@mgabor3141 mgabor3141 merged commit fd663da into mgabor3141:main May 2, 2026
1 check passed
@ian-ross ian-ross deleted the enclave-x86-vm branch May 2, 2026 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants