fix: detect detached Codex session lock holder - #1
Open
Onetech2 wants to merge 4 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Allow a detached Codex tool runner to acquire the Firstmate session lock only when exactly one live harness is rooted in the Firstmate repo, while preserving fail-closed behavior for ambiguous or unrelated harnesses.
What Changed
Risk Assessment
🚨 High: Captain, the fallback fixes the previously reported candidate-selection defects, but an unrelated interpreter ancestor can still bypass the new rooted, exactly-one-harness boundary and acquire the session lock.
Testing
Completed 1 recorded test check.
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-lock.sh:41- This contradicts the required “only when exactly one live harness” behavior: whenever any direct harness exists, all interpreter-hosted candidates are discarded. A directcodexprocess and a separate Node-hostedclaudeprocess rooted here therefore produce one candidate and acquire the lock despite two live harnesses. Count all identified harnesses at this shared candidate-selection boundary; suppress an interpreter only when it is proven to wrap the same direct harness.bin/fm-lock.sh:33- This contradicts the requirement to fail closed for unrelated harnesses. The argument regex treats any repo-rooted Node/Python process with a standalone argument such as--model codexas a harness; if it is the only match, the detached runner writes that unrelated PID into the lock. Identify the interpreter's executable/script entrypoint rather than searching its complete argument list for a harness word.🔧 Fix: Captain, fix detached harness candidate detection
1 error still open:
bin/fm-lock.sh:76- The required “only when exactly one live harness is rooted in the Firstmate repo” invariant remains bypassable: a detached runner whose ancestry includes an unrelated Node/Python process with an argument such as--model codexreturns that ancestor immediately, without checking its cwd or callingroot_harness_pid(). Restrict interpreter ancestry detection to the executable/script entrypoint (using the same semantics as the fallback) so unrelated ancestors fail closed.command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"🔧 Fix: Captain, stabilize backend and session-start tests
1 error still open:
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.