feat(autofix): extend review loop to all dev-bot PRs, add real-time triggers - #6528
Merged
Conversation
qwen-code-ci-bot
requested changes
Jul 8, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Hey @wenshao — the PR body doesn't follow the PR template. The template requires these sections:
## What this PR does## Why it's needed## Reviewer Test Plan(with### How to verify,### Evidence (Before & After),### Tested on)## Risk & Scope## Linked Issues<details>中文说明</details>(Chinese translation)
The current body uses ## Summary, ## Changes, ## Behavior, and ## Security — none of which match the template. The Reviewer Test Plan, Risk & Scope, Linked Issues, and Chinese translation sections are all missing.
Could you reformat the PR description to follow the template? The content is solid — it just needs to be restructured so reviewers can find what they need.
中文说明
@wenshao — 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<details>中文说明</details>（中文翻译）
当前用的是 ## Summary、## Changes、## Behavior、## Security，与模板不匹配。Reviewer Test Plan、Risk & Scope、Linked Issues 和中文翻译都缺失。
请按模板格式重新整理 PR 描述。内容本身写得不错，只需要调整结构即可。
— Qwen Code · qwen3.7-max
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
wenshao
force-pushed
the
feat/autofix-review-loop-all-bot-prs
branch
from
July 8, 2026 09:16
fc89a6f to
9abee55
Compare
Contributor
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
Collaborator
|
Thanks for pushing this. The direction makes sense: getting dev-bot PRs outside Before merging, I think this needs one small narrowing pass:
Also, the existing bot review is still asking for the PR body to follow the repository template, and the Ubuntu CI / delayed automatic review are still pending. Once those are handled, I think the core idea should be much easier to merge. 中文说明方向是对的：让非 合入前建议先收窄一下：
另外 PR body 还需要按模板调整，当前 Ubuntu CI 和 delayed review 也还在 pending。 |
wenshao
commented
Jul 8, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
Author
There was a problem hiding this comment.
Downgraded from Request changes to Comment: self-PR; CI failing: review-pr.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Collaborator
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
.github/workflows/qwen-autofix.yml:1008,1202,1244 |
BOT_COMMENT_FILTER regex duplicated across scan (via --arg bf) and address (inline regex literals) jobs — 3 copies total. If a new bot-comment marker is added to one location but missed in another, the scan and address phases silently disagree on which comments are actionable, causing either wasted runs or missed feedback. |
Define the regex once as a workflow-level env variable and reference via --arg bf in all jq invocations across both review-scan and review-address jobs. |
.github/workflows/qwen-autofix.yml:25-27 |
pull_request_review: submitted fires for all review states including approved, which is never actionable. Every approval by a trusted sender on a bot PR triggers the full scan pipeline (~7 API calls + runner minutes) only to find "nothing new." |
Add REVIEW_STATE: '${{ github.event.review.state }}' to the env block and early-exit when REVIEW_STATE == 'approved' before the sender trust check. |
scripts/tests/qwen-autofix-workflow.test.js / .github/workflows/qwen-autofix.yml:955-960 |
The branch-based issue-number extraction logic (autofix/issue-* → issue number, else → PR number) is the core new behavior enabling this PR's purpose, but has zero test assertions. A regression would silently break issue references for non-autofix/issue-* bot PRs. |
Add assertions: expect(reviewScanStep).toContain('"${BRANCH}" == "${BRANCH_PREFIX}"*') and expect(reviewScanStep).toContain('ISSUE="${PR}"'). |
— qwen3.7-max via Qwen Code /review
…, in-repo check, issue comments
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
…filter, branch prefix doc, test coverage
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Collaborator
Author
|
@qwen-code /triage |
1 similar comment
Collaborator
|
@qwen-code /triage |
Collaborator
|
Thanks for the PR @wenshao! Template: Compliant ✓ — all required sections present (What/Why/Test Plan/Risk/Linked Issues/中文说明). The previous CHANGES_REQUESTED review about template non-compliance is stale; the body has been updated. Problem: Observed bug with concrete evidence — PR #6520 demonstrates that the review loop missed bot PRs on non- Direction: Aligned ✓ — extending the autofix feedback loop to cover all bot PRs and adding real-time Size: Not applicable — no core module paths touched. Changes are in Approach: The scope feels right. Three focused changes: (1) add real-time trigger with proper security gating, (2) remove branch-prefix filter, (3) add issue-level comment scanning. No drive-by refactors or scope creep. The issue-level comment scanning is a nice addition that catches Moving on to code review. 🔍 中文说明感谢贡献 @wenshao！ 模板： 完整 ✓ — 所有必需章节齐全。之前关于模板不合规的 CHANGES_REQUESTED review 已过时，PR body 已更新。 问题： 已观测到的 bug，有具体证据 — PR #6520 证明 review 闭环遗漏了非 方向： 对齐 ✓ — 将 autofix 反馈闭环扩展到所有 bot PR 并添加实时触发器是 CI 基础设施的自然演进。 规模： 不适用 — 未触及核心模块路径。 方案： 范围合理。三个聚焦的变更：(1) 添加实时触发器并配合安全门控，(2) 移除分支前缀过滤，(3) 添加 issue 级评论扫描。无顺手重构或范围蔓延。 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Collaborator
Code ReviewIndependent proposal (before reading the diff): Add Comparison: The PR matches the independent proposal and goes further in useful ways:
Security review: The
No correctness bugs, security holes, or regressions found. Reuse check: The Test ResultsAll 45 tests pass, including new assertions for routing contract, issue-level comment scanning, and security gates: CI checks also passing: Classify PR ✓, Test (ubuntu) ✓, review-pr ✓, route ✓. TUI TestingN/A — this is a GitHub Actions workflow change with no TUI surface. Unit tests are the primary verification. 中文说明代码审查独立方案（阅读 diff 前）：添加 对比： PR 匹配独立方案并在有用的方面更进一步：issue 级评论扫描捕获了旧扫描遗漏的 安全审查： 路由任务通过 6 层门控全面防护：bot 作者、同仓库、main 目标、发送者权限、API 失败默认不信任、可信基础 checkout。未发现正确性 bug、安全漏洞或回归。 测试结果45 个测试全部通过。CI 检查也全部通过。 TUI 测试不适用 — 这是 GitHub Actions 工作流变更，无 TUI 界面。 — Qwen Code · qwen3.7-max |
Collaborator
ReflectionThis is a well-executed infrastructure fix. The problem is concrete (#6520), the solution is focused, and the security model is thorough. Going back to my independent proposal: the PR matches it and adds issue-level comment scanning — a genuine gap I missed. The The one tradeoff the PR acknowledges — the stale-watermark race when multiple address runs queue up — is real but low-risk. Changing to Every change in the diff is necessary for the stated goal. No unrelated edits, no scope creep. The SKILL.md update is a one-line text correction. The test additions cover exactly the new behavior. The previous CHANGES_REQUESTED review was about template non-compliance, which has been resolved. Template is now fully compliant. Verdict: Approve. Clean, focused, well-tested, security-conscious. 中文说明反思这是一个执行良好的基础设施修复。问题具体（#6520），方案聚焦，安全模型周全。 回到我的独立方案：PR 匹配并增加了 issue 级评论扫描——这是我遗漏的真实缺陷。 PR 承认的权衡——多个 address 运行排队时的 stale-watermark 竞态——是真实但低风险的。改为 diff 中的每个变更都是实现目标所必需的。无无关编辑，无范围蔓延。 结论： 批准。干净、聚焦、测试充分、安全意识强。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Collaborator
Author
|
@qwen-code /triage |
yiliang114
approved these changes
Jul 8, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. Review threads resolved, CI green. Minor follow-up: BOT_COMMENT_FILTER regex dedup can be done as a separate cleanup.
qwen-code-ci-bot
approved these changes
Jul 8, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
pull Bot
pushed a commit
to Stars1233/qwen-code
that referenced
this pull request
Jul 19, 2026
… green no-op (QwenLM#7165) * feat(autofix): label-driven takeover and release; fix forced-dispatch green no-op Takeover, exactly as designed: applying autofix/takeover (GitHub triage+ only — the permission gate is GitHub's own) summons the loop onto a PR, human-authored included; removing it releases the PR. The autofix/skip label opts any PR out at every engagement path — the autofix scan, the forced path, and the Fleet Shepherd walk — and wins when both labels are present. Every toggle gets a bilingual ack from the PAT-verified bot identity. The comment-command surface stays CLOSED: the pinned no-comment-commands contract test is untouched. Plumbing: pull_request labeled/unlabeled triggers; label events share the per-PR route group (the event class is triage-gated); the group expression also carries the QwenLM#7163 payload trust prefilter so the two open PRs converge on the same final text in either merge order; scan candidates become bot PRs ∪ takeover PRs minus skip PRs (deduped); fork label events are logged and dropped (no secrets to even post a rejection). The forced-validation replay immediately caught a latent bug live since QwenLM#6528: '(.isCrossRepository // true) | not' is false for EVERY input (jq's // treats false as empty), so every workflow_dispatch pr_number=N run — including the shepherd's conflict dispatches — validated to targets=[] and exited GREEN having done nothing. Fixed to '.isCrossRepository == false' (fail-closed on a missing field) with a replay case pinning the trap. Tests: autofix 55/55 (label engage/release pins, candidate-selection replay with skip-wins and fork cases, forced-validation replay across author/takeover/skip/closed/fork/missing-field); shepherd 12/12 (skip-filter replay). * feat(autofix): '@qwen-code /takeover' comment sugar over the takeover label Maintainer-mandated reopening of the comment surface, in the narrowest possible form: '@qwen-code /takeover' applies TAKEOVER_LABEL and '@qwen-code /takeover stop' removes it — nothing else. The label stays the single source of truth: engagement and release happen exclusively via the pull_request label events, so a manual label edit and the command are one mechanism with two entry points, and the command's whole blast radius is one label toggle. Gates: an expression-level startsWith prefilter keeps ordinary comments from ever starting a job; the body must match the constant EXACTLY after trimming (no parsing, no arguments); allowed senders are the PR author (who may lack label access — this is who the sugar is FOR) or a write+ collaborator via the same permission API used by review routing; closed PRs, non-PR comments, and the bot itself are ignored. The toggle job is PAT-verified and presence-aware (no-op toggles are explicit, since they fire no label event). The 'does not expose comment-triggered autofix commands' contract test is REWRITTEN into pinning this gated design, documenting the deliberate reversal. New behavioral replay drives the extracted command branch with a PATH-stubbed permission API across eight author/write/read/ exact-match/non-PR/closed/self cases. 56/56. * feat(autofix): raise the round cap to 50 while a PR is under takeover Large managed PRs routinely need dozens of feedback rounds — that is the point of takeover — so the unattended MAX_ROUNDS=5 would strangle exactly the PRs the label exists for. While TAKEOVER_LABEL is present the effective cap is TAKEOVER_MAX_ROUNDS=50: the circuit breaker stays (a bot/review-bot ping-pong is still bounded and every round still requires new trusted feedback or a conflict), it is just sized for explicitly delegated work. Removing the label restores the strict cap on the next scan. The scan computes the effective cap from the candidate's live labels and stamps it into the matrix target; the address job shadows the workflow-level MAX_ROUNDS with the matrix value, so every round message, marker, and cap gate uses the same number consistently (including QwenLM#7163's address-time cap discard once both merge). 57/57 with a verbatim cap-selection replay (labeled → 50, plain → 5). * feat(autofix): re-armable round windows, cap raised to 100, visible cap pause The round counter is DERIVED state, stored nowhere but in the bot's eval-marker comments on the PR — and counting is now windowed by the latest '<!-- takeover-ack engaged -->' comment. Re-engaging (label off→on, or repeating the takeover command on an already-managed PR, which now posts a re-arm ack instead of a silent no-op) starts a fresh window: a PR that exhausted its rounds continues under management with one human action, auditable in the PR timeline. The WATERMARK stays global across windows — feedback already addressed is never replayed — and a PR never taken over has no ack, so strict lifetime counting is unchanged. The prepare-side live round is windowed identically, so pre-reset markers can neither trip the cap nor look like same-ts round-advance duplicates (replay-proven). TAKEOVER_MAX_ROUNDS rises to 100 per maintainer sizing, and pausing at the cap is now VISIBLE on managed PRs: a bilingual notice with re-arm guidance, once per counting window (marker-deduped past the latest re-arm; a failed post retries next scan). 58/58: rearm windowing replay (no ack → lifetime; ack → round 0 with watermark preserved; new rounds count from 1; latest ack wins), the stale-gate re-arm interplay case, cap-selection at 100, and cap-notice dedup pins. * feat(autofix): collapsed-Chinese bilingual takeover comments; fix ESLint regex-spaces Every takeover-flow comment — engage ack, release ack, re-arm ack, and the cap-pause notice — now follows the project convention: English body plus Chinese collapsed under <details><summary>中文说明</summary> (pinned at exactly four sites). Bodies are built via printf so no workflow indentation leaks into the markdown: the previous literal multi-line strings embedded 10 leading spaces, which would have rendered the trailing marker comment as a visible code block. Also fixes the CI failure at 1497a7e: three extraction regexes used literal space runs, tripping ESLint no-regex-spaces — now {n} quantifiers. Lesson applied: prettier alone is not the lint gate. 58/58 + 12/12. * fix(autofix): fork-safe takeover release, verified cap-notice write, doc drift Review round (issue comment on head c2d8a89), all three findings: - F1 (bug): 'unlabeled' on a fork PR emitted the release ack unconditionally; fork pull_request runs carry no secrets, so takeover-ack failed its PAT identity check — a red run, reachable self-serve (the comment sugar accepts the fork PR's author, sticking the label; any later unlabel fired the red run). The unlabeled branch now mirrors the fork log-and-drop, and takeover-command — which runs in issue_comment context WITH secrets — refuses fork PRs up front with a bilingual explanation (adoption guidance), so the label can no longer stick to forks via the command at all. - F2 (convention): the scan's cap notice now verifies the PAT identity before writing (memoized per run): a rotated PAT would post under a foreign login that the AUTOFIX_BOT-scoped dedup can never see, reposting every scan. Scan header updated — its single write is identity-verified. - F3 (doc drift): two leftover '50' references updated to speak of TAKEOVER_MAX_ROUNDS. - Note adopted: the leading-whitespace prefilter/trim nuance is now documented at the prefilter. 58/58 + 12/12; collapsed-Chinese sites now pinned at five (the fork-refusal joins the four acks). * fix(autofix): window-keyed rounds, ordered commands, skip closure, marker scrub Second review round on the takeover feature (dev-bot GPT-5 review at c2d8a89) — all seven Criticals and three Suggestions: - Round windows are now keyed, not timestamped: every eval marker records the window key it was produced under (win=…, legacy counts as 'none'), the current key is the latest engage ack's created_at, and only current-key markers count toward the cap. An in-flight address job selected before a re-arm can no longer re-cap the fresh window with a late marker — and prepare discards any job whose key a re-arm superseded while it sat queued (conflicts stay actionable). - Takeover commands are serialized per PR (queued concurrency group): an older /takeover cannot land after a newer /takeover stop read the unlabeled state. - Skip closure at every remaining gap: the command refuses engage and re-arm on a skip-labeled PR (bilingual skip-blocked ack, no bogus window anchor); the label-event ack does the same; the scan's fresh per-PR fetch re-checks skip mid-scan; the shepherd re-checks the LIVE label immediately before its two mutating levers (fail closed — unreadable labels count as skipped). - Honest release on bot-authored PRs: removing takeover now says standard bot management continues (strict cap); only takeover mode ends. - Model files posted verbatim as PR comments (address-summary, no-action) are scrubbed of HTML comments — the rule the handoff DETAIL already applied — so induced output cannot forge control markers the scanners would trust. - Test hardening per the Suggestions: every bilingual body is asserted individually (8 printf bodies), and the gh pr list producers must request 'labels' in both workflows (fixture-driven consumers alone went green without it). 58/58 + 12/12; the rearm replay now proves the race the key model closes (an old-window round-50 marker landing after the ack counts 0). * fix(autofix): sever PR hooks from PAT pushes; scope label routing; fork-author command gate Third review round (yiliang114 P0-P3 + a second inline batch): - P0: the address/publish push steps carry the PAT while the branch carries PR-controlled .husky hooks (hooksPath was pointed there so the agent's commits get checked) — a pre-push hook would execute that code with the PAT in env. Both push sites now sever hooks (core.hooksPath=/dev/null + git push --no-verify). The force-push guard regex also false-positived on --no-verify ('-…f') and is now precise (-f word / +refspec). - P1: only the takeover label itself shares the per-PR route group; an unrelated label changed in the same batch operation can no longer cancel a queued takeover route. - P2: candidate INSPECTION is bounded (MAX_CANDIDATE_INSPECTIONS=60), not just emitted targets — idle candidates burn serial API calls and takeover widens the pool. Excess rotates to the next scan. - Fork-author command gate: author privilege is in-repo only — a fork-PR author can no longer summon even PAT-authored refusal comments onto their own PR (silent drop); write+ maintainers still reach the explanatory fork refusal. Replay-proven both ways. - Candidates jq now fails closed on the fork field (== false), matching the forced path and the //-trap NOTE. - The unlabeled path mirrors the labeled-path state guards: releasing a closed/non-main/fork PR acks nothing (it was never engaged). - Command-contract docs updated to the real side-effect set, and the four-path toggle (plus skip/fork refusals) now has a full gh-recording behavioral replay. 59/59 + 12/12. * fix(autofix): live cap-notice dedup key, line-proof marker scrub, checkout hook severing Round-3 review (issue comment at 427301c), all four findings: - R1 (regression): the window-key rename left the cap-notice dedup on a dangling REARM_TS — empty rt made every historical notice count, silently turning per-window dedup into per-lifetime, so a re-armed PR hitting the cap again paused with no reminder (the feature's headline case). Now NOTICE_RT=REARM_KEY with the 'none' corner falling back to lifetime dedup (created_at > 'none' is never true lexically, which would have flipped it into posting every scan). The extracted-jq replay proves all three regimes. - R2: the HTML-comment strip was line-oriented while jq scan() matches across newlines — a marker split over two lines survived the scrub and still parsed. All three publish sites now neutralize the opening token itself (sed 's/<!--/<!\-\-/g' — line-independent, and the backslashes render away in markdown). Proven end-to-end on a split forged marker, with the sed extracted verbatim. - R3 (pre-existing, folded in as the natural home): prepare's checkout -B ran PR-controlled post-checkout hooks with the PAT in env. Prepare now severs hooks like the push steps; the agent step — no PAT, sandboxed tools — re-points .husky itself so its commits still get checked. - R4: candidates now drain NEWEST-first (unique_by sorts ascending; sort_by(-.number) added), the comment states the real semantics instead of a rotation that never existed, and the free busy skip no longer consumes inspection budget. 59/59 + 12/12. * fix(autofix): takeover-ack state read fails closed Review suggestion, adopted verbatim: the ack's gh pr view fell back to '{}' on failure, defaulting HAS_SKIP to false — a transient API failure could post a wrong 'engaged' ack on a skip-labeled PR. It now exits like the sibling takeover-command job (a red ack job posts nothing; engagement is scan-driven and unaffected). Pinned both ways. * fix(autofix): command-style comments are instructions, not review feedback Motivating case: a maintainer posted '@qwen-code /triage' on a bot PR and the loop burned a full agent cycle (checkout, install, build, agent) to publish a no-action report explaining that the command is not feedback. The takeover command introduced here would self-trigger the same round on every engagement. Comments matching '^\s*@qwen-code /' (any author) are now excluded at all four feedback-decision sites — the scan's issue-comment count, the NEWEST computation, the LIVE_NEW recount, and the prompt renderer — alongside the existing bot-marker filter. Behaviorally proven: a trusted takeover-command comment newer than the live watermark no longer rescues a stale duplicate into an agent round, and the raw jq counts real feedback while dropping the command. 59/59 + 12/12. * fix(shepherd): fork check fails closed, matching the autofix convention Review suggestion, adopted verbatim: the fleet filter kept '.isCrossRepository != true' (fail-open on a missing field) while the autofix candidates migrated to '== false' in the same PR. Aligned; the filter replay now includes a missing-field row and proves it is rejected. 12/12. * fix(autofix): trusted runner staging, live author privilege, rotation, sentinel watermark Auto-review of the merged head (16 findings; 11 adopted, 3 rebutted in replies, 1 rename, 1 already-covered): - The address agent step invoked run-agent.mjs from the CHECKED-OUT PR branch with the model key in env — branch-controlled code on the host (takeover targets human branches). The runner is now staged from the trusted base into RUNNER_TEMP (same pattern as the schema gate) and invoked from there. - Author command privilege is LIVE, not durable: authors must hold triage+ today, so an ex-member's authorship no longer summons secret-bearing runs (the sugar's audience — members below write — still qualifies). - The terminal-handoff sentinel ts is excluded from watermark computation (scan and prepare): it is a flag, not an evaluation time, and it previously made a re-arm after a terminal handoff dead on arrival by filtering all future feedback forever. Terminal skipping stays round-based and thus window-scoped. - Candidate inspection gains a rotating start offset (fixed newest-first plus the budget starved the oldest tail FOREVER once the pool exceeded the budget). - issue_comment events get their own per-PR command group: bursts coalesce away from review routes, and pending-slot replacement is exactly latest-intent. - The queued toggle re-verifies OPEN + base=main; the cap notice honors dry-run and re-verifies live consent before posting; the release ack on a skip-labeled bot PR now says skip governs. - Shepherd: live_skip is reason-aware (an API outage is reported as fail-closed, never as consent withdrawn) and both levers check their budgets BEFORE the PAT-backed live read. - Tests: the supersede fixture is now discriminating (old-window marker lands AFTER the ack — timestamp-windowing would have counted it); the force-push guard catches combined short options (-uf); the sentinel-watermark rule and rotation are replayed; the labels test is renamed to stop claiming the comment surface is closed. 60/60 + 12/12. * test(autofix): restore recheck-before-checkout ordering pin; pin all win-marker sites Two review suggestions, both fallout from the merge-resolution test unification: the eligibility recheck's BEFORE-checkout ordering assertion (lost with the replaced pin test) is back inside the replay test, and all three eval-marker producers now carry per-site win= format pins (the global count-3 catches removal but not a lose-one-gain-a-duplicate swap). 60/60. * fix(autofix): trusted-commenter cmd groups; hooks severed at every host checkout; honest model-key framing - The issue_comment command group now requires a trusted-looking payload association (same prefilter pattern as reviews): an arbitrary commenter's rejected command can no longer cancel a maintainer's queued command out of the shared per-PR group. - Both verification checkouts (review gate and issue publish) sever hooks like every other host checkout — no secret sits in those steps, but branch post-checkout code on the host broke the convention (five severing sites now pinned). - The agent-step comment and the PR risk section now state the model key's real exposure explicitly: the CLI forwards OPENAI_API_KEY into the sandbox and the agent's job is to build/test the branch, so a taken-over branch's scripts can read AUTOFIX_OPENAI_API_KEY — an accepted, documented consequence of takeover (PAT remains fully severed via trusted staging + hook severing); the key should be low-privilege and rotatable. 60/60 + 12/12. * fix(autofix): label events get their own route group; non-takeover labels never start jobs Two review suggestions, adopted verbatim: pull_request label events now group as route-label-{N} (distinct from the review group — a simultaneous review and label toggle on the same PR can no longer cancel each other), and the route job gate filters pull_request events to the takeover label, so a triage labeling session across dozens of PRs burns zero runner slots. 60/60. --------- Co-authored-by: wenshao <wenshao@example.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Extends the autofix review loop so that every open PR authored by
qwen-code-dev-bot— not just those onautofix/issue-*branches — is automatically scanned for reviewer feedback and patched by the bot. It also adds a real-timepull_request_reviewtrigger so the bot responds within minutes instead of waiting up to 4 hours for the next scheduled sweep.Why it's needed
PR #6520 is a concrete example: the bot opened a fix PR on a
fix/*branch, a maintainer leftCHANGES_REQUESTEDwith inline comments, but nothing happened. The oldreview-scanjob filtered candidates bystartswith("autofix/issue-"), so any bot PR created on a different branch name was invisible to the feedback loop. The 4-hour-only schedule added further latency even for PRs that were in scope.Reviewer Test Plan
How to verify
review-scanjob in.github/workflows/qwen-autofix.yml— the candidate query usesgh pr list --author "${AUTOFIX_BOT}" --base mainwith no branch-prefix filter.routejob —pull_request_reviewevents are gated onPR_AUTHOR == AUTOFIX_BOT,PR_HEAD_REPO == REPO,PR_BASE_REF == main, and sender write+ permission.review-addressjob — the first checkout pinsref: github.event.repository.default_branch(trusted base), and the PR branch is only checked out after the trusted CLI bundle is built.npx vitest run scripts/tests/qwen-autofix-workflow.test.js— 45 tests pass, including new assertions for the routing contract and issue-level comment scanning.Evidence (Before & After)
N/A — this is a GitHub Actions workflow change with no TUI surface. The workflow structural tests (
scripts/tests/qwen-autofix-workflow.test.js) are the primary verification.Tested on
Environment (optional)
N/A — workflow YAML and test assertions only.
Risk & Scope
pull_request_reviewevent from a trusted sender on a bot PR. The existing per-PR concurrency group (cancel-in-progress: false) serializes bursts, and the MAX_ROUNDS=3 cap prevents infinite loops. Thepull_request_review_commenttrigger was intentionally dropped to avoid redundant runs on multi-comment reviews.cancel-in-progress: truerisks cancelling a push mid-flight.autofix/issue-*PRs continue to work identically; issue-number extraction from branch names is preserved for those PRs.Linked Issues
#6520
中文说明
这个 PR 做了什么
扩展 autofix review 闭环，使所有由
qwen-code-dev-bot提交的 open PR（不仅限于autofix/issue-*分支）都能被自动扫描 review 反馈并由 bot 修补。同时添加pull_request_review实时触发器，使 bot 在几分钟内响应，而不是等待最多 4 小时的下一次定时扫描。为什么需要
PR #6520 是一个具体例子：bot 在
fix/*分支上提了一个修复 PR，维护者留下了CHANGES_REQUESTED和 inline 评论，但什么也没发生。旧的review-scan用startswith("autofix/issue-")过滤候选 PR，所以任何非该前缀分支上的 bot PR 对反馈闭环不可见。4 小时定时扫描即使对在范围内的 PR 也增加了额外延迟。审查者测试计划
如何验证
review-scan— 候选查询使用gh pr list --author "${AUTOFIX_BOT}" --base main，无分支前缀过滤。route—pull_request_review事件门控检查PR_AUTHOR == AUTOFIX_BOT、PR_HEAD_REPO == REPO、PR_BASE_REF == main和 sender write+ 权限。review-address— 第一次 checkout 固定ref: github.event.repository.default_branch（可信基础），PR 分支仅在可信 CLI bundle 构建后才切出。npx vitest run scripts/tests/qwen-autofix-workflow.test.js— 45 个测试通过，包括路由契约和 issue 级评论扫描的新断言。证据（Before & After）
N/A — 这是一个 GitHub Actions 工作流变更，无 TUI 界面。工作流结构测试是主要验证手段。
测试平台
环境（可选）
N/A — 仅工作流 YAML 和测试断言。
风险与范围
pull_request_review事件上运行完整的 review-address agent。现有的 per-PR 并发组（cancel-in-progress: false）串行化突发请求，MAX_ROUNDS=3 上限防止无限循环。pull_request_review_comment触发器被有意移除以避免多评论 review 的冗余运行。cancel-in-progress: true有在推送中途取消的风险。autofix/issue-*PR 继续完全相同地工作；分支名 issue 号提取对这些 PR 仍然保留。关联 Issue
#6520