diff --git a/.changeset/cli-continue-reliability.md b/.changeset/cli-continue-reliability.md new file mode 100644 index 00000000000..1c2f4086752 --- /dev/null +++ b/.changeset/cli-continue-reliability.md @@ -0,0 +1,5 @@ +--- +"@kilocode/cli": patch +--- + +Improve --continue flag reliability by replacing fixed 2-second timeout with Promise-based response handling diff --git a/cli/src/cli.ts b/cli/src/cli.ts index 0a855e080f0..0a1120df5a0 100644 --- a/cli/src/cli.ts +++ b/cli/src/cli.ts @@ -15,8 +15,8 @@ import { requestRouterModelsAtom } from "./state/atoms/actions.js" import { loadHistoryAtom } from "./state/atoms/history.js" import { addPendingRequestAtom, + removePendingRequestAtom, TaskHistoryData, - taskHistoryDataAtom, updateTaskHistoryFiltersAtom, } from "./state/atoms/taskHistory.js" import { sendWebviewMessageAtom } from "./state/atoms/actions.js" @@ -649,24 +649,37 @@ export class CLI { favoritesOnly: false, }) - // Send task history request to extension - await this.store.set(sendWebviewMessageAtom, { - type: "taskHistoryRequest", - payload: { - requestId: Date.now().toString(), - workspace: "current", - sort: "newest", - favoritesOnly: false, - pageIndex: 0, - }, + // Create a unique request ID for tracking the response + const requestId = `${Date.now()}-${Math.random()}` + const TASK_HISTORY_TIMEOUT_MS = 5000 + + // Fetch task history with Promise-based response handling + const taskHistoryData = await new Promise((resolve, reject) => { + // Set up timeout + const timeout = setTimeout(() => { + this.store!.set(removePendingRequestAtom, requestId) + reject(new Error(`Task history request timed out after ${TASK_HISTORY_TIMEOUT_MS}ms`)) + }, TASK_HISTORY_TIMEOUT_MS) + + // Register the pending request - it will be resolved when the response arrives + this.store!.set(addPendingRequestAtom, { requestId, resolve, reject, timeout }) + + // Send task history request to extension + this.store!.set(sendWebviewMessageAtom, { + type: "taskHistoryRequest", + payload: { + requestId, + workspace: "current", + sort: "newest", + favoritesOnly: false, + pageIndex: 0, + }, + }).catch((err) => { + this.store!.set(removePendingRequestAtom, requestId) + reject(err) + }) }) - // Wait for the data to arrive (the response will update taskHistoryDataAtom through effects) - await new Promise((resolve) => setTimeout(resolve, 2000)) - - // Get the task history data - const taskHistoryData = this.store.get(taskHistoryDataAtom) - if (!taskHistoryData || !taskHistoryData.historyItems || taskHistoryData.historyItems.length === 0) { logs.warn("No previous tasks found for workspace", "CLI", { workspace }) console.error("\nNo previous tasks found for this workspace. Please start a new conversation.\n") @@ -696,7 +709,12 @@ export class CLI { logs.info("Task resume initiated", "CLI", { taskId: lastTask.id, task: lastTask.task }) } catch (error) { logs.error("Failed to resume conversation", "CLI", { error, workspace }) - console.error("\nFailed to resume conversation. Please try starting a new conversation.\n") + const errorMessage = error instanceof Error ? error.message : String(error) + if (errorMessage.includes("timed out")) { + console.error("\nFailed to fetch task history (request timed out). Please try again.\n") + } else { + console.error("\nFailed to resume conversation. Please try starting a new conversation.\n") + } process.exit(1) } } diff --git a/cli/src/state/atoms/__tests__/taskHistory.test.ts b/cli/src/state/atoms/__tests__/taskHistory.test.ts new file mode 100644 index 00000000000..dd965936682 --- /dev/null +++ b/cli/src/state/atoms/__tests__/taskHistory.test.ts @@ -0,0 +1,227 @@ +import { describe, it, expect, beforeEach, vi, afterEach } from "vitest" +import { createStore } from "jotai" +import { + taskHistoryPendingRequestsAtom, + addPendingRequestAtom, + removePendingRequestAtom, + resolveTaskHistoryRequestAtom, + type TaskHistoryData, +} from "../taskHistory.js" +import type { HistoryItem } from "@roo-code/types" + +/** + * Creates a minimal mock HistoryItem for testing + */ +function createMockHistoryItem(overrides: Partial = {}): HistoryItem { + return { + id: "task-1", + number: 1, + ts: Date.now(), + task: "Test task", + tokensIn: 100, + tokensOut: 200, + totalCost: 0.01, + ...overrides, + } +} + +describe("taskHistory atoms", () => { + let store: ReturnType + + beforeEach(() => { + store = createStore() + vi.useFakeTimers() + }) + + afterEach(() => { + vi.useRealTimers() + }) + + describe("addPendingRequestAtom", () => { + it("should add a pending request to the map", () => { + const resolve = vi.fn() + const reject = vi.fn() + const timeout = setTimeout(() => {}, 5000) + + store.set(addPendingRequestAtom, { + requestId: "test-123", + resolve, + reject, + timeout, + }) + + const pendingRequests = store.get(taskHistoryPendingRequestsAtom) + expect(pendingRequests.size).toBe(1) + expect(pendingRequests.has("test-123")).toBe(true) + + clearTimeout(timeout) + }) + }) + + describe("removePendingRequestAtom", () => { + it("should remove a pending request and clear its timeout", () => { + const resolve = vi.fn() + const reject = vi.fn() + const timeoutCallback = vi.fn() + const timeout = setTimeout(timeoutCallback, 5000) + + // Add the request first + store.set(addPendingRequestAtom, { + requestId: "test-456", + resolve, + reject, + timeout, + }) + + // Remove it + store.set(removePendingRequestAtom, "test-456") + + const pendingRequests = store.get(taskHistoryPendingRequestsAtom) + expect(pendingRequests.size).toBe(0) + + // Verify timeout was cleared + vi.advanceTimersByTime(6000) + expect(timeoutCallback).not.toHaveBeenCalled() + }) + + it("should do nothing if request ID does not exist", () => { + store.set(removePendingRequestAtom, "nonexistent") + const pendingRequests = store.get(taskHistoryPendingRequestsAtom) + expect(pendingRequests.size).toBe(0) + }) + }) + + describe("resolveTaskHistoryRequestAtom", () => { + it("should resolve a pending request with data", () => { + const resolve = vi.fn() + const reject = vi.fn() + const timeout = setTimeout(() => {}, 5000) + + // Add the request + store.set(addPendingRequestAtom, { + requestId: "test-789", + resolve, + reject, + timeout, + }) + + const mockData: TaskHistoryData = { + historyItems: [createMockHistoryItem()], + pageIndex: 0, + pageCount: 1, + } + + // Resolve it + store.set(resolveTaskHistoryRequestAtom, { + requestId: "test-789", + data: mockData, + }) + + // Verify resolve was called with data + expect(resolve).toHaveBeenCalledWith(mockData) + expect(reject).not.toHaveBeenCalled() + + // Verify request was removed + const pendingRequests = store.get(taskHistoryPendingRequestsAtom) + expect(pendingRequests.size).toBe(0) + }) + + it("should reject a pending request with error", () => { + const resolve = vi.fn() + const reject = vi.fn() + const timeout = setTimeout(() => {}, 5000) + + // Add the request + store.set(addPendingRequestAtom, { + requestId: "test-error", + resolve, + reject, + timeout, + }) + + // Resolve with error + store.set(resolveTaskHistoryRequestAtom, { + requestId: "test-error", + error: "Something went wrong", + }) + + // Verify reject was called + expect(reject).toHaveBeenCalledWith(expect.any(Error)) + expect(reject.mock.calls[0][0].message).toBe("Something went wrong") + expect(resolve).not.toHaveBeenCalled() + + // Verify request was removed + const pendingRequests = store.get(taskHistoryPendingRequestsAtom) + expect(pendingRequests.size).toBe(0) + }) + + it("should do nothing if request ID does not exist", () => { + const mockData: TaskHistoryData = { + historyItems: [], + pageIndex: 0, + pageCount: 0, + } + + // Should not throw + store.set(resolveTaskHistoryRequestAtom, { + requestId: "nonexistent", + data: mockData, + }) + }) + }) + + describe("Promise-based task history flow", () => { + it("should resolve promise when response arrives before timeout", async () => { + const TIMEOUT_MS = 5000 + const requestId = "flow-test-1" + + // Simulate the flow used in CLI.resumeConversation + const resultPromise = new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + store.set(removePendingRequestAtom, requestId) + reject(new Error(`Request timed out after ${TIMEOUT_MS}ms`)) + }, TIMEOUT_MS) + + store.set(addPendingRequestAtom, { requestId, resolve, reject, timeout }) + }) + + // Simulate response arriving + const mockData: TaskHistoryData = { + historyItems: [createMockHistoryItem()], + pageIndex: 0, + pageCount: 1, + } + + store.set(resolveTaskHistoryRequestAtom, { requestId, data: mockData }) + + // Should resolve with data + const result = await resultPromise + expect(result).toEqual(mockData) + }) + + it("should reject promise when timeout occurs", async () => { + const TIMEOUT_MS = 5000 + const requestId = "flow-test-2" + + // Simulate the flow used in CLI.resumeConversation + const resultPromise = new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + store.set(removePendingRequestAtom, requestId) + reject(new Error(`Request timed out after ${TIMEOUT_MS}ms`)) + }, TIMEOUT_MS) + + store.set(addPendingRequestAtom, { requestId, resolve, reject, timeout }) + }) + + // Advance time past timeout + vi.advanceTimersByTime(TIMEOUT_MS + 100) + + // Should reject with timeout error + await expect(resultPromise).rejects.toThrow(`Request timed out after ${TIMEOUT_MS}ms`) + + // Verify request was removed + const pendingRequests = store.get(taskHistoryPendingRequestsAtom) + expect(pendingRequests.size).toBe(0) + }) + }) +})