Skip to content

feat(daemon): add liveness beacon and down-detection for the afk daemon#413

Open
lukeforshort wants to merge 3 commits into
kunchenguid:mainfrom
lukeforshort:fm/afk-daemon-beacon-b4
Open

feat(daemon): add liveness beacon and down-detection for the afk daemon#413
lukeforshort wants to merge 3 commits into
kunchenguid:mainfrom
lukeforshort:fm/afk-daemon-beacon-b4

Conversation

@lukeforshort

Copy link
Copy Markdown

Intent

Management-audit finding B4/L2 ("nothing owns the daemon"): while state/.afk is set, the away-mode supervise daemon owns supervision, but nothing verified it was alive, so a silent daemon death was classified benign without any check. Goal: give the daemon its own liveness beacon and a down-detection check, mirroring the existing watcher idiom (the watcher touches state/.last-watcher-beat every poll and fm-guard.sh alarms when it goes stale) rather than inventing a new mechanism.

Decisions and tradeoffs made: (1) New shared bin/fm-daemon-lib.sh owns the daemon lock/beacon liveness contract in one place (one-owner rule) - it absorbed the lock-owner/pid-match helpers that were previously duplicated inline in fm-afk-start.sh, so fm-afk-start.sh now sources them instead of redefining them. (2) The daemon touches state/.last-daemon-beat at startup (before the first backoff sleep, so a just-started daemon reads alive immediately) and at the top of every main-loop cycle (so it stays fresh through the pane-gone and crash-backoff branches), and clears it on clean shutdown so a stopped daemon reads gone at once instead of aging out of grace. (3) fm_daemon_down is deliberately NOT gated on in-flight task count: away mode keeps the daemon armed for X mode and late crew reports even with an empty queue, so a dead daemon under afk is a fault regardless. (4) Grace defaults to FM_GUARD_GRACE then 300, matching fm-guard.sh. (5) Two alarm points added, both using the shared predicate: fm-guard.sh prints a bordered banner on the next fleet action, and the session-start AFK digest reports daemon-down - covering both the guard path and the recovery path the brief named, and distinguishing "process gone" from "beacon stale/wedged". Read-only sessions are told to report the lapse, not repair it.

Doc changes are minimal and respect one-owner: two lines added to the afk skill's reliability-properties section and one line to AGENTS.md's state-file index for the new beacon; the detection contract itself lives in the library and skill, not duplicated into AGENTS.md. Regression test: six new tests in tests/fm-daemon.test.sh, including one that proves fm-guard.sh (which was silent on base) now alarms when .afk is set and the daemon is dead.

What Changed

  • Added bin/fm-daemon-lib.sh as the single owner of the away-mode daemon's lock/beacon liveness contract, absorbing the lock-owner and pid-match helpers previously duplicated inline in bin/fm-afk-start.sh (which now sources them). The daemon touches state/.last-daemon-beat at startup and at the top of every main-loop cycle, and clears it on clean shutdown so a stopped daemon reads gone immediately rather than aging out of grace.
  • Wired the shared fm_daemon_down predicate into two alarm points: bin/fm-guard.sh prints a bordered banner on the next fleet action, and the bin/fm-session-start.sh AFK digest reports daemon-down, distinguishing "process gone" from a stale/wedged beacon; the session-start repair line is gated so read-only sessions are told to report the lapse rather than repair it. Down-detection is intentionally not gated on in-flight task count, since away mode keeps the daemon armed for X mode and late crew reports even with an empty queue.
  • Added tests/fm-daemon.test.sh covering the beacon and down-detection, including a regression proving fm-guard.sh now alarms when state/.afk is set and the daemon is dead. Updated the afk skill, AGENTS.md state-file index, and supporting docs to reference the new beacon.

Risk Assessment

✅ Low: Well-bounded change that mirrors the existing, proven watcher-beacon idiom, faithfully extracts previously-inline helpers, is covered by six new tests, and has both prior-round findings already resolved.

Testing

Completed 1 recorded test check.

  • Outcome: ⚠️ 1 error across 4 runs (1h23m22s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 1 info
  • ⚠️ bin/fm-session-start.sh:292 - The session-start AFK subsection prints the repair instruction 'Load /afk and restart the daemon (bin/fm-afk-start.sh)' unconditionally when the daemon is down, without checking READ_ONLY. This contradicts the stated design ('read-only sessions are told to report the lapse, not repair it') that the author correctly implemented in fm-guard.sh (which gates the equivalent line on READ_ONLY), and it directly conflicts with the very next 'NEXT STEP' block that tells a read-only session 'do not arm, drain, spawn, steer, merge, or repair fleet state from here.' A read-only session-start with .afk set and a dead daemon emits contradictory guidance. Gate the repair line on READ_ONLY, mirroring fm-guard.sh's read-only banner text.
  • ℹ️ bin/fm-daemon-lib.sh:112 - FM_DAEMON_PID is computed in fm_daemon_status and carries a shellcheck-disable comment claiming it is 'read by callers (fm-guard.sh) after this returns', but no caller (fm-guard.sh, fm-session-start.sh, or the tests) actually reads it. The computation and the justifying comment are both dead/misleading. Either drop FM_DAEMON_PID and its lock read, or correct the comment.

🔧 Fix: gate readonly daemon repair line, drop unused FM_DAEMON_PID
1 info still open:

  • ℹ️ bin/fm-guard.sh:71 - Startup-window transient: fm-afk-start.sh writes state/.afk before exec-ing the daemon, which then must run setup, acquire the lock, and touch the beacon. If any fleet action runs fm-guard.sh (or session start reads the AFK digest) in that sub-second-to-seconds window, fm_daemon_down returns true with FM_DAEMON_ALIVE=false and the banner false-alarms 'its process is gone'. It is self-correcting once the daemon holds the lock, the guard is advisory-only (operation still runs), and it requires a concurrent fleet action right after /afk, so it is very low impact - noting it only for awareness.
⚠️ **Test** - 1 error
  • 🚨 tests failed with exit code 1
  • 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: no code fix; failures are pre-existing environment/flaky, not the diff
1 error still open:

  • 🚨 tests failed with exit code 1
  • 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: no code fix needed; the three failures are pre-existing host PATH and flaky-timeout issues, not the diff
1 error still open:

  • 🚨 tests failed with exit code 1
  • 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: no fix; failures are pre-existing host env/flaky, not the diff
1 error still open:

  • 🚨 tests failed with exit code 1
  • 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"
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

While state/.afk is set the supervise daemon owns supervision, but nothing
verified it was alive, so a silent daemon death was read as benign. Add a
liveness beacon (state/.last-daemon-beat) the daemon touches at startup and
every cycle and clears on clean shutdown, mirroring the watcher's
.last-watcher-beat idiom. A shared fm-daemon-lib.sh owns the daemon
lock/beacon liveness contract; fm-guard.sh and the session-start digest now
alarm loudly when .afk is present but the daemon process is gone or its
beacon has gone stale.
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