Skip to content

Conversation

@Saga4
Copy link
Contributor

@Saga4 Saga4 commented Aug 12, 2025

PR Type

Enhancement


Description

  • Add LSP features to create git worktrees

  • Implement WorktreeParams dataclass

  • Use create_git_worktrees, remove_git_worktrees utils

  • Minor newline fix in pre-commit config


Diagram Walkthrough

flowchart LR
  Client["LSP Client"] -- "codeflash/createWorktree" --> ServerCreate["create_worktree handler"]
  ServerCreate -- "create_worktree_root_dir" --> UtilsRoot["create_worktree_root_dir"]
  ServerCreate -- "create_git_worktrees" --> UtilsTrees["create_git_worktrees"]
  Client -- "codeflash/removeWorktree" --> ServerRemove["remove_worktree handler"]
  ServerRemove -- "remove_git_worktrees" --> UtilsRemove["remove_git_worktrees"]
Loading

File Walkthrough

Relevant files
Enhancement
beta.py
Add LSP worktree creation and removal                                       

codeflash/lsp/beta.py

  • Imported worktree utils from git_utils
  • Added WorktreeParams dataclass
  • Implemented create_worktree LSP feature
  • Implemented remove_worktree LSP feature
+114/-0 
Configuration changes
.pre-commit-config.yaml
Fix end-of-file newline                                                                   

.pre-commit-config.yaml

  • Ensured newline at end of file
+1/-1     

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


saga4 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Dynamic Registry

Using dynamic attribute worktree_registry on the server can lead to unexpected states; consider initializing it in the server constructor.

# Store worktree info for later cleanup (use public attribute instead of private)
if not hasattr(server, "worktree_registry"):
    server.worktree_registry = {}
Inconsistent Payload

The remove_worktree handler omits functionName field in its response, unlike create_worktree; ensure consistent response schemas.

    return {"candidateId": params.candidateId, "status": "warning", "message": "No worktree found for candidate"}

try:
    worktree_info = server.worktree_registry[params.candidateId]
    worktree_root = worktree_info["worktree_root"]
    worktrees = worktree_info["worktrees"]
    function_name = worktree_info["function_name"]

    # Use CLI's existing cleanup function
    remove_git_worktrees(worktree_root, worktrees)

    # Remove from registry
    del server.worktree_registry[params.candidateId]

    server.show_message_log(
        f"Successfully removed worktrees for {function_name} (candidate: {params.candidateId})", "Info"
    )

except Exception as e:
    server.show_message_log(f"Error removing worktree: {e!s}", "Error")
    return {"candidateId": params.candidateId, "status": "error", "message": f"Error removing worktree: {e!s}"}
else:
    return {
        "candidateId": params.candidateId,
        "status": "success",
        "message": f"Successfully removed worktrees for {function_name}",
    }
Error Logging

Catching broad Exceptions hides stack traces; logging full traceback may help debugging.

except Exception as e:
    server.show_message_log(f"Error creating worktree: {e!s}", "Error")
    return {
        "functionName": params.functionName,
        "candidateId": params.candidateId,
        "status": "error",
        "message": f"Error creating worktree: {e!s}",
    }

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Validate gitRoot path existence

Validate that module_root exists and is a directory before attempting worktree
creation to catch invalid paths early. Log a warning and return an error response if
the check fails.

codeflash/lsp/beta.py [367]

 module_root = Path(params.gitRoot)
+if not module_root.exists() or not module_root.is_dir():
+    server.show_message_log(
+        f"Invalid gitRoot path: {module_root}", "Warning"
+    )
+    return {
+        "functionName": params.functionName,
+        "candidateId": params.candidateId,
+        "status": "error",
+        "message": f"Invalid gitRoot path: {module_root}"
+    }
Suggestion importance[1-10]: 6

__

Why: Checking existence of module_root prevents later failures in worktree creation and provides clear error feedback for invalid gitRoot paths.

Low
Check worktree_root before use

Guard against a falsy worktree_root return value to avoid indexing errors or using
an invalid path. Return an error if worktree_root is missing.

codeflash/lsp/beta.py [382]

 worktree_root, worktrees = create_git_worktrees(git_root, worktree_root_dir, module_root)
+if not worktree_root or not worktrees:
+    server.show_message_log("Failed to create git worktrees", "Error")
+    return {
+        "functionName": params.functionName,
+        "candidateId": params.candidateId,
+        "status": "error",
+        "message": "Failed to create git worktrees"
+    }
Suggestion importance[1-10]: 6

__

Why: Ensuring worktree_root is truthy before use avoids potential indexing errors and handles unexpected failures from create_git_worktrees.

Low
General
Return functionName on removal

Include functionName in the success response to mirror the create endpoint and help
clients correlate operations.

codeflash/lsp/beta.py [459-463]

 return {
     "candidateId": params.candidateId,
+    "functionName": function_name,
     "status": "success",
     "message": f"Successfully removed worktrees for {function_name}",
 }
Suggestion importance[1-10]: 5

__

Why: Adding functionName matches the create endpoint’s response format and improves client-side correlation of remove operations.

Low

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants