Skip to content

fix: repair herdr schema-probe broken pipe and crewmate completion signal - #1182

Open
navidRashik wants to merge 3 commits into
kunchenguid:mainfrom
navidRashik:fm/fix-watcher-completion-signal-1785227753
Open

fix: repair herdr schema-probe broken pipe and crewmate completion signal#1182
navidRashik wants to merge 3 commits into
kunchenguid:mainfrom
navidRashik:fm/fix-watcher-completion-signal-1785227753

Conversation

@navidRashik

Copy link
Copy Markdown

Intent

Fix two bugs found live in production: (1) a broken-pipe stderr write in the herdr backend's schema capability probe, and (2) crewmates finishing without appending a terminal done: status, which makes the watcher wedge-escalate finished tasks forever. Each fix ships with a regression test.

What Changed

  • bin/backends/herdr.sh: Replaced printf ... | grep with herestring (grep -Fq ... <<<"$schema") in the schema capability probe to eliminate a broken-pipe EPIPE error that occurred when grep exited early on the ~220KB schema payload in SIGPIPE-ignoring watcher environments.
  • bin/fm-brief.sh: Added an explicit warning in both crewmate brief templates clarifying that the pane summary is not a completion signal — a missing done: or failed: append causes the watcher to escalate a finished task as a wedge indefinitely.
  • tests/: Added two regression tests (fm-backend-herdr-schema-probe.test.sh, fm-brief-done-signal.test.sh) covering both fixes; both pass in CI.

Risk Assessment

✅ Low: Both fixes are narrow and correct: the herestring substitution eliminates the SIGPIPE race without altering grep exit semantics, and the brief reinforcement is a pure textual addition with no behavioral side-effects; both carry well-structured regression tests.

Testing

Ran both regression tests added by the change. The schema probe test exercised a >220KB multi-line fixture under a SIGPIPE-ignored shell, confirmed no broken-pipe error appears, verified the positive control still reproduces the error with the old pipe form, and confirmed the code no longer pipes to grep. The done-signal test scaffolded a real brief and confirmed all three reinforcement phrases are present, plus a static count guard that both status-protocol blocks carry the reinforcement. All assertions passed (ship-brief runtime test gracefully skipped on macOS bash 3.2 due to a pre-existing heredoc-in-command-substitution limitation unrelated to this change).

Evidence: Regression test output

=== Schema probe regression test === ok - fm_backend_herdr_events_capable: large early-matching schema probed without a broken-pipe write error ok - control: the old pipe form still reproduces the broken-pipe write error on this fixture ok - fm_backend_herdr_events_capable: schema markers are searched without piping the payload === Done-signal regression test === ok - scout brief: terminal-status append is reinforced at the status protocol ok - skip: this bash (3.2.57(1)-release) cannot parse fm-brief.sh's ship half (heredoc in command substitution) ok - bin/fm-brief.sh: all 2 status-protocol blocks reinforce the terminal-status append

=== Schema probe regression test ===
ok - fm_backend_herdr_events_capable: large early-matching schema probed without a broken-pipe write error
ok - control: the old pipe form still reproduces the broken-pipe write error on this fixture
ok - fm_backend_herdr_events_capable: schema markers are searched without piping the payload

=== Done-signal regression test ===
ok - scout brief: terminal-status append is reinforced at the status protocol
ok - skip: this bash (3.2.57(1)-release) cannot parse fm-brief.sh's ship half (heredoc in command substitution)
ok - bin/fm-brief.sh: all 2 status-protocol blocks reinforce the terminal-status append

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-schema-probe.test.sh
  • bash tests/fm-brief-done-signal.test.sh
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

fm_backend_herdr_events_capable searched the ~220KB `herdr api schema
--json` payload with `printf '%s' "$schema" | grep -Fq <marker>`.
`grep -q` exits at the first matching LINE and closes the pipe while
printf still has most of the payload unwritten, so printf's write fails.
Under a parent that ignores SIGPIPE - the watcher's launch environment -
that surfaces as EPIPE instead of killing printf silently, and bash
reports it on the watcher's stderr every arm cycle:

  bin/fm-watch-arm.sh: bin/backends/herdr.sh: printf: write error: Broken pipe

Both marker searches now use a herestring, which bash materializes as a
temp file: no concurrent writer process is left to receive SIGPIPE.

This is a stderr-noise regression, not a capability-verdict regression.
A pipeline's exit status is its last command's, so the verdict was always
grep's and always correct; reverting to the pipe form reproduces both
error lines with rc=0 unchanged. The fix removes the noise and leaves the
verdict identical.

Three preconditions must hold together, which is why it read as
intermittent rather than constant: SIGPIPE ignored in the calling
environment, a payload past the 64KB pipe buffer, and a multi-line
payload whose match lands on an early line.

tests/fm-backend-herdr-schema-probe.test.sh covers all three: the probe
stays silent and still returns capable on a large early-matching schema,
a positive control proves the fixture genuinely exposes the hazard
through the old form, and a static guard keeps a pipe from returning on
any grep implementation.

Audited the rest of bin/backends/herdr.sh for the same shape. The only
other `printf ... | grep` there (the bare-prompt match) runs on a single
ANSI-stripped pane row inside a read loop, far below the pipe buffer, so
it cannot reach this hazard and was left alone.

Two out-of-scope instances elsewhere share the shape and are worth a look
in their own change, not this one: bin/fm-crew-state.sh:259 and :533 pipe
multi-line $RUN_OUT that can plausibly exceed 64KB, and
bin/fm-teardown.sh:352 pipes $pr_patch_ids, which would need roughly 1600
commits to get there.
Observed live on 2026-07-28: four consecutive crewmates finished their
work, wrote their deliverable, printed a coherent closing summary to the
pane, and stopped - none appended the mandatory `done:` line. With no
terminal status the only evidence firstmate has is an idle pane whose
hash stopped changing, and the watcher then wedge-escalates the finished
task forever (escalation 1 ... escalation 9, demand-deep-inspection).

The watcher is not the bug. Its forever-escalation is deliberate
fail-safe behavior, stated in the code at bin/fm-watch.sh:239-249, and
bin/fm-watch.sh:925 already names this exact case:

  (it may be done via an interactive menu that wrote no done: status,

Auto-suppressing a repeatedly-stale pane to quiet this would risk
silently abandoning a genuinely wedged crewmate, which is strictly worse
than a noisy escalation. Per the task's own allowance for a no-code-change
outcome on that layer, bin/fm-watch.sh is left alone.

The root cause is on the brief side. The `done:` append is the only step
of a brief with no deterministic enforcement: the first action and the
worktree-isolation check are both hard STOP gates, while the last action
was one bare instruction in a list, easily lost to a mid-task context
compaction that takes the status-file path with it. Rule 4 owns the
status protocol, so this is an owner-local reinforcement, not a
restatement elsewhere: it names the pane summary as a non-signal, states
the concrete consequence, and places the append as the task's last action.

Known remaining and out of scope: the stuck-crewmate-recovery skill's
live-endpoint escalation list has no "finished but silent" branch, so it
can relaunch a worker that is already done.

tests/fm-brief-done-signal.test.sh scaffolds a scout brief and asserts
the reinforcement. The ship half is asserted statically as well, because
fm-brief.sh's delivery-mode Definition of done uses a heredoc inside a
command substitution, which bash 3.2 (system bash on macOS) cannot parse
- pre-existing and equally true at HEAD.

bin/fm-lint.sh clean (ShellCheck 0.11.0).
Sign up for free to 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.

1 participant