diff --git a/.github/scripts/ci/classify-pr-profile.sh b/.github/scripts/ci/classify-pr-profile.sh new file mode 100755 index 00000000000..f9659915c5f --- /dev/null +++ b/.github/scripts/ci/classify-pr-profile.sh @@ -0,0 +1,45 @@ +#!/usr/bin/env bash +# Fetch a PR's changed files and classify its CI profile in one step. +# +# The jq projection below is the input contract of classify-profile.mjs +# (it reads `filename`, `status`, `previous_filename` per JSONL entry). +# Both ci.yml's profile gate and qwen-code-pr-review.yml's docs-only +# downgrade consume the classification through THIS script, so the contract +# lives in exactly one place — a divergence between the two call sites once +# meant the same PR could classify differently in each workflow, silently, +# because both fall back to `full` on their own errors. +# +# Usage: classify-pr-profile.sh +# Prints the profile (docs_only | github_ci_only | full) on stdout. +# Exit codes: 0 classified; 2 file listing failed; 3 classifier failed. +set -euo pipefail + +repo="${1:?usage: classify-pr-profile.sh }" +pr="${2:?usage: classify-pr-profile.sh }" + +# mktemp + trap, not a fixed name: the self-hosted pool is persistent and +# shared, so a predictable path is a leftover-file landmine, and ci.yml's +# gate and the review gate can run concurrently for the same PR — two +# writers interleaving one JSONL would classify one job against the other +# job's file list. +tmp="${RUNNER_TEMP:-${TMPDIR:-/tmp}}" +files="$(mktemp "${tmp}/classify-pr-${pr}-files.XXXXXX")" +trap 'rm -f "$files"' EXIT + +if ! gh api --paginate "repos/${repo}/pulls/${pr}/files" \ + --jq '.[] | {filename, status, previous_filename}' > "${files}"; then + exit 2 +fi + +# The list-files endpoint caps at 3,000 entries. A truncated listing can be +# all docs while an omitted later entry is source, so any mismatch against +# the PR's own changed-file count conservatively classifies as `full`. +declared="$(gh api "repos/${repo}/pulls/${pr}" --jq '.changed_files')" || exit 2 +retrieved="$(wc -l < "${files}")" +if [ "${retrieved}" -ne "${declared}" ]; then + echo "classify-pr-profile: retrieved ${retrieved} file entries but PR declares ${declared}; classifying full." >&2 + echo "full" + exit 0 +fi + +node "$(dirname "$0")/classify-profile.mjs" "${files}" || exit 3 diff --git a/.github/scripts/ci/classify-pr-profile.test.mjs b/.github/scripts/ci/classify-pr-profile.test.mjs new file mode 100644 index 00000000000..a154972bcff --- /dev/null +++ b/.github/scripts/ci/classify-pr-profile.test.mjs @@ -0,0 +1,128 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +// Executes the real classify-pr-profile.sh with a stubbed `gh` (and, for the +// classifier-failure case, a stubbed `node`) on PATH. The wrapper's own +// comment declares its jq projection the single home of the classifier's +// input contract — these tests are what make that claim enforceable: dropping +// `status` from the projection turns a renamed source→docs file into a plain +// docs path (classifyFileEntry consults `previous_filename` only when +// `status === "renamed"`), which downgraded a source PR in the probe that +// motivated this file. The exit-code contract (2 listing / 3 classifier) is +// consumed by both ci.yml and the review workflow's docs-only gate. + +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { + chmodSync, + mkdirSync, + mkdtempSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import { execFileSync } from 'node:child_process'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const here = dirname(fileURLToPath(import.meta.url)); +const wrapper = join(here, 'classify-pr-profile.sh'); + +function run(scenario, { stubNodeFailure = false } = {}) { + const dir = mkdtempSync(join(tmpdir(), 'classify-pr-profile-')); + const bin = join(dir, 'bin'); + mkdirSync(bin); + const write = (name, body) => { + const p = join(bin, name); + writeFileSync(p, body); + chmodSync(p, 0o755); + }; + // The gh stub serves the two calls the wrapper makes: the paginated file + // listing and the PR object's changed_files count. For the listing it + // applies the wrapper's OWN `--jq` argument to a full API-shaped fixture + // with real jq — so the projection (the input contract this wrapper exists + // to be the single home of) is genuinely under test: dropping `status` or + // `previous_filename` from it changes what the classifier sees and turns + // the renamed-source scenario red, instead of the stub hardcoding the + // projected output and keeping every projection mutant green. + write( + 'gh', + [ + '#!/bin/bash', + 'jqfilter=""; prev=""', + 'for a in "$@"; do if [ "$prev" = "--jq" ]; then jqfilter="$a"; fi; prev="$a"; done', + 'case "$*" in', + ' *"/files"*)', + ' case "$SCENARIO" in', + ' list-fail) exit 1 ;;', + ' docs-only) FIXTURE=\'[{"filename":"docs/users/a.md","status":"modified","previous_filename":null,"sha":"x","additions":1},{"filename":"README.md","status":"modified","previous_filename":null,"sha":"y","additions":1}]\' ;;', + ' renamed-source) FIXTURE=\'[{"filename":"docs/new.md","status":"renamed","previous_filename":"packages/core/src/runtime.ts","sha":"z","additions":0}]\' ;;', + ' truncated) FIXTURE=\'[{"filename":"docs/users/a.md","status":"modified","previous_filename":null,"sha":"x","additions":1}]\' ;;', + ' declared-fails) FIXTURE=\'[{"filename":"docs/users/a.md","status":"modified","previous_filename":null,"sha":"x","additions":1}]\' ;;', + ' *) exit 9 ;;', + ' esac', + ' printf \'%s\' "$FIXTURE" | jq -c "$jqfilter" ;;', + ' *"repos/"*)', + ' case "$SCENARIO" in', + ' truncated) echo 5 ;;', + ' declared-fails) exit 1 ;;', + ' docs-only) echo 2 ;;', + ' renamed-source) echo 1 ;;', + ' *) exit 9 ;;', + ' esac ;;', + ' *) exit 9 ;;', + 'esac', + 'exit 0', + ].join('\n') + '\n', + ); + if (stubNodeFailure) { + write('node', '#!/bin/bash\nexit 1\n'); + } + try { + const stdout = execFileSync('bash', [wrapper, 'o/r', '42'], { + encoding: 'utf8', + env: { + ...process.env, + PATH: `${bin}:${process.env.PATH}`, + SCENARIO: scenario, + RUNNER_TEMP: dir, + }, + }); + return { code: 0, stdout: stdout.trim() }; + } catch (e) { + return { code: e.status, stdout: `${e.stdout ?? ''}`.trim() }; + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +test('classifies a docs-only listing as docs_only', () => { + assert.deepEqual(run('docs-only'), { code: 0, stdout: 'docs_only' }); +}); + +test('a renamed source→docs file classifies full (the projection carries status/previous_filename)', () => { + assert.deepEqual(run('renamed-source'), { code: 0, stdout: 'full' }); +}); + +test('a listing shorter than the PR-declared changed_files classifies full (3,000-file cap)', () => { + assert.deepEqual(run('truncated'), { code: 0, stdout: 'full' }); +}); + +test('exit 2 when the file listing fails', () => { + assert.equal(run('list-fail').code, 2); +}); + +test('exit 3 when the classifier fails', () => { + assert.equal(run('docs-only', { stubNodeFailure: true }).code, 3); +}); + +test('exit 2 when the changed_files fetch fails after a successful listing', () => { + // The truncation guard's precondition: a swallowed failure here leaves + // `declared` empty and the guard silently skipped (probed mutant + // `|| exit 2` → `|| true` classified a docs first page as docs_only). + const r = run('declared-fails'); + assert.equal(r.code, 2); +}); diff --git a/.github/scripts/ci/classify-profile.mjs b/.github/scripts/ci/classify-profile.mjs index 4188e9d10cc..12890755d21 100644 --- a/.github/scripts/ci/classify-profile.mjs +++ b/.github/scripts/ci/classify-profile.mjs @@ -16,8 +16,15 @@ export const GITHUB_CI_ONLY_FILES = new Set([ function isDocsOnlyFile(file) { const normalized = file.replace(/\\/g, '/'); return ( - /^docs\/.+\.(?:md|mdx)$/i.test(normalized) || - /^(?:README|CHANGELOG|CONTRIBUTING|CODE_OF_CONDUCT|SECURITY|SUPPORT|LICENSE|NOTICE)(?:\.[^/]*)?$/i.test( + // .md ONLY: MDX pages are executable (imported components, expressions) + // and keep the full profile with its runtime/build failure surface. + /^docs\/.+\.md$/i.test(normalized) || + // Extensionless or known-inert documentation extensions ONLY: the open + // `(?:\.[^/]*)?` form classified executable files named after reserved + // prose basenames (README.js, SECURITY.ts, LICENSE.sh) as docs, which + // would downgrade an automatic review over runnable code. MDX is + // excluded here for the same reason it is above. + /^(?:README|CHANGELOG|CONTRIBUTING|CODE_OF_CONDUCT|SECURITY|SUPPORT|LICENSE|NOTICE)(?:\.(?:md|txt|rst))?$/i.test( normalized, ) ); diff --git a/.github/scripts/ci/classify-profile.test.mjs b/.github/scripts/ci/classify-profile.test.mjs index 99e6721d9d4..2260919ba28 100644 --- a/.github/scripts/ci/classify-profile.test.mjs +++ b/.github/scripts/ci/classify-profile.test.mjs @@ -19,11 +19,20 @@ test('uses docs_only for markdown-only changes', () => { test('uses docs_only for uppercase and extensionless docs', () => { assert.equal( - classifyChangedFiles(['README.MD', 'docs/guide.MDX', 'LICENSE', 'README']), + classifyChangedFiles(['README.MD', 'docs/guide.MD', 'LICENSE', 'README']), 'docs_only', ); }); +test('MDX is executable content, never docs_only', () => { + // MDX pages can import components and carry expressions — a runtime/build + // failure surface the docs-only downgrade must not skip over. + assert.equal(classifyChangedFiles(['docs/guide.mdx']), 'full'); + assert.equal(classifyChangedFiles(['docs/guide.MDX']), 'full'); + assert.equal(classifyChangedFiles(['README.mdx']), 'full'); + assert.equal(classifyChangedFiles(['docs/usage.md', 'docs/guide.mdx']), 'full'); +}); + test('falls back to full for root docs names used as directories', () => { assert.equal(classifyChangedFiles(['README.md/evil.ts']), 'full'); assert.equal(classifyChangedFiles(['LICENSE.txt/src/index.ts']), 'full'); @@ -114,3 +123,14 @@ test('falls back to full for runtime markdown assets and instruction files', () ); assert.equal(classifyChangedFiles(['AGENTS.md']), 'full'); }); + +test('reserved prose basenames classify docs_only only with inert extensions', () => { + assert.equal(classifyChangedFiles(['README.md']), 'docs_only'); + assert.equal(classifyChangedFiles(['LICENSE']), 'docs_only'); + assert.equal(classifyChangedFiles(['SECURITY.txt']), 'docs_only'); + // Executable files named after reserved basenames must never downgrade a + // review: the open-extension form classified all of these as docs. + assert.equal(classifyChangedFiles(['README.js']), 'full'); + assert.equal(classifyChangedFiles(['SECURITY.ts']), 'full'); + assert.equal(classifyChangedFiles(['LICENSE.sh']), 'full'); +}); diff --git a/.github/scripts/upsert-bot-comment.sh b/.github/scripts/upsert-bot-comment.sh new file mode 100755 index 00000000000..965eda4e962 --- /dev/null +++ b/.github/scripts/upsert-bot-comment.sh @@ -0,0 +1,66 @@ +#!/usr/bin/env bash +# Upsert a marker-identified bot comment on a PR/issue. +# +# One implementation of the marker+author upsert protocol, shared by the +# docs-only relay and the stale-badge supersede step in +# qwen-code-pr-review.yml (the previous per-step copies had already drifted: +# one had retry/null-guards/dynamic login, the other none). The lookup is +# author-scoped — only comments by the authenticated login are upsert +# targets, so a participant posting the marker can never capture the upsert. +# +# A FAILED lookup is never treated as an EMPTY result: posting on a failed +# listing is how a transient 5xx mints a permanent duplicate (later runs +# PATCH only the `last` match), so every prerequisite — the authenticated +# login and the listing — is re-resolved inside the retry loop, and an +# attempt whose prerequisites failed retries instead of falling through to +# POST. On --update-only, a failed lookup exits 1 (the caller's warning +# path), never the no-op success reserved for a lookup that genuinely +# found nothing. +# +# Usage: upsert-bot-comment.sh [--update-only] +# --update-only: PATCH an existing bot-authored marker comment if present; +# succeed as a no-op when none exists (never POSTs). For +# superseding a badge without minting one where none was. +# Exit codes: 0 posted/updated/no-op; 1 all attempts failed. +set -euo pipefail + +repo="${1:?usage: upsert-bot-comment.sh [--update-only]}" +number="${2:?missing issue number}" +marker="${3:?missing marker}" +body_file="${4:?missing body file}" +update_only="${5:-}" + +body="$(cat "${body_file}")" + +for _attempt in 1 2 3; do + if bot_login="$(gh api user --jq '.login')" \ + && [ -n "${bot_login}" ] \ + && listing="$(gh api "repos/${repo}/issues/${number}/comments" \ + --method GET \ + --paginate \ + -F per_page=100)" \ + && existing_id="$(printf '%s' "${listing}" \ + | jq -sr --arg bot "${bot_login}" --arg marker "${marker}" '[.[][] + | select((.user.login // "") == $bot) + | select((.body // "") | contains($marker))] + | last | .id // empty')"; then + if [ -n "${existing_id}" ]; then + if gh api --method PATCH \ + "repos/${repo}/issues/comments/${existing_id}" \ + -f body="${body}" >/dev/null; then + echo "updated comment ${existing_id}" + exit 0 + fi + elif [ "${update_only}" = "--update-only" ]; then + echo "no existing comment; nothing to update" + exit 0 + elif gh api "repos/${repo}/issues/${number}/comments" \ + -f body="${body}" >/dev/null; then + echo "posted new comment" + exit 0 + fi + fi + sleep 10 +done +echo "all attempts failed" >&2 +exit 1 diff --git a/.github/scripts/upsert-bot-comment.test.mjs b/.github/scripts/upsert-bot-comment.test.mjs new file mode 100644 index 00000000000..6ecd2fd7649 --- /dev/null +++ b/.github/scripts/upsert-bot-comment.test.mjs @@ -0,0 +1,187 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +// Executes the real upsert-bot-comment.sh with a stubbed `gh` + `sleep` on +// PATH. The scenarios pin the protocol's load-bearing properties, each of +// which survived as a green mutant when it lived untested inside a workflow +// step: the author scope (a user-posted marker must not capture the upsert), +// the per-attempt re-resolution (a comment deleted mid-retry falls back to +// POST instead of PATCHing a stale id), and the --update-only no-op (a +// supersede must never mint a badge where none existed). + +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { + chmodSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import { execFileSync } from 'node:child_process'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const here = dirname(fileURLToPath(import.meta.url)); +const script = join(here, 'upsert-bot-comment.sh'); +const MARKER = ''; + +function run(scenario, { updateOnly = false } = {}) { + const dir = mkdtempSync(join(tmpdir(), 'upsert-bot-comment-')); + const bin = join(dir, 'bin'); + mkdirSync(bin); + const calls = join(dir, 'calls'); + writeFileSync(calls, ''); + const bodyFile = join(dir, 'body'); + writeFileSync(bodyFile, `${MARKER}\nhello`); + const write = (name, body) => { + writeFileSync(join(bin, name), body); + chmodSync(join(bin, name), 0o755); + }; + write('sleep', '#!/bin/bash\nexit 0\n'); + write( + 'gh', + [ + '#!/bin/bash', + 'echo "$*" >> "$CALLS"', + 'n=$(grep -c "method GET" "$CALLS" || true)', + 'case "$*" in', + ' "api user"*)', + ' if [ "$SCENARIO" = "user-fails" ]; then exit 1; fi', + ' echo bot ;;', + ' *"--method GET"*)', + ' case "$SCENARIO" in', + ' listing-always-fails) exit 1 ;;', + ' listing-fails-once)', + ' if [ "$n" -le 1 ]; then exit 1; else echo \'[{"id":7,"user":{"login":"bot"},"body":" old"}]\'; fi ;;', + ' fresh) echo "[]" ;;', + ' existing-bot) echo \'[{"id":7,"user":{"login":"bot"},"body":" old"}]\' ;;', + ' duplicate-pair) echo \'[{"id":5,"user":{"login":"bot"},"body":" older"},{"id":9,"user":{"login":"bot"},"body":" newer"}]\' ;;', + ' existing-user) echo \'[{"id":8,"user":{"login":"alice"},"body":" mine"}]\' ;;', + ' deleted-mid-retry)', + ' if [ "$n" -le 1 ]; then echo \'[{"id":9,"user":{"login":"bot"},"body":" old"}]\'; else echo "[]"; fi ;;', + ' esac ;;', + ' *"--method PATCH"*)', + ' if [ "$SCENARIO" = "deleted-mid-retry" ]; then exit 1; fi ;;', + ' *) : ;;', + 'esac', + 'exit 0', + ].join('\n') + '\n', + ); + let code = 0; + let stdout = ''; + try { + stdout = execFileSync( + 'bash', + [ + script, + 'o/r', + '42', + MARKER, + bodyFile, + ...(updateOnly ? ['--update-only'] : []), + ], + { + encoding: 'utf8', + env: { + ...process.env, + PATH: `${bin}:${process.env.PATH}`, + SCENARIO: scenario, + CALLS: calls, + }, + }, + ); + } catch (e) { + code = e.status; + stdout = `${e.stdout ?? ''}`; + } + const recorded = readFileSync(calls, 'utf8'); + rmSync(dir, { recursive: true, force: true }); + return { code, stdout, calls: recorded }; +} + +test('POSTs a fresh comment when no bot-authored marker exists', () => { + const r = run('fresh'); + assert.equal(r.code, 0); + assert.match(r.stdout, /posted new comment/); + assert.doesNotMatch(r.calls, /--method PATCH/); +}); + +test('PATCHes the existing bot-authored marker comment', () => { + const r = run('existing-bot'); + assert.equal(r.code, 0); + assert.match(r.stdout, /updated comment 7/); + assert.match(r.calls, /--method PATCH repos\/o\/r\/issues\/comments\/7/); +}); + +test('a user-authored marker comment never captures the upsert', () => { + const r = run('existing-user'); + assert.equal(r.code, 0); + assert.match(r.stdout, /posted new comment/); + assert.doesNotMatch(r.calls, /--method PATCH/); +}); + +test('falls back to POST when the target vanishes mid-retry (re-resolution)', () => { + const r = run('deleted-mid-retry'); + assert.equal(r.code, 0); + assert.match(r.stdout, /posted new comment/); + // Attempt 1 PATCHed the stale id and failed; attempt 2 re-resolved to + // empty and POSTed — a hoisted lookup would PATCH id 9 three times. + assert.match(r.calls, /--method PATCH repos\/o\/r\/issues\/comments\/9/); +}); + +test('--update-only is a no-op success when nothing exists', () => { + const r = run('fresh', { updateOnly: true }); + assert.equal(r.code, 0); + assert.match(r.stdout, /nothing to update/); + assert.doesNotMatch(r.calls, /--method PATCH/); + // And no POST either: the only api writes would be comment creation. + assert.doesNotMatch(r.calls, /issues\/42\/comments -f/); +}); + +test('a failed listing NEVER falls through to POST (retries, then PATCHes)', () => { + // The critical shape: the badge already exists, the first listing GET hits + // a transient failure. Conflating that failure with "no match" would POST + // a permanent duplicate; the fix retries and PATCHes the real comment. + const r = run('listing-fails-once'); + assert.equal(r.code, 0); + assert.match(r.stdout, /updated comment 7/); + assert.doesNotMatch(r.calls, /issues\/42\/comments -f/); +}); + +test('a persistently failing identity lookup exits 1 without writing', () => { + const r = run('user-fails'); + assert.equal(r.code, 1); + assert.doesNotMatch(r.calls, /--method PATCH/); + assert.doesNotMatch(r.calls, /issues\/42\/comments -f/); +}); + +test('--update-only with a failing lookup exits 1, not the no-op success', () => { + // The supersede caller's ::warning:: path depends on this: a failed lookup + // must not masquerade as "nothing to update". + const r = run('listing-always-fails', { updateOnly: true }); + assert.equal(r.code, 1); +}); + +test('--update-only PATCHes an existing bot-authored badge', () => { + const r = run('existing-bot', { updateOnly: true }); + assert.equal(r.code, 0); + assert.match(r.stdout, /updated comment 7/); + assert.match(r.calls, /--method PATCH repos\/o\/r\/issues\/comments\/7/); +}); + +test('with duplicate badges, the upsert refreshes the LAST (newest) one', () => { + // The header's documented duplicate-resolution semantics: after a + // transient failure once minted a pair, every subsequent upsert must + // target the newest — a flip to `first` would refresh the older comment + // while the newer stale one stays the visible latest. + const r = run('duplicate-pair'); + assert.equal(r.code, 0); + assert.match(r.stdout, /updated comment 9/); + assert.doesNotMatch(r.calls, /comments\/5/); +}); diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1d64329d472..4ce19b8b22b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,7 +50,7 @@ env: # BOTH the github_ci_only helper step and the full-profile Test step, so a # new helper test can't be added to one path and silently dropped from the # other. - HELPER_TESTS: '.github/scripts/pr-safety-precheck.test.mjs .github/scripts/cap-release-notes.test.mjs .github/scripts/ci/classify-profile.test.mjs .github/scripts/ci/main-failure-signature.test.mjs .github/scripts/classify-release-notes.test.mjs .github/scripts/dsw-swe-verified/make-manifest.test.mjs .github/scripts/resolve-sandbox-image.test.mjs .github/scripts/web-shell-visuals-publish.test.mjs .github/scripts/web-shell-visuals-compose.test.mjs .github/scripts/serve-ab-diff.test.mjs .github/scripts/qwen-triage-workflow.test.mjs .github/scripts/auto-minimize-spam.test.mjs .github/scripts/ci-runner-routing.test.mjs' + HELPER_TESTS: '.github/scripts/pr-safety-precheck.test.mjs .github/scripts/cap-release-notes.test.mjs .github/scripts/ci/classify-profile.test.mjs .github/scripts/ci/classify-pr-profile.test.mjs .github/scripts/upsert-bot-comment.test.mjs .github/scripts/ci/main-failure-signature.test.mjs .github/scripts/classify-release-notes.test.mjs .github/scripts/dsw-swe-verified/make-manifest.test.mjs .github/scripts/resolve-sandbox-image.test.mjs .github/scripts/web-shell-visuals-publish.test.mjs .github/scripts/web-shell-visuals-compose.test.mjs .github/scripts/serve-ab-diff.test.mjs .github/scripts/qwen-triage-workflow.test.mjs .github/scripts/auto-minimize-spam.test.mjs .github/scripts/ci-runner-routing.test.mjs' jobs: classify_pr: @@ -267,14 +267,20 @@ jobs: profile=full if [[ "${GITHUB_EVENT_NAME}" == "pull_request" && -n "${PR_NUMBER}" ]]; then if [[ "${IS_SAME_REPO_PR}" == "true" ]]; then - changed_files="${RUNNER_TEMP}/changed-files.jsonl" - if gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/files" --jq '.[] | {filename, status, previous_filename}' > "${changed_files}"; then - if ! profile="$(node .github/scripts/ci/classify-profile.mjs "${changed_files}")"; then - echo "::error::CI profile classifier exited non-zero; running full CI." - profile=full - fi - else + # Fetch + classify through the shared wrapper (also used by the + # review workflow's docs-only gate) so the classifier's input + # contract lives in one place. Exit 2 = listing failed, + # 3 = classifier failed. + set +e + profile="$(.github/scripts/ci/classify-pr-profile.sh "${GITHUB_REPOSITORY}" "${PR_NUMBER}")" + classify_rc=$? + set -e + if [ "$classify_rc" -eq 2 ]; then echo "::warning::Unable to list PR changed files; running full CI." + profile=full + elif [ "$classify_rc" -ne 0 ]; then + echo "::error::CI profile classifier exited non-zero; running full CI." + profile=full fi else echo "Fork PR detected; running full CI." diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 27f1e7df173..e1120b2ba7f 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -2992,6 +2992,7 @@ jobs: | select((.user.login // "") != $ab) | select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb) | select((.body // "") | test($bf) | not) + | select(((((.user.login // "") == $rb)) and ((.body // "") | test("' + BODY="$(printf '%s\n' \ + "$MARKER" \ + '' \ + "📄 **Docs-only change** — the automatic review ran at \`--effort medium\` (verified findings, no reverse audit; medium posts no inline comments). Outcome:" \ + '' \ + "> ${COMPLETION_LINE}" \ + '' \ + "Reviewed head: \`${EXPECTED_HEAD_SHA}\`. Full report in the [workflow run](${RUN_URL}). For a full high-effort review with inline comments, comment \`@qwen-code /review\`." \ + '' \ + '
中文说明' \ + '' \ + "📄 **纯文档变更** —— 自动评审以 \`--effort medium\` 运行(发现已验证、无反向审计;medium 不发布行内评论),结果见上方引用行。评审的 head:\`${EXPECTED_HEAD_SHA}\`。完整报告见 [workflow 运行](${RUN_URL});如需带行内评论的完整高强度(high-effort)评审,请评论 \`@qwen-code /review\`。" \ + '' \ + '
')" + # The shared upsert protocol (.github/scripts/upsert-bot-comment.sh) + # carries the load-bearing properties: author-scoped lookup (a PR + # participant posting the marker can never capture the upsert), + # per-attempt re-resolution, and bounded retry. Never fail the job + # over the relay: the review itself succeeded, and a failing step + # here would trip the post-failure fallback into announcing a + # review failure that never happened — losing the relay costs a + # comment, and the outcome line below keeps it recoverable from + # the job log. + printf '%s' "$BODY" > "${RUNNER_TEMP}/docs-only-relay-body.md" + if .github/scripts/upsert-bot-comment.sh \ + "${GITHUB_REPOSITORY}" "${PR_NUMBER}" \ + "$MARKER" \ + "${RUNNER_TEMP}/docs-only-relay-body.md"; then + echo "docs-only medium outcome relayed to PR #${PR_NUMBER}." + else + echo "::warning::Docs-only relay comment could not be posted after 3 attempts; the review itself succeeded. Outcome: ${COMPLETION_LINE}" + fi + + - name: 'Supersede stale docs-only badge' + # Three paths owe the badge correction, and only these three: + # (1) an automatic run whose classification POSITIVELY determined the + # PR is not (or no longer) docs-only — docs_only_medium == 'false' + # is three-valued and empty when the classifier failed or never + # ran, so a badge is never retired on ignorance; the review's own + # success is deliberately not required (a failed full review still + # leaves the badge misdescribing the head); + # (2) an explicit comment-mode review that actually completed — the + # badge's own CTA path, whose posted full review makes the badge + # redundant; a dispatch dry-run that posts nothing retires + # nothing; + # (3) a FAILED automatic docs-only review — the relay only runs on + # success, so without this path the badge would keep quoting the + # previous revision's outcome for a head whose own run died. + # The retired body is cause-neutral: it asserts only what is true on + # every covered path (an explicit review can complete on the very + # same SHA the badge describes). + if: |- + !cancelled() && + steps.context.outputs.should_run == 'true' && + steps.context.outputs.pr_number != '' && + ( + steps.review.outputs.docs_only_medium == 'false' || + ( + steps.context.outputs.auto_review == 'false' && + steps.context.outputs.review_mode == 'comment' && + steps.review.outputs.review_completed == 'true' + ) || + ( + failure() && + steps.review.outputs.docs_only_medium == 'true' + ) + ) + env: + GH_TOKEN: '${{ secrets.CI_BOT_PAT }}' + PR_NUMBER: '${{ steps.context.outputs.pr_number }}' + EXPECTED_HEAD_SHA: "${{ steps.review.outputs.expected_head_sha || '' }}" + DOCS_ONLY_MEDIUM: '${{ steps.review.outputs.docs_only_medium }}' + REVIEW_COMPLETED: '${{ steps.review.outputs.review_completed }}' + RUN_URL: '${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}' + run: |- + set -euo pipefail + # The write is bound to the reviewed head: re-read the live PR + # state immediately before the mutation (the same guard shape as + # the fallback-comment step). A run that failed before "Run + # review" emitted the reviewed SHA has nothing to bind to — a + # badge is never updated on ignorance. + if [ -z "$EXPECTED_HEAD_SHA" ]; then + echo "Skipping badge update: the reviewed head SHA is unknown." + exit 0 + fi + if ! PR_DATA="$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --json state,headRefOid --jq '[.state, .headRefOid] | @tsv')"; then + echo "::warning::Could not verify PR #${PR_NUMBER} before updating the docs-only badge." + exit 0 + fi + IFS=$'\t' read -r PR_STATE CURRENT_HEAD_SHA <<< "$PR_DATA" + if [ "$PR_STATE" != "OPEN" ]; then + echo "Skipping badge update: PR #${PR_NUMBER} is ${PR_STATE}." + exit 0 + fi + if [ "$CURRENT_HEAD_SHA" != "$EXPECTED_HEAD_SHA" ]; then + echo "Skipping badge update: PR #${PR_NUMBER} moved from ${EXPECTED_HEAD_SHA} to ${CURRENT_HEAD_SHA}." + exit 0 + fi + # --update-only makes this a strict no-op on PRs that never + # carried the badge; a failed lookup exits 1 (never the no-op), so + # the warning below fires instead of silently keeping a stale + # badge. The marker is defined once and used for both the body and + # the lookup — they must be byte-identical. + MARKER='' + if [ "$DOCS_ONLY_MEDIUM" = "true" ] && [ "$REVIEW_COMPLETED" != "true" ]; then + # A failed docs-only run: the singleton badge must not keep + # quoting the previous revision's success for this head. + BODY="$(printf '%s\n' \ + "$MARKER" \ + '' \ + "📄 **Docs-only change** — the automatic \`--effort medium\` review of head \`${EXPECTED_HEAD_SHA}\` **did not complete**, so no outcome currently applies. See the failure comment on this PR and the [workflow run](${RUN_URL})." \ + '' \ + '
中文说明' \ + '' \ + "📄 **纯文档变更** —— head \`${EXPECTED_HEAD_SHA}\` 的自动 \`--effort medium\` 评审**未能完成**,当前没有有效的评审结果。详见本 PR 上的失败评论与 [workflow 运行](${RUN_URL})。" \ + '' \ + '
')" + else + BODY="$(printf '%s\n' \ + "$MARKER" \ + '' \ + '📄 ~~Docs-only change~~ **(superseded)** — this badge no longer reflects the current review state of this PR. See the latest review activity on this PR.' \ + '' \ + '
中文说明' \ + '' \ + '📄 ~~纯文档变更~~ **(已失效)** —— 该徽章已不再反映本 PR 当前的评审状态。请以本 PR 上最新的评审动态为准。' \ + '' \ + '
')" + fi + printf '%s' "$BODY" > "${RUNNER_TEMP}/docs-only-supersede-body.md" + .github/scripts/upsert-bot-comment.sh \ + "${GITHUB_REPOSITORY}" "${PR_NUMBER}" \ + "$MARKER" \ + "${RUNNER_TEMP}/docs-only-supersede-body.md" \ + --update-only \ + || echo "::warning::Could not supersede the stale docs-only badge." + - name: 'Post fallback comment on failure' if: |- failure() && diff --git a/scripts/tests/qwen-pr-review-workflow.test.js b/scripts/tests/qwen-pr-review-workflow.test.js index df72108f69b..7c5362e03c3 100644 --- a/scripts/tests/qwen-pr-review-workflow.test.js +++ b/scripts/tests/qwen-pr-review-workflow.test.js @@ -1233,6 +1233,760 @@ describe('capture-tools step wiring', () => { }); }); +describe('docs-only medium gate', () => { + // The downgrade logic is inline bash in two steps; these tests extract and + // EXECUTE the load-bearing fragments (prompt branch, timeout floor, the + // completion-line allowlist) rather than asserting on their text, because + // the surviving mutations are behavioral: swapping the if/elif order makes + // parse-args force high effort back on AND post inline comments while the + // relay still claims medium posted nothing; flipping the floor comparison + // caps every size-tiered docs run at 90 minutes. + const run = (() => { + const doc = parse(workflow); + return doc.jobs['review-pr'].steps.find((s) => s.name === 'Run review').run; + })(); + + function promptBranchSource() { + const start = run.indexOf('PROMPT="/review ${REVIEW_URL}"'); + expect(start).toBeGreaterThan(-1); + const end = run.indexOf('\nfi', start) + '\nfi'.length; + return run.slice(start, end); + } + + function buildPrompt({ docsOnlyMedium, reviewMode }) { + const script = [ + 'set -euo pipefail', + 'REVIEW_URL="https://x/pull/1"', + `DOCS_ONLY_MEDIUM=${docsOnlyMedium}`, + `REVIEW_MODE=${reviewMode}`, + promptBranchSource(), + 'printf "%s" "$PROMPT"', + ].join('\n'); + return execFileSync('bash', ['-c', script], { encoding: 'utf8' }); + } + + it('emits --effort medium INSTEAD OF --comment on the docs-only path', () => { + const prompt = buildPrompt({ + docsOnlyMedium: 'true', + reviewMode: 'comment', + }); + expect(prompt).toContain('--effort medium'); + expect(prompt).not.toContain('--comment'); + }); + + it('keeps --comment on the non-docs comment path', () => { + const prompt = buildPrompt({ + docsOnlyMedium: 'false', + reviewMode: 'comment', + }); + expect(prompt).toContain('--comment'); + expect(prompt).not.toContain('--effort'); + }); + + function floorSource() { + const anchor = run.indexOf('# Medium measures at one-third to one-half'); + expect(anchor).toBeGreaterThan(-1); + const start = run.indexOf('EFFECTIVE_TIMEOUT_MINUTES=$((', anchor); + // The YAML parser strips the block scalar's base indentation, so the + // floor's closing `fi` sits at four spaces in the parsed text. + const end = run.indexOf('\n fi', start) + '\n fi'.length; + expect(start).toBeGreaterThan(-1); + expect(end).toBeGreaterThan(start); + return run.slice(start, end); + } + + it.each([ + [360, 180], + [180, 90], + [100, 90], + ])( + 'halves the size-aware budget with a 90-minute floor (%i → %i)', + (input, want) => { + const script = [ + 'set -euo pipefail', + `EFFECTIVE_TIMEOUT_MINUTES=${input}`, + floorSource(), + 'printf "%s" "$EFFECTIVE_TIMEOUT_MINUTES"', + ].join('\n'); + expect(execFileSync('bash', ['-c', script], { encoding: 'utf8' })).toBe( + String(want), + ); + }, + ); + + function completionBlockSource() { + const anchor = run.indexOf('machine-readable completion contract'); + expect(anchor).toBeGreaterThan(-1); + const start = run.lastIndexOf( + 'if [ "$DOCS_ONLY_MEDIUM" = "true" ]; then', + anchor, + ); + // Base indentation is stripped by the YAML parser: the block's outer `fi` + // sits at column 0, its inner allowlist `fi` at two spaces — so `\nfi` + // uniquely anchors the outer close. + const end = run.indexOf('\nfi', anchor) + '\nfi'.length; + expect(start).toBeGreaterThan(-1); + expect(end).toBeGreaterThan(start); + return run.slice(start, end); + } + + function relayLine(resultText) { + const dir = mkdtempSync(join(tmpdir(), 'review-completion-')); + try { + const gho = join(dir, 'gho'); + writeFileSync(gho, ''); + const script = [ + 'set -euo pipefail', + 'DOCS_ONLY_MEDIUM=true', + 'PR_NUMBER=123', + `GITHUB_OUTPUT="${gho}"`, + `RESULT_TEXT=$(cat "${join(dir, 'result')}")`, + completionBlockSource(), + ].join('\n'); + writeFileSync(join(dir, 'result'), resultText); + execFileSync('bash', ['-c', script], { encoding: 'utf8' }); + return readFileSync(gho, 'utf8').trim(); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + } + + it('relays only the not-posted disposition shape', () => { + expect( + relayLine( + 'prose...\nReview complete: pr-123 — Comment, not posted (0 Critical, 2 Suggestion)', + ), + ).toBe( + 'completion_line=Review complete: pr-123 — Comment, not posted (0 Critical, 2 Suggestion)', + ); + }); + + it.each([ + // The measured phantom: a posted-form disposition on a path that never posts. + 'Review complete: pr-123 — APPROVE posted', + 'Review complete: pr-123 — COMMENT posted (0 Critical, 1 Suggestion inline)', + // Reworded/missing completion lines. + 'The review finished fine, trust me.', + '', + ])('falls back to the neutral non-scrapable form for %j', (text) => { + const line = relayLine(text); + expect(line.startsWith('completion_line=(no relayable')).toBe(true); + // The fallback must never mint the reserved machine prefix. + expect(line).not.toContain('completion_line=Review complete:'); + }); + + it('accepts the Request-changes disposition a Critical-finding medium run emits', () => { + // compose-review caps only Approve at medium: a verified Critical still + // yields Request changes, and that is exactly the outcome the relay must + // not swallow into the neutral fallback. + const line = + 'Review complete: pr-123 — Request changes, not posted (1 Critical, 0 Suggestion)'; + expect(relayLine(`prose...\n${line}`)).toBe(`completion_line=${line}`); + }); + + it('relays the LAST completion line, not a stale or injected earlier one', () => { + const stale = + 'Review complete: pr-123 — Comment, not posted (9 Critical, 9 Suggestion)'; + const valid = + 'Review complete: pr-123 — Comment, not posted (0 Critical, 2 Suggestion)'; + expect(relayLine(`${stale}\nmore prose\n${valid}`)).toBe( + `completion_line=${valid}`, + ); + }); + + it('rejects the Approve verdict medium can never produce', () => { + // Widening the alternation to include Approve must turn this red: an + // injection-steered approval must not be republished under the bot's name. + const line = relayLine( + 'Review complete: pr-123 — Approve, not posted (0 Critical, 0 Suggestion)', + ); + expect(line.startsWith('completion_line=(no relayable')).toBe(true); + }); + + it("rejects another PR's completion line (target binding)", () => { + const line = relayLine( + 'Review complete: pr-999 — Comment, not posted (0 Critical, 2 Suggestion)', + ); + expect(line.startsWith('completion_line=(no relayable')).toBe(true); + }); + + it('classifies review_requested as an explicit ask, never automatic', () => { + const doc = parse(workflow); + const context = doc.jobs['review-pr'].steps.find((s) => s.id === 'context'); + // One assignment site, guarded on both the event and the action. + expect(context.run.match(/AUTO_REVIEW=true/g)).toHaveLength(1); + // The false DEFAULT is load-bearing too: without it, dispatch, + // issue-comment and review-comment triggers inherit whatever the + // environment carries and can enter the automatic downgrade path. + expect(context.run.match(/AUTO_REVIEW=false/g)).toHaveLength(1); + expect(context.run.indexOf('AUTO_REVIEW=false')).toBeLessThan( + context.run.indexOf('AUTO_REVIEW=true'), + ); + // Both halves of the guard: the event must be pull_request_target AND the + // action must not be review_requested. Pinning only the action half let a + // deleted event condition survive — the branch is shared with + // pull_request_review(_comment), whose actions are never review_requested, + // so a review-body `@qwen-code /review` would have downgraded silently. + expect(context.run).toMatch( + /= "pull_request_target" \] &&\s*\n\s*\[ "\$\{\{ github\.event\.action \}\}" != "review_requested" \]; then\s*\n\s*AUTO_REVIEW=true/, + ); + }); + + it('pins the relay marker producer↔filter contract, author-scoped', () => { + // Producer side: the marker literal as the relay step actually posts it. + const doc = parse(workflow); + const relay = doc.jobs['review-pr'].steps.find( + (s) => s.name === 'Report docs-only medium outcome', + ); + const m = relay.run.match(//); + expect(m).not.toBeNull(); + // Filter side: every autofix exclusion of that marker must carry the + // author scope ($rb) — a human quoting the marker stays actionable — + // and all six inline copies in qwen-autofix.yml must be present. + const autofix = readFileSync('.github/workflows/qwen-autofix.yml', 'utf8'); + const scoped = + autofix.match( + /\(\.user\.login \/\/ ""\) == \$rb\)\) and \(\(\.body \/\/ ""\) \| test\(" old"}]\'', + ' else echo "[]"; fi ;;', + ' *) : ;;', + 'esac', + 'exit 0', + ].join('\n') + '\n', + ); + const script = [ + 'set -euo pipefail', + 'GITHUB_REPOSITORY=o/r', + 'PR_NUMBER=42', + `RUNNER_TEMP="${dir}"`, + 'EXPECTED_HEAD_SHA=abc123', + 'COMPLETION_LINE="Review complete: pr-42 — Comment, not posted (0 Critical, 1 Suggestion)"', + 'RUN_URL=https://x', + relayRun, + ].join('\n'); + const stdout = execFileSync('bash', ['-c', script], { + encoding: 'utf8', + env: { + ...process.env, + PATH: `${bin}:${process.env.PATH}`, + SCENARIO: scenario, + CALLS: calls, + }, + }); + return { stdout, calls: readFileSync(calls, 'utf8') }; + } finally { + rmSync(dir, { recursive: true, force: true }); + } + } + + it('POSTs a fresh relay comment when none exists', () => { + const r = runRelay({ scenario: 'fresh' }); + expect(r.stdout).toContain('relayed to PR #42'); + expect(r.calls).toContain('api repos/o/r/issues/42/comments -f'); + expect(r.calls).not.toContain('--method PATCH'); + // The POSTed body must carry the marker — it is the dedup key both the + // upsert lookup and the supersede step match on; a body without it makes + // every push stack a new badge and supersede match nothing. + expect(r.calls).toContain(''); + // The badge is bound to the reviewed head: a later push must never be + // described by an earlier revision's outcome. + expect(r.calls).toContain('Reviewed head: `abc123`'); + }); + + it('PATCHes the existing bot-authored relay comment', () => { + const r = runRelay({ scenario: 'existing' }); + expect(r.stdout).toContain('relayed to PR #42'); + expect(r.calls).toContain( + 'api --method PATCH repos/o/r/issues/comments/777', + ); + }); + + it('warns and exits 0 when every attempt fails', () => { + const r = runRelay({ scenario: 'all-fail' }); + expect(r.stdout).toContain('::warning::'); + expect(r.stdout).toContain('the review itself succeeded'); + }); + + it('skips the relay when the head moved before the write', () => { + const r = runRelay({ scenario: 'moved-head' }); + expect(r.stdout).toContain('moved from abc123 to deadbeef'); + expect(r.calls).not.toContain('api repos/o/r/issues/42/comments'); + }); + + it('skips the relay when the PR closed before the write', () => { + const r = runRelay({ scenario: 'closed-pr' }); + expect(r.stdout).toContain('is MERGED'); + expect(r.calls).not.toContain('api repos/o/r/issues/42/comments'); + }); + + function normalizedIf(step) { + return step.if.replace(/\s+/g, ' ').trim(); + } + + it('pins the relay if: as the exact reviewed conjunction', () => { + // Full-string pin, not substrings: deleting or weakening any conjunct — + // or re-grouping them — edits this string, so every truth-table mutant + // reduces to a red test here without an Actions-expression evaluator. + const doc2 = parse(workflow); + const relay = doc2.jobs['review-pr'].steps.find( + (s) => s.name === 'Report docs-only medium outcome', + ); + expect(normalizedIf(relay)).toBe( + "steps.context.outputs.should_run == 'true' && " + + "steps.review.outcome == 'success' && " + + "steps.review.outputs.review_completed == 'true' && " + + "steps.review.outputs.docs_only_medium == 'true' && " + + "steps.context.outputs.pr_number != ''", + ); + }); + + it('pins the supersede if: including the OR grouping of its three paths', () => { + const doc2 = parse(workflow); + const supersede = doc2.jobs['review-pr'].steps.find( + (s) => s.name === 'Supersede stale docs-only badge', + ); + expect(normalizedIf(supersede)).toBe( + '!cancelled() && ' + + "steps.context.outputs.should_run == 'true' && " + + "steps.context.outputs.pr_number != '' && " + + "( steps.review.outputs.docs_only_medium == 'false' || " + + "( steps.context.outputs.auto_review == 'false' && " + + "steps.context.outputs.review_mode == 'comment' && " + + "steps.review.outputs.review_completed == 'true' ) || " + + "( failure() && steps.review.outputs.docs_only_medium == 'true' ) )", + ); + }); + + it('pins the review_completed wiring end to end', () => { + // The state/head guards exit 0 without running the review; the relay + // must require the dedicated output, and the run step must emit it + // AFTER those guards — a hoisted emit would open the relay gate for a + // closed/stale PR whose review never ran (position pinned below). The + // supersede step deliberately does NOT require it: a failed full review + // still owes the badge correction, gated on docs_only_medium == 'false' + // (empty on runs that failed before classifying). + const emitAt = runStep.indexOf('echo "review_completed=true"'); + expect(emitAt).toBeGreaterThan(-1); + expect(emitAt).toBeGreaterThan( + runStep.indexOf('if [ "$PR_STATE" != "OPEN" ]'), + ); + expect(emitAt).toBeGreaterThan( + runStep.indexOf('Skipping stale review run'), + ); + const doc2 = parse(workflow); + const relay = doc2.jobs['review-pr'].steps.find( + (s) => s.name === 'Report docs-only medium outcome', + ); + expect(relay.if).toContain( + "steps.review.outputs.review_completed == 'true'", + ); + const supersede = doc2.jobs['review-pr'].steps.find( + (s) => s.name === 'Supersede stale docs-only badge', + ); + // Path (1): a POSITIVE not-docs-only determination (three-valued output; + // empty = never determined) — deliberately without review success. + expect(supersede.if).toContain( + "steps.review.outputs.docs_only_medium == 'false'", + ); + // Path (2): an explicit comment-mode review that completed (the badge's + // CTA); a dispatch dry-run retires nothing. + expect(supersede.if).toContain( + "steps.context.outputs.auto_review == 'false'", + ); + expect(supersede.if).toContain( + "steps.context.outputs.review_mode == 'comment'", + ); + expect(supersede.if).toContain( + "steps.review.outputs.review_completed == 'true'", + ); + expect(supersede.if).toContain('!cancelled()'); + }); + + it('pins the supersede invocation shape (update-only, shared marker)', () => { + const doc2 = parse(workflow); + for (const name of [ + 'Report docs-only medium outcome', + 'Supersede stale docs-only badge', + ]) { + const step = doc2.jobs['review-pr'].steps.find((s) => s.name === name); + // One marker definition serving both the body and the lookup argument. + expect(step.run).toContain( + "MARKER=''", + ); + expect(step.run).toContain('"$MARKER"'); + } + const supersede = doc2.jobs['review-pr'].steps.find( + (s) => s.name === 'Supersede stale docs-only badge', + ); + expect(supersede.run).toContain('--update-only'); + }); + + it('pins the auto_review output→env wiring at both links', () => { + const doc2 = parse(workflow); + const context = doc2.jobs['review-pr'].steps.find( + (s) => s.id === 'context', + ); + expect(context.run).toContain('echo "auto_review=$AUTO_REVIEW"'); + const review = doc2.jobs['review-pr'].steps.find( + (s) => s.name === 'Run review', + ); + expect(review.env.AUTO_REVIEW).toBe( + '${{ steps.context.outputs.auto_review }}', + ); + }); +}); + +describe('supersede step and ci.yml rc-handling, executed', () => { + const doc = parse(workflow); + const supersedeRun = doc.jobs['review-pr'].steps.find( + (s) => s.name === 'Supersede stale docs-only badge', + ).run; + + function runSupersede({ + scenario, + docsOnlyMedium = 'false', + reviewCompleted = 'true', + expectedHeadSha = 'abc123', + }) { + const dir = mkdtempSync(join(tmpdir(), 'docs-supersede-')); + try { + const bin = join(dir, 'bin'); + mkdirSync(bin); + const calls = join(dir, 'calls'); + writeFileSync(calls, ''); + const write = (name, body) => { + writeFileSync(join(bin, name), body); + chmodSync(join(bin, name), 0o755); + }; + write('sleep', '#!/bin/bash\nexit 0\n'); + write( + 'gh', + [ + '#!/bin/bash', + 'echo "$*" >> "$CALLS"', + // The head-binding guard runs BEFORE the upsert attempts: pr view + // succeeds even in all-fail so that scenario still exercises the + // retry loop. + 'case "$*" in', + ' "pr view"*)', + ' if [ "$SCENARIO" = "moved-head" ]; then printf "OPEN\\tdeadbeef\\n";', + ' elif [ "$SCENARIO" = "closed-pr" ]; then printf "MERGED\\tabc123\\n";', + ' else printf "OPEN\\tabc123\\n"; fi', + ' exit 0 ;;', + 'esac', + 'if [ "$SCENARIO" = "all-fail" ]; then exit 1; fi', + 'case "$*" in', + ' "api user"*) echo bot ;;', + ' *"--method GET"*)', + ' echo \'[{"id":31,"user":{"login":"bot"},"body":" badge"}]\' ;;', + ' *) : ;;', + 'esac', + 'exit 0', + ].join('\n') + '\n', + ); + const script = [ + 'set -euo pipefail', + 'GITHUB_REPOSITORY=o/r', + 'PR_NUMBER=42', + `RUNNER_TEMP="${dir}"`, + `EXPECTED_HEAD_SHA=${expectedHeadSha}`, + `DOCS_ONLY_MEDIUM=${docsOnlyMedium}`, + `REVIEW_COMPLETED=${reviewCompleted}`, + 'RUN_URL=https://x', + supersedeRun, + 'echo "STEP_EXIT_OK"', + ].join('\n'); + const stdout = execFileSync('bash', ['-c', script], { + encoding: 'utf8', + cwd: process.cwd(), + env: { + ...process.env, + PATH: `${bin}:${process.env.PATH}`, + SCENARIO: scenario, + CALLS: calls, + }, + }); + return { stdout, calls: readFileSync(calls, 'utf8') }; + } finally { + rmSync(dir, { recursive: true, force: true }); + } + } + + it('supersedes an existing bot-authored badge (PATCH, update-only)', () => { + const r = runSupersede({ scenario: 'existing' }); + expect(r.calls).toContain( + 'api --method PATCH repos/o/r/issues/comments/31', + ); + expect(r.stdout).toContain('STEP_EXIT_OK'); + // Cause-neutral retired wording: it must hold even when an explicit full + // review completes on the SAME head the badge describes, so it may not + // claim the badge described an earlier revision. + expect(r.calls).toContain('(superseded)'); + expect(r.calls).toContain( + 'no longer reflects the current review state of this PR', + ); + expect(r.calls).not.toContain('earlier docs-only revision'); + }); + + it('updates the badge to a failure notice when a docs-only review failed', () => { + // The relay only runs on success; without this path the badge would keep + // quoting the previous revision's outcome for a head whose own run died. + const r = runSupersede({ + scenario: 'existing', + docsOnlyMedium: 'true', + reviewCompleted: '', + }); + expect(r.calls).toContain( + 'api --method PATCH repos/o/r/issues/comments/31', + ); + expect(r.calls).toContain('did not complete'); + expect(r.calls).toContain('abc123'); + expect(r.stdout).toContain('STEP_EXIT_OK'); + }); + + it('skips the badge update when the head moved before the write', () => { + const r = runSupersede({ scenario: 'moved-head' }); + expect(r.stdout).toContain('moved from abc123 to deadbeef'); + expect(r.calls).not.toContain('--method PATCH'); + }); + + it('skips the badge update when the PR closed before the write', () => { + const r = runSupersede({ scenario: 'closed-pr' }); + expect(r.stdout).toContain('is MERGED'); + expect(r.calls).not.toContain('--method PATCH'); + }); + + it('skips the badge update when the reviewed head SHA is unknown', () => { + // A run that failed before "Run review" emitted the SHA has nothing to + // bind to — a badge is never updated on ignorance. + const r = runSupersede({ scenario: 'existing', expectedHeadSha: '' }); + expect(r.stdout).toContain('reviewed head SHA is unknown'); + expect(r.calls).not.toContain('--method PATCH'); + }); + + it('warns and exits 0 when every supersede attempt fails', () => { + // The never-fail guard is load-bearing: a failing step here would trip + // the post-failure fallback into announcing a review failure that never + // happened (the same phantom the relay guard prevents). + const r = runSupersede({ scenario: 'all-fail' }); + expect(r.stdout).toContain('::warning::Could not supersede'); + expect(r.stdout).toContain('STEP_EXIT_OK'); + }); + + function ciRcFragment() { + const ci = readFileSync('.github/workflows/ci.yml', 'utf8'); + const ciDoc = parse(ci); + let run; + for (const job of Object.values(ciDoc.jobs)) { + for (const step of job.steps ?? []) { + if ((step.run ?? '').includes('classify-pr-profile.sh')) run = step.run; + } + } + expect(run).toBeTruthy(); + const start = run.indexOf('set +e'); + expect(start).toBeGreaterThan(-1); + const indent = run.slice(run.lastIndexOf('\n', start) + 1, start); + const end = run.indexOf(`\n${indent}fi`, start) + `\n${indent}fi`.length; + expect(end).toBeGreaterThan(start); + return run.slice(start, end); + } + + function runCiFragment(wrapper) { + const dir = mkdtempSync(join(tmpdir(), 'ci-rc-')); + try { + const stub = join(dir, '.github/scripts/ci'); + mkdirSync(stub, { recursive: true }); + writeFileSync(join(stub, 'classify-pr-profile.sh'), wrapper); + chmodSync(join(stub, 'classify-pr-profile.sh'), 0o755); + const script = [ + 'set -euo pipefail', + 'profile=full', + 'GITHUB_REPOSITORY=o/r', + 'PR_NUMBER=42', + ciRcFragment(), + 'printf "profile=%s" "$profile"', + ].join('\n'); + return execFileSync('bash', ['-c', script], { + encoding: 'utf8', + cwd: dir, + }); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + } + + it('ci.yml consumes the wrapper result on success', () => { + expect(runCiFragment('#!/bin/bash\necho docs_only\n')).toContain( + 'profile=docs_only', + ); + }); + + it.each([ + ['#!/bin/bash\nexit 2\n', 'Unable to list PR changed files'], + ['#!/bin/bash\nexit 3\n', 'classifier exited non-zero'], + ])('ci.yml falls back to full on wrapper failure (%#)', (wrapper, note) => { + // The probed mutant (deleting the rc handling) leaves profile EMPTY on + // failure — no downstream matrix bucket matches empty, and a broken PR + // would pass CI with zero tests run. + const out = runCiFragment(wrapper); + expect(out).toContain(note); + expect(out).toContain('profile=full'); + }); +}); + describe('upstream-timeout headroom (PR 8507 incident)', () => { // Three knobs, three failure modes: the SDK request timeout covers // connect+TTFB (three ~120s internal retries produced the 483s visible