feat(runner): microVM boot canary startup preflight (RIG-3148) - #847
Merged
Conversation
|
😎 Merged successfully - details. |
|
Compass engineering docs preview: https://compass-runner-rig-3148-boot.compass-eng-docs.pages.dev Deployed from |
Add the dynamic half of the microVM backend's startup preflight: a boot
canary that really boots a throwaway VM through the backend's own verbs
(Create->Start->Exec->Remove), proving the whole chain -- KVM, vsock,
image, guest supervisor, exec gate -- not just binary presence.
- `CanaryReport{BootLatency, GuestRSSBytes}` and
`(*MicroVMRuntime).BootCanary(ctx)` in microvm_preflight.go. The canary
owns the VM's whole lifetime inside the call, so it derives a bounded
ctx (caller deadline as-is when present, else a 90s bound) and severs
teardown from it so a timed-out boot still tears down. Remove and the
throwaway-workspace cleanup are joined into the return, never discarded.
Boot latency is the Start wall time; guest PSS is best-effort telemetry
(a read error leaves it at 0, logged, never fatal -- the gate is the
boot chain).
- Extend the runner startup gate: a `canaryBooter` probe interface
(distinct single-method probe, not a widened microVMPreflighter) and a
`runMicroVMPreflight` helper that runs VerifyMicroVMSupport, then -- only
once it passes -- BootCanary, logging the report. A microVM engine that
cannot canary is a fail-closed startup error naming the type, never a
silent skip.
- Hermetic seam-faked suite over the launch/client seams (no KVM), plus a
KVM-gated TestBootCanary riding the existing microVM CI leg. A one-line
cross-reference distinguishes the no-boot enablement smoke test from
this real boot canary.
Refs RIG-3148
Co-authored-by: Matt Wilkinson <matt@rigel.build>
… discipline (RIG-3148) Additive review-fix commit on the W3 boot-canary PR, closing the one gating finding plus three lows from the review round: - medium (test-adequacy): the PSS-read-error fail-open branch — the one fail-open seam on an otherwise fail-closed startup gate (record §(e)/OQ-10) — had no hermetic coverage, so a regression flipping it to fatal (spurious Runner-startup refusal on a host with an unreadable smaps_rollup) would pass every test. Added a `pssErr` knob to canaryFakeVM (threaded through the launch recorder) and TestBootCanaryPSSErrorNonFatal, which pins that a PSS read error leaves BootCanary succeeding with GuestRSSBytes == 0 and the session torn down. Mutation-verified RED on flip-to-fatal. - low (lock discipline): BootCanary read `session.vm` outside `m.mu`, the one production reader deviating from the microvmSession invariant. Now reads the handle under the lock, matching Stop (microvm_lifecycle.go). - low (doc): tightened the canaryTeardownGrace comment to state it bounds Remove only if/when Remove honors its ctx deadline — today Remove is deadline-agnostic — so the constant no longer implies an enforced ceiling it lacks. - low (test symmetry): TestBootCanaryNonZeroExitFails now carries the temp-leak and wasShutdown assertions its sibling failure-path tests carry. No behavior change on the happy path; the under-lock read is a visibility/ invariant tightening with no functional effect (single-owner canary session). Refs RIG-3148 Co-authored-by: Matt Wilkinson <matt@rigel.build>
…RIG-3148) Round-r1 review fold for the V5 W3 boot canary. All four findings are test-only; `microvm_preflight.go` is unchanged. ### Nonce round-trip (was the gating medium) The nonce-mismatch branch — exec returns exit 0 but stdout does not carry the minted nonce — was uncovered: `canaryFakeClient.Exec` always echoes the command back, so the fake structurally could not produce exit-0-with-wrong-stdout. That branch is the "exec gate" leg of the whole-chain claim (record §(e)): exit 0 alone only proves a call returned; the nonce is the sole assertion the guest ran our command and returned our data. Added a `stdout *string` override knob (mirroring the r0 `pssErr` knob) and `TestBootCanaryNonceMismatchFails`. Mutation-verified: disabling the nonce check goes RED, restored GREEN. ### Deadline guard (was low, tautological test) `TestBootCanaryHonorsCallerDeadline` could not fail on the bug it named — a 5s caller bound is clamped by `context.WithTimeout` regardless of the `if _, ok := ctx.Deadline(); !ok` guard, so a re-deriving implementation passes it. Added `TestBootCanaryHonorsLongerCallerDeadline` with a 10-minute caller deadline (the only case the guard actually protects: a longer caller deadline must pass through, not be clamped to the 90s canary bound). Mutation-verified: removing the guard goes RED for the new test while the old 5s test stays GREEN. ### Partial PSS (was low, half-covered contract) The real `VM.PSS()` returns a partial map alongside a joined error as its normal shape; `TestBootCanaryPSSErrorNonFatal` drove `pss=nil` so the partial-data-plus-error combination was undefended. Added `TestBootCanaryPartialPSSStillReported`. Mutation-verified: discarding the partial map inside the error branch goes RED. ### Path convention (was low) `boot_canary_microvm_test.go` built `<RunRoot>/microvm` by string concatenation beside the production `filepath.Join`; switched to `filepath.Join`. Gates green: gofmt, linux + `-tags microvm` builds, hermetic suite (11 tests), golangci-lint `--build-tags microvm` (0 issues), nilaway (exit 0). The remaining r1 low (threading a signal-cancellable ctx into the startup preflight callsite) is a control-flow-ordering judgment call and is filed separately for Matt. Spec-impact: none. Refs RIG-3148 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
force-pushed
the
compass-runner/rig-3148-boot-canary
branch
from
September 3, 2026 05:19
5be9b07 to
ee6a3a6
Compare
mattwilkinsonn
approved these changes
Sep 4, 2026
Contributor
|
/trunk merge |
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
`BootCanary`'s documented contract is that the always-run Remove teardown's error is joined into its return, never discarded — it gates Runner startup, so a silently-swallowed teardown failure would report a canary that leaked a live VMM+virtiofsd as a clean boot, inverting the fail-closed posture on the exact path the gate protects. The `#847` review loop found that contract had zero regression coverage: swallowing both `errors.Join` blocks in `microvm_preflight.go` left the entire hermetic suite green. The named-return + `errors.Join` idiom is precisely what a "tidy the error path" refactor flattens to a plain `defer m.Remove(...)`, and error-swallow discipline is CI-enforced repo law, so the join needs a test that keeps it correct. `TestBootCanaryTeardownErrorJoined` drives an otherwise-successful canary (boot + echo + nonce all pass) with a guest whose `Shutdown` fails, and asserts `BootCanary` returns a non-nil error carrying the teardown failure while the report is still assembled from what ran before teardown. A `shutdownErr` field on `canaryFakeVM` (threaded through `canaryLaunchRecorder`, mirroring the existing `pssErr` knob) is returned from `Shutdown`, so `Remove`'s own `errors.Join` surfaces it into the named return. Test-only; `microvm_preflight.go` is unchanged (the production code was already correct). Mutation-verified: gating off the teardown join reddens only this test and leaves the other ten green; restored to green. Stacked on `#847` (RIG-3148) because it exercises the `BootCanary` teardown code that PR introduces. Spec-impact: none. Refs RIG-3219 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…gate (RIG-3200) Round-2 review of the bundle re-scope found the round-1 fix left two findings partially open plus a widenable low; this closes all three additively. - ci-matrix darwinAffected (HIGH): the darwin gate body compiled the three pure-Go sidecars but its affected-trigger still keyed only on go/cmd/compass-app/, the GTK closure, and tools/macos-bundle/ — so a change to a sidecar source or the shared go/internal/ tree they import never ran the lane, and the release cut was its first execution (proven by #847's darwin cross-compile break). Widen the predicate with DARWIN_SIDECAR_PREFIXES (the three sidecar cmd dirs + the go/internal/ superset, deliberately broad over a drift-prone transitive set), mirror the same paths into the ci.yml in-step git-diff list, and add two trigger unit tests. - macos-bundle collision guard (MEDIUM): assertSidecarBasenamesDistinct keyed on basename(--binary), but the shell always stages at the hardcoded literal compass-app, so a sidecar named compass-app silently clobbered the shell when --binary had a different basename (false negative) and a matching non-shell basename wrongly threw (false positive). Introduce SHELL_EXECUTABLE_NAME, key the guard on it, thread it through staging + Info.plist so the three uses cannot drift, and add two collision regression tests. - darwin bundle sanity (LOW): the mount gate asserted mere presence for all six entries while build.sh's sanity loop asserts executable + `--version` exit 0 + version-stamp substring. Split into a BINARIES list (four Contents/MacOS/* entries, -x + --version + $dev substring) and a FILES list (Info.plist + index.html, -f), preserving rc accumulation and hdiutil detach. Deferred (non-blocking, routed to Matt at the review gate): the SSOT sidecar manifest and the permissive empty-sidecars grammar are structural judgment calls, not correctness gaps. Refs RIG-3200 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…200) Close round-3 review of the darwin sidecar gate. The round-2 widening covered the three sidecar cmd dirs plus go/internal/, but two of the three bundled sidecars (compass-stack, compass-server) also compile in go/server, go/events, and go/gen on non-test paths — and go/server is //go:build unix with a divergent syscall surface (syscall.Umask, ECONNREFUSED, Stat_t). A PR touching only those roots left darwinAffected false, re-opening the #847 cross-compile-break class for the sidecars. `go list -deps ./cmd/<sidecar>` resolves the bundled binaries' non-cmd first-party roots to exactly {events, gen, internal, server} (go/e2e is tests-only). Enumerate those package roots in DARWIN_SIDECAR_PREFIXES and mirror them into the ci.yml in-step pathspec, keeping the two lists in lockstep. Add a go/server trigger unit test (mutation-verified red-green). Also fold three review nits: restore the parseArgs docstring clauses the round-2 edit dropped (duplicate-flag + missing-required-flag throw paths), refresh the two stale darwin-lane trigger comments (step + job level), and add the missing blank line between the appended ci-matrix trigger tests. Spec-impact: none Ledger-impact: none Refs RIG-3200 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
added a commit
that referenced
this pull request
Sep 4, 2026
…er (RIG-3200) Round-4 review fold on PR #863. Closes the surviving medium and 3 lows from ReviewT4r4; NEW-1/LOW-2/LOW-4 already closed in round 3. - MEDIUM: the round-3 job-level darwin comment repair left a third clause dropped, leaving an ungrammatical sentence (regression vs the prior head). Re-insert "gate guard with the darwin affected gate: on a PR it runs only when setup" so the sentence is whole. Comment-only. - LOW: two of the three widened DARWIN_SIDECAR_PREFIXES entries (go/events, go/gen) survived deletion mutation silently. Add a trigger test per root (mutation-verified: dropping either now fails exactly its test) so a future edit that drops them from the const cannot pass green. - LOW: the sibling MACOS_BUNDLE_PATH_PREFIX docstring still described only the pre-widening go/internal surface, disagreeing with the const below it. Reword to defer to DARWIN_SIDECAR_PREFIXES — one source of truth. - LOW: go/go.mod + go/go.sum did not fire the darwin lane, yet a dependency bump or replace directive is a compile input to all four bundled binaries that can break the darwin cross-compile the same #847 way. Add both to the const and mirror into the ci.yml pathspec (lockstep invariant held). Gates: bun test ci-matrix+macos-bundle 60 pass/0 fail; biome 0 errors (2 known pre-existing complexity warnings); bash -n darwin run block clean. Refs RIG-3200 Co-authored-by: Matt Wilkinson <matt@rigel.build>
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
2 participants
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.
Add the dynamic half of the microVM backend's startup preflight: a boot
canary that really boots a throwaway VM through the backend's own verbs
(Create->Start->Exec->Remove), proving the whole chain -- KVM, vsock,
image, guest supervisor, exec gate -- not just binary presence.
CanaryReport{BootLatency, GuestRSSBytes}and(*MicroVMRuntime).BootCanary(ctx)in microvm_preflight.go. The canaryowns the VM's whole lifetime inside the call, so it derives a bounded
ctx (caller deadline as-is when present, else a 90s bound) and severs
teardown from it so a timed-out boot still tears down. Remove and the
throwaway-workspace cleanup are joined into the return, never discarded.
Boot latency is the Start wall time; guest PSS is best-effort telemetry
(a read error leaves it at 0, logged, never fatal -- the gate is the
boot chain).
canaryBooterprobe interface(distinct single-method probe, not a widened microVMPreflighter) and a
runMicroVMPreflighthelper that runs VerifyMicroVMSupport, then -- onlyonce it passes -- BootCanary, logging the report. A microVM engine that
cannot canary is a fail-closed startup error naming the type, never a
silent skip.
KVM-gated TestBootCanary riding the existing microVM CI leg. A one-line
cross-reference distinguishes the no-boot enablement smoke test from
this real boot canary.
Refs RIG-3148
Co-authored-by: Matt Wilkinson matt@rigel.build