From b711e051689e8b687daeb4d44011c903c7932aa4 Mon Sep 17 00:00:00 2001
From: Guillaume Moigneu <g@moigneu.com>
Date: Sun, 3 May 2026 04:59:13 +0000
Subject: [PATCH] fix(engine): plan errors fail fast; service caps consecutive
 failures
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two related orchestration safety nets called out in #134.

1. cmd/sync.go now checks plan.HasErrors after Plan and before
   RunOnce. If the plan already knows e.g. "unsupported VCS type",
   sync exits with ExitCodeError{Code: 2, Cause: ...} instead of
   re-discovering the same error per resource inside the apply.

2. engine.RunService now bounds consecutive failures at
   maxConsecutiveServiceFailures (5). A successful sync resets the
   counter; transient errors are absorbed. Once the cap is hit,
   RunService returns the wrapped last error so a process supervisor
   (systemd, kubelet, …) can intervene. Previously a misconfigured
   service retried forever, burning CPU and network indefinitely.

Closes #134.
---
 cmd/sync.go               |  8 +++++++
 internal/engine/engine.go | 46 ++++++++++++++++++++++++++++++++++-----
 2 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/cmd/sync.go b/cmd/sync.go
index 86d3c73..f575959 100644
--- a/cmd/sync.go
+++ b/cmd/sync.go
@@ -97,6 +97,14 @@ func runSync(_ *cobra.Command, _ []string) error {
 		telemetry.TrackError("sync", err)
 		return err
 	}
+	// Surface plan-level errors before apply (#134). A plan that already
+	// knows e.g. "unsupported VCS type" should fail fast instead of
+	// re-discovering the same error per resource inside RunOnce.
+	if plan.HasErrors {
+		err := fmt.Errorf("plan has errors; refusing to apply (run `gaal sync --dry-run` for details)")
+		telemetry.TrackError("sync", err)
+		return &ExitCodeError{Code: 2, Cause: err}
+	}
 
 	start := time.Now()
 	if err := eng.RunOnce(ctx); err != nil {
diff --git a/internal/engine/engine.go b/internal/engine/engine.go
index ae07d5a..bfeb880 100644
--- a/internal/engine/engine.go
+++ b/internal/engine/engine.go
@@ -159,13 +159,45 @@ func (e *Engine) RunOnce(ctx context.Context) error {
 	return nil
 }
 
-// RunService runs synchronisation in a loop until the context is cancelled.
+// maxConsecutiveServiceFailures bounds the number of consecutive failed
+// RunOnce iterations RunService tolerates before exiting. A misconfigured
+// service (bad config, dead remote) used to retry forever, burning CPU
+// and network indefinitely. After the cap, RunService returns the last
+// error so a process supervisor (systemd, kubelet, …) can intervene. #134.
+const maxConsecutiveServiceFailures = 5
+
+// RunService runs synchronisation in a loop until the context is cancelled
+// or maxConsecutiveServiceFailures is reached. A successful sync resets
+// the failure counter; transient errors are absorbed.
 func (e *Engine) RunService(ctx context.Context, interval time.Duration) error {
-	slog.Info("service mode started", "interval", interval)
+	slog.Info("service mode started", "interval", interval,
+		"failure_cap", maxConsecutiveServiceFailures)
+
+	consecutiveFailures := 0
+	var lastErr error
+
+	runIteration := func(label string) {
+		err := e.RunOnce(ctx)
+		if err != nil {
+			consecutiveFailures++
+			lastErr = err
+			slog.Error(label+" sync failed", "err", err,
+				"consecutive_failures", consecutiveFailures,
+				"failure_cap", maxConsecutiveServiceFailures)
+			return
+		}
+		if consecutiveFailures > 0 {
+			slog.Info("service: sync recovered", "after_failures", consecutiveFailures)
+		}
+		consecutiveFailures = 0
+		lastErr = nil
+	}
 
 	// Run immediately on startup.
-	if err := e.RunOnce(ctx); err != nil {
-		slog.Error("initial sync failed", "err", err)
+	runIteration("initial")
+	if consecutiveFailures >= maxConsecutiveServiceFailures {
+		return fmt.Errorf("service exiting after %d consecutive failures: %w",
+			consecutiveFailures, lastErr)
 	}
 
 	ticker := time.NewTicker(interval)
@@ -178,8 +210,10 @@ func (e *Engine) RunService(ctx context.Context, interval time.Duration) error {
 			return nil
 		case t := <-ticker.C:
 			slog.Info("periodic sync triggered", "time", t)
-			if err := e.RunOnce(ctx); err != nil {
-				slog.Error("periodic sync failed", "err", err)
+			runIteration("periodic")
+			if consecutiveFailures >= maxConsecutiveServiceFailures {
+				return fmt.Errorf("service exiting after %d consecutive failures: %w",
+					consecutiveFailures, lastErr)
 			}
 		}
 	}