Add a zsh -n gate: nothing was checking the files the shell sources - #95
Merged
Conversation
shellcheck cannot parse zsh, so the lint job's globs never looked at the ~25 files that get sourced into every interactive shell: dot_zshrc, dot_zshenv and every */*.zsh topic file. They had no CI coverage at all. That is worse than it first appears. A parse error in a topic file does not hard-fail the shell -- zsh reports it and carries on, silently dropping every alias and setting after the error. So a break ships green and shows up later as "some of my aliases disappeared", with no obvious cause. Verified by injecting `if [[ broken syntax` into system/alii.zsh: zsh -n catches it, the existing shellcheck steps do not, and the lint job passed. - ci.yaml: new "Syntax-check zsh files" step in the lint job. Installs zsh on Linux if absent; macOS runners have it. The file list is derived from globs rather than hardcoded, so a newly added topic file is covered automatically instead of being silently skipped, and the step fails if the globs ever match nothing. Uses -n so files are parsed but never executed -- they modify PATH, install hooks and start the prompt, so sourcing them in CI would be wrong. - CLAUDE.md: correct the lint row, which described only shellcheck and claimed ubuntu-only; it has been matrixed and has gained py_compile and the bump-deps self-test since. Record why zsh -n needs to be its own gate. Verified by extracting the step body from the workflow and running it verbatim: 25 files pass, and it correctly fails on a broken topic file and on a broken dot_zshenv.
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.
First of the three remaining backlog topics (CI quality gates). This closes one concrete, proven hole rather than adding gates speculatively.
The hole
shellcheckcannot parse zsh, so the lint job's shebang-based globs never looked at any of the ~25 files that get sourced into every interactive shell —dot_zshrc,dot_zshenv, and every*/*.zshtopic file. Zero CI coverage.I proved it by injecting
if [[ broken syntaxintosystem/alii.zsh:And the failure mode is quiet. A parse error in a sourced file does not hard-fail the shell — zsh reports it and carries on, silently dropping every alias and setting after the error. So a break would ship green and surface days later as "some of my aliases disappeared", with no obvious cause. That's precisely the kind of thing a syntax gate should catch.
The gate
A new
Syntax-check zsh filesstep in the existing (already matrixed)lintjob.-nparses without executing. The only safe choice: these files alterPATH, install hooks and start the prompt, so actually sourcing them in CI would be wrong.Verification
Rather than paraphrase the step, I extracted its body from the workflow YAML and ran it verbatim:
system/alii.zsh→FAIL, exit 1, parse error showndot_zshenv→FAIL,unmatched "Running it on both matrix legs also gives a free cross-version check, since Ubuntu's zsh may differ from macOS's 5.9.
Also
CLAUDE.md's CI table describedlintas ubuntu-only running just shellcheck. It has since been matrixed over both OSes and gainedpy_compileandbump-deps --self-test. Corrected, and recorded whyzsh -nhas to be its own gate.Not in this PR
Per discussion, no shell-startup performance guard — a wall-clock threshold would be flaky on shared runners, and the 180 ms figure is already documented.
Remaining backlog: unit tests, and a selective Python port of
bin/secretandbin/herdr-reload-all(the measurements argued against a broad migration — python3 starts ~2.5× slower than sh, which would penalise the twelve 1–13 linegit-*wrappers for no benefit).