[zhuowp]

No-Issue：本仓库未启用 Issues；通用引擎生命周期修复，应用侧消费由 pinvou-agent #839 单独跟进。

以真实目标 pinvou3-clean 的累计差异为准：引擎在完整 actor 操作之间执行 ready/commit 退场握手，检查目标、子代理、shell 和待交付结果；退场阶段等待 MCP boot 后再关闭连接池。宿主须在同步 predicate 与 commit 期间排除新外部提交。完成回执不保活引擎或事件队列，不新增 wire 字段、依赖或产品专用卸载策略。

本轮修复三个相关断点：空的 Interrupted checkpoint 不再错误阻塞退场；恢复时将旧代已接受的父消息按 FIFO 放入真实初始 User 输入，成功创建后才清旧账，并保护首轮执行前再次中断；活跃代已接受的正文由共享票据持有，只有实际 checkpoint 保留对应消息后才确认，避免中断丢失。既有历史软预算仍生效，仅保护当前尚未确认的输入批次，保留身份重定向、终态拒绝和错误语义。

验证：固定提交树，Windows / Rust 1.99 默认 owning TUI libtest；项目 scripts/dev-cargo.sh 锁定依赖编译。idle 23、parent-mail 5、existing-resume 1、identity 11，共 40 项通过、零失败。六处退化负控各由预期行为断言拒绝，恢复后通过。最终测试链接使用仅限本地测试目标的 bundled LLD（不修改仓库或全局配置），同一组 40 项再次通过，资源 Job 自然清退；六个负控按源码、目标、依赖和 profile 等价有限继承，没有宣称 LLD 路径重跑了负控。旧 MSVC 链接路径的资源门禁失败记录保留。

本轮三文件修复已完成五角色独立首审、复核、非实现者 v3 裁决与实际提交映射。历史测试和全量 sweep 不作为本轮重跑证据；未验证 Linux/all-features 全门禁、真实远端模型、全部 MCP 实物、release 或完整宿主 E2E。新提交须重新外审；CodeWhale 不自动合入。

长期 mail admission/retention/GC 等非阻断跟踪继续见 pinvou-agent #845。codewhale-hq#839 仍指向旧候选，本轮不声称应用已消费最终修复；后续需单独更新 gitlink、fork 登记、指纹与 guard。

本轮修复提交：d41467e213160aa2ea3f45046e7e5ae106fd3ded。

[asto18089]

Fresh 5-lane review (claims/root-cause, concurrency+performance, elegance/boundaries, smuggling/process, test adequacy) at head 0022e7374, base 11cba403d == current pinvou3-clean tip (merge-base identical, no rebase needed).

Verdict

Approve direction — 0 blocker / 0 major / 6 minors / 2 low. All five PR-body claims verified TRUE against the code; smuggling CLEAN; DCO PASS; fork-guard obligations correctly discharged in the companion parent PR codewhale-hq#839 (T4 register entry + 9 fingerprints + exact head pin, verified via the GitHub API).

Claim verification (all TRUE)

