From b40d4b63f602088b06d2b94e4c29e0af90cc8106 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 3 Apr 2026 23:24:00 +0900 Subject: [PATCH] refactor: centralize update targets and extension guardrails --- .../check-extension-plugin-sdk-boundary.mjs | 14 +--- scripts/check-no-extension-src-imports.ts | 26 +----- scripts/lib/extension-source-classifier.d.mts | 14 ++++ scripts/lib/extension-source-classifier.mjs | 42 ++++++++++ scripts/pnpm-runner.mjs | 20 +++++ scripts/test-live.mjs | 12 +-- scripts/test-projects.mjs | 12 +-- .../channel-import-guardrails.test.ts | 15 +--- src/cli/update-cli.test.ts | 64 ++++++++++++++ src/cli/update-cli/update-command.ts | 30 +++++-- src/infra/update-global.test.ts | 22 +++++ src/infra/update-global.ts | 83 +++++++++++++++---- src/infra/update-runner.ts | 20 ++++- .../extension-source-classifier.test.ts | 49 +++++++++++ test/scripts/pnpm-runner.test.ts | 23 ++++- 15 files changed, 354 insertions(+), 92 deletions(-) create mode 100644 scripts/lib/extension-source-classifier.d.mts create mode 100644 scripts/lib/extension-source-classifier.mjs create mode 100644 test/scripts/extension-source-classifier.test.ts diff --git a/scripts/check-extension-plugin-sdk-boundary.mjs b/scripts/check-extension-plugin-sdk-boundary.mjs index e335ded206..d8c82b8312 100644 --- a/scripts/check-extension-plugin-sdk-boundary.mjs +++ b/scripts/check-extension-plugin-sdk-boundary.mjs @@ -8,6 +8,7 @@ import { BUNDLED_PLUGIN_PATH_PREFIX, BUNDLED_PLUGIN_ROOT_DIR, } from "./lib/bundled-plugin-paths.mjs"; +import { classifyBundledExtensionSourcePath } from "./lib/extension-source-classifier.mjs"; import { diffInventoryEntries, normalizeRepoPath, @@ -63,17 +64,6 @@ function isCodeFile(fileName) { return /\.(ts|tsx|mts|cts|js|jsx|mjs|cjs)$/.test(fileName); } -function isTestLikeFile(relativePath) { - return ( - /(^|\/)(__tests__|fixtures|test|tests|test-support)\//.test(relativePath) || - /(^|\/)test-support\.(ts|tsx|mts|cts|js|jsx|mjs|cjs)$/.test(relativePath) || - /(^|\/)[^/]*test-(support|helpers|fixtures)\.(ts|tsx|mts|cts|js|jsx|mjs|cjs)$/.test( - relativePath, - ) || - /\.(test|spec)\.(ts|tsx|mts|cts|js|jsx|mjs|cjs)$/.test(relativePath) - ); -} - async function collectExtensionSourceFiles(rootDir) { const out = []; async function walk(dir) { @@ -91,7 +81,7 @@ async function collectExtensionSourceFiles(rootDir) { continue; } const relativePath = normalizeRepoPath(repoRoot, fullPath); - if (isTestLikeFile(relativePath)) { + if (classifyBundledExtensionSourcePath(relativePath).isTestLike) { continue; } out.push(fullPath); diff --git a/scripts/check-no-extension-src-imports.ts b/scripts/check-no-extension-src-imports.ts index 330f3cccf8..1ed9ccf7cf 100644 --- a/scripts/check-no-extension-src-imports.ts +++ b/scripts/check-no-extension-src-imports.ts @@ -1,34 +1,14 @@ import fs from "node:fs"; import path from "node:path"; import { collectFilesSync, isCodeFile, relativeToCwd } from "./check-file-utils.js"; +import { classifyBundledExtensionSourcePath } from "./lib/extension-source-classifier.mjs"; const FORBIDDEN_REPO_SRC_IMPORT = /["'](?:\.\.\/)+(?:src\/)[^"']+["']/; -function isProductionExtensionFile(filePath: string): boolean { - return !( - filePath.endsWith("/runtime-api.ts") || - filePath.endsWith("\\runtime-api.ts") || - filePath.includes(".test.") || - filePath.includes(".spec.") || - filePath.includes(".fixture.") || - filePath.includes(".snap") || - filePath.includes("test-harness") || - filePath.includes("test-support") || - filePath.includes("test-helpers") || - filePath.includes("test-fixtures") || - filePath.includes("/__tests__/") || - filePath.includes("/fixtures/") || - filePath.includes("/test/") || - filePath.includes("/tests/") || - filePath.includes("/coverage/") || - filePath.includes("/dist/") || - filePath.includes("/node_modules/") - ); -} - function collectExtensionSourceFiles(rootDir: string): string[] { return collectFilesSync(rootDir, { - includeFile: (filePath) => isCodeFile(filePath) && isProductionExtensionFile(filePath), + includeFile: (filePath) => + isCodeFile(filePath) && classifyBundledExtensionSourcePath(filePath).isProductionSource, }); } diff --git a/scripts/lib/extension-source-classifier.d.mts b/scripts/lib/extension-source-classifier.d.mts new file mode 100644 index 0000000000..f5c5fc657f --- /dev/null +++ b/scripts/lib/extension-source-classifier.d.mts @@ -0,0 +1,14 @@ +export type BundledExtensionSourceClassification = { + normalizedPath: string; + isCodeFile: boolean; + isRuntimeApiBarrel: boolean; + isPublicApiBarrel: boolean; + isTestLike: boolean; + isInfraArtifact: boolean; + isProductionSource: boolean; +}; + +export function normalizeExtensionSourcePath(filePath: string): string; +export function classifyBundledExtensionSourcePath( + filePath: string, +): BundledExtensionSourceClassification; diff --git a/scripts/lib/extension-source-classifier.mjs b/scripts/lib/extension-source-classifier.mjs new file mode 100644 index 0000000000..88bb3bc1a7 --- /dev/null +++ b/scripts/lib/extension-source-classifier.mjs @@ -0,0 +1,42 @@ +const CODE_FILE_RE = /\.(?:[cm]?ts|[cm]?js|tsx|jsx)$/u; +const DECLARATION_FILE_RE = /\.d\.ts$/u; +const RUNTIME_API_BARREL_RE = /(^|\/)runtime-api\.(?:[cm]?ts|[cm]?js|tsx|jsx)$/u; +const PUBLIC_API_BARREL_RE = /(^|\/)api\.(?:[cm]?ts|[cm]?js|tsx|jsx)$/u; +const TEST_LIKE_SEGMENT_RE = + /(^|\/)(?:__tests__|fixtures|test|tests|test-fixtures|test-support|test-utils)(?:\/|$)/u; +const TEST_LIKE_FILENAME_RE = + /(^|\/)[^/]*test-(?:support|helpers|fixtures|harness)\.(?:[cm]?ts|[cm]?js|tsx|jsx)$/u; +const TEST_SHARED_FILENAME_RE = /(^|\/)[^/]*\.test-[^/]*\.(?:[cm]?ts|[cm]?js|tsx|jsx)$/u; +const SNAPSHOT_FILE_RE = /\.snap$/u; +const SUFFIX_SKIP_RE = /\.(?:test|spec|fixture)\./u; +const INFRA_DIR_RE = /(^|\/)(?:coverage|dist|node_modules)(?:\/|$)/u; +const INFRA_NAME_RE = /(test-harness|test-support|test-helpers|test-fixtures)/u; + +export function normalizeExtensionSourcePath(filePath) { + return filePath.replaceAll("\\", "/"); +} + +export function classifyBundledExtensionSourcePath(filePath) { + const normalizedPath = normalizeExtensionSourcePath(filePath); + const isCodeFile = CODE_FILE_RE.test(normalizedPath) && !DECLARATION_FILE_RE.test(normalizedPath); + const isRuntimeApiBarrel = RUNTIME_API_BARREL_RE.test(normalizedPath); + const isPublicApiBarrel = PUBLIC_API_BARREL_RE.test(normalizedPath); + const isTestLike = + TEST_LIKE_SEGMENT_RE.test(normalizedPath) || + TEST_LIKE_FILENAME_RE.test(normalizedPath) || + TEST_SHARED_FILENAME_RE.test(normalizedPath) || + SUFFIX_SKIP_RE.test(normalizedPath) || + SNAPSHOT_FILE_RE.test(normalizedPath) || + INFRA_NAME_RE.test(normalizedPath); + const isInfraArtifact = INFRA_DIR_RE.test(normalizedPath); + + return { + normalizedPath, + isCodeFile, + isRuntimeApiBarrel, + isPublicApiBarrel, + isTestLike, + isInfraArtifact, + isProductionSource: isCodeFile && !isRuntimeApiBarrel && !isTestLike && !isInfraArtifact, + }; +} diff --git a/scripts/pnpm-runner.mjs b/scripts/pnpm-runner.mjs index a954a92878..d2a95ce3a4 100644 --- a/scripts/pnpm-runner.mjs +++ b/scripts/pnpm-runner.mjs @@ -1,3 +1,4 @@ +import { spawn } from "node:child_process"; import path from "node:path"; const WINDOWS_UNSAFE_CMD_CHARS_RE = /[&|<>%\r\n]/; @@ -51,3 +52,22 @@ export function resolvePnpmRunner(params = {}) { shell: false, }; } + +export function createPnpmRunnerSpawnSpec(params = {}) { + const runner = resolvePnpmRunner(params); + return { + command: runner.command, + args: runner.args, + options: { + stdio: params.stdio ?? "inherit", + env: params.env ?? runner.env ?? process.env, + shell: runner.shell, + windowsVerbatimArguments: runner.windowsVerbatimArguments, + }, + }; +} + +export function spawnPnpmRunner(params = {}) { + const spawnSpec = createPnpmRunnerSpawnSpec(params); + return spawn(spawnSpec.command, spawnSpec.args, spawnSpec.options); +} diff --git a/scripts/test-live.mjs b/scripts/test-live.mjs index 57cb5e9f1b..2a4d1ce49e 100644 --- a/scripts/test-live.mjs +++ b/scripts/test-live.mjs @@ -1,5 +1,4 @@ -import { spawn } from "node:child_process"; -import { resolvePnpmRunner } from "./pnpm-runner.mjs"; +import { spawnPnpmRunner } from "./pnpm-runner.mjs"; const forwardedArgs = []; let quietOverride; @@ -25,14 +24,9 @@ const env = { OPENCLAW_LIVE_TEST_QUIET: quietOverride ?? process.env.OPENCLAW_LIVE_TEST_QUIET ?? "1", }; -const pnpmRunner = resolvePnpmRunner({ +const child = spawnPnpmRunner({ pnpmArgs: ["exec", "vitest", "run", "--config", "vitest.live.config.ts", ...forwardedArgs], -}); -const child = spawn(pnpmRunner.command, pnpmRunner.args, { - stdio: "inherit", - env: pnpmRunner.env ?? env, - shell: pnpmRunner.shell, - windowsVerbatimArguments: pnpmRunner.windowsVerbatimArguments, + env, }); child.on("exit", (code, signal) => { diff --git a/scripts/test-projects.mjs b/scripts/test-projects.mjs index 7f12a38c9a..be82a036d2 100644 --- a/scripts/test-projects.mjs +++ b/scripts/test-projects.mjs @@ -1,10 +1,8 @@ -import { spawn } from "node:child_process"; import { acquireLocalHeavyCheckLockSync } from "./lib/local-heavy-check-runtime.mjs"; -import { resolvePnpmRunner } from "./pnpm-runner.mjs"; +import { spawnPnpmRunner } from "./pnpm-runner.mjs"; import { buildVitestArgs } from "./test-projects.test-support.mjs"; const vitestArgs = buildVitestArgs(process.argv.slice(2)); -const pnpmRunner = resolvePnpmRunner({ pnpmArgs: vitestArgs }); const releaseLock = acquireLocalHeavyCheckLockSync({ cwd: process.cwd(), env: process.env, @@ -21,11 +19,9 @@ const releaseLockOnce = () => { releaseLock(); }; -const child = spawn(pnpmRunner.command, pnpmRunner.args, { - stdio: "inherit", - env: pnpmRunner.env ?? process.env, - shell: pnpmRunner.shell, - windowsVerbatimArguments: pnpmRunner.windowsVerbatimArguments, +const child = spawnPnpmRunner({ + pnpmArgs: vitestArgs, + env: process.env, }); child.on("exit", (code, signal) => { diff --git a/src/channels/plugins/contracts/channel-import-guardrails.test.ts b/src/channels/plugins/contracts/channel-import-guardrails.test.ts index 6b84fa1648..731530dd94 100644 --- a/src/channels/plugins/contracts/channel-import-guardrails.test.ts +++ b/src/channels/plugins/contracts/channel-import-guardrails.test.ts @@ -2,6 +2,7 @@ import { readdirSync, readFileSync } from "node:fs"; import { dirname, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import { describe, expect, it } from "vitest"; +import { classifyBundledExtensionSourcePath } from "../../../../scripts/lib/extension-source-classifier.mjs"; import { BUNDLED_PLUGIN_PATH_PREFIX, BUNDLED_PLUGIN_ROOT_DIR, @@ -328,11 +329,7 @@ function collectExtensionSourceFiles(): string[] { normalizedFullPath.includes(sharedExtensionsDir) || normalizedFullPath.includes(`${extensionsDir}/shared/`), shouldSkipEntry: ({ entryName, normalizedFullPath }) => - normalizedFullPath.includes(".test.") || - normalizedFullPath.includes(".test-") || - normalizedFullPath.includes(".fixture.") || - normalizedFullPath.includes(".snap") || - normalizedFullPath.includes("test-support") || + classifyBundledExtensionSourcePath(normalizedFullPath).isTestLike || entryName === "api.ts" || entryName === "runtime-api.ts", }); @@ -368,13 +365,7 @@ function collectExtensionFiles(extensionId: string): string[] { const files = collectSourceFiles(cached, { rootDir: resolve(ROOT_DIR, "..", "extensions", extensionId), shouldSkipEntry: ({ entryName, normalizedFullPath }) => - normalizedFullPath.includes(".test.") || - normalizedFullPath.includes(".test-") || - normalizedFullPath.includes(".spec.") || - normalizedFullPath.includes(".fixture.") || - normalizedFullPath.includes(".snap") || - normalizedFullPath.includes("test-support") || - entryName === "test-support.ts" || + classifyBundledExtensionSourcePath(normalizedFullPath).isTestLike || entryName === "runtime-api.ts", }); extensionFilesCache.set(extensionId, files); diff --git a/src/cli/update-cli.test.ts b/src/cli/update-cli.test.ts index 73e7285be4..f638db13af 100644 --- a/src/cli/update-cli.test.ts +++ b/src/cli/update-cli.test.ts @@ -745,6 +745,70 @@ describe("update-cli", () => { expect(logs.join("\n")).toContain("expected installed version 2026.3.23-2, found 2026.3.23"); }); + it("uses the owning npm binary for package updates when PATH npm points elsewhere", async () => { + const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("darwin"); + const brewPrefix = createCaseDir("brew-prefix"); + const brewRoot = path.join(brewPrefix, "lib", "node_modules"); + const pkgRoot = path.join(brewRoot, "openclaw"); + const brewNpm = path.join(brewPrefix, "bin", "npm"); + const pathNpmRoot = createCaseDir("nvm-root"); + mockPackageInstallStatus(pkgRoot); + pathExists.mockResolvedValue(false); + + vi.mocked(runCommandWithTimeout).mockImplementation(async (argv) => { + if (!Array.isArray(argv)) { + return { + stdout: "", + stderr: "", + code: 0, + signal: null, + killed: false, + termination: "exit", + }; + } + if (argv[0] === "npm" && argv[1] === "root" && argv[2] === "-g") { + return { + stdout: `${pathNpmRoot}\n`, + stderr: "", + code: 0, + signal: null, + killed: false, + termination: "exit", + }; + } + if (argv[0] === brewNpm && argv[1] === "root" && argv[2] === "-g") { + return { + stdout: `${brewRoot}\n`, + stderr: "", + code: 0, + signal: null, + killed: false, + termination: "exit", + }; + } + return { + stdout: "", + stderr: "", + code: 0, + signal: null, + killed: false, + termination: "exit", + }; + }); + + await fs.mkdir(path.dirname(brewNpm), { recursive: true }); + await fs.writeFile(brewNpm, "", "utf8"); + await updateCommand({ yes: true }); + + platformSpy.mockRestore(); + + expect(runGatewayUpdate).not.toHaveBeenCalled(); + expect(runCommandWithTimeout).toHaveBeenCalledWith( + [brewNpm, "i", "-g", "openclaw@latest", "--no-fund", "--no-audit", "--loglevel=error"], + expect.any(Object), + ); + }); + it("prepends portable Git PATH for package updates on Windows", async () => { const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); const tempDir = createCaseDir("openclaw-update"); diff --git a/src/cli/update-cli/update-command.ts b/src/cli/update-cli/update-command.ts index aa28178b94..363a280798 100644 --- a/src/cli/update-cli/update-command.ts +++ b/src/cli/update-cli/update-command.ts @@ -33,8 +33,8 @@ import { cleanupGlobalRenameDirs, globalInstallArgs, resolveExpectedInstalledVersionFromSpec, + resolveGlobalInstallTarget, resolveGlobalInstallSpec, - resolveGlobalPackageRoot, } from "../../infra/update-global.js"; import { runGatewayUpdate, type UpdateRunResult } from "../../infra/update-runner.js"; import { syncPluginsForUpdateChannel, updateNpmInstalledPlugins } from "../../plugins/update.js"; @@ -350,8 +350,13 @@ async function runPackageInstallUpdate(params: { }); const installEnv = await createGlobalInstallEnv(); const runCommand = createGlobalCommandRunner(); - - const pkgRoot = await resolveGlobalPackageRoot(manager, runCommand, params.timeoutMs, params.root); + const installTarget = await resolveGlobalInstallTarget({ + manager, + runCommand, + timeoutMs: params.timeoutMs, + pkgRoot: params.root, + }); + const pkgRoot = installTarget.packageRoot; const packageName = (pkgRoot ? await readPackageName(pkgRoot) : await readPackageName(params.root)) ?? DEFAULT_PACKAGE_NAME; @@ -371,7 +376,7 @@ async function runPackageInstallUpdate(params: { const updateStep = await runUpdateStep({ name: "global update", - argv: globalInstallArgs(manager, installSpec, params.root), + argv: globalInstallArgs(installTarget, installSpec), env: installEnv, timeoutMs: params.timeoutMs, progress: params.progress, @@ -381,7 +386,13 @@ async function runPackageInstallUpdate(params: { let afterVersion = beforeVersion; const verifiedPackageRoot = - (await resolveGlobalPackageRoot(manager, runCommand, params.timeoutMs, params.root)) ?? pkgRoot; + ( + await resolveGlobalInstallTarget({ + manager: installTarget, + runCommand, + timeoutMs: params.timeoutMs, + }) + ).packageRoot ?? pkgRoot; if (verifiedPackageRoot) { afterVersion = await readPackageVersion(verifiedPackageRoot); const expectedVersion = resolveExpectedInstalledVersionFromSpec(packageName, installSpec); @@ -482,9 +493,16 @@ async function runGitUpdate(params: { installKind: params.installKind, timeoutMs: effectiveTimeout, }); + const runCommand = createGlobalCommandRunner(); + const installTarget = await resolveGlobalInstallTarget({ + manager, + runCommand, + timeoutMs: effectiveTimeout, + pkgRoot: params.root, + }); const installStep = await runUpdateStep({ name: "global install", - argv: globalInstallArgs(manager, updateRoot, params.root), + argv: globalInstallArgs(installTarget, updateRoot), cwd: updateRoot, env: installEnv, timeoutMs: effectiveTimeout, diff --git a/src/infra/update-global.test.ts b/src/infra/update-global.test.ts index 46335b609e..81ce29368c 100644 --- a/src/infra/update-global.test.ts +++ b/src/infra/update-global.test.ts @@ -16,7 +16,9 @@ import { isExplicitPackageInstallSpec, isMainPackageTarget, OPENCLAW_MAIN_PACKAGE_SPEC, + resolveGlobalInstallCommand, resolveGlobalPackageRoot, + resolveGlobalInstallTarget, resolveGlobalInstallSpec, resolveGlobalRoot, type CommandRunner, @@ -170,6 +172,19 @@ describe("update global helpers", () => { await expect(resolveGlobalPackageRoot("npm", runCommand, 1000, pkgRoot)).resolves.toBe( pkgRoot, ); + await expect( + resolveGlobalInstallTarget({ + manager: "npm", + runCommand, + timeoutMs: 1000, + pkgRoot, + }), + ).resolves.toEqual({ + manager: "npm", + command: brewNpm, + globalRoot: brewRoot, + packageRoot: pkgRoot, + }); expect(globalInstallArgs("npm", "openclaw@latest", pkgRoot)).toEqual([ brewNpm, "i", @@ -263,6 +278,10 @@ describe("update global helpers", () => { }); it("builds install argv and npm fallback argv", () => { + expect(resolveGlobalInstallCommand("npm")).toEqual({ + manager: "npm", + command: "npm", + }); expect(globalInstallArgs("npm", "openclaw@latest")).toEqual([ "npm", "i", @@ -296,6 +315,9 @@ describe("update global helpers", () => { "--loglevel=error", ]); expect(globalInstallFallbackArgs("pnpm", "openclaw@latest")).toBeNull(); + expect( + globalInstallArgs({ manager: "pnpm", command: "/opt/homebrew/bin/pnpm" }, "openclaw@latest"), + ).toEqual(["/opt/homebrew/bin/pnpm", "add", "-g", "openclaw@latest"]); }); it("cleans only renamed package directories", async () => { diff --git a/src/infra/update-global.ts b/src/infra/update-global.ts index 5462ceb7cb..877006d0af 100644 --- a/src/infra/update-global.ts +++ b/src/infra/update-global.ts @@ -14,6 +14,16 @@ export type CommandRunner = ( options: { timeoutMs: number; cwd?: string; env?: NodeJS.ProcessEnv }, ) => Promise<{ stdout: string; stderr: string; code: number | null }>; +export type ResolvedGlobalInstallCommand = { + manager: GlobalInstallManager; + command: string; +}; + +export type ResolvedGlobalInstallTarget = ResolvedGlobalInstallCommand & { + globalRoot: string | null; + packageRoot: string | null; +}; + const PRIMARY_PACKAGE_NAME = "openclaw"; const ALL_PACKAGE_NAMES = [PRIMARY_PACKAGE_NAME] as const; const GLOBAL_RENAME_PREFIX = "."; @@ -221,17 +231,36 @@ function resolvePreferredGlobalManagerCommand( return resolvePreferredNpmCommand(pkgRoot) ?? manager; } -export async function resolveGlobalRoot( +export function resolveGlobalInstallCommand( manager: GlobalInstallManager, + pkgRoot?: string | null, +): ResolvedGlobalInstallCommand { + return { + manager, + command: resolvePreferredGlobalManagerCommand(manager, pkgRoot), + }; +} + +function normalizeGlobalInstallCommand( + managerOrCommand: GlobalInstallManager | ResolvedGlobalInstallCommand, + pkgRoot?: string | null, +): ResolvedGlobalInstallCommand { + return typeof managerOrCommand === "string" + ? resolveGlobalInstallCommand(managerOrCommand, pkgRoot) + : managerOrCommand; +} + +export async function resolveGlobalRoot( + managerOrCommand: GlobalInstallManager | ResolvedGlobalInstallCommand, runCommand: CommandRunner, timeoutMs: number, pkgRoot?: string | null, ): Promise { - if (manager === "bun") { + const resolved = normalizeGlobalInstallCommand(managerOrCommand, pkgRoot); + if (resolved.manager === "bun") { return resolveBunGlobalRoot(); } - const command = resolvePreferredGlobalManagerCommand(manager, pkgRoot); - const argv = [command, "root", "-g"]; + const argv = [resolved.command, "root", "-g"]; const res = await runCommand(argv, { timeoutMs }).catch(() => null); if (!res || res.code !== 0) { return null; @@ -241,18 +270,38 @@ export async function resolveGlobalRoot( } export async function resolveGlobalPackageRoot( - manager: GlobalInstallManager, + managerOrCommand: GlobalInstallManager | ResolvedGlobalInstallCommand, runCommand: CommandRunner, timeoutMs: number, pkgRoot?: string | null, ): Promise { - const root = await resolveGlobalRoot(manager, runCommand, timeoutMs, pkgRoot); + const root = await resolveGlobalRoot(managerOrCommand, runCommand, timeoutMs, pkgRoot); if (!root) { return null; } return path.join(root, PRIMARY_PACKAGE_NAME); } +export async function resolveGlobalInstallTarget(params: { + manager: GlobalInstallManager | ResolvedGlobalInstallCommand; + runCommand: CommandRunner; + timeoutMs: number; + pkgRoot?: string | null; +}): Promise { + const command = normalizeGlobalInstallCommand(params.manager, params.pkgRoot); + const globalRoot = await resolveGlobalRoot( + command, + params.runCommand, + params.timeoutMs, + params.pkgRoot, + ); + return { + ...command, + globalRoot, + packageRoot: globalRoot ? path.join(globalRoot, PRIMARY_PACKAGE_NAME) : null, + }; +} + export async function detectGlobalInstallManagerForRoot( runCommand: CommandRunner, pkgRoot: string, @@ -330,30 +379,30 @@ export async function detectGlobalInstallManagerByPresence( } export function globalInstallArgs( - manager: GlobalInstallManager, + managerOrCommand: GlobalInstallManager | ResolvedGlobalInstallCommand, spec: string, pkgRoot?: string | null, ): string[] { - const command = resolvePreferredGlobalManagerCommand(manager, pkgRoot); - if (manager === "pnpm") { - return [command, "add", "-g", spec]; + const resolved = normalizeGlobalInstallCommand(managerOrCommand, pkgRoot); + if (resolved.manager === "pnpm") { + return [resolved.command, "add", "-g", spec]; } - if (manager === "bun") { - return [command, "add", "-g", spec]; + if (resolved.manager === "bun") { + return [resolved.command, "add", "-g", spec]; } - return [command, "i", "-g", spec, ...NPM_GLOBAL_INSTALL_QUIET_FLAGS]; + return [resolved.command, "i", "-g", spec, ...NPM_GLOBAL_INSTALL_QUIET_FLAGS]; } export function globalInstallFallbackArgs( - manager: GlobalInstallManager, + managerOrCommand: GlobalInstallManager | ResolvedGlobalInstallCommand, spec: string, pkgRoot?: string | null, ): string[] | null { - if (manager !== "npm") { + const resolved = normalizeGlobalInstallCommand(managerOrCommand, pkgRoot); + if (resolved.manager !== "npm") { return null; } - const command = resolvePreferredGlobalManagerCommand(manager, pkgRoot); - return [command, "i", "-g", spec, ...NPM_GLOBAL_INSTALL_OMIT_OPTIONAL_FLAGS]; + return [resolved.command, "i", "-g", spec, ...NPM_GLOBAL_INSTALL_OMIT_OPTIONAL_FLAGS]; } export async function cleanupGlobalRenameDirs(params: { diff --git a/src/infra/update-runner.ts b/src/infra/update-runner.ts index 9f85efe045..be8be307b1 100644 --- a/src/infra/update-runner.ts +++ b/src/infra/update-runner.ts @@ -28,8 +28,8 @@ import { globalInstallArgs, globalInstallFallbackArgs, resolveExpectedInstalledVersionFromSpec, + resolveGlobalInstallTarget, resolveGlobalInstallSpec, - resolveGlobalPackageRoot, } from "./update-global.js"; export type UpdateStepResult = { @@ -1001,6 +1001,12 @@ export async function runGatewayUpdate(opts: UpdateRunnerOptions = {}): Promise< const beforeVersion = await readPackageVersion(pkgRoot); const globalManager = await detectGlobalInstallManagerForRoot(runCommand, pkgRoot, timeoutMs); if (globalManager) { + const installTarget = await resolveGlobalInstallTarget({ + manager: globalManager, + runCommand, + timeoutMs, + pkgRoot, + }); const packageName = (await readPackageName(pkgRoot)) ?? DEFAULT_PACKAGE_NAME; await cleanupGlobalRenameDirs({ globalRoot: path.dirname(pkgRoot), @@ -1018,7 +1024,7 @@ export async function runGatewayUpdate(opts: UpdateRunnerOptions = {}): Promise< const updateStep = await runStep({ runCommand, name: "global update", - argv: globalInstallArgs(globalManager, spec, pkgRoot), + argv: globalInstallArgs(installTarget, spec), cwd: pkgRoot, timeoutMs, env: globalInstallEnv, @@ -1030,7 +1036,7 @@ export async function runGatewayUpdate(opts: UpdateRunnerOptions = {}): Promise< let finalStep = updateStep; if (updateStep.exitCode !== 0) { - const fallbackArgv = globalInstallFallbackArgs(globalManager, spec, pkgRoot); + const fallbackArgv = globalInstallFallbackArgs(installTarget, spec); if (fallbackArgv) { const fallbackStep = await runStep({ runCommand, @@ -1049,7 +1055,13 @@ export async function runGatewayUpdate(opts: UpdateRunnerOptions = {}): Promise< } const verifiedPackageRoot = - (await resolveGlobalPackageRoot(globalManager, runCommand, timeoutMs, pkgRoot)) ?? pkgRoot; + ( + await resolveGlobalInstallTarget({ + manager: installTarget, + runCommand, + timeoutMs, + }) + ).packageRoot ?? pkgRoot; const expectedVersion = resolveExpectedInstalledVersionFromSpec(packageName, spec); const verificationErrors = await collectInstalledGlobalPackageErrors({ packageRoot: verifiedPackageRoot, diff --git a/test/scripts/extension-source-classifier.test.ts b/test/scripts/extension-source-classifier.test.ts new file mode 100644 index 0000000000..aebf87e605 --- /dev/null +++ b/test/scripts/extension-source-classifier.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from "vitest"; +import { classifyBundledExtensionSourcePath } from "../../scripts/lib/extension-source-classifier.mjs"; + +describe("classifyBundledExtensionSourcePath", () => { + it("treats runtime barrels as non-production source", () => { + expect(classifyBundledExtensionSourcePath("extensions/msteams/runtime-api.ts")).toMatchObject({ + isCodeFile: true, + isRuntimeApiBarrel: true, + isTestLike: false, + isProductionSource: false, + }); + }); + + it("treats extension tests and fixtures as test-like across naming styles", () => { + expect( + classifyBundledExtensionSourcePath("extensions/feishu/src/monitor-handler.test.ts"), + ).toMatchObject({ + isTestLike: true, + isProductionSource: false, + }); + expect( + classifyBundledExtensionSourcePath("extensions/discord/src/test-fixtures/message.ts"), + ).toMatchObject({ + isTestLike: true, + isProductionSource: false, + }); + expect( + classifyBundledExtensionSourcePath("extensions/telegram/src/bot.test-harness.ts"), + ).toMatchObject({ + isTestLike: true, + isProductionSource: false, + }); + expect( + classifyBundledExtensionSourcePath("extensions/telegram/src/target-writeback.test-shared.ts"), + ).toMatchObject({ + isTestLike: true, + isProductionSource: false, + }); + }); + + it("keeps normal extension production files eligible for guardrails", () => { + expect(classifyBundledExtensionSourcePath("extensions/msteams/src/send.ts")).toMatchObject({ + isCodeFile: true, + isRuntimeApiBarrel: false, + isTestLike: false, + isProductionSource: true, + }); + }); +}); diff --git a/test/scripts/pnpm-runner.test.ts b/test/scripts/pnpm-runner.test.ts index d4e645a500..84a0385897 100644 --- a/test/scripts/pnpm-runner.test.ts +++ b/test/scripts/pnpm-runner.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { resolvePnpmRunner } from "../../scripts/pnpm-runner.mjs"; +import { createPnpmRunnerSpawnSpec, resolvePnpmRunner } from "../../scripts/pnpm-runner.mjs"; describe("resolvePnpmRunner", () => { it("uses npm_execpath when it points to pnpm", () => { @@ -67,4 +67,25 @@ describe("resolvePnpmRunner", () => { windowsVerbatimArguments: true, }); }); + + it("builds a shared spawn spec with inherited stdio and env overrides", () => { + const env = { PATH: "/custom/bin", FOO: "bar" }; + expect( + createPnpmRunnerSpawnSpec({ + npmExecPath: "", + pnpmArgs: ["exec", "vitest", "run"], + platform: "linux", + env, + }), + ).toEqual({ + command: "pnpm", + args: ["exec", "vitest", "run"], + options: { + stdio: "inherit", + env, + shell: false, + windowsVerbatimArguments: undefined, + }, + }); + }); });