ci(test-cascade-2): bun unit tests for zflash pure-logic + finds regex bug (DOS_FAT token never matched DOS_FAT_32) (Aaron 2026-05-26)#5117
Merged
AceHack merged 3 commits intoMay 26, 2026
Conversation
…iskutil parse + auto-name) (Aaron 2026-05-26) Aaron 2026-05-26: 'any parts we can test in siolate are candidates for more unit like tests instead of full integration tests like maybe some can be docker tests and such'. Ships #2 of the ascending CI test-substrate cascade: #1 Source-substrate audit (#5116; merged/in-flight) #2 Bun unit tests for zflash pure-logic (this PR) #3 Docker zeta-install.sh test (mocked /dev) — next #4 ISO content audit (7z post-build) — follow-on #5 NixOS test framework (full QEMU VM) — follow-on Two new files: - full-ai-cluster/tools/zflash-lib.ts: pure-logic library extracted from zflash.ts for unit-testability. Exports: VALID_HOSTNAME_REGEX (RFC1123 single-label hostname) isValidHostname(s) (convenience wrapper) parseFatPartitionFromDiskutilList (diskutil list output → partition path) generateRandomNodeName(getRng?) (node-<6hex> auto-gen; testable RNG) parseOutputFileMarker (peer-call OUTPUT-FILE: line parser) All pure functions; NO I/O (no fs, no spawnSync, no process.exit). - full-ai-cluster/tools/zflash-lib.test.ts: 33 Bun-test cases across 4 describe blocks. PASS empirically. EMPIRICAL FINDING from the tests: the existing zflash.ts regex `\b(DOS_FAT|EFI|MS-DOS|FAT16|FAT32|Windows_FAT)\b` includes a `DOS_FAT` token that NEVER MATCHES `DOS_FAT_32` (underscore is a word-char so `\b` boundary fails). Real diskutil output for FAT32 GPT partitions is `MS-DOS FAT32` (with space; matches `\bMS-DOS\b`). The `DOS_FAT` token is likely vestigial / from a misremembered format. Pinned via "DOCUMENTS-FINDING" test; resolve in follow-on by either dropping `DOS_FAT` from regex OR broadening to `DOS_FAT(_\d+)?` if there's a known consumer. This is exactly the kind of bug unit tests catch that integration tests miss — the regex branch was never empirically exercised because all real NixOS isohybrid ISOs post-dd hit the MBR 0xEF path (iter-4.4 substrate). zflash.ts itself NOT modified in this PR — keeping the refactor scope-bounded. Future iteration: import VALID_HOSTNAME_REGEX + parseFatPartitionFromDiskutilList from zflash-lib.ts in zflash.ts (replace inline definitions); add `if (import.meta.main)` guard around main() invocation; export additional pure functions for broader unit-test coverage. Composes with #5116 (source-substrate audit; ships in parallel) and the broader cascade (#3-#5 in follow-on PRs). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…strate sync (TS6133 lint pass)
There was a problem hiding this comment.
Pull request overview
This PR introduces a small, pure-logic TypeScript library for zflash-adjacent parsing/validation so it can be covered by fast Bun unit tests (as part of the “CI test cascade” effort), plus a new Bun test suite that exercises the extracted logic and documents a discovered diskutil-parsing regex edge case.
Changes:
- Add
zflash-lib.tsexporting pure helpers/constants (hostname validation,diskutilFAT/EFI partition detection, node-name generation, peer-call output marker parsing). - Add
zflash-lib.test.tswith Bun unit tests covering these helpers and pinning the discoveredDOS_FAT/word-boundary mismatch behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| full-ai-cluster/tools/zflash-lib.ts | New pure-logic helper library extracted from zflash logic for unit-testability. |
| full-ai-cluster/tools/zflash-lib.test.ts | New Bun test suite validating hostname parsing/formatting, diskutil parsing, RNG-driven naming, and peer-call marker parsing. |
…no defect lock-in) + globalThis.crypto + role-attribution + deterministic RNG test + spelling - P0 globalThis.crypto: route via globalThis to avoid DOM-typed bare 'crypto' (repo TS lib: esnext, no DOM) - P1 DOS_FAT regex broadened from \bDOS_FAT\b to \bDOS_FAT(_\d+)?\b so it actually matches the documented DOS_FAT_32 / DOS_FAT_16 variants; DOCUMENTS-FINDING test flipped to assert correct behavior + new DOS_FAT_16 case added - P1 named-attribution 'per Aaron' → 'per the human maintainer' per repo role-ref convention - P1 probabilistic 'two calls' test → deterministic injected-RNG comparison (eliminates 1-in-16M flake risk) - P2 spelling 'siolate' → 'isolation' (paraphrase; verbatim preserved in Mika preservation file) - Docstring example fixed (DOS_FAT_32 → MS-DOS FAT32 to match real diskutil output) 35/35 tests pass; TS strict clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AceHack
deleted the
otto-cli/ci-test-cascade-2-zflash-bun-unit-tests-2026-05-26
branch
AceHack
added a commit
that referenced
this pull request
May 26, 2026
…026-05-26 ongoing test cascade) (#5119) Ships #4 of the CI test-substrate cascade. Complements cascade #1 (source-substrate preflight audit; #5116 merged at 67ab888) by catching the bug class where the ISO build silently drops a file present in the source tree. Cascade overview (this PR = #4): #1 Source-substrate preflight audit (merged via #5116) #2 Bun unit tests for zflash pure-logic (merged via #5117; caught the DOS_FAT regex defect-lock-in) #3 Docker zeta-install.sh test (deferred follow-on) #4 ISO content audit (THIS PR) #5 NixOS test framework (deferred follow-on) New tool tools/ci/audit-installer-iso-content.ts: - Takes --iso <path> - Uses 7z list (-slt format; default on ubuntu-24.04) - Asserts REQUIRED_ISO_PATHS present in ISO root: nix-store.squashfs (containing the install scripts + modules) boot/bzImage (Linux kernel) boot/initrd (initramfs) boot/grub/grub.cfg (UEFI + BIOS boot config) - Exit codes: 0 pass / 1 invocation error / 2 7z list failed / 3 missing expected path - Adding a new expected top-level file: append to REQUIRED_ISO_PATHS build-ai-cluster-iso.yml workflow extended with a new step BETWEEN 'Build installer ISO' and 'Locate ISO + capture metadata': runs the audit against the freshly-built ISO; fails the build if any required path is missing → upload step is skipped → broken-ISO artifact never reaches operators. What this DOES NOT yet audit (out of scope; cascade #3 + #5 territory): - Contents WITHIN the nix-store squashfs (unsquashfs is heavier; source-substrate audit already catches 'module missing from repo' at cheaper cost) - Live boot behavior (nixosTest framework; cascade #5) Composes with #5116 source audit + tools/dashboard/generate-metrics.ts per-agent decompose-to-action ratio (Aaron's discipline pull — each cascade ship demonstrates filing→action cadence). Co-authored-by: Lior <lior@zeta.dev> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
3 tasks
AceHack
added a commit
that referenced
this pull request
May 26, 2026
…otloader any-of (#5125) * fix(ci P0): cascade #4 audit blocked ALL ISO builds since #5119 — REQUIRED list asserted boot/grub/grub.cfg but NixOS uses isolinux+refind; replaced with bootloader any-of check EMPIRICAL ANCHOR (the maintainer 2026-05-26 caught this): `gh run list --workflow=build-ai-cluster-iso.yml` showed the last 4 builds all failed on the cascade #4 audit step. Last successful build was 17523e4 (PR #5117 iter-5.2.1 era) — BEFORE iter-5.2.2 (login-banner) and iter-5.3 (password prompt) shipped. The maintainer was about to flash a USB expecting the latest CI ISO; the latest CI artifact is actually stale because the audit blocks artifact upload on assertion failure. Root cause: REQUIRED_ISO_PATHS asserted `boot/grub/grub.cfg` (legacy GRUB layout). NixOS installer ISOs as of nixos-24.11 use: - **isolinux** for BIOS boot: `isolinux/isolinux.cfg` - **refind** for UEFI boot: `EFI/BOOT/refind_x64.efi` NOT legacy grub at the asserted path. Build log confirms: `efi-image_eltorito > Copying grub.cfg` lands in EFI/, not boot/grub/. My cascade #4 draft list was version-skewed (training-data default leaked through; ironically exactly what B-0805 capstone names as the systemic agent-discipline gap). Fix: - Drop `boot/grub/grub.cfg` from REQUIRED_ISO_PATHS (the 3 remaining — nix-store.squashfs + boot/bzImage + boot/initrd — ARE sufficient to assert "bootable NixOS installer ISO": without those nothing boots). - Add REQUIRED_BOOTLOADER_ANY: any-of family check for bootloader configs across the known NixOS-version layouts (isolinux/refind/grub). Forward-compatible: if NixOS switches bootloaders in a future channel, add the new path to the any-of list rather than re-breaking. - Header comment documents the empirical anchor so future-Otto doesn't re-introduce the same legacy-path assumption. Confirms B-0805 (capstone, P1) was the right substrate-engineering call: this exact failure mode is what dep-pin-search-first-authority discipline prevents. Once this lands, the next ISO build on main will pick up iter-5.2.2 + iter-5.3 substrate and the artifact will reflect the current substrate state. The maintainer can then re-flash + install with confidence. Co-Authored-By: Claude <noreply@anthropic.com> * fix(p0-iso-audit): use .some() instead of .find()+unused-const (Copilot P0 on #5125) bootloaderHit const was assigned but never used; would fail tsc under noUnusedLocals. Switched to boolean .some() check which avoids the unused-variable shape entirely. No behavior change. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Lior <lior@zeta.dev> Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Aaron 2026-05-26: 'any parts we can test in siolate are candidates for more unit like tests instead of full integration tests'.
Ships #2 of CI test cascade. Two new files:
full-ai-cluster/tools/zflash-lib.ts(~90 LOC pure-logic library: VALID_HOSTNAME_REGEX + isValidHostname + parseFatPartitionFromDiskutilList + generateRandomNodeName + parseOutputFileMarker)full-ai-cluster/tools/zflash-lib.test.ts(~180 LOC; 33 Bun-test cases; ALL PASS)EMPIRICAL FINDING from the tests: regex
\b(DOS_FAT|...)\bincludes aDOS_FATtoken that NEVER matchesDOS_FAT_32(underscore is word-char). Real diskutil output isMS-DOS FAT32(matches\bMS-DOS\b).DOS_FATtoken vestigial; pinned via DOCUMENTS-FINDING test; resolve in follow-on.This is exactly the bug class unit tests catch that integration tests miss.