Repository navigation
feat(review): skip the automatic round when a synchronize only refreshes the base #10116
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 6 commits
8b8dc72
88fe914
fad703a
a7e8811
5ac05ba
21cce5f
3d449b8
90c99a7
4cf0f3a
a81269d
a15bd5c
64fffbf
422a5e7
402bb22
cba5231
6e77b50
cf5dc5d
cad0cf0
16b58ab
aae221b
c7499d5
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 |
|---|---|---|
| @@ -0,0 +1,149 @@ | ||
| #!/usr/bin/env bash | ||
| # Decide whether an automatic `synchronize` review round may be skipped | ||
| # because the push only refreshed the base branch (#10111): an update-branch | ||
| # merge fires `synchronize` like any push, but a full automatic round costs | ||
| # hours for a head whose PR-side diff is unchanged. | ||
| # | ||
| # Two facts are certified before any review compute is spent: every | ||
| # first-parent commit since the head the last completed automatic round | ||
| # reviewed is a two-parent merge of the base branch, and the PR's own | ||
| # three-dot diff kept the same canonical digest — every changed and | ||
| # context byte is hashed after stripping only unstable diff metadata | ||
| # (index lines, hunk offsets), so an upstream edit touching the PR's own | ||
| # hunks (whitespace included) fails the equality and the full round runs. | ||
| # The reviewed head is the newest SUBMITTED ledger-marked review | ||
| # (APPROVED, CHANGES_REQUESTED, or COMMENTED, with a submitted_at) posted | ||
| # by the AUTHENTICATED account — resolved live via `gh api user`, same | ||
| # norm as the fallback dedup — so a participant posting the marker text | ||
| # in their own review, a PENDING draft, or a DISMISSED review can never | ||
| # certify a head this gate would skip. | ||
| # | ||
| # Fail-open contract: every probe error or unmatched shape records | ||
| # skip=false and the full round runs, and the script never exits non-zero — | ||
| # the caller treats a step failure as a pipeline failure worth a fallback | ||
| # comment, which a skipped gate is not. | ||
| # | ||
| # Outputs (GITHUB_OUTPUT): skip=true|false, reviewed_sha, reason. | ||
| set -uo pipefail | ||
|
|
||
| # Required environment, restated so a misconfigured caller dies naming the | ||
| # missing input instead of probing git with empty arguments. | ||
| PR_NUMBER="${PR_NUMBER:?}" | ||
| EVENT_HEAD_SHA="${EVENT_HEAD_SHA:?}" | ||
| BASE_REF="${BASE_REF:?}" | ||
| RUN_URL="${RUN_URL:?}" | ||
| GITHUB_REPOSITORY="${GITHUB_REPOSITORY:?}" | ||
| GITHUB_OUTPUT="${GITHUB_OUTPUT:?}" | ||
| GITHUB_STEP_SUMMARY="${GITHUB_STEP_SUMMARY:?}" | ||
|
|
||
| SKIP=false | ||
| REVIEWED_SHA='' | ||
| REASON='' | ||
|
|
||
| # Canonical digest of the diff between two commits: every changed and | ||
| # context byte is hashed, stripping only unstable metadata (index lines, | ||
| # hunk offsets). `git patch-id` is whitespace-insensitive and would | ||
| # certify an indentation-only rewrite of PR-owned lines. | ||
| diff_digest() { | ||
| local diff | ||
| diff="$(git diff "$1" "$2" 2>/dev/null)" || return 1 | ||
| [[ -n "${diff}" ]] || return 0 | ||
| printf '%s\n' "${diff}" | | ||
| sed -E '/^index [0-9a-f]+\.\.[0-9a-f]+/d; /^@@ /d' | | ||
| git hash-object --stdin | ||
| } | ||
|
|
||
| decide() { | ||
| REASON='pr head fetch failed' | ||
| git fetch --no-tags --quiet origin "refs/pull/${PR_NUMBER}/head" || return | ||
|
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. Deferred to the next round on round-budget grounds — not disagreement; the finding is verified real (same disposition as this finding's sibling threads). The live-today entrances reproduce; the structural fix (workspace-wide sanitization/hardened wrapper: config isolation, hooks/fsmonitor disabling, 中文说明按轮次预算延后至下一轮——并非不同意;该发现已核实为真(与本发现同族线程同一处置)。当前即为活通道的入口均复现;结构性修复(全工作区净化/加固包装:配置隔离、禁用 hooks/fsmonitor、钉住
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. Deferred to the next round on round-budget grounds — not disagreement; the finding is verified real (same disposition as this finding's sibling threads). The insteadOf+sshCommand, grafts, and diff.context entrances reproduce as described; the structural closure this finding asks for is a dedicated work item exceeding this round's budget. This round ships R5-3/R7-1/R3-3/R5-2 with red-before witnesses; the hardening round follows with one executed sentinel per channel. 中文说明按轮次预算延后至下一轮——并非不同意;该发现已核实为真(与本发现同族线程同一处置)。insteadOf+sshCommand、grafts、diff.context 入口按描述复现;本发现要求的结构性封闭是一项超出本轮预算的专门工作。本轮落地带"修复前为红"见证的 R5-3/R7-1/R3-3/R5-2;加固轮随后进行,并为每个通道提供一个已执行哨兵。 |
||
| REASON='event head not reachable from the PR head ref' | ||
| git cat-file -e "${EVENT_HEAD_SHA}^{commit}" 2>/dev/null || return | ||
| REASON='base branch ref unavailable' | ||
| if ! git rev-parse -q --verify "origin/${BASE_REF}^{commit}" >/dev/null; then | ||
| git fetch --no-tags --quiet origin "${BASE_REF}" || return | ||
|
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. Deferred to the next round on round-budget grounds — not disagreement; the finding is verified real (same disposition as this finding's sibling threads). The 中文说明按轮次预算延后至下一轮——并非不同意;该发现已核实为真(与本发现同族线程同一处置)。
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. Deferred to the next round on round-budget grounds — not disagreement; the finding is verified real (same disposition as this finding's sibling threads). Exactly three channels are hardened today, and the stale- 中文说明按轮次预算延后至下一轮——并非不同意;该发现已核实为真(与本发现同族线程同一处置)。今天恰有三个通道被加固,过期的
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. Deferred to the next round on round-budget grounds — not disagreement; the finding is verified real (same disposition as this finding's sibling threads). The grafts entrance reproduces as described at round 11 (re-parented walk + falsified merge-base → 中文说明按轮次预算延后至下一轮——并非不同意;该发现已核实为真(与本发现同族线程同一处置)。grafts 入口按描述在第 11 轮复现(重排父系的行走 + 被伪造的 merge-base → |
||
| git rev-parse -q --verify "origin/${BASE_REF}^{commit}" >/dev/null || return | ||
|
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. Deferred to the next round on round-budget grounds — not disagreement; the finding is verified real (same disposition as this finding's sibling threads). The 中文说明按轮次预算延后至下一轮——并非不同意;该发现已核实为真(与本发现同族线程同一处置)。 |
||
| fi | ||
|
|
||
| REASON='reviewed-head lookup failed' | ||
| local bot_login reviews | ||
| bot_login="$(gh api user --jq '.login')" && [[ -n "${bot_login}" ]] || return | ||
| reviews="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/reviews" --method GET --paginate -F per_page=100)" || return | ||
| REVIEWED_SHA="$( | ||
| printf '%s' "${reviews}" | jq -sr --arg login "${bot_login}" ' | ||
| [.[][] | ||
| | select(.user.login == $login) | ||
| | select(.state == "APPROVED" or .state == "CHANGES_REQUESTED" | ||
| or .state == "COMMENTED") | ||
| | select(.submitted_at != null) | ||
| | select(.body | contains("<!-- qwen-review-ledger"))] | ||
|
yiliang114 marked this conversation as resolved.
Resolved
|
||
| | last | .commit_id // empty' | ||
| )" || return | ||
| REASON='no completed automatic round on this PR' | ||
| [[ -n "${REVIEWED_SHA}" ]] || return | ||
| REASON='reviewed head not in the fetched history' | ||
| git cat-file -e "${REVIEWED_SHA}^{commit}" 2>/dev/null || return | ||
| REASON='reviewed head is not an ancestor of the pushed head' | ||
| git merge-base --is-ancestor "${REVIEWED_SHA}" "${EVENT_HEAD_SHA}" 2>/dev/null || return | ||
|
|
||
| # Bounded walk: a stale reviewed head must not buy an unbounded ancestry | ||
| # scan, and past ten first-parent steps this is not the cheap shape the | ||
| # gate exists for anyway. | ||
| local count c p2 | ||
| REASON='commit walk failed' | ||
| count="$(git rev-list --first-parent --count "${REVIEWED_SHA}..${EVENT_HEAD_SHA}" 2>/dev/null)" || return | ||
| [[ "${count}" =~ ^[0-9]+$ ]] || return | ||
| REASON='no new commits since the reviewed head' | ||
| [[ "${count}" -ge 1 ]] || return | ||
| REASON="too many commits since the reviewed head (${count})" | ||
| [[ "${count}" -le 10 ]] || return | ||
| for c in $(git rev-list --first-parent "${REVIEWED_SHA}..${EVENT_HEAD_SHA}" 2>/dev/null); do | ||
| REASON="non-merge commit since the reviewed head: ${c}" | ||
| p2="$(git rev-parse -q --verify "${c}^2" 2>/dev/null)" || return | ||
| REASON="octopus merge since the reviewed head: ${c}" | ||
| if git rev-parse -q --verify "${c}^3" >/dev/null 2>&1; then return; fi | ||
|
yiliang114 marked this conversation as resolved.
Resolved
|
||
| REASON="merge of a non-base branch since the reviewed head: ${c}" | ||
| git merge-base --is-ancestor "${p2}" "origin/${BASE_REF}" 2>/dev/null || return | ||
|
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. Deferred to the follow-up queue — verified real, pairs with R2-4 (same disposition as this finding's sibling threads). The rewrite arm reproduces 中文说明延后至后续队列——已核实为真,与 R2-4 配对(与本发现同族线程同一处置)。重写分支在线上脚本上复现
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. Deferred to the follow-up queue — verified real, pairs with R2-4 (same disposition as this finding's sibling threads). No gate-only fix exists; the reviewed base commit must ride the marker, and the writer change lives outside this PR's footprint ( 中文说明延后至后续队列——已核实为真,与 R2-4 配对(与本发现同族线程同一处置)。不存在仅门侧的修复;已评审的 base 提交必须由标记携带,而写入端改动位于本 PR 足迹之外(
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. Deferred to the follow-up queue — verified real, pairs with R2-4 (same disposition as this finding's sibling threads). The rewritten-base probe reproduces 中文说明延后至后续队列——已核实为真,与 R2-4 配对(与本发现同族线程同一处置)。重写 base 探针在本 head 复现 |
||
| done | ||
|
|
||
| local mb_r mb_h digest_r digest_h | ||
| REASON='merge-base resolution failed' | ||
| mb_r="$(git merge-base "origin/${BASE_REF}" "${REVIEWED_SHA}" 2>/dev/null)" || return | ||
|
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. Deferred to the follow-up queue — verified real, pairs with R2-4 (same disposition as this finding's sibling threads). No gate-only fix exists: the reviewed base COMMIT must ride the ledger marker, and that writer change lives in 中文说明延后至后续队列——已核实为真,与 R2-4 配对(与本发现同族线程同一处置)。不存在仅门侧的修复:已评审的 base 提交必须由台账标记携带,而该写入端改动位于
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. Deferred to the follow-up queue — verified real, pairs with R2-4 (same disposition as this finding's sibling threads). Binding the certification to the base's identity requires the writer to record the reviewed base tip beside 中文说明延后至后续队列——已核实为真,与 R2-4 配对(与本发现同族线程同一处置)。把认证绑定到 base 的身份,需要写入端在
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. Deferred to the follow-up queue — verified real, pairs with R2-4 (same disposition as this finding's sibling threads). The force-push rewrite probe reproduces 中文说明延后至后续队列——已核实为真,与 R2-4 配对(与本发现同族线程同一处置)。强推重写探针在本 head 复现
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. Deferred to the follow-up queue — verified real, pairs with R2-4 (same disposition as this finding's sibling threads). The force-push rewrite probe still reproduces 中文说明延后至后续队列——已核实为真,与 R2-4 配对(与本发现同族线程同一处置)。强推重写探针在本 head 仍复现
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. Stays deferred to the follow-up queue (first recorded in round 12). The finding is verified real — a same-name base rewrite does slip through the ref-name-only certification while digest equality holds by construction — but its fix straddles this PR's footprint: recording the reviewed base COMMIT in the ledger marker is writer-side work in 中文说明继续保持延后处理,已记录至后续队列(第 12 轮首次记录)。该发现已核实为真——同名 base 分支被重写确实能穿过仅认分支名的认证,且摘要等式按构造成立——但其修复跨越本 PR 的足迹:在台账标记中记录该轮评审所对的 base 提交是
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. Stays deferred to the follow-up queue (first recorded in round 12). Same disposition as the round-12 thread: verified real, but the fix requires the writer-side base-commit field in 中文说明继续保持延后处理,已记录至后续队列(第 12 轮首次记录)。与第 12 轮线程处置相同:已核实为真,但修复需要 |
||
| mb_h="$(git merge-base "origin/${BASE_REF}" "${EVENT_HEAD_SHA}" 2>/dev/null)" || return | ||
|
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. Gate half fixed in Fixed here. The lookup now parses the marker payload and admits a marker only when its recorded Deferred. Emitting the reviewed round's 中文说明门侧一半已在 本次已修复:查询现在解析标记载荷,仅当标记记录的 已延后:在台账标记载荷中输出该轮评审所用的 |
||
| REASON='PR-side diff computation failed' | ||
| digest_r="$(diff_digest "${mb_r}" "${REVIEWED_SHA}")" || return | ||
| digest_h="$(diff_digest "${mb_h}" "${EVENT_HEAD_SHA}")" || return | ||
|
Comment on lines
+237
to
+238
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. Deferred to the follow-up queue — verified real, pairs with R2-4. The probe shape reproduces: after a same-named base rewrite, the landed three-dot diff gains content the reviewed round never saw, and the gate still emits 中文说明延后至后续队列——已核实为真,与 R2-4 配对。探针形态复现:同名 base 被重写后,落地的三点 diff 新增了已评审轮从未见过的内容,而门仍输出 |
||
| REASON='empty PR-side diff' | ||
| { [[ -n "${digest_r}" ]] && [[ -n "${digest_h}" ]]; } || return | ||
| REASON='the PR-side diff changed' | ||
| [[ "${digest_r}" = "${digest_h}" ]] || return | ||
|
|
||
| REASON='' | ||
| SKIP=true | ||
| } | ||
| decide | ||
|
|
||
| { | ||
| echo "skip=${SKIP}" | ||
| echo "reviewed_sha=${REVIEWED_SHA}" | ||
| echo "reason=${REASON}" | ||
| } >> "${GITHUB_OUTPUT}" | ||
| if [[ "${SKIP}" != 'true' ]]; then | ||
| echo "Base-refresh gate: full round proceeds (${REASON})." >> "${GITHUB_STEP_SUMMARY}" | ||
| exit 0 | ||
| fi | ||
| echo "Base-refresh gate: head ${EVENT_HEAD_SHA} only merges ${BASE_REF} into reviewed head ${REVIEWED_SHA}; skipping the automatic round." >> "${GITHUB_STEP_SUMMARY}" | ||
|
|
||
| # One marker-deduped note per PR, updated in place via the shared upsert | ||
| # helper (author-scoped lookup, retried). Blank line after the marker, or | ||
| # none of the prose renders — the HTML-block quirk the ack comment documents. | ||
| # Note failures only warn: the skip decision is already recorded above. | ||
| body_file="$(mktemp "${RUNNER_TEMP:-/tmp}/qwen-review-base-refresh.XXXXXX")" || exit 0 | ||
| # shellcheck disable=SC2016 | ||
| printf '<!-- qwen-review-base-refresh -->\n\n🔁 **Base refresh detected** — head `%s` only merges `%s` into the last reviewed head `%s`; the PR-side diff is unchanged, so that review still applies and no new automatic round was spent ([this run](%s)). Comment `@qwen-code /review` for a fresh full review.\n\n<details>\n<summary>中文说明</summary>\n\n🔁 **检测到 base 刷新** —— head `%s` 仅把 `%s` 合入上次已评审的 head `%s`,PR 侧 diff 未变,原有评审结论仍然适用,本次未消耗新的自动评审轮次([本次运行](%s))。如需全新完整评审,请评论 `@qwen-code /review`。\n\n</details>' \ | ||
| "${EVENT_HEAD_SHA}" "${BASE_REF}" "${REVIEWED_SHA}" "${RUN_URL}" \ | ||
| "${EVENT_HEAD_SHA}" "${BASE_REF}" "${REVIEWED_SHA}" "${RUN_URL}" > "${body_file}" | ||
| "$(dirname "${BASH_SOURCE[0]}")/upsert-bot-comment.sh" \ | ||
|
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. Stays deferred to the follow-up queue (first recorded in round 12). Verified again at this head: 中文说明继续保持延后处理,已记录至后续队列(第 12 轮首次记录)。已在本 head 上再次核实:
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. Stays deferred to the follow-up queue (first recorded in round 12). Disposition unchanged from the round-12 thread: the defect is verified (zero occurrences of the marker in 中文说明继续保持延后处理,已记录至后续队列(第 12 轮首次记录)。处置与第 12 轮线程相同:缺陷已核实(本 head 上该标记在
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. Stays deferred to the follow-up queue (first recorded in round 12). Verified at this head as in rounds 12–13: the marker appears 0 times in 中文说明继续保持延后处理,已记录至后续队列(第 12 轮首次记录)。本 head 上的核实结果与第 12–13 轮一致:该标记在 |
||
| "${GITHUB_REPOSITORY}" "${PR_NUMBER}" '<!-- qwen-review-base-refresh -->' "${body_file}" \ | ||
|
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. Deferred to the follow-up queue — verified real. 中文说明延后至后续队列——已核实为真。本 head 上 |
||
| || echo "::warning::could not post the base-refresh note" | ||
| rm -f "${body_file}" | ||
| exit 0 | ||
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.
Deferred to the next round on round-budget grounds — not disagreement; the finding is verified real (same disposition as this finding's sibling threads). The uploadpack entrance reproduces as described, and the structural closure (hardened wrapper/workspace sanitization with
--upload-pack=git-upload-packpinned, config isolation, grafts fail-open, per-channel sentinels) is a dedicated work item that exceeds this round's budget — this round ships four verified fixes (R5-3, R7-1, R3-3, R5-2) rather than a partial per-channel patch.中文说明
按轮次预算延后至下一轮——并非不同意;该发现已核实为真(与本发现同族线程同一处置)。uploadpack 入口按描述复现;结构性封闭(加固包装/工作区净化:钉住
--upload-pack=git-upload-pack、配置隔离、grafts 向开放侧倒、每通道哨兵)是一项超出本轮预算的专门工作——本轮已落地四项已验证修复(R5-3、R7-1、R3-3、R5-2),不做逐通道的局部修补。