From 807125dd82f3933ecbcdafcc316318eb44514824 Mon Sep 17 00:00:00 2001 From: Gourav NSS Date: Sun, 26 Apr 2026 08:54:54 +0530 Subject: [PATCH 01/11] simplify CodeRabbit workflow Signed-off-by: Gourav NSS --- .github/scripts/coderabbit_plan_trigger.js | 12 ++---------- .github/workflows/bot-coderabbit-plan-trigger.yml | 14 +------------- 2 files changed, 3 insertions(+), 23 deletions(-) diff --git a/.github/scripts/coderabbit_plan_trigger.js b/.github/scripts/coderabbit_plan_trigger.js index bffb0d439..eaca959d0 100644 --- a/.github/scripts/coderabbit_plan_trigger.js +++ b/.github/scripts/coderabbit_plan_trigger.js @@ -2,14 +2,9 @@ const CODERABBIT_MARKER = ''; -async function triggerCodeRabbitPlan(github, owner, repo, issue, marker = CODERABBIT_MARKER, isDryRun = false) { +async function triggerCodeRabbitPlan(github, owner, repo, issue, marker = CODERABBIT_MARKER) { const comment = `${marker} @coderabbitai plan`; - if (isDryRun) { - console.log(`[DRY RUN] Would trigger CodeRabbit plan for issue #${issue.number}`); - return true; - } - try { await github.rest.issues.createComment({ owner, @@ -105,11 +100,8 @@ async function main({ github, context }) { return console.log(`CodeRabbit plan already triggered for #${issue.number}`); } - // Check for dry run (default to true if not specified, for safety) - const isDryRun = (process.env.DRY_RUN || 'true').toLowerCase() === 'true'; - // Post CodeRabbit plan trigger - await triggerCodeRabbitPlan(github, owner, repo, issue, CODERABBIT_MARKER, isDryRun); + await triggerCodeRabbitPlan(github, owner, repo, issue, CODERABBIT_MARKER); logSummary(owner, repo, issue); } catch (err) { diff --git a/.github/workflows/bot-coderabbit-plan-trigger.yml b/.github/workflows/bot-coderabbit-plan-trigger.yml index 66ecc0a40..a2dcf22bb 100644 --- a/.github/workflows/bot-coderabbit-plan-trigger.yml +++ b/.github/workflows/bot-coderabbit-plan-trigger.yml @@ -5,16 +5,6 @@ name: CodeRabbit Plan Trigger on: issues: types: [labeled] - workflow_dispatch: - inputs: - dry_run: - description: "Run without posting comments" - required: false - default: "true" - type: choice - options: - - "true" - - "false" permissions: issues: write @@ -28,8 +18,7 @@ jobs: cancel-in-progress: false # Only run when the newly added label is beginner, intermediate, or advanced (case-insensitive) if: > - github.event_name == 'workflow_dispatch' || - (github.event.action == 'labeled' && + github.event.action == 'labeled' && github.event.issue.state == 'open' && contains(fromJson('["beginner","intermediate","advanced","Beginner","Intermediate","Advanced"]'), github.event.label.name)) @@ -45,7 +34,6 @@ jobs: - name: Trigger CodeRabbit Plan env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - DRY_RUN: ${{ github.event_name == 'workflow_dispatch' && inputs.dry_run || 'false' }} uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 #v9.0.0 with: script: | From 1191294ef0c02725945be2841dc117d0800c11d4 Mon Sep 17 00:00:00 2001 From: Gourav NSS Date: Sun, 26 Apr 2026 09:22:48 +0530 Subject: [PATCH 02/11] fix malformed CodeRabbit workflow condition Signed-off-by: Gourav NSS --- .github/workflows/bot-coderabbit-plan-trigger.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/bot-coderabbit-plan-trigger.yml b/.github/workflows/bot-coderabbit-plan-trigger.yml index a2dcf22bb..4a147b1e6 100644 --- a/.github/workflows/bot-coderabbit-plan-trigger.yml +++ b/.github/workflows/bot-coderabbit-plan-trigger.yml @@ -20,7 +20,7 @@ jobs: if: > github.event.action == 'labeled' && github.event.issue.state == 'open' && - contains(fromJson('["beginner","intermediate","advanced","Beginner","Intermediate","Advanced"]'), github.event.label.name)) + contains(fromJson('["beginner","intermediate","advanced","Beginner","Intermediate","Advanced"]'), github.event.label.name) steps: - name: Harden the runner From c537a55c1d54074f996b1997b4cf187ba37eda54 Mon Sep 17 00:00:00 2001 From: Gourav NSS Date: Thu, 14 May 2026 00:49:20 +0530 Subject: [PATCH 03/11] 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 04/11] 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 05/11] 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 06/11] 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: | From b042778364cbdd437e9e93bb69a75fafd7e2fc9b Mon Sep 17 00:00:00 2001 From: Gourav NSS Date: Mon, 25 May 2026 23:27:51 +0530 Subject: [PATCH 07/11] fix: use REST API for draft conversion to avoid contents:write permission The GraphQL convertPullRequestToDraft mutation requires contents: write, which raises security concerns for workflows triggered by fork PR reviews. Switch to the REST API pulls.update endpoint with draft: true, which works with the existing pull-requests: write permission. - Replace graphql mutation with github.rest.pulls.update - Remove node_id validation since we no longer need GraphQL node IDs - Update mocks and assertions in tests accordingly Addresses review feedback from exploreriii and darshit2308. Signed-off-by: Gourav NSS --- .../scripts/revision-guard/helpers/draft.js | 20 ++++++---------- .github/scripts/revision-guard/index.js | 7 ++++-- .github/scripts/revision-guard/index.test.js | 23 ++++++++++++------- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/.github/scripts/revision-guard/helpers/draft.js b/.github/scripts/revision-guard/helpers/draft.js index 23c620383..09267135c 100644 --- a/.github/scripts/revision-guard/helpers/draft.js +++ b/.github/scripts/revision-guard/helpers/draft.js @@ -7,19 +7,13 @@ 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 } - ); +async function convertToDraft(github, { owner, repo, pullNumber }) { + await github.rest.pulls.update({ + owner, + repo, + pull_number: pullNumber, + draft: true, + }); } module.exports = { diff --git a/.github/scripts/revision-guard/index.js b/.github/scripts/revision-guard/index.js index 94c1ad6e8..893ab015f 100644 --- a/.github/scripts/revision-guard/index.js +++ b/.github/scripts/revision-guard/index.js @@ -18,7 +18,6 @@ module.exports = async function revisionGuard({ github, context, core }) { !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.'); @@ -36,7 +35,11 @@ module.exports = async function revisionGuard({ github, context, core }) { } try { - await convertToDraft(github, pr.node_id); + await convertToDraft(github, { + owner: repo.owner, + repo: repo.repo, + pullNumber: pr.number, + }); core?.info?.(`Converted PR #${pr.number} to draft.`); } catch (error) { core?.error?.(`Failed to convert PR #${pr.number} to draft: ${error.message}`); diff --git a/.github/scripts/revision-guard/index.test.js b/.github/scripts/revision-guard/index.test.js index 08e8d3dd6..0022dd772 100644 --- a/.github/scripts/revision-guard/index.test.js +++ b/.github/scripts/revision-guard/index.test.js @@ -17,19 +17,22 @@ function freshRequire() { function createGithubMock() { const removedLabels = []; + const pullUpdates = []; return { removedLabels, - graphqlCalls: [], - graphql: async function (_query, variables) { - this.graphqlCalls.push(variables); - }, + pullUpdates, rest: { issues: { removeLabel: async ({ name }) => { removedLabels.push(name); }, }, + pulls: { + update: async (params) => { + pullUpdates.push(params); + }, + }, }, }; } @@ -71,7 +74,9 @@ describe('revision-guard index', () => { await handler({ github, context, core: { info() {} } }); - assert.deepEqual(github.graphqlCalls, [{ pullRequestId: 'PR_node_42' }]); + assert.deepEqual(github.pullUpdates, [ + { owner: 'hiero-ledger', repo: 'hiero-sdk-python', pull_number: 42, draft: true }, + ]); assert.deepEqual(github.removedLabels, [ 'queue:committers', 'status: ready-to-merge', @@ -93,7 +98,7 @@ describe('revision-guard index', () => { await handler({ github, context, core: { info() {} } }); - assert.equal(github.graphqlCalls.length, 0); + assert.equal(github.pullUpdates.length, 0); assert.equal(github.removedLabels.length, 0); }); @@ -112,7 +117,7 @@ describe('revision-guard index', () => { await handler({ github, context, core: { info() {} } }); - assert.equal(github.graphqlCalls.length, 0); + assert.equal(github.pullUpdates.length, 0); assert.equal(github.removedLabels.length, 0); }); @@ -137,7 +142,9 @@ describe('revision-guard index', () => { await handler({ github, context, core: { info() {} } }); // Draft conversion must also fire for configurable-label scenarios. - assert.deepEqual(github.graphqlCalls, [{ pullRequestId: 'PR_node_45' }]); + assert.deepEqual(github.pullUpdates, [ + { owner: 'hiero-ledger', repo: 'hiero-sdk-python', pull_number: 45, draft: true }, + ]); // Custom labels AND the matching default (queue:committers) must both be removed. assert.deepEqual(github.removedLabels, ['queue:committers', 'custom: one', 'custom: two']); }); From 97481d7dbeb03fae09c3d09512f105583d95df81 Mon Sep 17 00:00:00 2001 From: Gourav NSS Date: Sun, 7 Jun 2026 19:48:18 +0530 Subject: [PATCH 08/11] fix: convert revision guard PRs to draft via GraphQL Signed-off-by: Gourav NSS --- .../scripts/revision-guard/helpers/draft.js | 21 +++++++---- .github/scripts/revision-guard/index.js | 5 +-- .github/scripts/revision-guard/index.test.js | 37 +++++++++++-------- 3 files changed, 38 insertions(+), 25 deletions(-) diff --git a/.github/scripts/revision-guard/helpers/draft.js b/.github/scripts/revision-guard/helpers/draft.js index 09267135c..016ba2860 100644 --- a/.github/scripts/revision-guard/helpers/draft.js +++ b/.github/scripts/revision-guard/helpers/draft.js @@ -7,13 +7,20 @@ function isDraft(pr) { return pr?.draft === true; } -async function convertToDraft(github, { owner, repo, pullNumber }) { - await github.rest.pulls.update({ - owner, - repo, - pull_number: pullNumber, - draft: true, - }); +async function convertToDraft(github, { pullRequestId }) { + await github.graphql( + ` + mutation ConvertPullRequestToDraft($pullRequestId: ID!) { + convertPullRequestToDraft(input: { pullRequestId: $pullRequestId }) { + pullRequest { + id + isDraft + } + } + } + `, + { pullRequestId } + ); } module.exports = { diff --git a/.github/scripts/revision-guard/index.js b/.github/scripts/revision-guard/index.js index 893ab015f..90dd69ef7 100644 --- a/.github/scripts/revision-guard/index.js +++ b/.github/scripts/revision-guard/index.js @@ -18,6 +18,7 @@ module.exports = async function revisionGuard({ github, context, core }) { !repo?.repo || !pr || typeof pr.number !== 'number' || + typeof pr.node_id !== 'string' || reviewState !== 'changes_requested' ) { core?.info?.('Skipping revision guard due to missing or non-matching payload data.'); @@ -36,9 +37,7 @@ module.exports = async function revisionGuard({ github, context, core }) { try { await convertToDraft(github, { - owner: repo.owner, - repo: repo.repo, - pullNumber: pr.number, + pullRequestId: pr.node_id, }); core?.info?.(`Converted PR #${pr.number} to draft.`); } catch (error) { diff --git a/.github/scripts/revision-guard/index.test.js b/.github/scripts/revision-guard/index.test.js index 0022dd772..c99117fda 100644 --- a/.github/scripts/revision-guard/index.test.js +++ b/.github/scripts/revision-guard/index.test.js @@ -6,33 +6,41 @@ function freshRequire() { const helpersPath = require.resolve('./helpers/index.js'); const labelsPath = require.resolve('./helpers/labels.js'); const constantsPath = require.resolve('./helpers/constants.js'); + const draftPath = require.resolve('./helpers/draft.js'); delete require.cache[indexPath]; delete require.cache[helpersPath]; delete require.cache[labelsPath]; delete require.cache[constantsPath]; + delete require.cache[draftPath]; return require('./index.js'); } function createGithubMock() { const removedLabels = []; - const pullUpdates = []; + const graphqlCalls = []; return { removedLabels, - pullUpdates, + graphqlCalls, + graphql: async (query, variables) => { + graphqlCalls.push({ query, variables }); + return { + convertPullRequestToDraft: { + pullRequest: { + id: variables.pullRequestId, + isDraft: true, + }, + }, + }; + }, rest: { issues: { removeLabel: async ({ name }) => { removedLabels.push(name); }, }, - pulls: { - update: async (params) => { - pullUpdates.push(params); - }, - }, }, }; } @@ -74,9 +82,9 @@ describe('revision-guard index', () => { await handler({ github, context, core: { info() {} } }); - assert.deepEqual(github.pullUpdates, [ - { owner: 'hiero-ledger', repo: 'hiero-sdk-python', pull_number: 42, draft: true }, - ]); + assert.equal(github.graphqlCalls.length, 1); + assert.match(github.graphqlCalls[0].query, /convertPullRequestToDraft/); + assert.deepEqual(github.graphqlCalls[0].variables, { pullRequestId: 'PR_node_42' }); assert.deepEqual(github.removedLabels, [ 'queue:committers', 'status: ready-to-merge', @@ -98,7 +106,7 @@ describe('revision-guard index', () => { await handler({ github, context, core: { info() {} } }); - assert.equal(github.pullUpdates.length, 0); + assert.equal(github.graphqlCalls.length, 0); assert.equal(github.removedLabels.length, 0); }); @@ -117,7 +125,7 @@ describe('revision-guard index', () => { await handler({ github, context, core: { info() {} } }); - assert.equal(github.pullUpdates.length, 0); + assert.equal(github.graphqlCalls.length, 0); assert.equal(github.removedLabels.length, 0); }); @@ -142,9 +150,8 @@ describe('revision-guard index', () => { await handler({ github, context, core: { info() {} } }); // Draft conversion must also fire for configurable-label scenarios. - assert.deepEqual(github.pullUpdates, [ - { owner: 'hiero-ledger', repo: 'hiero-sdk-python', pull_number: 45, draft: true }, - ]); + assert.equal(github.graphqlCalls.length, 1); + assert.deepEqual(github.graphqlCalls[0].variables, { 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']); }); From 9158691baacd032e35f18d2c44c05ad8d6022f8f Mon Sep 17 00:00:00 2001 From: Gourav NSS Date: Sun, 7 Jun 2026 19:54:22 +0530 Subject: [PATCH 09/11] fix: support review bot token for draft conversion Signed-off-by: Gourav NSS --- .github/scripts/revision-guard/index.js | 5 +++++ .github/workflows/revision-guard.yml | 2 ++ 2 files changed, 7 insertions(+) diff --git a/.github/scripts/revision-guard/index.js b/.github/scripts/revision-guard/index.js index 90dd69ef7..e34b14f3e 100644 --- a/.github/scripts/revision-guard/index.js +++ b/.github/scripts/revision-guard/index.js @@ -42,6 +42,11 @@ module.exports = async function revisionGuard({ github, context, core }) { core?.info?.(`Converted PR #${pr.number} to draft.`); } catch (error) { core?.error?.(`Failed to convert PR #${pr.number} to draft: ${error.message}`); + if (!process.env.REVIEWBOT_TOKEN) { + core?.info?.( + 'Hint: configure REVIEWBOT_TOKEN with permission to convert pull requests to draft.' + ); + } throw error; } diff --git a/.github/workflows/revision-guard.yml b/.github/workflows/revision-guard.yml index 54966c024..8ae3a9e77 100644 --- a/.github/workflows/revision-guard.yml +++ b/.github/workflows/revision-guard.yml @@ -35,8 +35,10 @@ jobs: - name: Handle changes requested env: REVISION_GUARD_MANAGED_LABELS: ${{ vars.REVISION_GUARD_MANAGED_LABELS }} + REVIEWBOT_TOKEN: ${{ secrets.REVIEWBOT_TOKEN || '' }} uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: + github-token: ${{ secrets.REVIEWBOT_TOKEN || secrets.GITHUB_TOKEN }} script: | const handler = require('./.github/scripts/revision-guard/index.js'); await handler({ github, context, core }); From 393f8e24d5ad38df0f3f390d6224ae366b5f1bb0 Mon Sep 17 00:00:00 2001 From: Gourav NSS Date: Tue, 23 Jun 2026 22:04:20 +0530 Subject: [PATCH 10/11] fix: make revision guard draft conversion optional Signed-off-by: Gourav NSS --- .github/scripts/revision-guard/index.js | 36 +++++++------ .github/scripts/revision-guard/index.test.js | 56 ++++++++++++++++---- .github/scripts/revision-guard/package.json | 10 ++++ .github/workflows/test-revision-guard.yml | 38 +++++++++++++ 4 files changed, 114 insertions(+), 26 deletions(-) create mode 100644 .github/scripts/revision-guard/package.json create mode 100644 .github/workflows/test-revision-guard.yml diff --git a/.github/scripts/revision-guard/index.js b/.github/scripts/revision-guard/index.js index e34b14f3e..2be4b3052 100644 --- a/.github/scripts/revision-guard/index.js +++ b/.github/scripts/revision-guard/index.js @@ -18,7 +18,6 @@ module.exports = async function revisionGuard({ github, context, core }) { !repo?.repo || !pr || typeof pr.number !== 'number' || - typeof pr.node_id !== 'string' || reviewState !== 'changes_requested' ) { core?.info?.('Skipping revision guard due to missing or non-matching payload data.'); @@ -30,27 +29,30 @@ module.exports = async function revisionGuard({ github, context, core }) { return; } - if (isDraft(pr)) { - core?.info?.(`Skipping PR #${pr.number} because it is already a draft.`); - return; - } + const labelsToRemove = getPresentManagedLabels(pr.labels); - try { - await convertToDraft(github, { - pullRequestId: 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}`); - if (!process.env.REVIEWBOT_TOKEN) { + if (!isDraft(pr)) { + if (process.env.REVIEWBOT_TOKEN && typeof pr.node_id === 'string') { + try { + await convertToDraft(github, { + pullRequestId: 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}. ` + + 'Continuing with managed label cleanup.' + ); + } + } else { core?.info?.( - 'Hint: configure REVIEWBOT_TOKEN with permission to convert pull requests to draft.' + `Skipping draft conversion for PR #${pr.number}; REVIEWBOT_TOKEN is not configured.` ); } - throw error; + } else { + core?.info?.(`PR #${pr.number} is already a draft.`); } - const labelsToRemove = getPresentManagedLabels(pr.labels); if (labelsToRemove.length === 0) { core?.info?.(`No managed labels to remove for PR #${pr.number}.`); return; @@ -71,6 +73,6 @@ module.exports = async function revisionGuard({ github, context, core }) { `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 + throw error; } }; diff --git a/.github/scripts/revision-guard/index.test.js b/.github/scripts/revision-guard/index.test.js index c99117fda..75e33bf92 100644 --- a/.github/scripts/revision-guard/index.test.js +++ b/.github/scripts/revision-guard/index.test.js @@ -20,12 +20,19 @@ function freshRequire() { function createGithubMock() { const removedLabels = []; const graphqlCalls = []; + let graphqlError; return { removedLabels, graphqlCalls, + failGraphql(error) { + graphqlError = error; + }, graphql: async (query, variables) => { graphqlCalls.push({ query, variables }); + if (graphqlError) { + throw graphqlError; + } return { convertPullRequestToDraft: { pullRequest: { @@ -69,22 +76,22 @@ function createContext(overrides = {}) { describe('revision-guard index', () => { beforeEach(() => { delete process.env.REVISION_GUARD_MANAGED_LABELS; + delete process.env.REVIEWBOT_TOKEN; }); afterEach(() => { delete process.env.REVISION_GUARD_MANAGED_LABELS; + delete process.env.REVIEWBOT_TOKEN; }); - it('converts a ready PR to draft and removes only managed labels', async () => { + it('removes only managed labels without draft conversion when no review bot token is configured', async () => { const handler = freshRequire(); const github = createGithubMock(); const context = createContext(); await handler({ github, context, core: { info() {} } }); - assert.equal(github.graphqlCalls.length, 1); - assert.match(github.graphqlCalls[0].query, /convertPullRequestToDraft/); - assert.deepEqual(github.graphqlCalls[0].variables, { pullRequestId: 'PR_node_42' }); + assert.equal(github.graphqlCalls.length, 0); assert.deepEqual(github.removedLabels, [ 'queue:committers', 'status: ready-to-merge', @@ -110,7 +117,7 @@ describe('revision-guard index', () => { assert.equal(github.removedLabels.length, 0); }); - it('skips already-draft PRs', async () => { + it('removes managed labels from already-draft PRs without converting again', async () => { const handler = freshRequire(); const github = createGithubMock(); const context = createContext({ @@ -126,7 +133,40 @@ describe('revision-guard index', () => { await handler({ github, context, core: { info() {} } }); assert.equal(github.graphqlCalls.length, 0); - assert.equal(github.removedLabels.length, 0); + assert.deepEqual(github.removedLabels, ['queue:committers']); + }); + + it('converts to draft when a review bot token is configured', async () => { + process.env.REVIEWBOT_TOKEN = 'token'; + const handler = freshRequire(); + const github = createGithubMock(); + const context = createContext(); + + await handler({ github, context, core: { info() {} } }); + + assert.equal(github.graphqlCalls.length, 1); + assert.match(github.graphqlCalls[0].query, /convertPullRequestToDraft/); + assert.deepEqual(github.graphqlCalls[0].variables, { pullRequestId: 'PR_node_42' }); + assert.deepEqual(github.removedLabels, [ + 'queue:committers', + 'status: ready-to-merge', + ]); + }); + + it('still removes managed labels when draft conversion fails', async () => { + process.env.REVIEWBOT_TOKEN = 'token'; + const handler = freshRequire(); + const github = createGithubMock(); + github.failGraphql(new Error('Resource not accessible by integration')); + const context = createContext(); + + await handler({ github, context, core: { error() {}, info() {} } }); + + assert.equal(github.graphqlCalls.length, 1); + assert.deepEqual(github.removedLabels, [ + 'queue:committers', + 'status: ready-to-merge', + ]); }); it('uses configurable managed labels and still removes defaults', async () => { @@ -149,9 +189,7 @@ describe('revision-guard index', () => { await handler({ github, context, core: { info() {} } }); - // Draft conversion must also fire for configurable-label scenarios. - assert.equal(github.graphqlCalls.length, 1); - assert.deepEqual(github.graphqlCalls[0].variables, { pullRequestId: 'PR_node_45' }); + assert.equal(github.graphqlCalls.length, 0); // 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/scripts/revision-guard/package.json b/.github/scripts/revision-guard/package.json new file mode 100644 index 000000000..9328210f0 --- /dev/null +++ b/.github/scripts/revision-guard/package.json @@ -0,0 +1,10 @@ +{ + "name": "revision-guard", + "version": "1.0.0", + "description": "Revision guard automation scripts and tests", + "private": true, + "main": "index.js", + "scripts": { + "test": "node --test index.test.js" + } +} diff --git a/.github/workflows/test-revision-guard.yml b/.github/workflows/test-revision-guard.yml new file mode 100644 index 000000000..246ce835f --- /dev/null +++ b/.github/workflows/test-revision-guard.yml @@ -0,0 +1,38 @@ +name: Test Revision Guard + +on: + push: + paths: + - '.github/scripts/revision-guard/**' + - '.github/workflows/test-revision-guard.yml' + pull_request: + paths: + - '.github/scripts/revision-guard/**' + - '.github/workflows/test-revision-guard.yml' + +permissions: + contents: read + +jobs: + test: + if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork == false + runs-on: hl-sdk-py-lin-md + permissions: + contents: read + steps: + - name: Harden the runner + uses: step-security/harden-runner@9af89fc71515a100421586dfdb3dc9c984fbf411 # v2.19.4 + with: + egress-policy: audit + + - name: Checkout repository + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + + - name: Setup Node.js + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: '20' + + - name: Run Tests + working-directory: .github/scripts/revision-guard + run: npm test From daf0df3565637729acf3e6a2034966421139722e Mon Sep 17 00:00:00 2001 From: Gourav NSS Date: Tue, 23 Jun 2026 22:05:25 +0530 Subject: [PATCH 11/11] chore: align revision guard test workflow Signed-off-by: Gourav NSS --- .github/workflows/test-revision-guard.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-revision-guard.yml b/.github/workflows/test-revision-guard.yml index 246ce835f..52ce3690a 100644 --- a/.github/workflows/test-revision-guard.yml +++ b/.github/workflows/test-revision-guard.yml @@ -26,7 +26,7 @@ jobs: egress-policy: audit - name: Checkout repository - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Setup Node.js uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0