Merge pull request #188 from netresearch/feat/pr-status-186-187

fix(pr-status): hold --watch on unsettled CI; detect the invisible signature gate

Author: CybotTM

skills/git-workflow/references/pull-request-workflow.md

@@ -403,6 +403,22 @@ On `CLOSED / FIXED`, treat it as an ordinary already-fixed bot thread: reply wit
 
 Observed 2026-07-30 on a `docker:S8544` finding: gate `OK`, 0 open issues, 0 failing checks, `mergeStateStatus: BLOCKED` on one stale thread — a merge that looked inexplicably stuck until the thread was read.
 
+### `gh pr update-branch` re-writes the head UNSIGNED
+
+Both forms (merge and `--rebase`) create the new commit server-side, signed by
+nobody. In a repo that requires signed commits — including a requirement
+living in **classic branch protection**, which neither the rulesets endpoint
+nor a non-admin protection query can see — the PR then sits at
+`mergeStateStatus: BLOCKED` with every visible gate green (observed on a PR
+that reported request-review for an hour while the real blocker was the
+signature). Rebase locally instead: signing is wired into git, so a plain
+`git rebase origin/main` (or `git commit --amend --no-edit` when only the
+signature is missing) re-signs, then push with `--force-with-lease`. Two
+traps in that push: a checkout created from `FETCH_HEAD` has no lease
+baseline and fails with `stale info` — pass the lease explicitly as
+`--force-with-lease=<branch>:<remote-sha>`; and that remote SHA must be
+**measured** (`git ls-remote origin <branch>`), never retyped from memory.
+
 ### Signature verification: the GitHub API is the source of truth, not your keyring
 
 For "is this commit signed?" in a review, ask the API:

skills/git-workflow/scripts/pr-status.sh

@@ -408,6 +408,38 @@ evaluate() {
                     else "" end)
                  + " — the queue merges it once its own checks pass; enqueueing"
                  + " again only restarts them")}
+         # A required-signatures gate can live in CLASSIC branch protection,
+         # which neither the rulesets endpoint nor a non-admin protection
+         # query can see: mergeState sits at BLOCKED while every visible gate
+         # is green. When a branch commit is unsigned, that invisible gate is
+         # the prime suspect — and it must outrank the advisory review
+         # branches below, which otherwise mask it (observed: a
+         # `gh pr update-branch --rebase` re-wrote the head UNSIGNED and the
+         # PR reported request-review while the real blocker was the
+         # signature; #187). Guarded on checks_settled and zero failures so
+         # it only fires when nothing visible explains the BLOCKED.
+         # reviewDecision is consulted first: BLOCKED caused by a required
+         # human review (REVIEW_REQUIRED / CHANGES_REQUESTED) reaches this
+         # spot too, and authors who simply do not sign commits would get a
+         # history-rewriting rebase command for a review problem.
+         elif (($s.unsigned|length) > 0
+               and $s.mergeState == "BLOCKED"
+               and ($s.reviewDecision != "REVIEW_REQUIRED")
+               and ($s.reviewDecision != "CHANGES_REQUESTED")
+               and $s.checks_settled
+               and (($s.checks.failing|length) == 0)
+               and $s.unresolved_threads == 0) then
+           {action:"fix-signatures",
+            why:("\($s.unsigned|length) commit(s) on this branch carry no valid signature — \($s.unsigned|join(", "))"
+                 + " — and mergeState is BLOCKED with every visible gate green."
+                 + " A signature requirement in classic branch protection is"
+                 + " invisible to the rulesets endpoint and to non-admin"
+                 + " queries — fix the signatures before chasing review state."
+                 + " Note: gh pr update-branch re-writes the head UNSIGNED;"
+                 + " rebase locally instead"),
+            # No single quotes in here: the whole jq program lives in a
+            # single-quoted shell string (same trap as the sibling cmd below).
+            cmd:"git rebase --exec \"git commit --amend --no-edit -S\" $(git merge-base HEAD origin/\($s.base)) ; git push --force-with-lease"}
          # `|` binds looser than `and`, so the negation needs its own parens:
          # `a and b|not` parses as `(a and b)|not` and inverts the whole test.
          # has_copilot_review_on_head must be false too: the error row stays on
@@ -643,12 +675,20 @@ while :; do
   fi
   case "$act" in
     request-review)
+      # request-review while CI has not settled is not actionable yet — the
+      # ladder itself stamps "do not enqueue on this reading" onto the why.
+      # Keep waiting: a failing check fires the branch above, and once the
+      # checks settle this returns on the review state (#186). The quota
+      # dead-end is exempt — waiting cannot clear it, return immediately.
+      if [ "$(jq -r '.checks_settled' <<<"$s")" != "true" ] \
+         && [ "$(jq -r '.copilot_quota_hit // false' <<<"$s")" != "true" ]; then
+        :
       # A request-review whose cause is an exhausted review bot is a standing
       # condition, not an event: waiting cannot clear a quota ceiling, so every
       # re-arm of --watch returns instantly with the same line. Saying so is
       # what stops an operator re-arming it three times before switching to
       # `gh pr checks --watch`, which watches something that does move.
