fix(agent-core): roll the todo store back on undo - #3072
Conversation
Context.undo now rolls the agent tool store (todos) back to match the spliced history: ToolManager keeps checkpoint snapshots taken when each real user input enters history (the pushHistory funnel, shared by live and replay paths) and reseeded at compaction/clear; undo restores through updateStore so the live path appends compensating tools.update_store records while replay restore stays record-suppressed. The v1 engine also serves agent.getTodos now (AgentAPI -> rpcMethods -> SessionAPIImpl -> KimiCore -> SDK base client), replacing the NOT_IMPLEMENTED throw, so the TUI's post-undo todo refresh works on the legacy engine. Folding v2-session wires through a v1 agent resumes with the rolled-back store as well.
🦋 Changeset detectedLatest commit: 8bf4033 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 |
![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: 17e91844c1
ℹ️ 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".
| const pops = Math.min(removedUserTurns, this.storeCheckpoints.length - 1); | ||
| if (pops <= 0) return; | ||
| const target = this.storeCheckpoints[this.storeCheckpoints.length - pops]!; |
There was a problem hiding this comment.
When a session has more than 99 undo anchors since the last clear/compaction, callers can still request a larger /undo count because availability is based on context history, not this checkpoint limit. In that case pops is capped to the retained window and target becomes the oldest retained checkpoint, so undoing 100+ prompts restores todos from a turn that was just removed and then persists that wrong state via the compensating tools.update_store; please either retain/precheck the full undo depth or restore a safe baseline when the requested undo exceeds the retained checkpoints.
Useful? React with 👍 / 👎.
…ered Review feedback: with checkpoints trimmed to a fixed window, an undo deeper than the window restored the store snapshot of a turn the undo just removed and persisted it via the compensating record. The stack is 1:1 with live history anchors (pushes on anchor appends, pops on undo, reseeded on clear/compaction), so it is already bounded by history and needs no cap; add a regression test with distinct per-anchor values.
|
Addressed in 8bf4033: dropped the checkpoint cap entirely. The stack is 1:1 with live history anchors (pushed on anchor appends via |
1 participant
Related Issue
Resolve #3058
Problem
See linked issue. In short:
/undoremoved the conversation turns but left the todo list at the post-undo state — on the legacy v1 engine the store was never rolled back and the TUI's post-undo panel refresh silently no-op'd (getTodosthrewNOT_IMPLEMENTED), and v2 sessions resumed through the SDK's v1 wire fold hydrated the todo panel with the undone (newest) todos.What changed
ToolManagerkeeps checkpoint snapshots of the tool store, taken when each real user input (undo anchor) enters history via thepushHistoryfunnel — the single path shared by live appends, replay restore, and deferred flushes — and reseeded as a baseline at compaction and/clear(store contents survive, matching today's behavior and v2).Context.undorolls the store back by the number of anchors it actually removed, before anyundo_limitthrow.updateStore, so the live path appends compensatingtools.update_storerecords while replay restore stays record-suppressed — the append-only wire, the transcript's last-write-wins fold, and resume all stay self-consistent without truncation. Pre-fix wires self-heal on the next resume.agent.getTodos(AgentAPI →rpcMethods→SessionAPIImpl→KimiCore→ SDK base client), replacing theNOT_IMPLEMENTEDthrow; the v2 client override is untouched. The TUI's post-undo todo refresh works unchanged..undoable()checkpoint semantics with the same anchor predicate (isRealUserInput≡ v2'sisUndoAnchor), keeping engine behavior aligned.Tests (all in existing files): undo rolls the store back + resume roundtrip, compensating record on the wire, todos first written in the undone turn cleared, compaction-baseline restore on partial undo, background notifications skipped,
/clearreseeding; legacy and compensating wire shapes replayed on resume; SDK-levelgetTodosafterundoHistory. Full suites green: agent-core 4192 passed, node-sdk 388 passed.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.