fix(acp): stream agent-initiated turns - #2392
Conversation
🦋 Changeset detectedLatest commit: 0d7614c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@codex review |
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d7614cba8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| params.mode === 'resume' && | ||
| typeof this.harness.onSessionEvent === 'function' |
There was a problem hiding this comment.
Buffer cold-load events before replay
When session/load cold-loads a persisted session, autonomous work can begin inside harness.resumeSession()—for example, restored task reconciliation can launch a notification turn, and the lifecycle now starts the cron scheduler before resume returns. Because this temporary subscription is restricted to mode === 'resume', those live deltas and tool events occur before AcpSession installs its listener; replayHistory() cannot recover these transient events, so the client can miss part or all of an agent-initiated turn during load. The load path needs an atomic snapshot/live handoff rather than omitting the event bridge.
Useful? React with 👍 / 👎.
Related Issue
Resolves #2163
Problem
ACP update projection was scoped to an active
session/promptrequest. After that request settled, a task- or cron-triggered main-agent turn could run successfully in the engine while the connected ACP client received nosession/updatenotifications.There were two related lifecycle gaps that made the same path unsafe to attach: v2 could start restored cron work before SDK event wiring existed, and a
UserPromptSubmithook that blocked or failed before a turn was created had no correlated terminal event, leaving ACP prompt admission occupied.What changed
promptId/activationId, so autonomous turns cannot complete an unrelated prompt.prompt.completedwhen a pre-turn hook blocks or fails, allowing ACP to returnrefusaland admit the next request.session/resumeis materializing. Setup is serialized per canonical session id and every temporary registration has ownership-aware failure/dispose cleanup.@moonshot-ai/kimi-code.Scope: this guarantees live delivery for connected sessions and the cold
session/resumehandoff. It does not claim an atomic history/live cut for coldsession/load; that requires a producer-side snapshot cursor rather than replaying a second consumer buffer.Verification
The three real-engine E2E files were copied byte-for-byte to an untouched
upstream/mainworktree at691ec467and run on both trees:refusal, and the following prompt completed.Additional validation:
pnpm test: 1,017 test files passed; 16,743 passed, 3 expected failures, 83 skipped, 2 todo.pnpm typecheck,pnpm lint(0 errors), andpnpm buildpassed across the monorepo.pnpm sherif,pnpm lint:pkg, SDK API Extractor, and package declaration builds passed.An existing MCP initial-connect parity test intermittently observed v1 as
pendingwhile v2 had reachedconnected; the identical failure reproduced on untouched upstream, the affected assertion is outside this diff, and final SDK/root runs passed.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update. No command, configuration, or documented UI contract changed.