From 8b501c1c13435c3975ee9520a111b765eb444aa3 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 9 Aug 2026 03:38:37 -0700 Subject: [PATCH] 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. --- scripts/build-all.d.mts | 3 +- scripts/build-all.mjs | 41 +++++++++- scripts/lib/plugin-sdk-entries.d.mts | 1 + scripts/lib/plugin-sdk-entries.mjs | 7 +- ...e-extension-package-boundary-artifacts.mjs | 16 ++-- scripts/write-plugin-sdk-entry-dts.ts | 5 +- test/scripts/build-all.test.ts | 78 +++++++++++++++++++ ...tension-package-boundary-artifacts.test.ts | 11 +++ 8 files changed, 149 insertions(+), 13 deletions(-) diff --git a/scripts/build-all.d.mts b/scripts/build-all.d.mts index 1b53176ae9b8..98731cfa4144 100644 --- a/scripts/build-all.d.mts +++ b/scripts/build-all.d.mts @@ -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, diff --git a/scripts/build-all.mjs b/scripts/build-all.mjs index 6341cac73b83..5fe5e8cf4fa7 100644 --- a/scripts/build-all.mjs +++ b/scripts/build-all.mjs @@ -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, diff --git a/scripts/lib/plugin-sdk-entries.d.mts b/scripts/lib/plugin-sdk-entries.d.mts index dd5dd8eac65e..5a9c1a98acf7 100644 --- a/scripts/lib/plugin-sdk-entries.d.mts +++ b/scripts/lib/plugin-sdk-entries.d.mts @@ -9,6 +9,7 @@ export const deprecatedPublicPluginSdkEntrypoints: string[]; export const deprecatedBarrelPluginSdkEntrypoints: string[]; export function buildPluginSdkEntrySources(entries?: readonly string[]): Record; +export function listPluginSdkDeclarationOutputs(entries?: readonly string[]): string[]; export function buildPluginSdkPackageExports(): Record< string, { diff --git a/scripts/lib/plugin-sdk-entries.mjs b/scripts/lib/plugin-sdk-entries.mjs index 9adb7747c9cd..8cf086cc5c4b 100644 --- a/scripts/lib/plugin-sdk-entries.mjs +++ b/scripts/lib/plugin-sdk-entries.mjs @@ -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`), ]; } diff --git a/scripts/prepare-extension-package-boundary-artifacts.mjs b/scripts/prepare-extension-package-boundary-artifacts.mjs index 0f23499b6569..b097af083788 100644 --- a/scripts/prepare-extension-package-boundary-artifacts.mjs +++ b/scripts/prepare-extension-package-boundary-artifacts.mjs @@ -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) { diff --git a/scripts/write-plugin-sdk-entry-dts.ts b/scripts/write-plugin-sdk-entry-dts.ts index d80552d16488..39b5091f37e3 100644 --- a/scripts/write-plugin-sdk-entry-dts.ts +++ b/scripts/write-plugin-sdk-entry-dts.ts @@ -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)}`, diff --git a/test/scripts/build-all.test.ts b/test/scripts/build-all.test.ts index 612931f19eee..b1650652d7dd 100644 --- a/test/scripts/build-all.test.ts +++ b/test/scripts/build-all.test.ts @@ -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 }); diff --git a/test/scripts/prepare-extension-package-boundary-artifacts.test.ts b/test/scripts/prepare-extension-package-boundary-artifacts.test.ts index 34e75c572bf0..1049a279d39c 100644 --- a/test/scripts/prepare-extension-package-boundary-artifacts.test.ts +++ b/test/scripts/prepare-extension-package-boundary-artifacts.test.ts @@ -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");