Skip to content

fix(wip-limit): avoid sort|head SIGPIPE failing the WIP check when queue >10 - #61

Merged
saadqbal merged 1 commit into
mainfrom
fix/wip-limit-broken-pipe
Jun 18, 2026
Merged

fix(wip-limit): avoid sort|head SIGPIPE failing the WIP check when queue >10#61
saadqbal merged 1 commit into
mainfrom
fix/wip-limit-broken-pipe

Conversation

@shujaatTracebloc

@shujaatTraceblocshujaatTracebloc commented Jun 17, 2026

Copy link
Copy Markdown

Problem

The reusable WIP limit check (.github/workflows/wip-limit-check.yml) fails with exit code 2 for any PR when the Code review queue is over the limit and has more than 10 items. Seen on tracebloc-py-package#237 — queue at 36/30:

Code review queue depth: 36 (limit: 30)
sort: fflush failed: 'standard output': Broken pipe
sort: write error
##[error]Process completed with exit code 2.

Cause

The over-limit comment path builds its list with:

jq -r '...' /tmp/inreview.txt | sort | head -10

head -10 closes the pipe after 10 lines; sort then hits EPIPE on its next write and exits non-zero. Under set -euo pipefail that non-zero pipe status fails the whole step. It's a size/timing race — small queues let sort flush before head closes (why cli's check passed earlier at ≤30), so it only surfaced once the queue grew past 10 and over the limit.

Fix

Sort to a temp file, then head the file — sort never shares a pipe with a short-circuiting reader, so the broken pipe can't happen regardless of queue size. Output is unchanged (still the 10 oldest, sorted). The over-limit nudge comment now posts and the step exits 0.

Impact

This reusable workflow is called by every repo's PR CI, so the fix unblocks the WIP check fleet-wide. tracebloc-py-package#237's check stays red until this merges to main (its caller pins @main).

🤖 Generated with Claude Code


Note

Low Risk
CI-only bash change in the over-limit comment path; no app, auth, or data handling impact.

Overview
When the Code review queue is over the WIP limit and has more than 10 PRs, the reusable WIP limit check workflow could fail the step instead of posting its nudge comment. The over-limit path listed PRs with jq … | sort | head -10; with set -o pipefail, head closing the pipe after 10 lines made sort exit on EPIPE, so the job failed (e.g. exit code 2) even though the check is meant to be non-blocking.

The workflow now writes the sorted list to /tmp/inreview_sorted.txt, then runs head -10 on that file, so sort is not piped to a short-circuiting reader. Comment content is unchanged (still up to 10 oldest queue entries). A short comment in the YAML documents why sort | head must not be used here.

Reviewed by Cursor Bugbot for commit fb0dc8a. Bugbot is set up for automated code reviews on this repo. Configure here.

When the Code review queue is over the WIP limit AND has more than 10
PRs, the comment-building pipeline `jq ... | sort | head -10` fails the
step with exit 2: head closes the pipe after 10 lines, sort then gets
EPIPE on its next write ("sort: write error" / "fflush failed: Broken
pipe"), and `set -euo pipefail` propagates that non-zero status.
It's a size/timing race — small queues let sort finish before head
closes, which is why it only surfaced once the queue reached 36/30
(e.g. tracebloc-py-package#237's WIP check). Sort to a temp file, then
head the file, so sort never shares a pipe with a short-circuiting
reader.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@shujaatTraceblocshujaatTracebloc self-assigned this Jun 17, 2026
@LukasWodka

Copy link
Copy Markdown
Contributor

👋 Heads-up — Code review queue is at 32 / 30

Above the WIP limit. The team convention is to review existing PRs before opening new work.

Open PRs currently in Code review (oldest first):

Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.)

This was referenced Jun 17, 2026
@saadqbal
saadqbal merged commit e7ce01a into mainJun 18, 2026
5 checks passed
LukasWodka added a commit that referenced this pull request Jul 26, 2026
* fix(fr-gate): fail closed on an unreadable or absent Status
The gate treated "could not evaluate this item" as "this item is fine". An
unreadable or missing Status hit `continue`, and if every item was unreadable
the gate printed "PASSED". A missing or expired PROJECTS_KANBAN_TOKEN, a
repo-level secret shadowing the org one, or a transient GraphQL 5xx all
collapsed into a silent pass — the only promotion gate disarming itself, with
failure indistinguishable from success.
Two changes:
- resolve_status() retries up to 5 times and distinguishes an API failure
(rc=1) from a PR genuinely not on the board (rc=2). The empty-result retry is
the important half: the empty case is usually a RACE, not an absence.
fr-gate-caller.yml and add-to-kanban.yml both fire on `pull_request: opened`
and run concurrently, so the gate routinely queries the board before the card
exists. Observed on #61 — gate finished at 16:48:09, card
created at 16:48:14, and it passed on "not on kanban". Measured 4/32, 3/20,
3/17 items skipped this way on real promotion runs.
- The verdict blocks on BLOCKED, MISSING, or UNREADABLE, each with its own
actionable message, instead of only on BLOCKED.
This also closes the delete-card-then-add-a-label bypass: the label trigger
re-runs the gate, add-to-kanban does not recreate the card, and the retry now
gives the card time to reappear rather than passing on its absence.
Verified: the return-code contract holds under a stubbed gh (card -> 0,
no-card -> 2, API-failure -> 1), and actionlint passes.
Refs RFC-BACKEND-0008 D27-L2/L6 (tracebloc/backend#1266)
* fix(fr-gate): make the Status parse non-fatal and stop corrupting JSON
Bugbot on #71 caught two robustness bugs I introduced:
- The gh success capture used 2>&1, so any stderr warning was merged into the
JSON body and fed to jq — a spurious parse failure on an otherwise good call.
Capture stdout only (2>/dev/null).
- The jq|head pipeline was unguarded under set -euo pipefail. A jq error, or a
SIGPIPE from head closing the pipe early, would abort resolve_status mid-loop,
skip the remaining retries, and return an ambiguous code that the caller then
mislabelled as MISSING with a '5 attempts' message that never ran. Parse is
now non-fatal (2>/dev/null on jq, || true on the pipeline) so a failed parse
falls through to the next retry.
Also made the end-of-loop sleep an explicit if, not a '[ ] && sleep' list,
which is a set -e footgun as a bare statement.
Return-code contract re-verified under a stubbed gh, including a case where gh
writes to stderr but returns 0 — the JSON now parses cleanly where 2>&1 would
have broken it.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@shujaatTracebloc@LukasWodka@saadqbal