diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index d8dc2f7..a5e9743 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Shared reporting and PromptKit adoption remediation -Status: six open findings; R1 complete. R1–R3 are from the architectural review of +Status: five open findings; R1–R2 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. @@ -73,6 +73,12 @@ Do not freeze incidental text layout. ## R2 — P2: PromptKit configuration cancellation discards captured evidence +**Complete.** The loader handoff now explicitly distinguishes an unavailable +inspection from a captured one and retains the latter on cancellation. ConfigScope +exposes only its sources and findings on failure. Native-evidence, source-report, +reached-scope validation, cancellation, and separate delivery-failure checks pass, +as does the independent ckprompt suite with Go 1.27.0. + **Location:** [adapters.go](../../integrations/ckprompt/adapters.go), loadPromptKitEngine lines 487–495 and ConfigScope.Acquire lines 276–287. diff --git a/integrations/ckprompt/adapters.go b/integrations/ckprompt/adapters.go index 1a73085..46dfbd0 100644 --- a/integrations/ckprompt/adapters.go +++ b/integrations/ckprompt/adapters.go @@ -281,6 +281,9 @@ func ConfigScope[I, S any](load func(context.Context, ckconfig.Selection, I, S) if errors.As(err, &loadError) && loadError != nil { acquired.Value.Sources = loadError acquired.Value.Findings = &ckconfig.Findings{Records: loadError.Diagnostics(), Messages: messages.diagnostics} + } else if inspection != nil { + acquired.Value.Sources = *inspection + acquired.Value.Findings = &ckconfig.Findings{Records: inspection.Diagnostics(), Messages: messages.diagnostics} } else { acquired.Diagnostics = messages.diagnostics } @@ -289,9 +292,9 @@ func ConfigScope[I, S any](load func(context.Context, ckconfig.Selection, I, S) metadata := inspection.Metadata() return commandkit.Acquired[ckconfig.View]{ Value: ckconfig.View{ - Effective: inspection, - Sources: inspection, - Explanation: inspection, + Effective: *inspection, + Sources: *inspection, + Explanation: *inspection, Metadata: &metadata, Findings: &ckconfig.Findings{Records: inspection.Diagnostics(), Messages: messages.diagnostics}, }, @@ -322,7 +325,7 @@ func ConfigScope[I, S any](load func(context.Context, ckconfig.Selection, I, S) return commandkit.Acquired[[]configkit.Difference]{Diagnostics: messages.diagnostics, Explanation: messages.explanation}, fmt.Errorf("PromptKit scope %q comparison before native diff: %w", scope.Name, err) } - differences, err := leftInspection.Diff(rightInspection) + differences, err := leftInspection.Diff(*rightInspection) messages = messages.withOperationError(err) if err != nil { return commandkit.Acquired[[]configkit.Difference]{Diagnostics: messages.diagnostics, Explanation: messages.explanation}, fmt.Errorf("PromptKit scope %q native configuration diff: %w", scope.Name, err) @@ -471,31 +474,31 @@ func loadPromptKitEngine[I, S any]( state S, side string, load engineLoader[I, S], -) (*promptkit.Engine, promptkit.ConfigurationInspection, promptKitAcquisition, error) { +) (*promptkit.Engine, *promptkit.ConfigurationInspection, promptKitAcquisition, error) { engine, err := load(ctx, selection, input, state) if err != nil { var loadErr *configkit.LoadError if errors.As(err, &loadErr) { messages := promptKitDiagnostics("promptkit", side, loadErr.Sources(), loadErr.Diagnostics()) - return nil, promptkit.ConfigurationInspection{}, messages, err + return nil, nil, messages, err } - return nil, promptkit.ConfigurationInspection{}, promptKitAcquisition{}, err + return nil, nil, promptKitAcquisition{}, err } if engine == nil { - return nil, promptkit.ConfigurationInspection{}, promptKitAcquisition{}, errors.New("PromptKit configuration loader returned a nil engine") + return nil, nil, promptKitAcquisition{}, errors.New("PromptKit configuration loader returned a nil engine") } inspection, err := engine.InspectConfiguration() messages := promptKitDiagnostics("promptkit", side, inspection.Sources(), inspection.Diagnostics()) messages = messages.withOperationError(err) if err != nil { - return nil, inspection, messages, err + return nil, nil, messages, err } if ctx != nil { if err := ctx.Err(); err != nil { - return nil, promptkit.ConfigurationInspection{}, messages, err + return nil, &inspection, messages, err } } - return engine, inspection, messages, nil + return engine, &inspection, messages, nil } func appendPromptKitAcquisition(left, right promptKitAcquisition) promptKitAcquisition { diff --git a/integrations/ckprompt/adapters_test.go b/integrations/ckprompt/adapters_test.go index 732f1e2..5cb78bd 100644 --- a/integrations/ckprompt/adapters_test.go +++ b/integrations/ckprompt/adapters_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "fmt" "io" "reflect" "slices" @@ -858,18 +859,79 @@ func TestFromEngineRetainsConfigKitLoadExplanation(t *testing.T) { } } -func TestConfigScopeHonorsCancellationAfterLoad(t *testing.T) { +func TestConfigScopeRetainsCapturedEvidenceAfterCancellation(t *testing.T) { + engine := adapterEngine(t) + native, err := engine.InspectConfiguration() + if err != nil { + t.Fatal(err) + } ctx, cancel := context.WithCancel(context.Background()) defer cancel() - engine := adapterEngine(t) - scope := ConfigScope(func(context.Context, ckconfig.Selection, int, int) (*promptkit.Engine, error) { + scope := ConfigScope(func(context.Context, ckconfig.Selection, struct{}, struct{}) (*promptkit.Engine, error) { cancel() return engine, nil }) - - acquired, err := scope.Acquire(ctx, ckconfig.Request{Operation: ckconfig.Show}, 0, 0) - if !errors.Is(err, context.Canceled) || acquired.Value.Effective != nil { - t.Fatalf("cancelled scope acquisition = (%#v, %v), want cancellation and no value", acquired, err) + acquired, err := scope.Acquire(ctx, ckconfig.Request{}, struct{}{}, struct{}{}) + if !errors.Is(err, context.Canceled) || acquired.Value.Effective != nil || acquired.Value.Metadata != nil || acquired.Value.Explanation != nil || acquired.Value.Sources == nil || acquired.Value.Findings == nil { + t.Fatalf("canceled acquisition = %#v, %v", acquired, err) + } + if !reflect.DeepEqual(acquired.Value.Sources.Sources(), native.Sources()) || !reflect.DeepEqual(acquired.Value.Findings.Records, native.Diagnostics()) { + t.Fatalf("lost captured evidence: %#v", acquired.Value) + } + for _, operation := range []string{"sources", "validate"} { + for _, failOutput := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/failOutput=%t", operation, failOutput), func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + scope := ConfigScope(func(context.Context, ckconfig.Selection, struct{}, struct{}) (*promptkit.Engine, error) { + cancel() + return engine, nil + }) + unvisited := 0 + app, err := commandkit.New(commandkit.Identity{Name: "fixture"}, ckconfig.Command(ckconfig.Suite[struct{}, struct{}]{Scopes: []ckconfig.Scope[struct{}, struct{}]{ + {Name: "app", Operations: []ckconfig.Operation{ckconfig.Validate}, Acquire: func(context.Context, ckconfig.Request, struct{}, struct{}) (commandkit.Acquired[ckconfig.View], error) { + return commandkit.Acquired[ckconfig.View]{}, nil + }}, + scope, + {Name: "zeta", Operations: []ckconfig.Operation{ckconfig.Validate}, Acquire: func(context.Context, ckconfig.Request, struct{}, struct{}) (commandkit.Acquired[ckconfig.View], error) { + unvisited++ + return commandkit.Acquired[ckconfig.View]{}, nil + }}, + }})) + if err != nil { + t.Fatal(err) + } + args := []string{"config", operation, "--scope=promptkit", "--format=json"} + if operation == "validate" { + args = []string{"config", "validate", "--scope=all", "--details", "--format=json"} + } + var out bytes.Buffer + var writer io.Writer = &out + writeErr := errors.New("output failed") + if failOutput { + writer = failingDiagnosticWriter{err: writeErr} + } + got := app.Execute(ctx, args, commandkit.Streams{Out: writer, Err: io.Discard}) + if !errors.Is(got.Cause, context.Canceled) || unvisited != 0 { + t.Fatalf("outcome=%#v unvisited=%d", got, unvisited) + } + if failOutput { + if !errors.Is(got.Presentation.Output.WriteError, writeErr) { + t.Fatalf("lost presentation failure: %#v", got) + } + } else { + if operation == "sources" { + for _, source := range native.Sources() { + if !strings.Contains(out.String(), source.ID) { + t.Fatalf("missing acquired source: %s", out.String()) + } + } + } else if !strings.Contains(out.String(), `"state": "empty"`) || !strings.Contains(out.String(), `"scope": "app"`) || strings.Contains(out.String(), `"scope": "zeta"`) { + t.Fatalf("reached-scope evidence: %s", out.String()) + } + } + }) + } } }