mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(build): reject incomplete Plugin SDK declaration caches (#120961)
Require the unified declaration cache to contain every production Plugin SDK declaration before accepting or stamping it. Matching partial v4 stamps are now stale and rebuild instead of failing later in the canonical declaration writer.
This commit is contained in:
committed by
GitHub
parent
693b62f69e
commit
8b501c1c13
@@ -20,6 +20,7 @@ export type BuildAllStep = {
|
||||
env?: string[];
|
||||
inputs: BuildCacheEntry[];
|
||||
outputs: BuildCacheEntry[];
|
||||
requiredOutputs?: string[] | ((env: NodeJS.ProcessEnv) => string[]);
|
||||
restore?: "always";
|
||||
runOnHit?: {
|
||||
env?: NodeJS.ProcessEnv;
|
||||
@@ -81,7 +82,7 @@ export function resolveBuildAllStepCacheState(
|
||||
export function writeBuildAllStepCacheStamp(
|
||||
step: BuildAllStep,
|
||||
cacheState: BuildAllCacheState,
|
||||
params?: { rootDir?: string; fs?: typeof fs },
|
||||
params?: { rootDir?: string; fs?: typeof fs; env?: NodeJS.ProcessEnv },
|
||||
): void;
|
||||
export function resolveBuildAllStepCacheStampState(
|
||||
step: BuildAllStep,
|
||||
|
||||
+39
-2
@@ -8,7 +8,10 @@ import path from "node:path";
|
||||
import { performance } from "node:perf_hooks";
|
||||
import { pathToFileURL } from "node:url";
|
||||
import prettyMilliseconds from "pretty-ms";
|
||||
import { pluginSdkEntrypoints } from "./lib/plugin-sdk-entries.mjs";
|
||||
import {
|
||||
listPluginSdkDeclarationOutputs,
|
||||
pluginSdkEntrypoints,
|
||||
} from "./lib/plugin-sdk-entries.mjs";
|
||||
import {
|
||||
TSDOWN_PACKAGE_CONFIG_GROUP,
|
||||
TSDOWN_UNIFIED_CONFIG_GROUP,
|
||||
@@ -182,6 +185,10 @@ export const BUILD_ALL_STEPS = [
|
||||
...TSDOWN_UNIFIED_CACHE_INPUTS,
|
||||
],
|
||||
outputs: declarationCacheOutputs(["dist"]),
|
||||
requiredOutputs: (env) =>
|
||||
env.OPENCLAW_BUILD_PRIVATE_QA === "1"
|
||||
? listPluginSdkDeclarationOutputs(pluginSdkEntrypoints)
|
||||
: listPluginSdkDeclarationOutputs(),
|
||||
restore: "always",
|
||||
runOnHit: {
|
||||
env: { OPENCLAW_RUN_NODE_SKIP_DTS_BUILD: "1" },
|
||||
@@ -622,6 +629,14 @@ function normalizePortablePath(filePath) {
|
||||
return filePath.replaceAll("\\", "/");
|
||||
}
|
||||
|
||||
function resolveCacheRequiredOutputs(cache, env) {
|
||||
const outputs =
|
||||
typeof cache.requiredOutputs === "function"
|
||||
? cache.requiredOutputs(env)
|
||||
: (cache.requiredOutputs ?? []);
|
||||
return outputs.map((output) => normalizePortablePath(output));
|
||||
}
|
||||
|
||||
function resolveBuildCacheRoot(rootDir, env) {
|
||||
// Dev update preflight and final builds run in separate worktrees. A shared
|
||||
// root lets content signatures decide reuse without relocating built trees.
|
||||
@@ -709,7 +724,17 @@ export function resolveBuildAllStepCacheState(step, params = {}) {
|
||||
const stampedOutputs = Array.isArray(stamp?.outputs)
|
||||
? stamp.outputs.map((entry) => normalizePortablePath(entry))
|
||||
: [];
|
||||
const stampMatches = stamp?.version === BUILD_CACHE_VERSION && stamp.signature === signature;
|
||||
const requiredOutputs = resolveCacheRequiredOutputs(step.cache, params.env ?? process.env);
|
||||
const stampedOutputSet = new Set(stampedOutputs);
|
||||
// Restore trusts the stamp inventory, so legacy partial stamps must name the
|
||||
// complete current contract before either output tree can make them fresh.
|
||||
const stampIncludesRequiredOutputs = requiredOutputs.every((output) =>
|
||||
stampedOutputSet.has(output),
|
||||
);
|
||||
const stampMatches =
|
||||
stamp?.version === BUILD_CACHE_VERSION &&
|
||||
stamp.signature === signature &&
|
||||
stampIncludesRequiredOutputs;
|
||||
const actualOutputsPresent =
|
||||
stampedOutputs.length > 0 && hasAllFiles(rootDir, stampedOutputs, fsImpl);
|
||||
const cachedOutputsPresent =
|
||||
@@ -746,6 +771,18 @@ export function writeBuildAllStepCacheStamp(step, cacheState, params = {}) {
|
||||
}
|
||||
const fsImpl = params.fs ?? fs;
|
||||
const rootDir = params.rootDir ?? process.cwd();
|
||||
const requiredOutputs = resolveCacheRequiredOutputs(step.cache, params.env ?? process.env);
|
||||
const relativeOutputSet = new Set(
|
||||
cacheState.relativeOutputFiles.map((output) => normalizePortablePath(output)),
|
||||
);
|
||||
// Validate before copying so an incomplete run cannot mutate the cached tree
|
||||
// while leaving its previous stamp in place.
|
||||
if (
|
||||
!requiredOutputs.every((output) => relativeOutputSet.has(output)) ||
|
||||
!hasAllFiles(rootDir, requiredOutputs, fsImpl)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
for (const relativeFile of cacheState.relativeOutputFiles) {
|
||||
copyFileSync(
|
||||
fsImpl,
|
||||
|
||||
@@ -9,6 +9,7 @@ export const deprecatedPublicPluginSdkEntrypoints: string[];
|
||||
export const deprecatedBarrelPluginSdkEntrypoints: string[];
|
||||
|
||||
export function buildPluginSdkEntrySources(entries?: readonly string[]): Record<string, string>;
|
||||
export function listPluginSdkDeclarationOutputs(entries?: readonly string[]): string[];
|
||||
export function buildPluginSdkPackageExports(): Record<
|
||||
string,
|
||||
{
|
||||
|
||||
@@ -76,6 +76,11 @@ export const productionPluginSdkEntrypoints = pluginSdkEntrypoints.filter(
|
||||
(entry) => !nonProductionPluginSdkSubpathSet.has(entry),
|
||||
);
|
||||
|
||||
/** List flat plugin SDK declaration outputs for the selected entrypoints. */
|
||||
export function listPluginSdkDeclarationOutputs(entries = productionPluginSdkEntrypoints) {
|
||||
return entries.map((entry) => `dist/plugin-sdk/${entry}.d.ts`);
|
||||
}
|
||||
|
||||
const productionPluginSdkEntrypointSet = new Set(productionPluginSdkEntrypoints);
|
||||
|
||||
/** Private runtime facades required by bundled or separately published official plugins. */
|
||||
@@ -174,7 +179,7 @@ export function listPackagedPrivatePluginSdkRuntimeArtifacts() {
|
||||
/** List private artifacts that must stay out of package output. */
|
||||
export function listUnpackagedPrivatePluginSdkDistArtifacts() {
|
||||
return [
|
||||
...privateLocalOnlyPluginSdkEntrypoints.map((entry) => `dist/plugin-sdk/${entry}.d.ts`),
|
||||
...listPluginSdkDeclarationOutputs(privateLocalOnlyPluginSdkEntrypoints),
|
||||
...nonProductionPrivatePluginSdkEntrypoints.map((entry) => `dist/plugin-sdk/${entry}.js`),
|
||||
];
|
||||
}
|
||||
|
||||
@@ -11,7 +11,11 @@ import {
|
||||
resolveRepoToolBinPath,
|
||||
} from "./lib/local-heavy-check-runtime.mjs";
|
||||
import { parsePositiveInt } from "./lib/numeric-options.mjs";
|
||||
import { pluginSdkEntrypoints, productionPluginSdkEntrypoints } from "./lib/plugin-sdk-entries.mjs";
|
||||
import {
|
||||
listPluginSdkDeclarationOutputs,
|
||||
pluginSdkEntrypoints,
|
||||
productionPluginSdkEntrypoints,
|
||||
} from "./lib/plugin-sdk-entries.mjs";
|
||||
import { resolveRepoRoot } from "./lib/repo-root.mjs";
|
||||
import { resolveWindowsTaskkillPath } from "./lib/windows-taskkill.mjs";
|
||||
const repoRoot = resolveRepoRoot(import.meta.url);
|
||||
@@ -324,12 +328,10 @@ const ENTRY_SHIMS_INPUTS = [
|
||||
export function resolveBoundaryEntryShimRequiredOutputs(env = process.env) {
|
||||
const entries =
|
||||
env.OPENCLAW_BUILD_PRIVATE_QA === "1" ? pluginSdkEntrypoints : productionPluginSdkEntrypoints;
|
||||
return entries
|
||||
.flatMap((entry) => [
|
||||
`dist/plugin-sdk/${entry}.d.ts`,
|
||||
`packages/plugin-sdk/dist/src/plugin-sdk/${entry}.d.ts`,
|
||||
])
|
||||
.toSorted((a, b) => a.localeCompare(b));
|
||||
return [
|
||||
...listPluginSdkDeclarationOutputs(entries),
|
||||
...entries.map((entry) => `packages/plugin-sdk/dist/src/plugin-sdk/${entry}.d.ts`),
|
||||
].toSorted((a, b) => a.localeCompare(b));
|
||||
}
|
||||
|
||||
function isRelevantTypeInput(filePath) {
|
||||
|
||||
@@ -5,6 +5,7 @@ import path from "node:path";
|
||||
import { build } from "tsdown";
|
||||
import {
|
||||
buildPluginSdkEntrySources,
|
||||
listPluginSdkDeclarationOutputs,
|
||||
pluginSdkEntrypoints,
|
||||
productionPluginSdkEntrypoints,
|
||||
} from "./lib/plugin-sdk-entries.mjs";
|
||||
@@ -56,8 +57,8 @@ const flatDeclarationEntrypoints = shouldBuildPrivateQaEntries
|
||||
const flatDeclarationEntrypointSet = new Set(flatDeclarationEntrypoints);
|
||||
|
||||
if (USE_CANONICAL_DECLARATIONS) {
|
||||
for (const entry of flatDeclarationEntrypoints) {
|
||||
const declarationPath = path.join(distPluginSdkDir, `${entry}.d.ts`);
|
||||
for (const relativePath of listPluginSdkDeclarationOutputs(flatDeclarationEntrypoints)) {
|
||||
const declarationPath = path.resolve(process.cwd(), relativePath);
|
||||
if (!fs.existsSync(declarationPath)) {
|
||||
throw new Error(
|
||||
`Missing canonical plugin SDK declaration: ${path.relative(process.cwd(), declarationPath)}`,
|
||||
|
||||
@@ -23,6 +23,10 @@ import {
|
||||
restoreBuildAllStepCacheOutputs,
|
||||
writeBuildAllStepCacheStamp,
|
||||
} from "../../scripts/build-all.mjs";
|
||||
import {
|
||||
listPluginSdkDeclarationOutputs,
|
||||
pluginSdkEntrypoints,
|
||||
} from "../../scripts/lib/plugin-sdk-entries.mjs";
|
||||
|
||||
function getBuildAllStep(label: string) {
|
||||
const step = BUILD_ALL_STEPS.find((entry) => entry.label === label);
|
||||
@@ -58,6 +62,7 @@ function withBuildCacheFixture(
|
||||
recursive?: boolean;
|
||||
}
|
||||
>;
|
||||
requiredOutputs?: string[] | ((env: NodeJS.ProcessEnv) => string[]);
|
||||
restore?: "always";
|
||||
runOnHit?: {
|
||||
env?: NodeJS.ProcessEnv;
|
||||
@@ -383,6 +388,18 @@ describe("resolveBuildAllSteps", () => {
|
||||
}),
|
||||
]),
|
||||
);
|
||||
const requiredOutputs = unified.cache?.requiredOutputs;
|
||||
if (typeof requiredOutputs !== "function") {
|
||||
throw new Error("Missing tsdown-unified required output resolver");
|
||||
}
|
||||
const productionOutputs = requiredOutputs({});
|
||||
const privateQaOutputs = requiredOutputs({ OPENCLAW_BUILD_PRIVATE_QA: "1" });
|
||||
expect(productionOutputs).toEqual(listPluginSdkDeclarationOutputs());
|
||||
expect(privateQaOutputs).toEqual(listPluginSdkDeclarationOutputs(pluginSdkEntrypoints));
|
||||
expect(productionOutputs).toContain("dist/plugin-sdk/core.d.ts");
|
||||
expect(productionOutputs).toContain("dist/plugin-sdk/provider-auth-runtime.d.ts");
|
||||
expect(productionOutputs).not.toContain("dist/plugin-sdk/test-fixtures.d.ts");
|
||||
expect(privateQaOutputs).toContain("dist/plugin-sdk/test-fixtures.d.ts");
|
||||
});
|
||||
|
||||
it("uses a runtime artifact plus plugin SDK export profile for ci artifacts", () => {
|
||||
@@ -867,6 +884,67 @@ describe("resolveBuildAllStepCacheState", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects a matching legacy stamp that omits a required output", () => {
|
||||
withBuildCacheFixture(({ rootDir, step }) => {
|
||||
const legacyState = resolveBuildAllStepCacheState(step, { rootDir });
|
||||
writeBuildAllStepCacheStamp(step, legacyState, { rootDir });
|
||||
const legacyStamp = JSON.parse(fs.readFileSync(legacyState.stampPath!, "utf8"));
|
||||
expect(legacyStamp).toMatchObject({
|
||||
version: 4,
|
||||
signature: legacyState.signature,
|
||||
outputs: ["dist/output.js"],
|
||||
});
|
||||
fs.rmSync(path.join(rootDir, "dist"), { force: true, recursive: true });
|
||||
|
||||
const completeStep = {
|
||||
...step,
|
||||
cache: {
|
||||
...step.cache,
|
||||
requiredOutputs: ["dist/output.js", "dist/plugin-sdk/core.d.ts"],
|
||||
restore: "always" as const,
|
||||
},
|
||||
};
|
||||
const stale = resolveBuildAllStepCacheState(completeStep, { rootDir });
|
||||
|
||||
expect(stale).toMatchObject({
|
||||
fresh: false,
|
||||
reason: "stale",
|
||||
restorable: false,
|
||||
signature: legacyState.signature,
|
||||
stampedOutputs: ["dist/output.js"],
|
||||
});
|
||||
expect(restoreBuildAllStepCacheOutputs(stale, { rootDir })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
it("does not replace a cache stamp from incomplete current outputs", () => {
|
||||
withBuildCacheFixture(({ rootDir, outputPath, step }) => {
|
||||
const initialState = resolveBuildAllStepCacheState(step, { rootDir });
|
||||
writeBuildAllStepCacheStamp(step, initialState, { rootDir });
|
||||
const stampBefore = fs.readFileSync(initialState.stampPath!, "utf8");
|
||||
const cachedOutputPath = path.join(initialState.outputRoot!, "dist/output.js");
|
||||
expect(fs.readFileSync(cachedOutputPath, "utf8")).toBe("output");
|
||||
|
||||
fs.writeFileSync(outputPath, "incomplete refresh");
|
||||
const completeStep = {
|
||||
...step,
|
||||
cache: {
|
||||
...step.cache,
|
||||
requiredOutputs: ["dist/output.js", "dist/plugin-sdk/core.d.ts"],
|
||||
},
|
||||
};
|
||||
const incompleteState = resolveBuildAllStepCacheState(completeStep, { rootDir });
|
||||
expect(finalizeBuildAllStepCache(completeStep, incompleteState, { rootDir })).toBe(true);
|
||||
|
||||
expect(fs.readFileSync(initialState.stampPath!, "utf8")).toBe(stampBefore);
|
||||
expect(fs.readFileSync(cachedOutputPath, "utf8")).toBe("output");
|
||||
expect(resolveBuildAllStepCacheState(completeStep, { rootDir })).toMatchObject({
|
||||
fresh: false,
|
||||
reason: "stale",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it("marks cacheable steps stale when an input changes", () => {
|
||||
withBuildCacheFixture(({ rootDir, inputPath, step }) => {
|
||||
const cacheState = resolveBuildAllStepCacheState(step, { rootDir });
|
||||
|
||||
@@ -9,6 +9,10 @@ import { setTimeout as delay } from "node:timers/promises";
|
||||
import { pathToFileURL } from "node:url";
|
||||
import { MAX_TIMER_TIMEOUT_MS } from "@openclaw/normalization-core/number-coercion";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
listPluginSdkDeclarationOutputs,
|
||||
pluginSdkEntrypoints,
|
||||
} from "../../scripts/lib/plugin-sdk-entries.mjs";
|
||||
import { resolveWindowsTaskkillPath } from "../../scripts/lib/windows-taskkill.mjs";
|
||||
import {
|
||||
createPrefixedOutputWriter,
|
||||
@@ -625,6 +629,13 @@ describe("prepare-extension-package-boundary-artifacts", () => {
|
||||
OPENCLAW_BUILD_PRIVATE_QA: "1",
|
||||
});
|
||||
|
||||
expect(productionOutputs.filter((output) => output.startsWith("dist/plugin-sdk/"))).toEqual(
|
||||
listPluginSdkDeclarationOutputs().toSorted((a, b) => a.localeCompare(b)),
|
||||
);
|
||||
expect(privateQaOutputs.filter((output) => output.startsWith("dist/plugin-sdk/"))).toEqual(
|
||||
listPluginSdkDeclarationOutputs(pluginSdkEntrypoints).toSorted((a, b) => a.localeCompare(b)),
|
||||
);
|
||||
|
||||
expect(productionOutputs).toContain("dist/plugin-sdk/provider-auth-runtime.d.ts");
|
||||
expect(productionOutputs).not.toContain("dist/plugin-sdk/test-fixtures.d.ts");
|
||||
expect(privateQaOutputs).toContain("dist/plugin-sdk/provider-auth-runtime.d.ts");
|
||||
|
||||
Reference in New Issue
Block a user