Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 36 additions & 10 deletions bin/backends/herdr.sh
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,12 @@ FM_HOME="${FM_HOME:-${FM_ROOT_OVERRIDE:-$FM_ROOT}}"
# shellcheck source=bin/fm-agent-process-lib.sh
. "$FM_BACKEND_HERDR_ROOT/bin/fm-agent-process-lib.sh"

# The repo-wide bounded runner (bin/fm-timeout-lib.sh), the single owner of how
# this repo bounds a subprocess. Every synchronous Herdr CLI call below runs
# through it; see fm_backend_herdr_bounded.
# shellcheck source=bin/fm-timeout-lib.sh
. "$FM_BACKEND_HERDR_ROOT/bin/fm-timeout-lib.sh"

FM_BACKEND_HERDR_MIN_PROTOCOL=14
# events.subscribe (the native pane.agent_status_changed push stream) and its
# subscription_event schema first shipped at protocol 16 (verified: herdr
Expand Down Expand Up @@ -369,6 +375,23 @@ fm_backend_herdr_workspace_label() {
printf 'firstmate'
}

# FM_BACKEND_HERDR_CLI_TIMEOUT: the hard per-call bound in whole seconds on
# every synchronous Herdr CLI read or write. Without it a wedged server or a
# hung pane read blocks the supervisor that made the call forever and leaks the
# shell it ran in, one per probe. The long-lived `server` launch is exempt: its
# whole purpose is to outlive the call. Invalid or zero values fall back to 10.
FM_BACKEND_HERDR_CLI_TIMEOUT=${FM_BACKEND_HERDR_CLI_TIMEOUT:-10}

# fm_backend_herdr_bounded: run <command...> under the adapter's hard bound via
# the repo-wide bounded runner (bin/fm-timeout-lib.sh). Returns the command's
# own status, or 124 when the bound fires, and kills the whole child process
# group so a hung herdr and anything it spawned cannot outlive the call.
fm_backend_herdr_bounded() { # <command...>
local bound=$FM_BACKEND_HERDR_CLI_TIMEOUT
case "$bound" in ''|*[!0-9]*|0) bound=10 ;; esac
fm_run_timed "$bound" "$@"
}

