fix(deploy): drop piped grep -q from the prune guard - #12
Merged
Conversation
The artifact prune asked "is a bin/* channel still using this?" by piping the in-use list into `grep -q`. That grep stops reading at its first match and closes the pipe, so the writer can take SIGPIPE and exit 141; pipefail then reports the pipeline as failed and the caller reads a hit as a miss — removing the artifact the guard had just proved was live. Read the list from a here-string instead: there is no writer left to signal, so the status is grep's own verdict. Not reachable at the current scale — SIGPIPE needs the list to exceed the 64 KiB pipe buffer, and the layout defines three channels — so this is the safety property no longer resting on an unstated size assumption, not a live incident. Both copies of the block are fixed together: the manual script and the release workflow that actually runs it. Verified against a layout whose two oldest artifacts are the live release and stage targets, i.e. both outside KEEP=5 and protected only by this guard: both survive, the unreferenced artifact is still pruned, and a 200 KB list now returns 0 where it returned 141. Closes #11 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The deploy scripts carried a warning — twice in nginx-cutover.sh, and once more added alongside the prune fix — telling the reader that piping into `grep -q` SIGPIPEs under pipefail. It overstated a narrow failure mode: the writer only takes SIGPIPE once its output exceeds the pipe buffer, so at these payload sizes the idiom is fine, and grep needs no such caveat. Keep the code as it is — a here-string is the shorter way to write the in-use test regardless — and drop the lore. What survives in nginx-cutover.sh is the part that is actually non-obvious: that one scratch file is reused by every probe.
The previous commit removed this note as overstated lore. That was right for the prune guard, whose input is 69 bytes, and wrong here: these probes read the ~35 KB dashboard body, which is past the point where piping into `grep -q` under pipefail reports a hit as a miss. Restore it with the numbers attached — correct through 22 KB, wrong 1/5 at 24 KB, wrong 5/5 from 28 KB — so the note reads as a measurement rather than a superstition, states that the 64 KiB pipe capacity is not the operative limit, and says outright that a pipe is fine for short inputs. The point is that nobody has to re-derive this, and that nobody generalises it into a rule about grep.
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 #11
One line in each of the two copies of the artifact-prune block:
deploy/pi5-deploy.sh— the manual path..github/workflows/release.yml— the path that actually runs on everyv*tag.Plus a comment at each site recording why it is not a pipe, so the idiom does not
creep back.
Why
grep -qstops reading at its first match and closes the pipe; the writer can thentake
SIGPIPEand exit141, andpipefailreports the whole pipeline as failed.The caller reads that as "no match" — the exact inverse of the truth — and deletes
the artifact the guard had just proved a live channel points at. A here-string
leaves no writer to signal, so the status is grep's own verdict.
As the issue records, this is not reachable at the current scale. Measured on the
Pi, the verdict starts flipping at roughly 24 KB of payload (1/5 runs), and is
wrong every time from 28 KB;
in_useis one line perbin/*channel — 69 byteswith all three present, ~350x under that. (An earlier revision of the issue blamed
the 64 KiB pipe buffer; corrected in a comment there, with the curve.) The change is
about the safety property no longer resting on an unstated assumption about payload
size.
Two further commits sort out the commentary, which had the caveat in the wrong
places:
nginx-cutover.sh. The code shape stays — a here-string is simply the shorter wayto write the test — but at 69 bytes of input there is nothing to warn about, and
grep does not need a caveat attached to it.
genuinely in range: the HTTP probes in
nginx-cutover.shread the ~35 KB dashboardbody, past the point where the verdict flips every time. That note now carries the
curve, says the 64 KiB pipe capacity is not the operative limit, and states that
a pipe is fine for short inputs — so it reads as a measurement rather than folklore
and does not get generalised into a rule about grep.
Net effect across
deploy/and.github/: nogrep -qanywhere reads from a pipe;every one greps a file or a here-string; exactly one comment explains why, where it
is load-bearing.
Verification
Exercised the real prune logic against a layout built to be maximally hostile — the
two oldest artifacts are the live
bin/releaseandbin/stagetargets, so bothsit outside
KEEP=5and nothing but this guard protects them:Both live artifacts survive, the unreferenced
v3is still pruned, and theover-buffer case returns
0where it previously returned141.Also checked: no piped
grep -qremains anywhere underdeploy/or.github/;bash -nclean on the script and on the block embedded in the workflow;release.ymlstill parses as YAML with both jobs intact.Prune stays non-fatal — the surrounding
|| echo/trailingtrueare untouched.