From be1b4100dd8631ff8b896320d0a5832c93e2c77e Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sat, 27 Dec 2025 00:17:36 +0000 Subject: [PATCH] nonDottedPaths to avoid using .inline_script. --- cli/src/commands/app/app_metadata.ts | 12 +- cli/src/commands/script/script.ts | 21 +++- cli/src/commands/sync/sync.ts | 5 +- cli/src/utils/resource_folders.ts | 30 +++++ cli/test/sync_pull_push.test.ts | 119 ++++++++++++++++++ .../src/inline-scripts/extractor.ts | 14 ++- .../src/path-utils/path-assigner.ts | 22 +++- 7 files changed, 213 insertions(+), 10 deletions(-) diff --git a/cli/src/commands/app/app_metadata.ts b/cli/src/commands/app/app_metadata.ts index a37795470f..078ecc1e1a 100644 --- a/cli/src/commands/app/app_metadata.ts +++ b/cli/src/commands/app/app_metadata.ts @@ -38,6 +38,7 @@ import { import { mergeConfigWithConfigFile, SyncOptions } from "../../core/conf.ts"; import { resolveWorkspace } from "../../core/context.ts"; import { requireLogin } from "../../core/auth.ts"; +import { getNonDottedPaths } from "../../utils/resource_folders.ts"; const TOP_HASH = "__app_hash"; export const APP_BACKEND_FOLDER = "backend"; @@ -62,8 +63,13 @@ async function generateAppHash( try { const elems = await FSFSElement(runnablesFolder, [], true); for await (const f of elems.getChildren()) { - if (!rawApp && !f.path.includes(".inline_script.")) { - continue; + // For normal apps, skip non-script files (metadata files like app.yaml) + // For raw apps, all files in backend/ are scripts + if (!rawApp) { + const isMetadataFile = f.path.endsWith("app.yaml") || f.path.endsWith("app.json"); + if (isMetadataFile) { + continue; + } } if (exts.some((e) => f.path.endsWith(e))) { // Embed lock into hash @@ -442,7 +448,7 @@ async function updateAppInlineScripts( rawDeps?: Record, defaultTs: "bun" | "deno" = "bun" ): Promise { - const pathAssigner = newPathAssigner(defaultTs); + const pathAssigner = newPathAssigner(defaultTs, { skipInlineScriptSuffix: getNonDottedPaths() }); const processor: InlineScriptProcessor = async (inlineScript, context) => { const language = inlineScript.language as SupportedLanguage; diff --git a/cli/src/commands/script/script.ts b/cli/src/commands/script/script.ts index 97e2c09b45..7a0b61618d 100644 --- a/cli/src/commands/script/script.ts +++ b/cli/src/commands/script/script.ts @@ -57,6 +57,8 @@ import { execSync } from "node:child_process"; import { NewScript, Script } from "../../../gen/types.gen.ts"; import { isRawAppBackendPath as isRawAppBackendPathInternal, + isAppInlineScriptPath as isAppInlineScriptPathInternal, + isFlowInlineScriptPath as isFlowInlineScriptPathInternal, isFlowPath, isAppPath, } from "../../utils/resource_folders.ts"; @@ -79,6 +81,22 @@ export function isRawAppBackendPath(filePath: string): boolean { return isRawAppBackendPathInternal(filePath); } +/** + * Checks if a path is inside a normal app folder (inline script). + * Matches patterns like: .../myApp.app/... or .../myApp__app/... + */ +export function isAppInlineScriptPath(filePath: string): boolean { + return isAppInlineScriptPathInternal(filePath); +} + +/** + * Checks if a path is inside a flow folder (inline script). + * Matches patterns like: .../myFlow.flow/... or .../myFlow__flow/... + */ +export function isFlowInlineScriptPath(filePath: string): boolean { + return isFlowInlineScriptPathInternal(filePath); +} + type PushOptions = GlobalOptions; async function push(opts: PushOptions, filePath: string) { opts = await mergeConfigWithConfigFile(opts); @@ -211,7 +229,8 @@ export async function handleFile( codebases: SyncCodebase[] ): Promise { if ( - !path.includes(".inline_script.") && + !isAppInlineScriptPath(path) && + !isFlowInlineScriptPath(path) && !isRawAppBackendPath(path) && exts.some((exts) => path.endsWith(exts)) ) { diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index f139fc683e..753f871296 100644 --- a/cli/src/commands/sync/sync.ts +++ b/cli/src/commands/sync/sync.ts @@ -93,6 +93,7 @@ import { transformJsonPathToDir, getFolderSuffix, getFolderSuffixWithSep, + getNonDottedPaths, } from "../../utils/resource_folders.ts"; // Merge CLI options with effective settings, preserving CLI flags as overrides @@ -770,6 +771,8 @@ function ZipFSElement( {}, SEP, defaultTs, + undefined, // pathAssigner - let it create one + { skipInlineScriptSuffix: getNonDottedPaths() }, ); } catch (error) { log.error( @@ -811,7 +814,7 @@ function ZipFSElement( inlineScripts = extractInlineScriptsForApps( undefined, app?.["value"], - newPathAssigner(defaultTs), + newPathAssigner(defaultTs, { skipInlineScriptSuffix: getNonDottedPaths() }), (_, val) => val["name"], false, ); diff --git a/cli/src/utils/resource_folders.ts b/cli/src/utils/resource_folders.ts index 8f12da2af0..96beb91635 100644 --- a/cli/src/utils/resource_folders.ts +++ b/cli/src/utils/resource_folders.ts @@ -158,6 +158,36 @@ export function isRawAppBackendPath(filePath: string): boolean { return pattern.test(normalizedPath); } +/** + * Check if a path is inside a normal app folder (inline script). + * Matches patterns like: .../myApp.app/... or .../myApp__app/... + * This is used to detect inline scripts that belong to normal apps. + */ +export function isAppInlineScriptPath(filePath: string): boolean { + const suffixes = getFolderSuffixes(); + // Normalize path separators for consistent matching + const normalizedPath = filePath.replaceAll(SEP, "/"); + // Check if path contains pattern: *.[suffix]/ + const escapedSuffix = suffixes.app.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const pattern = new RegExp(`${escapedSuffix}/`); + return pattern.test(normalizedPath); +} + +/** + * Check if a path is inside a flow folder (inline script). + * Matches patterns like: .../myFlow.flow/... or .../myFlow__flow/... + * This is used to detect inline scripts that belong to flows. + */ +export function isFlowInlineScriptPath(filePath: string): boolean { + const suffixes = getFolderSuffixes(); + // Normalize path separators for consistent matching + const normalizedPath = filePath.replaceAll(SEP, "/"); + // Check if path contains pattern: *.[suffix]/ + const escapedSuffix = suffixes.flow.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const pattern = new RegExp(`${escapedSuffix}/`); + return pattern.test(normalizedPath); +} + // ============================================================================ // Path Manipulation Functions // ============================================================================ diff --git a/cli/test/sync_pull_push.test.ts b/cli/test/sync_pull_push.test.ts index e8409db8c7..36e48ac91e 100644 --- a/cli/test/sync_pull_push.test.ts +++ b/cli/test/sync_pull_push.test.ts @@ -26,7 +26,11 @@ import { isAppMetadataFile, isRawAppMetadataFile, transformJsonPathToDir, + isAppInlineScriptPath, + isFlowInlineScriptPath, + isRawAppBackendPath, } from "../src/utils/resource_folders.ts"; +import { newPathAssigner } from "../windmill-utils-internal/src/path-utils/path-assigner.ts"; // ============================================================================= // Test Fixtures - Every Type of Windmill Resource @@ -602,6 +606,121 @@ Deno.test("getFolderSuffix returns correct suffix based on nonDottedPaths settin } }); +Deno.test("newPathAssigner with skipInlineScriptSuffix removes .inline_script. from paths", () => { + // Test default behavior (with .inline_script. suffix) + const defaultAssigner = newPathAssigner("bun"); + const [defaultPath, defaultExt] = defaultAssigner.assignPath("my_script", "bun"); + assertEquals(defaultPath, "my_script.inline_script."); + assertEquals(defaultExt, "ts"); + + // Test with skipInlineScriptSuffix = false (explicit) + const withSuffixAssigner = newPathAssigner("bun", { skipInlineScriptSuffix: false }); + const [withSuffixPath, withSuffixExt] = withSuffixAssigner.assignPath("another_script", "python3"); + assertEquals(withSuffixPath, "another_script.inline_script."); + assertEquals(withSuffixExt, "py"); + + // Test with skipInlineScriptSuffix = true (no .inline_script. suffix) + const noSuffixAssigner = newPathAssigner("bun", { skipInlineScriptSuffix: true }); + const [noSuffixPath, noSuffixExt] = noSuffixAssigner.assignPath("clean_script", "bun"); + assertEquals(noSuffixPath, "clean_script."); + assertEquals(noSuffixExt, "ts"); + + // Test with skipInlineScriptSuffix = true and different language + const noSuffixPyAssigner = newPathAssigner("bun", { skipInlineScriptSuffix: true }); + const [noSuffixPyPath, noSuffixPyExt] = noSuffixPyAssigner.assignPath("python_script", "python3"); + assertEquals(noSuffixPyPath, "python_script."); + assertEquals(noSuffixPyExt, "py"); +}); + +Deno.test("newPathAssigner generates unique paths for duplicate names", () => { + const assigner = newPathAssigner("bun", { skipInlineScriptSuffix: true }); + + // First script + const [path1, ext1] = assigner.assignPath("my_script", "bun"); + assertEquals(path1, "my_script."); + assertEquals(ext1, "ts"); + + // Second script with same name should get counter + const [path2, ext2] = assigner.assignPath("my_script", "bun"); + assertEquals(path2, "my_script_1."); + assertEquals(ext2, "ts"); + + // Third script with same name should get incremented counter + const [path3, ext3] = assigner.assignPath("my_script", "python3"); + assertEquals(path3, "my_script_2."); + assertEquals(ext3, "py"); +}); + +Deno.test("isAppInlineScriptPath detects app inline scripts correctly", () => { + // Store original value + const wasNonDotted = getNonDottedPaths(); + + try { + // Test with dotted paths (default) + setNonDottedPaths(false); + assert(isAppInlineScriptPath("f/my_app.app/my_script.ts"), "Should detect script in .app folder"); + assert(isAppInlineScriptPath("f/my_app.app/app.yaml"), "Should detect metadata in .app folder"); + assert(!isAppInlineScriptPath("f/my_script.ts"), "Should not detect standalone script"); + assert(!isAppInlineScriptPath("f/my_flow.flow/flow.yaml"), "Should not detect flow files"); + + // Test with non-dotted paths + setNonDottedPaths(true); + assert(isAppInlineScriptPath("f/my_app__app/my_script.ts"), "Should detect script in __app folder"); + assert(isAppInlineScriptPath("f/my_app__app/app.yaml"), "Should detect metadata in __app folder"); + assert(!isAppInlineScriptPath("f/my_script.ts"), "Should not detect standalone script"); + assert(!isAppInlineScriptPath("f/my_flow__flow/flow.yaml"), "Should not detect flow files"); + } finally { + // Restore original value + setNonDottedPaths(wasNonDotted); + } +}); + +Deno.test("isFlowInlineScriptPath detects flow inline scripts correctly", () => { + // Store original value + const wasNonDotted = getNonDottedPaths(); + + try { + // Test with dotted paths (default) + setNonDottedPaths(false); + assert(isFlowInlineScriptPath("f/my_flow.flow/my_script.ts"), "Should detect script in .flow folder"); + assert(isFlowInlineScriptPath("f/my_flow.flow/flow.yaml"), "Should detect metadata in .flow folder"); + assert(!isFlowInlineScriptPath("f/my_script.ts"), "Should not detect standalone script"); + assert(!isFlowInlineScriptPath("f/my_app.app/app.yaml"), "Should not detect app files"); + + // Test with non-dotted paths + setNonDottedPaths(true); + assert(isFlowInlineScriptPath("f/my_flow__flow/my_script.ts"), "Should detect script in __flow folder"); + assert(isFlowInlineScriptPath("f/my_flow__flow/flow.yaml"), "Should detect metadata in __flow folder"); + assert(!isFlowInlineScriptPath("f/my_script.ts"), "Should not detect standalone script"); + assert(!isFlowInlineScriptPath("f/my_app__app/app.yaml"), "Should not detect app files"); + } finally { + // Restore original value + setNonDottedPaths(wasNonDotted); + } +}); + +Deno.test("isRawAppBackendPath detects raw app backend paths correctly", () => { + // Store original value + const wasNonDotted = getNonDottedPaths(); + + try { + // Test with dotted paths (default) + setNonDottedPaths(false); + assert(isRawAppBackendPath("f/my_app.raw_app/backend/script.ts"), "Should detect script in .raw_app/backend"); + assert(!isRawAppBackendPath("f/my_app.raw_app/index.html"), "Should not detect root files in raw_app"); + assert(!isRawAppBackendPath("f/my_script.ts"), "Should not detect standalone script"); + + // Test with non-dotted paths + setNonDottedPaths(true); + assert(isRawAppBackendPath("f/my_app__raw_app/backend/script.ts"), "Should detect script in __raw_app/backend"); + assert(!isRawAppBackendPath("f/my_app__raw_app/index.html"), "Should not detect root files in raw_app"); + assert(!isRawAppBackendPath("f/my_script.ts"), "Should not detect standalone script"); + } finally { + // Restore original value + setNonDottedPaths(wasNonDotted); + } +}); + Deno.test("Script fixture creates valid structure", () => { const pythonScript = createScriptFixture("test_script", "python3"); diff --git a/cli/windmill-utils-internal/src/inline-scripts/extractor.ts b/cli/windmill-utils-internal/src/inline-scripts/extractor.ts index 0239301ccb..62cc81ba1f 100644 --- a/cli/windmill-utils-internal/src/inline-scripts/extractor.ts +++ b/cli/windmill-utils-internal/src/inline-scripts/extractor.ts @@ -11,6 +11,14 @@ interface InlineScript { content: string; } +/** + * Options for extractInlineScripts function + */ +export interface ExtractInlineScriptsOptions { + /** When true, skip the .inline_script. suffix in file names */ + skipInlineScriptSuffix?: boolean; +} + /** * Extracts inline scripts from flow modules, converting them to separate files * and replacing the original content with file references. @@ -20,6 +28,7 @@ interface InlineScript { * @param separator - Path separator to use * @param defaultTs - Default TypeScript runtime to use ("bun" or "deno") * @param pathAssigner - Optional path assigner to reuse (for nested calls) + * @param options - Optional configuration options * @returns Array of inline scripts with their paths and content */ export function extractInlineScripts( @@ -27,10 +36,11 @@ export function extractInlineScripts( mapping: Record = {}, separator: string = "/", defaultTs?: "bun" | "deno", - pathAssigner?: PathAssigner + pathAssigner?: PathAssigner, + options?: ExtractInlineScriptsOptions ): InlineScript[] { // Create pathAssigner only if not provided (top-level call), but reuse it for nested calls - const assigner = pathAssigner ?? newPathAssigner(defaultTs ?? "bun"); + const assigner = pathAssigner ?? newPathAssigner(defaultTs ?? "bun", { skipInlineScriptSuffix: options?.skipInlineScriptSuffix }); return modules.flatMap((m) => { if (m.value.type == "rawscript") { diff --git a/cli/windmill-utils-internal/src/path-utils/path-assigner.ts b/cli/windmill-utils-internal/src/path-utils/path-assigner.ts index d34dcb35db..4b67e06abf 100644 --- a/cli/windmill-utils-internal/src/path-utils/path-assigner.ts +++ b/cli/windmill-utils-internal/src/path-utils/path-assigner.ts @@ -113,13 +113,27 @@ export interface PathAssigner { assignPath(summary: string | undefined, language: SupportedLanguage): [string, string]; } +export interface PathAssignerOptions { + defaultTs: "bun" | "deno"; + /** When true, skip the .inline_script. suffix in file names */ + skipInlineScriptSuffix?: boolean; +} + /** * Creates a new path assigner for inline scripts. * * @param defaultTs - Default TypeScript runtime ("bun" or "deno") + * @param options - Optional configuration (can pass options object instead of defaultTs) * @returns Path assigner function */ -export function newPathAssigner(defaultTs: "bun" | "deno"): PathAssigner { +export function newPathAssigner(defaultTs: "bun" | "deno" | PathAssignerOptions, options?: { skipInlineScriptSuffix?: boolean }): PathAssigner { + // Handle both old signature (defaultTs string) and new signature (options object) + const resolvedOptions: PathAssignerOptions = typeof defaultTs === "object" + ? defaultTs + : { defaultTs, skipInlineScriptSuffix: options?.skipInlineScriptSuffix }; + + const { defaultTs: tsRuntime, skipInlineScriptSuffix } = resolvedOptions; + let counter = 0; const seen_names = new Set(); function assignPath( @@ -143,9 +157,11 @@ export function newPathAssigner(defaultTs: "bun" | "deno"): PathAssigner { } seen_names.add(name); - const ext = getLanguageExtension(language, defaultTs); + const ext = getLanguageExtension(language, tsRuntime); - return [`${name}.inline_script.`, ext]; + // When skipInlineScriptSuffix is true, don't add .inline_script. to the path + const suffix = skipInlineScriptSuffix ? "." : ".inline_script."; + return [`${name}${suffix}`, ext]; } return { assignPath }; }