fix(gate): harden the remaining guards' final write against a closed pipe (#1838) - #1881
Merged
Conversation
…not forge a failure Piping a guard's output is the normal way to read it — `| tail`, `| rg FAIL`, `| head` — and check-god-files.sh reported a FALSE failure naming a different crate on each such run. Two mechanisms, both SIGPIPE: * Its crate-membership test was `printf ... | grep -qx`. `grep -q` exits the moment it matches, printf then dies writing into the closed pipe, and `set -o pipefail` reports the pipeline as failed — so a crate that IS present intermittently read as absent, and the guard blamed whichever crate the race landed on. Replaced with the pure-shell case membership check check-command-docs.sh already uses. * Guards printed their report incrementally while still deciding it. When stdout (or stderr under `2>&1`) is a pipe whose reader has exited, the next write dies of SIGPIPE — 141 outright, or EPIPE plus `set -e` = exit 1 where SIGPIPE is inherited ignored — and whatever partial state the scan had reached becomes the verdict. Every swept guard now decides its verdict fully before printing: failure lines are buffered into a variable and emitted in one final write, and that write runs under `trap '' PIPE` with its failure discarded, so an early-exiting reader can change neither the verdict nor the exit code. check-file-size.sh and check-left-behind.sh already buffered their reports and needed only the hardened final write. Swept: check-god-files, check-invariants, check-left-behind, check-file-size, check-command-docs, check-gate-parity, check-role-names, check-cargo-install-pins, check-repro-wiring. Output text is byte-identical in every case. check-role-names' subshell marker file is gone: the `sed | while` loop is now redirected from process substitution, so `fail=1` and the buffered report survive. Witness: scripts/test-guard-sigpipe.sh (make guard-sigpipe-test) pipes each guard into `head -1` and `true` and asserts exit 0. On the old scripts all nine `| true` cases die with exit 141 (18 of 27 cases pass); on the new ones all 27 pass. Closes #1815
# Conflicts: # Makefile
…pipe The nine guards #1815's sweep left out already decide their verdict before printing, but their final green OK line was still an unguarded pipe write: with stdout piped into a reader that has already exited (| true is the deterministic repro), the write dies of SIGPIPE (exit 141) or EPIPE under set -e (exit 1), forging a failure out of a green verdict. Copy the epilogue #1815 established: the verdict is decided first, then the final write runs under trap '' PIPE with its failure discarded. Output text is byte-identical. check-action-pins.sh's non-fatal missing-tag-comment report is part of the same decided-green tail, so it rides under the same trap; the unreachable skip branches are left alone, matching #1815's treatment of check-cargo-install-pins.sh. scripts/test-guard-sigpipe.sh grows a case per hardened guard: check-empty-diff.sh runs against the real HEAD~1 HEAD pair, and check-wire-schema.sh — which compiles the two schema exporters — is gated on a cargo toolchain being present, skipping loudly otherwise. Closes #1838 Refs #1815
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @macanderson, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
Reviewer's GuideHardens various gate shell guard scripts against SIGPIPE/EPIPE when their stdout is piped to an early‑exiting reader by buffering output, guarding final writes with trap '' PIPE and || true, and adds a dedicated test harness plus Makefile target to assert these guards remain green under such conditions. Sequence diagram for hardened guard stdout pipe handlingsequenceDiagram
actor Caller
participant GuardScript as check_guard.sh
participant PipeReader as pipe_reader_true
Caller->>GuardScript: run check_guard.sh | true
GuardScript->>GuardScript: run checks, decide status
GuardScript->>GuardScript: note() appends to report
GuardScript->>GuardScript: emit()
GuardScript->>GuardScript: trap '' PIPE
GuardScript->>PipeReader: printf '%s' "$report" >&2 || true
PipeReader-->>PipeReader: exit early
GuardScript-->>Caller: exit with decided status (e.g. 0)
Sequence diagram for guard-sigpipe-test harness executionsequenceDiagram
actor Developer
participant Makefile
participant TestHarness as test-guard-sigpipe.sh
participant GuardScript as check_guard.sh
participant ConsumerTrue as true
participant ConsumerHead as head -1
Developer->>Makefile: make guard-sigpipe-test
Makefile->>TestHarness: ./scripts/test-guard-sigpipe.sh
TestHarness->>GuardScript: run guard | true
GuardScript->>GuardScript: compute verdict, emit() with trap '' PIPE
GuardScript-->>TestHarness: exit status 0
TestHarness->>GuardScript: run guard | head -1
GuardScript->>GuardScript: compute verdict, emit() with trap '' PIPE
GuardScript-->>TestHarness: exit status 0
TestHarness-->>Developer: report guards survive SIGPIPE/EPIPE
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
macanderson
added a commit
that referenced
this pull request
Aug 6, 2026
… and a duplicate Makefile target (#1904) ## Problem Local `make gate` on main (`11c4ca1e`) after the ~20-PR merge flurry (merged untested during the GitHub Actions outage, #1899) found stella-pipeline **does not compile**: the Spend-struct refactor and #1789's degradable budget-abort arms merged textually clean but semantically never met — both `Aborted` arms in `witness_stage.rs` still referenced the pre-refactor `total`/`budget` bindings (E0425 ×4). This one break cascades: `lint`, `test`, `doc-warnings`, and the `wire-schema` serve-frame exporter all go red behind it. Separately, PR #1844 and its stacked PR #1881 each added the identical `guard-sigpipe-test` Makefile target, so every `make` invocation warns about the override. ## Fix - Align the two `Aborted` arms with their sibling `Completed` arms: `*spend.total += cost_usd` and `budget_abort(spend.budget.evaluate())`. Restores exactly the intent both parent PRs tested independently. - Delete the duplicate Makefile target block (kept the first). ## Verification (local — Actions is in a major outage, stated per policy) - `cargo check -p stella-pipeline -j 2`: clean (main: E0425 ×4). - `cargo test -p stella-pipeline -j 2 witness`: **123 passed, 0 failed** — the entire witness-stage suite, which cannot even compile on main, is the witness here; a new test would be dishonest (no new behavior, only the restoration of two intents that were each already tested). - `cargo clippy -p stella-pipeline --all-targets -- -D warnings`: clean. - `make` no longer emits the override warning. ## Remaining main breaks (sibling PRs, from the same gate run) - `toolset.rs` new god file (1756 > 1500) + the `MAX_SERVER_SCHEMA_BYTES` → private `crate::client::ingest` rustdoc link — in the toolset split PR. Refs #1899. ## Summary by Sourcery Fix witness-stage budget handling after a spend-struct refactor and clean up a duplicate Makefile target. Bug Fixes: - Correct witness-stage aborted-turn accounting to update the spend tracker and evaluate the budget from the spend structure, restoring compilation and intended budget behavior. Build: - Remove a duplicated guard-sigpipe-test Makefile target to eliminate override warnings on make runs.
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
1 participant
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.
What & why
The nine
scripts/check-*.shguards that #1815's sweep (PR #1844) left out already decide their verdict before printing, but their final green OK line was still an unguarded pipe write. With stdout piped into a reader that has already exited (| trueis the deterministic repro), that write dies of SIGPIPE (exit 141) or fails with EPIPE andset -eturns it into exit 1 — either way a green verdict reports as a failure. Only the exit code was forged, which is exactly the half a caller reads.This PR copies the epilogue PR #1844 established (exemplar:
scripts/check-left-behind.sh): the verdict is decided first, then the final write runs undertrap '' PIPEwith its failure discarded (|| true). Output text is byte-identical.Scripts hardened:
scripts/check-action-pins.sh— the non-fatal missing-tag-comment report is part of the same decided-green tail, so it rides under the same trap; the unreachable skip branches are left alone, matching fix(gate): decide guard verdicts before printing so a closed pipe cannot forge a failure (#1815) #1844's treatment ofcheck-cargo-install-pins.shscripts/check-brand-case.shscripts/check-design-refs.shscripts/check-empty-diff.shscripts/check-license-allowlist-parity.shscripts/check-no-scratch.shscripts/check-no-secrets.shscripts/check-stat-portability.shscripts/check-wire-schema.shscripts/test-guard-sigpipe.sh(make guard-sigpipe-test) grows a case pair (| true,| head -1) per hardened guard: the seven scanning guards join the main loop,check-empty-diff.shruns against the realHEAD~1 HEADpair (it takes a<base> <head>pair rather than scanning the tree), andcheck-wire-schema.sh— which compiles the two schema exporters — is gated on a cargo toolchain being onPATH, skipping loudly otherwise, per the issue's allowance.Based on
fix/1815-guards-survive-sigpipe(PR #1844) because that PR carries the harness and the epilogue pattern and has not merged yet; retarget tomainafter it lands.Closes #1838
Refs #1815
The witness
scripts/test-guard-sigpipe.sh, extended with 16 new cases, run both ways on this tree (cargo masked offPATH, so the wire-schema case skips):| truecases dying on the final OK write with rc=141 (action-pins, brand-case, design-refs, license-allowlist-parity, no-scratch, no-secrets, stat-portability, empty-diff) — the forged exit this PR removes. The new| head -1cases pass on the old scripts because these guards emit a single line that fits the pipe buffer.event.rs/deck_render.rs), already covered by open unbreak PRs fix(gate): unbreak main — file-size baseline skew and a stale gate-parity count #1845 / docs(gate): unbreak docs-guards — spell the gate's step count as twenty-five #1863 / fix(gate): unbreak main — clippy clone_on_copy, file-size baseline, gate-parity step count #1873.check-wire-schema.shcould not be exercised here (its harness case needs a cargo toolchain and pays a workspace build; this change was verified shell-only) — its epilogue is byte-for-byte the same shape as the eight witnessed ones, and its harness case will run wherever cargo is present.The gate
shellcheckclean on all 10 touched scripts (make shellcheckset)Closes #1838appears both above and as a commit trailerNothing left behind
Ground-rule check
stella-core; no new depsAnything reviewers should know?
trap '' PIPEis process-wide from the point it is set, but in every script it is set only after the last verdict-bearing computation, so it can only affect the best-effort report writes — the same placement PR #1844 reviewed nine times.Summary by Sourcery
Harden gate guard shell scripts against closed-pipe failures, align their reporting patterns, and add a dedicated test harness and Make target to ensure guards remain robust when their output is piped to early-exiting readers.
New Features:
Bug Fixes:
Enhancements:
Build:
Tests:
| true,| head -1) to ensure exit codes remain correct and stable.