ci: add @qwen-code /resolve - #5779
Merged
Merged
Conversation
yiliang114
force-pushed
the
codex/fix-conflicts-command
branch
from
June 23, 2026 14:11
f6ada02 to
bcd33a4
Compare
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. |
yiliang114
changed the title
wenshao
requested changes
Jun 23, 2026
wenshao
requested changes
Jun 23, 2026
Addresses the remaining /resolve review findings:
- Command injection (RCE): branch refs were inlined as ${{ }} into the
verify / show-artifacts run blocks; a branch named with `$(...)` or
backticks would execute on the runner that later holds CI_DEV_BOT_PAT.
Pass refs via env: and reference only "$BASE_REF" / "$HEAD_REF".
- Secret exposure: the agent step (with OPENAI_API_KEY) could run
PR-authored npm build/lint/test and exfiltrate the key. Drop
build/lint/typecheck/vitest from the agent coreTools (the credential-free
verification gate re-runs them) and install with `npm ci --ignore-scripts`
plus explicit patch-package so PR lifecycle scripts do not run.
- Edit scope: the verification gate now fails when the agent changed any
file the base branch did not, so a prompt-injected agent cannot smuggle
edits outside the conflict set. Prompt tightened to match.
Also test only real workspaces (guard packages/channels/<name> against the
non-workspace packages/channels path).
wenshao
requested changes
Jun 23, 2026
wenshao
marked this pull request as ready for review
June 23, 2026 23:12
Collaborator
|
Thanks for the PR @yiliang114! Template looks good ✓ On direction: this fills a real gap — maintainers currently have to ask contributors to rebase manually, which is slow and often ignored. A conservative same-repo-only On approach: the scope feels right. Reusing the existing 中文说明感谢贡献！ 模板完整 ✓ 方向：这解决了一个真实的维护者痛点——目前只能等贡献者手动 rebase，效率低且经常被忽略。保守的同仓库 方案：范围合理。复用现有的 — Qwen Code · qwen3.7-max |
Collaborator
Code ReviewSecurity ✅The security model is layered and internally consistent throughout. The agent runs with no GitHub token, Correctness ✅The Minor
Reuse ✅No new abstractions or utilities — the workflow reuses the existing Minimal change ✅All 3 files serve the stated goal: workflow modification, design doc, and workflow-guard tests. No drive-by refactors or scope creep. TestingThis is a GitHub Actions workflow change, not a CLI behavior change — tmux testing doesn't apply. The verification is the workflow-guard test suite, which asserts that the authorization, scope, and security guards remain pinned to the All 11 tests pass. They cover: authorize gate, scope guard, conflict-marker detection, agent credential isolation, dry-run path, and workflow_dispatch routing. 中文说明代码审查安全 ✅安全模型分层一致。Agent 不持有 GitHub token， 正确性 ✅
小问题
复用 ✅无新抽象——复用现有 最小改动 ✅3 个文件都服务于目标：workflow 修改、设计文档、workflow 守卫测试。无顺手重构或范围蔓延。 测试这是 GitHub Actions workflow 改动，不是 CLI 行为改动——tmux 测试不适用。验证方式是 workflow 守卫测试套件，断言授权、范围和安全性守卫保持在 — Qwen Code · qwen3.7-max |
Collaborator
ReflectionThis PR does what it says, and what it says is worth doing. The The code review came back clean. The The one thing I'd flag: there's a stray "ponytail:" in a verification gate comment that looks like a review artifact. Cosmetic, but the kind of thing that becomes permanent if nobody catches it before merge. My independent proposal would have been a separate workflow file for The only reason this has 中文说明反思这个 PR 做了它承诺的事情，而且值得做。 代码审查干净。 唯一要指出的是：验证门注释中有一个残留的 "ponytail:"，看起来像是代码审查遗留。纯装饰性的，但如果合并前没人发现就会变成永久性的。 我的独立方案原本建议用单独的 workflow 文件来实现 目前 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
requested changes
Jun 23, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
CI-blocking issue: scripts/tests/qwen-resolve-workflow.test.js is missing import { describe, expect, it } from 'vitest'; — ESLint reports 36 no-undef errors that will fail CI.
Fix: add import { describe, expect, it } from 'vitest'; after the existing imports (after line 9). The sibling test scripts/tests/no-ak-integration-ci.test.js already does this correctly.
Otherwise the implementation looks solid — security model is conservative, command injection is fixed, failure reporting is comprehensive. Once the ESLint issue is fixed, this should be ready for merge.
See the triaged review comments above for full details.
wenshao
requested changes
Jun 24, 2026
- test: import vitest globals so the lint gate stops reporting no-undef
in scripts/tests (this was the real CI-blocking lint failure)
- prepare: trap an unwritten decision and fail closed to "failed" so an
early gh/git error still reports back instead of a silent red run
- report: make the post-push result comment best-effort so a failed
comment POST can't leave the branch force-pushed with no explanation
- refresh: npm install instead of npm ci to tolerate lockfile drift
after a package.json conflict resolution
- verify: scan only resolution-touched files for leftover conflict
markers instead of git diff --check over the whole merged range, which
spuriously failed on pre-existing base whitespace
- prompt/artifact: use ${{ env.WORKDIR }} instead of hardcoded
/tmp/qwen-resolve
- authorize: drop the issue.state==open gate that also silenced /review
on closed PRs (resolve-pr keeps its own open guard)
- coreTools: document that the specifiers are advisory, not a security
boundary (sandbox + same-repo + no agent token are)
- test: assert the resolve-pr authorization/scope guards, that the agent
step carries no GitHub token, and the dry-run/workflow_dispatch paths
The build/typecheck/lint checks use `if ! cmd; then echo outcome=failed; exit 1; fi`, but the per-package test loop ran `npm run test` bare under `set -euo pipefail`. A test failure aborted the step before `outcome=fixed` was written, leaving OUTCOME empty so the report posted a generic "did not complete successfully" with no mention of which package failed. Mirror the other gates: record the failing package and set outcome=failed.
…) gate Address review feedback: the verification-gate test now also pins the agent-infrastructure-failure, missing-address-summary, and unresolved-index guards; and the failure-paths test asserts the Report-skipped-request step keeps its always() gate so an EXIT-trap decision=failed actually reports on a prepare crash. The best-effort skip-comment suggestion was already addressed in 0379530.
yiliang114
marked this pull request as ready for review
June 24, 2026 11:02
qwen-code-ci-bot
previously approved these changes
Jun 24, 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. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Code Review Summary
Deterministic checks: actionlint ✅ · yamllint ✅ · ESLint ⏭️ (deps not installed in review worktree)
Overall Assessment
The /resolve command is well-designed with strong security fundamentals: agent runs sandboxed without GitHub credentials, credential scoping is clean (separate steps for agent vs push), same-repo guard rejects forks before checkout, --force-with-lease prevents concurrent-push races, and the verification gate blanks GITHUB_TOKEN. The defense-in-depth approach is commendable.
Key Findings
- [Suggestion]
pull_request_review_commenttrigger silently ignores/resolve— inconsistent with/review - [Suggestion]
resolve-prruns-onlacksMAINTAINER_ECS_RUNNER_DISABLEDfallback that sibling jobs use - [Suggestion] Scope guard checks file names, not change content — defense-in-depth gap
- [Suggestion]
append_safe_filesed strips TypeScript generics and JSX from agent reports - [Suggestion] Agent prompt interpolates branch names via
${{ }}— confused-deputy risk - [Suggestion] Test coverage gaps on 5 critical security controls
Positive Observations
- Clean separation between agent step (no credentials) and push step (PAT)
- EXIT trap in prepare step ensures crash reporting even on early failures
persist-credentials: falseon checkout prevents ambient token leakage- Dry-run mode correctly gates the push step
- Concurrency group prevents overlapping resolve runs on the same PR
yiliang114
enabled auto-merge
June 24, 2026 11:56
…ing, test pins
Address review feedback on resolve-pr:
- runs-on honors MAINTAINER_ECS_RUNNER_DISABLED and falls back to a hosted
runner when ECS is toggled off, matching the review path (the verification
gate runs fine on an ephemeral hosted runner — better isolation, no cleanup).
- append_safe_file strips only active-content HTML elements instead of every
`<...>`, so TS generics (Map<string, number>) and JSX in the agent summary are
no longer garbled in the posted comment (GitHub sanitizes comment HTML anyway).
- the agent prompt reads the base branch name from context.md instead of
inlining ${{ base_ref }}, keeping a branch name out of the LLM prompt text.
- document that the scope guard's file-level granularity is intentional
defense-in-depth (real containment = sandbox + same-repo + no agent token).
- test: pin persist-credentials:false, GITHUB_TOKEN:'', sandbox, and the
--ignore-scripts install guards so a future edit can't silently drop them.
wenshao
previously approved these changes
Jun 24, 2026
qwen-code-ci-bot
requested changes
Jun 24, 2026
Collaborator
|
@qwen-code /triage |
qwen-code-ci-bot
approved these changes
Jun 24, 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. ✅
qwen-code-ci-bot
requested changes
Jun 25, 2026
- prepare: arm the fail-closed EXIT trap before mkdir; reject CR/LF in write_output so an attacker-set PR title can't inject GITHUB_OUTPUT keys - report: scrub the bot PAT from .git/config after push (self-hosted runner) - verify: surface a failed dependency refresh instead of a stale build error - scope guard: use -z/sort -zu like the conflict-marker check - prompt: require a Conventional Commit so the default merge message passes - tests: bound the resolve-pr slice; assert SHA-pinned lease + no-cancel guard
Collaborator
|
@qwen-code /triage |
qwen-code-ci-bot
approved these changes
Jun 25, 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. ✅ One cosmetic nit: stray "ponytail:" in a verification gate comment — fix at your discretion before merge.
wenshao
approved these changes
Jun 25, 2026
This was referenced Jun 25, 2026
yiliang114
added a commit
that referenced
this pull request
Jun 25, 2026
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; classify push failures (permission / moved / other) so forks get an actionable comment. - 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. Guard tests updated; 12/12 pass.
yiliang114
added a commit
that referenced
this pull request
Jun 25, 2026
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.
yiliang114
added a commit
that referenced
this pull request
Jun 25, 2026
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.
pull Bot
pushed a commit
to AmirulAndalib/qwen-code
that referenced
this pull request
Jun 25, 2026
…QwenLM#5870) * ci(qwen-resolve): support fork PRs and slim /resolve to conflict-only Closes the fork gap QwenLM#5779/QwenLM#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. * ci(qwen-resolve): avoid PR head ref collisions * ci(qwen-resolve): address review comments - 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).
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
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
Adds a maintainer-triggered
@qwen-code /resolvecommand that attempts to resolve merge conflicts on eligible pull requests. It lives inside the existing.github/workflows/qwen-code-pr-review.ymlworkflow as a newresolve-prjob (there is no separate workflow file) and reuses the sharedauthorizejob for permission checks. The first version only updates pull-request branches that live in this repository, runs the Qwen agent with no GitHub token andsandbox: true, verifies the result in an independent step that holds no credentials, and pushes through a separate publish step usingCI_DEV_BOT_PATwith--force-with-lease. Adry_runinput resolves without pushing.Why it's needed
Some pull requests sit blocked by merge conflicts while maintainers wait for contributors to update them. This gives maintainers a conservative manual command for same-repository branches without handing the agent push credentials or attempting to mutate external forks.
Reviewer Test Plan
How to verify
Review the routing and guards in the
resolve-prjob:@qwen-code /resolve(or aworkflow_dispatchwithcommand: resolve) only proceeds when theauthorizejob grantsshould_review == true(write+ permission), the PR is open, and the head branch is in this repository — fork PRs are reported as unsupported before checkout. The agent step receives no GitHub token and runs withsandbox: true; thecoreToolslist is advisory only — qwen-code's permission manager keys on the tool name and ignores the parenthesised command — so the actual containment is the same-repo author guard plus the sandbox plus the absent token, not the allowlist. An independent verify gate (no credentials) scans the resolution's changed files for leftover conflict markers, re-checksgit merge-tree, enforces a change-scope guard, and runs build, typecheck, lint, and per-package tests; each gate records which check failed instead of aborting silently. Only the final publish step pushes, with--force-with-lease, and the result comment is best-effort so a failed comment cannot leave a force-pushed branch unexplained.Evidence (Before & After)
N/A for UI. Local static verification passed:
actionlintandyamllinton.github/workflows/qwen-code-pr-review.ymlare clean,prettier --checkandeslintare clean, and the workflow-guard test suitescripts/tests/qwen-resolve-workflow.test.jspasses (9 tests — asserting the authorization and scope guards onresolve-pr, that the agent step carries no GitHub token, and the dry-run /workflow_dispatchpaths).Tested on
Environment (optional)
Local repository checkout on macOS; the workflow's runtime behavior runs on GitHub Actions.
Risk & Scope
Linked Issues
N/A
中文说明
What this PR does
新增一个由维护者触发的
@qwen-code /resolve命令，用于尝试解决符合条件的 PR 的 merge conflict。它并入现有的.github/workflows/qwen-code-pr-review.yml，作为一个新的resolve-prjob（没有单独的 workflow 文件），并复用共享的authorizejob 做权限校验。第一版只更新当前仓库内的 PR 分支，Qwen agent 阶段不注入 GitHub token 且开启sandbox: true，在一个不持有任何凭证的独立步骤里验证结果，并只在单独的 publish 步骤用CI_DEV_BOT_PAT+--force-with-lease推送。提供dry_run输入，可只解决不推送。Why it's needed
有些 PR 会因为 merge conflict 长时间阻塞，维护者只能等贡献者手动更新。这个改动给维护者一个保守的手动命令，先覆盖同仓库分支，不给 agent push 凭证，也不尝试修改外部 fork。
Reviewer Test Plan
How to verify
请检查
resolve-prjob 的路由与守卫：@qwen-code /resolve（或command: resolve的workflow_dispatch）只有在authorizejob 给出should_review == true（write+ 权限）、PR 为 open、且 head 分支在本仓库时才继续——fork PR 会在 checkout 前被判为 unsupported。agent 步骤不拿任何 GitHub token 且sandbox: true；coreTools列表只是建议——qwen-code 权限管理器按工具名判定、忽略括号里的命令——所以真正的约束是「同仓库作者守卫 + sandbox + 无 token」，不是这个白名单。独立的 verify gate（无凭证）会扫描本次解决改动过的文件有无残留冲突标记、复跑git merge-tree、执行改动范围守卫，并跑 build/typecheck/lint 和按包测试；每个 gate 失败都会明确记录是哪一项，而不是静默中止。只有最后的 publish 步骤会用--force-with-lease推送，且结果评论是 best-effort，避免「已 force-push 却没有说明」的情况。Evidence (Before & After)
非 UI 改动，N/A。已通过本地静态验证：对
.github/workflows/qwen-code-pr-review.yml跑actionlint、yamllint均干净，prettier --check、eslint干净，workflow 守卫测试scripts/tests/qwen-resolve-workflow.test.js通过（9 个用例——断言resolve-pr的授权与范围守卫、agent 步骤不带 GitHub token、以及 dry-run /workflow_dispatch路径）。Tested on
Environment (optional)
macOS 本地仓库 checkout；workflow 的实际运行发生在 GitHub Actions。
Risk & Scope
Linked Issues
N/A