diff --git a/src/@types/vscode.proposed.chatParticipantPrivate.d.ts b/src/@types/vscode.proposed.chatParticipantPrivate.d.ts index f0fad8b953..85f68a5569 100644 --- a/src/@types/vscode.proposed.chatParticipantPrivate.d.ts +++ b/src/@types/vscode.proposed.chatParticipantPrivate.d.ts @@ -126,6 +126,11 @@ declare module 'vscode' { */ readonly hasHooksEnabled: boolean; + /** + * Whether this request was submitted through Agents Voice Mode. + */ + readonly isVoiceModeInput?: boolean; + /** * When true, this request was initiated by the system (e.g. a terminal * command completion notification) rather than by the user typing a @@ -135,6 +140,41 @@ declare module 'vscode' { readonly isSystemInitiated?: boolean; } + /** + * A transient progress update intended for Voice Mode narration. + */ + export type ChatResponseVoiceProgressStage = 'investigating' | 'planning' | 'editing' | 'validating' | 'recovering'; + + export class ChatResponseVoiceProgressPart { + /** + * A stable identifier used to de-duplicate the progress update. + */ + readonly id: ChatResponseVoiceProgressStage; + /** + * The concise text to narrate. + */ + readonly value: string; + /** + * Creates a Voice Mode progress update. + * @param id A stable identifier used to de-duplicate the update. + * @param value The concise text to narrate. + */ + constructor(id: ChatResponseVoiceProgressStage, value: string); + } + + export interface ExtendedChatResponseParts { + ChatResponseVoiceProgressPart: ChatResponseVoiceProgressPart; + } + + export interface ChatResponseStream { + /** + * Reports transient progress for Voice Mode narration. + * @param id A stable identifier used to de-duplicate the update. + * @param value The concise text to narrate. + */ + voiceProgress(id: ChatResponseVoiceProgressStage, value: string): void; + } + export enum ChatRequestEditedFileEventKind { Keep = 1, Undo = 2, diff --git a/src/@types/vscode.proposed.chatSessionsProvider.d.ts b/src/@types/vscode.proposed.chatSessionsProvider.d.ts index 1003937f26..844de8f23d 100644 --- a/src/@types/vscode.proposed.chatSessionsProvider.d.ts +++ b/src/@types/vscode.proposed.chatSessionsProvider.d.ts @@ -717,7 +717,7 @@ declare module 'vscode' { readonly promo?: { readonly id: string; readonly discountPercent: number; - readonly endsAt: string; + readonly endsAt?: string; readonly message: string; }; readonly maxInputTokens?: number; diff --git a/src/github/githubRepository.ts b/src/github/githubRepository.ts index d3aad7ed7b..4eaba914b7 100644 --- a/src/github/githubRepository.ts +++ b/src/github/githubRepository.ts @@ -756,8 +756,10 @@ export class GitHubRepository extends Disposable { }); Logger.debug(`Fetch pull requests for branch - done`, this.id); - if (data?.repository && data.repository.pullRequests.nodes.length > 0) { - const prs = (await Promise.all(data.repository.pullRequests.nodes.map(node => parseGraphQLPullRequest(node, this)))).filter(pr => pr.head?.repo.owner === headOwner); + if (data?.repository) { + const nodes = [...data.repository.openPullRequests.nodes, ...data.repository.pullRequests.nodes] + .filter((pullRequest, index, pullRequests) => pullRequests.findIndex(candidate => candidate.number === pullRequest.number) === index); + const prs = (await Promise.all(nodes.map(node => parseGraphQLPullRequest(node, this)))).filter(pr => pr.head?.repo.owner === headOwner); if (prs.length === 0) { return undefined; } diff --git a/src/github/graphql.ts b/src/github/graphql.ts index 9515920a5c..f0bc75a5ea 100644 --- a/src/github/graphql.ts +++ b/src/github/graphql.ts @@ -906,6 +906,9 @@ export interface IssuesResponse { export interface PullRequestsResponse { repository: { + openPullRequests: { + nodes: PullRequest[] + } pullRequests: { nodes: PullRequest[] } diff --git a/src/github/queries.gql b/src/github/queries.gql index 42c84317c6..a62e492129 100644 --- a/src/github/queries.gql +++ b/src/github/queries.gql @@ -319,6 +319,11 @@ query PullRequest($owner: String!, $name: String!, $number: Int!) { query PullRequestForHead($owner: String!, $name: String!, $headRefName: String!) { repository(owner: $owner, name: $name) { + openPullRequests: pullRequests(first: 3, headRefName: $headRefName, states: [OPEN], orderBy: { field: CREATED_AT, direction: DESC }) { + nodes { + ...PullRequestFragment + } + } pullRequests(first: 3, headRefName: $headRefName, orderBy: { field: CREATED_AT, direction: DESC }) { nodes { ...PullRequestFragment diff --git a/src/github/queriesExtra.gql b/src/github/queriesExtra.gql index 3fa99a892f..b32727663b 100644 --- a/src/github/queriesExtra.gql +++ b/src/github/queriesExtra.gql @@ -330,6 +330,11 @@ query PullRequest($owner: String!, $name: String!, $number: Int!) { query PullRequestForHead($owner: String!, $name: String!, $headRefName: String!) { repository(owner: $owner, name: $name) { + openPullRequests: pullRequests(first: 3, headRefName: $headRefName, states: [OPEN], orderBy: { field: CREATED_AT, direction: DESC }) { + nodes { + ...PullRequestFragment + } + } pullRequests(first: 3, headRefName: $headRefName, orderBy: { field: CREATED_AT, direction: DESC }) { nodes { ...PullRequestFragment diff --git a/src/github/queriesLimited.gql b/src/github/queriesLimited.gql index 8c095dcf51..247397191a 100644 --- a/src/github/queriesLimited.gql +++ b/src/github/queriesLimited.gql @@ -292,6 +292,11 @@ query PullRequest($owner: String!, $name: String!, $number: Int!) { query PullRequestForHead($owner: String!, $name: String!, $headRefName: String!) { repository(owner: $owner, name: $name) { + openPullRequests: pullRequests(first: 3, headRefName: $headRefName, states: [OPEN], orderBy: { field: CREATED_AT, direction: DESC }) { + nodes { + ...PullRequestFragment + } + } pullRequests(first: 3, headRefName: $headRefName, orderBy: { field: CREATED_AT, direction: DESC }) { nodes { ...PullRequestFragment diff --git a/src/test/github/githubRepository.test.ts b/src/test/github/githubRepository.test.ts index e41ba80e0b..e83e87f550 100644 --- a/src/test/github/githubRepository.test.ts +++ b/src/test/github/githubRepository.test.ts @@ -16,6 +16,7 @@ import { MockExtensionContext } from '../mocks/mockExtensionContext'; import { GitHubManager } from '../../authentication/githubServer'; import { GitHubServerType } from '../../common/authentication'; import { CheckState, PullRequestCheckStatus } from '../../github/interface'; +import { PullRequestBuilder as GraphQLPullRequestBuilder } from '../builders/graphql/pullRequestBuilder'; describe('GitHubRepository', function () { let sinon: SinonSandbox; @@ -132,6 +133,43 @@ describe('GitHubRepository', function () { }); }); + describe('getPullRequestForBranch', function () { + it('prefers an open pull request over newer merged pull requests', async function () { + const url = 'https://github.com/some/repo'; + const remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom); + const rootUri = Uri.file('C:\\users\\test\\repo'); + const repo = new GitHubRepository(1, remote, rootUri, credentialStore, telemetry, true); + const openPullRequest = new GraphQLPullRequestBuilder() + .repository(repository => repository.pullRequest(pullRequest => pullRequest + .number(7231) + .state('OPEN'))) + .build().repository!.pullRequest!; + const mergedPullRequest = new GraphQLPullRequestBuilder() + .repository(repository => repository.pullRequest(pullRequest => pullRequest + .number(7492) + .state('MERGED') + .merged(true))) + .build().repository!.pullRequest!; + sinon.stub(repo, 'ensure').resolves(repo); + sinon.stub(repo, 'query').resolves({ + data: { + repository: { + openPullRequests: { + nodes: [openPullRequest], + }, + pullRequests: { + nodes: [mergedPullRequest], + }, + }, + }, + } as never); + + const pullRequest = await repo.getPullRequestForBranch('feature', 'me'); + + assert.strictEqual(pullRequest?.number, 7231); + }); + }); + describe('computeAwaitingApprovalStatuses', function () { function callComputeAwaitingApprovalStatuses( repo: GitHubRepository, diff --git a/src/test/view/reviewManager.test.ts b/src/test/view/reviewManager.test.ts index 1885f6ff54..0818a90ffa 100644 --- a/src/test/view/reviewManager.test.ts +++ b/src/test/view/reviewManager.test.ts @@ -6,10 +6,12 @@ import { default as assert } from 'assert'; import { SinonFakeTimers, SinonSandbox, createSandbox } from 'sinon'; import * as vscode from 'vscode'; +import type { Branch } from '../../api/api'; import { GitApiImpl } from '../../api/api1'; import { ITelemetry } from '../../common/telemetry'; import { CredentialStore } from '../../github/credentials'; import { FolderRepositoryManager } from '../../github/folderRepositoryManager'; +import { PullRequestMetadata } from '../../github/pullRequestGitHelper'; import { PullRequestModel } from '../../github/pullRequestModel'; import { RepositoriesManager } from '../../github/repositoriesManager'; import { CreatePullRequestHelper } from '../../view/createPullRequestHelper'; @@ -199,6 +201,104 @@ describe('ReviewManager polling', function () { assert.strictEqual(updateStateStub.called, true, 'poll should refresh state when active PR may be stale'); }); + it('uses the explicitly checked out pull request for the checked out branch', async function () { + await repository.createBranch('feature', true, 'head-sha'); + sinon.stub(manager, 'updateRepositories').resolves(true); + const localMetadata = sinon.stub(manager, 'getMatchingPullRequestMetadataForBranch').resolves({ + owner: 'owner', + repositoryName: 'repo', + prNumber: 7492, + }); + const requestedPullRequest = { + number: 7231, + remote: { + owner: 'owner', + repositoryName: 'repo', + }, + } as PullRequestModel; + const internal = reviewManager as unknown as { + _switchedToPullRequest?: PullRequestModel; + _switchedToPullRequestBranch?: string; + validateState(silent: boolean, updateLayout: boolean): Promise; + resolvePullRequest(metadata: PullRequestMetadata, useCache: boolean): Promise; + checkGitHubForPrBranch(branch: Branch): Promise; + }; + internal._switchedToPullRequest = requestedPullRequest; + internal._switchedToPullRequestBranch = 'feature'; + const resolvePullRequest = sinon.stub(internal, 'resolvePullRequest').resolves(undefined); + const checkGitHubForPrBranch = sinon.stub(internal, 'checkGitHubForPrBranch').resolves(undefined); + + await internal.validateState(true, false); + + assert.strictEqual(localMetadata.called, false); + assert.deepStrictEqual(resolvePullRequest.firstCall.args[0], { + owner: 'owner', + repositoryName: 'repo', + prNumber: 7231, + }); + assert.strictEqual(checkGitHubForPrBranch.called, false); + }); + + it('rechecks GitHub when active pull request metadata was not persisted', async function () { + await repository.createBranch('feature', true, 'head-sha'); + sinon.stub(manager, 'updateRepositories').resolves(true); + sinon.stub(manager, 'getMatchingPullRequestMetadataForBranch').resolves(undefined); + sinon.stub(manager, 'activePullRequest').get(() => ({ number: 7231 } as PullRequestModel)); + const internal = reviewManager as unknown as { + _cachedBranchName?: string; + validateState(silent: boolean, updateLayout: boolean): Promise; + hasNewPullRequests(): Promise; + checkGitHubForPrBranch(branch: Branch): Promise; + resolvePullRequest(metadata: PullRequestMetadata, useCache: boolean): Promise; + clear(quitReviewMode: boolean): Promise; + }; + internal._cachedBranchName = 'feature'; + sinon.stub(internal, 'hasNewPullRequests').resolves(false); + const pullRequestModel = {} as PullRequestModel; + const checkGitHubForPrBranch = sinon.stub(internal, 'checkGitHubForPrBranch').resolves({ + owner: 'owner', + repositoryName: 'repo', + prNumber: 7231, + model: pullRequestModel, + }); + const resolvePullRequest = sinon.stub(internal, 'resolvePullRequest').resolves(undefined); + const clear = sinon.stub(internal, 'clear').resolves(); + + await internal.validateState(true, false); + + assert.strictEqual(checkGitHubForPrBranch.calledOnce, true); + assert.deepStrictEqual(resolvePullRequest.firstCall.args[0], { + owner: 'owner', + repositoryName: 'repo', + prNumber: 7231, + model: pullRequestModel, + }); + assert.strictEqual(clear.called, false); + }); + + it('keeps the active pull request when its metadata recheck fails', async function () { + await repository.createBranch('feature', true, 'head-sha'); + sinon.stub(manager, 'updateRepositories').resolves(true); + sinon.stub(manager, 'getMatchingPullRequestMetadataForBranch').resolves(undefined); + sinon.stub(manager, 'activePullRequest').get(() => ({ number: 7231 } as PullRequestModel)); + const internal = reviewManager as unknown as { + _cachedBranchName?: string; + validateState(silent: boolean, updateLayout: boolean): Promise; + hasNewPullRequests(): Promise; + checkGitHubForPrBranch(branch: Branch): Promise; + clear(quitReviewMode: boolean): Promise; + }; + internal._cachedBranchName = 'feature'; + sinon.stub(internal, 'hasNewPullRequests').resolves(false); + const checkGitHubForPrBranch = sinon.stub(internal, 'checkGitHubForPrBranch').resolves(undefined); + const clear = sinon.stub(internal, 'clear').resolves(); + + await internal.validateState(true, false); + + assert.strictEqual(checkGitHubForPrBranch.calledOnce, true); + assert.strictEqual(clear.called, false); + }); + it('caps backoff at the maximum interval', async function () { sinon.stub(reviewManager, 'updateState').resolves(); diff --git a/src/view/reviewManager.ts b/src/view/reviewManager.ts index 86a0ecd6ca..b1460767b3 100644 --- a/src/view/reviewManager.ts +++ b/src/view/reviewManager.ts @@ -101,6 +101,7 @@ export class ReviewManager extends Disposable { * Used to enter review mode for this PR regardless of its state (open/closed/merged). */ private _switchedToPullRequest?: PullRequestModel; + private _switchedToPullRequestBranch?: string; /** * Track whether this repository is currently selected in the UI. * Used to show/hide the status bar item based on repository selection. @@ -642,30 +643,64 @@ export class ReviewManager extends Disposable { return; } - let matchingPullRequestMetadata = await this._folderRepoManager.getMatchingPullRequestMetadataForBranch(); + let switchedToPullRequest: PullRequestModel | undefined; + if (this._switchedToPullRequest && this._switchedToPullRequestBranch && this._switchedToPullRequestBranch === branch.name) { + switchedToPullRequest = this._switchedToPullRequest; + } else { + this._switchedToPullRequest = undefined; + this._switchedToPullRequestBranch = undefined; + } + + let matchingPullRequestMetadata = switchedToPullRequest ? { + owner: switchedToPullRequest.remote.owner, + repositoryName: switchedToPullRequest.remote.repositoryName, + prNumber: switchedToPullRequest.number, + } : await this._folderRepoManager.getMatchingPullRequestMetadataForBranch(); if (!matchingPullRequestMetadata) { Logger.appendLine(`No matching pull request metadata found locally for current branch ${branch.name}`, this.id); } - // One-shot self-heal: when local metadata exists for this branch, re-check GitHub once - // (per branch) in case the local metadata points to a stale closed PR. If GitHub returns - // a result, it overwrites the local metadata via associateBranchWithPullRequest. Subsequent - // checks for the same branch fall back to the branch-change/new-PR cache. - const needsStaleMetadataCheck = !!matchingPullRequestMetadata && !!branch.name && !this._staleMetadataCheckedBranches.has(branch.name); - if (this._cachedBranchName !== branch.name || await this.hasNewPullRequests() || needsStaleMetadataCheck) { + const activePullRequest = this._folderRepoManager.activePullRequest; + const branchChanged = this._cachedBranchName !== branch.name; + // Verify local metadata once per branch so stale associations can self-heal. + const shouldVerifyLocalMetadata = !switchedToPullRequest + && !!matchingPullRequestMetadata + && !!branch.name + && !this._staleMetadataCheckedBranches.has(branch.name); + // If an active PR loses its local metadata, retry GitHub before clearing the view. + const shouldRecoverMissingMetadata = !matchingPullRequestMetadata + && !!activePullRequest + && !branchChanged; + + let shouldCheckGitHub = false; + if (!switchedToPullRequest) { + shouldCheckGitHub = branchChanged || shouldRecoverMissingMetadata || shouldVerifyLocalMetadata; + if (!shouldCheckGitHub) { + shouldCheckGitHub = await this.hasNewPullRequests(); + } + } + + if (shouldCheckGitHub) { const metadataFromGithub = await this.checkGitHubForPrBranch(branch); if (metadataFromGithub) { matchingPullRequestMetadata = metadataFromGithub; } - if (needsStaleMetadataCheck && branch.name) { + if (shouldVerifyLocalMetadata && branch.name) { this._staleMetadataCheckedBranches.add(branch.name); } + } else if (switchedToPullRequest) { + Logger.appendLine(`Skipping GitHub check for branch ${branch.name}: using explicitly selected pull request #${switchedToPullRequest.number}`, this.id); } else { Logger.appendLine(`Skipping GitHub check for branch ${branch.name}: no new PRs since last check`, this.id); } this._cachedBranchName = branch.name; if (!matchingPullRequestMetadata) { + if (shouldRecoverMissingMetadata && activePullRequest) { + Logger.appendLine(`Keeping active pull request #${activePullRequest.number} after its branch metadata could not be refreshed`, this.id); + this._lastCommitSha = oldLastCommitSha; + return; + } Logger.appendLine( `No matching pull request metadata found on GitHub for current branch ${branch.name}`, this.id ); @@ -689,7 +724,7 @@ export class ReviewManager extends Disposable { Logger.appendLine(`Resolved PR #${matchingPullRequestMetadata.prNumber}, state is ${pr.state}`, this.id); // Check if the PR is open, if not, check if there's another PR from the same branch on GitHub - if (pr.state !== GithubItemStateEnum.Open) { + if (!switchedToPullRequest && pr.state !== GithubItemStateEnum.Open) { const metadataFromGithub = await this.checkGitHubForPrBranch(branch); if (metadataFromGithub && metadataFromGithub?.prNumber !== pr.number) { const prFromGitHub = await this.resolvePullRequest(metadataFromGithub, false); @@ -1347,6 +1382,7 @@ export class ReviewManager extends Disposable { this.showStatusBarIfSelected(); this.switchingToReviewMode = true; this._switchedToPullRequest = pr; + this._switchedToPullRequestBranch = undefined; try { await vscode.window.withProgress({ location: vscode.ProgressLocation.Notification }, async (progress) => { @@ -1410,6 +1446,7 @@ export class ReviewManager extends Disposable { } private setStatusForPr(pr: PullRequestModel) { + this._switchedToPullRequestBranch = this._repository.state.HEAD?.name; this.switchingToReviewMode = false; this.justSwitchedToReviewMode = true; this.statusBarItem.text = vscode.l10n.t('Pull Request #{0}', pr.number); @@ -1501,6 +1538,7 @@ export class ReviewManager extends Disposable { this._prNumber = undefined; this._folderRepoManager.activePullRequest = undefined; this._switchedToPullRequest = undefined; + this._switchedToPullRequestBranch = undefined; if (this._statusBarItem) { this._statusBarItem.hide();