diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 7ac0e9bb48..065d870b24 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -c3bc283f95d546c25998208dd46e9bd24641619a +39791c7861bb00c21045536df5c173856313dad5 diff --git a/backend/src/monitor.rs b/backend/src/monitor.rs index b3bc5c6357..4320203faf 100644 --- a/backend/src/monitor.rs +++ b/backend/src/monitor.rs @@ -4717,7 +4717,13 @@ async fn maintain_git_credentials_inner(db: &Pool) -> error::Result<() } }; - for repo in &settings.repositories { + // The workspace ordering above only decides which workspace comes first; + // within one, the repositories need the same least-recently-checked + // order or the tail of a large workspace never gets its turn. + let mut repositories: Vec<_> = settings.repositories.iter().collect(); + repositories.sort_by_key(|r| r.credential.as_ref().map(|c| c.checked_at)); + + for repo in repositories { if started.elapsed() >= GIT_CREDENTIAL_PASS_BUDGET { skipped += 1; continue; diff --git a/backend/windmill-worker/src/result_processor.rs b/backend/windmill-worker/src/result_processor.rs index 237a90bdec..82111a7165 100644 --- a/backend/windmill-worker/src/result_processor.rs +++ b/backend/windmill-worker/src/result_processor.rs @@ -820,7 +820,10 @@ struct GitSyncCheck { /// result then reaches the pull request through the managed comment alone. #[serde(default)] check_run_id: Option, - repo_url: String, + /// Only markers written before the repository URL moved out of job args + /// carry one; the resource path on the job is what is used now. + #[serde(default)] + repo_url: Option, #[serde(default)] pr_number: Option, #[serde(default)] @@ -1379,19 +1382,30 @@ async fn maybe_post_git_sync_check( let Ok(mut check) = serde_json::from_value::(marker) else { return; }; - // Markers carry the literal resource URL (job args are persisted, so a - // `$var:`-resolved URL must not land there); interpolate before calling - // GitHub. - check.repo_url = - match windmill_common::variables::get_variable_or_self(check.repo_url, db, workspace_id) - .await - { - Ok(u) => u, - Err(e) => { - tracing::error!("git sync-check: cannot interpolate repo url: {e:#}"); - return; - } - }; + // Job args are persisted, so the repository URL is not among them: it is + // re-resolved here from the resource path the pull job carries. A marker + // written before that change still has the URL, and is honoured until the + // last such job has drained. + let repo_url = match (row.repo_path.as_deref(), check.repo_url.clone()) { + (Some(path), _) => { + windmill_common::git_sync_ee::resolve_repo_url_interpolated(db, workspace_id, path) + .await + } + (None, Some(url)) => { + windmill_common::variables::get_variable_or_self(url, db, workspace_id).await + } + (None, None) => { + tracing::error!("git sync-check: marker names no repository"); + return; + } + }; + let repo_url = match repo_url { + Ok(u) => u, + Err(e) => { + tracing::error!("git sync-check: cannot resolve repo url: {e:#}"); + return; + } + }; // "In sync" on a PR that visibly changes files reads as a bug when those // files are outside the repo's sync filters — say what the scope is. let scope_note = if !is_deploy && success { @@ -1527,7 +1541,7 @@ async fn maybe_post_git_sync_check( if let Err(e) = windmill_common::git_sync_ee::update_check_run( db, workspace_id, - &check.repo_url, + &repo_url, check_run_id, conclusion, &title, @@ -1561,7 +1575,7 @@ async fn maybe_post_git_sync_check( if let Err(e) = windmill_common::git_sync_ee::upsert_pr_comment( db, workspace_id, - &check.repo_url, + &repo_url, pr_number, marker, &body,