diff --git a/.agents/skills/openclaw-pr-maintainer/SKILL.md b/.agents/skills/openclaw-pr-maintainer/SKILL.md index 7250a2a82449..221e7599dafe 100644 --- a/.agents/skills/openclaw-pr-maintainer/SKILL.md +++ b/.agents/skills/openclaw-pr-maintainer/SKILL.md @@ -284,6 +284,10 @@ gh search issues --repo openclaw/openclaw --match title,body --limit 50 \ ## Follow PR review and landing hygiene +- `scripts/pr` requires `git`, `gh`, `jq`, `rg` (ripgrep), `pnpm`, and `node` + on the maintainer host. Let its preflight fail loudly when one is missing. + Tests that source `scripts/pr-lib/*` directly must provide the same command + surface instead of weakening the production wrapper for a minimal test image. - At the start of code-changing or landing work that will need tests or heavy proof, classify source trust and pre-warm the safe backend through `$crabbox` in the background. Trusted maintainer code defaults to Blacksmith Testbox; diff --git a/scripts/pr b/scripts/pr index 21481941d581..60cc36943f02 100755 --- a/scripts/pr +++ b/scripts/pr @@ -184,6 +184,8 @@ Usage: --dev-wrapper permits a mismatched local wrapper only for subcommands classified advisory. OPENCLAW_PR_DEV_WRAPPER=1 is equivalent. + +Required commands: git, gh, jq, rg (ripgrep), pnpm, node. USAGE } @@ -202,7 +204,10 @@ require_cmds() { fi if [ "${#missing[@]}" -gt 0 ]; then - echo "Missing required command(s): ${missing[*]}" + echo "Missing required command(s): ${missing[*]}" >&2 + if [[ " ${missing[*]} " = *" rg "* ]]; then + echo "Install ripgrep and retry: https://github.com/BurntSushi/ripgrep#installation" >&2 + fi exit 1 fi } diff --git a/scripts/pr-lib/merge.sh b/scripts/pr-lib/merge.sh index 2417e1f0e9c0..28c7af17089c 100644 --- a/scripts/pr-lib/merge.sh +++ b/scripts/pr-lib/merge.sh @@ -28,30 +28,9 @@ print_file_list_with_limit() { auto_merge_unavailable_error() { local log_file="$1" - local auto_merge_pattern='[Aa][Uu][Tt][Oo][ -][Mm][Ee][Rr][Gg][Ee]' - local unavailable_pattern - local line - - while IFS= read -r line || [ -n "$line" ]; do - case "$line" in - *$auto_merge_pattern*) - for unavailable_pattern in \ - '[Nn][Oo][Tt] [Aa][Ll][Ll][Oo][Ww][Ee][Dd]' \ - '[Nn][Oo][Tt] [Ee][Nn][Aa][Bb][Ll][Ee][Dd]' \ - '[Nn][Oo][Tt] [Aa][Vv][Aa][Ii][Ll][Aa][Bb][Ll][Ee]' \ - '[Uu][Nn][Aa][Vv][Aa][Ii][Ll][Aa][Bb][Ll][Ee]' \ - '[Nn][Oo][Tt] [Cc][Oo][Nn][Ff][Ii][Gg][Uu][Rr][Ee][Dd]' \ - '[Nn][Oo][Tt] [Ss][Uu][Pp][Pp][Oo][Rr][Tt][Ee][Dd]' \ - '[Mm][Uu][Ss][Tt] [Bb][Ee] [Ee][Nn][Aa][Bb][Ll][Ee][Dd]'; do - case "$line" in - *$unavailable_pattern*) return 0 ;; - esac - done - ;; - esac - done < "$log_file" - - return 1 + rg -q -i -- \ + 'auto[- ]merge.*(not allowed|not enabled|not available|unavailable|not configured|not supported|must be enabled)|(not allowed|not enabled|not available|unavailable|not configured|not supported|must be enabled).*auto[- ]merge' \ + "$log_file" } mainline_drift_requires_sync() { diff --git a/test/scripts/pr-merge.test.ts b/test/scripts/pr-merge.test.ts index b2ae527265ce..40e37d479ee2 100644 --- a/test/scripts/pr-merge.test.ts +++ b/test/scripts/pr-merge.test.ts @@ -1,6 +1,6 @@ import { spawnSync } from "node:child_process"; -import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; -import { join } from "node:path"; +import { chmodSync, existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { delimiter, join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js"; @@ -12,6 +12,7 @@ const describePosix = process.platform === "win32" ? describe.skip : describe; type MergeScenario = { auto?: boolean; + autoError?: string; autoResult?: "enabled" | "inconclusive" | "unavailable"; checks?: "fail" | "green" | "pending"; existingAutoMethod?: "" | "MERGE" | "REBASE" | "SQUASH"; @@ -27,7 +28,23 @@ function runMerge(scenario: MergeScenario = {}) { const calls = join(root, "gh-calls.log"); const autoCalled = join(root, "auto-called"); const autoState = join(root, "auto-state"); + const bin = join(root, "bin"); + const rgCalls = join(root, "rg-calls.log"); + mkdirSync(bin, { recursive: true }); mkdirSync(localDir, { recursive: true }); + writeFileSync( + join(bin, "rg"), + `#!/usr/bin/env node +const { appendFileSync, readFileSync } = require("node:fs"); +const args = process.argv.slice(2); +appendFileSync(process.env.OPENCLAW_TEST_RG_CALLS, JSON.stringify(args) + "\\n"); +const pattern = args.at(-2); +const file = args.at(-1); +const flags = args.includes("-i") ? "i" : ""; +process.exit(new RegExp(pattern, flags).test(readFileSync(file, "utf8")) ? 0 : 1); +`, + ); + chmodSync(join(bin, "rg"), 0o755); writeFileSync( join(localDir, "prep.env"), `PREP_HEAD_SHA=${headSha}\nLOCAL_PREP_HEAD_SHA=${headSha}\n`, @@ -152,7 +169,7 @@ gh() { : > "$OPENCLAW_TEST_AUTO_CALLED" printf 'enabled\\n' > "$OPENCLAW_TEST_AUTO_STATE" if [ "$OPENCLAW_TEST_AUTO_RESULT" = "unavailable" ]; then - echo 'GraphQL: Pull request auto merge is not allowed for this repository' >&2 + echo "$OPENCLAW_TEST_AUTO_ERROR" >&2 return 1 fi if [ "$OPENCLAW_TEST_AUTO_RESULT" = "inconclusive" ]; then @@ -177,6 +194,8 @@ merge_run 123 "$OPENCLAW_TEST_AUTO_REQUESTED" env: { ...process.env, OPENCLAW_TEST_AUTO_CALLED: autoCalled, + OPENCLAW_TEST_AUTO_ERROR: + scenario.autoError ?? "GraphQL: Pull request auto merge is not allowed for this repository", OPENCLAW_TEST_AUTO_REQUESTED: scenario.auto ? "true" : "false", OPENCLAW_TEST_AUTO_RESULT: scenario.autoResult ?? "enabled", OPENCLAW_TEST_AUTO_STATE: autoState, @@ -191,12 +210,15 @@ merge_run 123 "$OPENCLAW_TEST_AUTO_REQUESTED" OPENCLAW_TEST_PRE_AUTO_META: preAutoMeta, OPENCLAW_TEST_REVIEW_ARTIFACTS: scenario.reviewArtifacts ?? "valid", OPENCLAW_TEST_REVIEW_RECOMMENDATION: scenario.recommendation ?? "ready", + OPENCLAW_TEST_RG_CALLS: rgCalls, OPENCLAW_TEST_ROOT: root, + PATH: `${bin}${delimiter}${process.env.PATH ?? ""}`, }, }); return { ...result, calls: existsSync(calls) ? readFileSync(calls, "utf8") : "", + rgCalls: existsSync(rgCalls) ? readFileSync(rgCalls, "utf8") : "", }; } @@ -301,7 +323,20 @@ describePosix("scripts/pr merge-run", () => { 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).toContain(`pr merge 123 --squash --match-head-commit ${headSha}`); + expect(result.rgCalls).toContain('"-q","-i","--"'); expect(result.stdout).toContain("auto-merge is unavailable"); expect(result.stdout).toContain("falling back"); }); + + it("recognizes unavailable auto-merge wording in reverse order", () => { + const result = runMerge({ + auto: true, + autoError: "GraphQL: Branch protection must be enabled before using auto-merge", + autoResult: "unavailable", + }); + + expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0); + expect(result.calls).toContain(`pr merge 123 --squash --match-head-commit ${headSha}`); + expect(result.stdout).toContain("auto-merge is unavailable"); + }); }); diff --git a/test/scripts/pr-wrappers.test.ts b/test/scripts/pr-wrappers.test.ts index 9585756da880..f63d973d21ce 100644 --- a/test/scripts/pr-wrappers.test.ts +++ b/test/scripts/pr-wrappers.test.ts @@ -3,11 +3,13 @@ import { spawnSync } from "node:child_process"; import { chmodSync, cpSync, + existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, + symlinkSync, writeFileSync, } from "node:fs"; import { tmpdir } from "node:os"; @@ -25,6 +27,7 @@ const canonicalMismatchMessage = (repo: string) => "Run scripts/pr from a checkout whose wrapper matches the canonical checkout or a fetched origin/main.", "", ].join("\n"); +const itPosix = process.platform === "win32" ? it.skip : it; function makeMismatchedWrapperRepo() { const root = realpathSync(mkdtempSync(join(realpathSync(tmpdir()), "openclaw-pr-dev-wrapper-"))); @@ -97,6 +100,7 @@ function makeMismatchedWrapperRepo() { const localRevision = git(linked, ["rev-parse", "HEAD"]).stdout.trim(); return { + bin, canonical, cleanup: () => rmSync(root, { recursive: true, force: true }), env: fixtureEnv, @@ -105,6 +109,16 @@ function makeMismatchedWrapperRepo() { }; } +function resolveCommand(command: string): string { + for (const dir of (process.env.PATH ?? "").split(delimiter)) { + const candidate = join(dir, command); + if (existsSync(candidate)) { + return realpathSync(candidate); + } + } + throw new Error(`command not found in test PATH: ${command}`); +} + function parseSubcommandClassifications(script: string): Map { const start = script.indexOf("# PR_SUBCOMMAND_CLASSIFICATIONS_BEGIN"); const end = script.indexOf("# PR_SUBCOMMAND_CLASSIFICATIONS_END"); @@ -157,6 +171,7 @@ describe("scripts/pr wrappers", () => { expect(script).toContain("scripts/pr ci-dispatch "); expect(script).toContain("scripts/pr merge-run [--auto-merge]"); expect(script).toContain("OPENCLAW_PR_AUTO_MERGE=1 is equivalent"); + expect(script).toContain("Required commands: git, gh, jq, rg (ripgrep), pnpm, node."); expect(script).toContain('review_init "$pr"'); expect(script).toContain('prepare_run "$pr"'); expect(script).toContain('ci_dispatch "$pr"'); @@ -165,6 +180,31 @@ describe("scripts/pr wrappers", () => { expect(script).toContain("only support PRs targeting main"); }); + 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"]) { + symlinkSync(resolveCommand(command), join(fixture.bin, command)); + } + + const result = spawnSync(join(fixture.canonical, "scripts", "pr"), ["ls"], { + cwd: fixture.canonical, + encoding: "utf8", + env: { + ...fixture.env, + PATH: fixture.bin, + }, + }); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("Missing required command(s): rg"); + expect(result.stderr).toContain("Install ripgrep and retry:"); + } finally { + fixture.cleanup(); + } + }); + it("classifies every dispatched subcommand", () => { const script = readScript("scripts/pr"); const classifications = parseSubcommandClassifications(script);