From 38b6099b4c5039232cf306a01ee814b3ec5dc04c Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Fri, 31 Jul 2026 11:42:43 +0200 Subject: [PATCH] fix: apply default workspace dependencies to raw app runnables (#10427) * fix: apply default workspace dependencies to raw app runnables Co-Authored-By: Claude Opus 5 (1M context) * test: state the tree-path constraint in the raw app deps regression test Co-Authored-By: Claude Opus 5 (1M context) * fix: honor defaultTs when loading raw app runnables for lock generation Co-Authored-By: Claude Opus 5 (1M context) * fix: resolve defaultTs from wmill.yaml at every raw app runnable read Co-Authored-By: Claude Opus 5 (1M context) * refactor: thread defaultTs from command entry instead of re-reading the config Co-Authored-By: Claude Opus 5 (1M context) * refactor: give pushObj an options object for its optional arguments Co-Authored-By: Claude Opus 5 (1M context) * docs: reattach the pushObj JSDoc after the options-object refactor Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) --- cli/src/commands/app/app.ts | 11 ++- cli/src/commands/app/app_metadata.ts | 39 ++++----- cli/src/commands/app/dev.ts | 15 +++- cli/src/commands/app/raw_apps.ts | 19 ++++- cli/src/commands/sync/sync.ts | 38 +++++---- cli/src/types.ts | 35 +++++--- cli/test/raw_app_workspace_deps_unit.test.ts | 84 ++++++++++++++++++++ 7 files changed, 187 insertions(+), 54 deletions(-) create mode 100644 cli/test/raw_app_workspace_deps_unit.test.ts diff --git a/cli/src/commands/app/app.ts b/cli/src/commands/app/app.ts index 6f0263c5cf..f597398a95 100644 --- a/cli/src/commands/app/app.ts +++ b/cli/src/commands/app/app.ts @@ -12,7 +12,7 @@ import * as wmill from "../../../gen/services.gen.ts"; import { ListableApp, Policy } from "../../../gen/types.gen.ts"; import { GlobalOptions, isSuperset } from "../../types.ts"; -import { getWmillYamlPath } from "../../core/conf.ts"; +import { getWmillYamlPath, mergeConfigWithConfigFile } from "../../core/conf.ts"; import { readInlinePathSync } from "../../utils/utils.ts"; import devCommand from "./dev.ts"; import lintCommand from "./lint.ts"; @@ -402,7 +402,14 @@ async function push( if (isRawAppByName || hasRawAppYaml) { const { pushRawApp } = await import("./raw_apps.ts"); - await pushRawApp(workspace.workspaceId, remotePath, absoluteFilePath); + const merged = await mergeConfigWithConfigFile(opts); + await pushRawApp( + workspace.workspaceId, + remotePath, + absoluteFilePath, + undefined, + merged.defaultTs, + ); log.info(colors.bold.underline.green("Raw app pushed")); } else { await pushApp(workspace.workspaceId, remotePath, absoluteFilePath); diff --git a/cli/src/commands/app/app_metadata.ts b/cli/src/commands/app/app_metadata.ts index 10e5373741..f4c75ec1f1 100644 --- a/cli/src/commands/app/app_metadata.ts +++ b/cli/src/commands/app/app_metadata.ts @@ -187,7 +187,20 @@ export async function generateAppLocksInternal( ); const appFile = (await yamlParseFile(appFilePath)) as AppFile; - const appValue = rawApp ? (appFile as RawAppFile).runnables : (appFile as NormalAppFile).value; + // Raw-app runnables live in their own files under backend/; raw_app.yaml only + // carries them in the legacy layout. Resolve them here, before any workspace + // dependency filtering, otherwise the deps of a file-based raw app are + // filtered against an empty runnables map and silently dropped. + const runnablesFolder = path.join(appFolder, APP_BACKEND_FOLDER); + let rawAppRunnables: Record = {}; + if (rawApp) { + rawAppRunnables = await loadRunnablesFromBackend(runnablesFolder, opts.defaultTs); + if (Object.keys(rawAppRunnables).length === 0) { + rawAppRunnables = (appFile as RawAppFile).runnables ?? {}; + } + } + + const appValue = rawApp ? rawAppRunnables : (appFile as NormalAppFile).value; const folderNormalized = appFolder.replaceAll(SEP, "/"); let filteredDeps: Record = {}; @@ -199,15 +212,7 @@ export async function generateAppLocksInternal( const hashes = await generateAppHash({}, appFolder, rawApp, opts.defaultTs); const isDirectlyStale = await isAppDirectlyStale(appFolder, hashes, conf); - // For raw apps in new format, runnables are in separate files under backend/ - let treeAppValue = structuredClone(appValue); - if (rawApp) { - const runnablesPath = path.join(appFolder, APP_BACKEND_FOLDER); - const runnablesFromFiles = await loadRunnablesFromBackend(runnablesPath); - if (Object.keys(runnablesFromFiles).length > 0) { - treeAppValue = runnablesFromFiles; - } - } + const treeAppValue = structuredClone(appValue); // First pass: add inline scripts as separate nodes, then add app node importing them const inlineScriptPaths: string[] = []; @@ -303,23 +308,13 @@ export async function generateAppLocksInternal( } if (rawApp) { - const runnablesPath = path.join(appFolder, APP_BACKEND_FOLDER); - - // Load runnables from separate files (new format) or fall back to raw_app.yaml (old format) - const rawAppFile = appFile as RawAppFile; - let runnables = await loadRunnablesFromBackend(runnablesPath); - if (Object.keys(runnables).length === 0 && rawAppFile.runnables) { - // Fall back to old format - runnables = rawAppFile.runnables; - } - // Replace inline scripts for changed runnables - replaceInlineScripts(runnables, runnablesPath + SEP, false); + replaceInlineScripts(rawAppRunnables, runnablesFolder + SEP, false); // Update the app runnables with new locks (writes to separate files) updatedScripts = await updateRawAppRunnables( workspace, - runnables, + rawAppRunnables, remote_path, appFolder, filteredDeps, diff --git a/cli/src/commands/app/dev.ts b/cli/src/commands/app/dev.ts index 1842e1d007..c4206403c6 100644 --- a/cli/src/commands/app/dev.ts +++ b/cli/src/commands/app/dev.ts @@ -49,6 +49,9 @@ import { loadNonDottedPathsSetting, } from "../../utils/resource_folders.ts"; +// Resolved once per `wmill app dev` run from wmill.yaml; a bare `.ts` under +// backend/ denotes this runtime, so readers must agree with the path assigner. +let defaultTs: "bun" | "deno" = "bun"; const DEFAULT_PORT = 4000; const DEFAULT_HOST = "localhost"; @@ -406,6 +409,7 @@ async function dev(opts: DevOptions, appFolder?: string) { : originalCwd; const relAppFolder = path.relative(workspaceRoot, targetDir) || "."; const mergedOpts = await mergeConfigWithConfigFile(opts); + defaultTs = mergedOpts.defaultTs ?? "bun"; const codebases = await listSyncCodebases(mergedOpts); const { buildPreviewTempScriptRefs } = await import( "../generate-metadata/generate-metadata.ts" @@ -491,7 +495,10 @@ async function dev(opts: DevOptions, appFolder?: string) { // change to trigger the watcher). const inferredSchemas: Record = {}; try { - Object.assign(inferredSchemas, await inferAllInlineSchemas(process.cwd())); + Object.assign( + inferredSchemas, + await inferAllInlineSchemas(process.cwd(), defaultTs), + ); } catch (err: any) { log.warn( colors.yellow( @@ -508,6 +515,7 @@ async function dev(opts: DevOptions, appFolder?: string) { try { const initialRunnables = await loadRunnablesFromBackend( path.join(process.cwd(), APP_BACKEND_FOLDER), + defaultTs, ); Object.assign( pathRunnableSchemas, @@ -675,6 +683,7 @@ async function dev(opts: DevOptions, appFolder?: string) { const result = await inferRunnableSchemaFromFile( process.cwd(), relativeToRunnables, + defaultTs, ); if (result) { // Store inferred schema in memory @@ -1437,7 +1446,7 @@ async function genRunnablesTs( const backendPath = path.join(localPath, APP_BACKEND_FOLDER); // Load runnables from separate files (new format) or fall back to raw_app.yaml (old format) - let runnables = await loadRunnablesFromBackend(backendPath); + let runnables = await loadRunnablesFromBackend(backendPath, defaultTs); if (Object.keys(runnables).length === 0) { // Fall back to old format @@ -1581,7 +1590,7 @@ async function loadRunnables(): Promise> { const backendPath = path.join(localPath, APP_BACKEND_FOLDER); // Load runnables from separate files (new format) or fall back to raw_app.yaml (old format) - let runnables = await loadRunnablesFromBackend(backendPath); + let runnables = await loadRunnablesFromBackend(backendPath, defaultTs); if (Object.keys(runnables).length === 0) { // Fall back to old format diff --git a/cli/src/commands/app/raw_apps.ts b/cli/src/commands/app/raw_apps.ts index fcfc68915e..93c2314b12 100644 --- a/cli/src/commands/app/raw_apps.ts +++ b/cli/src/commands/app/raw_apps.ts @@ -1,5 +1,6 @@ import { requireLogin } from "../../core/auth.ts"; import { resolveWorkspace, validatePath } from "../../core/context.ts"; +import { mergeConfigWithConfigFile } from "../../core/conf.ts"; import { colors } from "@cliffy/ansi/colors"; import * as log from "../../core/log.ts"; import { sep as SEP } from "node:path"; @@ -143,7 +144,8 @@ function getRunnableIdFromCodeFile(fileName: string): string | undefined { * Returns an empty object if the backend folder doesn't exist. * * @param backendPath - Path to the backend folder - * @param defaultTs - Default TypeScript runtime ("bun" or "deno") + * @param defaultTs - TypeScript runtime a bare `.ts` denotes. Must match what + * newRawAppPathAssigner used to write the file, or the round-trip relabels it. */ export async function loadRunnablesFromBackend( backendPath: string, @@ -344,6 +346,7 @@ export async function pushRawApp( remotePath: string, localPath: string, message?: string, + defaultTs: "bun" | "deno" = "bun", ): Promise { if (alreadySynced.includes(localPath)) { return; @@ -377,7 +380,10 @@ export async function pushRawApp( // Load runnables from separate YAML files in the backend folder // Falls back to reading from raw_app.yaml if no separate files exist (backward compat) const backendPath = path.join(localPath, APP_BACKEND_FOLDER); - const runnablesFromBackend = await loadRunnablesFromBackend(backendPath); + const runnablesFromBackend = await loadRunnablesFromBackend( + backendPath, + defaultTs, + ); let runnables: Record; if (Object.keys(runnablesFromBackend).length > 0) { @@ -539,7 +545,14 @@ async function pushRawAppCommand( } const workspace = await resolveWorkspace(opts); await requireLogin(opts); + const merged = await mergeConfigWithConfigFile(opts); - await pushRawApp(workspace.workspaceId, remotePath, filePath); + await pushRawApp( + workspace.workspaceId, + remotePath, + filePath, + undefined, + merged.defaultTs, + ); log.info(colors.bold.underline.green("Raw app pushed")); } diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index 3e6dd464fc..bbe0e46d6a 100644 --- a/cli/src/commands/sync/sync.ts +++ b/cli/src/commands/sync/sync.ts @@ -4628,13 +4628,17 @@ export async function push( newObj, opts.plainSecrets ?? false, alreadySynced, - opts.message, - originalWorkspaceSpecificPath, - permissionedAsContext, - isWsSpecific ? true : undefined, { - noninteractive: (opts.yes ?? false) || !process.stdin.isTTY, - skipReencrypt: opts.skipReencryptOnKeyChange, + message: opts.message, + originalLocalPath: originalWorkspaceSpecificPath, + permissionedAsContext, + wsSpecific: isWsSpecific ? true : undefined, + keyPushOpts: { + noninteractive: + (opts.yes ?? false) || !process.stdin.isTTY, + skipReencrypt: opts.skipReencryptOnKeyChange, + }, + defaultTs: opts.defaultTs, }, ); @@ -4754,13 +4758,17 @@ export async function push( obj, opts.plainSecrets ?? false, [], - opts.message, - localFilePath, // Pass the actual local file path - permissionedAsContext, - isAddedWsSpecific ? true : undefined, { - noninteractive: (opts.yes ?? false) || !process.stdin.isTTY, - skipReencrypt: opts.skipReencryptOnKeyChange, + message: opts.message, + originalLocalPath: localFilePath, + permissionedAsContext, + wsSpecific: isAddedWsSpecific ? true : undefined, + keyPushOpts: { + noninteractive: + (opts.yes ?? false) || !process.stdin.isTTY, + skipReencrypt: opts.skipReencryptOnKeyChange, + }, + defaultTs: opts.defaultTs, }, ); @@ -4871,7 +4879,7 @@ export async function push( undefined, opts.plainSecrets ?? false, alreadySynced, - opts.message, + { message: opts.message }, ); } else { // Flow folder doesn't exist locally — delete on server @@ -4913,7 +4921,7 @@ export async function push( undefined, opts.plainSecrets ?? false, alreadySynced, - opts.message, + { message: opts.message }, ); } else { // App folder doesn't exist locally — delete on server @@ -4956,7 +4964,7 @@ export async function push( undefined, opts.plainSecrets ?? false, alreadySynced, - opts.message, + { message: opts.message, defaultTs: opts.defaultTs }, ); } else { // The entire raw app folder was deleted locally, diff --git a/cli/src/types.ts b/cli/src/types.ts index f0cd58abbb..5e4e596297 100644 --- a/cli/src/types.ts +++ b/cli/src/types.ts @@ -174,6 +174,21 @@ function redactString(s: string): string { return s.slice(0, 5) + "*".repeat(s.length - 5); } +export interface PushObjOptions { + /** Optional commit/update message */ + message?: string; + /** The original local file path (used for branch-specific resource file resolution) */ + originalLocalPath?: string; + /** Identity to attribute the push to, for the types that carry one */ + permissionedAsContext?: PermissionedAsContext; + /** Whether the item is workspace-specific */ + wsSpecific?: boolean; + /** encryption_key push: non-interactive flag and explicit re-encryption choice */ + keyPushOpts?: PushWorkspaceKeyOptions; + /** TypeScript runtime a bare `.ts` denotes, for raw-app runnables */ + defaultTs?: "bun" | "deno"; +} + /** * Pushes an object to the workspace server based on its type * @param workspace - The workspace ID to push to @@ -182,9 +197,7 @@ function redactString(s: string): string { * @param newObj - The new object state to push * @param plainSecrets - Whether to store secrets in plain text * @param alreadySynced - Array to track already synced items - * @param message - Optional commit/update message - * @param originalLocalPath - The original local file path (used for branch-specific resource file resolution) - * @param keyPushOpts - Options for the encryption_key push: non-interactive flag and explicit re-encryption choice + * @param opts - Per-type extras; see PushObjOptions */ export async function pushObj( workspace: string, @@ -193,12 +206,16 @@ export async function pushObj( newObj: any, plainSecrets: boolean, alreadySynced: string[], - message?: string, - originalLocalPath?: string, - permissionedAsContext?: PermissionedAsContext, - wsSpecific?: boolean, - keyPushOpts?: PushWorkspaceKeyOptions, + opts: PushObjOptions = {}, ) { + const { + message, + originalLocalPath, + permissionedAsContext, + wsSpecific, + keyPushOpts, + defaultTs, + } = opts; const typeEnding = getTypeStrFromPath(p); if (typeEnding === "app") { @@ -212,7 +229,7 @@ export async function pushObj( if (!rawAppName) { throw new Error(`Could not extract raw app name from path: ${p}`); } - await pushRawApp(workspace, rawAppName, buildFolderPath(rawAppName, "raw_app"), message); + await pushRawApp(workspace, rawAppName, buildFolderPath(rawAppName, "raw_app"), message, defaultTs); } else if (typeEnding === "folder") { await pushFolder(workspace, p, befObj, newObj); } else if (typeEnding === "variable") { diff --git a/cli/test/raw_app_workspace_deps_unit.test.ts b/cli/test/raw_app_workspace_deps_unit.test.ts new file mode 100644 index 0000000000..a76064e8b9 --- /dev/null +++ b/cli/test/raw_app_workspace_deps_unit.test.ts @@ -0,0 +1,84 @@ +/** + * Raw app workspace dependencies + * + * A raw app keeps its runnables in `backend/`, not in `raw_app.yaml`. The + * workspace dependency filtering must resolve those files, otherwise the + * default `dependencies/package.json` is dropped and locks are regenerated + * against unpinned versions. + * + * Exercised through the legacy (tree-less) path: tree mode sources its deps + * from `getMismatchedWorkspaceDeps()`, which is only populated by an + * `uploadScripts` round-trip, so it cannot run offline. Both paths filter the + * same `appValue`, so resolving it correctly is what this pins. + */ + +import { expect, test } from "bun:test"; +import * as path from "node:path"; +import os from "node:os"; +import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises"; +import { generateAppLocksInternal } from "../src/commands/app/app_metadata.ts"; +import { Workspace } from "../src/commands/workspace/workspace.ts"; + +const stubWorkspace: Workspace = { + remote: "http://localhost:0/", + workspaceId: "test", + name: "test", + token: "test", +}; + +const APP_FOLDER = path.join("f", "example.raw_app"); + +async function withTempDir(fn: (tempDir: string) => Promise): Promise { + const tempDir = await mkdtemp(path.join(os.tmpdir(), "wmill_raw_app_deps_")); + const originalCwd = process.cwd(); + try { + process.chdir(tempDir); + await fn(tempDir); + } finally { + process.chdir(originalCwd); + await rm(tempDir, { recursive: true, force: true }); + } +} + +test("raw app: default workspace deps are picked up from backend runnables", async () => { + await withTempDir(async () => { + await mkdir(path.join(APP_FOLDER, "backend"), { recursive: true }); + await writeFile( + path.join(APP_FOLDER, "raw_app.yaml"), + `summary: "example raw app"\npolicy:\n execution_mode: publisher\n triggerables: {}\n`, + "utf-8", + ); + await writeFile( + path.join(APP_FOLDER, "backend", "test.ts"), + `import * as wmill from "windmill-client"\n\nexport async function main() {\n return wmill.getVariable("example")\n}\n`, + "utf-8", + ); + + await generateAppLocksInternal( + APP_FOLDER, + true, + false, + stubWorkspace, + { defaultTs: "bun" }, + true, // justUpdateMetadataLock — no backend round-trip + true, + ); + + expect( + await generateAppLocksInternal(APP_FOLDER, true, true, stubWorkspace, { defaultTs: "bun" }, false, true), + ).toBeUndefined(); + + // The runnable has no `package_json` annotation, so it uses the default + // manifest — adding it must invalidate the app. + await mkdir("dependencies", { recursive: true }); + await writeFile( + path.join("dependencies", "package.json"), + `{"dependencies": {"windmill-client": "1.742.0"}}`, + "utf-8", + ); + + expect( + await generateAppLocksInternal(APP_FOLDER, true, true, stubWorkspace, { defaultTs: "bun" }, false, true), + ).toEqual("f/example.raw_app"); + }); +});