-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: Add worktree detection and visual indicator with path isolation #1391
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
base: develop
Are you sure you want to change the base?
Changes from all commits
c12bdec
c9e7d24
49e46c8
e2d7540
192894a
2a2cb06
48590c8
8a32785
e4e4b57
a82bf79
9aa024a
8626eec
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 |
|---|---|---|
| @@ -0,0 +1,345 @@ | ||
| /** | ||
| * Worktree Detection Tests | ||
| * ======================== | ||
| * Tests for worktree detection logic and backend path resolution priority order. | ||
| * | ||
| * Test Coverage: | ||
| * 1. Regex pattern validation for detecting worktree directory structure | ||
| * 2. Backend path resolution priority order (ENV > worktree > settings > standard) | ||
| * 3. Cross-platform path handling (Unix / and Windows \\) | ||
| * 4. Edge cases and error handling | ||
| */ | ||
|
|
||
| import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; | ||
| import { mkdirSync, mkdtempSync, writeFileSync, rmSync, existsSync, realpathSync } from "fs"; | ||
| import { tmpdir } from "os"; | ||
| import path from "path"; | ||
| import { WORKTREE_PATTERN, WORKTREE_ROOT_PATTERN, WORKTREE_SPEC_PATTERN } from "../../shared/constants/worktree-patterns"; | ||
| import { detectAutoBuildSourcePath } from "../ipc-handlers/settings-handlers"; | ||
|
|
||
| // Test data directory | ||
| const TEST_DIR = mkdtempSync(path.join(tmpdir(), "worktree-test-")); | ||
|
|
||
| /** | ||
| * Safely remove directory with retry logic for Windows | ||
| * Windows can have file locking issues that require retries | ||
| */ | ||
| function safeRemoveDir(dirPath: string, retries = 3): void { | ||
| for (let i = 0; i < retries; i++) { | ||
| try { | ||
| if (existsSync(dirPath)) { | ||
| rmSync(dirPath, { recursive: true, force: true, maxRetries: 3, retryDelay: 100 }); | ||
| } | ||
| return; // Success | ||
| } catch (error) { | ||
| if (i === retries - 1) { | ||
| // Last attempt failed - log but don't fail test | ||
| console.warn(`Warning: Failed to clean up test directory ${dirPath}:`, error); | ||
| return; | ||
| } | ||
| // Wait before retry (helps with Windows file locking) | ||
| const delay = 200 * (i + 1); | ||
| const start = Date.now(); | ||
| while (Date.now() - start < delay) { | ||
| // Busy wait | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Mock @electron-toolkit/utils | ||
| vi.mock("@electron-toolkit/utils", () => ({ | ||
| is: { | ||
| dev: true, | ||
| windows: process.platform === "win32", | ||
| macos: process.platform === "darwin", | ||
| linux: process.platform === "linux", | ||
| }, | ||
| })); | ||
|
|
||
| // Mock electron | ||
| vi.mock("electron", () => ({ | ||
| app: { | ||
| getAppPath: vi.fn(() => tmpdir()), | ||
| getVersion: vi.fn(() => "2.7.5"), | ||
| getPath: vi.fn((name: string) => { | ||
| if (name === "userData") return path.join(tmpdir(), "userData"); | ||
| return tmpdir(); | ||
| }), | ||
| }, | ||
| ipcMain: { | ||
| handle: vi.fn(), | ||
| on: vi.fn(), | ||
| }, | ||
| shell: { | ||
| openPath: vi.fn(), | ||
| }, | ||
| dialog: { | ||
| showOpenDialog: vi.fn(), | ||
| showMessageBox: vi.fn(), | ||
| }, | ||
| })); | ||
|
|
||
| // Mock electron-updater | ||
| vi.mock("electron-updater", () => ({ | ||
| autoUpdater: { | ||
| autoDownload: true, | ||
| autoInstallOnAppQuit: true, | ||
| checkForUpdates: vi.fn(), | ||
| on: vi.fn(), | ||
| }, | ||
| })); | ||
|
|
||
| describe("Worktree Detection", () => { | ||
| let originalCwd: string; | ||
| let originalEnv: NodeJS.ProcessEnv; | ||
|
|
||
| beforeEach(() => { | ||
| // Save original values | ||
| originalCwd = process.cwd(); | ||
| originalEnv = { ...process.env }; | ||
|
|
||
| // Clear mocks | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| // Restore original values | ||
| process.chdir(originalCwd); | ||
| process.env = originalEnv; | ||
| }); | ||
|
|
||
| describe("Regex Pattern", () => { | ||
| it("should detect valid 3-digit spec numbers", () => { | ||
| const validPaths = [ | ||
| "/project/.auto-claude/worktrees/tasks/001-feature", | ||
| "/project/.auto-claude/worktrees/tasks/009-bug-fix", | ||
| "/project/.auto-claude/worktrees/tasks/123-enhancement", | ||
| "/project/.auto-claude/worktrees/tasks/999-test", | ||
| ]; | ||
|
|
||
| for (const testPath of validPaths) { | ||
| expect(WORKTREE_PATTERN.test(testPath)).toBe(true); | ||
| } | ||
| }); | ||
|
|
||
| it("should reject 1, 2, or 4+ digit spec numbers", () => { | ||
| const invalidPaths = [ | ||
| "/project/.auto-claude/worktrees/tasks/1-feature", // 1 digit | ||
| "/project/.auto-claude/worktrees/tasks/12-feature", // 2 digits | ||
| "/project/.auto-claude/worktrees/tasks/1234-feature", // 4 digits | ||
| "/project/.auto-claude/worktrees/tasks/12345-feature", // 5 digits | ||
| ]; | ||
|
|
||
| for (const testPath of invalidPaths) { | ||
| expect(WORKTREE_PATTERN.test(testPath)).toBe(false); | ||
| } | ||
| }); | ||
|
|
||
| it("should handle cross-platform path separators", () => { | ||
| const unixPath = "/project/.auto-claude/worktrees/tasks/009-feature"; | ||
| const windowsPath = "C:\\project\\.auto-claude\\worktrees\\tasks\\009-feature"; | ||
|
|
||
| expect(WORKTREE_PATTERN.test(unixPath)).toBe(true); | ||
| expect(WORKTREE_PATTERN.test(windowsPath)).toBe(true); | ||
| }); | ||
|
|
||
| it("should extract correct spec number", () => { | ||
| // Custom pattern that captures both root path AND spec number (for dual extraction test) | ||
| const extractPattern = /(.*\.auto-claude[/\\]worktrees[/\\]tasks[/\\]([0-9]{3})-[^/\\]+)/; | ||
| const testPath = "/project/.auto-claude/worktrees/tasks/009-add-feature/apps/backend"; | ||
|
|
||
| const match = testPath.match(extractPattern); | ||
| expect(match).not.toBeNull(); | ||
| expect(match![2]).toBe("009"); | ||
| }); | ||
|
|
||
| it("should extract worktree root path", () => { | ||
| const testPath = "/project/.auto-claude/worktrees/tasks/009-add-feature/apps/backend"; | ||
|
|
||
| const match = testPath.match(WORKTREE_ROOT_PATTERN); | ||
| expect(match).not.toBeNull(); | ||
| expect(match![1]).toBe("/project/.auto-claude/worktrees/tasks/009-add-feature"); | ||
| }); | ||
|
|
||
| it("should reject malformed paths", () => { | ||
| const malformedPaths = [ | ||
| "/project/worktrees/tasks/009-feature", // Missing .auto-claude | ||
| "/project/.auto-claude/tasks/009-feature", // Missing worktrees | ||
| "/project/.auto-claude/worktrees/009-feature", // Missing tasks | ||
| "/project/.auto-claude/worktrees/tasks/", // No spec directory | ||
| "/project/.auto-claude/worktrees/tasks/abc-feature", // Non-numeric spec | ||
| ]; | ||
|
|
||
| for (const testPath of malformedPaths) { | ||
| expect(WORKTREE_PATTERN.test(testPath)).toBe(false); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| describe("Backend Path Resolution", () => { | ||
| let testWorktreeDir: string; | ||
| let testBackendDir: string; | ||
| let testMarkerFile: string; | ||
|
|
||
| beforeEach(() => { | ||
| // Create test worktree structure | ||
| testWorktreeDir = path.join(TEST_DIR, ".auto-claude", "worktrees", "tasks", "009-test-feature"); | ||
| testBackendDir = path.join(testWorktreeDir, "apps", "backend"); | ||
| const runnersDir = path.join(testBackendDir, "runners"); | ||
| testMarkerFile = path.join(runnersDir, "spec_runner.py"); | ||
|
|
||
| // Create directories and marker file | ||
| mkdirSync(runnersDir, { recursive: true }); | ||
| writeFileSync(testMarkerFile, "# spec_runner.py"); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| // Clean up test files (with retry logic for Windows) | ||
| safeRemoveDir(TEST_DIR); | ||
| }); | ||
|
|
||
| it("should prioritize environment variable", async () => { | ||
| // Set up custom backend path via ENV var | ||
| const customBackendPath = path.join(TEST_DIR, "custom-backend"); | ||
| const customRunnersDir = path.join(customBackendPath, "runners"); | ||
| mkdirSync(customRunnersDir, { recursive: true }); | ||
| writeFileSync(path.join(customRunnersDir, "spec_runner.py"), "# custom"); | ||
|
|
||
| process.env.AUTO_CLAUDE_BACKEND_PATH = customBackendPath; | ||
|
|
||
| // Change to worktree directory (should be ignored due to ENV var priority) | ||
| process.chdir(testWorktreeDir); | ||
|
|
||
| // Call the actual resolver function | ||
| const detectedPath = detectAutoBuildSourcePath(); | ||
|
|
||
| // ENV var should take precedence over worktree | ||
| expect(detectedPath).toBe(customBackendPath); | ||
| expect(WORKTREE_PATTERN.test(process.cwd())).toBe(true); // Verify we're in worktree | ||
| expect(process.env.AUTO_CLAUDE_BACKEND_PATH).toBe(customBackendPath); // Verify ENV var is set | ||
| }); | ||
|
|
||
| it("should prioritize worktree local backend", () => { | ||
| // Ensure no ENV override | ||
| delete process.env.AUTO_CLAUDE_BACKEND_PATH; | ||
|
|
||
| // Change to worktree directory | ||
| process.chdir(testWorktreeDir); | ||
|
|
||
| // Call the actual resolver function | ||
| const detectedPath = detectAutoBuildSourcePath(); | ||
|
|
||
| // Should return the worktree local backend path | ||
| // Use realpathSync to resolve symlinks (e.g., /var -> /private/var on macOS) | ||
| const expectedPath = realpathSync(path.join(testWorktreeDir, "apps", "backend")); | ||
| const normalizedDetectedPath = detectedPath ? realpathSync(detectedPath) : null; | ||
| expect(normalizedDetectedPath).toBe(expectedPath); | ||
| expect(existsSync(path.join(expectedPath, "runners", "spec_runner.py"))).toBe(true); | ||
| }); | ||
|
|
||
| it("should use saved settings as priority 3", () => { | ||
| // Simulate settings check (without ENV var or worktree) | ||
| const settingsPath = "/custom/settings/backend"; | ||
|
|
||
| // Mock scenario: not in worktree, no ENV var | ||
| process.chdir(TEST_DIR); // Non-worktree directory | ||
| delete process.env.AUTO_CLAUDE_BACKEND_PATH; | ||
|
|
||
| const cwd = process.cwd(); | ||
| const inWorktree = WORKTREE_PATTERN.test(cwd); | ||
| const hasEnvVar = !!process.env.AUTO_CLAUDE_BACKEND_PATH; | ||
|
|
||
| expect(inWorktree).toBe(false); | ||
| expect(hasEnvVar).toBe(false); | ||
|
|
||
| // In this scenario, settings path should be checked | ||
| // (actual settings reading logic is in settings-handlers.ts) | ||
| expect(settingsPath).toBeTruthy(); | ||
| }); | ||
|
|
||
| it("should fall back to standard auto-detection", () => { | ||
| // No ENV var, not in worktree, no saved settings | ||
| process.chdir(TEST_DIR); | ||
| delete process.env.AUTO_CLAUDE_BACKEND_PATH; | ||
|
|
||
| const cwd = process.cwd(); | ||
| const inWorktree = WORKTREE_PATTERN.test(cwd); | ||
| const hasEnvVar = !!process.env.AUTO_CLAUDE_BACKEND_PATH; | ||
|
|
||
| expect(inWorktree).toBe(false); | ||
| expect(hasEnvVar).toBe(false); | ||
|
|
||
| // Standard auto-detection would check possiblePaths array | ||
| // This is the final fallback when all else fails | ||
| }); | ||
|
Comment on lines
+241
to
+275
Contributor
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. 🧹 Nitpick | 🔵 Trivial Tests verify preconditions but don't exercise the resolver for priority 3/4. These tests confirm the scenario setup (not in worktree, no ENV var) but don't call 🤖 Prompt for AI Agents |
||
|
|
||
| it("should validate marker file exists", () => { | ||
| const validBackendPath = testBackendDir; | ||
| const invalidBackendPath = path.join(TEST_DIR, "no-backend"); | ||
|
|
||
| // Valid path has marker file | ||
| const validMarkerPath = path.join(validBackendPath, "runners", "spec_runner.py"); | ||
| expect(existsSync(validBackendPath)).toBe(true); | ||
| expect(existsSync(validMarkerPath)).toBe(true); | ||
|
|
||
| // Invalid path does not have marker file | ||
| const invalidMarkerPath = path.join(invalidBackendPath, "runners", "spec_runner.py"); | ||
| expect(existsSync(invalidBackendPath)).toBe(false); | ||
| expect(existsSync(invalidMarkerPath)).toBe(false); | ||
| }); | ||
|
|
||
| it("should handle invalid paths gracefully", () => { | ||
| // Set ENV var to invalid path | ||
| process.env.AUTO_CLAUDE_BACKEND_PATH = "/does/not/exist"; | ||
|
|
||
| const envPath = process.env.AUTO_CLAUDE_BACKEND_PATH; | ||
| const markerPath = path.join(envPath, "runners", "spec_runner.py"); | ||
| const exists = existsSync(envPath) && existsSync(markerPath); | ||
|
|
||
| // Should detect as invalid | ||
| expect(exists).toBe(false); | ||
|
|
||
| // Logic should fall back to next priority | ||
| // (actual fallback is handled in settings-handlers.ts) | ||
| }); | ||
| }); | ||
|
|
||
| describe("Worktree Info Extraction", () => { | ||
| it("should extract spec number from directory name", () => { | ||
| const testCases = [ | ||
| { path: ".auto-claude/worktrees/tasks/001-feature", expected: "001" }, | ||
| { path: ".auto-claude/worktrees/tasks/009-bug-fix", expected: "009" }, | ||
| { path: ".auto-claude/worktrees/tasks/123-enhancement", expected: "123" }, | ||
| { path: ".auto-claude/worktrees/tasks/999-test", expected: "999" }, | ||
| ]; | ||
|
|
||
| for (const testCase of testCases) { | ||
| const match = testCase.path.match(WORKTREE_SPEC_PATTERN); | ||
| expect(match).not.toBeNull(); | ||
| expect(match![1]).toBe(testCase.expected); | ||
| } | ||
| }); | ||
|
|
||
| it("should handle paths with subdirectories", () => { | ||
| const fullPath = "/project/.auto-claude/worktrees/tasks/009-feature/apps/backend/runners"; | ||
|
|
||
| const match = fullPath.match(WORKTREE_SPEC_PATTERN); | ||
| expect(match).not.toBeNull(); | ||
| expect(match![1]).toBe("009"); | ||
| }); | ||
|
|
||
| it("should return null for non-worktree paths", () => { | ||
| const nonWorktreePaths = [ | ||
| "/project/apps/backend", | ||
| "/project/src/main", | ||
| "/Users/user/projects/my-app", | ||
| ]; | ||
|
|
||
|
|
||
| for (const testPath of nonWorktreePaths) { | ||
| expect(WORKTREE_PATTERN.test(testPath)).toBe(false); | ||
| } | ||
| }); | ||
| }); | ||
| }); | ||
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.
🧹 Nitpick | 🔵 Trivial
Consider replacing busy-wait with async delay.
The busy-wait loop blocks the event loop. While acceptable for test utilities, using an async delay would be cleaner.
♻️ Suggested improvement
Note: This would require making
safeRemoveDirasync and updating callers to await it.🤖 Prompt for AI Agents