diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index cc8ee6bc7d..ad9e056ea4 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,5 +1,5 @@ ## ๐Ÿ“Œ๐˜๐˜ด๐˜ด๐˜ถ๐˜ฆ๐˜ด - + - Refs # ## ๐Ÿ“Ž๐˜ž๐˜ฐ๐˜ณ๐˜ฌ ๐˜‹๐˜ฆ๐˜ด๐˜ค๐˜ณ๐˜ช๐˜ฑ๐˜ต๐˜ช๐˜ฐ๐˜ฏ diff --git a/.github/scripts/pr-issue-link-policy.test.mjs b/.github/scripts/pr-issue-link-policy.test.mjs index fa302840d8..7b266fe5e7 100644 --- a/.github/scripts/pr-issue-link-policy.test.mjs +++ b/.github/scripts/pr-issue-link-policy.test.mjs @@ -11,8 +11,18 @@ const workflowUrl = new URL("../workflows/repository-quality.yml", import.meta.u const callerWorkflowUrl = new URL("../workflows/pr-validation.yml", import.meta.url); const templateUrl = new URL("../PULL_REQUEST_TEMPLATE.md", import.meta.url); -function pullRequest({ title = "change", body = "", number = 7 } = {}) { - return { number, title, body }; +function pullRequest({ + title = "change", + body = "", + number = 7, + user = { login: "author", type: "User" }, + labels = [], +} = {}) { + return { number, title, body, user, labels }; +} + +function assignedIssue(number, overrides = {}) { + return { number, state: "open", assignees: [{ login: "author" }], ...overrides }; } function issueLoader(items) { @@ -64,13 +74,13 @@ test("accepts a same-repository open Issue", async () => { const result = await validatePullRequestIssueLink({ pullRequest: pullRequest({ title: "change (#1228)", body: "Refs #1228" }), repository: "Afternote/Afternote-FE", - loadIssue: issueLoader(new Map([[1228, { number: 1228, state: "open" }]])), + loadIssue: issueLoader(new Map([[1228, assignedIssue(1228)]])), }); assert.deepEqual(result, { issues: [1228], rejected: [] }); }); test("allows multiple pull requests to share one Issue regardless of closing behavior or state", async () => { - const loadIssue = issueLoader(new Map([[601, { number: 601, state: "closed" }]])); + const loadIssue = issueLoader(new Map([[601, assignedIssue(601, { state: "closed" })]])); const results = await Promise.all([ validatePullRequestIssueLink({ pullRequest: pullRequest({ number: 7, title: "first change (#601)", body: "Part of #601" }), @@ -111,7 +121,7 @@ test("accepts a closed Issue while warning about a missing reference", async () const result = await validatePullRequestIssueLink({ pullRequest: pullRequest({ title: "change (#2)", body: "Closes #1 and fixes #2" }), repository: "Afternote/Afternote-FE", - loadIssue: issueLoader(new Map([[2, { number: 2, state: "closed" }]])), + loadIssue: issueLoader(new Map([[2, assignedIssue(2, { state: "closed" })]])), }); assert.deepEqual(result, { @@ -120,6 +130,143 @@ test("accepts a closed Issue while warning about a missing reference", async () }); }); +test("rejects an author who is not an assignee of the representative Issue", async () => { + await assert.rejects( + validatePullRequestIssueLink({ + pullRequest: pullRequest({ title: "change (#1228)", body: "Refs #1228" }), + repository: "Afternote/Afternote-FE", + loadIssue: issueLoader(new Map([ + [1228, assignedIssue(1228, { assignees: [{ login: "someone-else" }] })], + ])), + }), + /์ž‘์„ฑ์ž @author๋Š” ๋Œ€ํ‘œ Issue #1228์˜ ๋‹ด๋‹น์ž\(@someone-else\)๊ฐ€ ์•„๋‹™๋‹ˆ๋‹ค/, + ); +}); + +test("rejects an unassigned representative Issue", async () => { + await assert.rejects( + validatePullRequestIssueLink({ + pullRequest: pullRequest({ title: "change (#1228)", body: "Refs #1228" }), + repository: "Afternote/Afternote-FE", + loadIssue: issueLoader(new Map([ + [1228, assignedIssue(1228, { assignees: [] })], + ])), + }), + /๋Œ€ํ‘œ Issue #1228์— ๋‹ด๋‹น์ž๊ฐ€ ์—†์Šต๋‹ˆ๋‹ค.*@author๋ฅผ ์–ด์‚ฌ์ธ/, + ); +}); + +test("matches the assignee login case-insensitively among multiple assignees", async () => { + const result = await validatePullRequestIssueLink({ + pullRequest: pullRequest({ + title: "change (#1228)", + body: "Refs #1228", + user: { login: "Author", type: "User" }, + }), + repository: "Afternote/Afternote-FE", + loadIssue: issueLoader(new Map([ + [1228, assignedIssue(1228, { assignees: [{ login: "someone-else" }, "author"] })], + ])), + }); + assert.deepEqual(result, { issues: [1228], rejected: [] }); +}); + +test("checks the assignee only on the representative Issue, not other references", async () => { + const result = await validatePullRequestIssueLink({ + pullRequest: pullRequest({ title: "change (#1228)", body: "Refs #1228\nRefs #1229" }), + repository: "Afternote/Afternote-FE", + loadIssue: issueLoader(new Map([ + [1228, assignedIssue(1228)], + [1229, assignedIssue(1229, { assignees: [{ login: "someone-else" }] })], + ])), + }); + assert.deepEqual(result, { issues: [1228, 1229], rejected: [] }); +}); + +test("exempts bot authors from the assignee check but keeps the Issue link requirement", async () => { + const loadIssue = issueLoader(new Map([[12, assignedIssue(12, { assignees: [] })]])); + const result = await validatePullRequestIssueLink({ + pullRequest: pullRequest({ + title: "chore(deps): bump upload-artifact (#12)", + body: "Refs #12", + user: { login: "dependabot[bot]", type: "Bot" }, + }), + repository: "Afternote/Afternote-FE", + loadIssue, + }); + assert.deepEqual(result, { issues: [12], rejected: [] }); + + await assert.rejects( + validatePullRequestIssueLink({ + pullRequest: pullRequest({ + title: "chore(deps): bump upload-artifact", + body: "", + user: { login: "dependabot[bot]", type: "Bot" }, + }), + repository: "Afternote/Afternote-FE", + loadIssue, + }), + /์ œ๋ชฉ์€ ๋Œ€ํ‘œ Issue ๋ฒˆํ˜ธ ํ•˜๋‚˜๋กœ ๋๋‚˜์•ผ ํ•ฉ๋‹ˆ๋‹ค/, + ); +}); + +test("the issue-assignee-exempt label skips the assignee check but keeps the Issue link requirement", async () => { + const labels = [{ name: "issue-assignee-exempt" }]; + const loadIssue = issueLoader(new Map([ + [1228, assignedIssue(1228, { assignees: [{ login: "someone-else" }] })], + ])); + const result = await validatePullRequestIssueLink({ + pullRequest: pullRequest({ title: "change (#1228)", body: "Refs #1228", labels }), + repository: "Afternote/Afternote-FE", + loadIssue, + }); + assert.deepEqual(result, { issues: [1228], rejected: [] }); + + const unassigned = await validatePullRequestIssueLink({ + pullRequest: pullRequest({ title: "change (#12)", body: "Refs #12", labels }), + repository: "Afternote/Afternote-FE", + loadIssue: issueLoader(new Map([[12, assignedIssue(12, { assignees: [] })]])), + }); + assert.deepEqual(unassigned, { issues: [12], rejected: [] }); + + await assert.rejects( + validatePullRequestIssueLink({ + pullRequest: pullRequest({ title: "change", body: "", labels }), + repository: "Afternote/Afternote-FE", + loadIssue, + }), + /์ œ๋ชฉ์€ ๋Œ€ํ‘œ Issue ๋ฒˆํ˜ธ ํ•˜๋‚˜๋กœ ๋๋‚˜์•ผ ํ•ฉ๋‹ˆ๋‹ค/, + ); +}); + +test("unrelated labels keep the assignee rejection and point at the exempt label", async () => { + await assert.rejects( + validatePullRequestIssueLink({ + pullRequest: pullRequest({ + title: "change (#1228)", + body: "Refs #1228", + labels: [{ name: "review-debt-exempt" }, "bug"], + }), + repository: "Afternote/Afternote-FE", + loadIssue: issueLoader(new Map([ + [1228, assignedIssue(1228, { assignees: [{ login: "someone-else" }] })], + ])), + }), + /๋‹ด๋‹น์ž\(@someone-else\)๊ฐ€ ์•„๋‹™๋‹ˆ๋‹ค.*`issue-assignee-exempt` ๋ผ๋ฒจ/, + ); +}); + +test("requires the pull request author login", async () => { + await assert.rejects( + validatePullRequestIssueLink({ + pullRequest: pullRequest({ title: "change (#1228)", body: "Refs #1228", user: null }), + repository: "Afternote/Afternote-FE", + loadIssue: issueLoader(new Map([[1228, assignedIssue(1228)]])), + }), + /pull_request\.user\.login ๊ฐ’์ด ์—†์Šต๋‹ˆ๋‹ค/, + ); +}); + test("rejects a missing Issue", async () => { await assert.rejects( validatePullRequestIssueLink({ @@ -216,6 +363,7 @@ test("the PR template tells authors to reuse an Issue across pull requests", asy assert.match(template, /๊ด€๋ จ๋œ ๊ธฐ์กด Issue๋ฅผ ์žฌ์‚ฌ์šฉ/); assert.match(template, /PR ์ œ๋ชฉ ๋์— ๋Œ€ํ‘œ Issue ํ•˜๋‚˜๋ฅผ `\(#N\)` ํ˜•์‹/); assert.match(template, /์—ฌ๋Ÿฌ PR์ด ๊ฐ™์€ Issue๋ฅผ ๊ณต์œ /); + assert.match(template, /๋Œ€ํ‘œ Issue์—๋Š” PR ์ž‘์„ฑ์ž๊ฐ€ ๋‹ด๋‹น์ž๋กœ ์ง€์ •/); assert.match(template, /์ตœ์ข… ์™„๋ฃŒํ•˜๋Š” PR์—์„œ๋งŒ Closes\/Fixes\/Resolves/); assert.match(template, /^- Refs #$/m); assert.doesNotMatch(template, /^- Closes #$/m); diff --git a/.github/scripts/validate-pr-issue-link.mjs b/.github/scripts/validate-pr-issue-link.mjs index b853d2bd7b..1659212f74 100644 --- a/.github/scripts/validate-pr-issue-link.mjs +++ b/.github/scripts/validate-pr-issue-link.mjs @@ -55,6 +55,29 @@ export function extractTitleIssueNumber(title) { return Number(matches[0][1]); } +function issueAssigneeLogins(issue) { + return (issue?.assignees ?? []) + .map((assignee) => typeof assignee === "string" ? assignee : assignee?.login) + .filter(Boolean); +} + +// ๋ด‡์€ Issue ๋‹ด๋‹น์ž๋กœ ์ง€์ •ํ•  ์ˆ˜ ์—†์œผ๋ฏ€๋กœ ๋‹ด๋‹น์ž ๋Œ€์กฐ์—์„œ ๋ฉด์ œํ•œ๋‹ค. +// review-debt-guardยทreview-request-all ์˜ ๋ด‡ ๋ฉด์ œ ๊ทœ์•ฝ๊ณผ ๊ฐ™์€ ๊ฒฝ๊ณ„๋‹ค. +function isBotAuthor(user, login) { + return user?.type === "Bot" || login.endsWith("[bot]"); +} + +const ISSUE_ASSIGNEE_EXEMPT_LABEL = "issue-assignee-exempt"; + +// ๋‹ค๋ฅธ ๋‹ด๋‹น์ž์˜ ๋ชจ๋“ˆ์ด develop ์„ ๊นจ๋œจ๋ ธ์„ ๋•Œ์ฒ˜๋Ÿผ ์–ด์‚ฌ์ธ ์ด๊ด€์„ ๊ธฐ๋‹ค๋ฆด ์ˆ˜ ์—†๋Š” +// ๊ธด๊ธ‰ PR ์€ ๋ผ๋ฒจ ํ•˜๋‚˜๋กœ ๋‹ด๋‹น์ž ๋Œ€์กฐ๋งŒ ๋ฉด์ œํ•œ๋‹ค โ€” review-debt-exempt ์™€ ๊ฐ™์€ ๊ทœ์•ฝ. +// Issue ์—ฐ๊ฒฐ ์š”๊ฑด ์ž์ฒด๋Š” ๋ฉด์ œํ•˜์ง€ ์•Š๋Š”๋‹ค. +export function hasIssueAssigneeExemptLabel(pullRequest) { + return (pullRequest?.labels ?? []) + .map((label) => typeof label === "string" ? label : label?.name) + .includes(ISSUE_ASSIGNEE_EXEMPT_LABEL); +} + export async function validatePullRequestIssueLink({ pullRequest, repository, loadIssue }) { if (typeof loadIssue !== "function") { throw new Error("loadIssue ํ•จ์ˆ˜๊ฐ€ ํ•„์š”ํ•ฉ๋‹ˆ๋‹ค."); @@ -63,6 +86,7 @@ export async function validatePullRequestIssueLink({ pullRequest, repository, lo if (!Number.isInteger(pullRequestNumber)) { throw new Error("pull_request.number ๊ฐ’์ด ์—†์Šต๋‹ˆ๋‹ค."); } + const author = requiredString(pullRequest?.user?.login, "pull_request.user.login"); const titleIssueNumber = extractTitleIssueNumber(pullRequest.title); if (titleIssueNumber === null) { @@ -85,6 +109,7 @@ export async function validatePullRequestIssueLink({ pullRequest, repository, lo const issues = []; const rejected = []; + const issuesByNumber = new Map(); for (const issueNumber of references) { let issue; try { @@ -97,6 +122,7 @@ export async function validatePullRequestIssueLink({ pullRequest, repository, lo rejected.push(`#${issueNumber}: Issue๊ฐ€ ์•„๋‹ˆ๋ผ PR`); continue; } + issuesByNumber.set(issueNumber, issue); issues.push(issueNumber); } @@ -111,6 +137,20 @@ export async function validatePullRequestIssueLink({ pullRequest, repository, lo `PR #${pullRequestNumber} ์ œ๋ชฉ์˜ ๋Œ€ํ‘œ Issue #${titleIssueNumber}๊ฐ€ ์‹ค์ œ Issue๋กœ ํ™•์ธ๋˜์ง€ ์•Š์•˜์Šต๋‹ˆ๋‹ค. ${titleRejection ?? "Issue ์กฐํšŒ ๊ฒฐ๊ณผ๋ฅผ ํ™•์ธํ•˜์„ธ์š”."}`, ); } + + if (!isBotAuthor(pullRequest.user, author) && !hasIssueAssigneeExemptLabel(pullRequest)) { + const assignees = issueAssigneeLogins(issuesByNumber.get(titleIssueNumber)); + if (assignees.length === 0) { + throw new Error( + `PR #${pullRequestNumber}์˜ ๋Œ€ํ‘œ Issue #${titleIssueNumber}์— ๋‹ด๋‹น์ž๊ฐ€ ์—†์Šต๋‹ˆ๋‹ค. Issue์— @${author}๋ฅผ ์–ด์‚ฌ์ธํ•œ ๋’ค ๋‹ค์‹œ ์‹คํ–‰ํ•˜์„ธ์š”.`, + ); + } + if (!assignees.some((login) => login.toLowerCase() === author.toLowerCase())) { + throw new Error( + `PR #${pullRequestNumber} ์ž‘์„ฑ์ž @${author}๋Š” ๋Œ€ํ‘œ Issue #${titleIssueNumber}์˜ ๋‹ด๋‹น์ž(${assignees.map((login) => `@${login}`).join(", ")})๊ฐ€ ์•„๋‹™๋‹ˆ๋‹ค. ๋ณธ์ธ์ด ๋‹ด๋‹นํ•˜๋Š” Issue๋กœ๋งŒ PR์„ ์—ด ์ˆ˜ ์žˆ์Šต๋‹ˆ๋‹ค. ๋‹ด๋‹น์ž ์ด๊ด€์„ ๊ธฐ๋‹ค๋ฆด ์ˆ˜ ์—†๋Š” ๊ธด๊ธ‰ ์ˆ˜์„ ์ด๋ฉด \`${ISSUE_ASSIGNEE_EXEMPT_LABEL}\` ๋ผ๋ฒจ์„ ๋ถ™์ธ ๋’ค ์žฌ๊ฒ€์ฆ(๋ณธ๋ฌธ ์ˆ˜์ • ๋˜๋Š” push)์„ ํŠธ๋ฆฌ๊ฑฐํ•˜์„ธ์š”.`, + ); + } + } return { issues, rejected }; } @@ -143,6 +183,9 @@ async function main() { repository, loadIssue: (issueNumber) => requestIssue(apiUrl, repository, token, issueNumber), }); + if (hasIssueAssigneeExemptLabel(pullRequest)) { + console.log(`${ISSUE_ASSIGNEE_EXEMPT_LABEL} ๋ผ๋ฒจ โ€” ๋‹ด๋‹น์ž ๋Œ€์กฐ ๊ฑด๋„ˆ๋œ€`); + } for (const warning of result.rejected) { console.log(`::warning::๋ฌดํšจ Issue ์ฐธ์กฐ: ${warning}`); }