fix: clear a stale git auto-pull failure and show the status time (#11100)

* fix: show when the last git auto-pull status was recorded

* chore: bump ee-repo-ref for the auto-pull status fix

* fix: show the git auto-pull status age with TimeAgo instead of a year-less date

* test: pin that a stale auto-pull recovery cannot overwrite a newer state

* chore: bump ee-repo-ref for the conditional auto-pull recovery

* fix: keep TimeAgo counting past the first hour in noSeconds mode

* chore: bump ee-repo-ref for the clear_auto_pull_failure contract note

* fix: guard TimeAgo's boundary scheduler against invalid dates and pin same-head newer failures

* chore: bump ee-repo-ref for the timestamp-guarded auto-pull recovery

* test: cover a same-second newer failure surviving a stale auto-pull recovery

* chore: bump ee-repo-ref for the whole-failure recovery match

* test: name the recovery helper after its input, not its staleness

* chore: update ee-repo-ref to c6df9fdd9826efb40d3586a9f97d17dee98ac6ef

This commit updates the EE repository reference after PR #793 was merged in windmill-ee-private.

Previous ee-repo-ref: 6aff80b80cae4944a4a78a6b9244019bc37f368b

New ee-repo-ref: c6df9fdd9826efb40d3586a9f97d17dee98ac6ef

Automated by sync-ee-ref workflow.

---------

Co-authored-by: Ruben Fiszel <ruben@windmill.dev>
Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>

backend/ee-repo-ref.txt

@@ -1 +1 @@
-c9b043f2860fdae150c8c4bf03f3ec98b7f300e5
+c6df9fdd9826efb40d3586a9f97d17dee98ac6ef

backend/tests/fixtures/git_sync_autopull_recovery.sql

@@ -0,0 +1,10 @@
+-- A workspace whose auto-pulled repository last recorded a head-check failure while
+-- already synced to head "aaa": the state a recovery write is decided on.
+
+INSERT INTO workspace (id, name, owner) VALUES ('ap-ws', 'ap-ws', 'test-user');
+
+INSERT INTO workspace_settings (workspace_id, git_sync) VALUES
+    ('ap-ws', '{"repositories":[{"git_repo_resource_path":"$res:u/admin/repo",
+        "auto_pull":{"enabled":true,"mode":"polling",
+            "last_synced_sha":{"main":"aaa"},
+            "last_pull_status":{"success":false,"at":1,"error":"head check failed: x"}}}]}');

backend/tests/git_sync_autopull_recovery.rs

