Skip to content

fix: show edit warning dialog only once per session #6059

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/core/webview/ClineProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ export class ClineProvider
public readonly latestAnnouncementId = "jul-09-2025-3-23-0" // Update for v3.23.0 announcement
public readonly providerSettingsManager: ProviderSettingsManager
public readonly customModesManager: CustomModesManager
public hasShownEditWarning = false // Session-based flag for edit warning dialog

constructor(
readonly context: vscode.ExtensionContext,
Expand Down
11 changes: 7 additions & 4 deletions src/core/webview/__tests__/ClineProvider.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2909,21 +2909,24 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => {

await Promise.all([edit1Promise, edit2Promise])

// Verify dialogs were shown for both edits
// With the new implementation, only the first edit shows the dialog
// The second edit happens after hasShownEditWarning is already true
expect(mockPostMessage).toHaveBeenCalledWith({
type: "showEditMessageDialog",
messageTs: 2000,
text: "Edited message 1",
images: undefined,
})
expect(mockPostMessage).toHaveBeenCalledWith({

// The second edit should not show a dialog
expect(mockPostMessage).not.toHaveBeenCalledWith({
type: "showEditMessageDialog",
messageTs: 4000,
text: "Edited message 2",
})

// Simulate user confirming both edits
// Simulate user confirming the first edit
await messageHandler({ type: "editMessageConfirm", messageTs: 2000, text: "Edited message 1" })
await messageHandler({ type: "editMessageConfirm", messageTs: 4000, text: "Edited message 2" })

// Both operations should complete without throwing
expect(mockCline.overwriteClineMessages).toHaveBeenCalled()
Expand Down
72 changes: 71 additions & 1 deletion src/core/webview/__tests__/webviewMessageHandler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -517,8 +517,9 @@ describe("webviewMessageHandler - message dialog preferences", () => {
})

describe("submitEditedMessage", () => {
it("should always show dialog for edit confirmation", async () => {
it("should show dialog for edit confirmation on first edit in session", async () => {
vi.mocked(mockClineProvider.getCurrentCline).mockReturnValue({} as any) // Mock current cline exists
mockClineProvider.hasShownEditWarning = false // Reset the flag

await webviewMessageHandler(mockClineProvider, {
type: "submitEditedMessage",
Expand All @@ -531,6 +532,75 @@ describe("webviewMessageHandler - message dialog preferences", () => {
messageTs: 123456789,
text: "edited content",
})
expect(mockClineProvider.hasShownEditWarning).toBe(true)
})

it("should not show dialog for subsequent edits in the same session", async () => {
const editMessageTs = 1234567890000 // Message to edit
const mockCline = {
taskId: "test-task-id",
apiConversationHistory: [
{ role: "user", content: "Previous message", ts: editMessageTs - 5000 },
{ role: "assistant", content: "Response", ts: editMessageTs - 4000 },
{ role: "user", content: "Message to edit", ts: editMessageTs },
{ role: "assistant", content: "Later response", ts: editMessageTs + 2000 },
],
clineMessages: [
{ ts: editMessageTs - 5000, type: "say", say: "text", text: "Much earlier message" },
{ ts: editMessageTs - 2000, type: "say", say: "text", text: "Earlier message" },
{ ts: editMessageTs, type: "say", say: "user_feedback", text: "Original message to edit" },
{ ts: editMessageTs + 2000, type: "say", say: "text", text: "Later message" },
],
overwriteClineMessages: vi.fn().mockResolvedValue(undefined),
overwriteApiConversationHistory: vi.fn().mockResolvedValue(undefined),
handleWebviewAskResponse: vi.fn(),
}

// Mock getCurrentCline to always return our mockCline
vi.mocked(mockClineProvider.getCurrentCline).mockReturnValue(mockCline as any)

vi.mocked(mockClineProvider.getTaskWithId).mockResolvedValue({
historyItem: { id: "test-task-id" } as any,
taskDirPath: "/test/path",
apiConversationHistoryFilePath: "/test/path/api.json",
uiMessagesFilePath: "/test/path/ui.json",
apiConversationHistory: [],
})
vi.mocked(mockClineProvider.initClineWithHistoryItem).mockResolvedValue({} as any)

mockClineProvider.hasShownEditWarning = true // Flag already set from previous edit

await webviewMessageHandler(mockClineProvider, {
type: "submitEditedMessage",
value: editMessageTs,
editedMessageContent: "edited content",
images: undefined,
})

// Should not show dialog
expect(mockClineProvider.postMessageToWebview).not.toHaveBeenCalledWith({
type: "showEditMessageDialog",
messageTs: editMessageTs,
text: "edited content",
})

// Should have called overwrite methods to remove messages from the timestamp onwards
// The cutoff is editMessageTs - 1000, so messages before that should remain
expect(mockCline.overwriteClineMessages).toHaveBeenCalledWith([
{ ts: editMessageTs - 5000, type: "say", say: "text", text: "Much earlier message" },
{ ts: editMessageTs - 2000, type: "say", say: "text", text: "Earlier message" },
])
expect(mockCline.overwriteApiConversationHistory).toHaveBeenCalledWith([
{ role: "user", content: "Previous message", ts: editMessageTs - 5000 },
{ role: "assistant", content: "Response", ts: editMessageTs - 4000 },
])

// Should have called handleWebviewAskResponse
expect(mockCline.handleWebviewAskResponse).toHaveBeenCalledWith(
"messageResponse",
"edited content",
undefined,
)
})
})
})
21 changes: 14 additions & 7 deletions src/core/webview/webviewMessageHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,13 +138,20 @@ export const webviewMessageHandler = async (
* Handles message editing operations with user confirmation
*/
const handleEditOperation = async (messageTs: number, editedContent: string, images?: string[]): Promise<void> => {
// Send message to webview to show edit confirmation dialog
await provider.postMessageToWebview({
type: "showEditMessageDialog",
messageTs,
text: editedContent,
images,
})
// Check if we've already shown the edit warning in this session
if (!provider.hasShownEditWarning) {
// First time editing in this session - show the warning dialog
provider.hasShownEditWarning = true
await provider.postMessageToWebview({
type: "showEditMessageDialog",
messageTs,
text: editedContent,
images,
})
} else {
// Already shown the warning in this session - proceed directly with the edit
await handleEditMessageConfirm(messageTs, editedContent, images)
}
}

/**
Expand Down