ci(qwen-resolve): support fork PRs and slim /resolve to conflict-only - #5870
Conversation
c0bc2bf to
6cd2c24
Compare
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. |
fdf57db to
eb2458c
Compare
|
(Re-run after author addressed review feedback) Thanks for the PR! Template looks good ✓ On direction: natural follow-up to #5779 and #5862 — extending On approach: the scope feels right. Two focused changes — fork support via Moving on to code review. 🔍 中文说明(作者处理完 review 反馈后的重新审查) 感谢贡献! 模板完整 ✓ 方向:这是 #5779 和 #5862 的自然延续——让 方案:范围合理。两个聚焦的改动——通过 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
|
(Re-run after author addressed review feedback) Code ReviewIndependent proposal (formed before reading the diff): to support fork PRs, I'd fetch the head via Comparison with the diff: the PR matches my proposal and improves on it — the collision-free Previous review feedback — all addressed:
Security review: inline token URL (never written to Reuse check: no new logic that duplicates existing utilities — this is a workflow-level change. TestsAll 13 guard tests pass, including the new push-failure classification test and the collision-free PR-head fetch test. Real-Scenario TestingN/A — this is a CI workflow change ( 中文说明独立方案(读 diff 之前):fork 支持用 与 diff 对比: PR 匹配我的方案并有所改进——防碰撞的 上轮 review 反馈——全部处理:
安全审查: inline token URL、SHA 钉住的 测试: 13/13 guard 测试通过。 真实场景测试: N/A——CI workflow 改动,非 CLI 行为变更。作者在 PR 描述里记录了用 — Qwen Code · qwen3.7-max |
|
(Re-run after author addressed review feedback) This PR is clean and ready to ship. Going back to my independent proposal from Stage 2: the PR matches it and then improves on it — the collision-free The previous review's two critical issues are both fixed. The dismissed CHANGES_REQUESTED suggestion about the success message — I agree with the author's implicit choice to leave it terse. CI runs automatically on the pushed branch; the merge gate is CI passing, not the The diff is minimal and focused: fork support + conflict-only scope, nothing else. Tests pass (13/13). Security controls are intact (sandbox, no agent token, scope guard, inline token URL, SHA-pinned force-with-lease). The author validated end-to-end against a real fork PR. Approving. ✅ 中文说明PR 已干净,可以合入。 回到 Stage 2 的独立方案:PR 匹配并改进——防碰撞的 上轮 review 的两个严重问题都已修复。被 dismiss 的 CHANGES_REQUESTED 建议(成功消息注明未跑测试)——我同意作者留简洁的选择。CI 会在推送的分支上自动运行;merge gate 是 CI 通过而非 diff 最小且聚焦:fork 支持 + 只做冲突解决。测试全过(13/13)。安全控制完整(sandbox、无 agent token、范围守卫、inline token URL、SHA 钉住 force-with-lease)。作者对真实 fork PR 做了端到端验证。 批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
eb2458c to
aa9e8e8
Compare
Closes the fork gap #5779/#5862 left for the maintainer /resolve command, so it can clear merge conflicts on community (fork) PRs, and narrows the command to exactly one job: resolve the conflict and push it back. - Fork PRs: fetch the head via refs/pull/N/head and push the resolved branch back to the PR's head repository (via Allow edits by maintainers) instead of bailing as unsupported. Validated end-to-end against a fork PR. - Conflict-only: drop the build/typecheck/lint/test gate and npm install/refresh; keep the structural checks (markers, index, merge-tree, default-merge, scope). Test fallout is left to the PR's own CI and follow-up tasks. - Push-failure classification: workflow_scope (the merge carries the base's .github/workflows/** changes, which a token without the workflow scope cannot push), permission (403 / 404), and moved (stale force-with-lease) each get an actionable comment; the redacted git stderr is logged for diagnosis. NOTE: the push bot's PAT (CI_DEV_BOT_PAT) needs the `workflow` scope, since resolving merges the base in and that update touches workflow files. Guard tests updated; 12/12 pass.
aa9e8e8 to
7cef8f9
Compare
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Suggestion] .github/workflows/qwen-code-pr-review.yml:1137 — The success message says "resolved and pushed" without noting that build, typecheck, lint, and tests were not run. Reviewers seeing this green-sounding comment may assume the resolution is verified, when a semantically broken merge could still pass the structural checks. Consider: echo "Qwen Code resolved the merge conflicts and pushed the branch update. Build/typecheck/lint/test were not run — please confirm CI passes before merging."
[Suggestion] .github/workflows/qwen-code-pr-review.yml:639 — The permissions comment still references "PR-controlled build/lint/test that run later in this job" — steps this PR removed. The rationale is now stale. Consider updating to: No PR-controlled code runs in this job; keeping the token read-only is defense in depth.
— qwen3.7-max via Qwen Code /review
- workflow_scope: anchor classification on GitHub's server phrase 'refusing to allow ... workflow' instead of a loose workflow.*scope, which the attacker-controlled branch name in git's rejected-ref echo could trip. - prepare: bail when the head repository was deleted (null headRepository → malformed push URL). - moved: include the run-artifact link, consistent with the other cases. - permission: drop 'could not read' (matched transient network errors).
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings at Critical or Suggestion level. Downgraded from Approve to Comment: CI still running.
The changes are well-structured — fork PR support via refs/pull/N/head fetch, inline push URL, classified failure messages, and 13/13 tests passing. The removal of the build/typecheck/lint/test gate is a deliberate design decision documented in the PR description.
— qwen3.7-max via Qwen Code /review
What this PR does
Extends the maintainer-triggered
@qwen-code /resolvecommand so it can clear merge conflicts on community (fork) pull requests, and narrows what the command does to exactly one thing — resolving the conflict and pushing it back.refs/pull/N/head(mirrored on the base repo for both in-repo and fork PRs) and pushes the resolved branch back to the pull request's own head repository through GitHub's "Allow edits by maintainers" mechanism. A rejected push is classified —workflow_scope(the merge carries the base branch's.github/workflows/**changes, which a PAT without theworkflowscope cannot push),permission(maintainer edits disabled, an org-owned fork, or a 403/404), ormoved(the head advanced, so--force-with-leasedeclined) — so the result comment tells the maintainer exactly what to do instead of failing generically.git merge-treeshows no remaining conflict, the top commit is not a default merge commit, and the change-scope guard (the agent may only touch files the merge itself touched). Whether the merged result passes is left to the pull request's own CI and any follow-up fix task.Why it's needed
Community pull requests routinely sit blocked on merge conflicts, and most of them come from forks — exactly the case the first version skipped. Pushing to a fork works through "Allow edits by maintainers," so the command can update those branches; it just needs to target the fork's repository. Once forks are in scope, the slow part for a stale PR is the conflict resolution itself, not running the full test suite — and a merged-but-still-red branch is a separate problem a maintainer (or a later task) handles, not a reason to withhold the resolution.
The push bot's token (
CI_DEV_BOT_PAT) must have theworkflowscope (classic PAT: checkworkflow; fine-grained: Workflows → Read and write). Resolving merges the base branch in, and that update touches.github/workflows/**files whenever the base changed them — which GitHub refuses to push without the scope. If the scope is missing,/resolveresolves the conflict but the push is rejected and the result comment says so.Reviewer Test Plan
How to verify
Static review of the workflow and the guard tests:
resolve-prno longer contains theunsupportedfork bail;preparefetches+refs/pull/${PR_NUMBER}/head:${head_fetch_ref}intorefs/remotes/origin/qwen-resolve/pr-${PR_NUMBER}/headso fork branches named like the base branch do not collide;Report resultpushes tohttps://x-access-token:${PUSH_TOKEN}@github.com/${HEAD_REPO}.gitwith--force-with-lease, the token passed inline (never written to.git/config), andpush.logis only grepped to classify the failure / echoed redacted for diagnosis, never written into the comment.Install dependencies/Refresh dependenciessteps are gone; the structural checks and the scope guard remain; the agent step still carries no GitHub token and runssandbox: true.The guard suite
scripts/tests/qwen-resolve-workflow.test.jswas updated for the new behavior and passes (13 tests, including the push-failure classification and collision-free PR-head fetch).Evidence (Before & After)
Validated end-to-end via
workflow_dispatchfrom this branch against a throwaway fork PR (a one-line conflict seeded indocs/...): the agent resolved the conflict,Report resultforce-pushed the resolution to the fork branch (commit authored byqwen-code-dev-bot), and the PR flippedCONFLICTING → MERGEABLEwith a "resolved and pushed" comment. This also surfaced and fixed theworkflowscope requirement above (the first attempts failedworkflow_scopeuntil the push token was granted the scope). Locally:npx vitest run scripts/tests/qwen-resolve-workflow.test.js→ 13/13 pass;git diff --checkclean.prettier --checkstill reports pre-existing workflowruns-onformatting churn, so this PR leaves that unrelated formatting alone.actionlintreports only a pre-existingdeployment: falsewarning unrelated to this change.Tested on
Environment (optional)
Local repository checkout on macOS (guard tests + linters); the workflows run on GitHub Actions
ubuntu-latest. Fork-push path exercised live on Actions.Risk & Scope
--force-with-lease, and the agent runs sandboxed with no token; the scope guard still fails closed on out-of-scope edits.workflowscope, a conflict the agent resolves inside a.github/workflows/**file is pushed to the (possibly fork) head branch and then runs in that repo's Actions. This is bounded by the no-token sandboxed agent, the scope guard (the agent may only touch files the merge itself changed), and write+ maintainer authorization, and it lands in the contributor's own CI context — not this repo's. Called out at the push site in the workflow./resolvepush is unchanged from ci: add@qwen-code /resolve#5779.workflowscope for the fork path to land.Linked Issues
Follow-up to #5779 and #5862.
中文说明
这个 PR 做了什么
扩展维护者触发的
@qwen-code /resolve,让它能清理社区(fork)PR 的 merge conflict,并把命令收敛成只做一件事——解冲突并推回去。refs/pull/N/head(base 仓库对 in-repo 和 fork PR 都有镜像)拉 head,并经 GitHub 的「Allow edits by maintainers」把解完冲突的分支推回 PR 自己的 head 仓库。推被拒会分类——workflow_scope(merge 带进了 base 的.github/workflows/**改动,token 没workflowscope 推不了)、permission(没勾 maintainer-edits / org fork / 403、404)、moved(head 移动导致 force-with-lease 拒绝)——评论给可操作指引,而不是笼统报错。git merge-tree无冲突、顶 commit 不是默认 merge commit、以及范围守卫(agent 只能动 merge 本身涉及的文件)。合并结果是否通过,交给 PR 自己的 CI 和后续修复任务。为什么需要
社区 PR 经常卡在 merge conflict,而它们大多来自 fork——正是第一版跳过的情况。推 fork 靠「Allow edits by maintainers」就能做,只是要把推送目标对准 fork 仓库。fork 支持之后,stale PR 慢的是解冲突本身而非跑全量测试;而「合完仍然红」是另一个问题,由维护者或后续任务处理,不该成为不给出解冲突结果的理由。
推送用的 bot token(
CI_DEV_BOT_PAT)必须带workflowscope(classic 勾workflow;fine-grained 开 Workflows → Read and write)。解冲突会把 base 合进来,只要 base 动过.github/workflows/**,这次推送就带 workflow 文件改动,GitHub 没这个 scope 会拒。缺 scope 时/resolve能解冲突但推送被拒,评论会说明。验证
已在本分支用
workflow_dispatch对一条一次性 fork PR(在 docs 里种了一行冲突)端到端验证:agent 解冲突 →Report resultforce-push 到 fork 分支(commit 作者qwen-code-dev-bot)→ PR 由CONFLICTING变MERGEABLE并评论 "resolved and pushed"。这一过程也暴露并修正了上面的workflowscope 前提(前几次因workflow_scope失败,给 token 补 scope 后通过)。本地vitest13/13 通过;git diff --check干净。prettier --check仍会报告 workflow 里既有runs-on格式化 churn,本 PR 不顺手改这类无关格式;actionlint只剩一个与本改动无关的既有deployment: false。风险与范围
--force-with-lease,agent 仍 sandbox 且无 token,范围守卫仍对越界改动 fail closed。workflowscope,因此 agent 在.github/workflows/**文件里解的冲突会被推到(可能是 fork 的)head 分支并在该仓库 Actions 运行。受无 token 的 sandbox agent、范围守卫(只能动 merge 本身改过的文件)、write+ 维护者授权约束,且落在贡献者自己的 CI 上下文(非本仓库)。workflow 推送处也有代码注释标注。/resolve推送与 ci: add@qwen-code /resolve#5779 一致,未改动。workflowscope。关联
Follow-up to #5779、#5862。