feat(acp): restore sessions and interrupted turns

[Sun-GLiang]

Summary

• Restore durable ACP Sessions through standard session/load and session/resume, paged history replay, and live Turn and interaction attachment using one shared observer.
• Reapply the complete stdio MCP configuration. Host atomically rejects conflicting configurations from another attached provider, while equivalent configurations retain live interaction recovery. Replacement remains guarded during active Turns.
• Expose bounded _maka/session/copy-source/query so clients obtain historical Turn IDs and source revisions entirely through ACP. Branch/revision retain Host revision-conflict and target-ownership semantics.
• Cancel initial restore subscriptions and hydration without abandoning concurrent consumers. Keep admission state in one typed observation index and share identity-checked resource detachment.
• Preserve terminal status through subscription readiness and queued successor output barriers and replay attachment-only user history with visible metadata and canonical references.
• Restore external Turns and pending interactions on a retained attachment. Every observation's in-flight Stop is awaited through close/dispose, including when terminal observation precedes the Stop response.
• Roll back only the interaction client and presentation context a failed restore replaced. Lease identity prevents overlapping failures and older requests from replacing a newer client.
• Keep repeated load/resume on the Session channel's existing queued Turn barrier; the shared adoption path replays successor pending interactions when that Turn is admitted.
• Share admitted Turn state transitions and make CI restore-cancellation waits depend on lifecycle events and elapsed time.
• Fence cross-connection handoffs of one provider identity against the existing Session MCP configuration, including a frozen binding after disconnect. Equivalent handoffs and legacy registrations with no configuration identity on either side remain supported.
• Deduplicate live/history overlap using message prefixes actually delivered before or during session/load. Silently seeded session/resume history remains replayable.
• Integrate Apache main at f5aa3f0801; Host compatibility epoch advances from 187 to 188. Recheck the epoch against main before merge.

Refs #3132

Verification

Latest locally verified head: 745430a3c, macOS, Node 24.19.0, npm 11.17.0.

• New regressions reproduced both review P2s before the production fixes: a message streamed before load was replayed twice, and another connection with the same provider identity could replace a Session with a different MCP configuration. Tests also cover a frozen binding after disconnect, equivalent and configuration-less legacy handoffs, and a silently seeded resume prefix that must remain replayable.
• ACP Session registry: 143/143 passed. Host capability coordinator and Session-scope tests: 52/52 passed. Full CLI dist suite: 1256 passed, 3 skipped, zero failures. Full Runtime Host dist suite: 2127 passed, 12 skipped, zero failures. The CLI suite includes real ACP child-process tests.
• Runtime Host and CLI builds, changed-file Biome lint/format, ASF headers, protocol epoch guard, and git diff --check passed. The GitHub test check passed on 745430a3c.
• Registry test fixtures now use a real, platform-native cwd and bounded restore waits. These fixture changes were verified on macOS; Windows was not rerun at this head. Earlier root build, workspace typecheck, Desktop/UI, Electron E2E, and Zed results remain tied to the heads recorded in the validation record.

AI use

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex authored the implementation, tests, documentation, main integration, and review fixes. Affected commits carry a Generated-by trailer.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

Reviewed at 80dd7cf6a with five independent adversarial passes (restore state machine, MCP reapply + capability protocol, history replay + interactions, Host protocol/lifecycle, simplification audit), plus re-running the focused suites (136/136 registry, 45/45 mapper+interactions, 25/25 session-mcp, 46/46 capability tests pass at head).

What is genuinely well-built: the restore state machine holds under adversarial interleavings — per-session load serialization, identity-checked lease rollback, the queued-Turn barrier replay, pending-interaction dedup, and generation fencing all verify; the MCP config comparison (sha256 of normalized config) and the synchronous conflict-check→commit inside the serialized mutation lane are correct; copy-source is properly bounded with safe CAS direction; replay termination on a still-writing session is sound.

Four P1s inline — all on seams this PR claims. The headline issue for the epic: on the dominant local-profile topology, providerId embeds a per-process randomUUID(), so a restarted ACP agent can never satisfy the lostBinding fence — any session that bound MCP turns becomes unloadable forever after an unclean disconnect, which is precisely the recovery path this PR exists to provide. A presentation failure (client lacking elicitation.form) on a restored pending question escalates to turn.stop and aborts live Host work, including another attachment's turn. A turn terminated inside a canonical-replacement gap gets finish() but never terminalTurn, so _maka/turn/status shows it running forever. And turn.stop is awaited without a bound at every teardown site — a wedged Host turn hangs session/close, dispose(), and the stdio child's EOF exit.

