From b8c5ddaa20bf1b134972cac769203182eab2d2c7 Mon Sep 17 00:00:00 2001 From: Will Li Date: Wed, 23 Jul 2025 03:17:54 -0700 Subject: [PATCH] clean code more --- src/core/checkpoints/__tests__/utils.test.ts | 107 ------------------- src/core/checkpoints/utils.ts | 51 --------- src/core/task/Task.ts | 32 ++---- src/core/webview/checkpointRestoreHandler.ts | 3 +- src/core/webview/webviewMessageHandler.ts | 1 - 5 files changed, 11 insertions(+), 183 deletions(-) delete mode 100644 src/core/checkpoints/__tests__/utils.test.ts delete mode 100644 src/core/checkpoints/utils.ts diff --git a/src/core/checkpoints/__tests__/utils.test.ts b/src/core/checkpoints/__tests__/utils.test.ts deleted file mode 100644 index eb4fccb96c..0000000000 --- a/src/core/checkpoints/__tests__/utils.test.ts +++ /dev/null @@ -1,107 +0,0 @@ -import { describe, it, expect } from "vitest" -import { isValidCheckpoint, hasValidCheckpoint, extractCheckpoint, type ValidCheckpoint } from "../utils" - -describe("checkpoint utils", () => { - describe("isValidCheckpoint", () => { - it("should return true for valid checkpoint", () => { - const checkpoint: ValidCheckpoint = { hash: "abc123" } - expect(isValidCheckpoint(checkpoint)).toBe(true) - }) - - it("should return false for null or undefined", () => { - expect(isValidCheckpoint(null)).toBe(false) - expect(isValidCheckpoint(undefined)).toBe(false) - }) - - it("should return false for non-object types", () => { - expect(isValidCheckpoint("string")).toBe(false) - expect(isValidCheckpoint(123)).toBe(false) - expect(isValidCheckpoint(true)).toBe(false) - expect(isValidCheckpoint([])).toBe(false) - }) - - it("should return false for objects without hash property", () => { - expect(isValidCheckpoint({})).toBe(false) - expect(isValidCheckpoint({ other: "property" })).toBe(false) - }) - - it("should return false for objects with non-string hash", () => { - expect(isValidCheckpoint({ hash: 123 })).toBe(false) - expect(isValidCheckpoint({ hash: null })).toBe(false) - expect(isValidCheckpoint({ hash: undefined })).toBe(false) - expect(isValidCheckpoint({ hash: {} })).toBe(false) - expect(isValidCheckpoint({ hash: [] })).toBe(false) - }) - - it("should return false for empty hash string", () => { - expect(isValidCheckpoint({ hash: "" })).toBe(false) - }) - - it("should return true for valid hash strings", () => { - expect(isValidCheckpoint({ hash: "a" })).toBe(true) - expect(isValidCheckpoint({ hash: "abc123def456" })).toBe(true) - expect(isValidCheckpoint({ hash: "commit-hash-with-dashes" })).toBe(true) - }) - }) - - describe("hasValidCheckpoint", () => { - it("should return true for message with valid checkpoint", () => { - const message = { checkpoint: { hash: "abc123" } } - expect(hasValidCheckpoint(message)).toBe(true) - }) - - it("should return false for null or undefined message", () => { - expect(hasValidCheckpoint(null)).toBe(false) - expect(hasValidCheckpoint(undefined)).toBe(false) - }) - - it("should return false for non-object message", () => { - expect(hasValidCheckpoint("string")).toBe(false) - expect(hasValidCheckpoint(123)).toBe(false) - }) - - it("should return false for message without checkpoint property", () => { - expect(hasValidCheckpoint({})).toBe(false) - expect(hasValidCheckpoint({ text: "message" })).toBe(false) - }) - - it("should return false for message with invalid checkpoint", () => { - expect(hasValidCheckpoint({ checkpoint: null })).toBe(false) - expect(hasValidCheckpoint({ checkpoint: "invalid" })).toBe(false) - expect(hasValidCheckpoint({ checkpoint: {} })).toBe(false) - expect(hasValidCheckpoint({ checkpoint: { hash: "" } })).toBe(false) - expect(hasValidCheckpoint({ checkpoint: { hash: 123 } })).toBe(false) - }) - - it("should work as type guard", () => { - const message: unknown = { checkpoint: { hash: "abc123" }, other: "data" } - if (hasValidCheckpoint(message)) { - // TypeScript should know message has checkpoint property - expect(message.checkpoint.hash).toBe("abc123") - } - }) - }) - - describe("extractCheckpoint", () => { - it("should extract valid checkpoint from message", () => { - const message = { checkpoint: { hash: "abc123" } } - const result = extractCheckpoint(message) - expect(result).toEqual({ hash: "abc123" }) - }) - - it("should return undefined for message without valid checkpoint", () => { - expect(extractCheckpoint({})).toBeUndefined() - expect(extractCheckpoint({ checkpoint: null })).toBeUndefined() - expect(extractCheckpoint({ checkpoint: { hash: "" } })).toBeUndefined() - expect(extractCheckpoint(null)).toBeUndefined() - expect(extractCheckpoint(undefined)).toBeUndefined() - }) - - it("should return the same checkpoint object reference", () => { - const checkpoint = { hash: "abc123" } - const message = { checkpoint } - const result = extractCheckpoint(message) - expect(result).toBe(checkpoint) - }) - }) -}) diff --git a/src/core/checkpoints/utils.ts b/src/core/checkpoints/utils.ts deleted file mode 100644 index 0b197d221d..0000000000 --- a/src/core/checkpoints/utils.ts +++ /dev/null @@ -1,51 +0,0 @@ -/** - * Checkpoint-related utilities and type definitions - */ - -/** - * Represents a valid checkpoint with required properties - */ -export interface ValidCheckpoint { - hash: string -} - -/** - * Type guard to check if an object is a valid checkpoint - * @param checkpoint - The object to check - * @returns True if the checkpoint is valid, false otherwise - */ -export function isValidCheckpoint(checkpoint: unknown): checkpoint is ValidCheckpoint { - return ( - checkpoint !== null && - checkpoint !== undefined && - typeof checkpoint === "object" && - "hash" in checkpoint && - typeof (checkpoint as any).hash === "string" && - (checkpoint as any).hash.length > 0 // Ensure hash is not empty - ) -} - -/** - * Validates if a message has a valid checkpoint for restoration - * @param message - The message object to check - * @returns True if the message contains a valid checkpoint, false otherwise - */ -export function hasValidCheckpoint(message: unknown): message is { checkpoint: ValidCheckpoint } { - if (!message || typeof message !== "object" || !("checkpoint" in message)) { - return false - } - - return isValidCheckpoint((message as any).checkpoint) -} - -/** - * Extracts a valid checkpoint from a message if it exists - * @param message - The message object to extract from - * @returns The valid checkpoint or undefined - */ -export function extractCheckpoint(message: unknown): ValidCheckpoint | undefined { - if (hasValidCheckpoint(message)) { - return message.checkpoint - } - return undefined -} diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 0b4ca93d7c..f3a2318290 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -716,27 +716,15 @@ export class Task extends EventEmitter { this.lastMessageTs = sayTs } - if (type === "user_feedback") { - await this.addToClineMessages({ - ts: sayTs, - type: "say", - say: type, - text, - images, - checkpoint, - contextCondense, - }) - } else { - await this.addToClineMessages({ - ts: sayTs, - type: "say", - say: type, - text, - images, - checkpoint, - contextCondense, - }) - } + await this.addToClineMessages({ + ts: sayTs, + type: "say", + say: type, + text, + images, + checkpoint, + contextCondense, + }) } } @@ -763,7 +751,6 @@ export class Task extends EventEmitter { this.apiConversationHistory = [] await this.providerRef.deref()?.postStateToWebview() - // Checkpoint will be saved in handleWebviewAskResponse before this message is created await this.say("text", task, images) this.isInitialized = true @@ -864,6 +851,7 @@ export class Task extends EventEmitter { responseText = text responseImages = images } + // Make sure that the api conversation history can be resumed by the API, // even if it goes out of sync with cline messages. let existingApiConversationHistory: ApiMessage[] = await this.getSavedApiConversationHistory() diff --git a/src/core/webview/checkpointRestoreHandler.ts b/src/core/webview/checkpointRestoreHandler.ts index 3aa11d3c23..ac86f0c4a0 100644 --- a/src/core/webview/checkpointRestoreHandler.ts +++ b/src/core/webview/checkpointRestoreHandler.ts @@ -4,14 +4,13 @@ import { saveTaskMessages } from "../task-persistence" import * as vscode from "vscode" import pWaitFor from "p-wait-for" import { t } from "../../i18n" -import { ValidCheckpoint } from "../checkpoints/utils" export interface CheckpointRestoreConfig { provider: ClineProvider currentCline: Task messageTs: number messageIndex: number - checkpoint: ValidCheckpoint + checkpoint: { hash: string } operation: "delete" | "edit" editData?: { editedContent: string diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 346c469ab2..5c68af6419 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -20,7 +20,6 @@ import { saveTaskMessages } from "../task-persistence" import { ClineProvider } from "./ClineProvider" import { handleCheckpointRestoreOperation } from "./checkpointRestoreHandler" -import { ValidCheckpoint, hasValidCheckpoint } from "../checkpoints/utils" import { changeLanguage, t } from "../../i18n" import { Package } from "../../shared/package" import { RouterName, toRouterName, ModelRecord } from "../../shared/api"