fix(ci): keep the review workflow under the expression-length limit - #8720
Conversation
The review workflow has been invalid since #8648 merged, so every event it declares has been dead for ~12 hours: Invalid workflow file: .github/workflows/qwen-code-pr-review.yml#L1 (Line: 751, Col: 14): Exceeded max expression length 21000 A `run:` body containing `${{ }}` is evaluated as ONE expression template, and GitHub caps a single expression at 21000 characters. "Run review" went 17705 -> 22282 chars in #8648 (17:00:32 on 2026-08-07); the first startup failure is stamped 17:00:50. #8683 took it to 24042. An over-limit expression does not fail a job — it invalidates the whole file, so no run is created at all. Across the 400 runs since that merge there is not one success, not one `pull_request_target` and not one `issue_comment`: both automatic review and `@qwen-code /review` were unreachable, while CI stayed green throughout because nothing covered it. Pass the three context values the script reads through the step's env, leaving the body free of `${{ }}`. The runner then never templates it and its length stops mattering. No behaviour changes: each substitution is a rename of the same value. Pin it with a test that walks every workflow and fails any templated run block over the limit, plus one that keeps this body untemplated — it is past 21000 on its own, so a single `${{ }}` added back takes the whole workflow down again. Both fail against the file currently on main, naming it: `qwen-code-pr-review.yml > review-pr > Run review: 24042 chars`. Mutation-tested, 4 of 4 caught: restoring a `${{ }}` in the body (3 tests), dropping either env binding, and hardcoding the value the env used to carry.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the quick fix! Template ✓ — all sections present, bilingual summary included. Problem: real, and independently verified. The review workflow has been dead since #8648. I checked the run history myself: the last successful run was 2026-08-07 16:58 UTC — two minutes before #8648 merged at 17:00:32 — and every run since has been a startup failure (31239579253 among them, concluding Direction — this restores broken core CI infrastructure; there is no direction question here, only urgency. Size — 17 production lines in the workflow file (+13/−4); the other two files are tests (+72/−7). No core-package paths touched, so the two-tier core gate does not apply. Approach — the minimal fix: move the three context values ( Risk — no high-risk path matches; no elevated risk signals. Moving on to code review. 🔍 中文说明感谢快速修复! 模板 ✓ —— 各节齐全,含中文摘要。 问题真实存在,且已独立核实。 评审 workflow 自 #8648 起已失效。我自查了 run 历史:最后一次成功是 2026-08-07 16:58 UTC——比 #8648 于 17:00:32 合入早两分钟——此后每一条 run 都是启动失败(包括 31239579253,报 方向 —— 这是修复损坏的核心 CI 基础设施,方向没有问题,只有紧迫性。 规模 —— workflow 文件生产代码 17 行(+13/−4),其余两个文件为测试(+72/−7)。未触及核心包路径,两级核心门禁不适用。 方案 —— 最小修复:把三个上下文值改由 step 的 风险 —— 未命中高风险路径;无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewReviewed statically at the head commit (per gate policy, PR code is never executed in this environment). No blockers found.
One non-blocking observation: the Testing (CI evidence at
|
| Check | Conclusion |
|---|---|
Classify PR |
✅ success |
Desktop Shell (ubuntu-22.04) |
✅ success |
Desktop Shell (windows-2022) |
✅ success |
Post Coverage Comment (ubuntu-latest, 22.x) |
✅ success |
Test (ubuntu-latest, Node 22.x) |
✅ success |
web-shell E2E Smoke (ubuntu-latest, Node 22.x) |
✅ success |
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。
Beyond the unit suite, there is already live evidence that GitHub accepts the fixed file: for pull_request events GitHub parses the workflow from the PR's merge ref, and an invalid file produces a startup-failure run on every such event — which is exactly the pattern of the last 12 hours (e.g. feat/workflow-policy-layer at 05:24 UTC, still failing). This PR's own pull_request event (opened 05:29 UTC) produced no startup-failure run: with the fix in the merge ref the file parses, and having no matching trigger it correctly creates no run at all. Validity is observable; the full behavioral confirmation will be the first automatic review that fires after merge, since pull_request_target evaluates the workflow from the base branch. No sandboxed lane (/verify, /tmux) applies here — both exercise CLI behavior, not workflow-file acceptance, so there is nothing they could add. The author's reported 129/129 local run and mutation-testing table are their own account and were not re-run in this environment.
Real-scenario (tmux) testing: N/A — CI-infrastructure change with nothing user-visible; unattended CI run.
中文说明
代码审查
按门禁规则在 head commit 上静态审查(此环境不执行 PR 代码)。未发现阻塞问题。
- 关键不变量成立。 head 版本的
Run review正文(约 761–1336 行)含有零个${{——新env:绑定(758–760 行)之后的下一个表达式已属于下一个 step(1337 行)。正文无可插值内容,21000 字符表达式上限对它完全不再适用。 - 语义保持不变。
MAX_TIMEOUT_MINUTES与大 PR 分支的EFFECTIVE_TIMEOUT_MINUTES改读$MAX_TIMEOUT_MINUTES_VAR;过期 run 守卫改读$EVENT_NAME/$EVENT_HEAD_SHA。EVENT_HEAD_SHA的三处读取仍全部位于pull_request_target分支内;其他事件下该 env 为"有定义的空值",与旧的内联展开逐字节一致,set -u行为不变。 - 沿用既有约定而非新发明。 通过 step
env:传上下文在本文件已有先例(contextjob 就以同样方式传EVENT_NAME),绑定上方新增的注释也为后续贡献者写明了约束。 - 测试三重锁定回归。 仓库级守卫:任何模板化且超过 21000 字符的
run块都会失败;专项测试:Run review正文必须保持无模板;resolve 测试同时断言两半——正文读取$MAX_TIMEOUT_MINUTES_VAR及其 env 绑定——任一半无法单独漂移。新增的indexOf(...) !== -1保护修复了切片中一个真实的潜在 bug。新测试所需的 import(parse、readdirSync、readFileSync、join)文件中已存在。
一个非阻塞观察:同文件中 /resolve 流程的 Resolution check run 块仍是模板化的,且约 14K 字符,已超过 21000 上限的一半。新的仓库级守卫把它从静默风险变为受监控风险,但若 resolve 流程继续增长,它就是下一个触顶候选。PR 描述中"目前没有其他 run 块超过上限的一半"对该块不成立。用同样方式为它去掉模板是很好的后续跟进,不必扩大本 PR。
测试(6c453a9 的 CI 证据)
经 checks API 获取;此处未复跑任何内容。目前无失败——执行新 workflow 测试的 Linux 单测套件仍在进行中,下表为实时快照,CI 落定后由 finalize 任务刷新。
(机器可读表格见英文部分,此处不重复。)
除单测套件外,已有线上证据表明 GitHub 接受了修复后的文件:对 pull_request 事件,GitHub 以 PR 的合并 ref 解析 workflow,文件无效时每次事件都会产生一条启动失败 run——这正是过去 12 小时的形态(如 feat/workflow-policy-layer 于 05:24 UTC 仍在失败)。而本 PR 自己的 pull_request 事件(05:29 UTC 开启)没有产生启动失败 run:合并 ref 中已含修复,文件解析成功,又没有匹配的触发器,于是正确地不创建任何 run。有效性已可观测;完整的行为确认将是合入后触发的第一次自动评审,因为 pull_request_target 以 base 分支的 workflow 为准。沙盒通道(/verify、/tmux)在此不适用——两者验证的都是 CLI 行为而非 workflow 文件可接受性,无信号可补。作者报告的本地 129/129 与变异测试表格为作者自述,未在此环境复跑。
真实场景(tmux)测试:N/A——纯 CI 基础设施改动,无用户可见行为;本次为无人值守 CI 运行。
— Qwen Code · qwen3.8-max
Reviewed at 6c453a9dc30d590d08b99786be566c93b505820c · re-run with @qwen-code /triage
|
Confidence: 4/5 — clean, minimal, root cause independently confirmed; the missing point is the still-running unit suite and the one follow-up named below, not any doubt about the change. Stepping back: this is exactly the fix I would have proposed. The workflow broke because a templated My one reservation is the one named in Stage 2: the Verdict: approve. The Linux unit suite (the check that runs the new tests) is still in progress on 中文说明置信度:4/5 —— 干净、最小化、根因已独立确认;未给满分是因为单测套件尚未出结果以及下述一项跟进,而非对改动本身有疑虑。 退一步看:这正是我会提出的修复。workflow 损坏是因为模板化的 唯一的保留意见已在 Stage 2 点名:同文件中 结论:批准。运行新测试的 Linux 单测套件在 — Qwen Code · qwen3.8-max Reviewed at |
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. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qqqys
left a comment
There was a problem hiding this comment.
Approving. I verified the premise and the fix independently rather than taking the description on trust, and both hold up.
Verified
- On
maintoday,review-pr›Run reviewis 24,042 characters with 4${{ }}— over the 21000 single-expression cap, so the whole workflow file is invalid, exactly as described. On this branch the same body is 23,930 characters with 0${{ }}, so the cap no longer applies to it at all. - All three substitutions land outside the
<<'QWEN_GH_WRAPPER'heredoc (lines 831–927 of the new file). That is the trap this class of change usually springs: inside a quoted heredoc GitHub still substitutes${{ }}but bash does not expand$VAR, so the swap would silently change behaviour. It doesn't here. EVENT_HEAD_SHAis now visible for the whole script instead of just thepull_request_targetbranch, but its only three uses (the staleness compare, the skip message, andEXPECTED_HEAD_SHA) are all still inside that branch, andset -uis unaffected since a stepenv:entry is always defined.- 129/129 on the two named files. This branch's base is identical to
mainfor every file it touches, and it merges clean. - The repo-wide guard's headroom claim checks out: across all 47 workflow files the largest templated
runblock is 8,141 chars (resolve-pr›Report result), so the new test is a guard rather than a backlog.
Two test-only notes, neither blocking
-
scripts/tests/qwen-resolve-workflow.test.js:551— the slice's start bound gets the new-1guard, but the end bound on the next line keeps the unguardedindexOf:const staleHeadCheck = runStep.slice( staleHeadStart, runStep.indexOf('PROMPT="/review ${REVIEW_URL}"'), // no -1 guard );
If that literal is ever reworded,
indexOfreturns-1andslice(start, -1)widens from thepull_request_targetbranch to nearly the whole 24k step. The threestaleHeadCheckassertions then pass against the entire script — including the bareexit 0, which occurs many times in it — and the scoping the slice exists for is gone with the suite still green. This is the same defect the PR just fixed one line above, so it's worth closing while you're in there. -
scripts/tests/qwen-pr-review-workflow.test.js:2178— the net walks.github/workflowsonly, sorun:blocks in composite actions under.github/actions/*/action.ymlsit outside it. No live gap (largest is 2,888 chars,post-coverage-comment), but it's the same growth pattern that tookRun reviewfrom 17,705 to 24,042 across two PRs, and GitHub rejects an over-limit composite action the same way. Cheap to fold into the same walk if you want the net to be genuinely repo-wide.
Neither note affects the fix. The regression test is the valuable half of this PR — the outage lasted ~12h with CI green precisely because nothing covered this.
中文
Approve。结论和修复我都独立复核过,成立。
已验证
- 当前
main上review-pr›Run review正文是 24,042 字符、含 4 处${{ }},确实超过 21000 的单表达式上限,整个 workflow 文件处于 invalid 状态;本分支同一正文 23,930 字符、0 处${{ }},上限对它不再适用。 - 三处替换全部落在
<<'QWEN_GH_WRAPPER'引号 heredoc(新文件 831–927 行)之外。这正是这类改动最容易踩的坑:引号 heredoc 内 GitHub 仍会替换${{ }},而 bash 不会展开$VAR,一旦落在里面就是静默的行为变化。这里没有。 EVENT_HEAD_SHA的可见范围从pull_request_target分支扩大到整个脚本,但它仅有的三处使用(新旧 head 比较、跳过信息、EXPECTED_HEAD_SHA)都仍在该分支内;set -u不受影响,因为 stepenv:条目总是有定义。- 指定的两个测试文件 129/129。本分支 base 在所涉文件上与
main完全一致,合并无冲突。 - 仓库级守卫的余量说法属实:47 个 workflow 文件中最大的模板化
run块是 8,141 字符(resolve-pr›Report result),所以新测试是守卫而非待办清单。
两点测试层面的意见,均不阻塞
scripts/tests/qwen-resolve-workflow.test.js:551—— 切片的起点加了新的-1保护,但下一行的终点仍是无保护的indexOf。该字面量一旦改写,indexOf返回-1,slice(start, -1)就从pull_request_target分支扩大到几乎整个 24k step,三条staleHeadCheck断言(包括在脚本中出现多次的裸exit 0)会对全文通过,切片本身的约束力静默消失。这与本 PR 上一行刚修好的正是同一类问题,顺手一起收掉比较好。scripts/tests/qwen-pr-review-workflow.test.js:2178—— 只遍历.github/workflows,.github/actions/*/action.yml里的run:块不在网内。目前没有实际缺口(最大 2,888 字符,post-coverage-comment),但这与Run review两个 PR 从 17,705 涨到 24,042 是同一种增长方式,且 GitHub 对超限的 composite action 同样会直接判无效。想让这张网名副其实的话,并入同一次遍历成本很低。
两点都不影响修复本身。这个 PR 更有价值的一半是回归测试——这次故障持续约 12 小时而 CI 全绿,恰恰是因为此前没有任何测试覆盖它。
wenshao
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查。 建议见行内评论。 未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| const staleHeadCheck = runStep.slice( | ||
| runStep.indexOf( | ||
| 'if [ "${{ github.event_name }}" = "pull_request_target" ]; then', | ||
| ), | ||
| staleHeadStart, | ||
| runStep.indexOf('PROMPT="/review ${REVIEW_URL}"'), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The stale-head slice gains a -1 guard for its start anchor, but its end anchor stays unguarded — and this is the one slice in the family where losing the anchor keeps every assertion green. The siblings protect themselves: smallPrArm/belowMaxArm carry not.toContain assertions that fail on an over-expanded slice, and the at-max arm explicitly guards its end anchor (expect(atMaxEnd).toBeGreaterThan(-1)). This slice has only positive assertions, so it is the lone exception — exactly the silent-degradation mode the comment above describes. — Failure scenario: if PROMPT="/review ${REVIEW_URL}" is ever reworded (the workflow already builds PROMPT conditionally with --effort/--comment suffixes, so churn is plausible), indexOf returns -1, slice(staleHeadStart, -1) silently expands to the entire rest of the run body, and all three positive toContain assertions still pass — the ordering the test name pins ("before invoking qwen") silently dies. Probe-confirmed: rewording the anchor kept the test green; adding the guard below flipped it to a hard failure under the same mutation.
| const staleHeadCheck = runStep.slice( | |
| runStep.indexOf( | |
| 'if [ "${{ github.event_name }}" = "pull_request_target" ]; then', | |
| ), | |
| staleHeadStart, | |
| runStep.indexOf('PROMPT="/review ${REVIEW_URL}"'), | |
| ); | |
| const promptStart = runStep.indexOf('PROMPT="/review ${REVIEW_URL}"'); | |
| expect(promptStart).toBeGreaterThan(staleHeadStart); | |
| const staleHeadCheck = runStep.slice(staleHeadStart, promptStart); |
中文说明
这个 stale-head 切片为起始锚点新增了 -1 保护,但结束锚点仍未加保护——而在整个切片家族中,这是唯一一个丢失锚点后所有断言依然全绿的切片。兄弟切片都有自保护:smallPrArm/belowMaxArm 带有 not.toContain 断言,切片过度展开时会失败;at-max 分支显式保护了结束锚点(expect(atMaxEnd).toBeGreaterThan(-1))。这个切片只有正向断言,因此是唯一的例外——恰恰就是上方注释所描述的静默退化模式。失败场景:如果 PROMPT="/review ${REVIEW_URL}" 将来被改写(workflow 中 PROMPT 本来就带条件拼接,如 --effort/--comment 后缀,改动是很可能发生的),indexOf 返回 -1,slice(staleHeadStart, -1) 会静默扩展到 run 正文的整个剩余部分,三个正向 toContain 断言全部照常通过——测试名称所钉住的顺序("在调用 qwen 之前")就悄无声息地失效了。已通过探针验证:改写锚点后测试仍为绿;加上以下保护后,同样的变异会变为硬性失败。
— qwen3.8-max via Qwen Code /review (v0.21.7)
|
Released in v0.21.8. |
What this PR does
Passes the three workflow-context values the review script reads through the step's
env:, so therun:body contains no${{ }}.Why it's needed
The review workflow is invalid right now, and has been for ~12 hours. Every run since #8648 merged is a startup failure:
A
run:body that contains${{ }}is evaluated as one expression template, and GitHub caps a single expression at 21000 characters. Line 751 is therun: |-ofreview-pr›Run review:Run reviewbodyefc7ec7a85(before #8648)f4802031d0(#8648, 2026-08-07 17:00:32)ee2e5be666(#8683)The first startup failure is stamped 17:00:50 — 18 seconds after #8648 merged.
An over-limit expression does not fail a job. It invalidates the whole file, so for the events that matter no run is created at all. Across the 400 runs since that merge:
pull_request_target(automatic review)issue_comment(@qwen-code /review)pushpull_requestZero successes. Both the automatic review and the manual
@qwen-code /reviewescape hatch were unreachable, and CI stayed green the entire time because nothing covered this.Reviewer Test Plan
How to verify
Expected: 129/129. Full scripts suite: 1053 passed, 50 files.
To see the live failure, check out
mainand run the same command — the new tests fail and name it:To see GitHub's own error, any recent run of this workflow will do, e.g. 31239579253.
Evidence (Before & After)
N/A for UI. Before is the error above and the run census; after is 129/129 with the
Run reviewbody at 23,930 characters and 0${{ }}inside it, so the limit no longer applies to it at all.Mutation-tested — 4 of 4 caught:
${{ }}back in the run bodyenvstops readingvars.QWEN_REVIEW_MAX_TIMEOUT_MINUTESEVENT_HEAD_SHAenv bindingThe first is the one that matters: it is the exact regression this PR fixes, and it is now caught three ways.
The two new tests are complementary. One walks every workflow in
.github/workflowsand fails any templatedrunblock over 21000 — a repo-wide net (nothing else is currently above half the limit, so this is a guard, not a backlog). The other keeps this specific body untemplated, because at ~24,000 characters it is past the limit on its own: one${{ }}added back takes the whole workflow down again.Three assertions in
qwen-resolve-workflow.test.jspinned the old literals and were updated to assert both halves — the body reading$MAX_TIMEOUT_MINUTES_VARand the env binding it to the repository variable — so neither can drift alone. One of them sliced on anindexOfwith no-1guard; that is fixed too.Tested on
Risk & Scope
set -uis unaffected because a stepenv:entry is always defined (empty when the context value is empty, exactly as the inline expansion was). TheEVENT_HEAD_SHAenv is visible for the whole script rather than only inside thepull_request_targetbranch, but all three of its uses are inside that branch, unchanged.Run reviewstays ~24,000 characters; it is simply no longer templated. Extracting it to a helper script remains worth doing separately.Linked Issues
Regression from #8648. Surfaced while investigating why #8708 never got an automatic review — it never could have, and neither could any other open PR.
中文说明
What this PR does
把评审脚本用到的三个 workflow 上下文值改为通过 step 的
env:传入,使run:正文中不再含有${{ }}。Why it's needed
这个评审 workflow 目前是无效状态,且已持续约 12 小时。 自 #8648 合入以来,每一次 run 都是启动失败:
含有
${{ }}的run:正文会被当作一个表达式模板求值,而 GitHub 对单个表达式的上限是 21000 字符。第 751 行正是review-pr›Run review的run: |-:Run review正文efc7ec7a85(#8648 之前)f4802031d0(#8648,2026-08-07 17:00:32)ee2e5be666(#8683)第一条启动失败的时间戳是 17:00:50——#8648 合入后 18 秒。
超限的表达式并不会让某个 job 失败,而是让整个文件失效,因此对关键事件而言根本不会创建 run。自那次合入以来的 400 条 run 中:
pull_request_target(自动评审)issue_comment(@qwen-code /review)pushpull_request零成功。自动评审和手动
@qwen-code /review这条兜底路径同时不可达,而 CI 全程是绿的——因为此前没有任何测试覆盖这一点。Reviewer Test Plan
How to verify
预期 129/129。scripts 全量套件:50 个文件,1053 passed。
想看到线上的真实失败,切到
main跑同样的命令——新测试会失败并直接点名:想看 GitHub 自己给出的报错,随便打开这个 workflow 最近的任意一条 run 即可,例如 31239579253。
Evidence (Before & After)
界面部分 N/A。Before 即上面的报错与 run 统计;After 是 129/129,
Run review正文 23,930 字符、其中${{ }}数量为 0,因此该上限对它完全不再适用。变异测试 —— 4 个全部被捕获:
${{ }}加回 run 正文env不再读取vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTESEVENT_HEAD_SHA的 env 绑定第一个才是重点:它正是本 PR 修复的那个回归,现在有三重拦截。
两个新测试互补。一个遍历
.github/workflows下所有 workflow,任何被模板化且超过 21000 的run块都会失败——这是仓库级的兜底网(目前没有其他 run 块超过上限的一半,所以它是守卫而非待办清单)。另一个专门保证这段正文不被模板化,因为它本身约 24,000 字符已经超限:只要加回一个${{ }},整个 workflow 就会再次失效。qwen-resolve-workflow.test.js中有三处断言钉的是旧字面量,已更新为同时断言两半——正文读取$MAX_TIMEOUT_MINUTES_VAR以及 env 绑定到仓库变量——这样任何一半都无法单独漂移。其中一处用indexOf切片且没有-1保护,一并修好。Tested on
Risk & Scope
set -u不受影响,因为 stepenv:条目总是有定义(上下文值为空时即空串,与原先的内联展开完全一致)。EVENT_HEAD_SHA这个 env 在整个脚本可见,而不再局限于pull_request_target分支内,但它的三处使用都在该分支内,未作改动。Run review仍是约 24,000 字符,只是不再被模板化。把它抽成独立的辅助脚本仍然值得单独去做。Linked Issues
#8648 引入的回归。是在排查 #8708 为何一直没有自动评审时发现的——它根本不可能有,其他任何 open PR 也一样。