fix: dedup herdr stale wakes for settled panes#268
Conversation
… hash An idle or done claude pane on the herdr backend repaints timer-driven volatile UI even when the crew has not changed - the recap line toggling, rotating tips, the animated "for Xm Ys" summary, and transient slash-completion menus. bin/fm-watch.sh's stale detection hashes a bounded pane capture, so each repaint mints a fresh stable hash and re-wakes the supervisor for a crew that is genuinely settled. Verified live against real herdr 0.7.1 on 2026-07-05: capturing pane content every 20s for ~7 minutes, a done pane produced 4 distinct hashes and an idle pane 8 (each a spurious stale re-wake), while a plain post-exit shell pane stayed at 1. Line-stripping the volatile rows before hashing does not converge, because those rows also shift a variable-length prefix through the tail-N hash window. For a backend with a native settled agent state (herdr reports idle for idle/done/blocked), the stale-dedup signal is now that semantic state (agentstate:idle) rather than the pane content, so a settled pane that merely repaints keeps one signal and is surfaced once; a genuine state change still mints a new signal and surfaces once. tmux (busy state always unknown) and a working/unknown herdr pane keep the pane-content hash exactly as before, so the proven default path is unchanged byte-for-byte. The resolved busy state is shared between this signal and the existing busy-suppression check, so herdr's agent.get is read once per pane per poll, not twice; the now-unused window_is_busy wrapper is removed in favor of busy_state_is_busy. Adds a colocated behavioral test (a settled herdr pane with a repainting capture surfaces one stale wake whose recorded signal is the semantic state, not a content hash) and a backend-verification section in docs/herdr-backend.md with the measured churn evidence. The related herdr away-mode injection wedge is already fixed upstream by kunchenguid#251; this change is scoped to the watcher stale-echo churn that kunchenguid#251 did not touch.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 874289c1c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| [ ! -e "$state/.stale-$key" ] || { reap "$pid"; fail "busy transition did not clear the stale suppressor"; } | ||
| printf 'idle\n' > "$fakebin/agent-status" | ||
| printf 'settled again\n' > "$fakebin/pane-body" | ||
| wait_for_exit "$pid" 40 || fail "watcher did not resurface a settled herdr pane after intervening work" |
There was a problem hiding this comment.
Increase the resurface test timeout
Captain, this new test races the watcher’s own polling cadence: after flipping the fake herdr pane back to idle, the watcher may need one poll to record the new agentstate:idle signal and two more polls to reach the n>=2 stale threshold, so the 40×0.1s budget is too tight once shell/herdr-stub overhead is included. I reproduced bash tests/fm-watch-triage.test.sh failing here with not ok - watcher did not resurface a settled herdr pane after intervening work; use a larger timeout or pre-seed the post-transition count so CI does not flake/fail on slower hosts.
Useful? React with 👍 / 👎.
Intent
Fix the herdr-backend watcher stale-echo churn: idle and done crews' herdr panes re-wake the supervisor every few minutes because bin/fm-watch.sh hashes a volatile bounded pane capture and claude's idle UI repaints timer-driven rows (the recap line toggling, rotating tips, the animated 'for Xm Ys' summary, transient slash-completion menus), so each repaint mints a fresh stable hash. Decision: dedup the stale signal on the native SEMANTIC agent state (herdr idle/done/blocked map to a constant 'agentstate:idle') instead of the pane content, because content line-stripping cannot converge - those volatile rows also shift a variable-length prefix through the tail-N hash window (measured). Constraints held: the tmux path stays byte-identical (busy state is always unknown there, so it keeps the pane-content hash); the resolved backend busy state is shared between this new stale signal and the existing busy-suppression check so herdr's agent.get is read once per pane per poll, not twice; the now-unused window_is_busy wrapper is removed in favor of the extracted busy_state_is_busy. Verified live against real herdr 0.7.1 on 2026-07-05 (a done pane churned 4 distinct hashes and an idle pane 8 over ~7 minutes, both collapsing to 1 under the semantic signal; a plain post-exit shell pane stayed at 1 and is intentionally left on the pane-hash fallback). The related herdr away-mode injection wedge is already fixed upstream by PR #251, so this change is deliberately scoped to only the watcher stale-echo churn #251 did not touch. Adds a colocated behavioral test in tests/fm-watch-triage.test.sh and a backend-verification section in docs/herdr-backend.md.
What Changed
bin/fm-watch.shnow deduplicates settled herdr stale panes using the semantic agent state instead of volatile pane hashes, while keeping tmux and busy-looking herdr panes on the existing content-hash path.Risk Assessment
✅ Low: Captain, the change is narrow, the semantic herdr stale signal matches the documented backend contract, and the added tests cover the prior regressions without introducing a material merge risk.
Testing
The configured full baseline had already passed; I reran the targeted watcher triage and session-start regression suites, then exercised a fake herdr idle pane through the real watcher and drain path, confirming semantic stale dedup wakes once, stores
agentstate:idle, and does not re-wake after volatile pane repaint.Evidence: fm-watch-triage transcript
Evidence: fm-session-start transcript
Evidence: Manual fake-herdr semantic stale evidence
Shows first watcher poll emittedstale: default:w1:p2, recordedhash=agentstate:idle, drained one stale wake, then stayed running with empty stdout and empty wake queue after volatile pane repaint.Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed (2) ✅
bin/fm-watch.sh:195- Using one constant stale key for every settled herdr period means.stale-$keyremainsagentstate:idleafter the first surfaced stale pane. If the same pane later does more work and returns to idle/done without a status/turn-ended signal, the stale path treats it as already surfaced and can suppress or delay the new completion wake. Clear the stale suppressor when the signal leaves settled-idle, or include a transition/generation component in the semantic key.🔧 Fix: Captain, reset stale suppressor on signal change
1 error still open:
bin/fm-watch.sh:195- Captain,idleis not always a settled herdr pane: the existing docs note that long foreground tool calls can reportagent_status=idlewhile the pane still shows the busy banner. Collapsing everyidleverdict toagentstate:idlemeans changing tool output no longer resets the stale generation; in afk mode it can queue a stale wake after two polls, and in normal mode it can keep or suppress the stale marker across real intervening work. Corroborateidlewith the pane busy regex before using the semantic settled signal, or keep the pane-content hash while the idle pane still looks busy.🔧 Fix: Captain, corroborate idle stale signal
✅ Re-checked - no issues remain.
🔧 **Test** - 1 issue found → auto-fixed ✅
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"🔧 Fix: Captain, stabilize session-start missing-tool fixture
✅ Re-checked - no issues remain.
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"Baseline already ran successfully before this validation:command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"bash tests/fm-watch-triage.test.sh | tee /tmp/no-mistakes-evidence/01KWRQ2K32MZPGK20MYN5K2X1D/fm-watch-triage.outbash tests/fm-session-start.test.sh | tee /tmp/no-mistakes-evidence/01KWRQ2K32MZPGK20MYN5K2X1D/fm-session-start.outManual fake-herdr watcher/drain scenario saved withtee /tmp/no-mistakes-evidence/01KWRQ2K32MZPGK20MYN5K2X1D/herdr-semantic-stale-manual.out✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.