diff --git a/cmd/releasekit/main.go b/cmd/releasekit/main.go index 41f4e84..c295df1 100644 --- a/cmd/releasekit/main.go +++ b/cmd/releasekit/main.go @@ -155,10 +155,6 @@ func candidateRequest(invocation cli.Invocation, settings cli.Settings, output i } func candidateFailureText(err error) string { - var validationError *releasekit.ValidationError - if errors.As(err, &validationError) { - return validationError.Error() - } return err.Error() } diff --git a/cmd/releasekit/main_test.go b/cmd/releasekit/main_test.go index 9dd3455..97f7f75 100644 --- a/cmd/releasekit/main_test.go +++ b/cmd/releasekit/main_test.go @@ -17,6 +17,7 @@ import ( "strings" "testing" + releasekit "gitea.maximumdirect.net/eric/ReleaseKit" "gitea.maximumdirect.net/eric/ReleaseKit/buildinfo" "gitea.maximumdirect.net/eric/ReleaseKit/cmd/releasekit/internal/cli" ) @@ -149,6 +150,20 @@ func TestCandidateCommandsPresentValidationDiagnosticsOnce(t *testing.T) { } } +func TestCandidateFailurePresentationRetainsCallbackContext(t *testing.T) { + diagnostic := &releasekit.ValidationError{ + Check: "examples", + Findings: []releasekit.ValidationFinding{{Path: "examples/run/main.go", Message: "unexpected output"}}, + } + err := fmt.Errorf("validate candidate captured-commit: %w", diagnostic) + text := candidateFailureText(err) + for _, part := range []string{"captured-commit", "examples", "examples/run/main.go", "unexpected output"} { + if !strings.Contains(text, part) { + t.Errorf("CLI presentation %q omits %q", text, part) + } + } +} + func TestPublishCommandPublishesConfiguredCandidate(t *testing.T) { repository := newCLIRepository(t) writeRepositoryFile(t, repository, "docs/releases/v1.2.3.md", "# Example Product v1.2.3\n\n## Compatibility\n\nNo action required.\n") diff --git a/docs/internal/validation.md b/docs/internal/validation.md index fdaba14..d92188e 100644 --- a/docs/internal/validation.md +++ b/docs/internal/validation.md @@ -21,6 +21,12 @@ for those child commands. Missing local objects produce inspection errors; source preparation retains its separate network-capable operations. See Git's [lazy-fetch controls](https://git-scm.com/docs/git) for the command mechanism. +Public validation errors convert their diagnostic fields to the internal +failure model for shared formatting. Converted failures retain their original +contextual cause and reuse its rendering when it already contains the same +diagnostics. The CLI presents the complete error chain, including candidate +context around callback-supplied diagnostics. + ## Adding a shared check When an optional check is accepted for implementation: diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 317a810..e555c19 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Shared validation foundation remediation -Status: Remediation in progress; findings 1–3 addressed, findings 4–6 pending. +Status: Remediation in progress; findings 1–4 addressed, findings 5–6 pending. Reviewed committed implementation `f53354b`, covering the eight foundation commits after `0c7129a`, against the [implementation plan](implementation.md), @@ -130,6 +130,15 @@ reads from fully populated candidates. ## 4. P2: Public and CLI error presentation loses diagnostic information +**Resolution:** Addressed. Public errors use the internal diagnostic formatter +with explicit conversions, including findings, targets, directives, +measurements, and causes. Matching converted diagnostics reuse their contextual +cause's rendering once. The CLI preserves outer wrappers, and callback errors +retain their public identity. Regression tests cover both candidate operations +and caller-created errors with and without causes. +`go test . ./cmd/releasekit ./internal/validation ./internal/apicheck` and +`go run ./internal/apicheck` passed. + **Owners:** [public diagnostics](../../validation_error.go), `ValidationError.Error` and its formatter; [CLI presentation](../../cmd/releasekit/main.go), `candidateFailureText`; [internal formatting](../../internal/validation/diagnostic.go). diff --git a/internal/validation/diagnostic.go b/internal/validation/diagnostic.go index 516f8cd..eddc834 100644 --- a/internal/validation/diagnostic.go +++ b/internal/validation/diagnostic.go @@ -73,6 +73,12 @@ func formatFailure(failure *Failure) string { summary.WriteString(": ") } summary.WriteString(finding.Message) + if finding.Target != "" { + fmt.Fprintf(&summary, " (target %q)", finding.Target) + } + if finding.Directive != "" { + fmt.Fprintf(&summary, " (directive %q)", finding.Directive) + } } if failure.Measurement != nil { fmt.Fprintf(&summary, ": measured %d bytes against a %d byte limit", failure.Measurement.Bytes, failure.Measurement.LimitBytes) diff --git a/validation_error.go b/validation_error.go index 9989277..c69e2bf 100644 --- a/validation_error.go +++ b/validation_error.go @@ -2,8 +2,7 @@ package releasekit import ( "errors" - "fmt" - "strings" + "slices" "gitea.maximumdirect.net/eric/ReleaseKit/internal/validation" ) @@ -32,21 +31,24 @@ type FileSizeMeasurement struct { // ValidationError describes a failed shared validation check returned by // [Check] or [Publish]. Cause preserves the contextual and underlying error chain. type ValidationError struct { - Check ValidationCheck // failed shared check - Findings []ValidationFinding // actionable observations, when available + Check ValidationCheck // failed shared check + Findings []ValidationFinding // actionable observations, when available Measurement *FileSizeMeasurement // measured file size, when applicable - Cause error // original contextual and underlying error chain + Cause error // original contextual and underlying error chain } -// Error returns the contextual validation error summary. +// Error returns check details alongside the contextual and underlying causes. func (failure *ValidationError) Error() string { if failure == nil { return "" } - if failure.Cause != nil { + diagnostic := internalValidationFailure(failure) + var original *validation.Failure + if errors.As(failure.Cause, &original) && sameValidationDiagnostics(diagnostic, original) { + // Converted causes already render these diagnostics and candidate context. return failure.Cause.Error() } - return formatPublicValidationError(failure) + return diagnostic.Error() } // Unwrap returns the original validation error chain. @@ -61,6 +63,10 @@ func convertValidationError(err error) error { if err == nil { return nil } + var publicFailure *ValidationError + if errors.As(err, &publicFailure) { + return err + } var failure *validation.Failure if !errors.As(err, &failure) { return err @@ -91,31 +97,30 @@ func convertValidationError(err error) error { return result } -func formatPublicValidationError(failure *ValidationError) string { - var summary strings.Builder - if failure.Check == "" { - summary.WriteString("validation failed") - } else { - fmt.Fprintf(&summary, "%s validation failed", failure.Check) - } +func internalValidationFailure(failure *ValidationError) *validation.Failure { + result := &validation.Failure{Check: validation.Check(failure.Check), Cause: failure.Cause} for _, finding := range failure.Findings { - summary.WriteString(": ") - if finding.Path != "" { - summary.WriteString(finding.Path) - if finding.Line > 0 { - fmt.Fprintf(&summary, ":%d", finding.Line) - if finding.Column > 0 { - fmt.Fprintf(&summary, ":%d", finding.Column) - } - } - summary.WriteString(": ") - } - summary.WriteString(finding.Message) + result.Findings = append(result.Findings, validation.Finding{ + Path: finding.Path, Line: finding.Line, Column: finding.Column, + Message: finding.Message, Target: finding.Target, Directive: finding.Directive, + }) } if failure.Measurement != nil { - fmt.Fprintf(&summary, ": measured %d bytes against a %d byte limit", failure.Measurement.Bytes, failure.Measurement.LimitBytes) + result.Measurement = &validation.FileSizeMeasurement{ + Bytes: failure.Measurement.Bytes, LimitBytes: failure.Measurement.LimitBytes, + } } - return summary.String() + return result +} + +func sameValidationDiagnostics(a, b *validation.Failure) bool { + if b == nil || a.Check != b.Check || !slices.Equal(a.Findings, b.Findings) { + return false + } + if a.Measurement == nil || b.Measurement == nil { + return a.Measurement == b.Measurement + } + return *a.Measurement == *b.Measurement } var _ error = (*ValidationError)(nil) diff --git a/validation_error_test.go b/validation_error_test.go new file mode 100644 index 0000000..9deeff3 --- /dev/null +++ b/validation_error_test.go @@ -0,0 +1,62 @@ +package releasekit_test + +import ( + "context" + "errors" + "strings" + "testing" + + releasekit "gitea.maximumdirect.net/eric/ReleaseKit" +) + +func TestValidationErrorPresentsFindingsAlongsideCause(t *testing.T) { + cause := errors.New("inspection failed") + for _, withCause := range []bool{false, true} { + diagnostic := &releasekit.ValidationError{ + Check: "file-size", + Findings: []releasekit.ValidationFinding{{Path: "main.go", Line: 3, Column: 2, Message: "oversized source", Target: "linked.go", Directive: "example directive"}}, + Measurement: &releasekit.FileSizeMeasurement{Bytes: 12, LimitBytes: 10}, + } + if withCause { + diagnostic.Cause = cause + } + for _, part := range []string{"file-size", "main.go:3:2", "oversized source", "linked.go", "example directive", "12 bytes", "10 byte limit"} { + if !strings.Contains(diagnostic.Error(), part) { + t.Errorf("withCause=%t: %q omits %q", withCause, diagnostic.Error(), part) + } + } + if withCause && (!errors.Is(diagnostic, cause) || !strings.Contains(diagnostic.Error(), cause.Error())) { + t.Fatalf("diagnostic lost its cause: %v", diagnostic) + } + } +} + +func TestCandidateOperationsPreserveCallbackValidationDiagnostics(t *testing.T) { + repo := newPublishFixture(t) + cause := errors.New("example execution failed") + diagnostic := &releasekit.ValidationError{ + Check: "examples", + Findings: []releasekit.ValidationFinding{{Path: "examples/run/main.go", Message: "unexpected output"}}, + Cause: cause, + } + request := releasekit.ReleaseRequest{ + RepositoryDir: repo.work, Product: "Sample App", Version: "v1.2.3", + AdditionalChecks: func(context.Context, releasekit.Candidate) error { return diagnostic }, + } + _, checkError := releasekit.Check(t.Context(), request) + result, publishError := releasekit.Publish(t.Context(), request) + for _, err := range []error{checkError, publishError} { + var got *releasekit.ValidationError + if !errors.As(err, &got) || got != diagnostic || !errors.Is(err, cause) { + t.Fatalf("callback diagnostic identity or cause lost: %v", err) + } + for _, part := range []string{"validate candidate", "examples/run/main.go", "unexpected output", cause.Error()} { + if !strings.Contains(err.Error(), part) { + t.Errorf("callback error %q omits %q", err, part) + } + } + } + if result.State != releasekit.PublicationNotAttempted || result.TagObject != "" { + t.Fatalf("callback failure publication result = %#v", result) + } +}