Skip to content

Commit fae6a92

Browse files
aaronpowellCopilot
aaronpowell
and
Copilot
authored
Centralize label management and fix permission issues (#2018)
* fix: Allow label operations on pull requests in external plugin approval workflow The sync-merged-pr-labels job needs pull-requests: write permission to add/remove labels on merged PRs. Previously it only had issues: write which is for issues, not pull requests. This fixes the permission error when workflows try to modify PR labels from a non-contributor account. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: Handle 403 permission errors when creating external plugin intake labels When running on PRs from fork contributors, the GitHub token may not have permission to create labels in the repository. This is expected and should not cause the workflow to fail. Allow the ensureLabel function to gracefully handle 403 Forbidden errors in addition to 422 (label already exists) errors. This fixes the sync-pr-state job failure in external-plugin-pr-quality-gates.yml when run on PRs from external contributors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: Centralize label management into a single workflow_dispatch workflow Create a new 'setup-labels' workflow that is manually dispatched and handles all label creation and updates. This workflow: - Creates all labels used by the repository - Updates descriptions if labels already exist - Reports success/failure counts - Fails if any labels cannot be created All individual workflows now assume labels exist and will fail (loudly) if they don't. This makes it clear to maintainers when the setup-labels workflow needs to be dispatched: - label-pr-intent.yml - skill-check-comment.yml - external-plugin-approval-command.yml - external-plugin-command-router.yml - external-plugin-rereview.yml - external-plugin-rereview-command.yml - eng/external-plugin-intake-state.mjs This approach is better because: - Single source of truth for label definitions - Avoids permission issues with fork contributors - Clear failure modes when labels are missing - Easier to maintain consistent label configuration - No more scattered label creation logic across workflows Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove unused ensureLabel methods and managedLabels constants Labels are now centrally managed by the setup-labels workflow and assumed to exist in all other workflows. Removed: - ensureLabel() methods from all 6 workflows and 1 JS module - managedLabels constants that were only used by ensureLabel - Promise.all() calls that invoked ensureLabel for each label - Updated syncManagedLabels in skill-check-comment.yml to remove ensureLabel call All workflows now assume labels exist and will fail if they don't, which is the desired behavior—it signals maintainers to dispatch the setup-labels workflow when new labels need to be created. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 5fb37f6 commit fae6a92

8 files changed

Lines changed: 170 additions & 274 deletions

‎.github/workflows/external-plugin-approval-command.yml‎

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ concurrency:
99
cancel-in-progress: false
1010

1111
permissions:
12-
issues: write
12+
pull-requests: write
13+
contents: read
1314

1415
jobs:
1516
sync-merged-pr-labels:
@@ -25,20 +26,6 @@ jobs:
2526
const prNumber = context.payload.pull_request.number;
2627
const staleLabels = ['awaiting-review', 'awaiting-approval', 'ready-for-review', 'rejected'];
2728
28-
try {
29-
await github.rest.issues.createLabel({
30-
owner: context.repo.owner,
31-
repo: context.repo.repo,
32-
name: 'approved',
33-
color: '1D76DB',
34-
description: 'Submission was approved by a maintainer'
35-
});
36-
} catch (error) {
37-
if (error.status !== 422) {
38-
throw error;
39-
}
40-
}
41-
4229
const { data: currentLabels } = await github.rest.issues.listLabelsOnIssue({
4330
owner: context.repo.owner,
4431
repo: context.repo.repo,

‎.github/workflows/external-plugin-command-router.yml‎

Lines changed: 8 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -275,49 +275,6 @@ jobs:
275275
PR_NUMBER: ${{ steps.approval_pr.outputs.pr-number }}
276276
with:
277277
script: |
278-
const managedLabels = {
279-
'external-plugin': {
280-
color: 'FEF2C0',
281-
description: 'Public external plugin submission'
282-
},
283-
'awaiting-review': {
284-
color: 'FBCA04',
285-
description: 'Submission is waiting for automated intake validation'
286-
},
287-
'ready-for-review': {
288-
color: '0E8A16',
289-
description: 'Submission passed intake validation and is ready for maintainer review'
290-
},
291-
'requires-submitter-fixes': {
292-
color: 'D93F0B',
293-
description: 'Submission has quality-gate findings that submitter must fix before maintainer review'
294-
},
295-
'approved': {
296-
color: '1D76DB',
297-
description: 'Submission was approved by a maintainer'
298-
},
299-
'rejected': {
300-
color: 'B60205',
301-
description: 'Submission was rejected by a maintainer'
302-
}
303-
};
304-
305-
async function ensureLabel(name, config) {
306-
try {
307-
await github.rest.issues.createLabel({
308-
owner: context.repo.owner,
309-
repo: context.repo.repo,
310-
name,
311-
color: config.color,
312-
description: config.description
313-
});
314-
} catch (error) {
315-
if (error.status !== 422) {
316-
throw error;
317-
}
318-
}
319-
}
320-
321278
async function removeLabel(issueNumber, name) {
322279
try {
323280
await github.rest.issues.removeLabel({
@@ -334,7 +291,14 @@ jobs:
334291
}
335292
336293
async function syncIssueLabels(issueNumber, desiredLabels) {
337-
await Promise.all(Object.entries(managedLabels).map(([name, config]) => ensureLabel(name, config)));
294+
const managedLabels = {
295+
'external-plugin': true,
296+
'awaiting-review': true,
297+
'ready-for-review': true,
298+
'requires-submitter-fixes': true,
299+
'approved': true,
300+
'rejected': true
301+
};
338302
339303
const currentLabels = await github.paginate(github.rest.issues.listLabelsOnIssue, {
340304
owner: context.repo.owner,
@@ -438,49 +402,6 @@ jobs:
438402
PLUGIN_NAME: ${{ steps.parse.outputs.plugin-name }}
439403
with:
440404
script: |
441-
const managedLabels = {
442-
'external-plugin': {
443-
color: 'FEF2C0',
444-
description: 'Public external plugin submission'
445-
},
446-
'awaiting-review': {
447-
color: 'FBCA04',
448-
description: 'Submission is waiting for automated intake validation'
449-
},
450-
'ready-for-review': {
451-
color: '0E8A16',
452-
description: 'Submission passed intake validation and is ready for maintainer review'
453-
},
454-
'requires-submitter-fixes': {
455-
color: 'D93F0B',
456-
description: 'Submission has quality-gate findings that submitter must fix before maintainer review'
457-
},
458-
'approved': {
459-
color: '1D76DB',
460-
description: 'Submission was approved by a maintainer'
461-
},
462-
'rejected': {
463-
color: 'B60205',
464-
description: 'Submission was rejected by a maintainer'
465-
}
466-
};
467-
468-
async function ensureLabel(name, config) {
469-
try {
470-
await github.rest.issues.createLabel({
471-
owner: context.repo.owner,
472-
repo: context.repo.repo,
473-
name,
474-
color: config.color,
475-
description: config.description
476-
});
477-
} catch (error) {
478-
if (error.status !== 422) {
479-
throw error;
480-
}
481-
}
482-
}
483-
484405
async function removeLabel(name) {
485406
try {
486407
await github.rest.issues.removeLabel({
@@ -496,7 +417,6 @@ jobs:
496417
}
497418
}
498419
499-
await Promise.all(Object.entries(managedLabels).map(([name, config]) => ensureLabel(name, config)));
500420
await github.rest.issues.addLabels({
501421
owner: context.repo.owner,
502422
repo: context.repo.repo,

‎.github/workflows/external-plugin-rereview-command.yml‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -180,34 +180,6 @@ jobs:
180180
PLUGIN_NAME: ${{ steps.parse.outputs.plugin-name }}
181181
with:
182182
script: |
183-
const managedLabels = {
184-
're-review-due': {
185-
color: 'FBCA04',
186-
description: 'Approved external plugin is due for six-month re-review'
187-
},
188-
're-review-follow-up': {
189-
color: 'D4C5F9',
190-
description: 'Six-month re-review needs maintainer follow-up before a final decision'
191-
}
192-
};
193-
194-
async function ensureLabel(name, config) {
195-
try {
196-
await github.rest.issues.createLabel({
197-
owner: context.repo.owner,
198-
repo: context.repo.repo,
199-
name,
200-
color: config.color,
201-
description: config.description
202-
});
203-
} catch (error) {
204-
if (error.status !== 422) {
205-
throw error;
206-
}
207-
}
208-
}
209-
210-
await Promise.all(Object.entries(managedLabels).map(([name, config]) => ensureLabel(name, config)));
211183
await github.rest.issues.addLabels({
212184
owner: context.repo.owner,
213185
repo: context.repo.repo,

‎.github/workflows/external-plugin-rereview.yml‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -26,37 +26,6 @@ jobs:
2626
const rereview = await import(pathToFileURL(path.join(process.env.GITHUB_WORKSPACE, 'eng', 'external-plugin-rereview.mjs')).href);
2727
const validation = await import(pathToFileURL(path.join(process.env.GITHUB_WORKSPACE, 'eng', 'external-plugin-validation.mjs')).href);
2828
29-
const managedLabels = {
30-
[rereview.REREVIEW_LABELS.due]: {
31-
color: 'FBCA04',
32-
description: 'Approved external plugin is due for six-month re-review'
33-
},
34-
[rereview.REREVIEW_LABELS.followUp]: {
35-
color: 'D4C5F9',
36-
description: 'Six-month re-review needs maintainer follow-up before a final decision'
37-
},
38-
[rereview.REREVIEW_LABELS.removed]: {
39-
color: 'B60205',
40-
description: 'External plugin was removed from the marketplace after re-review'
41-
}
42-
};
43-
44-
async function ensureLabel(name, config) {
45-
try {
46-
await github.rest.issues.createLabel({
47-
owner: context.repo.owner,
48-
repo: context.repo.repo,
49-
name,
50-
color: config.color,
51-
description: config.description
52-
});
53-
} catch (error) {
54-
if (error.status !== 422) {
55-
throw error;
56-
}
57-
}
58-
}
59-
6029
async function removeLabel(issueNumber, label) {
6130
try {
6231
await github.rest.issues.removeLabel({
@@ -90,8 +59,6 @@ jobs:
9059
return Math.max(0, Math.floor(Math.abs(diff) / (1000 * 60 * 60 * 24)));
9160
}
9261
93-
await Promise.all(Object.entries(managedLabels).map(([name, config]) => ensureLabel(name, config)));
94-
9562
const { plugins, errors } = validation.readExternalPlugins({ policy: 'marketplace' });
9663
if (errors.length > 0) {
9764
core.setFailed(errors.join('\n'));

‎.github/workflows/label-pr-intent.yml‎

Lines changed: 12 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -20,54 +20,18 @@ jobs:
2020
with:
2121
script: |
2222
const managedLabels = {
23-
'targets-main': {
24-
color: 'B60205',
25-
description: 'PR targets main instead of staged'
26-
},
27-
'branched-main': {
28-
color: 'D93F0B',
29-
description: 'PR appears to include plugin files materialized from main'
30-
},
31-
'skills': {
32-
color: '1D76DB',
33-
description: 'PR touches skills'
34-
},
35-
'plugin': {
36-
color: '5319E7',
37-
description: 'PR touches plugins'
38-
},
39-
'agent': {
40-
color: '0E8A16',
41-
description: 'PR touches agents'
42-
},
43-
'instructions': {
44-
color: 'FBCA04',
45-
description: 'PR touches instructions'
46-
},
47-
'new-submission': {
48-
color: '006B75',
49-
description: 'PR adds at least one new contribution'
50-
},
51-
'website-update': {
52-
color: '0052CC',
53-
description: 'PR touches website content or code'
54-
},
55-
'external-plugin': {
56-
color: 'FEF2C0',
57-
description: 'PR updates plugins/external.json'
58-
},
59-
'hooks': {
60-
color: 'C2E0C6',
61-
description: 'PR touches hooks'
62-
},
63-
'workflow': {
64-
color: 'BFD4F2',
65-
description: 'PR touches workflow automation'
66-
},
67-
'canvas-extension': {
68-
color: 'E4B9FF',
69-
description: 'PR touches canvas extensions'
70-
}
23+
'targets-main': true,
24+
'branched-main': true,
25+
'skills': true,
26+
'plugin': true,
27+
'agent': true,
28+
'instructions': true,
29+
'new-submission': true,
30+
'website-update': true,
31+
'external-plugin': true,
32+
'hooks': true,
33+
'workflow': true,
34+
'canvas-extension': true
7135
};
7236
7337
const matchesAny = (filename, patterns) => patterns.some((pattern) => pattern.test(filename));
@@ -95,22 +59,6 @@ jobs:
9559
}
9660
}
9761
98-
async function ensureLabel(name, { color, description }) {
99-
try {
100-
await github.rest.issues.createLabel({
101-
owner: context.repo.owner,
102-
repo: context.repo.repo,
103-
name,
104-
color,
105-
description
106-
});
107-
} catch (error) {
108-
if (error.status !== 422) {
109-
throw error;
110-
}
111-
}
112-
}
113-
11462
const files = await listAllFiles();
11563
const filenames = files.map((file) => file.filename);
11664
@@ -214,10 +162,6 @@ jobs:
214162
}
215163
}
216164
217-
await Promise.all(
218-
Object.entries(managedLabels).map(([name, config]) => ensureLabel(name, config))
219-
);
220-
221165
const currentLabels = await github.paginate(github.rest.issues.listLabelsOnIssue, {
222166
owner: context.repo.owner,
223167
repo: context.repo.repo,

0 commit comments

Comments
 (0)