fix(pi): enforce exact primary process ownership - #2653
Open
SamSherpaDev wants to merge 3 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
Audit Firstmate's Pi-primary orchestration against the established Claude-primary path and fix only evidence-backed tracked defects that materially break or weaken parity across isolated project work, durable task handling, steering and recovery, session restoration, and continuous supervision. Preserve all supported harness and runtime-backend compatibility and existing user configuration behavior; do not modify private configuration and do not exercise or change Herdr lifecycle behavior. Use current Pi 0.84.2 authoritative documentation, changelog, installed runtime source, live behavior, and executable public-interface regressions rather than inference; compare against the proven Claude path, document each concrete mismatch with reproducible evidence and an authoritative owner, prefer targeted ownership fixes over broad redesign, and seek disconfirming evidence before changing code. The evidence-backed defect found is that nested Pi commands auto-load project extensions and were incorrectly treated as owners merely because the actual primary was an ancestor, allowing them to overwrite primary supervision markers and claim watcher lifecycle; correct ownership to the exact lock-holder process and add regressions that fail before and pass after. Run the required focused tests and lint, commit, and complete the no-mistakes PR path through green CI. The relaunched worker configuration is Pi with openai-codex/gpt-5.6-sol at xhigh.
What Changed
Risk Assessment
✅ Low: The targeted exact-PID ownership fix is well bounded and includes executable regressions for marker preservation and watcher non-ownership.
Testing
Both targeted suites passed, and an end-to-end Pi 0.84.2 RPC probe showed a nested Pi command receives the read-only response, preserves both primary markers, and claims no watcher lifecycle; the base-commit counterfactual reproduced the original defect. The broader tmux live test was not run because tmux is unavailable.
Evidence: Pi 0.84.2 nested ownership acceptance transcript
Source: Pi 0.84.2 nested ownership acceptance transcript
Nested Pi auto-discovered the command, returned the read-only lock-owner notification, preserved both markers byte-for-byte, and produced no watcher lifecycle side effects.Evidence: Base-commit counterfactual reproducing the defect
Source: Base-commit counterfactual reproducing the defect
The base implementation reported a started Pi extension arm child and replaced both primary markers; the acceptance probe exited nonzero as expected.Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
bin/fm-test-run.sh tests/fm-pi-watch-extension.test.shbin/fm-test-run.sh tests/fm-turnend-guard.test.shRanpi-nested-ownership-e2e.pyagainst installed Pi 0.84.2 in RPC mode, invoking the auto-discovered/fm-watch-arm-picommand from a descendant process.Ran the same acceptance probe against extensions exported from base commit87681a40777bb061ef923ef98b494cd7ef6054b6; it failed as expected by overwriting both markers and reporting a started watcher child.✅ **Document** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.