-      if [ "$(jq -r '.copilot_quota_hit // false' <<<"$s")" = "true" ]; then
+      elif [ "$(jq -r '.copilot_quota_hit // false' <<<"$s")" = "true" ]; then
         echo "ACTIONABLE: request-review (UNSATISFIABLE — Copilot is OUT OF REVIEW QUOTA" \
              "for the month; this will NOT change until the monthly reset, on this or any" \
              "other PR. Do not re-arm this watch and do not re-request — review the diff" \
@@ -662,8 +702,12 @@ while :; do
       else
         echo "ACTIONABLE: request-review"
       fi
-      emit "$s"; exit 0 ;;
-    fix-ci|triage-ci|resolve-threads|rebase|resolve-conflicts|merge|blocked|none)
+      # Unsettled-CI hold: emit nothing, fall through to the wait line.
+      if [ "$(jq -r '.checks_settled' <<<"$s")" = "true" ] \
+         || [ "$(jq -r '.copilot_quota_hit // false' <<<"$s")" = "true" ]; then
+        emit "$s"; exit 0
+      fi ;;
+    fix-ci|triage-ci|resolve-threads|rebase|resolve-conflicts|merge|blocked|none|fix-signatures)
       echo "ACTIONABLE: $act"
       emit "$s"; exit 0 ;;
   esac

tests/test_pr_status_errored_review.sh

@@ -56,9 +56,22 @@ cat "$STUB_DIR/graphql.json"
 STUB
     chmod +x "$STUB_DIR/gh"
     python3 - "$STUB_DIR/graphql.json" "$@" <<'PY'
-import sys, json
+import sys, json, os
 out, bodies = sys.argv[1], sys.argv[2:]
 head = "deadbeefcafe"
+# Env overrides so the signature/settled ladder branches are testable:
+#   MERGE_STATE (default CLEAN), SIGNED (default 1),
+#   PENDING_CHECK=1 adds an IN_PROGRESS check run (checks_settled -> false).
+merge_state = os.environ.get("MERGE_STATE", "CLEAN")
+signed = os.environ.get("SIGNED", "1") == "1"
+review_decision = os.environ.get("REVIEW_DECISION") or None
+checks = [{"__typename": "CheckRun", "name": "CI", "conclusion": "SUCCESS",
+           "status": "COMPLETED", "detailsUrl": "u",
+           "startedAt": "2026-01-01T00:00:00Z"}]
+if os.environ.get("PENDING_CHECK", "0") == "1":
+    checks.append({"__typename": "CheckRun", "name": "slow", "conclusion": None,
+                   "status": "IN_PROGRESS", "detailsUrl": "u",
+                   "startedAt": "2026-01-01T00:00:00Z"})
 reviews = [{"author": {"login": "copilot-pull-request-reviewer"},
             "state": "COMMENTED", "commit": {"oid": head}, "body": b}
            for b in bodies]
@@ -67,20 +80,17 @@ json.dump({"data": {"repository": {
     "mergeCommitAllowed": True, "rebaseMergeAllowed": False, "squashMergeAllowed": False,
     "pullRequest": {
         "number": 1, "title": "t", "state": "OPEN", "isDraft": False,
-        "mergeable": "MERGEABLE", "mergeStateStatus": "CLEAN", "reviewDecision": None,
+        "mergeable": "MERGEABLE", "mergeStateStatus": merge_state, "reviewDecision": review_decision,
         "author": {"login": "someone"},
         "baseRefName": "main", "headRefName": "f", "headRefOid": head,
         "isCrossRepository": False,
         "reviews": {"nodes": reviews},
         "reviewRequests": {"nodes": []},
         "reviewThreads": {"nodes": []},
         "commits": {"nodes": [{"commit": {"oid": head, "statusCheckRollup": {
-            "state": "SUCCESS", "contexts": {"nodes": [
-                {"__typename": "CheckRun", "name": "CI", "conclusion": "SUCCESS",
-                 "status": "COMPLETED", "detailsUrl": "u",
-                 "startedAt": "2026-01-01T00:00:00Z"}]}}}}]},
+            "state": "SUCCESS", "contexts": {"nodes": checks}}}}]},
         "allCommits": {"nodes": [{"commit": {"oid": head,
-                                             "signature": {"isValid": True}}}]},
+                                             "signature": {"isValid": signed}}}]},
     }}}}, open(out, "w"))
 PY
 }
