-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(ci): surface blocked autofix takeover admission #8410
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
2591c39
ea34938
3c982e9
7170261
e41f39d
62a1074
339540a
e49aade
5d45d9f
bf6d796
d8d66b5
dfc6ec5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1881,6 +1881,16 @@ jobs: | |||||||||||||
| ${{ needs.route.outputs.do_review == 'true' }} | ||||||||||||||
| runs-on: 'ubuntu-latest' | ||||||||||||||
| timeout-minutes: 15 | ||||||||||||||
| # A forced scan can write the same status comment as review-address. | ||||||||||||||
| # Share its per-PR lock so neither writer can erase the other's state. | ||||||||||||||
| # Keep the predicate as narrow as the job's own `if:` — concurrency is | ||||||||||||||
| # evaluated BEFORE it, so without the do_review conjunct a dispatch with | ||||||||||||||
| # `phase: issue` + `pr_number: N` (route emits pr_number unconditionally) | ||||||||||||||
| # would park this skipped job in that PR's shared slot behind a 300-minute | ||||||||||||||
| # address round, stalling the issue phase that `needs` it. | ||||||||||||||
| concurrency: | ||||||||||||||
| group: "qwen-pr-head-write-${{ needs.route.outputs.do_review == 'true' && needs.route.outputs.pr_number || github.run_id }}" | ||||||||||||||
| cancel-in-progress: false | ||||||||||||||
| outputs: | ||||||||||||||
| targets: '${{ steps.scan.outputs.targets }}' | ||||||||||||||
| has_targets: '${{ steps.scan.outputs.has_targets }}' | ||||||||||||||
|
|
@@ -1907,6 +1917,186 @@ jobs: | |||||||||||||
| } | ||||||||||||||
| WORKDIR="$(mktemp -d)" | ||||||||||||||
|
|
||||||||||||||
| read_forced_pr_meta() { | ||||||||||||||
| local attempt meta | ||||||||||||||
| for attempt in 1 2 3; do | ||||||||||||||
|
Comment on lines
+1920
to
+1922
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R8-1: The same 3-attempt / linear-backoff / per-attempt-warning retry skeleton is pasted six times inside the one shell block this diff adds ( Suggested fix: extract one local retry helper beside the new functions and use it for the four plain-command reporter loops (the two reader loops keep their bespoke in-loop validation but share the backoff/warning shape): retry3() { # <warning-what> <cmd...> — warns per attempt, linear backoff
local what="$1" attempt; shift
for attempt in 1 2 3; do
if "$@"; then return 0; fi
echo "::warning::${what} (attempt ${attempt}/3)" >&2
[[ "${attempt}" -lt 3 ]] && sleep "${attempt}"
done
return 1
}中文说明R8-1:本 diff 新增的同一个 shell 块中,相同的"3 次尝试 / 线性退避 / 每次尝试告警"重试骨架被粘贴了六次( — qwen3.8-max via Qwen Code /review (v0.21.6) |
||||||||||||||
| if meta="$(gh pr view "${FORCED_PR}" --repo "${REPO}" \ | ||||||||||||||
| --json number,state,author,headRefName,isCrossRepository,baseRefName,labels,maintainerCanModify 2> /dev/null)" \ | ||||||||||||||
|
Comment on lines
+1923
to
+1924
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R4-3: All four new 中文说明四个新增的 — qwen3.8-max via Qwen Code /review (v0.21.5) |
||||||||||||||
| && jq -e 'type == "object" | ||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R1-4: Test gap — the shape-validation negative path of the new readers is never exercised: the retry tests feed only fully-valid payloads or an outright failing Suggested fix: add two reader scenarios to the existing harness — a 中文说明测试缺口——新读取器的结构校验负路径从未被执行:重试用例只喂完全合法的 payload 或直接失败的 建议修复:在现有测试装置中新增两个读取器场景——exit 0 输出 — qwen3.8-max via Qwen Code /review (v0.21.4) |
||||||||||||||
| and (.number | type == "number") | ||||||||||||||
|
Comment on lines
+1925
to
+1926
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R1-4: Still stands — the shape-validation negative path of the new readers is never exercised: the retry tests feed only fully-valid payloads or an outright failing Suggested fix: in the existing 中文说明[Suggestion] R1-4:仍然存在——新读取器的结构校验负路径从未被测试覆盖:重试测试要么喂完全合法的载荷,要么让 建议修复:在现有 — qwen3.8-max via Qwen Code /review (v0.21.4) |
||||||||||||||
| and (.state | type == "string") | ||||||||||||||
| and (.author.login | type == "string") | ||||||||||||||
| and (.headRefName | type == "string") | ||||||||||||||
| and (.baseRefName | type == "string") | ||||||||||||||
| and (.isCrossRepository | type == "boolean") | ||||||||||||||
| and (.labels | type == "array") | ||||||||||||||
| and (.maintainerCanModify | type == "boolean")' > /dev/null <<< "${meta}"; then | ||||||||||||||
|
Comment on lines
+1932
to
+1933
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Test gap (pattern, instance 2/4): the shape-validation negative path of Suggested fix: add fake- 中文说明测试缺口(模式,第 2/4 处): 建议修复:新增退出码为 0 但载荷不合法的 fake- — qwen3.8-max-preview via Qwen Code /review (v0.21.4) |
||||||||||||||
| printf '%s' "${meta}" | ||||||||||||||
| return 0 | ||||||||||||||
| fi | ||||||||||||||
| echo "::warning::Forced PR #${FORCED_PR} metadata lookup failed (attempt ${attempt}/3)" >&2 | ||||||||||||||
| [[ "${attempt}" -lt 3 ]] && sleep "${attempt}" | ||||||||||||||
| done | ||||||||||||||
| return 1 | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| # 'none' and HTTP 404 are DEFINITIVE answers, not lookup failures. | ||||||||||||||
| # GitHub returns 200 with permission 'none' for logins that exist but | ||||||||||||||
| # hold nothing here (bot-type logins such as dependabot[bot], and org | ||||||||||||||
| # logins), and 404 for logins that do not exist or are empty. Both | ||||||||||||||
| # mean "no write access" — the routine rejection this gate is for. | ||||||||||||||
| # Retrying them would burn 3 API calls plus back-off per candidate per | ||||||||||||||
| # scheduled tick, forever, and strand the caller on | ||||||||||||||
| # 'permission_lookup_failed': a red forced run (exit 1) whose blocked | ||||||||||||||
| # comment promises "a later scheduled scan will retry" — a retry that | ||||||||||||||
| # can never succeed — while the actionable "grant the fork author | ||||||||||||||
| # write access" guidance behind author_permission_* stays unreachable. | ||||||||||||||
| # Only genuinely transient answers (5xx, network, auth) retry. | ||||||||||||||
| read_live_permission() { | ||||||||||||||
| local login="$1" attempt permission err result='' | ||||||||||||||
| # An empty login can only 404; skip the call and answer terminally. | ||||||||||||||
| if [[ -z "${login}" ]]; then | ||||||||||||||
| printf 'none' | ||||||||||||||
| return 0 | ||||||||||||||
| fi | ||||||||||||||
| err="$(mktemp)" | ||||||||||||||
| for attempt in 1 2 3; do | ||||||||||||||
| if permission="$(gh api "repos/${REPO}/collaborators/${login}/permission" --jq '.permission // ""' 2> "${err}")" \ | ||||||||||||||
| && [[ "${permission}" =~ ^(admin|maintain|write|triage|read|none)$ ]]; then | ||||||||||||||
| result="${permission}" | ||||||||||||||
| break | ||||||||||||||
| fi | ||||||||||||||
| if grep -q 'HTTP 404' "${err}"; then | ||||||||||||||
| result='none' | ||||||||||||||
| break | ||||||||||||||
| fi | ||||||||||||||
| # Surface gh's own diagnosis instead of discarding it: a rate | ||||||||||||||
| # limit, an expired PAT and a 5xx all look identical otherwise. | ||||||||||||||
| echo "::warning::Permission lookup failed for ${login} (attempt ${attempt}/3): $(tr '\n' ' ' < "${err}")" >&2 | ||||||||||||||
| [[ "${attempt}" -lt 3 ]] && sleep "${attempt}" | ||||||||||||||
| done | ||||||||||||||
| rm -f "${err}" | ||||||||||||||
| [[ -n "${result}" ]] || return 1 | ||||||||||||||
| printf '%s' "${result}" | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| forced_admission_reason() { | ||||||||||||||
| jq -r --arg ab "${AUTOFIX_BOT}" --arg take "${TAKEOVER_LABEL}" --arg skip "${SKIP_LABEL}" ' | ||||||||||||||
| if (.state // "") != "OPEN" then "not_open" | ||||||||||||||
| elif (.baseRefName // "") != "main" then "wrong_base" | ||||||||||||||
| elif ([.labels[]?.name] | index($skip) != null) then "skip_label" | ||||||||||||||
|
Comment on lines
+1986
to
+1987
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] No classifier test case pins Suggested fix — one assertion in the classifier test: expect(
reason(
meta('human', ['autofix/takeover', 'autofix/skip'], {
isCrossRepository: true,
maintainerCanModify: false,
}),
),
).toBe('skip_label');中文说明分类器测试没有任何用例钉住 建议修复——在分类器测试中加一条断言(见上方代码块)。 — qwen3.8-max via Qwen Code /review (v0.21.5) |
||||||||||||||
| elif ((((.author.login // "") == $ab) or ([.labels[]?.name] | index($take) != null)) | not) then "unmanaged_author" | ||||||||||||||
| elif (.isCrossRepository == true) and (.maintainerCanModify != true) then "maintainer_edits_disabled" | ||||||||||||||
| elif (((.isCrossRepository == true) or (.isCrossRepository == false)) | not) then "cross_repo_state_missing" | ||||||||||||||
| else "eligible" | ||||||||||||||
| end' | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| report_forced_takeover_blocked() { | ||||||||||||||
| local reason="$1" actor status_ids status_id body attempt status_lookup_ok next_en next_zh err | ||||||||||||||
| [[ "${DRY_RUN}" == 'true' ]] && return 0 | ||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Reverse audit round 3: the reporter's Suggested fix: add one 中文说明[Suggestion] 反向审计第 3 轮:报告函数的 建议修复:新增一个 — qwen3.8-max via Qwen Code /review (v0.21.4) |
||||||||||||||
| [[ "$(jq -r --arg ab "${AUTOFIX_BOT}" --arg take "${TAKEOVER_LABEL}" ' | ||||||||||||||
| ((.author.login // "") == $ab) or ([.labels[]?.name] | index($take) != null) | ||||||||||||||
| ' <<< "${META}")" == 'true' ]] || return 0 | ||||||||||||||
|
Comment on lines
+1998
to
+2000
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R8-2: This re-derives "PR is managed" with a second copy of the classifier's
Suggested change
中文说明R8-2:此处用分类器 — qwen3.8-max via Qwen Code /review (v0.21.6) |
||||||||||||||
| case "${reason}" in | ||||||||||||||
| permission_lookup_failed|author_permission_*|maintainer_edits_disabled|cross_repo_state_missing) ;; | ||||||||||||||
| *) return 0 ;; | ||||||||||||||
|
Comment on lines
+2001
to
+2003
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Test gap (pattern, instance 1/4): this terminal/non-terminal gate has no behavioral test — a confirmed surviving mutant proves it. Changing Suggested fix: parameterize the existing reporter harness — one non-terminal reason (assert rc=0 and zero 中文说明测试缺口(模式,第 1/4 处):该终态/非终态门没有行为测试——已确认的存活变异体证明了这一点。把 建议修复:参数化现有 reporter 测试装置——一个非终态原因(断言 rc=0 且零 — qwen3.8-max-preview via Qwen Code /review (v0.21.4) |
||||||||||||||
| esac | ||||||||||||||
| actor='' | ||||||||||||||
| for attempt in 1 2 3; do | ||||||||||||||
| if actor="$(gh api user --jq '.login' 2> /dev/null)" && [[ -n "${actor}" ]]; then | ||||||||||||||
| break | ||||||||||||||
| fi | ||||||||||||||
| echo "::warning::PAT identity lookup failed (attempt ${attempt}/3)" >&2 | ||||||||||||||
| [[ "${attempt}" -lt 3 ]] && sleep "${attempt}" | ||||||||||||||
| done | ||||||||||||||
| if [[ "${actor}" != "${AUTOFIX_BOT}" ]]; then | ||||||||||||||
| echo "::warning::Blocked takeover status skipped: PAT authenticates as '${actor:-unknown}'" >&2 | ||||||||||||||
| return 1 | ||||||||||||||
|
Comment on lines
+2013
to
+2015
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] This PAT-identity guard warns
Suggested change
中文说明[Suggestion] 这个 PAT 身份守卫打出 (建议修复见上方 suggestion 代码块:改用 error 级别并同时输出实际与期望身份。) — qwen3.8-max via Qwen Code /review (v0.21.4) |
||||||||||||||
| fi | ||||||||||||||
| if [[ "${reason}" == 'maintainer_edits_disabled' ]]; then | ||||||||||||||
| next_en='Re-enable maintainer edits on the fork PR to resume takeover.' | ||||||||||||||
| next_zh='请在 fork PR 上重新允许 maintainer edits,以恢复 takeover。' | ||||||||||||||
| elif [[ "${reason}" == author_permission_* ]]; then | ||||||||||||||
| next_en='Grant the fork author write access, or remove the autofix/takeover label, to resume takeover.' | ||||||||||||||
| next_zh='请授予 fork 作者 write 权限,或移除 autofix/takeover 标签,以恢复 takeover。' | ||||||||||||||
| else | ||||||||||||||
| next_en='A later scheduled scan will retry without advancing the feedback watermark.' | ||||||||||||||
| next_zh='后续定时扫描会重试,本次不会推进反馈水位。' | ||||||||||||||
|
Comment on lines
+2023
to
+2025
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R4-1: The blocked comment gives an actionable remedy only for 中文说明blocked 评论只为 — qwen3.8-max via Qwen Code /review (v0.21.5)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 已修复 + 验证证据:author_permission_* 现在明确提示授予 write 权限或移除 autofix/takeover 标签,新增文案分支回归;聚焦用例 1/1 通过,完整 workflow 111/111 断言通过,Prettier 与 git diff --check 通过。 |
||||||||||||||
| fi | ||||||||||||||
| # Every other status writer resolves its run link from | ||||||||||||||
| # github.server_url / GITHUB_SERVER_URL. Hardcoding github.com here | ||||||||||||||
| # would make the one link this message exists to surface the only | ||||||||||||||
| # broken one on a GHES or proxied host. | ||||||||||||||
| body="$(printf '<!-- autofix-status -->\n\n⛔ **AutoFix blocked** — takeover admission stopped at `%s`, so no work was started. [View run](%s/%s/actions/runs/%s). %s\n\n<details>\n<summary>中文说明</summary>\n\n⛔ **AutoFix 已阻塞** —— takeover 准入停在 `%s`,因此本轮未开始处理。[查看运行](%s/%s/actions/runs/%s)。%s\n\n</details>' \ | ||||||||||||||
| "${reason}" "${GITHUB_SERVER_URL}" "${REPO}" "${GITHUB_RUN_ID}" "${next_en}" \ | ||||||||||||||
| "${reason}" "${GITHUB_SERVER_URL}" "${REPO}" "${GITHUB_RUN_ID}" "${next_zh}")" | ||||||||||||||
| status_ids='' | ||||||||||||||
| status_lookup_ok=false | ||||||||||||||
| err="$(mktemp)" | ||||||||||||||
| # Same filter as the sibling upsert in 'Post autofix status comment', | ||||||||||||||
| # including its two guards: `// ""` so a single comment with a null | ||||||||||||||
| # body cannot abort the whole program (jq exits 5, all three | ||||||||||||||
| # attempts fail, and the run reds out WITHOUT posting the very | ||||||||||||||
| # status it exists to post), and --arg so a repo-configured | ||||||||||||||
| # AUTOFIX_BOT_LOGIN containing " or \ is a mismatch instead of a jq | ||||||||||||||
| # parse error. Stays an inline id stream into `tail -1` — it never | ||||||||||||||
| # lands in a WORKDIR json file, so the WORKDIR page normalizer | ||||||||||||||
| # (add-with-empty-default) must NOT be applied here: it would wrap | ||||||||||||||
| # the id stream in an array and break the tail-1 consumer. | ||||||||||||||
| # pipefail is set LOCALLY here rather than relied on: this `if` | ||||||||||||||
| # must test gh's status, not jq's. A gh failure carrying an HTTP | ||||||||||||||
| # status prints the error body to stdout, so jq errors out and the | ||||||||||||||
| # retry fires — but a CONNECTION-level failure (TCP reset, TLS | ||||||||||||||
| # abort, DNS blip) leaves stdout EMPTY, and `jq -rs` then prints | ||||||||||||||
| # nothing and exits 0. Without pipefail that reads as success on | ||||||||||||||
| # nothing read: status_lookup_ok=true, the empty id takes the | ||||||||||||||
| # writer down the "no status comment yet" branch, and it posts a | ||||||||||||||
| # DUPLICATE ⛔ blocked comment beside the stale ✅ one — the exact | ||||||||||||||
| # two-status state this function exists to prevent — on a green | ||||||||||||||
| # run. `defaults.run.shell: bash` already gives every step in this | ||||||||||||||
| # file `-eo pipefail`, so this is redundant today; it is also the | ||||||||||||||
| # only guard that survives that default changing or this helper | ||||||||||||||
| # being lifted into a step that sets its own options. | ||||||||||||||
| for attempt in 1 2 3; do | ||||||||||||||
| if status_ids="$(set -o pipefail; gh api "repos/${REPO}/issues/${FORCED_PR}/comments" --paginate 2> "${err}" | | ||||||||||||||
| jq -rs --arg ab "${AUTOFIX_BOT}" --arg m '<!-- autofix-status -->' \ | ||||||||||||||
| '.[][] | select((.user.login // "") == $ab) | select((.body // "") | contains($m)) | .id')"; then | ||||||||||||||
| status_lookup_ok=true | ||||||||||||||
| break | ||||||||||||||
| fi | ||||||||||||||
| # Surface gh's own diagnosis instead of discarding it, exactly as | ||||||||||||||
| # read_live_permission does: a rate limit, an expired PAT and a | ||||||||||||||
| # 5xx are indistinguishable from 'attempt 3/3' alone. | ||||||||||||||
| echo "::warning::Takeover status lookup failed for #${FORCED_PR} (attempt ${attempt}/3): $(tr '\n' ' ' < "${err}")" >&2 | ||||||||||||||
| [[ "${attempt}" -lt 3 ]] && sleep "${attempt}" | ||||||||||||||
| done | ||||||||||||||
| rm -f "${err}" | ||||||||||||||
| if [[ "${status_lookup_ok}" != 'true' ]]; then | ||||||||||||||
| echo "::warning::Failed to read takeover status comments for #${FORCED_PR}" >&2 | ||||||||||||||
| return 1 | ||||||||||||||
| fi | ||||||||||||||
| status_id="$(tail -1 <<< "${status_ids}")" | ||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Test-oracle gap: the newest-status-comment selection Suggested fix: have the fake comments endpoint print 中文说明[Suggestion] 测试预言机缺口:最新评论选择 建议修复:让某个报告场景的假评论端点输出 — qwen3.8-max via Qwen Code /review (v0.21.4) |
||||||||||||||
| if [[ -n "${status_id}" ]]; then | ||||||||||||||
|
Comment on lines
+2079
to
+2080
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R1-2: The blocked-state PATCH targets the single Suggested fix: scope the claim or skip the write when work is in flight — e.g. soften the body to "this run started no work", or skip the PATCH when an address run is live for this PR (the scan step already queries check/run state elsewhere for 中文说明blocked 状态的 PATCH 直接写入唯一的 建议修复:限定文案范围或在有轮次在跑时跳过写入——例如把文案软化为 "this run started no work",或在该 PR 存在进行中的 address 运行时跳过 PATCH(scan 步骤已在别处为 — qwen3.8-max via Qwen Code /review (v0.21.4) |
||||||||||||||
| for attempt in 1 2 3; do | ||||||||||||||
|
Comment on lines
+2079
to
+2081
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] R1-2: Still stands — escalated from Suggestion on this round's verification. The blocked-status reporter PATCHes the newest shared Suggested fix: gate the blocked-status write on round state — skip the PATCH when the matched comment body is the in-flight round's live announcement (post a separate comment instead), or hoist the 中文说明[Critical] R1-2:仍然存在——经本轮验证,由 Suggestion 升级为 Critical。blocked 状态报告函数会 PATCH 最新的共享 建议修复:按轮次状态为 blocked 写入设门——当匹配到的评论正文是进行中轮次的实时公告时跳过 PATCH(改为另发一条评论),或把 — qwen3.8-max via Qwen Code /review (v0.21.4)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 已修复:forced 验证证据:聚焦并发契约回归 1/1 通过;完整 |
||||||||||||||
| if gh api --method PATCH "repos/${REPO}/issues/comments/${status_id}" -f body="${body}" > /dev/null; then | ||||||||||||||
| return 0 | ||||||||||||||
| fi | ||||||||||||||
| echo "::warning::Failed to update blocked takeover status for #${FORCED_PR} (attempt ${attempt}/3)" >&2 | ||||||||||||||
| [[ "${attempt}" -lt 3 ]] && sleep "${attempt}" | ||||||||||||||
| done | ||||||||||||||
| else | ||||||||||||||
| for attempt in 1 2 3; do | ||||||||||||||
| if gh pr comment "${FORCED_PR}" --repo "${REPO}" --body "${body}" > /dev/null; then | ||||||||||||||
| return 0 | ||||||||||||||
|
Comment on lines
+2089
to
+2091
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Suggested fix: cheapest consistent option — make the scan-side copy semantically identical to the announcement step (same 中文说明[Suggestion] 建议修复:最便宜的一致性选项——让扫描侧副本与公告步骤语义完全一致(同样的 — qwen3.8-max via Qwen Code /review (v0.21.4) |
||||||||||||||
| fi | ||||||||||||||
| echo "::warning::Failed to post blocked takeover status for #${FORCED_PR} (attempt ${attempt}/3)" >&2 | ||||||||||||||
| [[ "${attempt}" -lt 3 ]] && sleep "${attempt}" | ||||||||||||||
| done | ||||||||||||||
| fi | ||||||||||||||
| return 1 | ||||||||||||||
| } | ||||||||||||||
|
Comment on lines
+2096
to
+2098
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R3-1: Still stands — the reporter's terminal WRITE-failure fall-through (this 中文说明仍然存在——报告函数的终态写入失败 fall-through(PATCH 循环和新建评论循环都耗尽 3 次后的这个 — qwen3.8-max via Qwen Code /review (v0.21.5) |
||||||||||||||
|
|
||||||||||||||
| # Candidate PRs: open, same-repo, targeting main, and either | ||||||||||||||
| # authored by the dev-bot or opted in via TAKEOVER_LABEL. A PR | ||||||||||||||
| # carrying SKIP_LABEL is excluded everywhere — skip wins over | ||||||||||||||
|
|
@@ -1916,8 +2106,12 @@ jobs: | |||||||||||||
| # jq's // treats false as empty, so that form is false for EVERY | ||||||||||||||
| # input and silently green-no-op'd all forced dispatches. | ||||||||||||||
| if [[ -n "${FORCED_PR}" ]]; then | ||||||||||||||
| META="$(gh pr view "${FORCED_PR}" --repo "${REPO}" \ | ||||||||||||||
| --json number,state,author,headRefName,isCrossRepository,baseRefName,labels,maintainerCanModify 2> /dev/null || echo '{}')" | ||||||||||||||
| if ! META="$(read_forced_pr_meta)"; then | ||||||||||||||
| echo "::error::Forced PR #${FORCED_PR} admission blocked: metadata_fetch_failed" | ||||||||||||||
|
Comment on lines
+2109
to
+2110
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Suggested fix: mirror the terminal/transient split proposed for 中文说明
建议修复:与 — qwen3.8-max via Qwen Code /review (v0.21.5) |
||||||||||||||
| echo "targets=[]" >> "${GITHUB_OUTPUT}" | ||||||||||||||
| echo "has_targets=false" >> "${GITHUB_OUTPUT}" | ||||||||||||||
| exit 1 | ||||||||||||||
| fi | ||||||||||||||
| # Same admission as the scheduled scan below. In-repo PRs fail | ||||||||||||||
| # CLOSED on a missing isCrossRepository field (`.isCrossRepository | ||||||||||||||
| # == false`, never a `// true | not` default — jq's // treats false | ||||||||||||||
|
|
@@ -1927,34 +2121,40 @@ jobs: | |||||||||||||
| # gate runs in the shell case just below, mirroring the scan's | ||||||||||||||
| # per-candidate permission call) so the real-time route's fork | ||||||||||||||
| # pickup is not silently discarded here. | ||||||||||||||
| OK="$(jq -r --arg ab "${AUTOFIX_BOT}" --arg take "${TAKEOVER_LABEL}" --arg skip "${SKIP_LABEL}" \ | ||||||||||||||
| '(((.state // "") == "OPEN") | ||||||||||||||
| and (((.author.login // "") == $ab) or ([.labels[]?.name] | index($take) != null)) | ||||||||||||||
| and ([.labels[]?.name] | index($skip) | not) | ||||||||||||||
| and ((.baseRefName // "") == "main") | ||||||||||||||
| and (if (.isCrossRepository == true) | ||||||||||||||
| then (.maintainerCanModify == true) | ||||||||||||||
| else (.isCrossRepository == false) | ||||||||||||||
| end))' <<< "${META}")" | ||||||||||||||
| ADMISSION_REASON="$(forced_admission_reason <<< "${META}")" | ||||||||||||||
| # Fork only: the author must hold write+ RIGHT NOW (the same | ||||||||||||||
| # live-privilege rule the scan applies per candidate and | ||||||||||||||
| # review-address re-checks before pushing). In-repo PRs are gated | ||||||||||||||
| # by author/label alone. | ||||||||||||||
| if [[ "${OK}" == 'true' && "$(jq -r '.isCrossRepository == true' <<< "${META}")" == 'true' ]]; then | ||||||||||||||
| if [[ "${ADMISSION_REASON}" == 'eligible' && "$(jq -r '.isCrossRepository == true' <<< "${META}")" == 'true' ]]; then | ||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Test gap (pattern, instance 4/4): the Suggested fix: extend the existing forced-target test to replay this gate — assert that an eligible in-repo meta never reaches the permission call and an eligible cross-repo meta does (a small bash replay with a stubbed 中文说明测试缺口(模式,第 4/4 处): 建议修复:扩展现有 forced-target 测试以重放该门——断言合法的仓库内 meta 永远不会到达权限调用、而合法的跨仓库 meta 会到达(用记录调用的桩 — qwen3.8-max-preview via Qwen Code /review (v0.21.4) |
||||||||||||||
| FORK_AUTHOR="$(jq -r '.author.login // ""' <<< "${META}")" | ||||||||||||||
| FPERM="$(gh api "repos/${REPO}/collaborators/${FORK_AUTHOR}/permission" --jq '.permission // ""' 2> /dev/null || echo '')" | ||||||||||||||
| if ! FPERM="$(read_live_permission "${FORK_AUTHOR}")"; then | ||||||||||||||
| ADMISSION_REASON='permission_lookup_failed' | ||||||||||||||
| report_forced_takeover_blocked "${ADMISSION_REASON}" \ | ||||||||||||||
|
Comment on lines
+2132
to
+2133
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R1-9: The exit-code contract for terminal blockers is asymmetric and undocumented in the code. Still stands at this commit (re-checked): Suggested fix: document the contract where the paths diverge — lookup failures fail loud (exit 1) because visibility could not be guaranteed, while deterministic rejections exit 0 once the blocked status is posted or deliberately skipped. 中文说明终态阻塞的退出码契约不对称且代码中无文档(本轮复查仍成立): 建议修复:在路径分叉处补充注释说明契约——查询失败因无法保证可见性而显式失败(exit 1),确定性拒绝在 blocked 状态成功发布或有意跳过时 exit 0。 — qwen3.8-max via Qwen Code /review (v0.21.4) |
||||||||||||||
| || echo "::error::Forced PR #${FORCED_PR} blocked status update failed" | ||||||||||||||
|
Comment on lines
+2132
to
+2134
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The exit-code contract for terminal blockers is asymmetric and undocumented: Suggested fix: add a one-line comment at each 中文说明终态阻塞的退出码契约不对称且无文档: 建议修复:在每个 — qwen3.8-max-preview via Qwen Code /review (v0.21.4) |
||||||||||||||
| echo "::error::Forced PR #${FORCED_PR} admission blocked: ${ADMISSION_REASON}" | ||||||||||||||
|
Comment on lines
+2133
to
+2135
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R1-9: Still stands — the exit-code contract for terminal blockers is asymmetric and undocumented in the code. Suggested fix: add a short comment above the forced-admission block stating the contract: lookup failures exit 1 unconditionally (transient, must alert); deterministic rejections exit 0 once the blocked status is posted (terminal, reported); report failure exits 1 (the blocker could not be made visible). 中文说明[Suggestion] R1-9:仍然存在——终态阻塞的退出码契约不对称,且代码中没有任何文档。 建议修复:在强制准入块上方加一条简短注释说明契约:查询失败无条件 exit 1(瞬时错误,必须告警);确定性拒绝在 blocked 状态发出后 exit 0(终态,已报告);报告失败 exit 1(阻塞原因无法被公开)。 — qwen3.8-max via Qwen Code /review (v0.21.4) |
||||||||||||||
| echo "targets=[]" >> "${GITHUB_OUTPUT}" | ||||||||||||||
| echo "has_targets=false" >> "${GITHUB_OUTPUT}" | ||||||||||||||
| exit 1 | ||||||||||||||
| fi | ||||||||||||||
| case "${FPERM}" in | ||||||||||||||
| admin|maintain|write) | ||||||||||||||
| echo "🌿 forced fork PR #${FORCED_PR} admitted (author ${FORK_AUTHOR}=${FPERM})" | ||||||||||||||
| ;; | ||||||||||||||
| *) | ||||||||||||||
| echo "🧭 forced fork PR #${FORCED_PR} rejected: author ${FORK_AUTHOR} permission='${FPERM:-none}' below write" | ||||||||||||||
| OK='false' | ||||||||||||||
| ADMISSION_REASON="author_permission_${FPERM:-none}" | ||||||||||||||
| echo "🧭 forced fork PR #${FORCED_PR} rejected: ${ADMISSION_REASON}" | ||||||||||||||
| ;; | ||||||||||||||
| esac | ||||||||||||||
| fi | ||||||||||||||
| if [[ "${OK}" != "true" ]]; then | ||||||||||||||
| echo "❌ #${FORCED_PR} is not an open main-targeting PR owned by ${AUTOFIX_BOT} or labeled ${TAKEOVER_LABEL} (or it carries ${SKIP_LABEL}); a fork PR additionally needs maintainer edits allowed and a live write+ author" | ||||||||||||||
| if [[ "${ADMISSION_REASON}" != 'eligible' ]]; then | ||||||||||||||
| if ! report_forced_takeover_blocked "${ADMISSION_REASON}"; then | ||||||||||||||
| echo "::error::Forced PR #${FORCED_PR} blocked status update failed" | ||||||||||||||
|
Comment on lines
+2151
to
+2152
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] On the report-failure exit, the admission reason is never logged for Suggested fix: emit the reason in the failure branch too, e.g. 中文说明[Suggestion] 在报告失败的退出路径上, 建议修复:失败分支也输出原因,例如在报告调用之前(或其失败分支内)加 — qwen3.8-max via Qwen Code /review (v0.21.4) |
||||||||||||||
| echo "targets=[]" >> "${GITHUB_OUTPUT}" | ||||||||||||||
| echo "has_targets=false" >> "${GITHUB_OUTPUT}" | ||||||||||||||
| exit 1 | ||||||||||||||
| fi | ||||||||||||||
| echo "❌ Forced PR #${FORCED_PR} rejected: ${ADMISSION_REASON}" | ||||||||||||||
| echo "targets=[]" >> "${GITHUB_OUTPUT}" | ||||||||||||||
| echo "has_targets=false" >> "${GITHUB_OUTPUT}" | ||||||||||||||
| exit 0 | ||||||||||||||
|
|
@@ -2000,14 +2200,19 @@ jobs: | |||||||||||||
| # candidates alone exhaust the inspection budget. | ||||||||||||||
| while IFS=$'\t' read -r FPR FAUTHOR; do | ||||||||||||||
| [[ -z "${FPR}" ]] && continue | ||||||||||||||
| FPERM="$(gh api "repos/${REPO}/collaborators/${FAUTHOR}/permission" --jq '.permission // ""' 2> /dev/null || echo '')" | ||||||||||||||
| if ! FPERM="$(read_live_permission "${FAUTHOR}")"; then | ||||||||||||||
| echo "::warning::Fork takeover candidate #${FPR} blocked: permission_lookup_failed" | ||||||||||||||
| fleet_row "${FPR}" 'blocked' 'permission_lookup_failed' | ||||||||||||||
|
Comment on lines
+2203
to
+2205
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Test gap (pattern, instance 3/4): this new scan-loop blocked branch has no test — deleting the entire five-line block keeps all 110 tests green (the generic Suggested fix: add a static pin next to the existing fleet-row assertions (e.g. 中文说明测试缺口(模式,第 3/4 处):这个新的扫描循环 blocked 分支没有测试——删除整个五行代码块后全部 110 个测试仍全绿(通用的 建议修复:在现有 fleet-row 断言旁增加静态钉住(例如 — qwen3.8-max-preview via Qwen Code /review (v0.21.4)
Comment on lines
+2203
to
+2205
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] For Suggested fix: mirror the lookup-failure branch in the below-write 中文说明对 建议修复:在 below-write 的 — qwen3.8-max-preview via Qwen Code /review (v0.21.4)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 已修复 + 验证证据:定时扫描在 fork 作者权限低于 write 时现在写入 |
||||||||||||||
| continue | ||||||||||||||
| fi | ||||||||||||||
| case "${FPERM}" in | ||||||||||||||
| admin|maintain|write) | ||||||||||||||
| echo "🌿 fork takeover candidate #${FPR} admitted (author ${FAUTHOR}=${FPERM})" | ||||||||||||||
| CANDIDATES="${CANDIDATES} ${FPR}" | ||||||||||||||
| ;; | ||||||||||||||
| *) | ||||||||||||||
| echo "🧭 fork takeover candidate #${FPR} skipped: author ${FAUTHOR} permission='${FPERM:-none}' below write" | ||||||||||||||
| fleet_row "${FPR}" 'blocked' "author_permission_${FPERM:-none}" | ||||||||||||||
| ;; | ||||||||||||||
| esac | ||||||||||||||
| done < <(jq -rs --arg skip "${SKIP_LABEL}" ' | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] This concurrency group is broader than the job's own
if:condition — GitHub evaluates concurrency BEFORE the jobif, so runs that will only skip still occupy the shared per-PR slot.routeemitspr_numberunconditionally from the dispatch input (viasanitize_number), so aworkflow_dispatchwithphase: issueandpr_number: Nresolves the group toqwen-pr-head-write-Neven thoughdo_reviewis false and this job will skip.Failure scenario:
workflow_dispatchwithphase: issue+pr_number: Nwhile an address round for PR N is in flight → the skipped review-scan queues onqwen-pr-head-write-Nbehind the address leg (timeout-minutes: 300,cancel-in-progress: false);issue-autofixdeclaresneeds: ['route', 'review-scan'], so the issue phase the operator actually dispatched idles behind an unrelated address round it never interacts with — with only a "queued" badge as explanation. Nothing is cancelled (it self-recovers); the cost is a bounded but unexplained stall of up to the address round's length. The repo's ownqwen-triage-workflow.test.jspins exactly this trap: "a predicate that is broader than the job's own condition lets a run that will skip take the shared per-PR slot".中文说明
[Suggestion] 这个 concurrency group 比 job 自身的
if:条件更宽——GitHub 在 jobif之前评估 concurrency,因此只会跳过的运行也会占用共享的 per-PR 锁槽。route会无条件输出 dispatch 输入中的pr_number(经sanitize_number),所以phase: issue+pr_number: N的workflow_dispatch仍会把 group 解析为qwen-pr-head-write-N——即使do_review为 false、本 job 只会跳过。失败场景:PR N 正有一轮 address 在运行时,以
phase: issue+pr_number: N触发 workflow_dispatch → 被跳过的 review-scan 会在qwen-pr-head-write-N上排队,等在该 address 分片之后(timeout-minutes: 300、cancel-in-progress: false);由于issue-autofix声明needs: ['route', 'review-scan'],操作员实际派发的 issue 阶段会闲置在一个与之无关、永不会交互的 address 轮次后面——唯一的提示是 "queued" 标记。不会取消(可自愈);代价是最长可达 address 轮次时长的无解释等待。本仓库自己的qwen-triage-workflow.test.js恰好钉住了这个陷阱:"比 job 自身条件更宽的谓词会让将要跳过的运行占用共享的 per-PR 槽位"。建议修复:把 group 收敛到 job 的真实条件(见 suggestion 块),并同步更新 'serializes forced status writes with the matching address job' 中的断言。
— qwen3.8-max via Qwen Code /review (v0.21.6)