fix: recover retained Pi tasks and release stale anchors - #367
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
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
Reconcile the two retained non-Azure Firstmate repairs fm-internal-prs-rebase-land-b7 and fm-run-bounded-perl-supervisor-leak against their briefs, artifacts, preserved commit graphs, durable no-mistakes state, and current public main so work progresses to delivery or exact evidence-backed closure rather than occupying capacity indefinitely. Preserve every unlanded commit, carry only still-required current changes onto this clean branch, restore truthful completion evidence, and use focused executable proof. Work the repairs sequentially and deliver the first still-required coherent change, recording the remaining internal-PR repair as an explicit next delivery unless exact evidence proves it safe to close. The bounded-command supervisor must exit a childless Perl process-group anchor when the controller finish pipe reaches EOF after command reap, retry only EINTR, preserve explicit guarded retention on unverified cleanup, preserve status and TERM/KILL semantics, and include deterministic exact-process regressions. Priority correction: first fix fm-spawn --recover-direct-account so legacy local Pi-authored retained ship/scout tasks recover their dead endpoint instead of rejecting harness=pi and masking the refusal with SPAWN_TASK_TMP unbound; reuse the task-private Pi author snapshot, prove the observed worktree belongs to the recorded project, adopt then recheck its exact Git-dir and branch/detached-head identity, upgrade metadata, preserve task context and existing Claude/Codex recovery, and add focused executable success and early-refusal regressions. Do not convert Azure cloud placement into a local endpoint. Do not clean ambient processes, add broad scans/reapers, weaken tests, restart shared services, push main, or merge a PR. Routine engineering fixes and iteration are authorized; escalate only destructive, irreversible, security-sensitive, credential, or product decisions.
What Changed
Risk Assessment
✅ Low: The changes preserve the required process-group lifecycle semantics and make legacy local Pi recovery fail closed when its task-private author snapshot is unavailable, with no additional source-verifiable defects found.
Testing
The successful baseline was supplemented with focused executable recovery and process-lifecycle checks: legacy local Pi recovery reused its task-private author snapshot and upgraded exact worktree identity, failed closed when that snapshot was missing, rejected Azure placement without the prior unbound-variable masking error, and the bounded runner exited its exact childless anchor on EOF while preserving signal/status and TERM/KILL cleanup semantics; reviewer-visible CLI transcripts were captured and all checks passed.
Evidence: Legacy Pi direct-recovery behavioral evidence
Evidence: Bounded process-tree lifecycle evidence
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-spawn.sh:3941- The required behavior says legacy Pi recovery must “reuse the task-private Pi author snapshot,” but recovery only reuses it when the directory exists. If the original snapshot capture failed (an already-supported path that warns and launches) or the directory was later lost, this branch recreates it from the current ambient PI_CODING_AGENT_DIR/HOME and silently launches under potentially different credentials. Recovery should fail closed when the recorded task-private snapshot is unavailable instead of recapturing ambient identity; add an executable missing-snapshot refusal regression.🔧 Fix: Captain: refuse recovery without Pi author snapshot
2 issues (1 error, 1 warning) still open:
tests/fm-account-directory.test.sh:897- The Pi author-identity repair lacks verified fail-before proof: the executor’s exact public check exited 0 at cd8b64d, so the new missing-snapshot regression has not demonstrated that it reproduces the authorized failure before the fix. Because this is an auth-permission boundary and the repair proof is explicitly unverified, obtain executable fail-before/pass-after evidence before merging.bin/fm-spawn.sh- The fixer could not prove the semantic repair with a public/executable fail-before/pass-after regression and relevant integration or consumer compatibility evidence; primary-agent handoff is required.🔧 Fix: Validate Pi recovery refusal
1 warning still open:
✅ **Test** - passed
✅ No issues found.
if [ "${FM_AZURE_VALIDATION_CELL:-0}" = 1 ]; then exec "$FM_AZURE_VALIDATION_SHARD_BRIDGE" behavior --count "${FM_AZURE_VALIDATION_SHARD_COUNT:-8}"; else exec bin/fm-no-mistakes-test-command.sh; fiPre-phase baseline (already successful):if [ "${FM_AZURE_VALIDATION_CELL:-0}" = 1 ]; then exec "$FM_AZURE_VALIDATION_SHARD_BRIDGE" behavior --count "${FM_AZURE_VALIDATION_SHARD_COUNT:-8}"; else exec bin/fm-no-mistakes-test-command.sh; fiFM_TEST_FOCUSED=pi-direct-recovery tests/fm-account-directory.test.shtests/fm-process-tree.test.shManual evidence check: verified both captured transcripts and confirmedgit status --shortremained empty✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.