Skip to content

Commit 58e49d6

Browse files
committed
fix: explain author skip restrictions
1 parent 305bbfb commit 58e49d6

4 files changed

Lines changed: 43 additions & 15 deletions

File tree

__tests__/unit/github/interaction.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,9 @@ describe('ReviewInteractionHandler', () => {
183183
expect(octokit.rest.issues.createComment).toHaveBeenCalledWith(
184184
expect.objectContaining({
185185
issue_number: 123,
186-
body: expect.stringContaining('cannot skip this major finding'),
186+
body: expect.stringContaining(
187+
'PR authors cannot override blocking ReviewRouter findings by default'
188+
),
187189
})
188190
);
189191
});

dist/index.js

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27314,12 +27314,9 @@ var ReviewInteractionHandler = class {
2731427314
const severity = normalizeSeverity(extractInlineSeverity(parent.body));
2731527315
const role = await this.getRole(actor);
2731627316
const prAuthor = payload.pull_request?.user?.login || "";
27317-
const allowed = isRoleAllowed(role, severity, actor, prAuthor);
27318-
if (!allowed) {
27319-
await this.postNotice(
27320-
prNumber,
27321-
`@${actor} cannot ${command.kind} this ${severity} finding. Required role: ${severity === "minor" ? "write, maintain, or admin" : "maintain or admin"}.`
27322-
);
27317+
const denialReason = getRoleDenialReason(role, severity, actor, prAuthor);
27318+
if (denialReason) {
27319+
await this.postNotice(prNumber, denialReason);
2732327320
return;
2732427321
}
2732527322
const fingerprint = extractFindingFingerprint(parent.body) || findingFingerprintFromInlineComment(
@@ -27495,6 +27492,16 @@ function isRoleAllowed(role, severity, actor, prAuthor) {
2749527492
}
2749627493
return role === "write" || role === "maintain" || role === "admin";
2749727494
}
27495+
function getRoleDenialReason(role, severity, actor, prAuthor) {
27496+
if (isRoleAllowed(role, severity, actor, prAuthor)) {
27497+
return null;
27498+
}
27499+
const isBlocking = severity === "critical" || severity === "major";
27500+
if (isBlocking && actor.toLowerCase() === prAuthor.toLowerCase() && process.env.REVIEW_ROUTER_ALLOW_AUTHOR_SKIP !== "true") {
27501+
return `@${actor} cannot skip this ${severity} finding because PR authors cannot override blocking ReviewRouter findings by default. A maintainer or admin who is not the PR author can reply \`/rr skip\`, or the repository can explicitly set \`REVIEW_ROUTER_ALLOW_AUTHOR_SKIP=true\`.`;
27502+
}
27503+
return `@${actor} cannot skip this ${severity} finding. Required role: ${severity === "minor" ? "write, maintain, or admin" : "maintain or admin"}.`;
27504+
}
2749827505
function readEventPayload() {
2749927506
const eventPath = process.env.GITHUB_EVENT_PATH;
2750027507
if (!eventPath) {

dist/index.js.map

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/github/interaction.ts

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,12 +106,9 @@ export class ReviewInteractionHandler {
106106
const severity = normalizeSeverity(extractInlineSeverity(parent.body));
107107
const role = await this.getRole(actor);
108108
const prAuthor = payload.pull_request?.user?.login || '';
109-
const allowed = isRoleAllowed(role, severity, actor, prAuthor);
110-
if (!allowed) {
111-
await this.postNotice(
112-
prNumber,
113-
`@${actor} cannot ${command.kind} this ${severity} finding. Required role: ${severity === 'minor' ? 'write, maintain, or admin' : 'maintain or admin'}.`
114-
);
109+
const denialReason = getRoleDenialReason(role, severity, actor, prAuthor);
110+
if (denialReason) {
111+
await this.postNotice(prNumber, denialReason);
115112
return;
116113
}
117114

@@ -336,6 +333,28 @@ function isRoleAllowed(
336333
return role === 'write' || role === 'maintain' || role === 'admin';
337334
}
338335

336+
function getRoleDenialReason(
337+
role: RepoRole,
338+
severity: Severity,
339+
actor: string,
340+
prAuthor: string
341+
): string | null {
342+
if (isRoleAllowed(role, severity, actor, prAuthor)) {
343+
return null;
344+
}
345+
346+
const isBlocking = severity === 'critical' || severity === 'major';
347+
if (
348+
isBlocking &&
349+
actor.toLowerCase() === prAuthor.toLowerCase() &&
350+
process.env.REVIEW_ROUTER_ALLOW_AUTHOR_SKIP !== 'true'
351+
) {
352+
return `@${actor} cannot skip this ${severity} finding because PR authors cannot override blocking ReviewRouter findings by default. A maintainer or admin who is not the PR author can reply \`/rr skip\`, or the repository can explicitly set \`REVIEW_ROUTER_ALLOW_AUTHOR_SKIP=true\`.`;
353+
}
354+
355+
return `@${actor} cannot skip this ${severity} finding. Required role: ${severity === 'minor' ? 'write, maintain, or admin' : 'maintain or admin'}.`;
356+
}
357+
339358
function readEventPayload(): ReviewCommentEventPayload {
340359
const eventPath = process.env.GITHUB_EVENT_PATH;
341360
if (!eventPath) {

0 commit comments

Comments
 (0)