Skip to content

Update benchmarks and transition to krunkit for macOS engine - #60

Merged
enegalan merged 5 commits into
mainfrom
krunkit_transition
Jul 23, 2026
Merged

Update benchmarks and transition to krunkit for macOS engine#60
enegalan merged 5 commits into
mainfrom
krunkit_transition

Conversation

@enegalan

@enegalan enegalan commented Jul 23, 2026

Copy link
Copy Markdown
Owner
  • Revised benchmarks in BENCHMARKS.md to reflect performance comparisons of Calf, Docker Desktop, and OrbStack on macOS using the new krunkit engine.
  • Updated CHANGELOG.md to document the addition of the krunkit macOS engine and changes in runtime behavior.
  • Adjusted references in documentation and code to replace vfkit with krunkit, including updates to the Makefile and various source files.
  • Enhanced the development documentation to guide users on the new krunkit stack installation and usage.

This commit marks a significant shift in the macOS engine architecture, aiming for improved performance and resource management.

Summary by CodeRabbit

  • New Features

    • Updated macOS runtime to use the krunkit engine, including improved guest networking, port forwarding, and virtiofs/DAX behavior.
    • Added macOS app bundling for the krunkit stack and gvproxy.
    • Introduced guest-disk build/download/extract workflows and new build targets (krunkit-stack, guest-disk).
  • Bug Fixes

    • Improved concurrent runtime startup coordination and readiness handling.
    • Updated transient-startup error handling and improved migration disk-space guidance.
    • Prevented benchmark runs from being terminated by the parent watchdog.
  • Documentation

    • Updated guides and benchmark documentation to replace vfkit with krunkit, including simplified reproduce steps.
    • Added release notes for version 0.9.8.

- Revised benchmarks in `BENCHMARKS.md` to reflect performance comparisons of Calf, Docker Desktop, and OrbStack on macOS using the new krunkit engine.
- Updated `CHANGELOG.md` to document the addition of the krunkit macOS engine and changes in runtime behavior.
- Adjusted references in documentation and code to replace vfkit with krunkit, including updates to the Makefile and various source files.
- Enhanced the development documentation to guide users on the new krunkit stack installation and usage.

This commit marks a significant shift in the macOS engine architecture, aiming for improved performance and resource management.
@enegalan enegalan linked an issue Jul 23, 2026 that may be closed by this pull request
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@enegalan, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 23 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 58f89086-9b2a-4a5a-8a52-a31e29d7df8e

📥 Commits

Reviewing files that changed from the base of the PR and between 6364ad2 and 41243e4.

📒 Files selected for processing (4)
  • .github/workflows/release.yml
  • backend/internal/runtime/krunkit_darwin.go
  • scripts/guest-image/guest-provision.yaml
  • ui/macos/Runner.xcodeproj/project.pbxproj
📝 Walkthrough

Walkthrough

This PR migrates the macOS runtime from vfkit to krunkit with gvproxy, adds guest-disk packaging and DAX tooling, refactors benchmark orchestration, updates application bundling and startup synchronization, and aligns documentation and release metadata with version 0.9.8.

Changes

Krunkit macOS runtime migration