@@ -430,6 +440,83 @@ make_stub_reviews \
   "a-human|CHANGES_REQUESTED|found something else"
 check "reviews_on_head[a-human]" "APPROVED+CHANGES_REQUESTED" "$(run_flag '"reviews_on_head"."a-human"')"
 
+
+# --- Signature gate (#187): BLOCKED + unsigned + all green must name the ---
+# --- signature, even when a quota-errored Copilot row sits on the head    ---
+echo "case S1: BLOCKED + unsigned + quota error row — fix-signatures outranks request-review"
+MERGE_STATE=BLOCKED SIGNED=0 make_stub "$ERR_QUOTA"
+check "next.action" "fix-signatures" "$(run_next)"
+if status | jq -e '.next.why | test("classic branch protection")' >/dev/null; then
+    echo "  ok   why names the invisible classic protection"
+else
+    echo "  FAIL why does not name classic branch protection"
+    fail=1
+fi
+
+echo "case S2: BLOCKED + signed — the signature gate must stay quiet"
+MERGE_STATE=BLOCKED SIGNED=1 make_stub "$ERR_QUOTA"
+check "next.action" "request-review" "$(run_next)"
+
+echo "case S3: CLEAN + unsigned — no signature rule visible, merge stays open"
+MERGE_STATE=CLEAN SIGNED=0 make_stub "$REAL_REVIEW"
+check "next.action" "merge" "$(run_next)"
+
+echo "case S4: BLOCKED + unsigned + REVIEW_REQUIRED — review cause outranks the signature guess"
+MERGE_STATE=BLOCKED SIGNED=0 REVIEW_DECISION=REVIEW_REQUIRED make_stub "$ERR_QUOTA"
+if [ "$(run_next)" = "fix-signatures" ]; then
+    echo "  FAIL fired fix-signatures although a required review explains the BLOCKED"
+    fail=1
+else
+    echo "  ok   signature branch stays quiet when reviewDecision explains BLOCKED"
+fi
+
+# --- Watch settle-gate (#186): request-review with unsettled CI must WAIT ---
+watch() { PATH="$STUB_DIR:$PATH" bash "$SCRIPT" -R o/r 1 --watch --interval 1 --max-wait 2; }
+echo "case W1: watch + pending check + no review — holds until timeout, no ACTIONABLE"
+PENDING_CHECK=1 make_stub
+out=$(watch || true)
+case "$out" in
+    *"ACTIONABLE: request-review"*)
+        echo "  FAIL watch emitted request-review while CI was unsettled"; fail=1 ;;
+    *TIMEOUT*)
+        echo "  ok   watch held (timeout) instead of firing early" ;;
+    *)  echo "  FAIL unexpected watch output: $(printf '%s' "$out" | head -2)"; fail=1 ;;
+esac
+
+echo "case W2: watch + settled + no review — returns immediately"
+make_stub
+out=$(watch || true)
+case "$out" in
+    *"ACTIONABLE: request-review"*) echo "  ok   immediate return on settled CI" ;;
+    *) echo "  FAIL expected immediate ACTIONABLE: request-review"; fail=1 ;;
+esac
+
+echo "case W3: watch + pending check + QUOTA error — quota exemption still returns immediately"
+PENDING_CHECK=1 make_stub "$ERR_QUOTA"
+# Assert the discriminating signal: the exemption path's unique ACTIONABLE
+# prefix AND no TIMEOUT. The bare quota phrase also appears in every
+# "waiting:" line's why-text, so matching it passes vacuously when the
+# exemption is broken and the watch holds to timeout (found by mutation d).
+out=$(watch || true)
+case "$out" in
+    *"ACTIONABLE: request-review (UNSATISFIABLE"*)
+        case "$out" in
+            *TIMEOUT*) echo "  FAIL exemption fired only at timeout"; fail=1 ;;
+            *) echo "  ok   quota dead-end returns immediately despite unsettled CI" ;;
+        esac ;;
+    *) echo "  FAIL quota exemption did not fire (no UNSATISFIABLE ACTIONABLE line)"; fail=1 ;;
+esac
+
+echo "case W4: watch + BLOCKED + unsigned + settled — fix-signatures is an actionable event"
+MERGE_STATE=BLOCKED SIGNED=0 make_stub "$REAL_REVIEW"
+out=$(watch || true)
+case "$out" in
+    *"ACTIONABLE: fix-signatures"*) echo "  ok   watch returns immediately on the signature gate" ;;
+    *TIMEOUT*) echo "  FAIL watch held to timeout on its own headline scenario"; fail=1 ;;
+    *) echo "  FAIL unexpected watch output"; fail=1 ;;
+esac
+
+
 if [ "$fail" -eq 0 ]; then
     echo "all pass"
 else