Skip to content

Conversation

@joshspicer
Copy link
Member

(for main branch)

  • Restore PR extension check and install logic from commit 1cafcac
  • Renamed installPullRequestExtension to ensurePullRequestExtensionInstalled for clarity
  • Add install button in chat/multiDiff/context menu when PR extension is not installed
  • Show reload window prompt after successful installation to activate extension
  • Add github.copilot.cloud.sessions.installPullRequestExtension command
  • Improve error handling with user-friendly messages

- Restore PR extension check and install logic from commit 1cafcac
- Renamed installPullRequestExtension to ensurePullRequestExtensionInstalled for clarity
- Add install button in chat/multiDiff/context menu when PR extension is not installed
- Show reload window prompt after successful installation to activate extension
- Add github.copilot.cloud.sessions.installPullRequestExtension command
- Improve error handling with user-friendly messages
Copilot AI review requested due to automatic review settings October 26, 2025 22:56
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 restores and enhances the GitHub Pull Requests extension installation flow that was previously removed. It adds automatic detection of whether the extension is installed, provides UI commands to install it when missing, and improves the user experience with reload prompts and better error handling.

Key Changes:

  • Restored PR extension check and install logic with improved naming (ensurePullRequestExtensionInstalled)
  • Added install button in chat/multiDiff/context menu when PR extension is not installed
  • Implemented reload window prompt after successful installation

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/extension/chatSessions/vscode-node/chatSessions.ts Added PR extension detection, installation command, and context key management
package.json Added new command and menu contribution for installing PR extension

const enabled = this.configurationService.getConfig(ConfigKey.Internal.CopilotCloudEnabled);

if (enabled && !this.copilotCloudRegistrations) {
vscode.commands.executeCommand('setContext', prExtensionInstalledContextKey, this.isPullRequestExtensionInstalled());
Copy link

Copilot AI Oct 26, 2025

Choose a reason for hiding this comment

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

The context key should be set in the constructor or activation, not during conditional registration. This ensures the context is always accurate regardless of whether Copilot Cloud is enabled. Consider moving this line to the constructor.

Copilot uses AI. Check for mistakes.
return;
}

const isInsiders = this.envService.getEditorInfo().version.includes('insider');
Copy link

Copilot AI Oct 26, 2025

Choose a reason for hiding this comment

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

The version check for 'insider' is case-sensitive but VS Code Insiders versions may use different casing. Use case-insensitive matching: this.envService.getEditorInfo().version.toLowerCase().includes('insider')

Suggested change
const isInsiders = this.envService.getEditorInfo().version.includes('insider');
const isInsiders = this.envService.getEditorInfo().version.toLowerCase().includes('insider');

Copilot uses AI. Check for mistakes.
Comment on lines +216 to +238
const pollInterval = 100; // 100ms
let elapsed = 0;

while (elapsed < maxWaitTime) {
if (this.isPullRequestExtensionInstalled()) {
await vscode.commands.executeCommand('setContext', prExtensionInstalledContextKey, true);

const reloadAction = vscode.l10n.t('Reload Window');
const result = await vscode.window.showInformationMessage(
vscode.l10n.t('GitHub Pull Request extension installed successfully. Reload VS Code to activate it.'),
reloadAction
);

if (result === reloadAction) {
await vscode.commands.executeCommand('workbench.action.reloadWindow');
}
return;
}
await new Promise(resolve => setTimeout(resolve, pollInterval));
elapsed += pollInterval;
}

throw new Error('GitHub Pull Request extension installation timed out.');
Copy link

Copilot AI Oct 26, 2025

Choose a reason for hiding this comment

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

The polling mechanism blocks the extension host thread unnecessarily. Consider using the vscode.extensions.onDidChange event to listen for extension installation instead of polling.

Suggested change
const pollInterval = 100; // 100ms
let elapsed = 0;
while (elapsed < maxWaitTime) {
if (this.isPullRequestExtensionInstalled()) {
await vscode.commands.executeCommand('setContext', prExtensionInstalledContextKey, true);
const reloadAction = vscode.l10n.t('Reload Window');
const result = await vscode.window.showInformationMessage(
vscode.l10n.t('GitHub Pull Request extension installed successfully. Reload VS Code to activate it.'),
reloadAction
);
if (result === reloadAction) {
await vscode.commands.executeCommand('workbench.action.reloadWindow');
}
return;
}
await new Promise(resolve => setTimeout(resolve, pollInterval));
elapsed += pollInterval;
}
throw new Error('GitHub Pull Request extension installation timed out.');
// Wait for the extension to be installed, using onDidChange event
await new Promise<void>((resolve, reject) => {
const timeout = setTimeout(() => {
disposeListener();
reject(new Error('GitHub Pull Request extension installation timed out.'));
}, maxWaitTime);
const checkAndResolve = async () => {
if (this.isPullRequestExtensionInstalled()) {
clearTimeout(timeout);
disposeListener();
await vscode.commands.executeCommand('setContext', prExtensionInstalledContextKey, true);
const reloadAction = vscode.l10n.t('Reload Window');
const result = await vscode.window.showInformationMessage(
vscode.l10n.t('GitHub Pull Request extension installed successfully. Reload VS Code to activate it.'),
reloadAction
);
if (result === reloadAction) {
await vscode.commands.executeCommand('workbench.action.reloadWindow');
}
resolve();
}
};
const listener = vscode.extensions.onDidChange(() => {
void checkAndResolve();
});
function disposeListener() {
listener.dispose();
}
// Check immediately in case the extension is already installed
void checkAndResolve();
});

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

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

@copilot lets do that but I want a global listener for if the user just installs this themselves. but in the explicit install case we should continue showing the notification to reload

Copy link
Contributor

Copilot AI commented Oct 26, 2025

@joshspicer I've opened a new pull request, #1643, to work on those changes. Once the pull request is ready, I'll request review from you.

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