Repository navigation
fix: safely recover stale OMP secondmate markers on relaunch - #254
Merged
Merged
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
1 participant
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
Fix the remote OMP second-mate relaunch refusal that prevents Boatmate from restarting after suspension and a Firstmate code update, so InfoConnect builds can run there. The observed sequence was: wake fix PR #251 merged; Firstmate updated from 197f71a to 85a5f00 while Boatmate was suspended and its update was skipped; compute wake and remote doctor restored readiness; the next update synced Boatmate to 85a5f00;
fm-spawn.sh boatmate --secondmatethen refused with “OMP secondmate home has an unowned or malformed primary-integration marker ... refusing duplicate launch.” The likely cause was an older loaded-extension marker and/or marker PID no longer matching the dead lock owner; prove the cause from code and the observed error and reproduce in a fixture with an old-version marker plus distinct dead lock and marker PIDs. AC1 requires that proof in PR text and receipt. AC2 requires a regression reproducing the post-update, post-suspension refusal and preserving refusals for live owners, symlink, non-file, and unverifiable state. AC3 requires PR text with this exact post-merge sequence:bin/fm-compute-wake.sh boatmate;bin/fm-on.sh boatmate fm-remote-doctor.sh --fix;bin/fm-update.sh;bin/fm-on.sh boatmate fm-remote-secondmate-control.sh relaunch boatmate omp - -. The fixture must exercise the remote relaunch owner end to end with distinct dead lock and marker PIDs and an updated extension. Recovery must run throughfm-remote-secondmate-control.sh relaunch; do not clear or bypass the duplicate-launch guard or usefm_omp_clear_stale_runtime_markerson a live or unknown marker. Recover only an owned stale marker after proving owners absent and the marker/runtime/history are valid; live, foreign, or unverifiable marker state must refuse. If the owner path cannot safely recognize the observed marker, report the exact gap rather than widening the guard. Firstmate will gather any live evidence and perform the real Boatmate relaunch after merge; this worker must perform no live Boat actions, no state edits, no sandbox recreation, and no credential output. AC4 requires that safety statement plus a secret scan. Validate with the full No-Mistakes run; drive eligible gates, escalate any ask-user finding under its canonical key, and report the PR URL and green-check outcome when CI first passes.What Changed
Fix OMP secondmate relaunch recovery for an owned stale primary marker after an update. The reported “unowned or malformed primary-integration marker” refusal comes from direct spawn requiring the current extension hash and matching marker/lock PIDs; an older hash or distinct dead PIDs triggers it. The relaunch owner now checks runtime identity, validates older hashes against recorded fast-forward history, and proves both owners absent before retiring runtime markers. Saved sessions remain intact; live, foreign, malformed, symlinked, non-file, or unverifiable state still refuses recovery.
Add regressions for direct-launch refusals and remote owner-path recovery with an updated extension, an old-version marker, and distinct dead lock and marker PIDs. Coverage also checks uncertain process probes and retained-session preservation. Recovery uses fixture homes; live Boat actions, live state edits, sandbox recreation, and credential output remain outside this change’s recovery procedure.
Document the recovery checks. Firstmate should perform the live Boatmate recovery after merge, in this exact order:
Risk Assessment
✅ Low: Recovery is bounded to verified stale markers, independently refuses live owners, and preserves the duplicate-launch guard and retained sessions.
Testing
All live scenarios and targeted OMP/lifecycle regressions passed. The broader restart fixture’s sync failure also reproduces at base. CLI evidence was captured; no Boatmate actions or source changes were made.
Evidence: Live recovery and safety-refusal transcripts
Evidence: Remote lifecycle regression
Evidence: Test summary and baseline limitation
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed (2) ✅
bin/fm-control-lib.sh:288- Intent requires “live, foreign, or unverifiable marker state must refuse.” With a missing Herdr endpoint and a valid marker whose live PID equals the lock PID, the new condition[ "$marker_pid" != "$owner_pid" ]skips refusal; line 293 signals that process, then cleanup removes its marker after it exits. Thus recovery can kill a live owner instead of refusing. Enforce live-owner refusal before signaling in this shared preparation boundary, preserving ordinary graceful exit separately. Related sites: bin/fm-control-lib.sh:306 repeats the equality exemption; tests/fm-secondmate-restart.test.sh:1205 and :1276 configure matching owners and expect forced-stop success; docs/configuration.md:331 promises live-owner refusal. This conflicts with explicit acceptance criteria and requires reconciliation.🔧 Fix applied.
3 issues (2 errors, 1 warning) still open:
bin/fm-control-lib.sh:288- Intent requires “live, foreign, or unverifiable marker state must refuse.” With a missing Herdr endpoint and a valid marker whose live PID equals the lock PID, the new condition[ "$marker_pid" != "$owner_pid" ]skips refusal; line 293 signals that process, then cleanup removes its marker after it exits. Thus recovery can kill a live owner instead of refusing. Enforce live-owner refusal before signaling in this shared preparation boundary, preserving ordinary graceful exit separately. Related sites: bin/fm-control-lib.sh:306 repeats the equality exemption; tests/fm-secondmate-restart.test.sh:1205 and :1276 configure matching owners and expect forced-stop success; docs/configuration.md:331 promises live-owner refusal. This conflicts with explicit acceptance criteria and requires reconciliation.bin/fm-control-lib.sh:293- AC2 requires “preserving refusals for live owners.” With a missing endpoint, a valid dead marker PID, and a distinct live lock PID, this path signals the lock owner and clears artifacts after it exits. After suspension, that lock PID may belong to an unrelated process. Round 1 fixed live-marker refusal but left this sibling: the new check probes only marker_pid before "$stop_cmd" "$owner_pid". Refuse live lock owners before signaling; graceful exit already precedes preparation. Related sites: bin/fm-control-lib.sh:297 accepts signal-induced absence; tests/fm-secondmate-restart.test.sh:1019 covers only a lock owner that survives signaling; docs/configuration.md:331 promises live-owner refusal.tests/fm-secondmate-restart.test.sh:1392- The new FM_TEST_OMP_RESTART_ONLY mode adds an optional execution path that skips the remaining restart suite. Neither the requested regression nor full validation requires this selector. Remove the conditional shortcut and retain the regressions in the ordinary suite.🔧 Fix applied.
✅ Re-checked - no issues remain.
🔧 **Test** - 2 issues found → auto-fixed ✅
bin/fm-control-lib.sh:278- A failed kill -0 probe is treated as proof of absence, including EPERM. Through the real remote relaunch command, a valid marker naming live root-owned PID 2 passed preparation despite 'Operation not permitted'; relaunch replaced its marker with owner PID 2014483 and attempted another launch while PID 2 remained alive. The lock-owner probe has the same ambiguity. Distinguish confirmed absence from permission errors and other unknown results, refusing and preserving artifacts whenever either owner cannot be proven absent.TMPDIR="$PWD/.no-mistakes/test-tmp" bash tests/fm-remote-secondmate-lifecycle-e2e.test.sh— passedbash tests/fm-secondmate-restart.test.shwith workspace-local temporary storage — setup refusal; retried from an isolated source copybash .no-mistakes/test-code/tests/fm-secondmate-restart.test.sh— stopped at an earlier non-OMP restart fixture; the base commit reproduced the same failurebash .no-mistakes/test-code/tests/fm-omp-secondmate.test.sh— passedbash .no-mistakes/test-code/tests/phase-omp-driver.sh— executed the four existing OMP capability, preparation, restart, and closure-update regressions; passedbash .no-mistakes/live-refusals.sh— real remote relaunch command against an isolated Herdr serverbash .no-mistakes/live-recovery.shand recovery retries — native post-update recovery attemptsbash .no-mistakes/permission-probe.sh— reproduced permission-denied owner bypass through the real remote relaunch commandScanned the change diff and evidence logs for common credential signatures; stopped disposable servers, removed fixtures, and verified source remained unchanged🔧 Fix applied.
✅ Re-checked - no issues remain.
python3 .test-phase/live.pyexercised the real remote relaunch command and refusal boundaries.bash .test-phase/recover.shlaunched real Herdr and OMP;!bin/fm-session-start.shclaimed the session lock inside OMP.bash .test-phase/capture.shverified alive status, matching marker/lock ownership, completed relaunch, and retained session.python3 .test-phase/permission.pyandpython3 .test-phase/history.pyexercised permission and history refusals.Executed four focused OMP behavior functions fromtests/fm-secondmate-restart.test.sh.bash .test-phase/code/tests/fm-omp-secondmate.test.shpassed.bash tests/fm-remote-secondmate-lifecycle-e2e.test.shpassed.Broader restart-suite attempts encountered a fixture-sync failure; the same T2 failure reproduced against the base revision.Stopped disposable processes, removed temporary workspace directories, and verified clean git status.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.