fix: 安全恢复 Pi resume 会话锁#645
Open
codenum137 wants to merge 13 commits into
Open
Conversation
codenum137
force-pushed
the
fm/firstmate-pi-resume-lock-fix-m9
branch
from
July 17, 2026 07:03
cfeab74 to
d4aed08
Compare
added 13 commits
July 17, 2026 15:08
codenum137
force-pushed
the
fm/firstmate-pi-resume-lock-fix-m9
branch
from
July 17, 2026 07:14
d4aed08 to
88118f1
Compare
5 tasks
Owner
|
Automated reminder: thanks for the PR! This branch currently has a merge conflict with the base branch. When you get a chance, please rebase onto (or merge) the latest base branch, resolve the conflict, and push. After that, checks will re-run and the PR will get looked at again. Noted for firstmate#645 at |
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
恢复并继续验证现有 Firstmate PR #645,而不是创建新的实现或 PR。以 codenum137/firstmate 的 fm/firstmate-pi-resume-lock-fix-m9 分支和 cfeab74 为权威起点,完整验证 native Pi 0.80.7 resume 场景下的会话锁安全恢复及已有测试、文档和 fail-closed 行为;允许流水线正常对齐最新 main。保持现有自然中文标题和正文内容,由 no-mistakes 流水线自身更新现有 PR #645,生成确定性的 ## Pipeline 段落和 Updates from git push no-mistakes 来源标记,并使全部 CI 变绿。不得手工编辑 PR、手工推送、强推、重写提交、创建重复 PR、合并或修改全局 Pi。
What Changed
Risk Assessment
✅ Low: Captain, the latest commit resolves the reported rejection and retry-loop risks without weakening session-lock, primary-scope, resume, or away-mode ownership invariants.
Testing
已成功的完整基线之外,本轮又验证了锁竞争与拒绝、Pi watcher/away/primary-scope、turn-end guard、三种 locale、cmux/Herdr/tmux,以及 native Pi 0.80.7 保存会话的真实离线恢复;所有检查通过。该变更没有 UI,审阅证据采用终端轨迹和持久状态转换记录。
Evidence: Native Pi resume evidence
+ PI_VERSION=0.80.7 + initial_pi_pid=92319 + initial_pi_comm=pi + resume_old_state=dead + resumed_pi_pid=93012 + resumed_pi_comm=pi BASH-ONE FIRST_PI_EXIT=0 PI_RESUME_STARTING=11111111-1111-4111-8111-111111111111 READ-ONE BASH-TWO watcher: started Pi extension arm child 2 REARMED ok - Pi 0.80.7 live E2E created, exited, resumed, reclaimed before watcher startup, woke, re-armed, and cleaned upEvidence: Full native Pi E2E trace
Evidence: Session lock contract
Evidence: Pi watcher integration
Evidence: Composer locale matrix
locale=C bare-claude=empty bare-codex=empty ghost=empty typed=pending shell=unknown locale=C.UTF-8 bare-claude=empty bare-codex=empty ghost=empty typed=pending shell=unknown locale=en_US.UTF-8 bare-claude=empty bare-codex=empty ghost=empty typed=pending shell=unknownEvidence: cmux composer suite
/var/folders/_7/702wv9kj39gcb3_wqvrzq3r40000gn/T/no-mistakes-evidence/01KXNY8KD7QTMQFP3TGBYN8NWX/herdr-composer-suite.log)Evidence: Real tmux smoke test
Evidence: Turn-end guard suite
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 3 issues found → auto-fixed (5) ✅
tests/fm-pi-primary-live-e2e.test.sh:122- Authoritative criterion “完整验证 native Pi 0.80.7 resume 场景下的会话锁安全恢复” is not exercised: this opt-in E2E still launches plainpi, while the other coverage only invokes a mockedsession_startcallback from Node. Add a real Pi 0.80.7 create/exit/resume flow that leaves the old native PID in the lock, resumes the saved session, and verifies reclamation before watcher startup..pi/extensions/fm-primary-pi-watch.ts:156- A resumed session callsstartArm()even whenstate/.afkexists. Its--restartarm terminates the daemon-owned watcher and routes routine wakes directly into Pi, violating away mode's single-owner batching contract. Recover the session lock independently, but do not spawn or restart a watcher while away mode is active..pi/extensions/fm-primary-pi-watch.ts:95- Missing/dead locks are acquired without first verifying that this is a main or valid secondmate home. Since tracked Pi extensions also auto-load in trusted linked task worktrees, a crewmate/scout can create its own session lock and watcher, or act on an inherited supervisor home. Apply the existing primary-home scope predicate before recovery and arming.🔧 Fix: Validate native Pi resume lock recovery end-to-end
1 error still open:
tests/fm-pi-primary-live-e2e.test.sh:139- The live E2E copies the realauth.jsoninto$ROOT/.pi-live-e2e.$$, but that path is not ignored. A SIGKILL or host crash bypasses the EXIT cleanup and leaves credentials as untracked repository content that can be accidentally staged. CreateLABwith a mode-0700mktemp -dunder the system temporary directory before copying authentication data.🔧 Fix: Harden Pi resume scope, readiness, and isolated E2E
1 error still open:
.pi/extensions/fm-primary-pi-watch.ts:136- The.afkcheck only prevents a new arm; it does not stop an arm child created earlier bysession_start. When/afkis entered during a normal Pi session, the daemon therefore encounters the existing watcher singleton, while that extension-owned watcher sends the next wake directly to Pi instead of through away-mode batching. Transfer watcher ownership during AFK entry and cover the active-arm-to-AFK transition.🔧 Fix: Transfer Pi watcher ownership on away entry
2 errors still open:
.pi/extensions/fm-primary-pi-watch.ts:108- Away reconciliation is one-way: it stops the Pi arm when.afkappears but never restores it when the launch transaction removes the flag.fm-afk-launch.sh startwrites.afkbefore daemon creation and removes it on terminal/readiness failure, so rollback can leave normal mode with neither daemon nor extension watcher; the intentional-stop marker also suppresses the child's close notification. Re-arm after a rolled-back handoff, or move stop/reap into the launch transaction so rollback restores supervision..pi/extensions/fm-primary-pi-watch.ts:113- The active-child handoff depends on thisfs.watch, but construction is unguarded and the error handler permanently disables monitoring without reconciling ownership. If an error occurs while.afkexists, its failure wake is suppressed bysendWakeand the active extension arm can remain, recreating the daemon singleton collision with no alert. Catch setup failures and make monitor errors retry or stop the arm fail-closed before advertising the extension as loaded.🔧 Fix: Harden Pi AFK monitor rollback and failure recovery
2 issues (1 error, 1 warning) still open:
.pi/extensions/fm-primary-pi-watch.ts:183- Both monitor-recovery branches discard thesendWake()promise. If Pi rejects follow-up delivery during shutdown or another lifecycle race, that rejection is detached from the surroundingstartArm()chain and becomes unhandled, unlike the guarded child close/error paths. Return or catch these promises through a shared safe-delivery helper..pi/extensions/fm-primary-pi-watch.ts:154- A persistent monitor error retries every 250 ms without backoff. Each attempt callsisPrimaryHome(), which synchronously spawns the scope script and its Git probes, causing four blocking subprocess trees per second indefinitely in the Pi process. Use capped exponential backoff and avoid rechecking invariant primary scope on every retry while remaining fail-closed.🔧 Fix: Contain Pi wake rejection and back off monitor retries
✅ Re-checked - no issues remain.
🔧 **Test** - 1 issue found → auto-fixed (2) ✅
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: Pin tmux smoke test to Bash
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"🔧 Fix: Fix locale-dependent composer classification
✅ Re-checked - no issues remain.
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"已成功的配置基线: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"bash tests/fm-session-lock.test.shbash tests/fm-pi-watch-extension.test.shbash tests/fm-turnend-guard.test.shbash tests/fm-composer-lib.test.shbash tests/fm-backend-cmux.test.shbash tests/fm-backend-herdr.test.shbash tests/fm-backend-tmux-smoke.test.shFM_PI_LIVE_E2E=1 bash -x tests/fm-pi-primary-live-e2e.test.sh在C、C.UTF-8、en_US.UTF-8下直接验证 composer 分类矩阵隔离 clone 后运行git rev-parse HEAD,确认 E2E 来源为33341f6d16a2a08382872c7d8a08799dcff10abe清理生成状态后运行git status --short --branch并确认无残留测试目录🔧 **Document** - 1 issue found → auto-fixed (2) ✅
bin/fm-session-start.sh:252-fm-lock.shcan now fail for malformed locks, concurrent acquisition, missing harness ancestry, or publication errors, but this banner labels every failure as another live session holding the lock. The detailed error remains visible, but the headline contradicts the correctedAGENTS.mdguidance. Executable wording is outside this documentation-only pass.🔧 Fix: Clarify unowned session lock refusal documentation
1 warning still open:
bin/fm-session-start.sh:252-fm-lock.shcan refuse malformed locks, concurrent acquisition, missing harness ancestry, or publication errors without a live holder, but this banner and subsequent read-only helper messages still assert that another session owns the lock. Correcting those executable output strings is outside this documentation-only pass.🔧 Fix: Clarify session lock refusal output
✅ Re-checked - no issues remain.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.