diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 38300a0d5cc8..fc50c9c027a6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -653,6 +653,7 @@ jobs: const compactPlan = isCanonicalRepository && (eventName === "pull_request" || eventName === "push"); let changedNodeTestShards = null; + let changedExtensionFallbackShards = []; if ( compactPullRequest && changedPaths && @@ -663,6 +664,18 @@ jobs: } catch (error) { console.warn(`Changed Node test planning failed; using compact full suite: ${error}`); } + if ( + changedNodeTestShards === null && + typeof changedNodeTestPlan.createChangedExtensionFallbackShards === "function" + ) { + try { + changedExtensionFallbackShards = + changedNodeTestPlan.createChangedExtensionFallbackShards(changedPaths); + } catch (error) { + console.warn(`Changed extension fallback planning failed; using compact full suite: ${error}`); + changedExtensionFallbackShards = []; + } + } } // Heavy packaging lanes run only when the diff touches surfaces they // exist to prove: test-only diffs cannot change dist bytes, and QA @@ -706,10 +719,13 @@ jobs: const rawNodeTestShards = runNodeFull ? changedNodeTestShards ? changedNodeTestShards - : createNodeTestPlan({ - includeReleaseOnlyPluginShards: false, - compact: compactPlan, - }) + : [ + ...createNodeTestPlan({ + includeReleaseOnlyPluginShards: false, + compact: compactPlan, + }), + ...changedExtensionFallbackShards, + ] : []; const assignVitestFsCacheWriter = typeof nodeTestPlan.assignVitestFsCacheWriter === "function" diff --git a/scripts/lib/ci-changed-node-test-plan.mts b/scripts/lib/ci-changed-node-test-plan.mts index a40c74b5d6c2..8b21a762731e 100644 --- a/scripts/lib/ci-changed-node-test-plan.mts +++ b/scripts/lib/ci-changed-node-test-plan.mts @@ -14,11 +14,16 @@ import { isPolicyTestOwnedPath, resolvePolicyTestTargets, } from "./ci-node-test-plan.mts"; +import { + createExtensionTestProcessTargetChunks, + resolveExtensionTestConfig, +} from "./extension-test-plan.mts"; import { buildPluginSdkEntrySources, publicPluginSdkEntrypoints } from "./plugin-sdk-entries.mts"; type ChangedNodeTestShard = { checkName: string; configs: string[]; + includePatterns?: string[]; planConcurrency?: number; requiresDist: boolean; runner: string; @@ -212,6 +217,166 @@ function createBoundaryShard() { }; } +function resolvePreciseChangedTargets( + changedPaths: string[], + cwd: string, + additionalTargets: string[] = [], +) { + const resolveTargetPlan = (paths: string[]) => + resolveChangedTestTargetPlan(paths, { + broad: true, + combineSiblingWithImportGraph: true, + cwd, + forceFullImportGraph: true, + includeExtensionImpact: false, + }); + const plan = + changedPaths.length > 0 + ? resolveTargetPlan(changedPaths) + : { mode: "targets" as const, targets: [] }; + // Aggregate resolution must not let one precise path hide another path that + // contributes no tests. Partial plans silently drop coverage. + if ( + changedPaths.some((changedPath) => { + const changedPathPlan = resolveTargetPlan([changedPath]); + return changedPathPlan.mode !== "targets" || changedPathPlan.targets.length === 0; + }) || + plan.mode !== "targets" + ) { + return null; + } + const targets = [...new Set([...plan.targets, ...additionalTargets])]; + if ( + targets.length > MAX_CHANGED_NODE_TEST_TARGETS || + targets.some( + (target) => + /^test\/vitest\/vitest\.full-.*\.config\.ts$/u.test(target) || + splitNodeTestConfigs.has(target), + ) || + targets.some( + (target) => + !isTestFileTarget(target) || findUnmatchedExplicitTestTargets([target], cwd).length > 0, + ) + ) { + return null; + } + + const targetPlans = targets.map((target) => ({ + plans: buildVitestRunPlans([target], cwd), + target, + })); + if ( + targetPlans.some( + ({ plans }) => plans.length === 0 || plans.some((targetPlan) => !targetPlan.includePatterns), + ) + ) { + return null; + } + // Preserve special shard setup (for example Go and TUI PTY coverage) by using + // the compact plan until targeted jobs can carry per-config prerequisites. + if ( + targetPlans.some(({ plans }) => + plans.some(({ config }) => configsRequiringFullSuiteMetadata.has(config)), + ) + ) { + return null; + } + return targetPlans.map(({ target }) => target); +} + +function createChangedTargetShards( + targets: string[], + names: { checkName: string; shardName: string }, +) { + const targetChunks: string[][] = []; + for (let offset = 0; offset < targets.length; offset += CHANGED_NODE_TEST_TARGETS_PER_JOB) { + targetChunks.push(targets.slice(offset, offset + CHANGED_NODE_TEST_TARGETS_PER_JOB)); + } + return targetChunks.map((chunk, index) => { + const suffix = targetChunks.length === 1 ? "" : `-${index + 1}`; + const shard: ChangedNodeTestShard = { + checkName: `${names.checkName}${suffix}`, + configs: [], + requiresDist: false, + runner: DEFAULT_NODE_TEST_RUNNER, + shardName: `${names.shardName}${suffix}`, + targets: chunk, + }; + if (chunk.some((target) => SERIAL_CHANGED_TARGET_RE.test(target))) { + shard.planConcurrency = 1; + } + return shard; + }); +} + +function resolveChangedExtensionRoots(changedPaths: string[]) { + return [ + ...new Set( + changedPaths.flatMap((changedPath) => { + const [, extensionId] = changedPath.split("/"); + return extensionId ? [`extensions/${extensionId}`] : []; + }), + ), + ]; +} + +function createChangedExtensionConfigShards(extensionRoots: string[]) { + const rootsByConfig = new Map(); + for (const root of extensionRoots) { + const config = resolveExtensionTestConfig(root); + rootsByConfig.set(config, [...(rootsByConfig.get(config) ?? []), root]); + } + const plans: Array<{ config: string; includePatterns?: string[]; roots: string[] }> = [ + ...rootsByConfig, + ].flatMap(([config, roots]) => { + const chunks = createExtensionTestProcessTargetChunks(config, roots); + return chunks.length > 1 + ? chunks.map((includePatterns) => ({ config, includePatterns, roots })) + : [{ config, roots }]; + }); + return plans.map(({ config, includePatterns, roots }, index) => { + const suffix = plans.length === 1 ? "" : `-${index + 1}`; + const shard: ChangedNodeTestShard = { + checkName: `checks-node-changed-extensions-config${suffix}`, + configs: [config], + requiresDist: false, + runner: DEFAULT_NODE_TEST_RUNNER, + shardName: `changed-extensions-config${suffix}`, + }; + if (includePatterns) { + shard.includePatterns = includePatterns; + } + if (roots.some((root) => SERIAL_CHANGED_TARGET_RE.test(`${root}/`))) { + shard.planConcurrency = 1; + } + return shard; + }); +} + +/** + * The fail-safe cause leaves the non-extension diff's extension impact unbounded, + * so whole extension configs are required; precise targets would under-cover. + */ +export function createChangedExtensionFallbackShards( + changedPaths: string[], + options: CwdOptions = {}, +): ChangedNodeTestShard[] { + const cwd = options.cwd ?? process.cwd(); + const extensionPaths = changedPaths.filter((changedPath) => + changedPath.startsWith("extensions/"), + ); + if (extensionPaths.length === 0) { + return []; + } + const relevantPaths = extensionPaths.filter( + (changedPath) => existsSync(path.join(cwd, changedPath)) || !isTestFileTarget(changedPath), + ); + if (relevantPaths.length === 0) { + return []; + } + return createChangedExtensionConfigShards(resolveChangedExtensionRoots(relevantPaths)); +} + /** * Builds bounded PR jobs from precise changed-test targets. * Null means the caller must fail safe to the compact full-suite plan. @@ -259,99 +424,21 @@ export function createChangedNodeTestShards( return null; } - const resolveTargetPlan = (paths: string[]) => - resolveChangedTestTargetPlan(paths, { - broad: true, - combineSiblingWithImportGraph: true, - cwd, - forceFullImportGraph: true, - includeExtensionImpact: false, - }); - const plan = - regularLivePaths.length > 0 - ? resolveTargetPlan(regularLivePaths) - : { mode: "targets" as const, targets: [] }; - // Aggregate resolution must not let one precise path hide another path that - // contributes no tests. Partial plans silently drop coverage. - if ( - regularLivePaths.some((changedPath) => { - const changedPathPlan = resolveTargetPlan([changedPath]); - return changedPathPlan.mode !== "targets" || changedPathPlan.targets.length === 0; - }) - ) { - return null; - } - if (plan.mode !== "targets") { - return null; - } - const targets = [...new Set([...plan.targets, ...[...policyTargetsByPath.values()].flat()])]; - if ( - targets.length > MAX_CHANGED_NODE_TEST_TARGETS || - targets.some( - (target) => - /^test\/vitest\/vitest\.full-.*\.config\.ts$/u.test(target) || - splitNodeTestConfigs.has(target), - ) - ) { - return null; - } - - if ( - targets.some( - (target) => - !isTestFileTarget(target) || findUnmatchedExplicitTestTargets([target], cwd).length > 0, - ) - ) { - return null; - } - - const targetPlans = targets.map((target) => ({ - plans: buildVitestRunPlans([target], cwd), - target, - })); - if ( - targetPlans.some( - ({ plans }) => plans.length === 0 || plans.some((targetPlan) => !targetPlan.includePatterns), - ) - ) { - return null; - } - // Preserve special shard setup (for example Go and TUI PTY coverage) by using - // the compact plan until targeted jobs can carry per-config prerequisites. - if ( - targetPlans.some(({ plans }) => - plans.some(({ config }) => configsRequiringFullSuiteMetadata.has(config)), - ) - ) { + const targets = resolvePreciseChangedTargets( + regularLivePaths, + cwd, + [...policyTargetsByPath.values()].flat(), + ); + if (targets === null) { return null; } // Boundary-config targets run as regular nondist targets: the boundary // suite scans the checked-out tree and never consumes the built dist. - const orderedTargets = targetPlans.map(({ target }) => target); - const targetChunks: string[][] = []; - for ( - let offset = 0; - offset < orderedTargets.length; - offset += CHANGED_NODE_TEST_TARGETS_PER_JOB - ) { - targetChunks.push(orderedTargets.slice(offset, offset + CHANGED_NODE_TEST_TARGETS_PER_JOB)); - } const shards = [ - ...targetChunks.map((chunk, index) => { - const suffix = targetChunks.length === 1 ? "" : `-${index + 1}`; - const shard: ChangedNodeTestShard = { - checkName: `checks-node-changed${suffix}`, - configs: [], - requiresDist: false, - runner: DEFAULT_NODE_TEST_RUNNER, - shardName: `changed${suffix}`, - targets: chunk, - }; - if (chunk.some((target) => SERIAL_CHANGED_TARGET_RE.test(target))) { - shard.planConcurrency = 1; - } - return shard; + ...createChangedTargetShards(targets, { + checkName: "checks-node-changed", + shardName: "changed", }), ...(hasBuildArtifactAffectingChange(changedPaths) ? [] : [createBoundaryShard()]), ]; diff --git a/test/scripts/ci-changed-node-test-plan.test.ts b/test/scripts/ci-changed-node-test-plan.test.ts index 4d58571576bd..5256ea3e8b48 100644 --- a/test/scripts/ci-changed-node-test-plan.test.ts +++ b/test/scripts/ci-changed-node-test-plan.test.ts @@ -3,6 +3,7 @@ import { tmpdir } from "node:os"; import path from "node:path"; import { describe, expect, it } from "vitest"; import { + createChangedExtensionFallbackShards, createChangedNodeTestShards, hasBuildArtifactAffectingChange, hasPromptSnapshotAffectingChange, @@ -264,6 +265,95 @@ describe("CI changed Node test plan", () => { ).toBeNull(); }); + it("supplements mixed package diffs with the affected extension config", () => { + const changedPaths = [ + "packages/gateway-protocol/src/frame-guards.ts", + "extensions/codex/src/session-upstream-marker.ts", + ]; + + expect(createChangedNodeTestShards(changedPaths)).toBeNull(); + expect(createChangedExtensionFallbackShards(changedPaths)).toEqual([ + { + checkName: "checks-node-changed-extensions-config", + configs: ["test/vitest/vitest.extension-codex.config.ts"], + requiresDist: false, + runner: "blacksmith-8vcpu-ubuntu-2404", + shardName: "changed-extensions-config", + }, + ]); + }); + + it("preserves Matrix process bounds in mixed package fallbacks", () => { + const shards = createChangedExtensionFallbackShards([ + "packages/gateway-protocol/src/frame-guards.ts", + "extensions/matrix/src/channel.ts", + ]); + const targets = shards.flatMap((shard) => shard.includePatterns ?? []); + + expect(shards.length).toBeGreaterThan(1); + expect( + shards.every( + (shard) => + shard.configs[0] === "test/vitest/vitest.extension-matrix.config.ts" && + (shard.includePatterns?.length ?? 0) > 0 && + (shard.includePatterns?.length ?? 0) <= 40, + ), + ).toBe(true); + expect(targets.length).toBeGreaterThan(40); + expect(new Set(targets).size).toBe(targets.length); + }); + + it("skips extension fallback when no extension paths changed", () => { + expect( + createChangedExtensionFallbackShards([ + "packages/gateway-protocol/src/frame-guards.ts", + "src/agents/live-model-filter.ts", + ]), + ).toEqual([]); + }); + + it("falls back to the affected extension config for deleted sources", () => { + const cwd = mkdtempSync(path.join(tmpdir(), "openclaw-ci-extension-fallback-")); + try { + expect( + createChangedExtensionFallbackShards(["extensions/codex/src/deleted-session-runtime.ts"], { + cwd, + }), + ).toEqual([ + { + checkName: "checks-node-changed-extensions-config", + configs: ["test/vitest/vitest.extension-codex.config.ts"], + requiresDist: false, + runner: "blacksmith-8vcpu-ubuntu-2404", + shardName: "changed-extensions-config", + }, + ]); + expect( + createChangedExtensionFallbackShards( + ["extensions/codex/src/deleted-session-runtime.test.ts"], + { cwd }, + ), + ).toEqual([]); + } finally { + rmSync(cwd, { force: true, recursive: true }); + } + }); + + it("serializes the Memory Core extension fallback config", () => { + expect( + createChangedExtensionFallbackShards(["extensions/memory-core/src/memory/mmr.ts"]), + ).toEqual([ + { + checkName: "checks-node-changed-extensions-config", + configs: ["test/vitest/vitest.extension-memory.config.ts"], + planConcurrency: 1, + requiresDist: false, + runner: "blacksmith-8vcpu-ubuntu-2404", + shardName: "changed-extensions-config", + }, + ]); + }); + it("fails safe when a targeted config needs special shard setup", () => { expect(createChangedNodeTestShards(["scripts/docs-i18n/main.go"])).toBeNull(); expect(createChangedNodeTestShards(["src/tui/tui-pty-harness.e2e.test.ts"])).toBeNull(); diff --git a/test/scripts/ci-workflow-guards.test.ts b/test/scripts/ci-workflow-guards.test.ts index 0a5b549219f2..4ff98904556d 100644 --- a/test/scripts/ci-workflow-guards.test.ts +++ b/test/scripts/ci-workflow-guards.test.ts @@ -261,6 +261,29 @@ function runCiManifestFixture(options: { : ["test/scripts/sqlite-sessions-transcripts-flip-proof.built-cli.e2e.test.ts"], }] : null; + export const createChangedExtensionFallbackShards = (changedPaths) => + changedPaths.some((changedPath) => changedPath.startsWith("extensions/")) + ? changedPaths.some((changedPath) => changedPath.startsWith("extensions/matrix/")) + ? [{ + checkName: "changed-extension-fallback-plan", + configs: ["test/vitest/vitest.extension-matrix.config.ts"], + includePatterns: [ + "extensions/matrix/src/client.test.ts", + "extensions/matrix/src/monitor.test.ts", + ], + requiresDist: false, + runner: "ubuntu-24.04", + shardName: "changed-extension-fallback-plan", + }] + : [{ + checkName: "changed-extension-fallback-plan", + configs: [], + requiresDist: false, + runner: "ubuntu-24.04", + shardName: "changed-extension-fallback-plan", + targets: ["extensions/codex/src/focused.test.ts"], + }] + : []; export const hasBuildArtifactAffectingChange = (changedPaths) => !changedPaths.includes("test/scripts/sqlite-sessions-transcripts-flip-proof.built-cli.e2e.test.ts"); export const hasSqliteSessionLifecycleAffectingChange = (changedPaths) => @@ -5375,7 +5398,7 @@ printf '%s\n' "\${CURL_SUCCESS_IP:-203.0.113.7}" const changedPullRequest = runCiManifestFixture({ bundledPlanner: true, - changedPaths: ["src/focused.ts"], + changedPaths: ["src/focused.ts", "extensions/codex/src/focused.ts"], eventName: "pull_request", }); expect(changedPullRequest.status, changedPullRequest.output).toBe(0); @@ -5393,9 +5416,71 @@ printf '%s\n' "\${CURL_SUCCESS_IP:-203.0.113.7}" targets: ["src/focused.test.ts"], }), ]); + expect( + JSON.parse( + expectDefined( + changedPullRequest.outputs.checks_node_core_nondist_matrix, + "changed PR node matrix output", + ), + ).include, + ).not.toContainEqual( + expect.objectContaining({ check_name: "changed-extension-fallback-plan" }), + ); expect(changedPullRequest.outputs.run_checks_node_core_dist).toBe("true"); expect(changedPullRequest.outputs.run_sqlite_session_lifecycle).toBe("false"); + const mixedFallbackPullRequest = runCiManifestFixture({ + bundledPlanner: true, + changedPaths: [ + "packages/gateway-protocol/src/frame-guards.ts", + "extensions/codex/src/focused.ts", + ], + eventName: "pull_request", + }); + expect(mixedFallbackPullRequest.status, mixedFallbackPullRequest.output).toBe(0); + expect( + JSON.parse( + expectDefined( + mixedFallbackPullRequest.outputs.checks_node_core_nondist_matrix, + "mixed fallback PR node matrix output", + ), + ).include, + ).toEqual( + expect.arrayContaining([ + expect.objectContaining({ check_name: "bundled-node-plan" }), + expect.objectContaining({ check_name: "changed-extension-fallback-plan" }), + ]), + ); + + const matrixFallbackPullRequest = runCiManifestFixture({ + bundledPlanner: true, + changedPaths: [ + "packages/gateway-protocol/src/frame-guards.ts", + "extensions/matrix/src/channel.ts", + ], + eventName: "pull_request", + }); + expect(matrixFallbackPullRequest.status, matrixFallbackPullRequest.output).toBe(0); + expect( + JSON.parse( + expectDefined( + matrixFallbackPullRequest.outputs.checks_node_core_nondist_matrix, + "Matrix fallback PR node matrix output", + ), + ).include, + ).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + check_name: "changed-extension-fallback-plan", + configs: ["test/vitest/vitest.extension-matrix.config.ts"], + includePatterns: [ + "extensions/matrix/src/client.test.ts", + "extensions/matrix/src/monitor.test.ts", + ], + }), + ]), + ); + const sqliteLifecyclePullRequest = runCiManifestFixture({ bundledPlanner: true, changedPaths: ["src/sqlite-session-owner.ts"],