Skip to content

Commit 545a0ba

Browse files
Copilotalexr00
andauthored
Add "Request review from Copilot" button to Reviewers section (#8283)
* Initial plan * Initial plan for Copilot review request feature Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> * Add "Request review from Copilot" feature - Added addReviewerCopilot backend handler in pullRequestOverview.ts - Added canRequestCopilotReview property to PullRequest interface - Added Copilot review button in Reviewers section UI - Button shows only when Copilot is available and not already a reviewer - Follows same pattern as existing "Assign Copilot" feature Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> * Cache assignable users to avoid redundant API calls - Store assignable users in _assignableUsers member variable - Reuse cached data in addReviewerCopilot instead of fetching again - Improves performance by avoiding duplicate getAssignableUsers() calls Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> * Fix up * Add ChangeReviewersReply type for addReviewerCopilot reply - Created ChangeReviewersReply interface in views.ts - Updated addReviewerCopilot to use typed reply instead of inline object - Fixed copilotUser reference instead of using removed COPILOT_REVIEWER_ACCOUNT - Removed unused imports Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> * Use type in sidebar --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
1 parent 3008bf9 commit 545a0ba

File tree

11 files changed

+121
-26
lines changed

11 files changed

+121
-26
lines changed

src/common/copilot.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { EventType, TimelineEvent } from './timelineEvent';
7+
import { AccountType, IAccount } from '../github/interface';
78

89
export const COPILOT_SWE_AGENT = 'copilot-swe-agent';
910
export const COPILOT_CLOUD_AGENT = 'copilot-cloud-agent';
@@ -16,6 +17,15 @@ export const COPILOT_LOGINS = [
1617
'Copilot'
1718
];
1819

20+
export const COPILOT_REVIEWER_ACCOUNT: IAccount = {
21+
login: COPILOT_REVIEWER,
22+
id: COPILOT_REVIEWER_ID,
23+
url: '',
24+
avatarUrl: '',
25+
name: 'Copilot',
26+
accountType: AccountType.Bot
27+
};
28+
1929
export enum CopilotPRStatus {
2030
None = 0,
2131
Started = 1,

src/github/graphql.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,11 @@ export function isTeam(x: Actor | Team | Node | undefined | null): x is Team {
130130
return !!asTeam && (asTeam?.slug !== undefined);
131131
}
132132

133+
export function isBot(x: Actor | Team | Node | undefined | null): x is Actor {
134+
const asBot = x as Partial<Actor>;
135+
return !!asBot && !!asBot.id?.startsWith('BOT_');
136+
}
137+
133138
export interface Team {
134139
avatarUrl: string;
135140
name: string;
@@ -347,6 +352,14 @@ export interface GetReviewRequestsResponse {
347352
} | null;
348353
}
349354

355+
export interface AddReviewRequestResponse {
356+
requestReviews: {
357+
pullRequest: {
358+
id: string;
359+
};
360+
} | null;
361+
}
362+
350363
export interface PullRequestState {
351364
repository: {
352365
pullRequest: {

src/github/issueOverview.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
233233
isEnterprise: issue.githubRepository.remote.isEnterprise,
234234
isDarkTheme: vscode.window.activeColorTheme.kind === vscode.ColorThemeKind.Dark,
235235
canAssignCopilot: assignableUsers.find(user => COPILOT_ACCOUNTS[user.login]) !== undefined,
236+
canRequestCopilotReview: false,
236237
reactions: issue.item.reactions,
237238
isAuthor: issue.author.login === currentUser.login,
238239
};

src/github/pullRequestModel.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { GitHubRepository, GraphQLError, GraphQLErrorType } from './githubReposi
1717
import {
1818
AddCommentResponse,
1919
AddReactionResponse,
20+
AddReviewRequestResponse as AddReviewsResponse,
2021
AddReviewThreadResponse,
2122
DeleteReactionResponse,
2223
DeleteReviewResponse,
@@ -1247,21 +1248,26 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
12471248
input.botIds = reviewers.filter(r => r.accountType === AccountType.Bot).map(r => r.id);
12481249
}
12491250

1250-
const { data } = await mutate<GetReviewRequestsResponse>({
1251+
const { data } = await mutate<AddReviewsResponse>({
12511252
mutation: schema.AddReviewers,
12521253
variables: {
12531254
input
12541255
},
12551256
});
12561257

1257-
if (!data?.repository) {
1258+
if (!data?.requestReviews) {
12581259
Logger.error('Unexpected null repository while getting review requests', PullRequestModel.ID);
12591260
return;
12601261
}
12611262

1262-
const newReviewers: (IAccount | ITeam)[] = parseGraphQLReviewers(data, this.githubRepository);
1263-
if (this.reviewers?.length !== newReviewers.length || (this.reviewers.some(r => !newReviewers.some(rr => rr.id === r.id)))) {
1264-
this.reviewers = newReviewers;
1263+
const newReviewers: (IAccount | ITeam)[] = [...reviewers, ...teamReviewers].filter(r => !this.reviewers?.some(rr => rr.id === r.id));
1264+
if (this.reviewers?.length !== newReviewers.length) {
1265+
if (!this.reviewers) {
1266+
this.reviewers = newReviewers;
1267+
} else {
1268+
this.reviewers.push(...newReviewers);
1269+
}
1270+
this.reviewers = [...this.reviewers, ...newReviewers];
12651271
this._onDidChange.fire({ reviewers: true });
12661272
}
12671273
}

src/github/pullRequestOverview.ts

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ import { isCopilotOnMyBehalf, PullRequestModel } from './pullRequestModel';
2525
import { PullRequestReviewCommon, ReviewContext } from './pullRequestReviewCommon';
2626
import { pickEmail, reviewersQuickPick } from './quickPicks';
2727
import { parseReviewers } from './utils';
28-
import { CancelCodingAgentReply, DeleteReviewResult, MergeArguments, MergeResult, PullRequest, ReviewType } from './views';
29-
import { IComment } from '../common/comment';
30-
import { COPILOT_SWE_AGENT, copilotEventToStatus, CopilotPRStatus, mostRecentCopilotEvent } from '../common/copilot';
28+
import { CancelCodingAgentReply, ChangeReviewersReply, DeleteReviewResult, MergeArguments, MergeResult, PullRequest, ReviewType } from './views';
29+
import { COPILOT_ACCOUNTS, IComment } from '../common/comment';
30+
import { COPILOT_REVIEWER, COPILOT_REVIEWER_ACCOUNT, COPILOT_SWE_AGENT, copilotEventToStatus, CopilotPRStatus, mostRecentCopilotEvent } from '../common/copilot';
3131
import { commands, contexts } from '../common/executeCommands';
3232
import { disposeAll } from '../common/lifecycle';
3333
import Logger from '../common/logger';
@@ -54,6 +54,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
5454
private _repositoryDefaultBranch: string;
5555
private _existingReviewers: ReviewState[] = [];
5656
private _teamsCount = 0;
57+
private _assignableUsers: { [key: string]: IAccount[] } = {};
5758

5859
private _prListeners: vscode.Disposable[] = [];
5960
private _isUpdating: boolean = false;
@@ -257,7 +258,8 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
257258
mergeability,
258259
emailForCommit,
259260
coAuthors,
260-
hasReviewDraft
261+
hasReviewDraft,
262+
assignableUsers
261263
] = await Promise.all([
262264
this._folderRepositoryManager.resolvePullRequest(
263265
pullRequestModel.remote.owner,
@@ -279,6 +281,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
279281
this._folderRepositoryManager.getPreferredEmail(pullRequestModel),
280282
pullRequestModel.getCoAuthors(),
281283
pullRequestModel.validateDraftMode(),
284+
this._folderRepositoryManager.getAssignableUsers()
282285
]);
283286
if (!pullRequest) {
284287
throw new Error(
@@ -290,6 +293,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
290293
this.registerPrListeners();
291294
this._repositoryDefaultBranch = defaultBranch!;
292295
this._teamsCount = orgTeamsCount;
296+
this._assignableUsers = assignableUsers;
293297
this.setPanelTitle(`Pull Request #${pullRequestModel.number.toString()}`);
294298

295299
const isCurrentlyCheckedOut = pullRequestModel.equals(this._folderRepositoryManager.activePullRequest);
@@ -302,12 +306,16 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
302306
const reviewState = this.getCurrentUserReviewState(this._existingReviewers, currentUser);
303307

304308
Logger.debug('pr.initialize', PullRequestOverviewPanel.ID);
305-
const baseContext = this.getInitializeContext(currentUser, pullRequest, timelineEvents, repositoryAccess, viewerCanEdit, []);
309+
const users = this._assignableUsers[pullRequestModel.remote.remoteName] ?? [];
310+
const copilotUser = users.find(user => COPILOT_ACCOUNTS[user.login]);
311+
const isCopilotAlreadyReviewer = this._existingReviewers.some(reviewer => !isITeam(reviewer.reviewer) && reviewer.reviewer.login === COPILOT_REVIEWER);
312+
const baseContext = this.getInitializeContext(currentUser, pullRequest, timelineEvents, repositoryAccess, viewerCanEdit, users);
306313

307314
this.preLoadInfoNotRequiredForOverview(pullRequest);
308315

309316
const context: Partial<PullRequest> = {
310317
...baseContext,
318+
canRequestCopilotReview: copilotUser !== undefined && !isCopilotAlreadyReviewer,
311319
isCurrentlyCheckedOut: isCurrentlyCheckedOut,
312320
isRemoteBaseDeleted: pullRequest.isRemoteBaseDeleted,
313321
base: pullRequest.base.label,
@@ -415,6 +423,8 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
415423
return this.gotoChangesSinceReview(message);
416424
case 'pr.re-request-review':
417425
return this.reRequestReview(message);
426+
case 'pr.add-reviewer-copilot':
427+
return this.addReviewerCopilot(message);
418428
case 'pr.revert':
419429
return this.revert(message);
420430
case 'pr.open-session-log':
@@ -750,6 +760,26 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
750760
return PullRequestReviewCommon.reRequestReview(this.getReviewContext(), message);
751761
}
752762

763+
private async addReviewerCopilot(message: IRequestMessage<void>): Promise<void> {
764+
try {
765+
const copilotUser = this._assignableUsers[this._item.remote.remoteName]?.find(user => COPILOT_ACCOUNTS[user.login]);
766+
if (copilotUser) {
767+
await this._item.requestReview([COPILOT_REVIEWER_ACCOUNT], []);
768+
const newReviewers = await this._item.getReviewRequests();
769+
this._existingReviewers = parseReviewers(newReviewers!, await this._item.getTimelineEvents(), this._item.author);
770+
const reply: ChangeReviewersReply = {
771+
reviewers: this._existingReviewers
772+
};
773+
this._replyMessage(message, reply);
774+
} else {
775+
this._throwError(message, 'Copilot reviewer not found.');
776+
}
777+
} catch (e) {
778+
vscode.window.showErrorMessage(formatError(e));
779+
this._throwError(message, formatError(e));
780+
}
781+
}
782+
753783
private async revert(message: IRequestMessage<string>): Promise<void> {
754784
await this._folderRepositoryManager.createPullRequestHelper.revert(this._telemetry, this._extensionUri, this._folderRepositoryManager, this._item, async (pullRequest) => {
755785
const result: Partial<PullRequest> = { revertable: !pullRequest };

src/github/quickPicks.ts

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ import { Buffer } from 'buffer';
88
import * as vscode from 'vscode';
99
import { FolderRepositoryManager } from './folderRepositoryManager';
1010
import { GitHubRepository, TeamReviewerRefreshKind } from './githubRepository';
11-
import { AccountType, IAccount, ILabel, IMilestone, IProject, isISuggestedReviewer, isITeam, ISuggestedReviewer, ITeam, reviewerId, ReviewState } from './interface';
11+
import { IAccount, ILabel, IMilestone, IProject, isISuggestedReviewer, isITeam, ISuggestedReviewer, ITeam, reviewerId, ReviewState } from './interface';
1212
import { IssueModel } from './issueModel';
1313
import { DisplayLabel } from './views';
1414
import { COPILOT_ACCOUNTS } from '../common/comment';
15-
import { COPILOT_REVIEWER, COPILOT_REVIEWER_ID, COPILOT_SWE_AGENT } from '../common/copilot';
15+
import { COPILOT_REVIEWER, COPILOT_REVIEWER_ACCOUNT, COPILOT_SWE_AGENT } from '../common/copilot';
1616
import { emojify, ensureEmojis } from '../common/emoji';
1717
import Logger from '../common/logger';
1818
import { DataUri } from '../common/uri';
@@ -191,15 +191,7 @@ async function getReviewersQuickPickItems(folderRepositoryManager: FolderReposit
191191

192192
// If we removed the coding agent, add the Copilot reviewer instead
193193
if (hasCopilotSweAgent && !existingReviewers.find(user => (user.reviewer as IAccount).login === COPILOT_REVIEWER)) {
194-
const copilotReviewer: IAccount = {
195-
login: COPILOT_REVIEWER,
196-
id: COPILOT_REVIEWER_ID,
197-
url: '',
198-
avatarUrl: '',
199-
name: COPILOT_ACCOUNTS[COPILOT_REVIEWER]?.name ?? 'Copilot',
200-
accountType: AccountType.Bot
201-
};
202-
assignableUsers.push(copilotReviewer);
194+
assignableUsers.push(COPILOT_REVIEWER_ACCOUNT);
203195
}
204196

205197
// Suggested reviewers

src/github/utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -696,7 +696,7 @@ export function parseGraphQLReviewers(data: GraphQL.GetReviewRequestsResponse, r
696696
if (GraphQL.isTeam(reviewer.requestedReviewer)) {
697697
const team: ITeam = parseTeam(reviewer.requestedReviewer, repository);
698698
reviewers.push(team);
699-
} else if (GraphQL.isAccount(reviewer.requestedReviewer)) {
699+
} else if (GraphQL.isAccount(reviewer.requestedReviewer) || GraphQL.isBot(reviewer.requestedReviewer)) {
700700
const account: IAccount = parseAccount(reviewer.requestedReviewer, repository);
701701
reviewers.push(account);
702702
}

src/github/views.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ export interface Issue {
6767
isDarkTheme: boolean;
6868
isEnterprise: boolean;
6969
canAssignCopilot: boolean;
70+
canRequestCopilotReview: boolean;
7071
reactions: Reaction[];
7172
busy?: boolean;
7273
}
@@ -120,6 +121,10 @@ export interface ChangeAssigneesReply {
120121
events: TimelineEvent[];
121122
}
122123

124+
export interface ChangeReviewersReply {
125+
reviewers: ReviewState[];
126+
}
127+
123128
export interface SubmitReviewReply {
124129
events?: TimelineEvent[];
125130
reviewedEvent: ReviewEvent | CommentEvent;

webviews/common/context.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ export class PRContext {
9393
public readyForReviewAndMerge = (args: { mergeMethod: MergeMethod }): Promise<ReadyForReview> => this.postMessage({ command: 'pr.readyForReviewAndMerge', args });
9494

9595
public addReviewers = () => this.postMessage({ command: 'pr.change-reviewers' });
96+
public addReviewerCopilot = () => this.postMessage({ command: 'pr.add-reviewer-copilot' });
9697
public changeProjects = (): Promise<ProjectItemsReply> => this.postMessage({ command: 'pr.change-projects' });
9798
public removeProject = (project: IProjectItem) => this.postMessage({ command: 'pr.remove-project', args: project });
9899
public addMilestone = () => this.postMessage({ command: 'pr.add-milestone' });

webviews/components/sidebar.tsx

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import { closeIcon, copilotIcon, settingsIcon } from './icon';
88
import { Reviewer } from './reviewer';
99
import { COPILOT_LOGINS } from '../../src/common/copilot';
1010
import { gitHubLabelColor } from '../../src/common/utils';
11-
import { IAccount, IMilestone, IProjectItem, reviewerId, reviewerLabel, ReviewState } from '../../src/github/interface';
12-
import { PullRequest } from '../../src/github/views';
11+
import { IAccount, IMilestone, IProjectItem, isITeam, reviewerId, reviewerLabel, ReviewState } from '../../src/github/interface';
12+
import { ChangeReviewersReply, PullRequest } from '../../src/github/views';
1313
import PullRequestContext from '../common/context';
1414
import { Label } from '../common/label';
1515
import { AuthorLink, Avatar } from '../components/user';
@@ -53,9 +53,10 @@ function Section({
5353
);
5454
}
5555

56-
export default function Sidebar({ reviewers, labels, hasWritePermission, isIssue, projectItems: projects, milestone, assignees, canAssignCopilot }: PullRequest) {
56+
export default function Sidebar({ reviewers, labels, hasWritePermission, isIssue, projectItems: projects, milestone, assignees, canAssignCopilot, canRequestCopilotReview }: PullRequest) {
5757
const {
5858
addReviewers,
59+
addReviewerCopilot,
5960
addAssignees,
6061
addAssigneeYourself,
6162
addAssigneeCopilot,
@@ -68,8 +69,10 @@ export default function Sidebar({ reviewers, labels, hasWritePermission, isIssue
6869
} = useContext(PullRequestContext);
6970

7071
const [assigningCopilot, setAssigningCopilot] = useState(false);
72+
const [requestingCopilotReview, setRequestingCopilotReview] = useState(false);
7173

7274
const shouldShowCopilotButton = canAssignCopilot && assignees.every(assignee => !COPILOT_LOGINS.includes(assignee.login));
75+
const shouldShowCopilotReviewButton = canRequestCopilotReview && reviewers.every(reviewer => !isITeam(reviewer.reviewer) && !COPILOT_LOGINS.includes(reviewer.reviewer.login));
7376

7477
const updateProjects = async () => {
7578
const newProjects = await changeProjects();
@@ -83,10 +86,43 @@ export default function Sidebar({ reviewers, labels, hasWritePermission, isIssue
8386
id="reviewers"
8487
title="Reviewers"
8588
hasWritePermission={hasWritePermission}
86-
onHeaderClick={async () => {
89+
onHeaderClick={async (e) => {
90+
const target = e?.target as HTMLElement;
91+
if (target?.closest && target.closest('#request-copilot-review-btn')) {
92+
return;
93+
}
8794
const newReviewers = await addReviewers();
8895
updatePR({ reviewers: newReviewers.reviewers });
8996
}}
97+
iconButtonGroup={hasWritePermission && (
98+
<div className="icon-button-group">
99+
{shouldShowCopilotReviewButton ? (
100+
<button
101+
id="request-copilot-review-btn"
102+
className="icon-button"
103+
title="Request review from Copilot"
104+
disabled={requestingCopilotReview}
105+
onClick={async (e) => {
106+
e.stopPropagation();
107+
setRequestingCopilotReview(true);
108+
try {
109+
const newReviewers: ChangeReviewersReply = await addReviewerCopilot();
110+
updatePR({ reviewers: newReviewers.reviewers });
111+
} finally {
112+
setRequestingCopilotReview(false);
113+
}
114+
}}>
115+
{copilotIcon}
116+
</button>
117+
) : null}
118+
<button
119+
className="icon-button"
120+
title="Add Reviewers"
121+
>
122+
{settingsIcon}
123+
</button>
124+
</div>
125+
)}
90126
>
91127
{reviewers && reviewers.length ? (
92128
reviewers.map(state => (

0 commit comments

Comments
 (0)