From d7a85f5f350fa72e14af3af22b6990d43ddf6356 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 7 Nov 2026 12:04:46 +0000 Subject: [PATCH] Preserve ConfigKit result evidence alongside loader errors --- docs/roadmap/remediation.md | 9 ++- integrations/ckconfig/configkit_adapter.go | 13 +++- .../ckconfig/contracts_adapter_test.go | 72 +++++++++++++++---- 3 files changed, 76 insertions(+), 18 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index a5e9743..9d9ca96 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Shared reporting and PromptKit adoption remediation -Status: five open findings; R1–R2 complete. R1–R3 are from the architectural review of +Status: four open findings; R1–R3 complete. R1–R3 are from the architectural review of `f83866e44a8a9066db52cbd2aa5cccde0d69a839`. R4–R7 are from the algorithmic audit of `f5b2d1f80c4720b762f3301c29d5050710c210d0`; R4 is a correctness issue and R5–R7 are efficiency opportunities for consideration. @@ -114,6 +114,13 @@ should continue to stop domain work. ## R3 — P2: A native ConfigKit result accompanying an error loses evidence +**Complete.** Result-plus-error acquisition now retains sources, findings, and +ordinary messages without exposing effective, explanation, or metadata facets. +A matched LoadError still supplies the sole authoritative snapshot. Regression +checks cover application/cancellation causes, absent versus empty findings, +source and validation reports, warning delivery, and presentation failures. +The independent ckconfig suite passes with Go 1.27.0. + **Location:** [configkit_adapter.go](../../integrations/ckconfig/configkit_adapter.go), FromConfigKit.Acquire lines 35–47. diff --git a/integrations/ckconfig/configkit_adapter.go b/integrations/ckconfig/configkit_adapter.go index c1ac696..f229aa8 100644 --- a/integrations/ckconfig/configkit_adapter.go +++ b/integrations/ckconfig/configkit_adapter.go @@ -43,17 +43,24 @@ func FromConfigKit[I, S, T any]( acquired.Value.Sources = loadError acquired.Value.Findings = &Findings{Records: records, Messages: messages} acquired.Explanation = explanation + return acquired, err } - return acquired, err } if result == nil { + if err != nil { + return acquired, err + } return commandkit.Acquired[View]{}, fmt.Errorf("ConfigKit scope %q returned a nil result", name) } - metadata := result.Metadata() records := result.Diagnostics() messages, explanation := configKitDiagnostics(name, result.Sources(), records) - acquired.Value = View{Effective: result, Sources: result, Explanation: result, Metadata: &metadata, Findings: &Findings{Records: records, Messages: messages}} + acquired.Value = View{Sources: result, Findings: &Findings{Records: records, Messages: messages}} acquired.Explanation = explanation + if err != nil { + return acquired, err + } + metadata := result.Metadata() + acquired.Value.Effective, acquired.Value.Explanation, acquired.Value.Metadata = result, result, &metadata return acquired, nil } scope.Compare = func(ctx context.Context, request ComparisonRequest[I], state S) (commandkit.Acquired[[]configkit.Difference], error) { diff --git a/integrations/ckconfig/contracts_adapter_test.go b/integrations/ckconfig/contracts_adapter_test.go index 2b12f2f..da5b88b 100644 --- a/integrations/ckconfig/contracts_adapter_test.go +++ b/integrations/ckconfig/contracts_adapter_test.go @@ -140,8 +140,12 @@ func TestFromConfigKitRetainsLoadErrorAndDiagnostics(t *testing.T) { if loadErr == nil { t.Fatal("fixture did not produce a ConfigKit load error") } + partial, err := configkit.Load[struct{}](configkit.Mapping("unrelated result", map[string]any{})) + if err != nil { + t.Fatal(err) + } scope := FromConfigKit[struct{}, struct{}, struct{}]("app", "", func(context.Context, Selection, struct{}, struct{}) (*configkit.Result[struct{}], error) { - return nil, loadErr + return partial, loadErr }) acquired, gotErr := scope.Acquire(context.Background(), Request{}, struct{}{}, struct{}{}) if gotErr != loadErr { @@ -157,6 +161,9 @@ func TestFromConfigKitRetainsLoadErrorAndDiagnostics(t *testing.T) { if acquired.Explanation.Message != "" || len(acquired.Explanation.Details) != 1 { t.Fatalf("failure explanation = %#v", acquired.Explanation) } + if !reflect.DeepEqual(acquired.Value.Sources.Sources(), typed.Sources()) || !reflect.DeepEqual(acquired.Value.Findings.Records, typed.Diagnostics()) { + t.Fatal("LoadError snapshot was replaced or duplicated") + } var warning, terminal bytes.Buffer if err := acquired.Value.Findings.Messages[0].Write(&warning); err != nil { t.Fatal(err) @@ -175,25 +182,62 @@ func TestFromConfigKitRetainsLoadErrorAndDiagnostics(t *testing.T) { } } -func TestFromConfigKitDoesNotExposeResultFromFailedLoad(t *testing.T) { +func TestFromConfigKitRetainsResultEvidenceWithApplicationFailure(t *testing.T) { result, err := adapterSettingsLoad(configkit.Mapping("partial source", map[string]any{"greeting": "Hello"})) if err != nil { t.Fatal(err) } - loadErr := errors.New("loader returned a value with an error") - scope := FromConfigKit[struct{}, struct{}, adapterSettings]("app", "", func(context.Context, Selection, struct{}, struct{}) (*configkit.Result[adapterSettings], error) { - return result, loadErr - }) - - acquired, gotErr := scope.Acquire(context.Background(), Request{}, struct{}{}, struct{}{}) - if gotErr != loadErr { - t.Fatalf("load error identity = %v, want %v", gotErr, loadErr) + for _, cause := range []error{errors.New("post-load check failed"), context.Canceled} { + t.Run(cause.Error(), func(t *testing.T) { + scope := FromConfigKit("app", "", func(context.Context, Selection, struct{}, struct{}) (*configkit.Result[adapterSettings], error) { + return result, cause + }) + acquired, gotErr := scope.Acquire(context.Background(), Request{}, struct{}{}, struct{}{}) + if gotErr != cause || acquired.Value.Effective != nil || acquired.Value.Metadata != nil || acquired.Value.Explanation != nil || acquired.Value.Sources == nil || acquired.Value.Findings == nil { + t.Fatalf("failed acquisition = %#v, %v", acquired, gotErr) + } + if !reflect.DeepEqual(acquired.Value.Sources.Sources(), result.Sources()) || !reflect.DeepEqual(acquired.Value.Findings.Records, result.Diagnostics()) || len(acquired.Value.Findings.Messages) != 1 || len(acquired.Explanation.Details) != 0 { + t.Fatalf("lost or duplicated result evidence: %#v", acquired) + } + app := newConfigTestApp(t, Command(Suite[struct{}, struct{}]{Scopes: []Scope[struct{}, struct{}]{scope}})) + for _, args := range [][]string{{"config", "sources", "--format=json"}, {"config", "validate", "--details", "--format=json"}, {"config", "validate", "--verbose"}} { + var out, diag bytes.Buffer + got := app.Execute(context.Background(), args, commandkit.Streams{Out: &out, Err: &diag}) + if !errors.Is(got.Cause, cause) { + t.Fatalf("lost cause: %#v", got) + } + if args[1] == "sources" { + if !strings.Contains(out.String(), "partial source") { + t.Fatalf("lost sources: %s", out.String()) + } + } else if strings.Contains(strings.Join(args, " "), "--details") { + if !strings.Contains(out.String(), result.Diagnostics()[0].Message) || strings.Contains(diag.String(), result.Diagnostics()[0].Message) { + t.Fatalf("detailed evidence: output=%s diag=%s", out.String(), diag.String()) + } + } else if !strings.Contains(diag.String(), result.Diagnostics()[0].Message) { + t.Fatalf("lost ordinary warning: %s", diag.String()) + } + } + writeErr := errors.New("output failed") + got := app.Execute(context.Background(), []string{"config", "validate", "--details"}, commandkit.Streams{Out: failingOutput{err: writeErr}, Err: io.Discard}) + if !errors.Is(got.Cause, cause) || !errors.Is(got.Presentation.Output.WriteError, writeErr) { + t.Fatalf("lost operation/presentation cause: %#v", got) + } + }) } - if acquired.Value.Effective != nil || acquired.Value.Metadata != nil || acquired.Value.Sources != nil || acquired.Value.Explanation != nil || acquired.Value.Findings != nil { - t.Fatalf("failed load exposed result facets: %#v", acquired.Value) + empty, err := configkit.Load[map[string]any](configkit.Mapping("empty", map[string]any{})) + if err != nil { + t.Fatal(err) } - if len(acquired.Explanation.Details) != 0 { - t.Fatalf("warning-only result acquired terminal context: %#v", acquired.Explanation) + for _, result := range []*configkit.Result[map[string]any]{nil, empty} { + cause := errors.New("application failure") + scope := FromConfigKit("app", "", func(context.Context, Selection, struct{}, struct{}) (*configkit.Result[map[string]any], error) { + return result, cause + }) + got, err := scope.Acquire(context.Background(), Request{}, struct{}{}, struct{}{}) + if err != cause || (got.Value.Findings == nil) != (result == nil) { + t.Fatalf("unavailable/empty evidence: %#v %v", got, err) + } } }