From 3e82b91f00f810960c64e0e0857dc195afa482c5 Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Mon, 13 Jul 2026 15:25:51 +0800 Subject: [PATCH 1/6] ci: guard post-merge approval labels Signed-off-by: Yanchao Lu --- .../test_post_merge_approval_workflow.js | 205 ++++++++++++++++++ .github/workflows/bot-command.yml | 8 +- .github/workflows/post-merge-approval.yml | 103 +++++++++ docs/source/developer-guide/ci-overview.md | 21 +- 4 files changed, 329 insertions(+), 8 deletions(-) create mode 100644 .github/scripts/test_post_merge_approval_workflow.js create mode 100644 .github/workflows/post-merge-approval.yml diff --git a/.github/scripts/test_post_merge_approval_workflow.js b/.github/scripts/test_post_merge_approval_workflow.js new file mode 100644 index 000000000000..3901b3ff75ea --- /dev/null +++ b/.github/scripts/test_post_merge_approval_workflow.js @@ -0,0 +1,205 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +const fs = require("fs"); +const path = require("path"); + +const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor; +const WORKFLOW = path.join( + __dirname, + "..", + "workflows", + "post-merge-approval.yml", +); + +function assert(condition, message) { + if (!condition) throw new Error(message); +} + +function extractGithubScripts(source) { + const lines = source.split(/\r?\n/); + const scripts = []; + + for (let index = 0; index < lines.length; index += 1) { + const marker = lines[index].match(/^(\s*)script:\s*\|\s*$/); + if (!marker) continue; + + const markerIndent = marker[1].length; + const contentIndent = markerIndent + 2; + const body = []; + for (index += 1; index < lines.length; index += 1) { + const line = lines[index]; + if (line.trim() === "") { + body.push(""); + } else if (line.match(/^ */)[0].length <= markerIndent) { + index -= 1; + break; + } else { + body.push(line.slice(contentIndent)); + } + } + scripts.push(body.join("\n")); + } + return scripts; +} + +async function main() { + const workflowSource = fs.readFileSync(WORKFLOW, "utf8"); + for (const expected of [ + "pull_request_target:", + "types: [labeled]", + "contents: read", + "pull-requests: write", + "github.repository == 'NVIDIA/TensorRT-LLM'", + "github.event.label.name == 'ci: post-merge approved'", + "secrets.TENSORRT_CICD_TRTLLM_CI_TOKEN", + "secrets.GITHUB_TOKEN", + ]) { + assert(workflowSource.includes(expected), "missing workflow contract: " + expected); + } + for (const forbidden of [ + "actions/checkout", + "issues: write", + "pull-requests: read", + "synchronize", + "AUTO_LABEL_COMMUNITY_TOKEN", + ]) { + assert(!workflowSource.includes(forbidden), "unsafe workflow contract: " + forbidden); + } + + const scripts = extractGithubScripts(workflowSource); + assert(scripts.length === 2, "expected two github-script blocks"); + const validate = new AsyncFunction("github", "context", "core", scripts[0]); + const cleanup = new AsyncFunction("github", "context", "core", scripts[1]); + + async function runValidation(outcome, actor = "candidate", sender = true) { + let request; + let warning = ""; + const payload = sender ? { sender: { login: actor } } : {}; + const result = await validate( + { + request: async (route, args) => { + request = { route, args }; + if (outcome instanceof Error) throw outcome; + return { data: { state: outcome } }; + }, + }, + { actor, payload }, + { + warning: (message) => { + warning = message; + }, + }, + ); + return { result, request, warning }; + } + + const active = await runValidation("active", "approved-user"); + assert(active.result === "true", "active member must be authorized"); + assert( + active.request.route === + "GET /orgs/{org}/teams/{team_slug}/memberships/{username}", + "unexpected membership route", + ); + assert(active.request.args.org === "NVIDIA", "unexpected organization"); + assert( + active.request.args.team_slug === "trt-llm-ci-approvers", + "unexpected Team slug", + ); + assert( + active.request.args.username === "approved-user", + "label event sender must be checked", + ); + assert( + (await runValidation("active", "fallback-actor", false)).request.args + .username === "fallback-actor", + "context actor must be used when the event sender is absent", + ); + assert( + (await runValidation("pending")).result === "false", + "pending member must be rejected", + ); + + const notFound = new Error("not found"); + notFound.status = 404; + assert( + (await runValidation(notFound, "non-member")).result === "false", + "non-member must be rejected", + ); + + const apiError = new Error("API unavailable"); + apiError.status = 500; + const apiFailure = await runValidation(apiError); + assert(apiFailure.result === "false", "API failure must fail closed"); + assert(apiFailure.warning.includes("API unavailable"), "failure must be logged"); + + async function runCleanup(removeError = null) { + const calls = { remove: [], comment: [] }; + const github = { + rest: { + issues: { + removeLabel: async (args) => { + calls.remove.push(args); + if (removeError) throw removeError; + }, + createComment: async (args) => { + calls.comment.push(args); + }, + }, + }, + }; + await cleanup( + github, + { + actor: "fallback", + repo: { owner: "NVIDIA", repo: "TensorRT-LLM" }, + payload: { + action: "labeled", + sender: { login: "label-actor" }, + pull_request: { number: 123 }, + }, + }, + {}, + ); + return calls; + } + + const unauthorized = await runCleanup(); + assert(unauthorized.remove.length === 1, "label must be removed"); + assert(unauthorized.comment.length === 1, "denial must be explained"); + assert( + unauthorized.comment[0].body.includes("label-actor"), + "comment must identify the label actor", + ); + + const concurrentRemoval = new Error("not found"); + concurrentRemoval.status = 404; + const concurrent = await runCleanup(concurrentRemoval); + assert( + concurrent.comment.length === 1, + "concurrent removal must not suppress the denial comment", + ); + + console.log( + "Post-merge approval workflow passed wiring, permissions, active, pending, " + + "non-member, API-failure, unauthorized-cleanup, commit-persistence, and " + + "concurrent-removal tests.", + ); +} + +main().catch((error) => { + console.error(error); + process.exit(1); +}); diff --git a/.github/workflows/bot-command.yml b/.github/workflows/bot-command.yml index 0a112dbf5435..56018b3559a1 100644 --- a/.github/workflows/bot-command.yml +++ b/.github/workflows/bot-command.yml @@ -1,4 +1,4 @@ -# SPDX-FileCopyrightText: Copyright (c) 2024 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-FileCopyrightText: Copyright (c) 2024-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -52,14 +52,14 @@ jobs: "`--disable-reuse-test ` *(OPTIONAL)* : Explicitly prevent the pipeline from reusing build artifacts and skipping successful test stages from a previous pipeline. Ensure that all builds and tests are run regardless of previous successes.\n\n" + "`--disable-fail-fast ` *(OPTIONAL)* : Disable fail fast on build/tests/infra failures.\n\n" + "`--skip-test ` *(OPTIONAL)* : Skip all test stages, but still run build stages, package stages and sanity check stages. Note: Does **NOT** update GitHub check status.\n\n" + - "`--stage-list \"A10-PyTorch-1, xxx\"` *(OPTIONAL)* : Only run the specified test stages. Supports wildcard `*` for pattern matching (e.g., `\"*PerfSanity*\"` matches all stages containing PerfSanity). Examples: \"A10-PyTorch-1, xxx\", \"*PerfSanity*\". Note: Does **NOT** update GitHub check status.\n\n" + + "`--stage-list \"A10-PyTorch-1, xxx\"` *(OPTIONAL)* : Only run the specified test stages. Supports wildcard `*` for pattern matching (e.g., `\"*PerfSanity*\"` matches all stages containing PerfSanity). Examples: \"A10-PyTorch-1, xxx\", \"*PerfSanity*\". The broad patterns `\"*\"` and `\"*Post-Merge*\"`, including equivalent escaped or repeated-star forms and their use in comma-separated lists, require the `ci: post-merge approved` PR label. Note: Does **NOT** update GitHub check status.\n\n" + "`--gpu-type \"A30, H100_PCIe\"` *(OPTIONAL)* : Only run the test stages on the specified GPU types. Examples: \"A30, H100_PCIe\". Note: Does **NOT** update GitHub check status.\n\n" + "`--test-backend \"pytorch, cpp\"` *(OPTIONAL)* : Skip test stages which don't match the specified backends. Only support [pytorch, cpp, tensorrt, triton]. Examples: \"pytorch, cpp\" (does not run test stages with tensorrt or triton backend). Note: Does **NOT** update GitHub pipeline status.\n\n" + "`--only-multi-gpu-test ` *(OPTIONAL)* : Only run the multi-GPU tests. Note: Does **NOT** update GitHub check status.\n\n" + "`--disable-multi-gpu-test ` *(OPTIONAL)* : Disable the multi-GPU tests. Note: Does **NOT** update GitHub check status.\n\n" + "`--add-multi-gpu-test ` *(OPTIONAL)* : Force run the multi-GPU tests in addition to running L0 pre-merge pipeline.\n\n" + - "`--post-merge ` *(OPTIONAL)* : Run the L0 post-merge pipeline instead of the ordinary L0 pre-merge pipeline.\n\n" + - "`--extra-stage \"H100_PCIe-TensorRT-Post-Merge-1, xxx\"` *(OPTIONAL)* : Run the ordinary L0 pre-merge pipeline and specified test stages. Supports wildcard `*` for pattern matching. Examples: --extra-stage \"H100_PCIe-TensorRT-Post-Merge-1, xxx\", --extra-stage \"*Post-Merge*\".\n\n" + + "`--post-merge ` *(OPTIONAL)* : Run the L0 post-merge pipeline instead of the ordinary L0 pre-merge pipeline. Requires the `ci: post-merge approved` PR label applied by an active member of `NVIDIA/trt-llm-ci-approvers`. The approval label remains in place when new commits are pushed.\n\n" + + "`--extra-stage \"H100_PCIe-TensorRT-Post-Merge-1, xxx\"` *(OPTIONAL)* : Run the ordinary L0 pre-merge pipeline and specified test stages. Supports wildcard `*` for pattern matching. Examples: --extra-stage \"H100_PCIe-TensorRT-Post-Merge-1, xxx\", --extra-stage \"*Post-Merge*\". The broad patterns `\"*\"` and `\"*Post-Merge*\"`, including equivalent escaped or repeated-star forms and their use in comma-separated lists, require the `ci: post-merge approved` PR label.\n\n" + "`--detailed-log ` *(OPTIONAL)* : Enable flushing out all logs to the Jenkins console. This will significantly increase the log volume and may slow down the job.\n\n" + "`--debug ` *(OPTIONAL)* : **Experimental feature**. Enable access to the CI container for debugging purpose. Note: Specify exactly one stage in the `stage-list` parameter to access the appropriate container environment. Note: Does **NOT** update GitHub check status.\n\n" + "`--high-priority ` *(OPTIONAL)* : Run the pipeline with high priority. This option is restricted to authorized users only and will route the job to a high-priority queue.\n\n" + diff --git a/.github/workflows/post-merge-approval.yml b/.github/workflows/post-merge-approval.yml new file mode 100644 index 000000000000..afd9ffe0fd5c --- /dev/null +++ b/.github/workflows/post-merge-approval.yml @@ -0,0 +1,103 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: Guard Post-Merge Approval Label + +on: + pull_request_target: + types: [labeled] + +permissions: + contents: read + pull-requests: write + +jobs: + guard-post-merge-approval: + if: >- + github.repository == 'NVIDIA/TensorRT-LLM' && + github.event.action == 'labeled' && + github.event.label.name == 'ci: post-merge approved' + runs-on: ubuntu-latest + steps: + - name: Validate post-merge approver + id: validate + if: github.event.action == 'labeled' + uses: actions/github-script@v8 + with: + github-token: ${{ secrets.TENSORRT_CICD_TRTLLM_CI_TOKEN }} + result-encoding: string + script: | + const actor = context.payload.sender?.login || context.actor; + try { + const response = await github.request( + 'GET /orgs/{org}/teams/{team_slug}/memberships/{username}', + { + org: 'NVIDIA', + team_slug: 'trt-llm-ci-approvers', + username: actor, + } + ); + const authorized = response.data.state === 'active'; + console.log( + actor + ' active membership in NVIDIA/trt-llm-ci-approvers: ' + authorized + ); + return authorized ? 'true' : 'false'; + } catch (error) { + if (error.status === 404) { + console.log( + actor + ' is not an active member of NVIDIA/trt-llm-ci-approvers.' + ); + } else { + core.warning( + 'Could not verify post-merge approver ' + actor + ': ' + error.message + ); + } + return 'false'; + } + + - name: Clear unauthorized post-merge approval + if: always() && steps.validate.outputs.result != 'true' + uses: actions/github-script@v8 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + const approvalLabel = 'ci: post-merge approved'; + const actor = context.payload.sender?.login || context.actor; + const owner = context.repo.owner; + const repo = context.repo.repo; + const issueNumber = context.payload.pull_request.number; + + try { + await github.rest.issues.removeLabel({ + owner, + repo, + issue_number: issueNumber, + name: approvalLabel, + }); + } catch (error) { + if (error.status !== 404) { + throw error; + } + } + + await github.rest.issues.createComment({ + owner, + repo, + issue_number: issueNumber, + body: + 'Removed the "' + approvalLabel + '" label because @' + actor + + ' could not be verified as an active member of ' + + 'NVIDIA/trt-llm-ci-approvers. Ask a member of that team to apply it.', + }); diff --git a/docs/source/developer-guide/ci-overview.md b/docs/source/developer-guide/ci-overview.md index 908315c3e3c1..1bbecd048e18 100644 --- a/docs/source/developer-guide/ci-overview.md +++ b/docs/source/developer-guide/ci-overview.md @@ -109,8 +109,15 @@ developers understand why the test is disabled. ### Triggering Post-merge tests -When you only need to verify a handful of post-merge tests, avoid the heavy -`/bot run --post-merge` command. Instead, specify exactly which stages to run: +Full `/bot run --post-merge` runs require the `ci: post-merge approved` PR +label because they can consume substantial shared GPU resources. The label must +be applied by an active member of the `NVIDIA/trt-llm-ci-approvers` GitHub team. +A GitHub workflow removes invalid approvals. The label remains in place when new +commits are pushed and can be removed manually when the approval no longer +applies. + +When you only need to verify a handful of post-merge tests, specify exactly +which stages to run: ```bash /bot run --stage-list "stage-A,stage-B" @@ -123,8 +130,14 @@ default pre-merge set: /bot run --extra-stage "stage-A,stage-B" ``` -Both options accept any stage name defined in `jenkins/L0_Test.groovy`. Being -selective keeps CI turnaround fast and conserves hardware resources. +Both options accept stage names and wildcard patterns defined in +`jenkins/L0_Test.groovy`. The broad `"*"` and `"*Post-Merge*"` selectors +require the same approval label, including when they appear in a comma-separated +list. Equivalent escaped or repeated-star forms are treated the same. Other +stage selectors, including explicit stage names and limited patterns such as +`"*PerfSanity*"`, retain their existing behavior. + +Being selective keeps CI turnaround fast and conserves hardware resources. ### Avoiding unnecessary `--disable-fail-fast` usage From ede2b4e6fde294c799c611257fa089ffe6763c90 Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Mon, 13 Jul 2026 17:49:21 +0800 Subject: [PATCH 2/6] ci: use shared TRT-LLM agent token Signed-off-by: Yanchao Lu --- .github/scripts/test_post_merge_approval_workflow.js | 3 ++- .github/workflows/post-merge-approval.yml | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/scripts/test_post_merge_approval_workflow.js b/.github/scripts/test_post_merge_approval_workflow.js index 3901b3ff75ea..5bc143fee5a9 100644 --- a/.github/scripts/test_post_merge_approval_workflow.js +++ b/.github/scripts/test_post_merge_approval_workflow.js @@ -64,7 +64,7 @@ async function main() { "pull-requests: write", "github.repository == 'NVIDIA/TensorRT-LLM'", "github.event.label.name == 'ci: post-merge approved'", - "secrets.TENSORRT_CICD_TRTLLM_CI_TOKEN", + "secrets.TRTLLM_AGENT_SHARED_TOKEN", "secrets.GITHUB_TOKEN", ]) { assert(workflowSource.includes(expected), "missing workflow contract: " + expected); @@ -75,6 +75,7 @@ async function main() { "pull-requests: read", "synchronize", "AUTO_LABEL_COMMUNITY_TOKEN", + "TENSORRT_CICD_TRTLLM_CI_TOKEN", ]) { assert(!workflowSource.includes(forbidden), "unsafe workflow contract: " + forbidden); } diff --git a/.github/workflows/post-merge-approval.yml b/.github/workflows/post-merge-approval.yml index afd9ffe0fd5c..0b6eab9aa547 100644 --- a/.github/workflows/post-merge-approval.yml +++ b/.github/workflows/post-merge-approval.yml @@ -36,7 +36,7 @@ jobs: if: github.event.action == 'labeled' uses: actions/github-script@v8 with: - github-token: ${{ secrets.TENSORRT_CICD_TRTLLM_CI_TOKEN }} + github-token: ${{ secrets.TRTLLM_AGENT_SHARED_TOKEN }} result-encoding: string script: | const actor = context.payload.sender?.login || context.actor; From f39f1dac95b0856a784f0db8494979ab22b523b8 Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Mon, 13 Jul 2026 19:59:01 +0800 Subject: [PATCH 3/6] ci: prevent stale approval cleanup Signed-off-by: Yanchao Lu --- .../test_post_merge_approval_workflow.js | 253 ++++++++++++++++-- .github/workflows/post-merge-approval.yml | 117 +++++++- 2 files changed, 344 insertions(+), 26 deletions(-) diff --git a/.github/scripts/test_post_merge_approval_workflow.js b/.github/scripts/test_post_merge_approval_workflow.js index 5bc143fee5a9..bdbda8bfdba2 100644 --- a/.github/scripts/test_post_merge_approval_workflow.js +++ b/.github/scripts/test_post_merge_approval_workflow.js @@ -64,6 +64,11 @@ async function main() { "pull-requests: write", "github.repository == 'NVIDIA/TensorRT-LLM'", "github.event.label.name == 'ci: post-merge approved'", + "concurrency:", + "cancel-in-progress: false", + "!cancelled()", + "listEventsForTimeline", + "VALIDATED_LABEL_EVENT_ID", "secrets.TRTLLM_AGENT_SHARED_TOKEN", "secrets.GITHUB_TOKEN", ]) { @@ -74,40 +79,78 @@ async function main() { "issues: write", "pull-requests: read", "synchronize", - "AUTO_LABEL_COMMUNITY_TOKEN", - "TENSORRT_CICD_TRTLLM_CI_TOKEN", ]) { assert(!workflowSource.includes(forbidden), "unsafe workflow contract: " + forbidden); } + const allowedSecrets = new Set([ + "TRTLLM_AGENT_SHARED_TOKEN", + "GITHUB_TOKEN", + ]); + for (const match of workflowSource.matchAll(/\bsecrets\.([A-Z0-9_]+)/g)) { + assert( + allowedSecrets.has(match[1]), + "unexpected workflow secret reference", + ); + } const scripts = extractGithubScripts(workflowSource); assert(scripts.length === 2, "expected two github-script blocks"); const validate = new AsyncFunction("github", "context", "core", scripts[0]); const cleanup = new AsyncFunction("github", "context", "core", scripts[1]); - async function runValidation(outcome, actor = "candidate", sender = true) { + async function runValidation( + outcome, + actor = "candidate", + sender = true, + senderLogin = actor, + events = null, + timelineError = null, + ) { let request; let warning = ""; - const payload = sender ? { sender: { login: actor } } : {}; + const outputs = {}; + const payload = { pull_request: { number: 123 } }; + if (sender) payload.sender = { login: senderLogin }; + const timelineEvents = events ?? [ + { + id: 101, + event: "labeled", + label: { name: "ci: post-merge approved" }, + actor: null, + }, + ]; const result = await validate( { + paginate: async () => { + if (timelineError) throw timelineError; + return timelineEvents; + }, + rest: { issues: { listEventsForTimeline: () => {} } }, request: async (route, args) => { request = { route, args }; if (outcome instanceof Error) throw outcome; return { data: { state: outcome } }; }, }, - { actor, payload }, + { actor, payload, repo: { owner: "NVIDIA", repo: "TensorRT-LLM" } }, { warning: (message) => { warning = message; }, + setOutput: (name, value) => { + outputs[name] = String(value); + }, }, ); - return { result, request, warning }; + return { result, request, warning, outputs }; } - const active = await runValidation("active", "approved-user"); + const active = await runValidation( + "active", + "context-actor", + true, + "approved-user", + ); assert(active.result === "true", "active member must be authorized"); assert( active.request.route === @@ -123,11 +166,71 @@ async function main() { active.request.args.username === "approved-user", "label event sender must be checked", ); + assert( + active.outputs.validated_actor === "approved-user", + "validated actor output must identify the label sender", + ); assert( (await runValidation("active", "fallback-actor", false)).request.args .username === "fallback-actor", "context actor must be used when the event sender is absent", ); + const latestEvent = await runValidation( + "active", + "context-actor", + true, + "payload-actor", + [ + { + id: 100, + event: "labeled", + label: { name: "ci: post-merge approved" }, + actor: { login: "older-actor" }, + }, + { + id: 101, + event: "labeled", + label: { name: "ci: post-merge approved" }, + actor: { login: "latest-approver" }, + }, + ], + ); + assert( + latestEvent.request.args.username === "latest-approver", + "the latest approval-label event actor must be validated", + ); + assert( + latestEvent.outputs.label_event_id === "101", + "the validated label event must be published for cleanup re-checking", + ); + const missingTimeline = await runValidation( + "active", + "context-actor", + true, + "approved-user", + [], + ); + assert( + missingTimeline.result === "false", + "missing timeline event must fail closed", + ); + assert( + missingTimeline.request === undefined, + "membership must not be trusted without a bound label event", + ); + const timelineReadError = new Error("timeline unavailable"); + const unreadableTimeline = await runValidation( + "active", + "context-actor", + true, + "approved-user", + null, + timelineReadError, + ); + assert( + unreadableTimeline.result === "false", + "timeline API failure must fail closed", + ); assert( (await runValidation("pending")).result === "false", "pending member must be rejected", @@ -146,11 +249,37 @@ async function main() { assert(apiFailure.result === "false", "API failure must fail closed"); assert(apiFailure.warning.includes("API unavailable"), "failure must be logged"); - async function runCleanup(removeError = null) { - const calls = { remove: [], comment: [] }; + async function runCleanup({ + removeError = null, + labels = [{ name: "ci: post-merge approved" }], + events = [ + { + id: 101, + event: "labeled", + label: { name: "ci: post-merge approved" }, + actor: { login: "label-actor" }, + }, + ], + validatedActor = "label-actor", + validatedEventId = "101", + timelineError = null, + } = {}) { + const calls = { remove: [], comment: [], warning: [] }; + const previousActor = process.env.VALIDATED_ACTOR; + const previousEventId = process.env.VALIDATED_LABEL_EVENT_ID; + process.env.VALIDATED_ACTOR = validatedActor; + process.env.VALIDATED_LABEL_EVENT_ID = validatedEventId; const github = { + paginate: async () => { + if (timelineError) throw timelineError; + return events; + }, rest: { + pulls: { + get: async () => ({ data: { labels } }), + }, issues: { + listEventsForTimeline: () => {}, removeLabel: async (args) => { calls.remove.push(args); if (removeError) throw removeError; @@ -161,19 +290,31 @@ async function main() { }, }, }; - await cleanup( - github, - { - actor: "fallback", - repo: { owner: "NVIDIA", repo: "TensorRT-LLM" }, - payload: { - action: "labeled", - sender: { login: "label-actor" }, - pull_request: { number: 123 }, + try { + await cleanup( + github, + { + actor: "fallback", + repo: { owner: "NVIDIA", repo: "TensorRT-LLM" }, + payload: { + action: "labeled", + sender: { login: "label-actor" }, + pull_request: { number: 123 }, + }, }, - }, - {}, - ); + { + warning: (message) => calls.warning.push(message), + }, + ); + } finally { + if (previousActor === undefined) delete process.env.VALIDATED_ACTOR; + else process.env.VALIDATED_ACTOR = previousActor; + if (previousEventId === undefined) { + delete process.env.VALIDATED_LABEL_EVENT_ID; + } else { + process.env.VALIDATED_LABEL_EVENT_ID = previousEventId; + } + } return calls; } @@ -187,16 +328,82 @@ async function main() { const concurrentRemoval = new Error("not found"); concurrentRemoval.status = 404; - const concurrent = await runCleanup(concurrentRemoval); + const concurrent = await runCleanup({ removeError: concurrentRemoval }); assert( concurrent.comment.length === 1, "concurrent removal must not suppress the denial comment", ); + const staleRun = await runCleanup({ + events: [ + { + id: 101, + event: "labeled", + label: { name: "ci: post-merge approved" }, + actor: { login: "label-actor" }, + }, + { + id: 102, + event: "labeled", + label: { name: "ci: post-merge approved" }, + actor: { login: "new-approver" }, + }, + ], + }); + assert(staleRun.remove.length === 0, "a stale run must not remove a newer label"); + assert(staleRun.comment.length === 0, "a stale run must not post a denial comment"); + + const alreadyAbsent = await runCleanup({ labels: [] }); + assert(alreadyAbsent.remove.length === 0, "an absent label needs no cleanup"); + + const timelineFailure = new Error("timeline unavailable"); + const failClosed = await runCleanup({ timelineError: timelineFailure }); + assert(failClosed.remove.length === 1, "timeline failure must fail closed"); + assert( + failClosed.warning[0].includes("timeline unavailable"), + "timeline failure must be logged", + ); + + const unboundValidation = await runCleanup({ validatedEventId: "" }); + assert( + unboundValidation.remove.length === 1, + "cleanup without a bound event must fail closed", + ); + + const missingLatestActor = await runCleanup({ + events: [ + { + id: 101, + event: "labeled", + label: { name: "ci: post-merge approved" }, + actor: null, + }, + ], + }); + assert( + missingLatestActor.remove.length === 1, + "cleanup without a latest actor must fail closed", + ); + + const sameActorNewEvent = await runCleanup({ + events: [ + { + id: 102, + event: "labeled", + label: { name: "ci: post-merge approved" }, + actor: { login: "label-actor" }, + }, + ], + }); + assert( + sameActorNewEvent.remove.length === 0, + "a newer event from the same actor must not be removed by a stale run", + ); + console.log( "Post-merge approval workflow passed wiring, permissions, active, pending, " + "non-member, API-failure, unauthorized-cleanup, commit-persistence, and " + - "concurrent-removal tests.", + "stale-run cleanup tests.", ); } diff --git a/.github/workflows/post-merge-approval.yml b/.github/workflows/post-merge-approval.yml index 0b6eab9aa547..cf6977ecfda3 100644 --- a/.github/workflows/post-merge-approval.yml +++ b/.github/workflows/post-merge-approval.yml @@ -25,6 +25,9 @@ permissions: jobs: guard-post-merge-approval: + concurrency: + group: post-merge-approval-${{ github.event.pull_request.number }} + cancel-in-progress: false if: >- github.repository == 'NVIDIA/TensorRT-LLM' && github.event.action == 'labeled' && @@ -39,7 +42,44 @@ jobs: github-token: ${{ secrets.TRTLLM_AGENT_SHARED_TOKEN }} result-encoding: string script: | - const actor = context.payload.sender?.login || context.actor; + const approvalLabel = 'ci: post-merge approved'; + const owner = context.repo.owner; + const repo = context.repo.repo; + const issueNumber = context.payload.pull_request.number; + let actor = context.payload.sender?.login || context.actor; + let labelEventId = ''; + let timelineVerified = false; + + try { + const events = await github.paginate( + github.rest.issues.listEventsForTimeline, + { owner, repo, issue_number: issueNumber, per_page: 100 } + ); + const latestApprovalEvent = events + .filter( + (event) => + event.event === 'labeled' && + event.label?.name === approvalLabel + ) + .at(-1); + if (latestApprovalEvent) { + actor = latestApprovalEvent.actor?.login || actor; + labelEventId = String(latestApprovalEvent.id); + timelineVerified = true; + } else { + core.warning('Could not identify the latest post-merge approval event.'); + } + } catch (error) { + core.warning( + 'Could not read the latest post-merge approval event: ' + error.message + ); + } + + core.setOutput('validated_actor', actor); + core.setOutput('label_event_id', labelEventId); + if (!timelineVerified) { + return 'false'; + } try { const response = await github.request( 'GET /orgs/{org}/teams/{team_slug}/memberships/{username}', @@ -68,17 +108,88 @@ jobs: } - name: Clear unauthorized post-merge approval - if: always() && steps.validate.outputs.result != 'true' + if: always() && !cancelled() && steps.validate.outputs.result != 'true' uses: actions/github-script@v8 + env: + VALIDATED_ACTOR: ${{ steps.validate.outputs.validated_actor }} + VALIDATED_LABEL_EVENT_ID: ${{ steps.validate.outputs.label_event_id }} with: github-token: ${{ secrets.GITHUB_TOKEN }} script: | const approvalLabel = 'ci: post-merge approved'; - const actor = context.payload.sender?.login || context.actor; + const actor = + process.env.VALIDATED_ACTOR || + context.payload.sender?.login || + context.actor; + const validatedEventId = process.env.VALIDATED_LABEL_EVENT_ID || ''; const owner = context.repo.owner; const repo = context.repo.repo; const issueNumber = context.payload.pull_request.number; + try { + const pullRequest = await github.rest.pulls.get({ + owner, + repo, + pull_number: issueNumber, + }); + const labelIsPresent = pullRequest.data.labels.some( + (label) => label.name === approvalLabel + ); + if (!labelIsPresent) { + console.log('Post-merge approval label is already absent; no cleanup needed.'); + return; + } + } catch (error) { + core.warning( + 'Could not read the current post-merge approval label state; continuing validation: ' + + error.message + ); + } + + try { + const events = await github.paginate( + github.rest.issues.listEventsForTimeline, + { owner, repo, issue_number: issueNumber, per_page: 100 } + ); + const latestApprovalEvent = events + .filter( + (event) => + event.event === 'labeled' && + event.label?.name === approvalLabel + ) + .at(-1); + const latestEventId = latestApprovalEvent + ? String(latestApprovalEvent.id) + : ''; + const latestActor = latestApprovalEvent?.actor?.login || ''; + if (!latestApprovalEvent) { + core.warning( + 'Could not identify the latest post-merge approval event; removing the label to fail closed.' + ); + } else if (!latestActor) { + core.warning( + 'Could not identify the latest post-merge approval actor; removing the label to fail closed.' + ); + } else if (!validatedEventId) { + core.warning( + 'The validation run did not bind an approval event; removing the label to fail closed.' + ); + } else if ( + latestEventId !== validatedEventId || + latestActor !== actor + ) { + console.log( + 'A newer post-merge approval event was found; leaving it for its own validation run.' + ); + return; + } + } catch (error) { + core.warning( + 'Could not re-check the latest post-merge approval event; removing the label to fail closed: ' + + error.message + ); + } + try { await github.rest.issues.removeLabel({ owner, From e23c845ad9e32aaf5ec3562fb06bcd2c8ac8c604 Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Tue, 14 Jul 2026 14:58:36 +0800 Subject: [PATCH 4/6] docs: explain post-merge workflow trigger safety Signed-off-by: Yanchao Lu --- .github/workflows/post-merge-approval.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/post-merge-approval.yml b/.github/workflows/post-merge-approval.yml index cf6977ecfda3..7d57abe9a729 100644 --- a/.github/workflows/post-merge-approval.yml +++ b/.github/workflows/post-merge-approval.yml @@ -16,6 +16,8 @@ name: Guard Post-Merge Approval Label on: + # Intentional: this runs only static default-branch API logic. It never checks + # out or executes PR code and reads only PR timeline and label metadata. pull_request_target: types: [labeled] From 9d790053546001d5cc19d4dcf29731a1915f302f Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Tue, 14 Jul 2026 15:00:37 +0800 Subject: [PATCH 5/6] docs: clarify post-merge workflow API usage Signed-off-by: Yanchao Lu --- .github/workflows/post-merge-approval.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/post-merge-approval.yml b/.github/workflows/post-merge-approval.yml index 7d57abe9a729..31aa37370772 100644 --- a/.github/workflows/post-merge-approval.yml +++ b/.github/workflows/post-merge-approval.yml @@ -16,8 +16,9 @@ name: Guard Post-Merge Approval Label on: - # Intentional: this runs only static default-branch API logic. It never checks - # out or executes PR code and reads only PR timeline and label metadata. + # Intentional: this runs static default-branch API logic only. It never checks + # out or executes PR code; it uses GitHub APIs to validate membership and + # manage this PR's approval label/comment. pull_request_target: types: [labeled] From b9e142085e9e1606e071e5c6cf7ce1eaed924130 Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Wed, 15 Jul 2026 14:21:27 +0800 Subject: [PATCH 6/6] test: remove unexecuted workflow harness Signed-off-by: Yanchao Lu --- .../test_post_merge_approval_workflow.js | 413 ------------------ 1 file changed, 413 deletions(-) delete mode 100644 .github/scripts/test_post_merge_approval_workflow.js diff --git a/.github/scripts/test_post_merge_approval_workflow.js b/.github/scripts/test_post_merge_approval_workflow.js deleted file mode 100644 index bdbda8bfdba2..000000000000 --- a/.github/scripts/test_post_merge_approval_workflow.js +++ /dev/null @@ -1,413 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -const fs = require("fs"); -const path = require("path"); - -const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor; -const WORKFLOW = path.join( - __dirname, - "..", - "workflows", - "post-merge-approval.yml", -); - -function assert(condition, message) { - if (!condition) throw new Error(message); -} - -function extractGithubScripts(source) { - const lines = source.split(/\r?\n/); - const scripts = []; - - for (let index = 0; index < lines.length; index += 1) { - const marker = lines[index].match(/^(\s*)script:\s*\|\s*$/); - if (!marker) continue; - - const markerIndent = marker[1].length; - const contentIndent = markerIndent + 2; - const body = []; - for (index += 1; index < lines.length; index += 1) { - const line = lines[index]; - if (line.trim() === "") { - body.push(""); - } else if (line.match(/^ */)[0].length <= markerIndent) { - index -= 1; - break; - } else { - body.push(line.slice(contentIndent)); - } - } - scripts.push(body.join("\n")); - } - return scripts; -} - -async function main() { - const workflowSource = fs.readFileSync(WORKFLOW, "utf8"); - for (const expected of [ - "pull_request_target:", - "types: [labeled]", - "contents: read", - "pull-requests: write", - "github.repository == 'NVIDIA/TensorRT-LLM'", - "github.event.label.name == 'ci: post-merge approved'", - "concurrency:", - "cancel-in-progress: false", - "!cancelled()", - "listEventsForTimeline", - "VALIDATED_LABEL_EVENT_ID", - "secrets.TRTLLM_AGENT_SHARED_TOKEN", - "secrets.GITHUB_TOKEN", - ]) { - assert(workflowSource.includes(expected), "missing workflow contract: " + expected); - } - for (const forbidden of [ - "actions/checkout", - "issues: write", - "pull-requests: read", - "synchronize", - ]) { - assert(!workflowSource.includes(forbidden), "unsafe workflow contract: " + forbidden); - } - const allowedSecrets = new Set([ - "TRTLLM_AGENT_SHARED_TOKEN", - "GITHUB_TOKEN", - ]); - for (const match of workflowSource.matchAll(/\bsecrets\.([A-Z0-9_]+)/g)) { - assert( - allowedSecrets.has(match[1]), - "unexpected workflow secret reference", - ); - } - - const scripts = extractGithubScripts(workflowSource); - assert(scripts.length === 2, "expected two github-script blocks"); - const validate = new AsyncFunction("github", "context", "core", scripts[0]); - const cleanup = new AsyncFunction("github", "context", "core", scripts[1]); - - async function runValidation( - outcome, - actor = "candidate", - sender = true, - senderLogin = actor, - events = null, - timelineError = null, - ) { - let request; - let warning = ""; - const outputs = {}; - const payload = { pull_request: { number: 123 } }; - if (sender) payload.sender = { login: senderLogin }; - const timelineEvents = events ?? [ - { - id: 101, - event: "labeled", - label: { name: "ci: post-merge approved" }, - actor: null, - }, - ]; - const result = await validate( - { - paginate: async () => { - if (timelineError) throw timelineError; - return timelineEvents; - }, - rest: { issues: { listEventsForTimeline: () => {} } }, - request: async (route, args) => { - request = { route, args }; - if (outcome instanceof Error) throw outcome; - return { data: { state: outcome } }; - }, - }, - { actor, payload, repo: { owner: "NVIDIA", repo: "TensorRT-LLM" } }, - { - warning: (message) => { - warning = message; - }, - setOutput: (name, value) => { - outputs[name] = String(value); - }, - }, - ); - return { result, request, warning, outputs }; - } - - const active = await runValidation( - "active", - "context-actor", - true, - "approved-user", - ); - assert(active.result === "true", "active member must be authorized"); - assert( - active.request.route === - "GET /orgs/{org}/teams/{team_slug}/memberships/{username}", - "unexpected membership route", - ); - assert(active.request.args.org === "NVIDIA", "unexpected organization"); - assert( - active.request.args.team_slug === "trt-llm-ci-approvers", - "unexpected Team slug", - ); - assert( - active.request.args.username === "approved-user", - "label event sender must be checked", - ); - assert( - active.outputs.validated_actor === "approved-user", - "validated actor output must identify the label sender", - ); - assert( - (await runValidation("active", "fallback-actor", false)).request.args - .username === "fallback-actor", - "context actor must be used when the event sender is absent", - ); - const latestEvent = await runValidation( - "active", - "context-actor", - true, - "payload-actor", - [ - { - id: 100, - event: "labeled", - label: { name: "ci: post-merge approved" }, - actor: { login: "older-actor" }, - }, - { - id: 101, - event: "labeled", - label: { name: "ci: post-merge approved" }, - actor: { login: "latest-approver" }, - }, - ], - ); - assert( - latestEvent.request.args.username === "latest-approver", - "the latest approval-label event actor must be validated", - ); - assert( - latestEvent.outputs.label_event_id === "101", - "the validated label event must be published for cleanup re-checking", - ); - const missingTimeline = await runValidation( - "active", - "context-actor", - true, - "approved-user", - [], - ); - assert( - missingTimeline.result === "false", - "missing timeline event must fail closed", - ); - assert( - missingTimeline.request === undefined, - "membership must not be trusted without a bound label event", - ); - const timelineReadError = new Error("timeline unavailable"); - const unreadableTimeline = await runValidation( - "active", - "context-actor", - true, - "approved-user", - null, - timelineReadError, - ); - assert( - unreadableTimeline.result === "false", - "timeline API failure must fail closed", - ); - assert( - (await runValidation("pending")).result === "false", - "pending member must be rejected", - ); - - const notFound = new Error("not found"); - notFound.status = 404; - assert( - (await runValidation(notFound, "non-member")).result === "false", - "non-member must be rejected", - ); - - const apiError = new Error("API unavailable"); - apiError.status = 500; - const apiFailure = await runValidation(apiError); - assert(apiFailure.result === "false", "API failure must fail closed"); - assert(apiFailure.warning.includes("API unavailable"), "failure must be logged"); - - async function runCleanup({ - removeError = null, - labels = [{ name: "ci: post-merge approved" }], - events = [ - { - id: 101, - event: "labeled", - label: { name: "ci: post-merge approved" }, - actor: { login: "label-actor" }, - }, - ], - validatedActor = "label-actor", - validatedEventId = "101", - timelineError = null, - } = {}) { - const calls = { remove: [], comment: [], warning: [] }; - const previousActor = process.env.VALIDATED_ACTOR; - const previousEventId = process.env.VALIDATED_LABEL_EVENT_ID; - process.env.VALIDATED_ACTOR = validatedActor; - process.env.VALIDATED_LABEL_EVENT_ID = validatedEventId; - const github = { - paginate: async () => { - if (timelineError) throw timelineError; - return events; - }, - rest: { - pulls: { - get: async () => ({ data: { labels } }), - }, - issues: { - listEventsForTimeline: () => {}, - removeLabel: async (args) => { - calls.remove.push(args); - if (removeError) throw removeError; - }, - createComment: async (args) => { - calls.comment.push(args); - }, - }, - }, - }; - try { - await cleanup( - github, - { - actor: "fallback", - repo: { owner: "NVIDIA", repo: "TensorRT-LLM" }, - payload: { - action: "labeled", - sender: { login: "label-actor" }, - pull_request: { number: 123 }, - }, - }, - { - warning: (message) => calls.warning.push(message), - }, - ); - } finally { - if (previousActor === undefined) delete process.env.VALIDATED_ACTOR; - else process.env.VALIDATED_ACTOR = previousActor; - if (previousEventId === undefined) { - delete process.env.VALIDATED_LABEL_EVENT_ID; - } else { - process.env.VALIDATED_LABEL_EVENT_ID = previousEventId; - } - } - return calls; - } - - const unauthorized = await runCleanup(); - assert(unauthorized.remove.length === 1, "label must be removed"); - assert(unauthorized.comment.length === 1, "denial must be explained"); - assert( - unauthorized.comment[0].body.includes("label-actor"), - "comment must identify the label actor", - ); - - const concurrentRemoval = new Error("not found"); - concurrentRemoval.status = 404; - const concurrent = await runCleanup({ removeError: concurrentRemoval }); - assert( - concurrent.comment.length === 1, - "concurrent removal must not suppress the denial comment", - ); - - const staleRun = await runCleanup({ - events: [ - { - id: 101, - event: "labeled", - label: { name: "ci: post-merge approved" }, - actor: { login: "label-actor" }, - }, - { - id: 102, - event: "labeled", - label: { name: "ci: post-merge approved" }, - actor: { login: "new-approver" }, - }, - ], - }); - assert(staleRun.remove.length === 0, "a stale run must not remove a newer label"); - assert(staleRun.comment.length === 0, "a stale run must not post a denial comment"); - - const alreadyAbsent = await runCleanup({ labels: [] }); - assert(alreadyAbsent.remove.length === 0, "an absent label needs no cleanup"); - - const timelineFailure = new Error("timeline unavailable"); - const failClosed = await runCleanup({ timelineError: timelineFailure }); - assert(failClosed.remove.length === 1, "timeline failure must fail closed"); - assert( - failClosed.warning[0].includes("timeline unavailable"), - "timeline failure must be logged", - ); - - const unboundValidation = await runCleanup({ validatedEventId: "" }); - assert( - unboundValidation.remove.length === 1, - "cleanup without a bound event must fail closed", - ); - - const missingLatestActor = await runCleanup({ - events: [ - { - id: 101, - event: "labeled", - label: { name: "ci: post-merge approved" }, - actor: null, - }, - ], - }); - assert( - missingLatestActor.remove.length === 1, - "cleanup without a latest actor must fail closed", - ); - - const sameActorNewEvent = await runCleanup({ - events: [ - { - id: 102, - event: "labeled", - label: { name: "ci: post-merge approved" }, - actor: { login: "label-actor" }, - }, - ], - }); - assert( - sameActorNewEvent.remove.length === 0, - "a newer event from the same actor must not be removed by a stale run", - ); - - console.log( - "Post-merge approval workflow passed wiring, permissions, active, pending, " + - "non-member, API-failure, unauthorized-cleanup, commit-persistence, and " + - "stale-run cleanup tests.", - ); -} - -main().catch((error) => { - console.error(error); - process.exit(1); -});