fix(cua-sandbox): boot Windows Fleet sandboxes with UEFI firmware - #3125
Merged
Conversation
The Fleet cloud transport never set `firmware` on the VM template, so it inherited the Fleet schema default of BIOS (`default_firmware() -> Some(Firmware::Bios)`, libs/fleet/sdk-schema/src/common.rs:70-72). The Windows containerDisk is GPT/UEFI-only, so SeaBIOS has nothing to chainload: the VM boots, prints "Missing operating system", and never opens port 8000, leaving the readiness probe failing forever and warm pools stuck at readyReplicas: 0. Set EFI for Windows guests, keying off the same `os_type == "windows"` signal the local QEMU runtime already uses (runtime/qemu.py:300). Linux guests keep the omitted field and the server-side BIOS default, which is what the working Linux pools already run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This was referenced Aug 13, 2026
r33drichards
added a commit
that referenced
this pull request
Aug 13, 2026
from_registry hardcoded os_type="linux" with no way to override it, while from_file — the adjacent constructor building the same object from a different source — takes os_type and kind as keyword arguments. os_type is what selects UEFI, on the local QEMU runtime and in the Fleet transport alike. A Windows containerDisk pulled from a registry was therefore handed BIOS and could not boot, and the only escape was reaching past the constructor with dataclasses.replace(). That is the firmware failure fixed for Fleet in #3125 arriving through a different door. The default stays "linux", so existing callers are unaffected. Also repairs a test double in test_cloud.py: #3133 made Sandbox._create pass ephemeral= to runtime.start(), and the fake Runtime there does not accept it, so that test has been failing on main since it merged. It went unnoticed because cua-sandbox is not in the ci-test-python package matrix and its suite never runs on pull requests. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Why
Image.windows()sandboxes on Fleet cloud never become ready. The VM starts, the guest never boots, port 8000 never opens, the readiness probe never passes, and the warm pool sits atreadyReplicas: 0until claims fail withBindDeadlineExceeded.The Windows containerDisk is GPT/UEFI-only. I read the guest's own partition table straight out of the published layer (
public.ecr.aws/k5j5w0x5/cua-windows-2022:main-bac7daa3, range-GET → gunzip →qemu-ioon the qcow2):0xEE: a GPT protective MBR. Its bootstrap is the stock "Invalid partition table / Missing operating system" stub.EFI PARTsignature, valid GPT header.c12a7328-f81f-11d2-ba4b-00a0c93ec93b"EFI system partition", then "Microsoft reserved partition", then "Basic data partition".No active MBR partition and no BIOS boot partition. There is nothing for SeaBIOS to chainload.
And SeaBIOS is exactly what it got. From the
virt-launcherlog of a live failing VM, the generated domain XML has an empty<os>block with no<loader>/<nvram>, and the qemu command line has no-drive if=pflash— no OVMF anywhere. The VMI reports:Running, but nothing listening.
_template_request()never called.firmware(...), so the template inherited the Fleet schema default:Verified end to end, not just asserted
Given that the whole bug is "we asserted the artifact instead of booting it", asserting the artifact again would be a poor fix. So I booted it — a throwaway pool on
run.cua.aibuilt from this patch's own output, same image, same defaults (4 cpu / 4Gi),efithe only difference from the failing pool:Ready in 123 seconds. The
biospool with the same image had been Running-but-never-Ready for 22 minutes at that point. Test namespaces were deleted and confirmed gone.Unit tests:
37 passed, including two new ones —test_windows_image_boots_uefi(asserts the Windows case specifically) andtest_linux_image_leaves_firmware_at_the_schema_default(pins the no-change-for-Linux behaviour).This is not a regression from #3091 / #3114 / #3118
Please don't go hunting today's commits. This code path has never been able to boot a Windows image on Fleet.
git log -S"firmware"onfleet_cloud.pyreturns exactly two commits, and neither ever set EFI:firmware=Noneexplicitly.bios.It only became reachable now. Before #3118,
Image.windows()defaulted to version"11", which has no entry inBUILTIN_REGISTRY_IMAGES, socloud_registry_image()returnedNoneand the transport rejected it early — Windows-on-Fleet never got as far as the firmware question. #3118 flipped the default to"2022"and pinned a real, pullable Windows containerDisk, which walked the path straight into a latent BIOS default that had been sitting there since July.The lesson worth keeping
The existing Fleet unit tests passed the entire time while shipping a template that cannot boot. They assert the template we build — image reference, service ports — and a template asserting only "we set the fields we set" will happily describe an unbootable VM. #3118 added
tests/test_uefi_firmware.py(75 lines) for the local path in the very same commit that made the cloud path reachable with no firmware handling at all. The local runtime has had this guard for ages:and
registry/qemu_builder.py:354outright refuses to build a Windows VM without OVMF. The local path had the rule; the cloud path had nothing.The tests that would have caught it exist —
tests/integration/sandbox_sdk/test_windows_cloud_vm.pyand itssandbox_clitwin, both touched by #3118 — but they are gatedskipif(not os.environ.get("CUA_API_KEY")), so they skip in CI. That is the concrete mechanism behind that PR's "covered by unit tests against the typed template request, not by a live cloud call". Same local/cloud parity gap that motivated #3091 and #3118 — this time it bit the code those PRs shipped.The fix
Derived from the image rather than hardcoded by OS name, using the identical predicate the local runtime uses. Linux keeps the field omitted and the server-side BIOS default, matching the Linux pools that already run fine.
Known gaps deliberately left open
Image.from_registry(...)still gets BIOS.image.py:179hardcodesos_type="linux", so a private Windows containerDisk reached that way — e.g. thecua-server-windowsreferences used by the existinghermes-cua-poolandwindows-fleettemplates — still gets BIOS and still won't boot. Closing this properly needs afirmware=passthrough onFleetCloudTransportplumbed throughSandbox.create, which widens the public API surface. Left out of this PR on purpose; flagging it so someone can pick it up deliberately.cpu/memory defaults left alone. The broken pool ran 4 cpu / 4Gi (the Fleet schema defaults from
default_cpu_cores()/default_memory()) where the older hand-authored Windows pools use 10 cpu / 20Gi, which looks like a suspect. It isn't: the hand-authoredwindows-fleet-templatealso uses 4 cpu / 4Gi — withefi— and the verification boot above reached Ready on 4/4Gi. Firmware is the only differentiator between templates that boot and templates that don't, so no defaults are changed here..image_pull_secret("ecr-credentials")is now dead weight, not load-bearing — the public ECR pull succeeded with it attached on every VM in this investigation, including the one that booted. It was my leading suspect for this outage and it was wrong. Not removed here, because it turns out to have a real side effect worth deciding on separately:libs/fleet/backend/auth/pool_admission.regoonly enforces its image allowlist whenimagePullSecret == "ecr-credentials", and a template with no pull secret is allowed any image. Setting it unconditionally is therefore what makes non-allowlisted registries — e.g.ghcr.io/trycua/mini-swe:latest, whichlibs/python/cua-cli/README.mddocuments in a GitHub Actions example — fail with403 k8s request is not allowed. That deserves its own change, not a ride-along on a firmware fix.Not in scope, filed separately
Two other things surfaced while diagnosing this and are not addressed here:
OSGymSandboxWarmPoolstatus goes stale — a fully healthy pool can reportreadyReplicas: 0indefinitely. This misdirected the entire investigation into a false cluster-wide outage theory. Written up on its own.time_to_start, a 300s bind deadline) dies mid-flight with401 auth token is invalid. The stored credential already carries arefresh_token; nothing consumes it.🤖 Generated with Claude Code