starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-08-21 08:02:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(git-sync): PR deploy-preview comment, clearer check copy, app-only hints

Browse Source 
- The PR diff completion hook maintains one managed comment on the PR
  (Cloudflare deploy-preview style: workspace, status, commit, collapsible
  change list), upserted per synchronize via a hidden marker. The check run
  stays for required-check gating.
- A settings difference in the diff summary is worded by cause: the PR
  changes wmill.yaml, vs pre-existing drift between the repo's wmill.yaml
  and the workspace, vs undetermined (neutral wording).
- Deploy-status check titles name the target workspace ("Deployed 2
  change(s) to staging"), since GitHub shows a head commit's checks on any
  PR containing it and a bare "Deployed" read as if the PR had deployed.
- Token-based repos see a hint pointing at the open-pr-on-commit /
  open-pr-on-fork-commit workflows where the app-only PR toggles would be;
  an API-set toggle on a non-app repo now logs a warning naming the
  fallback; the design doc lists app-only features and their degradation.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm
This commit is contained in:
hugocasa
2026-07-08 09:56:49 +02:00
parent c407d2cc48
commit cc552f06e2
4 changed files with 81 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/ee-repo-ref.txt
+1 -1
View File
@@ -1 +1 @@
a4f14db140b094e62514a6ed0d92f6f4daf2f8c6
0a527a5378ef1781fd691d5d27669e63580c6eff
backend/windmill-worker/src/result_processor.rs
+48 -7
View File
@@ -730,6 +730,14 @@ pub async fn handle_receive_completed_job(
struct GitSyncCheck {
check_run_id: i64,
repo_url: String,
#[serde(default)]
pr_number: Option<i64>,
#[serde(default)]
head_sha: Option<String>,
/// Whether the PR itself modifies wmill.yaml (None = undetermined); picks
/// the wording for a settings difference in the diff summary.
#[serde(default)]
wmill_yaml_changed: Option<bool>,
}
/// Parsed diff summary from a (dry-run or real) pull result. `None` when the
@@ -985,8 +993,8 @@ async fn maybe_open_git_sync_deploy_pr(db: &DB, job_id: &uuid::Uuid, workspace_i
{
Ok(Some((branch, _))) => branch,
Ok(None) => {
tracing::debug!(
"git sync PR: repo {repo_path} in {workspace_id} is not app-backed, skipping"
tracing::warn!(
"git sync PR: repo {repo_path} in {workspace_id} has a PR-on-deploy toggle set but is not GitHub-App-backed; skipping (connect the repo through the GitHub App, or use the open-pr-on-commit workflow)"
);
return;
}
@@ -1079,7 +1087,7 @@ async fn maybe_post_git_sync_check(
if !success {
(
"failure",
"Deploy failed".to_string(),
format!("Deploy to {} failed", workspace_id),
"Deploying the latest commit failed. See the job in Windmill for details."
.to_string(),
)
@@ -1087,7 +1095,7 @@ async fn maybe_post_git_sync_check(
match parse_git_sync_changes(result_raw) {
Some((changes, settings_changed)) if changes.is_empty() && !settings_changed => (
"success",
"In sync".to_string(),
format!("In sync with {}", workspace_id),
format!(
"No changes to deploy to `{}` from this commit.",
workspace_id
@@ -1105,13 +1113,13 @@ async fn maybe_post_git_sync_check(
}
(
"success",
format!("Deployed {} change(s)", changes.len()),
format!("Deployed {} change(s) to {}", changes.len(), workspace_id),
lines.join("\n"),
)
}
None => (
"success",
"Deployed".to_string(),
format!("Deployed to {}", workspace_id),
format!("Windmill deployed the latest commit to `{}`.", workspace_id),
),
}
@@ -1139,7 +1147,11 @@ async fn maybe_post_git_sync_check(
)];
lines.extend(format_change_list(&changes));
if settings_changed {
lines.push("\nWorkspace settings would also change.".to_string());
lines.push(match check.wmill_yaml_changed {
Some(true) => "\nThis PR changes wmill.yaml: pulling also applies the updated workspace settings.".to_string(),
Some(false) => "\nIndependent of this PR, the workspace's git-sync settings differ from the repo's wmill.yaml and a pull updates them to match.".to_string(),
None => "\nA pull also updates the workspace's git-sync settings to match the repo's wmill.yaml.".to_string(),
});
}
(
"neutral",
@@ -1169,6 +1181,35 @@ async fn maybe_post_git_sync_check(
{
tracing::error!("git sync-check: failed to update check run: {e:#}");
}
// Phase 4 also maintains ONE managed comment on the PR (Cloudflare
// deploy-preview style): upserted on every synchronize, so reviewers see the
// current diff without opening the Checks tab.
if !is_deploy {
if let Some(pr_number) = check.pr_number {
let marker = "<!-- windmill-diff -->";
let head = check
.head_sha
.as_deref()
.map(|s| &s[..s.len().min(7)])
.unwrap_or("latest");
let body = format!(
"{marker}\n### Windmill deploy preview\n\n | | |\n|---|---|\n | **Workspace** | `{workspace_id}` |\n | **Status** | {title} |\n | **Commit** | `{head}` |\n\n <details><summary>Details</summary>\n\n{summary}\n\n</details>"
);
if let Err(e) = windmill_common::git_sync_ee::upsert_pr_comment(
db,
workspace_id,
&check.repo_url,
pr_number,
marker,
&body,
)
.await
{
tracing::warn!("git sync-check: failed to upsert PR diff comment: {e:#}");
}
}
}
}
pub async fn process_completed_job(
docs/git-sync-pull-design.md
+12
View File
@@ -253,6 +253,14 @@ every existing installation's org admin):
| Pull requests: write | instance-side PR creation (promotion/forks) | 2 |
| Checks: write | PR diff preview checks | 3 |
App-only features and their fallbacks: instant webhook sync (falls back to
polling), in-app PR creation for deploy branches (fall back to the
`open-pr-on-commit` / `open-pr-on-fork-commit` workflows; the toggles are
hidden for token repos and a set-but-inert toggle logs a warning), and the PR
diff comment/check + deploy status check (no fallback: they need the Checks
API and `pull_request` webhook deliveries). Token/PAT repositories keep the
full pull direction via polling.
Rollout behavior: until an org approves, webhook creation fails with a
distinguishable error → the instance shows "approval pending" and stays on
polling. Nothing breaks; latency is the only cost. The self-managed (GHES)
@@ -451,6 +459,10 @@ Frontend:
- Subscribe `pull_request` events; on open/synchronize run the existing
`dry_run: true` pull and post the diff as a check run (`checks: write`).
- The same completion hook maintains ONE managed comment on the PR
(Cloudflare deploy-preview style: workspace, status, commit, collapsible
change list), upserted per synchronize via a hidden `<!-- windmill-diff -->`
marker so reviewers see the current diff without opening the Checks tab.
### Phase 5 — fork sync, configured at the parent (replaces the `push-on-merge-to-forks` GitHub Action) — implemented
frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte
+20
View File
@@ -580,6 +580,16 @@
on:change={(e) => setPromotionOpenPrs(e.detail)}
/>
</div>
{:else if repoMode === 'promotion' && !repo.isUnsavedConnection}
<div class="text-2xs text-secondary mt-2">
Opening pull requests from Windmill uses the
<a
href="https://www.windmill.dev/docs/advanced/git_sync"
target="_blank"
class="text-blue-500 hover:underline">GitHub App</a
>. Token-based repositories can use the
<span class="font-mono">open-pr-on-commit</span> workflow instead.
</div>
{/if}
</div>
@@ -718,6 +728,16 @@
}}
on:change={(e) => setForkOpenPrs(e.detail)}
/>
{:else}
<div class="text-2xs text-secondary">
Opening pull requests for fork deploys uses the
<a
href="https://www.windmill.dev/docs/advanced/git_sync"
target="_blank"
class="text-blue-500 hover:underline">GitHub App</a
>. Token-based repositories can use the
<span class="font-mono">open-pr-on-fork-commit</span> workflow instead.
</div>
{/if}
</div>
{/if}