fix(dev-lead): trusted-author tracker links; no auto-merge after a failed deferral (#2045 follow-up)

scripts/dev-lead-fix-reviews.sh

@@ -3251,6 +3251,10 @@ case "$INTENT_TYPE" in
       if [ "$deferred_rc" -ne 0 ]; then
         rc=1
         echo "::warning::fix-reviews: a verified deferral could not be resolved — withholding the terminal marker so the pass is retried"
+        # Not fully applied: never let this pass become auto-mergeable (#1567), and
+        # drop the EXIT-trap restore so auto-merge held off at the start stays off.
+        _REVIEW_INCOMPLETE=1
+        _AM_NEEDS_RESTORE=0
         # dev-lead-retry.sh re-dispatches only on a retry marker; a merely absent
         # terminal is never retried. Guard aborts (3/4) are flagged for a human.
         if [ "$cp_rc" -ne 3 ] && [ "$cp_rc" -ne 4 ]; then
@@ -3398,6 +3402,8 @@ case "$INTENT_TYPE" in
         rc=1
         echo "::warning::fix-bot-comment: a verified deferral could not be resolved — withholding the terminal marker so the pass is retried"
         _fbc_terminal=""
+        _REVIEW_INCOMPLETE=1
+        _AM_NEEDS_RESTORE=0
         # The bot-comment retry selects only undispositioned comments and the
         # bot-thread retry only unreplied threads, so neither re-runs this resolver.
         # Hand it to a PR-wide fix-reviews retry. Guard aborts (3/4) are flagged for a human.
@@ -3421,7 +3427,7 @@ case "$INTENT_TYPE" in
         else
           echo "::notice::resolution gate closed (#1609): the fix-bot-comment pass did not advance PR #${PR_NUMBER}'s head — zero review threads resolved"
         fi
-        [ "${_DISPOSITIONS_UNRESOLVED:-0}" -eq 1 ] || try_enable_auto_merge
+        { [ "${_DISPOSITIONS_UNRESOLVED:-0}" -eq 1 ] || [ "${_REVIEW_INCOMPLETE:-0}" -eq 1 ]; } || try_enable_auto_merge
       fi
       # #2037: the pass ends failed so it is visibly not done; no auto-merge restore.
       if [ "${_DISPOSITIONS_UNRESOLVED:-0}" -eq 1 ]; then
@@ -3525,6 +3531,10 @@ case "$INTENT_TYPE" in
       if [ "$deferred_rc" -ne 0 ]; then
         rc=1
         echo "::warning::review-changes: a verified deferral could not be resolved — withholding the terminal marker so the pass is retried"
+        # Not fully applied: never let this pass become auto-mergeable (#1567), and
+        # drop the EXIT-trap restore so auto-merge held off at the start stays off.
+        _REVIEW_INCOMPLETE=1
+        _AM_NEEDS_RESTORE=0
         # dev-lead-retry.sh re-dispatches only on a retry marker; a merely absent
         # terminal is never retried.
         post_resolve_failed_marker "review-changes" "$cp_rc"

scripts/lib/deferred-thread-verify.sh

@@ -35,6 +35,10 @@ set -euo pipefail
 readonly _DTV_MARKER_PREFIX='<!-- dev-lead:deferred'
 readonly _DTV_MARKER_SUFFIX='-->'
 readonly _DTV_TRACKER_TITLE='dev-lead: deferred review findings'
+# Only text written by these author associations can link a thread to the tracker:
+# anyone who can open or comment on an issue must not be able to authorize
+# resolving a merge-blocking bot thread.
+readonly _DTV_TRUSTED_ASSOC='["OWNER","MEMBER","COLLABORATOR"]'
 
 # dtv_parse_deferral <reply_body>
 #   Extract the tracking-issue number from the single deferral marker in a reply.
@@ -153,7 +157,9 @@ dtv_text_mentions_thread() {
 #   <issue_comments_json> is the REST array of its comments ({body}). Echoes "ok"
 #   and returns 0 when the issue is an OPEN ISSUE (not a pull request) whose body or
 #   any comment links the thread (dtv_text_mentions_thread). Otherwise echoes a
-#   reason and returns 1: missing | not-an-issue | wrong-title | closed | no-mention.
+#   reason and returns 1: missing | not-an-issue | wrong-title | closed | no-mention |
+#   untrusted-mention. Only the issue body (when the issue's author_association is
+#   OWNER/MEMBER/COLLABORATOR) and comments by such authors can link the thread.
 #   The issue must carry the exact shared-tracker title
 #   `dev-lead: deferred review findings` (AC6), never a per-finding issue. An empty or
 #   unparseable <issue_json> (a failed or 404 fetch) reads as missing; unparseable
@@ -162,11 +168,12 @@ dtv_verify_tracking_issue() {
   local issue_json="${1:-}" comments_json="${2:-}" thread_id="${3:-}" db_id="${4:-}"
   local repo="${5:-}" pr="${6:-}"
   local facts
-  facts=$(jq -c '
+  facts=$(jq -c --argjson trusted "$_DTV_TRUSTED_ASSOC" '
       if type == "object" and ((.number // null) | type) == "number" then
         {kind: (if has("pull_request") and .pull_request != null then "pr" else "issue" end),
          state: ((.state // "") | ascii_downcase),
          title: (.title // ""),
+         trusted: (((.author_association // "") | ascii_upcase) as $a | $trusted | index($a) != null),
          body: (.body // "")}
       else empty end
     ' <<<"$issue_json" 2>/dev/null || true)
@@ -191,19 +198,31 @@ dtv_verify_tracking_issue() {
     echo "closed"
     return 1
   fi
-  local text
+  local text trusted
   text=$(jq -r '.body' <<<"$facts" 2>/dev/null) || text=""
-  if dtv_text_mentions_thread "$text" "$thread_id" "$db_id" "$repo" "$pr"; then
+  trusted=$(jq -r '.trusted' <<<"$facts" 2>/dev/null) || trusted="false"
+  if [[ "$trusted" == "true" ]] && dtv_text_mentions_thread "$text" "$thread_id" "$db_id" "$repo" "$pr"; then
     echo "ok"
     return 0
   fi
   local comment_text
-  comment_text=$(jq -r 'if type == "array" then .[] | objects | (.body // "") else empty end' \
-    <<<"${comments_json:-[]}" 2>/dev/null || true)
+  comment_text=$(jq -r --argjson trusted "$_DTV_TRUSTED_ASSOC" '
+      if type == "array" then .[] | objects
+        | select(((.author_association // "") | ascii_upcase) as $a | $trusted | index($a) != null)
+        | (.body // "")
+      else empty end' <<<"${comments_json:-[]}" 2>/dev/null || true)
   if dtv_text_mentions_thread "$comment_text" "$thread_id" "$db_id" "$repo" "$pr"; then
     echo "ok"
     return 0
   fi
+  # A link written only by an untrusted author is reported separately for the logs.
+  local any_text
+  any_text=$(jq -r 'if type == "array" then .[] | objects | (.body // "") else empty end' \
+    <<<"${comments_json:-[]}" 2>/dev/null || true)
+  if dtv_text_mentions_thread "${text}"$'\n'"${any_text}" "$thread_id" "$db_id" "$repo" "$pr"; then
+    echo "untrusted-mention"
+    return 1
+  fi
   echo "no-mention"
   return 1
 }

tests/dev-lead/unit/test_deferred_thread_verify.bats

@@ -178,16 +178,42 @@ ${marker}
 
 @test "dtv_verify_tracking_issue: open issue whose body links the thread -> ok" {
   local issue
-  issue=$(jq -cn --arg b "Deferred findings:\n- ${LINK}" '{number:2050,title:"dev-lead: deferred review findings",state:"open",body:$b}')
+  issue=$(jq -cn --arg b "Deferred findings:\n- ${LINK}" '{number:2050,title:"dev-lead: deferred review findings",author_association:"MEMBER",state:"open",body:$b}')
   run dtv_verify_tracking_issue "$issue" "[]" "$THREAD_ID" "$DB_ID"
   [ "$status" -eq 0 ]
   [ "$output" = "ok" ]
 }
 
 @test "dtv_verify_tracking_issue: open issue whose COMMENT links the thread -> ok" {
   local comments
-  comments=$(jq -cn --arg b "Appended: ${LINK}" '[{body:"unrelated"},{body:$b}]')
-  run dtv_verify_tracking_issue '{"number":2050,"title":"dev-lead: deferred review findings","state":"open","body":"Deferred findings"}' \
+  comments=$(jq -cn --arg b "Appended: ${LINK}" '[{author_association:"MEMBER",body:"unrelated"},{author_association:"MEMBER",body:$b}]')
+  run dtv_verify_tracking_issue '{"number":2050,"title":"dev-lead: deferred review findings","author_association":"MEMBER","state":"open","body":"Deferred findings"}' \
+    "$comments" "$THREAD_ID" "$DB_ID"
+  [ "$status" -eq 0 ]
+  [ "$output" = "ok" ]
+}
+
+@test "dtv_verify_tracking_issue: a link only in an UNTRUSTED author's issue body -> untrusted-mention" {
+  local issue
+  issue=$(jq -cn --arg b "Deferred findings:\n- ${LINK}" '{number:2050,title:"dev-lead: deferred review findings",author_association:"NONE",state:"open",body:$b}')
+  run dtv_verify_tracking_issue "$issue" "[]" "$THREAD_ID" "$DB_ID"
+  [ "$status" -eq 1 ]
+  [ "$output" = "untrusted-mention" ]
+}
+
+@test "dtv_verify_tracking_issue: a link only in an UNTRUSTED comment -> untrusted-mention" {
+  local comments
+  comments=$(jq -cn --arg b "Appended: ${LINK}" '[{author_association:"CONTRIBUTOR",body:$b}]')
+  run dtv_verify_tracking_issue '{"number":2050,"title":"dev-lead: deferred review findings","author_association":"MEMBER","state":"open","body":"Deferred findings"}' \
+    "$comments" "$THREAD_ID" "$DB_ID"
+  [ "$status" -eq 1 ]
+  [ "$output" = "untrusted-mention" ]
+}
+
+@test "dtv_verify_tracking_issue: a TRUSTED comment links the thread on an untrusted author's issue -> ok" {
+  local comments
+  comments=$(jq -cn --arg b "Appended: ${LINK}" '[{author_association:"NONE",body:"spam"},{author_association:"COLLABORATOR",body:$b}]')
+  run dtv_verify_tracking_issue '{"number":2050,"title":"dev-lead: deferred review findings","author_association":"NONE","state":"open","body":"Deferred findings"}' \
     "$comments" "$THREAD_ID" "$DB_ID"
   [ "$status" -eq 0 ]
   [ "$output" = "ok" ]
@@ -204,29 +230,29 @@ ${marker}
 
 @test "dtv_verify_tracking_issue: closed issue -> closed" {
   local issue
-  issue=$(jq -cn --arg b "${LINK}" '{number:2050,title:"dev-lead: deferred review findings",state:"closed",body:$b}')
+  issue=$(jq -cn --arg b "${LINK}" '{number:2050,title:"dev-lead: deferred review findings",author_association:"MEMBER",state:"closed",body:$b}')
   run dtv_verify_tracking_issue "$issue" "[]" "$THREAD_ID" "$DB_ID"
   [ "$status" -eq 1 ]
   [ "$output" = "closed" ]
 }
 
 @test "dtv_verify_tracking_issue: a pull request is not a tracking issue" {
   local issue
-  issue=$(jq -cn --arg b "${LINK}" '{number:2050,title:"dev-lead: deferred review findings",state:"open",body:$b,pull_request:{url:"x"}}')
+  issue=$(jq -cn --arg b "${LINK}" '{number:2050,title:"dev-lead: deferred review findings",author_association:"MEMBER",state:"open",body:$b,pull_request:{url:"x"}}')
   run dtv_verify_tracking_issue "$issue" "[]" "$THREAD_ID" "$DB_ID"
   [ "$status" -eq 1 ]
   [ "$output" = "not-an-issue" ]
 }
 
 @test "dtv_verify_tracking_issue: issue that does not mention the thread -> no-mention" {
-  run dtv_verify_tracking_issue '{"number":2050,"title":"dev-lead: deferred review findings","state":"open","body":"Deferred findings"}' \
+  run dtv_verify_tracking_issue '{"number":2050,"title":"dev-lead: deferred review findings","author_association":"MEMBER","state":"open","body":"Deferred findings"}' \
     '[{"body":"some other thread #discussion_r1"}]' "$THREAD_ID" "$DB_ID"
   [ "$status" -eq 1 ]
   [ "$output" = "no-mention" ]
 }
 
 @test "dtv_verify_tracking_issue: unparseable comments read as none (fail closed)" {
-  run dtv_verify_tracking_issue '{"number":2050,"title":"dev-lead: deferred review findings","state":"open","body":"x"}' "garbage" "$THREAD_ID" "$DB_ID"
+  run dtv_verify_tracking_issue '{"number":2050,"title":"dev-lead: deferred review findings","author_association":"MEMBER","state":"open","body":"x"}' "garbage" "$THREAD_ID" "$DB_ID"
   [ "$status" -eq 1 ]
   [ "$output" = "no-mention" ]
 }

tests/dev-lead/unit/test_fix_reviews.bats

@@ -1742,6 +1742,7 @@ GITEOF
 #   DEFER_INTENT           intent to run (default fix-reviews)
 #   DEFER_ENGINE_FAIL      "true" -> the engine exits non-zero (a failed pass)
 #   DEFER_THREADS_STUCK    "true" -> the deferral enumerator's thread pages never advance
+#   DEFER_AM_ON            non-empty -> the PR starts with auto-merge on (held, then restorable)
 _2045_run_case() {
   local tmpdir="$BATS_TEST_TMPDIR/workdir"
   mkdir -p "$tmpdir"
@@ -1804,6 +1805,11 @@ case "\$ARGS" in
   *"check-runs"*) echo '{"check_runs":[]}' ;;
   *"statuses"*) echo '[]' ;;
   *"pulls/"*"reviews"*) echo '[]' ;;
+  *"pulls/"*"auto_merge.merge_method"*) [ -n "\${DEFER_AM_ON:-}" ] && echo squash ;;
+  *"pulls/"*"auto_merge.commit_"*) : ;;
+  *"pulls/"*"--jq .auto_merge // empty"*) [ -z "\${DEFER_AM_ON:-}" ] && echo '{"merge_method":"squash"}' ;;
+  *"pulls/"*"--jq .state"*) echo open ;;
+  *"pulls/"*"--jq .head.sha"*) echo "${base_sha}" ;;
   *"pulls/"*) echo '{"head":{"sha":"${base_sha}"},"auto_merge":null}' ;;
   *"pr checkout"*) exit 0 ;;
   *"pr comment"*) exit 0 ;;
@@ -1869,7 +1875,7 @@ _2045_comments() {
 
 _2045_open_issue() {
   jq -cn --arg b "Deferred review findings\n- [ ] duration_ms — ${_2045_LINK}" \
-    '{number:2050,title:"dev-lead: deferred review findings",state:"open",body:$b}'
+    '{number:2050,title:"dev-lead: deferred review findings",author_association:"MEMBER",state:"open",body:$b}'
 }
 
 @test "resolve_deferred_bot_threads (#2045): verified deferral resolves on a NO-commit pass" {
@@ -1895,8 +1901,8 @@ _2045_open_issue() {
 
 @test "resolve_deferred_bot_threads (#2045): a link in a tracking-issue COMMENT is enough" {
   export DEFER_THREAD_COMMENTS="$(_2045_comments 'Deferring. <!-- dev-lead:deferred ref=#2050 -->')"
-  export DEFER_ISSUE_JSON='{"number":2050,"title":"dev-lead: deferred review findings","state":"open","body":"Deferred review findings"}'
-  export DEFER_ISSUE_COMMENTS="$(jq -cn --arg b "- duration_ms: ${_2045_LINK}" '[{body:$b}]')"
+  export DEFER_ISSUE_JSON='{"number":2050,"title":"dev-lead: deferred review findings","author_association":"MEMBER","state":"open","body":"Deferred review findings"}'
+  export DEFER_ISSUE_COMMENTS="$(jq -cn --arg b "- duration_ms: ${_2045_LINK}" '[{author_association:"MEMBER",body:$b}]')"
   _2045_run_case
   [ "$_HARNESS_STATUS" -eq 0 ]
   grep -q "PRRT_2045" "$_MUTATIONS_FILE"
@@ -1945,7 +1951,7 @@ _2045_open_issue() {
 
 @test "resolve_deferred_bot_threads (#2045): ref to a CLOSED issue -> stays open" {
   export DEFER_THREAD_COMMENTS="$(_2045_comments 'Deferring. <!-- dev-lead:deferred ref=#2050 -->')"
-  export DEFER_ISSUE_JSON="$(jq -cn --arg b "${_2045_LINK}" '{number:2050,title:"dev-lead: deferred review findings",state:"closed",body:$b}')"
+  export DEFER_ISSUE_JSON="$(jq -cn --arg b "${_2045_LINK}" '{number:2050,title:"dev-lead: deferred review findings",author_association:"MEMBER",state:"closed",body:$b}')"
   _2045_run_case
   [ "$_HARNESS_STATUS" -eq 0 ]
   [[ "$_HARNESS_OUTPUT" == *"(closed)"* ]]
@@ -1955,7 +1961,7 @@ _2045_open_issue() {
 
 @test "resolve_deferred_bot_threads (#2045): tracking issue that does not mention the thread -> stays open" {
   export DEFER_THREAD_COMMENTS="$(_2045_comments 'Deferring. <!-- dev-lead:deferred ref=#2050 -->')"
-  export DEFER_ISSUE_JSON='{"number":2050,"title":"dev-lead: deferred review findings","state":"open","body":"Deferred review findings (none linked)"}'
+  export DEFER_ISSUE_JSON='{"number":2050,"title":"dev-lead: deferred review findings","author_association":"MEMBER","state":"open","body":"Deferred review findings (none linked)"}'
   _2045_run_case
   [ "$_HARNESS_STATUS" -eq 0 ]
   [[ "$_HARNESS_OUTPUT" == *"(no-mention)"* ]]
@@ -1965,7 +1971,7 @@ _2045_open_issue() {
 
 @test "resolve_deferred_bot_threads (#2045): tracking-issue comments page failure -> stays open and counts as failure" {
   export DEFER_THREAD_COMMENTS="$(_2045_comments 'Deferring. <!-- dev-lead:deferred ref=#2050 -->')"
-  export DEFER_ISSUE_JSON='{"number":2050,"title":"dev-lead: deferred review findings","state":"open","body":"Deferred review findings (none linked)"}'
+  export DEFER_ISSUE_JSON='{"number":2050,"title":"dev-lead: deferred review findings","author_association":"MEMBER","state":"open","body":"Deferred review findings (none linked)"}'
   export DEFER_ISSUE_COMMENTS_FAIL=1
   _2045_run_case
   [ "$_HARNESS_STATUS" -eq 1 ]
@@ -2032,6 +2038,51 @@ _2045_open_issue() {
   grep -q "status=blocked reason=resolve-failed" "$BATS_TEST_TMPDIR/gh-calls"
 }
 
+@test "resolve_deferred_bot_threads (#2045): a tracker linked only by an untrusted author -> stays open" {
+  export DEFER_THREAD_COMMENTS="$(_2045_comments 'Deferring. <!-- dev-lead:deferred ref=#2050 -->')"
+  export DEFER_ISSUE_JSON="$(jq -cn --arg b "${_2045_LINK}" '{number:2050,title:"dev-lead: deferred review findings",author_association:"NONE",state:"open",body:$b}')"
+  _2045_run_case
+  [[ "$_HARNESS_OUTPUT" == *"untrusted-mention"* ]]
+  run grep -q "PRRT_2045" "$_MUTATIONS_FILE"
+  [ "$status" -eq 1 ]
+}
+
+@test "resolve_deferred_bot_threads (#2045): a failed deferral does not restore auto-merge held off at the start" {
+  export DEFER_AM_ON=1
+  export DEFER_THREAD_COMMENTS="$(_2045_comments 'Deferring. <!-- dev-lead:deferred ref=#2050 -->')"
+  export DEFER_ISSUE_JSON="$(_2045_open_issue)"
+  export DEFER_RESOLVE_FAIL=1
+  _2045_run_case
+  [ "$_HARNESS_STATUS" -eq 1 ]
+  [[ "$_HARNESS_OUTPUT" == *"holding auto-merge OFF"* ]]
+  grep -q "pr merge 54 --repo petry-projects/.github-private --disable-auto" "$BATS_TEST_TMPDIR/gh-calls"
+  run grep -E "pr merge.*--auto( |$)" "$BATS_TEST_TMPDIR/gh-calls"
+  [ "$status" -eq 1 ]
+}
+
+@test "resolve_deferred_bot_threads (#2045): a successful deferral still restores auto-merge held off at the start" {
+  export DEFER_AM_ON=1
+  export DEFER_THREAD_COMMENTS="$(_2045_comments 'Deferring. <!-- dev-lead:deferred ref=#2050 -->')"
+  export DEFER_ISSUE_JSON="$(_2045_open_issue)"
+  _2045_run_case
+  [ "$_HARNESS_STATUS" -eq 0 ]
+  grep -qE "pr merge 54 .*--auto --squash" "$BATS_TEST_TMPDIR/gh-calls"
+}
+
+@test "resolve_deferred_bot_threads (#2045): a failed deferral on a commit pass never enables auto-merge (#1567)" {
+  export DEFER_COMMIT=true
+  export DEFER_THREAD_COMMENTS="$(_2045_comments 'Deferring. <!-- dev-lead:deferred ref=#2050 -->')"
+  export DEFER_ISSUE_JSON="$(_2045_open_issue)"
+  export DEFER_RESOLVE_FAIL=1
+  _2045_run_case
+  [ "$_HARNESS_STATUS" -eq 1 ]
+  # try_enable_auto_merge never runs (the stub reports auto-merge as already on,
+  # so reaching it logs "auto-merge already enabled").
+  [[ "$_HARNESS_OUTPUT" != *"auto-merge already enabled"* ]]
+  run grep -E "pr merge.*--auto" "$BATS_TEST_TMPDIR/gh-calls"
+  [ "$status" -eq 1 ]
+}
+
 @test "resolve_deferred_bot_threads (#2045): a fix-bot-comment resolution failure posts a PR-wide fix-reviews retry marker" {
   # Neither the bot-comment retry (undispositioned comments) nor the bot-thread
   # retry (unreplied threads) would re-select this thread, so fix-reviews must.