Skip to content

Commit 092321d

Browse files
vincentkocvincentkoc
committed
fix(github): skip maintainer-owned Barnacle targets
1 parent f31311d commit 092321d

2 files changed

Lines changed: 195 additions & 42 deletions

File tree

‎scripts/github/barnacle-auto-response.mjs‎

Lines changed: 43 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,8 @@ const spamLabel = "r: spam";
183183
const dirtyLabel = "dirty";
184184
const badBarnacleLabel = "bad-barnacle";
185185
const maintainerAuthorLabel = "maintainer";
186+
const privilegedAuthorAssociations = new Set(["OWNER", "MEMBER", "COLLABORATOR"]);
187+
const privilegedRepositoryRoles = new Set(["admin", "maintain", "write"]);
186188
const candidateLabelValues = Object.values(candidateLabels);
187189
const noisyPrMessage =
188190
"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) {
607609
};
608610
}
609611

610-
async function isPrivilegedPullRequestAuthor(github, context, pullRequest, labelSet, isMaintainer) {
611-
const authorLogin = pullRequest.user?.login ?? "";
612-
if (labelSet.has(maintainerAuthorLabel) || pullRequest.author_association === "OWNER") {
613-
return true;
614-
}
615-
if (authorLogin && (await isMaintainer(authorLogin))) {
616-
return true;
617-
}
618-
612+
async function hasPrivilegedRepositoryRole(github, context, login) {
619613
try {
620614
const permission = await github.rest.repos.getCollaboratorPermissionLevel({
621615
owner: context.repo.owner,
622616
repo: context.repo.repo,
623-
username: authorLogin,
617+
username: login,
624618
});
625619
const roleName = (permission?.data?.role_name ?? "").toLowerCase();
626-
return roleName === "admin" || roleName === "maintain";
620+
const permissionName = (permission?.data?.permission ?? "").toLowerCase();
621+
return privilegedRepositoryRoles.has(roleName) || privilegedRepositoryRoles.has(permissionName);
627622
} catch (error) {
628623
if (error?.status !== 404) {
629624
throw error;
@@ -633,6 +628,26 @@ async function isPrivilegedPullRequestAuthor(github, context, pullRequest, label
633628
return false;
634629
}
635630

631+
async function isPrivilegedActor(github, context, login, isMaintainer) {
632+
if (!login) {
633+
return false;
634+
}
635+
return (await isMaintainer(login)) || (await hasPrivilegedRepositoryRole(github, context, login));
636+
}
637+
638+
async function isPrivilegedTargetAuthor(github, context, target, labelSet, isMaintainer) {
639+
const authorLogin = target.user?.login ?? "";
640+
const authorAssociation = String(target.author_association ?? "").toUpperCase();
641+
if (labelSet.has(maintainerAuthorLabel) || privilegedAuthorAssociations.has(authorAssociation)) {
642+
return true;
643+
}
644+
if (await isPrivilegedActor(github, context, authorLogin, isMaintainer)) {
645+
return true;
646+
}
647+
648+
return false;
649+
}
650+
636651
async function countMaintainerMentions(body, authorLogin, isMaintainer, owner) {
637652
if (!body) {
638653
return 0;
@@ -807,6 +822,15 @@ export async function runBarnacleAutoResponse({ github, context, core = console
807822
if (comment.user?.type === "Bot" || authorLogin.endsWith("[bot]")) {
808823
return;
809824
}
825+
if (
826+
(await isPrivilegedActor(github, context, authorLogin, isMaintainer)) ||
827+
(await isPrivilegedTargetAuthor(github, context, target, labelSet, isMaintainer))
828+
) {
829+
core.info(
830+
`Skipping Barnacle comment checks for #${target.number} because a maintainer is involved.`,
831+
);
832+
return;
833+
}
810834

811835
const commentBody = comment.body ?? "";
812836
const responses = [];
@@ -839,6 +863,13 @@ export async function runBarnacleAutoResponse({ github, context, core = console
839863
return;
840864
}
841865

866+
if (await isPrivilegedTargetAuthor(github, context, target, labelSet, isMaintainer)) {
867+
core.info(
868+
`Skipping Barnacle auto-response checks for #${target.number} because it is maintainer-authored.`,
869+
);
870+
return;
871+
}
872+
842873
if (issue) {
843874
const action = context.payload.action;
844875
if (action === "opened" || action === "edited") {
@@ -934,22 +965,7 @@ export async function runBarnacleAutoResponse({ github, context, core = console
934965
return;
935966
}
936967

937-
const isMaintainerAuthoredPullRequest = await isPrivilegedPullRequestAuthor(
938-
github,
939-
context,
940-
pullRequest,
941-
labelSet,
942-
isMaintainer,
943-
);
944-
if (isMaintainerAuthoredPullRequest) {
945-
await removeLabels(github, context, pullRequest.number, candidateLabelValues, labelSet);
946-
await removeLabels(github, context, pullRequest.number, [activePrLimitLabel], labelSet);
947-
core.info(
948-
`Skipping Barnacle candidate labels for maintainer-authored PR #${pullRequest.number}.`,
949-
);
950-
} else {
951-
await applyPullRequestCandidateLabels(github, context, core, pullRequest, labelSet);
952-
}
968+
await applyPullRequestCandidateLabels(github, context, core, pullRequest, labelSet);
953969

954970
if (labelSet.has(dirtyLabel)) {
955971
await github.rest.issues.createComment({

‎test/scripts/barnacle-auto-response.test.ts‎

Lines changed: 152 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,49 @@ function barnacleContext(
7373
};
7474
}
7575

76-
function barnacleGithub(files: ReturnType<typeof file>[]) {
76+
function barnacleIssueContext(
77+
issue: Record<string, unknown>,
78+
labels: string[] = [],
79+
options: Record<string, unknown> = {},
80+
) {
81+
return {
82+
repo: {
83+
owner: "openclaw",
84+
repo: "openclaw",
85+
},
86+
payload: {
87+
action: options.action ?? "opened",
88+
label: options.label,
89+
sender: options.sender,
90+
issue: {
91+
number: 456,
92+
title: "OpenClaw issue",
93+
body: "",
94+
author_association: "CONTRIBUTOR",
95+
user: {
96+
login: "contributor",
97+
},
98+
labels: labels.map((name) => ({ name })),
99+
...issue,
100+
},
101+
comment: options.comment,
102+
},
103+
};
104+
}
105+
106+
function barnacleGithub(
107+
files: ReturnType<typeof file>[],
108+
options: { maintainerLogins?: string[]; repositoryRoles?: Record<string, string> } = {},
109+
) {
110+
const maintainerLogins = new Set(
111+
(options.maintainerLogins ?? []).map((login) => login.toLowerCase()),
112+
);
113+
const repositoryRoles = Object.fromEntries(
114+
Object.entries(options.repositoryRoles ?? {}).map(([login, role]) => [
115+
login.toLowerCase(),
116+
role,
117+
]),
118+
);
77119
const calls = {
78120
addLabels: [] as Array<{ issue_number: number; labels: string[] }>,
79121
createComment: [] as Array<{ issue_number: number; body: string }>,
@@ -115,14 +157,25 @@ function barnacleGithub(files: ReturnType<typeof file>[]) {
115157
listFiles: async () => files,
116158
},
117159
repos: {
118-
getCollaboratorPermissionLevel: async () => ({
119-
data: {
120-
role_name: "read",
121-
},
122-
}),
160+
getCollaboratorPermissionLevel: async ({ username }: { username: string }) => {
161+
const role = repositoryRoles[username.toLowerCase()] ?? "read";
162+
return {
163+
data: {
164+
permission: role,
165+
role_name: role,
166+
},
167+
};
168+
},
123169
},
124170
teams: {
125-
getMembershipForUserInOrg: async () => {
171+
getMembershipForUserInOrg: async ({ username }: { username: string }) => {
172+
if (maintainerLogins.has(username.toLowerCase())) {
173+
return {
174+
data: {
175+
state: "active",
176+
},
177+
};
178+
}
126179
const error = new Error("not found") as Error & { status: number };
127180
error.status = 404;
128181
throw error;
@@ -234,7 +287,7 @@ describe("barnacle-auto-response", () => {
234287
expect(labels).not.toContain(candidateLabels.externalPluginCandidate);
235288
});
236289

237-
it("does not add candidate labels to maintainer-authored PRs", async () => {
290+
it("does not mutate maintainer-authored PRs", async () => {
238291
const { calls, github } = barnacleGithub([
239292
file("ui/src/app.ts"),
240293
file("src/gateway/server.ts"),
@@ -256,9 +309,12 @@ describe("barnacle-auto-response", () => {
256309
});
257310

258311
expect(calls.addLabels).toEqual([]);
312+
expect(calls.createComment).toEqual([]);
313+
expect(calls.removeLabel).toEqual([]);
314+
expect(calls.update).toEqual([]);
259315
});
260316

261-
it("removes stale Barnacle candidate and PR-limit labels from maintainer-authored PRs", async () => {
317+
it("leaves stale Barnacle labels alone on maintainer-authored PRs", async () => {
262318
const { calls, github } = barnacleGithub([
263319
file("ui/src/app.ts"),
264320
file("src/gateway/server.ts"),
@@ -282,12 +338,93 @@ describe("barnacle-auto-response", () => {
282338
},
283339
});
284340

285-
expect(calls.removeLabel).toEqual(
286-
expect.arrayContaining([
287-
expect.objectContaining({ name: candidateLabels.dirtyCandidate }),
288-
expect.objectContaining({ name: "r: too-many-prs" }),
289-
]),
290-
);
341+
expect(calls.addLabels).toEqual([]);
342+
expect(calls.createComment).toEqual([]);
343+
expect(calls.removeLabel).toEqual([]);
344+
expect(calls.update).toEqual([]);
345+
});
346+
347+
it("does not mutate maintainer-authored issues", async () => {
348+
const { calls, github } = barnacleGithub([]);
349+
350+
await runBarnacleAutoResponse({
351+
github,
352+
context: barnacleIssueContext({
353+
title: "TestFlight access",
354+
author_association: "OWNER",
355+
user: {
356+
login: "maintainer",
357+
},
358+
}),
359+
core: {
360+
info: () => undefined,
361+
},
362+
});
363+
364+
expect(calls.addLabels).toEqual([]);
365+
expect(calls.createComment).toEqual([]);
366+
expect(calls.update).toEqual([]);
367+
});
368+
369+
it("does not action close labels on maintainer-authored issues", async () => {
370+
const { calls, github } = barnacleGithub([]);
371+
372+
await runBarnacleAutoResponse({
373+
github,
374+
context: barnacleIssueContext(
375+
{
376+
title: "Need help with setup",
377+
author_association: "MEMBER",
378+
user: {
379+
login: "maintainer",
380+
},
381+
},
382+
["r: support"],
383+
{
384+
action: "labeled",
385+
label: { name: "r: support" },
386+
},
387+
),
388+
core: {
389+
info: () => undefined,
390+
},
391+
});
392+
393+
expect(calls.createComment).toEqual([]);
394+
expect(calls.update).toEqual([]);
395+
});
396+
397+
it("does not respond to maintainer comments on contributor items", async () => {
398+
const { calls, github } = barnacleGithub([], { maintainerLogins: ["maintainer"] });
399+
400+
await runBarnacleAutoResponse({
401+
github,
402+
context: barnacleIssueContext(
403+
{
404+
title: "Contributor issue",
405+
user: {
406+
login: "contributor",
407+
},
408+
},
409+
[],
410+
{
411+
action: "created",
412+
comment: {
413+
body: "testflight",
414+
user: {
415+
login: "maintainer",
416+
type: "User",
417+
},
418+
},
419+
},
420+
),
421+
core: {
422+
info: () => undefined,
423+
},
424+
});
425+
426+
expect(calls.createComment).toEqual([]);
427+
expect(calls.update).toEqual([]);
291428
});
292429

293430
it("does not close automation PRs for the active PR limit", async () => {

0 commit comments

Comments
 (0)