[quality] test: executed coverage for the Justfile VM recipes — _rootful_load_image, _build-bib, _run-vm

[kubestellar-hive]

Test Improvement

Adds tests/unit/justfile-vm-recipes_test.bats (22 tests) — executed
coverage for the three Justfile recipes that had none:
_rootful_load_image (Justfile:247-277), _build-bib
(Justfile:288-315) and _run-vm (Justfile:352-391).

These three carry the whole disk/ISO image pipeline: build-qcow2,
build-raw, build-iso, rebuild-*, run-vm-* and the build-vm/run-vm
aliases are all thin dispatchers onto them. Until now a regression in any of
their decisions only surfaced when a maintainer ran a real 15-25 minute image
build.

No production file is modified. The only file this PR adds or changes is
tests/unit/justfile-vm-recipes_test.bats.

How it runs

Against a sandbox copy of the Justfile, with podman, sudo, chown, jq,
ss, xdg-open, sleep and the recipes' own nested just invocations stubbed
on PATH — the same harness shape as tests/unit/justfile-build_test.bats. No
container starts, no privilege is escalated, no network is touched. The real
just is resolved before PATH is shadowed so the tests drive the real binary
while the recipes' internal just sudoif ... / just build-<type> ... calls hit
the stub and are asserted on.

What is asserted

_rootful_load_image — the SUDO_USER/UID short-circuit (must not touch
podman at all); skipping the copy when root storage already holds the image;
podman image scp <uid>@localhost::img:tag root@localhost::img:tag when the IDs
differ; the TMPDIR= staging directory and that it does not survive the recipe;
the podman pull fallback when podman inspect cannot resolve the image.

_build-bib — --type <type> --use-librepo=True --rootfs=btrfs; the
read-only /config.toml bind mount of the requested config; the shared
/var/lib/containers/storage mount; the target:tag reference; the BIB_IMAGE
override and the digest-pinned default; --privileged with
--security-opt label=type:unconfined_t; artifact relocation into output/ with
the staging directory removed; and that escalation is limited to the container
run and the artifact move/chown.

_run-vm — build-on-missing dispatch and the no-rebuild path; the iso
special case (output/bootiso/install.iso) versus output/<type>/disk.<type>;
loopback port selection including walking past ports ss reports as bound;
--device=/dev/kvm, TPM=Y, GPU=Y, CPU_CORES=4, RAM_SIZE=8G,
DISK_SIZE=64G; and the ephemeral docker.io/qemux/qemu invocation.

One characterization test, flagged as such

_build-bib: currently aborts with "USER: unbound variable" when USER is undefined pins a live defect, not a desired behavior. _build-bib ends with
sudo chown -R $USER:$USER output/ under set -u, so in any environment that
does not define USER the recipe fails after the image is already built,
leaving output/ owned by root. That is filed separately as #374 with the exact
one-line replacement; whoever applies it should update this single test to assert
the new chown argv. The test comment says so in place.

Verification

$ bats tests/unit/justfile-vm-recipes_test.bats
1..22
ok 1 .. ok 22

$ bats tests/unit          # whole suite, no regression
218 tests, 0 failures

Related Issue

Closes #376

Not related to #374 beyond the note above — that issue needs a Justfile change
this PR deliberately does not make.

Disjointness

This PR touches exactly one file, tests/unit/justfile-vm-recipes_test.bats,
which does not exist on main. Checked against every open finpilot PR:
#344 owns tests/unit/justfile-shell-sources_test.bats (shell-sources/lint
recipes), #353 and #373 own tests/unit/justfile-build_test.bats (the build
recipe), #368 owns tests/unit/image-identity_test.bats. #369 and #373 modify
Justfile; this PR does not, and its assertions do not depend on the sudoif
internals #369 changes or on the SHA_HEAD_SHORT build arg #373 removes.

---

Filed by quality agent (hold-gated mode). Human review required.

— hive: agent=quality backend=copilot model=claude-opus-5

[kubestellar-hive]

Important

Held for human review by the hive's ACMM level gate.

This PR was opened by the "quality" agent while Hive policy required a human checkpoint for that agent. Non-outreach agents are held at ACMM L3–L5; the outreach agent is always held because it publishes project-facing communication.

Hive will automatically remove the hold label once current policy no longer requires a level hold for "quality". If this is an outreach PR, a human must review it and remove the label.

[hanthor]

Thank you for this one — I went in expecting a vacuous bot-authored test PR and it is the opposite. I mutation-tested your assertions against the real recipes and they are genuinely load-bearing. Ten single-line breakages of Justfile, nine killed:

$ cd <worktree at 49b2dd6>
$ for each mutant: sed -i ... Justfile && bats tests/unit/justfile-vm-recipes_test.bats

rootfs btrfs -> ext4            -> not ok 6  passes the requested type and the fixed librepo/btrfs flags
drop :ro on the config mount    -> not ok 7  mounts the requested config read-only at /config.toml
drop --privileged from bib      -> not ok 11 runs the builder privileged with an unconfined label
label=type:unconfined_t -> disable -> not ok 11
port=8006 -> 9006               -> not ok 19, 20, 22
output/bootiso/install.iso -> boot.iso -> not ok 17 maps the iso type to output/bootiso/install.iso
drop --device=/dev/kvm          -> not ok 21 requests KVM, TPM, GPU and the documented VM sizing
CPU_CORES=4 -> 2                -> not ok 21
hardcode args="--type qcow2 "   -> not ok 7

One survivor only: adding :ro to the shared /var/lib/containers/storage mount (which would break a real bib run) passes all 22, because test 8 matches the mount as a substring. Worth tightening to an exact -v <path>:<path> token if you touch the file again, but that is a nit.

The blocking problem is that the _rootful_load_image block is not hermetic — it reads the ambient $UID instead of pinning it, so three of your 22 tests fail outright when the suite runs as root:

$ id -u
0
$ bats tests/unit/justfile-vm-recipes_test.bats
1..22
ok 1 _rootful_load_image: short-circuits under sudo without inspecting podman
ok 2 _rootful_load_image: skips the copy when root storage already has the image
not ok 3 _rootful_load_image: copies user storage to root when the IDs differ
not ok 4 _rootful_load_image: routes the copy through a TMPDIR it cleans up
not ok 5 _rootful_load_image: pulls into root storage when the image is unknown locally
ok 6 .. ok 22

The cause is unambiguous — --print-output-on-failure shows the recipe never reaches the code under test:

$ bats --print-output-on-failure -f "copies user storage to root" tests/unit/justfile-vm-recipes_test.bats
# + [[ -n '' ]]
# + [[ 0 -eq 0 ]]
# + echo 'Already root or running under sudo, no need to load image from user podman.'
# + exit 0

_rootful_load_image short-circuits on "${UID}" -eq "0" (Justfile:252), and tests 3, 4 and 5 stub podman/just but never arrange for a non-root $UID, so they assert against a recipe that exited at line 5. .github/workflows/unit-tests.yml runs on ubuntu-latest as an unprivileged user, so CI is green and this is invisible there — but it fails for anyone running the suite in a container, a rootful dev shell, or under sudo bats, which is a normal thing to do in a repo whose recipes are about rootful podman.

Worse than the three red tests is test 2, skips the copy when root storage already has the image. It asserts only negatives:

! grep -q 'podman image scp' "${JUST_LOG}"
! grep -q 'podman pull' "${JUST_LOG}"

Under root the recipe short-circuits before touching podman at all, so JUST_LOG is empty and both negatives hold trivially. It reports ok while testing nothing:

$ bats -f "skips the copy when root storage" tests/unit/justfile-vm-recipes_test.bats
1..1
ok 1 _rootful_load_image: skips the copy when root storage already has the image

That is a test that goes green in exactly the environment where its premise is false, which is the failure mode this PR exists to prevent elsewhere.

Bash makes $UID readonly, so you cannot simply export a fake one. Two workable directions: either skip the four _rootful_load_image non-short-circuit cases when [ "$(id -u)" -eq 0 ] — explicit and honest — or add a positive assertion to test 2 (that podman inspect was actually called, e.g. grep -q 'inspect -t image' "${PODMAN_LOG}") so it cannot pass vacuously, and let 3/4/5 skip. I would take both: the skip guard stops the red, the positive assertion stops the silent green.

Everything else checks out. Disjointness is real — I merged #366, #368, #371, #373 and this PR onto be4b0eb in one worktree and all five merged cleanly with no new failures attributable to this file; the only cross-PR breakage in that tree is iso/iso.toml kickstart ref matches the canonical vendor and name, which is the known #366/#368 semantic conflict and nothing to do with you. Your characterization test 14 for the USER: unbound variable defect is correctly flagged in place and I agree it belongs in #374 rather than here. Baseline for the record: bats tests/unit on the merge-base be4b0eb gives 195 ok / 1 not ok (build: does not add a build secret when GITHUB_TOKEN is unset, a pre-existing failure caused by GITHUB_TOKEN being set in my environment, not by anything here); with this PR, 197 ok / 1 not ok plus the three above.

Fix the hermeticity and I am happy to approve — the assertion content is some of the better test work I have reviewed in this repo.

---

Generated by Claude Code