Skip to content

Conversation

@DonJayamanne
Copy link
Collaborator

No description provided.

@DonJayamanne DonJayamanne self-assigned this Nov 2, 2025
@DonJayamanne DonJayamanne changed the base branch from don/yappy-chimpanzee to main November 2, 2025 04:22
return undefined;
}

async createWorktree(stream: vscode.ChatResponseStream): Promise<string | undefined> {
Copy link
Collaborator Author

@DonJayamanne DonJayamanne Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@osortega
I think getting the caller to decide whether to call this method or not is an improvement, as opposed to giving more responsibility to this method.
Callers already have all of the information using the public methods in this class.

I believe that will also improve readability as sometimes we pass in untitled id and other cases we pass in the real session id (what confuses me is the fact that here we MUST pass in the old untitled id & not the real session id).

I'd like to remove that ambiguity from this class/code.

This comment was marked as resolved.

@DonJayamanne DonJayamanne requested a review from osortega November 3, 2025 23:40
@DonJayamanne DonJayamanne marked this pull request as ready for review November 3, 2025 23:40
Copilot AI review requested due to automatic review settings November 3, 2025 23:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the worktree creation logic for Copilot CLI chat sessions by moving the isolation check from inside the createWorktreeIfNeeded method to its call site, and renames the method to simply createWorktree.

Key changes:

  • Renamed createWorktreeIfNeeded to createWorktree and removed the internal isolation check
  • Moved isolation preference check to the caller level, making the control flow more explicit
  • Adjusted the timing of when worktree path is stored relative to session creation

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants