From 7a863f7baad4924ddb98be028ea526e8c641a9bf Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Thu, 9 Jul 2026 21:23:28 -0400 Subject: [PATCH 01/42] clarify PR ready review checks --- AGENTS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 076d8e1426..1b5cc8a22d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -610,6 +610,9 @@ During the `ci` monitor phase, `bin/fm-crew-state.sh` also reads the ci step log For PR-based ship tasks, the ready signal depends on mode: `no-mistakes` reports `done: PR checks green` after CI is green, while `direct-PR` reports `done: PR ` after opening the PR. Run `bin/fm-pr-check.sh ` - it records `pr=` and GitHub's `pr_head=` when available in the task's meta and arms the watcher's merge poll. +Before presenting the PR as ready, verify the latest review or bot verdict on the PR's current head, not a proxy. +A green status check is not enough for PRs with review bots such as Cursor Bugbot. +Check unresolved review threads and review verdicts against the PR's current head, and treat a head that has not been reviewed yet as not clean. Tell the captain: the PR's full URL (always the complete `https://...` link, never a bare `#number` - the captain's terminal makes a full URL clickable), a one-paragraph summary, and, for `no-mistakes`, the risk level it emitted. (The check contract, for any custom `state/.check.sh` you write yourself: print one line only when firstmate should wake, print nothing otherwise, and finish before `FM_CHECK_TIMEOUT`.) From 7525a370ee2350b6a882dc9e65bcdf945419b62f Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Thu, 9 Jul 2026 19:44:55 -0700 Subject: [PATCH 02/42] fix: keep persistent secondmates out of the main backlog (#398) * fix secondmate backlog guidance * no-mistakes(review): Require reasons for captain backlog holds * no-mistakes(test): Document secondmate handoff skill requirement * fix secondmate teardown reminder * no-mistakes(document): sync teardown reminder docs to work-items-only backlog contract --- .agents/skills/secondmate-provisioning/SKILL.md | 1 + AGENTS.md | 10 +++++++--- bin/fm-teardown.sh | 7 +++---- docs/scripts.md | 2 +- tests/fm-secondmate-lifecycle-e2e.test.sh | 7 +++++-- 5 files changed, 17 insertions(+), 10 deletions(-) diff --git a/.agents/skills/secondmate-provisioning/SKILL.md b/.agents/skills/secondmate-provisioning/SKILL.md index 7fed501d37..cb626e12ba 100644 --- a/.agents/skills/secondmate-provisioning/SKILL.md +++ b/.agents/skills/secondmate-provisioning/SKILL.md @@ -83,6 +83,7 @@ For `no-mistakes` projects, seeding initializes only projects newly cloned into ## Backlog handoff +Apply `AGENTS.md` section 10's work-items-only backlog contract before creation or handoff. When a secondmate is created for a domain, existing main-backlog items that fall under its scope should become its work instead of staying stranded in the main backlog. Scope-matching is firstmate's judgment against the secondmate's natural-language scope, not a keyword rule. Read `data/backlog.md`, pick queued items that fit the new scope, and move them with: diff --git a/AGENTS.md b/AGENTS.md index 076d8e1426..579110e6a0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -544,7 +544,8 @@ No nudge is needed at spawn because the agent reads `AGENTS.md` fresh on launch. For already-live secondmates, use `bin/fm-config-push.sh` when only this inherited config needs to be pushed. Project worktrees start at detached HEAD on a clean default branch; ship briefs tell the crewmate to create its branch, while scout briefs keep the worktree scratch. After spawning, peek the endpoint to confirm the crewmate is processing the brief and handle any trust dialog with `harness-adapters`. -Add the task to `data/backlog.md` under In flight. +For a ship or scout task, add the task to `data/backlog.md` under In flight. +A secondmate spawn adds no backlog row: its identity and scope live in `data/secondmates.md`, its runtime lives in `state/.meta`, and section 10 owns the backlog contract. ### Supervise @@ -811,7 +812,10 @@ As a courtesy, mention cost when unusually much work is running (more than ~8 co ## 10. Backlog format `data/backlog.md` is the durable queue. -Update it on every dispatch, completion, and decision. +It tracks work items only, never agents; persistent secondmates never appear as backlog items. +Work routed to a secondmate is recorded in that secondmate home's own backlog, not the main backlog. +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; use `tasks-axi hold --reason "" --kind captain` for a captain-gated thread. +Update the backlog on every dispatch, completion, and decision for a work item. ```markdown ## In flight @@ -852,7 +856,7 @@ Map firstmate's real backlog operations to the approved commands: - Manage dependencies: `tasks-axi block --by ` and `tasks-axi unblock --by `, then `tasks-axi ready` to list queued work with no unresolved blockers. This is a dependency check only; future-dated items still stay queued until their date arrives. - Read an item's full notes: `tasks-axi show --full`. -- Hand a task off to a secondmate home: keep using `bin/fm-backlog-handoff.sh ...`; do not call bare `tasks-axi mv` for this path, because the helper resolves and validates the secondmate home before moving anything. +- Hand a task off to a secondmate home: load `secondmate-provisioning`, then keep using `bin/fm-backlog-handoff.sh ...`; do not call bare `tasks-axi mv` for this path, because the helper resolves and validates the secondmate home before moving anything. - Normalize the file: `tasks-axi render` rewrites every id'd task in canonical form and leaves free-form lines untouched. **Note hygiene:** Keep free-form backlog and task note/status prose free of volatile incidental specifics that rot: temp paths, in-flight versions, moving state locations, and ephemeral IDs. diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index ea07283a1d..4ecc406a7d 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -2,7 +2,8 @@ # Tear down a finished task: return the treehouse worktree, release the Orca # worktree, or retire a secondmate home; kill the recorded runtime endpoint, # clear volatile state, refresh/prune the project's clone for PR-based ship -# tasks, then print a backlog-refresh reminder. +# tasks, then print a backlog-refresh reminder for ship and scout teardowns +# (a secondmate teardown prints none, since secondmates are not backlog items). # REFUSES if the worktree holds work that has not LANDED, because cleanup # hard-resets/removes the worktree and kills its processes. Work has landed when it is # reachable from any remote-tracking branch (a fork counts as a remote, so @@ -298,15 +299,13 @@ work_is_landed() { backlog_refresh_reminder() { local pr done_cmd report_path + [ "$KIND" = secondmate ] && return 0 if fm_tasks_axi_backend_available "$CONFIG"; then case "$KIND" in scout) report_path="data/$ID/report.md" done_cmd="tasks-axi done $ID --report $report_path" ;; - secondmate) - done_cmd="tasks-axi done $ID --note \"retired\"" - ;; *) if [ "$MODE" = local-only ]; then done_cmd="tasks-axi done $ID --note \"local main\"" diff --git a/docs/scripts.md b/docs/scripts.md index 5f30918dda..661e4271e6 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -55,7 +55,7 @@ If you have changed away from the firstmate home in an interactive shell, invoke | `fm-pr-check.sh` | Record `pr=` and GitHub's `pr_head=` when available for a PR-ready task, then arm the watcher's merge poll | | `fm-pr-merge.sh` | Require a full GitHub PR URL, record `pr=` and available `pr_head=` via `fm-pr-check.sh`, parse it into `gh-axi pr merge --repo /`, default to `--squash` unless a merge method is forwarded, and reject malformed URLs or repo overrides | | `fm-promote.sh` | Promote a scout task in place so it becomes a protected ship task | -| `fm-teardown.sh` | Return a clean, landed ship worktree or retire/release a secondmate home; requires scout reports, checks child work, tolerates only provably stale worktree git locks, removes firstmate-owned hook artifacts, closes recorded backend endpoints under their owning home context, releases Orca worktrees through `orca worktree rm`, and prints the backlog-backend reminder | +| `fm-teardown.sh` | Return a clean, landed ship worktree or retire/release a secondmate home; requires scout reports, checks child work, tolerates only provably stale worktree git locks, removes firstmate-owned hook artifacts, closes recorded backend endpoints under their owning home context, releases Orca worktrees through `orca worktree rm`, and prints the backlog-backend reminder for ship and scout teardowns only (a secondmate home teardown prints no backlog reminder) | | `fm-harness.sh` | Detect the running harness; resolve the effective crewmate (`crew`) or secondmate-launch (`secondmate`) harness; expose optional `config/secondmate-harness` model and effort tokens with `secondmate-model` and `secondmate-effort` | | `fm-lock.sh` | Per-home firstmate session lock | | `fm-x-lib.sh` | Shared X-mode `.env`, alternate env-file, relay, dry-run config, platform-aware reply-thread splitting, outbound image payloads, and task-to-X-request meta-link helpers | diff --git a/tests/fm-secondmate-lifecycle-e2e.test.sh b/tests/fm-secondmate-lifecycle-e2e.test.sh index 13d43bc688..914fb7be7e 100755 --- a/tests/fm-secondmate-lifecycle-e2e.test.sh +++ b/tests/fm-secondmate-lifecycle-e2e.test.sh @@ -203,10 +203,13 @@ phase_recovery() { } phase_teardown() { + local teardown_out : > "$LOG" - PATH="$FAKEBIN:$PATH" FM_HOME="$HOME_DIR" FM_FAKE_TMUX_LOG="$LOG" FM_FAKE_TMUX_CAPTURE="$PANE" \ - "$ROOT/bin/fm-teardown.sh" design >/dev/null 2>&1 \ + teardown_out=$(PATH="$FAKEBIN:$PATH" FM_HOME="$HOME_DIR" FM_FAKE_TMUX_LOG="$LOG" FM_FAKE_TMUX_CAPTURE="$PANE" \ + "$ROOT/bin/fm-teardown.sh" design 2>&1) \ || fail "teardown failed for the empty secondmate home" + printf '%s\n' "$teardown_out" | grep -F 'Backlog:' >/dev/null \ + && fail "secondmate teardown emitted a main-backlog completion reminder" assert_absent "$SUB" "teardown did not remove the retired secondmate home" assert_absent "$HOME_DIR/state/design.meta" "teardown did not clear the parent meta" assert_no_grep '- design ' "$HOME_DIR/data/secondmates.md" "teardown did not remove the registry route" From 88ab96682a5b931c5fc9f941fede1deee254ebbf Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Thu, 9 Jul 2026 21:29:45 -0400 Subject: [PATCH 03/42] Harden session lock identity --- .opencode/plugins/fm-primary-watch-arm.js | 6 +- .pi/extensions/fm-primary-pi-watch.ts | 6 +- .pi/extensions/fm-primary-turnend-guard.ts | 6 +- CONTRIBUTING.md | 1 + bin/fm-lock.sh | 80 ++++++++-- tests/fm-grok-harness.test.sh | 3 +- tests/fm-lock.test.sh | 166 +++++++++++++++++++++ tests/fm-session-start.test.sh | 4 +- 8 files changed, 252 insertions(+), 20 deletions(-) create mode 100644 tests/fm-lock.test.sh diff --git a/.opencode/plugins/fm-primary-watch-arm.js b/.opencode/plugins/fm-primary-watch-arm.js index 5147f839a9..f6ce961d32 100644 --- a/.opencode/plugins/fm-primary-watch-arm.js +++ b/.opencode/plugins/fm-primary-watch-arm.js @@ -102,10 +102,14 @@ function shouldArm(paths) { } } +function firstLockLine(text) { + return text.split(/\r?\n/, 1)[0].trim(); +} + async function sessionOwnsLock(paths) { let lockPid = ""; try { - lockPid = readFileSync(`${paths.state}/.lock`, "utf8").trim(); + lockPid = firstLockLine(readFileSync(`${paths.state}/.lock`, "utf8")); } catch { return false; } diff --git a/.pi/extensions/fm-primary-pi-watch.ts b/.pi/extensions/fm-primary-pi-watch.ts index cd830b3faf..f96a7fa26f 100644 --- a/.pi/extensions/fm-primary-pi-watch.ts +++ b/.pi/extensions/fm-primary-pi-watch.ts @@ -42,10 +42,14 @@ function pidAlive(pid: string): boolean { } } +function firstLockLine(text: string): string { + return text.split(/\r?\n/, 1)[0].trim(); +} + function lockOwnership(): LockOwnership { let lockPid = ""; try { - lockPid = readFileSync(`${state}/.lock`, "utf8").trim(); + lockPid = firstLockLine(readFileSync(`${state}/.lock`, "utf8")); } catch { return "missing"; } diff --git a/.pi/extensions/fm-primary-turnend-guard.ts b/.pi/extensions/fm-primary-turnend-guard.ts index 2d61b2f7b7..cfa56d890c 100644 --- a/.pi/extensions/fm-primary-turnend-guard.ts +++ b/.pi/extensions/fm-primary-turnend-guard.ts @@ -32,10 +32,14 @@ function pidAlive(pid: string): boolean { } } +function firstLockLine(text: string): string { + return text.split(/\r?\n/, 1)[0].trim(); +} + function lockOwnership(): LockOwnership { let lockPid = ""; try { - lockPid = readFileSync(`${state}/.lock`, "utf8").trim(); + lockPid = firstLockLine(readFileSync(`${state}/.lock`, "utf8")); } catch { return "missing"; } diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8723c5f8be..14503c93f7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -72,6 +72,7 @@ for script in bin/*.sh bin/backends/*.sh; do bash -n "$script"; done # syntax- shellcheck bin/*.sh bin/backends/*.sh tests/*.sh # lint the toolbelt and behavior tests; CI enforces this for test_script in tests/*.test.sh; do bash "$test_script"; done # behavior tests, matching CI and no-mistakes commands.test tests/fm-wake-queue.test.sh # durable wake queue losslessness, catch-up, double-drain, duplicate-collapse, and drain liveness guard tests +tests/fm-lock.test.sh # session lock PID/start-time identity, stale PID reuse, same-holder re-acquire, and app-server rejection tests tests/fm-watcher-lock.test.sh # watcher singleton, lock-race, PID identity stability, watch-arm liveness, and guard-warning tests tests/fm-turnend-guard.test.sh # shared supervision predicate plus Claude Stop-hook scoping, loop guard, fail-open, and live watcher health tests tests/fm-arm-pretool-check.test.sh # PreToolUse watcher-arm seatbelt: CLI/stdin allow-deny table, fail-open, --claude output shaping, and all five harness wiring files diff --git a/bin/fm-lock.sh b/bin/fm-lock.sh index 33e4b0d279..feee40c374 100755 --- a/bin/fm-lock.sh +++ b/bin/fm-lock.sh @@ -1,8 +1,9 @@ #!/usr/bin/env bash # Acquire or inspect the per-home firstmate session lock. -# Writes the harness (agent) process PID found by walking the shell's ancestry, -# which lives as long as the firstmate session - unlike the transient subshell -# PID of any one tool call, which is dead moments after it is written. +# Writes the harness (agent) process PID and start time found by walking the +# shell's ancestry, which lives as long as the firstmate session - unlike the +# transient subshell PID of any one tool call, which is dead moments after it is +# written. # Usage: fm-lock.sh acquire; exit 1 if another live session holds it # fm-lock.sh status print holder and liveness; always exits 0 set -u @@ -17,17 +18,60 @@ mkdir -p "$STATE" # Known harness command names; extend when a new adapter is verified. HARNESS_RE='claude|codex|opencode|grok|^pi$' +process_start_time() { + local pid=$1 out + out=$(LC_ALL=C ps -o lstart= -p "$pid" 2>/dev/null) || return 1 + out=$(printf '%s\n' "$out" | sed 's/^[[:space:]]*//;s/[[:space:]]*$//') + [ -n "$out" ] || return 1 + printf '%s\n' "$out" +} + +process_command_text() { + local pid=$1 comm args + comm=$(ps -o comm= -p "$pid" 2>/dev/null) || return 1 + args=$(ps -o args= -p "$pid" 2>/dev/null || true) + printf '%s %s\n' "$(basename "$comm")" "$args" +} + +command_is_app_server() { + printf '%s' "$1" | grep -qE '(^|[[:space:]/])app-server([[:space:]]|$)' +} + +lock_pid() { + sed -n '1{s/^[[:space:]]*//;s/[[:space:]]*$//;p;q;}' "$1" 2>/dev/null +} + +lock_start_time() { + sed -n '2{s/^[[:space:]]*//;s/[[:space:]]*$//;p;q;}' "$1" 2>/dev/null +} + +write_lock() { + local pid=$1 start tmp + start=$(process_start_time "$pid") || return 1 + tmp="$LOCK.tmp.$$" + { + printf '%s\n' "$pid" + printf '%s\n' "$start" + } > "$tmp" || return 1 + mv -f "$tmp" "$LOCK" +} + harness_pid() { - local pid=$$ comm args + local pid=$$ comm args command for _ in 1 2 3 4 5 6 7 8; do comm=$(ps -o comm= -p "$pid" 2>/dev/null) || return 1 args=$(ps -o args= -p "$pid" 2>/dev/null) - if printf '%s' "$(basename "$comm")" | grep -qE "$HARNESS_RE"; then + command="$(basename "$comm") $args" + if ! command_is_app_server "$command" && printf '%s' "$(basename "$comm")" | grep -qE "$HARNESS_RE"; then echo "$pid"; return 0 fi # Bare interpreter (e.g. node): match the harness name in its script path. case "$comm" in - *node*|*python*) printf '%s' "$args" | grep -qE "$HARNESS_RE" && { echo "$pid"; return 0; } ;; + *node*|*python*) + if ! command_is_app_server "$command" && printf '%s' "$args" | grep -qE "$HARNESS_RE"; then + echo "$pid"; return 0 + fi + ;; esac pid=$(ps -o ppid= -p "$pid" 2>/dev/null | tr -d ' ') [ -n "$pid" ] && [ "$pid" -gt 1 ] || return 1 @@ -35,27 +79,33 @@ harness_pid() { return 1 } -holder_alive() { # true if $1 is a live process that looks like a harness - local pid=$1 comm +holder_alive() { + local lock=$1 pid recorded_start current_start command + pid=$(lock_pid "$lock") + recorded_start=$(lock_start_time "$lock") + [ -n "$recorded_start" ] || return 1 kill -0 "$pid" 2>/dev/null || return 1 - comm=$(ps -o comm= -p "$pid" 2>/dev/null) || return 1 - printf '%s' "$(basename "$comm") $(ps -o args= -p "$pid" 2>/dev/null)" | grep -qE "$HARNESS_RE" + current_start=$(process_start_time "$pid") || return 1 + [ "$current_start" = "$recorded_start" ] || return 1 + command=$(process_command_text "$pid") || return 1 + command_is_app_server "$command" && return 1 + printf '%s' "$command" | grep -qE "$HARNESS_RE" } if [ "${1:-}" = "status" ]; then if [ ! -f "$LOCK" ]; then echo "lock: free"; exit 0; fi - old=$(cat "$LOCK") - if holder_alive "$old"; then echo "lock: held by live harness pid $old"; else echo "lock: stale (pid $old dead or not a harness)"; fi + old=$(lock_pid "$LOCK") + if holder_alive "$LOCK"; then echo "lock: held by live harness pid $old"; else echo "lock: stale (pid $old dead or not a harness)"; fi exit 0 fi me=$(harness_pid) || { echo "error: cannot locate harness process in ancestry" >&2; exit 1; } if [ -f "$LOCK" ]; then - old=$(cat "$LOCK") - if [ "$old" != "$me" ] && holder_alive "$old"; then + old=$(lock_pid "$LOCK") + if [ "$old" != "$me" ] && holder_alive "$LOCK"; then echo "error: another live firstmate session holds the lock (pid $old); operate read-only until resolved" >&2 exit 1 fi fi -echo "$me" > "$LOCK" +write_lock "$me" || { echo "error: cannot record harness process identity" >&2; exit 1; } echo "lock acquired: harness pid $me" diff --git a/tests/fm-grok-harness.test.sh b/tests/fm-grok-harness.test.sh index 655efe2fee..fd462ccf27 100755 --- a/tests/fm-grok-harness.test.sh +++ b/tests/fm-grok-harness.test.sh @@ -121,10 +121,11 @@ test_fm_lock_recognizes_grok_holder() { home="$TMP_ROOT/lock-home" fakebin=$(fm_fakebin "$TMP_ROOT/lock-fake") mkdir -p "$home/state" - printf '%s\n' "$$" > "$home/state/.lock" + printf '%s\n%s\n' "$$" 'Mon Jan 1 00:00:00 2024' > "$home/state/.lock" cat > "$fakebin/ps" <<'SH' #!/usr/bin/env bash case "$*" in + *"lstart="*) printf '%s\n' 'Mon Jan 1 00:00:00 2024'; exit 0 ;; *"comm="*) printf '%s\n' '/usr/local/bin/grok'; exit 0 ;; *"args="*) printf '%s\n' 'grok'; exit 0 ;; esac diff --git a/tests/fm-lock.test.sh b/tests/fm-lock.test.sh new file mode 100644 index 0000000000..c5eb098c66 --- /dev/null +++ b/tests/fm-lock.test.sh @@ -0,0 +1,166 @@ +#!/usr/bin/env bash +# Session-lock identity tests for bin/fm-lock.sh. +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +LOCK="$ROOT/bin/fm-lock.sh" +TMP_ROOT=$(fm_test_tmproot fm-lock) +FM_LOCK_TEST_PIDS=() + +cleanup_lock_tests() { + local pid + for pid in "${FM_LOCK_TEST_PIDS[@]:-}"; do + kill "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + done + fm_test_cleanup +} +trap cleanup_lock_tests EXIT + +make_fake_ps() { + local fakebin=$1 + mkdir -p "$fakebin" + cat > "$fakebin/ps" <<'SH' +#!/usr/bin/env bash +set -u +pid="" +prev="" +for arg in "$@"; do + if [ "$prev" = "-p" ]; then + pid="$arg" + break + fi + prev="$arg" +done +[ -n "$pid" ] || exit 1 + +if [ "$pid" = "${FM_FAKE_HOLDER_PID:-}" ]; then + start=${FM_FAKE_HOLDER_START:-Mon Jan 1 00:00:00 2024} + comm=${FM_FAKE_HOLDER_COMM:-/usr/local/bin/codex} + args=${FM_FAKE_HOLDER_ARGS:-codex} + ppid=${FM_FAKE_HOLDER_PPID:-1} +else + start=${FM_FAKE_OTHER_START:-Tue Jan 2 00:00:00 2024} + comm=${FM_FAKE_OTHER_COMM:-/usr/local/bin/codex} + args=${FM_FAKE_OTHER_ARGS:-codex} + ppid=${FM_FAKE_OTHER_PPID:-${FM_FAKE_HOLDER_PID:-1}} +fi + +case "$*" in + *"lstart="*) printf '%s\n' "$start"; exit 0 ;; + *"comm="*) printf '%s\n' "$comm"; exit 0 ;; + *"args="*) printf '%s\n' "$args"; exit 0 ;; + *"ppid="*) printf '%s\n' "$ppid"; exit 0 ;; +esac +exit 1 +SH + chmod +x "$fakebin/ps" +} + +start_live_holder() { + sleep 300 >/dev/null 2>&1 & + FM_LOCK_TEST_PIDS+=("$!") + printf '%s\n' "$!" +} + +test_pid_reuse_with_mismatched_start_time_acquires() { + local home fakebin holder out status written_pid + home="$TMP_ROOT/reuse-home" + fakebin="$TMP_ROOT/reuse-fakebin" + mkdir -p "$home/state" + make_fake_ps "$fakebin" + + holder=$(start_live_holder) + printf '%s\n%s\n' "$holder" 'Sun Dec 31 23:59:59 2023' > "$home/state/.lock" + + status=0 + out=$(FM_HOME="$home" \ + FM_FAKE_HOLDER_PID="$holder" \ + FM_FAKE_HOLDER_START='Mon Jan 1 00:00:00 2024' \ + PATH="$fakebin:$PATH" \ + "$LOCK" 2>&1) || status=$? + + expect_code 0 "$status" "PID-reuse lock should be treated as stale and acquired" + assert_contains "$out" "lock acquired: harness pid" "PID-reuse lock did not acquire" + written_pid=$(sed -n '1p' "$home/state/.lock") + [ "$written_pid" != "$holder" ] || fail "PID-reuse acquisition kept the recycled holder pid" + pass "fm-lock treats a live harness-named PID with mismatched start time as stale" +} + +test_live_same_identity_blocks_another_session() { + local home fakebin holder out status + home="$TMP_ROOT/live-block-home" + fakebin="$TMP_ROOT/live-block-fakebin" + mkdir -p "$home/state" + make_fake_ps "$fakebin" + + holder=$(start_live_holder) + printf '%s\n%s\n' "$holder" 'Mon Jan 1 00:00:00 2024' > "$home/state/.lock" + + status=0 + out=$(FM_HOME="$home" \ + FM_FAKE_HOLDER_PID="$holder" \ + FM_FAKE_HOLDER_START='Mon Jan 1 00:00:00 2024' \ + PATH="$fakebin:$PATH" \ + "$LOCK" 2>&1) || status=$? + + expect_code 1 "$status" "live same-identity lock should block another session" + assert_contains "$out" "another live firstmate session holds the lock" "live same-identity lock did not block" + pass "fm-lock blocks a genuinely live same-identity holder" +} + +test_same_session_reacquire_succeeds() { + local home fakebin holder out status + home="$TMP_ROOT/reacquire-home" + fakebin="$TMP_ROOT/reacquire-fakebin" + mkdir -p "$home/state" + make_fake_ps "$fakebin" + + holder=$(start_live_holder) + printf '%s\n%s\n' "$holder" 'Mon Jan 1 00:00:00 2024' > "$home/state/.lock" + + status=0 + out=$(FM_HOME="$home" \ + FM_FAKE_HOLDER_PID="$holder" \ + FM_FAKE_HOLDER_START='Mon Jan 1 00:00:00 2024' \ + FM_FAKE_OTHER_COMM=/bin/bash \ + FM_FAKE_OTHER_ARGS=bash \ + FM_FAKE_OTHER_PPID="$holder" \ + PATH="$fakebin:$PATH" \ + "$LOCK" 2>&1) || status=$? + + expect_code 0 "$status" "same-session re-acquire should succeed" + assert_contains "$out" "lock acquired: harness pid $holder" "same-session re-acquire did not retain the holder pid" + pass "fm-lock still allows a genuine same-session re-acquire" +} + +test_app_server_holder_is_stale() { + local home fakebin holder out status + home="$TMP_ROOT/app-server-home" + fakebin="$TMP_ROOT/app-server-fakebin" + mkdir -p "$home/state" + make_fake_ps "$fakebin" + + holder=$(start_live_holder) + printf '%s\n%s\n' "$holder" 'Mon Jan 1 00:00:00 2024' > "$home/state/.lock" + + status=0 + out=$(FM_HOME="$home" \ + FM_FAKE_HOLDER_PID="$holder" \ + FM_FAKE_HOLDER_START='Mon Jan 1 00:00:00 2024' \ + FM_FAKE_HOLDER_COMM=/Applications/ChatGPT.app/Contents/Resources/codex \ + FM_FAKE_HOLDER_ARGS='/Applications/ChatGPT.app/Contents/Resources/codex app-server --port 0' \ + PATH="$fakebin:$PATH" \ + "$LOCK" 2>&1) || status=$? + + expect_code 0 "$status" "app-server holder should be treated as stale and acquired" + assert_contains "$out" "lock acquired: harness pid" "app-server holder did not allow acquisition" + pass "fm-lock rejects Codex app-server as a live session holder" +} + +test_pid_reuse_with_mismatched_start_time_acquires +test_live_same_identity_blocks_another_session +test_same_session_reacquire_succeeds +test_app_server_holder_is_stale diff --git a/tests/fm-session-start.test.sh b/tests/fm-session-start.test.sh index e324c3647f..3512036392 100755 --- a/tests/fm-session-start.test.sh +++ b/tests/fm-session-start.test.sh @@ -97,6 +97,7 @@ make_fake_ps_harness() { set -u harness=${FM_FAKE_HARNESS:-claude} case "$*" in + *"lstart="*) printf '%s\n' 'Mon Jan 1 00:00:00 2024'; exit 0 ;; *"comm="*) printf '/usr/local/bin/%s\n' "$harness"; exit 0 ;; *"args="*) printf '%s\n' "$harness"; exit 0 ;; esac @@ -118,6 +119,7 @@ for arg in "\$@"; do prev="\$arg" done case "\$*" in + *"lstart="*) printf '%s\n' 'Mon Jan 1 00:00:00 2024'; exit 0 ;; *"comm="*) if [ "\$pid" = "$holder_pid" ]; then printf '/usr/local/bin/pi\n' @@ -290,7 +292,7 @@ EOF sleep 300 & holder_pid=$! - printf '%s\n' "$holder_pid" > "$home/state/.lock" + printf '%s\n%s\n' "$holder_pid" 'Mon Jan 1 00:00:00 2024' > "$home/state/.lock" status=0 out=$(run_session_start "$home" "$root" "$fakebin:$BASE_PATH") || status=$? From 50f08415291646440ac720112c708f4bdcbb4ec3 Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Thu, 9 Jul 2026 22:24:35 -0400 Subject: [PATCH 04/42] no-mistakes(document): document two-line session lock format in AGENTS.md state listing --- AGENTS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/AGENTS.md b/AGENTS.md index 1b5cc8a22d..ef60ec724e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -102,6 +102,7 @@ state/ volatile runtime signals; gitignored x-poll.error generated X-mode relay diagnostic dedupe marker .wake-queue durable queued wakes: epochseqkindkeypayload .afk durable away-mode flag; present = sub-supervisor may inject escalations (set by /afk, cleared on user return) + .lock per-home session lock written by fm-lock.sh: harness PID on line 1, holder process start time on line 2; direct readers parse only the first-line PID, while acquire/status treat it as held only when that PID is live with a matching start time and is not a Codex app-server .watch.lock .wake-queue.lock watcher singleton and queue serialization locks .hash-* .count-* .stale-* .stale-since-* .wedge-escalations-* .seen-* .hb-surfaced-* .last-* .heartbeat-streak watcher internals; never touch .watch-triage.log watcher's absorbed-wake debug log (size-capped); never relied on, safe to delete From 600075e157ceac42a605a4f70bfa4ed3e1b2c06d Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Thu, 9 Jul 2026 19:52:23 -0700 Subject: [PATCH 05/42] fix(backlog-handoff): move full item blocks including indented bodies (#401) * fix(backlog-handoff): move full item blocks including indented bodies fm-backlog-handoff only moved the checklist header line, so multi-line item bodies were left orphaned in the source backlog and never reached the secondmate. Move the full block (header plus indented body lines) atomically, treating body membership by indentation so lines like ## Intent stay with the item, and add regression coverage. * no-mistakes(review): Captain: preserve EOF handoff terminators * no-mistakes(review): treat blank lines inside item bodies as movable body * no-mistakes(document): sync backlog-handoff docs with full-block move behavior --- .../skills/secondmate-provisioning/SKILL.md | 2 +- CONTRIBUTING.md | 1 + bin/fm-backlog-handoff.sh | 149 ++++-- tests/fm-backlog-handoff.test.sh | 423 ++++++++++++++++++ 4 files changed, 547 insertions(+), 28 deletions(-) create mode 100755 tests/fm-backlog-handoff.test.sh diff --git a/.agents/skills/secondmate-provisioning/SKILL.md b/.agents/skills/secondmate-provisioning/SKILL.md index cb626e12ba..48603220b6 100644 --- a/.agents/skills/secondmate-provisioning/SKILL.md +++ b/.agents/skills/secondmate-provisioning/SKILL.md @@ -94,7 +94,7 @@ bin/fm-backlog-handoff.sh ... After seeding, run this handoff for the new secondmate's in-scope queued items. The helper resolves the secondmate home from `data/secondmates.md` and mechanically moves each named item from the main `data/backlog.md` into the secondmate home's `data/backlog.md`. -It preserves the line and its section, so the item is neither duplicated nor lost. +It moves each item's whole block - the `- [ ] ...` header plus every following indented body line and blank separator, up to the next item or column-0 section heading - byte-exact under the same section, treating an indented `## ...` line as body rather than a section boundary, so neither the header nor its body is duplicated or orphaned. It refuses `## In flight` entries because active task ownership also lives in tmux and `state/`. It is idempotent; an item already in the secondmate backlog is skipped. It refuses any destination that is not a genuine seeded firstmate home with safe operational directories and a matching `.fm-secondmate-home` marker, so a move can never land in a project. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 98c930a378..6d1f22d73b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -104,6 +104,7 @@ tests/fm-secondmate-liveness.test.sh # session-start secondmate agent-liven tests/fm-secondmate-harness.test.sh # secondmate-vs-crewmate harness resolution, optional secondmate model/effort pins, primary-to-secondmate config inheritance, and config-push tests tests/fm-secondmate-lifecycle-e2e.test.sh # persistent secondmate routing, seeding, backlog handoff, spawn, recovery, teardown, and FM_HOME flow tests tests/fm-secondmate-safety.test.sh # secondmate home safety, idle charter, handoff validation, teardown boundary, and child-cleanup fail-closed tests +tests/fm-backlog-handoff.test.sh # full item-block handoff: header plus indented and blank-separated body moves byte-exact past next-item, section-heading, and end-of-file boundaries, indented ## lines stay body not section breaks, EOF terminators preserved, and body-carrying re-runs stay idempotent tests/fm-teardown.test.sh # fm-teardown.sh landed-work safety and reminder checks: fork-remote allow, squash/content landings, dirty and unlanded refusals, PR-head metadata, no-pr= branch discovery, tasks-axi/manual backlog reminder, --force override, stale-vs-live worktree git index.lock recovery tests/fm-review-diff.test.sh # fm-review-diff.sh authoritative review diff coverage: recorded pr_head=, fetched refs/pull//head, no-pr local branch behavior, and warning fallback tests/fm-pr-merge.test.sh # fm-pr-merge.sh records pr= and available pr_head= before merging, parses PR URLs into gh-axi number/--repo calls, defaults to squash, preserves explicit merge methods, rejects malformed URLs and repo overrides, and propagates real merge failures diff --git a/bin/fm-backlog-handoff.sh b/bin/fm-backlog-handoff.sh index acf9a29266..a3cabc4324 100755 --- a/bin/fm-backlog-handoff.sh +++ b/bin/fm-backlog-handoff.sh @@ -7,14 +7,20 @@ # # Scope-matching is firstmate's JUDGMENT: you pass the task-id keys you have # already judged in-scope for the secondmate. This script performs only the -# mechanical move - it removes each matched line from data/backlog.md under the -# active firstmate home and appends it, under the same section heading, to the -# secondmate home's data/backlog.md (home resolved from data/secondmates.md). It -# never changes a line's text, never writes into a project (it refuses a home -# that is not a firstmate home), and is idempotent: a key already present in the -# secondmate backlog is reported and skipped, so re-running converges. If any key -# matches neither backlog, nothing is moved. See AGENTS.md project management -# and task lifecycle. +# mechanical move - it removes each matched item BLOCK (the `- [ ] ...` +# header line plus every following body line - indented lines and blank +# separators between paragraphs - up to the next item line or column-0 section +# heading) from data/backlog.md under the active firstmate home and appends +# that full block, under the same section heading, to the secondmate home's +# data/backlog.md (home resolved from data/secondmates.md). Body membership is +# by position, not content: an indented line or a blank line continues the +# block, and indented lines that look like markdown headings (e.g. +# ` ## Intent`) stay in the item and are not treated as section boundaries. +# It never changes a line's text, never writes into a +# project (it refuses a home that is not a firstmate home), and is idempotent: +# a key already present in the secondmate backlog is reported and skipped (no +# duplicate header or body), so re-running converges. If any key matches neither +# backlog, nothing is moved. See AGENTS.md project management and task lifecycle. # Usage: fm-backlog-handoff.sh ... set -eu @@ -151,6 +157,13 @@ validate_backlog_file() { fi } +file_ends_with_lf() { + local path=$1 last_byte + [ -s "$path" ] || return 1 + last_byte=$(tail -c 1 "$path" | od -An -tx1 | tr -d '[:space:]') + [ "$last_byte" = "0a" ] +} + backlog_key_section() { local file=$1 key=$2 [ -f "$file" ] || return 1 @@ -250,50 +263,132 @@ if [ "$SUB_EXISTED" -eq 1 ]; then cp "$SUB_BACKLOG" "$SUB_BAK" fi -# Pass 1: drop the matched lines from the main backlog, capturing each removed -# line tagged with the "## " section heading it lived under. +# Pass 1: drop each matched item block from the main backlog, capturing every +# removed line (header + body) tagged with the "## " section heading it lived +# under. Body membership is position, not content: an indented line or a blank +# line continues the block (blank separators between paragraphs stay with the +# item), even when an indented line's content looks like a heading. Only a next +# item header or a column-0 "## " heading ends the block. : > "$MOVED_FILE" -awk -v keysfile="$KEYS_FILE" -v movedfile="$MOVED_FILE" ' +MAIN_FINAL_LF=0 +if file_ends_with_lf "$MAIN_BACKLOG"; then + MAIN_FINAL_LF=1 +fi +awk -v keysfile="$KEYS_FILE" -v movedfile="$MOVED_FILE" -v keptfile="$KEPT_FILE" \ + -v final_lf="$MAIN_FINAL_LF" ' + function emit_kept(rec) { + if (have_kept) print pending_kept > keptfile + pending_kept = rec + have_kept = 1 + } + function emit_moved(rec) { + if (have_moved) print pending_moved > movedfile + pending_moved = rec + have_moved = 1 + } BEGIN { while ((getline k < keysfile) > 0) { if (k != "") want[k] = 1 } section = "## Queued" + moving = 0 + } + /^## / { + moving = 0 + section = $0 + emit_kept($0) + last_source_kept = 1 + last_source_moved = 0 + next } - /^## / { section = $0; print; next } /^- \[[ x]\] / { rest = $0 sub(/^- \[[ x]\] +/, "", rest) id = rest sub(/[ \t].*/, "", id) - if (id in want) { print section "\t" $0 > movedfile; next } + if (id in want) { + emit_moved(section "\t" $0) + moving = 1 + last_source_kept = 0 + last_source_moved = 1 + next + } + moving = 0 + emit_kept($0) + last_source_kept = 1 + last_source_moved = 0 + next } - { print } -' "$MAIN_BACKLOG" > "$KEPT_FILE" + moving && /^([ \t].*)?$/ { + emit_moved(section "\t" $0) + last_source_kept = 0 + last_source_moved = 1 + next + } + { + moving = 0 + emit_kept($0) + last_source_kept = 1 + last_source_moved = 0 + } + END { + if (have_kept) { + if (last_source_kept && !final_lf) printf "%s", pending_kept > keptfile + else print pending_kept > keptfile + } + if (have_moved) { + if (last_source_moved && !final_lf) printf "%s", pending_moved > movedfile + else print pending_moved > movedfile + } + } +' "$MAIN_BACKLOG" -# Pass 2: insert each moved line at the end of its section in the sub backlog, -# creating the section heading if the sub backlog lacks it. -awk -v movedfile="$MOVED_FILE" ' - function flush(sec) { +# Pass 2: insert each moved block at the end of its section in the sub backlog, +# creating the section heading if the sub backlog lacks it. Records are one +# physical line each (section TAB line); multi-line bodies are consecutive +# records under the same section and reassemble in order. +MOVED_FINAL_LF=0 +if file_ends_with_lf "$MOVED_FILE"; then + MOVED_FINAL_LF=1 +fi +awk -v movedfile="$MOVED_FILE" -v moved_final_lf="$MOVED_FINAL_LF" ' + function add_record(rec, eol, tab, sec, line) { + tab = index(rec, "\t") + if (tab == 0) return + sec = substr(rec, 1, tab - 1) + line = substr(rec, tab + 1) + if (!(sec in items)) { order[++nsec] = sec } + items[sec] = items[sec] line eol + } + function flush(sec, needs_separator) { if (sec != "" && (sec in items) && !(sec in flushed)) { printf "%s", items[sec] + if (needs_separator && items[sec] !~ /\n$/) printf "\n" flushed[sec] = 1 } } BEGIN { nsec = 0 + have_pending = 0 while ((getline rec < movedfile) > 0) { - tab = index(rec, "\t") - if (tab == 0) continue - sec = substr(rec, 1, tab - 1) - line = substr(rec, tab + 1) - if (!(sec in items)) { order[++nsec] = sec } - items[sec] = items[sec] line "\n" + if (have_pending) add_record(pending, "\n") + pending = rec + have_pending = 1 + } + if (have_pending) { + add_record(pending, moved_final_lf ? "\n" : "") } cur = "" } - /^## / { flush(cur); cur = $0; print; next } + /^## / { flush(cur, 1); cur = $0; print; next } { print } END { - flush(cur) + needs_separator = 0 + for (i = 1; i <= nsec; i++) { + if (!(order[i] in flushed) && order[i] != cur) { + needs_separator = 1 + break + } + } + flush(cur, needs_separator) for (i = 1; i <= nsec; i++) { s = order[i] if (!(s in flushed)) { diff --git a/tests/fm-backlog-handoff.test.sh b/tests/fm-backlog-handoff.test.sh new file mode 100755 index 0000000000..1d683c65ee --- /dev/null +++ b/tests/fm-backlog-handoff.test.sh @@ -0,0 +1,423 @@ +#!/usr/bin/env bash +# tests/fm-backlog-handoff.test.sh - full item-block handoff (header + indented body). +# +# The happy single-line path and safety refusals live in the secondmate lifecycle +# and safety suites. This file owns the multi-line body contract: the full block +# moves byte-exact, nothing orphans in the source, and re-running is a no-op. +set -u + +# shellcheck source=tests/secondmate-helpers.sh disable=SC1091 +. "$(dirname "${BASH_SOURCE[0]}")/secondmate-helpers.sh" + +TMP_ROOT=$(fm_test_tmproot fm-backlog-handoff) + +setup_homes() { + local home=$1 subhome=$2 id=${3:-design} + mkdir -p "$home/data" "$home/state" + seed_secondmate_home_marker "$subhome" "$id" + local sub_abs + sub_abs=$(cd "$subhome" && pwd -P) + printf -- '- %s - feature work (home: %s; scope: feature work; projects: alpha; added 2026-07-09)\n' \ + "$id" "$sub_abs" > "$home/data/secondmates.md" +} + +# Exact multi-line block extract: header matching key plus following body lines +# (indented lines and blank separators between paragraphs), stopping at the next +# item header or unindented section heading (column-0 ##). +extract_item_block() { + local file=$1 key=$2 + awk -v key="$key" ' + /^- \[[ x]\] / { + rest = $0 + sub(/^- \[[ x]\] +/, "", rest) + id = rest + sub(/[ \t].*/, "", id) + if (capturing) exit + if (id == key) { print; capturing = 1; next } + next + } + capturing && /^## / { exit } + capturing && /^- \[[ x]\] / { exit } + capturing && /^([ \t].*)?$/ { print; next } + capturing { exit } + ' "$file" +} + +assert_block_equals() { + local label=$1 expected=$2 actual=$3 + if [ "$expected" != "$actual" ]; then + printf 'expected block:\n%s\nactual block:\n%s\n' "$expected" "$actual" >&2 + fail "$label" + fi +} + +test_body_moves_when_followed_by_another_item() { + local home="$TMP_ROOT/body-next-item-main" + local sub="$TMP_ROOT/body-next-item-sub" + setup_homes "$home" "$sub" + + cat > "$home/data/backlog.md" <<'EOF' +## Queued +- [ ] keep-a - stays first (repo: alpha) + keep-a body line +- [ ] body-item - has a body (repo: alpha) + Spec detail one. + ## Intent + Move the full block. + trailing body line +- [ ] keep-b - stays after (repo: beta) + keep-b body stays + +## Done +EOF + + local expected_block + expected_block=$(extract_item_block "$home/data/backlog.md" body-item) + + FM_HOME="$home" "$ROOT/bin/fm-backlog-handoff.sh" design body-item >/dev/null \ + || fail "handoff of body-followed-by-item failed" + + local dest_block + dest_block=$(extract_item_block "$sub/data/backlog.md" body-item) + assert_block_equals "destination body block mismatch after item-boundary handoff" \ + "$expected_block" "$dest_block" + + assert_no_grep 'body-item' "$home/data/backlog.md" "body-item header still in source" + assert_no_grep 'Spec detail one' "$home/data/backlog.md" "orphaned body line stayed in source" + assert_no_grep 'Move the full block' "$home/data/backlog.md" "orphaned body line stayed in source" + assert_no_grep 'trailing body line' "$home/data/backlog.md" "orphaned trailing body stayed in source" + # Indented heading must move with the item, not be left or treated as a section. + assert_no_grep '## Intent' "$home/data/backlog.md" "indented ## Intent left in source as if a section" + assert_grep ' ## Intent' "$sub/data/backlog.md" "indented ## Intent did not arrive at destination" + + assert_grep 'keep-a' "$home/data/backlog.md" "keep-a was wrongly removed" + assert_grep ' keep-a body line' "$home/data/backlog.md" "keep-a body was disturbed" + assert_grep 'keep-b' "$home/data/backlog.md" "keep-b was wrongly removed" + assert_grep ' keep-b body stays' "$home/data/backlog.md" "keep-b body was disturbed" + + # keep-a's body must not have grown the orphaned lines of body-item. + local keep_a_block + keep_a_block=$(extract_item_block "$home/data/backlog.md" keep-a) + assert_block_equals "keep-a block must not absorb orphaned body-item lines" \ + $'- [ ] keep-a - stays first (repo: alpha)\n keep-a body line' \ + "$keep_a_block" + + pass "body followed by another item moves intact with no source orphans" +} + +test_body_moves_when_followed_by_section_heading() { + local home="$TMP_ROOT/body-section-main" + local sub="$TMP_ROOT/body-section-sub" + setup_homes "$home" "$sub" + + cat > "$home/data/backlog.md" <<'EOF' +## Queued +- [ ] section-tail - body ends at section (repo: alpha) + last queued body + ## Intent + still body until column-0 section + +## Done +- [x] old-task - shipped - local main (merged 2026-07-01) +EOF + + local expected_block + expected_block=$(extract_item_block "$home/data/backlog.md" section-tail) + + FM_HOME="$home" "$ROOT/bin/fm-backlog-handoff.sh" design section-tail >/dev/null \ + || fail "handoff of body-followed-by-section failed" + + local dest_block + dest_block=$(extract_item_block "$sub/data/backlog.md" section-tail) + assert_block_equals "destination body block mismatch after section-boundary handoff" \ + "$expected_block" "$dest_block" + + assert_no_grep 'section-tail' "$home/data/backlog.md" "section-tail still in source" + assert_no_grep 'last queued body' "$home/data/backlog.md" "body orphaned before ## Done" + assert_no_grep 'still body until' "$home/data/backlog.md" "body after ## Intent orphaned" + assert_grep 'old-task' "$home/data/backlog.md" "Done section item was disturbed" + assert_grep '## Done' "$home/data/backlog.md" "Done section heading was disturbed" + + pass "body followed by section heading moves intact; section stays" +} + +test_body_moves_when_last_lines_of_file() { + local home="$TMP_ROOT/body-eof-main" + local sub="$TMP_ROOT/body-eof-sub" + setup_homes "$home" "$sub" + printf '%s\n' '## Queued' > "$sub/data/backlog.md" + + # No trailing newline after the last body line is still a valid file shape; + # printf builds that deliberately. + { + printf '%s\n' '## Queued' + printf '%s\n' '- [ ] eof-item - ends the file (repo: alpha)' + printf '%s\n' ' eof body line one' + printf '%s\n' ' ## Intent' + printf '%s' ' eof body line two' + } > "$home/data/backlog.md" + local expected_destination="$TMP_ROOT/body-eof-expected.md" + cp "$home/data/backlog.md" "$expected_destination" + + local expected_block + expected_block=$(extract_item_block "$home/data/backlog.md" eof-item) + + FM_HOME="$home" "$ROOT/bin/fm-backlog-handoff.sh" design eof-item >/dev/null \ + || fail "handoff of EOF body item failed" + + local dest_block + dest_block=$(extract_item_block "$sub/data/backlog.md" eof-item) + assert_block_equals "destination body block mismatch for EOF item" \ + "$expected_block" "$dest_block" + cmp -s "$expected_destination" "$sub/data/backlog.md" \ + || fail "EOF item transfer changed the final-record terminator" + + # Source should have no item residual - only the section heading remains. + if grep -E 'eof-item|eof body|## Intent' "$home/data/backlog.md" >/dev/null; then + fail "EOF item left residual header or body lines in source" + fi + assert_grep '## Queued' "$home/data/backlog.md" "Queued section heading was lost" + + pass "body as last lines of the file moves intact" +} + +test_eof_body_before_seeded_destination_section_keeps_boundary() { + local home="$TMP_ROOT/body-eof-seeded-main" + local sub="$TMP_ROOT/body-eof-seeded-sub" + setup_homes "$home" "$sub" + + { + printf '%s\n' '## Queued' + printf '%s\n' '- [ ] seeded-eof-item - ends the file (repo: alpha)' + printf '%s\n' ' seeded eof body one' + printf '%s' ' seeded eof body two' + } > "$home/data/backlog.md" + local expected_destination="$TMP_ROOT/body-eof-seeded-expected.md" + { + printf '%s\n' '## In flight' + printf '%s\n' '' + printf '%s\n' '## Queued' + printf '%s\n' '' + printf '%s\n' '- [ ] seeded-eof-item - ends the file (repo: alpha)' + printf '%s\n' ' seeded eof body one' + printf '%s\n' ' seeded eof body two' + printf '%s\n' '## Done' + } > "$expected_destination" + + FM_HOME="$home" "$ROOT/bin/fm-backlog-handoff.sh" design seeded-eof-item >/dev/null \ + || fail "handoff of EOF body into seeded backlog failed" + + cmp -s "$expected_destination" "$sub/data/backlog.md" \ + || fail "EOF body did not remain separate from the seeded ## Done heading" + + pass "EOF body before a seeded destination section keeps its boundary" +} + +test_untouched_eof_line_preserves_terminator() { + local home="$TMP_ROOT/untouched-eof-main" + local sub="$TMP_ROOT/untouched-eof-sub" + setup_homes "$home" "$sub" + + { + printf '%s\n' '## Queued' + printf '%s\n' '- [ ] move-item - remove this block (repo: alpha)' + printf '%s\n' ' move body' + printf '%s\n' '- [ ] keep-item - retain this block (repo: beta)' + printf '%s' ' keep body without a final newline' + } > "$home/data/backlog.md" + local expected_source="$TMP_ROOT/untouched-eof-expected.md" + { + printf '%s\n' '## Queued' + printf '%s\n' '- [ ] keep-item - retain this block (repo: beta)' + printf '%s' ' keep body without a final newline' + } > "$expected_source" + + FM_HOME="$home" "$ROOT/bin/fm-backlog-handoff.sh" design move-item >/dev/null \ + || fail "handoff before untouched EOF preservation check failed" + + cmp -s "$expected_source" "$home/data/backlog.md" \ + || fail "handoff changed an untouched final-record terminator" + + pass "untouched EOF line preserves its original terminator" +} + +test_body_handoff_is_idempotent() { + local home="$TMP_ROOT/body-idem-main" + local sub="$TMP_ROOT/body-idem-sub" + setup_homes "$home" "$sub" + + cat > "$home/data/backlog.md" <<'EOF' +## Queued +- [ ] neighbor - untouched (repo: alpha) + neighbor body +- [ ] idem-item - multi-line for re-run (repo: alpha) + ## Intent + Idempotent body must not duplicate. + final note + +## Done +EOF + + FM_HOME="$home" "$ROOT/bin/fm-backlog-handoff.sh" design idem-item >/dev/null \ + || fail "first handoff of body-carrying item failed" + + local main_after dest_after + main_after=$(cat "$home/data/backlog.md") + dest_after=$(cat "$sub/data/backlog.md") + + local out + out=$(FM_HOME="$home" "$ROOT/bin/fm-backlog-handoff.sh" design idem-item 2>&1) \ + || fail "idempotent re-run of body-carrying item failed" + assert_contains "$out" "already present" "re-run did not report skip of already-present key" + + [ "$main_after" = "$(cat "$home/data/backlog.md")" ] \ + || fail "idempotent re-run mutated the main backlog" + [ "$dest_after" = "$(cat "$sub/data/backlog.md")" ] \ + || fail "idempotent re-run mutated the secondmate backlog" + + local count + count=$(grep -cF -- '- [ ] idem-item - multi-line for re-run (repo: alpha)' "$sub/data/backlog.md") + [ "$count" -eq 1 ] || fail "idempotent re-run duplicated the item header (count=$count)" + count=$(grep -cF -- 'Idempotent body must not duplicate.' "$sub/data/backlog.md") + [ "$count" -eq 1 ] || fail "idempotent re-run duplicated a body line (count=$count)" + count=$(grep -cF -- ' ## Intent' "$sub/data/backlog.md") + [ "$count" -eq 1 ] || fail "idempotent re-run duplicated indented ## Intent (count=$count)" + + assert_grep 'neighbor' "$home/data/backlog.md" "neighbor item was disturbed by re-run" + assert_grep ' neighbor body' "$home/data/backlog.md" "neighbor body was disturbed by re-run" + + pass "body-carrying handoff is idempotent: re-run changes nothing" +} + +test_indented_heading_is_not_section_boundary() { + # Standalone focus on the tokenizer trap that caused the live incident. + local home="$TMP_ROOT/intent-trap-main" + local sub="$TMP_ROOT/intent-trap-sub" + setup_homes "$home" "$sub" design + + cat > "$home/data/backlog.md" <<'EOF' +## Queued +- [ ] ha-codex-fast-default-4e - harness default work (repo: firstmate) + Context for the secondmate. + ## Intent + Deliver the full spec, not the title alone. + ## Acceptance + - body survives handoff + - ## headings inside body stay body +- [ ] next-item - after the trap (repo: firstmate) +EOF + + local expected_block + expected_block=$(extract_item_block "$home/data/backlog.md" ha-codex-fast-default-4e) + + FM_HOME="$home" "$ROOT/bin/fm-backlog-handoff.sh" design ha-codex-fast-default-4e >/dev/null \ + || fail "handoff of ## Intent body item failed" + + local dest_block + dest_block=$(extract_item_block "$sub/data/backlog.md" ha-codex-fast-default-4e) + assert_block_equals "tokenizer trap: indented ## lines must move with the item" \ + "$expected_block" "$dest_block" + + # Source must not treat ## Intent / ## Acceptance as new sections that split the file. + if grep -E 'ha-codex-fast-default-4e|Deliver the full spec|body survives handoff' \ + "$home/data/backlog.md" >/dev/null; then + fail "tokenizer trap left item fragments in the source backlog" + fi + assert_grep 'next-item' "$home/data/backlog.md" "following item was lost after ## Intent body" + # Exactly one real Queued section; no spurious column-0 ## Intent section invented. + local heading_count + heading_count=$(grep -cE '^## ' "$home/data/backlog.md") + [ "$heading_count" -eq 1 ] || fail "source gained extra column-0 ## headings (count=$heading_count)" + heading_count=$(grep -cE '^## ' "$sub/data/backlog.md") + # sub scaffold has In flight / Queued / Done + [ "$heading_count" -eq 3 ] || fail "destination has unexpected ## section count (count=$heading_count)" + + pass "indented ## Intent / ## Acceptance are body, not section boundaries" +} + +test_multi_paragraph_body_with_internal_blanks_moves_whole() { + # The live re-orphan risk: a blank line inside a multi-paragraph body must not + # terminate the block and strand the paragraphs after it. Blank lines are body + # content and move with the item; only the next item header or a column-0 + # section heading ends the block. Includes an indented ## after a blank. + local home="$TMP_ROOT/multi-para-main" + local sub="$TMP_ROOT/multi-para-sub" + setup_homes "$home" "$sub" + + cat > "$home/data/backlog.md" <<'EOF' +## Queued +- [ ] before-multi - stays put (repo: alpha) + before body +- [ ] multi-para - multi-paragraph body (repo: alpha) + First paragraph line. + + Second paragraph after a blank. + ## Intent + + Indented heading then blank then more. + final line +- [ ] after-multi - subsequent item (repo: alpha) + after body + +## Done +EOF + + local expected_block + expected_block=$(extract_item_block "$home/data/backlog.md" multi-para) + + FM_HOME="$home" "$ROOT/bin/fm-backlog-handoff.sh" design multi-para >/dev/null \ + || fail "handoff of multi-paragraph body failed" + + local dest_block + dest_block=$(extract_item_block "$sub/data/backlog.md" multi-para) + assert_block_equals "multi-paragraph body with internal blanks must move whole" \ + "$expected_block" "$dest_block" + + # Every body line, including the ones after each internal blank, must leave the source. + assert_no_grep 'multi-para' "$home/data/backlog.md" "multi-para header still in source" + assert_no_grep 'First paragraph line' "$home/data/backlog.md" "first paragraph orphaned in source" + assert_no_grep 'Second paragraph after a blank' "$home/data/backlog.md" "post-blank paragraph orphaned in source" + assert_no_grep 'Indented heading then blank then more' "$home/data/backlog.md" "post-blank body orphaned in source" + assert_no_grep 'final line' "$home/data/backlog.md" "trailing body orphaned in source" + assert_no_grep '## Intent' "$home/data/backlog.md" "indented ## Intent left in source as if a section" + + # The post-blank paragraphs must actually arrive at the destination. + assert_grep ' Second paragraph after a blank.' "$sub/data/backlog.md" "post-blank paragraph did not arrive" + assert_grep ' Indented heading then blank then more.' "$sub/data/backlog.md" "post-blank body did not arrive" + assert_grep ' ## Intent' "$sub/data/backlog.md" "indented ## Intent did not arrive at destination" + + # Neighbors on both sides stay intact. + assert_grep 'before-multi' "$home/data/backlog.md" "before-multi was wrongly removed" + assert_grep ' before body' "$home/data/backlog.md" "before-multi body was disturbed" + assert_grep 'after-multi' "$home/data/backlog.md" "after-multi was wrongly removed" + assert_grep ' after body' "$home/data/backlog.md" "after-multi body was disturbed" + + # Idempotent re-run: already present, no duplication, no mutation. + local main_after dest_after + main_after=$(cat "$home/data/backlog.md") + dest_after=$(cat "$sub/data/backlog.md") + local out + out=$(FM_HOME="$home" "$ROOT/bin/fm-backlog-handoff.sh" design multi-para 2>&1) \ + || fail "idempotent re-run of multi-paragraph body failed" + assert_contains "$out" "already present" "re-run did not report skip of already-present key" + [ "$main_after" = "$(cat "$home/data/backlog.md")" ] \ + || fail "idempotent re-run mutated the main backlog" + [ "$dest_after" = "$(cat "$sub/data/backlog.md")" ] \ + || fail "idempotent re-run mutated the secondmate backlog" + local count + count=$(grep -cF -- ' Second paragraph after a blank.' "$sub/data/backlog.md") + [ "$count" -eq 1 ] || fail "idempotent re-run duplicated a post-blank paragraph (count=$count)" + + pass "multi-paragraph body with internal blank lines moves whole and is idempotent" +} + +test_body_moves_when_followed_by_another_item +test_body_moves_when_followed_by_section_heading +test_multi_paragraph_body_with_internal_blanks_moves_whole +test_body_moves_when_last_lines_of_file +test_eof_body_before_seeded_destination_section_keeps_boundary +test_untouched_eof_line_preserves_terminator +test_body_handoff_is_idempotent +test_indented_heading_is_not_section_boundary + +echo "ALL TESTS PASSED" From 732d4d4a37806643be0062bb39a83e9beaf2c7b0 Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Thu, 9 Jul 2026 21:39:19 -0400 Subject: [PATCH 06/42] feat: isolate browser sessions per spawn --- .agents/skills/harness-adapters/SKILL.md | 1 + bin/fm-spawn.sh | 129 +++++++++++ bin/fm-teardown.sh | 21 ++ docs/browser-axi-isolation.md | 28 +++ docs/configuration.md | 2 + tests/fm-browser-axi-isolation.test.sh | 260 +++++++++++++++++++++++ tests/fm-spawn-dispatch-profile.test.sh | 134 ++++++++---- 7 files changed, 532 insertions(+), 43 deletions(-) create mode 100644 docs/browser-axi-isolation.md create mode 100644 tests/fm-browser-axi-isolation.test.sh diff --git a/.agents/skills/harness-adapters/SKILL.md b/.agents/skills/harness-adapters/SKILL.md index c59ed2eb53..5d8bf449aa 100644 --- a/.agents/skills/harness-adapters/SKILL.md +++ b/.agents/skills/harness-adapters/SKILL.md @@ -27,6 +27,7 @@ Inheritance also copies the literal `config/crew-dispatch.json` file, so secondm Each adapter splits into mechanics and knowledge. The per-task mechanics, including launch command, autonomy flag, and crewmate turn-end hook, live in `bin/fm-spawn.sh`. The primary-session "no turn ends blind" guard contract and harness hook installation paths live in `docs/turnend-guard.md`. +Per-agent browser isolation is spawn-owned and harness-neutral; `docs/browser-axi-isolation.md` owns the mechanism and teardown cleanup contract. The primary-session watcher wake protocols are rendered from `docs/supervision-protocols/` by `bin/fm-supervision-instructions.sh`. The supervision knowledge lives here: busy signature, exit command, interrupt, dialogs, resume behavior, skill invocation, and quirks. diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 40d70a3fda..151b0a7d7e 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -50,6 +50,11 @@ # into the secondmate home's config/, so the secondmate's OWN crewmates, # dispatch profiles, and backlog backend inherit the primary's settings # (fm-config-inherit-lib.sh). +# Every spawn also gets a per-task chrome-devtools-axi browser identity: +# CHROME_DEVTOOLS_AXI_SESSION, CHROME_DEVTOOLS_AXI_PORT, and, by default, +# CHROME_DEVTOOLS_AXI_USER_DATA_DIR under $HOME/.fm-browser-profiles/. +# Set FM_BROWSER_AXI_PROFILE_MODE=ephemeral for a clean AXI isolated browser +# instead of a persistent per-task profile. # --scout records kind=scout in the task's meta (report deliverable, scratch worktree; # see AGENTS.md task lifecycle); --secondmate records kind=secondmate and launches in a # provisioned firstmate home; the default is kind=ship. @@ -475,6 +480,115 @@ json_escape() { printf '%s' "$1" | sed 's/\\/\\\\/g; s/"/\\"/g' } +browser_axi_safe_id() { + local raw=$1 safe + safe=$(printf '%s' "$raw" | LC_ALL=C sed 's/[^A-Za-z0-9._-]/-/g; s/--*/-/g; s/^-//; s/-$//') + [ -n "$safe" ] || safe=worker + printf '%s\n' "$safe" +} + +browser_axi_home_hash() { + local home_real hash + if home_real=$(cd "$FM_HOME" 2>/dev/null && pwd -P); then + : + else + home_real=$FM_HOME + fi + hash=$(printf '%s' "$home_real" | cksum | awk '{print $1}') + printf '%s\n' "$hash" +} + +browser_axi_port_is_free() { + local port=$1 + if command -v lsof >/dev/null 2>&1; then + if lsof -nP -iTCP:"$port" -sTCP:LISTEN >/dev/null 2>&1; then + return 1 + fi + return 0 + fi + if command -v nc >/dev/null 2>&1; then + if nc -z 127.0.0.1 "$port" >/dev/null 2>&1; then + return 1 + fi + return 0 + fi + return 2 +} + +browser_axi_alloc_port() { + local session=$1 start=19000 end=20999 count seed offset i port + if ! command -v lsof >/dev/null 2>&1 && ! command -v nc >/dev/null 2>&1; then + echo "error: cannot allocate chrome-devtools-axi port: need lsof or nc on PATH" >&2 + return 1 + fi + count=$((end - start + 1)) + seed=$(printf '%s' "$session" | cksum | awk '{print $1}') + offset=$((seed % count)) + i=0 + while [ "$i" -lt "$count" ]; do + port=$((start + ((offset + i) % count))) + if browser_axi_port_is_free "$port"; then + printf '%s\n' "$port" + return 0 + fi + i=$((i + 1)) + done + echo "error: no free chrome-devtools-axi port in 19000..20999 for $session" >&2 + return 1 +} + +browser_axi_profile_mode() { + local mode=${FM_BROWSER_AXI_PROFILE_MODE:-persistent} + case "$mode" in + ''|persistent) printf '%s\n' persistent ;; + ephemeral) printf '%s\n' ephemeral ;; + *) + echo "error: FM_BROWSER_AXI_PROFILE_MODE must be persistent or ephemeral, got '$mode'" >&2 + return 1 + ;; + esac +} + +browser_axi_resolve_mcp_path() { + local candidate npm_prefix + candidate=${CHROME_DEVTOOLS_AXI_MCP_PATH:-} + if [ -n "$candidate" ] && [ -f "$candidate" ]; then + printf '%s\n' "$candidate" + return 0 + fi + if command -v npm >/dev/null 2>&1; then + npm_prefix=$(npm prefix -g 2>/dev/null || true) + if [ -n "$npm_prefix" ]; then + candidate="$npm_prefix/lib/node_modules/chrome-devtools-mcp/build/src/bin/chrome-devtools-mcp.js" + if [ -f "$candidate" ]; then + printf '%s\n' "$candidate" + return 0 + fi + fi + fi + if [ -n "${HOME:-}" ] && [ -d "$HOME/.npm/_npx" ]; then + candidate=$(find "$HOME/.npm/_npx" -path '*/node_modules/chrome-devtools-mcp/build/src/bin/chrome-devtools-mcp.js' -type f 2>/dev/null | sort | tail -1) + if [ -n "$candidate" ]; then + printf '%s\n' "$candidate" + return 0 + fi + fi + return 0 +} + +browser_axi_env_prefix() { + printf '%s' 'env -u CHROME_DEVTOOLS_AXI_AUTO_CONNECT -u CHROME_DEVTOOLS_AXI_BROWSER_URL -u CHROME_DEVTOOLS_AXI_USER_DATA_DIR -u CHROME_DEVTOOLS_AXI_MCP_PATH' + printf ' CHROME_DEVTOOLS_AXI_SESSION=%s' "$(shell_quote "$BROWSER_AXI_SESSION")" + printf ' CHROME_DEVTOOLS_AXI_PORT=%s' "$(shell_quote "$BROWSER_AXI_PORT")" + printf ' CHROME_DEVTOOLS_AXI_BRIDGE_TIMEOUT_MS=%s' "$(shell_quote 60000)" + if [ -n "$BROWSER_AXI_USER_DATA_DIR" ]; then + printf ' CHROME_DEVTOOLS_AXI_USER_DATA_DIR=%s' "$(shell_quote "$BROWSER_AXI_USER_DATA_DIR")" + fi + if [ -n "$BROWSER_AXI_MCP_PATH" ]; then + printf ' CHROME_DEVTOOLS_AXI_MCP_PATH=%s' "$(shell_quote "$BROWSER_AXI_MCP_PATH")" + fi +} + resolved_existing_dir() { local path=$1 [ -d "$path" ] || { echo "error: firstmate home does not exist or is not a directory: $path" >&2; return 1; } @@ -651,6 +765,15 @@ fi # (docs/herdr-backend.md "Known gaps"). PROJ_ABS_REAL=$(cd "$PROJ_ABS" 2>/dev/null && pwd -P) || PROJ_ABS_REAL="$PROJ_ABS" +BROWSER_AXI_PROFILE_MODE=$(browser_axi_profile_mode) +BROWSER_AXI_SESSION=$(browser_axi_safe_id "fm-$(browser_axi_home_hash)-$ID") +BROWSER_AXI_PORT=$(browser_axi_alloc_port "$BROWSER_AXI_SESSION") +BROWSER_AXI_USER_DATA_DIR= +if [ "$BROWSER_AXI_PROFILE_MODE" = persistent ]; then + BROWSER_AXI_USER_DATA_DIR="${HOME:-$FM_HOME}/.fm-browser-profiles/$BROWSER_AXI_SESSION" +fi +BROWSER_AXI_MCP_PATH=$(browser_axi_resolve_mcp_path) + real_path_or_raw() { # local path=$1 real if real=$(cd "$path" 2>/dev/null && pwd -P); then @@ -978,6 +1101,11 @@ META_WINDOW=$T echo "tasktmp=$TASK_TMP" echo "model=${MODEL:-default}" echo "effort=${EFFORT:-default}" + echo "browser_axi_session=$BROWSER_AXI_SESSION" + echo "browser_axi_port=$BROWSER_AXI_PORT" + echo "browser_axi_profile_mode=$BROWSER_AXI_PROFILE_MODE" + [ -z "$BROWSER_AXI_USER_DATA_DIR" ] || echo "browser_axi_user_data_dir=$BROWSER_AXI_USER_DATA_DIR" + [ -z "$BROWSER_AXI_MCP_PATH" ] || echo "browser_axi_mcp_path=$BROWSER_AXI_MCP_PATH" # backend= is written only for a non-default (non-tmux) backend, so the # default path's meta stays byte-identical (absent backend= means tmux; # data/fm-backend-design-d7's P1 compatibility contract). @@ -1026,6 +1154,7 @@ if [ "$KIND" = secondmate ]; then sq_home=$(shell_quote "$PROJ_ABS") LAUNCH="FM_ROOT_OVERRIDE= FM_STATE_OVERRIDE= FM_DATA_OVERRIDE= FM_PROJECTS_OVERRIDE= FM_CONFIG_OVERRIDE= FM_HOME=$sq_home $LAUNCH" fi +LAUNCH="$(browser_axi_env_prefix) $LAUNCH" # Export GOTMPDIR into the crewmate's pane shell so the agent and every child # process (go build, go test, ...) inherit it. Sent before the launch command so # the env is set when the agent starts; the brief sleep lets the export land. diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index ea07283a1d..a76c3bf7da 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -128,6 +128,25 @@ meta_value() { fm_meta_get "$meta" "$key" } +stop_browser_axi_for_meta() { + local meta=$1 session port mcp env_args + session=$(meta_value "$meta" browser_axi_session) + [ -n "$session" ] || return 0 + if ! command -v chrome-devtools-axi >/dev/null 2>&1; then + echo "warning: chrome-devtools-axi not found; browser bridge for $session was not stopped" >&2 + return 0 + fi + port=$(meta_value "$meta" browser_axi_port) + mcp=$(meta_value "$meta" browser_axi_mcp_path) + env_args=(-u CHROME_DEVTOOLS_AXI_AUTO_CONNECT -u CHROME_DEVTOOLS_AXI_BROWSER_URL -u CHROME_DEVTOOLS_AXI_USER_DATA_DIR -u CHROME_DEVTOOLS_AXI_MCP_PATH) + env_args+=("CHROME_DEVTOOLS_AXI_SESSION=$session") + [ -z "$port" ] || env_args+=("CHROME_DEVTOOLS_AXI_PORT=$port") + [ -z "$mcp" ] || env_args+=("CHROME_DEVTOOLS_AXI_MCP_PATH=$mcp") + if ! env "${env_args[@]}" chrome-devtools-axi stop >/dev/null 2>&1; then + echo "warning: chrome-devtools-axi stop failed for $session; continuing teardown" >&2 + fi +} + require_orca_worktree_id() { local meta=$1 id id=$(meta_value "$meta" orca_worktree_id) @@ -875,6 +894,7 @@ cleanup_firstmate_home_children() { else child_t=$(fm_backend_target_of_meta "$child_meta") fi + stop_browser_axi_for_meta "$child_meta" if [ "$child_backend" = orca ] && [ "$child_kind" != secondmate ]; then child_orca_worktree_id=$(require_orca_worktree_id "$child_meta") || return 1 if [ -n "$child_wt" ] && [ -e "$child_wt" ]; then @@ -991,6 +1011,7 @@ if [ -d "$WT" ] && [ "$FORCE" != "--force" ]; then fi # Best-effort: drop the local task branch so the shared repo does not accumulate refs. +stop_browser_axi_for_meta "$META" if [ "$BACKEND" = orca ] && [ "$KIND" != secondmate ]; then if [ "$ORCA_PATH_MATCH_VERIFIED" != 1 ]; then require_orca_worktree_path_match_if_present "$ORCA_WORKTREE_ID" "$WT" || exit 1 diff --git a/docs/browser-axi-isolation.md b/docs/browser-axi-isolation.md new file mode 100644 index 0000000000..c6e8b1ae0a --- /dev/null +++ b/docs/browser-axi-isolation.md @@ -0,0 +1,28 @@ +# Browser AXI Isolation + +Date: 2026-07-10. + +`bin/fm-spawn.sh` provisions a browser automation identity for every ship, scout, and secondmate launch. +The identity is harness-neutral, so Claude, Codex, OpenCode, Pi, and Grok receive the same browser environment contract. +The contract uses `chrome-devtools-axi` as the worker browser path. +Each worker receives a sanitized `CHROME_DEVTOOLS_AXI_SESSION` derived from the firstmate home fingerprint and the task id. +Each worker receives an explicit free localhost `CHROME_DEVTOOLS_AXI_PORT` from the reserved `19000..20999` range. +The explicit port avoids relying on AXI's named-session port hash, because AXI documents and prior testing reproduced hash collisions on a busy machine. +Persistent mode is the default. +Persistent mode sets `CHROME_DEVTOOLS_AXI_USER_DATA_DIR` to `$HOME/.fm-browser-profiles/`. +That profile directory gives a task its own cookies, localStorage, IndexedDB, service workers, and login state. +Set `FM_BROWSER_AXI_PROFILE_MODE=ephemeral` before spawning a task when the task should use AXI's clean-slate isolated browser mode. +Ephemeral mode omits `CHROME_DEVTOOLS_AXI_USER_DATA_DIR`, so AXI launches with its own isolated temporary browser behavior. +Spawn always unsets `CHROME_DEVTOOLS_AXI_AUTO_CONNECT` and `CHROME_DEVTOOLS_AXI_BROWSER_URL` for the worker launch. +That prevents firstmate workers from attaching to the captain's real Chrome session or a globally configured browser endpoint. +Spawn also clears any ambient `CHROME_DEVTOOLS_AXI_USER_DATA_DIR` before applying the task-specific profile decision. +When a usable `chrome-devtools-mcp` script path is discoverable, spawn exports `CHROME_DEVTOOLS_AXI_MCP_PATH` for the worker. +That avoids depending on cold `npx chrome-devtools-mcp@latest` startup during the first browser action. +If no usable MCP script path is discoverable, spawn leaves the path unset and lets AXI use its normal fallback. +Task metadata records `browser_axi_session`, `browser_axi_port`, `browser_axi_profile_mode`, and the persistent profile or MCP path when present. +`bin/fm-teardown.sh` reads those metadata fields and runs `chrome-devtools-axi stop` best-effort before removing the task endpoint. +Teardown does not delete persistent browser profiles. +Persistent profile retention is deliberate so a respawned task can keep its browser identity. +A profile directory should be removed only after its AXI bridge is stopped and no Chrome process still uses that path. +The mechanism was validated before implementation with concurrent persistent workers, persistent restart, and ephemeral clean-slate sessions. +The validation covered both Claude CLI and Codex CLI workers. diff --git a/docs/configuration.md b/docs/configuration.md index 2e5c7fd443..b51da22377 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -121,6 +121,7 @@ Launch mechanics, including the verified command templates, live in [`bin/fm-spa Primary-session turn-end guard integrations for verified harnesses are tracked as repo-level hook files and documented in [`docs/turnend-guard.md`](turnend-guard.md). Primary-session watcher wake protocols are rendered at session start by [`bin/fm-supervision-instructions.sh`](../bin/fm-supervision-instructions.sh) from [`docs/supervision-protocols/`](supervision-protocols/). Claude and Grok use background-notify cycles, Codex uses bounded foreground checkpoints, Pi uses its generated primary watcher extension, and OpenCode uses its TUI plugin. +Per-agent browser isolation is provisioned by `fm-spawn.sh` for every firstmate-launched worker, and [`docs/browser-axi-isolation.md`](browser-axi-isolation.md) owns the browser identity contract. `config/crew-harness` is a local, gitignored file containing one adapter name for crewmate and scout launches. When it is absent or contains `default`, crewmates mirror the firstmate's own harness. `config/secondmate-harness` is a separate local, gitignored file containing the adapter the primary uses to launch secondmate agents, optionally followed by model and effort tokens on the same line. @@ -244,6 +245,7 @@ FM_DATA_OVERRIDE= # alternate data dir, mainly for tests FM_PROJECTS_OVERRIDE= # alternate projects dir, mainly for tests FM_CONFIG_OVERRIDE= # alternate config dir, mainly for tests FM_BACKEND= # optional runtime backend override for new spawns; tmux/herdr/zellij/orca/cmux support ship/scout spawns, codex-app is not accepted +FM_BROWSER_AXI_PROFILE_MODE=persistent # persistent uses $HOME/.fm-browser-profiles/; ephemeral omits CHROME_DEVTOOLS_AXI_USER_DATA_DIR HERDR_SESSION=default # herdr-only: named session for normal backend ops; not enough for destructive cleanup (docs/herdr-backend.md) FM_BACKEND_HERDR_COMPOSER_LINES=20 # herdr-only: tail lines scanned by composer-state guard/fallback paths; idle-baseline submit confirmation uses agent-state FM_BACKEND_HERDR_IDLE_RE='^Type a message\.\.\.$' # herdr-only: empty-composer placeholder regex after ANSI, border, and prompt stripping diff --git a/tests/fm-browser-axi-isolation.test.sh b/tests/fm-browser-axi-isolation.test.sh new file mode 100644 index 0000000000..cbdcf69109 --- /dev/null +++ b/tests/fm-browser-axi-isolation.test.sh @@ -0,0 +1,260 @@ +#!/usr/bin/env bash +# Tests for fm-spawn.sh's per-agent chrome-devtools-axi browser isolation. +# +# The tests use fake tmux/lsof/npm/chrome-devtools-axi shims so they assert the +# exact launch and teardown contracts without launching a real browser. +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +BASE_PATH=${FM_TEST_BASE_PATH:-/usr/bin:/bin:/usr/sbin:/sbin} +TMP_ROOT=$(fm_test_tmproot fm-browser-axi-isolation) +export FM_BACKEND=tmux + +make_browser_fakebin() { + local dir=$1 fakebin="$1/fakebin" + mkdir -p "$fakebin" + cat > "$fakebin/tmux" <<'SH' +#!/usr/bin/env bash +set -u +case "$*" in + *"#{pane_current_path}"*) printf '%s\n' "${FM_FAKE_PANE_PATH:-}"; exit 0 ;; +esac +case "${1:-}" in + display-message) printf 'firstmate\n'; exit 0 ;; + list-windows) exit 0 ;; + has-session|new-session|new-window|kill-window) exit 0 ;; + capture-pane) cat "${FM_FAKE_TMUX_CAPTURE:-/dev/null}"; exit 0 ;; + send-keys) + if [ -n "${FM_FAKE_LAUNCH_LOG:-}" ]; then + prev= + for a in "$@"; do + if [ "$prev" = "-l" ]; then + printf '%s\n' "$a" >> "$FM_FAKE_LAUNCH_LOG" + fi + prev=$a + done + fi + exit 0 + ;; +esac +exit 0 +SH + cat > "$fakebin/lsof" <<'SH' +#!/usr/bin/env bash +exit 1 +SH + cat > "$fakebin/npm" <<'SH' +#!/usr/bin/env bash +set -u +if [ "${1:-}" = prefix ] && [ "${2:-}" = -g ]; then + printf '%s\n' "$FM_FAKE_NPM_PREFIX" + exit 0 +fi +exit 1 +SH + cat > "$fakebin/chrome-devtools-axi" <<'SH' +#!/usr/bin/env bash +set -u +printf 'cmd=%s session=%s port=%s mcp=%s auto=%s browser=%s userdir=%s\n' \ + "${1:-}" \ + "${CHROME_DEVTOOLS_AXI_SESSION:-}" \ + "${CHROME_DEVTOOLS_AXI_PORT:-}" \ + "${CHROME_DEVTOOLS_AXI_MCP_PATH:-}" \ + "${CHROME_DEVTOOLS_AXI_AUTO_CONNECT:-}" \ + "${CHROME_DEVTOOLS_AXI_BROWSER_URL:-}" \ + "${CHROME_DEVTOOLS_AXI_USER_DATA_DIR:-}" >> "$FM_FAKE_AXI_LOG" +[ "${1:-}" = stop ] || exit 2 +exit 0 +SH + chmod +x "$fakebin/tmux" "$fakebin/lsof" "$fakebin/npm" "$fakebin/chrome-devtools-axi" + printf '%s\n' "$fakebin" +} + +make_spawn_world() { + local name=$1 w fakebin npm_prefix mcp + w="$TMP_ROOT/$name" + fakebin=$(make_browser_fakebin "$w") + npm_prefix="$w/npm-global" + mcp="$npm_prefix/lib/node_modules/chrome-devtools-mcp/build/src/bin/chrome-devtools-mcp.js" + mkdir -p "$(dirname "$mcp")" "$w/home/state" "$w/home/data" "$w/home/projects" "$w/user-home" + printf 'mcp\n' > "$mcp" + printf '%s\n%s\n%s\n%s\n' "$w" "$fakebin" "$npm_prefix" "$mcp" +} + +spawn_task_capture() { + local w=$1 fakebin=$2 npm_prefix=$3 id=$4 mode=${5:-persistent} proj wt launchlog out status + proj="$w/project-$id" + wt="$w/worktree-$id" + launchlog="$w/launch-$id.log" + fm_git_worktree "$proj" "$wt" "wt-$id" + mkdir -p "$w/home/data/$id" + printf 'brief for %s\n' "$id" > "$w/home/data/$id/brief.md" + : > "$launchlog" + out=$(PATH="$fakebin:$BASE_PATH" HOME="$w/user-home" TMUX="fake,1,0" \ + FM_FAKE_PANE_PATH="$wt" FM_FAKE_LAUNCH_LOG="$launchlog" FM_FAKE_NPM_PREFIX="$npm_prefix" \ + FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$w/home" \ + FM_STATE_OVERRIDE="$w/home/state" FM_DATA_OVERRIDE="$w/home/data" \ + FM_PROJECTS_OVERRIDE="$w/home/projects" FM_CONFIG_OVERRIDE="$w/home/config" \ + FM_BROWSER_AXI_PROFILE_MODE="$mode" FM_SPAWN_NO_GUARD=1 \ + "$ROOT/bin/fm-spawn.sh" "$id" "$proj" codex 2>&1) + status=$? + expect_code 0 "$status" "spawn $id should succeed: $out" + printf '%s\n' "$launchlog" +} + +meta_field() { + grep "^$2=" "$1" 2>/dev/null | tail -1 | cut -d= -f2- +} + +assert_port_in_spawn_range() { + local port=$1 label=$2 + case "$port" in + ''|*[!0-9]*) fail "$label: port is not numeric: $port" ;; + esac + [ "$port" -ge 19000 ] && [ "$port" -le 20999 ] || fail "$label: port $port is outside 19000..20999" +} + +test_persistent_spawn_wires_axi_env_and_meta() { + local w fakebin npm_prefix mcp launchlog launch meta session port profile + { + IFS= read -r w + IFS= read -r fakebin + IFS= read -r npm_prefix + IFS= read -r mcp + } < "$log" + mkdir -p "$state" + fm_write_meta "$meta" \ + "window=firstmate:fm-$id" \ + "worktree=$w/missing-worktree" \ + "project=$w/missing-project" \ + "harness=codex" \ + "kind=scout" \ + "mode=no-mistakes" \ + "yolo=off" \ + "tasktmp=$w/tasktmp" \ + "browser_axi_session=fm-test-$id" \ + "browser_axi_port=19399" \ + "browser_axi_profile_mode=persistent" \ + "browser_axi_user_data_dir=$w/user-home/.fm-browser-profiles/fm-test-$id" \ + "browser_axi_mcp_path=$mcp" + + out=$(PATH="$fakebin:$BASE_PATH" HOME="$w/user-home" FM_FAKE_AXI_LOG="$log" \ + FM_FAKE_TMUX_CAPTURE="$w/capture" FM_FAKE_NPM_PREFIX="$npm_prefix" \ + FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$w/home" \ + FM_STATE_OVERRIDE="$state" FM_DATA_OVERRIDE="$w/home/data" \ + FM_CONFIG_OVERRIDE="$w/home/config" FM_SPAWN_NO_GUARD=1 \ + "$ROOT/bin/fm-teardown.sh" "$id" --force 2>&1) + status=$? + expect_code 0 "$status" "teardown should succeed: $out" + assert_contains "$(cat "$log")" "cmd=stop session=fm-test-$id port=19399 mcp=$mcp auto= browser= userdir=" \ + "teardown did not stop AXI with the recorded isolated session and sanitized env" + [ ! -e "$meta" ] || fail "teardown: meta was not removed" + pass "teardown stops the recorded chrome-devtools-axi bridge before clearing task state" +} + +test_persistent_spawn_wires_axi_env_and_meta +test_distinct_tasks_get_distinct_axi_identities +test_ephemeral_spawn_omits_user_data_dir +test_teardown_stops_recorded_axi_bridge diff --git a/tests/fm-spawn-dispatch-profile.test.sh b/tests/fm-spawn-dispatch-profile.test.sh index 5b0bad7a38..0874fcc3a5 100755 --- a/tests/fm-spawn-dispatch-profile.test.sh +++ b/tests/fm-spawn-dispatch-profile.test.sh @@ -42,6 +42,15 @@ esac exit 0 SH chmod +x "$fakebin/tmux" + cat > "$fakebin/lsof" <<'SH' +#!/usr/bin/env bash +exit 1 +SH + cat > "$fakebin/npm" <<'SH' +#!/usr/bin/env bash +exit 1 +SH + chmod +x "$fakebin/lsof" "$fakebin/npm" fm_fake_exit0 "$fakebin" treehouse printf '%s\n' "$fakebin" } @@ -55,7 +64,7 @@ make_spawn_case() { wt="$case_dir/wt" launchlog="$case_dir/launch.log" fakebin=$(make_spawn_fakebin "$case_dir/fake") - mkdir -p "$home/data" "$home/projects" "$home/state" "$home/config" + mkdir -p "$home/data" "$home/projects" "$home/state" "$home/config" "$case_dir/user-home" printf '%s\n' "$harness" > "$home/config/crew-harness" fm_git_worktree "$proj" "$wt" "wt-$name" touch "$home/state/.last-watcher-beat" @@ -84,7 +93,7 @@ run_spawn() { local home=$1 wt=$2 fakebin=$3 launchlog=$4 shift 4 : > "$launchlog" - FM_ROOT_OVERRIDE='' FM_HOME="$home" \ + HOME="$(dirname "$home")/user-home" FM_ROOT_OVERRIDE='' FM_HOME="$home" \ FM_STATE_OVERRIDE="$home/state" FM_DATA_OVERRIDE="$home/data" \ FM_PROJECTS_OVERRIDE="$home/projects" FM_CONFIG_OVERRIDE="$home/config" \ FM_SPAWN_NO_GUARD=1 FM_FAKE_PANE_PATH="$wt" TMUX="fake,1,0" \ @@ -105,9 +114,28 @@ assert_meta_profile() { assert_grep "effort=$effort" "$meta" "meta missing effort=$effort" } -test_no_profile_keeps_claude_launch_unchanged() { - local rec id out status expected launch - id=profile-off-z1 +meta_field() { + grep "^$2=" "$1" 2>/dev/null | tail -1 | cut -d= -f2- +} + +assert_browser_axi_wrapper() { + local launch=$1 meta=$2 label=$3 session port profile + session=$(meta_field "$meta" browser_axi_session) + port=$(meta_field "$meta" browser_axi_port) + profile=$(meta_field "$meta" browser_axi_user_data_dir) + [ -n "$session" ] || fail "$label: meta missing browser_axi_session" + [ -n "$port" ] || fail "$label: meta missing browser_axi_port" + [ -n "$profile" ] || fail "$label: meta missing browser_axi_user_data_dir" + assert_contains "$launch" "env -u CHROME_DEVTOOLS_AXI_AUTO_CONNECT -u CHROME_DEVTOOLS_AXI_BROWSER_URL -u CHROME_DEVTOOLS_AXI_USER_DATA_DIR -u CHROME_DEVTOOLS_AXI_MCP_PATH" \ + "$label: launch missing AXI env sanitizer" + assert_contains "$launch" "CHROME_DEVTOOLS_AXI_SESSION='$session'" "$label: launch missing AXI session" + assert_contains "$launch" "CHROME_DEVTOOLS_AXI_PORT='$port'" "$label: launch missing AXI port" + assert_contains "$launch" "CHROME_DEVTOOLS_AXI_USER_DATA_DIR='$profile'" "$label: launch missing AXI profile" +} + +test_no_profile_keeps_claude_harness_launch_suffix() { + local rec id out status expected launch meta + id='profile-off-z1' rec=$(make_spawn_case profile-off claude "$id") read_case_record "$rec" @@ -115,17 +143,19 @@ test_no_profile_keeps_claude_launch_unchanged() { status=$? expect_code 0 "$status" "claude spawn without profile flags should succeed" assert_contains "$out" "spawned $id harness=claude" "spawn did not report claude" - assert_meta_profile "$HOME_DIR/state/$id.meta" claude default default + meta="$HOME_DIR/state/$id.meta" + assert_meta_profile "$meta" claude default default launch=$(cat "$LAUNCH_LOG") + assert_browser_axi_wrapper "$launch" "$meta" "no-profile" expected="CLAUDE_CODE_ENABLE_PROMPT_SUGGESTION=false claude --dangerously-skip-permissions \"\$(cat '$HOME_DIR/data/$id/brief.md')\"" - [ "$launch" = "$expected" ] || fail "no-profile claude launch changed"$'\n'"expected: $expected"$'\n'"actual: $launch" - pass "no --model/--effort records defaults and keeps the claude launch byte-identical" + assert_contains "$launch" "$expected" "no-profile claude harness launch changed" + pass "no --model/--effort records defaults and keeps the claude harness launch suffix unchanged" } test_active_dispatch_profile_requires_explicit_harness_for_ship() { local rec id out status - id=profile-required-ship-z11 + id='profile-required-ship-z11' rec=$(make_spawn_case profile-required-ship claude "$id") read_case_record "$rec" enable_dispatch_profile "$HOME_DIR" @@ -141,7 +171,7 @@ test_active_dispatch_profile_requires_explicit_harness_for_ship() { test_active_dispatch_profile_requires_explicit_harness_for_scout() { local rec id out status - id=profile-required-scout-z12 + id='profile-required-scout-z12' rec=$(make_spawn_case profile-required-scout claude "$id") read_case_record "$rec" enable_dispatch_profile "$HOME_DIR" @@ -156,8 +186,8 @@ test_active_dispatch_profile_requires_explicit_harness_for_scout() { } test_active_dispatch_profile_allows_explicit_harness() { - local rec id out status launch - id=profile-explicit-z13 + local rec id out status launch meta + id='profile-explicit-z13' rec=$(make_spawn_case profile-explicit claude "$id") read_case_record "$rec" enable_dispatch_profile "$HOME_DIR" @@ -167,8 +197,10 @@ test_active_dispatch_profile_allows_explicit_harness() { status=$? expect_code 0 "$status" "explicit harness should satisfy active dispatch-profile requirement" assert_contains "$out" "spawned $id harness=codex" "spawn did not report explicit codex harness" - assert_meta_profile "$HOME_DIR/state/$id.meta" codex gpt-5 high + meta="$HOME_DIR/state/$id.meta" + assert_meta_profile "$meta" codex gpt-5 high launch=$(cat "$LAUNCH_LOG") + assert_browser_axi_wrapper "$launch" "$meta" "explicit harness" assert_contains "$launch" "codex --model 'gpt-5' -c 'model_reasoning_effort=\"high\"' --dangerously-bypass-approvals-and-sandbox" \ "explicit harness launch did not thread model and effort" pass "active crew-dispatch profile allows an explicit resolved harness" @@ -176,7 +208,7 @@ test_active_dispatch_profile_allows_explicit_harness() { test_active_dispatch_profile_allows_positional_harness() { local rec id out status - id=profile-positional-z14 + id='profile-positional-z14' rec=$(make_spawn_case profile-positional claude "$id") read_case_record "$rec" enable_dispatch_profile "$HOME_DIR" @@ -191,8 +223,8 @@ test_active_dispatch_profile_allows_positional_harness() { } test_active_dispatch_profile_allows_raw_launch_command() { - local rec id out status launch - id=profile-raw-z15 + local rec id out status launch meta + id='profile-raw-z15' rec=$(make_spawn_case profile-raw claude "$id") read_case_record "$rec" enable_dispatch_profile "$HOME_DIR" @@ -202,55 +234,63 @@ test_active_dispatch_profile_allows_raw_launch_command() { status=$? expect_code 0 "$status" "raw launch command should satisfy active dispatch-profile requirement" assert_contains "$out" "spawned $id harness=custom-agent" "spawn did not report raw command harness" - assert_meta_profile "$HOME_DIR/state/$id.meta" custom-agent default default + meta="$HOME_DIR/state/$id.meta" + assert_meta_profile "$meta" custom-agent default default launch=$(cat "$LAUNCH_LOG") - [ "$launch" = "custom-agent --flag" ] || fail "raw launch command changed"$'\n'"actual: $launch" + assert_browser_axi_wrapper "$launch" "$meta" "raw launch" + assert_contains "$launch" "custom-agent --flag" "raw launch command changed" pass "active crew-dispatch profile allows the raw launch-command escape hatch" } test_claude_threads_model_and_effort() { - local rec id out status launch - id=profile-claude-z2 + local rec id out status launch meta + id='profile-claude-z2' rec=$(make_spawn_case profile-claude claude "$id") read_case_record "$rec" out=$(run_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR" --model sonnet --effort high) status=$? expect_code 0 "$status" "claude spawn with profile flags should succeed" - assert_meta_profile "$HOME_DIR/state/$id.meta" claude sonnet high + meta="$HOME_DIR/state/$id.meta" + assert_meta_profile "$meta" claude sonnet high launch=$(cat "$LAUNCH_LOG") + assert_browser_axi_wrapper "$launch" "$meta" "claude profile" assert_contains "$launch" "claude --dangerously-skip-permissions --model 'sonnet' --effort 'high'" \ "claude launch did not thread model and effort flags" pass "claude receives --model and --effort profile flags" } test_codex_threads_model_and_effort() { - local rec id out status launch - id=profile-codex-z3 + local rec id out status launch meta + id='profile-codex-z3' rec=$(make_spawn_case profile-codex codex "$id") read_case_record "$rec" out=$(run_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR" --model gpt-5 --effort high) status=$? expect_code 0 "$status" "codex spawn with profile flags should succeed" - assert_meta_profile "$HOME_DIR/state/$id.meta" codex gpt-5 high + meta="$HOME_DIR/state/$id.meta" + assert_meta_profile "$meta" codex gpt-5 high launch=$(cat "$LAUNCH_LOG") + assert_browser_axi_wrapper "$launch" "$meta" "codex profile" assert_contains "$launch" "codex --model 'gpt-5' -c 'model_reasoning_effort=\"high\"' --dangerously-bypass-approvals-and-sandbox" \ "codex launch did not thread model and reasoning effort config" pass "codex receives --model and model_reasoning_effort profile flags" } test_codex_omits_invalid_max_effort() { - local rec id out status launch - id=profile-codex-max-z4 + local rec id out status launch meta + id='profile-codex-max-z4' rec=$(make_spawn_case profile-codex-max codex "$id") read_case_record "$rec" out=$(run_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR" --model gpt-5 --effort max) status=$? expect_code 0 "$status" "codex spawn with unsupported max effort should omit the effort flag" - assert_meta_profile "$HOME_DIR/state/$id.meta" codex gpt-5 max + meta="$HOME_DIR/state/$id.meta" + assert_meta_profile "$meta" codex gpt-5 max launch=$(cat "$LAUNCH_LOG") + assert_browser_axi_wrapper "$launch" "$meta" "codex max" assert_contains "$launch" "codex --model 'gpt-5' --dangerously-bypass-approvals-and-sandbox" \ "codex launch did not preserve the model flag when max effort was omitted" assert_not_contains "$launch" "model_reasoning_effort" "codex launch must omit unsupported max reasoning effort" @@ -258,16 +298,18 @@ test_codex_omits_invalid_max_effort() { } test_grok_threads_model_and_reasoning_effort() { - local rec id out status launch - id=profile-grok-z5 + local rec id out status launch meta + id='profile-grok-z5' rec=$(make_spawn_case profile-grok grok "$id") read_case_record "$rec" out=$(run_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR" --model grok-4 --effort high) status=$? expect_code 0 "$status" "grok spawn with profile flags should succeed" - assert_meta_profile "$HOME_DIR/state/$id.meta" grok grok-4 high + meta="$HOME_DIR/state/$id.meta" + assert_meta_profile "$meta" grok grok-4 high launch=$(cat "$LAUNCH_LOG") + assert_browser_axi_wrapper "$launch" "$meta" "grok profile" assert_contains "$launch" "grok --always-approve --model 'grok-4' --reasoning-effort 'high'" \ "grok launch did not thread model and reasoning-effort flags" assert_not_contains "$launch" "--effort" "grok launch must use --reasoning-effort, not --effort" @@ -275,16 +317,18 @@ test_grok_threads_model_and_reasoning_effort() { } test_grok_omits_invalid_max_reasoning_effort() { - local rec id out status launch - id=profile-grok-max-z6 + local rec id out status launch meta + id='profile-grok-max-z6' rec=$(make_spawn_case profile-grok-max grok "$id") read_case_record "$rec" out=$(run_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR" --model grok-4 --effort max) status=$? expect_code 0 "$status" "grok spawn with unsupported max reasoning effort should omit the effort flag" - assert_meta_profile "$HOME_DIR/state/$id.meta" grok grok-4 max + meta="$HOME_DIR/state/$id.meta" + assert_meta_profile "$meta" grok grok-4 max launch=$(cat "$LAUNCH_LOG") + assert_browser_axi_wrapper "$launch" "$meta" "grok max" assert_contains "$launch" "grok --always-approve --model 'grok-4' \"\$(cat " \ "grok launch did not preserve the model flag when max effort was omitted" assert_not_contains "$launch" "--reasoning-effort" "grok launch must omit unsupported max reasoning effort" @@ -293,16 +337,18 @@ test_grok_omits_invalid_max_reasoning_effort() { } test_opencode_threads_model_and_ignores_effort_axis() { - local rec id out status launch - id=profile-opencode-z7 + local rec id out status launch meta + id='profile-opencode-z7' rec=$(make_spawn_case profile-opencode opencode "$id") read_case_record "$rec" out=$(run_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR" --model anthropic/claude-sonnet-4-5 --effort high) status=$? expect_code 0 "$status" "opencode spawn with model and ignored effort should succeed" - assert_meta_profile "$HOME_DIR/state/$id.meta" opencode anthropic/claude-sonnet-4-5 high + meta="$HOME_DIR/state/$id.meta" + assert_meta_profile "$meta" opencode anthropic/claude-sonnet-4-5 high launch=$(cat "$LAUNCH_LOG") + assert_browser_axi_wrapper "$launch" "$meta" "opencode profile" assert_contains "$launch" "opencode --model 'anthropic/claude-sonnet-4-5' --prompt" \ "opencode launch did not thread model" assert_not_contains "$launch" "--effort" "opencode launch must not pass unsupported --effort" @@ -312,16 +358,18 @@ test_opencode_threads_model_and_ignores_effort_axis() { } test_pi_omits_invalid_max_effort() { - local rec id out status launch - id=profile-pi-z8 + local rec id out status launch meta + id='profile-pi-z8' rec=$(make_spawn_case profile-pi pi "$id") read_case_record "$rec" out=$(run_spawn "$HOME_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$LAUNCH_LOG" "$id" "$PROJ_DIR" --model sonnet --effort max) status=$? expect_code 0 "$status" "pi spawn with max effort should not pass an invalid flag" - assert_meta_profile "$HOME_DIR/state/$id.meta" pi sonnet max + meta="$HOME_DIR/state/$id.meta" + assert_meta_profile "$meta" pi sonnet max launch=$(cat "$LAUNCH_LOG") + assert_browser_axi_wrapper "$launch" "$meta" "pi max" assert_contains "$launch" "pi --model 'sonnet' -e" "pi launch did not thread model" assert_not_contains "$launch" "--thinking" "pi launch must omit --thinking max because the CLI rejects it" pass "pi threads model and omits unsupported max effort" @@ -329,8 +377,8 @@ test_pi_omits_invalid_max_effort() { test_batch_forwards_shared_profile_flags() { local rec id1 id2 out status - id1=profile-batch-a-z9 - id2=profile-batch-b-z10 + id1='profile-batch-a-z9' + id2='profile-batch-b-z10' rec=$(make_spawn_case profile-batch claude "$id1" "$id2") read_case_record "$rec" enable_dispatch_profile "$HOME_DIR" @@ -348,7 +396,7 @@ test_batch_forwards_shared_profile_flags() { test_active_dispatch_profile_does_not_block_secondmate_launch() { local rec id sm out status - id=profile-secondmate-z16 + id='profile-secondmate-z16' rec=$(make_spawn_case profile-secondmate codex "$id") read_case_record "$rec" enable_dispatch_profile "$HOME_DIR" @@ -364,7 +412,7 @@ test_active_dispatch_profile_does_not_block_secondmate_launch() { pass "active crew-dispatch profile does not block secondmate launches" } -test_no_profile_keeps_claude_launch_unchanged +test_no_profile_keeps_claude_harness_launch_suffix test_active_dispatch_profile_requires_explicit_harness_for_ship test_active_dispatch_profile_requires_explicit_harness_for_scout test_active_dispatch_profile_allows_explicit_harness From 7d08b4ccaa8f7245b752c235d0b7ba9c7e74ce02 Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Thu, 9 Jul 2026 22:58:44 -0400 Subject: [PATCH 07/42] fix: harden browser isolation lifecycle --- bin/fm-spawn.sh | 19 +++++++----- bin/fm-teardown.sh | 43 ++++++++++++++++++++++++++ docs/browser-axi-isolation.md | 11 ++++--- tests/fm-browser-axi-isolation.test.sh | 33 ++++++++++++++++---- 4 files changed, 87 insertions(+), 19 deletions(-) diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 151b0a7d7e..b883bb0b50 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -51,8 +51,9 @@ # dispatch profiles, and backlog backend inherit the primary's settings # (fm-config-inherit-lib.sh). # Every spawn also gets a per-task chrome-devtools-axi browser identity: -# CHROME_DEVTOOLS_AXI_SESSION, CHROME_DEVTOOLS_AXI_PORT, and, by default, -# CHROME_DEVTOOLS_AXI_USER_DATA_DIR under $HOME/.fm-browser-profiles/. +# CHROME_DEVTOOLS_AXI_SESSION, an explicit CHROME_DEVTOOLS_AXI_PORT when a +# local port probe can find one, and, by default, CHROME_DEVTOOLS_AXI_USER_DATA_DIR +# under $HOME/.fm-browser-profiles/. # Set FM_BROWSER_AXI_PROFILE_MODE=ephemeral for a clean AXI isolated browser # instead of a persistent per-task profile. # --scout records kind=scout in the task's meta (report deliverable, scratch worktree; @@ -518,8 +519,8 @@ browser_axi_port_is_free() { browser_axi_alloc_port() { local session=$1 start=19000 end=20999 count seed offset i port if ! command -v lsof >/dev/null 2>&1 && ! command -v nc >/dev/null 2>&1; then - echo "error: cannot allocate chrome-devtools-axi port: need lsof or nc on PATH" >&2 - return 1 + echo "warning: cannot probe chrome-devtools-axi ports without lsof or nc; leaving CHROME_DEVTOOLS_AXI_PORT unset for AXI's session-hash fallback" >&2 + return 0 fi count=$((end - start + 1)) seed=$(printf '%s' "$session" | cksum | awk '{print $1}') @@ -533,8 +534,8 @@ browser_axi_alloc_port() { fi i=$((i + 1)) done - echo "error: no free chrome-devtools-axi port in 19000..20999 for $session" >&2 - return 1 + echo "warning: no free chrome-devtools-axi port in 19000..20999 for $session; leaving CHROME_DEVTOOLS_AXI_PORT unset for AXI's session-hash fallback" >&2 + return 0 } browser_axi_profile_mode() { @@ -579,7 +580,9 @@ browser_axi_resolve_mcp_path() { browser_axi_env_prefix() { printf '%s' 'env -u CHROME_DEVTOOLS_AXI_AUTO_CONNECT -u CHROME_DEVTOOLS_AXI_BROWSER_URL -u CHROME_DEVTOOLS_AXI_USER_DATA_DIR -u CHROME_DEVTOOLS_AXI_MCP_PATH' printf ' CHROME_DEVTOOLS_AXI_SESSION=%s' "$(shell_quote "$BROWSER_AXI_SESSION")" - printf ' CHROME_DEVTOOLS_AXI_PORT=%s' "$(shell_quote "$BROWSER_AXI_PORT")" + if [ -n "$BROWSER_AXI_PORT" ]; then + printf ' CHROME_DEVTOOLS_AXI_PORT=%s' "$(shell_quote "$BROWSER_AXI_PORT")" + fi printf ' CHROME_DEVTOOLS_AXI_BRIDGE_TIMEOUT_MS=%s' "$(shell_quote 60000)" if [ -n "$BROWSER_AXI_USER_DATA_DIR" ]; then printf ' CHROME_DEVTOOLS_AXI_USER_DATA_DIR=%s' "$(shell_quote "$BROWSER_AXI_USER_DATA_DIR")" @@ -1102,7 +1105,7 @@ META_WINDOW=$T echo "model=${MODEL:-default}" echo "effort=${EFFORT:-default}" echo "browser_axi_session=$BROWSER_AXI_SESSION" - echo "browser_axi_port=$BROWSER_AXI_PORT" + [ -z "$BROWSER_AXI_PORT" ] || echo "browser_axi_port=$BROWSER_AXI_PORT" echo "browser_axi_profile_mode=$BROWSER_AXI_PROFILE_MODE" [ -z "$BROWSER_AXI_USER_DATA_DIR" ] || echo "browser_axi_user_data_dir=$BROWSER_AXI_USER_DATA_DIR" [ -z "$BROWSER_AXI_MCP_PATH" ] || echo "browser_axi_mcp_path=$BROWSER_AXI_MCP_PATH" diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index a76c3bf7da..fd81a15917 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -147,6 +147,47 @@ stop_browser_axi_for_meta() { fi } +browser_axi_profile_root_for_path() { + local profile=$1 + case "$profile" in + */.fm-browser-profiles/*) printf '%s/.fm-browser-profiles\n' "${profile%/.fm-browser-profiles/*}" ;; + *) return 1 ;; + esac +} + +remove_browser_axi_profile_for_meta() { + local meta=$1 mode profile root profile_abs root_abs + mode=$(meta_value "$meta" browser_axi_profile_mode) + [ "$mode" = persistent ] || return 0 + profile=$(meta_value "$meta" browser_axi_user_data_dir) + [ -n "$profile" ] || return 0 + [ -e "$profile" ] || return 0 + if ! root=$(browser_axi_profile_root_for_path "$profile"); then + echo "warning: refusing to remove browser profile outside .fm-browser-profiles: $profile" >&2 + return 0 + fi + if ! root_abs=$(cd "$root" 2>/dev/null && pwd -P); then + echo "warning: refusing to remove browser profile because root is not accessible: $root" >&2 + return 0 + fi + if ! profile_abs=$(cd "$profile" 2>/dev/null && pwd -P); then + echo "warning: refusing to remove browser profile because it is not an accessible directory: $profile" >&2 + return 0 + fi + case "$profile_abs" in + "$root_abs"/*) ;; + *) + echo "warning: refusing to remove browser profile outside resolved root: $profile" >&2 + return 0 + ;; + esac + [ "$profile_abs" != "$root_abs" ] || { + echo "warning: refusing to remove browser profile root itself: $profile" >&2 + return 0 + } + rm -rf "$profile_abs" +} + require_orca_worktree_id() { local meta=$1 id id=$(meta_value "$meta" orca_worktree_id) @@ -941,6 +982,7 @@ cleanup_firstmate_home_children() { fi fi remove_grok_turnend_auth "$sub_state" "$child_id" + remove_browser_axi_profile_for_meta "$child_meta" rm -f "$sub_state/$child_id.status" "$sub_state/$child_id.turn-ended" "$sub_state/$child_id.check.sh" "$sub_state/$child_id.meta" "$sub_state/$child_id.pi-ext.ts" "$sub_state/$child_id.grok-turnend-token" done } @@ -1060,6 +1102,7 @@ if [ "$KIND" = secondmate ]; then remove_secondmate_registry_entry "$ID" fi remove_grok_turnend_auth "$STATE" "$ID" +remove_browser_axi_profile_for_meta "$META" # Remove the per-task temp root (/tmp/fm-/, incl. its gotmp/) recorded by spawn. # Read before the state-file rm below; empty (pre-fix tasks without tasktmp=) is a no-op. [ -n "$TASK_TMP" ] && rm -rf "$TASK_TMP" diff --git a/docs/browser-axi-isolation.md b/docs/browser-axi-isolation.md index c6e8b1ae0a..e37cf6f23b 100644 --- a/docs/browser-axi-isolation.md +++ b/docs/browser-axi-isolation.md @@ -6,8 +6,9 @@ Date: 2026-07-10. The identity is harness-neutral, so Claude, Codex, OpenCode, Pi, and Grok receive the same browser environment contract. The contract uses `chrome-devtools-axi` as the worker browser path. Each worker receives a sanitized `CHROME_DEVTOOLS_AXI_SESSION` derived from the firstmate home fingerprint and the task id. -Each worker receives an explicit free localhost `CHROME_DEVTOOLS_AXI_PORT` from the reserved `19000..20999` range. +Each worker receives an explicit free localhost `CHROME_DEVTOOLS_AXI_PORT` from the reserved `19000..20999` range when a probe can find one with `lsof` or `nc`. The explicit port avoids relying on AXI's named-session port hash, because AXI documents and prior testing reproduced hash collisions on a busy machine. +If no probe tool is available or the reserved range is full, spawn leaves `CHROME_DEVTOOLS_AXI_PORT` unset, warns, and lets AXI use its session-hash fallback. Persistent mode is the default. Persistent mode sets `CHROME_DEVTOOLS_AXI_USER_DATA_DIR` to `$HOME/.fm-browser-profiles/`. That profile directory gives a task its own cookies, localStorage, IndexedDB, service workers, and login state. @@ -19,10 +20,10 @@ Spawn also clears any ambient `CHROME_DEVTOOLS_AXI_USER_DATA_DIR` before applyin When a usable `chrome-devtools-mcp` script path is discoverable, spawn exports `CHROME_DEVTOOLS_AXI_MCP_PATH` for the worker. That avoids depending on cold `npx chrome-devtools-mcp@latest` startup during the first browser action. If no usable MCP script path is discoverable, spawn leaves the path unset and lets AXI use its normal fallback. -Task metadata records `browser_axi_session`, `browser_axi_port`, `browser_axi_profile_mode`, and the persistent profile or MCP path when present. +Task metadata records `browser_axi_session`, `browser_axi_profile_mode`, and the persistent profile, explicit port, or MCP path when present. `bin/fm-teardown.sh` reads those metadata fields and runs `chrome-devtools-axi stop` best-effort before removing the task endpoint. -Teardown does not delete persistent browser profiles. -Persistent profile retention is deliberate so a respawned task can keep its browser identity. -A profile directory should be removed only after its AXI bridge is stopped and no Chrome process still uses that path. +Teardown removes the persistent browser profile directory recorded in task metadata after the AXI bridge has been stopped. +Persistent profile retention lasts only while the task remains live, so same-id recovery respawns keep their browser identity until final teardown. +Profile removal is guarded to paths under `.fm-browser-profiles`, so metadata cannot direct teardown to remove an arbitrary directory. The mechanism was validated before implementation with concurrent persistent workers, persistent restart, and ephemeral clean-slate sessions. The validation covered both Claude CLI and Codex CLI workers. diff --git a/tests/fm-browser-axi-isolation.test.sh b/tests/fm-browser-axi-isolation.test.sh index cbdcf69109..628be4e8ea 100644 --- a/tests/fm-browser-axi-isolation.test.sh +++ b/tests/fm-browser-axi-isolation.test.sh @@ -116,6 +116,24 @@ assert_port_in_spawn_range() { [ "$port" -ge 19000 ] && [ "$port" -le 20999 ] || fail "$label: port $port is outside 19000..20999" } +test_port_allocator_degrades_when_probe_tools_are_missing() { + local w no_probe err out status + w="$TMP_ROOT/no-probe" + no_probe="$w/path" + err="$w/err.log" + mkdir -p "$no_probe" + + eval "$(sed -n '/^browser_axi_port_is_free()/,/^browser_axi_profile_mode()/p' "$ROOT/bin/fm-spawn.sh" | sed '$d')" + out=$(PATH="$no_probe" browser_axi_alloc_port fm-no-probe-z9 2>"$err") + status=$? + + expect_code 0 "$status" "port allocator should not fail when lsof and nc are missing" + [ -z "$out" ] || fail "port allocator should leave explicit port unset without probe tools, got: $out" + assert_contains "$(cat "$err")" "warning: cannot probe chrome-devtools-axi ports without lsof or nc" \ + "port allocator did not warn about missing probe tools" + pass "port allocator degrades to AXI's fallback when probe tools are missing" +} + test_persistent_spawn_wires_axi_env_and_meta() { local w fakebin npm_prefix mcp launchlog launch meta session port profile { @@ -209,8 +227,8 @@ EOF pass "ephemeral spawn keeps the isolated AXI session and omits the persistent browser profile" } -test_teardown_stops_recorded_axi_bridge() { - local w fakebin npm_prefix mcp state id meta log out status +test_teardown_stops_recorded_axi_bridge_and_removes_profile() { + local w fakebin npm_prefix mcp state id meta log profile out status { IFS= read -r w IFS= read -r fakebin @@ -223,8 +241,9 @@ EOF id='browser-teardown-z4' log="$w/axi-stop.log" meta="$state/$id.meta" + profile="$w/user-home/.fm-browser-profiles/fm-test-$id" : > "$log" - mkdir -p "$state" + mkdir -p "$state" "$profile/Default" fm_write_meta "$meta" \ "window=firstmate:fm-$id" \ "worktree=$w/missing-worktree" \ @@ -237,7 +256,7 @@ EOF "browser_axi_session=fm-test-$id" \ "browser_axi_port=19399" \ "browser_axi_profile_mode=persistent" \ - "browser_axi_user_data_dir=$w/user-home/.fm-browser-profiles/fm-test-$id" \ + "browser_axi_user_data_dir=$profile" \ "browser_axi_mcp_path=$mcp" out=$(PATH="$fakebin:$BASE_PATH" HOME="$w/user-home" FM_FAKE_AXI_LOG="$log" \ @@ -250,11 +269,13 @@ EOF expect_code 0 "$status" "teardown should succeed: $out" assert_contains "$(cat "$log")" "cmd=stop session=fm-test-$id port=19399 mcp=$mcp auto= browser= userdir=" \ "teardown did not stop AXI with the recorded isolated session and sanitized env" + [ ! -e "$profile" ] || fail "teardown: persistent browser profile was not removed" [ ! -e "$meta" ] || fail "teardown: meta was not removed" - pass "teardown stops the recorded chrome-devtools-axi bridge before clearing task state" + pass "teardown stops the recorded chrome-devtools-axi bridge and removes its persistent profile" } +test_port_allocator_degrades_when_probe_tools_are_missing test_persistent_spawn_wires_axi_env_and_meta test_distinct_tasks_get_distinct_axi_identities test_ephemeral_spawn_omits_user_data_dir -test_teardown_stops_recorded_axi_bridge +test_teardown_stops_recorded_axi_bridge_and_removes_profile From 4bc0824fdb28ff3b093989f287785c4717d47f82 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Thu, 9 Jul 2026 20:37:08 -0700 Subject: [PATCH 08/42] feat(herdr): make Herdr lab lifecycle safety deterministic for briefs (#402) * guard Herdr lab lifecycle in briefs * no-mistakes(review): Fix Herdr lab helper and provisioning safety * no-mistakes(review): Captain, harden Herdr lab lifecycle safety * no-mistakes(review): fix Herdr lab test cleanup ordering and brief help range * no-mistakes(review): reject leading options in Herdr lab run guard * no-mistakes(review): strip leading non-alnum in Herdr lab name generator * no-mistakes(document): document Herdr lab helper and --herdr-lab brief flag * no-mistakes(lint): add shellcheck disable for deliberate SC2016 literals in fm-brief herdr-lab * no-mistakes: apply CI fixes --- AGENTS.md | 2 + CONTRIBUTING.md | 3 +- bin/fm-brief.sh | 52 ++- bin/fm-herdr-lab.sh | 339 ++++++++++++++++++ docs/herdr-backend.md | 12 +- docs/scripts.md | 3 +- tests/fm-afk-inject-herdr-e2e.test.sh | 3 +- tests/fm-backend-autodetect-smoke.test.sh | 6 +- .../fm-backend-herdr-prune-safety-e2e.test.sh | 6 +- .../fm-backend-herdr-respawn-idem-e2e.test.sh | 8 +- tests/fm-backend-herdr-smoke.test.sh | 8 +- ...ckend-herdr-workspace-per-home-e2e.test.sh | 6 +- tests/fm-brief.test.sh | 88 +++++ tests/fm-herdr-lab.test.sh | 240 +++++++++++++ tests/herdr-test-safety.sh | 86 +---- 15 files changed, 762 insertions(+), 100 deletions(-) create mode 100755 bin/fm-herdr-lab.sh create mode 100755 tests/fm-herdr-lab.test.sh diff --git a/AGENTS.md b/AGENTS.md index 579110e6a0..ebe803e919 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -875,6 +875,8 @@ The scaffold reads the mode via `fm-project-mode.sh`, so you do not pass it. Ship briefs also include the project-memory contract: run `bin/fm-ensure-agents-md.sh` when the project already has agent-memory files or when the task produced durable project-intrinsic knowledge, then record proportionate learnings in `AGENTS.md`. For scout tasks add `--scout`: the scaffold swaps the definition of done for the report contract (findings to `data//report.md`, no branch, no push, no PR) and declares the worktree scratch; scout is mode-agnostic. Scout briefs do not include the project-memory step, because their deliverable is a report rather than a committed project change. +For a crewmate task that will drive Herdr lifecycle behavior, add `--herdr-lab`: the scaffold embeds the hard Herdr-isolation contract backed by `bin/fm-herdr-lab.sh` (a never-`default` lab session, a trailing `--session` on every Herdr call, guarded teardown, and a before/after fleet-state tripwire), and the flag is rejected for `--secondmate` briefs. +The flag must be explicit because the scaffold cannot read the `{TASK}` text it fills in later, so every ship or scout brief scaffolded without it carries a loud not-enabled gate telling the crewmate to stop and regenerate with `--herdr-lab` if the task turns out to touch Herdr lifecycle. For secondmates use `bin/fm-brief.sh --secondmate ...`. The scaffold writes a charter brief instead of a task brief. Set `FM_SECONDMATE_CHARTER=''` to fill the charter text and `FM_SECONDMATE_SCOPE=''` when the routing scope differs. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6d1f22d73b..495fcd68ad 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -94,7 +94,8 @@ tests/fm-fleet-sync.test.sh # project clone refresh: safe detached tests/fm-fleet-snapshot-view.test.sh # read-only fleet snapshot JSON and Markdown view coverage: stable ordering, current-state vs status-event separation, backlog parsing, scout reports, secondmate return-channel guidance, and operational directory overrides tests/fm-x-mode.test.sh # X-mode poll, inbox context round-trip, reply threading, dismiss, completion follow-up counters/caps, dry-run preview, and .env-presence activation tests tests/fm-tangle-guard.test.sh # primary-checkout tangle detection, read-only remediation suppression, and spawn/brief isolation tests -tests/fm-brief.test.sh # fm-brief.sh bash -n parse regression guard (issue #166) and clean no-mistakes/direct-PR/local-only brief generation tests +tests/fm-brief.test.sh # fm-brief.sh bash -n parse regression guard, clean delivery-mode scaffolds, and hard Herdr lifecycle declaration/contract tests +tests/fm-herdr-lab.test.sh # isolated Herdr lab helper fail-closed naming, scoped calls, forbidden operations, destructive-call guard ordering, and fleet-state tripwire tests tests/fm-dispatch-select.test.sh # deterministic crew-dispatch profile selection, quota-balanced tie/stale/fallback behavior, and backward-compatible first-profile selection tests tests/fm-spawn-batch.test.sh # batch dispatch and FM_HOME project-path scoping tests tests/fm-spawn-dispatch-profile.test.sh # concrete dispatch profile flags: active-profile backstop, harness/model/effort meta, launch templates, batch forwarding, and secondmate exemption diff --git a/bin/fm-brief.sh b/bin/fm-brief.sh index 53ca7d5df0..503e4d6744 100755 --- a/bin/fm-brief.sh +++ b/bin/fm-brief.sh @@ -6,7 +6,7 @@ # description, acceptance criteria, and context, and may adjust other sections # when the task genuinely deviates (e.g. working an existing external PR instead # of shipping a new one). -# Usage: fm-brief.sh [--scout] +# Usage: fm-brief.sh [--scout] [--herdr-lab] # fm-brief.sh --secondmate ... # --scout writes the scout contract instead: the deliverable is a report at # data//report.md (no branch, no push, no PR) and the worktree is scratch. @@ -17,6 +17,11 @@ # home's status file. # Set FM_SECONDMATE_CHARTER='' to fill the charter text. # Set FM_SECONDMATE_SCOPE='' to write a routing scope distinct from the charter text. +# --herdr-lab is mandatory when the task will issue Herdr lifecycle commands. +# It adds the hard isolation contract backed by bin/fm-herdr-lab.sh. +# The flag must be explicit because {TASK} is filled after scaffolding and the +# caller-supplied repo string cannot reliably identify this repo. Briefs made +# without it carry a loud declaration so an omitted contract cannot be silent. # For ship tasks, the definition of done is shaped by the project's delivery mode # (data/projects.md via fm-project-mode.sh; see AGENTS.md project management # and task lifecycle): @@ -37,7 +42,7 @@ set -eu SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" usage() { - sed -n '2,34p' "$0" | sed 's/^# \{0,1\}//' + sed -n '2,39p' "$0" | sed 's/^# \{0,1\}//' } case "${1:-}" in @@ -51,16 +56,23 @@ FM_HOME="${FM_HOME:-${FM_ROOT_OVERRIDE:-$FM_ROOT}}" DATA="${FM_DATA_OVERRIDE:-$FM_HOME/data}" STATE="${FM_STATE_OVERRIDE:-$FM_HOME/state}" KIND=ship +HERDR_LAB=0 POS=() for a in "$@"; do case "$a" in --scout) KIND=scout ;; --secondmate) KIND=secondmate ;; + --herdr-lab) HERDR_LAB=1 ;; *) POS+=("$a") ;; esac done ID=${POS[0]} +if [ "$KIND" = secondmate ] && [ "$HERDR_LAB" -eq 1 ]; then + echo "error: --herdr-lab applies only to crewmate ship or scout briefs" >&2 + exit 1 +fi + BRIEF="$DATA/$ID/brief.md" [ -e "$BRIEF" ] && { echo "error: $BRIEF already exists" >&2; exit 1; } mkdir -p "$DATA/$ID" @@ -140,6 +152,38 @@ fi REPO=${POS[1]} +if [ "$HERDR_LAB" -eq 1 ]; then +HERDR_LAB_HELPER=$(shell_quote "$FM_ROOT/bin/fm-herdr-lab.sh") +# shellcheck disable=SC2016 # single quotes are deliberate: these lines are literal brief text whose backtick-wrapped $(...) and "$HERDR_LAB_SESSION" snippets must reach the reading agent verbatim, not expand at scaffold time; only the '"$VAR"' break-outs interpolate. +HERDR_SECTION=$(printf '%s\n' \ +'# Herdr isolation - HARD SAFETY CONTRACT' \ +'This brief was explicitly scaffolded with `--herdr-lab` because the task will drive Herdr lifecycle behavior.' \ +'On Herdr 0.7.3 the API socket is not relocatable by `HERDR_CONFIG_PATH`, `XDG_CONFIG_HOME`, or `HOME`.' \ +'A named non-`default` session plus a trailing `--session ` on every call is the only viable local isolation.' \ +'' \ +'1. Set `HERDR_LAB_HELPER='"$HERDR_LAB_HELPER"'` and generate the session name with `HERDR_LAB_SESSION=$("$HERDR_LAB_HELPER" name '"$ID"')`.' \ +' Install `trap '\''"$HERDR_LAB_HELPER" teardown "$HERDR_LAB_SESSION"'\'' EXIT` before provisioning, then provision only with `"$HERDR_LAB_HELPER" provision "$HERDR_LAB_SESSION"`.' \ +'2. Run every task-specific non-lifecycle Herdr command through `"$HERDR_LAB_HELPER" run "$HERDR_LAB_SESSION" `.' \ +' The helper appends the required trailing `--session "$HERDR_LAB_SESSION"`; `HERDR_SESSION` alone is never accepted as isolation.' \ +'3. Teardown only through `"$HERDR_LAB_HELPER" teardown "$HERDR_LAB_SESSION"`.' \ +' It re-checks refuse-default immediately before stop and again immediately before delete, and fails closed on ambiguity.' \ +'4. If an experiment requires a deliberate mid-run session stop, use only `"$HERDR_LAB_HELPER" stop "$HERDR_LAB_SESSION"`; it performs the same immediate refuse-default check.' \ +'5. Forbidden commands: direct `herdr server stop`, every other server-global operation such as `herdr server live-handoff` or reload/update operations, direct `herdr session stop`, direct `herdr session delete`, and any Herdr call scoped only by ambient or inline `HERDR_SESSION`.' \ +'6. The helper records the live default session before provisioning and verifies the identical fleet state after teardown.' \ +' A missing, stopped, or changed default session is a hard tripwire failure, never a cleanup warning to ignore.' \ +'' \ +'Never bypass the helper, even for a read-only lifecycle probe or cleanup after failure.' \ +'The captain fleet uses the running `default` session.') +else +HERDR_SECTION=$(cat <<'EOF' +# Herdr lifecycle declaration - NOT ENABLED +**HARD SAFETY GATE:** this scaffold cannot inspect the task text that replaces `{TASK}` later. +If the task will start, stop, delete, restart, profile, or otherwise drive Herdr lifecycle behavior, stop and regenerate the brief with `--herdr-lab` before dispatch. +Do not add Herdr lifecycle commands to this unguarded brief by hand. +EOF +) +fi + if [ "$KIND" = scout ]; then cat > "$BRIEF" < +# fm-herdr-lab.sh prepare +# fm-herdr-lab.sh provision +# fm-herdr-lab.sh run +# fm-herdr-lab.sh stop +# fm-herdr-lab.sh teardown +# +# Session names must begin with "fm-lab-" and can never be "default". +# Every Herdr call made here carries a trailing --session . +# The run command rejects caller-supplied --session flags, any leading option +# before the subcommand, all session lifecycle operations, and every server +# operation. +# Session stop is available only through guarded stop or teardown, and session +# delete is available only through teardown. +# Both paths perform a fresh refuse-default check immediately before each +# destructive call. +# Provision records the running default session as a fleet-state tripwire and +# teardown requires that record to be identical afterward. +set -u + +fm_herdr_lab_error() { + echo "fm-herdr-lab: $*" >&2 +} + +fm_herdr_lab_validate_name() { # + local name=${1:-} + [[ "$name" =~ ^fm-lab-[a-zA-Z0-9][a-zA-Z0-9_-]*$ ]] && return 0 + case "$name" in + default) fm_herdr_lab_error "refusing session name 'default'" ;; + '') fm_herdr_lab_error "refusing an empty session name" ;; + *) fm_herdr_lab_error "session name must start with 'fm-lab-' and contain only letters, digits, underscores, or dashes: $name" ;; + esac + return 1 +} + +fm_herdr_lab_state_dir() { + printf '%s' "${FM_HERDR_LAB_STATE_DIR:-${TMPDIR:-/tmp}/fm-herdr-lab-${UID}}" +} + +fm_herdr_lab_tripwire_path() { # + printf '%s/%s.fleet-state.json' "$(fm_herdr_lab_state_dir)" "$1" +} + +fm_herdr_lab_raw() { # + local name=$1 + shift + HERDR_SESSION="$name" herdr "$@" --session "$name" +} + +fm_herdr_lab_session_list() { # + fm_herdr_lab_raw "$1" session list --json +} + +fm_herdr_lab_fleet_state() { # + local name=$1 sessions snapshot + sessions=$(fm_herdr_lab_session_list "$name" 2>/dev/null) || { + fm_herdr_lab_error "cannot read Herdr sessions for the fleet-state tripwire" + return 1 + } + snapshot=$(printf '%s' "$sessions" | jq -c ' + [.sessions[]? | select(.default == true)] + | if length == 1 and .[0].name == "default" and .[0].running == true + then .[0] | {name, default, running, socket_path} + else empty + end + ' 2>/dev/null) + [ -n "$snapshot" ] || { + fm_herdr_lab_error "fleet-state tripwire requires exactly one running default session" + return 1 + } + printf '%s\n' "$snapshot" +} + +fm_herdr_lab_prepare() { # + local name=$1 sessions state_dir tripwire + fm_herdr_lab_validate_name "$name" || return 1 + command -v herdr >/dev/null 2>&1 || { fm_herdr_lab_error "herdr is required"; return 1; } + command -v jq >/dev/null 2>&1 || { fm_herdr_lab_error "jq is required"; return 1; } + + sessions=$(fm_herdr_lab_session_list "$name" 2>/dev/null) || { + fm_herdr_lab_error "cannot list Herdr sessions before provisioning '$name'" + return 1 + } + if printf '%s' "$sessions" | jq -e --arg name "$name" '.sessions[]? | select(.name == $name)' >/dev/null 2>&1; then + fm_herdr_lab_error "session '$name' already exists; refusing to adopt or overwrite it" + return 1 + fi + + state_dir=$(fm_herdr_lab_state_dir) + tripwire=$(fm_herdr_lab_tripwire_path "$name") + mkdir -p "$state_dir" || return 1 + [ ! -e "$tripwire" ] || { + fm_herdr_lab_error "tripwire already exists for '$name'; refusing ambiguous ownership" + return 1 + } + fm_herdr_lab_fleet_state "$name" > "$tripwire" || { + rm -f "$tripwire" + return 1 + } +} + +fm_herdr_lab_refuse_if_default() { # + local name=$1 info flag + fm_herdr_lab_validate_name "$name" || return 1 + info=$(fm_herdr_lab_session_list "$name" 2>/dev/null) || { + fm_herdr_lab_error "refusing destructive call because session list failed" + return 1 + } + flag=$(printf '%s' "$info" | jq -r --arg name "$name" \ + '.sessions[]? | select(.name == $name) | .default' 2>/dev/null) + [ "$flag" = false ] && return 0 + fm_herdr_lab_error "refusing destructive call for '$name': session is absent or default (default=${flag:-})" + return 1 +} + +fm_herdr_lab_cli() { # + local name=$1 arg + shift + fm_herdr_lab_validate_name "$name" || return 1 + [ "$#" -gt 0 ] || { fm_herdr_lab_error "run requires Herdr arguments"; return 1; } + case "$1" in + -*) + fm_herdr_lab_error "run forbids a leading option before the Herdr subcommand; it could shift a server or session lifecycle operation past the guard or subvert session isolation" + return 1 + ;; + esac + for arg in "$@"; do + case "$arg" in + --session|--session=*) + fm_herdr_lab_error "run forbids caller-supplied --session; the helper appends the lab session" + return 1 + ;; + esac + done + case "$1 ${2:-}" in + "server "*) + fm_herdr_lab_error "run forbids server operations; use provision for the named lab server" + return 1 + ;; + "session list") ;; + "session "*) + fm_herdr_lab_error "run forbids session lifecycle operations; use guarded teardown" + return 1 + ;; + esac + fm_herdr_lab_raw "$name" "$@" +} + +fm_herdr_lab_cancel_provision() { # + local pid=$1 attempt=0 + if kill -0 "$pid" 2>/dev/null; then + kill -TERM "$pid" 2>/dev/null || true + while kill -0 "$pid" 2>/dev/null && [ "$attempt" -lt 10 ]; do + sleep 0.1 + attempt=$((attempt + 1)) + done + if kill -0 "$pid" 2>/dev/null; then + kill -KILL "$pid" 2>/dev/null || true + fi + fi + wait "$pid" 2>/dev/null || true +} + +fm_herdr_lab_provision() { # + local name=$1 sessions tripwire running attempt server_pid + fm_herdr_lab_validate_name "$name" || return 1 + command -v herdr >/dev/null 2>&1 || { fm_herdr_lab_error "herdr is required"; return 1; } + command -v jq >/dev/null 2>&1 || { fm_herdr_lab_error "jq is required"; return 1; } + + sessions=$(fm_herdr_lab_session_list "$name" 2>/dev/null) || { + fm_herdr_lab_error "cannot list Herdr sessions before provisioning '$name'" + return 1 + } + if printf '%s' "$sessions" | jq -e --arg name "$name" '.sessions[]? | select(.name == $name)' >/dev/null 2>&1; then + tripwire=$(fm_herdr_lab_tripwire_path "$name") + [ -f "$tripwire" ] || { + fm_herdr_lab_error "missing fleet-state tripwire for existing session '$name'; refusing to adopt it" + return 1 + } + fm_herdr_lab_refuse_if_default "$name" || return 1 + running=$(printf '%s' "$sessions" | jq -r --arg name "$name" \ + '.sessions[]? | select(.name == $name) | .running' 2>/dev/null) + [ "$running" = false ] || { + fm_herdr_lab_error "session '$name' is not stopped; refusing to re-provision it" + return 1 + } + fm_herdr_lab_check_tripwire "$name" || return 1 + else + fm_herdr_lab_prepare "$name" || return 1 + fi + fm_herdr_lab_raw "$name" server >/dev/null 2>&1 & + server_pid=$! + attempt=0 + while [ "$attempt" -lt 50 ]; do + running=$(fm_herdr_lab_cli "$name" status --json 2>/dev/null | jq -r '.server.running // false' 2>/dev/null) || running=false + if [ "$running" = true ]; then + fm_herdr_lab_refuse_if_default "$name" || { + fm_herdr_lab_cancel_provision "$server_pid" + return 1 + } + return 0 + fi + sleep 0.2 + attempt=$((attempt + 1)) + done + fm_herdr_lab_cancel_provision "$server_pid" + fm_herdr_lab_error "lab session '$name' did not report running within 10 seconds" + return 1 +} + +fm_herdr_lab_check_tripwire() { # + local name=$1 tripwire before after + tripwire=$(fm_herdr_lab_tripwire_path "$name") + [ -f "$tripwire" ] || { + fm_herdr_lab_error "missing fleet-state tripwire for '$name'; refusing unverified teardown" + return 1 + } + before=$(cat "$tripwire") + after=$(fm_herdr_lab_fleet_state "$name") || return 1 + [ "$before" = "$after" ] || { + fm_herdr_lab_error "FLEET-STATE TRIPWIRE FAILED: default session changed during lab work" + fm_herdr_lab_error "before: $before" + fm_herdr_lab_error "after: $after" + return 1 + } +} + +fm_herdr_lab_verify_tripwire() { # + local name=$1 tripwire + fm_herdr_lab_check_tripwire "$name" || return 1 + tripwire=$(fm_herdr_lab_tripwire_path "$name") + rm -f "$tripwire" +} + +fm_herdr_lab_stop() { # + local name=$1 tripwire + fm_herdr_lab_validate_name "$name" || return 1 + tripwire=$(fm_herdr_lab_tripwire_path "$name") + [ -f "$tripwire" ] || { + fm_herdr_lab_error "missing fleet-state tripwire for '$name'; refusing stop" + return 1 + } + fm_herdr_lab_refuse_if_default "$name" || return 1 + fm_herdr_lab_raw "$name" session stop "$name" --json +} + +fm_herdr_lab_teardown() { # + local name=$1 tripwire sessions delete_status=0 + fm_herdr_lab_validate_name "$name" || return 1 + tripwire=$(fm_herdr_lab_tripwire_path "$name") + [ -f "$tripwire" ] || { + fm_herdr_lab_error "missing fleet-state tripwire for '$name'; refusing destructive calls" + return 1 + } + sessions=$(fm_herdr_lab_session_list "$name" 2>/dev/null) || { + fm_herdr_lab_error "cannot list Herdr sessions before teardown" + return 1 + } + if ! printf '%s' "$sessions" | jq -e --arg name "$name" '.sessions[]? | select(.name == $name)' >/dev/null 2>&1; then + fm_herdr_lab_verify_tripwire "$name" + return + fi + fm_herdr_lab_stop "$name" >/dev/null 2>&1 || true + sleep 0.5 + fm_herdr_lab_refuse_if_default "$name" || return 1 + fm_herdr_lab_raw "$name" session delete "$name" --json >/dev/null 2>&1 || delete_status=$? + sessions=$(fm_herdr_lab_session_list "$name" 2>/dev/null) || { + fm_herdr_lab_error "cannot confirm removal of lab session '$name' after teardown" + return 1 + } + if printf '%s' "$sessions" | jq -e --arg name "$name" '.sessions[]? | select(.name == $name)' >/dev/null 2>&1; then + if [ "$delete_status" -ne 0 ]; then + fm_herdr_lab_error "session delete failed for '$name' and the lab session remains" + else + fm_herdr_lab_error "lab session '$name' remains after teardown" + fi + return 1 + fi + fm_herdr_lab_verify_tripwire "$name" +} + +fm_herdr_lab_name() { #