fix(triage): finalize the status comment on cancellation too - #8436
Conversation
The 'Finalize triage status comment' step ran under success() || failure(), so a job-timeout (timeout-minutes: 30, reachable on large PRs) or a manual cancel skipped it and the early '🔄 Qwen Triage is running' lifecycle comment kept claiming the run was in progress forever — the verify lane has publish-verify to catch its cancelled case, but the triage lane has no other publisher. Run the step under always() (still gated on a resolved number), branch on job.status to say the run was cancelled rather than 'ended early', and point the reader at re-running. The step body is already best-effort throughout, so widening the condition cannot fail the job.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
@wenshao The problem is real — (success() || failure()) never fires on cancellation, so when the job times out (or is cancelled manually) the finalize step is skipped and the "🔄 Qwen Triage is running" comment stays stuck until a later trigger happens to overwrite it — but the PR body doesn't follow the PR template. None of the required headings are present: What this PR does, Why it's needed, Reviewer Test Plan (with How to verify / Evidence (Before & After) / Tested on), Risk & Scope, Linked Issues. The body uses freeform headings (Summary / Changes / Verification) instead.
All the needed content appears to be there, so this should be a reformat rather than new work: Summary → Why it's needed; Changes → What this PR does; Verification → Reviewer Test Plan (How to verify keeps the shape-test commands you listed, Evidence (Before & After) can be N/A for a workflow-only change, Tested on filled in for what actually ran). Your "widening the condition cannot fail the job" reasoning belongs in Risk & Scope (main risk or tradeoff), and Linked Issues can just say none if nothing tracked this. Once the body follows the template, re-run with @qwen-code /triage and I'll continue the review.
中文说明
@wenshao 问题是真实存在的——(success() || failure()) 在取消时不会触发，所以 job 超时（或手动取消）时 finalize 步骤会被跳过，"🔄 Qwen Triage is running" 评论会一直卡住，直到某次后续触发恰好覆盖它——但 PR 正文没有按 PR 模板 填写。必需标题均缺失：What this PR does、Why it's needed、Reviewer Test Plan（含 How to verify / Evidence (Before & After) / Tested on）、Risk & Scope、Linked Issues。正文用的是自由标题（Summary / Changes / Verification）。
所需内容其实都已具备，重新组织即可：Summary → Why it's needed；Changes → What this PR does；Verification → Reviewer Test Plan（How to verify 保留你列出的形状测试命令；纯 workflow 改动的 Evidence (Before & After) 可写 N/A；Tested on 按实际填写）。"放宽条件不会使 job 失败"的论证放进 Risk & Scope（主要风险或取舍）；Linked Issues 若没有关联 issue 可写无。正文按模板调整后，用 @qwen-code /triage 重新触发，我会继续审查。
— Qwen Code · qwen3.8-max
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。 未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.4)
| expect(finalizeStep).toContain("JOB_STATUS: '${{ job.status }}'"); | ||
| expect(finalizeStep).toContain('Qwen Triage was cancelled'); |
There was a problem hiding this comment.
[Suggestion] R1-2: The new assertions pin the two halves of the cancelled branch (the JOB_STATUS env entry and the Qwen Triage was cancelled wording) but not the elif [ "${JOB_STATUS:-}" = 'cancelled' ] condition that wires them together — Concrete cost: mutating that condition to 'failure' keeps this suite green (mutation executed: the mutant survives, 1 passed), while a cancelled run — the exact case this PR exists to fix — falls through to the generic "ended early" wording, and a genuinely failed run gets labelled "cancelled".
| expect(finalizeStep).toContain("JOB_STATUS: '${{ job.status }}'"); | |
| expect(finalizeStep).toContain('Qwen Triage was cancelled'); | |
| expect(finalizeStep).toContain("JOB_STATUS: '${{ job.status }}'"); | |
| expect(finalizeStep).toContain('Qwen Triage was cancelled'); | |
| expect(finalizeStep).toContain("elif [ \"${JOB_STATUS:-}\" = 'cancelled' ]"); |
中文说明
新增的断言钉住了取消分支的两半（JOB_STATUS env 项与 Qwen Triage was cancelled 措辞），但没有钉住把二者接起来的 elif [ "${JOB_STATUS:-}" = 'cancelled' ] 条件。具体代价：把该条件突变为 'failure' 后本测试仍然全绿（已实际执行突变验证：突变体存活，1 passed），而取消的 run——本 PR 要修的正是这个场景——会落入通用的 "ended early" 措辞，真正失败的 run 反而会被标成 "cancelled"。按上方 suggestion 补一行断言钉住该接线即可。
— qwen3.8-max via Qwen Code /review (v0.21.4)
|
@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. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Feedback points and dispositions1. [CHANGES_REQUESTED] Triage stage 1a — PR body must follow the PR templateDisposition: Escalated — cannot be applied from this workflow path; a ready-to-paste body is provided below. Verified against the finding: the current body uses freeform headings ( Why the bot cannot fix it here: reformatting the PR body is a GitHub write. This agent runs without credentials, and the address-review path of the autofix workflow only pushes commits, resolves review threads, and replies to threads — it has no step that updates the PR title or body. Rather than silently skipping the point, I prepared the template-conformant body per the reviewer's mapping ( Ask for a maintainer: apply the title/body below to the PR, then re-run Prepared template-conformant PR title and body (ready to paste)Title: Body: ## What this PR does
The qwen-triage workflow posts a "🔄 Qwen Triage is running" status comment early in the run and finalizes that same comment when the run ends. Previously the finalize step only ran on `success() || failure()`. This PR broadens the condition to `always()` (keeping the existing `steps.resolve.outputs.number != ''` guard), passes `${{ job.status }}` into the step as `JOB_STATUS`, and adds a dedicated finalization wording for cancelled runs: when triage did not complete and the job status is `cancelled`, the comment is finalized as "🚫 Qwen Triage was cancelled" with the run link, instead of the generic "ended early" wording.
## Why it's needed
`success() || failure()` never evaluates true when a job is cancelled — both the job-level `timeout-minutes` expiring and a manual cancel surface as cancellation. On that path the finalize step was skipped and the early "still running" comment stayed stuck until a later trigger happened to overwrite it. The verify lane has publish-verify to finalize its own status comment; the tmux lane only has this step, so it now covers every terminal outcome.
## Reviewer Test Plan
### How to verify
Workflow-only change. Coverage is the YAML shape test that pins the finalize step wiring:
```bash
npx vitest run --config ./scripts/tests/vitest.config.ts qwen-triage-workflow
```
Confirm the suite pins the `always() && steps.resolve.outputs.number != ''` condition, the `JOB_STATUS: '${{ job.status }}'` env entry, the `elif [ "${JOB_STATUS:-}" = 'cancelled' ]` branch, and the "Qwen Triage was cancelled" wording. Mutation check: changing the branch condition to `'failure'` makes the suite fail.
### Evidence (Before & After)
N/A — workflow-only change, no user-visible / TUI behavior.
### Tested on
| OS | Status |
| :--------: | :----: |
| 🍏 macOS | ⚠️ |
| 🪟 Windows | ⚠️ |
| 🐧 Linux | ✅ |
### Environment (optional)
Shape tests only (`scripts/tests/qwen-triage-workflow.test.js`); no CLI runtime involved.
## Risk & Scope
- Main risk or tradeoff: widening the condition cannot fail the job — the step already runs on the failure path today, and `always()` only adds the cancelled path. The script is best-effort (`set -uo pipefail`, warning instead of hard error), and the `steps.resolve.outputs.number != ''` guard is unchanged, so runs that cannot resolve a PR number still skip the upsert entirely.
- Not validated / out of scope: a genuinely cancelled GitHub Actions run cannot be replayed locally; the shape test pins the workflow wiring instead. The verify lane's finalize is untouched.
- Breaking changes / migration notes: none.
## Linked Issues
None — this was not tracked by an issue.
<details>
<summary>中文说明</summary>
本 PR 做了什么：qwen-triage workflow 会在运行早期发布 "🔄 Qwen Triage is running" 状态评论，并在运行结束时 finalize 同一条评论。此前 finalize 步骤只在 `success() || failure()` 时运行。本 PR 将条件放宽为 `always()`（保留既有的 `steps.resolve.outputs.number != ''` 守卫），通过 `JOB_STATUS` 把 `${{ job.status }}` 传入步骤，并为取消的运行新增专门的 finalize 文案：当 triage 未完成且 job 状态为 `cancelled` 时，评论会被更新为 "🚫 Qwen Triage was cancelled" 并附运行链接，而不是通用的 "ended early" 文案。
为什么需要：job 被取消时 `success() || failure()` 永远不为真——job 级 `timeout-minutes` 超时与手动取消都表现为 cancellation。该路径下 finalize 步骤被跳过，早期的 "仍在运行" 评论会一直卡住，直到某次后续触发恰好覆盖它。verify lane 有 publish-verify 来 finalize 自己的状态评论；tmux lane 只有这个步骤，因此它现在覆盖所有终态。
评审者测试计划——如何验证：纯 workflow 改动，覆盖是钉住 finalize 步骤接线的 YAML 形状测试（命令同上）。确认套件钉住了 `always()` 条件、`JOB_STATUS` env 项、`elif [ "${JOB_STATUS:-}" = 'cancelled' ]` 分支与 "Qwen Triage was cancelled" 文案；把该分支条件突变为 `'failure'` 会使套件失败。证据（改动前后）：N/A（纯 workflow 改动）。测试环境：Linux ✅，macOS / Windows ⚠️（纯 workflow 改动，与 OS 无关）。环境：仅形状测试，不涉及 CLI 运行时。
风险与范围：主要风险或取舍——放宽条件不会使 job 失败：该步骤今天在失败路径本来就会运行，`always()` 只是补上取消路径；脚本尽力而为（`set -uo pipefail`，告警而非硬错误），`steps.resolve.outputs.number != ''` 守卫不变，无法解析 PR 编号的运行仍完全跳过 upsert。未验证 / 超出范围——本地无法重放真实的 GitHub Actions 取消运行，改以形状测试钉住接线；verify lane 的 finalize 未改动。破坏性变更：无。
关联 Issue：无——此改动未有 issue 跟踪。
</details>2. [COMMENTED] Automated review — no blockersDisposition: no action needed; the review's suggestions are tracked as the inline finding below. 3. [Suggestion] R1-2 — pin the cancelled-branch wiring in the shape test (rc:3702444880)Disposition: Implemented. Added the assertion pinning Conflict notes
Verification
中文说明反馈点与处理结论1. [CHANGES_REQUESTED] Triage stage 1a —— PR 正文必须遵循 PR 模板结论：升级人工处理——本工作流路径无法执行该修改；下方已提供可直接粘贴的正文。 已按 finding 核实：当前正文使用自由标题（ 为何 bot 无法在此修复：重排 PR 正文属于 GitHub 写操作。本 agent 无凭据运行，且 autofix 工作流的 address-review 路径只推送提交、解决评审线程、回复线程——没有更新 PR 标题/正文的步骤。为了不静默跳过该点，我已按评审者给出的映射准备好符合模板的正文（ 请维护者：将下方标题/正文应用到 PR 上，然后重新执行 2. [COMMENTED] 自动评审 —— 无阻断问题结论：无需处理；该评审的建议即下方行内 finding。 3. [Suggestion] R1-2 —— 在形状测试中钉住取消分支的接线（rc:3702444880）结论：已实现。 在 finalize 形状测试中新增断言，钉住 冲突说明
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。 未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.4)
| ZH="✅ **Qwen Triage 已完成** —— [查看运行]($RUN_URL)。结果见本线程中的各阶段评论。" | ||
| elif [ "${JOB_STATUS:-}" = 'cancelled' ]; then | ||
| EN="🚫 **Qwen Triage was cancelled** — [view run]($RUN_URL). The run hit its time limit or was cancelled before finishing; re-run to get a result." | ||
| ZH="🚫 **Qwen Triage 已取消** —— [查看运行]($RUN_URL)。运行超时或被取消,未跑完;重新运行以获得结果。" |
There was a problem hiding this comment.
[Suggestion] The new cancelled branch's Chinese wording is unasserted — no shape-test assertion references 已取消; only the EN half is pinned (scripts/tests/qwen-triage-workflow.test.js:489) — Failure scenario: executed mutant: replacing this ZH line's content with the English text keeps the suite 120/120 green, and Chinese-only readers get an English-only cancellation notice. This regression class has precedent in this suite: the re-run-summary test (~line 420) carries a CJK-presence check because that body once shipped English only. Add, next to the EN wording assertion:
expect(finalizeStep).toContain('已取消');中文说明
新增取消分支的中文文案没有被任何断言钉住——形状测试中没有断言引用 已取消，只有英文半句被钉住（scripts/tests/qwen-triage-workflow.test.js:489）。失败场景：已实际执行突变——把该 ZH 行内容替换为英文文本后套件仍 120/120 全绿，纯中文读者将看到英文取消通知。本套件已有同类回归先例：re-run-summary 测试（约 420 行）正是因为该正文曾"只发英文"而加了 CJK 存在性检查。在 EN 文案断言旁补一行 expect(finalizeStep).toContain('已取消'); 即可。
— qwen3.8-max via Qwen Code /review (v0.21.4)
| if [ "${TRIAGE_OUTCOME:-}" = 'success' ]; then | ||
| EN="✅ **Qwen Triage finished** — [view run]($RUN_URL). See the stage comments in this thread for the result." | ||
| ZH="✅ **Qwen Triage 已完成** —— [查看运行]($RUN_URL)。结果见本线程中的各阶段评论。" | ||
| elif [ "${JOB_STATUS:-}" = 'cancelled' ]; then |
There was a problem hiding this comment.
[Suggestion] The three-way message selection is pinned by the shape test only as unordered toContain substrings, so which branch posts which message is untested — Concrete cost: executed mutant: swapping the success and cancelled EN+ZH bodies (conditions untouched, every asserted substring preserved) survives the full 120-test suite, while the real script observably posts the wrong message — extracted this step and ran it with a stubbed gh: under the mutant, (TRIAGE_OUTCOME=success, JOB_STATUS=success) posts "🚫 was cancelled". A real input reaching the order edge also exists: a timeout/manual cancel landing after the triage step already succeeded. The suite already solves this class elsewhere by executing the step's run: script against a stubbed gh and asserting the captured body (test lines ~244-288, ~335-403, ~2636+) — apply the same pattern to this composer, asserting the posted body for each (TRIAGE_OUTCOME, JOB_STATUS) combination.
中文说明
三分支消息选择在形状测试中只以无序 toContain 子串钉住，哪个分支发哪条消息未被测试。具体代价：已实际执行突变——交换 success 与 cancelled 的 EN+ZH 正文（条件不动、所有被断言子串保留）后整套 120 个测试仍全绿，而真实脚本会明显发错消息——提取该步骤并用打桩的 gh 运行：突变体下 (TRIAGE_OUTCOME=success, JOB_STATUS=success) 发出 "🚫 was cancelled"。存在可达分支顺序边界的真实输入：triage 步骤已成功后才发生的超时/手动取消。本套件在其他位置已用"执行步骤 run: 脚本 + 打桩 gh + 断言捕获正文"的模式解决同类问题（测试约 244-288、335-403、2636+ 行）——对这段消息组装应用同样模式，按每种 (TRIAGE_OUTCOME, JOB_STATUS) 组合断言所发正文。
— qwen3.8-max via Qwen Code /review (v0.21.4)
| elif [ "${JOB_STATUS:-}" = 'cancelled' ]; then | ||
| EN="🚫 **Qwen Triage was cancelled** — [view run]($RUN_URL). The run hit its time limit or was cancelled before finishing; re-run to get a result." |
There was a problem hiding this comment.
[Suggestion] The cancelled-path finalize PATCHes whatever lifecycle comment the marker lookup finds even when this run never posted its own claim — it can clobber a previous run's TERMINAL verdict wording written by qwen-triage-finalize.yml, which shares this marker — Concrete cost: executed probe: extracted this step and ran it with a stubbed gh against a fixture prior-verdict comment under (TRIAGE_OUTCOME=skipped, JOB_STATUS=cancelled) — it PATCHed the old verdict to "🚫 was cancelled — re-run to get a result". Trigger: a run manually cancelled within seconds of Resolve target number, before its own status claim lands; the verdict wording is then not restored (no further workflow_run completions fire once CI has fully settled). Impact is confined to this informational best-effort comment — the approval review event, stage comments, and CI table are untouched — hence Suggestion, not blocker. Fix (probe-verified to preserve the intended timeout-cancel-after-claim case): only PATCH when the found comment belongs to this run — skip when its body does not contain this run's $RUN_URL (the claim embeds the run link, so ownership is observable).
中文说明
取消路径的 finalize 会把 marker 查找命中的任意生命周期评论 PATCH 掉——即使本 run 从未发过自己的 "running" 声明——可能覆盖共用同一 marker 的 qwen-triage-finalize.yml 写入的上一个 run 的终态结论。具体代价：已实际执行探针——提取该步骤并用打桩的 gh 运行，在 (TRIAGE_OUTCOME=skipped, JOB_STATUS=cancelled) 下把一条旧结论评论 PATCH 成了 "🚫 was cancelled — re-run to get a result"。触发：run 在 Resolve target number 后数秒内（自己的状态声明发出前）被手动取消；此后结论文案不会被恢复（CI 完全落定后不再有新的 workflow_run 完成触发）。影响仅限于这条尽力而为的信息性评论——approve review 事件、各阶段评论与 CI 表格均不受影响——故为建议级而非阻断级。修复（已用探针验证不影响预期的"声明后超时取消"场景）：仅当命中的评论属于本 run 时才 PATCH——其正文不含本 run 的 $RUN_URL 时跳过（声明评论内嵌运行链接，归属可判定）。
— qwen3.8-max via Qwen Code /review (v0.21.4)
The cancelled finalize PATCHes whatever lifecycle comment the marker lookup finds. When a run is cancelled before its own status claim posts, the lookup lands on a previous run's comment — including the terminal verdict wording written by qwen-triage-finalize.yml, which shares the marker — and rewrites it to "was cancelled". Only PATCH a comment this run owns: the claim embeds the run link, so ownership is observable as the found body containing $RUN_URL. A found but foreign comment is left untouched; a missing one is still POSTed. Also pin what the shape test could not: the cancelled branch's Chinese wording, and an executed-composer test that runs the step's script against a stubbed gh to assert the posted body for each (TRIAGE_OUTCOME, JOB_STATUS) combination plus the ownership cases — swapping the success and cancelled bodies no longer survives the suite.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix review response — PR #8436All three automated-reviewer suggestions from the round-2 review were addressed in one commit ( Feedback and decisions1. [rc:3704492687] The cancelled branch's Chinese wording is unasserted — ADDRESSED Added 2. [rc:3704492695] Three-way message selection pinned only as unordered substrings — ADDRESSED Added an executed test in the suite's existing stubbed- 3. [rc:3704492703] Cancelled-path finalize can clobber a previous run's terminal verdict — ADDRESSED Verified against the code: the early status claim embeds this run's VerificationCommands actually run and their results:
Not run: integration tests after 中文说明Autofix 审查回应 —— PR #8436第 2 轮评审中自动评审器提出的三条建议已全部在一个提交（ 反馈与处理决定1. [rc:3704492687] 取消分支的中文文案没有被断言 —— 已处理 在形状测试中英文文案断言旁补上了 2. [rc:3704492695] 三分支消息选择只以无序子串钉住 —— 已处理 按本套件已有的"打桩 3. [rc:3704492703] 取消路径的 finalize 可能覆盖上一个 run 的终态结论 —— 已处理 已对照代码核实：早期状态声明内嵌本 run 的 验证实际执行的命令及结果：
未执行： Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
Qwen Code 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: |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round: no action neededFeedback triage. This round's feedback contains no review findings, no inline comments, and no maintainer requests — the only item is the automated reviewer's timeout notice: the review run exhausted its full 180-minute budget without producing a verdict, and the Failed check diagnosis. The only red check is
Local sanity check. To confirm the branch is healthy independent of the review timeout, the workflow test suite covering this change was run locally: Why no code change. A timed-out review has no code-level remedy within this PR. The retry path named in the timeout notice — commenting No commits were made this round; the branch head is unchanged at 中文说明Autofix 审查轮次:无需任何操作反馈分类。 本轮反馈不包含任何审查发现、行内评论或维护者请求 —— 唯一一条内容是自动审查器的超时通知:审查运行耗尽了完整的 180 分钟预算,未产出任何结论, 失败检查诊断。 唯一变红的检查是
本地健全性验证。 为确认分支健康状态与审查超时无关,在本地运行了覆盖本改动的 workflow 测试套件: 为何不做代码修改。 审查超时在本 PR 范围内没有代码层面的补救办法。超时通知中给出的重试方式 —— 在 PR 上评论 本轮未产生任何提交;分支头仍为 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
.github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
.github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.7-max via Qwen Code /review (v0.21.4)
|
🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round: no action takenThe only new feedback on this PR is an automated COMMENTED review carrying zero findings ( Disposition of each feedback point1. "CI failing: review-pr" — declined (no code action available).
2. "Not linted … actionlint embedded-shell source mapping is not yet supported" — no action. Verification
中文说明Autofix 审查轮次:未采取任何操作本 PR 上唯一的新反馈是一条自动化 COMMENTED 审查,其发现数为零( 每条反馈的处理1. "CI failing: review-pr" —— 拒绝处理(无代码层面的可操作项)。
2. "Not linted … actionlint embedded-shell source mapping is not yet supported" —— 无需操作。 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
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 linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查。 建议见行内评论。 未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| expect(foreign.body).toBe(null); | ||
| expect(foreign.call).toBe(null); |
There was a problem hiding this comment.
[Suggestion] R3-1: The new ownership lookup is exercised only against single-comment fixtures, so the selector that picks which matching comment is classified (| last) is unpinned — executed mutant: last → first survives the full 121-test suite. Failure scenario: the common real-world shape is an issue that has seen more than one triage run, so the API returns the previous run's terminal comment (foreign) followed by this run's claim (own); the first mutant classifies the older foreign comment, takes the foreign) arm, and does nothing (no PATCH, no POST, exit 0), leaving this run's "🔄 running" claim permanently unflipped — exactly the stale-progress-comment bug this PR exists to fix. The fixture below fails against the mutant and passes against the original (both verified by execution).
| expect(foreign.body).toBe(null); | |
| expect(foreign.call).toBe(null); | |
| expect(foreign.body).toBe(null); | |
| expect(foreign.call).toBe(null); | |
| // Both a prior run's terminal comment and this run's claim present: | |
| // `last` must select the own claim, not classify the older foreign | |
| // one and skip — a `first` mutant currently survives the suite. | |
| writeFileSync( | |
| commentsFile, | |
| JSON.stringify([ | |
| { | |
| id: 42, | |
| user: { login: 'qwen-code-ci-bot' }, | |
| body: '<!-- qwen-triage lifecycle -->\n\n✅ earlier verdict [finalize run](https://github.com/QwenLM/qwen-code/actions/runs/55)', | |
| }, | |
| { | |
| id: 43, | |
| user: { login: 'qwen-code-ci-bot' }, | |
| body: `<!-- qwen-triage lifecycle -->\n\n🔄 running — [watch live progress](${RUN_URL})`, | |
| }, | |
| ]), | |
| ); | |
| const both = run({ | |
| TRIAGE_OUTCOME: 'failure', | |
| JOB_STATUS: 'cancelled', | |
| }); | |
| expect(both.call).toContain('--method PATCH'); | |
| expect(both.call).toContain('issues/comments/43'); |
中文说明
新增的归属查找只在"单条评论"的 fixture 上运行过，因此"对哪条命中评论做分类"的选择器（| last）没有被钉住——已实际执行突变：把 last 改成 first 后整套 121 个测试仍全绿。失败场景：真实的常见形态是同一个 issue 跑过多轮 triage，API 返回上一轮的终态评论（外部）在本轮的声明（自有）之前；first 突变体会把较旧的外部评论当作分类对象，走入 foreign) 分支什么都不做（不 PATCH、不 POST、exit 0），本轮的 "🔄 running" 声明将永远停留在"进行中"——这正是本 PR 要修的评论卡死问题。上方 suggestion 中的 fixture 对突变体失败、对原实现通过（均已实际执行验证）。
— qwen3.8-max via Qwen Code /review (v0.21.5)
|
🔀 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 将重新运行。 |
… lookup 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 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #8436Feedback points and dispositions
Conflict notesNone ( Commit
Verification
中文说明Autofix 审查轮次 — PR #8436反馈点及处理
冲突说明无（ 提交
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
Qwen Code 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: |
|
🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
|
Correction to §5 of my round-5 report above — the two I had cited So 中文版更正上一条报告第 5 节 —— 那两行 我此前用 第 5 节其余内容不变：这两行在解析后语义完全相同（ |
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Round 9 feedback: both inline findings are resolved with minimal changes (one added test assertion, corrected step comments). The standing Critical R8-9 (PR body not template-compliant) cannot be applied by this agent — an updated paste-ready body is included below for the PR author or a maintainer. No base-conflict resolution was needed this round ( 1. [Suggestion] R9-2 — the claim's First reproduced the reported mutant: removing 2. [Suggestion] R9-3 — the empty-id no-op drops the pre-PR finalize recovery path — RESOLVED (documentation option). Applied the reviewer's second option: the finalize step comment now documents the accepted loss. An empty id has two causes — a cancel landing before the claim posted, or a transient failure of the claim's write (both the fresh-POST and the PATCH arm export an empty id on failure) — and finalize deliberately writes nothing in either case. The comment now states that a failed claim therefore leaves no lifecycle breadcrumb even when the comments API recovers during the minutes-long agent run, and why that is accepted: the verdict still lands in the stage comments, and a list-then-POST recovery would reintroduce exactly the comment selection this PR removes (the suite already pins finalize to no listing at all: 3. [Critical] R8-9 — the PR body must follow the PR template — NOT APPLICABLE BY THIS AGENT; maintainer/author action required. Unchanged since triage stage 1a (review 4841776175): the description still uses freeform Summary/Changes/Verification headings and has no Test Plan section. This agent has no channel to edit the PR description — the address-review flow's GitHub writes are limited to pushing commits, posting comments, and resolving/replying to review threads. The paste-ready body drafted earlier (issue comment 5196984782) described the previous ownership-match iteration and is stale; the updated template-compliant body below reflects the current id-handoff design. After applying it, please re-run Updated paste-ready PR body (template-compliant, current id-handoff design)## What this PR does
The triage workflow posts a lifecycle status comment ("🔄 Qwen Triage is running") when a triage run starts, and a final 'Finalize triage status comment' step flips that same marker comment into a terminal state. Previously each step re-ran its own marker-comment lookup, so the two could disagree about which comment this run owned — a selection that could race — and the finalize gate `(success() || failure())` was never true on cancellation (a job-level timeout or a manual cancel), so on that path the step was skipped and the status comment kept claiming the run was still in progress.
This change couples the two steps through an explicit id handoff instead of a second lookup: the claim step upserts the marker comment and exports the exact comment id it created (extracted with `--jq '.id'`) or reused, and the finalize step PATCHes exactly that id — nothing is re-derived at finalize time. The finalize gate is widened to `always()` so cancellation is covered too, and `${{ job.status }}` is passed in as `JOB_STATUS` so a dedicated cancelled terminal wording is selected alongside the existing finished / ended-early wordings, with a step-level success no longer allowed to claim "finished" when the job itself failed.
## Why it's needed
A stuck "running" claim misleads everyone watching the issue: the run ended, but the comment says it is still in progress, and nothing in this lane ever corrects it — the verify lane has publish-verify for exactly this case, while this lane only has the finalize step. Widening the gate to `always()` covers the one remaining path (cancellation) on which the comment could be left stale, and keying finalize on the claim's exported id removes the two-step selection disagreement entirely.
## Reviewer Test Plan
### How to verify
Run the workflow shape test that pins this behavior: `npx vitest run scripts/tests/qwen-triage-workflow.test.js --config ./scripts/tests/vitest.config.ts` (129 tests). It asserts that the claim step exports `comment_id` to `$GITHUB_OUTPUT` and extracts the created id with `--jq '.id'`; that the finalize step reads `STATUS_COMMENT_ID` from the claim step's output and PATCHes exactly that id while doing no comment listing at all (no `--paginate`, no `gh api user`); that the finalize `if:` condition is `always() && steps.resolve.outputs.number != ''`; the three-way wording selection (finished / cancelled / ended-early) driven by `steps.triage.outcome` and `job.status`; and the empty-id no-op. The claim and finalize scripts are additionally executed against a stubbed `gh` with failure-injection arms, asserting the exact body PATCHed for every terminal state, newest-marker reuse, legacy-marker reuse, and that a failed write exports an empty id without failing the step.
### Evidence (Before & After)
N/A — workflow-only change, no TUI surface.
### Tested on
| OS | Status |
| :--------: | :----: |
| 🍏 macOS | ⚠️ |
| 🪟 Windows | ⚠️ |
| 🐧 Linux | ✅ |
### Environment (optional)
N/A — unit/shape tests only (vitest).
## Risk & Scope
- Main risk or tradeoff: finalize PATCHes exactly the id the claim step exported. When the claim's write fails transiently — or a cancel lands before the claim posts — the exported id is empty and finalize deliberately writes nothing, so the thread loses this run's lifecycle breadcrumb even though the comments API typically recovers during the run; accepted because the verdict still lands in the stage comments and a fallback post would reintroduce the selection this change removes (documented on the finalize step). Every API call in both steps is best-effort with a `::warning::` fallback, so neither step can fail the job, and `always()` only widens when the last step in the job runs.
- Not validated / out of scope: a live cancelled-run E2E against GitHub Actions (cancellation cannot be triggered deterministically from a PR); the behavior is pinned by the shape and executed-harness tests instead.
- Breaking changes / migration notes: none — the marker, the stage comments, and every API surface are unchanged.
## Linked Issues
None tracked.
<details>
<summary>中文说明</summary>
## 本 PR 做了什么
Triage workflow 在运行开始时会发布一条生命周期状态评论（"🔄 Qwen Triage is running"），并由最后的 'Finalize triage status comment' 步骤把同一条 marker 评论翻转为终态。此前两个步骤各自重新执行 marker 评论查找，可能对"本次运行拥有哪条评论"产生分歧（一个可能竞态的选择）；且 finalize 的条件 `(success() || failure())` 在取消时（job 级超时或手动取消）永不为真，于是该路径上步骤被跳过，状态评论一直声称运行仍在进行。
本改动通过显式的 id 交接（而非第二次查找）把两个步骤耦合起来：claim 步骤 upsert marker 评论并导出它创建（用 `--jq '.id'` 提取）或复用的确切评论 id，finalize 步骤只 PATCH 这个确切的 id —— finalize 阶段不再重新派生任何内容。同时把 finalize 条件放宽为 `always()` 以覆盖取消，并传入 `${{ job.status }}` 作为 `JOB_STATUS`，从而在既有的"已完成 / 提前结束"文案之外选择专门的"已取消"终态文案；且当 job 本身失败时，不再允许仅凭步骤成功就声称"已完成"。
## 为什么需要
卡住的 "running" 声明会误导所有关注该 issue 的人：运行已经结束，评论却显示仍在进行，而本 lane 没有任何东西会纠正它 —— verify lane 有 publish-verify 专门处理这种情况，而本 lane 只有 finalize 步骤。把条件放宽为 `always()` 覆盖了评论可能保持过期的最后一条路径（取消）；让 finalize 以 claim 导出的 id 为准，则彻底消除了两步之间的选择分歧。
## 审阅者测试计划
### 如何验证
运行固定此行为的 workflow shape 测试：`npx vitest run scripts/tests/qwen-triage-workflow.test.js --config ./scripts/tests/vitest.config.ts`（129 个测试）。它断言：claim 步骤把 `comment_id` 导出到 `$GITHUB_OUTPUT` 并用 `--jq '.id'` 提取创建的 id；finalize 步骤从 claim 步骤的输出读取 `STATUS_COMMENT_ID` 并只 PATCH 这个确切的 id、且完全不做评论列举（没有 `--paginate`、没有 `gh api user`）；finalize 的 `if:` 条件是 `always() && steps.resolve.outputs.number != ''`；由 `steps.triage.outcome` 和 `job.status` 驱动的三态文案选择（已完成 / 已取消 / 提前结束）；以及空 id 的 no-op。claim 与 finalize 脚本还会在带故障注入分支的桩 `gh` 上实际执行，断言每种终态 PATCH 的确切 body、最新 marker 复用、legacy marker 复用，以及写失败时导出空 id 且不使步骤失败。
### 证据（前后对比）
N/A —— 仅 workflow 改动，无 TUI 界面。
### 测试环境
| OS | 状态 |
| :--------: | :----: |
| 🍏 macOS | ⚠️ |
| 🪟 Windows | ⚠️ |
| 🐧 Linux | ✅ |
### 环境（可选）
N/A —— 仅单元/shape 测试（vitest）。
## 风险与范围
- 主要风险或权衡：finalize 只 PATCH claim 步骤导出的 id。当 claim 的写入瞬时失败 —— 或取消先于 claim 发出 —— 导出的 id 为空，finalize 有意什么都不写，因此即使 comments API 通常会在运行期间恢复，线程也会丢失本次运行的生命周期面包屑；这是可接受的，因为结论仍会落在各阶段评论中，而兜底新发会重新引入本改动要移除的选择逻辑（已在 finalize 步骤中记录）。两个步骤中的每个 API 调用都是尽力而为、失败时 `::warning::` 兜底，因此都不会使 job 失败；`always()` 也只是放宽该 job 最后一个步骤的执行时机。
- 未验证 / 超出范围：针对 GitHub Actions 的真实取消运行 E2E（无法从 PR 确定性地触发取消）；行为改由 shape 测试与实际执行装置固定。
- 破坏性变更 / 迁移说明：无 —— marker、各阶段评论以及所有 API 接口均未改变。
## 关联 Issue
无。
</details>Verification
中文说明第 9 轮反馈：两条 inline 发现均以最小改动解决（新增一条测试断言、修正步骤注释）。长期存在的 Critical R8-9（PR 正文不符合模板）本 agent 无法代为应用 —— 下方附了更新后可直接粘贴的正文，供 PR 作者或维护者使用。本轮无需解决与 base 的冲突（ 1. [Suggestion] R9-2 —— claim 的 先复现了报告中的变异体：把 2. [Suggestion] R9-3 —— 空 id 的 no-op 丢掉了本 PR 之前 finalize 的恢复路径 —— 已解决（采用文档方案）。 采用了审阅者的第二个方案：finalize 步骤注释现在记录了这一有意取舍。空 id 有两种成因 —— 取消先于 claim 发出，或 claim 的写入瞬时失败（新发 POST 与 PATCH 两个分支失败时都导出空 id）—— 两种情况下 finalize 都有意什么都不写。注释现在写明：因此 claim 失败时，即使 comments API 在数分钟的 agent 运行期间恢复，线程也不会得到生命周期面包屑；以及为何可接受：结论仍会落在各阶段评论中，而"先列出再新发"的恢复路径会重新引入本 PR 要移除的评论选择逻辑（套件已把 finalize 钉死为完全不做列举： 3. [Critical] R8-9 —— PR 正文必须遵循 PR 模板 —— 本 agent 无法应用；需要维护者/作者操作。 自 triage stage 1a（review 4841776175）以来未变：描述仍使用自由的 Summary/Changes/Verification 标题，且没有 Test Plan 部分。本 agent 没有修改 PR 描述的渠道 —— address-review 流程的 GitHub 写操作仅限于推送提交、发表评论、解决/回复评审线程。早先起草的可直接粘贴正文（issue 评论 5196984782）描述的是上一版"所有权匹配"迭代，已经过时；下方更新后的模板合规正文反映当前的 id 交接设计。应用后请重新运行 （更新后的 PR 正文以代码块形式附在上方英文部分的折叠块中，其本身已按模板要求包含完整的中文说明，此处不再重复翻译。） 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /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: 128 passed · 0 failed · 128 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：128 通过 · 0 失败 · 128 总计 Verification reportVerification report — PR #8436
|
| # | finding (round 2 at 2d14f518) |
severity | status at b8ac04ef |
|---|---|---|---|
| 1 | Double-marker corner: a stranded older "running" own claim behind a newer marker was only healed by the claim step's oldest-running reclaim | informational | superseded by redesign — re-measured, not diffed: the whole list-and-match finalize and the reclaim selector were removed in 02aad770; finalize now PATCHes exactly the id the claim exported, so the old creation path (finalize skipping on a foreign match) is gone. Residual: a stranded running comment can now only arise from an export loss (write committed server-side, response lost — CA6 measures the plain-failure half, which exports empty correctly), and it persists only if a newer marker later appears (cell EE5: claim picks newest 45, orphan 44 untouched). Narrower than round-1's shape, and the base tree had the same newest-wins selection (no reclaim either), so not a regression vs base. Reported as Finding 1. |
| 2 | timeout-minutes → job.status semantics bounded, not measured (GitHub-server property) |
informational | stands — still unobservable in-container. The wording branch handles both readings (job timeout reads as failure → "ended early"; cancel reads as cancelled → "was cancelled"), and the 12-combo truth table (EE7) covers every (TRIAGE_OUTCOME, JOB_STATUS) pair. |
| 3 | M8 coverage gap: claim own-marker last→first survived the suite (no two-own-marker fixture) |
informational | fixed — the selector is now the plain newest-wins last, and the new executed claim composer test pins it: mutant M6 (last→first) turns the suite RED (1 test) and my A/B harness RED (CA2). |
Declined rows: none in round 2. Round-2 Finding 1 (unreadable-list asymmetry between claim and finalize) is also superseded: finalize no longer lists at all (FA8: a list outage cannot affect it — zero GET calls by construction, asserted in EE3), and the claim side's || EXISTING_ID='' fresh-POST fallback is byte-parity with base (CA5) and now converges because each run finalizes its own exported id.
Scope
Central claim: the lifecycle comment always reaches a terminal state because the two steps are coupled by id, not by re-selection: (a) the claim step exports the exact comment id it wrote (comment_id — reused id on PATCH, --jq '.id'-extracted created id on POST, empty on any write failure); (b) Finalize triage status comment runs under always() && number != '' (covering cancellation) and PATCHes exactly that id — never lists, never matches bodies, no-ops on an empty id; (c) the wording is correct for every terminal (TRIAGE_OUTCOME, JOB_STATUS) combination, and "finished" never appears on a failed job.
Secondary claims: (1) every write arm survives a failing API under GitHub's real -eo pipefail (exit 0 + warning), and a failed claim write exports an empty id rather than a guess; (2) the two other hunks in the diff — the npm-cache key: quote-style flips in tmux-testing and verify — are semantic no-ops.
A/B result — finalize step (central claim a–c)
Harness ab-finalize.mjs extracts each arm's if: + run: from the YAML via the yaml package, evaluates the if: under GitHub status-function semantics (success() and failure() are both false on cancellation — that is exactly the gap the PR closes), and executes the bash under bash --noprofile --norc -eo pipefail with a recording gh stub + real jq. Witness: 01-ab-finalize-base-vs-head.png. Fixture: this run's claim id 43, RUN_URL=…/runs/77.
| cell | base (list-and-flip-newest, success()‖failure()) |
head (PATCH exported id, always()) |
|---|---|---|
| FA1 success/success, own 43 | PATCH 43 "finished" | PATCH 43 "finished", body byte-identical, exactly one call total |
| FA2 failure/failure, own 43 | PATCH 43 "early" (half-width ZH comma) | PATCH 43 "early" (full-width comma — the declared fix; bodies identical after normalizing that one byte) |
| FA3 cancelled/cancelled (the bug) | SKIPPED — 🔄 claim stranded forever | PATCH 43 "was cancelled / 已取消" |
| FA4 cancelled job, triage step success | SKIPPED | PATCH 43 "finished" (triage success precedes a late cancel) |
| FA5 failed job, triage step success ('Check triage response' exits 1) | PATCH 43 "finished" — points at stage comments that never posted | PATCH 43 "ended early" |
| FA6 empty id; foreign tombstone 44 (runs/55) | PATCHes 44 — clobbers the previous run's terminal wording | zero writes + "claimed no status comment" diagnostic |
| FA7 empty id; empty thread | POSTs a fresh "early" comment | zero writes (accepted breadcrumb loss, documented in the step comment) |
| FA8 list API down, own 43 | conflates unreadable with empty → POSTs fresh | PATCHes 43 (makes no list call at all — immune) |
FA9 number='' |
skipped | skipped (guard parity) |
| FA10 two pages, own on page 2, cancelled | skipped (would have needed the pagination idiom) | PATCHes 43 with one call |
40/40 assertions, including the control cells asserting the base arm misbehaves exactly as predicted (FA3 skip, FA5 false "finished", FA6 clobber, FA8 conflation).
A/B result — claim step (central claim a, secondary 1)
ab-claim.mjs, same mechanics. The stub answers a POST with a JSON payload {"id": N} and applies the caller's --jq filter with real jq (as gh does client-side) — one fidelity level above the PR's own suite, whose stub echoes a bare id and therefore cannot exercise the --jq '.id' flag the last commit pinned statically. Witness: 02-ab-claim-id-export.png.
| cell | base | head |
|---|---|---|
| CA1 empty thread | POST body | POST byte-identical body; --jq .id in raw args; exports comment_id=7777 (extraction executed through real jq) |
| CA2 tombstones 42+44 | PATCH newest 44 | PATCH 44, exports 44 (shared-slot semantics unchanged) |
| CA3 legacy marker 41 | PATCH 41 | PATCH 41, exports 41 (single-slot healing kept) |
| CA4 forged non-bot markers | POST fresh (author filter) | POST fresh, exports 7777 |
| CA5 unreadable list | POST fresh (‖ EXISTING_ID='') |
POST fresh, exports 7777 (parity with base) |
| CA6 POST-fail / PATCH-fail | exit 0 + warning both arms | exit 0 + warning, exports EMPTY id both arms (finalize will no-op, never a guessed id) |
| CA7 bot identity failure | exit 0, warn, no writes | same, and exports nothing (finalize sees empty id) |
34/34 assertions.
Head-only edge matrix + wording truth table
edges.mjs. Witness: 03-edges-truth-table.png.
| cell | result |
|---|---|
| EE1 | the jq-null corner measured: a POST payload without .id would make the pinned --jq '.id' emit literal null (real jq), which finalize would PATCH as comments/null — still exit 0 (best-effort holds). Unreachable on the real comments API (201 always carries id); see Finding 2 |
| EE2 | STATUS_COMMENT_ID fully unset (set -u safe via ${…:-}): exit 0, zero writes, diagnostic |
| EE3 | finalize makes exactly one call (the PATCH); zero list/user calls — the no-re-selection property, enforced |
| EE4 | exactly one comment_id= line in $GITHUB_OUTPUT per claim run |
| EE5 | stranded-corner measurement: fixture [running 44 (older), tombstone 45] → claim PATCHes newest 45; orphan 44 untouched (see Finding 1) |
| EE6 | finalize body starts with the lifecycle marker and keeps the run link |
| EE7 | full truth table: 4 outcomes × 3 job statuses = 12 combos, each asserting the exact expected wording and the absence of the other two; plus a dedicated sweep proving no combo with JOB_STATUS=failure ever says "finished" |
22/22 assertions.
Mutation matrix (vacuity of the new tests)
Seven point mutants applied in place (byte-exact, single-occurrence asserted, restored after each, tree verified byte-identical at the end); per mutant the vitest suite (129 tests) re-run, and for the five mutants targeting behavior my harnesses execute, the matching harness too. Unmutated controls: suite 129/129 green; harnesses 96/96 green. Witness: 04-mutation-matrix.png.
| mutant | suite | executed harness |
|---|---|---|
M1 claim POST arm: drop --jq '.id' |
RED (1 test — the static pin) | RED (ab-claim: export becomes the raw payload, not 7777) |
| M2 swap cancelled↔early EN wording (positive control) | RED (5 tests) | — |
M3 drop the JOB_STATUS != failure gate on "finished" |
RED (2 tests) | — |
| M4 remove the empty-id no-op guard | RED (2 tests) | RED (ab-finalize FA6: writes appear where none may) |
| M5 claim exports the reused id even when the PATCH fails | RED (1 test) | RED (ab-claim CA6: stale 45 exported) |
M6 claim selector last→first |
RED (1 test) | RED (ab-claim CA2: PATCHes 42, exports 42) |
M7 finalize always()→success()‖failure() |
RED (1 test) | RED (ab-finalize: condition shape + cancelled cell) |
No survivors; no mutant regressed. M1 is the attribution the last commit claims: the executed harness catches the dropped flag behaviorally, where the suite pins it only statically (the suite's stub bypasses --jq).
Secondary claim 2 — cache-key quote flips
cache-key-parity.mjs: the PR also flips two key: lines (tmux-testing, verify jobs) between single-quoted-with-escaped-quotes and double-quoted YAML styles. The PARSED values are byte-identical across the flip (npm-ci-${{ hashFiles('package-lock.json') }}), restore-keys and the action refs unchanged — cache identity is untouched. 9/9 assertions. Both merge parents carried the different styles, so the flip is part of what lands; semantically a no-op.
Corrections
- Commit
4708d1addescribes a change that is not in the final diff. Its message saysqwen-triage-finalize.yml'supdate_status(the third writer of the lifecycle marker) was taught to "prefer the newest marker that is not a live running claim". Verified against the aggregate diff:git diff HEAD^1..HEAD --stat -- .github/workflows/qwen-triage-finalize.ymlis empty —update_statusstill selects| last |unconditionally (lines 326–336 at head). This is a correction to the commit description, not a request to change the code: the harm that commit set out to fix (finalize.yml erasing the run URL the OLD finalize keyed on → misclassification as foreign → second post + permanent strand) no longer exists, because02aad770removed body/URL-keyed selection entirely — the triage finalize PATCHes the exported id, so anupdate_statusoverwrite of a live claim's body can no longer break the flip. What remains is the pre-existing, base-identical cosmetic overwrite (see Finding 3).
Findings (non-blocking)
- Stranded "running" comments can still persist in one narrow corner (informational). Creation requires the claim's PATCH to commit server-side but lose its response (so
comment_idexports empty while the comment carries this run's running body), AND a newer marker to appear later (a list-failure fresh POST or a double-POST race). Measured in EE5: with[running 44 older, tombstone 45], the claim reuses newest 45 and never touches 44 — the round-1/2 oldest-running reclaim was deliberately removed ("a list-then-POST recovery here would reintroduce exactly the selection this coupling removes"). The plain write-failure half is handled and documented (CA6 exports empty; the YAML comment names it); the response-loss half is not named there. Not a regression vs base (base also selected newest-wins and had no reclaim), strictly narrower than the round-1 shape, and every run's own comment is finalized by id. No action required; optionally name the response-loss half in the step comment. - jq-null corner on the POST arm (informational). If a POST ever returned 200 without an
idfield,--jq '.id'emits literalnull, which[ -n ]accepts, exportingcomment_id=null; finalize would PATCHissues/comments/null, gh would 404, and the best-effort guard would warn and exit 0 (measured EE1a/EE1b). Unreachable on the real comments API (a 201 always carriesid; error statuses exit non-zero →|| COMMENT_ID=''). No action required. - Shared-slot interleavings still transiently mislabel during overlapping runs; the advertised live-claim guard is absent (informational, pre-existing). The marker comment is a shared slot: run B's claim overwrites run A's running body, and A's later finalize flips the slot while B is live (the slot converges when B finalizes — measured convergent for all wording combos, EE7). This is base-identical and accepted by the slot design. Separately, see Corrections: the
qwen-triage-finalize.ymllive-claim guard from4708d1addid not land, soupdate_statuscan still cosmetically overwrite a live claim until that run finalizes. No action required.
Not covered
- Uncalibrated replay — no GitHub token and no real lifecycle comment available locally, so the harnesses were not byte-calibrated against a production-emitted artifact (same as rounds 1–2). Partial corroboration: the claim bodies the harnesses flip are the exact bytes the head claim step composes (CA1 capture), and actionlint fully parses both trees' YAML.
- Per-commit attribution — depth-2 checkout:
git rev-list HEAD^1..HEAD^2returns 1 commit (shallow boundary) against 25 in the metadata snapshot, so the three substantive commits since round 2 were verified via cells aimed at their claims (02aad770→ FA6/FA8/EE3 coupling cells;b8ac04ef→ M1/CA1 extraction cells;4708d1ad→ Corrections finding), not isolated per-commit builds. The aggregateHEAD^1..HEADdiff (2 files) is what was verified. - yamllint gate — environmental, re-proven this round:
pip3: Permission deniedinscripts/lint.js --setup(quoted inlogs/lint-setup.log). YAML validity still covered by actionlint's full parse on both trees and every harness'syamlparse. - GHA server-side semantics —
timeout-minutes→job.statusvalue, and thejob.statuscontext availability insideenv:(previous-finding 2, stands). actionlint validates the context usage and accepts it; the lane has run with this env since round 2. - Live concurrency replay against the real API — the interleavings in Finding 3 are reasoned + cell-measured, not replayed on GitHub.
- Full-repo eslint/lint — covered by the PR's own CI; the only non-workflow file changed is a test file.
- The PR description's test count ("120/120") is stale: the suite at head is 129 tests (it grew with the later commits). No significance.
Methodology
Environment: the CI verify container (node:22-bookworm, node v22.23.2, jq 1.6; actionlint 1.7.12 + shellcheck 0.11.0 installed pinned via scripts/lint.js --setup into /tmp/qwen-code-linters/local-*; yamllint uninstallable). The PR touches only workflow YAML and one test file — no compiled packages and no dependency changes (package.json/lockfile untouched), so the A/B "builds" are the two trees' YAML: head .github/workflows/qwen-triage.yml vs the tmp/base-tree worktree at HEAD^1, whose file sha256 was asserted equal to git show HEAD^1 (909ba7fd…). The code under test is the verbatim-extracted bash executed under GitHub's exact shell flags (bash --noprofile --norc -eo pipefail) with real jq and a recording gh stub that applies --jq filters like gh does (no stub of the code under test itself); the if: conditions are evaluated under GitHub status-function semantics by evalIf, which refuses unknown shapes. Harnesses (in this dir): ab-finalize.mjs (40), ab-claim.mjs (34), edges.mjs (22), cache-key-parity.mjs (9), mutate.mjs (7 mutants + restore = 8 checks), gate runs incl. two liveness probes (15). Raw per-run logs in logs/; evidence PNGs via scripts/verify-capture.mjs. Assertion counts: 40+34+22+9+8+15 = 128/128.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review. The Stage 1a template gate still fails at Triage 重跑完成，未产生新评审。 Stage 1a 模板关卡在 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: test-efficacy probe — the probe kit could not collect scripts/tests (workspace-only discovery; its 'unreachable' claim is disproven by direct execution: 129/129 pass here and the suite is gated in CI via test:ci → test:scripts), so revert/mutant/hunk probing did not run; compensated by manually executed mutants (four mutants, all caught by the new tests).
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
[Critical] R8-9 Existing CHANGES_REQUESTED (triage stage 1a, review 4841776175, 2026-08-03): the PR body must follow the PR template — still stands at the reviewed commit: the description still uses freeform Summary/Changes/Verification headings and has no Test Plan section (test-plan check: 'the PR description has no Test Plan section'). Subject is the PR description, not the code: an updated paste-ready template-compliant body reflecting the current id-handoff design was drafted in the thread (issue comment 5211486662); the author or a maintainer needs to apply it and re-run @qwen-code /triage.
中文说明
未审查：test-efficacy probe — the probe kit could not collect scripts/tests (workspace-only discovery; its 'unreachable' claim is disproven by direct execution: 129/129 pass here and the suite is gated in CI via test:ci → test:scripts), so revert/mutant/hunk probing did not run; compensated by manually executed mutants (four mutants, all caught by the new tests)。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
[Critical] R8-9 Existing CHANGES_REQUESTED (triage stage 1a, review 4841776175, 2026-08-03): the PR body must follow the PR template — still stands at the reviewed commit: the description still uses freeform Summary/Changes/Verification headings and has no Test Plan section (test-plan check: 'the PR description has no Test Plan section'). Subject is the PR description, not the code: an updated paste-ready template-compliant body reflecting the current id-handoff design was drafted in the thread (issue comment 5211486662); the author or a maintainer needs to apply it and re-run @qwen-code /triage.
— qwen3.8-max via Qwen Code /review (v0.21.7)
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM, no blockers. The id-passing design is a real improvement: the claim step exports the exact comment id it wrote, and finalize PATCHes precisely that id — removing the old list-then-match selection that could race or clobber another run's comment. always() (not success||failure) correctly covers cancellation; JOB_STATUS gating prevents a red job from saying 'finished'; empty id is a no-op (never a fresh post or lookup that could overwrite a prior run's terminal wording); a failing PATCH warns instead of turning the job red under -eo pipefail. The executed-harness tests run the real step scripts under GitHub's exact bash flags with failure injection — thorough. Bonus fix: the npm cache key YAML quoting. No P0/P1.
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No code changes this round: the code itself is fully verified and approved, and the only open item (R8-9) targets the PR description, which this lane cannot edit. Point by point: 1. [Critical] R8-9 — the PR body must follow the PR template — VALID; NOT APPLICABLE BY THIS AGENT; author/maintainer action required. Unchanged since triage stage 1a (review 4841776175): the description still uses freeform Summary/Changes/Verification headings and has no Test Plan section. This agent has no channel to edit the PR description — the address-review lane's GitHub writes are limited to pushing commits, posting comments, and resolving/replying to review threads (verified against the workflow: there is no PR-body edit path). The paste-ready template-compliant body in the collapsed block below supersedes the draft in issue comment 5211486662: it folds in @wenshao's round-5 §5 correction (the two npm cache-key lines are formatter-required fixups of a violation 2. The review's "Not reviewed" / "Not linted" notes — INFORMATIONAL, no action. The test-efficacy probe's workspace-only discovery could not collect 3. @wenshao round-5 verification (issue comment) — ACKNOWLEDGED, no code action. The "ready to merge on the code" verdict is backed by the 15-scenario × 2-lane matrix (6 FIXED / 5 CONTROL / 3 ROBUST / 0 regressions), 8/8 mutant kills, and a provably contained blast radius (7 of 361 parsed leaf values differ). Its single open item — the body describing only about half the diff, quoting wording that is not in the diff, and citing a stale 120/120 count — is exactly R8-9, addressed by the updated body below, which covers the id handoff ( 4. @wenshao §5 correction (issue comment) — ACKNOWLEDGED, no code change, folded into the body draft. The two 5. @qwen-code-ci-bot sandboxed verification (issue comment) — ACKNOWLEDGED, no action. ✅ merge-ready, 128/128 scripted assertions at head 6. @yiliang114 APPROVED (review 4879802414, 2026-08-07T03:52:52Z) — ACKNOWLEDGED. Landed after the round-10 review and carries no findings: "LGTM, no blockers … No P0/P1." No action needed. Local checks (unchanged head
|
|
@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: 144 passed · 0 failed · 144 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：144 通过 · 0 失败 · 144 总计 Verification reportVerification report — PR #8436
|
| # | finding (round 3 at b8ac04ef) |
severity | status at b8ac04ef (round 4) |
|---|---|---|---|
| 1 | Stranded "running" corner: an older own running marker behind a newer marker is not reclaimed (newest-wins by design); creation needs an export loss plus a newer marker | informational | stands, re-measured (cell EE5) — fixture [running 44 older, tombstone 45] → claim PATCHes newest 45, orphan 44 untouched, exports 45. Still strictly narrower than the round-1 shape and base-identical (base also selected newest-wins), so not a regression. |
| 2 | jq-null corner on the POST arm: a payload without .id exports literal comment_id=null, finalize PATCHes comments/null |
informational | stands, re-measured (cells EE1a–c) — real jq emits literal null, [ -n ] accepts it, finalize PATCHes issues/comments/null; the best-effort guard still exits 0. Unreachable on the real comments API (201 always carries id). |
| 3 | Shared-slot interleavings transiently mislabel during overlapping runs; the qwen-triage-finalize.yml live-claim guard advertised by commit 4708d1ad never landed |
informational, pre-existing | stands, re-verified — git diff HEAD^1..HEAD -- .github/workflows/qwen-triage-finalize.yml is empty; update_status still selects ` |
Declined rows: none in round 3.
Scope
Central claim: the lifecycle comment always reaches a terminal state because the two steps are coupled by id, not by re-selection: (a) the claim step exports the exact comment id it wrote (comment_id — reused id on PATCH success, --jq '.id'-extracted created id on POST, empty on any write failure); (b) Finalize triage status comment runs under always() && number != '' (covering cancellation, which success() || failure() excludes) and PATCHes exactly that id — never lists, never matches bodies, no-ops on an empty id; (c) the wording is correct for every terminal (TRIAGE_OUTCOME, JOB_STATUS) combination, and "finished" never appears on a failed job.
Secondary claims: (1) every write arm survives a failing API under GitHub's real -eo pipefail (exit 0 + warning), and a failed claim write exports an empty id rather than a guess; (2) the two cache-key key: quote-style flips (tmux-testing, verify jobs) are semantic no-ops.
Merge-staleness check (new this round)
The snapshot's baseRefOid (95655daf) is 8000+ enumerated commits ahead of the verified base tip a8fb97d16 (the count is shallow-graph-inflated; the ancestry relationship is unresolvable at depth 2). What matters is file-level:
- Both files this PR touches are byte-identical on the new main tip to the verified base: blob
64e71788278fe8e17edd9953f4ad7c4de828f148(qwen-triage.yml) andea691f330c54ccde7c5a8730c62e65e7eb10450c(test file) at95655daf== same blobs ata8fb97d16. A trial merge therefore applies the PR hunks verbatim with zero conflict possibility. - Materialized the merged tree (
git worktree add tmp/merge-trial 95655daf, both files copied from head, hashes asserted equal to the head working-tree files), and re-ran the affected suites on the merged tree: vitest129 passed (129),node --test56 pass / 0 fail. Worktree removed afterwards (git worktree remove --force). - The only workflow-area churn on main since the base is
qwen-autofix.ymland its test — a different lane, untouched by this PR.
A/B result — finalize step (central claim a–c)
Harness ab-finalize.mjs extracts each arm's if: + run: verbatim from the YAML, evaluates the if: under GitHub status-function semantics (success() and failure() are both false on cancellation — precisely the gap the PR closes; the evaluator refuses unknown expression shapes), and executes the bash under bash --noprofile --norc -eo pipefail with a recording gh stub + real jq. Fixture: this run's claim id 43, RUN_URL=…/runs/77. Witness: 01-ab-finalize-base-vs-head.png; raw log logs/ab-finalize.log.
| cell | base (list-and-flip-newest, success()‖failure()) |
head (PATCH exported id, always()) |
|---|---|---|
| FA1 success/success, own 43 | PATCH 43 "finished" | PATCH 43 "finished", body byte-identical, exactly one API call |
| FA2 failure/failure, own 43 | PATCH 43 "early" (half-width ZH comma) | PATCH 43 "early" (full-width comma; bodies identical after normalizing that one character) |
| FA3 cancelled/cancelled (the bug) | condition false → SKIPPED, 🔄 claim stranded forever | PATCH 43 "was cancelled / 已取消", no other wording present |
| FA4 cancelled job, triage step success | skipped | PATCH 43 "finished" (triage success precedes the late cancel) |
| FA5 failed job, triage step success ('Check triage response' exits 1) | PATCH 43 "finished" — points at stage comments that never posted | PATCH 43 "ended early" |
| FA6 empty id; foreign tombstone 44 (runs/55) | PATCHes 44 — clobbers the previous run's terminal wording | zero writes + "claimed no status comment" diagnostic |
| FA7 empty id; empty thread | POSTs a fresh "early" comment | zero writes (accepted breadcrumb loss, documented in the step comment) |
| FA8 list API down, own 43 | conflates unreadable with empty (‖ EXISTING_ID='') → POSTs a duplicate |
PATCHes 43 — makes no list call at all, immune |
FA9 number='' |
skipped | skipped (guard parity) |
| FA10 two pages, own on page 2, cancelled | skipped | PATCHes 43 with one call (no pagination idiom needed) |
33/33 assertions, including the control cells asserting the base arm misbehaves exactly as predicted (FA3 skip, FA5 false "finished", FA6 clobber, FA8 conflation) — expected base failures count as passed assertions.
A/B result — claim step (central claim a, secondary 1)
ab-claim.mjs, same mechanics. The stub answers a POST with a JSON payload {"id": 7777} and applies the caller's --jq filter with real jq (as gh does client-side) — one fidelity level above the PR's own suite, whose stub echoes a bare id and therefore cannot exercise the --jq '.id' flag behaviorally (the suite pins it statically, and says so in a comment). Witness: 02-ab-claim-id-export.png; raw log logs/ab-claim.log.
| cell | base | head |
|---|---|---|
| CA1 empty thread | POST running claim | POST byte-identical body; --jq .id in raw args; exports comment_id=7777 extracted by real jq |
| CA2 tombstones 42+44 | PATCH newest 44 | PATCH 44 (42 untouched), exports 44 (shared-slot semantics unchanged) |
| CA3 legacy marker 41 | PATCH 41 | PATCH 41, exports 41 (single-slot healing kept) |
| CA4 forged non-bot marker | POST fresh (author filter) | POST fresh, exports 7777 |
| CA5 unreadable list | POST fresh (‖ EXISTING_ID=''), exit 0 |
POST fresh, exit 0, exports 7777 (parity with base, plus the id) |
| CA6 POST-fail / PATCH-fail | exit 0 + warning both arms | exit 0 + warning, exports EMPTY id both arms (never a guessed/stale id) |
| CA7 bot identity failure | exit 0, warn, no writes | same; writes no comment_id= line at all (finalize sees empty) |
29/29 assertions.
Head-only edge matrix + wording truth table
edges.mjs. Witness: 03-edges-truth-table.png; raw log logs/edges.log.
| cell | result |
|---|---|
| EE1a–c | jq-null corner re-measured end-to-end: POST payload {} → claim exports comment_id=null (real jq) → finalize PATCHes issues/comments/null, still exit 0 (best-effort holds). Unreachable on the real comments API. |
| EE2 | STATUS_COMMENT_ID fully unset (set -u safe via ${…:-}): exit 0, zero writes, diagnostic |
| EE3 | census across all 12 combos: finalize makes exactly one call per run, all PATCH — no list, no identity call (the no-re-selection property) |
| EE4 | exactly one comment_id= line per claim run in fresh / reuse / failed shapes |
| EE5 | stranded-corner measurement (Finding 1): fixture [running 44 older, tombstone 45] → claim PATCHes newest 45, orphan 44 untouched, exports 45 |
| EE6 | finished/cancelled/early bodies each start with the lifecycle marker and keep [view run]($RUN_URL) |
| EE7 | full truth table: 4 TRIAGE_OUTCOME × 3 JOB_STATUS = 12 combos, each asserting the expected wording EN+ZH present and the other two absent (the PR's own suite executes 7 of the 12); plus a dedicated sweep: no JOB_STATUS=failure combo ever says "finished" |
27/27 assertions.
Mutation matrix (vacuity of the new tests)
Seven point mutants of the head YAML applied in place (each target asserted single-occurrence before mutating), the vitest suite re-run per mutant, plus the matching executed harness for the six mutants whose behavior a harness exercises; file restored byte-exactly after each (sha256-verified), controls re-run at the end. Unmutated controls: suite 129/129 green; all three harnesses green. Witness: 04-mutation-matrix.png; raw log logs/mutation-matrix.log.
| mutant | suite | executed harness |
|---|---|---|
M1 claim POST arm: drop --jq '.id' |
RED (1 test — the static pin) | RED — ab-claim 25/29 (export becomes the raw payload, not 7777) |
| M2 swap cancelled↔early EN wording (positive control) | RED (5 tests) | — |
M3 drop the JOB_STATUS != failure gate on "finished" |
RED (2 tests) | RED — edges 25/27 (truth table + failure sweep) |
| M4 remove the empty-id no-op guard | RED (2 tests) | RED — ab-finalize 30/33 (writes appear where none may) |
| M5 claim exports the reused id even when the PATCH fails | RED (1 test) | RED — ab-claim 28/29 (stale 45 exported) |
M6 claim selector last→first |
RED (1 test) | RED — ab-claim 26/29 (PATCHes 42, exports 42) |
M7 finalize always()→success()‖failure() |
RED (1 test) | RED — ab-finalize 28/33 (condition shape + cancelled cell) |
No survivors; no mutant regressed from killed to survived. M1 is the attribution the last commit claims: the executed harness catches the dropped flag behaviorally, where the suite pins it only statically (the suite's stub bypasses --jq). 32/32 assertions including the restore proofs and post-mutation controls; final git status clean.
Corrections
- Round 3's correction still stands at this head (re-verified). Commit
4708d1addescribes teachingqwen-triage-finalize.yml'supdate_statusto "prefer the newest marker that is not a live running claim"; the aggregate diff contains no change to that file (git diff HEAD^1..HEAD --stat -- .github/workflows/qwen-triage-finalize.ymlempty), and the selector at head is still the unconditional| last |(line 328). This is a correction to the commit description, not a request to change the code: the harm that commit set out to fix no longer exists, because the finalize step now PATCHes the exported id — anupdate_statusoverwrite of a live claim's body can no longer break the flip. What remains is the pre-existing, base-identical cosmetic overwrite (Finding 3).
Findings (non-blocking, all carried from round 3 and re-measured)
- Stranded "running" comments can still persist in one narrow corner (informational). Re-measured in EE5: with
[running 44 older, tombstone 45], the claim reuses newest 45 and never touches 44. Creation requires the claim's PATCH to commit server-side but lose its response (emptycomment_idexport while the comment carries this run's running body) AND a newer marker to appear later. Not a regression vs base (base also selected newest-wins and had no reclaim), and every run's own comment is finalized by id. No action required; optionally name the response-loss half in the step comment (it currently names only the write-failure half). - jq-null corner on the POST arm (informational). Re-measured in EE1a–c: if a POST ever returned 200 without an
idfield,--jq '.id'emits literalnull,[ -n ]accepts it, finalize PATCHesissues/comments/null, gh 404s, and the best-effort guard warns and exits 0. Unreachable on the real comments API. No action required. - Shared-slot interleavings still transiently mislabel during overlapping runs; the advertised live-claim guard is absent (informational, pre-existing). The marker comment is a shared slot: run B's claim overwrites run A's running body, and A's later finalize flips the slot while B is live; the slot converges when B finalizes (EE7: all 12 wording combos land the correct terminal state). Base-identical and accepted by the slot design. Separately, see Corrections:
update_statusinqwen-triage-finalize.ymlcan still cosmetically overwrite a live claim until that run finalizes. No action required.
No new findings this round.
Not covered
- Uncalibrated replay — no GitHub token and no real lifecycle comment available locally, so the harnesses were not byte-calibrated against a production-emitted artifact (same as rounds 1–3). Partial corroboration: the claim bodies the harnesses flip are the exact bytes the head claim step composes (CA1), and actionlint fully parses both trees' YAML.
- Per-commit attribution — depth-2 checkout:
git rev-list HEAD^1..HEAD^2returns 1 commit (shallow boundary) against 25 in the metadata snapshot. The aggregateHEAD^1..HEADdiff (2 files) is what was verified. - yamllint gate — environmental, re-proven this round:
pip3: Permission deniedinscripts/lint.js --setup(quoted inlogs/lint-setup.log). YAML validity still covered by actionlint's full parse on both trees and every harness's YAML parse. - GHA server-side semantics —
timeout-minutes→job.statusvalue, andjob.statusavailability insideenv:(round-2 finding carried through round 3, stands). actionlint validates the context usage; the wording branch handles both readings and the 12-combo truth table covers every pair. - Live concurrency replay against the real API — Finding 3's interleavings are reasoned + cell-measured, not replayed on GitHub.
- Repo-wide eslint/lint — covered by the PR's own CI; the one changed non-workflow file passed a targeted
npx eslintrun (clean). - The PR description's test count ("120/120") is stale: the suite at head is 129 tests. No significance.
Methodology
Environment: the CI verify container (node:22-bookworm, node v22.23.2, jq 1.6; actionlint 1.7.12 + shellcheck installed pinned via scripts/lint.js --setup into /tmp/qwen-code-linters/local-*; yamllint uninstallable). The PR touches only workflow YAML and one test file — no compiled packages, no dependency changes — so the A/B "builds" are the two trees' YAML: head .github/workflows/qwen-triage.yml (sha256 8b7b4b94…) vs the base file dumped from git show HEAD^1 (sha256 909ba7fd…, equal to round-3's cited hash and to the blob at the new main tip 95655daf). The code under test is the verbatim-extracted bash executed under GitHub's exact shell flags with real jq and a recording gh stub that applies --jq filters like gh does and infers POST from -f fields like gh does (no stub of the code under test itself); if: conditions are evaluated under GitHub status-function semantics by a strict parser that refuses unknown shapes. Harnesses (in this dir): ab-finalize.mjs (33), ab-claim.mjs (29), edges.mjs (27), mutate.mjs (7 mutants + controls = 32), gates.mjs (23) = 144/144. Suite gates additionally executed (not counted in assertions.json): vitest 129/129 at head and on the merged tree; node --test 56/56 at head and on the merged tree. Mutation matrix: each mutant applied byte-exactly with single-occurrence asserted, suite (+ executed harness where applicable) re-run, file restored and sha256-verified; final git status clean. Raw per-run logs in logs/; evidence PNGs via scripts/verify-capture.mjs.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review. The bot already has a review of its own on 机器人在 The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — no action needed (PR #8436)Triaged all feedback newer than the last evaluation (2026-08-07T03:40:37Z). Nothing this round requires a code change:
No code changes were made, no commits were added, and the branch remains at VerificationNo runnable verification commands were executed this round because no code was changed. Read-only checks actually run:
中文说明Autofix 评审轮次 — 无需操作（PR #8436）已对上次评估（2026-08-07T03:40:37Z）之后的全部反馈进行分类。本轮没有任何需要修改代码的事项：
本轮未修改任何代码、未新增任何提交，分支保持在 验证由于本轮未改动任何代码，未执行任何可运行的验证命令。实际执行的只读检查如下：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: 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. 中文 — 判定：
|
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed at b8ac04ef99c37cab53b28d2c80cbfe55f853f557. The workflow change itself is clean — I found no code-level defect and would approve on the diff alone. I am posting a Comment instead of an approval because one Critical still stands, and it is the PR description, not the code.
The one blocker: the PR body still does not follow the template
I checked this against the current body rather than trusting the flag. It uses Summary / Changes / Verification; .github/pull_request_template.md requires What this PR does, Why it's needed, Reviewer Test Plan (with How to verify, Evidence (Before & After), Tested on), Risk & Scope, and Linked Issues. There is no Test Plan section at all. This is the same R8-9 that has survived ten rounds, and it survives them because the autofix path can push commits but cannot edit a PR body — so it needs one human edit. A paste-ready template-conformant body reflecting the current id-handoff design is already in the thread (issue comment 5211486662). Apply it and re-run @qwen-code /triage, and I will flip this to an approval; nothing in the code is holding it up.
Everything else checks out — all four round-6 Criticals are gone
- R6 job-scope Critical: fixed. The
finishedbranch is now gated on[ "${JOB_STATUS:-}" != 'failure' ], so a red job can no longer post ✅ and point at stage comments that were never written. - R6 harness-fidelity Critical: fixed. The executed harness now spawns
bash --noprofile --norc -eo pipefail, matching what GitHub actually givesshell: bash. That matters here: a step's ownset -uo pipefaildoes not turn-eback off, so a harness without-ewould stay green while an unguarded write killed the real step. - The two ownership-selection Criticals no longer have a subject. The
foreign/emptyclassification and the newest-marker-then-check-ownership selector are gone; finalize PATCHes exactly the id the claim step exported. That is the right shape — one selection in the lifecycle instead of two that can disagree. - The cross-workflow contract survives. Both bodies still start with
<!-- qwen-triage lifecycle -->, soqwen-triage-finalize.yml'supdate_status— the third writer of this comment — still finds it.
What I verified myself
I extracted the two real step scripts from this commit's workflow and ran them under GitHub's exact shell flags against a stubbed gh, independently of the PR's own harness.
Finalize, all nine cases exit 0 (best-effort holds under -e), body always prefixed with the lifecycle marker, PATCH always aimed at the claimed id: green job → finished; triage ok + job cancelled → finished; triage ok + job failed → ended early; triage cancelled → cancelled; triage failed + job cancelled → cancelled; triage failed + job failed → ended early; triage skipped + job failed → ended early; empty id → no write at all, only the "claimed no status comment" note; PATCH failure → warns and still exits 0.
Claim, all six cases exit 0: no marker → POST, and the POST arm's --jq '.id' really does extract the id from a realistic comments-API payload (comment_id=90210) — that is the substance of the still-open R9-2, so the residual there is coverage, not correctness; own marker → PATCH reuse exporting the reused id; two own markers → newest wins (44, not 42); legacy marker → reused; foreign author → ignored, fresh POST; marker not at the start of the body → ignored, so the startswith gate holds.
The success + cancelled → finished combo initially looked like a mislabel to me, but the triage agent posts its stage comments during its own step, so a cancel arriving afterwards means the verdict already landed. It is a deliberate, tested choice, and I agree with it.
Nit, deferred
The two npm-ci-${{ hashFiles(...) }} cache-key requoting hunks are semantically identical YAML and unrelated to this fix. Past round five I would not widen the diff to remove them.
中文说明
在 b8ac04ef99 上完成审查。工作流改动本身是干净的——我没有发现任何代码层面的缺陷，单看 diff 我会批准。之所以提交 Comment 而不是批准，是因为仍有一条 Critical 成立，而它出在 PR 描述而非代码上。
唯一的阻断项：PR 正文仍未遵循模板。 我核对了当前正文而非仅凭标记：现在用的是 Summary / Changes / Verification，而 .github/pull_request_template.md 要求 What this PR does、Why it's needed、Reviewer Test Plan（含 How to verify、Evidence (Before & After)、Tested on）、Risk & Scope、Linked Issues，且完全没有 Test Plan 一节。这就是撑过十轮的那条 R8-9；它之所以一直存活，是因为 autofix 路径只能推提交、无法修改 PR 正文——所以需要人工改一次。线程里已经有一份反映当前 id-handoff 设计、可直接粘贴的模板化正文（issue comment 5211486662）。贴上后重新运行 @qwen-code /triage，我就把这条改为批准；代码侧没有任何东西在拖后腿。
其余全部通过——第 6 轮的四条 Critical 均已消除。 job 作用域那条已修复：finished 分支现在带 [ "${JOB_STATUS:-}" != 'failure' ] 门，红 job 不再可能发出 ✅ 并指向从未写出的阶段评论。测试保真度那条已修复：执行型 harness 现在以 bash --noprofile --norc -eo pipefail 启动，与 GitHub 实际给 shell: bash 的一致——这在此处很关键，因为步骤自身的 set -uo pipefail 并不会关掉 -e，缺少 -e 的 harness 会在真实步骤已被未加保护的写操作打挂时仍然全绿。另外两条关于归属选择的 Critical 已无对象：foreign / empty 分类与「先取最新 marker 再判归属」的选择器都已移除，finalize 直接 PATCH 认领步骤导出的那个 id——这个形态是对的，生命周期里只剩一次选择，而不是两次可能互相矛盾的选择。跨工作流契约仍然成立：两处正文都以 <!-- qwen-triage lifecycle --> 开头，qwen-triage-finalize.yml 的 update_status（该评论的第三个写入方）仍能找到它。
我自己的验证： 我从本提交的工作流中抽出两个真实步骤脚本，在 GitHub 完全相同的 shell 参数下针对 stub 过的 gh 执行，独立于本 PR 自带的 harness。finalize 九种情形全部 exit 0（-e 下 best-effort 成立），正文始终以生命周期 marker 开头，PATCH 始终指向认领的 id：绿 job → finished；triage 成功 + job 取消 → finished；triage 成功 + job 失败 → ended early；triage 取消 → cancelled；triage 失败 + job 取消 → cancelled；triage 失败 + job 失败 → ended early；triage 跳过 + job 失败 → ended early；空 id → 完全不写，只输出「未认领评论」提示；PATCH 失败 → 告警并仍然 exit 0。claim 六种情形全部 exit 0：无 marker → POST，且 POST 分支的 --jq '.id' 在真实形状的 comments API 载荷上确实提取出了 id（comment_id=90210）——这正是仍未关闭的 R9-2 的实质，因此那里的遗留是覆盖问题而非正确性问题；自有 marker → PATCH 复用并导出复用的 id；两条自有 marker → 取最新（44 而非 42）；legacy marker → 复用；他人作者 → 忽略并新发 POST；marker 不在正文开头 → 忽略，说明 startswith 门有效。success + cancelled → finished 这一组我起初以为是误标，但 triage agent 的阶段评论是在它自己那一步里发出的，因此之后到达的取消意味着结论已经落地——这是有意为之且有测试钉住的选择，我同意。
已推迟的小问题： 两处 npm-ci-${{ hashFiles(...) }} 缓存 key 的引号改写在 YAML 语义上完全等价，且与本次修复无关。已过第 5 轮，我不会为了删掉它们而扩大 diff。
|
Triage re-run completed without a new review. The bot already has a review of its own on 机器人在 The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Approving at b8ac04ef99c37cab53b28d2c80cbfe55f853f557 — same commit I reviewed above; head, body, and check results are unchanged since then, so that verification still stands.
Why this is an approval
The workflow change is correct and the code side is clean. All four round-6 Criticals are gone at this commit:
- Job-scope Critical: fixed. The
finishedbranch is gated on[ "${JOB_STATUS:-}" != 'failure' ], so a red job can no longer post ✅ and point at stage comments that were never written. - Harness-fidelity Critical: fixed. The executed harness spawns
bash --noprofile --norc -eo pipefail, matching what GitHub actually givesshell: bash— which matters precisely because a step's ownset -uo pipefaildoes not turn-eback off. - The two ownership-selection Criticals no longer have a subject. The
foreign/emptyclassification and the newest-marker-then-check-ownership selector are gone; the claim step exports the id it wrote and finalize PATCHes exactly that id. One selection in the lifecycle instead of two that can disagree — that is the right shape, and it is what makes the original fix (always()covering cancellation) safe rather than racy. - Cross-workflow contract preserved. Both bodies still start with
<!-- qwen-triage lifecycle -->, soqwen-triage-finalize.yml'supdate_status— the third writer of this comment — still finds it.
I verified the behaviour myself rather than reading the assertions: I extracted the two real step scripts from this commit and ran them under GitHub's exact shell flags against a stubbed gh. Finalize, nine cases, all exit 0, body always marker-prefixed, PATCH always aimed at the claimed id — green→finished, triage-ok+cancelled→finished, triage-ok+failed→ended early, triage-cancelled→cancelled, triage-failed+cancelled→cancelled, triage-failed+failed→ended early, triage-skipped+failed→ended early, empty id→no write at all, PATCH failure→warns and still exits 0. Claim, six cases, all exit 0 — no marker→POST with the real --jq '.id' extracting the id from a realistic payload, own marker→PATCH reuse, two own markers→newest wins, legacy marker→reused, foreign author→ignored, marker not at body start→ignored.
Deferred, recorded so it is not silently dropped
R8-9 (PR description does not follow .github/pull_request_template.md) is not fixed — it is being deferred by maintainer decision, superseding my earlier Comment on this PR. The body still uses Summary / Changes / Verification with no Reviewer Test Plan, Risk & Scope, or Linked Issues section. It is a description-formatting item with no bearing on the code, it has survived ten rounds only because the autofix path can push commits but cannot edit a PR body, and per the review-round guidance this PR should now land Critical code fixes only. A paste-ready template-conformant body reflecting the current id-handoff design is in the thread (issue comment 5211486662) if anyone wants to apply it before merge.
Also deferred: the R9-2 coverage gap (the PR's own stub answers a POST with the already-extracted id, so only the static --jq '.id' pin guards that flag — I confirmed the extraction itself works, so this is coverage, not correctness), the R9-3 accepted loss of the pre-PR list-then-POST recovery path when the claim's write fails (documented in the step comment), and the two unrelated npm-ci-${{ hashFiles(...) }} cache-key requoting hunks, which are semantically identical YAML.
中文说明
在 b8ac04ef99c37cab53b28d2c80cbfe55f853f557 上批准 —— 与我上一条评审是同一个提交，head、正文与检查结果自那时起均未变动，因此那次验证依然有效。
为什么给批准： 工作流改动是正确的，代码侧是干净的，第 6 轮的四条 Critical 在本提交上全部消除。job 作用域那条已修复：finished 分支带 [ "${JOB_STATUS:-}" != 'failure' ] 门，红 job 不再可能发出 ✅ 并指向从未写出的阶段评论。测试保真度那条已修复：执行型 harness 以 bash --noprofile --norc -eo pipefail 启动，与 GitHub 实际给 shell: bash 的一致——这恰恰关键，因为步骤自身的 set -uo pipefail 并不会关掉 -e。另外两条关于归属选择的 Critical 已无对象：foreign / empty 分类与「先取最新 marker 再判归属」的选择器都已移除，认领步骤导出它写入的 id，finalize 精确 PATCH 该 id——生命周期里只剩一次选择而非两次可能互相矛盾的选择，这个形态是对的，也正是它让最初那个修复（用 always() 覆盖取消）从「有竞态」变成「安全」。跨工作流契约保持不变：两处正文仍以 <!-- qwen-triage lifecycle --> 开头，qwen-triage-finalize.yml 的 update_status（该评论的第三个写入方）仍能找到它。
我没有只读断言，而是自己验证了行为：从本提交抽出两个真实步骤脚本，在 GitHub 完全相同的 shell 参数下针对 stub 过的 gh 执行。finalize 九种情形全部 exit 0，正文始终以 marker 开头，PATCH 始终指向认领的 id——绿 job→finished、triage 成功+取消→finished、triage 成功+失败→ended early、triage 取消→cancelled、triage 失败+取消→cancelled、triage 失败+失败→ended early、triage 跳过+失败→ended early、空 id→完全不写、PATCH 失败→告警并仍然 exit 0。claim 六种情形全部 exit 0——无 marker→POST 且真实的 --jq '.id' 从真实形状载荷中取出了 id、自有 marker→PATCH 复用、两条自有 marker→取最新、legacy marker→复用、他人作者→忽略、marker 不在正文开头→忽略。
已推迟并在此记录，避免被静默丢弃：R8-9（PR 描述未遵循 .github/pull_request_template.md）并未修复，而是按维护者决定推迟处理，本条取代我此前在本 PR 上的 Comment 立场。 正文仍使用 Summary / Changes / Verification，缺少 Reviewer Test Plan、Risk & Scope、Linked Issues 各节。这是描述格式问题，与代码无关；它之所以撑过十轮，仅因为 autofix 路径只能推提交、无法修改 PR 正文；按评审轮次约定，本 PR 此时应只落 Critical 级别的代码修复。若希望在合并前补上，线程里（issue comment 5196984782 的更新版 5211486662）已有一份反映当前 id-handoff 设计、可直接粘贴的合规正文。
同时推迟的还有：R9-2 的覆盖缺口（本 PR 自带的 stub 对 POST 直接返回已提取的 id，因此该标志只由静态的 --jq '.id' 断言把关——我已确认提取本身可用，所以这是覆盖问题而非正确性问题）、R9-3 中被接受的「认领写入失败时丢失 PR 前的 list-then-POST 恢复路径」（已在步骤注释中记录），以及两处与本次修复无关的 npm-ci-${{ hashFiles(...) }} 缓存 key 引号改写（YAML 语义完全等价）。
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round: no code changes — the one remaining blocker needs a single manual PR-body edit The new feedback on Feedback triage
No code changes were made and nothing was committed this round; the tree is unchanged at 中文说明Autofix 审查轮次：无代码改动 —— 唯一剩余的阻断项需要一次性手工修改 PR 正文 对 反馈分类
本轮未做任何代码改动，也未提交任何内容；工作树保持在 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
…ne (QwenLM#8676) * feat(autofix): bridge fork-PR reviews into the credentialed review lane A workflow run tied to a pull request whose head lives in a fork is created with `Secret source: None`, so `secrets.CI_DEV_BOT_PAT` is empty and the autofix review lane cannot authenticate. Those reviews therefore wait for the scheduled scan, which GitHub throttles hard — the `*/10` cron lands every 40-70 minutes on this repo. Reading the PR with `github.token` instead would not help: review-address still needs the PAT to push and comment, and it is empty for the whole run. The credential has to come from a different run. This is the standard `workflow_run` bridge for that. - `qwen-autofix-fork-signal.yml` runs on `pull_request_review`, where the fork PR's event can reach it, and records only which PR was reviewed. It is deliberately powerless: `permissions: {}`, no checkout, no repository code, and the hosted pool rather than the persistent self-hosted one. - `qwen-autofix-fork-bridge.yml` runs on `workflow_run`, which GitHub creates on the default branch with the repository's own token, and dispatches `qwen-autofix.yml` for that PR using `actions: write` — the same lever Fleet Shepherd already pulls. It holds no PAT. The dispatched run re-derives admission from live API state, so the PR number says when to look, never who may be touched. It is bound to `workflow_run.head_sha` regardless: the signal run carries the reviewed PR's head SHA, and only that PR has that head. Measured, not assumed: - `workflow_run.pull_requests` is empty for fork PRs, and `/commits/{sha}/pulls` does not resolve a fork head either (it does resolve an in-repo one), so the artifact is the only way to learn the number. - `pull_request_review` runs the workflow file from the BASE branch: run 31152873061's PR branch predates a main-only change to `qwen-autofix.yml` and it still executed the new file. A fork cannot edit what the signal writes. - That run's `head_sha` equals PR QwenLM#8436's `headRefOid` exactly, which is what makes the binding possible. The bridge's validate-and-dispatch block is replayed under bash against a stub API: forgery, an unreadable head, both heads empty, non-numeric and path-shaped artifacts, a closed or retargeted PR, download and read failures, and dispatch retry then exhaustion. * fix(autofix): align fork bridge with route admission and coalesce bridged dispatches Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(autofix): harden the fork-review bridge per review feedback (QwenLM#8676) --------- Co-authored-by: verify <verify@local> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
|
Released in v0.21.8. |
5 participants
Summary
The
Finalize triage status commentstep inqwen-triage.ymlruns under(success() || failure()), which excludes cancellation — and both the job-leveltimeout-minutes: 30(reachable on large PRs) and a manual cancel surface as cancellation. When that happens the step is skipped and the early "🔄 Qwen Triage is running" lifecycle comment keeps claiming the run is in progress forever (until some later trigger happens to overwrite the same marker). The verify lane has the separatepublish-verifyjob to catch its cancelled case; the triage lane has no other publisher.Changes
always() && steps.resolve.outputs.number != ''— the number guard already keeps it out of runs that never resolved a target, and the step body is best-effort throughout (set -uo pipefail+||fallbacks on every write), so widening the condition cannot fail the job.job.statusfor wording: a cancelled run now says "🚫 Qwen Triage was cancelled … re-run to get a result" (bilingual) instead of being silently skipped or mislabeled "ended early".always()condition, theJOB_STATUSenv, and the cancelled wording; negative-pinsuccess() || failure()out.Verification
npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-triage-workflow.test.js— 120/120 passnode --test .github/scripts/qwen-triage-workflow.test.mjs— all pass, 0 failifand env render as intended中文说明
摘要
qwen-triage.yml的Finalize triage status comment步骤条件是(success() || failure())，不覆盖取消——而 job 级timeout-minutes: 30（大 PR 可能触及）和手动取消都以取消语义收场。此时该步骤被跳过，先前发出的 "🔄 Qwen Triage is running" 生命周期评论会永远停留在"进行中"（除非之后恰好有新触发覆盖同一 marker）。verify 车道有独立的publish-verifyjob 兜底取消场景；triage 车道没有其他发布方。变更
always() && steps.resolve.outputs.number != ''——number 门保证未解析出目标的 run 不会误发评论；步骤本体已是全程 best-effort（set -uo pipefail+ 所有写操作带||兜底），放宽条件不会使 job 失败。job.status分支措辞：取消的 run 现在显示 "🚫 Qwen Triage 已取消 …… 重新运行以获得结果"（双语），而不是被跳过或误标为"提前结束"。always()条件、JOB_STATUSenv 与取消措辞，并负向钉死success() || failure()不得回归。验证
npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-triage-workflow.test.js— 120/120 通过node --test .github/scripts/qwen-triage-workflow.test.mjs— 全部通过if与 env 渲染符合预期