From e84ea747b4dbf4c04fb0ec067d68e9157317a126 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Sat, 6 Dec 2025 00:59:44 +0000 Subject: [PATCH 1/2] fix: checkpoint warning now shows configured timeout instead of hardcoded 5 seconds --- .../checkpoints/__tests__/checkpoint.test.ts | 21 +++++++++++-------- src/core/checkpoints/index.ts | 2 +- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/core/checkpoints/__tests__/checkpoint.test.ts b/src/core/checkpoints/__tests__/checkpoint.test.ts index 299ff823b8..1b13706d47 100644 --- a/src/core/checkpoints/__tests__/checkpoint.test.ts +++ b/src/core/checkpoints/__tests__/checkpoint.test.ts @@ -475,7 +475,9 @@ describe("Checkpoint functionality", () => { const provider = mockTask.providerRef.deref() provider?.postMessageToWebview({ type: "checkpointInitWarning", - checkpointWarning: i18nModule.t("common:errors.wait_checkpoint_long_time", { timeout: 5 }), + checkpointWarning: i18nModule.t("common:errors.wait_checkpoint_long_time", { + timeout: mockTask.checkpointTimeout, + }), }) } @@ -486,11 +488,11 @@ describe("Checkpoint functionality", () => { expect(simulateConditionCheck(4000)).toBe(false) expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() - // Test: At 5 seconds, warning should be sent + // Test: At 5 seconds, warning should be sent with configured timeout (15 seconds) expect(simulateConditionCheck(5000)).toBe(false) expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({ type: "checkpointInitWarning", - checkpointWarning: "Checkpoint initialization is taking longer than 5 seconds...", + checkpointWarning: "Checkpoint initialization is taking longer than 15 seconds...", }) // Test: At 6 seconds, warning should not be sent again (warningShown is true) @@ -558,10 +560,11 @@ describe("Checkpoint functionality", () => { }) it("should use WARNING_THRESHOLD_MS constant of 5000ms", () => { - // Verify the warning threshold is 5 seconds by checking the implementation + // Verify the warning threshold is 5 seconds - this determines WHEN to show the warning + // but the timeout value shown to user should be the configured checkpointTimeout const WARNING_THRESHOLD_MS = 5000 expect(WARNING_THRESHOLD_MS).toBe(5000) - expect(WARNING_THRESHOLD_MS / 1000).toBe(5) // Used in the i18n call + expect(WARNING_THRESHOLD_MS / 1000).toBe(5) // Used to check elapsed time }) it("should convert checkpointTimeout to milliseconds", () => { @@ -581,11 +584,11 @@ describe("Checkpoint functionality", () => { const i18nModule = await import("../../../i18n") vi.clearAllMocks() - // Test warning message i18n key - const warningMessage = i18nModule.t("common:errors.wait_checkpoint_long_time", { timeout: 5 }) - expect(warningMessage).toBe("Checkpoint initialization is taking longer than 5 seconds...") + // Test warning message i18n key - should use configured timeout + const warningMessage = i18nModule.t("common:errors.wait_checkpoint_long_time", { timeout: 15 }) + expect(warningMessage).toBe("Checkpoint initialization is taking longer than 15 seconds...") - // Test timeout error message i18n key + // Test timeout error message i18n key - should use configured timeout const errorMessage = i18nModule.t("common:errors.init_checkpoint_fail_long_time", { timeout: 30 }) expect(errorMessage).toBe("Checkpoint initialization failed after 30 seconds") }) diff --git a/src/core/checkpoints/index.ts b/src/core/checkpoints/index.ts index 64a8ad1cfe..0b03be15dd 100644 --- a/src/core/checkpoints/index.ts +++ b/src/core/checkpoints/index.ts @@ -86,7 +86,7 @@ export async function getCheckpointService(task: Task, { interval = 250 }: { int // Show warning if we're past the threshold and haven't shown it yet if (!warningShown && elapsed >= WARNING_THRESHOLD_MS) { warningShown = true - sendCheckpointInitWarn(task, "WAIT_TIMEOUT", WARNING_THRESHOLD_MS / 1000) + sendCheckpointInitWarn(task, "WAIT_TIMEOUT", task.checkpointTimeout) } console.log( From d85a5f2d9e25adfc9d24df7828f645eb99cbc4e9 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Sat, 6 Dec 2025 01:19:35 +0000 Subject: [PATCH 2/2] fix: use configured timeout for warning threshold condition --- .../checkpoints/__tests__/checkpoint.test.ts | 32 +++++++++++-------- src/core/checkpoints/index.ts | 4 +-- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/core/checkpoints/__tests__/checkpoint.test.ts b/src/core/checkpoints/__tests__/checkpoint.test.ts index 1b13706d47..1fd3e3b896 100644 --- a/src/core/checkpoints/__tests__/checkpoint.test.ts +++ b/src/core/checkpoints/__tests__/checkpoint.test.ts @@ -467,9 +467,11 @@ describe("Checkpoint functionality", () => { // Simulate the condition function that runs inside pWaitFor let warningShown = false + const checkpointTimeoutMs = mockTask.checkpointTimeout * 1000 // 15000ms const simulateConditionCheck = (elapsedMs: number) => { // This simulates what happens inside the pWaitFor condition function (lines 85-100) - if (!warningShown && elapsedMs >= 5000) { + // The warning is shown when elapsed time exceeds the configured timeout + if (!warningShown && elapsedMs >= checkpointTimeoutMs) { warningShown = true // This is what the actual code does at line 91-94 const provider = mockTask.providerRef.deref() @@ -484,20 +486,20 @@ describe("Checkpoint functionality", () => { return !!mockTask.checkpointService && !!mockTask.checkpointService.isInitialized } - // Test: At 4 seconds, no warning should be sent - expect(simulateConditionCheck(4000)).toBe(false) + // Test: At 14 seconds (just under configured timeout of 15s), no warning should be sent + expect(simulateConditionCheck(14000)).toBe(false) expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() - // Test: At 5 seconds, warning should be sent with configured timeout (15 seconds) - expect(simulateConditionCheck(5000)).toBe(false) + // Test: At 15 seconds (configured timeout), warning should be sent with configured timeout + expect(simulateConditionCheck(15000)).toBe(false) expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({ type: "checkpointInitWarning", checkpointWarning: "Checkpoint initialization is taking longer than 15 seconds...", }) - // Test: At 6 seconds, warning should not be sent again (warningShown is true) + // Test: At 16 seconds, warning should not be sent again (warningShown is true) vi.clearAllMocks() - expect(simulateConditionCheck(6000)).toBe(false) + expect(simulateConditionCheck(16000)).toBe(false) expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() }) @@ -559,12 +561,16 @@ describe("Checkpoint functionality", () => { }) }) - it("should use WARNING_THRESHOLD_MS constant of 5000ms", () => { - // Verify the warning threshold is 5 seconds - this determines WHEN to show the warning - // but the timeout value shown to user should be the configured checkpointTimeout - const WARNING_THRESHOLD_MS = 5000 - expect(WARNING_THRESHOLD_MS).toBe(5000) - expect(WARNING_THRESHOLD_MS / 1000).toBe(5) // Used to check elapsed time + it("should use configured checkpointTimeout for warning threshold", () => { + // The warning threshold now uses the configured checkpointTimeout value + // This ensures the warning message matches when it actually appears + mockTask.checkpointTimeout = 15 + const checkpointTimeoutMs = mockTask.checkpointTimeout * 1000 + expect(checkpointTimeoutMs).toBe(15000) + + mockTask.checkpointTimeout = 30 + const checkpointTimeoutMs30 = mockTask.checkpointTimeout * 1000 + expect(checkpointTimeoutMs30).toBe(30000) }) it("should convert checkpointTimeout to milliseconds", () => { diff --git a/src/core/checkpoints/index.ts b/src/core/checkpoints/index.ts index 0b03be15dd..6d3761bcc9 100644 --- a/src/core/checkpoints/index.ts +++ b/src/core/checkpoints/index.ts @@ -83,8 +83,8 @@ export async function getCheckpointService(task: Task, { interval = 250 }: { int () => { const elapsed = Date.now() - checkpointInitStartTime - // Show warning if we're past the threshold and haven't shown it yet - if (!warningShown && elapsed >= WARNING_THRESHOLD_MS) { + // Show warning if we're past the configured timeout and haven't shown it yet + if (!warningShown && elapsed >= checkpointTimeoutMs) { warningShown = true sendCheckpointInitWarn(task, "WAIT_TIMEOUT", task.checkpointTimeout) }