diff --git a/scripts/pr b/scripts/pr index c363313d1cdd..977e226c2bbb 100755 --- a/scripts/pr +++ b/scripts/pr @@ -43,31 +43,31 @@ requested_subcommand="${1-}" # script from the repository root so behavior stays consistent across worktrees. script_self="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/$(basename "${BASH_SOURCE[0]}")" script_parent_dir="$(dirname "$script_self")" +pr_wrapper_components=( + scripts/pr + scripts/pr-lib + scripts/lib/plain-gh.sh + scripts/lib/plain-gh.mjs + scripts/lib/direct-run.mjs + scripts/watch-pr-ci.mjs +) if common_git_dir=$(git -C "$script_parent_dir" rev-parse --path-format=absolute --git-common-dir 2>/dev/null); then canonical_repo_root="$(dirname "$common_git_dir")" canonical_self="$canonical_repo_root/scripts/$(basename "${BASH_SOURCE[0]}")" if [ "$script_self" != "$canonical_self" ] && [ -x "$canonical_self" ]; then - if ! git -C "$script_parent_dir" diff --quiet HEAD -- \ - ":(top)scripts/pr" ":(top)scripts/pr-lib" ":(top)scripts/lib/plain-gh.sh"; then + if ! git -C "$script_parent_dir" diff --quiet HEAD -- "${pr_wrapper_components[@]/#/:(top)}"; then echo "scripts/pr wrapper files have uncommitted changes in this worktree." >&2 echo "Refusing to run unreviewed wrapper code from: $script_parent_dir" >&2 exit 1 fi linked_wrapper_revision=$( - git -C "$script_parent_dir" rev-parse \ - HEAD:scripts/pr \ - HEAD:scripts/pr-lib \ - HEAD:scripts/lib/plain-gh.sh 2>/dev/null || true + git -C "$script_parent_dir" rev-parse "${pr_wrapper_components[@]/#/HEAD:}" 2>/dev/null || true ) canonical_wrapper_revision=$( - git -C "$canonical_repo_root" rev-parse \ - HEAD:scripts/pr \ - HEAD:scripts/pr-lib \ - HEAD:scripts/lib/plain-gh.sh 2>/dev/null || true + git -C "$canonical_repo_root" rev-parse "${pr_wrapper_components[@]/#/HEAD:}" 2>/dev/null || true ) canonical_wrapper_clean=1 - if ! git -C "$canonical_repo_root" diff --quiet HEAD -- \ - ":(top)scripts/pr" ":(top)scripts/pr-lib" ":(top)scripts/lib/plain-gh.sh"; then + if ! git -C "$canonical_repo_root" diff --quiet HEAD -- "${pr_wrapper_components[@]/#/:(top)}"; then canonical_wrapper_clean=0 fi if [ -n "$linked_wrapper_revision" ] && @@ -82,10 +82,7 @@ if common_git_dir=$(git -C "$script_parent_dir" rev-parse --path-format=absolute # refs/remotes/... explicitly: bare "origin/main" is a DWIM name that a # local branch or tag named origin/main could shadow, spoofing the anchor. anchor_wrapper_revision=$( - git -C "$script_parent_dir" rev-parse \ - refs/remotes/origin/main:scripts/pr \ - refs/remotes/origin/main:scripts/pr-lib \ - refs/remotes/origin/main:scripts/lib/plain-gh.sh 2>/dev/null || true + git -C "$script_parent_dir" rev-parse "${pr_wrapper_components[@]/#/refs/remotes/origin/main:}" 2>/dev/null || true ) if [ -z "$linked_wrapper_revision" ] || [ -z "$anchor_wrapper_revision" ] || @@ -104,10 +101,10 @@ if common_git_dir=$(git -C "$script_parent_dir" rev-parse --path-format=absolute echo "subcommand '$requested_subcommand' is classified $requested_classification; dev-wrapper opt-in is unavailable." >&2 fi # HEAD blobs are authoritative here: the uncommitted-wrapper guard above - # already exited for any staged or unstaged edit to these three paths, so + # already exited for any staged or unstaged edit to these paths, so # the working tree matches HEAD and this list matches what was rejected. differing_wrapper_components=() - for wrapper_component in scripts/pr scripts/pr-lib scripts/lib/plain-gh.sh; do + for wrapper_component in "${pr_wrapper_components[@]}"; do linked_component_revision=$(git -C "$script_parent_dir" rev-parse "HEAD:$wrapper_component" 2>/dev/null || true) anchor_component_revision=$(git -C "$script_parent_dir" rev-parse "refs/remotes/origin/main:$wrapper_component" 2>/dev/null || true) if [ -z "$linked_component_revision" ] || @@ -206,13 +203,13 @@ USAGE require_cmds() { local missing=() local cmd - for cmd in git jq rg pnpm node; do + for cmd in git gh jq rg pnpm node; do if ! command -v "$cmd" >/dev/null 2>&1; then missing+=("$cmd") fi done if ! OPENCLAW_GH_BIN="$(resolve_plain_gh_bin)"; then - missing+=("gh") + missing+=("real-gh") else export OPENCLAW_GH_BIN fi @@ -226,10 +223,6 @@ require_cmds() { fi } -gh() { - gh_plain "$@" -} - require_main_target_pr() { local pr="$1" local base diff --git a/scripts/pr-lib/ci-dispatch.mjs b/scripts/pr-lib/ci-dispatch.mjs index a06deda97a17..91b5948d2da9 100644 --- a/scripts/pr-lib/ci-dispatch.mjs +++ b/scripts/pr-lib/ci-dispatch.mjs @@ -2,7 +2,7 @@ import { spawnSync } from "node:child_process"; import { isDirectRunUrl } from "../lib/direct-run.mjs"; -import { execPlainGh } from "../lib/plain-gh.mjs"; +import { execGhJson, execGhRead, execPlainGh } from "../lib/plain-gh.mjs"; const SHA_PATTERN = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/u; @@ -41,29 +41,27 @@ function buildCiDispatchArgs(record) { } function listCiRuns(headRefOid) { - return JSON.parse( - execPlainGh( - [ - "run", - "list", - "--commit", - headRefOid, - "--workflow", - "ci.yml", - "--event", - "workflow_dispatch", - "--limit", - "20", - "--json", - "databaseId,url,headSha,createdAt,status", - ], - { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }, - ), + return execGhJson( + [ + "run", + "list", + "--commit", + headRefOid, + "--workflow", + "ci.yml", + "--event", + "workflow_dispatch", + "--limit", + "20", + "--json", + "databaseId,url,headSha,createdAt,status", + ], + { stdio: ["ignore", "pipe", "pipe"] }, ); } function readCurrentPrHeadOid(pr) { - return execPlainGh(["pr", "view", String(pr), "--json", "headRefOid", "--jq", ".headRefOid"], { + return execGhRead(["pr", "view", String(pr), "--json", "headRefOid", "--jq", ".headRefOid"], { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"], }).trim(); diff --git a/scripts/pr-lib/merge.sh b/scripts/pr-lib/merge.sh index 28c7af17089c..6ba06e6913fb 100644 --- a/scripts/pr-lib/merge.sh +++ b/scripts/pr-lib/merge.sh @@ -134,12 +134,15 @@ merge_verify() { fi mark_pr_operation_side_effects_started - gh pr checks "$pr" --required --watch --fail-fast >.local/merge-checks-watch.log 2>&1 || true + # Wait only for the attached CI workflow here. The direct required-check + # query below remains the merge authority, so optional contexts cannot stall it. + node "$script_parent_dir/watch-pr-ci.mjs" "$pr" "$PREP_HEAD_SHA" \ + --completion ci-run >.local/merge-checks-watch.log 2>&1 || true local checks_json local checks_err_file local checks_exit_status checks_err_file=$(mktemp) - if checks_json=$(gh pr checks "$pr" --required --json name,bucket,state 2>"$checks_err_file"); then + if checks_json=$(gh_plain pr checks "$pr" --required --json name,bucket,state 2>"$checks_err_file"); then checks_exit_status=0 else checks_exit_status=$? @@ -268,7 +271,7 @@ merge_run() { local encoded_ref encoded_ref=$(jq -rn --arg value "heads/$head_ref" '$value|@uri') - if gh api -X DELETE "repos/$repo_owner/$repo_name/git/refs/$encoded_ref" >/dev/null 2>&1; then + if gh_plain api -X DELETE "repos/$repo_owner/$repo_name/git/refs/$encoded_ref" >/dev/null 2>&1; then return 0 fi @@ -331,7 +334,7 @@ merge_run() { if [ -n "$existing_auto_method" ]; then echo "Auto-merge is already enabled with $existing_auto_method; re-arming it as pinned SQUASH." - if ! gh pr merge "$pr" --disable-auto >.local/merge-output.log 2>&1; then + if ! gh_plain pr merge "$pr" --disable-auto >.local/merge-output.log 2>&1; then print_relevant_log_excerpt .local/merge-output.log exit 1 fi @@ -356,7 +359,7 @@ merge_run() { else # GitHub's EnablePullRequestAutoMergeInput contract keeps expectedHeadOid # as the head that must match to allow the eventual merge. - if gh pr merge "$pr" \ + if gh_plain pr merge "$pr" \ --auto \ --squash \ --match-head-commit "$PREP_HEAD_SHA" \ @@ -387,7 +390,7 @@ merge_run() { existing_auto_method=$(printf '%s\n' "$auto_meta" | jq -r '.autoMergeRequest.mergeMethod // ""') if [ "$auto_head_sha" = "$PREP_HEAD_SHA" ] && [ -n "$existing_auto_method" ]; then echo "Auto-merge enablement was inconclusive; clearing the observed $existing_auto_method request to fail closed." - if ! gh pr merge "$pr" --disable-auto >>.local/merge-output.log 2>&1; then + if ! gh_plain pr merge "$pr" --disable-auto >>.local/merge-output.log 2>&1; then print_relevant_log_excerpt .local/merge-output.log exit 1 fi @@ -412,7 +415,7 @@ merge_run() { fi if [ "$merge_submitted" != "true" ]; then - if ! gh pr merge "$pr" \ + if ! gh_plain pr merge "$pr" \ "$merge_flag" \ --match-head-commit "$PREP_HEAD_SHA" \ >.local/merge-output.log 2>&1 @@ -484,7 +487,7 @@ merge_run() { echo echo "- Prepared head SHA: [$PREP_HEAD_SHA]($prep_sha_url)" echo "- Landed commit: [$landed_sha]($landed_sha_url)" - } | gh pr comment "$pr" -F - 2>&1 + } | gh_plain pr comment "$pr" -F - 2>&1 ); then ok=1 break diff --git a/scripts/pr-lib/push.sh b/scripts/pr-lib/push.sh index 82516c366dd1..3387eeddfe5e 100644 --- a/scripts/pr-lib/push.sh +++ b/scripts/pr-lib/push.sh @@ -159,7 +159,7 @@ GRAPHQL rm -f "$variables_file" local result - result=$(gh api graphql --input - <<< "$payload" 2>&1) || { + result=$(gh_plain api graphql --input - <<< "$payload" 2>&1) || { echo "GraphQL push failed: $result" >&2 return 1 } diff --git a/scripts/pr-lib/review.sh b/scripts/pr-lib/review.sh index 550ca08b06d5..9e1c064b82e0 100644 --- a/scripts/pr-lib/review.sh +++ b/scripts/pr-lib/review.sh @@ -32,7 +32,7 @@ review_claim() { local user_log user_log=".local/review-claim-user-attempt-$attempt.log" - if reviewer=$(gh api user --jq .login 2>"$user_log"); then + if reviewer=$(gh_plain api user --jq .login 2>"$user_log"); then printf "%s\n" "$reviewer" >"$user_log" break fi @@ -54,7 +54,7 @@ review_claim() { local claim_log claim_log=".local/review-claim-assignee-attempt-$attempt.log" - if gh pr edit "$pr" --add-assignee "$reviewer" >"$claim_log" 2>&1; then + if gh_plain pr edit "$pr" --add-assignee "$reviewer" >"$claim_log" 2>&1; then echo "review claim succeeded: @$reviewer assigned to PR #$pr" return 0 fi diff --git a/scripts/pr-lib/worktree.sh b/scripts/pr-lib/worktree.sh index 323fa63e65ad..61508f8f3dbd 100644 --- a/scripts/pr-lib/worktree.sh +++ b/scripts/pr-lib/worktree.sh @@ -140,7 +140,7 @@ pr_meta_json() { if [ "$actual_file_count" -ne "$expected_file_count" ]; then if ! files=$( set -o pipefail - gh api --paginate "repos/{owner}/{repo}/pulls/$pr/files?per_page=100" | + gh_plain api --paginate "repos/{owner}/{repo}/pulls/$pr/files?per_page=100" | jq -cs ' add | map({ diff --git a/scripts/watch-pr-ci.d.mts b/scripts/watch-pr-ci.d.mts index 778a3256b0d7..e4e39e2cb841 100644 --- a/scripts/watch-pr-ci.d.mts +++ b/scripts/watch-pr-ci.d.mts @@ -6,6 +6,7 @@ export interface WatchPrCiArgs { attachTimeout: number; timeout: number; interval: number; + completion: "rollup" | "ci-run"; } export interface RollupCheck { @@ -59,6 +60,10 @@ export interface RunAttachmentClassification { warning?: string; } +export type AttachedCiRunClassification = + | { verdict: "PENDING" | "GREEN" } + | { verdict: "FAILING"; conclusion: string }; + export interface PollUntilDeadlineOptions { deadline: number; interval: number; @@ -80,4 +85,5 @@ export function classifyRunAttachment( run: RunStatus, after?: number, ): RunAttachmentClassification; +export function classifyAttachedCiRun(run: RunStatus): AttachedCiRunClassification; export function pollUntilDeadline(options: PollUntilDeadlineOptions): Promise; diff --git a/scripts/watch-pr-ci.mjs b/scripts/watch-pr-ci.mjs index 36ac82d90635..21ff3ef3f197 100644 --- a/scripts/watch-pr-ci.mjs +++ b/scripts/watch-pr-ci.mjs @@ -5,7 +5,7 @@ import { isDirectRunUrl } from "./lib/direct-run.mjs"; import { execGhJson } from "./lib/plain-gh.mjs"; const USAGE = - "Usage: node scripts/watch-pr-ci.mjs [--repo owner/repo] [--after run-id] [--attach-timeout 900] [--timeout 3600] [--interval 120]"; + "Usage: node scripts/watch-pr-ci.mjs [--repo owner/repo] [--after run-id] [--attach-timeout 900] [--timeout 3600] [--interval 120] [--completion rollup|ci-run]"; const FAILURE_CONCLUSIONS = new Set([ "ACTION_REQUIRED", "CANCELLED", @@ -51,6 +51,7 @@ export function parseArgs(argv) { "attach-timeout": { type: "string", default: "900" }, timeout: { type: "string", default: "3600" }, interval: { type: "string", default: "120" }, + completion: { type: "string", default: "rollup" }, }, }); } catch { @@ -67,6 +68,7 @@ export function parseArgs(argv) { attachTimeout: positiveInteger(parsed.values["attach-timeout"], "--attach-timeout"), timeout: positiveInteger(parsed.values.timeout, "--timeout"), interval: positiveInteger(parsed.values.interval, "--interval"), + completion: parsed.values.completion, }; if (parsed.values.after !== undefined) { args.after = positiveInteger(parsed.values.after, "--after"); @@ -77,6 +79,9 @@ export function parseArgs(argv) { if (!/^[^/\s]+\/[^/\s]+$/u.test(args.repo)) { throw new Error("--repo must be owner/repo"); } + if (!new Set(["rollup", "ci-run"]).has(args.completion)) { + throw new Error("--completion must be rollup or ci-run"); + } return args; } @@ -255,6 +260,15 @@ export function classifyRunAttachment(runId, run, after) { }; } +export function classifyAttachedCiRun(run) { + if (run.status !== "completed") { + return { verdict: "PENDING" }; + } + return run.conclusion === "success" + ? { verdict: "GREEN" } + : { verdict: "FAILING", conclusion: run.conclusion ?? "unknown" }; +} + export function collectRollupContexts(fetchPage) { const firstPage = fetchPage(null); const firstContexts = firstPage?.statusCheckRollup?.contexts; @@ -419,6 +433,24 @@ async function main(argv = process.argv.slice(2)) { interval: args.interval, poll: () => { try { + if (args.completion === "ci-run") { + const blocked = precheck(readPr(args.pr, args.repo), args.headSha, true); + if (blocked !== null) { + return blocked; + } + const run = readRun(args.repo, runId); + const result = classifyAttachedCiRun(run); + console.log( + `STATUS run=${String(run.status)} conclusion=${String(run.conclusion ?? "pending")}`, + ); + if (result.verdict === "FAILING") { + return emit(`FAILING checks=CI workflow (${result.conclusion})`, 15); + } + if (result.verdict === "GREEN") { + return emit("GREEN", 0); + } + return undefined; + } const pr = readRollup(args.pr, args.repo); const blocked = precheck(pr, args.headSha, true); if (blocked !== null) { diff --git a/test/scripts/pr-ci-dispatch.test.ts b/test/scripts/pr-ci-dispatch.test.ts index 3d803288e777..6ce7b12af6ca 100644 --- a/test/scripts/pr-ci-dispatch.test.ts +++ b/test/scripts/pr-ci-dispatch.test.ts @@ -1,5 +1,5 @@ import { spawnSync } from "node:child_process"; -import { chmodSync, existsSync, readFileSync, writeFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js"; @@ -12,15 +12,16 @@ const describePosix = process.platform === "win32" ? describe.skip : describe; function createFakeGh() { const tempDir = tempDirs.make("openclaw-pr-ci-dispatch-"); - const fakeGh = join(tempDir, "gh"); + const binDir = join(tempDir, "bin"); + const pathGh = join(binDir, "gh"); + const realGh = join(tempDir, "real-gh"); const calls = join(tempDir, "calls.log"); const dispatched = join(tempDir, "dispatched"); const seenRunList = join(tempDir, "seen-run-list"); - writeFileSync( - fakeGh, - `#!/usr/bin/env bash + mkdirSync(binDir); + const fakeGhScript = `#!/usr/bin/env bash set -euo pipefail -printf '%s\\n' "$*" >> "$OPENCLAW_TEST_GH_CALLS" +printf '%s\\t%s\\n' "$(basename "$0")" "$*" >> "$OPENCLAW_TEST_GH_CALLS" case "$1 $2" in "run list") if [ "\${OPENCLAW_TEST_GH_MODE:-}" = "pending-head-change" ]; then @@ -42,10 +43,12 @@ case "$1 $2" in "workflow run") : > "$OPENCLAW_TEST_GH_DISPATCHED" ;; *) echo "unexpected gh invocation: $*" >&2; exit 2 ;; esac -`, - ); - chmodSync(fakeGh, 0o755); - return { calls, dispatched, fakeGh, seenRunList }; +`; + writeFileSync(pathGh, fakeGhScript); + writeFileSync(realGh, fakeGhScript); + chmodSync(pathGh, 0o755); + chmodSync(realGh, 0o755); + return { binDir, calls, dispatched, realGh, seenRunList }; } function runDispatch( @@ -71,13 +74,14 @@ function runDispatch( env: { ...process.env, NODE_OPTIONS: nodeOptions, - OPENCLAW_GH_BIN: fakeGh.fakeGh, + OPENCLAW_GH_BIN: fakeGh.realGh, OPENCLAW_TEST_CHANGED_HEAD_SHA: changedSha, OPENCLAW_TEST_GH_CALLS: fakeGh.calls, OPENCLAW_TEST_GH_DISPATCHED: fakeGh.dispatched, OPENCLAW_TEST_GH_MODE: options.mode ?? "", OPENCLAW_TEST_GH_SEEN_RUN_LIST: fakeGh.seenRunList, OPENCLAW_TEST_HEAD_SHA: sha, + PATH: `${fakeGh.binDir}:${process.env.PATH ?? ""}`, }, }, ); @@ -119,9 +123,14 @@ describePosix("scripts/pr ci-dispatch", () => { expect(result.stdout).toContain( "observed_run_url=https://github.com/openclaw/openclaw/actions/runs/99", ); - expect(readFileSync(fakeGh.calls, "utf8")).toContain( - `workflow run ci.yml --ref contributor/fix-hosted-gates -f target_ref=${sha} -f release_gate=true -f pull_request_number=12345`, + const calls = readFileSync(fakeGh.calls, "utf8"); + const callLines = calls.trim().split("\n"); + expect(callLines).toContain( + `real-gh\tworkflow run ci.yml --ref contributor/fix-hosted-gates -f target_ref=${sha} -f release_gate=true -f pull_request_number=12345`, ); + expect(callLines.some((call) => call.startsWith(`gh\trun list --commit ${sha}`))).toBe(true); + expect(callLines.some((call) => call.startsWith("gh\tpr view 12345"))).toBe(true); + expect(callLines.some((call) => /^real-gh\t(?:run list|pr view)/u.test(call))).toBe(false); }); it("refuses a fork-local branch name before invoking GitHub", () => { @@ -133,11 +142,12 @@ describePosix("scripts/pr ci-dispatch", () => { encoding: "utf8", env: { ...process.env, - OPENCLAW_GH_BIN: fakeGh.fakeGh, + OPENCLAW_GH_BIN: fakeGh.realGh, OPENCLAW_TEST_GH_CALLS: fakeGh.calls, OPENCLAW_TEST_GH_DISPATCHED: fakeGh.dispatched, OPENCLAW_TEST_GH_SEEN_RUN_LIST: fakeGh.seenRunList, OPENCLAW_TEST_HEAD_SHA: sha, + PATH: `${fakeGh.binDir}:${process.env.PATH ?? ""}`, }, }, ); diff --git a/test/scripts/pr-merge.test.ts b/test/scripts/pr-merge.test.ts index 40e37d479ee2..88f9c2dd52a2 100644 --- a/test/scripts/pr-merge.test.ts +++ b/test/scripts/pr-merge.test.ts @@ -85,6 +85,7 @@ process.exit(new RegExp(pattern, flags).test(readFileSync(file, "utf8")) ? 0 : 1 const shell = ` set -euo pipefail source "$OPENCLAW_TEST_MERGE_SCRIPT" +script_parent_dir="$OPENCLAW_TEST_SCRIPTS_DIR" enter_worktree() { :; } require_artifact() { :; } validate_review_artifact_data() { @@ -118,8 +119,17 @@ git() { fi return 0 } -gh() { - printf '%s\\n' "$*" >> "$OPENCLAW_TEST_GH_CALLS" +node() { + if [[ "\${1-}" = */scripts/watch-pr-ci.mjs ]]; then + printf 'watch %s\\n' "$*" >> "$OPENCLAW_TEST_GH_CALLS" + return 0 + fi + command node "$@" +} +gh_route() { + local route="$1" + shift + printf '%s %s\\n' "$route" "$*" >> "$OPENCLAW_TEST_GH_CALLS" case "$1 $2" in "pr checks") case " $* " in @@ -185,6 +195,8 @@ gh() { *) echo "unexpected gh invocation: $*" >&2; return 2 ;; esac } +gh() { gh_route path "$@"; } +gh_plain() { gh_route plain "$@"; } merge_run 123 "$OPENCLAW_TEST_AUTO_REQUESTED" `; @@ -212,6 +224,7 @@ merge_run 123 "$OPENCLAW_TEST_AUTO_REQUESTED" OPENCLAW_TEST_REVIEW_RECOMMENDATION: scenario.recommendation ?? "ready", OPENCLAW_TEST_RG_CALLS: rgCalls, OPENCLAW_TEST_ROOT: root, + OPENCLAW_TEST_SCRIPTS_DIR: join(process.cwd(), "scripts"), PATH: `${bin}${delimiter}${process.env.PATH ?? ""}`, }, }); @@ -272,7 +285,11 @@ describePosix("scripts/pr merge-run", () => { const result = runMerge({ mergeStateStatus: "CLEAN" }); expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0); - expect(result.calls).toContain(`pr merge 123 --squash --match-head-commit ${headSha}`); + expect(result.calls).toContain(`plain pr merge 123 --squash --match-head-commit ${headSha}`); + expect(result.calls).toContain(`scripts/watch-pr-ci.mjs 123 ${headSha} --completion ci-run`); + expect(result.calls).toContain("plain pr checks 123 --required --json name,bucket,state"); + expect(result.calls).toContain("path pr view 123 --json state,isDraft"); + expect(result.calls).not.toContain("--required --watch"); expect(result.calls).not.toContain("--auto"); expect(result.stdout).toContain("merge-run complete for PR #123"); }); @@ -281,8 +298,10 @@ describePosix("scripts/pr merge-run", () => { const result = runMerge({ auto: true }); expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0); - expect(result.calls).toContain(`pr merge 123 --auto --squash --match-head-commit ${headSha}`); - expect(result.calls.match(/^pr merge /gmu)).toHaveLength(1); + expect(result.calls).toContain( + `plain pr merge 123 --auto --squash --match-head-commit ${headSha}`, + ); + expect(result.calls.match(/^plain pr merge /gmu)).toHaveLength(1); expect(result.stdout).toContain("AUTO-MERGE ENABLED"); expect(result.stdout).toContain("required checks and branch up-to-dateness"); }); diff --git a/test/scripts/pr-metadata.test.ts b/test/scripts/pr-metadata.test.ts index ec2c901b8f01..1fc807df0da1 100644 --- a/test/scripts/pr-metadata.test.ts +++ b/test/scripts/pr-metadata.test.ts @@ -78,7 +78,10 @@ function readPrMetadata( ) { return spawnSync( "bash", - ["-c", "set -euo pipefail; source scripts/pr-lib/worktree.sh; pr_meta_json 42"], + [ + "-c", + "set -euo pipefail; source scripts/lib/plain-gh.sh; source scripts/pr-lib/worktree.sh; pr_meta_json 42", + ], { cwd: process.cwd(), env: { @@ -89,6 +92,7 @@ function readPrMetadata( FAKE_GRAPHQL_FILE_COUNT: options.graphqlFileCount ?? "100", FAKE_HEAD_AFTER: options.headAfter ?? "head-a", FAKE_REST_FILE_COUNT: options.restFileCount ?? "101", + OPENCLAW_GH_BIN: join(fakeGhDir, "gh"), PATH: `${fakeGhDir}:${process.env.PATH}`, }, encoding: "utf8", diff --git a/test/scripts/pr-operation-lock.test.ts b/test/scripts/pr-operation-lock.test.ts index 104e4f32f01a..10dd8c715bff 100644 --- a/test/scripts/pr-operation-lock.test.ts +++ b/test/scripts/pr-operation-lock.test.ts @@ -172,7 +172,10 @@ function writeOperationFixture(repoDir: string, name: string, commands: string[] function installPrCliFixture(repoDir: string) { const files = [ "scripts/pr", + "scripts/watch-pr-ci.mjs", "scripts/lib/plain-gh.sh", + "scripts/lib/plain-gh.mjs", + "scripts/lib/direct-run.mjs", "scripts/pr-lib/worktree.sh", "scripts/pr-lib/operation-lock.sh", "scripts/pr-lib/process-group-runner.mjs", diff --git a/test/scripts/pr-prepare-gates.test.ts b/test/scripts/pr-prepare-gates.test.ts index 95817135a413..a626c6b3416d 100644 --- a/test/scripts/pr-prepare-gates.test.ts +++ b/test/scripts/pr-prepare-gates.test.ts @@ -710,7 +710,7 @@ describe("GraphQL fork publication", () => { const result = runGatesBash( [ - 'gh() { cat > .local/graphql-payload.json; printf \'%s\\n\' \'{"data":{"createCommitOnBranch":{"commit":{"oid":"signed-head","url":"https://example.test/commit"}}}}\'; }', + 'gh_plain() { cat > .local/graphql-payload.json; printf \'%s\\n\' \'{"data":{"createCommitOnBranch":{"commit":{"oid":"signed-head","url":"https://example.test/commit"}}}}\'; }', `graphql_push_to_fork example/repo topic ${headSha}`, 'test "$(jq -r .variables.input.message.headline .local/graphql-payload.json)" = "reviewed fixup"', 'test "$(jq -r .variables.input.message.body .local/graphql-payload.json)" = "Co-authored-by: Helper "', @@ -740,7 +740,7 @@ describe("GraphQL fork publication", () => { const result = runGatesBash( [ - "gh() { touch .local/gh-called; return 99; }", + "gh_plain() { touch .local/gh-called; return 99; }", `graphql_push_to_fork example/repo topic ${headSha}`, ].join("\n"), { cwd: repoDir, sourcePush: true }, @@ -779,7 +779,7 @@ describe("GraphQL fork publication", () => { const result = runGatesBash( [ - "gh() { touch .local/gh-called; return 99; }", + "gh_plain() { touch .local/gh-called; return 99; }", `graphql_push_to_fork example/repo topic ${headSha}`, ].join("\n"), { cwd: repoDir, sourcePush: true }, diff --git a/test/scripts/pr-review-artifact-validation.test.ts b/test/scripts/pr-review-artifact-validation.test.ts index 7a42b669706e..2b29c1189207 100644 --- a/test/scripts/pr-review-artifact-validation.test.ts +++ b/test/scripts/pr-review-artifact-validation.test.ts @@ -207,6 +207,7 @@ function runMergeVerification(checks: "api-error" | "invalid-json" | "no-require [ "set -euo pipefail", 'source "$1"', + 'script_parent_dir=$(cd "$(dirname "$1")/.." && pwd)', 'fixture_root="$2"', 'enter_worktree() { cd "$fixture_root"; }', 'require_artifact() { [ -s "$1" ]; }', @@ -214,7 +215,8 @@ function runMergeVerification(checks: "api-error" | "invalid-json" | "no-require `pr_meta_json() { printf '%s\\n' '{"isDraft":false,"headRefOid":"${head}"}'; }`, "mark_pr_operation_side_effects_started() { :; }", "git() { :; }", - `gh() { case "$*" in *"--json name,bucket,state"*) ${checksResponse};; *) return 0;; esac; }`, + "node() { :; }", + `gh_plain() { case "$*" in *"--json name,bucket,state"*) ${checksResponse};; *) return 0;; esac; }`, "merge_verify 42", ].join("\n"), "pr-merge-verification", diff --git a/test/scripts/pr-wrappers.test.ts b/test/scripts/pr-wrappers.test.ts index 92e34a0505bb..ffcfd9a8c71f 100644 --- a/test/scripts/pr-wrappers.test.ts +++ b/test/scripts/pr-wrappers.test.ts @@ -72,6 +72,9 @@ function makeMismatchedWrapperRepo() { mkdirSync(join(canonical, "scripts", "lib"), { recursive: true }); cpSync("scripts/pr-lib", join(canonical, "scripts", "pr-lib"), { recursive: true }); writeFileSync(join(canonical, "scripts", "pr"), readScript("scripts/pr")); + cpSync("scripts/watch-pr-ci.mjs", join(canonical, "scripts", "watch-pr-ci.mjs")); + cpSync("scripts/lib/plain-gh.mjs", join(canonical, "scripts", "lib", "plain-gh.mjs")); + cpSync("scripts/lib/direct-run.mjs", join(canonical, "scripts", "lib", "direct-run.mjs")); writeFileSync( join(canonical, "scripts", "lib", "plain-gh.sh"), "resolve_plain_gh_bin() { printf '/usr/bin/true\\n'; }\ngh_plain() { :; }\n", @@ -170,7 +173,12 @@ describe("scripts/pr wrappers", () => { expect(script).toContain("unset COLORTERM"); expect(script).toContain('source "$script_parent_dir/lib/plain-gh.sh"'); expect(script).toContain("OPENCLAW_GH_BIN="); - expect(script).toContain("gh_plain"); + expect(script).toContain("for cmd in git gh jq rg pnpm node"); + expect(script).toContain('missing+=("real-gh")'); + expect(script).not.toContain("gh() {"); + expect(script).toContain("scripts/watch-pr-ci.mjs"); + expect(script).toContain("scripts/lib/plain-gh.mjs"); + expect(script).toContain("scripts/lib/direct-run.mjs"); expect(script).toContain("scripts/pr review-init "); expect(script).toContain("scripts/pr prepare-run "); expect(script).toContain("scripts/pr ci-dispatch "); @@ -185,11 +193,29 @@ describe("scripts/pr wrappers", () => { expect(script).toContain("only support PRs targeting main"); }); + it("routes cached reads and writer-sensitive operations through their owning gh seams", () => { + const script = readScript("scripts/pr"); + 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(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"'); + expect(push).toContain('gh_plain api graphql --input - <<< "$payload"'); + expect(merge).toContain('gh_plain pr merge "$pr"'); + expect(merge).toContain('gh_plain pr comment "$pr"'); + expect(merge).toContain("gh_plain api -X DELETE"); + }); + itPosix("fails loudly at preflight when ripgrep is unavailable", () => { const fixture = makeMismatchedWrapperRepo(); try { rmSync(join(fixture.bin, "rg")); - for (const command of ["bash", "basename", "dirname", "git", "jq", "pnpm", "node"]) { + for (const command of ["bash", "basename", "dirname", "git", "gh", "jq", "pnpm", "node"]) { rmSync(join(fixture.bin, command), { force: true }); symlinkSync(resolveCommand(command), join(fixture.bin, command)); } @@ -343,6 +369,9 @@ describe("scripts/pr wrappers", () => { mkdirSync(join(repo, "scripts", "pr-lib"), { recursive: true }); writeFileSync(join(repo, "scripts", "pr"), readScript("scripts/pr")); writeFileSync(join(repo, "scripts", "lib", "plain-gh.sh"), "# canonical\n"); + writeFileSync(join(repo, "scripts", "lib", "plain-gh.mjs"), "// canonical\n"); + writeFileSync(join(repo, "scripts", "lib", "direct-run.mjs"), "// canonical\n"); + writeFileSync(join(repo, "scripts", "watch-pr-ci.mjs"), "// canonical\n"); writeFileSync(join(repo, "scripts", "pr-lib", "merge.sh"), "# canonical\n"); chmodSync(join(repo, "scripts", "pr"), 0o755); @@ -364,6 +393,17 @@ describe("scripts/pr wrappers", () => { expect(dirtyLinkedResult.stderr).toContain("scripts/pr wrapper files have uncommitted changes"); expect(git(linked, ["restore", "scripts/pr-lib/merge.sh"]).status).toBe(0); + writeFileSync(join(linked, "scripts", "watch-pr-ci.mjs"), "// dirty watcher\n"); + const dirtyWatcherResult = spawnSync(join(linked, "scripts", "pr"), ["ls"], { + cwd: linked, + encoding: "utf8", + }); + expect(dirtyWatcherResult.status).toBe(1); + expect(dirtyWatcherResult.stderr).toContain( + "scripts/pr wrapper files have uncommitted changes", + ); + expect(git(linked, ["restore", "scripts/watch-pr-ci.mjs"]).status).toBe(0); + // A dirty canonical checkout no longer blocks a linked worktree whose // committed wrapper matches the origin/main trust anchor; without that // anchor it must still refuse. @@ -402,6 +442,9 @@ describe("scripts/pr wrappers", () => { mkdirSync(join(repo, "scripts", "pr-lib"), { recursive: true }); writeFileSync(join(repo, "scripts", "pr"), readScript("scripts/pr")); writeFileSync(join(repo, "scripts", "lib", "plain-gh.sh"), "# canonical\n"); + writeFileSync(join(repo, "scripts", "lib", "plain-gh.mjs"), "// canonical\n"); + writeFileSync(join(repo, "scripts", "lib", "direct-run.mjs"), "// canonical\n"); + writeFileSync(join(repo, "scripts", "watch-pr-ci.mjs"), "// canonical\n"); writeFileSync(join(repo, "scripts", "pr-lib", "merge.sh"), "# canonical\n"); chmodSync(join(repo, "scripts", "pr"), 0o755); @@ -481,4 +524,67 @@ exit 1 expect(result.status).toBe(0); expect(result.stderr).toBe(""); }); + + it("resolves review writer identity and assignment through the real GitHub CLI", () => { + const dir = mkdtempSync(join(tmpdir(), "openclaw-pr-review-writer-")); + const bin = join(dir, "bin"); + const pathCalls = join(dir, "path-calls.log"); + const realCalls = join(dir, "real-calls.log"); + const realGh = join(dir, "real-gh"); + mkdirSync(bin); + writeFileSync( + join(bin, "gh"), + `#!/bin/sh +printf '%s\n' "$*" >> "$OPENCLAW_TEST_PATH_CALLS" +exit 9 +`, + ); + writeFileSync( + realGh, + `#!/bin/sh +printf '%s\n' "$*" >> "$OPENCLAW_TEST_REAL_CALLS" +case "$1 $2" in + "api user") printf 'maintainer\n' ;; + "pr edit") exit 0 ;; + *) exit 2 ;; +esac +`, + ); + chmodSync(join(bin, "gh"), 0o755); + chmodSync(realGh, 0o755); + + const result = spawnSync( + "bash", + [ + "-c", + [ + "source scripts/lib/plain-gh.sh", + "source scripts/pr-lib/review.sh", + 'enter_worktree() { cd "$OPENCLAW_TEST_ROOT"; mkdir -p .local; }', + "mark_pr_operation_side_effects_started() { :; }", + "print_relevant_log_excerpt() { :; }", + "review_claim 42", + ].join("\n"), + ], + { + cwd: process.cwd(), + encoding: "utf8", + env: { + ...process.env, + OPENCLAW_GH_BIN: realGh, + OPENCLAW_TEST_PATH_CALLS: pathCalls, + OPENCLAW_TEST_REAL_CALLS: realCalls, + OPENCLAW_TEST_ROOT: dir, + PATH: `${bin}${delimiter}${process.env.PATH ?? ""}`, + }, + }, + ); + const realInvocations = readFileSync(realCalls, "utf8"); + rmSync(dir, { recursive: true, force: true }); + + expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0); + expect(realInvocations).toContain("api user --jq .login"); + expect(realInvocations).toContain("pr edit 42 --add-assignee maintainer"); + expect(existsSync(pathCalls)).toBe(false); + }); }); diff --git a/test/scripts/test-projects.test.ts b/test/scripts/test-projects.test.ts index 1eafcce9d62b..8d5ae4a6e17b 100644 --- a/test/scripts/test-projects.test.ts +++ b/test/scripts/test-projects.test.ts @@ -1705,6 +1705,8 @@ describe("scripts/test-projects changed-target routing", () => { "scripts/lib/direct-run.mjs": [ "test/scripts/changed-lanes.test.ts", "test/scripts/direct-run-entrypoints.test.ts", + "test/scripts/pr-operation-lock.test.ts", + "test/scripts/pr-wrappers.test.ts", ], "scripts/lib/npm-verify-exec.ts": ["test/scripts/npm-verify-exec.test.ts"], "scripts/lib/plugin-npm-runtime-build.mjs": [ diff --git a/test/scripts/watch-pr-ci.test.ts b/test/scripts/watch-pr-ci.test.ts index 85fe8031a794..67ae9ca8c7e7 100644 --- a/test/scripts/watch-pr-ci.test.ts +++ b/test/scripts/watch-pr-ci.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { buildFindRunArgs, + classifyAttachedCiRun, classifyRollup, classifyRunAttachment, collectRollupContexts, @@ -21,6 +22,7 @@ describe("watch-pr-ci", () => { attachTimeout: 900, timeout: 3600, interval: 120, + completion: "rollup", }); expect( parseArgs([ @@ -36,6 +38,8 @@ describe("watch-pr-ci", () => { "90", "--interval", "5", + "--completion", + "ci-run", ]), ).toMatchObject({ repo: "fork/project", @@ -43,6 +47,7 @@ describe("watch-pr-ci", () => { attachTimeout: 30, timeout: 90, interval: 5, + completion: "ci-run", }); expect(parseArgs(["1", sha.toUpperCase()]).headSha).toBe(sha); }); @@ -56,6 +61,9 @@ describe("watch-pr-ci", () => { expect(() => parseArgs(["1", sha, "--after", "0"])).toThrow( "--after must be a positive integer", ); + expect(() => parseArgs(["1", sha, "--completion", "required"])).toThrow( + "--completion must be rollup or ci-run", + ); }); it("builds a pull-request-only run attachment query", () => { @@ -193,6 +201,22 @@ describe("watch-pr-ci", () => { ).toEqual({ verdict: "PENDING", pendingCount: 1, failingNames: [], supersededCount: 0 }); }); + it("lets an attached successful CI run finish while an optional context remains pending", () => { + expect( + classifyRollup({ + state: "PENDING", + contexts: { + nodes: [ + { kind: "CheckRun", name: "optional proof", status: "IN_PROGRESS", conclusion: null }, + ], + }, + }).verdict, + ).toBe("PENDING"); + expect(classifyAttachedCiRun({ status: "completed", conclusion: "success" })).toEqual({ + verdict: "GREEN", + }); + }); + it.each(["FAILURE", "ERROR"])( "keeps identity-less same-name cancellations failing for aggregate %s", (state) => {