Add spotlight mode for testing worktree changes in main repo#375
Add spotlight mode for testing worktree changes in main repo#375
Conversation
Adds a new MCP tool `taskyou_spotlight` that enables syncing worktree changes back to the main repository for testing. This bridges the gap between isolated task development and application runtime. Features: - `start`: Enables spotlight mode, stashes main repo changes, syncs files - `sync`: Manually syncs changed files from worktree to main repo - `status`: Shows current spotlight state and pending changes - `stop`: Disables spotlight and restores main repo to original state Inspired by Conductor's spotlight testing feature. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove ineffectual assignment in test (use _ = output) - Remove unnecessary nil check before len() in status function Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
13c2bd7 to
1677296
Compare
There was a problem hiding this comment.
Pull request overview
This pull request adds a new MCP tool taskyou_spotlight that enables syncing worktree changes back to the main repository for testing. This feature bridges the gap between isolated task development in worktrees and application runtime in the main repo, inspired by Conductor's spotlight testing feature.
Changes:
- Added
taskyou_spotlightMCP tool with four actions:start,stop,sync, andstatus - Implemented file syncing logic that copies git-tracked and untracked files from worktree to main repo
- Added comprehensive test suite covering basic workflows and error cases
- Updated documentation with usage examples and use cases
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 12 comments.
| File | Description |
|---|---|
| skills/taskyou/SKILL.md | Documents the new spotlight tool with examples and use cases |
| internal/mcp/server.go | Implements the spotlight tool registration and all four action handlers (start, stop, sync, status) |
| internal/mcp/server_test.go | Adds comprehensive tests for spotlight functionality including status, start/stop flow, worktree requirement, and sync operations |
| .gitignore | Adds the .spotlight-active state file to be ignored by git |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return s.spotlightStart(worktreePath, mainRepoDir) | ||
| case "stop": | ||
| return s.spotlightStop(worktreePath, mainRepoDir) | ||
| case "sync": |
There was a problem hiding this comment.
The 'sync' action doesn't check if spotlight mode is active before syncing. This could lead to unexpected behavior where files are synced to the main repo without a stash being created first, potentially overwriting uncommitted changes. Consider adding a check like the one in 'stop' action to ensure spotlight is active before syncing.
| case "sync": | |
| case "sync": | |
| if !isSpotlightActive(worktreePath) { | |
| return "", fmt.Errorf("spotlight mode is not active. Use 'start' to enable spotlight before syncing") | |
| } |
internal/mcp/server.go
Outdated
| // First, stash any uncommitted changes in the main repo to preserve them | ||
| stashCmd := exec.Command("git", "stash", "push", "-m", "spotlight-backup-"+time.Now().Format("20060102-150405")) | ||
| stashCmd.Dir = mainRepoDir | ||
| stashOutput, _ := stashCmd.CombinedOutput() | ||
| stashCreated := !strings.Contains(string(stashOutput), "No local changes") | ||
|
|
There was a problem hiding this comment.
The stash detection logic relies on checking for the substring "No local changes" in the output. This approach is fragile because git output messages can vary across different git versions, locales, or if git changes its output format. A more reliable approach would be to check the exit code of the stash command (exit code 0 with specific output) or use 'git diff --quiet' to detect if there are uncommitted changes before attempting to stash.
| // First, stash any uncommitted changes in the main repo to preserve them | |
| stashCmd := exec.Command("git", "stash", "push", "-m", "spotlight-backup-"+time.Now().Format("20060102-150405")) | |
| stashCmd.Dir = mainRepoDir | |
| stashOutput, _ := stashCmd.CombinedOutput() | |
| stashCreated := !strings.Contains(string(stashOutput), "No local changes") | |
| // First, stash any uncommitted changes in the main repo to preserve them. | |
| // Use `git diff --quiet` to robustly detect changes instead of parsing git's output. | |
| hasChanges := false | |
| // Check for unstaged changes | |
| diffCmd := exec.Command("git", "diff", "--quiet") | |
| diffCmd.Dir = mainRepoDir | |
| if err := diffCmd.Run(); err != nil { | |
| // Non-zero exit status indicates there are changes; treat other errors the same | |
| hasChanges = true | |
| } | |
| // Check for staged changes | |
| diffCachedCmd := exec.Command("git", "diff", "--cached", "--quiet") | |
| diffCachedCmd.Dir = mainRepoDir | |
| if err := diffCachedCmd.Run(); err != nil { | |
| hasChanges = true | |
| } | |
| stashCreated := false | |
| if hasChanges { | |
| stashCmd := exec.Command("git", "stash", "push", "-m", "spotlight-backup-"+time.Now().Format("20060102-150405")) | |
| stashCmd.Dir = mainRepoDir | |
| // Consider the stash "created" only if the command succeeds. | |
| if err := stashCmd.Run(); err == nil { | |
| stashCreated = true | |
| } | |
| } |
internal/mcp/server.go
Outdated
| checkoutCmd := exec.Command("git", "checkout", ".") | ||
| checkoutCmd.Dir = mainRepoDir | ||
| checkoutCmd.CombinedOutput() | ||
|
|
||
| // Clean any untracked files that were added | ||
| cleanCmd := exec.Command("git", "clean", "-fd") | ||
| cleanCmd.Dir = mainRepoDir | ||
| cleanCmd.CombinedOutput() |
There was a problem hiding this comment.
The git commands 'git checkout .' and 'git clean -fd' are executed without checking their errors. If these commands fail, the main repository might not be properly restored, but the user would still receive a success message. The error should be captured and handled appropriately, potentially warning the user if restoration fails.
internal/mcp/server.go
Outdated
| srcPath := filepath.Join(worktreePath, file) | ||
| dstPath := filepath.Join(mainRepoDir, file) | ||
|
|
There was a problem hiding this comment.
The file sync operation doesn't validate or sanitize file paths before copying. If a git repository contains files with path traversal sequences (e.g., '../../../etc/passwd'), the filepath.Join could potentially write files outside the intended main repo directory. While git typically prevents such files from being tracked, consider adding path validation to ensure all destination paths are within the mainRepoDir using filepath.Clean and checking that the result starts with mainRepoDir.
| srcPath := filepath.Join(worktreePath, file) | |
| dstPath := filepath.Join(mainRepoDir, file) | |
| // Clean and validate relative path to prevent path traversal | |
| cleanFile := filepath.Clean(file) | |
| if cleanFile == ".." || strings.HasPrefix(cleanFile, ".."+string(os.PathSeparator)) || filepath.IsAbs(cleanFile) { | |
| failed++ | |
| continue | |
| } | |
| srcPath := filepath.Join(worktreePath, cleanFile) | |
| dstPath := filepath.Join(mainRepoDir, cleanFile) | |
| dstPath = filepath.Clean(dstPath) | |
| // Ensure destination path is within mainRepoDir | |
| prefix := mainRepoDir | |
| if prefix != "" { | |
| prefix = filepath.Clean(prefix) | |
| } | |
| if dstPath != prefix && !strings.HasPrefix(dstPath, prefix+string(os.PathSeparator)) { | |
| failed++ | |
| continue | |
| } |
| // handleSpotlight handles spotlight mode operations. | ||
| func (s *Server) handleSpotlight(action, worktreePath, mainRepoDir string) (string, error) { | ||
| switch action { | ||
| case "start": | ||
| return s.spotlightStart(worktreePath, mainRepoDir) | ||
| case "stop": | ||
| return s.spotlightStop(worktreePath, mainRepoDir) | ||
| case "sync": | ||
| return s.spotlightSync(worktreePath, mainRepoDir) | ||
| case "status": | ||
| return s.spotlightStatus(worktreePath, mainRepoDir) | ||
| default: | ||
| return "", fmt.Errorf("unknown spotlight action: %s", action) | ||
| } | ||
| } |
There was a problem hiding this comment.
The spotlight operations don't validate that worktreePath and mainRepoDir are valid git repositories before executing git commands. If either path is not a git repository, the git commands will fail with potentially confusing error messages. Consider adding validation (e.g., checking for .git directory or running 'git rev-parse --git-dir') before performing operations to provide clearer error messages to users.
internal/mcp/server.go
Outdated
| // Check if destination exists and is the same | ||
| dstData, err := os.ReadFile(dstPath) | ||
| if err == nil && string(srcData) == string(dstData) { |
There was a problem hiding this comment.
Comparing file contents using byte-to-byte string comparison (string(srcData) == string(dstData)) may not be the most efficient approach for large files. While functionally correct, consider using bytes.Equal(srcData, dstData) which is more idiomatic and can be more efficient as it doesn't require converting to strings.
| // spotlightSync syncs git-tracked files from worktree to main repo. | ||
| // It compares files between the worktree and main repo, copying any that differ. | ||
| func (s *Server) spotlightSync(worktreePath, mainRepoDir string) (string, error) { | ||
| // Get list of all git-tracked files in the worktree | ||
| lsFilesCmd := exec.Command("git", "ls-files") | ||
| lsFilesCmd.Dir = worktreePath | ||
| lsFilesOutput, err := lsFilesCmd.Output() | ||
| if err != nil { | ||
| return "", fmt.Errorf("failed to list tracked files: %w", err) | ||
| } | ||
|
|
||
| // Also get untracked files (new files not yet added) | ||
| untrackedCmd := exec.Command("git", "ls-files", "--others", "--exclude-standard") | ||
| untrackedCmd.Dir = worktreePath | ||
| untrackedOutput, _ := untrackedCmd.Output() | ||
|
|
||
| // Also get uncommitted changes | ||
| diffCmd := exec.Command("git", "diff", "--name-only", "HEAD") | ||
| diffCmd.Dir = worktreePath | ||
| diffOutput, _ := diffCmd.Output() | ||
|
|
||
| // Build set of all files to check | ||
| fileSet := make(map[string]bool) | ||
| for _, file := range strings.Split(strings.TrimSpace(string(lsFilesOutput)), "\n") { | ||
| if file != "" { | ||
| fileSet[file] = true | ||
| } | ||
| } | ||
| for _, file := range strings.Split(strings.TrimSpace(string(untrackedOutput)), "\n") { | ||
| if file != "" { | ||
| fileSet[file] = true | ||
| } | ||
| } | ||
| for _, file := range strings.Split(strings.TrimSpace(string(diffOutput)), "\n") { | ||
| if file != "" { | ||
| fileSet[file] = true | ||
| } | ||
| } |
There was a problem hiding this comment.
The sync operation only copies files from the worktree to the main repo but doesn't handle file deletions. If a file is deleted in the worktree, it won't be deleted in the main repo during sync. This could lead to discrepancies where the main repo has files that were removed from the worktree. Consider using 'git diff --name-status' to detect deletions and remove those files from the main repo as well.
| // TestSpotlightSync tests the sync action | ||
| func TestSpotlightSync(t *testing.T) { | ||
| database := testDB(t) | ||
|
|
||
| // Create temp directories | ||
| worktreeDir := t.TempDir() | ||
| mainRepoDir := t.TempDir() | ||
|
|
||
| // Initialize repos | ||
| runGit(t, mainRepoDir, "init") | ||
| runGit(t, mainRepoDir, "config", "user.email", "test@test.com") | ||
| runGit(t, mainRepoDir, "config", "user.name", "Test") | ||
| os.WriteFile(filepath.Join(mainRepoDir, "file.txt"), []byte("original"), 0644) | ||
| runGit(t, mainRepoDir, "add", ".") | ||
| runGit(t, mainRepoDir, "commit", "-m", "initial") | ||
|
|
||
| runGit(t, worktreeDir, "init") | ||
| runGit(t, worktreeDir, "config", "user.email", "test@test.com") | ||
| runGit(t, worktreeDir, "config", "user.name", "Test") | ||
| os.WriteFile(filepath.Join(worktreeDir, "file.txt"), []byte("modified"), 0644) | ||
| runGit(t, worktreeDir, "add", ".") | ||
| runGit(t, worktreeDir, "commit", "-m", "initial") | ||
|
|
||
| // Create project | ||
| if err := database.CreateProject(&db.Project{Name: "spotlight-sync-test", Path: mainRepoDir}); err != nil { | ||
| t.Fatalf("failed to create project: %v", err) | ||
| } | ||
|
|
||
| // Create task | ||
| task := &db.Task{ | ||
| Title: "Spotlight Sync Test", | ||
| Status: db.StatusProcessing, | ||
| Project: "spotlight-sync-test", | ||
| } | ||
| if err := database.CreateTask(task); err != nil { | ||
| t.Fatalf("failed to create task: %v", err) | ||
| } | ||
|
|
||
| // Set the worktree path (normally done by executor) | ||
| task.WorktreePath = worktreeDir | ||
| if err := database.UpdateTask(task); err != nil { | ||
| t.Fatalf("failed to update task with worktree: %v", err) | ||
| } | ||
|
|
||
| // First start spotlight | ||
| startReq := map[string]interface{}{ | ||
| "jsonrpc": "2.0", | ||
| "id": 1, | ||
| "method": "tools/call", | ||
| "params": map[string]interface{}{ | ||
| "name": "taskyou_spotlight", | ||
| "arguments": map[string]interface{}{ | ||
| "action": "start", | ||
| }, | ||
| }, | ||
| } | ||
| reqBytes, _ := json.Marshal(startReq) | ||
| reqBytes = append(reqBytes, '\n') | ||
|
|
||
| server, output := testServer(database, task.ID, string(reqBytes)) | ||
| server.Run() | ||
| _ = output // first response not needed | ||
|
|
||
| // Now modify the worktree file | ||
| os.WriteFile(filepath.Join(worktreeDir, "file.txt"), []byte("modified again"), 0644) | ||
|
|
||
| // Sync changes | ||
| syncReq := map[string]interface{}{ | ||
| "jsonrpc": "2.0", | ||
| "id": 2, | ||
| "method": "tools/call", | ||
| "params": map[string]interface{}{ | ||
| "name": "taskyou_spotlight", | ||
| "arguments": map[string]interface{}{ | ||
| "action": "sync", | ||
| }, | ||
| }, | ||
| } | ||
| reqBytes, _ = json.Marshal(syncReq) | ||
| reqBytes = append(reqBytes, '\n') | ||
|
|
||
| server, output = testServer(database, task.ID, string(reqBytes)) | ||
| server.Run() | ||
|
|
||
| var resp jsonRPCResponse | ||
| if err := json.Unmarshal(output.Bytes(), &resp); err != nil { | ||
| t.Fatalf("failed to parse response: %v", err) | ||
| } | ||
|
|
||
| if resp.Error != nil { | ||
| t.Fatalf("sync failed: %s", resp.Error.Message) | ||
| } | ||
|
|
||
| // Verify file was synced | ||
| mainContent, err := os.ReadFile(filepath.Join(mainRepoDir, "file.txt")) | ||
| if err != nil { | ||
| t.Fatalf("failed to read main repo file: %v", err) | ||
| } | ||
| if string(mainContent) != "modified again" { | ||
| t.Errorf("file not synced, got: %s", string(mainContent)) | ||
| } | ||
| } |
There was a problem hiding this comment.
There's no test coverage for calling 'sync' action without first calling 'start'. While the code will technically work (it will sync files), this could be confusing because no stash was created. Consider adding a test to verify the behavior when sync is called without spotlight being active, or add validation to prevent sync when spotlight is not active.
| // handleSpotlight handles spotlight mode operations. | ||
| func (s *Server) handleSpotlight(action, worktreePath, mainRepoDir string) (string, error) { | ||
| switch action { | ||
| case "start": | ||
| return s.spotlightStart(worktreePath, mainRepoDir) | ||
| case "stop": | ||
| return s.spotlightStop(worktreePath, mainRepoDir) | ||
| case "sync": | ||
| return s.spotlightSync(worktreePath, mainRepoDir) | ||
| case "status": | ||
| return s.spotlightStatus(worktreePath, mainRepoDir) | ||
| default: | ||
| return "", fmt.Errorf("unknown spotlight action: %s", action) | ||
| } | ||
| } |
There was a problem hiding this comment.
The spotlight operations are not protected against concurrent access. If multiple spotlight operations run simultaneously on the same worktree (e.g., sync and stop), there could be race conditions with the state file and git operations. While this may be unlikely in practice (single MCP server per task), consider adding mutex protection around spotlight operations or documenting that they should not be called concurrently.
| os.WriteFile(filepath.Join(worktreeDir, "newfile.txt"), []byte("new content"), 0644) | ||
| runGit(t, worktreeDir, "add", "README.md") | ||
| runGit(t, worktreeDir, "commit", "-m", "initial") | ||
|
|
||
| // Create project | ||
| if err := database.CreateProject(&db.Project{Name: "spotlight-flow-test", Path: mainRepoDir}); err != nil { | ||
| t.Fatalf("failed to create project: %v", err) | ||
| } | ||
|
|
||
| // Create task | ||
| task := &db.Task{ | ||
| Title: "Spotlight Flow Test", | ||
| Status: db.StatusProcessing, | ||
| Project: "spotlight-flow-test", | ||
| } | ||
| if err := database.CreateTask(task); err != nil { | ||
| t.Fatalf("failed to create task: %v", err) | ||
| } | ||
|
|
||
| // Set the worktree path (normally done by executor) | ||
| task.WorktreePath = worktreeDir | ||
| if err := database.UpdateTask(task); err != nil { | ||
| t.Fatalf("failed to update task with worktree: %v", err) | ||
| } | ||
|
|
||
| // Step 1: Start spotlight | ||
| startReq := map[string]interface{}{ | ||
| "jsonrpc": "2.0", | ||
| "id": 1, | ||
| "method": "tools/call", | ||
| "params": map[string]interface{}{ | ||
| "name": "taskyou_spotlight", | ||
| "arguments": map[string]interface{}{ | ||
| "action": "start", | ||
| }, | ||
| }, | ||
| } | ||
| reqBytes, _ := json.Marshal(startReq) | ||
| reqBytes = append(reqBytes, '\n') | ||
|
|
||
| server, output := testServer(database, task.ID, string(reqBytes)) | ||
| server.Run() | ||
|
|
||
| var resp jsonRPCResponse | ||
| if err := json.Unmarshal(output.Bytes(), &resp); err != nil { | ||
| t.Fatalf("failed to parse start response: %v", err) | ||
| } | ||
|
|
||
| if resp.Error != nil { | ||
| t.Fatalf("start failed: %s", resp.Error.Message) | ||
| } | ||
|
|
||
| result := resp.Result.(map[string]interface{}) | ||
| content := result["content"].([]interface{}) | ||
| text := content[0].(map[string]interface{})["text"].(string) | ||
|
|
||
| if !strings.Contains(text, "enabled") { | ||
| t.Errorf("expected spotlight to be enabled, got: %s", text) | ||
| } | ||
|
|
||
| // Verify state file was created | ||
| stateFile := filepath.Join(worktreeDir, ".spotlight-active") | ||
| if _, err := os.Stat(stateFile); os.IsNotExist(err) { | ||
| t.Error("spotlight state file not created") | ||
| } | ||
|
|
||
| // Verify files were synced | ||
| mainReadme, err := os.ReadFile(filepath.Join(mainRepoDir, "README.md")) | ||
| if err != nil { | ||
| t.Fatalf("failed to read main repo README: %v", err) | ||
| } | ||
| if string(mainReadme) != "# Worktree Changes" { | ||
| t.Errorf("README not synced, got: %s", string(mainReadme)) | ||
| } |
There was a problem hiding this comment.
The test creates an untracked file 'newfile.txt' (line 971) but doesn't verify whether it gets synced to the main repo. This creates incomplete test coverage for the untracked files feature. Consider adding a verification step after the start action to ensure untracked files are properly synced, similar to the README.md verification at line 1038-1044.
- Use git diff --quiet for robust stash detection instead of parsing output - Add path traversal validation to prevent writing outside main repo - Use bytes.Equal for efficient file comparison - Handle file deletions during sync via git diff --diff-filter=D - Preserve state file if stash pop fails to allow manual recovery - Add proper error handling for git checkout and git clean commands - Require spotlight to be active before syncing - Add test for sync-without-start error case Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
taskyou_spotlightthat enables syncing worktree changes back to the main repository for testingFeatures
startsyncstatusstopUse Cases
Test plan
TestSpotlightStatus- status when inactiveTestSpotlightStartStopFlow- full start/stop flowTestSpotlightRequiresWorktree- error handlingTestSpotlightSync- manual sync action🤖 Generated with Claude Code