fix(herdr): stop Broken-pipe stderr noise on every capability probe - #1335
Open
wameson wants to merge 1 commit into
Open
fix(herdr): stop Broken-pipe stderr noise on every capability probe#1335wameson wants to merge 1 commit into
wameson wants to merge 1 commit into
Conversation
fm_backend_herdr_events_capable piped the (large) API schema into `grep -Fq`. grep -q exits on first match and closes the read end of the pipe while printf is still writing, so printf gets EPIPE and prints `printf: write error: Broken pipe` to stderr on essentially every watcher cycle. Replace the two pipe-into-grep tests with `case` globs that match the in-memory schema variable directly. No subshell, no pipe, no reader that can close early, so no EPIPE; `*literal*` is a literal substring match equivalent to grep -F, so capability detection is behaviorally identical (capable only when both events.subscribe and pane.agent_status_changed are present) and not weakened. The only other pipe-into-grep in the file (the bare-prompt regex on a single stripped terminal line) is a regex match on a tiny buffer that never overflows the pipe, so it produces no such noise and is left unchanged. Add colocated regression tests proving both-tokens-present is capable, either-token-missing is not capable, and the probe leaves stderr clean (the tests run under `trap '' PIPE`, the disposition under which the noise actually surfaces, and fail against the old grep-piped code).
5 tasks
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.
Intent
Fix the 'printf: write error: Broken pipe' stderr noise the herdr backend writes on every capability check. Root cause: fm_backend_herdr_events_capable in bin/backends/herdr.sh piped the large API schema into 'grep -Fq'; grep -q exits on first match and closes the read end of the pipe while printf is still writing, so printf gets EPIPE and prints the broken-pipe message to stderr on essentially every watcher cycle. Fix: replace the two printf|grep -Fq lines with in-memory 'case $schema in token' substring globs - no subshell, no pipe, no early-closing reader, so no EPIPE, and faster. Behavior must NOT change: the case glob is semantically identical to grep -F (literal substring match), returning success only when BOTH events.subscribe and pane.agent_status_changed are present and return 1 otherwise; neither token contains a glob metacharacter so the match is equivalent - detection unchanged, not weakened. The one other pipe-into-grep in the file (a grep -qE regex on a single stripped terminal line) is a tiny buffer that never overflows the pipe, produces no such noise, and can't take the same substring fix, so it was left unchanged. Added three colocated regression tests in tests/fm-backend-herdr.test.sh proving both-tokens-present is capable, either-token-missing is not capable, and the probe leaves stderr clean; the clean-stderr test runs under 'trap "" PIPE' against a >64KiB schema and was verified to fail against the old grep-piped code and pass against the fix. Firstmate-repo change to shared tracked tooling following firstmate-coding-guidelines. Delivery is via the wameson/firstmate fork (no push access to upstream kunchenguid/firstmate); the PR must be opened by no-mistakes so it carries the required signature line.
What Changed
printf '%s' "$schema" | grep -Fq ...lines infm_backend_herdr_events_capable(bin/backends/herdr.sh) with in-memorycase "$schema" in *token*substring globs, so no subshell or pipe is created andgrep -qcan no longer close the read end early and triggerprintf: write error: Broken pipeon stderr each watcher cycle. Detection is unchanged: the probe still succeeds only when bothevents.subscribeandpane.agent_status_changedare present.tests/fm-backend-herdr.test.shcovering both-tokens-present (capable), a token missing (not capable), and a clean-stderr probe run undertrap "" PIPEagainst a >64KiB schema; the stderr test was verified to fail against the old grep-piped code and pass against the fix.Risk Assessment
✅ Low: A well-bounded, single-purpose fix that replaces two pipe-into-grep substring checks with semantically identical in-memory case globs, with three colocated regression tests that correctly reproduce and pin the EPIPE noise.
Testing
Exercised the herdr capability-probe fix end-to-end. The full
tests/fm-backend-herdr.test.shsuite passes (exit 0) with the three new colocated regression tests. I proved the fix matters and the test is meaningful by reproducing the exact end-user symptom — a 240KiB schema undertrap "" PIPEmakes the oldprintf | grep -Fqpath emitprintf: write error: Broken pipetwice to stderr, while the newcase "$schema" in *TOKEN*globs leave stderr clean — and by reverting herdr.sh to the old code, which flips the suite to a failingnot ok - capability probe emitted 'Broken pipe'(exit 1). Detection is confirmed unchanged (capable iff both tokens present). This is a shell/CLI-only change with no user-visible UI surface, so a CLI/stderr transcript is the appropriate evidence rather than a screenshot. Worktree restored clean; no transient artifacts left behind.Evidence: Old-vs-new stderr behavior (240KiB schema under trap '' PIPE)
Evidence: Regression test catches old code (suite run against reverted herdr.sh)
Evidence: Full evidence transcript
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-backend-herdr.test.sh— full suite passes (exit 0), including the three newtest_events_capable_*casesReverted herdr.sh to the oldprintf | grep -Fqcode and re-ran the suite: it fails (exit 1) withnot ok - capability probe emitted 'Broken pipe' on stderr, proving the regression test catches the bug; then restored the fix and confirmedgit status --porcelaincleanDirect old-vs-new symptom reproduction undertrap "" PIPEagainst a 240KiB schema: old code printsprintf: write error: Broken pipetwice, new case-glob code produces empty/clean stderr✅ **Document** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.