perf(ci): run docs-only automatic reviews at medium effort - #8648
Conversation
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.
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查。 建议见行内评论。 未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.6)
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.
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
.github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted. [Critical] R2-9 .github/workflows/qwen-code-pr-review.yml: the singleton docs-only medium status comment is not cleared or superseded when a later push changes the PR to a full/source profile. A docs-only revision can leave a stale bot-authored medium outcome visible after the current head receives a full review.
中文说明
.github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。 [Critical] R2-9 .github/workflows/qwen-code-pr-review.yml: the singleton docs-only medium status comment is not cleared or superseded when a later push changes the PR to a full/source profile. A docs-only revision can leave a stale bot-authored medium outcome visible after the current head receives a full review.
— gpt-5.6-sol via Qwen Code /review (v0.21.6)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Unresolved, please confirm: [Critical] R2-5 (inline, .github/workflows/qwen-code-pr-review.yml:988): the gate carries no truncation guard — factual, it compares nothing against the PR's changedFiles — but whether the paginated files endpoint actually truncates (the claimed 3,000-file cap) is GitHub API behavior this run could not establish locally; cannot tell.
Not reviewed: the executable-script lint — .github/scripts/ci/classify-pr-profile.sh: shellcheck is not installed.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
[Critical] R2-2: Still standing (re-checked at f178af5): the relay step fires after a SKIPPED review — docs_only_medium=true is written (~line 1003) before the closed-PR (~1011) and stale-head (~1018) exit 0 paths, so steps.review.outcome == 'success' satisfies the relay if: (1237-1241) and it posts that the automatic medium review ran when no review ran; on a closed PR the false comment persists. Mechanism independently re-traced by this round.
[Critical] R2-3: Still standing (re-checked at f178af5): the relay's upsert selects the last marker-bearing comment without verifying it belongs to the relay bot (line 1277) — the queued-acknowledgement step it mirrors filters by author (line 129). A participant-posted marker captures EXISTING_ID and suppresses or hijacks the authoritative relay. Mechanism independently re-traced by this round.
[Critical] R2-1: Still standing (re-checked at f178af5): the medium-branch completion-line allowlist (line 1230) accepts any \S+ target token — probed this round: whitespace-free markdown link / autolink / HTML-comment tokens all pass — and also accepts Approve, which a medium run can never produce. Note: Request changes must stay accepted (a medium run with a verified Critical completes as Request changes); only Approve is unreachable for medium.
[Critical] R2-7: Still standing (re-checked at f178af5): the classifier's root-prose regex (classify-profile.mjs:20, ^(?:README|CHANGELOG|...)(?:\.[^/]*)?$) accepts arbitrary extensions, so README.js / SECURITY.ts / LICENSE.sh classify as docs_only and an automatic PR touching only such an executable file is downgraded to medium.
[Critical] R2-9: Still standing (re-checked at f178af5): the docs-only medium badge is upserted only by docs-only runs; when a later push makes the PR non-docs-only nothing deletes or supersedes the marker comment (the full run does not reference it; cleanup only filters marked comments; the workflow DELETE calls remove only autofix/precheck comments), so the stale badge persists after the head receives a full review.
[Critical] R2-10: Still standing (re-checked at f178af5): all six BOT_COMMENT_FILTER sites exclude the new relay marker by body text alone; a trusted human or actionable review comment quoting <!-- qwen-review docs-only-medium --> is dropped from the actionable-feedback count.
中文说明
未决，请确认：[Critical] R2-5 (inline, .github/workflows/qwen-code-pr-review.yml:988): the gate carries no truncation guard — factual, it compares nothing against the PR's changedFiles — but whether the paginated files endpoint actually truncates (the claimed 3,000-file cap) is GitHub API behavior this run could not establish locally; cannot tell.
未审查：the executable-script lint — .github/scripts/ci/classify-pr-profile.sh: shellcheck is not installed。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
[Critical] R2-2: Still standing (re-checked at f178af5): the relay step fires after a SKIPPED review — docs_only_medium=true is written (~line 1003) before the closed-PR (~1011) and stale-head (~1018) exit 0 paths, so steps.review.outcome == 'success' satisfies the relay if: (1237-1241) and it posts that the automatic medium review ran when no review ran; on a closed PR the false comment persists. Mechanism independently re-traced by this round.
[Critical] R2-3: Still standing (re-checked at f178af5): the relay's upsert selects the last marker-bearing comment without verifying it belongs to the relay bot (line 1277) — the queued-acknowledgement step it mirrors filters by author (line 129). A participant-posted marker captures EXISTING_ID and suppresses or hijacks the authoritative relay. Mechanism independently re-traced by this round.
[Critical] R2-1: Still standing (re-checked at f178af5): the medium-branch completion-line allowlist (line 1230) accepts any \S+ target token — probed this round: whitespace-free markdown link / autolink / HTML-comment tokens all pass — and also accepts Approve, which a medium run can never produce. Note: Request changes must stay accepted (a medium run with a verified Critical completes as Request changes); only Approve is unreachable for medium.
[Critical] R2-7: Still standing (re-checked at f178af5): the classifier's root-prose regex (classify-profile.mjs:20, ^(?:README|CHANGELOG|...)(?:\.[^/]*)?$) accepts arbitrary extensions, so README.js / SECURITY.ts / LICENSE.sh classify as docs_only and an automatic PR touching only such an executable file is downgraded to medium.
[Critical] R2-9: Still standing (re-checked at f178af5): the docs-only medium badge is upserted only by docs-only runs; when a later push makes the PR non-docs-only nothing deletes or supersedes the marker comment (the full run does not reference it; cleanup only filters marked comments; the workflow DELETE calls remove only autofix/precheck comments), so the stale badge persists after the head receives a full review.
[Critical] R2-10: Still standing (re-checked at f178af5): all six BOT_COMMENT_FILTER sites exclude the new relay marker by body text alone; a trusted human or actionable review comment quoting <!-- qwen-review docs-only-medium --> is dropped from the actionable-feedback count.
— qwen3.8-max via Qwen Code /review (v0.21.6)
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.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.7)
…adge 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.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: reverse audit — reached the 5-round hard cap without two consecutive dry rounds; the final round still reported a probe-verified finding, so convergence was not established.
Not reviewed: the executable-script lint — .github/scripts/ci/classify-pr-profile.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/upsert-bot-comment.sh: shellcheck is not installed.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未审查：reverse audit — reached the 5-round hard cap without two consecutive dry rounds; the final round still reported a probe-verified finding, so convergence was not established。
未审查：the executable-script lint — .github/scripts/ci/classify-pr-profile.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/upsert-bot-comment.sh: shellcheck is not installed。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.7)
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).
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: the executable-script lint — .github/scripts/ci/classify-pr-profile.sh: shellcheck is not installed.
Not reviewed: the executable-script lint — .github/scripts/upsert-bot-comment.sh: shellcheck is not installed.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未审查：the executable-script lint — .github/scripts/ci/classify-pr-profile.sh: shellcheck is not installed。
未审查：the executable-script lint — .github/scripts/upsert-bot-comment.sh: shellcheck is not installed。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.7)
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.
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
.github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted. [Critical] R3-1 .github/workflows/qwen-code-pr-review.yml: The supersede comment falsely says the badge described an earlier revision and no longer reflects the current head when an explicit full review completes on the same unchanged docs-only SHA. Use cause-neutral wording or distinct messages for same-head explicit review and later-head classification paths.
中文说明
R3-1 .github/workflows/qwen-code-pr-review.yml：当显式 full review 在同一个未变化的纯文档 SHA 上完成时，supersede 评论却错误声称该徽章描述的是更早修订且已不再反映当前 head。请使用对两条路径都成立的中性措辞，或分别处理同 head 显式评审与后续 head 分类。
中文说明
.github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。 [Critical] R3-1 .github/workflows/qwen-code-pr-review.yml: The supersede comment falsely says the badge described an earlier revision and no longer reflects the current head when an explicit full review completes on the same unchanged docs-only SHA. Use cause-neutral wording or distinct messages for same-head explicit review and later-head classification paths.
中文说明
R3-1 .github/workflows/qwen-code-pr-review.yml：当显式 full review 在同一个未变化的纯文档 SHA 上完成时，supersede 评论却错误声称该徽章描述的是更早修订且已不再反映当前 head。请使用对两条路径都成立的中性措辞，或分别处理同 head 显式评审与后续 head 分类。
— gpt-5.6-sol via Qwen Code /review (v0.21.5)
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
[Critical] R3-2 (existing thread, comment 3736530395, .github/scripts/ci/classify-profile.mjs:19) — still stands at e3a20ec: docs/**/*.mdx classifies docs_only, and this diff newly routes that classification into the review-effort downgrade; MDX can carry executable content (imported components, expressions), and this PR's own R2-7 precedent treated executable content in the docs-only set as blocking. Mitigating observations from this round: docs/ currently contains zero .mdx files and no CI in this repo builds the docs site, so the hazard is latent — but the classifier's docs-only set is not limited to non-executable prose as the PR's rationale claims.
[Critical] R3-4 (existing thread, comment 3736530401, .github/workflows/qwen-code-pr-review.yml:1260) — still stands at e3a20ec: a failed automatic docs-only review of a later revision leaves the badge quoting the previous revision's successful outcome — the relay requires outcome == 'success' while both supersede paths require docs_only_medium == 'false' or an explicit comment-mode completion, so no step updates the singleton on that shape, and the badge carries no SHA binding. (The failure fallback comment does announce the failure, but the badge's quoted Outcome and run link keep describing the older revision until a later run corrects it.)
[Critical] R3-3 (existing thread, comment 3736530407, .github/workflows/qwen-code-pr-review.yml:1304) — still stands at e3a20ec: neither the relay nor the supersede step re-reads the live PR head before its upsert; a head pushed after the stale-head guard (checked at step start) lets the write publish the old head's classification/outcome onto the new head. The synchronize cancel-in-progress narrows the window but does not close it — cancellation is async, and the badge body carries no SHA to bind it to the reviewed revision.
中文说明
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
[Critical] R3-2 (existing thread, comment 3736530395, .github/scripts/ci/classify-profile.mjs:19) — still stands at e3a20ec: docs/**/*.mdx classifies docs_only, and this diff newly routes that classification into the review-effort downgrade; MDX can carry executable content (imported components, expressions), and this PR's own R2-7 precedent treated executable content in the docs-only set as blocking. Mitigating observations from this round: docs/ currently contains zero .mdx files and no CI in this repo builds the docs site, so the hazard is latent — but the classifier's docs-only set is not limited to non-executable prose as the PR's rationale claims.
[Critical] R3-4 (existing thread, comment 3736530401, .github/workflows/qwen-code-pr-review.yml:1260) — still stands at e3a20ec: a failed automatic docs-only review of a later revision leaves the badge quoting the previous revision's successful outcome — the relay requires outcome == 'success' while both supersede paths require docs_only_medium == 'false' or an explicit comment-mode completion, so no step updates the singleton on that shape, and the badge carries no SHA binding. (The failure fallback comment does announce the failure, but the badge's quoted Outcome and run link keep describing the older revision until a later run corrects it.)
[Critical] R3-3 (existing thread, comment 3736530407, .github/workflows/qwen-code-pr-review.yml:1304) — still stands at e3a20ec: neither the relay nor the supersede step re-reads the live PR head before its upsert; a head pushed after the stale-head guard (checked at step start) lets the write publish the old head's classification/outcome onto the new head. The synchronize cancel-in-progress narrows the window but does not close it — cancellation is async, and the badge body carries no SHA to bind it to the reviewed revision.
— qwen3.8-max via Qwen Code /review (v0.21.7)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 268 passed · 0 failed · 268 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：268 通过 · 0 失败 · 268 总计 Verification reportPR 8648 Deep Verification —
|
| cell | arm | scenario (event shape / classification) | oracle | result |
|---|---|---|---|---|
| control | base | Run review contains no gate at all |
absence of DOCS_ONLY_MEDIUM / wrapper call |
✅ absent |
| B | base | automatic (comment-mode) PR | constructed PROMPT |
/review … --comment, timeout 360 |
| A | head | automatic + docs_only |
PROMPT, outputs, timeout, classifier calls |
/review … --effort medium (no --comment), docs_only_medium=true, 360→180, 1 call |
| C | head | automatic + full |
same | /review … --comment, docs_only_medium=false, 360 |
| D | head | automatic + wrapper exit 2 | same | /review … --comment, docs_only_medium= empty, ::warning::, 360 |
| E | head | automatic + wrapper exit 3 | same | same fail-open shape as D |
| F | head | explicit (AUTO_REVIEW=false), stub would say docs_only |
same + call count | /review … --comment, docs_only_medium= empty, 0 classifier calls |
| G | head | automatic + github_ci_only |
same | /review … --comment, docs_only_medium=false |
| H | head | dispatch dry-run (report mode) | PROMPT |
bare /review … (no flags) |
| ladder | head | docs_only budget 360/180/100/90/720 | timeout | 180/90/90/90/360 — halve-with-90-floor holds |
33/33 assertions passed (harness/ab-gate.mjs, harness/ab-gate.results.json). Cell B vs cell A is the load-bearing flip; all five non-docs_only cells (C/D/E/F/G) stayed on the full --comment path and the explicit path never invoked the classifier, so the downgrade is exactly as narrow as claimed.
2. Wire oracles on the changed surface
Executed with stubbed gh/sleep on PATH (the stubbed peer records every call; assertions are on the recorded wire, not on the step's own claims). Witness: 02-wire-oracle-wrapper-relay-supersede.png. 31/31 passed (harness/wire-harness.mjs).
- Wrapper (
classify-pr-profile.sh) against real jq + API-shaped fixtures (the stub applies the wrapper's own--jqargument, so the projection contract is genuinely under test): docs-only listing →docs_only;packages/core/src/skills/bundled/review/SKILL.md→full(the test-plan probe); renamed source→docs →full; renamed docs→docs →docs_only; truncated listing (1 retrieved vs 5 declared) →fullwith the stderr notice;README.js/LICENSE.sh→full, bareLICENSE→docs_only(the inert-extension restriction); listing failure → exit 2;changed_filesfetch failure → exit 2 (not swallowed). - Self-classification control: PR 8648's own 10-file diff driven through the real wrapper classifies
full— the PR adding the gate can never be downgraded by it. - Relay step, executed: fresh PR → POST with the marker leading the body, the completion line quoted verbatim, run link and bilingual block present; existing bot badge → PATCH id 777 (no stacking); a human-posted marker comment does not capture the upsert (POSTs fresh, never PATCHes 888); all-attempts-failed →
::warning::and exit 0 (the job must not trip the failure fallback). - Supersede step, executed: existing badge →
--update-onlyPATCH with strikethrough body, marker preserved; no badge → strict no-op (no POST, no PATCH); failed lookup → exit 0 with warning (never the silent no-op); human-owned marker cannot be overwritten. - Autofix filter oracle (real jq): the author-scoped marker clause is present at all six
qwen-autofix.ymlsites; executed against a fixture it excludes exactly the bot-authored badge (3→2) while a human quoting the marker stays actionable. - Completion-line allowlist vs the real producer contract: SKILL.md Step 9 (line 813) and
packages/cli/src/commands/review/compose-review.ts:688-689agree a medium run caps Approve→Comment but still emitsRequest changesfor a verified Critical — exactly the(Comment|Request changes), not postedshape the allowlist accepts, withpr-<n>target binding and posted-form/Approve rejection pinned by mutation M3 (§4).
Informational note (not a finding against this PR): SKILL.md also contains the phrasing "a medium verdict never exceeds Comment" (lines 597/666/1151/1246), which read literally would forbid Request changes at medium. Line 813 of the same file, DESIGN.md §575, and the compose-review implementation all state the Approve-only cap; the workflow allowlist matches the implementation. The skill doc is untouched by this PR — flagging only so the next reader of the relay code is not misled by the stale phrasing.
3. Relay/supersede gating truth table
GitHub expressions cannot execute locally, so the harness parses the real if: expressions from the workflow, pins their load-bearing clauses textually, and evaluates a 1:1 JS mirror over 14 state combinations (three-valued docs_only_medium × auto/explicit × comment/report mode × success/failure × closed/stale-head races × cancelled). 22/22 passed (harness/truth-table.mjs). Witness: 04-supersede-relay-truth-table.png. Key semantics held: classifier failure ('') never retires a badge; a failed-but-classified full run still does (path 1 needs no review success); a dispatch dry-run retires nothing; the relay never announces a review that the state/head guards skipped (review_completed requirement).
One observed benign window, by design: the gate runs before the state/stale-head guards, so a PR closed (or pushed to) between event dispatch and execution can have docs_only_medium=false emitted and the badge superseded even though the review itself is skipped. authorize excludes action == 'closed' entirely (verified in-file), so this is reachable only as a narrow timing race, and path 1's documented intent ("the review's own success is deliberately not required") covers it — the badge is retired only on a positive determination. Informational.
4. Mutation matrix (vacuity + attribution)
Each mutant is a single-point edit of the production file, applied and restored via git checkout (post-restore tree re-ran green; git status clean). 7/7 killed, no survivors. Witness: 03-mutation-matrix-all-killed.png.
| mutant | change | suite that caught it | killed by |
|---|---|---|---|
| M1 | prompt branch order swapped (--comment first) |
vitest | "emits --effort medium INSTEAD OF --comment on the docs-only path" |
| M2 | classifier failure sets DOCS_ONLY_MEDIUM=false instead of '' |
vitest | "falls back to the full review when the wrapper fails" |
| M3 | allowlist widened to accept Approve |
vitest | "rejects the Approve verdict medium can never produce" |
| M4 | || exit 2 deleted from the changed_files fetch |
node --test | "exit 2 when the changed_files fetch fails after a successful listing" |
| M5 | timeout floor comparison flipped (-lt 90 → -gt 90) |
vitest | "halves the size-aware budget with a 90-minute floor" |
| M6 | supersede path 1 widened to != 'true' (fires on '') |
vitest | "pins the review_completed wiring end to end" |
| M7 | jq projection drops status |
node --test | "a renamed source→docs file classifies full (the projection carries status/previous_filename)" |
Every mutant was killed by exactly the test the PR claims pins that behavior — attribution holds, and the positive controls (M4, M7) prove the harnesses can fail. The PR's suites were also run unmutated: vitest 82/82, node --test 85/85 across the four helper suites (including the untouched qwen-triage-workflow.test.mjs ownership-recovery pins the description claims). Both new helper test files are registered in ci.yml HELPER_TESTS (consumed by both Test-workflow paths). Witness: 05-pr-suites-green.png.
5. Lint gates
| gate | scope | result |
|---|---|---|
bash -n |
both new .sh scripts |
clean |
bash -n |
all five edited embedded run: blocks (context, Run review, relay, supersede, ci.yml classify; ${{ }} expressions placeholder-substituted) |
clean (72/536/41/24/24 lines) |
| shellcheck 0.11.0 | both new .sh scripts |
clean |
| actionlint 1.7.12 (repo's exact flags) | whole repo | clean |
| yamllint | — | could not run (container has no pip module; pip3 and apt-get install both failed) — see Not covered |
Findings
None blocking. The two informational observations (§2 skill-doc phrasing, §3 benign race window) are recorded for the maintainer; neither requires a change in this PR.
Not covered
- Per-commit attribution: the metadata names 6 commits, but the depth-2 merge-ref checkout reaches only the merge/base/head OIDs (
git rev-list HEAD^1..HEAD^2= 1,--is-shallow-repository= true). The aggregateHEAD^1..HEADdiff was verified; individual commit claims (rounds 1–5) were not separable. - yamllint: not installable in this container (
python3 -m pipmissing;scripts/lint.js --setupexits on it;apt-get install yamllintfailed). Structural YAML validity is partially compensated by actionlint and by every harness parsing the workflows with theyamlpackage, and the author's test plan reports yamllint 1.35.1 clean — but that is the author's claim, not measured here. - Replay calibration: first round, no GitHub token, no
previous-report.md, and no retrievable real emitted relay comment — the harnesses execute the extracted bash and its wire calls but are uncalibrated against an actual production workflow run's output. A real first relay posting (and its shape on the PR page) remains unobserved until the lane runs. - End-to-end medium review behavior (wall-clock inside the halved budget; the model actually emitting the completion line): depends on the merged 8642/7855 mechanics, not on this diff. This round verified the workflow/bash/script wiring, not a live review run.
- Live
gh api --paginatesemantics: stubs served API-shaped fixtures through real jq with the wrapper's own--jq; the real endpoint's page-concatenation behavior was not exercised. The upsert'sjq -s+.[][]is shape-robust to both flat and per-page arrays, but that is an argument, not a measurement. - ESLint (
npm run lint:ci) was not run; the PR adds no production TS/JS, and the added JS/MJS test files were executed by their suites. - Reviewer Test Plan step "behavior review:
--effortthreads deterministically sinceresolveEffort(PR 7855)": the flag's consumption by the review CLI was not re-verified here (out-of-diff dependency); the prompt construction feeding it is proven in §1.
Methodology
One container (the lane's own node:22-bookworm runtime; node v22.23.2, jq 1.x, gh present but unauthenticated — unused). Working tree = refs/pull/8648/merge (529ab4baf0); base control = scratch worktree at HEAD^1 (9e1b1eb49e), removed after the A/B cells were captured. No npm install/build was needed for the A/B: the diff touches only workflow YAML, shell scripts, and test files (dependency-tree delta verified empty), and the harnesses execute the extracted artifacts directly. Five harnesses in harness/ parse the workflow YAML with the yaml package, slice the real run: blocks between anchor strings, and execute them under bash with set -euo pipefail and stubbed executables (gh, sleep, the classifier wrapper) on PATH that record every invocation to a calls file — assertions read the recorded wire and the step's output files, never the step's self-report. The autofix filter was executed under real jq. Mutations were applied as exact single-occurrence string edits, tested, and restored (git diff --exit-code verified after each; final git status clean). Raw per-assertion results live in harness/*.results.json, suite logs in logs/pr-suites.txt, and the full diff in full-diff.txt.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM, no blockers. The core risk — a false docs-only classification downgrading a code-touching PR — is well-defended and verified by executing the real classifier: every ambiguity path (docs+code mixed, deleted-code-only, non-.md under docs/, empty file list, rename code->docs, lockfile, nested README.md) falls back to 'full'; only truly docs-only shapes get medium. The review workflow checks out the default branch so the PR can't tamper with the classifier; explicit /review comments and dispatch never downgrade; --effort medium is valid and an effective --comment forces high. The upsert-bot-comment helper is idempotent (marker+author lookup, PATCH newest, no duplication on failed listing), passes PR content as data (no gh injection), and --update-only never POSTs. Tests 29/29 new + 82/82 full workflow suite pass; CI green. P3s only: classification isn't bound to the exact reviewed SHA (existing stale-head/count guards make a false downgrade unreachable); add numeric-shape guards on repo/pr interpolation (defense-in-depth); gate runs classification before the closed/stale skip (minor API waste); skip the trailing sleep on the final retry attempt; fix a 'five copies' comment (six sites).
|
@qwen-code /resolve |
…ailure 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 <qwen-coder@alibabacloud.com>
|
Gate results for the re-run at the current head (autofix fixes + the
Moving on to code review. 🔍 中文说明当前 head（autofix 修复 +
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Code review (static — per policy I did not build or run any PR code). The three Criticals that blocked the previous head are genuinely closed at this one — I verified each against the code at this commit, not against the fix summary:
The round-5 Suggestions are closed too: the supersede wording is cause-neutral (pinned both ways — new wording present, old wording absent), the One contract check I ran independently: the relay's allowlist accepts exactly No new blockers. Two standing nits, neither blocking: the upsert helper sleeps 10 s after its final failed attempt (pure waste on the all-fail path), and the PR body still predates the badge machinery it ships. Testing — the PR's own CI on this commit, via API (I did not run any PR code; evidence below is GitHub-set metadata plus the green job's log):
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The Sandboxed verification is the lane that settles the end-to-end claim (a real docs-only PR actually reviewed at medium): it already passed 268/268 scripted assertions on the previous head, and a fresh run on this head was still in flight at posting time — 中文说明代码审查（静态审查——按策略未构建或运行任何 PR 代码）。阻断上一 head 的三个 Critical 在本 head 已真正闭环——逐条对照本 commit 代码核实，而非采信修复摘要：
round-5 的建议也全部闭环：supersede 措辞改为原因中立（双向钉住）、 独立做了一项契约核对：relay 白名单恰好接受 无新阻断项。两个遗留小问题，均不阻塞：upsert 助手在最后一次失败尝试后仍 sleep 10 秒（全失败路径上的纯浪费）；PR 描述仍早于其所交付的徽章机制。 测试证据：以上 CI 表格取自该 commit 的 API 检查结果（未运行 PR 代码）； 沙箱验证是能够敲定端到端主张（真实纯文档 PR 以 medium 评审）的通道：上一 head 已通过 268/268 断言，本 head 的新一轮在发布时仍在运行—— — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — right problem, engineering that holds up, and the three Criticals that blocked the previous head are verifiably closed; only cosmetic nits remain. My independent proposal for this problem was: classify with the classifier ci.yml already trusts, downgrade the prompt and halve the budget on the automatic path only, post one receipt comment — and make sure that receipt can never lie. This PR matches that shape and goes further, and this re-run confirmed the extra machinery earned its place: the previous round's three findings all hit the badge lifecycle, and each is now closed in code, not merely claimed — I re-read the classifier regexes, both write-step head guards, and the failure-path supersede branch at this commit, and the executed-fragment suites covering them ran green in this head's CI (49 files / 1045 tests in the workflow-pinning batch, plus the node --test helper suites incl. the MDX and truncation-guard cases). The fail-open posture is intact end to end: listing/classifier errors → full review; explicit requests → full review; any non-docs file → full review; unknown reviewed SHA → badge untouched. The merge with main broke nothing either side pins. If I had to maintain this in six months, the shared classifier wrapper and the executed tests are exactly the things I'd thank the author for. What keeps this at 4 rather than 5: the upsert helper sleeps 10 s after its final failed attempt (pure waste on the all-fail path), and the PR body still describes the original narrow change rather than the badge machinery that grew out of five review rounds — worth a refresh before merge, but neither blocks. One pre-existing wart this PR touches but does not cause: the review skill's prose says a medium verdict "never exceeds Comment" while its implementation (and this relay's allowlist, correctly) admit Request changes for a verified Critical — that wording lives in a file this diff doesn't touch and belongs in a separate fix. Verdict: approve. The PR's own CI on this head completed green ( Bookkeeping: the approval is pinned to the reviewed commit via 中文说明置信度：4/5 —— 问题真实、工程扎实，阻断上一 head 的三个 Critical 已可核实地闭环；仅剩外观性小问题。 我对这个问题的独立方案是：用 ci.yml 已信任的分类器做分类，仅在自动路径降档 prompt 并将预算减半，发一条回执评论——并确保这条回执永远不会说谎。本 PR 与该形态一致且走得更远，本次复跑确认额外机制确有其必要性：上一轮的三个发现全部落在徽章生命周期上，如今每一个都已在代码中闭环，而非口头声称——我在本 commit 上重读了分类器正则、两个写入步骤的 head 守卫与失败路径的 supersede 分支，覆盖它们的执行式片段套件在本 head 的 CI 中全绿（workflow 钉固批次 49 个文件 / 1045 个测试，加上 node --test 助手套件，含 MDX 与截断守卫用例）。 失败开放姿态端到端保持：列表/分类器出错 → 完整评审；显式请求 → 完整评审；任何非文档文件 → 完整评审；被评审 SHA 未知 → 徽章不动。与 main 的合并也未破坏任何一方钉住的内容。若六个月后由我维护，共享分类器 wrapper 与执行式测试正是值得感谢的部分。 给 4 分而非 5 分的原因：upsert 助手在最后一次失败尝试后仍 sleep 10 秒（全失败路径上的纯浪费）；PR 描述仍只描述最初的窄改动，未覆盖五轮评审中生长出来的徽章机制——合并前建议更新，但均不阻塞。另有一个本 PR 触及但未引起的既有瑕疵：review skill 的行文称 medium 裁决"从不超过 Comment"，而其实现（以及本 relay 的白名单，正确地）允许核实 Critical 时的 Request changes——该措辞位于本 diff 未触碰的文件中，应另行修复。 结论：批准。 该 head 的 PR 自身 CI 已全绿（ 备忘：批准通过 — Qwen Code · qwen3.8-max Reviewed at |
|
Triage re-run completed without a new review. The bot already has a review of its own on 机器人在 The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
@qwen-code /resolve |
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #8648One commit: Critical findings — all fixed
Suggestions — all addressed
Re-verified from earlier roundsAll 47 previously fixed findings re-checked at the new HEAD: the full executed suites (which pin each fix behaviorally) are green, and the code-level facts were spot-checked (author-scoped exclusion at all six autofix sites, Notes
VerificationCommands actually run this round (after the final code state):
中文说明Autofix 评审轮次 —— PR #8648一次提交： Critical 发现 —— 全部修复
Suggestion —— 全部处理
前几轮发现的复核此前已修复的 47 项发现在新 HEAD 上全部复核：完整执行套件（对每项修复做了行为级钉扎）全绿，并抽查了代码级事实（autofix 六处均带作者限定的排除、wrapper 中的 备注
验证本轮（最终代码状态下）实际执行的命令：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge resolution — PR #8648 (docs-only medium-effort reviews)Root causeMain merged Textual or semanticTextual. Both sides are pure additions after identical line 1234; neither No semantic overlap exists either: main's test reads step env of What is load-bearing
What I could not verifyNo build or tests were run. Each side's tests pin the workflow the other side 中文说明根因：main 合入了 #8673（ 纯文本冲突：双方都在第 1234 行之后纯新增，未修改同一段逻辑。解决方式为 关键不变量：本 PR 的标记契约测试要求 未验证项：未运行构建或测试（本命令只解决冲突）。两侧测试互为对方改动的 |
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 324 passed · 0 failed · 324 total 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：324 通过 · 0 失败 · 324 总计 Verification reportPR 8648 Deep Verification (round 2) —
|
| # | Prior finding | Severity | Status at new head |
|---|---|---|---|
| 1 | Docs-only gate runs before the state/stale-head guards, so in an extreme race a badge could be superseded even when the review is skipped | informational | No longer reachable. Round-6 added a pre-write state/head re-read to both the relay and the supersede step. Re-measured: on a docs-only PR found CLOSED at the guard, docs_only_medium=true is emitted but review_completed is not, so the relay's if: is false; no supersede path fires (path 1 needs 'false', path 2 needs explicit+completed, path 3 needs failure()); and even a triggered write would now skip the closed PR. Verified by A/B cell I, wire scenarios R4/S6, and the truth table. |
| 2 | Untouched SKILL.md phrasing ("a medium verdict never exceeds Comment") contradicts the Approve-only cap the implementation and the workflow allowlist use | informational | Stands. SKILL.md is not in this diff (git diff HEAD^1..HEAD --name-only confirms). Re-verified the allowlist `(Comment |
2. Scope
- Central claim: automatic
pull_request_targetreviews of docs-only PRs downgrade to--effort medium(no--comment), relay the outcome via an allowlisted completion line; every other path (explicit requests, classifier failure, non-docs profiles) is untouched. - Secondary claim 1 (round 6): badge writes are bound to the reviewed head — relay and supersede re-read live PR state/head immediately before the mutation and skip unless still OPEN at the reviewed SHA; a failed docs-only review retires the singleton badge with cause-neutral wording.
- Secondary claim 2 (round 6): MDX is executable content and never classifies
docs_only; the classifier keeps its fail-open contract. - Out of scope (see §8 Not covered): medium-review runtime behavior (merged 8642/7855), per-commit attribution, yamllint, live-API pagination, real-run replay calibration.
3. Central claim — A/B load-bearing proof
Environment per cell: the real run: block of the Run review step, extracted from qwen-code-pr-review.yml with a YAML parser and sliced through the PROMPT construction, executed under bash with set -euo pipefail; the classifier seam is a stubbed .github/scripts/ci/classify-pr-profile.sh on the step's relative path that records its invocations; gh/qwen/sleep are PATH stubs that log every call. Base arm = scratch worktree at HEAD^1 (d6b4ac8e34); the PR touches no package.json/lockfile (verified: dependency-file delta over HEAD^1..HEAD = 0), so the control is a clean code A/B. Witness: 01-ab-gate-base-vs-head.png. 49/49 passed (harness/ab-gate.mjs).
| cell | arm | scenario | oracle | result |
|---|---|---|---|---|
| B0 | base | "Run review" has no gate at all | absence of docs_only_medium / wrapper call |
✅ absent |
| B | base | automatic comment-mode PR | constructed PROMPT | /review … --comment, timeout 360, 0 classifier calls |
| A | head | automatic + docs_only |
PROMPT / outputs / timeout / call count | /review … --effort medium (no --comment), docs_only_medium=true, 360→180, 1 call |
| C | head | automatic + full |
same | /review … --comment, docs_only_medium=false, 360 |
| D | head | wrapper exit 2 | same | /review … --comment, docs_only_medium= empty, ::warning::, 360 |
| E | head | wrapper exit 3 | same | same fail-open shape as D |
| F | head | explicit (AUTO_REVIEW=false), stub would say docs_only |
+ call count | /review … --comment, empty, 0 classifier calls |
| G | head | github_ci_only |
same | /review … --comment, docs_only_medium=false |
| H | head | dispatch report mode | PROMPT | bare /review … |
| I | head | docs_only but PR CLOSED at guard | outputs / reach | exit 0, docs_only_medium=true emitted, no expected_head_sha, PROMPT never reached |
| ladder | head | budgets 360/180/100/90/720 | timeout | 180/90/90/90/360 — halve-with-90-floor holds |
Cell B vs A is the load-bearing flip; the five non-docs_only cells (C/D/E/F/G) all stayed on the full --comment path, and the explicit path never invoked the classifier — the downgrade is exactly as narrow as claimed.
4. Wire oracles on the changed surface
Executed with stubbed gh/sleep on PATH (the stub records every wire call; assertions read the recorded wire, never the step's self-report). Witness: 02-wire-oracle-wrapper-relay-supersede.png. 77/77 passed (harness/wire-harness.mjs).
Classifier wrapper (classify-pr-profile.sh) against real jq + API-shaped fixtures (the stub applies the wrapper's own --jq, so the projection contract is genuinely under test): docs-only listing → docs_only; round-6 MDX probes — docs/guide.mdx, docs/usage.md+docs/guide.mdx, and README.mdx all → full; markdown under a src/ tree → full (the test-plan probe); renamed source→docs → full, renamed docs→docs → docs_only; truncated listing (1 of 5) → full with the stderr notice; README.js/LICENSE.sh → full, bare LICENSE → docs_only; listing failure → exit 2; changed_files fetch failure → exit 2 (not swallowed). Self-classification control: PR 8648's own 10-file diff driven through the real wrapper classifies full.
Relay step, executed: fresh PR → POST with the marker leading, completion line quoted, Reviewed head: <sha> present, run link + bilingual block; existing bot badge → PATCH id 777 (no stacking); a human-posted marker does not capture the upsert (POSTs fresh, never PATCHes the human's id). Round-6 skip-on-write probes: PR CLOSED before write → exit 0, zero POST/PATCH; head moved → exit 0, zero writes; gh pr view failure → warn + exit 0, zero writes; all-attempts-failed → ::warning:: + exit 0 (the job is never failed).
Supersede step, executed: positive not-docs-only (path 1) → PATCH strikethrough body, marker preserved, cause-neutral (no failure language); round-6 failed docs-only run (path 3) → PATCH "did not complete" body naming the reviewed SHA, cause-neutral (names no quota/timeout/abort cause); no badge → strict no-op; unknown SHA → skip with no gh pr view call; head moved / non-OPEN → skip, zero writes; failed lookup → exit 0 with warning (never the silent no-op); explicit-completed (path 2) → superseded body, not the failure notice.
Completion-line allowlist (real grep, regex extracted from the workflow): accepts Comment, not posted and Request changes, not posted (the SKILL.md shapes a medium run can emit), rejects Approve, any posted form, a stale line bound to another PR, an injection-steered verdict word, trailing text, and the low-effort quick-pass shape. Witness: 05-completion-line-allowlist.png. 11/11 passed (harness/allowlist.mjs).
Autofix filter oracle (real jq): the author-scoped marker clause is present at all six qwen-autofix.yml sites (all added by this diff); executed against a fixture it excludes exactly the bot-authored badge (3→2) while a human quoting the marker stays actionable.
5. Relay/supersede gating truth table + mutation matrix
GitHub expressions cannot execute locally, so the harness parses the real if: expressions, pins them textually, and evaluates a 1:1 JS mirror over every consistent state combination. Witness: 04-supersede-relay-truth-table.png. 161/161 passed (harness/truth-table.mjs) across 50 combos × 3 + pins + spot checks. Held: classifier failure ('') never retires a badge; a failed-but-classified full run still retires (path 1 needs no success); a dispatch dry-run retires nothing; relay needs review_completed; relay and supersede never write in the same run; the relay if: carries no status function (implicit success()), supersede names failure()/cancelled().
Mutation matrix (single-point edits, applied & restored via git checkout, tree verified clean). Witness: 03-mutation-matrix-all-killed.png. 9/9 killed, no survivors, each by exactly the test the PR claims pins it; unmutated controls green before and after (harness/mutation-matrix.mjs).
| mutant | change | killed by |
|---|---|---|
| M1 | medium prompt branch disabled | "emits --effort medium INSTEAD OF --comment" |
| M2 | classifier failure recorded false not '' |
"falls back to the full review when the wrapper fails" |
| M3 | allowlist widened to accept Approve | "rejects the Approve verdict medium can never produce" |
| M4 | round-6: relay head-moved skip disabled | "skips the relay when the head moved before the write" |
| M5 | round-6: relay closed-PR skip disabled | "skips the relay when the PR closed before the write" |
| M6 | round-6: supersede path 3 removed | "pins the supersede if: including the OR grouping" |
| M7 | round-6: MDX re-admitted as inert docs | "MDX is executable content, never docs_only" |
| M8 | round-6: reviewed-head SHA removed from relay body | "POSTs a fresh relay comment…" (SHA assertion) |
| M9 | budget floor comparison flipped | "halves the size-aware budget with a 90-minute floor" |
All four round-6 guards (M4/M5/M6/M8) and the MDX rule (M7) are pinned — the vacuity check holds for the new behavior, not just the carried-over core.
6. Targeted gates (suites + lint)
- PR's own suites at head (witness
06-pr-suites-green.png): vitestqwen-pr-review-workflow.test.js92/92; node --test helpers —classify-profile.test.mjs14/14,classify-pr-profile.test.mjs6/6,upsert-bot-comment.test.mjs10/10,qwen-triage-workflow.test.mjs56/56 (86/86 total). Both new helper test files are registered in ci.ymlHELPER_TESTSand consumed by both Test-workflow paths. - bash -n: 8/8 clean — the two new
.shscripts plus all five embeddedrun:blocks the PR added/edited (context, Run review, relay, supersede, ci.yml classify fragment), extracted via a YAML parser with${{ }}expressions substituted. - actionlint 1.7.12 (repo's exact flags, whole repo): clean, exit 0 (witness
07-lint-gates.png). - shellcheck 0.11.0 (repo's exact flags): the two new scripts are clean under default checks; under the repo's opt-in
--enable=allthey emit 4 SC2292 style notes ([ ]vs[[ ]]), matching 182 pre-existing on base — house style, not a regression. Repo-wide delta: base 2425 → head 2429 findings (+4, only the new scripts). The effective CI shellcheck gate exits 0 on both arms (lint.js pipesshellcheck | sedwithout pipefail, masking the exit code). - Liveness (6/6): each gate proven live before its clean result was cited — bash -n catches a planted unclosed
if, shellcheck catches a planted unused variable (SC2034), actionlint catches a planted malformed expression. - ESLint: the repo gate targets only
.ts,.tsx(this PR adds none); direct eslint on the changed.js/.mjsfiles is clean (exit 0). - yamllint: could not run — see §8 Not covered.
7. Findings
Finding 1 — two Prettier formatting violations introduced by this PR (severity: low, cosmetic, non-behavioral). Base is prettier-clean on both files; this PR's added lines are not. Reproduce:
npx prettier --check .github/scripts/ci/classify-profile.test.mjs .github/workflows/qwen-code-pr-review.yml
# → [warn] on both (exit 1); base versions pass
.github/scripts/ci/classify-profile.test.mjs:33— the new MDX test line is 82 chars (> the documented 80-charprintWidth)..github/workflows/qwen-code-pr-review.yml:1302— the newCOMPLETION_LINEenv uses a double-quoted YAML scalar with\"escapes; the repo style is single-quoted.
Context: these extensions are not covered by the repo's commit-time hook (lint-staged runs prettier only on *.{js,jsx,ts,tsx} and *.{json,md}, not .mjs/.yml), and the CI Prettier step runs prettier --write . without a dirty-tree check, so neither gate fails — which is how it slipped through. Existing base .mjs/workflow .yml files are all prettier-clean, so this deviates from the repo's de-facto standard and documented conventions.
Measured minimal fix
npx prettier --write on the two files (measured on a scratch copy with --ignore-path /dev/null): prettier --check then passes; the mjs diff is wrap-only (line 33 → 4 lines) and the yml diff is quote-only (line 1302). Confirmed behavior-neutral: the parsed COMPLETION_LINE value is byte-identical before/after, and all four gate steps survive the reformat; the .mjs change is whitespace-only inside one expression.
No behavioral findings. The central claim, fail-open direction, head-binding, MDX classification, and badge lifecycle are all proven correct.
8. Not covered
- Per-commit attribution: the depth-2 merge-ref checkout reaches only the merge/base/head OIDs (
git rev-list HEAD^1..HEAD^2= 1; round-6 commit6196fd9and prior heade3a20ecare not reachable). The aggregateHEAD^1..HEADdiff was verified, and round-6 behaviors were verified behaviorally by targeted probes (§4, M4–M8), but not separable commit-by-commit. - yamllint: not installable in this container (
pip3permission denied;scripts/lint.js --setupfails on it) — an environment fault, not a code issue. Structural YAML validity is partially compensated by actionlint and by every harness parsing the workflows with theyamlpackage; the author's test plan reports yamllint 1.35.1 clean (author's claim, not measured here). - Replay calibration: no GitHub token and no retrievable real emitted relay comment, so the harnesses execute the extracted bash and its wire calls but are uncalibrated against an actual production run's output. A real first relay posting remains unobserved until the lane runs.
- End-to-end medium review behavior (wall-clock inside the halved budget; the model actually emitting the completion line): depends on merged 8642/7855 mechanics, not this diff.
- Live
gh api --paginatesemantics: stubs served API-shaped fixtures through real jq with the wrapper's own--jq; the real endpoint's page-concatenation was not exercised. - ESLint via the repo gate: the gate targets only
.ts,.tsx(this PR adds none). I ran eslint directly on the changed.js/.mjsfiles (clean, exit 0), but that is outside the repo's gate scope.
9. Methodology
One container (the lane's own node:22-bookworm runtime; node v22.23.2, jq 1.6, gh present but unauthenticated — unused). Working tree = refs/pull/8648/merge (ef469eb4de); base control = scratch worktree at HEAD^1 (d6b4ac8e34). No npm install/build was needed for the A/B: the diff touches only workflow YAML, shell scripts, and test files (dependency-tree delta verified empty). Harnesses in harness/ parse the workflow YAML with the yaml package, slice the real run: blocks, and execute them under bash with stubbed executables (gh, qwen, sleep, the classifier wrapper) that record every invocation to a calls file; assertions read the recorded wire and the step's output files, never the step's self-report. The autofix filter and completion-line allowlist were executed under real jq/grep. Mutations were applied as exact single-occurrence string edits and restored (git diff --quiet verified after each; post-mutation suite green). Lint tools are the repo's pinned versions (actionlint 1.7.12, shellcheck 0.11.0 via scripts/lint.js --setup), run with the repo's exact flags, and each gate was proven live with a planted violation before its clean result was cited. Raw per-assertion results live in harness/*.results.json; suite logs in logs/; the prettier fix measurement in scratch-fix/.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — PR #8648 (no action needed)This round's feedback contained no actionable items, so no changes were made and nothing was committed. What was in scope, and how each item was handled:
中文说明Autofix 评审轮次 —— PR #8648（无需改动）本轮反馈中没有任何需要处理的项目，因此未做任何改动，也没有提交。 各项在 scope 内的内容及其处理方式：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Re-reviewed the new push (my earlier approval was stale-dismissed by it). The delta tightens rather than loosens: (1) MDX is now excluded from docs_only — the classifier matches docs/**.md only, because MDX pages are executable (imported components, expressions) with a runtime/build failure surface the medium downgrade must not skip; root-level prose files (README/CHANGELOG/LICENSE etc., .md/.txt/.rst) are the other docs_only source; both changes make classification more conservative. (2) The new docs-only outcome relay binds the write to the reviewed head — it re-reads live PR state + headRefOid immediately before the mutation and skips if the PR is no longer OPEN or the head moved, which closes the 'classification not bound to the reviewed SHA' gap I flagged before. (3) A badge-correction path covers a FAILED docs-only review (the relay only runs on success, so otherwise the badge would keep quoting the previous revision's outcome). All three are safety improvements; the original analysis (every ambiguity falls back to full, default-branch checkout so the PR can't tamper with the classifier, --comment forces high) still holds. No P0/P1.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
…wenLM#8720) The review workflow has been invalid since QwenLM#8648 merged, so every event it declares has been dead for ~12 hours: Invalid workflow file: .github/workflows/qwen-code-pr-review.yml#L1 (Line: 751, Col: 14): Exceeded max expression length 21000 A `run:` body containing `${{ }}` is evaluated as ONE expression template, and GitHub caps a single expression at 21000 characters. "Run review" went 17705 -> 22282 chars in QwenLM#8648 (17:00:32 on 2026-08-07); the first startup failure is stamped 17:00:50. QwenLM#8683 took it to 24042. An over-limit expression does not fail a job — it invalidates the whole file, so no run is created at all. Across the 400 runs since that merge there is not one success, not one `pull_request_target` and not one `issue_comment`: both automatic review and `@qwen-code /review` were unreachable, while CI stayed green throughout because nothing covered it. Pass the three context values the script reads through the step's env, leaving the body free of `${{ }}`. The runner then never templates it and its length stops mattering. No behaviour changes: each substitution is a rename of the same value. Pin it with a test that walks every workflow and fails any templated run block over the limit, plus one that keeps this body untemplated — it is past 21000 on its own, so a single `${{ }}` added back takes the whole workflow down again. Both fail against the file currently on main, naming it: `qwen-code-pr-review.yml > review-pr > Run review: 24042 chars`. Mutation-tested, 4 of 4 caught: restoring a `${{ }}` in the body (3 tests), dropping either env binding, and hardcoding the value the env used to carry. Co-authored-by: verify <verify@local>
A comment of `@qwen-code /review` plus a newline and a body has never
triggered anything. The shape match tried to accept it with
startsWith(body, format('@qwen-code /review{0}', '\n'))
but GitHub expression string literals are NOT escape-processed: that
'\n' is a literal backslash + n, so the branch matched nothing. The
command was silently ignored — no run, no feedback, in a path whose
whole job is to be the manual escape hatch.
Measured on a live runner rather than assumed:
startsWith(<LF body>, format(…, '\n')) => false
startsWith(<LF body>, format(…, fromJSON('"\n"'))) => true
startsWith(<CRLF body>, format(…, fromJSON('"\n"'))) => false
startsWith(<CRLF body>, format(…, fromJSON('"\r"'))) => true
fromJSON parses JSON, which IS escape-processed, so it yields a real
newline. Both endings are needed: the REST API sends LF, the web UI
sends CRLF, and an LF pattern does not match a CRLF body. Applied to all
7 shape matches (6 /review, 1 /resolve).
The shell half had the matching gap: the command line is taken as
everything before the first LF, which on CRLF keeps a trailing CR. IFS
has no CR, so word splitting produced tokens like `--timeout=300<CR>`
that failed the numeric check with no visible cause. Strip it.
Three tests pin this: no shape match may use a non-escape-processed
literal, every shape match must carry both endings, and the CR strip
must follow the first-line split. All three fail against the workflow on
main. Mutation-tested: dropping the CR branches or the CR strip each
fails exactly its own test.
Found while re-triggering the PRs stranded by the QwenLM#8648 outage — 19
multi-line trigger comments were accepted by `authorize`, then silently
dropped by this branch.
Co-authored-by: verify <verify@local>
|
Released in v0.21.8. |
What this PR does
Adds a deterministic gate to the PR-review workflow: an automatic review of a docs-only PR runs at
--effort mediuminstead of high. Everything else is untouched — every explicit request (workflow_dispatch,@qwen-code /reviewin any comment form) keeps the full high-effort review, and any PR touching anything beyond pure documentation is never downgraded.Mechanics:
.github/scripts/ci/classify-profile.mjs— the same classifier the Test workflow's CI-profile gate already trusts, conservative by construction: onlydocs/**.md(x)and root-level prose files (README, CHANGELOG, …) classifydocs_only; markdown under anysrc/tree staysfull, which matches the review skill's own "markdown inside a source tree counts as source" rule (a PR editing the bundled review SKILL.md keeps its full review — probed explicitly). A failed file listing or classifier run falls back to the full review with a warning.--commentis dropped together with the downgrade, because an effective--commentforces effort back to high (parse-args) and medium never posts inline comments. In its place, a newReport docs-only medium outcomestep posts one PR comment relaying the review CLI's verbatimReview complete:line — the machine-readable completion contract that exists precisely for CI wrappers — plus a pointer for requesting the full high-effort review. The workflow asserts nothing the run did not print.pull_request_targetsets the newauto_reviewflag; the trigger-command paths never do.Why it's needed
The measured baseline (33 completed CI reviews) shows review time barely correlates with PR size: a 1-line docs PR burned 57 minutes of high-effort review. On a diff with zero source lines, the two passes medium drops — the adversarial personas and the reverse audit — have no failure mode to hunt: no control flow, no state, no injection surface, no callers.
The gate is deliberately this narrow because the counterfactual analysis over six dissected CI runs killed the broader version: a "small PR → medium" rule (≤300 changed lines) would have missed a probe-confirmed Critical that only the reverse audit surfaced on a 254-line source PR. Docs-only is the subset where that failure mode is structurally absent, and the conservative classifier keeps every borderline case (renames, source-tree markdown, mixed diffs) on the full path.
Reviewer Test Plan
How to verify
yamllint .github/workflows/qwen-code-pr-review.yml— clean.node --test .github/scripts/qwen-triage-workflow.test.mjs— the workflow-pinning suite passes (the ownership-recovery text this suite pins is untouched).{"filename":"docs/users/foo.md"}+{"filename":"README.md"}→docs_only;{"filename":"packages/core/src/skills/bundled/review/SKILL.md"}→full.bash -non all three edited embedded scripts (context, Run review, the new relay step) — clean.PROMPT="/review <url> --effort medium"; the--effortflag threads deterministically since the resolved-effort recovery landed (resolveEffort, PR 7855), and the medium roster/coverage/compose contract readsplan.effort(PR 8642's base mechanics).Evidence (Before & After)
Before: docs-only PRs get the full high pipeline (57–180 min measured). After: the same PRs run medium (expected ~25–60 min) with a relayed completion line on the PR; a maintainer who wants more comments
@qwen-code /reviewand gets the unchanged full review.Tested on
Environment (optional)
yamllint 1.35.1, node --test, python-yaml script extraction +
bash -n.Risk & Scope
docs/(e.g. a script referenced by CI) still gets medium's full finder fan-out including security and build gates; only the personas and the reverse audit are dropped.Linked Issues
Follow-up to the /review latency work in 8642 (merged).
中文说明
这个 PR 做了什么
给 PR-review workflow 加一个确定性门：自动触发的评审若目标是纯文档 PR，以
--effort medium运行。其余一切不变——所有显式请求（workflow_dispatch、任意形式的@qwen-code /review评论）都保持完整 high 评审；只要 diff 触及任何非纯文档文件，永不降档。机制：
.github/scripts/ci/classify-profile.mjs——Test workflow 的 CI-profile 门已在用的同一个分类器，构造上保守：只有docs/**.md(x)与根级 prose 文件（README、CHANGELOG 等）判为docs_only；任何src/树下的 markdown 保持full，与 review skill 自己"源码树内 markdown 算 source"的规则一致（改 bundled review SKILL.md 的 PR 仍走完整评审，已显式探针验证）。文件列表拉取或分类器失败一律回退完整评审并告警。--comment：有效的--comment会把 effort 强制回 high（parse-args），且 medium 永不发行内评论。取而代之，新增的Report docs-only medium outcome步骤在 PR 上发一条评论，逐字转发 review CLI 的Review complete:行——正是为 CI wrapper 设计的机器可读完成契约——并附上请求完整评审的入口。workflow 不断言任何 run 没打印过的内容。pull_request_target会置新的auto_review标志；命令触发路径永不置位。为什么需要
33 个已完成 CI 评审的基线显示评审耗时与 PR 大小几乎无关：1 行的 docs PR 烧了 57 分钟 high 评审。零源码行的 diff 上，medium 砍掉的两类 pass——对抗人格与反向审计——没有可猎杀的失效模式：没有控制流、没有状态、没有注入面、没有调用方。
门收得这么窄是有实测依据的：对 6 个解剖过的 CI run 做反事实推演，更宽的"小 PR → medium"版本（≤300 变更行）会漏掉一个只有反向审计才挖出的、probe 实证的 Critical——它发生在一个 254 行的源码 PR 上。纯文档是该失效模式结构性缺席的子集，保守分类器把一切边界情形（重命名、源码树 markdown、混合 diff）都留在完整路径上。
验证
yamllint干净；node --test .github/scripts/qwen-triage-workflow.test.mjs通过（其钉住的 ownership-recovery 文本未动）；分类器探针：docs/users/foo.md+README.md→docs_only，packages/core/src/skills/bundled/review/SKILL.md→full；三段内嵌脚本bash -n干净。--effort标志的确定性传导依赖已合入的 resolveEffort（PR 7855）与 plan.effort 契约（PR 8642）。风险与范围
@qwen-code /review取完整评审。docs/夹带可执行内容的 PR 仍会经过 medium 的完整 finder 扇出（含安全与构建门），只少人格与反审。关联
/review 延迟工作（8642，已合入）的后续。