Conversation
github-actions
Bot
added
scope: cli
scope: shared
scope: components
scope: agent-runtime
labels
7f26003 to
20d3534
Compare
github-actions
Bot
added
the
status:needs-pr-attention
20d3534 to
3683272
Compare
github-actions
Bot
removed
scope: components
status:needs-pr-attention
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3345e722a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const autonomousIndex = this.pushAssistantEntry(`assistant:autonomous-${turnId}`); | ||
| const autonomous = this.history[autonomousIndex]; | ||
| if (autonomous) { | ||
| autonomous.acpTurnId = turnId; |
There was a problem hiding this comment.
A finished autonomous entry now carries acpTurnId: "auto:…", but the existing renderer enables Fork for any finished entry with an acpTurnId, SessionForkService forwards that value as forkSessionTurnId, and edit-and-resend likewise selects the nearest preceding assistant's acpTurnId. After any cron/task turn, forking that entry—or editing the next user turn—therefore sends a deliberately non-fork-position ID to Kimi and fails. Suppress autonomous entries in both selection paths, or otherwise reject them before dispatch.
AGENTS.md reference: apps/cli/src/session/AGENTS.md:L97-L103
Useful? React with 👍 / 👎.
| ); | ||
| // A dying ACP process ends any engine-opened turn it hosted: drop the | ||
| // activity marker so busy status and the idle-GC guard release the session. | ||
| this.onSessionTerminated((sessionId) => this.handler?.clearEngineTurnActivity(sessionId)); |
There was a problem hiding this comment.
When an engine turn's ACP process crashes before emitting turnEnded, this hook does not observe that process exit: onSessionTerminated is driven by the Session's terminated event from explicit Session.terminate(), while the unexpected ACP-child exit callback in session.ts only nulls agentProcess. The engineTurn marker consequently remains set indefinitely, so live status stays running and hasActiveTurn prevents idle GC until somebody explicitly terminates the whole Session. Clear the marker from the actual ACP process-exit path.
Useful? React with 👍 / 👎.
c3345e7 to
6ae0425
Compare
|
Thanks — valid catch. Autonomous entries ( The fix landed in the stacked follow-up #654 (commit d27b19e) rather than this PR to keep #653 under the community 1000-line gate; behavior and tests are identical to having it here. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ae0425086
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -1 +1 @@ | |||
| Subproject commit d3f218c27db5eb338e08698dd2fccb5de1f30cf6 | |||
| Subproject commit 1c5acbc6a4aad173472faec46b006a3bc1fb4684 | |||
There was a problem hiding this comment.
When this commit is released as-is, built-in Kimi still downloads the artifact pinned by apps/cli/src/agent/kimi-runtime-manifest.json, whose sourceCommit remains f255222..., rather than the newly referenced 1c5acbc... submodule code that emits turnId, turnOrigin, and turnEnded. Since managed-agent-runtime.ts constructs the production Kimi runtime exclusively from that manifest, installed users continue running the unstamped adapter and autonomous output is still merged into the previous turn or dropped after restart; package the new submodule revision and update the manifest version/checksum before shipping.
AGENTS.md reference: AGENTS.md:L44-L46
Useful? React with 👍 / 👎.
|
Correct on the facts, and this sequencing is deliberate (also called out in this PR's Summary and in the linked decision note):
Follow-ups required after #9 merges, before this PR's behavior reaches users: (1) build + publish the new managed-runtime artifact, (2) bump |
…y entries An agent engine can open turns of its own behind the client's back (a kimi cron fire, a task wake). Their ACP updates carried no turn identity, so the history layer appended them to the last client turn's assistant entry: the real reply was folded away and the interrupting turn's text became the visible answer, and after a daemon restart such updates were dropped outright. - history-apply: route turn-stamped updates by ownership — scoped to kimi-stamped turns only (auto: id or turnOrigin marker): adopt only a delta-accepting entry or create assistant:autonomous-<turnId>. Every other id keeps the exact legacy last-wins restamping (claude per-message uuids, codex collab child turns are boundaries inside one client turn). turnEnded finalizes the owning entry once; acpTurnOrigin persists. - session-transient-store: route marked updates to a synthesized autonomous target even while a client turn owns the session; maintain an engine-turn activity marker (set on first marked update, cleared on end marker or ACP process termination) feeding hasActiveTurn (idle-GC guard) and live status unknown -> running. - Bump the kimi submodule for the matching stamping (turnId auto:<n> + turnOrigin + turnEnded on engine-opened turns). Model: kimi-code/kimi-for-coding
Model: kimi-code/kimi-for-coding
6ae0425 to
4567ff9
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4567ff9ce6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (isAutonomousTurnId(turnId) || readLodyTurnOrigin(update) !== undefined) { | ||
| const entryIndex = this.ensureEntryForAcpTurn(turnId); |
There was a problem hiding this comment.
While an autonomous cron/task turn is running, this branch creates an unfinished assistant entry, so resolveActiveAssistantTurnId returns its assistant:autonomous-auto:… entry ID and the Stop/interrupt controls pass that ID to requestSessionCancel. However, SessionExecutionService.cancelSession only interrupts IDs owned by getActiveTurnId, currentTurnBySession, or turnRuntimeBySession; engine-opened turns have none of those, so the request follows the stale-turn path, reports success, and never calls the agent client while the work continues. Add an engine-turn cancellation path or prevent these entries from being exposed as cancellable client turns.
Useful? React with 👍 / 👎.
1 participant
Related issue
Closes #640
Problem / pressure
The Kimi engine opens turns behind the client's back — a cron fire, a task wake — and its ACP server forwards their content by design. Those updates carried no turn identity, so the history layer routed them into the last client turn's assistant entry (the never-expiring finalized-turn target built for stragglers), or dropped them when no client turn existed in memory. Observed in production sessions: the user turn's real reply folded into a collapsed "thinking process" while the cron turn's text became the visible answer; a session showing "completed" (
executionState: idle) while an engine turn ran for 15+ minutes; the 20-min idle GC SIGKILLing the agent process mid-cron-turn; engine-turn output dropped after a daemon restart.Summary
_meta.lody.turnId = auto:<n>(non-numeric, can never parse into asession/forkposition) +_meta.lody.turnOrigin; a_meta.lody.turnEndedmarker is emitted when such a turn ends.@lody/sharedhistory-apply): onlyauto:/turnOrigin-marked updates get turn-aware routing — owner entry → adopt only a delta-accepting entry → else createassistant:autonomous-<turnId>. Every other stamped id keeps the exact legacy last-wins restamping (claude per-message boundary uuids and codex collab child turns are boundaries inside one client turn).turnEndedfinalizes the owning entry once.acpTurnOriginis persisted on the entry.apps/cli): marked updates always take a synthesized autonomous target — never the active/finalized client turn's — covering the no-target drop and rich content. An engine-turn activity marker (set on first marked update, cleared on end marker or ACP process termination — process-bounded, no wall clock) feedshasActiveTurn(idle-GC guard) and upgrades live statusunknown→running.Visual explanation
flowchart LR subgraph kimi [Kimi engine + ACP server] UT[user turn 40] --> T41[cron turn 41, origin=cron_job] T41 --> S["_meta.lody: turnId=auto:41, turnOrigin, turnEnded"] end S --> R{apps/cli enqueueACPUpdate} R -->|marked| AT[autonomous target<br/>assistant:autonomous-auto:41] R -->|unmarked| LT[active/finalized client target] AT --> HA{history-apply} LT --> HA HA -->|auto:/turnOrigin only| E1[own entry: adopt-if-accepting<br/>or create autonomous entry] HA -->|any other id| E2[legacy last-wins restamping<br/>claude uuids, codex collab] S --> M[engineTurn activity marker<br/>transient store] M -->|set| BUSY[live status running + GC guard] M -->|turnEnded / process exit| IDLE[clear] E1 --> UI[divider: Scheduled task<br/>+ own finished turn]Before / after
assistant:autonomous-<turnId>; content survivesturnEndedmarker finalizes it onceexecutionState: idlewhile the engine turn runs; idle GC kills the agent mid-turnrunning; GC guard sees the engine turn; marker clears on end/process exitTest plan
packages/shared: full suite 1133 passing, incl. new cases — engine-turn separation, cross-batch continuity, died-before-output adoption hole, end-marker finalization (once-only timing), claude multi-uuid last-wins, codex collab inline restamping.apps/cli: full suite 2715 passing (3 failures reproduce on unmodifiedorigin/mainin this environment: two Claude credential-store probes, one macOS/varvs/private/varworktree-GC assertion) — new store cases for autonomous-target gating and engine-turn activity note/clear/replace.packages/components: 16 passing, incl. divider emission on both incremental and cache-reuse paths.acp-server(submodule): 163 passing (1 pre-existing env e2e failure on base).check-i18n,pnpm run docs checkclean.runningduring the engine turn,idleafter; provider-500-empty-turn + adjacent cron fire no longer merges.Context handoff
Instructions for reviewing agents
packages/shared/src/acp/history-apply.ts(the three-way turnId branch inapply()+ensureEntryForAcpTurnadoption condition +finalizeEntryOnce),apps/cli/src/lib/session-transient-store.ts(autonomousACPUpdateTargetFromgating, engine-turn marker lifecycle),apps/cli/src/lib/message-handler.ts(enqueueACPUpdatetarget precedence), kimiacp-server/src/session.ts(stamp + end-marker emission).auto:/turnOriginmarkers only (legacy path kept for every other stamped id); adoption allowed only on delta-accepting entries; engine-turn liveness bounded by ACP process termination rather than a wall clock;auto:<n>as the non-fork id format.Authoring context
acpTurnOrigin(additive Loro schema change); deterministic entry idsassistant:autonomous-<turnId>; non-numericauto:ids can never resolve intosession/forkpositions; legacy last-wins restamping preserved byte-for-byte for all unmarked ids.Original user prompt
Show original prompt