From 9d362f07c7772952574bf4c4d5b1c11ee967dba6 Mon Sep 17 00:00:00 2001 From: Lior Date: Tue, 26 May 2026 03:28:46 -0400 Subject: [PATCH] =?UTF-8?q?fix(postmerge-5120):=204=20Copilot=20findings?= =?UTF-8?q?=20=E2=80=94=20P0=20install.sh-on-devShell=20+=20P1=20=C2=A724/?= =?UTF-8?q?mkpasswd=20misframe=20+=20Nit=20workflow=20comment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P0 (full-ai-cluster/flake.nix shellHook): running tools/setup/install.sh automatically on every `nix develop` shell entry has large host-side side effects (apt/brew installs, profile edits, network fetches, possible sudo prompts) and breaks typical devShell expectations. Reliably FAILS on NixOS (no apt) which is in supportedSystems. Fix: drop the auto-run shellHook, replace with a one-line hint pointing operators at install.sh for manual host setup when needed. (The maintainer 2026-05-26 question "is nixos debian based or we going to have to update our deps managment like it uses apt?" lands on the exact root cause — NixOS uses nix, not apt; install.sh currently only supports brew + apt paths.) P1 (flake.nix comment block): reworded to drop the inaccurate "4th consumer per §24" framing (§24 specifies three; saying "4th" while citing §24 was inconsistent) and to drop the mkpasswd claim (mkpasswd is NOT in tools/setup/manifests/{brew,apt}; for the iter-5.3 prompt-for-password substrate, mkpasswd comes via the NixOS installer ISO's environment.systemPackages, not via install.sh). New comment states the actual contract: install.sh is the canonical dev-laptop tooling source per §24's three-consumer set; devShell is nix-managed admin tooling only. Nit (build-ai-cluster-iso.yml audit step): reworded to honestly describe the 7z source-of-truth split — CI relies on ubuntu-24.04 runner-default 7z (not install.sh; the workflow doesn't invoke install.sh); manifests cover local-dev parity only. Future-proof note added for ubuntu-version change. The 4th post-merge thread (P1 on tools/ci/audit-installer-iso-content.ts line 183 about exit-3 doc) is stale: 585f9d2cf already widened the exit-3 description to "missing path OR present-but-empty"; the review was based on an earlier commit. Resolving as no-op alongside this fix. Co-Authored-By: Claude --- .github/workflows/build-ai-cluster-iso.yml | 19 +++++++----- full-ai-cluster/flake.nix | 35 ++++++++-------------- 2 files changed, 24 insertions(+), 30 deletions(-) diff --git a/.github/workflows/build-ai-cluster-iso.yml b/.github/workflows/build-ai-cluster-iso.yml index 628384dc6a..bb2ad649c7 100644 --- a/.github/workflows/build-ai-cluster-iso.yml +++ b/.github/workflows/build-ai-cluster-iso.yml @@ -135,13 +135,18 @@ jobs: fi iso_abs="$(pwd)/${iso_candidates[0]}" cd .. - # Declarative 7z management (the maintainer 2026-05-26 — "it - # does not have to be exact version but just declarative - # managed like other thinks we manage at that level in - # install.sh"). p7zip-full is in tools/setup/manifests/apt; - # ubuntu-24.04 runners default-install 7z so this is a - # belt-and-suspenders: manifest declares the dep + runner - # ships it. Call bun directly (no nix-shell wrap needed). + # 7z source-of-truth: + # - CI (this workflow): relies on ubuntu-24.04 runners' + # default 7z install (verified present on the standard + # runner image). + # - Local dev laptops: tools/setup/manifests/{brew,apt} + # declare p7zip / p7zip-full so install.sh's macos.sh / + # linux.sh paths install it during host-setup refresh. + # The CI runner-image pre-install is the load-bearing source + # here (this workflow does NOT run install.sh). Manifests + # cover local-dev parity only. If we ever switch off + # ubuntu-24.04, add an explicit "apt-get install -y + # p7zip-full" step before this line. bun tools/ci/audit-installer-iso-content.ts --iso "$iso_abs" - name: Locate ISO + capture metadata diff --git a/full-ai-cluster/flake.nix b/full-ai-cluster/flake.nix index ef65e9c183..a2ea7dcb0a 100644 --- a/full-ai-cluster/flake.nix +++ b/full-ai-cluster/flake.nix @@ -172,13 +172,17 @@ devShells.default = pkgs.mkShell { name = "zeta-ai-cluster-admin"; # Nix-managed admin tooling (k8s + age/sops + nix observability). - # Host-level CI substrate (bun, p7zip, mkpasswd) is NOT duplicated - # here — it comes via tools/setup/install.sh manifests per the - # maintainer 2026-05-26: "nix needs to run our install.sh too - # for setup". Single source of truth = the install.sh manifests - # at tools/setup/manifests/{brew,apt}. Nix devShell is the 4th - # way install.sh is consumed (alongside dev laptops, CI runners, - # devcontainer images per GOVERNANCE.md §24). + # Host-level dev-laptop tooling (bun, p7zip, etc.) is managed + # SEPARATELY via tools/setup/install.sh manifests at + # tools/setup/manifests/{brew,apt} — that's the canonical + # consumer-of-record per GOVERNANCE.md §24 (dev laptops, CI + # runners, devcontainer images). The nix devShell does NOT + # auto-run install.sh on entry: Copilot P0 on post-merge of + # #5120 flagged that auto-run has large host-side side effects + # (apt/brew installs, network fetches, possible sudo prompts) + # and breaks devShell expectations + reliably fails on NixOS + # hosts which don't have apt at all. Operators run install.sh + # manually when needed (rare; usually after pulling main). packages = with pkgs; [ nix-output-monitor nvd nh kubectl kubernetes-helm k9s argocd @@ -188,22 +192,7 @@ ]; shellHook = '' echo "zeta-ai-cluster admin shell." - # Per the maintainer 2026-05-26: "nix needs to run our install.sh - # too for setup". The nix devShell is the 4th consumer of the - # canonical install.sh entry-point. Run it idempotently on shell - # entry so host-level tooling (bun, p7zip, mkpasswd, etc.) stays - # in sync with the manifests without separate operator action. - # install.sh is detect-first-install-else-update + safe to - # re-run; cost on no-op refresh is single-digit seconds. - if command -v git >/dev/null 2>&1; then - _zeta_root="$(git rev-parse --show-toplevel 2>/dev/null || true)" - if [ -n "$_zeta_root" ] && [ -x "$_zeta_root/tools/setup/install.sh" ]; then - echo " Running tools/setup/install.sh (idempotent host-setup refresh)..." - bash "$_zeta_root/tools/setup/install.sh" || \ - echo " WARNING: install.sh exited non-zero; continuing devShell." - fi - unset _zeta_root - fi + echo " Host setup (rare): bash tools/setup/install.sh" echo " Build USB ISO: nix build .#installer-iso" echo " Build host system: nixos-rebuild build --flake .#" echo " Talk to cluster: kubectl / k9s / argocd / cilium / hubble"