diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 275507590a0..6f2ec166b6b 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -1112,6 +1112,21 @@ spawn_omp_abort_endpoint_stopped() { # [meta] esac } +# Remove exactly the per-generation runtime marker artifacts a finished or +# aborted OMP secondmate generation left behind, so a later launch's entry +# validation is not refused by the prior generation's leftovers. Reports +# failure instead of partially succeeding silently. +spawn_omp_secondmate_retire_generation_markers() { + local failure=0 + rm -f -- "$STATE/$ID.omp-ext.ts" "$STATE/$ID.omp-ready" \ + "$STATE/$ID.omp-started" "$STATE/$ID.omp-doorbell-ready" \ + "$STATE/$ID.omp-doorbell-failed" || failure=1 + if [ -d "$STATE/$ID.omp-doorbell-ready.requests" ]; then + rm -rf -- "$STATE/$ID.omp-doorbell-ready.requests" || failure=1 + fi + return "$failure" +} + # Retire exactly the artifacts a failed OMP secondmate launch created and the # next launch's entry validation checks, so a later launch is accepted instead # of refusing on the dead generation's leftovers. This runs only after the @@ -1125,12 +1140,7 @@ spawn_omp_abort_endpoint_stopped() { # [meta] # markers are retired, but a live owner prevents session-pointer repair. spawn_omp_secondmate_abort_retire_generation() { local marker lock_pid pointer named keep session candidate count pointer_tmp live_owner=0 failure=0 - rm -f -- "$STATE/$ID.omp-ext.ts" "$STATE/$ID.omp-ready" \ - "$STATE/$ID.omp-started" "$STATE/$ID.omp-doorbell-ready" \ - "$STATE/$ID.omp-doorbell-failed" || failure=1 - if [ -d "$STATE/$ID.omp-doorbell-ready.requests" ]; then - rm -rf -- "$STATE/$ID.omp-doorbell-ready.requests" || failure=1 - fi + spawn_omp_secondmate_retire_generation_markers || failure=1 marker="$PROJ_ABS/state/.omp-primary-extension-loaded" if [ -f "$marker" ] && [ ! -L "$marker" ]; then if ! fm_omp_primary_marker_read "$marker"; then @@ -2404,12 +2414,29 @@ if [ "$HARNESS" = omp ]; then ;; esac fi - for artifact in "$STATE/$ID.omp-ext.ts" "$STATE/$ID.omp-ready" "$STATE/$ID.omp-started"; do - if [ -e "$artifact" ] || [ -L "$artifact" ]; then - echo "error: refusing OMP secondmate launch because worker-only artifact exists at $artifact" >&2 + if [ "$OMP_SECONDMATE_RELAUNCH" = 1 ]; then + for artifact in \ + "$STATE/$ID.omp-ext.ts" "$STATE/$ID.omp-ready" \ + "$STATE/$ID.omp-started" "$STATE/$ID.omp-doorbell-ready" \ + "$STATE/$ID.omp-doorbell-failed"; do + if [ -L "$artifact" ] || { [ -e "$artifact" ] && [ ! -f "$artifact" ]; }; then + echo "error: refusing OMP secondmate relaunch through unsafe artifact path: $artifact" >&2 + exit 1 + fi + done + OMP_REQUESTS_DIR="$STATE/$ID.omp-doorbell-ready.requests" + if [ -L "$OMP_REQUESTS_DIR" ] || { [ -e "$OMP_REQUESTS_DIR" ] && [ ! -d "$OMP_REQUESTS_DIR" ]; }; then + echo "error: refusing OMP secondmate relaunch through unsafe artifact path: $OMP_REQUESTS_DIR" >&2 exit 1 fi - done + else + for artifact in "$STATE/$ID.omp-ext.ts" "$STATE/$ID.omp-ready" "$STATE/$ID.omp-started"; do + if [ -e "$artifact" ] || [ -L "$artifact" ]; then + echo "error: refusing OMP secondmate launch because worker-only artifact exists at $artifact" >&2 + exit 1 + fi + done + fi else if [ "$RELAUNCH" -eq 1 ]; then OMP_PRIOR_META=$RELAUNCH_META @@ -3182,6 +3209,10 @@ if [ "$KIND" = secondmate ]; then exit 1 } fi + if [ "$OMP_SECONDMATE_RELAUNCH" = 1 ] && ! spawn_omp_secondmate_retire_generation_markers; then + echo "error: could not retire the prior OMP secondmate generation's runtime artifacts; refusing launch" >&2 + exit 1 + fi fi if [ "${FM_SKIP_SECONDMATE_INHERIT:-0}" != 1 ]; then CONFIG_INHERIT_LOCK=$(fm_config_inherit_lock_path "$PROJ_ABS") || { diff --git a/docs/configuration.md b/docs/configuration.md index 41490a7977b..b9eee9c1645 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -308,7 +308,8 @@ For Pi and pi-signed secondmate launches, `fm-spawn.sh` starts the selected exec For OMP secondmate launches, it explicitly loads that home's `.omp/extensions/fm-primary-omp.ts`, keeps exact resume state under `state/omp-sessions`, and the adapter publishes `state/.omp-session` when it binds the selected conversation. After proving a failed launch's owned endpoint stopped, cleanup retires its generation artifacts while preserving the persistent home, endpoint metadata, durable inbox, and real session files. Without a live marker or lock owner, cleanup preserves an already-correct session binding or atomically repairs it to the pre-launch retained session, or to the unique saved session when no valid binding exists; an empty store permits pointer removal, while ambiguous saved sessions require explicit reconciliation. -[`tests/fm-omp-secondmate.test.sh`](../tests/fm-omp-secondmate.test.sh) and [`tests/fm-remote-secondmate-lifecycle-e2e.test.sh`](../tests/fm-remote-secondmate-lifecycle-e2e.test.sh) cover failed-bind recovery for local and remote homes. +A relaunch whose recorded endpoint is proven dead or missing retires the prior generation's runtime markers (adapter, ready, started, doorbell markers and request receipts) only after the endpoint, session-lock, and integration-marker live-owner checks pass, so a wake-on-delivery restore after a successful generation is accepted. +[`tests/fm-omp-secondmate.test.sh`](../tests/fm-omp-secondmate.test.sh) and [`tests/fm-remote-secondmate-lifecycle-e2e.test.sh`](../tests/fm-remote-secondmate-lifecycle-e2e.test.sh) cover failed-bind recovery and post-generation restore relaunch for local and remote homes. Firstmate's tracked `.omp/config.yml` keeps OMP's todo tool enabled but disables its end-of-turn incomplete-todo reminders because the session todo is a projection of long-lived fleet work rather than work that must finish within the current captain turn. This prevents OMP from scheduling a second model turn after a captain-facing answer solely because projected fleet work remains incomplete. The separately discovered `.omp/extensions/fm-fleet-hooks.ts` fails open while redacting credential-shaped values from text chunks in tool results, reporting valid `fm-todo-project.sh --check` drift through hidden next-turn context, and injecting a bounded roster, OPEN DECISIONS, in-flight PR, and Ready/In-flight backlog snapshot during compaction. diff --git a/tests/fm-backend-herdr-presentation-e2e.test.sh b/tests/fm-backend-herdr-presentation-e2e.test.sh index bc203ada8e7..0374bcc01a2 100755 --- a/tests/fm-backend-herdr-presentation-e2e.test.sh +++ b/tests/fm-backend-herdr-presentation-e2e.test.sh @@ -1320,6 +1320,21 @@ teardown_task aflat "$SECOND_HOME_A" > "$TMP_ROOT/aflat-teardown.out" 2> "$TMP_R || fail "flat cross-home contention fixture teardown failed" pass "real Herdr lab: session lock contention from a secondmate home falls back flat with no journal" +# Retire the multi-home slot owners before whole-session restarts make their +# worktrees reusable, while still proving exact-pane teardown and focus safety. +for META_HOME_PAIR in \ + "p1:$HOME_DIR" "p2:$HOME_DIR" "pcw:$HOME_DIR" \ + "a1:$SECOND_HOME_A" "a2:$SECOND_HOME_A" "acw:$SECOND_HOME_A" \ + "b1:$SECOND_HOME_B" "b2:$SECOND_HOME_B" "bcw:$SECOND_HOME_B" +do + TASK_ID=${META_HOME_PAIR%%:*} + TASK_HOME=${META_HOME_PAIR#*:} + teardown_task "$TASK_ID" "$TASK_HOME" > "$TMP_ROOT/td-$TASK_ID.out" 2> "$TMP_ROOT/td-$TASK_ID.err" \ + || fail "multi-home teardown of $TASK_ID failed: $(cat "$TMP_ROOT/td-$TASK_ID.err")" +done +assert_focus_is "$CAPTAIN_FOCUS" "multi-home teardown" +pass "real Herdr lab: multi-home exact-pane teardowns restore captain focus without workspace close authority" + # Same-identity recovery replaces only one exact agent-free husk in its # original projected workspace. # Exercise both the leading fm- identity style seen in Hi Bit work and the @@ -1523,20 +1538,11 @@ lab tab get "$FLAT_TAB_ID" >/dev/null 2>&1 \ || fail "correction removed the seeded flat secondmate child tab" pass "real Herdr lab: legacy projection labels and flat secondmate tabs are left unmigrated" -# Teardown multi-home projected tasks by exact pane only. -for META_HOME_PAIR in \ - "p1:$HOME_DIR" "p2:$HOME_DIR" "pcw:$HOME_DIR" "post-legacy:$HOME_DIR" \ - "a1:$SECOND_HOME_A" "a2:$SECOND_HOME_A" "acw:$SECOND_HOME_A" \ - "alpha:$HOME_DIR" \ - "b1:$SECOND_HOME_B" "b2:$SECOND_HOME_B" "bcw:$SECOND_HOME_B" -do - TASK_ID=${META_HOME_PAIR%%:*} - TASK_HOME=${META_HOME_PAIR#*:} - teardown_task "$TASK_ID" "$TASK_HOME" > "$TMP_ROOT/td-$TASK_ID.out" 2> "$TMP_ROOT/td-$TASK_ID.err" \ - || fail "multi-home teardown of $TASK_ID failed: $(cat "$TMP_ROOT/td-$TASK_ID.err")" -done -assert_focus_is "$CAPTAIN_FOCUS" "multi-home teardown" -pass "real Herdr lab: multi-home exact-pane teardowns restore captain focus without workspace close authority" +teardown_task alpha "$HOME_DIR" > "$TMP_ROOT/td-alpha.out" 2> "$TMP_ROOT/td-alpha.err" \ + || fail "secondmate alpha teardown failed: $(cat "$TMP_ROOT/td-alpha.err")" +teardown_task post-legacy "$HOME_DIR" > "$TMP_ROOT/td-post-legacy.out" 2> "$TMP_ROOT/td-post-legacy.err" \ + || fail "post-legacy teardown failed: $(cat "$TMP_ROOT/td-post-legacy.err")" +assert_focus_is "$CAPTAIN_FOCUS" "post-legacy teardown" stop_herdr_snapshotter # Missing, renamed, and duplicate tokens are read-only recovery diagnostics. diff --git a/tests/fm-omp-secondmate.test.sh b/tests/fm-omp-secondmate.test.sh index eead6f41dbe..1c135e9753e 100755 --- a/tests/fm-omp-secondmate.test.sh +++ b/tests/fm-omp-secondmate.test.sh @@ -907,6 +907,92 @@ test_duplicate_recovery_states() { pass "OMP secondmate recovery refuses live, ambiguous, and unreadable duplicates and relaunches only proven dead endpoints" } +test_slept_generation_relaunch_retires_markers() { + local out dead_pid retained version + + # A generation that ran turns and then lost its endpoint leaves its own + # runtime artifacts behind: the adapter file, readiness, turn-start and + # doorbell markers, and request receipts. A relaunch whose recorded + # endpoint is proven missing must retire them after the live-owner checks, + # not refuse on them. + setup_case slept-generation + write_meta + mkdir -p "$HOME_DIR/state/omp-sessions" + retained="$HOME_DIR/state/omp-sessions/selected.jsonl" + printf '{"type":"session"}\n' > "$retained" + printf '%s\n' "$retained" > "$HOME_DIR/state/.omp-session" + dead_pid=$( { sleep 0.05 & echo "$!"; wait; } 2>/dev/null ) + printf '%s\n' "$dead_pid" > "$HOME_DIR/state/.lock" + version=$(fm_primary_watch_version "$HOME_DIR/.omp/extensions/fm-primary-omp.ts" "$HOME_DIR") + printf '%s\n%s\n%s\n%s\n' "$version" "$dead_pid" "$TEST_OMP_BUN" "$TEST_OMP_BIN" \ + > "$HOME_DIR/state/.omp-primary-extension-loaded" + : > "$MAIN_STATE/$TASK_ID.omp-ext.ts" + : > "$MAIN_STATE/$TASK_ID.omp-ready" + : > "$MAIN_STATE/$TASK_ID.omp-started" + : > "$MAIN_STATE/$TASK_ID.omp-doorbell-ready" + : > "$MAIN_STATE/$TASK_ID.omp-doorbell-failed" + mkdir -p "$MAIN_STATE/$TASK_ID.omp-doorbell-ready.requests" + : > "$MAIN_STATE/$TASK_ID.omp-doorbell-ready.requests/request.1" + out=$(FM_TEST_STATE_MODE=missing run_spawn 2>&1) \ + || fail "OMP secondmate relaunch after a slept generation was refused: $out" + assert_absent "$MAIN_STATE/$TASK_ID.omp-ready" \ + "slept-generation relaunch left the prior readiness marker" + assert_absent "$MAIN_STATE/$TASK_ID.omp-started" \ + "slept-generation relaunch left the prior turn-start marker" + assert_absent "$MAIN_STATE/$TASK_ID.omp-doorbell-ready.requests/request.1" \ + "slept-generation relaunch left the prior request receipts" + assert_contains "$(cat "$LAUNCH_LOG")" "$retained" \ + "slept-generation relaunch did not resume the retained session" + + # The same prior artifacts remain refusal evidence when their paths are + # unsafe, even though the endpoint is proven missing. + setup_case slept-generation-symlink + write_meta + : > "$MAIN_STATE/$TASK_ID.omp-ext.ts" + : > "$MAIN_STATE/$TASK_ID.omp-ready" + ln -s "$CASE/nonexistent-target" "$MAIN_STATE/$TASK_ID.omp-started" + out=$(FM_TEST_STATE_MODE=missing run_spawn 2>&1) \ + && fail "OMP secondmate relaunch accepted a symlinked generation artifact" + assert_contains "$out" 'unsafe artifact path' \ + "symlinked generation artifact refusal was not actionable" + [ "$(count_new_windows)" = 0 ] \ + || fail "symlinked generation artifact refusal created another endpoint" + + # A live session-lock owner still refuses and must not disturb the prior + # generation's artifacts. + setup_case slept-generation-live-owner + write_meta + : > "$MAIN_STATE/$TASK_ID.omp-ext.ts" + : > "$MAIN_STATE/$TASK_ID.omp-ready" + : > "$MAIN_STATE/$TASK_ID.omp-started" + printf '%s\n' "$AGENT_PID" > "$HOME_DIR/state/.lock" + out=$(FM_TEST_STATE_MODE=missing run_spawn 2>&1) \ + && fail "OMP secondmate relaunch ignored a live session-lock owner" + assert_contains "$out" 'live session-lock owner' \ + "live session-lock owner refusal was not actionable" + assert_present "$MAIN_STATE/$TASK_ID.omp-started" \ + "live-owner refusal removed the prior generation's turn-start marker" + assert_present "$MAIN_STATE/$TASK_ID.omp-ext.ts" \ + "live-owner refusal removed the prior generation's adapter file" + [ "$(count_new_windows)" = 0 ] \ + || fail "live session-lock owner refusal created another endpoint" + + # Without a recorded endpoint the same leftovers are still worker-only + # artifacts and refuse the launch exactly as before. + setup_case slept-generation-no-meta + : > "$MAIN_STATE/$TASK_ID.omp-ext.ts" + : > "$MAIN_STATE/$TASK_ID.omp-ready" + : > "$MAIN_STATE/$TASK_ID.omp-started" + out=$(FM_TEST_STATE_MODE=missing run_spawn 2>&1) \ + && fail "OMP secondmate launch accepted worker-only artifacts without a recorded endpoint" + assert_contains "$out" 'worker-only artifact' \ + "worker-only artifact refusal was not actionable" + [ "$(count_new_windows)" = 0 ] \ + || fail "worker-only artifact refusal created another endpoint" + + pass "a slept OMP secondmate generation's relaunch retires prior runtime markers only after live-owner checks pass" +} + test_post_meta_abort_preserves_home() { local out setup_case abort @@ -1016,4 +1102,5 @@ test_rendered_legacy_launch_executes test_legacy_launch_rejects_empty_path_components test_legacy_launch_rejects_runtime_fallback test_duplicate_recovery_states +test_slept_generation_relaunch_retires_markers test_post_meta_abort_preserves_home diff --git a/tests/fm-remote-secondmate-lifecycle-e2e.test.sh b/tests/fm-remote-secondmate-lifecycle-e2e.test.sh index 4659ab77eae..12b24e86386 100755 --- a/tests/fm-remote-secondmate-lifecycle-e2e.test.sh +++ b/tests/fm-remote-secondmate-lifecycle-e2e.test.sh @@ -1581,6 +1581,91 @@ set -e rm -f "$HERDR_FORCE_IDLE" pass "remote OMP delivery replaces the reproducible typed no-turn regression with one bound inbox request, programmatic turn, and handled acknowledgement" +# --- wake-on-delivery restore after a successful generation ----------------- +# The remote-omp secondmate completed a real generation and then lost its +# endpoint as if its compute had slept: the pane is gone but its generation's +# runtime artifacts are still on disk. A live owner must still refuse, and a +# genuinely dead owner must let the ordinary relaunch retire the prior +# generation's markers instead of refusing on them. + +assert_present "$OMP_TURN_STARTED" \ + "the remote OMP generation lost its own turn-start marker before the restore case" +assert_present "$OMP_READY" \ + "the remote OMP generation lost its own doorbell marker before the restore case" +OMP_PRIOR_STARTED=$(cat "$OMP_TURN_STARTED") + +# A live home owner still refuses the relaunch and leaves the prior +# generation's artifacts untouched. +if ! kill -0 "$OMP_LISTENER_PID" 2>/dev/null; then + ( sleep 300 & echo "$!" > "$TMP_ROOT/lock-owner.pid" ) + OMP_LISTENER_PID=$(cat "$TMP_ROOT/lock-owner.pid") + printf '%s\n' "$OMP_LISTENER_PID" > "$OMP_REMOTE_HOME/state/.lock" +fi +set +e +remote_env "$ROOT/bin/fm-spawn.sh" remote-omp --secondmate \ + > "$TMP_ROOT/remote-omp-live-relaunch.out" 2>&1 +live_relaunch_rc=$? +set -e +[ "$live_relaunch_rc" -ne 0 ] \ + || fail "remote OMP relaunch launched over a live session-lock owner" +case "$(cat "$TMP_ROOT/remote-omp-live-relaunch.out")" in + *'live session-lock owner'*|*'live primary-integration marker owner'*) ;; + *) fail "live-owner remote OMP relaunch refusal did not name its owner:"$'\n'"$(cat "$TMP_ROOT/remote-omp-live-relaunch.out")" ;; +esac +assert_present "$OMP_TURN_STARTED" \ + "live-owner refusal removed the prior generation's turn-start marker" +assert_present "$OMP_READY" \ + "live-owner refusal removed the prior generation's doorbell marker" + +# The compute sleep takes the agent process; the generation's artifacts stay. +kill -TERM "$OMP_LISTENER_PID" 2>/dev/null || true +listener_gone=0 +while kill -0 "$OMP_LISTENER_PID" 2>/dev/null; do + listener_gone=$((listener_gone + 1)) + [ "$listener_gone" -le 250 ] || fail "remote OMP listener did not exit on SIGTERM" + sleep 0.02 +done +assert_present "$OMP_TURN_STARTED" \ + "compute sleep removed the prior generation's turn-start marker" + +# An awake compute-managed route takes the wake-on-delivery restore path: the +# ordinary secondmate launch must accept the slept generation's leftovers. +mkdir -p "$PARENT/data/boat" +printf 'lifecycle=ready\n' > "$PARENT/data/boat/remote-omp.meta" +: > "$OMP_ACTIVE_PID" +sent_before_restore=$(wc -l < "$OMP_SENT" | tr -d ' ') +records_before_restore=$(find "$OMP_INBOX" -name '*.msg' | wc -l | tr -d ' ') +remote_env "$ROOT/bin/fm-send.sh" fm-remote-omp \ + "wake-on-delivery restore after a successful generation" \ + > "$TMP_ROOT/remote-omp-restore.out" 2>&1 \ + || fail "wake-on-delivery restore after a successful generation failed:"$'\n'"$(cat "$TMP_ROOT/remote-omp-restore.out")" +grep -Eq '^request=[0-9a-f]{16} record=[0-9]+ state=(recorded|handled)$' \ + "$TMP_ROOT/remote-omp-restore.out" \ + || fail "restored remote OMP delivery did not report its durable machine line:"$'\n'"$(cat "$TMP_ROOT/remote-omp-restore.out")" +[ "$(remote_env "$ROOT/bin/fm-on.sh" remote-omp fm-remote-secondmate-control.sh state remote-omp)" = alive ] \ + || fail "wake-on-delivery restore did not reach a live endpoint" +OMP_RESTORED_PID=$(cat "$OMP_ACTIVE_PID") +case "$OMP_RESTORED_PID" in + ''|*[!0-9]*) fail "restored remote OMP listener did not publish its pid" ;; +esac +[ "$OMP_RESTORED_PID" != "$OMP_LISTENER_PID" ] \ + || fail "wake-on-delivery restore rebound the dead listener instead of a new process" +OMP_LISTENER_PID=$OMP_RESTORED_PID +kill -0 "$OMP_LISTENER_PID" 2>/dev/null \ + || fail "restored remote OMP listener is not running" +[ "$(find "$OMP_INBOX" -name '*.msg' | wc -l | tr -d ' ')" = "$((records_before_restore + 1))" ] \ + || fail "restored remote OMP delivery did not durably record exactly one inbox record" +[ "$(wc -l < "$OMP_SENT" | tr -d ' ')" = "$((sent_before_restore + 1))" ] \ + || fail "restored remote OMP delivery did not trigger exactly one programmatic turn" +assert_grep '"deliverAs":"steer","triggerTurn":true' "$OMP_SENT" \ + "restored remote OMP delivery did not use programmatic triggerTurn" +[ "$(cat "$OMP_TURN_STARTED")" != "$OMP_PRIOR_STARTED" ] \ + || fail "restored remote OMP generation kept the prior generation's turn-start marker" +rm -f "$PARENT/data/boat/remote-omp.meta" +OMP_RESTORED_PANE=$(sed -n 's/^herdr_pane_id=//p' "$OMP_CONTROL_STATE/remote-omp.meta") +[ -z "$OMP_RESTORED_PANE" ] || "$REMOTE_ROOT/bin/herdr" pane close "$OMP_RESTORED_PANE" +pass "a successful remote OMP generation slept and restored on delivery retires its prior runtime markers and delivers exactly once" + # --- failed-bind generation retirement -------------------------------------- # The paid pilot's failed bind left the aborted generation's whole artifact # set behind: a session pointer naming a session file that never existed, a