fix(agent-core-v2): normalize interrupted turns on restore and drop retired-generation events - #3224
Conversation
|
commit: |
![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: 21b15925a9
ℹ️ 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".
| } | ||
| this.executeEvent(event, true); | ||
| if (event instanceof TurnPrompt) { | ||
| const turnId = nextTurnId++; |
There was a problem hiding this comment.
When a queued turn is cancelled, TurnCancel causes the turn fold to skip that reserved ID, so a later TurnPrompt may correspond to a higher ID than the number of prompts seen. For example, after turn 0 completes, queued turn 1 is cancelled, and turn 2 starts before a crash, this local counter assigns the restored prompt to turn 1; restore then appends turn.ended for the cancelled turn and leaves the actual turn 2 open. Derive the prompt's ID from the replayed turn state, which already incorporates TurnCancel, rather than incrementing a separate counter.
Useful? React with 👍 / 👎.
ee45ad3 to
97f603a
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97f603a514
ℹ️ 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".
| reason: 'interrupted', | ||
| interruptReason: 'aborted', | ||
| }); | ||
| this.executeEvent(event, true); |
There was a problem hiding this comment.
When an agent ID is recreated within an already-live session, bindSessionTranscript subscribes the new generation during onDidCreate before restore() runs, but this silent: true call suppresses the observable turn.ended event. The live transcript is backfilled only on its initial attachment, so it retains the retired generation's running turn even though the journal and activity state now mark it interrupted; subsequent transcript reads can therefore show the old turn as running alongside new turns. Emit/project this synthetic ending to already-bound consumers, or explicitly refresh that agent's transcript after restoration.
Useful? React with 👍 / 👎.
97f603a to
e0652ce
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0652ce711
ℹ️ 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".
| }) satisfies z.ZodType<McpOAuthAuthorizationUrlUpdateData>; | ||
|
|
||
| export const turnEndReasonSchema = z.enum(['completed', 'cancelled', 'failed', 'blocked']) satisfies z.ZodType<TurnEndReason>; | ||
| export const turnEndReasonSchema = z.enum(['completed', 'cancelled', 'failed', 'blocked', 'interrupted']) satisfies z.ZodType<TurnEndReason>; |
There was a problem hiding this comment.
When a process dies mid-turn, restore now emits reason: 'interrupted' as a raw turn.ended event over /api/v1/ws, but packages/protocol/src/events.ts still restricts both TurnEndReason and turnEndReasonSchema to the four old values. Consequently, clients validating session events with the exported sessionEventMessageSchema reject exactly this recovery event instead of applying it. Update the shared protocol type/schema as well, or translate the reason before broadcasting on the v1 surface.
Useful? React with 👍 / 👎.
| time?: number; | ||
| turnId: number; | ||
| reason: 'completed' | 'cancelled' | 'failed' | 'blocked'; | ||
| reason: 'completed' | 'cancelled' | 'failed' | 'blocked' | 'interrupted'; |
There was a problem hiding this comment.
When an agent ID is recreated while its session transcript store remains live, the store retains the retired generation's running prompt entity and the new projector receives this synthetic interrupted ending. The fresh code now publishes the ending, but this branch only upserts the failed turn and idle activity; neither restore nor the projector emits a terminal prompt.upsert, so consumers show an idle agent with a prompt still running indefinitely. Settle the prompt associated with the interrupted turn during restore or projection.
Useful? React with 👍 / 👎.
e0652ce to
c1d4138
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1d4138164
ℹ️ 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".
| reason: 'interrupted', | ||
| interruptReason: 'aborted', |
There was a problem hiding this comment.
This restore fix changes user-visible CLI behavior by closing turns that previously remained stuck after a restart, but the commit contains no .changeset/ entry. Add a patch changeset for @moonshot-ai/kimi-code; otherwise this user-relevant recovery fix will be omitted from the generated release changelog.
AGENTS.md reference: AGENTS.md:L85-L86
Useful? React with 👍 / 👎.
| reason: 'interrupted', | ||
| interruptReason: 'aborted', |
There was a problem hiding this comment.
When the process exits after persisting an active turn.cancel with reason: 'user_cancelled' but before the normal turn.ended is written, the turn remains in openTurns and this unconditional ending closes it as interrupted/aborted. Downstream session and transcript projections map that reason to failed, so a deliberate cancellation is reported as a failure after restart even though the journal retained the cancellation reason; track active cancellation records and synthesize cancelled/user_cancelled for this case.
Useful? React with 👍 / 👎.
c1d4138 to
ba5d0c1
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba5d0c1754
ℹ️ 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".
| } | ||
| this.executeEvent(event, true); | ||
| if (event instanceof TurnPrompt) { | ||
| openTurns.add(turnClock.nextTurnId); |
There was a problem hiding this comment.
For legacy journals without durable turn.ended records—the existing multi-turn resume fixtures model exactly this shape—every TurnPrompt is added here and remains in openTurns even after a later turn starts. Restoring a completed two-turn session therefore appends synthetic interrupted endings for both turns, persists false failures into the journal, and changes the session's last-turn outcome; because turn execution is serial, later turn evidence must retire earlier inferred-open turns so only the genuinely trailing turn is normalized.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L22-L22
Useful? React with 👍 / 👎.
| this.markerOp('interruption', { turnId: event.turnId, reason: event.interruptReason }), | ||
| ); | ||
| } | ||
| if (event.reason === 'interrupted') { |
There was a problem hiding this comment.
When the journal ends after an active turn.cancel was persisted but before the normal turn.ended and prompt.aborted, restore now synthesizes a cancelled ending rather than an interrupted one. In an already-live session transcript, this condition excludes that recovery event, leaving the retired generation's running or queued prompt entities nonterminal even though the turn and agent activity are now terminal; the recovery-cancelled path must also emit appropriate terminal prompt operations.
Useful? React with 👍 / 👎.
ba5d0c1 to
8654a10
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8654a1093d
ℹ️ 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".
| for (const [promptId, prev] of this.prompts) { | ||
| if (prev.status !== 'running' && prev.status !== 'queued') continue; |
There was a problem hiding this comment.
When an active prompt is cancelled while another prompt remains queued, AgentPromptService.settle() starts that queued prompt next, but this loop marks both the running prompt and every queued prompt as aborted. Starting the queued prompt emits no event that restores its transcript status to running, so clients display a prompt that is actively executing as aborted until another terminal event arrives. Only the active prompt should be settled for an ordinary cancelled turn; mass-settling queued prompts should be limited to recovery paths where the queue was actually lost.
Useful? React with 👍 / 👎.
8654a10 to
2c911eb
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c911ebae8
ℹ️ 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 latest = this.latestGeneration.get(agent.agentId) ?? 0; | ||
| const retired = this.retiredGeneration.get(agent.agentId) ?? 0; | ||
| if (agent.generation < latest || agent.generation <= retired) return; |
There was a problem hiding this comment.
When an out-of-band host creates a context with makeAgentScopeContext and omits generation, its generation defaults to 0; if that context was never activated, both missing map entries also become 0, so this condition treats the unknown context as retired and silently drops every agent event. Previously this abnormal lifecycle path threw, and the stated strict guard should still reject unknown contexts; distinguish absent generation records from a recorded retired generation instead of using 0 as both the sentinel and a valid generation.
Useful? React with 👍 / 👎.
2c911eb to
dfad585
Compare
…etired-generation events
dfad585 to
35f6a33
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35f6a33d9a
ℹ️ 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".
| agentState.contributeState(interruptionReminderKey); | ||
| this._register( | ||
| eventBus.subscribe(TurnEnded, (event) => { | ||
| if (dispatcher.restorePhase === 'restoring') return; |
There was a problem hiding this comment.
If the process crashes after persisting an active turn.cancel with reason: 'user_cancelled' but before the normal turn.ended handler appends its interruption reminder, restore() now synthesizes the cancelled ending while restorePhase is still restoring, so this guard discards the only notification that would add the reminder. The next user prompt can therefore send the model partial output from the cancelled turn without warning that it is incomplete; suppress replayed historical endings without suppressing the newly synthesized recovery ending, or append its reminder after restoration.
Useful? React with 👍 / 👎.
1 participant
Related Issue
Follow-up to #3206.
Problem
#3206 stopped the crash loop and made agent scope teardown awaitable, but two gaps remain when a session is restored with a dangling turn (journal ends mid-turn, e.g. the last record is
turn.step.startedafter a process kill):What changed
agent-core-v2):EventBusServicetracks the latest and retired generations per agentId. Current-generation events publish normally; events from a retired generation (including post-deactivation stragglers) are dropped safely; foreign/unknown/future generations still throw, preserving the strict guard on abnormal paths.interruptedterminal state:TurnEndReason/TurnEndedPayloadgain'interrupted', withinterruptReasonadded to the wire serialization.EventDispatcherService.restore()detects unclosed turns fromTurnPrompt/TurnEndedfacts and appendsturn.ended(interrupted, aborted)before the agent becomes available — without re-executing the old turn. Session outcome mapsinterruptedonto the existing session-failed contract.events-zod, and the transcript contract accept the newinterruptedreason. The v1 legacy status edge maps it tofailed(v1 protocol unchanged); transcript live projection and cold rebuild both fold it to the failed turn state; the ACP edge maps it toend_turn(ACP has no interrupted stop reason).Checklist
/approve).gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.