test(qa): verify all four distro baselines in QEMU - #220
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
No merge conflicts. GitHub reports MERGEABLE against PyRo1121/fix-smoke-observed-verdicts, and git merge-tree is also clean against that base and against current main. This is not an explicit product or API break, so I am not blocking it.
I am not merging, closing, approving, rebasing, or retargeting this PR. The runner still sits on the smoke-verdict stack (base 71f92605 is not on main). Changing the base would be an author workflow decision, not a conflict fix.
The diff replaces the local Ubuntu-only QEMU script with one four-distro runner. Docs and the committed receipt point at scripts/benchmark-qemu.sh. There are no remaining callers of scripts/benchmark-qemu-ubuntu.sh. That is a local QA rename, not a CLI, JSON, or release-pipeline contract change.
Checks I ran here: bash -n scripts/benchmark-qemu.sh is clean; the committed runner SHA-256 889cff76a4b8647ee03c3f2976b4cc97acde4f862d3445d83df48acf268be246 matches benchmarks/records/qemu-four-distros-20260905.json. I did not re-run the four-guest KVM suite, install host packages, or change the lockfile.
Author-scoped leftovers I will not force through:
- Debian 13 /
libapt-pkg.so.6.0ABI compatibility is explicitly left separate. I will not retarget the published Debian artifact or invent a Trixie guest profile. - Published Fedora package defects remain. The passing Fedora receipt is a staged debug candidate from
525c74d4(#217), not a published archive fix. - Published Debian still depends on the native APT frontend fix in open #218 (
34854798). Expanding this PR to land that product change would be a different stack, not a conflict fix. - Image publisher signatures are documented as unverified. Adding independent signature checks would change the author’s stated runner contract.
- Exhaustive CLI inventory, cold-cache trials, and repeated-guest statistics are left as remaining work. I will not invent a follow-up harness for those.
- Later
benchmarks/README.mddesign-contract sections still describe the Ubuntu-only subset and unimplemented guest targets. Rewriting that ledger is author documentation scope, not a merge conflict.
Sent by Cursor Automation: Untitled
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e759450b0a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| [[ -z "$staged_dir" ]] || args+=(--staged-dir "$staged_dir") | ||
| [[ "$benchmark" == false ]] || args+=(--benchmark) | ||
| for target in arch debian ubuntu fedora; do | ||
| "$0" --distro "$target" --evidence-dir "$suite/$target" "${args[@]}" || rc=1 |
There was a problem hiding this comment.
When --distro all is running and the operator presses Ctrl-C, the active child exits with 130, but this || rc=1 handler treats that signal exit like an ordinary distro failure and advances to the next guest. Consequently, cancelling a potentially long four-distro run starts another target instead of stopping; preserve signal exit codes and terminate the loop while continuing only after normal test failures.
Useful? React with 👍 / 👎.
|
Follow-up commit ebec120 adds failure-path coverage and preserves interrupted suite evidence.
Current-code live verification passed Arch, Debian 12, Ubuntu 24.04, and Fedora 44 without --benchmark. Fedora required a separate retry after Docker controller creation timed out. The committed default_path_follow_up receipt retains both the failed suite-OmHcRb and passing run-N4VEvQ. This is not presented as one uninterrupted green suite. Each passing profile has a guest exit of zero, reboot proof, native package checks, and verified key/disk/controller cleanup. The earlier suite-iaP5Nd remains historical benchmark evidence. No timing comparisons were collected under the competing Cargo load in this follow-up. Shell fixtures, syntax, JSON checks, executed-runner SHA-256 matching, and diff checks pass. Automated review abstained, and independent review remains unavailable under the no-subagent restriction. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebec12009e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ssh_service=sshd | ||
| case "$distro" in | ||
| arch) | ||
| image_url=https://geo.mirror.pkgbuild.com/images/latest/Arch-Linux-x86_64-cloudimg-20260901.583572.qcow2 |
There was a problem hiding this comment.
When Arch refreshes its cloud images, this dated object under /images/latest/ can disappear, causing both --distro arch and the sequential all-distro suite to fail during download despite having a pinned checksum. The repository's own design identifies this catalog as a discovery URL rather than an immutable input (benchmarks/README.md:212-219); use the corresponding immutable versioned image URL instead.
Useful? React with 👍 / 👎.
| hyperfine --shell=none --output=pipe --warmup 3 --runs 30 --export-json evidence/info.json \ | ||
| --command-name 'OMG installed info' "$bin info tree" --command-name 'Native info' "${native[*]}" |
There was a problem hiding this comment.
When --benchmark is enabled, the runner exports samples without capturing hyperfine --version, so results obtained from potentially different distro-packaged hyperfine releases cannot be reproduced or audited reliably. This also conflicts with the performance-record contract in benchmarks/README.md:264-268; persist the installed version alongside info.json and include it in the committed receipt.
Useful? React with 👍 / 👎.
8d991d1
into
PyRo1121/fix-smoke-observed-verdicts
1 participant
Why
The Ubuntu-only runner could not prove that the supported distro baselines work under real guest kernels. Extend one runner rather than copy its SSH, reboot, and cleanup logic four times.
Scope
Replace scripts/benchmark-qemu-ubuntu.sh with scripts/benchmark-qemu.sh. Add pinned x86_64 profiles for Arch 20260901, Debian 12, Ubuntu 24.04, and Fedora 44. Support sequential all-distro runs, published or staged inputs, and optional warm info measurements.
Each guest checks its identity, strict SSH host keys, sudo, a changed boot ID after reboot, package search, install, exact native name/version parity, remove, and native absence. Debian and Ubuntu also check local-file consent and installation. The receipt and decision trail are committed under benchmarks/records. README instructions identify the supported baselines and remaining limits.
Tradeoffs
Reuse native package managers for fixtures and state assertions. APT fixture downloads use HTTPS on Ubuntu, bounded IPv4 requests, and no unused translation or desktop indexes. Signature checks remain enabled. One guest per distro and debug candidates do not establish release speedups.
A teardown timeout exposed unnecessary cleanup through Docker exec. The final runner removes the controller first and deletes known temporary files from the host-owned run directory. It does not report PASS when cleanup fails.
Blast Radius
This replaces a local script, not the release pipeline. No host package installation, merge, release publication, or deployment occurred. Missing staged artifacts do not fall back to published artifacts.
The passing set uses unchanged published Arch and Ubuntu archives and fixed Debian and Fedora candidates. The APT fix is separate at #218. Published Fedora defects and Debian 13 libapt ABI incompatibility remain. Exhaustive CLI coverage is not claimed.
Verification
The final frozen runner completed suite-iaP5Nd with four PASS results. All four guests rebooted, checked native package state, and produced 30 samples for each of two commands. All 240 raw samples are retained. Controller absence and deletion of private keys, seed images, overlays, and base images were checked after teardown.
Shell syntax checks, invalid-argument checks, release-smoke fixtures, receipt shape checks, exact runner SHA-256 matching, and git diff --check pass. The Debian candidate build and Clippy used -j 2 and CARGO_BUILD_JOBS=2 in a disposable Bookworm build controller.
Automated review abstained outside its project root. Diff and comment review were manual. Independent subagent review was not available under the session restriction. The task transcript was checked against the final receipt.