fix(diff): print errors once, from the command, not inline

CLAUDE.md

@@ -330,9 +330,11 @@ The tool prioritizes **accuracy over convenience**:
 
 When using structured output (`--output json` or `--output yaml`):
 
-1. **Errors go to BOTH places**: Write errors to stderr (for human visibility) AND include them in the structured output (for machine parsing). This ensures:
+1. **Errors are printed once, and also go in the structured output**: The renderers never print errors. The
+   processor returns them (each naming its resource) and `main` prints the returned error to stderr as `Error: ...`;
+   the renderer includes them in the structured output (for machine parsing). This ensures:
    - CI/CD tools can parse errors programmatically from stdout
-   - Humans see errors immediately in stderr
+   - Humans see each error once, on stderr
    - The structured output is always valid JSON/YAML, even when errors occur
 
 2. **Consistent error structure**: Use the same `OutputError` type (defined in `result/types.go`) across all commands:

README.md

@@ -189,7 +189,7 @@ otherwise not warned about. Its message then begins "did not verify added resour
 apiserver (--dry-run-on=existing)" and has no cause key. Built-in types, single-version CRDs and
 resources requested at their storage version never get this note. A failed XR's diffs are withheld, so no summary is shown
 for it: the summary says how far to trust the diffs that are shown, and the XR's error is reported in
