Skip to content

fix(bin): retire cleared lane records and stop their wakes #4997

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
keenvc wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(bin): retire cleared lane records and stop their wakes #4997

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 35 additions & 0 deletions bin/backends/herdr.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2056,6 +2056,41 @@ fm_backend_herdr_workspace_presence_state() { # <session> <workspace_id>
esac
}

# fm_backend_herdr_projection_workspace_remove_focus_preserving: confirm one
# disposable projected workspace is gone, closing its remaining panes through
# the existing focus-preserving pane close when it is not. The recorded task
# pane's own close removes the emptied workspace, but the recorded pane can
# already be gone (a restored husk, a server restart) while the workspace's
# saved layout survives; that close then fails on a nonexistent pane and the
# workspace is left for the next server restart to resurrect as a live agent in
# the wrong directory. This closes whatever panes the workspace still holds and
# then requires structured absence. A pane holding a live or unknown agent
# refuses rather than closing it, and `workspace close` is never called (Herdr
# 0.7.5 steals focus on an emptying close). Returns 0 only when the workspace is
# confirmed gone.
fm_backend_herdr_projection_workspace_remove_focus_preserving() { # <session> <workspace-id>
local session=$1 workspace=$2 presence panes pane state
[ -n "$session" ] && [ -n "$workspace" ] || return 1
presence=$(fm_backend_herdr_workspace_presence_state "$session" "$workspace")
[ "$presence" = dead ] && return 0
[ "$presence" = present ] || return 1
panes=$(fm_backend_herdr_cli "$session" pane list --workspace "$workspace" 2>/dev/null) || return 1
panes=$(printf '%s' "$panes" | jq -r '.result.panes[]?.pane_id // empty' 2>/dev/null) || return 1
while IFS= read -r pane; do
[ -n "$pane" ] || continue
state=$(fm_backend_herdr_pane_agent_state "$session" "$pane")
case "$state" in
dead|no-agent) ;;
*) return 1 ;;
esac
fm_backend_herdr_projection_close_pane_focus_preserving "$session" "$pane" "$state" || return 1
done <<FMEOF
$panes
FMEOF
presence=$(fm_backend_herdr_workspace_presence_state "$session" "$workspace")
[ "$presence" = dead ]
}

# fm_backend_herdr_explicit_close_pane_confirmed: issue one explicit close and
# succeed only when a structured follow-up proves the exact pane is gone.
fm_backend_herdr_explicit_close_pane_confirmed() { # <session> <pane_id>
Expand Down
48 changes: 43 additions & 5 deletions bin/fm-backend.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,14 +362,38 @@ fm_backend_target_of_meta() { # <meta-file>
[ -n "$window" ] && printf '%s' "$window"
}

# fm_backend_meta_endpoint_cleared_value: the single explicit
# `endpoint_cleared=<reason>` stamp a record may carry once its endpoint is
# already gone (a workspace or pane closed by an earlier cleanup, or an agent
# that died to a provider cap). Absent, empty, duplicated, or malformed returns
# 1 so an ambiguous stamp is never mistaken for a confirmed cleared endpoint.
fm_backend_meta_endpoint_cleared_value() { # <meta-file>
local meta=$1 count value
count=$(grep -c '^endpoint_cleared=' "$meta" 2>/dev/null || true)
[ "$count" -eq 1 ] || return 1
value=$(grep '^endpoint_cleared=' "$meta" | cut -d= -f2-)
[ -n "$value" ] || return 1
case "$value" in *$'\n'*|*$'\r'*|*$'\t'*) return 1 ;; esac
printf '%s' "$value"
}

