ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-12 21:53:00 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(maintainers): route PR reads through Octopool (#120525)

Browse Source 
* fix(maintainers): route PR reads through Octopool

* test(maintainers): cover PR transport ownership

* fix(maintainers): scope merge wait to CI run
This commit is contained in:
Peter Steinberger
2026-08-08 03:56:36 -07:00
committed by GitHub
parent 205753eef7
commit d1dea39bbb
17 changed files with 285 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files
scripts/pr
+17 -24
View File
@@ -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
scripts/pr-lib/ci-dispatch.mjs
+18 -20
View File
@@ -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();
scripts/pr-lib/merge.sh
+11 -8
View File
@@ -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
scripts/pr-lib/push.sh
+1 -1
View File
@@ -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
}
scripts/pr-lib/review.sh
+2 -2
View File
@@ -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
scripts/pr-lib/worktree.sh
+1 -1
View File
@@ -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({
scripts/watch-pr-ci.d.mts
+6
View File
@@ -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<T> {
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<T>(options: PollUntilDeadlineOptions<T>): Promise<T | undefined>;
scripts/watch-pr-ci.mjs
+33 -1
View File
@@ -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 <pr-number> <head-sha> [--repo owner/repo] [--after run-id] [--attach-timeout 900] [--timeout 3600] [--interval 120]";
"Usage: node scripts/watch-pr-ci.mjs <pr-number> <head-sha> [--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) {
test/scripts/pr-ci-dispatch.test.ts
+24 -14
View File
@@ -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 ?? ""}`,
},
},
);
test/scripts/pr-merge.test.ts
+24 -5
View File
@@ -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");
});
test/scripts/pr-metadata.test.ts
+5 -1
View File
@@ -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",
test/scripts/pr-operation-lock.test.ts
+3
View File
@@ -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",
test/scripts/pr-prepare-gates.test.ts
+3 -3
View File
@@ -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 <helper@example.com>"',
@@ -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 },
test/scripts/pr-review-artifact-validation.test.ts
+3 -1
View File
@@ -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",
test/scripts/pr-wrappers.test.ts
+108 -2
View File
@@ -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 <PR>");
expect(script).toContain("scripts/pr prepare-run <PR>");
expect(script).toContain("scripts/pr ci-dispatch <PR>");
@@ -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);
});
});
test/scripts/test-projects.test.ts
+2
View File
@@ -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": [
test/scripts/watch-pr-ci.test.ts
+24
View File
@@ -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) => {