Skip to content

Add completion change actions#633

Merged
edelauna merged 11 commits into
Zoo-Code-Org:mainfrom
ivanarifin:feature/completion-change-actions
Jun 24, 2026
Merged

Add completion change actions#633
edelauna merged 11 commits into
Zoo-Code-Org:mainfrom
ivanarifin:feature/completion-change-actions

Conversation

@ivanarifin

@ivanarifin ivanarifin commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Related GitHub Issue

Closes: #661

Description

This PR adds a Kilo Code-inspired completion experience that helps users view and undo the changes made for the latest prompt after an attempt_completion result.

Key implementation details:

  • Adds inline See New Changes and Restore Changes buttons to the completion result row, following the Kilo Code legacy placement/pattern while preserving the existing Start New Task completion action.
  • Scopes the actions to the checkpoint created after the latest user prompt so users can inspect only the new changes from that prompt.
  • Uses existing checkpoint diff and restore behavior so Restore Changes restores files and rewinds the task state to the last prompt checkpoint.
  • Adds typed webview messages and shared checkpoint lookup logic in @roo-code/types.
  • Adds localized labels for the new actions.

Reviewers should pay attention to the checkpoint selection logic and the restore flow, since those define exactly which changes are shown and undone.

Test Procedure

Validated with:

  • git diff --check
  • pnpm --filter @roo-code/types build
  • cd src && npx vitest run core/webview/__tests__/completionCheckpoint.spec.ts core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts
  • cd webview-ui && npx vitest run src/components/chat/__tests__/ChatView.clear-approval-buttons.spec.tsx src/components/chat/__tests__/SeeNewChangesButtons.spec.tsx
  • cd webview-ui && pnpm exec tsc --noEmit
  • Commit hook ran prettier --write and repository lint successfully.

Manual behavior verified locally:

  • Completion result shows Kilo-style inline See New Changes and Restore Changes buttons when a latest-prompt checkpoint exists.
  • See New Changes opens the checkpoint diff scoped to the latest prompt.
  • Restore Changes asks for confirmation, then restores files and rewinds task state to the latest prompt checkpoint.
  • Bottom completion action remains Start New Task.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Testing: New and/or updated tests have been added to cover my changes (if applicable).
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Not included. This is a small chat-row UI addition modeled after the existing Kilo Code legacy completion action pattern.

Documentation Updates

  • No documentation updates are required.
  • Yes, documentation updates are required. User-facing docs may want to mention that completion results now provide prompt-scoped change review and restore actions when checkpoints are available.

Additional Notes

This is intentionally inspired by Kilo Code legacy: the completion result itself carries the user-facing actions to review or undo the new changes, making it easier to validate an agent's work before starting a new task.

Get in Touch

ivanarifin

Summary by CodeRabbit

  • New Features
    • Added “See New Changes” and “Restore Changes” actions on completion results, enabling users to preview diffs and restore files/task state from the relevant checkpoint.
  • Bug Fixes
    • Improved completion checkpoint handling with safer restore behavior, including timeout handling and clearer error reporting when checkpoint operations fail.
  • Documentation
    • Added localized chat UI strings for the new actions across supported languages.
  • Tests
    • Expanded unit/integration coverage for checkpoint diff/restore flows, UI button behavior, and timeout/failure scenarios.

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds inline "see new changes" and "restore changes" actions to the chat UI's completion result row. A new CompletionCheckpoint type and getCompletionCheckpoint function anchor these actions to the correct checkpoint. Two new webview message types drive diff and restore operations in the backend handler, which cancels the task, waits for re-initialization, then calls the checkpoint API. Translations are added across 18 locales.

Changes

Completion Checkpoint Diff/Restore Feature

