mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
fix: resolve task persistence issue for Desktop directory on macOS
- Add arePathsEqual utility function for proper path comparison - Update useTaskSearch hook to use arePathsEqual for workspace filtering - Add comprehensive tests for Desktop directory path handling - Fix path normalization to handle mixed separators and edge cases This ensures tasks are properly filtered and persist when switching panels, especially when the workspace is the Desktop directory. Fixes #8471
This commit is contained in:
parent
8622d93f59
commit
b4659fbfb2
4 changed files with 383 additions and 1 deletions
|
|
@ -1,4 +1,6 @@
|
|||
import { renderHook, act } from "@/utils/test-utils"
|
||||
import * as path from "path"
|
||||
import * as os from "os"
|
||||
|
||||
import type { HistoryItem } from "@roo-code/types"
|
||||
|
||||
|
|
@ -284,4 +286,156 @@ describe("useTaskSearch", () => {
|
|||
// When not searching, it should fall back to newest
|
||||
expect(result.current.sortOption).toBe("mostRelevant")
|
||||
})
|
||||
|
||||
describe("Desktop directory handling", () => {
|
||||
it("should correctly filter tasks when workspace is Desktop on macOS", () => {
|
||||
const desktopPath = path.join(os.homedir(), "Desktop")
|
||||
const desktopPathWithSlash = path.join(os.homedir(), "Desktop/")
|
||||
|
||||
const desktopTaskHistory: HistoryItem[] = [
|
||||
{
|
||||
id: "desktop-task-1",
|
||||
number: 1,
|
||||
task: "Task created in Desktop",
|
||||
ts: new Date("2022-02-16T12:00:00").getTime(),
|
||||
tokensIn: 100,
|
||||
tokensOut: 50,
|
||||
totalCost: 0.01,
|
||||
workspace: desktopPath,
|
||||
},
|
||||
{
|
||||
id: "desktop-task-2",
|
||||
number: 2,
|
||||
task: "Another Desktop task",
|
||||
ts: new Date("2022-02-17T12:00:00").getTime(),
|
||||
tokensIn: 200,
|
||||
tokensOut: 100,
|
||||
totalCost: 0.02,
|
||||
workspace: desktopPathWithSlash, // With trailing slash
|
||||
},
|
||||
{
|
||||
id: "other-task",
|
||||
number: 3,
|
||||
task: "Task from different workspace",
|
||||
ts: new Date("2022-02-15T12:00:00").getTime(),
|
||||
tokensIn: 150,
|
||||
tokensOut: 75,
|
||||
totalCost: 0.05,
|
||||
workspace: "/workspace/project1",
|
||||
},
|
||||
]
|
||||
|
||||
mockUseExtensionState.mockReturnValue({
|
||||
taskHistory: desktopTaskHistory,
|
||||
cwd: desktopPath,
|
||||
} as any)
|
||||
|
||||
const { result } = renderHook(() => useTaskSearch())
|
||||
|
||||
// Should show both Desktop tasks despite different path formats
|
||||
expect(result.current.tasks).toHaveLength(2)
|
||||
expect(result.current.tasks[0].id).toBe("desktop-task-2")
|
||||
expect(result.current.tasks[1].id).toBe("desktop-task-1")
|
||||
})
|
||||
|
||||
it("should handle Desktop path variations on Windows", () => {
|
||||
// Mock Windows platform
|
||||
const originalPlatform = process.platform
|
||||
Object.defineProperty(process, "platform", {
|
||||
value: "win32",
|
||||
configurable: true,
|
||||
})
|
||||
|
||||
const desktopPath = "C:\\Users\\testuser\\Desktop"
|
||||
const desktopPathMixed = "C:/Users/testuser/Desktop"
|
||||
const desktopPathLowerCase = "c:\\users\\testuser\\desktop"
|
||||
|
||||
const windowsDesktopTaskHistory: HistoryItem[] = [
|
||||
{
|
||||
id: "win-desktop-task-1",
|
||||
number: 1,
|
||||
task: "Windows Desktop task 1",
|
||||
ts: new Date("2022-02-16T12:00:00").getTime(),
|
||||
tokensIn: 100,
|
||||
tokensOut: 50,
|
||||
totalCost: 0.01,
|
||||
workspace: desktopPath,
|
||||
},
|
||||
{
|
||||
id: "win-desktop-task-2",
|
||||
number: 2,
|
||||
task: "Windows Desktop task 2",
|
||||
ts: new Date("2022-02-17T12:00:00").getTime(),
|
||||
tokensIn: 200,
|
||||
tokensOut: 100,
|
||||
totalCost: 0.02,
|
||||
workspace: desktopPathMixed, // Mixed separators
|
||||
},
|
||||
{
|
||||
id: "win-desktop-task-3",
|
||||
number: 3,
|
||||
task: "Windows Desktop task 3",
|
||||
ts: new Date("2022-02-18T12:00:00").getTime(),
|
||||
tokensIn: 150,
|
||||
tokensOut: 75,
|
||||
totalCost: 0.03,
|
||||
workspace: desktopPathLowerCase, // Different case
|
||||
},
|
||||
]
|
||||
|
||||
mockUseExtensionState.mockReturnValue({
|
||||
taskHistory: windowsDesktopTaskHistory,
|
||||
cwd: desktopPath,
|
||||
} as any)
|
||||
|
||||
const { result } = renderHook(() => useTaskSearch())
|
||||
|
||||
// Should show all Desktop tasks despite path variations
|
||||
expect(result.current.tasks).toHaveLength(3)
|
||||
expect(result.current.tasks[0].id).toBe("win-desktop-task-3")
|
||||
expect(result.current.tasks[1].id).toBe("win-desktop-task-2")
|
||||
expect(result.current.tasks[2].id).toBe("win-desktop-task-1")
|
||||
|
||||
// Restore original platform
|
||||
Object.defineProperty(process, "platform", {
|
||||
value: originalPlatform,
|
||||
configurable: true,
|
||||
})
|
||||
})
|
||||
|
||||
it("should not lose tasks when switching between panels with Desktop workspace", () => {
|
||||
const desktopPath = path.join(os.homedir(), "Desktop")
|
||||
|
||||
const desktopTaskHistory: HistoryItem[] = [
|
||||
{
|
||||
id: "persistent-task-1",
|
||||
number: 1,
|
||||
task: "Task that should persist",
|
||||
ts: new Date("2022-02-16T12:00:00").getTime(),
|
||||
tokensIn: 100,
|
||||
tokensOut: 50,
|
||||
totalCost: 0.01,
|
||||
workspace: desktopPath,
|
||||
},
|
||||
]
|
||||
|
||||
// Initial render - tasks should be visible
|
||||
mockUseExtensionState.mockReturnValue({
|
||||
taskHistory: desktopTaskHistory,
|
||||
cwd: desktopPath,
|
||||
} as any)
|
||||
|
||||
const { result, rerender } = renderHook(() => useTaskSearch())
|
||||
|
||||
expect(result.current.tasks).toHaveLength(1)
|
||||
expect(result.current.tasks[0].id).toBe("persistent-task-1")
|
||||
|
||||
// Simulate switching panels (component remount)
|
||||
rerender()
|
||||
|
||||
// Tasks should still be visible after remount
|
||||
expect(result.current.tasks).toHaveLength(1)
|
||||
expect(result.current.tasks[0].id).toBe("persistent-task-1")
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ import { Fzf } from "fzf"
|
|||
|
||||
import { highlightFzfMatch } from "@/utils/highlight"
|
||||
import { useExtensionState } from "@/context/ExtensionStateContext"
|
||||
import { arePathsEqual } from "@/utils/path"
|
||||
|
||||
type SortOption = "newest" | "oldest" | "mostExpensive" | "mostTokens" | "mostRelevant"
|
||||
|
||||
|
|
@ -26,7 +27,8 @@ export const useTaskSearch = () => {
|
|||
const presentableTasks = useMemo(() => {
|
||||
let tasks = taskHistory.filter((item) => item.ts && item.task)
|
||||
if (!showAllWorkspaces) {
|
||||
tasks = tasks.filter((item) => item.workspace === cwd)
|
||||
// Use arePathsEqual for proper path comparison that handles Desktop directory correctly
|
||||
tasks = tasks.filter((item) => arePathsEqual(item.workspace, cwd))
|
||||
}
|
||||
return tasks
|
||||
}, [taskHistory, showAllWorkspaces, cwd])
|
||||
|
|
|
|||
147
webview-ui/src/utils/__tests__/path.spec.ts
Normal file
147
webview-ui/src/utils/__tests__/path.spec.ts
Normal file
|
|
@ -0,0 +1,147 @@
|
|||
import { describe, it, expect, beforeEach, vi } from "vitest"
|
||||
import * as path from "path"
|
||||
import * as os from "os"
|
||||
import { arePathsEqual } from "../path"
|
||||
|
||||
describe("arePathsEqual", () => {
|
||||
const originalPlatform = process.platform
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
Object.defineProperty(process, "platform", {
|
||||
value: originalPlatform,
|
||||
})
|
||||
})
|
||||
|
||||
describe("cross-platform path comparison", () => {
|
||||
it("should return true for identical paths", () => {
|
||||
expect(arePathsEqual("/home/user/project", "/home/user/project")).toBe(true)
|
||||
expect(arePathsEqual("C:\\Users\\project", "C:\\Users\\project")).toBe(true)
|
||||
})
|
||||
|
||||
it("should return true for paths with different separators", () => {
|
||||
expect(arePathsEqual("/home/user/project", "/home/user/project/")).toBe(true)
|
||||
expect(arePathsEqual("C:\\Users\\project", "C:\\Users\\project\\")).toBe(true)
|
||||
})
|
||||
|
||||
it("should normalize paths with . and .. segments", () => {
|
||||
expect(arePathsEqual("/home/user/../user/project", "/home/user/project")).toBe(true)
|
||||
expect(arePathsEqual("/home/./user/project", "/home/user/project")).toBe(true)
|
||||
})
|
||||
|
||||
it("should handle undefined and null paths", () => {
|
||||
expect(arePathsEqual(undefined, undefined)).toBe(true)
|
||||
expect(arePathsEqual(null as any, null as any)).toBe(true)
|
||||
expect(arePathsEqual(undefined, "/home/user")).toBe(false)
|
||||
expect(arePathsEqual("/home/user", undefined)).toBe(false)
|
||||
})
|
||||
|
||||
it("should handle empty strings", () => {
|
||||
expect(arePathsEqual("", "")).toBe(true)
|
||||
expect(arePathsEqual("", "/home/user")).toBe(false)
|
||||
expect(arePathsEqual("/home/user", "")).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe("Windows-specific behavior", () => {
|
||||
beforeEach(() => {
|
||||
Object.defineProperty(process, "platform", {
|
||||
value: "win32",
|
||||
configurable: true,
|
||||
})
|
||||
})
|
||||
|
||||
it("should perform case-insensitive comparison on Windows", () => {
|
||||
expect(arePathsEqual("C:\\Users\\Project", "c:\\users\\project")).toBe(true)
|
||||
expect(arePathsEqual("C:\\USERS\\PROJECT", "c:\\Users\\Project")).toBe(true)
|
||||
})
|
||||
|
||||
it("should handle mixed separators on Windows", () => {
|
||||
expect(arePathsEqual("C:\\Users\\Project", "C:/Users/Project")).toBe(true)
|
||||
expect(arePathsEqual("C:/Users/Project", "C:\\Users\\Project")).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe("POSIX-specific behavior", () => {
|
||||
beforeEach(() => {
|
||||
Object.defineProperty(process, "platform", {
|
||||
value: "darwin",
|
||||
configurable: true,
|
||||
})
|
||||
})
|
||||
|
||||
it("should perform case-sensitive comparison on POSIX systems", () => {
|
||||
expect(arePathsEqual("/Users/Project", "/users/project")).toBe(false)
|
||||
expect(arePathsEqual("/Users/Project", "/Users/Project")).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe("Desktop directory handling", () => {
|
||||
it("should correctly compare Desktop paths on macOS", () => {
|
||||
Object.defineProperty(process, "platform", {
|
||||
value: "darwin",
|
||||
configurable: true,
|
||||
})
|
||||
|
||||
const desktopPath = "/Users/testuser/Desktop"
|
||||
const desktopPathWithSlash = "/Users/testuser/Desktop/"
|
||||
const desktopPathNormalized = path.normalize("/Users/testuser/Desktop")
|
||||
|
||||
expect(arePathsEqual(desktopPath, desktopPath)).toBe(true)
|
||||
expect(arePathsEqual(desktopPath, desktopPathWithSlash)).toBe(true)
|
||||
expect(arePathsEqual(desktopPath, desktopPathNormalized)).toBe(true)
|
||||
})
|
||||
|
||||
it("should correctly compare Desktop paths on Windows", () => {
|
||||
Object.defineProperty(process, "platform", {
|
||||
value: "win32",
|
||||
configurable: true,
|
||||
})
|
||||
|
||||
const desktopPath = "C:\\Users\\testuser\\Desktop"
|
||||
const desktopPathWithSlash = "C:\\Users\\testuser\\Desktop\\"
|
||||
const desktopPathMixedCase = "c:\\users\\testuser\\desktop"
|
||||
const desktopPathForwardSlash = "C:/Users/testuser/Desktop"
|
||||
|
||||
expect(arePathsEqual(desktopPath, desktopPath)).toBe(true)
|
||||
expect(arePathsEqual(desktopPath, desktopPathWithSlash)).toBe(true)
|
||||
expect(arePathsEqual(desktopPath, desktopPathMixedCase)).toBe(true)
|
||||
expect(arePathsEqual(desktopPath, desktopPathForwardSlash)).toBe(true)
|
||||
})
|
||||
|
||||
it("should handle relative Desktop paths", () => {
|
||||
const homeDir = os.homedir()
|
||||
const desktopRelative = path.join("~", "Desktop").replace("~", homeDir)
|
||||
const desktopAbsolute = path.join(homeDir, "Desktop")
|
||||
|
||||
expect(arePathsEqual(desktopRelative, desktopAbsolute)).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe("edge cases", () => {
|
||||
it("should handle paths with multiple slashes", () => {
|
||||
expect(arePathsEqual("/home//user///project", "/home/user/project")).toBe(true)
|
||||
expect(arePathsEqual("C:\\\\Users\\\\\\project", "C:\\Users\\project")).toBe(true)
|
||||
})
|
||||
|
||||
it("should handle root paths", () => {
|
||||
expect(arePathsEqual("/", "/")).toBe(true)
|
||||
expect(arePathsEqual("C:\\", "C:\\")).toBe(true)
|
||||
|
||||
// Root paths should keep their trailing slash
|
||||
Object.defineProperty(process, "platform", {
|
||||
value: "win32",
|
||||
configurable: true,
|
||||
})
|
||||
expect(arePathsEqual("C:\\", "c:/")).toBe(true)
|
||||
})
|
||||
|
||||
it("should return false for different paths", () => {
|
||||
expect(arePathsEqual("/home/user/project1", "/home/user/project2")).toBe(false)
|
||||
expect(arePathsEqual("/home/user", "/home/user/project")).toBe(false)
|
||||
})
|
||||
})
|
||||
})
|
||||
79
webview-ui/src/utils/path.ts
Normal file
79
webview-ui/src/utils/path.ts
Normal file
|
|
@ -0,0 +1,79 @@
|
|||
/**
|
||||
* Normalizes a path for consistent comparison
|
||||
* - Resolves ./.. segments
|
||||
* - Removes duplicate slashes
|
||||
* - Standardizes path separators
|
||||
* - Removes trailing slashes (except for root paths)
|
||||
*/
|
||||
function normalizePath(p: string): string {
|
||||
if (!p) return ""
|
||||
|
||||
// First, replace all backslashes with forward slashes for consistency
|
||||
let normalized = p.replace(/\\/g, "/")
|
||||
|
||||
// Remove duplicate slashes
|
||||
normalized = normalized.replace(/\/+/g, "/")
|
||||
|
||||
// Handle drive letters on Windows (e.g., C:/ -> C:/)
|
||||
if (/^[a-zA-Z]:/.test(normalized)) {
|
||||
// Ensure drive letter is uppercase for consistency
|
||||
normalized = normalized[0].toUpperCase() + normalized.slice(1)
|
||||
}
|
||||
|
||||
// Split into parts and resolve . and .. segments
|
||||
const parts = normalized.split("/")
|
||||
const resolved: string[] = []
|
||||
|
||||
for (const part of parts) {
|
||||
if (part === "..") {
|
||||
resolved.pop()
|
||||
} else if (part !== "." && part !== "") {
|
||||
resolved.push(part)
|
||||
} else if (part === "" && resolved.length === 0) {
|
||||
// Keep the first empty part for absolute paths
|
||||
resolved.push(part)
|
||||
}
|
||||
}
|
||||
|
||||
// Rejoin the parts
|
||||
normalized = resolved.join("/")
|
||||
|
||||
// Ensure absolute paths start with /
|
||||
if (p.startsWith("/") && !normalized.startsWith("/")) {
|
||||
normalized = "/" + normalized
|
||||
}
|
||||
|
||||
// Remove trailing slash, except for root paths
|
||||
if (normalized.length > 1 && normalized.endsWith("/")) {
|
||||
// Don't remove trailing slash from root paths like "/" or "C:/"
|
||||
if (!(normalized === "/" || /^[a-zA-Z]:\/$/i.test(normalized))) {
|
||||
normalized = normalized.slice(0, -1)
|
||||
}
|
||||
}
|
||||
|
||||
return normalized
|
||||
}
|
||||
|
||||
/**
|
||||
* Safe path comparison that works across different platforms
|
||||
* Handles case-insensitive comparison on Windows and normalizes paths
|
||||
* This is especially important for comparing workspace paths like Desktop directory
|
||||
*/
|
||||
export function arePathsEqual(path1?: string, path2?: string): boolean {
|
||||
if (!path1 && !path2) {
|
||||
return true
|
||||
}
|
||||
if (!path1 || !path2) {
|
||||
return false
|
||||
}
|
||||
|
||||
path1 = normalizePath(path1)
|
||||
path2 = normalizePath(path2)
|
||||
|
||||
// Windows paths are case-insensitive
|
||||
if (process.platform === "win32") {
|
||||
return path1.toLowerCase() === path2.toLowerCase()
|
||||
}
|
||||
|
||||
return path1 === path2
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue