feat(agent-core-v2): persist the terminal turn.ended wire record - #2457
Conversation
- add a persisted turn.ended op (turnId, reason, error, durationMs) dispatched from the loop's runTurn finally block, alongside the event - fold the record back in the transcript cold rebuild: terminal state (blocked folded into failed, mirroring the live wire edge), durationMs, error message and endedAt; journals without the record keep the grouping default - restrict the test harness's snapshot waiters to emit entries so the same-named wire record no longer shadows the turn.ended event
The checker names a unique symbol key __@name@NNNN, where NNNN is a compilation-global counter that shifts with unrelated type additions and churns the generated manifest. Render the stable __@name form instead.
|
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: d1d7bd9cf5
ℹ️ 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".
| endedTurns.size > 0 | ||
| ? base.items.map((item) => { | ||
| if (item.kind !== 'turn') return item; | ||
| const record = endedTurns.get(item.ordinal); |
There was a problem hiding this comment.
When a hidden promptless turn exists, this ordinal lookup can apply the wrong terminal state to a later visible turn. I checked RetryStepRequest: it opens a real newTurn with origin retry but contributes no context messages, and groupTurns skips retry-origin messages, so after a retry turn followed by the next normal user turn the base item with ordinal 1 is the later user turn while endedTurns.get(1) is the retry's turn.ended; after restart the user's turn can show the retry's failed/cancelled state, duration, or error. The cold fold needs a mapping from persisted engine turn ids (or must skip hidden ids) rather than assuming display ordinals always match.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — confirmed: RetryStepRequest opens a real newTurn with origin 'retry' and contributes no context messages, so engine turn ids drift from the grouping ordinals. Fixed in 5d5861c: the fold now replays the loop's turn-clock records (turn.prompt / turn.cancel) to find hidden turns (retry turns and queued-then-cancelled reservations), maps engine ids to ordinals past the hidden ids, and drops the hidden turns' own end records. Added regression tests for both drift shapes.
RetryStepRequest opens a real engine turn with origin 'retry' but contributes no context messages, and a queued-then-cancelled reservation consumes an engine id without starting. Both make engine turn ids drift from the grouping ordinals, so matching turn.ended by ordinal could stamp a later visible turn with the wrong terminal state. Replay the loop's turn-clock records (turn.prompt / turn.cancel) and map engine ids to ordinals past the hidden ids; the hidden turns' own end records map nowhere and are dropped.
…shot, bash auth hardening, fs_search tool, user tool RPC - SessionManager: ensure_loaded with is_valid_shape cold-miss rebuild (TS read-model bug parity); turn.ended terminal records (agent-core-v2 MoonshotAI#2457) - TaskService: notification bookkeeping snapshot/restore for /undo (MoonshotAI#2055) - callbacks: bash_native_authorize — dangerous commands need explicit user allow rule; session/auto/yolo approval no longer blanket-approves - NativeToolset: fs_search tool (root validation, missing-root error) - main: register/unregister tool RPCs (SDK registerTool parity)
1 participant
Related Issue
No linked issue — the problem is explained below.
Problem
Turn observability gaps in the v2 engine and the transcript history rebuild:
turn.ended(with areason) on the event bus but never persists it. After a restart, the transcript cold rebuild (wire.jsonl→ snapshot) hardcodes every historical turn's state tocompleted— cancelled and failed turns are indistinguishable in history, and the turn'sdurationMs/ terminal error are lost.state-manifest.d.tsrenders aunique symbolproperty key with the checker's compilation-global id (__@mediaStripSnapshotBrand@2376). That id shifts with any unrelated type addition, so the manifest churns and produces noisy diffs.What changed
1. Persist
turn.endedand rebuild it on the cold pathWhat was done:
turn.ended({turnId, reason, error?, durationMs?}), dispatched fromrunTurn'sfinallyblock right before the event is published — the same pattern asturn.prompt+turn.started. Older readers tolerate the unknown record type (skip + warn), so no wire protocol bump is needed, and journals without the record keep the grouping default (completed).foldWireRecordFacts) folds the record back into the matching turn item: terminal state (blockedfolded intofailed, mirroring the live wire edge),durationMs, error message, andendedAt(record time). Engine turn ids are mapped to grouping ordinals through a replay of the loop's turn-clock records (turn.prompt/turn.cancel), so hidden turns — retry turns (RetryStepRequest: a realnewTurnwith no context messages) and queued-then-cancelled reservations — cannot shift a later turn's terminal state onto the wrong item.until/take) now resolve on emit entries only, so the same-named wire record no longer shadows theturn.endedevent in event-stream snapshots.durationMs/time, and the transcript fold gains last-wins,blocked→failed, malformed/unknown-id tolerance, and hidden-turn drift (retry / cancelled-queued) cases.2. Stabilize unique-symbol keys in the state manifest
What was done:
gen-state-manifestrenders__@name@NNNNunique-symbol property keys as the stable__@nameform, so the generated manifest no longer churns on unrelated type additions.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.