fix(minify): failed render must not truncate the existing output - #180
Merged
Conversation
argsh minify -t -o rendered the template with a direct redirect into the output file, so the shell truncated a previously-good artifact before envsubst even ran. A failed render left a 0-byte file behind — this once shipped an empty lo-up installer. Render into a mktemp file, verify the exit code and that the render is non-empty, then mv over the output (cat for non-regular targets like the /dev/stdout default). On failure the temp file is removed and the output is left untouched, with an error naming what failed. Also detect non-GNU envsubst drop-ins (e.g. renvsubst) up front via --version, since they reject the GNU SHELL-FORMAT argument with an unhelpful "Unknown flag". Fixes #178 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016DJzGS3pNBzZs1jaEQ74Co
Review round: mv handed ${out} mktemp's 0600 mode where the old redirect
kept existing perms/umask. chmod --reference the existing file (644 for a
new one) before the mv; perms assertion added to the happy-path test and
mutation-checked.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016DJzGS3pNBzZs1jaEQ74Co
This was referenced Aug 28, 2026
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.
Fixes #178
Problem
argsh minify -t <template> -o <out>rendered the template with a direct redirect:The shell opens and truncates
${out}beforeenvsubstruns, so a failed render destroys a previously-good artifact and leaves a 0-byte file behind. This shipped an emptylo-upinstaller once (lok8sinstall/buildwritesdocs/public/lo-up, served tocurl | sh).Fix
mktempfile first, verify the render exit code AND that the render is non-empty (-s), only thenmvover${out}. For non-regular targets (the/dev/stdoutdefault),catinstead ofmv.${out}is left untouched, and the error names what failed and states that the output was preserved.envsubstdrop-ins up front (envsubst --version | grep -q "GNU gettext"). renvsubst — which lok8s installs asenvsubst— rejects the GNU SHELL-FORMAT positional withERROR: Unknown flag, which gave no hint that the wrong binary was on PATH. The check fails early naming the culprit and the next action (install gettext or fix PATH). This was the issue's optional second suggestion; implemented since it is small and prescribed.Tests
Four new bats tests in
libraries/main.bats, using fakeminifier/envsubstexecutables on PATH (not function stubs, so they also hold against the minified argsh):Mutation-checked: with the direct-redirect shape temporarily restored, the failed-render and empty-render tests fail on the truncated (empty) output file; with the fix they pass.
Full
libraries/suite: 366 pass locally; the 6 pre-existing failures (attrs bare--/--tests, twoargsh::lintdiscovery tests) fail identically on a cleanorigin/maincheckout in the same environment and are unrelated to this change.🤖 Generated with Claude Code
https://claude.ai/code/session_016DJzGS3pNBzZs1jaEQ74Co