mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-19 00:02:03 +00:00
fix(git-sync): EE-gate auto-pull UI, fork pull clone_ref, no-op push PR gate
- CE: the auto-pull and fork-PR toggles are disabled with an EE badge, and new sync repos only default them on when licensed (basic git sync is available on CE since #8493, but auto-pull is EE and the backend rejects it) - The pull modal passes clone_ref for wm-fork- forks (wm-fork/<tracked>/<id>) so a manual pull fetches the fork branch instead of the tracked branch head - PR-on-deploy skips no-op pushes: when the push script reports pushed=false (e.g. the deploy was caused by an auto-pull), the completion hook no longer ensures a PR, so closed PRs aren't recreated by the sync loop Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm
This commit is contained in:
@@ -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<bool> {
|
||||
serde_json::from_str::<serde_json::Value>(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/<base>/<id>` 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() {
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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: '',
|
||||
|
||||
@@ -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}
|
||||
<Toggle
|
||||
checked={repo.auto_pull?.enabled ?? false}
|
||||
disabled={!$enterpriseLicense}
|
||||
eeOnly
|
||||
options={{
|
||||
right: 'Automatically deploy changes from Git',
|
||||
rightTooltip:
|
||||
@@ -721,6 +723,8 @@
|
||||
{#if isGithubApp}
|
||||
<Toggle
|
||||
checked={repo.fork_open_prs ?? false}
|
||||
disabled={!$enterpriseLicense}
|
||||
eeOnly
|
||||
options={{
|
||||
right: 'Open a pull request when a fork deploys',
|
||||
rightTooltip:
|
||||
|
||||
@@ -12,7 +12,7 @@
|
||||
Edit3
|
||||
} from 'lucide-svelte'
|
||||
import GitDiffPreview from '../GitDiffPreview.svelte'
|
||||
import { JobService } from '$lib/gen'
|
||||
import { JobService, ResourceService } from '$lib/gen'
|
||||
import { workspaceStore, userWorkspaces } from '$lib/stores'
|
||||
import { sendUserToast } from '$lib/toast'
|
||||
import hubPaths from '$lib/hubPaths.json'
|
||||
@@ -147,9 +147,29 @@
|
||||
const workspace = $workspaceStore
|
||||
if (!workspace) return
|
||||
|
||||
// A dev workspace pulls from its environment-label branch (dev/staging),
|
||||
// not the resource's tracked branch.
|
||||
// A dev workspace pulls from its environment-label branch (dev/staging)
|
||||
// and a fork from its wm-fork/<tracked>/<id> 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({
|
||||
|
||||
Reference in New Issue
Block a user