From 1db2922e3855ac068cc0aabc5df8f0228f19a01d Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Fri, 1 May 2026 14:52:55 +0000 Subject: [PATCH] fix(cli): only preserve case for raw-app runnableIds, not app/flow summaries (#9000) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(cli): only preserve case for raw-app runnableIds, not app/flow summaries PR #8940 stopped lowercasing in sanitizeForFilesystem to fix #8939, where a raw-app runnableId like CamelCaseTSRunnable produced a CamelCase YAML metadata file but a lowercased code file, making them desync and register as duplicate runnables on push. That fix overshot. sanitizeForFilesystem is also reached by newPathAssigner, which serves normal apps and flows where the input is the script's human summary ("Get Users Data") rather than an identifier. There the on-disk filename is the only artifact — there's no companion YAML to keep in sync — so lowercasing was the right behavior. Removing it changed both the on-disk filename and the !inline reference in app.yaml / flow.yaml from get_users_data.inline_script.ts to Get_Users_Data.inline_script.ts on the next pull, surfacing as unwanted case churn for users updating to 1.693.x. Add a preserveCase option to sanitizeForFilesystem (default false → lowercase). newRawAppPathAssigner opts in; newPathAssigner stays on the default. Update unit tests accordingly and add an end-to-end raw-app round-trip in raw_app_sync.test.ts that pushes a CamelCase backend runnable, pulls it back, and asserts both YAML and code file preserve case with no lowercase orphan. Co-Authored-By: Claude Opus 4.7 (1M context) * test(cli): use readdir for exact-case orphan check on Windows The CamelCase round-trip test used fileExists("camelcasetsrunnable.ts") to assert no lowercase orphan was produced, which false-positives on Windows since the filesystem is case-insensitive and resolves the lookup to the existing CamelCaseTSRunnable.ts. Switch to readdir + toContain so the exact on-disk casing is compared identically on Linux and Windows. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- ..._scripts_failure_preprocessor_unit.test.ts | 6 +- cli/test/raw_app_sync.test.ts | 87 ++++++++++++++++++- cli/test/sync_pull_push.test.ts | 4 +- .../src/path-utils/path-assigner.ts | 12 ++- 4 files changed, 100 insertions(+), 9 deletions(-) diff --git a/cli/test/inline_scripts_failure_preprocessor_unit.test.ts b/cli/test/inline_scripts_failure_preprocessor_unit.test.ts index b9b35f0912..78af37a02e 100644 --- a/cli/test/inline_scripts_failure_preprocessor_unit.test.ts +++ b/cli/test/inline_scripts_failure_preprocessor_unit.test.ts @@ -524,7 +524,7 @@ describe("extractInlineScripts with mapping preserves file paths", () => { const contentScript = scripts.find((s) => !s.is_lock); // Should use assigner path based on summary, not mapped - expect(contentScript!.path).toContain("Get_Users_Data"); + expect(contentScript!.path).toContain("get_users_data"); }); test("mapped modules and unmapped modules coexist", () => { @@ -538,7 +538,7 @@ describe("extractInlineScripts with mapping preserves file paths", () => { const paths = scripts.filter((s) => !s.is_lock).map((s) => s.path); expect(paths[0]).toBe("my_custom_name.ts"); - expect(paths[1]).toContain("Step_B"); // assigner-generated from summary + expect(paths[1]).toContain("step_b"); // assigner-generated from summary }); test("lock path is derived from mapped content path", () => { @@ -560,7 +560,7 @@ describe("extractInlineScripts with mapping preserves file paths", () => { const scripts = extractInlineScripts([mod], {}, "/", "bun"); const lockScript = scripts.find((s) => s.is_lock); - expect(lockScript!.path).toContain("Get_Users_Data"); + expect(lockScript!.path).toContain("get_users_data"); expect(lockScript!.path).toEndWith(".lock"); }); diff --git a/cli/test/raw_app_sync.test.ts b/cli/test/raw_app_sync.test.ts index 95d812511f..ee20bc1947 100644 --- a/cli/test/raw_app_sync.test.ts +++ b/cli/test/raw_app_sync.test.ts @@ -2,7 +2,7 @@ import { expect, test } from "bun:test"; import { withTestBackend } from "./test_backend.ts"; import { addWorkspace } from "../workspace.ts"; import * as path from "node:path"; -import { writeFile, readFile, stat, rm, mkdir } from "node:fs/promises"; +import { writeFile, readFile, stat, rm, mkdir, readdir } from "node:fs/promises"; // ============================================================================= // RAW APP SYNC TESTS @@ -508,3 +508,88 @@ excludes: []`, "utf-8"); expect(hasRawApp).toBeTruthy(); }); }); + +test("Raw App: CamelCase backend runnableId round-trips without duplicates", async () => { + await withTestBackend(async (backend, tempDir) => { + const testWorkspace = { + remote: backend.baseUrl, + workspaceId: backend.workspace, + name: "raw_app_camelcase_test", + token: backend.token + }; + await addWorkspace(testWorkspace, { force: true, configDir: backend.testConfigDir }); + + await writeFile(`${tempDir}/wmill.yaml`, `defaultTs: bun +includes: + - "**" +excludes: []`, "utf-8"); + + const appDir = path.join(tempDir, "f", "test", "camelcase_app.raw_app"); + const backendDir = path.join(appDir, "backend"); + await mkdir(path.join(tempDir, "f", "test"), { recursive: true }); + await createRawAppOnDisk(appDir); + + // Add a backend runnable whose id contains uppercase letters. The YAML + // metadata file is named `${runnableId}.yaml` and must stay in sync with + // the code file's casing — the bug fixed here desynced them. + await mkdir(backendDir, { recursive: true }); + await writeFile(path.join(backendDir, "CamelCaseTSRunnable.yaml"), "type: inline\n", "utf-8"); + await writeFile( + path.join(backendDir, "CamelCaseTSRunnable.ts"), + `export async function main(x: number): Promise {\n return \`Result: \${x}\`;\n}\n`, + "utf-8" + ); + + const pushResult1 = await backend.runCLICommand( + ["sync", "push", "--yes"], + tempDir, "raw_app_camelcase_test" + ); + expect(pushResult1.code).toEqual(0); + + await rm(appDir, { recursive: true }); + + const pullResult = await backend.runCLICommand( + ["sync", "pull", "--yes"], + tempDir, "raw_app_camelcase_test" + ); + expect(pullResult.code).toEqual(0); + + // YAML and code file must both come back with the original case so + // loadRunnablesFromBackend pairs them as one runnable, not two. + // Use readdir for exact-case comparison: Windows is case-insensitive at + // the filesystem level, so fileExists("camelcasetsrunnable.ts") would + // resolve to CamelCaseTSRunnable.ts and false-positive the orphan check. + const backendEntries = await readdir(backendDir); + expect(backendEntries).toContain("CamelCaseTSRunnable.yaml"); + expect(backendEntries).toContain("CamelCaseTSRunnable.ts"); + expect(backendEntries).not.toContain("camelcasetsrunnable.ts"); + expect(backendEntries).not.toContain("camelcasetsrunnable.yaml"); + + const pulledContent = await readFileContent(path.join(backendDir, "CamelCaseTSRunnable.ts")); + expect(pulledContent).toContain("Result:"); + + // A second push must not surface a duplicate lowercase runnable: the + // dry-run change list should only mention the one app, no orphan code + // file getting registered as a separate inline runnable. + const dryRun = await backend.runCLICommand( + ["sync", "push", "--dry-run", "--json-output"], + tempDir, "raw_app_camelcase_test" + ); + expect(dryRun.code).toEqual(0); + + let jsonOutput: any = null; + try { + jsonOutput = JSON.parse(dryRun.stdout.trim()); + } catch { + const jsonMatch = dryRun.stdout.match(/\{[\s\S]*\}/); + if (jsonMatch) { + try { jsonOutput = JSON.parse(jsonMatch[0]); } catch { /* ignore */ } + } + } + expect(jsonOutput !== null).toBeTruthy(); + const changes = (jsonOutput.changes ?? []) as Array<{ path: string }>; + // No change should reference a lowercased runnable filename. + const lowercased = changes.filter((c) => c.path.includes("camelcasetsrunnable")); + expect(lowercased).toEqual([]); + }); +}); diff --git a/cli/test/sync_pull_push.test.ts b/cli/test/sync_pull_push.test.ts index 2efa6ac5a4..e64af246f2 100644 --- a/cli/test/sync_pull_push.test.ts +++ b/cli/test/sync_pull_push.test.ts @@ -631,10 +631,10 @@ test("newPathAssigner with skipInlineScriptSuffix removes .inline_script. from p expect(noSuffixPyExt).toEqual("py"); }); -test("path assigners preserve mixed-case summaries", () => { +test("newPathAssigner lowercases summaries; newRawAppPathAssigner preserves case", () => { const inlineAssigner = newPathAssigner("bun", { skipInlineScriptSuffix: true }); const [inlinePath] = inlineAssigner.assignPath("CamelCaseTSRunnable", "bun"); - expect(inlinePath).toEqual("CamelCaseTSRunnable."); + expect(inlinePath).toEqual("camelcasetsrunnable."); const rawAssigner = newRawAppPathAssigner("bun"); const [rawPath] = rawAssigner.assignPath("CamelCaseTSRunnable", "bun"); 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 90a70a82a8..94d9085b94 100644 --- a/cli/windmill-utils-internal/src/path-utils/path-assigner.ts +++ b/cli/windmill-utils-internal/src/path-utils/path-assigner.ts @@ -118,8 +118,11 @@ export function getLanguageFromExtension( */ const WINDOWS_RESERVED = /^(con|prn|aux|nul|com[0-9]|lpt[0-9])$/; -export function sanitizeForFilesystem(summary: string): string { - const name = summary +export function sanitizeForFilesystem( + summary: string, + options?: { preserveCase?: boolean }, +): string { + let name = summary .replaceAll(" ", "_") // Remove characters invalid on Windows/Unix/Mac: / \ : * ? " < > | // Also remove control characters (0x00-0x1F) and DEL (0x7F) @@ -129,6 +132,9 @@ export function sanitizeForFilesystem(summary: string): string { .replace(/_+/g, "_") // Trim leading/trailing dots and underscores (hidden files, Windows edge cases) .replace(/^[._]+|[._]+$/g, ""); + if (!options?.preserveCase) { + name = name.toLowerCase(); + } // Prefix Windows reserved device names (CON, PRN, AUX, NUL, COM0-9, LPT0-9) return WINDOWS_RESERVED.test(name.toLowerCase()) ? `_${name}` : name; } @@ -211,7 +217,7 @@ export function newRawAppPathAssigner(defaultTs: "bun" | "deno"): PathAssigner { ): [string, string] { let name; - name = summary ? sanitizeForFilesystem(summary) : ""; + name = summary ? sanitizeForFilesystem(summary, { preserveCase: true }) : ""; let original_name = name;