Skip to content

Commit d0b069c

Browse files
committed
fix: clear pending edit operations when Start New Task is clicked
- Clear pending edit operations in clearTask() method to prevent them from being processed - Add safeguard in createTaskWithHistoryItem() to clear stale pending operations - Only process pending edits for tasks being resumed, not cleared/replaced - Add comprehensive test coverage for the fix Fixes #7949 - Start New Task no longer reopens completed session at earlier cancel point
1 parent c4c4780 commit d0b069c

File tree

2 files changed

+321
-4
lines changed

2 files changed

+321
-4
lines changed

src/core/webview/ClineProvider.ts

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -817,6 +817,13 @@ export class ClineProvider
817817
public async createTaskWithHistoryItem(historyItem: HistoryItem & { rootTask?: Task; parentTask?: Task }) {
818818
await this.removeClineFromStack()
819819

820+
// Clear any pending edit operations for this task to prevent them from
821+
// being processed when the task is recreated (fixes issue #7949)
822+
const operationId = `task-${historyItem.id}`
823+
if (this.clearPendingEditOperation(operationId)) {
824+
this.log(`[createTaskWithHistoryItem] Cleared stale pending edit operation for task ${historyItem.id}`)
825+
}
826+
820827
// If the history item has a saved mode, restore it and its associated API configuration.
821828
if (historyItem.mode) {
822829
// Validate that the mode still exists
@@ -894,16 +901,26 @@ export class ClineProvider
894901
)
895902

896903
// Check if there's a pending edit after checkpoint restoration
897-
const operationId = `task-${task.taskId}`
898-
const pendingEdit = this.getPendingEditOperation(operationId)
899-
if (pendingEdit) {
900-
this.clearPendingEditOperation(operationId) // Clear the pending edit
904+
// Only process if this is actually a checkpoint restoration (not a new task creation)
905+
const checkOperationId = `task-${task.taskId}`
906+
const pendingEdit = this.getPendingEditOperation(checkOperationId)
907+
if (pendingEdit && historyItem.ts) {
908+
// Only process pending edits for tasks that are being resumed, not cleared
909+
this.clearPendingEditOperation(checkOperationId) // Clear the pending edit
901910

902911
this.log(`[createTaskWithHistoryItem] Processing pending edit after checkpoint restoration`)
903912

904913
// Process the pending edit after a short delay to ensure the task is fully initialized
905914
setTimeout(async () => {
906915
try {
916+
// Verify the task is still active and not abandoned
917+
if (task.abandoned || this.getCurrentTask()?.taskId !== task.taskId) {
918+
this.log(
919+
`[createTaskWithHistoryItem] Task ${task.taskId} is no longer active, skipping pending edit`,
920+
)
921+
return
922+
}
923+
907924
// Find the message index in the restored state
908925
const { messageIndex, apiConversationHistoryIndex } = (() => {
909926
const messageIndex = task.clineMessages.findIndex((msg) => msg.ts === pendingEdit.messageTs)
@@ -2554,6 +2571,14 @@ export class ClineProvider
25542571
if (this.clineStack.length > 0) {
25552572
const task = this.clineStack[this.clineStack.length - 1]
25562573
console.log(`[clearTask] clearing task ${task.taskId}.${task.instanceId}`)
2574+
2575+
// Clear any pending edit operations for this task to prevent them from
2576+
// being processed when a new task is created (fixes issue #7949)
2577+
const operationId = `task-${task.taskId}`
2578+
if (this.clearPendingEditOperation(operationId)) {
2579+
this.log(`[clearTask] Cleared pending edit operation for task ${task.taskId}`)
2580+
}
2581+
25572582
await this.removeClineFromStack()
25582583
}
25592584
}
Lines changed: 292 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,292 @@
1+
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
2+
import * as vscode from "vscode"
3+
import { ClineProvider } from "../ClineProvider"
4+
import { ContextProxy } from "../../config/ContextProxy"
5+
import { Task } from "../../task/Task"
6+
7+
// Mock vscode module
8+
vi.mock("vscode", () => ({
9+
ExtensionContext: vi.fn(),
10+
ExtensionMode: { Development: 1, Production: 2, Test: 3 },
11+
Uri: {
12+
file: vi.fn((path: string) => ({ fsPath: path, toString: () => path })),
13+
parse: vi.fn((uri: string) => ({ fsPath: uri })),
14+
},
15+
workspace: {
16+
getConfiguration: vi.fn(() => ({
17+
get: vi.fn(),
18+
update: vi.fn(),
19+
})),
20+
workspaceFolders: undefined,
21+
onDidChangeConfiguration: vi.fn(),
22+
},
23+
window: {
24+
showErrorMessage: vi.fn(),
25+
showInformationMessage: vi.fn(),
26+
createTextEditorDecorationType: vi.fn(() => ({
27+
dispose: vi.fn(),
28+
})),
29+
onDidChangeActiveTextEditor: vi.fn(),
30+
},
31+
env: {
32+
uriScheme: "vscode",
33+
language: "en",
34+
machineId: "test-machine-id",
35+
sessionId: "test-session-id",
36+
appName: "Visual Studio Code",
37+
},
38+
version: "1.0.0",
39+
commands: {
40+
executeCommand: vi.fn(),
41+
},
42+
Range: vi.fn(),
43+
Position: vi.fn(),
44+
}))
45+
46+
// Mock other dependencies
47+
vi.mock("../../config/ContextProxy")
48+
vi.mock("../../task/Task")
49+
vi.mock("../../../services/mcp/McpServerManager", () => ({
50+
McpServerManager: {
51+
getInstance: vi.fn().mockResolvedValue(undefined),
52+
unregisterProvider: vi.fn(),
53+
},
54+
}))
55+
vi.mock("../../../services/marketplace", () => ({
56+
MarketplaceManager: vi.fn().mockImplementation(() => ({
57+
getMarketplaceItems: vi.fn().mockResolvedValue({ organizationMcps: [], marketplaceItems: [], errors: [] }),
58+
getInstallationMetadata: vi.fn().mockResolvedValue({ project: {}, global: {} }),
59+
cleanup: vi.fn(),
60+
})),
61+
}))
62+
vi.mock("../../config/CustomModesManager", () => ({
63+
CustomModesManager: vi.fn().mockImplementation(() => ({
64+
getCustomModes: vi.fn().mockResolvedValue([]),
65+
dispose: vi.fn(),
66+
})),
67+
}))
68+
vi.mock("../../config/ProviderSettingsManager", () => ({
69+
ProviderSettingsManager: vi.fn().mockImplementation(() => ({
70+
listConfig: vi.fn().mockResolvedValue([]),
71+
getModeConfigId: vi.fn().mockResolvedValue(undefined),
72+
resetAllConfigs: vi.fn(),
73+
})),
74+
}))
75+
vi.mock("../../../integrations/workspace/WorkspaceTracker", () => ({
76+
default: vi.fn().mockImplementation(() => ({
77+
dispose: vi.fn(),
78+
})),
79+
}))
80+
vi.mock("@roo-code/telemetry", () => ({
81+
TelemetryService: {
82+
instance: {
83+
setProvider: vi.fn(),
84+
captureCodeActionUsed: vi.fn(),
85+
captureModeSwitch: vi.fn(),
86+
},
87+
},
88+
}))
89+
vi.mock("@roo-code/cloud", () => ({
90+
CloudService: {
91+
hasInstance: vi.fn().mockReturnValue(false),
92+
instance: {
93+
isAuthenticated: vi.fn().mockReturnValue(false),
94+
getAllowList: vi.fn().mockResolvedValue("*"),
95+
getUserInfo: vi.fn().mockReturnValue(null),
96+
getOrganizationSettings: vi.fn().mockReturnValue(null),
97+
isTaskSyncEnabled: vi.fn().mockReturnValue(false),
98+
canShareTask: vi.fn().mockResolvedValue(false),
99+
getUserSettings: vi.fn().mockReturnValue(null),
100+
},
101+
},
102+
BridgeOrchestrator: {
103+
isEnabled: vi.fn().mockReturnValue(false),
104+
connectOrDisconnect: vi.fn(),
105+
getInstance: vi.fn().mockReturnValue(null),
106+
subscribeToTask: vi.fn(),
107+
unsubscribeFromTask: vi.fn(),
108+
},
109+
getRooCodeApiUrl: vi.fn().mockReturnValue("https://api.roo-code.com"),
110+
}))
111+
112+
describe("ClineProvider - clearTask with pending operations", () => {
113+
let provider: ClineProvider
114+
let mockContext: vscode.ExtensionContext
115+
let mockOutputChannel: vscode.OutputChannel
116+
let mockContextProxy: ContextProxy
117+
118+
beforeEach(() => {
119+
// Create mock instances
120+
mockContext = {
121+
globalStorageUri: { fsPath: "/test/storage" },
122+
extension: { packageJSON: { version: "1.0.0", name: "roo-code" } },
123+
} as any
124+
125+
mockOutputChannel = {
126+
appendLine: vi.fn(),
127+
} as any
128+
129+
mockContextProxy = {
130+
getValues: vi.fn().mockReturnValue({
131+
taskHistory: [],
132+
mode: "code",
133+
apiConfiguration: { apiProvider: "anthropic" },
134+
}),
135+
getValue: vi.fn(),
136+
setValue: vi.fn(),
137+
setValues: vi.fn(),
138+
getProviderSettings: vi.fn().mockReturnValue({ apiProvider: "anthropic" }),
139+
setProviderSettings: vi.fn(),
140+
resetAllState: vi.fn(),
141+
extensionUri: { fsPath: "/test/extension" },
142+
globalStorageUri: { fsPath: "/test/storage" },
143+
} as any
144+
145+
// Create provider instance
146+
provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", mockContextProxy)
147+
})
148+
149+
afterEach(() => {
150+
vi.clearAllMocks()
151+
})
152+
153+
it("should clear pending edit operations when clearTask is called", async () => {
154+
// Setup: Create a mock task with a pending edit operation
155+
const mockTask = {
156+
taskId: "test-task-123",
157+
instanceId: "instance-1",
158+
clineMessages: [],
159+
apiConversationHistory: [],
160+
abandoned: false,
161+
parentTask: undefined,
162+
abortTask: vi.fn().mockResolvedValue(undefined),
163+
overwriteClineMessages: vi.fn(),
164+
overwriteApiConversationHistory: vi.fn(),
165+
handleWebviewAskResponse: vi.fn(),
166+
emit: vi.fn(),
167+
on: vi.fn(),
168+
off: vi.fn(),
169+
} as any
170+
171+
// Add task to stack
172+
;(provider as any).clineStack = [mockTask]
173+
174+
// Set a pending edit operation for this task
175+
const operationId = `task-${mockTask.taskId}`
176+
;(provider as any).setPendingEditOperation(operationId, {
177+
messageTs: 123456789,
178+
editedContent: "edited content",
179+
images: [],
180+
messageIndex: 0,
181+
apiConversationHistoryIndex: 0,
182+
})
183+
184+
// Verify the pending operation exists
185+
expect((provider as any).getPendingEditOperation(operationId)).toBeDefined()
186+
187+
// Act: Call clearTask
188+
await provider.clearTask()
189+
190+
// Assert: Pending operation should be cleared
191+
expect((provider as any).getPendingEditOperation(operationId)).toBeUndefined()
192+
expect((provider as any).clineStack.length).toBe(0)
193+
})
194+
195+
it("should not process pending edits when createTaskWithHistoryItem is called after clearTask", async () => {
196+
// Setup: Create a history item
197+
const historyItem = {
198+
id: "test-task-123",
199+
ts: Date.now(),
200+
task: "Test task",
201+
mode: "code",
202+
workspace: "/test/workspace",
203+
}
204+
205+
// Set a pending edit operation that should be cleared
206+
const operationId = `task-${historyItem.id}`
207+
;(provider as any).setPendingEditOperation(operationId, {
208+
messageTs: 123456789,
209+
editedContent: "edited content that should not be processed",
210+
images: [],
211+
messageIndex: 0,
212+
apiConversationHistoryIndex: 0,
213+
})
214+
215+
// Mock Task constructor to capture task creation
216+
const mockTaskInstance = {
217+
taskId: historyItem.id,
218+
instanceId: "instance-1",
219+
clineMessages: [],
220+
apiConversationHistory: [],
221+
abandoned: false,
222+
overwriteClineMessages: vi.fn(),
223+
overwriteApiConversationHistory: vi.fn(),
224+
handleWebviewAskResponse: vi.fn(),
225+
emit: vi.fn(),
226+
on: vi.fn(),
227+
off: vi.fn(),
228+
}
229+
230+
vi.mocked(Task).mockImplementation(() => mockTaskInstance as any)
231+
232+
// Act: Create task with history item (simulating "Start New Task" after completion)
233+
await provider.createTaskWithHistoryItem(historyItem as any)
234+
235+
// Wait a bit to ensure any setTimeout callbacks would have executed
236+
await new Promise((resolve) => setTimeout(resolve, 200))
237+
238+
// Assert: Pending operation should have been cleared and not processed
239+
expect((provider as any).getPendingEditOperation(operationId)).toBeUndefined()
240+
expect(mockTaskInstance.handleWebviewAskResponse).not.toHaveBeenCalled()
241+
expect(mockTaskInstance.overwriteClineMessages).not.toHaveBeenCalled()
242+
})
243+
244+
it("should handle clearTask when no task is active", async () => {
245+
// Setup: No tasks in stack
246+
;(provider as any).clineStack = []
247+
248+
// Act & Assert: Should not throw
249+
await expect(provider.clearTask()).resolves.not.toThrow()
250+
})
251+
252+
it("should clear pending operations even if task removal fails", async () => {
253+
// Setup: Create a mock task that will throw during removal
254+
const mockTask = {
255+
taskId: "test-task-456",
256+
instanceId: "instance-2",
257+
abandoned: false,
258+
abortTask: vi.fn().mockRejectedValue(new Error("Abort failed")),
259+
emit: vi.fn(),
260+
on: vi.fn(),
261+
off: vi.fn(),
262+
} as any
263+
264+
;(provider as any).clineStack = [mockTask]
265+
266+
// Set a pending edit operation
267+
const operationId = `task-${mockTask.taskId}`
268+
;(provider as any).setPendingEditOperation(operationId, {
269+
messageTs: 987654321,
270+
editedContent: "content",
271+
images: [],
272+
messageIndex: 0,
273+
apiConversationHistoryIndex: 0,
274+
})
275+
276+
// Spy on removeClineFromStack to simulate failure
277+
const removeSpy = vi
278+
.spyOn(provider as any, "removeClineFromStack")
279+
.mockRejectedValue(new Error("Remove failed"))
280+
281+
// Act: Call clearTask (should clear pending operation before attempting removal)
282+
try {
283+
await provider.clearTask()
284+
} catch {
285+
// Expected to throw due to mocked failure
286+
}
287+
288+
// Assert: Pending operation should still be cleared despite removal failure
289+
expect((provider as any).getPendingEditOperation(operationId)).toBeUndefined()
290+
expect(removeSpy).toHaveBeenCalled()
291+
})
292+
})

0 commit comments

Comments
 (0)