feat: rename crates as per minimal one, build new binaries to bucket - #603
Merged
Conversation
📝 WalkthroughWalkthroughThe workspace splits ChangesCLI crate split
CI, release, promote, and docs updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.github/workflows/ci.yml:
- Around line 27-28: The workflow-level permissions are too broad because
contents: write is inherited by all jobs, including PR runs. Update the ci
workflow so the top-level permissions are read-only, and keep contents: write
only on the jobs.release job by referencing the release job definition in the
workflow.
- Around line 176-195: The arm64 release lane installs cross but still uses
cargo build, so it is not actually cross-compiling for
aarch64-unknown-linux-musl. Update the Build static release binaries step in the
CI workflow to invoke cross build for the same package set, matching the amd64
lane’s cross-compilation approach. Keep the existing target and package list
intact, and adjust the job so the installed cross tool is the one used for
building.
- Around line 240-266: The release job is collecting the wrong paths: the
`actions/download-artifact@v8` step leaves each artifact in its own subdirectory
under `artifacts/`, and `Create Release` is currently passing those directories
to `gh release create` instead of the actual binaries. Update the release asset
collection in the workflow so `gh release create` receives file paths directly
from the artifact subdirectories (or flatten the tree with `merge-multiple:
true`), and fix `Generate completions` in the same job to write into the created
`artifacts/completions/...` directories so the completion files are actually
included with the release assets.
In @.github/workflows/promote.yml:
- Around line 162-198: The Promote CLI version step currently resolves
SHA/PLATFORMS but then falls through a TODO and succeeds, which can incorrectly
trigger downstream docs rebuilds without actually promoting anything. Update the
run block in the Promote CLI version job so it fails fast after SHA resolution
until the new minimal-one promotion write exists, using the existing promote
step logic and the resolved SHA output to gate later steps. Ensure the step
exits non-zero with a clear message instead of continuing past the TODO.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c2cb2f3b-9210-4081-b089-98575d658ee5
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (41)
.github/workflows/ci-macos.yml.github/workflows/ci.yml.github/workflows/promote.yml.minimal/minimal.tomlCargo.tomlcrates/minimal/Cargo.tomlcrates/minimal/src/autospawn.rscrates/minimal/src/client.rscrates/minimal/src/main.rscrates/minimal2/Cargo.tomlcrates/minimal2/src/main.rscrates/mip/Cargo.tomlcrates/mip/build.rscrates/mip/src/cmd_add.rscrates/mip/src/cmd_cache.rscrates/mip/src/cmd_check.rscrates/mip/src/cmd_dep.rscrates/mip/src/cmd_dump.rscrates/mip/src/cmd_init.rscrates/mip/src/cmd_materialize.rscrates/mip/src/cmd_patched_build.rscrates/mip/src/cmd_pkg.rscrates/mip/src/cmd_plan.rscrates/mip/src/cmd_remote_build.rscrates/mip/src/cmd_rexec.rscrates/mip/src/cmd_run.rscrates/mip/src/cmd_status.rscrates/mip/src/cmd_update.rscrates/mip/src/cmd_upload_cache.rscrates/mip/src/main.rsdocs/specs/01-spec-minvmd-host-daemon/01-spec-minvmd-host-daemon.mddocs/specs/01-spec-minvmd-host-daemon/architecture.mddocs/specs/02-spec-minvmd-linux-kvm/02-spec-minvmd-linux-kvm.mddocs/specs/03-spec-networking/03-spec-networking.mddocs/specs/03-spec-networking/architecture.mddocs/specs/03-spec-networking/test-plan.mddocs/specs/03-spec-networking/test-plan.shdocs/specs/06-spec-ssh-host-key-in-beacon/06-spec-ssh-host-key-in-beacon.mdjustfilescripts/fetch-artifact.shscripts/fetch-libkrun.sh
💤 Files with no reviewable changes (2)
- crates/minimal2/Cargo.toml
- crates/minimal2/src/main.rs
✅ Files skipped from review due to trivial changes (5)
- docs/specs/06-spec-ssh-host-key-in-beacon/06-spec-ssh-host-key-in-beacon.md
- docs/specs/03-spec-networking/test-plan.md
- docs/specs/02-spec-minvmd-linux-kvm/02-spec-minvmd-linux-kvm.md
- docs/specs/03-spec-networking/03-spec-networking.md
- docs/specs/03-spec-networking/architecture.md
🚧 Files skipped from review as they are similar to previous changes (8)
- crates/mip/Cargo.toml
- scripts/fetch-libkrun.sh
- .minimal/minimal.toml
- crates/minimal/Cargo.toml
- Cargo.toml
- scripts/fetch-artifact.sh
- crates/mip/src/main.rs
- crates/minimal/src/main.rs
Comment on lines
+162
to
+198
| - name: Promote CLI version | ||
| id: promote | ||
| env: | ||
| DRY_RUN: ${{ inputs.dry_run }} | ||
| # Pass dispatch inputs via env (NOT inline ${{ inputs.* }} in `run:`) | ||
| # to avoid shell template injection — see GitHub's hardening guide. | ||
| INPUT_SHA: ${{ inputs.sha }} | ||
| INPUT_PLATFORMS: ${{ inputs.platforms }} | ||
| run: | | ||
| # Trim leading/trailing whitespace; users often paste SHAs with stray spaces. | ||
| SHA="$(printf '%s' "$INPUT_SHA" | sed 's/^[[:space:]]*//;s/[[:space:]]*$//')" | ||
| PLATFORMS="$(printf '%s' "$INPUT_PLATFORMS" | sed 's/^[[:space:]]*//;s/[[:space:]]*$//')" | ||
|
|
||
| if [ "$PLATFORMS" = "all" ]; then | ||
| PLATFORMS="amd64-linux,arm64-linux" | ||
| fi | ||
|
|
||
| if [ -z "$SHA" ]; then | ||
| echo "No SHA provided, finding latest archive in bucket..." | ||
| LATEST=$(gcloud storage ls -l "gs://minimal-shim/archives/minimalone-*.tar.zst" \ | ||
| | grep -v "^TOTAL:" \ | ||
| | sort -k2 \ | ||
| | tail -1 \ | ||
| | awk '{print $NF}') | ||
| if [ -z "$LATEST" ]; then | ||
| echo "ERROR: No archive found in bucket" | ||
| exit 1 | ||
| fi | ||
| # Extract SHA: archives/minimalone-<sha>.tar.zst → <sha> | ||
| SHA="$(basename "$LATEST" | sed 's/^minimalone-//;s/\.tar\.zst$//')" | ||
| echo "Latest SHA: ${SHA}" | ||
| fi | ||
|
|
||
| # TODO: implement minimal-one-specific promotion process!! | ||
|
|
||
| # Export resolved SHA for the docs-rebuild dispatch step below. | ||
| echo "sha=${SHA}" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail fast here until the new promotion write exists.
After resolving SHA, this step only hits a TODO and exits successfully. An approved run can now report success and trigger docs rebuilds without changing the promoted CLI for any platform.
Minimal safe guard
- # TODO: implement minimal-one-specific promotion process!!
-
- # Export resolved SHA for the docs-rebuild dispatch step below.
- echo "sha=${SHA}" >> "$GITHUB_OUTPUT"
+ echo "minimalone promotion is not implemented yet" >&2
+ exit 1📝 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
| - name: Promote CLI version | |
| id: promote | |
| env: | |
| DRY_RUN: ${{ inputs.dry_run }} | |
| # Pass dispatch inputs via env (NOT inline ${{ inputs.* }} in `run:`) | |
| # to avoid shell template injection — see GitHub's hardening guide. | |
| INPUT_SHA: ${{ inputs.sha }} | |
| INPUT_PLATFORMS: ${{ inputs.platforms }} | |
| run: | | |
| # Trim leading/trailing whitespace; users often paste SHAs with stray spaces. | |
| SHA="$(printf '%s' "$INPUT_SHA" | sed 's/^[[:space:]]*//;s/[[:space:]]*$//')" | |
| PLATFORMS="$(printf '%s' "$INPUT_PLATFORMS" | sed 's/^[[:space:]]*//;s/[[:space:]]*$//')" | |
| if [ "$PLATFORMS" = "all" ]; then | |
| PLATFORMS="amd64-linux,arm64-linux" | |
| fi | |
| if [ -z "$SHA" ]; then | |
| echo "No SHA provided, finding latest archive in bucket..." | |
| LATEST=$(gcloud storage ls -l "gs://minimal-shim/archives/minimalone-*.tar.zst" \ | |
| | grep -v "^TOTAL:" \ | |
| | sort -k2 \ | |
| | tail -1 \ | |
| | awk '{print $NF}') | |
| if [ -z "$LATEST" ]; then | |
| echo "ERROR: No archive found in bucket" | |
| exit 1 | |
| fi | |
| # Extract SHA: archives/minimalone-<sha>.tar.zst → <sha> | |
| SHA="$(basename "$LATEST" | sed 's/^minimalone-//;s/\.tar\.zst$//')" | |
| echo "Latest SHA: ${SHA}" | |
| fi | |
| # TODO: implement minimal-one-specific promotion process!! | |
| # Export resolved SHA for the docs-rebuild dispatch step below. | |
| echo "sha=${SHA}" >> "$GITHUB_OUTPUT" | |
| - name: Promote CLI version | |
| id: promote | |
| env: | |
| DRY_RUN: ${{ inputs.dry_run }} | |
| # Pass dispatch inputs via env (NOT inline ${{ inputs.* }} in `run:`) | |
| # to avoid shell template injection — see GitHub's hardening guide. | |
| INPUT_SHA: ${{ inputs.sha }} | |
| INPUT_PLATFORMS: ${{ inputs.platforms }} | |
| run: | | |
| # Trim leading/trailing whitespace; users often paste SHAs with stray spaces. | |
| SHA="$(printf '%s' "$INPUT_SHA" | sed 's/^[[:space:]]*//;s/[[:space:]]*$//')" | |
| PLATFORMS="$(printf '%s' "$INPUT_PLATFORMS" | sed 's/^[[:space:]]*//;s/[[:space:]]*$//')" | |
| if [ "$PLATFORMS" = "all" ]; then | |
| PLATFORMS="amd64-linux,arm64-linux" | |
| fi | |
| if [ -z "$SHA" ]; then | |
| echo "No SHA provided, finding latest archive in bucket..." | |
| LATEST=$(gcloud storage ls -l "gs://minimal-shim/archives/minimalone-*.tar.zst" \ | |
| | grep -v "^TOTAL:" \ | |
| | sort -k2 \ | |
| | tail -1 \ | |
| | awk '{print $NF}') | |
| if [ -z "$LATEST" ]; then | |
| echo "ERROR: No archive found in bucket" | |
| exit 1 | |
| fi | |
| # Extract SHA: archives/minimalone-<sha>.tar.zst → <sha> | |
| SHA="$(basename "$LATEST" | sed 's/^minimalone-//;s/\.tar\.zst$//')" | |
| echo "Latest SHA: ${SHA}" | |
| fi | |
| echo "minimalone promotion is not implemented yet" >&2 | |
| exit 1 |
🤖 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 @.github/workflows/promote.yml around lines 162 - 198, The Promote CLI
version step currently resolves SHA/PLATFORMS but then falls through a TODO and
succeeds, which can incorrectly trigger downstream docs rebuilds without
actually promoting anything. Update the run block in the Promote CLI version job
so it fails fast after SHA resolution until the new minimal-one promotion write
exists, using the existing promote step logic and the resolved SHA output to
gate later steps. Ensure the step exits non-zero with a clear message instead of
continuing past the TODO.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 @.github/workflows/ci.yml:
- Around line 245-246: The chmod step in the CI workflow is using a glob that
includes a macOS brace arm even though only Linux artifacts are downloaded, so
the shell leaves the unmatched pattern literal and the command fails. Update the
“Make binaries executable” step in the workflow to match only the actual
downloaded artifact names from the download step, and remove the unused macOS
pattern so the chmod invocation succeeds under the default shell.
In `@crates/minimal/src/main.rs`:
- Around line 370-375: The daemon startup path in
autospawn::ensure_daemon_running is using the wrong backend selection and
ignores the socket resolver inputs, so update it to start the backend implied by
global.minvmd and global.minimal_dir instead of always launching minvmd. Adjust
the call sites in minimal::main and any shared helpers so the selected backend
matches connect_daemon/global socket resolution, and ensure the logic handles
the Linux default --minvmd=false path that expects the native
providers/local-0/ssh.sock backend.
- Around line 568-572: The attach path in `main.rs` is emitting a
`tracing::info!` “found session” diagnostic that can leak into the remote
command’s stdout under the default `info` filter. Change this log in the session
lookup/attach flow to `tracing::debug!` or remove it entirely, keeping the rest
of the attach behavior unchanged so `minimal attach --command ...` does not
prepend diagnostics to stdout.
- Around line 415-418: The LAST ACTIVITY calculation in main should use the
newest available timestamp instead of always preferring last_stdout; update the
logic in the attrs.last_stdout/last_stdin handling to compare both values and
select the later one, preserving the existing map/formatting flow afterward. Use
the last_stdout and last_stdin fields in this block as the anchor when adjusting
the selection logic.
- Around line 872-883: The forward parsing in the ssh setup path still passes
malformed port specs through instead of validating them locally. Update the
logic in main’s forward handling to split on the first and last colon, preserve
IPv6 hosts correctly, and validate both LOCAL_PORT and REMOTE_PORT before
building the forward argument. Use the existing forward parsing block around
args.forward, parts, local_port, remote_host, and remote_port to replace the
current splitn(3) approach with stricter validation and an early error return
for invalid ports.
- Around line 975-983: Clamp the rewritten client key to 0600 in the key-write
path because the current OpenOptions::mode(0o600) in the write logic for
client_key_path only affects newly created files, not existing ones that are
truncated. Update the file handling around the OpenOptions/open and write_all
flow in main.rs so the existing client.key permissions are explicitly set to
0600 before or immediately after opening and before writing the new key
material.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d4be8173-93f5-448e-9076-d2c3ca5ba8c0
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (41)
.github/workflows/ci-macos.yml.github/workflows/ci.yml.github/workflows/promote.yml.minimal/minimal.tomlCargo.tomlcrates/minimal/Cargo.tomlcrates/minimal/src/autospawn.rscrates/minimal/src/client.rscrates/minimal/src/main.rscrates/minimal2/Cargo.tomlcrates/minimal2/src/main.rscrates/mip/Cargo.tomlcrates/mip/build.rscrates/mip/src/cmd_add.rscrates/mip/src/cmd_cache.rscrates/mip/src/cmd_check.rscrates/mip/src/cmd_dep.rscrates/mip/src/cmd_dump.rscrates/mip/src/cmd_init.rscrates/mip/src/cmd_materialize.rscrates/mip/src/cmd_patched_build.rscrates/mip/src/cmd_pkg.rscrates/mip/src/cmd_plan.rscrates/mip/src/cmd_remote_build.rscrates/mip/src/cmd_rexec.rscrates/mip/src/cmd_run.rscrates/mip/src/cmd_status.rscrates/mip/src/cmd_update.rscrates/mip/src/cmd_upload_cache.rscrates/mip/src/main.rsdocs/specs/01-spec-minvmd-host-daemon/01-spec-minvmd-host-daemon.mddocs/specs/01-spec-minvmd-host-daemon/architecture.mddocs/specs/02-spec-minvmd-linux-kvm/02-spec-minvmd-linux-kvm.mddocs/specs/03-spec-networking/03-spec-networking.mddocs/specs/03-spec-networking/architecture.mddocs/specs/03-spec-networking/test-plan.mddocs/specs/03-spec-networking/test-plan.shdocs/specs/06-spec-ssh-host-key-in-beacon/06-spec-ssh-host-key-in-beacon.mdjustfilescripts/fetch-artifact.shscripts/fetch-libkrun.sh
💤 Files with no reviewable changes (2)
- crates/minimal2/Cargo.toml
- crates/minimal2/src/main.rs
✅ Files skipped from review due to trivial changes (6)
- docs/specs/06-spec-ssh-host-key-in-beacon/06-spec-ssh-host-key-in-beacon.md
- docs/specs/03-spec-networking/test-plan.md
- docs/specs/03-spec-networking/architecture.md
- docs/specs/01-spec-minvmd-host-daemon/architecture.md
- docs/specs/02-spec-minvmd-linux-kvm/02-spec-minvmd-linux-kvm.md
- docs/specs/03-spec-networking/03-spec-networking.md
🚧 Files skipped from review as they are similar to previous changes (9)
- scripts/fetch-libkrun.sh
- crates/mip/Cargo.toml
- scripts/fetch-artifact.sh
- Cargo.toml
- justfile
- docs/specs/03-spec-networking/test-plan.sh
- crates/minimal/Cargo.toml
- .minimal/minimal.toml
- crates/mip/src/main.rs
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/promote.yml (1)
195-198: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPromotion step still silently "succeeds" with no actual promotion.
After resolving
SHA, the step only reaches a TODO and then unconditionally exportssha, so this job reports success and the docs rebuild dispatches even though nothing was promoted for MinimalOne. This is the same gap flagged previously and remains unresolved; the PR description does note this as deferred follow-up work, but the false-success + downstreamdocsdispatch side effect is worth gating explicitly until the real promotion write lands.🐛 Suggested guard
- # TODO: implement minimal-one-specific promotion process!! - - # Export resolved SHA for the docs-rebuild dispatch step below. - echo "sha=${SHA}" >> "$GITHUB_OUTPUT" + echo "::error::minimalone promotion is not implemented yet" >&2 + exit 1🤖 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 @.github/workflows/promote.yml around lines 195 - 198, The promotion logic in the MinimalOne branch still falls through to exporting `sha` even though no promotion happens, so the job and downstream docs dispatch look successful when they should not. Update the relevant promotion step in the promote workflow to explicitly gate the `echo "sha=${SHA}" >> "$GITHUB_OUTPUT"` behavior behind a real MinimalOne promotion path, using the existing `SHA` resolution and the TODO block as the locator, and fail or skip the output/dispatch when promotion is not implemented yet..github/workflows/ci.yml (1)
241-268: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy liftRelease job still has broken artifact paths —
gh release create/completions will fail at runtime.
actions/download-artifact@v8withpattern: "*-linux-*"matches 6 artifacts and nomerge-multiple: true, so by default each lands in its own per-artifact subdirectory (artifacts/mip-linux-amd64/mip,artifacts/minimal-linux-amd64/minimal, ...). But:
- The completions step (lines 251-259) invokes
artifacts/mip-linux-amd64,artifacts/minimal-linux-amd64,artifacts/minimald-linux-amd64directly as if these are the executables — they're directories, not files, so every invocation fails (errors are swallowed by|| true, silently producing no completion files).- Even switching to
merge-multiple: truedoesn't fix this:actions/upload-artifactpreserves the original basename inside the artifact (mip,minimal,minimald), not the artifactname. Flattening would instead collide the amd64 and arm64 binaries of the same name onto the same path, silently dropping one architecture.chmod +x artifacts/*-{linux,macos}-*(line 247) still has the previously-flagged unmatched*-macos-*brace arm (no macOS artifacts are downloaded here), which fails the step under the default shell.gh release create "$(ls)"(line 266) would then publish directories (or nothing useful) as release assets instead of the binaries.These are the same root issues raised in earlier reviews on this job (download-artifact subdirectory layout, unmatched chmod glob) that appear to have regressed when this PR reworked the job to handle the new
mip/minimal/minimaldmatrix.🐛 Suggested fix — rename binaries to their final asset name before upload, then download flat
- name: Upload binary (mip) + run: cp target/x86_64-unknown-linux-musl/release/mip mip-linux-amd64 uses: actions/upload-artifact@v7 with: name: mip-linux-amd64 - path: target/x86_64-unknown-linux-musl/release/mip + path: mip-linux-amd64 retention-days: 7(repeat for each of the 6 binary uploads, then in
release:)- name: Download artifacts uses: actions/download-artifact@v8 with: pattern: "*-linux-*" path: artifacts/ + merge-multiple: true - name: Make binaries executable - run: chmod +x artifacts/*-{linux,macos}-* + run: chmod +x artifacts/*-linux-*🤖 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 @.github/workflows/ci.yml around lines 241 - 268, The release job is still using the wrong downloaded artifact layout, so the completion commands and gh release step are pointing at directories instead of binaries. Fix the workflow so the uploaded binaries are renamed to their final asset names before upload, then have the release job download them in a flat layout and reference the actual executable files in the Generate completions and Create Release steps; use the release job’s actions/download-artifact@v8, the chmod step, and gh release create commands as the anchors when updating the paths.
🤖 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 @.github/workflows/ci-macos.yml:
- Line 282: The XDG_RUNTIME_DIR setup in the CI workflow is hiding mktemp
failures because the assignment happens inside export. Update the shell step to
use a separate assignment for XDG_RUNTIME_DIR before exporting it, so a failed
mktemp-d call is not silently ignored; keep the change localized to the workflow
step that currently sets XDG_RUNTIME_DIR.
---
Duplicate comments:
In @.github/workflows/ci.yml:
- Around line 241-268: The release job is still using the wrong downloaded
artifact layout, so the completion commands and gh release step are pointing at
directories instead of binaries. Fix the workflow so the uploaded binaries are
renamed to their final asset names before upload, then have the release job
download them in a flat layout and reference the actual executable files in the
Generate completions and Create Release steps; use the release job’s
actions/download-artifact@v8, the chmod step, and gh release create commands as
the anchors when updating the paths.
In @.github/workflows/promote.yml:
- Around line 195-198: The promotion logic in the MinimalOne branch still falls
through to exporting `sha` even though no promotion happens, so the job and
downstream docs dispatch look successful when they should not. Update the
relevant promotion step in the promote workflow to explicitly gate the `echo
"sha=${SHA}" >> "$GITHUB_OUTPUT"` behavior behind a real MinimalOne promotion
path, using the existing `SHA` resolution and the TODO block as the locator, and
fail or skip the output/dispatch when promotion is not implemented yet.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 407627d4-313c-40ac-834a-83df55d9e1de
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (41)
.github/workflows/ci-macos.yml.github/workflows/ci.yml.github/workflows/promote.yml.minimal/minimal.tomlCargo.tomlcrates/minimal/Cargo.tomlcrates/minimal/src/autospawn.rscrates/minimal/src/client.rscrates/minimal/src/main.rscrates/minimal2/Cargo.tomlcrates/minimal2/src/main.rscrates/mip/Cargo.tomlcrates/mip/build.rscrates/mip/src/cmd_add.rscrates/mip/src/cmd_cache.rscrates/mip/src/cmd_check.rscrates/mip/src/cmd_dep.rscrates/mip/src/cmd_dump.rscrates/mip/src/cmd_init.rscrates/mip/src/cmd_materialize.rscrates/mip/src/cmd_patched_build.rscrates/mip/src/cmd_pkg.rscrates/mip/src/cmd_plan.rscrates/mip/src/cmd_remote_build.rscrates/mip/src/cmd_rexec.rscrates/mip/src/cmd_run.rscrates/mip/src/cmd_status.rscrates/mip/src/cmd_update.rscrates/mip/src/cmd_upload_cache.rscrates/mip/src/main.rsdocs/specs/01-spec-minvmd-host-daemon/01-spec-minvmd-host-daemon.mddocs/specs/01-spec-minvmd-host-daemon/architecture.mddocs/specs/02-spec-minvmd-linux-kvm/02-spec-minvmd-linux-kvm.mddocs/specs/03-spec-networking/03-spec-networking.mddocs/specs/03-spec-networking/architecture.mddocs/specs/03-spec-networking/test-plan.mddocs/specs/03-spec-networking/test-plan.shdocs/specs/06-spec-ssh-host-key-in-beacon/06-spec-ssh-host-key-in-beacon.mdjustfilescripts/fetch-artifact.shscripts/fetch-libkrun.sh
💤 Files with no reviewable changes (2)
- crates/minimal2/Cargo.toml
- crates/minimal2/src/main.rs
✅ Files skipped from review due to trivial changes (7)
- docs/specs/06-spec-ssh-host-key-in-beacon/06-spec-ssh-host-key-in-beacon.md
- docs/specs/03-spec-networking/architecture.md
- docs/specs/01-spec-minvmd-host-daemon/architecture.md
- crates/mip/Cargo.toml
- docs/specs/03-spec-networking/test-plan.md
- docs/specs/02-spec-minvmd-linux-kvm/02-spec-minvmd-linux-kvm.md
- docs/specs/03-spec-networking/03-spec-networking.md
🚧 Files skipped from review as they are similar to previous changes (9)
- scripts/fetch-libkrun.sh
- scripts/fetch-artifact.sh
- justfile
- Cargo.toml
- .minimal/minimal.toml
- crates/minimal/Cargo.toml
- docs/specs/03-spec-networking/test-plan.sh
- crates/mip/src/main.rs
- crates/minimal/src/main.rs
bryan-minimal
approved these changes
Jul 1, 2026
bryan-minimal
left a comment
Member
There was a problem hiding this comment.
LGTM, nothing in this should break build-servers and can prep for the changes to make mip work in the CI when we get to the later patches
10 tasks
norrietaylor
added a commit
that referenced
this pull request
Jul 10, 2026
…der index Unit 3 of the per-VM writable ext4 volume spec (R3.1-R3.5): the operational contracts that follow from user data living permanently on the volume. - R3.1 (sessions): DiskLoader::new no longer hard-fails on a corrupt sessions/index.json — the index is derived state, so a parse failure falls back to an empty index, the existing self-heal pass rebuilds it from per-session record.json files, and a valid index is flushed back. Transient read I/O errors still fail. A torn index write now survives restarts, so this path is reachable in practice. - R3.2 (minimald): the guest boot path resets `providers/` (boot- ephemeral host keys, known_hosts, socket paths) after mounting the volume, while `sessions/` and `cache/` are untouchable by construction — no glob over the state root. - R3.3 (minimald, rescoped to the git-push path — the tar receiver in rpc.rs has no ported client, #603): a push into a session worktree with uncommitted tracked changes is rejected loudly by a new pre-receive hook ("[remote rejected] (pre-receive hook declined)", non-zero `git push` exit) instead of post-receive's silent skip; `git push -o force-checkout` overrides and discards those changes. receive-pack now advertises push options and sets receive.denyCurrentBranch=ignore — the hook pair owns worktree consistency, and git's default guard refused every re-push to the branch post-receive last checked out. Hooks live in src/git_hooks/*.sh via include_str!. - R3.4 (minvmd): new ProviderIndex — a persistent JSON map of SessionId -> { image_path, vm_id } at <state>/session_index.json, atomic rename writes, corrupt-file loads as empty (derived data). - R3.5 (minvmd): the `run` supervisor maintains the index by polling ListSessions over the existing host->guest bridge UDS (MINVMD_SESSION_POLL_SECS, default 15, 0 disables). This deliberately replaces the spec's guest->host SessionLifecycle RPC: a held-open guest->host vsock wedges against host->guest traffic (#588), and lifecycle events fire exactly while the client's own host->guest connection is open, so one-shot emits cannot be serialized safely either. Entries for other VMs — and entries for a VM that stopped — persist: the mapping is what future multi-VM routing (#311) needs. architecture.md updated accordingly (ledger row settled-by-avoidance). e2e (gated MINVMD_E2E=1, verified on macOS/HVF): a created session appears in session_index.json with this VM's image path and vm_id, a destroyed one is removed, and a session live at `minvmd stop` persists. Refs: #583 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
norrietaylor
added a commit
that referenced
this pull request
Jul 10, 2026
…der index Unit 3 of the per-VM writable ext4 volume spec (R3.1-R3.5): the operational contracts that follow from user data living permanently on the volume. - R3.1 (sessions): DiskLoader::new no longer hard-fails on a corrupt sessions/index.json — the index is derived state, so a parse failure falls back to an empty index, the existing self-heal pass rebuilds it from per-session record.json files, and a valid index is flushed back. Transient read I/O errors still fail. A torn index write now survives restarts, so this path is reachable in practice. - R3.2 (minimald): the guest boot path resets `providers/` (boot- ephemeral host keys, known_hosts, socket paths) after mounting the volume, while `sessions/` and `cache/` are untouchable by construction — no glob over the state root. - R3.3 (minimald, rescoped to the git-push path — the tar receiver in rpc.rs has no ported client, #603): a push into a session worktree with uncommitted tracked changes is rejected loudly by a new pre-receive hook ("[remote rejected] (pre-receive hook declined)", non-zero `git push` exit) instead of post-receive's silent skip; `git push -o force-checkout` overrides and discards those changes. receive-pack now advertises push options and sets receive.denyCurrentBranch=ignore — the hook pair owns worktree consistency, and git's default guard refused every re-push to the branch post-receive last checked out. Hooks live in src/git_hooks/*.sh via include_str!. - R3.4 (minvmd): new ProviderIndex — a persistent JSON map of SessionId -> { image_path, vm_id } at <state>/session_index.json, atomic rename writes, corrupt-file loads as empty (derived data). - R3.5 (minvmd): the `run` supervisor maintains the index by polling ListSessions over the existing host->guest bridge UDS (MINVMD_SESSION_POLL_SECS, default 15, 0 disables). This deliberately replaces the spec's guest->host SessionLifecycle RPC: a held-open guest->host vsock wedges against host->guest traffic (#588), and lifecycle events fire exactly while the client's own host->guest connection is open, so one-shot emits cannot be serialized safely either. Entries for other VMs — and entries for a VM that stopped — persist: the mapping is what future multi-VM routing (#311) needs. architecture.md updated accordingly (ledger row settled-by-avoidance). e2e (gated MINVMD_E2E=1, verified on macOS/HVF): a created session appears in session_index.json with this VM's image path and vm_id, a destroyed one is removed, and a session live at `minvmd stop` persists. Refs: #583 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
norrietaylor
added a commit
that referenced
this pull request
Jul 14, 2026
…der index Unit 3 of the per-VM writable ext4 volume spec (R3.1-R3.5): the operational contracts that follow from user data living permanently on the volume. - R3.1 (sessions): DiskLoader::new no longer hard-fails on a corrupt sessions/index.json — the index is derived state, so a parse failure falls back to an empty index, the existing self-heal pass rebuilds it from per-session record.json files, and a valid index is flushed back. Transient read I/O errors still fail. A torn index write now survives restarts, so this path is reachable in practice. - R3.2 (minimald): the guest boot path resets `providers/` (boot- ephemeral host keys, known_hosts, socket paths) after mounting the volume, while `sessions/` and `cache/` are untouchable by construction — no glob over the state root. - R3.3 (minimald, rescoped to the git-push path — the tar receiver in rpc.rs has no ported client, #603): a push into a session worktree with uncommitted tracked changes is rejected loudly by a new pre-receive hook ("[remote rejected] (pre-receive hook declined)", non-zero `git push` exit) instead of post-receive's silent skip; `git push -o force-checkout` overrides and discards those changes. receive-pack now advertises push options and sets receive.denyCurrentBranch=ignore — the hook pair owns worktree consistency, and git's default guard refused every re-push to the branch post-receive last checked out. Hooks live in src/git_hooks/*.sh via include_str!. - R3.4 (minvmd): new ProviderIndex — a persistent JSON map of SessionId -> { image_path, vm_id } at <state>/session_index.json, atomic rename writes, corrupt-file loads as empty (derived data). - R3.5 (minvmd): the `run` supervisor maintains the index by polling ListSessions over the existing host->guest bridge UDS (MINVMD_SESSION_POLL_SECS, default 15, 0 disables). This deliberately replaces the spec's guest->host SessionLifecycle RPC: a held-open guest->host vsock wedges against host->guest traffic (#588), and lifecycle events fire exactly while the client's own host->guest connection is open, so one-shot emits cannot be serialized safely either. Entries for other VMs — and entries for a VM that stopped — persist: the mapping is what future multi-VM routing (#311) needs. architecture.md updated accordingly (ledger row settled-by-avoidance). e2e (gated MINVMD_E2E=1, verified on macOS/HVF): a created session appears in session_index.json with this VM's image path and vm_id, a destroyed one is removed, and a session live at `minvmd stop` persists. Refs: #583 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
norrietaylor
added a commit
that referenced
this pull request
Jul 15, 2026
…der index Unit 3 of the per-VM writable ext4 volume spec (R3.1-R3.5): the operational contracts that follow from user data living permanently on the volume. - R3.1 (sessions): DiskLoader::new no longer hard-fails on a corrupt sessions/index.json — the index is derived state, so a parse failure falls back to an empty index, the existing self-heal pass rebuilds it from per-session record.json files, and a valid index is flushed back. Transient read I/O errors still fail. A torn index write now survives restarts, so this path is reachable in practice. - R3.2 (minimald): the guest boot path resets `providers/` (boot- ephemeral host keys, known_hosts, socket paths) after mounting the volume, while `sessions/` and `cache/` are untouchable by construction — no glob over the state root. - R3.3 (minimald, rescoped to the git-push path — the tar receiver in rpc.rs has no ported client, #603): a push into a session worktree with uncommitted tracked changes is rejected loudly by a new pre-receive hook ("[remote rejected] (pre-receive hook declined)", non-zero `git push` exit) instead of post-receive's silent skip; `git push -o force-checkout` overrides and discards those changes. receive-pack now advertises push options and sets receive.denyCurrentBranch=ignore — the hook pair owns worktree consistency, and git's default guard refused every re-push to the branch post-receive last checked out. Hooks live in src/git_hooks/*.sh via include_str!. - R3.4 (minvmd): new ProviderIndex — a persistent JSON map of SessionId -> { image_path, vm_id } at <state>/session_index.json, atomic rename writes, corrupt-file loads as empty (derived data). - R3.5 (minvmd): the `run` supervisor maintains the index by polling ListSessions over the existing host->guest bridge UDS (MINVMD_SESSION_POLL_SECS, default 15, 0 disables). This deliberately replaces the spec's guest->host SessionLifecycle RPC: a held-open guest->host vsock wedges against host->guest traffic (#588), and lifecycle events fire exactly while the client's own host->guest connection is open, so one-shot emits cannot be serialized safely either. Entries for other VMs — and entries for a VM that stopped — persist: the mapping is what future multi-VM routing (#311) needs. architecture.md updated accordingly (ledger row settled-by-avoidance). e2e (gated MINVMD_E2E=1, verified on macOS/HVF): a created session appears in session_index.json with this VM's image path and vm_id, a destroyed one is removed, and a session live at `minvmd stop` persists. Refs: #583 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 17, 2026
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.
The diff isnt as good as I had hoped, note i changed no rust code just
mv minimal mipandmv minimal2 minimalminimal=>mip,minimal2=>minimalminimalto shell out tomipsetup-minimal- which installs the last-released minimal CLI. Future PR.Future PR:
setup-minimal-based tasks to use the latest-releasedmipSummary by CodeRabbit
mipCLI for build/test/packaging and remote workflows.minimalCLI into a session/mesh-focused tool (including SSH proxying/forwarding andlogincertificate issuance).minimalinstead of the prior variant.mip.mip,minimal, andminimald, generate completions, and upload a singleminimalone-<SHA>.tar.zstbundle.minimalinstead of the prior variant.