fix(bin): stop provider-table lookup from writing broken-pipe errors to stderr - #6001
Conversation
Fixes kunchenguid#5956 fm_quota_single_provider_for_harness returned from its while read loop as soon as it found a match, closing the pipe while fm_quota_single_provider_table's printf could still be writing. Where SIGPIPE is ignored, as on GitHub Actions runners, bash then prints "printf: write error: Broken pipe" on the resolver's stderr, which intermittently broke the one-diagnostic-line assertions in tests/fm-dispatch-resolve.test.sh. Read the whole table before answering, the way fm_control_harness_supported already does, so the writer always finishes. Return values and output are unchanged. Reproduced by running tests/fm-dispatch-resolve.test.sh with SIGPIPE ignored on a single pinned core under CPU contention: 30 of 30 runs failed before the fix, 0 of 30 after. Note: reproducing requires setting the trap inside the tested shell because nice(1) resets an inherited SIGPIPE ignore to SIG_DFL. tests/fm-quota-choose.test.sh passes and bin/fm-lint.sh is clean.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ss. ci-1 (bin/fm-quota-axi-lib.sh:154). Invariant: looking up a harness must always end with status 0 and print the provider, even when the caller runs under `set -e`. The loop body `[ -z "$found" ] && [ "$harness" = "$1" ] && found=$provider` now ends in `|| :`. Every iteration succeeds and the whole table is still read. Only `fm_quota_single_provider_for_harness` loops over the table this way, so this is the one place the fix was needed. One caveat: on bash 5.3 the old code did not actually exit under `set -e`, because the `while` loop is not the function's last command, so the new `set -e` test would have passed before this fix too. The change makes the loop's success explicit, as the user asked. ci-2 (regression coverage). I added three cases to the existing `tests/fm-quota-choose.test.sh`, all calling the public lookup function after sourcing the library: 1. With SIGPIPE ignored (`trap "" PIPE`), it looks up every harness 200 times and checks that nothing reaches stderr. 2. A deterministic version of the race: the table function is wrapped so it writes the first row, pauses 0.2 s, then writes the rest. With SIGPIPE ignored, it checks that looking up `claude` prints `claude` and writes nothing to stderr. The stress loop alone reproduced the bug in only about 1 of 5 local runs, which is why this case exists. 3. A direct call under `set -e` prints `claude`. Verification: - `bash tests/fm-quota-choose.test.sh`: all pass. - Same test against the pre-PR library (fa48367, via `FM_ROOT_OVERRIDE`): fails with `printf: write error: Broken pipe`. The deterministic case failed in one run and the stress loop caught it in another. - `shellcheck` on both files: clean. - `tests/fm-dispatch-resolve.test.sh`: passes
|
Speaking as Kun's firstmate: whole thread read (body + Greptile; linked #5956 OPEN ready-for-pr). Diff reviewed against main tip Verdict: Tip vs main: Attestation: MATCH — body CI / NM: Require no-mistakes SUCCESS (run 36413510896). CI 36413510766: Behavior portable serial 5 FAILURE (job 108899064696 — VISION.md per-rule:
Next: waiting on green CI (serial 5). Do not merge while CI red. No Firstmate flag. No workflow approvals this pass (runs already executing on head). |
|
Friendly nudge: head |
|
Speaking as Kun's firstmate: whole thread re-read (body + Greptile + prior waiting-ci stamp + author nudge that serial 5 re-ran green). Diff re-reviewed vs main HEAD contract-class: restore — VISION.md (each rule):
Auto-merging (restore + otherwise ready). |
|
Speaking as Kun's firstmate: this is merged. Thank you @tiago-peixoto — really appreciate you taking the time on this. |
2 participants
Intent
Fixes #5956
Implement ready-for-pr issue #5956: tests/fm-dispatch-resolve.test.sh fails intermittently in CI because a provider-table lookup in bin/fm-quota-axi-lib.sh writes "printf: write error: Broken pipe" to stderr. fm_quota_single_provider_for_harness reads the table from fm_quota_single_provider_table through a pipe and returns on the first match while the writer may still be printing; on GitHub Actions SIGPIPE is ignored, so bash reports the write error, which becomes a second diagnostic line. Looking up a harness in the provider table must never produce stderr output, whatever the timing and whether or not SIGPIPE is ignored, with return values and output unchanged.
What Changed
fm_quota_single_provider_for_harnessinbin/fm-quota-axi-lib.shnow reads the whole output offm_quota_single_provider_tablebefore it answers. Before this change it returned on the first match, which closed the pipe while the table was still being written. Where SIGPIPE is ignored (for example on GitHub Actions), the writer then printedprintf: write error: Broken pipeto stderr.Fixes #5956
Risk Assessment
✅ Low: This is a small, well-bounded change. The loop now drains the whole process-substitution table before answering, so the writer cannot hit EPIPE. First-match semantics, output and return codes are preserved for every table row (all have non-empty providers) and for harnesses not in the table, and the && chain inside the loop is safe under set -e.
Testing
I sourced the real library function and ran it in a stress loop with SIGPIPE ignored, as GitHub Actions does. This reproduced the bug on the base commit: 7 broken-pipe lines sequentially and 229, then 171, under parallel load. The target commit produced no stderr output in every run, including with default SIGPIPE handling. Output and exit codes matched the base for every harness and for the unknown and edge-case inputs. The issue's failing test, tests/fm-dispatch-resolve.test.sh, passed 3 out of 3 times with SIGPIPE ignored and printed no broken-pipe lines. Everything passed. I removed the temporary files afterwards and the worktree is clean.
Evidence: SIGPIPE-ignored stress transcript, base vs target
Source: SIGPIPE-ignored stress transcript, base vs target
base fa48367: stderr lines=171 (base-lib.sh: line 138: printf: write error: Broken pipe) target a1608f1: stderr lines=0 fm-dispatch-resolve.test.sh with SIGPIPE ignored: passed 3/3Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-quota-axi-lib.sh:151- No regression test accompanies the fix. The original failure depends on timing (the writer is still printing after the reader has returned) and on SIGPIPE being ignored, which makes a deterministic test hard to build against a 7-row table. The existing tests/fm-dispatch-resolve.test.sh paths still exercise the behavior: return values and output are unchanged for every table row and for a harness that is not in the table. No action needed.✅ **Test** - passed
✅ No issues found.
trap '' PIPEstress driver sourcing the base fa48367 lib versus the target lib: 3000×3 lookups sequentially, then 4 parallel runs of 4000×3 lookupsStress driver on the target lib with default SIGPIPE handling (3000×3 lookups)Parity check of base versus target output and exit code for claude, codex, grok, kimi, cursor, agy, muse, omp, pi, opencode, the empty string, bogus, and 'claude claude', with SIGPIPE ignoredbash -c "trap '' PIPE; exec bash tests/fm-dispatch-resolve.test.sh"run 3 times✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.