Skip to content

feat(cli): add ACP live session lifecycle - #4862

Open
Sun-GLiang wants to merge 9 commits into
apache:mainfrom
Sun-GLiang:codex/acp-live-session-lifecycle
Open

feat(cli): add ACP live session lifecycle#4862
Sun-GLiang wants to merge 9 commits into
apache:mainfrom
Sun-GLiang:codex/acp-live-session-lifecycle

Conversation

@Sun-GLiang

@Sun-GLiang Sun-GLiang commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

ACP clients can now prompt, stream, cancel, and close their owned Maka Sessions through Runtime Host. This implements PR4 of the latest #3132 checklist on top of merged PR3 (#4051, b67778abd). The branch has been rebased onto current main; the diff no longer includes PR3.

  • Keep durable-session ownership separate from single-flight live attachment. First prompt attaches and starts observation before turn.start; later prompts reuse the attachment. Host admission owns capacity, and close releases the slot without deleting or archiving the durable Session.
  • Once Host creation commits, return the Session ID even if optional configuration projection fails. Grant mutation ownership only when returning the ID. Handle pending attach, cancellation, close, shutdown, and subscription failure without admitting an unintended Turn.
  • Use session-channel metadata revision changes plus the shared Session catalog reader and existing option projection to send complete config_option_update notifications, including adding/removing model-dependent thinking options. Serialize asynchronous configuration projection/delivery: an attached local set publishes its committed options before its response; subscription refreshes observed during that set follow its notification. Suppress queued updates after close and finish an in-flight delivery before close returns.
  • Reuse session-channel/projector sequence validation, recovery, assistant delta folding, and terminal classification. Map text/thinking to ACP chunks, authoritative complete/abort/nonrecoverable error to end_turn, and explicit ACP cancellation to cancelled. Stop only the exact Host identity from the matching subscription root or the dispatched start result. Retain the admitted identity across observation teardown until Stop settles. If Stop delivery fails, complete the ACP prompt with cancelled, preserve the Host error as a diagnostic, and leave Host terminal state unchanged.
  • Keep one-shot initial subscription open exclusively on the reconnecting connection. Initial admission cannot silently retry on a replacement Host; recovery uses the existing reconnecting path. No client capacity counter, exported Host subscription limit, or test-only inspection API.

Refs #3132

Tool streaming, interactions, and load/resume remain in subsequent checklist PRs. Nonempty client-provided mcpServers remain explicitly unsupported; supported prompt input is ordered text and local resource links.

Verification

  • Review follow-up (3275e4aba): independently reproduced both cancellation/teardown findings before fixing them. All 52 registry tests pass, including late admission after observation closes, failed Stop before/after start via cancel and AbortSignal, and exact Stop after attachment failure. Complete CLI suite: 899 passed, 3 skipped, including real ACP/Host process tests. Full build, typecheck, lint, format, ASF headers, desktop/UI knip and diff checks passed. The full workspace test suite and Zed UI smoke were not repeated for this follow-up.

Earlier validation:

  • ACP coverage: 80 tests passed in the final CLI suite, including real ACP child-process + Runtime Host tests for streaming, cancellation, external-client configuration updates, 17 unattached Sessions, Host capacity rejection before Turn creation, and slot reuse after close.
  • Final complete CLI suite: 887 passed, 3 skipped, including a stalled-configuration-read/active-close regression.
  • npm run build, npm run typecheck, npm run lint, npm run format:check, npm run check:asf-headers, both documented desktop/UI knip checks, and git diff --check passed.
  • npm test: all workspace suites passed (Runtime Host: 1791 passed, 12 skipped). The final ACP close-race hardening was subsequently rebuilt and verified with the complete CLI suite and repository static checks.
  • Existing real-client evidence from the initial PR revision: Zed 1.18.1 completed create/prompt, streamed output, Escape cancellation, and archive-driven session/close. Its captured session/new payload includes mcpServers: []. This update revalidated the protocol with the official SDK and real Host; the Zed UI smoke was not repeated.

AI use

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

Tool(s) and scope: Codex assisted with implementation, tests, verification, and the PR description; the initial PR also included Codex-assisted real-client validation.

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

@github-actions github-actions Bot added the effort/XXL Over 2500 readable lines label Sep 5, 2026
Preserve committed session reachability, publish authoritative configuration changes through the existing session channel, and harden attachment and close races.

