From fae6a92c9d9b3f80abcc53df0e772dc9b363d4a1 Mon Sep 17 00:00:00 2001 From: Aaron Powell Date: Wed, 17 Jun 2026 14:12:07 +1000 Subject: [PATCH] Centralize label management and fix permission issues (#2018) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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> --- .../external-plugin-approval-command.yml | 17 +- .../external-plugin-command-router.yml | 96 +----------- .../external-plugin-rereview-command.yml | 28 ---- .../workflows/external-plugin-rereview.yml | 33 ---- .github/workflows/label-pr-intent.yml | 80 ++-------- .github/workflows/setup-labels.yml | 148 ++++++++++++++++++ .github/workflows/skill-check-comment.yml | 20 --- eng/external-plugin-intake-state.mjs | 22 --- 8 files changed, 170 insertions(+), 274 deletions(-) create mode 100644 .github/workflows/setup-labels.yml diff --git a/.github/workflows/external-plugin-approval-command.yml b/.github/workflows/external-plugin-approval-command.yml index 78b411d6..486e5c0a 100644 --- a/.github/workflows/external-plugin-approval-command.yml +++ b/.github/workflows/external-plugin-approval-command.yml @@ -9,7 +9,8 @@ concurrency: cancel-in-progress: false permissions: - issues: write + pull-requests: write + contents: read jobs: sync-merged-pr-labels: @@ -25,20 +26,6 @@ jobs: const prNumber = context.payload.pull_request.number; const staleLabels = ['awaiting-review', 'awaiting-approval', 'ready-for-review', 'rejected']; - try { - await github.rest.issues.createLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - name: 'approved', - color: '1D76DB', - description: 'Submission was approved by a maintainer' - }); - } catch (error) { - if (error.status !== 422) { - throw error; - } - } - const { data: currentLabels } = await github.rest.issues.listLabelsOnIssue({ owner: context.repo.owner, repo: context.repo.repo, diff --git a/.github/workflows/external-plugin-command-router.yml b/.github/workflows/external-plugin-command-router.yml index e5680c2e..5f0b77f6 100644 --- a/.github/workflows/external-plugin-command-router.yml +++ b/.github/workflows/external-plugin-command-router.yml @@ -275,49 +275,6 @@ jobs: PR_NUMBER: ${{ steps.approval_pr.outputs.pr-number }} with: script: | - const managedLabels = { - 'external-plugin': { - color: 'FEF2C0', - description: 'Public external plugin submission' - }, - 'awaiting-review': { - color: 'FBCA04', - description: 'Submission is waiting for automated intake validation' - }, - 'ready-for-review': { - color: '0E8A16', - description: 'Submission passed intake validation and is ready for maintainer review' - }, - 'requires-submitter-fixes': { - color: 'D93F0B', - description: 'Submission has quality-gate findings that submitter must fix before maintainer review' - }, - 'approved': { - color: '1D76DB', - description: 'Submission was approved by a maintainer' - }, - 'rejected': { - color: 'B60205', - description: 'Submission was rejected by a maintainer' - } - }; - - async function ensureLabel(name, config) { - try { - await github.rest.issues.createLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - name, - color: config.color, - description: config.description - }); - } catch (error) { - if (error.status !== 422) { - throw error; - } - } - } - async function removeLabel(issueNumber, name) { try { await github.rest.issues.removeLabel({ @@ -334,7 +291,14 @@ jobs: } async function syncIssueLabels(issueNumber, desiredLabels) { - await Promise.all(Object.entries(managedLabels).map(([name, config]) => ensureLabel(name, config))); + const managedLabels = { + 'external-plugin': true, + 'awaiting-review': true, + 'ready-for-review': true, + 'requires-submitter-fixes': true, + 'approved': true, + 'rejected': true + }; const currentLabels = await github.paginate(github.rest.issues.listLabelsOnIssue, { owner: context.repo.owner, @@ -438,49 +402,6 @@ jobs: PLUGIN_NAME: ${{ steps.parse.outputs.plugin-name }} with: script: | - const managedLabels = { - 'external-plugin': { - color: 'FEF2C0', - description: 'Public external plugin submission' - }, - 'awaiting-review': { - color: 'FBCA04', - description: 'Submission is waiting for automated intake validation' - }, - 'ready-for-review': { - color: '0E8A16', - description: 'Submission passed intake validation and is ready for maintainer review' - }, - 'requires-submitter-fixes': { - color: 'D93F0B', - description: 'Submission has quality-gate findings that submitter must fix before maintainer review' - }, - 'approved': { - color: '1D76DB', - description: 'Submission was approved by a maintainer' - }, - 'rejected': { - color: 'B60205', - description: 'Submission was rejected by a maintainer' - } - }; - - async function ensureLabel(name, config) { - try { - await github.rest.issues.createLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - name, - color: config.color, - description: config.description - }); - } catch (error) { - if (error.status !== 422) { - throw error; - } - } - } - async function removeLabel(name) { try { await github.rest.issues.removeLabel({ @@ -496,7 +417,6 @@ jobs: } } - await Promise.all(Object.entries(managedLabels).map(([name, config]) => ensureLabel(name, config))); await github.rest.issues.addLabels({ owner: context.repo.owner, repo: context.repo.repo, diff --git a/.github/workflows/external-plugin-rereview-command.yml b/.github/workflows/external-plugin-rereview-command.yml index e34f6ecb..ce96cf56 100644 --- a/.github/workflows/external-plugin-rereview-command.yml +++ b/.github/workflows/external-plugin-rereview-command.yml @@ -180,34 +180,6 @@ jobs: PLUGIN_NAME: ${{ steps.parse.outputs.plugin-name }} with: script: | - const managedLabels = { - 're-review-due': { - color: 'FBCA04', - description: 'Approved external plugin is due for six-month re-review' - }, - 're-review-follow-up': { - color: 'D4C5F9', - description: 'Six-month re-review needs maintainer follow-up before a final decision' - } - }; - - async function ensureLabel(name, config) { - try { - await github.rest.issues.createLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - name, - color: config.color, - description: config.description - }); - } catch (error) { - if (error.status !== 422) { - throw error; - } - } - } - - await Promise.all(Object.entries(managedLabels).map(([name, config]) => ensureLabel(name, config))); await github.rest.issues.addLabels({ owner: context.repo.owner, repo: context.repo.repo, diff --git a/.github/workflows/external-plugin-rereview.yml b/.github/workflows/external-plugin-rereview.yml index 1cf07459..c359c47b 100644 --- a/.github/workflows/external-plugin-rereview.yml +++ b/.github/workflows/external-plugin-rereview.yml @@ -26,37 +26,6 @@ jobs: const rereview = await import(pathToFileURL(path.join(process.env.GITHUB_WORKSPACE, 'eng', 'external-plugin-rereview.mjs')).href); const validation = await import(pathToFileURL(path.join(process.env.GITHUB_WORKSPACE, 'eng', 'external-plugin-validation.mjs')).href); - const managedLabels = { - [rereview.REREVIEW_LABELS.due]: { - color: 'FBCA04', - description: 'Approved external plugin is due for six-month re-review' - }, - [rereview.REREVIEW_LABELS.followUp]: { - color: 'D4C5F9', - description: 'Six-month re-review needs maintainer follow-up before a final decision' - }, - [rereview.REREVIEW_LABELS.removed]: { - color: 'B60205', - description: 'External plugin was removed from the marketplace after re-review' - } - }; - - async function ensureLabel(name, config) { - try { - await github.rest.issues.createLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - name, - color: config.color, - description: config.description - }); - } catch (error) { - if (error.status !== 422) { - throw error; - } - } - } - async function removeLabel(issueNumber, label) { try { await github.rest.issues.removeLabel({ @@ -90,8 +59,6 @@ jobs: return Math.max(0, Math.floor(Math.abs(diff) / (1000 * 60 * 60 * 24))); } - await Promise.all(Object.entries(managedLabels).map(([name, config]) => ensureLabel(name, config))); - const { plugins, errors } = validation.readExternalPlugins({ policy: 'marketplace' }); if (errors.length > 0) { core.setFailed(errors.join('\n')); diff --git a/.github/workflows/label-pr-intent.yml b/.github/workflows/label-pr-intent.yml index 825fbf2a..b6b0a8d1 100644 --- a/.github/workflows/label-pr-intent.yml +++ b/.github/workflows/label-pr-intent.yml @@ -20,54 +20,18 @@ jobs: with: script: | const managedLabels = { - 'targets-main': { - color: 'B60205', - description: 'PR targets main instead of staged' - }, - 'branched-main': { - color: 'D93F0B', - description: 'PR appears to include plugin files materialized from main' - }, - 'skills': { - color: '1D76DB', - description: 'PR touches skills' - }, - 'plugin': { - color: '5319E7', - description: 'PR touches plugins' - }, - 'agent': { - color: '0E8A16', - description: 'PR touches agents' - }, - 'instructions': { - color: 'FBCA04', - description: 'PR touches instructions' - }, - 'new-submission': { - color: '006B75', - description: 'PR adds at least one new contribution' - }, - 'website-update': { - color: '0052CC', - description: 'PR touches website content or code' - }, - 'external-plugin': { - color: 'FEF2C0', - description: 'PR updates plugins/external.json' - }, - 'hooks': { - color: 'C2E0C6', - description: 'PR touches hooks' - }, - 'workflow': { - color: 'BFD4F2', - description: 'PR touches workflow automation' - }, - 'canvas-extension': { - color: 'E4B9FF', - description: 'PR touches canvas extensions' - } + 'targets-main': true, + 'branched-main': true, + 'skills': true, + 'plugin': true, + 'agent': true, + 'instructions': true, + 'new-submission': true, + 'website-update': true, + 'external-plugin': true, + 'hooks': true, + 'workflow': true, + 'canvas-extension': true }; const matchesAny = (filename, patterns) => patterns.some((pattern) => pattern.test(filename)); @@ -95,22 +59,6 @@ jobs: } } - async function ensureLabel(name, { color, description }) { - try { - await github.rest.issues.createLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - name, - color, - description - }); - } catch (error) { - if (error.status !== 422) { - throw error; - } - } - } - const files = await listAllFiles(); const filenames = files.map((file) => file.filename); @@ -214,10 +162,6 @@ jobs: } } - await Promise.all( - Object.entries(managedLabels).map(([name, config]) => ensureLabel(name, config)) - ); - const currentLabels = await github.paginate(github.rest.issues.listLabelsOnIssue, { owner: context.repo.owner, repo: context.repo.repo, diff --git a/.github/workflows/setup-labels.yml b/.github/workflows/setup-labels.yml new file mode 100644 index 00000000..7098546c --- /dev/null +++ b/.github/workflows/setup-labels.yml @@ -0,0 +1,148 @@ +name: Setup Repository Labels + +on: + workflow_dispatch + +permissions: + issues: write + +jobs: + setup-labels: + runs-on: ubuntu-latest + steps: + - name: Create or update labels + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + with: + script: | + const labels = { + // Intent labels for PR categorization + 'targets-main': { + color: 'B60205', + description: 'PR targets main instead of staged' + }, + 'branched-main': { + color: 'D93F0B', + description: 'PR appears to include plugin files materialized from main' + }, + 'skills': { + color: '1D76DB', + description: 'PR touches skills' + }, + 'plugin': { + color: '5319E7', + description: 'PR touches plugins' + }, + 'agent': { + color: '0E8A16', + description: 'PR touches agents' + }, + 'instructions': { + color: 'FBCA04', + description: 'PR touches instructions' + }, + 'new-submission': { + color: '006B75', + description: 'PR adds at least one new contribution' + }, + 'website-update': { + color: '0052CC', + description: 'PR touches website content or code' + }, + 'external-plugin': { + color: 'FEF2C0', + description: 'Public external plugin submission' + }, + 'hooks': { + color: 'C2E0C6', + description: 'PR touches hooks' + }, + 'workflow': { + color: 'BFD4F2', + description: 'PR touches workflow automation' + }, + // External plugin intake state labels + 'awaiting-review': { + color: 'FBCA04', + description: 'Submission is waiting for automated intake validation' + }, + 'ready-for-review': { + color: '0E8A16', + description: 'Submission passed intake validation and is ready for maintainer review' + }, + 'requires-submitter-fixes': { + color: 'D93F0B', + description: 'Submission has quality-gate findings that submitter must fix before maintainer review' + }, + 'approved': { + color: '1D76DB', + description: 'Submission was approved by a maintainer' + }, + 'rejected': { + color: 'B60205', + description: 'Submission was rejected by a maintainer' + }, + // Re-review labels + 'removed': { + color: 'B60205', + description: 'External plugin was removed from the marketplace after re-review' + }, + 're-review-follow-up': { + color: 'D4C5F9', + description: 'Six-month re-review needs maintainer follow-up before a final decision' + }, + 'awaiting-approval': { + color: 'FBCA04', + description: 'External plugin awaiting maintainer approval' + } + }; + + let created = 0; + let updated = 0; + let failed = 0; + + for (const [name, config] of Object.entries(labels)) { + try { + await github.rest.issues.createLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + name, + color: config.color, + description: config.description + }); + created++; + core.info(`✓ Created label: ${name}`); + } catch (error) { + if (error.status === 422) { + // Label already exists, try to update it + try { + await github.rest.issues.updateLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + name, + color: config.color, + description: config.description + }); + updated++; + core.info(`✓ Updated label: ${name}`); + } catch (updateError) { + failed++; + core.error(`✗ Failed to update label ${name}: ${updateError.message}`); + } + } else { + failed++; + core.error(`✗ Failed to create label ${name}: ${error.message}`); + } + } + } + + core.info(` + Label setup complete: + - Created: ${created} + - Updated: ${updated} + - Failed: ${failed} + - Total: ${Object.keys(labels).length} + `); + + if (failed > 0) { + throw new Error(`Failed to setup ${failed} label(s)`); + } diff --git a/.github/workflows/skill-check-comment.yml b/.github/workflows/skill-check-comment.yml index 7c27c243..f427cc71 100644 --- a/.github/workflows/skill-check-comment.yml +++ b/.github/workflows/skill-check-comment.yml @@ -42,27 +42,7 @@ jobs: } }; - async function ensureLabel(name, { color, description }) { - try { - await github.rest.issues.createLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - name, - color, - description - }); - } catch (error) { - if (error.status !== 422) { - throw error; - } - } - } - async function syncManagedLabels(issueNumber, desiredLabels) { - await Promise.all( - Object.entries(managedLabels).map(([name, config]) => ensureLabel(name, config)) - ); - const currentLabels = await github.paginate(github.rest.issues.listLabelsOnIssue, { owner: context.repo.owner, repo: context.repo.repo, diff --git a/eng/external-plugin-intake-state.mjs b/eng/external-plugin-intake-state.mjs index 2f7a09e9..f9351281 100644 --- a/eng/external-plugin-intake-state.mjs +++ b/eng/external-plugin-intake-state.mjs @@ -33,22 +33,6 @@ const EXTERNAL_PLUGIN_INTAKE_SYNC_LABELS = Object.freeze([ "rejected", ]); -async function ensureLabel({ github, owner, repo, name, config }) { - try { - await github.rest.issues.createLabel({ - owner, - repo, - name, - color: config.color, - description: config.description, - }); - } catch (error) { - if (error.status !== 422) { - throw error; - } - } -} - async function removeLabel({ github, owner, repo, issueNumber, name }) { try { await github.rest.issues.removeLabel({ @@ -65,12 +49,6 @@ async function removeLabel({ github, owner, repo, issueNumber, name }) { } export async function syncExternalPluginIntakeLabels({ github, owner, repo, issueNumber, desiredLabels }) { - await Promise.all( - Object.entries(EXTERNAL_PLUGIN_INTAKE_LABELS).map(([name, config]) => - ensureLabel({ github, owner, repo, name, config }) - ) - ); - const currentLabels = await github.paginate(github.rest.issues.listLabelsOnIssue, { owner, repo,