Skip to content

fix(pi): enforce exact primary process ownership - #2653

Open
SamSherpaDev wants to merge 3 commits into
kunchenguid:mainfrom
SamSherpaDev:fm/firstmate-pi-orchestration-parity
Open

fix(pi): enforce exact primary process ownership#2653
SamSherpaDev wants to merge 3 commits into
kunchenguid:mainfrom
SamSherpaDev:fm/firstmate-pi-orchestration-parity

Conversation

@SamSherpaDev

Copy link
Copy Markdown

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

  • Require Pi watcher and turn-end extensions to match the session lock holder’s exact PID, preventing nested Pi commands from replacing primary markers or claiming watcher lifecycle.
  • Add descendant-process and live Pi regressions covering marker preservation, lock ownership, and watcher-arm suppression.
  • Document Pi’s process-exact ownership boundary and regression coverage.

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.

Pi version: 0.84.2
Primary lock-holder PID: 54727 (live parent of nested Pi RPC process)
Nested Pi auto-discovered command: True
Nested arm command accepted by Pi RPC: True
User-visible extension notification: watcher: read-only - session lock is held by another firstmate session
Watcher marker preserved byte-for-byte: True
Turn-end marker preserved byte-for-byte: True
Nested watcher lifecycle side effects: none
Nested Pi exit code: 0
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.

Pi version: 0.84.2
Primary lock-holder PID: 54737 (live parent of nested Pi RPC process)
Nested Pi auto-discovered command: True
Nested arm command accepted by Pi RPC: True
User-visible extension notification: <missing>
Watcher marker preserved byte-for-byte: False
Turn-end marker preserved byte-for-byte: False
Nested watcher lifecycle side effects: none
Nested Pi exit code: 0
Raw RPC records:
{"id": "78fbc0bb-be25-4294-b96b-784e0f8f6edd", "message": "watcher: started Pi extension arm child 1; future ordinary re-arms are automatic; call fm_watch_arm_pi again only after a later notification says the cycle is missing, failed, or unhealthy", "method": "notify", "notifyType": "info", "type": "extension_ui_request"}
{"command": "get_commands", "data": {"commands": [{"description": "Arm firstmate watcher supervision through the Pi extension instead of foreground bash.", "name": "fm-watch-arm-pi", "source": "extension", "sourceInfo": {"baseDir": "/private/var/folders/r4/lj5nyr393lv0s3j7ltth6dxc0000gn/T/no-mistakes-evidence/01M0E5DSD2K7EB8BKBVXKK9DYD/pi-before-fix-project/.pi", "origin": "top-level", "path": "/private/var/folders/r4/lj5nyr393lv0s3j7ltth6dxc0000gn/T/no-mistakes-evidence/01M0E5DSD2K7EB8BKBVXKK9DYD/pi-before-fix-project/.pi/extensions/fm-primary-pi-watch.ts", "scope": "project", "source": "auto"}}, {"description": "Manage llama.cpp router models", "name": "llama", "source": "extension", "sourceInfo": {"origin": "top-level", "path": "<inline:llama.cpp>", "scope": "temporary", "source": "inline"}}]}, "id": "commands", "success": true, "type": "response"}
{"command": "prompt", "id": "arm", "success": true, "type": "response"}

Counterfactual acceptance-probe exit: 1 (expected nonzero for the base implementation)

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.sh
  • bin/fm-test-run.sh tests/fm-turnend-guard.test.sh
  • Ran pi-nested-ownership-e2e.py against installed Pi 0.84.2 in RPC mode, invoking the auto-discovered /fm-watch-arm-pi command from a descendant process.
  • Ran the same acceptance probe against extensions exported from base commit 87681a40777bb061ef923ef98b494cd7ef6054b6; it failed as expected by overwriting both markers and reporting a started watcher child.
✅ **Document** - passed

✅ No issues found.

⚠️ **Lint** - 1 warning
  • ⚠️ linter found issues (exit code 127)
✅ **Push** - passed

✅ No issues found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant