From 34b38b56a4464adf4c6d00ff22d685aef22b00b9 Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Wed, 16 Sep 2026 17:07:40 +0800 Subject: [PATCH 01/21] [None][infra] Check semantic conflicts with CodeRabbit Signed-off-by: Yanchao Lu --- .coderabbit.yaml | 35 ++++ .../coderabbit_semantic_review.test.js | 149 ++++++++++++++++++ .../workflows/coderabbit-semantic-review.yml | 122 ++++++++++++++ .github/workflows/precommit-check.yml | 3 + AGENTS.md | 15 ++ 5 files changed, 324 insertions(+) create mode 100644 .github/scripts/coderabbit_semantic_review.test.js create mode 100644 .github/workflows/coderabbit-semantic-review.yml diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 2101ac66113d..fcb0f643e119 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -77,6 +77,41 @@ reviews: drafts: false base_branches: ["main", "release/.+"] + # Best-effort semantic compatibility; rerun when the target branch advances. + # @coderabbitai run pre-merge checks + pre_merge_checks: + custom_checks: + - name: "Semantic conflict with target branch" + mode: warning + instructions: | + Detect behavioral incompatibilities when this PR is combined with its + current target branch, even when Git can merge without text conflicts. + Use repository and Git evidence, not claims in the PR description. + Resolve and report the PR head SHA, current target branch SHA, and + merge-base SHA. Verify the live target and PR head; a cached local ref + or the target SHA from an earlier review is not sufficient. If these + revisions or necessary code/history are unavailable, return Inconclusive + and explain the missing evidence. Never invent a SHA or claim freshness. + + Compare merge-base..PR-head and merge-base..target, then inspect their + combined behavior. Follow affected callers, callees, configuration, + bindings, and tests across files, including unchanged dependencies. + Prioritize API/return-value contracts, changed defaults, tensor shapes + and dtypes, resource lifetimes, and distributed synchronization. + Example: the target changes a function's return units while this PR + adds a caller expecting the old units in another file. + + Fail only for a concrete incompatibility between the PR changes and + the target code. Cite both relevant code locations, the violated + contract, a triggering scenario, confidence, and a minimal regression + test. Exclude unrelated pre-existing bugs and style suggestions. + Do not execute repository code or claim tests were run. + Pass only if the required context was inspected and no supported + conflict was found; this is not proof of semantic compatibility. + If either branch changes during analysis, return Inconclusive. + State that the result applies only to the reported SHA pair and must + be rerun after either branch changes. + path_filters: # Vendored/adapted FlashInfer kernels; excluded from review. - "!tensorrt_llm/_torch/attention/backends/prims_ts/**" diff --git a/.github/scripts/coderabbit_semantic_review.test.js b/.github/scripts/coderabbit_semantic_review.test.js new file mode 100644 index 000000000000..f9d4f1aa8081 --- /dev/null +++ b/.github/scripts/coderabbit_semantic_review.test.js @@ -0,0 +1,149 @@ +// 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 assert = require('node:assert/strict'); +const {test} = require('node:test'); +const fs = require('node:fs'); +const path = require('node:path'); +const workflow = fs.readFileSync( + path.join(__dirname, '../workflows/coderabbit-semantic-review.yml'), 'utf8'); +const marker = ' script: |\n'; +assert.ok(workflow.includes(marker)); +const script = workflow.slice(workflow.indexOf(marker) + marker.length) + .split('\n').map(line => line.replace(/^ {12}/, '')).join('\n'); +const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor; +const execute = new AsyncFunction('github', 'context', 'core', 'process', script); + +const HEAD = 'a'.repeat(40); +const BASE = 'b'.repeat(40); +const NEW_BASE = 'c'.repeat(40); + +function harness(overrides = {}) { + const pr = {number: 12, state: 'open', draft: false, + head: {sha: HEAD}, base: {ref: 'main', sha: 'outdated-event-sha'}, + labels: [{name: 'ai: semantic-conflict'}], ...overrides}; + const comments = []; + const posted = []; + let target = BASE; + const calls = []; + const github = { + rest: { + pulls: { + list: 'list-pulls', + get: async args => {calls.push(['get-pr', args]); return {data: pr};}, + }, + git: {getRef: async args => { + calls.push(['get-ref', args]); + return {data: {object: {sha: target}}}; + }}, + issues: { + listComments: 'list-comments', + createComment: async args => { + posted.push(args); + comments.push({body: args.body, user: {login: 'github-actions[bot]'}}); + return {data: {html_url: `https://github.com/example/repo/pull/12#${posted.length}`}}; + }, + }, + }, + paginate: async (method, args) => { + calls.push([method, args]); + if (method === 'list-pulls') return [pr]; + assert.equal(method, 'list-comments'); + return comments; + }, + }; + const summaries = []; + const core = {info: () => {}, summary: { + addRaw(text) {summaries.push(text); return this;}, + async write() {}, + }}; + const context = {repo: {owner: 'example', repo: 'repo'}, + eventName: 'pull_request_target', payload: {pull_request: {number: 12}}}; + return {pr, comments, posted, calls, summaries, + setTarget: sha => {target = sha;}, + run: (eventName = 'pull_request_target', pullNumber = '') => + execute(github, {...context, eventName}, core, {env: {DISPATCH_PULL_NUMBER: pullNumber}}), + }; +} + +test('requests the live main SHA and never treats dispatch as an AI verdict', async () => { + const h = harness(); + await h.run(); + assert.equal(h.posted.length, 1); + assert.match(h.posted[0].body, /^@coderabbitai run pre-merge checks/); + assert.ok(h.posted[0].body.includes(HEAD)); + assert.ok(h.posted[0].body.includes(BASE)); + assert.ok(!h.posted[0].body.includes('outdated-event-sha')); + assert.match(h.posted[0].body, /return Inconclusive/); + assert.match(h.summaries[0], /No AI verdict is asserted/); + assert.deepEqual(h.calls.find(c => c[0] === 'get-ref')[1], + {owner: 'example', repo: 'repo', ref: 'heads/main'}); +}); + +test('a main-only update triggers another review with unchanged PR head', async () => { + const h = harness(); + await h.run('push'); + h.setTarget(NEW_BASE); + await h.run('push'); + assert.equal(h.posted.length, 2); + assert.ok(h.posted[1].body.includes(`${HEAD}:${NEW_BASE}`)); + assert.ok(!h.posted[1].body.includes(BASE)); +}); + +test('a PR-only update triggers another review', async () => { + const h = harness(); + await h.run(); + h.pr.head.sha = 'd'.repeat(40); + await h.run(); + assert.equal(h.posted.length, 2); + assert.ok(h.posted[1].body.includes(h.pr.head.sha)); +}); + +test('duplicates are suppressed, but workflow dispatch retries the same pair', async () => { + const h = harness(); + await h.run(); + await h.run('push'); + assert.equal(h.posted.length, 1); + await h.run('workflow_dispatch', '12'); + assert.equal(h.posted.length, 2); +}); + +test('a marker posted by another user cannot suppress requests', async () => { + const h = harness(); + h.comments.push({body: ``, + user: {login: 'someone-else'}}); + await h.run(); + assert.equal(h.posted.length, 1); +}); + +test('closed, draft, unlabelled, and release PRs are not reviewed', async () => { + for (const overrides of [{state: 'closed'}, {draft: true}, {labels: []}, + {base: {ref: 'release/1.0'}}]) { + const h = harness(overrides); + await h.run(); + await h.run('workflow_dispatch', '12'); + assert.equal(h.posted.length, 0); + assert.ok(!h.calls.some(c => c[0] === 'get-ref')); + } +}); + +test('invalid dispatch inputs and unsupported events cannot post comments', async () => { + for (const value of ['', '0', '-1', '1.2', '12x', '9007199254740992']) { + const h = harness(); + await assert.rejects(h.run('workflow_dispatch', value), /positive integer/); + assert.equal(h.posted.length, 0); + } + await assert.rejects(harness().run('issue_comment'), /Unsupported event/); +}); diff --git a/.github/workflows/coderabbit-semantic-review.yml b/.github/workflows/coderabbit-semantic-review.yml new file mode 100644 index 000000000000..b4a41e2217fc --- /dev/null +++ b/.github/workflows/coderabbit-semantic-review.yml @@ -0,0 +1,122 @@ +# 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: CodeRabbit Semantic Conflict Review + +on: + pull_request_target: + branches: [main] + types: [opened, reopened, synchronize, ready_for_review, labeled, edited, auto_merge_enabled] + push: + branches: [main] + workflow_dispatch: + inputs: + pull_number: + description: 'Open PR number to recheck (requires ai: semantic-conflict label)' + required: true + type: string + +permissions: + contents: read + pull-requests: read + issues: write + +# Coalesce repeated events per PR or main; each run reads current refs. +concurrency: + group: coderabbit-semantic-conflict-${{ github.event.pull_request.number || inputs.pull_number || 'main' }} + cancel-in-progress: false + +jobs: + request-review: + name: Request advisory semantic review + if: github.repository == 'NVIDIA/TensorRT-LLM' + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + # API-only trusted workflow code; no repository checkout is needed. + - name: Request CodeRabbit checks for the current SHA pair + uses: actions/github-script@v8 + env: + DISPATCH_PULL_NUMBER: ${{ inputs.pull_number }} + with: + script: | + const LABEL = 'ai: semantic-conflict'; + const BOT = 'github-actions[bot]'; + + const pullNumber = process.env.DISPATCH_PULL_NUMBER; + const repo = context.repo; + let numbers; + if (context.eventName === 'workflow_dispatch') { + const raw = (pullNumber || '').trim(); + const number = Number(raw); + if (!/^[1-9][0-9]*$/.test(raw) || !Number.isSafeInteger(number)) { + throw new Error('pull_number must be a positive integer'); + } + numbers = [number]; + } else if (context.eventName === 'push') { + const pulls = await github.paginate(github.rest.pulls.list, { + ...repo, state: 'open', base: 'main', per_page: 100, + }); + numbers = pulls.filter(pr => !pr.draft && pr.labels.some(l => l.name === LABEL)) + .map(pr => pr.number); + } else if (context.eventName === 'pull_request_target') { + numbers = [context.payload.pull_request.number]; + } else { + throw new Error(`Unsupported event: ${context.eventName}`); + } + + for (const number of numbers) { + // Read current metadata instead of trusting a possibly queued event payload. + const {data: pr} = await github.rest.pulls.get({...repo, pull_number: number}); + if (pr.state !== 'open' || pr.draft || pr.base.ref !== 'main' || + !pr.labels.some(l => l.name === LABEL)) { + core.info(`PR #${number}: not an open, non-draft, opted-in main PR; skipped.`); + continue; + } + const {data: ref} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); + const head = pr.head.sha; + const base = ref.object.sha; + const marker = ``; + const comments = await github.paginate(github.rest.issues.listComments, { + ...repo, issue_number: number, per_page: 100, + }); + // Only our own bot's comments can suppress an automatic request. A manual + // dispatch always retries, including after a CodeRabbit timeout or failure. + if (context.eventName !== 'workflow_dispatch' && comments.some(comment => + comment.user?.login === BOT && comment.body?.includes(marker))) { + core.info(`PR #${number}: this SHA pair has already been requested.`); + continue; + } + const body = [ + '@coderabbitai run pre-merge checks', + '', + marker, + `Semantic compatibility review requested for PR head \`${head}\` and main \`${base}\`.`, + 'Run the configured "Semantic conflict with target branch" check against these revisions.', + 'Verify both live refs. If either differs or cannot be inspected, return Inconclusive.', + 'Report the actual head, target, and merge-base SHAs with the analysis.', + '', + 'This is an advisory request, not a passing check. Previous results for a different', + 'SHA pair are stale; a successful workflow only means the request was posted.', + ].join('\n'); + const {data: comment} = await github.rest.issues.createComment({ + ...repo, issue_number: number, body, + }); + core.info(`PR #${number}: requested ${head} + ${base}: ${comment.html_url}`); + await core.summary.addRaw( + `PR #${number}: [requested CodeRabbit analysis](${comment.html_url}) for ` + + `head \`${head}\` + main \`${base}\`. No AI verdict is asserted.\n\n` + ).write(); + } diff --git a/.github/workflows/precommit-check.yml b/.github/workflows/precommit-check.yml index 3e68e6df598d..4931583e9bf7 100644 --- a/.github/workflows/precommit-check.yml +++ b/.github/workflows/precommit-check.yml @@ -36,6 +36,9 @@ jobs: - name: Test stale pull request cleanup workflow run: node .github/scripts/cleanup_stale_prs.test.js + - name: Test semantic conflict review requests + run: node --test .github/scripts/coderabbit_semantic_review.test.js + - uses: actions/setup-python@v6 with: python-version: '3.12' diff --git a/AGENTS.md b/AGENTS.md index 7d729ff65553..9d0183129862 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -176,6 +176,21 @@ CI is triggered by posting comments on the PR. Basic commands: For a full list of up-to-date bot commands, post `/bot help` as a PR comment and check the bot's reply. +### Advisory semantic conflict review + +CodeRabbit's `Semantic conflict with target branch` pre-merge check in +`.coderabbit.yaml` looks for behavioral incompatibilities across branches. It is +best-effort and warning-only; missing revision/history evidence is Inconclusive. + +For a non-draft PR targeting `main`, maintainers can add the +`ai: semantic-conflict` label to opt into automatic rechecks on PR and main +updates. The `CodeRabbit Semantic Conflict Review` workflow only posts requests; +its success is not an AI verdict. Read CodeRabbit's result and verify its head +and target SHAs still match the live branches. Use workflow dispatch with the +PR number to retry a request, or comment `@coderabbitai run pre-merge checks`. +The custom check requires CodeRabbit Custom Pre-Merge Checks access. This +advisory pilot does not provide a merge-queue check or block merges. + ### Trouble Shooting - Use `TLLM_LOG_LEVEL_BY_MODULE` to enable per-module log filtering (e.g., `"debug:_torch,runtime;info:serve"`); see [Module-Level Logging](docs/source/developer-guide/overview.md#module-level-logging) for details. From 7d4a6e2ce53dd3287113832a03b308ac44617c9b Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Wed, 16 Sep 2026 17:20:09 +0800 Subject: [PATCH 02/21] [None][infra] Serialize semantic review requests Signed-off-by: Yanchao Lu --- .../coderabbit_semantic_review.test.js | 25 ++++++++++-- .../workflows/coderabbit-semantic-review.yml | 39 +++++++++---------- 2 files changed, 40 insertions(+), 24 deletions(-) diff --git a/.github/scripts/coderabbit_semantic_review.test.js b/.github/scripts/coderabbit_semantic_review.test.js index f9d4f1aa8081..c1b47bdf8d32 100644 --- a/.github/scripts/coderabbit_semantic_review.test.js +++ b/.github/scripts/coderabbit_semantic_review.test.js @@ -30,10 +30,12 @@ const HEAD = 'a'.repeat(40); const BASE = 'b'.repeat(40); const NEW_BASE = 'c'.repeat(40); +/** Run the actual workflow script against an in-memory GitHub API. */ function harness(overrides = {}) { const pr = {number: 12, state: 'open', draft: false, head: {sha: HEAD}, base: {ref: 'main', sha: 'outdated-event-sha'}, labels: [{name: 'ai: semantic-conflict'}], ...overrides}; + const prs = [pr]; const comments = []; const posted = []; let target = BASE; @@ -42,7 +44,10 @@ function harness(overrides = {}) { rest: { pulls: { list: 'list-pulls', - get: async args => {calls.push(['get-pr', args]); return {data: pr};}, + get: async args => { + calls.push(['get-pr', args]); + return {data: prs.find(p => p.number === args.pull_number)}; + }, }, git: {getRef: async args => { calls.push(['get-ref', args]); @@ -53,13 +58,13 @@ function harness(overrides = {}) { createComment: async args => { posted.push(args); comments.push({body: args.body, user: {login: 'github-actions[bot]'}}); - return {data: {html_url: `https://github.com/example/repo/pull/12#${posted.length}`}}; + return {data: {html_url: `https://github.com/example/repo/pull/${args.issue_number}#${posted.length}`}}; }, }, }, paginate: async (method, args) => { calls.push([method, args]); - if (method === 'list-pulls') return [pr]; + if (method === 'list-pulls') return prs; assert.equal(method, 'list-comments'); return comments; }, @@ -71,7 +76,7 @@ function harness(overrides = {}) { }}; const context = {repo: {owner: 'example', repo: 'repo'}, eventName: 'pull_request_target', payload: {pull_request: {number: 12}}}; - return {pr, comments, posted, calls, summaries, + return {pr, prs, comments, posted, calls, summaries, setTarget: sha => {target = sha;}, run: (eventName = 'pull_request_target', pullNumber = '') => execute(github, {...context, eventName}, core, {env: {DISPATCH_PULL_NUMBER: pullNumber}}), @@ -147,3 +152,15 @@ test('invalid dispatch inputs and unsupported events cannot post comments', asyn } await assert.rejects(harness().run('issue_comment'), /Unsupported event/); }); + +test('a coalesced event sweeps all opted-in PRs; manual retry is limited to its PR', async () => { + const h = harness(); + h.prs.push({...h.pr, number: 13, head: {sha: 'd'.repeat(40)}}); + await h.run(); + assert.deepEqual(h.posted.map(c => c.issue_number), [12, 13]); + h.setTarget(NEW_BASE); + await h.run(); + assert.deepEqual(h.posted.map(c => c.issue_number), [12, 13, 12, 13]); + await h.run('workflow_dispatch', '13'); + assert.deepEqual(h.posted.map(c => c.issue_number), [12, 13, 12, 13, 13]); +}); diff --git a/.github/workflows/coderabbit-semantic-review.yml b/.github/workflows/coderabbit-semantic-review.yml index b4a41e2217fc..533d1b5e6af3 100644 --- a/.github/workflows/coderabbit-semantic-review.yml +++ b/.github/workflows/coderabbit-semantic-review.yml @@ -33,15 +33,18 @@ permissions: pull-requests: read issues: write -# Coalesce repeated events per PR or main; each run reads current refs. -concurrency: - group: coderabbit-semantic-conflict-${{ github.event.pull_request.number || inputs.pull_number || 'main' }} - cancel-in-progress: false - jobs: request-review: name: Request advisory semantic review - if: github.repository == 'NVIDIA/TensorRT-LLM' + if: >- + github.repository == 'NVIDIA/TensorRT-LLM' && + (github.event_name != 'pull_request_target' || + contains(github.event.pull_request.labels.*.name, 'ai: semantic-conflict')) + # Serialize eligible jobs, so skipped PR events cannot replace queued work. + # Every run sweeps opted-in PRs so coalesced pending events are not lost. + concurrency: + group: coderabbit-semantic-conflict-requests + cancel-in-progress: false runs-on: ubuntu-latest timeout-minutes: 10 steps: @@ -57,25 +60,21 @@ jobs: const pullNumber = process.env.DISPATCH_PULL_NUMBER; const repo = context.repo; - let numbers; + let retryNumber; if (context.eventName === 'workflow_dispatch') { const raw = (pullNumber || '').trim(); - const number = Number(raw); - if (!/^[1-9][0-9]*$/.test(raw) || !Number.isSafeInteger(number)) { + retryNumber = Number(raw); + if (!/^[1-9][0-9]*$/.test(raw) || !Number.isSafeInteger(retryNumber)) { throw new Error('pull_number must be a positive integer'); } - numbers = [number]; - } else if (context.eventName === 'push') { - const pulls = await github.paginate(github.rest.pulls.list, { - ...repo, state: 'open', base: 'main', per_page: 100, - }); - numbers = pulls.filter(pr => !pr.draft && pr.labels.some(l => l.name === LABEL)) - .map(pr => pr.number); - } else if (context.eventName === 'pull_request_target') { - numbers = [context.payload.pull_request.number]; - } else { + } else if (!['push', 'pull_request_target'].includes(context.eventName)) { throw new Error(`Unsupported event: ${context.eventName}`); } + const pulls = await github.paginate(github.rest.pulls.list, { + ...repo, state: 'open', base: 'main', per_page: 100, + }); + const numbers = pulls.filter(pr => !pr.draft && pr.labels.some(l => l.name === LABEL)) + .map(pr => pr.number); for (const number of numbers) { // Read current metadata instead of trusting a possibly queued event payload. @@ -94,7 +93,7 @@ jobs: }); // Only our own bot's comments can suppress an automatic request. A manual // dispatch always retries, including after a CodeRabbit timeout or failure. - if (context.eventName !== 'workflow_dispatch' && comments.some(comment => + if (number !== retryNumber && comments.some(comment => comment.user?.login === BOT && comment.body?.includes(marker))) { core.info(`PR #${number}: this SHA pair has already been requested.`); continue; From 6f370ae7f5b7d05b4f06070ba58e4dfde861e0da Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Wed, 16 Sep 2026 17:32:10 +0800 Subject: [PATCH 03/21] [None][infra] Report unavailable semantic review retries Signed-off-by: Yanchao Lu --- .github/scripts/coderabbit_semantic_review.test.js | 8 +++++++- .github/workflows/coderabbit-semantic-review.yml | 5 +++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/scripts/coderabbit_semantic_review.test.js b/.github/scripts/coderabbit_semantic_review.test.js index c1b47bdf8d32..bf33165a2efb 100644 --- a/.github/scripts/coderabbit_semantic_review.test.js +++ b/.github/scripts/coderabbit_semantic_review.test.js @@ -138,7 +138,7 @@ test('closed, draft, unlabelled, and release PRs are not reviewed', async () => {base: {ref: 'release/1.0'}}]) { const h = harness(overrides); await h.run(); - await h.run('workflow_dispatch', '12'); + await assert.rejects(h.run('workflow_dispatch', '12'), /retry not posted/); assert.equal(h.posted.length, 0); assert.ok(!h.calls.some(c => c[0] === 'get-ref')); } @@ -153,6 +153,12 @@ test('invalid dispatch inputs and unsupported events cannot post comments', asyn await assert.rejects(harness().run('issue_comment'), /Unsupported event/); }); +test('a nonexistent manual retry fails even if another PR received a request', async () => { + const h = harness(); + await assert.rejects(h.run('workflow_dispatch', '99'), /PR #99: retry not posted/); + assert.deepEqual(h.posted.map(c => c.issue_number), [12]); +}); + test('a coalesced event sweeps all opted-in PRs; manual retry is limited to its PR', async () => { const h = harness(); h.prs.push({...h.pr, number: 13, head: {sha: 'd'.repeat(40)}}); diff --git a/.github/workflows/coderabbit-semantic-review.yml b/.github/workflows/coderabbit-semantic-review.yml index 533d1b5e6af3..e52ba54901c1 100644 --- a/.github/workflows/coderabbit-semantic-review.yml +++ b/.github/workflows/coderabbit-semantic-review.yml @@ -75,6 +75,7 @@ jobs: }); const numbers = pulls.filter(pr => !pr.draft && pr.labels.some(l => l.name === LABEL)) .map(pr => pr.number); + let retryPosted = false; for (const number of numbers) { // Read current metadata instead of trusting a possibly queued event payload. @@ -113,9 +114,13 @@ jobs: const {data: comment} = await github.rest.issues.createComment({ ...repo, issue_number: number, body, }); + if (number === retryNumber) retryPosted = true; core.info(`PR #${number}: requested ${head} + ${base}: ${comment.html_url}`); await core.summary.addRaw( `PR #${number}: [requested CodeRabbit analysis](${comment.html_url}) for ` + `head \`${head}\` + main \`${base}\`. No AI verdict is asserted.\n\n` ).write(); } + if (retryNumber && !retryPosted) { + throw new Error(`PR #${retryNumber}: retry not posted; requires an open, non-draft main PR with the ${LABEL} label.`); + } From d798ce5e523c20024b1548e0f93820bf44038a8b Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Wed, 16 Sep 2026 21:08:54 +0800 Subject: [PATCH 04/21] [None][infra] Publish advisory semantic checks separately Signed-off-by: Yanchao Lu --- .coderabbit.yaml | 7 + .../coderabbit_semantic_review.test.js | 159 +++++++++++++++++- .../coderabbit-semantic-review-tests.yml | 42 +++++ .../workflows/coderabbit-semantic-review.yml | 128 +++++++++++++- .github/workflows/precommit-check.yml | 3 - AGENTS.md | 23 ++- 6 files changed, 341 insertions(+), 21 deletions(-) create mode 100644 .github/workflows/coderabbit-semantic-review-tests.yml diff --git a/.coderabbit.yaml b/.coderabbit.yaml index fcb0f643e119..b3da90eae55b 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -112,6 +112,13 @@ reviews: State that the result applies only to the reported SHA pair and must be rerun after either branch changes. + Begin the explanation with exactly one machine-readable result line: + SEMANTIC_RESULT head= target= merge_base= verdict= + Replace each SHA with the inspected full lowercase 40-character SHA; + VERDICT must be PASS, FAIL, or INCONCLUSIVE. Do not emit this line if + any SHA cannot be verified; explain the missing evidence instead. + This line is used to publish the advisory GitHub Check for this pair. + path_filters: # Vendored/adapted FlashInfer kernels; excluded from review. - "!tensorrt_llm/_torch/attention/backends/prims_ts/**" diff --git a/.github/scripts/coderabbit_semantic_review.test.js b/.github/scripts/coderabbit_semantic_review.test.js index bf33165a2efb..e2acec8b9204 100644 --- a/.github/scripts/coderabbit_semantic_review.test.js +++ b/.github/scripts/coderabbit_semantic_review.test.js @@ -19,16 +19,17 @@ const fs = require('node:fs'); const path = require('node:path'); const workflow = fs.readFileSync( path.join(__dirname, '../workflows/coderabbit-semantic-review.yml'), 'utf8'); -const marker = ' script: |\n'; -assert.ok(workflow.includes(marker)); -const script = workflow.slice(workflow.indexOf(marker) + marker.length) - .split('\n').map(line => line.replace(/^ {12}/, '')).join('\n'); +const scripts = [...workflow.matchAll(/^ {10}script: \|\n((?: {12}[^\n]*(?:\n|$)|\n)*)/gm)] + .map(match => match[1].split('\n').map(line => line.replace(/^ {12}/, '')).join('\n')); +assert.equal(scripts.length, 2); const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor; -const execute = new AsyncFunction('github', 'context', 'core', 'process', script); +const execute = new AsyncFunction('github', 'context', 'core', 'process', scripts[0]); +const publish = new AsyncFunction('github', 'context', 'core', scripts[1]); const HEAD = 'a'.repeat(40); const BASE = 'b'.repeat(40); const NEW_BASE = 'c'.repeat(40); +const MERGE_BASE = 'e'.repeat(40); /** Run the actual workflow script against an in-memory GitHub API. */ function harness(overrides = {}) { @@ -38,10 +39,12 @@ function harness(overrides = {}) { const prs = [pr]; const comments = []; const posted = []; + const checks = []; let target = BASE; const calls = []; const github = { rest: { + checks: {create: async args => {checks.push(args);}}, pulls: { list: 'list-pulls', get: async args => { @@ -76,7 +79,7 @@ function harness(overrides = {}) { }}; const context = {repo: {owner: 'example', repo: 'repo'}, eventName: 'pull_request_target', payload: {pull_request: {number: 12}}}; - return {pr, prs, comments, posted, calls, summaries, + return {pr, prs, comments, posted, checks, calls, summaries, setTarget: sha => {target = sha;}, run: (eventName = 'pull_request_target', pullNumber = '') => execute(github, {...context, eventName}, core, {env: {DISPATCH_PULL_NUMBER: pullNumber}}), @@ -93,6 +96,9 @@ test('requests the live main SHA and never treats dispatch as an AI verdict', as assert.ok(!h.posted[0].body.includes('outdated-event-sha')); assert.match(h.posted[0].body, /return Inconclusive/); assert.match(h.summaries[0], /No AI verdict is asserted/); + assert.equal(h.checks[0].conclusion, 'neutral'); + assert.equal(h.checks[0].head_sha, HEAD); + assert.match(h.checks[0].output.summary, /No AI verdict yet/); assert.deepEqual(h.calls.find(c => c[0] === 'get-ref')[1], {owner: 'example', repo: 'repo', ref: 'heads/main'}); }); @@ -153,10 +159,11 @@ test('invalid dispatch inputs and unsupported events cannot post comments', asyn await assert.rejects(harness().run('issue_comment'), /Unsupported event/); }); -test('a nonexistent manual retry fails even if another PR received a request', async () => { +test('a nonexistent manual retry cannot affect another PR', async () => { const h = harness(); await assert.rejects(h.run('workflow_dispatch', '99'), /PR #99: retry not posted/); - assert.deepEqual(h.posted.map(c => c.issue_number), [12]); + assert.deepEqual(h.posted, []); + assert.deepEqual(h.checks, []); }); test('a coalesced event sweeps all opted-in PRs; manual retry is limited to its PR', async () => { @@ -170,3 +177,139 @@ test('a coalesced event sweeps all opted-in PRs; manual retry is limited to its await h.run('workflow_dispatch', '13'); assert.deepEqual(h.posted.map(c => c.issue_number), [12, 13, 12, 13, 13]); }); + +test('a manual retry cannot request an unrelated PR without an existing marker', async () => { + const h = harness(); + h.prs.push({...h.pr, number: 13}); + await h.run('workflow_dispatch', '13'); + assert.deepEqual(h.posted.map(comment => comment.issue_number), [13]); +}); + +/** Build the observed CodeRabbit result format with a verifiable revision record. */ +function resultBody(verdict = 'FAIL', head = HEAD, target = BASE) { + const status = {PASS: '✅ Passed', FAIL: '⚠️ Warning', INCONCLUSIVE: '❓ Inconclusive'}[verdict]; + return '\n' + + `| Semantic Conflict With Target Branch | ${status} | Explanation preview |\n` + + '
\nFull details: Semantic Conflict With Target Branch\n' + + `SEMANTIC_RESULT head=${head} target=${target} merge_base=${MERGE_BASE} verdict=${verdict}\n` + + 'Evidence and a minimal regression input.\n
'; +} + +/** Exercise the result publisher without checking out or executing PR code. */ +function resultHarness(body = resultBody()) { + const request = harness(); + const comment = {body, user: {login: 'coderabbitai[bot]', type: 'Bot'}, + html_url: 'https://github.com/example/repo/pull/12#issuecomment-123'}; + const checks = [{id: 1, app: {slug: 'github-actions'}, + external_id: `semantic-conflict:12:${HEAD}:${BASE}`}]; + const updated = []; + const warnings = []; + let mergeBase = MERGE_BASE; + let target = BASE; + let afterCompare = () => {}; + const github = { + rest: { + issues: {getComment: async () => ({data: comment})}, + pulls: {get: async () => ({data: request.pr})}, + git: {getRef: async () => ({data: {object: {sha: target}}})}, + checks: {listForRef: 'list-checks', update: async args => {updated.push(args);}}, + }, + paginate: async () => checks, + request: async (route, args) => { + assert.equal(route, 'GET /repos/{owner}/{repo}/compare/{basehead}'); + assert.equal(args.basehead, `${BASE}...${HEAD}`); + afterCompare(); + return {data: {merge_base_commit: {sha: mergeBase}}}; + }, + }; + const core = {warning: text => warnings.push(text), + summary: {addRaw() {return this;}, async write() {}}}; + return {comment, checks, updated, warnings, pr: request.pr, + setMergeBase: sha => {mergeBase = sha;}, + setTarget: sha => {target = sha;}, + afterCompare: callback => {afterCompare = callback;}, + run: () => publish(github, {repo: {owner: 'example', repo: 'repo'}, + payload: {issue: {number: 12}, comment: {id: 123}}}, core), + }; +} + +test('verified conflicts and inconclusive results publish neutral checks and annotations', async () => { + for (const verdict of ['FAIL', 'INCONCLUSIVE']) { + const h = resultHarness(resultBody(verdict)); + await h.run(); + assert.equal(h.updated[0].conclusion, 'neutral'); + assert.match(h.updated[0].output.title, /⚠️/); + assert.equal(h.updated[0].details_url, h.comment.html_url); + assert.equal(h.warnings.length, 1); + } +}); + +test('only a verified pass publishes success, including a passed table without full details', async () => { + const body = resultBody('PASS'); + const record = body.match(/SEMANTIC_RESULT[^\n]+/)[0]; + for (const text of [body, '\n' + + `| Semantic conflict with target branch | ✅ Passed | ${record} |`]) { + const h = resultHarness(text); + await h.run(); + assert.equal(h.updated[0].conclusion, 'success'); + assert.equal(h.warnings.length, 0); + } +}); + +test('stale head or target results never overwrite the current check', async () => { + for (const body of [resultBody('PASS', NEW_BASE), resultBody('PASS', HEAD, NEW_BASE)]) { + const h = resultHarness(body); + await h.run(); + assert.deepEqual(h.updated, []); + } +}); + +test('missing, contradictory, or invalid evidence cannot publish a pass', async () => { + for (const body of [ + resultBody('PASS').replace(/SEMANTIC_RESULT[^\n]+/, 'Clone failed'), + resultBody('PASS').replace('✅ Passed', '❓ Inconclusive'), + resultBody('PASS').replace('', resultBody('FAIL') + ''), + ]) { + const h = resultHarness(body); + await h.run(); + assert.equal(h.updated[0].conclusion, 'neutral'); + } + const h = resultHarness(resultBody('PASS')); + h.setMergeBase(NEW_BASE); + await h.run(); + assert.equal(h.updated[0].conclusion, 'neutral'); +}); + +test('untrusted authors, unrelated reviews, and unrequested fixtures cannot publish results', async () => { + for (const change of [ + h => {h.comment.user.login = 'someone-else';}, + h => {h.comment.user.type = 'User';}, + h => {h.comment.body = h.comment.body.replace('', '');}, + h => {h.comment.body = h.comment.body.replaceAll('Semantic Conflict With Target Branch', 'Controlled Experiment');}, + h => {h.checks.length = 0;}, + h => {h.checks[0].app.slug = 'another-app';}, + h => {h.pr.draft = true;}, + h => {h.pr.labels = [];}, + ]) { + const h = resultHarness(); + change(h); + await h.run(); + assert.deepEqual(h.updated, []); + } +}); + +test('head or main updates during analysis cannot publish a stale pass', async () => { + for (const change of [h => {h.pr.head.sha = NEW_BASE;}, h => h.setTarget(NEW_BASE)]) { + const h = resultHarness(resultBody('PASS')); + h.afterCompare(() => change(h)); + await h.run(); + assert.deepEqual(h.updated, []); + } +}); + +test('a retry updates only the newest matching check', async () => { + const h = resultHarness(); + h.checks.push({...h.checks[0], id: 2}); + await h.run(); + assert.equal(h.updated[0].check_run_id, 2); +}); diff --git a/.github/workflows/coderabbit-semantic-review-tests.yml b/.github/workflows/coderabbit-semantic-review-tests.yml new file mode 100644 index 000000000000..be616d93f4e4 --- /dev/null +++ b/.github/workflows/coderabbit-semantic-review-tests.yml @@ -0,0 +1,42 @@ +# 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: CodeRabbit Semantic Review Tests + +on: + pull_request: + paths: + - '.github/workflows/coderabbit-semantic-review*.yml' + - '.github/scripts/coderabbit_semantic_review*.js' + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + test: + name: Semantic review workflow tests + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: false + - name: Test semantic review automation + run: node --test .github/scripts/coderabbit_semantic_review.test.js diff --git a/.github/workflows/coderabbit-semantic-review.yml b/.github/workflows/coderabbit-semantic-review.yml index e52ba54901c1..b8ed4ef4674d 100644 --- a/.github/workflows/coderabbit-semantic-review.yml +++ b/.github/workflows/coderabbit-semantic-review.yml @@ -21,6 +21,8 @@ on: types: [opened, reopened, synchronize, ready_for_review, labeled, edited, auto_merge_enabled] push: branches: [main] + issue_comment: + types: [created, edited] workflow_dispatch: inputs: pull_number: @@ -30,20 +32,24 @@ on: permissions: contents: read - pull-requests: read - issues: write jobs: request-review: name: Request advisory semantic review + permissions: + contents: read + pull-requests: read + issues: write + checks: write if: >- github.repository == 'NVIDIA/TensorRT-LLM' && + github.event_name != 'issue_comment' && (github.event_name != 'pull_request_target' || contains(github.event.pull_request.labels.*.name, 'ai: semantic-conflict')) - # Serialize eligible jobs, so skipped PR events cannot replace queued work. - # Every run sweeps opted-in PRs so coalesced pending events are not lost. + # Automatic runs sweep opted-in PRs so coalesced events are not lost. + # Manual retries have their own group and cannot replace a pending sweep. concurrency: - group: coderabbit-semantic-conflict-requests + group: ${{ github.event_name == 'workflow_dispatch' && format('coderabbit-semantic-retry-{0}', inputs.pull_number) || 'coderabbit-semantic-conflict-requests' }} cancel-in-progress: false runs-on: ubuntu-latest timeout-minutes: 10 @@ -74,7 +80,7 @@ jobs: ...repo, state: 'open', base: 'main', per_page: 100, }); const numbers = pulls.filter(pr => !pr.draft && pr.labels.some(l => l.name === LABEL)) - .map(pr => pr.number); + .map(pr => pr.number).filter(number => !retryNumber || number === retryNumber); let retryPosted = false; for (const number of numbers) { @@ -99,6 +105,21 @@ jobs: core.info(`PR #${number}: this SHA pair has already been requested.`); continue; } + // A posted request is not a semantic pass. Keep a neutral check + // until a matching, verified CodeRabbit result arrives. + await github.rest.checks.create({ + ...repo, + name: 'Semantic conflict with target branch', + head_sha: head, + external_id: `semantic-conflict:${number}:${head}:${base}`, + status: 'completed', + conclusion: 'neutral', + output: { + title: 'Awaiting CodeRabbit analysis', + summary: `No AI verdict yet for PR #${number}, head ${head} + main ${base}. ` + + 'This advisory check does not block merging.', + }, + }); const body = [ '@coderabbitai run pre-merge checks', '', @@ -124,3 +145,98 @@ jobs: if (retryNumber && !retryPosted) { throw new Error(`PR #${retryNumber}: retry not posted; requires an open, non-draft main PR with the ${LABEL} label.`); } + + publish-result: + name: Publish advisory semantic result + permissions: + contents: read + pull-requests: read + issues: read + checks: write + if: >- + github.repository == 'NVIDIA/TensorRT-LLM' && + github.event_name == 'issue_comment' && + github.event.issue.pull_request && + github.event.comment.user.login == 'coderabbitai[bot]' && + contains(github.event.comment.body, 'Semantic conflict with target branch') + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + # Read comments as data. Never checkout or execute PR code with this token. + - name: Publish a result only for the current requested revision pair + uses: actions/github-script@v8 + with: + script: | + const name = 'Semantic conflict with target branch'; + const repo = context.repo; + const number = context.payload.issue.number; + const {data: comment} = await github.rest.issues.getComment({ + ...repo, comment_id: context.payload.comment.id, + }); + const body = comment.body || ''; + if (comment.user?.login !== 'coderabbitai[bot]' || comment.user?.type !== 'Bot') return; + if (!body.includes('') && + !body.includes('')) return; + const row = body.split('\n').map(line => line.split('|').map(cell => cell.trim())) + .find(cells => cells[1]?.toLowerCase() === name.toLowerCase()); + if (!row) return; + const details = body.match(/Full details: Semantic conflict with target branch<\/summary>([\s\S]*?)<\/details>/i); + const result = details ? details[1] : row.join('|'); + + const {data: pr} = await github.rest.pulls.get({...repo, pull_number: number}); + if (pr.state !== 'open' || pr.draft || pr.base.ref !== 'main' || + !pr.labels.some(label => label.name === 'ai: semantic-conflict')) return; + const {data: ref} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); + const head = pr.head.sha; + const base = ref.object.sha; + const checks = await github.paginate(github.rest.checks.listForRef, { + ...repo, ref: head, check_name: name, filter: 'all', per_page: 100, + }); + const check = checks.filter(check => check.app?.slug === 'github-actions' && + check.external_id === `semantic-conflict:${number}:${head}:${base}`) + .sort((a, b) => b.id - a.id)[0]; + if (!check) return; // Includes unrelated manual fixture evaluations. + + const pattern = /SEMANTIC_RESULT head=([a-f0-9]{40}) target=([a-f0-9]{40}) merge_base=([a-f0-9]{40}) verdict=(PASS|FAIL|INCONCLUSIVE)\b/g; + const records = [...new Set([...result.matchAll(pattern)].map(match => match[0]))]; + let verdict = 'INCONCLUSIVE'; + let reason = 'The result has no unique, verifiable revision record.'; + if (records.length === 1) { + const [, reportedHead, reportedBase, mergeBase, reportedVerdict] = + [...records[0].matchAll(pattern)][0]; + // A stale comment must never overwrite the current revision's result. + if (reportedHead !== head || reportedBase !== base) return; + const {data: comparison} = await github.request( + 'GET /repos/{owner}/{repo}/compare/{basehead}', + {...repo, basehead: `${base}...${head}`}); + const expectedStatus = {PASS: /Passed/i, FAIL: /Warning|Error/i, INCONCLUSIVE: /Inconclusive/i}; + if (comparison.merge_base_commit?.sha === mergeBase && expectedStatus[reportedVerdict].test(row[2])) { + verdict = reportedVerdict; + reason = `Verified head ${head}, target ${base}, merge base ${mergeBase}.`; + } else { + reason = 'The reported verdict or merge base could not be verified.'; + } + } + // Recheck refs after fetching the result and history. + const {data: current} = await github.rest.pulls.get({...repo, pull_number: number}); + const {data: currentRef} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); + if (current.head.sha !== head || currentRef.object.sha !== base || + current.state !== 'open' || current.draft || current.base.ref !== 'main' || + !current.labels.some(label => label.name === 'ai: semantic-conflict')) return; + const titles = { + PASS: 'No semantic conflict found (best effort)', + FAIL: '⚠️ Possible semantic conflict', + INCONCLUSIVE: '⚠️ Semantic analysis inconclusive', + }; + await github.rest.checks.update({ + ...repo, check_run_id: check.id, status: 'completed', + conclusion: verdict === 'PASS' ? 'success' : 'neutral', + details_url: comment.html_url, + output: { + title: titles[verdict], + summary: `${reason}\n\n[CodeRabbit analysis](${comment.html_url})\n\n` + + 'Advisory only; applies to this SHA pair and does not prove compatibility.', + }, + }); + if (verdict !== 'PASS') core.warning(`${titles[verdict]}: ${comment.html_url}`); + await core.summary.addRaw(`${titles[verdict]}: [CodeRabbit analysis](${comment.html_url})\n`).write(); diff --git a/.github/workflows/precommit-check.yml b/.github/workflows/precommit-check.yml index 4931583e9bf7..3e68e6df598d 100644 --- a/.github/workflows/precommit-check.yml +++ b/.github/workflows/precommit-check.yml @@ -36,9 +36,6 @@ jobs: - name: Test stale pull request cleanup workflow run: node .github/scripts/cleanup_stale_prs.test.js - - name: Test semantic conflict review requests - run: node --test .github/scripts/coderabbit_semantic_review.test.js - - uses: actions/setup-python@v6 with: python-version: '3.12' diff --git a/AGENTS.md b/AGENTS.md index 9d0183129862..a1ad31cb4f0c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -184,10 +184,25 @@ best-effort and warning-only; missing revision/history evidence is Inconclusive. For a non-draft PR targeting `main`, maintainers can add the `ai: semantic-conflict` label to opt into automatic rechecks on PR and main -updates. The `CodeRabbit Semantic Conflict Review` workflow only posts requests; -its success is not an AI verdict. Read CodeRabbit's result and verify its head -and target SHAs still match the live branches. Use workflow dispatch with the -PR number to retry a request, or comment `@coderabbitai run pre-merge checks`. +updates. The `CodeRabbit Semantic Conflict Review` workflow requests analysis +and publishes results; its request job succeeding is not an AI verdict. The independent +`Semantic conflict with target branch` GitHub Check starts neutral while +awaiting analysis. The workflow publishes CodeRabbit's result only after +verifying the bot author, requested head/target pair, and merge-base SHA. +PASS becomes success; conflicts and Inconclusive results stay neutral with +a warning title and a link to the analysis. GitHub has no warning conclusion; +the publishing job emits a warning annotation but can still succeed. +Read the result and verify its SHAs still match the live branches. + +Use workflow dispatch with the PR number to retry only that PR, or comment +`@coderabbitai run pre-merge checks`. Automatic events sweep all opted-in PRs. +Only requested revision pairs receive published results; unrelated manual +fixture experiments do not change a PR's semantic Check. Missing results stay +neutral, and stale results cannot mark a newer pair as passing. + +`CodeRabbit Semantic Review Tests` is a separate, read-only workflow for the +automation's unit tests; passing it is not a semantic verdict. These tests +are not part of the existing `Pre-commit Check`. The custom check requires CodeRabbit Custom Pre-Merge Checks access. This advisory pilot does not provide a merge-queue check or block merges. From 0c8ad406f0036f2e19223b04ef9a6bfcbc447309 Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Wed, 16 Sep 2026 21:12:32 +0800 Subject: [PATCH 05/21] [None][infra] Accept normalized CodeRabbit result casing Signed-off-by: Yanchao Lu --- .github/scripts/coderabbit_semantic_review.test.js | 5 +++-- .github/workflows/coderabbit-semantic-review.yml | 7 ++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/.github/scripts/coderabbit_semantic_review.test.js b/.github/scripts/coderabbit_semantic_review.test.js index e2acec8b9204..d8a1674e5686 100644 --- a/.github/scripts/coderabbit_semantic_review.test.js +++ b/.github/scripts/coderabbit_semantic_review.test.js @@ -234,11 +234,12 @@ function resultHarness(body = resultBody()) { } test('verified conflicts and inconclusive results publish neutral checks and annotations', async () => { - for (const verdict of ['FAIL', 'INCONCLUSIVE']) { - const h = resultHarness(resultBody(verdict)); + for (const body of [resultBody('FAIL'), resultBody('INCONCLUSIVE'), resultBody('FAIL').toLowerCase()]) { + const h = resultHarness(body); await h.run(); assert.equal(h.updated[0].conclusion, 'neutral'); assert.match(h.updated[0].output.title, /⚠️/); + assert.match(h.updated[0].output.title, body.includes('INCONCLUSIVE') ? /inconclusive/ : /Possible semantic conflict/); assert.equal(h.updated[0].details_url, h.comment.html_url); assert.equal(h.warnings.length, 1); } diff --git a/.github/workflows/coderabbit-semantic-review.yml b/.github/workflows/coderabbit-semantic-review.yml index b8ed4ef4674d..ddda50427648 100644 --- a/.github/workflows/coderabbit-semantic-review.yml +++ b/.github/workflows/coderabbit-semantic-review.yml @@ -197,13 +197,14 @@ jobs: .sort((a, b) => b.id - a.id)[0]; if (!check) return; // Includes unrelated manual fixture evaluations. - const pattern = /SEMANTIC_RESULT head=([a-f0-9]{40}) target=([a-f0-9]{40}) merge_base=([a-f0-9]{40}) verdict=(PASS|FAIL|INCONCLUSIVE)\b/g; - const records = [...new Set([...result.matchAll(pattern)].map(match => match[0]))]; + const pattern = /semantic_result head=([a-f0-9]{40}) target=([a-f0-9]{40}) merge_base=([a-f0-9]{40}) verdict=(pass|fail|inconclusive)\b/g; + const records = [...new Set([...result.toLowerCase().matchAll(pattern)].map(match => match[0]))]; let verdict = 'INCONCLUSIVE'; let reason = 'The result has no unique, verifiable revision record.'; if (records.length === 1) { - const [, reportedHead, reportedBase, mergeBase, reportedVerdict] = + const [, reportedHead, reportedBase, mergeBase, rawVerdict] = [...records[0].matchAll(pattern)][0]; + const reportedVerdict = rawVerdict.toUpperCase(); // A stale comment must never overwrite the current revision's result. if (reportedHead !== head || reportedBase !== base) return; const {data: comparison} = await github.request( From 8b1b56e6d3320c113dc092281e121723036d8caf Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Wed, 16 Sep 2026 22:00:00 +0800 Subject: [PATCH 06/21] [None][infra] Fail advisory semantic conflicts and preview PR results Signed-off-by: Yanchao Lu --- .coderabbit.yaml | 3 + .../coderabbit_semantic_review.test.js | 94 ++++++++++++--- .../workflows/coderabbit-semantic-review.yml | 113 +++++++++++++----- AGENTS.md | 25 +++- 4 files changed, 187 insertions(+), 48 deletions(-) diff --git a/.coderabbit.yaml b/.coderabbit.yaml index b3da90eae55b..34e7d853c7a5 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -111,6 +111,9 @@ reviews: If either branch changes during analysis, return Inconclusive. State that the result applies only to the reported SHA pair and must be rerun after either branch changes. + Explain that CodeRabbit can make mistakes, including false positives. + This is an advisory check: a failure does not block merging while the + check remains non-required. Other merge requirements still apply. Begin the explanation with exactly one machine-readable result line: SEMANTIC_RESULT head= target= merge_base= verdict= diff --git a/.github/scripts/coderabbit_semantic_review.test.js b/.github/scripts/coderabbit_semantic_review.test.js index d8a1674e5686..7456a5ec474d 100644 --- a/.github/scripts/coderabbit_semantic_review.test.js +++ b/.github/scripts/coderabbit_semantic_review.test.js @@ -198,23 +198,26 @@ function resultBody(verdict = 'FAIL', head = HEAD, target = BASE) { /** Exercise the result publisher without checking out or executing PR code. */ function resultHarness(body = resultBody()) { const request = harness(); - const comment = {body, user: {login: 'coderabbitai[bot]', type: 'Bot'}, + const comment = {id: 123, body, user: {login: 'coderabbitai[bot]', type: 'Bot'}, html_url: 'https://github.com/example/repo/pull/12#issuecomment-123'}; + const comments = [comment]; const checks = [{id: 1, app: {slug: 'github-actions'}, external_id: `semantic-conflict:12:${HEAD}:${BASE}`}]; const updated = []; const warnings = []; + const failures = []; + const summaries = []; let mergeBase = MERGE_BASE; let target = BASE; let afterCompare = () => {}; const github = { rest: { - issues: {getComment: async () => ({data: comment})}, + issues: {getComment: async () => ({data: comment}), listComments: 'list-comments'}, pulls: {get: async () => ({data: request.pr})}, git: {getRef: async () => ({data: {object: {sha: target}}})}, checks: {listForRef: 'list-checks', update: async args => {updated.push(args);}}, }, - paginate: async () => checks, + paginate: async method => method === 'list-comments' ? comments : checks, request: async (route, args) => { assert.equal(route, 'GET /repos/{owner}/{repo}/compare/{basehead}'); assert.equal(args.basehead, `${BASE}...${HEAD}`); @@ -222,29 +225,42 @@ function resultHarness(body = resultBody()) { return {data: {merge_base_commit: {sha: mergeBase}}}; }, }; - const core = {warning: text => warnings.push(text), - summary: {addRaw() {return this;}, async write() {}}}; - return {comment, checks, updated, warnings, pr: request.pr, + const core = {warning: text => warnings.push(text), setFailed: text => failures.push(text), + summary: {addRaw(text) {summaries.push(text); return this;}, async write() {}}}; + return {comment, comments, checks, updated, warnings, failures, summaries, pr: request.pr, setMergeBase: sha => {mergeBase = sha;}, setTarget: sha => {target = sha;}, afterCompare: callback => {afterCompare = callback;}, - run: () => publish(github, {repo: {owner: 'example', repo: 'repo'}, - payload: {issue: {number: 12}, comment: {id: 123}}}, core), + run: (eventName = 'issue_comment') => publish(github, {repo: {owner: 'example', repo: 'repo'}, + eventName, payload: {issue: {number: 12}, comment: {id: 123}, + pull_request: {number: 12, head: {sha: HEAD}}}}, core), }; } -test('verified conflicts and inconclusive results publish neutral checks and annotations', async () => { - for (const body of [resultBody('FAIL'), resultBody('INCONCLUSIVE'), resultBody('FAIL').toLowerCase()]) { +test('verified conflicts publish failure with a false-positive and non-blocking notice', async () => { + for (const body of [resultBody('FAIL'), resultBody('FAIL').toLowerCase()]) { const h = resultHarness(body); await h.run(); - assert.equal(h.updated[0].conclusion, 'neutral'); - assert.match(h.updated[0].output.title, /⚠️/); - assert.match(h.updated[0].output.title, body.includes('INCONCLUSIVE') ? /inconclusive/ : /Possible semantic conflict/); + assert.equal(h.updated[0].conclusion, 'failure'); + assert.match(h.updated[0].output.title, /Possible semantic conflict/); + assert.match(h.updated[0].output.summary, /CodeRabbit can make mistakes, including false positives/); + assert.match(h.updated[0].output.summary, /must remain non-required; its failure does not block merging/); assert.equal(h.updated[0].details_url, h.comment.html_url); - assert.equal(h.warnings.length, 1); + assert.equal(h.failures.length, 1); + assert.match(h.failures[0], /does not block merging/); + assert.match(h.summaries[0], /false positives/); } }); +test('inconclusive results stay neutral and warn without failing the job', async () => { + const h = resultHarness(resultBody('INCONCLUSIVE')); + await h.run(); + assert.equal(h.updated[0].conclusion, 'neutral'); + assert.match(h.updated[0].output.title, /inconclusive/); + assert.equal(h.warnings.length, 1); + assert.equal(h.failures.length, 0); +}); + test('only a verified pass publishes success, including a passed table without full details', async () => { const body = resultBody('PASS'); const record = body.match(/SEMANTIC_RESULT[^\n]+/)[0]; @@ -254,6 +270,7 @@ test('only a verified pass publishes success, including a passed table without f await h.run(); assert.equal(h.updated[0].conclusion, 'success'); assert.equal(h.warnings.length, 0); + assert.equal(h.failures.length, 0); } }); @@ -314,3 +331,52 @@ test('a retry updates only the newest matching check', async () => { await h.run(); assert.equal(h.updated[0].check_run_id, 2); }); + +test('PR preview uses the same verdict mapping without writing a Check, even for drafts', async () => { + for (const verdict of ['PASS', 'FAIL', 'INCONCLUSIVE']) { + const h = resultHarness(resultBody(verdict).toLowerCase()); + h.pr.draft = true; + h.pr.labels = []; + h.checks.length = 0; + await h.run('pull_request'); + assert.deepEqual(h.updated, []); + assert.equal(h.failures.length, verdict === 'FAIL' ? 1 : 0); + assert.equal(h.warnings.length, verdict === 'INCONCLUSIVE' ? 1 : 0); + assert.match(h.summaries[0], /Read-only PR preview/); + assert.match(h.summaries[0], /non-required/); + } +}); + +test('PR preview cannot reuse another SHA pair, an untrusted reply, or an older PR run', async () => { + for (const change of [ + h => {h.comment.body = resultBody('FAIL', NEW_BASE);}, + h => {h.comment.body = resultBody('FAIL', HEAD, NEW_BASE);}, + h => {h.comment.user.login = 'someone-else';}, + h => {h.comment.user.type = 'User';}, + h => {h.comments.length = 0;}, + h => {h.pr.head.sha = NEW_BASE;}, + ]) { + const h = resultHarness(); + change(h); + await h.run('pull_request'); + assert.deepEqual(h.updated, []); + assert.deepEqual(h.failures, []); + assert.equal(h.warnings.length, 1); + } +}); + +test('PR preview chooses the latest matching reply and rechecks live refs', async () => { + const h = resultHarness(resultBody('FAIL')); + h.comments.push({...h.comment, id: 124, body: resultBody('PASS')}); + await h.run('pull_request'); + assert.deepEqual(h.failures, []); + assert.match(h.summaries[0], /No semantic conflict found/); + for (const change of [h => {h.pr.head.sha = NEW_BASE;}, h => h.setTarget(NEW_BASE)]) { + const stale = resultHarness(); + stale.afterCompare(() => change(stale)); + await stale.run('pull_request'); + assert.deepEqual(stale.updated, []); + assert.deepEqual(stale.failures, []); + assert.equal(stale.warnings.length, 1); + } +}); diff --git a/.github/workflows/coderabbit-semantic-review.yml b/.github/workflows/coderabbit-semantic-review.yml index ddda50427648..eac8049141d5 100644 --- a/.github/workflows/coderabbit-semantic-review.yml +++ b/.github/workflows/coderabbit-semantic-review.yml @@ -16,6 +16,12 @@ name: CodeRabbit Semantic Conflict Review on: + pull_request: + branches: [main] + paths: + - '.coderabbit.yaml' + - '.github/workflows/coderabbit-semantic-review*.yml' + - '.github/scripts/coderabbit_semantic_review*.js' pull_request_target: branches: [main] types: [opened, reopened, synchronize, ready_for_review, labeled, edited, auto_merge_enabled] @@ -43,7 +49,7 @@ jobs: checks: write if: >- github.repository == 'NVIDIA/TensorRT-LLM' && - github.event_name != 'issue_comment' && + contains(fromJSON('["push", "pull_request_target", "workflow_dispatch"]'), github.event_name) && (github.event_name != 'pull_request_target' || contains(github.event.pull_request.labels.*.name, 'ai: semantic-conflict')) # Automatic runs sweep opted-in PRs so coalesced events are not lost. @@ -117,7 +123,8 @@ jobs: output: { title: 'Awaiting CodeRabbit analysis', summary: `No AI verdict yet for PR #${number}, head ${head} + main ${base}. ` + - 'This advisory check does not block merging.', + 'CodeRabbit can make mistakes. This check is advisory and must remain non-required; ' + + 'its failure does not block merging under that configuration.', }, }); const body = [ @@ -161,18 +168,52 @@ jobs: contains(github.event.comment.body, 'Semantic conflict with target branch') runs-on: ubuntu-latest timeout-minutes: 5 - steps: - # Read comments as data. Never checkout or execute PR code with this token. + steps: &result-steps + # Shared with the read-only PR preview. No checkout or execution of PR code. - name: Publish a result only for the current requested revision pair uses: actions/github-script@v8 with: script: | const name = 'Semantic conflict with target branch'; const repo = context.repo; - const number = context.payload.issue.number; - const {data: comment} = await github.rest.issues.getComment({ - ...repo, comment_id: context.payload.comment.id, - }); + const preview = context.eventName === 'pull_request'; + const number = preview ? context.payload.pull_request.number : context.payload.issue.number; + const advisory = 'CodeRabbit can make mistakes, including false positives. Review the evidence. ' + + 'This check is advisory and must remain non-required; its failure does not block merging ' + + 'under that configuration. Other merge requirements still apply.'; + const {data: pr} = await github.rest.pulls.get({...repo, pull_number: number}); + const eligible = pr => pr.state === 'open' && pr.base.ref === 'main' && + (preview || (!pr.draft && pr.labels.some(label => label.name === 'ai: semantic-conflict'))); + if (!eligible(pr)) return; + if (preview && pr.head.sha !== context.payload.pull_request.head.sha) { + core.warning('This preview run is stale; use a run for the current PR head.'); + return; + } + const {data: ref} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); + const head = pr.head.sha; + const base = ref.object.sha; + let comment; + if (preview) { + const comments = await github.paginate(github.rest.issues.listComments, { + ...repo, issue_number: number, per_page: 100, + }); + comment = comments.filter(c => c.user?.login === 'coderabbitai[bot]' && + c.user?.type === 'Bot' && c.body?.toLowerCase().includes(name.toLowerCase()) && + c.body.toLowerCase().includes(`semantic_result head=${head} target=${base} `)) + .sort((a, b) => b.id - a.id)[0]; + if (!comment) { + const message = `No CodeRabbit result for head ${head} + main ${base}. ` + + 'This preview has no AI verdict. Request a custom pre-merge evaluation for these SHAs, ' + + 'then rerun this job after the reply arrives.'; + core.warning(message); + await core.summary.addRaw(`${message}\n\n${advisory}\n`).write(); + return; + } + } else { + ({data: comment} = await github.rest.issues.getComment({ + ...repo, comment_id: context.payload.comment.id, + })); + } const body = comment.body || ''; if (comment.user?.login !== 'coderabbitai[bot]' || comment.user?.type !== 'Bot') return; if (!body.includes('') && @@ -183,19 +224,16 @@ jobs: const details = body.match(/Full details: Semantic conflict with target branch<\/summary>([\s\S]*?)<\/details>/i); const result = details ? details[1] : row.join('|'); - const {data: pr} = await github.rest.pulls.get({...repo, pull_number: number}); - if (pr.state !== 'open' || pr.draft || pr.base.ref !== 'main' || - !pr.labels.some(label => label.name === 'ai: semantic-conflict')) return; - const {data: ref} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); - const head = pr.head.sha; - const base = ref.object.sha; - const checks = await github.paginate(github.rest.checks.listForRef, { - ...repo, ref: head, check_name: name, filter: 'all', per_page: 100, - }); - const check = checks.filter(check => check.app?.slug === 'github-actions' && - check.external_id === `semantic-conflict:${number}:${head}:${base}`) - .sort((a, b) => b.id - a.id)[0]; - if (!check) return; // Includes unrelated manual fixture evaluations. + let check; + if (!preview) { + const checks = await github.paginate(github.rest.checks.listForRef, { + ...repo, ref: head, check_name: name, filter: 'all', per_page: 100, + }); + check = checks.filter(check => check.app?.slug === 'github-actions' && + check.external_id === `semantic-conflict:${number}:${head}:${base}`) + .sort((a, b) => b.id - a.id)[0]; + if (!check) return; // Includes unrelated manual fixture evaluations. + } const pattern = /semantic_result head=([a-f0-9]{40}) target=([a-f0-9]{40}) merge_base=([a-f0-9]{40}) verdict=(pass|fail|inconclusive)\b/g; const records = [...new Set([...result.toLowerCase().matchAll(pattern)].map(match => match[0]))]; @@ -222,22 +260,37 @@ jobs: const {data: current} = await github.rest.pulls.get({...repo, pull_number: number}); const {data: currentRef} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); if (current.head.sha !== head || currentRef.object.sha !== base || - current.state !== 'open' || current.draft || current.base.ref !== 'main' || - !current.labels.some(label => label.name === 'ai: semantic-conflict')) return; + !eligible(current)) { + if (preview) core.warning('The PR or main changed during this preview; rerun for fresh evidence.'); + return; + } const titles = { PASS: 'No semantic conflict found (best effort)', - FAIL: '⚠️ Possible semantic conflict', + FAIL: 'Possible semantic conflict — CodeRabbit may be wrong', INCONCLUSIVE: '⚠️ Semantic analysis inconclusive', }; - await github.rest.checks.update({ + const summary = `${reason}\n\n[CodeRabbit analysis](${comment.html_url})\n\n${advisory}`; + if (!preview) await github.rest.checks.update({ ...repo, check_run_id: check.id, status: 'completed', - conclusion: verdict === 'PASS' ? 'success' : 'neutral', + conclusion: {PASS: 'success', FAIL: 'failure', INCONCLUSIVE: 'neutral'}[verdict], details_url: comment.html_url, output: { title: titles[verdict], - summary: `${reason}\n\n[CodeRabbit analysis](${comment.html_url})\n\n` + - 'Advisory only; applies to this SHA pair and does not prove compatibility.', + summary, }, }); - if (verdict !== 'PASS') core.warning(`${titles[verdict]}: ${comment.html_url}`); - await core.summary.addRaw(`${titles[verdict]}: [CodeRabbit analysis](${comment.html_url})\n`).write(); + await core.summary.addRaw(`${preview ? 'Read-only PR preview\n\n' : ''}` + + `${titles[verdict]}\n\n${summary}\n`).write(); + if (verdict === 'FAIL') core.setFailed(`${titles[verdict]}. ${advisory} ${comment.html_url}`); + if (verdict === 'INCONCLUSIVE') core.warning(`${titles[verdict]}: ${comment.html_url}`); + + preview-result: + name: Semantic conflict preview (advisory) + if: github.event_name == 'pull_request' + permissions: + contents: read + pull-requests: read + issues: read + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: *result-steps diff --git a/AGENTS.md b/AGENTS.md index a1ad31cb4f0c..20c373d00e04 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -180,7 +180,8 @@ For a full list of up-to-date bot commands, post `/bot help` as a PR comment and CodeRabbit's `Semantic conflict with target branch` pre-merge check in `.coderabbit.yaml` looks for behavioral incompatibilities across branches. It is -best-effort and warning-only; missing revision/history evidence is Inconclusive. +best-effort; missing revision/history evidence is Inconclusive. The CodeRabbit +comment uses warning mode; GitHub Check conclusions are published separately. For a non-draft PR targeting `main`, maintainers can add the `ai: semantic-conflict` label to opt into automatic rechecks on PR and main @@ -189,9 +190,13 @@ and publishes results; its request job succeeding is not an AI verdict. The inde `Semantic conflict with target branch` GitHub Check starts neutral while awaiting analysis. The workflow publishes CodeRabbit's result only after verifying the bot author, requested head/target pair, and merge-base SHA. -PASS becomes success; conflicts and Inconclusive results stay neutral with -a warning title and a link to the analysis. GitHub has no warning conclusion; -the publishing job emits a warning annotation but can still succeed. +PASS becomes success; a verified FAIL makes both the semantic Check and its +publishing job fail (red). Inconclusive results stay neutral with a warning +annotation. Results explain that CodeRabbit can make mistakes, including false +positives. Keep this advisory Check and workflow non-required: their failures +then do not block merging. Reviewers can document a false positive and merge +once the other requirements are met. This workflow does not change repository +rules; adding it to required checks would make failures block merging. Read the result and verify its SHAs still match the live branches. Use workflow dispatch with the PR number to retry only that PR, or comment @@ -206,6 +211,18 @@ are not part of the existing `Pre-commit Check`. The custom check requires CodeRabbit Custom Pre-Merge Checks access. This advisory pilot does not provide a merge-queue check or block merges. +Changes to the semantic automation also run `Semantic conflict preview +(advisory)` on `pull_request`, including fork drafts. It reads real CodeRabbit +replies for the current head/main pair with read-only permissions and uses the +same verification and verdict mapping as the publisher, without writing Checks. +Before the configuration is merged, request `@coderabbitai evaluate custom +pre-merge check` with `--name "Semantic conflict with target branch"`, +`--mode warning`, and `--instructions` containing the check instructions from +`.coderabbit.yaml` and the current head/main SHAs. After the reply arrives, +rerun the preview job. A missing reply produces a warning and no AI verdict; +a verified conflict makes the preview job red. Previewing does not validate +the production event triggers or privileged Check writes. + ### Trouble Shooting - Use `TLLM_LOG_LEVEL_BY_MODULE` to enable per-module log filtering (e.g., `"debug:_torch,runtime;info:serve"`); see [Module-Level Logging](docs/source/developer-guide/overview.md#module-level-logging) for details. From 3bd0df05a5c1326edc764c496efb05dc83114f2e Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Wed, 16 Sep 2026 22:16:09 +0800 Subject: [PATCH 07/21] [None][infra] Exercise real semantic conflict failure display on GitHub Signed-off-by: Yanchao Lu --- .../coderabbit_semantic_review.test.js | 2 +- .../workflows/coderabbit-semantic-review.yml | 57 ++++++++++++++----- 2 files changed, 44 insertions(+), 15 deletions(-) diff --git a/.github/scripts/coderabbit_semantic_review.test.js b/.github/scripts/coderabbit_semantic_review.test.js index 7456a5ec474d..c81ef4d1c3e2 100644 --- a/.github/scripts/coderabbit_semantic_review.test.js +++ b/.github/scripts/coderabbit_semantic_review.test.js @@ -225,7 +225,7 @@ function resultHarness(body = resultBody()) { return {data: {merge_base_commit: {sha: mergeBase}}}; }, }; - const core = {warning: text => warnings.push(text), setFailed: text => failures.push(text), + const core = {info: () => {}, warning: text => warnings.push(text), setFailed: text => failures.push(text), summary: {addRaw(text) {summaries.push(text); return this;}, async write() {}}}; return {comment, comments, checks, updated, warnings, failures, summaries, pr: request.pr, setMergeBase: sha => {mergeBase = sha;}, diff --git a/.github/workflows/coderabbit-semantic-review.yml b/.github/workflows/coderabbit-semantic-review.yml index eac8049141d5..e13ca668536c 100644 --- a/.github/workflows/coderabbit-semantic-review.yml +++ b/.github/workflows/coderabbit-semantic-review.yml @@ -177,6 +177,7 @@ jobs: const name = 'Semantic conflict with target branch'; const repo = context.repo; const preview = context.eventName === 'pull_request'; + const fixture = preview && process.env.SEMANTIC_FIXTURE_TEST === 'true'; const number = preview ? context.payload.pull_request.number : context.payload.issue.number; const advisory = 'CodeRabbit can make mistakes, including false positives. Review the evidence. ' + 'This check is advisory and must remain non-required; its failure does not block merging ' + @@ -185,15 +186,25 @@ jobs: const eligible = pr => pr.state === 'open' && pr.base.ref === 'main' && (preview || (!pr.draft && pr.labels.some(label => label.name === 'ai: semantic-conflict'))); if (!eligible(pr)) return; - if (preview && pr.head.sha !== context.payload.pull_request.head.sha) { + if (preview && !fixture && pr.head.sha !== context.payload.pull_request.head.sha) { core.warning('This preview run is stale; use a run for the current PR head.'); return; } - const {data: ref} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); - const head = pr.head.sha; - const base = ref.object.sha; + let head = pr.head.sha; + let base; + if (fixture) { + head = '9ae0c8db42244bbc1e538cefd44ec304760b0eef'; + base = '3b7d13a768918d6182cb30f439c596d74eaf0b91'; + } else { + const {data: ref} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); + base = ref.object.sha; + } let comment; - if (preview) { + if (fixture) { + ({data: comment} = await github.rest.issues.getComment({ + owner: 'NVIDIA', repo: 'TensorRT-LLM', comment_id: 5697984104, + })); + } else if (preview) { const comments = await github.paginate(github.rest.issues.listComments, { ...repo, issue_number: number, per_page: 100, }); @@ -247,7 +258,8 @@ jobs: if (reportedHead !== head || reportedBase !== base) return; const {data: comparison} = await github.request( 'GET /repos/{owner}/{repo}/compare/{basehead}', - {...repo, basehead: `${base}...${head}`}); + {...(fixture ? {owner: 'chzblych', repo: 'TensorRT-LLM'} : repo), + basehead: `${base}...${head}`}); const expectedStatus = {PASS: /Passed/i, FAIL: /Warning|Error/i, INCONCLUSIVE: /Inconclusive/i}; if (comparison.merge_base_commit?.sha === mergeBase && expectedStatus[reportedVerdict].test(row[2])) { verdict = reportedVerdict; @@ -257,19 +269,22 @@ jobs: } } // Recheck refs after fetching the result and history. - const {data: current} = await github.rest.pulls.get({...repo, pull_number: number}); - const {data: currentRef} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); - if (current.head.sha !== head || currentRef.object.sha !== base || - !eligible(current)) { - if (preview) core.warning('The PR or main changed during this preview; rerun for fresh evidence.'); - return; + if (!fixture) { + const {data: current} = await github.rest.pulls.get({...repo, pull_number: number}); + const {data: currentRef} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); + if (current.head.sha !== head || currentRef.object.sha !== base || + !eligible(current)) { + if (preview) core.warning('The PR or main changed during this preview; rerun for fresh evidence.'); + return; + } } const titles = { PASS: 'No semantic conflict found (best effort)', FAIL: 'Possible semantic conflict — CodeRabbit may be wrong', INCONCLUSIVE: '⚠️ Semantic analysis inconclusive', }; - const summary = `${reason}\n\n[CodeRabbit analysis](${comment.html_url})\n\n${advisory}`; + const scope = fixture ? 'KNOWN CONFLICT FIXTURE: expected red failure; not a verdict on this PR. ' : ''; + const summary = `${scope}${reason}\n\n[CodeRabbit analysis](${comment.html_url})\n\n${advisory}`; if (!preview) await github.rest.checks.update({ ...repo, check_run_id: check.id, status: 'completed', conclusion: {PASS: 'success', FAIL: 'failure', INCONCLUSIVE: 'neutral'}[verdict], @@ -281,7 +296,8 @@ jobs: }); await core.summary.addRaw(`${preview ? 'Read-only PR preview\n\n' : ''}` + `${titles[verdict]}\n\n${summary}\n`).write(); - if (verdict === 'FAIL') core.setFailed(`${titles[verdict]}. ${advisory} ${comment.html_url}`); + core.info(`${scope}Verified semantic verdict: ${verdict}; head=${head}; target=${base}`); + if (verdict === 'FAIL') core.setFailed(`${scope}${titles[verdict]}. ${advisory} ${comment.html_url}`); if (verdict === 'INCONCLUSIVE') core.warning(`${titles[verdict]}: ${comment.html_url}`); preview-result: @@ -294,3 +310,16 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 5 steps: *result-steps + + known-conflict-display-test: + name: Known conflict fixture - expected red failure (not a PR verdict) + if: github.event_name == 'pull_request' && github.event.pull_request.number == 19268 + permissions: + contents: read + pull-requests: read + issues: read + env: + SEMANTIC_FIXTURE_TEST: 'true' + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: *result-steps From c011ccb54219781a5bd5563f4a1c6a1941e93d3a Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Wed, 16 Sep 2026 22:18:14 +0800 Subject: [PATCH 08/21] [None][infra] Remove temporary display test after live GitHub validation Signed-off-by: Yanchao Lu --- .../workflows/coderabbit-semantic-review.yml | 58 +++++-------------- 1 file changed, 15 insertions(+), 43 deletions(-) diff --git a/.github/workflows/coderabbit-semantic-review.yml b/.github/workflows/coderabbit-semantic-review.yml index e13ca668536c..1853054aec07 100644 --- a/.github/workflows/coderabbit-semantic-review.yml +++ b/.github/workflows/coderabbit-semantic-review.yml @@ -177,7 +177,6 @@ jobs: const name = 'Semantic conflict with target branch'; const repo = context.repo; const preview = context.eventName === 'pull_request'; - const fixture = preview && process.env.SEMANTIC_FIXTURE_TEST === 'true'; const number = preview ? context.payload.pull_request.number : context.payload.issue.number; const advisory = 'CodeRabbit can make mistakes, including false positives. Review the evidence. ' + 'This check is advisory and must remain non-required; its failure does not block merging ' + @@ -186,25 +185,15 @@ jobs: const eligible = pr => pr.state === 'open' && pr.base.ref === 'main' && (preview || (!pr.draft && pr.labels.some(label => label.name === 'ai: semantic-conflict'))); if (!eligible(pr)) return; - if (preview && !fixture && pr.head.sha !== context.payload.pull_request.head.sha) { + if (preview && pr.head.sha !== context.payload.pull_request.head.sha) { core.warning('This preview run is stale; use a run for the current PR head.'); return; } - let head = pr.head.sha; - let base; - if (fixture) { - head = '9ae0c8db42244bbc1e538cefd44ec304760b0eef'; - base = '3b7d13a768918d6182cb30f439c596d74eaf0b91'; - } else { - const {data: ref} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); - base = ref.object.sha; - } + const {data: ref} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); + const head = pr.head.sha; + const base = ref.object.sha; let comment; - if (fixture) { - ({data: comment} = await github.rest.issues.getComment({ - owner: 'NVIDIA', repo: 'TensorRT-LLM', comment_id: 5697984104, - })); - } else if (preview) { + if (preview) { const comments = await github.paginate(github.rest.issues.listComments, { ...repo, issue_number: number, per_page: 100, }); @@ -258,8 +247,7 @@ jobs: if (reportedHead !== head || reportedBase !== base) return; const {data: comparison} = await github.request( 'GET /repos/{owner}/{repo}/compare/{basehead}', - {...(fixture ? {owner: 'chzblych', repo: 'TensorRT-LLM'} : repo), - basehead: `${base}...${head}`}); + {...repo, basehead: `${base}...${head}`}); const expectedStatus = {PASS: /Passed/i, FAIL: /Warning|Error/i, INCONCLUSIVE: /Inconclusive/i}; if (comparison.merge_base_commit?.sha === mergeBase && expectedStatus[reportedVerdict].test(row[2])) { verdict = reportedVerdict; @@ -269,22 +257,19 @@ jobs: } } // Recheck refs after fetching the result and history. - if (!fixture) { - const {data: current} = await github.rest.pulls.get({...repo, pull_number: number}); - const {data: currentRef} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); - if (current.head.sha !== head || currentRef.object.sha !== base || - !eligible(current)) { - if (preview) core.warning('The PR or main changed during this preview; rerun for fresh evidence.'); - return; - } + const {data: current} = await github.rest.pulls.get({...repo, pull_number: number}); + const {data: currentRef} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); + if (current.head.sha !== head || currentRef.object.sha !== base || + !eligible(current)) { + if (preview) core.warning('The PR or main changed during this preview; rerun for fresh evidence.'); + return; } const titles = { PASS: 'No semantic conflict found (best effort)', FAIL: 'Possible semantic conflict — CodeRabbit may be wrong', INCONCLUSIVE: '⚠️ Semantic analysis inconclusive', }; - const scope = fixture ? 'KNOWN CONFLICT FIXTURE: expected red failure; not a verdict on this PR. ' : ''; - const summary = `${scope}${reason}\n\n[CodeRabbit analysis](${comment.html_url})\n\n${advisory}`; + const summary = `${reason}\n\n[CodeRabbit analysis](${comment.html_url})\n\n${advisory}`; if (!preview) await github.rest.checks.update({ ...repo, check_run_id: check.id, status: 'completed', conclusion: {PASS: 'success', FAIL: 'failure', INCONCLUSIVE: 'neutral'}[verdict], @@ -296,8 +281,8 @@ jobs: }); await core.summary.addRaw(`${preview ? 'Read-only PR preview\n\n' : ''}` + `${titles[verdict]}\n\n${summary}\n`).write(); - core.info(`${scope}Verified semantic verdict: ${verdict}; head=${head}; target=${base}`); - if (verdict === 'FAIL') core.setFailed(`${scope}${titles[verdict]}. ${advisory} ${comment.html_url}`); + core.info(`Verified semantic verdict: ${verdict}; head=${head}; target=${base}`); + if (verdict === 'FAIL') core.setFailed(`${titles[verdict]}. ${advisory} ${comment.html_url}`); if (verdict === 'INCONCLUSIVE') core.warning(`${titles[verdict]}: ${comment.html_url}`); preview-result: @@ -310,16 +295,3 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 5 steps: *result-steps - - known-conflict-display-test: - name: Known conflict fixture - expected red failure (not a PR verdict) - if: github.event_name == 'pull_request' && github.event.pull_request.number == 19268 - permissions: - contents: read - pull-requests: read - issues: read - env: - SEMANTIC_FIXTURE_TEST: 'true' - runs-on: ubuntu-latest - timeout-minutes: 5 - steps: *result-steps From 8c17858d5f78d9f5aaaade73c1527bb9c0e5cf22 Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Thu, 17 Sep 2026 11:15:59 +0800 Subject: [PATCH 09/21] [None][infra] Separate semantic preview from production checks Signed-off-by: Yanchao Lu --- .../coderabbit_semantic_review.test.js | 55 ++++++- .../coderabbit_semantic_review_result.js | 136 +++++++++++++++++ .../coderabbit-semantic-review-tests.yml | 48 +++++- .../workflows/coderabbit-semantic-review.yml | 139 ++---------------- AGENTS.md | 33 +++-- 5 files changed, 260 insertions(+), 151 deletions(-) create mode 100644 .github/scripts/coderabbit_semantic_review_result.js diff --git a/.github/scripts/coderabbit_semantic_review.test.js b/.github/scripts/coderabbit_semantic_review.test.js index c81ef4d1c3e2..4350644422d8 100644 --- a/.github/scripts/coderabbit_semantic_review.test.js +++ b/.github/scripts/coderabbit_semantic_review.test.js @@ -24,7 +24,7 @@ const scripts = [...workflow.matchAll(/^ {10}script: \|\n((?: {12}[^\n]*(?:\n|$) assert.equal(scripts.length, 2); const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor; const execute = new AsyncFunction('github', 'context', 'core', 'process', scripts[0]); -const publish = new AsyncFunction('github', 'context', 'core', scripts[1]); +const publish = require('./coderabbit_semantic_review_result.js'); const HEAD = 'a'.repeat(40); const BASE = 'b'.repeat(40); @@ -204,6 +204,7 @@ function resultHarness(body = resultBody()) { const checks = [{id: 1, app: {slug: 'github-actions'}, external_id: `semantic-conflict:12:${HEAD}:${BASE}`}]; const updated = []; + const outputs = {}; const warnings = []; const failures = []; const summaries = []; @@ -225,15 +226,16 @@ function resultHarness(body = resultBody()) { return {data: {merge_base_commit: {sha: mergeBase}}}; }, }; - const core = {info: () => {}, warning: text => warnings.push(text), setFailed: text => failures.push(text), + const core = {setOutput: (key, value) => {outputs[key] = value;}, info: () => {}, + warning: text => warnings.push(text), setFailed: text => failures.push(text), summary: {addRaw(text) {summaries.push(text); return this;}, async write() {}}}; - return {comment, comments, checks, updated, warnings, failures, summaries, pr: request.pr, + return {comment, comments, checks, updated, warnings, failures, summaries, outputs, pr: request.pr, setMergeBase: sha => {mergeBase = sha;}, setTarget: sha => {target = sha;}, afterCompare: callback => {afterCompare = callback;}, - run: (eventName = 'issue_comment') => publish(github, {repo: {owner: 'example', repo: 'repo'}, + run: (eventName = 'issue_comment') => publish({github, context: {repo: {owner: 'example', repo: 'repo'}, eventName, payload: {issue: {number: 12}, comment: {id: 123}, - pull_request: {number: 12, head: {sha: HEAD}}}}, core), + pull_request: {number: 12, head: {sha: HEAD}}}}, core}), }; } @@ -340,7 +342,10 @@ test('PR preview uses the same verdict mapping without writing a Check, even for h.checks.length = 0; await h.run('pull_request'); assert.deepEqual(h.updated, []); - assert.equal(h.failures.length, verdict === 'FAIL' ? 1 : 0); + assert.equal(h.failures.length, 0); // The separate verdict job displays failures. + assert.equal(h.outputs.verdict, verdict); + assert.match(h.outputs.message, /false positives/); + assert.match(h.outputs.summary, /CodeRabbit analysis/); assert.equal(h.warnings.length, verdict === 'INCONCLUSIVE' ? 1 : 0); assert.match(h.summaries[0], /Read-only PR preview/); assert.match(h.summaries[0], /non-required/); @@ -362,6 +367,7 @@ test('PR preview cannot reuse another SHA pair, an untrusted reply, or an older assert.deepEqual(h.updated, []); assert.deepEqual(h.failures, []); assert.equal(h.warnings.length, 1); + assert.equal(h.outputs.verdict, 'INCONCLUSIVE'); } }); @@ -378,5 +384,42 @@ test('PR preview chooses the latest matching reply and rechecks live refs', asyn assert.deepEqual(stale.updated, []); assert.deepEqual(stale.failures, []); assert.equal(stale.warnings.length, 1); + assert.equal(stale.outputs.verdict, 'INCONCLUSIVE'); + } +}); + +test('preview never exports an AI approval for malformed or contradictory evidence', async () => { + for (const body of [ + resultBody('PASS').replace('', ''), + resultBody('PASS').replace('✅ Passed', '❓ Inconclusive'), + resultBody('PASS').replace('', resultBody('FAIL') + ''), + ]) { + const h = resultHarness(body); + await h.run('pull_request'); + assert.equal(h.outputs.verdict, 'INCONCLUSIVE'); + assert.deepEqual(h.updated, []); + } +}); + + +test('the separate AI job displays the verified verdict and fails only for conflicts', async () => { + const preview = fs.readFileSync( + path.join(__dirname, '../workflows/coderabbit-semantic-review-tests.yml'), 'utf8'); + const displayScript = [...preview.matchAll(/^ {10}script: \|\n((?: {12}[^\n]*(?:\n|$)|\n)*)/gm)] + .at(-1)[1].replace(/^ {12}/gm, ''); + const display = new AsyncFunction('core', 'process', displayScript); + for (const verdict of ['PASS', 'FAIL']) { + const h = resultHarness(resultBody(verdict)); + await h.run('pull_request'); + const failures = []; + const summaries = []; + await display({setFailed: message => failures.push(message), summary: { + addRaw(text) {summaries.push(text); return this;}, async write() {}, + }}, {env: {SEMANTIC_VERDICT: h.outputs.verdict, + SEMANTIC_MESSAGE: h.outputs.message, SEMANTIC_SUMMARY: h.outputs.summary}}); + assert.equal(failures.length, verdict === 'FAIL' ? 1 : 0); + assert.match(summaries[0], /Verified head/); + assert.match(summaries[0], /false positives/); + if (verdict === 'FAIL') assert.match(failures[0], /does not block merging/); } }); diff --git a/.github/scripts/coderabbit_semantic_review_result.js b/.github/scripts/coderabbit_semantic_review_result.js new file mode 100644 index 000000000000..53d7fb708770 --- /dev/null +++ b/.github/scripts/coderabbit_semantic_review_result.js @@ -0,0 +1,136 @@ +// 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. + +// Shared by the trusted publisher and read-only PR preview. +module.exports = async ({github, context, core}) => { + const name = 'Semantic conflict with target branch'; + const repo = context.repo; + const preview = context.eventName === 'pull_request'; + if (preview) core.setOutput('verdict', 'INCONCLUSIVE'); + const number = preview ? context.payload.pull_request.number : context.payload.issue.number; + const advisory = 'CodeRabbit can make mistakes, including false positives. Review the evidence. ' + + 'This check is advisory and must remain non-required; its failure does not block merging ' + + 'under that configuration. Other merge requirements still apply.'; + const {data: pr} = await github.rest.pulls.get({...repo, pull_number: number}); + const eligible = pr => pr.state === 'open' && pr.base.ref === 'main' && + (preview || (!pr.draft && pr.labels.some(label => label.name === 'ai: semantic-conflict'))); + if (!eligible(pr)) return; + if (preview && pr.head.sha !== context.payload.pull_request.head.sha) { + core.warning('This preview run is stale; use a run for the current PR head.'); + return; + } + const {data: ref} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); + const head = pr.head.sha; + const base = ref.object.sha; + let comment; + if (preview) { + const comments = await github.paginate(github.rest.issues.listComments, { + ...repo, issue_number: number, per_page: 100, + }); + comment = comments.filter(c => c.user?.login === 'coderabbitai[bot]' && + c.user?.type === 'Bot' && c.body?.toLowerCase().includes(name.toLowerCase()) && + c.body.toLowerCase().includes(`semantic_result head=${head} target=${base} `)) + .sort((a, b) => b.id - a.id)[0]; + if (!comment) { + const message = `No CodeRabbit result for head ${head} + main ${base}. ` + + 'This preview has no AI verdict. Request a custom pre-merge evaluation for these SHAs, ' + + 'then rerun the tests and result lookup job after the reply arrives.'; + core.warning(message); + await core.summary.addRaw(`${message}\n\n${advisory}\n`).write(); + return; + } + } else { + ({data: comment} = await github.rest.issues.getComment({ + ...repo, comment_id: context.payload.comment.id, + })); + } + const body = comment.body || ''; + if (comment.user?.login !== 'coderabbitai[bot]' || comment.user?.type !== 'Bot') return; + if (!body.includes('') && + !body.includes('')) return; + const row = body.split('\n').map(line => line.split('|').map(cell => cell.trim())) + .find(cells => cells[1]?.toLowerCase() === name.toLowerCase()); + if (!row) return; + const details = body.match(/Full details: Semantic conflict with target branch<\/summary>([\s\S]*?)<\/details>/i); + const result = details ? details[1] : row.join('|'); + + let check; + if (!preview) { + const checks = await github.paginate(github.rest.checks.listForRef, { + ...repo, ref: head, check_name: name, filter: 'all', per_page: 100, + }); + check = checks.filter(check => check.app?.slug === 'github-actions' && + check.external_id === `semantic-conflict:${number}:${head}:${base}`) + .sort((a, b) => b.id - a.id)[0]; + if (!check) return; // Includes unrelated manual fixture evaluations. + } + + const pattern = /semantic_result head=([a-f0-9]{40}) target=([a-f0-9]{40}) merge_base=([a-f0-9]{40}) verdict=(pass|fail|inconclusive)\b/g; + const records = [...new Set([...result.toLowerCase().matchAll(pattern)].map(match => match[0]))]; + let verdict = 'INCONCLUSIVE'; + let reason = 'The result has no unique, verifiable revision record.'; + if (records.length === 1) { + const [, reportedHead, reportedBase, mergeBase, rawVerdict] = + [...records[0].matchAll(pattern)][0]; + const reportedVerdict = rawVerdict.toUpperCase(); + // A stale comment must never overwrite the current revision's result. + if (reportedHead !== head || reportedBase !== base) return; + const {data: comparison} = await github.request( + 'GET /repos/{owner}/{repo}/compare/{basehead}', + {...repo, basehead: `${base}...${head}`}); + const expectedStatus = {PASS: /Passed/i, FAIL: /Warning|Error/i, INCONCLUSIVE: /Inconclusive/i}; + if (comparison.merge_base_commit?.sha === mergeBase && expectedStatus[reportedVerdict].test(row[2])) { + verdict = reportedVerdict; + reason = `Verified head ${head}, target ${base}, merge base ${mergeBase}.`; + } else { + reason = 'The reported verdict or merge base could not be verified.'; + } + } + // Recheck refs after fetching the result and history. + const {data: current} = await github.rest.pulls.get({...repo, pull_number: number}); + const {data: currentRef} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); + if (current.head.sha !== head || currentRef.object.sha !== base || + !eligible(current)) { + if (preview) core.warning('The PR or main changed during this preview; rerun for fresh evidence.'); + return; + } + const titles = { + PASS: 'No semantic conflict found (best effort)', + FAIL: 'Possible semantic conflict — CodeRabbit may be wrong', + INCONCLUSIVE: '⚠️ Semantic analysis inconclusive', + }; + const summary = `${reason}\n\n[CodeRabbit analysis](${comment.html_url})\n\n${advisory}`; + if (!preview) await github.rest.checks.update({ + ...repo, check_run_id: check.id, status: 'completed', + conclusion: {PASS: 'success', FAIL: 'failure', INCONCLUSIVE: 'neutral'}[verdict], + details_url: comment.html_url, + output: { + title: titles[verdict], + summary, + }, + }); + await core.summary.addRaw(`${preview ? 'Read-only PR preview\n\n' : ''}` + + `${titles[verdict]}\n\n${summary}\n`).write(); + core.info(`Verified semantic verdict: ${verdict}; head=${head}; target=${base}`); + const message = `${titles[verdict]}. ${advisory} ${comment.html_url}`; + if (preview) { + core.setOutput('verdict', verdict); + core.setOutput('summary', `${titles[verdict]}\n\n${summary}`); + core.setOutput('message', message); + } else if (verdict === 'FAIL') { + core.setFailed(message); + } + if (verdict === 'INCONCLUSIVE') core.warning(`${titles[verdict]}: ${comment.html_url}`); +}; diff --git a/.github/workflows/coderabbit-semantic-review-tests.yml b/.github/workflows/coderabbit-semantic-review-tests.yml index be616d93f4e4..d745427fc0f1 100644 --- a/.github/workflows/coderabbit-semantic-review-tests.yml +++ b/.github/workflows/coderabbit-semantic-review-tests.yml @@ -13,11 +13,13 @@ # See the License for the specific language governing permissions and # limitations under the License. -name: CodeRabbit Semantic Review Tests +name: CodeRabbit Semantic Review Preview on: pull_request: + branches: [main] paths: + - '.coderabbit.yaml' - '.github/workflows/coderabbit-semantic-review*.yml' - '.github/scripts/coderabbit_semantic_review*.js' workflow_dispatch: @@ -31,7 +33,15 @@ concurrency: jobs: test: - name: Semantic review workflow tests + name: Automation tests and result lookup (not AI approval) + permissions: + contents: read + pull-requests: read + issues: read + outputs: + verdict: ${{ steps.result.outputs.verdict }} + summary: ${{ steps.result.outputs.summary }} + message: ${{ steps.result.outputs.message }} runs-on: ubuntu-latest timeout-minutes: 5 steps: @@ -40,3 +50,37 @@ jobs: persist-credentials: false - name: Test semantic review automation run: node --test .github/scripts/coderabbit_semantic_review.test.js + - name: Read CodeRabbit verdict for the current PR and main + id: result + if: github.event_name == 'pull_request' + uses: actions/github-script@v8 + with: + script: | + const readResult = require('./.github/scripts/coderabbit_semantic_review_result.js'); + await readResult({github, context, core}); + + verdict: + name: >- + ${{ needs.test.outputs.verdict == 'PASS' && 'AI - no conflict found (advisory)' || + needs.test.outputs.verdict == 'FAIL' && 'AI - possible conflict (advisory)' || + 'AI - no current verdict (advisory)' }} + needs: test + if: >- + github.event_name == 'pull_request' && + contains(fromJSON('["PASS", "FAIL"]'), needs.test.outputs.verdict) + permissions: {} + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Display the verified AI verdict + uses: actions/github-script@v8 + env: + SEMANTIC_VERDICT: ${{ needs.test.outputs.verdict }} + SEMANTIC_SUMMARY: ${{ needs.test.outputs.summary }} + SEMANTIC_MESSAGE: ${{ needs.test.outputs.message }} + with: + script: | + await core.summary.addRaw(process.env.SEMANTIC_SUMMARY).write(); + if (process.env.SEMANTIC_VERDICT === 'FAIL') { + core.setFailed(process.env.SEMANTIC_MESSAGE); + } diff --git a/.github/workflows/coderabbit-semantic-review.yml b/.github/workflows/coderabbit-semantic-review.yml index 1853054aec07..013308341eca 100644 --- a/.github/workflows/coderabbit-semantic-review.yml +++ b/.github/workflows/coderabbit-semantic-review.yml @@ -16,12 +16,6 @@ name: CodeRabbit Semantic Conflict Review on: - pull_request: - branches: [main] - paths: - - '.coderabbit.yaml' - - '.github/workflows/coderabbit-semantic-review*.yml' - - '.github/scripts/coderabbit_semantic_review*.js' pull_request_target: branches: [main] types: [opened, reopened, synchronize, ready_for_review, labeled, edited, auto_merge_enabled] @@ -168,130 +162,17 @@ jobs: contains(github.event.comment.body, 'Semantic conflict with target branch') runs-on: ubuntu-latest timeout-minutes: 5 - steps: &result-steps - # Shared with the read-only PR preview. No checkout or execution of PR code. + steps: + # Load only the trusted workflow revision, never PR code with write access. + - uses: actions/checkout@v6 + with: + ref: ${{ github.workflow_sha }} + persist-credentials: false + sparse-checkout: .github/scripts/coderabbit_semantic_review_result.js + sparse-checkout-cone-mode: false - name: Publish a result only for the current requested revision pair uses: actions/github-script@v8 with: script: | - const name = 'Semantic conflict with target branch'; - const repo = context.repo; - const preview = context.eventName === 'pull_request'; - const number = preview ? context.payload.pull_request.number : context.payload.issue.number; - const advisory = 'CodeRabbit can make mistakes, including false positives. Review the evidence. ' + - 'This check is advisory and must remain non-required; its failure does not block merging ' + - 'under that configuration. Other merge requirements still apply.'; - const {data: pr} = await github.rest.pulls.get({...repo, pull_number: number}); - const eligible = pr => pr.state === 'open' && pr.base.ref === 'main' && - (preview || (!pr.draft && pr.labels.some(label => label.name === 'ai: semantic-conflict'))); - if (!eligible(pr)) return; - if (preview && pr.head.sha !== context.payload.pull_request.head.sha) { - core.warning('This preview run is stale; use a run for the current PR head.'); - return; - } - const {data: ref} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); - const head = pr.head.sha; - const base = ref.object.sha; - let comment; - if (preview) { - const comments = await github.paginate(github.rest.issues.listComments, { - ...repo, issue_number: number, per_page: 100, - }); - comment = comments.filter(c => c.user?.login === 'coderabbitai[bot]' && - c.user?.type === 'Bot' && c.body?.toLowerCase().includes(name.toLowerCase()) && - c.body.toLowerCase().includes(`semantic_result head=${head} target=${base} `)) - .sort((a, b) => b.id - a.id)[0]; - if (!comment) { - const message = `No CodeRabbit result for head ${head} + main ${base}. ` + - 'This preview has no AI verdict. Request a custom pre-merge evaluation for these SHAs, ' + - 'then rerun this job after the reply arrives.'; - core.warning(message); - await core.summary.addRaw(`${message}\n\n${advisory}\n`).write(); - return; - } - } else { - ({data: comment} = await github.rest.issues.getComment({ - ...repo, comment_id: context.payload.comment.id, - })); - } - const body = comment.body || ''; - if (comment.user?.login !== 'coderabbitai[bot]' || comment.user?.type !== 'Bot') return; - if (!body.includes('') && - !body.includes('')) return; - const row = body.split('\n').map(line => line.split('|').map(cell => cell.trim())) - .find(cells => cells[1]?.toLowerCase() === name.toLowerCase()); - if (!row) return; - const details = body.match(/Full details: Semantic conflict with target branch<\/summary>([\s\S]*?)<\/details>/i); - const result = details ? details[1] : row.join('|'); - - let check; - if (!preview) { - const checks = await github.paginate(github.rest.checks.listForRef, { - ...repo, ref: head, check_name: name, filter: 'all', per_page: 100, - }); - check = checks.filter(check => check.app?.slug === 'github-actions' && - check.external_id === `semantic-conflict:${number}:${head}:${base}`) - .sort((a, b) => b.id - a.id)[0]; - if (!check) return; // Includes unrelated manual fixture evaluations. - } - - const pattern = /semantic_result head=([a-f0-9]{40}) target=([a-f0-9]{40}) merge_base=([a-f0-9]{40}) verdict=(pass|fail|inconclusive)\b/g; - const records = [...new Set([...result.toLowerCase().matchAll(pattern)].map(match => match[0]))]; - let verdict = 'INCONCLUSIVE'; - let reason = 'The result has no unique, verifiable revision record.'; - if (records.length === 1) { - const [, reportedHead, reportedBase, mergeBase, rawVerdict] = - [...records[0].matchAll(pattern)][0]; - const reportedVerdict = rawVerdict.toUpperCase(); - // A stale comment must never overwrite the current revision's result. - if (reportedHead !== head || reportedBase !== base) return; - const {data: comparison} = await github.request( - 'GET /repos/{owner}/{repo}/compare/{basehead}', - {...repo, basehead: `${base}...${head}`}); - const expectedStatus = {PASS: /Passed/i, FAIL: /Warning|Error/i, INCONCLUSIVE: /Inconclusive/i}; - if (comparison.merge_base_commit?.sha === mergeBase && expectedStatus[reportedVerdict].test(row[2])) { - verdict = reportedVerdict; - reason = `Verified head ${head}, target ${base}, merge base ${mergeBase}.`; - } else { - reason = 'The reported verdict or merge base could not be verified.'; - } - } - // Recheck refs after fetching the result and history. - const {data: current} = await github.rest.pulls.get({...repo, pull_number: number}); - const {data: currentRef} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); - if (current.head.sha !== head || currentRef.object.sha !== base || - !eligible(current)) { - if (preview) core.warning('The PR or main changed during this preview; rerun for fresh evidence.'); - return; - } - const titles = { - PASS: 'No semantic conflict found (best effort)', - FAIL: 'Possible semantic conflict — CodeRabbit may be wrong', - INCONCLUSIVE: '⚠️ Semantic analysis inconclusive', - }; - const summary = `${reason}\n\n[CodeRabbit analysis](${comment.html_url})\n\n${advisory}`; - if (!preview) await github.rest.checks.update({ - ...repo, check_run_id: check.id, status: 'completed', - conclusion: {PASS: 'success', FAIL: 'failure', INCONCLUSIVE: 'neutral'}[verdict], - details_url: comment.html_url, - output: { - title: titles[verdict], - summary, - }, - }); - await core.summary.addRaw(`${preview ? 'Read-only PR preview\n\n' : ''}` + - `${titles[verdict]}\n\n${summary}\n`).write(); - core.info(`Verified semantic verdict: ${verdict}; head=${head}; target=${base}`); - if (verdict === 'FAIL') core.setFailed(`${titles[verdict]}. ${advisory} ${comment.html_url}`); - if (verdict === 'INCONCLUSIVE') core.warning(`${titles[verdict]}: ${comment.html_url}`); - - preview-result: - name: Semantic conflict preview (advisory) - if: github.event_name == 'pull_request' - permissions: - contents: read - pull-requests: read - issues: read - runs-on: ubuntu-latest - timeout-minutes: 5 - steps: *result-steps + const publish = require('./.github/scripts/coderabbit_semantic_review_result.js'); + await publish({github, context, core}); diff --git a/AGENTS.md b/AGENTS.md index 20c373d00e04..b157de94b5b2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -205,23 +205,28 @@ Only requested revision pairs receive published results; unrelated manual fixture experiments do not change a PR's semantic Check. Missing results stay neutral, and stale results cannot mark a newer pair as passing. -`CodeRabbit Semantic Review Tests` is a separate, read-only workflow for the -automation's unit tests; passing it is not a semantic verdict. These tests -are not part of the existing `Pre-commit Check`. The custom check requires CodeRabbit Custom Pre-Merge Checks access. This advisory pilot does not provide a merge-queue check or block merges. -Changes to the semantic automation also run `Semantic conflict preview -(advisory)` on `pull_request`, including fork drafts. It reads real CodeRabbit -replies for the current head/main pair with read-only permissions and uses the -same verification and verdict mapping as the publisher, without writing Checks. -Before the configuration is merged, request `@coderabbitai evaluate custom -pre-merge check` with `--name "Semantic conflict with target branch"`, -`--mode warning`, and `--instructions` containing the check instructions from -`.coderabbit.yaml` and the current head/main SHAs. After the reply arrives, -rerun the preview job. A missing reply produces a warning and no AI verdict; -a verified conflict makes the preview job red. Previewing does not validate -the production event triggers or privileged Check writes. +Changes to the semantic automation run the separate, read-only `CodeRabbit +Semantic Review Preview` workflow on `pull_request`, including fork drafts. +It shows two checks: `Automation tests and result lookup (not AI approval)` +runs the unit tests and reads real CodeRabbit replies; the `AI` check is green +only for a verified PASS and red for a verified FAIL. A missing, stale, or +inconclusive verdict leaves `AI - no current verdict (advisory)` skipped (gray). +The lookup job succeeding is not an AI pass. These tests are not part of +`Pre-commit Check`. The production request/publish workflow does not run on +`pull_request`, so its unrelated jobs do not appear in the preview. + +Both workflows use the same result verifier. The production publisher loads it +from the trusted workflow commit; the fork preview has read-only permissions +and writes no custom Checks. Before the configuration is merged, request +`@coderabbitai evaluate custom pre-merge check` with `--name "Semantic conflict +with target branch"`, `--mode warning`, and `--instructions` containing the check +instructions from `.coderabbit.yaml` and the current head/main SHAs. After the +reply arrives, rerun the **tests and result lookup** job to refresh its outputs +and dependent AI check. Rerunning only the AI check reuses the previous outputs. +Previewing does not validate production event triggers or privileged Check writes. ### Trouble Shooting From 92af46e04f6f4a974c0144586b52df7d266b2797 Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Thu, 17 Sep 2026 11:18:00 +0800 Subject: [PATCH 10/21] [None][infra] Keep skipped AI verdict names readable Signed-off-by: Yanchao Lu --- .github/workflows/coderabbit-semantic-review-tests.yml | 5 +---- AGENTS.md | 3 ++- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/.github/workflows/coderabbit-semantic-review-tests.yml b/.github/workflows/coderabbit-semantic-review-tests.yml index d745427fc0f1..040d64a112ec 100644 --- a/.github/workflows/coderabbit-semantic-review-tests.yml +++ b/.github/workflows/coderabbit-semantic-review-tests.yml @@ -60,10 +60,7 @@ jobs: await readResult({github, context, core}); verdict: - name: >- - ${{ needs.test.outputs.verdict == 'PASS' && 'AI - no conflict found (advisory)' || - needs.test.outputs.verdict == 'FAIL' && 'AI - possible conflict (advisory)' || - 'AI - no current verdict (advisory)' }} + name: AI verdict (advisory; skipped = unavailable) needs: test if: >- github.event_name == 'pull_request' && diff --git a/AGENTS.md b/AGENTS.md index b157de94b5b2..06f999d017f4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -213,7 +213,8 @@ Semantic Review Preview` workflow on `pull_request`, including fork drafts. It shows two checks: `Automation tests and result lookup (not AI approval)` runs the unit tests and reads real CodeRabbit replies; the `AI` check is green only for a verified PASS and red for a verified FAIL. A missing, stale, or -inconclusive verdict leaves `AI - no current verdict (advisory)` skipped (gray). +inconclusive verdict leaves `AI verdict (advisory; skipped = unavailable)` +skipped (gray). The lookup job succeeding is not an AI pass. These tests are not part of `Pre-commit Check`. The production request/publish workflow does not run on `pull_request`, so its unrelated jobs do not appear in the preview. From c8fc965a0e86e28b88b0d165c43be5175be9d1e4 Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Thu, 17 Sep 2026 17:23:52 +0800 Subject: [PATCH 11/21] [None][infra] Rate-limit semantic reviews and audit merged revisions Signed-off-by: Yanchao Lu --- .coderabbit.yaml | 16 +- .../coderabbit_semantic_review.test.js | 658 +++++++++--------- .../coderabbit_semantic_review_request.js | 193 +++++ .../coderabbit_semantic_review_result.js | 274 +++++--- .../coderabbit-semantic-review-tests.yml | 4 +- .../workflows/coderabbit-semantic-review.yml | 168 ++--- AGENTS.md | 118 ++-- 7 files changed, 832 insertions(+), 599 deletions(-) create mode 100644 .github/scripts/coderabbit_semantic_review_request.js diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 34e7d853c7a5..0d46304b5da5 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -77,19 +77,19 @@ reviews: drafts: false base_branches: ["main", "release/.+"] - # Best-effort semantic compatibility; rerun when the target branch advances. - # @coderabbitai run pre-merge checks + # Scheduled/on-demand evaluation is requested by coderabbit-semantic-review.yml. + # Keep this off during ordinary reviews so they cannot bypass its cost policy. pre_merge_checks: custom_checks: - name: "Semantic conflict with target branch" - mode: warning + mode: "off" instructions: | Detect behavioral incompatibilities when this PR is combined with its - current target branch, even when Git can merge without text conflicts. + requested target revision, even when Git can merge without text conflicts. Use repository and Git evidence, not claims in the PR description. - Resolve and report the PR head SHA, current target branch SHA, and - merge-base SHA. Verify the live target and PR head; a cached local ref - or the target SHA from an earlier review is not sufficient. If these + Resolve and report the requested PR head SHA, target SHA, and + merge-base SHA. Independently verify these fixed revisions; do not + substitute newer live refs or revisions from an earlier review. If these revisions or necessary code/history are unavailable, return Inconclusive and explain the missing evidence. Never invent a SHA or claim freshness. @@ -108,7 +108,7 @@ reviews: Do not execute repository code or claim tests were run. Pass only if the required context was inspected and no supported conflict was found; this is not proof of semantic compatibility. - If either branch changes during analysis, return Inconclusive. + Analyze the requested historical pair even if live refs advance. State that the result applies only to the reported SHA pair and must be rerun after either branch changes. Explain that CodeRabbit can make mistakes, including false positives. diff --git a/.github/scripts/coderabbit_semantic_review.test.js b/.github/scripts/coderabbit_semantic_review.test.js index 4350644422d8..e65830ca83f9 100644 --- a/.github/scripts/coderabbit_semantic_review.test.js +++ b/.github/scripts/coderabbit_semantic_review.test.js @@ -17,409 +17,405 @@ const assert = require('node:assert/strict'); const {test} = require('node:test'); const fs = require('node:fs'); const path = require('node:path'); -const workflow = fs.readFileSync( - path.join(__dirname, '../workflows/coderabbit-semantic-review.yml'), 'utf8'); -const scripts = [...workflow.matchAll(/^ {10}script: \|\n((?: {12}[^\n]*(?:\n|$)|\n)*)/gm)] - .map(match => match[1].split('\n').map(line => line.replace(/^ {12}/, '')).join('\n')); -assert.equal(scripts.length, 2); -const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor; -const execute = new AsyncFunction('github', 'context', 'core', 'process', scripts[0]); +const request = require('./coderabbit_semantic_review_request.js'); const publish = require('./coderabbit_semantic_review_result.js'); +const {AUDIT, requests} = publish; +const APPROVED = 'ci: full pre-merge approved'; +const HEAD = 'a'.repeat(40), BASE = 'b'.repeat(40), NEW_BASE = 'c'.repeat(40); +const MERGED = 'd'.repeat(40), MERGE_BASE = 'e'.repeat(40), TREE = 'f'.repeat(40); +const HOUR = 3600000, DAY = 24 * HOUR; +const NOW = Date.parse('2026-09-17T08:00:00Z'); +const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor; -const HEAD = 'a'.repeat(40); -const BASE = 'b'.repeat(40); -const NEW_BASE = 'c'.repeat(40); -const MERGE_BASE = 'e'.repeat(40); +function resultBody(verdict, pair) { + const status = {PASS: '✅ Passed', FAIL: '⚠️ Warning', INCONCLUSIVE: '❓ Inconclusive'}[verdict]; + return '\n' + + `| Semantic Conflict With Target Branch | ${status} | Explanation preview |\n` + + '
\nFull details: Semantic Conflict With Target Branch\n' + + `SEMANTIC_RESULT head=${pair.head} target=${pair.target} merge_base=${pair.mergeBase} verdict=${verdict}\n` + + (pair.merged ? `SEMANTIC_MERGED sha=${pair.merged}\n` : '') + + 'Code evidence and a minimal regression input.\n
'; +} -/** Run the actual workflow script against an in-memory GitHub API. */ +// Execute the production request and publication modules against an in-memory API. function harness(overrides = {}) { - const pr = {number: 12, state: 'open', draft: false, - head: {sha: HEAD}, base: {ref: 'main', sha: 'outdated-event-sha'}, - labels: [{name: 'ai: semantic-conflict'}], ...overrides}; - const prs = [pr]; - const comments = []; - const posted = []; - const checks = []; - let target = BASE; - const calls = []; + const pr = {number: 12, state: 'open', merged: false, draft: false, + head: {sha: HEAD}, base: {ref: 'main'}, labels: [], merge_commit_sha: MERGED, + auto_merge: null, ...overrides}; + const prs = [pr], comments = [], checks = [], posted = [], writes = [], calls = []; + const outputs = {}, warnings = [], failures = [], summaries = []; + let target = BASE, now = NOW, count = 30, age = 0, progressCount = 1; + let mergeTree = TREE, expectedMergeBase = MERGE_BASE, afterCompare = () => {}; + let rules = [{type: 'pull_request', parameters: {allowed_merge_methods: ['squash']}}]; + let mergeParents; const github = { rest: { - checks: {create: async args => {checks.push(args);}}, - pulls: { - list: 'list-pulls', - get: async args => { - calls.push(['get-pr', args]); - return {data: prs.find(p => p.number === args.pull_number)}; - }, - }, - git: {getRef: async args => { - calls.push(['get-ref', args]); - return {data: {object: {sha: target}}}; + pulls: {list: 'pulls', get: async args => { + calls.push(['pull', args]); + const found = prs.find(p => p.number === args.pull_number); + if (!found) throw Object.assign(new Error('Not Found'), {status: 404}); + return {data: structuredClone(found)}; }}, + git: { + getRef: async args => { calls.push(['ref', args]); return {data: {object: {sha: target}}}; }, + getCommit: async args => { calls.push(['commit', args]); return {data: { + sha: args.commit_sha, tree: {sha: mergeTree}, parents: mergeParents || + (pr.merged ? [{sha: BASE}] : [{sha: target}, {sha: pr.head.sha}]), + }}; }, + }, issues: { - listComments: 'list-comments', + listComments: 'comments', getComment: async args => ({data: comments.find(c => c.id === args.comment_id)}), createComment: async args => { - posted.push(args); - comments.push({body: args.body, user: {login: 'github-actions[bot]'}}); - return {data: {html_url: `https://github.com/example/repo/pull/${args.issue_number}#${posted.length}`}}; + posted.push(args); writes.push(['comment', args]); + const comment = {id: comments.length + 1, issue_number: args.issue_number, body: args.body, + user: {login: 'github-actions[bot]', type: 'Bot'}, created_at: new Date(now).toISOString(), + html_url: `https://github.com/example/repo/pull/${args.issue_number}#${comments.length + 1}`}; + comments.push(comment); return {data: comment}; + }, + }, + checks: { + listForRef: 'checks', create: async args => { + const check = {...args, id: checks.length + 1, app: {slug: 'github-actions'}}; + checks.push(check); writes.push(['create', args]); return {data: check}; + }, update: async args => { + Object.assign(checks.find(c => c.id === args.check_run_id), args); + writes.push(['update', args]); return {data: {}}; }, }, }, paginate: async (method, args) => { calls.push([method, args]); - if (method === 'list-pulls') return prs; - assert.equal(method, 'list-comments'); - return comments; + if (method === 'pulls') return prs.filter(p => p.state === args.state); + if (method === 'comments') return comments.filter(c => (c.issue_number || 12) === args.issue_number); + assert.equal(method, 'checks'); + return checks.filter(c => c.head_sha === args.ref && c.name === args.check_name); }, + request: async (route, args) => { + calls.push([route, args]); + if (route.endsWith('/rules/branches/{branch}')) return {data: rules}; + assert.equal(route, 'GET /repos/{owner}/{repo}/compare/{basehead}'); + const data = args.basehead.endsWith(`...${pr.head.sha}`) ? + {behind_by: count, merge_base_commit: {sha: expectedMergeBase, + commit: {committer: {date: new Date(NOW - age).toISOString()}}}} : + {status: progressCount ? 'ahead' : 'identical', ahead_by: progressCount}; + afterCompare(); return {data}; + }, + }; + github.paginate.iterator = async function* () { + yield {data: prs.filter(p => p.state === 'closed')}; }; - const summaries = []; - const core = {info: () => {}, summary: { - addRaw(text) {summaries.push(text); return this;}, - async write() {}, - }}; - const context = {repo: {owner: 'example', repo: 'repo'}, - eventName: 'pull_request_target', payload: {pull_request: {number: 12}}}; - return {pr, prs, comments, posted, checks, calls, summaries, - setTarget: sha => {target = sha;}, - run: (eventName = 'pull_request_target', pullNumber = '') => - execute(github, {...context, eventName}, core, {env: {DISPATCH_PULL_NUMBER: pullNumber}}), + const core = {setOutput: (k, v) => {outputs[k] = v;}, info() {}, + warning: v => warnings.push(v), error: v => warnings.push(v), setFailed: v => failures.push(v), + summary: {addRaw(v) {summaries.push(v); return this;}, async write() {}}}; + const context = {repo: {owner: 'example', repo: 'repo'}, eventName: 'pull_request_target', + payload: {action: 'opened', pull_request: {number: 12, head: {sha: HEAD}}}}; + return {pr, prs, comments, checks, posted, writes, calls, outputs, warnings, failures, summaries, github, core, + setTarget: v => {target = v;}, setNow: v => {now = v;}, setLag: (n, a) => {count = n; age = a;}, + setProgress: v => {progressCount = v;}, setTree: v => {mergeTree = v;}, + setRules: v => {rules = v;}, setParents: v => {mergeParents = v;}, + setMergeBase: v => {expectedMergeBase = v;}, afterCompare: fn => {afterCompare = fn;}, + async run(eventName = 'pull_request_target', action = 'opened', manual = '12', authorized = false) { + process.env.DISPATCH_PULL_NUMBER = manual; + process.env.SEMANTIC_APPROVAL_VALIDATED = String(authorized); + try { + await request({github, core, now, context: {...context, eventName, + payload: {...context.payload, action, label: {name: APPROVED}}}}); + } finally { + delete process.env.DISPATCH_PULL_NUMBER; + delete process.env.SEMANTIC_APPROVAL_VALIDATED; + } + }, + reply(verdict = 'PASS', pair = requests(comments)[0] || + {head: pr.head.sha, target, mergeBase: MERGE_BASE}) { + const comment = {id: comments.length + 1, body: resultBody(verdict, pair), + created_at: new Date(now).toISOString(), user: {login: 'coderabbitai[bot]', type: 'Bot'}, + html_url: `https://github.com/example/repo/pull/12#${comments.length + 1}`}; + comments.push(comment); return comment; + }, + publish: (comment, preview = false) => publish({github, core, context: { + ...context, eventName: preview ? 'pull_request' : 'issue_comment', + payload: {...context.payload, issue: {number: 12}, comment: {id: comment?.id}}, + }}), }; } -test('requests the live main SHA and never treats dispatch as an AI verdict', async () => { - const h = harness(); - await h.run(); - assert.equal(h.posted.length, 1); - assert.match(h.posted[0].body, /^@coderabbitai run pre-merge checks/); - assert.ok(h.posted[0].body.includes(HEAD)); - assert.ok(h.posted[0].body.includes(BASE)); - assert.ok(!h.posted[0].body.includes('outdated-event-sha')); - assert.match(h.posted[0].body, /return Inconclusive/); - assert.match(h.summaries[0], /No AI verdict is asserted/); - assert.equal(h.checks[0].conclusion, 'neutral'); - assert.equal(h.checks[0].head_sha, HEAD); - assert.match(h.checks[0].output.summary, /No AI verdict yet/); - assert.deepEqual(h.calls.find(c => c[0] === 'get-ref')[1], - {owner: 'example', repo: 'repo', ref: 'heads/main'}); +for (const [count, age, expected] of [[0, 3 * DAY, 0], [29, DAY - 1, 0], + [30, 0, 1], [1, DAY, 1], [29, DAY, 1]]) { + test(`first analysis: ${count} target commits, age ${age} => ${expected} requests`, async () => { + const h = harness(); h.setLag(count, age); await h.run(); + assert.equal(h.posted.length, expected); + assert.equal(h.checks[0].conclusion, 'neutral'); + if (expected) { + assert.match(h.posted[0].body, /^@coderabbitai evaluate custom pre-merge check/); + assert.match(h.posted[0].body, /--mode warning/); + assert.match(h.posted[0].body, /No AI verdict is asserted/); + assert.ok(h.posted[0].body.includes(HEAD) && h.posted[0].body.includes(BASE)); + } + }); +} + +test('creation and ready events use the threshold; drafts and unrelated targets are excluded', async () => { + for (const action of ['opened', 'ready_for_review']) { + const h = harness(); h.setLag(1, 0); await h.run('pull_request_target', action); + assert.equal(h.posted.length, 0); + } + for (const overrides of [{draft: true}, {state: 'closed'}, {base: {ref: 'feature/a'}}]) { + const h = harness(overrides); await h.run(); + assert.equal(h.writes.length, 0); + await assert.rejects(h.run('workflow_dispatch'), /requires a non-draft open or merged/); + } }); -test('a main-only update triggers another review with unchanged PR head', async () => { - const h = harness(); - await h.run('push'); - h.setTarget(NEW_BASE); - await h.run('push'); - assert.equal(h.posted.length, 2); - assert.ok(h.posted[1].body.includes(`${HEAD}:${NEW_BASE}`)); - assert.ok(!h.posted[1].body.includes(BASE)); +test('release PRs use their actual target and need no opt-in label', async () => { + const h = harness({base: {ref: 'release/1.2'}}); await h.run(); + assert.equal(h.posted.length, 1); + assert.ok(h.calls.filter(([kind]) => kind === 'ref').every(([, args]) => args.ref === 'heads/release/1.2')); + const reply = h.reply(); await h.publish(reply); + assert.equal(h.checks[0].conclusion, 'success'); }); -test('a PR-only update triggers another review', async () => { - const h = harness(); - await h.run(); - h.pr.head.sha = 'd'.repeat(40); - await h.run(); - assert.equal(h.posted.length, 2); - assert.ok(h.posted[1].body.includes(h.pr.head.sha)); +test('an hourly scan visits eligible PRs; a PR event or manual retry visits only its PR', async () => { + const h = harness(); h.prs.push({...h.pr, number: 13}); await h.run(); + assert.deepEqual(h.posted.map(c => c.issue_number), [12]); + await h.run('schedule'); assert.deepEqual(h.posted.map(c => c.issue_number), [12, 13]); + await h.run('workflow_dispatch', '', '13'); + assert.deepEqual(h.posted.map(c => c.issue_number), [12, 13, 13]); }); -test('duplicates are suppressed, but workflow dispatch retries the same pair', async () => { - const h = harness(); - await h.run(); - await h.run('push'); - assert.equal(h.posted.length, 1); - await h.run('workflow_dispatch', '12'); - assert.equal(h.posted.length, 2); +test('dispatch rejects malformed or missing PRs without touching other PRs', async () => { + for (const raw of ['', '0', '-1', '12x', '9007199254740992']) { + const h = harness(); await assert.rejects(h.run('workflow_dispatch', '', raw), /positive integer/); + assert.equal(h.writes.length, 0); + } + const h = harness(); await assert.rejects(h.run('workflow_dispatch', '', '99'), /Not Found/); + assert.equal(h.writes.length, 0); + await assert.rejects(h.run('push'), /Unsupported event/); }); -test('a marker posted by another user cannot suppress requests', async () => { - const h = harness(); - h.comments.push({body: ``, - user: {login: 'someone-else'}}); - await h.run(); +test('subsequent thresholds use the last completed analysis, not the original old base', async () => { + const h = harness(); h.setLag(80, 3 * DAY); await h.run(); h.reply(); + h.setTarget(NEW_BASE); h.setNow(NOW + 2 * HOUR); h.setProgress(1); await h.run('schedule'); assert.equal(h.posted.length, 1); + assert.equal(h.checks[0].conclusion, 'neutral'); + assert.match(h.checks[0].output.title, /stale/); + h.setProgress(30); await h.run('schedule'); assert.equal(h.posted.length, 2); }); -test('closed, draft, unlabelled, and release PRs are not reviewed', async () => { - for (const overrides of [{state: 'closed'}, {draft: true}, {labels: []}, - {base: {ref: 'release/1.0'}}]) { - const h = harness(overrides); - await h.run(); - await assert.rejects(h.run('workflow_dispatch', '12'), /retry not posted/); - assert.equal(h.posted.length, 0); - assert.ok(!h.calls.some(c => c[0] === 'get-ref')); +test('24 hours with new target commits triggers, while an unchanged target never does', async () => { + for (const [progress, expected] of [[0, 1], [1, 2]]) { + const h = harness(); await h.run(); h.reply(); h.setNow(NOW + DAY); + h.setTarget(NEW_BASE); h.setProgress(progress); await h.run('schedule'); + assert.equal(h.posted.length, expected); } }); -test('invalid dispatch inputs and unsupported events cannot post comments', async () => { - for (const value of ['', '0', '-1', '1.2', '12x', '9007199254740992']) { - const h = harness(); - await assert.rejects(h.run('workflow_dispatch', value), /positive integer/); - assert.equal(h.posted.length, 0); - } - await assert.rejects(harness().run('issue_comment'), /Unsupported event/); +test('a PR head update invalidates the verdict without bypassing the target threshold', async () => { + const h = harness(); await h.run(); await h.publish(h.reply()); + h.pr.head.sha = MERGED; h.setNow(NOW + 2 * HOUR); h.setProgress(0); await h.run(); + assert.equal(h.posted.length, 1); + assert.equal(h.checks.at(-1).head_sha, MERGED); + assert.equal(h.checks.at(-1).conclusion, 'neutral'); }); -test('a nonexistent manual retry cannot affect another PR', async () => { - const h = harness(); - await assert.rejects(h.run('workflow_dispatch', '99'), /PR #99: retry not posted/); - assert.deepEqual(h.posted, []); - assert.deepEqual(h.checks, []); +test('approval and auto-merge bypass thresholds but share a one-hour cooldown across SHAs', async () => { + const h = harness({labels: [{name: APPROVED}], auto_merge: {enabled_by: 'maintainer'}}); + h.setLag(0, 0); await h.run('pull_request_target', 'labeled', '12', true); + assert.equal(h.posted.length, 1); + h.setTarget(NEW_BASE); h.setNow(NOW + HOUR - 1); + await h.run('pull_request_target', 'auto_merge_enabled'); + assert.equal(h.posted.length, 1); assert.equal(h.checks.at(-1).conclusion, 'neutral'); + h.setNow(NOW + HOUR); await h.run('pull_request_target', 'auto_merge_enabled'); + assert.equal(h.posted.length, 2); }); -test('a coalesced event sweeps all opted-in PRs; manual retry is limited to its PR', async () => { - const h = harness(); - h.prs.push({...h.pr, number: 13, head: {sha: 'd'.repeat(40)}}); - await h.run(); - assert.deepEqual(h.posted.map(c => c.issue_number), [12, 13]); - h.setTarget(NEW_BASE); - await h.run(); - assert.deepEqual(h.posted.map(c => c.issue_number), [12, 13, 12, 13]); - await h.run('workflow_dispatch', '13'); - assert.deepEqual(h.posted.map(c => c.issue_number), [12, 13, 12, 13, 13]); +test('an unvalidated approval label cannot bypass the threshold', async () => { + const h = harness({labels: [{name: APPROVED}]}); h.setLag(0, 0); + await h.run('pull_request_target', 'labeled'); assert.equal(h.posted.length, 0); }); -test('a manual retry cannot request an unrelated PR without an existing marker', async () => { - const h = harness(); - h.prs.push({...h.pr, number: 13}); - await h.run('workflow_dispatch', '13'); - assert.deepEqual(h.posted.map(comment => comment.issue_number), [13]); +test('pending exact pairs deduplicate, manual retry bypasses cooldown and clears a pass', async () => { + const h = harness(); await h.run(); await h.run(); assert.equal(h.posted.length, 1); + await h.publish(h.reply()); assert.equal(h.checks[0].conclusion, 'success'); + await h.run('workflow_dispatch'); assert.equal(h.posted.length, 2); + assert.equal(h.checks[0].conclusion, 'neutral'); }); -/** Build the observed CodeRabbit result format with a verifiable revision record. */ -function resultBody(verdict = 'FAIL', head = HEAD, target = BASE) { - const status = {PASS: '✅ Passed', FAIL: '⚠️ Warning', INCONCLUSIVE: '❓ Inconclusive'}[verdict]; - return '\n' + - `| Semantic Conflict With Target Branch | ${status} | Explanation preview |\n` + - '
\nFull details: Semantic Conflict With Target Branch\n' + - `SEMANTIC_RESULT head=${head} target=${target} merge_base=${MERGE_BASE} verdict=${verdict}\n` + - 'Evidence and a minimal regression input.\n
'; -} +test('missing analysis cannot repeatedly spend AI calls as the target changes each hour', async () => { + const h = harness(); await h.run(); h.setNow(NOW + 2 * HOUR); h.setTarget(NEW_BASE); + await h.run('schedule'); assert.equal(h.posted.length, 1); +}); -/** Exercise the result publisher without checking out or executing PR code. */ -function resultHarness(body = resultBody()) { - const request = harness(); - const comment = {id: 123, body, user: {login: 'coderabbitai[bot]', type: 'Bot'}, - html_url: 'https://github.com/example/repo/pull/12#issuecomment-123'}; - const comments = [comment]; - const checks = [{id: 1, app: {slug: 'github-actions'}, - external_id: `semantic-conflict:12:${HEAD}:${BASE}`}]; - const updated = []; - const outputs = {}; - const warnings = []; - const failures = []; - const summaries = []; - let mergeBase = MERGE_BASE; - let target = BASE; - let afterCompare = () => {}; - const github = { - rest: { - issues: {getComment: async () => ({data: comment}), listComments: 'list-comments'}, - pulls: {get: async () => ({data: request.pr})}, - git: {getRef: async () => ({data: {object: {sha: target}}})}, - checks: {listForRef: 'list-checks', update: async args => {updated.push(args);}}, - }, - paginate: async method => method === 'list-comments' ? comments : checks, - request: async (route, args) => { - assert.equal(route, 'GET /repos/{owner}/{repo}/compare/{basehead}'); - assert.equal(args.basehead, `${BASE}...${HEAD}`); - afterCompare(); - return {data: {merge_base_commit: {sha: mergeBase}}}; - }, - }; - const core = {setOutput: (key, value) => {outputs[key] = value;}, info: () => {}, - warning: text => warnings.push(text), setFailed: text => failures.push(text), - summary: {addRaw(text) {summaries.push(text); return this;}, async write() {}}}; - return {comment, comments, checks, updated, warnings, failures, summaries, outputs, pr: request.pr, - setMergeBase: sha => {mergeBase = sha;}, - setTarget: sha => {target = sha;}, - afterCompare: callback => {afterCompare = callback;}, - run: (eventName = 'issue_comment') => publish({github, context: {repo: {owner: 'example', repo: 'repo'}, - eventName, payload: {issue: {number: 12}, comment: {id: 123}, - pull_request: {number: 12, head: {sha: HEAD}}}}, core}), - }; +test('untrusted or malformed request markers cannot suppress analysis', async () => { + const h = harness(); await h.run(); h.comments[0].user.login = 'someone-else'; + h.comments.push({id: 100, user: {login: 'github-actions[bot]', type: 'Bot'}, + body: ''}); + await h.run(); assert.equal(h.posted.length, 2); +}); + +for (const verdict of ['PASS', 'FAIL', 'INCONCLUSIVE']) { + test(`verified ${verdict} maps to the advisory conclusion and notices`, async () => { + const h = harness(); await h.run(); const reply = h.reply(verdict); + reply.body = reply.body.toLowerCase(); await h.publish(reply); + assert.equal(h.checks[0].conclusion, {PASS: 'success', FAIL: 'failure', INCONCLUSIVE: 'neutral'}[verdict]); + assert.equal(h.failures.length, verdict === 'FAIL' ? 1 : 0); + assert.match(h.checks[0].output.summary, /false positives/); + assert.match(h.checks[0].output.summary, /non-required; its failure does not block merging/); + }); } -test('verified conflicts publish failure with a false-positive and non-blocking notice', async () => { - for (const body of [resultBody('FAIL'), resultBody('FAIL').toLowerCase()]) { - const h = resultHarness(body); - await h.run(); - assert.equal(h.updated[0].conclusion, 'failure'); - assert.match(h.updated[0].output.title, /Possible semantic conflict/); - assert.match(h.updated[0].output.summary, /CodeRabbit can make mistakes, including false positives/); - assert.match(h.updated[0].output.summary, /must remain non-required; its failure does not block merging/); - assert.equal(h.updated[0].details_url, h.comment.html_url); - assert.equal(h.failures.length, 1); - assert.match(h.failures[0], /does not block merging/); - assert.match(h.summaries[0], /false positives/); +test('malformed, contradictory, stale and untrusted results never publish a pass', async () => { + for (const corrupt of [ + c => {c.user.login = 'attacker';}, c => {c.user.type = 'User';}, + c => {c.body = c.body.replace('', '');}, + c => {c.body = c.body.replace('✅ Passed', '❓ Inconclusive');}, + c => {c.body = c.body.replaceAll(HEAD, NEW_BASE);}, + c => {c.body = c.body.replaceAll(MERGE_BASE, NEW_BASE);}, + c => {c.body = c.body.replace('', c.body.replace('PASS', 'FAIL') + '');}, + ]) { + const h = harness(); await h.run(); const reply = h.reply(); corrupt(reply); await h.publish(reply); + assert.equal(h.checks[0].conclusion, 'neutral'); } + const h = harness(); await h.run(); const reply = h.reply(); h.comments.shift(); + await h.publish(reply); assert.equal(h.checks[0].conclusion, 'neutral'); }); -test('inconclusive results stay neutral and warn without failing the job', async () => { - const h = resultHarness(resultBody('INCONCLUSIVE')); - await h.run(); - assert.equal(h.updated[0].conclusion, 'neutral'); - assert.match(h.updated[0].output.title, /inconclusive/); - assert.equal(h.warnings.length, 1); - assert.equal(h.failures.length, 0); -}); - -test('only a verified pass publishes success, including a passed table without full details', async () => { - const body = resultBody('PASS'); - const record = body.match(/SEMANTIC_RESULT[^\n]+/)[0]; - for (const text of [body, '\n' + - `| Semantic conflict with target branch | ✅ Passed | ${record} |`]) { - const h = resultHarness(text); - await h.run(); - assert.equal(h.updated[0].conclusion, 'success'); - assert.equal(h.warnings.length, 0); - assert.equal(h.failures.length, 0); +test('live ref changes during verification cannot publish a current pass', async () => { + for (const change of [h => {h.pr.head.sha = NEW_BASE;}, h => h.setTarget(NEW_BASE)]) { + const h = harness(); await h.run(); const reply = h.reply(); + h.afterCompare(() => change(h)); await h.publish(reply); + assert.equal(h.checks[0].conclusion, 'neutral'); } }); -test('stale head or target results never overwrite the current check', async () => { - for (const body of [resultBody('PASS', NEW_BASE), resultBody('PASS', HEAD, NEW_BASE)]) { - const h = resultHarness(body); - await h.run(); - assert.deepEqual(h.updated, []); - } +test('post-merge audit ignores thresholds and cooldown and pins the historical target', async () => { + const h = harness({merged: true, state: 'closed', base: {ref: 'release/1.2'}}); + h.setTarget(NEW_BASE); h.setLag(0, 0); await h.run('pull_request_target', 'closed'); + const pair = requests(h.comments)[0]; + assert.equal(pair.target, BASE); assert.equal(pair.merged, MERGED); + assert.equal(h.checks[0].name, AUDIT); assert.equal(h.checks[0].head_sha, MERGED); + assert.ok(!h.calls.some(([kind]) => kind === 'ref')); + const reply = h.reply('FAIL'); await h.publish(reply); + assert.equal(h.checks[0].conclusion, 'failure'); + assert.match(h.posted.at(-1).body, /Post-merge semantic audit: FAIL/); + await h.publish(reply); assert.equal(h.posted.length, 2); // No duplicate audit receipt. }); -test('missing, contradictory, or invalid evidence cannot publish a pass', async () => { - for (const body of [ - resultBody('PASS').replace(/SEMANTIC_RESULT[^\n]+/, 'Clone failed'), - resultBody('PASS').replace('✅ Passed', '❓ Inconclusive'), - resultBody('PASS').replace('', resultBody('FAIL') + ''), - ]) { - const h = resultHarness(body); - await h.run(); - assert.equal(h.updated[0].conclusion, 'neutral'); - } - const h = resultHarness(resultBody('PASS')); - h.setMergeBase(NEW_BASE); - await h.run(); - assert.equal(h.updated[0].conclusion, 'neutral'); +test('a matching pre-merge result and actual tree can serve the audit without another AI call', async () => { + const h = harness(); await h.run(); const reply = h.reply(); await h.publish(reply); + h.pr.merged = true; h.pr.state = 'closed'; h.setTarget(NEW_BASE); + await h.run('pull_request_target', 'closed'); + assert.equal(h.posted.filter(c => c.body.startsWith('@coderabbitai')).length, 1); + assert.equal(h.checks.at(-1).name, AUDIT); assert.equal(h.checks.at(-1).conclusion, 'success'); }); -test('untrusted authors, unrelated reviews, and unrequested fixtures cannot publish results', async () => { - for (const change of [ - h => {h.comment.user.login = 'someone-else';}, - h => {h.comment.user.type = 'User';}, - h => {h.comment.body = h.comment.body.replace('', '');}, - h => {h.comment.body = h.comment.body.replaceAll('Semantic Conflict With Target Branch', 'Controlled Experiment');}, - h => {h.checks.length = 0;}, - h => {h.checks[0].app.slug = 'another-app';}, - h => {h.pr.draft = true;}, - h => {h.pr.labels = [];}, - ]) { - const h = resultHarness(); - change(h); - await h.run(); - assert.deepEqual(h.updated, []); - } +test('a matching pre-merge request in flight can complete the audit after merge', async () => { + const h = harness(); await h.run(); const original = requests(h.comments)[0]; + h.pr.merged = true; h.pr.state = 'closed'; await h.run('pull_request_target', 'closed'); + assert.equal(h.posted.length, 1); assert.equal(h.checks.at(-1).conclusion, 'neutral'); + await h.publish(h.reply('PASS', original)); assert.equal(h.checks.at(-1).conclusion, 'success'); }); -test('head or main updates during analysis cannot publish a stale pass', async () => { - for (const change of [h => {h.pr.head.sha = NEW_BASE;}, h => h.setTarget(NEW_BASE)]) { - const h = resultHarness(resultBody('PASS')); - h.afterCompare(() => change(h)); - await h.run(); - assert.deepEqual(h.updated, []); +test('a changed final tree, changed target or inconclusive pre-merge result requires fresh audit', async () => { + for (const scenario of ['tree', 'target', 'inconclusive']) { + const h = harness(); await h.run(); h.reply(scenario === 'inconclusive' ? 'INCONCLUSIVE' : 'PASS'); + h.pr.merged = true; h.pr.state = 'closed'; + if (scenario === 'tree') h.setTree(NEW_BASE); + if (scenario === 'target') h.setParents([{sha: NEW_BASE}]); + await h.run('pull_request_target', 'closed'); + assert.equal(h.posted.length, 2); + assert.equal(requests(h.comments)[0].merged, MERGED); } }); -test('a retry updates only the newest matching check', async () => { - const h = resultHarness(); - h.checks.push({...h.checks[0], id: 2}); - await h.run(); - assert.equal(h.updated[0].check_run_id, 2); +test('audit refuses an unverified merge method and an audit reply for another merged commit', async () => { + const h = harness({merged: true, state: 'closed'}); h.setRules([]); + await assert.rejects(h.run('pull_request_target', 'closed'), /squash-only/); + assert.equal(h.posted.length, 0); + h.setParents([{sha: BASE}, {sha: HEAD}]); await h.run('pull_request_target', 'closed'); + const reply = h.reply(); reply.body = reply.body.replace(`sha=${MERGED}`, `sha=${NEW_BASE}`); + await h.publish(reply); assert.equal(h.checks[0].conclusion, 'neutral'); }); -test('PR preview uses the same verdict mapping without writing a Check, even for drafts', async () => { +test('read-only draft preview accepts only a verified current pair and never writes', async () => { for (const verdict of ['PASS', 'FAIL', 'INCONCLUSIVE']) { - const h = resultHarness(resultBody(verdict).toLowerCase()); - h.pr.draft = true; - h.pr.labels = []; - h.checks.length = 0; - await h.run('pull_request'); - assert.deepEqual(h.updated, []); - assert.equal(h.failures.length, 0); // The separate verdict job displays failures. - assert.equal(h.outputs.verdict, verdict); - assert.match(h.outputs.message, /false positives/); - assert.match(h.outputs.summary, /CodeRabbit analysis/); - assert.equal(h.warnings.length, verdict === 'INCONCLUSIVE' ? 1 : 0); - assert.match(h.summaries[0], /Read-only PR preview/); - assert.match(h.summaries[0], /non-required/); + const h = harness({draft: true}); const reply = h.reply(verdict); await h.publish(reply, true); + assert.equal(h.outputs.verdict, verdict); assert.equal(h.writes.length, 0); + assert.equal(h.failures.length, 0); assert.match(h.outputs.summary, /Verified head/); } + const h = harness({draft: true}); h.reply('PASS'); h.setTarget(NEW_BASE); await h.publish(null, true); + assert.equal(h.outputs.verdict, 'INCONCLUSIVE'); assert.equal(h.writes.length, 0); }); -test('PR preview cannot reuse another SHA pair, an untrusted reply, or an older PR run', async () => { - for (const change of [ - h => {h.comment.body = resultBody('FAIL', NEW_BASE);}, - h => {h.comment.body = resultBody('FAIL', HEAD, NEW_BASE);}, - h => {h.comment.user.login = 'someone-else';}, - h => {h.comment.user.type = 'User';}, - h => {h.comments.length = 0;}, - h => {h.pr.head.sha = NEW_BASE;}, - ]) { - const h = resultHarness(); - change(h); - await h.run('pull_request'); - assert.deepEqual(h.updated, []); - assert.deepEqual(h.failures, []); - assert.equal(h.warnings.length, 1); - assert.equal(h.outputs.verdict, 'INCONCLUSIVE'); +test('the real preview display script fails for conflicts and keeps the advisory notice', async () => { + const text = fs.readFileSync(path.join(__dirname, '../workflows/coderabbit-semantic-review-tests.yml'), 'utf8'); + const script = [...text.matchAll(/^ {10}script: \|\n((?: {12}[^\n]*(?:\n|$)|\n)*)/gm)].at(-1)[1] + .replace(/^ {12}/gm, ''); + const display = new AsyncFunction('core', 'process', script); + for (const verdict of ['PASS', 'FAIL']) { + const h = harness({draft: true}); await h.publish(h.reply(verdict), true); + await display(h.core, {env: {SEMANTIC_VERDICT: h.outputs.verdict, + SEMANTIC_MESSAGE: h.outputs.message, SEMANTIC_SUMMARY: h.outputs.summary}}); + assert.equal(h.failures.length, verdict === 'FAIL' ? 1 : 0); + assert.match(h.summaries.at(-1), /false positives/); } }); -test('PR preview chooses the latest matching reply and rechecks live refs', async () => { - const h = resultHarness(resultBody('FAIL')); - h.comments.push({...h.comment, id: 124, body: resultBody('PASS')}); - await h.run('pull_request'); - assert.deepEqual(h.failures, []); - assert.match(h.summaries[0], /No semantic conflict found/); - for (const change of [h => {h.pr.head.sha = NEW_BASE;}, h => h.setTarget(NEW_BASE)]) { - const stale = resultHarness(); - stale.afterCompare(() => change(stale)); - await stale.run('pull_request'); - assert.deepEqual(stale.updated, []); - assert.deepEqual(stale.failures, []); - assert.equal(stale.warnings.length, 1); - assert.equal(stale.outputs.verdict, 'INCONCLUSIVE'); - } +test('a manual retry cannot be satisfied by an older comment event', async () => { + const h = harness(); await h.run(); const oldReply = h.reply(); await h.publish(oldReply); + h.setNow(NOW + HOUR); await h.run('workflow_dispatch'); await h.publish(oldReply); + assert.equal(h.checks[0].conclusion, 'neutral'); + await h.publish(h.reply('FAIL')); assert.equal(h.checks[0].conclusion, 'failure'); }); -test('preview never exports an AI approval for malformed or contradictory evidence', async () => { - for (const body of [ - resultBody('PASS').replace('', ''), - resultBody('PASS').replace('✅ Passed', '❓ Inconclusive'), - resultBody('PASS').replace('', resultBody('FAIL') + ''), - ]) { - const h = resultHarness(body); - await h.run('pull_request'); - assert.equal(h.outputs.verdict, 'INCONCLUSIVE'); - assert.deepEqual(h.updated, []); - } +test('a new audit request cannot be overwritten by an older pre-merge result', async () => { + const h = harness(); await h.run(); const oldReply = h.reply('INCONCLUSIVE'); + h.pr.merged = true; h.pr.state = 'closed'; await h.run('pull_request_target', 'closed'); + await h.publish(h.reply('FAIL')); oldReply.body = oldReply.body.replaceAll('INCONCLUSIVE', 'PASS') + .replace('❓ Inconclusive', '✅ Passed'); + await h.publish(oldReply); assert.equal(h.checks.at(-1).conclusion, 'failure'); }); +test('the hourly scan recovers recent merged PRs without auditing old closed history', async () => { + const h = harness({merged: true, state: 'closed', merged_at: new Date(NOW - HOUR).toISOString(), + updated_at: new Date(NOW).toISOString()}); + h.prs.push({...h.pr, number: 13, merged_at: new Date(NOW - 2 * DAY).toISOString()}); + await h.run('schedule'); assert.deepEqual(h.posted.map(c => c.issue_number), [12]); +}); -test('the separate AI job displays the verified verdict and fails only for conflicts', async () => { - const preview = fs.readFileSync( - path.join(__dirname, '../workflows/coderabbit-semantic-review-tests.yml'), 'utf8'); - const displayScript = [...preview.matchAll(/^ {10}script: \|\n((?: {12}[^\n]*(?:\n|$)|\n)*)/gm)] - .at(-1)[1].replace(/^ {12}/gm, ''); - const display = new AsyncFunction('core', 'process', displayScript); - for (const verdict of ['PASS', 'FAIL']) { - const h = resultHarness(resultBody(verdict)); - await h.run('pull_request'); - const failures = []; - const summaries = []; - await display({setFailed: message => failures.push(message), summary: { - addRaw(text) {summaries.push(text); return this;}, async write() {}, - }}, {env: {SEMANTIC_VERDICT: h.outputs.verdict, - SEMANTIC_MESSAGE: h.outputs.message, SEMANTIC_SUMMARY: h.outputs.summary}}); - assert.equal(failures.length, verdict === 'FAIL' ? 1 : 0); - assert.match(summaries[0], /Verified head/); - assert.match(summaries[0], /false positives/); - if (verdict === 'FAIL') assert.match(failures[0], /does not block merging/); +test('a manual retry reports when refs change before the request is posted', async () => { + const h = harness(); h.afterCompare(() => h.setTarget(NEW_BASE)); + await assert.rejects(h.run('workflow_dispatch'), /Manual retry not posted/); + assert.equal(h.posted.length, 0); +}); + +test('crossing the time threshold updates a previously waiting Check to awaiting analysis', async () => { + const h = harness(); h.setLag(1, 0); await h.run(); + h.setNow(NOW + DAY); await h.run(); + assert.equal(h.checks.length, 1); assert.equal(h.posted.length, 1); + assert.equal(h.checks[0].output.title, 'Awaiting CodeRabbit analysis'); +}); + +test('approval validation checks the current label, actor and active membership', async () => { + const workflow = fs.readFileSync(path.join(__dirname, '../workflows/coderabbit-semantic-review.yml'), 'utf8'); + const script = [...workflow.matchAll(/^ {10}script: \|\n((?: {12}[^\n]*(?:\n|$)|\n)*)/gm)][0][1] + .replace(/^ {12}/gm, ''); + const validate = new AsyncFunction('github', 'context', 'core', script); + for (const [label, actor, membership, expected] of [ + [true, 'maintainer', 'active', true], [false, 'maintainer', 'active', false], + [true, 'different-user', 'active', false], [true, 'maintainer', 'pending', false], + [true, 'maintainer', 404, false], + ]) { + const github = {rest: { + pulls: {get: async () => ({data: {number: 12, labels: label ? [{name: APPROVED}] : []}})}, + issues: {listEventsForTimeline: 'timeline'}, + teams: {getMembershipForUser: async args => { + assert.equal(args.username, 'maintainer'); assert.equal(args.team_slug, 'trt-llm-ci-approvers'); + if (membership === 404) throw Object.assign(new Error('Not Found'), {status: 404}); + return {data: {state: membership}}; + }}, + }, paginate: async () => [{event: 'labeled', label: {name: APPROVED}, actor: {login: actor}}]}; + const actual = await validate(github, {repo: {owner: 'example', repo: 'repo'}, + payload: {pull_request: {number: 12}, sender: {login: 'maintainer'}}}, {warning() {}}); + assert.equal(actual, expected); } }); diff --git a/.github/scripts/coderabbit_semantic_review_request.js b/.github/scripts/coderabbit_semantic_review_request.js new file mode 100644 index 000000000000..8a4a10669be2 --- /dev/null +++ b/.github/scripts/coderabbit_semantic_review_request.js @@ -0,0 +1,193 @@ +// 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('node:fs'); +const publish = require('./coderabbit_semantic_review_result.js'); +const {NAME, AUDIT, NOTICE, supported, compare, requests, parseResult, matches, identity, evidence, candidateTree} = publish; +const HOUR = 60 * 60 * 1000; +const DAY = 24 * HOUR; +const APPROVED = 'ci: full pre-merge approved'; + +function command(pair) { + const config = fs.readFileSync('.coderabbit.yaml', 'utf8'); + const match = config.match(/name: "Semantic conflict with target branch"[\s\S]*?instructions: \|\n((?: {10}[^\n]*(?:\n|$)|\n)*)/); + if (!match) throw new Error('Missing semantic check instructions'); + const instructions = match[1].replace(/^ {10}/gm, '').trim() + '\n' + + `Inspect these fixed revisions: head=${pair.head}, target=${pair.target}, merge_base=${pair.mergeBase}. ` + + (pair.merged ? `This is a post-merge audit. Also inspect the actual merged code at ${pair.merged}. ` + + `Include SEMANTIC_MERGED sha=${pair.merged} immediately after the result line. ` + + 'Later changes to the target branch do not invalidate this historical audit.' : + `This is a pre-merge analysis against ${pair.branch}. The result applies only to this pair.`); + return `@coderabbitai evaluate custom pre-merge check --name "${NAME}" --mode warning ` + + `--instructions ${JSON.stringify(instructions.replace(/\s+/g, ' '))}`; +} + +async function requestOne({github, context, core, number, manual, now}) { + const repo = context.repo; + const {data: pr} = await github.rest.pulls.get({...repo, pull_number: number}); + if (!supported(pr.base.ref) || (!pr.merged && (pr.state !== 'open' || pr.draft))) { + if (manual) throw new Error(`PR #${number}: requires a non-draft open or merged main/release PR.`); + return; + } + const pair = await evidence(github, repo, pr); + const comments = await github.paginate(github.rest.issues.listComments, { + ...repo, issue_number: number, per_page: 100, + }); + const history = requests(comments).filter(r => r.branch === pair.branch); + const results = comments.toSorted((a, b) => b.id - a.id).map(parseResult).filter(Boolean); + const resultFor = r => results.find(v => matches(v, r) && v.comment.id > r.id && + Date.parse(v.comment.created_at) >= Date.parse(r.created_at)); + const reusable = history.find(r => r.head === pair.head && r.target === pair.target && + r.mergeBase === pair.mergeBase && (!pair.merged ? !r.merged : + (r.merged === pair.merged || (!r.merged && r.tree && r.tree === pair.tree)))); + const checks = await github.paginate(github.rest.checks.listForRef, { + ...repo, ref: pair.merged || pair.head, check_name: pair.merged ? AUDIT : NAME, + filter: 'all', per_page: 100, + }); + let check = checks.filter(c => c.app?.slug === 'github-actions' && + c.external_id?.startsWith(`semantic-v2:${number}:`)).sort((a, b) => b.id - a.id)[0]; + const currentId = identity(pr, pair); + // Clear a stale green/red result even if this revision is below the AI threshold. + if (check && check.external_id !== currentId && !pair.merged) { + await github.rest.checks.update({...repo, check_run_id: check.id, status: 'completed', + conclusion: 'neutral', output: {title: 'Previous semantic result is stale', + summary: `No current verdict for head ${pair.head} + ${pair.branch} ${pair.target}. ${NOTICE}`}}); + } + async function ensureCheck(reason) { + if (check?.external_id === currentId) return; + const output = {title: pair.merged ? 'Post-merge audit awaiting analysis' : + check ? 'Previous semantic result is stale' : 'No current AI verdict', + summary: `${reason} Head ${pair.head} + ${pair.branch} ${pair.target}. ${NOTICE}`}; + if (check) { + await github.rest.checks.update({...repo, check_run_id: check.id, external_id: currentId, + status: 'completed', conclusion: 'neutral', output}); + check.external_id = currentId; + } else { + ({data: check} = await github.rest.checks.create({...repo, + name: pair.merged ? AUDIT : NAME, head_sha: pair.merged || pair.head, + external_id: currentId, status: 'completed', conclusion: 'neutral', output})); + } + } + if (reusable && !manual) { + const result = resultFor(reusable); + // Reuse an in-flight exact pair too. The publisher can complete the audit + // when its pre-merge reply arrives, provided the final tree also matches. + if (!pair.merged || reusable.merged || !result || result.verdict !== 'INCONCLUSIVE') { + await ensureCheck('This version already has an analysis request.'); + if (result) await publish({github, core, context: {...context, eventName: 'issue_comment', + payload: {issue: {number}, comment: {id: result.comment.id}}}}); + core.info(`PR #${number}: reusing the exact revision pair.`); + return; + } + } + const intent = context.eventName === 'pull_request_target' && + (context.payload.action === 'auto_merge_enabled' && pr.auto_merge || + context.payload.action === 'labeled' && context.payload.label?.name === APPROVED && + pr.labels.some(l => l.name === APPROVED) && process.env.SEMANTIC_APPROVAL_VALIDATED === 'true'); + let reason = manual ? 'Manual retry' : pair.merged ? 'Post-merge audit' : intent ? 'Merge intent' : ''; + const last = history.find(r => !r.merged); + if (!reason) { + const completed = history.find(r => !r.merged && ['PASS', 'FAIL'].includes(resultFor(r)?.verdict)); + if (last && !completed && now - Date.parse(last.created_at) < DAY) { + await ensureCheck('No completed analysis; new-pair requests wait 24 hours. Manual retry remains available.'); + return; + } + let count = pair.comparison.behind_by; + let since = Date.parse(pair.comparison.merge_base_commit.commit.committer.date); + if (completed) { + const progress = await compare(github, repo, completed.target, pair.target); + if (['ahead', 'identical'].includes(progress.status)) { + count = progress.ahead_by; + since = Date.parse(resultFor(completed).comment.created_at); + } + } + if (!count || (count < 30 && now - since < DAY)) { + await ensureCheck('Below the 24-hour / 30-commit threshold; waiting for target changes.'); + return; + } + reason = '24-hour / 30-commit threshold'; + } + // Approval and auto-merge share this budget even when the branch SHAs change. + // Count any recent pre-merge request, so a routine scan cannot double the cost. + if (!manual && !pair.merged && last && now - Date.parse(last.created_at) < HOUR) { + await ensureCheck('Pre-merge analysis is in its one-hour cooldown; old results are stale.'); + return; + } + const {data: current} = await github.rest.pulls.get({...repo, pull_number: number}); + if (current.head.sha !== pair.head || current.base.ref !== pair.branch || + current.state !== pr.state || current.merged !== pr.merged || current.draft !== pr.draft || + (pair.merged && current.merge_commit_sha !== pair.merged)) { + if (manual) throw new Error('Manual retry not posted: PR changed during validation.'); + return; + } + if (!pair.merged) { + const {data: ref} = await github.rest.git.getRef({...repo, ref: `heads/${pair.branch}`}); + if (ref.object.sha !== pair.target) { + if (manual) throw new Error('Manual retry not posted: target changed during validation.'); + return; + } + } + await ensureCheck(reason); + await github.rest.checks.update({...repo, check_run_id: check.id, + status: 'completed', conclusion: 'neutral', + output: {title: 'Awaiting CodeRabbit analysis', + summary: `${reason}: head ${pair.head}, target ${pair.target}. ${NOTICE}`}}); + if (!pair.merged) pair.tree = await candidateTree(github, repo, current, pair.target); + const {comparison, ...record} = pair; + const {data: comment} = await github.rest.issues.createComment({...repo, issue_number: number, + body: `${command(pair)}\n\n\n\n` + + `${reason} for PR #${number}. ${NOTICE} No AI verdict is asserted by posting this request.`}); + core.info(`PR #${number}: ${reason}; ${comment.html_url}`); + await core.summary.addRaw(`PR #${number}: [${reason}](${comment.html_url}). No AI verdict is asserted.\n\n`).write(); +} + +module.exports = async ({github, context, core, now = Date.now()}) => { + let numbers; + const manual = context.eventName === 'workflow_dispatch'; + if (manual) { + const raw = (process.env.DISPATCH_PULL_NUMBER || '').trim(); + const number = Number(raw); + if (!/^[1-9][0-9]*$/.test(raw) || !Number.isSafeInteger(number)) { + throw new Error('pull_number must be a positive integer'); + } + numbers = [number]; + } else if (context.eventName === 'pull_request_target') { + numbers = [context.payload.pull_request.number]; + } else if (context.eventName === 'schedule') { + const pulls = await github.paginate(github.rest.pulls.list, { + ...context.repo, state: 'open', per_page: 100, + }); + numbers = pulls.filter(p => !p.draft && supported(p.base.ref)).map(p => p.number); + // Recover recent merges if an event was dropped or a release branch still + // lacks the workflow. Do not backfill the repository's entire merge history. + for await (const response of github.paginate.iterator(github.rest.pulls.list, { + ...context.repo, state: 'closed', sort: 'updated', direction: 'desc', per_page: 100, + })) { + numbers.push(...response.data.filter(p => supported(p.base.ref) && + Date.parse(p.merged_at) >= now - DAY).map(p => p.number)); + if (!response.data.length || Date.parse(response.data.at(-1).updated_at) < now - DAY) break; + } + numbers = [...new Set(numbers)]; + } else throw new Error(`Unsupported event: ${context.eventName}`); + for (const number of numbers) { + try { + await requestOne({github, context, core, number, manual, now}); + } catch (error) { + if (context.eventName !== 'schedule') throw error; + core.error(`PR #${number}: ${error.message}`); + core.setFailed('Some PRs could not be scanned; see per-PR errors.'); + } + } +}; diff --git a/.github/scripts/coderabbit_semantic_review_result.js b/.github/scripts/coderabbit_semantic_review_result.js index 53d7fb708770..97af5ea0f9ff 100644 --- a/.github/scripts/coderabbit_semantic_review_result.js +++ b/.github/scripts/coderabbit_semantic_review_result.js @@ -13,124 +13,192 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Shared by the trusted publisher and read-only PR preview. -module.exports = async ({github, context, core}) => { - const name = 'Semantic conflict with target branch'; - const repo = context.repo; +const NAME = 'Semantic conflict with target branch'; +const AUDIT = 'Semantic conflict audit (post-merge)'; +const NOTICE = 'CodeRabbit can make mistakes, including false positives. Review the evidence. ' + + 'This check is advisory and must remain non-required; its failure does not block merging ' + + 'under that configuration. Other merge requirements still apply.'; +const supported = ref => ref === 'main' || /^release\/.+/.test(ref); +const isBot = (comment, login) => comment.user?.login === login && comment.user?.type === 'Bot'; +const compare = async (github, repo, base, head) => (await github.request( + 'GET /repos/{owner}/{repo}/compare/{basehead}', {...repo, basehead: `${base}...${head}`} +)).data; + +// Request metadata is written only by the trusted workflow, never accepted from PR authors. +function requests(comments) { + return comments.filter(c => isBot(c, 'github-actions[bot]')).flatMap(c => { + const text = c.body?.match(//); + if (!text) return []; + try { + const request = JSON.parse(text[1]); + if (![request.head, request.target, request.mergeBase].every(s => /^[a-f0-9]{40}$/.test(s)) || + !supported(request.branch)) return []; + return [{...request, created_at: c.created_at, id: c.id}]; + } catch { return []; } + }).sort((a, b) => b.id - a.id); +} + +function parseResult(comment) { + const body = comment.body || ''; + if (!isBot(comment, 'coderabbitai[bot]') || + (!body.includes('') && + !body.includes(''))) return; + const row = body.split('\n').map(line => line.split('|').map(cell => cell.trim())) + .find(cells => cells[1]?.toLowerCase() === NAME.toLowerCase()); + if (!row) return; + const details = body.match(/Full details: Semantic conflict with target branch<\/summary>([\s\S]*?)<\/details>/i); + const result = details ? details[1] : row.join('|'); + const pattern = /semantic_result head=([a-f0-9]{40}) target=([a-f0-9]{40}) merge_base=([a-f0-9]{40}) verdict=(pass|fail|inconclusive)\b/g; + const matches = [...new Map([...result.toLowerCase().matchAll(pattern)].map(m => [m[0], m])).values()]; + if (matches.length !== 1) return; + const [, head, target, mergeBase, rawVerdict] = matches[0]; + const verdict = rawVerdict.toUpperCase(); + const status = {PASS: /Passed/i, FAIL: /Warning|Error/i, INCONCLUSIVE: /Inconclusive/i}; + if (!status[verdict].test(row[2])) return; + const merged = [...new Set([...result.toLowerCase().matchAll(/semantic_merged sha=([a-f0-9]{40})\b/g)].map(m => m[1]))]; + if (merged.length > 1) return; + return {head, target, mergeBase, verdict, merged: merged[0], comment}; +} + +const matches = (result, request) => result && result.head === request.head && + result.target === request.target && result.mergeBase === request.mergeBase && + result.merged === request.merged; +const identity = (pr, pair) => `semantic-v2:${pr.number}:${pair.head}:${pair.target}:${pair.merged || 'open'}`; + +async function evidence(github, repo, pr) { + let target; + let tree; + let merged; + if (pr.merged) { + merged = pr.merge_commit_sha; + const {data: commit} = await github.rest.git.getCommit({...repo, commit_sha: merged}); + // The repository requires squash merges. A normal two-parent merge is also + // unambiguous; rebases must not silently be treated as a squash merge. + if (commit.parents.length === 1) { + const {data: rules} = await github.request('GET /repos/{owner}/{repo}/rules/branches/{branch}', + {...repo, branch: pr.base.ref}); + if (!rules.some(r => r.type === 'pull_request' && + r.parameters?.allowed_merge_methods?.length === 1 && + r.parameters.allowed_merge_methods[0] === 'squash')) { + throw new Error('Cannot verify a squash-only merge policy; historical target is inconclusive.'); + } + } else if (commit.parents.length !== 2 || commit.parents[1].sha !== pr.head.sha) { + throw new Error('Cannot verify the historical merge parents.'); + } + target = commit.parents[0].sha; + tree = commit.tree.sha; + } else { + const {data: ref} = await github.rest.git.getRef({...repo, ref: `heads/${pr.base.ref}`}); + target = ref.object.sha; + + } + const comparison = await compare(github, repo, target, pr.head.sha); + return {head: pr.head.sha, target, mergeBase: comparison.merge_base_commit.sha, + branch: pr.base.ref, merged, tree, comparison}; +} + +// This extra API read is needed only when actually requesting AI, not on every scan. +async function candidateTree(github, repo, pr, target) { + if (!pr.merge_commit_sha) return; + try { + const {data: candidate} = await github.rest.git.getCommit({...repo, commit_sha: pr.merge_commit_sha}); + if (candidate.parents.length === 2 && candidate.parents[0].sha === target && + candidate.parents[1].sha === pr.head.sha) return candidate.tree.sha; + } catch (error) { + if (error.status !== 404 && error.status !== 409) throw error; + } +} + +// Both the privileged publisher and the read-only PR preview use this verifier. +async function publish({github, context, core}) { const preview = context.eventName === 'pull_request'; if (preview) core.setOutput('verdict', 'INCONCLUSIVE'); + const repo = context.repo; const number = preview ? context.payload.pull_request.number : context.payload.issue.number; - const advisory = 'CodeRabbit can make mistakes, including false positives. Review the evidence. ' + - 'This check is advisory and must remain non-required; its failure does not block merging ' + - 'under that configuration. Other merge requirements still apply.'; const {data: pr} = await github.rest.pulls.get({...repo, pull_number: number}); - const eligible = pr => pr.state === 'open' && pr.base.ref === 'main' && - (preview || (!pr.draft && pr.labels.some(label => label.name === 'ai: semantic-conflict'))); - if (!eligible(pr)) return; - if (preview && pr.head.sha !== context.payload.pull_request.head.sha) { - core.warning('This preview run is stale; use a run for the current PR head.'); + if (!supported(pr.base.ref) || (!pr.merged && (pr.state !== 'open' || (!preview && pr.draft)))) return; + if (preview && (pr.merged || pr.head.sha !== context.payload.pull_request.head.sha)) { + core.warning('This preview is stale; use a run for the current PR head.'); return; } - const {data: ref} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); - const head = pr.head.sha; - const base = ref.object.sha; - let comment; - if (preview) { - const comments = await github.paginate(github.rest.issues.listComments, { - ...repo, issue_number: number, per_page: 100, - }); - comment = comments.filter(c => c.user?.login === 'coderabbitai[bot]' && - c.user?.type === 'Bot' && c.body?.toLowerCase().includes(name.toLowerCase()) && - c.body.toLowerCase().includes(`semantic_result head=${head} target=${base} `)) - .sort((a, b) => b.id - a.id)[0]; - if (!comment) { - const message = `No CodeRabbit result for head ${head} + main ${base}. ` + - 'This preview has no AI verdict. Request a custom pre-merge evaluation for these SHAs, ' + - 'then rerun the tests and result lookup job after the reply arrives.'; + const pair = await evidence(github, repo, pr); + const comments = await github.paginate(github.rest.issues.listComments, { + ...repo, issue_number: number, per_page: 100, + }); + const allRequests = requests(comments); + const latestRequest = allRequests.find(r => r.branch === pair.branch && + r.head === pair.head && r.target === pair.target && r.mergeBase === pair.mergeBase && + (pair.merged ? (r.merged === pair.merged || (!r.merged && r.tree && r.tree === pair.tree)) : !r.merged)); + const candidates = preview ? comments.toSorted((a, b) => b.id - a.id) : + [(await github.rest.issues.getComment({...repo, comment_id: context.payload.comment.id})).data]; + let result; + for (const comment of candidates) { + const parsed = parseResult(comment); + if (!parsed || parsed.head !== pair.head || parsed.target !== pair.target || + parsed.mergeBase !== pair.mergeBase) continue; + if (preview && !parsed.merged) { result = parsed; break; } + if (latestRequest && matches(parsed, latestRequest) && + comment.id > latestRequest.id && + Date.parse(comment.created_at) >= Date.parse(latestRequest.created_at)) { + result = parsed; break; + } + } + if (!result) { + if (preview) { + const message = `No verified CodeRabbit verdict for head ${pair.head} + ${pair.branch} ${pair.target}. ` + + 'Request a custom evaluation and rerun the tests and result lookup job after the reply arrives.'; core.warning(message); - await core.summary.addRaw(`${message}\n\n${advisory}\n`).write(); - return; + await core.summary.addRaw(`${message}\n\n${NOTICE}`).write(); } + return; + } + const {data: current} = await github.rest.pulls.get({...repo, pull_number: number}); + if (current.head.sha !== pair.head || current.base.ref !== pair.branch || + current.merged !== pr.merged || current.state !== pr.state || (!preview && current.draft)) return; + if (pair.merged) { + if (current.merge_commit_sha !== pair.merged) return; } else { - ({data: comment} = await github.rest.issues.getComment({ - ...repo, comment_id: context.payload.comment.id, - })); + const {data: ref} = await github.rest.git.getRef({...repo, ref: `heads/${pair.branch}`}); + if (ref.object.sha !== pair.target) { + if (preview) core.warning('The target changed during this preview; the result is stale.'); + return; + } } - const body = comment.body || ''; - if (comment.user?.login !== 'coderabbitai[bot]' || comment.user?.type !== 'Bot') return; - if (!body.includes('') && - !body.includes('')) return; - const row = body.split('\n').map(line => line.split('|').map(cell => cell.trim())) - .find(cells => cells[1]?.toLowerCase() === name.toLowerCase()); - if (!row) return; - const details = body.match(/Full details: Semantic conflict with target branch<\/summary>([\s\S]*?)<\/details>/i); - const result = details ? details[1] : row.join('|'); - - let check; + const title = {PASS: 'No semantic conflict found (best effort)', + FAIL: 'Possible semantic conflict — CodeRabbit may be wrong', + INCONCLUSIVE: 'Semantic analysis inconclusive'}[result.verdict]; + const summary = `${pair.merged ? `Post-merge audit of ${pair.merged}. ` : ''}` + + `Verified head ${pair.head}, target ${pair.target}, merge base ${pair.mergeBase}.\n\n` + + `[CodeRabbit analysis](${result.comment.html_url})\n\n${NOTICE}`; if (!preview) { const checks = await github.paginate(github.rest.checks.listForRef, { - ...repo, ref: head, check_name: name, filter: 'all', per_page: 100, + ...repo, ref: pair.merged || pair.head, check_name: pair.merged ? AUDIT : NAME, + filter: 'all', per_page: 100, }); - check = checks.filter(check => check.app?.slug === 'github-actions' && - check.external_id === `semantic-conflict:${number}:${head}:${base}`) + const check = checks.filter(c => c.app?.slug === 'github-actions' && c.external_id === identity(pr, pair)) .sort((a, b) => b.id - a.id)[0]; - if (!check) return; // Includes unrelated manual fixture evaluations. - } - - const pattern = /semantic_result head=([a-f0-9]{40}) target=([a-f0-9]{40}) merge_base=([a-f0-9]{40}) verdict=(pass|fail|inconclusive)\b/g; - const records = [...new Set([...result.toLowerCase().matchAll(pattern)].map(match => match[0]))]; - let verdict = 'INCONCLUSIVE'; - let reason = 'The result has no unique, verifiable revision record.'; - if (records.length === 1) { - const [, reportedHead, reportedBase, mergeBase, rawVerdict] = - [...records[0].matchAll(pattern)][0]; - const reportedVerdict = rawVerdict.toUpperCase(); - // A stale comment must never overwrite the current revision's result. - if (reportedHead !== head || reportedBase !== base) return; - const {data: comparison} = await github.request( - 'GET /repos/{owner}/{repo}/compare/{basehead}', - {...repo, basehead: `${base}...${head}`}); - const expectedStatus = {PASS: /Passed/i, FAIL: /Warning|Error/i, INCONCLUSIVE: /Inconclusive/i}; - if (comparison.merge_base_commit?.sha === mergeBase && expectedStatus[reportedVerdict].test(row[2])) { - verdict = reportedVerdict; - reason = `Verified head ${head}, target ${base}, merge base ${mergeBase}.`; - } else { - reason = 'The reported verdict or merge base could not be verified.'; + if (!check) return; + await github.rest.checks.update({...repo, check_run_id: check.id, status: 'completed', + conclusion: {PASS: 'success', FAIL: 'failure', INCONCLUSIVE: 'neutral'}[result.verdict], + details_url: result.comment.html_url, output: {title, summary}}); + if (pair.merged) { + const marker = ``; + if (!comments.some(c => isBot(c, 'github-actions[bot]') && c.body?.includes(marker))) { + await github.rest.issues.createComment({...repo, issue_number: number, + body: `${marker}\n**Post-merge semantic audit: ${result.verdict}**\n\n${summary}`}); + } } } - // Recheck refs after fetching the result and history. - const {data: current} = await github.rest.pulls.get({...repo, pull_number: number}); - const {data: currentRef} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); - if (current.head.sha !== head || currentRef.object.sha !== base || - !eligible(current)) { - if (preview) core.warning('The PR or main changed during this preview; rerun for fresh evidence.'); - return; - } - const titles = { - PASS: 'No semantic conflict found (best effort)', - FAIL: 'Possible semantic conflict — CodeRabbit may be wrong', - INCONCLUSIVE: '⚠️ Semantic analysis inconclusive', - }; - const summary = `${reason}\n\n[CodeRabbit analysis](${comment.html_url})\n\n${advisory}`; - if (!preview) await github.rest.checks.update({ - ...repo, check_run_id: check.id, status: 'completed', - conclusion: {PASS: 'success', FAIL: 'failure', INCONCLUSIVE: 'neutral'}[verdict], - details_url: comment.html_url, - output: { - title: titles[verdict], - summary, - }, - }); - await core.summary.addRaw(`${preview ? 'Read-only PR preview\n\n' : ''}` + - `${titles[verdict]}\n\n${summary}\n`).write(); - core.info(`Verified semantic verdict: ${verdict}; head=${head}; target=${base}`); - const message = `${titles[verdict]}. ${advisory} ${comment.html_url}`; + await core.summary.addRaw(`${preview ? 'Read-only PR preview\n\n' : ''}${title}\n\n${summary}\n`).write(); + core.info(`Verified semantic verdict: ${result.verdict}; head=${pair.head}; target=${pair.target}`); + const message = `${title}. ${NOTICE} ${result.comment.html_url}`; if (preview) { - core.setOutput('verdict', verdict); - core.setOutput('summary', `${titles[verdict]}\n\n${summary}`); + core.setOutput('verdict', result.verdict); + core.setOutput('summary', `${title}\n\n${summary}`); core.setOutput('message', message); - } else if (verdict === 'FAIL') { - core.setFailed(message); - } - if (verdict === 'INCONCLUSIVE') core.warning(`${titles[verdict]}: ${comment.html_url}`); -}; + } else if (result.verdict === 'FAIL') core.setFailed(message); + if (result.verdict === 'INCONCLUSIVE') core.warning(message); +} + +module.exports = publish; +Object.assign(module.exports, {NAME, AUDIT, NOTICE, supported, compare, requests, parseResult, matches, identity, evidence, candidateTree}); diff --git a/.github/workflows/coderabbit-semantic-review-tests.yml b/.github/workflows/coderabbit-semantic-review-tests.yml index 040d64a112ec..76ec3beaa71b 100644 --- a/.github/workflows/coderabbit-semantic-review-tests.yml +++ b/.github/workflows/coderabbit-semantic-review-tests.yml @@ -17,7 +17,7 @@ name: CodeRabbit Semantic Review Preview on: pull_request: - branches: [main] + branches: [main, 'release/**'] paths: - '.coderabbit.yaml' - '.github/workflows/coderabbit-semantic-review*.yml' @@ -50,7 +50,7 @@ jobs: persist-credentials: false - name: Test semantic review automation run: node --test .github/scripts/coderabbit_semantic_review.test.js - - name: Read CodeRabbit verdict for the current PR and main + - name: Read CodeRabbit verdict for the current PR and target branch id: result if: github.event_name == 'pull_request' uses: actions/github-script@v8 diff --git a/.github/workflows/coderabbit-semantic-review.yml b/.github/workflows/coderabbit-semantic-review.yml index 013308341eca..87f6387928d5 100644 --- a/.github/workflows/coderabbit-semantic-review.yml +++ b/.github/workflows/coderabbit-semantic-review.yml @@ -17,16 +17,16 @@ name: CodeRabbit Semantic Conflict Review on: pull_request_target: - branches: [main] - types: [opened, reopened, synchronize, ready_for_review, labeled, edited, auto_merge_enabled] - push: - branches: [main] + branches: [main, 'release/**'] + types: [opened, reopened, synchronize, ready_for_review, labeled, edited, auto_merge_enabled, closed] + schedule: + - cron: '23 * * * *' issue_comment: types: [created, edited] workflow_dispatch: inputs: pull_number: - description: 'Open PR number to recheck (requires ai: semantic-conflict label)' + description: 'Main/release PR to recheck; merged PRs receive a post-merge audit' required: true type: string @@ -35,7 +35,7 @@ permissions: jobs: request-review: - name: Request advisory semantic review + name: Request advisory semantic review or audit permissions: contents: read pull-requests: read @@ -43,134 +43,92 @@ jobs: checks: write if: >- github.repository == 'NVIDIA/TensorRT-LLM' && - contains(fromJSON('["push", "pull_request_target", "workflow_dispatch"]'), github.event_name) && - (github.event_name != 'pull_request_target' || - contains(github.event.pull_request.labels.*.name, 'ai: semantic-conflict')) - # Automatic runs sweep opted-in PRs so coalesced events are not lost. - # Manual retries have their own group and cannot replace a pending sweep. + contains(fromJSON('["schedule", "pull_request_target", "workflow_dispatch"]'), github.event_name) && + (github.event.action != 'closed' || github.event.pull_request.merged) && + (github.event.action != 'labeled' || github.event.label.name == 'ci: full pre-merge approved') + # Requests and publications share a queue: hourly scans cannot race PR events. + # ponytail: one queue, 100 pending; use per-PR dispatch if scans delay events. concurrency: - group: ${{ github.event_name == 'workflow_dispatch' && format('coderabbit-semantic-retry-{0}', inputs.pull_number) || 'coderabbit-semantic-conflict-requests' }} - cancel-in-progress: false + group: coderabbit-semantic-state + queue: max runs-on: ubuntu-latest - timeout-minutes: 10 + timeout-minutes: 30 steps: - # API-only trusted workflow code; no repository checkout is needed. - - name: Request CodeRabbit checks for the current SHA pair + - uses: actions/checkout@v6 + with: + # Always execute the default branch's trusted code, including release events. + ref: ${{ github.event.repository.default_branch }} + persist-credentials: false + sparse-checkout: | + /.coderabbit.yaml + /.github/scripts/coderabbit_semantic_review*.js + sparse-checkout-cone-mode: false + - name: Verify approval-label author using the existing approver team + id: approval + if: github.event.action == 'labeled' uses: actions/github-script@v8 - env: - DISPATCH_PULL_NUMBER: ${{ inputs.pull_number }} with: + github-token: ${{ secrets.TRTLLM_AGENT_SHARED_TOKEN }} + result-encoding: string script: | - const LABEL = 'ai: semantic-conflict'; - const BOT = 'github-actions[bot]'; - - const pullNumber = process.env.DISPATCH_PULL_NUMBER; - const repo = context.repo; - let retryNumber; - if (context.eventName === 'workflow_dispatch') { - const raw = (pullNumber || '').trim(); - retryNumber = Number(raw); - if (!/^[1-9][0-9]*$/.test(raw) || !Number.isSafeInteger(retryNumber)) { - throw new Error('pull_number must be a positive integer'); - } - } else if (!['push', 'pull_request_target'].includes(context.eventName)) { - throw new Error(`Unsupported event: ${context.eventName}`); - } - const pulls = await github.paginate(github.rest.pulls.list, { - ...repo, state: 'open', base: 'main', per_page: 100, + const {data: pr} = await github.rest.pulls.get({ + ...context.repo, pull_number: context.payload.pull_request.number, }); - const numbers = pulls.filter(pr => !pr.draft && pr.labels.some(l => l.name === LABEL)) - .map(pr => pr.number).filter(number => !retryNumber || number === retryNumber); - let retryPosted = false; - - for (const number of numbers) { - // Read current metadata instead of trusting a possibly queued event payload. - const {data: pr} = await github.rest.pulls.get({...repo, pull_number: number}); - if (pr.state !== 'open' || pr.draft || pr.base.ref !== 'main' || - !pr.labels.some(l => l.name === LABEL)) { - core.info(`PR #${number}: not an open, non-draft, opted-in main PR; skipped.`); - continue; - } - const {data: ref} = await github.rest.git.getRef({...repo, ref: 'heads/main'}); - const head = pr.head.sha; - const base = ref.object.sha; - const marker = ``; - const comments = await github.paginate(github.rest.issues.listComments, { - ...repo, issue_number: number, per_page: 100, - }); - // Only our own bot's comments can suppress an automatic request. A manual - // dispatch always retries, including after a CodeRabbit timeout or failure. - if (number !== retryNumber && comments.some(comment => - comment.user?.login === BOT && comment.body?.includes(marker))) { - core.info(`PR #${number}: this SHA pair has already been requested.`); - continue; - } - // A posted request is not a semantic pass. Keep a neutral check - // until a matching, verified CodeRabbit result arrives. - await github.rest.checks.create({ - ...repo, - name: 'Semantic conflict with target branch', - head_sha: head, - external_id: `semantic-conflict:${number}:${head}:${base}`, - status: 'completed', - conclusion: 'neutral', - output: { - title: 'Awaiting CodeRabbit analysis', - summary: `No AI verdict yet for PR #${number}, head ${head} + main ${base}. ` + - 'CodeRabbit can make mistakes. This check is advisory and must remain non-required; ' + - 'its failure does not block merging under that configuration.', - }, - }); - const body = [ - '@coderabbitai run pre-merge checks', - '', - marker, - `Semantic compatibility review requested for PR head \`${head}\` and main \`${base}\`.`, - 'Run the configured "Semantic conflict with target branch" check against these revisions.', - 'Verify both live refs. If either differs or cannot be inspected, return Inconclusive.', - 'Report the actual head, target, and merge-base SHAs with the analysis.', - '', - 'This is an advisory request, not a passing check. Previous results for a different', - 'SHA pair are stale; a successful workflow only means the request was posted.', - ].join('\n'); - const {data: comment} = await github.rest.issues.createComment({ - ...repo, issue_number: number, body, + if (!pr.labels.some(l => l.name === 'ci: full pre-merge approved')) return false; + const events = await github.paginate(github.rest.issues.listEventsForTimeline, { + ...context.repo, issue_number: pr.number, per_page: 100, + }); + const latest = events.filter(e => e.event === 'labeled' && + e.label?.name === 'ci: full pre-merge approved').at(-1); + if (!latest?.actor?.login || latest.actor.login !== context.payload.sender?.login) return false; + try { + const {data} = await github.rest.teams.getMembershipForUser({ + org: 'NVIDIA', team_slug: 'trt-llm-ci-approvers', username: latest.actor.login, }); - if (number === retryNumber) retryPosted = true; - core.info(`PR #${number}: requested ${head} + ${base}: ${comment.html_url}`); - await core.summary.addRaw( - `PR #${number}: [requested CodeRabbit analysis](${comment.html_url}) for ` + - `head \`${head}\` + main \`${base}\`. No AI verdict is asserted.\n\n` - ).write(); - } - if (retryNumber && !retryPosted) { - throw new Error(`PR #${retryNumber}: retry not posted; requires an open, non-draft main PR with the ${LABEL} label.`); + return data.state === 'active'; + } catch (error) { + if (![403, 404].includes(error.status)) throw error; + core.warning('The approval-label author could not be verified; no immediate AI request.'); + return false; } + - name: Apply the threshold, cooldown and audit policy + if: github.event.action != 'labeled' || steps.approval.outputs.result == 'true' + uses: actions/github-script@v8 + env: + DISPATCH_PULL_NUMBER: ${{ inputs.pull_number }} + SEMANTIC_APPROVAL_VALIDATED: ${{ steps.approval.outputs.result }} + with: + script: | + const request = require('./.github/scripts/coderabbit_semantic_review_request.js'); + await request({github, context, core}); publish-result: name: Publish advisory semantic result permissions: contents: read pull-requests: read - issues: read + issues: write checks: write if: >- github.repository == 'NVIDIA/TensorRT-LLM' && github.event_name == 'issue_comment' && github.event.issue.pull_request && github.event.comment.user.login == 'coderabbitai[bot]' && - contains(github.event.comment.body, 'Semantic conflict with target branch') + (contains(github.event.comment.body, 'pre-merge-checks-results') || + contains(github.event.comment.body, 'pre_merge_checks_walkthrough_start')) + concurrency: + group: coderabbit-semantic-state + queue: max runs-on: ubuntu-latest timeout-minutes: 5 steps: - # Load only the trusted workflow revision, never PR code with write access. - uses: actions/checkout@v6 with: - ref: ${{ github.workflow_sha }} + ref: ${{ github.event.repository.default_branch }} persist-credentials: false sparse-checkout: .github/scripts/coderabbit_semantic_review_result.js sparse-checkout-cone-mode: false - - name: Publish a result only for the current requested revision pair + - name: Verify the requested revisions and publish the advisory verdict uses: actions/github-script@v8 with: script: | diff --git a/AGENTS.md b/AGENTS.md index 06f999d017f4..92ef506221e8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -178,56 +178,74 @@ For a full list of up-to-date bot commands, post `/bot help` as a PR comment and ### Advisory semantic conflict review -CodeRabbit's `Semantic conflict with target branch` pre-merge check in -`.coderabbit.yaml` looks for behavioral incompatibilities across branches. It is -best-effort; missing revision/history evidence is Inconclusive. The CodeRabbit -comment uses warning mode; GitHub Check conclusions are published separately. - -For a non-draft PR targeting `main`, maintainers can add the -`ai: semantic-conflict` label to opt into automatic rechecks on PR and main -updates. The `CodeRabbit Semantic Conflict Review` workflow requests analysis -and publishes results; its request job succeeding is not an AI verdict. The independent -`Semantic conflict with target branch` GitHub Check starts neutral while -awaiting analysis. The workflow publishes CodeRabbit's result only after -verifying the bot author, requested head/target pair, and merge-base SHA. -PASS becomes success; a verified FAIL makes both the semantic Check and its -publishing job fail (red). Inconclusive results stay neutral with a warning -annotation. Results explain that CodeRabbit can make mistakes, including false -positives. Keep this advisory Check and workflow non-required: their failures -then do not block merging. Reviewers can document a false positive and merge -once the other requirements are met. This workflow does not change repository -rules; adding it to required checks would make failures block merging. -Read the result and verify its SHAs still match the live branches. - -Use workflow dispatch with the PR number to retry only that PR, or comment -`@coderabbitai run pre-merge checks`. Automatic events sweep all opted-in PRs. -Only requested revision pairs receive published results; unrelated manual -fixture experiments do not change a PR's semantic Check. Missing results stay -neutral, and stale results cannot mark a newer pair as passing. - -The custom check requires CodeRabbit Custom Pre-Merge Checks access. This -advisory pilot does not provide a merge-queue check or block merges. - -Changes to the semantic automation run the separate, read-only `CodeRabbit -Semantic Review Preview` workflow on `pull_request`, including fork drafts. -It shows two checks: `Automation tests and result lookup (not AI approval)` -runs the unit tests and reads real CodeRabbit replies; the `AI` check is green -only for a verified PASS and red for a verified FAIL. A missing, stale, or -inconclusive verdict leaves `AI verdict (advisory; skipped = unavailable)` -skipped (gray). -The lookup job succeeding is not an AI pass. These tests are not part of -`Pre-commit Check`. The production request/publish workflow does not run on -`pull_request`, so its unrelated jobs do not appear in the preview. - -Both workflows use the same result verifier. The production publisher loads it -from the trusted workflow commit; the fork preview has read-only permissions -and writes no custom Checks. Before the configuration is merged, request -`@coderabbitai evaluate custom pre-merge check` with `--name "Semantic conflict -with target branch"`, `--mode warning`, and `--instructions` containing the check -instructions from `.coderabbit.yaml` and the current head/main SHAs. After the -reply arrives, rerun the **tests and result lookup** job to refresh its outputs -and dependent AI check. Rerunning only the AI check reuses the previous outputs. -Previewing does not validate production event triggers or privileged Check writes. +The `CodeRabbit Semantic Conflict Review` workflow performs best-effort semantic +compatibility analysis for open, non-draft PRs targeting `main` or `release/**`. +No opt-in label is needed. It compares both branches from their merge base and +follows affected callers, contracts, configuration, and tests across files. + +- PR creation, reopening, updates, and becoming ready evaluate the threshold; + they do not automatically spend an AI call. An hourly scan also evaluates it. +- The first analysis needs new target commits and either 24 hours since the + merge-base commit or at least 30 target commits beyond that base. After a + completed PASS/FAIL analysis, count from its target SHA and completion time. + PR updates invalidate the old verdict but do not bypass these thresholds. +- An authorized `ci: full pre-merge approved` label or enabling auto-merge + bypasses the threshold. Approval-label authors are checked against the existing + `trt-llm-ci-approvers` team using its existing token. All pre-merge requests + share a one-hour cooldown, including when the SHA pair changes. A signal + during cooldown is skipped; ordinary scans and the post-merge audit remain. +- Exact revision pairs, including requests still awaiting a reply, are deduplicated. + With no completed analysis, new-pair routine requests wait 24 hours to avoid + repeated service-failure retries. A missing reply for the same pair requires + a manual retry. Workflow dispatch accepts one PR number and bypasses the + thresholds, cooldown and deduplication for that PR only. +- Merge events request a post-merge audit regardless of thresholds or cooldown. + The hourly scan recovers merges from the preceding 24 hours. The audit pins + the actual merge commit and historical target, including for release PRs; + later target updates do not invalidate it. Squash-only rules or the two-parent + merge must establish the historical target; ambiguous rebase history is rejected. + A pre-merge result/request can be reused only when its head/target pair and + GitHub's recorded test-merge tree match the actual merged tree. Otherwise the + audit includes the actual merged code in a new analysis request. + +This dedicated custom check is `off` in `.coderabbit.yaml` during ordinary +reviews. The workflow explicitly requests `evaluate custom pre-merge check` +with warning mode and the configured instructions so regular reviews cannot +bypass the spending policy. CodeRabbit Custom Pre-Merge Checks access is needed. +Its native Post-Merge Actions only support the default branch; this workflow +uses an explicit command on the merged PR instead. Acceptance of Actions-bot +commands, especially on merged/release PRs, requires deployment validation. +An absent/rejected AI reply remains without a verdict, never a semantic pass. + +The `Semantic conflict with target branch` Check starts neutral. Stale results +become neutral when the PR event or hourly scan observes a version change. +The verifier checks the bot identity, most recent trusted request, exact revision +record, and GitHub merge base. PASS becomes success; FAIL makes the Check and +publishing job red; Inconclusive remains neutral. A successful request job only +means orchestration succeeded. Checks on the actual merge SHA use the distinct +`Semantic conflict audit (post-merge)` name, with a receipt linking the analysis +on the original PR. Evidence includes code locations and regression scenarios. + +CodeRabbit can make mistakes, including false positives. Keep these checks and +workflows non-required: their failures then do not block merging. No required +waiting gate is added, and auto-merge does not wait for this analysis. Audit does +not revert code or modify branches. Repository rules remain unchanged. +The thresholds and cooldown limit frequency, not total calls per PR. + +Changes to this automation run the separate read-only `CodeRabbit Semantic +Review Preview` workflow, including fork drafts. `Automation tests and result +lookup (not AI approval)` runs Node tests and reads actual CodeRabbit replies; +`AI verdict (advisory; skipped = unavailable)` runs only for a verified current +PASS/FAIL and is otherwise gray/skipped. `precommit-check.yml` is unchanged. + +Both workflows use the same verifier. Privileged jobs load only trusted default +branch scripts, never PR code. The preview has read-only permissions. Before +merge, request `@coderabbitai evaluate custom pre-merge check` with the name +`Semantic conflict with target branch`, `--mode warning`, and `--instructions` +containing the configured instructions and fixed head/target/merge-base SHAs. +After the reply arrives, rerun the **tests and result lookup** job; rerunning only +the AI job reuses old outputs. Preview tests do not establish production trigger, +permission, command-acceptance or post-merge behavior. ### Trouble Shooting From aaf813699401ca32e02da7b232375ed7d664d521 Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Fri, 18 Sep 2026 20:52:47 +0800 Subject: [PATCH 12/21] [None][infra] Check cross-branch test contracts in semantic reviews Signed-off-by: Yanchao Lu --- .coderabbit.yaml | 14 ++++++++++++++ AGENTS.md | 2 ++ 2 files changed, 16 insertions(+) diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 0d46304b5da5..e66f2429aab1 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -101,6 +101,16 @@ reviews: Example: the target changes a function's return units while this PR adds a caller expecting the old units in another file. + Inspect unit tests, fixtures, mocks, and test doubles added or changed + on either branch against the combined production code. Trace their + exercised callers and callees; verify accepted positional/keyword + arguments, required object attributes, and return-value contracts. + Check both directions: target changes can invalidate PR tests, and + PR changes can invalidate target tests. Report each independently + supported incompatibility; do not stop after the first finding. + A newly broken test is a semantic conflict even if production + workloads remain usable; do not dismiss test-double mismatches. + Fail only for a concrete incompatibility between the PR changes and the target code. Cite both relevant code locations, the violated contract, a triggering scenario, confidence, and a minimal regression @@ -108,6 +118,10 @@ reviews: Do not execute repository code or claim tests were run. Pass only if the required context was inspected and no supported conflict was found; this is not proof of semantic compatibility. + For every verdict, describe both production and test-contract + findings with inspected file/line evidence. For PASS, explain why + the affected contracts remain compatible, or why a category does + not apply. Missing evidence must be Inconclusive, never a bare PASS. Analyze the requested historical pair even if live refs advance. State that the result applies only to the reported SHA pair and must be rerun after either branch changes. diff --git a/AGENTS.md b/AGENTS.md index 92ef506221e8..627d5606674e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -182,6 +182,8 @@ The `CodeRabbit Semantic Conflict Review` workflow performs best-effort semantic compatibility analysis for open, non-draft PRs targeting `main` or `release/**`. No opt-in label is needed. It compares both branches from their merge base and follows affected callers, contracts, configuration, and tests across files. +It explicitly checks tests and test doubles changed on either branch against +combined production signatures, required attributes, and return contracts. - PR creation, reopening, updates, and becoming ready evaluate the threshold; they do not automatically spend an AI call. An hourly scan also evaluates it. From da218b046da029a69e25cf9816d5169cb1d1e48b Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Fri, 18 Sep 2026 22:25:07 +0800 Subject: [PATCH 13/21] [None][infra] Restore semantic review prompt after failed replay Signed-off-by: Yanchao Lu --- .coderabbit.yaml | 14 -------------- AGENTS.md | 2 -- 2 files changed, 16 deletions(-) diff --git a/.coderabbit.yaml b/.coderabbit.yaml index e66f2429aab1..0d46304b5da5 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -101,16 +101,6 @@ reviews: Example: the target changes a function's return units while this PR adds a caller expecting the old units in another file. - Inspect unit tests, fixtures, mocks, and test doubles added or changed - on either branch against the combined production code. Trace their - exercised callers and callees; verify accepted positional/keyword - arguments, required object attributes, and return-value contracts. - Check both directions: target changes can invalidate PR tests, and - PR changes can invalidate target tests. Report each independently - supported incompatibility; do not stop after the first finding. - A newly broken test is a semantic conflict even if production - workloads remain usable; do not dismiss test-double mismatches. - Fail only for a concrete incompatibility between the PR changes and the target code. Cite both relevant code locations, the violated contract, a triggering scenario, confidence, and a minimal regression @@ -118,10 +108,6 @@ reviews: Do not execute repository code or claim tests were run. Pass only if the required context was inspected and no supported conflict was found; this is not proof of semantic compatibility. - For every verdict, describe both production and test-contract - findings with inspected file/line evidence. For PASS, explain why - the affected contracts remain compatible, or why a category does - not apply. Missing evidence must be Inconclusive, never a bare PASS. Analyze the requested historical pair even if live refs advance. State that the result applies only to the reported SHA pair and must be rerun after either branch changes. diff --git a/AGENTS.md b/AGENTS.md index 627d5606674e..92ef506221e8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -182,8 +182,6 @@ The `CodeRabbit Semantic Conflict Review` workflow performs best-effort semantic compatibility analysis for open, non-draft PRs targeting `main` or `release/**`. No opt-in label is needed. It compares both branches from their merge base and follows affected callers, contracts, configuration, and tests across files. -It explicitly checks tests and test doubles changed on either branch against -combined production signatures, required attributes, and return contracts. - PR creation, reopening, updates, and becoming ready evaluate the threshold; they do not automatically spend an AI call. An hourly scan also evaluates it. From b122843c28be84e20efc08e96332a465619109cc Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Fri, 18 Sep 2026 23:39:03 +0800 Subject: [PATCH 14/21] [None][infra] Focus semantic reviews on cross-branch contracts Signed-off-by: Yanchao Lu --- .coderabbit.yaml | 66 ++++++++----------- .../coderabbit_semantic_review.test.js | 30 +++++++++ .../coderabbit_semantic_review_result.js | 12 +++- AGENTS.md | 7 ++ 4 files changed, 76 insertions(+), 39 deletions(-) diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 0d46304b5da5..784771add2d5 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -84,43 +84,35 @@ reviews: - name: "Semantic conflict with target branch" mode: "off" instructions: | - Detect behavioral incompatibilities when this PR is combined with its - requested target revision, even when Git can merge without text conflicts. - Use repository and Git evidence, not claims in the PR description. - Resolve and report the requested PR head SHA, target SHA, and - merge-base SHA. Independently verify these fixed revisions; do not - substitute newer live refs or revisions from an earlier review. If these - revisions or necessary code/history are unavailable, return Inconclusive - and explain the missing evidence. Never invent a SHA or claim freshness. - - Compare merge-base..PR-head and merge-base..target, then inspect their - combined behavior. Follow affected callers, callees, configuration, - bindings, and tests across files, including unchanged dependencies. - Prioritize API/return-value contracts, changed defaults, tensor shapes - and dtypes, resource lifetimes, and distributed synchronization. - Example: the target changes a function's return units while this PR - adds a caller expecting the old units in another file. - - Fail only for a concrete incompatibility between the PR changes and - the target code. Cite both relevant code locations, the violated - contract, a triggering scenario, confidence, and a minimal regression - test. Exclude unrelated pre-existing bugs and style suggestions. - Do not execute repository code or claim tests were run. - Pass only if the required context was inspected and no supported - conflict was found; this is not proof of semantic compatibility. - Analyze the requested historical pair even if live refs advance. - State that the result applies only to the reported SHA pair and must - be rerun after either branch changes. - Explain that CodeRabbit can make mistakes, including false positives. - This is an advisory check: a failure does not block merging while the - check remains non-required. Other merge requirements still apply. - - Begin the explanation with exactly one machine-readable result line: - SEMANTIC_RESULT head= target= merge_base= verdict= - Replace each SHA with the inspected full lowercase 40-character SHA; - VERDICT must be PASS, FAIL, or INCONCLUSIVE. Do not emit this line if - any SHA cannot be verified; explain the missing evidence instead. - This line is used to publish the advisory GitHub Check for this pair. + Verify the requested head, target and merge base with Git. Compare both + diffs from the merge base and inspect the combined code. Use only those + revisions and source evidence; do not infer compatibility from earlier + reviews. + + First audit the tests: trace new or modified fixtures, fakes, mocks, + subclasses and monkeypatch replacements to the production functions they + exercise, in both directions across the two diffs. For each affected path, + compare actual call arguments, keywords and required attributes with the + replacement implementation. Report test-only incompatibilities too. Example: + run(x, trace=None) is incompatible with a replacement run(x); accepting + trace or a verified adapter resolves it. + + Then audit other affected contracts: return units, defaults, tensor + shapes/dtypes, resource lifetimes and synchronization. Exclude unrelated + pre-existing defects and style. Do not stop after a different unsupported + feature combination. + + FAIL if combining the branches breaks a concrete contract. Give the + triggering call/input and cite both sides. PASS requires a coverage summary + identifying inspected test and production paths and why they remain + compatible. Return INCONCLUSIVE for missing necessary evidence. Do not run + repository code. + + Put the evidence first, using immutable GitHub blob links with full SHAs and + line numbers from head and target. End with one record: SEMANTIC_RESULT + head= target= merge_base= verdict= + Use verified full lowercase SHAs; omit the record if unavailable. A record + alone is insufficient. path_filters: # Vendored/adapted FlashInfer kernels; excluded from review. diff --git a/.github/scripts/coderabbit_semantic_review.test.js b/.github/scripts/coderabbit_semantic_review.test.js index e65830ca83f9..e165c141f9b7 100644 --- a/.github/scripts/coderabbit_semantic_review.test.js +++ b/.github/scripts/coderabbit_semantic_review.test.js @@ -34,6 +34,8 @@ function resultBody(verdict, pair) { '
\nFull details: Semantic Conflict With Target Branch\n' + `SEMANTIC_RESULT head=${pair.head} target=${pair.target} merge_base=${pair.mergeBase} verdict=${verdict}\n` + (pair.merged ? `SEMANTIC_MERGED sha=${pair.merged}\n` : '') + + `Caller: https://github.com/example/repo/blob/${pair.head}/caller.py#L12\n` + + `Implementation: https://github.com/example/repo/blob/${pair.target}/callee.py#L25\n` + 'Code evidence and a minimal regression input.\n
'; } @@ -279,6 +281,34 @@ test('malformed, contradictory, stale and untrusted results never publish a pass await h.publish(reply); assert.equal(h.checks[0].conclusion, 'neutral'); }); +test('unsupported PASS/FAIL becomes inconclusive instead of reusing an older verdict', async () => { + for (const verdict of ['PASS', 'FAIL']) { + for (const removeEvidence of [ + body => body.replace(/https:\/\/github\.com\/example\/repo\/blob\/\S+/g, ''), + body => body.replace(`/blob/${BASE}/`, `/blob/${NEW_BASE}/`), + body => body.replaceAll('/example/repo/blob/', '/unrelated/repo/blob/'), + body => body.replace(/#L\d+/g, ''), + ]) { + const h = harness(); await h.run(); await h.publish(h.reply('PASS')); + const reply = h.reply(verdict); reply.body = removeEvidence(reply.body); + await h.publish(reply); + assert.equal(h.checks[0].conclusion, 'neutral'); + assert.match(h.checks[0].output.summary, /source links.*both revisions/); + assert.equal(h.failures.length, 0); + await h.publish(null, true); + assert.equal(h.outputs.verdict, 'INCONCLUSIVE'); + } + } +}); + +test('a result after its explanation is accepted with immutable citations', async () => { + const h = harness(); await h.run(); const reply = h.reply('FAIL'); + const record = reply.body.match(/SEMANTIC_RESULT[^\n]+\n/)[0]; + reply.body = reply.body.replace(record, '').replace('', `${record}`); + await h.publish(reply); + assert.equal(h.checks[0].conclusion, 'failure'); +}); + test('live ref changes during verification cannot publish a current pass', async () => { for (const change of [h => {h.pr.head.sha = NEW_BASE;}, h => h.setTarget(NEW_BASE)]) { const h = harness(); await h.run(); const reply = h.reply(); diff --git a/.github/scripts/coderabbit_semantic_review_result.js b/.github/scripts/coderabbit_semantic_review_result.js index 97af5ea0f9ff..c40b8915b0d8 100644 --- a/.github/scripts/coderabbit_semantic_review_result.js +++ b/.github/scripts/coderabbit_semantic_review_result.js @@ -52,12 +52,18 @@ function parseResult(comment) { const matches = [...new Map([...result.toLowerCase().matchAll(pattern)].map(m => [m[0], m])).values()]; if (matches.length !== 1) return; const [, head, target, mergeBase, rawVerdict] = matches[0]; - const verdict = rawVerdict.toUpperCase(); + let verdict = rawVerdict.toUpperCase(); const status = {PASS: /Passed/i, FAIL: /Warning|Error/i, INCONCLUSIVE: /Inconclusive/i}; if (!status[verdict].test(row[2])) return; const merged = [...new Set([...result.toLowerCase().matchAll(/semantic_merged sha=([a-f0-9]{40})\b/g)].map(m => m[1]))]; if (merged.length > 1) return; - return {head, target, mergeBase, verdict, merged: merged[0], comment}; + // This checks citation presence, not the correctness of the AI's reasoning. + const repository = comment.html_url?.match(/^https:\/\/github\.com\/([^/]+\/[^/]+)\/pull\//i)?.[1].toLowerCase(); + const citations = [...result.matchAll(/https:\/\/github\.com\/([^/\s]+\/[^/\s]+)\/blob\/([a-f0-9]{40})\/[^\s<>)|]+#L[1-9]\d*/gi)] + .filter(m => m[1].toLowerCase() === repository).map(m => m[2].toLowerCase()); + const missingEvidence = verdict !== 'INCONCLUSIVE' && ![head, target].every(sha => citations.includes(sha)); + if (missingEvidence) verdict = 'INCONCLUSIVE'; + return {head, target, mergeBase, verdict, missingEvidence, merged: merged[0], comment}; } const matches = (result, request) => result && result.head === request.head && @@ -169,6 +175,8 @@ async function publish({github, context, core}) { INCONCLUSIVE: 'Semantic analysis inconclusive'}[result.verdict]; const summary = `${pair.merged ? `Post-merge audit of ${pair.merged}. ` : ''}` + `Verified head ${pair.head}, target ${pair.target}, merge base ${pair.mergeBase}.\n\n` + + (result.missingEvidence ? 'CodeRabbit did not provide source links with full SHAs and line numbers for both revisions; ' + + 'its reported verdict is treated as inconclusive.\n\n' : '') + `[CodeRabbit analysis](${result.comment.html_url})\n\n${NOTICE}`; if (!preview) { const checks = await github.paginate(github.rest.checks.listForRef, { diff --git a/AGENTS.md b/AGENTS.md index 92ef506221e8..d58ef2754ec5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -225,6 +225,13 @@ publishing job red; Inconclusive remains neutral. A successful request job only means orchestration succeeded. Checks on the actual merge SHA use the distinct `Semantic conflict audit (post-merge)` name, with a receipt linking the analysis on the original PR. Evidence includes code locations and regression scenarios. +The instructions first discover cross-branch interactions, then verify their +contracts, including test replacements and the production paths they exercise. +Explanations precede the machine record and must cite immutable source links +with full SHAs and line numbers from both head and target. A PASS/FAIL without +those citations becomes Inconclusive, including in the preview; an older PASS +cannot substitute for that incomplete reply. Citation presence does not prove +the AI's reasoning or the cited code is correct. CodeRabbit can make mistakes, including false positives. Keep these checks and workflows non-required: their failures then do not block merging. No required From 358195413de6f5a182df4883ebf3f902146bbe41 Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Fri, 18 Sep 2026 23:48:29 +0800 Subject: [PATCH 15/21] [None][infra] Preserve semantic result records in failure details Signed-off-by: Yanchao Lu --- .coderabbit.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 784771add2d5..bb8e439ee23c 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -109,7 +109,9 @@ reviews: repository code. Put the evidence first, using immutable GitHub blob links with full SHAs and - line numbers from head and target. End with one record: SEMANTIC_RESULT + line numbers from head and target. Include this required record in + Explanation; for FAIL also repeat it verbatim in Resolution so it survives + result rendering: SEMANTIC_RESULT head= target= merge_base= verdict= Use verified full lowercase SHAs; omit the record if unavailable. A record alone is insufficient. From 7702469ff32b8673fc620e22b565d1a340e891b4 Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Sat, 19 Sep 2026 00:20:10 +0800 Subject: [PATCH 16/21] [None][infra] Keep semantic verdicts ordered across retries Signed-off-by: Yanchao Lu --- .../coderabbit_semantic_review.test.js | 31 ++++++++++++++++++- .../coderabbit_semantic_review_request.js | 2 +- .../coderabbit_semantic_review_result.js | 7 ++--- AGENTS.md | 6 +++- 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/.github/scripts/coderabbit_semantic_review.test.js b/.github/scripts/coderabbit_semantic_review.test.js index e165c141f9b7..0504867b8ea1 100644 --- a/.github/scripts/coderabbit_semantic_review.test.js +++ b/.github/scripts/coderabbit_semantic_review.test.js @@ -66,7 +66,7 @@ function harness(overrides = {}) { }}; }, }, issues: { - listComments: 'comments', getComment: async args => ({data: comments.find(c => c.id === args.comment_id)}), + listComments: 'comments', createComment: async args => { posted.push(args); writes.push(['comment', args]); const comment = {id: comments.length + 1, issue_number: args.issue_number, body: args.body, @@ -397,6 +397,35 @@ test('a manual retry cannot be satisfied by an older comment event', async () => await h.publish(h.reply('FAIL')); assert.equal(h.checks[0].conclusion, 'failure'); }); +test('delayed result events reconcile the newest verdict for open PRs and audits', async () => { + for (const merged of [false, true]) { + for (const verdict of ['PASS', 'FAIL', 'INCONCLUSIVE']) { + const h = harness({merged, state: merged ? 'closed' : 'open'}); + await h.run(); + const older = h.reply(verdict === 'PASS' ? 'FAIL' : 'PASS'); + const newer = h.reply(verdict); + await h.publish(newer); + await h.publish(older); + assert.equal(h.checks[0].conclusion, + {PASS: 'success', FAIL: 'failure', INCONCLUSIVE: 'neutral'}[verdict]); + assert.equal(h.checks[0].details_url, newer.html_url); + if (merged) assert.equal(h.posted.length, 2); // Request and one current audit receipt. + } + } +}); + +test('preview waits for the latest manual retry before accepting a verdict', async () => { + const h = harness(); await h.run(); await h.publish(h.reply('PASS')); + h.setNow(NOW + HOUR); await h.run('workflow_dispatch'); + const writes = h.writes.length; + await h.publish(null, true); + assert.equal(h.outputs.verdict, 'INCONCLUSIVE'); + assert.equal(h.writes.length, writes); + await h.publish(h.reply('FAIL'), true); + assert.equal(h.outputs.verdict, 'FAIL'); + assert.equal(h.writes.length, writes); +}); + test('a new audit request cannot be overwritten by an older pre-merge result', async () => { const h = harness(); await h.run(); const oldReply = h.reply('INCONCLUSIVE'); h.pr.merged = true; h.pr.state = 'closed'; await h.run('pull_request_target', 'closed'); diff --git a/.github/scripts/coderabbit_semantic_review_request.js b/.github/scripts/coderabbit_semantic_review_request.js index 8a4a10669be2..7fba1983719a 100644 --- a/.github/scripts/coderabbit_semantic_review_request.js +++ b/.github/scripts/coderabbit_semantic_review_request.js @@ -87,7 +87,7 @@ async function requestOne({github, context, core, number, manual, now}) { if (!pair.merged || reusable.merged || !result || result.verdict !== 'INCONCLUSIVE') { await ensureCheck('This version already has an analysis request.'); if (result) await publish({github, core, context: {...context, eventName: 'issue_comment', - payload: {issue: {number}, comment: {id: result.comment.id}}}}); + payload: {issue: {number}}}}); core.info(`PR #${number}: reusing the exact revision pair.`); return; } diff --git a/.github/scripts/coderabbit_semantic_review_result.js b/.github/scripts/coderabbit_semantic_review_result.js index c40b8915b0d8..d5f79b7ff931 100644 --- a/.github/scripts/coderabbit_semantic_review_result.js +++ b/.github/scripts/coderabbit_semantic_review_result.js @@ -135,14 +135,13 @@ async function publish({github, context, core}) { const latestRequest = allRequests.find(r => r.branch === pair.branch && r.head === pair.head && r.target === pair.target && r.mergeBase === pair.mergeBase && (pair.merged ? (r.merged === pair.merged || (!r.merged && r.tree && r.tree === pair.tree)) : !r.merged)); - const candidates = preview ? comments.toSorted((a, b) => b.id - a.id) : - [(await github.rest.issues.getComment({...repo, comment_id: context.payload.comment.id})).data]; let result; - for (const comment of candidates) { + // Reconcile the latest API result, even when an older comment event is replayed. + for (const comment of comments.toSorted((a, b) => b.id - a.id)) { const parsed = parseResult(comment); if (!parsed || parsed.head !== pair.head || parsed.target !== pair.target || parsed.mergeBase !== pair.mergeBase) continue; - if (preview && !parsed.merged) { result = parsed; break; } + if (preview && !latestRequest && !parsed.merged) { result = parsed; break; } if (latestRequest && matches(parsed, latestRequest) && comment.id > latestRequest.id && Date.parse(comment.created_at) >= Date.parse(latestRequest.created_at)) { diff --git a/AGENTS.md b/AGENTS.md index d58ef2754ec5..d5cd2be9ad24 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -220,7 +220,11 @@ An absent/rejected AI reply remains without a verdict, never a semantic pass. The `Semantic conflict with target branch` Check starts neutral. Stale results become neutral when the PR event or hourly scan observes a version change. The verifier checks the bot identity, most recent trusted request, exact revision -record, and GitHub merge base. PASS becomes success; FAIL makes the Check and +record, and GitHub merge base. Publication and preview select the newest applicable +reply after that request; delayed events cannot restore an older verdict, and +pending manual retries cannot reuse an earlier PASS. Before deployment, the preview +also accepts manual evaluations when no trusted request exists for the pair. +PASS becomes success; FAIL makes the Check and publishing job red; Inconclusive remains neutral. A successful request job only means orchestration succeeded. Checks on the actual merge SHA use the distinct `Semantic conflict audit (post-merge)` name, with a receipt linking the analysis From ca0336514e09f7b9de3f8f5111c4c262b8166a3a Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Thu, 24 Sep 2026 09:51:00 +0800 Subject: [PATCH 17/21] [None][infra] Bound semantic scans and preserve complete AI replies Signed-off-by: Yanchao Lu --- .coderabbit.yaml | 9 +- .github/coderabbit-semantic-review.md | 111 ++++++++++ .../coderabbit_semantic_review.test.js | 204 +++++++++++++++++- .../coderabbit_semantic_review_request.js | 86 ++++++-- .../coderabbit_semantic_review_result.js | 21 +- .../workflows/coderabbit-semantic-review.yml | 14 +- AGENTS.md | 81 +------ 7 files changed, 409 insertions(+), 117 deletions(-) create mode 100644 .github/coderabbit-semantic-review.md diff --git a/.coderabbit.yaml b/.coderabbit.yaml index bb8e439ee23c..5cfb03672509 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -108,10 +108,11 @@ reviews: compatible. Return INCONCLUSIVE for missing necessary evidence. Do not run repository code. - Put the evidence first, using immutable GitHub blob links with full SHAs and - line numbers from head and target. Include this required record in - Explanation; for FAIL also repeat it verbatim in Resolution so it survives - result rendering: SEMANTIC_RESULT + Reply in a normal PR chat comment, not a custom-check table. Start with + SEMANTIC_REVIEW_V3 on its own line. Include the full evidence for every + verdict, including PASS, using immutable GitHub blob links with full SHAs + and line numbers from head and target. Include this record on one line: + SEMANTIC_RESULT head= target= merge_base= verdict= Use verified full lowercase SHAs; omit the record if unavailable. A record alone is insufficient. diff --git a/.github/coderabbit-semantic-review.md b/.github/coderabbit-semantic-review.md new file mode 100644 index 000000000000..268474667942 --- /dev/null +++ b/.github/coderabbit-semantic-review.md @@ -0,0 +1,111 @@ + + +# Advisory semantic conflict review + +The `CodeRabbit Semantic Conflict Review` workflow performs best-effort semantic +compatibility analysis for open, non-draft PRs targeting `main` or `release/**`. +No opt-in label is needed. It compares both branches from their merge base and +follows affected callers, contracts, configuration, and tests across files. + +- PR creation, reopening, updates, and becoming ready evaluate the threshold; + they do not automatically spend an AI call. A scan every six hours also evaluates it. +- The first analysis needs new target commits and either 24 hours since the + merge-base commit or at least 30 target commits beyond that base. After a + completed PASS/FAIL analysis, count from its target SHA and completion time. + PR updates invalidate the old verdict but do not bypass these thresholds. +- An authorized `ci: full pre-merge approved` label or enabling auto-merge + bypasses the threshold. Approval-label authors are checked against the existing + `trt-llm-ci-approvers` team using its existing token. All pre-merge requests + share a one-hour cooldown, including when the SHA pair changes. A signal + during cooldown is skipped; ordinary scans and the post-merge audit remain. +- Exact revision pairs, including requests still awaiting a reply, are deduplicated. + A missing, truncated or inconclusive latest reply pauses routine requests, + including for new revision pairs, until a verified reply or a manual/merge-intent + retry. This avoids repeatedly paying for unusable replies. Workflow dispatch + accepts one PR number and bypasses the thresholds, cooldown and deduplication + for that PR only. +- Merge events request a post-merge audit regardless of thresholds or cooldown. + The six-hour scan recovers merges from the preceding 24 hours. The audit pins + the actual merge commit and historical target, including for release PRs; + later target updates do not invalidate it. Squash-only rules or the two-parent + merge must establish the historical target; ambiguous rebase history is rejected. + A pre-merge result/request can be reused only when its head/target pair and + GitHub's recorded test-merge tree match the actual merged tree. Otherwise the + audit includes the actual merged code in a new analysis request. + +## Request transport and scan limits + +The workflow reads the semantic instructions from `.coderabbit.yaml`; the native +custom check remains `off` during ordinary reviews. It requests an ordinary +CodeRabbit PR chat reply with `SEMANTIC_REVIEW_V3`, the full revision record and +source links. Native custom-check PASS table cells can truncate those details. +Legacy table replies remain readable but truncated evidence never becomes PASS. +The request explicitly forbids formal review submission or Request Changes. + +Commands use the existing `TRTLLM_AGENT_SHARED_TOKEN` and require the +`trtllm-agent` User account (ID `296075020`), verified before scanning. There is +no fallback to `github-actions[bot]`, whose commands may be ignored. Repository +reads and Check publication still use `GITHUB_TOKEN`; neither token executes PR +code. Acceptance of the service-account commands, especially on merged/release +PRs, requires a deployment pilot. Missing replies never imply semantic success. + +Scheduled scans run at minute 23 every six hours (UTC). Each scan sends at most +20 new AI requests, including audits, to avoid an initial burst across all open +PRs. Recent merged PRs are considered first; open PRs use a rotating starting +point. Exact-pair deduplication avoids resending completed or in-flight requests. +Scans restart from live state, not a saved cursor. Saturation can delay PRs; +use a manual dispatch for a specific PR rather than relying on a resume guarantee. + +Each token's remaining REST budget is read once, then tracked from response +headers, including pagination. A scan stops below a 100-request reserve or on +primary/secondary rate limiting and reports processed PRs and new requests. +Ordinary permission errors remain failures. Limits on a scheduled scan do not +change single-PR event or manual-dispatch behavior. Frequency reduction does not +reduce the peak cost of one scan. A rate-limited or missed scan can also delay +post-merge recovery beyond its 24-hour lookback. + +## Result verification + +The `Semantic conflict with target branch` Check starts neutral. Stale results +become neutral when the PR event or scheduled scan observes a version change. +The verifier checks the bot identity, most recent trusted request, exact revision +record, and GitHub merge base. Publication and preview select the newest applicable +reply after that request; delayed events cannot restore an older verdict, and +pending manual retries cannot reuse an earlier PASS. Before deployment, the preview +also accepts manual evaluations when no trusted request exists for the pair. +PASS becomes success; FAIL makes the Check and +publishing job red; Inconclusive remains neutral. A successful request job only +means orchestration succeeded. Checks on the actual merge SHA use the distinct +`Semantic conflict audit (post-merge)` name, with a receipt linking the analysis +on the original PR. Evidence includes code locations and regression scenarios. +The instructions first discover cross-branch interactions, then verify their +contracts, including test replacements and the production paths they exercise. +Replies must cite immutable source links +with full SHAs and line numbers from both head and target. A PASS/FAIL without +those citations becomes Inconclusive, including in the preview; an older PASS +cannot substitute for that incomplete reply. Citation presence does not prove +the AI's reasoning or the cited code is correct. + +CodeRabbit can make mistakes, including false positives. Keep these checks and +workflows non-required: their failures then do not block merging. No required +waiting gate is added, and auto-merge does not wait for this analysis. Audit does +not revert code or modify branches. Repository rules remain unchanged. +The thresholds and cooldown limit frequency, not total calls per PR. + +Changes to this automation run the separate read-only `CodeRabbit Semantic +Review Preview` workflow, including fork drafts. `Automation tests and result +lookup (not AI approval)` runs Node tests and reads actual CodeRabbit replies; +`AI verdict (advisory; skipped = unavailable)` runs only for a verified current +PASS/FAIL and is otherwise gray/skipped. `precommit-check.yml` is unchanged. + +Both workflows use the same verifier. Privileged jobs load only trusted default +branch scripts, never PR code. The preview has read-only permissions. Before +merge, a maintainer can use the exported `command(pair)` function in +`.github/scripts/coderabbit_semantic_review_request.js` to prepare a chat request +with fixed `head`, `target`, `mergeBase` and `branch`, then post it on the PR. +After the reply arrives, rerun the **tests and result lookup** job; rerunning only +the AI job reuses old outputs. Preview tests do not establish production trigger, +permission, command-acceptance or post-merge behavior. diff --git a/.github/scripts/coderabbit_semantic_review.test.js b/.github/scripts/coderabbit_semantic_review.test.js index 0504867b8ea1..6b543226c57e 100644 --- a/.github/scripts/coderabbit_semantic_review.test.js +++ b/.github/scripts/coderabbit_semantic_review.test.js @@ -20,6 +20,7 @@ const path = require('node:path'); const request = require('./coderabbit_semantic_review_request.js'); const publish = require('./coderabbit_semantic_review_result.js'); const {AUDIT, requests} = publish; +const COMMAND_USER = {login: 'trtllm-agent', id: 296075020, type: 'User'}; const APPROVED = 'ci: full pre-merge approved'; const HEAD = 'a'.repeat(40), BASE = 'b'.repeat(40), NEW_BASE = 'c'.repeat(40); const MERGED = 'd'.repeat(40), MERGE_BASE = 'e'.repeat(40), TREE = 'f'.repeat(40); @@ -106,12 +107,57 @@ function harness(overrides = {}) { github.paginate.iterator = async function* () { yield {data: prs.filter(p => p.state === 'closed')}; }; + const createComment = github.rest.issues.createComment; + const commandGithub = {rest: { + users: {getAuthenticated: async () => ({data: {...COMMAND_USER}})}, + issues: {createComment: async args => { + const response = await createComment(args); + response.data.user = {...COMMAND_USER}; + return response; + }}, + }}; + // Exercise the production Octokit hook around API reads, writes and pagination. + for (const client of [github, commandGithub]) { + const hooks = []; + client.hook = { + wrap: (name, fn) => {assert.equal(name, 'request'); hooks.push(fn);}, + remove: (name, fn) => {assert.equal(name, 'request'); hooks.splice(hooks.indexOf(fn), 1);}, + }; + client.remaining = 15000; + client.apiError = null; + client.rest.rateLimit = {get: async () => ({data: {resources: {core: {remaining: client.remaining}}}})}; + const invoke = async fn => { + const perform = async () => { + if (client.apiError) throw client.apiError; + const response = await fn(); + response.headers = {'x-ratelimit-remaining': String(client.remaining)}; + return response; + }; + return hooks.reduceRight((next, hook) => () => hook(next, {}), perform)(); + }; + for (const group of Object.values(client.rest)) { + for (const [name, fn] of Object.entries(group)) { + if (typeof fn === 'function') group[name] = (...args) => invoke(() => fn(...args)); + } + } + if (client.request) { + const original = client.request; + client.request = (...args) => invoke(() => original(...args)); + } + if (client.paginate) { + const original = client.paginate, iterator = original.iterator; + client.paginate = async (...args) => (await invoke(async () => ({data: await original(...args)}))).data; + client.paginate.iterator = async function* (...args) { + for await (const response of iterator(...args)) yield await invoke(async () => response); + }; + } + } const core = {setOutput: (k, v) => {outputs[k] = v;}, info() {}, warning: v => warnings.push(v), error: v => warnings.push(v), setFailed: v => failures.push(v), summary: {addRaw(v) {summaries.push(v); return this;}, async write() {}}}; const context = {repo: {owner: 'example', repo: 'repo'}, eventName: 'pull_request_target', payload: {action: 'opened', pull_request: {number: 12, head: {sha: HEAD}}}}; - return {pr, prs, comments, checks, posted, writes, calls, outputs, warnings, failures, summaries, github, core, + return {pr, prs, comments, checks, posted, writes, calls, outputs, warnings, failures, summaries, github, commandGithub, core, setTarget: v => {target = v;}, setNow: v => {now = v;}, setLag: (n, a) => {count = n; age = a;}, setProgress: v => {progressCount = v;}, setTree: v => {mergeTree = v;}, setRules: v => {rules = v;}, setParents: v => {mergeParents = v;}, @@ -120,7 +166,7 @@ function harness(overrides = {}) { process.env.DISPATCH_PULL_NUMBER = manual; process.env.SEMANTIC_APPROVAL_VALIDATED = String(authorized); try { - await request({github, core, now, context: {...context, eventName, + await request({github, commandGithub, core, now, context: {...context, eventName, payload: {...context.payload, action, label: {name: APPROVED}}}}); } finally { delete process.env.DISPATCH_PULL_NUMBER; @@ -148,8 +194,9 @@ for (const [count, age, expected] of [[0, 3 * DAY, 0], [29, DAY - 1, 0], assert.equal(h.posted.length, expected); assert.equal(h.checks[0].conclusion, 'neutral'); if (expected) { - assert.match(h.posted[0].body, /^@coderabbitai evaluate custom pre-merge check/); - assert.match(h.posted[0].body, /--mode warning/); + assert.match(h.posted[0].body, /^@coderabbitai\n/); + assert.match(h.posted[0].body, /normal PR chat comment/); + assert.match(h.posted[0].body, /SEMANTIC_REVIEW_V3/); assert.match(h.posted[0].body, /No AI verdict is asserted/); assert.ok(h.posted[0].body.includes(HEAD) && h.posted[0].body.includes(BASE)); } @@ -176,7 +223,7 @@ test('release PRs use their actual target and need no opt-in label', async () => assert.equal(h.checks[0].conclusion, 'success'); }); -test('an hourly scan visits eligible PRs; a PR event or manual retry visits only its PR', async () => { +test('a six-hour scan visits eligible PRs; a PR event or manual retry visits only its PR', async () => { const h = harness(); h.prs.push({...h.pr, number: 13}); await h.run(); assert.deepEqual(h.posted.map(c => c.issue_number), [12]); await h.run('schedule'); assert.deepEqual(h.posted.map(c => c.issue_number), [12, 13]); @@ -434,7 +481,7 @@ test('a new audit request cannot be overwritten by an older pre-merge result', a await h.publish(oldReply); assert.equal(h.checks.at(-1).conclusion, 'failure'); }); -test('the hourly scan recovers recent merged PRs without auditing old closed history', async () => { +test('the six-hour scan recovers recent merged PRs without auditing old closed history', async () => { const h = harness({merged: true, state: 'closed', merged_at: new Date(NOW - HOUR).toISOString(), updated_at: new Date(NOW).toISOString()}); h.prs.push({...h.pr, number: 13, merged_at: new Date(NOW - 2 * DAY).toISOString()}); @@ -478,3 +525,148 @@ test('approval validation checks the current label, actor and active membership' assert.equal(actual, expected); } }); + +function chatReply(reply) { + const details = reply.body.match(/Full details:[^\n]+\n([\s\S]*?)<\/details>/)[1]; + reply.body = `SEMANTIC_REVIEW_V3\n${details}`; + return reply; +} + +test('full chat PASS and FAIL retain source evidence beyond the native 201-character cell', async () => { + for (const verdict of ['PASS', 'FAIL']) { + const h = harness(); await h.run(); + const reply = chatReply(h.reply(verdict)); + assert.ok(reply.body.length > 201); + await h.publish(reply); + assert.equal(h.checks[0].conclusion, verdict === 'PASS' ? 'success' : 'failure'); + await h.publish(null, true); assert.equal(h.outputs.verdict, verdict); + } +}); + +test('chat transport preserves author, revision, citation and unique-record validation', async () => { + for (const corrupt of [ + c => {c.user.type = 'User';}, c => {c.user.login = 'someone-else';}, + c => {c.body = c.body.replace('SEMANTIC_REVIEW_V3', '');}, + c => {c.body = c.body.replaceAll(HEAD, NEW_BASE);}, + c => {c.body = c.body.replaceAll(MERGE_BASE, NEW_BASE);}, + c => {c.body = c.body.replace(/https:\/\/github.com\/\S+/g, '');}, + c => {c.body += c.body.replace('verdict=PASS', 'verdict=FAIL');}, + ]) { + const h = harness(); await h.run(); + const reply = chatReply(h.reply()); corrupt(reply); await h.publish(reply); + assert.equal(h.checks[0].conclusion, 'neutral'); + } +}); + +test('missing, truncated or inconclusive replies cannot cause daily new-pair AI retries', async () => { + for (const kind of ['missing', 'truncated-record', 'truncated-evidence', 'inconclusive']) { + const h = harness(); await h.run(); + if (kind !== 'missing') { + const reply = h.reply(kind === 'inconclusive' ? 'INCONCLUSIVE' : 'PASS'); + if (kind.startsWith('truncated')) { + const record = reply.body.match(/SEMANTIC_RESULT[^\n]+/)[0]; + const explanation = kind === 'truncated-record' ? 'coverage '.repeat(30) + record : record + ' evidence '.repeat(30); + reply.body = '\n' + + `| Semantic conflict with target branch | ✅ Passed | ${explanation.slice(0, 201)} |`; + } + } + for (let day = 1; day <= 7; day++) { + h.setNow(NOW + day * DAY); h.setTarget(day.toString().repeat(40)); await h.run('schedule'); + } + assert.equal(h.posted.length, 1, kind); + assert.match(h.checks.at(-1).output.summary, /manual retry/); + await h.run('workflow_dispatch'); assert.equal(h.posted.length, 2); + await h.publish(chatReply(h.reply())); assert.equal(h.checks.at(-1).conclusion, 'success'); + } +}); + +test('commands require the pinned User account and never fall back to the Actions bot', async () => { + for (const user of [ + {...COMMAND_USER, id: 1}, {...COMMAND_USER, login: 'another-user'}, + {...COMMAND_USER, type: 'Bot'}, {login: 'github-actions[bot]', type: 'Bot'}, + ]) { + const h = harness(); h.commandGithub.rest.users.getAuthenticated = async () => ({data: user}); + await assert.rejects(h.run('schedule'), /require the trtllm-agent/); + assert.equal(h.writes.length, 0); + } + const h = harness(); await h.run(); + assert.deepEqual(h.comments[0].user, COMMAND_USER); + assert.equal(requests(h.comments).length, 1); + h.comments[0].user.id = 1; assert.equal(requests(h.comments).length, 0); +}); + +test('scheduled scans cap new AI requests at 20 and later scans consider the remaining PRs', async () => { + const h = harness(); + for (let n = 13; n < 57; n++) h.prs.push({...h.pr, number: n}); + await h.run('schedule'); assert.equal(h.posted.length, 20); + assert.match(h.warnings.at(-1), /20 new AI requests/); + h.setNow(NOW + 6 * HOUR); await h.run('schedule'); + assert.equal(h.posted.length, 40); + assert.equal(new Set(h.posted.map(p => p.issue_number)).size, 40); + await h.run('workflow_dispatch'); assert.equal(h.posted.length, 41); +}); + +test('scheduled scans prioritize recent merge recovery over open PR requests', async () => { + const h = harness(); + for (let n = 13; n < 40; n++) h.prs.push({...h.pr, number: n}); + h.prs.push({...h.pr, number: 100, state: 'closed', merged: true, + merged_at: new Date(NOW).toISOString(), updated_at: new Date(NOW).toISOString()}); + await h.run('schedule'); assert.equal(h.posted[0].issue_number, 100); + assert.match(h.posted[0].body, /Post-merge audit/); + assert.equal(h.posted.length, 20); +}); + +test('either token below its reserve stops scanning before PR reads; hooks are removed afterward', async () => { + for (const token of ['github', 'commandGithub']) { + const h = harness(); h[token].remaining = 99; await h.run('schedule'); + assert.equal(h.calls.length, 0); assert.equal(h.writes.length, 0); + assert.match(h.warnings.at(-1), /100-request reserve/); + await h.run('workflow_dispatch'); assert.equal(h.posted.length, 1); + } +}); + +test('response headers can stop a scan mid-PR before further pagination or writes', async () => { + const h = harness(); h.prs.push({...h.pr, number: 13}); + h.afterCompare(() => {h.github.remaining = 99;}); await h.run('schedule'); + assert.equal(h.calls.filter(([kind]) => kind === 'pull').length, 1); + assert.equal(h.calls.filter(([kind]) => kind === 'comments').length, 0); + assert.equal(h.writes.length, 0); assert.equal(h.failures.length, 0); + assert.match(h.warnings.at(-1), /100-request reserve/); +}); + +test('quota guard covers closed-list pagination and rotates open-PR starts', async () => { + const visited = []; + for (let round = 0; round < 2; round++) { + const h = harness(); h.prs.push({...h.pr, number: 13}); h.setNow(NOW + round * 6 * HOUR); + h.afterCompare(() => {h.github.remaining = 99;}); await h.run('schedule'); + visited.push(h.calls.find(([kind]) => kind === 'pull')[1].pull_number); + } + assert.notEqual(visited[0], visited[1]); + const h = harness(); let pages = 0; + h.github.paginate.iterator = async function* () { + await h.github.rest.pulls.get({pull_number: 12}); + h.github.remaining = 99; + await h.github.rest.pulls.get({pull_number: 12}); pages++; + yield {data: [{...h.pr, updated_at: new Date(NOW).toISOString()}]}; + await h.github.rest.pulls.get({pull_number: 12}); pages++; + yield {data: []}; + }; + await h.run('schedule'); assert.equal(pages, 1); assert.equal(h.writes.length, 0); +}); + +test('rate-limit exits stop the scan, while unrelated permission errors remain failures', async () => { + for (const error of [ + {status: 403, response: {headers: {'x-ratelimit-remaining': '0'}}}, + {status: 403, response: {headers: {'retry-after': '60'}}}, + {status: 403, message: 'You have exceeded a secondary rate limit'}, {status: 429}, + ]) { + const h = harness(); h.prs.push({...h.pr, number: 13}); + h.afterCompare(() => {h.github.apiError = Object.assign(new Error('Limited'), error);}); + await h.run('schedule'); assert.equal(h.writes.length, 0); assert.equal(h.failures.length, 0); + assert.equal(h.calls.filter(([kind]) => kind === 'pull').length, 1); + assert.match(h.warnings.at(-1), /Scheduled scan stopped/); + } + const h = harness(); h.prs.push({...h.pr, number: 13}); + h.afterCompare(() => {h.github.apiError = Object.assign(new Error('Resource not accessible'), {status: 403});}); + await h.run('schedule'); assert.ok(h.failures.length > 0); +}); diff --git a/.github/scripts/coderabbit_semantic_review_request.js b/.github/scripts/coderabbit_semantic_review_request.js index 7fba1983719a..762b68c9ae5e 100644 --- a/.github/scripts/coderabbit_semantic_review_request.js +++ b/.github/scripts/coderabbit_semantic_review_request.js @@ -15,7 +15,7 @@ const fs = require('node:fs'); const publish = require('./coderabbit_semantic_review_result.js'); -const {NAME, AUDIT, NOTICE, supported, compare, requests, parseResult, matches, identity, evidence, candidateTree} = publish; +const {NAME, AUDIT, NOTICE, supported, compare, requests, parseResult, matches, identity, evidence, candidateTree, isCommandUser} = publish; const HOUR = 60 * 60 * 1000; const DAY = 24 * HOUR; const APPROVED = 'ci: full pre-merge approved'; @@ -30,11 +30,11 @@ function command(pair) { `Include SEMANTIC_MERGED sha=${pair.merged} immediately after the result line. ` + 'Later changes to the target branch do not invalidate this historical audit.' : `This is a pre-merge analysis against ${pair.branch}. The result applies only to this pair.`); - return `@coderabbitai evaluate custom pre-merge check --name "${NAME}" --mode warning ` + - `--instructions ${JSON.stringify(instructions.replace(/\s+/g, ' '))}`; + return `@coderabbitai\nPlease perform this advisory semantic analysis and reply in a normal PR chat comment. ` + + `Do not invoke the custom pre-merge check command or submit a review/request changes.\n\n${instructions}`; } -async function requestOne({github, context, core, number, manual, now}) { +async function requestOne({github, commandGithub, context, core, number, manual, now}) { const repo = context.repo; const {data: pr} = await github.rest.pulls.get({...repo, pull_number: number}); if (!supported(pr.base.ref) || (!pr.merged && (pr.state !== 'open' || pr.draft))) { @@ -99,18 +99,18 @@ async function requestOne({github, context, core, number, manual, now}) { let reason = manual ? 'Manual retry' : pair.merged ? 'Post-merge audit' : intent ? 'Merge intent' : ''; const last = history.find(r => !r.merged); if (!reason) { - const completed = history.find(r => !r.merged && ['PASS', 'FAIL'].includes(resultFor(r)?.verdict)); - if (last && !completed && now - Date.parse(last.created_at) < DAY) { - await ensureCheck('No completed analysis; new-pair requests wait 24 hours. Manual retry remains available.'); + const previous = last && resultFor(last); + if (last && !['PASS', 'FAIL'].includes(previous?.verdict)) { + await ensureCheck('Previous analysis has no verified verdict; use a manual retry. Routine requests are paused.'); return; } let count = pair.comparison.behind_by; let since = Date.parse(pair.comparison.merge_base_commit.commit.committer.date); - if (completed) { - const progress = await compare(github, repo, completed.target, pair.target); + if (previous) { + const progress = await compare(github, repo, last.target, pair.target); if (['ahead', 'identical'].includes(progress.status)) { count = progress.ahead_by; - since = Date.parse(resultFor(completed).comment.created_at); + since = Date.parse(previous.comment.created_at); } } if (!count || (count < 30 && now - since < DAY)) { @@ -146,14 +146,24 @@ async function requestOne({github, context, core, number, manual, now}) { summary: `${reason}: head ${pair.head}, target ${pair.target}. ${NOTICE}`}}); if (!pair.merged) pair.tree = await candidateTree(github, repo, current, pair.target); const {comparison, ...record} = pair; - const {data: comment} = await github.rest.issues.createComment({...repo, issue_number: number, + const {data: comment} = await commandGithub.rest.issues.createComment({...repo, issue_number: number, body: `${command(pair)}\n\n\n\n` + `${reason} for PR #${number}. ${NOTICE} No AI verdict is asserted by posting this request.`}); core.info(`PR #${number}: ${reason}; ${comment.html_url}`); await core.summary.addRaw(`PR #${number}: [${reason}](${comment.html_url}). No AI verdict is asserted.\n\n`).write(); + return true; } -module.exports = async ({github, context, core, now = Date.now()}) => { +class ScanStopped extends Error {} + +function rateLimited(error) { + const headers = error.response?.headers || {}; + return error.status === 429 || error.status === 403 && + (headers['x-ratelimit-remaining'] === '0' || headers['retry-after'] !== undefined || + /rate limit|abuse detection/i.test(error.message)); +} + +async function run({github, commandGithub, context, core, now, progress}) { let numbers; const manual = context.eventName === 'workflow_dispatch'; if (manual) { @@ -169,7 +179,10 @@ module.exports = async ({github, context, core, now = Date.now()}) => { const pulls = await github.paginate(github.rest.pulls.list, { ...context.repo, state: 'open', per_page: 100, }); - numbers = pulls.filter(p => !p.draft && supported(p.base.ref)).map(p => p.number); + const open = pulls.filter(p => !p.draft && supported(p.base.ref)).map(p => p.number).sort((a, b) => a - b); + // Rotate the starting PR each scan; no persistent cursor or exact resume is implied. + const offset = Math.floor(now / (6 * HOUR)) % (open.length || 1); + numbers = []; // Recover recent merges if an event was dropped or a release branch still // lacks the workflow. Do not backfill the repository's entire merge history. for await (const response of github.paginate.iterator(github.rest.pulls.list, { @@ -179,15 +192,56 @@ module.exports = async ({github, context, core, now = Date.now()}) => { Date.parse(p.merged_at) >= now - DAY).map(p => p.number)); if (!response.data.length || Date.parse(response.data.at(-1).updated_at) < now - DAY) break; } - numbers = [...new Set(numbers)]; + numbers = [...new Set([...numbers, ...open.slice(offset), ...open.slice(0, offset)])]; } else throw new Error(`Unsupported event: ${context.eventName}`); for (const number of numbers) { + if (context.eventName === 'schedule' && progress.requested >= 20) { + throw new ScanStopped('Reached the limit of 20 new AI requests for this scan.'); + } try { - await requestOne({github, context, core, number, manual, now}); + if (await requestOne({github, commandGithub, context, core, number, manual, now})) progress.requested++; + progress.processed++; } catch (error) { - if (context.eventName !== 'schedule') throw error; + if (context.eventName !== 'schedule' || error instanceof ScanStopped || rateLimited(error)) throw error; core.error(`PR #${number}: ${error.message}`); core.setFailed('Some PRs could not be scanned; see per-PR errors.'); } } +} + +module.exports = async ({github, commandGithub, context, core, now = Date.now()}) => { + const scheduled = context.eventName === 'schedule'; + const progress = {processed: 0, requested: 0}; + const guards = []; + let stopReason = 'Complete'; + try { + if (scheduled) { + for (const client of [github, commandGithub]) { + let remaining = Infinity; + const guard = async (request, options) => { + if (remaining < 100) throw new ScanStopped('REST quota is below the 100-request reserve.'); + const response = await request(options); + const value = response.headers?.['x-ratelimit-remaining']; + if (value !== undefined) remaining = Number(value); + return response; + }; + client.hook.wrap('request', guard); + guards.push([client, guard]); + const {data} = await client.rest.rateLimit.get(); + remaining = data.resources.core.remaining; + } + } + const {data: user} = await commandGithub.rest.users.getAuthenticated(); + if (!isCommandUser(user)) throw new Error('Semantic commands require the trtllm-agent User account.'); + await run({github, commandGithub, context, core, now, progress}); + } catch (error) { + if (!scheduled || !(error instanceof ScanStopped || rateLimited(error))) throw error; + stopReason = error.message; + core.warning(`Scheduled scan stopped: ${stopReason} Unvisited PRs wait for a later scan or manual dispatch.`); + } finally { + for (const [client, guard] of guards) client.hook.remove('request', guard); + if (scheduled) await core.summary.addRaw(`Scheduled scan: ${progress.processed} PRs processed; ` + + `${progress.requested} new AI requests. ${stopReason}. Scans restart from live state, not a saved cursor.\n`).write(); + } }; +Object.assign(module.exports, {command}); diff --git a/.github/scripts/coderabbit_semantic_review_result.js b/.github/scripts/coderabbit_semantic_review_result.js index d5f79b7ff931..99650789ca48 100644 --- a/.github/scripts/coderabbit_semantic_review_result.js +++ b/.github/scripts/coderabbit_semantic_review_result.js @@ -20,13 +20,15 @@ const NOTICE = 'CodeRabbit can make mistakes, including false positives. Review 'under that configuration. Other merge requirements still apply.'; const supported = ref => ref === 'main' || /^release\/.+/.test(ref); const isBot = (comment, login) => comment.user?.login === login && comment.user?.type === 'Bot'; +// Pin the existing service account by ID as well as login; never trust arbitrary PAT users. +const isCommandUser = user => user?.login === 'trtllm-agent' && user.id === 296075020 && user.type === 'User'; const compare = async (github, repo, base, head) => (await github.request( 'GET /repos/{owner}/{repo}/compare/{basehead}', {...repo, basehead: `${base}...${head}`} )).data; // Request metadata is written only by the trusted workflow, never accepted from PR authors. function requests(comments) { - return comments.filter(c => isBot(c, 'github-actions[bot]')).flatMap(c => { + return comments.filter(c => isCommandUser(c.user) || isBot(c, 'github-actions[bot]')).flatMap(c => { const text = c.body?.match(//); if (!text) return []; try { @@ -40,21 +42,24 @@ function requests(comments) { function parseResult(comment) { const body = comment.body || ''; - if (!isBot(comment, 'coderabbitai[bot]') || - (!body.includes('') && + if (!isBot(comment, 'coderabbitai[bot]')) return; + // Chat replies retain full evidence for PASS as well as FAIL. Native custom + // checks may truncate PASS to a 201-character cell; keep old replies readable. + const standalone = body.match(/^SEMANTIC_REVIEW_V3[ \t]*\r?$/m); + if (!standalone && (!body.includes('') && !body.includes(''))) return; const row = body.split('\n').map(line => line.split('|').map(cell => cell.trim())) .find(cells => cells[1]?.toLowerCase() === NAME.toLowerCase()); - if (!row) return; + if (!standalone && !row) return; const details = body.match(/Full details: Semantic conflict with target branch<\/summary>([\s\S]*?)<\/details>/i); - const result = details ? details[1] : row.join('|'); + const result = standalone ? body.slice(standalone.index + standalone[0].length) : details ? details[1] : row.join('|'); const pattern = /semantic_result head=([a-f0-9]{40}) target=([a-f0-9]{40}) merge_base=([a-f0-9]{40}) verdict=(pass|fail|inconclusive)\b/g; const matches = [...new Map([...result.toLowerCase().matchAll(pattern)].map(m => [m[0], m])).values()]; if (matches.length !== 1) return; const [, head, target, mergeBase, rawVerdict] = matches[0]; let verdict = rawVerdict.toUpperCase(); const status = {PASS: /Passed/i, FAIL: /Warning|Error/i, INCONCLUSIVE: /Inconclusive/i}; - if (!status[verdict].test(row[2])) return; + if (!standalone && !status[verdict].test(row[2])) return; const merged = [...new Set([...result.toLowerCase().matchAll(/semantic_merged sha=([a-f0-9]{40})\b/g)].map(m => m[1]))]; if (merged.length > 1) return; // This checks citation presence, not the correctness of the AI's reasoning. @@ -151,7 +156,7 @@ async function publish({github, context, core}) { if (!result) { if (preview) { const message = `No verified CodeRabbit verdict for head ${pair.head} + ${pair.branch} ${pair.target}. ` + - 'Request a custom evaluation and rerun the tests and result lookup job after the reply arrives.'; + 'Request a semantic evaluation and rerun the tests and result lookup job after the reply arrives.'; core.warning(message); await core.summary.addRaw(`${message}\n\n${NOTICE}`).write(); } @@ -208,4 +213,4 @@ async function publish({github, context, core}) { } module.exports = publish; -Object.assign(module.exports, {NAME, AUDIT, NOTICE, supported, compare, requests, parseResult, matches, identity, evidence, candidateTree}); +Object.assign(module.exports, {NAME, AUDIT, NOTICE, supported, compare, requests, parseResult, matches, identity, evidence, candidateTree, isCommandUser}); diff --git a/.github/workflows/coderabbit-semantic-review.yml b/.github/workflows/coderabbit-semantic-review.yml index 87f6387928d5..27340b96e458 100644 --- a/.github/workflows/coderabbit-semantic-review.yml +++ b/.github/workflows/coderabbit-semantic-review.yml @@ -20,7 +20,7 @@ on: branches: [main, 'release/**'] types: [opened, reopened, synchronize, ready_for_review, labeled, edited, auto_merge_enabled, closed] schedule: - - cron: '23 * * * *' + - cron: '23 */6 * * *' issue_comment: types: [created, edited] workflow_dispatch: @@ -46,7 +46,7 @@ jobs: contains(fromJSON('["schedule", "pull_request_target", "workflow_dispatch"]'), github.event_name) && (github.event.action != 'closed' || github.event.pull_request.merged) && (github.event.action != 'labeled' || github.event.label.name == 'ci: full pre-merge approved') - # Requests and publications share a queue: hourly scans cannot race PR events. + # Requests and publications share a queue: scheduled scans cannot race PR events. # ponytail: one queue, 100 pending; use per-PR dispatch if scans delay events. concurrency: group: coderabbit-semantic-state @@ -97,10 +97,15 @@ jobs: env: DISPATCH_PULL_NUMBER: ${{ inputs.pull_number }} SEMANTIC_APPROVAL_VALIDATED: ${{ steps.approval.outputs.result }} + SEMANTIC_COMMAND_TOKEN: ${{ secrets.TRTLLM_AGENT_SHARED_TOKEN }} with: script: | const request = require('./.github/scripts/coderabbit_semantic_review_request.js'); - await request({github, context, core}); + if (!process.env.SEMANTIC_COMMAND_TOKEN) throw new Error('Missing semantic command token.'); + const commandGithub = new github.constructor({ + auth: process.env.SEMANTIC_COMMAND_TOKEN, retry: {enabled: false}, + }); + await request({github, commandGithub, context, core}); publish-result: name: Publish advisory semantic result @@ -114,7 +119,8 @@ jobs: github.event_name == 'issue_comment' && github.event.issue.pull_request && github.event.comment.user.login == 'coderabbitai[bot]' && - (contains(github.event.comment.body, 'pre-merge-checks-results') || + (contains(github.event.comment.body, 'SEMANTIC_REVIEW_V3') || + contains(github.event.comment.body, 'pre-merge-checks-results') || contains(github.event.comment.body, 'pre_merge_checks_walkthrough_start')) concurrency: group: coderabbit-semantic-state diff --git a/AGENTS.md b/AGENTS.md index d5cd2be9ad24..a13f6ac3c281 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -178,85 +178,8 @@ For a full list of up-to-date bot commands, post `/bot help` as a PR comment and ### Advisory semantic conflict review -The `CodeRabbit Semantic Conflict Review` workflow performs best-effort semantic -compatibility analysis for open, non-draft PRs targeting `main` or `release/**`. -No opt-in label is needed. It compares both branches from their merge base and -follows affected callers, contracts, configuration, and tests across files. - -- PR creation, reopening, updates, and becoming ready evaluate the threshold; - they do not automatically spend an AI call. An hourly scan also evaluates it. -- The first analysis needs new target commits and either 24 hours since the - merge-base commit or at least 30 target commits beyond that base. After a - completed PASS/FAIL analysis, count from its target SHA and completion time. - PR updates invalidate the old verdict but do not bypass these thresholds. -- An authorized `ci: full pre-merge approved` label or enabling auto-merge - bypasses the threshold. Approval-label authors are checked against the existing - `trt-llm-ci-approvers` team using its existing token. All pre-merge requests - share a one-hour cooldown, including when the SHA pair changes. A signal - during cooldown is skipped; ordinary scans and the post-merge audit remain. -- Exact revision pairs, including requests still awaiting a reply, are deduplicated. - With no completed analysis, new-pair routine requests wait 24 hours to avoid - repeated service-failure retries. A missing reply for the same pair requires - a manual retry. Workflow dispatch accepts one PR number and bypasses the - thresholds, cooldown and deduplication for that PR only. -- Merge events request a post-merge audit regardless of thresholds or cooldown. - The hourly scan recovers merges from the preceding 24 hours. The audit pins - the actual merge commit and historical target, including for release PRs; - later target updates do not invalidate it. Squash-only rules or the two-parent - merge must establish the historical target; ambiguous rebase history is rejected. - A pre-merge result/request can be reused only when its head/target pair and - GitHub's recorded test-merge tree match the actual merged tree. Otherwise the - audit includes the actual merged code in a new analysis request. - -This dedicated custom check is `off` in `.coderabbit.yaml` during ordinary -reviews. The workflow explicitly requests `evaluate custom pre-merge check` -with warning mode and the configured instructions so regular reviews cannot -bypass the spending policy. CodeRabbit Custom Pre-Merge Checks access is needed. -Its native Post-Merge Actions only support the default branch; this workflow -uses an explicit command on the merged PR instead. Acceptance of Actions-bot -commands, especially on merged/release PRs, requires deployment validation. -An absent/rejected AI reply remains without a verdict, never a semantic pass. - -The `Semantic conflict with target branch` Check starts neutral. Stale results -become neutral when the PR event or hourly scan observes a version change. -The verifier checks the bot identity, most recent trusted request, exact revision -record, and GitHub merge base. Publication and preview select the newest applicable -reply after that request; delayed events cannot restore an older verdict, and -pending manual retries cannot reuse an earlier PASS. Before deployment, the preview -also accepts manual evaluations when no trusted request exists for the pair. -PASS becomes success; FAIL makes the Check and -publishing job red; Inconclusive remains neutral. A successful request job only -means orchestration succeeded. Checks on the actual merge SHA use the distinct -`Semantic conflict audit (post-merge)` name, with a receipt linking the analysis -on the original PR. Evidence includes code locations and regression scenarios. -The instructions first discover cross-branch interactions, then verify their -contracts, including test replacements and the production paths they exercise. -Explanations precede the machine record and must cite immutable source links -with full SHAs and line numbers from both head and target. A PASS/FAIL without -those citations becomes Inconclusive, including in the preview; an older PASS -cannot substitute for that incomplete reply. Citation presence does not prove -the AI's reasoning or the cited code is correct. - -CodeRabbit can make mistakes, including false positives. Keep these checks and -workflows non-required: their failures then do not block merging. No required -waiting gate is added, and auto-merge does not wait for this analysis. Audit does -not revert code or modify branches. Repository rules remain unchanged. -The thresholds and cooldown limit frequency, not total calls per PR. - -Changes to this automation run the separate read-only `CodeRabbit Semantic -Review Preview` workflow, including fork drafts. `Automation tests and result -lookup (not AI approval)` runs Node tests and reads actual CodeRabbit replies; -`AI verdict (advisory; skipped = unavailable)` runs only for a verified current -PASS/FAIL and is otherwise gray/skipped. `precommit-check.yml` is unchanged. - -Both workflows use the same verifier. Privileged jobs load only trusted default -branch scripts, never PR code. The preview has read-only permissions. Before -merge, request `@coderabbitai evaluate custom pre-merge check` with the name -`Semantic conflict with target branch`, `--mode warning`, and `--instructions` -containing the configured instructions and fixed head/target/merge-base SHAs. -After the reply arrives, rerun the **tests and result lookup** job; rerunning only -the AI job reuses old outputs. Preview tests do not establish production trigger, -permission, command-acceptance or post-merge behavior. +For trigger rules, retries and result verification, see the +[semantic review operator guide](.github/coderabbit-semantic-review.md). ### Trouble Shooting From 664b0d978786580ce9f5b0f2d72c445e580f1ed1 Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Thu, 24 Sep 2026 10:07:12 +0800 Subject: [PATCH 18/21] [None][infra] Tighten semantic reply format and publication trigger Signed-off-by: Yanchao Lu --- .coderabbit.yaml | 3 +-- .github/workflows/coderabbit-semantic-review.yml | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 5cfb03672509..9228047f0411 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -112,8 +112,7 @@ reviews: SEMANTIC_REVIEW_V3 on its own line. Include the full evidence for every verdict, including PASS, using immutable GitHub blob links with full SHAs and line numbers from head and target. Include this record on one line: - SEMANTIC_RESULT - head= target= merge_base= verdict= + SEMANTIC_RESULT head= target= merge_base= verdict= Use verified full lowercase SHAs; omit the record if unavailable. A record alone is insufficient. diff --git a/.github/workflows/coderabbit-semantic-review.yml b/.github/workflows/coderabbit-semantic-review.yml index 27340b96e458..ee394816dc99 100644 --- a/.github/workflows/coderabbit-semantic-review.yml +++ b/.github/workflows/coderabbit-semantic-review.yml @@ -119,9 +119,7 @@ jobs: github.event_name == 'issue_comment' && github.event.issue.pull_request && github.event.comment.user.login == 'coderabbitai[bot]' && - (contains(github.event.comment.body, 'SEMANTIC_REVIEW_V3') || - contains(github.event.comment.body, 'pre-merge-checks-results') || - contains(github.event.comment.body, 'pre_merge_checks_walkthrough_start')) + contains(github.event.comment.body, 'SEMANTIC_REVIEW_V3') concurrency: group: coderabbit-semantic-state queue: max From 725778c8e0a39d0d30a265e55c87a90ecc331895 Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Thu, 24 Sep 2026 13:50:08 +0800 Subject: [PATCH 19/21] [None][infra] Refresh changed semantic review revisions during scheduled scans Signed-off-by: Yanchao Lu --- .github/coderabbit-semantic-review.md | 12 ++- .../coderabbit_semantic_review.test.js | 85 ++++++++++++++++--- .../coderabbit_semantic_review_request.js | 29 ++++--- 3 files changed, 97 insertions(+), 29 deletions(-) diff --git a/.github/coderabbit-semantic-review.md b/.github/coderabbit-semantic-review.md index 268474667942..6e1fe44193b4 100644 --- a/.github/coderabbit-semantic-review.md +++ b/.github/coderabbit-semantic-review.md @@ -11,11 +11,17 @@ No opt-in label is needed. It compares both branches from their merge base and follows affected callers, contracts, configuration, and tests across files. - PR creation, reopening, updates, and becoming ready evaluate the threshold; - they do not automatically spend an AI call. A scan every six hours also evaluates it. + they do not automatically spend an AI call. - The first analysis needs new target commits and either 24 hours since the merge-base commit or at least 30 target commits beyond that base. After a - completed PASS/FAIL analysis, count from its target SHA and completion time. - PR updates invalidate the old verdict but do not bypass these thresholds. + completed PASS/FAIL analysis, PR events count from its target SHA and completion + time. PR updates invalidate the old verdict but do not bypass these thresholds + in the event handler. +- Every six hours, a scan refreshes a completed PASS/FAIL analysis when either + the PR head or target SHA has changed. It bypasses the 24-hour / 30-commit + threshold, including when only the PR changes. The first analysis still uses + the threshold. Deduplication, the one-hour cooldown, unusable-reply protection + and scan limits still apply, so a refresh is not guaranteed in the next scan. - An authorized `ci: full pre-merge approved` label or enabling auto-merge bypasses the threshold. Approval-label authors are checked against the existing `trt-llm-ci-approvers` team using its existing token. All pre-merge requests diff --git a/.github/scripts/coderabbit_semantic_review.test.js b/.github/scripts/coderabbit_semantic_review.test.js index 6b543226c57e..5481cd89a266 100644 --- a/.github/scripts/coderabbit_semantic_review.test.js +++ b/.github/scripts/coderabbit_semantic_review.test.js @@ -190,15 +190,17 @@ function harness(overrides = {}) { for (const [count, age, expected] of [[0, 3 * DAY, 0], [29, DAY - 1, 0], [30, 0, 1], [1, DAY, 1], [29, DAY, 1]]) { test(`first analysis: ${count} target commits, age ${age} => ${expected} requests`, async () => { - const h = harness(); h.setLag(count, age); await h.run(); - assert.equal(h.posted.length, expected); - assert.equal(h.checks[0].conclusion, 'neutral'); - if (expected) { - assert.match(h.posted[0].body, /^@coderabbitai\n/); - assert.match(h.posted[0].body, /normal PR chat comment/); - assert.match(h.posted[0].body, /SEMANTIC_REVIEW_V3/); - assert.match(h.posted[0].body, /No AI verdict is asserted/); - assert.ok(h.posted[0].body.includes(HEAD) && h.posted[0].body.includes(BASE)); + for (const event of ['pull_request_target', 'schedule']) { + const h = harness(); h.setLag(count, age); await h.run(event); + assert.equal(h.posted.length, expected); + assert.equal(h.checks[0].conclusion, 'neutral'); + if (expected) { + assert.match(h.posted[0].body, /^@coderabbitai\n/); + assert.match(h.posted[0].body, /normal PR chat comment/); + assert.match(h.posted[0].body, /SEMANTIC_REVIEW_V3/); + assert.match(h.posted[0].body, /No AI verdict is asserted/); + assert.ok(h.posted[0].body.includes(HEAD) && h.posted[0].body.includes(BASE)); + } } }); } @@ -241,19 +243,21 @@ test('dispatch rejects malformed or missing PRs without touching other PRs', asy await assert.rejects(h.run('push'), /Unsupported event/); }); -test('subsequent thresholds use the last completed analysis, not the original old base', async () => { +test('PR event thresholds use the last completed analysis, not the original old base', async () => { const h = harness(); h.setLag(80, 3 * DAY); await h.run(); h.reply(); - h.setTarget(NEW_BASE); h.setNow(NOW + 2 * HOUR); h.setProgress(1); await h.run('schedule'); + h.setTarget(NEW_BASE); h.setNow(NOW + 2 * HOUR); h.setProgress(1); await h.run(); assert.equal(h.posted.length, 1); assert.equal(h.checks[0].conclusion, 'neutral'); assert.match(h.checks[0].output.title, /stale/); - h.setProgress(30); await h.run('schedule'); assert.equal(h.posted.length, 2); + h.setProgress(30); await h.run(); assert.equal(h.posted.length, 2); }); -test('24 hours with new target commits triggers, while an unchanged target never does', async () => { +test('PR events require target progress even after 24 hours', async () => { for (const [progress, expected] of [[0, 1], [1, 2]]) { const h = harness(); await h.run(); h.reply(); h.setNow(NOW + DAY); - h.setTarget(NEW_BASE); h.setProgress(progress); await h.run('schedule'); + if (progress) h.setTarget(NEW_BASE); + else h.pr.head.sha = MERGED; + h.setProgress(progress); await h.run(); assert.equal(h.posted.length, expected); } }); @@ -266,6 +270,59 @@ test('a PR head update invalidates the verdict without bypassing the target thre assert.equal(h.checks.at(-1).conclusion, 'neutral'); }); +for (const changed of ['head', 'target', 'both']) { + test(`scheduled refresh bypasses thresholds after a ${changed} change`, async () => { + for (const branch of ['main', 'release/1.2']) { + for (const verdict of ['PASS', 'FAIL']) { + const h = harness({base: {ref: branch}}); await h.run(); await h.publish(h.reply(verdict)); + h.setNow(NOW + 6 * HOUR); h.setLag(1, 0); + if (changed !== 'target') h.pr.head.sha = MERGED; + if (changed !== 'head') h.setTarget(NEW_BASE); + h.setProgress(changed === 'head' ? 0 : 1); + await h.run('pull_request_target', 'synchronize'); + assert.equal(h.posted.length, 1); + await h.run('schedule'); + assert.equal(h.posted.length, 2); + const pair = requests(h.comments)[0]; + assert.equal(pair.head, changed === 'target' ? HEAD : MERGED); + assert.equal(pair.target, changed === 'head' ? BASE : NEW_BASE); + assert.equal(pair.branch, branch); + assert.equal(pair.reason, 'Scheduled refresh of changed revisions'); + assert.equal(h.checks.at(-1).conclusion, 'neutral'); + h.setNow(NOW + 12 * HOUR); await h.run('schedule'); + assert.equal(h.posted.length, 2); // Await the new pair's reply. + } + } + }); +} + +test('scheduled refresh deduplicates an unchanged completed pair across scans', async () => { + const h = harness(); await h.run(); await h.publish(h.reply()); + for (const elapsed of [6 * HOUR, DAY, 2 * DAY]) { + h.setNow(NOW + elapsed); await h.run('schedule'); + assert.equal(h.posted.length, 1); + assert.equal(h.checks[0].conclusion, 'success'); + } +}); + +test('scheduled refresh respects cooldown and catches a head-only change in a later scan', async () => { + const h = harness(); await h.run(); h.reply(); + h.pr.head.sha = MERGED; h.setProgress(0); h.setNow(NOW + HOUR - 1); + await h.run('schedule'); assert.equal(h.posted.length, 1); + assert.match(h.checks.at(-1).output.summary, /cooldown/); + h.setNow(NOW + HOUR); await h.run('schedule'); + assert.equal(h.posted.length, 2); +}); + +test('an older PASS does not bypass a pending newer request during scheduled refresh', async () => { + const h = harness(); await h.run(); h.reply(); await h.run('workflow_dispatch'); + h.pr.head.sha = MERGED; h.setProgress(0); h.setNow(NOW + 6 * HOUR); + await h.run('schedule'); assert.equal(h.posted.length, 2); + assert.match(h.checks.at(-1).output.summary, /no verified verdict/); + h.reply('PASS', requests(h.comments)[0]); await h.run('schedule'); + assert.equal(h.posted.length, 3); +}); + test('approval and auto-merge bypass thresholds but share a one-hour cooldown across SHAs', async () => { const h = harness({labels: [{name: APPROVED}], auto_merge: {enabled_by: 'maintainer'}}); h.setLag(0, 0); await h.run('pull_request_target', 'labeled', '12', true); diff --git a/.github/scripts/coderabbit_semantic_review_request.js b/.github/scripts/coderabbit_semantic_review_request.js index 762b68c9ae5e..275486f1239f 100644 --- a/.github/scripts/coderabbit_semantic_review_request.js +++ b/.github/scripts/coderabbit_semantic_review_request.js @@ -104,20 +104,25 @@ async function requestOne({github, commandGithub, context, core, number, manual, await ensureCheck('Previous analysis has no verified verdict; use a manual retry. Routine requests are paused.'); return; } - let count = pair.comparison.behind_by; - let since = Date.parse(pair.comparison.merge_base_commit.commit.committer.date); - if (previous) { - const progress = await compare(github, repo, last.target, pair.target); - if (['ahead', 'identical'].includes(progress.status)) { - count = progress.ahead_by; - since = Date.parse(previous.comment.created_at); + if (previous && context.eventName === 'schedule' && + (last.head !== pair.head || last.target !== pair.target)) { + reason = 'Scheduled refresh of changed revisions'; + } else { + let count = pair.comparison.behind_by; + let since = Date.parse(pair.comparison.merge_base_commit.commit.committer.date); + if (previous) { + const progress = await compare(github, repo, last.target, pair.target); + if (['ahead', 'identical'].includes(progress.status)) { + count = progress.ahead_by; + since = Date.parse(previous.comment.created_at); + } } + if (!count || (count < 30 && now - since < DAY)) { + await ensureCheck('Below the 24-hour / 30-commit threshold; waiting for target changes.'); + return; + } + reason = '24-hour / 30-commit threshold'; } - if (!count || (count < 30 && now - since < DAY)) { - await ensureCheck('Below the 24-hour / 30-commit threshold; waiting for target changes.'); - return; - } - reason = '24-hour / 30-commit threshold'; } // Approval and auto-merge share this budget even when the branch SHAs change. // Count any recent pre-merge request, so a routine scan cannot double the cost. From 991804e0d08e7b1586d0848c4af670b3a56fdc51 Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Thu, 24 Sep 2026 13:54:23 +0800 Subject: [PATCH 20/21] [None][infra] Limit semantic review edit events to target branch changes Signed-off-by: Yanchao Lu --- .github/coderabbit-semantic-review.md | 7 +++-- .../coderabbit_semantic_review.test.js | 28 ++++++++++++++++++- .../workflows/coderabbit-semantic-review.yml | 1 + 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/.github/coderabbit-semantic-review.md b/.github/coderabbit-semantic-review.md index 6e1fe44193b4..0f6e2ae04613 100644 --- a/.github/coderabbit-semantic-review.md +++ b/.github/coderabbit-semantic-review.md @@ -7,11 +7,12 @@ SPDX-License-Identifier: Apache-2.0 The `CodeRabbit Semantic Conflict Review` workflow performs best-effort semantic compatibility analysis for open, non-draft PRs targeting `main` or `release/**`. -No opt-in label is needed. It compares both branches from their merge base and +It compares both branches from their merge base and follows affected callers, contracts, configuration, and tests across files. -- PR creation, reopening, updates, and becoming ready evaluate the threshold; - they do not automatically spend an AI call. +- PR creation, reopening, commit updates, becoming ready, and target-branch changes + evaluate the threshold; they do not automatically spend an AI call. Title and + description edits skip the request job before checkout or PR API reads. - The first analysis needs new target commits and either 24 hours since the merge-base commit or at least 30 target commits beyond that base. After a completed PASS/FAIL analysis, PR events count from its target SHA and completion diff --git a/.github/scripts/coderabbit_semantic_review.test.js b/.github/scripts/coderabbit_semantic_review.test.js index 5481cd89a266..29324dfb12ad 100644 --- a/.github/scripts/coderabbit_semantic_review.test.js +++ b/.github/scripts/coderabbit_semantic_review.test.js @@ -217,7 +217,7 @@ test('creation and ready events use the threshold; drafts and unrelated targets } }); -test('release PRs use their actual target and need no opt-in label', async () => { +test('release PRs use their actual target branch', async () => { const h = harness({base: {ref: 'release/1.2'}}); await h.run(); assert.equal(h.posted.length, 1); assert.ok(h.calls.filter(([kind]) => kind === 'ref').every(([, args]) => args.ref === 'heads/release/1.2')); @@ -225,6 +225,32 @@ test('release PRs use their actual target and need no opt-in label', async () => assert.equal(h.checks[0].conclusion, 'success'); }); +test('the request job accepts base-branch edits and skips title, description and absent changes', () => { + const text = fs.readFileSync(path.join(__dirname, '../workflows/coderabbit-semantic-review.yml'), 'utf8'); + const condition = text.match(/request-review:[\s\S]*? if: >-\n((?: {6}[^\n]+\n)+)/)[1]; + // Missing GitHub context properties evaluate as empty; optional chaining models that here. + const evaluate = new Function('github', 'contains', 'fromJSON', + `return Boolean(${condition.replace('github.event.changes.base.ref', 'github.event.changes?.base?.ref')});`); + const github = {repository: 'NVIDIA/TensorRT-LLM', event_name: 'pull_request_target', event: {}}; + const allowed = event => { + github.event = event; + return evaluate(github, (list, value) => list.includes(value), JSON.parse); + }; + for (const changes of [undefined, {}, {title: {from: 'Old title'}}, {body: {from: 'Old body'}}, + {title: {from: 'Old title'}, body: {from: 'Old body'}}, {base: {sha: {from: BASE}}}]) { + assert.equal(allowed({action: 'edited', changes}), false); + } + for (const from of ['main', 'release/1.2']) { + assert.equal(allowed({action: 'edited', changes: {base: {ref: {from}}}}), true); + } + for (const action of ['opened', 'reopened', 'synchronize', 'ready_for_review', 'auto_merge_enabled']) { + assert.equal(allowed({action}), true); + } + for (const event_name of ['schedule', 'workflow_dispatch']) { + github.event_name = event_name; assert.equal(allowed({}), true); + } +}); + test('a six-hour scan visits eligible PRs; a PR event or manual retry visits only its PR', async () => { const h = harness(); h.prs.push({...h.pr, number: 13}); await h.run(); assert.deepEqual(h.posted.map(c => c.issue_number), [12]); diff --git a/.github/workflows/coderabbit-semantic-review.yml b/.github/workflows/coderabbit-semantic-review.yml index ee394816dc99..778717bcf1f8 100644 --- a/.github/workflows/coderabbit-semantic-review.yml +++ b/.github/workflows/coderabbit-semantic-review.yml @@ -45,6 +45,7 @@ jobs: github.repository == 'NVIDIA/TensorRT-LLM' && contains(fromJSON('["schedule", "pull_request_target", "workflow_dispatch"]'), github.event_name) && (github.event.action != 'closed' || github.event.pull_request.merged) && + (github.event.action != 'edited' || github.event.changes.base.ref) && (github.event.action != 'labeled' || github.event.label.name == 'ci: full pre-merge approved') # Requests and publications share a queue: scheduled scans cannot race PR events. # ponytail: one queue, 100 pending; use per-PR dispatch if scans delay events. From 264edc7401b57c3554fa238a3f2a1a46a13e1713 Mon Sep 17 00:00:00 2001 From: Yanchao Lu Date: Thu, 24 Sep 2026 14:50:55 +0800 Subject: [PATCH 21/21] [None][infra] Unify semantic recheck thresholds and bound unavailable-result retries Signed-off-by: Yanchao Lu --- .github/coderabbit-semantic-review.md | 49 +++---- .../coderabbit_semantic_review.test.js | 129 ++++++++++++------ .../coderabbit_semantic_review_request.js | 59 ++++---- 3 files changed, 143 insertions(+), 94 deletions(-) diff --git a/.github/coderabbit-semantic-review.md b/.github/coderabbit-semantic-review.md index 0f6e2ae04613..833c9935c461 100644 --- a/.github/coderabbit-semantic-review.md +++ b/.github/coderabbit-semantic-review.md @@ -10,30 +10,28 @@ compatibility analysis for open, non-draft PRs targeting `main` or `release/**`. It compares both branches from their merge base and follows affected callers, contracts, configuration, and tests across files. -- PR creation, reopening, commit updates, becoming ready, and target-branch changes - evaluate the threshold; they do not automatically spend an AI call. Title and - description edits skip the request job before checkout or PR API reads. +- PR creation, reopening, commit updates, becoming ready, target-branch changes + and six-hour scans evaluate the same threshold. - The first analysis needs new target commits and either 24 hours since the merge-base commit or at least 30 target commits beyond that base. After a - completed PASS/FAIL analysis, PR events count from its target SHA and completion - time. PR updates invalidate the old verdict but do not bypass these thresholds - in the event handler. -- Every six hours, a scan refreshes a completed PASS/FAIL analysis when either - the PR head or target SHA has changed. It bypasses the 24-hour / 30-commit - threshold, including when only the PR changes. The first analysis still uses - the threshold. Deduplication, the one-hour cooldown, unusable-reply protection - and scan limits still apply, so a refresh is not guaranteed in the next scan. + completed PASS/FAIL analysis, a changed head or target qualifies after 24 hours + from that reply or 30 additional target commits. A head-only change therefore + qualifies after 24 hours even when the target has not advanced. If the latest + request has no valid verdict, use its request time and target as the baseline. + Rewritten target history falls back to the merge-base threshold. - An authorized `ci: full pre-merge approved` label or enabling auto-merge bypasses the threshold. Approval-label authors are checked against the existing - `trt-llm-ci-approvers` team using its existing token. All pre-merge requests - share a one-hour cooldown, including when the SHA pair changes. A signal + `trt-llm-ci-approvers` team using its existing token. Automatic pre-merge requests + for each PR and target branch share a one-hour cooldown, even across SHA changes. A signal during cooldown is skipped; ordinary scans and the post-merge audit remain. -- Exact revision pairs, including requests still awaiting a reply, are deduplicated. - A missing, truncated or inconclusive latest reply pauses routine requests, - including for new revision pairs, until a verified reply or a manual/merge-intent - retry. This avoids repeatedly paying for unusable replies. Workflow dispatch - accepts one PR number and bypasses the thresholds, cooldown and deduplication - for that PR only. +- Exact revision pairs are deduplicated. When a reply is missing, incomplete or + Inconclusive, a scheduled scan may retry that version once, at least six hours + after its latest request. This applies to pre-merge analysis and audits. A + verified FAIL is a completed analysis, not a reason to retry. After the one + automatic retry, the same version needs a manual retry; changed versions are + evaluated under the ordinary threshold and get their own retry allowance. + Workflow dispatch accepts one PR number and bypasses the thresholds, cooldown + and deduplication for that PR only. - Merge events request a post-merge audit regardless of thresholds or cooldown. The six-hour scan recovers merges from the preceding 24 hours. The audit pins the actual merge commit and historical target, including for release PRs; @@ -62,22 +60,27 @@ PRs, requires a deployment pilot. Missing replies never imply semantic success. Scheduled scans run at minute 23 every six hours (UTC). Each scan sends at most 20 new AI requests, including audits, to avoid an initial burst across all open PRs. Recent merged PRs are considered first; open PRs use a rotating starting -point. Exact-pair deduplication avoids resending completed or in-flight requests. +point. The single automatic retry also counts toward the 20-request limit. Scans restart from live state, not a saved cursor. Saturation can delay PRs; use a manual dispatch for a specific PR rather than relying on a resume guarantee. Each token's remaining REST budget is read once, then tracked from response headers, including pagination. A scan stops below a 100-request reserve or on primary/secondary rate limiting and reports processed PRs and new requests. -Ordinary permission errors remain failures. Limits on a scheduled scan do not -change single-PR event or manual-dispatch behavior. Frequency reduction does not -reduce the peak cost of one scan. A rate-limited or missed scan can also delay +Ordinary permission errors remain failures. The 20-request cap and 100-request +reserve apply only to scheduled scans; GitHub's own rate limits can affect every +API operation, including single-PR events, manual requests and result publication. +Frequency reduction does not reduce the peak cost of one scan. A rate-limited or missed scan can also delay post-merge recovery beyond its 24-hour lookback. ## Result verification The `Semantic conflict with target branch` Check starts neutral. Stale results become neutral when the PR event or scheduled scan observes a version change. +This rule applies to every pre-merge verdict; historical audit verdicts stay +attached to their fixed merged revisions. Only formatted CodeRabbit analysis +replies update the Check. Receiving a reply publishes a result, not another AI +request. The verifier checks the bot identity, most recent trusted request, exact revision record, and GitHub merge base. Publication and preview select the newest applicable reply after that request; delayed events cannot restore an older verdict, and diff --git a/.github/scripts/coderabbit_semantic_review.test.js b/.github/scripts/coderabbit_semantic_review.test.js index 29324dfb12ad..d3541c13b3d0 100644 --- a/.github/scripts/coderabbit_semantic_review.test.js +++ b/.github/scripts/coderabbit_semantic_review.test.js @@ -269,22 +269,14 @@ test('dispatch rejects malformed or missing PRs without touching other PRs', asy await assert.rejects(h.run('push'), /Unsupported event/); }); -test('PR event thresholds use the last completed analysis, not the original old base', async () => { - const h = harness(); h.setLag(80, 3 * DAY); await h.run(); h.reply(); - h.setTarget(NEW_BASE); h.setNow(NOW + 2 * HOUR); h.setProgress(1); await h.run(); - assert.equal(h.posted.length, 1); - assert.equal(h.checks[0].conclusion, 'neutral'); - assert.match(h.checks[0].output.title, /stale/); - h.setProgress(30); await h.run(); assert.equal(h.posted.length, 2); -}); - -test('PR events require target progress even after 24 hours', async () => { - for (const [progress, expected] of [[0, 1], [1, 2]]) { - const h = harness(); await h.run(); h.reply(); h.setNow(NOW + DAY); - if (progress) h.setTarget(NEW_BASE); - else h.pr.head.sha = MERGED; - h.setProgress(progress); await h.run(); - assert.equal(h.posted.length, expected); +test('events and scans count target progress from the last analysis, not the old merge base', async () => { + for (const event of ['pull_request_target', 'schedule']) { + const h = harness(); h.setLag(80, 3 * DAY); await h.run(); h.reply(); + h.setTarget(NEW_BASE); h.setNow(NOW + 2 * HOUR); h.setProgress(29); await h.run(event); + assert.equal(h.posted.length, 1); + assert.equal(h.checks[0].conclusion, 'neutral'); + assert.match(h.checks[0].output.title, /stale/); + h.setProgress(30); await h.run(event); assert.equal(h.posted.length, 2); } }); @@ -297,7 +289,7 @@ test('a PR head update invalidates the verdict without bypassing the target thre }); for (const changed of ['head', 'target', 'both']) { - test(`scheduled refresh bypasses thresholds after a ${changed} change`, async () => { + test(`events and scans require 24 hours for a ${changed} change below 30 target commits`, async () => { for (const branch of ['main', 'release/1.2']) { for (const verdict of ['PASS', 'FAIL']) { const h = harness({base: {ref: branch}}); await h.run(); await h.publish(h.reply(verdict)); @@ -307,15 +299,17 @@ for (const changed of ['head', 'target', 'both']) { h.setProgress(changed === 'head' ? 0 : 1); await h.run('pull_request_target', 'synchronize'); assert.equal(h.posted.length, 1); - await h.run('schedule'); + await h.run('schedule'); assert.equal(h.posted.length, 1); + h.setNow(NOW + DAY - 1); await h.run('schedule'); assert.equal(h.posted.length, 1); + h.setNow(NOW + DAY); await h.run(); assert.equal(h.posted.length, 2); const pair = requests(h.comments)[0]; assert.equal(pair.head, changed === 'target' ? HEAD : MERGED); assert.equal(pair.target, changed === 'head' ? BASE : NEW_BASE); assert.equal(pair.branch, branch); - assert.equal(pair.reason, 'Scheduled refresh of changed revisions'); + assert.equal(pair.reason, '24-hour / 30-commit threshold'); assert.equal(h.checks.at(-1).conclusion, 'neutral'); - h.setNow(NOW + 12 * HOUR); await h.run('schedule'); + h.setNow(NOW + DAY + HOUR); await h.run('schedule'); assert.equal(h.posted.length, 2); // Await the new pair's reply. } } @@ -323,29 +317,31 @@ for (const changed of ['head', 'target', 'both']) { } test('scheduled refresh deduplicates an unchanged completed pair across scans', async () => { - const h = harness(); await h.run(); await h.publish(h.reply()); - for (const elapsed of [6 * HOUR, DAY, 2 * DAY]) { - h.setNow(NOW + elapsed); await h.run('schedule'); - assert.equal(h.posted.length, 1); - assert.equal(h.checks[0].conclusion, 'success'); + for (const verdict of ['PASS', 'FAIL']) { + const h = harness(); await h.run(); await h.publish(h.reply(verdict)); + for (const elapsed of [6 * HOUR, DAY, 2 * DAY]) { + h.setNow(NOW + elapsed); await h.run('schedule'); + assert.equal(h.posted.length, 1); + assert.equal(h.checks[0].conclusion, verdict === 'PASS' ? 'success' : 'failure'); + } } }); -test('scheduled refresh respects cooldown and catches a head-only change in a later scan', async () => { +test('scheduled refresh respects cooldown even when 30 target commits qualify', async () => { const h = harness(); await h.run(); h.reply(); - h.pr.head.sha = MERGED; h.setProgress(0); h.setNow(NOW + HOUR - 1); + h.setTarget(NEW_BASE); h.setProgress(30); h.setNow(NOW + HOUR - 1); await h.run('schedule'); assert.equal(h.posted.length, 1); assert.match(h.checks.at(-1).output.summary, /cooldown/); h.setNow(NOW + HOUR); await h.run('schedule'); assert.equal(h.posted.length, 2); }); -test('an older PASS does not bypass a pending newer request during scheduled refresh', async () => { - const h = harness(); await h.run(); h.reply(); await h.run('workflow_dispatch'); - h.pr.head.sha = MERGED; h.setProgress(0); h.setNow(NOW + 6 * HOUR); +test('new revisions use the latest request time when that request has no valid reply', async () => { + const h = harness(); await h.run(); h.reply(); h.setNow(NOW + 2 * HOUR); + await h.run('workflow_dispatch'); + h.pr.head.sha = MERGED; h.setProgress(0); h.setNow(NOW + DAY); await h.run('schedule'); assert.equal(h.posted.length, 2); - assert.match(h.checks.at(-1).output.summary, /no verified verdict/); - h.reply('PASS', requests(h.comments)[0]); await h.run('schedule'); + h.setNow(NOW + DAY + 2 * HOUR); await h.run('schedule'); assert.equal(h.posted.length, 3); }); @@ -641,7 +637,7 @@ test('chat transport preserves author, revision, citation and unique-record vali } }); -test('missing, truncated or inconclusive replies cannot cause daily new-pair AI retries', async () => { +test('missing, truncated or inconclusive replies get one scheduled retry after six hours', async () => { for (const kind of ['missing', 'truncated-record', 'truncated-evidence', 'inconclusive']) { const h = harness(); await h.run(); if (kind !== 'missing') { @@ -653,16 +649,58 @@ test('missing, truncated or inconclusive replies cannot cause daily new-pair AI `| Semantic conflict with target branch | ✅ Passed | ${explanation.slice(0, 201)} |`; } } - for (let day = 1; day <= 7; day++) { - h.setNow(NOW + day * DAY); h.setTarget(day.toString().repeat(40)); await h.run('schedule'); - } + h.setNow(NOW + 6 * HOUR - 1); await h.run('schedule'); assert.equal(h.posted.length, 1, kind); - assert.match(h.checks.at(-1).output.summary, /manual retry/); - await h.run('workflow_dispatch'); assert.equal(h.posted.length, 2); + h.setNow(NOW + 6 * HOUR); await h.run(); assert.equal(h.posted.length, 1); + await h.run('schedule'); assert.equal(h.posted.length, 2, kind); + assert.equal(requests(h.comments)[0].retry, true); + assert.equal(h.checks.at(-1).conclusion, 'neutral'); + h.reply('INCONCLUSIVE'); + for (const elapsed of [12 * HOUR, DAY, 7 * DAY]) { + h.setNow(NOW + elapsed); await h.run('schedule'); + assert.equal(h.posted.length, 2, kind); + } + await h.run('workflow_dispatch'); assert.equal(h.posted.length, 3); await h.publish(chatReply(h.reply())); assert.equal(h.checks.at(-1).conclusion, 'success'); } }); +test('changed revisions are reassessed after an unavailable result and receive a fresh retry allowance', async () => { + const h = harness(); h.setLag(80, 3 * DAY); await h.run(); h.reply('INCONCLUSIVE'); + h.setNow(NOW + 6 * HOUR); await h.run('schedule'); assert.equal(h.posted.length, 2); + h.pr.head.sha = MERGED; h.setProgress(0); + h.setNow(NOW + DAY); await h.run('schedule'); assert.equal(h.posted.length, 2); + h.setNow(NOW + DAY + 6 * HOUR); await h.run('schedule'); assert.equal(h.posted.length, 3); + assert.equal(requests(h.comments)[0].retry, undefined); + h.setNow(NOW + DAY + 12 * HOUR); await h.run('schedule'); assert.equal(h.posted.length, 4); + assert.equal(requests(h.comments)[0].retry, true); + h.setNow(NOW + 3 * DAY); await h.run('schedule'); assert.equal(h.posted.length, 4); +}); + +test('30 new target commits allow a new version after an unavailable reply', async () => { + const h = harness(); await h.run(); h.reply('INCONCLUSIVE'); + h.setTarget(NEW_BASE); h.setNow(NOW + 2 * HOUR); h.setProgress(29); + await h.run('schedule'); assert.equal(h.posted.length, 1); + h.setProgress(30); await h.run('schedule'); assert.equal(h.posted.length, 2); +}); + +test('a valid reply arriving before the retry scan prevents a second request', async () => { + const h = harness(); await h.run(); h.setNow(NOW + 6 * HOUR); h.reply('FAIL'); + await h.run('schedule'); assert.equal(h.posted.length, 1); + assert.equal(h.checks[0].conclusion, 'failure'); +}); + +test('post-merge audits allow one retry without changing their fixed merged revisions', async () => { + const h = harness({merged: true, state: 'closed', merged_at: new Date(NOW).toISOString(), + updated_at: new Date(NOW).toISOString()}); + await h.run('pull_request_target', 'closed'); h.reply('INCONCLUSIVE'); + h.setTarget(NEW_BASE); h.setNow(NOW + 6 * HOUR); await h.run('schedule'); + assert.equal(h.posted.length, 2); + const pair = requests(h.comments)[0]; + assert.equal(pair.merged, MERGED); assert.equal(pair.target, BASE); assert.equal(pair.retry, true); + h.setNow(NOW + 12 * HOUR); await h.run('schedule'); assert.equal(h.posted.length, 2); +}); + test('commands require the pinned User account and never fall back to the Actions bot', async () => { for (const user of [ {...COMMAND_USER, id: 1}, {...COMMAND_USER, login: 'another-user'}, @@ -683,10 +721,17 @@ test('scheduled scans cap new AI requests at 20 and later scans consider the rem for (let n = 13; n < 57; n++) h.prs.push({...h.pr, number: n}); await h.run('schedule'); assert.equal(h.posted.length, 20); assert.match(h.warnings.at(-1), /20 new AI requests/); - h.setNow(NOW + 6 * HOUR); await h.run('schedule'); - assert.equal(h.posted.length, 40); - assert.equal(new Set(h.posted.map(p => p.issue_number)).size, 40); - await h.run('workflow_dispatch'); assert.equal(h.posted.length, 41); + for (let round = 1; round <= 6; round++) { + const before = h.posted.length; + h.setNow(NOW + round * 6 * HOUR); await h.run('schedule'); + assert.ok(h.posted.length - before <= 20); + } + assert.equal(new Set(h.posted.map(p => p.issue_number)).size, 45); + for (const number of h.prs.map(p => p.number)) { + assert.ok(h.posted.filter(p => p.issue_number === number).length <= 2); + } + const beforeManual = h.posted.length; + await h.run('workflow_dispatch'); assert.equal(h.posted.length, beforeManual + 1); }); test('scheduled scans prioritize recent merge recovery over open PR requests', async () => { diff --git a/.github/scripts/coderabbit_semantic_review_request.js b/.github/scripts/coderabbit_semantic_review_request.js index 275486f1239f..5adabf98aa30 100644 --- a/.github/scripts/coderabbit_semantic_review_request.js +++ b/.github/scripts/coderabbit_semantic_review_request.js @@ -80,49 +80,50 @@ async function requestOne({github, commandGithub, context, core, number, manual, external_id: currentId, status: 'completed', conclusion: 'neutral', output})); } } + let retry = false; if (reusable && !manual) { const result = resultFor(reusable); // Reuse an in-flight exact pair too. The publisher can complete the audit // when its pre-merge reply arrives, provided the final tree also matches. if (!pair.merged || reusable.merged || !result || result.verdict !== 'INCONCLUSIVE') { - await ensureCheck('This version already has an analysis request.'); - if (result) await publish({github, core, context: {...context, eventName: 'issue_comment', - payload: {issue: {number}}}}); - core.info(`PR #${number}: reusing the exact revision pair.`); - return; + retry = context.eventName === 'schedule' && !['PASS', 'FAIL'].includes(result?.verdict) && + now - Date.parse(reusable.created_at) >= 6 * HOUR && + !history.some(r => r.retry && matches(r, pair)); + if (!retry) { + await ensureCheck('This version already has an analysis request.'); + if (result) await publish({github, core, context: {...context, eventName: 'issue_comment', + payload: {issue: {number}}}}); + core.info(`PR #${number}: reusing the exact revision pair.`); + return; + } } } const intent = context.eventName === 'pull_request_target' && (context.payload.action === 'auto_merge_enabled' && pr.auto_merge || context.payload.action === 'labeled' && context.payload.label?.name === APPROVED && pr.labels.some(l => l.name === APPROVED) && process.env.SEMANTIC_APPROVAL_VALIDATED === 'true'); - let reason = manual ? 'Manual retry' : pair.merged ? 'Post-merge audit' : intent ? 'Merge intent' : ''; + let reason = manual ? 'Manual retry' : retry ? 'Automatic retry after unavailable analysis' : + pair.merged ? 'Post-merge audit' : intent ? 'Merge intent' : ''; const last = history.find(r => !r.merged); if (!reason) { - const previous = last && resultFor(last); - if (last && !['PASS', 'FAIL'].includes(previous?.verdict)) { - await ensureCheck('Previous analysis has no verified verdict; use a manual retry. Routine requests are paused.'); - return; - } - if (previous && context.eventName === 'schedule' && - (last.head !== pair.head || last.target !== pair.target)) { - reason = 'Scheduled refresh of changed revisions'; - } else { - let count = pair.comparison.behind_by; - let since = Date.parse(pair.comparison.merge_base_commit.commit.committer.date); - if (previous) { - const progress = await compare(github, repo, last.target, pair.target); - if (['ahead', 'identical'].includes(progress.status)) { - count = progress.ahead_by; - since = Date.parse(previous.comment.created_at); - } - } - if (!count || (count < 30 && now - since < DAY)) { - await ensureCheck('Below the 24-hour / 30-commit threshold; waiting for target changes.'); - return; + let count = pair.comparison.behind_by; + let since = Date.parse(pair.comparison.merge_base_commit.commit.committer.date); + let changed = count > 0; + if (last) { + const previous = resultFor(last); + const progress = await compare(github, repo, last.target, pair.target); + if (['ahead', 'identical'].includes(progress.status)) { + count = progress.ahead_by; + since = Date.parse(['PASS', 'FAIL'].includes(previous?.verdict) ? + previous.comment.created_at : last.created_at); + changed = last.head !== pair.head || last.target !== pair.target; } - reason = '24-hour / 30-commit threshold'; } + if (!changed || (count < 30 && now - since < DAY)) { + await ensureCheck('Below the 24-hour / 30-commit threshold; waiting for changed revisions to qualify.'); + return; + } + reason = '24-hour / 30-commit threshold'; } // Approval and auto-merge share this budget even when the branch SHAs change. // Count any recent pre-merge request, so a routine scan cannot double the cost. @@ -152,7 +153,7 @@ async function requestOne({github, commandGithub, context, core, number, manual, if (!pair.merged) pair.tree = await candidateTree(github, repo, current, pair.target); const {comparison, ...record} = pair; const {data: comment} = await commandGithub.rest.issues.createComment({...repo, issue_number: number, - body: `${command(pair)}\n\n\n\n` + + body: `${command(pair)}\n\n\n\n` + `${reason} for PR #${number}. ${NOTICE} No AI verdict is asserted by posting this request.`}); core.info(`PR #${number}: ${reason}; ${comment.html_url}`); await core.summary.addRaw(`PR #${number}: [${reason}](${comment.html_url}). No AI verdict is asserted.\n\n`).write();