Browse Source

Merge pull request #1682 from GitlyHallows/bug/old-task-deletion

Fix old task deletion bug
Chris Estreich 10 months ago
parent
commit
bfd60097d0
3 changed files with 143 additions and 22 deletions
  1. 66 22
      src/core/webview/ClineProvider.ts
  2. 72 0
      src/core/webview/__tests__/ClineProvider.test.ts
  3. 5 0
      src/extension.ts

+ 66 - 22
src/core/webview/ClineProvider.ts

@@ -2208,33 +2208,51 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
 	}> {
 		const history = ((await this.getGlobalState("taskHistory")) as HistoryItem[] | undefined) || []
 		const historyItem = history.find((item) => item.id === id)
-		if (historyItem) {
-			const taskDirPath = path.join(this.contextProxy.globalStorageUri.fsPath, "tasks", id)
-			const apiConversationHistoryFilePath = path.join(taskDirPath, GlobalFileNames.apiConversationHistory)
-			const uiMessagesFilePath = path.join(taskDirPath, GlobalFileNames.uiMessages)
-			const fileExists = await fileExistsAtPath(apiConversationHistoryFilePath)
-			if (fileExists) {
-				const apiConversationHistory = JSON.parse(await fs.readFile(apiConversationHistoryFilePath, "utf8"))
-				return {
-					historyItem,
-					taskDirPath,
-					apiConversationHistoryFilePath,
-					uiMessagesFilePath,
-					apiConversationHistory,
-				}
-			}
+		if (!historyItem) {
+			throw new Error("Task not found in history")
+		}
+
+		const taskDirPath = path.join(this.contextProxy.globalStorageUri.fsPath, "tasks", id)
+		const apiConversationHistoryFilePath = path.join(taskDirPath, GlobalFileNames.apiConversationHistory)
+		const uiMessagesFilePath = path.join(taskDirPath, GlobalFileNames.uiMessages)
+
+		const fileExists = await fileExistsAtPath(apiConversationHistoryFilePath)
+		if (!fileExists) {
+			// Instead of silently deleting, throw a specific error
+			throw new Error("TASK_FILES_MISSING")
+		}
+
+		const apiConversationHistory = JSON.parse(await fs.readFile(apiConversationHistoryFilePath, "utf8"))
+		return {
+			historyItem,
+			taskDirPath,
+			apiConversationHistoryFilePath,
+			uiMessagesFilePath,
+			apiConversationHistory,
 		}
-		// if we tried to get a task that doesn't exist, remove it from state
-		// FIXME: this seems to happen sometimes when the json file doesnt save to disk for some reason
-		await this.deleteTaskFromState(id)
-		throw new Error("Task not found")
 	}
 
 	async showTaskWithId(id: string) {
 		if (id !== this.getCurrentCline()?.taskId) {
-			// Non-current task.
-			const { historyItem } = await this.getTaskWithId(id)
-			await this.initClineWithHistoryItem(historyItem) // Clears existing task.
+			try {
+				const { historyItem } = await this.getTaskWithId(id)
+				await this.initClineWithHistoryItem(historyItem)
+			} catch (error) {
+				if (error.message === "TASK_FILES_MISSING") {
+					const response = await vscode.window.showWarningMessage(
+						"This task's files are missing. Would you like to remove it from the task list?",
+						"Remove",
+						"Keep",
+					)
+
+					if (response === "Remove") {
+						await this.deleteTaskFromState(id)
+						await this.postStateToWebview()
+					}
+					return
+				}
+				throw error
+			}
 		}
 
 		await this.postMessageToWebview({ type: "action", action: "chatButtonClicked" })
@@ -2702,4 +2720,30 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
 
 		return properties
 	}
+
+	async validateTaskHistory() {
+		const history = ((await this.getGlobalState("taskHistory")) as HistoryItem[] | undefined) || []
+		const validTasks: HistoryItem[] = []
+
+		for (const item of history) {
+			const taskDirPath = path.join(this.contextProxy.globalStorageUri.fsPath, "tasks", item.id)
+			const apiConversationHistoryFilePath = path.join(taskDirPath, GlobalFileNames.apiConversationHistory)
+
+			if (await fileExistsAtPath(apiConversationHistoryFilePath)) {
+				validTasks.push(item)
+			}
+		}
+
+		if (validTasks.length !== history.length) {
+			await this.updateGlobalState("taskHistory", validTasks)
+			await this.postStateToWebview()
+
+			const removedCount = history.length - validTasks.length
+			if (removedCount > 0) {
+				await vscode.window.showInformationMessage(
+					`Cleaned up ${removedCount} task(s) with missing files from history.`,
+				)
+			}
+		}
+	}
 }

