fix(repo): say why a target commit became unreachable instead of implying a bad pin (#23)

internal/controllers/repo/repo.go

@@ -184,7 +184,24 @@ func (e *external) Observe(ctx context.Context, mg resource.Managed) (reconciler
 	if !isTargetRepoSynced {
 		e.log.Debug("Target commit not found in target remote repository", "commitId", cr.Status.TargetCommitId, "branch", cr.Status.TargetBranch)
 		if !cr.Spec.EnableUpdate {
-			return reconciler.ExternalObservation{}, e.failSync(ctx, cr, fmt.Errorf("target commit %s not found on branch %s while enableUpdate is false", cr.Status.TargetCommitId, cr.Status.TargetBranch))
+			// Stays an error on purpose. The resource genuinely cannot be reconciled and a human
+			// has to choose between re-pinning and allowing updates, so surfacing it as
+			// Synced=False/ReconcileError is right — TC12 and TC13 pin that contract deliberately,
+			// and TC13 covers the auto-heal once enableUpdate is flipped.
+			//
+			// What WAS wrong is the wording. "target commit %s not found on branch %s while
+			// enableUpdate is false" describes what the check found and reads as though the commit
+			// was pinned wrong. Usually it was not: the commit was discarded along with the history
+			// that contained it, when the target repository was recreated or force-pushed. Nine
+			// Repo CRs hit this simultaneously on one such event, and the message sent the reader
+			// looking for nine bad pins (#23).
+			//
+			// Note this is NOT the #22 situation, despite both being reached from this function.
+			// There the provider refused on an ambiguity nothing would ever resolve, so the
+			// resource was stranded with no path back. Here there is a real decision for a person
+			// to make, and reporting it until they make it is correct.
+			return reconciler.ExternalObservation{}, e.failSync(ctx, cr, unreachableTargetCommitError(
+				cr.Status.TargetCommitId, cr.Status.TargetBranch, cr.Spec.ToRepo.Url))
 		}
 		return reconciler.ExternalObservation{
 			ResourceExists:   true,

internal/controllers/repo/unreachable_commit.go

@@ -0,0 +1,31 @@
+package repo
+
+import (
+	"fmt"
+)
+
+// unreachableTargetCommitError reports a recorded target commit that the target branch can no
+// longer reach, when spec.enableUpdate forbids doing anything about it.
+//
+// The wording is the point. The previous message — "target commit X not found on branch main while
+// enableUpdate is false" — describes what the check found, and reads as though the commit was
+// pinned wrong. Usually it was not: the commit was discarded along with the history that contained
+// it, when the target repository was recreated or force-pushed. Nine Repo CRs hit this at once on a
+// single such event, and the message sent the reader looking for nine separate bad pins (#23).
+//
+// It stays an error. There is a real decision here for a person to make, so reporting it until they
+// make it is correct — unlike #22, where the provider refused on an ambiguity nothing would ever
+// resolve and the resource was stranded with no path back.
+//
+// What the message must carry: which commit and target (so the resource is identifiable), the CAUSE
+// (so the reader does not go hunting for a mistake that was not made), and both remedies. Naming
+// only enableUpdate would push operators toward allowing overwrites they may not want.
+func unreachableTargetCommitError(commitID, branch, toRepoURL string) error {
+	return fmt.Errorf(
+		"target commit %s is no longer reachable from branch %s of %s, and spec.enableUpdate is false. "+
+			"This usually means the target repository was recreated or its history rewritten, which "+
+			"discards the commit rather than invalidating it — it does not mean the commit was pinned "+
+			"wrong. Set enableUpdate to true to let the provider re-push, or recreate this resource to "+
+			"pin the current history.",
+		commitID, branch, toRepoURL)
+}

internal/controllers/repo/unreachable_commit_test.go

@@ -0,0 +1,58 @@
+package repo
+
+import (
+	"strings"
+	"testing"
+)
+
+// #23. The message must name the CAUSE and both remedies, not just the symptom.
+//
+// "target commit X not found on branch main while enableUpdate is false" reads as a bad pin. In the
+// incident that prompted this, nothing was pinned wrong — one target repository was recreated,
+// orphaning the recorded commits of nine resources at once. A message that implies the wrong remedy
+// sends the reader to fix something that is not broken.
+//
+// The error itself is deliberately preserved: a person has to choose between re-pinning and
+// allowing updates, and TC12/TC13 pin that contract.
+func TestUnreachableTargetCommitErrorExplainsTheCause(t *testing.T) {
+	const (
+		commit = "131ceb6bc"
+		branch = "main"
+		url    = "https://github.com/krateo-blueprints/demo-service-catalog.git"
+	)
+	msg := unreachableTargetCommitError(commit, branch, url).Error()
+
+	// Identifiers, so the operator knows which resource and which target.
+	for _, want := range []string{commit, branch, url} {
+		if !strings.Contains(msg, want) {
+			t.Errorf("message must identify %q, got: %s", want, msg)
+		}
+	}
+
+	// The cause. Without it the reader assumes a mistaken pin.
+	for _, want := range []string{"recreated", "history"} {
+		if !strings.Contains(msg, want) {
+			t.Errorf("message must explain the cause (%q), got: %s", want, msg)
+		}
+	}
+
+	// Both remedies. Naming only the flag pushes operators toward allowing overwrites they may not
+	// want; re-creating the resource re-pins to the current history instead.
+	for _, want := range []string{"enableUpdate", "recreate"} {
+		if !strings.Contains(msg, want) {
+			t.Errorf("message must offer the %q remedy, got: %s", want, msg)
+		}
+	}
+}
+
+// TC12 asserts the Synced message contains "target commit". That substring is load-bearing for the
+// integration suite, and it is also what makes the error greppable across a fleet — nine resources
+// failing for one reason should be findable as one string.
+func TestUnreachableTargetCommitErrorKeepsTheGreppablePrefix(t *testing.T) {
+	msg := unreachableTargetCommitError("abc123", "main", "https://example.com/repo.git").Error()
+
+	if !strings.Contains(msg, "target commit") {
+		t.Errorf("message must keep the 'target commit' substring: TC12 matches on it and it is how "+
+			"this failure is found across a fleet. Got: %s", msg)
+	}
+}