1. Problem statement — TRUE. Base Op::Shutdown (engine.rs:4207) breaks unconditionally, dropping steers/queued evidence with no quiescence check; autonomous turns run entirely inside handle_send_message before next_run_input, so host observation cannot see them; terminal persistence is host-side and invisible to the actor.
2. Ready/commit handshake — TRUE. can_retire_without_losing_work (idle_shutdown.rs:22-65) gates scheduled goal continuation, advisor ownership (Arc::get_mut), goal_state.is_active(), live subagents (incl. queued follow-ups), live-or-unreported shells (with poll()), plus emptiness of rx_op/rx_subagent_completion/rx_steer/pending_steers/rx_approval/rx_user_input and tx_event full capacity. Retirement is only evaluated from next_run_input, i.e. between complete operations; a running turn wins the race and is preserved.
3. Host predicate — TRUE. In handle.rs there is no await between receipt.await and commit.send; refusal/cancel drops the commit sender and the actor resumes (tests pin the cancel token untouched and continued op usability).
4. MCP boot join + receipt-only observers — TRUE. shutdown_mcp_pool joins the boot worker before shutdown_all, closing the base race where a late boot handshake could insert a live transport after teardown; wait_run_completion now owns only the watch receiver ('static future via use<>), pinned by the op-channel Disconnected assert.
5. No Op / wire field / dependency / advisor store / product policy — TRUE. ops.rs byte-identical to base; no manifest changes; host policy isolated behind the injected host_is_idle closure.

Producer-by-producer enumeration found no ungated autonomous producer: ops (self-dispatch clone included), steers + pending steers, approvals (incl. child routing), user input, child completions (deliver-before-status-clear ordering under the manager write lock, recheck with guard held), shells (pull-only, completion_reported), goal continuations (every state), advisor emissions (single clone site, verified in advisor.rs), boot (teardown join). Compaction, live-runtime-authority, run-completion watch, and turn-scoped machinery are structurally excluded from the idle boundary. No autonomous lost-work interleaving was constructible.

Verification run (Linux arm64, this host)

• CI fork-ci check green at this head: fmt + workspace clippy -D warnings + nextest all-features + doctests + lockfile drift.
• Locally reproduced: cargo fmt --check clean; all 294 forkguard_* lib tests pass (the PR's 9 included); run_completion::tests 7 passed / 0 failed — all counts match the body exactly. (An earlier parallel run appeared to stall — that was CPU starvation from an unrelated concurrent cargo run on this box; the serial rerun finished 294/294 in 17.4s.)

Elegance / boundaries

Not reinvention: base EngineHandle exposes no retire/idle API; run_completion::observe is output-only; the side channel mirrors the established tx_approval/tx_subagent_completion non-Op handshakes; an Op-based design would trip the compile-time protocol parity check (protocol_parity.rs:912) or force a wire extension for a process-lifetime handshake — the module doc's justification is real. The two manager helpers compose existing primitives (queued_follow_up_counts_for_session) rather than duplicating; no composable predicate covered the retirement need. Both load-bearing comment claims verified (get_mut synchronization with the final clone drop; deliver-before-status-clear in finish_terminal_result). Every doc sentence of shutdown_when_idle verified against the code. Boundary clean: no product semantics in the engine, host policy stays in the closure.

Smuggling

CLEAN. Hunk-by-hunk: all 7 files map to the claims; the wait_for_mcp_boot → join_mcp_boot split is required (shutdown must join without the UI-emitting drain; all 3 pre-existing callers keep identical behavior via the composite); both new manager helpers have exactly one production caller; all 3 EngineHandle literal construction sites updated. No manifest/CI/docs/i18n changes. Secrets clean.

Findings

MINOR

• M1 — commit-window op loss is silent and contract-only (idle_shutdown.rs:58 + engine.rs:4213-4223). After can_retire passes, a host send() still enqueues successfully; the actor breaks and drops rx_op without draining, so the op vanishes with no error signal — the host cannot detect its own contract violation. Additionally, reserved-but-unsent permits (e.g. runtime_threads.rs:7638 reserve→await→send) are invisible to rx_op.is_empty() (tokio Permit::send succeeds even on a closed receiver and drops the value). Suggest a final drain-and-fail of rx_op at retirement teardown (mirroring drop_all_steers for Op::Shutdown), or at least one doc sentence extending the host obligation to in-flight reservations.
• M2 — MCP boot settlement is dropped if boot finishes during the handshake (engine.rs:4225-4243). Pool safety is fine (join before shutdown_all, pinned by test), but the Finished boot event and the durable maybe_inject_mcp_boot_briefing note never run for a retired engine. Infrastructure-only loss; worth one sentence in the body/doc since claim 2 enumerates the ready-check inputs and boot is deliberately absent from them.
• M3 — commit.await has no cancel-token escape (idle_shutdown.rs:82). Within the public API the window is bounded (no await between receipt and commit.send), but if the host blocks inside its sync host_is_idle closure, the engine parks indefinitely and cancel() is inert for the duration. Hardening option: tokio::select! commit vs cancel_token.cancelled().
• M4 — hosted engines with an unclaimed background shell completion can never retire by waiting: the wake arm requires automatic_followups_allowed (engine.rs:3235-3237), the claiming turn is contract-forbidden during retirement, so completion_reported never flips. Covered by the doc's no-progress disclaimer, but retirement is not a full replacement for Op::Shutdown in hosted mode with background shells.
• M5 — test gaps mirroring M1/M2 (mutation-verified no-catch): the four non-op queue conjuncts (rx_steer/pending_steers/rx_approval/rx_user_input — no test queues any of them); the shell predicate has_live_or_unreported_work_for_session (zero test coverage repo-wide); the region-B select arm (engine.rs:3207, scheduled-goal delay); the MCP test's "no events" half (a Progress-only fixture means a join→drain substitution passes — queueing a Finished update would pin it); the "proposal expired" actor-side abort path (handle.rs:135).
• M6 — unbiased select in next_operation_or_retirement (idle_shutdown.rs:15-20) where the neighboring site is biased; with op polled first (engine.rs:3195-3207): FIFO intent is enforced only by the downstream refusal, not locally. Adding biased; op-first would express the ordering intent where it lives.

LOW

• L1 — commit subject/body are Chinese; CONTRIBUTING.md requires English descriptions (CI validates format only; 3 of the last 30 landed subjects are Chinese, so practice is mixed). Squash-merge takes the English PR title, but an amend to English before merge would comply with the written rule.
• L2 — companion codewhale-hq#839 draft-state discrepancy: both PR bodies call it a draft and its own checklist gates readiness on #149 landing, but GitHub reports isDraft: false.

NIT

• The Arc::get_mut quiescence probe is novel in this crate; the invariant ("only the actor creates watcher clones") holds today (single clone site engine.rs:6563) and fails safe, but naming the sanctioned clone site in the comment would harden it against future second-clone-site refactors.
• Naming vocabulary split (retire vs shutdown: IdleRetirement/next_operation_or_retirement vs shutdown_when_idle/accept_idle_shutdown) must be inferred by the reader.
• shell helper takes session_id while every sibling _for_session fn uses active_session_id and carries the !is_empty() guard, which the new fn omits (behavior fine — caller always passes a real id).
• idle_shutdown.rs:56-57 states "the embedding host has already excluded new external submissions" as fact, but at that point host_is_idle has not run yet — it is a contract restatement, not an observation.
• Retirement refusals are un-rate-limited: a tight host retry loop pays goal-lock + per-agent scan + per-shell waitpid(WNOHANG) per attempt; hosts should back off.
• Test-side busy-spin on channel capacity (tests.rs ~1214) and 6-deep .unwrap() chains — cosmetic.

Bottom line

The PR does real work at the right boundary: reclamation safety moves from host observation to an actor-owned quiescence handshake, complete and root-caused for its claimed scope, with no unrelated changes and no defect found in the handshake core. The residuals are contract-scoped or fail-safe; M5 is the set worth addressing before merge if convenient — especially a shell-side test, since that predicate currently has zero coverage anywhere.

[asto18089]

Fix wave pushed at 7058ac395 (one commit on top of the reviewed 0022e7374; tree asserted locally). Local gates on this head: cargo fmt --check clean; cargo clippy -p codewhale-tui --lib --all-features --locked -- -D warnings (CI allow-list) clean; all forkguard_* lib tests 301/0 (294 + 7 new); run_completion::tests 7/0. The full-workspace gate runs in CI.

Fixed from the fresh review

• M3 (commit-await wedge): accept_idle_shutdown now escapes the commit wait when the cancel token fires for the first time during an uncommitted proposal — a host blocked forever inside its sync host_is_idle closure can no longer pin the actor (and cancel() is no longer inert mid-handshake). Two properties are pinned by new tests: the escape must refuse and leave the actor usable; and a token already latched by an earlier cancelled turn must not make retirement refuse or hang — commit still wins that race (forkguard_idle_shutdown_fresh_cancel_escapes_a_wedged_commit_wait, ..._stale_cancelled_turn_does_not_refuse_retirement). The snapshot is taken before readiness is offered, so the escape does not depend on scheduling between ready.send and entering the wait.
• M6 (local FIFO intent): next_operation_or_retirement is now biased; op-first, matching the neighboring timed wait, so the drain-site ordering no longer leans on the downstream refusal.
• M1 (silent commit-window loss): the committed break now counts residual inputs (count_residual_inputs) and emits a tracing::warn! — sends past the handshake already returned Ok, so the contract violation must not vanish without a trace. Doc comments on shutdown_when_idle and the quiescence check now state the full host obligation explicitly including in-flight reserve() permits, which is_empty() cannot observe.
• M2 (boot settlement): the module doc now records that the teardown join deliberately drops the boot's settlement event and failure briefing for the retiring incarnation.
• Nits: the advisor-gate comment now names the sanctioned clone site (the actor-loop advisor dispatch) and warns that a second clone site would permanently refuse retirement; the "host has already excluded" comment is now phrased as the contract it is; shutdown_when_idle's doc also states the call blocks until the actor's next input boundary (retry loops should back off). The shell helper's parameter is renamed to the sibling active_session_id convention with the same empty-id guard (behavior-neutral — callers always pass a real session id).
• M5 (test gaps): new pins for every previously mutation-uncatchable gate — ..._every_pending_input_queue_blocks_retirement (rx_steer / pending_steers / rx_approval / rx_user_input), ..._live_and_unreported_shell_work_gates_retirement plus a has_live_or_unreported_work_for_session unit test and session-owned seed helpers (the shell predicate had zero coverage repo-wide), ..._timed_goal_wait_still_serves_the_retirement_arm (region-B arm), ..._actor_abort_fails_the_host_commit_send (proposal-expired direction), and the MCP fixture now queues a Finished update with a connection failure so a join→drain substitution actually blocks on the full event queue instead of passing silently. The three original handshake tests wrap accept_idle_shutdown in timeouts, so a wedged regression fails an assertion instead of hanging the CI job.

Not fixed (deliberate, please confirm)

• M4 (hosted engine + unclaimed background shell completion can never retire by waiting): documented no-progress property of the current design, not a defect; changing it means allowing a claiming turn in hosted mode — out of scope for this wave.
• Renaming nit (retire vs shutdown vocabulary): renaming shutdown_when_idle/IdleRetirement churns the fork-guard fingerprints registered in the parent PR — not worth the churn.
• L1 (Chinese commit message on 0022e7374): that is the author's commit; squash-merge takes the English PR title anyway, so I left it untouched.
• L2 (companion PR fix(rlm): enforce child model for RLM fanout codewhale-hq/Codewhale#839 is not actually in draft): that is the parent PR's state toggle, not mine to flip.

Parent-side coordination needed

Companion Pinvou/pinvou-agent#839 pins this branch by SHA: its EXPECTED_HEAD must move to 7058ac3955842094dbe2259af512a19a21e5561f (one commit added, so the commit-count expectation changes too), and the T4 register may want the 7 new forkguard_ test names added. All names referenced by the existing 9 fingerprints are unchanged, so the name-presence fingerprints survive as-is.

[asto18089]

Fresh review @ 0a67733 — full re-review from scratch

Scope. Base 11cba403d is the current pinvou3-clean tip, so the rebase requirement is already satisfied (merge-base == tip; MERGEABLE). The combined diff (author 0022e7374 + hardening 7058ac395 + follow-up 0a677330b pushed by this review, see below) was re-reviewed from scratch with 11 independent lanes: claims/root-cause, idle-shutdown deep-dive, integration wiring, shell/subagent gates, mutation pinning, smuggling audit, adversarial concurrency, performance/resources, conventions/hygiene, local gates, and a parent fork-guard audit.

Key-point verdicts

1. Useful & complete — YES. All four PR claims verified TRUE against the code. The root cause — host-side "looks idle" predicates cannot see the actor's accepted-but-unpersisted work, and pool shutdown raced the boot worker — is eliminated, not patched: certification moved into the actor at its own sealed boundary, and teardown joins the boot worker before shutdown_all (shared with Op::Shutdown). The residual gate covers every input queue, goals, subagents (live work + queued follow-ups + parked mail as of this review), shells, advisor ownership and the event queue; the two documented host-contract duties (new external submissions incl. in-flight reserve() permits, host consumer persistence) are correctly stated as host duties, matching the claim wording. No production caller exists in-repo yet — the consumer is the companion app PR — and the foundation-first split conforms to the upstream-first boundary.
2. Elegant — YES. The two-oneshot proposal over mpsc(1) matches the crate's existing actor round-trip idiom (approval, MCP bootstrap); no watch/Notify reinvention; the Op/wire protocol and its exhaustive parity checks stay untouched; new pub surface is minimal (shutdown_when_idle only); module placement and naming follow sibling conventions.
3. Smuggling — CLEAN. Exactly 8 files, no renames/deletions; hunk-by-hunk classification found zero unrelated items and zero hidden semantic changes to pre-PR code (the two behavior deltas — teardown boot-join and the empty-id guard on PR-introduced code — are claimed/disclosed). Exactly 2 commits in range at review time; DCO Signed-off-by present and matching on each.
4. Bug-free — no known blocker/major remains after this review's fix wave. Two loss windows were found by the fresh lanes and are fixed in 0a677330b:

Fixed by this review in 0a677330b (pushed to this branch)

1. Phantom commit (invariant break). A host commit.send(()) landing while the cancel arm resolves can return Ok against a proposal the actor abandons — the biased select only orders within a single poll — so the host could see Ok(true) for a retirement the actor silently ignored, then hang forever in the mandatory wait_run_completion. Window is instruction-scale and needs a cancel racing the predicate, but it contradicts the documented "commit still wins" invariant. shutdown_when_idle now requires an actor-side commit ack: Ok(true) only after the actor consumed the commit; a lost ack reports false. Pinned: stale-token commit must ack; fresh-cancel escape must NOT ack; host-completion flow end-to-end through the handle.
2. Parked-child queued mail was invisible to the retirement gate (accepted-work loss). Follow-ups accepted for interrupted children take the queue-only arm and land in the in-memory queued_mail map without touching pending_follow_ups, so has_live_work_for_session could certify retirement while that accepted work sat undelivered — the commit dropped it silently, contradicting "preserving … queued inputs by refusing". The gate now refuses while session-owned queued_mail is non-empty; pinned end-to-end through the real agents/followup path.
3. Test budgets widened (5s→10s, 500ms→2s): both fired under concurrent build load during this review; they are fail-fast headroom, not timing assertions.
4. Biased op-first arbitration is now pinned by a deterministic two-input test (the arm swap previously left every test green).

Mutation verification (mutate → expect red → restore, worktree clean at 0a677330b after each): biased-arm swap → arbitration pin RED; queued_mail arm removal → parked-mail pin RED; ack-send removal → stale-token pin RED.

Local gates at 0a677330b

cargo fmt --all -- --check ✓; CI-style clippy -p codewhale-tui --lib --all-features --locked -- -D warnings ✓ (the 8 pre-existing base lints are allowlisted by fork-ci.yml; none introduced); forkguard_ 303 passed / 0 failed (301 + 2 new pins); run_completion:: 7/0; tools::shell::tests::forkguard 7/0; MCP-boot pin 1/0. GitHub CI was green at 7058ac395 (13/13); the new head's CI runs separately. All 226 CodeWhale fingerprint pins from the parent PR's fork-guard.sh re-validated against the new tree — 226/226 present.

Remaining notes (disclosed, none blocking)

• The idle main-wait select is intentionally not biased (fairness): an op+proposal tie consumes the proposal and returns Ok(false) — safe via the documented downstream-refusal backstop, but a host cannot distinguish "busy actor" from "host not idle". Cosmetic.
• Post-commit residual surfacing stops at the loop break: inputs delivered during teardown (e.g. while join_mcp_boot waits out a connect timeout) are dropped without the residual warn or SteerDropped events. Host-contract-violation-only; acceptable.
• The host-side ack.await → Ok(false) mapping is not directly pinned — the racing interleaving is inherently non-deterministic; the actor-side ack send/drop outcomes are pinned RED as above.
• Pre-existing, out of scope: flush_pending_persist clears persist_pending before writing and only warns on failure; a panicked boot worker latches mcp_boot_in_flight for the incarnation (the retirement join itself still resolves).
• Latent: an agent-owned session shell that finishes while idle is never claimed by the wake arms (both filter owner_agent.is_none()), so retirement would be refused until cleanup — unreachable today (no production ShellJobOwner::Some).
• PR body: the Testing section reflects the author commit (7 idle-shutdown tests); the branch now carries 16 forkguard_idle_shutdown* + shell/engine pins. Worth refreshing.

Companion parent PR (author-side checklist)

Pinvou/pinvou-agent#839 pins gitlink 0022e7374 and EXPECTED_HEAD / EXPECTED_COMMITS=94 — all stale. Before marking it ready: re-pin gitlink + EXPECTED_HEAD/EXPECTED_COMMITS to the final merged public tip; refresh the register prose (the "tree byte-identical to 63a400c6" claim no longer holds — runtime source changed in 7058ac395/0a677330b); reconcile the draft-state wording (isDraft: false vs "keep draft until the dependency lands"); re-run the final-SHA gates. Fingerprints need no change (verified above). Nothing here blocks this PR — the fork guard lives in the parent repo, so #149 is independently mergeable.

Verdict: approve direction — claims TRUE, root cause addressed, smuggling CLEAN, no blocker/major open at 0a677330b.

[asto18089]

Round-3 review — fresh from-scratch re-review (11 lanes) + wave-3 896176f87

Third full pass on this PR, started from zero (new 11-lane fan-out, no reuse of earlier round conclusions). Rebase check first: pinvou3-clean tip is still 11cba403d == this PR's base, freshly fetched — rebase is a no-op; 896176f87 is the new head.

Verdicts on the four review axes

1. Useful & root-caused: YES. All four body claims re-verified TRUE against code with per-claim evidence (readiness arms are real code; host predicate runs synchronously inside the sealed boundary between readiness and commit; boot-join precedes shutdown_all and is watch-based so it does not need an event consumer; no Op/wire/dependency/advisor-store surface added — EngineRunInput is private, *.toml diff empty). All three named failure modes are eliminated at the root: the actor-side certification closes the autonomous-turn TOCTOU, the synchronous host predicate moves the persistence check to the party that owns it, and the teardown join closes the late-transport window. Usefulness is anchored in a real consumer: the companion host PR wires shutdown_when_idle + wait_run_completion receipts into its recycle path, so this is not dead machinery — correct foundation/app sequencing per the fork boundaries.
2. Elegant: YES. The handshake reuses existing teardown instead of building a parallel shutdown; join_mcp_boot is a clean join/join+drain split of wait_for_mcp_boot; the Arc::get_mut advisor trick and the capacity proxy for "no undelivered event" both carry their WHY; the gate is assembled from existing documented state. The two-phase commit + ack is the minimum handshake that survives the three named races, and the alternatives (extend Op::Shutdown — wire-exposed and destructive; host-side-only checks — empirically lose the TOCTOU window; async predicate — reintroduces the pin problem) are rightly rejected.
3. Smuggling: CLEAN. Hunk-by-hunk walk of all 8 files (+1141/−22): every deletion accounted for (two drain-site recv blocks → next_operation_or_retirement, verbatim-move shutdown_mcp_pool, wait_run_completion body preserved in owned-future form); no existing test assertions weakened (pure additions + compile-mandated fixture fields); zero log/error-message changes outside new code; DCO pass on all commits; upstream-first boundary clean (no product-specific identifiers anywhere).
4. Bugs: 3 MAJOR-class retirement wedges found this round, all fixed in wave-3 (details below). Performance: the no-retirement common path is unchanged (biassed select short-circuits on the first ready arm; the gate never runs unless a retirement request is dequeued); no unbounded growth introduced; boot-join latency is bounded by per-connect timeouts and is disclosed below as a follow-up.

MAJOR findings fixed in wave-3 (896176f87, +463/−50, 8 files)

• M1 — pending_follow_ups leak wedged retirement forever. The counter had no removal site anywhere; a terminal commit never zeroed it; followup-then-cancel/step-limit/abort/budget leaves delivered-but-unconsumed counts > 0 permanently, and an idle engine never runs cleanup. Fix: drop the counter at terminal commit (update_from_result_with_persist) + warn when non-zero.
• M2 — mail parked on a dead-end child gated retirement forever. followup refuses Completed/Failed/Cancelled/BudgetExhausted, so queued mail on those children has no remaining delivery path, yet the gate arm counted it as accepted work. Fix: the arm now requires a live delivery path (can_still_deliver_mail: Running, or Interrupted with a continuable checkpoint), with the gate doc stating the semantics.
• M3 — child-owned finished shell evidence gated retirement with no possible claimer. Production nested agents stamp owner_agent: Some (with_owner_agent at the subagent tool context → shell_job_owner_from_context), the retirement gate counted their finished-unreported shells, but the idle wake arming filters owner_agent.is_none() and the boundary drain suppresses child-owned completions from the parent stream — nothing could ever flip completion_reported. Fix: the gate is owner-aware — finished-unreported evidence gates only for parent-owned jobs; running agent-owned shells still gate; and confirm_stop_when_reaped() + stop_unconfirmed are now consulted so the gate agrees with holds_write_scope (an unconfirmed stop is live work until the exit is observed — pinned against a real child).
• M4 — the queue-only receipt's promise was false across resume. Resume re-dispatches under a NEW id; followup on the old id can only park more mail on the retained record, so mail accepted while parked was stranded even after a resume. Fix: the resume re-keys the parked mailbox onto the resumed incarnation, which also makes M2's "Interrupted-continuable is deliverable" premise true.

Minors also fixed: goal-state gate arm switched to try_lock (sibling fail-closed convention; a host holding the shared goal state now defers retirement instead of deadlocking the actor before the receipt), stale-token commit-wait comment scoped honestly (the latched-token case trades actor liveness for retirement availability), shutdown_mcp_pool join note scoped to the current-generation boot, engine README now points at shutdown_when_idle, shutdown_when_idle doc split with the three Err arms + event-drain obligation + cancel-beats-commit mapping, residual warn logs the per-queue breakdown, has_queued_follow_ups_for_session replaces the HashMap fold, capacity-1 request channel documented as load-bearing, accept_idle_shutdown takes the IdleRetirement struct.

Evidence at head 896176f87

• forkguard_idle_shutdown 20/0, pending MCP boot 1/0, run_completion 13/0, shell gate pins 2/0 (real-child unconfirmed-stop hold is unix-gated), resume mailbox-migration pin 1/0, full forkguard_ sweep 313/0 (serialized runner), CI-style clippy --workspace --all-features green, Cargo.lock unchanged, fmt clean. (Local observation, not a defect: the default parallel libtest runner wedges on this machine — the four affected engine tests pass standalone and serialized; CI's nextest runs each test in its own process and is the authoritative parallel gate.)
• Mutations 6/6 RED: counter removal, deliverable-mail arm, owner-aware shell gate, stop_unconfirmed arm, goal-state arm, actor-side commit ack. (Two known non-deterministic/non-pinnable residuals disclosed in-test: biased; removal is killed probabilistically under tokio rotation; try_lock-vs-lock is behavior-identical when uncontended.)
• PR body Testing section refreshed: the old "7 passed" receipts were from the first commit only; head receipts + wave receipts are now recorded separately.

Disclosed, not fixed (non-blocking)

• Boot join has no overall cap: an early retirement pays the current boot's connect window (per-leg timeouts, batches of 8) before shutdown_all; joining to settlement is the safer semantic — an abort-based fast path is a reasonable follow-up if teardown latency matters.
• Post-commit residual inputs are surfaced by the warn only; the window between the pre-break count and engine drop is unobservable by construction (host contract).
• A stale-latched token + a host predicate that blocks forever pins the actor until the host drops its commit sender (fresh tokens escape; documented in wave-3 comments/doc).
• SyncSession-invalidated boots are not joined at teardown (they own only the dropped pool; transports die with the task — pre-existing, now scope-noted).
• Handle-side ack-lost → Ok(false) mapping is unpinned by test (needs a multi-thread-host race); documented in the method contract.

Author-side (unchanged from round 2)

The companion draft must re-pin after this PR lands: gitlink + EXPECTED_HEAD/EXPECTED_COMMITS (now 95 commits above 11cba403d after wave-3) + register prose (the commit-ack contract and the head receipts; "byte-identical to 63a400c6" is only true of the old pin) + final-SHA gates, before it leaves draft.

[asto18089]

Round-4 fresh review — REQUEST_CHANGES

Fresh from-scratch re-review at head 896176f87 (no rebase needed: base 11cba403d is still the pinvou3-clean tip; CI 14/14 green at head). Eleven independent review roles: claims/root-cause, hunk-by-hunk smuggling scan, handshake-core correctness, retirement-gate audit, MCP-boot/receipt audit, shell+subagent integration, elegance, performance, exceptional states, test/mutation quality, plus locally re-run gates. The four round-3 MAJORs are verified fixed; two new MAJORs found this round block.

What holds (all verified against code, not taken from the body)

• Claims 1–5 true and root-caused. The problem is real on base (Op::Shutdown races an unbiased select against rx_subagent_completion; queued child results and finished-shell evidence are dropped), and the fix addresses the root cause — engine-owned quiescence certification — rather than host-side heuristics. Claim 6 (companion PR Pinvou/pinvou-agent#839 consuming this API) is also confirmed: that PR's body explicitly depends on this one.
• Smuggling CLEAN. Every hunk in all 10 files is on-claim; the three adjacent behavior changes (terminal-commit counter removal, resume mail re-key, teardown boot-join) are disclosed in commit messages and required by the feature; all 25 deleted lines traced to their fate; DCO clean.
• Core mechanics sound. Gate is synchronous before ready.send (race-free preflight); no lock held across any .await; every try_lock treats contention as refuse; commit-beats-cancel requires the actor ack; every refusal path leaves no latched state; host-predicate panic/abort/refusal all recover (second attempt verified by tests).
• Hot path clean. With retirement not armed, the only per-boundary cost is one extra allocation-free mpsc receiver poll inside an existing biased select. No new polling loops, no unbounded growth introduced (one pre-shaped leak noted below).
• Boot-join and receipts correct. shutdown_mcp_pool joins the boot before shutdown_all on every natural run-loop exit; wait_run_completion now owns only a cloned watch::Receiver, published strictly after the engine future is dropped.
• Tests and mutations. All 6 claimed mutations are genuinely caught; I re-verified 3 of them live-RED locally (counter removal no-op, can_still_deliver_mail arm deletion, shell owner-exemption removal). Local serialized receipts at head: idle_shutdown 20/0, pending_mcp_boot 1/0, run_completion::tests 7/0, full forkguard_ sweep 307/0, cargo check --all-features --locked green. Local clippy 1.99 shows 8 errors — all at pre-existing sites outside the diff hunks (toolchain drift; CI's check gate is the authority).

Blocking — MAJOR

M-A. Mail parked on an already-resumed predecessor id strands forever and permanently refuses idle retirement

All links proven at head:

1. After a resume, the old record is retained in agents as Interrupted with a continuable checkpoint (receipt note: "prior terminal record stays intact", subagent/mod.rs:6841-6846).
2. A later followup on the old id resolves through the direct agents hit in resolve_agent_ref_inner (mod.rs:8010-8020) — there is no resume_targets redirect — and followup_child_by_id (mod.rs:6330) only refuses Completed/Failed/Cancelled/BudgetExhausted, so it accepts and parks mail on the old id again.
3. That mail has no remaining delivery path: the resume re-key (mod.rs:6854-6860) already ran, and the idempotent-resume branch (mod.rs:6636-6643) follows up the new incarnation's id without migrating queued_mail[old].
4. Yet can_still_deliver_mail (mod.rs:7319-7326) treats the old record as deliverable (Interrupted + continuable checkpoint), so has_live_work_for_session → can_retire_without_losing_work refuses retirement forever. The only escape is the 1h age-eviction in cleanup_inner (mod.rs:8355-8364), which is tool-use-triggered (during idle-to-retirement nothing invokes it) and silently destroys the accepted mail — breaking the queue-only receipt's promise.

The migration comment (mod.rs:6849-6853) acknowledges "followup on the old id can only park further mail on the retained record — it can never deliver", but the gate consequence (permanent STUCK-CLOSE wedge) is unhandled. This is the same wedge class the PR's own re-key was written to fix, reintroduced by post-resume traffic. Suggested directions (any one closes it): redirect/migrate in followup_child_by_id when the id is a resume_targets key; migrate parked mail in the idempotent-resume branch too; or exclude resume_targets-keyed predecessors from can_still_deliver_mail (their mail is by then dead-end bookkeeping, matching the gate's own dead-end doctrine). Please add a test that follows up the old id after a resume and asserts retirement can still commit.

M-B. Sealed-wait window loses autonomous work silently

The preflight is race-free, but between ready.send and the commit crossing, the actor is parked in the two-arm select: no shell wake poll, no completion drain. During this window:

• A parent-owned background shell that finishes leaves a finished-unreported record that nothing will ever claim, and count_residual_inputs (idle_shutdown.rs:96-103) covers only the six input channels — shells and tx_event are not counted, so the module's "the caller surfaces the count instead of letting the engine drop it silently" promise (idle_shutdown.rs:92-95) does not hold for this class. The evidence is lost with the manager.
• A child terminal commit landing in rx_subagent_completion in the same window is counted and warned, but its parent delivery is lost (the record itself is already durably persisted, so the loss is the briefing, not the outcome).

The window is host-controlled and normally sub-millisecond (shutdown_when_idle has no await between predicate and commit send), so likelihood is low — but it is a real loss hole that contradicts the gate comment at idle_shutdown.rs:74-76, which holds only for the synchronous preflight. Cheap fix: include shell-state and completion-queue/tx_event status in the post-commit residual report (or re-run the shell/completion checks after the commit resolves and surface them the same way). Even a documented widening of count_residual_inputs closes the silence.

Non-blocking minors (selected; happy to detail any)

1. Residual loss is structurally unobservable for reserve() permits and direct shared-state writes during the window — documented host contract, but the "never silent" claim over-promises (same class as M-B).
2. Stale-latched cancel token disarms the commit-wait escape (idle_shutdown.rs:128-154): a host blocking forever inside its predicate wedges the whole actor, not just retirement. Documented trade; worth a host-side backpressure note in shutdown_when_idle docs.
3. Second concurrent proposer gets a misleading Err ("retry") after the first proposer commits — the channel is closed, retry can never succeed; wait_run_completion is the actual signal (engine.rs:2245-2247, handle.rs:104-120).
4. queued_mail entries leak when the child goes terminal or is age-evicted without mail pruning (inserts at mod.rs:6270/6400; removals only at 6365/6854). Gate-correct but memory-only leak, KBs per entry, engine-lifetime bound.
5. Cleanup can evict a deliverable-mail child after 1h (cleanup_inner retain) while the gate still treats the mail as accepted work — gate and retention disagree on the lifetime of the same accepted work (related to M-A).
6. followup accepts mail for Interrupted-without-continuable-checkpoint (Ok receipt) which the gate immediately classifies as dead-end bookkeeping; receipt promise already broken at accept time.
7. Boot join has no direct backstop — bounded only transitively by per-server connect timeouts; a large configured timeout delays teardown by that much (engine.rs:8117-8124).
8. Test gaps: concurrent double retirement; engine drop between proposal and receipt; deliverability of migrated mail after resume (depth is asserted, drain is not); the goal_state try_lock contention refuse arm; the post-commit residual warn path. The boot-inserted-connection-before-shutdown_all ordering and a real-engine detached-waiter delivery are also untested (current tests use synthetic receipts).
9. Elegance nits: the six-channel enumeration is triplicated (emptiness gate, residual count, residual warn); the main idle select lacks biased; and leans on the downstream refusal for ordering; Result<bool> encodes four distinct refusals; seed_session_shell_for_test duplicates the ~40-field BackgroundShell literal.

Verdict

Substance is approve-direction once M-A and M-B land — both have small, well-scoped fixes and the existing test scaffolding supports pinning them. Everything else about this PR (root cause, boundaries, honesty of docs, mutation coverage) checks out. Changes requested for the two MAJORs; the minors can be addressed at your discretion or as follow-ups.

[asto18089]

Round-5 fresh review — REQUEST_CHANGES (rebased head 7428f4e3a)

Fresh from-scratch re-review, 12 independent lanes (claims/root-cause, handshake-core, retirement-gates+mail, sealed-window/residuals, engine/handle/shell/run-completion integration, smuggling scan, elegance/boundaries, performance, exceptional states, interleaving hunt, test adequacy, rebase-interaction audit), all at the rebased head. No conclusions reused from earlier rounds.

Rebase: pinvou3-clean had advanced 5 commits (37be2844f → 6f7a3a748). Rebased the PR onto 6f7a3a748 and pushed: head is now 7428f4e3a (all 5 commits patch-identical, git range-diff all =; hunk-by-hunk old-vs-new delta byte-identical, so the auto-merge dropped nothing). CI at push time hit a repo-wide runner outage (every branch's workflows failing with zero steps at the same minute); reruns are in flight — the identical patches were 14/14 green at the previous head, and local gates at this exact tree pass: cargo check --all-features --locked green, forkguard_ 308/0, forkguard_idle_shutdown 21/0, run_completion filter 13/0.

What holds (verified against code, not the body)

• All 6 claims true and root-caused. Base Op::Shutdown races an unbiased select against queued child results and drops finished-shell evidence; the fix is actor-side quiescence certification (not host heuristics), with the persistence half handled by the documented synchronous host predicate. The Round-4 follow-up claims also trace end-to-end: the resume-chain redirect walks resume_targets before status checks and enqueue (subagent/mod.rs:6332-6339), with a genuine real-resume regression; the post-commit residual snapshot runs synchronously at the same boundary before any teardown await and honestly covers inputs, event slots (incl. reserved permits), and shell work (lock-unknown → conservative).
• Round-4 M-A/M-B status. The old-id followup wedge is fixed by the redirect (real delivery consumed from the successor's input, pinned). The sealed-wait window is closed more strongly than the fix itself requires: gate-pass freezes every autonomous producer (child completions publish under the manager write lock before Running clears; shells are pull-polled under their lock; advisor watcher certified by Arc::get_mut with std's Acquire-unique proof) — no autonomous work can land between readiness and commit.
• Handshake core SOUND under every interleaving examined. Commit-beats-cancel is airtight via the ack (biased select + ack-only-on-consumed-commit + Ok(false) when send races the escape); zero latched state on every refusal; no lock across any .await; every gate acquisition is try_lock/fail-safe. Happens-before chains verified on the actual primitives (incl. pending_follow_ups AcqRel/Acquire and std Arc::get_mut ordering).
• Smuggling CLEAN. Every hunk on-claim; all 27 deleted lines traced; per-commit DCO holds; disclosed adjacent changes (boot join, receipt-only waiters, successor-addressed receipts, counter cleanup) are each required and backward-compatible for every in-repo caller.
• Performance SOUND. Hot path (retirement not armed) = one extra zero-allocation recv poll per boundary (~tens of ns, tokio 1.54.1 semantics verified); armed-path costs bounded and proposal-paced; teardown join bound is ceil(N/8) × connect_timeout, not the sum. Not a reinvented wheel — no pre-existing quiescence primitive exists, and the ~165-line production core is near-minimal for the guarantee.
• Rebase interactions COMPATIBLE. #142 adds a wire-invariant error-provenance field with zero new sends/awaits; #150 is a pure function; #152 leaves the project-owned MCP shutdown path untouched; #136's shell.rs edits are region-disjoint from the PR's hunks.
• Tests: all 6 claimed mutations land on specific non-vacuous tests (the newest commit's 2 as well). I live-verified the two Round-4 mutations again at this tree — both still RED; one Lane finding ("cp.continuable guard removal survives the suite") was refuted by live run: child_permission_gate::auto_review_guardian_denial_refuses_the_call_with_a_receipt catches it (767/1). The goal-contention gap below was confirmed by live run: flipping unwrap_or(true) → unwrap_or(false) (uncertain-is-retire) keeps the entire idle-shutdown suite green.

Blocking — MAJOR

M-1. can_still_deliver_mail admits mail no resume can ever deliver → permanent retirement refusal, later silently stranded

can_still_deliver_mail (subagent/mod.rs:7323-7327) treats any Interrupted child with checkpoint.continuable as deliverable. But every actual delivery path additionally requires non-empty messages: the resume filter cp.continuable && !cp.messages.is_empty() (mod.rs:6687-6688), the tool-layer should_resume (coord.rs:395-403), and subagent_checkpoint_is_continuable (mod.rs:8723-8731). Meanwhile interrupt_child_by_id seeds exactly the excluded shape for children interrupted before their first checkpoint: build_subagent_checkpoint(&agent_id, &reason, &[], agent.steps_taken, true) (mod.rs:6910-6913) — continuable=true, zero messages.

Reachable flow: spawn → interrupt before the first checkpoint (admission window) → any queue-only followup parks mail → the record accepts mail the gate counts as live work, yet no resume, drain, or status transition can ever clear it (cancel/cleanup stale-cancel act only on Running; terminal records are immutable). shutdown_when_idle then refuses forever; the only escape is op-driven 1h age eviction, which removes the record without inspecting queued_mail (mod.rs:8378-8387) — silently stranding the accepted mail the gate had protected. This is the same "bookkeeping no drain can clear" class the PR's own gate doctrine excludes (mod.rs:8560-8562), on the refuse side. The PR's own test fixtures use exactly this shape (insert_test_interrupted_continuable_agent(..., Vec::new())) and assert the refusal as correct "mail a resume could still deliver" — a resume cannot deliver it.

Fix is one predicate: align can_still_deliver_mail with the resume filter (continuable && !messages.is_empty()), turning such mail into ordinary dead-end bookkeeping. Add a test that interrupts a child before its first checkpoint, parks mail, and asserts the gate is idle.

M-2. Resume-migrated mail is never delivered, the resume receipt misreports queue_depth: 0, and the mail is silently lost when the successor terminalizes

The resume re-key (mod.rs:6858-6864) moves queued_mail[predecessor] onto the successor but never delivers it: build_resume_prompt carries only the new followup text, the spawn does not flush the mailbox, and the only drain is a subsequent followup hitting the Running arm (mod.rs:6371). The receipt built at mod.rs:6842-6853 hardcodes queue_depth: 0, woke: true — the caller is told the queue is empty while N accepted messages were just parked, undelivered, on the successor (the PR's own migration test pins exactly this parked state).

If the successor terminalizes before that further followup, finish_terminal_result (mod.rs:8540-8585) drops the pending_follow_ups counter (with a warn) but never touches queued_mail; the gate then classifies the mail as dead-end bookkeeping (terminal status), retirement_residuals does not observe queued_mail, and retirement commits over silently lost accepted work with no diagnostic — the exact STUCK-OPEN shape this PR exists to prevent, on an ordinary flow: followup parked on a busy child → interrupt → resume → successor finishes its turn → retire.

Fix (small): flush migrated mail into the successor's input at resume time (the Running-arm drain path already exists and is pinned), and report the real queue depth in the resume receipt. Add a test that parks mail, resumes, and consumes the migrated message content from the successor's input without any further followup.

Non-blocking (selected minors; happy to detail any)

1. Test gap (live-confirmed): the goal_state try_lock contention refuse arm (unwrap_or(true), idle_shutdown.rs:53-60) is unpinned — mutating it to "uncertain is retire" keeps the whole idle-shutdown suite green (I verified). Cheap test: hold the shared Arc lock in a spawned task, assert refusal.
2. Result<bool> + strings encodes 8 distinct outcomes; an Err can't distinguish "engine gone" from "another proposer committed" without cross-referencing wait_run_completion (documented in prose). A 4-variant enum is enforceable — cheapest now, while there are zero production callers.
3. The six-input-channel enumeration is hand-written three times (gate idle_shutdown.rs:82-88, residuals :97-102, warn engine.rs:3456-3461); a future seventh channel updated inconsistently silently defeats the gate. One named struct would be the single point of truth.
4. A host goal-state flip during the sealed window is the one gate category both mutable in-window and unobservable post-commit (residuals never re-read goal_state); one boolean would close it.
5. Stale-latched token + hung predicate seals the actor so completely that even Op::Shutdown is no longer serviced (only the escape-less commit or runtime abort ends it). Disclosed as a trade; the docs mention predicate promptness but not the Shutdown starvation.
6. shutdown_mcp_pool's boot join has no explicit deadline (bounded by max per-server connect timeout, ceil(N/8) × timeout); worth one doc line. The join-before-shutdown_all ordering itself is verified correct on all teardown paths but only pinned with an empty synthetic pool.
7. Mail/bookkeeping maps are never GC'd for evicted agents (resume_targets ~130 B/cycle, queued_mail message-sized; engine-lifetime bound), and the redirect is unreachable for an age-evicted predecessor through resolve_agent_ref_inner (hard-errors first, mod.rs:8028-8034 — fail-closed, truthful, but "follow-ups belong to the latest incarnation" only holds within the retention window).
8. Post-commit residual loss (contract violations, boot settlement) is tracing::warn-only and the warn block is never executed by any run()-driving test; persistence failure on the retirement tail is likewise warn-only while completion stays NormalReturn (truthfully disclaimed, easy to over-trust in embeddings).
9. Test pins worth adding while cheap: second-proposer-after-commit resolution, migrated-mail content delivery (currently map-move only; the FIFO drain is pinned separately), a two-hop resume chain (a→b→c).
10. Elegance nits: third ~33-field BackgroundShell test literal (the PR's own seed_agent_shell_for_test shows the better delegate-and-patch pattern); holds_write_scope() not reused at shell.rs:3633; gate runs the full agent/shell scan before the cheap queue-emptiness checks (reorder); un-biased idle select costs one wasted round-trip per collision; resume-chain walk would be wedged by a hypothetical cycle (acyclic today — a debug bound is cheap insurance); the newest commit's one-line message doesn't itemize its doc/test changes.

Verdict

Substance remains approve-direction — root cause, boundaries, concurrency discipline, docs honesty, mutation coverage, and rebase compatibility all check out, and the Round-4 follow-up genuinely fixed the old-id wedge. But M-1 and M-2 are real holes in the PR's own core promise ("preserves accepted work"): one permanently wedges retirement on mail that looks deliverable but isn't, the other silently drops accepted mail after a resume while telling the caller the queue is empty. Both have small, well-scoped fixes and the existing test scaffolding pins them naturally. Changes requested for the two MAJORs; minors at your discretion.

[zhuowp]

No-Issue：本仓库未启用 Issues；通用引擎生命周期修复，应用侧消费由 pinvou-agent #839 单独跟进。

以真实目标 pinvou3-clean 的累计差异为准：引擎在完整 actor 操作之间执行 ready/commit 退场握手，检查目标、子代理、shell 和待交付结果；退场阶段等待 MCP boot 后再关闭连接池。宿主须在同步 predicate 与 commit 期间排除新外部提交。完成回执不保活引擎或事件队列，不新增 wire 字段、依赖或产品专用卸载策略。

本轮修复三个相关断点：空的 Interrupted checkpoint 不再错误阻塞退场；恢复时将旧代已接受的父消息按 FIFO 放入真实初始 User 输入，成功创建后才清旧账，并保护首轮执行前再次中断；活跃代已接受的正文由共享票据持有，只有实际 checkpoint 保留对应消息后才确认，避免中断丢失。既有历史软预算仍生效，仅保护当前尚未确认的输入批次，保留身份重定向、终态拒绝和错误语义。

验证：固定提交树，Windows / Rust 1.99 默认 owning TUI libtest；项目 scripts/dev-cargo.sh 锁定依赖编译。idle 23、parent-mail 5、existing-resume 1、identity 11，共 40 项通过、零失败。六处退化负控各由预期行为断言拒绝，恢复后通过。最终测试链接使用仅限本地测试目标的 bundled LLD（不修改仓库或全局配置），同一组 40 项再次通过，资源 Job 自然清退；六个负控按源码、目标、依赖和 profile 等价有限继承，没有宣称 LLD 路径重跑了负控。旧 MSVC 链接路径的资源门禁失败记录保留。

本轮三文件修复已完成五角色独立首审、复核、非实现者 v3 裁决与实际提交映射。历史测试和全量 sweep 不作为本轮重跑证据；未验证 Linux/all-features 全门禁、真实远端模型、全部 MCP 实物、release 或完整宿主 E2E。新提交须重新外审；CodeWhale 不自动合入。

长期 mail admission/retention/GC 等非阻断跟踪继续见 pinvou-agent #845。codewhale-hq#839 仍指向旧候选，本轮不声称应用已消费最终修复；后续需单独更新 gitlink、fork 登记、指纹与 guard。

本轮修复提交：d41467e213160aa2ea3f45046e7e5ae106fd3ded。

@asto18089 请复核新版本。修改后旧批准不沿用；本轮只交付修复并请求复审，不自动合入。