Generated-by: Codex
@Sun-GLiang
Sun-GLiang force-pushed the codex/acp-live-session-lifecycle branch from 9c03a65 to 1ea7f79 Compare September 7, 2026 14:33
@Sun-GLiang
Sun-GLiang marked this pull request as ready for review September 7, 2026 15:47
Cancel authoritative roots on retained attachments even when no ACP prompt is
active. Reject non-regular local resources with a nonblocking open before reads.

ACP v1 cannot retract streamed chunks: reject non-prefix text/thinking revisions
with unsupported_stream_revision, propagate recovery projection errors, and stop
the exact live prompt root. Document this limitation instead of inventing revision
message IDs. Remove unused connection state and duplicate observation handlers.

Cover external roots, FIFO admission, text/thinking clearing, recovery failures,
and subsequent prompts through the existing attachment and mapper seams.

Generated-by: Codex
The POSIX FIFO regression intentionally skips Windows. Regenerate the required skip inventory so the CI inventory check matches the test declarations.

Generated-by: Codex
The capacity scenario shares one harness deadline across 17 creates and 17 turns. Complete independent sessions concurrently before checking retained subscription admission, and give this multi-operation scenario an explicit bounded budget.

A 1-second fixture response delay reproduced the 15-second timeout before the change. With concurrent prompts the same delay and original deadline pass, including four simultaneous repetitions. Capacity rejection, no-turn-on-rejection, and close slot reuse assertions remain intact.

Generated-by: Codex

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for implementing the live ACP lifecycle. I reviewed 8e56e03c with three focused reviewer passes and independent reconciliation. Reusing the Runtime Host channel/projector and keeping execution in the Host is the right direction. Two recoverable P2 cancellation/teardown cases remain inline.

Please keep the fix in the existing prompt/attachment lifecycle: observation ending is not proof that a dispatched Turn was never admitted, and ACP cancellation completion is distinct from successful delivery of Host Stop. No extra cancellation authority or global timeout framework is needed. Repeated exact Stop is already idempotent at the Host; the documented append-only stream-revision limitation is not an additional finding.

Validation: 59 focused checks passed across the targeted runs, plus two independent current-SHA probes reproducing the inline cases. No fresh full CLI build or Zed acceptance run. AI-assisted review; primary-agent reconciliation.

简体中文

感谢实现 ACP 实时生命周期。基于 8e56e03c,三路 reviewer 后由主审独立复核。复用 Host channel/projector、执行归 Host 的方向正确,仍有两项可恢复的 P2,见行内。

修复留在现有 prompt/attachment 生命周期:观察结束不能证明已发送 Turn 未准入;ACP 取消响应与 Host Stop 成功交付也不是同一事实。不需要新增取消权威或全局超时框架。Host 已保证重复 exact Stop 幂等;已声明的 append-only 修订限制不另列问题。

59 项定向检查通过,另有两个当前 SHA 探针复现行内问题。未重新完整构建 CLI 或验收 Zed。AI 辅助评审,主审已复核。

Comment thread packages/cli/src/acp/session-registry.ts
Comment thread packages/cli/src/acp/session-registry.ts Outdated
Keep the Host-returned Turn snapshot until a dispatched start settles so subscription teardown cannot retire exact Stop prematurely. End the cancelled prompt observation when Stop delivery fails, return cancelled in either start ordering, and retain the delivery error on stderr and the teardown result.

Add regressions for late admission during disposal and failed Stop before/after start via both session/cancel and AbortSignal. Verify attachment failure also stops the admitted identity.

Validation: complete CLI suite 899 passed, 3 skipped; build, typecheck, lint, format:check, ASF headers, desktop/UI knip, and diff checks passed.

Generated-by: Codex
Regenerate the Windows test inventory from the merged sources, retaining the ACP FIFO exclusion and current main entries.

Generated-by: Codex
Wait for pending configuration setters before delivering refreshes from a first or replacement attachment. Cover both races so a delayed setter response cannot overwrite newer configuration notifications.

Generated-by: Codex

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fixes. I rechecked 1ef5b621 and confirmed that both previous P2s are resolved. I found one adjacent cancellation gap and have left it inline. It also reproduces on 8e56e03c; I missed it in the earlier review, so this is not a regression introduced by the latest fixes.

