diff --git a/cli/src/commands/flow/flow.ts b/cli/src/commands/flow/flow.ts index eff742ee68..f0974cf354 100644 --- a/cli/src/commands/flow/flow.ts +++ b/cli/src/commands/flow/flow.ts @@ -56,13 +56,20 @@ export async function pushFlow( } const localFlow = (await yamlParseFile(localPath + "flow.yaml")) as FlowFile; + const fileReader = async (path: string) => await readFile(localPath + path, "utf-8"); await replaceInlineScripts( localFlow.value.modules, - async (path: string) => await readFile(localPath + path, "utf-8"), + fileReader, log, localPath, SEP ); + if (localFlow.value.failure_module) { + await replaceInlineScripts([localFlow.value.failure_module], fileReader, log, localPath, SEP); + } + if (localFlow.value.preprocessor_module) { + await replaceInlineScripts([localFlow.value.preprocessor_module], fileReader, log, localPath, SEP); + } if (flow) { if (isSuperset(localFlow, flow)) { @@ -252,13 +259,20 @@ async function preview( const localFlow = (await yamlParseFile(flowPath + "flow.yaml")) as FlowFile; // Replace inline scripts with their actual content + const fileReader = async (path: string) => await readFile(flowPath + path, "utf-8"); await replaceInlineScripts( localFlow.value.modules, - async (path: string) => await readFile(flowPath + path, "utf-8"), + fileReader, log, flowPath, SEP ); + if (localFlow.value.failure_module) { + await replaceInlineScripts([localFlow.value.failure_module], fileReader, log, flowPath, SEP); + } + if (localFlow.value.preprocessor_module) { + await replaceInlineScripts([localFlow.value.preprocessor_module], fileReader, log, flowPath, SEP); + } const input = opts.data ? await resolve(opts.data) : {}; diff --git a/cli/src/commands/flow/flow_metadata.ts b/cli/src/commands/flow/flow_metadata.ts index cb2e5336e6..d52d05a3ed 100644 --- a/cli/src/commands/flow/flow_metadata.ts +++ b/cli/src/commands/flow/flow_metadata.ts @@ -19,6 +19,7 @@ import { } from "../../utils/metadata.ts"; import { ScriptLanguage } from "../../utils/script_common.ts"; import { extractInlineScripts as extractInlineScriptsForFlows } from "../../../windmill-utils-internal/src/inline-scripts/extractor.ts"; +import { newPathAssigner } from "../../../windmill-utils-internal/src/path-utils/path-assigner.ts"; import { generateHash, getHeaders, writeIfChanged } from "../../utils/utils.ts"; import { exts } from "../script/script.ts"; @@ -121,14 +122,21 @@ export async function generateFlowLockInternal( } log.info(`Recomputing locks of ${changedScripts.join(", ")} in ${folder}`); + const fileReader = async (path: string) => await readFile(folder + SEP + path, "utf-8"); await replaceInlineScripts( flowValue.value.modules, - async (path: string) => await readFile(folder + SEP + path, "utf-8"), + fileReader, log, folder + SEP!, SEP, changedScripts ); + if (flowValue.value.failure_module) { + await replaceInlineScripts([flowValue.value.failure_module], fileReader, log, folder + SEP!, SEP, changedScripts); + } + if (flowValue.value.preprocessor_module) { + await replaceInlineScripts([flowValue.value.preprocessor_module], fileReader, log, folder + SEP!, SEP, changedScripts); + } //removeChangedLocks flowValue.value = await updateFlow( @@ -138,12 +146,20 @@ export async function generateFlowLockInternal( filteredDeps ); + const lockAssigner = newPathAssigner(opts.defaultTs ?? "bun"); const inlineScripts = extractInlineScriptsForFlows( flowValue.value.modules, {}, SEP, - opts.defaultTs + opts.defaultTs, + lockAssigner ); + if (flowValue.value.failure_module) { + inlineScripts.push(...extractInlineScriptsForFlows([flowValue.value.failure_module], {}, SEP, opts.defaultTs, lockAssigner)); + } + if (flowValue.value.preprocessor_module) { + inlineScripts.push(...extractInlineScriptsForFlows([flowValue.value.preprocessor_module], {}, SEP, opts.defaultTs, lockAssigner)); + } inlineScripts.forEach((s) => { writeIfChanged(process.cwd() + SEP + folder + SEP + s.path, s.content); }); @@ -176,7 +192,15 @@ async function filterWorkspaceDependenciesForFlow( rawWorkspaceDependencies: Record, folder: string ): Promise> { - const inlineScripts = extractInlineScriptsForFlows(structuredClone(flowValue.modules), {}, SEP, undefined); + const clonedValue = structuredClone(flowValue); + const depAssigner = newPathAssigner("bun"); + const inlineScripts = extractInlineScriptsForFlows(clonedValue.modules, {}, SEP, undefined, depAssigner); + if (clonedValue.failure_module) { + inlineScripts.push(...extractInlineScriptsForFlows([clonedValue.failure_module], {}, SEP, undefined, depAssigner)); + } + if (clonedValue.preprocessor_module) { + inlineScripts.push(...extractInlineScriptsForFlows([clonedValue.preprocessor_module], {}, SEP, undefined, depAssigner)); + } // Filter out lock files and map to common interface const scripts = inlineScripts diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index 7d48a14848..6375a55ada 100644 --- a/cli/src/commands/sync/sync.ts +++ b/cli/src/commands/sync/sync.ts @@ -592,14 +592,35 @@ function ZipFSElement( } let inlineScripts; try { + const assigner = newPathAssigner(defaultTs, { skipInlineScriptSuffix: getNonDottedPaths() }); inlineScripts = extractInlineScriptsForFlows( flow.value.modules as any, {}, SEP, defaultTs, - undefined, // pathAssigner - let it create one + assigner, { skipInlineScriptSuffix: getNonDottedPaths() }, ); + if (flow.value.failure_module) { + inlineScripts.push(...extractInlineScriptsForFlows( + [flow.value.failure_module], + {}, + SEP, + defaultTs, + assigner, + { skipInlineScriptSuffix: getNonDottedPaths() }, + )); + } + if (flow.value.preprocessor_module) { + inlineScripts.push(...extractInlineScriptsForFlows( + [flow.value.preprocessor_module], + {}, + SEP, + defaultTs, + assigner, + { skipInlineScriptSuffix: getNonDottedPaths() }, + )); + } } catch (error) { log.error( `Failed to extract inline scripts for flow at path: ${p}`, diff --git a/cli/test/inline_scripts_failure_preprocessor.test.ts b/cli/test/inline_scripts_failure_preprocessor.test.ts new file mode 100644 index 0000000000..1a9150a257 --- /dev/null +++ b/cli/test/inline_scripts_failure_preprocessor.test.ts @@ -0,0 +1,498 @@ +/** + * Unit tests for failure_module and preprocessor_module inline script + * extraction (pull) and replacement (push). + * + * These tests verify that rawscript content in failure_module and + * preprocessor_module is correctly extracted to !inline references + * and resolved back, matching the existing behavior for regular modules. + */ + +import { expect, test, describe } from "bun:test"; +import { extractInlineScripts, extractCurrentMapping } from "../windmill-utils-internal/src/inline-scripts/extractor.ts"; +import { replaceInlineScripts } from "../windmill-utils-internal/src/inline-scripts/replacer.ts"; +import { newPathAssigner } from "../windmill-utils-internal/src/path-utils/path-assigner.ts"; +import type { FlowModule } from "../windmill-utils-internal/src/gen/types.gen.ts"; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeRawscriptModule( + id: string, + content: string, + language: "bun" | "python3" | "deno" = "bun", + lock?: string, +): FlowModule { + return { + id, + value: { + type: "rawscript" as const, + content, + language, + lock: lock, + input_transforms: {}, + }, + }; +} + +const noopLogger = { + info: () => {}, + error: () => {}, +}; + +// --------------------------------------------------------------------------- +// extractInlineScripts — PULL direction +// --------------------------------------------------------------------------- + +describe("extractInlineScripts for failure_module / preprocessor_module", () => { + test("extracts rawscript from failure_module wrapped in array", () => { + const failureModule = makeRawscriptModule( + "failure", + 'export function main() { throw new Error("handler"); }', + "bun", + ); + + const scripts = extractInlineScripts([failureModule], {}, "/", "bun"); + + expect(scripts.length).toBeGreaterThanOrEqual(1); + const script = scripts.find((s) => !s.is_lock); + expect(script).toBeDefined(); + expect(script!.content).toBe( + 'export function main() { throw new Error("handler"); }', + ); + // The module content should have been replaced with an !inline reference + expect(failureModule.value.content).toStartWith("!inline "); + }); + + test("extracts rawscript from preprocessor_module wrapped in array", () => { + const preprocessorModule = makeRawscriptModule( + "preprocessor", + "export function main() { return {}; }", + "python3", + ); + + const scripts = extractInlineScripts( + [preprocessorModule], + {}, + "/", + "bun", + ); + + expect(scripts.length).toBeGreaterThanOrEqual(1); + const script = scripts.find((s) => !s.is_lock); + expect(script).toBeDefined(); + expect(script!.content).toBe("export function main() { return {}; }"); + expect(script!.language).toBe("python3"); + expect(preprocessorModule.value.content).toStartWith("!inline "); + }); + + test("extracts lock alongside content", () => { + const mod = makeRawscriptModule( + "failure", + "console.log('hi')", + "bun", + "some-lock-content", + ); + + const scripts = extractInlineScripts([mod], {}, "/", "bun"); + + const contentScript = scripts.find((s) => !s.is_lock); + const lockScript = scripts.find((s) => s.is_lock); + expect(contentScript).toBeDefined(); + expect(lockScript).toBeDefined(); + expect(lockScript!.content).toBe("some-lock-content"); + expect((mod.value as any).lock).toStartWith("!inline "); + }); + + test("shared pathAssigner prevents collisions when summaries match", () => { + // If a regular module and failure_module share the same summary, + // a shared PathAssigner deduplicates via its internal counter. + const regular = makeRawscriptModule("a", "code_a", "bun"); + regular.summary = "my step"; + const failure = makeRawscriptModule("failure", "code_failure", "bun"); + failure.summary = "my step"; // same summary — would collide without shared assigner + + const assigner = newPathAssigner("bun"); + const scripts1 = extractInlineScripts([regular], {}, "/", "bun", assigner); + const scripts2 = extractInlineScripts([failure], {}, "/", "bun", assigner); + + const allPaths = [...scripts1, ...scripts2] + .filter((s) => !s.is_lock) + .map((s) => s.path); + + // All paths should be unique despite identical summaries + expect(allPaths.length).toBe(2); + expect(new Set(allPaths).size).toBe(2); + }); + + test("without shared pathAssigner, identical summaries produce duplicate paths", () => { + // Demonstrates the problem that sharing a PathAssigner solves. + const regular = makeRawscriptModule("a", "code_a", "bun"); + regular.summary = "my step"; + const failure = makeRawscriptModule("failure", "code_failure", "bun"); + failure.summary = "my step"; + + // Separate assigners — each starts with a fresh counter + const scripts1 = extractInlineScripts([regular], {}, "/", "bun"); + const scripts2 = extractInlineScripts([failure], {}, "/", "bun"); + + const allPaths = [...scripts1, ...scripts2] + .filter((s) => !s.is_lock) + .map((s) => s.path); + + // Without a shared assigner, the paths collide + expect(allPaths.length).toBe(2); + expect(new Set(allPaths).size).toBe(1); // both got the same path + }); + + test("skips non-rawscript failure_module (identity type)", () => { + const identityModule: FlowModule = { + id: "failure", + value: { type: "identity" as any }, + }; + const scripts = extractInlineScripts([identityModule], {}, "/", "bun"); + expect(scripts).toEqual([]); + }); +}); + +// --------------------------------------------------------------------------- +// replaceInlineScripts — PUSH direction +// --------------------------------------------------------------------------- + +describe("replaceInlineScripts for failure_module / preprocessor_module", () => { + test("resolves !inline reference back to file content", async () => { + const failureModule = makeRawscriptModule( + "failure", + "!inline failure.inline_script.ts", + "bun", + ); + + const files: Record = { + "failure.inline_script.ts": 'export function main() { return "error handled"; }', + }; + + await replaceInlineScripts( + [failureModule], + async (path) => { + if (!(path in files)) throw new Error(`File not found: ${path}`); + return files[path]; + }, + noopLogger, + "/tmp/test/", + "/", + ); + + expect(failureModule.value.content).toBe( + 'export function main() { return "error handled"; }', + ); + }); + + test("resolves !inline reference for preprocessor_module", async () => { + const preprocessorModule = makeRawscriptModule( + "preprocessor", + "!inline preprocessor.inline_script.py", + "python3", + ); + + const files: Record = { + "preprocessor.inline_script.py": "def main(): return {}", + }; + + await replaceInlineScripts( + [preprocessorModule], + async (path) => { + if (!(path in files)) throw new Error(`File not found: ${path}`); + return files[path]; + }, + noopLogger, + "/tmp/test/", + "/", + ); + + expect(preprocessorModule.value.content).toBe("def main(): return {}"); + }); + + test("resolves !inline lock reference", async () => { + const mod = makeRawscriptModule( + "failure", + "!inline failure.inline_script.ts", + "bun", + "!inline failure.inline_script.lock", + ); + + const files: Record = { + "failure.inline_script.ts": "code here", + "failure.inline_script.lock": "lock-data-here", + }; + + await replaceInlineScripts( + [mod], + async (path) => { + if (!(path in files)) throw new Error(`File not found: ${path}`); + return files[path]; + }, + noopLogger, + "/tmp/test/", + "/", + ); + + expect(mod.value.content).toBe("code here"); + expect((mod.value as any).lock).toBe("lock-data-here"); + }); + + test("leaves non-inline content untouched", async () => { + const mod = makeRawscriptModule( + "failure", + "export function main() { return 1; }", + "bun", + ); + + await replaceInlineScripts( + [mod], + async () => { + throw new Error("should not be called"); + }, + noopLogger, + "/tmp/test/", + "/", + ); + + expect(mod.value.content).toBe("export function main() { return 1; }"); + }); +}); + +// --------------------------------------------------------------------------- +// Round-trip: extract then replace +// --------------------------------------------------------------------------- + +describe("round-trip extract → replace for failure_module / preprocessor_module", () => { + test("failure_module content survives extract + replace", async () => { + const originalContent = 'export function main(error: any) {\n console.error(error);\n return { handled: true };\n}'; + const failureModule = makeRawscriptModule( + "failure", + originalContent, + "bun", + ); + + // PULL: extract inline scripts (mutates module in place) + const extracted = extractInlineScripts([failureModule], {}, "/", "bun"); + expect(failureModule.value.content).toStartWith("!inline "); + + // Build a virtual filesystem from extracted scripts + const files: Record = {}; + for (const s of extracted) { + files[s.path] = s.content; + } + + // PUSH: replace inline references back + await replaceInlineScripts( + [failureModule], + async (path) => { + if (!(path in files)) throw new Error(`File not found: ${path}`); + return files[path]; + }, + noopLogger, + "/tmp/test/", + "/", + ); + + expect(failureModule.value.content).toBe(originalContent); + }); + + test("preprocessor_module content survives extract + replace", async () => { + const originalContent = "def main():\n return {\"preprocessed\": True}"; + const preprocessorModule = makeRawscriptModule( + "preprocessor", + originalContent, + "python3", + ); + + const extracted = extractInlineScripts( + [preprocessorModule], + {}, + "/", + "bun", + ); + expect(preprocessorModule.value.content).toStartWith("!inline "); + + const files: Record = {}; + for (const s of extracted) { + files[s.path] = s.content; + } + + await replaceInlineScripts( + [preprocessorModule], + async (path) => { + if (!(path in files)) throw new Error(`File not found: ${path}`); + return files[path]; + }, + noopLogger, + "/tmp/test/", + "/", + ); + + expect(preprocessorModule.value.content).toBe(originalContent); + }); + + test("failure_module with lock survives extract + replace", async () => { + const originalContent = "export function main() { return 42; }"; + const originalLock = "package-lock-contents-here"; + const mod = makeRawscriptModule( + "failure", + originalContent, + "bun", + originalLock, + ); + + const extracted = extractInlineScripts([mod], {}, "/", "bun"); + + const files: Record = {}; + for (const s of extracted) { + files[s.path] = s.content; + } + + await replaceInlineScripts( + [mod], + async (path) => { + if (!(path in files)) throw new Error(`File not found: ${path}`); + return files[path]; + }, + noopLogger, + "/tmp/test/", + "/", + ); + + expect(mod.value.content).toBe(originalContent); + expect((mod.value as any).lock).toBe(originalLock); + }); + + test("full flow with modules + failure_module + preprocessor_module round-trips", async () => { + const regularContent = "export function main() { return 'step1'; }"; + const failureContent = "export function main(e: any) { return e; }"; + const preprocessorContent = "def main():\n pass"; + + const modules = [makeRawscriptModule("a", regularContent, "bun")]; + const failureModule = makeRawscriptModule("failure", failureContent, "bun"); + const preprocessorModule = makeRawscriptModule("preprocessor", preprocessorContent, "python3"); + + // Extract all (mimicking sync.ts pull logic) + const allExtracted = [ + ...extractInlineScripts(modules, {}, "/", "bun"), + ...extractInlineScripts([failureModule], {}, "/", "bun"), + ...extractInlineScripts([preprocessorModule], {}, "/", "bun"), + ]; + + // All modules should now have !inline references + expect(modules[0].value.content).toStartWith("!inline "); + expect(failureModule.value.content).toStartWith("!inline "); + expect(preprocessorModule.value.content).toStartWith("!inline "); + + // All paths should be unique + const paths = allExtracted.filter((s) => !s.is_lock).map((s) => s.path); + expect(new Set(paths).size).toBe(paths.length); + + // Build filesystem + const files: Record = {}; + for (const s of allExtracted) { + files[s.path] = s.content; + } + + const fileReader = async (path: string) => { + if (!(path in files)) throw new Error(`File not found: ${path}`); + return files[path]; + }; + + // Replace all (mimicking flow.ts push logic) + await replaceInlineScripts(modules, fileReader, noopLogger, "/tmp/", "/"); + await replaceInlineScripts([failureModule], fileReader, noopLogger, "/tmp/", "/"); + await replaceInlineScripts([preprocessorModule], fileReader, noopLogger, "/tmp/", "/"); + + expect(modules[0].value.content).toBe(regularContent); + expect(failureModule.value.content).toBe(failureContent); + expect(preprocessorModule.value.content).toBe(preprocessorContent); + }); +}); + +// --------------------------------------------------------------------------- +// extractCurrentMapping +// --------------------------------------------------------------------------- + +describe("extractCurrentMapping for failure_module / preprocessor_module", () => { + test("extracts mapping from failure_module via optional param", () => { + const failureModule: FlowModule = makeRawscriptModule( + "failure", + "!inline failure.inline_script.ts", + "bun", + ); + + const mapping = extractCurrentMapping( + undefined, + {}, + failureModule, + undefined, + ); + + expect(mapping["failure"]).toBe("failure.inline_script.ts"); + }); + + test("extracts mapping from preprocessor_module via optional param", () => { + const preprocessorModule: FlowModule = makeRawscriptModule( + "preprocessor", + "!inline preprocessor.inline_script.py", + "python3", + ); + + const mapping = extractCurrentMapping( + undefined, + {}, + undefined, + preprocessorModule, + ); + + expect(mapping["preprocessor"]).toBe("preprocessor.inline_script.py"); + }); + + test("extracts mapping from modules + failure + preprocessor combined", () => { + const modules: FlowModule[] = [ + makeRawscriptModule("a", "!inline a.inline_script.ts", "bun"), + ]; + const failureModule = makeRawscriptModule( + "failure", + "!inline failure.inline_script.ts", + "bun", + ); + const preprocessorModule = makeRawscriptModule( + "preprocessor", + "!inline preprocessor.inline_script.py", + "python3", + ); + + const mapping = extractCurrentMapping( + modules, + {}, + failureModule, + preprocessorModule, + ); + + expect(mapping["a"]).toBe("a.inline_script.ts"); + expect(mapping["failure"]).toBe("failure.inline_script.ts"); + expect(mapping["preprocessor"]).toBe("preprocessor.inline_script.py"); + }); + + test("ignores non-inline content in failure_module", () => { + const failureModule = makeRawscriptModule( + "failure", + "export function main() {}", + "bun", + ); + + const mapping = extractCurrentMapping( + undefined, + {}, + failureModule, + undefined, + ); + + expect(mapping["failure"]).toBeUndefined(); + }); +}); diff --git a/cli/windmill-utils-internal/src/inline-scripts/extractor.ts b/cli/windmill-utils-internal/src/inline-scripts/extractor.ts index df9a1db08b..a3572ce7eb 100644 --- a/cli/windmill-utils-internal/src/inline-scripts/extractor.ts +++ b/cli/windmill-utils-internal/src/inline-scripts/extractor.ts @@ -150,8 +150,17 @@ export function extractInlineScripts( */ export function extractCurrentMapping( modules: FlowModule[] | undefined, - mapping: Record = {} + mapping: Record = {}, + failureModule?: FlowModule, + preprocessorModule?: FlowModule, ): Record { + if (failureModule) { + extractCurrentMapping([failureModule], mapping); + } + if (preprocessorModule) { + extractCurrentMapping([preprocessorModule], mapping); + } + if (!modules || !Array.isArray(modules)) { return mapping; }