Skip to content

[WRONG BRANCH] fix(ci): invalidate maintainer-sponsored on synchronize (treat sponsorship as head-specific) #46

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
luvs01 wants to merge 1 commit into
base: main
Choose a base branch
from
+22 −11
Draft

[WRONG BRANCH] fix(ci): invalidate maintainer-sponsored on synchronize (treat sponsorship as head-specific) #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .github/scripts/pr-sponsored-surface.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ const RESTRICTED_PREFIXES = [
"src/oauth/",
];

const HEAD_SPECIFIC_APPROVAL_LABELS = [
"test-exception-approved",
"suppression-approved",
"generated-change-approved",
"dependency-change-approved",
"maintainer-sponsored",
];

const RESTRICTED_FILES = new Set([
// Release and packaging automation executed by the release workflow.
"scripts/release.ts",
Expand Down Expand Up @@ -80,6 +88,7 @@ function assessSponsoredSurface({
}

module.exports = {
HEAD_SPECIFIC_APPROVAL_LABELS,
RESTRICTED_FILES,
RESTRICTED_PREFIXES,
assessSponsoredSurface,
Expand Down
12 changes: 11 additions & 1 deletion .github/scripts/pr-sponsored-surface.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,17 @@

const { describe, it } = require("node:test");
const assert = require("node:assert/strict");
const { assessSponsoredSurface, isRestrictedPath } = require("./pr-sponsored-surface.cjs");
const {
HEAD_SPECIFIC_APPROVAL_LABELS,
assessSponsoredSurface,
isRestrictedPath,
} = require("./pr-sponsored-surface.cjs");

describe("HEAD_SPECIFIC_APPROVAL_LABELS", () => {
it("invalidates maintainer sponsorship when the reviewed revision changes", () => {
assert.ok(HEAD_SPECIFIC_APPROVAL_LABELS.includes("maintainer-sponsored"));
});
Comment on lines +11 to +14

@coderabbitai coderabbitai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Assert the complete head-specific approval-label contract.

Line 13 checks only maintainer-sponsored. The test still passes if any of the other four labels are removed or renamed. The synchronization workflow would then stop clearing that approval after a new commit.

Assert the exact five-label set.

Proposed assertion 
-    assert.ok(HEAD_SPECIFIC_APPROVAL_LABELS.includes("maintainer-sponsored"));
+    assert.deepStrictEqual([...HEAD_SPECIFIC_APPROVAL_LABELS].sort(), [
+      "dependency-change-approved",
+      "generated-change-approved",
+      "maintainer-sponsored",
+      "suppression-approved",
+      "test-exception-approved",
+    ].sort());

As per path instructions, .github/** is a security boundary, and workflow changes require explicit security review per MAINTAINERS.md.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
describe("HEAD_SPECIFIC_APPROVAL_LABELS", () => {
it("invalidates maintainer sponsorship when the reviewed revision changes", () => {
assert.ok(HEAD_SPECIFIC_APPROVAL_LABELS.includes("maintainer-sponsored"));
});
describe("HEAD_SPECIFIC_APPROVAL_LABELS", () => {
it("invalidates maintainer sponsorship when the reviewed revision changes", () => {
assert.deepStrictEqual([...HEAD_SPECIFIC_APPROVAL_LABELS].sort(), [
"dependency-change-approved",
"generated-change-approved",
"maintainer-sponsored",
"suppression-approved",
"test-exception-approved",
].sort());
});
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/scripts/pr-sponsored-surface.test.cjs around lines 11 - 14, Update
the HEAD_SPECIFIC_APPROVAL_LABELS test to assert the exact five-label contract,
rather than only checking that "maintainer-sponsored" is included. Use the
complete expected label set defined by HEAD_SPECIFIC_APPROVAL_LABELS and
preserve the existing test’s purpose of validating all labels cleared when the
reviewed revision changes.

Source: Path instructions

});

describe("isRestrictedPath", () => {
it("covers auth, workflow, release, and dependency surfaces", () => {
Expand Down
12 changes: 2 additions & 10 deletions .github/workflows/pr-hygiene.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ jobs:
const { assessHygiene } = require(
path.join(process.cwd(), ".github", "scripts", "pr-hygiene.cjs"),
);
const { assessSponsoredSurface } = require(
const { HEAD_SPECIFIC_APPROVAL_LABELS, assessSponsoredSurface } = require(
path.join(process.cwd(), ".github", "scripts", "pr-sponsored-surface.cjs"),
);

Expand Down Expand Up @@ -79,12 +79,7 @@ jobs:
// them, so a contributor cannot obtain one narrow exception and
// then push unreviewed violations under the same label.
if (context.payload.action === "synchronize") {
for (const name of [
"test-exception-approved",
"suppression-approved",
"generated-change-approved",
"dependency-change-approved",
]) {
for (const name of HEAD_SPECIFIC_APPROVAL_LABELS) {
if (labels.has(name)) {
await github.rest.issues.removeLabel({
owner, repo, issue_number: pull_number, name,
Expand All @@ -93,9 +88,6 @@ jobs:
}
}
}
// Sponsorship is head-independent: it is about which surfaces the
// change touches, not about the state of a particular revision, so
// it is NOT cleared by the synchronize sweep above.
const failures = [
...assessHygiene({ files, labels: [...labels] }),
...assessSponsoredSurface({
Expand Down
Loading