Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 108 additions & 8 deletions scripts/dev-lead-fix-reviews.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ source "$(dirname "$0")/lib/review-change-evidence.sh"
source "$(dirname "$0")/lib/open-review-threads.sh"
source "$(dirname "$0")/lib/resolution-integrity.sh"
source "$(dirname "$0")/lib/addressed-claim-verify.sh"
# PR issue-comment disposition verifier (#1813): the issue-comment sibling of
# addressed-claim-verify.sh. Turns a dev-lead comment-disposition reply into a
# machine-checkable claim the harness verifies before minimizing the original
# comment RESOLVED. Sourced before claim-landing.sh, whose `fixed`-disposition
# sweep (#2156) reuses its parser.
source "$(dirname "$0")/lib/comment-disposition-verify.sh"
# Claim landing (#2013): "did the push land?" as a pure verdict, and retraction of
# this pass's claim replies whose commit did not land on the remote head.
source "$(dirname "$0")/lib/claim-landing.sh"
Expand All @@ -26,11 +32,6 @@ source "$(dirname "$0")/lib/test-tamper-guard.sh"
source "$(dirname "$0")/lib/sync-stub-guard.sh"
# Test-regression guard (#2013): a pass may not push with the suite newly red.
source "$(dirname "$0")/lib/test-regression-guard.sh"
# PR issue-comment disposition verifier (#1813): the issue-comment sibling of
# addressed-claim-verify.sh. Turns a dev-lead comment-disposition reply into a
# machine-checkable claim the harness verifies before minimizing the original
# comment RESOLVED.
source "$(dirname "$0")/lib/comment-disposition-verify.sh"
# Review-thread deferral verifier (#2045): the thread-side sibling of the
# `out-of-scope` disposition. A bot thread deferred to an open tracking issue that
# links it is resolved by resolve_deferred_bot_threads.
Expand Down Expand Up @@ -125,6 +126,93 @@ github_knows_commit() {
return 2
}

