fix(ci): render the queued-acknowledgement comment - #8726
Conversation
The ack comment posted on every PR that requests a review was built as <!-- qwen-review-ack -->_Qwen Code review request accepted. …[workflow run](URL)._ with the prose glued straight onto the marker. A line opening with `<!--` starts an HTML block, and that block runs to the line containing the closing delimiter INCLUSIVE — the rest of that line is still inside it and never parsed as Markdown. So the comment shipped as raw source: no emphasis, and the link to the workflow run was dead text. That link is the only pointer a PR author gets to their review run — `issue_comment` runs are not attached to a commit, so they never appear in the PR's checks list. Losing it leaves no way to reach the run from the PR. Measured through GitHub's own renderer (POST /markdown, mode=gfm) on the exact bodies: marker + text -> 0 <a>, 0 <em> marker + \n + text -> 1 <a>, 1 <em> marker + \n\n + text -> 1 <a>, 1 <em> Use the blank-line form, matching how autofix-status already builds its body. The marker text is unchanged, so the `contains(...)` upsert lookup still finds prior acks and updates them in place. Pinned by a test that scans every marker in the workflow and rejects one with prose glued to it, skipping comment lines. It fails against main, naming the offending line.
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. |
|
Re-running the gate on the new head — two PR commits landed since the last pass (the marker-guard regex widening with declared gaps, and the ack link-shape pin with the deduped scan), plus a merge from main.
Moving on to code review. 🔍 中文说明对最新 head 重新过门——距上次通过以来新增了两个 PR 提交（放宽标记守卫正则并如实声明缺口、钉住 ack 链接形状并去重扫描），外加一次来自 main 的合并。
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. The HTML-block diagnosis is right — a line opening with <!-- swallows the rest of that same line, so the glued prose shipped as raw source with a dead link, and the blank-line fix is the minimal correct one. Since the marker substring itself is unchanged, the existing-ack lookup still matches old comments too. The regression test sensibly allows the escaped-newline and string-terminator cases while catching any future glue. CI green on this head.
|
Qwen Code review timed out. Qwen review timed out after 10800 seconds (of the 180-minute budget). For large PRs, retry with a longer timeout by commenting: |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
.github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
.github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— deepseek-v4-flash via Qwen Code /review (v0.21.7)
Resolves the conflict with #8723 (both PRs appended a describe to the same test file) and addresses the four review findings. R1-1 (Critical) — the guard never inspected the line it protects. The regex `<!--[^>]*-->` matched across newlines, so the `<!--` inside this PR's own explanatory comment opened a match that ran to the ACK_BODY marker's `-->`; the comment-line skip then discarded the whole span. Verified: with the comment retained and ACK_BODY reverted to the glued form, the old guard passed. `[^>\n]` confines a match to one line. R1-2 — the `\n` exemption was unconditional, but bash only expands `\n` in printf's format string or ANSI-C `$'…'`. In a plain double-quoted assignment it is a literal backslash-n, so the prose stays on the marker's physical line and still breaks. The exemption is now gated on the quoting form. R1-3 — the `"`/`'`/`)` whitelist treated "the marker closes its literal" as proof nothing follows. It now reads the remainder of the literal and, when that is empty, an adjacent literal concatenated onto it. R1-4 — the guard scanned one file; it now walks every workflow. Zero offenders repo-wide, so it starts clean rather than with a backlog. Rewritten around one question — does this marker OPEN a string literal, and what remains inside it — which is what separates BUILDING a comment body from REFERENCING a marker. That distinction is why `jq contains("<!-- m -->")` and `printf '<!-- m -->' "$VAR"` stay silent: 51 false positives under the first cut, 0 under this one. Known gap, documented in the test rather than papered over: a body split across printf arguments is not caught. Modelling which literal is the format string is required, and every cheap approximation flagged the legitimate `printf '<!-- m -->' "$VAR"` form. Mutation-tested against the review's own scenarios, 3 of 3 caught: the glued form with the comment block retained (which the old guard passed), the double-quoted `\n` form, and adjacent-string concatenation.
|
@qwen-code /triage |
|
Sandboxed verification: The verification run did not complete, so the phases below may be partial or missing entirely. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 40 passed · 0 failed · 40 total 中文 — 判定：
|
| cell | body reaching the wire | GitHub POST /markdown (gfm) |
marked | markdown-it |
|---|---|---|---|---|
base (HEAD^1) |
<!\-\- qwen-review-ack -->_Qwen Code … [workflow run](URL)._ — 1 line |
0 <a>, 0 <em> — plain text, dead link |
0/0 | 0/0 |
head (HEAD^2) |
marker + \n\n + prose — 3 lines |
1 <a>, 1 <em>, live href |
1/1 | 1/1 |
single-\n variant (PR table row 2) |
— | 1 <a>, 1 <em> |
— | — |
The head render contains the exact After HTML the PR quotes, byte-for-byte:
<p dir="auto"><em>Qwen Code review request accepted. Review is queued in <a href="https://github.com/QwenLM/qwen-code/actions/runs/31243224971">workflow run</a>.</em></p>.
Assertions A1–A15 (15/15) in 01-ab-body.mjs; witness: 01-ab-base-vs-head-github-render.png.
Secondary claim — upsert compatibility (marker byte-identical ⇒ prior acks are PATCHed, not duplicated): the jq filter was extracted verbatim from the head run block and driven through the real jq binary with fixture comment pages. Old glued-form ack alone → selected for PATCH (id returned); mixed page → latest bot ack; human-authored marker comments → never selected; paginated pages → slurped. The filter itself is byte-identical between base and head. B1–B7 (7/7) in 02-upsert.mjs; witness: 02-upsert-jq-cells.png.
Secondary claim — the new guard test holds down what it claims (commit 2 hardened it per review round 1): each cell ran the real test file via vitest against a fixture tree of all 47 workflows with one mutated copy:
| cell | mutation | expected | observed |
|---|---|---|---|
| C0 head control | none | green | green (1 passed) |
| C1 base | base workflow (reverted hunk) | red, names offender | red: qwen-code-pr-review.yml: <!\-\- qwen-review-ack -->_Qwen Code review request accept |
| M1 (R1-1) | glued body re-introduced, PR's comment block kept | red | red |
| M1b | M1 workflow + OLD [^>] regex, isolated file |
green (regression survives) | green — old guard misses it |
| M1c | OLD [^>] regex over ALL unmutated workflows |
red (false positive) | red — flags legit qwen-autofix.yml marker |
| M2 (R1-2) | ACK_BODY="<!\-\- m -->\n\n_prose" (double quotes: literal \n) |
red | red |
| M3 (R1-3) | adjacent-literal '<!\-\- m -->''_prose_' |
red | red |
| M4 | printf-argument split (documented known gap) | green (not caught) | green |
| M5 | legit printf '<!\-\- m -->' "$VAR" reference |
green (no FP) | green |
| M6 (R1-4) | glued marker planted in qwen-triage.yml |
red naming that file | red, names qwen-triage.yml |
G1–G10 (10/10) in 03-guard-matrix.mjs (+ per-cell vitest logs); witness: 03-guard-mutation-matrix.png. The vacuity check is C1: the test fails against base at the intended assertion (expect(offenders).toEqual([]) diff with the named line), and the positive control is C0/M1 — the harness provably goes red on the regression and green only when the guard is correct.
Corrections
Two statements in the PR description do not match measured reality; neither changes the code:
- Failure-snippet quote. The description shows the guard failing on
mainwith"<!\-\- qwen-review-ack -->_Qwen Code review request accepted. Review is qu". The actual vitest diff prints"qwen-code-pr-review.yml: <!\-\- qwen-review-ack -->_Qwen Code review request accept"— the snippet is 56 characters from the marker (24 chars), so it ends ataccept, not mid-accepted. The test does fail on base and does name the offending line, as claimed; only the quoted truncation is off. - Test count 101/101. That is the pre-merge count: the merge with
main(fix(ci): match /review commands followed by a newline #8723) brought threecommand shape matchingtests into the same file (101 + 3 = 104, consistent with the commit's conflict-resolution note), so the post-merge file holds 104 tests — 104/104 pass (gate H1). Same for the suite total: see gates.
Findings
No blocking findings. Two informational notes:
- (info) Known gap is real and honestly documented. The body-split-across-printf-arguments shape (
printf '%s%s' '<!\-\- m -->' 'prose') escapes the guard — confirmed by M4 going green. The test comment states this gap rather than papering over it; no action required. Completeness reporting only. - (info) The old regex was worse than the review round recorded. M1c shows the pre-hardening
[^>]pattern, when combined with the all-workflows walk, false-positives on the legitimate<!\-\- autofix-eval … -->marker inqwen-autofix.ymleven with no regression present — in addition to missing the R1-1 regression (M1b). Extra evidence the[^>\n]hardening is load-bearing; nothing to fix.
Targeted gates
| gate | result |
|---|---|
scripts/tests/qwen-pr-review-workflow.test.js at head |
104/104 (H1; witness 04-targeted-gates.png) |
.github/scripts/qwen-triage-workflow.test.mjs via node --test |
56/56 — the PR's exact claim (H2) |
full scripts suite (npm run test:scripts) |
949/949 executed tests pass, 49/50 files; install-script.test.js uncollected: its module-scope guard throws when zip is missing on a CI host, and this container ships no zip. The file is byte-identical between base and head (empty diff) and the guard runs before any test logic → environmental, proven not a regression (H3) |
| actionlint 1.7.12, repo flags, whole head tree | 0 findings (H4); base version of the changed file also 0 (H5) |
bash -n on both extracted ack steps |
clean (H6) |
| shellcheck 0.11.0, repo flags, both extracted steps | identical finding classes on both arms (SC2148 no-shebang, SC2154 for the four env:-supplied vars) — zero new classes on head (H7) |
| marker-writer census outside the guard's scope | 0 writers in .github/actions/ or scripts/ (readers only; composite actions post no comments) (H8) |
Not covered
- Per-commit attribution: the depth-2 checkout leaves only 1 of the 2 PR commits reachable (
git rev-list HEAD^1..HEAD^2= 1 vs 2 in the metadata snapshot — shallow boundary). The aggregateHEAD^1..HEADdiff was verified; per-commit separation was out of reach. The mutation matrix covers commit 2's claims behaviorally regardless. - yamllint: the repo wrapper installs it via
pip3, which this container does not ship. actionlint + shellcheck + bash -n all ran. - The 105 tests inside
install-script.test.js: not collectible here (nozipbinary, uid 1000, no apt access). Proven environmental above; the PR's own CI (ubuntu-latestshipszip) exercises them. - Live comment round-trip (
ghPOST/PATCH → storedbody_html): no GitHub token exists in this environment by design; the A/B uses the same renderer endpoint the author measured with and reproduces the wire shape — the rendering half of the pipeline — not the comment-storage half. The upsert harness covers the lookup half against realjq. - Windows/macOS: N/A — the change is a Linux-bash CI step, matching the PR's Tested-on table.
POST /markdownrate-limit resilience: 7 anonymous calls succeeded; a throttled rerun of the A/B would need a token.
Methodology
Environment: the CI verify container (node:22-bookworm, uid 1000, node v22.23.2, jq present, no zip/pip3; POST api.github.com/markdown anonymously reachable — established by probe, used as the primary rendering oracle). The base arm came from git show HEAD^1:.github/workflows/qwen-code-pr-review.yml parsed with the repo's yaml package; both run blocks executed under bash --noprofile --norc with a stub gh on PATH capturing argv payloads at the wire boundary. Rendering cells: GitHub POST /markdown (mode=gfm, repo context) corroborated by marked 15.0.12 and markdown-it 14.2.0 from the repo's own node_modules. The guard matrix ran the real test file via the repo's vitest config in per-cell fixture trees under tmp/pr8726-verify-…/fixtures/ (module resolution walks up to the root node_modules; the test reads only .github/workflows relative to cwd). Harnesses, per-cell vitest logs, gate logs, and raw captures live in tmp/pr8726-verify-20260808-160715/ (01-ab-body.mjs, 02-upsert.mjs, 03-guard-matrix.mjs, 04-gates.mjs, 03-cell-*.log, gate-*.log, evidence/*.png).
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: reverse audit — stopped before round 4 by the review time budget. 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.
中文说明
已审查。 建议见行内评论。 未审查：反向审计——评审时间预算不足，未能开始第 4 轮。 未检查（工具限制，非阻断）：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)
| # containing the closing delimiter, and the REST of that line stays | ||
| # inside it — so gluing the text on shipped it as raw source with a | ||
| # dead link. Verified against GitHub's own renderer. | ||
| ACK_BODY="$(printf '<!-- qwen-review-ack -->\n\n_Qwen Code review request accepted. Review is queued in [workflow run](%s)._' "$RUN_URL")" |
There was a problem hiding this comment.
[Suggestion] No test pins the %s → "$RUN_URL" weaving on this changed line — the new regression test constrains only the marker/body separator. Failure scenario: a future edit drops %s (or the argument) → bash printf with a leftover argument and no conversion spec exits 0 under set -euo pipefail and emits [workflow run]() → the ack comment ships with an empty workflow-run link while the whole suite stays green (verified in bash at this commit). Because issue_comment-triggered reviews never appear in the PR's checks list, the author silently loses the only pointer to their review run — the exact defect this PR fixes, reintroduced with no red test. Suggested fix: assert in this file's source-level style that the printf carrying the marker contains %s and is passed "$RUN_URL" (stronger: execute the ack script with a stubbed gh and assert the captured body contains the marker, a blank line, and the run URL).
中文说明
本行改动中 %s → "$RUN_URL" 的编织没有任何测试固定——新增的回归测试只约束了标记与正文之间的分隔。失败场景：未来的编辑删掉 %s（或参数）→ bash printf 带着多余参数且无转换说明符，在 set -euo pipefail 下仍以 0 退出并输出 [workflow run]() → ack 评论带着空的 workflow-run 链接发出，而整个套件保持全绿（已在本提交的 bash 中验证）。由于 issue_comment 触发的评审从不出现在 PR 的 checks 列表中，作者会悄然失去指向其评审 run 的唯一入口——这正是本 PR 修复的缺陷，且无测试变红地被重新引入。建议修复：以本文件现有的源码级断言风格，断言承载标记的 printf 含有 %s 且传入了 "$RUN_URL"（更强：用打桩的 gh 执行 ack 脚本，断言捕获的 body 含标记、空行与 run URL）。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| // `\n`: printf's format string, or ANSI-C `$'…'`. In a plain | ||
| // double-quoted assignment `\n` is a literal backslash-n, so the | ||
| // prose stays on the marker's physical line and still breaks. | ||
| if (glued.startsWith('\\n') && /printf\s+'|\$'/.test(prefix)) continue; |
There was a problem hiding this comment.
[Suggestion] The \n exemption searches the whole line prefix for printf ' or $' instead of anchoring to the literal the marker opens — an unrelated printf ' earlier on the same physical line blesses a glued plain double-quoted assignment. Failure scenario: printf '%s' x; ACK_BODY="<!-- qwen-review-ack -->\n_prose_" → offenders=[] even though bash keeps \n as literal backslash-n in a plain double-quoted assignment, so the posted body is one physical line starting <!-- — precisely the raw-source/dead-link rendering this test exists to prevent — passing green (confirmed by running the guard verbatim; bash od confirms \n stays two literal chars). Suggested fix: anchor the exemption to the prefix tail — if (glued.startsWith('\\n') && /printf\s+'$|\$'$/.test(prefix)) continue; — verified that the repro then flags and the real workflow dir still scans clean. Note: this line also carries two sibling gaps (double-quoted printf formats; line-wrapped printfs) — see the adjacent comments; the fixes compose.
中文说明
\n 豁免在整个行前缀中搜索 printf ' 或 $'，而不是锚定到标记所打开的字面量——同一物理行靠前位置的无关 printf ' 会为一个粘接的普通双引号赋值放行。失败场景：printf '%s' x; ACK_BODY="<!-- qwen-review-ack -->\n_prose_" → offenders=[]，尽管 bash 在普通双引号赋值中把 \n 保持为字面的反斜杠-n，发出的 body 因此是以 <!-- 开头的单一物理行——正是本测试要防止的原始文本/死链接渲染——却全绿通过（已用原样守卫运行确认；bash od 确认 \n 保持为两个字符）。建议修复：把豁免锚定到前缀末尾——if (glued.startsWith('\\n') && /printf\s+'$|\$'$/.test(prefix)) continue;——已验证复现场景随后会被标记，且真实 workflows 目录仍无违规。注意：本行还有两个同族缺口（双引号 printf 格式串；换行 printf）——见相邻评论，修复可以组合。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| const prefix = text.slice(lineStart, m.index); | ||
| // Prose in a YAML or shell comment never reaches a comment body. | ||
| if (/^\s*#/.test(prefix)) continue; | ||
| const quote = m.index === lineStart ? null : text[m.index - 1]; |
There was a problem hiding this comment.
[Suggestion] Markers at physical line start are skipped unconditionally (quote === null), which puts heredoc- and YAML-block-scalar-built comment bodies outside the guard. Failure scenario: moving the ack body into $(cat <<'BODY' … <!-- qwen-review-ack -->_Qwen Code… BODY) → offenders=[] even though GitHub renders that line as raw source with a dead link (confirmed by running the guard verbatim). Heredocs are already a common multi-line idiom in this repo's workflows (11 <<' uses across 6 files), so a routine refactor resurrects this PR's exact regression with the guard green. The known-gap comment declares the printf-arguments gap but not this one. Suggested fix: extend the scan to line-start markers inside heredocs opened in a run: block, or declare this gap in the same comment block so the test name does not promise coverage the rule does not provide.
中文说明
物理行首的标记被无条件跳过（quote === null），这使得 heredoc / YAML 块标量构造的评论 body 完全落在守卫之外。失败场景：把 ack body 移入 $(cat <<'BODY' … <!-- qwen-review-ack -->_Qwen Code… BODY) → offenders=[]，尽管 GitHub 会把该行渲染为带死链接的原始文本（已用原样守卫运行确认）。heredoc 已是本仓库 workflows 中常见的多行写法（6 个文件 11 处 <<'），一次常规重构就会让本 PR 修复的回归在守卫全绿时复活。已知缺口注释声明了 printf 参数拆分缺口，但没有声明这一个。建议修复：把扫描扩展到 run: 块中 heredoc 里的行首标记，或在同一注释块中声明该缺口，使测试名称不承诺规则并未提供的覆盖。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| // `\n`: printf's format string, or ANSI-C `$'…'`. In a plain | ||
| // double-quoted assignment `\n` is a literal backslash-n, so the | ||
| // prose stays on the marker's physical line and still breaks. | ||
| if (glued.startsWith('\\n') && /printf\s+'|\$'/.test(prefix)) continue; |
There was a problem hiding this comment.
[Suggestion] The exemption regex /printf\s+'|\$'/ accepts only single-quoted printf formats, but printf expands \n in its format string regardless of shell quoting — the comment above documents the broader rule. Failure scenario: BODY="$(printf "<!-- m -->\n\n_prose_")" renders correctly, yet the guard fails CI because the prefix matches neither alternative (confirmed by running the guard verbatim; no such line exists today, so this is a false-positive mine for a future edit). Note: this line also carries the unanchored-prefix gap (see the separate comment); together the exemption wants both quote-style widening and tail-anchoring — e.g. /printf\s+['"]$|\$'$/.
中文说明
豁免正则 /printf\s+'|\$'/ 只接受单引号的 printf 格式串，但 printf 展开格式串中的 \n 与 shell 引号无关——上方注释写的是更宽的规则。失败场景：BODY="$(printf "<!-- m -->\n\n_prose_")" 渲染正确，守卫却让 CI 失败，因为前缀两个分支都不匹配（已用原样守卫运行确认；目前不存在这样的行，属于面向未来编辑的误报地雷）。注意：本行还存在前缀未锚定的缺口（见另一条评论）；合并来看，豁免既需要放宽引号形式、也需要锚定到末尾——例如 /printf\s+['"]$|\$'$/。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| // (POST /markdown, mode=gfm): marker+text -> 0 <a>/0 <em>; marker+"\n"+text | ||
| // and marker+"\n\n"+text -> 1 <a>/1 <em>. | ||
| // | ||
| // The rule keys off ONE thing: a marker that opens a string literal, and |
There was a problem hiding this comment.
[Suggestion] The "marker opens a string literal" rule misses the same bug class when the marker sits mid-literal but at the start of a rendered line. Failure scenario: BODY="$(printf 'Thanks for the review!\n\n<!-- qwen-review-ack -->_queued._')" → offenders=[] because the char before <!-- is a space, yet after printf expansion the marker lands at line start with glued prose — the exact defect this PR fixes, shipped with the regression test green (confirmed by running the guard verbatim; no current workflow has this shape, so this is a guard-coverage gap). Suggested fix: state this gap in the known-gap comment, or walk backward to the nearest literal-internal \n (in printf/$' contexts) and flag glued prose when the marker starts a rendered line.
中文说明
"标记打开字符串字面量"规则会漏掉同一缺陷类：标记位于字面量中间、却落在渲染行的行首。失败场景：BODY="$(printf 'Thanks for the review!\n\n<!-- qwen-review-ack -->_queued._')" → offenders=[]，因为 <!-- 前的字符是空格；但 printf 展开后标记落在行首且正文粘接——正是本 PR 修复的缺陷，在回归测试全绿时发出（已用原样守卫运行确认；当前没有 workflow 是这种形状，属于守卫覆盖缺口）。建议修复：在已知缺口注释中声明该缺口，或回溯到字面量内最近的 \n（printf/$' 上下文中），当标记位于渲染行首时标记粘接正文。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| // Only a marker that OPENS a string literal can be building a body. | ||
| if (quote !== "'" && quote !== '"') continue; | ||
| const rest = text.slice(m.index + m[0].length); | ||
| const end = rest.indexOf(quote); |
There was a problem hiding this comment.
[Suggestion] rest.indexOf(quote) scans to end-of-file instead of end of the string/physical line, so a marker merely quoted inside prose is only un-flagged while no matching quote exists anywhere later in the file. Failure scenario: qwen-issue-followup-bot.yml:430's documentation example (--body "<!-- qwen-issue-bot:related -->) survives today only because that file (ending at line 444) happens to contain no " after line 430 — verified by awk. The moment any future edit adds a " after that line — and this file is a living LLM prompt — the test fails CI naming documentation prose as an offender even though no comment body is being built (appending one trailing line containing a " flips it — confirmed by probe). Suggested fix: bound the scan to the marker's physical line before searching for the closing quote — a glued body is by definition on the marker's own line, so no true positive is lost.
中文说明
rest.indexOf(quote) 会一直扫描到文件末尾，而不是到字符串/物理行结束，因此一个只是被引号包在文档文本里的标记，只有在文件后面任何位置都不存在配对引号时才不被标记。失败场景：qwen-issue-followup-bot.yml:430 的文档示例（--body "<!-- qwen-issue-bot:related -->）今天幸免，仅仅因为该文件（444 行结束）在 430 行之后恰好没有任何 "——已用 awk 验证。一旦未来任何编辑在该行之后加上一个 "——而这个文件是持续演进的 LLM prompt——测试就会把文档文本当成违规者让 CI 失败，尽管根本没有评论 body 在构造（在末尾追加一行包含 " 的文本即可复现——已通过探针确认）。建议修复：在搜索闭合引号前把扫描限制在标记所在的物理行——按定义粘接的 body 必在标记自己的行上，不会丢失任何真阳性。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| // `\n`: printf's format string, or ANSI-C `$'…'`. In a plain | ||
| // double-quoted assignment `\n` is a literal backslash-n, so the | ||
| // prose stays on the marker's physical line and still breaks. | ||
| if (glued.startsWith('\\n') && /printf\s+'|\$'/.test(prefix)) continue; |
There was a problem hiding this comment.
[Suggestion] The exemption only recognizes printf ' / $' on the same physical line as the marker, so a line-wrapped printf whose format opens with the marker is falsely flagged — and wrapped printfs are an established style in the very workflow this PR edits (lines 1397, 1504, 1515); wrapping this PR's own ~150-char ack line would trip it. Failure scenario: BODY="$(printf \ followed by a newline, then '<!-- m -->\n\nprose' "$X")" → the marker line's prefix is just ', the exemption misses, and correct code is reported as an offender (confirmed against the real guard: wrapped form FAILs, semantically identical one-line form PASSes; echo -e '<!-- m -->\n\nprose', which also expands \n, is likewise falsely flagged). Suggested fix: detect the format-string role instead of same-line text — scan the prefix backwards for an unmatched printf opener, or accept the exemption when the previous physical line ends in an open printf continuation.
中文说明
豁免只认与标记同一物理行上的 printf ' / $'，因此格式串以标记开头的换行 printf 会被误报——而换行 printf 正是本 PR 编辑的这个 workflow 里的既有写法（1397、1504、1515 行）；把本 PR 自己这行约 150 字符的 ack 行换行就会触发。失败场景：BODY="$(printf \ 后接换行，再接 '<!-- m -->\n\nprose' "$X")" → 标记所在行的前缀只是 '，豁免落空，正确的代码被报为违规（已用真实守卫确认：换行形式 FAIL，语义相同的一行形式 PASS；同样会展开 \n 的 echo -e '<!-- m -->\n\nprose' 也被误报）。建议修复：识别"格式串"这一角色而不是同一行文本——向前扫描前缀中未闭合的 printf 起始，或当前一物理行以未闭合的 printf 续行结尾时给予豁免。
— qwen3.8-max via Qwen Code /review (v0.21.7)
There was a problem hiding this comment.
Declined deliberately, and the limitation is now declared in the test's known-gap comment block rather than left silent.
Two reasons. First, this is a latent false-positive mine, not a coverage gap: no workflow today wraps a printf whose format string opens with a marker — the wrapped printfs in qwen-code-pr-review.yml (1397/1504/1515) all pass "$MARKER" as an argument, which the guard reads cleanly — so the rule misses nothing that exists. Second, detecting the format-string role across wrapped lines means modelling every \n-expanding context: backslash continuations, newlines inside $( ), and (as your comment notes) echo -e. That is the same modelling whose cheap approximations produced 51 false positives repo-wide in round 1; this guard's charter is to catch the glued-marker regression class at near-zero maintenance cost, not to parse shell.
The failure mode if this is ever triggered is self-exposing: CI goes red on correct code, names the exact file and line, and whoever wrapped the line can widen the exemption against the real shape instead of a guessed one. Happy to revisit the moment that shape lands.
中文说明
经过慎重考虑后拒绝，且该局限现已在测试的已知缺口注释块中明说，而非静默保留。
理由有二。其一，这是一个潜在的误报地雷，而非覆盖缺口：当前没有任何 workflow 使用格式串以标记开头的换行 printf——qwen-code-pr-review.yml 中的换行 printf（1397/1504/1515 行）都把 "$MARKER" 作为参数传入，守卫能正确读取——因此现有规则没有漏掉任何实际存在的形式。其二，跨换行识别"格式串"角色意味着要为每一个会展开 \n 的上下文建模：反斜杠续行、$( ) 内部换行，以及（如您的评论所述）echo -e。这正是第 1 轮中那些廉价近似在全仓产生 51 个误报的同类建模；本守卫的职责是以接近零的维护成本捕获"标记粘正文"这一回归类，而不是解析 shell。
即便未来真的触发，其失败模式也是自我暴露的：正确代码让 CI 变红、点名具体文件与行，届时换行该行的人可以针对真实形态（而非猜测的形态）放宽豁免。该形态一旦落地，乐意重新考虑。
| if (glued === '') { | ||
| // The literal ended at the marker — but an adjacent literal on the | ||
| // same line concatenates onto it at runtime. | ||
| const after = rest.slice(end + 1); |
There was a problem hiding this comment.
[Suggestion] The adjacent-word check treats only a following quote as runtime gluing; a marker literal immediately followed by an unquoted $(…) is silently skipped, and the gap is undeclared. Failure scenario: BODY='<!-- m -->'$(gen-prose) is valid shell in assignment context, and at runtime the substitution's output concatenates onto the marker with no newline — the exact raw-source/dead-link regression this PR fixes — yet offenders=[]; the quoted twin BODY='<!-- m -->'"$(gen-prose)" IS caught, so the miss is specific to the unquoted form (both confirmed against the real guard, plus bash od showing the glued output). Suggested fix: treat after[0] === '$' && after[1] === '(' as a suspect concatenation (flag it, or require a leading \n inside), or add this shape to the known-gap comment.
中文说明
相邻词检查只把后面跟着的引号当作运行时粘接；标记字面量后紧跟未加引号的 $(…) 会被静默跳过，且该缺口未声明。失败场景：BODY='<!-- m -->'$(gen-prose) 在赋值上下文中是合法 shell，运行时替换结果会直接粘到标记后面、没有换行——正是本 PR 修复的原始文本/死链接回归——却 offenders=[]；加了引号的孪生形式 BODY='<!-- m -->'"$(gen-prose)" 会被捕获，说明漏报专属于未加引号的形式（均已用真实守卫确认，bash od 也显示了粘接输出）。建议修复：把 after[0] === '$' && after[1] === '(' 视为可疑拼接（标记它，或要求其内部以 \n 开头），或把该形状加入已知缺口注释。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| const rest = text.slice(m.index + m[0].length); | ||
| const end = rest.indexOf(quote); | ||
| if (end === -1) continue; | ||
| let glued = rest.slice(0, end); |
There was a problem hiding this comment.
[Suggestion] A REAL newline inside a multi-line quoted string is treated as gluing: the only separation exemption tests for the two-character \n escape, although the exemption comment below states "A real newline separates them". Failure scenario: this repo's own guidance (qwen-issue-followup-bot.yml:427-432) recommends breaking --body "<!-- marker --> + blank line + prose across lines inside one double-quoted argument; a step written that way renders correctly (this PR's own measurement: marker + newline + prose → 1 <a> / 1 <em>), but glued starts with a real 0x0A, glued.startsWith('\\n') is false, and the test fails CI on a correct body (confirmed against the real guard with a probe workflow file; no current workflow uses the shape, so the trigger is latent). Suggested fix: if (glued.startsWith('\n')) continue; — note includes('\n') is too broad: it would also exempt <!-- m -->prose\nmore, where prose still rides the marker's line — or declare the multi-line shape out of scope in the known-gap comment.
中文说明
多行引号字符串里的真实换行会被当作粘接：唯一的分隔豁免只检测两字符的 \n 转义，尽管下方的豁免注释写着"真实换行会分隔它们"。失败场景：本仓库自己的指引（qwen-issue-followup-bot.yml:427-432）推荐在同一个双引号参数里跨行写 --body "<!-- 标记 --> + 空行 + 正文；这样写出的步骤渲染完全正确（本 PR 自己的实测：标记 + 换行 + 正文 → 1 个 <a> / 1 个 <em>），但 glued 以真实的 0x0A 开头，glued.startsWith('\\n') 为假，测试会让一个正确的 body 在 CI 中失败（已用真实守卫加探针 workflow 文件确认；当前没有 workflow 使用该形状，触发是潜在的）。建议修复：if (glued.startsWith('\n')) continue;——注意 includes('\n') 太宽：它会把 <!-- m -->prose\nmore 也豁免掉，而那里正文仍粘在标记那一行——或在已知缺口注释中声明多行形状不在范围内。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| // contains("<!-- m -->")` and `printf '<!-- m -->' "$VAR"` leave nothing | ||
| // after the marker and are fine, while `="<!-- m -->prose"` does not. | ||
| // | ||
| // Known gap, stated rather than papered over: a body split across printf |
There was a problem hiding this comment.
[Suggestion] The declared known-gaps omit bodies assembled across statements — the very workflow this PR fixes builds two bot comments that way ("Report skipped request" / "Report result", lines ~2076-2085 and ~2187-2235: one echo per line into a file, posted via --body-file). Failure scenario: an edit such as echo -n "<!-- qwen-resolve-result -->" followed by echo "prose" posts <!-- qwen-resolve-result -->prose — the exact regression this PR responds to — while the guard stays green, because each literal passes in isolation (confirmed with a probe file against the real guard: NOT flagged). Today's plain echo appends a newline, so both bodies render — but an echo → echo -n edit ships the dead-link regression with the guard green. Suggested fix: declare this gap alongside the existing one, matching this comment block's own "stated rather than papered over" principle: the guard models a single literal only; bodies assembled across statements or files are out of scope.
中文说明
已声明的已知缺口遗漏了跨语句拼装的 body——本 PR 修复的这个 workflow 自己就用这种方式构造两条 bot 评论（"Report skipped request" / "Report result"，约 2076-2085 与 2187-2235 行：每行一个 echo 写入文件，再经 --body-file 发出）。失败场景：诸如 echo -n "<!-- qwen-resolve-result -->" 后跟 echo "prose" 的编辑会发出 <!-- qwen-resolve-result -->prose——正是本 PR 响应的回归——而守卫保持全绿，因为每个字面量单独看都合规（已用探针文件对真实守卫确认：未被标记）。今天的普通 echo 会追加换行，两条 body 渲染正常——但一次 echo → echo -n 的修改就会让死链接回归在守卫全绿时发出。建议修复：按本注释块自己"明说而非掩盖"的原则，在既有缺口旁声明该缺口：守卫只对单个字面量建模；跨语句或跨文件拼装的 body 不在范围内。
— qwen3.8-max via Qwen Code /review (v0.21.7)
|
@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 冲突，直到移除标签或达到轮次上限。移除 |
|
🤖 AutoFix ran out of time before finishing (timeout (1080000ms)) (attempt 1/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31267686731 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix ran out of time before finishing (timeout (1080000ms)) (attempt 2/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31270826835 🧠 Handled by Qwen Code · model/模型 |
Pin the workflow-run URL weaving into the ack printf, bound the marker scan to the marker's physical line, anchor the newline exemption to the literal the marker opens, widen it to double-quoted printf formats, and flag unquoted command-substitution concatenation. Declare the remaining coverage gaps in the test instead of papering over them. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Review round 3 summary — PR #8726Ten new suggestions arrived from the automated reviewer (qwen3.8-max, round 2), plus the four round-1 findings from @wenshao's review (deepseek-v4-flash), which were already fixed in Round-1 findings — already fixed in
|
| Finding | Disposition |
|---|---|
| rc:3740711816 [Critical] guard regex spans newlines, swallowing the marker it exists to protect | Fixed earlier: /<!\-\-[^>\n]*-->/g. Re-verified in code and by mutation (glued body with the comment block retained now fails the guard). |
rc:3740711828 \n exemption unconditional |
Fixed earlier: gated on a printf/$' prefix. Re-verified by probe (plain double-quoted \n form still red). |
rc:3740711832 whitelist exemptions for "/'/) |
Fixed earlier: rule rewritten to "marker opens a string literal" plus adjacent-literal read. Re-verified by probe (adjacent-literal mutant still red). |
| rc:3740711834 guard scans only one workflow | Fixed earlier: walks every file in .github/workflows. Re-verified (zero offenders repo-wide at head). |
Round-2 findings — dispositions
| Finding | Disposition |
|---|---|
rc:3741097771 no test pins %s → "$RUN_URL" |
Implemented: new test pins the workflow-run URL into the ack printf asserts the printf carrying the marker contains %s and is passed "$RUN_URL". Mutants dropping %s, the argument, or the printf are all red (simulated). |
rc:3741097774 \n exemption unanchored over the whole prefix |
Implemented: exemption anchored to the prefix tail (/printf\s+['"]$|\$'$/). Probe printf '%s' x; ACK_BODY="<!\-\- m -->\n_prose" is now red. |
| rc:3741097775 line-start markers (heredocs, YAML block scalars) outside the guard | Implemented via the offered alternative: gap declared in the test's known-gap comment block instead of extending the scan to heredoc interiors. |
| rc:3741097777 exemption accepts only single-quoted printf formats | Implemented, composed with the anchor fix: /printf\s+['"]$|\$'$/. Probe BODY="$(printf "<!\-\- m -->\n\n_prose_")" is green (no false positive). |
rc:3741097778 marker mid-literal landing at a rendered line start after \n expansion |
Implemented via the offered alternative: gap declared in the known-gap comment block. |
| rc:3741097781 closing-quote scan runs to end-of-file | Implemented: the scan is bounded to the marker's physical line. Probe with a later " elsewhere in the file is green; the qwen-issue-followup-bot.yml:430 doc example can no longer flip on an unrelated future quote. |
| rc:3741097783 line-wrapped printf whose format opens with the marker | Declined — see the reply on its thread; the limitation is declared in the known-gap comment. |
rc:3741097785 unquoted $(…) after a marker literal silently skipped |
Implemented: BODY='<!\-\- m -->'$(gen-prose) is now flagged (its output is invisible to a static scan). Verified no current workflow uses the shape, so the repo scan stays clean. |
| rc:3741097787 real newline in a multi-line literal treated as gluing | Resolved by the bounded scan (rc:3741097781): glued can no longer contain a real newline, and a multi-line literal where the marker ends its line is skipped via end === -1 — a real newline does separate, exactly as the old comment claimed. Probe green. |
| rc:3741097790 known-gaps omit bodies assembled across statements | Implemented via the offered alternative: gap declared in the known-gap comment block (the guard models a single literal; bodies built line-by-line into a --body-file are out of scope). |
Changes
scripts/tests/qwen-pr-review-workflow.test.js(+52/−14, commitaff47ff8d6): bounded marker scan, anchored and widened\nexemption, unquoted-$(…)flag, URL-weaving pin test, and the declared-gap comment block covering the five accepted limitations.- Merged
origin/main(commite781f473a1) — see conflict notes.
Conflict notes
--conflict false was supplied and no merge conflicts occurred. The merge of origin/main was still necessary for a different reason: npm run build failed on this branch with TS2504 in integrations/external-context/src/http-client.ts — the exact failure that main's #8693 ("make the project typecheckable…") was created to fix by overriding types to ["node"] in that workspace's tsconfig. The branch had last merged main at d91c66119b, one commit before that fix landed, so the branch carried the trigger (transitive @types/jsdom + root vitest/globals) without the fix. The merge brings the fix plus the other commits main advanced by; no files in this PR's own diff were touched by it.
Verification
Commands actually run this round (all at final head aff47ff8d6 unless noted):
npm run build— passed (exit 0, no TS errors). Before the merge it failed withsrc/http-client.ts(124,29): error TS2504, proven pre-existing (the PR's three-dot diff touches only the workflow and the test file; the fix exists on main as fix(integration-tests): make the project typecheckable and fix what that found #8693).npm run typecheck— passed (exit 0).npm run lint— passed (exit 0, zero findings).npx vitest run --config ./scripts/tests/vitest.config.ts qwen-pr-review-workflow— 105/105 passed (104 prior + the new URL-pin test).npm run test:scripts— 50/50 files, 1055 passed + 14 skipped (merge-interference check; includes the changed file).- Mutation matrix against the guard (probe workflow dropped into
.github/workflows/, removed after each run): unanchored-prefix escape red; double-quoted printf format green; multi-line marker-at-line-end green; doc prose with a later quote elsewhere green; unquoted$(…)red; plain double-quoted\n(round-1 M2) red; adjacent literal (round-1 M3) red; legitprintf '<!\-\- m -->' "$VAR"reference (round-1 M5) green — 8/8 as expected. - URL-pin simulation against mutated copies of the ack line: dropping
%s, dropping"$RUN_URL", and removing the printf all turn the new assertion red; head passes. npx prettier --writeon the changed file.
中文说明
评审第 3 轮总结 — PR #8726
本轮收到自动评审者（qwen3.8-max，第 2 轮）的 10 条新建议，以及 @wenshao 评审（deepseek-v4-flash）第 1 轮的 4 条发现——后者已在 5d2998483b 中修复，本轮已重新验证。10 条新建议中 9 条已在代码中处理（提交 e781f473a1、aff47ff8d6）；1 条被拒绝并记录了理由（见该线索的回复）。
第 1 轮发现 — 已在 5d2998483b 中修复，本轮重新验证
| 发现 | 处理 |
|---|---|
| rc:3740711816 [Critical] 守卫正则跨行匹配，吞掉了它本要保护的标记 | 此前已修复：/<!\-\-[^>\n]*-->/g。已在代码中重新验证，并通过变异测试确认（保留注释块的粘接正文现在会让守卫变红）。 |
rc:3740711828 \n 豁免无条件放行 |
此前已修复：限定于 printf/$' 前缀。探针重新验证（普通双引号 \n 形式仍为红）。 |
rc:3740711832 "/'/) 白名单豁免 |
此前已修复：规则重写为"标记打开字符串字面量"并读取相邻字面量。探针重新验证（相邻字面量变体仍为红）。 |
| rc:3740711834 守卫只扫描一个 workflow | 此前已修复：遍历 .github/workflows 全部文件。重新验证（head 上全仓零违规）。 |
第 2 轮发现 — 处理结果
| 发现 | 处理 |
|---|---|
rc:3741097771 没有测试固定 %s → "$RUN_URL" 的编织 |
已实现：新增测试 pins the workflow-run URL into the ack printf，断言承载标记的 printf 含有 %s 且传入 "$RUN_URL"。删掉 %s、删掉参数、删掉 printf 三种变体均为红（已模拟验证）。 |
rc:3741097774 \n 豁免在整个行前缀上未锚定 |
已实现：豁免锚定到前缀末尾（/printf\s+['"]$|\$'$/）。探针 printf '%s' x; ACK_BODY="<!\-\- m -->\n_prose" 现在为红。 |
| rc:3741097775 行首标记（heredoc、YAML 块标量）落在守卫之外 | 按建议提供的替代方案实现：在测试的已知缺口注释块中声明该缺口，而不扩展扫描到 heredoc 内部。 |
| rc:3741097777 豁免只接受单引号 printf 格式串 | 已实现，与锚定修复合并：/printf\s+['"]$|\$'$/。探针 BODY="$(printf "<!\-\- m -->\n\n_prose_")" 为绿（不再误报）。 |
rc:3741097778 标记位于字面量中间、却在 \n 展开后落在渲染行首 |
按建议提供的替代方案实现：在已知缺口注释块中声明该缺口。 |
| rc:3741097781 闭合引号扫描直到文件末尾 | 已实现：扫描被限制在标记所在的物理行内。文件后部存在另一个 " 的探针为绿；qwen-issue-followup-bot.yml:430 的文档示例不再会因未来某处无关引号而翻转。 |
| rc:3741097783 格式串以标记开头的换行 printf | 拒绝 — 见该线索的回复；该局限已在已知缺口注释中声明。 |
rc:3741097785 标记字面量后未加引号的 $(…) 被静默跳过 |
已实现：BODY='<!\-\- m -->'$(gen-prose) 现在会被标记（其输出对静态扫描不可见）。已确认当前没有 workflow 使用该形式，全仓扫描保持干净。 |
| rc:3741097787 多行字面量里的真实换行被当作粘接 | 由限制扫描（rc:3741097781）解决：glued 不再可能包含真实换行；标记位于行尾的多行字面量经 end === -1 被跳过——真实换行确实起分隔作用，与旧注释的声明一致。探针为绿。 |
| rc:3741097790 已知缺口遗漏了跨语句拼装的 body | 按建议提供的替代方案实现：在已知缺口注释块中声明该缺口（守卫只对单个字面量建模；逐行写入 --body-file 拼装的 body 不在范围内）。 |
变更
scripts/tests/qwen-pr-review-workflow.test.js（+52/−14，提交aff47ff8d6）：限制标记扫描范围、锚定并放宽\n豁免、未加引号$(…)标记、URL 编织固定测试，以及在注释块中声明五项已接受的局限。- 合并
origin/main（提交e781f473a1）——见冲突说明。
冲突说明
调用参数为 --conflict false，合并未发生任何冲突。合并 origin/main 仍然必要，原因另有所在：本分支上 npm run build 失败，报 integrations/external-context/src/http-client.ts 的 TS2504——正是 main 上 #8693（"make the project typecheckable…"）通过在该 workspace 的 tsconfig 中把 types 覆盖为 ["node"] 所要修复的失败。本分支上次合并 main 停在 d91c66119b，恰在该修复落地前一个提交，因此分支带有触发条件（传递安装的 @types/jsdom + 根配置 vitest/globals）却没有修复。本次合并带入了该修复及 main 后续推进的其他提交；本 PR 自身 diff 中的文件未受影响。
验证
本轮实际执行的命令（除特别说明外均在最终 head aff47ff8d6 上）：
npm run build— 通过（exit 0，无 TS 错误）。合并前失败于src/http-client.ts(124,29): error TS2504，已证明为既有问题（本 PR 的三点 diff 只触及 workflow 与测试文件；修复以 fix(integration-tests): make the project typecheckable and fix what that found #8693 的形式存在于 main）。npm run typecheck— 通过（exit 0）。npm run lint— 通过（exit 0，零发现）。npx vitest run --config ./scripts/tests/vitest.config.ts qwen-pr-review-workflow— 105/105 通过（原 104 个 + 新增 URL 固定测试）。npm run test:scripts— 50/50 文件，1055 通过 + 14 跳过（合并干扰检查；含被改动文件）。- 守卫变异矩阵（探针 workflow 临时放入
.github/workflows/，每次运行后删除）：未锚定前缀逃逸 红；双引号 printf 格式串 绿；多行且标记位于行尾 绿；文档文本且文件后部另有引号 绿；未加引号$(…)红；普通双引号\n（第 1 轮 M2）红；相邻字面量（第 1 轮 M3）红；合法的printf '<!\-\- m -->' "$VAR"引用（第 1 轮 M5）绿 —— 8/8 符合预期。 - URL 固定测试对 ack 行变异副本的模拟：删除
%s、删除"$RUN_URL"、移除 printf 均使新断言变红；head 通过。 - 对改动文件执行
npx prettier --write。
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/模型 qwen3.8-max
|
🤖 AutoFix could not start — a setup step failed (or the run was cancelled) before the agent ran, so no fix was attempted. This is normally a transient infra issue, a broken base build, or a cancelled run — not this PR. It will retry on the next scan. AutoFix failed before producing a verified commit (the run crashed or timed out before it could explain why). Run log: https://github.com/QwenLM/qwen-code/actions/runs/31275438474 🧠 Handled by Qwen Code · model/模型 |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not explored to full depth (tool budget reached): PR #8726 (QwenLM/qwen-code) rebuilds the queued-ack comme...: the remaining ~63 of the 137 grep matches (tails of qwen-autofix.yml past line 2794, qwen-triage.yml past 4212, qwen-issue-followup-bot.yml past 433) were not e…. 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.
中文说明
已审查。 建议见行内评论。 未探索到全部深度（达到工具调用预算）：PR #8726 (QwenLM/qwen-code) rebuilds the queued-ack comme...：the remaining ~63 of the 137 grep matches (tails of qwen-autofix.yml past line 2794, qwen-triage.yml past 4212, qwen-issue-followup-bot.yml past 433) were not e…。 未检查（工具限制，非阻断）：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.8)
| it('never glues prose onto a comment marker, in any workflow', () => { | ||
| expect(files.length).toBeGreaterThan(0); |
There was a problem hiding this comment.
[Suggestion] The guard scans only .github/workflows/*.yml, but marker-headed bot comment bodies are also built in .github/scripts/*.mjs — three live builders: pr-safety-precheck.mjs:119 (renderManualRequiredComment opens a template literal with <!-- qwen-pr-precheck:manual-required -->), web-shell-visuals-publish.mjs:217 and serve-ab-diff.mjs:151. The known-gaps comment enumerates six shape gaps but is silent about the entire scripts surface, so the confidence this describe block creates exceeds its reach. — Failure scenario: a future edit glues prose onto the marker line in any of those scripts (e.g. collapsing renderManualRequiredComment's first two template lines into one) → the identical raw-Markdown/dead-link bug this PR fixes ships on issue/PR comments while this test stays green.
Minimum fix: declare the surface in the known-gaps comment. Better: widen the scan to .github/scripts with a rule for JS literals — the marker-glued-in-a-template-literal shape is the same one-literal-one-line rule the scanner already implements.
中文说明
守卫只扫描 .github/workflows/*.yml，但带标记的机器人评论正文同样在 .github/scripts/*.mjs 中构造——现存三处：pr-safety-precheck.mjs:119（renderManualRequiredComment 返回以 <!-- qwen-pr-precheck:manual-required --> 开头的模板字面量）、web-shell-visuals-publish.mjs:217、serve-ab-diff.mjs:151。known-gaps 注释列举了六种形状缺口，却对整个 scripts 面只字未提，于是这个 describe 块给出的保证超出了它的实际覆盖。失败场景：未来某次编辑在这些脚本里把正文粘到标记行上（例如把 renderManualRequiredComment 的前两行模板合并成一行）→ 本 PR 所修复的「原始 Markdown 文本、死链接」缺陷会原样出现在 issue/PR 评论里，而该测试仍然为绿。最小修复：在 known-gaps 注释中声明该面；更好的做法是把扫描扩展到 .github/scripts 并加上针对 JS 字面量的规则——模板字面量中的粘接形状与扫描器已实现的「一个字面量一行」规则相同。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // otherwise let one match span lines, swallow the real marker, and get | ||
| // discarded by the comment skip below — the guard would then pass with | ||
| // the very regression present. | ||
| const re = /<!--[^>\n]*-->/g; |
There was a problem hiding this comment.
[Suggestion] A marker whose content contains > never matches /<!--[^>\n]*-->/g, so glued prose on such a marker is invisible to the guard; this gap is not in the known-gaps list. Probe-verified against the verbatim scan logic: BODY="<!-- qwen-triage: A -> B -->prose" → zero offenders, while the same shape without the inner > is flagged. All 48 current workflow markers inventoried — none contains >, so exposure is forward-looking. — Failure scenario: a future workflow uses a marker with > inside it (arrow-style labels are a natural human-readable marker shape) with prose glued → [^>\n]* stops at the inner >, no match is produced, the line is skipped entirely, and the glued-prose regression ships silently in the exact form this PR exists to catch.
Fix: allow > inside the marker while keeping the match single-line, then re-run the suite —
const re = /<!--[^\n]*?-->/g;— or append this shape to the known-gaps comment so the disclosure stays complete.
中文说明
内容包含 > 的标记永远匹配不到 /<!--[^>\n]*-->/g，因此这类标记上粘正文对守卫是不可见的；该缺口不在 known-gaps 列表中。已用逐字提取的扫描逻辑验证：BODY="<!-- qwen-triage: A -> B -->prose" → 零违规，而去掉内部 > 的相同形状会被标记。已清点当前全部 48 个 workflow 标记——没有一个包含 >，所以风险是前瞻性的。失败场景：未来某个 workflow 使用内部带 > 的标记（箭头式标签是很自然的标记命名方式）并粘上正文 → [^>\n]* 在内部 > 处停止，整行被跳过，粘接正文的回归会以本 PR 要修复的确切形式悄悄溜过。修复：允许标记内含 > 但保持单行匹配（如上），然后重跑套件；或者把该形状补进 known-gaps 注释。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| } else if (after[0] === '$' && after[1] === '(') { | ||
| glued = after; | ||
| } |
There was a problem hiding this comment.
[Suggestion] The same-line continuation model after an empty marker literal only recognizes a following quoted literal or $(…); every other continuation bash concatenates at runtime passes the guard. Probe-verified shapes (guard logic run verbatim → offenders=[]; bash → glued, exit 0): BODY="<!-- m -->"$PROSE ($VAR expansion), BODY='<!-- m -->'prose (unquoted word), BODY='<!-- m -->'$'prose' (ANSI-C literal), backtick substitution, BODY="<!-- m -->"\ + newline + "prose" (no-space backslash continuation), and BODY="$(printf '<!-- m -->')prose" (text after the wrapping subshell's ) — proven by running the actual guard test against a fixture workflow). None is declared in the known-gaps comment, which presents the adjacency model as deliberate and complete. (Aggregates three independently found, probe-verified findings.) — Failure scenario: each shape is one keystroke from this PR's own ACK_BODY="$(printf '…')" pattern → the marker glues to prose at runtime → the exact raw-source/dead-link bug this PR fixes ships with the guard reporting offenders === [].
Fix options: declare these shapes in known-gaps, or treat any non-whitespace continuation outside the marker literal as suspect. Caution if closing the hole: a naive "any text after )" rule produced 11 false positives on existing jq contains("<!-- … -->")) references during probing — the discriminator must require the marker literal to sit inside a subshell-wrapped assignment (="(…" in the prefix).
中文说明
空标记字面量之后的同行续接模型只识别紧随的引号字面量或 $(…)；其余任何会被 bash 在运行时拼接的续接形式都能通过守卫。已用逐字提取的守卫逻辑 + bash 逐一验证（守卫 offenders=[]，bash 实际粘接且 exit 0）：BODY="<!-- m -->"$PROSE（$VAR 展开）、BODY='<!-- m -->'prose（未加引号的词）、BODY='<!-- m -->'$'prose'（ANSI-C 字面量）、反引号替换、BODY="<!-- m -->"\ + 换行 + "prose"（无空格反斜杠续行）、以及 BODY="$(printf '<!-- m -->')prose"（文本落在包裹子shell 的 ) 之后——通过向守卫测试投放 fixture workflow 实证）。known-gaps 注释未声明其中任何一种，而该注释把邻接模型表述为刻意且完备的设计。（本条聚合了三条独立发现、均经探针验证的 finding。）失败场景：上述每种形状距离本 PR 自己的 ACK_BODY="$(printf '…')" 模式都只有一步之遥 → 运行时标记与正文粘接 → 本 PR 所修复的「原始文本、死链接」缺陷在守卫全绿的情况下再次上线。修复选项：把这些形状声明进 known-gaps，或把标记字面量之外任何非空白续接都视为可疑。若要堵洞请注意：探针验证时，朴素的") 后任何文本"规则会在现有 jq contains("<!-- … -->")) 引用上产生 11 个误报——判别条件必须要求标记字面量位于子shell 包裹的赋值内部（前缀中含 ="(…"）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // Prose in a YAML or shell comment never reaches a comment body. | ||
| if (/^\s*#/.test(prefix)) continue; |
There was a problem hiding this comment.
[Suggestion] The comment skip only fires for comments that OPEN a physical line; an end-of-line YAML or shell comment containing a quoted marker with glued prose is still scanned and flagged, contradicting the rationale comment above it. Probe-verified against the verbatim scan logic: foo: bar # example: '<!-- m -->prose' and echo hi # docs '<!-- m -->prose' here are both pushed into offenders, while the full-line-comment control is correctly skipped. The limit is absent from the known-gaps enumeration. — Failure scenario: a future workflow edit documents a glued-marker shape in a trailing inline comment — e.g. noting the exact regression this PR fixes, the way this PR's own workflow comment does — → CI fails on a false "glued prose" accusation for prose that sits in a comment and cannot reach any comment body.
Fix: declare the limit in the known-gaps comment alongside its siblings, or strip trailing-comment regions before scanning (a # starts a comment only when preceded by whitespace and not inside an open literal).
中文说明
注释跳过只对"以注释开头"的物理行生效；行尾的 YAML 或 shell 注释中若包含带粘接正文的引号标记，仍会被扫描并标记，与上方的理由注释相矛盾。已用逐字提取的扫描逻辑验证：foo: bar # example: '<!-- m -->prose' 和 echo hi # docs '<!-- m -->prose' here 都会进入 offenders，而整行注释的对照组被正确跳过。该限制不在 known-gaps 枚举中。失败场景：未来某次编辑在行尾内联注释里记录粘接标记的形状——例如像本 PR 自己的 workflow 注释那样注明它所修复的这个回归——→ CI 会误报"粘接正文"，尽管注释里的正文根本不可能进入任何评论 body。修复：把该限制补进 known-gaps 注释，或在扫描前剥掉行尾注释区域（仅当 # 前有空白且不在未闭合字面量内部时才视为注释起点）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| if (glued.startsWith('\\n') && /printf\s+['"]$|\$'$/.test(prefix)) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
[Suggestion] The \n exemption /printf\s+['"]$|\$'$/ requires the format-opening quote to immediately follow printf, so printf forms with options between the builtin and the format are flagged although their bodies are correctly separated at runtime. Probe-verified: printf -v BODY '<!-- m -->\n\nprose' and printf -- '<!-- m -->\n\nprose' are both flagged, while bash expands \n in both (real blank line, exit 0); YAML double-quoted scalars (body: "<!-- m -->\nprose") are flagged although the YAML parser expands \n to a real newline. printf -v BODY already builds marker-headed bodies in this corpus (qwen-triage.yml:1007 — unflagged today only because its marker literal ends the format). The gap is undeclared. — Failure scenario: refactoring the ack assignment to printf -v ACK_BODY '…' — the no-subshell variant of the very shape this PR introduces as the sanctioned pattern — fails CI on a false positive with no explanation.
Probe-verified fix direction (broaden the exemption; the YAML sub-shape is a different axis and needs its own treatment):
if (glued.startsWith('\\n') && /printf\s+(?:-\S+\s+(?:\S+\s+)?|--\s+)?['"]$|\$'$/.test(prefix)) {— or declare these shapes in the known-gaps comment.
中文说明
\n 豁免 /printf\s+['"]$|\$'$/ 要求格式串的开头引号紧跟在 printf 之后，因此 builtin 与格式串之间带选项的 printf 形式会被标记——尽管它们的 body 在运行时是正确分隔的。已用探针验证：printf -v BODY '<!-- m -->\n\nprose' 和 printf -- '<!-- m -->\n\nprose' 均被标记，而 bash 对两者都会展开 \n（产生真实空行，exit 0）；YAML 双引号标量（body: "<!-- m -->\nprose"）也被标记，尽管 YAML 解析器会把 \n 展开为真实换行。语料库中已有用 printf -v BODY 构造带标记正文的先例（qwen-triage.yml:1007——今天未被标记仅仅因为它的标记字面量恰好结束格式串）。该缺口未被声明。失败场景：把 ack 赋值重构为 printf -v ACK_BODY '…'——正是本 PR 引入的推荐形状的无子shell变体——会在 CI 上因误报而失败，且没有任何解释。已验证的修复方向（放宽豁免；YAML 子形状属于另一个维度，需单独处理，见上代码块），或把这些形状声明进 known-gaps 注释。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
@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: 68 passed · 0 failed · 68 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：68 通过 · 0 失败 · 68 总计 Verification report<!-- qwen-triage:verify --> Sandboxed verification: Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 68 passed · 0 failed · 68 total 中文 — 判定：`merge-ready` · 68/68 脚本化断言通过结论：
Verification reportPR 8726 Deep Verification (round 2) —
|
| # | round-1 finding | severity | status at ab4af897 |
|---|---|---|---|
| 1 | description quotes the guard's failure snippet as …Review is qu; actual output ends at …request accept |
correction (info) | stands — re-measured at the new head (cell C1): qwen-code-pr-review.yml: <!\-\- qwen-review-ack -->_Qwen Code review request accept; description unchanged |
| 2 | description's "101/101" was pre-merge; file held 104 at 5d299848 |
correction (info) | stands, updated count — the file now holds 105 tests, 105/105 pass (H1); the +1 is the new pins the workflow-run URL into the ack printf test; description still says 101 |
| 3 | known gap (printf-arg-split body) is real and honestly documented | info | stands — re-measured (M4 green); the round-2 test comment now documents a longer gap list, whose two probeable members (arg split, mid-literal marker) were each driven (M4, M10 green) |
| 4 | the pre-round-1 [^>] regex also false-positived on qwen-autofix.yml with no regression present |
info | superseded — that FP was measured against the round-1 guard, which the depth-2 clone cannot resurrect; at the round-2 head, the single-point [^>] mutant of the current test no longer false-positives on the clean tree (M1c green) while the miss behavior remains (M1b green with the regression present). The \n hardening is load-bearing for detection; its FP half was absorbed by the round-2 line-bounding logic. Nothing to fix |
Declined rows: none. Deferred rows: none.
Central claim + A/B
Central claim: the ack body glued its prose onto the <!\-\- qwen-review-ack --> marker; a line opening <!\-\- starts a type-2 HTML block that runs to the line containing --> inclusive, so the prose and the workflow-run link shipped unrendered. At this head the body is built as ACK_BODY="$(printf '<!\-\- qwen-review-ack -->\n\n_…(%s)._' "$RUN_URL")" — the round-2 commit also wove the URL through a %s conversion.
Both arms' Post queued acknowledgement step (job ack-review-request) was extracted verbatim from the workflow YAML with the repo's yaml package, executed under bash --noprofile --norc with the step's own set -euo pipefail and its env: block, with a stub gh recording the exact bytes at the API boundary (assertions at the destination). RUN_URL set to the run the PR quotes in its After HTML so the byte comparison is exact.
| cell | body reaching the wire | GitHub POST /markdown (gfm) |
marked | markdown-it |
|---|---|---|---|---|
| base-post | glued one-liner (byte-exact, A4) | 0 <a>, 0 <em> — marker consumed as HTML comment, prose raw with dead link (A18–A19) |
0/0 | deviates (see Findings 3) |
| head-post | marker + \n\n + prose, URL woven exactly once, no literal \n (A9–A11) |
1 <a>, 1 <em>, equals PR-quoted After HTML byte-for-byte (A20–A21) |
1/1 | 1/1 |
single-\n variant |
— | 1/1 (PR table row 2, A22) | — | — |
| base-patch | prior glued ack (id 12345, among decoys) found → PATCH, glued body | — | — | — |
| head-patch | same prior ack found → PATCH id 12345 with the fixed body; human-quoted marker and bystander bot comment skipped (A13–A17) | — | — | — |
Witness: 01-ab-base-vs-head-github-render.png. Assertions A1–A26 + F1 (27/27) in 01-ab-body.mjs.
Secondary claim — upsert compatibility
The jq filter was extracted verbatim from the head run block (byte-identical to base, F1/U8) and driven through the real jq binary: old glued ack selected for PATCH (U1), mixed page picks the latest bot ack and never a human-quoted marker (U2–U3), paginated pages slurped with last winning (U4), and the fixed-form body also matches contains() so a once-PATCHed comment is updated in place rather than stacked (U5). Step-level PATCH behavior on both arms is in the A/B table above. Witness: 02-upsert-jq-cells.png. B/U1–U8 (8/8) in 02-upsert-jq.mjs.
Secondary claim — the guard tests hold down what they claim
Each cell ran the real test file (or a single-point mutant of it) via the repo's vitest config inside a fixture tree of all 47 workflows with one mutated member, cwd = fixture. -t filtered to one test per cell except C0/C1.
| cell | mutation | expected | observed |
|---|---|---|---|
| C0 | head tree, real test | green, 2 passed | green (2 passed) |
| C1 | qwen-code-pr-review.yml = base |
red, guard names offender at expect(offenders).toEqual([]) |
red: qwen-code-pr-review.yml: <!\-\- qwen-review-ack -->_Qwen Code review request accept (7 tests red against base overall; the guard's is the one quoted) |
| M1 | glued body re-introduced, PR comment block kept | red | red |
| M1b | M1 + test mutant [^>] |
green (regression survives) | green — \n hardening load-bearing for detection |
| M1c | clean tree + test mutant [^>] |
green at round-2 head (re-measure of R1-finding 4) | green, zero offenders |
| M2 | ACK_BODY="<!\-\- m -->\n\n_x_" (double-quoted assignment) |
red | red |
| M3 | adjacent literal '<!\-\- m -->''_x_' |
red | red |
| M4 | printf '%s%s' '<!\-\- m -->' '_x_' (documented gap) |
green | green |
| M5 | printf '<!\-\- m -->' "$VAR" (legit reference) |
green | green |
| M6 | glued marker planted in a second workflow | red naming that file | red, names zz-planted-ack.yml |
| M7 | '<!\-\- m -->'$(gen_body) (round-2 clause) |
red | red |
| M7b | M7 + test mutant with the $( branch deleted |
green (clause load-bearing) | green |
| M8 | printf "<!\-\- m -->\n\n_x_" double-quoted format (round-2 widening) |
green | green |
| M8b | M8 + test mutant narrowing the exemption to single quotes | red (widening load-bearing) | red |
| M9 | $'<!\-\- m -->\n\n_x_' ANSI-C quoting |
green | green |
| M10 | printf 'x\n<!\-\- m -->prose' (round-2 documented gap) |
green | green |
| V1 | ack line with %s and "$RUN_URL" dropped |
URL-pin test red | red, names %s |
| V2 | ack line keeping %s, dropping "$RUN_URL" |
URL-pin test red | red, names "$RUN_URL" |
| S1/S2 | bash semantics: %s with no arg, and leftover arg with no %s |
exit 0, silent | exit 0 both — nothing else in the pipeline catches either, which is why the pin test exists |
| S3 | head printf shape with RUN_URL set | exact woven body | exact |
Witness: 03-guard-mutation-matrix.png. G1–G21 (21/21) in 03-guard-matrix.mjs. Vacuity: C1 fails against base at the intended assertion; positive controls are C0/M1 (harness provably goes red on the regression) and V1/V2 (pin test provably red on its two mutants).
Corrections
- Failure-snippet quote (stands). The description shows the guard failing on
mainwith"<!\-\- qwen-review-ack -->_Qwen Code review request accepted. Review is qu". The actual vitest diff at this head prints"qwen-code-pr-review.yml: <!\-\- qwen-review-ack -->_Qwen Code review request accept"(56-char slice from the marker). The test does fail on base and names the offending line; only the quoted truncation is stale. - Test counts (stands, updated). "101/101" and "1054 passed" are pre-merge figures. At this head the changed file holds 105 tests (105/105, H1); the full scripts suite executes 950/950 in this container with
install-script.test.jsuncollectible (nozip, environmental — see Not covered).
Findings
No blocking findings. Informational notes:
- (info) Known gaps are real and honestly documented. M4 (printf-arg split) and M10 (mid-literal marker landing at a rendered line start) escape the guard, exactly as the test comment states. Completeness reporting only; no action required.
- (info) The
\nhardening's role narrowed at this head. Re-measured M1c: un-hardening the regex on the round-2 code no longer false-positives on the clean tree (the round-1 FP is superseded by the line-bounding logic), while M1b shows the un-hardened regex still lets the glued regression survive. The clause remains load-bearing; the previous round's FP evidence is historical. - (info) markdown-it is not a conforming oracle for the base shape. On the glued body, GitHub and marked both yield 0
<a>/0<em>(GitHub consumes the marker as an HTML comment and ships the prose raw; marked ships the whole line raw), but markdown-it 14.2.0 closes the HTML block and re-parses the rest of the line, rendering<em>/<a>— deviating from CommonMark html-block rule 2 and from the livebody_htmlbehavior the PR measured. The A/B therefore treats GitHub + marked as oracles; the harness asserts the deviation itself (A25) so the divergence is visible, not hidden.
Not covered
- Per-commit attribution: the depth-2 checkout leaves 1 of the 5 commits in the metadata snapshot reachable (
git rev-list HEAD^1..HEAD^2= 1). The aggregateHEAD^1..HEADdiff was verified; the mutation matrix covers the round-2 commit's claims behaviorally. Per-commit separation out of reach. - yamllint: the repo wrapper installs it via
pip3, which this container does not ship. actionlint + shellcheck +bash -nall ran. - The tests inside
install-script.test.js: not collectible here (module-scope guard throws whenzipis absent on a CI host; container ships nozip). The file is byte-identical base vs head (emptygit diff --stat), so the failure is environmental, proven not a regression. - Live comment round-trip (
ghPOST/PATCH → storedbody_html): no GitHub token by design. The A/B reproduces the wire shape through the same renderer endpoint the author measured with (rendering half) and the step-level cells exercise the lookup/PATCH branch selection against a stub peer (lookup half); the storage half remains unexercised. - Windows/macOS: N/A — the change is a Linux-bash CI step, matching the PR's Tested-on table.
POST /markdownthrottling: 12 anonymous calls succeeded across the round; a throttled rerun would need a token.
Methodology
Environment: the CI verify container (node:22-bookworm, node v22.23.2, jq present, no zip/pip3; POST api.github.com/markdown anonymously reachable — probed and used as the primary rendering oracle; actionlint/shellcheck installed via the repo's node scripts/lint.js --setup). Both arms' ack steps were extracted with the repo's yaml package from git show HEAD^1:… and the head file, executed under bash --noprofile --norc with a stub gh on PATH capturing argv payloads and --body bytes at the wire boundary; rendering cells used GitHub POST /markdown (mode=gfm, repo context) corroborated by marked 15.0.12 and markdown-it 14.2.0 from the repo's node_modules. The guard matrix ran the real test file (or single-point mutants, each asserted to differ by exactly one hunk) via the repo's vitest config in per-cell fixture trees of all 47 workflows under tmp/pr8726-verify-20260808-231004/fixtures/ (module resolution walks up to the root node_modules; the test reads .github/workflows relative to cwd). Gates: vitest/node --test suites, actionlint and shellcheck with the repo's exact flag sets (liveness-proven by planted violations), bash -n on both extracted steps, and a repo-wide marker census. Harnesses, per-cell logs, gate logs, and the four captures live in tmp/pr8726-verify-20260808-231004/ (01-ab-body.mjs, 02-upsert-jq.mjs, 03-guard-matrix.mjs, 04-gates.mjs, logs/, evidence/).
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
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。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| expect(ackLine).toBeDefined(); | ||
| expect(ackLine).toContain('%s'); | ||
| expect(ackLine).toContain('"$RUN_URL"'); |
There was a problem hiding this comment.
[Suggestion] The URL-pin test asserts %s and "$RUN_URL" merely co-exist on the ack line, not that the URL is woven into the markdown link — a displaced %s or marker-as-argument refactor passes both tests and re-ships the dead-link comment this PR fixes.
Failure scenario: a future edit rewrites the ack line to ACK_BODY="$(printf '%s%s' '<!-- qwen-review-ack -->' "$RUN_URL")" (marker moved out of the format into its own argument — the natural refactor, since $RUN_URL is already an argument) or moves %s outside the link parens ([workflow run]%s._). Both forms keep %s and "$RUN_URL" somewhere on the line, so the two toContain assertions pass. Real bash confirms the output renders [workflow run]https://… with no parens / the URL glued onto the marker line — GitHub's HTML block swallows the line, the comment ships as raw source with a dead link again, and CI stays green.
| expect(ackLine).toBeDefined(); | |
| expect(ackLine).toContain('%s'); | |
| expect(ackLine).toContain('"$RUN_URL"'); | |
| expect(ackLine).toContain('[workflow run](%s)'); |
中文说明
URL 固定测试只断言 %s 与 "$RUN_URL" 同时出现在 ack 行上，并未断言 URL 真的织入了 markdown 链接——一旦 %s 被移出链接括号、或标记被改成 printf 的独立参数，两个 toContain 断言仍然通过，死链评论会再次发出。失败场景：把 ack 行改写为 printf '%s%s' '<!-- qwen-review-ack -->' "$RUN_URL"，或把 %s 移出括号。bash 实测输出为 [workflow run]https://…（无括号）或 URL 直接粘在标记行——GitHub 的 HTML block 吞掉整行，评论以原始文本发出且链接失效，CI 依旧全绿。建议改为断言链接形状，例如 expect(ackLine).toContain('[workflow run](%s)')。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| const dir = '.github/workflows'; | ||
| const files = readdirSync(dir).filter((f) => /\.ya?ml$/.test(f)); |
There was a problem hiding this comment.
[Suggestion] The new guard pastes the identical workflow-directory scan idiom already present 130 lines above in this file (describe('workflow expression length') at lines 2178-2179) — two independent enumerations of .github/workflows with the same \.ya?ml$ filter.
Failure scenario: any future change to the scan — a new workflow-file extension, excluding a subdirectory, or the dir moving — must be applied in both places; applying it in one silently leaves the two guards scanning different file sets with no failure signal and no way to notice.
| const dir = '.github/workflows'; | |
| const files = readdirSync(dir).filter((f) => /\.ya?ml$/.test(f)); | |
| const workflowFiles = readdirSync('.github/workflows').filter((f) => /\.ya?ml$/.test(f)); |
中文说明
新守卫把本文件 130 行之前已有的工作流目录扫描惯用法又粘贴了一遍（workflow expression length describe 块，2178-2179 行）——两处各自枚举 .github/workflows 且使用相同的 \.ya?ml$ 过滤。失败场景：将来任何对扫描范围的改动（新增扩展名、排除子目录、目录迁移）都必须在两处同时进行；只改一处会让两个守卫静默扫描不同的文件集合，没有任何失败信号。建议把枚举提升到模块作用域（第 23 行已有的 const workflow = readFileSync(...) 旁），两个 describe 共用。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
|
Code review. My independent proposal before reading the diff: separate the marker from the prose with a blank line — What I verified in the diff and the surrounding code:
Testing. Evidence in this section is first-hand from this pass, not the author's self-report:
Orchestration checks (authorize / review-pr / ack-review-request and friends) are event-gated and report skipped on this commit by design. 中文说明代码审查。 读 diff 前我的独立方案：用空行把标记与正文分开（用带 diff 与周边代码中核查过的点：
测试。 本节证据均为本轮一手核实，不是作者自述：
（CI 表格见上方英文部分；编排类检查 authorize / review-pr / ack-review-request 等按事件门控，跳过属设计使然。） — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — observed bug with a live reproduction, a minimal byte-safe fix, regression guards that fail on This is exactly what a good small fix looks like. I re-derived the diagnosis from scratch this pass — a line opening with — Qwen Code · qwen3.8-max Reviewed at |
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. ✅
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Autofix review-response summary — PR #8726 (review round 2)Commit: All five inline findings are [Suggestion] findings from the automated reviewer on the marker-guard test. Each was either closed in code (where the fix is a verified one-liner) or declared in the guard's existing "Known gaps" comment, which is the fix option each finding explicitly offers. Probe verification below was done by running the real test against temporary fixture workflows in Finding dispositions
No findings were declined, deferred, or escalated. Probe evidence (temporary fixtures, since deleted)Run through the actual guard test: Verification
中文说明Autofix 审查响应摘要 — PR #8726（审查第 2 轮）提交： 五条行内 finding 全部来自自动审查器对标记守卫测试的 [Suggestion]。每一条要么在代码中关闭（修复为经验证的一行改动时），要么在守卫既有的 "Known gaps" 注释中声明——后者也是每条 finding 明确给出的修复选项之一。下文的探针验证通过在 各 finding 处置
没有 finding 被拒绝、搁置或升级。 探针证据（临时 fixture，已删除）通过真实守卫测试运行： 验证
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/模型 |
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Review round summary — PR #8726Commit: Feedback points and dispositions1. [Suggestion] URL-pin test only asserts
|
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not explored to full depth (tool budget reached): PR #8726 (QwenLM/qwen-code, author @wenshao): fixes the q...: could not run the actual vitest suite ( npm run test:scripts ) — no node_modules installed; replaced with a verbatim standalone replication of both new tests.; PR #8726 (QwenLM/qwen-code, author @wenshao): fixes the q...: did not independently re-verify GitHub's live renderer (POST /markdown); relied on CommonMark HTML-block type-2 semantics plus the author's stated measurement.; PR #8726 (QwenLM/qwen-code, author @wenshao): fixes the q...: could not run npm run test:scripts (vitest) itself** — no node_modules in this worktree; I substituted a verbatim standalone replication of both new tests' …. Not reviewed: reverse audit — stopped before round 4 by the review time budget. 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.
中文说明
已审查。 建议见行内评论。 未探索到全部深度（达到工具调用预算）：PR #8726 (QwenLM/qwen-code, author @wenshao): fixes the q...：could not run the actual vitest suite ( npm run test:scripts ) — no node_modules installed; replaced with a verbatim standalone replication of both new tests.；PR #8726 (QwenLM/qwen-code, author @wenshao): fixes the q...：did not independently re-verify GitHub's live renderer (POST /markdown); relied on CommonMark HTML-block type-2 semantics plus the author's stated measurement.；PR #8726 (QwenLM/qwen-code, author @wenshao): fixes the q...：could not run npm run test:scripts (vitest) itself** — no node_modules in this worktree; I substituted a verbatim standalone replication of both new tests' …。 未审查：反向审计——评审时间预算不足，未能开始第 4 轮。 未检查（工具限制，非阻断）：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.8)
| if ( | ||
| glued.startsWith('\\n') && | ||
| /printf\s+(?:-\S+\s+(?:\S+\s+)?|--\s+)?['"]$|\$'$/.test(prefix) | ||
| ) { | ||
| continue; |
There was a problem hiding this comment.
[Suggestion] The \n exemption models incompletely where a backslash-n escape actually separates marker from body — three probe-verified shapes flag correctly-rendering bodies (spurious CI red), and none is declared in the known-gaps enumeration (which documents the analogous YAML case): (1) jq/JSON-string escape — BODY="$(jq -rn '"<!-- m -->\n" + $body')": JSON's \n becomes a real newline in jq output (verified), the prose renders on its own line, but the prefix ends jq -rn '" and matches neither exemption branch (a fixture reproduced the failure); (2) printf '%b' argument position — BODY="$(printf '%b' '<!-- m -->\nprose')": bash expands escapes in a %b argument (od-verified real newline), but the marker literal is not in the format position, so it is flagged; (3) insignificant whitespace before the escape — printf '<!-- m --> \nprose': a stray trailing space after the marker defeats glued.startsWith('\\n') although the HTML block still closes on the marker line and the prose renders (A/B probe: spaced variant flagged, unspaced passes). — Suggested fix: widen the exemption (/^\s*\\n/ instead of startsWith('\\n') under the same prefix condition; add a '%b'-argument alternative), and declare the jq/JSON shape in the known-gaps comment beside the YAML case — or declare all three if the strictness is deliberate.
中文说明
[建议] \n 豁免对"反斜杠 n 实际上把标记与正文分开"的情形建模不完整——已用探针验证的三种形式会把渲染正常的正文判成违规（CI 误报红），且都未在 known-gaps 枚举中声明（枚举里已记录了类似的 YAML 情形）：(1) jq/JSON 字符串转义——BODY="$(jq -rn '"<!-- m -->\n" + $body')"：JSON 的 \n 在 jq 输出中变成真实换行（已验证），正文在独立行上正常渲染，但 prefix 以 jq -rn '" 结尾，两个豁免分支都不匹配（fixture 复现了失败）；(2) printf '%b' 参数位——BODY="$(printf '%b' '<!-- m -->\nprose')"：bash 会对 %b 参数展开转义（od 验证为真实换行），却因标记字面量不在格式串位置而被标记；(3) 转义前的无关空白——printf '<!-- m --> \nprose'：标记后一个多余的行尾空格使 glued.startsWith('\\n') 失效，但 HTML block 仍在标记行结束、正文照常渲染（A/B 探针：带空格变体被标记，不带空格通过）。— 建议修复：放宽豁免（同一 prefix 条件下用 /^\s*\\n/ 代替 startsWith('\\n')；为 '%b' 参数位增加一个豁免分支），并把 jq/JSON 形式与 YAML 情形并列补进 known-gaps 注释——如果刻意保持严格，则把三种形式都声明出来。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| const quote = m.index === lineStart ? null : text[m.index - 1]; | ||
| // Only a marker that OPENS a string literal can be building a body. | ||
| if (quote !== "'" && quote !== '"') continue; |
There was a problem hiding this comment.
[Suggestion] Whitespace between the opening quote and the marker evades this check (text[m.index - 1] is a space, not a quote), but GFM/CommonMark still open an HTML block on a marker line with up to 3 leading spaces (and an indented code block at 4+) — the exact regression this guard exists to catch ships green, and the shape is absent from the known-gaps enumeration. — Failure scenario: a future edit writes BODY=" <!-- m -->_prose [workflow run](%s)._" with one cosmetic space. Probe-verified: a verbatim replica of this scanner passes the 1/3/4-space shapes, while marked and markdown-it emit 0 <a>/0 <em> for 1–3 leading spaces — identical to the original bug — and raw <pre><code> at 4. — Suggested fix: treat whitespace-only space between the opening quote and the marker as still opening the literal — match the end of prefix against /(['"])[ \t]*$/ and use the captured quote. Probe-verified caveat: also allow [ \t]* before the exemption's trailing ['"]$ anchor, or a format-position space before the marker becomes a new false positive; alternatively declare the shape in known-gaps.
中文说明
[建议] 开引号与标记之间的空白可以绕过这一检查（此时 text[m.index - 1] 是空格而不是引号），但 GFM/CommonMark 对行首至多 3 个空格的标记行仍会开启 HTML block（4 个及以上则是缩进代码块）——本守卫要拦截的回归会原样绿灯上线，且该形状不在 known-gaps 枚举中。— 失败场景：未来某次编辑写出 BODY=" <!-- m -->_prose [workflow run](%s)._"（一个无意义的空格）。已用探针验证：逐字复制的扫描器对 1/3/4 空格形状全部放行，而 marked 与 markdown-it 在 1–3 个前导空格下输出 0 个 <a>/0 个 <em>——与原始缺陷完全一致——4 个空格时则是原始 <pre><code>。— 建议修复：把"开引号与标记之间仅有空白"仍视为打开字面量——用 /(['"])[ \t]*$/ 匹配 prefix 末尾并取捕获的引号。探针验证的注意事项：豁免条件里行尾的 ['"]$ 锚点前也要允许 [ \t]*，否则格式串位置在标记前带空格会变成新的误报；或者把该形状声明进 known-gaps。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // stays literal; and marker-headed bodies built outside | ||
| // `.github/workflows` — the `.github/scripts/*.mjs` comment builders | ||
| // (template literals and pushed marker lines) are not scanned. All are |
There was a problem hiding this comment.
[Suggestion] The enumeration scopes builder-pattern gaps to bodies built outside .github/workflows, but a live builder inside the scanned directory is equally invisible to the guard: pr-force-push-reminder.yml holds its marker only as a standalone JS constant (line 101) and assembles the body with [MARKER, '', english, …].join('\n') — the scanner sees no glue site in that file, and "built outside .github/workflows … not scanned" implies everything inside is covered. — Failure scenario: probe-verified — changing that join separator to '' makes the emitted body start <!-- pr-force-push-reminder -->Please do not rebase… (prose glued onto the marker — the exact dead-rendering regression this PR fixes), while both marker tests still pass. — Suggested fix: extend this enumeration to cover marker constants joined/concatenated to prose at runtime inside workflow files (naming the pr-force-push-reminder.yml array-join builder), or add a targeted assertion pinning that file's MARKER, '', separator element.
中文说明
[建议] 枚举把"拼装器模式"的缺口限定在 .github/workflows 之外构造的正文，但扫描目录内部同样存在一个对守卫不可见的现存拼装器：pr-force-push-reminder.yml 只把 marker 作为独立 JS 常量（第 101 行）持有，正文用 [MARKER, '', english, …].join('\n') 拼装——扫描器在该文件里看不到任何粘接点，而"在 .github/workflows 之外构造……不被扫描"的措辞暗示目录内的一切都在覆盖范围内。— 失败场景：已用探针验证——把该 join 分隔符改成 '' 后，发出的正文以 <!-- pr-force-push-reminder -->Please do not rebase… 开头（正文直接粘在标记上——正是本 PR 修复的"死渲染"回归），而两个 marker 测试仍然通过。— 建议修复：把此枚举扩展为覆盖 workflow 文件内部在运行时经 join/拼接与正文相连的 marker 常量（点名 pr-force-push-reminder.yml 的数组 join 拼装器），或者增加一个针对性断言，固定该文件中 MARKER, '', 的分隔元素。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round: no action takenThis round found no actionable feedback on PR #8726, so no code changes were made.
The PR head remains at the last pushed commit ( 中文说明Autofix 审查轮次：未采取任何操作本轮未发现针对 PR #8726 的可处理反馈，因此未做任何代码改动。
PR 的 head 仍停留在上次推送的提交（ Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@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: 84 passed · 0 failed · 84 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：84 通过 · 0 失败 · 84 总计 Verification reportPR 8726 Deep Verification (round 3) —
|
| # | round-2 finding | severity | status at 9bb5a8f2 |
|---|---|---|---|
| 1 | description quotes the guard's failure snippet as …Review is qu; actual output ends at …request accept |
correction (info) | stands — re-measured (cell C1): qwen-code-pr-review.yml: <!\-\- qwen-review-ack -->_Qwen Code review request accept (56-char slice from the marker); description unchanged |
| 2 | description's "101/101" / "1054 passed" were pre-merge figures | correction (info) | stands, counts re-measured — changed file holds 105 tests, 105/105 pass (T1, C0); full scripts suite executes 950/950 across 50 collectible files (T2) with install-script.test.js (72 tests) uncollectible — no zip in this container and no root to install it; the file is byte-identical base vs head and the collection error names zip (T2b/T2c), so the failure is environmental |
| 3 | known gaps (printf-arg split M4, mid-literal marker M10) are real and honestly documented | info | stands, list widened and re-probed — M4/M10 re-measured green; the round-3 comment now declares a longer gap list, whose probeable members were each driven: subshell-tail continuation BODY="$(printf '<!\-\- m -->')prose" (G1, green), unquoted heredoc-position body (G4, green), trailing end-of-line comment (G2, red — flagged by design, as declared), YAML double-quoted scalar (G3, red — flagged by design, as declared) |
| 4 | markdown-it deviates from GitHub on the glued shape | info | stands in numbers, mechanism corrected — see Corrections 3 |
| 5 | round-1 [^>] false-positive history on qwen-autofix.yml (superseded at round-2 head) |
info | closed — the round-3 change goes the other way ([^>\n] → [^\n]): the narrowing mutant is still FP-free on the clean tree (W3 green) while the arrow marker escapes it (W2 green with the regression present), and the widened class adds no FP on legitimate >-bearing references (W4 green, C0 green over all 48 workflows) |
Declined rows: none. Deferred rows: none.
Central claim + A/B
Central claim: the ack body glued its prose onto the <!\-\- qwen-review-ack --> marker; a line opening <!\-\- starts a type-2 HTML block that runs to the line containing --> inclusive, so the prose and the workflow-run link shipped unrendered. At this head the body is built as ACK_BODY="$(printf '<!\-\- qwen-review-ack -->\n\n_…[workflow run](%s)._' "$RUN_URL")" — unchanged byte-for-byte since round 2; the base tip moved (4d6246bd → 73e9eab6) and still carries the glued one-liner, so the A/B is re-measured against it, not carried forward.
Both arms' Post queued acknowledgement step (job ack-review-request) was extracted verbatim from the workflow YAML with the repo's yaml package, executed under bash --noprofile --norc with the step's own set -euo pipefail and its env: block, with a stub gh recording argv at the API boundary (assertions at the destination). RUN_URL set to the run the PR quotes in its After HTML so the byte comparison is exact.
| cell | body reaching the wire | GitHub POST /markdown (gfm) |
marked | markdown-it |
|---|---|---|---|---|
| base-post | glued one-liner (byte-exact, A5) | 0 <a>, 0 <em> — marker consumed, prose raw, link dead (A19, A23–A24) |
0/0 (A25) | default preset 1/1 (deviates, A26); html:true 0/0 (A27) |
| head-post | marker + \n\n + prose, URL woven exactly once, no literal \n, marker on line 1 (A6–A9, T6c) |
1 <a>, 1 <em>, equals PR-quoted After HTML byte-for-byte after trimming the API envelope newline (A20, A22) |
1/1 (A25) | 1/1 |
single-\n variant |
— | 1/1 (PR table row 2, A21) | — | — |
| base-patch | prior glued ack (id 12345, among a human-quoted marker and a different-marker decoy across two pages) found → PATCH, glued body (A12, A14) | — | — | — |
| head-patch | same prior ack found → PATCH id 12345 with the fixed body; human-quoted marker and different-marker bot comment never selected (A13, A15–A17) | — | — | — |
Witness: 01-ab-base-vs-head-github-render.png. Assertions A1–A27 (27/27) in 01-ab-body.mjs.
Secondary claim — upsert compatibility
The jq filter was extracted verbatim from the head run block and is byte-identical to base (A18), then driven through the real jq binary on fixture pages shaped like real gh --paginate output (concatenated JSON documents):
| cell | expectation | result |
|---|---|---|
| U1 lone prior glued ack | selected for PATCH | ✓ |
| U2 human quote + decoy + two bot acks across pages | latest bot ack wins | ✓ |
| U3 human-quoted marker only | no id | ✓ |
| U4 different-marker bot comment only | no id | ✓ |
| U5 once-PATCHed fixed-form body | still matches contains() — updated in place, no stacking |
✓ |
| U6 empty pages | no id | ✓ |
U7 same marker from a non-github-actions[bot] login |
skipped (login filter load-bearing) | ✓ |
| U8 acks on two pages | last wins in slurped order |
✓ |
Witness: 02-upsert-jq-cells.png. U1–U8 (8/8) in 02-upsert-jq.mjs; step-level POST-vs-PATCH branch selection on both arms is in the A/B table (A12–A17).
Secondary claim — the guard and pin tests hold down what they claim
Each cell ran the real test file (or a single-point mutant of it — each mutant asserted to differ by exactly one line) via the repo's vitest config inside a fixture tree of all 48 workflows, cwd = fixture. -t filtered to the guard test except C0/C1/P-cells.
| cell | mutation | expected | observed |
|---|---|---|---|
| C0 | head tree, real test, full file | green | green — 105/105 |
| C1 | qwen-code-pr-review.yml = base tip |
red at the intended assertion | red — 2/105 (guard + pin test, both in this PR's describe); guard names the offender (snippet above) |
| M1 | glued ACK_BODY restored, PR's explanatory comment block kept | red | red, names qwen-code-pr-review.yml |
| M2 | ACK_BODY="<!\-\- m -->\n\n_x_" (double-quoted assignment, literal \n) |
red | red |
| M3 | adjacent literal '<!\-\- m -->''_x_' |
red | red |
| M4 | printf '%s%s' '<!\-\- m -->' '_x_' (declared gap: arg split) |
green | green |
| M5 | printf '<!\-\- m -->' "$VAR" (legit reference) |
green | green |
| M6 | glued marker planted in a second workflow | red naming that file | red, names zz-planted.yml |
| M7 | '<!\-\- m -->'$(gen_body) unquoted command substitution |
red | red |
| M8 | printf "<!\-\- m -->\n\n_x_" double-quoted format (exemption widened in round 2) |
green | green |
| M9 | $'<!\-\- m -->\n\n_x_' ANSI-C quoting |
green | green |
| M10 | printf 'x\n<!\-\- m -->prose' (declared gap: mid-literal) |
green | green |
| W1 | arrow marker '<!\-\- status >= done -->prose' (round-3 widening) |
red | red |
| W2 | W1 + test mutant narrowed back to [^>\n] |
green (miss ⇒ widening load-bearing) | green |
| W3 | clean tree + narrowed mutant | green (no FP either way) | green |
| W4 | jq 'contains("<!\-\- a -> b -->")' legit ref with > inside |
green (widening adds no FP) | green |
| G1 | BODY="$(printf '<!\-\- m -->')prose" (declared gap: subshell tail) |
green | green |
| G2 | glued marker quoted in a trailing end-of-line comment (declared FP) | red — flagged by design | red |
| G3 | body: "<!\-\- m -->\nprose" YAML double-quoted scalar (declared FP) |
red — flagged by design | red |
| G4 | unquoted marker in heredoc position (declared gap) | green | green |
| P1 | ack line [workflow run](%s) → [workflow run]()%s (round-3 shape pin) |
pin red, guard green | pin red, guard green |
| P2 | %s dropped, "$RUN_URL" kept |
pin red | pin red |
| P3 | "$RUN_URL" dropped, %s kept |
pin red | pin red |
| S1/S2 | bash semantics: %s with no arg; leftover arg with no %s |
exit 0, silent | exit 0 both — nothing else in the pipeline catches either, which is why the pin test exists |
Witness: 03-guard-mutation-matrix.png. 34/34 scripted checks in 03-guard-matrix.mjs. Vacuity controls: C1 fails against base at the intended assertion (expect(offenders).toEqual([]) with the offending line in the diff); positive controls are M1/W1 (harness provably goes red on the regression) and P1–P3 (pin test provably red on its three mutants). The round-3 dedupe (workflowFiles hoisted to module scope, shared with the expression-length describe) is covered by C0/T1 — all 105 tests including both consumers of the shared list are green.
Targeted gates
| gate | result |
|---|---|
| changed test file at repo root (T1) | green — 105/105 |
| full scripts suite (T2) | green — 950/950 across 50 collectible files; install-script.test.js uncollectible (no zip), proven environmental (T2b/T2c) |
bash -n on both extracted steps (T3) |
clean both arms |
| actionlint with repo flags on the changed workflow (T4) | clean; liveness-proven by a planted undefined property (T4b) |
| shellcheck with repo flags, env contract modeled (T5) | zero errors/warnings both arms; the one residue delta fully attributed (T5b, see Findings 2); liveness-proven (T5c) |
| marker census (T6/T6b/T6c) | 4 referencing files, no glued builder anywhere, head body keeps the marker on line 1 |
Witness: 04-targeted-gates.png. T1–T6c (15/15) in 04-gates.mjs.
Corrections
- Failure-snippet quote (stands from round 2). The description shows the guard failing on
mainwith"<!\-\- qwen-review-ack -->_Qwen Code review request accepted. Review is qu". The actual vitest diff at this head prints"qwen-code-pr-review.yml: <!\-\- qwen-review-ack -->_Qwen Code review request accept"(56-char slice from the marker). The test does fail on base and names the offending line; only the quoted truncation is stale. - Suite counts (stands from round 2, re-measured). "101/101", "50 files, 1054 passed" are the author's figures. At this head: changed file 105/105 (T1); full scripts suite 950/950 across 50 collectible files in this container (T2);
install-script.test.js's 72 tests are uncollectible withoutzip(environmental, T2b/T2c). The description's.github/scripts/qwen-triage-workflow.test.mjs: 56/56refers to a file this PR does not touch (empty diff) and which belongs to a different vitest root; it was not gated here. - Round-2 report's markdown-it mechanism (new this round). Round 2 described markdown-it's deviation as "closes the HTML block and re-parses the rest of the line". The observed counts were right; the mechanism was not. Measured at this head: markdown-it's default preset disables the html_block rule entirely (
html: false), so the glued line parses as an ordinary paragraph and renders 1/1; with{ html: true }markdown-it agrees with GitHub (0/0) (A26/A27). The description's own rendering claims are unaffected — they were always measured against GitHub's renderer.
Findings
No blocking findings. Informational notes:
- (info) Known gaps and known FPs are real and honestly documented. Every probeable member of the widened declaration was driven (table above): the four gaps (M4, M10, G1, G4) stay green exactly as declared; the two declared false positives (G2 trailing comment, G3 YAML scalar) do fire, exactly as declared. Completeness reporting only; no action required.
- (info) The new printf line carries one unbraced expansion. shellcheck with the repo's exact flags (env contract modeled) finds zero errors/warnings on both arms, and the only residue delta is attributable: head adds exactly one SC2250 style note because the new line writes
"$RUN_URL"where the base line wrote"${RUN_URL}"(T5b). The repo's shellcheck gate lints tracked.shfiles, never inline run blocks, so no gate is affected; bracing it is an optional nit. - (info) C1's red count moved from 7 (round 2) to 2. The base tip advanced since round 2, and the other 103 tests now all pass against it — the two remaining reds (guard + URL pin) are exactly this PR's two tests. Not a defect — context for anyone comparing rounds.
Not covered
- Per-commit attribution: the depth-2 checkout leaves only the merge commit reachable —
git rev-list HEAD^1..HEAD^2returns 1 (of the 8 commits in the metadata snapshot),git rev-parse --is-shallow-repository= true. The aggregateHEAD^1..HEADdiff was verified; the two delta commits' claims are covered behaviorally by the W/P/G cells. Per-commit separation was out of reach. - yamllint:
scripts/lint.js --setupinstalls it viapip3, which this container does not ship (install fails with permission denied). actionlint 1.7.12 + shellcheck 0.11.0 +bash -nall ran (liveness-proven). - The 72 tests inside
install-script.test.js: not collectible here (module-scope guard throws whenzipis absent; no root to install it). Proven environmental, not a regression: the file is byte-identical base vs head and the collection error nameszip(T2b/T2c). .github/scripts/qwen-triage-workflow.test.mjs("56/56" in the description): unchanged by this PR (empty diff); belongs to the.github/scriptsvitest root, not the scripts suite gated here.- Live comment round-trip (
ghPOST/PATCH → storedbody_html): no GitHub token by design. The A/B reproduces the wire shape through the same renderer endpoint the author measured with (rendering half) and the step-level cells exercise lookup/PATCH branch selection against a stub peer (lookup half); the storage half remains unexercised. - Windows/macOS: N/A — the change is a Linux-bash CI step, matching the PR's Tested-on table.
POST /markdownthrottling: ~8 anonymous calls succeeded this round (one failed 422 on a harness bug, fixed); a throttled rerun would need a token.
Methodology
Environment: the CI verify container (node:22-bookworm, node v22.23.2, jq present, no zip/pip3; POST api.github.com/markdown anonymously reachable — used as the primary rendering oracle; actionlint/shellcheck installed via node scripts/lint.js --setup at the repo-pinned versions). Both arms' ack steps were extracted with the repo's yaml package from git show HEAD^1:… / HEAD:… and executed under bash --noprofile --norc with a stub gh on PATH recording argv (NUL-separated) at the wire boundary; rendering cells used GitHub POST /markdown (mode=gfm, repo context) corroborated by marked and markdown-it from the repo's node_modules. The guard/pin matrix ran the real test file (or single-point mutants, each asserted one line apart) via the repo's vitest config in per-cell fixture trees built from git archive HEAD .github plus scripts/tests/{test file, test-setup.ts} and packages/core/src/utils/errorParsing.ts (cwd = fixture; module resolution walks up to the root node_modules). Gates: changed-file and full scripts suites at the repo root, bash -n on both extracted steps, actionlint and shellcheck with the repo's exact flag sets (each liveness-proven by a planted violation), and a repo-wide marker census (4 files reference the ack marker — two workflows, the test file, and a JSDoc/filter-prefix in cleanup.ts; no glued builder anywhere, and the head wire body still opens with the marker on line 1 as that consumer anchors). Evidence captures were produced with scripts/verify-capture.mjs. Harnesses, per-cell JSON/logs, and the four captures live in tmp/pr8726-verify-20260809-054238/ (01-ab-body.mjs, 02-upsert-jq.mjs, 03-guard-matrix.mjs, 04-gates.mjs, logs/, evidence/); the per-cell fixture trees are rebuilt by 03-guard-matrix.mjs on every run and were swept after capture to keep the artifact small.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
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. ✅
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round: no action taken — PR #8726This round found no actionable feedback, so no code changes were made and no commit was created. Triage summary
Critical-only modeCritical-only mode is active after 5 change-producing rounds. The single deferred non-Critical item (a VerificationNo code changes were made this round, so no build/typecheck/lint/test commands were run. The working tree is unmodified ( 中文说明Autofix 评审轮次：未采取行动 — PR #8726本轮未发现任何可处理的反馈，因此未做任何代码改动，也未创建提交。 分类摘要
仅 Critical 模式在完成 5 个产生改动的轮次后，仅 Critical 模式已激活。唯一被延后的非 Critical 条目（ 验证本轮未做任何代码改动，因此未运行 build/typecheck/lint/测试命令。工作树未被修改（ Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 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-review at head 9bb5a8f after the hardening rounds: the core fix is intact (blank line between the marker and the prose, built via printf with RUN_URL passed as an argument, not a format), and the marker guard is properly hardened — it keys off a marker opening a string literal and what remains inside that literal, stays bounded to the physical line, and the residual known gaps are explicitly stated in the comment rather than papered over. The ack link shape is pinned and the workflow scan is deduped. My earlier approval was dismissed by the force-pushes, so re-approving. CI green on this head. Nothing blocks merge.
|
Released in v0.21.9. |
4 participants
What this PR does
Puts a blank line between the
<!-- qwen-review-ack -->marker and the prose of the queued-acknowledgement comment, so the comment actually renders.Why it's needed
The ack comment posted on every PR that requests a review was built as:
A line opening with
<!--starts an HTML block, and that block runs to the line containing the closing delimiter inclusive — the rest of that line is still inside it and is never parsed as Markdown. The comment therefore shipped as raw source: no emphasis, and the link to the workflow run was dead text.Live example: #8687 (comment). Its
body_htmlfrom the API is the literal source string, with no<a>and no<em>.That link matters more than it looks.
issue_commentruns are not attached to a commit, so a review triggered by@qwen-code /reviewnever appears in the PR's checks list — this comment is the only pointer the author has to their run. With the link dead, there is no path from the PR to the review.Measured, not assumed
Through GitHub's own renderer (
POST /markdown,mode=gfm, repo context), on the exact bodies:<a><em>\n+ text\n\n+ textThe first row reproduces what shipped, which is what makes the other two trustworthy. This PR uses the blank-line form, matching how
autofix-statusalready builds its body.Reviewer Test Plan
How to verify
Expected: 101/101. Full scripts suite: 50 files, 1054 passed.
.github/scripts/qwen-triage-workflow.test.mjs: 56/56.Render the new body yourself:
Now returns
<em>…<a href="…">workflow run</a>…</em>; with the marker and text on one line it returns the source text.Evidence (Before & After)
Before —
body_htmlof the live comment on #8687 is plain source, no tags. After — the same body through the same renderer:The new test scans every marker in the workflow and rejects any with prose glued to it (skipping comment lines, since the rule is explained with an example of the bad form). It fails against
mainand names the offending line:Tested on
Risk & Scope
contains("<!-- qwen-review-ack -->")upsert lookup still finds prior acks — existing comments get PATCHed into the fixed form the next time their PR triggers a review, rather than stacking a second comment.Linked Issues
Spotted on #8687 while re-triggering the PRs stranded by the #8648 outage.
中文说明
What this PR does
在
<!-- qwen-review-ack -->标记与后面的正文之间加一个空行，让这条"已排队"确认评论能够正常渲染。Why it's needed
每个请求评审的 PR 上都会发一条 ack 评论，它原本是这样拼出来的：
以
<!--开头的行会启动一个 HTML block，而这个 block 一直延续到包含结束定界符的那一行——该行的剩余部分仍然在 block 内部，永远不会被当作 Markdown 解析。于是这条评论是以原始文本形式发出去的：没有斜体，指向 workflow run 的链接也是死的。线上实例：#8687 (comment)。通过 API 取它的
body_html，得到的就是那串字面源码，既没有<a>也没有<em>。这个链接比看上去更重要。
issue_comment触发的 run 不挂在任何 commit 上，因此通过@qwen-code /review触发的评审根本不会出现在 PR 的 checks 列表里——这条评论是作者找到自己那次 run 的唯一入口。链接一死，就没有任何从 PR 通往评审的路径了。实测，而非假设
用 GitHub 自己的渲染器（
POST /markdown，mode=gfm，带仓库上下文）跑这几种确切的 body：<a><em>\n+ 正文\n\n+ 正文第一行复现了线上实际发出的效果，这正是后两行结果可信的依据。本 PR 采用空行形式，与
autofix-status既有的拼装方式一致。Reviewer Test Plan
How to verify
预期 101/101。scripts 全量套件：50 个文件、1054 passed。
.github/scripts/qwen-triage-workflow.test.mjs：56/56。也可以自己渲染一下新的 body：
现在会返回
<em>…<a href="…">workflow run</a>…</em>；而把标记和正文放在同一行时，返回的就是那串源文本。Evidence (Before & After)
Before —— #8687 上那条线上评论的
body_html是纯源码，没有任何标签。After —— 同一个 body 经同一个渲染器：新测试会扫描 workflow 中的每一个标记，拒绝任何把正文直接粘在标记后面的写法（跳过注释行，因为这条规则本身就是用一个反面示例来解释的）。它在
main上会失败并精确点出问题行：Tested on
Risk & Scope
contains("<!-- qwen-review-ack -->")的 upsert 查找仍能找到既有的 ack——现存评论会在其 PR 下次触发评审时被 PATCH 成修好的形式，而不会再叠一条新评论。Linked Issues
在重新触发被 #8648 停摆卡住的 PR 时，于 #8687 上发现。