Skip to content

sessions: port Copilot CLI PR detection to session open#4708

Merged
rzhao271 merged 1 commit intomainfrom
copilot/complicated-alligator
Mar 26, 2026
Merged

sessions: port Copilot CLI PR detection to session open#4708
rzhao271 merged 1 commit intomainfrom
copilot/complicated-alligator

Conversation

@osortega
Copy link
Contributor

What changed

  • ported the PR detection flow in chatSessions/vscode-node/copilotCLIChatSessions.ts to match PR sessions: stop polling for PR detection #4699
  • removed the old refresh-time/debounced PR detection path
  • trigger PR detection when an existing Copilot CLI session is opened instead
  • persist both pullRequestUrl and pullRequestState when detection succeeds
  • added focused unit coverage for the new session-open behavior

Why

The newer node-side chat sessions file still had the older background refresh-based detection logic. This brings it in sync with the behavior already introduced elsewhere so PR metadata is discovered at session open time and state stays consistent.

Validation

  • npm run compile
  • npm run lint
  • npm run test:unit -- src/extension/chatSessions/vscode-node/test/copilotCLIChatSessions.spec.ts

Reviewer notes

  • the new tests mock copilotCLIShim.ps1 the same way other terminal-related specs do so Vitest can import the module cleanly
  • handlePullRequestCreated was also updated to preserve pullRequestState for consistency with the session-open detection path

Copilot AI review requested due to automatic review settings March 26, 2026 04:07
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

Ports Copilot CLI pull request detection in the VS Code-node chat sessions implementation from refresh-time polling/debounced checks to a session-open triggered flow (aligned with prior work), and persists both PR URL and state when detected.

Changes:

  • Remove the refresh-time/debounced PR detection path from CopilotCLIChatSessionContentProvider.
  • Trigger PR detection as a fire-and-forget operation when an existing session is opened, persisting pullRequestUrl + pullRequestState.
  • Add unit tests covering session-open PR detection, persistence, and the merged-state skip path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/extension/chatSessions/vscode-node/copilotCLIChatSessions.ts Moves PR detection to session-open, persists PR URL/state, updates detection helpers’ return types.
src/extension/chatSessions/vscode-node/test/copilotCLIChatSessions.spec.ts Adds focused unit coverage for session-open PR detection behavior and persistence.

await this.copilotCLIWorktreeManagerService.setWorktreeProperties(session.sessionId, {
...worktreeProperties,
pullRequestUrl: prUrl,
pullRequestState: prState,
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

handlePullRequestCreated can persist pullRequestState: undefined when session.createdPullRequestUrl is already present (because prState is only set in the !prUrl branch). This can unintentionally clear an existing persisted PR state and leave the session metadata inconsistent. Consider defaulting prState to 'open' (as in copilotCLIChatSessionsContribution.ts) and/or preserving the existing worktreeProperties.pullRequestState when prState is undefined (e.g. prState ?? worktreeProperties.pullRequestState).

Suggested change
pullRequestState: prState,
pullRequestState: prState ?? worktreeProperties.pullRequestState,

Copilot uses AI. Check for mistakes.
@rzhao271 rzhao271 added this pull request to the merge queue Mar 26, 2026
Merged via the queue into main with commit 8a9d8f6 Mar 26, 2026
23 checks passed
@rzhao271 rzhao271 deleted the copilot/complicated-alligator branch March 26, 2026 17:12
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