# sweep_fixed_dispositions <ref> <pass|earlier> — the issue-comment half of both
# claim sweeps (#2156). A `disposition=fixed` reply posted as a PR issue comment
# cites a commit just like a review-thread claim, but the sweeps listed only
# `pulls/{pr}/comments`, so a "fixed" reply whose commit never landed stayed up.
# Every such reply OUR account posted in the window (`pass`: since PASS_START_ISO,
# `earlier`: before it) whose cited commit is not on <ref> is rewritten with
# cl_retract_disposition_body. As for review-thread claims:
# - pass: a commit the push guard rebased onto a foreign commit is re-pointed
# at its landed successor instead (cl_map_sha, #2032);
# - earlier: a commit missing from a still-shallow checkout, one GitHub knows
# (pushed, then rewritten), or one GitHub cannot be asked about is
# left as is.
# Best-effort: returns 1 on a listing/PATCH failure; the caller folds it in.
sweep_fixed_dispositions() {
local ref="${1:-}" window="${2:-pass}"
local bot_user="${BOT_USER:-donpetry-bot}" comments rows
comments=$(gh api --paginate "repos/${REPO}/issues/${PR_NUMBER}/comments?per_page=100" 2>/dev/null | jq -s 'add // []' 2>/dev/null) || {
echo "::warning::sweep_fixed_dispositions: could not list issue comments on PR #${PR_NUMBER} — its ${window} \`fixed\` disposition replies were NOT verified (#2156)"
return 1
}
rows=$(cl_select_fixed_dispositions "$comments" "$bot_user" "${PASS_START_ISO:-}" "$window") || {
echo "::warning::sweep_fixed_dispositions: could not parse the issue comments on PR #${PR_NUMBER} — its ${window} \`fixed\` disposition replies were NOT verified (#2156)"
return 1
}
[ -z "$rows" ] && return 0

local id sha reason body new_body rebased known_rc deepened=false
local retracted=0 remapped=0 failed=0
while IFS=$'\t' read -r id sha; do
[ -z "$id" ] && continue
reason="unverifiable"
if [ -n "$sha" ]; then
if [ "$window" = "earlier" ] && ! git cat-file -e "${sha}^{commit}" 2>/dev/null; then
if [ "$deepened" = false ]; then
git_history_deepen "${BASE_REF:-main}"
deepened=true
fi
if ! git cat-file -e "${sha}^{commit}" 2>/dev/null \
&& [ "$(git rev-parse --is-shallow-repository 2>/dev/null || echo true)" != "false" ]; then
echo "::notice::sweep_fixed_dispositions: commit ${sha} of reply ${id} is not in this shallow checkout — left as is (#2156)"
continue
fi
fi
if [ -n "$ref" ] && git merge-base --is-ancestor "$sha" "$ref" 2>/dev/null; then
continue
fi
reason="not-on-ref"
if [ "$window" = "earlier" ]; then
known_rc=0
github_knows_commit "$sha" || known_rc=$?
if [ "$known_rc" -eq 0 ]; then
echo "::notice::sweep_fixed_dispositions: commit ${sha} of reply ${id} is known to GitHub but not on the remote head (pushed, then rewritten) — left as is (#2156)"
continue
elif [ "$known_rc" -ne 1 ]; then
echo "::notice::sweep_fixed_dispositions: could not ask GitHub about commit ${sha} of reply ${id} — left as is (#2156)"
continue
fi
fi
fi

body=$(printf '%s' "$comments" | jq -r --arg id "$id" 'first(.[] | select((.id | tostring) == $id)) | .body // ""' 2>/dev/null || echo "")
rebased=""
if [ "$window" = "pass" ] && [ "$reason" = "not-on-ref" ] && [ -n "$ref" ]; then
rebased=$(cl_map_sha "$sha" "${_PUSH_GUARD_REWRITES:-}") || rebased=""
fi
if [ -n "$rebased" ] && git merge-base --is-ancestor "$rebased" "$ref" 2>/dev/null \
&& new_body=$(cl_rewrite_disposition_sha "$body" "$rebased"); then
if gh api -X PATCH "repos/${REPO}/issues/comments/${id}" -f body="$new_body" >/dev/null 2>&1; then
remapped=$((remapped + 1))
echo "::notice::\`fixed\` reply ${id} on PR #${PR_NUMBER}: ${sha} was rebased onto a foreign commit and landed as ${rebased} — disposition updated, not retracted (#2156)"
continue
fi
echo "::warning::could not update \`fixed\` reply ${id} on PR #${PR_NUMBER} to the rebased commit ${rebased} — retracting it instead (#2156)"
fi
new_body=$(cl_retract_disposition_body "$body" "$reason")
if gh api -X PATCH "repos/${REPO}/issues/comments/${id}" -f body="$new_body" >/dev/null 2>&1; then
retracted=$((retracted + 1))
echo "::warning::retracted \`fixed\` disposition reply ${id} on PR #${PR_NUMBER} (${reason}; cited ${sha:-<none>}, remote head ${ref:-<unknown>}) (#2156)"
else
echo "::error::could not retract \`fixed\` disposition reply ${id} on PR #${PR_NUMBER} (${reason}) — a false 'fixed' reply may remain (#2156)"
failed=1
fi
done <<< "$rows"
echo "::notice::sweep_fixed_dispositions: retracted ${retracted} ${window} \`fixed\` disposition reply(ies) on PR #${PR_NUMBER} (${remapped} re-pointed at rebased commits) (#2156)"
[ "$failed" -eq 0 ]
}

