Conversation
zzinit removed its helper functions and itself before checking the status of the completion and zpmod stages, so a failed zmodload left the user with a "run `zzinit` again" contract and nothing to call. Return before the unset when either optional stage failed, remove the helpers only on success, and extend the zpmod diagnostic with the retry hint the source-stage failure path already gives. Add a fixture that plants an unloadable zpmod.so, observes the failed run keeping zzinit defined, and proves the retry succeeds and cleans up. Closes #200
Deploying src with
|
| Latest commit: |
6685c83
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8b0007e4.zi-src.pages.dev |
| Branch Preview URL: | https://bug-200.zi-src.pages.dev |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused fix, regression test, and checksum update are consistent and complete.
Pull request overview
Keeps zzinit retryable when optional loader stages fail, aligning behavior with its documented contract.
Changes:
- Preserve helper functions on optional-stage failure.
- Improve the zpmod retry diagnostic.
- Add regression coverage and regenerate the checksum.
File summaries
| File | Description |
|---|---|
| tests/installers.sh | Tests failure recovery and successful retry. |
| public/zsh/init.zsh | Retains helpers until initialization succeeds. |
| public/checksum.txt | Updates the loader checksum. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
github-actions Bot
pushed a commit
that referenced
this pull request
Sep 18, 2026
Co-authored-by: Sal <ss-o@users.noreply.github.com> d19ae48
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.
Closes #200
zzinitremoved its helper functions and itself before it checked the status of the completion and zpmod stages, so a failedzmodload zi/zpmodprinted "rebuild it withzi module build", returned 1, and left nothing to call afterwards. The comment above theunsetand the file header both promised the opposite.Changes
public/zsh/init.zsh: return 1 before theunset -fwhen either optional stage failed; remove the helpers and return 0 only on success. The zpmod diagnostic now ends with "then runzzinitagain", matching the hint the source-stage failure path already gives. The hint stays inside the existingZI[MUTE_WARNINGS]guard.tests/installers.sh:test_init_keeps_helpers_when_zpmod_failsplants a non-ELFzpmod.so, asserts the first run returns 1 withzzinitstill defined and the rebuild hint on stderr, removes the file, and asserts the retry returns 0 and unsets the helpers. On the unmodified loader it fails atfirst_retryable:1and the retry returns 127.public/checksum.txt: regenerated withsh public/sh/generate-checksums.sh.Behavior notes
ZI[MUTE_WARNINGS]=1and a brokenzpmod.so,zzinitnow returns 1 silently and leaves the eight helper functions defined, where before they were removed. That is the contract the header already documents (helpers are kept on failure so the user can retry); the retry is cheap becausezi.zshguards re-sourcing withZI[SOURCED].docs/README.mdis unchanged: it does not claim retryability, so the issue's alternative (dropping the claim from the docs) had nothing to drop.Verification
sh ./tests/installers.sh: 21 ok, including the new fixturezsh -f -n public/zsh/init.zsh1e8b0a6withzsh-lint.json: exit 0