feat(vscode): conversation undo with exclusive, broadcast-pushed transcript refresh#1839
feat(vscode): conversation undo with exclusive, broadcast-pushed transcript refresh#1839wbxl2000 wants to merge 7 commits into
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: a2daa463f3
ℹ️ 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".
|
|
||
| // Return a fresh replay so the webview can re-render the truncated | ||
| // conversation, mirroring how LoadKimiSessionHistory re-hydrates views. | ||
| const resumeState = runtime.session.getResumeState(); |
There was a problem hiding this comment.
After runtime.undoHistory(count) mutates the engine, Session.getResumeState() still returns the Node SDK's cached state: Session.undoHistory() only issues the RPC and does not update that cache. For a newly created session the cache is absent, so Undo succeeds in the engine and then returns Session history is unavailable; for a resumed session it replays the pre-undo conversation. Reload the session or obtain fresh context before constructing the returned events, otherwise the webview remains inconsistent with the model context.
Useful? React with 👍 / 👎.
| // Model and thinking effort are applied only when the session is created | ||
| // (see openSession). An existing session keeps its own — the global config | ||
| // values are defaults for new sessions, matching CLI/TUI resume semantics. | ||
| // Changes made in the pickers reach the active session through the | ||
| // SaveConfig handler instead. |
There was a problem hiding this comment.
When a user selects a different model and immediately sends, the picker starts bridge.saveConfig() without awaiting it, while StreamChat is dispatched independently with the newly selected model. Removing the model synchronization here lets openSession() return the existing session before SaveConfig reaches session.setModel(), so the prompt can run on the previous model despite the composer showing the new one. Preserve resumed models on attach, but ensure the model submitted with a prompt is applied or serialized with the pending save before that turn begins.
Useful? React with 👍 / 👎.
…del before the turn
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0d06697c9
ℹ️ 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.ensureOpen(); | ||
| if (this.isBusy) { | ||
| throw new Error("Wait for the current response to finish before undoing."); | ||
| } | ||
| await this.session.undoHistory(count); |
There was a problem hiding this comment.
When the user double-clicks the new undo button, or sends a prompt before the undo RPC and replay finish, every request can pass this isBusy check because undo itself never marks the runtime as busy or exclusive. Two undoHistory(1) calls can therefore remove two turns instead of one, while a concurrent prompt can race reloadSession() and leave the UI out of sync. Treat undo plus history refresh as one exclusive session operation and disable further actions while it is pending.
Useful? React with 👍 / 👎.
| if (resumeState?.agents["main"] === undefined) { | ||
| throw new Error("Session history is unavailable."); | ||
| } | ||
| return { events: replaySessionToWebviewEvents(resumeState, runtime.id) }; |
There was a problem hiding this comment.
When the same session is open in both the sidebar and a tab, this replay is returned only to the webview that invoked the RPC. The runtime otherwise broadcasts session events to all subscribers, but undo emits no equivalent update, so the other view continues showing the removed turn even though subsequent prompts use the truncated engine history. Broadcast or otherwise reload the replay for every subscribed webview.
Useful? React with 👍 / 👎.
| // mutates the engine but does not update the cache, which would otherwise | ||
| // replay the pre-undo conversation (resumed sessions) or be absent | ||
| // entirely (sessions created in this process). | ||
| await runtime.session.reloadSession(); |
There was a problem hiding this comment.
If reloadSession() fails after undoHistory() has already succeeded—for example because provider or plugin configuration became invalid—the RPC reports an error and the webview keeps its old transcript, although the engine history has permanently been truncated. The next prompt then runs against context different from what the user sees. Handle this partial-success path by refreshing/resetting the UI or returning an explicit successful undo result with recovery state.
Useful? React with 👍 / 👎.
…grade reload failures
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6a843cb6e
ℹ️ 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 events = await bridge.loadSessionHistory(changedSessionId); | ||
| await store.loadSession(changedSessionId, events); |
There was a problem hiding this comment.
If the user switches conversations while loadSessionHistory is awaiting replay or file-baseline work, this captured store still calls loadSession(changedSessionId, events) and switches the UI back to the session they just left. Re-read useChatStore.getState().sessionId after the await and discard the result unless it still matches changedSessionId.
Useful? React with 👍 / 👎.
| try { | ||
| await this.session.reloadSession(); | ||
| } catch (error) { | ||
| this.log("Failed to reload the session after undo; views rehydrate from persisted records", error); | ||
| } |
There was a problem hiding this comment.
When Session.reloadSession() fails after the undo has landed, swallowing the error and broadcasting a history refresh does not actually recover from persisted records: LoadKimiSessionHistory reuses this attached runtime and reads session.getResumeState(), while the SDK only updates that cached state after a successful reload. The webview can therefore replay the pre-undo transcript and still show the success toast; allow the failure to propagate or explicitly replace/reload the runtime before broadcasting.
Useful? React with 👍 / 👎.
…tive session after loading
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cab4e6ee01
ℹ️ 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 (useChatStore.getState().sessionId !== changedSessionId) return; | ||
| void (async () => { | ||
| try { | ||
| const events = await bridge.loadSessionHistory(changedSessionId, true); |
There was a problem hiding this comment.
When the same conversation is open in multiple views (for example, the sidebar and a tab), the runtime broadcasts ConversationHistoryChanged to every subscriber and each webview executes this reload: true request concurrently. The inspected KimiCore.reloadSession() implementation in packages/agent-core/src/rpc/core-impl.ts:546-568 closes, deletes, and resumes the active runtime without serialization, so overlapping calls can race while closing the same session or instantiate duplicate runtimes for the same ID, producing failed reloads or leaked background resources. Reload the SDK session once on the extension side, then let every view replay the already-refreshed history without issuing another reload.
Useful? React with 👍 / 👎.
…om the shared refreshed state
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87c1aa17d1
ℹ️ 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 events = await bridge.loadSessionHistory(changedSessionId); | ||
| // The user may have switched conversations while the history was | ||
| // loading — never yank the UI back to a session they have left. | ||
| const store = useChatStore.getState(); | ||
| if (store.sessionId !== changedSessionId) return; |
There was a problem hiding this comment.
If the user switches conversations while this undo-triggered history request is pending, the post-load check only prevents the webview store from returning to the old session. LoadKimiSessionHistory has already called ctx.resumeSession() in apps/vscode/src/handlers/session.handler.ts:130, which attaches the host view and its file manager to changedSessionId; if this stale request finishes after the newer session load, the UI continues showing the new session while ctx.getSession() points to the undone one. Subsequent session-less operations such as Undo, SaveConfig, or Abort can therefore modify the wrong conversation, so the host attachment also needs request-generation/current-session protection.
Useful? React with 👍 / 👎.
| await this.session.undoHistory(count); | ||
| await this.session.reloadSession(); | ||
| for (const webviewId of this.webviewIds) { | ||
| this.broadcast(Events.ConversationHistoryChanged, { sessionId: this.id }, webviewId); |
There was a problem hiding this comment.
When reloadSession() fails after session.undoHistory(count) has succeeded—for example while refreshing provider or plugin configuration—the RPC rejects and no ConversationHistoryChanged event is broadcast even though the conversation was already truncated. The webview consequently keeps the pre-undo transcript and reports “Failed to undo”; retrying then removes an additional turn. This partial-success path should notify the UI that the undo landed (or return a distinct refresh error) rather than presenting the whole operation as uncommitted.
Useful? React with 👍 / 👎.
…ad of rehydrating views
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26d2dfabbd
ℹ️ 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".
| ({ sessionId: changedSessionId, events }: { sessionId: string; events: UIStreamEvent[] }) => { | ||
| const store = useChatStore.getState(); | ||
| if (store.sessionId !== changedSessionId) return; | ||
| void store.loadSession(changedSessionId, events).catch((error: unknown) => { |
There was a problem hiding this comment.
When an idle conversation has unsent media attached and the user clicks Undo, this calls the generic loadSession path, which resets draftMedia (as well as the pending input and queue) before replaying history. Undoing a completed turn should not discard unrelated composer state; apply the replay through a transcript-only update or explicitly preserve the current draft.
Useful? React with 👍 / 👎.
| // The runtime broadcasts ConversationHistoryChanged to every subscribed | ||
| // view on success; each view rehydrates via LoadKimiSessionHistory. | ||
| await runtime.undoHistory(count); |
There was a problem hiding this comment.
When the undone turn was the one that injected editor context—such as the latest turn after switching to file B in onFileChange mode—injectedEditorContextSessions still records B after this call. A subsequent prompt from B therefore skips injection even though undo removed that context from the engine history, leaving the agent without editor context until another file is selected; the injection bookkeeping needs to be restored consistently with the removed turn.
Useful? React with 👍 / 👎.
wbxl2000
changed the title
1 participant
Related Issue
No issue — this is a feature parity addition for the VS Code extension. Scope note: the underlying bug fixes (reliable cancel, preserved session model on attach) were split into #1845 to land promptly; this PR is the remaining
feat: conversation undo.Problem
The CLI has
/undo(double-Esc) to drop the last user turns and rewind the conversation; the VS Code extension exposed fork and file-level undo, but no way to rewind the conversation itself, even though the SDK already supportssession.undoHistory(count). Users reported "还无法回退" as a top pain point.What changed
undoChatbridge method +SessionRuntime.undoHistory(count): guarded to idle sessions, matchingsession.undoHistory(count)semantics (last N user turns; engine reports compaction-boundary/nothing-to-undo errors, surfaced via toast).reloadSession+ one replay computation. Double-clicking the button or sending a prompt mid-undo is rejected instead of removing extra turns or mixing states.ConversationHistoryChangedbroadcast payload — webviews apply it locally with no engine round trip, so a view can never be re-attached to a stale session or race another view's reload. A failed post-undo reload is worded distinctly (Undo applied, but refreshing the conversation failed: ... Use Reset Kimi to reconcile.) so it can never be mistaken for "nothing was undone".Tests: undo exclusivity (second undo rejected while the first is pending), busy-turn rejection, transcript push in the broadcast payload, distinct reload-failure wording + guard release, boundary validation for
undoChatparams, and end-to-end integration tests that prompt a real session (fresh and resumed) and verify the engine truncation and the pushed transcript.Checklist
gen-changesetsskill, or this PR needs no changeset. (No changeset: vscode-extension-only change, nothing enters the CLI bundle.)gen-docsskill, or this PR needs no doc update. (CLI-parity UX; no user-facing docs affected.)