diff --git a/.agents/skills/stuck-crewmate-recovery/SKILL.md b/.agents/skills/stuck-crewmate-recovery/SKILL.md index 64d809c798d..890365aa90e 100644 --- a/.agents/skills/stuck-crewmate-recovery/SKILL.md +++ b/.agents/skills/stuck-crewmate-recovery/SKILL.md @@ -27,6 +27,7 @@ For a REMOTE secondmate, `fm-crew-state` and `fm-peek` read the actual remote en Recover a genuinely stuck remote mate only through `bin/fm-spawn.sh --secondmate`, never raw herdr pane close/kill surgery, which strands the endpoint binding. Treat the digest's endpoint result as a presence signal, not proof that the task's work or validation run is gone. +A digest line of `endpoint: closed` means the finished worker's pane was retired on purpose while the isolated copy is still retained for landing; do not relaunch that worker. Read the targeted current state with `bin/fm-crew-state.sh ` before deciding to relaunch. A no-mistakes run matched to the crew's branch and current code remains authoritative when the endpoint is dead: handle a terminal or parked run through the normal lifecycle, and keep supervising an active run instead of creating a duplicate worker. diff --git a/AGENTS.md b/AGENTS.md index a50f6afe5cf..eb3c3d846e4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -363,6 +363,8 @@ The worker reports the PR when CI first becomes green rather than waiting for me For PR-based ship tasks, the ready signal depends on mode: `no-mistakes` reports `done: PR checks green` after CI is green, while `direct-PR` reports `done: PR ` after opening the PR. Run `bin/fm-pr-check.sh ` - it records `pr=` and the forge's `pr_head=` when available in the task's meta and arms the watcher's merge poll. +A finished ship or scout pane is closed once the worker process has exited, when the captain has opted in with `config/close-exited-panes` or `FM_CLOSE_EXITED_PANES=1`. +The isolated copy stays until landed cleanup, and that empty pane is not a stuck worker. Tell the captain the PR's full URL, always the complete `https://...` link rather than a bare `#number`, a concise outcome summary, and the no-mistakes risk level when applicable. A captain instruction to merge is explicit authority; `yolo` is the only standing routine merge authority. For any custom `state/.check.sh` you write yourself, keep it an ordinary single-link mode-`0700` file, print one line only when firstmate should wake, print nothing otherwise, finish before `FM_CHECK_TIMEOUT`, then bind its current bytes with `bin/fm-check-register.sh ` before the watcher may execute it. diff --git a/bin/backends/tmux.sh b/bin/backends/tmux.sh index 9eed5f3ec3e..edeb7734f88 100644 --- a/bin/backends/tmux.sh +++ b/bin/backends/tmux.sh @@ -42,6 +42,21 @@ fm_backend_tmux_capture() { # tmux capture-pane -p -t "$1" -S -"$2" } +# fm_backend_tmux_capture_joined: like fm_backend_tmux_capture, but with `-J` +# so a logical line that soft-wrapped across the terminal width comes back as +# ONE row instead of split across several. Plain fm_backend_tmux_capture must +# stay byte-identical to the legacy inline tmux command (callers above rely on +# that), so this is a separate function rather than an added flag there. Used +# by proof checks that need the FULL logical line - e.g. +# fm-teardown.sh's --close-pane trailing-shell-glyph-only check +# (fm_composer_trailing_shell_glyph_only in bin/fm-composer-lib.sh), where +# inspecting only the last wrapped fragment of a long unsubmitted command can +# read a lone trailing prompt glyph as proof of an empty prompt (Greptile P1: +# task fm-close-exited-panes review). +fm_backend_tmux_capture_joined() { # + tmux capture-pane -p -J -t "$1" -S -"$2" +} + # fm_backend_tmux_send_key: one named key. Mirrors fm-send.sh's --key path: # `tmux display-message -p -t "$T" '#{pane_id}' >/dev/null`, then # `tmux send-keys -t "$T" "$2"`. diff --git a/bin/fm-backend.sh b/bin/fm-backend.sh index 2882f4a6af2..ebbe7d91fd5 100644 --- a/bin/fm-backend.sh +++ b/bin/fm-backend.sh @@ -706,6 +706,44 @@ fm_backend_capture() { # [expected-label] esac } +# fm_backend_capture_joined: like fm_backend_capture, but soft-wrapped rows +# come back joined into one logical line where the backend supports it (tmux +# `-J`), instead of split across several physical rows. For a caller that +# must prove something about one FULL logical line - e.g. --close-pane's +# trailing-shell-glyph-only proof (fm-teardown.sh) - inspecting only the last +# physical row of a wrapped line can find a lone trailing prompt glyph and +# wrongly treat unsubmitted typed text as an empty prompt (Greptile P1: task +# fm-close-exited-panes review). Backends without a documented join primitive +# fall back to fm_backend_capture unchanged. +fm_backend_capture_joined() { # [expected-label] + local backend=$1 + shift + fm_backend_source "$backend" || return 1 + case "$backend" in + tmux) fm_backend_tmux_capture_joined "$@" ;; + *) fm_backend_capture "$backend" "$@" ;; + esac +} + +# fm_backend_capture_joined_reliable: 0 only for a backend whose +# fm_backend_capture_joined performs genuine logical-line joining of +# soft-wrapped rows (tmux -J). Every other backend's arm above silently falls +# back to the row-oriented fm_backend_capture, so its "joined" output still +# splits a long unsubmitted command across physical rows. A caller proving a +# full logical line is empty before an irreversible action (fm-teardown.sh's +# --close-pane composer-emptiness fallback proof) must not trust that +# fallback capture on a backend where this returns non-zero - inspecting only +# the last physical row of a wrapped line can find a lone trailing prompt +# glyph and wrongly treat unsubmitted typed text as an empty prompt (Greptile +# P1: task fm-close-exited-panes review - Herdr reaches the same close logic +# through an unchanged row-oriented capture). +fm_backend_capture_joined_reliable() { # + case "$1" in + tmux) return 0 ;; + *) return 1 ;; + esac +} + # fm_backend_send_key: one backend-supported named special key. fm_backend_send_key() { # [expected-label] local backend=$1 diff --git a/bin/fm-composer-lib.sh b/bin/fm-composer-lib.sh index 07b3b02fffb..a31d174d3d7 100644 --- a/bin/fm-composer-lib.sh +++ b/bin/fm-composer-lib.sh @@ -454,6 +454,55 @@ EOF return 1 } +# fm_composer_trailing_shell_glyph_only: true when , trimmed, ENDS in +# one of the shell prompt glyphs (FM_COMPOSER_SHELL_PROMPT_GLYPHS) with +# nothing after it. A real interactive shell's PS1 draws arbitrary text +# BEFORE the glyph (user, host, cwd, git branch, ...), unlike an agent CLI's +# minimal glyph-anchored composer, so a live PS1 never matches this library's +# glyph-ANCHORED shapes above and the screen classifier always reads +# `unknown` for it - whether or not anything was typed after the prompt. A +# caller that must positively rule out unsubmitted typed text before trusting +# that `unknown` (e.g. pane-close cleanup, task fm-close-exited-panes) uses +# this instead: unlike fm_composer_leading_shell_glyph_var, the glyph need not +# lead the row, only trail it, so `user@host:~$` reads true and +# `user@host:~$ rm -rf` reads false. +# +# A trailing glyph alone is NOT sufficient proof: a real idle PS1 draws +# exactly one prompt glyph (its own, trailing), but unsubmitted typed text +# ending in a prompt-glyph character - `cat >`, `echo $` - reads as a SECOND +# occurrence once appended after that real prompt (`user@host:~$ cat >`, +# `user@host:~$ echo $`), and a suffix-only check would wrongly call that an +# empty prompt (task fm-close-exited-panes review). So this also requires the +# trailing glyph to be the ONLY prompt-glyph character anywhere in the line; +# two or more occurrences means typed text follows the real prompt and reads +# false. +# +# A bare `>` with NOTHING else on the line is still not proof, even as the +# sole occurrence: bash/zsh/dash all default PS2 (the secondary prompt shown +# mid multiline input - an unclosed quote, heredoc, or paren) to exactly +# `> ` with no host/path/branch prefix, which trims to the same lone `>` a +# real idle PS1 would never draw on its own. A `>` preceded by other prompt +# content (`user@host:~>`) is unambiguous and still accepted (Greptile P1: +# task fm-close-exited-panes review). +fm_composer_trailing_shell_glyph_only() { # + local __fmtg_trimmed=$1 __fmtg_glyph __fmtg_matched='' __fmtg_class __fmtg_count + fm_composer_normalize_trim_var __fmtg_trimmed + [ -n "$__fmtg_trimmed" ] || return 1 + while IFS= read -r __fmtg_glyph; do + [ -n "$__fmtg_glyph" ] || continue + case "$__fmtg_trimmed" in + *"$__fmtg_glyph") __fmtg_matched=$__fmtg_glyph; break ;; + esac + done < [--force] +# Usage: fm-teardown.sh [--force|--close-pane] # --force skips ordinary-task dirty and landed-work checks, skips scout report # checks, and discards secondmate child work for kind=secondmate. Only use it # when the captain has explicitly said to discard the work. +# --close-pane is ordinary finished-worker cleanup, not landing and not discard. +# It closes a ship or scout backend pane only when the recovery-grade +# classifier reports the agent gone (dead, or already missing), the task is +# finished (a recorded pr=, a scout report, or a done:/failed: status), and +# the composer is not pending. It never returns the isolated copy, never +# removes task records, and never discards unlanded work. A live agent, an +# idle live agent, a pending composer, a secondmate, an unverified backend +# (zellij, orca, cmux), or a Herdr pane that is the captain's active tab is +# refused and the pane is kept. After a confirmed close it records +# pane_closed=1 so supervision does not treat the leftover husk as stale or +# as a missing-endpoint recovery. Full teardown after landing still returns +# the copy. --close-pane and --force cannot be combined. # # Transient / stale worktree git lock recovery (teardown-lock-race): a crew process # killed mid-git-operation can leave a .git/worktrees//index.lock (or, for a @@ -179,7 +191,23 @@ if [ "$#" -lt 1 ] || ! fm_task_id_path_safe "$1"; then exit 2 fi ID=$1 -FORCE=${2:-} +shift +FORCE= +CLOSE_PANE=0 +for arg in "$@"; do + case "$arg" in + --force) FORCE=--force ;; + --close-pane) CLOSE_PANE=1 ;; + *) + echo "error: invalid teardown option: $arg" >&2 + exit 2 + ;; + esac +done +if [ "$CLOSE_PANE" = 1 ] && [ "$FORCE" = --force ]; then + echo "error: --close-pane cannot be combined with --force" >&2 + exit 2 +fi # shellcheck source=bin/fm-wake-lib.sh . "$SCRIPT_DIR/fm-wake-lib.sh" # Supervision lease guard: post-landing cleanup is overlap territory between @@ -658,12 +686,14 @@ remote_secondmate_teardown_locked() { return "$rc" } -if remote_secondmate_teardown_locked; then - exit 0 -else - remote_teardown_rc=$? +if [ "$CLOSE_PANE" != 1 ]; then + if remote_secondmate_teardown_locked; then + exit 0 + else + remote_teardown_rc=$? + fi + [ "$remote_teardown_rc" -eq 3 ] || exit "$remote_teardown_rc" fi -[ "$remote_teardown_rc" -eq 3 ] || exit "$remote_teardown_rc" # This is the first cleanup authorization check. It is metadata-only and must # complete before fm-guard, a backend command, file removal, branch deletion, @@ -2387,6 +2417,221 @@ $session $lock_path" return 1 } +teardown_status_is_terminal_outcome() { + local last + last=$(tail -n 1 "$STATE/$ID.status" 2>/dev/null || true) + case "$last" in + done:*|failed:*) return 0 ;; + esac + return 1 +} + +# A pane close is ordinary finished-worker cleanup, never a substitute for +# landed teardown. Crash husks without a finish signal stay so recovery can +# still see them. +teardown_task_finished_for_pane_close() { + case "$KIND" in + secondmate) return 1 ;; + scout) + if [ -f "$DATA/$ID/report.md" ] && [ ! -L "$DATA/$ID/report.md" ]; then + return 0 + fi + ;; + esac + [ -n "$PR_URL" ] && return 0 + teardown_status_is_terminal_outcome +} + +teardown_composer_blocks_pane_close() { + local verdict cap line last='' + # A dead agent's shell prompt reads `unknown` by the fleet-wide dead-shell + # safety rule (bin/fm-composer-lib.sh) even with no typed content, so + # requiring the proven `empty` verdict here would refuse every ordinary + # exit. Real unsent input is positively detected as pending/pending-unproven + # and still blocks. A composer-inspection FAILURE (the classifier itself + # erroring) blocks too, defaulting to safe. + verdict=$(fm_backend_composer_state "$BACKEND" "$T" "fm-$ID" 2>/dev/null) || return 0 + case "$verdict" in + empty) return 1 ;; + pending|pending-unproven) return 0 ;; + esac + # verdict is `unknown`. A real shell PS1 (user/host/cwd/branch drawn before + # the prompt glyph) never matches fm-composer-lib.sh's glyph-anchored + # composer shapes, so an ordinary dead-shell exit always reads `unknown` + # here regardless of whether something was typed after the prompt - + # trusting `unknown` alone would not establish that pending text is absent + # (task fm-close-exited-panes review). Prove it directly instead: only a + # capture whose bottom-most non-blank row ends in a bare prompt glyph, with + # nothing after it, is safe. A capture failure or any other trailing + # content blocks. + # A joined capture, not the plain capture, so a long unsubmitted command + # that soft-wrapped across the terminal width is inspected as ONE logical + # line rather than just its last physical fragment - a lone trailing + # prompt glyph on that fragment alone is not proof the composer is empty + # (Greptile P1: task fm-close-exited-panes review). Only trust this on a + # backend where fm_backend_capture_joined actually performs that join + # (fm_backend_capture_joined_reliable): every other backend's "joined" + # capture silently falls back to the row-oriented plain capture, so a + # wrapped command would still reach this proof one physical fragment at a + # time and could pass it wrongly (Greptile P1: task fm-close-exited-panes + # review - Herdr reaches this same fallback through an unjoined capture). + fm_backend_capture_joined_reliable "$BACKEND" || return 0 + cap=$(fm_backend_capture_joined "$BACKEND" "$T" "${FM_COMPOSER_CAPTURE_LINES:-20}" 2>/dev/null) || return 0 + while IFS= read -r line; do + fm_composer_normalize_trim_var line + [ -n "$line" ] && last=$line + done <> "$tmp" || { rm -f "$tmp"; return 1; } + mv -f "$tmp" "$META" +} + +teardown_endpoint_confirmed_absent() { + case "$BACKEND" in + herdr) + fm_backend_source herdr || return 1 + fm_backend_herdr_endpoint_confirmed_gone "$T" + ;; + *) + [ "$(fm_backend_agent_state "$BACKEND" "$T")" = missing ] + ;; + esac +} + +teardown_herdr_pane_is_captain_active_tab() { + local session=$1 pane=$2 before focused_tab info tab_id + before=$(fm_backend_herdr_projection_focus_snapshot "$session") || return 0 + focused_tab=${before#*$'\t'} + [ -n "$focused_tab" ] || return 0 + info=$(fm_backend_herdr_cli "$session" pane get "$pane" 2>/dev/null) || return 0 + tab_id=$(printf '%s' "$info" | jq -r '.result.pane.tab_id // empty' 2>/dev/null) || tab_id= + [ -n "$tab_id" ] || return 0 + [ "$tab_id" = "$focused_tab" ] +} + +teardown_close_herdr_task_pane() { + local session pane journal retire=0 + teardown_herdr_preflight_target "$T" "$ID" || return 1 + fm_backend_herdr_parse_target "$T" || return 1 + session=$FM_BACKEND_HERDR_SESSION + pane=$FM_BACKEND_HERDR_PANE + if teardown_herdr_pane_is_captain_active_tab "$session" "$pane"; then + echo "REFUSED: herdr pane $T for $ID is the captain's active tab; keeping the pane." >&2 + return 1 + fi + journal="$STATE/$ID.herdr-presentation" + if { [ -e "$journal" ] || [ -L "$journal" ]; } \ + && [ "$(meta_value "$META" herdr_session)" = "$session" ] \ + && [ "$(meta_value "$META" herdr_pane_id)" = "$pane" ] \ + && [ "$T" = "$session:$pane" ] \ + && fm_backend_herdr_projection_endpoint_matches_journal \ + "$session" "$(meta_value "$META" herdr_workspace_id)" "$journal" "$ID"; then + retire=1 + if teardown_herdr_session_lock_held "$session"; then + fm_backend_herdr_projection_close_pane_focus_preserving "$session" "$pane" || true + else + echo "warning: herdr presentation focus lock unavailable; refusing a concurrent focus-unsafe pane close" >&2 + return 1 + fi + elif teardown_herdr_session_lock_held "$session"; then + fm_backend_herdr_kill_serialized "$session" "$pane" 2>/dev/null || true + else + echo "warning: herdr session presentation lock path is unavailable; skipping the pane close rather than closing unlocked" >&2 + return 1 + fi + if ! teardown_endpoint_confirmed_absent; then + echo "error: herdr pane $T for $ID is not confirmed gone after close; keeping the pane and every durable record" >&2 + return 1 + fi + if [ "$retire" = 1 ]; then + rm -f "$journal" + fi + return 0 +} + +teardown_close_recorded_endpoint() { + case "$BACKEND" in + herdr) + teardown_close_herdr_task_pane + ;; + orca) + echo "REFUSED: backend $BACKEND has no recovery-grade agent-state classifier; keeping the pane." >&2 + return 1 + ;; + *) + fm_backend_kill "$BACKEND" "$T" "$(meta_value "$META" zellij_tab_id)" "fm-$ID" 2>/dev/null || true + teardown_endpoint_confirmed_absent + ;; + esac +} + +teardown_close_exited_pane() { + local agent_state already + if [ "$KIND" = secondmate ]; then + echo "REFUSED: --close-pane does not apply to a secondmate; its idle pane is healthy." >&2 + return 1 + fi + case "$KIND" in + ship|scout) ;; + *) + echo "REFUSED: --close-pane applies only to ship or scout workers." >&2 + return 1 + ;; + esac + if ! teardown_task_finished_for_pane_close; then + echo "REFUSED: task $ID is not a finished ship or scout; keeping the pane so an unfinished exit can still be recovered." >&2 + return 1 + fi + already=$(fm_meta_get "$META" pane_closed) + agent_state=$(fm_backend_agent_state "$BACKEND" "$T" 2>/dev/null || printf 'unreadable') + if [ "$already" = 1 ] && [ "$agent_state" = missing ]; then + echo "close-pane $ID complete (endpoint already gone, copy retained at $WT)" + return 0 + fi + case "$agent_state" in + missing) + teardown_mark_pane_closed || return 1 + echo "close-pane $ID complete (endpoint already gone, copy retained at $WT)" + return 0 + ;; + dead) ;; + alive) + echo "REFUSED: task $ID still has a live agent; keeping the pane." >&2 + return 1 + ;; + *) + echo "REFUSED: cannot prove the agent for $ID has exited (state $agent_state); keeping the pane." >&2 + return 1 + ;; + esac + if teardown_composer_blocks_pane_close; then + echo "REFUSED: task $ID still has pending composer text; keeping the pane." >&2 + return 1 + fi + teardown_close_recorded_endpoint || { + echo "error: could not close the exited pane for $ID; keeping the pane and the isolated copy" >&2 + return 1 + } + if ! teardown_endpoint_confirmed_absent; then + echo "error: pane $T for $ID is still present after close; keeping the pane and the isolated copy" >&2 + return 1 + fi + teardown_mark_pane_closed || return 1 + echo "close-pane $ID complete (window $T closed, copy retained at $WT)" +} + preflight_firstmate_home_herdr_children() { # local home=$1 sub_state child_meta child_id child_backend child_target child_kind child_home child_wt sub_state="$home/state" @@ -2521,6 +2766,11 @@ remove_secondmate_registry_entry() { validate_pr_poll_cleanup "$STATE" "$ID" || exit 1 +if [ "$CLOSE_PANE" = 1 ]; then + teardown_close_exited_pane + exit $? +fi + if [ "$KIND" = secondmate ]; then LOCAL_REGISTRY_LOCK=$(secondmate_registry_lock_path "$STATE") fm_lock_acquire_wait "$LOCAL_REGISTRY_LOCK" || exit 1 diff --git a/bin/fm-test-run.sh b/bin/fm-test-run.sh index 2d3a64bcadb..033ab6c47ed 100755 --- a/bin/fm-test-run.sh +++ b/bin/fm-test-run.sh @@ -208,7 +208,7 @@ family_for_basename() { fm-send-inbox.test.sh|fm-spawn-batch.test.sh|\ fm-spawn-dispatch-profile.test.sh|\ fm-trace-context-spawn.test.sh|fm-spawn-worktree-settle.test.sh|\ - fm-teardown-endpoint-safety.test.sh) + fm-teardown-endpoint-safety.test.sh|fm-teardown-close-pane.test.sh) printf '%s\n' backend-dispatch ;; fm-pr-check-security.test.sh|fm-pr-merge.test.sh|fm-review-diff.test.sh|\ diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index b3ef0344812..f805d90c084 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -15,6 +15,14 @@ # signal: ... status/turn-end signals, surfaced when a listed status # has a captain-relevant verb OR a no-verb signal's crew # is not provably working, unless afk is active +# A finished ship or scout whose recovery-grade classifier reports the agent +# gone is closed through bin/fm-teardown.sh --close-pane instead of a stale +# wake, so an empty pane waiting on merge does not keep alarming; the copy stays +# until landed teardown. Live agents and unfinished exits are left alone. Per +# VISION.md ("autonomy exists only as an explicit grant, never as a default"), +# this automatic retirement is itself opt-in: it stays inert until +# config/close-exited-panes exists (or FM_CLOSE_EXITED_PANES=1 is set), exactly +# the config-is-the-opt-in idiom bin/fm-inbox.sh uses for `say`/`ask`. # stale: a provably-working stale is ALWAYS absorbed (with a wedge # timer) regardless of what the status log says - an active # run-step or busy pane outranks even a captain-relevant log @@ -368,6 +376,7 @@ recorded_windows() { local meta w seen= for meta in "$STATE"/*.meta; do [ -e "$meta" ] || continue + [ "$(fm_meta_get "$meta" pane_closed)" = 1 ] && continue w=$(fm_backend_target_of_meta "$meta") [ -n "$w" ] || continue case "$seen" in @@ -378,6 +387,55 @@ recorded_windows() { done } +# Explicit opt-in for automatic pane retirement (see header note above): off +# until the captain configures it, so upgrading this watcher never starts +# closing panes on its own. FM_CLOSE_EXITED_PANES wins when set; otherwise the +# config file's mere existence is the grant, same idiom as fm-inbox.sh's +# config/inbox-* opt-in for say/ask. +close_exited_panes_enabled() { + case "${FM_CLOSE_EXITED_PANES:-}" in + 1|true|yes) return 0 ;; + 0|false|no) return 1 ;; + esac + [ -e "${FM_CONFIG_OVERRIDE:-$FM_HOME/config}/close-exited-panes" ] +} + +# Close a finished ship/scout pane whose agent has actually exited, without +# landing the copy. Cheap local gates avoid calling teardown on every poll: +# unfinished crash husks stay visible for recovery. Teardown owns the close. +close_finished_exited_pane() { # + local win=$1 task=$2 meta kind last agent_state data_dir report + [ -n "$task" ] || return 1 + meta="$STATE/$task.meta" + [ -f "$meta" ] || return 1 + [ "$(fm_meta_get "$meta" pane_closed)" = 1 ] && return 0 + kind=$(window_kind "$win") + case "$kind" in + ship|scout) ;; + *) return 1 ;; + esac + data_dir="${FM_DATA_OVERRIDE:-$FM_HOME/data}" + report="$data_dir/$task/report.md" + last=$(last_status_line "$STATE/$task.status") + if [ -z "$(fm_meta_get "$meta" pr)" ]; then + case "$last" in + done:*|failed:*) ;; + *) + if [ "$kind" != scout ] || [ ! -f "$report" ] || [ -L "$report" ]; then + return 1 + fi + ;; + esac + fi + agent_state=$(fm_backend_agent_state "$(window_backend "$win")" "$win" 2>/dev/null || printf 'unreadable') + case "$agent_state" in + dead|missing) ;; + *) return 1 ;; + esac + FM_HOME="$FM_HOME" FM_STATE_OVERRIDE="$STATE" \ + "$SCRIPT_DIR/fm-teardown.sh" "$task" --close-pane >/dev/null 2>&1 +} + # Print the oldest structurally valid row in a local secondmate's foreign queue. # This is a read-only observation: the receiving home owns acknowledgement and # this parent never changes the row or the foreign queue. @@ -1304,6 +1362,12 @@ EOF while IFS= read -r w; do kind=$(window_kind "$w") task=$(window_to_task "$w" "$STATE") + if [ -n "$task" ] && close_exited_panes_enabled && close_finished_exited_pane "$w" "$task"; then + key=$(window_key "$w") + clear_pause_tracking "$key" + triage_log "closed exited pane: $w" + continue + fi # Steering-inbox loss detection runs before the secondmate stale # exemption below, because a mate's steers land in an inbox too. [ -z "$task" ] || inbox_steer_check "$w" "$task" diff --git a/docs/agent-control.md b/docs/agent-control.md index af50ab75058..f37c49b7b28 100644 --- a/docs/agent-control.md +++ b/docs/agent-control.md @@ -45,7 +45,7 @@ The clear is refused before anything is sent when the recorded backend cannot de **Teardown and discard are not verbs and will not become verbs.** `exit` stops an agent and preserves everything else. -Removing a worktree, closing an endpoint, or discarding work stays with [`bin/fm-teardown.sh`](../bin/fm-teardown.sh), which owns the landed-work test. +Removing a worktree, closing an endpoint, or discarding work stays with [`bin/fm-teardown.sh`](../bin/fm-teardown.sh), which owns the landed-work test and the finished-worker `--close-pane` path that retires an already-exited ship or scout pane without returning the copy. **`resume` is not a verb.** It is not deterministic across the verified adapters: codex and grok resume only from a session id printed at exit, opencode continues the most recent session for the cwd, and claude, pi, pi-signed, and kimi have no verified pane-resume contract. diff --git a/docs/architecture.md b/docs/architecture.md index a25bb20430f..f36a2462068 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -15,6 +15,7 @@ A pane holding a file newer than the start of its own quiet window, anywhere in That deferral re-surfaces on the same `FM_PAUSE_RESURFACE_SECS` cadence as a declared wait, with a reason naming the write evidence rather than a wedge, and it is bounded to one pruned, depth-bounded, wall-clock-bounded walk (`FM_WORKTREE_WRITE_PRUNE`, `FM_WORKTREE_WRITE_MAXDEPTH`, `FM_WORKTREE_WRITE_TIMEOUT`) taken only in the branch that was about to escalate, never on every poll. Every absence of write evidence, including a missing worktree record, a torn-down worktree, a walk that outlives its wall-clock bound on a hung mount, and a failed walk, leaves the existing escalation schedule untouched, so a crew that writes nothing still escalates exactly as before. A secondmate's recorded worktree is never probed for write activity, because it is a provisioned firstmate home whose own supervision keeps writing inside it whether or not the mate produces anything, so its panes keep escalating on the unchanged schedule. +A finished ship or scout whose agent has actually exited is closed through `bin/fm-teardown.sh --close-pane` instead of a stale wake, so an empty pane waiting on merge does not keep alarming while the isolated copy stays until landed teardown; the watcher only does this automatically once the captain opts in with `config/close-exited-panes` (or `FM_CLOSE_EXITED_PANES=1`), so upgrading never starts closing panes on its own. A busy pane is otherwise exempt from staleness, but only until its latest `state/.turn-ended` marker reaches `FM_BUSY_TURN_MAX_SECS`, or its `state/.meta` spawn record reaches that age before any turn completes; past that bound it is routed through the same wedge escalation, with the identical reason, escalation count, worktree-write deferral, and `demand-deep-inspection` marker, for inspection only - never an automatic interrupt, signal, or restart. A crew that declared an external wait (`paused:`) or a verified captain-held transfer is the one exception to that bound: its busy verdict supplies liveness while identifying the long-running foreground call as the declared wait, so it takes the bounded `FM_PAUSE_RESURFACE_SECS` recheck instead of a wedge escalation. Lifting the declaration restores the unchanged busy-pane wedge path, while a pane that is no longer busy returns to the existing idle declared-wait classification. diff --git a/docs/configuration.md b/docs/configuration.md index df27bcbda26..f897aa79288 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -94,6 +94,8 @@ A metadata-routed selector returns the recorded backend target (`terminal=` for Only metadata-routed task selectors carry secondmate-marker and Codex-harness context; explicit endpoint escape hatches do not. These five sentences are the single owner of the task-selector vocabulary; backend guides and other documents point here instead of restating the resolution order. `fm-teardown.sh ` takes a task id directly and validates the complete metadata-only endpoint identity before any runtime dispatch or cleanup mutation. +`--close-pane` is owned by that script's header: it closes a confirmed-exited finished ship or scout pane, records `pane_closed=1`, and leaves the isolated copy and task records in place until landed teardown. +`bin/fm-watch.sh` only calls `--close-pane` on its own once the captain opts in: `config/close-exited-panes` (any content, mere presence is the grant) or `FM_CLOSE_EXITED_PANES=1`; `FM_CLOSE_EXITED_PANES=0` forces it off even when the config file exists. Missing, empty, duplicate, malformed, backend-inconsistent, or task-mismatched endpoint records are preserved and refused. Legacy tmux metadata remains cleanup-compatible when its exact window name is `fm-`; opaque non-tmux endpoints require their recorded `endpoint_task_id=` binding. `FM_HOME` determines Herdr's home label: the primary home uses `firstmate`, and a secondmate home marked by `.fm-secondmate-home` uses `2ndmate-`. diff --git a/docs/scripts.md b/docs/scripts.md index 5408ce683d7..653e930cf84 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -115,7 +115,7 @@ The shared no-mistakes gate refusal for fleet lifecycle entrypoints is summarize | `fm-pr-check.sh` | Record validated `pr=` and `pr_head=` values, then atomically arm a static merge poll | | `fm-pr-merge.sh` | Record PR metadata, then merge a task's canonical full GitHub or GitLab URL | | `fm-promote.sh` | Promote a scout task in place to a protected ship task with an explicit delivery mode | -| `fm-teardown.sh` | Fail-closed teardown: return landed ship worktrees, require completed scout deliverables, retire secondmate homes | +| `fm-teardown.sh` | Fail-closed teardown: return landed ship worktrees, require completed scout deliverables, retire secondmate homes; `--close-pane` retires an already-exited ship or scout pane without landing the copy | | `fm-harness.sh` | Detect the running harness and resolve crew or secondmate harness, model, and effort | | `fm-lock.sh` | Per-home firstmate session lock | | `fm-x-lib.sh` | Shared Relay config, relay, and reply-threading helpers | diff --git a/tests/fm-composer-lib.test.sh b/tests/fm-composer-lib.test.sh index 2d61ffda865..b89163bbcd5 100755 --- a/tests/fm-composer-lib.test.sh +++ b/tests/fm-composer-lib.test.sh @@ -62,6 +62,69 @@ test_bare_shell_prompt_with_command_is_not_empty() { pass "fm_composer_classify_content: a bare shell prompt carrying a command is not empty" } +# --- fm_composer_trailing_shell_glyph_only: a caller's narrower positive proof +# that the generic `unknown` verdict for a bare dead-shell prompt (see above - +# a real PS1's user/host/cwd prefix never matches this library's +# glyph-ANCHORED shapes, so it always reads `unknown`, typed content or not) +# holds no unsubmitted typed text. Task fm-close-exited-panes review. + +test_trailing_shell_glyph_only_matches_a_real_ps1_prompt() { + local prompt + for prompt in 'user@host:~$' 'user@host:~ %' 'root@host:/etc #' 'C:\Users\me>'; do + fm_composer_trailing_shell_glyph_only "$prompt" \ + || fail "a real PS1 prompt '$prompt' ending in a bare glyph must read true, did not" + done + pass "fm_composer_trailing_shell_glyph_only: a real PS1 prompt ending bare is true" +} + +test_trailing_shell_glyph_only_rejects_typed_content() { + local line + for line in 'user@host:~$ rm -rf /tmp/x' 'user@host:~$ ls -la' ' $ echo hi '; do + ! fm_composer_trailing_shell_glyph_only "$line" \ + || fail "a prompt with typed content after it '$line' must read false, read true" + done + pass "fm_composer_trailing_shell_glyph_only: typed content after the prompt is false" +} + +test_trailing_shell_glyph_only_rejects_typed_content_ending_in_a_glyph() { + local line + # Unsubmitted typed text that itself ends in a prompt-glyph character (a + # redirection `cat >`, a bare `echo $`) appended after a real PS1 must never + # read true just because the LINE happens to end in a glyph: the real + # prompt's own glyph plus the typed one is a second occurrence, proof that + # something was typed. Greptile P1: bin/fm-composer-lib.sh#477. + for line in 'user@host:~$ cat >' 'user@host:~$ echo $' 'root@host:/etc # cat >'; do + ! fm_composer_trailing_shell_glyph_only "$line" \ + || fail "typed text ending in a glyph after a real prompt '$line' must read false, read true" + done + pass "fm_composer_trailing_shell_glyph_only: typed content that itself ends in a glyph is false" +} + +test_trailing_shell_glyph_only_rejects_blank_or_glyphless() { + local line + for line in '' ' ' 'no glyph here'; do + ! fm_composer_trailing_shell_glyph_only "$line" \ + || fail "a blank or glyph-less line '$line' must read false, read true" + done + pass "fm_composer_trailing_shell_glyph_only: blank or glyph-less input is false" +} + +test_trailing_shell_glyph_only_rejects_bare_continuation_prompt() { + local line + # bash/zsh/dash all default PS2 (the secondary prompt shown mid an + # incomplete multiline command - unclosed quote, heredoc, or paren) to + # exactly `> `, which trims to a lone `>` indistinguishable from a real + # idle PS1 that happens to draw nothing before its own glyph. Trusting + # that shape as proof of an empty prompt would let pane-close cleanup + # discard a still-pending multiline command. Greptile P1: task + # fm-close-exited-panes review. + for line in '>' ' > '; do + ! fm_composer_trailing_shell_glyph_only "$line" \ + || fail "a bare continuation prompt '$line' must read false, read true" + done + pass "fm_composer_trailing_shell_glyph_only: a bare continuation prompt is false" +} + # --- Preserved: shell glyph inside a composer box is the harness prompt ------ test_bordered_shell_glyph_is_empty() { @@ -606,6 +669,11 @@ test_selected_content_is_composer_scoped_and_wrap_normalized() { test_bare_shell_glyphs_are_unknown test_stripped_unbordered_content_uses_plain_content test_bare_shell_prompt_with_command_is_not_empty +test_trailing_shell_glyph_only_matches_a_real_ps1_prompt +test_trailing_shell_glyph_only_rejects_typed_content +test_trailing_shell_glyph_only_rejects_typed_content_ending_in_a_glyph +test_trailing_shell_glyph_only_rejects_blank_or_glyphless +test_trailing_shell_glyph_only_rejects_bare_continuation_prompt test_bordered_shell_glyph_is_empty test_agent_glyphs_are_empty_bordered_and_bare test_empty_content_is_empty diff --git a/tests/fm-session-start.test.sh b/tests/fm-session-start.test.sh index e74eceb7abf..23645f5f5c5 100755 --- a/tests/fm-session-start.test.sh +++ b/tests/fm-session-start.test.sh @@ -1341,6 +1341,27 @@ EOF pass "herdr endpoint liveness is reported per task: alive for a live pane, dead for a gone one" } +test_endpoint_liveness_closed_pane() { + local rec root home fakebin out + rec=$(new_world liveness-closed) + IFS='|' read -r root home fakebin < "$home/state/task-closed.meta" + + out=$(run_session_start "$home" "$root" "$fakebin:$BASE_PATH") + assert_contains "$out" "endpoint: closed (backend=tmux window=fm-sess:gone-window)" \ + "a finished worker's closed pane must not report as a dead endpoint" + assert_not_contains "$out" "endpoint: dead (backend=tmux window=fm-sess:gone-window)" \ + "a closed pane was still reported dead" + + pass "a pane_closed task is reported closed rather than dead" +} + # --- composition: real scripts run, not reimplemented ------------------------ test_composition_invokes_real_scripts() { @@ -2479,6 +2500,7 @@ test_status_tail_line_cap test_orphan_status_logs_are_printed test_endpoint_liveness_tmux test_endpoint_liveness_herdr +test_endpoint_liveness_closed_pane test_composition_invokes_real_scripts test_branch_outcome_replay_and_lease_sweep test_non_pi_session_start_leaves_branch_state_untouched diff --git a/tests/fm-teardown-close-pane.test.sh b/tests/fm-teardown-close-pane.test.sh new file mode 100755 index 00000000000..e62d6145b5f --- /dev/null +++ b/tests/fm-teardown-close-pane.test.sh @@ -0,0 +1,491 @@ +#!/usr/bin/env bash +# Behavioral coverage for fm-teardown.sh --close-pane: a confirmed-exited +# ship or scout pane closes while the isolated copy stays, a live agent is +# left alone, and landed teardown still returns the copy. +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +fm_git_identity fmtest fmtest@example.invalid + +TEARDOWN="$ROOT/bin/fm-teardown.sh" +TMP_ROOT=$(fm_test_tmproot fm-teardown-close-pane) +command -v tmux >/dev/null 2>&1 || { echo "skip: tmux not found"; exit 0; } +REAL_TMUX=$(command -v tmux) +SOCKET="fm-close-pane-$$" +SESSION=closepane + +cleanup_tmux() { + "$REAL_TMUX" -L "$SOCKET" kill-server >/dev/null 2>&1 || true +} +trap cleanup_tmux EXIT + +mkdir -p "$TMP_ROOT/shim" +cat > "$TMP_ROOT/shim/tmux" </dev/null || true +} + +open_shell_window() { # + kill_task_window + "$REAL_TMUX" -L "$SOCKET" new-window -d -t "$SESSION:" -n "$WNAME" -c "$1" \ + || fail "could not create shell window $WNAME" +} + +open_agent_window() { # + local cwd=$1 cmd=$2 + kill_task_window + "$REAL_TMUX" -L "$SOCKET" new-window -d -t "$SESSION:" -n "$WNAME" -c "$cwd" \ + "exec $(printf '%q' "$cmd")" \ + || fail "could not create agent window $WNAME" +} + +window_exists() { + "$REAL_TMUX" -L "$SOCKET" list-windows -t "$SESSION" -F '#{window_name}' | grep -Fqx "$WNAME" +} + +wait_for_state() { # [tries] + local target=$1 expected=$2 tries=${3:-100} got i=0 + while [ "$i" -lt "$tries" ]; do + got=$(PATH="$TMP_ROOT/shim:$PATH" bash -c ' + . "$1" + fm_backend_agent_state tmux "$2" + ' _ "$ROOT/bin/fm-backend.sh" "$target") + [ "$got" = "$expected" ] && return 0 + sleep 0.05 + i=$((i + 1)) + done + fail "agent state for $target never became $expected (last $got)" +} + +make_case() { # + local name=$1 case_dir fakebin + case_dir="$TMP_ROOT/$name" + fakebin="$case_dir/fakebin" + mkdir -p "$case_dir/state" "$case_dir/config" "$case_dir/data" "$fakebin" + cat > "$fakebin/treehouse" <<'SH' +#!/usr/bin/env bash +printf '%s\n' "$*" >> "${FM_TREEHOUSE_LOG:?}" +exit 0 +SH + cat > "$fakebin/gh-axi" <<'SH' +#!/usr/bin/env bash +printf '%s\n' "count: 0 (showing first 0)" "pull_requests[]: []" +exit 0 +SH + cat > "$fakebin/gh" <<'SH' +#!/usr/bin/env bash +exit 0 +SH + cat > "$fakebin/no-mistakes" <<'SH' +#!/usr/bin/env bash +exit 0 +SH + cat > "$fakebin/tasks-axi" <<'SH' +#!/usr/bin/env bash +[ "${1:-}" = --version ] && { printf '0.2.4\n'; exit 0; } +exit 0 +SH + chmod +x "$fakebin"/* + git init -q --bare "$case_dir/origin.git" + git -C "$case_dir/origin.git" symbolic-ref HEAD refs/heads/main + git clone -q "$case_dir/origin.git" "$case_dir/_seed" 2>/dev/null + git -C "$case_dir/_seed" -c user.email=t@t -c user.name=t \ + commit -q --allow-empty -m "origin baseline" + git -C "$case_dir/_seed" push -q origin main + rm -rf "$case_dir/_seed" + git clone -q "$case_dir/origin.git" "$case_dir/project" + git -C "$case_dir/project" remote set-head origin main 2>/dev/null || true + git -C "$case_dir/project" worktree add -q -b "fm/$name" "$case_dir/wt" main + touch "$case_dir/state/.last-watcher-beat" + printf '%s\n' "$case_dir" +} + +write_ship_meta() { # [extra=] + local case_dir=$1 + shift + fm_write_meta "$case_dir/state/task-x1.meta" \ + "window=$SESSION:$WNAME" \ + "endpoint_task_id=task-x1" \ + "worktree=$case_dir/wt" \ + "project=$case_dir/project" \ + "kind=ship" \ + "mode=no-mistakes" \ + "$@" +} + +run_teardown() { # [args...] + local case_dir=$1 + shift + FM_TREEHOUSE_LOG="$case_dir/treehouse.log" \ + FM_ROOT_OVERRIDE="$ROOT" \ + FM_HOME="$case_dir" \ + FM_STATE_OVERRIDE="$case_dir/state" \ + FM_DATA_OVERRIDE="$case_dir/data" \ + FM_CONFIG_OVERRIDE="$case_dir/config" \ + PATH="$TMP_ROOT/shim:$case_dir/fakebin:$PATH" \ + "$TEARDOWN" task-x1 "$@" +} + +test_exited_pr_open_closes_pane_keeps_copy() { + local case_dir rc + case_dir=$(make_case exited-pr-open) + open_shell_window "$case_dir/wt" + wait_for_state "$SESSION:$WNAME" dead + write_ship_meta "$case_dir" \ + 'pr=https://github.com/example/repo/pull/7' + git -C "$case_dir/wt" -c user.email=t@t -c user.name=t \ + commit -q --allow-empty -m "task work" + : > "$case_dir/treehouse.log" + + set +e + run_teardown "$case_dir" --close-pane > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + [ "$rc" -eq 0 ] || fail "close-pane refused an exited PR-open worker: $(cat "$case_dir/stderr")" + window_exists && fail "exited pane was still open after --close-pane" + [ -d "$case_dir/wt" ] || fail "close-pane discarded the isolated copy" + [ -f "$case_dir/state/task-x1.meta" ] || fail "close-pane removed task records" + grep -qx 'pane_closed=1' "$case_dir/state/task-x1.meta" \ + || fail "close-pane did not record pane_closed=1" + [ ! -s "$case_dir/treehouse.log" ] \ + || fail "close-pane returned the copy: $(cat "$case_dir/treehouse.log")" + pass "exited worker with PR still open: pane closed, copy retained" +} + +# make_herdr_case: a project/worktree fixture like make_case, but with no +# tmux window - the herdr wrapped-input coverage below drives backend=herdr +# through a canned fake `herdr` CLI instead of a real tmux pane. +make_herdr_case() { # -> echoes case dir + local name=$1 case_dir + case_dir="$TMP_ROOT/$name" + mkdir -p "$case_dir/state" "$case_dir/config" "$case_dir/data" "$case_dir/fakebin" "$case_dir/responses" + git init -q --bare "$case_dir/origin.git" + git -C "$case_dir/origin.git" symbolic-ref HEAD refs/heads/main + git clone -q "$case_dir/origin.git" "$case_dir/_seed" 2>/dev/null + git -C "$case_dir/_seed" -c user.email=t@t -c user.name=t \ + commit -q --allow-empty -m "origin baseline" + git -C "$case_dir/_seed" push -q origin main + rm -rf "$case_dir/_seed" + git clone -q "$case_dir/origin.git" "$case_dir/project" + git -C "$case_dir/project" remote set-head origin main 2>/dev/null || true + git -C "$case_dir/project" worktree add -q -b "fm/$name" "$case_dir/wt" main + touch "$case_dir/state/.last-watcher-beat" + printf '%s\n' "$case_dir" +} + +# make_herdr_fakebin: a `herdr` stub that logs every call (unit-separated, to +# $FM_HERDR_LOG) and answers `status --json` unconditionally, but otherwise +# returns the canned response read from /responses/.out (or +# non-zero from .exit), consumed IN CALL ORDER - same convention as +# tests/fm-backend-herdr.test.sh's make_herdr_fakebin, duplicated here rather +# than shared so this file's tmux-only cases stay untouched by herdr-only +# fixture code. +make_herdr_fakebin() { # + cat > "$1/fakebin/herdr" <<'SH' +#!/usr/bin/env bash +set -u +LOG="${FM_HERDR_LOG:?}" +RESP="${FM_HERDR_RESPONSES:?}" +COUNT_FILE="$RESP/.count" +{ + printf 'HERDR_SESSION=%s' "${HERDR_SESSION:-}" + for a in "$@"; do printf '\x1f%s' "$a"; done + printf '\n' +} >> "$LOG" +if [ "${1:-}" = status ] && [ "${2:-}" = --json ]; then + printf '{"client":{"version":"0.7.1","protocol":14},"server":{"running":true}}\n' + exit 0 +fi +n=$(( $(cat "$COUNT_FILE" 2>/dev/null || echo 0) + 1 )) +echo "$n" > "$COUNT_FILE" +if [ -f "$RESP/$n.exit" ]; then + exit "$(cat "$RESP/$n.exit")" +fi +[ -f "$RESP/$n.out" ] && cat "$RESP/$n.out" +exit 0 +SH + chmod +x "$1/fakebin/herdr" +} + +run_herdr_teardown() { # [args...] + local case_dir=$1 + shift + FM_TREEHOUSE_LOG="$case_dir/treehouse.log" \ + FM_ROOT_OVERRIDE="$ROOT" \ + FM_HOME="$case_dir" \ + FM_STATE_OVERRIDE="$case_dir/state" \ + FM_DATA_OVERRIDE="$case_dir/data" \ + FM_CONFIG_OVERRIDE="$case_dir/config" \ + FM_HERDR_LOG="$case_dir/herdr.log" \ + FM_HERDR_RESPONSES="$case_dir/responses" \ + PATH="$case_dir/fakebin:$PATH" \ + "$TEARDOWN" task-x1 "$@" +} + +# The Herdr adapter has no logical-line-join primitive (unlike tmux's +# `capture-pane -J`), so fm_backend_capture_joined falls back to its +# row-oriented plain capture for it. A long unsubmitted command that +# soft-wraps still splits across physical rows there, and a check that only +# proves the LAST physical row is a bare trailing prompt glyph would wrongly +# call that an empty composer - the same class of bug already fixed for tmux, +# left open on Herdr (Greptile P1: task fm-close-exited-panes review, PR +# #2970 - "Herdr reaches the same close logic through an unchanged +# row-oriented capture"). fm_backend_capture_joined_reliable (bin/fm-backend.sh) +# closes this by refusing to trust that fallback proof at all on Herdr: an +# `unknown` composer verdict blocks the close outright, the same as a +# genuine capture failure. +test_herdr_unknown_composer_keeps_pane() { + local case_dir rc pane=w1:p2 target + command -v jq >/dev/null 2>&1 || { echo "skip: jq not found (required by the herdr adapter)"; return; } + case_dir=$(make_herdr_case herdr-wrapped-glyph) + make_herdr_fakebin "$case_dir" + target="default:$pane" + + # Call 1: fm_backend_herdr_pane_agent_state's presence probe ("pane get"). + printf '{"result":{"pane":{"pane_id":"%s"}}}\n' "$pane" > "$case_dir/responses/1.out" + # Call 2: the agent probe on that pane - no registered agent, so the + # worker reads confirmed-exited (agent_state "dead"), same as a real + # crewmate process that already quit. + printf '{"error":{"code":"agent_not_found","message":"agent target %s not found"}}\n' "$pane" \ + > "$case_dir/responses/2.out" + # Call 3: the composer's ANSI pane read. A bare trailing prompt glyph with + # nothing else in view is exactly the shape a real idle shell PS1 draws + # AND the shape the last physical row of a wrapped unsubmitted command + # ending in that glyph would also show - fm-composer-lib.sh cannot tell + # them apart from one physical row alone, which is why this must read + # `unknown` rather than `empty` (mirrors + # tests/fm-backend-herdr.test.sh:test_composer_state_unknown_when_no_composer_row_found). + printf '> \n' > "$case_dir/responses/3.out" + # Call 4: the fallback trailing-glyph-only proof's plain (row-oriented) + # capture - only reached, and only trusted, pre-fix. It models a real + # prompt row followed by a long unsubmitted command that soft-wrapped onto + # a second physical row ending in a lone '>' - without the reliable-join + # gate, inspecting just that last physical row would wrongly read as a + # bare trailing glyph with nothing after it and "prove" the composer + # empty, discarding the pending command on close. + printf 'user@host:~$ echo hello world\nxxxxxxxxxxxxxxxxxxxx>\n' > "$case_dir/responses/4.out" + + fm_write_meta "$case_dir/state/task-x1.meta" \ + "window=$target" \ + "endpoint_task_id=task-x1" \ + "worktree=$case_dir/wt" \ + "project=$case_dir/project" \ + "kind=ship" \ + "mode=no-mistakes" \ + "backend=herdr" \ + "herdr_session=default" \ + "herdr_workspace_id=ws1" \ + "herdr_tab_id=t1" \ + "herdr_pane_id=$pane" \ + 'pr=https://github.com/example/repo/pull/7' + git -C "$case_dir/wt" -c user.email=t@t -c user.name=t \ + commit -q --allow-empty -m "task work" + : > "$case_dir/treehouse.log" + + set +e + run_herdr_teardown "$case_dir" --close-pane > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + [ "$rc" -ne 0 ] \ + || fail "close-pane closed a herdr pane whose composer could not be proven empty: $(cat "$case_dir/stdout")" + grep -q 'pending composer' "$case_dir/stderr" \ + || fail "herdr unknown-composer refusal did not name pending composer text: $(cat "$case_dir/stderr")" + grep -q 'pane_closed=1' "$case_dir/state/task-x1.meta" \ + && fail "herdr unknown-composer close-pane recorded pane_closed" + [ ! -s "$case_dir/treehouse.log" ] \ + || fail "close-pane returned the copy for a herdr unknown-composer refusal: $(cat "$case_dir/treehouse.log")" + pass "herdr backend, composer state unprovable: pane kept" +} + +test_live_agent_keeps_pane() { + local case_dir rc + case_dir=$(make_case live-agent) + cat > "$case_dir/claude" <<'SH' +#!/bin/bash +read -r _ +SH + chmod +x "$case_dir/claude" + open_agent_window "$case_dir/wt" "$case_dir/claude" + wait_for_state "$SESSION:$WNAME" alive + write_ship_meta "$case_dir" \ + 'pr=https://github.com/example/repo/pull/7' + : > "$case_dir/treehouse.log" + + set +e + run_teardown "$case_dir" --close-pane > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + [ "$rc" -ne 0 ] || fail "close-pane closed a live agent" + grep -q 'live agent' "$case_dir/stderr" \ + || fail "live-agent refusal did not name the live agent: $(cat "$case_dir/stderr")" + window_exists || fail "live agent pane was closed" + [ -d "$case_dir/wt" ] || fail "live-agent close-pane discarded the copy" + grep -q 'pane_closed=1' "$case_dir/state/task-x1.meta" \ + && fail "live-agent close-pane recorded pane_closed" + kill_task_window + pass "live agent: pane kept" +} + +test_unsubmitted_typed_text_keeps_pane() { + local case_dir rc + case_dir=$(make_case unsubmitted-text) + open_shell_window "$case_dir/wt" + wait_for_state "$SESSION:$WNAME" dead + "$REAL_TMUX" -L "$SOCKET" send-keys -t "$SESSION:$WNAME" -l 'rm -rf /tmp/not-yet-submitted' + write_ship_meta "$case_dir" \ + 'pr=https://github.com/example/repo/pull/7' + git -C "$case_dir/wt" -c user.email=t@t -c user.name=t \ + commit -q --allow-empty -m "task work" + : > "$case_dir/treehouse.log" + + set +e + run_teardown "$case_dir" --close-pane > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + [ "$rc" -ne 0 ] \ + || fail "close-pane closed a pane with unsubmitted typed text: $(cat "$case_dir/stdout")" + grep -q 'pending composer' "$case_dir/stderr" \ + || fail "unsubmitted-text refusal did not name pending composer text: $(cat "$case_dir/stderr")" + window_exists || fail "pane with unsubmitted typed text was closed" + [ ! -s "$case_dir/treehouse.log" ] \ + || fail "close-pane returned the copy for an unsubmitted-text refusal: $(cat "$case_dir/treehouse.log")" + kill_task_window + pass "unsubmitted typed text in the shell: pane kept" +} + +test_wrapped_unsubmitted_glyph_keeps_pane() { + local case_dir rc target cursor_x width remaining pad_len text + case_dir=$(make_case wrapped-glyph) + open_shell_window "$case_dir/wt" + wait_for_state "$SESSION:$WNAME" dead + target="$SESSION:$WNAME" + + # Build one long unsubmitted token that soft-wraps across several + # terminal rows and ends in a single trailing shell-glyph character + # ('>'), landing that glyph on the LAST physical row together with 9 + # other non-glyph characters - so that row alone reads as an + # ordinary "text then one trailing glyph" line, same as a real idle + # prompt, while the real prompt's own glyph lives on an earlier + # physical row invisible to a last-row-only check (task + # fm-close-exited-panes review, Greptile P1: bin/fm-composer-lib.sh:503). + cursor_x=$("$REAL_TMUX" -L "$SOCKET" display-message -p -t "$target" '#{cursor_x}') + width=$("$REAL_TMUX" -L "$SOCKET" display-message -p -t "$target" '#{pane_width}') + remaining=$((width - cursor_x)) + pad_len=$((remaining + width * 2 + 10)) + text=$(printf 'x%.0s' $(seq 1 $((pad_len - 1)))) + text="${text}>" + "$REAL_TMUX" -L "$SOCKET" send-keys -t "$target" -l "$text" + + write_ship_meta "$case_dir" \ + 'pr=https://github.com/example/repo/pull/7' + git -C "$case_dir/wt" -c user.email=t@t -c user.name=t \ + commit -q --allow-empty -m "task work" + : > "$case_dir/treehouse.log" + + set +e + run_teardown "$case_dir" --close-pane > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + [ "$rc" -ne 0 ] \ + || fail "close-pane closed a pane with wrapped unsubmitted text: $(cat "$case_dir/stdout")" + grep -q 'pending composer' "$case_dir/stderr" \ + || fail "wrapped-text refusal did not name pending composer text: $(cat "$case_dir/stderr")" + window_exists || fail "pane with wrapped unsubmitted text was closed" + [ ! -s "$case_dir/treehouse.log" ] \ + || fail "close-pane returned the copy for a wrapped-text refusal: $(cat "$case_dir/treehouse.log")" + kill_task_window + pass "wrapped unsubmitted text ending in a shell glyph: pane kept" +} + +test_unfinished_exit_keeps_pane() { + local case_dir rc + case_dir=$(make_case unfinished) + open_shell_window "$case_dir/wt" + wait_for_state "$SESSION:$WNAME" dead + write_ship_meta "$case_dir" + + set +e + run_teardown "$case_dir" --close-pane > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + [ "$rc" -ne 0 ] || fail "close-pane closed an unfinished exit husk" + window_exists || fail "unfinished exit pane was closed" + kill_task_window + pass "unfinished exit: pane kept for recovery" +} + +test_teardown_after_close_still_returns_copy() { + local case_dir rc + case_dir=$(make_case teardown-after-close) + open_shell_window "$case_dir/wt" + wait_for_state "$SESSION:$WNAME" dead + write_ship_meta "$case_dir" \ + 'pr=https://github.com/example/repo/pull/7' + git -C "$case_dir/wt" -c user.email=t@t -c user.name=t \ + commit -q --allow-empty -m "landed work" + git -C "$case_dir/wt" push -q origin "fm/teardown-after-close" + git -C "$case_dir/wt" fetch -q origin + : > "$case_dir/treehouse.log" + + run_teardown "$case_dir" --close-pane > "$case_dir/close.out" 2> "$case_dir/close.err" \ + || fail "close-pane failed before landed teardown: $(cat "$case_dir/close.err")" + [ ! -s "$case_dir/treehouse.log" ] \ + || fail "close-pane returned the copy early: $(cat "$case_dir/treehouse.log")" + [ -d "$case_dir/wt" ] || fail "close-pane discarded the copy before landed teardown" + + set +e + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + [ "$rc" -eq 0 ] || fail "landed teardown after close-pane failed: $(cat "$case_dir/stderr")" + grep -q 'return' "$case_dir/treehouse.log" \ + || fail "landed teardown did not return the copy: $(cat "$case_dir/treehouse.log")" + [ ! -f "$case_dir/state/task-x1.meta" ] \ + || fail "landed teardown left task records" + pass "teardown after merge still returns the copy" +} + +test_recorded_windows_skips_closed_panes() { + local home w + home="$TMP_ROOT/watch-skip" + mkdir -p "$home/state" + fm_write_meta "$home/state/open.meta" "window=$SESSION:open-win" "kind=ship" + fm_write_meta "$home/state/closed.meta" \ + "window=$SESSION:closed-win" "kind=ship" "pane_closed=1" + w=$(STATE="$home/state" FM_STATE_OVERRIDE="$home/state" FM_HOME="$home" \ + bash -c ' + SCRIPT_DIR="$1" + STATE="$2" + # shellcheck source=/dev/null + . "$SCRIPT_DIR/fm-watch.sh" + recorded_windows + ' _ "$ROOT/bin" "$home/state") + case "$w" in + *"$SESSION:open-win"*) ;; + *) fail "recorded_windows dropped the open pane: $w" ;; + esac + case "$w" in + *"$SESSION:closed-win"*) fail "recorded_windows still listed a closed pane: $w" ;; + esac + pass "watcher inventory skips pane_closed endpoints" +} + +test_exited_pr_open_closes_pane_keeps_copy +test_unsubmitted_typed_text_keeps_pane +test_wrapped_unsubmitted_glyph_keeps_pane +test_herdr_unknown_composer_keeps_pane +test_live_agent_keeps_pane +test_unfinished_exit_keeps_pane +test_teardown_after_close_still_returns_copy +test_recorded_windows_skips_closed_panes diff --git a/tests/fm-watch-close-exited-panes-optin.test.sh b/tests/fm-watch-close-exited-panes-optin.test.sh new file mode 100755 index 00000000000..b681482f089 --- /dev/null +++ b/tests/fm-watch-close-exited-panes-optin.test.sh @@ -0,0 +1,72 @@ +#!/usr/bin/env bash +# tests/fm-watch-close-exited-panes-optin.test.sh - the watcher's automatic +# exited-pane retirement (bin/fm-watch.sh: close_finished_exited_pane) is +# gated behind an explicit captain opt-in (VISION.md: "autonomy exists only +# as an explicit grant, never as a default"), so upgrading the watcher never +# starts closing panes on its own. close_exited_panes_enabled() is the real, +# unmodified gate function; these tests call it directly (bin/fm-watch.sh is +# safe to source for unit tests - see its own header note). +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +TMP_ROOT=$(fm_test_tmproot fm-watch-close-exited-panes-optin) + +gate_result() { # [env-assignment...] -> "0" (enabled) or "1" (disabled) + local home=$1 + shift + # shellcheck disable=SC2016 # inner bash -c body expands via its own positional params, not this shell + env "$@" FM_STATE_OVERRIDE="$home/state" FM_HOME="$home" bash -c ' + SCRIPT_DIR="$1" + STATE="$2" + FM_HOME="$3" + # shellcheck source=/dev/null + . "$SCRIPT_DIR/fm-watch.sh" + close_exited_panes_enabled + printf "%d\n" "$?" + ' _ "$ROOT/bin" "$home/state" "$home" +} + +test_disabled_by_default() { + local home rc + home="$TMP_ROOT/default" + mkdir -p "$home/state" "$home/config" + rc=$(gate_result "$home") + [ "$rc" -eq 1 ] || fail "automatic pane close must be disabled with no config and no env var, got rc=$rc" + pass "close_exited_panes_enabled: disabled by default (no config, no env var)" +} + +test_enabled_by_config_file_presence() { + local home rc + home="$TMP_ROOT/config-present" + mkdir -p "$home/state" "$home/config" + : > "$home/config/close-exited-panes" + rc=$(gate_result "$home") + [ "$rc" -eq 0 ] || fail "an existing config/close-exited-panes must enable automatic close, got rc=$rc" + pass "close_exited_panes_enabled: enabled once config/close-exited-panes exists" +} + +test_enabled_by_env_override() { + local home rc + home="$TMP_ROOT/env-on" + mkdir -p "$home/state" "$home/config" + rc=$(gate_result "$home" FM_CLOSE_EXITED_PANES=1) + [ "$rc" -eq 0 ] || fail "FM_CLOSE_EXITED_PANES=1 must enable automatic close, got rc=$rc" + pass "close_exited_panes_enabled: FM_CLOSE_EXITED_PANES=1 enables it" +} + +test_env_override_forces_off_even_with_config() { + local home rc + home="$TMP_ROOT/env-off" + mkdir -p "$home/state" "$home/config" + : > "$home/config/close-exited-panes" + rc=$(gate_result "$home" FM_CLOSE_EXITED_PANES=0) + [ "$rc" -eq 1 ] || fail "FM_CLOSE_EXITED_PANES=0 must force automatic close off, got rc=$rc" + pass "close_exited_panes_enabled: FM_CLOSE_EXITED_PANES=0 overrides an existing config file" +} + +test_disabled_by_default +test_enabled_by_config_file_presence +test_enabled_by_env_override +test_env_override_forces_off_even_with_config