Layer / File(s) Summary
Runtime implementation and startup coordination
backend/internal/runtime/*, backend/internal/daemon/*
Darwin runtime selection now constructs Krunkit; guest operations, disk fetching, networking, port forwarding, lifecycle control, and concurrent startup handling are updated.
Guest disk build and packaging
Makefile, scripts/guest-image/*, .github/workflows/release.yml
Guest disk paths, asset names, provisioning, packing, unpacking, release targets, and CI setup use krunkit-compatible tooling.
Application bundling and startup wiring
scripts/bundle-krunkit-macos.sh, ui/macos/..., backend/cmd/calf/*
macOS builds bundle krunkit, libkrun, firmware, and gvproxy; daemon startup uses backend runtime readiness and benchmark watchdogs are disabled.
Benchmark framework
scripts/benchmarks/*, BENCHMARKS.md
Benchmarks add warmups, repeats, medians, cache flushing, VM interleaving, krunkit lifecycle handling, and revised bind-mount and idle-memory metrics.
Krunkit toolchain patches
scripts/spikes/*
New scripts build the DAX-enabled krunkit/libkrun stack and provide a threaded virtio-FS worker implementation.
Documentation and release metadata
.cursor/rules/*, CLAUDE.md, DEVELOPMENT.md, ROADMAP.md, CHANGELOG.md, backend/version/version.go, ui/pubspec.yaml
Repository guidance, roadmap, changelog, versions, and runtime references are updated for krunkit and guest disks.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

  • enegalan/calf#19: Related runtime command-error classification for krunkit and gvproxy.
  • enegalan/calf#51: Related runtime startup coordination changes.
  • enegalan/calf#53: Earlier vfkit runtime and guest-disk architecture replaced by this migration.

Sequence Diagram(s)

sequenceDiagram
  participant CalfApp
  participant CalfDaemon
  participant Krunkit
  participant Gvproxy
  participant DockerAPI
  CalfApp->>CalfDaemon: Start daemon
  CalfDaemon->>Krunkit: EnsureRuntimeRunning
  Krunkit->>Gvproxy: Start networking and forwarding API
  Krunkit->>DockerAPI: Boot guest and wait for readiness
  DockerAPI-->>Krunkit: API ready
  Krunkit->>Gvproxy: Synchronize published container ports
  Krunkit-->>CalfDaemon: Runtime ready
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 52.54% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: benchmark updates and the macOS engine move from vfkit to krunkit.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch krunkit_transition

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 13

🧹 Nitpick comments (5)
scripts/spikes/patches/libkrun-fs-worker.rs (1)

234-244: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Return epoll registration failures from queue_loop.

The ctl(Add, ...) results for both queue_evt and stop_fd are discarded, so an epoll_wait failure can only occur while QUEUE_TAG is absent. That leaves the thread blocked only on stop_fd instead of processing its virtqueue. Propagate the registration error and abort the loop/setup rather than silently swallowing it.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/spikes/patches/libkrun-fs-worker.rs` around lines 234 - 244, The
queue_loop epoll setup currently discards registration failures for queue_evt
and stop_fd. Update both epoll.ctl calls to propagate errors from queue_loop and
abort setup before entering the wait loop, preserving normal registration
behavior when both succeed.
backend/internal/runtime/krunkit_darwin.go (1)

43-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add doc comments to the new exported/unexported funcs.

krunkitPidPath, gvproxyPidPath, gvproxySockPath, gvproxyAPISockPath (and krunkitAlive/gvproxyAlive at Lines 123-129) lack the required // doc comment immediately above the declaration. As per coding guidelines: "Every Go function and method must have an English // doc comment immediately above its declaration."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/internal/runtime/krunkit_darwin.go` around lines 43 - 57, Add an
English // doc comment immediately above each declaration of krunkitPidPath,
gvproxyPidPath, gvproxySockPath, gvproxyAPISockPath, krunkitAlive, and
gvproxyAlive, describing the function’s purpose without changing their behavior.

Source: Coding guidelines

scripts/guest-image/build-guest.sh (1)

13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Drop the unused ROOT_DIR assignment.

ROOT_DIR is only assigned and never referenced elsewhere in scripts/guest-image/build-guest.sh, so remove it to fix the SC2034 warning.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/guest-image/build-guest.sh` at line 13, Remove the unused ROOT_DIR
assignment from build-guest.sh, leaving SCRIPT_DIR and the remaining build logic
unchanged.

Source: Linters/SAST tools

scripts/guest-image/guest-provision.yaml (1)

108-108: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Do not downgrade every virtiofs failure to non-DAX.

The unconditional || suppresses the first mount error and falls back for unrelated failures such as a missing share or invalid mount setup. Limit fallback to a confirmed unsupported-DAX case; otherwise preserve and surface the original failure.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/guest-image/guest-provision.yaml` at line 108, Update the ExecStart
virtiofs mount command to retry without DAX only when the first mount fails
specifically because DAX is unsupported; preserve and surface the original error
for missing shares, invalid configuration, or other failures. Keep the existing
non-DAX fallback unchanged once that condition is confirmed.

Sources: Coding guidelines, Learnings

scripts/bundle-krunkit-macos.sh (1)

55-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the blanket install_name_tool error suppression.

Line 59 hides malformed-binary and rewrite failures. Inspect the current libkrun install name first, then rewrite only when needed; let an attempted rewrite failure stop bundling.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/bundle-krunkit-macos.sh` around lines 55 - 66, Update the libkrun
rewrite logic around the initial install_name_tool call to inspect the current
dependency first and invoke install_name_tool only when its path differs from
`@loader_path/`../lib/libkrun.1.dylib. Remove the blanket error suppression so any
attempted rewrite failure propagates and stops bundling, while preserving the
existing handling for absolute Homebrew-style paths.

Sources: Coding guidelines, Learnings

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/internal/daemon/runtime_ready.go`:
- Around line 17-18: Update the startup coordination around runtimeStartMu so
callers do not wait unconditionally for an active VM startup. Represent the
in-flight start with a shared completion channel and result, then select between
startup completion and the caller’s ctx.Done(), returning promptly when the
context is canceled while preserving shared startup-result handling for
concurrent callers.

In `@backend/internal/runtime/krunkit_darwin.go`:
- Around line 157-159: Update the ensureGuestDisk call in Krunkit.Start to
derive the guest-disk download context from a background or otherwise
longer-lived parent, allowing its intended 45-minute timeout to apply
independently of startCtx’s 3-minute deadline; preserve the existing startCtx
for the remaining startup flow.
- Around line 359-369: Remove the cmd.ProcessState != nil condition from the
early-exit check in the gvproxy startup loop. Use gvproxyAlive() alone to detect
that gvproxy exited before creating the sockets, while preserving the existing
error message and socket readiness checks.

In `@BENCHMARKS.md`:
- Around line 31-33: Replace the persistent sudoers installation instructions in
the benchmark setup with a safer flow: authenticate once using sudo -v before
running the benchmark, or create a narrowly scoped temporary rule that is
validated with visudo and removed immediately afterward. Do not leave any
passwordless sudo configuration in /etc/sudoers.d/.

In `@CHANGELOG.md`:
- Around line 12-18: Rewrite the v0.9.8 changelog entry in user-facing terms,
removing implementation details such as component/library names, source
filenames, build commands, filesystem paths, and protocol jargon. Describe only
the observable macOS improvements: faster startup and file operations, reliable
guest disk persistence, and container port networking through the host.

In `@DEVELOPMENT.md`:
- Around line 67-77: Remove the “Docker attach/stdout can be empty” limitation
from the migration documentation and fix the underlying stream-forwarding path
used by Docker attach and stdout over vsock. Add an end-to-end regression test
covering non-empty container output; if the operation cannot be supported, make
it return a precise unsupported-operation error rather than an empty successful
response.

In `@ROADMAP.md`:
- Around line 176-178: Update the Phase 4 roadmap entries for warm starts, cold
starts, and image/layer caching to remove Lima-specific wording and describe
current engine-neutral behavior, or explicitly label them as historical. Keep
the documented performance and persistence criteria accurate for the current
krunkit-only macOS runtime.
- Around line 225-226: Update the ROADMAP.md success metrics to match the
corresponding cold-start and idle-RAM values documented in BENCHMARKS.md: 8.0 s
for cold start to the first container and 0.07 GB idle RAM. If the roadmap
metrics intentionally use a different measurement scope, rename or clarify the
metric labels so the scopes are distinct rather than contradictory.

In `@scripts/benchmarks/_common.sh`:
- Around line 525-537: Make scripts/benchmarks/_common.sh and
scripts/benchmarks/run-all.sh safe under set -u when runtime_env is empty by
replacing its direct array expansion with the compatible empty-array-safe form,
or enforce Bash 4.4+ before execution. Apply the fix to every runtime_env
expansion in the benchmark startup paths, preserving behavior when the array
contains environment entries.

In `@scripts/benchmarks/run-all.sh`:
- Around line 215-244: Replace the side-effecting should_skip_product checks in
resume_all_vms and pause_other_vms with the pure product_installed check, while
preserving the existing skip behavior for unavailable products and the
keep-product handling.

In `@scripts/guest-image/pack-guest-disk.sh`:
- Around line 41-49: Align the EFI asset naming between the release output in
pack-guest-disk.sh and the lookup performed by unpack-guest-disk.sh. Ensure the
emitted EFI file is discoverable by the unpack script, either by using its
expected adjacent filename or by updating the unpack logic to resolve the
architecture-specific name while preserving EFI restoration.

In `@scripts/spikes/install-krunkit-stack.sh`:
- Around line 115-137: Enforce exact substitution counts for both patches in the
read/writeback patching block: assert that the read-path target replacement
occurs exactly once when the marker is absent, and assert that n3 equals one
after the writeback substitution. Raise an error on any mismatch before writing
the patched file or reporting success, matching the failure behavior of sibling
patches.

In `@ui/macos/Runner.xcodeproj/project.pbxproj`:
- Line 341: Update the Krunkit bundling block in the Xcode build shell script to
propagate failures from bundle-krunkit-macos.sh instead of swallowing them with
“|| echo”. Keep the existing executable check, but make a distributable build
fail when the bundling script runs unsuccessfully or required runtime resources
are unavailable.

---

Nitpick comments:
In `@backend/internal/runtime/krunkit_darwin.go`:
- Around line 43-57: Add an English // doc comment immediately above each
declaration of krunkitPidPath, gvproxyPidPath, gvproxySockPath,
gvproxyAPISockPath, krunkitAlive, and gvproxyAlive, describing the function’s
purpose without changing their behavior.

In `@scripts/bundle-krunkit-macos.sh`:
- Around line 55-66: Update the libkrun rewrite logic around the initial
install_name_tool call to inspect the current dependency first and invoke
install_name_tool only when its path differs from
`@loader_path/`../lib/libkrun.1.dylib. Remove the blanket error suppression so any
attempted rewrite failure propagates and stops bundling, while preserving the
existing handling for absolute Homebrew-style paths.

In `@scripts/guest-image/build-guest.sh`:
- Line 13: Remove the unused ROOT_DIR assignment from build-guest.sh, leaving
SCRIPT_DIR and the remaining build logic unchanged.

In `@scripts/guest-image/guest-provision.yaml`:
- Line 108: Update the ExecStart virtiofs mount command to retry without DAX
only when the first mount fails specifically because DAX is unsupported;
preserve and surface the original error for missing shares, invalid
configuration, or other failures. Keep the existing non-DAX fallback unchanged
once that condition is confirmed.

In `@scripts/spikes/patches/libkrun-fs-worker.rs`:
- Around line 234-244: The queue_loop epoll setup currently discards
registration failures for queue_evt and stop_fd. Update both epoll.ctl calls to
propagate errors from queue_loop and abort setup before entering the wait loop,
preserving normal registration behavior when both succeed.
🪄 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 Plus

Run ID: d2fb80a6-7794-40a8-a939-c5a69b1b4a5f

📥 Commits

Reviewing files that changed from the base of the PR and between 7e8a511 and bb6e3c9.

📒 Files selected for processing (39)
  • .cursor/rules/calf.mdc
  • .github/workflows/release.yml
  • BENCHMARKS.md
  • CHANGELOG.md
  • CLAUDE.md
  • DEVELOPMENT.md
  • Makefile
  • ROADMAP.md
  • backend/cmd/calf/main.go
  • backend/cmd/calf/watchdog_unix.go
  • backend/cmd/calf/watchdog_windows.go
  • backend/internal/constants/constants.go
  • backend/internal/daemon/core.go
  • backend/internal/daemon/migrate.go
  • backend/internal/daemon/runtime_ready.go
  • backend/internal/migration/disk.go
  • backend/internal/runtime/command_error.go
  • backend/internal/runtime/guest_darwin.go
  • backend/internal/runtime/guest_disk_fetch_darwin.go
  • backend/internal/runtime/krunkit_darwin.go
  • backend/internal/runtime/select_darwin.go
  • backend/internal/runtime/select_other.go
  • backend/test/runtime/guest_disk_zstd_test.go
  • backend/version/version.go
  • docs/phase5-race.md
  • scripts/benchmarks/_common.sh
  • scripts/benchmarks/measure-product.sh
  • scripts/benchmarks/run-all.sh
  • scripts/bundle-krunkit-macos.sh
  • scripts/guest-image/build-guest.sh
  • scripts/guest-image/guest-provision.yaml
  • scripts/guest-image/pack-guest-disk.sh
  • scripts/guest-image/prepare-guest-disk.sh
  • scripts/guest-image/unpack-guest-disk.sh
  • scripts/spikes/build-krunkit-dax.sh
  • scripts/spikes/install-krunkit-stack.sh
  • scripts/spikes/patches/libkrun-fs-worker.rs
  • ui/macos/Runner.xcodeproj/project.pbxproj
  • ui/pubspec.yaml
💤 Files with no reviewable changes (1)
  • docs/phase5-race.md

Comment thread backend/internal/daemon/runtime_ready.go Outdated
Comment thread backend/internal/runtime/krunkit_darwin.go Outdated
Comment thread backend/internal/runtime/krunkit_darwin.go
Comment thread BENCHMARKS.md Outdated
Comment thread CHANGELOG.md Outdated
Comment thread scripts/benchmarks/_common.sh
Comment thread scripts/benchmarks/run-all.sh
Comment on lines +41 to +49
RELEASE_DISK="${DIST_DIR}/calf-guest-disk-${ARCH}.raw.zst"
cp "$OUT" "$RELEASE_DISK"
echo "release asset: $RELEASE_DISK"
if [[ -n "$EFI_OUT" ]]; then
RELEASE_EFI="${DIST_DIR}/calf-vfkit-efi-${ARCH}.zst"
RELEASE_EFI="${DIST_DIR}/calf-guest-efi-${ARCH}.zst"
cp "$EFI_OUT" "$RELEASE_EFI"
echo "release asset: $RELEASE_EFI"
fi
echo "users extract with scripts/guest-image/unpack-vfkit-disk.sh or Calf first-run download"
echo "users extract with scripts/guest-image/unpack-guest-disk.sh or Calf first-run download"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make the release EFI asset discoverable by the unpack script.

unpack-guest-disk.sh looks for efi-store.zst, but this emits calf-guest-efi-${ARCH}.zst. Users following Line 49 after downloading release assets therefore skip EFI restoration. Either emit the expected adjacent name or update unpacking to derive the architecture-specific EFI filename.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/guest-image/pack-guest-disk.sh` around lines 41 - 49, Align the EFI
asset naming between the release output in pack-guest-disk.sh and the lookup
performed by unpack-guest-disk.sh. Ensure the emitted EFI file is discoverable
by the unpack script, either by using its expected adjacent filename or by
updating the unpack logic to resolve the architecture-specific name while
preserving EFI restoration.

Comment thread scripts/spikes/install-krunkit-stack.sh
Comment thread ui/macos/Runner.xcodeproj/project.pbxproj Outdated
- Updated `BENCHMARKS.md` to clarify the requirements for passwordless purge and provided alternative methods for running benchmarks.
- Revised `CHANGELOG.md` to reflect improvements in macOS engine performance and changes in guest disk persistence.
- Improved runtime management in the daemon by implementing a shared completion mechanism for starting the runtime, allowing concurrent calls to return promptly.
- Added new PID file management functions for `krunkit` and `gvproxy` to enhance process monitoring.
- Updated various scripts to improve installation and runtime behavior, including adjustments to guest image provisioning and benchmark execution.

These changes aim to streamline the user experience and improve performance metrics for the macOS engine.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/guest-image/guest-provision.yaml`:
- Around line 97-114: Indent the entire heredoc content for calf-mount-virtiofs,
including the shebang, shell commands, case block, EOF marker, and chmod
context, to the same YAML indentation level required under script: | so
guest-provision.yaml parses and provisions correctly.
🪄 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 Plus

Run ID: 85eb3435-4d84-4477-81c7-fbf5dce1dea7

📥 Commits

Reviewing files that changed from the base of the PR and between bb6e3c9 and 6364ad2.

📒 Files selected for processing (15)
  • BENCHMARKS.md
  • CHANGELOG.md
  • ROADMAP.md
  • backend/internal/daemon/core.go
  • backend/internal/daemon/runtime_ready.go
  • backend/internal/runtime/krunkit_darwin.go
  • scripts/benchmarks/_common.sh
  • scripts/benchmarks/run-all.sh
  • scripts/bundle-krunkit-macos.sh
  • scripts/guest-image/build-guest.sh
  • scripts/guest-image/guest-provision.yaml
  • scripts/guest-image/unpack-guest-disk.sh
  • scripts/spikes/install-krunkit-stack.sh
  • scripts/spikes/patches/libkrun-fs-worker.rs
  • ui/macos/Runner.xcodeproj/project.pbxproj
💤 Files with no reviewable changes (1)
  • scripts/guest-image/build-guest.sh
🚧 Files skipped from review as they are similar to previous changes (11)
  • ui/macos/Runner.xcodeproj/project.pbxproj
  • scripts/guest-image/unpack-guest-disk.sh
  • CHANGELOG.md
  • scripts/spikes/patches/libkrun-fs-worker.rs
  • scripts/bundle-krunkit-macos.sh
  • backend/internal/runtime/krunkit_darwin.go
  • BENCHMARKS.md
  • scripts/spikes/install-krunkit-stack.sh
  • ROADMAP.md
  • scripts/benchmarks/_common.sh
  • scripts/benchmarks/run-all.sh

Comment on lines +97 to +114
cat > /usr/local/sbin/calf-mount-virtiofs <<'EOF'
#!/bin/sh
set -e
mkdir -p /mnt/calf
if err=$(mount -t virtiofs -o dax=always,noatime calf-mounts /mnt/calf 2>&1); then
exit 0
fi
case "$err" in
*dax*|*Invalid*argument*|*invalid*argument*|*Operation*not*supported*|*operation*not*supported*)
exec mount -t virtiofs -o noatime calf-mounts /mnt/calf
;;
*)
echo "$err" >&2
exit 1
;;
esac
EOF
chmod +x /usr/local/sbin/calf-mount-virtiofs

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Indent the heredoc inside the YAML block scalar.

Lines 98-113 are outside script: |, so this file cannot be parsed or provisioned. Indent every heredoc line to the same YAML level as line 97.

Proposed fix
       cat > /usr/local/sbin/calf-mount-virtiofs <<'EOF'
-#!/bin/sh
-set -e
-mkdir -p /mnt/calf
-if err=$(mount -t virtiofs -o dax=always,noatime calf-mounts /mnt/calf 2>&1); then
-  exit 0
-fi
-case "$err" in
-  *dax*|*Invalid*argument*|*invalid*argument*|*Operation*not*supported*|*operation*not*supported*)
-    exec mount -t virtiofs -o noatime calf-mounts /mnt/calf
-    ;;
-  *)
-    echo "$err" >&2
-    exit 1
-    ;;
-esac
-EOF
+      #!/bin/sh
+      set -e
+      mkdir -p /mnt/calf
+      if err=$(mount -t virtiofs -o dax=always,noatime calf-mounts /mnt/calf 2>&1); then
+        exit 0
+      fi
+      case "$err" in
+        *dax*|*Invalid*argument*|*invalid*argument*|*Operation*not*supported*|*operation*not*supported*)
+          exec mount -t virtiofs -o noatime calf-mounts /mnt/calf
+          ;;
+        *)
+          echo "$err" >&2
+          exit 1
+          ;;
+      esac
+      EOF
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
cat > /usr/local/sbin/calf-mount-virtiofs <<'EOF'
#!/bin/sh
set -e
mkdir -p /mnt/calf
if err=$(mount -t virtiofs -o dax=always,noatime calf-mounts /mnt/calf 2>&1); then
exit 0
fi
case "$err" in
*dax*|*Invalid*argument*|*invalid*argument*|*Operation*not*supported*|*operation*not*supported*)
exec mount -t virtiofs -o noatime calf-mounts /mnt/calf
;;
*)
echo "$err" >&2
exit 1
;;
esac
EOF
chmod +x /usr/local/sbin/calf-mount-virtiofs
cat > /usr/local/sbin/calf-mount-virtiofs <<'EOF'
#!/bin/sh
set -e
mkdir -p /mnt/calf
if err=$(mount -t virtiofs -o dax=always,noatime calf-mounts /mnt/calf 2>&1); then
exit 0
fi
case "$err" in
*dax*|*Invalid*argument*|*invalid*argument*|*Operation*not*supported*|*operation*not*supported*)
exec mount -t virtiofs -o noatime calf-mounts /mnt/calf
;;
*)
echo "$err" >&2
exit 1
;;
esac
EOF
chmod +x /usr/local/sbin/calf-mount-virtiofs
🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 100-100: syntax error: could not find expected ':'

(syntax)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/guest-image/guest-provision.yaml` around lines 97 - 114, Indent the
entire heredoc content for calf-mount-virtiofs, including the shebang, shell
commands, case block, EOF marker, and chmod context, to the same YAML
indentation level required under script: | so guest-provision.yaml parses and
provisions correctly.

Source: Linters/SAST tools

enegalan added 3 commits July 23, 2026 15:21
- Simplified the error handling logic in the `startGvproxy` function by removing unnecessary checks for the process state.
- Enhanced clarity in the error message when `gvproxy` exits before creating the expected socket file.

These changes improve the robustness of the process monitoring in the macOS engine.
- Added an environment variable `CALF_REQUIRE_KRUNKIT` to the release workflow to ensure that the macOS build fails if krunkit bundling is unsuccessful.
- Updated the Xcode project shell script to include error handling for the krunkit bundling process, providing a warning if the bundling fails and enforcing a failure in CI builds when required.

These changes improve the reliability of the macOS build process by ensuring necessary components are bundled correctly.
- Reformatted the shell script in `guest-provision.yaml` to enhance clarity and maintainability.
- Adjusted indentation and spacing for better structure without altering functionality.

These changes aim to streamline the script for easier understanding and future modifications.
@enegalan
enegalan merged commit 8c808b4 into main Jul 23, 2026
3 checks passed
@enegalan
enegalan deleted the krunkit_transition branch July 23, 2026 13:58
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.

Transition to krunkit in MacOS

1 participant