diff --git a/bin/backends/playbot.sh b/bin/backends/playbot.sh index 0698b3582e..58e78677d9 100644 --- a/bin/backends/playbot.sh +++ b/bin/backends/playbot.sh @@ -370,6 +370,13 @@ fm_backend_playbot_endpoint_confirmed_gone() { # [ "$state" = missing ] } +fm_backend_playbot_teardown_remove_worktree() { # + local workspace_id=${1:-} pre_removal_check=${2:-} + [ -n "$workspace_id" ] && [ -n "$pre_removal_check" ] || return 2 + "$pre_removal_check" || return 2 + fm_backend_playbot_remove_worktree "$workspace_id" >/dev/null 2>&1 +} + # fm_backend_playbot_teardown: teardown-authority endpoint retirement (plan # section 3.7), printing exactly one proof token: retired | retained: | # refuse:. Turn stop, thread archive, and workspace removal are all @@ -380,9 +387,11 @@ fm_backend_playbot_endpoint_confirmed_gone() { # # fm_backend_playbot_endpoint_confirmed_gone before retiring records with an # orphan/retention receipt. The adapter never touches meta, routes, outboxes, # or txn records itself. -fm_backend_playbot_teardown() { # -> proof +fm_backend_playbot_teardown() { # -> proof local meta=${1:-} id=${2:-} target=${3:-} worktree=${4:-} workspace_id=${5:-} thread_id=${6:-} - [ -n "$meta" ] && [ -n "$id" ] && [ -n "$target" ] && [ -n "$workspace_id" ] && [ -n "$thread_id" ] || { + local pre_removal_check=${7:-} remove_rc + [ -n "$meta" ] && [ -n "$id" ] && [ -n "$target" ] && [ -n "$workspace_id" ] \ + && [ -n "$thread_id" ] && [ -n "$pre_removal_check" ] || { printf 'refuse:incomplete-task-identity' return 1 } @@ -392,9 +401,15 @@ fm_backend_playbot_teardown() { # /dev/null) || state=unreadable case "$state" in missing) - if fm_backend_playbot_remove_worktree "$workspace_id" >/dev/null 2>&1; then + if fm_backend_playbot_teardown_remove_worktree "$workspace_id" "$pre_removal_check"; then printf 'retired' return 0 + else + remove_rc=$? + fi + if [ "$remove_rc" -eq 2 ]; then + printf 'refuse:worktree-safety-recheck-failed' + return 1 fi printf 'retained:workspace-removal-failed-after-thread-gone' return 0 @@ -409,12 +424,19 @@ fm_backend_playbot_teardown() { # /dev/null 2>&1; then + if fm_backend_playbot_teardown_remove_worktree "$workspace_id" "$pre_removal_check"; then + printf 'retired' + return 0 + else + remove_rc=$? + fi + if [ "$remove_rc" -eq 2 ]; then + printf 'refuse:worktree-safety-recheck-failed' + return 1 + else printf 'retained:thread-archived-workspace-removal-failed' return 0 fi - printf 'retired' - return 0 ;; *) echo "error: playbot teardown refuses unreadable endpoint playbot:$thread_id ($state)" >&2 diff --git a/bin/fm-pr-check.sh b/bin/fm-pr-check.sh index 96cb14dc93..1cec29deba 100755 --- a/bin/fm-pr-check.sh +++ b/bin/fm-pr-check.sh @@ -67,10 +67,9 @@ fi # pr_head is recorded only when the forge's CLI can supply it. gh exposes the # head commit as a selectable field; plain glab exposes it only inside its JSON # output, which would need a JSON processor firstmate does not require, so a -# GitLab task records no pr_head. Both consumers already treat it as optional: -# bin/fm-teardown.sh reads the head from the forge at teardown rather than from -# metadata and falls back to its provider-agnostic content check, and -# bin/fm-review-diff.sh resolves the head from the remote when none is recorded. +# GitLab task records no pr_head. Both consumers treat it as optional. +# bin/fm-teardown.sh's header owns the landed-work proof when no head is +# available; bin/fm-review-diff.sh resolves the head from the remote. WT=$(grep '^worktree=' "$META" | tail -1 | cut -d= -f2- || true) PR_HEAD= if [ "$PROVIDER" = github ] && [ -n "$WT" ] && [ -d "$WT" ] && command -v gh >/dev/null 2>&1; then diff --git a/bin/fm-supervise-daemon.sh b/bin/fm-supervise-daemon.sh index b104d207ef..4420136a96 100755 --- a/bin/fm-supervise-daemon.sh +++ b/bin/fm-supervise-daemon.sh @@ -93,6 +93,8 @@ # re-surfaces as a recheck (default 3600) # FM_ESCALATE_BATCH_SECS buffer window for batched escalation # digests; 0 = flush immediately (default 90) +# FM_REFILL_COVERED_SECS seconds a capacity-freeing captain escalate +# covers a same-window refill (default 120) # FM_HEARTBEAT_SCAN_SECS cadence for the catch-all status scan # (default 300) # FM_HOUSEKEEPING_TICK seconds between housekeeping passes while @@ -423,7 +425,23 @@ classify_heartbeat() { # Advisory fleet refill: firstmate (or the away-mode primary injection) must # re-evaluate ready work against free capacity. The daemon never selects or # spawns from this wake itself. -classify_refill() { +# A capacity-freeing captain status already forces capacity re-evaluation. Its +# same-window refill wake must not produce a second injection. Coverage is +# one-shot and bounded so a later refill-only wake still escalates. +REFILL_COVERED_SECS_DEFAULT=120 +classify_refill() { # [state] + local state=${1:-} age covered + if [ -n "$state" ]; then + covered="$state/.subsuper-refill-covered" + if [ -f "$covered" ]; then + age=$(_file_age "$covered") + rm -f "$covered" + if [ "$age" -lt "${FM_REFILL_COVERED_SECS:-$REFILL_COVERED_SECS_DEFAULT}" ]; then + printf 'self|refill covered by capacity-freeing captain escalate' + return + fi + fi + fi printf 'escalate|refill: re-evaluate ready work against free capacity' } @@ -530,16 +548,19 @@ sync_pause_markers_from_signal() { # # heartbeat catch-all scan does not re-fire it. The single source of truth for # the .subsuper-seen-status- dedup state: called from both the per-wake # escalate path and the catch-all scan. -mark_status_seen() { # - local state=$1 task=$2 line=$3 +mark_status_seen() { # [arm-refill-coverage] + local state=$1 task=$2 line=$3 arm_refill_coverage=${4:-1} printf '%s' "$line" > "$state/.subsuper-seen-status-$(_stale_key "$task")" + if [ "$arm_refill_coverage" = 1 ] && status_frees_capacity "$line"; then + _now > "$state/.subsuper-refill-covered" + fi } # Mark every captain-relevant status line a per-wake classification escalated as # seen, so the catch-all scan does not re-escalate the same line within # HEARTBEAT_SCAN_SECS. Mirrors classify_signal/classify_stale's relevance test. -mark_escalated_seen() { # - local kind=$1 arg=$2 state=$3 f last task +mark_escalated_seen() { # [arm-refill-coverage] + local kind=$1 arg=$2 state=$3 arm_refill_coverage=${4:-1} f last task case "$kind" in signal) for f in $arg; do @@ -548,13 +569,13 @@ mark_escalated_seen() { # [ -n "$last" ] || continue status_is_captain_relevant "$last" || continue task=$(basename "$f"); task="${task%.status}" - mark_status_seen "$state" "$task" "$last" + mark_status_seen "$state" "$task" "$last" "$arm_refill_coverage" done ;; stale) task=$(window_to_task "$arg" "$state") last=$(last_status_line "$state/$task.status") [ -n "$last" ] && status_is_captain_relevant "$last" \ - && mark_status_seen "$state" "$task" "$last" ;; + && mark_status_seen "$state" "$task" "$last" "$arm_refill_coverage" ;; esac } @@ -1208,8 +1229,8 @@ is_wake_reason() { # # --- dispatch one wake reason to self-handle or escalate -------------------- # Side effects: logging, marker records, escalation buffer appends. -handle_wake() { # - local reason=$1 state=$2 decision action distilled task last stale_detail +handle_wake() { # [arm-refill-coverage] + local reason=$1 state=$2 arm_refill_coverage=${3:-1} decision action distilled task last stale_detail local kind="" arg="" if should_force_self "$reason"; then log "wake force-self (FM_INJECT_SKIP): $reason" @@ -1227,7 +1248,7 @@ handle_wake() { # esac ;; check:*) decision=$(classify_check "$reason") ;; heartbeat|heartbeat:*) decision=$(classify_heartbeat) ;; - refill|refill:*) decision=$(classify_refill) ;; + refill|refill:*) decision=$(classify_refill "$state") ;; *) decision=$(classify_unknown "$reason") ;; esac action=${decision%%|*} @@ -1240,7 +1261,7 @@ handle_wake() { # # A terminal-stale escalate must not leave a persistence marker behind, or # housekeeping re-escalates the same pane as a false wedge later. [ "$kind" = "stale" ] && stale_marker_remove "$arg" "$state" - mark_escalated_seen "$kind" "$arg" "$state" + mark_escalated_seen "$kind" "$arg" "$state" "$arm_refill_coverage" [ "${FM_ESCALATE_BATCH_SECS:-$ESCALATE_BATCH_SECS_DEFAULT}" -le 0 ] && { escalate_flush "$state" || true; } ;; pause) @@ -1289,8 +1310,8 @@ handle_wake() { # } handle_durable_wakes() { # - local fallback_reason=$1 state=$2 out err tab epoch sequence kind key payload rest - local handled=0 ack_through ack_generation + local fallback_reason=$1 state=$2 out err tab epoch sequence kind key payload rest f last + local handled=0 ack_through ack_generation arm_refill_coverage=1 out=$(mktemp "$state/.subsuper-wake-drain.XXXXXX") || return 1 err=$(mktemp "$state/.subsuper-wake-drain.XXXXXX") || { rm -f "$out"; return 1; } if ! "$FM_DAEMON_DIR/fm-wake-drain.sh" > "$out" 2> "$err"; then @@ -1300,11 +1321,26 @@ handle_durable_wakes() { # fi tab=$(printf '\t') + while IFS="$tab" read -r epoch sequence kind key payload rest; do + case "$epoch" in ''|*[!0-9]*) continue ;; esac + case "$sequence" in ''|*[!0-9]*) continue ;; esac + [ "$kind" = signal ] || continue + for f in ${payload#signal: }; do + case "$f" in *.status) ;; *) continue ;; esac + [ -e "$f" ] || continue + last=$(last_status_line "$f") + if status_frees_capacity "$last"; then + _now > "$state/.subsuper-refill-covered" + arm_refill_coverage=0 + break 2 + fi + done + done < "$out" while IFS="$tab" read -r epoch sequence kind key payload rest; do case "$epoch" in ''|*[!0-9]*) continue ;; esac case "$sequence" in ''|*[!0-9]*) continue ;; esac case "$kind" in signal|stale|check|heartbeat|refill) ;; *) continue ;; esac - handle_wake "$payload" "$state" + handle_wake "$payload" "$state" "$arm_refill_coverage" handled=$((handled + 1)) done < "$out" [ "$handled" -gt 0 ] || handle_wake "$fallback_reason" "$state" @@ -1456,13 +1492,26 @@ fm_super_main() { # --- shutdown: flush buffered escalations, reap child, release lock ------- local WATCHER_PID="" CUR_TMP="" + stop_watcher_bounded() { # [tenths-of-a-second] + local pid=$1 limit=${2:-50} i=0 + [ -n "$pid" ] || return 0 + fm_pid_alive "$pid" || return 0 + kill -TERM "$pid" 2>/dev/null || true + while [ "$i" -lt "$limit" ] && fm_pid_alive "$pid"; do + sleep 0.1 + i=$((i + 1)) + done + if fm_pid_alive "$pid"; then + kill -KILL "$pid" 2>/dev/null || true + fi + wait "$pid" 2>/dev/null || true + } cleanup() { trap - TERM INT wedge_alarm_stop_active_notifier escalate_flush "$STATE" 2>/dev/null || true if [ -n "${WATCHER_PID:-}" ]; then - kill "$WATCHER_PID" 2>/dev/null || true - wait "$WATCHER_PID" 2>/dev/null || true + stop_watcher_bounded "$WATCHER_PID" fi if [ -n "${CUR_TMP:-}" ]; then rm -f "$CUR_TMP" 2>/dev/null || true diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index c67fd44e3a..64c68db5f9 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -5,26 +5,24 @@ # tasks, then print a backlog-refresh reminder for ship and scout teardowns # (a secondmate teardown prints none, since secondmates are not backlog items). # REFUSES if the worktree holds work that has not LANDED, because cleanup -# hard-resets/removes the worktree and kills its processes. Work has landed when it is -# reachable from any remote-tracking branch (a fork counts as a remote, so -# upstream-contribution PRs pushed to a fork satisfy this in any mode), OR - for a -# normal ship task whose commits are not so reachable - when its PR is merged and -# GitHub reports a PR head that contains the current local work, or its content is -# already present in the up-to-date default branch. This recognizes the common -# squash-merge-then-delete-branch flow, where the branch's own commits live nowhere -# on a remote yet the change is fully in main. -# The PR itself is resolved from the task's recorded pr= when present, or - when -# no pr= was ever recorded (e.g. a yolo-authorized merge on a repo with no PR CI, -# where the usual "checks green" fm-pr-check.sh trigger never fires) - by looking -# up a merged PR whose head branch matches the worktree's branch, fetching its head -# via refs/pull//head when the branch itself was deleted. So a missing pr= never -# by itself causes a false refusal of landed work. -# A gh lookup error falls back to the content check; if that is also inconclusive, -# teardown refuses rather than risk discarding unlanded work. -# Uncommitted changes are never landed. -# local-only projects additionally accept work merged into the local default -# branch (firstmate performs that merge after configured approval) as a fallback -# for the common case where there is no remote at all. +# hard-resets/removes the worktree and kills its processes. Default-deny for ship +# tasks: teardown proceeds only on positive land proof against a live default tip, +# never because a feature branch is merely reachable from a remote. Positive land is +# (A) current HEAD is an ancestor of the live default tip D, (T) tree equality of +# HEAD vs D when no pr= is recorded or the forge reports MERGED (squash), or (M) +# forge MERGED with current HEAD contained in the PR head. Live D is fetched from +# origin (else the sole remote); after fetch the remote default branch name and OID +# must still match the fetched snapshot (same-branch force-push drift refuses). +# Recorded pr= overlay: OPEN always refuses; CLOSED and unconfirmed accept only A +# (never T); MERGED accepts A/M/T. Missing ordinary-ship worktree refuses (restore +# an inspectable worktree or captain --force); pr_head alone never authorizes. +# The same dirty+land+four-way classify recheck runs after quiescence immediately +# before every ordinary destructive worktree return or removal: each Treehouse +# return attempt (including lock retries) and Playbot workspace deletion after +# endpoint archival or confirmed absence. Uncommitted changes are never landed. +# A missing pr= still discovers a merged PR by branch when possible so +# yolo/no-CI merges are not false-refused. local-only keeps the existing merge-to- +# local-default carveout when there is no remote. # Scout tasks (kind=scout in meta) carve out of that check: their worktree is # declared scratch and the report at data//report.md is the work # product. Teardown proceeds only once the report exists and the shared @@ -695,7 +693,11 @@ playbot_retire_records() { } playbot_teardown_endpoint() { - local proof gone_rc + local pre_workspace_removal_check=${1:-} proof gone_rc + [ -n "$pre_workspace_removal_check" ] || { + echo "error: playbot teardown has no pre-removal safety check; preserving task state" >&2 + return 1 + } fm_backend_source playbot || { echo "error: playbot backend adapter could not be loaded; preserving task state" >&2 return 1 @@ -707,7 +709,7 @@ playbot_teardown_endpoint() { # Adapter: stop current turn, archive exact task-owned thread, optionally # archive/remove workspace. Prints: retired | retained: | refuse: set +e - proof=$(fm_backend_playbot_teardown "$META" "$ID" "$T" "$WT" "$PLAYBOT_WORKSPACE_ID" "$PLAYBOT_THREAD_ID") + proof=$(fm_backend_playbot_teardown "$META" "$ID" "$T" "$WT" "$PLAYBOT_WORKSPACE_ID" "$PLAYBOT_THREAD_ID" "$pre_workspace_removal_check") gone_rc=$? set -e case "$gone_rc:$proof" in @@ -946,63 +948,225 @@ pr_is_merged() { unpushed_patches_are_in_pr_head "$head" } -# Is the branch's content already present in the up-to-date default branch? Fetches -# first, then 3-way merges the default branch with HEAD: when HEAD introduces nothing -# the default branch does not already contain (e.g. its change landed via squash) the -# merged tree equals the default branch's tree. This isolates branch-only changes, so -# unrelated commits the default branch gained past the merge-base do not count as -# "added". Returns non-zero when inconclusive (no default ref, or a merge conflict), -# so the caller refuses rather than guesses. -content_in_default() { - local name ref default_tree merged_tree - name=$(default_branch) || return 1 - if git -C "$WT" remote get-url origin >/dev/null 2>&1; then - git -C "$WT" fetch --quiet origin "+refs/heads/$name:refs/remotes/origin/$name" >/dev/null 2>&1 || return 1 - ref="refs/remotes/origin/$name" - elif git -C "$WT" rev-parse --quiet --verify "refs/heads/$name" >/dev/null 2>&1; then - ref="refs/heads/$name" +# Prefer origin; else the sole remote. Used for live default tip D. +teardown_default_remote() { + local remotes n + [ -n "${PROJ:-}" ] && [ -d "$PROJ" ] || return 1 + if git -C "$PROJ" remote get-url origin >/dev/null 2>&1; then + printf '%s\n' origin + return 0 + fi + remotes=$(git -C "$PROJ" remote 2>/dev/null) || return 1 + n=$(printf '%s\n' "$remotes" | sed '/^$/d' | wc -l | tr -d ' ') + [ "$n" = 1 ] || return 1 + printf '%s\n' "$(printf '%s\n' "$remotes" | sed '/^$/d' | head -1)" +} + +# Fetch live default tip D; after fetch re-read remote default name + OID and +# require both match the fetched snapshot (closes same-branch OID drift). Echo D. +live_default_tip() { + local remote name fetch_ref D live_sym_output live_sym live_name live_oid_output live_oid initial_symref=0 + remote=$(teardown_default_remote) || return 1 + live_sym_output=$(git -C "$PROJ" ls-remote --symref "$remote" HEAD 2>/dev/null) || return 1 + live_sym=$(printf '%s\n' "$live_sym_output" | awk '/^ref:/ { print $2; exit }') + if [ -n "$live_sym" ]; then + initial_symref=1 + name=${live_sym#refs/heads/} + [ "$name" != "$live_sym" ] || return 1 else - return 1 + name=$(default_branch) || return 1 + fi + [ -n "$name" ] || return 1 + fetch_ref="refs/remotes/$remote/$name" + git -C "$PROJ" fetch --quiet "$remote" "+refs/heads/$name:$fetch_ref" >/dev/null 2>&1 || return 1 + D=$(git -C "$PROJ" rev-parse --verify --quiet "$fetch_ref^{commit}" 2>/dev/null) || return 1 + [ -n "$D" ] || return 1 + live_sym_output=$(git -C "$PROJ" ls-remote --symref "$remote" HEAD 2>/dev/null) || return 1 + live_sym=$(printf '%s\n' "$live_sym_output" | awk '/^ref:/ { print $2; exit }') + if [ "$initial_symref" = 1 ]; then + [ -n "$live_sym" ] || return 1 + fi + if [ -n "$live_sym" ]; then + live_name=${live_sym#refs/heads/} + [ "$live_name" = "$name" ] || return 1 + fi + live_oid_output=$(git -C "$PROJ" ls-remote "$remote" "refs/heads/$name" 2>/dev/null) || return 1 + live_oid=$(printf '%s\n' "$live_oid_output" | awk 'NR == 1 { print $1; exit }') + [ -n "$live_oid" ] || return 1 + [ "$live_oid" = "$D" ] || return 1 + # Ensure the worktree object db can resolve D (shared repo for linked worktrees). + if [ -n "${WT:-}" ] && [ -d "$WT" ]; then + git -C "$WT" rev-parse --verify --quiet "$D^{commit}" >/dev/null 2>&1 || return 1 + fi + printf '%s\n' "$D" +} + +# Tree equality of tip vs D (merge-tree). Provenance-blind; caller gates on PR state. +land_tree_eq() { + local D=$1 tip=${2:-HEAD} default_tree merged_tree repo + [ -n "$D" ] || return 1 + if [ -n "${WT:-}" ] && [ -d "$WT" ]; then + repo=$WT + else + repo=$PROJ fi - default_tree=$(git -C "$WT" rev-parse --quiet --verify "$ref^{tree}" 2>/dev/null) || return 1 + [ -n "$repo" ] && [ -d "$repo" ] || return 1 + default_tree=$(git -C "$repo" rev-parse --quiet --verify "$D^{tree}" 2>/dev/null) || return 1 [ -n "$default_tree" ] || return 1 - merged_tree=$(git -C "$WT" merge-tree --write-tree "$ref" HEAD 2>/dev/null) || return 1 + merged_tree=$(git -C "$repo" merge-tree --write-tree "$D" "$tip" 2>/dev/null) || return 1 merged_tree=$(printf '%s\n' "$merged_tree" | head -1) [ "$merged_tree" = "$default_tree" ] } -# Is this task's own recorded PR (pr= in meta) still open? An open PR is -# affirmative proof the task's own work has not landed, even when its content -# already matches the default branch because a sibling PR carried the same -# change first. Returns success only when a pr= is recorded and GitHub reports -# its state as OPEN; returns non-zero for no recorded PR, any other state, or a -# gh error, so the caller keeps its content fallback for those cases. +# content_in_default: tree equality vs live D (with post-fetch OID). Used when T is allowed. +content_in_default() { + local D + [ -d "${WT:-}" ] || return 1 + D=$(live_default_tip) || return 1 + land_tree_eq "$D" HEAD +} + +# Forge classify for recorded pr=. Sets RECORDED_PR_STATE and RECORDED_PR_HEAD. +# Returns: 0 OPEN, 1 MERGED, 3 CLOSED, 2 unconfirmed/malformed/lookup. +RECORDED_PR_STATE= +RECORDED_PR_HEAD= +RECORDED_PR_PROVIDER= +classify_recorded_pr_state() { + local view state head lines raw glab_cwd states state_count forge_cwd + RECORDED_PR_STATE= + RECORDED_PR_HEAD= + RECORDED_PR_PROVIDER= + [ -n "${PR_URL:-}" ] || return 2 + if ! fm_pr_url_parse "$PR_URL"; then + return 2 + fi + RECORDED_PR_PROVIDER=$FM_PR_PROVIDER + if [ -n "${PROJ:-}" ] && [ -d "$PROJ" ]; then + forge_cwd=$PROJ + else + forge_cwd=${WT:-$PWD} + fi + case "$FM_PR_PROVIDER" in + github) + view=$(cd "$forge_cwd" && gh pr view "$PR_URL" --json state,headRefOid -q '.state + "\t" + .headRefOid' 2>/dev/null) || return 2 + [ -n "$view" ] || return 2 + lines=$(printf '%s\n' "$view" | wc -l | tr -d ' ') + [ "$lines" = 1 ] || return 2 + case "$view" in *$'\t'*) ;; *) return 2 ;; esac + state=${view%%$'\t'*} + head=${view#*$'\t'} + [ -n "$state" ] || return 2 + case "$state" in + OPEN) + RECORDED_PR_STATE=OPEN + RECORDED_PR_HEAD=$head + return 0 + ;; + MERGED) + fm_pr_head_valid "$head" || return 2 + RECORDED_PR_STATE=MERGED + RECORDED_PR_HEAD=$head + return 1 + ;; + CLOSED) + if [ -n "$head" ] && ! fm_pr_head_valid "$head"; then + return 2 + fi + RECORDED_PR_STATE=CLOSED + RECORDED_PR_HEAD=$head + return 3 + ;; + *) return 2 ;; + esac + ;; + gitlab) + command -v glab >/dev/null 2>&1 || return 2 + glab_cwd=$forge_cwd + raw=$(cd "$glab_cwd" && glab mr view "$FM_PR_NUMBER" -R "https://$FM_PR_HOST/$FM_PR_PATH" 2>/dev/null) || return 2 + states=$(printf '%s\n' "$raw" | sed -n 's/^state:[[:space:]]*//p' | sed '/^$/d') + state_count=$(printf '%s\n' "$states" | sed '/^$/d' | wc -l | tr -d ' ') + [ "$state_count" = 1 ] || return 2 + state=$states + case "$state" in + open|opened) RECORDED_PR_STATE=OPEN; return 0 ;; + merged) RECORDED_PR_STATE=MERGED; return 1 ;; + closed) RECORDED_PR_STATE=CLOSED; return 3 ;; + *) return 2 ;; + esac + ;; + *) return 2 ;; + esac +} + +# Positive land proof for current WT HEAD against live D + PR overlay (v5 §3). +# Sets WORK_UNLANDED_PR_OPEN=1 when refusing OPEN. +ship_land_proven() { + local branch=$1 D current pr_rc=0 + WORK_UNLANDED_PR_OPEN= + [ -d "${WT:-}" ] || return 1 + current=$(git -C "$WT" rev-parse --verify HEAD 2>/dev/null) || return 1 + + # Four-way PR overlay first when pr= is set so OPEN never reaches T. + if [ -n "${PR_URL:-}" ]; then + classify_recorded_pr_state || pr_rc=$? + if [ "$pr_rc" -eq 0 ]; then + WORK_UNLANDED_PR_OPEN=1 + return 1 + fi + fi + + D=$(live_default_tip) || { + # No live remote D: allow only via pr_is_merged (GitHub M) when readable MERGED. + if [ -n "${PR_URL:-}" ] && [ "${RECORDED_PR_STATE:-}" = MERGED ]; then + pr_is_merged "$branch" && return 0 + fi + pr_is_merged "$branch" && return 0 + return 1 + } + + # A: ancestry + if git -C "$WT" merge-base --is-ancestor "$current" "$D" 2>/dev/null; then + return 0 + fi + + # M: MERGED + containment (GitHub head; GitLab often lacks head oid → T under MERGED) + if [ -n "${PR_URL:-}" ] && [ "${RECORDED_PR_STATE:-}" = MERGED ]; then + if [ -n "${RECORDED_PR_HEAD:-}" ]; then + ensure_commit_object "$PR_URL" "$RECORDED_PR_HEAD" || true + if git -C "$WT" merge-base --is-ancestor "$current" "$RECORDED_PR_HEAD" 2>/dev/null; then + return 0 + fi + unpushed_patches_are_in_pr_head "$RECORDED_PR_HEAD" && return 0 + fi + # FR1: MERGED ⇒ tree equality allowed + land_tree_eq "$D" HEAD && return 0 + return 1 + fi + + # T: only when no pr= (unconfirmed with pr= must not use T — R2-R1) + if [ -z "${PR_URL:-}" ]; then + # Discover merged PR by branch (existing behavior) or tree equality + pr_is_merged "$branch" && return 0 + land_tree_eq "$D" HEAD && return 0 + return 1 + fi + + # CLOSED or unconfirmed: A only (already failed) + return 1 +} + +# Is this task's own recorded PR still open? Back-compat helper. recorded_pr_is_open() { - local state + local pr_rc=0 [ -n "$PR_URL" ] || return 1 - state=$(cd "$WT" && gh pr view "$PR_URL" --json state -q '.state' 2>/dev/null) || return 1 - case "$state" in - OPEN|open) return 0 ;; - *) return 1 ;; - esac + classify_recorded_pr_state || pr_rc=$? + [ "$pr_rc" -eq 0 ] } -# Has the worktree's committed work actually LANDED, though its commits are not -# reachable from any remote-tracking branch? True when a merged PR proves the -# current local work is contained in the PR head. A recorded-but-open PR is -# positive proof the work is NOT landed, so it refuses without consulting the -# provenance-blind content check - otherwise a sibling PR's identical content in -# the default branch would falsely green-light teardown of the still-open task. -# Otherwise falls back to the content check, which covers the no-PR, gh-error, -# and squash-merge-then-delete paths. False only for genuinely unlanded work. +# Has the worktree's committed work LANDED? Default-deny positive proof (v5). work_is_landed() { local branch=$1 - pr_is_merged "$branch" && return 0 - if recorded_pr_is_open; then - WORK_UNLANDED_PR_OPEN=1 - return 1 - fi - content_in_default + ship_land_proven "$branch" } backlog_refresh_reminder() { @@ -1172,16 +1336,22 @@ cleanup_stale_lock_for_safety_check() { # Return a worktree/home via `treehouse return --force`, tolerating a transient or # stale git index.lock left by a killed crew process. See the script header. teardown_treehouse_return() { - local dir=$1 cd_dir=$2 label=$3 post_cleanup_check=${4:-} - local out lock attempt=0 max_retries lock_desc + local dir=$1 cd_dir=$2 label=$3 pre_return_check=${4:-} + local out lock attempt=0 max_retries lock_desc return_rc # Capture stdout+stderr so non-lock failures stay visible and lock failures can # be matched by signature even when the lock file is already gone mid-check. - if out=$( ( cd "$cd_dir" && treehouse return --force "$dir" ) 2>&1 ); then + if out=$(teardown_treehouse_return_attempt "$dir" "$cd_dir" "$pre_return_check" 2>&1); then [ -n "$out" ] && printf '%s\n' "$out" return 0 + else + return_rc=$? fi [ -n "$out" ] && printf '%s\n' "$out" >&2 + if [ "$return_rc" -eq 125 ]; then + echo "teardown: $label return aborted because safety checks failed" >&2 + return 1 + fi if ! treehouse_return_is_index_lock_error "$out"; then return 1 @@ -1202,12 +1372,18 @@ teardown_treehouse_return() { echo "teardown: $label return failed with transient git lock ($lock_desc); waiting ${TREEHOUSE_RETURN_LOCK_RETRY_WAIT_SECS}s and retrying ($attempt/${max_retries})" >&2 sleep "$TREEHOUSE_RETURN_LOCK_RETRY_WAIT_SECS" - if out=$( ( cd "$cd_dir" && treehouse return --force "$dir" ) 2>&1 ); then + if out=$(teardown_treehouse_return_attempt "$dir" "$cd_dir" "$pre_return_check" 2>&1); then [ -n "$out" ] && printf '%s\n' "$out" echo "teardown: $label return succeeded on retry; lock cleared on its own" >&2 return 0 + else + return_rc=$? fi [ -n "$out" ] && printf '%s\n' "$out" >&2 + if [ "$return_rc" -eq 125 ]; then + echo "teardown: $label return aborted on retry because safety checks failed" >&2 + return 1 + fi if ! treehouse_return_is_index_lock_error "$out"; then echo "teardown: $label return failed with a non-lock error after retry; aborting" >&2 @@ -1223,18 +1399,18 @@ teardown_treehouse_return() { if fm_lock_is_provably_stale "$lock" "$dir" "$STALE_WORKTREE_LOCK_AGE_SECS"; then rm -f "$lock" echo "teardown: removed provably-stale git lock $lock (age >= ${STALE_WORKTREE_LOCK_AGE_SECS}s, no live holder) and retrying $label return" >&2 - if [ -n "$post_cleanup_check" ]; then - if ! "$post_cleanup_check"; then - echo "teardown: $label return aborted after stale-lock cleanup because safety checks failed" >&2 - return 1 - fi - fi - if out=$( ( cd "$cd_dir" && treehouse return --force "$dir" ) 2>&1 ); then + if out=$(teardown_treehouse_return_attempt "$dir" "$cd_dir" "$pre_return_check" 2>&1); then [ -n "$out" ] && printf '%s\n' "$out" echo "teardown: $label return succeeded after stale-lock cleanup" >&2 return 0 + else + return_rc=$? fi [ -n "$out" ] && printf '%s\n' "$out" >&2 + if [ "$return_rc" -eq 125 ]; then + echo "teardown: $label return aborted after stale-lock cleanup because safety checks failed" >&2 + return 1 + fi echo "teardown: $label return still failing after stale-lock cleanup" >&2 return 1 fi @@ -1247,14 +1423,28 @@ teardown_treehouse_return() { return 1 } +teardown_treehouse_return_attempt() { + local dir=$1 cd_dir=$2 pre_return_check=${3:-} + if [ -n "$pre_return_check" ] && ! "$pre_return_check"; then + return 125 + fi + ( cd "$cd_dir" && treehouse return --force "$dir" ) +} + validate_worktree_teardown_safety() { local dirty_raw dirty unpushed_raw unpushed DEFAULT unmerged_raw unmerged branch - [ -d "$WT" ] || return 0 [ "$FORCE" != "--force" ] || return 0 case "$KIND" in secondmate|scout) return 0 ;; esac + # Missing ordinary ship worktree: refuse (F4). No pr_head allow path. + if [ ! -d "${WT:-}" ]; then + echo "REFUSED: ordinary ship task $ID has no inspectable worktree at ${WT:-}; landing cannot be positively proven." >&2 + echo "Restore an inspectable worktree and retry, or get the captain's explicit OK to discard, then --force." >&2 + return 1 + fi + if ! dirty_raw=$(git -C "$WT" status --porcelain 2>/dev/null); then if worktree_safety_blocked_by_lock "uncommitted changes"; then return "$TEARDOWN_WORKTREE_SAFETY_LOCK_BLOCKED" @@ -1275,6 +1465,7 @@ validate_worktree_teardown_safety() { fi unpushed=$(printf '%s\n' "$unpushed_raw" | head -5) + # local-only carveout: merge into local default when not on any remote (existing). if [ -n "$unpushed" ] && [ "$MODE" = local-only ]; then DEFAULT=$(default_branch) || { echo "REFUSED: cannot determine default branch for $PROJ; expected origin/HEAD, main, or master." >&2; return 1; } if ! unmerged_raw=$(git -C "$WT" log --oneline HEAD --not "$DEFAULT" -- 2>/dev/null); then @@ -1293,27 +1484,53 @@ validate_worktree_teardown_safety() { echo "Merge the branch into local $DEFAULT first (bin/fm-merge-local.sh after the captain approves), or push to a fork/remote, or get the captain's explicit OK to discard, then --force." >&2 return 1 fi - elif [ -n "$dirty" ]; then + return 0 + fi + + if [ -n "$dirty" ]; then echo "REFUSED: worktree $WT has uncommitted changes." >&2 echo "uncommitted changes present" >&2 echo "Commit them (or get the captain's explicit OK to discard, then --force)." >&2 return 1 - elif [ -n "$unpushed" ]; then - branch=${TEARDOWN_WORKTREE_BRANCH_FOR_SAFETY:-} - if [ -z "$branch" ]; then - branch=$(git -C "$WT" rev-parse --abbrev-ref HEAD 2>/dev/null || echo HEAD) - TEARDOWN_WORKTREE_BRANCH_FOR_SAFETY=$branch - fi - if ! work_is_landed "$branch"; then - echo "REFUSED: worktree $WT has work not on any remote and not landed." >&2 - if [ -n "${WORK_UNLANDED_PR_OPEN:-}" ]; then - echo "PR $PR_URL is still open (not merged); its content may already be in the default branch via a sibling PR, but this task's own PR has not landed." >&2 - fi - printf 'unpushed commits:\n%s\n' "$unpushed" >&2 - echo "Push the branch, land its PR, or get the captain's explicit OK to discard, then --force." >&2 - return 1 - fi fi + + # Always require positive land proof (replace empty-unpushed fall-through ALLOW). + branch=${TEARDOWN_WORKTREE_BRANCH_FOR_SAFETY:-} + if [ -z "$branch" ]; then + branch=$(git -C "$WT" rev-parse --abbrev-ref HEAD 2>/dev/null || echo HEAD) + TEARDOWN_WORKTREE_BRANCH_FOR_SAFETY=$branch + fi + if ship_land_proven "$branch"; then + return 0 + fi + + if [ -n "${WORK_UNLANDED_PR_OPEN:-}" ]; then + echo "REFUSED: worktree $WT belongs to task $ID, whose recorded PR is still open: $PR_URL${RECORDED_PR_PROVIDER:+ ($RECORDED_PR_PROVIDER)}" >&2 + echo "Tearing down now deletes the metadata bin/fm-pr-merge.sh needs, stranding a PR with no guarded path to land it." >&2 + echo "Get the PR merged with bin/fm-pr-merge.sh once it is green and merge is authorized, or close it on the forge, then retry teardown." >&2 + return 1 + fi + if [ -n "$unpushed" ]; then + echo "REFUSED: worktree $WT has work not on any remote and not landed." >&2 + printf 'unpushed commits:\n%s\n' "$unpushed" >&2 + echo "Push the branch, land its PR, or get the captain's explicit OK to discard, then --force." >&2 + else + echo "REFUSED: worktree $WT is reachable from a remote but has no positive land proof on the live default branch or merged PR." >&2 + echo "Land the change (merge/squash into the default branch or merge the PR), or get the captain's explicit OK to discard, then --force." >&2 + fi + return 1 +} + +validate_worktree_teardown_safety_with_lock_recovery() { + local safety_rc + if validate_worktree_teardown_safety; then + return 0 + else + safety_rc=$? + fi + [ "$safety_rc" -eq "$TEARDOWN_WORKTREE_SAFETY_LOCK_BLOCKED" ] || return "$safety_rc" + cleanup_stale_lock_for_safety_check "$WT" || return 1 + validate_worktree_teardown_safety } # Fix 1 (see script header): does the active-or-most-recent no-mistakes run in @@ -2553,18 +2770,13 @@ if [ "$BACKEND" = playbot ] && [ "$KIND" != scout ] && [ "$KIND" != secondmate ] } fi -if [ -d "$WT" ] && [ "$FORCE" != "--force" ]; then - if validate_worktree_teardown_safety; then - : - else - safety_rc=$? - if [ "$safety_rc" -eq "$TEARDOWN_WORKTREE_SAFETY_LOCK_BLOCKED" ]; then - cleanup_stale_lock_for_safety_check "$WT" || exit 1 - validate_worktree_teardown_safety || exit 1 - else - exit 1 - fi - fi +if [ "$FORCE" != "--force" ]; then + case "$KIND" in + secondmate|scout) ;; + *) + validate_worktree_teardown_safety_with_lock_recovery || exit 1 + ;; + esac fi # Every landed/discard-work refusal above has now passed (or --force skipped @@ -2607,7 +2819,8 @@ if [ "$BACKEND" = orca ] && [ "$KIND" != secondmate ]; then fi if [ -d "$WT" ]; then branch=$(git -C "$WT" rev-parse --abbrev-ref HEAD 2>/dev/null || echo HEAD) - if [ "$branch" != "HEAD" ]; then + default_for_cleanup=$(default_branch 2>/dev/null || true) + if [ "$branch" != "HEAD" ] && [ "$branch" != "$default_for_cleanup" ]; then if git -C "$WT" checkout --detach -q 2>/dev/null; then git -C "$WT" branch -D "$branch" >/dev/null 2>&1 || true fi @@ -2617,6 +2830,7 @@ if [ "$BACKEND" = orca ] && [ "$KIND" != secondmate ]; then "$WT/.fm-grok-turnend" "$WT/.fm-kimi-turnend" fi [ -z "$T_ORCA" ] || fm_backend_kill "$BACKEND" "$T" "$(meta_value "$META" zellij_tab_id)" "fm-$ID" 2>/dev/null || true + validate_worktree_teardown_safety_with_lock_recovery || exit 1 fm_backend_remove_worktree "$BACKEND" "$ORCA_WORKTREE_ID" elif [ "$BACKEND" = playbot ] && [ "$KIND" != secondmate ]; then # After common dirty/unlanded validation: stop turn, archive thread/workspace @@ -2624,7 +2838,8 @@ elif [ "$BACKEND" = playbot ] && [ "$KIND" != secondmate ]; then # only after endpoint proof (plan v3 §3.7). if [ -d "$WT" ]; then branch=$(git -C "$WT" rev-parse --abbrev-ref HEAD 2>/dev/null || echo HEAD) - if [ "$branch" != "HEAD" ]; then + default_for_cleanup=$(default_branch 2>/dev/null || true) + if [ "$branch" != "HEAD" ] && [ "$branch" != "$default_for_cleanup" ]; then if git -C "$WT" checkout --detach -q 2>/dev/null; then git -C "$WT" branch -D "$branch" >/dev/null 2>&1 || true fi @@ -2633,7 +2848,8 @@ elif [ "$BACKEND" = playbot ] && [ "$KIND" != secondmate ]; then "$WT/.opencode/plugins/fm-busy-state.js" \ "$WT/.fm-grok-turnend" "$WT/.fm-kimi-turnend" fi - playbot_teardown_endpoint || exit 1 + validate_worktree_teardown_safety_with_lock_recovery || exit 1 + playbot_teardown_endpoint validate_worktree_teardown_safety_with_lock_recovery || exit 1 if [ "$PLAYBOT_ENDPOINT_RETIRED" = 1 ]; then playbot_retire_records else @@ -2642,7 +2858,8 @@ elif [ "$BACKEND" = playbot ] && [ "$KIND" != secondmate ]; then fi elif [ -d "$WT" ] && [ "$KIND" != secondmate ]; then branch=$(git -C "$WT" rev-parse --abbrev-ref HEAD 2>/dev/null || echo HEAD) - if [ "$branch" != "HEAD" ]; then + default_for_cleanup=$(default_branch 2>/dev/null || true) + if [ "$branch" != "HEAD" ] && [ "$branch" != "$default_for_cleanup" ]; then if git -C "$WT" checkout --detach -q 2>/dev/null; then git -C "$WT" branch -D "$branch" >/dev/null 2>&1 || true fi @@ -2654,11 +2871,11 @@ elif [ -d "$WT" ] && [ "$KIND" != secondmate ]; then # to pool. treehouse resolves the pool from the working directory, so run it from # the project. teardown_treehouse_return tolerates transient and stale git locks # left by a killed crew process; see the script header for retry and stale-lock proof. - post_lock_cleanup_check= + pre_return_check= if [ "$FORCE" != "--force" ] && [ "$KIND" != scout ] && [ "$KIND" != secondmate ]; then - post_lock_cleanup_check=validate_worktree_teardown_safety + pre_return_check=validate_worktree_teardown_safety_with_lock_recovery fi - teardown_treehouse_return "$WT" "$PROJ" "worktree" "$post_lock_cleanup_check" || { + teardown_treehouse_return "$WT" "$PROJ" "worktree" "$pre_return_check" || { echo "error: treehouse return failed for worktree $WT; teardown aborted" >&2 exit 1 } diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index a3db57ce6b..6c25b4637f 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -732,7 +732,7 @@ fm_recovery_marker_arm_check() { } fm_lock_try_acquire() { - local lockdir=$1 pid steal cur rc steal_owner primary_owner + local lockdir=$1 pid steal cur rc steal_pid steal_owner primary_owner FM_LOCK_HELD_PID= FM_LOCK_OWNER_DIR= FM_LOCK_RECOVERED_PID= @@ -751,11 +751,34 @@ fm_lock_try_acquire() { return 1 fi + # A steal mutex is the terminal serialization layer. Never recurse into a + # .steal.steal chain when a caller encounters a stale mutex directly. + case "$lockdir" in + *.steal) + FM_LOCK_HELD_PID=$pid + return 1 + ;; + esac + steal="$lockdir.steal" - if ! fm_lock_try_acquire "$steal"; then - FM_LOCK_HELD_PID=$(cat "$lockdir/pid" 2>/dev/null || true) - FM_LOCK_OWNER_DIR= - return 1 + if ! fm_lock_try_create "$steal"; then + steal_pid=$(cat "$steal/pid" 2>/dev/null || true) + if fm_pid_alive "$steal_pid" || fm_lock_mid_acquire_is_fresh "$steal" "$steal_pid"; then + FM_LOCK_HELD_PID=$(cat "$lockdir/pid" 2>/dev/null || true) + FM_LOCK_OWNER_DIR= + return 1 + fi + steal_owner= + if [ -L "$steal" ]; then + steal_owner=$(fm_lock_link_owner "$steal" 2>/dev/null || true) + fi + if ! fm_lock_recheck_stale_owner "$steal" "$steal_owner" "$steal_pid" \ + || ! fm_lock_remove_path "$steal" \ + || ! fm_lock_try_create "$steal"; then + FM_LOCK_HELD_PID=$(cat "$lockdir/pid" 2>/dev/null || true) + FM_LOCK_OWNER_DIR= + return 1 + fi fi steal_owner=${FM_LOCK_OWNER_DIR:-} diff --git a/docs/gitlab-merge-watch.md b/docs/gitlab-merge-watch.md index 0540ed296d..54cbeded3b 100644 --- a/docs/gitlab-merge-watch.md +++ b/docs/gitlab-merge-watch.md @@ -197,4 +197,5 @@ It refuses a GitLab merge request URL rather than sending it to the wrong forge, A GitLab task records no `pr_head=`. `gh` exposes the head commit as a selectable field, while plain `glab` exposes it only inside its JSON output, which would need a JSON processor firstmate does not require. -Both consumers already treat it as optional: `bin/fm-teardown.sh` reads the head from the forge at teardown rather than from metadata and falls back to its provider-agnostic content check, and `bin/fm-review-diff.sh` resolves the head from the remote when none is recorded. +Both consumers treat it as optional. +[`bin/fm-teardown.sh`](../bin/fm-teardown.sh)'s header owns the landed-work proof when no head is available, while `bin/fm-review-diff.sh` resolves the head from the remote when none is recorded. diff --git a/tests/fm-afk-inject-e2e.test.sh b/tests/fm-afk-inject-e2e.test.sh index 65de2e6e1a..742633e171 100755 --- a/tests/fm-afk-inject-e2e.test.sh +++ b/tests/fm-afk-inject-e2e.test.sh @@ -389,7 +389,11 @@ test_scenario_c() { start_daemon echo "done: PR https://example.test/pr/300" > "$STATE_DIR/fake-c1.status" - sleep 6 + local i=0 + while [ "$i" -lt 200 ] && ! grep -q 'Supervisor escalate' "$LOG_FILE" 2>/dev/null; do + sleep 0.1 + i=$((i + 1)) + done # Exactly one terminal-safe marker in the submitted log (no duplicate, no loss). local marker_count diff --git a/tests/fm-afk-inject-herdr-e2e.test.sh b/tests/fm-afk-inject-herdr-e2e.test.sh index e761336e7b..1c302fdb70 100755 --- a/tests/fm-afk-inject-herdr-e2e.test.sh +++ b/tests/fm-afk-inject-herdr-e2e.test.sh @@ -439,7 +439,11 @@ test_scenario_c() { start_daemon echo "done: PR https://example.test/pr/300" > "$STATE_DIR/fake-c1.status" - sleep 8 + local i=0 + while [ "$i" -lt 300 ] && ! grep -q 'Supervisor escalate' "$LOG_FILE" 2>/dev/null; do + sleep 0.1 + i=$((i + 1)) + done local marker_count marker_count=$(awk -F '\t' '{ hex=$1; count += gsub(/e281a3/, "", hex) } END { print count + 0 }' "$LOG_FILE") diff --git a/tests/fm-daemon.test.sh b/tests/fm-daemon.test.sh index 90584b7a3f..04f7cc883f 100755 --- a/tests/fm-daemon.test.sh +++ b/tests/fm-daemon.test.sh @@ -699,6 +699,39 @@ test_durable_wake_batch_handles_refill_before_acknowledgement() { pass "durable mixed wake batches handle refill before acknowledgement" } +test_capacity_freeing_captain_status_covers_same_window_refill() { + local dir state decision + dir=$(make_supercase refill-covered) + state="$dir/state" + printf 'done: PR https://example.test/pr/42\n' > "$state/task.status" + + append_wake "$state" refill refill "refill: re-evaluate ready work against free capacity" \ + || fail "covered refill: refill append failed" + append_wake "$state" signal task.status "signal: $state/task.status" \ + || fail "covered refill: signal append failed" + + FM_STATE_OVERRIDE="$state" handle_durable_wakes "signal: $state/task.status" "$state" \ + || fail "covered refill: batch handling failed" + + grep -F 'task.status: done: PR https://example.test/pr/42' "$state/.subsuper-escalations" >/dev/null \ + || fail "covered refill: done status was not escalated" + if grep -F 'refill: re-evaluate ready work against free capacity' "$state/.subsuper-escalations" >/dev/null 2>&1; then + fail "covered refill: same-window refill still escalated" + fi + [ ! -e "$state/.subsuper-refill-covered" ] \ + || fail "covered refill: one-shot marker was not consumed" + [ ! -s "$state/.wake-queue" ] || fail "covered refill: handled batch remained queued" + + append_wake "$state" refill refill "refill: re-evaluate ready work against free capacity" \ + || fail "covered refill: second refill append failed" + decision=$(classify_refill "$state") + case "$decision" in + escalate\|refill:*) ;; + *) fail "covered refill: later refill-only wake did not escalate (got: $decision)" ;; + esac + pass "capacity-freeing captain status covers same-window refill without dropping later refill-only wakes" +} + test_terminal_stale_escalate_leaves_no_marker() { local dir state win key dir=$(make_supercase stale-terminal-nomarker) @@ -1886,6 +1919,7 @@ test_handle_wake_routes_self_and_escalate test_inject_skip_forces_self test_is_wake_reason_distinguishes_status_stdout test_durable_wake_batch_handles_refill_before_acknowledgement +test_capacity_freeing_captain_status_covers_same_window_refill test_terminal_stale_escalate_leaves_no_marker test_signal_escalate_marks_seen_no_catchall_refire test_collapse_newlines_pure diff --git a/tests/fm-gotmp.test.sh b/tests/fm-gotmp.test.sh index 26e30f58f0..0cf6ee41c9 100755 --- a/tests/fm-gotmp.test.sh +++ b/tests/fm-gotmp.test.sh @@ -42,8 +42,9 @@ TMP_ROOT=$(mktemp -d "${TMPDIR:-/tmp}/fm-gotmp-tests.XXXXXX") # Build a fake FM_HOME/FM_ROOT so the real fm-teardown.sh (symlinked in) resolves # state and helper scripts inside it. Stub the helper scripts fm-teardown calls so no -# live tmux/treehouse/fleet state is touched. A nonexistent worktree path makes both -# `if [ -d "$WT" ]` guards skip, so teardown runs straight to the cleanup + state rm. +# live tmux/treehouse/fleet state is touched. The fixture intentionally has no +# worktree, so its teardown calls use --force to bypass landing proof and isolate +# tasktmp cleanup behavior. make_fake_root() { local id=$1 tasktmp=$2 local fake="$TMP_ROOT/$id" @@ -123,7 +124,7 @@ test_teardown_removes_tasktmp_dir() { # Sanity: dir + contents exist before teardown. [ -d "$task_tmp/gotmp" ] || fail "precondition: gotmp missing before teardown" # Run the REAL teardown against the fake root. - FM_HOME="$fake" bash "$fake/bin/fm-teardown.sh" "$id" >/dev/null 2>&1 \ + FM_HOME="$fake" bash "$fake/bin/fm-teardown.sh" "$id" --force >/dev/null 2>&1 \ || fail "teardown exited non-zero with a valid tasktmp" [ ! -e "$task_tmp" ] \ || fail "teardown did not remove the tasktmp dir ($task_tmp still exists)" @@ -184,7 +185,7 @@ kind=ship mode=no-mistakes yolo=off META - FM_HOME="$fake" bash "$fake/bin/fm-teardown.sh" "$id" >/dev/null 2>&1 \ + FM_HOME="$fake" bash "$fake/bin/fm-teardown.sh" "$id" --force >/dev/null 2>&1 \ || fail "teardown exited non-zero when tasktmp= was absent" pass "fm-teardown skips gracefully when tasktmp= is absent (backward compat)" } @@ -197,7 +198,7 @@ test_teardown_skips_gracefully_when_dir_missing() { [ ! -e "$task_tmp" ] || fail "precondition: task_tmp should not exist yet" local fake fake=$(make_fake_root "$id" "$task_tmp") - FM_HOME="$fake" bash "$fake/bin/fm-teardown.sh" "$id" >/dev/null 2>&1 \ + FM_HOME="$fake" bash "$fake/bin/fm-teardown.sh" "$id" --force >/dev/null 2>&1 \ || fail "teardown exited non-zero when tasktmp dir was missing" [ ! -e "$task_tmp" ] || fail "teardown created/left the tasktmp dir unexpectedly" pass "fm-teardown skips gracefully when tasktmp= points to a nonexistent dir" diff --git a/tests/fm-playbot-backend.test.sh b/tests/fm-playbot-backend.test.sh index 3edf89aca2..f395117f95 100644 --- a/tests/fm-playbot-backend.test.sh +++ b/tests/fm-playbot-backend.test.sh @@ -301,17 +301,66 @@ fm_backend_playbot_abort_cleanup_confirmed thread-gone workspace-gone "$TMP_ROOT pass "abort cleanup confirmation requires thread, workspace, and worktree absence" TD_OUT=$(fm_backend_playbot_teardown "$STATE/be-ep.meta" be-ep playbot:thread-complete \ - "$WORKTREE_TASK" workspace-task thread-complete 2>"$TMP_ROOT/td.err") && TD_RC=0 || TD_RC=$? + "$WORKTREE_TASK" workspace-task thread-complete : 2>"$TMP_ROOT/td.err") && TD_RC=0 || TD_RC=$? [ "$TD_RC" -ne 0 ] || fail "teardown must refuse a live endpoint when archive/stop cannot complete" case "$TD_OUT" in refuse:*) : ;; *) fail "teardown refusal must print a refuse: proof token, got $TD_OUT" ;; esac TD_RET=$(fm_backend_playbot_teardown "$STATE/be-ep.meta" be-ep playbot:thread-archived \ - "$WORKTREE_TASK" workspace-task thread-archived) \ + "$WORKTREE_TASK" workspace-task thread-archived :) \ || fail "an already-gone endpoint must report retained or retired, not refuse" case "$TD_RET" in retained:*|retired) : ;; *) fail "already-gone teardown must print retained: or retired, got $TD_RET" ;; esac TD_MM=$(fm_backend_playbot_teardown "$STATE/be-ep.meta" be-ep playbot:thread-complete \ - "$WORKTREE_TASK" workspace-task thread-OTHER 2>/dev/null) && TD_RC=0 || TD_RC=$? + "$WORKTREE_TASK" workspace-task thread-OTHER : 2>/dev/null) && TD_RC=0 || TD_RC=$? [ "$TD_RC" -ne 0 ] && [ "$TD_MM" = 'refuse:target-thread-mismatch' ] \ || fail "teardown must refuse a target/thread identity mismatch" pass "teardown refuses live/mismatched endpoints and reports retained/retired for a confirmed-gone thread" +PLAYBOT_TEARDOWN_LOG="$TMP_ROOT/teardown-order.log" +playbot_teardown_safety_check() { + [ ! -e "$WORKTREE_TASK/late-worker-write" ] +} +fm_backend_playbot_lane() { + case "${1:-}" in + agent-state) printf '%s\n' "${PLAYBOT_TEST_STATE:-alive}" ;; + stop) printf 'stop\n' >> "$PLAYBOT_TEARDOWN_LOG" ;; + archive) + printf 'archive\n' >> "$PLAYBOT_TEARDOWN_LOG" + printf 'late\n' > "$WORKTREE_TASK/late-worker-write" + ;; + delete) printf 'delete\n' >> "$PLAYBOT_TEARDOWN_LOG" ;; + *) return 1 ;; + esac +} +: > "$PLAYBOT_TEARDOWN_LOG" +TD_RACE=$(fm_backend_playbot_teardown "$STATE/be-ep.meta" be-ep playbot:thread-complete \ + "$WORKTREE_TASK" workspace-task thread-complete playbot_teardown_safety_check) \ + && TD_RC=0 || TD_RC=$? +[ "$TD_RC" -ne 0 ] && [ "$TD_RACE" = 'refuse:worktree-safety-recheck-failed' ] \ + || fail "teardown must refuse a worker write created while the endpoint quiesces" +grep -qxF archive "$PLAYBOT_TEARDOWN_LOG" \ + || fail "race fixture did not archive the endpoint before the safety recheck" +if grep -qxF delete "$PLAYBOT_TEARDOWN_LOG"; then + fail "teardown deleted the workspace after the post-quiescence safety recheck failed" +fi +pass "teardown rechecks worktree safety after Playbot quiescence and before workspace deletion" + +PLAYBOT_TEST_STATE=missing +: > "$PLAYBOT_TEARDOWN_LOG" +TD_MISSING=$(fm_backend_playbot_teardown "$STATE/be-ep.meta" be-ep playbot:thread-archived \ + "$WORKTREE_TASK" workspace-task thread-archived playbot_teardown_safety_check) \ + && TD_RC=0 || TD_RC=$? +[ "$TD_RC" -ne 0 ] && [ "$TD_MISSING" = 'refuse:worktree-safety-recheck-failed' ] \ + || fail "teardown must safety-check an already-gone endpoint before workspace deletion" +if grep -qxF delete "$PLAYBOT_TEARDOWN_LOG"; then + fail "teardown deleted the workspace after the confirmed-gone safety recheck failed" +fi +rm -f "$WORKTREE_TASK/late-worker-write" +: > "$PLAYBOT_TEARDOWN_LOG" +TD_SAFE=$(fm_backend_playbot_teardown "$STATE/be-ep.meta" be-ep playbot:thread-archived \ + "$WORKTREE_TASK" workspace-task thread-archived :) \ + || fail "teardown must remove an already-gone endpoint when the safety recheck passes" +[ "$TD_SAFE" = retired ] || fail "safe confirmed-gone teardown must report retired" +grep -qxF delete "$PLAYBOT_TEARDOWN_LOG" \ + || fail "safe confirmed-gone teardown did not reach workspace deletion" +pass "confirmed-gone Playbot deletion also requires a passing worktree safety recheck" + printf 'fm-playbot-backend: all tests passed\n' diff --git a/tests/fm-public-followup.test.sh b/tests/fm-public-followup.test.sh index fe15e239e2..4ae3ad6a24 100755 --- a/tests/fm-public-followup.test.sh +++ b/tests/fm-public-followup.test.sh @@ -1110,13 +1110,14 @@ test_cleanup_refuses_while_a_public_reply_is_owed() { local home rc home=$(make_home cleanup-guard) seed_commitment "$home" pf-guard req-guard discord main ship-task + fm_git_init_commit "$home/projects/sample" fm_write_meta "$home/state/ship-task.meta" \ "window=firstmate:fm-ship-task" \ - "worktree=$home/projects/gone" \ + "worktree=$home/projects/sample" \ "project=$home/projects/sample" \ "harness=codex" \ "kind=ship" \ - "mode=no-mistakes" + "mode=local-only" rc=0 PATH="$home/fakebin:$PATH" FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$home" \ diff --git a/tests/fm-remote-job.test.sh b/tests/fm-remote-job.test.sh index f2ef8ce643..165ec6afcd 100755 --- a/tests/fm-remote-job.test.sh +++ b/tests/fm-remote-job.test.sh @@ -62,8 +62,8 @@ SH cat > "$REMOTE_ROOT/bin/fm-shutdown-job.sh" <<'SH' #!/bin/bash trap '' HUP INT TERM -printf 'started\n' > "$1" -sleep 3 +printf '%s\n' "$$" > "$1" +sleep 30 printf 'ran\n' > "$2" SH cat > "$REMOTE_ROOT/bin/fm-output-job.sh" <<'SH' @@ -163,10 +163,14 @@ case ":$FM_REMOTE_JOB_OPERATOR_PATH:" in esac pass "operator PATH resolves the authorized Nix profile bin link" +# Match the production Linux launcher: the restart supervisor owns an isolated +# process group so replacement can stop both it and its serving child. +set -m HOME="$ACCOUNT_HOME" PATH="$RUNTIME_BIN:/usr/bin:/bin:/usr/sbin:/sbin" FM_FAKE_PERL_LOG="$FAKE_PERL_LOG" \ FM_ROOT_OVERRIDE="$REMOTE_ROOT" FM_REMOTE_JOB_STATE_ROOT="$STATE_ROOT" \ FM_REMOTE_JOB_PLATFORM_OVERRIDE=Linux FM_REMOTE_JOB_TIMEOUT=5 \ "$REMOTE_ROOT/bin/fm-remote-job-worker.sh" > "$TMP_ROOT/worker.out" 2> "$TMP_ROOT/worker.err" & +set +m for _ in $(seq 1 100); do [ -f "$STATE_ROOT/worker.ready" ] && break sleep 0.05 @@ -314,12 +318,11 @@ fm_remote_job_reap "$ACCOUNT_HOME" "$FIRST_JOB_ID" || fail "the blocking job cou fm_remote_job_reap "$ACCOUNT_HOME" "$JOB_ID" || fail "the expired queued job could not be reaped" pass "the worker expires queued jobs before they can mutate" -FIRST_DELAYED_SIDE_EFFECT="$TMP_ROOT/first-delayed-side-effect" SECOND_DELAYED_SIDE_EFFECT="$TMP_ROOT/second-delayed-side-effect" -FM_REMOTE_JOB_QUEUE_TIMEOUT=5 +FM_REMOTE_JOB_QUEUE_TIMEOUT=60 FM_REMOTE_JOB_TIMEOUT=3 fm_remote_job_stage "$ACCOUNT_HOME" "$REMOTE_ROOT" "$REMOTE_HOME" \ - fm-delay-job.sh 1.8 "$FIRST_DELAYED_SIDE_EFFECT" < /dev/null > /dev/null + fm-timeout-job.sh < /dev/null > /dev/null FIRST_JOB_ID=$FM_REMOTE_JOB_ID FIRST_JOB_DIR="$STATE_ROOT/jobs/$FIRST_JOB_ID" for _ in $(seq 1 100); do @@ -328,15 +331,24 @@ for _ in $(seq 1 100); do done [ "$(fm_remote_job_read_state "$FIRST_JOB_DIR" 2>/dev/null || true)" = running ] \ || fail "the first delayed job did not begin running" +FM_REMOTE_JOB_TIMEOUT=10 fm_remote_job_stage "$ACCOUNT_HOME" "$REMOTE_ROOT" "$REMOTE_HOME" \ - fm-delay-job.sh 1.8 "$SECOND_DELAYED_SIDE_EFFECT" < /dev/null > /dev/null + fm-touch-job.sh "$SECOND_DELAYED_SIDE_EFFECT" < /dev/null > /dev/null JOB_ID=$FM_REMOTE_JOB_ID +SECOND_JOB_DIR="$STATE_ROOT/jobs/$JOB_ID" fm_remote_job_wait "$ACCOUNT_HOME" "$FIRST_JOB_ID" || fail "$FM_REMOTE_JOB_ERROR" +[ "$FM_REMOTE_JOB_EXIT" -eq 124 ] || fail "the queue-blocking job did not consume its execution window" +QUEUE_RELEASED_AT=$(date +%s) fm_remote_job_wait "$ACCOUNT_HOME" "$JOB_ID" || fail "$FM_REMOTE_JOB_ERROR" [ "$FM_REMOTE_JOB_EXIT" -eq 0 ] || fail "queue time consumed the second job's execution timeout" assert_present "$SECOND_DELAYED_SIDE_EFFECT" "the queued job did not receive its full execution timeout" +SECOND_DEADLINE=$(fm_remote_job_read_deadline "$SECOND_JOB_DIR") \ + || fail "the queued job did not record an execution deadline" +[ "$SECOND_DEADLINE" -ge $((QUEUE_RELEASED_AT + 8)) ] \ + || fail "the queued job's execution deadline was not established after queue release" fm_remote_job_reap "$ACCOUNT_HOME" "$FIRST_JOB_ID" || fail "the first delayed job could not be reaped" fm_remote_job_reap "$ACCOUNT_HOME" "$JOB_ID" || fail "the second delayed job could not be reaped" +FM_REMOTE_JOB_TIMEOUT=3 pass "queued jobs receive a fresh bounded execution window" if command -v shasum >/dev/null 2>&1; then @@ -444,7 +456,13 @@ done assert_present "$STATE_ROOT/worker.ready" "the replacement worker did not become ready" fm_remote_job_wait "$ACCOUNT_HOME" "$JOB_ID" || fail "$FM_REMOTE_JOB_ERROR" [ "$FM_REMOTE_JOB_EXIT" -eq 125 ] || fail "the interrupted job did not publish an unknown-completion result" -sleep 3 +SHUTDOWN_COMMAND_PID=$(cat "$STARTED") +for _ in $(seq 1 100); do + kill -0 "$SHUTDOWN_COMMAND_PID" 2>/dev/null || break + sleep 0.05 +done +kill -0 "$SHUTDOWN_COMMAND_PID" 2>/dev/null \ + && fail "the active command remained alive after worker shutdown" assert_absent "$SHUTDOWN_SIDE_EFFECT" "the active command mutated after worker shutdown" fm_remote_job_reap "$ACCOUNT_HOME" "$JOB_ID" || fail "the interrupted job could not be reaped" pass "worker shutdown terminates the active command tree before replacement" @@ -471,7 +489,13 @@ done || fail "the Linux supervisor did not restart a crashed worker" fm_remote_job_wait "$ACCOUNT_HOME" "$JOB_ID" || fail "$FM_REMOTE_JOB_ERROR" [ "$FM_REMOTE_JOB_EXIT" -eq 125 ] || fail "worker crash recovery did not publish unknown completion" -sleep 3 +CRASH_COMMAND_PID=$(cat "$CRASH_STARTED") +for _ in $(seq 1 100); do + kill -0 "$CRASH_COMMAND_PID" 2>/dev/null || break + sleep 0.05 +done +kill -0 "$CRASH_COMMAND_PID" 2>/dev/null \ + && fail "the orphaned command remained alive after worker crash recovery" assert_absent "$CRASH_SIDE_EFFECT" "an orphaned command mutated after worker crash recovery" fm_remote_job_reap "$ACCOUNT_HOME" "$JOB_ID" || fail "the crash-recovered job could not be reaped" fm_remote_job_probe "$ACCOUNT_HOME" || fail "the restarted worker did not remain ready" diff --git a/tests/fm-remote-secondmate-lifecycle-e2e.test.sh b/tests/fm-remote-secondmate-lifecycle-e2e.test.sh index 9e6bfbba4d..a095831dff 100755 --- a/tests/fm-remote-secondmate-lifecycle-e2e.test.sh +++ b/tests/fm-remote-secondmate-lifecycle-e2e.test.sh @@ -455,7 +455,7 @@ projects_snapshot() { # } mkdir -p "$TMP_ROOT/seed-parent/projects" fm_git_init_commit "$TMP_ROOT/seed-parent/projects/resident" -git init -q --bare "$TMP_ROOT/beta.git" +git init -q --bare -b main "$TMP_ROOT/beta.git" fm_git_init_commit "$TMP_ROOT/beta-src" git -C "$TMP_ROOT/beta-src" remote add origin "file://$TMP_ROOT/beta.git" git -C "$TMP_ROOT/beta-src" push -q -u origin HEAD diff --git a/tests/fm-teardown.test.sh b/tests/fm-teardown.test.sh index 990569b67e..ad276c68ce 100755 --- a/tests/fm-teardown.test.sh +++ b/tests/fm-teardown.test.sh @@ -1,60 +1,7 @@ #!/usr/bin/env bash -# Tests for bin/fm-teardown.sh's landed-work safety and stale-lock recovery. -# -# The check refuses to tear down a worktree whose work has not LANDED, because -# treehouse return hard-resets the worktree. "Landed" means reachable from a remote -# OR - for a normal ship task whose commits are not so reachable - its PR is merged -# and GitHub reports a PR head that contains the current local work, or its content -# is already in the up-to-date default branch. -# -# Covers four fixes: -# - local-only fork-remote: a fork IS a remote, so fork-pushed upstream- -# contribution PRs are teardown-eligible (the pre-fix code false-refused them). -# - squash-merge-then-delete-branch: the branch's own commits live nowhere on a -# remote after a squash merge deletes the head branch, yet the change is fully in -# main. Reachability alone false-refused this common GitHub flow; the check now -# recognizes a merged PR head containing the local work (or the content already -# in main) as landed. -# - open-PR veto: content-in-default is provenance-blind, so a SIBLING PR that -# landed the same change makes an unmerged task's content look "landed" and the -# old check reaped its still-open PR's state. A recorded-but-open PR now vetoes -# the content fallback: the task's own work has not landed until its own PR merges. -# - teardown-lock-race: a killed crew process can leave a transient worktree -# git index.lock that blocks teardown. The return path retries on the lock -# error signature (even if the lock self-clears mid-check), then only removes a -# provably stale lock before re-running safety checks. -# -# Matrix: -# (a) local-only + HEAD on a fork remote-tracking branch -> ALLOW (fork fix) -# (b) local-only + truly unpushed work (no remote, not main) -> REFUSE (safety) -# (c) local-only + merged into local main, no remote -> ALLOW (no regression) -# (d) no-mistakes + HEAD on origin remote-tracking branch -> ALLOW (no regression) -# (e) no-mistakes + unpushed, no PR, content not in default -> REFUSE (safety) -# (f) local-only + truly unpushed + --force -> ALLOW (escape hatch) -# (g) no-mistakes + squash-merged PR, exact PR head -> ALLOW (squash fix) -# (h) no-mistakes + no PR but content already in default -> ALLOW (content fallback) -# (i) no-mistakes + dirty worktree, even when work landed -> REFUSE (dirty wins) -# (j) no-mistakes + gh lookup errors + content not in default -> REFUSE (fail-safe) -# (k) no-mistakes + merged PR but HEAD moved afterward -> REFUSE (stale PR) -# (l) no-mistakes + stale origin/main but fetched content -> ALLOW (fresh fetch) -# (m) no-mistakes + local HEAD ancestor of merged PR head -> ALLOW (lagging local) -# (n) no-mistakes + replayed unpushed patch in merged PR head -> ALLOW (replayed local) -# (o) fm-pr-check rerun after HEAD moved -> no stale pr_head -# (p) fm-pr-check when local HEAD lags -> record remote PR head -# (q) no-mistakes + NO pr= recorded, PR discovered by branch -> ALLOW (yolo/no-CI merge) -# (za) no-mistakes + recorded pr= OPEN + sibling content in default -> REFUSE (open-PR veto) -# (zb) no-mistakes + recorded pr= MERGED + sibling content in default -> ALLOW (own PR landed) -# -# Also covers backlog teardown-lock-race: a git index.lock left in the worktree by a -# killed crew process (bin/fm-teardown.sh's teardown_treehouse_return). -# (r) provably-stale index.lock (old mtime, no live holder) -> lock removed, ALLOW -# (s) index.lock with a live holder, any age -> lock kept, REFUSE -# (t) lsof error while checking index.lock -> lock kept, REFUSE -# (u) dirty worktree after stale lock cleanup -> lock removed, REFUSE -# (v) non-linked repo index.lock -> lock removed, ALLOW -# (w) index.lock mtime read failure -> lock kept, REFUSE -# (x) transient lock cleared after first failed return -> retry ALLOW -# (y) persistent lock (never clears, not provably stale) -> REFUSE loudly +# Regression tests for bin/fm-teardown.sh's landed-work safety and stale-lock recovery. +# That script's header is the single owner of the complete safety contract. +# Case-local comments describe only the evidence each fixture isolates. set -u # shellcheck source=tests/lib.sh disable=SC1091 @@ -354,6 +301,72 @@ land_equivalent_patch_on_origin_branch() { } # Override gh-axi so every call fails, simulating an API/network error. + +# glab reports a merged GitLab MR via single state: field. + +add_gh_pr_closed() { + local case_dir=$1 + cat > "$case_dir/fakebin/gh-axi" <<'SH' +#!/usr/bin/env bash +case "${1:-} ${2:-}" in + "pr list") + printf '%s\n' "count: 1 (showing first 1)" "pull_requests[1]{number,state}:" " 7,closed" ; exit 0 ;; + "pr view") + printf '%s\n' "pull_request:" " number: 7" " state: closed" ; exit 0 ;; +esac +exit 0 +SH + cat > "$case_dir/fakebin/gh" <<'SH' +#!/usr/bin/env bash +case "${1:-} ${2:-}" in + "pr view") + case " $* " in + *"state,headRefOid"*) printf '%s\t%s\n' 'CLOSED' '0000000000000000000000000000000000000000' ; exit 0 ;; + *"state"*) printf '%s\n' 'CLOSED' ; exit 0 ;; + esac + ;; +esac +echo "error: pull request not found" >&2 +exit 1 +SH + chmod +x "$case_dir/fakebin/gh-axi" "$case_dir/fakebin/gh" +} + +add_glab_mr_merged() { + local case_dir=$1 + cat > "$case_dir/fakebin/glab" <<'SH' +#!/usr/bin/env bash +case "${1:-} ${2:-}" in + "mr view") + printf '%s\n' 'state: merged' 'title: t' + exit 0 + ;; +esac +exit 1 +SH + chmod +x "$case_dir/fakebin/glab" +} + +add_glab_mr_open() { + local case_dir=$1 + cat > "$case_dir/fakebin/glab" <<'SH' +#!/usr/bin/env bash +case "${1:-} ${2:-}" in + "mr view") + printf '%s\n' 'state: open' 'title: t' + exit 0 + ;; +esac +exit 1 +SH + chmod +x "$case_dir/fakebin/glab" +} + +add_gh_pr_state_only() { + # state + head for classify; used by open/closed/merged paths already via add_gh_* + : +} + add_gh_axi_error() { local case_dir=$1 cat > "$case_dir/fakebin/gh-axi" <<'SH' @@ -2707,6 +2720,395 @@ EOF pass "the run abort and the leaked-process reap both complete before the destructive worktree return" } +test_process_reap_mutation_refuses_before_worktree_return() { + local case_dir rc pid + case_dir=$(make_case reap-mutation-refusal) + write_meta "$case_dir" no-mistakes ship + land_shippable_commit "$case_dir" + + ( + cd "$case_dir/wt" || exit 1 + trap 'printf "%s\n" post-reap > dirty-after-reap.txt; exit 0' TERM + while :; do sleep 1; done + ) & + pid=$! + disown + sleep 0.3 + kill -0 "$pid" 2>/dev/null || fail "reap-mutation-refusal: setup writer did not start" + + cat > "$case_dir/fakebin/treehouse" <> "$case_dir/treehouse.log" +EOF + chmod +x "$case_dir/fakebin/treehouse" + + rc=0 + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" || rc=$? + kill -0 "$pid" 2>/dev/null && { kill -KILL "$pid" 2>/dev/null || true; } + + expect_code 1 "$rc" "reap-mutation-refusal: teardown should refuse post-reap changes" + assert_present "$case_dir/wt/dirty-after-reap.txt" \ + "reap-mutation-refusal: TERM handler did not create the post-reap change" + assert_grep "has uncommitted changes" "$case_dir/stderr" \ + "reap-mutation-refusal: post-reap safety check did not report the change" + assert_absent "$case_dir/treehouse.log" \ + "reap-mutation-refusal: teardown returned the worktree after it became dirty" + assert_present "$case_dir/state/task-x1.meta" \ + "reap-mutation-refusal: teardown removed task metadata after refusing" + pass "post-reap worktree mutations are revalidated before destructive return" +} + +test_playbot_archive_mutation_refuses_before_workspace_deletion() { + local case_dir rc + case_dir=$(make_case playbot-archive-mutation-refusal) + fm_write_meta "$case_dir/state/task-x1.meta" \ + "window=playbot:thread-task-x1" \ + "endpoint_task_id=task-x1" \ + "worktree=$case_dir/wt" \ + "project=$case_dir/project" \ + "kind=ship" \ + "mode=local-only" \ + "backend=playbot" \ + "playbot_project_id=project-alpha" \ + "playbot_project_root_id=root-alpha" \ + "playbot_workspace_id=workspace-task-x1" \ + "playbot_thread_id=thread-task-x1" \ + "playbot_route_gen=1" \ + "playbot_delivery_id=delivery-task-x1" + land_shippable_commit "$case_dir" + printf '%s\n' '{"route":"preserve"}' > "$case_dir/state/task-x1.playbot-route.json" + printf '%s\n' '{"outbox":"preserve"}' > "$case_dir/state/task-x1.playbot-outbox.json" + + cat > "$case_dir/playbot-lanes.mjs" <<'JS' +import { appendFileSync, writeFileSync } from "node:fs"; + +const [command] = process.argv.slice(2); +const log = process.env.FM_PLAYBOT_TEST_LOG; +const worktree = process.env.FM_PLAYBOT_TEST_WORKTREE; +if (command === "validate-endpoint") process.exit(0); +if (command === "agent-state") { + process.stdout.write("alive\n"); + process.exit(0); +} +appendFileSync(log, `${command}\n`); +if (command === "archive") { + writeFileSync(`${worktree}/late-worker-write.txt`, "created while Playbot archived the thread\n"); +} +process.exit(0); +JS + + rc=0 + FM_PLAYBOT_LANES_OVERRIDE="$case_dir/playbot-lanes.mjs" \ + FM_PLAYBOT_TEST_LOG="$case_dir/playbot.log" \ + FM_PLAYBOT_TEST_WORKTREE="$case_dir/wt" \ + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" || rc=$? + + expect_code 1 "$rc" "playbot-archive-mutation-refusal: teardown should refuse the late worker write" + assert_grep "archive" "$case_dir/playbot.log" \ + "playbot-archive-mutation-refusal: fixture did not archive the Playbot thread" + if grep -qxF delete "$case_dir/playbot.log"; then + fail "playbot-archive-mutation-refusal: teardown deleted the workspace after the late write" + fi + assert_present "$case_dir/wt/late-worker-write.txt" \ + "playbot-archive-mutation-refusal: late worker write was not preserved" + assert_grep "has uncommitted changes" "$case_dir/stderr" \ + "playbot-archive-mutation-refusal: post-archive safety check did not report the late write" + assert_present "$case_dir/state/task-x1.meta" \ + "playbot-archive-mutation-refusal: teardown removed task metadata after refusing" + assert_present "$case_dir/state/task-x1.playbot-route.json" \ + "playbot-archive-mutation-refusal: teardown removed the Playbot route after refusing" + assert_present "$case_dir/state/task-x1.playbot-outbox.json" \ + "playbot-archive-mutation-refusal: teardown removed the Playbot outbox after refusing" + pass "Playbot archive-time mutations refuse workspace deletion and preserve task records" +} + + +# --- v5 default-deny regressions (plan v5 break matrix) --- + +test_open_pr_on_clean_pushed_branch_refuses() { + # R1-742: pushed feature + OPEN pr= → empty unpushed must still refuse. + local case_dir rc tip + case_dir=$(make_case open-pr-clean-pushed) + write_meta "$case_dir" no-mistakes ship + append_pr_meta_url "$case_dir" + wt_commit_file "$case_dir" feature.txt unique-open "open feature" + git -C "$case_dir/wt" push -q origin fm/task-x1 + git -C "$case_dir/project" fetch -q origin + tip=$(git -C "$case_dir/wt" rev-parse HEAD) + git -C "$case_dir/origin.git" cat-file -t "$tip" >/dev/null + add_gh_pr_open "$case_dir" + # Armed poll shape: meta + poll files would be destroyed by a false allow. + printf 'url=https://github.com/example/repo/pull/7\n' > "$case_dir/state/task-x1.pr-poll" + touch "$case_dir/state/task-x1.pr-poll-registration" + + set +e + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "open-pr-clean-pushed: must refuse (empty unpushed is not landing)" + grep -q REFUSED "$case_dir/stderr" || fail "open-pr-clean-pushed: no REFUSED" + grep -q "still open" "$case_dir/stderr" || fail "open-pr-clean-pushed: did not cite open PR" + [ -f "$case_dir/state/task-x1.meta" ] || fail "open-pr-clean-pushed: meta destroyed" + [ -f "$case_dir/state/task-x1.pr-poll" ] || fail "open-pr-clean-pushed: poll destroyed" + pass "pushed open-PR ship refuses teardown and keeps meta+poll (R1-742)" +} + +test_missing_worktree_refuses() { + # F4: missing WT always refuses; pr_head never authorizes. + local case_dir rc + case_dir=$(make_case missing-wt-refuse) + write_meta "$case_dir" no-mistakes ship + append_pr_meta_url "$case_dir" + tip=$(git -C "$case_dir/wt" rev-parse HEAD) + printf 'pr_head=%s\n' "$tip" >> "$case_dir/state/task-x1.meta" + # Land tip on main so a pr_head-ancestry allow would incorrectly pass. + git -C "$case_dir/wt" push -q origin HEAD:main + git -C "$case_dir/project" fetch -q origin + add_gh_pr_merged_for_head "$case_dir" "$tip" + rm -rf "$case_dir/wt" + # Point meta at missing path (write_meta already set worktree; recreate meta line). + sed -i.bak 's|^worktree=.*|worktree='"$case_dir"'/wt|' "$case_dir/state/task-x1.meta" + rm -f "$case_dir/state/task-x1.meta.bak" + + set +e + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "missing-wt: must refuse" + grep -q REFUSED "$case_dir/stderr" || fail "missing-wt: no REFUSED" + grep -q "no inspectable worktree" "$case_dir/stderr" || fail "missing-wt: wrong reason" + [ -f "$case_dir/state/task-x1.meta" ] || fail "missing-wt: meta destroyed" + pass "missing worktree refuses even with pr_head and MERGED (F4)" +} + +test_no_pr_unique_pushed_refuses() { + # R1-D: pushed unique commits, no pr=, not in default → refuse. + local case_dir rc + case_dir=$(make_case no-pr-unique-pushed) + write_meta "$case_dir" no-mistakes ship + wt_commit_file "$case_dir" unique.txt only-here "unique" + git -C "$case_dir/wt" push -q origin fm/task-x1 + git -C "$case_dir/project" fetch -q origin + + set +e + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "no-pr-unique-pushed: must refuse" + grep -q REFUSED "$case_dir/stderr" || fail "no-pr-unique-pushed: no REFUSED" + [ -f "$case_dir/state/task-x1.meta" ] || fail "no-pr-unique-pushed: meta destroyed" + pass "pushed unique work without pr= refuses without positive land proof (R1-D)" +} + +test_closed_unmerged_pushed_refuses() { + # R1-A: CLOSED is not landing; unique tip not in default. + local case_dir rc + case_dir=$(make_case closed-unmerged) + write_meta "$case_dir" no-mistakes ship + append_pr_meta_url "$case_dir" + wt_commit_file "$case_dir" closed.txt only "closed unique" + git -C "$case_dir/wt" push -q origin fm/task-x1 + git -C "$case_dir/project" fetch -q origin + add_gh_pr_closed "$case_dir" + + set +e + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "closed-unmerged: must refuse" + grep -q REFUSED "$case_dir/stderr" || fail "closed-unmerged: no REFUSED" + [ -f "$case_dir/state/task-x1.meta" ] || fail "closed-unmerged: meta destroyed" + pass "CLOSED unmerged pushed work refuses (R1-A)" +} + +test_gitlab_merged_squash_tree_allows() { + # FR1: GitLab MERGED + tree on D, tip not ancestor → allow via T. + local case_dir rc + case_dir=$(make_case gitlab-squash-merged) + write_meta "$case_dir" no-mistakes ship + printf '%s\n' 'pr=https://gitlab.com/example/repo/-/merge_requests/7' >> "$case_dir/state/task-x1.meta" + wt_commit_file "$case_dir" feature.txt hello "add feature" + land_on_origin_main "$case_dir" feature.txt hello + # Tip is NOT ancestor of main (squash); tree matches. + add_glab_mr_merged "$case_dir" + + set +e + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "gitlab-squash-merged: should allow via MERGED+tree" + ! grep -q REFUSED "$case_dir/stderr" || fail "gitlab-squash-merged: REFUSED" + pass "GitLab MERGED squash with tree on live D allows (FR1)" +} + +test_gitlab_open_refuses() { + local case_dir rc + case_dir=$(make_case gitlab-open) + write_meta "$case_dir" no-mistakes ship + printf '%s\n' 'pr=https://gitlab.com/example/repo/-/merge_requests/7' >> "$case_dir/state/task-x1.meta" + wt_commit_file "$case_dir" feature.txt hello "add feature" + land_on_origin_main "$case_dir" feature.txt hello + add_glab_mr_open "$case_dir" + + set +e + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "gitlab-open: must refuse" + grep -q REFUSED "$case_dir/stderr" || fail "gitlab-open: no REFUSED" + pass "GitLab OPEN refuses even with sibling content in default" +} + +test_unconfirmed_sibling_tree_refuses() { + # R2-R1: gh error + sibling tree + pr= → no T under unconfirmed. + local case_dir rc + case_dir=$(make_case unconfirmed-sibling) + write_meta "$case_dir" no-mistakes ship + append_pr_meta_url "$case_dir" + wt_commit_file "$case_dir" feature.txt hello "add feature" + land_on_origin_main "$case_dir" feature.txt hello + add_gh_axi_error "$case_dir" + # Break gh too so classify is unconfirmed. + cat > "$case_dir/fakebin/gh" <<'SH' +#!/usr/bin/env bash +echo "error: API" >&2 +exit 1 +SH + chmod +x "$case_dir/fakebin/gh" + + set +e + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "unconfirmed-sibling: must refuse" + grep -q REFUSED "$case_dir/stderr" || fail "unconfirmed-sibling: no REFUSED" + [ -f "$case_dir/state/task-x1.meta" ] || fail "unconfirmed-sibling: meta destroyed" + pass "unconfirmed forge + sibling tree refuses (R2-R1)" +} + +test_default_oid_force_push_after_fetch_refuses() { + # V4-F1: same-branch force-push of main after fetch → refuse. + # Interpose git so the first live_default_tip fetch of main is followed by a + # remote force-push back to baseline before the post-fetch ls-remote OID read. + local case_dir rc baseline tip real_git + case_dir=$(make_case default-oid-drift) + write_meta "$case_dir" no-mistakes ship + baseline=$(git -C "$case_dir/project" rev-parse origin/main) + wt_commit_file "$case_dir" feature.txt on-main "land me" + tip=$(git -C "$case_dir/wt" rev-parse HEAD) + # Push tip onto main so initial land would pass ancestry. + git -C "$case_dir/wt" push -q origin HEAD:main + git -C "$case_dir/project" fetch -q origin + # Also keep task branch pushed. + git -C "$case_dir/wt" push -q origin fm/task-x1 2>/dev/null || true + + real_git=${REAL_GIT_FOR_TEST:-$(command -v git)} + cat > "$case_dir/fakebin/git" < "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "default-oid-drift: must refuse when live main OID drifts after fetch" + grep -q REFUSED "$case_dir/stderr" || fail "default-oid-drift: no REFUSED" + [ -f "$case_dir/state/task-x1.meta" ] || fail "default-oid-drift: meta destroyed" + # Live main should exclude tip after drift. + live=$(git -C "$case_dir/origin.git" rev-parse refs/heads/main) + [ "$live" = "$baseline" ] || fail "default-oid-drift: fixture did not force-push main (live=$live tip=$tip baseline=$baseline)" + pass "same-branch default OID force-push after fetch refuses (V4-F1)" +} + +test_default_symref_disappears_after_fetch_refuses() { + local case_dir rc real_git + case_dir=$(make_case default-symref-disappears) + write_meta "$case_dir" no-mistakes ship + land_shippable_commit "$case_dir" + real_git=${REAL_GIT_FOR_TEST:-$(command -v git)} + cat > "$case_dir/fakebin/git" < "$case_dir/symref-count" + if [ "\$count" -eq 2 ]; then + exec "$real_git" -C "\$2" ls-remote "\$5" HEAD + fi +fi +exec "$real_git" "\$@" +SH + chmod +x "$case_dir/fakebin/git" + + rc=0 + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" || rc=$? + + expect_code 1 "$rc" "default-symref-disappears: teardown should refuse" + assert_grep "REFUSED" "$case_dir/stderr" \ + "default-symref-disappears: teardown did not report a refusal" + assert_present "$case_dir/state/task-x1.meta" \ + "default-symref-disappears: teardown removed task metadata" + pass "a remote default symref disappearing after fetch refuses teardown" +} + +test_default_symref_lookup_failure_after_fetch_refuses() { + local case_dir rc real_git + case_dir=$(make_case default-symref-failure) + write_meta "$case_dir" no-mistakes ship + land_shippable_commit "$case_dir" + real_git=${REAL_GIT_FOR_TEST:-$(command -v git)} + cat > "$case_dir/fakebin/git" < "$case_dir/symref-count" + [ "\$count" -ne 2 ] || exit 1 +fi +exec "$real_git" "\$@" +SH + chmod +x "$case_dir/fakebin/git" + + rc=0 + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" || rc=$? + + expect_code 1 "$rc" "default-symref-failure: teardown should refuse" + assert_grep "REFUSED" "$case_dir/stderr" \ + "default-symref-failure: teardown did not report a refusal" + assert_present "$case_dir/state/task-x1.meta" \ + "default-symref-failure: teardown removed task metadata" + pass "a failed post-fetch remote default lookup refuses teardown" +} + test_local_only_fork_remote_allows test_teardown_prompts_tasks_axi_done_when_compatible test_teardown_manual_backend_prompts_hand_edit_even_when_tasks_axi_present @@ -2738,6 +3140,16 @@ test_pr_check_records_remote_head_when_local_lags test_content_in_default_fallback_allows test_open_pr_with_sibling_content_in_default_refuses test_merged_pr_with_sibling_content_allows +test_open_pr_on_clean_pushed_branch_refuses +test_missing_worktree_refuses +test_no_pr_unique_pushed_refuses +test_closed_unmerged_pushed_refuses +test_gitlab_merged_squash_tree_allows +test_gitlab_open_refuses +test_unconfirmed_sibling_tree_refuses +test_default_oid_force_push_after_fetch_refuses +test_default_symref_disappears_after_fetch_refuses +test_default_symref_lookup_failure_after_fetch_refuses test_content_fallback_refreshes_stale_origin_ref test_dirty_worktree_refuses test_gh_error_and_content_absent_refuses @@ -2768,3 +3180,5 @@ test_process_spawned_during_grace_is_reaped_on_later_pass test_persistent_scan_refuses_after_bounded_retries test_process_exit_during_identity_lookup_does_not_refuse test_run_abort_precedes_process_reap_precedes_worktree_removal +test_process_reap_mutation_refuses_before_worktree_return +test_playbot_archive_mutation_refuses_before_workspace_deletion diff --git a/tests/fm-watcher-lock.test.sh b/tests/fm-watcher-lock.test.sh index a3628b1694..d3b693a9db 100755 --- a/tests/fm-watcher-lock.test.sh +++ b/tests/fm-watcher-lock.test.sh @@ -310,6 +310,28 @@ test_lock_live_steal_mutex_is_not_reclaimed() { pass "live steal mutex is not reclaimed" } +test_lock_dead_steal_mutex_is_reclaimed_without_recursion() { + local dir state lockdir dead rc newpid + dir=$(make_case lock-dead-stealer) + state="$dir/state" + lockdir="$state/.contend.lock" + dead=$(dead_pid) + mkdir "$lockdir" "$lockdir.steal" + printf '%s\n' "$dead" > "$lockdir/pid" + printf '%s\n' "$dead" > "$lockdir.steal/pid" + rc=0 + newpid=$(FM_LOCK_STALE_AFTER=0 FM_STATE_OVERRIDE="$state" bash -c ' + . "$1" + if fm_lock_try_acquire "$2"; then cat "$2/pid"; else exit 7; fi + ' _ "$LIB" "$lockdir") || rc=$? + [ "$rc" -eq 0 ] || fail "acquirer failed to reclaim a dead steal mutex (rc=$rc)" + [ -n "$newpid" ] && [ "$newpid" != "$dead" ] \ + || fail "reclaimed lock did not record a new owner" + [ ! -e "$lockdir.steal.steal" ] && [ ! -L "$lockdir.steal.steal" ] \ + || fail "dead steal mutex reclamation created a recursive steal chain" + pass "dead steal mutex is reclaimed without recursive steal chains" +} + test_lock_does_not_steal_live_lock() { local dir state lockdir live out lockpid dir=$(make_case lock-live-noop) @@ -1110,6 +1132,7 @@ test_lock_single_winner_under_concurrency test_lock_steals_dead_pid_lock test_lock_stale_steal_single_winner_under_concurrency test_lock_live_steal_mutex_is_not_reclaimed +test_lock_dead_steal_mutex_is_reclaimed_without_recursion test_lock_does_not_steal_live_lock test_lock_empty_pid_uses_minimum_grace test_lock_late_claim_loses_after_recreate diff --git a/tests/lib.sh b/tests/lib.sh index 915741ba0d..1c4711c105 100644 --- a/tests/lib.sh +++ b/tests/lib.sh @@ -202,7 +202,7 @@ fm_git_identity() { fm_git_init_commit() { local dir=$1 mkdir -p "$dir" - git -C "$dir" init -q + git -C "$dir" init -q -b main printf '# %s\n' "$(basename "$dir")" > "$dir/README.md" git -C "$dir" add README.md git -C "$dir" -c user.name='Firstmate Tests' -c user.email='tests@example.invalid' commit -qm initial