From 2e705107ff26876b906c61485c652cca6f27cd71 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Fri, 2 Oct 2026 11:07:18 +0700 Subject: [PATCH] fix(crabbox): start remote checks from code-only worktrees (#163035) Co-authored-by: Vincent Koc --- scripts/AGENTS.md | 5 +- scripts/crabbox-wrapper.mjs | 2 + scripts/lib/tooling-dependencies.d.mts | 5 + ...endencies.mjs => tooling-dependencies.mjs} | 71 ++++++++++----- scripts/lib/tsx-cli-shim.d.mts | 2 + scripts/lib/tsx-cli-shim.mjs | 27 +++++- scripts/pr-lib/wrapper-components.txt | 2 +- scripts/watch-pr-ci.mjs | 4 +- .../direct-run-entrypoints.test-support.ts | 4 + .../tooling-dependencies.test-support.mts | 91 +++++++++++++++++++ test/scripts/tooling-dependencies.test.ts | 49 ++++++++++ test/scripts/watch-pr-ci-dependencies.test.ts | 21 +++-- 12 files changed, 244 insertions(+), 39 deletions(-) create mode 100644 scripts/lib/tooling-dependencies.d.mts rename scripts/lib/{watch-pr-ci-dependencies.mjs => tooling-dependencies.mjs} (64%) create mode 100644 test/scripts/tooling-dependencies.test-support.mts create mode 100644 test/scripts/tooling-dependencies.test.ts diff --git a/scripts/AGENTS.md b/scripts/AGENTS.md index fe7ea451588d..34caf409f76a 100644 --- a/scripts/AGENTS.md +++ b/scripts/AGENTS.md @@ -59,7 +59,10 @@ without changing the receipt or substituting today's main or the check's base. `OPENCLAW_PR_TOOLING_ROOT` selects a full checkout of the same repository for materialized wrappers' third-party dependencies; otherwise `openclaw.pr.toolingRoot` in the canonical checkout's Git config applies, then the canonical checkout itself. -The standalone CI watcher resolves missing packages from the same tooling root when its checkout has no `node_modules`, with the same explicit-root identity checks and exact package versions. +The standalone CI watcher and Crabbox entrypoint resolve missing third-party +packages from the same tooling root when their checkout has no `node_modules`, +with the same explicit-root identity checks and exact package versions. They +never link an installation or resolve workspace packages from another checkout. The wrapper still selects and verifies code against the existing trust anchor. Installed package versions must exactly match the anchor manifest. On mismatch, an explicitly selected, separate, clean `main` checkout is fetched, fast-forwarded, diff --git a/scripts/crabbox-wrapper.mjs b/scripts/crabbox-wrapper.mjs index cd4ca10e67c9..cb680c0ae154 100755 --- a/scripts/crabbox-wrapper.mjs +++ b/scripts/crabbox-wrapper.mjs @@ -5,4 +5,6 @@ await runTsxCliShim(import.meta.url, { implementation: "./crabbox-wrapper.mts", detached: process.platform !== "win32", terminationOwner: "implementation", + toolingDependencies: "crabbox", + failureTool: "crabbox", }); diff --git a/scripts/lib/tooling-dependencies.d.mts b/scripts/lib/tooling-dependencies.d.mts new file mode 100644 index 000000000000..06ca8b1afa5c --- /dev/null +++ b/scripts/lib/tooling-dependencies.d.mts @@ -0,0 +1,5 @@ +export function toolingDependencyOptions( + checkout: string, + consumer: string, + options?: { tsx?: boolean }, +): { execArgv?: string[]; tsxImport?: string }; diff --git a/scripts/lib/watch-pr-ci-dependencies.mjs b/scripts/lib/tooling-dependencies.mjs similarity index 64% rename from scripts/lib/watch-pr-ci-dependencies.mjs rename to scripts/lib/tooling-dependencies.mjs index b1dd6aa8d4d8..ada3628f6dc8 100644 --- a/scripts/lib/watch-pr-ci-dependencies.mjs +++ b/scripts/lib/tooling-dependencies.mjs @@ -1,10 +1,42 @@ import { spawnSync } from "node:child_process"; import { readFileSync, realpathSync, statSync } from "node:fs"; -import { registerHooks } from "node:module"; -import { dirname, isAbsolute, join, resolve } from "node:path"; -import { pathToFileURL } from "node:url"; +import { createRequire, registerHooks } from "node:module"; +import { dirname, isAbsolute, join, relative, resolve } from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; import { resolveConfiguredModulesDir } from "./tsx-cli-shim.mjs"; +function contained(root, target) { + const physical = realpathSync(target); + const rel = relative(root, physical); + if (!rel || rel === ".." || rel.startsWith("../") || rel.startsWith("..\\") || isAbsolute(rel)) { + throw new Error("Tooling package escapes its installed dependency owner."); + } + return physical; +} + +function qualifiedPackage(checkout, root, specifier, consumer) { + const name = specifier + .split("/") + .slice(0, specifier.startsWith("@") ? 2 : 1) + .join("/"); + const manifest = JSON.parse(readFileSync(join(checkout, "package.json"), "utf8")); + const required = + manifest.dependencies?.[name] ?? + manifest.devDependencies?.[name] ?? + manifest.optionalDependencies?.[name]; + const modules = realpathSync(join(root, "node_modules")); + // A workspace link would execute another checkout's source. Only installed + // third-party packages can fill missing dependencies in this checkout. + const directory = contained(modules, join(modules, name)); + const installed = JSON.parse(readFileSync(join(directory, "package.json"), "utf8")); + if (!required || installed.name !== name || installed.version !== required) { + throw new Error( + `Installed ${consumer} dependency '${name}' has version ${installed.version}; this checkout requires ${required ?? "undeclared"}. Tooling root: ${root}. Refresh that tooling root with pnpm install --frozen-lockfile in a clean main checkout or install dependencies locally.`, + ); + } + return directory; +} + // Keep repository identity aligned with scripts/pr's tooling-root bootstrap. function repository(url, root) { if (/^(?:\.?\.?\/|\/)/.test(url)) { @@ -20,7 +52,7 @@ function repository(url, root) { .toLowerCase()}`; } -export function watchPrCiDependencyOptions(checkout) { +export function toolingDependencyOptions(checkout, consumer, { tsx = false } = {}) { // A configured pnpm modules directory keeps the shim's existing link contract. if ( statSync(join(checkout, "node_modules"), { throwIfNoEntry: false })?.isDirectory() || @@ -93,15 +125,23 @@ export function watchPrCiDependencyOptions(checkout) { const hook = new URL(import.meta.url); hook.searchParams.set("root", root); hook.searchParams.set("checkout", checkout); - console.error(`[watch-pr-ci] resolving missing packages from scripts/pr tooling root ${root}`); + hook.searchParams.set("consumer", consumer); + let tsxImport; + if (tsx) { + const directory = qualifiedPackage(checkout, root, "tsx/esm", consumer); + const require = createRequire(join(directory, "package.json")); + tsxImport = pathToFileURL(contained(directory, require.resolve("tsx/esm"))).href; + } + console.error(`[${consumer}] resolving missing packages from scripts/pr tooling root ${root}`); // A node_modules link would change scripts/pr's wrapper selection in this checkout. - return { execArgv: ["--import", hook.href] }; + return { execArgv: ["--import", hook.href], ...(tsxImport ? { tsxImport } : {}) }; } const params = new URL(import.meta.url).searchParams; const root = params.get("root"); if (root) { const checkout = params.get("checkout"); + const consumer = params.get("consumer"); const parentURL = pathToFileURL(join(root, "package.json")).href; registerHooks({ resolve(specifier, context, nextResolve) { @@ -118,23 +158,8 @@ if (root) { } resolved = nextResolve(specifier, { ...context, parentURL }); } - const name = specifier - .split("/") - .slice(0, specifier.startsWith("@") ? 2 : 1) - .join("/"); - const manifest = JSON.parse(readFileSync(join(checkout, "package.json"), "utf8")); - const requiredVersion = - manifest.dependencies?.[name] ?? - manifest.devDependencies?.[name] ?? - manifest.optionalDependencies?.[name]; - const installedVersion = JSON.parse( - readFileSync(join(root, "node_modules", name, "package.json"), "utf8"), - ).version; - if (!requiredVersion || installedVersion !== requiredVersion) { - throw new Error( - `Installed watch-pr-ci dependency '${name}' has version ${installedVersion}; this checkout requires ${requiredVersion ?? "undeclared"}. Tooling root: ${root}. Refresh that tooling root with pnpm install --frozen-lockfile in a clean main checkout or install dependencies locally.`, - ); - } + const directory = qualifiedPackage(checkout, root, specifier, consumer); + contained(directory, fileURLToPath(resolved.url)); return resolved; }, }); diff --git a/scripts/lib/tsx-cli-shim.d.mts b/scripts/lib/tsx-cli-shim.d.mts index 784e5bc33712..79ba9ae81828 100644 --- a/scripts/lib/tsx-cli-shim.d.mts +++ b/scripts/lib/tsx-cli-shim.d.mts @@ -9,9 +9,11 @@ export type CliShimOptions = { forceKillDelayMs?: number; stdio?: StdioOptions; terminationOwner?: "implementation"; + toolingDependencies?: string; }; export function resolveForwardedNodeCompilerArgs(execArgv?: readonly string[]): string[]; +export function resolveConfiguredModulesDir(checkoutRoot: string): string | undefined; export function resolveTsxImport(checkoutRoot: string): string; export function registerToolingTsx(): Promise; export function runNodeCliShim(moduleUrl: string | URL, options: CliShimOptions): Promise; diff --git a/scripts/lib/tsx-cli-shim.mjs b/scripts/lib/tsx-cli-shim.mjs index 131eca3146c1..e227fd7471fe 100644 --- a/scripts/lib/tsx-cli-shim.mjs +++ b/scripts/lib/tsx-cli-shim.mjs @@ -54,7 +54,7 @@ export function resolveTsxImport(checkoutRoot) { ); } -export async function registerToolingTsx() { +function configureToolingTsx() { // tsx indexes the entire shared disk cache before expiration, coupling startup // to other checkouts' cache size. This flag retains its in-process Map and // reaches descendant tooling before their loaders initialize. @@ -70,6 +70,10 @@ export async function registerToolingTsx() { ) { process.env.TSX_TSCONFIG_PATH = checkoutTsconfig; } +} + +export async function registerToolingTsx() { + configureToolingTsx(); await import(resolveTsxImport(SHIM_CHECKOUT_ROOT)); } @@ -202,6 +206,25 @@ export function runNodeCliShim(moduleUrl, options = {}) { return runCliShim(moduleUrl, options, []); } -export function runTsxCliShim(moduleUrl, options = {}) { +export async function runTsxCliShim(moduleUrl, options = {}) { + if (options.toolingDependencies) { + try { + const { toolingDependencyOptions } = await import("./tooling-dependencies.mjs"); + const tooling = toolingDependencyOptions(SHIM_CHECKOUT_ROOT, options.toolingDependencies, { + tsx: true, + }); + if (tooling.tsxImport) { + configureToolingTsx(); + // Install qualified resolution before TSX loads source imports. Keeping + // its absolute preload also preserves fork() without a workspace link. + return runCliShim(moduleUrl, options, [...tooling.execArgv, "--import", tooling.tsxImport]); + } + } catch (error) { + console.error(error); + writeFailureTrailer(options.failureTool, 1); + process.exitCode = 1; + return; + } + } return runCliShim(moduleUrl, options, ["--import", new URL("../tsx.mjs", import.meta.url).href]); } diff --git a/scripts/pr-lib/wrapper-components.txt b/scripts/pr-lib/wrapper-components.txt index 9a770ca0f19e..f36aa4918c34 100644 --- a/scripts/pr-lib/wrapper-components.txt +++ b/scripts/pr-lib/wrapper-components.txt @@ -177,7 +177,7 @@ scripts/lib/vitest-worker-artifacts.mts scripts/lib/vitest-worker-cache-policy.mts scripts/lib/vitest-worker-declarations.mts scripts/lib/vitest-worker-run.mts -scripts/lib/watch-pr-ci-dependencies.mjs +scripts/lib/tooling-dependencies.mjs scripts/lib/watch-pr-ci-rollup.mts scripts/lib/windows-taskkill.mjs scripts/pnpm-runner.mts diff --git a/scripts/watch-pr-ci.mjs b/scripts/watch-pr-ci.mjs index 5bc085108bc6..b6615cbd7c5f 100644 --- a/scripts/watch-pr-ci.mjs +++ b/scripts/watch-pr-ci.mjs @@ -1,11 +1,11 @@ #!/usr/bin/env node import { fileURLToPath } from "node:url"; +import { toolingDependencyOptions } from "./lib/tooling-dependencies.mjs"; import { runNodeCliShim } from "./lib/tsx-cli-shim.mjs"; -import { watchPrCiDependencyOptions } from "./lib/watch-pr-ci-dependencies.mjs"; try { await runNodeCliShim(import.meta.url, { - ...watchPrCiDependencyOptions(fileURLToPath(new URL("..", import.meta.url))), + ...toolingDependencyOptions(fileURLToPath(new URL("..", import.meta.url)), "watch-pr-ci"), implementation: "./watch-pr-ci.mts", // Native PR supervision owns this pipe; the metadata helper forwards it to gh. // Inheriting only stdin/stdout/stderr would leave its environment flag dangling. diff --git a/test/scripts/direct-run-entrypoints.test-support.ts b/test/scripts/direct-run-entrypoints.test-support.ts index a2a25fb00ca5..d92752c7dbde 100644 --- a/test/scripts/direct-run-entrypoints.test-support.ts +++ b/test/scripts/direct-run-entrypoints.test-support.ts @@ -115,6 +115,10 @@ export async function withShimFixture( "scripts/lib/local-check-runtime.mts", path.join(checkoutRoot, "scripts", "lib", "local-check-runtime.mts"), ); + copyFileSync( + "scripts/lib/tooling-dependencies.mjs", + path.join(checkoutRoot, "scripts", "lib", "tooling-dependencies.mjs"), + ); writeFileSync(path.join(checkoutRoot, "pnpm-lock.yaml"), "lockfileVersion: '9.0'\n"); outcome = { value: await run({ diff --git a/test/scripts/tooling-dependencies.test-support.mts b/test/scripts/tooling-dependencies.test-support.mts new file mode 100644 index 000000000000..d240dbfe328e --- /dev/null +++ b/test/scripts/tooling-dependencies.test-support.mts @@ -0,0 +1,91 @@ +import { spawnSync } from "node:child_process"; +import { copyFileSync, mkdirSync, writeFileSync } from "node:fs"; +import { join, resolve } from "node:path"; + +export function createToolingDependencyFixture(root: string) { + const checkout = join(root, "checkout"); + const tooling = join(root, "tooling #root"); + const lib = join(checkout, "scripts", "lib"); + mkdirSync(lib, { recursive: true }); + for (const directory of [checkout, tooling]) { + for (const args of [ + ["init", "--quiet", directory], + ["-C", directory, "remote", "add", "origin", "https://github.com/openclaw/openclaw.git"], + ]) { + const result = spawnSync("git", args, { encoding: "utf8" }); + if (result.status !== 0) { + throw new Error(result.stderr); + } + } + } + for (const name of ["tsx-cli-shim.mjs", "tooling-dependencies.mjs", "local-check-runtime.mts"]) { + copyFileSync(resolve("scripts/lib", name), join(lib, name)); + } + for (const name of ["tsx.mjs", "crabbox-wrapper.mjs"]) { + copyFileSync(resolve("scripts", name), join(checkout, "scripts", name)); + } + writeFileSync( + join(checkout, "package.json"), + JSON.stringify({ devDependencies: { tsx: "1.0.0", "fixture-pkg": "1.0.0" } }), + ); + function writePackage(name: string, source: string, version = "1.0.0") { + const directory = join(tooling, "node_modules", name); + mkdirSync(directory, { recursive: true }); + writeFileSync( + join(directory, "package.json"), + JSON.stringify({ + name, + version, + type: "module", + exports: name === "tsx" ? { "./esm": "./index.mjs" } : "./index.mjs", + }), + ); + writeFileSync(join(directory, "index.mjs"), source); + return directory; + } + // Exercise the preload contract without installing a compiler in the fixture. + writePackage("tsx", 'process.env.TOOLING_FIXTURE_PRELOADED = "1";'); + writePackage("fixture-pkg", 'export default "qualified";'); + writeFileSync( + join(checkout, "scripts/crabbox-wrapper.mts"), + `import assert from "node:assert/strict"; +import value from "fixture-pkg"; +assert.equal(value, "qualified"); +assert.equal(process.env.TOOLING_FIXTURE_PRELOADED, "1"); +assert.equal(process.env.TSX_DISABLE_CACHE, "1"); +assert.equal(process.argv[2], "--help"); +console.log("qualified bootstrap OK"); +`, + ); + writeFileSync( + join(checkout, "scripts/ordinary.mjs"), + 'import { runTsxCliShim } from "./lib/tsx-cli-shim.mjs"; await runTsxCliShim(import.meta.url, { implementation: "./crabbox-wrapper.mts" });', + ); + const env: NodeJS.ProcessEnv = { + ...process.env, + NODE_OPTIONS: "", + NODE_PATH: "", + OPENCLAW_PR_TOOLING_ROOT: tooling, + }; + for (const name of [ + "PNPM_CONFIG_MODULES_DIR", + "pnpm_config_modules_dir", + "npm_config_modules_dir", + "OPENCLAW_PR_GIT", + "TOOLING_FIXTURE_PRELOADED", + ]) { + delete env[name]; + } + return { + checkout, + tooling, + writePackage, + run: (entrypoint = "crabbox-wrapper.mjs") => + spawnSync(process.execPath, [join(checkout, "scripts", entrypoint), "--help"], { + cwd: root, + env, + encoding: "utf8", + timeout: 10_000, + }), + }; +} diff --git a/test/scripts/tooling-dependencies.test.ts b/test/scripts/tooling-dependencies.test.ts new file mode 100644 index 000000000000..a202d355c8d8 --- /dev/null +++ b/test/scripts/tooling-dependencies.test.ts @@ -0,0 +1,49 @@ +import { existsSync, mkdirSync, renameSync, symlinkSync } from "node:fs"; +import { join } from "node:path"; +import { afterEach, expect, it } from "vitest"; +import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js"; +import { createToolingDependencyFixture } from "./tooling-dependencies.test-support.mts"; + +const tempDirs = useAutoCleanupTempDirTracker(afterEach); + +it("bootstraps the opted-in entrypoint without linking dependencies", () => { + const fixture = createToolingDependencyFixture(tempDirs.make("openclaw-tooling-bootstrap-")); + const result = fixture.run(); + expect(result.status, result.stderr).toBe(0); + expect(result.stdout).toBe("qualified bootstrap OK\n"); + expect(existsSync(join(fixture.checkout, "node_modules"))).toBe(false); + + const ordinary = fixture.run("ordinary.mjs"); + expect(ordinary.status).toBe(1); + expect(ordinary.stderr).toContain("Repository dependencies are missing"); + expect(ordinary.stdout).toBe(""); + expect(existsSync(join(fixture.checkout, "node_modules"))).toBe(false); +}); + +it.each(["tsx", "fixture-pkg"])("rejects stale %s before executing its source", (name) => { + const fixture = createToolingDependencyFixture(tempDirs.make("openclaw-tooling-version-")); + fixture.writePackage(name, 'console.log("STALE PACKAGE EXECUTED");', "0.0.0-stale"); + const result = fixture.run(); + expect(result.status).toBe(1); + expect(result.stderr).toContain(`'${name}' has version 0.0.0-stale`); + expect(result.stderr).toContain("requires 1.0.0"); + expect(result.stderr.trimEnd()).toMatch(/\[crabbox\] FAILED \(exit 1\)$/); + expect(result.stdout + result.stderr).not.toContain("STALE PACKAGE EXECUTED"); + expect(existsSync(join(fixture.checkout, "node_modules"))).toBe(false); +}); + +it.each(["tsx", "fixture-pkg"])("refuses %s linked to another checkout's source", (name) => { + const root = tempDirs.make("openclaw-tooling-workspace-"); + const fixture = createToolingDependencyFixture(root); + const workspace = join(root, "workspace"); + mkdirSync(workspace); + const installed = join(fixture.tooling, "node_modules", name); + const external = join(workspace, name); + renameSync(installed, external); + symlinkSync(external, installed, process.platform === "win32" ? "junction" : "dir"); + const result = fixture.run(); + expect(result.status).toBe(1); + expect(result.stderr).toContain("Tooling package escapes its installed dependency owner"); + expect(result.stdout).toBe(""); + expect(existsSync(join(fixture.checkout, "node_modules"))).toBe(false); +}); diff --git a/test/scripts/watch-pr-ci-dependencies.test.ts b/test/scripts/watch-pr-ci-dependencies.test.ts index dcf5d5054aa1..923947f580f4 100644 --- a/test/scripts/watch-pr-ci-dependencies.test.ts +++ b/test/scripts/watch-pr-ci-dependencies.test.ts @@ -2,7 +2,7 @@ import { spawnSync } from "node:child_process"; import { copyFileSync, existsSync, mkdirSync, symlinkSync, writeFileSync } from "node:fs"; import { join, resolve } from "node:path"; import { afterEach, expect, it, vi } from "vitest"; -import { watchPrCiDependencyOptions } from "../../scripts/lib/watch-pr-ci-dependencies.mjs"; +import { toolingDependencyOptions } from "../../scripts/lib/tooling-dependencies.mjs"; import { requireNodeTool } from "../helpers/node-toolchain.js"; import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js"; @@ -104,7 +104,7 @@ it.each([ } else { vi.stubEnv("PNPM_CONFIG_MODULES_DIR", join(tooling, "node_modules")); } - expect(watchPrCiDependencyOptions(checkout)).toEqual({}); + expect(toolingDependencyOptions(checkout, "watch-pr-ci")).toEqual({}); expect(notice).not.toHaveBeenCalled(); } else if (["missing", "different", "subdirectory", "sparse"].includes(source)) { const reasons: Record = { @@ -113,10 +113,10 @@ it.each([ subdirectory: "Not a repository top level", sparse: "Sparse checkout", }; - expect(() => watchPrCiDependencyOptions(checkout)).toThrow(reasons[source]); + expect(() => toolingDependencyOptions(checkout, "watch-pr-ci")).toThrow(reasons[source]); expect(notice).not.toHaveBeenCalled(); } else { - watchPrCiDependencyOptions(checkout); + toolingDependencyOptions(checkout, "watch-pr-ci"); expect(notice.mock.calls).toEqual([ [ `[watch-pr-ci] resolving missing packages from scripts/pr tooling root ${source === "config" ? tooling : canonical}`, @@ -153,11 +153,7 @@ it("checks fallback versions before loading through the watcher child and preser optionalDependencies: { "@fixture/scoped": "1.0.0" }, }), ); - for (const file of [ - "tsx-cli-shim.mjs", - "local-check-runtime.mts", - "watch-pr-ci-dependencies.mjs", - ]) { + for (const file of ["tsx-cli-shim.mjs", "local-check-runtime.mts", "tooling-dependencies.mjs"]) { copyFileSync(resolve("scripts/lib", file), join(lib, file)); } copyFileSync(resolve("scripts/watch-pr-ci.mjs"), join(checkout, "scripts/watch-pr-ci.mjs")); @@ -166,7 +162,12 @@ it("checks fallback versions before loading through the watcher child and preser writePackage(tooling, "@fixture/scoped", "scoped"); writeFileSync( join(tooling, "node_modules/@fixture/scoped/package.json"), - JSON.stringify({ version: "1.0.0", type: "module", exports: { "./subpath": "./index.mjs" } }), + JSON.stringify({ + name: "@fixture/scoped", + version: "1.0.0", + type: "module", + exports: { "./subpath": "./index.mjs" }, + }), ); writePackage(tooling, "local-pkg", "fallback"); writeFileSync(