# sweep_earlier_claims — the start-of-pass sweep over EARLIER passes' claim replies
# (#2032). retract_unlanded_claims runs only inside the pass that posted a reply, so
# a run cancelled or killed after its model replied (a concurrency cancel, a lost
Expand All @@ -135,6 +223,8 @@ github_knows_commit() {
# claimed files, or was committed before the comment it answers is retracted;
# - a retracted reply whose claim now passes is restored (cl_restore_body), so a
# wrongly retracted fix recovers without a new commit.
# Earlier `fixed` disposition replies on issue comments are swept too
# (sweep_fixed_dispositions, #2156).
# It never retracts on an unknown reference: when the remote head cannot be read,
# or a cited commit is missing from a checkout that is still shallow, it skips.
# Best-effort: returns 1 on a listing/PATCH failure; the caller does not stop.
Expand All @@ -158,6 +248,10 @@ sweep_earlier_claims() {
return 0
fi

# Earlier `fixed` disposition replies on the PR's issue comments (#2156).
local failed=0
sweep_fixed_dispositions "$ref" earlier || failed=1

local bot_user="${BOT_USER:-donpetry-bot}" comments rows
comments=$(gh api --paginate "repos/${REPO}/pulls/${PR_NUMBER}/comments?per_page=100" 2>/dev/null | jq -s 'add // []' 2>/dev/null) || {
echo "::warning::sweep_earlier_claims: could not list review comments on PR #${PR_NUMBER} — earlier claim replies were NOT verified (#2032)"
Expand All @@ -167,10 +261,10 @@ sweep_earlier_claims() {
echo "::warning::sweep_earlier_claims: could not parse the review comments on PR #${PR_NUMBER} — earlier claim replies were NOT verified (#2032)"
return 1
}
[ -z "$rows" ] && return 0
[ -z "$rows" ] && return "$failed"

local id state claim finding_at sha files facts on_ref touches commit_date own cumulative verdict
local body new_body deepened=false retracted=0 restored=0 failed=0 known_rc
local body new_body deepened=false retracted=0 restored=0 known_rc
while IFS=$'\x1f' read -r id state claim finding_at; do
[ -z "$id" ] && continue
sha=""
Expand Down Expand Up @@ -3247,6 +3341,8 @@ stamp_pass_replies() {
# posted since PASS_START_ISO, verify its commit is one this pass produced AND is
# on the REMOTE head (acv_claim_in_pass); otherwise rewrite it with
# cl_retract_body, which strips both markers so it can never resolve a thread.
# This pass's `fixed` disposition replies on issue comments are swept against the
# same reference head (sweep_fixed_dispositions, #2156).
#
# The reference head is the remote head. When there is no upstream (rc 1) on an
# `ok` outcome, the local HEAD is what was pushed; in every other case — including
Expand Down Expand Up @@ -3276,13 +3372,17 @@ retract_unlanded_claims() {
fi
fi

# This pass's `fixed` disposition replies on the PR's issue comments (#2156).
local failed=0
sweep_fixed_dispositions "$ref" pass || failed=1

local comments
comments=$(gh api --paginate "repos/${REPO}/pulls/${PR_NUMBER}/comments?per_page=100" 2>/dev/null | jq -s 'add // []' 2>/dev/null) || {
echo "::warning::retract_unlanded_claims: could not list review comments on PR #${PR_NUMBER} — this pass's claim replies were NOT verified (#2013)"
return 1
}

local rows retracted=0 failed=0 id sha facts on_ref in_base reason body new_body
local rows retracted=0 id sha facts on_ref in_base reason body new_body
rows=$(cl_select_pass_claims "$comments" "$bot_user" "$PASS_START_ISO") || {
echo "::warning::retract_unlanded_claims: could not parse the review comments on PR #${PR_NUMBER} — this pass's claim replies were NOT verified (#2013)"
return 1
Expand Down
106 changes: 106 additions & 0 deletions scripts/lib/claim-landing.sh
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@
# from earlier passes (a cancelled run never reached its own sweep, #2032) and
# cl_earlier_claim_verdict decides each: an unlanded one is retracted, and a
# retracted one whose fix is on the remote head is restored (cl_restore_body).
# - both sweeps also cover OUR PR issue-comment replies marked
# `disposition=fixed` (#2156): cl_select_fixed_dispositions picks them, and one
# whose cited commit is not on the remote head is rewritten with
# cl_retract_disposition_body (or re-pointed with cl_rewrite_disposition_sha
# when the push guard rebased it). Other dispositions cite no commit and are
# never selected.
#
# PURITY / TESTABILITY (ADR-0004)
# Every cl_* function except cl_remote_head is PURE: no network, no git, no gh.
Expand All @@ -42,6 +48,11 @@ if ! declare -F acv_parse_claim >/dev/null 2>&1; then
# shellcheck source=addressed-claim-verify.sh
source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/addressed-claim-verify.sh"
fi
# cdv_parse_disposition is the single disposition parser (#2156).
if ! declare -F cdv_parse_disposition >/dev/null 2>&1; then
# shellcheck source=comment-disposition-verify.sh
source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/comment-disposition-verify.sh"
fi

# The retraction marker. Matches the shared `<!-- dev-lead… -->` agent-marker
# discriminator, so a retracted reply is still recognised as ours, but it is NOT the
Expand Down Expand Up @@ -434,6 +445,101 @@ cl_earlier_claim_verdict() {
return 0
}

# cl_select_fixed_dispositions <comments_json> <bot_user> <since_iso> <pass|earlier>
# The issue-comment counterpart of the claim selectors (#2156). <comments_json> is
# the REST `issues/{pr}/comments` array ({id, user.login, body, created_at}).
# Emits one `<id>\t<sha>` row per comment that our account authored and that
# carries a `disposition=fixed` comment-disposition marker, created at or after
# <since_iso> (`pass`) or before it (`earlier`). <sha> is the cited commit, or
# empty when the marker does not parse (unverifiable -> the caller retracts).
# invalid / answered / informational / out-of-scope replies cite no commit and
# are never selected. An empty <since_iso> or a non-array payload selects
# nothing. Pure.
cl_select_fixed_dispositions() {
local comments_json="${1:-}" bot_user="${2:-}" since="${3:-}" window="${4:-pass}"
[[ -z "$since" ]] && return 0
local bot_stripped="${bot_user%\[bot\]}"

local rows
rows=$(jq -r \
--arg u "$bot_user" --arg us "$bot_stripped" --arg since "$since" --arg w "$window" \
--arg prefix "$_CDV_MARKER_PREFIX" '
if type == "array" then
.[] | objects
| (.user.login // "") as $l
| select($l == $u or $l == $us or $l == ($us + "[bot]"))
| (.created_at // "") as $at
| select(if $w == "earlier" then $at < $since else $at >= $since end)
| (.body // "") as $b
| select($b | contains($prefix))
| select($b | test("<!-- dev-lead:comment-disposition [^\\n]*disposition=fixed\\s"))
| [(.id | tostring), ($b | @base64)]
| @tsv
else empty end
' <<<"$comments_json" 2>/dev/null) || return 1
[[ -z "$rows" ]] && return 0

local id body_b64 body parsed sha
while IFS=$'\t' read -r id body_b64; do
[[ -z "$id" ]] && continue
body=$(base64 --decode <<<"$body_b64" 2>/dev/null || base64 -d <<<"$body_b64" 2>/dev/null || printf '')
sha=""
if parsed=$(cdv_parse_disposition "$body"); then
[[ "$(jq -r '.disposition // ""' <<<"$parsed" 2>/dev/null)" == "fixed" ]] || continue
sha=$(jq -r '.sha // ""' <<<"$parsed" 2>/dev/null || printf '')
fi
printf '%s\t%s\n' "$id" "$sha"
done <<<"$rows"
}

# cl_retract_disposition_body <body> <reason>
# The retracted form of a `fixed` disposition reply (#2156): every
# comment-disposition marker is removed (so the minimizer can never act on it), a
# plain-language retraction is prefixed, the original text is kept as a quote,
# and `<!-- dev-lead:retracted reason=<reason> -->` is appended so the reply is
# still recognised as ours. <reason> is restricted to [a-z-] (anything else
# becomes `unknown`). Pure.
cl_retract_disposition_body() {
local body="${1:-}" reason="${2:-unknown}"
[[ "$reason" =~ ^[a-z][a-z-]*$ ]] || reason="unknown"

local stripped quoted
stripped=$(jq -Rsr '
gsub("<!--[[:space:]]*dev-lead:comment-disposition[^\\n]*?-->"; "")
| sub("\\s+$"; "")
' <<<"$body" 2>/dev/null) || stripped=""
quoted=$(printf '%s\n' "$stripped" | sed 's/^/> /')

printf '%s\n\n%s\n\n%s\n\n%s%s -->\n' \
"**⚠️ Retracted by the dev-lead harness (\`${reason}\`).** This reply's \`fixed\` disposition cited a commit that is not on the PR branch (the push did not land), so it no longer counts as a fix. The comment stays open; the next dev-lead pass re-answers it against the pushed branch." \
"${_CL_ORIGINAL_HEADER} (unverified — it does not count as a fix):" \
"$quoted" \
"$_CL_RETRACTED_PREFIX" "$reason"
}

# cl_rewrite_disposition_sha <body> <new_sha>
# <body> with its `fixed` disposition marker's sha replaced by <new_sha> (a full
# 40-hex SHA); every other byte is kept. Used when the push guard rebased the
# cited commit and it landed under a new SHA (#2156, as cl_rewrite_claim_sha).
# Returns 1 (echoing nothing) when the body has no valid `fixed` disposition or
# <new_sha> is not a full SHA. Pure.
cl_rewrite_disposition_sha() {
local body="${1:-}" new_sha="${2:-}"
[[ "$new_sha" =~ ^[0-9a-f]{40}$ ]] || return 1
local parsed old
parsed=$(cdv_parse_disposition "$body") || return 1
[[ "$(jq -r '.disposition // ""' <<<"$parsed" 2>/dev/null)" == "fixed" ]] || return 1
old=$(jq -r '.sha // ""' <<<"$parsed" 2>/dev/null) || return 1

local before rest after middle new_middle
before="${body%%"$_CDV_MARKER_PREFIX"*}"
rest="${body#*"$_CDV_MARKER_PREFIX"}"
after="${rest#*"$_CDV_MARKER_SUFFIX"}"
middle="${rest%%"$_CDV_MARKER_SUFFIX"*}"
new_middle="${middle/"sha=${old}"/"sha=${new_sha}"}"
printf '%s%s%s%s%s\n' "$before" "$_CDV_MARKER_PREFIX" "$new_middle" "$_CDV_MARKER_SUFFIX" "$after"
}

# cl_remote_head
# The single impure reader: fetch the current branch's upstream and echo the TRUE
# remote head SHA (not a stale remote-tracking ref). Returns 1 (echoing nothing)
Expand Down
Loading
Loading