diff --git a/cli/src/commands/script/script.ts b/cli/src/commands/script/script.ts index 7d90d367b9..a1e0f7deff 100644 --- a/cli/src/commands/script/script.ts +++ b/cli/src/commands/script/script.ts @@ -180,20 +180,25 @@ export async function findResourceFile(path: string) { let contentBasePathJSON = splitPath[0] + "." + splitPath[1] + ".json"; let contentBasePathYAML = splitPath[0] + "." + splitPath[1] + ".yaml"; - // Check for branch-specific metadata files first + // Check for workspace-specific metadata files first, using the wmill.yaml + // config key for the current git branch as the filename suffix (falls back + // to the branch name when no matching workspace entry exists). const currentBranch = getCurrentGitBranch(); + const wsName = currentBranch + ? await specificItems.resolveWsNameForGitBranch(currentBranch) + : null; const candidates = [contentBasePathJSON, contentBasePathYAML]; - if (currentBranch) { - // Add branch-specific candidates at the beginning (higher priority) + if (wsName) { + // Add workspace-specific candidates at the beginning (higher priority) const branchSpecificJSON = specificItems.toWorkspaceSpecificPath( contentBasePathJSON, - currentBranch + wsName ); const branchSpecificYAML = specificItems.toWorkspaceSpecificPath( contentBasePathYAML, - currentBranch + wsName ); candidates.unshift(branchSpecificJSON, branchSpecificYAML); } diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index 33d123edb4..920f1a2965 100644 --- a/cli/src/commands/sync/sync.ts +++ b/cli/src/commands/sync/sync.ts @@ -46,6 +46,7 @@ import { } from "../../utils/utils.ts"; import { getEffectiveSettings, + getWorkspaceNames, mergeConfigWithConfigFile, parseSyncBehavior, SyncOptions, @@ -122,6 +123,27 @@ function resolveWsNameFromBranch(opts: SyncOptions, branchName: string): string return match ? match[0] : branchName; } +// Resolve wsNameForConfig from CLI flags. Prefers --branch → matching config key, +// then --workspace → matching config key (incl. when --base-url is set). Returns +// undefined when no flag-based resolution applies; callers then fall back to +// inferWsNameFromProfile on the resolved workspace profile. +export function resolveWsNameForConfigFromFlags( + opts: SyncOptions & { branch?: string; workspace?: string }, +): string | undefined { + if (opts.branch) { + return resolveWsNameFromBranch(opts, opts.branch); + } + if (opts.workspace) { + // Use getWorkspaceNames so reserved keys (e.g. commonSpecificItems) are filtered out, + // matching the behavior of findWorkspaceByGitBranch / inferWsNameFromProfile. + const validKeys = getWorkspaceNames(opts.workspaces); + if (validKeys.includes(opts.workspace)) { + return opts.workspace; + } + } + return undefined; +} + // Warn if --workspace overrides auto-detected branch or if workspace not in config. function warnWorkspaceOverride(opts: SyncOptions, wsNameForConfig: string | undefined): void { if (!wsNameForConfig || !opts.workspaces) return; @@ -2181,27 +2203,29 @@ export async function pull( // Resolve workspace name for config lookups. // --branch resolves git branch → workspace name (deprecated but still supported). - // --workspace (without --base-url) selects a workspace config entry by name. - // When --base-url is used with --workspace, --workspace is a profile selector only; - // --branch should still drive config lookups. + // --workspace selects a workspace config entry by name when it matches one, + // regardless of --base-url. If it doesn't match any entry it's treated as a + // profile/credential selector only. const hasExplicitCredentials = !!opts.baseUrl; let wsNameForConfig: string | undefined; - if (opts.branch) { - if (!hasExplicitCredentials && !branchDeprecationWarned) { - log.warn("⚠️ --branch/--env is deprecated. Use --workspace instead."); - branchDeprecationWarned = true; - } - wsNameForConfig = resolveWsNameFromBranch(opts, opts.branch); - } else if (opts.workspace && !hasExplicitCredentials) { - // --workspace without --base-url: use as workspace config name - wsNameForConfig = opts.workspace; - warnWorkspaceOverride(opts, wsNameForConfig); + if (opts.branch && !hasExplicitCredentials && !branchDeprecationWarned) { + log.warn("⚠️ --branch/--env is deprecated. Use --workspace instead."); + branchDeprecationWarned = true; } - // Validate workspace configuration early (skipped when override is used) + wsNameForConfig = resolveWsNameForConfigFromFlags(opts); + + if (!opts.branch && opts.workspace && !hasExplicitCredentials) { + // Warn if override doesn't match a config key, or mismatches the auto-detected branch + warnWorkspaceOverride(opts, opts.workspace); + } + + // Validate workspace configuration early. Skip when ANY explicit flag is set + // (even a --workspace value that doesn't match a config key — the user opted + // out of branch-based auto-detection). try { - await validateBranchConfiguration(opts, wsNameForConfig); + await validateBranchConfiguration(opts, wsNameForConfig ?? opts.workspace); } catch (error) { if (error instanceof Error && error.message.includes("overrides")) { log.error(error.message); @@ -2723,20 +2747,23 @@ export async function push( const hasExplicitCredentials = !!opts.baseUrl; let wsNameForConfig: string | undefined; - if (opts.branch) { - if (!hasExplicitCredentials && !branchDeprecationWarned) { - log.warn("⚠️ --branch/--env is deprecated. Use --workspace instead."); - branchDeprecationWarned = true; - } - wsNameForConfig = resolveWsNameFromBranch(opts, opts.branch); - } else if (opts.workspace && !hasExplicitCredentials) { - wsNameForConfig = opts.workspace; - warnWorkspaceOverride(opts, wsNameForConfig); + if (opts.branch && !hasExplicitCredentials && !branchDeprecationWarned) { + log.warn("⚠️ --branch/--env is deprecated. Use --workspace instead."); + branchDeprecationWarned = true; } - // Validate workspace configuration early (skipped when override is used) + wsNameForConfig = resolveWsNameForConfigFromFlags(opts); + + if (!opts.branch && opts.workspace && !hasExplicitCredentials) { + // Warn if override doesn't match a config key, or mismatches the auto-detected branch + warnWorkspaceOverride(opts, opts.workspace); + } + + // Validate workspace configuration early. Skip when ANY explicit flag is set + // (even a --workspace value that doesn't match a config key — the user opted + // out of branch-based auto-detection). try { - await validateBranchConfiguration(opts, wsNameForConfig); + await validateBranchConfiguration(opts, wsNameForConfig ?? opts.workspace); } catch (error) { if (error instanceof Error && error.message.includes("overrides")) { log.error(error.message); diff --git a/cli/src/commands/trigger/trigger.ts b/cli/src/commands/trigger/trigger.ts index aaa43880ac..1cadc8d9d2 100644 --- a/cli/src/commands/trigger/trigger.ts +++ b/cli/src/commands/trigger/trigger.ts @@ -34,6 +34,7 @@ import { import { fromWorkspaceSpecificPath, isWorkspaceSpecificFile, + resolveWsNameForGitBranch, } from "../../core/specific_items.ts"; import { getCurrentGitBranch } from "../../utils/git.ts"; import { requireLogin } from "../../core/auth.ts"; @@ -584,14 +585,17 @@ function checkIfValidTrigger(kind: string | undefined): kind is TriggerType { } } -function extractTriggerKindFromPath(filePath: string): string | undefined { +async function extractTriggerKindFromPath(filePath: string): Promise { let pathToAnalyze = filePath; - // If this is a branch-specific file, convert it to the base path first + // If this is a workspace-specific file, convert it to the base path first. + // Resolve the wmill.yaml config key for the current branch (falls back to + // the branch name when no matching workspace entry exists). if (isWorkspaceSpecificFile(filePath)) { const currentBranch = getCurrentGitBranch(); if (currentBranch) { - pathToAnalyze = fromWorkspaceSpecificPath(filePath, currentBranch); + const wsName = await resolveWsNameForGitBranch(currentBranch); + pathToAnalyze = fromWorkspaceSpecificPath(filePath, wsName); } } @@ -615,7 +619,7 @@ async function push(opts: GlobalOptions, filePath: string, remotePath: string) { console.log(colors.bold.yellow("Pushing trigger...")); - const triggerKind = extractTriggerKindFromPath(filePath); + const triggerKind = await extractTriggerKindFromPath(filePath); if (!checkIfValidTrigger(triggerKind)) { throw new Error("Invalid trigger kind: " + triggerKind); } diff --git a/cli/src/core/specific_items.ts b/cli/src/core/specific_items.ts index 3d0d9df248..fbdffad153 100644 --- a/cli/src/core/specific_items.ts +++ b/cli/src/core/specific_items.ts @@ -4,10 +4,22 @@ import { isFileResource, isFilesetResource } from "../utils/utils.ts"; import { SyncOptions, findWorkspaceByGitBranch, + readConfigFile, WorkspaceEntryConfig, } from "./conf.ts"; import { TRIGGER_TYPES } from "../types.ts"; +/** + * Resolve the effective workspace name (wmill.yaml config key) for a given + * git branch. Falls back to the branch name itself when no matching workspace + * entry exists (legacy behavior). + */ +export async function resolveWsNameForGitBranch(branchName: string): Promise { + const config = await readConfigFile({ warnIfMissing: false }); + const match = findWorkspaceByGitBranch(config.workspaces, branchName); + return match ? match[0] : branchName; +} + export interface SpecificItemsConfig { variables?: string[]; resources?: string[]; diff --git a/cli/test/workspace_key_filename_integration.test.ts b/cli/test/workspace_key_filename_integration.test.ts new file mode 100644 index 0000000000..ae22f191bf --- /dev/null +++ b/cli/test/workspace_key_filename_integration.test.ts @@ -0,0 +1,182 @@ +import { describe, expect, test } from "bun:test"; +import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises"; +import { execSync } from "node:child_process"; +import os from "node:os"; +import path from "node:path"; +import { stringify as yamlStringify } from "yaml"; + +import { resolveWsNameForGitBranch } from "../src/core/specific_items.ts"; +import { findResourceFile } from "../src/commands/script/script.ts"; +import { resolveWsNameForConfigFromFlags } from "../src/commands/sync/sync.ts"; +import type { SyncOptions } from "../src/core/conf.ts"; + +// Integration tests covering the bug where workspace-specific filenames used +// the raw git branch name instead of the wmill.yaml workspace config key. +// A per-item helper (findResourceFile) now resolves the effective wsName from +// wmill.yaml before falling back to the branch name. + +async function withGitRepoAndConfig( + config: unknown, + branch: string, + fn: (tempDir: string) => Promise, +): Promise { + const tempDir = await mkdtemp(path.join(os.tmpdir(), "wmill_wskey_")); + const originalCwd = process.cwd(); + try { + // Set up a minimal git repo on the target branch so getCurrentGitBranch() + // returns the expected value. An initial commit is needed so HEAD points + // somewhere and `git rev-parse --abbrev-ref HEAD` succeeds. + execSync(`git init -q -b ${branch}`, { cwd: tempDir }); + execSync(`git config user.email test@example.com`, { cwd: tempDir }); + execSync(`git config user.name test`, { cwd: tempDir }); + + await writeFile( + path.join(tempDir, "wmill.yaml"), + yamlStringify(config), + ); + + execSync(`git add wmill.yaml && git commit -q -m init`, { cwd: tempDir }); + + process.chdir(tempDir); + await fn(tempDir); + } finally { + process.chdir(originalCwd); + await rm(tempDir, { recursive: true, force: true }); + } +} + +describe("resolveWsNameForGitBranch", () => { + test("returns the wmill.yaml config key for a branch matched via gitBranch field", async () => { + await withGitRepoAndConfig( + { + workspaces: { + myKey: { gitBranch: "main", workspaceId: "prod" }, + }, + }, + "main", + async () => { + const wsName = await resolveWsNameForGitBranch("main"); + expect(wsName).toEqual("myKey"); + }, + ); + }); + + test("returns the wmill.yaml config key when the branch equals the key and gitBranch is not set", async () => { + await withGitRepoAndConfig( + { + workspaces: { + staging: { workspaceId: "stg_workspace" }, + }, + }, + "staging", + async () => { + const wsName = await resolveWsNameForGitBranch("staging"); + expect(wsName).toEqual("staging"); + }, + ); + }); + + test("falls back to the branch name when no matching workspace entry exists", async () => { + await withGitRepoAndConfig( + { + workspaces: { + production: { gitBranch: "main" }, + }, + }, + "feature-x", + async () => { + const wsName = await resolveWsNameForGitBranch("feature-x"); + expect(wsName).toEqual("feature-x"); + }, + ); + }); +}); + +describe("resolveWsNameForConfigFromFlags", () => { + test("--workspace matching a config key resolves, even with --base-url", () => { + const opts: SyncOptions & { branch?: string; workspace?: string } = { + workspace: "test", + baseUrl: "http://127.0.0.1:8080/", + workspaces: { + test: { gitBranch: "main" }, + prod: { gitBranch: "main" }, + }, + } as any; + expect(resolveWsNameForConfigFromFlags(opts)).toEqual("test"); + }); + + test("--workspace not in config with --base-url returns undefined (treat as ad-hoc credential)", () => { + const opts: SyncOptions & { branch?: string; workspace?: string } = { + workspace: "adhocWorkspaceId", + baseUrl: "https://other.windmill.dev/", + workspaces: { + test: { gitBranch: "main" }, + }, + } as any; + expect(resolveWsNameForConfigFromFlags(opts)).toBeUndefined(); + }); + + test("--workspace matching config key resolves even without --base-url", () => { + const opts: SyncOptions & { branch?: string; workspace?: string } = { + workspace: "prod", + workspaces: { test: {}, prod: {} }, + } as any; + expect(resolveWsNameForConfigFromFlags(opts)).toEqual("prod"); + }); + + test("--branch takes precedence and looks up by gitBranch", () => { + const opts: SyncOptions & { branch?: string; workspace?: string } = { + branch: "main", + workspace: "someOtherKey", + workspaces: { + test: { gitBranch: "main" }, + prod: { gitBranch: "release" }, + }, + } as any; + expect(resolveWsNameForConfigFromFlags(opts)).toEqual("test"); + }); + + test("no flags returns undefined", () => { + const opts: SyncOptions & { branch?: string; workspace?: string } = { + workspaces: { test: {} }, + } as any; + expect(resolveWsNameForConfigFromFlags(opts)).toBeUndefined(); + }); + + test("reserved key 'commonSpecificItems' is not accepted as a config key", () => { + const opts: SyncOptions & { branch?: string; workspace?: string } = { + workspace: "commonSpecificItems", + workspaces: { + test: {}, + commonSpecificItems: { variables: ["f/**"] } as any, + }, + } as any; + expect(resolveWsNameForConfigFromFlags(opts)).toBeUndefined(); + }); +}); + +describe("findResourceFile picks the wsName-named file, not the branch-named file", () => { + test("finds the workspace-specific resource file using the config key as suffix", async () => { + await withGitRepoAndConfig( + { + workspaces: { + myKey: { gitBranch: "main", workspaceId: "prod" }, + }, + }, + "main", + async (tempDir) => { + // Only the config-key-named file exists on disk. A wmill without this + // fix would look for `f/foo.main.resource.yaml` (the branch name) and + // either miss this file or pick a stale branch-named file. + await mkdir(path.join(tempDir, "f"), { recursive: true }); + await writeFile( + path.join(tempDir, "f/foo.myKey.resource.yaml"), + "value: {}\nresource_type: text\n", + ); + + const found = await findResourceFile("f/foo.resource.file.txt"); + expect(found).toEqual("f/foo.myKey.resource.yaml"); + }, + ); + }); +});