Layer / File(s) Summary
CompletionCheckpoint type, getCompletionCheckpoint function, and unit tests
packages/types/src/message.ts, packages/types/src/__tests__/message.test.ts
Extends clineSays with "task", defines CompletionCheckpoint { ts, commitHash } interface and getCompletionCheckpoint function to locate the correct checkpoint anchor by scanning message history backward to the latest user-prompt boundary before a completion-result, then forward to the next checkpoint_saved. Three unit tests validate the lookup across different boundary conditions.
WebviewMessage extension for completion checkpoint types
packages/types/src/vscode-extension-host.ts
Adds completionCheckpointDiff and completionCheckpointRestore as new WebviewMessage.type union members, and introduces optional checkpointTs?: number field to specify which checkpoint to operate on.
Backend handler: completionCheckpointDiff and completionCheckpointRestore
src/core/webview/webviewMessageHandler.ts
Imports getCompletionCheckpoint and implements resolveCompletionCheckpoint helper to derive checkpoints from clineMessages by explicit checkpointTs match or fallback. Adds two switch cases: diff calls checkpointDiff in to-current mode; restore cancels the task, polls isInitialized up to 3s, then calls checkpointRestore in restore mode with error handling. Early return added to existing checkpointRestore timeout path.
Backend handler comprehensive test suite
src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts, src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts
Mocks p-wait-for, sets up mockCline.isInitialized, mockCline.checkpointDiff, and mockProvider.cancelTask. Test suite covers diff/restore routing, checkpoint boundary matching, fallback behavior, timeout handling (triggers cancelTask without restore), restore failure (logs error and shows failure toast), and condition-met paths. Console spy setup suppresses test output via beforeAll/afterAll.
SeeNewChangesButtons React component
webview-ui/src/components/chat/SeeNewChangesButtons.tsx
Two-state UI component: initial state renders "see new changes" and "restore changes" buttons (restore disabled if checkpoint.commitHash is missing); restore state swaps to confirmation ("confirm restore" and "cancel"). Posts completionCheckpointDiff and completionCheckpointRestore messages via vscode.postMessage, uses react-i18next for localized labels/tooltips, and manages local restoringChanges state.
SeeNewChangesButtons component tests
webview-ui/src/components/chat/__tests__/SeeNewChangesButtons.spec.tsx
Vitest + React Testing Library mocking vscode.postMessage, useTranslation, and VSCodeButton. Tests verify: diff posts correct message, restore requires confirmation before posting, cancel reverts to initial state, tooltips render correctly, and restore is disabled when commitHash is empty.
ChatRow completion result integration
webview-ui/src/components/chat/ChatRow.tsx
Imports CompletionCheckpoint type and SeeNewChangesButtons component, extends ChatRowProps with optional completionCheckpoint, updates destructuring, and conditionally renders SeeNewChangesButtons in both say and ask completion_result paths when message is non-partial, checkpoints enabled, and completionCheckpoint provided.
ChatView completion checkpoint memoization and passing
webview-ui/src/components/chat/ChatView.tsx
Imports getCompletionCheckpoint, introduces memoized completionCheckpoint (from getCompletionCheckpoint(messages)) and completionResultTs (latest completion_result timestamp by backward scan for say: "completion_result" or non-empty ask: "completion_result"). Updates primary-button completion_result handler to call startNewTask(), passes completionCheckpoint to ChatRow only when row timestamp matches completionResultTs, and adds both to dependency list.
ChatView integration tests
webview-ui/src/components/chat/__tests__/ChatView.clear-approval-buttons.spec.tsx
Imports fireEvent, extends mock ClineMessage with optional checkpoint field, updates ChatRow mock to render checkpoint action buttons on completion_result with checkpoint, adds test helpers for completion sequences with/without latest-prompt checkpoints, creates button-label constants, and verifies inline checkpoint actions appear conditionally while "start new task" remains visible.
i18n translations (18 locales)
webview-ui/src/i18n/locales/{ca,de,en,es,fr,hi,id,it,ja,ko,nl,pl,pt-BR,ru,tr,vi,zh-CN,zh-TW}/chat.json
Adds seeNewChanges and restoreChanges translation keys (each with title and tooltip) to all supported chat locale files.
Documentation formatting
README.md
Whitespace/indentation adjustment on closing details tag.

Sequence Diagram