@@ -0,0 +1,143 @@
+//! A recorded auto-pull failure is cleared once the tracked head is observed again
+//! at the already-synced sha, and only then: the decision is taken on a snapshot,
+//! so the write must re-check the stored row rather than overwrite it.
+#![cfg(all(feature = "enterprise", feature = "private"))]
+
+use sqlx::{Pool, Postgres};
+use std::collections::HashMap;
+use uuid::Uuid;
+use windmill_common::workspaces::AutoPullStatus;
+use windmill_git_sync::{clear_auto_pull_failure, persist_auto_pull_state};
+
+const WS: &str = "ap-ws";
+const REPO: &str = "$res:u/admin/repo";
+
+/// The failure the fixture records, as the poller would have read it.
+fn fixture_failure() -> AutoPullStatus {
+    AutoPullStatus {
+        synced_sha: None,
+        at: 1,
+        job_id: None,
+        success: false,
+        error: Some("head check failed: x".to_string()),
+    }
+}
+
+fn recovered(head: &str) -> AutoPullStatus {
+    AutoPullStatus {
+        synced_sha: Some(head.to_string()),
+        at: 2,
+        job_id: None,
+        success: true,
+        error: None,
+    }
+}
+
+async fn stored_auto_pull(db: &Pool<Postgres>) -> anyhow::Result<serde_json::Value> {
+    let git_sync: serde_json::Value =
+        sqlx::query_scalar("SELECT git_sync FROM workspace_settings WHERE workspace_id = $1")
+            .bind(WS)
+            .fetch_one(db)
+            .await?;
+    Ok(git_sync["repositories"][0]["auto_pull"].clone())
+}
+
+/// The recovery every test below runs, decided on the fixture's failure at head
+/// "aaa": live in the first test, stale in the two that move the stored state
+/// first.
+async fn recovery_for_the_fixture_failure(db: &Pool<Postgres>) -> anyhow::Result<()> {
+    clear_auto_pull_failure(
+        db,
+        WS,
+        REPO,
+        "main",
+        "aaa",
+        &fixture_failure(),
+        &recovered("aaa"),
+    )
+    .await?;
+    Ok(())
+}
+
+#[sqlx::test(fixtures("git_sync_autopull_recovery"))]
+async fn recovery_clears_the_failure_at_the_synced_head(db: Pool<Postgres>) -> anyhow::Result<()> {
+    recovery_for_the_fixture_failure(&db).await?;
+
+    let auto_pull = stored_auto_pull(&db).await?;
+    assert_eq!(auto_pull["last_pull_status"]["success"], true);
+    assert!(auto_pull["last_pull_status"].get("error").is_none());
+    assert_eq!(auto_pull["last_pull_status"]["synced_sha"], "aaa");
+    assert_eq!(
+        auto_pull["last_synced_sha"]["main"], "aaa",
+        "the sha map is not part of a recovery write"
+    );
+    Ok(())
+}
+
+/// Between the poller observing head "aaa" unchanged and its recovery write, a
+/// webhook may have enqueued newer head "bbb". The stale recovery must leave that
+/// optimistic state (sha, success, job id) in place; the job's completion hook
+/// relies on it, and rolling the sha back would re-enqueue "bbb" on the next tick.
+#[sqlx::test(fixtures("git_sync_autopull_recovery"))]
+async fn stale_recovery_leaves_a_newer_state_alone(db: Pool<Postgres>) -> anyhow::Result<()> {
+    let job_id = Uuid::new_v4();
+    let advanced = AutoPullStatus {
+        synced_sha: Some("bbb".to_string()),
+        at: 3,
+        job_id: Some(job_id),
+        success: true,
+        error: None,
+    };
+    persist_auto_pull_state(
+        &db,
+        WS,
+        REPO,
+        &HashMap::from([("main".to_string(), "bbb".to_string())]),
+        &advanced,
+    )
+    .await?;
+
+    recovery_for_the_fixture_failure(&db).await?;
+
+    let auto_pull = stored_auto_pull(&db).await?;
+    assert_eq!(auto_pull["last_synced_sha"]["main"], "bbb");
+    assert_eq!(auto_pull["last_pull_status"]["synced_sha"], "bbb");
+    assert_eq!(auto_pull["last_pull_status"]["job_id"], job_id.to_string());
+    assert_eq!(auto_pull["last_pull_status"]["at"], 3);
+    Ok(())
+}
+
+/// The head can stay at "aaa" while a newer failure is recorded (a later head
+/// check, a pull job that failed). A recovery decided on the older failure must
+/// not paper over the newer one, whether it differs by timestamp or, within the
+/// same second, only by its error.
+#[sqlx::test(fixtures("git_sync_autopull_recovery"))]
+async fn stale_recovery_keeps_a_newer_failure_at_the_same_head(
+    db: Pool<Postgres>,
+) -> anyhow::Result<()> {
+    let same_sha = HashMap::from([("main".to_string(), "aaa".to_string())]);
+    for newer in [
+        AutoPullStatus {
+            at: 5,
+            error: Some("head check failed: later".to_string()),
+            ..fixture_failure()
+        },
+        AutoPullStatus {
+            error: Some("head check failed: same second".to_string()),
+            ..fixture_failure()
+        },
+    ] {
+        persist_auto_pull_state(&db, WS, REPO, &same_sha, &newer).await?;
+
+        recovery_for_the_fixture_failure(&db).await?;
+
+        let auto_pull = stored_auto_pull(&db).await?;
+        assert_eq!(auto_pull["last_pull_status"]["success"], false);
+        assert_eq!(auto_pull["last_pull_status"]["at"], newer.at);
+        assert_eq!(
+            auto_pull["last_pull_status"]["error"],
+            newer.error.as_deref().unwrap()
+        );
+    }
+    Ok(())
+}

backend/windmill-git-sync/src/lib.rs

@@ -14,10 +14,10 @@ pub mod git_sync_oss;
 
 #[cfg(feature = "private")]
 pub use git_sync_ee::{
-    enqueue_git_pull_dry_run, enqueue_git_pull_job, handle_deployment_metadata,
-    handle_deployment_metadata_batch, handle_fork_branch_creation, persist_auto_pull_state,
-    reconcile_and_enqueue_pull, reconcile_fork_branch_pull, record_auto_pull_failure,
-    tally_deployed_object_changes,
+    clear_auto_pull_failure, enqueue_git_pull_dry_run, enqueue_git_pull_job,
+    handle_deployment_metadata, handle_deployment_metadata_batch, handle_fork_branch_creation,
+    persist_auto_pull_state, reconcile_and_enqueue_pull, reconcile_fork_branch_pull,
+    record_auto_pull_failure, tally_deployed_object_changes,
 };
 
 #[cfg(not(feature = "private"))]

frontend/src/lib/components/TimeAgo.svelte

