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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ tests/fm-session-start.test.sh # fm-session-start.sh: ABSENT vs empty
tests/fm-grok-harness.test.sh # grok adapter spawn hook, token guard, teardown cleanup, and session-lock detection tests
tests/fm-fleet-sync.test.sh # project clone refresh: safe detached recovery, STUCK drift reports, benign skips, and bootstrap relay
tests/fm-x-mode.test.sh # X-mode poll, inbox context round-trip, reply threading, dismiss, completion follow-up counters/caps, dry-run preview, and .env-presence activation tests
tests/fm-tangle-guard.test.sh # primary-checkout tangle detection, read-only remediation suppression, and spawn/brief isolation tests
tests/fm-tangle-guard.test.sh # primary-checkout tangle detection, read-only remediation suppression, spawn/brief isolation, and fm-spawn tmux window construction (append-form creation, name pinning, window-id targeting) tests
tests/fm-brief.test.sh # fm-brief.sh bash -n parse regression guard (issue #166) and clean no-mistakes/direct-PR/local-only brief generation tests
tests/fm-spawn-batch.test.sh # batch dispatch and FM_HOME project-path scoping tests
tests/fm-spawn-dispatch-profile.test.sh # concrete dispatch profile flags: active-profile backstop, harness/model/effort meta, launch templates, batch forwarding, and secondmate exemption
Expand Down
22 changes: 18 additions & 4 deletions bin/backends/tmux.sh
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,28 @@ fm_backend_tmux_container_ensure() {
# fm_backend_tmux_create_task: create the task's window in <proj-abs>,
# refusing an existing <window-name> in <session>. Mirrors fm-spawn.sh's
# duplicate-check-then-new-window sequence, including the exact error text
# (session:window, matching how fm-spawn.sh composed its own $T).
fm_backend_tmux_create_task() { # <session> <window-name> <proj-abs>
local ses=$1 wname=$2 proj_abs=$3
# (session:window, matching how fm-spawn.sh composed its own $T). Prints the
# created window's stable window id on stdout for the caller to target.
#
# Robustness (fm-spawn tmux window handling under a non-default captain config):
# - Capture a STABLE window id with -P -F '#{window_id}', and let tmux append
# at the next free index by targeting the session with a trailing colon
# ("$ses:"), so a non-default base-index (e.g. base-index 1) cannot collide.
# - PIN the window name by disabling automatic-rename and allow-rename on the
# new window: the captain's tmux may rename the window away from fm-<id> once
# treehouse cd's into the worktree, which would break name-based targeting.
# The returned window id lets callers target the window even if its name is ever
# lost, so worktree discovery cannot fall back to the active client's window.
fm_backend_tmux_create_task() { # <session> <window-name> <proj-abs> -> prints window id
local ses=$1 wname=$2 proj_abs=$3 wid
if tmux list-windows -t "$ses" -F '#{window_name}' | grep -qx "$wname"; then
echo "error: window $ses:$wname already exists" >&2
return 1
fi
tmux new-window -d -t "$ses" -n "$wname" -c "$proj_abs"
wid=$(tmux new-window -dP -F '#{window_id}' -t "$ses:" -n "$wname" -c "$proj_abs") || return 1
tmux set-window-option -t "$wid" automatic-rename off 2>/dev/null || true
tmux set-window-option -t "$wid" allow-rename off 2>/dev/null || true
printf '%s\n' "$wid"
}

# fm_backend_tmux_current_path: the live pane's current working directory, or
Expand Down
23 changes: 20 additions & 3 deletions bin/fm-spawn.sh
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,14 @@ case "$BACKEND" in
tmux)
SES=$(fm_backend_tmux_container_ensure)
T="$SES:$W"
fm_backend_tmux_create_task "$SES" "$W" "$PROJ_ABS" || exit 1
# #134 robustness (tmux): fm_backend_tmux_create_task captures a stable window
# id and pins the window name (automatic-rename/allow-rename off) so a captain's
# non-default tmux config cannot rename the window away from fm-<id> once
# treehouse cd's into the worktree. WT_TARGET carries that stable id for the
# rename-critical worktree-detection steps below; the persisted window= handle
# stays $T (the name form), which is safe now that rename is disabled.
WID=$(fm_backend_tmux_create_task "$SES" "$W" "$PROJ_ABS") || exit 1
WT_TARGET="$WID"
;;
herdr)
# fm_backend_herdr_workspace_label resolves the target workspace from
Expand Down Expand Up @@ -750,6 +757,12 @@ EOF
T="$ORCA_TERMINAL"
;;
esac
# #134 robustness: only tmux needs a worktree-detection target distinct from $T -
# its rename-safe stable window id, set as WT_TARGET=$WID in the tmux branch above.
# Every other backend addresses its pane/surface by the id already in $T, so default
# WT_TARGET to $T for them (and for any future backend) - the shared treehouse-get +
# worktree-detection steps below must never reference an unbound WT_TARGET under set -u.
: "${WT_TARGET:=$T}"
spawn_send_text_line() { # <target> <text>
case "$BACKEND" in
tmux) fm_backend_tmux_send_text_line "$1" "$2" ;;
Expand Down Expand Up @@ -786,11 +799,15 @@ spawn_send_key() { # <target> <key>
esac
}
if [ "$KIND" != secondmate ] && [ "$BACKEND" != orca ]; then
spawn_send_text_line "$T" 'treehouse get'
spawn_send_text_line "$WT_TARGET" 'treehouse get'