sequenceDiagram
  participant User
  participant ChatView
  participant SeeNewChangesButtons
  participant vscode as VSCode Webview API
  participant webviewMessageHandler
  participant currentCline

  User->>ChatView: views completion_result row
  ChatView->>ChatView: getCompletionCheckpoint(messages) → CompletionCheckpoint
  ChatView->>SeeNewChangesButtons: completionCheckpoint prop

  alt User clicks "see new changes"
    User->>SeeNewChangesButtons: click diff button
    SeeNewChangesButtons->>vscode: postMessage({ type: "completionCheckpointDiff", checkpointTs })
    vscode->>webviewMessageHandler: completionCheckpointDiff
    webviewMessageHandler->>currentCline: checkpointDiff({ ts, commitHash, mode: "to-current" })
  else User clicks "restore changes" → confirm
    User->>SeeNewChangesButtons: click restore → confirm
    SeeNewChangesButtons->>vscode: postMessage({ type: "completionCheckpointRestore", checkpointTs })
    vscode->>webviewMessageHandler: completionCheckpointRestore
    webviewMessageHandler->>currentCline: cancelTask()
    webviewMessageHandler->>webviewMessageHandler: pWaitFor(isInitialized, 3s)
    webviewMessageHandler->>currentCline: checkpointRestore({ ts, commitHash, mode: "restore" })
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

awaiting-review

Suggested reviewers

  • taltas
  • JamesRobert20
  • navedmerchant
  • hannesrudolph
  • edelauna

Poem

🐇 Hop back in time with a checkpoint click,
See what changed — the new and the slick!
Restore the past if the diff goes awry,
A bunny-sized button reaching up high.
Commit hash in paw, the trail never ends,
Around every corner, the timeline bends! 🕰️

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title "Add completion change actions" clearly and concisely describes the main feature being introduced across the pull request.
Description check ✅ Passed The pull request description is comprehensive and follows the template structure, including issue linkage, implementation details, test procedures, and checklists.
Linked Issues check ✅ Passed All code changes directly implement the requirements from issue #661: inline completion checkpoint actions with confirmation flows, checkpoint-scoped logic, and reuse of existing diff/restore mechanisms.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing completion checkpoint actions as specified in issue #661; no unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.46154% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/core/webview/webviewMessageHandler.ts 88.46% 1 Missing and 2 partials ⚠️
webview-ui/src/components/chat/ChatRow.tsx 33.33% 2 Missing ⚠️
webview-ui/src/components/chat/ChatView.tsx 91.66% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (3)
webview-ui/src/components/chat/utils/__tests__/completionCheckpoint.spec.ts (1)

1-19: ⚡ Quick win

Move this pure-logic test out of webview-ui test scope.

This file tests a shared pure utility (getCompletionCheckpoint) rather than React/webview behavior, so it should live in package-local/unit test scope to avoid duplicated cross-layer coverage.

As per coding guidelines, webview-ui tests should focus on React/webview behavior, while pure logic should use package-local unit tests.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@webview-ui/src/components/chat/utils/__tests__/completionCheckpoint.spec.ts`
around lines 1 - 19, The test for the getCompletionCheckpoint utility function
is currently located in the webview-ui test scope, but since this function is
pure logic imported from `@roo-code/types` and not React/webview-specific
behavior, it should be moved to the appropriate package-local unit test
directory where the actual utility implementation lives. Move the
completionCheckpoint.spec.ts test file from the webview-ui test directory to the
package-local unit tests for the `@roo-code/types` package to follow coding
guidelines that separate webview-specific tests from pure logic tests.

Source: Coding guidelines

src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts (1)

163-172: ⚡ Quick win

Add a timeout-path unit test for completionCheckpointRestore.

Current tests cover success/no-checkpoint, but not the re-init timeout path (where restore should not proceed). Add a test that forces pWaitFor rejection and asserts checkpointRestore is not called.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts` around
lines 163 - 172, Add a new unit test in the
webviewMessageHandler.checkpoint.spec.ts file that covers the timeout/re-init
path for completionCheckpointRestore. Create a test case following the same
structure as the existing "restores files and task state to the checkpoint
created after the latest prompt" test, but configure the test setup so that
pWaitFor rejects (simulating a timeout). In this timeout scenario, assert that
mockCline.checkpointRestore is not called, demonstrating that the restore does
not proceed when the re-init process times out.

Source: Coding guidelines

webview-ui/src/components/chat/__tests__/ChatView.clear-approval-buttons.spec.tsx (1)

160-176: ⚡ Quick win

Use task-shaped first message in completion fixtures.

At Line 161 and Line 166, these helpers start with say: "text". Using say: "task" keeps the fixture aligned with ChatView’s task-state invariant and avoids false confidence from non-representative state.

