fix(ci): make owner-approved Testbox landing reliable (#104726)

steipete · web-flow · commit 2a479bd4ac85 · 2026-07-11T15:04:25.000-07:00
* fix(ci): isolate nested pnpm verification

* test(google): verify OAuth abort signal forwarding

* docs: treat owner land requests as Testbox approval
diff --git a/AGENTS.md b/AGENTS.md
@@ -116,7 +116,7 @@ Skills own workflows; root owns hard policy and routing.
 - Package manager/runtime: repo defaults only. No swaps without approval.
 - Install: `pnpm install` (keep Bun lock/patches aligned if touched). Agent dependency installation for tests/builds defaults to the selected remote box; do not reconcile a local Codex worktree just to run validation.
 - CLI: `pnpm openclaw ...` or `pnpm dev`; build: `pnpm build`.
-- Agent tests default remote through Crabbox, including focused tests. Trusted maintainer code defaults to Blacksmith Testbox. Contributor/fork code remains untrusted unless a maintainer explicitly approves credentialed execution after review; use secretless fork CI or sanitized direct AWS Crabbox, never a credential-hydrated Testbox. Sanitized AWS must launch an installed trusted Crabbox binary from a clean trusted `main` checkout and fetch only the remote PR via `--fresh-pr`; never execute a wrapper, config, or command from the untrusted local checkout. Before warmup, unset `CRABBOX_AWS_INSTANCE_PROFILE` and all `CRABBOX_TAILSCALE*` overrides; fail closed unless resolved `aws.instanceProfile` is empty. Force `--network public --tailscale=false`, clear exit-node/LAN flags, and require `crabbox inspect` to report public networking with no Tailscale state before any script. Upload trusted `scripts/crabbox-untrusted-bootstrap.sh` from clean `main` alongside `--fresh-pr`; it proves the remote IMDSv2 IAM credentials endpoint returns 404, verifies the reviewed head SHA, unsets `NODE_OPTIONS`, installs pinned Node/pnpm, verifies the package-manager pin, isolates `HOME`, installs dependencies, then runs the requested test. Use a newly warmed lease bound to one reviewed head SHA, set `CRABBOX_ENV_ALLOW=CI`, and use `--no-hydrate`. Never reuse a trusted/previously hydrated lease or carry an untrusted lease across head revisions; stop and rewarm when the SHA changes. No repo `OPENCLAW_*` allowlist, existing auth profile, instance role, tailnet/LAN access, moving PR head, or ambient Node preload may reach untrusted execution. When a code task is likely to need tests, classify source trust, pre-warm the safe backend immediately, keep working while it hydrates, reuse trusted leases or same-SHA untrusted leases, then stop before handoff.
+- Agent tests default remote through Crabbox, including focused tests. Trusted maintainer code defaults to Blacksmith Testbox. Contributor/fork code remains untrusted unless a maintainer explicitly approves credentialed execution after review; an explicit owner/maintainer instruction to land named, reviewed PRs is that approval, so do not ask twice. Otherwise use secretless fork CI or sanitized direct AWS Crabbox, never a credential-hydrated Testbox. Sanitized AWS must launch an installed trusted Crabbox binary from a clean trusted `main` checkout and fetch only the remote PR via `--fresh-pr`; never execute a wrapper, config, or command from the untrusted local checkout. Before warmup, unset `CRABBOX_AWS_INSTANCE_PROFILE` and all `CRABBOX_TAILSCALE*` overrides; fail closed unless resolved `aws.instanceProfile` is empty. Force `--network public --tailscale=false`, clear exit-node/LAN flags, and require `crabbox inspect` to report public networking with no Tailscale state before any script. Upload trusted `scripts/crabbox-untrusted-bootstrap.sh` from clean `main` alongside `--fresh-pr`; it proves the remote IMDSv2 IAM credentials endpoint returns 404, verifies the reviewed head SHA, unsets `NODE_OPTIONS`, installs pinned Node/pnpm, verifies the package-manager pin, isolates `HOME`, installs dependencies, then runs the requested test. Use a newly warmed lease bound to one reviewed head SHA, set `CRABBOX_ENV_ALLOW=CI`, and use `--no-hydrate`. Never reuse a trusted/previously hydrated lease or carry an untrusted lease across head revisions; stop and rewarm when the SHA changes. No repo `OPENCLAW_*` allowlist, existing auth profile, instance role, tailnet/LAN access, moving PR head, or ambient Node preload may reach untrusted execution. When a code task is likely to need tests, classify source trust, pre-warm the safe backend immediately, keep working while it hydrates, reuse trusted leases or same-SHA untrusted leases, then stop before handoff.
 - Test commands (run inside the selected remote box by default): `pnpm test <path-or-filter> [vitest args...]`, `pnpm test:changed`, `pnpm test:serial`, `pnpm test:coverage`; never raw `vitest`.
 - If raw Vitest is unavoidable, use `vitest run ...`; bare `vitest ...` starts local watch mode and will not exit on its own.
 - Local agent test execution is opt-in: only when the user explicitly requests local proof or Testbox is unavailable and the fallback is reported. In a Codex worktree or linked/sparse checkout, the narrow local fallback is `node scripts/run-vitest.mjs <path-or-filter>`; never direct local `pnpm test*`.
@@ -179,7 +179,7 @@ Skills own workflows; root owns hard policy and routing.
 - Contributor PRs: parsed context requires authored `What Problem This Solves` and `Evidence` sections. Do not require field-level proof forms; reviewers inspect code, tests, and CI for correctness.
 - PR artifacts/screenshots: attach to PR/comment/external artifact store. Never push screenshots, videos, proof images, or proof assets to OpenClaw or any product repo branch, including temp artifact branches. Use Crabbox artifact publishing plus the manifest URL. Do not commit `.github/pr-assets`.
 - CI polling: exact SHA, relevant checks only, minimal fields. Skip routine noise (`Auto response`, `Labeler`, docs agents, performance/stale). Logs only after failure/completion or concrete need.
-- Agent PR landing to `main`: use only the repo-native `scripts/pr` wrapper: run `scripts/pr review-init <PR>`, follow its emitted checkout/guard guidance, initialize and complete review artifacts with `scripts/pr review-artifacts-init <PR>`, validate them with `scripts/pr review-validate-artifacts <PR>`, then run `OPENCLAW_TESTBOX=1 scripts/pr prepare-run <PR>` and `scripts/pr merge-run <PR>`. The Testbox flag is mandatory for agents so prepare verifies hosted CI/Testbox on the current head or reuses a patch-identical pre-rebase run green within 24 hours instead of running full gates locally. Do not rebase only because `main` advanced; merge drift is advisory unless strict drift is explicitly enabled, while GitHub still blocks conflicts. Do not idle on `auto-response` or `check-docs`.
+- Agent PR landing to `main`: use only the repo-native `scripts/pr` wrapper: run `scripts/pr review-init <PR>`, follow its emitted checkout/guard guidance, initialize and complete review artifacts with `scripts/pr review-artifacts-init <PR>`, validate them with `scripts/pr review-validate-artifacts <PR>`, then run `OPENCLAW_TESTBOX=1 scripts/pr prepare-run <PR>` and `scripts/pr merge-run <PR>`. The Testbox flag is mandatory for agents so prepare verifies hosted CI/Testbox on the current head or reuses a patch-identical pre-rebase run green within 24 hours instead of running full gates locally. For owner-approved reviewed fork code without hosted Testbox, use `OPENCLAW_PR_GATES_REMOTE=testbox` instead. Do not rebase only because `main` advanced; merge drift is advisory unless strict drift is explicitly enabled, while GitHub still blocks conflicts. Do not idle on `auto-response` or `check-docs`.
 
 ## Code
 
diff --git a/extensions/google/oauth.local-login.test.ts b/extensions/google/oauth.local-login.test.ts
@@ -36,6 +36,7 @@ describe("loginGeminiCliOAuth local browser flow", () => {
   it("prints the auth URL before attempting best-effort browser launch", async () => {
     const events: string[] = [];
     const { loginGeminiCliOAuth } = await import("./oauth.js");
+    const signal = new AbortController().signal;
     const openUrl = vi.fn(async () => {
       events.push("open");
     });
@@ -50,6 +51,7 @@ describe("loginGeminiCliOAuth local browser flow", () => {
       note: async () => {},
       prompt: async () => "",
       progress: { update: () => {}, stop: () => {} },
+      signal,
     });
 
     expect(result).toEqual({
@@ -65,6 +67,6 @@ describe("loginGeminiCliOAuth local browser flow", () => {
     expect(waitForLocalCallbackMock).toHaveBeenCalledWith(
       expect.objectContaining({ expectedState: "state-123" }),
     );
-    expect(exchangeCodeForTokensMock).toHaveBeenCalledWith("oauth-code", "pkce-verifier");
+    expect(exchangeCodeForTokensMock).toHaveBeenCalledWith("oauth-code", "pkce-verifier", signal);
   });
 });
diff --git a/scripts/check-gateway-cpu-scenarios.mjs b/scripts/check-gateway-cpu-scenarios.mjs
@@ -261,7 +261,7 @@ function buildPrivateQaEnv(env, qaState) {
     OPENCLAW_ENABLE_PRIVATE_QA_CLI: "1",
     OPENCLAW_RUN_NODE_SKIP_DTS_BUILD: env.OPENCLAW_RUN_NODE_SKIP_DTS_BUILD ?? "1",
     OPENCLAW_TEST_DISABLE_UPDATE_CHECK: env.OPENCLAW_TEST_DISABLE_UPDATE_CHECK ?? "1",
-    PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN: env.PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN ?? "false",
+    PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN: "false",
   };
 }
 
@@ -282,7 +282,7 @@ async function runGatewayCpuScenarios(options, params = {}) {
   const inputEnv = params.env ?? process.env;
   const baseEnv = {
     ...inputEnv,
-    PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN: inputEnv.PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN ?? "false",
+    PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN: "false",
   };
   fs.mkdirSync(options.outputDir, { recursive: true });
 
diff --git a/test/scripts/check-gateway-cpu-scenarios.test.ts b/test/scripts/check-gateway-cpu-scenarios.test.ts
@@ -153,6 +153,7 @@ describe("gateway CPU scenario guard", () => {
     ]);
 
     const result = await testing.runGatewayCpuScenarios(options, {
+      env: { ...process.env, PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN: "install" },
       silent: true,
       spawnSync: (command: string, args: string[], opts?: { env?: Record<string, string> }) => {
         calls.push({ args, command, env: opts?.env });
@@ -289,6 +290,7 @@ describe("gateway CPU scenario guard", () => {
 
     const result = await testing.runGatewayCpuScenarios(options, {
       cwd,
+      env: { ...process.env, PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN: "install" },
       silent: true,
       spawnSync: (_command: string, args: string[], opts?: { env?: Record<string, string> }) => {
         calls.push({ args, env: opts?.env });
