From 7e9c0c2a6f81c0ebb788990582577bf01f4598fe Mon Sep 17 00:00:00 2001 From: beatzaplenty Date: Mon, 20 Jul 2026 17:25:22 +0000 Subject: [PATCH] Rewrite codex-maintenance.sh to scope CI checks to changed files CI was running a full eval of every host + package on every push/PR, which was slow enough to routinely time out the Gitea runner. Default mode now diffs against a base ref and scopes nixpkgs-fmt/statix/eval to the files that changed and the hosts/packages they can affect; a change to flake.nix/flake.lock/variables.nix/modules/common/* (or any other modules/*.nix outside platforms//build-types, whose blast radius isn't inferable from the path) falls back to evaluating everything. The old full sweep moves behind --full-check, which CI never passes; --dry-run adds build-planning on top of whichever scope is active. Also trims codex-setup.sh's redundant full host eval loop -- that's what codex-maintenance.sh is for; setup should just install tooling. --- .gitea/workflows/check-nixos.yml | 10 +- .github/workflows/check-nixos.yml | 10 +- AGENTS.md | 7 +- CLAUDE.md | 51 ++++-- README.md | 11 +- docs/flake-lock-automation.md | 11 +- scripts/bump-nixpkgs-release.sh | 5 +- scripts/codex-maintenance.sh | 284 +++++++++++++++++++++++++----- scripts/codex-setup.sh | 11 +- 9 files changed, 323 insertions(+), 77 deletions(-) diff --git a/.gitea/workflows/check-nixos.yml b/.gitea/workflows/check-nixos.yml index 2e8db3c..a16ddf2 100644 --- a/.gitea/workflows/check-nixos.yml +++ b/.gitea/workflows/check-nixos.yml @@ -13,9 +13,17 @@ jobs: steps: - name: Check out repository uses: actions/checkout@v4 + with: + fetch-depth: 0 - name: Install Nix uses: DeterminateSystems/nix-installer-action@v19 - - name: Run maintenance checks (secrets, fmt, lint, eval) + # Scoped to files changed since the PR base / previous push -- see + # scripts/codex-maintenance.sh. CI never passes --full-check: that + # full sweep is for local/manual use, since it's slow enough to time + # out this runner. + - name: Run maintenance checks (secrets, fmt, lint, eval -- changed files only) + env: + MAINT_BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.before }} run: bash scripts/codex-maintenance.sh diff --git a/.github/workflows/check-nixos.yml b/.github/workflows/check-nixos.yml index 2e8db3c..a16ddf2 100644 --- a/.github/workflows/check-nixos.yml +++ b/.github/workflows/check-nixos.yml @@ -13,9 +13,17 @@ jobs: steps: - name: Check out repository uses: actions/checkout@v4 + with: + fetch-depth: 0 - name: Install Nix uses: DeterminateSystems/nix-installer-action@v19 - - name: Run maintenance checks (secrets, fmt, lint, eval) + # Scoped to files changed since the PR base / previous push -- see + # scripts/codex-maintenance.sh. CI never passes --full-check: that + # full sweep is for local/manual use, since it's slow enough to time + # out this runner. + - name: Run maintenance checks (secrets, fmt, lint, eval -- changed files only) + env: + MAINT_BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.before }} run: bash scripts/codex-maintenance.sh diff --git a/AGENTS.md b/AGENTS.md index 871437e..a84fb12 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -35,9 +35,14 @@ Use these commands when validating changes: ```bash bash scripts/codex-setup.sh bash scripts/codex-maintenance.sh -bash scripts/codex-maintenance.sh dry-run ``` +With no flags, `codex-maintenance.sh` scopes fmt-check/statix/eval to files +changed against a base ref — this is what CI runs on every push/PR. For the +full sweep (every host, every package — slow; CI never runs this), use +`bash scripts/codex-maintenance.sh --full-check` (add `--dry-run` for build +planning on top of whichever scope is active). + Host evaluation is safe when limited to drvPath checks: ```bash diff --git a/CLAUDE.md b/CLAUDE.md index 52d2589..ccd4b87 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,11 +37,22 @@ machines when deployed. # One-time environment bootstrap (installs Nix if missing, prints hosts) bash scripts/codex-setup.sh -# Full validation: secret grep, nixpkgs-fmt --check, statix lint, eval all hosts +# Changed-files-only validation: secret grep (whole repo), nixpkgs-fmt --check +# and statix on changed *.nix files, eval of the hosts/packages those changes +# can affect. This is what CI runs on every push/PR. bash scripts/codex-maintenance.sh -# Same, plus a dry-run build (no result symlink) of every host's toplevel -bash scripts/codex-maintenance.sh dry-run +# Full sweep: nixpkgs-fmt --check/statix over the whole tree, eval every host +# and package. Slow (minutes) -- CI never runs this; use it locally before a +# release or after touching modules/common/*, flake.nix, or variables.nix for +# extra confidence beyond the automatic full-fallback those paths already +# trigger in the default mode (see below). +bash scripts/codex-maintenance.sh --full-check + +# Either mode, plus a dry-run build (no result symlink) of every host/package +# in whichever scope is active +bash scripts/codex-maintenance.sh --dry-run +bash scripts/codex-maintenance.sh --full-check --dry-run # List the hosts the flake currently exposes nix eval --json .#nixosConfigurations --apply builtins.attrNames | jq -r '.[]' @@ -58,18 +69,28 @@ maintenance script pulls them via `nix run github:NixOS/nixpkgs/nixos-25.11#` you touched (plus any `config.system.build.tarball` -/`diskoImagesScript`/package output affected) rather than looping over every -host — `codex-maintenance.sh` evaluates every `nixosConfigurations` host plus -every package/tarball/image variant and is slow to run after each small -change. Reserve a full -`codex-maintenance.sh` run for changes that plausibly affect every host -(`modules/common/*`, `flake.nix`, `variables.nix`) or as a final check before -committing. This is a session-workflow preference only — it does not apply to -CI, which should keep running the full script on every push/PR regardless of -diff size; that's the point of it. +With no flags, `codex-maintenance.sh` diffs against a base ref (env +`MAINT_BASE_SHA`, else the PR base SHA in CI, else `HEAD^` locally) and scopes +fmt-check/statix to the changed `*.nix` files and eval to the hosts/packages +those changes can affect — a `hosts//host.nix` edit only evals that +host's targets, a `modules/platforms/.nix` edit only evals that +platform's hosts, and so on. A change to `flake.nix`, `flake.lock`, +`variables.nix`, `modules/common/*`, or any other `modules/*.nix` file outside +`platforms/`/`build-types/` (whose blast radius isn't safely inferable from +the path alone) falls back to evaluating every host and package, same as +`--full-check` would, just without the whole-tree fmt/statix sweep. This +exists because the whole-tree sweep is what was timing out CI; **CI always +runs the plain, no-flag form and never passes `--full-check`.** + +The default mode's diff is against the working tree (uncommitted and staged +edits included, not just committed ones), so it's already the right tool for +an interactive session too: after editing one or two hosts/modules, plain +`bash scripts/codex-maintenance.sh` naturally scopes to just what you +touched. Reserve `--full-check` for changes that plausibly affect every host +(`modules/common/*`, `flake.nix`, `variables.nix` — though the default mode +already falls back to evaluating everything for those paths, `--full-check` +additionally re-checks fmt/statix over the whole tree) or as a final check +before committing. ## Scripts diff --git a/README.md b/README.md index b47631f..04d3d2b 100644 --- a/README.md +++ b/README.md @@ -74,10 +74,19 @@ Safe validation commands for Codex and local review: ```bash bash scripts/codex-setup.sh -bash scripts/codex-maintenance.sh dry-run bash scripts/codex-maintenance.sh ``` +`codex-maintenance.sh` with no flags (what CI runs on every push/PR) scopes +fmt-check/statix/eval to files changed against a base ref — fast, but only +as thorough as the diff. For the full sweep (every host, every package, +fmt-check and statix over the whole tree — slow, CI never runs this): + +```bash +bash scripts/codex-maintenance.sh --full-check +bash scripts/codex-maintenance.sh --full-check --dry-run +``` + For individual host evaluation: ```bash diff --git a/docs/flake-lock-automation.md b/docs/flake-lock-automation.md index f17e008..d003ad0 100644 --- a/docs/flake-lock-automation.md +++ b/docs/flake-lock-automation.md @@ -8,9 +8,14 @@ and to verify that declared NixOS hosts still evaluate after dependency updates. - A scheduled workflow runs `nix flake update` once per week. - On GitHub, any resulting `flake.lock` change is proposed through a pull request. - On Gitea, the workflow can commit and push `flake.lock` directly when PR automation is not configured. -- A separate CI workflow evaluates every configured host before merge, listed - dynamically via `nix eval --json .#nixosConfigurations --apply builtins.attrNames` - rather than hand-enumerated, so it can't drift as `-` +- A separate CI workflow runs `scripts/codex-maintenance.sh` before merge. + Its default mode scopes eval to the hosts/packages a change can affect, + determined from a git diff against the PR base — but a `flake.lock` change + is treated as repo-wide and always falls back to evaluating every host, so + a lock-file update PR still gets full coverage. Hosts are still listed + dynamically via + `nix eval --json .#nixosConfigurations --apply builtins.attrNames` rather + than hand-enumerated, so that fallback can't drift as `-` targets are added or removed. See `README.md` for the current target list. ## Why hosts should stop using `--upgrade-all` diff --git a/scripts/bump-nixpkgs-release.sh b/scripts/bump-nixpkgs-release.sh index cefa85c..60a2e53 100755 --- a/scripts/bump-nixpkgs-release.sh +++ b/scripts/bump-nixpkgs-release.sh @@ -129,5 +129,6 @@ echo "flake.lock still points at the old input revisions until refreshed. Either echo " nix flake update nixpkgs home-manager # just these two inputs" echo " nix flake update # everything — see docs/flake-lock-automation.md" echo -echo "Then run 'bash scripts/codex-maintenance.sh dry-run' before committing —" -echo "a channel bump can shift option defaults across every host." +echo "Then run 'bash scripts/codex-maintenance.sh --full-check --dry-run' before" +echo "committing — a channel bump can shift option defaults across every host," +echo "and only --dry-run actually builds anything to catch that." diff --git a/scripts/codex-maintenance.sh b/scripts/codex-maintenance.sh index 8e5f845..1da37d4 100755 --- a/scripts/codex-maintenance.sh +++ b/scripts/codex-maintenance.sh @@ -1,4 +1,21 @@ #!/usr/bin/env bash +# Validation entry point for CI and local/agent review. +# +# Default mode (what CI runs on every push/PR): fmt-check, statix, and eval +# are scoped to files that actually changed against a base ref, plus +# whichever hosts/packages those changes can affect. This exists because +# the unscoped sweep below is slow enough to time out CI runners -- see +# --full-check. +# +# --full-check: the historical full sweep (every host, every package, +# fmt --check ./statix check . over the whole tree). Slow -- minutes, not +# seconds. CI never passes this; run it locally before a release or after +# touching modules/common/*, flake.nix, or variables.nix if you want extra +# confidence beyond what the changed-files scope already covers for those +# paths (see below). +# +# --dry-run: adds `nix build --dry-run --no-link` for whatever scope is +# active (changed-files scope by default, full scope under --full-check). set -euo pipefail script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -7,7 +24,39 @@ source "${script_dir}/lib/nix-bootstrap.sh" # shellcheck source=lib/nix-eval.sh source "${script_dir}/lib/nix-eval.sh" -MODE="${1:-validate}" +repo_root="$(cd "${script_dir}/.." && pwd)" +cd "$repo_root" + +full_check=false +dry_run=false + +usage() { + cat <<'EOF' +Usage: scripts/codex-maintenance.sh [--full-check] [--dry-run] + + --full-check Run the full sweep: fmt-check and statix over the whole + repo, eval every host and package. Slow. Never run by CI. + --dry-run Additionally run `nix build --dry-run --no-link` for + whatever scope is active. + +With neither flag (the CI default), fmt-check/statix/eval are scoped to +files changed against a base ref (env MAINT_BASE_SHA, else the PR base, +else HEAD^), plus the hosts/packages those changes can affect. +EOF +} + +for arg in "$@"; do + case "$arg" in + --full-check) full_check=true ;; + --dry-run) dry_run=true ;; + -h|--help) usage; exit 0 ;; + *) + echo "Unknown argument: $arg" >&2 + usage >&2 + exit 1 + ;; + esac +done ensure_nix_profile @@ -16,12 +65,6 @@ if ! command -v nix >/dev/null 2>&1; then exit 127 fi -hosts="$(list_flake_targets .)" - -echo "Hosts:" -echo "$hosts" - -echo echo "Checking for obvious committed secrets..." if grep -RInE 'github_pat_|ghp_|access-tokens|hashedPassword[[:space:]]*=' \ --exclude-dir=.git \ @@ -33,49 +76,200 @@ else echo "No obvious token patterns found." fi +mapfile -t all_hosts < <(list_flake_targets .) +mapfile -t all_packages < <(nix eval --json "${NIX_EVAL_FLAGS[@]}" .#packages.x86_64-linux --apply builtins.attrNames | jq -r '.[]') + +# host_targets_for_dir +# Prints the nixosConfigurations target names whose hostPath is +# ./hosts//host.nix, derived straight from flake.nix's generatedTargets +# (one mkTarget { ... } call per line) rather than a hand-maintained table, +# so it can't drift the way a copied mapping would. +host_targets_for_dir() { + local dir="$1" + grep -oE '^[[:space:]]*[A-Za-z0-9_-]+ = mkTarget \{[^}]*hostPath = \./hosts/'"${dir}"'/host\.nix;[^}]*\};' flake.nix \ + | sed -E 's/^[[:space:]]*([A-Za-z0-9_-]+) = mkTarget.*/\1/' \ + || true +} + +declare -a changed_files=() +scope_desc="full repo" + +if ! $full_check; then + resolve_base_ref() { + if [[ -n "${MAINT_BASE_SHA:-}" ]] && git cat-file -e "${MAINT_BASE_SHA}^{commit}" 2>/dev/null; then + echo "$MAINT_BASE_SHA" + return + fi + if git rev-parse --verify -q HEAD^ >/dev/null 2>&1; then + echo "HEAD^" + return + fi + git hash-object -t tree /dev/null + } + + base_ref="$(resolve_base_ref)" + echo + echo "Changed-files scope: diffing against ${base_ref}" + mapfile -t changed_files < <(git diff --name-only --diff-filter=ACMR "$base_ref" -- . | sort -u) + + if [[ ${#changed_files[@]} -eq 0 ]]; then + echo "No changed files detected." + else + printf ' %s\n' "${changed_files[@]}" + fi + scope_desc="changed files only (base: ${base_ref})" +fi + +# Whole-tree fmt/lint always run under --full-check; otherwise scoped below. +declare -a changed_nix_files=() +for f in "${changed_files[@]:-}"; do + [[ "$f" == *.nix && -f "$f" ]] && changed_nix_files+=("$f") +done + echo echo "Checking Nix formatting with nixpkgs-fmt..." -nix run "${NIX_EVAL_FLAGS[@]}" github:NixOS/nixpkgs/nixos-25.11#nixpkgs-fmt -- --check . +if $full_check; then + nix run "${NIX_EVAL_FLAGS[@]}" github:NixOS/nixpkgs/nixos-25.11#nixpkgs-fmt -- --check . +elif [[ ${#changed_nix_files[@]} -gt 0 ]]; then + nix run "${NIX_EVAL_FLAGS[@]}" github:NixOS/nixpkgs/nixos-25.11#nixpkgs-fmt -- --check "${changed_nix_files[@]}" +else + echo "No changed .nix files; skipping." +fi echo echo "Running statix lint..." -nix run "${NIX_EVAL_FLAGS[@]}" github:NixOS/nixpkgs/nixos-25.11#statix -- check . +if $full_check; then + nix run "${NIX_EVAL_FLAGS[@]}" github:NixOS/nixpkgs/nixos-25.11#statix -- check . +elif [[ ${#changed_nix_files[@]} -gt 0 ]]; then + for f in "${changed_nix_files[@]}"; do + nix run "${NIX_EVAL_FLAGS[@]}" github:NixOS/nixpkgs/nixos-25.11#statix -- check "$f" + done +else + echo "No changed .nix files; skipping." +fi + +# Figure out which hosts/packages this run needs to eval (and, under +# --dry-run, build). full_check always means "everything"; otherwise a +# change to flake.nix/flake.lock/variables.nix/modules/common/* (repo-wide +# inputs) or to any other modules/*.nix outside platforms//build-types +# (whose blast radius isn't safely inferable from the path alone -- see +# CLAUDE.md's "Grep modules/build-types/*.nix for each build type's imports +# list") also falls back to everything, on the same reasoning CLAUDE.md +# already gives interactive sessions for when to run the full sweep. +# Anything more targeted -- a host.nix, a platform module, a build-type +# module -- narrows to just the hosts it can affect. +declare -A affected_hosts=() +eval_packages=false + +if $full_check; then + for h in "${all_hosts[@]}"; do affected_hosts[$h]=1; done + eval_packages=true +else + full_fallback=false + for f in "${changed_files[@]:-}"; do + case "$f" in + flake.nix|flake.lock|variables.nix|modules/common/*) + full_fallback=true + ;; + esac + done + + if ! $full_fallback; then + for f in "${changed_files[@]:-}"; do + case "$f" in + hosts/*/*) + hostdir="${f#hosts/}" + hostdir="${hostdir%%/*}" + while IFS= read -r t; do + [[ -n "$t" ]] && affected_hosts[$t]=1 + done < <(host_targets_for_dir "$hostdir") + ;; + modules/platforms/*.nix) + platform="$(basename "$f" .nix)" + for h in "${all_hosts[@]}"; do + [[ "$h" == "${platform}-"* ]] && affected_hosts[$h]=1 + done + ;; + modules/build-types/*.nix) + buildtype="$(basename "$f" .nix)" + for h in "${all_hosts[@]}"; do + [[ "$h" == *"-${buildtype}" ]] && affected_hosts[$h]=1 + done + ;; + modules/installer/*) + # iso.nix (imported by both the "installer" nixosConfigurations + # target and netbootSystem, which backs packages.pxe) pulls in + # common.nix, so a common.nix change reaches all three. + affected_hosts[installer]=1 + eval_packages=true + ;; + modules/pxe-boot/*) + # stage-installer-artifacts.nix is imported by + # modules/build-types/pxe-boot.nix only -- same blast radius as a + # build-types/*.nix change, not a packages one. + for h in "${all_hosts[@]}"; do + [[ "$h" == *"-pxe-boot" ]] && affected_hosts[$h]=1 + done + ;; + modules/*) + full_fallback=true + ;; + esac + done + fi + + if $full_fallback; then + echo + echo "Changed files affect shared config; falling back to evaluating every host/package." + for h in "${all_hosts[@]}"; do affected_hosts[$h]=1; done + eval_packages=true + fi +fi + +mapfile -t hosts < <(for h in "${!affected_hosts[@]}"; do echo "$h"; done | sort) echo -echo "Evaluating host toplevel derivations..." -for host in $hosts; do - echo "==> $host" - nix eval --raw "${NIX_EVAL_FLAGS[@]}" ".#nixosConfigurations.${host}.config.system.build.toplevel.drvPath" +if [[ ${#hosts[@]} -eq 0 ]]; then + echo "No hosts affected by changed files; skipping host eval." +else + echo "Evaluating host toplevel derivations (${scope_desc})..." + for host in "${hosts[@]}"; do + echo "==> $host" + nix eval --raw "${NIX_EVAL_FLAGS[@]}" ".#nixosConfigurations.${host}.config.system.build.toplevel.drvPath" - # lxc-* hosts deploy via a directly pct-restore-able tarball instead of - # nixos-install (see docs/auto-installer.md); proxmox-* hosts can - # alternatively be built as a standalone disk image (see - # docs/proxmox-images.md). Both are otherwise-unvalidated buildable - # surface, easy to silently break without this. - case "$host" in - lxc-*) - echo "==> $host (tarball)" - nix eval --raw "${NIX_EVAL_FLAGS[@]}" ".#nixosConfigurations.${host}.config.system.build.tarball.drvPath" - ;; - proxmox-*) - echo "==> $host (diskoImagesScript)" - nix eval --raw "${NIX_EVAL_FLAGS[@]}" ".#nixosConfigurations.${host}.config.system.build.diskoImagesScript.drvPath" - ;; - esac -done + # lxc-* hosts deploy via a directly pct-restore-able tarball instead of + # nixos-install (see docs/auto-installer.md); proxmox-* hosts can + # alternatively be built as a standalone disk image (see + # docs/proxmox-images.md). Both are otherwise-unvalidated buildable + # surface, easy to silently break without this. + case "$host" in + lxc-*) + echo "==> $host (tarball)" + nix eval --raw "${NIX_EVAL_FLAGS[@]}" ".#nixosConfigurations.${host}.config.system.build.tarball.drvPath" + ;; + proxmox-*) + echo "==> $host (diskoImagesScript)" + nix eval --raw "${NIX_EVAL_FLAGS[@]}" ".#nixosConfigurations.${host}.config.system.build.diskoImagesScript.drvPath" + ;; + esac + done +fi echo -echo "Evaluating buildable packages..." -packages="$(nix eval --json "${NIX_EVAL_FLAGS[@]}" .#packages.x86_64-linux --apply builtins.attrNames | jq -r '.[]')" -for pkg in $packages; do - echo "==> packages.x86_64-linux.${pkg}" - nix eval --raw "${NIX_EVAL_FLAGS[@]}" ".#packages.x86_64-linux.${pkg}" -done +if ! $eval_packages; then + echo "No packages affected by changed files; skipping package eval." +else + echo "Evaluating buildable packages..." + for pkg in "${all_packages[@]}"; do + echo "==> packages.x86_64-linux.${pkg}" + nix eval --raw "${NIX_EVAL_FLAGS[@]}" ".#packages.x86_64-linux.${pkg}" + done +fi -if [[ "$MODE" == "dry-run" ]]; then +if $dry_run; then echo - echo "Running dry-run builds for all hosts. This will not create result symlinks." - for host in $hosts; do + echo "Running dry-run builds for the active scope. This will not create result symlinks." + for host in "${hosts[@]:-}"; do echo "==> Dry-run build: $host" nix build --dry-run --no-link "${NIX_EVAL_FLAGS[@]}" ".#nixosConfigurations.${host}.config.system.build.toplevel" @@ -91,12 +285,14 @@ if [[ "$MODE" == "dry-run" ]]; then esac done - echo - echo "Running dry-run builds for all packages." - for pkg in $packages; do - echo "==> Dry-run build: packages.x86_64-linux.${pkg}" - nix build --dry-run --no-link "${NIX_EVAL_FLAGS[@]}" ".#packages.x86_64-linux.${pkg}" - done + if $eval_packages; then + echo + echo "Running dry-run builds for packages." + for pkg in "${all_packages[@]}"; do + echo "==> Dry-run build: packages.x86_64-linux.${pkg}" + nix build --dry-run --no-link "${NIX_EVAL_FLAGS[@]}" ".#packages.x86_64-linux.${pkg}" + done + fi fi echo diff --git a/scripts/codex-setup.sh b/scripts/codex-setup.sh index 4d19207..957a612 100755 --- a/scripts/codex-setup.sh +++ b/scripts/codex-setup.sh @@ -82,13 +82,6 @@ if ! command -v jq >/dev/null 2>&1; then fi echo "Available NixOS hosts:" -hosts="$(list_flake_targets .)" -echo "$hosts" +list_flake_targets . -echo "Evaluating all host toplevel derivations..." -for host in $hosts; do - echo "==> Evaluating $host" - nix eval --raw "${NIX_EVAL_FLAGS[@]}" ".#nixosConfigurations.${host}.config.system.build.toplevel.drvPath" -done - -echo "Codex setup complete." +echo "Codex setup complete. Run bash scripts/codex-maintenance.sh to validate changes."