Suggested test fixture adjustment
 const completionAskWithoutCheckpoint = (): ClineMessage[] => [
-	{ type: "say", say: "text", ts: 1, text: "Initial task" },
+	{ type: "say", say: "task", ts: 1, text: "Initial task" },
 	{ type: "ask", ask: "completion_result", ts: 2, text: "Task complete", partial: false },
 ]

 const completionAskWithCheckpoint = (): ClineMessage[] => [
-	{ type: "say", say: "text", ts: 1, text: "Initial task" },
+	{ type: "say", say: "task", ts: 1, text: "Initial task" },
 	{
 		type: "say",
 		say: "checkpoint_saved",
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@webview-ui/src/components/chat/__tests__/ChatView.clear-approval-buttons.spec.tsx`
around lines 160 - 176, The test fixtures completionAskWithoutCheckpoint and
completionAskWithCheckpoint both start with messages containing say: "text", but
they should use say: "task" for the initial message to align with ChatView's
task-state invariant and ensure the fixtures represent realistic test state.
Change the say property from "text" to "task" in the first message object of
both helper functions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/core/webview/webviewMessageHandler.ts`:
- Around line 1313-1327: After the pWaitFor timeout is caught and the
checkpoint_timeout error message is displayed in the first try-catch block,
execution continues to the checkpointRestore call even though the task
initialization failed. Add a return statement immediately after the
vscode.window.showErrorMessage call in the first catch block to abort the
restore flow and prevent attempting to restore on an uninitialized task.

In `@webview-ui/src/components/chat/ChatView.tsx`:
- Around line 132-143: The completionResultTs useMemo hook only checks for
message.type === "say" when searching for completion_result messages, but it
should also match message.type === "ask". Update the conditional logic to accept
both "say" and "ask" types so that ask-type completion_result messages are
captured, allowing completionCheckpoint to be properly wired into the row
downstream where the inline actions are rendered.

In `@webview-ui/src/components/chat/SeeNewChangesButtons.tsx`:
- Around line 41-50: The VSCodeButton components for seeNewChangesCallback and
restoreChangesCallback are missing tooltip attributes that expose the newly
added translation keys seeNewChanges.tooltip and restoreChanges.tooltip. Add a
title attribute to both buttons using the t() translation function to wire in
the appropriate tooltip copy (t("chat:seeNewChanges.tooltip") for the first
button and t("chat:restoreChanges.tooltip") for the second button), so users can
see the contextual guidance when hovering over the buttons.
- Around line 13-21: The callbacks seeNewChangesCallback,
restoreChangesCallback, and confirmRestoreChangesCallback post messages without
including the checkpoint identity, which causes a race condition if new messages
arrive between button click and handler execution. Update both
vscode.postMessage calls (in the callbacks for "completionCheckpointDiff" and
"completionCheckpointRestore" message types) to include checkpointTs in the
message payload, add checkpoint.ts to the dependency arrays of these callbacks
to ensure they update when the checkpoint changes, and update the corresponding
test assertions in SeeNewChangesButtons.spec.tsx to expect the new payload
format with the checkpoint identity included.

---

Nitpick comments:
In `@src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts`:
- Around line 163-172: Add a new unit test in the
webviewMessageHandler.checkpoint.spec.ts file that covers the timeout/re-init
path for completionCheckpointRestore. Create a test case following the same
structure as the existing "restores files and task state to the checkpoint
created after the latest prompt" test, but configure the test setup so that
pWaitFor rejects (simulating a timeout). In this timeout scenario, assert that
mockCline.checkpointRestore is not called, demonstrating that the restore does
not proceed when the re-init process times out.

In
`@webview-ui/src/components/chat/__tests__/ChatView.clear-approval-buttons.spec.tsx`:
- Around line 160-176: The test fixtures completionAskWithoutCheckpoint and
completionAskWithCheckpoint both start with messages containing say: "text", but
they should use say: "task" for the initial message to align with ChatView's
task-state invariant and ensure the fixtures represent realistic test state.
Change the say property from "text" to "task" in the first message object of
both helper functions.

In `@webview-ui/src/components/chat/utils/__tests__/completionCheckpoint.spec.ts`:
- Around line 1-19: The test for the getCompletionCheckpoint utility function is
currently located in the webview-ui test scope, but since this function is pure
logic imported from `@roo-code/types` and not React/webview-specific behavior, it
should be moved to the appropriate package-local unit test directory where the
actual utility implementation lives. Move the completionCheckpoint.spec.ts test
file from the webview-ui test directory to the package-local unit tests for the
`@roo-code/types` package to follow coding guidelines that separate
webview-specific tests from pure logic tests.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c9c5752c-acd2-4ec5-9ebc-3a22539b1042

📥 Commits

Reviewing files that changed from the base of the PR and between 6daa153 and 54ee431.

📒 Files selected for processing (29)
  • packages/types/src/message.ts
  • packages/types/src/vscode-extension-host.ts
  • src/core/webview/__tests__/completionCheckpoint.spec.ts
  • src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts
  • src/core/webview/webviewMessageHandler.ts
  • webview-ui/src/components/chat/ChatRow.tsx
  • webview-ui/src/components/chat/ChatView.tsx
  • webview-ui/src/components/chat/SeeNewChangesButtons.tsx
  • webview-ui/src/components/chat/__tests__/ChatView.clear-approval-buttons.spec.tsx
  • webview-ui/src/components/chat/__tests__/SeeNewChangesButtons.spec.tsx
  • webview-ui/src/components/chat/utils/__tests__/completionCheckpoint.spec.ts
  • webview-ui/src/i18n/locales/ca/chat.json
  • webview-ui/src/i18n/locales/de/chat.json
  • webview-ui/src/i18n/locales/en/chat.json
  • webview-ui/src/i18n/locales/es/chat.json
  • webview-ui/src/i18n/locales/fr/chat.json
  • webview-ui/src/i18n/locales/hi/chat.json
  • webview-ui/src/i18n/locales/id/chat.json
  • webview-ui/src/i18n/locales/it/chat.json
  • webview-ui/src/i18n/locales/ja/chat.json
  • webview-ui/src/i18n/locales/ko/chat.json
  • webview-ui/src/i18n/locales/nl/chat.json
  • webview-ui/src/i18n/locales/pl/chat.json
  • webview-ui/src/i18n/locales/pt-BR/chat.json
  • webview-ui/src/i18n/locales/ru/chat.json
  • webview-ui/src/i18n/locales/tr/chat.json
  • webview-ui/src/i18n/locales/vi/chat.json
  • webview-ui/src/i18n/locales/zh-CN/chat.json
  • webview-ui/src/i18n/locales/zh-TW/chat.json

Comment thread src/core/webview/webviewMessageHandler.ts
Comment thread webview-ui/src/components/chat/ChatView.tsx
Comment thread webview-ui/src/components/chat/SeeNewChangesButtons.tsx
Comment thread webview-ui/src/components/chat/SeeNewChangesButtons.tsx Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts (1)

206-213: ⚡ Quick win

Assert the timeout error-reporting side effect in the timeout-path test.

Line 206 verifies cancel/restore behavior, but it doesn’t verify the user-visible error path that this branch is responsible for.

Suggested test assertion
 it("does not restore when task re-initialization times out", async () => {
 	;(pWaitFor as any).mockRejectedValueOnce(new Error("timed out"))

 	await webviewMessageHandler(mockProvider, { type: "completionCheckpointRestore", checkpointTs: 4 })

 	expect(mockProvider.cancelTask).toHaveBeenCalled()
 	expect(mockCline.checkpointRestore).not.toHaveBeenCalled()
+	const vscode = await import("vscode")
+	expect(vscode.window.showErrorMessage).toHaveBeenCalled()
 })
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts` around
lines 206 - 213, The test "does not restore when task re-initialization times
out" verifies that cancelTask is called and checkpointRestore is not called when
pWaitFor rejects with a timeout error, but it's missing an assertion for the
user-visible error reporting side effect. Add an expectation to verify that the
error reporting mechanism (such as an error notification or message sent to the
user) is invoked when the timeout occurs in the timeout path of the
webviewMessageHandler function.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts`:
- Around line 206-213: The test "does not restore when task re-initialization
times out" verifies that cancelTask is called and checkpointRestore is not
called when pWaitFor rejects with a timeout error, but it's missing an assertion
for the user-visible error reporting side effect. Add an expectation to verify
that the error reporting mechanism (such as an error notification or message
sent to the user) is invoked when the timeout occurs in the timeout path of the
webviewMessageHandler function.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 294d8467-d93e-422d-b110-748b0ab5a9a0

📥 Commits

Reviewing files that changed from the base of the PR and between 54ee431 and cfae65f.

📒 Files selected for processing (10)
  • packages/types/src/__tests__/message.test.ts
  • packages/types/src/message.ts
  • packages/types/src/vscode-extension-host.ts
  • src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts
  • src/core/webview/webviewMessageHandler.ts
  • webview-ui/src/components/chat/ChatRow.tsx
  • webview-ui/src/components/chat/ChatView.tsx
  • webview-ui/src/components/chat/SeeNewChangesButtons.tsx
  • webview-ui/src/components/chat/__tests__/ChatView.clear-approval-buttons.spec.tsx
  • webview-ui/src/components/chat/__tests__/SeeNewChangesButtons.spec.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
  • webview-ui/src/components/chat/tests/SeeNewChangesButtons.spec.tsx
  • webview-ui/src/components/chat/SeeNewChangesButtons.tsx
  • webview-ui/src/components/chat/ChatView.tsx
  • webview-ui/src/components/chat/ChatRow.tsx
  • src/core/webview/webviewMessageHandler.ts
  • webview-ui/src/components/chat/tests/ChatView.clear-approval-buttons.spec.tsx
  • packages/types/src/message.ts

ivanarifin and others added 4 commits June 16, 2026 14:02
…rror

- Mock `console.log` and `console.error` in `ClineProvider.flicker-free-cancel.spec.ts` to prevent test output pollution.
- Add assertion for `vscode.window.showErrorMessage` with `"errors.checkpoint_timeout"` in `webviewMessageHandler.checkpoint.spec.ts`.

@edelauna edelauna left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks for adding this, had a couple comments.

Could you also add a screen recording showing this change in action.

Thank you.

Comment thread webview-ui/src/components/chat/SeeNewChangesButtons.tsx Outdated
Comment thread packages/types/src/message.ts Outdated
Comment thread src/core/webview/webviewMessageHandler.ts
Comment thread src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts
Comment thread src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts
Comment thread src/core/webview/webviewMessageHandler.ts Outdated
Comment thread src/core/webview/webviewMessageHandler.ts
Comment thread src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts Outdated
Comment thread packages/types/src/__tests__/message.test.ts Outdated
@github-actions github-actions Bot added the awaiting-author PR is waiting for the author to address requested changes label Jun 21, 2026
…mpletion-change-actions

# Conflicts:
#	src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts
@ivanarifin

Copy link
Copy Markdown
Contributor Author

Nice! Thanks for adding this, had a couple comments.

Could you also add a screen recording showing this change in action.

Thank you.

Screen.Recording.2026-06-22.at.05.01.36.mp4

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts (1)

211-219: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Consider adding a restore fallback test for symmetry.

The new fallback test at lines 211-219 verifies that completionCheckpointDiff correctly falls back to the latest completion checkpoint when checkpointTs is omitted. For completeness, consider adding a parallel test that verifies the same fallback behavior for completionCheckpointRestore.

While not strictly necessary (both operations share resolveCompletionCheckpoint logic), a symmetric test would make the coverage more explicit and easier to understand.

📝 Suggested additional test
it("falls back to the latest completion checkpoint when checkpoint timestamp is omitted (restore)", async () => {
	const callOrder: string[] = []
	mockProvider.cancelTask.mockImplementation(async () => callOrder.push("cancelTask"))
	mockCline.checkpointRestore.mockImplementation(async () => callOrder.push("checkpointRestore"))

	await webviewMessageHandler(mockProvider, { type: "completionCheckpointRestore" })

	expect(mockCline.checkpointRestore).toHaveBeenCalledWith({
		ts: 4,
		commitHash: "latest-prompt-checkpoint",
		mode: "restore",
	})
})
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts` around
lines 211 - 219, Add a symmetric test case to verify that
completionCheckpointRestore also falls back to the latest completion checkpoint
when checkpointTs is omitted. Create a new test following the same pattern as
the existing fallback test for completionCheckpointDiff, but for the
completionCheckpointRestore operation type. Mock mockProvider.cancelTask and
mockCline.checkpointRestore appropriately, invoke webviewMessageHandler with
type set to completionCheckpointRestore and no checkpointTs parameter, then
assert that mockCline.checkpointRestore is called with the expected parameters
(ts: 4, commitHash: "latest-prompt-checkpoint", and mode: "restore").
src/core/webview/webviewMessageHandler.ts (1)

924-1009: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Wrap case block in braces for consistency and linter compliance.

The importRooHistory case declares variables (latestProgress) without wrapping the case body in braces. Other cases in this file that declare variables (e.g., askResponse, checkpointRestore, completionCheckpointDiff) consistently use braces. While the code functions correctly, wrapping in braces prevents static analysis warnings and aligns with the established pattern.

♻️ Wrap case in braces
-		case "importRooHistory": {
+		case "importRooHistory": {{
 			let latestProgress = {
 				copiedFileCount: 0,
 				totalFileCount: 0,
 				importedTaskCount: 0,
 				totalTaskCount: 0,
 			}
 
 			try {
 				// ... rest of case body ...
 			}
 			break
-		}
+		}}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/webview/webviewMessageHandler.ts` around lines 924 - 1009, The
importRooHistory case block should be wrapped in curly braces to create proper
block scope for the latestProgress variable and maintain consistency with other
cases in this switch statement that declare variables (such as askResponse,
checkpointRestore, completionCheckpointDiff). Ensure that the case
"importRooHistory" has an opening brace after the colon and wraps the entire
case body (including the try-catch block and the break statement) with a closing
brace to comply with linter rules and established patterns in this file.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts`:
- Around line 211-219: Add a symmetric test case to verify that
completionCheckpointRestore also falls back to the latest completion checkpoint
when checkpointTs is omitted. Create a new test following the same pattern as
the existing fallback test for completionCheckpointDiff, but for the
completionCheckpointRestore operation type. Mock mockProvider.cancelTask and
mockCline.checkpointRestore appropriately, invoke webviewMessageHandler with
type set to completionCheckpointRestore and no checkpointTs parameter, then
assert that mockCline.checkpointRestore is called with the expected parameters
(ts: 4, commitHash: "latest-prompt-checkpoint", and mode: "restore").

In `@src/core/webview/webviewMessageHandler.ts`:
- Around line 924-1009: The importRooHistory case block should be wrapped in
curly braces to create proper block scope for the latestProgress variable and
maintain consistency with other cases in this switch statement that declare
variables (such as askResponse, checkpointRestore, completionCheckpointDiff).
Ensure that the case "importRooHistory" has an opening brace after the colon and
wraps the entire case body (including the try-catch block and the break
statement) with a closing brace to comply with linter rules and established
patterns in this file.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8bfe2ccf-4712-4a9a-b6a2-57e3bac827f0

📥 Commits

Reviewing files that changed from the base of the PR and between 12befec and 0991ea5.

📒 Files selected for processing (27)
  • README.md
  • packages/types/src/__tests__/message.test.ts
  • packages/types/src/message.ts
  • packages/types/src/vscode-extension-host.ts
  • src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts
  • src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts
  • src/core/webview/webviewMessageHandler.ts
  • webview-ui/src/components/chat/SeeNewChangesButtons.tsx
  • webview-ui/src/components/chat/__tests__/SeeNewChangesButtons.spec.tsx
  • webview-ui/src/i18n/locales/ca/chat.json
  • webview-ui/src/i18n/locales/de/chat.json
  • webview-ui/src/i18n/locales/en/chat.json
  • webview-ui/src/i18n/locales/es/chat.json
  • webview-ui/src/i18n/locales/fr/chat.json
  • webview-ui/src/i18n/locales/hi/chat.json
  • webview-ui/src/i18n/locales/id/chat.json
  • webview-ui/src/i18n/locales/it/chat.json
  • webview-ui/src/i18n/locales/ja/chat.json
  • webview-ui/src/i18n/locales/ko/chat.json
  • webview-ui/src/i18n/locales/nl/chat.json
  • webview-ui/src/i18n/locales/pl/chat.json
  • webview-ui/src/i18n/locales/pt-BR/chat.json
  • webview-ui/src/i18n/locales/ru/chat.json
  • webview-ui/src/i18n/locales/tr/chat.json
  • webview-ui/src/i18n/locales/vi/chat.json
  • webview-ui/src/i18n/locales/zh-CN/chat.json
  • webview-ui/src/i18n/locales/zh-TW/chat.json
✅ Files skipped from review due to trivial changes (15)
  • webview-ui/src/i18n/locales/es/chat.json
  • README.md
  • webview-ui/src/i18n/locales/de/chat.json
  • webview-ui/src/i18n/locales/ru/chat.json
  • webview-ui/src/i18n/locales/it/chat.json
  • webview-ui/src/i18n/locales/tr/chat.json
  • webview-ui/src/i18n/locales/ca/chat.json
  • webview-ui/src/i18n/locales/ja/chat.json
  • webview-ui/src/i18n/locales/hi/chat.json
  • webview-ui/src/i18n/locales/zh-CN/chat.json
  • webview-ui/src/i18n/locales/fr/chat.json
  • webview-ui/src/i18n/locales/pt-BR/chat.json
  • src/core/webview/tests/ClineProvider.flicker-free-cancel.spec.ts
  • webview-ui/src/i18n/locales/en/chat.json
  • webview-ui/src/i18n/locales/pl/chat.json
🚧 Files skipped from review as they are similar to previous changes (10)
  • packages/types/src/tests/message.test.ts
  • webview-ui/src/components/chat/SeeNewChangesButtons.tsx
  • packages/types/src/vscode-extension-host.ts
  • webview-ui/src/i18n/locales/ko/chat.json
  • webview-ui/src/i18n/locales/vi/chat.json
  • webview-ui/src/components/chat/tests/SeeNewChangesButtons.spec.tsx
  • webview-ui/src/i18n/locales/zh-TW/chat.json
  • webview-ui/src/i18n/locales/nl/chat.json
  • packages/types/src/message.ts
  • webview-ui/src/i18n/locales/id/chat.json

@ivanarifin ivanarifin requested a review from edelauna June 22, 2026 03:26
@github-actions github-actions Bot added awaiting-review PR changes are ready and waiting for maintainer re-review and removed awaiting-author PR is waiting for the author to address requested changes labels Jun 22, 2026
Comment thread src/core/webview/webviewMessageHandler.ts
@ivanarifin ivanarifin requested a review from edelauna June 22, 2026 14:51
Comment thread src/core/webview/webviewMessageHandler.ts
Comment thread webview-ui/src/components/chat/SeeNewChangesButtons.tsx
Comment thread webview-ui/src/components/chat/ChatView.tsx

@edelauna edelauna left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was reviewing this more in depth and wanted to also request some architectural changes to the feature to not allow the webview to pass a checkpointTs.

Comment thread packages/types/src/vscode-extension-host.ts
Comment thread src/core/webview/webviewMessageHandler.ts
Comment thread src/core/webview/webviewMessageHandler.ts
@github-actions github-actions Bot removed the awaiting-review PR changes are ready and waiting for maintainer re-review label Jun 22, 2026
@github-actions github-actions Bot added the awaiting-author PR is waiting for the author to address requested changes label Jun 22, 2026
@ivanarifin ivanarifin requested a review from edelauna June 23, 2026 07:25
@github-actions github-actions Bot added awaiting-review PR changes are ready and waiting for maintainer re-review and removed awaiting-author PR is waiting for the author to address requested changes labels Jun 23, 2026

@edelauna edelauna left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool feature, thanks for adding it.

@edelauna edelauna enabled auto-merge June 24, 2026 22:38
@edelauna edelauna added this pull request to the merge queue Jun 24, 2026
Merged via the queue into Zoo-Code-Org:main with commit 0084cc8 Jun 24, 2026
11 checks passed
hacker-b2k pushed a commit to hacker-b2k/Zoo-Code that referenced this pull request Jun 29, 2026
* feat: add completion change actions

* fix: address completion action review feedback

* test(webview): mock console output and assert on checkpoint timeout error

- Mock `console.log` and `console.error` in `ClineProvider.flicker-free-cancel.spec.ts` to prevent test output pollution.
- Add assertion for `vscode.window.showErrorMessage` with `"errors.checkpoint_timeout"` in `webviewMessageHandler.checkpoint.spec.ts`.

* fix: address completion checkpoint review feedback

* feat(ChatRow): adding key to react component

---------

Co-authored-by: Elliott de Launay <edelauna@gmail.com>
# Conflicts:
#	webview-ui/src/components/chat/ChatView.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-review PR changes are ready and waiting for maintainer re-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ENHANCEMENT] Add completion change review and restore actions

2 participants