@@ -27,18 +27,16 @@
 	let interval
 
 	onMount(() => {
-		// compact schedules itself below; it needs no fixed rate.
-		if (compact) return
+		// compact and noSeconds schedule themselves below; they need no fixed rate.
+		if (compact || noSeconds) return
 
-		// Update every minute for noSeconds mode, every second otherwise.
-		const intervalMs = noSeconds ? 60000 : 1000
 		interval = setInterval(() => {
 			computeDate()
 			if (!isRecent) {
 				clearInterval(interval)
 				interval = undefined
 			}
-		}, intervalMs)
+		}, 1000)
 
 		// Add explicit cleanup
 		return () => {
@@ -49,21 +47,27 @@
 	// Waking on the boundary of the unit on screen, rather than at a fixed rate: `2h` only
 	// changes on the hour, and a row that reads `5d` must not hold a 1s timer to find that
 	// out. Re-armed when `date` changes, so an item edited to now leaves its day-long wait.
+	// noSeconds rides the same schedule: its relative forms keep counting hours and days,
+	// so a fixed-rate timer that stops once the date is an hour old would freeze them.
 	$effect(() => {
-		if (!compact) return
+		if (!compact && !noSeconds) return
 		const at = date
+		// An absent or unparsable date has no boundary to wait for: the delay below would
+		// be NaN, which setTimeout runs immediately, and the tick would re-arm itself in
+		// a tight loop.
+		if (Number.isNaN(new Date(at).getTime())) return
 		let handle: ReturnType<typeof setTimeout> | undefined
 		const tick = () => {
 			computeDate()
-			handle = setTimeout(tick, compactDelayMs(at))
+			handle = setTimeout(tick, nextUnitBoundaryMs(at))
 		}
-		handle = setTimeout(tick, compactDelayMs(at))
+		handle = setTimeout(tick, nextUnitBoundaryMs(at))
 		return () => {
 			handle && clearTimeout(handle)
 		}
 	})
 
-	function compactDelayMs(dateString: string): number {
+	function nextUnitBoundaryMs(dateString: string): number {
 		const secs = secondsAgo(new Date(dateString))
 		const left =
 			secs < 60

frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte

@@ -22,6 +22,7 @@
 	import type { GitSyncRepository } from './GitSyncContext.svelte'
 	import GitSyncModeDisplay from './GitSyncModeDisplay.svelte'
 	import Toggle from '$lib/components/Toggle.svelte'
+	import TimeAgo from '$lib/components/TimeAgo.svelte'
 	import EEOnly from '$lib/components/EEOnly.svelte'
 	import { GitSyncService, ResourceService, VariableService } from '$lib/gen'
 
@@ -57,6 +58,10 @@
 	const gitSyncTestJob = $derived(idx !== null ? gitSyncContext.gitSyncTestJobs?.[idx] : null)
 	let confirmingDelete = $state(false)
 
+	function pullStatusDate(status: { at: number }): string {
+		return new Date(status.at * 1000).toISOString()
+	}
+
 	// Enable/disable automatic repo → workspace pulls, managing the optional
 	// auto_pull object without binding into a possibly-undefined value.
 	function setAutoPullEnabled(enabled: boolean) {
@@ -919,10 +924,18 @@
 											{#if repo.auto_pull.last_pull_status.success}
 												Last synced{repo.auto_pull.last_pull_status.synced_sha
 													? ` to ${repo.auto_pull.last_pull_status.synced_sha.slice(0, 7)}`
-													: ''}.
+													: ''}
+												<TimeAgo
+													date={pullStatusDate(repo.auto_pull.last_pull_status)}
+													noSeconds
+												/>.
 											{:else}
 												<span class="text-red-600 dark:text-red-400">
-													Last sync failed{repo.auto_pull.last_pull_status.error
+													Last sync failed
+													<TimeAgo
+														date={pullStatusDate(repo.auto_pull.last_pull_status)}
+														noSeconds
+													/>{repo.auto_pull.last_pull_status.error
 														? `: ${repo.auto_pull.last_pull_status.error}`
 														: ''}.
 												</span>
@@ -1000,13 +1013,21 @@
 											{#if repo.auto_pull.last_pull_status.success}
 												Last synced{repo.auto_pull.last_pull_status.synced_sha
 													? ` to ${repo.auto_pull.last_pull_status.synced_sha.slice(0, 7)}`
-													: ''}.
+													: ''}
+												<TimeAgo
+													date={pullStatusDate(repo.auto_pull.last_pull_status)}
+													noSeconds
+												/>.
 												{viaWebhook
 													? ' Syncing instantly via webhook.'
 													: ' Checking the tracked branch about every minute.'}
 											{:else}
 												<span class="text-red-600 dark:text-red-400">
-													Last sync failed{repo.auto_pull.last_pull_status.error
+													Last sync failed
+													<TimeAgo
+														date={pullStatusDate(repo.auto_pull.last_pull_status)}
+														noSeconds
+													/>{repo.auto_pull.last_pull_status.error
 														? `: ${repo.auto_pull.last_pull_status.error}`
 														: ''}.
 												</span>