From b25761bdacc3f113e51fece0c17eb275cbf4ee74 Mon Sep 17 00:00:00 2001 From: Mithrandir System Date: Mon, 24 Aug 2026 12:02:21 -0400 Subject: [PATCH 1/9] feat(bin): close exited ship and scout panes before landing A finished worker that has already quit left an empty pane until merge teardown. Close that pane as ordinary cleanup once the agent is confirmed gone, keep the isolated copy, and skip stale/missing-endpoint recovery until landed teardown returns the copy. --- .../skills/stuck-crewmate-recovery/SKILL.md | 1 + AGENTS.md | 2 + bin/fm-session-start.sh | 5 +- bin/fm-teardown.sh | 228 ++++++++++++++- bin/fm-test-run.sh | 2 +- bin/fm-watch.sh | 47 +++ docs/agent-control.md | 2 +- docs/architecture.md | 1 + docs/configuration.md | 1 + docs/scripts.md | 2 +- tests/fm-session-start.test.sh | 22 ++ tests/fm-teardown-close-pane.test.sh | 272 ++++++++++++++++++ 12 files changed, 574 insertions(+), 11 deletions(-) create mode 100755 tests/fm-teardown-close-pane.test.sh 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..d9fb0cfe53e 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. +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/fm-session-start.sh b/bin/fm-session-start.sh index 9eb50b4263e..137d967f6b3 100755 --- a/bin/fm-session-start.sh +++ b/bin/fm-session-start.sh @@ -811,7 +811,10 @@ for meta in "$STATE"/*.meta; do window=$(fm_meta_get "$meta" window) target=$(fm_backend_target_of_meta "$meta") - if [ -n "$window" ]; then + if [ "$(fm_meta_get "$meta" pane_closed)" = 1 ]; then + backend=$(fm_backend_of_meta "$meta") + printf 'endpoint: closed (backend=%s window=%s)\n' "$backend" "${window:-none}" + elif [ -n "$window" ]; then backend=$(fm_backend_of_meta "$meta") if fm_backend_target_exists "$backend" "${target:-$window}" "fm-$id"; then printf 'endpoint: alive (backend=%s window=%s)\n' "$backend" "$window" diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index 117d3533437..2807ea3131b 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -58,10 +58,22 @@ # releases its durable treehouse lease so the pool slot is freed, # never left leased forever. If the treehouse return fails, teardown leaves the # leased home and state in place instead of hiding a still-held lease. -# Usage: fm-teardown.sh [--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,185 @@ $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 + verdict=$(fm_backend_composer_state "$BACKEND" "$T" "fm-$ID" 2>/dev/null || printf 'unknown') + case "$verdict" in + pending|pending-unproven) return 0 ;; + esac + return 1 +} + +teardown_mark_pane_closed() { + local tmp + tmp=$(mktemp "$META.tmp.XXXXXX") || return 1 + if ! { grep -vE '^pane_closed=' "$META" || true; } > "$tmp"; then + rm -f "$tmp" + return 1 + fi + printf 'pane_closed=1\n' >> "$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 +2730,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..32bbd1a753a 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -15,6 +15,10 @@ # 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. # 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 +372,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 +383,42 @@ recorded_windows() { done } +# 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 +1345,12 @@ EOF while IFS= read -r w; do kind=$(window_kind "$w") task=$(window_to_task "$w" "$STATE") + if [ -n "$task" ] && 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..bfad510fd84 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. 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..d1e0d9b6d40 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -94,6 +94,7 @@ 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. 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-session-start.test.sh b/tests/fm-session-start.test.sh index e74eceb7abf..f6fc9c43285 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..d9c8ac80813 --- /dev/null +++ b/tests/fm-teardown-close-pane.test.sh @@ -0,0 +1,272 @@ +#!/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" +} + +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_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_live_agent_keeps_pane +test_unfinished_exit_keeps_pane +test_teardown_after_close_still_returns_copy +test_recorded_windows_skips_closed_panes From 9b1424ab9a48946a08f6adc7bcd47113f4f94c0d Mon Sep 17 00:00:00 2001 From: Mithrandir System Date: Mon, 24 Aug 2026 12:22:46 -0400 Subject: [PATCH 2/9] fix(document): apply no-mistakes document fixes --- tests/fm-session-start.test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fm-session-start.test.sh b/tests/fm-session-start.test.sh index f6fc9c43285..23645f5f5c5 100755 --- a/tests/fm-session-start.test.sh +++ b/tests/fm-session-start.test.sh @@ -1349,7 +1349,7 @@ $rec EOF make_fake_toolchain "$fakebin" make_fake_ps_claude "$fakebin" - make_fake_tmux "$fakebin" + make_fake_tmux "$fakebin" "fm-sess:gone-window-never-alive" printf 'window=fm-sess:gone-window\nkind=ship\npane_closed=1\n' > "$home/state/task-closed.meta" From 0362b0adcbca82216a116e9207c45a819379f46e Mon Sep 17 00:00:00 2001 From: Mithrandir System Date: Mon, 24 Aug 2026 12:40:33 -0400 Subject: [PATCH 3/9] no-mistakes: apply CI fixes --- AGENTS.md | 2 +- bin/fm-teardown.sh | 10 ++- bin/fm-watch.sh | 21 +++++- docs/architecture.md | 2 +- docs/configuration.md | 1 + .../fm-watch-close-exited-panes-optin.test.sh | 71 +++++++++++++++++++ 6 files changed, 102 insertions(+), 5 deletions(-) create mode 100755 tests/fm-watch-close-exited-panes-optin.test.sh diff --git a/AGENTS.md b/AGENTS.md index d9fb0cfe53e..eb3c3d846e4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -363,7 +363,7 @@ 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. +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. diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index 2807ea3131b..a7db73fe159 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -2444,7 +2444,15 @@ teardown_task_finished_for_pane_close() { teardown_composer_blocks_pane_close() { local verdict - verdict=$(fm_backend_composer_state "$BACKEND" "$T" "fm-$ID" 2>/dev/null || printf 'unknown') + # 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. Only a composer-inspection FAILURE (the classifier + # itself erroring, distinct from it successfully reading `unknown`) is + # unproven in a way this gate cannot tell apart from unsent input, so that + # failure - and it alone - blocks the close instead of defaulting to safe. + verdict=$(fm_backend_composer_state "$BACKEND" "$T" "fm-$ID" 2>/dev/null) || return 0 case "$verdict" in pending|pending-unproven) return 0 ;; esac diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index 32bbd1a753a..f805d90c084 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -18,7 +18,11 @@ # 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. +# 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 @@ -383,6 +387,19 @@ 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. @@ -1345,7 +1362,7 @@ EOF while IFS= read -r w; do kind=$(window_kind "$w") task=$(window_to_task "$w" "$STATE") - if [ -n "$task" ] && close_finished_exited_pane "$w" "$task"; then + 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" diff --git a/docs/architecture.md b/docs/architecture.md index bfad510fd84..f36a2462068 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -15,7 +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. +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 d1e0d9b6d40..f897aa79288 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -95,6 +95,7 @@ Only metadata-routed task selectors carry secondmate-marker and Codex-harness co 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/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..f353f8efa96 --- /dev/null +++ b/tests/fm-watch-close-exited-panes-optin.test.sh @@ -0,0 +1,71 @@ +#!/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 + 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 From 838b6c9245f20e5340de8d152427409f2ca97af2 Mon Sep 17 00:00:00 2001 From: Mithrandir System Date: Mon, 24 Aug 2026 13:00:16 -0400 Subject: [PATCH 4/9] no-mistakes: apply CI fixes --- bin/fm-composer-lib.sh | 27 +++++++++++++++++++++ bin/fm-teardown.sh | 26 ++++++++++++++++---- tests/fm-composer-lib.test.sh | 36 ++++++++++++++++++++++++++++ tests/fm-teardown-close-pane.test.sh | 28 ++++++++++++++++++++++ 4 files changed, 112 insertions(+), 5 deletions(-) diff --git a/bin/fm-composer-lib.sh b/bin/fm-composer-lib.sh index 07b3b02fffb..77e6c741c18 100644 --- a/bin/fm-composer-lib.sh +++ b/bin/fm-composer-lib.sh @@ -454,6 +454,33 @@ 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. +fm_composer_trailing_shell_glyph_only() { # + local __fmtg_trimmed=$1 __fmtg_glyph + 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") return 0 ;; + esac + done </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. + cap=$(fm_backend_capture "$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 <'; 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_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" +} + # --- Preserved: shell glyph inside a composer box is the harness prompt ------ test_bordered_shell_glyph_is_empty() { @@ -606,6 +639,9 @@ 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_blank_or_glyphless test_bordered_shell_glyph_is_empty test_agent_glyphs_are_empty_bordered_and_bare test_empty_content_is_empty diff --git a/tests/fm-teardown-close-pane.test.sh b/tests/fm-teardown-close-pane.test.sh index d9c8ac80813..16ca0b79ddf 100755 --- a/tests/fm-teardown-close-pane.test.sh +++ b/tests/fm-teardown-close-pane.test.sh @@ -192,6 +192,33 @@ SH 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_unfinished_exit_keeps_pane() { local case_dir rc case_dir=$(make_case unfinished) @@ -266,6 +293,7 @@ test_recorded_windows_skips_closed_panes() { } test_exited_pr_open_closes_pane_keeps_copy +test_unsubmitted_typed_text_keeps_pane test_live_agent_keeps_pane test_unfinished_exit_keeps_pane test_teardown_after_close_still_returns_copy From 1e409a59846bf9af5b048c3fd7f642cfb3392b81 Mon Sep 17 00:00:00 2001 From: Mithrandir System Date: Mon, 24 Aug 2026 13:10:10 -0400 Subject: [PATCH 5/9] no-mistakes: apply CI fixes --- bin/fm-composer-lib.sh | 19 ++++++++++++++++--- tests/fm-composer-lib.test.sh | 15 +++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/bin/fm-composer-lib.sh b/bin/fm-composer-lib.sh index 77e6c741c18..1750ee103da 100644 --- a/bin/fm-composer-lib.sh +++ b/bin/fm-composer-lib.sh @@ -466,19 +466,32 @@ EOF # 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. fm_composer_trailing_shell_glyph_only() { # - local __fmtg_trimmed=$1 __fmtg_glyph + 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") return 0 ;; + *"$__fmtg_glyph") __fmtg_matched=$__fmtg_glyph; break ;; esac done <`, 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 @@ -641,6 +655,7 @@ 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_bordered_shell_glyph_is_empty test_agent_glyphs_are_empty_bordered_and_bare From 18910638bf6a6426ef1498623ff6369217369d4d Mon Sep 17 00:00:00 2001 From: Mithrandir System Date: Mon, 24 Aug 2026 13:24:50 -0400 Subject: [PATCH 6/9] no-mistakes: apply CI fixes --- bin/fm-composer-lib.sh | 9 +++++++++ tests/fm-composer-lib.test.sh | 17 +++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/bin/fm-composer-lib.sh b/bin/fm-composer-lib.sh index 1750ee103da..a31d174d3d7 100644 --- a/bin/fm-composer-lib.sh +++ b/bin/fm-composer-lib.sh @@ -476,6 +476,14 @@ EOF # 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 @@ -489,6 +497,7 @@ fm_composer_trailing_shell_glyph_only() { # $FM_COMPOSER_SHELL_PROMPT_GLYPHS EOF [ -n "$__fmtg_matched" ] || return 1 + [ "$__fmtg_matched" = '>' ] && [ "$__fmtg_trimmed" = '>' ] && return 1 __fmtg_class=$(printf '%s' "$FM_COMPOSER_SHELL_PROMPT_GLYPHS" | tr -d '\n') __fmtg_count=$(printf '%s' "$__fmtg_trimmed" | tr -dc "$__fmtg_class" | wc -c) [ "$__fmtg_count" -eq 1 ] diff --git a/tests/fm-composer-lib.test.sh b/tests/fm-composer-lib.test.sh index f7eb4af2345..b89163bbcd5 100755 --- a/tests/fm-composer-lib.test.sh +++ b/tests/fm-composer-lib.test.sh @@ -109,6 +109,22 @@ test_trailing_shell_glyph_only_rejects_blank_or_glyphless() { 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() { @@ -657,6 +673,7 @@ 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 From 1bfa191212c097ba3097179ffd5d699c83c0b6b0 Mon Sep 17 00:00:00 2001 From: Mithrandir System Date: Mon, 24 Aug 2026 13:40:09 -0400 Subject: [PATCH 7/9] no-mistakes: apply CI fixes --- bin/backends/tmux.sh | 15 ++++++++++ bin/fm-backend.sh | 19 ++++++++++++ bin/fm-teardown.sh | 7 ++++- tests/fm-teardown-close-pane.test.sh | 45 ++++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 1 deletion(-) 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..df3faa24177 100644 --- a/bin/fm-backend.sh +++ b/bin/fm-backend.sh @@ -706,6 +706,25 @@ 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_send_key: one backend-supported named special key. fm_backend_send_key() { # [expected-label] local backend=$1 diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index 9264a564066..e5624768b9c 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -2464,7 +2464,12 @@ teardown_composer_blocks_pane_close() { # 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. - cap=$(fm_backend_capture "$BACKEND" "$T" "${FM_COMPOSER_CAPTURE_LINES:-20}" 2>/dev/null) || return 0 + # 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). + 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 diff --git a/tests/fm-teardown-close-pane.test.sh b/tests/fm-teardown-close-pane.test.sh index 16ca0b79ddf..e1957c04705 100755 --- a/tests/fm-teardown-close-pane.test.sh +++ b/tests/fm-teardown-close-pane.test.sh @@ -219,6 +219,50 @@ test_unsubmitted_typed_text_keeps_pane() { 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) @@ -294,6 +338,7 @@ test_recorded_windows_skips_closed_panes() { test_exited_pr_open_closes_pane_keeps_copy test_unsubmitted_typed_text_keeps_pane +test_wrapped_unsubmitted_glyph_keeps_pane test_live_agent_keeps_pane test_unfinished_exit_keeps_pane test_teardown_after_close_still_returns_copy From a4063f5f6804d22facbd1f700d840a61d5c5392a Mon Sep 17 00:00:00 2001 From: Mithrandir System Date: Mon, 24 Aug 2026 14:25:26 -0400 Subject: [PATCH 8/9] no-mistakes: apply CI fixes --- bin/fm-backend.sh | 19 ++++ bin/fm-teardown.sh | 9 +- tests/fm-teardown-close-pane.test.sh | 146 +++++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 1 deletion(-) diff --git a/bin/fm-backend.sh b/bin/fm-backend.sh index df3faa24177..ebbe7d91fd5 100644 --- a/bin/fm-backend.sh +++ b/bin/fm-backend.sh @@ -725,6 +725,25 @@ fm_backend_capture_joined() { # [expected-label] 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-teardown.sh b/bin/fm-teardown.sh index e5624768b9c..1f3db334496 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -2468,7 +2468,14 @@ teardown_composer_blocks_pane_close() { # 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). + # (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 diff --git a/tests/fm-teardown-close-pane.test.sh b/tests/fm-teardown-close-pane.test.sh index e1957c04705..e62d6145b5f 100755 --- a/tests/fm-teardown-close-pane.test.sh +++ b/tests/fm-teardown-close-pane.test.sh @@ -163,6 +163,151 @@ test_exited_pr_open_closes_pane_keeps_copy() { 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) @@ -339,6 +484,7 @@ test_recorded_windows_skips_closed_panes() { 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 From 020e3ed2e5de4257f60a552eeb6fa0a5c1f760f2 Mon Sep 17 00:00:00 2001 From: Mithrandir System Date: Mon, 24 Aug 2026 14:59:32 -0400 Subject: [PATCH 9/9] no-mistakes: apply CI fixes --- tests/fm-watch-close-exited-panes-optin.test.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/fm-watch-close-exited-panes-optin.test.sh b/tests/fm-watch-close-exited-panes-optin.test.sh index f353f8efa96..b681482f089 100755 --- a/tests/fm-watch-close-exited-panes-optin.test.sh +++ b/tests/fm-watch-close-exited-panes-optin.test.sh @@ -16,6 +16,7 @@ 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"