# Wait for the treehouse subshell: the pane's cwd moves from the project to the worktree.
# Target the stable window id, not the name: if the name is ever lost (e.g. an
# automatic-rename slips through), display-message -t <bad-name> falls back to the
# active client's window, which would misread firstmate's OWN pane path as the
# worktree and tangle a hook into the primary checkout. The window id never lies.
for _ in $(seq 1 60); do
p=$(spawn_current_path "$T" || true)
p=$(spawn_current_path "$WT_TARGET" || true)
if [ -n "$p" ] && [ "$p" != "$PROJ_ABS" ]; then
WT="$p"
break
Expand Down
65 changes: 14 additions & 51 deletions tests/fm-backend.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -713,56 +713,20 @@ SH
printf '%s\n' "$fb"
}

run_spawn_case() { # <bin-root> <fakebin> <log> <state> <data> <config> <proj> -- <spawn args...>
local bin=$1 fb=$2 log=$3 state=$4 data=$5 config=$6 proj=$7; shift 7
[ "${1:-}" = -- ] && shift
: > "$log"
env PATH="$fb:$PATH" FM_ROOT_OVERRIDE="$bin" \
FM_STATE_OVERRIDE="$state" FM_DATA_OVERRIDE="$data" FM_CONFIG_OVERRIDE="$config" \
FM_PROJECTS_OVERRIDE="$TMP_ROOT/unused-projects" \
FM_SPAWN_NO_GUARD=1 TMUX="fake,1,0" FM_TMUX_LOG="$log" \
"$bin/bin/fm-spawn.sh" "$@"
}

test_spawn_conformance_old_vs_new() {
local old_bin fb proj wt data id log_old log_new out_old out_new
local state_old state_new config_old config_new
old_bin=$(build_old_bin spawn-old)
proj="$TMP_ROOT/spawn-project"; wt="$TMP_ROOT/spawn-wt"; data="$TMP_ROOT/spawn-data"
id="spawnconform1"
fm_git_worktree "$proj" "$wt" "fm/$id"
fb=$(make_spawn_fakebin "$TMP_ROOT/spawn-fake" "$wt")
mkdir -p "$data/$id"
printf 'test brief content\n' > "$data/$id/brief.md"
state_old="$TMP_ROOT/spawn-state-old"; state_new="$TMP_ROOT/spawn-state-new"
config_old="$TMP_ROOT/spawn-config-old"; config_new="$TMP_ROOT/spawn-config-new"
mkdir -p "$state_old" "$state_new" "$config_old" "$config_new"
log_old="$TMP_ROOT/spawn-old.log"; log_new="$TMP_ROOT/spawn-new.log"

out_old=$(run_spawn_case "$old_bin" "$fb" "$log_old" "$state_old" "$data" "$config_old" "$proj" -- "$id" "$proj" claude 2>&1)
local rc_old=$?
out_new=$(run_spawn_case "$ROOT" "$fb" "$log_new" "$state_new" "$data" "$config_new" "$proj" -- "$id" "$proj" claude 2>&1)
local rc_new=$?

expect_code 0 "$rc_old" "old fm-spawn.sh should succeed"$'\n'"$out_old"
expect_code 0 "$rc_new" "new fm-spawn.sh should succeed"$'\n'"$out_new"
[ "$out_old" = "$out_new" ] || fail "fm-spawn.sh stdout differs old vs new"$'\n'"--- old ---"$'\n'"$out_old"$'\n'"--- new ---"$'\n'"$out_new"
assert_contains "$out_new" "spawned $id harness=claude kind=ship mode=no-mistakes yolo=off window=firstmate:fm-$id worktree=$wt" \
"spawn output missing the expected summary line"

diff -u "$log_old" "$log_new" > "$TMP_ROOT/spawn-diff.txt" 2>&1 \
|| fail "fm-spawn.sh: tmux command log differs old vs new"$'\n'"$(cat "$TMP_ROOT/spawn-diff.txt")"

# Sanity: the log actually captured the session/window lifecycle so an
# accidentally-empty log (e.g. a fake tmux path typo) cannot pass silently.
assert_contains "$(cat "$log_new")" $'\x1f''new-window' "spawn tmux log missing new-window"
assert_contains "$(cat "$log_new")" $'\x1f''treehouse get' "spawn tmux log missing the treehouse get send"
assert_contains "$(cat "$log_new")" $'\x1f''-l'$'\x1f'"CLAUDE_CODE_ENABLE_PROMPT_SUGGESTION=false claude --dangerously-skip-permissions \"\$(cat '$data/$id/brief.md')\"" \
"spawn tmux log missing the literal launch-command send"

rm -rf "/tmp/fm-$id"
pass "fm-spawn.sh: tmux command log and printed summary line are byte-identical old vs new for a ship-task claude spawn"
}
# NOTE: the old-vs-new spawn command-log conformance test that used to live here
# was retired. It asserted the P1 backend refactor was a byte-for-byte pure
# extraction of the spawn window-creation/targeting sequence, but that sequence
# is now DELIBERATELY changed: fm-spawn drives the tmux backend to capture a
# stable window id, pin the window name (automatic-rename/allow-rename off), and
# target that id for the rename-critical spawn steps (robustness under a
# captain's non-default tmux config). A byte-identical old-vs-new diff can no
# longer hold there by design. That intended sequence is now authoritatively and
# comprehensively verified - via a recording fake-tmux - by
# tests/fm-tangle-guard.test.sh ("fm-spawn: appends windows by session-colon,
# pins the name, and targets the window id"), and the real tmux create/kill path
# by tests/fm-backend-tmux-smoke.test.sh. The send/peek/teardown conformance
# tests below remain pure extractions and stay. (make_spawn_fakebin is retained;
# test_spawn_default_backend_writes_no_meta_field still uses it.)

# --- old vs new: fm-teardown.sh ----------------------------------------------

Expand Down Expand Up @@ -963,7 +927,6 @@ test_resolve_selector_three_forms
test_backend_of_selector_matches_explicit_target_meta
test_send_conformance_old_vs_new
test_peek_conformance_old_vs_new
test_spawn_conformance_old_vs_new
test_teardown_conformance_old_vs_new
test_spawn_refuses_unknown_backend_flag
test_spawn_refuses_unknown_fm_backend_env
Expand Down
88 changes: 88 additions & 0 deletions tests/fm-tangle-guard.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,96 @@ test_spawn_isolation_abort() {
pass "fm-spawn: aborts unless the resolved worktree is a genuine, isolated worktree"
}

# --- GUARD 1c: fm-spawn tmux window construction ----------------------------

# The prevention guard also depends on fm-spawn building robust tmux commands
# under a non-default tmux config (base-index 1, automatic-rename on). A RECORDING
# fake tmux logs every invocation and returns a sentinel window id, so these
# assertions pin the command construction deterministically, with no live tmux:
# - window creation targets the session with a trailing colon (append form), so
# tmux appends at the next free index instead of the active window index, which
# collides under base-index 1;
# - the window id is captured (-P -F #{window_id}) and automatic-rename/allow-rename
# are disabled so the fm-<id> name survives treehouse cd'ing into the worktree;
# - the treehouse-get send-keys and the worktree wait loop target that stable
# window id, never the (possibly-renamed) name - a lost name would let
# display-message fall back to the active client's window and misread firstmate's
# OWN pane as the worktree, tangling a hook into the primary checkout.
make_spawn_record_fakebin() {
local dir=$1 fakebin
fakebin=$(fm_fakebin "$dir")
cat > "$fakebin/tmux" <<'SH'
#!/usr/bin/env bash
set -u
[ -n "${FM_TMUX_REC:-}" ] && printf 'tmux %s\n' "$*" >> "$FM_TMUX_REC"
case "$*" in
*"#{pane_current_path}"*) printf '%s\n' "${FM_FAKE_PANE_PATH:-}"; exit 0 ;;
esac
case "${1:-}" in
display-message) printf 'firstmate\n'; exit 0 ;;
new-window) printf '%s\n' "@spawnwid"; exit 0 ;;
list-windows) exit 0 ;;
has-session|new-session|send-keys|set-window-option) exit 0 ;;
esac
exit 0
SH
chmod +x "$fakebin/tmux"
fm_fake_exit0 "$fakebin" treehouse
printf '%s\n' "$fakebin"
}

run_spawn_record() {
local home=$1 id=$2 proj=$3 pane=$4 fakebin=$5 rec=$6
mkdir -p "$home/data/$id"
printf 'brief\n' > "$home/data/$id/brief.md"
FM_ROOT_OVERRIDE='' FM_HOME="$home" \
FM_STATE_OVERRIDE="$home/state" FM_DATA_OVERRIDE="$home/data" \
FM_PROJECTS_OVERRIDE="$home/projects" FM_CONFIG_OVERRIDE="$home/config" \
FM_SPAWN_NO_GUARD=1 FM_FAKE_PANE_PATH="$pane" TMUX="fake,1,0" \
FM_TMUX_REC="$rec" \
PATH="$fakebin:$PATH" \
"$ROOT/bin/fm-spawn.sh" "$id" "$proj" codex 2>&1
}

test_spawn_tmux_window_construction() {
local home proj fakebin rec wt out status
home="$TMP_ROOT/spawn-rec-home"
mkdir -p "$home/data"
proj=$(make_repo "$TMP_ROOT/spawn-rec-proj")
fakebin=$(make_spawn_record_fakebin "$TMP_ROOT/spawn-rec-fake")
rec="$TMP_ROOT/spawn-rec.log"
: > "$rec"
wt="$TMP_ROOT/spawn-rec-wt"
git -C "$proj" worktree add -q --detach "$wt" >/dev/null 2>&1

out=$(run_spawn_record "$home" rec-win-gg7 "$proj" "$wt" "$fakebin" "$rec"); status=$?
expect_code 0 "$status" "spawn into a genuine worktree should succeed"
assert_contains "$out" "spawned rec-win-gg7" "recording spawn did not report success"

# Bug 1 fix: append-form window creation (trailing colon on the session target).
assert_grep "new-window -dP -F #{window_id} -t firstmate: -n fm-rec-win-gg7" "$rec" \
"new-window must append at the session (trailing colon) and capture the window id"
assert_no_grep "new-window -dP -F #{window_id} -t firstmate -n" "$rec" \
"new-window must not target the bare session name (collides under base-index 1)"

# Bug 2 fix (a): pin the window name against automatic-rename / allow-rename.
assert_grep "set-window-option -t @spawnwid automatic-rename off" "$rec" \
"must disable automatic-rename on the spawned window"
assert_grep "set-window-option -t @spawnwid allow-rename off" "$rec" \
"must disable allow-rename on the spawned window"

# Bug 2 fix (b): treehouse-get and the worktree wait loop target the stable id.
assert_grep "send-keys -t @spawnwid treehouse get Enter" "$rec" \
"treehouse get must be sent to the stable window id"
assert_grep "display-message -p -t @spawnwid #{pane_current_path}" "$rec" \
"the worktree wait loop must query the stable window id, not the name"

pass "fm-spawn: appends windows by session-colon, pins the name, and targets the window id"
}

test_lib_classification
test_guard_banner
test_bootstrap_line
test_brief_assertion_precedes_branch
test_spawn_isolation_abort
test_spawn_tmux_window_construction