+ 72 - 0
src/core/webview/__tests__/ClineProvider.test.ts

@@ -54,6 +54,78 @@ jest.mock("../../contextProxy", () => {
 	}
 })
 
+describe("validateTaskHistory", () => {
+	let provider: ClineProvider
+	let mockContext: vscode.ExtensionContext
+	let mockOutputChannel: vscode.OutputChannel
+	let mockUpdate: jest.Mock
+
+	beforeEach(() => {
+		// Reset mocks
+		jest.clearAllMocks()
+
+		mockUpdate = jest.fn()
+
+		// Setup basic mocks
+		mockContext = {
+			globalState: {
+				get: jest.fn(),
+				update: mockUpdate,
+				keys: jest.fn().mockReturnValue([]),
+			},
+			secrets: { get: jest.fn(), store: jest.fn(), delete: jest.fn() },
+			extensionUri: {} as vscode.Uri,
+			globalStorageUri: { fsPath: "/test/path" },
+			extension: { packageJSON: { version: "1.0.0" } },
+		} as unknown as vscode.ExtensionContext
+
+		mockOutputChannel = { appendLine: jest.fn() } as unknown as vscode.OutputChannel
+		provider = new ClineProvider(mockContext, mockOutputChannel)
+	})
+
+	test("should remove tasks with missing files", async () => {
+		// Mock the global state with some test data
+		const mockHistory = [
+			{ id: "task1", ts: Date.now() },
+			{ id: "task2", ts: Date.now() },
+		]
+
+		// Setup mocks
+		jest.spyOn(mockContext.globalState, "get").mockReturnValue(mockHistory)
+
+		// Mock fileExistsAtPath to only return true for task1
+		const mockFs = require("../../../utils/fs")
+		mockFs.fileExistsAtPath = jest.fn().mockImplementation((path) => Promise.resolve(path.includes("task1")))
+
+		// Call validateTaskHistory
+		await provider.validateTaskHistory()
+
+		// Verify the results
+		const expectedHistory = [expect.objectContaining({ id: "task1" })]
+
+		expect(mockUpdate).toHaveBeenCalledWith("taskHistory", expect.arrayContaining(expectedHistory))
+		expect(mockUpdate.mock.calls[0][1].length).toBe(1)
+	})
+
+	test("should handle empty history", async () => {
+		// Mock empty history
+		jest.spyOn(mockContext.globalState, "get").mockReturnValue([])
+
+		await provider.validateTaskHistory()
+
+		expect(mockUpdate).toHaveBeenCalledWith("taskHistory", [])
+	})
+
+	test("should handle null history", async () => {
+		// Mock null history
+		jest.spyOn(mockContext.globalState, "get").mockReturnValue(null)
+
+		await provider.validateTaskHistory()
+
+		expect(mockUpdate).toHaveBeenCalledWith("taskHistory", [])
+	})
+})
+
 // Mock dependencies
 jest.mock("vscode")
 jest.mock("delay")

+ 5 - 0
src/extension.ts

@@ -59,6 +59,11 @@ export function activate(context: vscode.ExtensionContext) {
 	const provider = new ClineProvider(context, outputChannel)
 	telemetryService.setProvider(provider)
 
+	// Validate task history on extension activation
+	provider.validateTaskHistory().catch((error) => {
+		outputChannel.appendLine(`Failed to validate task history: ${error}`)
+	})
+
 	context.subscriptions.push(
 		vscode.window.registerWebviewViewProvider(ClineProvider.sideBarId, provider, {
 			webviewOptions: { retainContextWhenHidden: true },