fix(review): stop the agent transcript from executing workflow commands - #8683
Merged
Conversation
The review agent streams its entire transcript to stdout, and the runner scans every line for workflow commands. A tool result that quotes a file containing one therefore gets EXECUTED. Observed on run 31167034020 (PR #8681). That PR changes an `actions/setup-node` input, so the agent read the action's own main.ts, which legitimately contains: core.info(`##[add-matcher]${path.join(matchersPath, 'tsc.json')}`); The runner took the rest of the JSON line as a matcher path: ##[error]Unable to process command '{"type":"user","uuid":...' successfully. ##[error]The path '...' is too long, or a component of the specified path is too long. Three of those, and the step failed after 1h37m — a full review discarded for quoting a file. Nothing about that PR is unusual: any review whose transcript quotes `##[...]` or `::...::` breaks the same way, including every review of this repository's own workflows. Wrap the agent invocation in `::stop-commands::`, with a token randomised per attempt so no output the agent produces can guess it and re-enable parsing early. Parsing resumes immediately after PIPESTATUS is captured: `echo` clobbers PIPESTATUS, so resuming any earlier would read the echo's status instead of the agent's and report every timeout or crash as a clean run. Resuming is on the errexit-disabled straight line, so it is reached on success, crash and timeout alike — leaving it off would silence the job's own ::error:: and the fallback comment's diagnostics for the rest of the run. Tested by driving the real extracted retry loop with a stub agent that emits `##[add-matcher]`, asserting the bracket contains it, that the token is random rather than fixed, and that parsing resumes on success, hard exit and timeout. Mutation-checked: removing the guard, never resuming, resuming before the status capture, and using a fixed token each fail.
Collaborator
Collaborator
|
Re-run after the author pushed the requested fixes (
Moving on to code review. 🔍 中文说明作者推送修复后的重跑（
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewHead moved 1. Resume swallowed by a partial trailing line — fixed. The resume is now 2. Ordering assertions got teeth — fixed. All four anchors (stop line, agent invocation, 3. Per-attempt randomness pinned — fixed. 4. Agent produces output and then dies — fixed. The new The new lines read correctly in context: the token is generated under Non-blocking, carried over from the last pass — fine as follow-ups, not blockers:
Testing evidence — the PR's own CI (unattended run; no PR code executed here)
The scripts suite ran inside the green ubuntu Test job — confirmed from the job log itself, not the PR description: Not verified, and by design not locally verifiable: the live GitHub runner honoring the bracket end-to-end. That path exists only on GitHub's runners and activates first after merge ( 中文说明代码审查Head 已从 1. resume 被残行吞掉——已修复。 resume 改为 2. 顺序断言长出了牙齿——已修复。 四个锚点（stop 行、agent 调用、 3. 逐次尝试的随机性被钉住——已修复。 4. agent 先产出再死亡的场景——已修复。 新增 新增代码在上下文中读起来正确：token 在 非阻断项，自上一轮延续——作为后续跟进即可，不阻塞：
测试证据——PR 自己的 CI（无人值守运行；此处未执行任何 PR 代码）本 head 的 scripts 套件在绿色的 ubuntu Test 任务中真实运行——来自任务日志本身而非 PR 描述： 未验证、且按设计无法本地验证：真实 GitHub runner 端到端遵守括号。该路径只存在于 GitHub runner 上，且合并后才首次生效（ — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — all four round-1 findings fixed and pinned by tests that execute the real workflow bash; CI green on this head; the remaining items are follow-up material, not blockers. My independent proposal for "the transcript executes workflow commands" was exactly this shape — a The 4/5, not 5/5, is the named nits: History for the thread: the bot's earlier approval (round 1, commit Approving, pinned to the reviewed commit. ✅ 中文说明置信度：4/5 —— 第一轮的全部四个发现均已修复，且由执行真实 workflow bash 的测试钉住；本 head 的 CI 为绿；剩余事项属于后续跟进，不构成阻塞。 我对"会话内容执行了工作流命令"的独立方案正是这个形态——用逐次尝试随机 token 的 给 4/5 而非 5/5，是因为这些点名的瑕疵： 线程历史备忘：bot 第一轮在 批准，钉在被审查的提交上。✅ — Qwen Code · qwen3.8-max Reviewed at |
Contributor
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
approved these changes
Aug 7, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
Collaborator
Author
Code reviewRight fix for a real incident, and bracketing only the agent (rather than the whole step) is the correct scope. One correctness gap in the resume, and the three new tests have less grip than they look. I checked the parser behaviour against
That is why the incident fired from inside a JSON line: 1. The resume can be swallowed by a partial trailing line (
|
Collaborator
Author
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
Collaborator
|
🤝 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 冲突，直到移除标签或达到轮次上限。移除 |
Collaborator
|
🤖 AutoFix crashed before it could evaluate the feedback (attempt 1/100) — 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/31178493535 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
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)
Collaborator
|
Qwen Code review did not complete successfully. Run review failed. See workflow logs for details. A transient error is retried automatically; if you are seeing this, retry with |
Collaborator
|
🤖 AutoFix crashed before it could evaluate the feedback (attempt 2/100) — 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/31181591158 🧠 Handled by Qwen Code · model/模型 |
Collaborator
|
🤖 AutoFix crashed before it could evaluate the feedback (attempt 3/100) — 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/31184163929 🧠 Handled by Qwen Code · model/模型 |
Collaborator
|
🤖 AutoFix crashed before it could evaluate the feedback (attempt 4/100) — 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/31186520332 🧠 Handled by Qwen Code · model/模型 |
Collaborator
|
🤖 AutoFix stopped after 5 consecutive rounds that failed to push anything (timeouts and/or gate rejections). Retrying at the same per-round budget is not converging — this usually means the PR is too large or conflicts with a fast-moving 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/31188562190 🧠 Handled by Qwen Code · model/模型 |
Collaborator
|
⏸️ Takeover paused: this PR reached its round cap (100/100). Comment 中文说明⏸️ 托管已暂停：本 PR 达到轮次上限（100/100）。评论 |
Collaborator
Author
|
@qwen-code /triage |
Collaborator
|
Sandboxed verification: The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. 中文 — 判定：
|
qwen-code-ci-bot
requested changes
Aug 7, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Needs the four fixes from the review thread — all independently re-verified against the current head, details in my stage-2 comment: the resume needs a leading newline so a SIGKILL'd final line can't swallow it, the stop-before-agent assertion needs a -1 guard, token randomness needs a two-attempt comparison, and the suite needs an output-then-die scenario. The bones are right — once those land this is an approve. 🙏
qwen-code-ci-bot
requested changes
Aug 7, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
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)
Collaborator
|
Qwen Code review did not complete successfully. Run review failed. See workflow logs for details. A transient error is retried automatically; if you are seeing this, retry with |
Round-2 review follow-ups on the stop-commands guard. The resume was `echo`d, so a `--kill-after` SIGKILL that cut the agent off mid-line appended it to that fragment. The runner matches `::cmd::` at a line start only, so parsing stayed off for the rest of the job — losing the retry `::warning::` and every later diagnostic on the one path the guard exists to survive. Emit it with a leading newline. The ordering assertions had no teeth: `indexOf` returns -1 when a line is deleted or reworded, and -1 satisfies `toBeLessThan`. Deleting the stop line left the suite green. Every anchor is now asserted present. Cover the outcomes no scenario reached: an agent that streams and then dies (the stub `timeout` exited before ever running it), a failing log write (the only early return left unpinned), and a retry, which pins the bracket as per-attempt with a token the previous attempt cannot reuse. Mutation-tested, 7 of 7 caught: reverting the printf, moving the resume past the tee check or before the PIPESTATUS capture, hoisting the bracket out of the function, fixing the token, and deleting either end.
Collaborator
Author
|
@qwen-code /triage |
Collaborator
|
Sandboxed verification: The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. 中文 — 判定：
|
qwen-code-ci-bot
approved these changes
Aug 8, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
doudouOUC
approved these changes
Aug 8, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
APPROVE — 0 Critical, 2 Suggestions (both comment accuracy only; neither blocks merge).
Verified at head 8c8e4cd6 in a scratch tree (git archive of the PR head), not from the diff alone.
The mechanism is right
::stop-commands::<token> opens immediately before timeout … qwen … | tee, and printf '\n::%s::\n' closes it immediately after local ps=("${PIPESTATUS[@]}"). Every return in run_review_once (tee failure, 124/137, non-zero exit, empty log, no result event, error result, quota) is downstream of the resume, and set +e is still in force between the two, so no agent outcome can skip it. Both markers go to the step's stdout, not into the pipeline, so $LOG_PATH and the "type":"result" parsing are untouched. Bracketing only the agent — not the step — is the correct scope: $GITHUB_OUTPUT / $GITHUB_STEP_SUMMARY are file-based and unaffected either way.
I confirmed the parser asymmetry against actions/runner@main rather than the docs, since the whole fix turns on it — Runner.Common/ActionCommand.cs: TryParseV2 does TrimStart() then StartsWith("::") (line start only), while TryParse does IndexOf("##[") (matches anywhere). That is exactly why the leading newline in the resume is load-bearing and why the incident fired from mid-JSON.
The four fixes from round 2 all landed, and I re-derived them by mutation
I ran my own mutations against the head workflow (not the PR's table) and re-ran the full file each time:
| mutation | result |
|---|---|
resume with echo instead of printf '\n…' |
2 failed ✅ |
| delete the stop line | 6 failed ✅ |
| never resume | 6 failed ✅ |
resume before the PIPESTATUS capture |
25 failed ✅ |
move the resume past the tee-failure return |
1 failed ✅ |
hoist the bracket out of run_review_once |
2 failed ✅ |
| fixed token | 1 failed ✅ |
7/7 caught. Clean run: 98/98 passing (npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-pr-review-workflow.test.js).
The three tests I checked hardest, because they are the ones that could have been theatre:
timeout_partial_linegenuinely produces the shape that breaks anechoresume — the stub prints…"91- truncwith no trailing newline, andexpect(lines.some((l) => l.endsWith('"91- trunc')))fails on theechomutant rather than passing vacuously. This is the R2-1 blocker actually pinned.- The tee-failure scenario reaches the one early return the other scenarios never touch, via a
logPathwhose parent does not exist — soteefails with ENOENT regardless of privileges, and nothing is written outside the temp dir. - The
-1guards on the source-ordering anchors have teeth: deleting or rewording the stop line now fails, where-1 < Nused to pass.new Set(tokens).size === 2over a real two-attempt retry is what pins the randomness —token.length > 16alone is still satisfied by the 17-char literal prefix, so that assertion is decoration, but the retry one covers it.
timeout stub rewrite is behaviour-preserving for all pre-existing scenarios (the new branch is gated on timeout_partial_line), and retryLoopSource() extraction anchors are unchanged.
CI is green on head; every review thread on the PR is resolved and I checked the one Critical (R2-1) against the code as it stands rather than trusting the flag.
Two Suggestions — the rationale comments, not the code
Both are inline. Both were raised in the round-1 self-review and are still on head; the code they justify is correct, so this is text that will mislead the next reader, not a defect.
Note on merge state
reviewDecision is still CHANGES_REQUESTED from qwen-code-ci-bot's stale round-2 review, whose four findings are all fixed. My approval will not clear it — mergeStateStatus stays BLOCKED until that review is re-run (@qwen-code /review) or dismissed.
Out of scope, already acknowledged in the description — not blockers
qwen-triage.yml:1496/:3062are the sametimeout … qwen … | teeshape with no bracket. A shared shell function would carry the resume-after-PIPESTATUSinvariant once instead of asking each call site to remember it; worth a follow-up issue.- Containment ends before model-derived text is re-emitted:
detail(fromgrep -oiE 'reset at [^]]*') andsubtype=${RESULT_SUBTYPE}flow intoREASON→fail()→echo … >&2, outside the bracket, where a##[…]matches mid-line again. The runner scans stderr through the sameActionCommandManager, so the mechanism is identical. Narrow and pre-existing; this PR strictly reduces the surface.
中文说明
批准 —— 0 个阻断项，2 条建议（均为注释准确性问题，都不阻断合并）。
在 head 8c8e4cd6 上用 git archive 拉出的独立工作树完成验证，未仅凭 diff 判断。
机制正确。 ::stop-commands::<token> 恰好开在 agent 调用之前，printf '\n::%s::\n' 恰好关在 local ps=("${PIPESTATUS[@]}") 之后；run_review_once 中每一条 return 都在 resume 下游，且两者之间 set +e 仍然有效，因此任何 agent 结局都不会跳过 resume。两个标记只进入步骤 stdout，不进入管道，$LOG_PATH 与 "type":"result" 解析不受影响。只包裹 agent 而非整个步骤是正确的范围划分。
我直接对照 actions/runner@main 源码核实了两种命令形式的不对称性（Runner.Common/ActionCommand.cs：TryParseV2 先 TrimStart() 再 StartsWith("::")，仅行首匹配；TryParse 用 IndexOf("##[")，行内任意位置匹配）——这正是 resume 必须以换行开头的原因。
第二轮的四项修复全部落地，并由我自己的变异测试独立复核：7 个变异全部被捕获（详见上表英文部分），干净运行 98/98 通过。其中 timeout_partial_line 确实产出了会击穿 echo 式 resume 的形态；tee 失败场景走到了其他场景都到不了的提前返回路径，且路径父目录不存在，不会在临时目录外写入任何东西；顺序断言的 -1 守卫确实有约束力。
CI 在 head 上全绿；PR 上所有评审线程均已解决，唯一的 Critical（R2-1）我按当前代码逐行复核而非仅看标记状态。
两条建议见行内评论：都是第一轮自审已指出、但在 head 上仍未修正的注释表述问题——被它们解释的代码本身是对的，问题在于会误导后来的读者。
合并状态提示：reviewDecision 仍是 qwen-code-ci-bot 第二轮的 CHANGES_REQUESTED（其四项发现已全部修复）。我的批准不会清除它，mergeStateStatus 会保持 BLOCKED，直到该评审被重跑（@qwen-code /review）或被 dismiss。
范围之外、描述中已声明、非阻断项：qwen-triage.yml:1496 / :3062 是同一形态且无守卫，建议后续用共享 shell 函数承载"resume 必须在 PIPESTATUS 捕获之后"这一不变量；另外 detail 与 subtype=${RESULT_SUBTYPE} 经 REASON → fail() → echo … >&2 在括号外重新输出，runner 同样会扫描 stderr，##[…] 仍会行内匹配——面窄且是既有问题，本 PR 只是缩小了暴露面。
| # of the JSON line as a matcher path and errored. Observed on run | ||
| # 31167034020 (PR #8681): three `Unable to process command`, and | ||
| # 1h37m of review work discarded. Any PR whose review quotes a file | ||
| # containing `##[...]` or `::...::` breaks the same way — this |
Collaborator
There was a problem hiding this comment.
[Suggestion] The ::...:: half of this claim is not reachable, and stating it here misdescribes the threat model the guard defends against.
Verified against actions/runner@main, src/Runner.Common/ActionCommand.cs:
TryParseV2(::cmd::…) —message.TrimStart()thenStartsWith("::"): line start only.TryParse(##[cmd]…) —message.IndexOf("##["): anywhere in the line.
Under --output-format stream-json every transcript line starts with {, and JSON escapes newlines as \n, so quoted file content can never place a ::…:: at a line start. Only the ##[…] form is reachable from a quoted file — which is exactly why the incident fired from inside a JSON line, and exactly why the resume below needs its leading newline. As written, the comment implies the two forms are symmetric; a future reader could reasonably conclude the leading-newline printf is interchangeable with an echo.
Suggest narrowing to the ##[...] form (the ::...:: asymmetry is worth one clause, since it is what makes the resume fragile).
Raised in the round-1 self-review; still on head.
中文说明
这句里的 ::...:: 一半不可达，写在这里会误述该守卫真正防的威胁模型。
对照 actions/runner@main 的 src/Runner.Common/ActionCommand.cs 核实：TryParseV2（::cmd::…）先 TrimStart() 再 StartsWith("::")，仅行首匹配；TryParse（##[cmd]…）用 IndexOf("##[")，行内任意位置匹配。
在 --output-format stream-json 下每行会话都以 { 开头，且 JSON 把换行转义成 \n，因此被引用的文件内容永远无法把 ::…:: 放到行首。能从引用文件触发的只有 ##[…] 形式——这正是事故从 JSON 行内部触发的原因，也正是下面 resume 必须以换行开头的原因。按现在的写法，注释暗示两种形式对称，后来的读者可能因此认为那个带前导换行的 printf 与 echo 可以互换。
建议收窄为只讲 ##[...] 形式（::...:: 的不对称性值得单独一句，因为它才是 resume 脆弱的根源）。第一轮自审已提出，head 上仍未修改。
| # Resume BEFORE anything else can exit: errexit is still off here, | ||
| # so this line is reached on every agent outcome — timeout, crash | ||
| # or success. Leaving it off would silently swallow this job's own | ||
| # ::error:: and the fallback comment's diagnostics for the rest of |
Collaborator
There was a problem hiding this comment.
[Suggestion] Both specifics in this justification are wrong, while the real justification is stronger and sits one screen below.
this job's own ::error::— this step has no::error::.fail()(line 753) doesecho "$message" >&2,echo "failure_reason=…" >> "$GITHUB_OUTPUT"andecho "$message" >> "$GITHUB_STEP_SUMMARY". Nothing there is a stdout workflow command.the fallback comment's diagnostics for the rest of the run—Post fallback comment on failureis a separate step, and_stopProcessCommandcannot cross a step boundary:ActionCommandManageris constructed per handler inRunner.Worker/Handlers/Handler.cs(hostContext.CreateService<IActionCommandManager>(), non-singleton). It also readsfailure_reasonfrom$GITHUB_OUTPUT, which is file-based either way. The blast radius is the remainder of this step, not the run.
The one thing a missing resume actually loses is echo "::warning::Transient review failure (${REASON}) — retrying once…" on line 1281 — same step, after the resume, on the retry path this guard exists to survive. The test's own comment already says exactly that ("losing the retry ::warning::"); the workflow comment is the one that drifted.
The code is correct as-is — only the stated reason needs narrowing to the retry ::warning:: and to "the rest of this step".
Raised in the round-1 self-review; still on head.
中文说明
这段理由里的两个具体说法都不成立，而真正的理由更有力、就在下方一屏处。
this job's own ::error::—— 本步骤没有任何::error::。fail()（753 行）只做echo "$message" >&2、echo "failure_reason=…" >> "$GITHUB_OUTPUT"和echo "$message" >> "$GITHUB_STEP_SUMMARY"，其中没有 stdout 工作流命令。the fallback comment's diagnostics for the rest of the run——Post fallback comment on failure是独立步骤，而_stopProcessCommand无法跨步骤：ActionCommandManager在Runner.Worker/Handlers/Handler.cs中按 handler（即按步骤）构造（hostContext.CreateService<IActionCommandManager>()，非单例）。该步骤还是从$GITHUB_OUTPUT读failure_reason，本就是基于文件的。影响范围是本步骤的剩余部分，而不是整个 run。
resume 缺失真正会丢掉的，是 1281 行的 echo "::warning::Transient review failure (${REASON}) — retrying once…" —— 同一步骤、位于 resume 之后、正好在这个守卫要保住的重试路径上。测试里的注释已经写对了（"losing the retry ::warning::"），是 workflow 注释这边跑偏了。
代码本身没问题，只需把理由收窄到重试的 ::warning:: 和"本步骤的剩余部分"。第一轮自审已提出，head 上仍未修改。
Collaborator
|
Correction to my review above — the "Note on merge state" section is wrong, and I should have re-read the state right before submitting rather than relying on the snapshot I took ~30 minutes earlier.
Everything else in the review stands: the verification, the 7/7 mutation results, the 98/98 run, and the two inline comment-accuracy Suggestions (neither blocking). 中文说明对上面评审的更正 —— "合并状态提示"那一节是错的。我用的是约 30 分钟前的快照，提交前应当重新拉取一次状态。
评审的其余部分不变：验证过程、7/7 变异结果、98/98 运行，以及两条行内注释准确性建议（均不阻断）。 |
wenshao
commented
Aug 8, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
Author
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。
— kimi-k3 via Qwen Code /review (v0.21.7)
ken-jo
pushed a commit
to ken-jo/qwen-code
that referenced
this pull request
Aug 8, 2026
…wenLM#8720) The review workflow has been invalid since QwenLM#8648 merged, so every event it declares has been dead for ~12 hours: Invalid workflow file: .github/workflows/qwen-code-pr-review.yml#L1 (Line: 751, Col: 14): Exceeded max expression length 21000 A `run:` body containing `${{ }}` is evaluated as ONE expression template, and GitHub caps a single expression at 21000 characters. "Run review" went 17705 -> 22282 chars in QwenLM#8648 (17:00:32 on 2026-08-07); the first startup failure is stamped 17:00:50. QwenLM#8683 took it to 24042. An over-limit expression does not fail a job — it invalidates the whole file, so no run is created at all. Across the 400 runs since that merge there is not one success, not one `pull_request_target` and not one `issue_comment`: both automatic review and `@qwen-code /review` were unreachable, while CI stayed green throughout because nothing covered it. Pass the three context values the script reads through the step's env, leaving the body free of `${{ }}`. The runner then never templates it and its length stops mattering. No behaviour changes: each substitution is a rename of the same value. Pin it with a test that walks every workflow and fails any templated run block over the limit, plus one that keeps this body untemplated — it is past 21000 on its own, so a single `${{ }}` added back takes the whole workflow down again. Both fail against the file currently on main, naming it: `qwen-code-pr-review.yml > review-pr > Run review: 24042 chars`. Mutation-tested, 4 of 4 caught: restoring a `${{ }}` in the body (3 tests), dropping either env binding, and hardcoding the value the env used to carry. Co-authored-by: verify <verify@local>
Collaborator
|
Released in v0.21.8. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Wraps the review agent's invocation in
::stop-commands::, so the transcript it streams to stdout is logged rather than executed.Why it's needed
The agent streams its entire transcript to stdout, and the runner scans every line for workflow commands. A tool result that quotes a file containing one gets executed.
Observed on run 31167034020 (PR #8681). That PR changes an
actions/setup-nodeinput, so the agent read the action's ownmain.ts, which legitimately contains:##[add-matcher]takes the rest of the line as a file path — here, the remainder of the JSON:Three of those (09:59, 10:23, 10:44) and the step failed after 1h37m. A complete review discarded for quoting a file.
Nothing about that PR is special. Any review whose transcript quotes
##[...]or::...::breaks identically — including every review of this repository's own workflow files.grep -rn "stop-commands" .github/workflows/ .qwen/skills/returns nothing today, so no path is currently protected.This PR reproduced it on itself.
pull_request_targetruns the workflow from the base branch, so the guard was not active while reviewing the branch that adds it — and the review quotes this very diff. Run 31175067054: 36Unable to process commanderrors, andRun reviewwent red even though the agent finished cleanly and posted its review. The step's ownfail()never ran (the fallback comment fell back to the defaultFAILURE_REASON), so the failure came from the runner rejecting commands it found in the transcript, not from the script. Reviewers get a bogus "review did not complete successfully" comment on top of a review that did complete.Reviewer Test Plan
How to verify
Expected: 98/98. Six cases drive the real extracted retry loop with a stub agent that prints
##[add-matcher]mid-transcript:::stop-commands::<token>…::<token>::bracket, and the review still reportsoutcome=successsuccess,hardexitandtimeout_kill::cmd::at a line start only, so anechohere would leave parsing off for the rest of the jobindexOfreturns -1 when a line is deleted or reworded, and -1 satisfiestoBeLessThanTo see the original failure, the run above still has it in its logs:
Evidence (Before & After)
N/A for UI. Before is the log excerpt above; after is the 98/98 run and the mutation table.
Mutation-tested — 7 of 7 caught:
echoinstead ofprintf '\n…'run_review_onceThe ordering mutation failing 20 tests is the point:
echoclobbersPIPESTATUS, so resuming any earlier would read the echo's status instead of the agent's and misreport every timeout and crash as a clean run.Negative control for the assertion strength: with the stop line deleted, the previous ordering assertion passed (
-1 < N) — green on a workflow with no guard at all. The current one fails.Tested on
Risk & Scope
$GITHUB_OUTPUT,$GITHUB_STEP_SUMMARY,::error::— happens outside it and is unaffected, because file-based outputs were never stdout commands to begin with.echolines and a local variable inside one existing step.Linked Issues
Surfaced by the failing review on #8681. Not a defect in that PR — it only happens to touch
actions/setup-node, which is what led the agent to quote the file.中文说明
What this PR does
把评审 agent 的调用包在
::stop-commands::中，使它输出到 stdout 的会话内容只被记录，而不被执行。Why it's needed
agent 会把整份会话流式打到 stdout，而 runner 会扫描每一行寻找工作流命令。于是一个引用了含命令文本之文件的工具结果，就会被当成命令执行。
在 run 31167034020（PR #8681）上观察到。该 PR 改的是
actions/setup-node的一个输入，所以 agent 去读了这个 action 自己的main.ts，其中合法地写着：##[add-matcher]会把该行剩余部分当作文件路径——这里就是那一大坨 JSON 的其余部分：这样的错误出现三次（09:59、10:23、10:44），步骤在 1h37m 后失败。一整次评审因为引用了一个文件而作废。
这个 PR 本身没有任何特殊之处。任何会话中引用到
##[...]或::...::的评审都会以同样方式失败——包括对本仓库自身 workflow 文件的每一次评审。grep -rn "stop-commands" .github/workflows/ .qwen/skills/目前没有任何结果，即当前没有任何路径受到保护。本 PR 在自己身上复现了这个问题。
pull_request_target用的是 base 分支上的 workflow，所以在评审"引入这个守卫的分支"时，守卫本身并未生效——而这次评审恰好引用了本 PR 的 diff。Run 31175067054：36 次Unable to process command，Run review变红，尽管 agent 干净地跑完并且已经发出了评审。步骤自己的fail()从未执行（兜底评论用的是默认FAILURE_REASON），所以失败来自 runner 拒绝它在会话内容里发现的命令，而不是脚本判定。结果是：一次已经完成的评审之上，又叠了一条误导性的"review did not complete successfully"。Reviewer Test Plan
How to verify
预期 98/98。六个用例用真实抽取的重试循环驱动一个会在会话中间打印
##[add-matcher]的 stub agent：::stop-commands::<token>…::<token>::括号之内，且评审仍然报告outcome=successsuccess、hardexit和timeout_kill三种结局都会恢复命令解析::cmd::，此处用echo会让命令解析在本 job 余下的时间里一直关着indexOf返回 -1，而 -1 恒满足toBeLessThan原始失败仍可在该 run 的日志中看到：
Evidence (Before & After)
界面部分 N/A。Before 即上面的日志摘录；After 是 98/98 与下面的变异表。
变异测试 —— 7 个全部被捕获：
echo而非printf '\n…'恢复run_review_once之外顺序那个变异导致 20 个测试失败正是重点：
echo会覆盖PIPESTATUS，提前恢复就会读到 echo 的状态而非 agent 的，从而把每一次超时和崩溃都误报成正常完成。断言强度的负向对照：删掉 stop 那一行之后，修改前的顺序断言依然通过（
-1 < N）——在一个完全没有守卫的 workflow 上跑出全绿。当前的断言会失败。Tested on
Risk & Scope
$GITHUB_OUTPUT、$GITHUB_STEP_SUMMARY、::error::——都发生在括号之外，不受影响，因为基于文件的输出本来就不是 stdout 命令。echo和一个局部变量。Linked Issues
由 #8681 上失败的评审暴露。这不是那个 PR 的缺陷——它只是恰好改到了
actions/setup-node，从而引导 agent 去引用了那个文件。