ci: retry merge-ref checkout to fix transient "not our ref" failures#5732
Merged
Conversation
yiliang114
changed the title
yiliang114
force-pushed
the
ci/checkout-pr-head-sha
branch
from
June 23, 2026 02:47
1782a59 to
9486521
Compare
post_coverage_comment used if: always(), so on a cancelled run it still ran on the hosted pool. During rapid pushes the cancelled run's coverage job sat queued for a hosted runner, keeping the run non-terminal and holding the branch concurrency slot — so the next push's required Test (ubuntu-latest) check waited behind it (observed ~9 min, with thrash). Switch to !cancelled(): coverage still posts on test failure, but a cancelled run releases the slot immediately. Its coverage would be stale anyway.
yiliang114
force-pushed
the
ci/checkout-pr-head-sha
branch
from
June 23, 2026 02:48
9486521 to
53fd5e1
Compare
`/triage` comments trigger the authorize+triage jobs on hosted runners, which queue behind CI during peak hours (~2 min vs 4 s off-peak observed). Route the comment-triggered triage job to the ECS self-hosted pool, which is not bound by the GitHub-hosted concurrency limit. Safe scope: only `issue_comment` events; the job is read-only and checks out the base ref (never PR-head code). Gated on the same MAINTAINER_ECS_RUNNER_DISABLED kill-switch as `authorize`, so a disabled ECS pool falls back to hosted. PR/issues/dispatch triggers stay hosted.
yiliang114
force-pushed
the
ci/checkout-pr-head-sha
branch
from
June 23, 2026 02:54
7c9de93 to
c72db9d
Compare
yiliang114
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens GitHub Actions workflows to reduce intermittent required-check failures and improve responsiveness of /triage comment-triggered runs by adjusting checkout refs, cancellation behavior, and runner routing.
Changes:
- Update Lint/Test checkouts to fetch
refs/pull/<N>/headby ref name (avoiding PR merge-ref churn) with fallbacks for non-PR events. - Prevent the coverage-comment job from running on cancelled workflows to release branch concurrency sooner.
- Route
/triageruns triggered viaissue_commentto the ECS self-hosted runner pool, behind a kill-switch.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .github/workflows/qwen-triage.yml | Routes /triage issue_comment runs to the ECS runner pool with a kill-switch fallback to hosted. |
| .github/workflows/ci.yml | Switches Lint/Test checkout ref to refs/pull/N/head and updates the coverage-comment job condition to avoid cancelled-run queuing. |
Comments suppressed due to low confidence (1)
.github/workflows/ci.yml:303
post_coverage_commentno longer usesalways(). Without it, this job will be skipped whenever thetestjob fails, so coverage comments won’t be posted on failing test runs (only on successful ones). If the intent is “run on success/failure but not on cancellation”, keepalways()and add the!cancelled()guard.
# !cancelled() not always(): don't let a cancelled run hold the concurrency slot here.
if: |-
${{
!cancelled() &&
needs.classify_pr.outputs.skip_ci != 'true' &&
github.event_name == 'pull_request' &&
github.event.pull_request.head.repo.full_name == github.repository
}}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Collaborator
|
Thanks for the PR! (Re-triage after the revision addressing @qqqys's review.) Template looks good ✓ — all required sections present, bilingual body, clear reviewer test plan. On direction: squarely aligned. CI flakiness from transient merge-ref lag is a real drag on merge throughput, and the fix targets a concrete, observed failure mode. No CHANGELOG precedent needed — this is infra, not user-facing. On approach: the revised scope is even tighter than the original — now just the checkout back-off/retry for the required Lint/Test jobs and the One thing to flag before code review: the Lint job's base checkout uses Moving on to code review. 🔍 中文说明感谢贡献！（根据 @qqqys 评审意见修改后的重新 triage。） 模板完整 ✓ — 所有章节齐全，双语正文，清晰的评审验证计划。 方向：完全对齐。merge ref 瞬时延迟导致的 CI 不稳定是影响合并吞吐的实际问题，修复针对具体的、已观察到的失败模式。无需 CHANGELOG 先例——基础设施改动，非用户功能。 方案：修改后的范围比原版更紧凑——现在仅 代码审查前需要指出一点：Lint job 的基础 checkout 使用 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Collaborator
|
Code review: Clean, surgical diff — one file ( Lint checkout retry (lines ~110-128): The pattern is correct — first checkout with One finding: the base Lint checkout has Test checkout retry (lines ~226-244): Same pattern, correctly includes the Coverage comment (line ~316): No security concerns. No AGENTS.md violations. No reuse issues (standard CI results (revised PR):
The checkout retry succeeds on all platforms that have completed — confirming the merge-ref back-off works. No regressions from the Tmux testing: N/A — CI workflow change, no user-visible or TUI surface. CI check results serve as evidence. 中文说明代码审查： diff 干净、精准——一个文件（ Lint checkout 重试（约 110-128 行）：模式正确——第一次 checkout 带 一个发现：基础 Lint checkout 有 Test checkout 重试（约 226-244 行）：同样模式，正确包含 覆盖率评论（约 316 行）： 无安全问题。无 AGENTS.md 违规。无复用问题（标准 CI 结果（修改后的 PR）： Classify PR ✓、Lint ✓、Test (ubuntu-latest) ✓、Test (macos-latest) ✓、CodeQL ✓、Integration (no AK) ✓。checkout 重试在所有已完成的平台上成功——确认 merge ref 退避有效。 Tmux 测试： N/A——CI 工作流改动，无用户可见或 TUI 表现。CI 检查结果即为证据。 — Qwen Code · qwen3.7-max |
Collaborator
|
Stepping back to think about this holistically. The revised PR is a tight, well-reasoned CI fix. After The core insight is sound: GitHub builds the merge ref asynchronously, so a back-off-and-retry on the same ref is the right fix. Falling back to a different ref (head SHA) would silently drop merge-result validation, which is a worse trade-off than a 10-second delay on the rare transient failure. My independent proposal for this problem would have been essentially the same — add One minor finding from code review: the Lint retry checkout is missing CI evidence confirms the checkout retry works across all completed platforms (Lint ✓, Test ubuntu ✓, Test macos ✓, CodeQL ✓, Integration ✓). No regressions detected. The Windows test is still pending but the checkout pattern is identical to the other platforms. If I had to maintain this in six months, I'd thank the author for the inline comments explaining the async merge-ref build and the same-ref retry rationale. Approving. ✅ 中文说明退一步整体审视。 修改后的 PR 是一组紧凑、推理清晰的 CI 修复。在 @qqqys 的评审之后，作者做了正确的决定：撤回 head ref pin 以恢复合并结果校验，撤回自建 triage 路由以保持信任边界紧凑，范围仅限 checkout 重试和 核心洞察正确：GitHub 异步构建 merge ref，因此在 同一个 ref 上退避重试是正确的修复。回退到不同的 ref（head SHA）会悄悄丢弃合并结果校验，这比罕见瞬时故障时的 10 秒延迟更糟糕。 我对这个问题的独立方案本质上相同——添加 代码审查的一个小发现：Lint 重试 checkout 缺少基础 Lint checkout 具有的 CI 证据确认 checkout 重试在所有已完成平台上有效（Lint ✓、Test ubuntu ✓、Test macos ✓、CodeQL ✓、Integration ✓）。未检测到回归。Windows 测试仍在进行中，但 checkout 模式与其他平台相同。 如果六个月后我来维护，会感谢作者在注释中解释异步 merge ref 构建和同 ref 重试的推理。批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
approved these 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.
LGTM, looks ready to ship. ✅
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. |
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.
[Suggestion] Three other checkout steps in ci.yml were not updated to the refs/pull/N/head pattern: post_coverage_comment (line 317), codeql (line 347), and integration_no_ak (line 369). The integration_no_ak job runs exclusively on pull_request events and still resolves to refs/pull/N/merge — the exact ref this PR aims to avoid. Consider updating all checkouts for consistency, or adding comments explaining why the default is acceptable for non-required jobs.
— qwen3.7-max via Qwen Code /review
On workflow_dispatch the Test job runs (its if: !cancelled() overrides the implicit success() gate when classify_pr is skipped), so its checkout must honor the branch_ref input like Lint does. Without it, dispatch runs validated github.ref in Test while Lint validated the user-supplied branch_ref.
yiliang114
commented
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.
No review findings. Downgraded from Approve to Comment: CI still running.
All three changes are correct and well-reasoned:
- Checkout ref: the
refs/pull/N/headexpression is character-for-character identical between Lint and Test jobs, with correct fallback chains for all four trigger events (pull_request,push,merge_group,workflow_dispatch). Operator precedence is sound —&&binds tighter than||. !cancelled(): correctly replacesalways().cancelled()is a status check function, so its presence in theifcondition overrides the default skip-on-dependency-failure behavior. Coverage comments still post when tests fail; only cancelled runs skip the job, releasing the concurrency slot as intended.- ECS routing: the
runs-onexpression follows the establishedauthorizejob pattern with theMAINTAINER_ECS_RUNNER_DISABLEDkill-switch andfromJSONlabel arrays.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
previously approved these 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.
LGTM, looks ready to ship. ✅
Collaborator
|
Thanks for working on the CI queueing/checkout reliability issues. I found two concerns that I think we should address before merging:
The YAML shape itself looks fine; this is mainly about preserving required-check semantics and keeping the self-hosted runner boundary tight. |
qqqys
previously approved these changes
Jun 23, 2026
qqqys
left a comment
qqqys
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed the workflow changes. I left comments on the checkout/runner trust-boundary concerns, but approving per maintainer decision.
… ECS routing Per review feedback: checking out refs/pull/N/head dropped merge-result validation, and merge queue isn't enabled on main to backstop it. Keep merge-ref semantics (github.ref) for the required Lint/Test checkouts and retry once to absorb the transient merge-ref lag the PR-head change was working around. Also revert /triage routing to ECS: that job runs an agent (sandbox:false, shell/write tools, repo tokens) over untrusted PR/issue text, so it stays on ephemeral hosted runners, not the persistent self-hosted pool.
yiliang114
changed the title
The merge ref (refs/pull/N/merge) is built asynchronously, so a back-to-back retry can hit the still-unbuilt ref again. Sleep 10s before retrying so the retry absorbs the multi-second build lag, not just instantaneous blips.
Collaborator
Author
|
Thanks — took both. 1. Checkout — reverted the 2. Net diff is now just the checkout back-off/retry plus the coverage-comment |
Collaborator
Author
|
@qwen-code /triage |
qwen-code-ci-bot
approved these 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.
LGTM, looks ready to ship. ✅
yiliang114
enabled auto-merge (squash)
June 23, 2026 08:35
qqqys
approved these changes
Jun 23, 2026
| if: "${{ steps.checkout.outcome == 'failure' }}" | ||
| uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 | ||
| with: | ||
| ref: '${{ github.event.inputs.branch_ref || github.ref }}' |
Collaborator
There was a problem hiding this comment.
[Suggestion] The Lint job's initial checkout (line 120) specifies fetch-depth: 0, but this retry checkout (the pre-existing line repurposed as fallback) has no fetch-depth and defaults to 1. If the first checkout fails and the retry succeeds, the Lint job runs against a shallow clone — any lint step that depends on git history would see truncated results.
Suggested change
| ref: '${{ github.event.inputs.branch_ref || github.ref }}' | |
| uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 | |
| with: | |
| ref: '${{ github.event.inputs.branch_ref || github.ref }}' | |
| fetch-depth: 0 |
— qwen3.7-max via Qwen Code /review
| fetch-depth: 0 | ||
| - name: 'Back off for the merge ref to build' | ||
| if: "${{ steps.checkout.outcome == 'failure' }}" | ||
| run: 'sleep 10' |
Collaborator
There was a problem hiding this comment.
[Suggestion] The backoff step is run: 'sleep 10' with no diagnostic output. When the retry fires, there's no ::warning:: annotation in the Actions log, making it hard to track how often the retry triggers or to debug checkout failures at a glance.
Suggested change
| run: 'sleep 10' | |
| run: |- | |
| echo "::warning::Initial checkout failed for ref ${{ github.ref }} (likely transient merge-ref lag). Retrying after 10s backoff." | |
| sleep 10 |
— qwen3.7-max via Qwen Code /review
| - name: 'Checkout' | ||
| id: 'checkout' | ||
| uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 | ||
| continue-on-error: true |
Collaborator
There was a problem hiding this comment.
[Suggestion] The continue-on-error: true on this checkout is a load-bearing part of the retry mechanism — without it, a checkout failure immediately fails the job and the retry step is never reached. There's no comment explaining this dependency, so a future maintainer could reasonably remove it as defensive clutter, silently killing the retry. A one-line comment prevents this.
Suggested change
| continue-on-error: true | |
| continue-on-error: true # required: lets the job reach the retry step below |
(Same applies to the Test job's continue-on-error on line 233.)
— qwen3.7-max via Qwen Code /review
This was referenced Jun 23, 2026
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
The required Lint and Test jobs check out the PR merge ref (
github.refresolves torefs/pull/N/mergeon pull requests). GitHub builds that ref asynchronously, so right after a push it can briefly 404 and fail the checkout withcouldn't find remote ref. This makes the first checkoutcontinue-on-errorand, on failure, backs off for a few seconds and retries the same merge ref once — so a transient lag no longer fails a required check, while still validating the merge result rather than just the PR tip. It also stops the coverage-comment job from holding a concurrency slot when a run is cancelled (always()→!cancelled()).Why it's needed
Transient merge-ref checkout failures were flaking the required Lint/Test checks and blocking otherwise-green PRs. An earlier revision of this PR pinned the checkout to
refs/pull/N/headto dodge the lag, but that dropped merge-result validation — a PR could pass required checks yet break after merging withmain— and merge queue is not enabled onmainto backstop it. Backing off and retrying the merge-ref checkout fixes the flake without giving up merge semantics. The back-off matters because the merge ref is built asynchronously: a back-to-back retry would likely hit the still-unbuilt ref again.Reviewer Test Plan
How to verify
ci.ymldiff: the Lint and TestCheckoutsteps now run withcontinue-on-error: trueplus anid, followed by a shortsleepand a secondCheckout, both gated onsteps.checkout.outcome == 'failure'. Both checkouts use the samegithub.event.inputs.branch_ref || github.refref, so the retry targets the identical (merge) ref — no fallback to a different ref.outcome == 'failure'is false) and the job proceeds exactly as before.actionlintpasses onci.yml.Evidence (Before & After)
N/A — CI-config change, no user-visible behavior.
Tested on
Workflow-only change, validated via
actionlintand expression review rather than a local run.Environment (optional)
N/A
Risk & Scope
codeql,integration_no_ak) check out the same merge ref and remain exposed to the same lag — they are intentionally not retried here; a uniform fix would warrant a shared composite action and is left as follow-up. Also out of scope: enabling merge queue onmainas the longer-term merge-result gate;/triagecomment latency on hosted runners (kept on hosted — routing that agent job to the persistent self-hosted pool widens the trust boundary, per review).Linked Issues
N/A
中文说明
这个 PR 做了什么
必需的 Lint 和 Test job 检出的是 PR 的 merge ref（
github.ref在 pull request 上解析为refs/pull/N/merge）。GitHub 异步构建这个 ref，所以在 push 之后它可能短暂 404，导致 checkout 报couldn't find remote ref失败。本 PR 让第一次 checkout 带continue-on-error，失败时退避几秒再用同一个 merge ref 重试一次——使瞬时延迟不再让必需检查失败，同时仍然校验合并结果而非只校验 PR 分支顶端。另外，让 coverage 评论 job 在 run 被取消时不再占用 concurrency 槽（always()→!cancelled()）。为什么需要
瞬时的 merge-ref checkout 失败一直在让必需的 Lint/Test 检查变 flaky，阻塞本该通过的 PR。本 PR 早先的版本改用
refs/pull/N/head来绕开延迟，但那丢掉了合并结果校验——PR 可能通过必需检查却在合并进main后挂掉——而main上又没启用 merge queue 来兜底。退避后重试 merge-ref checkout 在不放弃合并语义的前提下修掉了这个 flake。退避是必要的：merge ref 是异步构建的，背靠背立即重试很可能再次撞上尚未构建好的 ref。评审验证计划
如何验证
ci.ymldiff：Lint 和 Test 的Checkout步骤现在带continue-on-error: true和id，后面跟一个短sleep和第二个Checkout，两者都由steps.checkout.outcome == 'failure'门控。两次 checkout 用同一个github.event.inputs.branch_ref || github.ref，所以重试针对的是完全相同的（merge）ref——不会 fallback 到不同的 ref。outcome == 'failure'为假），job 与之前完全一致。actionlint在ci.yml上通过。证据（前后对比）
N/A——CI 配置改动，无用户可见行为。
测试平台
仅工作流改动，通过
actionlint和表达式审查验证，未本地运行。环境（可选）
N/A
风险与范围
codeql、integration_no_ak）检出同一个 merge ref，同样暴露于这个延迟——此处有意不给它们加重试；统一修复应当用共享的 composite action，留作 follow-up。同样范围外：在main上启用 merge queue 作为更长期的合并结果 gate；hosted runner 上/triage评论的排队延迟（保留在 hosted——把那个 agent job 路由到持久自建池会扩大信任边界，依评审意见）。关联 Issue
N/A