diff --git a/backend/windmill-worker/src/result_processor.rs b/backend/windmill-worker/src/result_processor.rs index 0e1d06c9fd..437d4c33b3 100644 --- a/backend/windmill-worker/src/result_processor.rs +++ b/backend/windmill-worker/src/result_processor.rs @@ -942,13 +942,34 @@ fn git_sync_deploy_pr_head_branch( }) } +/// Whether the push job's result says a commit was actually pushed. `None` +/// when the result doesn't carry the flag (hub script versions predating it). +#[cfg(all(feature = "enterprise", feature = "private"))] +fn git_sync_push_result_pushed(result: &str) -> Option { + serde_json::from_str::(result) + .ok()? + .get("pushed")? + .as_bool() +} + /// When a git-sync push job carrying `__git_sync_open_pr` succeeds, open (or /// reopen) the PR for the branch it pushed: `wm-fork//` for a fork /// deploy, `wm_deploy/**` for a promotion deploy. Runs outbound with the /// installation token, so it works regardless of webhook reachability. /// Best-effort: failures are logged, never propagated. #[cfg(all(feature = "enterprise", feature = "private"))] -async fn maybe_open_git_sync_deploy_pr(db: &DB, job_id: &uuid::Uuid, workspace_id: &str) { +async fn maybe_open_git_sync_deploy_pr( + db: &DB, + job_id: &uuid::Uuid, + workspace_id: &str, + result: &str, +) { + // A no-op push (workspace already matches the repo โ€” e.g. the deploy was + // itself caused by an auto-pull) must not ensure a PR: it would recreate + // PRs the user closed and spam creation attempts with no diff. + if git_sync_push_result_pushed(result) == Some(false) { + return; + } let row = match sqlx::query!( r#"SELECT args->'__git_sync_open_pr' as "marker", @@ -1300,7 +1321,7 @@ pub async fn process_completed_job( #[cfg(all(feature = "enterprise", feature = "private"))] if job.kind == JobKind::DeploymentCallback { maybe_post_git_sync_check(db, &job_id, &workspace_id, true, result.get()).await; - maybe_open_git_sync_deploy_pr(db, &job_id, &workspace_id).await; + maybe_open_git_sync_deploy_pr(db, &job_id, &workspace_id, result.get()).await; } // Asset-trigger fan-out: best-effort, never propagates errors. @@ -1843,7 +1864,23 @@ pub fn extract_error_value( #[cfg(all(test, feature = "enterprise", feature = "private"))] mod git_sync_pr_tests { - use super::git_sync_deploy_pr_head_branch; + use super::{git_sync_deploy_pr_head_branch, git_sync_push_result_pushed}; + + #[test] + fn push_result_pushed_flag() { + assert_eq!( + git_sync_push_result_pushed(r#"{"pushed": true}"#), + Some(true) + ); + assert_eq!( + git_sync_push_result_pushed(r#"{"pushed": false}"#), + Some(false) + ); + // Older hub script versions return null / no flag: undetermined. + assert_eq!(git_sync_push_result_pushed("null"), None); + assert_eq!(git_sync_push_result_pushed(r#"{"other": 1}"#), None); + assert_eq!(git_sync_push_result_pushed("not json"), None); + } #[test] fn fork_branch_wins_and_strips_the_id_prefix() { diff --git a/docs/git-sync-pull-design.md b/docs/git-sync-pull-design.md index 2be120d0c0..8961b0f047 100644 --- a/docs/git-sync-pull-design.md +++ b/docs/git-sync-pull-design.md @@ -453,6 +453,11 @@ Frontend: branch (fork branch wins, else the `wm_deploy/**` formula) and calls `ensure_pull_request`. Outbound with the installation token, so no webhook reachability is needed; app-backed repos only. +- No-op pushes skip PR creation: the push script reports `pushed: false` when + nothing was committed (e.g. the deploy was itself caused by an auto-pull, so + the workspace already matches the repo), and the hook returns early โ€” a PR + the user closed isn't recreated by the sync loop. Results without the flag + (older script pins) keep ensuring the PR. - Fork-branch routing edge cases (ยง7) hardened here. ### Phase 4 โ€” PR diff preview checks (optional) diff --git a/frontend/src/lib/components/git_sync/GitSyncContext.svelte.ts b/frontend/src/lib/components/git_sync/GitSyncContext.svelte.ts index 0f77de5504..a015a9db5a 100644 --- a/frontend/src/lib/components/git_sync/GitSyncContext.svelte.ts +++ b/frontend/src/lib/components/git_sync/GitSyncContext.svelte.ts @@ -686,9 +686,14 @@ export function createGitSyncContext(workspace: string) { collapsed: false, // New connections default to pulling changes from Git (webhook with a // polling fallback), forks included. Existing repos load without - // auto_pull and stay off. - auto_pull: { enabled: true, mode: 'auto', sync_forks: true }, - fork_open_prs: true + // auto_pull and stay off. Auto-pull is EE-only (the backend rejects an + // enabled setting on CE), so only default it on when licensed. + ...(get(enterpriseLicense) + ? { + auto_pull: { enabled: true, mode: 'auto', sync_forks: true }, + fork_open_prs: true + } + : {}) }) gitSyncTestJobs.push({ jobId: '', diff --git a/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte b/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte index 729687bb45..4add84738c 100644 --- a/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte +++ b/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte @@ -17,7 +17,7 @@ import DetectionFlow from './DetectionFlow.svelte' import { sendUserToast } from '$lib/toast' import { fade } from 'svelte/transition' - import { workspaceStore, userWorkspaces } from '$lib/stores' + import { workspaceStore, userWorkspaces, enterpriseLicense } from '$lib/stores' import type { GitSyncRepository } from './GitSyncContext.svelte' import GitSyncModeDisplay from './GitSyncModeDisplay.svelte' import Toggle from '$lib/components/Toggle.svelte' @@ -632,6 +632,8 @@ {:else} / branch, not the resource's + // tracked branch. clone_ref falls back to the tracked branch in the + // pull script when the override branch doesn't exist yet. const currentWs = $userWorkspaces?.find((w) => w.id === workspace) + const isFork = workspace.startsWith('wm-fork-') || Boolean(currentWs?.parent_workspace_id) + let cloneRef: string | undefined = undefined + if (currentWs?.is_dev_workspace) { + cloneRef = currentWs.dev_workspace_label ?? 'dev' + } else if (isFork) { + try { + const resource = await ResourceService.getResource({ + workspace, + path: gitRepoResourcePath + }) + const trackedBranch = (resource.value as any)?.branch + if (trackedBranch) { + cloneRef = `wm-fork/${trackedBranch}/${workspace.replace(/^wm-fork-/, '')}` + } + } catch (e) { + console.warn('Could not resolve tracked branch for fork pull:', e) + } + } const payload = { workspace_id: workspace, repo_url_resource_path: gitRepoResourcePath, @@ -159,9 +179,7 @@ settings_json: JSON.stringify(uiState), use_promotion_overrides: currentGitSyncSettings?.repositories?.[repoIndex!]?.use_individual_branch === true, - ...(currentWs?.is_dev_workspace - ? { clone_ref: currentWs.dev_workspace_label ?? 'dev' } - : {}) + ...(cloneRef ? { clone_ref: cloneRef } : {}) } const jobId = await JobService.runScriptByPath({