From f5c2cc9a427ca9e6573d4129cf00d601979cf999 Mon Sep 17 00:00:00 2001 From: BFE Engineering Date: Sun, 30 Aug 2026 08:45:46 -0600 Subject: [PATCH] review: iteration-3 fixes (convergence pass) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ci-cd.md phrasing made time-invariant (true before and after the visibility flip; the prior wording asserted a state not yet real). - ui-shot: liveness check before the tap QMP call too — the commit-message claim of "before every QMP call" is now actually true. - busybox fetcher checks its pin before downloading, mirroring the kernel-tarball fetcher's ordering. - Fragment disable-arm: absence of a deps-gated symbol is a valid disable outcome; only "still set" is fatal (the iteration-2 symmetrization overcorrected). Negative-tested both directions under set -e. Iteration 3 produced four findings, none blocking, all applied — the recursive review is converged. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018HUayid7W5w7jBdb9Rrj1K --- build/build-kernel.sh | 13 ++++++++----- docs/ci-cd.md | 6 +++--- qemu/lib.sh | 10 ++++++---- qemu/tests/ui-shot.sh | 1 + 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/build/build-kernel.sh b/build/build-kernel.sh index 114f68b..73f097c 100755 --- a/build/build-kernel.sh +++ b/build/build-kernel.sh @@ -129,11 +129,14 @@ if [ -n "${WARDEN_KCONFIG_FRAGMENT:-}" ]; then frag_fail=1 } ;; "# CONFIG_"*" is not set") - # Symmetric with the enable arm: the exact disable line must be - # present. A symbol absent entirely means a typo'd/renamed option, - # not a successful disable. - grep -qxF "$line" "$SRC/.config" || { - echo "FATAL: fragment line '$line' not reflected in the final .config" >&2 + # A disable succeeded if the symbol is NOT set: Kconfig writes either + # the literal "is not set" line or (when dependencies gate the symbol + # out) nothing at all — both are valid outcomes. Only "still =value" + # is a failed disable. (A typo'd symbol disables nothing and is + # harmless by construction.) + opt="${line#\# }"; opt="${opt% is not set}" + grep -qE "^$opt=" "$SRC/.config" && { + echo "FATAL: fragment disabled '$opt' but it is still set in the final .config" >&2 frag_fail=1 } ;; esac diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 4e13a52..0873833 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -1,9 +1,9 @@ # CI/CD `.github/workflows/ci.yml` — every job runs on GitHub-hosted `ubuntu-latest`. -No self-hosted runner is (or may be) reachable from this repo's workflows: -the repo is public, and a fork PR that gets one approved run could -otherwise execute code on private infrastructure (ADR-0007). +No self-hosted runner is (or may be) reachable from this repo's workflows — +on a public repo, a fork PR that gets one approved run could otherwise +execute code on private infrastructure (ADR-0007). ## Jobs diff --git a/qemu/lib.sh b/qemu/lib.sh index 2470945..66324b7 100644 --- a/qemu/lib.sh +++ b/qemu/lib.sh @@ -16,14 +16,16 @@ qemu_get_busybox() { local out="${OUT:-$QEMU_DIR/out}" mkdir -p "$out" BB="${BUSYBOX:-$out/busybox-armv7l}" - if [ ! -f "$BB" ]; then - qemu_log "downloading $BB_URL" - curl --retry 3 --retry-delay 5 --retry-connrefused -fSL "$BB_URL" -o "$BB" - fi + # Pin first: a missing pin refuses BEFORE downloading, same ordering as + # build/fetch-kernel-tarball.sh. [ -f "$sha_file" ] || { echo "FATAL: no pinned sha256 for busybox (expected $sha_file) — refusing to build from an unverified binary" >&2 exit 1 } + if [ ! -f "$BB" ]; then + qemu_log "downloading $BB_URL" + curl --retry 3 --retry-delay 5 --retry-connrefused -fSL "$BB_URL" -o "$BB" + fi local want got want="$(cat "$sha_file")" got="$(sha256sum "$BB" | awk '{print $1}')" diff --git a/qemu/tests/ui-shot.sh b/qemu/tests/ui-shot.sh index 11a3a8a..4b4fdb7 100755 --- a/qemu/tests/ui-shot.sh +++ b/qemu/tests/ui-shot.sh @@ -123,6 +123,7 @@ done # Tap the "Metrics" tab: pixel (373,40) of 720x720 scaled to the QMP absolute # range 0..32767 — switching tabs must repaint the content area. Poll for the # repaint rather than guessing a delay. +vm_alive_or_die qmp tap 16975 1820 changed=0 # 90s, matching the first-frame budget: TCG repaints are CPU-bound and a