From c537a55c1d54074f996b1997b4cf187ba37eda54 Mon Sep 17 00:00:00 2001 From: Gourav NSS Date: Thu, 14 May 2026 00:49:20 +0530 Subject: [PATCH 1/4] feat: add revision guard draft conversion flow Signed-off-by: Gourav NSS --- .../revision-guard/helpers/constants.js | 24 +++ .../scripts/revision-guard/helpers/draft.js | 29 ++++ .../scripts/revision-guard/helpers/index.js | 14 ++ .../scripts/revision-guard/helpers/labels.js | 38 +++++ .github/scripts/revision-guard/index.js | 45 ++++++ .github/scripts/revision-guard/index.test.js | 141 ++++++++++++++++++ .github/workflows/revision-guard.yml | 37 +++++ 7 files changed, 328 insertions(+) create mode 100644 .github/scripts/revision-guard/helpers/constants.js create mode 100644 .github/scripts/revision-guard/helpers/draft.js create mode 100644 .github/scripts/revision-guard/helpers/index.js create mode 100644 .github/scripts/revision-guard/helpers/labels.js create mode 100644 .github/scripts/revision-guard/index.js create mode 100644 .github/scripts/revision-guard/index.test.js create mode 100644 .github/workflows/revision-guard.yml diff --git a/.github/scripts/revision-guard/helpers/constants.js b/.github/scripts/revision-guard/helpers/constants.js new file mode 100644 index 000000000..1255a99ff --- /dev/null +++ b/.github/scripts/revision-guard/helpers/constants.js @@ -0,0 +1,24 @@ +const DEFAULT_MANAGED_LABELS = [ + 'queue:junior-committer', + 'queue:committers', + 'queue:maintainers', + 'status: ready-to-merge', + 'status: failed checks', + 'open to community review', +]; + +function parseManagedLabels(value) { + if (typeof value !== 'string' || value.trim().length === 0) { + return [...DEFAULT_MANAGED_LABELS]; + } + + return value + .split(',') + .map(label => label.trim()) + .filter(Boolean); +} + +module.exports = { + DEFAULT_MANAGED_LABELS, + parseManagedLabels, +}; diff --git a/.github/scripts/revision-guard/helpers/draft.js b/.github/scripts/revision-guard/helpers/draft.js new file mode 100644 index 000000000..23c620383 --- /dev/null +++ b/.github/scripts/revision-guard/helpers/draft.js @@ -0,0 +1,29 @@ +function isBotAuthor(pr) { + const login = pr?.user?.login || ''; + return pr?.user?.type === 'Bot' || login.endsWith('[bot]'); +} + +function isDraft(pr) { + return pr?.draft === true; +} + +async function convertToDraft(github, pullRequestId) { + await github.graphql( + ` + mutation($pullRequestId: ID!) { + convertPullRequestToDraft(input: { pullRequestId: $pullRequestId }) { + pullRequest { + isDraft + } + } + } + `, + { pullRequestId } + ); +} + +module.exports = { + convertToDraft, + isBotAuthor, + isDraft, +}; diff --git a/.github/scripts/revision-guard/helpers/index.js b/.github/scripts/revision-guard/helpers/index.js new file mode 100644 index 000000000..872b3acef --- /dev/null +++ b/.github/scripts/revision-guard/helpers/index.js @@ -0,0 +1,14 @@ +const { DEFAULT_MANAGED_LABELS, parseManagedLabels } = require('./constants'); +const { convertToDraft, isBotAuthor, isDraft } = require('./draft'); +const { getManagedLabels, getPresentManagedLabels, removeManagedLabels } = require('./labels'); + +module.exports = { + DEFAULT_MANAGED_LABELS, + parseManagedLabels, + convertToDraft, + isBotAuthor, + isDraft, + getManagedLabels, + getPresentManagedLabels, + removeManagedLabels, +}; diff --git a/.github/scripts/revision-guard/helpers/labels.js b/.github/scripts/revision-guard/helpers/labels.js new file mode 100644 index 000000000..6a8bd23e6 --- /dev/null +++ b/.github/scripts/revision-guard/helpers/labels.js @@ -0,0 +1,38 @@ +const { parseManagedLabels } = require('./constants'); + +function getManagedLabels() { + return parseManagedLabels(process.env.REVISION_GUARD_MANAGED_LABELS); +} + +function getPresentManagedLabels(prLabels, managedLabels = getManagedLabels()) { + const currentLabels = Array.isArray(prLabels) + ? prLabels + .map(label => typeof label === 'string' ? label : label?.name) + .filter(Boolean) + : []; + + return managedLabels.filter(label => currentLabels.includes(label)); +} + +async function removeManagedLabels(github, { owner, repo, issueNumber, labels }) { + for (const name of labels) { + try { + await github.rest.issues.removeLabel({ + owner, + repo, + issue_number: issueNumber, + name, + }); + } catch (error) { + if (error?.status !== 404) { + throw error; + } + } + } +} + +module.exports = { + getManagedLabels, + getPresentManagedLabels, + removeManagedLabels, +}; diff --git a/.github/scripts/revision-guard/index.js b/.github/scripts/revision-guard/index.js new file mode 100644 index 000000000..0f328db31 --- /dev/null +++ b/.github/scripts/revision-guard/index.js @@ -0,0 +1,45 @@ +const { + convertToDraft, + getPresentManagedLabels, + isBotAuthor, + isDraft, + removeManagedLabels, +} = require('./helpers'); + +module.exports = async function revisionGuard({ github, context, core }) { + const reviewState = context.payload.review?.state; + const pr = context.payload.pull_request; + + if (!pr || reviewState !== 'changes_requested') { + return; + } + + if (isBotAuthor(pr)) { + core?.info?.(`Skipping PR #${pr.number} because it is bot-authored.`); + return; + } + + if (isDraft(pr)) { + core?.info?.(`Skipping PR #${pr.number} because it is already a draft.`); + return; + } + + await convertToDraft(github, pr.node_id); + core?.info?.(`Converted PR #${pr.number} to draft.`); + + const labelsToRemove = getPresentManagedLabels(pr.labels); + if (labelsToRemove.length === 0) { + core?.info?.(`No managed labels to remove for PR #${pr.number}.`); + return; + } + + await removeManagedLabels(github, { + owner: context.repo.owner, + repo: context.repo.repo, + issueNumber: pr.number, + labels: labelsToRemove, + }); + core?.info?.( + `Removed managed labels from PR #${pr.number}: ${labelsToRemove.join(', ')}.` + ); +}; diff --git a/.github/scripts/revision-guard/index.test.js b/.github/scripts/revision-guard/index.test.js new file mode 100644 index 000000000..bcddeb147 --- /dev/null +++ b/.github/scripts/revision-guard/index.test.js @@ -0,0 +1,141 @@ +const { describe, it, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); + +function freshRequire() { + const indexPath = require.resolve('./index.js'); + const helpersPath = require.resolve('./helpers/index.js'); + const labelsPath = require.resolve('./helpers/labels.js'); + const constantsPath = require.resolve('./helpers/constants.js'); + + delete require.cache[indexPath]; + delete require.cache[helpersPath]; + delete require.cache[labelsPath]; + delete require.cache[constantsPath]; + + return require('./index.js'); +} + +function createGithubMock() { + const removedLabels = []; + + return { + removedLabels, + graphqlCalls: [], + graphql: async function (_query, variables) { + this.graphqlCalls.push(variables); + }, + rest: { + issues: { + removeLabel: async ({ name }) => { + removedLabels.push(name); + }, + }, + }, + }; +} + +function createContext(overrides = {}) { + return { + repo: { owner: 'hiero-ledger', repo: 'hiero-sdk-python' }, + payload: { + review: { state: 'changes_requested' }, + pull_request: { + number: 42, + node_id: 'PR_node_42', + draft: false, + user: { login: 'contributor', type: 'User' }, + labels: [ + { name: 'queue:committers' }, + { name: 'status: ready-to-merge' }, + { name: 'some other label' }, + ], + }, + ...overrides, + }, + }; +} + +describe('revision-guard index', () => { + beforeEach(() => { + delete process.env.REVISION_GUARD_MANAGED_LABELS; + }); + + afterEach(() => { + delete process.env.REVISION_GUARD_MANAGED_LABELS; + }); + + it('converts a ready PR to draft and removes only managed labels', async () => { + const handler = freshRequire(); + const github = createGithubMock(); + const context = createContext(); + + await handler({ github, context, core: { info() {} } }); + + assert.deepEqual(github.graphqlCalls, [{ pullRequestId: 'PR_node_42' }]); + assert.deepEqual(github.removedLabels, [ + 'queue:committers', + 'status: ready-to-merge', + ]); + }); + + it('skips bot-authored PRs', async () => { + const handler = freshRequire(); + const github = createGithubMock(); + const context = createContext({ + pull_request: { + number: 43, + node_id: 'PR_node_43', + draft: false, + user: { login: 'github-actions[bot]', type: 'Bot' }, + labels: [{ name: 'queue:committers' }], + }, + }); + + await handler({ github, context, core: { info() {} } }); + + assert.equal(github.graphqlCalls.length, 0); + assert.equal(github.removedLabels.length, 0); + }); + + it('skips already-draft PRs', async () => { + const handler = freshRequire(); + const github = createGithubMock(); + const context = createContext({ + pull_request: { + number: 44, + node_id: 'PR_node_44', + draft: true, + user: { login: 'contributor', type: 'User' }, + labels: [{ name: 'queue:committers' }], + }, + }); + + await handler({ github, context, core: { info() {} } }); + + assert.equal(github.graphqlCalls.length, 0); + assert.equal(github.removedLabels.length, 0); + }); + + it('uses configurable managed labels', async () => { + process.env.REVISION_GUARD_MANAGED_LABELS = 'custom: one, custom: two'; + const handler = freshRequire(); + const github = createGithubMock(); + const context = createContext({ + pull_request: { + number: 45, + node_id: 'PR_node_45', + draft: false, + user: { login: 'contributor', type: 'User' }, + labels: [ + { name: 'custom: one' }, + { name: 'queue:committers' }, + { name: 'custom: two' }, + ], + }, + }); + + await handler({ github, context, core: { info() {} } }); + + assert.deepEqual(github.removedLabels, ['custom: one', 'custom: two']); + }); +}); diff --git a/.github/workflows/revision-guard.yml b/.github/workflows/revision-guard.yml new file mode 100644 index 000000000..d3a84e462 --- /dev/null +++ b/.github/workflows/revision-guard.yml @@ -0,0 +1,37 @@ +name: Revision Guard - Review Events + +on: + pull_request_review: + types: [submitted] + +permissions: + contents: read + +jobs: + guard: + if: github.event.review.state == 'changes_requested' + runs-on: hl-sdk-py-lin-md + permissions: + pull-requests: write + issues: write + contents: read + concurrency: + group: revision-guard-review-${{ github.event.pull_request.number }} + cancel-in-progress: false + steps: + - name: Harden Runner + uses: step-security/harden-runner@8d3c67de8e2fe68ef647c8db1e6a09f647780f40 # v2.19.0 + with: + egress-policy: audit + + - name: Checkout repository + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + sparse-checkout: .github/scripts + + - name: Handle changes requested + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + const handler = require('./.github/scripts/revision-guard/index.js'); + await handler({ github, context, core }); From ccb988f7cdb5eabf3da4031e5c374d32447e6cf0 Mon Sep 17 00:00:00 2001 From: Gourav NSS Date: Thu, 14 May 2026 17:09:37 +0530 Subject: [PATCH 2/4] fix: address review feedback on revision-guard draft conversion flow - fix: add defensive guard on context.payload and context.repo before dereferencing (CodeRabbit major) - fix: parseManagedLabels now merges custom labels with DEFAULT_MANAGED_LABELS instead of silently discarding defaults (darshit2308 blocking) - test: add graphqlCalls assertion in configurable-labels test to catch draft-conversion regressions (CodeRabbit trivial) - fix: checkout uses trusted base SHA instead of PR head/merge ref to prevent fork PRs from running untrusted code with write-scoped token (CodeRabbit critical) Signed-off-by: Gourav NSS --- .../revision-guard/helpers/constants.js | 7 ++++++- .github/scripts/revision-guard/index.js | 21 ++++++++++++++----- .github/scripts/revision-guard/index.test.js | 7 +++++-- .github/workflows/revision-guard.yml | 3 +++ 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/.github/scripts/revision-guard/helpers/constants.js b/.github/scripts/revision-guard/helpers/constants.js index 1255a99ff..3e63d4dd8 100644 --- a/.github/scripts/revision-guard/helpers/constants.js +++ b/.github/scripts/revision-guard/helpers/constants.js @@ -12,10 +12,15 @@ function parseManagedLabels(value) { return [...DEFAULT_MANAGED_LABELS]; } - return value + const customLabels = value .split(',') .map(label => label.trim()) .filter(Boolean); + + // Merge custom labels with defaults so that setting REVISION_GUARD_MANAGED_LABELS + // adds to the managed set rather than silently discarding the default labels. + const merged = new Set([...DEFAULT_MANAGED_LABELS, ...customLabels]); + return [...merged]; } module.exports = { diff --git a/.github/scripts/revision-guard/index.js b/.github/scripts/revision-guard/index.js index 0f328db31..d10ef7e85 100644 --- a/.github/scripts/revision-guard/index.js +++ b/.github/scripts/revision-guard/index.js @@ -7,10 +7,21 @@ const { } = require('./helpers'); module.exports = async function revisionGuard({ github, context, core }) { - const reviewState = context.payload.review?.state; - const pr = context.payload.pull_request; + const payload = context?.payload; + const repo = context?.repo; + const reviewState = payload?.review?.state; + const pr = payload?.pull_request; - if (!pr || reviewState !== 'changes_requested') { + if ( + !payload || + !repo?.owner || + !repo?.repo || + !pr || + typeof pr.number !== 'number' || + !pr.node_id || + reviewState !== 'changes_requested' + ) { + core?.info?.('Skipping revision guard due to missing or non-matching payload data.'); return; } @@ -34,8 +45,8 @@ module.exports = async function revisionGuard({ github, context, core }) { } await removeManagedLabels(github, { - owner: context.repo.owner, - repo: context.repo.repo, + owner: repo.owner, + repo: repo.repo, issueNumber: pr.number, labels: labelsToRemove, }); diff --git a/.github/scripts/revision-guard/index.test.js b/.github/scripts/revision-guard/index.test.js index bcddeb147..08e8d3dd6 100644 --- a/.github/scripts/revision-guard/index.test.js +++ b/.github/scripts/revision-guard/index.test.js @@ -116,7 +116,7 @@ describe('revision-guard index', () => { assert.equal(github.removedLabels.length, 0); }); - it('uses configurable managed labels', async () => { + it('uses configurable managed labels and still removes defaults', async () => { process.env.REVISION_GUARD_MANAGED_LABELS = 'custom: one, custom: two'; const handler = freshRequire(); const github = createGithubMock(); @@ -136,6 +136,9 @@ describe('revision-guard index', () => { await handler({ github, context, core: { info() {} } }); - assert.deepEqual(github.removedLabels, ['custom: one', 'custom: two']); + // Draft conversion must also fire for configurable-label scenarios. + assert.deepEqual(github.graphqlCalls, [{ pullRequestId: 'PR_node_45' }]); + // Custom labels AND the matching default (queue:committers) must both be removed. + assert.deepEqual(github.removedLabels, ['queue:committers', 'custom: one', 'custom: two']); }); }); diff --git a/.github/workflows/revision-guard.yml b/.github/workflows/revision-guard.yml index d3a84e462..0d6d255c0 100644 --- a/.github/workflows/revision-guard.yml +++ b/.github/workflows/revision-guard.yml @@ -27,6 +27,9 @@ jobs: - name: Checkout repository uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: + # Use trusted base commit, not PR head/merge ref, to prevent fork PRs + # from executing untrusted changes with a write-scoped token. + ref: ${{ github.event.pull_request.base.sha }} sparse-checkout: .github/scripts - name: Handle changes requested From c3537b09bf9a192f72048ef2d645d889d3f3136a Mon Sep 17 00:00:00 2001 From: Gourav NSS Date: Thu, 14 May 2026 17:19:08 +0530 Subject: [PATCH 3/4] fix: wrap state-mutating ops in try/catch with contextual error logging - convertToDraft re-throws on failure so the workflow fails loudly - removeManagedLabels does not re-throw since draft conversion is the primary goal; a label-cleanup failure is logged via core.error but does not fail the run Signed-off-by: Gourav NSS --- .github/scripts/revision-guard/index.js | 35 +++++++++++++++++-------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/.github/scripts/revision-guard/index.js b/.github/scripts/revision-guard/index.js index d10ef7e85..94c1ad6e8 100644 --- a/.github/scripts/revision-guard/index.js +++ b/.github/scripts/revision-guard/index.js @@ -35,8 +35,13 @@ module.exports = async function revisionGuard({ github, context, core }) { return; } - await convertToDraft(github, pr.node_id); - core?.info?.(`Converted PR #${pr.number} to draft.`); + try { + await convertToDraft(github, pr.node_id); + core?.info?.(`Converted PR #${pr.number} to draft.`); + } catch (error) { + core?.error?.(`Failed to convert PR #${pr.number} to draft: ${error.message}`); + throw error; + } const labelsToRemove = getPresentManagedLabels(pr.labels); if (labelsToRemove.length === 0) { @@ -44,13 +49,21 @@ module.exports = async function revisionGuard({ github, context, core }) { return; } - await removeManagedLabels(github, { - owner: repo.owner, - repo: repo.repo, - issueNumber: pr.number, - labels: labelsToRemove, - }); - core?.info?.( - `Removed managed labels from PR #${pr.number}: ${labelsToRemove.join(', ')}.` - ); + try { + await removeManagedLabels(github, { + owner: repo.owner, + repo: repo.repo, + issueNumber: pr.number, + labels: labelsToRemove, + }); + core?.info?.( + `Removed managed labels from PR #${pr.number}: ${labelsToRemove.join(', ')}.` + ); + } catch (error) { + core?.error?.( + `Failed to remove labels from PR #${pr.number}: ${error.message}. ` + + `Labels to remove: ${labelsToRemove.join(', ')}.` + ); + // Don't re-throw; draft conversion succeeded and is the primary goal + } }; From 533d4f34522fabbcf945193dc9b282c52c52017d Mon Sep 17 00:00:00 2001 From: Gourav N S S Date: Thu, 14 May 2026 20:06:32 +0530 Subject: [PATCH 4/4] Update .github/workflows/revision-guard.yml Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Gourav N S S --- .github/workflows/revision-guard.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/revision-guard.yml b/.github/workflows/revision-guard.yml index 0d6d255c0..54966c024 100644 --- a/.github/workflows/revision-guard.yml +++ b/.github/workflows/revision-guard.yml @@ -33,6 +33,8 @@ jobs: sparse-checkout: .github/scripts - name: Handle changes requested + env: + REVISION_GUARD_MANAGED_LABELS: ${{ vars.REVISION_GUARD_MANAGED_LABELS }} uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: script: |