From 650ec0f65520a12a6de6fe87dcb8726be205a76d Mon Sep 17 00:00:00 2001 From: Alex Ross <38270282+alexr00@users.noreply.github.com> Date: Fri, 2 Oct 2026 09:00:09 +0200 Subject: [PATCH 1/4] Load a preview in the webview first --- src/commands.ts | 6 +- src/github/externalUriOpener.ts | 39 ++- src/github/folderRepositoryManager.ts | 35 ++- src/github/githubRepository.ts | 41 ++- src/github/issueOverview.ts | 24 +- src/github/overviewRestorer.ts | 2 +- src/github/pullRequestOverview.ts | 89 ++++-- src/github/queriesShared.gql | 43 +++ src/github/views.ts | 4 + src/test/github/externalUriOpener.test.ts | 89 +++++- .../github/folderRepositoryManager.test.ts | 66 +++++ src/test/github/githubRepository.test.ts | 129 ++++++++- src/test/github/pullRequestOverview.test.ts | 272 +++++++++++++++++- webviews/common/context.tsx | 13 +- webviews/components/comment.tsx | 12 +- webviews/components/header.tsx | 55 ++-- webviews/components/sidebar.tsx | 23 +- webviews/editorWebview/app.tsx | 13 +- webviews/editorWebview/index.css | 41 ++- webviews/editorWebview/overview.tsx | 26 +- webviews/editorWebview/test/app.test.tsx | 88 +++++- 21 files changed, 1023 insertions(+), 87 deletions(-) diff --git a/src/commands.ts b/src/commands.ts index 73aca12251..6e9d848036 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -565,14 +565,14 @@ export function registerCommands( })); - const resolvePr = async (context: BaseContext | undefined): Promise<{ folderManager: FolderRepositoryManager, pr: PullRequestModel } | undefined> => { + const resolvePr = async (context: BaseContext | undefined, loadMode: 'default' | 'overview' = 'default'): Promise<{ folderManager: FolderRepositoryManager, pr: PullRequestModel } | undefined> => { if (!context) { return undefined; } const folderManager = folderRepositoryManagerResolver.getManagerForRepository(context.owner, context.repo); - const pr = await folderManager.resolvePullRequest(context.owner, context.repo, context.number, true); + const pr = await folderManager.resolvePullRequest(context.owner, context.repo, context.number, true, loadMode); if (!pr) { return undefined; } @@ -1102,7 +1102,7 @@ export function registerCommands( repo: argument.pullRequestDetails.repository.name, number: argument.pullRequestDetails.number, preventDefaultContextMenuItems: true, - }))?.pr; + }, 'overview'))?.pr; } else if (PRChatContextItem.is(argument)) { issueModel = argument.pr; } else if (IssueChatContextItem.is(argument)) { diff --git a/src/github/externalUriOpener.ts b/src/github/externalUriOpener.ts index a83c329b85..de424f701e 100644 --- a/src/github/externalUriOpener.ts +++ b/src/github/externalUriOpener.ts @@ -9,8 +9,10 @@ import { IssueOverviewPanel } from './issueOverview'; import { PullRequestOverviewPanel } from './pullRequestOverview'; import { getGitHubIssueOrPullRequestUriOpenerPriority, openWithDefaultExternalOpener, parseGitHubIssueOrPullRequestUri } from '../common/externalUri'; import { Disposable } from '../common/lifecycle'; +import Logger from '../common/logger'; import { OPEN_PULL_LINKS, PR_SETTINGS_NAMESPACE } from '../common/settingKeys'; import { ITelemetry } from '../common/telemetry'; +import { formatError } from '../common/utils'; import { EXTENSION_ID } from '../constants'; class GitHubIssueOrPullRequestExternalUriOpener extends Disposable implements vscode.ExternalUriOpener { @@ -44,21 +46,30 @@ class GitHubIssueOrPullRequestExternalUriOpener extends Disposable implements vs const folderRepositoryManager = this._folderRepositoryManagerResolver.getManagerForRepository(identity.owner, identity.repo); if (identity.kind === 'pullRequest') { - const pullRequest = await folderRepositoryManager.resolvePullRequest(identity.owner, identity.repo, identity.number, true); - if (token.isCancellationRequested) { - return; + const pullRequest = folderRepositoryManager.resolvePullRequest(identity.owner, identity.repo, identity.number, true, 'overview').then(pullRequest => { + if (token.isCancellationRequested) { + throw new vscode.CancellationError(); + } + if (!pullRequest) { + throw new Error(vscode.l10n.t('Unable to find pull request #{0} in {1}/{2}.', identity.number, identity.owner, identity.repo)); + } + return pullRequest; + }); + // Start the webview while the first repository and PR requests are in flight. + try { + await PullRequestOverviewPanel.createOrShow( + this._telemetry, + this._context.extensionUri, + folderRepositoryManager, + identity, + pullRequest, + ); + } catch (error) { + if (!(error instanceof vscode.CancellationError)) { + Logger.error(`Failed to open pull request: ${formatError(error)}`, 'GitHubIssueOrPullRequestExternalUriOpener'); + await vscode.window.showErrorMessage(formatError(error)); + } } - if (!pullRequest) { - await vscode.window.showErrorMessage(vscode.l10n.t('Unable to find pull request #{0} in {1}/{2}.', identity.number, identity.owner, identity.repo)); - return; - } - await PullRequestOverviewPanel.createOrShow( - this._telemetry, - this._context.extensionUri, - folderRepositoryManager, - identity, - pullRequest, - ); } else { const issue = await folderRepositoryManager.resolveIssue(identity.owner, identity.repo, identity.number, true, true); if (token.isCancellationRequested) { diff --git a/src/github/folderRepositoryManager.ts b/src/github/folderRepositoryManager.ts index ae5063cc0e..2d6a0efec3 100644 --- a/src/github/folderRepositoryManager.ts +++ b/src/github/folderRepositoryManager.ts @@ -2461,7 +2461,7 @@ export class FolderRepositoryManager extends Disposable { //#region Git related APIs - private async resolveItem(owner: string, repositoryName: string): Promise { + private async resolveItem(owner: string, repositoryName: string, resolveMetadata: boolean = true): Promise { let githubRepo = this._githubRepositories.find(repo => { const ret = repo.remote.owner.toLowerCase() === owner.toLowerCase() && @@ -2472,7 +2472,7 @@ export class FolderRepositoryManager extends Disposable { if (!githubRepo) { Logger.appendLine(`GitHubRepository not found: ${owner}/${repositoryName}`, this.id); // try to create the repository - githubRepo = await this.createGitHubRepositoryFromOwnerName(owner, repositoryName); + githubRepo = await this.createGitHubRepositoryFromOwnerName(owner, repositoryName, resolveMetadata); } return githubRepo; } @@ -2490,11 +2490,23 @@ export class FolderRepositoryManager extends Disposable { repositoryName: string, pullRequestNumber: number, useCache: boolean = false, + loadMode: 'default' | 'overview' = 'default', ): Promise { - const githubRepo = await this.resolveItem(owner, repositoryName); + const githubRepo = await this.resolveItem(owner, repositoryName, loadMode !== 'overview'); Logger.trace(`Found GitHub repo for pr #${pullRequestNumber}: ${githubRepo ? 'yes' : 'no'}`, this.id); if (githubRepo) { - const pr = await githubRepo.getPullRequest(pullRequestNumber, 'FolderRepositoryManager.resolvePullRequest', useCache); + const pullRequestPromise = githubRepo.getPullRequest(pullRequestNumber, 'FolderRepositoryManager.resolvePullRequest', useCache, false, loadMode); + let pr: PullRequestModel | undefined; + if (loadMode === 'overview') { + // Both are needed to render the overview, but neither depends on the other. + const [accessibleRepository, pullRequest] = await Promise.all([ + this.validateGitHubRepositoryAccess(githubRepo), + pullRequestPromise, + ]); + pr = accessibleRepository ? pullRequest : undefined; + } else { + pr = await pullRequestPromise; + } Logger.trace(`Found GitHub pr repo for pr #${pullRequestNumber}: ${pr ? 'yes' : 'no'}`, this.id); return pr; } @@ -3046,7 +3058,7 @@ export class FolderRepositoryManager extends Disposable { }); } - async createGitHubRepositoryFromOwnerName(owner: string, repositoryName: string): Promise { + async createGitHubRepositoryFromOwnerName(owner: string, repositoryName: string, resolveMetadata: boolean = true): Promise { const existing = this.findExistingGitHubRepository({ owner, repositoryName }); if (existing) { return existing; @@ -3059,7 +3071,12 @@ export class FolderRepositoryManager extends Disposable { const gitRemotes = await parseRepositoryRemotesAsync(this.repository); const gitRemote = gitRemotes.find(r => r.owner === owner && r.repositoryName === repositoryName); const uri = gitRemote?.url ?? `https://github.com/${owner}/${repositoryName}`; - const repo = await this.createAndAddGitHubRepository(new Remote(gitRemote?.remoteName ?? repositoryName, uri, new Protocol(uri)), this._credentialStore); + const repo = await this.createGitHubRepository(new Remote(gitRemote?.remoteName ?? repositoryName, uri, new Protocol(uri)), this._credentialStore, undefined, true); + return resolveMetadata ? this.validateGitHubRepositoryAccess(repo) : repo; + } + + private async validateGitHubRepositoryAccess(repo: GitHubRepository): Promise { + const { owner, repositoryName } = repo.remote; let reason: string; try { await repo.getMetadata(); @@ -3069,15 +3086,17 @@ export class FolderRepositoryManager extends Disposable { Logger.appendLine(`Repository ${owner}/${repositoryName} is not accessible: ${e}`, this.id); } Logger.appendLine(`Repository ${owner}/${repositoryName} is not accessible.`, this.id); - this._inaccessibleRepos.add(repoKey); + this._inaccessibleRepos.add(`${owner.toLowerCase()}/${repositoryName.toLowerCase()}`); this.removeGitHubRepository(repo.remote); + const gitRemotes = await parseRepositoryRemotesAsync(this.repository); + const hasLocalRemote = gitRemotes.some(remote => remote.owner === owner && remote.repositoryName === repositoryName); /* __GDPR__ "repository.inaccessible" : { "hasLocalRemote" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" }, "reason" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" } } */ - this.telemetry.sendTelemetryEvent('repository.inaccessible', { hasLocalRemote: (!!gitRemote).toString(), reason }); + this.telemetry.sendTelemetryEvent('repository.inaccessible', { hasLocalRemote: hasLocalRemote.toString(), reason }); return undefined; } diff --git a/src/github/githubRepository.ts b/src/github/githubRepository.ts index 2c657db56f..5b40514d72 100644 --- a/src/github/githubRepository.ts +++ b/src/github/githubRepository.ts @@ -66,6 +66,7 @@ import { convertRESTPullRequestToRawPullRequest, getAvatarWithEnterpriseFallback, getOverrideBranch, + GraphQLAccount, isInCodespaces, parseAccount, parseGraphQLIssue, @@ -76,6 +77,7 @@ import { parseMilestone, restPaginate, } from './utils'; +import { PullRequestPreview } from './views'; import { AuthenticationError, AuthProvider, GitHubServerType, isSamlError } from '../common/authentication'; import { Disposable, disposeAll } from '../common/lifecycle'; @@ -1301,13 +1303,44 @@ export class GitHubRepository extends Disposable { } } - async getPullRequest(id: number, callerName: string, useCache: boolean = false, silent: boolean = false): Promise { + async getPullRequestPreview(number: number): Promise { + if (!Number.isSafeInteger(number) || number <= 0) { + throw new Error(`Invalid pull request number: ${number}`); + } + const { query, remote, schema } = await this.ensure(); + type PreviewData = Omit & { + author: GraphQLAccount | null; + baseRefName: string; + headRefName: string; + baseRepository: { owner: { login: string } }; + headRepository: { owner: { login: string } } | null; + }; + const { data } = await query<{ repository: { pullRequest: PreviewData | null } | null }>({ + query: schema.PullRequestPreview, + variables: { owner: remote.owner, name: remote.repositoryName, number }, + }); + if (!data.repository?.pullRequest) { + throw new Error(`Unable to load pull request preview for ${remote.owner}/${remote.repositoryName}#${number}`); + } + // A preview must never populate the shared cache of actionable PR models. + const { author, baseRefName, headRefName, baseRepository, headRepository, ...preview } = data.repository.pullRequest; + return { + ...preview, + author: parseAccount(author, this), + base: `${baseRepository.owner.login}/${remote.repositoryName}:${baseRefName}`, + head: headRepository ? `${headRepository.owner.login}/${remote.repositoryName}:${headRefName}` : '', + }; + } + + async getPullRequest(id: number, callerName: string, useCache: boolean = false, silent: boolean = false, loadMode: 'default' | 'overview' = 'default'): Promise { if (useCache && this._pullRequestModelsByNumber.has(id)) { Logger.debug(`Using cached pull request model for ${id}`, this.id); return this._pullRequestModelsByNumber.get(id)!.model; } - if (!(await this.isPlausibleItemNumber(id))) { + // Explicit overview requests already identify a PR; the max-number lookup is + // only useful for speculative references extracted from text. + if (!Number.isSafeInteger(id) || id <= 0 || (loadMode === 'default' && !(await this.isPlausibleItemNumber(id)))) { Logger.debug(`Skipping pull request fetch for implausible number ${id} (caller: ${callerName})`, this.id); return; } @@ -1331,7 +1364,9 @@ export class GitHubRepository extends Disposable { Logger.debug(`Fetch pull request ${id} - done`, this.id); const pr = this.createOrUpdatePullRequestModel(await parseGraphQLPullRequest(data.repository.pullRequest, this), silent); - await pr.getLastUpdateTime(new Date(pr.item.updatedAt)); + if (loadMode === 'default') { + await pr.getLastUpdateTime(new Date(pr.item.updatedAt)); + } let repoIds = GitHubRepository._succeededPullRequests.get(id); if (!repoIds) { repoIds = new Set(); diff --git a/src/github/issueOverview.ts b/src/github/issueOverview.ts index cc7b02de03..84d54cd044 100644 --- a/src/github/issueOverview.ts +++ b/src/github/issueOverview.ts @@ -42,6 +42,7 @@ export class IssueOverviewPanel extends W protected _identity: UnresolvedIdentity; protected _folderRepositoryManager: FolderRepositoryManager; protected _scrollPosition = { x: 0, y: 0 }; + private _identityUpdateSequence = 0; protected static _getViewColumn(toTheSide: boolean, panel?: IssueOverviewPanel): number | undefined { const tabViewColumn = vscode.window.tabGroups.activeTabGroup.viewColumn; @@ -56,7 +57,7 @@ export class IssueOverviewPanel extends W extensionUri: vscode.Uri, folderRepositoryManager: FolderRepositoryManager, identity: UnresolvedIdentity, - issue?: IssueModel, + issue?: IssueModel | Promise, toTheSide: boolean = false, _preserveFocus: boolean = true, existingPanel?: vscode.WebviewPanel @@ -377,7 +378,8 @@ export class IssueOverviewPanel extends W * Update the panel with an unresolved identity and optional model. * If no model is provided, it will be resolved from the identity. */ - public async updateWithIdentity(foldersManager: FolderRepositoryManager, identity: UnresolvedIdentity, issueModel?: TItem, progressLocation?: string): Promise { + public async updateWithIdentity(foldersManager: FolderRepositoryManager, identity: UnresolvedIdentity, issueModel?: TItem | Promise, progressLocation?: string): Promise { + const updateSequence = ++this._identityUpdateSequence; this._identity = identity; this._folderRepositoryManager = foldersManager; @@ -393,6 +395,20 @@ export class IssueOverviewPanel extends W } } + if (issueModel instanceof Promise) { + try { + issueModel = await issueModel; + } catch (error) { + if (updateSequence === this._identityUpdateSequence && !this._item) { + this.dispose(); + } + throw error; + } + } + if (this.isDisposed || updateSequence !== this._identityUpdateSequence) { + return; + } + // If no model provided, resolve it from the identity if (!issueModel) { const resolvedModel = await this.resolveModel(identity); @@ -404,6 +420,10 @@ export class IssueOverviewPanel extends W issueModel = resolvedModel; } + if (this.isDisposed || updateSequence !== this._identityUpdateSequence) { + return; + } + if (progressLocation) { return vscode.window.withProgress({ location: { viewId: progressLocation } }, () => this.updateItem(issueModel!)); } else { diff --git a/src/github/overviewRestorer.ts b/src/github/overviewRestorer.ts index 95ce7416eb..9d31987486 100644 --- a/src/github/overviewRestorer.ts +++ b/src/github/overviewRestorer.ts @@ -43,7 +43,7 @@ export class OverviewRestorer extends Disposable implements vscode.WebviewPanelS } return IssueOverviewPanel.createOrShow(this._telemetry, this._context.extensionUri, folderManager, identity, issueModel, undefined, true, webviewPanel); } else { - const pullRequestModel = await folderManager.resolvePullRequest(state.owner, state.repo, state.number, true); + const pullRequestModel = await folderManager.resolvePullRequest(state.owner, state.repo, state.number, true, 'overview'); if (!pullRequestModel) { webviewPanel.dispose(); return; diff --git a/src/github/pullRequestOverview.ts b/src/github/pullRequestOverview.ts index 2d3f66c77d..70f1294407 100644 --- a/src/github/pullRequestOverview.ts +++ b/src/github/pullRequestOverview.ts @@ -71,6 +71,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel | undefined; private _updateSequence = 0; + private _previewSequence = 0; private _resolveCommentThreadQueue: Promise = Promise.resolve(); public static override async createOrShow( @@ -78,19 +79,12 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel, toTheSide: boolean = false, preserveFocus: boolean = true, existingPanel?: vscode.WebviewPanel ) { - /* __GDPR__ - "pr.openDescription" : { - "isCopilot" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" } - } - */ - telemetry.sendTelemetryEvent('pr.openDescription', { isCopilot: (issue?.author.login === COPILOT_SWE_AGENT) ? 'true' : 'false' }); - const key = panelKey(identity.owner, identity.repo, identity.number); let panel = this._panels.get(key); if (existingPanel && panel && panel._panel !== existingPanel) { @@ -116,6 +110,14 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { const enterpriseUri = pullRequest.remote.isEnterprise ? getEnterpriseUri() : undefined; @@ -458,7 +456,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { + if (updateSequence !== this._updateSequence) { + return; + } + this._assignableUsers = assignableUsers; + const users = assignableUsers[pullRequestModel.remote.remoteName] ?? []; + const canAssignCopilot = users.some(user => COPILOT_ACCOUNTS[user.login]); + const reviewers = parseReviewers(requestedReviewers, [...(pullRequestModel.timelineEvents ?? timelineEvents)], pullRequest.author); + const isCopilotAlreadyReviewer = reviewers.some(reviewer => !isITeam(reviewer.reviewer) && reviewer.reviewer.login === COPILOT_REVIEWER); + await this._postMessage({ + command: 'pr.update', + pullrequest: { + canAssignCopilot, + canRequestCopilotReview: canAssignCopilot && !isCopilotAlreadyReviewer, + } satisfies Partial, + }); + Logger.debug(`Deferred assignable users loaded in ${Math.round(performance.now() - assignableUsersStart)}ms`, PullRequestOverviewPanel.ID); + }).catch(error => { + Logger.error(`Failed to update deferred assignable users: ${formatError(error)}`, PullRequestOverviewPanel.ID); + }); const deferredDataPromise = Promise.all([ measureDeferred('statusChecks', pullRequestModel.getStatusChecks()), reviewRequestsPromise, @@ -544,8 +565,6 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { const latestTimelineEvents = [...(pullRequestModel.timelineEvents ?? timelineEvents)]; const reviewers = parseReviewers(requestedReviewers!, latestTimelineEvents, pullRequest.author); - const copilotUser = users.find(user => COPILOT_ACCOUNTS[user.login]); - const isCopilotAlreadyReviewer = reviewers.some(reviewer => !isITeam(reviewer.reviewer) && reviewer.reviewer.login === COPILOT_REVIEWER); const isCopilotOnBehalf = await isCopilotOnMyBehalf(pullRequest, currentUser, coAuthors); if (updateSequence !== this._updateSequence) { return; @@ -565,7 +584,6 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel, }); const deferredTimingSummary = [...deferredTimings] @@ -632,7 +650,9 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel, progressLocation?: string ): Promise { - await super.updateWithIdentity(folderRepositoryManager, identity, pullRequestModel, progressLocation); + const previewSequence = ++this._previewSequence; + let loading = true; + const isLoading = () => loading && !this.isDisposed && previewSequence === this._previewSequence && !this._item; + const update = super.updateWithIdentity(folderRepositoryManager, identity, pullRequestModel, progressLocation); + if (isLoading() && (!pullRequestModel || pullRequestModel instanceof Promise)) { + void (async () => { + try { + const start = Date.now(); + const repository = await folderRepositoryManager.createGitHubRepositoryFromOwnerName(identity.owner, identity.repo, false); + if (!repository || !isLoading()) { + return; + } + const preview = await repository.getPullRequestPreview(identity.number); + if (isLoading()) { + await this._postMessage({ command: 'pr.preview', pullrequest: preview }); + Logger.debug(`PR overview preview loaded in ${Date.now() - start}ms`, PullRequestOverviewPanel.ID); + } + } catch (error) { + Logger.error(`Unable to load PR overview preview: ${formatError(error)}`, PullRequestOverviewPanel.ID); + } + })(); + } + try { + await update; + } finally { + loading = false; + } // Notify that this PR overview is now active - PullRequestOverviewPanel._onVisible.fire(this._item); + if (!this.isDisposed && this._item) { + PullRequestOverviewPanel._onVisible.fire(this._item); + } } protected override async _onDidReceiveMessage(message: IRequestMessage) { @@ -1407,6 +1455,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel; + export interface PullRequest extends Issue { isCopilotOnMyBehalf: boolean; isAgentSessionsWorkspace: boolean; diff --git a/src/test/github/externalUriOpener.test.ts b/src/test/github/externalUriOpener.test.ts index 595e87e8f5..723e92aea4 100644 --- a/src/test/github/externalUriOpener.test.ts +++ b/src/test/github/externalUriOpener.test.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { default as assert } from 'assert'; -import { createSandbox, SinonSandbox } from 'sinon'; +import { createSandbox, SinonSandbox, SinonStub } from 'sinon'; import * as vscode from 'vscode'; import { RemoteOnlyRepository } from '../../api/remoteOnlyRepository'; import { CredentialStore } from '../../github/credentials'; @@ -12,6 +12,8 @@ import { registerGitHubIssueOrPullRequestExternalUriOpener } from '../../github/ import { FolderRepositoryManager } from '../../github/folderRepositoryManager'; import { FolderRepositoryManagerResolver } from '../../github/folderRepositoryManagerResolver'; import { RepositoriesManager } from '../../github/repositoriesManager'; +import { PullRequestModel } from '../../github/pullRequestModel'; +import { PullRequestOverviewPanel } from '../../github/pullRequestOverview'; import { MockExtensionContext } from '../mocks/mockExtensionContext'; import { MockTelemetry } from '../mocks/mockTelemetry'; @@ -68,4 +70,89 @@ describe('GitHubIssueOrPullRequestExternalUriOpener', () => { context.dispose(); } }); + + describe('opening pull requests', () => { + const uri = vscode.Uri.parse('https://github.com/aaa/bbb/pull/1000'); + let context: MockExtensionContext; + let opener: vscode.ExternalUriOpener; + let cancellation: vscode.CancellationTokenSource; + let resolvePullRequest: (pr: PullRequestModel | undefined) => void; + let rejectPullRequest: (error: Error) => void; + let resolvePullRequestStub: SinonStub, ReturnType>; + + beforeEach(() => { + context = new MockExtensionContext(); + const telemetry = new MockTelemetry(); + const credentialStore = new CredentialStore(telemetry, context); + const repositoriesManager = new RepositoriesManager(credentialStore, telemetry); + const resolver = new FolderRepositoryManagerResolver(context, repositoriesManager, telemetry); + cancellation = new vscode.CancellationTokenSource(); + context.subscriptions.push(credentialStore, repositoriesManager, resolver, cancellation); + sandbox.stub(vscode.window, 'registerExternalUriOpener').callsFake((_id, value) => { + opener = value; + return new vscode.Disposable(() => undefined); + }); + context.subscriptions.push(registerGitHubIssueOrPullRequestExternalUriOpener(context, resolver, telemetry)); + sandbox.stub(opener as any, 'isOpenPullLinksEnabled').returns(true); + const pendingPullRequest = new Promise((resolve, reject) => { + resolvePullRequest = resolve; + rejectPullRequest = reject; + }); + resolvePullRequestStub = sandbox.stub(FolderRepositoryManager.prototype, 'resolvePullRequest').returns(pendingPullRequest); + }); + + afterEach(() => { + PullRequestOverviewPanel.findPanel('aaa', 'bbb', 1000)?.dispose(); + context.dispose(); + }); + + it('creates the first tab and loads its HTML before resolving the PR', async () => { + const createWebviewPanel = sandbox.spy(vscode.window, 'createWebviewPanel'); + const showError = sandbox.stub(vscode.window, 'showErrorMessage').resolves(undefined); + const opening = opener.openExternalUri(uri, { sourceUri: uri }, cancellation.token); + try { + assert.strictEqual(createWebviewPanel.callCount, 1); + assert.ok(createWebviewPanel.firstCall.returnValue.webview.html.includes('webview-pr-description.js')); + assert.ok(PullRequestOverviewPanel.findPanel('aaa', 'bbb', 1000)); + sandbox.assert.calledOnce(resolvePullRequestStub); + sandbox.assert.calledWithExactly(resolvePullRequestStub, 'aaa', 'bbb', 1000, true, 'overview'); + } finally { + resolvePullRequest(undefined); + await opening; + } + assert.strictEqual(showError.firstCall.args[0], 'Unable to find pull request #1000 in aaa/bbb.'); + assert.strictEqual(PullRequestOverviewPanel.findPanel('aaa', 'bbb', 1000), undefined); + }); + + it('does not create a tab for an already-cancelled request', async () => { + const createWebviewPanel = sandbox.spy(vscode.window, 'createWebviewPanel'); + cancellation.cancel(); + await opener.openExternalUri(uri, { sourceUri: uri }, cancellation.token); + + sandbox.assert.notCalled(createWebviewPanel); + sandbox.assert.notCalled(resolvePullRequestStub); + }); + + it('closes the new tab without an error when cancelled during resolution', async () => { + const showError = sandbox.stub(vscode.window, 'showErrorMessage').resolves(undefined); + const opening = opener.openExternalUri(uri, { sourceUri: uri }, cancellation.token); + assert.ok(PullRequestOverviewPanel.findPanel('aaa', 'bbb', 1000)); + cancellation.cancel(); + resolvePullRequest(undefined); + await opening; + + assert.strictEqual(PullRequestOverviewPanel.findPanel('aaa', 'bbb', 1000), undefined); + sandbox.assert.notCalled(showError); + }); + + it('reports resolution failures and closes the new tab', async () => { + const showError = sandbox.stub(vscode.window, 'showErrorMessage').resolves(undefined); + const opening = opener.openExternalUri(uri, { sourceUri: uri }, cancellation.token); + rejectPullRequest(new Error('PR lookup failed')); + await opening; + + assert.strictEqual(showError.firstCall.args[0], 'PR lookup failed'); + assert.strictEqual(PullRequestOverviewPanel.findPanel('aaa', 'bbb', 1000), undefined); + }); + }); }); diff --git a/src/test/github/folderRepositoryManager.test.ts b/src/test/github/folderRepositoryManager.test.ts index 82c5a1e9c2..fd89d3e728 100644 --- a/src/test/github/folderRepositoryManager.test.ts +++ b/src/test/github/folderRepositoryManager.test.ts @@ -28,6 +28,8 @@ import { PullRequestReviewCommon, ReviewContext } from '../../github/pullRequest import { IRequestMessage } from '../../common/webview'; import { PullRequestMergeability } from '../../github/interface'; import { PullRequest } from '../../github/views'; +import { RepositoryBuilder } from '../builders/rest/repoBuilder'; +import { UserBuilder } from '../builders/rest/userBuilder'; describe('PullRequestManager', function () { let sinon: SinonSandbox; @@ -53,6 +55,70 @@ describe('PullRequestManager', function () { sinon.restore(); }); + describe('overview resolution', function () { + const metadata = { ...new RepositoryBuilder().build(), currentUser: new UserBuilder().build() }; + + beforeEach(function () { + sinon.stub(GitHubRepository.prototype, 'ensure').callsFake(async function (this: GitHubRepository) { + return this; + }); + }); + + afterEach(function () { + for (const repo of manager.gitHubRepositories) { + repo.dispose(); + } + manager.dispose(); + if (manager.context instanceof MockExtensionContext) { + manager.context.dispose(); + } + }); + + it('fetches the PR concurrently with cold repository metadata', async function () { + let resolveMetadata: (value: typeof metadata) => void; + const pendingMetadata = new Promise(resolve => resolveMetadata = resolve); + const getMetadata = sinon.stub(GitHubRepository.prototype, 'getMetadata').returns(pendingMetadata); + const getPullRequest = sinon.stub(GitHubRepository.prototype, 'getPullRequest').callsFake(async function (this: GitHubRepository) { + const item = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1347).build(), this); + return new PullRequestModel(manager.credentialStore, telemetry, this, this.remote, item); + }); + const updates = sinon.stub(PullRequestModel.prototype, 'getLastUpdateTime').resolves(new Date()); + const opening = manager.resolvePullRequest('owner', 'repo', 1347, false, 'overview'); + try { + await new Promise(resolve => setImmediate(resolve)); + sinon.assert.calledOnce(getMetadata); + sinon.assert.calledOnce(getPullRequest); + sinon.assert.calledWithExactly(getPullRequest, 1347, 'FolderRepositoryManager.resolvePullRequest', false, false, 'overview'); + } finally { + resolveMetadata!(metadata); + } + const pr = await opening; + assert.strictEqual(pr?.number, 1347); + sinon.assert.notCalled(updates); + }); + + it('shares repository creation between concurrent preview and full loads', async function () { + const [previewRepository, fullRepository] = await Promise.all([ + manager.createGitHubRepositoryFromOwnerName('owner', 'repo', false), + manager.createGitHubRepositoryFromOwnerName('owner', 'repo', false), + ]); + + assert.ok(previewRepository); + assert.strictEqual(previewRepository, fullRepository); + assert.deepStrictEqual(manager.gitHubRepositories, [previewRepository]); + }); + + it('still rejects and remembers inaccessible repositories', async function () { + const getMetadata = sinon.stub(GitHubRepository.prototype, 'getMetadata').rejects(new Error('Not Found')); + sinon.stub(GitHubRepository.prototype, 'getPullRequest').resolves(undefined); + + assert.strictEqual(await manager.resolvePullRequest('owner', 'repo', 1347, false, 'overview'), undefined); + assert.strictEqual(manager.gitHubRepositories.length, 0); + assert.strictEqual(await manager.resolvePullRequest('owner', 'repo', 1347, false, 'overview'), undefined); + sinon.assert.calledOnce(getMetadata); + }); + }); + describe('updateRepositories', function () { it('skips a repository after a 404 without affecting healthy repositories', async function () { const inaccessibleUrl = 'https://github.com/owner/missing'; diff --git a/src/test/github/githubRepository.test.ts b/src/test/github/githubRepository.test.ts index bed96bce9e..389df414fb 100644 --- a/src/test/github/githubRepository.test.ts +++ b/src/test/github/githubRepository.test.ts @@ -5,7 +5,7 @@ import { default as assert } from 'assert'; import { NetworkStatus } from 'apollo-boost'; -import { SinonSandbox, createSandbox } from 'sinon'; +import { SinonSandbox, createSandbox, match } from 'sinon'; import { CredentialStore } from '../../github/credentials'; import { MockCommandRegistry } from '../mocks/mockCommandRegistry'; import { MockTelemetry } from '../mocks/mockTelemetry'; @@ -16,10 +16,14 @@ import { Uri } from 'vscode'; import { MockExtensionContext } from '../mocks/mockExtensionContext'; import { GitHubManager } from '../../authentication/githubServer'; import { GitHubServerType } from '../../common/authentication'; -import { CheckState, PullRequestCheckStatus } from '../../github/interface'; +import { CheckState, GithubItemStateEnum, PullRequestCheckStatus } from '../../github/interface'; import { PullRequestBuilder as GraphQLPullRequestBuilder } from '../builders/graphql/pullRequestBuilder'; import Logger from '../../common/logger'; import { LoggingApolloClient, LoggingOctokit } from '../../github/loggingOctokit'; +import { MockGitHubRepository } from '../mocks/mockGitHubRepository'; +import { PullRequestModel } from '../../github/pullRequestModel'; +import { visit } from 'graphql'; +import { parseAccount } from '../../github/utils'; describe('GitHubRepository', function () { let sinon: SinonSandbox; @@ -70,6 +74,127 @@ describe('GitHubRepository', function () { }); }); + describe('getPullRequest', function () { + let repo: MockGitHubRepository; + + beforeEach(function () { + const url = 'https://github.com/owner/repo'; + const remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom); + repo = new MockGitHubRepository(remote, credentialStore, telemetry, sinon); + }); + + afterEach(function () { + repo.dispose(); + }); + + it('loads a read-only preview without populating the PR model cache', async function () { + const preview = { + number: 1347, title: 'Preview', titleHTML: 'Preview', + body: 'Description', bodyHTML: '

Description

', url: 'https://github.com/owner/repo/pull/1347', + state: GithubItemStateEnum.Open, isDraft: true, createdAt: '2026-10-01T10:00:00Z', + author: { __typename: 'User', id: 'author', login: 'contributor', url: 'https://github.com/contributor', avatarUrl: '' }, + baseRefName: 'main', headRefName: 'feature', + baseRepository: { owner: { login: 'owner' } }, headRepository: { owner: { login: 'contributor' } }, + }; + const query = sinon.stub(repo, 'query').resolves({ + data: { repository: { pullRequest: preview } }, + loading: false, stale: false, networkStatus: NetworkStatus.ready, + }); + + const { author, baseRefName, headRefName, baseRepository, headRepository, ...content } = preview; + assert.deepStrictEqual(await repo.getPullRequestPreview(1347), { + ...content, + author: parseAccount(author, repo), + base: 'owner/repo:main', + head: 'contributor/repo:feature', + }); + assert.strictEqual(repo.getExistingPullRequestModel(1347), undefined); + sinon.assert.calledOnce(query); + assert.strictEqual(query.firstCall.args[0].query, repo.schema.PullRequestPreview); + assert.deepStrictEqual(query.firstCall.args[0].variables, { owner: 'owner', name: 'repo', number: 1347 }); + const fields: string[] = []; + visit(repo.schema.PullRequestPreview, { Field(node) { fields.push(node.name.value); } }); + assert.ok(fields.includes('titleHTML') && fields.includes('bodyHTML')); + for (const field of ['commits', 'suggestedReviewers', 'mergeable', 'mergeStateStatus', 'reactionGroups', 'reviewThreads']) { + assert.ok(!fields.includes(field), `Preview must not query ${field}`); + } + assert.ok(!fields.includes('email'), 'Preview must not require additional user scopes'); + + query.resolves({ + data: { repository: { pullRequest: { ...preview, author: null, headRepository: null } } }, + loading: false, stale: false, networkStatus: NetworkStatus.ready, + }); + const deletedAuthorPreview = await repo.getPullRequestPreview(1347); + assert.deepStrictEqual(deletedAuthorPreview.author, parseAccount(null, repo)); + assert.strictEqual(deletedAuthorPreview.head, ''); + }); + + it('rejects missing previews and invalid preview numbers', async function () { + const query = sinon.stub(repo, 'query').resolves({ + data: { repository: { pullRequest: null } }, + loading: false, stale: false, networkStatus: NetworkStatus.ready, + }); + for (const number of [0, -1, NaN, Infinity, 1.5]) { + await assert.rejects(repo.getPullRequestPreview(number), /Invalid pull request number/); + } + sinon.assert.notCalled(query); + await assert.rejects(repo.getPullRequestPreview(1347), /Unable to load pull request preview/); + assert.strictEqual(repo.getExistingPullRequestModel(1347), undefined); + }); + + it('loads an overview with only the PR query and reuses its cached model', async function () { + const data = new GraphQLPullRequestBuilder().build(); + const query = sinon.stub(repo, 'query').resolves({ data, loading: false, stale: false, networkStatus: NetworkStatus.ready }); + const updates = sinon.stub(PullRequestModel.prototype, 'getLastUpdateTime').resolves(new Date()); + + const pr = await repo.getPullRequest(1347, 'test', false, false, 'overview'); + + assert.ok(pr); + assert.strictEqual(pr.title, data.repository!.pullRequest.title); + assert.strictEqual(pr.bodyHTML, data.repository!.pullRequest.bodyHTML); + sinon.assert.calledOnce(query); + assert.strictEqual(query.firstCall.args[0].query, repo.schema.PullRequest); + sinon.assert.notCalled(updates); + assert.strictEqual(await repo.getPullRequest(1347, 'test', true, false, 'overview'), pr); + sinon.assert.calledOnce(query); + }); + + it('preserves number validation and update checks for default loads', async function () { + const query = sinon.stub(repo, 'query').resolves({ + data: new GraphQLPullRequestBuilder().build(), + loading: false, stale: false, networkStatus: NetworkStatus.ready, + }); + const maxItemResult = { + data: { repository: { issues: { edges: [{ node: { number: 1347 } }] } } }, + loading: false, stale: false, networkStatus: NetworkStatus.ready, + }; + query.withArgs(match.has('query', repo.schema.MaxIssue)).resolves(maxItemResult); + query.withArgs(match.has('query', repo.schema.MaxPullRequest)).resolves(maxItemResult); + const updates = sinon.stub(PullRequestModel.prototype, 'getLastUpdateTime').resolves(new Date()); + + assert.ok(await repo.getPullRequest(1347, 'test')); + + assert.strictEqual(query.callCount, 3); + sinon.assert.calledOnce(updates); + }); + + it('rejects invalid overview numbers without a network request', async function () { + const query = sinon.spy(repo, 'query'); + for (const number of [0, -1, NaN, Infinity, 1.5]) { + assert.strictEqual(await repo.getPullRequest(number, 'test', false, false, 'overview'), undefined); + } + sinon.assert.notCalled(query); + }); + + it('logs a failed overview fetch instead of returning a partial model', async function () { + sinon.stub(repo, 'query').rejects(new Error('PR unavailable')); + const logError = sinon.spy(Logger, 'error'); + + assert.strictEqual(await repo.getPullRequest(1347, 'test', false, false, 'overview'), undefined); + assert.strictEqual(logError.firstCall.args[0], 'Unable to fetch PR: Error: PR unavailable'); + }); + }); + describe('isGitHubDotCom', function () { it('detects when the remote is pointing to github.com', function () { const url = 'https://github.com/some/repo'; diff --git a/src/test/github/pullRequestOverview.test.ts b/src/test/github/pullRequestOverview.test.ts index 959e52b15e..78a582d2cd 100644 --- a/src/test/github/pullRequestOverview.test.ts +++ b/src/test/github/pullRequestOverview.test.ts @@ -5,7 +5,7 @@ import { default as assert } from 'assert'; import * as vscode from 'vscode'; -import { SinonSandbox, createSandbox, match as sinonMatch } from 'sinon'; +import { SinonSandbox, SinonStub, createSandbox, match as sinonMatch } from 'sinon'; import { FolderRepositoryManager } from '../../github/folderRepositoryManager'; import { MockTelemetry } from '../mocks/mockTelemetry'; @@ -23,12 +23,15 @@ import { GitApiImpl } from '../../api/api1'; import { CredentialStore } from '../../github/credentials'; import { GitHubServerType } from '../../common/authentication'; import { GitHubRemote } from '../../common/remote'; -import { CheckState, GithubItemStateEnum } from '../../github/interface'; +import { CheckState, GithubItemStateEnum, IAccount, PullRequestMergeability } from '../../github/interface'; import { CreatePullRequestHelper } from '../../view/createPullRequestHelper'; import { RepositoriesManager } from '../../github/repositoriesManager'; import { MockThemeWatcher } from '../mocks/mockThemeWatcher'; import { TimelineEvent } from '../../common/timelineEvent'; import { PullRequestReviewCommon } from '../../github/pullRequestReviewCommon'; +import { COPILOT_REVIEWER_ACCOUNT } from '../../common/copilot'; +import Logger from '../../common/logger'; +import { PullRequest, PullRequestPreview } from '../../github/views'; const EXTENSION_URI = vscode.Uri.joinPath(vscode.Uri.file(__dirname), '../../..'); @@ -316,6 +319,271 @@ describe('PullRequestOverview', function () { }); }); + describe('deferred assignable users', function () { + let prModel: PullRequestModel; + let webviewPanel: vscode.WebviewPanel; + let messages: { command: string; pullrequest?: Partial }[]; + let onDidReceiveMessage: vscode.EventEmitter<{ command: string }>; + let resolveUsers: (users: { [key: string]: IAccount[] }) => void; + let rejectUsers: (error: Error) => void; + let usersPromise: Promise<{ [key: string]: IAccount[] }>; + let getAssignableUsers: SinonStub, ReturnType>; + let getReviewRequests: SinonStub<[], ReturnType>; + let getPreview: SinonStub<[number], Promise>; + const preview: PullRequestPreview = { + number: 1000, title: 'Preview title', titleHTML: 'Preview title', + body: 'Preview description', bodyHTML: '

Preview description

', url: 'https://github.com/aaa/bbb/pull/1000', + author: COPILOT_REVIEWER_ACCOUNT, createdAt: '2026-10-01T10:00:00Z', + state: GithubItemStateEnum.Open, isDraft: false, base: 'aaa/bbb:main', head: 'aaa/bbb:feature', + }; + + beforeEach(function () { + const prItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo); + prModel = new PullRequestModel(credentialStore, telemetry, repo, remote, prItem); + sinon.stub(pullRequestManager, 'createGitHubRepositoryFromOwnerName').resolves(repo); + getPreview = sinon.stub(repo, 'getPullRequestPreview').resolves(preview); + sinon.stub(pullRequestManager, 'getCurrentUser').resolves(prModel.author); + sinon.stub(prModel, 'canEdit').resolves(true); + getReviewRequests = sinon.stub(prModel, 'getReviewRequests').resolves([]); + sinon.stub(prModel, 'getTimelineEvents').resolves([]); + sinon.stub(prModel, 'validateDraftMode').resolves(false); + sinon.stub(prModel, 'getStatusChecks').resolves([{ state: CheckState.Success, statuses: [] }, null]); + sinon.stub(prModel, 'getMergeability').resolves({ mergeability: PullRequestMergeability.Mergeable }); + sinon.stub(pullRequestManager, 'getBranchNameForPullRequest').resolves(undefined); + sinon.stub(pullRequestManager, 'mergeQueueMethodForBranch').resolves(undefined); + sinon.stub(pullRequestManager, 'isHeadUpToDateWithBase').resolves(true); + sinon.stub(pullRequestManager, 'getPreferredEmail').resolves(undefined); + sinon.stub(pullRequestManager, 'checkBranchUpToDate').resolves(); + usersPromise = new Promise((resolve, reject) => { + resolveUsers = resolve; + rejectUsers = reject; + }); + getAssignableUsers = sinon.stub(pullRequestManager, 'getAssignableUsers').returns(usersPromise); + + messages = []; + webviewPanel = vscode.window.createWebviewPanel(PullRequestOverviewPanel.viewType, '#1000', vscode.ViewColumn.One, {}); + onDidReceiveMessage = new vscode.EventEmitter(); + context.subscriptions.push(webviewPanel, onDidReceiveMessage); + sinon.stub(webviewPanel.webview, 'onDidReceiveMessage').callsFake(onDidReceiveMessage.event); + sinon.stub(webviewPanel.webview, 'postMessage').callsFake(async message => { + messages.push(message.res); + return true; + }); + }); + + async function openPanel(model: PullRequestModel | Promise = prModel): Promise { + const identity = { owner: remote.owner, repo: remote.repositoryName, number: prModel.number }; + const opening = PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity, model, false, true, webviewPanel); + onDidReceiveMessage.fire({ command: 'ready' }); + await opening; + await new Promise(resolve => setImmediate(resolve)); + } + + afterEach(async function () { + resolveUsers({}); + await new Promise(resolve => setImmediate(resolve)); + }); + + it('shows the title and description while the full PR is still pending', async function () { + let resolveModel: (model: PullRequestModel) => void; + const opening = openPanel(new Promise(resolve => resolveModel = resolve)); + try { + await new Promise(resolve => setImmediate(resolve)); + assert.deepStrictEqual(messages.find(message => message.command === 'pr.preview')?.pullrequest, preview); + assert.strictEqual(messages.some(message => message.command === 'pr.initialize'), false); + sinon.assert.notCalled(getAssignableUsers); + } finally { + resolveModel!(prModel); + await opening; + } + assert.ok(messages.some(message => message.command === 'pr.initialize')); + }); + + it('does not fetch a preview for an already available PR model', async function () { + await openPanel(); + sinon.assert.notCalled(getPreview); + }); + + it('ignores a preview from an older lookup while a newer lookup is pending', async function () { + let resolvePreview: (value: PullRequestPreview) => void; + let resolveFirst: (value: PullRequestModel) => void; + let resolveSecond: (value: PullRequestModel) => void; + getPreview.onFirstCall().returns(new Promise(resolve => resolvePreview = resolve)); + const first = openPanel(new Promise(resolve => resolveFirst = resolve)); + await new Promise(resolve => setImmediate(resolve)); + const second = openPanel(new Promise(resolve => resolveSecond = resolve)); + try { + await new Promise(resolve => setImmediate(resolve)); + messages.length = 0; + resolvePreview!({ ...preview, title: 'Outdated preview' }); + await new Promise(resolve => setImmediate(resolve)); + assert.strictEqual(messages.some(message => message.command === 'pr.preview'), false); + } finally { + resolveFirst!(prModel); + resolveSecond!(prModel); + await Promise.all([first, second]); + } + }); + + it('does not let a late preview replace the complete PR', async function () { + let resolvePreview: (value: PullRequestPreview) => void; + let resolveModel: (value: PullRequestModel) => void; + getPreview.returns(new Promise(resolve => resolvePreview = resolve)); + const opening = openPanel(new Promise(resolve => resolveModel = resolve)); + await new Promise(resolve => setImmediate(resolve)); + resolveModel!(prModel); + await opening; + resolvePreview!(preview); + await new Promise(resolve => setImmediate(resolve)); + + assert.strictEqual(messages.some(message => message.command === 'pr.preview'), false); + assert.ok(messages.some(message => message.command === 'pr.initialize')); + }); + + it('ignores a preview that finishes after the panel is closed', async function () { + let resolvePreview: (value: PullRequestPreview) => void; + let resolveModel: (value: PullRequestModel) => void; + getPreview.returns(new Promise(resolve => resolvePreview = resolve)); + const opening = openPanel(new Promise(resolve => resolveModel = resolve)); + await new Promise(resolve => setImmediate(resolve)); + webviewPanel.dispose(); + resolvePreview!(preview); + resolveModel!(prModel); + await opening; + + assert.strictEqual(messages.some(message => message.command === 'pr.preview'), false); + }); + + it('logs a preview failure without preventing full PR initialization', async function () { + let resolveModel: (value: PullRequestModel) => void; + getPreview.rejects(new Error('Preview unavailable')); + const logError = sinon.spy(Logger, 'error'); + const opening = openPanel(new Promise(resolve => resolveModel = resolve)); + try { + await new Promise(resolve => setImmediate(resolve)); + assert.ok(logError.getCalls().some(call => typeof call.args[0] === 'string' && call.args[0].includes('Preview unavailable'))); + } finally { + resolveModel!(prModel); + await opening; + } + assert.ok(messages.some(message => message.command === 'pr.initialize')); + }); + + it('loads the webview before a supplied PR model resolves', async function () { + let resolveModel: (model: PullRequestModel) => void; + const pendingModel = new Promise(resolve => resolveModel = resolve); + const opening = openPanel(pendingModel); + try { + assert.ok(webviewPanel.webview.html.includes('webview-pr-description.js')); + assert.strictEqual(messages.some(message => message.command === 'pr.initialize'), false); + } finally { + resolveModel!(prModel); + await opening; + } + assert.strictEqual(messages.find(message => message.command === 'pr.initialize')?.pullrequest?.title, prModel.title); + }); + + it('does not initialize a closed panel when its PR model resolves', async function () { + let resolveModel: (model: PullRequestModel) => void; + const pendingModel = new Promise(resolve => resolveModel = resolve); + const opening = openPanel(pendingModel); + webviewPanel.dispose(); + resolveModel!(prModel); + await opening; + + assert.strictEqual(messages.some(message => message.command === 'pr.initialize'), false); + sinon.assert.notCalled(getAssignableUsers); + }); + + it('does not overwrite a newer model when an older lookup finishes', async function () { + let resolveModel: (model: PullRequestModel) => void; + const pendingModel = new Promise(resolve => resolveModel = resolve); + const opening = openPanel(pendingModel); + await openPanel(); + messages.length = 0; + resolveModel!(prModel); + await opening; + + assert.strictEqual(messages.some(message => message.command === 'pr.initialize'), false); + sinon.assert.calledOnce(getAssignableUsers); + }); + + it('initializes the PR and loads checks before cold assignable users finish', async function () { + const opening = openPanel(); + await new Promise(resolve => setImmediate(resolve)); + try { + const initial = messages.find(message => message.command === 'pr.initialize')?.pullrequest; + assert.ok(initial, 'PR initialization must not wait for assignable users'); + assert.strictEqual(initial.title, prModel.title); + assert.strictEqual(initial.body, prModel.body); + assert.strictEqual(initial.canAssignCopilot, false); + assert.strictEqual(initial.canRequestCopilotReview, false); + assert.ok(messages.some(message => message.command === 'pr.update' && message.pullrequest?.status?.state === CheckState.Success)); + } finally { + resolveUsers({ [remote.remoteName]: [COPILOT_REVIEWER_ACCOUNT] }); + await opening; + await new Promise(resolve => setImmediate(resolve)); + } + + assert.ok(messages.some(message => message.command === 'pr.update' + && message.pullrequest?.canAssignCopilot === true + && message.pullrequest.canRequestCopilotReview === true)); + }); + + it('keeps Copilot actions unavailable when no assignable Copilot account exists', async function () { + resolveUsers({ [remote.remoteName]: [prModel.author] }); + await openPanel(); + + assert.ok(messages.some(message => message.command === 'pr.update' + && message.pullrequest?.canAssignCopilot === false + && message.pullrequest.canRequestCopilotReview === false)); + }); + + it('does not offer another Copilot review when one is already requested', async function () { + getReviewRequests.resolves([COPILOT_REVIEWER_ACCOUNT]); + resolveUsers({ [remote.remoteName]: [COPILOT_REVIEWER_ACCOUNT] }); + await openPanel(); + + assert.ok(messages.some(message => message.command === 'pr.update' + && message.pullrequest?.canAssignCopilot === true + && message.pullrequest.canRequestCopilotReview === false)); + }); + + it('logs assignable-user failures without preventing PR initialization', async function () { + const logError = sinon.spy(Logger, 'error'); + await openPanel(); + rejectUsers(new Error('Assignable users unavailable')); + await new Promise(resolve => setImmediate(resolve)); + + assert.ok(messages.some(message => message.command === 'pr.initialize')); + sinon.assert.calledWith(logError, 'Failed to update deferred assignable users: Assignable users unavailable', PullRequestOverviewPanel.ID); + assert.strictEqual(messages.some(message => message.pullrequest?.canAssignCopilot === true), false); + }); + + it('ignores assignable users from an older update', async function () { + await openPanel(); + getAssignableUsers.resolves({}); + await openPanel(); + messages.length = 0; + + resolveUsers({ [remote.remoteName]: [COPILOT_REVIEWER_ACCOUNT] }); + await new Promise(resolve => setImmediate(resolve)); + + assert.deepStrictEqual(messages, []); + }); + + it('ignores assignable users after the panel is disposed', async function () { + await openPanel(); + webviewPanel.dispose(); + messages.length = 0; + + resolveUsers({ [remote.remoteName]: [COPILOT_REVIEWER_ACCOUNT] }); + await new Promise(resolve => setImmediate(resolve)); + + assert.deepStrictEqual(messages, []); + }); + }); + describe('mergePullRequest', function () { it('prompts to delete the local branch when GitHub deletes branches after merge', async function () { repo.buildMetadata(repository => repository.delete_branch_on_merge!(true)); diff --git a/webviews/common/context.tsx b/webviews/common/context.tsx index 73e3aae5cf..64bcb29589 100644 --- a/webviews/common/context.tsx +++ b/webviews/common/context.tsx @@ -11,7 +11,7 @@ import { CloseResult, DescriptionResult, OpenCommitChangesArgs, OpenLocalFileArg import { IComment } from '../../src/common/comment'; import { EventType, ReviewEvent, SessionLinkInfo, TimelineEvent } from '../../src/common/timelineEvent'; import { IProjectItem, MergeMethod, PullRequestCheckStatus, ReadyForReview } from '../../src/github/interface'; -import { CancelCodingAgentReply, ChangeAssigneesReply, ChangeBaseReply, ConvertToDraftReply, DeleteReviewResult, FileUploadCompletedMessage, MergeArguments, MergeResult, ProjectItemsReply, PullRequest, ReadyForReviewReply, SubmitReviewArgs, SubmitReviewReply, UploadFilesReply } from '../../src/github/views'; +import { CancelCodingAgentReply, ChangeAssigneesReply, ChangeBaseReply, ConvertToDraftReply, DeleteReviewResult, FileUploadCompletedMessage, MergeArguments, MergeResult, ProjectItemsReply, PullRequest, PullRequestPreview, ReadyForReviewReply, SubmitReviewArgs, SubmitReviewReply, UploadFilesReply } from '../../src/github/views'; /** * Encode a {@linkcode Uint8Array} as a base64 string. Uses fixed-size chunks to @@ -32,6 +32,9 @@ function bytesToBase64(bytes: Uint8Array): string { const MAX_UPLOAD_SIZE_BYTES = 25 * 1024 * 1024; export class PRContext { + public preview: PullRequestPreview | undefined; + public onPreviewChange: ((preview: PullRequestPreview | undefined) => void) | null = null; + constructor( public pr: PullRequest | undefined = getState(), public onchange: ((ctx: PullRequest | undefined) => void) | null = null, @@ -531,6 +534,8 @@ export class PRContext { }; setPR = (pr: PullRequest | undefined) => { + this.preview = undefined; + this.onPreviewChange?.(undefined); this.pr = pr; setState(this.pr); if (this.onchange) { @@ -554,6 +559,12 @@ export class PRContext { handleMessage = (message: any) => { switch (message.command) { + case 'pr.preview': + if (!this.pr) { + this.preview = message.pullrequest; + this.onPreviewChange?.(this.preview); + } + return; case 'pr.clear': this.setPR(undefined); return; diff --git a/webviews/components/comment.tsx b/webviews/components/comment.tsx index adde992549..30242de31d 100644 --- a/webviews/components/comment.tsx +++ b/webviews/components/comment.tsx @@ -12,7 +12,7 @@ import { AuthorLink, Avatar } from './user'; import { IComment } from '../../src/common/comment'; import { CommentEvent, EventType, ReviewEvent } from '../../src/common/timelineEvent'; import { GithubItemStateEnum } from '../../src/github/interface'; -import { PullRequest, ReviewCommentContext, ReviewType } from '../../src/github/views'; +import { PullRequest, PullRequestPreview, ReviewCommentContext, ReviewType } from '../../src/github/views'; import { ariaAnnouncementForReview } from '../common/aria'; import { COMMENT_TEXTAREA_ID } from '../common/constants'; import PullRequestContext from '../common/context'; @@ -182,8 +182,14 @@ export function CommentView(commentProps: Props) { ); } +export const CommentPreview = (preview: PullRequestPreview) => ( + + + +); + type CommentBoxProps = { - for: IComment | ReviewEvent | PullRequest | CommentEvent; + for: IComment | ReviewEvent | PullRequest | CommentEvent | PullRequestPreview; header?: React.ReactChild; onFocus?: React.FocusEventHandler; onMouseEnter?: React.MouseEventHandler; @@ -191,7 +197,7 @@ type CommentBoxProps = { children?: React.ReactNode; }; -function isReviewEvent(comment: IComment | ReviewEvent | PullRequest | CommentEvent): comment is ReviewEvent { +function isReviewEvent(comment: CommentBoxProps['for']): comment is ReviewEvent { return (comment as ReviewEvent).authorAssociation !== undefined; } diff --git a/webviews/components/header.tsx b/webviews/components/header.tsx index 9e04929ead..bb4c39187a 100644 --- a/webviews/components/header.tsx +++ b/webviews/components/header.tsx @@ -10,7 +10,7 @@ import { AuthorLink, Avatar } from './user'; import { copilotEventToStatus, CopilotPRStatus, mostRecentCopilotEvent } from '../../src/common/copilot'; import { CopilotStartedEvent, TimelineEvent } from '../../src/common/timelineEvent'; import { GithubItemStateEnum, StateReason } from '../../src/github/interface'; -import { BaseContext, CodingAgentContext, OverviewContext, PullRequest } from '../../src/github/views'; +import { BaseContext, CodingAgentContext, OverviewContext, PullRequest, PullRequestPreview } from '../../src/github/views'; import { EDIT_TITLE_BUTTON_ID } from '../common/constants'; import PullRequestContext from '../common/context'; import { useStateProp } from '../common/hooks'; @@ -73,6 +73,43 @@ export function Header({ ); } +export function HeaderPreview(preview: PullRequestPreview) { + return <> +
+ +
+ +
+ + Loading... +
+ ; +} + +function TitleText({ titleHTML, number, url, context, onOpen }: Pick & { + context?: BaseContext; + onOpen?: () => void; +}) { + return

+ + {' '} + { + event.preventDefault(); + onOpen(); + } : undefined} + > + #{number} + +

; +} + interface TitleProps { title: string; titleHTML: string; @@ -128,21 +165,7 @@ function Title({ title, titleHTML, number, url, inEditMode, setEditMode, setCurr const displayTitle = (
-

- - {' '} - { - event.preventDefault(); - void openOnGitHub(); - }} - > - #{number} - -

+ {canEdit ?