-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: implement stable project ID feature (Sprint 1) #6688
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1367,12 +1367,33 @@ export class ClineProvider | |
| apiConversationHistory: Anthropic.MessageParam[] | ||
| }> { | ||
| const history = this.getGlobalState("taskHistory") ?? [] | ||
| const historyItem = history.find((item) => item.id === id) | ||
|
|
||
| // First try to find by task ID | ||
| let historyItem = history.find((item) => item.id === id) | ||
|
|
||
| // If not found by ID and the ID looks like a project ID (UUID format), | ||
| // try to find by project ID | ||
| if (!historyItem && id.match(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i)) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This UUID regex pattern appears twice in this file (lines 1376 and 1376). Consider extracting it to a constant like for reusability and consistency. |
||
| // Get the current workspace root | ||
| const workspaceRoot = this.cwd | ||
| if (workspaceRoot) { | ||
| // Check if this workspace has the given project ID | ||
| const { getProjectId } = await import("../../utils/projectId") | ||
| const currentProjectId = await getProjectId(workspaceRoot) | ||
|
|
||
| if (currentProjectId === id) { | ||
| // Find the most recent task for this project ID | ||
| historyItem = history | ||
| .filter((item) => item.projectId === id) | ||
| .sort((a, b) => (b.ts || 0) - (a.ts || 0))[0] | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (historyItem) { | ||
| const { getTaskDirectoryPath } = await import("../../utils/storage") | ||
| const globalStoragePath = this.contextProxy.globalStorageUri.fsPath | ||
| const taskDirPath = await getTaskDirectoryPath(globalStoragePath, id) | ||
| const taskDirPath = await getTaskDirectoryPath(globalStoragePath, historyItem.id) | ||
| const apiConversationHistoryFilePath = path.join(taskDirPath, GlobalFileNames.apiConversationHistory) | ||
| const uiMessagesFilePath = path.join(taskDirPath, GlobalFileNames.uiMessages) | ||
| const fileExists = await fileExistsAtPath(apiConversationHistoryFilePath) | ||
|
|
@@ -1392,7 +1413,9 @@ export class ClineProvider | |
|
|
||
| // 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) | ||
| if (historyItem) { | ||
| await this.deleteTaskFromState(historyItem.id) | ||
| } | ||
| throw new Error("Task not found") | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" | ||
| import fs from "fs/promises" | ||
| import path from "path" | ||
| import { v4 as uuidv4 } from "uuid" | ||
| import { getProjectId, generateProjectId, hasProjectId } from "../projectId" | ||
| import { fileExistsAtPath } from "../fs" | ||
|
|
||
| // Mock dependencies | ||
| vi.mock("fs/promises") | ||
| vi.mock("../fs") | ||
| vi.mock("uuid") | ||
|
|
||
| describe("projectId", () => { | ||
| const mockWorkspaceRoot = "/test/workspace" | ||
| const mockProjectId = "12345678-1234-1234-1234-123456789012" | ||
| const projectIdPath = path.join(mockWorkspaceRoot, ".rooprojectid") | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks() | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| vi.restoreAllMocks() | ||
| }) | ||
|
|
||
| describe("getProjectId", () => { | ||
| it("should return project ID when file exists and contains valid ID", async () => { | ||
| vi.mocked(fileExistsAtPath).mockResolvedValue(true) | ||
| vi.mocked(fs.readFile).mockResolvedValue(mockProjectId) | ||
|
|
||
| const result = await getProjectId(mockWorkspaceRoot) | ||
|
|
||
| expect(result).toBe(mockProjectId) | ||
| expect(fileExistsAtPath).toHaveBeenCalledWith(projectIdPath) | ||
| expect(fs.readFile).toHaveBeenCalledWith(projectIdPath, "utf8") | ||
| }) | ||
|
|
||
| it("should return null when file does not exist", async () => { | ||
| vi.mocked(fileExistsAtPath).mockResolvedValue(false) | ||
|
|
||
| const result = await getProjectId(mockWorkspaceRoot) | ||
|
|
||
| expect(result).toBeNull() | ||
| expect(fileExistsAtPath).toHaveBeenCalledWith(projectIdPath) | ||
| expect(fs.readFile).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| it("should return null when file is empty", async () => { | ||
| vi.mocked(fileExistsAtPath).mockResolvedValue(true) | ||
| vi.mocked(fs.readFile).mockResolvedValue("") | ||
|
|
||
| const result = await getProjectId(mockWorkspaceRoot) | ||
|
|
||
| expect(result).toBeNull() | ||
| }) | ||
|
|
||
| it("should trim whitespace from project ID", async () => { | ||
| vi.mocked(fileExistsAtPath).mockResolvedValue(true) | ||
| vi.mocked(fs.readFile).mockResolvedValue(` ${mockProjectId} \n`) | ||
|
|
||
| const result = await getProjectId(mockWorkspaceRoot) | ||
|
|
||
| expect(result).toBe(mockProjectId) | ||
| }) | ||
|
|
||
| it("should handle read errors gracefully", async () => { | ||
| vi.mocked(fileExistsAtPath).mockResolvedValue(true) | ||
| vi.mocked(fs.readFile).mockRejectedValue(new Error("Read error")) | ||
|
|
||
| const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) | ||
|
|
||
| const result = await getProjectId(mockWorkspaceRoot) | ||
|
|
||
| expect(result).toBeNull() | ||
| expect(consoleErrorSpy).toHaveBeenCalledWith("Failed to read project ID: Error: Read error") | ||
| }) | ||
| }) | ||
|
|
||
| describe("generateProjectId", () => { | ||
| it("should generate and write a new project ID", async () => { | ||
| vi.mocked(uuidv4).mockReturnValue(mockProjectId as any) | ||
| vi.mocked(fs.writeFile).mockResolvedValue() | ||
|
|
||
| const result = await generateProjectId(mockWorkspaceRoot) | ||
|
|
||
| expect(result).toBe(mockProjectId) | ||
| expect(uuidv4).toHaveBeenCalled() | ||
| expect(fs.writeFile).toHaveBeenCalledWith(projectIdPath, mockProjectId, "utf8") | ||
| }) | ||
|
|
||
| it("should handle write errors", async () => { | ||
| vi.mocked(uuidv4).mockReturnValue(mockProjectId as any) | ||
| vi.mocked(fs.writeFile).mockRejectedValue(new Error("Write error")) | ||
|
|
||
| await expect(generateProjectId(mockWorkspaceRoot)).rejects.toThrow("Write error") | ||
| }) | ||
| }) | ||
|
|
||
| describe("hasProjectId", () => { | ||
| it("should return true when project ID file exists", async () => { | ||
| vi.mocked(fileExistsAtPath).mockResolvedValue(true) | ||
|
|
||
| const result = await hasProjectId(mockWorkspaceRoot) | ||
|
|
||
| expect(result).toBe(true) | ||
| expect(fileExistsAtPath).toHaveBeenCalledWith(projectIdPath) | ||
| }) | ||
|
|
||
| it("should return false when project ID file does not exist", async () => { | ||
| vi.mocked(fileExistsAtPath).mockResolvedValue(false) | ||
|
|
||
| const result = await hasProjectId(mockWorkspaceRoot) | ||
|
|
||
| expect(result).toBe(false) | ||
| expect(fileExistsAtPath).toHaveBeenCalledWith(projectIdPath) | ||
| }) | ||
| }) | ||
| }) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding test cases for:
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| import * as fs from "fs/promises" | ||
| import * as path from "path" | ||
| import { v4 as uuidv4 } from "uuid" | ||
| import { fileExistsAtPath } from "./fs" | ||
|
|
||
| const PROJECT_ID_FILENAME = ".rooprojectid" | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The file format and purpose could be documented in a README or contributing guide. Also, consider adding a comment here indicating that migration logic will be added in Sprint 2. |
||
|
|
||
| /** | ||
| * Gets the project ID from the .rooprojectid file in the workspace root. | ||
| * Returns null if the file doesn't exist or can't be read. | ||
| * | ||
| * @param workspaceRoot The root directory of the workspace | ||
| * @returns The project ID string or null | ||
| */ | ||
| export async function getProjectId(workspaceRoot: string): Promise<string | null> { | ||
| try { | ||
| const projectIdPath = path.join(workspaceRoot, PROJECT_ID_FILENAME) | ||
| const exists = await fileExistsAtPath(projectIdPath) | ||
|
|
||
| if (!exists) { | ||
| return null | ||
| } | ||
|
|
||
| const content = await fs.readFile(projectIdPath, "utf8") | ||
| const projectId = content.trim() | ||
|
|
||
| // Validate that it's not empty | ||
| if (!projectId) { | ||
| return null | ||
| } | ||
|
|
||
| return projectId | ||
| } catch (error) { | ||
| // If we can't read the file for any reason, return null | ||
| console.error(`Failed to read project ID: ${error}`) | ||
| return null | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Generates a new project ID and writes it to the .rooprojectid file. | ||
| * | ||
| * @param workspaceRoot The root directory of the workspace | ||
| * @returns The generated project ID | ||
| */ | ||
| export async function generateProjectId(workspaceRoot: string): Promise<string> { | ||
| const projectId = uuidv4() | ||
| const projectIdPath = path.join(workspaceRoot, PROJECT_ID_FILENAME) | ||
|
|
||
| await fs.writeFile(projectIdPath, projectId, "utf8") | ||
|
|
||
| return projectId | ||
| } | ||
|
|
||
| /** | ||
| * Checks if a project has a project ID file. | ||
| * | ||
| * @param workspaceRoot The root directory of the workspace | ||
| * @returns True if the project has a .rooprojectid file | ||
| */ | ||
| export async function hasProjectId(workspaceRoot: string): Promise<boolean> { | ||
| const projectIdPath = path.join(workspaceRoot, PROJECT_ID_FILENAME) | ||
| return await fileExistsAtPath(projectIdPath) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding telemetry or metrics here to track how often project ID generation fails. This could help identify common issues users face.