chore(pr): dev-wrapper opt-in — advisory subcommands may dogfood modified wrappers (#111656)

* chore(pr): dev-wrapper opt-in — advisory subcommands may dogfood modified wrappers * fix(pr): hermetic dev-wrapper test fixture and type narrowing
2026-07-21 02:06:43 +00:00 · 2026-07-19 22:58:15 -07:00
parent 6e19c36d41
commit 104128dd32
3 changed files with 273 additions and 10 deletions
@@ -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.

@@ -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 <<USAGE
 Usage:
+  scripts/pr [--dev-wrapper] <subcommand> ...
  scripts/pr ls
  scripts/pr gc [--dry-run]
  scripts/pr lock-recover <PR> <OWNER_OID> --confirmed-no-running-tools
@@ -128,6 +179,9 @@ Usage:
  scripts/pr merge-verify <PR>
  scripts/pr merge-run <PR>
    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
 }

@@ -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<string, string> {
+  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<string, string>();
+  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");