diff --git a/scripts/github/barnacle-auto-response.mjs b/scripts/github/barnacle-auto-response.mjs index 0f8e40d526c..fea9d760873 100644 --- a/scripts/github/barnacle-auto-response.mjs +++ b/scripts/github/barnacle-auto-response.mjs @@ -183,6 +183,8 @@ const spamLabel = "r: spam"; const dirtyLabel = "dirty"; const badBarnacleLabel = "bad-barnacle"; const maintainerAuthorLabel = "maintainer"; +const privilegedAuthorAssociations = new Set(["OWNER", "MEMBER", "COLLABORATOR"]); +const privilegedRepositoryRoles = new Set(["admin", "maintain", "write"]); const candidateLabelValues = Object.values(candidateLabels); const noisyPrMessage = "Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch."; @@ -607,23 +609,16 @@ function createMaintainerChecker(github, context) { }; } -async function isPrivilegedPullRequestAuthor(github, context, pullRequest, labelSet, isMaintainer) { - const authorLogin = pullRequest.user?.login ?? ""; - if (labelSet.has(maintainerAuthorLabel) || pullRequest.author_association === "OWNER") { - return true; - } - if (authorLogin && (await isMaintainer(authorLogin))) { - return true; - } - +async function hasPrivilegedRepositoryRole(github, context, login) { try { const permission = await github.rest.repos.getCollaboratorPermissionLevel({ owner: context.repo.owner, repo: context.repo.repo, - username: authorLogin, + username: login, }); const roleName = (permission?.data?.role_name ?? "").toLowerCase(); - return roleName === "admin" || roleName === "maintain"; + const permissionName = (permission?.data?.permission ?? "").toLowerCase(); + return privilegedRepositoryRoles.has(roleName) || privilegedRepositoryRoles.has(permissionName); } catch (error) { if (error?.status !== 404) { throw error; @@ -633,6 +628,26 @@ async function isPrivilegedPullRequestAuthor(github, context, pullRequest, label return false; } +async function isPrivilegedActor(github, context, login, isMaintainer) { + if (!login) { + return false; + } + return (await isMaintainer(login)) || (await hasPrivilegedRepositoryRole(github, context, login)); +} + +async function isPrivilegedTargetAuthor(github, context, target, labelSet, isMaintainer) { + const authorLogin = target.user?.login ?? ""; + const authorAssociation = String(target.author_association ?? "").toUpperCase(); + if (labelSet.has(maintainerAuthorLabel) || privilegedAuthorAssociations.has(authorAssociation)) { + return true; + } + if (await isPrivilegedActor(github, context, authorLogin, isMaintainer)) { + return true; + } + + return false; +} + async function countMaintainerMentions(body, authorLogin, isMaintainer, owner) { if (!body) { return 0; @@ -807,6 +822,15 @@ export async function runBarnacleAutoResponse({ github, context, core = console if (comment.user?.type === "Bot" || authorLogin.endsWith("[bot]")) { return; } + if ( + (await isPrivilegedActor(github, context, authorLogin, isMaintainer)) || + (await isPrivilegedTargetAuthor(github, context, target, labelSet, isMaintainer)) + ) { + core.info( + `Skipping Barnacle comment checks for #${target.number} because a maintainer is involved.`, + ); + return; + } const commentBody = comment.body ?? ""; const responses = []; @@ -839,6 +863,13 @@ export async function runBarnacleAutoResponse({ github, context, core = console return; } + if (await isPrivilegedTargetAuthor(github, context, target, labelSet, isMaintainer)) { + core.info( + `Skipping Barnacle auto-response checks for #${target.number} because it is maintainer-authored.`, + ); + return; + } + if (issue) { const action = context.payload.action; if (action === "opened" || action === "edited") { @@ -934,22 +965,7 @@ export async function runBarnacleAutoResponse({ github, context, core = console return; } - const isMaintainerAuthoredPullRequest = await isPrivilegedPullRequestAuthor( - github, - context, - pullRequest, - labelSet, - isMaintainer, - ); - if (isMaintainerAuthoredPullRequest) { - await removeLabels(github, context, pullRequest.number, candidateLabelValues, labelSet); - await removeLabels(github, context, pullRequest.number, [activePrLimitLabel], labelSet); - core.info( - `Skipping Barnacle candidate labels for maintainer-authored PR #${pullRequest.number}.`, - ); - } else { - await applyPullRequestCandidateLabels(github, context, core, pullRequest, labelSet); - } + await applyPullRequestCandidateLabels(github, context, core, pullRequest, labelSet); if (labelSet.has(dirtyLabel)) { await github.rest.issues.createComment({ diff --git a/test/scripts/barnacle-auto-response.test.ts b/test/scripts/barnacle-auto-response.test.ts index 82379eff4bc..286df6d3be0 100644 --- a/test/scripts/barnacle-auto-response.test.ts +++ b/test/scripts/barnacle-auto-response.test.ts @@ -73,7 +73,49 @@ function barnacleContext( }; } -function barnacleGithub(files: ReturnType[]) { +function barnacleIssueContext( + issue: Record, + labels: string[] = [], + options: Record = {}, +) { + return { + repo: { + owner: "openclaw", + repo: "openclaw", + }, + payload: { + action: options.action ?? "opened", + label: options.label, + sender: options.sender, + issue: { + number: 456, + title: "OpenClaw issue", + body: "", + author_association: "CONTRIBUTOR", + user: { + login: "contributor", + }, + labels: labels.map((name) => ({ name })), + ...issue, + }, + comment: options.comment, + }, + }; +} + +function barnacleGithub( + files: ReturnType[], + options: { maintainerLogins?: string[]; repositoryRoles?: Record } = {}, +) { + const maintainerLogins = new Set( + (options.maintainerLogins ?? []).map((login) => login.toLowerCase()), + ); + const repositoryRoles = Object.fromEntries( + Object.entries(options.repositoryRoles ?? {}).map(([login, role]) => [ + login.toLowerCase(), + role, + ]), + ); const calls = { addLabels: [] as Array<{ issue_number: number; labels: string[] }>, createComment: [] as Array<{ issue_number: number; body: string }>, @@ -115,14 +157,25 @@ function barnacleGithub(files: ReturnType[]) { listFiles: async () => files, }, repos: { - getCollaboratorPermissionLevel: async () => ({ - data: { - role_name: "read", - }, - }), + getCollaboratorPermissionLevel: async ({ username }: { username: string }) => { + const role = repositoryRoles[username.toLowerCase()] ?? "read"; + return { + data: { + permission: role, + role_name: role, + }, + }; + }, }, teams: { - getMembershipForUserInOrg: async () => { + getMembershipForUserInOrg: async ({ username }: { username: string }) => { + if (maintainerLogins.has(username.toLowerCase())) { + return { + data: { + state: "active", + }, + }; + } const error = new Error("not found") as Error & { status: number }; error.status = 404; throw error; @@ -234,7 +287,7 @@ describe("barnacle-auto-response", () => { expect(labels).not.toContain(candidateLabels.externalPluginCandidate); }); - it("does not add candidate labels to maintainer-authored PRs", async () => { + it("does not mutate maintainer-authored PRs", async () => { const { calls, github } = barnacleGithub([ file("ui/src/app.ts"), file("src/gateway/server.ts"), @@ -256,9 +309,12 @@ describe("barnacle-auto-response", () => { }); expect(calls.addLabels).toEqual([]); + expect(calls.createComment).toEqual([]); + expect(calls.removeLabel).toEqual([]); + expect(calls.update).toEqual([]); }); - it("removes stale Barnacle candidate and PR-limit labels from maintainer-authored PRs", async () => { + it("leaves stale Barnacle labels alone on maintainer-authored PRs", async () => { const { calls, github } = barnacleGithub([ file("ui/src/app.ts"), file("src/gateway/server.ts"), @@ -282,12 +338,93 @@ describe("barnacle-auto-response", () => { }, }); - expect(calls.removeLabel).toEqual( - expect.arrayContaining([ - expect.objectContaining({ name: candidateLabels.dirtyCandidate }), - expect.objectContaining({ name: "r: too-many-prs" }), - ]), - ); + expect(calls.addLabels).toEqual([]); + expect(calls.createComment).toEqual([]); + expect(calls.removeLabel).toEqual([]); + expect(calls.update).toEqual([]); + }); + + it("does not mutate maintainer-authored issues", async () => { + const { calls, github } = barnacleGithub([]); + + await runBarnacleAutoResponse({ + github, + context: barnacleIssueContext({ + title: "TestFlight access", + author_association: "OWNER", + user: { + login: "maintainer", + }, + }), + core: { + info: () => undefined, + }, + }); + + expect(calls.addLabels).toEqual([]); + expect(calls.createComment).toEqual([]); + expect(calls.update).toEqual([]); + }); + + it("does not action close labels on maintainer-authored issues", async () => { + const { calls, github } = barnacleGithub([]); + + await runBarnacleAutoResponse({ + github, + context: barnacleIssueContext( + { + title: "Need help with setup", + author_association: "MEMBER", + user: { + login: "maintainer", + }, + }, + ["r: support"], + { + action: "labeled", + label: { name: "r: support" }, + }, + ), + core: { + info: () => undefined, + }, + }); + + expect(calls.createComment).toEqual([]); + expect(calls.update).toEqual([]); + }); + + it("does not respond to maintainer comments on contributor items", async () => { + const { calls, github } = barnacleGithub([], { maintainerLogins: ["maintainer"] }); + + await runBarnacleAutoResponse({ + github, + context: barnacleIssueContext( + { + title: "Contributor issue", + user: { + login: "contributor", + }, + }, + [], + { + action: "created", + comment: { + body: "testflight", + user: { + login: "maintainer", + type: "User", + }, + }, + }, + ), + core: { + info: () => undefined, + }, + }); + + expect(calls.createComment).toEqual([]); + expect(calls.update).toEqual([]); }); it("does not close automation PRs for the active PR limit", async () => {