fix(scripts): avoid false PR head changes during GitHub outages (#125517)

gh exit status was treated as proof that stdout held a PR object. The Octopool cache shim reports upstream 5xx responses as exit 0 with empty stdout, so reads now use a bounded-retry validation helper. Convert 9 of 28 gh pr view sites where an empty payload could produce a wrong decision.

scripts/pr

@@ -240,8 +240,9 @@ require_cmds() {
 
 require_main_target_pr() {
   local pr="$1"
-  local base
-  base=$(gh pr view "$pr" --json baseRefName --jq .baseRefName)
+  local base base_json
+  base_json=$(read_pr_view_json "$pr" "baseRefName") || exit 1
+  base=$(pr_view_string_field "$base_json" "baseRefName" "$pr" "Retry the scripts/pr command.") || exit 1
   if [ "$base" != "main" ]; then
     echo "scripts/pr prepare and merge commands only support PRs targeting main; PR #$pr targets $base." >&2
     echo "Use the reviewed release-branch landing flow for non-main PRs." >&2

scripts/pr-lib/common.sh

@@ -145,6 +145,60 @@ bootstrap_deps_if_needed() {
   fi
 }
 
+read_pr_view_json() {
+  local pr="$1"
+  local fields="$2"
+  local max_attempts=3
+  local temp_dir
+  temp_dir=$(mktemp -d "${TMPDIR:-/tmp}/openclaw-pr-view.XXXXXX") || {
+    echo "Unable to create temporary storage for GitHub PR metadata." >&2
+    return 1
+  }
+  local stdout_file="$temp_dir/stdout"
+  local stderr_file="$temp_dir/stderr"
+  local attempt exit_code reason
+
+  for attempt in $(seq 1 "$max_attempts"); do
+    exit_code=0
+    if gh pr view "$pr" --json "$fields" >"$stdout_file" 2>"$stderr_file"; then
+      if [ -s "$stdout_file" ] && jq -se 'length == 1 and (.[0] | type == "object")' "$stdout_file" >/dev/null 2>&1; then
+        cat "$stdout_file"
+        rm -rf "$temp_dir"
+        return 0
+      fi
+      if [ ! -s "$stdout_file" ]; then
+        reason="gh pr view returned empty stdout"
+      else
+        reason="gh pr view did not return one JSON object"
+      fi
+    else
+      exit_code=$?
+      reason="gh pr view exited with status $exit_code"
+    fi
+    [ "$attempt" -eq "$max_attempts" ] || sleep "$attempt"
+  done
+
+  echo "GitHub API failure while reading PR #$pr: $reason after $max_attempts attempts." >&2
+  [ ! -s "$stderr_file" ] || cat "$stderr_file" >&2
+  rm -rf "$temp_dir"
+  return 1
+}
+
+pr_view_string_field() {
+  local json="$1" field="$2" pr="$3" remedy="${4:-Retry the command.}" label value
+  case "$field" in
+    headRefOid) label="a head SHA" ;;
+    baseRefName) label="a base branch" ;;
+    headRefName) label="a head branch" ;;
+    *) label="a non-empty .$field string" ;;
+  esac
+  if ! value=$(printf '%s\n' "$json" | jq -er --arg field "$field" '.[$field] | if type == "string" and length > 0 then . else error("missing string field") end' 2>/dev/null); then
+    echo "GitHub PR metadata for #$pr did not include $label. $remedy" >&2
+    return 1
+  fi
+  printf '%s\n' "$value"
+}
+
 wait_for_pr_head_sha() {
   local pr="$1"
   local expected_sha="$2"

scripts/pr-lib/gates.sh

@@ -4,8 +4,8 @@ run_hosted_prepare_gates() {
   local changelog_only="$3"
   local recent_sha=""
   local remote_record remote_head remote_head_ref remote_is_cross_repository
-  remote_record=$(gh pr view "$pr" --json headRefName,headRefOid,isCrossRepository)
-  remote_head=$(printf '%s\n' "$remote_record" | jq -r .headRefOid)
+  remote_record=$(read_pr_view_json "$pr" "headRefName,headRefOid,isCrossRepository") || return 1
+  remote_head=$(pr_view_string_field "$remote_record" "headRefOid" "$pr" "Re-run prepare-init.") || return 1
   remote_head_ref=$(printf '%s\n' "$remote_record" | jq -r .headRefName)
   remote_is_cross_repository=$(printf '%s\n' "$remote_record" | jq -r .isCrossRepository)
   if [ "$remote_head" != "$current_head" ]; then

scripts/pr-lib/merge.sh

@@ -242,7 +242,7 @@ merge_run() {
   source .local/prep.env
 
   local pr_meta_json
-  pr_meta_json=$(gh pr view "$pr" --json state,isDraft)
+  pr_meta_json=$(read_pr_view_json "$pr" "state,isDraft") || exit 1
   local is_draft
   is_draft=$(printf '%s\n' "$pr_meta_json" | jq -r .isDraft)
   if [ "$is_draft" = "true" ]; then
@@ -312,7 +312,7 @@ merge_run() {
   # artifact, exact-head, required-check, and drift check in merge_verify.
   if [ "$auto_merge_requested" = "true" ]; then
     local auto_meta
-    auto_meta=$(gh pr view "$pr" --json state,headRefOid,mergeable,mergeStateStatus,autoMergeRequest)
+    auto_meta=$(read_pr_view_json "$pr" "state,headRefOid,mergeable,mergeStateStatus,autoMergeRequest") || exit 1
     local auto_head_sha
     auto_head_sha=$(printf '%s\n' "$auto_meta" | jq -r .headRefOid)
     if [ "$auto_head_sha" != "$PREP_HEAD_SHA" ]; then
@@ -338,7 +338,7 @@ merge_run() {
         print_relevant_log_excerpt .local/merge-output.log
         exit 1
       fi
-      auto_meta=$(gh pr view "$pr" --json state,headRefOid,mergeable,mergeStateStatus,autoMergeRequest)
+      auto_meta=$(read_pr_view_json "$pr" "state,headRefOid,mergeable,mergeStateStatus,autoMergeRequest") || exit 1
       auto_head_sha=$(printf '%s\n' "$auto_meta" | jq -r .headRefOid)
       mergeable=$(printf '%s\n' "$auto_meta" | jq -r '.mergeable // "UNKNOWN"')
       merge_state_status=$(printf '%s\n' "$auto_meta" | jq -r '.mergeStateStatus // "UNKNOWN"')
@@ -365,7 +365,7 @@ merge_run() {
         --match-head-commit "$PREP_HEAD_SHA" \
         >.local/merge-output.log 2>&1
       then
-        auto_meta=$(gh pr view "$pr" --json state,headRefOid,mergeable,mergeStateStatus,autoMergeRequest)
+        auto_meta=$(read_pr_view_json "$pr" "state,headRefOid,mergeable,mergeStateStatus,autoMergeRequest") || exit 1
         auto_head_sha=$(printf '%s\n' "$auto_meta" | jq -r .headRefOid)
         state=$(printf '%s\n' "$auto_meta" | jq -r .state)
         existing_auto_method=$(printf '%s\n' "$auto_meta" | jq -r '.autoMergeRequest.mergeMethod // ""')
@@ -385,7 +385,7 @@ merge_run() {
           exit 1
         fi
       else
-        auto_meta=$(gh pr view "$pr" --json state,headRefOid,autoMergeRequest)
+        auto_meta=$(read_pr_view_json "$pr" "state,headRefOid,autoMergeRequest") || exit 1
         auto_head_sha=$(printf '%s\n' "$auto_meta" | jq -r .headRefOid)
         existing_auto_method=$(printf '%s\n' "$auto_meta" | jq -r '.autoMergeRequest.mergeMethod // ""')
         if [ "$auto_head_sha" = "$PREP_HEAD_SHA" ] && [ -n "$existing_auto_method" ]; then

scripts/pr-lib/push.sh

@@ -179,8 +179,9 @@ verify_pr_head_branch_matches_expected() {
   local pr="$1"
   local expected_head="$2"
 
-  local current_head
-  current_head=$(gh pr view "$pr" --json headRefName --jq .headRefName)
+  local current_head current_head_json
+  current_head_json=$(read_pr_view_json "$pr" "headRefName") || exit 1
+  current_head=$(pr_view_string_field "$current_head_json" "headRefName" "$pr" "Re-run prepare-init.") || exit 1
   if [ "$current_head" != "$expected_head" ]; then
     echo "PR head branch changed from $expected_head to $current_head. Re-run prepare-init."
     exit 1

scripts/pr-lib/worktree.sh

@@ -268,10 +268,13 @@ enter_worktree() {
 
 pr_meta_json() {
   local pr="$1"
-  local metadata files expected_file_count actual_file_count head_before head_after
-  metadata=$(gh pr view "$pr" --json number,title,state,isDraft,author,baseRefName,headRefName,headRefOid,headRepository,headRepositoryOwner,url,body,labels,assignees,changedFiles,additions,deletions,statusCheckRollup,files)
-  head_before=$(printf '%s\n' "$metadata" | jq -r .headRefOid)
-  expected_file_count=$(printf '%s\n' "$metadata" | jq -r .changedFiles)
+  local metadata files expected_file_count actual_file_count head_before head_after head_after_json
+  metadata=$(read_pr_view_json "$pr" "number,title,state,isDraft,author,baseRefName,headRefName,headRefOid,headRepository,headRepositoryOwner,url,body,labels,assignees,changedFiles,additions,deletions,statusCheckRollup,files") || return 1
+  head_before=$(pr_view_string_field "$metadata" "headRefOid" "$pr" "Retry review initialization.") || return 1
+  if ! expected_file_count=$(printf '%s\n' "$metadata" | jq -er '.changedFiles | if type == "number" and . >= 0 and . == floor then . else error("invalid changed file count") end' 2>/dev/null); then
+    echo "Invalid PR metadata for #$pr: changedFiles must be a non-negative integer." >&2
+    return 1
+  fi
 
   # `gh pr view --json files` is cacheable but stops at 100 entries. Use it
   # when complete; only large or incomplete responses spend uncached REST quota.
@@ -327,7 +330,8 @@ pr_meta_json() {
     fi
   fi
 
-  head_after=$(gh pr view "$pr" --json headRefOid | jq -r .headRefOid)
+  head_after_json=$(read_pr_view_json "$pr" "headRefOid") || return 1
+  head_after=$(pr_view_string_field "$head_after_json" "headRefOid" "$pr" "Retry review initialization.") || return 1
   if [ "$head_after" != "$head_before" ]; then
     echo "PR head changed while collecting file metadata for #$pr (started at $head_before, ended at $head_after). Retry review initialization." >&2
     return 1

test/scripts/pr-merge.test.ts

@@ -5,6 +5,7 @@ import { afterEach, describe, expect, it } from "vitest";
 import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js";
 
 const tempDirs = useAutoCleanupTempDirTracker(afterEach);
+const commonScript = join(process.cwd(), "scripts/pr-lib/common.sh");
 const mergeScript = join(process.cwd(), "scripts/pr-lib/merge.sh");
 const headSha = "0123456789abcdef0123456789abcdef01234567";
 const landedSha = "fedcba9876543210fedcba9876543210fedcba98";
@@ -89,6 +90,7 @@ process.exit(new RegExp(pattern, flags).test(readFileSync(file, "utf8")) ? 0 : 1
 
   const shell = `
 set -euo pipefail
+source "$OPENCLAW_TEST_COMMON_SCRIPT"
 source "$OPENCLAW_TEST_MERGE_SCRIPT"
 script_parent_dir="$OPENCLAW_TEST_SCRIPTS_DIR"
 enter_worktree() { :; }
@@ -259,6 +261,7 @@ merge_run 123 "$OPENCLAW_TEST_AUTO_REQUESTED"
       OPENCLAW_TEST_COMMENT_BODY: commentBody,
       OPENCLAW_TEST_COMMENT_EMPTY: scenario.commentEmpty ? "true" : "false",
       OPENCLAW_TEST_COMMENT_FAILURES: String(scenario.commentFailures ?? 0),
+      OPENCLAW_TEST_COMMON_SCRIPT: commonScript,
       OPENCLAW_TEST_DISABLED_AUTO_META: disabledAutoMeta,
       OPENCLAW_TEST_GH_CALLS: calls,
       OPENCLAW_TEST_LANDED_SHA: landedSha,

test/scripts/pr-metadata.test.ts

@@ -16,16 +16,41 @@ function createFakeGh(): string {
 set -euo pipefail
 
 if [ "$1" = "pr" ] && [ "$2" = "view" ]; then
+  pr_view_count=0
+  if [ -f "$FAKE_PR_VIEW_COUNT_FILE" ]; then
+    IFS= read -r pr_view_count < "$FAKE_PR_VIEW_COUNT_FILE"
+  fi
+  pr_view_count=$((pr_view_count + 1))
+  printf '%s\n' "$pr_view_count" > "$FAKE_PR_VIEW_COUNT_FILE"
+
+  failure_target_matches=0
+  if [ "\${FAKE_PR_VIEW_FAILURE_TARGET:-all}" = "all" ] || {
+    [ "\${FAKE_PR_VIEW_FAILURE_TARGET:-all}" = "head" ] && [[ "$*" != *changedFiles* ]]
+  }; then
+    failure_target_matches=1
+  fi
+  if [ -n "\${FAKE_PR_VIEW_FAILURE_MODE:-}" ] && [ "$failure_target_matches" = "1" ] && {
+    [ "\${FAKE_PR_VIEW_FAILURE_COUNT:--1}" = "-1" ] || [ "$pr_view_count" -le "\${FAKE_PR_VIEW_FAILURE_COUNT}" ]
+  }; then
+    echo "HTTP 503: No server is currently available to service your request. (https://api.github.com/graphql)" >&2
+    case "$FAKE_PR_VIEW_FAILURE_MODE" in
+      empty) exit 0 ;;
+      exit) exit 7 ;;
+      non-json) printf 'upstream unavailable\n'; exit 0 ;;
+      null) printf 'null\n'; exit 0 ;;
+    esac
+  fi
+
   if [[ "$*" == *changedFiles* ]]; then
     if [ "\${FAKE_REJECT_REVIEW_REQUESTS:-0}" = "1" ] && [[ "$*" == *reviewRequests* ]]; then
       echo "GraphQL: Resource not accessible by integration (repository.pullRequest.reviewRequests.nodes.0.requestedReviewer)" >&2
       exit 1
     fi
-    jq -nc --argjson changedFiles "\${FAKE_CHANGED_FILES:-101}" --argjson fileCount "\${FAKE_GRAPHQL_FILE_COUNT:-100}" --argjson includeChangeType "\${FAKE_GRAPHQL_CHANGE_TYPE:-true}" '
+    jq -nc --arg headRefOid "\${FAKE_HEAD_BEFORE-head-a}" --argjson changedFiles "\${FAKE_CHANGED_FILES:-101}" --argjson fileCount "\${FAKE_GRAPHQL_FILE_COUNT:-100}" --argjson includeChangeType "\${FAKE_GRAPHQL_CHANGE_TYPE:-true}" '
       {
         number: 42,
         url: "https://example.test/pr/42",
-        headRefOid: "head-a",
+        headRefOid: $headRefOid,
         changedFiles: $changedFiles,
         files: [
           range(0; $fileCount)
@@ -77,6 +102,10 @@ function readPrMetadata(
     graphqlChangeType?: boolean;
     graphqlFileCount?: string;
     headAfter?: string;
+    headBefore?: string;
+    prViewFailureCount?: string;
+    prViewFailureMode?: "empty" | "exit" | "non-json" | "null";
+    prViewFailureTarget?: "all" | "head";
     rejectReviewRequests?: boolean;
     restFileCount?: string;
   } = {},
@@ -85,7 +114,7 @@ function readPrMetadata(
     "bash",
     [
       "-c",
-      "set -euo pipefail; source scripts/lib/plain-gh.sh; source scripts/pr-lib/worktree.sh; pr_meta_json 42",
+      "set -euo pipefail; source scripts/lib/plain-gh.sh; source scripts/pr-lib/worktree.sh; source scripts/pr-lib/common.sh; pr_meta_json 42",
     ],
     {
       cwd: process.cwd(),
@@ -96,6 +125,11 @@ function readPrMetadata(
         FAKE_GRAPHQL_CHANGE_TYPE: options.graphqlChangeType === false ? "false" : "true",
         FAKE_GRAPHQL_FILE_COUNT: options.graphqlFileCount ?? "100",
         FAKE_HEAD_AFTER: options.headAfter ?? "head-a",
+        FAKE_HEAD_BEFORE: options.headBefore ?? "head-a",
+        FAKE_PR_VIEW_COUNT_FILE: join(fakeGhDir, "pr-view-count"),
+        FAKE_PR_VIEW_FAILURE_COUNT: options.prViewFailureCount ?? "-1",
+        FAKE_PR_VIEW_FAILURE_MODE: options.prViewFailureMode ?? "",
+        FAKE_PR_VIEW_FAILURE_TARGET: options.prViewFailureTarget ?? "all",
         FAKE_REJECT_REVIEW_REQUESTS: options.rejectReviewRequests ? "1" : "0",
         FAKE_REST_FILE_COUNT: options.restFileCount ?? "101",
         OPENCLAW_GH_BIN: join(fakeGhDir, "gh"),
@@ -216,4 +250,74 @@ describe("PR metadata", () => {
       "PR head changed while collecting file metadata for #42 (started at head-a, ended at head-b). Retry review initialization.",
     );
   });
+
+  it("rejects metadata without an observed initial head SHA", () => {
+    const result = readPrMetadata(createFakeGh(), {
+      changedFiles: "2",
+      graphqlFileCount: "2",
+      headBefore: "",
+    });
+
+    expect(result.status).toBe(1);
+    expect(result.stderr).toContain(
+      "GitHub PR metadata for #42 did not include a head SHA. Retry review initialization.",
+    );
+    expect(result.stderr).not.toContain("PR head changed");
+  });
+
+  it("rejects a non-numeric changed file count before shell comparison", () => {
+    const result = readPrMetadata(createFakeGh(), { changedFiles: "null" });
+
+    expect(result.status).toBe(1);
+    expect(result.stderr).toContain(
+      "Invalid PR metadata for #42: changedFiles must be a non-negative integer.",
+    );
+    expect(result.stderr).not.toContain("integer expected");
+    expect(result.stderr).not.toContain("integer expression expected");
+  });
+
+  it.each([
+    ["empty stdout", "empty", "returned empty stdout"],
+    ["a non-zero exit", "exit", "exited with status 7"],
+    ["non-JSON stdout", "non-json", "did not return one JSON object"],
+    ["a non-object JSON value", "null", "did not return one JSON object"],
+  ] as const)("reports a GitHub API failure for %s", (_label, prViewFailureMode, detail) => {
+    const result = readPrMetadata(createFakeGh(), { prViewFailureMode });
+
+    expect(result.status).toBe(1);
+    expect(result.stderr).toContain(
+      `GitHub API failure while reading PR #42: gh pr view ${detail}`,
+    );
+    expect(result.stderr).toContain("HTTP 503: No server is currently available");
+    expect(result.stderr).not.toContain("integer expected");
+    expect(result.stderr).not.toContain("PR head changed");
+  });
+
+  it("reports an API failure when the post-collection head read fails", () => {
+    const result = readPrMetadata(createFakeGh(), {
+      changedFiles: "2",
+      graphqlFileCount: "2",
+      prViewFailureMode: "empty",
+      prViewFailureTarget: "head",
+    });
+
+    expect(result.status).toBe(1);
+    expect(result.stderr).toContain(
+      "GitHub API failure while reading PR #42: gh pr view returned empty stdout",
+    );
+    expect(result.stderr).not.toContain("PR head changed");
+  });
+
+  it("recovers when a transient API failure is followed by a valid PR object", () => {
+    const result = readPrMetadata(createFakeGh(), {
+      changedFiles: "2",
+      graphqlFileCount: "2",
+      prViewFailureCount: "1",
+      prViewFailureMode: "empty",
+    });
+
+    expect(result.status).toBe(0);
+    expect(result.stderr).toBe("");
+    expect(JSON.parse(result.stdout)).toMatchObject({ headRefOid: "head-a", changedFiles: 2 });
+  });
 });

test/scripts/pr-wrappers.test.ts

@@ -34,7 +34,7 @@ function makeMismatchedWrapperRepo() {
   mkdirSync(bin, { recursive: true });
   mkdirSync(home, { recursive: true });
   // This fixture exercises wrapper trust routing, not the host command inventory.
-  for (const command of ["jq", "pnpm", "rg"]) {
+  for (const command of ["pnpm", "rg"]) {
     const commandPath = join(bin, command);
     writeFileSync(commandPath, "#!/bin/sh\nexit 0\n");
     chmodSync(commandPath, 0o755);
@@ -44,7 +44,7 @@ function makeMismatchedWrapperRepo() {
   const ghStub = join(bin, "gh");
   writeFileSync(
     ghStub,
-    '#!/bin/sh\nif [ "$1" = "pr" ] && [ "$2" = "view" ]; then\n  echo not-main\n  exit 0\nfi\nexit 0\n',
+    '#!/bin/sh\nif [ "$1" = "pr" ] && [ "$2" = "view" ]; then\n  printf \'{"baseRefName":"not-main"}\\n\'\n  exit 0\nfi\nexit 0\n',
   );
   chmodSync(ghStub, 0o755);
 
@@ -219,13 +219,15 @@ describe("scripts/pr wrappers", () => {
 
   it("routes cached reads and writer-sensitive operations through their owning gh seams", () => {
     const script = readScript("scripts/pr");
+    const common = readScript("scripts/pr-lib/common.sh");
     const worktree = readScript("scripts/pr-lib/worktree.sh");
     const review = readScript("scripts/pr-lib/review.sh");
     const push = readScript("scripts/pr-lib/push.sh");
     const merge = readScript("scripts/pr-lib/merge.sh");
 
-    expect(script).toContain('base=$(gh pr view "$pr" --json baseRefName --jq .baseRefName)');
-    expect(worktree).toContain('metadata=$(gh pr view "$pr" --json');
+    expect(script).toContain('base_json=$(read_pr_view_json "$pr" "baseRefName")');
+    expect(common).toContain('gh pr view "$pr" --json "$fields"');
+    expect(worktree).toContain('metadata=$(read_pr_view_json "$pr"');
     expect(worktree).toContain('gh_plain api --paginate "repos/{owner}/{repo}/pulls/$pr/files');
     expect(review).toContain("reviewer=$(gh_plain api user --jq .login");
     expect(review).toContain('gh_plain pr edit "$pr" --add-assignee "$reviewer"');