ci(release): report required Test checks on release PRs and auto-approve - #5250
Conversation
Release-sync PRs skipped the matrix Test job at job level, so the required "Test (os, Node 22.x)" contexts were never reported and the PR could not merge. Move the skip to step level so the job still expands its matrix and concludes as a work-free pass. Also add a CI_DEV_BOT_PAT approval on the release PR to cover one of the two required reviews.
|
Thanks for the PR @yiliang114! Template looks good ✓ On direction: This fixes a real operational deadlock — release-sync PRs get stuck waiting for required On approach: Tight and well-scoped. Moving
Moving on to code review. 🔍 中文说明感谢 @yiliang114 的 PR! 模板完整 ✓ 方向:这修的是一个真实的 CI 死锁——release-sync PR 因为 matrix job 在 job 级被 skip,导致 required 的 方案:范围精准。把
进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal (before reading diff): To fix the release PR deadlock, I'd do two things: (1) change the Diff comparison: The PR's approach matches this exactly. Clean, focused, every line earns its place:
No correctness bugs. No security concerns. The Validation performed:
TestingThis is a pure CI/workflow change with no user-visible or TUI behavior — tmux testing is N/A (as the author's own test plan states). The only way to verify end-to-end is on an actual release-sync PR, which can't be exercised locally. YAML validation confirms the workflow files are syntactically correct. 中文说明代码审查独立方案(读 diff 之前): 要修复 release PR 死锁,我会做两件事:(1) 让 Diff 对比: PR 的方案与我的完全一致。干净、聚焦,每一行都有存在的理由:
无正确性 bug,无安全问题。 已验证:
测试这是纯 CI/工作流改动,没有用户可见或 TUI 行为——tmux 测试不适用(作者自己的测试计划也标注了 N/A)。唯一能端到端验证的方式是在真实的 release-sync PR 上运行,这无法在本地模拟。YAML 验证确认工作流文件语法正确。 — Qwen Code · qwen3.7-max |
VerdictApproving. This is a clean, focused fix for a real operational problem — stuck release PR #5243 is concrete evidence this isn't theoretical. The fix is the textbook solution: move My independent proposal matched the PR's approach exactly. Every line in the diff earns its place — no scope creep, no unnecessary abstraction, no drive-by refactors. YAML validation passes on both files. If I had to maintain this in six months, I'd thank the author for the clear inline comment explaining why the job-level guard moved. One non-blocking follow-up: 中文说明结论通过。这是一个干净、聚焦的修复,解决了一个真实的运维问题——卡住的 release PR #5243 是具体证据,这不是假设性的问题。 修复方案是标准做法:把 我的独立方案与 PR 的方案完全一致。diff 中的每一行都有存在的理由——没有范围蔓延,没有不必要的抽象,没有顺手重构。两个文件的 YAML 验证都通过。如果六个月后我来维护这段代码,我会感谢作者写了清晰的内联注释解释为什么移动了 job 级守卫。 一个非阻塞的跟进: — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
There was a problem hiding this comment.
Pull request overview
Updates the automated release workflows so release version-bump PRs (chore(release): vX.Y.Z) can satisfy branch protection requirements and progress to merge with less manual intervention.
Changes:
- Adjusts CI
testmatrix behavior so required per-OSTest (...)check contexts are always reported, while still no-oping work on release-sync PRs via per-step guards. - Adds an automated “Approve release PR” step in the release workflow to contribute one of the required approvals using a separate bot PAT.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
.github/workflows/release.yml |
Auto-approves the created release PR using CI_DEV_BOT_PAT before enabling auto-merge. |
.github/workflows/ci.yml |
Ensures the test matrix always expands (thus reporting required Test (...) contexts) and moves skip_ci gating to individual steps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
After dropping the inner 'true' comparison, double quotes are no longer required, so yamllint's quoted-strings rule wants single quotes.
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 /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
What this PR does
Two changes to the automated release flow, both aimed at letting the
chore(release): vX.Y.Zversion-bump PR merge without manual babysitting.First, it fixes a deadlock in CI for release-sync PRs.
classify_prdetects these PRs and setsskip_ci=true, and thetestjob carried that condition at the job level. A matrix job skipped at job level never expands, so the per-OS contextsTest (ubuntu-latest, Node 22.x),Test (macos-latest, Node 22.x), andTest (windows-latest, Node 22.x)— which are required by branch protection — were never reported. The PR then sat forever waiting for status checks that would never arrive. This moves theskip_ciguard from the job level down to each working step. The job (and its matrix) now always runs, but on release PRs every step no-ops, so each required context reports a work-free success that satisfies branch protection.Second, it adds an
Approve release PRstep inrelease.ymlthat usesCI_DEV_BOT_PATto approve the release PR right after it is created.mainrequires two approving reviews; this covers one of them automatically, so a maintainer only has to add the second.Why it's needed
Release PRs like #5243 currently cannot auto-merge: the required
Test (...)contexts never report (somergeStateStatusstays blocked), and the two required approvals are entirely manual. The release job already enables auto-merge, but it can never fire. After this change the required checks report, one approval is supplied by the bot, and the release PR is one human click away from merging.Reviewer Test Plan
How to verify
The behavior only manifests on an actual release-sync PR (head
release/*, titlechore(release):, authorqwen-code-ci-bot, same-repo), so this is verified by reading the workflow logic plus YAML validation rather than a local run.testjobifno longer referencesskip_ci; it is now${{ !cancelled() }}, so the matrix always expands and the three requiredTest (...)contexts are always created.test(Checkout, Set up Node, Configure npm, Install dependencies, Run tests) and the threealways()upload/report steps now carryneeds.classify_pr.outputs.skip_ci != 'true', so on a release PR they all skip and the job concludes as success with no work done.skip_ciisfalse, so every step runs exactly as before — no behavior change.release.ymlapproves the PR withCI_DEV_BOT_PATbefore enabling auto-merge.Prerequisites this PR depends on (repo config, not code):
CI_DEV_BOT_PATmust belong to a bot account distinct from the PR authorqwen-code-ci-bot(GitHub forbids self-approval) and that account must have write access for the approval to count. Both are confirmed in place.Evidence (Before & After)
N/A — CI/workflow change, no user-visible or TUI output.
actionlintand YAML parsing pass for both files.Tested on
Environment (optional)
N/A — workflow-only change; no local runtime involved.
Risk & Scope
Test (...)checks report success without actually running tests. This is intentional — the release was already validated byquality+integrationinrelease.ymlbefore publishing, so re-running on the version-bump PR is redundant. The skip path is gated by all fourclassify_prconditions, so it cannot be triggered by a normal or fork PR.Linked Issues
Surfaced by the stuck release PR #5243 (referenced, not auto-closing).
中文说明
这个 PR 做了什么
对自动 release 流程做两处改动,目标都是让
chore(release): vX.Y.Z版本号 PR 不用人工盯着就能合并。其一,修复 release-sync PR 在 CI 上的死锁。
classify_pr会识别这类 PR 并设skip_ci=true,而testjob 之前把这个条件写在 job 级。matrix job 在 job 级被跳过时根本不会展开,于是分支保护要求的三个 per-OS contextTest (ubuntu-latest, Node 22.x)/ macos / windows 永远不会上报,PR 就一直卡在等待这些永远不会到来的状态检查上。本 PR 把skip_ci守卫从 job 级下沉到每个干活的 step:job(及其 matrix)始终运行,但在 release PR 上每个 step 都空跑,于是每个 required context 都报一个“没干活的 success”,满足分支保护。其二,在
release.yml新增Approve release PR步骤,在 PR 创建后立即用CI_DEV_BOT_PAT自动 approve。main要求两个 approve,这一步自动覆盖其中一个,维护者只需再点一次。为什么需要
像 #5243 这样的 release PR 目前无法自动合并:required 的
Test (...)context 从不上报(mergeStateStatus一直 blocked),两个 required approve 也全靠手动。release job 虽然已开启 auto-merge,但永远触发不了。改完后 required check 能上报、一个 approve 由 bot 提供,release PR 离合并只差一次人工点击。审查验证
该行为只在真实的 release-sync PR(head
release/*、标题chore(release):、作者qwen-code-ci-bot、同仓库)上出现,因此通过审阅工作流逻辑 + YAML 校验来验证,而非本地运行。testjob 的if不再引用skip_ci,改为${{ !cancelled() }},matrix 始终展开,三个 requiredTest (...)context 始终被创建。test里所有干活的 step(Checkout / Set up Node / Configure npm / Install deps / Run tests)以及三个带always()的上传/报告 step 都加上了skip_ci != 'true',release PR 上全部跳过,job 以 success 收尾且不做任何实际工作。skip_ci为false,每个 step 行为与之前完全一致,无变化。release.yml在开启 auto-merge 之前用CI_DEV_BOT_PATapprove 该 PR。依赖的前提(属仓库配置,非代码):
CI_DEV_BOT_PAT必须是与 PR 作者qwen-code-ci-bot不同的 bot 账号(GitHub 禁止给自己的 PR approve),且该账号需有 write 权限,approve 才计入 required reviews。两者均已确认到位。风险与范围
Test (...)check 会在不真正跑测试的情况下报 success。这是有意为之——release 在发布前已由release.yml的quality+integration验证过,版本号 PR 再跑一遍属冗余。跳过路径受classify_pr四个条件共同约束,普通 PR / fork PR 无法触发。