AI-assisted review: 66 focused checks passed, and seven regression checks failed against the previous registry implementation. The additional registry probe below fails on both heads. I did not repeat the Zed smoke test.

简体中文

感谢修复。我复核了 1ef5b621,之前两项 P2 均已解决。还有一个相邻的取消问题,已写在行内评论。它在 8e56e03c 上也能复现,是我上轮遗漏的边界,并非本轮修复引入的回归。

本次为 AI 辅助评审:66 项针对性检查通过,7 项回归检查在旧 registry 实现上失败;新增的 registry 探针在新旧两个 head 都失败。本轮未重跑 Zed。

}
return;
}
if (active.startSettled) return;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Retain cancellation while dispatched admission is still unknown. In a connection-loss/recovery path, the Host can admit turn.start before its response is lost. The request then rejects as dispatched/outcome-unknown, sets startSettled, and reaches this return without either an observed root or startedTurn. If the subscription recovers afterward, the original prompt has already returned cancelled and no attempt remains to stop the recovered running Turn.

The registry probe cancels a pending start, rejects it with RuntimeHostRequestInterruptedError('turn.start', 'command', 'dispatched', 'connection_lost'), and then supplies the recovered original root. On both 1ef5b621 and 8e56e03c, the result is cancelled, the recovered root is running, and Stop count is 0. This probe uses the existing attachment fixture; the reconnecting-client and Host recovery paths independently establish that a dispatched command is not automatically replayed and admission can survive a lost response.

The invariant is that settling the request does not settle an unknown admission outcome. Please keep the original Session/Turn cancellation intent in the existing attempt until Host query/subscription facts resolve it, then reuse the exact Stop path. A definitive rejection can retire it. This does not require blindly replaying start or adding another execution state machine. One regression covering unknown admission → cancellation → recovered root would protect this boundary.

简体中文

P2:已发出的 start 准入结果仍未知时,保留取消意图。 Host 可能已经准入 Turn,但 start 回执因断线丢失。请求以 dispatched/outcome-unknown 失败后,startSettled 使这里在没有 root 或 startedTurn 时提前返回。随后订阅恢复出原来的运行中 Turn,ACP prompt 却已经返回 cancelled,也没有保留负责发送 Stop 的 attempt。

复用现有 attachment 夹具的探针:取消 pending start → 抛出 dispatched 的连接中断错误 → 恢复原 root。新旧两个 head 都得到 cancelled、root 仍 running、Stop 次数为 0。这是 registry 层复现;现有 reconnecting-client 和 Host recovery 路径分别证明命令不会自动重放,以及回执丢失后准入仍可能存在。

应保持的不变量是:请求结束不代表未知的准入结果已经确定。请在现有 attempt 中保留原 Session/Turn 的取消意图,待 Host 查询或订阅事实确定结果后复用精确 Stop;明确拒绝才可释放。无需盲目重发 start,也无需增加第二套执行状态机。补一条“未知准入 → 取消 → 恢复 root”的回归即可保护这个边界。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Astro-Han Fixed in c17ca7f. Could you take another look?

A dispatched turn.start interruption now leaves admission unresolved in the existing prompt attempt. Cancellation remains latched until subscription recovery or turn.query supplies the original Turn identity, then reuses the exact Stop path. An authoritative not_found or terminal result completes cleanup; a transient query failure keeps the cancellation intent. No start replay was added.

The original regression failed before the fix with zero Stop calls and passes afterward. Added 10 cases covering cancellation before/after interruption, subscription recovery after query timeout, query outcomes, and shutdown after observation closes. Registry tests: 64/64; full CLI suite: 967 passed, 3 skipped. Local lint, formatting, build, typecheck, and desktop/UI knip checks passed. Remote CI test and all non-skipped CLI package validation checks are now green.

AI-assisted implementation and verification: Codex.

Keep the original prompt attempt until Host subscription or turn.query facts resolve a dispatched start whose response was lost. Stop the recovered exact Turn, or retire the attempt on authoritative not_found or terminal state, without replaying start.

Cover cancellation before and after interruption, subscription recovery after a query timeout, authoritative query outcomes, and shutdown after observation closes.

Generated-by: Codex
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XXL Over 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants