fix(bin): select empty worktree-pool slots and attribute live occupancy (land of upstream #1923)

AGENTS.md

@@ -290,7 +290,7 @@ Write the task-specific brief under section 11 before spawning.
 
 Spawn only through `bin/fm-spawn.sh` after the profile and backend checks in section 4.
 The spawn must resolve a genuine isolated task worktree distinct from the primary checkout; a failed isolation assertion stops the task.
-A spawn also refuses before allocating when a pool slot that the worktree pool would hand out still holds live work; the refusal names the slot, the evidence, and the apparent owner, and is a stop-and-investigate result rather than an obstacle to bypass.
+A spawn allocates only a demonstrably empty pool slot and skips any that still holds live work, so it refuses before allocating only when the pool offers no empty slot; that refusal names each slot, its evidence, and the apparent owner, and is a stop-and-investigate result rather than an obstacle to bypass.
 Every ship and scout spawn must also answer why an agent turn was necessary, through `--reason-code` in the closed vocabulary owned by `bin/fm-reasoning-lib.sh`; a broken or absent deterministic reader is never legitimate reasoning demand, so it is `TOOLING_GAP`, which is never counted as justified reasoning and requires `--tooling-gap-item` naming the open backlog item that repairs the reader.
 After spawning, confirm the worker is processing the brief, handle any trust dialog through `harness-adapters`, and record ship or scout work as under way.
 A persistent secondmate is recorded in the secondmate registry and runtime state, never as a backlog work item.

bin/fm-spawn.sh

@@ -848,6 +848,19 @@ SPAWN_TASK_LOCK=
 SPAWN_TASK_LOCK_HELD=0
 CONFIG_INHERIT_LOCK=
 CONFIG_INHERIT_LOCK_HELD=0
+SLOT_HOLDER_PID=
+WT_POOL_LOCK=
+WT_POOL_LOCK_HELD=0
+
+# Stop occupying the chosen pool slot. Called as soon as the pane's own shell is
+# inside it, and again from the abort path so an aborted spawn never leaves the
+# slot looking in-use to the rest of the fleet.
+release_slot_holder() {
+  [ -n "$SLOT_HOLDER_PID" ] || return 0
+  kill "$SLOT_HOLDER_PID" 2>/dev/null || true
+  wait "$SLOT_HOLDER_PID" 2>/dev/null || true
+  SLOT_HOLDER_PID=
+}
 
 parse_orca_worktree_result() {
   local raw=$1 rest
@@ -868,6 +881,7 @@ parse_orca_worktree_result() {
 
 spawn_abort_cleanup() {
   local status=$?
+  release_slot_holder
   if [ "$HERDR_PROJECTION_ABORT_CLEANUP" = 1 ] \
      && [ "$HERDR_PRESENTATION_ORDER_LOCK_HELD" != 1 ]; then
     if ! spawn_herdr_presentation_order_lock_acquire "${HERDR_PROJECTION_ABORT_SESSION:-}"; then
@@ -915,6 +929,10 @@ spawn_abort_cleanup() {
       fi
     fi
   fi
+  if [ "$WT_POOL_LOCK_HELD" = 1 ]; then
+    WT_POOL_LOCK_HELD=0
+    fm_lock_release "$WT_POOL_LOCK" || true
+  fi
   if [ "$SPAWN_TASK_LOCK_HELD" = 1 ]; then
     SPAWN_TASK_LOCK_HELD=0
     fm_lock_release "$SPAWN_TASK_LOCK" || true
@@ -953,6 +971,90 @@ spawn_herdr_presentation_order_lock_release() {
   fm_lock_release "$HERDR_PRESENTATION_ORDER_LOCK" || true
 }
 
+# One machine-private exclusive lock per physical pool, keyed by the resolved
+# project path. Unlike `treehouse get`, the select-then-enter allocation claims
+# nothing until the holder process occupies the chosen slot, so two spawns
+# selecting concurrently would both pick the same first-clean slot. The
+# contenders are every home spawning into the pool, so the lock can never live
+# under $STATE, which is per-home; it uses the same machine-private /tmp
+# namespace shape as the herdr presentation lock (bin/backends/herdr.sh), with
+# the same ownership and mode validation before use.
+spawn_pool_select_lock_namespace_mode() {
+  if [ "$(uname -s 2>/dev/null)" = Darwin ]; then
+    stat -f '%Lp' "$1" 2>/dev/null
+  else
+    stat -c '%a' "$1" 2>/dev/null
+  fi
+}
+
+spawn_pool_select_lock_namespace_uid() {
+  if [ "$(uname -s 2>/dev/null)" = Darwin ]; then
+    stat -f '%u' "$1" 2>/dev/null
+  else
+    stat -c '%u' "$1" 2>/dev/null
+  fi
+}
+
+spawn_pool_select_lock_namespace_valid() {
+  local dir=$1 expected_uid owner mode
+  [ -d "$dir" ] && [ ! -L "$dir" ] || return 1
+  expected_uid=$(id -u 2>/dev/null) || return 1
+  owner=$(spawn_pool_select_lock_namespace_uid "$dir") || return 1
+  mode=$(spawn_pool_select_lock_namespace_mode "$dir") || return 1
+  [ "$owner" = "$expected_uid" ] && [ "$mode" = 700 ]
+}
+
+spawn_pool_select_lock_path() {  # <pool-real>
+  local pool=$1 hash key dir
+  [ -n "$pool" ] || return 1
+  if command -v shasum >/dev/null 2>&1; then
+    hash=$(printf '%s' "$pool" | shasum -a 256 2>/dev/null | awk '{print $1}')
+  elif command -v sha256sum >/dev/null 2>&1; then
+    hash=$(printf '%s' "$pool" | sha256sum 2>/dev/null | awk '{print $1}')
+  else
+    return 1
+  fi
+  [ -n "$hash" ] || return 1
+  key=${hash:0:32}
+  dir=/tmp/firstmate-worktree-pool
+  if [ ! -e "$dir" ] && [ ! -L "$dir" ]; then
+    if ! mkdir -m 700 "$dir" 2>/dev/null; then
+      spawn_pool_select_lock_namespace_valid "$dir" || return 1
+    fi
+  fi
+  spawn_pool_select_lock_namespace_valid "$dir" || return 1
+  printf '%s/select-%s.lock' "$dir" "$key"
+}
+
+# A lock that cannot be acquired refuses the spawn: proceeding unlocked would
+# reopen the very race the lock closes. The wait is bounded well above the
+# window one holder spans (selection through the pane settling into the slot).
+spawn_pool_select_lock_acquire() {  # <pool-real>
+  local pool=$1 attempt max
+  if ! WT_POOL_LOCK=$(spawn_pool_select_lock_path "$pool"); then
+    echo "error: cannot resolve the pool selection lock for $pool: /tmp/firstmate-worktree-pool is not this user's mode-700 directory and could not be created; refusing to choose a slot unserialized" >&2
+    return 1
+  fi
+  attempt=0
+  max=${FM_SPAWN_POOL_LOCK_POLLS:-1200}
+  while [ "$attempt" -lt "$max" ]; do
+    if fm_lock_try_acquire "$WT_POOL_LOCK"; then
+      WT_POOL_LOCK_HELD=1
+      return 0
+    fi
+    sleep 0.1
+    attempt=$((attempt + 1))
+  done
+  echo "error: another spawn is choosing a slot in this pool (lock $WT_POOL_LOCK held by pid ${FM_LOCK_HELD_PID:-unknown}); refusing to choose a slot unserialized" >&2
+  return 1
+}
+
+spawn_pool_select_lock_release() {
+  [ "$WT_POOL_LOCK_HELD" = 1 ] || return 0
+  WT_POOL_LOCK_HELD=0
+  fm_lock_release "$WT_POOL_LOCK" || true
+}
+
 # Batch dispatch (see header): when the first positional is an `id=repo` pair, treat every
 # positional as one and spawn each by re-execing this script in single-task mode. We use
 # the FM_ROOT path (not $0) so it works whatever cwd or relative path invoked us, and reuse
@@ -1516,15 +1618,45 @@ real_path_or_raw() {  # <path>
   fi
 }
 
-# Pool-allocation safety, BEFORE any endpoint exists. `treehouse get` resets a
-# slot as part of acquiring it, so the only place this can be checked is ahead
-# of the allocation - by the time the pane's cwd reveals the worktree, an
-# unlanded branch has already been detached. Running it here (rather than beside
-# the `treehouse get` send below) also means a refusal never leaves an orphan
-# window behind. Secondmate spawns own their home, and Orca owns its own
-# worktree, so neither goes through the treehouse pool.
+# Pool allocation, BEFORE any endpoint exists. `treehouse get` resets a slot as
+# part of acquiring it, so the only place this can be decided is ahead of the
+# allocation - by the time the pane's cwd reveals the worktree, an unlanded
+# branch has already been detached. Running it here (rather than beside the
+# acquire below) also means a refusal never leaves an orphan window behind.
+# Secondmate spawns own their home, and Orca owns its own worktree, so neither
+# goes through the treehouse pool.
+#
+# The guard both refuses and CHOOSES: `treehouse get` takes no slot argument and
+# hands out the first available slot, so one parked slot would otherwise
+# blockade a pool whose later slots are empty. A named slot is entered by name
+# below; empty output means the pool offered nothing to steer to and the
+# allocation falls back to `treehouse get`.
+#
+# The whole select-to-enter window runs under the cross-home pool lock:
+# `select` claims nothing by itself, so a second spawn selecting concurrently
+# would deterministically choose the same slot. The lock is released once the
+# pane has settled into the slot, alongside the holder, or by the abort path;
+# when nothing was selected it is released right here, because the `treehouse
+# get` fallback claims its slot atomically.
+WT_SLOT_NAME=
+WT_SLOT_REAL=
 if [ "$KIND" != secondmate ] && [ "$BACKEND" != orca ]; then
-  "$FM_ROOT/bin/fm-worktree-guard.sh" check "$PROJ_ABS_REAL" || exit 1
+  spawn_pool_select_lock_acquire "$PROJ_ABS_REAL" || exit 1
+  wt_selection=$("$FM_ROOT/bin/fm-worktree-guard.sh" select "$PROJ_ABS_REAL") || exit 1
+  if [ -n "$wt_selection" ]; then
+    WT_SLOT_NAME=${wt_selection%%$'\t'*}
+    WT_SLOT_REAL=$(real_path_or_raw "${wt_selection#*$'\t'}")
+    # Hold the chosen slot from the moment it is chosen until the pane's own
+    # shell is inside it. treehouse reports a slot in-use, and `get` skips it,
+    # while ANY process's cwd is inside it (docs/verification/worktree-allocation.md),
+    # so this occupies the slot across the window in which another home's
+    # `treehouse get` could otherwise still take it. The abort path releases it
+    # too, so a spawn that never got started leaves nothing occupied.
+    ( cd "$WT_SLOT_REAL" 2>/dev/null && exec sleep 300 ) >/dev/null 2>&1 &
+    SLOT_HOLDER_PID=$!
+  else
+    spawn_pool_select_lock_release
+  fi
 fi
 
 # Session-provider container-ensure + task creation. tmux stays exactly as P1
@@ -1956,7 +2088,14 @@ kimi_spawn_fail() {  # <detail>
 }
 
 if [ "$KIND" != secondmate ] && [ "$BACKEND" != orca ]; then
-  spawn_send_text_line "$WT_TARGET" 'treehouse get'
+  if [ -n "$WT_SLOT_NAME" ]; then
+    # `enter` is deliberate: unlike `get` it acquires the slot chosen above
+    # without resetting it, so the slot base placement further below stays the
+    # only thing that moves this worktree's HEAD.
+    spawn_send_text_line "$WT_TARGET" "treehouse enter $(shell_quote "$WT_SLOT_NAME")"
+  else
+    spawn_send_text_line "$WT_TARGET" 'treehouse get'
+  fi
 
   # 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
@@ -1978,12 +2117,17 @@ if [ "$KIND" != secondmate ] && [ "$BACKEND" != orca ]; then
   # a mismatch just becomes the new candidate rather than resetting the wait, so a
   # pane that is already settled by the first real read only costs the one existing
   # inter-poll sleep as confirmation, not a whole extra cycle on top.
+  #
+  # When a slot was chosen by name, the pane must arrive at THAT slot: any other
+  # path means the pane went somewhere this spawn never inspected, which is
+  # exactly what must never be recorded as the worktree.
   candidate=""
   for _ in $(seq 1 60); do
     p=$(spawn_current_path "$WT_TARGET" || true)
     if [ -n "$p" ]; then
       p_real=$(real_path_or_raw "$p")
-      if [ "$p_real" != "$PROJ_ABS_REAL" ]; then
+      if [ "$p_real" != "$PROJ_ABS_REAL" ] \
+         && { [ -z "$WT_SLOT_REAL" ] || [ "$p_real" = "$WT_SLOT_REAL" ]; }; then
         if [ -n "$candidate" ] && [ "$p_real" = "$candidate" ]; then
           WT="$p"
           break
@@ -1997,12 +2141,22 @@ if [ "$KIND" != secondmate ] && [ "$BACKEND" != orca ]; then
     fi
     sleep 1
   done
+  release_slot_holder
+  spawn_pool_select_lock_release
   if [ -z "$WT" ]; then
-    echo "error: treehouse get did not enter a worktree within 60s; inspect window $T" >&2
+    if [ -n "$WT_SLOT_NAME" ]; then
+      echo "error: 'treehouse enter $WT_SLOT_NAME' did not enter $WT_SLOT_REAL within 60s; inspect window $T" >&2
+    else
+      echo "error: treehouse get did not enter a worktree within 60s; inspect window $T" >&2
+    fi
     exit 1
   fi
 
-  validate_spawn_worktree "treehouse get" "$T"
+  if [ -n "$WT_SLOT_NAME" ]; then
+    validate_spawn_worktree "treehouse enter $WT_SLOT_NAME" "$T"
+  else
+    validate_spawn_worktree "treehouse get" "$T"
+  fi
 fi
 
 # Place the slot at the resolved slot base, so the worker reads the code the