fix(ci): run affected extension suites when changed-test planning falls back (#122885)

The PR-changed test planner fails safe to the compact full-suite plan for any diff touching packages/**, but that compact plan excludes all extension test configs, so mixed package+extension PRs landed with zero extension test execution (escapes: PR #120534 breaking extensions/codex run-attempt.native-hook-relay.test.ts, PRs #122163/#121522 and cd7b7f639d breaking media-understanding-provider.test.ts and thread-lifecycle.test.ts on main full runs). The preflight now appends whole-config shards for the diff's touched extensions whenever the precise plan fails safe; whole configs (not precise targets) because the fail-safe cause leaves the non-extension diff's extension impact unbounded.
This commit is contained in:
Peter Steinberger
2026-08-12 18:51:00 -07:00
committed by GitHub
parent 10a1a43f4b
commit fba9ad43bc
4 changed files with 370 additions and 92 deletions
+20 -4
View File
@@ -653,6 +653,7 @@ jobs:
const compactPlan = const compactPlan =
isCanonicalRepository && (eventName === "pull_request" || eventName === "push"); isCanonicalRepository && (eventName === "pull_request" || eventName === "push");
let changedNodeTestShards = null; let changedNodeTestShards = null;
let changedExtensionFallbackShards = [];
if ( if (
compactPullRequest && compactPullRequest &&
changedPaths && changedPaths &&
@@ -663,6 +664,18 @@ jobs:
} catch (error) { } catch (error) {
console.warn(`Changed Node test planning failed; using compact full suite: ${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 // Heavy packaging lanes run only when the diff touches surfaces they
// exist to prove: test-only diffs cannot change dist bytes, and QA // exist to prove: test-only diffs cannot change dist bytes, and QA
@@ -706,10 +719,13 @@ jobs:
const rawNodeTestShards = runNodeFull const rawNodeTestShards = runNodeFull
? changedNodeTestShards ? changedNodeTestShards
? changedNodeTestShards ? changedNodeTestShards
: createNodeTestPlan({ : [
includeReleaseOnlyPluginShards: false, ...createNodeTestPlan({
compact: compactPlan, includeReleaseOnlyPluginShards: false,
}) compact: compactPlan,
}),
...changedExtensionFallbackShards,
]
: []; : [];
const assignVitestFsCacheWriter = const assignVitestFsCacheWriter =
typeof nodeTestPlan.assignVitestFsCacheWriter === "function" typeof nodeTestPlan.assignVitestFsCacheWriter === "function"
+174 -87
View File
@@ -14,11 +14,16 @@ import {
isPolicyTestOwnedPath, isPolicyTestOwnedPath,
resolvePolicyTestTargets, resolvePolicyTestTargets,
} from "./ci-node-test-plan.mts"; } from "./ci-node-test-plan.mts";
import {
createExtensionTestProcessTargetChunks,
resolveExtensionTestConfig,
} from "./extension-test-plan.mts";
import { buildPluginSdkEntrySources, publicPluginSdkEntrypoints } from "./plugin-sdk-entries.mts"; import { buildPluginSdkEntrySources, publicPluginSdkEntrypoints } from "./plugin-sdk-entries.mts";
type ChangedNodeTestShard = { type ChangedNodeTestShard = {
checkName: string; checkName: string;
configs: string[]; configs: string[];
includePatterns?: string[];
planConcurrency?: number; planConcurrency?: number;
requiresDist: boolean; requiresDist: boolean;
runner: string; 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<string, string[]>();
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. * Builds bounded PR jobs from precise changed-test targets.
* Null means the caller must fail safe to the compact full-suite plan. * Null means the caller must fail safe to the compact full-suite plan.
@@ -259,99 +424,21 @@ export function createChangedNodeTestShards(
return null; return null;
} }
const resolveTargetPlan = (paths: string[]) => const targets = resolvePreciseChangedTargets(
resolveChangedTestTargetPlan(paths, { regularLivePaths,
broad: true, cwd,
combineSiblingWithImportGraph: true, [...policyTargetsByPath.values()].flat(),
cwd, );
forceFullImportGraph: true, if (targets === null) {
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)),
)
) {
return null; return null;
} }
// Boundary-config targets run as regular nondist targets: the boundary // Boundary-config targets run as regular nondist targets: the boundary
// suite scans the checked-out tree and never consumes the built dist. // 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 = [ const shards = [
...targetChunks.map((chunk, index) => { ...createChangedTargetShards(targets, {
const suffix = targetChunks.length === 1 ? "" : `-${index + 1}`; checkName: "checks-node-changed",
const shard: ChangedNodeTestShard = { shardName: "changed",
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;
}), }),
...(hasBuildArtifactAffectingChange(changedPaths) ? [] : [createBoundaryShard()]), ...(hasBuildArtifactAffectingChange(changedPaths) ? [] : [createBoundaryShard()]),
]; ];
@@ -3,6 +3,7 @@ import { tmpdir } from "node:os";
import path from "node:path"; import path from "node:path";
import { describe, expect, it } from "vitest"; import { describe, expect, it } from "vitest";
import { import {
createChangedExtensionFallbackShards,
createChangedNodeTestShards, createChangedNodeTestShards,
hasBuildArtifactAffectingChange, hasBuildArtifactAffectingChange,
hasPromptSnapshotAffectingChange, hasPromptSnapshotAffectingChange,
@@ -264,6 +265,95 @@ describe("CI changed Node test plan", () => {
).toBeNull(); ).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", () => { it("fails safe when a targeted config needs special shard setup", () => {
expect(createChangedNodeTestShards(["scripts/docs-i18n/main.go"])).toBeNull(); expect(createChangedNodeTestShards(["scripts/docs-i18n/main.go"])).toBeNull();
expect(createChangedNodeTestShards(["src/tui/tui-pty-harness.e2e.test.ts"])).toBeNull(); expect(createChangedNodeTestShards(["src/tui/tui-pty-harness.e2e.test.ts"])).toBeNull();
+86 -1
View File
@@ -261,6 +261,29 @@ function runCiManifestFixture(options: {
: ["test/scripts/sqlite-sessions-transcripts-flip-proof.built-cli.e2e.test.ts"], : ["test/scripts/sqlite-sessions-transcripts-flip-proof.built-cli.e2e.test.ts"],
}] }]
: null; : 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) => export const hasBuildArtifactAffectingChange = (changedPaths) =>
!changedPaths.includes("test/scripts/sqlite-sessions-transcripts-flip-proof.built-cli.e2e.test.ts"); !changedPaths.includes("test/scripts/sqlite-sessions-transcripts-flip-proof.built-cli.e2e.test.ts");
export const hasSqliteSessionLifecycleAffectingChange = (changedPaths) => export const hasSqliteSessionLifecycleAffectingChange = (changedPaths) =>
@@ -5375,7 +5398,7 @@ printf '%s\n' "\${CURL_SUCCESS_IP:-203.0.113.7}"
const changedPullRequest = runCiManifestFixture({ const changedPullRequest = runCiManifestFixture({
bundledPlanner: true, bundledPlanner: true,
changedPaths: ["src/focused.ts"], changedPaths: ["src/focused.ts", "extensions/codex/src/focused.ts"],
eventName: "pull_request", eventName: "pull_request",
}); });
expect(changedPullRequest.status, changedPullRequest.output).toBe(0); 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"], 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_checks_node_core_dist).toBe("true");
expect(changedPullRequest.outputs.run_sqlite_session_lifecycle).toBe("false"); 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({ const sqliteLifecyclePullRequest = runCiManifestFixture({
bundledPlanner: true, bundledPlanner: true,
changedPaths: ["src/sqlite-session-owner.ts"], changedPaths: ["src/sqlite-session-owner.ts"],