diff --git a/.github/workflows/git-sync-test.yml b/.github/workflows/git-sync-test.yml index 3c4d37622a..4b99c62eae 100644 --- a/.github/workflows/git-sync-test.yml +++ b/.github/workflows/git-sync-test.yml @@ -9,6 +9,10 @@ on: - "backend/windmill-api-integration-tests/tests/git_sync*" - "backend/ee-repo-ref.txt" - "backend/windmill-common/src/workspaces.rs" + - "backend/windmill-worker/src/result_processor.rs" + - "backend/windmill-api-workspaces/**" + - "cli/src/commands/sync/**" + - "cli/src/utils/git.ts" - "integration_tests/test/git_sync_test.py" - ".github/workflows/git-sync-test.yml" pull_request: @@ -18,6 +22,10 @@ on: - "backend/windmill-api-integration-tests/tests/git_sync*" - "backend/ee-repo-ref.txt" - "backend/windmill-common/src/workspaces.rs" + - "backend/windmill-worker/src/result_processor.rs" + - "backend/windmill-api-workspaces/**" + - "cli/src/commands/sync/**" + - "cli/src/utils/git.ts" - "integration_tests/test/git_sync_test.py" - ".github/workflows/git-sync-test.yml" @@ -50,8 +58,8 @@ jobs: echo "Changed files:" echo "$CHANGED_FILES" - # Direct git sync file changes — always relevant - if echo "$CHANGED_FILES" | grep -qE '^(backend/windmill-git-sync/|backend/windmill-api-integration-tests/tests/git_sync|backend/windmill-common/src/workspaces\.rs|integration_tests/test/git_sync|\.github/workflows/git-sync-test\.yml)'; then + # Direct git sync file changes — always relevant. + if echo "$CHANGED_FILES" | grep -qE '^(backend/windmill-git-sync/|backend/windmill-worker/src/result_processor\.rs|backend/windmill-api-workspaces/|backend/windmill-api-integration-tests/tests/git_sync|backend/windmill-common/src/workspaces\.rs|cli/src/commands/sync/|cli/src/utils/git\.ts|integration_tests/test/git_sync|\.github/workflows/git-sync-test\.yml)'; then echo "should_run=true" >> "$GITHUB_OUTPUT" echo "Relevant: direct git sync file changes" exit 0 diff --git a/backend/.sqlx/query-079e1dab42a783a1e5e4cba5faa854b4b727b1fab89e42f2efaf97c63dad6384.json b/backend/.sqlx/query-079e1dab42a783a1e5e4cba5faa854b4b727b1fab89e42f2efaf97c63dad6384.json new file mode 100644 index 0000000000..8c58b6cf54 --- /dev/null +++ b/backend/.sqlx/query-079e1dab42a783a1e5e4cba5faa854b4b727b1fab89e42f2efaf97c63dad6384.json @@ -0,0 +1,28 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT parent_workspace_id, is_dev_workspace FROM workspace WHERE id = $1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "parent_workspace_id", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "is_dev_workspace", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + true, + false + ] + }, + "hash": "079e1dab42a783a1e5e4cba5faa854b4b727b1fab89e42f2efaf97c63dad6384" +} diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index d0497c1e02..efc5187476 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -aaa6cb89b05b76139252c64f057e53b94d12ac60 +8bf73f803158bcbf7b8d55a36f4a1ebfcc1bbcd9 diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 99fb33e417..f20fce538d 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -826,21 +826,37 @@ async fn reject_dev_label_matching_tracked_branch( Ok(()) } -/// Reject parent-only git-sync settings on a fork workspace. Auto-pull, fork -/// PRs, and promotion mode are all configured at the parent: repo → fork sync is -/// routed by the parent's webhook/poller (`sync_forks`), a fork-owned auto-pull -/// would register a second webhook on the same GitHub repo per fork, and a -/// fork's deploys always go to its `wm-fork/**` branch so a promotion repo could -/// never take effect there. +/// Reject parent-only git-sync settings on a fork workspace. Auto-pull and fork +/// PRs are configured at the parent: repo → fork sync is routed by the parent's +/// webhook/poller (`sync_forks`), and a fork-owned auto-pull would register a +/// second webhook on the same GitHub repo per fork. Promotion mode is rejected +/// on throwaway forks (their deploys always go to their `wm-fork/**` branch, so +/// a promotion repo could never take effect) but allowed on a **dev workspace**, +/// which deploys per-item `wm_deploy/**` branches that promote into the parent. async fn reject_parent_only_git_sync_settings_on_fork<'a>( db: &DB, w_id: &str, - mut repos: impl Iterator, + repos: impl Iterator, ) -> Result<()> { - let offending = repos.find_map(|r| { + let row = sqlx::query!( + "SELECT parent_workspace_id, is_dev_workspace FROM workspace WHERE id = $1", + w_id + ) + .fetch_optional(db) + .await?; + let is_fork = row + .as_ref() + .and_then(|r| r.parent_workspace_id.as_ref()) + .is_some() + || w_id.starts_with(windmill_common::workspaces::WM_FORK_PREFIX); + if !is_fork { + return Ok(()); + } + let is_dev = row.map(|r| r.is_dev_workspace).unwrap_or(false); + let offending = repos.into_iter().find_map(|r| { if r.auto_pull.as_ref().is_some_and(|a| a.enabled) { Some("Auto-pull") - } else if r.use_individual_branch.unwrap_or(false) { + } else if r.use_individual_branch.unwrap_or(false) && !is_dev { Some("Promotion mode") } else if r.fork_open_prs { Some("Opening PRs for fork deploys") @@ -848,17 +864,7 @@ async fn reject_parent_only_git_sync_settings_on_fork<'a>( None } }); - let Some(offending) = offending else { - return Ok(()); - }; - let parent = sqlx::query_scalar!( - "SELECT parent_workspace_id FROM workspace WHERE id = $1", - w_id - ) - .fetch_optional(db) - .await? - .flatten(); - if parent.is_some() || w_id.starts_with(windmill_common::workspaces::WM_FORK_PREFIX) { + if let Some(offending) = offending { return Err(Error::BadRequest(format!( "{offending} cannot be configured on a fork workspace: it is managed from the parent workspace's git sync settings" ))); @@ -3130,6 +3136,95 @@ async fn check_open_prs_license<'a>( Ok(()) } +/// Promotion mode (`use_individual_branch`: per-item `wm_deploy/**` deploy +/// branches) is an EE feature; runtime-gate it like auto-pull and PR creation +/// so an enterprise binary without an active plan can't enable it via either +/// git-sync edit endpoint. +#[cfg(feature = "enterprise")] +async fn check_promotion_license<'a>( + mut repos: impl Iterator, +) -> Result<()> { + if repos.any(|r| r.use_individual_branch.unwrap_or(false)) { + check_git_sync_ee_license("Promotion mode").await?; + } + Ok(()) +} + +/// Promotion on a dev workspace needs the dev-aware sync script (hub >= 28796): +/// an older pinned script bundles a CLI that force-disables per-item branches +/// on every fork, so enabling promotion would silently keep deploying to the +/// env-label branch. Reject with an actionable error instead (the dispatcher +/// demotes inherited configs the same way). Roots run promotion on any script +/// version, and auto-managed repositories (no pin) always use the latest. +#[cfg(feature = "enterprise")] +async fn check_dev_promotion_script_version<'a>( + db: &DB, + w_id: &str, + repos: impl Iterator, +) -> Result<()> { + let mut offending: Option = None; + for r in repos { + if !r.use_individual_branch.unwrap_or(false) { + continue; + } + if !r.is_script_meets_min_version(28796)? { + offending = Some(r.effective_script_path().to_string()); + break; + } + } + let Some(offending) = offending else { + return Ok(()); + }; + let is_dev = sqlx::query!( + "SELECT parent_workspace_id, is_dev_workspace FROM workspace WHERE id = $1", + w_id + ) + .fetch_optional(db) + .await? + .map(|r| r.is_dev_workspace) + .unwrap_or(false); + if !is_dev { + return Ok(()); + } + Err(Error::BadRequest(format!( + "Promotion mode on a dev workspace requires git sync script version 28796 or newer, \ + but this repository pins '{offending}'. Update the pinned sync script (or reset it to \ + auto-managed) first." + ))) +} + +/// A dev workspace's promotion must target its parent ("prod") workspace's own +/// git repository (same URL and branch) — that is what "promote to prod" means. +/// A fork-created dev inherits prod's repo; an **attached** dev keeps its own, +/// which may be unrelated. Reject enabling promotion on a repo the parent does +/// not track so the UI can't present an unrelated repo as prod's target. The +/// deploy path re-checks the same invariant (a resource edit could break it +/// after save), via the shared `dev_promotion_target_matches_parent`. +#[cfg(all(feature = "enterprise", feature = "private"))] +async fn check_dev_promotion_targets_parent_repo<'a>( + db: &DB, + w_id: &str, + repos: impl Iterator, +) -> Result<()> { + for r in repos.filter(|r| r.use_individual_branch.unwrap_or(false)) { + if !windmill_common::git_sync_ee::dev_promotion_target_matches_parent( + db, + w_id, + &r.git_repo_resource_path, + ) + .await? + { + return Err(Error::BadRequest( + "Promotion mode on a dev workspace must reuse the parent workspace's git repository \ + (same URL and branch), but this repository is not one the parent tracks — promotion \ + would target a repository the parent does not sync with." + .to_string(), + )); + } + } + Ok(()) +} + #[cfg(feature = "enterprise")] async fn check_git_sync_access(_db: &DB, _w_id: &str) -> Result<()> { Ok(()) @@ -3274,6 +3369,25 @@ async fn edit_git_sync_config( } #[cfg(feature = "enterprise")] check_open_prs_license(git_sync_settings.repositories.iter()).await?; + #[cfg(feature = "enterprise")] + check_promotion_license(git_sync_settings.repositories.iter()).await?; + #[cfg(feature = "enterprise")] + check_dev_promotion_script_version(&db, &w_id, git_sync_settings.repositories.iter()) + .await?; + #[cfg(all(feature = "enterprise", feature = "private"))] + check_dev_promotion_targets_parent_repo(&db, &w_id, git_sync_settings.repositories.iter()) + .await?; + // Promotion mode: EE only (mirrors edit_git_sync_repository). + #[cfg(not(feature = "enterprise"))] + if git_sync_settings + .repositories + .iter() + .any(|r| r.use_individual_branch.unwrap_or(false)) + { + return Err(Error::BadRequest( + "Promotion mode is an Enterprise Edition feature".to_string(), + )); + } // Preserve server-owned auto-pull state (webhook id/secret, synced sha, last // status) that the redacted GET response omits — otherwise a whole-config // save from the UI would drop the webhook secret (breaking delivery) or @@ -3486,6 +3600,13 @@ async fn edit_git_sync_repository( } #[cfg(feature = "enterprise")] check_open_prs_license(std::iter::once(&new_config.repository)).await?; + #[cfg(feature = "enterprise")] + check_promotion_license(std::iter::once(&new_config.repository)).await?; + #[cfg(feature = "enterprise")] + check_dev_promotion_script_version(&db, &w_id, std::iter::once(&new_config.repository)).await?; + #[cfg(all(feature = "enterprise", feature = "private"))] + check_dev_promotion_targets_parent_repo(&db, &w_id, std::iter::once(&new_config.repository)) + .await?; // Promotion mode: EE only #[cfg(not(feature = "enterprise"))] diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index f1679677fb..5be60363b7 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -167,7 +167,7 @@ pub enum ObjectType { DatatableMigration, } -pub const LATEST_GIT_SYNC_SCRIPT_PATH: &str = "hub/28790/sync-script-to-git-repo-windmill"; +pub const LATEST_GIT_SYNC_SCRIPT_PATH: &str = "hub/28796/sync-script-to-git-repo-windmill"; /// Hub script that applies a repository's state back into a workspace /// (the repo → Windmill / "pull" direction). Same script the UI runs from @@ -175,7 +175,7 @@ pub const LATEST_GIT_SYNC_SCRIPT_PATH: &str = "hub/28790/sync-script-to-git-repo /// ignores the slug, so the slug is kept free of characters that would be /// percent-encoded into the run URL (a `:` becomes `%3A`, which some hardened /// reverse proxies reject as double-encoding when the client re-encodes it). -pub const GIT_SYNC_PULL_SCRIPT_PATH: &str = "hub/28789/git-sync-init-repository-windmill"; +pub const GIT_SYNC_PULL_SCRIPT_PATH: &str = "hub/28795/git-sync-init-repository-windmill"; /// Prefix used to identify fork workspaces. A workspace whose id starts with this string is a /// fork of another workspace. diff --git a/backend/windmill-worker/src/result_processor.rs b/backend/windmill-worker/src/result_processor.rs index c2645f7cde..aeb9fdb329 100644 --- a/backend/windmill-worker/src/result_processor.rs +++ b/backend/windmill-worker/src/result_processor.rs @@ -908,8 +908,10 @@ async fn maybe_reconcile_git_sync_auto_pull( /// derivation: a dev workspace deploys to its environment-label branch /// (`dev`/`staging`), other fork workspaces to `wm-fork//`, /// else the promotion `wm_deploy/**` formula (per-folder or per-item form). -/// `None` when the deploy stays on the base branch (workspace-wide mode) and -/// has no PR to open. +/// A dev workspace in promotion mode is the exception: it takes the promotion +/// `wm_deploy/**` formula (per-item PRs into the parent) instead of its label +/// branch. `None` when the deploy stays on the base branch (workspace-wide +/// mode) and has no PR to open. #[cfg(all(feature = "enterprise", feature = "private"))] fn git_sync_deploy_pr_head_branch( workspace_id: &str, @@ -922,18 +924,23 @@ fn git_sync_deploy_pr_head_branch( item_parent_path: &str, path_type: &str, ) -> Option { - if dev_workspace_label.is_some() { - return Some(windmill_common::workspaces::dev_workspace_branch( - dev_workspace_label, - )); - } - let is_fork = parent_workspace_id.is_some() - || workspace_id.starts_with(windmill_common::workspaces::WM_FORK_PREFIX); - if is_fork { - let suffix = workspace_id - .strip_prefix(windmill_common::workspaces::WM_FORK_PREFIX) - .unwrap_or(workspace_id); - return Some(format!("wm-fork/{base}/{suffix}")); + let is_dev = dev_workspace_label.is_some(); + // A dev workspace with promotion on falls through to the wm_deploy/** + // formula below; the label/fork branches only apply when promotion is off. + if !(is_dev && use_individual_branch) { + if is_dev { + return Some(windmill_common::workspaces::dev_workspace_branch( + dev_workspace_label, + )); + } + let is_fork = parent_workspace_id.is_some() + || workspace_id.starts_with(windmill_common::workspaces::WM_FORK_PREFIX); + if is_fork { + let suffix = workspace_id + .strip_prefix(windmill_common::workspaces::WM_FORK_PREFIX) + .unwrap_or(workspace_id); + return Some(format!("wm-fork/{base}/{suffix}")); + } } if !use_individual_branch { return None; @@ -2220,4 +2227,77 @@ mod git_sync_pr_tests { Some("dev".to_string()) ); } + + #[test] + fn dev_workspace_promotion_uses_wm_deploy_branch() { + // Promotion on: a dev workspace gets per-item wm_deploy/** branches + // (namespaced by its own id), not its env-label branch. + assert_eq!( + git_sync_deploy_pr_head_branch( + "dev", + Some("prod"), + Some("dev"), + "main", + true, + false, + "f/folder/my_script", + "", + "script" + ), + Some("wm_deploy/dev/script/f__folder__my_script".to_string()) + ); + // Per-folder form still honored for a promotion dev workspace. + assert_eq!( + git_sync_deploy_pr_head_branch( + "dev", + Some("prod"), + Some("dev"), + "main", + true, + true, + "f/folder/my_script", + "", + "script" + ), + Some("wm_deploy/dev/f__folder".to_string()) + ); + // Promotion off: the env-label branch still wins. + assert_eq!( + git_sync_deploy_pr_head_branch( + "dev", + Some("prod"), + Some("dev"), + "main", + false, + false, + "f/x/y", + "", + "script" + ), + Some("dev".to_string()) + ); + } + + #[test] + fn dev_promotion_user_group_items_open_no_pr() { + // User/group objects get no wm_deploy branch even on a dev workspace; the + // CLI isolates them to the env-label branch, so the backend opens no PR + // (never a PR from the env-label branch into the parent for these). + for path_type in ["user", "group"] { + assert_eq!( + git_sync_deploy_pr_head_branch( + "dev", + Some("prod"), + Some("dev"), + "main", + true, + false, + "u/alice", + "", + path_type + ), + None + ); + } + } } diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index 29d83862e4..2ddb4833b8 100644 --- a/cli/src/commands/sync/sync.ts +++ b/cli/src/commands/sync/sync.ts @@ -2865,17 +2865,20 @@ export async function pull( } const clonedBranchName = getCurrentGitBranch() ?? "main"; - // Fork / dev workspaces force-disable use_individual_branch / group_by_folder - // (1:1 with the hub script's inner()). Dev workspaces have a prefix-less id, so - // detect them via the parent-workspace id the backend passes. + // Throwaway forks force-disable use_individual_branch / group_by_folder + // (1:1 with the hub script's inner()). A dev workspace is the exception: it + // honors promotion mode and gets per-item wm_deploy/** branches. Dev + // workspaces have a prefix-less id, so detect them via the environment label + // the backend passes with the deploy. const targetIsFork = isForkWorkspace( workspace.workspaceId, opts.parentWorkspaceId, ); - const useIndividualBranch = targetIsFork + const forceOffPromotion = targetIsFork && !opts.devWorkspaceLabel; + const useIndividualBranch = forceOffPromotion ? false : !!opts.useIndividualBranch; - const groupByFolder = targetIsFork ? false : !!opts.groupByFolder; + const groupByFolder = forceOffPromotion ? false : !!opts.groupByFolder; // Fork-of-a-fork: when the parent workspace is itself a fork, root the new // branch on the parent's fork branch (the content this fork diverged from). @@ -3447,13 +3450,14 @@ export async function gitDeploy( } } - // Fork / dev workspaces force-disable use_individual_branch / group_by_folder - // (1:1 with the hub script's inner()): they always sync to their own - // wm-fork// branch, and — critically — that disabling also - // flips the include/promotion derivation below. Dev workspaces have a - // prefix-less id, so detect them via the parent-workspace id too. + // Throwaway forks force-disable use_individual_branch / group_by_folder (1:1 + // with the hub script's inner()): they always sync to their own + // wm-fork// branch, and — critically — that disabling also flips + // the include/promotion derivation below. A dev workspace is the exception: it + // honors promotion mode, detected via the environment label the backend passes. const isFork = isForkWorkspace(opts.workspace ?? "", opts.parentWorkspaceId); - const useIndividualBranch = isFork ? false : !!opts.useIndividualBranch; + const useIndividualBranch = + isFork && !opts.devWorkspaceLabel ? false : !!opts.useIndividualBranch; // Derive the include filters from the deployed items (replaces the hub // script's regexFromPath + per-kind --include-* construction). diff --git a/cli/src/utils/git.ts b/cli/src/utils/git.ts index 441133afe4..da1ac14414 100644 --- a/cli/src/utils/git.ts +++ b/cli/src/utils/git.ts @@ -107,10 +107,12 @@ export interface GitSyncDeployItem { commit_msg?: string; } -// A fork or dev workspace syncs to its own wm-fork// branch. The hub -// script force-disables use_individual_branch / group_by_folder for these (that -// disabling also changes the include/promotion derivation — so callers must apply -// it BEFORE deriving includes, not just for branch naming). +// A throwaway fork syncs to its own wm-fork// branch. The hub script +// force-disables use_individual_branch / group_by_folder for these (that disabling +// also changes the include/promotion derivation — so callers must apply it BEFORE +// deriving includes, not just for branch naming). A dev workspace is the exception: +// with promotion on it keeps use_individual_branch / group_by_folder and gets +// per-item wm_deploy/** branches like a root workspace. // // Fork-ness is "has a parent workspace" OR the "wm-fork-" id prefix. Regular forks // get an auto-generated `wm-fork-` id, but dev workspaces keep a custom id @@ -165,11 +167,28 @@ export function computeGitSyncDeployBranch(params: { clonedBranchName, } = params; - if (isForkWorkspace(workspaceId, parentWorkspaceId)) { + // A dev workspace in promotion mode falls through to the wm_deploy/** formula + // below (per-item/-folder PRs that promote into its parent). Throwaway forks, + // and dev workspaces with promotion off, sync to their own wm-fork// + // (or env-label) branch. + const isDevWorkspace = !!devWorkspaceLabel; + if ( + isForkWorkspace(workspaceId, parentWorkspaceId) && + !(isDevWorkspace && useIndividualBranch) + ) { return forkBranchName(workspaceId, clonedBranchName, devWorkspaceLabel); } - if (items.length === 0) return null; + // A dev workspace's deploys must never fall through to the base branch — that + // is its parent's tracked branch, so a null here would push dev content + // straight to prod. Anything without its own wm_deploy/** branch (user/group + // objects, an unresolvable ref) goes to the dev's env-label branch instead. + // A root workspace has no such isolation, so its fallback stays null (base). + const fallback = isDevWorkspace + ? forkBranchName(workspaceId, clonedBranchName, devWorkspaceLabel) + : null; + + if (items.length === 0) return fallback; const first = items[0]; // `use_individual_branch` disables debouncing, so items is length 1 here. @@ -178,11 +197,16 @@ export function computeGitSyncDeployBranch(params: { first.path_type === "user" || first.path_type === "group" ) { - return null; + return fallback; } - const ref = first.path ?? first.parent_path; - if (!ref) return null; + // `||` not `??`: the backend serializes a path that no longer matches the repo + // filter (a rename out of the included set) as "" with the old path in + // parent_path. That "" must fall back to parent_path — mirroring the backend's + // `!item_path.is_empty()` derivation. `??` would keep "", return null, and skip + // the branch checkout, letting the removal land on the tracked base branch. + const ref = first.path || first.parent_path; + if (!ref) return fallback; return groupByFolder ? `wm_deploy/${workspaceId}/${ref.split("/").slice(0, 2).join("__")}` diff --git a/cli/test/git_unit.test.ts b/cli/test/git_unit.test.ts index 2db2834bd2..9665b8581a 100644 --- a/cli/test/git_unit.test.ts +++ b/cli/test/git_unit.test.ts @@ -117,6 +117,50 @@ describe("computeGitSyncDeployBranch", () => { ).toBe("staging"); }); + test("dev workspace in promotion mode -> per-item wm_deploy branch, not the label branch", () => { + expect( + computeGitSyncDeployBranch({ + ...base, + workspaceId: "staging-ws", + parentWorkspaceId: "prod", + devWorkspaceLabel: "staging", + useIndividualBranch: true, + items: [{ path_type: "script", path: "f/foo/bar" }], + }) + ).toBe("wm_deploy/staging-ws/script/f__foo__bar"); + }); + + test("dev promotion user/group objects go to the env-label branch, never the base", () => { + // Non-branchable objects must not fall through to null (= the parent's + // tracked branch) on a dev workspace — that would push dev content to prod. + for (const path_type of ["user", "group"]) { + expect( + computeGitSyncDeployBranch({ + ...base, + workspaceId: "staging-ws", + parentWorkspaceId: "prod", + devWorkspaceLabel: "staging", + useIndividualBranch: true, + items: [{ path_type, path: "u/alice", parent_path: null }], + }) + ).toBe("staging"); + } + }); + + test("dev workspace in promotion mode honors group_by_folder", () => { + expect( + computeGitSyncDeployBranch({ + ...base, + workspaceId: "staging-ws", + parentWorkspaceId: "prod", + devWorkspaceLabel: "staging", + useIndividualBranch: true, + groupByFolder: true, + items: [{ path_type: "script", path: "f/foo/bar" }], + }) + ).toBe("wm_deploy/staging-ws/f__foo"); + }); + test("use_individual_branch=false -> null (stay on base/main, workspace-wide mode)", () => { expect( computeGitSyncDeployBranch({ @@ -148,6 +192,21 @@ describe("computeGitSyncDeployBranch", () => { ).toBe("wm_deploy/prod/flow/f__x__y"); }); + test("falls back to parent_path when the backend serializes path as \"\" (rename out of filter)", () => { + // The backend emits "" (not null) for a path that no longer matches the repo + // filter; it must still get its own branch, not fall through to the base. + expect( + computeGitSyncDeployBranch({ + ...base, + workspaceId: "staging-ws", + parentWorkspaceId: "prod", + devWorkspaceLabel: "staging", + useIndividualBranch: true, + items: [{ path_type: "resource", path: "", parent_path: "f/folder/old" }], + }) + ).toBe("wm_deploy/staging-ws/resource/f__folder__old"); + }); + test("user/group objects never get a dedicated branch", () => { expect( computeGitSyncDeployBranch({ diff --git a/cli/test/gitsync_promotion.test.ts b/cli/test/gitsync_promotion.test.ts index 3d362442ac..1eee7508b0 100644 --- a/cli/test/gitsync_promotion.test.ts +++ b/cli/test/gitsync_promotion.test.ts @@ -231,6 +231,138 @@ test.skipIf(shouldSkipOnCI())( }, ); +/** + * Dev-workspace promotion. The backend passes `--dev-workspace-label` + + * `--parent-workspace-id` for a dev workspace's deploys; with promotion on it + * must behave like a root (per-item `wm_deploy/**`) for branchable objects, and + * — critically — non-branchable objects (user/group) must fall back to the dev's + * env-label branch, NEVER the cloned base (which for a dev is the parent's + * tracked branch, so a leak would push dev content straight to prod). + */ +test.skipIf(shouldSkipOnCI())( + "git-sync promotion (dev workspace): script -> wm_deploy branch; group -> env-label branch; main never touched", + async () => { + await withTestBackend(async (backend) => { + const ws = backend.workspace; // "test" + await addWorkspace( + { + remote: backend.baseUrl, + workspaceId: ws, + name: ws, + token: backend.token, + } as any, + { force: true, configDir: backend.testConfigDir }, + ); + + const bareDir = await mkdtemp(join(tmpdir(), "wmill_devpromo_bare_")); + execFileSync("git", ["init", "--bare", "--initial-branch=main", bareDir]); + const seedDir = await mkdtemp(join(tmpdir(), "wmill_devpromo_seed_")); + git(seedDir, "init", "--initial-branch=main"); + git(seedDir, "config", "user.email", "seed@windmill.dev"); + git(seedDir, "config", "user.name", "seed"); + await writeFile(join(seedDir, "README.md"), "# dev promo\n"); + git(seedDir, "add", "-A"); + git(seedDir, "commit", "-m", "seed"); + git(seedDir, "remote", "add", "origin", `file://${bareDir}`); + git(seedDir, "push", "-u", "origin", "main"); + const seedMain = remoteHead(bareDir, "main"); + + await backend.apiRequest!(`/api/w/${ws}/folders/create`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ name: "promo", owners: [], extra_perms: {} }), + }); + await backend.apiRequest!(`/api/w/${ws}/scripts/create`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + path: "f/promo/foo", + summary: "", + description: "", + content: "export async function main() { return 1 }", + language: "bun", + }), + }); + await backend.apiRequest!(`/api/w/${ws}/resources/create`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + path: "u/test/devpromo_repo", + resource_type: "git_repository", + value: { url: `file://${bareDir}`, branch: "main", token: "" }, + }), + }); + await backend.updateGitSyncConfig!({ + git_sync_settings: { + repositories: [ + { + git_repo_resource_path: "u/test/devpromo_repo", + script_path: "f/**", + use_individual_branch: true, + group_by_folder: false, + settings: { include_path: ["f/**", "g/**"], include_type: ["script", "group"] }, + }, + ], + }, + }); + + // The dev flags the backend passes with every dev-workspace deploy. + const devFlags = ["--dev-workspace-label", "dev", "--parent-workspace-id", "prod"]; + const commitAndPush = (work: string) => { + git(work, "config", "user.email", "test@windmill.dev"); + git(work, "config", "user.name", "test"); + git(work, "add", "-A"); + try { + git(work, "diff", "--cached", "--quiet"); + } catch { + git(work, "commit", "-m", "deploy"); + } + git(work, "push", "--porcelain", "-u", "origin", "HEAD"); + }; + + // Script: gets its own wm_deploy branch (namespaced by the dev's id), main untouched. + const workS = await mkdtemp(join(tmpdir(), "wmill_devpromo_s_")); + git(workS, "clone", `file://${bareDir}`, "."); + await writeFile(join(workS, "wmill.yaml"), "defaultTs: bun\nincludes:\n - f/**\nexcludes: []\n"); + const resS = await backend.runCLICommand( + [ + "sync", "git-deploy", "--repository", "u/test/devpromo_repo", + "--use-individual-branch", ...devFlags, + "--git-deploy-items", JSON.stringify([{ path_type: "script", path: "f/promo/foo", commit_msg: "deploy foo" }]), + ], + workS, + ); + expect(resS.code).toBe(0); + commitAndPush(workS); + expect(remoteBranches(bareDir)).toContain(`refs/heads/wm_deploy/${ws}/script/f__promo__foo`); + expect(remoteHead(bareDir, "main")).toBe(seedMain); + + // Group (non-branchable): must land on the dev env-label branch, NEVER main. + const workG = await mkdtemp(join(tmpdir(), "wmill_devpromo_g_")); + git(workG, "clone", `file://${bareDir}`, "."); + await writeFile(join(workG, "wmill.yaml"), "defaultTs: bun\nincludes:\n - f/**\n - g/**\nexcludes: []\n"); + const resG = await backend.runCLICommand( + [ + "sync", "git-deploy", "--repository", "u/test/devpromo_repo", + "--use-individual-branch", ...devFlags, + "--git-deploy-items", JSON.stringify([{ path_type: "group", path: "g/all", commit_msg: "deploy group" }]), + ], + workG, + ); + expect(resG.code).toBe(0); + commitAndPush(workG); + // The anti-leak regression: a group deploy must not advance main. + expect(remoteHead(bareDir, "main")).toBe(seedMain); + expect(remoteBranches(bareDir)).toContain("refs/heads/dev"); + + await rm(bareDir, { recursive: true, force: true }); + await rm(seedDir, { recursive: true, force: true }); + await rm(workS, { recursive: true, force: true }); + await rm(workG, { recursive: true, force: true }); + }); + }, +); + /** * Regression test for the promotion trigger-include bug (fix/gitsync-promotion- * trigger-export): deploying a trigger (or any excluded-by-default kind: diff --git a/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte b/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte index 06d28969ba..30a8c3ef47 100644 --- a/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte +++ b/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte @@ -33,7 +33,8 @@ repository = null, onAdd = null, isCollapsible = true, - showEmptyState = false + showEmptyState = false, + devPromotion = false } = $props<{ idx?: number | null isSecondary?: boolean @@ -44,6 +45,9 @@ onAdd?: (() => void) | null isCollapsible?: boolean showEmptyState?: boolean + // Dev workspace: this is the single inherited repo, and promotion is a + // toggle on it (reuse prod's repo) rather than a separately-configured one. + devPromotion?: boolean }>() const gitSyncContext = getGitSyncContext() @@ -78,6 +82,9 @@ ($workspaceStore?.startsWith('wm-fork-') ?? false) || !!currentWorkspaceData?.parent_workspace_id ) + // A dev workspace is a fork that DOES run promotion mode (per-item + // wm_deploy/** branches into its parent), unlike a throwaway fork. + const isDevWorkspace = $derived(!!currentWorkspaceData?.is_dev_workspace) function setSyncForks(v: boolean) { if (repo?.auto_pull) repo.auto_pull = { ...repo.auto_pull, sync_forks: v } } @@ -87,6 +94,45 @@ function setPromotionOpenPrs(v: boolean) { if (repo) repo.promotion_open_prs = v } + // The promotion toggles persist immediately and must not overlap: concurrent + // whole-repository saves can complete out of order (enabling runs extra + // backend checks), letting a stale earlier state overwrite the latest one. + let savingDevPromotion = $state(false) + async function setDevPromotion(v: boolean) { + if (!repo || idx === null || savingDevPromotion) return + const prevIndiv = repo.use_individual_branch + const prevGbf = repo.group_by_folder + repo.use_individual_branch = v + if (!v) repo.group_by_folder = false + savingDevPromotion = true + try { + await gitSyncContext.saveRepository(idx) + } catch (e) { + // The backend rejects promotion mode without an active EE plan; revert + // the optimistic toggle instead of leaving it stuck on until reload. + if (repo) { + repo.use_individual_branch = prevIndiv + repo.group_by_folder = prevGbf + } + sendUserToast(`Could not ${v ? 'enable' : 'disable'} Git promotion: ${e}`, true) + } finally { + savingDevPromotion = false + } + } + async function setGroupByFolder(v: boolean) { + if (!repo || idx === null || savingDevPromotion) return + const prev = repo.group_by_folder + repo.group_by_folder = v + savingDevPromotion = true + try { + await gitSyncContext.saveRepository(idx) + } catch (e) { + if (repo) repo.group_by_folder = prev + sendUserToast(`Could not change promotion granularity: ${e}`, true) + } finally { + savingDevPromotion = false + } + } let targetBranch = $state(undefined) // Default to main, will be updated when resource is available // The branch this fork workspace syncs with, mirroring the CLI/hub-script @@ -580,7 +626,7 @@
Push to Git on deploy (Windmill → Git)
- {#if !isFork} + {#if !isFork || (isDevWorkspace && repoMode === 'promotion')} {/if} @@ -593,10 +639,42 @@ Push to repo - {#if repoMode === 'promotion' && isGithubApp} + {#if devPromotion && !repo.isUnsavedConnection}
+ setDevPromotion(e.detail)} + /> + {#if repo.use_individual_branch} +
+ setGroupByFolder(e.detail)} + /> +
+ {/if} +
+ {/if} + {#if repoMode === 'promotion' && isGithubApp} +
+ setForkOpenPrs(e.detail)} > @@ -749,22 +827,22 @@ {#if !isGithubApp && !loadingResourceInfo}
- Pull for this repository checks the tracked branch about every minute; - longer gaps make drift and merge conflicts more likely. For instant pull, - connect the repository through the + Pull for this repository checks the tracked branch about every minute; longer + gaps make drift and merge conflicts more likely. For instant pull, connect the + repository through the GitHub App - (which also lets Windmill manage pull requests), or push changes into - Windmill with the + (which also lets Windmill manage pull requests), or push changes into Windmill + with the sync GitHub workflow. If you already push changes with a GitHub Action, keep either the - Action or automatic pull, not both, so they don't fight over deploys. + >. If you already push changes with a GitHub Action, keep either the Action or + automatic pull, not both, so they don't fight over deploys.
{/if} diff --git a/frontend/src/lib/components/git_sync/GitSyncSection.svelte b/frontend/src/lib/components/git_sync/GitSyncSection.svelte index 42e147453c..3e5fe7c70d 100644 --- a/frontend/src/lib/components/git_sync/GitSyncSection.svelte +++ b/frontend/src/lib/components/git_sync/GitSyncSection.svelte @@ -42,13 +42,17 @@ const gitSyncAllowed = $derived(gitSyncStatus.enabled) const isFreeTier = $derived(gitSyncAllowed && !$enterpriseLicense) - // Fork/dev workspaces never run promotion mode: their deploys always go to - // the fork's own wm-fork/** branch, so a promotion repo could never take - // effect (the backend rejects it too). Mirrors the backend/CLI fork rule. + // Throwaway forks never run promotion mode: their deploys always go to the + // fork's own wm-fork/** branch, so a promotion repo could never take effect + // (the backend rejects it too). A dev workspace is the exception — it deploys + // per-item wm_deploy/** branches that promote into its parent. Mirrors the + // backend/CLI rule. + const currentWorkspace = $derived($userWorkspaces?.find((w) => w.id === $workspaceStore)) const isFork = $derived( - ($workspaceStore?.startsWith('wm-fork-') ?? false) || - !!$userWorkspaces?.find((w) => w.id === $workspaceStore)?.parent_workspace_id + ($workspaceStore?.startsWith('wm-fork-') ?? false) || !!currentWorkspace?.parent_workspace_id ) + const isDevWorkspace = $derived(!!currentWorkspace?.is_dev_workspace) + const showPromotion = $derived(!isFork || isDevWorkspace) const hasConfiguredRepos = $derived( gitSyncContext?.repositories?.some((r) => r.git_repo_resource_path) ?? false ) @@ -70,6 +74,16 @@ // Derived state for repository categorization const primarySync = $derived(gitSyncContext?.getPrimarySyncRepository() || null) const primaryPromotion = $derived(gitSyncContext?.getPrimaryPromotionRepository() || null) + // A dev workspace reuses the single repo it inherited from prod: whether it's + // currently in sync or promotion mode, it's the same one card, toggled between + // the two — so a dev never configures a separate promotion repo. + const devPrimaryRepo = $derived(isDevWorkspace ? (primarySync ?? primaryPromotion) : null) + // The single-repo dev UX (one card + promotion toggle, secondaries hidden) is + // only safe when the dev actually has one repo — i.e. it inherited prod's on + // fork. An ATTACHED dev keeps its own repos: with more than one, fall back to + // the normal layout so none are hidden and we don't present an unrelated repo + // as prod's promotion target. + const devSingleRepo = $derived(isDevWorkspace && (gitSyncContext?.repositories?.length ?? 0) <= 1) const secondarySync = $derived(gitSyncContext?.getSecondarySyncRepositories() || []) const secondaryPromotion = $derived(gitSyncContext?.getSecondaryPromotionRepositories() || []) @@ -139,17 +153,18 @@
gitSyncContext.addSyncRepository()} isCollapsible={false} - showEmptyState={primarySync?.repo === null} + showEmptyState={(devSingleRepo ? devPrimaryRepo : primarySync)?.repo == null} + devPromotion={devSingleRepo && !!$enterpriseLicense} /> {#if $enterpriseLicense} - - {#if primarySync && !primarySync.repo?.isUnsavedConnection} + + {#if primarySync && !primarySync.repo?.isUnsavedConnection && !devSingleRepo} {#if secondarySync.length > 0 || secondarySyncExpanded}