diff --git a/cli/src/commands/sync/pull.ts b/cli/src/commands/sync/pull.ts index 68fd002eb5..d75dc81912 100644 --- a/cli/src/commands/sync/pull.ts +++ b/cli/src/commands/sync/pull.ts @@ -150,7 +150,18 @@ export async function downloadZip( } 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.`)); + log.info( + colors.red( + `Workspace id '${workspace.workspaceId}' not found on ${workspace.remote}` + + (workspace.name !== workspace.workspaceId + ? ` (resolved from profile '${workspace.name}')` + : "") + + `.\n` + + `Note this is the workspace *id* sent to the API, which is not necessarily what you passed to --workspace:\n` + + ` - check 'wmill workspace list' (the 'workspace id' column)\n` + + ` - check the 'workspaces' block of wmill.yaml ('workspaceId' overrides the workspace name)` + ) + ); } else { log.info(colors.red(`Failed to request tarball from API: ${zipResponse.status} ${zipResponse.statusText}`)); if (body) log.info(colors.red(body)); diff --git a/cli/src/core/conf.ts b/cli/src/core/conf.ts index 1fcce7a43f..786ef48cc2 100644 --- a/cli/src/core/conf.ts +++ b/cli/src/core/conf.ts @@ -145,7 +145,9 @@ function getGitRepoRoot(): string | null { } export const GLOBAL_CONFIG_OPT = { noCdToRoot: false }; -function findWmillYaml(): string | null { + +// Pure upward search: no chdir, no logging. findWmillYaml() adds the chdir. +function locateWmillYaml(): string | null { const startDir = resolve(process.cwd()); const isInGitRepo = isGitRepository(); const gitRoot = isInGitRepo ? getGitRepoRoot() : null; @@ -176,6 +178,13 @@ function findWmillYaml(): string | null { currentDir = parentDir; } + return foundPath; +} + +function findWmillYaml(): string | null { + const startDir = resolve(process.cwd()); + const foundPath = locateWmillYaml(); + // If wmill.yaml was found in a parent directory, warn the user and change working directory if ( !GLOBAL_CONFIG_OPT.noCdToRoot && @@ -198,6 +207,37 @@ export function getWmillYamlPath(): string | null { return findWmillYaml(); } +/** + * Look up one `workspaces` entry, for diagnostics only. readConfigFile() must + * not be used for that: it chdirs to the config's directory, exits on an + * unsupported syncBehavior and throws on a malformed file. A diagnostic may + * never fail or relocate the command it is diagnosing. + */ +export async function peekWorkspaceEntry( + workspaceName: string +): Promise { + if (RESERVED_WORKSPACE_KEYS.has(workspaceName)) { + return undefined; + } + const wmillYamlPath = locateWmillYaml(); + if (!wmillYamlPath) { + return undefined; + } + try { + const conf = (await yamlParseFile(wmillYamlPath)) as SyncOptions; + const workspaces = + conf?.workspaces ?? + conf?.gitBranches ?? + conf?.environments ?? + conf?.git_branches; + const entry = (workspaces as any)?.[workspaceName]; + return typeof entry === "object" && entry !== null ? entry : undefined; + } catch (e) { + log.debug(`Failed to parse ${wmillYamlPath} for workspace lookup: ${e}`); + return undefined; + } +} + let legacyConfigWarned = false; export async function readConfigFile(opts?: { warnIfMissing?: boolean }): Promise { diff --git a/cli/src/core/context.ts b/cli/src/core/context.ts index 502157f0a7..80b27ea52e 100644 --- a/cli/src/core/context.ts +++ b/cli/src/core/context.ts @@ -20,6 +20,7 @@ import { import { getLastUsedProfile, setLastUsedProfile } from "./branch-profiles.ts"; import { readConfigFile, + peekWorkspaceEntry, findWorkspaceByGitBranch, getEffectiveWorkspaceId, getWmillYamlPath, @@ -219,6 +220,9 @@ async function tryResolveWorkspace( // First try: look up workspace by name in wmill.yaml workspaces config const config = await readConfigFile({ warnIfMissing: false }); const wsEntry = config.workspaces?.[opts.workspace] as WorkspaceEntryConfig | undefined; + // What wmill.yaml said to target, kept for the fallback below: a profile + // found by name can silently point somewhere else entirely. + let configuredTarget: { workspaceId: string; baseUrl: string } | undefined; if (wsEntry?.baseUrl) { const workspaceId = getEffectiveWorkspaceId(opts.workspace, wsEntry); let normalizedBaseUrl: string; @@ -231,6 +235,8 @@ async function tryResolveWorkspace( }; } + configuredTarget = { workspaceId, baseUrl: normalizedBaseUrl }; + // Find matching profile by baseUrl + workspaceId const allProfs = await allWorkspaces(opts.configDir); const matching = allProfs.filter( @@ -283,6 +289,22 @@ async function tryResolveWorkspace( ), }; } + if ( + configuredTarget && + (e.workspaceId !== configuredTarget.workspaceId || + e.remote !== configuredTarget.baseUrl) + ) { + log.warnStderr( + colors.yellow( + `⚠️ Falling back to the local profile named '${opts.workspace}' (${e.workspaceId} on ${e.remote}), which does NOT match wmill.yaml:\n` + + ` wmill.yaml maps workspace '${opts.workspace}' to ${configuredTarget.workspaceId} on ${configuredTarget.baseUrl}, but no profile targets it.\n` + + ` Run: wmill workspace add ${configuredTarget.workspaceId} ${configuredTarget.baseUrl}` + ) + ); + } + log.infoStderr( + `Using local profile '${e.name}' → ${e.workspaceId} on ${e.remote}` + ); (opts as any).__secret_workspace = e; return { isError: false, value: e }; } @@ -486,6 +508,8 @@ export async function resolveWorkspace( return process.exit(-1); } + let resolved: Workspace | undefined; + // Try to find existing workspace profile by name, then by workspaceId + remote if (opts.workspace) { let existingWorkspace = await getWorkspaceByName( @@ -523,19 +547,45 @@ export async function resolveWorkspace( ); return process.exit(-1); } - return { + resolved = { ...existingWorkspace, token: opts.token, }; } } - return { + resolved ??= { remote: normalizedBaseUrl, workspaceId: opts.workspace, name: opts.workspace, token: opts.token, }; + + // --base-url pins the target, so wmill.yaml's `workspaces` block is never + // consulted and `--workspace` reaches the API as a workspace id. Name the + // id being sent, and the mapping being skipped, before the request 404s + // on an id the user never typed. + // Only an explicit `workspaceId:` is worth reporting: an entry without one + // maps the name to itself, leaving nothing to correct. + const yamlEntry = await peekWorkspaceEntry(opts.workspace); + const yamlWorkspaceId = yamlEntry?.workspaceId; + if (yamlWorkspaceId && yamlWorkspaceId !== resolved.workspaceId) { + log.warnStderr( + colors.yellow( + `⚠️ --base-url is set, so wmill.yaml is not consulted: workspace id '${resolved.workspaceId}' is sent to the API.\n` + + ` wmill.yaml maps workspace '${opts.workspace}' to workspace id '${yamlWorkspaceId}'${yamlEntry!.baseUrl ? ` on ${yamlEntry!.baseUrl}` : ""}.\n` + + ` Use '--workspace ${yamlWorkspaceId}', or drop --base-url/--token to resolve through wmill.yaml.` + ) + ); + } + log.infoStderr( + `Using workspace id '${resolved.workspaceId}' on ${normalizedBaseUrl} (--base-url given` + + (resolved.name !== resolved.workspaceId + ? `, profile '${resolved.name}')` + : ")") + ); + (opts as any).__secret_workspace = resolved; + return resolved; } else { log.infoStderr( colors.red( diff --git a/cli/test/base_url_workspace_resolution_unit.test.ts b/cli/test/base_url_workspace_resolution_unit.test.ts new file mode 100644 index 0000000000..66f618d113 --- /dev/null +++ b/cli/test/base_url_workspace_resolution_unit.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, test } from "bun:test"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; + +import { resolveWorkspace } from "../src/core/context.ts"; +import { getWorkspaceConfigFilePath } from "../windmill-utils-internal/src/config/config.ts"; +import type { GlobalOptions } from "../src/types.ts"; + +const BASE_URL = "http://localhost:9999/"; + +// --base-url pins the target: --workspace reaches the API as a workspace id and +// wmill.yaml is not consulted. The warning that says so may only peek at the +// file — readConfigFile() exits on an unsupported syncBehavior and throws on a +// malformed one, so resolving through it lets an unrelated config fail a +// command that never needed it. +async function withWmillYaml( + wmillYaml: string, + fn: (opts: GlobalOptions) => Promise +): Promise { + const repoDir = await mkdtemp(path.join(os.tmpdir(), "wmill_baseurl_repo_")); + const configDir = await mkdtemp(path.join(os.tmpdir(), "wmill_baseurl_conf_")); + const originalCwd = process.cwd(); + try { + await writeFile(path.join(repoDir, "wmill.yaml"), wmillYaml); + await writeFile(await getWorkspaceConfigFilePath(configDir), ""); + + process.chdir(repoDir); + await fn({ + configDir, + baseUrl: BASE_URL, + token: "sometoken", + workspace: "staging", + } as GlobalOptions); + } finally { + process.chdir(originalCwd); + await rm(repoDir, { recursive: true, force: true }); + await rm(configDir, { recursive: true, force: true }); + } +} + +describe("--base-url workspace resolution", () => { + const rejectedConfigs: [string, string][] = [ + ["an unsupported syncBehavior", "syncBehavior: v2\n"], + ["a malformed file", 'workspaces:\n staging:\n baseUrl: "unterminated\n'], + ]; + + for (const [label, wmillYaml] of rejectedConfigs) { + test(`resolves despite ${label}`, async () => { + await withWmillYaml(wmillYaml, async (opts) => { + const workspace = await resolveWorkspace(opts); + expect(workspace.workspaceId).toBe("staging"); + expect(workspace.remote).toBe(BASE_URL); + }); + }); + } + + test("a workspaces mapping never overrides the explicit workspace id", async () => { + await withWmillYaml( + "workspaces:\n staging:\n baseUrl: http://elsewhere.example/\n workspaceId: admins\n", + async (opts) => { + const workspace = await resolveWorkspace(opts); + expect(workspace.workspaceId).toBe("staging"); + expect(workspace.remote).toBe(BASE_URL); + } + ); + }); +});