# fm_backend_herdr_cli: run `herdr <args...>` scoped to <session>, setting
# BOTH the HERDR_SESSION env var AND appending a trailing `--session <name>`
# CLI flag. Verified empirically (docs/herdr-backend.md "Session targeting: the
Expand All @@ -393,21 +416,24 @@ fm_backend_herdr_cli() { # <session> <herdr-subcommand-and-args...>
# stderr is buffered (stdout streams untouched) so a protocol_mismatch
# refusal can be recognized and retried once on a compatible client; see
# "client selection" below. A failed command's stderr is replayed verbatim.
# The long-lived `server` launch is exec'd straight through: buffering its
# stderr would hold this call open for the server's whole lifetime.
# Every call below runs under fm_backend_herdr_bounded (the
# FM_BACKEND_HERDR_CLI_TIMEOUT contract) so a hung server cannot wedge the
# caller. The long-lived `server` launch is the one exemption: it is exec'd
# straight through, because buffering its stderr would hold this call open
# for the server's whole lifetime and a bound would kill the server.
if [ "${1:-}" = server ]; then
HERDR_SESSION="$session" "$client_bin" "$@" --session "$session"
return $?
fi
failed_bin=$client_bin
{ err=$(HERDR_SESSION="$session" "$failed_bin" "$@" --session "$session" 2>&1 1>&3 3>&-) || rc=$?; } 3>&1
{ err=$(fm_backend_herdr_bounded env HERDR_SESSION="$session" "$failed_bin" "$@" --session "$session" 2>&1 1>&3 3>&-) || rc=$?; } 3>&1
if [ "$rc" -ne 0 ]; then
case "$err" in
*protocol_mismatch*)
fm_backend_herdr_client_select "$session" force
selected_bin=$(fm_backend_herdr_bin)
if [ "$selected_bin" != "$failed_bin" ]; then
HERDR_SESSION="$session" "$selected_bin" "$@" --session "$session"
fm_backend_herdr_bounded env HERDR_SESSION="$session" "$selected_bin" "$@" --session "$session"
return $?
fi
;;
Expand Down Expand Up @@ -468,7 +494,7 @@ fm_backend_herdr_client_candidates() {
# client did not report. Never fails.
fm_backend_herdr_client_status() { # <bin> <session>
local bin=$1 session=$2 out
out=$(HERDR_SESSION="$session" "$bin" status --json --session "$session" 2>/dev/null) || out=
out=$(fm_backend_herdr_bounded env HERDR_SESSION="$session" "$bin" status --json --session "$session" 2>/dev/null) || out=
printf '%s' "$out" | jq -r '
[ (if (.server | type) == "object" and .server.running != null then (.server.running | tostring) else "" end),
(if (.server | type) == "object" and (.server | has("compatible"))
Expand Down Expand Up @@ -520,7 +546,7 @@ fm_backend_herdr_tool_check() {
fm_backend_herdr_version_check() {
fm_backend_herdr_tool_check || return 1
local status protocol version
status=$(herdr status --json 2>/dev/null) || { echo "error: 'herdr status --json' failed; is herdr installed correctly?" >&2; return 1; }
status=$(fm_backend_herdr_bounded herdr status --json 2>/dev/null) || { echo "error: 'herdr status --json' failed; is herdr installed correctly?" >&2; return 1; }
protocol=$(printf '%s' "$status" | jq -r '.client.protocol // empty' 2>/dev/null)
version=$(printf '%s' "$status" | jq -r '.client.version // empty' 2>/dev/null)
case "$protocol" in
Expand Down Expand Up @@ -3525,7 +3551,7 @@ fm_backend_herdr_pane_for_tab() { # <session> <workspace_id> <tab_id>
# normally carry meta), best-effort.
fm_backend_herdr_resolve_bare_selector() { # <name>
local name=$1 sessions session tabs tab_id wsid pane_id
sessions=$(herdr session list --json 2>/dev/null | jq -r '.sessions[]? | select(.running == true) | .name' 2>/dev/null)
sessions=$(fm_backend_herdr_bounded herdr session list --json 2>/dev/null | jq -r '.sessions[]? | select(.running == true) | .name' 2>/dev/null)
while IFS= read -r session; do
[ -n "$session" ] || continue
tabs=$(fm_backend_herdr_cli "$session" tab list 2>/dev/null) || continue
Expand Down Expand Up @@ -3589,7 +3615,7 @@ fm_backend_herdr_list_live() { # <session>
# ~/.config/herdr/sessions/<name>/herdr.sock). Empty on any failure.
fm_backend_herdr_socket_path() { # <session>
local session=$1
herdr session list --json 2>/dev/null \
fm_backend_herdr_bounded herdr session list --json 2>/dev/null \
| jq -r --arg name "$session" '.sessions[]? | select(.name == $name) | .socket_path // empty' 2>/dev/null \
| head -1
}
Expand All @@ -3613,10 +3639,10 @@ fm_backend_herdr_events_capable() { # <session>
if [ -z "${FM_BACKEND_HERDR_EVENT_READER:-}" ]; then
command -v python3 >/dev/null 2>&1 || return 1
fi
protocol=$(herdr status --json 2>/dev/null | jq -r '.client.protocol // empty' 2>/dev/null)
protocol=$(fm_backend_herdr_bounded herdr status --json 2>/dev/null | jq -r '.client.protocol // empty' 2>/dev/null)
case "$protocol" in ''|*[!0-9]*) return 1 ;; esac
[ "$protocol" -ge "$FM_BACKEND_HERDR_MIN_EVENTS_PROTOCOL" ] || return 1
schema=$(herdr api schema --json 2>/dev/null) || return 1
schema=$(fm_backend_herdr_bounded herdr api schema --json 2>/dev/null) || return 1
printf '%s' "$schema" | grep -Fq 'events.subscribe' || return 1
printf '%s' "$schema" | grep -Fq 'pane.agent_status_changed' || return 1
return 0
Expand Down
1 change: 1 addition & 0 deletions bin/fm-test-run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ family_for_basename() {
printf '%s\n' live-harness-optin
;;
fm-backend-herdr.test.sh|fm-backend-tmux-smoke.test.sh|fm-backend.test.sh|\
fm-backend-herdr-probe-timeout.test.sh|\
fm-tmux-agent-liveness.test.sh|\
fm-control.test.sh|fm-control-relaunch.test.sh|\
fm-herdr-session-cleanup.test.sh|fm-send-resolve-key.test.sh|fm-send-strict.test.sh|\
Expand Down
1 change: 1 addition & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -1040,6 +1040,7 @@ FM_TASK_ID= # internal task-worker marker fm-spawn.sh exports into s
HERDR_SESSION=default # herdr-only: named session for normal backend ops; not enough for destructive cleanup (docs/herdr-backend.md)
FM_BACKEND_HERDR_SUBMIT_POLLS=6 # herdr-only: agent-state samples spread across each Enter attempt's budget when confirming a submit (docs/herdr-backend.md "Current transport behavior")
FM_BACKEND_HERDR_SUBMIT_MIN_SLEEP=0.6 # herdr-only: minimum per-Enter confirmation budget before polling agent-state after an idle baseline
FM_BACKEND_HERDR_CLI_TIMEOUT=10 # herdr-only: whole-second hard bound on every synchronous herdr CLI read/write, so a hung probe cannot block a supervisor or leak its shell; invalid or zero values fall back to 10, and the long-lived `herdr server` launch is exempt (docs/herdr-backend.md "Current transport behavior")
FM_ZELLIJ_SESSION=firstmate # zellij-only: named session for normal backend ops and test isolation (docs/zellij-backend.md)
CMUX_SOCKET_PASSWORD= # cmux-only: socket password fallback when config/cmux-socket-password is absent (docs/cmux-backend.md)
FM_SESSION_START_STATUS_TAIL=5 # state/*.status lines printed per task in the session-start digest; each line is capped by bin/fm-line-cap-lib.sh
Expand Down
5 changes: 5 additions & 0 deletions docs/herdr-backend.md
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,11 @@ Herdr passes its server startup environment to every later pane, so retaining th
An already-running server is reused without restart or environment changes.
Explicit named-session routing and unrelated launch environment remain intact.

Every synchronous Herdr CLI read or write runs under a hard per-call bound (`FM_BACKEND_HERDR_CLI_TIMEOUT`, default 10 seconds) through the repo-wide bounded runner in `bin/fm-timeout-lib.sh`, so a wedged server or a hung pane read cannot block a supervisor indefinitely or leak the shell that made the call.
The bound kills the whole child process group and reports Herdr timeout as exit 124.
The long-lived `herdr server` launch is the one exemption, because its purpose is to outlive the call and a bound would kill the server.
`tests/fm-backend-herdr-probe-timeout.test.sh` pins the bound, the process reaping, and the server exemption against a TERM-ignoring fake herdr.

Literal text and Enter are separate operations on `fm-send.sh`'s typed plane; ordinary local text steers instead use the durable steering inbox and send only its best-effort constant doorbell through this adapter.
Spawn-time fixed commands may use Herdr's atomic run primitive.
Enter, Escape, and Ctrl-C are supported.
Expand Down
157 changes: 157 additions & 0 deletions tests/fm-backend-herdr-probe-timeout.test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
#!/usr/bin/env bash
# tests/fm-backend-herdr-probe-timeout.test.sh - proves every synchronous herdr
# CLI read in bin/backends/herdr.sh runs under a real process-level bound, and
# that a hung probe's process is gone once that bound fires. The property is the
# adapter's own timeout discipline, so it is pinned with a fake herdr that
# ignores TERM and never answers a read; no real herdr installation is needed.
set -u

# shellcheck source=tests/lib.sh
. "$(dirname "${BASH_SOURCE[0]}")/lib.sh"

command -v jq >/dev/null 2>&1 || { echo "skip: jq not found (required by the herdr adapter)"; exit 0; }

TMP_ROOT=$(fm_test_tmproot fm-backend-herdr-probe-timeout)
HANG_PIDS="$TMP_ROOT/hang-pids"
: > "$HANG_PIDS"

# A herdr stub that answers the server-state liveness read so target_ready
# passes, records its own pid, and then never returns from a real read. It
# ignores TERM (with a self-deadline so a broken adapter cannot hang the suite
# forever), so only the runner's KILL escalation can reap it.
make_hanging_herdr_fakebin() { # <dir> -> echoes fakebin dir
local dir=$1 fb="$1/fakebin"
mkdir -p "$fb"
cat > "$fb/herdr" <<'SH'
#!/usr/bin/env bash
set -u
printf '%s\n' "$$" >> "$FM_HANG_PIDS"
if [ "${1:-}" = status ] && [ "${2:-}" = --json ]; then
printf '{"client":{"protocol":22},"server":{"running":true}}\n'
exit 0
fi
trap '' TERM
deadline=$((SECONDS + 20))
while [ "$SECONDS" -lt "$deadline" ]; do
sleep 1
done
exit 0
SH
chmod +x "$fb/herdr"
printf '%s\n' "$fb"
}

# A fake whose server launch is a short, normal-lived command: it exits 0 after
# a delay LONGER than the bound under test, so a wrongly bounded server call
# would be killed (124) instead of completing.
make_server_launch_fakebin() { # <dir> <sleep-seconds> -> echoes fakebin dir
local dir=$1 nap=$2 fb="$1/fakebin-server"
mkdir -p "$fb"
cat > "$fb/herdr" <<SH
#!/usr/bin/env bash
sleep $nap
exit 0
SH
chmod +x "$fb/herdr"
printf '%s\n' "$fb"
}

# reap_hung_fakes: best-effort cleanup so a failed assertion cannot leave a
# stray fixture behind; registered on EXIT alongside lib.sh's own cleanup.
reap_hung_fakes() {
local pid
[ -f "$HANG_PIDS" ] || return 0
while IFS= read -r pid; do
case "$pid" in ''|*[!0-9]*) continue ;; esac
kill -KILL "$pid" 2>/dev/null || true
done < "$HANG_PIDS"
}
trap 'reap_hung_fakes; fm_test_cleanup' EXIT

# every_fake_is_gone: true only when each recorded pid is gone (or a zombie the
# init reaper is about to collect), after a short bounded settle. A still-live
# process after the grace window is a leaked shell and fails the case.
every_fake_is_gone() {
local attempt=0 pid state
while [ "$attempt" -lt 30 ]; do
local all_gone=1
while IFS= read -r pid; do
case "$pid" in ''|*[!0-9]*) continue ;; esac
if kill -0 "$pid" 2>/dev/null; then
state=$(ps -o stat= -p "$pid" 2>/dev/null | tr -d ' ')
case "$state" in
''|Z*) ;;
*) all_gone=0 ;;
esac
fi
done < "$HANG_PIDS"
[ "$all_gone" -eq 1 ] && return 0
attempt=$((attempt + 1))
sleep 0.2
done
return 1
}

run_adapter_snippet() { # <fakebin> <bound-seconds> <snippet>
local fb=$1 bound=$2 snippet=$3
PATH="$fb:$PATH" FM_HANG_PIDS="$HANG_PIDS" FM_BACKEND_HERDR_CLI_TIMEOUT="$bound" \
FM_HOME="$TMP_ROOT/ambient-home" \
bash -c ". \"\$0/bin/backends/herdr.sh\"; $snippet" "$ROOT"
}

test_capture_probe_is_bounded_and_reaped() {
local dir fb start elapsed out rc
dir="$TMP_ROOT/capture"; mkdir -p "$dir"
fb=$(make_hanging_herdr_fakebin "$dir")
start=$SECONDS
out=$(run_adapter_snippet "$fb" 1 'fm_backend_herdr_capture fmtest:w1:p2 40' 2>/dev/null)
rc=$?
elapsed=$((SECONDS - start))
[ "$rc" -ne 0 ] || fail "a hung capture read must fail rather than return success (out='$out')"
[ "$elapsed" -lt 15 ] || fail "a hung capture read ignored the bound and ran ${elapsed}s"
every_fake_is_gone || fail "a hung capture read leaked its herdr process past the bound: $(tr '\n' ' ' < "$HANG_PIDS")"
pass "capture read: a TERM-ignoring hung herdr is bounded and its process is reaped"
}

test_composer_state_probe_is_bounded_and_reaped() {
local dir fb start elapsed out
dir="$TMP_ROOT/composer"; mkdir -p "$dir"
fb=$(make_hanging_herdr_fakebin "$dir")
start=$SECONDS
out=$(run_adapter_snippet "$fb" 1 'fm_backend_herdr_composer_state fmtest:w1:p2' 2>/dev/null)
elapsed=$((SECONDS - start))
[ "$out" = unknown ] || fail "a hung composer probe must read unknown, got '$out'"
[ "$elapsed" -lt 20 ] || fail "a hung composer probe ignored the bound and ran ${elapsed}s"
every_fake_is_gone || fail "a hung composer probe leaked its herdr process past the bound: $(tr '\n' ' ' < "$HANG_PIDS")"
pass "composer_state probe: a TERM-ignoring hung herdr is bounded and its process is reaped"
}

test_generic_cli_read_is_bounded_and_reaped() {
local dir fb start elapsed rc
dir="$TMP_ROOT/cli"; mkdir -p "$dir"
fb=$(make_hanging_herdr_fakebin "$dir")
start=$SECONDS
run_adapter_snippet "$fb" 1 'fm_backend_herdr_cli fmtest pane read w1:p2 --source recent --lines 200' >/dev/null 2>&1
rc=$?
elapsed=$((SECONDS - start))
[ "$rc" -eq 124 ] || fail "a hung fm_backend_herdr_cli read must return 124 (the bound), got $rc"
[ "$elapsed" -lt 10 ] || fail "a hung fm_backend_herdr_cli read ignored the bound and ran ${elapsed}s"
every_fake_is_gone || fail "a hung cli read leaked its herdr process past the bound: $(tr '\n' ' ' < "$HANG_PIDS")"
pass "fm_backend_herdr_cli: the shared read owner bounds and reaps a hung herdr"
}

test_server_launch_is_exempt_from_the_bound() {
local dir fb rc
dir="$TMP_ROOT/server"; mkdir -p "$dir"
fb=$(make_server_launch_fakebin "$dir" 2)
run_adapter_snippet "$fb" 1 'fm_backend_herdr_cli fmtest server' >/dev/null 2>&1
rc=$?
[ "$rc" -eq 0 ] \
|| fail "the long-lived server launch must not be bounded (got rc=$rc; a 1s bound would kill it)"
pass "fm_backend_herdr_cli: the long-lived server launch stays exempt from the bound"
}

test_capture_probe_is_bounded_and_reaped
test_composer_state_probe_is_bounded_and_reaped
test_generic_cli_read_is_bounded_and_reaped
test_server_launch_is_exempt_from_the_bound