From 04cfad2d7e6b41f54095dfe9be1a2062f18b0227 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Mon, 7 Sep 2026 10:25:28 +0800 Subject: [PATCH] fix: changed checks misclassify unusual Git filenames (#126198) Preserve exact Git pathname tokens through changed-check routing while normalizing explicit operator paths only at CLI ingress. Cover leading-space and newline filenames with real Git regressions, and keep staged delegation portable on Windows. Co-authored-by: qingminlong --- scripts/changed-lanes.mts | 54 +++++------ scripts/check-changed.mts | 61 ++++++------ scripts/ci-changed-scope.mjs | 21 +++-- scripts/generate-npm-package-lock.mts | 6 +- scripts/lib/changed-extensions.mts | 18 +--- scripts/lib/changed-path-facts.mjs | 11 +-- scripts/lib/ci-node-test-plan.mts | 11 +-- scripts/lib/extension-test-plan.mts | 7 +- scripts/test-projects.test-support.mts | 35 +++---- src/scripts/ci-changed-scope.test.ts | 40 +++++--- test/scripts/changed-lanes.test.ts | 94 ++++++++++++++----- test/scripts/changed-path-facts.test.ts | 7 +- .../scripts/ci-changed-node-test-plan.test.ts | 9 ++ test/scripts/ci-node-test-plan.test.ts | 15 ++- .../scripts/generate-npm-package-lock.test.ts | 10 ++ test/scripts/test-extension.test.ts | 31 +++++- test/scripts/test-projects.test.ts | 17 ++++ 17 files changed, 273 insertions(+), 174 deletions(-) diff --git a/scripts/changed-lanes.mts b/scripts/changed-lanes.mts index 05ba5901bb14..ccbf9252c606 100644 --- a/scripts/changed-lanes.mts +++ b/scripts/changed-lanes.mts @@ -18,7 +18,7 @@ const DEADCODE_SOURCE_PATH_RE = /^(?:src|extensions|ui|packages)\/.+\.[cm]?[jt]s /** Returns whether any changed path is production source knip scans. */ export function hasDeadcodeScannedSource(changedPaths: string[]): boolean { - return changedPaths.map(normalizeChangedPath).some((p) => DEADCODE_SOURCE_PATH_RE.test(p)); + return changedPaths.some((path) => DEADCODE_SOURCE_PATH_RE.test(path)); } const PROTOCOL_EVENT_COVERAGE_INPUT_RE = @@ -26,11 +26,10 @@ const PROTOCOL_EVENT_COVERAGE_INPUT_RE = export function hasProtocolEventCoverageInput(changedPaths: string[]): boolean { // Match the guard's scan roots and excluded directories, including deleted inputs. - return changedPaths - .map(normalizeChangedPath) - .some( - (p) => PROTOCOL_EVENT_COVERAGE_INPUT_RE.test(p) && !/\/(?:Tests|\.build|build)\//u.test(p), - ); + return changedPaths.some( + (path) => + PROTOCOL_EVENT_COVERAGE_INPUT_RE.test(path) && !/\/(?:Tests|\.build|build)\//u.test(path), + ); } const SCRIPTS_TYPECHECK_PATH_RE = @@ -85,16 +84,14 @@ export function isConfigDocSchemaSourcePath(file: string): boolean { /** Config docs consume core schema/help plus the bundled plugin metadata pipeline. */ export function hasConfigDocInput(changedPaths: string[]): boolean { - return changedPaths - .map(normalizeChangedPath) - .some( - (changedPath) => - !getChangedPathFacts(changedPath).isChangedLaneTest && - (CONFIG_DOC_BASELINE_PATHS.has(changedPath) || - isConfigDocSchemaSourcePath(changedPath) || - CONFIG_DOC_INPUT_PATH_RE.test(changedPath) || - BUNDLED_CHANNEL_CONFIG_METADATA_PATH_RE.test(changedPath)), - ); + return changedPaths.some( + (changedPath) => + !getChangedPathFacts(changedPath).isChangedLaneTest && + (CONFIG_DOC_BASELINE_PATHS.has(changedPath) || + isConfigDocSchemaSourcePath(changedPath) || + CONFIG_DOC_INPUT_PATH_RE.test(changedPath) || + BUNDLED_CHANNEL_CONFIG_METADATA_PATH_RE.test(changedPath)), + ); } /** @@ -173,7 +170,7 @@ export function createEmptyChangedLanes() { } export function isChangedLaneTestPath(changedPath: string) { - return getChangedPathFacts(normalizeChangedPath(changedPath)).isChangedLaneTest; + return getChangedPathFacts(changedPath).isChangedLaneTest; } /** @@ -184,9 +181,9 @@ export function detectChangedLanes( changedPaths: string[], options: DetectChangedLanesOptions = {}, ): ChangedLaneResult { - const paths = [...new Set(changedPaths.map(normalizeChangedPath).filter(Boolean))] - .toSorted((left, right) => left.localeCompare(right)) - .filter((changedPath) => changedPath !== "--"); + const paths = [...new Set(changedPaths.filter(Boolean))].toSorted((left, right) => + left.localeCompare(right), + ); const lanes = createEmptyChangedLanes(); const reasons = []; let extensionImpactFromCore = false; @@ -457,13 +454,13 @@ export function listChangedPathsFromGit(params: { } function runGitNameOnlyDiff(extraArgs: string[], cwd = process.cwd()): string[] { - const output = execFileSync("git", ["diff", "--name-only", ...extraArgs], { + const output = execFileSync("git", ["diff", "--name-only", "-z", ...extraArgs], { cwd, stdio: ["ignore", "pipe", "pipe"], encoding: "utf8", maxBuffer: GIT_OUTPUT_MAX_BUFFER, }); - return output.split("\n").map(normalizeChangedPath).filter(Boolean); + return output.split("\0").filter(Boolean); } function gitOutputText(value: unknown) { @@ -484,26 +481,20 @@ function isGitNoMergeBaseError(error: unknown) { } function runGitLsFiles(extraArgs: string[], cwd = process.cwd()): string[] { - const output = execFileSync("git", ["ls-files", ...extraArgs], { + const output = execFileSync("git", ["ls-files", "-z", ...extraArgs], { cwd, stdio: ["ignore", "pipe", "pipe"], encoding: "utf8", maxBuffer: GIT_OUTPUT_MAX_BUFFER, }); - return output.split("\n").map(normalizeChangedPath).filter(Boolean); + return output.split("\0").filter(Boolean); } /** * Lists staged changed paths for pre-commit checks. */ export function listStagedChangedPaths(cwd = process.cwd()) { - const output = execFileSync("git", ["diff", "--cached", "--name-only", "--diff-filter=ACMRD"], { - cwd, - stdio: ["ignore", "pipe", "pipe"], - encoding: "utf8", - maxBuffer: GIT_OUTPUT_MAX_BUFFER, - }); - return output.split("\n").map(normalizeChangedPath).filter(Boolean); + return runGitNameOnlyDiff(["--cached", "--diff-filter=ACMRD"], cwd); } /** @@ -647,6 +638,7 @@ function parseArgs(argv: string[]) { }, ); parsed.paths.push(...explicitPaths); + parsed.paths = parsed.paths.map((changedPath) => normalizeChangedPath(changedPath)); return parsed; } diff --git a/scripts/check-changed.mts b/scripts/check-changed.mts index 88022572f0f8..7d1e6a048f4a 100644 --- a/scripts/check-changed.mts +++ b/scripts/check-changed.mts @@ -164,19 +164,16 @@ function createChangedCheckChildEnv(baseEnv: NodeJS.ProcessEnv = process.env) { } function hasAndroidVersionSyncPath(paths: string[]) { - return paths.some((changedPath) => - ANDROID_VERSION_SYNC_PATHS.has(normalizeChangedPath(changedPath)), - ); + return paths.some((changedPath) => ANDROID_VERSION_SYNC_PATHS.has(changedPath)); } function hasMacosAppCiPath(paths: string[]) { // The metadata test has its own command; production edits still need native app proof. // Swift test-target sources do not feed the packaged app; native CI still covers them. return paths.some((changedPath) => { - const normalized = normalizeChangedPath(changedPath); return ( - normalized !== SWIFT_BUILD_CACHE_METADATA_TEST_PATH && - (MACOS_APP_CI_PATH_RE.test(normalized) || isMacosToolingPath(normalized)) + changedPath !== SWIFT_BUILD_CACHE_METADATA_TEST_PATH && + (MACOS_APP_CI_PATH_RE.test(changedPath) || isMacosToolingPath(changedPath)) ); }); } @@ -299,7 +296,16 @@ function buildDelegatedChangedCheckArgv(argv: string[], options: { cwd?: string if (stagedPaths.length === 0) { return [...timedArgs, "--no-changes"]; } - return [...timedArgs, "--base", "HEAD", "--head", "HEAD", "--", ...stagedPaths]; + return [ + ...timedArgs, + "--paths-from-git", + "--base", + "HEAD", + "--head", + "HEAD", + "--", + ...stagedPaths, + ]; } export function shouldRunNpmLockGuard(paths: string[]) { @@ -315,9 +321,7 @@ export function shouldRunPromptSnapshotOwnerTest(paths: string[]) { } export function shouldRunControlUiI18nVerify(paths: string[]) { - return paths.some((changedPath) => - CONTROL_UI_I18N_VERIFY_PATH_RE.test(normalizeChangedPath(changedPath)), - ); + return paths.some((changedPath) => CONTROL_UI_I18N_VERIFY_PATH_RE.test(changedPath)); } export function shouldRunRuntimeSidecarBaselineCheck(paths: string[]) { @@ -326,47 +330,35 @@ export function shouldRunRuntimeSidecarBaselineCheck(paths: string[]) { /** Returns whether changed files can drift bundled doctor-contract declarations or closures. */ export function shouldRunDoctorContractOwnerTests(paths: string[]) { - return paths.some((changedPath) => - DOCTOR_CONTRACT_OWNER_TEST_PATH_RE.test(normalizeChangedPath(changedPath)), - ); + return paths.some((changedPath) => DOCTOR_CONTRACT_OWNER_TEST_PATH_RE.test(changedPath)); } /** Returns whether changed files can affect the sessions/transcripts SQLite schema baseline. */ export function shouldRunSqliteSessionSchemaBaselineCheck(paths: string[]) { - return paths.some((changedPath) => - SQLITE_SESSION_SCHEMA_BASELINE_PATH_RE.test(normalizeChangedPath(changedPath)), - ); + return paths.some((changedPath) => SQLITE_SESSION_SCHEMA_BASELINE_PATH_RE.test(changedPath)); } /** Returns whether changed files can alter Plugin SDK exports or surface budgets. */ export function shouldRunPluginSdkSurfaceChecks(paths: string[]) { - return paths.some((changedPath) => - PLUGIN_SDK_SURFACE_PATH_RE.test(normalizeChangedPath(changedPath)), - ); + return paths.some((changedPath) => PLUGIN_SDK_SURFACE_PATH_RE.test(changedPath)); } /** Returns whether changed files can alter deprecated API or plugin-boundary results. */ export function shouldRunDeprecationHygieneChecks(paths: string[]) { - return paths.some((changedPath) => - DEPRECATION_HYGIENE_PATH_RE.test(normalizeChangedPath(changedPath)), - ); + return paths.some((changedPath) => DEPRECATION_HYGIENE_PATH_RE.test(changedPath)); } /** Returns whether changed files can alter wrapper-shadowing results. */ export function shouldRunWrapperShadowingCheck(paths: string[]) { - return paths.some((changedPath) => - WRAPPER_SHADOWING_PATH_RE.test(normalizeChangedPath(changedPath)), - ); + return paths.some((changedPath) => WRAPPER_SHADOWING_PATH_RE.test(changedPath)); } export function shouldRunAppcastOwnerTest(paths: string[]) { - return paths.some((changedPath) => normalizeChangedPath(changedPath) === "appcast.xml"); + return paths.includes("appcast.xml"); } export function shouldRunTestTempCreationReport(paths: string[]) { - return paths.some( - (changedPath) => getChangedPathFacts(normalizeChangedPath(changedPath)).isChangedLaneTest, - ); + return paths.some((changedPath) => getChangedPathFacts(changedPath).isChangedLaneTest); } export function createNpmLockGuardCommand(paths: string[]) { @@ -1276,8 +1268,8 @@ function printSummary(timings: ChangedCheckTiming[], options: ChangedCheckRunOpt function parseArgs(argv: string[]) { const separatorIndex = argv.indexOf("--"); const flagArgv = separatorIndex === -1 ? argv : argv.slice(0, separatorIndex); - const explicitPaths = - separatorIndex === -1 ? [] : argv.slice(separatorIndex + 1).map(normalizeChangedPath); + const explicitPaths = separatorIndex === -1 ? [] : argv.slice(separatorIndex + 1); + const preservePathTokens = flagArgv.includes("--paths-from-git"); const args = { base: "origin/main", head: "HEAD", @@ -1289,7 +1281,7 @@ function parseArgs(argv: string[]) { paths: new Array(), }; const parsed = parseFlagArgs( - flagArgv, + flagArgv.filter((arg) => arg !== "--paths-from-git"), args, [ stringFlag("--base", "base"), @@ -1306,12 +1298,15 @@ function parseArgs(argv: string[]) { if (arg.startsWith("-")) { throw new Error(`Unknown option: ${arg}`); } - target.paths.push(normalizeChangedPath(arg)); + target.paths.push(arg); return "handled"; }, }, ); parsed.paths.push(...explicitPaths); + if (!preservePathTokens) { + parsed.paths = parsed.paths.map((changedPath) => normalizeChangedPath(changedPath)); + } return parsed; } diff --git a/scripts/ci-changed-scope.mjs b/scripts/ci-changed-scope.mjs index 478b75f754ba..b9781f8be270 100644 --- a/scripts/ci-changed-scope.mjs +++ b/scripts/ci-changed-scope.mjs @@ -566,7 +566,7 @@ export function shouldRunNativeI18n(changedPaths) { return ( !Array.isArray(changedPaths) || changedPaths.length === 0 || - changedPaths.some((path) => NATIVE_I18N_SCOPE_RE.test(path.trim())) + changedPaths.some((path) => NATIVE_I18N_SCOPE_RE.test(path)) ); } @@ -670,15 +670,16 @@ export function listChangedPaths( cwd, preferFirstParent: preferMergeHeadFirstParent, }); - const output = execFileSync("git", ["diff", "--no-renames", "--name-only", diffBase, head], { - cwd, - stdio: ["ignore", "pipe", "pipe"], - encoding: "utf8", - }); - return output - .split("\n") - .map((line) => line.trim()) - .filter((line) => line.length > 0); + const output = execFileSync( + "git", + ["diff", "--no-renames", "--name-only", "-z", diffBase, head], + { + cwd, + stdio: ["ignore", "pipe", "pipe"], + encoding: "utf8", + }, + ); + return output.split("\0").filter(Boolean); } /** diff --git a/scripts/generate-npm-package-lock.mts b/scripts/generate-npm-package-lock.mts index e8efb6039aed..5581dcf1dfa0 100644 --- a/scripts/generate-npm-package-lock.mts +++ b/scripts/generate-npm-package-lock.mts @@ -1228,11 +1228,7 @@ function npmLockPackageDirsForChangedPaths(changedPaths: string[]) { let hasAmbiguousDependencyPolicyChange = false; let hasLockfileChange = false; - for (const rawPath of changedPaths) { - const changedPath = rawPath - .trim() - .replaceAll("\\", "/") - .replace(/^\.\/+/u, ""); + for (const changedPath of changedPaths) { if (!changedPath) { continue; } diff --git a/scripts/lib/changed-extensions.mts b/scripts/lib/changed-extensions.mts index 2e59e5c0833c..ac2665ca4085 100644 --- a/scripts/lib/changed-extensions.mts +++ b/scripts/lib/changed-extensions.mts @@ -19,10 +19,6 @@ function runGit(args: string[]) { }); } -function normalizeRelative(inputPath: string) { - return inputPath.split(path.sep).join("/"); -} - function hasGitCommit(ref: string | undefined) { if (!ref || /^0+$/.test(ref)) { return false; @@ -69,21 +65,18 @@ function listChangedPaths(base: string, head = "HEAD") { throw new Error("A git base revision is required to list changed extensions."); } - return runGit(["diff", "--name-only", base, head]) - .split("\n") - .map((line) => line.trim()) - .filter((line) => line.length > 0); + return runGit(["diff", "--name-only", "-z", base, head]).split("\0").filter(Boolean); } function listAvailableExtensionIdsFromGit() { const packageFiles = runGit([ "ls-files", + "-z", "--", `:(glob)${BUNDLED_PLUGIN_PATH_PREFIX}*/package.json`, ]) - .split("\n") - .map((line) => normalizeRelative(line.trim())) - .filter((line) => line.length > 0); + .split("\0") + .filter(Boolean); return packageFiles .flatMap((file) => { const match = file.match(new RegExp(`^${BUNDLED_PLUGIN_PATH_PREFIX}([^/]+)/package\\.json$`)); @@ -122,8 +115,7 @@ export function detectChangedExtensionIds(changedPaths: string[]) { const availableExtensionIds = new Set(listAvailableExtensionIds()); const extensionIds = new Set(); - for (const rawPath of changedPaths) { - const relativePath = normalizeRelative(rawPath.trim()); + for (const relativePath of changedPaths) { if (!relativePath) { continue; } diff --git a/scripts/lib/changed-path-facts.mjs b/scripts/lib/changed-path-facts.mjs index e03d0583485a..ff11340c5d2d 100644 --- a/scripts/lib/changed-path-facts.mjs +++ b/scripts/lib/changed-path-facts.mjs @@ -36,13 +36,12 @@ const ROOT_TEST_SOURCE_PATH_RE = /^test\/(?!fixtures\/).*\.[cm]?tsx?$/u; /** * Normalizes a changed file path into repo-relative POSIX form. * @param {unknown} inputPath + * @param {NodeJS.Platform} [platform] * @returns {string} */ -export function normalizeChangedPath(inputPath) { - return (typeof inputPath === "string" ? inputPath : "") - .trim() - .replaceAll("\\", "/") - .replace(/^\.\/+/u, ""); +export function normalizeChangedPath(inputPath, platform = process.platform) { + const path = (typeof inputPath === "string" ? inputPath : "").trim(); + return (platform === "win32" ? path.replaceAll("\\", "/") : path).replace(/^\.\/+/u, ""); } /** @@ -51,7 +50,7 @@ export function normalizeChangedPath(inputPath) { * @returns {{ path: string; surface: ChangedPathSurface; isChangedLaneTest: boolean; isRootTestSource: boolean; isTestOnly: boolean; isNativeOnly: boolean }} */ export function getChangedPathFacts(inputPath) { - const path = typeof inputPath === "string" ? inputPath.trim() : ""; + const path = typeof inputPath === "string" ? inputPath : ""; const surface = SURFACE_PATTERNS.find(([, pattern]) => pattern.test(path))?.[0] ?? "unknown"; return { diff --git a/scripts/lib/ci-node-test-plan.mts b/scripts/lib/ci-node-test-plan.mts index c90467f94a28..1e83ccca85aa 100644 --- a/scripts/lib/ci-node-test-plan.mts +++ b/scripts/lib/ci-node-test-plan.mts @@ -124,16 +124,11 @@ const policyTestWatches = [ })), ] satisfies readonly PolicyTestWatch[]; -function normalizeChangedPath(changedPath: string): string { - return changedPath.replaceAll("\\", "/").replace(/^\.\//u, ""); -} - /** Resolve policy tests whose scanned source surface intersects this diff. */ export function resolvePolicyTestTargets(changedPaths: readonly string[]): string[] { - const normalizedPaths = changedPaths.map(normalizeChangedPath); return policyTestWatches .filter(({ watchGlobs }) => - normalizedPaths.some((changedPath) => + changedPaths.some((changedPath) => watchGlobs.some((watchGlob) => matchesGlob(changedPath, watchGlob)), ), ) @@ -142,9 +137,8 @@ export function resolvePolicyTestTargets(changedPaths: readonly string[]): strin /** True when the policy tests are the complete bounded owner for this path. */ export function isPolicyTestOwnedPath(changedPath: string): boolean { - const normalizedPath = normalizeChangedPath(changedPath); return policyTestWatches.some(({ ownerGlobs }) => - ownerGlobs?.some((ownerGlob) => matchesGlob(normalizedPath, ownerGlob)), + ownerGlobs?.some((ownerGlob) => matchesGlob(changedPath, ownerGlob)), ); } @@ -2005,7 +1999,6 @@ export function createNodeTestShards(options: NodeTestPlanOptions = {}): NodeTes const changedTestPlans = includeReleaseOnlyPluginShards ? [] : (options.changedPaths ?? []) - .map(normalizeChangedPath) .filter( (file) => isTestFileTarget(file) && diff --git a/scripts/lib/extension-test-plan.mts b/scripts/lib/extension-test-plan.mts index de5dd1092a91..a5b86aa1b132 100644 --- a/scripts/lib/extension-test-plan.mts +++ b/scripts/lib/extension-test-plan.mts @@ -176,7 +176,7 @@ export const GIT_LS_FILES_MAX_BUFFER_BYTES = 16 * 1024 * 1024; export function listTrackedTestPlanFiles(cwd: string, pathspecs: readonly string[]) { // Query only the planner-owned tree: a full-repo inventory can overflow // spawnSync's buffer and either truncate the plan or force directory walks. - const result = spawnSync("git", ["ls-files", "--", ...pathspecs], { + const result = spawnSync("git", ["ls-files", "-z", "--", ...pathspecs], { cwd, encoding: "utf8", maxBuffer: GIT_LS_FILES_MAX_BUFFER_BYTES, @@ -185,10 +185,7 @@ export function listTrackedTestPlanFiles(cwd: string, pathspecs: readonly string if (result.status !== 0 || result.error) { return null; } - return result.stdout - .split("\n") - .map((line) => line.trim().replaceAll("\\", "/")) - .filter(Boolean); + return result.stdout.split("\0").filter(Boolean); } function loadTrackedRepoTestFiles() { diff --git a/scripts/test-projects.test-support.mts b/scripts/test-projects.test-support.mts index fe3f45557d1e..0d4ec75b79f2 100644 --- a/scripts/test-projects.test-support.mts +++ b/scripts/test-projects.test-support.mts @@ -1225,10 +1225,7 @@ function listExplicitTestTargetFilesFromGit(cwd: string) { if (result.status !== 0) { return null; } - return result.stdout - .split("\0") - .map((line) => normalizePathPattern(line.trim())) - .filter((line) => line.length > 0 && isImportableGraphFile(line)); + return result.stdout.split("\0").filter((line) => line.length > 0 && isImportableGraphFile(line)); } function listExplicitTestTargetFilesForCwd(cwd: string) { @@ -1694,7 +1691,7 @@ function listImportGraphGrepMatches( }; const result = spawnSync( "git", - ["grep", "-l", "--fixed-strings", "-f", "-", "--", ...grepPaths], + ["grep", "-l", "-z", "--fixed-strings", "-f", "-", "--", ...grepPaths], spawnOptions, ); for (const term of missing) { @@ -1705,10 +1702,7 @@ function listImportGraphGrepMatches( // Source archives use the same filesystem inventory and native reader as the full graph. const candidates = ( result.status === 0 - ? result.stdout - .split("\n") - .map((line) => normalizePathPattern(line.trim())) - .filter((file) => trackedFiles.has(file)) + ? result.stdout.split("\0").filter((file) => trackedFiles.has(file)) : [...trackedFiles].filter((file) => !testFilesOnly || isTestFileTarget(file)) ).toSorted((left, right) => left.localeCompare(right)); // Per-term membership protects the broad cap and helper first-success rule. @@ -1771,18 +1765,17 @@ function resolveAffectedTestsFromTargetedImportScan( cwd: string, options: ImportGraphOptions & { direct?: boolean } = {}, ) { - const normalized = normalizePathPattern(changedPath); const tooling = options.tooling === true; const files = listImportGraphFilesForCwd(cwd, { tooling }); const fileSet = new Set(files); - if (!fileSet.has(normalized)) { + if (!fileSet.has(changedPath)) { return []; } const testFiles = new Set( files.filter((file) => isTestFileTarget(file) && !file.endsWith(".live.test.ts")), ); - let frontier = [normalized]; + let frontier = [changedPath]; const seen = new Set(frontier); const targets = []; const extensions = tooling ? TOOLING_IMPORTABLE_FILE_EXTENSIONS : IMPORTABLE_FILE_EXTENSIONS; @@ -1850,14 +1843,12 @@ export function hasImportGraphImpactOnTargets( cwd = process.cwd(), options: ImportGraphOptions = {}, ) { - const changed = new Set(changedPaths.map(normalizePathPattern)); + const changed = new Set(changedPaths); if (changed.size === 0) { return false; } const files = listImportGraphFilesForCwd(cwd, options); - const targets = Array.isArray(targetPaths) - ? targetPaths.map(normalizePathPattern) - : files.filter(targetPaths); + const targets = Array.isArray(targetPaths) ? targetPaths : files.filter(targetPaths); if (targets.some((file) => changed.has(file))) { return true; } @@ -1898,16 +1889,15 @@ function resolveAffectedTestsFromImportGraph( cwd: string, options: { forceFull?: boolean } = {}, ) { - const normalized = normalizePathPattern(changedPath); if (options.forceFull !== true) { - const targetedTargets = resolveAffectedTestsFromTargetedImportScan(normalized, cwd); + const targetedTargets = resolveAffectedTestsFromTargetedImportScan(changedPath, cwd); if (targetedTargets !== null) { return targetedTargets; } } const { reverseImports, testFiles } = getImportGraph(cwd); - const queue = [normalized]; + const queue = [changedPath]; const seen = new Set(queue); const targets = []; @@ -3118,10 +3108,9 @@ function resolveGithubYamlGuardTargets(changedPath: string) { } function resolveDirectToolingReferenceTests(changedPath: string, cwd: string) { - const normalized = normalizePathPattern(changedPath); return ( - listImportGraphGrepMatches(cwd, [normalized], { tooling: true, testFilesOnly: true }).get( - normalized, + listImportGraphGrepMatches(cwd, [changedPath], { tooling: true, testFilesOnly: true }).get( + changedPath, ) ?? [] ) .filter( @@ -3129,7 +3118,7 @@ function resolveDirectToolingReferenceTests(changedPath: string, cwd: string) { file !== "test/scripts/test-projects.test.ts" && !file.endsWith(".live.test.ts") && isTestFileTarget(file) && - references.has(normalized), + references.has(changedPath), ) .map(({ file }) => file); } diff --git a/src/scripts/ci-changed-scope.test.ts b/src/scripts/ci-changed-scope.test.ts index 5c29eea3c689..04634bf53fc2 100644 --- a/src/scripts/ci-changed-scope.test.ts +++ b/src/scripts/ci-changed-scope.test.ts @@ -5,6 +5,7 @@ import os from "node:os"; import path from "node:path"; import { bundledPluginFile } from "openclaw/plugin-sdk/test-fixtures"; import { afterEach, describe, expect, it } from "vitest"; +import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js"; const { detectChangedScope, @@ -18,7 +19,7 @@ const { } = await import("../../scripts/ci-changed-scope.mjs"); const markerPaths: string[] = []; -const tempDirs: string[] = []; +const tempDirs = useAutoCleanupTempDirTracker(afterEach); afterEach(() => { for (const markerPath of markerPaths) { @@ -27,10 +28,6 @@ afterEach(() => { } catch {} } markerPaths.length = 0; - for (const tempDir of tempDirs) { - fs.rmSync(tempDir, { force: true, recursive: true }); - } - tempDirs.length = 0; }); function parseGitHubOutput(output: string): Record { @@ -56,8 +53,7 @@ function writeRepoFile(repoDir: string, filePath: string, contents: string): voi } function createSyntheticMergeRepo(prefix: string): { repoDir: string; staleBase: string } { - const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); - tempDirs.push(repoDir); + const repoDir = tempDirs.make(prefix); git(repoDir, ["init", "-b", "main"]); git(repoDir, ["config", "user.email", "ci@example.invalid"]); @@ -914,8 +910,7 @@ describe("detectChangedScope", () => { }); it("reports both sides of a rename so deleted paths force safe planning", () => { - const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-ci-scope-rename-")); - tempDirs.push(repoDir); + const repoDir = tempDirs.make("openclaw-ci-scope-rename-"); git(repoDir, ["init", "-b", "main"]); git(repoDir, ["config", "user.email", "ci@example.invalid"]); git(repoDir, ["config", "user.name", "CI"]); @@ -930,6 +925,28 @@ describe("detectChangedScope", () => { expect(listChangedPaths(base, "HEAD", repoDir)).toEqual(["src/new.ts", "src/old.ts"]); }); + it("preserves leading spaces and newlines in Git filename tokens", () => { + if (process.platform === "win32") { + return; + } + const repoDir = tempDirs.make("openclaw-ci-scope-raw-paths-"); + git(repoDir, ["init", "-b", "main"]); + git(repoDir, ["config", "user.email", "ci@example.invalid"]); + git(repoDir, ["config", "user.name", "CI"]); + writeRepoFile(repoDir, "README.md", "base\n"); + git(repoDir, ["add", "."]); + git(repoDir, ["commit", "-m", "base"]); + const base = git(repoDir, ["rev-parse", "HEAD"]); + const changedPaths = [" scripts/changed-lanes.mts", "scripts/changed\nlanes.mts"]; + for (const changedPath of changedPaths) { + writeRepoFile(repoDir, changedPath, "export {};\n"); + } + git(repoDir, ["add", "--", ...changedPaths]); + git(repoDir, ["commit", "-m", "raw paths"]); + + expect(listChangedPaths(base, "HEAD", repoDir).toSorted()).toEqual(changedPaths.toSorted()); + }); + it("drops oversized changed-path payloads before workflow environment interpolation", () => { const outputPath = path.join(os.tmpdir(), `openclaw-ci-scope-output-${Date.now()}.txt`); markerPaths.push(outputPath); @@ -962,10 +979,7 @@ describe("detectChangedScope", () => { ])( "runs zero-install scope detection for %s", (_label, changedPath, manifest, failSafe, cliArgs) => { - const repoDir = fs.realpathSync( - fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-ci-scope-empty-")), - ); - tempDirs.push(repoDir); + const repoDir = fs.realpathSync(tempDirs.make("openclaw-ci-scope-empty-")); const outputPath = path.join(repoDir, "github-output.txt"); const scriptPath = path.join(repoDir, "scripts/ci-changed-scope.mjs"); diff --git a/test/scripts/changed-lanes.test.ts b/test/scripts/changed-lanes.test.ts index c028a4991713..9f816e199d5d 100644 --- a/test/scripts/changed-lanes.test.ts +++ b/test/scripts/changed-lanes.test.ts @@ -709,6 +709,37 @@ describe("scripts/changed-lanes", () => { expectLanes(result.lanes, { tooling: true }); }); + it("keeps raw Git lookalikes broad without retargeting owner checks", () => { + const paths = [" scripts/changed-lanes.mts", "scripts/changed\nlanes.mts"]; + const result = detectChangedLanes(paths); + const plan = createChangedCheckPlan(result); + + expectLanes(result.lanes, { all: true, tooling: true }); + expect(plan.commands.find((command) => command.name === "format changed files")).toEqual({ + name: "format changed files", + args: ["format:check", "--no-error-on-unmatched-pattern", "--", ...paths], + }); + expect(plan.commands.map((command) => command.args[0])).not.toContain( + "plugin-sdk:check-exports", + ); + }); + + it("preserves POSIX backslashes in explicit paths", () => { + if (process.platform === "win32") { + return; + } + const result = runRepoScript("scripts/changed-lanes.mjs", [ + "--json", + "--", + String.raw`scripts\changed-lanes.mts`, + ]); + + expect(result.status).toBe(0); + expect(parseChangedLaneOutput(result.stdout).paths).toEqual([ + String.raw`scripts\changed-lanes.mts`, + ]); + }); + it("falls back to a two-dot diff when a delegated checkout has no merge base", () => { const dir = makeTempRepoRoot(tempDirs, "openclaw-changed-lanes-no-merge-base-"); git(dir, ["init", "-q", "--initial-branch=main"]); @@ -1107,12 +1138,12 @@ describe("scripts/changed-lanes", () => { expect(hasDeadcodeScannedSource(changedPaths)).toBe(expected); }); - it("ignores the explicit path separator", () => { - const result = detectChangedLanes(["--", "scripts/test-live-acp-bind-docker.sh"]); + it("preserves an exact -- filename after the explicit path separator", () => { + const result = runRepoScript("scripts/changed-lanes.mjs", ["--json", "--", "--"]); - expect(result.paths).toEqual(["scripts/test-live-acp-bind-docker.sh"]); - expect(result.lanes.liveDockerTooling).toBe(true); - expect(result.lanes.all).toBe(false); + expect(result.status).toBe(0); + expect(parseChangedLaneOutput(result.stdout).paths).toEqual(["--"]); + expect(parseChangedLaneOutput(result.stdout).lanes.all).toBe(true); }); it("routes a subagent-announce-only Docker diff through the live Docker lane", () => { @@ -1893,19 +1924,20 @@ describe("scripts/changed-lanes", () => { git(dir, ["init", "-q", "--initial-branch=main"]); writeFileSync(path.join(dir, "README.md"), "initial\n", "utf8"); commitAll(dir, "initial"); - mkdirSync(path.join(dir, "src"), { recursive: true }); - writeFileSync(path.join(dir, "src", "staged.ts"), "export const staged = 1;\n", "utf8"); - git(dir, ["add", "src/staged.ts"]); + const stagedPath = process.platform === "win32" ? "src/staged file.ts" : " src/staged\nfile.ts"; + writeRepoFile(dir, stagedPath, "export const staged = 1;\n"); + git(dir, ["add", "--", stagedPath]); const args = buildChangedCheckCrabboxArgs(["--staged", "--timed"], { cwd: dir }); expect(args.slice(args.indexOf("check:changed") + 1)).toEqual([ "--timed", + "--paths-from-git", "--base", "HEAD", "--head", "HEAD", "--", - "src/staged.ts", + stagedPath, ]); }); @@ -2075,20 +2107,23 @@ describe("scripts/changed-lanes", () => { it.each([ ...[ - githubActivityHelper, - `./${githubActivityHelper}`, - githubActivityHelper.replaceAll("/", "\\"), - ].map((helperPath) => ({ + { broad: false, helperPath: githubActivityHelper }, + { broad: true, helperPath: `./${githubActivityHelper}` }, + { broad: true, helperPath: githubActivityHelper.replaceAll("/", "\\") }, + ].map(({ broad, helperPath }) => ({ + broad, name: `routes hidden maintainer helper ${helperPath} to tooling instead of all lanes`, paths: [helperPath], excludesTests: true, })), { + broad: false, name: "routes gitignore changes to tooling instead of all lanes", paths: [".gitignore"], excludesTests: true, }, { + broad: false, name: "routes root hygiene config changes to tooling instead of all lanes", paths: [ ".dockerignore", @@ -2112,11 +2147,13 @@ describe("scripts/changed-lanes", () => { excludesTests: true, }, { + broad: false, name: "routes VS Code workspace settings to tooling instead of all lanes", paths: [".vscode/settings.json", ".vscode/extensions.json"], excludesTests: true, }, { + broad: false, name: "routes legacy root sandbox Dockerfile moves to tooling instead of all lanes", paths: [ "Dockerfile.sandbox", @@ -2129,18 +2166,19 @@ describe("scripts/changed-lanes", () => { excludesTests: true, }, { + broad: false, name: "routes legacy root asset deletions as tooling during root cleanup", paths: ["assets/avatar-placeholder.svg", "assets/chrome-extension/icons/icon128.png"], excludesTests: false, }, - ])("$name", ({ paths, excludesTests }) => { + ])("$name", ({ paths, excludesTests, broad = false }) => { const result = detectChangedLanes(paths); const commands = createChangedCheckPlan(result).commands.map((command) => command.args[0]); - expectLanes(result.lanes, { tooling: true }); - expect(result.extensionImpactFromCore).toBe(false); - expect(commands).toContain("lint:scripts"); - expect(commands).not.toContain("tsgo:all"); + expectLanes(result.lanes, broad ? { all: true } : { tooling: true }); + expect(result.extensionImpactFromCore).toBe(broad); + expect(commands).toContain(broad ? "lint" : "lint:scripts"); + expect(commands.includes("tsgo:all")).toBe(broad); if (excludesTests) { expect(commands).not.toContain("test"); } @@ -2790,11 +2828,8 @@ describe("scripts/changed-lanes", () => { } }); - it.each([ - "apps/macos/Tests/OpenClawIPCTests/MacNodeHostWorkerTests.swift", - "./apps/macos/Tests/OpenClawIPCTests/RemovedTests.swift", - "apps\\macos\\Tests\\OpenClawIPCTests\\Nested\\WorkerTests.swift", - ])("keeps Swift test-only changes out of local packaging tests: %s", (changedPath) => { + it("keeps exact Swift test-only changes out of local packaging tests", () => { + const changedPath = "apps/macos/Tests/OpenClawIPCTests/MacNodeHostWorkerTests.swift"; const plan = createChangedCheckPlan(detectChangedLanes([changedPath]), { env: { PATH: "/usr/bin" }, platform: "darwin", @@ -2808,6 +2843,19 @@ describe("scripts/changed-lanes", () => { expect(plan.commands.map((command) => command.args[0])).not.toContain("test:macos:ci"); }); + it.each([ + "./apps/macos/Tests/OpenClawIPCTests/RemovedTests.swift", + String.raw`apps\macos\Tests\OpenClawIPCTests\Nested\WorkerTests.swift`, + ])("fails safe for noncanonical Swift path %s", (changedPath) => { + const result = detectChangedLanes([changedPath]); + const commands = createChangedCheckPlan(result).commands.map((command) => command.args[0]); + + expectLanes(result.lanes, { all: true }); + expect(result.extensionImpactFromCore).toBe(true); + expect(commands).toContain("lint"); + expect(commands).not.toContain("lint:apps"); + }); + it("preserves the full changed gate alongside a Swift test change", () => { const fullPlan = createChangedCheckPlan(detectChangedLanes(["pnpm-lock.yaml"])); const mixedPlan = createChangedCheckPlan( diff --git a/test/scripts/changed-path-facts.test.ts b/test/scripts/changed-path-facts.test.ts index 95c87473c4be..f660ea99f86a 100644 --- a/test/scripts/changed-path-facts.test.ts +++ b/test/scripts/changed-path-facts.test.ts @@ -65,9 +65,14 @@ describe("changed path facts", () => { }); it("keeps normalization separate from classification", () => { - expect(normalizeChangedPath(" .\\extensions\\slack\\src\\index.test.ts ")).toBe( + expect(normalizeChangedPath(" .\\extensions\\slack\\src\\index.test.ts ", "win32")).toBe( "extensions/slack/src/index.test.ts", ); + expect(normalizeChangedPath(String.raw`.\extensions\slack\src\index.test.ts`, "darwin")).toBe( + String.raw`.\extensions\slack\src\index.test.ts`, + ); expect(getChangedPathFacts("./src/config/defaults.ts").surface).toBe("unknown"); + expect(getChangedPathFacts(" src/config/defaults.ts").surface).toBe("unknown"); + expect(getChangedPathFacts(String.raw`src\config\defaults.ts`).surface).toBe("unknown"); }); }); diff --git a/test/scripts/ci-changed-node-test-plan.test.ts b/test/scripts/ci-changed-node-test-plan.test.ts index e54cd7c821ac..ba0a45805c98 100644 --- a/test/scripts/ci-changed-node-test-plan.test.ts +++ b/test/scripts/ci-changed-node-test-plan.test.ts @@ -502,6 +502,15 @@ describe("CI changed Node test plan", () => { expect(createChangedNodeTestShards(["package.json"])).toBeNull(); }); + it("fails safe for raw Git paths that resemble normalized script paths", () => { + for (const changedPath of [ + " scripts/changed-lanes.mts", + String.raw`scripts\changed-lanes.mts`, + ]) { + expect(createChangedNodeTestShards([changedPath]), changedPath).toBeNull(); + } + }); + it("keeps minimal-gateway boot coverage reachable from gateway startup changes", () => { // A gateway startup stall must fail in the gateway lane; the boot smoke is // selected purely through the import graph, so a rename or an import shape diff --git a/test/scripts/ci-node-test-plan.test.ts b/test/scripts/ci-node-test-plan.test.ts index 3e6c4c6e9a52..faabdf836514 100644 --- a/test/scripts/ci-node-test-plan.test.ts +++ b/test/scripts/ci-node-test-plan.test.ts @@ -13,6 +13,7 @@ import { createNodeTestShards, createVitestCacheWarmGroups, isExclusiveCompactShardName, + isPolicyTestOwnedPath, resolvePolicyTestTargets, } from "../../scripts/lib/ci-node-test-plan.mts"; import * as testTimings from "../../scripts/lib/ci-test-timings.mts"; @@ -414,6 +415,16 @@ describe("scripts/lib/ci-node-test-plan.mts", () => { expect(resolvePolicyTestTargets(["docs/web/control-ui.md"])).toEqual([]); }); + it("matches policy owners only for exact changed paths", () => { + const changedPath = "ui/src/styles/base.css"; + expect(isPolicyTestOwnedPath(changedPath)).toBe(true); + expect(resolvePolicyTestTargets([changedPath])).not.toEqual([]); + for (const lookalike of [` ${changedPath}`, String.raw`ui\src\pages\chat\view.ts`]) { + expect(isPolicyTestOwnedPath(lookalike), lookalike).toBe(false); + expect(resolvePolicyTestTargets([lookalike]), lookalike).toEqual([]); + } + }); + it("projects cache-warm groups from the owned node test plan", () => { const groups = createVitestCacheWarmGroups(); expect(groups).toHaveLength(12); @@ -3013,7 +3024,9 @@ describe("scripts/lib/ci-node-test-plan.mts", () => { includeReleaseOnlyPluginShards: false, changedPaths: [ ...STORE_ALIAS_CHANGED_PATHS.toReversed(), - "./src/plugins/tools.optional.test.ts", + " src/plugins/tools.optional.test.ts", + String.raw`src\plugins\tools.optional.test.ts`, + "src/plugins/tools.optional.test.ts", PLUGIN_PRERELEASE_NPM_SPEC_TEST, "src/plugins/contracts/plugin-sdk-subpaths.test.ts", "src/plugins/loader.test.ts", diff --git a/test/scripts/generate-npm-package-lock.test.ts b/test/scripts/generate-npm-package-lock.test.ts index 03a49ffa8333..97cbc14c8672 100644 --- a/test/scripts/generate-npm-package-lock.test.ts +++ b/test/scripts/generate-npm-package-lock.test.ts @@ -705,6 +705,16 @@ describe("generate-npm-package-lock", () => { ).toEqual(["extensions/acpx"]); }); + it("does not normalize raw Git filename boundaries into package manifests", () => { + expect( + npmLockPackageDirsForChangedPaths([ + " extensions/acpx/package.json", + "extensions/acpx/package.json ", + String.raw`extensions\acpx\package.json`, + ]), + ).toEqual([]); + }); + it("targets the changed publishable gateway protocol manifest", () => { expect( npmLockPackageDirsForChangedPaths(["packages/gateway-protocol/package.json"]).map( diff --git a/test/scripts/test-extension.test.ts b/test/scripts/test-extension.test.ts index 93b0f94abc13..4273e6206232 100644 --- a/test/scripts/test-extension.test.ts +++ b/test/scripts/test-extension.test.ts @@ -14,7 +14,7 @@ import { tmpdir } from "node:os"; import path from "node:path"; import { setTimeout as delay } from "node:timers/promises"; import { bundledPluginFile, bundledPluginRoot } from "openclaw/plugin-sdk/test-fixtures"; -import { beforeAll, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { parseCLI } from "vitest/node"; import { detectChangedExtensionIds, @@ -26,6 +26,7 @@ import { createExtensionTestProcessTargetChunks, createExtensionTestShards, listExtensionTestFilesForRoots, + listTrackedTestPlanFiles, resolveExtensionBatchPlan, resolveExtensionTestConfig, resolveExtensionTestPlan, @@ -40,11 +41,13 @@ import { } from "../../scripts/test-extension-batch.mts"; import { expectNoNodeFsScans } from "../../src/test-utils/fs-scan-assertions.js"; import { waitForPidFile } from "../helpers/process-wait.js"; +import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js"; import { extensionCatchAllExcludedTestRoots } from "../vitest/vitest.extensions.config.ts"; const scriptPath = path.join(process.cwd(), "scripts", "test-extension.mts"); const posixIt = process.platform === "win32" ? it.skip : it; const MATRIX_TEST_PROCESS_FILE_LIMIT = 40; +const tempDirs = useAutoCleanupTempDirTracker(afterEach); type RunGroupParams = VitestBatchRunParams; @@ -229,6 +232,22 @@ describe("scripts/test-extension.mts", () => { } }); + posixIt("preserves newline and leading-space tokens in the tracked Git inventory", () => { + const root = tempDirs.make("openclaw-extension-git-paths-"); + const trackedPaths = [" extensions/example.test.ts", "extensions/example\npath.test.ts"]; + expect(spawnSync("git", ["init", "-q", "--initial-branch=main"], { cwd: root }).status).toBe(0); + for (const trackedPath of trackedPaths) { + const absolutePath = path.join(root, trackedPath); + mkdirSync(path.dirname(absolutePath), { recursive: true }); + writeFileSync(absolutePath, "export {};\n"); + } + expect(spawnSync("git", ["add", "--", ...trackedPaths], { cwd: root }).status).toBe(0); + + expect(listTrackedTestPlanFiles(root, [":(glob)**/*.test.ts"])?.toSorted()).toEqual( + trackedPaths.toSorted(), + ); + }); + it.each([ ["watch", ["--watch"]], ["short watch", ["-w"]], @@ -304,6 +323,16 @@ describe("scripts/test-extension.mts", () => { expect(extensionIds).toEqual(["firecrawl", "line", "slack"]); }); + it("does not normalize extension path lookalikes", () => { + expect( + detectChangedExtensionIds([ + " extensions/slack/src/channel.ts", + String.raw`extensions\slack\src\channel.ts`, + " src/line/message.test.ts", + ]), + ).toEqual([]); + }); + it("lists available extension ids", () => { const extensionIds = listAvailableExtensionIds(); diff --git a/test/scripts/test-projects.test.ts b/test/scripts/test-projects.test.ts index 92f9b647ff1a..3efa7ea19cc7 100644 --- a/test/scripts/test-projects.test.ts +++ b/test/scripts/test-projects.test.ts @@ -3016,6 +3016,23 @@ describe("scripts/test-projects changed-target routing", () => { ).toStrictEqual([]); }); + it("fails safe for raw Git paths that explicit-path normalization would rewrite", () => { + for (const changedPath of [ + " scripts/changed-lanes.mts", + String.raw`scripts\changed-lanes.mts`, + ]) { + expect( + resolveChangedTestTargetPlanForArgs( + ["--changed", "origin/main"], + process.cwd(), + () => [changedPath], + { broad: true }, + ), + changedPath, + ).toEqual({ mode: "broad", targets: [] }); + } + }); + it("keeps unknown root surface skip reasons available to changed-mode callers", () => { expect( resolveChangedTestTargetPlanForArgs(["--changed", "origin/main"], process.cwd(), () => [