diff --git a/scripts/AGENTS.md b/scripts/AGENTS.md index b9e815d83ab..40ec829bd0a 100644 --- a/scripts/AGENTS.md +++ b/scripts/AGENTS.md @@ -20,7 +20,7 @@ This directory owns local tooling, script wrappers, and generated-artifact helpe ## PR Prepare Gates -- `scripts/pr` serializes review, prepare, and merge operations per PR across linked worktrees; `scripts/pr gc` skips active or indeterminate locks. A successful command return is the trusted synchronous-completion contract: every PR-state-mutating child must be joined before returning, and such work must never daemonize or explicitly escape both the operation group and lock-notification FD. A failed command auto-releases only while its explicit pre-side-effect validation marker remains active; failures after mutation/tool launch, interruptions, and controller loss stay locked because detached children cannot be disproved. After verifying no child tools remain, use the reported exact-OID `scripts/pr lock-recover` command. Never bypass or delete these refs manually. +- `scripts/pr` serializes review, prepare, and merge operations per PR across linked worktrees; `scripts/pr gc` skips active or indeterminate locks. Its subcommand classification table is the canonical wrapper trust boundary: a mismatched local wrapper may run only a classified `advisory` subcommand with `--dev-wrapper` or `OPENCLAW_PR_DEV_WRAPPER=1`; classified `landing` subcommands always require canonical/origin-main wrapper code. A successful command return is the trusted synchronous-completion contract: every PR-state-mutating child must be joined before returning, and such work must never daemonize or explicitly escape both the operation group and lock-notification FD. A failed command auto-releases only while its explicit pre-side-effect validation marker remains active; failures after mutation/tool launch, interruptions, and controller loss stay locked because detached children cannot be disproved. After verifying no child tools remain, use the reported exact-OID `scripts/pr lock-recover` command. Never bypass or delete these refs manually. - `scripts/pr prepare-gates` holds the heavy-check lock for its whole local gate block (`scripts/pr-gates-lock.mjs`), so concurrent gate runs across `.worktrees` queue as units instead of dying on child lock timeouts or vitest no-output watchdog kills. - `OPENCLAW_PR_GATES_REMOTE=testbox` runs the full-suite `pnpm test` gate on a Blacksmith Testbox through `scripts/crabbox-wrapper.mjs` (same delegation as `check:changed`); `pnpm build`/`pnpm check` stay local. The `tbx_` lease id and Actions run URL land in `.local/gates.env` (`REMOTE_GATES_*`) and `.local/prep.md`. Use it for reviewed trusted code when a loaded host makes the local 88-shard run stall-kill; contributor/fork code stays on secretless CI or sanitized AWS unless a maintainer explicitly approves credentialed execution. diff --git a/scripts/pr b/scripts/pr index 7566cbca5fd..23558ce46a7 100755 --- a/scripts/pr +++ b/scripts/pr @@ -9,6 +9,36 @@ export CLICOLOR_FORCE=0 export FORCE_COLOR=0 unset COLORTERM +# This is the single source of truth for the canonical-wrapper trust boundary. +# Advisory commands may run a mismatched local wrapper only with the explicit +# developer opt-in; landing commands must always use canonical/origin-main code. +# Classification is independent of serialization: ci-dispatch remains locked +# because GitHub exposes neither dispatch deduplication nor a correlation ID. +# PR_SUBCOMMAND_CLASSIFICATIONS_BEGIN +pr_subcommand_classification() { + case "$1" in + ls | ci-dispatch) + printf 'advisory\n' + ;; + gc | lock-recover | review-init | review-checkout-main | review-checkout-pr | review-claim | review-guard | review-artifacts-init | review-validate-artifacts | review-tests | prepare-init | prepare-validate-commit | prepare-gates | prepare-push | prepare-sync-head | prepare-run | merge-verify | merge-run) + printf 'landing\n' + ;; + *) return 1 ;; + esac +} +# PR_SUBCOMMAND_CLASSIFICATIONS_END + +dev_wrapper_opt_in=0 +if [ "${OPENCLAW_PR_DEV_WRAPPER:-}" = "1" ]; then + dev_wrapper_opt_in=1 +fi +if [ "${1-}" = "--dev-wrapper" ]; then + dev_wrapper_opt_in=1 + export OPENCLAW_PR_DEV_WRAPPER=1 + shift +fi +requested_subcommand="${1-}" + # If invoked from a linked worktree copy of this script, re-exec the canonical # script from the repository root so behavior stays consistent across worktrees. script_self="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/$(basename "${BASH_SOURCE[0]}")" @@ -60,19 +90,39 @@ if common_git_dir=$(git -C "$script_parent_dir" rev-parse --path-format=absolute if [ -z "$linked_wrapper_revision" ] || [ -z "$anchor_wrapper_revision" ] || [ "$linked_wrapper_revision" != "$anchor_wrapper_revision" ]; then - echo "scripts/pr implementation differs between this worktree and the canonical checkout, and does not match origin/main." >&2 - echo "Refusing to silently substitute canonical wrapper code from: $canonical_repo_root" >&2 - echo "Run scripts/pr from a checkout whose wrapper matches the canonical checkout or a fetched origin/main." >&2 - exit 1 + requested_classification=$(pr_subcommand_classification "$requested_subcommand" 2>/dev/null || true) + if [ "$dev_wrapper_opt_in" = "1" ] && [ "$requested_classification" = "advisory" ]; then + if [ "${OPENCLAW_PR_DEV_WRAPPER_BANNER_SHOWN:-}" != "1" ]; then + local_head_revision=$(git -C "$script_parent_dir" rev-parse HEAD 2>/dev/null || printf 'unknown') + echo "WARNING: running local scripts/pr revision $local_head_revision via dev-wrapper opt-in." >&2 + echo "subcommand '$requested_subcommand' is classified advisory." >&2 + echo "The local wrapper differs from the canonical checkout and origin/main; landing subcommands remain refused." >&2 + export OPENCLAW_PR_DEV_WRAPPER_BANNER_SHOWN=1 + fi + else + if [ "$dev_wrapper_opt_in" = "1" ] && [ -n "$requested_classification" ]; then + echo "subcommand '$requested_subcommand' is classified $requested_classification; dev-wrapper opt-in is unavailable." >&2 + fi + echo "scripts/pr implementation differs between this worktree and the canonical checkout, and does not match origin/main." >&2 + echo "Refusing to silently substitute canonical wrapper code from: $canonical_repo_root" >&2 + echo "Run scripts/pr from a checkout whose wrapper matches the canonical checkout or a fetched origin/main." >&2 + exit 1 + fi fi fi fi is_locked_pr_command() { - case "$1" in - review-init | review-checkout-main | review-checkout-pr | review-claim | review-guard | review-artifacts-init | review-validate-artifacts | review-tests | prepare-init | prepare-validate-commit | prepare-gates | prepare-push | prepare-sync-head | prepare-run | ci-dispatch | merge-verify | merge-run) return 0 ;; - *) return 1 ;; - esac + # Advisory trust classification permits local dogfood, but dispatches still + # serialize with landing operations because the remote mutation is not atomic. + if [ "$1" = "ci-dispatch" ]; then + return 0 + fi + local classification + classification=$(pr_subcommand_classification "$1") || return 1 + [ "$classification" = "landing" ] || return 1 + # gc manages per-PR locks itself; lock-recover performs an exact-OID CAS. + [ "$1" != "gc" ] && [ "$1" != "lock-recover" ] } is_main_only_pr_command() { @@ -107,6 +157,7 @@ source "$script_parent_dir/lib/plain-gh.sh" usage() { cat < ... scripts/pr ls scripts/pr gc [--dry-run] scripts/pr lock-recover --confirmed-no-running-tools @@ -128,6 +179,9 @@ Usage: scripts/pr merge-verify scripts/pr merge-run OPENCLAW_PR_MERGE_METHOD=merge|rebase preserves the PR commit series. + + --dev-wrapper permits a mismatched local wrapper only for subcommands + classified advisory. OPENCLAW_PR_DEV_WRAPPER=1 is equivalent. USAGE } diff --git a/test/scripts/pr-wrappers.test.ts b/test/scripts/pr-wrappers.test.ts index b0aade6e31a..92e6fbbfbde 100644 --- a/test/scripts/pr-wrappers.test.ts +++ b/test/scripts/pr-wrappers.test.ts @@ -1,6 +1,15 @@ // PR wrapper tests cover maintainer helper command delegation. import { spawnSync } from "node:child_process"; -import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { + chmodSync, + cpSync, + mkdirSync, + mkdtempSync, + readFileSync, + realpathSync, + rmSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; @@ -9,6 +18,126 @@ function readScript(path: string): string { return readFileSync(path, "utf8"); } +const canonicalMismatchMessage = (repo: string) => + [ + "scripts/pr implementation differs between this worktree and the canonical checkout, and does not match origin/main.", + `Refusing to silently substitute canonical wrapper code from: ${repo}`, + "Run scripts/pr from a checkout whose wrapper matches the canonical checkout or a fetched origin/main.", + "", + ].join("\n"); + +function makeMismatchedWrapperRepo() { + const root = realpathSync(mkdtempSync(join(realpathSync(tmpdir()), "openclaw-pr-dev-wrapper-"))); + const home = join(root, "home"); + const canonicalPath = join(root, "canonical"); + const linkedPath = join(root, "linked"); + const originPath = join(root, "origin.git"); + mkdirSync(home, { recursive: true }); + + const fixtureEnv = { + ...process.env, + GIT_CONFIG_GLOBAL: "/dev/null", + GIT_CONFIG_NOSYSTEM: "1", + HOME: home, + XDG_CONFIG_HOME: join(home, ".config"), + }; + const git = (cwd: string, args: string[]) => { + const result = spawnSync("git", args, { + cwd, + encoding: "utf8", + env: fixtureEnv, + stdio: "pipe", + }); + expect(result.status, `git ${args.join(" ")}\n${result.stderr}`).toBe(0); + return result; + }; + + git(root, ["init", "--bare", "-b", "main", originPath]); + git(root, ["init", "-b", "main", canonicalPath]); + const canonical = realpathSync(canonicalPath); + const origin = realpathSync(originPath); + 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")); + writeFileSync( + join(canonical, "scripts", "lib", "plain-gh.sh"), + "resolve_plain_gh_bin() { printf '/usr/bin/true\\n'; }\ngh_plain() { :; }\n", + ); + chmodSync(join(canonical, "scripts", "pr"), 0o755); + + git(canonical, ["config", "user.name", "OpenClaw Test"]); + git(canonical, ["config", "user.email", "test@example.invalid"]); + git(canonical, ["config", "commit.gpgSign", "false"]); + git(canonical, ["config", "core.hooksPath", "/dev/null"]); + git(canonical, ["remote", "add", "origin", origin]); + git(canonical, ["add", "scripts"]); + git(canonical, ["commit", "-m", "test: canonical wrapper"]); + git(canonical, ["push", "-u", "origin", "main"]); + git(canonical, ["worktree", "add", "-b", "feature", linkedPath, "main"]); + + const linked = realpathSync(linkedPath); + git(linked, ["config", "user.name", "OpenClaw Test"]); + git(linked, ["config", "user.email", "test@example.invalid"]); + git(linked, ["config", "commit.gpgSign", "false"]); + expect(git(linked, ["rev-parse", "refs/remotes/origin/main"]).stdout.trim()).toBe( + git(canonical, ["rev-parse", "main"]).stdout.trim(), + ); + + writeFileSync( + join(linked, "scripts", "pr-lib", "gates.sh"), + 'ci_dispatch() { echo "local wrapper executed"; }\n', + ); + git(linked, ["add", "scripts/pr-lib/gates.sh"]); + git(linked, ["commit", "-m", "test: local wrapper"]); + const localRevision = git(linked, ["rev-parse", "HEAD"]).stdout.trim(); + + return { + canonical, + cleanup: () => rmSync(root, { recursive: true, force: true }), + env: fixtureEnv, + linked, + localRevision, + }; +} + +function parseSubcommandClassifications(script: string): Map { + const start = script.indexOf("# PR_SUBCOMMAND_CLASSIFICATIONS_BEGIN"); + const end = script.indexOf("# PR_SUBCOMMAND_CLASSIFICATIONS_END"); + expect(start).toBeGreaterThanOrEqual(0); + expect(end).toBeGreaterThan(start); + const table = script.slice(start, end); + const classifications = new Map(); + const armPattern = /^\s+([^\n)]+)\)\s*\n\s+printf '(landing|advisory)\\n'/gm; + for (const match of table.matchAll(armPattern)) { + const commandGroup = match[1]; + const classification = match[2]; + if (commandGroup === undefined || classification === undefined) { + throw new Error("classification regexp returned incomplete captures"); + } + for (const command of commandGroup.split("|").map((value) => value.trim())) { + classifications.set(command, classification); + } + } + return classifications; +} + +function parseDispatchedSubcommands(script: string): string[] { + const start = script.lastIndexOf(' case "$cmd" in'); + expect(start).toBeGreaterThanOrEqual(0); + const end = script.indexOf("\n esac", start); + expect(end).toBeGreaterThan(start); + const commands: string[] = []; + const armPattern = /^\s{4}([^\n)]+)\)/gm; + for (const match of script.slice(start, end).matchAll(armPattern)) { + const commandGroup = match[1]; + if (commandGroup === undefined) { + throw new Error("dispatch regexp returned an incomplete capture"); + } + commands.push(...commandGroup.split("|").map((value) => value.trim())); + } + return commands.filter((command) => command !== "*"); +} + describe("scripts/pr wrappers", () => { it("keeps the main PR helper usage and command table aligned", () => { const script = readScript("scripts/pr"); @@ -30,6 +159,86 @@ describe("scripts/pr wrappers", () => { expect(script).toContain("only support PRs targeting main"); }); + it("classifies every dispatched subcommand", () => { + const script = readScript("scripts/pr"); + const classifications = parseSubcommandClassifications(script); + const dispatched = parseDispatchedSubcommands(script); + + expect([...classifications.keys()].sort()).toEqual([...dispatched, "lock-recover"].sort()); + expect(classifications.get("ls")).toBe("advisory"); + expect(classifications.get("ci-dispatch")).toBe("advisory"); + for (const command of dispatched.filter((value) => !["ls", "ci-dispatch"].includes(value))) { + expect(classifications.get(command), command).toBe("landing"); + } + }); + + it("runs a mismatched advisory wrapper locally with an explicit developer opt-in", () => { + const fixture = makeMismatchedWrapperRepo(); + try { + const cliResult = spawnSync( + join(fixture.linked, "scripts", "pr"), + ["--dev-wrapper", "ci-dispatch", "123"], + { + cwd: fixture.linked, + encoding: "utf8", + env: fixture.env, + }, + ); + expect(cliResult.status, cliResult.stderr).toBe(0); + expect(cliResult.stdout).toContain("local wrapper executed"); + expect(cliResult.stderr).toContain( + `WARNING: running local scripts/pr revision ${fixture.localRevision} via dev-wrapper opt-in.`, + ); + expect(cliResult.stderr).toContain("subcommand 'ci-dispatch' is classified advisory."); + expect(cliResult.stderr).toContain("landing subcommands remain refused"); + + const envResult = spawnSync(join(fixture.linked, "scripts", "pr"), ["ci-dispatch", "123"], { + cwd: fixture.linked, + encoding: "utf8", + env: { ...fixture.env, OPENCLAW_PR_DEV_WRAPPER: "1" }, + }); + expect(envResult.status, envResult.stderr).toBe(0); + expect(envResult.stdout).toContain("local wrapper executed"); + expect(envResult.stderr).toContain("subcommand 'ci-dispatch' is classified advisory."); + } finally { + fixture.cleanup(); + } + }); + + it("keeps the existing mismatch refusal for advisory commands without opt-in", () => { + const fixture = makeMismatchedWrapperRepo(); + try { + const result = spawnSync(join(fixture.linked, "scripts", "pr"), ["ci-dispatch", "123"], { + cwd: fixture.linked, + encoding: "utf8", + env: fixture.env, + }); + expect(result.status).toBe(1); + expect(result.stderr).toBe(canonicalMismatchMessage(fixture.canonical)); + } finally { + fixture.cleanup(); + } + }); + + it("refuses developer opt-in for a mismatched landing command", () => { + const fixture = makeMismatchedWrapperRepo(); + try { + const result = spawnSync( + join(fixture.linked, "scripts", "pr"), + ["--dev-wrapper", "prepare-run", "123"], + { cwd: fixture.linked, encoding: "utf8", env: fixture.env }, + ); + expect(result.status).toBe(1); + expect(result.stderr).toContain( + "subcommand 'prepare-run' is classified landing; dev-wrapper opt-in is unavailable.", + ); + expect(result.stderr).toContain(canonicalMismatchMessage(fixture.canonical).trim()); + expect(result.stdout).not.toContain("local wrapper executed"); + } finally { + fixture.cleanup(); + } + }); + it("keeps merge wrapper modes delegated to the main PR helper", () => { const script = readScript("scripts/pr-merge");