From 660abe9b488f455b85b1dcd2ce251f62c5c5319c Mon Sep 17 00:00:00 2001 From: JTInventory Date: Mon, 27 Jul 2026 14:44:49 +0000 Subject: [PATCH 1/4] fix: release Codex session locks cleanly Track Codex locks by stable thread identity so isolated tool calls preserve ownership and matching SessionEnd hooks release only their own home's lock. Preserve Grok precedence, legacy numeric owners, and JT's existing PreToolUse hook. --- .codex/hooks.json | 22 ++ bin/fm-codex-session-lock-hook.sh | 62 +++++ bin/fm-harness.sh | 3 + bin/fm-lock.sh | 64 +++-- bin/fm-session-lock-lib.sh | 121 +++++++-- docs/configuration.md | 6 + docs/scripts.md | 3 +- tests/fm-codex-session-lock-live-e2e.test.sh | 33 +++ tests/fm-codex-session-lock.test.sh | 251 +++++++++++++++++++ tests/fm-session-start.test.sh | 13 +- 10 files changed, 527 insertions(+), 51 deletions(-) create mode 100755 bin/fm-codex-session-lock-hook.sh create mode 100755 tests/fm-codex-session-lock-live-e2e.test.sh create mode 100755 tests/fm-codex-session-lock.test.sh diff --git a/.codex/hooks.json b/.codex/hooks.json index 4812c6b6b9f..140cf82e405 100644 --- a/.codex/hooks.json +++ b/.codex/hooks.json @@ -1,5 +1,16 @@ { "hooks": { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "bash -lc 'payload=$(cat 2>/dev/null || true); [ -n \"$payload\" ] || exit 0; command -v jq >/dev/null 2>&1 || exit 0; root=$(pwd -P) || exit 0; [ -x \"$root/bin/fm-codex-session-lock-hook.sh\" ] || exit 0; [ -f \"$root/AGENTS.md\" ] || exit 0; [ -f \"$root/.codex/hooks.json\" ] || exit 0; jq -e \"any(.hooks.SessionStart[]?.hooks[]?.command?; type == \\\"string\\\" and contains(\\\"fm-codex-session-lock-hook.sh\\\"))\" \"$root/.codex/hooks.json\" >/dev/null 2>&1 || exit 0; printf \"%s\" \"$payload\" | exec \"$root/bin/fm-codex-session-lock-hook.sh\"'", + "timeout": 3 + } + ] + } + ], "PreToolUse": [ { "matcher": "Bash", @@ -11,6 +22,17 @@ } ] } + ], + "SessionEnd": [ + { + "hooks": [ + { + "type": "command", + "command": "bash -lc 'payload=$(cat 2>/dev/null || true); [ -n \"$payload\" ] || exit 0; command -v jq >/dev/null 2>&1 || exit 0; root=$(pwd -P) || exit 0; [ -x \"$root/bin/fm-codex-session-lock-hook.sh\" ] || exit 0; [ -f \"$root/AGENTS.md\" ] || exit 0; [ -f \"$root/.codex/hooks.json\" ] || exit 0; jq -e \"any(.hooks.SessionEnd[]?.hooks[]?.command?; type == \\\"string\\\" and contains(\\\"fm-codex-session-lock-hook.sh\\\"))\" \"$root/.codex/hooks.json\" >/dev/null 2>&1 || exit 0; printf \"%s\" \"$payload\" | exec \"$root/bin/fm-codex-session-lock-hook.sh\"'", + "timeout": 3 + } + ] + } ] } } diff --git a/bin/fm-codex-session-lock-hook.sh b/bin/fm-codex-session-lock-hook.sh new file mode 100755 index 00000000000..18e52f8e56d --- /dev/null +++ b/bin/fm-codex-session-lock-hook.sh @@ -0,0 +1,62 @@ +#!/usr/bin/env bash +# Codex SessionStart/SessionEnd adapter for the per-home session lock. +# Usage: | fm-codex-session-lock-hook.sh +# +# SessionStart claims the lock before the first model turn so a visible Codex +# ancestry PID is retained when available. A later PID-isolated tool call from +# the same thread preserves that owner instead of replacing it with a transient +# fallback PID. Failure to claim stays silent because fm-session-start.sh owns +# the complete read-only diagnostic. +# +# SessionEnd removes only a regular lock whose Codex thread marker exactly +# matches the ending session. The comparison and removal run under the same +# acquisition lock as fm-lock.sh. A missing, malformed, unreadable, symlinked, +# differently owned, or concurrently busy lock is left untouched. Codex allows +# at most three seconds for SessionEnd hooks, so this adapter never waits for a +# busy acquisition lock and never delays a clean /quit. +set -u + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +FM_ROOT="${FM_ROOT_OVERRIDE:-$(cd "$SCRIPT_DIR/.." && pwd)}" +FM_HOME="${FM_HOME:-${FM_ROOT_OVERRIDE:-$FM_ROOT}}" +STATE="${FM_STATE_OVERRIDE:-$FM_HOME/state}" +LOCK="$STATE/.lock" +PAYLOAD=$(cat 2>/dev/null || true) + +command -v jq >/dev/null 2>&1 || exit 0 +EVENT=$(printf '%s' "$PAYLOAD" | jq -r '.hook_event_name // empty' 2>/dev/null) || exit 0 +SESSION_ID=$(printf '%s' "$PAYLOAD" | jq -r '.session_id // empty' 2>/dev/null) || exit 0 +[ -n "$SESSION_ID" ] || exit 0 +case "$SESSION_ID" in *[!A-Za-z0-9._:-]*) exit 0 ;; esac +if [ -n "${CODEX_THREAD_ID:-}" ] && [ "$CODEX_THREAD_ID" != "$SESSION_ID" ]; then + exit 0 +fi +export CODEX_THREAD_ID="$SESSION_ID" + +if [ "$EVENT" = SessionStart ]; then + "$SCRIPT_DIR/fm-lock.sh" >/dev/null 2>&1 || true + exit 0 +fi +[ "$EVENT" = SessionEnd ] || exit 0 +[ -e "$LOCK" ] || [ -L "$LOCK" ] || exit 0 + +# shellcheck source=bin/fm-wake-lib.sh +. "$SCRIPT_DIR/fm-wake-lib.sh" +CLAIM_LOCK="$STATE/.lock.acquire" +if ! fm_lock_try_acquire "$CLAIM_LOCK"; then + exit 0 +fi +release_claim_lock() { + fm_lock_release "$CLAIM_LOCK" +} +trap release_claim_lock EXIT +trap 'exit 0' HUP INT TERM + +[ -f "$LOCK" ] && [ ! -L "$LOCK" ] || exit 0 +OWNER=$(cat "$LOCK" 2>/dev/null) || exit 0 +# shellcheck source=bin/fm-session-lock-lib.sh +. "$SCRIPT_DIR/fm-session-lock-lib.sh" +MARKER=$(fm_codex_owner_marker "$OWNER" 2>/dev/null || true) +[ -n "$(fm_codex_owner_kind "$OWNER" 2>/dev/null || true)" ] || exit 0 +[ "$MARKER" = "$SESSION_ID" ] || exit 0 +rm -f "$LOCK" 2>/dev/null || true diff --git a/bin/fm-harness.sh b/bin/fm-harness.sh index f39f8f914da..44829343009 100755 --- a/bin/fm-harness.sh +++ b/bin/fm-harness.sh @@ -27,6 +27,9 @@ detect_own() { # It does NOT set CLAUDECODE despite being Claude-Code-compatible, so this marker # is unambiguous when firstmate runs natively on grok. [ "${GROK_AGENT:-}" = "1" ] && { echo grok; return; } + # Codex exposes a stable per-session marker to tool processes. Grok must stay + # ahead of this check because its child environment may inherit that marker. + [ -n "${CODEX_THREAD_ID:-}" ] && { echo codex; return; } # Layer 2: walk the parent chain and match the command name. local pid=$$ comm args for _ in 1 2 3 4 5 6 7 8; do diff --git a/bin/fm-lock.sh b/bin/fm-lock.sh index 083675b2beb..5dd03c8c665 100755 --- a/bin/fm-lock.sh +++ b/bin/fm-lock.sh @@ -1,8 +1,7 @@ #!/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. +# Acquire or inspect the per-home Firstmate session lock. +# Writes a verified harness PID. Codex adds its stable thread marker and whether +# the PID came from verified ancestry or a PID-isolated fallback. # Usage: fm-lock.sh acquire; exit 1 unless ownership is verified # fm-lock.sh status print holder and liveness; always exits 0 set -u @@ -17,23 +16,30 @@ mkdir -p "$STATE" 2>/dev/null || { exit 1 } -# Harness identity (FM_HARNESS_RE, ancestry walk, holder liveness) is owned by -# the shared session-lock lib so the Claude Stop auto-arm applies the exact -# same identity contract. # shellcheck source=bin/fm-session-lock-lib.sh . "$SCRIPT_DIR/fm-session-lock-lib.sh" -if [ "${1:-}" = "status" ]; then +if [ "${1:-}" = status ]; then if [ ! -f "$LOCK" ]; then echo "lock: free"; exit 0; fi - old=$(cat "$LOCK" 2>/dev/null) || { - echo "lock: unreadable" - exit 0 - } - if fm_harness_pid_alive "$old"; then echo "lock: held by live harness pid $old"; else echo "lock: stale (pid $old dead or not a harness)"; fi + old=$(cat "$LOCK" 2>/dev/null) || { echo "lock: unreadable"; exit 0; } + fm_session_lock_holder_state "$old" + holder_status=$? + case "$holder_status" in + 0) case "$old" in + *'|codex:'*) echo "lock: held by live Codex session owner $old" ;; + *) echo "lock: held by live harness pid $old" ;; + esac ;; + 1) echo "lock: stale (owner $old dead or not a harness)" ;; + 2) echo "lock: held by unverifiable Codex session owner $old" ;; + *) echo "lock: invalid owner record; manual inspection required" ;; + esac exit 0 fi -me=$(fm_harness_ancestry_pid) || { echo "error: cannot locate harness process in ancestry" >&2; exit 1; } +owner=$(fm_session_lock_owner) || { + echo "error: cannot locate harness process in ancestry" >&2 + exit 1 +} probe=$(mktemp "$STATE/.lock-write.XXXXXX" 2>/dev/null) || { echo "error: cannot write session lock; operate read-only until resolved" >&2 exit 1 @@ -66,12 +72,27 @@ if [ -e "$LOCK" ] || [ -L "$LOCK" ]; then echo "error: session lock is unreadable; operate read-only until resolved" >&2 exit 1 } - if [ "$old" != "$me" ] && fm_harness_pid_alive "$old"; then - echo "error: another live firstmate session holds the lock (pid $old); operate read-only until resolved" >&2 - exit 1 + old_marker=$(fm_codex_owner_marker "$old" 2>/dev/null || true) + owner_marker=$(fm_codex_owner_marker "$owner" 2>/dev/null || true) + if [ -n "$old_marker" ] && [ "$old_marker" = "$owner_marker" ]; then + owner=$old + elif [ "$old" != "$owner" ]; then + fm_session_lock_holder_state "$old" + holder_status=$? + case "$holder_status" in + 0) + echo "error: another live firstmate session holds the lock (owner $old); operate read-only until resolved" >&2 + exit 1 ;; + 2) + echo "error: cannot verify whether another Codex session holds the lock (owner $old); operate read-only until resolved" >&2 + exit 1 ;; + 3) + echo "error: session lock has an invalid owner record; operate read-only until resolved" >&2 + exit 1 ;; + esac fi fi -if ! { printf '%s\n' "$me" > "$LOCK"; } 2>/dev/null; then +if ! { printf '%s\n' "$owner" > "$LOCK"; } 2>/dev/null; then echo "error: cannot write session lock; operate read-only until resolved" >&2 exit 1 fi @@ -79,9 +100,12 @@ written=$(cat "$LOCK" 2>/dev/null) || { echo "error: cannot verify session lock ownership; operate read-only until resolved" >&2 exit 1 } -if [ ! -f "$LOCK" ] || [ -L "$LOCK" ] || [ "$written" != "$me" ]; then +if [ ! -f "$LOCK" ] || [ -L "$LOCK" ] || [ "$written" != "$owner" ]; then echo "error: session lock ownership verification failed; operate read-only until resolved" >&2 exit 1 fi release_claim_lock -echo "lock acquired: harness pid $me" +case "$owner" in + *'|codex:'*) echo "lock acquired: Codex session owner $owner" ;; + *) echo "lock acquired: harness pid $owner" ;; +esac diff --git a/bin/fm-session-lock-lib.sh b/bin/fm-session-lock-lib.sh index abc54e62853..aa8ec279c85 100644 --- a/bin/fm-session-lock-lib.sh +++ b/bin/fm-session-lock-lib.sh @@ -1,38 +1,56 @@ #!/usr/bin/env bash # Shared session-lock harness identity. # -# ONE owner of the "which verified-harness process holds this home's session -# lock, and does the current process descend from that same harness?" decision. -# bin/fm-lock.sh uses it to acquire and inspect state/.lock; -# bin/fm-claude-stop-autoarm.sh uses it to prove a Stop hook fires inside the -# lock-owning primary session before it may arm or rewake. +# ONE owner of the "which verified-harness session holds this home's session +# lock, and does the current process belong to that same session?" decision. +# Codex owners use one of these formats: +# |codex:|harness +# |codex:|fallback +# `harness` means the PID was verified from ancestry and can prove liveness. +# `fallback` means PID isolation hid the harness, so only the thread marker can +# prove same-session ownership. Legacy two-field Codex owners remain readable +# and fail closed. # This file is sourced by scripts and has no side effects on source. -# Known harness command names; extend when a new adapter is verified. FM_HARNESS_RE='claude|codex|opencode|grok|^pi$' -# Walk the current process ancestry (up to 8 hops) and print the first pid whose -# command looks like a verified harness. The harness pid lives as long as the -# session, unlike the transient subshell pid of any one tool call. -fm_harness_ancestry_pid() { +fm_verified_harness_ancestry_pid() { local pid=$$ comm args for _ in 1 2 3 4 5 6 7 8; do - comm=$(ps -o comm= -p "$pid" 2>/dev/null) || return 1 + comm=$(ps -o comm= -p "$pid" 2>/dev/null) || break args=$(ps -o args= -p "$pid" 2>/dev/null) if printf '%s' "$(basename "$comm")" | grep -qE "$FM_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 "$FM_HARNESS_RE" && { echo "$pid"; return 0; } ;; esac pid=$(ps -o ppid= -p "$pid" 2>/dev/null | tr -d ' ') - [ -n "$pid" ] && [ "$pid" -gt 1 ] || return 1 + if [ -z "$pid" ] || [ "$pid" -le 1 ]; then + break + fi done return 1 } -# True if $1 is a live process that looks like a verified harness. +# Compatibility name for existing callers in older JT worktrees. +fm_harness_ancestry_pid() { + fm_verified_harness_ancestry_pid +} + +fm_session_lock_owner() { + local pid + if [ "${GROK_AGENT:-}" != "1" ] && [ -n "${CODEX_THREAD_ID:-}" ]; then + if pid=$(fm_verified_harness_ancestry_pid); then + printf '%s|codex:%s|harness\n' "$pid" "$CODEX_THREAD_ID" + else + printf '%s|codex:%s|fallback\n' "$$" "$CODEX_THREAD_ID" + fi + return 0 + fi + fm_verified_harness_ancestry_pid +} + fm_harness_pid_alive() { local pid=$1 comm kill -0 "$pid" 2>/dev/null || return 1 @@ -40,16 +58,69 @@ fm_harness_pid_alive() { printf '%s' "$(basename "$comm") $(ps -o args= -p "$pid" 2>/dev/null)" | grep -qE "$FM_HARNESS_RE" } -# True when state dir $1 holds a session lock whose pid is the harness ancestor -# of the current process: this script runs inside the session that owns the -# home's fleet lock. A missing lock, a lock held by another live harness, or an -# ancestry that cannot be resolved all fail closed. -fm_session_lock_owned_by_self() { - local state=$1 lock_pid my_pid - lock_pid=$(cat "$state/.lock" 2>/dev/null || true) - case "$lock_pid" in - ''|*[!0-9]*) return 1 ;; +fm_codex_owner_marker() { + local owner=$1 pid rest marker suffix + pid=${owner%%|*} + case "$pid" in ''|*[!0-9]*) return 1 ;; esac + rest=${owner#*|} + case "$rest" in codex:*) rest=${rest#codex:} ;; *) return 1 ;; esac + marker=${rest%%|*} + case "$marker" in ''|*[!A-Za-z0-9._:-]*) return 1 ;; esac + if [ "$rest" != "$marker" ]; then + suffix=${rest#*|} + case "$suffix" in harness|fallback) ;; *) return 1 ;; esac + fi + printf '%s\n' "$marker" +} + +fm_codex_owner_kind() { + local owner=$1 rest marker suffix + fm_codex_owner_marker "$owner" >/dev/null || return 1 + rest=${owner#*|codex:} + marker=${rest%%|*} + if [ "$rest" = "$marker" ]; then + printf '%s\n' legacy + return 0 + fi + suffix=${rest#*|} + printf '%s\n' "$suffix" +} + +# Return 0 when owner $1 is live or belongs to the current Codex thread, 1 when +# it is provably stale, 2 when another Codex thread cannot verify it, and 3 for +# an invalid owner record. +fm_session_lock_holder_state() { + local owner=$1 pid marker kind + case "$owner" in + *'|codex:'*) + pid=${owner%%|*} + case "$pid" in ''|*[!0-9]*) return 3 ;; esac + marker=$(fm_codex_owner_marker "$owner") || return 3 + if [ -n "${CODEX_THREAD_ID:-}" ] && [ "$CODEX_THREAD_ID" = "$marker" ]; then + return 0 + fi + kind=$(fm_codex_owner_kind "$owner") || return 3 + if [ "$kind" = harness ]; then + fm_harness_pid_alive "$pid" + return $? + fi + return 2 + ;; + *) + case "$owner" in ''|*[!0-9]*) return 3 ;; esac + fm_harness_pid_alive "$owner" + ;; esac - my_pid=$(fm_harness_ancestry_pid) || return 1 - [ "$my_pid" = "$lock_pid" ] +} + +fm_session_lock_owned_by_self() { + local state=$1 owner marker my_pid + owner=$(cat "$state/.lock" 2>/dev/null || true) + if marker=$(fm_codex_owner_marker "$owner"); then + [ -n "${CODEX_THREAD_ID:-}" ] && [ "$CODEX_THREAD_ID" = "$marker" ] + return $? + fi + case "$owner" in ''|*[!0-9]*) return 1 ;; esac + my_pid=$(fm_verified_harness_ancestry_pid) || return 1 + [ "$my_pid" = "$owner" ] } diff --git a/docs/configuration.md b/docs/configuration.md index 5ee83045ff5..35122161136 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -303,6 +303,12 @@ Ship/scout panes export `FM_CBM_TASK_ID` and `FM_CBM_CLI` when CBM env injection Runtime tuning via environment variables (defaults shown): +Codex primaries use the tracked `SessionStart` and `SessionEnd` hooks to claim +and release only their own home's session lock. Structured Codex owners retain +the stable `CODEX_THREAD_ID` across PID-isolated tool calls. Numeric lock files +remain supported for other harnesses and older homes, and `GROK_AGENT=1` takes +precedence over an inherited Codex marker. + ```sh FM_HOME= # optional operational home; unset means this repo root FM_ROOT_OVERRIDE= # override firstmate repo root and tangle-guard target; also legacy whole-root override when FM_HOME is unset diff --git a/docs/scripts.md b/docs/scripts.md index e2a087f1626..9bbdf769227 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -65,7 +65,8 @@ Each file also starts with a short header comment. | `fm-promote.sh` | Promote a scout task in place to a protected ship task | | `fm-teardown.sh` | Fail-closed teardown: return landed ship worktrees, require completed scout deliverables, retire secondmate homes | | `fm-harness.sh` | Detect the running harness and resolve crew or secondmate harness, model, and effort | -| `fm-lock.sh` | Per-home firstmate session lock | +| `fm-lock.sh` | Per-home session lock with Codex thread ownership and legacy PID support | +| `fm-codex-session-lock-hook.sh` | Bounded Codex SessionStart claim and exact-thread SessionEnd release adapter | | `fm-x-lib.sh` | Shared X-mode config, relay, and reply-threading helpers | | `fm-x-poll.sh` | One bounded X relay poll: stash newly offered mentions and emit their once-only wake | | `fm-x-reply.sh` | Post or dry-run preview a composed X-mode reply or follow-up | diff --git a/tests/fm-codex-session-lock-live-e2e.test.sh b/tests/fm-codex-session-lock-live-e2e.test.sh new file mode 100755 index 00000000000..46752589030 --- /dev/null +++ b/tests/fm-codex-session-lock-live-e2e.test.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +# Opt-in real Codex /quit -> SessionEnd regression. No provider request is made. +set -u + +if [ "${FM_CODEX_LOCK_LIVE_E2E:-0}" != 1 ]; then + echo "skip: set FM_CODEX_LOCK_LIVE_E2E=1 to run the Codex /quit lock regression" + exit 0 +fi + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +fail() { printf 'not ok - %s\n' "$1" >&2; exit 1; } +command -v codex >/dev/null 2>&1 || fail "codex not found" +command -v script >/dev/null 2>&1 || fail "script not found" +command -v timeout >/dev/null 2>&1 || fail "timeout not found" + +LAB="$ROOT/.codex-session-lock-live-e2e.$$" +HOME_DIR="$LAB/fmhome" +TRANSCRIPT="$LAB/codex.typescript" +CODEX_VERSION=$(codex --version) +cleanup() { rm -rf "$LAB"; } +trap cleanup EXIT +mkdir -p "$HOME_DIR/state" + +printf '/quit\r' \ + | timeout 60 env -u CODEX_THREAD_ID FM_HOME="$HOME_DIR" FM_ROOT_OVERRIDE="$ROOT" \ + script -qfec "codex --dangerously-bypass-hook-trust -C '$ROOT'" "$TRANSCRIPT" \ + >/dev/null 2>&1 \ + || fail "Codex /quit session did not close cleanly: $(tail -20 "$TRANSCRIPT" 2>/dev/null)" + +[ ! -e "$HOME_DIR/state/.lock" ] \ + || fail "Codex /quit left the session lock behind: $(cat "$HOME_DIR/state/.lock" 2>/dev/null)" +grep -F '/quit' "$TRANSCRIPT" >/dev/null || fail "Codex transcript did not contain /quit" +printf 'ok - %s live /quit fired SessionEnd and released the per-home lock\n' "$CODEX_VERSION" diff --git a/tests/fm-codex-session-lock.test.sh b/tests/fm-codex-session-lock.test.sh new file mode 100755 index 00000000000..e29695e567e --- /dev/null +++ b/tests/fm-codex-session-lock.test.sh @@ -0,0 +1,251 @@ +#!/usr/bin/env bash +# Focused behavior tests for Codex marker-owned session locks and lifecycle hooks. +set -u + +# shellcheck source=tests/lib.sh disable=SC1091 +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +LOCK="$ROOT/bin/fm-lock.sh" +HOOK="$ROOT/bin/fm-codex-session-lock-hook.sh" +TMP_ROOT=$(fm_test_tmproot fm-codex-session-lock-tests) +BASE_PATH=${FM_TEST_BASE_PATH:-/usr/bin:/bin:/usr/sbin:/sbin} + +make_home() { + local home="$TMP_ROOT/$1" + mkdir -p "$home/state" + printf '%s\n' "$home" +} + +make_hidden_ps() { + local dir=$1 + mkdir -p "$dir" + printf '%s\n' '#!/usr/bin/env bash' 'exit 1' > "$dir/ps" + chmod +x "$dir/ps" +} + +make_live_owner_ps() { + local dir=$1 command_name=${2:-codex} + mkdir -p "$dir" + sed "s/@COMMAND@/$command_name/g" > "$dir/ps" <<'SH' +#!/usr/bin/env bash +set -u +pid= +previous= +for argument in "$@"; do + [ "$previous" = -p ] && pid=$argument + previous=$argument +done +[ "$pid" = "${FM_FAKE_LIVE_PID:-}" ] || exit 1 +case "$*" in + *"comm="*) printf '%s\n' @COMMAND@ ;; + *"args="*) printf '%s\n' @COMMAND@ ;; + *"ppid="*) printf '%s\n' 1 ;; + *) exit 1 ;; +esac +SH + chmod +x "$dir/ps" +} + +make_any_grok_ps() { + local dir=$1 + mkdir -p "$dir" + sed 's/^+//' > "$dir/ps" <<'SH' ++#!/usr/bin/env bash ++case "$*" in ++ *"comm="*) printf '%s\n' grok ;; ++ *"args="*) printf '%s\n' grok ;; ++ *"ppid="*) printf '%s\n' 1 ;; ++ *) exit 1 ;; ++esac +SH + chmod +x "$dir/ps" +} + +run_lock() { + local home=$1 thread=$2 fakebin=$3 + env -u CLAUDECODE -u PI_CODING_AGENT -u GROK_AGENT \ + FM_HOME="$home" CODEX_THREAD_ID="$thread" PATH="$fakebin:$BASE_PATH" \ + bash "$LOCK" +} + +run_hook() { + local home=$1 event=$2 session=$3 + printf '{"hook_event_name":"%s","session_id":"%s","cwd":"%s"}\n' \ + "$event" "$session" "$ROOT" \ + | FM_HOME="$home" CODEX_THREAD_ID="$session" bash "$HOOK" +} + +test_hook_registration_preserves_jt_pretool() { + local command + jq -e '.hooks.PreToolUse[0].hooks[0].command | contains("fm-cd-pretool-check.sh")' \ + "$ROOT/.codex/hooks.json" >/dev/null || fail "JT PreToolUse hook was not preserved" + jq -e '[.hooks.SessionStart[]?.hooks[] | select(.command | contains("fm-codex-session-lock-hook.sh"))] | length == 1' \ + "$ROOT/.codex/hooks.json" >/dev/null || fail "Codex SessionStart lock hook is not registered exactly once" + jq -e '.hooks.SessionEnd | length == 1' "$ROOT/.codex/hooks.json" >/dev/null \ + || fail "Codex SessionEnd hook is not registered exactly once" + command=$(jq -r '.hooks.SessionEnd[0].hooks[0].command' "$ROOT/.codex/hooks.json") + # shellcheck disable=SC2016 + assert_contains "$command" 'root=$(pwd -P)' "Codex SessionEnd hook is not pwd-anchored" + jq -e '.hooks.SessionStart[0].hooks[0].timeout == 3 and .hooks.SessionEnd[0].hooks[0].timeout == 3' \ + "$ROOT/.codex/hooks.json" >/dev/null || fail "Codex lock hooks must use the three-second bound" + pass "Codex lock hooks are bounded and preserve JT PreToolUse" +} + +test_matching_session_end_only_releases_regular_exact_owner() { + local home owner + home=$(make_home exact-release) + printf '%s\n' '999999|codex:thread-clean|fallback' > "$home/state/.lock" + run_hook "$home" SessionEnd thread-clean + [ ! -e "$home/state/.lock" ] || fail "matching SessionEnd left the lock behind" + + for owner in \ + '4321' \ + '4321|codex:thread-other|harness' \ + '4321|codex:thread-clean|unknown' \ + '4321|unexpected|codex:thread-clean|harness' \ + 'not-an-owner'; do + printf '%s\n' "$owner" > "$home/state/.lock" + run_hook "$home" SessionEnd thread-clean + [ "$(cat "$home/state/.lock")" = "$owner" ] || fail "SessionEnd changed disconfirming owner '$owner'" + done + rm -f "$home/state/.lock" + ln -s "$home/state/missing-target" "$home/state/.lock" + run_hook "$home" SessionEnd thread-clean + [ -L "$home/state/.lock" ] || fail "SessionEnd followed or removed a symlinked lock" + pass "SessionEnd removes only the matching regular lock" +} + +test_session_start_retains_verified_harness_owner() { + local home fakecodex owner + home=$(make_home start-owner) + fakecodex="$home/codex" + ln -s /bin/bash "$fakecodex" + # shellcheck disable=SC2016 + FM_HOOK_PATH="$HOOK" FM_HOME="$home" CODEX_THREAD_ID=thread-start \ + "$fakecodex" -c 'printf '\''{"hook_event_name":"SessionStart","session_id":"thread-start"}\n'\'' | bash "$FM_HOOK_PATH"' + owner=$(cat "$home/state/.lock") + case "$owner" in *'|codex:thread-start|harness') ;; *) fail "unexpected SessionStart owner: $owner" ;; esac + pass "SessionStart retains a visible Codex harness PID" +} + +test_same_thread_preserves_existing_owner() { + local home fakebin before out + home=$(make_home same-thread) + fakebin="$home/fakebin" + make_hidden_ps "$fakebin" + before='8123|codex:thread-same|fallback' + printf '%s\n' "$before" > "$home/state/.lock" + out=$(run_lock "$home" thread-same "$fakebin") || fail "same Codex thread could not reacquire: $out" + [ "$(cat "$home/state/.lock")" = "$before" ] || fail "same thread replaced the stable owner" + pass "same Codex thread preserves its owner across PID isolation" +} + +test_dead_verified_owner_is_reclaimed() { + local home fakebin owner + home=$(make_home dead-owner) + fakebin="$home/fakebin" + make_hidden_ps "$fakebin" + printf '%s\n' '99999999|codex:thread-dead|harness' > "$home/state/.lock" + run_lock "$home" thread-new "$fakebin" >/dev/null || fail "dead verified owner was not reclaimed" + owner=$(cat "$home/state/.lock") + case "$owner" in *'|codex:thread-new|fallback') ;; *) fail "unexpected reclaimed owner: $owner" ;; esac + pass "provably dead verified Codex owners are reclaimed" +} + +test_different_threads_remain_excluded() { + local home fakebin sleeper owner out status + home=$(make_home other-thread) + fakebin="$home/fakebin" + sleep 60 & sleeper=$! + make_live_owner_ps "$fakebin" codex + owner="$sleeper|codex:thread-live|harness" + printf '%s\n' "$owner" > "$home/state/.lock" + status=0 + out=$(FM_FAKE_LIVE_PID="$sleeper" run_lock "$home" thread-other "$fakebin" 2>&1) || status=$? + kill "$sleeper" 2>/dev/null || true + wait "$sleeper" 2>/dev/null || true + expect_code 1 "$status" "different live Codex thread must be excluded" + assert_contains "$out" "another live firstmate session holds the lock" "live owner refusal was not explicit" + + make_hidden_ps "$fakebin" + owner='17|codex:thread-hidden|fallback' + printf '%s\n' "$owner" > "$home/state/.lock" + status=0 + out=$(run_lock "$home" thread-other "$fakebin" 2>&1) || status=$? + expect_code 1 "$status" "different thread must not reclaim a fallback owner" + assert_contains "$out" "cannot verify whether another Codex session holds the lock" "fallback refusal lost its reason" + pass "different Codex threads stay excluded for live and fallback owners" +} + +test_grok_precedence_and_primary_lock_protection() { + local home fakebin grokbin sleeper out status owner + out=$(env -u CLAUDECODE -u PI_CODING_AGENT GROK_AGENT=1 CODEX_THREAD_ID=inherited-thread \ + FM_ROOT_OVERRIDE="$ROOT" "$ROOT/bin/fm-harness.sh") + [ "$out" = grok ] || fail "Grok did not precede inherited CODEX_THREAD_ID: $out" + + home=$(make_home grok-owner-format) + grokbin="$home/grokbin" + make_any_grok_ps "$grokbin" + env -u CLAUDECODE -u PI_CODING_AGENT GROK_AGENT=1 CODEX_THREAD_ID=inherited-thread \ + FM_HOME="$home" PATH="$grokbin:$BASE_PATH" bash "$LOCK" >/dev/null + owner=$(cat "$home/state/.lock") + case "$owner" in ''|*[!0-9]*) fail "Grok owner was incorrectly structured as Codex: $owner" ;; esac + + home=$(make_home grok-primary) + fakebin="$home/fakebin" + sleep 60 & sleeper=$! + make_live_owner_ps "$fakebin" grok + printf '%s\n' "$sleeper" > "$home/state/.lock" + status=0 + out=$(FM_FAKE_LIVE_PID="$sleeper" run_lock "$home" crewmate-thread "$fakebin" 2>&1) || status=$? + expect_code 1 "$status" "Codex crewmate must not steal a live Grok primary lock" + run_hook "$home" SessionEnd crewmate-thread + [ "$(cat "$home/state/.lock")" = "$sleeper" ] || fail "Codex SessionEnd changed the Grok primary owner" + kill "$sleeper" 2>/dev/null || true + wait "$sleeper" 2>/dev/null || true + pass "Grok precedence prevents Codex crewmates from stealing or releasing the primary lock" +} + +test_two_homes_release_only_their_own_lock() { + local home_a home_b owner_b + home_a=$(make_home home-a) + home_b=$(make_home home-b) + printf '%s\n' '1001|codex:shared-thread|fallback' > "$home_a/state/.lock" + owner_b='1002|codex:shared-thread|fallback' + printf '%s\n' "$owner_b" > "$home_b/state/.lock" + run_hook "$home_a" SessionEnd shared-thread + [ ! -e "$home_a/state/.lock" ] || fail "home A matching lock was not released" + [ "$(cat "$home_b/state/.lock")" = "$owner_b" ] || fail "home A release crossed into home B" + run_hook "$home_b" SessionEnd different-thread + [ "$(cat "$home_b/state/.lock")" = "$owner_b" ] || fail "mismatched thread released home B" + pass "independent homes cannot release each other's lock" +} + +test_numeric_legacy_lock_contract() { + local home fakebin sleeper out status + home=$(make_home numeric-legacy) + fakebin="$home/fakebin" + sleep 60 & sleeper=$! + make_live_owner_ps "$fakebin" grok + printf '%s\n' "$sleeper" > "$home/state/.lock" + status=0 + out=$(FM_FAKE_LIVE_PID="$sleeper" run_lock "$home" new-thread "$fakebin" 2>&1) || status=$? + expect_code 1 "$status" "live numeric legacy owner must stay exclusive" + run_hook "$home" SessionEnd new-thread + [ "$(cat "$home/state/.lock")" = "$sleeper" ] || fail "SessionEnd removed a numeric legacy owner" + kill "$sleeper" 2>/dev/null || true + wait "$sleeper" 2>/dev/null || true + make_hidden_ps "$fakebin" + run_lock "$home" new-thread "$fakebin" >/dev/null || fail "dead numeric legacy owner was not reclaimable" + pass "numeric legacy locks preserve live exclusion and stale recovery" +} + +test_hook_registration_preserves_jt_pretool +test_matching_session_end_only_releases_regular_exact_owner +test_session_start_retains_verified_harness_owner +test_same_thread_preserves_existing_owner +test_dead_verified_owner_is_reclaimed +test_different_threads_remain_excluded +test_grok_precedence_and_primary_lock_protection +test_two_homes_release_only_their_own_lock +test_numeric_legacy_lock_contract diff --git a/tests/fm-session-start.test.sh b/tests/fm-session-start.test.sh index 5de7a88999a..9c2b8b28d42 100755 --- a/tests/fm-session-start.test.sh +++ b/tests/fm-session-start.test.sh @@ -20,6 +20,10 @@ # does not reimplement their logic set -u +# This suite supplies harness identity per case. Ambient Codex identity must not +# turn direct lock cases into same-thread acquisitions. +unset CODEX_THREAD_ID 2>/dev/null || true + # shellcheck source=tests/lib.sh . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" # shellcheck source=tests/wake-helpers.sh @@ -406,13 +410,12 @@ SH # run_session_start # Drop every harness env marker from bin/fm-harness.sh detect_own so the # surrounding interactive shell cannot leak past the suite's fake ps harness. -# Markers today: CLAUDECODE (claude), PI_CODING_AGENT (pi), GROK_AGENT (grok). -# codex and opencode have no env markers (ancestry only). Without this, a local -# claude/pi/grok session fails cases that pin a different fake harness while CI -# (no ambient markers) still passes. +# Markers today: CLAUDECODE (claude), PI_CODING_AGENT (pi), CODEX_THREAD_ID +# (codex), and GROK_AGENT (grok). Without this, a local harness marker can leak +# into cases that pin a different fake harness. run_session_start() { local home=$1 root=$2 path=$3 - env -u CLAUDECODE -u PI_CODING_AGENT -u GROK_AGENT \ + env -u CLAUDECODE -u PI_CODING_AGENT -u CODEX_THREAD_ID -u GROK_AGENT \ FM_BACKEND="${FM_BACKEND:-}" FM_HOME="$home" FM_ROOT_OVERRIDE="$root" PATH="$path" \ "$SESSION_START" } From 0f9472019347926aaa37692d3b2fd21fa1d6a2a7 Mon Sep 17 00:00:00 2001 From: JTInventory Date: Mon, 27 Jul 2026 14:56:51 +0000 Subject: [PATCH 2/4] no-mistakes(review): Fix Codex session-lock lifecycle compatibility --- .codex/hooks.json | 4 +- bin/fm-codex-session-lock-hook.sh | 23 +++++++-- bin/fm-lock.sh | 2 + bin/fm-session-lock-lib.sh | 13 +++-- tests/fm-codex-session-lock.test.sh | 74 ++++++++++++++++++++++++++++- 5 files changed, 107 insertions(+), 9 deletions(-) diff --git a/.codex/hooks.json b/.codex/hooks.json index 140cf82e405..5e99427f225 100644 --- a/.codex/hooks.json +++ b/.codex/hooks.json @@ -5,7 +5,7 @@ "hooks": [ { "type": "command", - "command": "bash -lc 'payload=$(cat 2>/dev/null || true); [ -n \"$payload\" ] || exit 0; command -v jq >/dev/null 2>&1 || exit 0; root=$(pwd -P) || exit 0; [ -x \"$root/bin/fm-codex-session-lock-hook.sh\" ] || exit 0; [ -f \"$root/AGENTS.md\" ] || exit 0; [ -f \"$root/.codex/hooks.json\" ] || exit 0; jq -e \"any(.hooks.SessionStart[]?.hooks[]?.command?; type == \\\"string\\\" and contains(\\\"fm-codex-session-lock-hook.sh\\\"))\" \"$root/.codex/hooks.json\" >/dev/null 2>&1 || exit 0; printf \"%s\" \"$payload\" | exec \"$root/bin/fm-codex-session-lock-hook.sh\"'", + "command": "bash -lc 'payload=$(cat 2>/dev/null || true); [ -n \"$payload\" ] || exit 0; command -v node >/dev/null 2>&1 || exit 0; root=$(pwd -P) || exit 0; [ -x \"$root/bin/fm-codex-session-lock-hook.sh\" ] || exit 0; [ -f \"$root/AGENTS.md\" ] || exit 0; [ -f \"$root/.codex/hooks.json\" ] || exit 0; node -e \"const fs=require(\\\"fs\\\");const event=process.argv[1];try{const doc=JSON.parse(fs.readFileSync(process.argv[2],\\\"utf8\\\"));const entries=doc?.hooks?.[event];if(!Array.isArray(entries)||!entries.some(group=>Array.isArray(group?.hooks)&&group.hooks.some(hook=>typeof hook?.command===\\\"string\\\"&&hook.command.includes(\\\"fm-codex-session-lock-hook.sh\\\"))))process.exit(1)}catch{process.exit(1)}\" SessionStart \"$root/.codex/hooks.json\" >/dev/null 2>&1 || exit 0; printf \"%s\" \"$payload\" | exec \"$root/bin/fm-codex-session-lock-hook.sh\"'", "timeout": 3 } ] @@ -28,7 +28,7 @@ "hooks": [ { "type": "command", - "command": "bash -lc 'payload=$(cat 2>/dev/null || true); [ -n \"$payload\" ] || exit 0; command -v jq >/dev/null 2>&1 || exit 0; root=$(pwd -P) || exit 0; [ -x \"$root/bin/fm-codex-session-lock-hook.sh\" ] || exit 0; [ -f \"$root/AGENTS.md\" ] || exit 0; [ -f \"$root/.codex/hooks.json\" ] || exit 0; jq -e \"any(.hooks.SessionEnd[]?.hooks[]?.command?; type == \\\"string\\\" and contains(\\\"fm-codex-session-lock-hook.sh\\\"))\" \"$root/.codex/hooks.json\" >/dev/null 2>&1 || exit 0; printf \"%s\" \"$payload\" | exec \"$root/bin/fm-codex-session-lock-hook.sh\"'", + "command": "bash -lc 'payload=$(cat 2>/dev/null || true); [ -n \"$payload\" ] || exit 0; command -v node >/dev/null 2>&1 || exit 0; root=$(pwd -P) || exit 0; [ -x \"$root/bin/fm-codex-session-lock-hook.sh\" ] || exit 0; [ -f \"$root/AGENTS.md\" ] || exit 0; [ -f \"$root/.codex/hooks.json\" ] || exit 0; node -e \"const fs=require(\\\"fs\\\");const event=process.argv[1];try{const doc=JSON.parse(fs.readFileSync(process.argv[2],\\\"utf8\\\"));const entries=doc?.hooks?.[event];if(!Array.isArray(entries)||!entries.some(group=>Array.isArray(group?.hooks)&&group.hooks.some(hook=>typeof hook?.command===\\\"string\\\"&&hook.command.includes(\\\"fm-codex-session-lock-hook.sh\\\"))))process.exit(1)}catch{process.exit(1)}\" SessionEnd \"$root/.codex/hooks.json\" >/dev/null 2>&1 || exit 0; printf \"%s\" \"$payload\" | exec \"$root/bin/fm-codex-session-lock-hook.sh\"'", "timeout": 3 } ] diff --git a/bin/fm-codex-session-lock-hook.sh b/bin/fm-codex-session-lock-hook.sh index 18e52f8e56d..19967268bec 100755 --- a/bin/fm-codex-session-lock-hook.sh +++ b/bin/fm-codex-session-lock-hook.sh @@ -23,9 +23,26 @@ STATE="${FM_STATE_OVERRIDE:-$FM_HOME/state}" LOCK="$STATE/.lock" PAYLOAD=$(cat 2>/dev/null || true) -command -v jq >/dev/null 2>&1 || exit 0 -EVENT=$(printf '%s' "$PAYLOAD" | jq -r '.hook_event_name // empty' 2>/dev/null) || exit 0 -SESSION_ID=$(printf '%s' "$PAYLOAD" | jq -r '.session_id // empty' 2>/dev/null) || exit 0 +command -v node >/dev/null 2>&1 || exit 0 +PARSED=$(printf '%s' "$PAYLOAD" | node -e ' +let payload = ""; +process.stdin.setEncoding("utf8"); +process.stdin.on("data", chunk => payload += chunk); +process.stdin.on("end", () => { + try { + const value = JSON.parse(payload); + if (!value || Array.isArray(value) + || typeof value.hook_event_name !== "string" + || typeof value.session_id !== "string") process.exit(1); + process.stdout.write(value.hook_event_name + "\n" + value.session_id); + } catch { + process.exit(1); + } +}); +' 2>/dev/null) || exit 0 +case "$PARSED" in *$'\n'*) ;; *) exit 0 ;; esac +EVENT=${PARSED%%$'\n'*} +SESSION_ID=${PARSED#*$'\n'} [ -n "$SESSION_ID" ] || exit 0 case "$SESSION_ID" in *[!A-Za-z0-9._:-]*) exit 0 ;; esac if [ -n "${CODEX_THREAD_ID:-}" ] && [ "$CODEX_THREAD_ID" != "$SESSION_ID" ]; then diff --git a/bin/fm-lock.sh b/bin/fm-lock.sh index 5dd03c8c665..9f2bae8bbfa 100755 --- a/bin/fm-lock.sh +++ b/bin/fm-lock.sh @@ -76,6 +76,8 @@ if [ -e "$LOCK" ] || [ -L "$LOCK" ]; then owner_marker=$(fm_codex_owner_marker "$owner" 2>/dev/null || true) if [ -n "$old_marker" ] && [ "$old_marker" = "$owner_marker" ]; then owner=$old + elif [ "$old" = "${owner%%|*}" ] && [ -n "$owner_marker" ]; then + : elif [ "$old" != "$owner" ]; then fm_session_lock_holder_state "$old" holder_status=$? diff --git a/bin/fm-session-lock-lib.sh b/bin/fm-session-lock-lib.sh index aa8ec279c85..89976c2e139 100644 --- a/bin/fm-session-lock-lib.sh +++ b/bin/fm-session-lock-lib.sh @@ -38,9 +38,16 @@ fm_harness_ancestry_pid() { fm_verified_harness_ancestry_pid } +fm_codex_thread_active() { + [ "${CLAUDECODE:-}" != "1" ] \ + && [ "${PI_CODING_AGENT:-}" != "true" ] \ + && [ "${GROK_AGENT:-}" != "1" ] \ + && [ -n "${CODEX_THREAD_ID:-}" ] +} + fm_session_lock_owner() { local pid - if [ "${GROK_AGENT:-}" != "1" ] && [ -n "${CODEX_THREAD_ID:-}" ]; then + if fm_codex_thread_active; then if pid=$(fm_verified_harness_ancestry_pid); then printf '%s|codex:%s|harness\n' "$pid" "$CODEX_THREAD_ID" else @@ -96,7 +103,7 @@ fm_session_lock_holder_state() { pid=${owner%%|*} case "$pid" in ''|*[!0-9]*) return 3 ;; esac marker=$(fm_codex_owner_marker "$owner") || return 3 - if [ -n "${CODEX_THREAD_ID:-}" ] && [ "$CODEX_THREAD_ID" = "$marker" ]; then + if fm_codex_thread_active && [ "$CODEX_THREAD_ID" = "$marker" ]; then return 0 fi kind=$(fm_codex_owner_kind "$owner") || return 3 @@ -117,7 +124,7 @@ fm_session_lock_owned_by_self() { local state=$1 owner marker my_pid owner=$(cat "$state/.lock" 2>/dev/null || true) if marker=$(fm_codex_owner_marker "$owner"); then - [ -n "${CODEX_THREAD_ID:-}" ] && [ "$CODEX_THREAD_ID" = "$marker" ] + fm_codex_thread_active && [ "$CODEX_THREAD_ID" = "$marker" ] return $? fi case "$owner" in ''|*[!0-9]*) return 1 ;; esac diff --git a/tests/fm-codex-session-lock.test.sh b/tests/fm-codex-session-lock.test.sh index e29695e567e..134cd957ebb 100755 --- a/tests/fm-codex-session-lock.test.sh +++ b/tests/fm-codex-session-lock.test.sh @@ -61,6 +61,32 @@ SH chmod +x "$dir/ps" } +make_codex_parent_ps() { + local dir=$1 + mkdir -p "$dir" + sed 's/^+//' > "$dir/ps" <<'SH' ++#!/usr/bin/env bash ++set -u ++pid= ++previous= ++for argument in "$@"; do ++ [ "$previous" = -p ] && pid=$argument ++ previous=$argument ++done ++case "$*" in ++ *"comm="*) ++ if [ "$pid" = "${FM_FAKE_CODEX_PID:?}" ]; then printf '%s\n' codex; else printf '%s\n' bash; fi ++ ;; ++ *"args="*) ++ if [ "$pid" = "${FM_FAKE_CODEX_PID:?}" ]; then printf '%s\n' codex; else printf '%s\n' bash; fi ++ ;; ++ *"ppid="*) printf '%s\n' "${FM_FAKE_CODEX_PID:?}" ;; ++ *) exit 1 ;; ++esac +SH + chmod +x "$dir/ps" +} + run_lock() { local home=$1 thread=$2 fakebin=$3 env -u CLAUDECODE -u PI_CODING_AGENT -u GROK_AGENT \ @@ -86,11 +112,28 @@ test_hook_registration_preserves_jt_pretool() { command=$(jq -r '.hooks.SessionEnd[0].hooks[0].command' "$ROOT/.codex/hooks.json") # shellcheck disable=SC2016 assert_contains "$command" 'root=$(pwd -P)' "Codex SessionEnd hook is not pwd-anchored" + case "$command" in *jq*) fail "Codex SessionEnd hook still requires jq" ;; esac jq -e '.hooks.SessionStart[0].hooks[0].timeout == 3 and .hooks.SessionEnd[0].hooks[0].timeout == 3' \ "$ROOT/.codex/hooks.json" >/dev/null || fail "Codex lock hooks must use the three-second bound" pass "Codex lock hooks are bounded and preserve JT PreToolUse" } +test_hooks_work_when_jq_fails() { + local home fakebin + home=$(make_home no-jq) + fakebin="$home/fakebin" + mkdir -p "$fakebin" + printf '%s\n' '#!/usr/bin/env bash' 'exit 127' > "$fakebin/jq" + chmod +x "$fakebin/jq" + printf '{"hook_event_name":"SessionStart","session_id":"thread-no-jq"}\n' \ + | FM_HOME="$home" CODEX_THREAD_ID=thread-no-jq PATH="$fakebin:$BASE_PATH" bash "$HOOK" + [ -f "$home/state/.lock" ] || fail "SessionStart did not acquire without jq" + printf '{"hook_event_name":"SessionEnd","session_id":"thread-no-jq"}\n' \ + | FM_HOME="$home" CODEX_THREAD_ID=thread-no-jq PATH="$fakebin:$BASE_PATH" bash "$HOOK" + [ ! -e "$home/state/.lock" ] || fail "SessionEnd did not release without jq" + pass "Codex lifecycle hooks do not depend on jq" +} + test_matching_session_end_only_releases_regular_exact_owner() { local home owner home=$(make_home exact-release) @@ -206,6 +249,24 @@ test_grok_precedence_and_primary_lock_protection() { pass "Grok precedence prevents Codex crewmates from stealing or releasing the primary lock" } +test_non_codex_markers_precede_inherited_thread() { + local fakebin out marker value + fakebin="$(make_home marker-precedence)/fakebin" + make_any_grok_ps "$fakebin" + for marker in CLAUDECODE PI_CODING_AGENT GROK_AGENT; do + case "$marker" in + CLAUDECODE) value=1 ;; + PI_CODING_AGENT) value=true ;; + GROK_AGENT) value=1 ;; + esac + out=$(env -u CLAUDECODE -u PI_CODING_AGENT -u GROK_AGENT \ + "$marker=$value" CODEX_THREAD_ID=inherited-thread PATH="$fakebin:$BASE_PATH" \ + bash -c '. "$1"; fm_session_lock_owner' _ "$ROOT/bin/fm-session-lock-lib.sh") + case "$out" in ''|*[!0-9]*) fail "$marker inherited Codex ownership: $out" ;; esac + done + pass "Claude, Pi, and Grok markers precede inherited Codex threads" +} + test_two_homes_release_only_their_own_lock() { local home_a home_b owner_b home_a=$(make_home home-a) @@ -222,7 +283,7 @@ test_two_homes_release_only_their_own_lock() { } test_numeric_legacy_lock_contract() { - local home fakebin sleeper out status + local home fakebin sleeper out status owner home=$(make_home numeric-legacy) fakebin="$home/fakebin" sleep 60 & sleeper=$! @@ -233,6 +294,15 @@ test_numeric_legacy_lock_contract() { expect_code 1 "$status" "live numeric legacy owner must stay exclusive" run_hook "$home" SessionEnd new-thread [ "$(cat "$home/state/.lock")" = "$sleeper" ] || fail "SessionEnd removed a numeric legacy owner" + + make_codex_parent_ps "$fakebin" + FM_FAKE_CODEX_PID="$sleeper" run_lock "$home" same-session "$fakebin" >/dev/null \ + || fail "same session could not upgrade its numeric legacy lock" + owner=$(cat "$home/state/.lock") + [ "$owner" = "$sleeper|codex:same-session|harness" ] \ + || fail "numeric legacy lock was not upgraded to the structured owner: $owner" + printf '%s\n' "$sleeper" > "$home/state/.lock" + kill "$sleeper" 2>/dev/null || true wait "$sleeper" 2>/dev/null || true make_hidden_ps "$fakebin" @@ -241,11 +311,13 @@ test_numeric_legacy_lock_contract() { } test_hook_registration_preserves_jt_pretool +test_hooks_work_when_jq_fails test_matching_session_end_only_releases_regular_exact_owner test_session_start_retains_verified_harness_owner test_same_thread_preserves_existing_owner test_dead_verified_owner_is_reclaimed test_different_threads_remain_excluded test_grok_precedence_and_primary_lock_protection +test_non_codex_markers_precede_inherited_thread test_two_homes_release_only_their_own_lock test_numeric_legacy_lock_contract From f828d7953db50c82d69fe5d7fd2e8437079ecf57 Mon Sep 17 00:00:00 2001 From: JTInventory Date: Mon, 27 Jul 2026 15:23:00 +0000 Subject: [PATCH 3/4] no-mistakes(document): Document Codex session-lock lifecycle ownership --- .agents/skills/harness-adapters/SKILL.md | 13 +++++++------ AGENTS.md | 6 ++++-- docs/configuration.md | 21 +++++++++++++++------ 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/.agents/skills/harness-adapters/SKILL.md b/.agents/skills/harness-adapters/SKILL.md index 2da3e8d4900..6958292ef87 100644 --- a/.agents/skills/harness-adapters/SKILL.md +++ b/.agents/skills/harness-adapters/SKILL.md @@ -115,12 +115,13 @@ The decision persists for the repo, so later worktrees of the same project skip Resume after exit with `codex resume `. The session id is printed on quit. -**Primary-session guard fact (verified 2026-07-08, codex-cli 0.142.1).** -The firstmate PRIMARY's own `.codex/hooks.json` registers a Stop hook that pipes Codex's Stop payload to `bin/fm-turnend-guard.sh`. -Codex Stop hooks block on exit 2 and expose `stop_hook_active` for the same one-block loop safety Claude uses. -Codex's Stop payload includes `cwd`, but the tracked primary hook does not use it to choose the guard executable. -Verified on 2026-07-08: Codex runs the Stop hook command with process PWD set to the hook-loaded project root, and no `CODEX_PROJECT_DIR`, `CODEX_WORKSPACE_ROOT`, or `CODEX_CWD` root variable is set. -The tracked hook anchors to `pwd -P`, verifies that root is firstmate-shaped and hook-bearing, and then invokes `bin/fm-turnend-guard.sh` with the original payload. +**Primary-session hook facts.** +The firstmate PRIMARY's own `.codex/hooks.json` registers the bounded +`SessionStart` and `SessionEnd` lock hooks described in +`docs/configuration.md`, plus the Bash `PreToolUse` primary-directory guard. +These tracked hooks anchor to the hook-loaded project's `pwd -P`. +JT does not auto-wire `bin/fm-turnend-guard.sh` into a Codex Stop hook in +Phase B; the guard remains callable as documented in `docs/turnend-guard.md`. Codex's primary watcher protocol is `bin/fm-watch-checkpoint.sh --seconds "${FM_CODEX_WATCH_CHECKPOINT:-180}"`, not `bin/fm-watch-arm.sh`. The checkpoint is deliberately foreground and bounded so Codex regains control regularly to process user messages and queued wakes. diff --git a/AGENTS.md b/AGENTS.md index 9c5e196b67c..98136da54b4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -144,7 +144,9 @@ Bootstrap is detect, then consent, then install. Never install anything the captain has not approved in this session. Run `bin/fm-session-start.sh` once at every session start. -It acquires the session lock, performs locked stale Herdr projection cleanup, runs `bin/fm-bootstrap.sh`, drains the wake queue, and prints the recovery digest in that order. +It verifies or acquires the session lock, performs locked stale Herdr projection cleanup, runs `bin/fm-bootstrap.sh`, drains the wake queue, and prints the recovery digest in that order. +For Codex, the tracked `SessionStart` hook may already have claimed the lock for this thread before the script runs; `bin/fm-session-start.sh` reuses that owner. +The matching lifecycle and compatibility rules are owned by `docs/configuration.md` under "Codex session lock lifecycle." Do not run `bin/fm-lock.sh` and `bin/fm-bootstrap.sh` separately as the normal startup path. Before checking the toolchain, bootstrap adds existing `$HOME/.nvm/versions/node/*/bin` and `$HOME/.local/bin` directories to `PATH` without moving them ahead of an explicit caller path. The same shared normalization runs before tool lookup in spawn, teardown, and the read-only supervision model, so clean non-interactive shells can find HOME-installed Axi tools consistently. @@ -303,7 +305,7 @@ Load `harness-adapters` before any spawn, recovery, trust-dialog handling, harne You may have been restarted mid-flight. Reconcile reality with your records before doing anything else: -1. Use the lock result printed by `bin/fm-session-start.sh`; it records the harness process PID, which is session-stable. +1. Use the lock result printed by `bin/fm-session-start.sh`; it records the verified per-home session owner. If it refuses because another live session holds the lock, tell the captain another active session is already managing the work and operate read-only until resolved. 2. Keep the wake records drained and printed by `bin/fm-session-start.sh` as the first work queue for this recovery turn. 3. Read `data/backlog.md`, `data/secondmates.md` if present, every `state/*.meta`, and every `state/*.status`. diff --git a/docs/configuration.md b/docs/configuration.md index 35122161136..3f46b1dd6af 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -299,16 +299,25 @@ CLI and index metering do not depend on Codex session logs that get wiped for di Ship/scout panes export `FM_CBM_TASK_ID` and `FM_CBM_CLI` when CBM env injection runs so wrapper lines can tag the task. +## Codex session lock lifecycle + +Codex primaries use the tracked `SessionStart` hook to claim their home's +session lock before the first model turn. The structured owner combines the +stable `CODEX_THREAD_ID` with a verified harness PID when one is visible. +Later PID-isolated calls from the same thread preserve that owner; a different +thread remains excluded. + +The tracked `SessionEnd` hook releases only a regular, non-symlink lock in the +same home whose structured thread marker exactly matches the ending session. +It leaves numeric legacy locks and malformed, unreadable, differently owned, +or concurrently busy locks untouched. Numeric lock files remain supported for +other harnesses and older homes. `GROK_AGENT=1` takes precedence over an +inherited `CODEX_THREAD_ID`, so a Grok primary is never treated as Codex. + ## Environment variables Runtime tuning via environment variables (defaults shown): -Codex primaries use the tracked `SessionStart` and `SessionEnd` hooks to claim -and release only their own home's session lock. Structured Codex owners retain -the stable `CODEX_THREAD_ID` across PID-isolated tool calls. Numeric lock files -remain supported for other harnesses and older homes, and `GROK_AGENT=1` takes -precedence over an inherited Codex marker. - ```sh FM_HOME= # optional operational home; unset means this repo root FM_ROOT_OVERRIDE= # override firstmate repo root and tangle-guard target; also legacy whole-root override when FM_HOME is unset From ed2ed4a7ca46b98a1081c75577e364e2b4c3b31f Mon Sep 17 00:00:00 2001 From: JTInventory Date: Mon, 27 Jul 2026 15:46:55 +0000 Subject: [PATCH 4/4] no-mistakes: apply CI fixes --- tests/fm-codex-session-lock.test.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/fm-codex-session-lock.test.sh b/tests/fm-codex-session-lock.test.sh index 134cd957ebb..818b968c75f 100755 --- a/tests/fm-codex-session-lock.test.sh +++ b/tests/fm-codex-session-lock.test.sh @@ -125,6 +125,7 @@ test_hooks_work_when_jq_fails() { mkdir -p "$fakebin" printf '%s\n' '#!/usr/bin/env bash' 'exit 127' > "$fakebin/jq" chmod +x "$fakebin/jq" + ln -s "$(command -v node)" "$fakebin/node" printf '{"hook_event_name":"SessionStart","session_id":"thread-no-jq"}\n' \ | FM_HOME="$home" CODEX_THREAD_ID=thread-no-jq PATH="$fakebin:$BASE_PATH" bash "$HOOK" [ -f "$home/state/.lock" ] || fail "SessionStart did not acquire without jq"