fix(afk): prevent repeated Herdr escalation delivery - #2655
Open
xmadscientist wants to merge 11 commits into
Open
fix(afk): prevent repeated Herdr escalation delivery#2655xmadscientist wants to merge 11 commits into
xmadscientist wants to merge 11 commits into
Conversation
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 repeated away-mode escalation delivery on the Herdr backend. Reproduce and identify the causal path across Herdr submit confirmation, escalation-buffer retirement, daemon shutdown/flush, and terminal reconciliation, using the observed repeated buffered status/paused-wait digest, wedge marker, stale supervision, and delayed exact-terminal cleanup only as diagnostic evidence rather than prescribing the patch. Ensure a successfully delivered escalation retires exactly once and cannot be re-injected by catch-all scans or stale buffered state. Preserve type-once, Enter-only retry, durable delivery, affirmatively empty composer, and no-loss contracts. Preserve safe shutdown semantics so unconfirmed or failed delivery remains durable while confirmed delivery cannot block SIGTERM shutdown indefinitely. Keep every Herdr lifecycle action inside the named non-default afk-herdr-repeated-escalation-submit lab via bin/fm-herdr-lab.sh, never touch the captain's default Herdr session, and never bypass the helper or use forbidden server-global/direct lifecycle commands. Add executable regression coverage through public interfaces for confirmed and unconfirmed delivery, buffer retirement, repeated scan/dedupe behavior, shutdown during and after flush, and guarded terminal reconciliation where applicable. Keep executable repeated scan/dedupe coverage on the real daemon heartbeat path, scope the Herdr CLI deadline only around supervisor-target existence, composer, and submit transport calls so worker reads retain existing timeout behavior, and require affirmative fm-herdr-lab helper provenance plus a running named non-default lab before Herdr E2E or launch tests perform lifecycle actions. Review every affected supported primary harness and runtime backend, scope backend-specific changes, and document non-applicable axes with evidence. Update authoritative maintainer/runtime documentation and current verification evidence only where behavior or guarantees change, without incident chronology; keep docs/herdr-backend.md as the Herdr-specific mechanism owner, retain operational pointers elsewhere, and remove duplicated mechanism detail. Run focused tests, bin/fm-lint.sh, bin/fm-doc-audience-check.sh, and safe automated Herdr verification available in the isolated lab. The captain will manually test the UI after the PR is ready and instructed that manual UI testing not be performed here. Preserve every previously accepted pipeline fix commit. Do not discard state, force cleanup, kill shared supervision broadly, hand-edit, cherry-pick, reset, drop commits, force-push, merge, modify unrelated behavior, or weaken any safety contract.
What Changed
Risk Assessment
✅ Low: The change is well-bounded and closes the confirmed-delivery shutdown race while preserving unconfirmed buffers, heartbeat deduplication, transport scope, and guarded Herdr lab ownership.
Testing
No baseline output was supplied; targeted portable tests, terminal reconciliation, a red-before-green base counterfactual, and guarded Herdr 0.8.0 live scenarios passed after a command-scoped locale correction. Evidence transcripts were captured, while the full suite, lint, documentation static checks, and manual UI testing were intentionally not run under this test-phase boundary; UI testing remains reserved for the captain.
Evidence: Live named-lab Herdr delivery evidence
Source: Live named-lab Herdr delivery evidence
Evidence: Base-commit regression reproduction
Source: Base-commit regression reproduction
Evidence: Daemon retirement and shutdown evidence
Source: Daemon retirement and shutdown evidence
Evidence: Real heartbeat dedupe evidence
Source: Real heartbeat dedupe evidence
Evidence: Lab provenance guard evidence
Source: Lab provenance guard evidence
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
🔧 **Rebase** - 1 issue found → auto-fixed ✅
docs/architecture.md- merge conflict rebasing onto origin/main🔧 Fix applied.
✅ Re-checked - no issues remain.
🔧 **Review** - 4 issues found → auto-fixed (2) ✅
bin/fm-supervise-daemon.sh:1179- The required criterion says to "scope the Herdr CLI deadline only around supervisor-target existence, composer, and submit transport calls," but this hunk also wrapspane_is_busy, and its busy-state and capture reads inherit the deadline. The new test even expectsbusy 7. Confirm the intended behavior or remove the wrapper from the busy path so only the three authorized operations are bounded.bin/fm-supervise-daemon.sh:601- The timeout validator rejects only the literal0; values such as00reachfm_run_timedas zero and disable its deadline. A wedged Herdr call can then block shutdown indefinitely despite the documented zero-uses-default guarantee. Reject any numerically zero representation before exporting the timeout.tests/fm-daemon.test.sh:813- On the regression failure path, the test signals the watcher's entire process group without verifying that the watcher is still the daemon's child and its own group leader. If isolation is the behavior that regressed, this can signal the test runner or sibling processes. Apply the production parentage andpgid == watcher_pidauthority check before group signaling.tests/fm-backend-herdr.test.sh:41- Fixture PIDs remain recorded after their processes are killed and reaped during individual tests. EXIT cleanup later signals those stale numeric PIDs again, so PID reuse can terminate an unrelated process. Unregister each reaped fixture or verify a durable identity before signaling it.🔧 Fix: Captain: harden Herdr timeout and process cleanup
3 issues (1 error, 2 warnings) still open:
tests/fm-backend-herdr.test.sh:67- EXIT cleanup scans the shared process group and signals every exactsleep 300. Process-group membership is not fixture ownership, so a concurrent suite or second invocation can lose an unrelated process. Remove this fallback now that fixture stdio is redirected, or track an exact child/group identity.bin/fm-supervise-daemon.sh:137- The usage header claims every Herdr supervisor transport call is bounded, butpane_is_busyintentionally leaves native-state and capture reads unbounded. Name only target existence, composer state, and submit, as required, and correct the same overstatement indocs/configuration.md:625,docs/herdr-backend.md:208, anddocs/verification/supervision.md:518.bin/backends/herdr.sh:85- Sourcing the newly imported timeout library executes its top-levelset -u, so the documented interactive zsh diagnostic pathfm_backend_source herdrnow permanently enables nounset in the caller and can abort later unset-variable reads. Preserve and restore the caller's option state, or make the sourced timeout library option-neutral.🔧 Fix: Captain: tighten Herdr timeout scope and cleanup ownership
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
tests/fm-daemon.test.shtests/fm-herdr-lab.test.shtests/fm-backend-herdr.test.shreproduced a setup-only Apple Perl locale failure under ambientC.UTF-8LC_ALL=C LC_CTYPE=C LANG=C tests/fm-backend-herdr.test.shtests/fm-backend.test.shtests/fm-wake-daemon-lifecycle-e2e.test.shtests/fm-afk-return.test.shHERDR_LAB_SESSION=<helper-named afk-herdr-repeated-escalation-submit lab> tests/fm-afk-launch.test.shHERDR_LAB_SESSION=<same lab> tests/fm-afk-inject-herdr-e2e.test.shscenarios A-FBase counterfactual:FM_AFK_HERDR_SCENARIOS=e ... tests/fm-afk-inject-herdr-e2e.test.shagainst archived commit87681a40777bb061ef923ef98b494cd7ef6054b6Reviewed the affected harness/backend applicability table and commit preservation withgit diffandgit log --reverseVerified clean worktree, removed base-reproduction directory, absent lab tripwires, and no surviving test processes✅ **Document** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.