fix(cli): ignore current review run in presubmit CI - #6397
Conversation
|
Thanks for the PR! Template looks good ✓ Problem: This is an observed bug with clear evidence. In PR #6387, the review bot produced an approving verdict but the published review was downgraded to Direction: Aligned. The fix targets exactly the right layer — CI classification in Approach: Scope feels right. Two files — a surgical filter in 中文说明感谢贡献! 模板完整 ✓ 问题: 已观测到的 bug,证据清晰。PR #6387 中,bot 给出了 approve 结论,但实际 review 被降级为 方向: 对齐。修复精准定位在 方案: 范围合理。两个文件—— — Qwen Code · qwen3.7-max |
Code ReviewClean, surgical fix. Two files, minimal surface area.
One thing worth noting: when the filter removes all check-runs and there are no statuses, the classification becomes
No blockers found. No AGENTS.md violations. TestingNew regression test (presubmit.test.ts)Existing review tests (regression check)TypecheckAll 23 tests pass, typecheck clean. 中文说明代码审查干净、精确的修复。两个文件,最小改动面积。
值得注意的一点:当过滤器移除所有 check-run 且没有 statuses 时,分类结果变为
未发现阻塞问题。未违反 AGENTS.md 规范。 测试所有 23 个测试通过,typecheck 无错误。 — Qwen Code · qwen3.7-max |
|
This is a clean fix for a real, observed problem. PR #6387 showed the bot approving but getting downgraded to The approach is the right one: filter by My independent proposal before reading the diff was essentially identical — filter the current run's check-run out of Tests pass (23/23), typecheck clean. Ships it. ✅ 中文说明这是一个针对真实、已观测问题的干净修复。PR #6387 证明了 bot 给出 approve 但被降级为 方案正确:通过 我在看 diff 之前的独立方案基本相同——在 测试全通过(23/23),typecheck 无错误。可以合入。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
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. |
doudouOUC
left a comment
There was a problem hiding this comment.
Qwen Code Review Summary for PR #6397
Review completed with 3 inline comments posted (all suggestions, no blockers).
Findings
- Test coverage —
presubmit.test.tsfocuses on a single happy-path scenario; several new code paths inpresubmit.ts(e.g.,GITHUB_RUN_IDunset, non-matching URLs, mixed status states) are not exercised. - Observability —
classifyCifilters out the current Actions run silently. The output JSON and logs expose no field (e.g.,filteredSelfRuns) indicating what was excluded, making future misclassifications hard to debug. totalChecksambiguity — The reportedtotalChecksuses the post-filter count, so a PR whose only check is the review run itself reports0checks, indistinguishable from a repository with no CI.
Verdict
No critical issues found. The fix is correct and narrowly scoped — URL substring matching with /actions/runs/${runId}/ is safe, and the filter is applied consistently across all usage sites. The inline comments are non-blocking suggestions for additional coverage and observability.
|
本轮已处理 review comments:
验证: |
What this PR does
Updates
/review presubmitCI classification so the currently running GitHub Actions review run does not count as pending CI for its own approval decision. The filter is intentionally narrow: it only ignores check-runs whosedetails_urlorhtml_urlbelongs to the currentGITHUB_RUN_ID.Adds a regression test for the observed failure mode where the real PR CI has already completed successfully but
🧐 Qwen Pull Request Review / review-pris stillin_progress.Why it's needed
The automated Qwen Code review flow can otherwise become self-referential. In PR #6387, the bot found no issues and produced an approving verdict, but the published review was downgraded to
COMMENTEDwithCI still runningbecause thereview-prjob was still running at the moment it submitted the review.This prevents a clean bot approval from clearing stale bot review state, even when the only pending check is the review workflow itself. Real PR CI and commit statuses still participate in the pending/failing gate.
such as #6397 (comment)
Reviewer Test Plan
How to verify
Run the focused review command tests and CLI typecheck:
The new regression test should show that a pending check-run from the current
GITHUB_RUN_IDis ignored, while the remaining successful PR CI lets the presubmit report stayall_passwithout aCI still runningdowngrade reason.Evidence (Before & After)
N/A — non-UI CI classification fix.
Tested on
Environment (optional)
Local worktree under
/tmp/qwen-review-ci-self-check;npm cicompleted before verification.Risk & Scope
GITHUB_RUN_ID, not by job name or workflow name./reviewbehavior when noGITHUB_RUN_IDis present. Local runs continue to classify remote pending checks the same way as before.Linked Issues
Resolves #6396
中文说明
这个 PR 做了什么
更新
/review presubmit的 CI 分类逻辑,让当前正在运行的 GitHub Actions review run 不再把自己的 check-run 当作 pending CI。过滤范围刻意保持很窄:只忽略details_url或html_url属于当前GITHUB_RUN_ID的 check-run。新增回归测试覆盖已观察到的问题:真实 PR CI 已经成功完成,但
🧐 Qwen Pull Request Review / review-pr自己仍处于in_progress时,不应把审批降级成 comment。为什么需要
自动 Qwen Code review 流程之前会形成自引用等待。PR #6387 中,bot 没发现问题并给出 approve verdict,但实际发布的 review 被降级成
COMMENTED,原因是CI still running;当时正在 pending 的正是同一个review-prjob 自己。这会阻止干净的 bot approve 清掉旧的 bot review 状态,即使唯一 pending 的 check 是 review workflow 自己。真实 PR CI 和 commit status 仍然会继续参与 pending/failing gate。
Reviewer Test Plan
How to verify
运行聚焦的 review 命令测试和 CLI typecheck:
新增回归测试应证明:来自当前
GITHUB_RUN_ID的 pending check-run 会被忽略;其余真实 PR CI 成功时,presubmit report 保持all_pass,并且不会出现CI still running降级原因。Evidence (Before & After)
N/A — 非 UI 的 CI 分类修复。
Tested on
Environment (optional)
本地 worktree:
/tmp/qwen-review-ci-self-check;验证前已完成npm ci。Risk & Scope
GITHUB_RUN_ID匹配当前 Actions run URL,不按 job name 或 workflow name 泛化过滤。GITHUB_RUN_ID的本地/review行为。本地运行仍和之前一样分类远端 pending checks。Linked Issues
Resolves #6396