From 38acaa3653728bf9e0ae6f746edf433703b4ab63 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sat, 28 Mar 2026 10:46:01 +0000 Subject: [PATCH] =?UTF-8?q?fix(cli):=20fix=2013=20CLI=20bugs=20=E2=80=94?= =?UTF-8?q?=20exit=20codes,=20sync=20tar=20fallback,=20variable=20encrypti?= =?UTF-8?q?on,=20JSON=20output=20(#8582)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(cli): fix 13 CLI bugs — exit codes, sync tar fallback, variable encryption, JSON output, parent dirs Co-Authored-By: Claude Opus 4.6 (1M context) * fix(cli): address PR review — TarAsZip.folder(), retry timeout, stderr hint Co-Authored-By: Claude Opus 4.6 (1M context) * fix(cli): update resource-type list test to handle empty state message Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Claude Opus 4.6 (1M context) --- cli/src/commands/audit/audit.ts | 7 + cli/src/commands/flow/flow.ts | 46 +++++-- cli/src/commands/job/job.ts | 27 +++- .../commands/resource-type/resource-type.ts | 4 + cli/src/commands/resource/resource.ts | 4 +- cli/src/commands/schedule/schedule.ts | 5 +- cli/src/commands/script/script.ts | 45 +++++-- cli/src/commands/sync/pull.ts | 126 ++++++++++++++---- cli/src/commands/trigger/trigger.ts | 5 +- cli/src/commands/variable/variable.ts | 9 +- cli/src/commands/workspace/workspace.ts | 2 +- cli/src/core/context.ts | 2 +- cli/src/utils/resource_folders.ts | 2 +- cli/test/standalone_commands.test.ts | 6 +- 14 files changed, 232 insertions(+), 58 deletions(-) diff --git a/cli/src/commands/audit/audit.ts b/cli/src/commands/audit/audit.ts index 8c65413153..e0a85e89b6 100644 --- a/cli/src/commands/audit/audit.ts +++ b/cli/src/commands/audit/audit.ts @@ -42,6 +42,13 @@ async function list( log.info("No audit logs found."); return; } + if (logs.every((l) => l.operation === "redacted")) { + log.info(colors.yellow( + "Audit log details are not available on the Community Edition.\n" + + "Upgrade to the Enterprise Edition for full audit logging with operation details." + )); + return; + } new Table() .header(["ID", "Timestamp", "Username", "Operation", "Action", "Resource"]) .padding(2) diff --git a/cli/src/commands/flow/flow.ts b/cli/src/commands/flow/flow.ts index 231ac8c8b0..ad1a5e4d3f 100644 --- a/cli/src/commands/flow/flow.ts +++ b/cli/src/commands/flow/flow.ts @@ -252,6 +252,7 @@ async function list( } } async function get(opts: GlobalOptions & { json?: boolean }, path: string) { + if (opts.json) log.setSilent(true); const workspace = await resolveWorkspace(opts); await requireLogin(opts); const f = await wmill.getFlowByPath({ @@ -328,18 +329,39 @@ async function run( i++; } - if (!opts.silent) { - log.info(colors.green.underline.bold("Flow ran to completion")); - log.info("\n"); + // Wait for flow completion with retry (handles race when --silent skips module tracking) + const MAX_RETRIES = 600; // ~60 seconds at 100ms intervals + let retries = 0; + while (retries < MAX_RETRIES) { + try { + const jobInfo = await wmill.getCompletedJob({ + workspace: workspace.workspaceId, + id, + }); + + if (!opts.silent) { + log.info(colors.green.underline.bold("Flow ran to completion")); + log.info("\n"); + } + + if (jobInfo.success === false) { + process.exitCode = 1; + } + + if (opts.silent) { + console.log(JSON.stringify(jobInfo.result ?? {})); + } else { + log.info(JSON.stringify(jobInfo.result ?? {}, null, 2)); + } + + break; + } catch { + retries++; + await new Promise((resolve) => setTimeout(resolve, 100)); + } } - const jobInfo = await wmill.getCompletedJob({ - workspace: workspace.workspaceId, - id, - }); - if (opts.silent) { - console.log(JSON.stringify(jobInfo.result ?? {})); - } else { - log.info(JSON.stringify(jobInfo.result ?? {}, null, 2)); + if (retries >= MAX_RETRIES) { + throw new Error(`Timed out waiting for flow ${id} to complete`); } } @@ -551,7 +573,7 @@ export async function bootstrap( await loadNonDottedPathsSetting(); const flowDirFullPath = buildFolderPath(flowPath, "flow"); - mkdirSync(flowDirFullPath, { recursive: false }); + mkdirSync(flowDirFullPath, { recursive: true }); const newFlowDefinition = defaultFlowDefinition(); if (opts.summary !== undefined) { diff --git a/cli/src/commands/job/job.ts b/cli/src/commands/job/job.ts index 27bf02c90d..10d9de2562 100644 --- a/cli/src/commands/job/job.ts +++ b/cli/src/commands/job/job.ts @@ -92,7 +92,7 @@ async function list( .border(true) .body( jobs.map((j: any) => [ - j.id.substring(0, 8), + j.id, getJobStatus(j), j.script_path ?? j.raw_code?.substring(0, 30) ?? "-", j.created_by ?? j.email ?? "-", @@ -170,12 +170,35 @@ async function logs( const workspace = await resolveWorkspace(opts); await requireLogin(opts); + // Check if this is a flow job (flows don't have top-level logs) + try { + const job = await wmill.getJob({ + workspace: workspace.workspaceId, + id, + }); + const jobKind = (job as any).job_kind; // job_kind not in generated types yet + if (jobKind === "flow" || jobKind === "flowpreview") { + log.info(colors.yellow( + "Flow jobs don't have direct logs. Each step runs as a separate job.\n" + + "Use 'wmill job list --all' to see sub-jobs, then 'wmill job logs ' for individual step logs." + )); + return; + } + } catch { + // If we can't get the job info, proceed with trying to get logs anyway + } + const jobLogs = await wmill.getJobLogs({ workspace: workspace.workspaceId, id, }); - console.log(jobLogs); + if (jobLogs == null || jobLogs === "") { + log.info("No logs available for this job."); + } else { + console.error("to remove ansi colors, use: | sed 's/\\x1B\\[[0-9;]\\{1,\\}[A-Za-z]//g'"); + console.log(jobLogs); + } } async function cancel( diff --git a/cli/src/commands/resource-type/resource-type.ts b/cli/src/commands/resource-type/resource-type.ts index a4b42582a4..96c8429eb4 100644 --- a/cli/src/commands/resource-type/resource-type.ts +++ b/cli/src/commands/resource-type/resource-type.ts @@ -97,6 +97,10 @@ async function list(opts: GlobalOptions & { schema?: boolean; json?: boolean }) if (opts.json) { console.log(JSON.stringify(res)); + } else if (res.length === 0) { + log.info("No custom resource types found in this workspace."); + log.info("Built-in types like 'postgresql', 'slack', 'mysql', etc. are available from the Windmill Hub."); + return; } else if (opts.schema) { new Table() .header(["Workspace", "Name", "Schema"]) diff --git a/cli/src/commands/resource/resource.ts b/cli/src/commands/resource/resource.ts index 0f35c02293..e1c9052261 100644 --- a/cli/src/commands/resource/resource.ts +++ b/cli/src/commands/resource/resource.ts @@ -1,4 +1,4 @@ -import { stat, writeFile, readdir, readFile } from "node:fs/promises"; +import { mkdir, stat, writeFile, readdir, readFile } from "node:fs/promises"; import { stringify as yamlStringify } from "yaml"; import nodePath from "node:path"; @@ -203,6 +203,7 @@ async function newResource(opts: GlobalOptions, path: string) { resource_type: "", description: "", }; + await mkdir(nodePath.dirname(filePath), { recursive: true }); await writeFile(filePath, yamlStringify(template as Record), { flag: "wx", encoding: "utf-8", @@ -211,6 +212,7 @@ async function newResource(opts: GlobalOptions, path: string) { } async function get(opts: GlobalOptions & { json?: boolean }, path: string) { + if (opts.json) log.setSilent(true); const workspace = await resolveWorkspace(opts); await requireLogin(opts); const r = await wmill.getResource({ diff --git a/cli/src/commands/schedule/schedule.ts b/cli/src/commands/schedule/schedule.ts index 586581552c..db57cabc64 100644 --- a/cli/src/commands/schedule/schedule.ts +++ b/cli/src/commands/schedule/schedule.ts @@ -1,4 +1,5 @@ -import { stat, writeFile } from "node:fs/promises"; +import { mkdir, stat, writeFile } from "node:fs/promises"; +import { dirname } from "node:path"; import { stringify as yamlStringify } from "yaml"; import { Command } from "@cliffy/command"; @@ -70,6 +71,7 @@ async function newSchedule(opts: GlobalOptions, path: string) { is_flow: false, enabled: false, }; + await mkdir(dirname(filePath), { recursive: true }); await writeFile(filePath, yamlStringify(template as Record), { flag: "wx", encoding: "utf-8", @@ -78,6 +80,7 @@ async function newSchedule(opts: GlobalOptions, path: string) { } async function get(opts: GlobalOptions & { json?: boolean }, path: string) { + if (opts.json) log.setSilent(true); const workspace = await resolveWorkspace(opts); await requireLogin(opts); const s = await wmill.getSchedule({ diff --git a/cli/src/commands/script/script.ts b/cli/src/commands/script/script.ts index ac498d039b..41e1c16d76 100644 --- a/cli/src/commands/script/script.ts +++ b/cli/src/commands/script/script.ts @@ -23,10 +23,13 @@ import { import { Workspace } from "../workspace/workspace.ts"; import { + checkifMetadataUptodate, generateScriptMetadataInternal, getRawWorkspaceDependencies, parseMetadataFile, + readLockfile, } from "../../utils/metadata.ts"; +import { generateHash } from "../../utils/utils.ts"; import { WorkspaceDependenciesLanguage, ScriptLanguage, @@ -122,6 +125,23 @@ async function push(opts: PushOptions, filePath: string) { } await requireLogin(opts); + + // Warn if metadata appears stale (content changed since last generate-metadata) + try { + const content = await readFile(filePath, "utf-8"); + const remotePath = removeExtensionToPath(filePath).replaceAll(SEP, "/"); + const contentHash = await generateHash(content + remotePath); + const conf = await readLockfile(); + if (!(await checkifMetadataUptodate(remotePath, contentHash, conf))) { + log.warn(colors.yellow( + `Metadata for ${filePath} appears stale (content changed since last 'wmill generate-metadata').\n` + + `The schema and lock may not match the current code. Consider running 'wmill generate-metadata' first.` + )); + } + } catch { + // Don't block push if staleness check fails + } + const codebases = await listSyncCodebases(opts as SyncOptions); await handleFile( @@ -964,16 +984,20 @@ async function run( await track_job(workspace.workspaceId, id); } - while (true) { + const MAX_RETRIES = 600; // ~60 seconds at 100ms intervals + let retries = 0; + while (retries < MAX_RETRIES) { try { - const result = - ( - await wmill.getCompletedJob({ - workspace: workspace.workspaceId, - id, - }) - ).result ?? {}; + const completedJob = await wmill.getCompletedJob({ + workspace: workspace.workspaceId, + id, + }); + if (completedJob.success === false) { + process.exitCode = 1; + } + + const result = completedJob.result ?? {}; if (opts.silent) { console.log(JSON.stringify(result)); } else { @@ -982,9 +1006,13 @@ async function run( break; } catch { + retries++; await new Promise((resolve) => setTimeout(resolve, 100)); } } + if (retries >= MAX_RETRIES) { + throw new Error(`Timed out waiting for job ${id} to complete`); + } } export async function track_job(workspace: string, id: string) { @@ -1081,6 +1109,7 @@ async function show(opts: GlobalOptions, path: string) { } async function get(opts: GlobalOptions & { json?: boolean }, path: string) { + if (opts.json) log.setSilent(true); const workspace = await resolveWorkspace(opts); await requireLogin(opts); const s = await wmill.getScriptByPath({ diff --git a/cli/src/commands/sync/pull.ts b/cli/src/commands/sync/pull.ts index 826c1de090..0b4da237e1 100644 --- a/cli/src/commands/sync/pull.ts +++ b/cli/src/commands/sync/pull.ts @@ -3,9 +3,72 @@ import { colors } from "@cliffy/ansi/colors"; import { Command } from "@cliffy/command"; import * as log from "../../core/log.ts"; import JSZip from "jszip"; +import { extract } from "tar-stream"; +import { Readable } from "node:stream"; import { Workspace } from "../workspace/workspace.ts"; import { getHeaders } from "../../utils/utils.ts"; +/** + * Adapter that wraps tar entries in a JSZip-compatible interface + * so ZipFSElement in sync.ts can consume it without changes. + */ +class TarAsZip { + files: Record }> = {}; + + constructor(entries: Map) { + for (const [name, entry] of entries) { + const content = entry.content; + this.files[name] = { + dir: entry.isDir, + name, + async(_type: "text") { + return content; + }, + }; + } + } + + /** Return a filtered view containing only entries under the given prefix, with relative paths. */ + folder(prefix: string): TarAsZip | null { + const normalized = prefix.endsWith("/") ? prefix : prefix + "/"; + const sub = new TarAsZip(new Map()); + for (const [name, file] of Object.entries(this.files)) { + if (name.startsWith(normalized)) { + const relative = name.slice(normalized.length); + if (relative) { + sub.files[relative] = { ...file, name: relative }; + } + } + } + return Object.keys(sub.files).length > 0 ? sub : null; + } +} + +async function parseTarResponse(response: Response): Promise { + const buffer = Buffer.from(await response.arrayBuffer()); + const entries = new Map(); + const ex = extract(); + + return new Promise((resolve, reject) => { + ex.on("entry", (header, stream, next) => { + const chunks: Buffer[] = []; + stream.on("data", (chunk: Buffer) => chunks.push(chunk)); + stream.on("end", () => { + entries.set(header.name, { + content: Buffer.concat(chunks).toString("utf-8"), + isDir: header.type === "directory", + }); + next(); + }); + stream.on("error", reject); + stream.resume(); + }); + ex.on("finish", () => resolve(new TarAsZip(entries))); + ex.on("error", reject); + Readable.from(buffer).pipe(ex); + }); +} + export async function downloadZip( workspace: Workspace, plainSecrets: boolean | undefined, @@ -21,7 +84,7 @@ export async function downloadZip( includeKey?: boolean, skipWorkspaceDependencies?: boolean, defaultTs?: "bun" | "deno" -): Promise { +): Promise { const requestHeaders = new Headers(); requestHeaders.set("Authorization", "Bearer " + workspace.token); requestHeaders.set("Content-Type", "application/octet-stream"); @@ -34,38 +97,51 @@ export async function downloadZip( } const includeWorkspaceDependenciesValue = !(skipWorkspaceDependencies ?? false); - const url = workspace.remote + - "api/w/" + - workspace.workspaceId + - `/workspaces/tarball?archive_type=zip&plain_secret=${plainSecrets ?? false + const baseParams = `&plain_secret=${plainSecrets ?? false }&skip_variables=${skipVariables ?? false}&skip_resources=${skipResources ?? false }&skip_secrets=${skipSecrets ?? false}&include_schedules=${includeSchedules ?? false }&include_triggers=${includeTriggers ?? false}&include_users=${includeUsers ?? false }&include_groups=${includeGroups ?? false}&include_settings=${includeSettings ?? false }&include_key=${includeKey ?? false}&include_workspace_dependencies=${includeWorkspaceDependenciesValue}&default_ts=${defaultTs ?? "bun"}&skip_resource_types=${skipResourceTypes ?? false}&settings_version=v2`; - const zipResponse = await fetch(url, { - headers: requestHeaders, - method: "GET", - } - ); + const baseUrl = workspace.remote + "api/w/" + workspace.workspaceId + "/workspaces/tarball?"; - if (!zipResponse.ok) { - const body = await zipResponse.text(); - if (zipResponse.status === 404 || body.includes("no rows returned")) { - log.info(colors.red(`Workspace '${workspace.workspaceId}' not found on ${workspace.remote}. Please check your --workspace and try again.`)); - } else { - log.info(colors.red(`Failed to request tarball from API: ${zipResponse.status} ${zipResponse.statusText}`)); - if (body) { - log.info(colors.red(body)); - } - } - return process.exit(1); - } else { - log.debug(`Downloaded zip/tarball successfully`); + // Try zip first (standard format), fall back to tar if zip is not supported + const zipUrl = baseUrl + "archive_type=zip" + baseParams; + const zipResponse = await fetch(zipUrl, { headers: requestHeaders, method: "GET" }); + + if (zipResponse.ok) { + log.debug("Downloaded zip archive successfully"); + const blob = await zipResponse.blob(); + return await JSZip.loadAsync((await blob.arrayBuffer()) as any); } - const blob = await zipResponse.blob(); - return await JSZip.loadAsync((await blob.arrayBuffer()) as any); + + const body = await zipResponse.text(); + + // If zip format is not supported (backend compiled without zip feature), try tar + if (zipResponse.status === 400 && body.includes("Invalid Archive Type")) { + log.debug("Zip archive not supported by backend, falling back to tar"); + const tarUrl = baseUrl + "archive_type=tar" + baseParams; + const tarResponse = await fetch(tarUrl, { headers: requestHeaders, method: "GET" }); + + if (tarResponse.ok) { + log.debug("Downloaded tar archive successfully"); + return await parseTarResponse(tarResponse); + } + + const tarBody = await tarResponse.text(); + log.info(colors.red(`Failed to request tarball from API: ${tarResponse.status} ${tarResponse.statusText}`)); + if (tarBody) log.info(colors.red(tarBody)); + return process.exit(1); + } + + if (zipResponse.status === 404 || body.includes("no rows returned")) { + log.info(colors.red(`Workspace '${workspace.workspaceId}' not found on ${workspace.remote}. Please check your --workspace and try again.`)); + } else { + log.info(colors.red(`Failed to request tarball from API: ${zipResponse.status} ${zipResponse.statusText}`)); + if (body) log.info(colors.red(body)); + } + return process.exit(1); } function stub(_opts: GlobalOptions & { override: boolean }, _dir: string) { diff --git a/cli/src/commands/trigger/trigger.ts b/cli/src/commands/trigger/trigger.ts index d3c61114fc..c904095405 100644 --- a/cli/src/commands/trigger/trigger.ts +++ b/cli/src/commands/trigger/trigger.ts @@ -1,4 +1,5 @@ -import { stat, writeFile } from "node:fs/promises"; +import { mkdir, stat, writeFile } from "node:fs/promises"; +import { dirname } from "node:path"; import { stringify as yamlStringify } from "yaml"; import * as wmill from "../../../gen/services.gen.ts"; @@ -400,6 +401,7 @@ async function newTrigger(opts: GlobalOptions & { kind: string }, path: string) if (e.message?.startsWith("File already exists")) throw e; } const template = triggerTemplates[kind]; + await mkdir(dirname(filePath), { recursive: true }); await writeFile(filePath, yamlStringify(template), { flag: "wx", encoding: "utf-8", @@ -408,6 +410,7 @@ async function newTrigger(opts: GlobalOptions & { kind: string }, path: string) } async function get(opts: GlobalOptions & { json?: boolean; kind?: string }, path: string) { + if (opts.json) log.setSilent(true); const workspace = await resolveWorkspace(opts); await requireLogin(opts); diff --git a/cli/src/commands/variable/variable.ts b/cli/src/commands/variable/variable.ts index c01c083706..d902b8f79f 100644 --- a/cli/src/commands/variable/variable.ts +++ b/cli/src/commands/variable/variable.ts @@ -1,4 +1,5 @@ -import { stat, writeFile } from "node:fs/promises"; +import { mkdir, stat, writeFile } from "node:fs/promises"; +import { dirname } from "node:path"; import { stringify as yamlStringify } from "yaml"; import { requireLogin } from "../../core/auth.ts"; @@ -63,6 +64,7 @@ async function newVariable(opts: GlobalOptions, path: string) { is_secret: false, description: "", }; + await mkdir(dirname(filePath), { recursive: true }); await writeFile(filePath, yamlStringify(template as Record), { flag: "wx", encoding: "utf-8", @@ -71,6 +73,7 @@ async function newVariable(opts: GlobalOptions, path: string) { } async function get(opts: GlobalOptions & { json?: boolean }, path: string) { + if (opts.json) log.setSilent(true); const workspace = await resolveWorkspace(opts); await requireLogin(opts); const v = await wmill.getVariable({ @@ -215,10 +218,10 @@ async function add( undefined, { value, - is_secret: !opts.public && !opts.plainSecrets, + is_secret: !opts.public, description: "", }, - opts.plainSecrets ?? false + true // value from CLI is always plaintext — tell API not to treat it as pre-encrypted ); log.info(colors.bold.underline.green(`Variable ${remotePath} pushed`)); } diff --git a/cli/src/commands/workspace/workspace.ts b/cli/src/commands/workspace/workspace.ts index c51544fe90..1b5d287069 100644 --- a/cli/src/commands/workspace/workspace.ts +++ b/cli/src/commands/workspace/workspace.ts @@ -422,7 +422,7 @@ async function whoami(_opts: GlobalOptions) { const { resolveWorkspace } = await import("../../core/context.ts"); try { const ws = await resolveWorkspace(_opts); - log.info("Active: " + colors.green.bold(`${activeName || "none"}`) + ` (fork workspace: ${ws.workspaceId})`); + log.info("Active: " + colors.green.bold(ws.workspaceId) + ` (fork of ${activeName || "unknown"})`); } catch { log.info("Active: " + colors.green.bold(activeName || "none") + " (fork branch)"); } diff --git a/cli/src/core/context.ts b/cli/src/core/context.ts index 6ff1461960..29a3acdf86 100644 --- a/cli/src/core/context.ts +++ b/cli/src/core/context.ts @@ -366,7 +366,7 @@ export async function tryResolveBranchWorkspace( selectedProfile.name = `${selectedProfile.name}/${workspaceIdIfForked}`; selectedProfile.workspaceId = workspaceIdIfForked; log.info( - `Inferred workspace id \`${workspaceId}\` from branch name because this is a workspace fork branch (\`${rawBranch}\`). ` + `Using fork workspace \`${workspaceIdIfForked}\` (parent: \`${workspaceId}\`) from branch \`${rawBranch}\`` ); } diff --git a/cli/src/utils/resource_folders.ts b/cli/src/utils/resource_folders.ts index b4f204b1c6..f720fdc680 100644 --- a/cli/src/utils/resource_folders.ts +++ b/cli/src/utils/resource_folders.ts @@ -48,7 +48,7 @@ let _nonDottedPathsLogged = false; */ export function setNonDottedPaths(value: boolean): void { if (value && !_nonDottedPathsLogged) { - log.info("Using non-dotted paths (__flow, __app, __raw_app)"); + log.debug("Using non-dotted paths (__flow, __app, __raw_app)"); _nonDottedPathsLogged = true; } _nonDottedPaths = value; diff --git a/cli/test/standalone_commands.test.ts b/cli/test/standalone_commands.test.ts index 1d75680354..bc17ef0d32 100644 --- a/cli/test/standalone_commands.test.ts +++ b/cli/test/standalone_commands.test.ts @@ -139,8 +139,10 @@ describe("resource-type commands", () => { ); expect(result.code).toEqual(0); - // Table headers should be present - expect(result.stdout).toContain("Name"); + // When empty, shows helpful message; when populated, shows table with Name header + const hasTable = result.stdout.includes("Name"); + const hasEmptyMessage = result.stdout.includes("No custom resource types"); + expect(hasTable || hasEmptyMessage).toBe(true); }); });