From 23e71b3d2a164c39b449a36524fe108b3eae3f15 Mon Sep 17 00:00:00 2001 From: Thoughts-One <119157221+Thoughts-One@users.noreply.github.com> Date: Wed, 7 Oct 2026 12:22:53 +0200 Subject: [PATCH 1/7] feat(bin): add opt-in --herdr-resume-lock-wait to fm-spawn (#6649) * fix(herdr): make exact-resume presentation-lock wait instead of a bounded timeout The exact-resume path in bin/fm-spawn.sh used the same 50-attempt-then- give-up lock acquire as the new-task-create path, but the two paths are not equivalent on contention: a create has no prior state to strand and can safely fall back to a flat layout, while a resume is recovering a specific existing identity that a concurrent recovery may legitimately be holding the lock for. Giving up there does not degrade gracefully, it hard-fails the resume outright. The suite's own concurrent cross-home recoveries test already asserts both concurrent recoveries succeed with a genuine reclaim, and the file's header comment already (inaccurately) claimed lock contention falls back to the ordinary flat layout for both paths alike, so the intended contract was always that recoveries serialize and both succeed, not that either one refuses under a short bound. Give spawn_herdr_presentation_order_lock_acquire a wait mode that uses this file's own established fm_lock_acquire_wait idiom (already used for its other fleet-shared locks) instead of the bounded loop, and use it only at the exact-resume call site. The new-task-create call site is unchanged and keeps its bounded-then-flat-fallback behavior, which is already covered by its own passing test. Dead-owner PID-liveness reclaim inside fm_lock_try_acquire still bounds the wait against a holder that crashed mid-hold. Adds a deterministic regression test that holds the shared session lock from an unrelated process for well past the old bound, then asserts the resume succeeds with a genuine reclaim and took close to the full hold duration, so a fix that merely widens the bound rather than genuinely waiting is still caught. The existing concurrent cross-home recovery test exercises this under real timing but does not reliably outlast a fixed bound on its own. Corrects the header comment's claim that create and resume share one bounded-then-flat-fallback behavior on lock contention; they no longer do. * no-mistakes(document): Document Herdr recovery waiting for presentation lock * no-mistakes(document): Update stale hard-refusal claim in verification log * no-mistakes(ci): Fixed the Greptile finding on tests/fm-backend-herdr-presentation-e2e.test.sh:1389 by bounding the resume lock-wait regression's spawn_task call. Added an optional 4th `deadline_seconds` arg to the `spawn_task` helper (defaults to empty, so all ~20 other existing call sites are unaffected and unwrapped by `timeout`). The lock-wait test now passes `LOCK_WAIT_HOLD_SECONDS + 60` (90s) as the deadline, and a dedicated check for exit code 124 emits a clear "hung for over Xs instead of waiting out a Ys lock hold" diagnostic before falling through to the existing pass/fail assertions, which are unchanged. No product code was touched. Verified with `bash -n`, `shellcheck -x` (no warnings), a standalone reproduction of the timeout/no-timeout/success paths, the project's `bin/fm-lint.sh --fast` on the file (clean), and the full `tests/fm-lint.test.sh` suite (all 46 assertions pass) * no-mistakes(ci): Replaced the direct `timeout "$deadline_seconds"` call in `spawn_task()` (tests/fm-backend-herdr-presentation-e2e.test.sh) with the repo's portable bounded-execution helper: sourced `bin/fm-timeout-lib.sh` at the top of the file and changed `deadline_cmd=(timeout "$deadline_seconds")` to `deadline_cmd=(fm_run_timed "$deadline_seconds")`. This removes the GNU/BSD `timeout` dependency that would fail with exit 127 on a stock macOS host without coreutils, while preserving identical semantics (exit 124 on bound-hit, command's own exit otherwise), which the existing `[ "$LOCK_WAIT_STATUS" -eq 124 ]` diagnostic check already relies on. Verified: `bash -n` syntax check, `bin/fm-lint.sh --fast` clean, full `tests/fm-lint.test.sh` suite (46/46 pass), and a standalone repro confirming `fm_run_timed` returns 124 on timeout and 0 on success identically to the prior `timeout` call. No other direct `timeout` calls exist in this file or elsewhere in the PR's diff, so no sibling sites remain * fix(herdr): gate exact-resume lock wait behind --herdr-resume-lock-wait Keep refuse-by-default on presentation-order lock contention for Herdr exact resume. Callers that need concurrent recoveries to serialize must pass --herdr-resume-lock-wait; unbounded blocking on a third-party session lock is never the default. Update docs and the real-Herdr e2e suite so the default path asserts the refusal and the opt-in path asserts the wait. * no-mistakes(test): Fix e2e test's lost exit status after if/fi with no else branch * docs(herdr): stop advertising --herdr-resume-lock-wait on --relaunch The relaunch path reuses the recorded endpoint and never takes the presentation-order lock, so the flag is inert there. Drop it from the --relaunch usage line and state where the flag applies. * no-mistakes(review): Clarify lock-wait docs; simplify bash-3.2-safe spawn_task helper * no-mistakes(ci): Fixed ci-1 (Greptile P2). In tests/fm-backend-herdr-presentation-e2e.test.sh, the failure cleanup `cleanup_all` stopped only `LOCK_CONTENTION_OWNER_PID`. It now also stops `LOCK_REFUSE_HOLDER_PID` and `LOCK_WAIT_HOLDER_PID`, the holders of the two new contention cases, so a `fail` before their explicit `wait` no longer leaves them running. Both new PIDs are initialised empty next to the existing one, and each is cleared right after its successful `wait` so cleanup never touches a finished PID. I changed nothing else. `bash -n` passes. The real Herdr e2e run passed both new cases ("default resumed identity refuses session lock contention" and "--herdr-resume-lock-wait waits out session lock contention instead of refusing"). The full run hit my 550s timeout in a later, unrelated case, after the new cases passed --- bin/fm-spawn.sh | 75 ++++++--- docs/herdr-backend.md | 9 +- docs/verification/runtime-backends.md | 5 +- .../fm-backend-herdr-presentation-e2e.test.sh | 150 +++++++++++++++++- 4 files changed, 210 insertions(+), 29 deletions(-) diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 0e58e9c29c0..983633cb5ec 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -1,8 +1,8 @@ #!/usr/bin/env bash # Spawn a direct report: a crewmate in a treehouse or Orca worktree, or a # secondmate in its isolated firstmate home. -# Usage: fm-spawn.sh --mode --yolo [--branch-prefix ] [--base-branch ] [--harness |harness|launch-command] [--model ] [--effort ] [--backend ] -# fm-spawn.sh --scout [--base-branch ] [--harness |harness|launch-command] [--model ] [--effort ] [--backend ] +# Usage: fm-spawn.sh --mode --yolo [--branch-prefix ] [--base-branch ] [--harness |harness|launch-command] [--model ] [--effort ] [--backend ] [--herdr-resume-lock-wait] +# fm-spawn.sh --scout [--base-branch ] [--harness |harness|launch-command] [--model ] [--effort ] [--backend ] [--herdr-resume-lock-wait] # fm-spawn.sh [] [--harness |harness|launch-command] [--model ] [--effort ] [--backend ] --secondmate # --mode and --yolo are this task's delivery contract, REQUIRED for every ship # spawn and refused on --scout and --secondmate spawns. Firstmate resolves both @@ -140,14 +140,23 @@ # authority, and every ambiguous recovery stays on the flat fallback after # duplicate-agent risk is independently absent. Treehouse allocation and task # metadata are unchanged. -# A clean projected create or exact resume makes one bounded attempt to hold -# the one session-scoped presentation-order lock (keyed by named session plus -# canonical socket, outside any home's state/) through launch handoff. Lock -# contention warns and falls back to the ordinary flat layout before any -# projection mutation. The exact response-derived new workspace is inserted -# immediately after its owning parent (firstmate or 2ndmate-) contiguous -# child block. Ordering never authorizes lifecycle cleanup, and any -# unavailable, ambiguous, or failed move warns while the spawn continues. +# A clean projected create and an exact resume both hold the one +# session-scoped presentation-order lock (keyed by named session plus +# canonical socket, outside any home's state/) through launch handoff. +# On contention a create makes one bounded attempt and falls back to the +# ordinary flat layout before any projection mutation. A resume refuses by +# default on the same contention (it does not degrade flat; a concurrent +# resume is a hard failure). Pass --herdr-resume-lock-wait to opt that +# resume into waiting for the lock instead, so two concurrent recoveries +# can serialize and each still replace its own exact husk. The flag acts +# only on that fresh ship or scout spawn path: --relaunch reuses the +# recorded endpoint without taking this lock, so the flag has no effect +# there, and a secondmate spawn never projects. Unbounded +# blocking on a third-party session lock is never the default. The exact +# response-derived new workspace is inserted immediately after its owning +# parent (firstmate or 2ndmate-) contiguous child block. Ordering never +# authorizes lifecycle cleanup, and any unavailable, ambiguous, or failed +# move warns while the spawn continues. # Every projected create, prune, and move captures and verifies the named # session's exact active workspace and tab. A detected focus change restores # only that exact tab id; an ambiguous pre-operation snapshot refuses the @@ -676,6 +685,9 @@ BASE_BRANCH= BASE_BRANCH_SET=0 TRACEPARENT_SET=0 RELAUNCH=0 +# Opt-in only: exact-resume presentation-order lock waits instead of refusing. +# Absent/unset keeps upstream refuse-on-contention. See header. +HERDR_RESUME_LOCK_WAIT=0 POS=() want_value= for a in "$@"; do @@ -741,6 +753,7 @@ for a in "$@"; do KIND_SET=1 ;; --relaunch) RELAUNCH=1 ;; + --herdr-resume-lock-wait) HERDR_RESUME_LOCK_WAIT=1 ;; --harness) want_value=harness ;; --harness=*) HARNESS_ARG=${a#--harness=} @@ -1419,14 +1432,30 @@ spawn_abort_cleanup() { } trap spawn_abort_cleanup EXIT -# One bounded lock per live Herdr session/socket, shared across all homes. -# is required so secondmate and primary spawns serialize against the -# same session without writing any other home's state directory. +# One lock per live Herdr session/socket, shared across all homes. +# is required so secondmate and primary spawns serialize against the same +# session without writing any other home's state directory. +# +# Default mode is one BOUNDED attempt. A clean create uses that default and +# falls back to the ordinary flat layout on contention. An exact resume also +# defaults to the bounded attempt and hard-refuses on contention (it does not +# degrade flat). Passing mode `wait` makes this call WAIT for the lock instead +# (`fm_lock_acquire_wait`, the same unbounded-wait idiom this file already uses +# for its other fleet-shared locks). Only the recovery path under the explicit +# --herdr-resume-lock-wait opt-in passes `wait`, so unbounded blocking on a +# third-party session lock never becomes the default for every caller. +# Dead-owner reclaim inside `fm_lock_try_acquire` still bounds a wait against a +# holder that crashed mid-hold. spawn_herdr_presentation_order_lock_acquire() { - local session=${1:-} attempt lock_path + local session=${1:-} mode=${2:-} attempt lock_path [ -n "$session" ] || session=$(fm_backend_herdr_session) lock_path=$(fm_backend_herdr_presentation_session_lock_path "$session") || return 1 HERDR_PRESENTATION_ORDER_LOCK="$lock_path" + if [ "$mode" = wait ]; then + fm_lock_acquire_wait "$HERDR_PRESENTATION_ORDER_LOCK" + HERDR_PRESENTATION_ORDER_LOCK_HELD=1 + return 0 + fi attempt=0 while [ "$attempt" -lt 50 ]; do if fm_lock_try_acquire "$HERDR_PRESENTATION_ORDER_LOCK"; then @@ -1502,6 +1531,7 @@ if [ "${#POS[@]}" -gt 0 ] && [ "${POS[0]}" != "$idpart" ] && case "$idpart" in * [ "$YOLO_SET" -eq 0 ] || shared_args+=(--yolo "$YOLO") [ "$BRANCH_PREFIX_SET" -eq 0 ] || shared_args+=(--branch-prefix "$BRANCH_PREFIX") [ "$BASE_BRANCH_SET" -eq 0 ] || shared_args+=(--base-branch "$BASE_BRANCH") + [ "$HERDR_RESUME_LOCK_WAIT" -eq 0 ] || shared_args+=(--herdr-resume-lock-wait) for pair in "${POS[@]}"; do case "$pair" in *=*) : ;; @@ -3712,10 +3742,19 @@ else echo "error: herdr presentation recovery could not ensure its exact named session" >&2 exit 1 } - spawn_herdr_presentation_order_lock_acquire "$HERDR_SES" || { - echo "error: herdr presentation recovery could not acquire its session lock; refusing a concurrent resume" >&2 - exit 1 - } + # Refuse-by-default on contention. Wait only when the caller opted in + # with --herdr-resume-lock-wait (see header). + if [ "$HERDR_RESUME_LOCK_WAIT" = 1 ]; then + spawn_herdr_presentation_order_lock_acquire "$HERDR_SES" wait || { + echo "error: herdr presentation recovery could not resolve its session lock" >&2 + exit 1 + } + else + spawn_herdr_presentation_order_lock_acquire "$HERDR_SES" || { + echo "error: herdr presentation recovery could not acquire its session lock; refusing a concurrent resume" >&2 + exit 1 + } + fi if [ -e "$STATE/$ID.meta" ] || [ -L "$STATE/$ID.meta" ]; then herdr_projection_existing_meta_allows_flat "$STATE/$ID.meta" || exit 1 fi diff --git a/docs/herdr-backend.md b/docs/herdr-backend.md index c28afacd8be..ac111a097a6 100644 --- a/docs/herdr-backend.md +++ b/docs/herdr-backend.md @@ -457,8 +457,13 @@ Any of these preserves the candidate and lets session startup continue with at m - A failed journal publication or projected workspace create stops that spawn instead of falling back flat. So a Herdr create failure surfaces as a spawn failure in every Herdr home, rather than only in homes that opted in. Every earlier degradation on the fresh projected-create path (no session server, contended presentation lock, absent or ambiguous parent) still warns and continues flat. -- Recovery of an existing presentation journal deliberately refuses the spawn when the shared presentation lock is contended, rather than falling back flat. - Default-on makes that refusal reachable in any Herdr home. +- Recovery of an existing presentation journal refuses by default when the shared presentation lock is contended, rather than falling back flat. + Pass `fm-spawn.sh --herdr-resume-lock-wait` to opt that recovery into waiting for the lock instead, so concurrent recoveries can serialize. + The flag applies to a fresh ship or scout spawn that recovers a journal. + The multi-task path forwards the flag to each per-pair spawn. + `fm-spawn.sh --relaunch` and `--secondmate` take no exact-resume presentation-order lock, so the flag has no effect there. + Dead-owner reclaim still stops the wait when a holder crashed. + Unbounded blocking on the session lock is never the default. - Existing layouts are not force-renamed or rearranged. - Missing or ambiguous restart bindings fall back to the ordinary home workspace while the old projection remains untouched. - Crashes, lost responses, failed exact-pane cleanup, or human renames can leave quarantined spaces. diff --git a/docs/verification/runtime-backends.md b/docs/verification/runtime-backends.md index ce429421552..7fe1ba7df17 100644 --- a/docs/verification/runtime-backends.md +++ b/docs/verification/runtime-backends.md @@ -1424,7 +1424,10 @@ ok - real Herdr lab validation completed on Herdr 0.8.0 with the default-session ``` The projected spawn in that run used the historical empty opt-in file, so a home that had already enabled the projection keeps it without any migration step. -One concurrent cross-home recovery case refused under contention on a loaded machine and passed on an immediate rerun; recovery-path presentation lock contention is a deliberate hard refusal rather than a flat fallback, which default-on now makes reachable from any Herdr home. +One concurrent cross-home recovery case refused under contention on a loaded machine and passed on an immediate rerun; recovery-path presentation lock contention remains a deliberate hard refusal by default rather than a flat fallback, which default-on makes reachable from any Herdr home. +Callers that need concurrent recoveries to serialize can pass `fm-spawn.sh --herdr-resume-lock-wait`. +The flag applies to a fresh ship or scout spawn, and the multi-task path forwards it to each per-pair spawn. +It has no effect on `--relaunch` and `--secondmate`, because those paths take no exact-resume presentation-order lock. That run measured the default-on projection on Herdr 0.8.0 only, while the focus-flash regression below was last run on 0.7.5 before the flip, so neither run covered a defective release under default-on projection; the version floor and the focus-flash suite's Part C close that gap. The restored-shell session-start cleanup ran on 2026-07-24 against Herdr 0.7.5 protocol 17: diff --git a/tests/fm-backend-herdr-presentation-e2e.test.sh b/tests/fm-backend-herdr-presentation-e2e.test.sh index 4aef0afc349..1be3a2054ea 100755 --- a/tests/fm-backend-herdr-presentation-e2e.test.sh +++ b/tests/fm-backend-herdr-presentation-e2e.test.sh @@ -8,6 +8,8 @@ set -u ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" HERDR_LAB_HELPER=${HERDR_LAB_HELPER:-$ROOT/bin/fm-herdr-lab.sh} +# shellcheck source=bin/fm-timeout-lib.sh +. "$ROOT/bin/fm-timeout-lib.sh" fail() { printf 'not ok - %s\n' "$1" >&2; cleanup_all; exit 1; } pass() { printf 'ok - %s\n' "$1"; } @@ -278,13 +280,18 @@ export HERDR_SESSION="$HERDR_LAB_SESSION" HERDR_LAB_SESSION LAB_READY=0 RECORDED_WORKTREES="" LOCK_CONTENTION_OWNER_PID= +LOCK_REFUSE_HOLDER_PID= +LOCK_WAIT_HOLDER_PID= cleanup_all() { - local wt - if [ -n "$LOCK_CONTENTION_OWNER_PID" ]; then - kill "$LOCK_CONTENTION_OWNER_PID" 2>/dev/null || true - wait "$LOCK_CONTENTION_OWNER_PID" 2>/dev/null || true - LOCK_CONTENTION_OWNER_PID= - fi + local wt pid + for pid in "$LOCK_CONTENTION_OWNER_PID" "$LOCK_REFUSE_HOLDER_PID" "$LOCK_WAIT_HOLDER_PID"; do + [ -n "$pid" ] || continue + kill "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + done + LOCK_CONTENTION_OWNER_PID= + LOCK_REFUSE_HOLDER_PID= + LOCK_WAIT_HOLDER_PID= while IFS= read -r wt; do [ -n "$wt" ] || continue [ -d "$wt" ] || continue @@ -406,10 +413,14 @@ Verify projected workspace behavior for $id. EOF } -spawn_task() { # +spawn_task() { # [extra fm-spawn args...]; SPAWN_DEADLINE_SECONDS bounds the run local id=$1 home=$2 project=$3 + shift 3 + local -a deadline_cmd=() + [ -z "${SPAWN_DEADLINE_SECONDS:-}" ] || deadline_cmd=(fm_run_timed "$SPAWN_DEADLINE_SECONDS") FM_GATE_REFUSE_BYPASS=1 FM_SPAWN_NO_GUARD=1 FM_HOME="$home" FM_ROOT_OVERRIDE="$ROOT" \ - "$ROOT/bin/fm-spawn.sh" "$id" "$project" "sh -c 'while :; do sleep 60; done'" --mode no-mistakes --yolo off --backend herdr + ${deadline_cmd[@]+"${deadline_cmd[@]}"} "$ROOT/bin/fm-spawn.sh" "$id" "$project" "sh -c 'while :; do sleep 60; done'" \ + --mode no-mistakes --yolo off --backend herdr "$@" } finish_concurrent_spawn() { # @@ -1349,6 +1360,129 @@ teardown_task "$BRAVO_WAVE_ID" "$SECOND_HOME_B" > "$TMP_ROOT/bravo-wave-teardown "$REAL_TREEHOUSE" return --force "$BRAVO_WAVE_NEW_WT" >/dev/null 2>&1 || true pass "real Herdr lab: concurrent cross-home recoveries replace exact husks under one session lock with no focus drift" +# Exact-resume presentation-lock contention refuses by default. Hold the +# shared session lock from an unrelated process past the bounded-retry window +# and assert the default resume hard-refuses without the opt-in flag. +LOCK_REFUSE_ID=lock-refuse-resume-r1 +mkdir -p "$HOME_DIR/data/$LOCK_REFUSE_ID" +write_ship_brief "$HOME_DIR" "$LOCK_REFUSE_ID" 'Resume lock-refuse fixture.' +spawn_task "$LOCK_REFUSE_ID" "$HOME_DIR" "$RECOVERY_PROJECT_DIR" > "$TMP_ROOT/lock-refuse-first.out" 2> "$TMP_ROOT/lock-refuse-first.err" \ + || fail "lock-refuse recovery fixture failed: $(cat "$TMP_ROOT/lock-refuse-first.err")" +LOCK_REFUSE_META="$HOME_DIR/state/$LOCK_REFUSE_ID.meta" +LOCK_REFUSE_OLD_WT=$(remember_meta_worktree "$LOCK_REFUSE_META") +LOCK_REFUSE_OLD_PANE=$(grep '^herdr_pane_id=' "$LOCK_REFUSE_META" | cut -d= -f2-) +PATH="$HERDR_ORIGINAL_PATH" "$HERDR_LAB_HELPER" stop "$HERDR_LAB_SESSION" >/dev/null \ + || fail "could not stop the isolated session for resume lock-refuse" +PATH="$HERDR_ORIGINAL_PATH" "$HERDR_LAB_HELPER" provision "$HERDR_LAB_SESSION" \ + || fail "could not reprovision the isolated session for resume lock-refuse" + +LOCK_REFUSE_READY="$TMP_ROOT/lock-refuse-ready" +LOCK_REFUSE_HOLD_SECONDS=15 +LOCK_REFUSE_PATH=$(session_presentation_lock_path) \ + || fail "could not resolve session lock for resume lock-refuse" +ROOT="$ROOT" READY="$LOCK_REFUSE_READY" HOLD="$LOCK_REFUSE_HOLD_SECONDS" LOCK="$LOCK_REFUSE_PATH" bash -c ' + . "$ROOT/bin/fm-wake-lib.sh" + fm_lock_try_acquire "$LOCK" || exit 1 + : > "$READY" + sleep "$HOLD" + fm_lock_release "$LOCK" +' & +LOCK_REFUSE_HOLDER_PID=$! +while [ ! -e "$LOCK_REFUSE_READY" ] && kill -0 "$LOCK_REFUSE_HOLDER_PID" 2>/dev/null; do sleep 0.01; done +[ -e "$LOCK_REFUSE_READY" ] || fail "could not hold the session presentation lock for resume lock-refuse" + +LOCK_REFUSE_FOCUS=$(focus_snapshot) +if spawn_task "$LOCK_REFUSE_ID" "$HOME_DIR" "$RECOVERY_PROJECT_DIR" \ + > "$TMP_ROOT/lock-refuse-resume.out" 2> "$TMP_ROOT/lock-refuse-resume.err"; then + LOCK_REFUSE_STATUS=0 +else + LOCK_REFUSE_STATUS=$? +fi +if [ "$LOCK_REFUSE_STATUS" -eq 0 ]; then + kill "$LOCK_REFUSE_HOLDER_PID" 2>/dev/null || true + wait "$LOCK_REFUSE_HOLDER_PID" 2>/dev/null || true + fail "default resumed identity succeeded under session lock contention instead of refusing: $(cat "$TMP_ROOT/lock-refuse-resume.out")" +fi +wait "$LOCK_REFUSE_HOLDER_PID" || fail "resume lock-refuse lock holder failed" +LOCK_REFUSE_HOLDER_PID= +[ "$LOCK_REFUSE_STATUS" -ne 0 ] \ + || fail "default resumed identity returned success under contention" +grep -F "refusing a concurrent resume" "$TMP_ROOT/lock-refuse-resume.err" >/dev/null 2>&1 \ + || fail "default resume under contention did not refuse with the concurrent-resume message: $(cat "$TMP_ROOT/lock-refuse-resume.err")" +# Fixture metadata and husk must be unchanged after the refused resume. +[ "$(grep '^herdr_pane_id=' "$LOCK_REFUSE_META" | cut -d= -f2-)" = "$LOCK_REFUSE_OLD_PANE" ] \ + || fail "refused resume mutated the recorded pane id" +assert_focus_is "$LOCK_REFUSE_FOCUS" "resume lock-refuse" +# Leave the journal/meta in place so the opt-in wait path below can resume the +# same identity after another stop/reprovision cycle. +pass "real Herdr lab: default resumed identity refuses session lock contention" + +# With --herdr-resume-lock-wait, the same exact resume WAITS for session lock +# contention rather than treating a short bounded window as fatal. Hold the +# shared session lock from an unrelated process for a duration well past any +# plausible bounded-retry window so the assertion below is deterministic +# rather than a race that could pass by luck on a fast machine. +LOCK_WAIT_ID=$LOCK_REFUSE_ID +LOCK_WAIT_META=$LOCK_REFUSE_META +LOCK_WAIT_OLD_WT=$LOCK_REFUSE_OLD_WT +LOCK_WAIT_WSID=$(grep '^herdr_workspace_id=' "$LOCK_WAIT_META" | cut -d= -f2-) +LOCK_WAIT_OLD_PANE=$LOCK_REFUSE_OLD_PANE +PATH="$HERDR_ORIGINAL_PATH" "$HERDR_LAB_HELPER" stop "$HERDR_LAB_SESSION" >/dev/null \ + || fail "could not stop the isolated session for resume lock-wait" +PATH="$HERDR_ORIGINAL_PATH" "$HERDR_LAB_HELPER" provision "$HERDR_LAB_SESSION" \ + || fail "could not reprovision the isolated session for resume lock-wait" + +LOCK_WAIT_READY="$TMP_ROOT/lock-wait-ready" +LOCK_WAIT_HOLD_SECONDS=30 +LOCK_WAIT_PATH=$(session_presentation_lock_path) \ + || fail "could not resolve session lock for resume lock-wait" +ROOT="$ROOT" READY="$LOCK_WAIT_READY" HOLD="$LOCK_WAIT_HOLD_SECONDS" LOCK="$LOCK_WAIT_PATH" bash -c ' + . "$ROOT/bin/fm-wake-lib.sh" + fm_lock_try_acquire "$LOCK" || exit 1 + : > "$READY" + sleep "$HOLD" + fm_lock_release "$LOCK" +' & +LOCK_WAIT_HOLDER_PID=$! +while [ ! -e "$LOCK_WAIT_READY" ] && kill -0 "$LOCK_WAIT_HOLDER_PID" 2>/dev/null; do sleep 0.01; done +[ -e "$LOCK_WAIT_READY" ] || fail "could not hold the session presentation lock for resume lock-wait" + +LOCK_WAIT_DEADLINE_SECONDS=$((LOCK_WAIT_HOLD_SECONDS + 60)) +LOCK_WAIT_FOCUS=$(focus_snapshot) +LOCK_WAIT_START=$(date +%s) +if SPAWN_DEADLINE_SECONDS=$LOCK_WAIT_DEADLINE_SECONDS \ + spawn_task "$LOCK_WAIT_ID" "$HOME_DIR" "$RECOVERY_PROJECT_DIR" --herdr-resume-lock-wait \ + > "$TMP_ROOT/lock-wait-resume.out" 2> "$TMP_ROOT/lock-wait-resume.err"; then + LOCK_WAIT_STATUS=0 +else + LOCK_WAIT_STATUS=$? +fi +LOCK_WAIT_ELAPSED=$(( $(date +%s) - LOCK_WAIT_START )) +wait "$LOCK_WAIT_HOLDER_PID" || fail "resume lock-wait lock holder failed" +LOCK_WAIT_HOLDER_PID= +if [ "$LOCK_WAIT_STATUS" -eq 124 ]; then + fail "opt-in resumed recovery hung for over ${LOCK_WAIT_DEADLINE_SECONDS}s instead of waiting out a ${LOCK_WAIT_HOLD_SECONDS}s session lock hold" +fi +[ "$LOCK_WAIT_STATUS" -eq 0 ] \ + || fail "opt-in resumed identity refused instead of waiting out session lock contention: $(cat "$TMP_ROOT/lock-wait-resume.err")" +[ "$LOCK_WAIT_ELAPSED" -ge $((LOCK_WAIT_HOLD_SECONDS - 5)) ] \ + || fail "opt-in resumed recovery returned after ${LOCK_WAIT_ELAPSED}s, too soon to have genuinely waited out a ${LOCK_WAIT_HOLD_SECONDS}s hold" +LOCK_WAIT_NEW_WT=$(remember_meta_worktree "$LOCK_WAIT_META") +[ "$(grep '^herdr_workspace_id=' "$LOCK_WAIT_META" | cut -d= -f2-)" = "$LOCK_WAIT_WSID" ] \ + || fail "opt-in resume lock-wait flattened the task into a different workspace" +LOCK_WAIT_NEW_PANE=$(grep '^herdr_pane_id=' "$LOCK_WAIT_META" | cut -d= -f2-) +[ "$LOCK_WAIT_NEW_PANE" != "$LOCK_WAIT_OLD_PANE" ] \ + || fail "opt-in resume lock-wait reused the old husk pane" +if lab pane get "$LOCK_WAIT_OLD_PANE" >/dev/null 2>&1; then + fail "opt-in resume lock-wait left the old husk pane behind" +fi +assert_focus_is "$LOCK_WAIT_FOCUS" "resume lock-wait" +teardown_task "$LOCK_WAIT_ID" "$HOME_DIR" > "$TMP_ROOT/lock-wait-teardown.out" 2> "$TMP_ROOT/lock-wait-teardown.err" \ + || fail "resume lock-wait fixture teardown failed: $(cat "$TMP_ROOT/lock-wait-teardown.err")" +"$REAL_TREEHOUSE" return --force "$LOCK_WAIT_OLD_WT" >/dev/null 2>&1 || true +"$REAL_TREEHOUSE" return --force "$LOCK_WAIT_NEW_WT" >/dev/null 2>&1 || true +pass "real Herdr lab: --herdr-resume-lock-wait waits out session lock contention instead of refusing" + # Seed a legacy old-format primary projection and a flat secondmate tab; correction must not migrate them. LEGACY_OUT=$(lab workspace create --cwd "$PROJECT_DIR" --label "firstmate/legacy-seed ยท p:AbCdEfGhIjKlMnOpQrStUv" --no-focus) \ || fail "could not seed a legacy old-format presentation space" From 0f34fab4364dbeb475896a813a75dc34b0201070 Mon Sep 17 00:00:00 2001 From: Vytautas Stankus Date: Wed, 7 Oct 2026 13:23:44 +0300 Subject: [PATCH 2/7] fix(bin): let TERM stop a watcher blocked in a pane capture on bash 3.2 (#6762) * fix(bin): let TERM stop a watcher blocked in a pane capture on bash 3.2 Stock macOS bash 3.2 holds a HUP or TERM until a running command substitution's child exits, and the watcher read every pane through $(fm_backend_capture ...). A blocked backend read therefore held the watcher's stop for as long as the read lasted, and a stopped watcher left the hung read orphaned. tests/fm-watch-triage.test.sh test_term_stops_a_watcher_blocked_inside_a_poll failed on /bin/bash 3.2 for this reason while passing on bash 5. Pane captures now go through watcher_capture, which runs the read as a waited background process group recorded like a check's, so the stop is honored at once and watcher_cleanup stops a read still in flight along with its per-call output file. * no-mistakes(review): Run drain-ring idle capture in watcher shell, add regression test * no-mistakes(document): Document watcher TERM handling for blocked checks and captures * no-mistakes(test): Silence bash 3.2 setpgid race noise from watcher captures * fix(bin): verify the capture group and scope the stop claim to pane reads watcher_capture now confirms its background read leads its own process group, as run_check_capture already does, so watcher_cleanup never relies on a group that set -m failed to create. The comment and continuity doc now say only fm_backend_capture pane reads go through watcher_capture; agent-state and composer-state reads still run inside command substitutions. --- bin/fm-watch.sh | 85 ++++++++++++++++++++++++++++++------- docs/watcher-continuity.md | 3 +- tests/fm-wake-queue.test.sh | 85 +++++++++++++++++++++++++++++++++++++ 3 files changed, 156 insertions(+), 17 deletions(-) diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index dcd1f7c4933..16b7845dfb2 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -549,7 +549,8 @@ inbox_steer_check() { # return 0 ;; esac - tail40=$(fm_backend_capture "$backend" "$w" 40 "$(window_label "$w")" 2>/dev/null) || tail40= + watcher_capture "$backend" "$w" 40 "$(window_label "$w")" || WATCHER_CAPTURE= + tail40=$WATCHER_CAPTURE if window_is_busy "$w" "$tail40"; then [ "$verb" != retry ] || return 0 if ! count=$(fm_task_inbox_record_busy "$STATE" "$task" "$rec"); then @@ -763,7 +764,8 @@ signal_turnend_panes_churned() { # ... [ "$hash_bytes" = 32 ] || return 1 prev=$(cat "$hash_file" 2>/dev/null) || return 1 [[ $prev =~ ^[0-9a-f]{32}$ ]] || return 1 - now=$(fm_backend_capture "$backend" "$w" 40 "$label" 2>/dev/null) || return 1 + watcher_capture "$backend" "$w" 40 "$label" || return 1 + now=$WATCHER_CAPTURE [ -n "$now" ] || return 1 [ "$(printf '%s' "$now" | hash_pane)" != "$prev" ] || return 1 churned_keys+=("$key") @@ -869,27 +871,29 @@ secondmate_in_active_turn() { # local w=$1 idle=$2 tail40 [ -n "$w" ] || return 1 [ "$idle" -lt "$BUSY_TURN_MAX_SECS" ] || return 1 - tail40=$(fm_backend_capture "$(window_backend "$w")" "$w" 40 "$(window_label "$w")" 2>/dev/null) || return 1 + watcher_capture "$(window_backend "$w")" "$w" 40 "$(window_label "$w")" || return 1 + tail40=$WATCHER_CAPTURE window_is_busy "$w" "$tail40" } -# First token of the semantic busy classification for : busy, idle, -# unknown, or dead. Capture failure and a missing window are unknown, never -# idle. Empty inbox and a fresh watcher beacon are not consulted. +# Set SECONDMATE_BUSY_CLASS to the first token of the semantic busy +# classification for : busy, idle, unknown, or dead. Capture failure and +# a missing window are unknown, never idle. Empty inbox and a fresh watcher +# beacon are not consulted. Call it directly, never in $(...), so its pane +# capture runs in the watcher's own shell. +SECONDMATE_BUSY_CLASS= secondmate_busy_class() { # local w=$1 task meta tail40 verdict + SECONDMATE_BUSY_CLASS=unknown task=$(window_to_task "$w" "$STATE") meta="$STATE/$task.meta" if [ -z "$w" ] || [ -z "$task" ] || [ ! -f "$meta" ]; then - printf 'unknown' return 0 fi - tail40=$(fm_backend_capture "$(window_backend "$w")" "$w" 40 "$(window_label "$w")" 2>/dev/null) || { - printf 'unknown' - return 0 - } + watcher_capture "$(window_backend "$w")" "$w" 40 "$(window_label "$w")" || return 0 + tail40=$WATCHER_CAPTURE verdict=$(fm_busy_classify_meta "$meta" "$task" "$STATE" "$tail40") - printf '%s' "${verdict%% *}" + SECONDMATE_BUSY_CLASS=${verdict%% *} } # 0 iff a child ring is authorized: exact idle, a live agent, and a composer @@ -899,7 +903,8 @@ secondmate_busy_class() { # secondmate_idle_ring_safe() { # local w=$1 backend agent_state cstate [ -n "$w" ] || return 1 - [ "$(secondmate_busy_class "$w")" = idle ] || return 1 + secondmate_busy_class "$w" + [ "$SECONDMATE_BUSY_CLASS" = idle ] || return 1 backend=$(window_backend "$w") agent_state=$(fm_backend_agent_state "$backend" "$w" 2>/dev/null || true) [ "$agent_state" = alive ] || return 1 @@ -2138,8 +2143,10 @@ fm_active_check_stop() { # parse of the next command substitution, the body then fails to parse ("trap: # line 2: unexpected EOF while looking for matching `)'", or nothing at all), # and the signal is consumed, so a stop request could leave this watcher -# polling forever while its stopper waits (fixed upstream in bash 5.3). INT -# keeps its trap because bash ignores a direct SIGINT while a child runs. +# polling forever while its stopper waits (fixed upstream in bash 5.3). Bash +# 3.2 holds HUP and TERM until a running command substitution's child exits, so +# fm_backend_capture pane reads go through watcher_capture instead. INT keeps +# its trap because bash ignores a direct SIGINT while a child runs. watcher_stop_signals() { trap - HUP TERM trap 'exit 1' INT @@ -2176,6 +2183,50 @@ run_check_capture() { fm_check_output_cleanup } +FM_CAPTURE_OUTPUT= +WATCHER_CAPTURE= + +fm_capture_output_cleanup() { + [ -z "$FM_CAPTURE_OUTPUT" ] || rm -f -- "$FM_CAPTURE_OUTPUT" + FM_CAPTURE_OUTPUT= +} + +# watcher_capture: fm_backend_capture into WATCHER_CAPTURE with its exit status. +# The read runs as a waited background process group rather than inside $(...), +# so a stop is not held for a blocked read (watcher_stop_signals). The group is +# recorded like a check's, so watcher_cleanup stops a read still in flight. +watcher_capture() { # [expected-label] + local rc pgid + fm_capture_output_cleanup + WATCHER_CAPTURE= + FM_CAPTURE_OUTPUT=$(mktemp "$STATE/.fm-capture-output.XXXXXX") || return 1 + FM_CHECK_SIGNAL_PENDING= + trap 'FM_CHECK_SIGNAL_PENDING=1' HUP INT TERM + # The group's stderr is redirected before the fork: bash 3.2 on macOS can + # print a harmless "child setpgid ... Operation not permitted" race from the + # child before the command's own redirections apply. + set -m + { fm_backend_capture "$@" < /dev/null > "$FM_CAPTURE_OUTPUT" & } 2>/dev/null + FM_ACTIVE_CHECK_PID=$! + FM_ACTIVE_CHECK_PGID=$FM_ACTIVE_CHECK_PID + set +m + watcher_stop_signals + [ -z "$FM_CHECK_SIGNAL_PENDING" ] || exit 1 + pgid=$(ps -o pgid= -p "$FM_ACTIVE_CHECK_PID" 2>/dev/null | tr -d '[:space:]') + if [ -n "$pgid" ] && [ "$pgid" != "$FM_ACTIVE_CHECK_PGID" ]; then + fm_active_check_stop || true + fm_capture_output_cleanup + return 1 + fi + wait "$FM_ACTIVE_CHECK_PID" + rc=$? + FM_ACTIVE_CHECK_PID= + fm_active_check_stop || { fm_capture_output_cleanup; return 1; } + WATCHER_CAPTURE=$(cat "$FM_CAPTURE_OUTPUT" 2>/dev/null || true) + fm_capture_output_cleanup + return "$rc" +} + # 0 when any signaled status file carries a captain-relevant event in the bytes # appended since this watcher last classified it. The start offset is the # classified-position field in that file's .seen-* marker, and fm-classify-lib.sh's @@ -2539,6 +2590,7 @@ watcher_cleanup() { fi fm_active_check_stop || cleanup_status=1 fm_check_output_cleanup + fm_capture_output_cleanup fm_custom_check_snapshot_cleanup if [ "$owns_lock" -eq 1 ] \ && ! fm_recovery_transition "$WATCHER_DOWNTIME_MARKER" "$transition" "$WATCH_LOCK" \ @@ -3016,7 +3068,8 @@ EOF if [ "$kind" = secondmate ] && ! status_is_paused_or_captain_held "$last"; then continue fi - tail40=$(fm_backend_capture "$(window_backend "$w")" "$w" 40 "$(window_label "$w")" 2>/dev/null) || continue + watcher_capture "$(window_backend "$w")" "$w" 40 "$(window_label "$w")" || continue + tail40=$WATCHER_CAPTURE h=$(printf '%s' "$tail40" | hash_pane) hf="$STATE/.hash-$key" cf="$STATE/.count-$key" diff --git a/docs/watcher-continuity.md b/docs/watcher-continuity.md index f14717596b4..b82ccf2e712 100644 --- a/docs/watcher-continuity.md +++ b/docs/watcher-continuity.md @@ -402,7 +402,7 @@ Only the watcher process touches `state/.last-watcher-beat`. No helper process can make a wedged watcher appear healthy. An arm whose own script path sits under a disposable no-mistakes validation checkout (`.no-mistakes/worktrees/`) refuses with the typed failure line before touching any state, because a watcher started there outlives the validation step and keeps writing the real home's state from a checkout about to be deleted. Once per poll the watcher checks that its home, its state directory, and its own code root still exist, and exits with a logged reason when one is gone, scoped to itself alone, so a torn-down temporary home or a discarded checkout never leaves an orphan watcher behind. -The watcher uses bash's native fatal handling for HUP and TERM, including during a blocked poll, so both run its EXIT cleanup. +The watcher uses bash's native fatal handling for HUP and TERM, including during a blocked check or a blocked `fm_backend_capture` pane read, so both run its EXIT cleanup and stop that read. `watcher_stop_signals` in `bin/fm-watch.sh` owns the signal-handling rationale. The EXIT cleanup bounds its wait for `state/.watcher-down.lock` while persisting recovery state with `FM_WATCHER_CLEANUP_LOCK_BOUND` (default 2 seconds). Only positive decimal integers are accepted, including leading-zero forms such as `08`; empty, non-numeric, and zero values (including `00`) fall back to 2 seconds. @@ -468,6 +468,7 @@ They also prove that a legacy or handoff-phase watcher marker from an absent rep `tests/fm-watch-triage.test.sh` proves TERM stops a watcher blocked inside a poll's pane capture and still releases its lock and records an acknowledgeable stop. It also exercises a single TERM with a live foreign downtime-marker lock holder, retained stale singleton and subsequent arm-style recovery, including decimal `08` and zero `00` cleanup bounds. It checks that a newly appended keyed decision is classified without rereading earlier status bytes, so signal handling can return to the watcher's beacon refresh even when the status history is long. +`tests/fm-wake-queue.test.sh` proves TERM likewise stops a watcher blocked in the drain-ring idle check's pane capture. `tests/fm-watcher-lock.test.sh` covers: diff --git a/tests/fm-wake-queue.test.sh b/tests/fm-wake-queue.test.sh index 40dacd953c7..dc97435ec73 100755 --- a/tests/fm-wake-queue.test.sh +++ b/tests/fm-wake-queue.test.sh @@ -914,6 +914,90 @@ test_secondmate_proven_idle_ring_lets_the_child_drain() { pass "a proven-idle leftover row is rung so the child home can drain without a parent alarm" } +# The idle proof that gates a drain ring reads the mate's pane. A TERM that +# lands while that read is blocked must still stop the watcher at once and run +# its cleanup, as it does for every other pane read. +test_term_stops_a_watcher_blocked_in_the_drain_ring_idle_capture() { + local dir state sub fakebin fifo out pid holder i rc orphan + dir=$(make_case secondmate-ring-blocked-capture) + state="$dir/state" + sub="$dir/secondmate" + fakebin="$dir/fakebin" + fifo="$dir/pane.fifo" + out="$dir/watch-blocked.out" + mkdir -p "$sub/state" + printf 'mate\n' > "$sub/.fm-secondmate-home" + printf 'window=firstmate:fm-mate\nkind=secondmate\nharness=claude\nbackend=tmux\nhome=%s\n' \ + "$sub" > "$state/mate.meta" + printf '100\t7\tcheck\trouted\tcheck: routed row\n' > "$sub/state/.wake-queue" + install_secondmate_alive_tmux "$fakebin" + install_secondmate_stall_date "$fakebin" + "$ROOT/bin/fm-busy-event.sh" arm "$state" mate >/dev/null \ + || fail "could not arm the mate's busy contract" + "$ROOT/bin/fm-busy-event.sh" apply "$state" mate idle --current-gen \ + --source claude-hook --event stop >/dev/null \ + || fail "could not mark the mate idle" + + printf '1000\n' > "$dir/now" + PATH="$fakebin:$PATH" FM_FAKE_NOW_FILE="$dir/now" FM_HOME="$dir" FM_ROOT_OVERRIDE="$ROOT" \ + FM_STATE_OVERRIDE="$state" FM_FAKE_TMUX_SENT="$dir/sent" \ + FM_SECONDMATE_WAKE_STALL_SECS=1 FM_POLL=1 FM_SIGNAL_GRACE=0 \ + FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 \ + secondmate_stall_watch_leg "$dir" "first" progress mate "$(printf '1000\t100-7')" + + # The active-turn read returns, then the idle-proof read blocks on a FIFO + # whose write end the holder keeps open without writing. + mv "$fakebin/tmux" "$fakebin/tmux-alive" + cat > "$fakebin/tmux" <<'SH' +#!/usr/bin/env bash +if [ "${1:-}" = capture-pane ]; then + n=$(( $(cat "$FM_FAKE_CAPTURE_COUNT" 2>/dev/null || echo 0) + 1 )) + printf '%s\n' "$n" > "$FM_FAKE_CAPTURE_COUNT" + [ "$n" -ge 2 ] || exit 0 + exec cat "$FM_FAKE_TMUX_CAPTURE" +fi +exec "$(dirname "$0")/tmux-alive" "$@" +SH + chmod +x "$fakebin/tmux" + mkfifo "$fifo" + ( exec 3> "$fifo"; : > "$dir/capture-blocked"; exec sleep 30 ) & + holder=$! + + printf '1002\n' > "$dir/now" + touch "$state/.secondmate-liveness-tick" + PATH="$fakebin:$PATH" FM_FAKE_NOW_FILE="$dir/now" FM_HOME="$dir" FM_ROOT_OVERRIDE="$ROOT" \ + FM_STATE_OVERRIDE="$state" FM_FAKE_TMUX_SENT="$dir/sent" \ + FM_FAKE_TMUX_CAPTURE="$fifo" FM_FAKE_CAPTURE_COUNT="$dir/captures" \ + FM_SECONDMATE_WAKE_STALL_SECS=1 FM_SECONDMATE_LIVENESS_SECS=99999999 \ + FM_POLL=1 FM_SIGNAL_GRACE=0 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 \ + "$WATCH" > "$out" 2>&1 & + pid=$! + i=0 + while [ ! -e "$dir/capture-blocked" ] && [ "$i" -lt 300 ]; do + sleep 0.1 + i=$((i + 1)) + done + if [ ! -e "$dir/capture-blocked" ] || ! is_live_non_zombie "$pid"; then + kill "$holder" 2>/dev/null || true; kill "$pid" 2>/dev/null || true; wait "$pid" 2>/dev/null || true + fail "the watcher never blocked inside the drain-ring idle capture: $(cat "$out")" + fi + kill "$pid" 2>/dev/null || true + wait_for_exit "$pid" 100 + rc=$? + orphan=$(pgrep -f "cat $fifo" || true) + [ -z "$orphan" ] || pkill -f "cat $fifo" 2>/dev/null || true + kill "$holder" 2>/dev/null || true + wait "$holder" 2>/dev/null || true + [ "$rc" -ne 124 ] || fail "TERM did not stop a watcher blocked in the drain-ring idle capture" + [ -z "$orphan" ] || fail "a watcher stopped in the drain-ring idle capture left that capture running" + [ "$(cat "$dir/captures")" = 2 ] \ + || fail "the watcher did not block in the drain-ring idle capture: $(cat "$dir/captures") pane reads" + [ ! -e "$state/.watch.lock" ] \ + || fail "a watcher stopped in the drain-ring idle capture kept its singleton lock" + [ ! -s "$dir/sent" ] || fail "a watcher stopped before its idle proof still rang the mate" + pass "TERM stops a watcher blocked in the drain-ring idle capture and runs its cleanup" +} + # Busy and unknown panes are never typed into. Busy still defers inside the # active-turn bound. Unknown keeps the parent alarm. Empty inbox is not idle # proof, so the unknown fixture starts with no instruction records. @@ -3432,6 +3516,7 @@ test_secondmate_reprovisioned_queue_starts_a_fresh_interval test_secondmate_active_turn_defers_stall_until_the_turn_ends test_secondmate_long_lived_mate_mid_turn_is_not_a_stall test_secondmate_proven_idle_ring_lets_the_child_drain +test_term_stops_a_watcher_blocked_in_the_drain_ring_idle_capture test_secondmate_busy_and_unknown_panes_are_not_rung test_secondmate_genuine_stall_after_idle_ring_still_alarms test_secondmate_stall_marker_rejects_symlink From 47aff866dbe0612bd43df66d8fa76576e06a2b3e Mon Sep 17 00:00:00 2001 From: cloud-practitioner <113355863+cloud-practitioner@users.noreply.github.com> Date: Wed, 7 Oct 2026 21:52:45 +1100 Subject: [PATCH 3/7] feat: add per-home worker tool exclusions (#6750) * feat(spawn): add per-home worker tool exclusions Add an optional per-home config/crew-exclude-tools file listing tool names to hide from workers, one per line, with blank lines and # comments allowed. It applies to every ship and scout launch and relaunch in that home, is never inherited by another home, and does not affect secondmate agents. Pi and pi-signed apply it through --exclude-tools, which also covers MCP tool names. Any other runtime, and a raw launch command, refuses the launch when the list is non-empty rather than ignoring it. Malformed entries are refused before provisioning, and before a relaunch stops a running worker. Exclusions that match no tool in the worker's loaded registry are reported as unverified warnings in its status record instead of refusing the worker. Closes #6744 * no-mistakes(review): Preserve UTF-8 exclusion paths and verify Pi lifecycle behavior * no-mistakes(document): Clarify worker tool exclusion documentation * no-mistakes(ci): Fixed ci-1 in bin/fm-exclude-tools-lib.sh: a failed read now returns an error before printing names, so all shared launch and relaunch callers refuse rather than silently dropping exclusions. Added deterministic regression coverage for a file disappearing after readability checks across Pi/pi-signed ship and scout launches. Reproduced the original failure; verified 83 spawn checks, 77 relaunch checks, direct parser/runtime failure cases, full targeted lint, Bash syntax, and git diff --check. Relaunch tests passed with existing fixture-cleanup permission warnings. ci-2 remains unchanged per the user's decision; the outer executor owns the fresh CI run --- bin/fm-control.sh | 11 + bin/fm-exclude-tools-lib.sh | 64 +++++ bin/fm-spawn.sh | 42 ++- docs/agent-control.md | 1 + docs/configuration.md | 39 ++- tests/fm-control-relaunch.test.sh | 50 ++++ tests/fm-spawn-dispatch-profile.test.sh | 323 +++++++++++++++++++++++- 7 files changed, 518 insertions(+), 12 deletions(-) create mode 100644 bin/fm-exclude-tools-lib.sh diff --git a/bin/fm-control.sh b/bin/fm-control.sh index 118c447a607..e5642b9e33b 100755 --- a/bin/fm-control.sh +++ b/bin/fm-control.sh @@ -76,6 +76,9 @@ # worker account pin (bin/fm-worker-account-lib.sh) here, so a pin # that no longer resolves or is signed out refuses before the old # agent stops. +# The same pre-stop refusal applies to this home's worker tool +# exclusions (bin/fm-exclude-tools-lib.sh): a malformed list, or a +# replacement runtime that cannot hide the listed tools. # --note is required for a ship or scout, whose replacement # inherits the local copy but none of the conversation; a # secondmate reconciles its own home's records at startup, so its @@ -176,6 +179,8 @@ DATA="${FM_DATA_OVERRIDE:-$FM_HOME/data}" . "$SCRIPT_DIR/fm-wake-lib.sh" # shellcheck source=bin/fm-worker-account-lib.sh . "$SCRIPT_DIR/fm-worker-account-lib.sh" +# shellcheck source=bin/fm-exclude-tools-lib.sh +. "$SCRIPT_DIR/fm-exclude-tools-lib.sh" POLL=${FM_CONTROL_POLL:-0.5} SETTLE_WAIT=${FM_CONTROL_SETTLE_WAIT:-5} @@ -956,6 +961,12 @@ resolve_relaunch_profile() { [ "$account_model" != default ] || account_model= fm_worker_account_select "$TARGET_HARNESS" "${FM_CONFIG_OVERRIDE:-$FM_HOME/config}" \ "$account_model" "$TARGET_HARNESS" >/dev/null || return 1 + # Likewise config/crew-exclude-tools: a malformed file, or a replacement + # runtime that cannot hide the listed tools, refuses here, before the old + # agent stops. Secondmate agents are not covered. + if [ "$KIND" != secondmate ]; then + fm_exclude_tools_check "$TARGET_HARNESS" 0 "${FM_CONFIG_OVERRIDE:-$FM_HOME/config}" >/dev/null || return 1 + fi } # safe_checkpoint: prove, before anything is stopped, that the work a relaunch diff --git a/bin/fm-exclude-tools-lib.sh b/bin/fm-exclude-tools-lib.sh new file mode 100644 index 00000000000..0306a7e8026 --- /dev/null +++ b/bin/fm-exclude-tools-lib.sh @@ -0,0 +1,64 @@ +#!/usr/bin/env bash +# fm-exclude-tools-lib.sh - shared parsing and runtime-support checks for +# config/crew-exclude-tools, sourced by bin/fm-spawn.sh and bin/fm-control.sh. +# +# docs/configuration.md "Worker tool exclusions" owns the file format and +# operator contract; bin/fm-spawn.sh's header owns launch-flag mechanics. +# Name verification belongs inside the worker's loaded-tool registry, never +# in an out-of-band server connection from these launch-time checks. + +# fm_exclude_tools_names : print the comma-joined names, empty when +# the file is absent or lists nothing. Non-zero with the reason on stderr when +# the file or an entry is invalid. +fm_exclude_tools_names() { + local config=$1 file line names='' present + file=$config/crew-exclude-tools + present=$(perl -MErrno=ENOENT -e ' + if (lstat $ARGV[0]) { print 1 } + elsif ($! == ENOENT) { print 0 } + else { die "error: cannot inspect config/crew-exclude-tools: $!\n" } + ' -- "$file") || return 1 + [ "$present" = 1 ] || return 0 + if [ ! -f "$file" ] || [ ! -r "$file" ]; then + echo "error: config/crew-exclude-tools must be a readable regular file" >&2 + return 1 + fi + while IFS= read -r line || [ -n "$line" ]; do + line=${line#"${line%%[![:space:]]*}"} + line=${line%"${line##*[![:space:]]}"} + case "$line" in + '' | '#'*) continue ;; + esac + if [ -n "${line//[A-Za-z0-9_.-]/}" ]; then + echo "error: config/crew-exclude-tools has a malformed entry '$line'; expected one tool name per line using only letters, digits, _ . and - (blank lines and # comment lines are allowed)" >&2 + return 1 + fi + names="${names:+$names,}$line" + done <"$file" || { + echo "error: cannot read config/crew-exclude-tools" >&2 + return 1 + } + printf '%s' "$names" +} + +# fm_exclude_tools_check : succeed when +# this launch can honor the home's list (empty list, or a runtime that hides +# tools); otherwise refuse naming the file. Prints the names on stdout. +fm_exclude_tools_check() { + local harness=$1 raw=$2 config=$3 names + names=$(fm_exclude_tools_names "$config") || return 1 + if [ -n "$names" ]; then + if [ "$raw" = 1 ]; then + echo "error: config/crew-exclude-tools lists tools to hide, but a raw launch command cannot hide tools; remove the entries or launch a runtime that supports them (pi, pi-signed)" >&2 + return 1 + fi + case "$harness" in + pi | pi-signed) ;; + *) + echo "error: config/crew-exclude-tools lists tools to hide, but the $harness runtime cannot hide tools, so this launch is refused rather than running with them available; empty the file or launch a runtime that supports exclusion (pi, pi-signed)" >&2 + return 1 + ;; + esac + fi + printf '%s' "$names" +} diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 983633cb5ec..d2feb0b65fd 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -342,6 +342,10 @@ # worktree, or record exists and names the accepted values. The file is read # on every spawn and relaunch, so a change reaches the next launch without a # restart, and it is inherited into secondmate homes (bin/fm-config-inherit-lib.sh). +# Worker tool exclusions: +# docs/configuration.md "Worker tool exclusions" owns config/crew-exclude-tools +# and its operator contract. Resolve it with bin/fm-exclude-tools-lib.sh +# before provisioning; __PIEXCLUDE__ below owns the Pi launch substitution. # Worker account pin (config/claude-account, config/pi-account): # Opt-in. With no file, a Claude or Pi launch is unchanged: Claude still # receives this process's own CLAUDE_CONFIG_DIR when it is set, and Pi the @@ -368,6 +372,9 @@ # __PIAPPROVE__ optional --approve on a seeded Pi/pi-signed secondmate when # that executable advertises the flag (empty otherwise; session # trust for the launch cwd only, never a trust.json rewrite) +# __PIEXCLUDE__ optional ` --exclude-tools ''` from +# config/crew-exclude-tools on Pi/pi-signed ship and scout +# launches (supplies its own leading space, empty otherwise) # __PIRESUME__ optional relaunch-only `--session ` that keeps a # Pi replacement on the session the endpoint's runtime already # reports (relaunch_resume_args below owns it; it supplies its @@ -552,6 +559,8 @@ PROJECTS="${FM_PROJECTS_OVERRIDE:-$FM_HOME/projects}" CONFIG="${FM_CONFIG_OVERRIDE:-$FM_HOME/config}" # shellcheck source=bin/fm-config-inherit-lib.sh . "$SCRIPT_DIR/fm-config-inherit-lib.sh" +# shellcheck source=bin/fm-exclude-tools-lib.sh +. "$SCRIPT_DIR/fm-exclude-tools-lib.sh" if ! LAUNCH_ENV_ENABLED=$(fm_config_source_present "$CONFIG/launch-env-allowlist"); then exit 1 fi @@ -2110,7 +2119,7 @@ launch_template() { ;; opencode) printf '%s' 'OPENCODE_CONFIG_CONTENT='\''{"permission":{"*":"allow"}__EFFORTFLAG__}'\'' opencode __MODELFLAG__--prompt "$(__OPINPUT__ encode launch-brief < __BRIEF__)"' ;; pi | pi-signed) - printf '%s' '__PIBIN____PITUIMODE____PIAPPROVE____PIRESUME__' + printf '%s' '__PIBIN____PITUIMODE____PIAPPROVE____PIEXCLUDE____PIRESUME__' if [ "$kind" = secondmate ]; then printf '%s' ' __MODELFLAG____EFFORTFLAG__-e __PITURNEND__ -e __PIWATCH__ "$(__OPINPUT__ encode launch-brief < __BRIEF__)"' else @@ -2359,6 +2368,13 @@ if [ "$KIND" = secondmate ] && [ "$HARNESS" = rovo ]; then exit 1 fi +# config/crew-exclude-tools (header above): refuse before worker provisioning +# if this launch cannot honor the list. Secondmate agents are not covered. +EXCLUDE_TOOLS= +if [ "$KIND" != secondmate ]; then + EXCLUDE_TOOLS=$(fm_exclude_tools_check "$HARNESS" "$RAW_LAUNCH" "$CONFIG") || exit 1 +fi + case "$HARNESS" in devin) DEVIN_BIN=$(command -v devin) || { @@ -2385,6 +2401,9 @@ pi | pi-signed) PI_APPROVE=' --approve' fi LAUNCH=${LAUNCH//__PIAPPROVE__/$PI_APPROVE} + PI_EXCLUDE= + [ -z "$EXCLUDE_TOOLS" ] || PI_EXCLUDE=" --exclude-tools $(shell_quote "$EXCLUDE_TOOLS")" + LAUNCH=${LAUNCH//__PIEXCLUDE__/$PI_EXCLUDE} LAUNCH="FM_PI_HARNESS=$HARNESS $LAUNCH" ;; cursor) @@ -4714,6 +4733,10 @@ EOF // tool calls) and stays a wake NOTIFICATION touch for the watcher, never // current-state truth. import { execFile } from "node:child_process"; +import { appendFileSync } from "node:fs"; +const excludeTools = "$EXCLUDE_TOOLS".split(",").filter(Boolean); +const excludeFile = $(perl -MJSON::PP -MEncode=decode_utf8 -e 'print encode_json(decode_utf8($ARGV[0]))' -- "$CONFIG/crew-exclude-tools"); +const statusFile = $(perl -MJSON::PP -MEncode=decode_utf8 -e 'print encode_json(decode_utf8($ARGV[0]))' -- "$STATE/$ID.status"); const busyEvent = (state: string, event: string) => new Promise((resolve) => { execFile("$FM_ROOT/bin/fm-busy-event.sh", [ @@ -4722,7 +4745,22 @@ const busyEvent = (state: string, event: string) => ], () => resolve()); }); export default function (pi: any) { - pi.on("agent_start", () => busyEvent("busy", "agent-start")); + let checkedExclusions = false; + pi.on("agent_start", async () => { + await busyEvent("busy", "agent-start"); + // Verify only this worker's registry, never connect servers from Firstmate. + // Check before actions so the warning cannot supersede this turn's terminal status. + if (!checkedExclusions && excludeTools.length) { + const loaded = new Set(pi.getAllTools().map((tool: any) => tool.name)); + const unmatched = excludeTools.filter((name) => !loaded.has(name)); + if (unmatched.length) { + appendFileSync(statusFile, "note [at=" + Math.floor(Date.now() / 1000) + "]: warning: " + excludeFile + + " unmatched exclusion entries (unverified: absent from the worker's loaded-tool registry; excluded tools or unavailable servers cannot be verified): " + + unmatched.join(", ") + "\n"); + } + checkedExclusions = true; + } + }); pi.on("agent_settled", (_event: any, ctx: any) => { if (ctx && typeof ctx.isIdle === "function" && !ctx.isIdle()) return; return busyEvent("idle", "agent-settled"); diff --git a/docs/agent-control.md b/docs/agent-control.md index a543e881938..280cb900de7 100644 --- a/docs/agent-control.md +++ b/docs/agent-control.md @@ -74,6 +74,7 @@ A relaunch does take one session reference when the endpoint's own runtime recor A recorded raw-command basename that differs from its resolved adapter cannot reproduce the command actually running, so relaunch refuses before the checkpoint unless the caller passes an explicit `--harness` to choose the replacement runtime deliberately. A harness change resets model and effort unless they are named too, because a model chosen for one adapter does not transfer to another. A Claude or Pi replacement must also pass the home's [worker account pin](configuration.md#worker-account-pin-configclaude-account-configpi-account), so a pin that no longer resolves or is signed out refuses before the old agent stops. + Ship and scout replacements also pass the [worker tool exclusion checks](configuration.md#worker-tool-exclusions-configcrew-exclude-tools) at this step. 2. **Safe checkpoint.** The recorded worktree must exist and be a worktree root; its head and dirty state are recorded. For a `kind=secondmate` task, the home's identity marker must match and its child records must be readable, so a relaunch can never strand child work behind an unreadable home. diff --git a/docs/configuration.md b/docs/configuration.md index 63695826c71..760c0195672 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -9,7 +9,7 @@ Start with the directory layout, then use the setting reference for the behavior | --- | --- | | Firstmate's code, private files, or project location | [FM_HOME](#fm_home) and [operational home layout](#operational-home-layout-and-state) | | Task windows and worker tools | [Runtime backend](#runtime-backend-configbackend--fm_backend) and [harness support](#harness-support) | -| Worker permissions, accounts, or environment | [Claude permission mode](#claude-permission-mode-configclaude-permission-mode), [worker account pin](#worker-account-pin-configclaude-account-configpi-account), and [worker launch environment](#worker-launch-environment-configlaunch-env-allowlist) | +| Worker permissions, accounts, or environment | [Claude permission mode](#claude-permission-mode-configclaude-permission-mode), [worker account pin](#worker-account-pin-configclaude-account-configpi-account), [worker tool exclusions](#worker-tool-exclusions-configcrew-exclude-tools), and [worker launch environment](#worker-launch-environment-configlaunch-env-allowlist) | | Backlog, preferences, and memory | [Backlog backend](#backlog-backend-taskstoml--configbacklog-backend), [captain preferences](#captain-preferences-datacaptainmd--datacaptain-sharedmd), and [startup memory budget](#startup-memory-budget-configstartup-memory-budget) | | Supervision and presentation | [Pi supervision branch](#pi-supervision-branch), [supervision host](#supervision-host-configsupervision-host), and [Calm preference](#calm-preference-configcalm) | | Persistent secondmates | [Secondmate routes](#secondmate-routes-datasecondmatesmd) | @@ -869,6 +869,43 @@ The file is a captain-wide safety preference, so it is inherited into secondmate The [Claude adapter reference](../.agents/skills/harness-adapters/references/harness/claude.md) records the permission-mode observations and the distinct startup dialogs. +## Worker tool exclusions (config/crew-exclude-tools) + +The optional local, gitignored `config/crew-exclude-tools` hides named tools from this home's ship and scout workers, for example to keep an MCP server's write tools out of reach while its read tools stay available. +The contract is runtime-neutral: a runtime must support hiding the listed tool names or refuse the launch, and a non-empty list is never silently ignored. +With no file, or a file with no entries, every launch on every runtime is unchanged. + +Create the file with one tool name per line, such as `mcp____` for an MCP tool. +Blank lines and lines beginning with `#` are allowed, and surrounding whitespace on a line is trimmed; a trailing comment on an entry line is not allowed. +The file is read from this home's own configuration directory on every launch, so a change reaches the next worker or relaunch without a restart. +It is not in the inherited configuration set, so no other home, including a secondmate home, receives it; create the file in each home that wants it. +It does not apply to a secondmate's own agent, which neither reads nor refuses on it. + +### Runtime support + +| Runtime | With a non-empty list | +| --- | --- | +| `pi`, `pi-signed` | Hides listed tool names, MCP tools included, on every ship and scout spawn and relaunch. | +| Every other runtime, and a raw launch command | The launch refuses with an error naming `config/crew-exclude-tools`, because that runtime has no verified way to hide tools. | + +A relaunch validates the list and the replacement runtime's support before stopping the running worker, so an exclusion-list refusal preserves the running agent. + +### Validation + +An entry may use only `A-Z`, `a-z`, `0-9`, `_`, `.`, and `-`. +An entry with any other character, including internal whitespace, a comma, or a `*`, refuses the launch and names the offending entry. +An unreadable or nonregular file, or a path inspection error, also refuses and names the configuration file. +For a new worker, these checks run before its endpoint, local copy, or task record is created; Firstmate never launches with a partial list. +Only exact tool names are accepted, not wildcard patterns. +Firstmate checks syntax and runtime support before launch but never runs `pi mcp list` or otherwise connects to servers to validate names. +When its first agent run starts, after Pi's startup tool-loading boundary, the worker extension compares the launch's exclusion list with its own loaded-tool registry and appends a timestamped warning note to `state/.status` naming the configuration file and every unmatched entry for the supervisor. +The check runs before worker actions so it cannot supersede a terminal status emitted during the turn. +An unmatched entry is reported as **unverified**, not valid: Pi versions that omit excluded tools from the registry cannot distinguish a correct exclusion from a typo, and a server that has not connected cannot verify its tools either. +Names present in the registry produce no report; unknown or unverified names do not refuse the launch. +Each relaunch installs a fresh worker extension with the home's current list, so the replacement performs the same check. + +[`bin/fm-exclude-tools-lib.sh`](../bin/fm-exclude-tools-lib.sh) implements parsing and pre-launch validation for this contract; [`bin/fm-spawn.sh`](../bin/fm-spawn.sh)'s header owns the launch-flag mechanics. + ## Worker account pin (config/claude-account, config/pi-account) A home that mixes accounts for one runner, such as a work login and a personal one, can pin the account its own Claude and Pi workers launch on. diff --git a/tests/fm-control-relaunch.test.sh b/tests/fm-control-relaunch.test.sh index a214f057b7e..6400bd33b4a 100755 --- a/tests/fm-control-relaunch.test.sh +++ b/tests/fm-control-relaunch.test.sh @@ -814,6 +814,54 @@ test_worker_account_pin_follows_the_relaunch() { pass "fm-control relaunch: the replacement follows the home's current worker account pin" } +test_pi_exclude_tools_follow_the_relaunch() { + local dir out rc id=rl-pi-excl + dir=$(new_case pi-exclude "$id") + add_ship_task "$dir" "$id" pi + printf pi > "$dir/fake/command" + printf pi > "$dir/fake/becomes" + printf '#!/usr/bin/env bash\nprintf "Options: --tui-mode\\n"\n' > "$dir/fakebin/pi" + chmod +x "$dir/fakebin/pi" + mkdir -p "$dir/home/config" + printf '%s\n' '# hide writes' 'mcp__srv__writeTool' 'mcp__srv__adminTool' > "$dir/home/config/crew-exclude-tools" + out=$(run_control "$dir" "$id" relaunch --note "keep exclusions"); rc=$? + expect_code 0 "$rc" "a Pi relaunch with exclusions should succeed"$'\n'"$out" + assert_contains "$(cat "$dir/fake/literal")" "--exclude-tools 'mcp__srv__writeTool,mcp__srv__adminTool'" \ + "the relaunched Pi worker must keep the home's tool exclusions" + rm "$dir/home/config/crew-exclude-tools" + : > "$dir/fake/literal" + printf pi > "$dir/fake/command" + out=$(run_control "$dir" "$id" relaunch --note "exclusions removed"); rc=$? + expect_code 0 "$rc" "a Pi relaunch after the file is removed should succeed"$'\n'"$out" + assert_not_contains "$(cat "$dir/fake/literal")" "--exclude-tools" \ + "a relaunch without the file must launch with no exclusions" + pass "fm-control relaunch: a Pi replacement keeps the home's tool exclusions" +} + +test_exclude_tools_refusals_happen_before_the_agent_stops() { + local dir out rc id=rl-excl-refuse + dir=$(new_case excl-refuse "$id") + add_ship_task "$dir" "$id" pi + printf pi > "$dir/fake/command" + printf pi > "$dir/fake/becomes" + printf '#!/usr/bin/env bash\nprintf "Options: --tui-mode\\n"\n' > "$dir/fakebin/pi" + chmod +x "$dir/fakebin/pi" + mkdir -p "$dir/home/config" + printf '%s\n' 'two words' > "$dir/home/config/crew-exclude-tools" + out=$(run_control "$dir" "$id" relaunch --note "bad list"); rc=$? + expect_code 1 "$rc" "a malformed exclusion list must refuse the relaunch" + assert_contains "$out" "config/crew-exclude-tools has a malformed entry" "refusal must name the entry" + [ "$(cat "$dir/fake/command")" = pi ] || fail "a malformed exclusion list stopped the running agent" + [ ! -s "$dir/fake/literal" ] || fail "a refused relaunch sent lifecycle input" + printf '%s\n' 'mcp__srv__writeTool' > "$dir/home/config/crew-exclude-tools" + out=$(run_control "$dir" "$id" relaunch --harness codex --note "switch runtime"); rc=$? + expect_code 1 "$rc" "relaunching onto a runtime that cannot hide tools must refuse" + assert_contains "$out" "config/crew-exclude-tools" "refusal must name the config file" + [ "$(cat "$dir/fake/command")" = pi ] || fail "an unhonorable exclusion list stopped the running agent" + [ ! -s "$dir/fake/literal" ] || fail "a refused relaunch sent lifecycle input" + pass "fm-control relaunch: exclusion-list refusals happen before the running agent stops" +} + test_explicit_model_wins_over_the_recorded_one() { local dir out rc dir=$(new_case explicit rl7) @@ -2457,6 +2505,8 @@ test_same_harness_relaunch_keeps_the_profile_axes test_native_ultra_relaunch_preserves_profile_and_rejects_before_stop test_signed_out_worker_account_pin_refuses_before_stop test_worker_account_pin_follows_the_relaunch +test_pi_exclude_tools_follow_the_relaunch +test_exclude_tools_refusals_happen_before_the_agent_stops test_explicit_model_wins_over_the_recorded_one test_relaunch_onto_an_unverified_harness_is_refused test_prior_harness_turnend_registry_entry_is_cleared diff --git a/tests/fm-spawn-dispatch-profile.test.sh b/tests/fm-spawn-dispatch-profile.test.sh index b69665863f3..e660d35dfb3 100755 --- a/tests/fm-spawn-dispatch-profile.test.sh +++ b/tests/fm-spawn-dispatch-profile.test.sh @@ -949,15 +949,6 @@ test_pi_signed_threads_shared_pi_profile_and_preserves_identity() { assert_present "$HOME_DIR/state/$id.busy-gen" "pi-signed spawn did not arm the busy-state contract" assert_contains "$(cat "$HOME_DIR/state/$id.busy-state")" "state=busy source=fm-spawn" \ "pi-signed spawn did not seed the busy-state record from the launch brief" - local ext gen - ext=$(cat "$HOME_DIR/state/$id.pi-ext.ts") - gen=$(cat "$HOME_DIR/state/$id.busy-gen") - assert_contains "$ext" 'pi.on("agent_start"' "pi extension lost the semantic agent_start busy edge" - assert_contains "$ext" 'pi.on("agent_settled"' "pi extension lost the semantic agent_settled idle edge" - assert_contains "$ext" 'ctx.isIdle()' "pi extension no longer confirms idle with ctx.isIdle()" - assert_contains "$ext" "\"--gen\", \"$gen\"" "pi extension does not carry the armed incarnation gen" - assert_contains "$ext" '"--source", "pi-ext"' "pi extension does not attribute its semantic source" - assert_contains "$ext" 'pi.on("turn_end"' "pi extension lost the turn-end notification touch" pass "pi-signed shares Pi launch semantics while preserving its configured and recorded identity" } @@ -1116,6 +1107,313 @@ test_pi_approve_probe_omits_unsupported_flag() { pass "Pi approve probing omits --approve when help does not advertise it" } +# config/crew-exclude-tools: per-home worker tool exclusions (fm-spawn header). +write_exclude_file() { # ... + local home=$1 + shift + printf '%s\n' "$@" > "$home/config/crew-exclude-tools" +} + +test_pi_exclude_tools_reach_ship_and_scout_launches() { + local harness rec id out status launch kindflag + for harness in pi pi-signed; do + for kindflag in --ship --scout; do + id="excl-${harness}-${kindflag#--}-z9a" + rec=$(make_spawn_case "excl-${harness}-${kindflag#--}" "$harness" "$id") + read_case_record "$rec" + write_exclude_file "$HOME_DIR" '# hide the write tools' '' \ + ' mcp__tracker__editIssue ' 'mcp__tracker__createPage' 'mcp__other-srv__tool.v2' + if [ "$kindflag" = --scout ]; then + out=$(run_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR" --scout) + else + out=$(run_ship_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR") + fi + status=$? + expect_code 0 "$status" "$harness $kindflag spawn with exclusions should succeed"$'\n'"$out" + launch=$(cat "$LAUNCH_LOG") + assert_contains "$launch" "--exclude-tools 'mcp__tracker__editIssue,mcp__tracker__createPage,mcp__other-srv__tool.v2' " \ + "$harness $kindflag launch must carry the comma-joined, quoted exclusion list" + done + done + pass "Pi and pi-signed ship and scout launches carry the configured tool exclusions" +} + +test_pi_exclude_tools_worker_registry_reports() { + local harness kindflag scenario case_name rec id out status report + command -v node >/dev/null 2>&1 || fail "node is required to drive Pi worker exclusion reporting" + for harness in pi pi-signed; do + for kindflag in --ship --scout; do + for scenario in matched unmatched unverified; do + id="excl-registry-${harness}-${kindflag#--}-${scenario}" + case_name=$id + [ "$scenario" != unmatched ] || case_name="josรฉ-$id" + rec=$(make_spawn_case "$case_name" "$harness" "$id") + read_case_record "$rec" + case "$scenario" in + matched) write_exclude_file "$HOME_DIR" 'mcp__tracker__editIssue' ;; + unmatched) write_exclude_file "$HOME_DIR" 'mcp__tracker__editIssu' 'mcp__tracker__createIssu' ;; + unverified) write_exclude_file "$HOME_DIR" 'mcp__offline__executeWrite' ;; + esac + if [ "$kindflag" = --scout ]; then + out=$(run_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR" --scout) + else + out=$(run_ship_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR") + fi + expect_code 0 "$?" "$harness $kindflag registry-report spawn should succeed: $out" + out=$(EXT_PATH="$HOME_DIR/state/$id.pi-ext.ts" STATUS_FILE="$HOME_DIR/state/$id.status" \ + EXCLUDE_FILE="$HOME_DIR/config/crew-exclude-tools" TURNEND="$HOME_DIR/state/$id.turn-ended" \ + TASK_ID="$id" BUSY_EVENT="$ROOT/bin/fm-busy-event.sh" \ + SCENARIO="$scenario" node --input-type=module 2>&1 <<'JS' +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { appendFileSync, existsSync, readFileSync, rmSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { setTimeout } from "node:timers/promises"; +import { pathToFileURL } from "node:url"; +const status = () => existsSync(process.env.STATUS_FILE) ? readFileSync(process.env.STATUS_FILE, "utf8") : ""; +const before = status(); +const stateDir = dirname(process.env.STATUS_FILE); +const genFile = join(stateDir, process.env.TASK_ID + ".busy-gen"); +const gen = readFileSync(genFile, "utf8").trim(); +const busy = () => { + const [version, ...fields] = readFileSync(join(stateDir, process.env.TASK_ID + ".busy-state"), "utf8").trim().split(" "); + assert.equal(version, "v1"); + const { ts, ...record } = Object.fromEntries(fields.map((field) => field.split("="))); + assert.match(ts, /^\d+$/); + return record; +}; +const seeded = busy(); +assert.deepEqual(seeded, { gen, seq: "1", state: "busy", source: "fm-spawn", event: "launch-brief" }); +const handlers = {}; +let tools = []; +const extension = await import(pathToFileURL(process.env.EXT_PATH).href); +extension.default({ + on: (name, handler) => { handlers[name] = handler; }, + events: { on() {} }, + getAllTools: () => tools, +}); +assert.equal(status(), before, "registration must not validate against a registry that has not loaded yet"); +assert.deepEqual(busy(), seeded, "registration must not change busy state"); +if (process.env.SCENARIO !== "unverified") { + tools = [{ name: "mcp__tracker__editIssue" }, { name: "mcp__tracker__createIssue" }]; +} +rmSync(process.env.TURNEND, { force: true }); +await handlers.agent_start(); +const started = { gen, seq: "2", state: "busy", source: "pi-ext", event: "agent-start" }; +assert.deepEqual(busy(), started, "agent_start must publish a generation-bound Pi busy event"); +const after = status(); +if (process.env.SCENARIO === "matched") { + assert.equal(after, before, "an exact match must produce no report"); +} else { + const report = after.slice(before.length); + assert.match(report, /^note \[at=\d+\]: warning: /); + assert.ok(report.includes(process.env.EXCLUDE_FILE), "the report must identify the config file"); + assert.ok(report.includes("unmatched exclusion entries")); + assert.ok(report.includes("unverified"), "absence must never imply validity"); + const missing = process.env.SCENARIO === "unmatched" + ? ["mcp__tracker__editIssu", "mcp__tracker__createIssu"] + : ["mcp__offline__executeWrite"]; + for (const name of missing) assert.ok(report.includes(name), "every unmatched entry must be reported: " + name); + assert.ok(!report.includes("mcp__tracker__editIssue"), "a loaded tool must not be reported"); +} +await handlers.agent_settled({}, { isIdle: () => false }); +assert.deepEqual(busy(), started, "a continuation must stay busy even when agent_settled fires"); +await handlers.turn_end(); +for (let attempt = 0; attempt < 100 && !existsSync(process.env.TURNEND); attempt++) { + await setTimeout(10); +} +assert.ok(existsSync(process.env.TURNEND), "reporting must preserve turn-end notification"); +assert.deepEqual(busy(), started, "an inner turn boundary must not mark the worker idle"); +await handlers.agent_settled({}, { isIdle: () => true }); +assert.deepEqual(busy(), { gen, seq: "3", state: "idle", source: "pi-ext", event: "agent-settled" }); +await handlers.agent_start(); +assert.deepEqual(busy(), { gen, seq: "4", state: "busy", source: "pi-ext", event: "agent-start" }); +assert.equal(status(), after, "report once per worker incarnation, not on every turn"); +appendFileSync(process.env.STATUS_FILE, "done: completed task\n"); +const completed = status(); +await handlers.agent_settled({}, { isIdle: () => true }); +assert.deepEqual(busy(), { gen, seq: "5", state: "idle", source: "pi-ext", event: "agent-settled" }); +assert.equal(status(), completed, "a reporting extension must not supersede a terminal worker status"); +const replacementGen = execFileSync(process.env.BUSY_EVENT, ["arm", stateDir, process.env.TASK_ID], { encoding: "utf8" }).trim(); +assert.notEqual(replacementGen, gen, "relaunch must mint a new generation"); +const replacement = busy(); +assert.deepEqual(replacement, { gen: replacementGen, seq: "1", state: "busy", source: "fm-spawn", event: "launch-brief" }); +await handlers.agent_settled({}, { isIdle: () => true }); +assert.deepEqual(busy(), replacement, "a stale extension must not clear its replacement's busy state"); +await handlers.agent_start(); +assert.deepEqual(busy(), replacement, "a stale extension must not publish into its replacement's generation"); +assert.equal(readFileSync(genFile, "utf8").trim(), replacementGen); +assert.equal(status(), completed, "stale lifecycle events must not repeat exclusion warnings"); +JS + ) + status=$? + expect_code 0 "$status" "$harness $kindflag $scenario worker registry check failed: $out" + if [ "$scenario" != matched ]; then + report=$(bash -c '. "$1/bin/fm-classify-lib.sh"; scan_unread_surface_lines "$2"' \ + _ "$ROOT" "$HOME_DIR/state") + expect_code 0 "$?" "the supervisor's unread-status consumer should succeed" + assert_contains "$report" "$HOME_DIR/config/crew-exclude-tools" "the supervisor must see the exclusion warning even after done" + assert_contains "$report" "unverified" "the supervisor must see that exclusions remain unverified" + fi + done + done + done + pass "Pi worker registries report unmatched and unverified exclusions without reporting exact matches" +} + +test_pi_exclude_tools_absent_and_empty_lists() { + local rec id out status launch + id=excl-absent-z9b + rec=$(make_spawn_case excl-absent pi "$id") + read_case_record "$rec" + out=$(run_ship_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR") + expect_code 0 "$?" "Pi spawn without the file should succeed" + assert_not_contains "$(cat "$LAUNCH_LOG")" "--exclude-tools" "an absent file must add no exclusions" + + id=excl-empty-z9c + rec=$(make_spawn_case excl-empty pi "$id") + read_case_record "$rec" + write_exclude_file "$HOME_DIR" '# nothing yet' '' + out=$(run_ship_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR") + expect_code 0 "$?" "Pi spawn with a comment-only file should succeed" + assert_not_contains "$(cat "$LAUNCH_LOG")" "--exclude-tools" "a file with no entries must add no exclusions" + + local harness + for harness in claude codex; do + id="excl-${harness}-empty-z9d" + rec=$(make_spawn_case "excl-${harness}-empty" "$harness" "$id") + read_case_record "$rec" + write_exclude_file "$HOME_DIR" '# nothing yet' '' + out=$(run_ship_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR") + status=$? + expect_code 0 "$status" "$harness spawn with an empty exclusion list should succeed"$'\n'"$out" + assert_not_contains "$(cat "$LAUNCH_LOG")" "--exclude-tools" "$harness must receive no exclusion flag" + done + pass "an absent or empty exclusion list leaves every runtime unaffected" +} + +test_exclude_tools_non_pi_runtime_refuses_non_empty_list() { + local harness rec id out status + for harness in claude codex grok; do + id="excl-refuse-${harness}-z9i" + rec=$(make_spawn_case "excl-refuse-${harness}" "$harness" "$id") + read_case_record "$rec" + write_exclude_file "$HOME_DIR" 'mcp__srv__writeTool' + out=$(run_ship_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR") + status=$? + expect_code 1 "$status" "$harness spawn with a non-empty exclusion list must refuse" + assert_contains "$out" "config/crew-exclude-tools" "the $harness refusal must name the config file" + assert_contains "$out" "$harness runtime cannot hide tools" "the $harness refusal must name the runtime" + [ ! -s "$LAUNCH_LOG" ] || fail "$harness launched despite an unhonorable exclusion list" + [ ! -e "$HOME_DIR/state/$id.meta" ] || fail "$harness refusal still wrote a task record" + done + # A raw launch command cannot receive the flag, Pi-named or not. + id=excl-refuse-raw-z9j + rec=$(make_spawn_case excl-refuse-raw pi "$id") + read_case_record "$rec" + write_exclude_file "$HOME_DIR" 'mcp__srv__writeTool' + out=$(run_ship_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR" "pi Verify this adapter") + status=$? + expect_code 1 "$status" "a raw launch with a non-empty exclusion list must refuse" + assert_contains "$out" "config/crew-exclude-tools" "the raw-launch refusal must name the config file" + [ ! -s "$LAUNCH_LOG" ] || fail "a raw launch ignored the exclusion list" + pass "a runtime that cannot hide tools, or a raw launch, refuses a non-empty exclusion list" +} + +test_pi_exclude_tools_malformed_entry_refuses_before_endpoint() { + local rec id out status bad n=0 + for bad in 'two words' 'a,b' 'mcp__srv__tool;rm' '*' 'mcp__srv__*Admin' 'quote'"'"'d' "mcp__srv__\$X"; do + n=$((n + 1)) + id="excl-bad-$n-z9e" + rec=$(make_spawn_case "excl-bad-$n" pi "$id") + read_case_record "$rec" + write_exclude_file "$HOME_DIR" 'mcp__srv__good' "$bad" + out=$(run_ship_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR") + status=$? + expect_code 1 "$status" "malformed entry '$bad' must refuse the spawn" + assert_contains "$out" "config/crew-exclude-tools has a malformed entry '$bad'" \ + "the refusal must name the malformed entry '$bad'" + [ ! -s "$LAUNCH_LOG" ] || fail "a malformed exclusion entry '$bad' still launched a worker" + [ ! -e "$HOME_DIR/state/$id.meta" ] || fail "a malformed exclusion entry '$bad' still wrote a task record" + done + pass "a malformed exclusion entry refuses the spawn before any launch or record" +} + +test_pi_exclude_tools_read_failure_refuses_before_launch() { + local harness kind rec id out status bash_env + for harness in pi pi-signed; do + for kind in ship scout; do + id="excl-read-failure-${harness}-${kind}" + rec=$(make_spawn_case "$id" "$harness" "$id") + read_case_record "$rec" + write_exclude_file "$HOME_DIR" 'mcp__srv__writeTool' + bash_env="$CASE_DIR/remove-exclusions.bash" + # Make the file disappear immediately after its readability check, without + # sleeps or permission assumptions (chmod alone would not fail as root). + cat > "$bash_env" <<'SH' +function [ { + local status=0 + builtin test "${@:1:$#-1}" || status=$? + if builtin [ "$#" -eq 4 ] && builtin [ "$1" = '!' ] && + builtin [ "$2" = -r ] && + builtin [ "$3" = "${FM_CONFIG_OVERRIDE:-}/crew-exclude-tools" ]; then + rm -f -- "$3" + fi + return "$status" +} +SH + if [ "$kind" = scout ]; then + out=$(BASH_ENV="$bash_env" run_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR" --scout) + else + out=$(BASH_ENV="$bash_env" run_ship_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR") + fi + status=$? + [ ! -e "$HOME_DIR/config/crew-exclude-tools" ] || fail "the read-failure injection did not remove the list" + expect_code 1 "$status" "$harness $kind must refuse when the checked exclusion file cannot be opened" + assert_contains "$out" "error: cannot read config/crew-exclude-tools" "the refusal must explain the read failure" + [ ! -s "$LAUNCH_LOG" ] || fail "$harness $kind launched despite the unreadable exclusion list" + [ ! -e "$HOME_DIR/state/$id.meta" ] || fail "$harness $kind read failure still wrote a task record" + done + done + pass "Pi and pi-signed ship and scout launches refuse when the exclusion file disappears after its checks" +} + +test_pi_exclude_tools_do_not_leak_across_homes_or_to_secondmates() { + local rec_a rec_b id_a id_b launch sm out status + id_a=excl-home-a-z9f + id_b=excl-home-b-z9g + rec_a=$(make_spawn_case excl-home-a pi "$id_a") + read_case_record "$rec_a" + write_exclude_file "$HOME_DIR" 'mcp__srv__writeTool' + run_ship_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id_a" "$PROJ_DIR" >/dev/null + expect_code 0 "$?" "home A spawn should succeed" + assert_contains "$(cat "$LAUNCH_LOG")" "--exclude-tools 'mcp__srv__writeTool'" "home A lost its own exclusions" + + # A second home running the same code, with no file of its own, inherits nothing. + rec_b=$(make_spawn_case excl-home-b pi "$id_b") + read_case_record "$rec_b" + run_ship_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id_b" "$PROJ_DIR" >/dev/null + expect_code 0 "$?" "home B spawn should succeed" + assert_not_contains "$(cat "$LAUNCH_LOG")" "--exclude-tools" "home B must not inherit home A's exclusions" + + # A seeded secondmate agent is unaffected even in the home that has the file. + id_a=excl-sm-z9h + rec_a=$(make_spawn_case excl-sm codex "$id_a") + read_case_record "$rec_a" + printf '%s\n' pi > "$HOME_DIR/config/secondmate-harness" + write_exclude_file "$HOME_DIR" 'mcp__srv__writeTool' + sm="$CASE_DIR/secondmate-home" + make_seeded_secondmate_home "$sm" "$id_a" + sm=$(cd "$sm" && pwd -P) + out=$(run_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id_a" "$sm" --secondmate) + status=$? + expect_code 0 "$status" "Pi secondmate spawn should succeed"$'\n'"$out" + launch=$(cat "$LAUNCH_LOG") + assert_contains "$launch" "fm-primary-turnend-guard.ts" "secondmate launch was not the Pi secondmate shape" + assert_not_contains "$launch" "--exclude-tools" "a secondmate's own agent must not receive the worker exclusions" + pass "exclusions stay in the home that configured them and skip secondmate agents" +} + test_batch_forwards_shared_profile_flags() { local rec id1 id2 out status id1=profile-batch-a-z9 @@ -1931,6 +2229,13 @@ test_pi_signed_persistent_secondmate_uses_pi_extensions_and_identity test_pi_seeded_secondmate_preapproves_project_trust test_pi_worker_launch_omits_seeded_home_approve test_pi_approve_probe_omits_unsupported_flag +test_pi_exclude_tools_reach_ship_and_scout_launches +test_pi_exclude_tools_worker_registry_reports +test_pi_exclude_tools_absent_and_empty_lists +test_exclude_tools_non_pi_runtime_refuses_non_empty_list +test_pi_exclude_tools_malformed_entry_refuses_before_endpoint +test_pi_exclude_tools_read_failure_refuses_before_launch +test_pi_exclude_tools_do_not_leak_across_homes_or_to_secondmates test_batch_forwards_shared_profile_flags test_claude_forwards_firstmate_config_dir_when_set test_lavish_server_address_is_exported_to_worker_launch From ac0811c4c820d82009d7a3b8f09555cbf38340c8 Mon Sep 17 00:00:00 2001 From: Tiago Date: Wed, 7 Oct 2026 11:32:21 -0300 Subject: [PATCH 4/7] feat(bin): defer spawns beyond a declared per-project capacity, opt-in (#5343) * refactor(bin): share the local Firstmate home walk from the wake library Teardown's walk over the root home and its registered local secondmate homes moves into bin/fm-wake-lib.sh as fm_local_firstmate_state_dirs, next to fm_firstmate_root_home, so a second consumer can count task records across this machine's homes without a copy. Teardown keeps its exact refusal wording through a thin wrapper. * feat(bin): defer spawns beyond a project's declared machine capacity A project whose machine-local resource only serves a few workers at once had no way to tell Firstmate so: every queued item was launched, and the surplus workers spent full-context turns retrying the resource. config/project-capacity in the root home now declares how many workers each named project admits at once on this machine. bin/fm-spawn.sh counts the ship and scout records on the same project origin across the root and its local secondmate homes, skipping ones whose ready PR is recorded, while holding the shared project lock through publication. A spawn with every place held exits 75 before any brief render, endpoint, worktree, record, or backlog move, so the item stays queued; batches report it as deferred. Undeclared projects keep today's uncapped dispatch, and an unreadable declaration refuses rather than guessing the limit. Refs #4237 * no-mistakes(review): Document that capacity matches the clone directory name * no-mistakes(document): Rewrap stale fm-wake-lib root-home doc comment * no-mistakes(review): Dedupe local state dirs by identity to avoid double-counting * no-mistakes(document): Rewrap fm_local_firstmate_state_dirs error doc comment * no-mistakes(ci): I fixed all four Greptile findings. All 14 tests in tests/fm-project-capacity.test.sh pass, and shellcheck at warning level is clean on the changed files. Each new test failed against the old code and passes now. - **ci-1 (spaced names):** a declaration line must give a name its capacity whenever the name is a valid clone directory name. `fm_project_capacity_lookup` now trims each line, skips blank lines and lines whose first non-blank character is `#`, and takes the last field as the capacity. Everything before that field is the name, so it may contain spaces. The old error cases still refuse: a single field is rejected, and trailing text leaves a last field that is not an integer. The library header and docs/configuration.md now say a name starting with `#` cannot be declared. New test `test_spaced_project_name_is_declared` declares `my heavy project 1` next to an indented comment line and gets a deferral. - **ci-2 (unreadable records):** the holder count must never silently leave out a holder. `fm_project_capacity_occupants` now refuses when a local home's state directory exists but cannot be read or listed, or when a `.meta` file cannot be read. The error names the path, and `fm-spawn.sh` shows it in its existing refusal message. New test `test_unreadable_holders_refuse_admission` covers an unreadable record in the root home and an unreadable state directory in a registered local secondmate home, then checks that the spawn is admitted once both are readable. The test is skipped when run as root. - **ci-3 (Orca lock):** any spawn that can become a holder for a capped project must take that project's lock. The lookup now also reports whether the declaration caps any project at all, and an Orca spawn takes the per-origin lock whenever it does. This covers every capped same-origin clone. It also covers some cases where no same-origin clone is capped, because a spawn cannot find clones under other directory names without searching for them. With no declaration file, Orca still skips the lock. The comments in the library and in the `fm-spawn.sh` header are updated. The Orca test now clones the origin as `project-2`, which has no declaration, and checks that its Orca spawn refuses while the lock is held and publishes no record. - **ci-4 (worktrees):** `assert_nothing_created` now also compares the project's `git worktree list` from before and after a deferred spawn. Both tests that call it take that snapshot first. Files changed: bin/fm-project-capacity-lib.sh, bin/fm-spawn.sh, docs/configuration.md, tests/fm-project-capacity.test.sh * fix(bin): declare capacity for a project name that begins with # A clone directory whose name begins with # was skipped as a comment, so that project stayed uncapped. A line is a declaration when the # is written against the rest of the name and the line ends with a capacity; a # followed by whitespace stays a comment. * no-mistakes(document): Rewrap project-capacity library header comment * no-mistakes(ci): Lint 2 fails because this PR's code pushes ShellCheck past its memory cap. ShellCheck ran out of memory analyzing bin/fm-teardown.sh in CI (reason=memory, rc=251, peak about 8.39 GB). On current main the same file passes at about 7.29 GB. **Cause:** the new `fm_local_firstmate_state_dirs` function in bin/fm-wake-lib.sh had a conditional `. fm-secondmate-registry-lib.sh` with a `# shellcheck source=` directive inside the function. ShellCheck followed that source again, inside a function scope, wherever fm-wake-lib.sh is sourced, and bin/fm-teardown.sh is the heaviest root that sources it. Measured locally with `shellcheck --norc --external-sources bin/fm-teardown.sh`: - current main (fd325b1b): 7.29 GB - main merged with this PR: 7.86 GB - the same merge without the in-function source: 7.27 GB **Rule this restores:** this change must not make any lint root heavier than it is on main. That function holds the only new nested source in the change. **Fix:** I removed the in-function source, which no caller needs. Both callers already load the registry library at top level before calling the function: - bin/fm-teardown.sh sources it directly. - bin/fm-spawn.sh, the only user of bin/fm-project-capacity-lib.sh, gets it through bin/fm-ff-lib.sh. I also documented the requirement in the function's comment and in the "Requires" note in bin/fm-project-capacity-lib.sh. No behaviour changes. **Verification:** - ShellCheck on head: bin/fm-teardown.sh peaks at 7.12 GB and bin/fm-spawn.sh at 6.68 GB, both with rc=0. bin/fm-wake-lib.sh and bin/fm-project-capacity-lib.sh lint clean. - tests/fm-project-capacity.test.sh, tests/fm-teardown.test.sh (102 ok) and tests/fm-teardown-endpoint-safety.test.sh all pass. Files changed: bin/fm-wake-lib.sh, bin/fm-project-capacity-lib.sh * fix(bin): release the Herdr session lock when reclaim finishes A concurrent resume in another home waits five seconds for that lock. Reclaim is the last presentation change on the recovery path, so holding the lock through the launch tail made the waiter time out. The contributions arm check also freezes its one-second clock, the same way the budget tests do, because an unfrozen clock can tick past before the first forge read. * no-mistakes(review): Keep Herdr session lock through launch handoff after reclaim * no-mistakes(review): Skip the spawning task's own record in capacity count * no-mistakes(review): Restore release test comment above its test * docs: scope PR-ready re-evaluation to a declared project capacity A ready pull request frees a place only when that project declares capacity, so the always-loaded backlog contract should re-evaluate on that handoff only in that case. --- .../skills/operational-home-layout/SKILL.md | 1 + AGENTS.md | 4 +- bin/fm-pr-check.sh | 2 + bin/fm-project-capacity-lib.sh | 229 +++++++ bin/fm-spawn.sh | 76 ++- bin/fm-teardown.sh | 47 +- bin/fm-test-run.sh | 1 + bin/fm-wake-lib.sh | 75 +- docs/configuration.md | 23 + tests/fm-contributions.test.sh | 3 + tests/fm-project-capacity.test.sh | 642 ++++++++++++++++++ 11 files changed, 1043 insertions(+), 60 deletions(-) create mode 100644 bin/fm-project-capacity-lib.sh create mode 100755 tests/fm-project-capacity.test.sh diff --git a/.agents/skills/operational-home-layout/SKILL.md b/.agents/skills/operational-home-layout/SKILL.md index 2786265f63b..49c728d93af 100644 --- a/.agents/skills/operational-home-layout/SKILL.md +++ b/.agents/skills/operational-home-layout/SKILL.md @@ -24,6 +24,7 @@ config/crew-harness crewmate harness override; LOCAL, gitignored; absent or "de config/claude-permission-mode optional one-token permission posture for every Claude worker launch: absent or "bypass" keeps --dangerously-skip-permissions, "auto" launches with --permission-mode auto; LOCAL, gitignored; inherited by secondmate homes; see docs/configuration.md "Claude permission mode" config/claude-account config/pi-account optional per-home worker account pin for Claude and Pi launches; LOCAL, gitignored, not inherited; absent keeps today's ambient account; present refuses a launch unless the pinned account resolves and is signed in (section 4 owns the refusal rule); see docs/configuration.md "Worker account pin" config/crew-dispatch.json optional crewmate dispatch profiles; LOCAL, gitignored; firstmate-maintained but human-editable natural-language rules that choose a per-task harness/model/effort profile (section 4). Inherited by secondmate homes +config/project-capacity optional per-machine count of workers each named project admits at once, read from the root home by every local home; LOCAL, gitignored; see docs/configuration.md "Project capacity" config/secondmate-harness harness the PRIMARY uses to launch SECONDMATE agents, optionally followed by a model and effort token on the same line (" [] []"; section 4); LOCAL, gitignored; absent or "default" harness falls back to config/crew-harness then firstmate's own. The primary's own setting; NOT inherited into secondmate homes (secondmates do not spawn secondmates) config/backlog-backend backlog backend override; LOCAL, gitignored; absent or "tasks-axi" = the configured tasks-axi backend, "manual" = force routine backlog updates to hand-editing; inherited by secondmate homes (section 10) config/backend runtime session-provider backend override for new tasks; LOCAL, gitignored; absent = falls through to runtime auto-detection (the runtime firstmate itself is executing inside), then tmux; tmux is the verified reference backend (docs/tmux-backend.md), herdr has its own required CI lane (docs/herdr-backend.md), while zellij, orca, and cmux remain experimental with no dedicated real-backend CI lane (docs/zellij-backend.md, docs/orca-backend.md, docs/cmux-backend.md) - herdr and cmux can also be selected by runtime auto-detection, zellij and orca never are (always explicit), and codex-app is not accepted; see docs/codex-app-backend.md; inherited by secondmate homes under the primary-authoritative contract in secondmate-provisioning diff --git a/AGENTS.md b/AGENTS.md index fc3fbb67cc7..90be0c1d908 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -61,7 +61,6 @@ Tracked files hold shared instructions and tooling; `data/` holds durable privat Load `operational-home-layout` when locating, interpreting, or changing Firstmate home, config, data, state, project, or generated runtime paths. - A `state/.status` line is a wake event, not current-state truth; `bin/fm-crew-state.sh` owns current-state reconciliation. Treat `data/captain.md` as the domain-local record of captain preferences, optional `data/captain-shared.md` as the main-authoritative shared captain-preference file for secondmate inheritance, and `data/learnings.md` as curated home-local knowledge, regardless of harness memory. @@ -195,6 +194,7 @@ An unregistered project or absent registry resolves to `no-mistakes` with yolo o Record the resulting mode, `yolo` merge posture, and the one-line reason for any deviation in the backlog item note. Treat file or subsystem overlap as a risk signal rather than an automatic reason to wait, and dispatch isolated work immediately with no concurrency cap when each change can be independently implemented and validated and the selected delivery path can reconcile ordinary rebases or conflicts. +A project's declared machine capacity (`config/project-capacity`) still bounds that dispatch: a spawn beyond it exits 75 without launching, and its item stays queued rather than blocked. Serialize only for a true semantic dependency, shared mutable external state, incompatible concurrent migration, or another concrete condition that makes independent progress or reconciliation unsafe; same-file editing alone is insufficient, and genuine blockers remain durable. Write the task-specific brief under section 11 before spawning. Fill the task subsections according to section 11. @@ -366,7 +366,7 @@ A decision is simply a task held for the captain: create the task with `bin/fm-t When a main-side thread such as a pending captain decision or relay reminder is worth durable tracking, file it as its own work item and hold it through that wrapper. Captain calls discovered by investigations or visual reviews follow `captain-hold-lifecycle`, which owns their completion gate and recorded-answer rules. When the automatic transition gate applies, dispatch and completion move the item themselves - `bin/fm-spawn.sh` and `bin/fm-teardown.sh` own those transitions and refuse rather than report success without them - so what remains yours is filing the item before dispatch, recording decisions, and keeping notes current; `docs/configuration.md` owns gate applicability and the manual-backend exception. -Re-evaluate queued work after every teardown and heartbeat, dispatching items only when dependencies and time gates have cleared. +Re-evaluate queued work after every teardown and heartbeat, and also after a recorded PR-ready handoff when `config/project-capacity` caps that project, dispatching items only when dependencies, time gates, and project capacity have cleared. `.tasks.toml`, `docs/configuration.md`, and current `tasks-axi --help` own the backlog schema, compatibility, retention, and routine command syntax. Use compatible `tasks-axi` when the configured backend selects it, always through `bin/fm-tasks-axi.sh` so the call reaches this home's backlog from any directory, and the documented manual path otherwise; keep only the configured recent Done entries. diff --git a/bin/fm-pr-check.sh b/bin/fm-pr-check.sh index 4091bcce5be..977225c4357 100755 --- a/bin/fm-pr-check.sh +++ b/bin/fm-pr-check.sh @@ -17,6 +17,8 @@ # draft state does not refuse, matching how the head read below is optional. # bin/fm-pr-merge.sh records through this script with FM_PR_CHECK_MERGE=1 and # skips this refusal, because its own merge-time draft refusal is authoritative. +# The recorded pr= also frees the task's place in a declared project capacity +# (bin/fm-project-capacity-lib.sh). # Usage: fm-pr-check.sh set -eu diff --git a/bin/fm-project-capacity-lib.sh b/bin/fm-project-capacity-lib.sh new file mode 100644 index 00000000000..a184c3d28a9 --- /dev/null +++ b/bin/fm-project-capacity-lib.sh @@ -0,0 +1,229 @@ +#!/usr/bin/env bash +# fm-project-capacity-lib.sh - how many workers a project admits at once on this +# machine, and whether a fresh worker spawn still fits. +# +# A project can depend on a machine-local resource that only a few workers can +# use at the same time: a heavy test suite, a local editor stack, a device. +# Firstmate cannot see which part of a worker's life touches that resource, so +# the captain declares how many workers the project admits on this machine, and +# bin/fm-spawn.sh defers a fresh worker beyond that number instead of launching +# it only to spend full-context turns retrying the resource. A deferred task +# keeps its queued backlog item and is dispatched again when a place frees. +# Without a declaration nothing changes and dispatch stays uncapped +# (AGENTS.md section 7). +# +# This file is the single owner of the declaration format, of what holds a +# place, and of the admission verdict. docs/configuration.md "Project capacity" +# is the operator reference, and bin/fm-spawn.sh owns where the check runs. +# +# Declaration: config/project-capacity in the local root Firstmate home (the home +# bin/fm-wake-lib.sh's fm_firstmate_root_home resolves), so every home on this +# machine reads the same number for the same machine's resources. One line per +# project: +# +# is the project's registered name, which is the basename of its +# clone directory, and is a positive integer of at most six digits. +# The capacity is the last whitespace-separated field, so the name before it may +# contain spaces. Blank lines are ignored. A line is a comment when it is `#`, +# when `#` is followed by whitespace, or when it starts with `#` and its last +# field is not an integer. A project whose name begins with `#` is declared by +# writing that `#` immediately against the rest of the name and ending the line +# with the capacity, for example `#repo 2`. A name that is `#`, or that begins +# with `#` followed by a space, is the same spelling as a comment and cannot be +# declared. Any other shape, a project named twice, or an unreadable file makes +# the whole declaration unreadable, and bin/fm-spawn.sh then refuses every fresh +# ship or scout spawn from this machine's homes rather than guessing which limit +# was meant. +# +# Occupancy: a place is held by every task record, in any local Firstmate home on +# this machine (fm_local_firstmate_state_dirs in bin/fm-wake-lib.sh), that +# - is not a secondmate, which is a persistent home rather than a worker, +# - names the same project identity, meaning its project resolves to the same +# shared project lock path (fm_treehouse_project_lock_path), which is keyed +# by the project's resolved origin, so workers in any clone of that origin +# are counted, and +# - has no recorded PR handoff: the pr= line bin/fm-pr-check.sh records when a +# worker's PR is ready, after which the worker waits on review or merge and +# no longer uses local resources. +# A place therefore frees when a PR-based ship records its ready PR, or when any +# task is cleaned up and its record removed. A local-only ship and a scout have +# no recorded handoff and hold their place until cleanup. A worker that is +# steered back into work after its PR handoff is not counted again. +# The declaration is matched by the spawning clone's directory name, so clones +# of one origin share the cap only when they use that same directory name. A +# clone of that origin under a different directory name finds no declaration +# and is not capped, though its workers still count as holders for a +# same-origin clone that is capped. +# A record whose project directory no longer exists cannot be matched and holds +# no place. Remote homes are never walked, because their workers run on another +# machine. +# +# Race safety: bin/fm-spawn.sh evaluates admission while holding the shared +# project lock and keeps holding it until the new task record is published, so +# two concurrent spawns for one project can never both publish from the same +# count. A spawn on an uncapped project can still publish a holder for a capped +# same-origin clone, so every backend takes that lock whenever the declaration +# caps any project; Orca, which otherwise never takes it, is included. Freeing a +# place needs no lock, because removing a record or adding pr= only ever lowers +# the count. +# +# Requires bin/fm-wake-lib.sh (root home, local homes, project lock path), +# bin/fm-secondmate-registry-lib.sh (which the local-homes walk reads), and +# bin/fm-backend.sh (fm_meta_get) to be sourced first. No side effects on source. + +# Exit status of a spawn deferred because the project is at capacity: the +# sysexits "temporary failure" code, so a caller can tell a deferral that leaves +# the task queued from an ordinary failure. +# shellcheck disable=SC2034 # read by bin/fm-spawn.sh after sourcing. +FM_PROJECT_CAPACITY_DEFER_EXIT=75 + +# The config directory holding this machine's declaration: the spawning home's +# own when that home is the local root (so an override of it +# applies), otherwise the root home's config/. +fm_project_capacity_config_dir() { # + local home=$1 config=$2 root home_real + root=$(fm_firstmate_root_home "$home") || return 1 + home_real=$(CDPATH='' cd -- "$home" 2>/dev/null && pwd -P) || return 1 + if [ "$root" = "$home_real" ]; then + printf '%s\n' "$config" + else + printf '%s/config\n' "$root" + fi +} + +# Read the declared capacity for one project. +# Sets FM_PROJECT_CAPACITY_FILE to the declaration path, FM_PROJECT_CAPACITY +# to the project's capacity, or to empty when the project declares none, and +# FM_PROJECT_CAPACITY_ANY to 1 when the declaration caps any project at all. +# Returns 1 with FM_PROJECT_CAPACITY_ERROR when the declaration is unreadable. +fm_project_capacity_lookup() { # + local name=$2 line lineno=0 pname pcap seen='|' + FM_PROJECT_CAPACITY_FILE="$1/project-capacity" + FM_PROJECT_CAPACITY= + FM_PROJECT_CAPACITY_ANY= + FM_PROJECT_CAPACITY_ERROR= + if [ ! -e "$FM_PROJECT_CAPACITY_FILE" ] && [ ! -L "$FM_PROJECT_CAPACITY_FILE" ]; then + return 0 + fi + if [ ! -f "$FM_PROJECT_CAPACITY_FILE" ] || [ ! -r "$FM_PROJECT_CAPACITY_FILE" ]; then + FM_PROJECT_CAPACITY_ERROR="$FM_PROJECT_CAPACITY_FILE is not a readable regular file" + return 1 + fi + while IFS= read -r line || [ -n "$line" ]; do + lineno=$((lineno + 1)) + line=${line%$'\r'} + line=${line#"${line%%[![:space:]]*}"} + line=${line%"${line##*[![:space:]]}"} + # '#' followed by whitespace is always a comment, including one that ends + # with a number. A line that begins with '#' glued to the rest of a name is + # a declaration only when its last field is an integer; any other such line + # stays a comment, so a note does not refuse every spawn. + case "$line" in + '' | '#' | '#'[[:space:]]*) continue ;; + '#'*) + case "${line##*[[:space:]]}" in + *[!0-9]*) continue ;; + esac + ;; + esac + # The capacity is the last field, so the name before it may hold spaces. + pcap=${line##*[[:space:]]} + pname=${line%"$pcap"} + pname=${pname%"${pname##*[![:space:]]}"} + if [ -z "$pname" ]; then + FM_PROJECT_CAPACITY_ERROR="$FM_PROJECT_CAPACITY_FILE line $lineno is not ' '" + FM_PROJECT_CAPACITY= + return 1 + fi + case "$pcap" in + '' | *[!0-9]* | 0*) + FM_PROJECT_CAPACITY_ERROR="$FM_PROJECT_CAPACITY_FILE line $lineno gives $pname a capacity that is not a positive integer" + FM_PROJECT_CAPACITY= + return 1 + ;; + esac + if [ "${#pcap}" -gt 6 ]; then + FM_PROJECT_CAPACITY_ERROR="$FM_PROJECT_CAPACITY_FILE line $lineno gives $pname a capacity longer than six digits" + FM_PROJECT_CAPACITY= + return 1 + fi + case "$seen" in + *"|$pname|"*) + FM_PROJECT_CAPACITY_ERROR="$FM_PROJECT_CAPACITY_FILE line $lineno names $pname a second time" + FM_PROJECT_CAPACITY= + return 1 + ;; + esac + seen="$seen$pname|" + [ "$pname" != "$name" ] || FM_PROJECT_CAPACITY=$pcap + done < "$FM_PROJECT_CAPACITY_FILE" + [ "$seen" = '|' ] || FM_PROJECT_CAPACITY_ANY=1 + return 0 +} + +# Count the task records holding a place in one project's capacity. +# is fm_treehouse_project_lock_path for the project being +# admitted, and is that project's own directory, which matches +# without recomputing its identity. The local homes come from +# fm_local_firstmate_state_dirs . is the task being +# admitted; its own record in is the one this spawn replaces, so +# it is not counted. +# Sets FM_PROJECT_CAPACITY_OCCUPANTS to the count and +# FM_PROJECT_CAPACITY_OCCUPANT_IDS to a comma-separated list of the holders, +# each outside qualified with its home. Returns 1 with +# FM_PROJECT_CAPACITY_ERROR when the local homes cannot be enumerated, or when +# a state directory or task record in them cannot be read, since skipping it +# could undercount the holders. +fm_project_capacity_occupants() { # + local want=$1 own=$2 first=$3 self=$4 state meta kind project lock id label i + local -a cache_dirs cache_locks + FM_PROJECT_CAPACITY_OCCUPANTS=0 + FM_PROJECT_CAPACITY_OCCUPANT_IDS= + FM_PROJECT_CAPACITY_ERROR= + fm_local_firstmate_state_dirs "$first" || { + FM_PROJECT_CAPACITY_ERROR=$FM_LOCAL_FIRSTMATE_ERROR + return 1 + } + cache_dirs=("$own") + cache_locks=("$want") + for state in "${FM_LOCAL_FIRSTMATE_STATES[@]}"; do + if [ -e "$state" ] && { [ ! -d "$state" ] || [ ! -r "$state" ] || [ ! -x "$state" ]; }; then + FM_PROJECT_CAPACITY_ERROR="local Firstmate state directory $state cannot be read" + return 1 + fi + for meta in "$state"/*.meta; do + [ -f "$meta" ] && [ ! -L "$meta" ] || continue + [ "$meta" != "$first/$self.meta" ] || continue + [ -r "$meta" ] || { + FM_PROJECT_CAPACITY_ERROR="task record $meta cannot be read" + return 1 + } + kind=$(fm_meta_get "$meta" kind) + [ "$kind" != secondmate ] || continue + [ -z "$(fm_meta_get "$meta" pr)" ] || continue + project=$(fm_meta_get "$meta" project) + [ -n "$project" ] || continue + lock= + i=0 + while [ "$i" -lt "${#cache_dirs[@]}" ]; do + if [ "${cache_dirs[$i]}" = "$project" ]; then + lock=${cache_locks[$i]} + break + fi + i=$((i + 1)) + done + if [ "$i" -ge "${#cache_dirs[@]}" ]; then + lock=$(fm_treehouse_project_lock_path "$project" 2>/dev/null) || lock= + cache_dirs+=("$project") + cache_locks+=("$lock") + fi + [ -n "$lock" ] && [ "$lock" = "$want" ] || continue + id=$(basename "$meta" .meta) + label=$id + [ "$state" = "$first" ] || label="$id in $(dirname "$state")" + FM_PROJECT_CAPACITY_OCCUPANTS=$((FM_PROJECT_CAPACITY_OCCUPANTS + 1)) + FM_PROJECT_CAPACITY_OCCUPANT_IDS="${FM_PROJECT_CAPACITY_OCCUPANT_IDS:+$FM_PROJECT_CAPACITY_OCCUPANT_IDS, }$label" + done + done + return 0 +} diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index d2feb0b65fd..445d9e13ca0 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -182,6 +182,20 @@ # fm_firstmate_root_home resolves, so a home seeded from another machine anchors # that lock itself rather than failing to resolve one; # contention refuses rather than waits. +# Project capacity: when this machine declares how many workers a project +# admits at once (config/project-capacity; bin/fm-project-capacity-lib.sh owns +# the declaration, what holds a place, and the race argument), a fresh ship or +# scout spawn counts the places already held while holding that same +# project-identity lock - taken on every backend whenever the declaration caps +# any project, Orca included, because an uncapped clone's worker still holds a +# place for a capped clone of the same origin - and holds it through metadata +# publication. A spawn that finds +# every place held prints one `deferred:` line and exits 75 before any brief +# render, endpoint, worktree, record, or backlog move exists, so the task stays +# exactly as queued as it was; an unreadable declaration refuses with exit 1. +# A batch reports such a pair as `batch: DEFERRED` and exits 75 when nothing +# else failed. A relaunch and a --secondmate spawn are never counted against +# capacity. # With no harness arg, a crewmate/scout spawn resolves the CREW harness only when # config/crew-dispatch.json is absent. When that file exists, crewmate/scout # spawns require an explicit harness so firstmate cannot silently skip dispatch @@ -665,6 +679,8 @@ fm_backlog_directory_present "$STATE" "state directory" || { . "$SCRIPT_DIR/fm-remote-readiness-lib.sh" # shellcheck source=bin/fm-timeout-lib.sh . "$SCRIPT_DIR/fm-timeout-lib.sh" +# shellcheck source=bin/fm-project-capacity-lib.sh +. "$SCRIPT_DIR/fm-project-capacity-lib.sh" # shellcheck source=bin/fm-worker-account-lib.sh . "$SCRIPT_DIR/fm-worker-account-lib.sh" # Fail closed before any fleet mutation: a no-mistakes gate agent must never spawn @@ -1554,16 +1570,17 @@ if [ "${#POS[@]}" -gt 0 ] && [ "${POS[0]}" != "$idpart" ] && case "$idpart" in * echo "error: batch dispatch does not support --secondmate; spawn each secondmate explicitly" >&2 rc=2 continue - elif [ "$KIND" = scout ]; then - if FM_SPAWN_NO_GUARD=1 "$FM_ROOT/bin/fm-spawn.sh" "${pair%%=*}" "${pair#*=}" "${shared_args[@]+"${shared_args[@]}"}" --scout; then :; else - echo "batch: FAILED to spawn ${pair%%=*} (${pair#*=})" >&2 - rc=1 - fi - else - if FM_SPAWN_NO_GUARD=1 "$FM_ROOT/bin/fm-spawn.sh" "${pair%%=*}" "${pair#*=}" "${shared_args[@]+"${shared_args[@]}"}"; then :; else - echo "batch: FAILED to spawn ${pair%%=*} (${pair#*=})" >&2 - rc=1 - fi + fi + pair_args=("${pair%%=*}" "${pair#*=}" "${shared_args[@]+"${shared_args[@]}"}") + [ "$KIND" != scout ] || pair_args+=(--scout) + pair_rc=0 + FM_SPAWN_NO_GUARD=1 "$FM_ROOT/bin/fm-spawn.sh" "${pair_args[@]}" || pair_rc=$? + if [ "$pair_rc" -eq "$FM_PROJECT_CAPACITY_DEFER_EXIT" ]; then + echo "batch: DEFERRED ${pair%%=*} (${pair#*=}) - its project is at capacity, so it stays queued" >&2 + [ "$rc" -ne 0 ] || rc=$FM_PROJECT_CAPACITY_DEFER_EXIT + elif [ "$pair_rc" -ne 0 ]; then + echo "batch: FAILED to spawn ${pair%%=*} (${pair#*=})" >&2 + rc=1 fi done exit "$rc" @@ -3099,17 +3116,52 @@ else WT="" BRIEF="$DATA/$ID/brief.md" fi -if [ "$RELAUNCH" -eq 0 ] && [ "$KIND" != secondmate ] && [ "$BACKEND" != orca ]; then +# Project capacity admission (bin/fm-project-capacity-lib.sh owns the +# declaration, what holds a place, and why this is race-safe). A fresh worker +# for a project whose declared capacity is already held is deferred here, before +# any brief render, endpoint, worktree, record, or backlog move exists, so the +# deferral leaves the task exactly as queued as it was. A relaunch replaces a +# worker that already holds a place, and a secondmate is not a worker. +SPAWN_PROJECT_CAPACITY= +SPAWN_PROJECT_CAPACITY_ANY= +if [ "$RELAUNCH" -eq 0 ] && [ "$KIND" != secondmate ]; then + SPAWN_CAPACITY_CONFIG=$(fm_project_capacity_config_dir "$FM_HOME" "$CONFIG") || { + echo "error: could not resolve the root Firstmate home that declares project capacity for $PROJ_ABS" >&2 + exit 1 + } + if ! fm_project_capacity_lookup "$SPAWN_CAPACITY_CONFIG" "$(basename "$PROJ_ABS")"; then + echo "error: spawn refused: the project capacity declaration is unreadable ($FM_PROJECT_CAPACITY_ERROR); fix it so the captain's worker limits are known (docs/configuration.md \"Project capacity\")" >&2 + exit 1 + fi + SPAWN_PROJECT_CAPACITY=$FM_PROJECT_CAPACITY + SPAWN_PROJECT_CAPACITY_ANY=$FM_PROJECT_CAPACITY_ANY +fi +if [ "$RELAUNCH" -eq 0 ] && [ "$KIND" != secondmate ] && + { [ "$BACKEND" != orca ] || [ -n "$SPAWN_PROJECT_CAPACITY_ANY" ]; }; then SPAWN_TREEHOUSE_PROJECT_LOCK=$(fm_treehouse_project_lock_path "$PROJ_ABS") || { echo "error: could not resolve the shared Treehouse project lock for $PROJ_ABS" >&2 exit 1 } if ! fm_lock_try_acquire "$SPAWN_TREEHOUSE_PROJECT_LOCK"; then - echo "error: another Treehouse slot allocation or return is in progress for $PROJ_ABS; refusing to race it" >&2 + if [ "$BACKEND" = orca ]; then + echo "error: another spawn or cleanup holds the shared project lock for $PROJ_ABS; refusing to race its capacity admission" >&2 + else + echo "error: another Treehouse slot allocation or return is in progress for $PROJ_ABS; refusing to race it" >&2 + fi exit 1 fi SPAWN_TREEHOUSE_PROJECT_LOCK_HELD=1 fi +if [ -n "$SPAWN_PROJECT_CAPACITY" ]; then + if ! fm_project_capacity_occupants "$SPAWN_TREEHOUSE_PROJECT_LOCK" "$PROJ_ABS" "$STATE" "$ID"; then + echo "error: spawn refused: project $(basename "$PROJ_ABS") declares a capacity of $SPAWN_PROJECT_CAPACITY, but this machine's task records cannot all be read to count it ($FM_PROJECT_CAPACITY_ERROR)" >&2 + exit 1 + fi + if [ "$FM_PROJECT_CAPACITY_OCCUPANTS" -ge "$SPAWN_PROJECT_CAPACITY" ]; then + echo "deferred: project $(basename "$PROJ_ABS") admits $SPAWN_PROJECT_CAPACITY worker(s) at once on this machine ($FM_PROJECT_CAPACITY_FILE) and $FM_PROJECT_CAPACITY_OCCUPANTS already hold a place ($FM_PROJECT_CAPACITY_OCCUPANT_IDS); task $ID was not launched and its backlog item stays queued - dispatch it again once one of them records its ready PR or is cleaned up" >&2 + exit "$FM_PROJECT_CAPACITY_DEFER_EXIT" + fi +fi [ -f "$BRIEF" ] || { echo "error: task $ID has no brief at inaccessible data path $BRIEF" >&2 exit 1 diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index 1a0f6c14e78..30b2d1b4507 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -2337,50 +2337,15 @@ teardown_live_slot_path() { canonical_existing_dir "$WT" } +# Every local Firstmate state directory whose records can name a pool slot this +# task's slot might also be; bin/fm-wake-lib.sh's fm_local_firstmate_state_dirs +# owns the walk and what it refuses. collect_local_firstmate_states() { - local record_state=$1 root home reg line child known existing i=0 - local -a homes - TREEHOUSE_OWNER_STATES=("$record_state") - root=$(fm_firstmate_root_home "$FM_HOME") || { - echo "REFUSED: cannot resolve the root Firstmate home; nothing was changed" >&2 + fm_local_firstmate_state_dirs "$1" || { + echo "REFUSED: $FM_LOCAL_FIRSTMATE_ERROR; nothing was changed" >&2 return 1 } - homes=("$root") - while [ "$i" -lt "${#homes[@]}" ]; do - home=${homes[$i]} - i=$((i + 1)) - known=0 - for existing in "${TREEHOUSE_OWNER_STATES[@]}"; do - [ "$existing" != "$home/state" ] || known=1 - done - [ "$known" = 1 ] || TREEHOUSE_OWNER_STATES+=("$home/state") - reg="$home/data/secondmates.md" - [ ! -e "$reg" ] && [ ! -L "$reg" ] && continue - [ -f "$reg" ] && [ ! -L "$reg" ] || { - echo "REFUSED: local Firstmate registry is unsafe at $reg; nothing was changed" >&2 - return 1 - } - while IFS= read -r line || [ -n "$line" ]; do - case "$line" in - "- "*) - secondmate_registry_parse_line "$line" || { - echo "REFUSED: malformed local Firstmate registry entry in $reg; nothing was changed" >&2 - return 1 - } - [ "$SECONDMATE_REGISTRY_REMOTE" -eq 0 ] || continue - child=$(canonical_existing_dir "$SECONDMATE_REGISTRY_HOME") || { - echo "REFUSED: registered local Firstmate home is unavailable: $SECONDMATE_REGISTRY_HOME; nothing was changed" >&2 - return 1 - } - known=0 - for existing in "${homes[@]}"; do - [ "$existing" != "$child" ] || known=1 - done - [ "$known" = 1 ] || homes+=("$child") - ;; - esac - done < "$reg" - done + TREEHOUSE_OWNER_STATES=("${FM_LOCAL_FIRSTMATE_STATES[@]}") } require_exclusive_worktree_slot_record() { diff --git a/bin/fm-test-run.sh b/bin/fm-test-run.sh index 0676f6dc517..340d57b48cf 100755 --- a/bin/fm-test-run.sh +++ b/bin/fm-test-run.sh @@ -393,6 +393,7 @@ family_for_basename() { fm-trace-context-spawn.test.sh|fm-spawn-worktree-settle.test.sh|\ fm-spawn-compact-adviser-disable.test.sh|\ fm-spawn-compact-adviser-disable-remote.test.sh|\ + fm-project-capacity.test.sh|\ fm-teardown-endpoint-safety.test.sh) printf '%s\n' backend-dispatch ;; diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index ddc708aadc8..c4d51a24731 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -1432,10 +1432,10 @@ fm_task_set_lock_path() { # # the walk at the current home, which is the correct answer rather than an # error: the parent lives on another machine, so its filesystem can neither hold # nor be observed by a lock taken here, and a remote-seeded home is itself the -# top of the local tree that bin/fm-teardown.sh's collect_local_firstmate_states -# enumerates (that walk already skips remote registry entries for the same -# reason). Refusing a remote binding instead made every operation anchored here -# fail closed inside a remote secondmate home and its local descendants. +# top of the local tree that fm_local_firstmate_state_dirs below enumerates +# (that walk already skips remote registry entries for the same reason). +# Refusing a remote binding instead made every operation anchored here fail +# closed inside a remote secondmate home and its local descendants. # # Everything else still fails closed: an unreadable or malformed binding, an # unreachable local parent, a cycle, and a chain deeper than the bound. @@ -1464,7 +1464,72 @@ fm_firstmate_root_home() { printf '%s\n' "$home" } -# The one lock serializing Treehouse slot allocation and return for a project. +# Every Firstmate state directory on THIS machine whose task records can share a +# machine-local resource with : itself, then the local +# root home and each local secondmate home registered below it, walked through +# every data/secondmates.md breadth-first. Remote registry entries are skipped, +# because their workers run on another machine. +# +# Sets FM_LOCAL_FIRSTMATE_STATES to that list, first and without +# duplicates however each directory is spelled. Returns 1 with +# FM_LOCAL_FIRSTMATE_ERROR naming what could not be proved - an unresolvable +# root, an unsafe or malformed registry, or an unavailable registered local +# home - so a caller refuses rather than treating an unreadable home as one +# with no tasks. Requires bin/fm-secondmate-registry-lib.sh to be sourced first. +# shellcheck disable=SC2034 # FM_LOCAL_FIRSTMATE_ERROR is read by callers. +fm_local_firstmate_state_dirs() { # + local first=$1 root home reg line child known existing i=0 + local -a homes + FM_LOCAL_FIRSTMATE_STATES=("$first") + FM_LOCAL_FIRSTMATE_ERROR= + root=$(fm_firstmate_root_home "$FM_HOME") || { + FM_LOCAL_FIRSTMATE_ERROR="cannot resolve the root Firstmate home" + return 1 + } + homes=("$root") + while [ "$i" -lt "${#homes[@]}" ]; do + home=${homes[$i]} + i=$((i + 1)) + known=0 + for existing in "${FM_LOCAL_FIRSTMATE_STATES[@]}"; do + if [ "$existing" = "$home/state" ] || [ "$existing" -ef "$home/state" ]; then + known=1 + fi + done + [ "$known" = 1 ] || FM_LOCAL_FIRSTMATE_STATES+=("$home/state") + reg="$home/data/secondmates.md" + [ ! -e "$reg" ] && [ ! -L "$reg" ] && continue + [ -f "$reg" ] && [ ! -L "$reg" ] || { + FM_LOCAL_FIRSTMATE_ERROR="local Firstmate registry is unsafe at $reg" + return 1 + } + while IFS= read -r line || [ -n "$line" ]; do + case "$line" in + "- "*) + secondmate_registry_parse_line "$line" || { + FM_LOCAL_FIRSTMATE_ERROR="malformed local Firstmate registry entry in $reg" + return 1 + } + [ "$SECONDMATE_REGISTRY_REMOTE" -eq 0 ] || continue + child=$([ -d "$SECONDMATE_REGISTRY_HOME" ] && + CDPATH='' cd -- "$SECONDMATE_REGISTRY_HOME" 2>/dev/null && pwd -P) || { + FM_LOCAL_FIRSTMATE_ERROR="registered local Firstmate home is unavailable: $SECONDMATE_REGISTRY_HOME" + return 1 + } + known=0 + for existing in "${homes[@]}"; do + [ "$existing" != "$child" ] || known=1 + done + [ "$known" = 1 ] || homes+=("$child") + ;; + esac + done < "$reg" + done +} + +# The one lock serializing Treehouse slot allocation and return for a project, +# and project capacity admission (bin/fm-project-capacity-lib.sh), which a +# fresh spawn evaluates under it on every backend. # # It is anchored in the local root home's state directory so that every home on # this machine that can reach the same pool - the root, and each secondmate home diff --git a/docs/configuration.md b/docs/configuration.md index 760c0195672..0eecb1af049 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1061,6 +1061,29 @@ When stripping is enabled, the hooks directory is read-only, so a hook manager r The flag is a home-wide attribution choice, so it is inherited into secondmate homes under the [`secondmate-provisioning`](../.agents/skills/secondmate-provisioning/SKILL.md) inherited-local-material contract and a secondmate's own workers keep AI trailers too. Per-machine Cursor `cli-config.json` attribution-off is not this contract: it does not travel with Firstmate, defaults back to on when unset, and only feeds the CLI's request to the server, so it suppresses the trailer rather than preventing it. +## Project capacity (config/project-capacity) + +The optional local, gitignored `config/project-capacity` tells Firstmate how many workers a project can run at once on this machine, for a project whose machine-local resource - a heavy test suite, a local editor stack, a device - only serves a few workers at a time. +Without it, dispatch stays uncapped as `AGENTS.md` section 7 describes, and a surplus worker is launched only to spend full-context turns waiting for the resource. +The file lives in the machine's root Firstmate home, so every local secondmate home reads the same limit, and it holds one line per project: + +```text +# heavy suite serves two workers +my-project 2 +``` + +The name is the project's registered name, which is its clone directory name and may contain spaces, and the number, the last field on the line, is a positive integer. +A line that is only `#`, or that begins with `#` followed by whitespace, is a comment, as is a `#` line whose last field is not an integer. +A project name may begin with `#` when that `#` is written immediately against the rest of the name and the line ends with the project's capacity. +A name that is `#`, or that begins with `#` and a space, cannot be declared, because that line is a comment. +A place is held by every ship or scout on that project in the root home or any local secondmate home registered under it, including one working in a separate clone of the same origin, until its ready PR is recorded or it is cleaned up; a local-only ship or a scout holds its place until cleanup. +The declaration is matched by the spawning clone's directory name, so clones of the same origin share the cap only when they use that same directory name. +A clone of that origin under a different directory name finds no declaration and is not capped, though its workers are still counted as holders for a same-origin clone that is capped. +When every place is held, `bin/fm-spawn.sh` launches nothing, creates no record, leaves the backlog item queued, prints one `deferred:` line naming the holders, and exits 75, so Firstmate dispatches the item again once a place frees. +A malformed or unreadable file refuses every fresh ship or scout spawn until it is fixed, rather than guessing the intended limit, and so does a local home's state directory or task record that cannot be read while counting a capped project's holders. +Firstmate cannot see which part of a worker's life uses the resource, so the number bounds whole workers from launch to handoff, and the tightest resource every worker needs should decide it. +[`bin/fm-project-capacity-lib.sh`](../bin/fm-project-capacity-lib.sh) owns the file format, what holds a place, and why concurrent spawns cannot both take the last one. + ## Crew dispatch profiles (config/crew-dispatch.json) `config/crew-dispatch.json` is an optional local, gitignored file containing natural-language rules that firstmate reads before dispatching a crewmate or scout. diff --git a/tests/fm-contributions.test.sh b/tests/fm-contributions.test.sh index 47ccb09854b..3c71ee9132e 100755 --- a/tests/fm-contributions.test.sh +++ b/tests/fm-contributions.test.sh @@ -1024,6 +1024,9 @@ test_arm_plumbs_a_configured_budget_into_the_check_shim() { # before the first forge call, so nothing is ever observed. /bin/date +%s > "$home/forge/clock" printf 'hang\n' > "$home/forge/fault" + # Freeze the clock. An unfrozen one-second budget can tick past before the + # first forge call, so the generated check never writes forge/calls. + /bin/date +%s > "$home/forge/clock" if [ "$mode" = configured ]; then with_home "$home" env FM_CONTRIBUTIONS_BUDGET=1 "$ROOT/bin/fm-contributions.sh" arm >/dev/null \ || fail 'arm with a configured budget failed' diff --git a/tests/fm-project-capacity.test.sh b/tests/fm-project-capacity.test.sh new file mode 100755 index 00000000000..bc64b8283b1 --- /dev/null +++ b/tests/fm-project-capacity.test.sh @@ -0,0 +1,642 @@ +#!/usr/bin/env bash +# Behavior tests for project capacity admission: a project that declares how +# many workers it admits at once on this machine never gets a fresh worker +# launched beyond that number (bin/fm-project-capacity-lib.sh owns the +# contract; bin/fm-spawn.sh runs the check). +# +# Every case drives the real bin/fm-spawn.sh against a real project clone with +# an origin, fake tmux and treehouse binaries that log every call, and a real +# markdown backlog when tasks-axi is installed. A deferred spawn is judged by +# what it left behind - no task record, no rendered launch brief, no endpoint, +# no worktree allocation, and a backlog item still queued - never by wording +# alone. +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +unset TASKS_AXI_BACKEND || : + +SPAWN="$ROOT/bin/fm-spawn.sh" +TEARDOWN="$ROOT/bin/fm-teardown.sh" +TMP_ROOT=$(fm_test_tmproot fm-project-capacity) +DEFER_EXIT=75 +HAVE_TASKS_AXI=0 +command -v tasks-axi >/dev/null 2>&1 && HAVE_TASKS_AXI=1 + +# --- fixture ---------------------------------------------------------------- + +write_brief() { # + mkdir -p "$1/data/$2" + cat > "$1/data/$2/brief.md" < [task-id...] + local home=$1 id + shift + mkdir -p "$home/state" "$home/config" "$home/data" "$home/projects" + touch "$home/state/.last-watcher-beat" + printf '%s\n' codex > "$home/config/crew-harness" + if [ "$HAVE_TASKS_AXI" = 1 ]; then + printf '%s\n' '# Backlog' '' '## In flight' '' '## Queued' '' '## Done' \ + > "$home/data/backlog.md" + cat > "$home/.tasks.toml" <<'EOF' +backend = "markdown" + +[markdown] +path = "data/backlog.md" +EOF + fi + for id in "$@"; do + write_brief "$home" "$id" + add_item "$home" "$id" + done +} + +# A case: one home, a project clone with an origin, and fake tmux/treehouse +# that record every call so a deferral can be proved to have created nothing. +make_case() { # [task-id...] + local name=$1 case_dir fakebin + shift + case_dir="$TMP_ROOT/$name" + mkdir -p "$case_dir" + fakebin=$(fm_fakebin "$case_dir") + : > "$case_dir/calls.log" + cat > "$fakebin/tmux" <<'SH' +#!/usr/bin/env bash +printf '%s\n' "$*" >> "$FM_FAKE_CALL_LOG" +case "$*" in + *"#{pane_current_path}"*) + # A spawn that is told to hold waits here, after its capacity check and + # while it still holds the project lock, until the test releases it. + if [ -n "${FM_FAKE_HOLD:-}" ]; then + : > "$FM_FAKE_HOLD.reached" + i=0 + while [ ! -f "$FM_FAKE_HOLD.release" ]; do + i=$((i + 1)) + [ "$i" -lt 400 ] || exit 1 + sleep 0.05 + done + fi + printf '%s\n' "${FM_FAKE_PANE_PATH:-}" + exit 0 + ;; +esac +case "${1:-}" in display-message) printf 'firstmate\n' ;; esac +exit 0 +SH + cat > "$fakebin/treehouse" <<'SH' +#!/usr/bin/env bash +printf 'treehouse %s\n' "$*" >> "$FM_FAKE_CALL_LOG" +exit 0 +SH + chmod +x "$fakebin/tmux" "$fakebin/treehouse" + fm_fake_exit0 "$fakebin" gh gh-axi no-mistakes + fm_git_init_commit "$case_dir/project" + fm_git_add_origin "$case_dir/project" "$case_dir/project.origin.git" + fm_git_init_commit "$case_dir/other-project" + fm_git_add_origin "$case_dir/other-project" "$case_dir/other-project.origin.git" + make_home "$case_dir/home" "$@" + printf '%s\n' "$case_dir" +} + +add_item() { # + [ "$HAVE_TASKS_AXI" = 1 ] || return 0 + tasks-axi add "$2" "item for $2" --kind ship --file "$1/data/backlog.md" >/dev/null +} + +row_state() { # + tasks-axi show "$2" --file "$1/data/backlog.md" 2>/dev/null | + sed -n 's/^ state: *//p' | head -1 +} + +declare_capacity() { # ... + local home=$1 + shift + printf '%s\n' "$@" > "$home/config/project-capacity" +} + +# A live worker already on a project: the record shape bin/fm-spawn.sh +# publishes, with its backlog item In flight so cleanup can close it. +write_live() { # [extra-line...] + local home=$1 id=$2 project=$3 + shift 3 + fm_write_meta "$home/state/$id.meta" \ + "window=firstmate:fm-$id" \ + "endpoint_task_id=$id" \ + "worktree=$home/absent-worktree-$id" \ + "project=$project" \ + "harness=codex" \ + "kind=ship" \ + "mode=no-mistakes" \ + "yolo=off" \ + "spawn_gen=s-$id" \ + "$@" + if [ "$HAVE_TASKS_AXI" = 1 ]; then + add_item "$home" "$id" + tasks-axi start "$id" --file "$home/data/backlog.md" >/dev/null + fi +} + +# One isolated worktree per spawn, so every admitted launch has its own copy. +new_worktree() { # + git -C "$1/project" worktree add --quiet -b "wt-$2" "$1/wt-$2" + printf '%s\n' "$1/wt-$2" +} + +run_spawn() { # + local case_dir=$1 home=$2 pane=$3 + shift 3 + FM_ROOT_OVERRIDE='' FM_HOME="$home" \ + FM_STATE_OVERRIDE='' FM_DATA_OVERRIDE='' FM_PROJECTS_OVERRIDE='' FM_CONFIG_OVERRIDE='' \ + FM_SPAWN_NO_GUARD=1 TMUX="fake,1,0" FM_BACKEND=tmux \ + FM_FAKE_PANE_PATH="$pane" FM_FAKE_CALL_LOG="$case_dir/calls.log" \ + PATH="$case_dir/fakebin:$PATH" \ + "$SPAWN" "$@" 2>&1 +} + +spawn_ship() { # [pane-path] + local case_dir=$1 id=$2 pane=${3:-} + [ -n "$pane" ] || pane=$(new_worktree "$case_dir" "$id") + run_spawn "$case_dir" "$case_dir/home" "$pane" "$id" "$case_dir/project" --mode no-mistakes --yolo off +} + +# Everything a deferred spawn must not have created for ; +# is worktree_list taken before the spawn. +assert_nothing_created() { # + local case_dir=$1 home=$2 id=$3 before=$4 worktrees=$5 after + assert_absent "$home/state/$id.meta" "a deferred spawn published a task record for $id" + assert_absent "$home/data/$id/launch-brief.md" "a deferred spawn rendered a launch brief for $id" + after=$(call_count "$case_dir") + [ "$after" -eq "$before" ] || + fail "a deferred spawn touched the terminal or worktree pool for $id: $(tail -n +"$((before + 1))" "$case_dir/calls.log")" + assert_equals "$worktrees" "$(worktree_list "$case_dir")" "a deferred spawn left a git worktree for $id" + if [ "$HAVE_TASKS_AXI" = 1 ]; then + [ "$(row_state "$home" "$id")" = queued ] || + fail "a deferred spawn moved $id's backlog item: $(row_state "$home" "$id")" + fi +} + +call_count() { wc -l < "$1/calls.log" | tr -d ' '; } + +worktree_list() { git -C "$1/project" worktree list --porcelain; } + +# --- cases ------------------------------------------------------------------ + +# The reported incident's shape before any declaration: the project is already +# busy, and nothing caps a further launch. Absent a declaration that stays true. +test_undeclared_capacity_keeps_dispatch_uncapped() { + local case_dir home out rc=0 + case_dir=$(make_case undeclared task-c) + home="$case_dir/home" + write_live "$home" live-a "$case_dir/project" + write_live "$home" live-b "$case_dir/project" + out=$(spawn_ship "$case_dir" task-c) || rc=$? + expect_code 0 "$rc" "an undeclared project refused a spawn: $out" + assert_contains "$out" "spawned task-c" "an undeclared project did not launch the worker" + assert_present "$home/state/task-c.meta" "an undeclared project's spawn published no record" + pass "a project with no declared capacity keeps today's uncapped dispatch" +} + +test_available_capacity_admits_the_worker() { + local case_dir home out rc=0 + case_dir=$(make_case available task-c) + home="$case_dir/home" + declare_capacity "$home" "# heavy suite serves two workers" "project 2" "other-project 1" + write_live "$home" live-a "$case_dir/project" + out=$(spawn_ship "$case_dir" task-c) || rc=$? + expect_code 0 "$rc" "a spawn with a free place was refused: $out" + assert_contains "$out" "spawned task-c" "a spawn with a free place did not launch" + assert_not_contains "$out" "deferred:" "a spawn with a free place reported a deferral" + if [ "$HAVE_TASKS_AXI" = 1 ]; then + [ "$(row_state "$home" task-c)" = in_flight ] || fail "an admitted spawn did not move its item In flight" + fi + pass "a spawn is admitted while its project still has a free place" +} + +test_exhausted_capacity_defers_without_leaving_anything_behind() { + local case_dir home out rc=0 before worktrees + case_dir=$(make_case exhausted task-c) + home="$case_dir/home" + declare_capacity "$home" "project 2" + write_live "$home" live-a "$case_dir/project" + write_live "$home" live-b "$case_dir/project" + before=$(call_count "$case_dir") + worktrees=$(worktree_list "$case_dir") + out=$(spawn_ship "$case_dir" task-c "$case_dir/unused") || rc=$? + expect_code "$DEFER_EXIT" "$rc" "a spawn beyond capacity was not deferred: $out" + assert_contains "$out" "deferred: project project admits 2 worker(s) at once on this machine ($home/config/project-capacity) and 2 already hold a place (live-a, live-b)" \ + "the deferral did not name the capacity and its holders" + assert_contains "$out" "task task-c was not launched and its backlog item stays queued" \ + "the deferral did not say the task stays queued" + assert_nothing_created "$case_dir" "$home" task-c "$before" "$worktrees" + pass "a spawn beyond capacity is deferred before any record, brief, endpoint, worktree, or backlog move exists" +} + +# The capacity is the last field, so a project whose clone directory name holds +# spaces can be declared. An indented '#' line is still a comment. +test_spaced_project_name_is_declared() { + local case_dir home spaced out rc=0 + case_dir=$(make_case spaced task-c) + home="$case_dir/home" + spaced="$case_dir/my heavy project" + git clone -q "$(git -C "$case_dir/project" remote get-url origin)" "$spaced" + declare_capacity "$home" " # my heavy project 9" "my heavy project 1" "project 5" + write_live "$home" live-a "$spaced" + out=$(run_spawn "$case_dir" "$home" "$case_dir/unused" task-c "$spaced" --mode no-mistakes --yolo off) || rc=$? + expect_code "$DEFER_EXIT" "$rc" "a project whose name holds spaces was not capped by its declaration: $out" + assert_contains "$out" "deferred: project my heavy project admits 1 worker(s) at once" \ + "the deferral did not use the spaced project's declared capacity" + assert_absent "$home/state/task-c.meta" "the deferred spaced-name spawn published a record" + pass "a project name with spaces is declared by taking the capacity from the last field" +} + +# A clone directory may be named with a leading '#'. That name is declared when +# the '#' is written against the rest of the name and the line ends with the +# capacity. A '#' followed by whitespace stays a comment even when the line +# ends with a number, and a '#' note that is not a capacity stays a comment. +test_hash_prefixed_project_name_is_declared() { + local case_dir home hashed spaced out rc=0 wt + case_dir=$(make_case hash-name task-c task-d) + home="$case_dir/home" + hashed="$case_dir/#hash-project" + spaced="$case_dir/# serves" + git clone -q "$(git -C "$case_dir/project" remote get-url origin)" "$hashed" + git clone -q "$(git -C "$case_dir/other-project" remote get-url origin)" "$spaced" + declare_capacity "$home" "# serves 2" "#not-a-capacity" "#hash-project 1" + write_live "$home" live-a "$hashed" + out=$(run_spawn "$case_dir" "$home" "$case_dir/unused" task-c "$hashed" --mode no-mistakes --yolo off) || rc=$? + expect_code "$DEFER_EXIT" "$rc" "a project whose name begins with # was not capped by its declaration: $out" + assert_contains "$out" "deferred: project #hash-project admits 1 worker(s) at once" \ + "the deferral did not use the hash-prefixed project's declared capacity" + assert_absent "$home/state/task-c.meta" "the deferred hash-prefixed spawn published a record" + + write_live "$home" live-b "$spaced" + write_live "$home" live-c "$spaced" + git -C "$spaced" worktree add --quiet -b wt-d "$case_dir/wt-d" + wt="$case_dir/wt-d" + rc=0 + out=$(run_spawn "$case_dir" "$home" "$wt" task-d "$spaced" --mode no-mistakes --yolo off) || rc=$? + expect_code 0 "$rc" "a '#' comment that ends with a number was read as a capacity: $out" + assert_contains "$out" "spawned task-d" "a project whose declaration line is a comment did not stay uncapped" + pass "a project name beginning with # is declared, and a # comment stays a comment" +} + +# A home reached through a symlink is still one home: its workers hold one +# place each, not one per spelling of its state directory. +test_symlinked_home_counts_each_worker_once() { + local case_dir home out rc=0 + case_dir=$(make_case symlinked-home task-c) + home="$case_dir/home" + ln -s "$home" "$case_dir/home-link" + declare_capacity "$home" "project 2" + write_live "$home" live-a "$case_dir/project" + out=$(run_spawn "$case_dir" "$case_dir/home-link" "$(new_worktree "$case_dir" task-c)" \ + task-c "$case_dir/project" --mode no-mistakes --yolo off) || rc=$? + expect_code 0 "$rc" "a spawn through a symlinked home counted its own worker twice: $out" + assert_contains "$out" "spawned task-c" "a spawn through a symlinked home did not launch" + pass "a home reached through a symlink counts each of its workers once" +} + +# A fresh spawn that restarts an existing task id replaces that task's own +# record, so the record does not hold a place against it. Any other task on the +# project still sees that record as a holder. +test_restart_does_not_count_its_own_record() { + local case_dir home out rc=0 + case_dir=$(make_case restart task-c task-d) + home="$case_dir/home" + declare_capacity "$home" "project 1" + fm_write_meta "$home/state/task-c.meta" \ + "window=firstmate:fm-task-c" \ + "project=$case_dir/project" \ + "kind=ship" + out=$(spawn_ship "$case_dir" task-c) || rc=$? + assert_not_contains "$out" "deferred:" "a restart was deferred by its own record" + [ "$rc" -ne "$DEFER_EXIT" ] || fail "a restart exited with the deferral code: $out" + rc=0 + out=$(spawn_ship "$case_dir" task-d "$case_dir/unused") || rc=$? + expect_code "$DEFER_EXIT" "$rc" "another task ignored the restarted task's place: $out" + assert_contains "$out" "1 already hold a place (task-c)" "another task did not count the restarted task" + pass "a restart of a task id does not count that task's own record" +} + +# A place frees when a worker records its ready PR and when a task is cleaned +# up; each release admits exactly one more worker. +test_release_frees_a_place() { + local case_dir home out rc=0 + case_dir=$(make_case release task-c task-d) + home="$case_dir/home" + declare_capacity "$home" "project 2" + write_live "$home" live-a "$case_dir/project" + write_live "$home" live-b "$case_dir/project" + out=$(spawn_ship "$case_dir" task-c "$case_dir/unused") || rc=$? + expect_code "$DEFER_EXIT" "$rc" "the full project admitted a worker: $out" + + # PR handoff: the line bin/fm-pr-check.sh records for a ready PR. + printf 'pr=%s\n' "https://github.com/o/r/pull/7" >> "$home/state/live-a.meta" + rc=0 + out=$(spawn_ship "$case_dir" task-c) || rc=$? + expect_code 0 "$rc" "a recorded PR handoff did not free a place: $out" + rc=0 + out=$(spawn_ship "$case_dir" task-d "$case_dir/unused") || rc=$? + expect_code "$DEFER_EXIT" "$rc" "one freed place admitted two workers: $out" + assert_contains "$out" "2 already hold a place (live-b, task-c)" "the new worker did not take the freed place" + + # Cleanup: the real teardown removes the record, which frees its place. + rc=0 + out=$(FM_ROOT_OVERRIDE='' FM_HOME="$home" FM_STATE_OVERRIDE='' FM_DATA_OVERRIDE='' FM_CONFIG_OVERRIDE='' \ + FM_FAKE_CALL_LOG="$case_dir/calls.log" PATH="$case_dir/fakebin:$PATH" \ + "$TEARDOWN" live-b 2>&1) || rc=$? + expect_code 0 "$rc" "cleanup of a live worker failed: $out" + assert_absent "$home/state/live-b.meta" "cleanup left the worker's record" + rc=0 + out=$(spawn_ship "$case_dir" task-d) || rc=$? + expect_code 0 "$rc" "cleanup did not free a place: $out" + pass "a recorded PR handoff or a cleanup each frees exactly one place" +} + +# Only workers on the same project identity hold places: a secondmate record, a +# record for another project, and a scout on another project are ignored, while +# a scout and a legacy record without kind= on this project count. +test_occupancy_counts_only_this_projects_workers() { + local case_dir home out rc=0 + case_dir=$(make_case occupancy task-c) + home="$case_dir/home" + declare_capacity "$home" "project 3" + write_live "$home" scout-a "$case_dir/project" + sed -i.bak 's/^kind=ship$/kind=scout/' "$home/state/scout-a.meta" && rm -f "$home/state/scout-a.meta.bak" + fm_write_meta "$home/state/legacy-b.meta" "window=firstmate:fm-legacy-b" "project=$case_dir/project" + write_live "$home" other-c "$case_dir/other-project" + fm_write_meta "$home/state/mate-d.meta" "window=remote:mate-d" "project=$case_dir/project" "kind=secondmate" + out=$(spawn_ship "$case_dir" task-c) || rc=$? + expect_code 0 "$rc" "records outside this project's workers took a place: $out" + rc=0 + write_brief "$home" task-e + add_item "$home" task-e + out=$(spawn_ship "$case_dir" task-e "$case_dir/unused") || rc=$? + expect_code "$DEFER_EXIT" "$rc" "the third worker on the project was not counted: $out" + assert_contains "$out" "3 already hold a place (legacy-b, scout-a, task-c)" \ + "occupancy counted the wrong records" + pass "only this project's ship and scout records hold places" +} + +# Capacity belongs to the machine: a local secondmate home's workers on a +# separate clone of the same origin count, the declaration comes from the root +# home even when the secondmate spawns, and a remote home's workers never count. +test_capacity_is_shared_by_every_local_home() { + local case_dir root mate out rc=0 mate_project + case_dir=$(make_case machine) + root="$case_dir/home" + mate="$case_dir/mate" + make_home "$mate" task-m + mate_project="$mate/projects/project" + git clone -q "$(git -C "$case_dir/project" remote get-url origin)" "$mate_project" + printf '%s\n' schema=fm-secondmate-parent.v1 route=local "parent_home=$root" > "$mate/.fm-secondmate-parent" + printf -- '- mate - a local mate (home: %s; scope: project work; projects: project; added 2026-09-01)\n' "$mate" \ + > "$root/data/secondmates.md" + printf -- '- far - a remote mate (host: far.example; root: /srv/fm; home: %s/remote-home; scope: other; projects: project; added 2026-09-01)\n' "$case_dir" \ + >> "$root/data/secondmates.md" + mkdir -p "$case_dir/remote-home/state" + fm_write_meta "$case_dir/remote-home/state/far-x.meta" "window=w" "project=$case_dir/project" "kind=ship" + declare_capacity "$root" "project 1" + write_live "$root" live-a "$case_dir/project" + git -C "$mate_project" worktree add --quiet -b wt-m "$case_dir/wt-m" + out=$(run_spawn "$case_dir" "$mate" "$case_dir/wt-m" task-m "$mate_project" --mode no-mistakes --yolo off) || rc=$? + expect_code "$DEFER_EXIT" "$rc" "a local secondmate launched past the machine's capacity: $out" + assert_contains "$out" "admits 1 worker(s) at once on this machine ($root/config/project-capacity) and 1 already hold a place (live-a in $root)" \ + "the secondmate did not count the root home's worker against the root's declaration" + assert_absent "$mate/state/task-m.meta" "the deferred secondmate spawn published a record" + printf 'pr=%s\n' "https://github.com/o/r/pull/8" >> "$root/state/live-a.meta" + rc=0 + out=$(run_spawn "$case_dir" "$mate" "$case_dir/wt-m" task-m "$mate_project" --mode no-mistakes --yolo off) || rc=$? + expect_code 0 "$rc" "the remote home's worker, or a handed-off one, took the machine's place: $out" + pass "every local home shares one declared capacity per project origin, and remote homes do not count" +} + +# A local home's state directory or task record that cannot be read could hide a +# holder, so admission refuses instead of counting without it. +test_unreadable_holders_refuse_admission() { + local case_dir root mate out rc=0 + if [ "$(id -u)" = 0 ]; then + printf 'ok - skipped the unreadable-holder case (root reads files regardless of mode)\n' + return 0 + fi + case_dir=$(make_case unreadable-holders task-c) + root="$case_dir/home" + mate="$case_dir/mate" + make_home "$mate" + printf '%s\n' schema=fm-secondmate-parent.v1 route=local "parent_home=$root" > "$mate/.fm-secondmate-parent" + printf -- '- mate - a local mate (home: %s; scope: project work; projects: project; added 2026-09-01)\n' "$mate" \ + > "$root/data/secondmates.md" + declare_capacity "$root" "project 2" + + write_live "$root" live-a "$case_dir/project" + chmod 000 "$root/state/live-a.meta" + out=$(spawn_ship "$case_dir" task-c "$case_dir/unused") || rc=$? + chmod 600 "$root/state/live-a.meta" + expect_code 1 "$rc" "an unreadable task record did not refuse admission: $out" + assert_contains "$out" "task record $root/state/live-a.meta cannot be read" \ + "the refusal did not name the unreadable task record" + assert_absent "$root/state/task-c.meta" "a spawn published past an unreadable task record" + + chmod 000 "$mate/state" + rc=0 + out=$(spawn_ship "$case_dir" task-c "$case_dir/unused") || rc=$? + chmod 755 "$mate/state" + expect_code 1 "$rc" "an unreadable local state directory did not refuse admission: $out" + assert_contains "$out" "local Firstmate state directory $mate/state cannot be read" \ + "the refusal did not name the unreadable state directory" + assert_absent "$root/state/task-c.meta" "a spawn published past an unreadable state directory" + + rc=0 + out=$(spawn_ship "$case_dir" task-c) || rc=$? + expect_code 0 "$rc" "a readable machine did not admit the worker: $out" + pass "an unreadable state directory or task record refuses admission rather than undercounting" +} + +# Two spawns racing for the last place: the one holding the project lock +# publishes, the other cannot publish while it waits and is deferred afterwards. +test_concurrent_spawns_cannot_both_take_the_last_place() { + local case_dir home out rc=0 hold i wt + case_dir=$(make_case concurrent task-a task-b) + home="$case_dir/home" + declare_capacity "$home" "project 1" + hold="$case_dir/hold" + wt=$(new_worktree "$case_dir" task-a) + FM_FAKE_HOLD="$hold" spawn_ship "$case_dir" task-a "$wt" > "$case_dir/a.out" 2>&1 & + i=0 + while [ ! -f "$hold.reached" ]; do + i=$((i + 1)) + [ "$i" -lt 400 ] || fail "the first spawn never reached its launch: $(cat "$case_dir/a.out")" + sleep 0.05 + done + out=$(spawn_ship "$case_dir" task-b "$case_dir/unused") || rc=$? + [ "$rc" -ne 0 ] || fail "a concurrent spawn launched while the last place was being taken: $out" + assert_absent "$home/state/task-b.meta" "a concurrent spawn published past the last place" + : > "$hold.release" + wait || true + assert_contains "$(cat "$case_dir/a.out")" "spawned task-a" "the lock-holding spawn did not finish" + rc=0 + out=$(spawn_ship "$case_dir" task-b "$case_dir/unused") || rc=$? + expect_code "$DEFER_EXIT" "$rc" "the retried spawn was not deferred once the place was taken: $out" + assert_absent "$home/state/task-b.meta" "the retried spawn published past capacity" + pass "concurrent spawns can never both take the last place" +} + +# A spawn that fails after admission removes nothing it did not create and +# leaves no record, so it holds no place afterwards. +test_failed_spawn_after_admission_holds_no_place() { + local case_dir home out rc=0 + case_dir=$(make_case failed task-a task-b) + home="$case_dir/home" + declare_capacity "$home" "project 1" + printf '%s\n' '# Task' "## Captain's intent" '{TASK}' '' '## Firstmate spec' 'x' > "$home/data/task-a/brief.md" + out=$(spawn_ship "$case_dir" task-a "$case_dir/unused") || rc=$? + [ "$rc" -ne 0 ] && [ "$rc" -ne "$DEFER_EXIT" ] || fail "an invalid brief did not fail after admission (exit $rc): $out" + assert_contains "$out" "still contains {TASK}" "the spawn did not fail where expected" + assert_absent "$home/state/task-a.meta" "the failed spawn left a record" + rc=0 + out=$(spawn_ship "$case_dir" task-b) || rc=$? + expect_code 0 "$rc" "a failed spawn kept holding the only place: $out" + pass "a spawn that fails after admission leaves no record and holds no place" +} + +test_unreadable_declaration_refuses_every_spawn() { + local case_dir home out rc label body before worktrees + case_dir=$(make_case unreadable task-c) + home="$case_dir/home" + while IFS='|' read -r label body; do + [ -n "$label" ] || continue + printf '%b' "$body" > "$home/config/project-capacity" + before=$(call_count "$case_dir") + worktrees=$(worktree_list "$case_dir") + rc=0 + out=$(spawn_ship "$case_dir" task-c "$case_dir/unused") || rc=$? + expect_code 1 "$rc" "$label: an unreadable declaration did not refuse: $out" + assert_contains "$out" "the project capacity declaration is unreadable" "$label: the refusal did not name the declaration" + assert_nothing_created "$case_dir" "$home" task-c "$before" "$worktrees" + done <<'ROWS' +missing capacity|project\n +zero capacity|project 0\n +non-numeric capacity|other-project two\n +trailing text|project 2 # suite\n +named twice|project 2\nproject 3\n +ROWS + rm -f "$home/config/project-capacity" + mkdir "$home/config/project-capacity" + rc=0 + out=$(spawn_ship "$case_dir" task-c "$case_dir/unused") || rc=$? + expect_code 1 "$rc" "a declaration that is not a file did not refuse: $out" + pass "an unreadable declaration refuses every fresh spawn rather than guessing the limit" +} + +test_batch_reports_a_deferred_pair() { + local case_dir home out rc=0 + case_dir=$(make_case batch task-c) + home="$case_dir/home" + declare_capacity "$home" "project 1" + write_live "$home" live-a "$case_dir/project" + out=$(run_spawn "$case_dir" "$home" "$case_dir/unused" "task-c=$case_dir/project" --mode no-mistakes --yolo off) || rc=$? + expect_code "$DEFER_EXIT" "$rc" "a batch whose only pair was deferred did not exit with the deferral status: $out" + assert_contains "$out" "batch: DEFERRED task-c ($case_dir/project) - its project is at capacity, so it stays queued" \ + "the batch did not report the deferral" + assert_not_contains "$out" "batch: FAILED" "the batch reported a deferral as a failure" + pass "a batch reports a capacity deferral as deferred, not failed" +} + +# Orca owns its own worktrees and never takes the Treehouse allocation lock, so +# a declared capacity is what makes an Orca spawn take the shared project lock, +# even from an uncapped clone of a capped origin whose worker would still hold a +# place: it refuses while another holder has it, and defers at capacity before +# asking Orca for anything but its runtime status. +test_orca_spawn_is_admitted_under_the_shared_project_lock() { + local case_dir home out out2 rc=0 rc2 holder i + command -v node >/dev/null 2>&1 || { + printf 'ok - skipped the Orca capacity case (node, which the Orca status check needs, is not installed)\n' + return 0 + } + case_dir=$(make_case orca task-o) + home="$case_dir/home" + cat > "$case_dir/fakebin/orca" <<'SH' +#!/usr/bin/env bash +if [ "${1:-}" = status ]; then + printf '{"ok":true,"result":{"runtime":{"reachable":true,"state":"ready"}}}\n' + exit 0 +fi +printf 'orca %s\n' "$*" >> "$FM_FAKE_CALL_LOG" +exit 1 +SH + chmod +x "$case_dir/fakebin/orca" + git clone -q "$(git -C "$case_dir/project" remote get-url origin)" "$case_dir/project-2" + declare_capacity "$home" "project 1" + + # shellcheck disable=SC2016 # expanded by the holder's own shell + FM_HOME="$home" FM_STATE_OVERRIDE='' bash -c ' + . "$1/bin/fm-wake-lib.sh" + lock=$(fm_treehouse_project_lock_path "$2") || exit 1 + fm_lock_try_acquire "$lock" || exit 1 + : > "$3.held" + while [ ! -f "$3.release" ]; do sleep 0.05; done + fm_lock_release "$lock" + ' _ "$ROOT" "$case_dir/project" "$case_dir/holder" & + holder=$! + i=0 + while [ ! -f "$case_dir/holder.held" ]; do + i=$((i + 1)) + [ "$i" -lt 200 ] || fail "the lock holder never took the project lock" + sleep 0.05 + done + out=$(run_spawn "$case_dir" "$home" "$case_dir/unused" task-o "$case_dir/project" \ + --backend orca --mode no-mistakes --yolo off) || rc=$? + rc2=0 + out2=$(run_spawn "$case_dir" "$home" "$case_dir/unused" task-o "$case_dir/project-2" \ + --backend orca --mode no-mistakes --yolo off) || rc2=$? + : > "$case_dir/holder.release" + wait "$holder" || true + expect_code 1 "$rc" "an Orca spawn ignored a held project lock: $out" + assert_contains "$out" "another spawn or cleanup holds the shared project lock for $case_dir/project; refusing to race its capacity admission" \ + "the Orca spawn did not refuse on the shared project lock" + expect_code 1 "$rc2" "an Orca spawn from an uncapped same-origin clone ignored the held project lock: $out2" + assert_contains "$out2" "another spawn or cleanup holds the shared project lock for $case_dir/project-2" \ + "the uncapped same-origin clone's Orca spawn did not refuse on the shared project lock" + assert_absent "$home/state/task-o.meta" "the uncapped clone's Orca spawn published a record while the lock was held" + assert_no_grep "orca " "$case_dir/calls.log" "the Orca spawn asked Orca for more than its runtime status" + + write_live "$home" live-a "$case_dir/project" + rc=0 + out=$(run_spawn "$case_dir" "$home" "$case_dir/unused" task-o "$case_dir/project" \ + --backend orca --mode no-mistakes --yolo off) || rc=$? + expect_code "$DEFER_EXIT" "$rc" "an Orca spawn beyond capacity was not deferred: $out" + assert_absent "$home/state/task-o.meta" "the deferred Orca spawn published a record" + assert_no_grep "orca " "$case_dir/calls.log" "the deferred Orca spawn created an Orca worktree" + pass "an Orca spawn takes the shared project lock whenever a same-origin clone is capped and defers before creating anything" +} + +test_undeclared_capacity_keeps_dispatch_uncapped +test_available_capacity_admits_the_worker +test_exhausted_capacity_defers_without_leaving_anything_behind +test_spaced_project_name_is_declared +test_hash_prefixed_project_name_is_declared +test_symlinked_home_counts_each_worker_once +test_release_frees_a_place +test_restart_does_not_count_its_own_record +test_occupancy_counts_only_this_projects_workers +test_capacity_is_shared_by_every_local_home +test_unreadable_holders_refuse_admission +test_concurrent_spawns_cannot_both_take_the_last_place +test_failed_spawn_after_admission_holds_no_place +test_unreadable_declaration_refuses_every_spawn +test_batch_reports_a_deferred_pair +test_orca_spawn_is_admitted_under_the_shared_project_lock From d8f8a112cf6f9bf19bab3cb74dc6bf3775f68ac4 Mon Sep 17 00:00:00 2001 From: Rajesh Rajendiran Date: Wed, 7 Oct 2026 18:18:18 -0500 Subject: [PATCH 5/7] no-mistakes(review): Fail closed capacity reads and restore watcher cleanup --- bin/fm-project-capacity-lib.sh | 55 ++++++++++++------ bin/fm-wake-lib.sh | 8 ++- bin/fm-watch.sh | 95 +++++++++++++++---------------- tests/fm-project-capacity.test.sh | 56 +++++++++++++++++- tests/fm-watcher-lock.test.sh | 48 ++++++++++++++++ 5 files changed, 191 insertions(+), 71 deletions(-) diff --git a/bin/fm-project-capacity-lib.sh b/bin/fm-project-capacity-lib.sh index a184c3d28a9..98cd5e14fd3 100644 --- a/bin/fm-project-capacity-lib.sh +++ b/bin/fm-project-capacity-lib.sh @@ -67,9 +67,9 @@ # place needs no lock, because removing a record or adding pr= only ever lowers # the count. # -# Requires bin/fm-wake-lib.sh (root home, local homes, project lock path), -# bin/fm-secondmate-registry-lib.sh (which the local-homes walk reads), and -# bin/fm-backend.sh (fm_meta_get) to be sourced first. No side effects on source. +# Requires bin/fm-wake-lib.sh (root home, local homes, project lock path) and +# bin/fm-secondmate-registry-lib.sh (which the local-homes walk reads) to be +# sourced first. No side effects on source. # Exit status of a spawn deferred because the project is at capacity: the # sysexits "temporary failure" code, so a caller can tell a deferral that leaves @@ -97,7 +97,8 @@ fm_project_capacity_config_dir() { # # FM_PROJECT_CAPACITY_ANY to 1 when the declaration caps any project at all. # Returns 1 with FM_PROJECT_CAPACITY_ERROR when the declaration is unreadable. fm_project_capacity_lookup() { # - local name=$2 line lineno=0 pname pcap seen='|' + local name=$2 line lineno=0 pname pcap contents previous duplicate + local -a seen_names=() FM_PROJECT_CAPACITY_FILE="$1/project-capacity" FM_PROJECT_CAPACITY= FM_PROJECT_CAPACITY_ANY= @@ -109,6 +110,10 @@ fm_project_capacity_lookup() { # FM_PROJECT_CAPACITY_ERROR="$FM_PROJECT_CAPACITY_FILE is not a readable regular file" return 1 fi + if ! contents=$(cat "$FM_PROJECT_CAPACITY_FILE"); then + FM_PROJECT_CAPACITY_ERROR="$FM_PROJECT_CAPACITY_FILE could not be read" + return 1 + fi while IFS= read -r line || [ -n "$line" ]; do lineno=$((lineno + 1)) line=${line%$'\r'} @@ -147,17 +152,19 @@ fm_project_capacity_lookup() { # FM_PROJECT_CAPACITY= return 1 fi - case "$seen" in - *"|$pname|"*) - FM_PROJECT_CAPACITY_ERROR="$FM_PROJECT_CAPACITY_FILE line $lineno names $pname a second time" - FM_PROJECT_CAPACITY= - return 1 - ;; - esac - seen="$seen$pname|" + duplicate=0 + for previous in "${seen_names[@]}"; do + [ "$previous" != "$pname" ] || duplicate=1 + done + if [ "$duplicate" -eq 1 ]; then + FM_PROJECT_CAPACITY_ERROR="$FM_PROJECT_CAPACITY_FILE line $lineno names $pname a second time" + FM_PROJECT_CAPACITY= + return 1 + fi + seen_names+=("$pname") [ "$pname" != "$name" ] || FM_PROJECT_CAPACITY=$pcap - done < "$FM_PROJECT_CAPACITY_FILE" - [ "$seen" = '|' ] || FM_PROJECT_CAPACITY_ANY=1 + done <<< "$contents" + [ "${#seen_names[@]}" -eq 0 ] || FM_PROJECT_CAPACITY_ANY=1 return 0 } @@ -175,7 +182,7 @@ fm_project_capacity_lookup() { # # a state directory or task record in them cannot be read, since skipping it # could undercount the holders. fm_project_capacity_occupants() { # - local want=$1 own=$2 first=$3 self=$4 state meta kind project lock id label i + local want=$1 own=$2 first=$3 self=$4 state meta kind pr project lock id label i line contents local -a cache_dirs cache_locks FM_PROJECT_CAPACITY_OCCUPANTS=0 FM_PROJECT_CAPACITY_OCCUPANT_IDS= @@ -198,10 +205,22 @@ fm_project_capacity_occupants() { # FM_PROJECT_CAPACITY_ERROR="task record $meta cannot be read" return 1 } - kind=$(fm_meta_get "$meta" kind) + if ! contents=$(cat "$meta"); then + FM_PROJECT_CAPACITY_ERROR="task record $meta cannot be read" + return 1 + fi + kind= + pr= + project= + while IFS= read -r line || [ -n "$line" ]; do + case "$line" in + kind=*) kind=${line#*=} ;; + pr=*) pr=${line#*=} ;; + project=*) project=${line#*=} ;; + esac + done <<< "$contents" [ "$kind" != secondmate ] || continue - [ -z "$(fm_meta_get "$meta" pr)" ] || continue - project=$(fm_meta_get "$meta" project) + [ -z "$pr" ] || continue [ -n "$project" ] || continue lock= i=0 diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index 625f059e1ab..39587e33328 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -1478,7 +1478,7 @@ fm_firstmate_root_home() { # with no tasks. Requires bin/fm-secondmate-registry-lib.sh to be sourced first. # shellcheck disable=SC2034 # FM_LOCAL_FIRSTMATE_ERROR is read by callers. fm_local_firstmate_state_dirs() { # - local first=$1 root home reg line child known existing i=0 + local first=$1 root home reg registry line child known existing i=0 local -a homes FM_LOCAL_FIRSTMATE_STATES=("$first") FM_LOCAL_FIRSTMATE_ERROR= @@ -1503,6 +1503,10 @@ fm_local_firstmate_state_dirs() { # FM_LOCAL_FIRSTMATE_ERROR="local Firstmate registry is unsafe at $reg" return 1 } + if ! registry=$(cat "$reg"); then + FM_LOCAL_FIRSTMATE_ERROR="local Firstmate registry cannot be read at $reg" + return 1 + fi while IFS= read -r line || [ -n "$line" ]; do case "$line" in "- "*) @@ -1523,7 +1527,7 @@ fm_local_firstmate_state_dirs() { # [ "$known" = 1 ] || homes+=("$child") ;; esac - done < "$reg" + done <<< "$registry" done } diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index 16b7845dfb2..ad133417795 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -2455,6 +2455,44 @@ evict_stalled_holder() { ! fm_pid_alive "$pid" } +PR_POLL_CONTROL_LOCK= +PR_POLL_PUBLISH_LOCK= + +pr_poll_control_release() { + [ -z "$PR_POLL_CONTROL_LOCK" ] || fm_lock_release "$PR_POLL_CONTROL_LOCK" || return 1 + PR_POLL_CONTROL_LOCK= +} + +pr_poll_publish_release() { + [ -z "$PR_POLL_PUBLISH_LOCK" ] || fm_lock_release "$PR_POLL_PUBLISH_LOCK" || return 1 + PR_POLL_PUBLISH_LOCK= +} + +watcher_cleanup() { + local cleanup_status=0 owns_lock=0 transition=release-lock + pr_poll_publish_release || cleanup_status=1 + pr_poll_control_release || cleanup_status=1 + if [ "$(cat "$WATCH_LOCK/pid" 2>/dev/null || true)" = "${WATCHER_PID:-}" ]; then + owns_lock=1 + if [ "${WATCHER_RECOVERY_PENDING:-0}" -eq 1 ] \ + && [ "${FM_WATCH_DELIVERED_REASON:-}" = "check: rearm-resurface" ]; then + transition=release-lock-existing + fi + fi + fm_active_check_stop || cleanup_status=1 + fm_check_output_cleanup + fm_capture_output_cleanup + fm_custom_check_snapshot_cleanup + if [ "$owns_lock" -eq 1 ] \ + && ! fm_recovery_transition "$WATCHER_DOWNTIME_MARKER" "$transition" "$WATCH_LOCK" \ + downtime "$CLEANUP_LOCK_BOUND"; then + echo "watcher: recovery state could not be persisted; retaining stale lock evidence" >&2 + cleanup_status=1 + fi + return "$cleanup_status" +} + +WATCHER_PID=${BASHPID:-$$} EVICTED_PID= EVICTED_BEAT_AGE= BEAT="$STATE/.last-watcher-beat" @@ -2485,6 +2523,14 @@ while ! fm_lock_try_acquire "$WATCH_LOCK"; do fi exit 0 done +trap watcher_cleanup EXIT +watcher_stop_signals +printf '%s\n' "$FM_HOME" > "$WATCH_LOCK/fm-home" || true +printf '%s\n' "$WATCH_PATH" > "$WATCH_LOCK/watcher-path" || true +# shellcheck disable=SC2034 # Consumed by wake() in the separately linted transition owner. +FM_WATCH_DELIVERY_PID=$WATCHER_PID +FM_WATCH_DELIVERY_IDENTITY=$(fm_pid_identity "$WATCHER_PID" 2>/dev/null || true) +printf '%s\n' "$FM_WATCH_DELIVERY_IDENTITY" > "$WATCH_LOCK/pid-identity" 2>/dev/null || true if [ -n "$EVICTED_PID" ]; then echo "watcher: replaced stalled pid $EVICTED_PID (beacon ${EVICTED_BEAT_AGE}s past hard bound ${WATCHER_STALL_BOUND}s)" fi @@ -2564,55 +2610,6 @@ reconcile_requests_detached() { RECONCILE_REQUEST_PID=$! } -PR_POLL_CONTROL_LOCK= -PR_POLL_PUBLISH_LOCK= - -pr_poll_control_release() { - [ -z "$PR_POLL_CONTROL_LOCK" ] || fm_lock_release "$PR_POLL_CONTROL_LOCK" || return 1 - PR_POLL_CONTROL_LOCK= -} - -pr_poll_publish_release() { - [ -z "$PR_POLL_PUBLISH_LOCK" ] || fm_lock_release "$PR_POLL_PUBLISH_LOCK" || return 1 - PR_POLL_PUBLISH_LOCK= -} - -watcher_cleanup() { - local cleanup_status=0 owns_lock=0 transition=release-lock - pr_poll_publish_release || cleanup_status=1 - pr_poll_control_release || cleanup_status=1 - if [ "$(cat "$WATCH_LOCK/pid" 2>/dev/null || true)" = "${WATCHER_PID:-}" ]; then - owns_lock=1 - if [ "${WATCHER_RECOVERY_PENDING:-0}" -eq 1 ] \ - && [ "${FM_WATCH_DELIVERED_REASON:-}" = "check: rearm-resurface" ]; then - transition=release-lock-existing - fi - fi - fm_active_check_stop || cleanup_status=1 - fm_check_output_cleanup - fm_capture_output_cleanup - fm_custom_check_snapshot_cleanup - if [ "$owns_lock" -eq 1 ] \ - && ! fm_recovery_transition "$WATCHER_DOWNTIME_MARKER" "$transition" "$WATCH_LOCK" \ - downtime "$CLEANUP_LOCK_BOUND"; then - echo "watcher: recovery state could not be persisted; retaining stale lock evidence" >&2 - cleanup_status=1 - fi - return "$cleanup_status" -} -trap watcher_cleanup EXIT -watcher_stop_signals -# This watcher's own pid, as recorded in the lock by fm_lock_claim (which writes -# ${BASHPID:-$$} from this same main shell). Read directly, never via a command -# substitution, so it matches the stored holder pid for the self-eviction check. -WATCHER_PID=${BASHPID:-$$} -printf '%s\n' "$FM_HOME" > "$WATCH_LOCK/fm-home" || true -printf '%s\n' "$WATCH_PATH" > "$WATCH_LOCK/watcher-path" || true -# shellcheck disable=SC2034 # Consumed by wake() in the separately linted transition owner. -FM_WATCH_DELIVERY_PID=$WATCHER_PID -FM_WATCH_DELIVERY_IDENTITY=$(fm_pid_identity "$WATCHER_PID" 2>/dev/null || true) -printf '%s\n' "$FM_WATCH_DELIVERY_IDENTITY" > "$WATCH_LOCK/pid-identity" 2>/dev/null || true - [ -e "$STATE/.last-heartbeat" ] || touch "$STATE/.last-heartbeat" # A merged poll may have queued its terminal wake and then lost the process diff --git a/tests/fm-project-capacity.test.sh b/tests/fm-project-capacity.test.sh index bc64b8283b1..349758271e6 100755 --- a/tests/fm-project-capacity.test.sh +++ b/tests/fm-project-capacity.test.sh @@ -23,6 +23,8 @@ TMP_ROOT=$(fm_test_tmproot fm-project-capacity) DEFER_EXIT=75 HAVE_TASKS_AXI=0 command -v tasks-axi >/dev/null 2>&1 && HAVE_TASKS_AXI=1 +export FM_REAL_CAT +FM_REAL_CAT=$(command -v cat) # --- fixture ---------------------------------------------------------------- @@ -102,7 +104,14 @@ SH printf 'treehouse %s\n' "$*" >> "$FM_FAKE_CALL_LOG" exit 0 SH - chmod +x "$fakebin/tmux" "$fakebin/treehouse" + cat > "$fakebin/cat" <<'SH' +#!/usr/bin/env bash +for arg in "$@"; do + [ -z "${FM_FAIL_READ_PATH:-}" ] || [ "$arg" != "$FM_FAIL_READ_PATH" ] || exit 1 +done +exec "$FM_REAL_CAT" "$@" +SH + chmod +x "$fakebin/tmux" "$fakebin/treehouse" "$fakebin/cat" fm_fake_exit0 "$fakebin" gh gh-axi no-mistakes fm_git_init_commit "$case_dir/project" fm_git_add_origin "$case_dir/project" "$case_dir/project.origin.git" @@ -163,7 +172,7 @@ run_spawn() { # FM_STATE_OVERRIDE='' FM_DATA_OVERRIDE='' FM_PROJECTS_OVERRIDE='' FM_CONFIG_OVERRIDE='' \ FM_SPAWN_NO_GUARD=1 TMUX="fake,1,0" FM_BACKEND=tmux \ FM_FAKE_PANE_PATH="$pane" FM_FAKE_CALL_LOG="$case_dir/calls.log" \ - PATH="$case_dir/fakebin:$PATH" \ + FM_FAIL_READ_PATH="${FM_FAIL_READ_PATH:-}" PATH="$case_dir/fakebin:$PATH" \ "$SPAWN" "$@" 2>&1 } @@ -263,6 +272,21 @@ test_spaced_project_name_is_declared() { pass "a project name with spaces is declared by taking the capacity from the last field" } +test_literal_project_names_do_not_collide() { + local case_dir home named wt out rc=0 + case_dir=$(make_case literal-name task-c) + home="$case_dir/home" + named="$case_dir/b" + git clone -q "$(git -C "$case_dir/project" remote get-url origin)" "$named" + git -C "$named" worktree add --quiet -b wt-literal "$case_dir/wt-literal" + wt="$case_dir/wt-literal" + declare_capacity "$home" "a|b 1" "b 1" + out=$(run_spawn "$case_dir" "$home" "$wt" task-c "$named" --mode no-mistakes --yolo off) || rc=$? + expect_code 0 "$rc" "literal project names were treated as duplicate patterns: $out" + assert_contains "$out" "spawned task-c" "the literally named project did not launch" + pass "project declaration names are compared literally" +} + # A clone directory may be named with a leading '#'. That name is declared when # the '#' is written against the rest of the name and the line ends with the # capacity. A '#' followed by whitespace stays a comment even when the line @@ -513,6 +537,32 @@ test_failed_spawn_after_admission_holds_no_place() { pass "a spawn that fails after admission leaves no record and holds no place" } +test_read_failures_refuse_admission() { + local case_dir root mate out rc=0 path expected + case_dir=$(make_case read-failures task-c) + root="$case_dir/home" + mate="$case_dir/mate" + make_home "$mate" + printf '%s\n' schema=fm-secondmate-parent.v1 route=local "parent_home=$root" > "$mate/.fm-secondmate-parent" + printf -- '- mate - a local mate (home: %s; scope: project work; projects: project; added 2026-09-01)\n' "$mate" \ + > "$root/data/secondmates.md" + declare_capacity "$root" "project 2" + write_live "$root" live-a "$case_dir/project" + + while IFS='|' read -r path expected; do + rc=0 + out=$(FM_FAIL_READ_PATH="$path" spawn_ship "$case_dir" task-c "$case_dir/unused") || rc=$? + expect_code 1 "$rc" "a failed read admitted a worker: $out" + assert_contains "$out" "$expected" "the refusal did not identify the failed read" + assert_absent "$root/state/task-c.meta" "a failed read still published a task record" + done < "$2" + while [ ! -e "$3" ]; do sleep 0.02; done + fm_lock_release "$FM_WAKE_QUEUE_LOCK" + ' _ "$LIB" "$held" "$release" & + holder=$! + i=0 + while [ "$i" -lt 100 ] && [ ! -e "$held" ]; do + sleep 0.02 + i=$((i + 1)) + done + [ -e "$held" ] || fail "wake lock holder did not start" + + PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_POLL=5 FM_SIGNAL_GRACE=1 \ + FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH" > "$out" 2>&1 & + pid=$! + i=0 + while [ "$i" -lt 100 ]; do + [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" = "$pid" ] && break + sleep 0.02 + i=$((i + 1)) + done + [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" = "$pid" ] \ + || fail "watcher did not acquire its lock before the recovery wait" + kill -TERM "$pid" || fail "could not stop watcher during recovery wait" + sleep 0.1 + : > "$release" + wait "$holder" || fail "wake lock holder did not release" + wait_for_exit "$pid" 100 || status=$? + [ "$status" -ne 124 ] || fail "watcher did not stop during the recovery wait" + [ ! -e "$state/.watch.lock" ] && [ ! -L "$state/.watch.lock" ] \ + || fail "TERM after watcher lock acquisition left the singleton lock behind" + [ -f "$state/.watcher-down" ] \ + || fail "TERM after watcher lock acquisition did not publish downtime" + pass "TERM after singleton acquisition runs watcher cleanup" +} + test_stale_watch_lock_reclaimed() { local dir state fakebin out dead_pid pid live lock_pid i dir=$(make_case stale-lock) @@ -1552,6 +1599,7 @@ test_pid_identity_is_locale_invariant test_pid_identity_is_terminal_width_invariant test_proc_pid_identity_ignores_wall_clock_and_detects_pid_reuse test_msys_pid_identity_uses_proc +test_term_after_lock_acquisition_runs_cleanup test_stale_watch_lock_reclaimed test_stale_watch_reclaim_publishes_before_clear test_live_stale_watch_lock_is_actionable From 7e0e47dd6601a1a8f23db846dfc0b694e3fb599f Mon Sep 17 00:00:00 2001 From: Rajesh Rajendiran Date: Wed, 7 Oct 2026 18:26:06 -0500 Subject: [PATCH 6/7] no-mistakes(review): Remove duplicate contribution clock freeze --- tests/fm-contributions.test.sh | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/fm-contributions.test.sh b/tests/fm-contributions.test.sh index 3c71ee9132e..47ccb09854b 100755 --- a/tests/fm-contributions.test.sh +++ b/tests/fm-contributions.test.sh @@ -1024,9 +1024,6 @@ test_arm_plumbs_a_configured_budget_into_the_check_shim() { # before the first forge call, so nothing is ever observed. /bin/date +%s > "$home/forge/clock" printf 'hang\n' > "$home/forge/fault" - # Freeze the clock. An unfrozen one-second budget can tick past before the - # first forge call, so the generated check never writes forge/calls. - /bin/date +%s > "$home/forge/clock" if [ "$mode" = configured ]; then with_home "$home" env FM_CONTRIBUTIONS_BUDGET=1 "$ROOT/bin/fm-contributions.sh" arm >/dev/null \ || fail 'arm with a configured budget failed' From 9bfb4a79275ca4dad8399590baffcd3975c898fb Mon Sep 17 00:00:00 2001 From: Rajesh Rajendiran Date: Wed, 7 Oct 2026 19:16:07 -0500 Subject: [PATCH 7/7] no-mistakes(document): Align documentation with merged runtime features --- .agents/skills/operational-home-layout/SKILL.md | 1 + docs/configuration.md | 4 ++-- docs/herdr-backend.md | 5 +---- docs/verification/runtime-backends.md | 6 ++---- tests/fm-backend-herdr-presentation-e2e.test.sh | 6 +++--- 5 files changed, 9 insertions(+), 13 deletions(-) diff --git a/.agents/skills/operational-home-layout/SKILL.md b/.agents/skills/operational-home-layout/SKILL.md index a468ae37c3b..6a3caf774ea 100644 --- a/.agents/skills/operational-home-layout/SKILL.md +++ b/.agents/skills/operational-home-layout/SKILL.md @@ -23,6 +23,7 @@ bin/ helper scripts, committed; read each script's header before config/crew-harness crewmate harness override; LOCAL, gitignored; absent or "default" = same as firstmate. Inherited as the literal file: a concrete primary adapter value also controls a secondmate home's own crewmates (section 4) config/claude-permission-mode optional one-token permission posture for every Claude worker launch: absent or "bypass" keeps --dangerously-skip-permissions, "auto" launches with --permission-mode auto; LOCAL, gitignored; inherited by secondmate homes; see docs/configuration.md "Claude permission mode" config/claude-account config/pi-account optional per-home worker account pin for Claude and Pi launches; LOCAL, gitignored, not inherited; absent keeps today's ambient account; present refuses a launch unless the pinned account resolves and is signed in (section 4 owns the refusal rule); see docs/configuration.md "Worker account pin" +config/crew-exclude-tools optional per-home tool exclusion list for ship and scout workers; LOCAL, gitignored, not inherited; see docs/configuration.md "Worker tool exclusions" config/crew-dispatch.json optional crewmate dispatch profiles; LOCAL, gitignored; firstmate-maintained but human-editable natural-language rules that choose a per-task harness/model/effort profile (section 4). Inherited by secondmate homes config/project-capacity optional per-machine count of workers each named project admits at once, read from the root home by every local home; LOCAL, gitignored; see docs/configuration.md "Project capacity" config/secondmate-harness harness the PRIMARY uses to launch SECONDMATE agents, optionally followed by a model and effort token on the same line (" [] []"; section 4); LOCAL, gitignored; absent or "default" harness falls back to config/crew-harness then firstmate's own. The primary's own setting; NOT inherited into secondmate homes (secondmates do not spawn secondmates) diff --git a/docs/configuration.md b/docs/configuration.md index ba30f5fdbde..c4706ffac1c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -8,7 +8,7 @@ Start with the directory layout, then use the setting reference for the behavior | What you want to configure | Start here | | --- | --- | | Firstmate's code, private files, or project location | [FM_HOME](#fm_home) and [operational home layout](#operational-home-layout-and-state) | -| Task windows and worker tools | [Runtime backend](#runtime-backend-configbackend--fm_backend) and [harness support](#harness-support) | +| Task windows and worker tools | [Runtime backend](#runtime-backend-configbackend--fm_backend), [harness support](#harness-support), and [project capacity](#project-capacity-configproject-capacity) | | Worker permissions, accounts, or environment | [Claude permission mode](#claude-permission-mode-configclaude-permission-mode), [worker account pin](#worker-account-pin-configclaude-account-configpi-account), [worker tool exclusions](#worker-tool-exclusions-configcrew-exclude-tools), and [worker launch environment](#worker-launch-environment-configlaunch-env-allowlist) | | Backlog, preferences, and memory | [Backlog backend](#backlog-backend-taskstoml--configbacklog-backend), [captain preferences](#captain-preferences-datacaptainmd--datacaptain-sharedmd), and [startup memory budget](#startup-memory-budget-configstartup-memory-budget) | | Supervision and presentation | [Pi supervision branch](#pi-supervision-branch), [supervision host](#supervision-host-configsupervision-host), and [Calm preference](#calm-preference-configcalm) | @@ -1072,7 +1072,7 @@ The file lives in the machine's root Firstmate home, so every local secondmate h my-project 2 ``` -The name is the project's registered name, which is its clone directory name and may contain spaces, and the number, the last field on the line, is a positive integer. +The name is the project's registered name, which is its clone directory name and may contain spaces, and the number, the last field on the line, is a positive integer of at most six digits. A line that is only `#`, or that begins with `#` followed by whitespace, is a comment, as is a `#` line whose last field is not an integer. A project name may begin with `#` when that `#` is written immediately against the rest of the name and the line ends with the project's capacity. A name that is `#`, or that begins with `#` and a space, cannot be declared, because that line is a comment. diff --git a/docs/herdr-backend.md b/docs/herdr-backend.md index ac111a097a6..52561726e48 100644 --- a/docs/herdr-backend.md +++ b/docs/herdr-backend.md @@ -458,10 +458,7 @@ Any of these preserves the candidate and lets session startup continue with at m So a Herdr create failure surfaces as a spawn failure in every Herdr home, rather than only in homes that opted in. Every earlier degradation on the fresh projected-create path (no session server, contended presentation lock, absent or ambiguous parent) still warns and continues flat. - Recovery of an existing presentation journal refuses by default when the shared presentation lock is contended, rather than falling back flat. - Pass `fm-spawn.sh --herdr-resume-lock-wait` to opt that recovery into waiting for the lock instead, so concurrent recoveries can serialize. - The flag applies to a fresh ship or scout spawn that recovers a journal. - The multi-task path forwards the flag to each per-pair spawn. - `fm-spawn.sh --relaunch` and `--secondmate` take no exact-resume presentation-order lock, so the flag has no effect there. + Callers can opt a recovery into waiting so concurrent recoveries serialize; [`fm-spawn.sh --help`](../bin/fm-spawn.sh) owns the exact flag, applicable spawn paths, and batch forwarding. Dead-owner reclaim still stops the wait when a holder crashed. Unbounded blocking on the session lock is never the default. - Existing layouts are not force-renamed or rearranged. diff --git a/docs/verification/runtime-backends.md b/docs/verification/runtime-backends.md index 7fe1ba7df17..7a852128d78 100644 --- a/docs/verification/runtime-backends.md +++ b/docs/verification/runtime-backends.md @@ -1424,10 +1424,8 @@ ok - real Herdr lab validation completed on Herdr 0.8.0 with the default-session ``` The projected spawn in that run used the historical empty opt-in file, so a home that had already enabled the projection keeps it without any migration step. -One concurrent cross-home recovery case refused under contention on a loaded machine and passed on an immediate rerun; recovery-path presentation lock contention remains a deliberate hard refusal by default rather than a flat fallback, which default-on makes reachable from any Herdr home. -Callers that need concurrent recoveries to serialize can pass `fm-spawn.sh --herdr-resume-lock-wait`. -The flag applies to a fresh ship or scout spawn, and the multi-task path forwards it to each per-pair spawn. -It has no effect on `--relaunch` and `--secondmate`, because those paths take no exact-resume presentation-order lock. +One concurrent cross-home recovery case refused under contention on a loaded machine and passed on an immediate rerun, demonstrating hard refusal rather than a flat fallback. +[Herdr backend operational compromises](../herdr-backend.md#operational-compromises) owns the current recovery-lock behavior. That run measured the default-on projection on Herdr 0.8.0 only, while the focus-flash regression below was last run on 0.7.5 before the flip, so neither run covered a defective release under default-on projection; the version floor and the focus-flash suite's Part C close that gap. The restored-shell session-start cleanup ran on 2026-07-24 against Herdr 0.7.5 protocol 17: diff --git a/tests/fm-backend-herdr-presentation-e2e.test.sh b/tests/fm-backend-herdr-presentation-e2e.test.sh index 1be3a2054ea..a84d31adcd9 100755 --- a/tests/fm-backend-herdr-presentation-e2e.test.sh +++ b/tests/fm-backend-herdr-presentation-e2e.test.sh @@ -1329,9 +1329,9 @@ PATH="$HERDR_ORIGINAL_PATH" "$HERDR_LAB_HELPER" stop "$HERDR_LAB_SESSION" >/dev/ PATH="$HERDR_ORIGINAL_PATH" "$HERDR_LAB_HELPER" provision "$HERDR_LAB_SESSION" \ || fail "could not reprovision the isolated session for concurrent recovery" CONCURRENT_RECOVERY_FOCUS=$(focus_snapshot) -spawn_task "$PRIMARY_WAVE_ID" "$HOME_DIR" "$RECOVERY_PROJECT_DIR" > "$TMP_ROOT/primary-wave-resume.out" 2> "$TMP_ROOT/primary-wave-resume.err" & +spawn_task "$PRIMARY_WAVE_ID" "$HOME_DIR" "$RECOVERY_PROJECT_DIR" --herdr-resume-lock-wait > "$TMP_ROOT/primary-wave-resume.out" 2> "$TMP_ROOT/primary-wave-resume.err" & PRIMARY_WAVE_PID=$! -spawn_task "$BRAVO_WAVE_ID" "$SECOND_HOME_B" "$RECOVERY_PROJECT_DIR" > "$TMP_ROOT/bravo-wave-resume.out" 2> "$TMP_ROOT/bravo-wave-resume.err" & +spawn_task "$BRAVO_WAVE_ID" "$SECOND_HOME_B" "$RECOVERY_PROJECT_DIR" --herdr-resume-lock-wait > "$TMP_ROOT/bravo-wave-resume.out" 2> "$TMP_ROOT/bravo-wave-resume.err" & BRAVO_WAVE_PID=$! wait "$PRIMARY_WAVE_PID" || fail "concurrent primary recovery failed: $(cat "$TMP_ROOT/primary-wave-resume.err")" wait "$BRAVO_WAVE_PID" || fail "concurrent secondmate recovery failed: $(cat "$TMP_ROOT/bravo-wave-resume.err")" @@ -1358,7 +1358,7 @@ teardown_task "$BRAVO_WAVE_ID" "$SECOND_HOME_B" > "$TMP_ROOT/bravo-wave-teardown "$REAL_TREEHOUSE" return --force "$BRAVO_WAVE_OLD_WT" >/dev/null 2>&1 || true "$REAL_TREEHOUSE" return --force "$PRIMARY_WAVE_NEW_WT" >/dev/null 2>&1 || true "$REAL_TREEHOUSE" return --force "$BRAVO_WAVE_NEW_WT" >/dev/null 2>&1 || true -pass "real Herdr lab: concurrent cross-home recoveries replace exact husks under one session lock with no focus drift" +pass "real Herdr lab: opted-in concurrent cross-home recoveries replace exact husks under one session lock with no focus drift" # Exact-resume presentation-lock contention refuses by default. Hold the # shared session lock from an unrelated process past the bounded-retry window