feat(reconcile): validate every document before applying anything

internal/reconcile/permission_reconciler.go

@@ -32,6 +32,32 @@ func NewPermissionReconciler(client PermissionAPI, header string) *PermissionRec
 
 func (r *PermissionReconciler) Kind() string { return KindPermission }
 
+// Validate checks every entry, and the in-file slug conflicts, without touching
+// the server, so a bad entry stops the whole file before anything applies.
+func (r *PermissionReconciler) Validate(spec []byte) error {
+	var specs []PermissionSpec
+	if err := decodeSpec(spec, &specs); err != nil {
+		return fmt.Errorf("parse %s spec: %w", KindPermission, err)
+	}
+	seen := map[string]PermissionSpec{}
+	for _, s := range specs {
+		if err := validatePermissionSpec(s); err != nil {
+			return fmt.Errorf("invalid permission spec %s: %w", s, err)
+		}
+		slug := s.slug()
+		if prev, dup := seen[slug]; dup {
+			if prev.Namespace != s.Namespace || prev.Name != s.Name {
+				return fmt.Errorf("permissions %s and %s collide on the same slug %q", prev, s, slug)
+			}
+			if prev.Delete != s.Delete {
+				return fmt.Errorf("permission %s is listed both with and without delete", s)
+			}
+		}
+		seen[slug] = s
+	}
+	return nil
+}
+
 func (r *PermissionReconciler) Reconcile(ctx context.Context, spec []byte, dryRun bool) (Report, error) {
 	var specs []PermissionSpec
 	if err := decodeSpec(spec, &specs); err != nil {

internal/reconcile/platformuser_reconciler.go

@@ -33,6 +33,21 @@ func NewPlatformUserReconciler(client PlatformUserAPI, header string) *PlatformU
 
 func (r *PlatformUserReconciler) Kind() string { return KindPlatformUser }
 
+// Validate checks every entry without touching the server, so a bad entry stops
+// the whole file before anything applies.
+func (r *PlatformUserReconciler) Validate(spec []byte) error {
+	var specs []PlatformUserSpec
+	if err := decodeSpec(spec, &specs); err != nil {
+		return fmt.Errorf("parse %s spec: %w", KindPlatformUser, err)
+	}
+	for _, s := range specs {
+		if err := validateSpec(s); err != nil {
+			return fmt.Errorf("invalid platform-user spec %+v: %w", s, err)
+		}
+	}
+	return nil
+}
+
 func (r *PlatformUserReconciler) Reconcile(ctx context.Context, spec []byte, dryRun bool) (Report, error) {
 	var specs []PlatformUserSpec
 	if err := yaml.Unmarshal(spec, &specs); err != nil {

internal/reconcile/preference_reconciler.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"sort"
+	"strings"
 
 	"connectrpc.com/connect"
 	"github.com/raystack/frontier/internal/bootstrap/schema"
@@ -33,6 +34,27 @@ func NewPreferenceReconciler(client PreferenceAPI, header string) *PreferenceRec
 
 func (r *PreferenceReconciler) Kind() string { return KindPreference }
 
+// Validate checks the server-free parts of every entry (name present, no
+// duplicates) so a bad entry stops the whole file before anything applies. The
+// unknown-name check needs the server's trait list, so it stays in the diff.
+func (r *PreferenceReconciler) Validate(spec []byte) error {
+	var specs []PreferenceSpec
+	if err := decodeSpec(spec, &specs); err != nil {
+		return fmt.Errorf("parse %s spec: %w", KindPreference, err)
+	}
+	seen := map[string]struct{}{}
+	for _, s := range specs {
+		if strings.TrimSpace(s.Name) == "" {
+			return fmt.Errorf("preference name is required")
+		}
+		if _, dup := seen[s.Name]; dup {
+			return fmt.Errorf("preference %q is listed more than once", s.Name)
+		}
+		seen[s.Name] = struct{}{}
+	}
+	return nil
+}
+
 func (r *PreferenceReconciler) Reconcile(ctx context.Context, spec []byte, dryRun bool) (Report, error) {
 	var specs []PreferenceSpec
 	if err := decodeSpec(spec, &specs); err != nil {

internal/reconcile/reconcile.go

@@ -15,12 +15,14 @@ import (
 )
 
 // Reconciler makes a single resource kind match its desired-state spec.
-// Export is the reverse direction and is part of the contract: it reads the
-// current server state and returns it as a spec value, ready to be marshalled
-// into a desired-state document. Reconciling an exported document must plan
-// no changes.
+// Validate checks a spec without touching the server or applying anything, so
+// the whole file can be checked before the first change is made. Export is the
+// reverse direction and is part of the contract: it reads the current server
+// state and returns it as a spec value, ready to be marshalled into a
+// desired-state document. Reconciling an exported document must plan no changes.
 type Reconciler interface {
 	Kind() string
+	Validate(spec []byte) error
 	Reconcile(ctx context.Context, spec []byte, dryRun bool) (Report, error)
 	Export(ctx context.Context) (spec any, err error)
 }
@@ -73,7 +75,12 @@ func parseDocuments(registry map[string]Reconciler, data []byte) ([]parsedDocume
 			return nil, fmt.Errorf("parse desired-state: %w", err)
 		}
 		if doc.Kind == "" {
-			continue
+			// A blank document (a stray "---") is fine to skip, but one that carries
+			// content without a kind is a mistake: fail instead of dropping it silently.
+			if isBlankDocument(doc) {
+				continue
+			}
+			return nil, fmt.Errorf("a document has content but no kind")
 		}
 		if doc.APIVersion != "" && doc.APIVersion != apiVersion {
 			return nil, fmt.Errorf("document kind %q has unsupported apiVersion %q (want %q)", doc.Kind, doc.APIVersion, apiVersion)
@@ -102,6 +109,20 @@ func parseDocuments(registry map[string]Reconciler, data []byte) ([]parsedDocume
 	return docs, nil
 }
 
+// isBlankDocument reports whether a document carries no content at all (a stray
+// "---" separator), as opposed to content that is missing its kind.
+func isBlankDocument(doc document) bool {
+	if doc.APIVersion != "" {
+		return false
+	}
+	specBytes, err := yaml.Marshal(&doc.Spec)
+	if err != nil {
+		return false
+	}
+	s := strings.TrimSpace(string(specBytes))
+	return s == "" || s == "null"
+}
+
 // decodeSpec unmarshals a kind's spec with unknown fields rejected, so a typo
 // in a field name (like `delet: true` for `delete`) fails the plan instead of
 // being silently ignored, which would make a run quietly do the wrong thing.
@@ -124,6 +145,13 @@ func Run(ctx context.Context, registry map[string]Reconciler, data []byte, dryRu
 	if err != nil {
 		return nil, err
 	}
+	// Validate every document before applying anything, so a bad entry in a later
+	// document stops the run before the first change is made.
+	for _, doc := range docs {
+		if err := registry[doc.kind].Validate(doc.spec); err != nil {
+			return nil, fmt.Errorf("validate %s: %w", doc.kind, err)
+		}
+	}
 	var reports []Report
 	for _, doc := range docs {
 		rep, err := registry[doc.kind].Reconcile(ctx, doc.spec, dryRun)

internal/reconcile/reconcile_test.go

@@ -10,7 +10,8 @@ import (
 
 type fakeReconciler struct{ called int }
 
-func (f *fakeReconciler) Kind() string { return KindPlatformUser }
+func (f *fakeReconciler) Kind() string            { return KindPlatformUser }
+func (f *fakeReconciler) Validate(_ []byte) error { return nil }
 func (f *fakeReconciler) Reconcile(_ context.Context, _ []byte, _ bool) (Report, error) {
 	f.called++
 	return Report{Kind: KindPlatformUser}, nil
@@ -21,13 +22,26 @@ func (f *fakeReconciler) Export(_ context.Context) (any, error) { return []strin
 // dies part-way through.
 type partialReconciler struct{}
 
-func (partialReconciler) Kind() string { return KindPlatformUser }
+func (partialReconciler) Kind() string            { return KindPlatformUser }
+func (partialReconciler) Validate(_ []byte) error { return nil }
 func (partialReconciler) Reconcile(_ context.Context, _ []byte, _ bool) (Report, error) {
 	return Report{Kind: KindPlatformUser, Applied: 2, Planned: []string{"add a", "add b", "add c"}},
 		errors.New("apply failed on the third op")
 }
 func (partialReconciler) Export(_ context.Context) (any, error) { return []string{}, nil }
 
+// validateFailReconciler rejects any spec at validation time and records whether
+// it was ever dispatched.
+type validateFailReconciler struct{ called int }
+
+func (v *validateFailReconciler) Kind() string            { return KindPermission }
+func (v *validateFailReconciler) Validate(_ []byte) error { return errors.New("bad entry") }
+func (v *validateFailReconciler) Reconcile(_ context.Context, _ []byte, _ bool) (Report, error) {
+	v.called++
+	return Report{Kind: KindPermission}, nil
+}
+func (v *validateFailReconciler) Export(_ context.Context) (any, error) { return []string{}, nil }
+
 func TestRun_SpecHandling(t *testing.T) {
 	t.Run("rejects a document missing its spec (not an empty list)", func(t *testing.T) {
 		rec := &fakeReconciler{}
@@ -68,6 +82,36 @@ func TestRun_SpecHandling(t *testing.T) {
 		assert.ErrorContains(t, err, `no reconciler registered for kind "Unknown"`)
 		assert.Zero(t, rec.called) // the whole file is checked before any dispatch
 	})
+
+	t.Run("a document that fails validation blocks every apply", func(t *testing.T) {
+		first := &fakeReconciler{}          // a valid document that records applies
+		second := &validateFailReconciler{} // a later document that fails validation
+		reg := map[string]Reconciler{KindPlatformUser: first, KindPermission: second}
+		data := []byte("kind: PlatformUser\nspec: []\n---\nkind: Permission\nspec: []\n")
+
+		_, err := Run(context.Background(), reg, data, false)
+		assert.ErrorContains(t, err, "validate Permission")
+		assert.Zero(t, first.called) // validation runs for every document before any apply
+		assert.Zero(t, second.called)
+	})
+
+	t.Run("a document with content but no kind is rejected", func(t *testing.T) {
+		rec := &fakeReconciler{}
+		reg := map[string]Reconciler{KindPlatformUser: rec}
+
+		_, err := Run(context.Background(), reg, []byte("spec:\n  - type: user\n"), false)
+		assert.ErrorContains(t, err, "content but no kind")
+		assert.Zero(t, rec.called)
+	})
+
+	t.Run("a blank trailing document is skipped, not rejected", func(t *testing.T) {
+		rec := &fakeReconciler{}
+		reg := map[string]Reconciler{KindPlatformUser: rec}
+
+		_, err := Run(context.Background(), reg, []byte("kind: PlatformUser\nspec: []\n---\n"), false)
+		assert.NoError(t, err)
+		assert.Equal(t, 1, rec.called)
+	})
 }
 
 func TestExport_Errors(t *testing.T) {

internal/reconcile/role_reconciler.go

@@ -41,6 +41,27 @@ func NewRoleReconciler(client RoleAPI, header string) *RoleReconciler {
 
 func (r *RoleReconciler) Kind() string { return KindRole }
 
+// Validate checks every entry, and rejects duplicate role names, without touching
+// the server, so a bad entry stops the whole file before anything applies.
+func (r *RoleReconciler) Validate(spec []byte) error {
+	var specs []RoleSpec
+	if err := decodeSpec(spec, &specs); err != nil {
+		return fmt.Errorf("parse %s spec: %w", KindRole, err)
+	}
+	seen := map[string]struct{}{}
+	for _, s := range specs {
+		_, isPredefined := predefinedRole(s.Name)
+		if err := validateRoleSpec(s, isPredefined); err != nil {
+			return fmt.Errorf("invalid role spec %q: %w", s.Name, err)
+		}
+		if _, dup := seen[s.Name]; dup {
+			return fmt.Errorf("role %q is listed more than once", s.Name)
+		}
+		seen[s.Name] = struct{}{}
+	}
+	return nil
+}
+
 func (r *RoleReconciler) Reconcile(ctx context.Context, spec []byte, dryRun bool) (Report, error) {
 	var specs []RoleSpec
 	if err := decodeSpec(spec, &specs); err != nil {