-
Notifications
You must be signed in to change notification settings - Fork 1
refactor: cross-platform foundation for macOS support #89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Split platform-specific code into _linux.go and _darwin.go files across resources, network, devices, ingress, vmm, and vm_metrics packages. Add hypervisor abstraction with registration pattern (RegisterSocketName, RegisterVsockDialerFactory, RegisterClientFactory) to decouple instance management from specific hypervisor implementations. Add "vz" to the OpenAPI hypervisor type enum, erofs disk format support, and insecure registry option for builds. No behavioral changes on Linux. macOS can now compile but has no VM functionality yet. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
✱ Stainless preview buildsThis PR will update the
|
This comment has been minimized.
This comment has been minimized.
- Restore persisting result.Logs to disk after build completion, since streamed log lines can be dropped when the bounded channel overflows - Add docker tag + push after building the builder image locally so it is available in the registry for builder VMs to pull - Synchronize log streaming with build result delivery by waiting for logsDone channel before sending build_result, preventing the host from closing the connection before all logs are delivered Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
sjmiller609
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, maybe some opportunity for deduplication
| func detectCPUCapacity() (int64, error) { | ||
| return int64(runtime.NumCPU()), nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks simpler than the linux one, idk if we could do the same there or not
lib/resources/disk_darwin.go
Outdated
| // Auto-detect from filesystem using statfs | ||
| var stat unix.Statfs_t | ||
| dataDir := cfg.DataDir | ||
| if err := unix.Statfs(dataDir, &stat); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the implementations for darwin and linux look almost identical here
This comment has been minimized.
This comment has been minimized.
Rework builder image provisioning: - Embed generic/Dockerfile with go:embed instead of reading from filesystem - Remove hardcoded "hypeman/builder:latest" default; if BUILDER_IMAGE is unset, build from embedded Dockerfile (dev mode) - After docker build+save, write image directly into OCI layout cache and call ImportLocalImage, bypassing docker push entirely - Move RecoverPendingBuilds after ensureBuilderImage to prevent race where recovered builds fail with "builder image is being prepared" Also fix review feedback: - Add sector alignment to erofs disk conversion (macOS VF compat) - Remove redundant ForLinux OCI client aliases and update callers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Addressed the remaining review feedback in 3658218: Cursorbot bugs (all fixed):
Steven's observations:
Additionally, this commit includes the builder image rework:
|
This comment has been minimized.
This comment has been minimized.
Add three tests verifying the builder image import pipeline: - TestDockerSaveTarballToOCILayoutRoundtrip: full pipeline from docker save tarball → load → OCI layout → existsInLayout → extractMetadata → unpackLayers with rootfs verification - TestDockerSaveToOCILayoutCacheHit: verifies pullAndExport skips remote pull when image exists in OCI layout cache (uses bogus registry URL that would fail if pull was attempted) - TestImportLocalImageFromOCICache: end-to-end integration test simulating buildBuilderFromDockerfile's flow: write to OCI cache → ImportLocalImage → async build → verify GetImage metadata and GetDiskPath returns valid ext4 disk Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON. A Cloud Agent has been kicked off to fix the reported issue.
|
Bugbot Autofix prepared fixes for 1 of the 1 bugs found in the latest run.
Or push these changes by commenting: Preview (262b30fcb7)diff --git a/lib/providers/providers.go b/lib/providers/providers.go
--- a/lib/providers/providers.go
+++ b/lib/providers/providers.go
@@ -259,6 +259,7 @@
RegistryCACert: registryCACert,
DefaultTimeout: cfg.BuildTimeout,
RegistrySecret: cfg.JwtSecret, // Use same secret for registry tokens
+ DockerSocket: cfg.DockerSocket,
}
// Apply defaults if not set |
Merge disk_darwin.go and disk_linux.go into disk.go since both implementations are nearly identical — the only difference was syscall.Statfs (linux) vs unix.Statfs (darwin). Use unix.Statfs from golang.org/x/sys/unix which works on both platforms. Addresses review feedback from @sjmiller. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
disk_darwin.go and disk_linux.go were unified into disk.go in PR #89 but snuck back in during the rebase as new files with no conflicts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Summary
_linux.goand_darwin.gofiles acrossresources,network,devices,ingress,vmm, andvm_metricspackagesRegisterSocketName,RegisterVsockDialerFactory,RegisterClientFactory) to decouple instance management from specific hypervisor implementations"vz"to the OpenAPI hypervisor type enum, erofs disk format support, and insecure registry option for buildsNo behavioral changes on Linux. macOS can now compile but has no VM functionality yet.
This is part 1 of 2 for macOS/Virtualization.framework support. Part 2 (
feat/vz-hypervisor) adds the actual vz hypervisor implementation.Test plan
make build-linuxsucceedsmake test-linuxpasses (failures are Docker Hub rate limiting, not code changes)test-darwin)🤖 Generated with Claude Code
Note
Medium Risk
Touches VM lifecycle/build pipeline (vsock connection path, builder image preparation, log persistence) and image/disk creation logic, which could affect build reliability and instance startup even on Linux despite being largely additive/guarded.
Overview
Establishes a cross-platform foundation by splitting multiple subsystems into Linux vs macOS implementations (networking, devices/GPU passthrough, resources, VM metrics, VMM/ingress binaries), adding macOS stubs/no-ops where features aren’t supported yet, and updating the API hypervisor enum to include
vz.Refactors hypervisor integration to be registration/factory-based (
RegisterClientFactory, platform-specific starters, per-instanceGetVsockDialer) and adds platform-specific hypervisor access checks (KVM on Linux vs Virtualization.framework on Apple Silicon).Improves the build system: builder-agent now streams logs over vsock while retaining an authoritative buffered log; build manager can bootstrap a builder image locally from an embedded Dockerfile via Docker + OCI cache import when
BUILDER_IMAGEis unset, gates build execution until the builder image is ready, and switches to per-instance vsock dialing. Image handling is adjusted to always target Linux VM platform for pulls/manifests and disk image creation is made 4K-sector aligned for Virtualization.framework compatibility.Written by Cursor Bugbot for commit 964eacb. This will update automatically on new commits. Configure here.