Skip to content

Commit 58d69dc

Browse files
authored
devops(revision): read the issue from its own repo, move collect-review to a file (#43946)
1 parent 3db8701 commit 58d69dc

2 files changed

Lines changed: 204 additions & 128 deletions

File tree

‎.github/actions/collect-review/action.yml‎

Lines changed: 4 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ description: >
55
is for, with its text and the comments on it; the pull request, with its description and
66
commits; and the comments, reviews and unresolved review threads on the pull request. Only the
77
comments of people with write access, the maintainers, and of the bot itself get through. Fails
8-
when the maintainers left nothing to address.
8+
when the maintainers left nothing to address. collect-review.ts next to this file does the work.
99
inputs:
1010
pull-request:
1111
description: 'Number of the pull request'
@@ -39,138 +39,14 @@ runs:
3939
id: collect
4040
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
4141
env:
42+
ACTION_PATH: ${{ github.action_path }}
4243
PULL_NUMBER: ${{ inputs.pull-request }}
4344
DIR: ${{ inputs.dir }}
4445
with:
4546
github-token: ${{ inputs.token }}
4647
script: |
47-
const fs = require('fs');
48-
const path = require('path');
49-
const { PULL_NUMBER, DIR } = process.env;
50-
const { owner, repo } = context.repo;
51-
const pull_number = Number(PULL_NUMBER);
52-
fs.mkdirSync(DIR, { recursive: true });
53-
54-
const { data: pr } = await github.rest.pulls.get({ owner, repo, pull_number });
55-
const { data: { commit: { sha: base } } } = await github.rest.repos.getBranch({ owner, repo, branch: pr.base.ref });
56-
const commits = await github.paginate(github.rest.pulls.listCommits, { owner, repo, pull_number, per_page: 100 });
57-
const fields = 'author { __typename login } body createdAt url';
58-
const { repository: { pullRequest: data } } = await github.graphql(`
59-
query($owner: String!, $repo: String!, $number: Int!) {
60-
repository(owner: $owner, name: $repo) {
61-
pullRequest(number: $number) {
62-
closingIssuesReferences(first: 1) { nodes { number } }
63-
comments(last: 100) { totalCount nodes { ${fields} } }
64-
reviews(last: 100) { totalCount nodes { ${fields} state } }
65-
reviewThreads(last: 100) {
66-
totalCount
67-
nodes {
68-
isResolved isOutdated path line originalLine
69-
comments(first: 100) { totalCount nodes { ${fields} fullDatabaseId diffHunk } }
70-
}
71-
}
72-
}
73-
}
74-
}`, { owner, repo, number: pull_number });
75-
76-
// The issue that the pull request closes. A coverage PR closes none: its branch names the issue.
77-
const issueNumber = data.closingIssuesReferences.nodes[0]?.number ?? Number(pr.head.ref.match(/^triage\/(\d+)\//)?.[1] ?? 0);
78-
const issueData = issueNumber ? (await github.graphql(`
79-
query($owner: String!, $repo: String!, $number: Int!) {
80-
repository(owner: $owner, name: $repo) {
81-
issue(number: $number) { number url title body author { login } comments(last: 100) { totalCount nodes { ${fields} } } }
82-
}
83-
}`, { owner, repo, number: issueNumber })).repository.issue : null;
84-
85-
const connections = [['comments on the PR', data.comments], ['reviews', data.reviews], ['review threads', data.reviewThreads], ...data.reviewThreads.nodes.map(t => [`comments in a thread on ${t.path}`, t.comments])];
86-
if (issueData)
87-
connections.push(['comments on the issue', issueData.comments]);
88-
for (const [name, connection] of connections) {
89-
if (connection.totalCount > connection.nodes.length)
90-
core.warning(`Read only ${connection.nodes.length} of ${connection.totalCount} ${name}`);
91-
}
92-
93-
// People with write access are the maintainers. The bot is the app; before the app posted
94-
// the triage notes, github-actions did, with the marker.
95-
const maintainers = new Map();
96-
const roleOf = async c => {
97-
const author = c.author;
98-
if (author?.__typename === 'Bot' && author.login.replace(/\[bot\]$/, '') === 'microsoft-playwright-automation')
99-
return 'bot';
100-
if (author?.__typename === 'Bot' && author.login === 'github-actions' && c.body.includes('<!-- playwright-ai-triage -->'))
101-
return 'bot';
102-
if (author?.__typename !== 'User')
103-
return undefined;
104-
if (!maintainers.has(author.login)) {
105-
const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ owner, repo, username: author.login }).catch(() => ({ data: {} }));
106-
maintainers.set(author.login, ['admin', 'write'].includes(data.permission));
107-
}
108-
return maintainers.get(author.login) ? 'maintainer' : undefined;
109-
};
110-
// A comment from a maintainer or from the bot, or undefined for anyone else.
111-
const entry = async c => {
112-
const role = await roleOf(c);
113-
return role && { role, author: role === 'bot' ? 'microsoft-playwright-automation[bot]' : c.author.login, createdAt: c.createdAt, url: c.url, body: c.body };
114-
};
115-
116-
const issue = issueData && {
117-
number: issueData.number,
118-
url: issueData.url,
119-
title: issueData.title,
120-
author: issueData.author?.login,
121-
body: issueData.body,
122-
comments: [],
123-
};
124-
for (const c of issueData?.comments.nodes ?? []) {
125-
const e = await entry(c);
126-
if (e)
127-
issue.comments.push(e);
128-
}
129-
130-
const comments = [];
131-
for (const c of data.comments.nodes) {
132-
const e = await entry(c);
133-
if (e)
134-
comments.push({ kind: 'comment', ...e });
135-
}
136-
for (const r of data.reviews.nodes) {
137-
const e = r.body.trim() && await entry(r);
138-
if (e)
139-
comments.push({ kind: 'review', state: r.state, ...e });
140-
}
141-
for (const t of data.reviewThreads.nodes) {
142-
if (t.isResolved)
143-
continue;
144-
const thread = [];
145-
for (const c of t.comments.nodes) {
146-
// The id is what the agent replies to, in a reply call.
147-
const e = await entry(c);
148-
if (e)
149-
thread.push({ id: Number(c.fullDatabaseId), ...e });
150-
}
151-
if (thread.some(c => c.role === 'maintainer'))
152-
comments.push({ kind: 'thread', path: t.path, line: t.line ?? t.originalLine, outdated: t.isOutdated, diffHunk: t.comments.nodes[0].diffHunk, createdAt: thread[0].createdAt, comments: thread });
153-
}
154-
comments.sort((a, b) => a.createdAt.localeCompare(b.createdAt));
155-
if (!comments.some(c => c.role === 'maintainer' || c.comments?.some(t => t.role === 'maintainer')))
156-
throw new Error('There are no comments from maintainers to address, other than in resolved threads');
157-
158-
fs.writeFileSync(path.join(DIR, 'review.json'), JSON.stringify({
159-
issue,
160-
pullRequest: {
161-
number: pr.number,
162-
url: pr.html_url,
163-
title: pr.title,
164-
body: pr.body,
165-
head: pr.head.sha,
166-
base,
167-
commits: commits.map(c => ({ sha: c.sha, date: c.commit.committer.date, message: c.commit.message })),
168-
},
169-
comments,
170-
}, null, 2));
171-
core.setOutput('head', pr.head.sha);
172-
core.setOutput('base', base);
173-
core.setOutput('branch', pr.head.ref);
48+
const collect = require(require('path').join(process.env.ACTION_PATH, 'collect-review.ts'));
49+
await collect({ github, context, core });
17450
- name: Upload the review
17551
uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2
17652
with:
Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
// Collects the context of a revision into review.json in the directory, for the revision agent.
2+
// The file has three parts: the issue that the pull request is for, with its text and the comments
3+
// on it; the pull request, with its description and commits; and the comments, reviews and
4+
// unresolved review threads on the pull request. Only the comments of people with write access,
5+
// the maintainers, and of the bot itself get through. Throws when the maintainers left nothing to
6+
// address. action.yml next to this file runs it through actions/github-script, with the number of
7+
// the pull request and the directory in the environment, and uploads the directory.
8+
//
9+
// Node removes the types when it loads this file, and it checks none of them. Use only types that
10+
// Node can remove: no enum, no namespace, no parameter properties, no `import x = require()`. The
11+
// file stays CommonJS.
12+
13+
const fs: typeof import('fs') = require('fs');
14+
const path: typeof import('path') = require('path');
15+
16+
type Repo = { owner: string, repo: string };
17+
type Author = { __typename: string, login: string } | null;
18+
type Connection<T> = { totalCount: number, nodes: T[] };
19+
type Comment = { author: Author, body: string, createdAt: string, url: string };
20+
type Review = Comment & { state: string };
21+
type ThreadComment = Comment & { fullDatabaseId: string, diffHunk: string };
22+
type Thread = { isResolved: boolean, isOutdated: boolean, path: string, line: number | null, originalLine: number | null, comments: Connection<ThreadComment> };
23+
type PullRequestData = {
24+
closingIssuesReferences: Connection<{ number: number, repository: { name: string, owner: { login: string } } }>,
25+
comments: Connection<Comment>,
26+
reviews: Connection<Review>,
27+
reviewThreads: Connection<Thread>,
28+
};
29+
type IssueData = { number: number, url: string, title: string, body: string, author: { login: string } | null, comments: Connection<Comment> };
30+
type GitHub = {
31+
rest: {
32+
pulls: {
33+
get(params: Repo & { pull_number: number }): Promise<{ data: { number: number, html_url: string, title: string, body: string | null, head: { sha: string, ref: string }, base: { ref: string } } }>,
34+
listCommits: unknown,
35+
},
36+
repos: {
37+
getBranch(params: Repo & { branch: string }): Promise<{ data: { commit: { sha: string } } }>,
38+
getCollaboratorPermissionLevel(params: Repo & { username: string }): Promise<{ data: { permission?: string } }>,
39+
},
40+
},
41+
paginate<T>(method: unknown, params: object): Promise<T[]>,
42+
graphql<T>(query: string, variables: object): Promise<T>,
43+
};
44+
type Core = {
45+
warning(message: string): void,
46+
setOutput(name: string, value: string | number): void,
47+
};
48+
type Context = { repo: Repo };
49+
50+
// A comment in review.json: from a maintainer or from the bot. Anyone else's is dropped.
51+
type Role = 'maintainer' | 'bot';
52+
type Entry = { role: Role, author: string, createdAt: string, url: string, body: string };
53+
type Item =
54+
| ({ kind: 'comment' } & Entry)
55+
| ({ kind: 'review', state: string } & Entry)
56+
| { kind: 'thread', path: string, line: number | null, outdated: boolean, diffHunk: string, createdAt: string, comments: ({ id: number } & Entry)[] };
57+
58+
async function collect({ github, context, core }: { github: GitHub, context: Context, core: Core }): Promise<void> {
59+
const { owner, repo } = context.repo;
60+
const pull_number = Number(process.env.PULL_NUMBER);
61+
const dir = process.env.DIR!;
62+
fs.mkdirSync(dir, { recursive: true });
63+
64+
const { data: pr } = await github.rest.pulls.get({ owner, repo, pull_number });
65+
const { data: { commit: { sha: base } } } = await github.rest.repos.getBranch({ owner, repo, branch: pr.base.ref });
66+
const commits = await github.paginate<{ sha: string, commit: { committer: { date: string }, message: string } }>(github.rest.pulls.listCommits, { owner, repo, pull_number, per_page: 100 });
67+
const fields = 'author { __typename login } body createdAt url';
68+
const { repository: { pullRequest: data } } = await github.graphql<{ repository: { pullRequest: PullRequestData } }>(`
69+
query($owner: String!, $repo: String!, $number: Int!) {
70+
repository(owner: $owner, name: $repo) {
71+
pullRequest(number: $number) {
72+
closingIssuesReferences(first: 1) { nodes { number repository { name owner { login } } } }
73+
comments(last: 100) { totalCount nodes { ${fields} } }
74+
reviews(last: 100) { totalCount nodes { ${fields} state } }
75+
reviewThreads(last: 100) {
76+
totalCount
77+
nodes {
78+
isResolved isOutdated path line originalLine
79+
comments(first: 100) { totalCount nodes { ${fields} fullDatabaseId diffHunk } }
80+
}
81+
}
82+
}
83+
}
84+
}`, { owner, repo, number: pull_number });
85+
86+
// The issue that the pull request closes. It can be in another repo of the owner: a fix for
87+
// playwright-cli or playwright-mcp lands here. A coverage PR closes none: its branch names
88+
// the issue, triage/<n>/... for an issue in this repo and triage/<repo>-<n>/... for one in another.
89+
const closing = data.closingIssuesReferences.nodes[0];
90+
const branchIssue = pr.head.ref.match(/^triage\/(?:([^/]+)-)?(\d+)\//);
91+
const issueRef = closing ? { owner: closing.repository.owner.login, repo: closing.repository.name, number: closing.number }
92+
: branchIssue ? { owner, repo: branchIssue[1] ?? repo, number: Number(branchIssue[2]) }
93+
: null;
94+
let issueData: IssueData | null = null;
95+
if (issueRef) {
96+
try {
97+
issueData = (await github.graphql<{ repository: { issue: IssueData | null } }>(`
98+
query($owner: String!, $repo: String!, $number: Int!) {
99+
repository(owner: $owner, name: $repo) {
100+
issue(number: $number) { number url title body author { login } comments(last: 100) { totalCount nodes { ${fields} } } }
101+
}
102+
}`, issueRef)).repository.issue;
103+
} catch (e) {
104+
// The issue is context for the agent, not a requirement: the comments to address are on the PR.
105+
core.warning(`Could not read the issue ${issueRef.owner}/${issueRef.repo}#${issueRef.number}: ${(e as Error).message}`);
106+
}
107+
}
108+
109+
const connections: [string, Connection<unknown>][] = [['comments on the PR', data.comments], ['reviews', data.reviews], ['review threads', data.reviewThreads], ...data.reviewThreads.nodes.map((t): [string, Connection<unknown>] => [`comments in a thread on ${t.path}`, t.comments])];
110+
if (issueData)
111+
connections.push(['comments on the issue', issueData.comments]);
112+
for (const [name, connection] of connections) {
113+
if (connection.totalCount > connection.nodes.length)
114+
core.warning(`Read only ${connection.nodes.length} of ${connection.totalCount} ${name}`);
115+
}
116+
117+
// People with write access are the maintainers. The bot is the app; before the app posted
118+
// the triage notes, github-actions did, with the marker.
119+
const maintainers = new Map<string, boolean>();
120+
const roleOf = async (c: Comment): Promise<Role | undefined> => {
121+
const author = c.author;
122+
if (author?.__typename === 'Bot' && author.login.replace(/\[bot\]$/, '') === 'microsoft-playwright-automation')
123+
return 'bot';
124+
if (author?.__typename === 'Bot' && author.login === 'github-actions' && c.body.includes('<!-- playwright-ai-triage -->'))
125+
return 'bot';
126+
if (author?.__typename !== 'User')
127+
return undefined;
128+
if (!maintainers.has(author.login)) {
129+
const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ owner, repo, username: author.login }).catch(() => ({ data: {} as { permission?: string } }));
130+
maintainers.set(author.login, ['admin', 'write'].includes(data.permission!));
131+
}
132+
return maintainers.get(author.login) ? 'maintainer' : undefined;
133+
};
134+
// A comment from a maintainer or from the bot, or undefined for anyone else.
135+
const entry = async (c: Comment): Promise<Entry | undefined> => {
136+
const role = await roleOf(c);
137+
return role && { role, author: role === 'bot' ? 'microsoft-playwright-automation[bot]' : c.author!.login, createdAt: c.createdAt, url: c.url, body: c.body };
138+
};
139+
140+
const issue = issueData && {
141+
number: issueData.number,
142+
url: issueData.url,
143+
title: issueData.title,
144+
author: issueData.author?.login,
145+
body: issueData.body,
146+
comments: [] as Entry[],
147+
};
148+
for (const c of issueData?.comments.nodes ?? []) {
149+
const e = await entry(c);
150+
if (e)
151+
issue!.comments.push(e);
152+
}
153+
154+
const comments: Item[] = [];
155+
for (const c of data.comments.nodes) {
156+
const e = await entry(c);
157+
if (e)
158+
comments.push({ kind: 'comment', ...e });
159+
}
160+
for (const r of data.reviews.nodes) {
161+
const e = r.body.trim() && await entry(r);
162+
if (e)
163+
comments.push({ kind: 'review', state: r.state, ...e });
164+
}
165+
for (const t of data.reviewThreads.nodes) {
166+
if (t.isResolved)
167+
continue;
168+
const thread: ({ id: number } & Entry)[] = [];
169+
for (const c of t.comments.nodes) {
170+
// The id is what the agent replies to, in a reply call.
171+
const e = await entry(c);
172+
if (e)
173+
thread.push({ id: Number(c.fullDatabaseId), ...e });
174+
}
175+
if (thread.some(c => c.role === 'maintainer'))
176+
comments.push({ kind: 'thread', path: t.path, line: t.line ?? t.originalLine, outdated: t.isOutdated, diffHunk: t.comments.nodes[0].diffHunk, createdAt: thread[0].createdAt, comments: thread });
177+
}
178+
comments.sort((a, b) => a.createdAt.localeCompare(b.createdAt));
179+
if (!comments.some(c => c.kind === 'thread' ? c.comments.some(t => t.role === 'maintainer') : c.role === 'maintainer'))
180+
throw new Error('There are no comments from maintainers to address, other than in resolved threads');
181+
182+
fs.writeFileSync(path.join(dir, 'review.json'), JSON.stringify({
183+
issue,
184+
pullRequest: {
185+
number: pr.number,
186+
url: pr.html_url,
187+
title: pr.title,
188+
body: pr.body,
189+
head: pr.head.sha,
190+
base,
191+
commits: commits.map(c => ({ sha: c.sha, date: c.commit.committer.date, message: c.commit.message })),
192+
},
193+
comments,
194+
}, null, 2));
195+
core.setOutput('head', pr.head.sha);
196+
core.setOutput('base', base);
197+
core.setOutput('branch', pr.head.ref);
198+
}
199+
200+
module.exports = collect;

0 commit comments

Comments
 (0)