Repository navigation
fix(docs): handle SIGPIPE in changelog-promote.sh so --preview | head exits cleanly - #3002
Conversation
… exits cleanly Piping tools/changelog-promote.sh --preview into head/less (normal usage at release cut, where preview output runs hundreds of lines) sprayed a BrokenPipeError traceback on stderr and exited 120 because the python heredoc inherited python's default ignore-SIGPIPE behavior. Restore the default SIGPIPE action (guarded by hasattr for platforms without SIGPIPE) so the script exits silently like cat/grep when the reader closes the pipe early. Safe for the promote path: all file mutations (CHANGELOG.write_text, fragment unlinks) happen before any stdout writes. Adds tools/test-changelog-promote.sh, a hermetic dev-run harness (modeled on tools/test-freshen-decide.sh, not wired into CI) covering the piped preview, unpiped preview, and promote paths against a throwaway fixture root. Refs #2958 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com>
There was a problem hiding this comment.
Wheels Bot — Reviewer A
This PR fixes a real usability bug: tools/changelog-promote.sh --preview | head sprayed a BrokenPipeError traceback on stderr and exited 141 instead of exiting silently. The fix is correct, minimal, and thoroughly reasoned. My verdict is approve.
Correctness
The fix is sound on all counts:
- SIGPIPE handler placement is correct — it runs before any stdout I/O, so there is no window where Python's default
SIG_IGNdisposition could surface as an exception. hasattr(signal, 'SIGPIPE')guard is correct —SIGPIPEdoes not exist on Windows; without the guard the script would crash on that platform. Lucee and Adobe CF both run on Linux/macOS whereSIGPIPEis present, so the real code path always executes.- Safety of the promote path is verified in the code:
CHANGELOG.write_text(...)and allpath.unlink()calls appear before the threeprint()summary lines. A SIGPIPE exit there leaves the filesystem fully updated, matching the PR's claim. - Exit semantics match the PR description: with
SIG_DFLrestored, a SIGPIPE kills the Python process via signal (status 141 on Linux), identical tocat bigfile | head. The outer shell'sset -euo pipefailis unaffected becausepython3is the final command in the pipeline.
Tests
tools/test-changelog-promote.sh is a clean hermetic harness with three well-chosen cases covering the regression, the unpiped baseline, and the promote path. A few observations, none blocking:
- Modeled correctly on
tools/test-freshen-decide.sh— sameset -uo pipefail(deliberately omits-eso a failing pipeline does not abort the harness mid-run). This is the right pattern for test scripts that accumulate failures and report at the end via thefailflag. - The fixture's
big.fixed.mdproduces ~127 KB of output, reliably overflowing the 64 KB pipe buffer afterhead -1exits — SIGPIPE is deterministic, not a race. - Test 3 checks for substring
'Removed 1 fragment'against Python output"Removed 1 fragment(s): big.fixed.md"— correct substring match. make_fixtureintentionally leavestmpunscoped (nolocal) so callers can reference$tmpafter the call. Correct for this usage; worth knowing if the file grows.
Commits
fix(docs): handle SIGPIPE in changelog-promote.sh so --preview | head exits cleanly
- Type
fix, scopedocs(changelog tooling is docs infrastructure — defensible). Header is 82 chars, under the 100-char limit. - Commit body explains the why (CPython's default
SIG_IGNsurfaces asBrokenPipeErrorwhen EPIPE fires at aprint()call) — good signal-to-noise ratio. - DCO sign-off present:
Signed-off-by: Peter Amiri <peter@alurium.com>.
Docs
The PR body notes changelog: none — internal release tooling. That is correct; no user-facing changelog fragment is warranted for a fix to the changelog tooling itself.
No CFML is touched, so cross-engine compatibility, CLAUDE.md anti-pattern, and security checklists do not apply.
Wheels Bot — Reviewer B (round 1)A's review is accurate and the approve verdict holds. The fix is minimal, correct, and well-scoped to bash/Python tooling — no CFML, no cross-engine concerns, nothing that warrants holding the PR. SycophancyNone detected. A cited specific evidence for every claim: SIGPIPE handler placement, Windows guard rationale, promote-path safety, test structure, commit format. False positivesNone detected. One minor inaccuracy: A states the commit header is 82 chars; the actual count is 83. Still well under the 100-char limit — no material effect on the verdict. Missed issuesNone detected. The diff touches only Verdict alignmentA's approve is consistent with the findings: clean fix, hermetic regression test, valid commit, DCO sign-off present. ConvergenceAligned on approve. The fix is correct, the test covers the regression, and there are no outstanding disputes or missed issues to resolve. |
What
tools/changelog-promote.sh --preview | headsprayed aBrokenPipeErrortraceback on stderr and exited 120 (reproduced on develop @345b30dcb). Preview output currently runs 400+ lines, so piping intohead/lessis normal usage at release cut.The python heredoc inherits CPython's default ignore-SIGPIPE behavior, so the EPIPE from the closed pipe surfaces as an unhandled exception at
print("\n".join(bullets))instead of a silent exit.Fix
Restore the default SIGPIPE action at the top of the heredoc (guarded by
hasattr(signal, "SIGPIPE")so the script stays harmless on platforms without it). The script now exits silently likecat bigfile | head.Safe for the promote path: all file mutations (
CHANGELOG.write_text(...), fragment unlinks) happen before any stdout writes, so a SIGPIPE exit can never interrupt a half-finished promote.Semantics note: with
SIG_DFLthe process exits via signal (status 141) when the reader closes early — identical tocat bigfile | head. Under a plain shell the pipeline status ishead's 0; underpipefailit is 141. Both are expected, and the script's ownset -euo pipefailis unaffected because python3 is the final command.Tests
New
tools/test-changelog-promote.sh— hermetic dev-run harness modeled ontools/test-freshen-decide.sh(NOT wired into CI, matching its siblings). It copies the script into a throwaway fixture root (the script derives PROJECT_ROOT from its own location), builds a >64KB fragment so the pipe buffer deterministically overflows afterhead -1exits, and covers:--preview | head— stderr must contain noBrokenPipeError/Traceback; piped output starts with### Fixed(FAILED before the fix, passes after)--previewunpiped — exit 0, empty stderr1.2.3 2026-01-01) — exit 0, version section written, fragment deleted, summary printedManual smoke in the repo root:
tools/changelog-promote.sh --preview | headprints 10 clean lines with empty stderr,git status --porcelainconfirms the harness never touches real repo files.Test evidence (CFML suites)
No CFML is touched (
tools/-only diff: one bash/python script + one bash test harness), so baseline == final by construction. Full core suite run anyway on Lucee 7 + SQLite (dockerwheels-test-lucee7:v1.0.0): HTTP 200, 4305 pass / 0 fail / 0 error (301 bundles, 4323 specs).Harness note: a host-side run through a remapped port (61217→60007) showed 12
testClientSpecfailures (status 0) — proven to be the known port-remap artifact (TestClient self-requests the request's Host header, unreachable inside the container); the same bundle passes 53/53 from inside the container, and the in-container full suite above is fully green.changelog: none — internal release tooling.
Refs #2958
🤖 Generated with Claude Code