-`errors[]` and on stderr; re-running after fixing it gives the full summary. (Before this was derived
+`errors[]` and the command's error; re-running after fixing it gives the full summary. (Before this was derived
 from the diffs, a failed XR's warning could appear incidentally, because it was raised mid-calculation.)
 For `comp` the summary is run-wide, in the
 top-level `warnings[]`, not per composition.
@@ -1145,11 +1145,11 @@ Exit-code precedence (per `DetermineExitCode`): any error in the run beats diff
 
 **Human-readable output** (`crossplane-diff xr invalid-xr.yaml`):
 
-Per Unix convention, errors go to stderr and diff content goes to stdout. When validation is the only failure, stdout is empty and the structured failure detail appears on stderr, prefixed by an `ERROR: <resourceID>:` marker:
+Per Unix convention, errors go to stderr and diff content goes to stdout. When validation is the only failure, stdout is empty and the structured failure detail appears on stderr, printed once, as the error the command fails with (`Error: ...`):
 
 ```
 # stderr
-ERROR: XNopResource/invalid-schema-xr: cannot validate resources: ns.diff.example.org/v1alpha1/XNopResource default/invalid-schema-xr:
+Error: unable to process resource XNopResource/invalid-schema-xr: cannot validate resources: ns.diff.example.org/v1alpha1/XNopResource default/invalid-schema-xr:
   spec.coolField: Invalid value: "number": ... [schema]
 ns.nop.example.org/v1alpha1/XDownstreamResource default/invalid-schema-xr:
   spec.forProvider.configData: Invalid value: "boolean": ... [schema]
@@ -1166,9 +1166,9 @@ Each per-resource block starts with a header that includes the resource identity
 - Namespaced resource: `<apiVersion>/<Kind> <namespace>/<name>:`
 - Resource without `metadata.name` (e.g. a resource discovered missing a schema before it was named): collapses to just `<apiVersion>/<Kind>:`
 
-Each indented error line has the shape `<message> [<type>]`, where `<type>` is one of `[schema]`, `[cel]`, `[unknownField]`, or `[defaulting]`. A bad value is appended as `(got <value>)` when it isn't already substring-present in the message. When some inputs in a batched run succeed and others fail validation, the successful diffs appear on stdout and the failing inputs' `ERROR:` blocks appear on stderr.
+Each indented error line has the shape `<message> [<type>]`, where `<type>` is one of `[schema]`, `[cel]`, `[unknownField]`, or `[defaulting]`. A bad value is appended as `(got <value>)` when it isn't already substring-present in the message. When some inputs in a batched run succeed and others fail validation, the successful diffs appear on stdout and the failing inputs' `Error:` blocks appear on stderr.
 
-This per-resource block form belongs to the local validator. A cluster rejection has nothing to break into field-level lines — the apiserver hands back one message — so it prints as a single `ERROR:` line and carries no `[<type>]` suffix. Its `"admission"` type is visible in structured output only.
+This per-resource block form belongs to the local validator. A cluster rejection has nothing to break into field-level lines — the apiserver hands back one message — so it prints as a single `Error:` line and carries no `[<type>]` suffix. Its `"admission"` type is visible in structured output only.
 
 **Machine-readable output** (`crossplane-diff xr invalid-xr.yaml --output json`):
 
@@ -1222,7 +1222,7 @@ The `OutputError` schema:
 | Field | Type | Description |
 |-------|------|-------------|
 | `resourceID` | string | Identifies which user-supplied input the diff was processing (one entry per batched run). Format: `<Kind>/<name>`. |
-| `message` | string | Human-readable error string — the same text written to stderr. |
+| `message` | string | Human-readable error string — the same text the command prints to stderr as part of its error. |
 | `validationFailures` | `[]ResourceValidationFailure`, optional | Structured per-resource breakdown. Set for local schema-validation failures and for cluster rejections during the dry run; `nil` for tool, IO, render, and scope-check errors. |
 
 `ResourceValidationFailure` carries `apiVersion`, `kind`, `name`, `namespace`, `status` (one of `"invalid"` or `"missingSchema"` — `"valid"` rows are filtered out), and `errors`, a list of `FieldValidationError` records:

cmd/diff/diff_integration_test.go

@@ -485,6 +485,12 @@ func runIntegrationTest(t *testing.T, testType DiffTestType, tt IntegrationTestC
 		t.Errorf("expected exit code %d, got %d", tt.expectedExitCode, exitCode.Code)
 	}
 
+	// Print a returned error as main does, the only place errors are printed, so the stderr
+	// assertions see what a user sees.
+	if err != nil {
+		fmt.Fprintf(&stderr, "Error: %v\n", err)
+	}
+
 	// Assert stderr expectations before any of the early returns below, so they apply in every
 	// output mode (human-readable, JSON, and expected-error cases alike).
 	for _, want := range tt.expectedStderrContains {

cmd/diff/diffprocessor/comp_processor.go

@@ -270,18 +270,37 @@ func (p *DefaultCompDiffProcessor) DiffComposition(ctx context.Context, composit
 	totalXRErrors := len(output.Errors)
 	output.Errors = withInterruption(ctx, output.Errors)
 
-	// Return an error so CI/CD pipelines get a non-zero exit code when impact analysis failed. The
-	// report is returned with it, so the caller still renders it (structured output stays valid).
+	// Return an error so CI/CD pipelines get a non-zero exit code. The report is returned with it, so
+	// the caller still renders it (structured output stays valid). The renderers do not print errors,
+	// so the returned error names each failing resource; the command prints it once. A run in which
+	// only some compositions failed still succeeds: each failure is in the output, under its composition.
+	var errs []error
+
 	if totalXRErrors > 0 {
-		return output, errors.Errorf("impact analysis failed for %d XR(s)", totalXRErrors)
+		errs = append(errs, errors.Wrapf(outputErrorsJoined(output.Errors[:totalXRErrors]), "impact analysis failed for %d XR(s)", totalXRErrors))
 	}
 
-	// Return error if all compositions failed
 	if compositionErrors > 0 && compositionErrors == len(output.Compositions) {
-		return output, errors.New("failed to process all compositions")
+		failed := make([]dt.OutputError, 0, compositionErrors)
+		for _, comp := range output.Compositions {
+			failed = append(failed, *comp.Error)
+		}
+
+		errs = append(errs, errors.Wrap(outputErrorsJoined(failed), "failed to process all compositions"))
+	}
+
+	return output, errors.Join(errs...)
+}
+
+// outputErrorsJoined returns errs as one error that names each one's resource, the way PerformDiff
+// names the resources it failed to process.
+func outputErrorsJoined(errs []dt.OutputError) error {
+	joined := make([]error, 0, len(errs))
+	for _, e := range errs {
+		joined = append(joined, errors.Errorf("unable to process resource %s: %s", e.ResourceID, e.Message))
 	}
 
-	return output, nil
+	return errors.Join(joined...)
 }
 
 // preflightResourceRefs resolves user --resource refs against every supplied composition before

cmd/diff/diffprocessor/comp_processor_test.go

@@ -1640,15 +1640,15 @@ func TestDefaultCompDiffProcessor_DiffComposition_ReportErrorOutput(t *testing.T
 		t.Error("Expected error due to XR processing failure, got nil")
 	}
 
-	// The renderer writes each as FormatError() does: "ERROR: {ResourceID}: {Message}".
 	if output == nil || len(output.Errors) != 1 {
 		t.Fatalf("Expected one error in the report, got: %+v", output)
 	}
 
-	got := output.Errors[0].FormatError()
-	for _, want := range []string{"ERROR:", "XResource/fail-xr", "render pipeline failed"} {
+	// The returned error is the only place a human sees the failure, so it names the resource.
+	got := err.Error()
+	for _, want := range []string{"impact analysis failed for 1 XR(s)", "XResource/fail-xr", "render pipeline failed"} {
 		if !strings.Contains(got, want) {
-			t.Errorf("Expected the report's error to contain %q, got: %q", want, got)
+			t.Errorf("Expected the returned error to contain %q, got: %q", want, got)
 		}
 	}
 }

cmd/diff/diffprocessor/diff_processor.go

@@ -331,7 +331,7 @@ func (p *DefaultDiffProcessor) PerformDiff(ctx context.Context, resources []*un.
 	}
 
 	// Third: renders that reach one resource in ways no single diff can represent. Collected into the
-	// report so they reach errors[] and stderr like any other error; the report is still rendered, so
+	// report so they reach errors[] and the returned error like any other error; the report is still rendered, so
 	// structured output stays valid.
 	for _, err := range validator.RenderOverlaps(groups) {
 		errs = append(errs, err)

cmd/diff/diffprocessor/diff_processor_test.go

@@ -1178,13 +1178,13 @@ func TestDefaultDiffProcessor_PerformDiff_ReportErrorOutput(t *testing.T) {
 		t.Fatal("PerformDiff() expected error but got none")
 	}
 
-	// The renderer writes each as FormatError() does: "ERROR: {ResourceID}: {Message}".
+	// The returned error is the only place a human sees the failure, so it names the resource.
 	if len(report.Errors) != 1 {
 		t.Fatalf("Expected one error in the report, got: %v", report.Errors)
 	}
 
-	if got := report.Errors[0].FormatError(); !strings.Contains(got, "ERROR: XR1/my-xr-1:") || !strings.Contains(got, "composition not found") {
-		t.Errorf("Expected the report's error to name XR1/my-xr-1 and 'composition not found', got: %q", got)
+	if got := err.Error(); !strings.Contains(got, "XR1/my-xr-1") || !strings.Contains(got, "composition not found") {
+		t.Errorf("Expected the returned error to name XR1/my-xr-1 and 'composition not found', got: %q", got)
 	}
 }
 

cmd/diff/diffprocessor/errors.go

@@ -46,7 +46,7 @@ const (
 	ExitCodeInterrupted = 130
 )
 
-// ErrInterrupted is the error an interrupted run reports, in errors[] and on stderr, in place of the
+// ErrInterrupted is the error an interrupted run reports, in errors[] and as the command's error, in place of the
 // cancelled calls the interruption caused.
 var ErrInterrupted = errors.New("run interrupted by signal; results are incomplete")
 
@@ -197,7 +197,7 @@ func NewOutputError(resourceID string, err error) dt.OutputError {
 //
 // The inner SchemaValidationError deliberately gets an empty ResourceID: its
 // Error() would otherwise prefix "schema validation error for <id>: ", which
-// both duplicates the id that OutputError.FormatError already prints and
+// both duplicates the id OutputError.ResourceID already carries and
 // mislabels an admission rejection as a schema problem.
 func NewAdmissionRejectionError(rejectedDesc string, obj *un.Unstructured, err error) *SchemaValidationError {
 	msg := fmt.Sprintf("the cluster rejected %s: %s", rejectedDesc, err.Error())

cmd/diff/diffprocessor/warnings.go

@@ -43,9 +43,9 @@ import (
 //
 // # Why it writes to stderr itself
 //
-// Owning the human rendering here means one mechanism produces both views, the same dual-emission
-// contract OutputError follows, and the text stays fit for a CLI rather than taking a logger's
-// development-mode format (timestamp, level, caller).
+// Owning the human rendering here means one mechanism produces both views, and the text stays fit
+// for a CLI rather than taking a logger's development-mode format (timestamp, level, caller).
+// Errors differ: they are returned to the command, which prints them once.
 //
 // # Why identical advisories are collapsed
 //

cmd/diff/main.go

@@ -256,11 +256,8 @@ func main() {
 	err := ctx.Run()
 	// Handle error output - commands set exitCode.Code based on their results
 	if err != nil {
-		// Schema validation errors are already rendered by the processors as part of the
-		// diff output, so don't duplicate them here. Only print other (tool) errors.
-		if !dp.IsSchemaValidationError(err) {
-			fmt.Fprintf(os.Stderr, "Error: %v\n", err)
-		}
+		// The renderers do not print errors, so this is the only place a human sees them.
+		fmt.Fprintf(os.Stderr, "Error: %v\n", err)
 		// If the command returned an error but didn't set an exit code, default to tool error
 		if exitCode.Code == dp.ExitCodeSuccess {
 			exitCode.Code = dp.ExitCodeToolError

cmd/diff/renderer/comp_diff_renderer.go

@@ -40,8 +40,9 @@ const (
 // Both human-readable and structured (JSON/YAML) renderers implement this interface.
 type CompDiffRenderer interface {
 	// RenderCompDiff renders the complete composition diff output.
-	// Top-level and tool errors go to DiffOptions.Stderr and are included in
-	// structured output payloads. Per-composition messages (diffs, "no changes",
+	// Top-level errors are included in structured output payloads but not
+	// written to stderr: the processor returns them to the command, which
+	// prints them once. Per-composition messages (diffs, "no changes",
 	// per-composition errors) go to DiffOptions.Stdout as part of the diff narrative.
 	RenderCompDiff(output *dt.CompDiffOutput) error
 }
@@ -63,8 +64,8 @@ func NewDefaultCompDiffRenderer(logger logging.Logger, diffRenderer DiffRenderer
 }
 
 // RenderCompDiff renders the composition diff in human-readable format.
-// Top-level errors go to r.opts.Stderr. Per-composition output (diffs, status
-// messages, per-composition errors) goes to r.opts.Stdout.
+// Per-composition output (diffs, status messages, per-composition errors) goes
+// to r.opts.Stdout. Top-level errors are not printed; see CompDiffRenderer.
 func (r *DefaultCompDiffRenderer) RenderCompDiff(output *dt.CompDiffOutput) error {
 	stdout := r.opts.Stdout
 
@@ -82,18 +83,7 @@ func (r *DefaultCompDiffRenderer) RenderCompDiff(output *dt.CompDiffOutput) erro
 
 	// One run-wide summary of unverified additions; see DiffRenderer.RenderDiffs for why it is derived
 	// at render time, and why the per-composition reuse of the diff renderer above does not emit it.
-	if err := writeWarnings(r.opts.Stderr, dt.CompDryRunWarnings(output)); err != nil {
-		return err
-	}
-
-	// Write top-level errors to stderr
-	for _, e := range output.Errors {
-		if _, err := fmt.Fprintln(r.opts.Stderr, e.FormatError()); err != nil {
-			return errors.Wrap(err, "failed to write error to stderr")
-		}
-	}
-
-	return nil
+	return writeWarnings(r.opts.Stderr, dt.CompDryRunWarnings(output))
 }
 
 // renderComposition renders every section belonging to a single composition: its own changes,
@@ -612,8 +602,8 @@ func NewStructuredCompDiffRenderer(logger logging.Logger, opts DiffOptions) Comp
 }
 
 // RenderCompDiff renders the composition diff in structured format (JSON/YAML).
-// Top-level errors go to both r.opts.Stderr (for human visibility) and the
-// structured output payload. Per-composition data goes to r.opts.Stdout.
+// Top-level errors go into the payload only; see CompDiffRenderer.
+// Per-composition data goes to r.opts.Stdout.
 func (r *StructuredCompDiffRenderer) RenderCompDiff(output *dt.CompDiffOutput) error {
 	// One run-wide summary of unverified additions, derived here from the diffs and appended after the
 	// warnings raised during the run. See DiffRenderer.RenderDiffs.
@@ -626,14 +616,9 @@ func (r *StructuredCompDiffRenderer) RenderCompDiff(output *dt.CompDiffOutput) e
 	v2.Warnings = warnings
 
 	// Anything but v2, including the empty value, is v1.
-	var (
-		jsonOutput any = v2
-		errs           = v2.Errors
-	)
-
+	var jsonOutput any = v2
 	if r.opts.OutputVersion != OutputVersionV2 {
-		v1 := v2.v1()
-		jsonOutput, errs = v1, v1.Errors
+		jsonOutput = v2.v1()
 	}
 
 	var (
@@ -662,18 +647,7 @@ func (r *StructuredCompDiffRenderer) RenderCompDiff(output *dt.CompDiffOutput) e
 	}
 
 	// Of the warnings, only the ones derived here go to stderr: the rest went there when raised.
-	if err := writeWarnings(r.opts.Stderr, dryRun); err != nil {
-		return err
-	}
-
-	// Write errors to stderr for human visibility (they're also included in the structured output)
-	for _, e := range errs {
-		if _, err := fmt.Fprintln(r.opts.Stderr, e.FormatError()); err != nil {
-			return errors.Wrap(err, "failed to write error to stderr")
-		}
-	}
-
-	return nil
+	return writeWarnings(r.opts.Stderr, dryRun)
 }
 
 // buildStructuredCompOutput converts internal CompDiffOutput to the v2 document, from which v1 is