# fm_backend_validate_task_endpoint: validate a task cleanup record entirely
# from its durable metadata before any runtime command or cleanup mutation.
# The validation binds the exact task id, selected backend, target, project,
# and worktree. New non-tmux records carry endpoint_task_id because their
# opaque runtime ids do not encode the task label. Legacy tmux records remain
# valid only when their window name itself is exactly fm-<task-id>.
# With --allow-cleared, a record whose window is absent but which carries one
# explicit endpoint_cleared stamp is accepted as an agent-less cleared endpoint
# instead of refused: there is no live endpoint left to validate structurally,
# and the stamp is the stronger agent-less evidence (a window may be dead while
# an agent is gone; a cleared stamp records a close already performed). The
# cleared contract sets FM_BACKEND_VALIDATED_ENDPOINT_CLEARED to the reason and
# leaves FM_BACKEND_VALIDATED_TARGET empty; every caller that needs to operate
# on a live endpoint must therefore stay strict and must not pass the flag.
# On success, sets FM_BACKEND_VALIDATED_BACKEND and
# FM_BACKEND_VALIDATED_TARGET. On failure, prints one refusal and returns 1.
# FM_BACKEND_VALIDATED_TARGET (and FM_BACKEND_VALIDATED_ENDPOINT_CLEARED when
# cleared). On failure, prints one refusal and returns 1.
fm_backend_meta_exact_value() { # <meta-file> <key>
local meta=$1 key=$2 count value
count=$(grep -c "^$key=" "$meta" 2>/dev/null || true)
Expand Down Expand Up @@ -404,11 +428,12 @@ fm_backend_orca_worktree_id_valid() { # <value>
esac
}

fm_backend_validate_task_endpoint() { # <meta-file> <task-id>
local meta=$1 id=$2 backend_count backend window worktree project binding_count binding
fm_backend_validate_task_endpoint() { # <meta-file> <task-id> [--allow-cleared]
local meta=$1 id=$2 allow_cleared=${3:-} backend_count backend window cleared worktree project binding_count binding
local session pane recorded_session workspace tab terminal worktree_id surface
FM_BACKEND_VALIDATED_BACKEND=
FM_BACKEND_VALIDATED_TARGET=
FM_BACKEND_VALIDATED_ENDPOINT_CLEARED=
[ -f "$meta" ] && [ ! -L "$meta" ] || {
echo "REFUSED: task $id has no regular endpoint metadata at $meta; preserving task state." >&2
return 1
Expand All @@ -417,10 +442,15 @@ fm_backend_validate_task_endpoint() { # <meta-file> <task-id>
echo "REFUSED: task endpoint identity has an invalid task id; preserving task state." >&2
return 1
esac
window=$(fm_backend_meta_exact_value "$meta" window) || {
window=$(fm_backend_meta_exact_value "$meta" window) || window=
cleared=
if [ -z "$window" ] && [ "$allow_cleared" = --allow-cleared ]; then
cleared=$(fm_backend_meta_endpoint_cleared_value "$meta") || cleared=
fi
if [ -z "$window" ] && [ -z "$cleared" ]; then
echo "REFUSED: task $id has a missing, empty, or ambiguous window endpoint; preserving task state." >&2
return 1
}
fi
worktree=$(fm_backend_meta_exact_value "$meta" worktree) || {
echo "REFUSED: task $id has a missing, empty, or ambiguous worktree identity; preserving task state." >&2
return 1
Expand Down Expand Up @@ -462,6 +492,14 @@ fm_backend_validate_task_endpoint() { # <meta-file> <task-id>
return 1
fi

if [ -n "$cleared" ]; then
# shellcheck disable=SC2034 # Output globals are consumed by sourcing callers.
FM_BACKEND_VALIDATED_ENDPOINT_CLEARED=$cleared
FM_BACKEND_VALIDATED_BACKEND=$backend
FM_BACKEND_VALIDATED_TARGET=
return 0
fi

case "$backend" in
tmux)
session=${window%%:*}
Expand Down
97 changes: 69 additions & 28 deletions bin/fm-teardown.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,13 @@
# an abandoned attempt left behind never counts as a published incarnation:
# the record still reads as a legacy record, so the endpoint gate runs again
# and the retry still needs --legacy-record.
# An explicit endpoint_cleared=<reason> stamp on a record with no window= line
# is accepted as stronger agent-less evidence than a dead window: it records a
# close already performed, so teardown proceeds with no flag and no --force,
# and no endpoint command is issued. bin/fm-backend.sh's
# fm_backend_validate_task_endpoint owns the stamp's shape and is the only
# reader that accepts it (--allow-cleared). The stamp never relaxes the
# unlanded-work refusal, which only --force can authorize.
#
# Transient / stale worktree git lock recovery (teardown-lock-race): a crew process
# killed mid-git-operation can leave a .git/worktrees/<wt>/index.lock (or, for a
Expand Down Expand Up @@ -433,6 +440,15 @@ fm_backlog_record_present "$META" "task record" "$STATE" || {
}
TEARDOWN_META_KIND=$(fm_meta_get "$META" kind)
[ -n "$TEARDOWN_META_KIND" ] || TEARDOWN_META_KIND=ship
# An explicit endpoint_cleared stamp on a record with no window means the
# endpoint was already closed, so there is no live incarnation left to identify
# and neither the spawn_gen gate below nor the endpoint probe needs to run. Read
# it here, ahead of the incarnation gate; the endpoint validator re-affirms the
# same value later.
TEARDOWN_ENDPOINT_CLEARED=
if [ -z "$(fm_backend_meta_exact_value "$META" window 2>/dev/null || true)" ]; then
TEARDOWN_ENDPOINT_CLEARED=$(fm_backend_meta_endpoint_cleared_value "$META" 2>/dev/null || true)
fi
TEARDOWN_CLEANUP_RECOVERY=$(fm_meta_get "$META" cleanup_recovery)
TEARDOWN_META_SPAWN_GEN=
TEARDOWN_LEGACY_PENDING=0
Expand All @@ -457,10 +473,11 @@ fi
if [ "$TEARDOWN_BACKLOG_APPLIES" = 1 ]; then
if ! fm_backlog_meta_spawn_gen "$META" "$STATE"; then
TEARDOWN_LEGACY_GEN_COUNT=$(LC_ALL=C awk -F= '$1 == "spawn_gen" { count++ } END { print count + 0 }' "$META" 2>/dev/null || printf '0\n')
if [ "$TEARDOWN_LEGACY_GEN_COUNT" = 0 ] && [ "$LEGACY_RECORD_GIVEN" = 1 ]; then
# A record that predates the incarnation field: acceptance is gated later,
# once the recorded endpoint is known, so its state can be confirmed dead
# or agent-less before any cleanup decision is made.
if [ "$TEARDOWN_LEGACY_GEN_COUNT" = 0 ] \
&& { [ "$LEGACY_RECORD_GIVEN" = 1 ] || [ -n "$TEARDOWN_ENDPOINT_CLEARED" ]; }; then
# A record that predates the incarnation field, or one whose endpoint was
# already cleared: acceptance is gated later, once the recorded endpoint is
# known cleared or confirmed dead or agent-less, before any cleanup decision.
TEARDOWN_LEGACY_PENDING=1
elif [ "$TEARDOWN_LEGACY_GEN_COUNT" = 0 ]; then
echo "error: task $ID's record has no spawn_gen that identifies one exact incarnation ($FM_BACKLOG_TRANSITION_ERROR); refusing automatic teardown - relaunch the task to publish an unambiguous incarnation, then retry teardown, or pass --legacy-record once its recorded endpoint is confirmed dead or agent-less" >&2
Expand Down Expand Up @@ -968,13 +985,16 @@ fi
# This is the first cleanup authorization check. It is metadata-only and must
# complete before fm-guard, a backend command, file removal, branch deletion,
# worktree return, registry change, or process termination can run.
fm_backend_validate_task_endpoint "$META" "$ID" || exit 1
fm_backend_validate_task_endpoint "$META" "$ID" --allow-cleared || exit 1
BACKEND=$FM_BACKEND_VALIDATED_BACKEND
T=$FM_BACKEND_VALIDATED_TARGET
TEARDOWN_ENDPOINT_CLEARED=$FM_BACKEND_VALIDATED_ENDPOINT_CLEARED
TEARDOWN_WINDOW_DISPLAY=$T
[ -z "$TEARDOWN_ENDPOINT_CLEARED" ] || TEARDOWN_WINDOW_DISPLAY="cleared:$TEARDOWN_ENDPOINT_CLEARED"
WT=$(fm_meta_get "$META" worktree)
PROJ=$(fm_meta_get "$META" project)
T_ORCA=
[ "$BACKEND" != orca ] || T_ORCA=$T
if [ "$BACKEND" = orca ] && [ -z "$TEARDOWN_ENDPOINT_CLEARED" ]; then T_ORCA=$T; fi
if [ "${FM_TEARDOWN_GUARD_DONE:-0}" != 1 ]; then
"$FM_ROOT/bin/fm-guard.sh" || true
fi
Expand Down Expand Up @@ -1020,15 +1040,21 @@ MODE=$(grep '^mode=' "$META" | cut -d= -f2- || true)
# passed, immediately before the close marker binds to it, so any refusal
# leaves the record byte-identical.
if [ "$TEARDOWN_LEGACY_PENDING" = 1 ]; then
TEARDOWN_LEGACY_ENDPOINT=$(fm_backend_agent_state "$BACKEND" "$T")
case "$TEARDOWN_LEGACY_ENDPOINT" in
dead|missing) ;;
*)
echo "REFUSED: task $ID's record predates spawn_gen and its recorded endpoint reads '$TEARDOWN_LEGACY_ENDPOINT', not confidently dead or agent-less; --legacy-record teardown is refused while an agent may still be bound to it. Nothing was changed." >&2
echo "Reconcile the endpoint first (bin/fm-crew-state.sh $ID), or relaunch the task to publish an unambiguous incarnation, then retry teardown." >&2
exit 1
;;
esac
if [ -n "$TEARDOWN_ENDPOINT_CLEARED" ]; then
# The record's own explicit cleared stamp is stronger agent-less evidence
# than a dead window, so no endpoint probe is needed or possible.
TEARDOWN_LEGACY_ENDPOINT=cleared
else
TEARDOWN_LEGACY_ENDPOINT=$(fm_backend_agent_state "$BACKEND" "$T")
case "$TEARDOWN_LEGACY_ENDPOINT" in
dead|missing) ;;
*)
echo "REFUSED: task $ID's record predates spawn_gen and its recorded endpoint reads '$TEARDOWN_LEGACY_ENDPOINT', not confidently dead or agent-less; --legacy-record teardown is refused while an agent may still be bound to it. Nothing was changed." >&2
echo "Reconcile the endpoint first (bin/fm-crew-state.sh $ID), or relaunch the task to publish an unambiguous incarnation, then retry teardown." >&2
exit 1
;;
esac
fi
if [ -n "$TEARDOWN_LEGACY_RETAINED_STAMP" ]; then
TEARDOWN_META_SPAWN_GEN=$TEARDOWN_LEGACY_RETAINED_STAMP
else
Expand Down Expand Up @@ -2815,7 +2841,7 @@ preflight_descendant_treehouse_slots() {
if ! fm_treehouse_pool_slot "$project" "$worktree"; then
continue
fi
fm_backend_validate_task_endpoint "$meta" "$task_id" || return 1
fm_backend_validate_task_endpoint "$meta" "$task_id" --allow-cleared || return 1
require_exclusive_worktree_slot_record "$meta" "$task_id" "$state" "$worktree" || return 1
owner_rc=0
require_owned_worktree_slot_record "$task_id" "$worktree" || owner_rc=$?
Expand All @@ -2833,7 +2859,7 @@ validate_firstmate_home_children_removal() {
for child_meta in "$sub_state"/*.meta; do
[ -e "$child_meta" ] || continue
child_id=$(basename "$child_meta" .meta)
fm_backend_validate_task_endpoint "$child_meta" "$child_id" || return 1
fm_backend_validate_task_endpoint "$child_meta" "$child_id" --allow-cleared || return 1
validate_pr_poll_cleanup "$sub_state" "$child_id" || return 1
child_wt=$(meta_value "$child_meta" worktree)
child_kind=$(meta_value "$child_meta" kind)
Expand Down Expand Up @@ -2976,10 +3002,10 @@ preflight_firstmate_home_herdr_children() { # <home>
for child_meta in "$sub_state"/*.meta; do
[ -e "$child_meta" ] || continue
child_id=$(basename "$child_meta" .meta)
fm_backend_validate_task_endpoint "$child_meta" "$child_id" || return 1
fm_backend_validate_task_endpoint "$child_meta" "$child_id" --allow-cleared || return 1
child_backend=$FM_BACKEND_VALIDATED_BACKEND
child_target=$FM_BACKEND_VALIDATED_TARGET
if [ "$child_backend" = herdr ]; then
if [ "$child_backend" = herdr ] && [ -z "$FM_BACKEND_VALIDATED_ENDPOINT_CLEARED" ]; then
teardown_herdr_preflight_target "$child_target" "$child_id" || return 1
fi
child_kind=$(meta_value "$child_meta" kind)
Expand Down Expand Up @@ -3183,7 +3209,7 @@ if [ "$KIND" = secondmate ]; then
preflight_descendant_task_locks "$HOME_PATH" || exit 1
validate_firstmate_home_children_removal "$HOME_PATH" || exit 1
preflight_descendant_treehouse_slots || exit 1
if [ "$BACKEND" = herdr ]; then
if [ "$BACKEND" = herdr ] && [ -z "$TEARDOWN_ENDPOINT_CLEARED" ]; then
teardown_herdr_preflight_target "$T" "$ID" || exit 1
fi
preflight_firstmate_home_herdr_children "$HOME_PATH" || exit 1
Expand Down Expand Up @@ -3297,7 +3323,7 @@ fi
# refuses before any destructive step.
TEARDOWN_HERDR_SESSION=
TEARDOWN_HERDR_PANE=
if [ "$BACKEND" = herdr ]; then
if [ "$BACKEND" = herdr ] && [ -z "$TEARDOWN_ENDPOINT_CLEARED" ]; then
teardown_herdr_preflight_target "$T" "$ID" || exit 1
fm_backend_herdr_parse_target "$T" || exit 1
TEARDOWN_HERDR_SESSION=$FM_BACKEND_HERDR_SESSION
Expand Down Expand Up @@ -3491,6 +3517,12 @@ if [ "$HERDR_PRESENTATION_RETIRE_CANDIDATE" = 1 ]; then
else
echo "warning: herdr presentation focus lock unavailable; refusing a concurrent focus-unsafe pane close" >&2
fi
elif [ -n "$TEARDOWN_ENDPOINT_CLEARED" ]; then
# The record already carries an explicit cleared stamp, so there is no live
# endpoint left to close; the cleared contract is the whole endpoint proof.
# Any leftover presentation journal is stale for the same reason.
rm -f "$HERDR_PRESENTATION_JOURNAL"
:
elif [ "$BACKEND" = herdr ]; then
if teardown_herdr_session_lock_held "$TEARDOWN_HERDR_SESSION"; then
fm_backend_herdr_kill_serialized "$TEARDOWN_HERDR_SESSION" "$TEARDOWN_HERDR_PANE" 2>/dev/null || true
Expand All @@ -3503,11 +3535,18 @@ elif [ "$BACKEND" != orca ]; then
fi
if [ "$HERDR_PRESENTATION_RETIRE_CANDIDATE" = 1 ]; then
if [ "$(fm_backend_herdr_pane_agent_state "$HERDR_PRESENTATION_SESSION" "$HERDR_PRESENTATION_PANE")" = dead ]; then
rm -f "$HERDR_PRESENTATION_JOURNAL"
fm_backend_source herdr || true
if fm_backend_herdr_projection_workspace_remove_focus_preserving \
"$HERDR_PRESENTATION_SESSION" "$HERDR_PRESENTATION_WORKSPACE"; then
rm -f "$HERDR_PRESENTATION_JOURNAL"
else
echo "error: herdr projection workspace $HERDR_PRESENTATION_WORKSPACE for $ID is not confirmed gone although its task pane is; retaining every durable task record and the presentation journal so a rerun can remove the workspace before it is restored" >&2
exit 1
fi
else
echo "warning: exact herdr task-pane close could not be confirmed for $ID; retaining the presentation journal and attempting no workspace cleanup" >&2
fi
elif [ "$BACKEND" = herdr ] \
elif [ "$BACKEND" = herdr ] && [ -z "$TEARDOWN_ENDPOINT_CLEARED" ] \
&& { [ -e "$HERDR_PRESENTATION_JOURNAL" ] || [ -L "$HERDR_PRESENTATION_JOURNAL" ]; }; then
echo "warning: herdr presentation journal for $ID remains quarantined; no workspace cleanup was attempted" >&2
fi
Expand All @@ -3517,7 +3556,7 @@ fi
# the locked close. Only a structured not-found proves the pane gone; unknown
# presence, missing or malformed endpoint identity, and missing confirmation
# machinery all refuse.
if [ "$BACKEND" = herdr ]; then
if [ "$BACKEND" = herdr ] && [ -z "$TEARDOWN_ENDPOINT_CLEARED" ]; then
fm_backend_source herdr || true
if ! declare -F fm_backend_herdr_endpoint_confirmed_gone >/dev/null 2>&1; then
echo "error: herdr endpoint confirmation is unavailable for $ID; retaining every durable task record" >&2
Expand Down Expand Up @@ -3559,7 +3598,9 @@ if [ "$KIND" = secondmate ]; then
fi
remove_grok_turnend_auth "$STATE" "$ID" || exit 1
remove_kimi_turnend_auth "$STATE" "$ID" || exit 1
fm_backend_clear_transition "$BACKEND" "$STATE" "$T" || true
if [ -z "$TEARDOWN_ENDPOINT_CLEARED" ]; then
fm_backend_clear_transition "$BACKEND" "$STATE" "$T" || true
fi
# Remove the per-task temp root (/tmp/fm-<id>/, incl. its gotmp/) recorded by spawn.
# Read before the state-file rm below; empty (pre-fix tasks without tasktmp=) is a no-op.
[ -n "$TASK_TMP" ] && rm -rf "$TASK_TMP"
Expand Down Expand Up @@ -3620,10 +3661,10 @@ if [ -d "$STATE" ]; then
"$SCRIPT_DIR/fm-home-summary-refresh.sh" --best-effort || true
fi
if [ "$TEARDOWN_LEGACY_ACCEPTED" = 1 ]; then
echo "teardown $ID complete (window $T, worktree $WT, legacy record accepted without spawn_gen: endpoint $TEARDOWN_LEGACY_ENDPOINT, incarnation $TEARDOWN_META_SPAWN_GEN)"
echo "teardown $ID complete (window $TEARDOWN_WINDOW_DISPLAY, worktree $WT, legacy record accepted without spawn_gen: endpoint $TEARDOWN_LEGACY_ENDPOINT, incarnation $TEARDOWN_META_SPAWN_GEN)"
elif teardown_owns_worktree; then
echo "teardown $ID complete (window $T, worktree $WT)"
echo "teardown $ID complete (window $TEARDOWN_WINDOW_DISPLAY, worktree $WT)"
else
echo "teardown $ID complete (window $T; pool slot $WT left to task $TEARDOWN_SLOT_REASSIGNED_TO${TEARDOWN_SLOT_REASSIGNED_HOME:+ (home $TEARDOWN_SLOT_REASSIGNED_HOME)}, which it was reassigned to)"
echo "teardown $ID complete (window $TEARDOWN_WINDOW_DISPLAY; pool slot $WT left to task $TEARDOWN_SLOT_REASSIGNED_TO${TEARDOWN_SLOT_REASSIGNED_HOME:+ (home $TEARDOWN_SLOT_REASSIGNED_HOME)}, which it was reassigned to)"
fi
backlog_refresh_reminder
Loading
Loading