diff --git a/.claude/agents/implementer.md b/.claude/agents/implementer.md index 99cdb0d617b..8b2dfc33473 100644 --- a/.claude/agents/implementer.md +++ b/.claude/agents/implementer.md @@ -24,7 +24,7 @@ Keep the branch coherent: change all consumers and declarations affected by an i ## Verify the behavior -Apply root `CLAUDE.md`'s test selection and scheduling policy. Run host checks with `cargo run -- --ci host`; build an affected image with `cargo run -- --build-only`. Use filtered `cargo test` runs only for behavior that requires the guest. Do not run a broad integration suite merely because a PR needs evidence. +Apply root `CLAUDE.md`'s test selection and scheduling policy. Run host checks with `cargo run -- --ci host`; build an affected image with `cargo run -- --build-only`. Dispatch filtered guest rows (`tests/CLAUDE.md`) only for behavior that requires the guest. Do not run a broad integration suite merely because a PR needs evidence. For relevant metal tests, prepare committed images with `cargo test --test toyos-build -- --metal --metal-readback `. This writes `/request.txt` without touching a machine. Send the request to the orchestrator so compatible tests can share boots. Never execute metal runs yourself. diff --git a/.claude/agents/reviewer.md b/.claude/agents/reviewer.md index 9ca6e810675..b8a13fcac3f 100644 --- a/.claude/agents/reviewer.md +++ b/.claude/agents/reviewer.md @@ -43,7 +43,7 @@ Hardware oracles come from vendor specifications or independent evidence checked - New dependencies obey root's source, licensing and self-hosting rules. Another implementation's source is not directly cited as ToyOS's authority. Source, tests and manifests name Linux only as a host or as part of an externally defined interface. No production machinery ships solely to satisfy a test. - Host tools beyond Rust and QEMU are declared in `issues/the-build-runs-host-tools-outside-rust-and-qemu.md`. No host-only binaries or ToyOS-maintained Python, Perl or shell. Fork changes and every consumed pin land together under `implementer.md`'s fork rules. - A package has a meaningful `description` and is accounted for by the workspace. Member-level `[profile]` and `[patch]` sections are invalid. Changed features and configuration arms must be covered by `build/clippy.rs`. Issue edits obey `issues/README.md`. -- Preserve CI security: actions are pinned by full commit hash or valid local path; runners are GitHub-hosted. No privileged triggers that execute untrusted code as main. Only main publishes caches other refs restore; nightly release alone receives `contents: write`. Do not bypass required checks, hide failures with `continue-on-error`, or introduce a write-capable cache on a PR. +- Preserve CI security: actions are pinned by full commit hash or valid local path; runners are GitHub-hosted, except the guest runner, which only a `workflow_dispatch`-only workflow may name. No privileged triggers that execute untrusted code as main. Only main publishes caches other refs restore; nightly release alone receives `contents: write`. Do not bypass required checks, hide failures with `continue-on-error`, or introduce a write-capable cache on a PR. - For workflow, build or harness changes, read `build/CLAUDE.md` and `tests/CLAUDE.md` for their operational contracts, including CI cache provenance and corpus membership. - An actuator-controlled kernel static is accessed only under its guard or in actuator-only code. A `cfg(not(feature = ...))` arm ships and is not an exemption. - Changed or removed commands, flags, steps and documents have their instruction references updated in the same change. For deleted documents search both name and path outside `issues/`. Replace contradictory instruction rules rather than appending exceptions. diff --git a/.github/workflows/guest-rows.yml b/.github/workflows/guest-rows.yml new file mode 100644 index 00000000000..9decab8e6c2 --- /dev/null +++ b/.github/workflows/guest-rows.yml @@ -0,0 +1,56 @@ +name: guest-rows +# What a dispatcher finds its own run by. +run-name: guest-rows ${{ inputs.rev }} ${{ inputs.filter }} + +# Guest rows on demand: `cargo test --test toyos-build -- ` at `rev`, on +# the guest runner (`cargo run -- --ci rows`, build/ci.rs). The runner is a +# dedicated, otherwise idle host that takes one job at a time, so this queue is +# the only one its guests wait in. It keeps its workspace and its toolchain +# store (`~/.cache/toyos`) between jobs; a revision whose toolchain is not in +# the store builds it here first. +# +# The host provides: x86-64 Linux with `/dev/kvm` the runner's user opens; QEMU +# at build/ci/qemu-version for both architectures, with the firmware +# descriptors of its x86-64 and AArch64 UEFI firmware; rustup's stable on +# `PATH`; git, a C and C++ toolchain, CMake and Python 3 for a toolchain build. +# +# `workflow_dispatch` alone, and never callable: a dispatch needs write access +# to the repository, and no pull request reaches this runner's label from this +# repository's workflows (build/ci.rs's `only_a_dispatch_reaches_a_self_hosted_runner`). +# The job reads no secret. +on: + workflow_dispatch: + inputs: + rev: + description: The commit to test, as a full hash + required: true + type: string + filter: + description: The harness's arguments after `--`, split at whitespace; empty runs the whole suite + required: false + type: string + +permissions: + contents: read + +jobs: + rows: + runs-on: [self-hosted, toyos-guest] + # A wedge guard, wide enough for a toolchain the store does not hold. + timeout-minutes: 360 + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + with: + ref: ${{ inputs.rev }} + # `target/` is the warm build; `git clean -ffdx` would remove it. + clean: false + # The fork a toolchain the store lacks is built from; the build + # fetches its nested submodules itself. + submodules: true + # Nothing on this machine outlives the job holding the token. + persist-credentials: false + + # One argument, which the driver splits at whitespace. + - env: + FILTER: ${{ inputs.filter }} + run: cargo run -- --ci rows "$FILTER" diff --git a/CLAUDE.md b/CLAUDE.md index dc6cea3d149..75dd498c239 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -49,15 +49,15 @@ Rust and QEMU are the development prerequisites. Other host tools must be builda - **Choose the cheapest sufficient test.** First make bugs unrepresentable, then test logic and failure handling on the host, including hardware models. Prefer metal for actual hardware behavior; QEMU is last. Receiving malformed input is a unit or module test, even when the real input arrives from another process. Crossing a component boundary does not itself justify an integration test. - **Keep tests focused.** Userland is tested as units and modules, with at most a cheap smoke test. Avoid scenarios that install software, launch applications and drive unrelated steps. Kernel integration and hardware tests must name the behavior host tests cannot establish. Each test has a clear claim, controlled inputs and a meaningful failure; automated tests do not depend on the public internet. -- **Declare where integration tests run.** Record supported metal machines and QEMU configurations with the test registration or harness metadata. Before opening a code PR, run the relevant integration tests locally on those targets. The broad integration matrix runs nightly, not for every PR or merge-queue entry. Host checks remain routine. Inspect the actual harness and workflows; these instructions do not themselves change their scheduling. +- **Declare where integration tests run.** Record supported metal machines and QEMU configurations with the test registration or harness metadata. Before opening a code PR, run the relevant integration tests on those targets. The broad integration matrix runs nightly, not for every PR or merge-queue entry. Host checks remain routine. Inspect the actual harness and workflows; these instructions do not themselves change their scheduling. - **AArch64 waits for a metal bench.** Until an AArch64 metal bench exists, there is no AArch64-only work. A PR runs aarch64 guest tests only when it changes AArch64 code or the shared code beneath it, such as memory management, context switch, the loader, and shared atomics or DMA ordering that only aarch64 exposes; nightly covers the rest. An aarch64-only red that the change did not introduce is a sighting unless it shows a defect in shared code. - **Amortize boots.** Batch compatible metal tests in a shared boot. Separate boots need a reason, such as a boot-only property, destructive test or incompatible image. Plenty of useful metal tests are welcome; unnecessary reboots are not. - **Use real hardware for timing and audio.** QEMU establishes order, completion, contents and counts; its clock is only a hang ceiling. Benchmarks and exploratory comparisons are evidence, not automatically regression tests. A timing test has a justified bound for its declared machine. -- **Run what can change.** Use `cargo run -- --ci host` for host checks, `cargo run -- --build-only` for an image, and filtered `cargo test` invocations for relevant guest behavior. Repeat checks only when a change, failure or unresolved concern could invalidate their result. A new commit, comment deletion or rename alone does not require another integration run; compile where necessary. +- **Run what can change.** Use `cargo run -- --ci host` for host checks, `cargo run -- --build-only` for an image, and filtered guest rows for relevant guest behavior. Repeat checks only when a change, failure or unresolved concern could invalidate their result. A new commit, comment deletion or rename alone does not require another integration run; compile where necessary. - **Investigate reds.** Do not rerun a failure merely to get green. Fix a defect introduced by the change. A real unrelated defect may be recorded without blocking the PR once its independence is established. Repair or delete a defective or redundant test according to its value, preserving evidence of any real bug it exposed. A guest red that appears only under host noise is not investigated or fixed; it may be recorded as a sighting. - **Show evidence.** Record commands, exit codes, logs, revisions and machines. Identify earlier evidence reused and why later edits do not invalidate it. Use an independent oracle where available and a negative control where a defect would otherwise land unseen; the whole change reverted onto the measured base is the control for the whole change. Another agent is not an independent oracle. Do not add a test for what a type or straightforward inspection already establishes. -Guest tests run only on a quiet host; agents may run their own selected ones. Metal runs are coordinated and executed by the orchestrator; implementers prepare them with `--metal-readback`, which touches no machine. +Guest tests run on the guest runner through `guest-rows.yml`'s dispatch (`tests/CLAUDE.md`), and agents may dispatch their own selected rows; aarch64 guests also run on a quiet Mac on demand. Metal runs are coordinated and executed by the orchestrator; implementers prepare them with `--metal-readback`, which touches no machine. ## Working safely diff --git a/build/ci.rs b/build/ci.rs index 1e1a8e0343d..32e3ef039ec 100644 --- a/build/ci.rs +++ b/build/ci.rs @@ -50,6 +50,9 @@ const USAGE: &str = "cargo run -- --ci , where is one of: image the shipped image of every architecture, on the sysroot its job restored (ci.yml) guest the guest suite, on the sysroot its job restored (nightly) + rows [...] `cargo test --test toyos-build -- ...`, each split at + whitespace, on this host's own toolchain store, held to the + instrument as `guest` is (guest-rows.yml) release put the sysroot main's nightly restored up as its toolchain release publish put main's SDK crates on crates.io (publish.yml)"; @@ -64,6 +67,8 @@ enum Job { Bootstrap, Image, Guest, + /// The harness's arguments. + Rows(Vec), Release, Publish, } @@ -96,6 +101,10 @@ fn parse(words: &[String]) -> Result { }; return Ok(Job::Host(Some(part))); } + // Split again at whitespace: a workflow hands the arguments over as one. + if words.first().map(String::as_str) == Some("rows") { + return Ok(Job::Rows(words[1..].iter().flat_map(|w| w.split_whitespace()).map(String::from).collect())); + } let job = match words.first().map(String::as_str) { Some("host") => Job::Host(None), Some("seal") => Job::Seal, @@ -130,7 +139,8 @@ pub fn dispatch(root: &Path, args: &[String]) { Job::Toolchain => vec![step("the stores of this tree's toolchain", || release::toolchain(root))], Job::Bootstrap => vec![step("this tree's toolchain", || release::bootstrap(root))], Job::Image => image(root), - Job::Guest => guest(root, &suite_args(&["--jobs", "1"])), + Job::Guest => guest(root, &suite_args(&["--jobs", "1"]), true), + Job::Rows(args) => guest(root, &suite_args(&args.iter().map(String::as_str).collect::>()), false), Job::Release => vec![step("main's toolchain release", || release::release(root))], Job::Publish => vec![step("the SDK crates on crates.io", || publish(root))], }; @@ -1307,7 +1317,10 @@ fn suite_args(args: &[&str]) -> Vec { /// A guest job's own `$TMPDIR`, same rule as [`host`]: nothing the suite /// writes past a `toyos_tmpdir::TempDir` — the harness's own `Run`, its lanes, /// every boot image — survives past the last step, which reds on it. -fn guest(root: &Path, suite: &[String]) -> Vec { +/// +/// `install`: the sysroot the job restored ([`release::install`]). Without it +/// the suite builds from the host's own store, as a dev host does. +fn guest(root: &Path, suite: &[String], install: bool) -> Vec { let tmp = toyos_tmpdir::TempDir::new("ci-guest"); let short = Path::new(toyos_tmpdir::SHORT_BASE); let before = toyos_tmpdir::gone_roots(short); @@ -1319,7 +1332,7 @@ fn guest(root: &Path, suite: &[String]) -> Vec { .iter() .map(|&arch| step(&format!("the {} instrument", arch.name()), || instrument(root, arch))) .collect(); - if steps.iter().all(|s| s.verdict.is_ok()) { + if install && steps.iter().all(|s| s.verdict.is_ok()) { steps.push(step("the toolchain", || release::install(root))); } if steps.iter().all(|s| s.verdict.is_ok()) { @@ -1393,8 +1406,8 @@ fn instrument(root: &Path, arch: Arch) -> Result { if have != want { return Err(format!( "{line}: this runs QEMU {have} and build/ci/qemu-version declares {want}. The \ - container image's digest is what pins it, so moving it is a commit that says the \ - instrument moved" + host's QEMU is held to the declaration, and moving the declaration is a commit \ + that says the instrument moved" )); } if native && node && !arch.accel().is_hardware() { @@ -1804,6 +1817,10 @@ mod tests { assert_eq!(parse(&words("scope")), Ok(Job::Scope)); assert_eq!(parse(&words("image")), Ok(Job::Image)); assert_eq!(parse(&words("guest")), Ok(Job::Guest)); + assert_eq!(parse(&words("rows")), Ok(Job::Rows(vec![]))); + assert_eq!(parse(&words("rows virtsmp --jobs 1")), Ok(Job::Rows(words("virtsmp --jobs 1")))); + let one = vec!["rows".to_string(), " virtsmp --jobs 1 ".to_string()]; + assert_eq!(parse(&one), Ok(Job::Rows(words("virtsmp --jobs 1")))); assert_eq!(parse(&words("release")), Ok(Job::Release)); assert!(parse(&words("guest 3/12")).is_err()); assert!(parse(&words("tcg")).is_err()); @@ -1937,13 +1954,44 @@ mod tests { assert!(text.contains("\n host:"), "ci.yml runs no job `host`"); } - /// Every workflow's `pull_request:` trigger names `main` alone, and none - /// asks for a runner this project does not have — a self-hosted label - /// queues until it times out, silently. + /// The events a workflow's `on:` names, in its block form or either inline + /// form; a shape this does not read is refused. + fn triggers(name: &str, text: &str) -> Vec { + let line = text.lines().find(|l| l.starts_with("on:")).unwrap_or_else(|| panic!("{name}: no `on:`")); + let inline = line["on:".len()..].trim(); + if !inline.is_empty() { + return inline.trim_matches(['[', ']']).split(',').map(|t| t.trim().to_string()).collect(); + } + let block = text.split_once("\non:\n").expect("`on:` is a line of its own").1; + block + .lines() + .take_while(|l| l.is_empty() || l.starts_with(' ') || l.starts_with('#')) + .filter_map(|l| l.strip_prefix(" ")) + .filter(|l| !l.starts_with([' ', '#']) && !l.is_empty()) + .map(|l| l.split(':').next().unwrap_or("").trim().to_string()) + .collect() + } + + /// A `runs-on:` value naming one of GitHub's hosted images. Anything else — + /// a self-hosted label, a list, an expression, a value on later lines — is + /// treated as a self-hosted runner. + fn hosted(runner: &str) -> bool { + let runner = runner.trim(); + ["ubuntu-", "macos-", "windows-"].iter().any(|p| runner.starts_with(p)) + && runner.chars().all(|c| c.is_ascii_alphanumeric() || matches!(c, '-' | '.')) + } + + /// Every workflow's `pull_request:` trigger names `main` alone, and a job + /// off GitHub's hosted runners is only in a workflow `workflow_dispatch` + /// alone starts: a dispatch needs write access to the repository, and no + /// pull request, its target, a push, a schedule or another workflow's call + /// reaches the self-hosted runner from this repository's own workflows, + /// directly or through a reusable workflow. #[test] - fn workflows_run_against_main_on_hosted_runners() { + fn only_a_dispatch_reaches_a_self_hosted_runner() { let dir = repo_root().join(".github/workflows"); let mut seen = 0; + let mut self_hosted = Vec::new(); for entry in std::fs::read_dir(&dir).expect(".github/workflows is readable").flatten() { let text = std::fs::read_to_string(entry.path()).expect("a readable workflow"); let name = entry.file_name().to_string_lossy().into_owned(); @@ -1952,14 +2000,34 @@ mod tests { let branches = after.lines().next().unwrap_or("").trim(); assert_eq!(branches, "branches: [main]", "{name}"); } + let on = triggers(&name, &text); + assert!(!on.is_empty(), "{name}: no trigger read"); + // A reusable workflow runs its jobs in this repository's context: + // one from elsewhere could name the self-hosted label. + for called in text.lines().filter_map(|l| l.strip_prefix(" uses:")) { + assert!(called.trim().starts_with("./.github/workflows/"), "{name}: a job uses{called}"); + } for runner in text.lines().filter_map(|l| l.trim_start().strip_prefix("runs-on:")) { - assert!( - !runner.contains("self-hosted") && !runner.contains("toyos"), - "{name}: runs-on:{runner}" - ); + if !hosted(runner) { + assert_eq!(on, ["workflow_dispatch"], "{name}: runs-on:{runner} is started by {on:?}"); + self_hosted.push(name.clone()); + } } } - assert_eq!(seen, 5, "ci.yml, nightly.yml and publish.yml, and guest.yml and toolchain.yml"); + assert_eq!(self_hosted, ["guest-rows.yml"], "the workflows that reach a self-hosted runner"); + assert_eq!(seen, 6, "ci.yml, nightly.yml, publish.yml and guest-rows.yml, and guest.yml and toolchain.yml"); + } + + #[test] + fn the_trigger_and_runner_readers_read_what_they_refuse() { + let block = "name: x\non:\n # why\n pull_request:\n branches: [main]\n merge_group:\n\njobs:\n"; + assert_eq!(triggers("x", block), ["pull_request", "merge_group"]); + assert_eq!(triggers("x", "on: [push, pull_request_target]\n"), ["push", "pull_request_target"]); + assert_eq!(triggers("x", "on: workflow_dispatch\n"), ["workflow_dispatch"]); + assert!(hosted(" ubuntu-24.04") && hosted(" macos-latest")); + for runner in [" [self-hosted, toyos-guest]", " toyos-guest", " ${{ inputs.runner }}", "", " ubuntu-24.04 # x"] { + assert!(!hosted(runner), "{runner:?}"); + } } #[test] diff --git a/issues/the-build-runs-host-tools-outside-rust-and-qemu.md b/issues/the-build-runs-host-tools-outside-rust-and-qemu.md index 1b84493ae62..e89a17e6cdb 100644 --- a/issues/the-build-runs-host-tools-outside-rust-and-qemu.md +++ b/issues/the-build-runs-host-tools-outside-rust-and-qemu.md @@ -22,9 +22,10 @@ arrives and is not one. M4 and M5 are stages of `issues/toyos-builds-itself.md`. | `git log` and `git status` for os-release | the commit, its committer time and whether the tree is that commit's, which `build/build.rs`'s `release` writes into `/system/etc/os-release` in every image build | admitted: the owner's ruling on the dependency audit (2026-10-09), "Drop gix, use git", which covers these two reads and nothing else this table names: they were gitoxide's, gitoxide is no tool the build sheds while `git` stays for the worktree row, and for them it held 84 of the root `Cargo.lock`'s packages | M4 runs it in the guest | | `cc`, `c++` and `ar` on a Linux host, `build-essential` in CI's containers | rustc links every host binary through `cc`; `cc` and `c++` compile LLVM, clang, LLD and `rustc_llvm` (`build/llvm.rs` names both to bootstrap), and the tablegens of the ToyOS-hosted clang's `NATIVE` build (`build/hostedclang.rs`); `cc` compiles ring's C and assembly in every `--ci host` run, as the dev-dependency oracle of `toyos-crypto` and `toyos-update` (`lib/crypto`, `lib/update`); `ar` archives what `cc::Build` compiles, ring's included | admitted: no Rust tool compiles C or C++, or takes rustc's host link; ring is the independent oracle the crypto crate is held to, and the build's own HTTP agent and doom's build script are on `toyos-crypto`'s provider (#1020) | M5: no host in the loop | | the toolchain's own `clang`, `llvm-ar`, `rust-lld` and `llvm-config`, built from `ToyOSOrg/llvm-project` | rustc links every guest binary with `rust-lld`; `clang` compiles the C corpus (`tests/common/compile.rs`) and, with `llvm-ar`, through `cc::Build` the C of every guest crate that has any (`build/clang.rs`, `build/build.rs`'s `GuestEnv`): doomgeneric, and ring's C and assembly in a test crate's `https_get` and `ring_kat`; rustc's bootstrap asks `llvm-config` how to link LLVM | admitted: our fork's C++, which ToyOS can one day build and run; no Rust tool compiles C, `cc::Build` archives with an `ar`, bootstrap reads LLVM through `llvm-config`, and `CLAUDE.md` links everything with `rust-lld` | M5: no host in the loop | -| `ovmf-generic`, `qemu-efi-aarch64` | the x86-64 and AArch64 UEFI firmware of CI's guest containers (`build/firmware.rs`), packaged by Debian apart from QEMU | admitted: QEMU's own firmware, and no Rust firmware does its job | the instrument's QEMU carries its own firmware | +| `ovmf-generic`, `qemu-efi-aarch64` | the x86-64 and AArch64 UEFI firmware of CI's guest containers and of the guest runner's host (`build/firmware.rs`), packaged by Debian apart from QEMU | admitted: QEMU's own firmware, and no Rust firmware does its job | the instrument's QEMU carries its own firmware | | `ca-certificates` | the trust store `git` and `curl` verify against in CI's containers | admitted: data both of them need | goes when neither runs there | | Debian's `sid` image, Docker Hub's `sid-20260918` copied once to `ghcr.io/toyosorg/debian` and pulled with the run's token | the container of `guest.yml`'s suite, pinned to digest `a2aa4626…`, whose dated snapshot archive `deps` installs QEMU from; and, pulled anonymously from ECR Public's copy of Docker Hub's, of `nightly.yml`'s `portability-linux`, `sid` as it stands | admitted: the runner's Ubuntu 24.04 publishes QEMU 8.2.2 (Launchpad, noble, every pocket) and `build/ci/qemu-version` declares 11.1.1, which that snapshot archive carries | the runner's own OS carries the declared QEMU, or the build system builds it | +| GitHub's Actions runner on the guest runner's host | takes `guest-rows.yml`'s jobs | admitted: the owner's ruling (2026-11-04) runs guest tests on a dedicated self-hosted runner; it is the agent every GitHub-hosted job already runs under | goes with GitHub Actions | | Docker on the runner | Actions pulls and runs every `container:` image with it, those two jobs' | refused: Go, not C or C++ ToyOS can one day build and run, and Actions runs a `container:` with Docker alone | the container goes with the image row | | `skopeo` | `skopeo copy --all --preserve-digests` copied Docker Hub's `sid-20260918` index, every manifest by its digest, to `ghcr.io/toyosorg/debian`, once, from a `run:` step of a workflow that ran on a branch and is not in `main`; a new pin is a new copy made the same way, and the copy it replaces is deleted | refused: Go, not C or C++ ToyOS can one day build and run | a Rust tool makes the copy, or the container goes with the image row | | a bench's Ubuntu and every tool `tests/metal/src/lib.rs` runs on it over `ssh` (`sgdisk`, `partx` and `lsblk` among them, for the internal install) | the metal loop and the internal install, on a bench and never on a development host | outside the rule: recovery equipment on a test machine, not the build's host | they leave with Ubuntu (`issues/the-machine-updates-itself-without-ubuntu.md`) | diff --git a/issues/the-nightly-is-frozen-so-nothing-runs-the-guest-matrix.md b/issues/the-nightly-is-frozen-so-nothing-runs-the-guest-matrix.md new file mode 100644 index 00000000000..dffd631c765 --- /dev/null +++ b/issues/the-nightly-is-frozen-so-nothing-runs-the-guest-matrix.md @@ -0,0 +1,18 @@ +--- +status: assigned +kind: tooling +opened: 2026-11-04 +--- + +# The nightly is frozen, so nothing runs the guest matrix or the aarch64 rows a PR does not run + +The owner froze the nightly on 2026-11-04 and disabled `nightly.yml` on +GitHub. Root `CLAUDE.md` says the broad integration matrix runs nightly and +that nightly covers the aarch64 guest rows a PR does not run; while it is +frozen, neither happens. Its other jobs stop with it: the host cache's +writer, main's toolchain release, and the fresh-machine and macOS builds. + +Guest rows run only where an agent dispatches them on the guest runner +(`guest-rows.yml`), so a regression in a row no PR selected goes unseen. + +**Exit**: the nightly resumes. Held by the owner. diff --git a/tests/CLAUDE.md b/tests/CLAUDE.md index 0746f958b65..66c45c20bec 100644 --- a/tests/CLAUDE.md +++ b/tests/CLAUDE.md @@ -2,6 +2,10 @@ The mechanics live where the work is: profiles and shapes in `tests/common/`, registration in `tests/toyos.rs` — read those, not this file, for how the harness works. +## Where guests run + +Guest rows run on the guest runner, a dedicated idle host that takes one job at a time: `gh workflow run guest-rows.yml -f rev= -f filter=''`, found by its title in `gh run list --workflow guest-rows.yml` and read with `gh run view --log`. A run that other work on its host shared ends `HOST NOT QUIET` with exit 3: neither red nor green, and re-run. + ## Caveats that bite every agent - **A host that sleeps mid-run is reported, not measured** — a run whose wall clock jumped against the monotonic one reports `INVL` per test and exits 2. A wild outlier *not* marked that way is a real finding. diff --git a/tests/checks.rs b/tests/checks.rs index 9eb714bf8b4..20aea2a0cb9 100644 --- a/tests/checks.rs +++ b/tests/checks.rs @@ -16,6 +16,8 @@ mod checks { mod claims_checks; #[path = "clock.rs"] mod clock_checks; + #[path = "hostload.rs"] + mod hostload_checks; #[path = "metal.rs"] mod metal_checks; #[path = "qemu.rs"] @@ -105,6 +107,11 @@ mod checks { clock_checks::self_check() } + #[test] + fn other_work_detector() -> Result<(), String> { + hostload_checks::self_check() + } + /// What a suspend is worth to a verdict, staged rather than reasoned about. /// /// `clock_checks::self_check` gates the detector; this gates what the suite @@ -909,6 +916,22 @@ mod checks { return Err(format!("a suspended run exits {}, and it has to be 2", suspended.exit_code())); } + // Other work beside the run outranks a red, and a quiet host with other + // work under a CPU on average changes nothing. + let busy = common::hostload::Reading { other: Duration::from_secs(61), over: Duration::from_secs(60) }; + let calm = common::hostload::Reading { other: Duration::from_secs(59), over: Duration::from_secs(60) }; + let mut noisy = Tally::new(); + noisy.record(outcome("a_red", Some("the disk came back short"), Duration::ZERO)); + noisy.host = Some(busy); + let text = noisy.summary(1, Duration::from_secs(60), Duration::ZERO); + if noisy.exit_code() != 3 || !text.contains("test result: HOST NOT QUIET.") { + return Err(format!("a run beside other work exits {}:\n{text}", noisy.exit_code())); + } + noisy.host = Some(calm); + if noisy.exit_code() != 1 { + return Err(format!("a red on a quiet host exits {}, and it has to be 1", noisy.exit_code())); + } + // The clean case, so that none of the above is passing because everything // reds. let mut clean = Tally::new(); diff --git a/tests/checks/hostload.rs b/tests/checks/hostload.rs new file mode 100644 index 00000000000..0c405ff31ad --- /dev/null +++ b/tests/checks/hostload.rs @@ -0,0 +1,81 @@ +//! The other-work reading, against a child that spins: other work until it is +//! reaped, this run's after. + +use std::time::Duration; + +use toyos_build::eprintln; + +use crate::common::hostload; + +/// The CPU time the staged other work spins for. +const SPIN: Duration = Duration::from_secs(1); + +pub fn self_check() -> Result<(), String> { + let start = hostload::mark(); + // SAFETY: the child calls only `clock_gettime` and `_exit`, both + // async-signal-safe, so a fork of this threaded process is sound. + let pid = unsafe { libc::fork() }; + if pid == 0 { + let mut now = libc::timespec { tv_sec: 0, tv_nsec: 0 }; + loop { + unsafe { libc::clock_gettime(libc::CLOCK_PROCESS_CPUTIME_ID, &mut now) }; + if now.tv_sec as u64 >= SPIN.as_secs() { + unsafe { libc::_exit(0) }; + } + } + } + if pid < 0 { + return Err(format!("fork: {}", std::io::Error::last_os_error())); + } + // Exited, and not yet reaped: its time is in no `getrusage` of ours. + let mut info: libc::siginfo_t = unsafe { std::mem::zeroed() }; + // SAFETY: `info` is a valid out-pointer and `pid` is this process's child. + if unsafe { libc::waitid(libc::P_PID, pid as libc::id_t, &mut info, libc::WEXITED | libc::WNOWAIT) } != 0 { + return Err(format!("waitid: {}", std::io::Error::last_os_error())); + } + let unreaped = start.reading(); + let (_, total, cpus) = start.host_since(); + let mut ru: libc::rusage = unsafe { std::mem::zeroed() }; + let mut status = 0; + // SAFETY: valid out-pointers, and the child is exited. + if unsafe { libc::wait4(pid, &mut status, 0, &mut ru) } != pid { + return Err(format!("wait4: {}", std::io::Error::last_os_error())); + } + let reaped = start.reading(); + let tv = |t: libc::timeval| Duration::new(t.tv_sec as u64, t.tv_usec as u32 * 1_000); + let spun = tv(ru.ru_utime) + tv(ru.ru_stime); + + // The counters' unit: every CPU accrues a tick of some kind per tick of + // wall clock, whatever the load. + let per_cpu = total.as_secs_f64() / f64::from(cpus) / unreaped.over.as_secs_f64(); + if !(0.9..=1.1).contains(&per_cpu) { + return Err(format!( + "the host's counters summed {total:?} over {cpus} CPU(s) in {:?}, {per_cpu:.2} of \ + wall clock per CPU: they are not in the unit read", + unreaped.over + )); + } + // A tick per CPU at each end is what the counters can be off by. + let slack = hostload::ticks(2) * cpus; + if spun < SPIN / 2 { + return Err(format!("the child spun {spun:?} of CPU, and it was to spin {SPIN:?}")); + } + if unreaped.other + slack < spun { + return Err(format!( + "an unreaped child spun {spun:?} and the reading saw {:?} of other work", + unreaped.other + )); + } + if reaped.other + spun > unreaped.other + slack { + return Err(format!( + "reaping a child that spun {spun:?} took the other work from {:?} to {:?}: its time \ + is not counted as this run's", + unreaped.other, reaped.other + )); + } + eprintln!( + " [hostload] a child's {spun:.2?} read as other work until reaped ({:.2?} → {:.2?})", + unreaped.other, reaped.other + ); + Ok(()) +} diff --git a/tests/common/hostload.rs b/tests/common/hostload.rs new file mode 100644 index 00000000000..4209ba28902 --- /dev/null +++ b/tests/common/hostload.rs @@ -0,0 +1,163 @@ +//! Whether the host did work besides this run's. +//! +//! A guest's vCPUs are admitted against the host's CPUs +//! (`buildlock::guest_cpus`); work that is not this run's takes them, and the +//! guest is descheduled at the host's pace. The kernel's own counters give the +//! host's busy CPU time (`/proc/stat`, Mach's `HOST_CPU_LOAD_INFO`), and +//! `getrusage` gives this run's: itself and every child it has reaped. The +//! difference is the other work. +//! +//! Read only once every child is reaped: a live child's time is not yet in +//! `RUSAGE_CHILDREN`, so it reads as other work. + +use std::time::{Duration, Instant}; + +/// Both readings at one moment. +pub struct Mark { + at: Instant, + host: Host, + own: Duration, +} + +/// What other work used between a [`Mark`] and the reading. +#[derive(Clone, Copy, Debug)] +pub struct Reading { + pub other: Duration, + pub over: Duration, +} + +impl Reading { + /// Other work averaging at most one host CPU. Past that, at least one CPU + /// the guest budget admitted guests against was not theirs for the whole + /// run. + pub fn quiet(&self) -> bool { + self.other <= self.over + } + + /// The other work's CPU time, as host CPUs busy on average. + pub fn cpus(&self) -> f64 { + self.other.as_secs_f64() / self.over.as_secs_f64().max(f64::MIN_POSITIVE) + } +} + +pub fn mark() -> Mark { + Mark { at: Instant::now(), host: Host::read(), own: own() } +} + +impl Mark { + pub fn reading(&self) -> Reading { + let (host, own) = (Host::read(), own()); + let (busy, _) = host.since(&self.host); + Reading { other: busy.saturating_sub(own.saturating_sub(self.own)), over: self.at.elapsed() } + } + + /// The host's busy and total CPU time since this mark, and the CPUs they + /// sum over. + #[allow(dead_code, reason = "the self-check's")] + pub(crate) fn host_since(&self) -> (Duration, Duration, u32) { + let now = Host::read(); + let (busy, total) = now.since(&self.host); + (busy, total, now.cpus) + } +} + +/// This process's CPU time and every reaped descendant's. +fn own() -> Duration { + let usage = |who| { + let mut ru: libc::rusage = unsafe { std::mem::zeroed() }; + // SAFETY: `ru` is a valid out-pointer for the call. + assert_eq!(unsafe { libc::getrusage(who, &mut ru) }, 0, "getrusage"); + let tv = |t: libc::timeval| Duration::new(t.tv_sec as u64, t.tv_usec as u32 * 1_000); + tv(ru.ru_utime) + tv(ru.ru_stime) + }; + usage(libc::RUSAGE_SELF) + usage(libc::RUSAGE_CHILDREN) +} + +/// `ticks` of the counters' unit, `USER_HZ` on Linux and the scheduler's tick +/// on macOS. +pub(crate) fn ticks(ticks: u64) -> Duration { + // SAFETY: no preconditions. + let hz = unsafe { libc::sysconf(libc::_SC_CLK_TCK) }; + assert!(hz > 0, "sysconf(_SC_CLK_TCK) said {hz}"); + Duration::from_nanos(ticks.saturating_mul(1_000_000_000 / hz as u64)) +} + +/// The host's cumulative CPU counters, summed over its CPUs. +struct Host { + /// Linux: user nice system idle iowait irq softirq steal. macOS: user + /// system idle nice, 32-bit and wrapping. + fields: Vec, + cpus: u32, +} + +#[cfg(target_os = "linux")] +impl Host { + /// `/proc/stat`'s aggregate line. Its guest and guest_nice fields are + /// left out because user and nice already include them. + fn read() -> Host { + let stat = std::fs::read_to_string("/proc/stat").expect("/proc/stat is readable"); + let line = stat.lines().find(|l| l.starts_with("cpu ")).expect("/proc/stat has a cpu line"); + let fields: Vec = line + .split_whitespace() + .skip(1) + .take(8) + .map(|f| f.parse().expect("/proc/stat's cpu fields are numbers")) + .collect(); + assert_eq!(fields.len(), 8, "/proc/stat's cpu line: {line}"); + let cpus = stat + .lines() + .filter(|l| l.strip_prefix("cpu").is_some_and(|n| n.starts_with(|c: char| c.is_ascii_digit()))) + .count() as u32; + Host { fields, cpus } + } + + /// Busy and total time since `earlier`. Idle and iowait are idle; steal is + /// busy, because a hypervisor ran something else. + fn since(&self, earlier: &Host) -> (Duration, Duration) { + let d: Vec = self.fields.iter().zip(&earlier.fields).map(|(a, b)| a - b).collect(); + let total: u64 = d.iter().sum(); + (ticks(total - d[3] - d[4]), ticks(total)) + } +} + +#[cfg(target_os = "macos")] +impl Host { + fn read() -> Host { + // libc's own declarations of these are deprecated for the `mach2` crate. + extern "C" { + static mach_task_self_: libc::mach_port_t; + fn mach_host_self() -> libc::mach_port_t; + fn mach_port_deallocate(task: libc::mach_port_t, name: libc::mach_port_t) -> libc::c_int; + } + let mut info = libc::host_cpu_load_info { cpu_ticks: [0; libc::CPU_STATE_MAX as usize] }; + let mut count = libc::HOST_CPU_LOAD_INFO_COUNT; + // SAFETY: `info` is the flavour's structure and `count` its size in + // `integer_t`s; the host port's send right is released after the call. + let status = unsafe { + let host = mach_host_self(); + let status = libc::host_statistics( + host, + libc::HOST_CPU_LOAD_INFO, + (&raw mut info).cast(), + &mut count, + ); + mach_port_deallocate(mach_task_self_, host); + status + }; + assert_eq!(status, 0, "host_statistics(HOST_CPU_LOAD_INFO)"); + // SAFETY: no preconditions. + let cpus = unsafe { libc::sysconf(libc::_SC_NPROCESSORS_ONLN) } as u32; + Host { fields: info.cpu_ticks.map(u64::from).to_vec(), cpus } + } + + fn since(&self, earlier: &Host) -> (Duration, Duration) { + let d: Vec = self + .fields + .iter() + .zip(&earlier.fields) + .map(|(&a, &b)| u64::from((a as u32).wrapping_sub(b as u32))) + .collect(); + let total: u64 = d.iter().sum(); + (ticks(total - d[libc::CPU_STATE_IDLE as usize]), ticks(total)) + } +} diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 07513beef62..611898dd3ab 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -2,6 +2,7 @@ pub mod audio; /// A claimed function at the unit, on the T14. pub mod claims; pub mod clock; +pub mod hostload; pub mod lane; pub mod compile; /// The device boot: what `tests/images/metaldevicecase.toml` measures. diff --git a/tests/toyos.rs b/tests/toyos.rs index 059211fe704..955e2bda3b6 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -8608,6 +8608,8 @@ struct Tally { invalid: Vec<(String, Duration)>, /// Every outcome's `elapsed`, summed. tested: Duration, + /// What other work used on the host over the run, once it has ended. + host: Option, } impl Tally { @@ -8618,6 +8620,7 @@ impl Tally { stalls: Vec::new(), invalid: Vec::new(), tested: Duration::ZERO, + host: None, } } @@ -8636,9 +8639,14 @@ impl Tally { } } - /// **Three statuses**: 0 green, 1 red, and 2 when the run established - /// nothing, because the host stopped in the middle of it. + /// **Four statuses**: 0 green, 1 red, 2 when the host stopped in the + /// middle of a test, and 3 when other work on the host shared it with the + /// guests. A run on a host that was not quiet establishes nothing, reds + /// included. fn exit_code(&self) -> i32 { + if self.host.is_some_and(|r| !r.quiet()) { + return 3; + } if !self.failures.is_empty() { return 1; } @@ -8679,6 +8687,13 @@ impl Tally { "host: {} core(s); a guest wider than that waits vcpus/cores longer again", qemu::host_cores() )); + if let Some(host) = self.host { + say(format!( + "host: other work used {:.0?} of CPU beside this run, {:.2} CPUs on average", + host.other, + host.cpus() + )); + } if suspended >= common::clock::SUSPENDED_AT_LEAST { // The elapsed figure below is monotonic and therefore already // excludes it, which is worth saying: the two numbers do not add up @@ -8739,6 +8754,21 @@ impl Tally { .to_string(), ); } + 3 => { + say(format!( + "test result: HOST NOT QUIET. {} passed, {} failed, {} invalidated, \ + {total} total ({elapsed})", + self.passed, + self.failures.len(), + self.invalid.len(), + )); + say( + "This is not a red or a green. Other work on the host took more than a CPU \ + from the guests, so no verdict here is this tree's alone; re-run on a quiet \ + host." + .to_string(), + ); + } _ => say(format!( "test result: ok. {} passed, {total} total ({elapsed})", self.passed @@ -9023,9 +9053,9 @@ fn main() { } }; - // Three statuses for the three things this can establish, as the - // ordinary suite has: green, red, and "measured nothing" — a run that - // staged images and never reached the machine has no claim to make. + // Three statuses for the three things this can establish: green, red, + // and "measured nothing" — a run that staged images and never reached + // the machine has no claim to make. run.exit( match metal::run(mode, parsed.metal_host, &selected, &boots, &rust_bins, !nocapture && !debug_mode) { @@ -9062,24 +9092,13 @@ fn main() { let mut tally = Tally::new(); let suite_start = common::clock::mark(); + let host_start = common::hostload::mark(); let tasks = build_tasks(&machine_to_run, &screen_to_run); let total = tasks.len(); eprintln!("\nrunning {total} tests, {width} wide\n"); run_tasks(tasks, width, &test_config).into_iter().for_each(|o| tally.record(o)); - // Three exit statuses, because there are three things a run can establish — - // see [`Tally::exit_code`], which is where the whole decision now lives. - // - // Nor may it be 1. A red sends an agent hunting a defect, and the defect is - // not there — the lid was closed. CLAUDE.md already documents the signature - // and documents it as something a *human* must notice before recording a - // finding, which is exactly the judgement a status code should carry - // instead. So: 2, with a headline that names it and says re-run. - // - // A run with both real failures and invalidated tests exits 1: a red that - // survives is still a red, and re-running the suspended ones does not make - // it green. // What this run cost cargo: a kernel build is ~6.9 s of wall clock and // ~29.6 s of CPU after any edit under `kernel/`, and a full run used to // make 45 of them. @@ -9090,6 +9109,8 @@ fn main() { kernels.len(), ); + // Every worker has joined and every guest and build it started is reaped. + tally.host = Some(host_start.reading()); let summary = tally.summary(total, suite_start.elapsed(), suite_start.suspended()); summary.lines().for_each(|line| eprintln!("{line}")); run.exit(tally.exit_code());