Skip to content

fix(afk): prevent repeated Herdr escalation delivery - #2655

Open
xmadscientist wants to merge 11 commits into
kunchenguid:mainfrom
xmadscientist:fm/afk-herdr-repeated-escalation-submit
Open

fix(afk): prevent repeated Herdr escalation delivery#2655
xmadscientist wants to merge 11 commits into
kunchenguid:mainfrom
xmadscientist:fm/afk-herdr-repeated-escalation-submit

Conversation

@xmadscientist

Copy link
Copy Markdown

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

  • Make away-mode shutdown cooperative so confirmed Herdr submissions retire through one path, while unconfirmed work remains durable and cleanup cannot resend it.
  • Bound Herdr supervisor target, composer, and submit calls without changing worker-read timeouts, and safely stop the daemon's verified watcher process group.
  • Add guarded lab and daemon regression coverage for repeated scans, shutdown timing, buffer cleanup, and terminal reconciliation, with updated runtime documentation.

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

lab_session=fm-lab-afk-herdr-repeat-72373-31579
ok - clear-stale: removes escalations buffer, sidecar, and wedge marker
ok - clear-stale: leaves the durable wake-queue intact (no pending work dropped)
ok - launcher paths: relative home and state ignore CDPATH before daemon command construction
ok - launcher paths: absolute symlink spellings are preserved
ok - launcher paths: unresolved relative FM_HOME fails loudly
ok - launcher paths: unresolved relative FM_STATE_OVERRIDE fails loudly
ok - refresh: daemon already alive - stale artifacts preserved (current session's buffer kept)
ok - stop-ordering: daemon SIGTERM'd while .afk still present (active delivery may settle)
ok - stop-ordering: .afk cleared last
ok - stop-ordering: daemon-terminal record removed
ok - stop identity: stale lock cannot signal an unrelated live process
ok - failed start: away flag and delivery artifacts roll back
skip: tmux not found (concurrent start)
ok - launcher lock: incomplete publication receives initialization grace
ok - launcher signal: TERM exits and releases the lifecycle lock
fm-afk-launch: daemon launched in non-visible herdr workspace ws-partial (pane lab:pane-exact), supervising lab:captain
ok - herdr create: malformed response recovers durable exact ownership
fm-afk-launch: herdr create failed after returning exact ids; closing lab:pane-exact
fm-afk-launch: recorded terminal teardown is unconfirmed; preserving exact id
ok - herdr create error: unconfirmed exact id is persisted for reconciliation
fm-afk-launch: failed to run daemon in herdr pane lab:pane-exact; closing it
fm-afk-launch: recorded terminal teardown is unconfirmed; preserving exact id
ok - herdr run failure: unconfirmed exact id remains reconcilable
fm-afk-launch: failed to persist daemon terminal record; closing tmux:exact-session
ok - record failure: newly created terminal is closed by exact id
fm-afk-launch: daemon did not become ready; closing tmux:exact-session
ok - readiness failure: exact terminal and durable record roll back
fm-afk-launch: daemon did not become ready; closing tmux:exact-session
fm-afk-launch: recorded terminal teardown is unconfirmed; preserving exact id
ok - readiness failure: unconfirmed terminal retains its reconciliation id
ok - tmux absence: clean missing differs from transport probe failure
ok - native lifecycle: launcher owns state with no terminal
ok - native lifecycle: uniform stop clears state without closing a terminal
ok - native entry: launcher-prepared lifecycle state is not rewritten
fm-afk-launch: reconciling leaked daemon terminal tmux:exact-session
fm-afk-launch: recorded terminal teardown is unconfirmed; preserving exact id
ok - teardown failure: exact terminal record is preserved
ok - record publication: failed atomic rename preserves the complete prior record
fm-afk-launch: daemon terminal record is malformed; refusing to act on it
ok - record read: malformed record fails closed without acting on a partial id
fm-afk-launch: daemon terminal record is malformed; refusing to act on it
fm-afk-launch: malformed daemon terminal record; refusing to stop away mode
ok - stop: malformed terminal record preserves away state and fails closed
fm-afk-launch: failed to create detached tmux daemon session 'fm-afk-daemon-3077714769-80909-24684-1787183211'
ok - tmux launch: planned exact target is recorded before creation and removed on failure
fm-afk-launch: failed to create detached tmux daemon session 'fm-afk-daemon-1992066620-80946-25110-1787183211'
ok - tmux launch: unique names eliminate collision teardown
ok - stop validation: malformed record causes no daemon or state side effects
ok - launcher lock: incomplete metadata fails acquisition and releases lock
fm-afk-launch: failed to clear away-mode flag
fm-afk-launch: away mode stopped; terminal teardown remains recorded for retry
ok - stop state: away-flag removal failure is surfaced
fm-afk-launch: away-mode daemon did not exit after SIGTERM; preserving lifecycle state
ok - stop liveness: captured live daemon preserves lifecycle state after lock release
ok - stop reconciliation: a later retry cleans lifecycle state only after daemon absence is exact
fm-afk-launch: daemon terminal record is malformed; refusing to act on it
fm-afk-launch: daemon terminal record is malformed; refusing to act on it
ok - refresh record: malformed terminal identity fails closed
fm-afk-launch: failed to clear stale away-mode artifacts
ok - clear failure: native entry aborts and restores prior state
fm-afk-launch: reconciling leaked daemon terminal tmux:exact-session
fm-afk-launch: terminal close command failed, but exact absence was confirmed
ok - confirmed absence: cleanup succeeds and removes the stale record
fm-afk-launch: rollback restoration incomplete; backup retained at /var/folders/k_/yr71hk2s5sgdy0s1gr_phgsh0000gn/T//fm-afk-restore-fail.KTnZSd/state/.afk-launch-backup.F9yGcr
ok - rollback restore: incomplete restoration retains its recovery backup
ok - flag failure: lifecycle aborts without active state
ok - herdr e2e: captain tab pane count unchanged after start (no split)
ok - herdr e2e: daemon launched in a separate non-visible workspace
ok - herdr e2e: daemon pane is NOT in the captain's tab
ok - herdr e2e: daemon terminal scoped to the lab session
ok - herdr e2e: captain tab pane count restored after stop
ok - herdr e2e: daemon workspace removed by exact id on stop
ok - herdr e2e: record + .afk cleared on stop
skip: tmux not found (tmux e2e)
evidence - scenario=A human_submissions=1 injection_submissions=1 merged_lines=0
ok - real herdr Scenario A: partial input defers injection; digest arrives clean after idle
evidence - scenario=B injection_count=1 user_submissions=0 swallowed_enter_retried=true
ok - real herdr Scenario B: swallowed Enter (via the herdr shim) produces exactly one clean digest
evidence - scenario=C injection_count=1 buffer_bytes=0 age_sidecar=absent repeated_scan_reinjection=false
ok - real herdr Scenario C: confirmed delivery retires its buffer and repeated catch-all scans cannot re-buffer it
evidence - scenario=D buffer_bytes=61 wedge_marker=present daemon_exited=true
ok - real herdr Scenario D: an unconfirmed delivery remains durable across daemon shutdown
evidence - scenario=E injection_count=1 buffer_bytes=0 age_sidecar=absent wedge_marker=absent daemon_exited=true
ok - real herdr Scenario E: SIGTERM during confirmed submit retires once and exits without re-injection
evidence - scenario=F injection_count=1 buffer_bytes=0 daemon_exited=true
ok - real herdr Scenario F: shutdown after retirement exits with one delivered digest
selected real-herdr afk injection e2e tests passed
Evidence: Base-commit regression reproduction

Source: Base-commit regression reproduction

base_commit=87681a40777bb061ef923ef98b494cd7ef6054b6
lab_session=fm-lab-afk-herdr-repeat-26656-16350
not ok - Scenario E: shutdown re-injected a delivery after Herdr had confirmed it
expected-red - base commit reproduced confirmed-submit shutdown regression (rc=1)
Evidence: Daemon retirement and shutdown evidence

Source: Daemon retirement and shutdown evidence

ok - fm-afk-start.sh fails before daemon startup when the afk flag cannot be written
ok - fm-afk-start.sh ignores stale pidfile-only live pids
ok - fm-afk-start.sh reclaims stale daemon locks whose live pid identity no longer matches
ok - supervise daemon state root is scoped by FM_HOME
ok - routine signal self-handles
ok - captain-relevant status verbs escalate
ok - check + unknown escalate; heartbeat self-handles
ok - transient stale self-handles and records a persistence marker
ok - enriched stale wedges bypass status absorption without disturbing busy workers
ok - stale + terminal status escalates immediately
ok - paused reasons with captain phrases remain pause-classified
ok - handle_wake on a paused stale records a pause marker, drops the wedge marker, and does not escalate
ok - handle_wake records a declared pause from a routine signal for long-cadence rechecks
ok - a terminal signal clears pause and stale tracking across both supervisors
ok - housekeeping migrates a normal-watcher's declared pause into daemon tracking
ok - housekeeping clears an already-resumed watcher pause across both supervisors
ok - housekeeping seeds pause tracking from status without a watcher marker
ok - persistent stale escalates after threshold and clears its marker
ok - resumed (busy) stale clears its marker without escalating
ok - housekeeping re-surfaces a stale declared pause on the long cadence and resets its window
ok - confirmed pause recheck delivery stays retired until the next pause cadence
ok - housekeeping clears a paused marker whose pane became busy again, without escalating
ok - housekeeping clears a paused marker once the crew is no longer declaring the pause
ok - housekeeping moves an existing stale marker to pause before wedge escalation
ok - housekeeping clears tracking when a crew leaves pause
ok - persistent herdr stale resolves the target from metadata and escalates
ok - herdr idle busy-footer stale clears through capture corroboration
ok - resumed herdr stale clears through backend-aware busy state
ok - persistent Orca stale resolves the terminal from metadata
ok - multiple escalations flush as a single batched digest
ok - batch flush measures max-delay from the first append, not the last
ok - catch-all scan escalates a missed terminal once, not twice
ok - handle_wake routes routine->self and captain->escalate
ok - INJECT_SKIP forces self-handle, bypassing captain-relevant classification
ok - is_wake_reason distinguishes watcher wake reasons from singleton-status stdout
ok - terminal-stale escalate removes its marker so housekeeping does not re-escalate
ok - captain signal escalate marks seen so the catch-all scan does not re-fire
ok - _collapse_newlines replaces newlines with literal separator
ok - shutdown request starts no new submit and preserves the pending buffer
ok - daemon shutdown reaps its exact watcher group across the env-to-script exec boundary
ok - afk flag absent: daemon does not inject, buffer preserved
ok - busy-guard defers injection when supervisor pane is busy
ok - marker detection: marker -> stay afk, no marker -> exit afk
ok - /afk invocation is exempt from afk exit (no self-cancel)
ok - should_exit_afk returns false when afk is not active
ok - strip_injection_marker removes the sentinel marker cleanly
ok - pane_input_pending detects partial input on the cursor line
ok - pane_input_pending: a blank unidentified cursor row defers (strict container-proof rule)
ok - pane_input_pending: only proven empty agent prompts pass
ok - fm_tmux_composer_state: a bare shell prompt ($/%/#/>) reads unknown, never empty (dead-shell injection safety)
ok - fm_tmux_composer_state: a bordered composer box and bare agent glyphs (❯/›) still read empty
ok - fm_tmux_composer_state: only matching edge borders form a composer box
ok - pane_input_pending preserves bright placeholder-like drafts in styled captures
ok - classify_signal dedupes against the catch-all scan seen marker
ok - classify_stale dedupes against the signal path seen marker
ok - AFK nonterminal working:+merged keeps wedge aging and re-escalates at bound
ok - genuine done: and merge-check events still escalate
ok - pane_input_pending: an idle bordered composer is NOT pending (afk-invx-i5)
ok - pane_input_pending: text inside a bordered composer is still pending
ok - submit-ACK confirms a submit when the composer returns to a bordered-empty box
ok - submit-ACK reports pending on a persistently swallowed Enter (type-once)
ok - max-defer on an empty stuck pane types once, alarms, and preserves the buffer
ok - max-defer flushes and clears the buffer on an empty bordered pane
ok - max-defer on a pending composer alarms without typing
ok - normal flush clears a stale wedge marker
ok - below MAX_DEFER: no inject, no alarm, buffer preserved
ok - max-defer does not flush or alarm while afk is inactive
ok - library mode: sourcing the daemon defaults FM_WEDGE_ALARM_EXEC to discard (no test can fire a real notification)
ok - wake helpers replace inherited notifier overrides with the safe recorder
ok - the discard seam suppresses every notifier, including command: (fires nothing)
ok - direct notifier helpers honor the discard seam, including command:
ok - osascript channel routes through the notifier seam with the summary (never a real notification)
ok - herdr channel routes through the notifier seam with the summary (never a real notification)
ok - command channel runs the captain command with the summary on $1 and on stdin
ok - command channel failures redact configured commands while logging their exit status
ok - unknown channel directives are redacted while the alarm keeps running
ok - off disables every active alert regardless of directive position (marker and tmux flash are unaffected)
ok - auto resolves to the macOS osascript notifier on Darwin (default-on)
ok - auto on a non-macOS platform selects no built-in OS channel (the marker or a configured command carries it)
ok - config/wedge-alarm selects every configured channel and skips comment and blank lines
ok - a failing channel logs and falls back to the next channel, never crashing the alarm
ok - a hung notifier is bounded, logged, and falls through to the next channel
ok - a backgrounded command notifier remains bounded until its process group is reaped
ok - a hung notifier override is bounded, logged, and proceeds to the next channel
/Users/olsatools_fernand/.no-mistakes/worktrees/986b74e45875/01M0E4F95C7SBXAEYX2Z6SKEVY/bin/fm-supervise-daemon.sh: line 793: 67274 Terminated: 15          sh -c 'sleep 30 & printf "%s" "$!" > "$1"; wait' sh "$child_file"
ok - daemon shutdown stops and reaps the active notifier process group
ok - inject_wedge_alarm writes the marker AND emits the active alert even with no tmux status-line (herdr backend)
ok - in-process wedge throttle prevents alert spam when the marker cannot persist
ok - fm-send returns 3 with a non-error no-resend warning when confirmation stays pending
ok - fm-send exits non-zero when initial text send fails
ok - fm-send exits non-zero unless delivery is proven empty
ok - discover_supervisor_backend: override > TMUX_PANE > HERDR_ENV+HERDR_PANE_ID > tmux fallback
ok - discover_supervisor_target: override > TMUX_PANE > herdr '<session>:<pane-id>' composition > firstmate:0 fallback
ok - pane_is_busy: herdr native busy_state='busy' short-circuits without a capture fallback
ok - primary busy guard isolates rendered signatures by detected harness
ok - pane_is_busy: omitted backend defaults to tmux for Grok's isolated fallback
ok - pane_input_pending: dispatches through fm_backend_composer_state for backend=herdr
ok - inject_msg: herdr busy-guard defers before ever attempting a submit
ok - inject_msg: herdr composer-guard defers before ever attempting a submit
ok - inject_msg: herdr pane-gone check defers before any busy/composer/submit call
ok - inject_msg: dispatches busy-guard/composer-guard/submit through the herdr backend and succeeds on a confirmed empty composer
ok - Herdr timeout applies to supervisor transport without changing worker reads
ok - inject_msg: defers on a dead-shell/unreadable composer (unknown), never typing the escalation into a shell
ok - inject_msg: unrecognized composer states defer by default
Evidence: Real heartbeat dedupe evidence

Source: Real heartbeat dedupe evidence

ok - lifecycle: routine self-handles, terminal survives a watcher restart, buffers once, no dup, injects once
ok - lifecycle: stale pane transient self-handles, persistent escalates once and clears, resumed clears quietly
Evidence: Lab provenance guard evidence

Source: Lab provenance guard evidence

ok - fm-herdr-lab: names fail closed and require the lab prefix
ok - fm-herdr-lab: provisioning, scoped calls, guarded teardown, and fleet tripwire are deterministic
ok - fm-herdr-lab: missing tripwire refuses teardown before any Herdr call
ok - fm-herdr-lab: changed default fleet state is a hard failure
ok - fm-herdr-lab: an owned stopped lab can re-provision safely
ok - fm-herdr-lab: failed deletion retains ownership until absence is confirmed
ok - fm-herdr-lab: running-owned guard requires helper provenance and live server state
ok - fm-herdr-lab: timed-out provisioning cancels the launch before teardown

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 wraps pane_is_busy, and its busy-state and capture reads inherit the deadline. The new test even expects busy 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 literal 0; values such as 00 reach fm_run_timed as 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 and pgid == watcher_pid authority 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 exact sleep 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, but pane_is_busy intentionally leaves native-state and capture reads unbounded. Name only target existence, composer state, and submit, as required, and correct the same overstatement in docs/configuration.md:625, docs/herdr-backend.md:208, and docs/verification/supervision.md:518.
  • ⚠️ bin/backends/herdr.sh:85 - Sourcing the newly imported timeout library executes its top-level set -u, so the documented interactive zsh diagnostic path fm_backend_source herdr now 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.sh
  • tests/fm-herdr-lab.test.sh
  • tests/fm-backend-herdr.test.sh reproduced a setup-only Apple Perl locale failure under ambient C.UTF-8
  • LC_ALL=C LC_CTYPE=C LANG=C tests/fm-backend-herdr.test.sh
  • tests/fm-backend.test.sh
  • tests/fm-wake-daemon-lifecycle-e2e.test.sh
  • tests/fm-afk-return.test.sh
  • HERDR_LAB_SESSION=<helper-named afk-herdr-repeated-escalation-submit lab> tests/fm-afk-launch.test.sh
  • HERDR_LAB_SESSION=&lt;same lab&gt; tests/fm-afk-inject-herdr-e2e.test.sh scenarios A-F
  • Base counterfactual: FM_AFK_HERDR_SCENARIOS=e ... tests/fm-afk-inject-herdr-e2e.test.sh against archived commit 87681a40777bb061ef923ef98b494cd7ef6054b6
  • Reviewed the affected harness/backend applicability table and commit preservation with git diff and git log --reverse
  • Verified clean worktree, removed base-reproduction directory, absent lab tripwires, and no surviving test processes
✅ **Document** - passed

✅ No issues found.

⚠️ **Lint** - 1 warning
  • ⚠️ linter found issues (exit code 127)
✅ **Push** - passed

✅ No issues found.

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