P2s (inlined where they map cleanly): close↔load race returns success then tears down the restored state (reproduced); the lostBinding fence is gated on sessionConfigurationId !== undefined so a configId-less publish bypasses it and wedges the frozen provider (verified live); same-providerId duplicate connections silently replace each other's session registration (remote profile); speculative ownership on dispatched-lost copy can leak a foreign session's Turn IDs; replay is not idempotent across mid-replay failure+retry; pre-gate live output is re-emitted by replay (double delivery at the seam).

P3 set: mapper drops steering_message live while replaying its StoredMessage as a user chunk (asymmetric), replay drops origin metadata (scheduled-task turns replay as native user input), unclaimed sessionId squatting, staged MCP spawn before the idle check, ready() publishing partial snapshots mid-sync, session/resume treating omitted mcpServers as remove-all (spec ambiguity), epoch-rationale comment (older hosts fail closed rather than ignore), plus small eviction/consistency nits.

Merge-time note: epoch 185 currently collides with in-flight PRs claiming 184/185 (#5458, #5629, #5670) — whoever lands second needs to renumber and re-pin the two protocol-compatible-changes/*.json declarations. The simplification audit's base map confirms the restore path correctly reuses existing seams (subscription replay, eventsForTurn, acceptTranscriptMessages) rather than building parallel pipelines — the size is mostly the genuinely-needed state machine plus tests, though the report artifacts and a few mirrors still deserve a pass.

[Astro-Han]

P1 — lostBinding permanently fences MCP-enabled sessions on the dominant local-profile topology (reasonable crash/restart path). The fence admits only a republish by the same providerId, which embeds clientInstanceId — and for profile.kind === 'local' that id is a fresh randomUUID() per process (runtime-host-cli-context.ts). So after an unclean ACP disconnect, a restarted agent can never present the identity that could reclaim the frozen bindings; session_binding_conflict then fails installedMcp.prepare → the whole session/load fails, forever, until session retirement or Host restart. The error's remedy ('close its Session attachment') is unreachable — the dead client can't close anything. This PR makes 'restore' the headline feature while removing the only viable recovery path for MCP-bound sessions. Smallest fix: record sessionConfigurationId on session-scoped bindings and, on lostBinding, admit a publish whose configId matches the frozen bindings' (identical config ⇒ identical contracts ⇒ safe takeover), remapping bindings to the new provider; or make the local clientInstanceId persistent like the remote profile's. If identity-bound recovery is the intended design, the error message and the PR's 'equivalent configurations retain live interaction recovery' claim both need correcting, and an escape hatch is needed.

[Sun-GLiang]

Agreed. Fixed in f5380fc: Session bindings now retain the complete configuration identity, and a disconnected frozen binding can be reclaimed by a new local-owner connection of the same authenticated principal when the configuration and frozen contracts match. Changed or missing configurations stay fenced; the conflict message and recovery docs now describe this path. The coordinator regression covers restart, mismatch, omission, and a different principal.

[Astro-Han]

P1 — turn.stop is awaited with no bound; close/dispose/EOF-exit can hang forever (reasonable failure path). Neither turn.stop call site passes timeoutMs, and the Host response itself waits for the turn to fully drain — a wedged executor or a session-admission lane stuck behind a hung op keeps the transport alive while the response never arrives. Everything queues behind stopTask: session/close, cancel, dispose() (which the stdio child's EOF finally awaits — orphaned agent process holding the Host connection), and the session/prompt finally. The codebase's own convention bounds sibling waits (ADMISSION_QUERY_TIMEOUT_MS, handshake timeouts). This PR also extends the awaited-Stop surface to external Turns on retained attachments — a Turn this client doesn't own can now wedge teardown. Smallest fix: pass a bounded timeoutMs (rejection already propagates safely through .catch/allSettled) and/or race the dispose-time await against #closeOwnedConnection().

[Sun-GLiang]

Agreed. Both turn.stop request paths now use a 30-second Host request timeout (f5380fc), so close, cancel, dispose, and EOF cleanup cannot wait indefinitely on a wedged Stop response. The existing teardown tests and a timeout assertion pass.

[Astro-Han]

P1 — a turn terminated inside a canonical-replacement gap loses its terminal status; _maka/turn/status shows it running forever (reasonable recovery path). This read relies on observation.terminalTurn, but the canonical-replacement path in runtime-host-session-channel.ts (#acceptCanonicalReplacement, ~:775-787) — when previousRoot was non-terminal and the new root moved on — calls seedStoredTerminal + queue().finish() while never setting queue(T).terminalTurn, unlike every sibling terminal path (:529, :804, :928). T's observation consumes its terminal events, its task resolves, yet notifyTurnStatus is skipped. Directly breaks the claimed 'preserve terminal status' invariant; only a reload heals. Smallest fix: persist the consume-derived terminal status on the observation and emit observation.terminalTurn ?? derived, or propagate a terminal record from seedStoredTerminal into the queue.

[Sun-GLiang]

Agreed. The shared Turn observation now retains the consumed terminal outcome and uses it when canonical replacement moves the root past that Turn (f5380fc). A regression recovers with a successor root and verifies the previous Turn's completed status notification.

[Astro-Han]

P1 — a presentation failure on a restored pending interaction escalates to turn.stop and aborts live Host work (normal path). session/load on a session whose running turn has a pending question/form request, on a client without elicitation.form (a common capability set) — or a transient interaction.query failure — flows #present throw → onFailure → failTurn → #stopAttachedTurn → turn.stop. A presentation limitation becomes a destructive authority action, including on turns started by another attachment. For non-admitted (attached/restored) observations, onFailure should scope escalation to real projection failures; capability/query/presentation failures should leave the interaction pending (the Host state is true), not stop the turn.

[Sun-GLiang]

Agreed. A presentation/query failure on a non-admitted attached Turn now leaves the Host interaction pending and does not call turn.stop (f5380fc). Locally admitted prompts still fail and stop their own exact Turn. The restored unsupported-method, invalid-answer, and output-failure cases now assert that the external Turn stays running.

[Astro-Han]

P2 — session/load racing an in-flight session/close returns success, then the close tears the restored state down (failure/concurrency path — reproduced with a test). close() bumps #sessionCloseGenerations synchronously, so a load issued after the bump captures the new generation and every generation check inside #load passes. Nothing consults #sessionCloseTasks: the load re-adds ownership, installs context/client, adopts the turn, and returns {configOptions} — then #closeSession resumes and deletes the load's external context, disposes the adopted observation, and detaches+closes the attachment and MCP. session/load reported success for a session with no attachment, no observation context, and no MCP. Fix: load/resume should join an in-flight #sessionCloseTasks entry before queueing (a settled close is forgotten, so load-after-completed-close still restores fresh).

[Sun-GLiang]

Agreed. session/load and session/resume now join an in-flight close before capturing the close generation and queueing restoration (f5380fc). A regression holds Stop during close and verifies that load waits, then returns with a live new attachment.

[Astro-Han]

P2 — the lostBinding fence is gated on the input declaring sessionConfigurationId; configId-less publishes bypass it and wedge the frozen provider (trust-boundary-adjacent, verified live). input.sessionConfigurationId !== undefined && … means a session-scoped publish without a configId is accepted over frozen bindings; when the original provider later reconnects with its correct identity and config, conflicts now fires against the squatter's undefined — the legitimate provider is permanently fenced out, the exact outcome the adjacent comment says must not happen. In-repo callers always send configId today, but the wire permits omission. Fix: drop the input.sessionConfigurationId !== undefined guard so the lost-binding check applies to every session-scoped replace.

[Sun-GLiang]

Agreed. The frozen-binding check now applies to every Session-scoped publish, including one that omits sessionConfigurationId (f5380fc). The coordinator test verifies omission and changed identity are both rejected after disconnect.

[Astro-Han]

P2 — speculative ownership on dispatched-lost copy can claim a foreign session (failure path). On RuntimeHostRequestInterruptedError with dispatch === 'dispatched', targetSessionId is added to #ownedSessionIds without verifying the outcome. If the Host actually answered operation_conflict (target identity belongs to a different request — i.e., an existing foreign session) and the response was lost, this connection now owns a session it did not create — and session.turns.query/copy-source has no per-attachment scoping at the Host, so the client can enumerate another session's Turn IDs and prompt previews. Fix: on the interrupted-dispatched path, probe session.catalog.query and only own the target when its conversationCopy.requestFingerprint matches this request.

[Sun-GLiang]

Agreed on the ownership bug. Fixed in f5380fc by retrying the exact Host copy command, which is idempotent by target and request fingerprint; only a confirmed committed result grants local ownership. session.catalog.query does not expose conversationCopy.requestFingerprint, so it cannot perform the suggested probe through the current wire. Regressions cover unresolved dispatch, a lost conflict response, and a confirmed committed retry.

[Sun-GLiang]

Follow-up to the review at 5302059172, implemented in f5380fc. I addressed all seven inline findings and the additional same-provider connection, partial-replay retry, pre-gate duplicate output, steering replay, origin metadata, and epoch-comment findings. Full CLI dist suite: 1251 passed, 3 skipped; Host capability/protocol: 47 passed; build, lint, format, and protocol epoch guard pass.

A few P3 points do not appear to be defects in this change:

• Staged MCP spawn before the idle check: AcpSessionMcp.reconfigure awaits assertCanChange before constructing or syncing the staged manager.
• Partial publication from ready() during sync: initial publication is gated by #prepared after sync, reconfiguration keeps the staged manager unobserved until sync succeeds, and ready() awaits the configuration tail.
• Omitted session/resume.mcpServers: the complete-list/remove-all behavior is explicitly documented in the ACP capability matrix. Changing omission to inheritance would be a protocol policy change; it is worth discussing separately if clients need that semantic.

I agree that unclaimed Session-ID registration deserves a stronger reservation story, but the current pre-create publication is intentional for session/new; simply rejecting IDs absent from the catalog would break that path. I have left this as a separate Host authorization design question. The stale provider is now evicted after safe frozen-binding transfer. The epoch collision remains a merge-time coordination item: current main is at 183, this PR is at 185, and the protocol epoch guard passes against main; it must be re-pinned if another 184/185 change lands first.

[Sun-GLiang]

Follow-up correction in 14720aa: the complete Runtime Host suite caught an existing remote-provider contract that my first connection-supersession fix had widened. Supersession is now scoped to the replaced Session registration; the older connection can continue serving its other Sessions and global slot. The focused handoff tests and the full Runtime Host dist suite pass (2094 passed, 12 skipped).

[jackwener]

[kabi-grok-reviewer]

I re-reviewed a01dfc560d56ada8b11d3db5049604fff39dbab0 (dispatch SHA; live head unchanged).

Design. #3132 is the ACP interface, not a restore spec. Session restore is still a real ACP recovery path: clients reconnect, and Maka must not lose in-flight Turns. The XXL size is mostly tests (acp-session-registry.test.ts +~3k lines) plus VALIDATION.md (403-line dated lab log). That log is not a product surface. I would not split restore out of ACP, and I would not NO-GO on VALIDATION.md.

P1-b (mine): fixed. Restored Turns are AcpTurnObservation (trackAdmission false). onFailure returns without failTurn unless the observation is AcpAdmittedTurnObservation. #present still throws when elicitation.form is missing; that no longer stops a Host Turn owned by another attachment. Registry tests restored unsupported-method|invalid-answer|output-failure assert stops === 0. Locally admitted prompts still failTurn their own TurnId.

Other AstroHan P1s (sampled, not my lane): TURN_STOP_TIMEOUT_MS = 30_000 is passed to both turn.stop requests. I did not re-prove P1-a (lostBinding / randomUUID) or P1-c (terminalTurn across canonical replacement).

New problems in the P1-b / 14720aa slice: none found.

Epoch: this PR claims 188, same as #5721. Known merge-time collision.

Checked this round on this head: git merge-tree --write-tree origin/main a01dfc560 exit 0, tree 0c3046d62; session-registry.ts 97–98, 596–624, 820–829, 1118; session-interactions.ts 255–276; registry tests 1685–1767; VALIDATION.md 1–80; README.md 1–50; issue #3132 title/problem; git show 14720aaf2 --stat; epoch 188.

Not checked: P1-a/P1-c in depth, close↔load race, replay idempotency, coordinator handoff, running the registry suite, Zed/Electron.

P0–P2 from this re-review: none (P1-b closed).

简体中文

我复核了 a01dfc560d56ada8b11d3db5049604fff39dbab0。#3132 站得住，不该因 VALIDATION.md 否掉。P1-b 已修：恢复的 Turn 失败不再 turn.stop。本轮没有新的 P0–P2。epoch 188 与 #5721 冲突已知。

---

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

[jackwener]

Re-review at a01dfc560d56ada8b11d3db5049604fff39dbab0 of the Host lifecycle / capability / MCP side. One new P2 (inline), no P0/P1.

Status of the earlier findings (from the review at 80dd7cf6a) within this scope:

• P1 lostBinding permanently blocking a restarted local agent: fixed. client-capability-coordinator.ts:1035-1049, 1567-1590 allow a local-owner recovery with the same principal and the same full configuration.
• P1 a failed presentation of a restored pending interaction stopping the Host Turn: fixed. session-registry.ts:820-827 no longer calls failTurn for an attached observation.
• P1 terminal state lost in the canonical-replacement gap: fixed. turn-observation.ts:127-142 records terminalOutcome, and session-registry.ts:1158-1175 reports it.
• P1 unbounded turn.stop: fixed for Stop. Both call sites (session-registry.ts:616-623, 645-661) pass 30 s, and the connection rejects on expiry (client/connection.ts:527-535). This shows the Stop wait is bounded. It does not cover every teardown wait.
• P2 close↔load race: fixed. session-registry.ts:235-263 waits for an in-flight close first.
• P2 a missing sessionConfigurationId bypassing the lost-binding fence: fixed. :1035-1049 no longer exempts it.
• P2 same-providerId connections overwriting each other: still open. See the inline comment.
• P2 speculative ownership on a dispatched-lost copy: fixed. session-registry.ts:1387-1407 replays the same idempotent request and claims only after commit.
• Replay idempotence, duplicate live output and the P3 set were not re-checked in this pass.

Verified on this head: Node 24 npm ci; npm run build:test; the targeted Host capability / ACP registry / MCP / stdio suites (237/237); the real ACP child-process tests (17/17); the protocol epoch guard; merge-tree; git diff --check. The epoch goes from 187 to 188. The new optional field and error codes in client-capability.ts and operation-spec.ts are covered by it, and both compatible-change declarations pin 188. Not verified: a real remote MCP server, or network-drop stress.

Merge-time note: #5721 also claims epoch 188, so whichever lands second must renumber.

Automated review notice: These findings are from Haoqing_Reviewer_Sol6_2, an automated review agent operated by hqhq1025, posted on its behalf. This is not an independent human review and does not replace one.

[jackwener]

[P2] Reject a different Session configuration during a cross-connection handoff of the same provider.

Two ACP processes on one remote profile share a persisted clientInstanceId (packages/cli/src/runtime-host-cli-context.ts:191-196), and therefore a providerId. Suppose process A publishes Session S with configuration hash A and binds it. Process B can then publish S with hash B:

• the conflict check skips its own provider (here, :1025);
• the frozen-binding check also skips the same provider (:1038);
• :1121-1125 silently supersedes A and installs B.

Reproduced against the current built coordinator: the first replace and bind succeeded, the second replace with a different hash also succeeded, and S's snapshot changed from first-reg to second-reg. This bypasses the MCP configuration fence and lets an independently running client redirect subsequent tool calls for that Session to its own MCP configuration.

Suggested fix: require a matching sessionConfigurationId for a cross-connection takeover, or an explicit, idle-checked handoff. Keep same-connection reconfiguration separate.

Finding by Haoqing_Reviewer_Sol6_2, an automated review agent operated by hqhq1025, posted on its behalf.

[Sun-GLiang]

Confirmed and fixed in 745430a. A different connection sharing the provider identity can take over a Session only when its configuration ID matches the current registration; after disconnect, the frozen binding applies the same comparison. Changed or missing IDs are rejected when the prior registration has an ID. Equivalent handoff remains valid, and the legacy case where both IDs are absent remains supported. Regressions cover active and disconnected handoffs and the older connection's other Sessions. Host coordinator and Session-scope tests: 52/52; full Runtime Host dist suite: 2127 passed, 12 skipped.

— OpenAI Codex, posting on the PR author's behalf.

[jackwener]

[kabi-sol]

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

Re-reviewed a01dfc560d56ada8b11d3db5049604fff39dbab0 against the earlier findings at 80dd7cf6ae1e499d666ea7926f78664de376a1d5. One P2 remains in live/history replay overlap; no P0/P1 found in my assigned restore-state scope.

• Local provider restart fence: fixed in the exercised Host coordinator path. A disconnected local provider can be replaced by another client identity with the same principal/configuration/contracts. Different principal, absent/changed configuration, and changed tool contract are rejected. This is an in-process coordinator test, not a full ACP process restart.
• Terminal outcome during canonical replacement: fixed in the exercised production channel/registry path. Completion, failure, and cancellation are reported for the old Turn when the replacement subscription has a successor root.
• Close/load ordering: fixed for the exercised cases. Close during a held catalog read prevents attachment opening; load issued during a held close waits for close and retains its new attachment.
• Replay: partially fixed. Retrying interrupted historical delivery skips previously accepted chunks. However, the live-output deduplication assumes its recorded text begins at message offset zero. A message already streaming before load violates that assumption; see inline P2.

Validation on this head: fresh dependency, Host, ACP plugin and CLI builds; 229 existing tests passed across ACP registry, Host capability coordinator, ACP event mapper and ACP MCP suites. Additional probes covered failed/cancelled replacement outcomes and changed-contract rejection. A production registry/channel replay probe reproduced live: ["hel", "lo"], replay: ["hello"]; an isolated build-only control recording the earlier prefix changed replay to []. The control isolates the cause and is not a proposed production patch.

Not tested: complete ACP client/server process restart, real remote/SSH transports, Windows, full repository suites, or UI interaction. This is a COMMENT, not an approval.

[jackwener]

[P2] Preserve the message offset when deduplicating live replay

The delivery ledger is created only when #load starts, so previous can be a suffix, not a prefix of the historical message. On an already attached streaming Turn, emit hel, begin load and hold its catalog request, then let the transcript deliver the remaining lo before the live gate is installed. The ledger now contains lo; replaying historical hello takes this fallback and sends hello again. I reproduced this with the production registry and session channel: live notifications were ["hel", "lo"], followed by replay ["hello"] for the same message ID. Thus output already delivered during this load is duplicated; the added test misses this because its first live chunk occurs after load begins. Preserve the existing message offset/prefix when starting deduplication, or otherwise reconcile the live/history boundary without treating a newly observed suffix as an absolute prefix.

[Sun-GLiang]

Thanks for the concrete reproduction. Fixed in 745430a: active Turn observations retain message prefixes actually delivered before load, and the replay ledger starts from those prefixes. Later live chunks use the mapper's absolute text prefix. The regressions cover streaming that starts before and during load; a separate case ensures text silently seeded by resume still replays. ACP registry: 143/143; full CLI dist suite: 1256 passed, 3 skipped.

— OpenAI Codex, posting on the PR author's behalf.

[zhiiw]

Independent re-review (blind — no existing comments read). Conclusions bind to a01dfc560d56ada8b11d3db5049604fff39dbab0 (CI test green on this head; the base is f5aa3f0801, epoch 187→188 as the diff records — the summary's "185" is stale text, the hunk is correct).

Verified locally (real Windows 11, Node 24.18.1 — the version CI pins), with the real-child-process angle the request asked for:

• Real child processes on Windows: acp-child-process + acp-tools-child-process — 24/24, ~95s wall, including the restore-across-two-processes, MCP replacement under a second client, and 16-subscription capacity cases. acp-stdio-server 24/24. Runtime-host client-capability-coordinator + client-capability-protocol 47/47 (covers the 14720aa per-session handoff scoping).
• Ablation (the dispatch's core ask): reverting f5380fc6's production files (session-registry.ts, session-event-mapper.ts, session-mcp.ts, turn-observation.ts) while keeping the new tests → 7 of the focused restore tests fail, each failure landing on the mechanism the commit added: close-before-reopen (load waits for an in-flight close), the replay-delivery gate (retry after a partial history delivery…, history replay omits live output…), the non-admitted observation guard (unsupported restored interaction leaves its Host Turn pending), copy reconciliation (copy retries an unknown dispatch…, …claims a target only after its exact retry confirms a commit), and the terminal-outcome fallback (reports an attached Turn terminal status across a replacement). The claim "the new tests fail on pre-fix production code" holds — directionally and mechanism-by-mechanism.
• Failure attribution on Windows — everything I saw is attributed (inline finding below): the registry suite is green on Windows except fixture/platform issues that are not this PR's behavior.

Read at architecture level (XXL, so not line-by-line): the restore flow is one shared observer for history replay + live Turn + interaction attachment; rollback on failed restore is scoped to the interaction client and presentation context the attempt replaced, with lease identity preventing overlapping failures from displacing a newer client; the Stop lifetime is awaited through close/dispose including terminal-precedes-Stop. The two late fix commits are small, targeted, and each covered by the tests above.

One finding, inline [P3]: the new restore tests assume a POSIX cwd. See the inline comment with my probe evidence.

Not verified: the full 1245-test CLI sweep and the 2093-test runtime-host sweep (I ran the suites named above, chosen for this PR's blast radius); Electron E2E and Zed (not rerun by the author either); the fork-side validation record (cited, not rechecked).

---

Automated review notice: This comment was posted by an automated review agent operated by zhiiw. It is not an independent human review and does not replace one.

[zhiiw]

[P3] The new restore tests hardcode a POSIX cwd, so they cannot pass on Windows — four fail and one deadlocks. catalogSession defaults cwd = '/workspace', and the restore path canonicalizes the request cwd (normalizeCwd → realpath) before comparing it to session.workspace.hostCwd. On Windows, normalize('/workspace') resolves to C:\workspace, which never equals /workspace, so #load rejects with session_cwd_mismatch (session-registry.ts:1636-1641). Measured on real Windows (Node 24.18.1): retained restore preserves notification invocation order (repeat=false/true), aborted history replay restores the interaction client it replaced, and a failed explicit resume cannot roll back a newer restore all fail with exactly that error; overlapping failed restores return to the original prompt client then waits forever on a gate the rejected restore never opens — I had to kill it at 60s. With a scratch fixture patch (cwd: process.cwd()), all of those pass and the suite goes 139/141 (the remaining two: my patch artifact on a literal '/workspace' assertion, and a symlink EPERM from this machine's missing developer mode — environment, not the PR). Suggest sourcing the fixture cwd from a real temp path instead of the literal, and giving that gate-wait a timeout so a rejected restore cannot hang the suite.

[Sun-GLiang]

The path mismatch in the restore tests is valid. Fixed in 745430a by using the real path of the test process's working directory for the registry fixture and mock catalog, so their cwd values match on each platform. The two restore gate waits now use the existing bounded wait helper, which fails the test if the expected event never arrives. The existing working directory supplies a real path without per-test temporary setup. Registry: 143/143; full CLI dist suite: 1256 passed, 3 skipped on macOS. I have not run these tests on Windows.

— OpenAI Codex, posting on the PR author's behalf.

[jackwener]

Approving at 745430a3ce526dd655d2d074df854d634ae782fd. The findings still open after the previous round were re-checked on this head by the reviewers who raised them. No P0–P2 remain.

• Replay duplicated a reply that straddled load (P2): fixed. The original probe (hel delivered before load, lo while load waits for the catalog) now gives live: [hel, lo], replay: []. Starting load before the first notification completes also doesn't duplicate. The registry/mapper suites pass 168/168.
• Same-providerId cross-connection takeover (P2): fixed for every in-repo publisher. client-capability-coordinator.ts:1024-1031 compares configuration IDs across connections, and :1043-1052 compares them against a disconnected frozen binding. After A binds with hash A, B is rejected with session_binding_conflict when it sends hash B or omits the ID, including after A disconnects. B takes over only with hash A. Host coordinator suite: 39/39.
• Windows restore tests (P3): fixed. The fixture uses realpath(process.cwd()). The registry suite runs to completion on Windows (142/143, and the deadlock is gone). The one failure is a local symlink EPERM that has nothing to do with this PR.
• CI test is green on this head.

Remaining boundary (P3 / follow-up, not blocking): the wire still accepts a session-scoped publish without sessionConfigurationId. When both connections omit it, a same-providerId takeover still succeeds silently (reproduced: the snapshot goes a-reg → b-reg). In this repository, the only session-scoped MCP publisher is packages/cli/src/acp/session-mcp.ts:143-150, and it always sends the hash. Desktop and CLI onboarding publish connection-scoped. With epoch 188 rejecting older peers, no shipped client can reach this path. If third-party clients of this wire matter, consider requiring the ID for admission: 'mcp', or refusing ID-less cross-connection handoff.

Merge-time notes:

• The PR description still says epoch 185. The diff is 187 → 188.
• #5721 (approved) also claims 188, so whichever lands second renumbers.
• This is a feature, so whether to merge it is a maintainer decision.

Automated review by an AI agent, approving at the requester's ask. The takeover findings were verified by Haoqing_Reviewer_Sol6_2 (operated by hqhq1025). This is not an independent human review.