perf(ci): run docs-only automatic reviews at medium effort (#8648)

* perf(ci): run docs-only automatic reviews at medium effort

A 1-line docs PR costs the same 57-180 minute high-effort review as a code
change, and on a diff with zero source lines the passes medium drops - the
adversarial personas and the reverse audit - have no failure mode to hunt.
Counterfactual analysis over six dissected CI runs showed the one case
where those passes caught a real Critical was a source PR, which this gate
never touches: classification reuses the Test workflow's conservative
classify-profile.mjs (docs/**.md(x) + root prose only; markdown under any
src/ tree stays full, matching the review skill's own source rule), and
any fetch or classifier failure falls back to the full review.

Only the automatic pull_request_target review downgrades; every explicit
request (workflow_dispatch, @qwen-code /review) keeps full high effort.
Because an effective --comment forces high and medium never posts, the
downgrade drops --comment and a new step relays the review CLI's verbatim
"Review complete:" line - its machine-readable completion contract - as a
single PR comment, with a pointer for requesting the full review. The
docs-only budget is the size-aware timeout halved with a 90-minute floor.

* perf(ci): address review feedback on the docs-only medium gate

All nine review suggestions, each verified before fixing:

- review_requested is an explicit ask: the AUTO_REVIEW flag now excludes
  that action (authorize write-permission-checks its requester), so a
  maintainer requesting the bot's review gets the full high-effort run.
- The fetch-and-classify wrapper is extracted to
  .github/scripts/ci/classify-pr-profile.sh and consumed by both ci.yml
  and the review gate, so the classifier's input contract lives in one
  place; distinct exit codes preserve each caller's fallback messages.
- Neither completion-line fallback mints the reserved "Review complete: "
  prefix anymore, and the relayed line passes a strict not-posted
  disposition allowlist - on this never-posts path any posted-form
  disposition is false by definition (the measured phantom APPROVE
  posted), so it falls back to a neutral non-scrapable form.
- The relay upserts by its marker (mirroring the queued-acknowledgement
  step) instead of stacking a comment per push, retries the POST/PATCH
  three times, and never fails the job - a failed relay after a
  successful review must not trip the failure fallback into announcing
  a review failure that never happened.
- The Chinese relay copy no longer parses as "发行" and renders
  high-effort as 高强度 rather than 高档.
- The qwen-review docs-only-medium marker is registered in all six
  BOT_COMMENT_FILTER sites in qwen-autofix.yml, so clean docs-only
  relays cannot select PRs into autofix rounds as actionable feedback.
- The gate's behavioral invariants are pinned in
  scripts/tests/qwen-pr-review-workflow.test.js by executing the
  extracted bash: prompt-branch order (--effort medium instead of
  --comment), the halve-with-90-minute-floor arithmetic, the
  completion-line allowlist including the phantom shapes, AUTO_REVIEW
  exclusivity, the six-site marker registration, and the shared-wrapper
  routing in both workflows.

* perf(ci): harden the docs-only gate against round-2 review findings

Thirteen findings across two review passes; every fix is executed by a
test rather than asserted as text where the finding was behavioral.

- The relay marker exclusion in qwen-autofix.yml is author-scoped at all
  six filter sites: only the relay bot's own marker comment is filtered,
  so a human quoting the marker stays actionable feedback.
- classify-pr-profile.sh guards the 3,000-file listing cap (any mismatch
  against the PR's declared changed_files classifies full), uses
  mktemp+trap instead of a fixed path on the shared persistent pool, and
  ships its own node:test suite (renamed source→docs pins the projection
  contract; exit codes 2/3 pinned) registered in HELPER_TESTS.
- classify-profile.mjs restricts reserved root prose basenames to inert
  extensions - README.js / SECURITY.ts / LICENSE.sh classify full.
- The completion-line allowlist binds to pr-<number> and to the only
  verdict a medium run can produce (Comment, not posted) - a stale line
  for another PR or an Approve-shaped injection falls back to neutral.
- A dedicated review_completed output gates the relay: the state/head
  guards exit 0 without running the review, and outcome==success alone
  would have announced a review that never ran.
- The relay upsert filters by the authenticated bot login, re-resolves
  the comment id on every attempt, and falls back to POST when the PATCH
  target is gone - a participant posting the marker can no longer capture
  the upsert, a transient listing failure no longer mints duplicates.
- The gate and relay are now executed under stubbed executables in
  qwen-pr-review-workflow.test.js (docs_only/full/failure/explicit
  scenarios; POST/PATCH/never-fail branches), the AUTO_REVIEW pin covers
  both guard halves, and the marker contract is pinned producer-side and
  filter-side.

* perf(ci): fix the medium Request-changes swallow and the stale docs badge

Round-3 review findings (2 Critical, 8 test-gap Suggestions), each fix
executed by a test where the finding was behavioral:

- The completion-line allowlist accepts `Request changes, not posted` -
  compose-review caps only Approve at medium, so a docs-only run that
  verifies a Critical legitimately emits Request changes, and the old
  Comment-only allowlist swallowed exactly the blocker-finding outcome
  into the neutral fallback. Target binding to pr-<number> is unchanged
  and now pinned by a test, as is the last-line selection over a stale or
  injected earlier completion line.
- A stale docs-only badge can no longer outlive its revision: the full
  automatic review path now supersedes the bot-authored marker comment
  (strikethrough + superseded note) via a new --update-only mode that
  never mints a badge where none existed.
- The marker+author upsert protocol is extracted to
  .github/scripts/upsert-bot-comment.sh - one implementation shared by
  the relay and the supersede step (the per-step copies had already
  drifted), with its own node:test suite covering the author scope, the
  per-attempt re-resolution (deleted-mid-retry falls back to POST), and
  the --update-only no-op; registered in HELPER_TESTS.
- The classify-pr-profile gh stub now applies the wrapper's own --jq
  argument with real jq over API-shaped fixtures, so the projection
  contract is genuinely under test (negative control: dropping `status`
  turns the renamed-source scenario red).
- New pins: review_completed wiring end to end (run-step emit + both
  consumers' if clauses), the auto_review output->env wiring at both
  links, and both AUTO_REVIEW guard halves.

* perf(ci): never let a failed lookup mint or keep a stale docs badge

Round-4 review findings (1 Critical, 7 Suggestions):

- The upsert script no longer conflates failed lookups with empty
  results: the authenticated login, the listing, and the jq extraction
  are all resolved inside the retry loop as one prerequisite chain, an
  attempt whose prerequisites failed retries instead of falling through
  to POST (the shape that minted a permanent duplicate badge off one
  transient 5xx), and --update-only exits 1 on a failed lookup so the
  supersede warning fires instead of a false no-op success. New tests
  pin the failed-listing-then-PATCH path, the persistent identity
  failure, the update-only failure exit, and the update-only PATCH.
- Supersede now covers every path that owes the correction: a FAILED
  full review and an EXPLICIT requested review (the badge's own CTA)
  both retire the badge, gated only on docs_only_medium == 'false' -
  empty on runs that failed before classifying, so a badge is never
  superseded on ignorance. The body is cause-neutral: it asserts only
  that the badge described an earlier revision.
- The marker literal is defined once per step (MARKER variable, the
  qwen-triage convention) and shared between body and lookup argument;
  a pin requires the definition and --update-only on the supersede
  invocation.
- New behavioral pins: the Approve verdict stays rejected by the
  allowlist, github_ci_only never downgrades (CI helpers are
  executable), and review_completed's emit position is asserted AFTER
  the closed-PR and stale-head guards (the hoist mutant survived
  position-independent contains checks).

* perf(ci): make docs_only_medium three-valued and pin the untested guards

Round-5 review findings (1 Critical, 6 Suggestions):

- docs_only_medium no longer conflates "determined not docs-only" with
  "never determined": the output is three-valued ('' when the
  classification failed or never ran), so a transient classifier failure
  or a dispatch dry-run can no longer retire a still-accurate badge. The
  supersede condition names its two licensed paths explicitly - a
  POSITIVE not-docs-only determination (without requiring review
  success), or an explicit comment-mode review that completed (the
  badge's CTA; report-mode dry runs retire nothing).
- The count-mismatch fallback in classify-pr-profile.sh logs to stderr,
  so a systematic divergence is distinguishable from every PR genuinely
  classifying full.
- Six probed surviving mutants now each turn a test red: the supersede
  body is executed (existing-badge PATCH and the never-fail guard),
  ci.yml's rc-handling fragment is executed (exit 0/2/3 with the
  full fallback), the changed_files fetch failure exits 2, duplicate
  badges PATCH the last (newest) comment, and the relay's POSTed body
  must carry the marker that keys both the upsert and the supersede.

* perf(ci): bind the docs badge to the reviewed head and retire it on failure

MDX pages are executable (imported components, expressions), so the
classifier no longer treats them as inert docs-only changes. The relay
and supersede writes re-read the live PR state/head immediately before
the mutation and skip unless the PR is still open at the reviewed SHA,
the badge body names that SHA, a failed docs-only review now retires the
singleton badge instead of leaving the previous revision's outcome
visible, and the retired wording is cause-neutral (an explicit review
can complete on the very head the badge describes).

Co-authored-by: Qwen-Coder <[email protected]>

---------

Co-authored-by: verify <verify@local>
Co-authored-by: Qwen-Coder <[email protected]>
Co-authored-by: qwen-code-dev-bot <[email protected]>
This commit is contained in:
authored and GitHub committed 2026-08-07 17:00:32 +00:00
1 parent efc7ec7a85
commit f4802031d0
10 files changed
+1505 -12

No files matched your search

+45
View File
@@ -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 <owner/repo> <pr-number>
# 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 <owner/repo> <pr-number>}"
pr="${2:?usage: classify-pr-profile.sh <owner/repo> <pr-number>}"
# 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
@@ -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);
});
+9 -2
View File
@@ -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,
)
);
+21 -1
View File
@@ -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');
});
+66
View File
@@ -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 <owner/repo> <issue-number> <marker> <body-file> [--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 <owner/repo> <issue-number> <marker> <body-file> [--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
+187
View File
@@ -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 = '<!-- test-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":"<!-- test-marker --> old"}]\'; fi ;;',
' fresh) echo "[]" ;;',
' existing-bot) echo \'[{"id":7,"user":{"login":"bot"},"body":"<!-- test-marker --> old"}]\' ;;',
' duplicate-pair) echo \'[{"id":5,"user":{"login":"bot"},"body":"<!-- test-marker --> older"},{"id":9,"user":{"login":"bot"},"body":"<!-- test-marker --> newer"}]\' ;;',
' existing-user) echo \'[{"id":8,"user":{"login":"alice"},"body":"<!-- test-marker --> mine"}]\' ;;',
' deleted-mid-retry)',
' if [ "$n" -le 1 ]; then echo \'[{"id":9,"user":{"login":"bot"},"body":"<!-- test-marker --> 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/);
});
+14 -8
View File
@@ -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."
+6
View File
@@ -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("<!-- qwen-review docs-only-medium "))) | not)
| select((.body // "") | test($cf) | not) ] | length' \
"${WORKDIR}/ic.json")"
@@ -3775,6 +3776,7 @@ jobs:
| select((.user.login // "") != $ab)
| select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb)
| select((.body // "") | test("<!-- (autofix-eval|autofix-rearm|qwen-triage|qwen-review-suggestion-summary|pr-force-push|qwen-review-ack) ") | not)
| select(((((.user.login // "") == $rb)) and ((.body // "") | test("<!-- qwen-review docs-only-medium "))) | not)
| select((.body // "") | test("^\\s*@qwen-code /") | not) | .created_at))
+ (.[3] | map(select((.conclusion // .state // "") | IN("FAILURE", "FAILED", "ERROR", "TIMED_OUT", "ACTION_REQUIRED", "CANCELLED"))
| select(((.workflowName // "") != "Qwen Autofix") or (((.name // "") | startswith("review-address"))))
@@ -3860,6 +3862,7 @@ jobs:
| select((.user.login // "") != $ab)
| select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb)
| select((.body // "") | test("<!-- (autofix-eval|autofix-rearm|qwen-triage|qwen-review-suggestion-summary|pr-force-push|qwen-review-ack) ") | not)
| select(((((.user.login // "") == $rb)) and ((.body // "") | test("<!-- qwen-review docs-only-medium "))) | not)
| select((.body // "") | test("^\\s*@qwen-code /") | not)) | length)
+ (.[3] | map(select((.conclusion // .state // "") | IN("FAILURE", "FAILED", "ERROR", "TIMED_OUT", "ACTION_REQUIRED", "CANCELLED"))
| select(((.workflowName // "") != "Qwen Autofix") or (((.name // "") | startswith("review-address"))))
@@ -3947,6 +3950,7 @@ jobs:
| {at: (.created_at // ""), login: (.user.login // ""), assoc: (.author_association // "")} ]
+ [ ($ic | add)[]
| select((.body // "") | test("<!-- (autofix-eval|autofix-rearm|qwen-triage|qwen-review-suggestion-summary|pr-force-push|qwen-review-ack) ") | not)
| select(((((.user.login // "") == $rb)) and ((.body // "") | test("<!-- qwen-review docs-only-medium "))) | not)
| select((.body // "") | test("^\\s*@qwen-code /") | not)
| select(((.body // "") | contains("**[Critical]**")) | not)
| {at: (.created_at // ""), login: (.user.login // ""), assoc: (.author_association // "")} ])
@@ -4012,6 +4016,7 @@ jobs:
| select((.user.login // "") != $ab)
| select(((.user.login // "") == $rb) or ((.user.login // "") | IN($over[])))
| select((.body // "") | test("<!-- (autofix-eval|autofix-rearm|qwen-triage|qwen-review-suggestion-summary|pr-force-push|qwen-review-ack) ") | not)
| select(((((.user.login // "") == $rb)) and ((.body // "") | test("<!-- qwen-review docs-only-medium "))) | not)
| select((.body // "") | test("^\\s*@qwen-code /") | not)
| select(((.body // "") | contains("**[Critical]**")) | not)
| "- PR comment by @\(.user.login): \(.html_url // $pr_url)"' \
@@ -4091,6 +4096,7 @@ jobs:
| select((.user.login // "") != $ab)
| select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb)
| select((.body // "") | test("<!-- (autofix-eval|autofix-rearm|qwen-triage|qwen-review-suggestion-summary|pr-force-push|qwen-review-ack) ") | not)
| select(((((.user.login // "") == $rb)) and ((.body // "") | test("<!-- qwen-review docs-only-medium "))) | not)
| select((.body // "") | test("^\\s*@qwen-code /") | not)
| select(($critical_only | not)
or (((.user.login // "") != $rb) and (((.user.login // "") | IN($over[])) | not))
+275 -1
View File
@@ -443,6 +443,13 @@ jobs:
# input or a /review --timeout=N comment). When false, "Run review"
# replaces this default with a PR-size-aware tier instead.
TIMEOUT_EXPLICIT=false
# True only for the automatic pull_request_target review — every
# other path is a human asking for a review explicitly, and an
# explicit ask always gets the full high-effort run. That includes
# the review_requested action: a maintainer clicking "Request
# review" is an explicit ask (authorize write-permission-checks the
# requester for exactly that action), so it is excluded below.
AUTO_REVIEW=false
TRIGGER_COMMAND="${TRIGGER_BODY%%$'\n'*}"
if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then
@@ -467,6 +474,10 @@ jobs:
fi
PR_NUMBER="${{ github.event.pull_request.number }}"
REVIEW_MODE="comment"
if [ "${{ github.event_name }}" = "pull_request_target" ] &&
[ "${{ github.event.action }}" != "review_requested" ]; then
AUTO_REVIEW=true
fi
else
echo "Unsupported event: ${{ github.event_name }}" >&2
exit 1
@@ -495,6 +506,7 @@ jobs:
echo "review_mode=$REVIEW_MODE"
echo "timeout_minutes=$TIMEOUT_MINUTES"
echo "timeout_explicit=$TIMEOUT_EXPLICIT"
echo "auto_review=$AUTO_REVIEW"
} >> "$GITHUB_OUTPUT"
- name: 'Setup Node.js for hosted review'
@@ -684,6 +696,7 @@ jobs:
OPENAI_MODEL: '${{ vars.QWEN_PR_REVIEW_MODEL }}'
PR_NUMBER: '${{ steps.context.outputs.pr_number }}'
REVIEW_MODE: '${{ steps.context.outputs.review_mode }}'
AUTO_REVIEW: '${{ steps.context.outputs.auto_review }}'
TIMEOUT_MINUTES: '${{ steps.context.outputs.timeout_minutes }}'
TIMEOUT_EXPLICIT: '${{ steps.context.outputs.timeout_explicit }}'
# Timeout headroom for CI reviews, measured against the PR 8507
@@ -984,6 +997,56 @@ jobs:
fi
fi
# Docs-only automatic reviews run at medium effort. The classifier is
# the same one the Test workflow's CI-profile gate already trusts
# (.github/scripts/ci/classify-profile.mjs) and it is conservative by
# construction: only docs/**.md and root-level prose files classify
# docs_only — MDX is executable (imported components, expressions)
# and stays `full`, as does markdown under any src/ tree, matching
# the review skill's own "markdown inside a source tree counts as
# source" rule — and any fetch or classifier failure falls back to
# the full review. On a diff with zero source lines, the passes
# medium drops (adversarial personas, reverse audit) have no
# failure mode to hunt;
# medium keeps the verified finder fan-out. `--comment` is dropped
# with the downgrade — an effective --comment forces high
# (parse-args), and medium never posts — so the "Report docs-only
# medium outcome" step relays the review's completion line instead.
# Explicit requests (dispatch, @qwen-code /review) never downgrade.
# Three-valued on purpose: 'true' (classified docs-only), 'false'
# (POSITIVELY classified not-docs-only by a successful automatic
# classification), '' (never determined — explicit run, or the
# classification failed). The supersede step keys on 'false', so
# conflating "never determined" with "determined not docs-only"
# would retire a still-accurate badge on a transient classifier
# failure or a dry run.
DOCS_ONLY_MEDIUM=""
if [ "${AUTO_REVIEW:-false}" = "true" ]; then
# Fetch + classify through the shared wrapper so this gate and
# ci.yml's profile gate cannot drift on the classifier's input
# contract. Exit 2 = listing failed, 3 = classifier failed —
# either way the fallback is the full review.
set +e
profile="$(.github/scripts/ci/classify-pr-profile.sh "${REPO}" "${PR_NUMBER}")"
classify_rc=$?
set -e
if [ "$classify_rc" -ne 0 ]; then
echo "::warning::Docs-only gate could not classify PR files (exit ${classify_rc}); running the full review."
elif [ "$profile" = "docs_only" ]; then
DOCS_ONLY_MEDIUM=true
# Medium measures at one-third to one-half of high, so halve
# the size-aware budget with a 90-minute floor.
EFFECTIVE_TIMEOUT_MINUTES=$(( EFFECTIVE_TIMEOUT_MINUTES / 2 ))
if [ "$EFFECTIVE_TIMEOUT_MINUTES" -lt 90 ]; then
EFFECTIVE_TIMEOUT_MINUTES=90
fi
echo "PR #${PR_NUMBER} is docs-only; automatic review runs at --effort medium (${EFFECTIVE_TIMEOUT_MINUTES}-minute budget)."
else
DOCS_ONLY_MEDIUM=false
fi
fi
echo "docs_only_medium=$DOCS_ONLY_MEDIUM" >> "$GITHUB_OUTPUT"
if ! PR_DATA="$(gh pr view "$PR_NUMBER" --repo "$REPO" --json state,headRefOid --jq '[.state, .headRefOid] | @tsv')"; then
fail "Failed to determine state for PR #${PR_NUMBER}."
fi
@@ -1010,7 +1073,9 @@ jobs:
} >> "$GITHUB_OUTPUT"
PROMPT="/review ${REVIEW_URL}"
if [ "$REVIEW_MODE" = "comment" ]; then
if [ "$DOCS_ONLY_MEDIUM" = "true" ]; then
PROMPT="${PROMPT} --effort medium"
elif [ "$REVIEW_MODE" = "comment" ]; then
PROMPT="${PROMPT} --comment"
fi
@@ -1192,6 +1257,215 @@ jobs:
fail "$REASON" 1 "$KIND"
done
if [ "$DOCS_ONLY_MEDIUM" = "true" ]; then
# The review CLI's machine-readable completion contract — batch
# drivers detect completion by this exact line, and it is the one
# verdict statement the relay step may quote (asserting a verdict
# the run did not print is the failure the review skill measures).
# The line is model-authored, so relaying it under the bot's name
# gets a strict allowlist, not a prefix check: this path never
# posts (no --comment), so any `posted`-form disposition is false
# by definition — the skill's DESIGN.md records a measured phantom
# `APPROVE posted` — and anything outside the not-posted shape
# (including injection-steered text) falls back to the neutral
# form, which deliberately does NOT start with the reserved
# `Review complete: ` prefix so log scrapers never parse it.
COMPLETION_LINE="$(printf '%s\n' "$RESULT_TEXT" | grep -E '^Review complete: ' | tail -n1 || true)"
# Bound to THIS PR's target and to the verdicts a medium run can
# produce: Comment, or Request changes when it verified a Critical
# (compose-review caps only Approve at medium — a docs-only run
# that found a blocker is exactly the outcome the relay must not
# swallow). A stale line for another PR, or an Approve-shaped
# injection, must not be republished under the bot's name.
if ! printf '%s' "$COMPLETION_LINE" \
| grep -qE "^Review complete: pr-${PR_NUMBER} — (Comment|Request changes), not posted \([0-9]+ Critical, [0-9]+ Suggestion\)$"; then
COMPLETION_LINE=""
fi
echo "completion_line=${COMPLETION_LINE:-(no relayable \"Review complete:\" line in the run output — see the run log)}" >> "$GITHUB_OUTPUT"
fi
# Distinct from the step's exit code: the state/head guards above
# exit 0 without running Qwen (closed PR, stale head), and the
# relay must not announce a review that never ran.
echo "review_completed=true" >> "$GITHUB_OUTPUT"
- name: 'Report docs-only medium outcome'
if: |-
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 != ''
env:
GH_TOKEN: '${{ secrets.CI_BOT_PAT }}'
PR_NUMBER: '${{ steps.context.outputs.pr_number }}'
EXPECTED_HEAD_SHA: '${{ steps.review.outputs.expected_head_sha }}'
COMPLETION_LINE: "${{ steps.review.outputs.completion_line || '(no relayable \"Review complete:\" line in the run output — see the run log)' }}"
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 stale-head guard in
# "Run review" checked at step start; a push that landed since
# must not receive this head's outcome (the fallback-comment step
# uses the same guard shape).
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 relaying the docs-only outcome; skipping the relay."
exit 0
fi
IFS=$'\t' read -r PR_STATE CURRENT_HEAD_SHA <<< "$PR_DATA"
if [ "$PR_STATE" != "OPEN" ]; then
echo "Skipping docs-only relay: PR #${PR_NUMBER} is ${PR_STATE}."
exit 0
fi
if [ "$CURRENT_HEAD_SHA" != "$EXPECTED_HEAD_SHA" ]; then
echo "Skipping docs-only relay: PR #${PR_NUMBER} moved from ${EXPECTED_HEAD_SHA} to ${CURRENT_HEAD_SHA}."
exit 0
fi
# Medium never posts inline comments and --comment would force high
# effort back on (parse-args), so a docs-only automatic review
# reports through this single relay instead. The quoted line arrives
# allowlisted by "Run review" (not-posted disposition shape only) —
# this step asserts nothing the run did not print. The marker is
# defined ONCE and used for both the body and the upsert lookup —
# the two must be byte-identical or the upsert posts duplicates.
MARKER='<!-- qwen-review docs-only-medium -->'
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\`." \
'' \
'<details><summary>中文说明</summary>' \
'' \
"📄 **纯文档变更** —— 自动评审以 \`--effort medium\` 运行(发现已验证、无反向审计;medium 不发布行内评论),结果见上方引用行。评审的 head:\`${EXPECTED_HEAD_SHA}\`。完整报告见 [workflow 运行](${RUN_URL});如需带行内评论的完整高强度(high-effort)评审,请评论 \`@qwen-code /review\`。" \
'' \
'</details>')"
# 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='<!-- qwen-review docs-only-medium -->'
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})." \
'' \
'<details><summary>中文说明</summary>' \
'' \
"📄 **纯文档变更** —— head \`${EXPECTED_HEAD_SHA}\` 的自动 \`--effort medium\` 评审**未能完成**,当前没有有效的评审结果。详见本 PR 上的失败评论与 [workflow 运行](${RUN_URL})。" \
'' \
'</details>')"
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.' \
'' \
'<details><summary>中文说明</summary>' \
'' \
'📄 ~~纯文档变更~~ **(已失效)** —— 该徽章已不再反映本 PR 当前的评审状态。请以本 PR 上最新的评审动态为准。' \
'' \
'</details>')"
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() &&
@@ -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(/<!-- qwen-review docs-only-medium -->/);
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\("<!-- qwen-review docs-only-medium "\)\)\) \| not\)/g,
) ?? [];
expect(scoped.length).toBeGreaterThanOrEqual(6);
// No body-only exclusion of the marker may survive anywhere: every
// marker test must carry the author scope, and a filter missing
// `| not` inverts to keep ONLY the badge — it then drops out of the
// scoped count above, so either mutant fails this pin.
const markerTests =
autofix.match(/test\("<!-- qwen-review docs-only-medium "\)/g) ?? [];
expect(markerTests.length).toBe(scoped.length);
});
it('routes classification through the shared classify-pr-profile wrapper', () => {
// Both this gate and ci.yml must consume the classifier via the shared
// script so its input contract lives in one place.
expect(run).toContain('.github/scripts/ci/classify-pr-profile.sh');
const ci = readFileSync('.github/workflows/ci.yml', 'utf8');
expect(ci).toContain('.github/scripts/ci/classify-pr-profile.sh');
expect(ci).not.toContain(
"--jq '.[] | {filename, status, previous_filename}'",
);
});
});
describe('docs-only gate and relay, executed', () => {
// R2-4 / R3-4: the classification gate and the relay step are the feature's
// two integration boundaries; both are executed here with stubbed
// executables rather than asserted as text, because the probed mutants
// (flipped PATCH/POST branch, swapped exit-code handling, inverted
// docs_only comparison) all stayed green under text-only assertions.
const doc = parse(workflow);
const runStep = doc.jobs['review-pr'].steps.find(
(s) => s.name === 'Run review',
).run;
const relayRun = doc.jobs['review-pr'].steps.find(
(s) => s.name === 'Report docs-only medium outcome',
).run;
function gateSource() {
const start = runStep.indexOf('DOCS_ONLY_MEDIUM=""');
const endAnchor =
'echo "docs_only_medium=$DOCS_ONLY_MEDIUM" >> "$GITHUB_OUTPUT"';
const end = runStep.indexOf(endAnchor) + endAnchor.length;
expect(start).toBeGreaterThan(-1);
expect(end).toBeGreaterThan(start);
return runStep.slice(start, end);
}
function runGate({ autoReview, wrapper }) {
const dir = mkdtempSync(join(tmpdir(), 'docs-gate-'));
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 gho = join(dir, 'gho');
writeFileSync(gho, '');
const script = [
'set -euo pipefail',
`AUTO_REVIEW=${autoReview}`,
'REPO=o/r',
'PR_NUMBER=42',
'EFFECTIVE_TIMEOUT_MINUTES=360',
`GITHUB_OUTPUT="${gho}"`,
gateSource(),
'printf "timeout=%s" "$EFFECTIVE_TIMEOUT_MINUTES"',
].join('\n');
const stdout = execFileSync('bash', ['-c', script], {
encoding: 'utf8',
cwd: dir,
});
return { stdout, output: readFileSync(gho, 'utf8').trim() };
} finally {
rmSync(dir, { recursive: true, force: true });
}
}
it('downgrades and halves the budget when the classifier says docs_only', () => {
const r = runGate({
autoReview: 'true',
wrapper: '#!/bin/bash\necho docs_only\n',
});
expect(r.output).toBe('docs_only_medium=true');
expect(r.stdout).toContain('timeout=180');
});
it('keeps the full review for a full classification', () => {
const r = runGate({
autoReview: 'true',
wrapper: '#!/bin/bash\necho full\n',
});
expect(r.output).toBe('docs_only_medium=false');
expect(r.stdout).toContain('timeout=360');
});
it('falls back to the full review when the wrapper fails', () => {
const r = runGate({ autoReview: 'true', wrapper: '#!/bin/bash\nexit 2\n' });
// Empty, not 'false': a failed classification is 'never determined', and
// the supersede step must not read it as a positive determination.
expect(r.output).toBe('docs_only_medium=');
expect(r.stdout).toContain('could not classify');
expect(r.stdout).toContain('timeout=360');
});
it('keeps the full review for github_ci_only (CI helpers are executable)', () => {
const r = runGate({
autoReview: 'true',
wrapper: '#!/bin/bash\necho github_ci_only\n',
});
expect(r.output).toBe('docs_only_medium=false');
expect(r.stdout).toContain('timeout=360');
});
it('never classifies on an explicit (non-automatic) run', () => {
const r = runGate({
autoReview: 'false',
wrapper: '#!/bin/bash\necho docs_only\n',
});
expect(r.output).toBe('docs_only_medium=');
expect(r.stdout).toContain('timeout=360');
});
function runRelay({ scenario }) {
const dir = mkdtempSync(join(tmpdir(), 'docs-relay-'));
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, while moved-head/closed-pr exercise the guard.
'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 \'{"login":"relay-bot"}\' | jq -r .login; exit 0 ;;',
' *"--method GET"*)',
' if [ "$SCENARIO" = "existing" ]; then',
' echo \'[{"id":777,"user":{"login":"relay-bot"},"body":"<!-- qwen-review docs-only-medium --> 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('<!-- qwen-review docs-only-medium -->');
// 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='<!-- qwen-review docs-only-medium -->'",
);
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":"<!-- qwen-review docs-only-medium --> 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