ci(qwen-resolve): run the /resolve job on a hosted runner - #5862
Merged
Conversation
The conflict-resolution agent step runs with `sandbox: true`, which on Linux needs docker or podman to launch the sandbox. The self-hosted ECS pool ships no container runtime, so the job died at the agent step (exit 44, "failed to determine command for sandbox") before it could resolve anything — see run 28158417390. Pin the job to ubuntu-latest, which ships docker and is ephemeral (a good fit for running the untrusted PR's build/lint/test in the verification gate). Update the cleanup and PAT-scrub comments that assumed a reused self-hosted workspace; both steps stay as defensive no-ops on hosted runners.
Collaborator
|
Thanks for the PR! Template looks good ✓ On direction: this is a straightforward fix for a real breakage — the On approach: the change is minimal and focused — one Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 方向：这是对真实故障的直接修复—— 方案：改动最小且聚焦——简化一个 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
added
priority/P2
Collaborator
Code ReviewIndependent proposal (before reading the diff): the ECS pool can't run Docker, so the resolve job's Comparison with the diff: the PR matches this exactly. One expression change ( No correctness bugs, security holes, or regressions. The security controls tested in
No AGENTS.md violations. Real-Scenario TestingN/A — this is a pure CI workflow configuration change. There is no CLI behavior to drive in tmux. The PR author explicitly defers e2e validation: "a follow-up dry-run on an in-repo conflicting PR should confirm the sandbox actually starts on hosted before relying on the command." This is reasonable — the fix can only be fully validated by triggering The existing 中文说明代码审查独立提案（在读 diff 之前）：ECS pool 无法运行 Docker，因此 resolve job 的 与 diff 的对比： PR 与此完全一致。一个表达式改动（ 无正确性 bug、安全漏洞或回归。
无 AGENTS.md 违规。 真实场景测试N/A——这是纯 CI workflow 配置改动，没有可以在 tmux 中驱动的 CLI 行为。PR 作者明确推迟端到端验证："后续应在一个本仓库的冲突 PR 上 dry-run 一次，确认 hosted 上 sandbox 真能起来再依赖这个命令。" 这是合理的——修复只能通过在一个真实的冲突 PR 上触发 现有 — Qwen Code · qwen3.7-max |
Collaborator
|
This is a clean, minimal fix for a real CI breakage. The My independent proposal was identical — there's really only one fix here, and the PR lands it with zero scope creep. The diff is 1 expression change + 3 comment rewrites, every character serving the stated goal. The security controls (sandbox, no-token agent, yamllint clean, existing workflow tests unaffected, e2e deferred to a real conflicting PR (reasonable — the fix lives or dies on GitHub Actions infra, not local behavior). One minor nice-to-have that isn't a blocker: a test assertion like Approving. ✅ 中文说明这是一个干净、最小的修复，解决了一个真实的 CI 故障。 我的独立提案与此完全一致——这个问题本质上只有一种修复方式，PR 以零范围蔓延落地了它。diff 是 1 个表达式改动 + 3 处注释重写，每个字符都服务于声明的目标。安全控制（sandbox、无 token 的 agent、 yamllint 干净，现有 workflow 测试不受影响，e2e 推迟到真实冲突 PR（合理——修复取决于 GitHub Actions 基础设施，不是本地行为）。 一个锦上添花但非阻塞项：添加类似 批准 ✅ — Qwen Code · qwen3.7-max |
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. ✅
qqqys
approved these changes
Jun 25, 2026
| # otherwise read the token. Runs regardless of push outcome. | ||
| # Scrub the PAT from .git/config defensively. Hosted runners are | ||
| # ephemeral so .git/config does not persist, but scrubbing the token | ||
| # as soon as the push is done keeps it out of the workspace that the |
Collaborator
There was a problem hiding this comment.
[Suggestion] The comment says scrubbing "keeps it out of the workspace that the untrusted PR's build/lint/test step runs in," but the verification gate (line ~898) executes before this step ever writes the PAT to .git/config (line 1130). The untrusted code has already finished by the time the token appears on disk — step ordering is the real protection, not scrubbing.
The scrub is still good defense-in-depth, but the comment could mislead a future maintainer into thinking it's safe to reorder steps (e.g., moving verification after push for fresher results), which would actually expose the PAT.
Suggested change
| # as soon as the push is done keeps it out of the workspace that the | |
| # Scrub the PAT from .git/config defensively. Hosted runners are | |
| # ephemeral so .git/config does not persist across jobs. The real | |
| # guarantee is that the verification gate (untrusted PR code) runs | |
| # BEFORE this step writes the token; do NOT reorder push before | |
| # verify. Runs regardless of push outcome. |
— qwen3.7-max via Qwen Code /review
| # determine command for sandbox"). Hosted runners ship docker and are | ||
| # ephemeral, which also suits running the untrusted PR's build/typecheck/ | ||
| # lint/test in the verification gate below. | ||
| runs-on: 'ubuntu-latest' |
Collaborator
There was a problem hiding this comment.
[Suggestion] The existing test suite in scripts/tests/qwen-resolve-workflow.test.js asserts security controls (persist-credentials: false, "sandbox": true, etc.) but never checks the runner type. A future edit that re-adds the ECS conditional (MAINTAINER_ECS_RUNNER_DISABLED pattern used by 3 other jobs) would pass all existing tests and only fail at runtime with exit 44.
Consider adding an assertion to the pins the core security controls on resolve-pr test:
// sandbox: true needs docker; hosted runners ship it, ECS does not.
expect(resolveJob).toContain("runs-on: 'ubuntu-latest'");
expect(resolveJob).not.toContain('ecs-qwen');
expect(resolveJob).not.toContain('MAINTAINER_ECS_RUNNER_DISABLED');— qwen3.7-max via Qwen Code /review
| # between PRs (hosted runners are ephemeral, so they need no such cleanup). | ||
| runs-on: "${{ vars.MAINTAINER_ECS_RUNNER_DISABLED != 'true' && fromJSON('[\"self-hosted\", \"linux\", \"x64\", \"ecs-qwen\"]') || fromJSON('[\"ubuntu-latest\"]') }}" | ||
| # Pinned to an ephemeral hosted runner. The conflict-resolution agent step | ||
| # runs with `sandbox: true`, which on Linux needs docker or podman to launch |
Collaborator
There was a problem hiding this comment.
[Suggestion] The comment explains why ECS can't be used here, but doesn't note that this intentionally diverges from the MAINTAINER_ECS_RUNNER_DISABLED kill-switch pattern still used by three other jobs in this file (lines 86, 128, 206). A maintainer doing a consistency pass across the file might "re-unify" the pattern, silently re-breaking /resolve.
Consider adding one line to the comment block:
# Unlike the ack, review-config, and authorize jobs, this one must
# NEVER use the ECS runner regardless of MAINTAINER_ECS_RUNNER_DISABLED.— qwen3.7-max via Qwen Code /review
yiliang114
enabled auto-merge
June 25, 2026 10:50
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
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
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
Pins the maintainer-triggered
/resolveconflict-resolution job to an ephemeral hosted runner (ubuntu-latest) instead of routing it onto the self-hosted ECS pool.Why it's needed
The job's agent step runs with
sandbox: true, which on Linux launches the sandbox through docker or podman. The self-hosted ECS pool has no container runtime, so the agent step fails before doing any work — exit 44,failed to determine command for sandbox— and the run reports a generic "did not complete successfully" on the PR (e.g. run 28158417390 triggered on #4943). Hostedubuntu-latestships docker, so the sandbox can start; it is also ephemeral, which suits running the untrusted PR's build/typecheck/lint/test in the verification gate.Reviewer Test Plan
How to verify
Static review of the
resolve-prjob:runs-onis nowubuntu-latest, with no remaining self-hosted-only dependency — norunner.*gating, no hardcoded ECS paths, and the secrets it uses (CI_BOT_PAT,CI_DEV_BOT_PAT,OPENAI_*) are repo-level and available on hosted runners. The cleanup and PAT-scrub steps are unchanged and remain harmless defensive no-ops on an ephemeral runner; only their comments were updated to drop the stale "reused self-hosted workspace" assumption.yamllinton the workflow is clean.End-to-end is intentionally deferred (see Out of scope): a follow-up dry-run on an in-repo conflicting PR should confirm the sandbox actually starts on hosted before relying on the command.
Evidence (Before & After)
N/A (CI configuration change). Before: run 28158417390 — agent step exit 44,
QWEN_SANDBOX is true but failed to determine command for sandbox; install docker or podman. After: the job targetsubuntu-latest, which ships docker.Tested on
Environment (optional)
GitHub Actions; workflow-only change. Verified locally with
yamllint.Risk & Scope
MAINTAINER_ECS_RUNNER_DISABLEDkill-switch — it is now always hosted. This is intentional: the kill-switch existed to move work off ECS, and the docker-backed sandbox requirement means this job must stay off ECS until the pool gains a container runtime. Cost is hosted-runner minutes for a low-frequency, maintainer-triggered command./resolverun on hosted (sandbox start → verify gate → push). Fork-PR support and replacement-PR creation remain out of scope, as in ci: add@qwen-code /resolve#5779.Linked Issues
Follow-up fix to #5779.
中文说明
What this PR does
把维护者触发的
/resolve冲突解决 job 固定到临时 hosted runner（ubuntu-latest），不再路由到自建 ECS pool。Why it's needed
该 job 的 agent 步骤以
sandbox: true运行，在 Linux 上要通过 docker 或 podman 启动 sandbox。自建 ECS pool 没有容器运行时，所以 agent 步骤在干活之前就失败了——exit 44、failed to determine command for sandbox——PR 上只显示笼统的 "did not complete successfully"（例如在 #4943 上触发的 run 28158417390）。Hostedubuntu-latest自带 docker，sandbox 能起；它还是临时的，适合在 verification gate 里跑不受信任 PR 的 build/typecheck/lint/test。Reviewer Test Plan
How to verify
静态审查
resolve-prjob：runs-on现在是ubuntu-latest，没有残留的 self-hosted 专属依赖——没有runner.*门控、没有硬编码 ECS 路径，用到的 secrets（CI_BOT_PAT、CI_DEV_BOT_PAT、OPENAI_*）都是 repo 级、hosted runner 同样可用。cleanup 和 PAT-scrub 步骤逻辑没动，在临时 runner 上是无害的防御性 no-op，只更新了其注释里过时的「复用 self-hosted 工作区」假设。workflow 跑yamllint干净。端到端验证有意推迟（见 Out of scope）：后续应在一个本仓库的冲突 PR 上 dry-run 一次，确认 hosted 上 sandbox 真能起来再依赖这个命令。
Evidence (Before & After)
N/A（CI 配置改动）。Before：run 28158417390——agent 步骤 exit 44，
QWEN_SANDBOX is true but failed to determine command for sandbox; install docker or podman。After：job 跑在自带 docker 的ubuntu-latest上。Tested on
见上表。
Environment (optional)
GitHub Actions；纯 workflow 改动。本地用
yamllint验证。Risk & Scope
MAINTAINER_ECS_RUNNER_DISABLEDkill-switch——现在恒定 hosted。这是有意的：kill-switch 本来就是用来把任务挪离 ECS，而 docker sandbox 的需求意味着在 ECS pool 装上容器运行时之前，这个 job 必须留在 ECS 之外。代价是这个低频、维护者触发的命令会消耗 hosted runner 分钟数。/resolve运行（sandbox 启动 → verify gate → push）。fork PR 支持与 replacement-PR 创建仍然 out of scope，与 ci: add@qwen-code /resolve#5779 一致。Linked Issues
#5779 的后续修复。