Skip to content

Conversation

@DonJayamanne
Copy link
Collaborator

No description provided.

@DonJayamanne DonJayamanne changed the title Add more tests to Copilot CLI More tests for Copilot CLI and refactor permission handling Nov 8, 2025
@DonJayamanne DonJayamanne marked this pull request as ready for review November 8, 2025 09:19
Copilot AI review requested due to automatic review settings November 8, 2025 09:19
@vs-code-engineering vs-code-engineering bot added this to the November 2025 milestone Nov 8, 2025
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 CopilotCLI session permission handling architecture, moving from a centralized permission handler to a session-based approach. The changes improve modularity by allowing each session to manage its own permission requests through attached handlers, and add comprehensive test coverage for the new architecture.

Key Changes

  • Refactored permission handling from global CopilotCLIPermissionsHandler class to session-level handlers via CopilotCLISessionOptions
  • Added stream attachment mechanism to sessions, separating stream lifecycle from request handling
  • Introduced comprehensive unit tests for CopilotCLI chat session participant
  • Extracted UncommittedChangesStep constant to improve maintainability

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 14 comments.

Show a summary per file
File Description
src/extension/agents/copilotcli/node/copilotCli.ts Removed CopilotCLIPermissionsHandler class and introduced CopilotCLISessionOptions interface with permission handler registration
src/extension/agents/copilotcli/node/copilotcliSession.ts Added attachPermissionHandler and stream attachment methods; refactored handleRequest to remove stream parameter
src/extension/agents/copilotcli/node/copilotcliSessionService.ts Updated session creation/retrieval to use new options interface; removed isEmpty tracking from session items
src/extension/agents/copilotcli/node/permissionHelpers.ts Added requestPermission helper function to encapsulate permission request logic
src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts Refactored request handling flow; replaced vscode.commands calls with IRunCommandExecutionService
src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts Extracted UncommittedChangesStep constant for consistency
src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts New comprehensive test file covering participant request handling scenarios
src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts Updated tests to reflect new permission handler and stream attachment APIs
src/extension/agents/copilotcli/node/test/copilotCliSessionService.spec.ts Updated tests to work with new CopilotCLISessionOptions interface

Tyriar
Tyriar previously approved these changes Nov 8, 2025
@DonJayamanne DonJayamanne added this pull request to the merge queue Nov 8, 2025
Merged via the queue into main with commit 5c67247 Nov 8, 2025
16 checks passed
@DonJayamanne DonJayamanne deleted the don/xenial-rook branch November 8, 2025 21:51
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.

4 participants