Retain captured PromptKit evidence after cancellation
This commit is contained in:
@@ -1,6 +1,6 @@
|
|||||||
# Shared reporting and PromptKit adoption remediation
|
# 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
|
`f83866e44a8a9066db52cbd2aa5cccde0d69a839`. R4–R7 are from the algorithmic
|
||||||
audit of `f5b2d1f80c4720b762f3301c29d5050710c210d0`; R4 is a correctness
|
audit of `f5b2d1f80c4720b762f3301c29d5050710c210d0`; R4 is a correctness
|
||||||
issue and R5–R7 are efficiency opportunities for consideration.
|
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
|
## 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),
|
**Location:** [adapters.go](../../integrations/ckprompt/adapters.go),
|
||||||
loadPromptKitEngine lines 487–495 and ConfigScope.Acquire lines 276–287.
|
loadPromptKitEngine lines 487–495 and ConfigScope.Acquire lines 276–287.
|
||||||
|
|
||||||
|
|||||||
@@ -281,6 +281,9 @@ func ConfigScope[I, S any](load func(context.Context, ckconfig.Selection, I, S)
|
|||||||
if errors.As(err, &loadError) && loadError != nil {
|
if errors.As(err, &loadError) && loadError != nil {
|
||||||
acquired.Value.Sources = loadError
|
acquired.Value.Sources = loadError
|
||||||
acquired.Value.Findings = &ckconfig.Findings{Records: loadError.Diagnostics(), Messages: messages.diagnostics}
|
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 {
|
} else {
|
||||||
acquired.Diagnostics = messages.diagnostics
|
acquired.Diagnostics = messages.diagnostics
|
||||||
}
|
}
|
||||||
@@ -289,9 +292,9 @@ func ConfigScope[I, S any](load func(context.Context, ckconfig.Selection, I, S)
|
|||||||
metadata := inspection.Metadata()
|
metadata := inspection.Metadata()
|
||||||
return commandkit.Acquired[ckconfig.View]{
|
return commandkit.Acquired[ckconfig.View]{
|
||||||
Value: ckconfig.View{
|
Value: ckconfig.View{
|
||||||
Effective: inspection,
|
Effective: *inspection,
|
||||||
Sources: inspection,
|
Sources: *inspection,
|
||||||
Explanation: inspection,
|
Explanation: *inspection,
|
||||||
Metadata: &metadata,
|
Metadata: &metadata,
|
||||||
Findings: &ckconfig.Findings{Records: inspection.Diagnostics(), Messages: messages.diagnostics},
|
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)
|
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)
|
messages = messages.withOperationError(err)
|
||||||
if err != nil {
|
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)
|
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,
|
state S,
|
||||||
side string,
|
side string,
|
||||||
load engineLoader[I, S],
|
load engineLoader[I, S],
|
||||||
) (*promptkit.Engine, promptkit.ConfigurationInspection, promptKitAcquisition, error) {
|
) (*promptkit.Engine, *promptkit.ConfigurationInspection, promptKitAcquisition, error) {
|
||||||
engine, err := load(ctx, selection, input, state)
|
engine, err := load(ctx, selection, input, state)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
var loadErr *configkit.LoadError
|
var loadErr *configkit.LoadError
|
||||||
if errors.As(err, &loadErr) {
|
if errors.As(err, &loadErr) {
|
||||||
messages := promptKitDiagnostics("promptkit", side, loadErr.Sources(), loadErr.Diagnostics())
|
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 {
|
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()
|
inspection, err := engine.InspectConfiguration()
|
||||||
messages := promptKitDiagnostics("promptkit", side, inspection.Sources(), inspection.Diagnostics())
|
messages := promptKitDiagnostics("promptkit", side, inspection.Sources(), inspection.Diagnostics())
|
||||||
messages = messages.withOperationError(err)
|
messages = messages.withOperationError(err)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, inspection, messages, err
|
return nil, nil, messages, err
|
||||||
}
|
}
|
||||||
if ctx != nil {
|
if ctx != nil {
|
||||||
if err := ctx.Err(); err != 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 {
|
func appendPromptKitAcquisition(left, right promptKitAcquisition) promptKitAcquisition {
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"bytes"
|
"bytes"
|
||||||
"context"
|
"context"
|
||||||
"errors"
|
"errors"
|
||||||
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"reflect"
|
"reflect"
|
||||||
"slices"
|
"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())
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
defer cancel()
|
defer cancel()
|
||||||
engine := adapterEngine(t)
|
scope := ConfigScope(func(context.Context, ckconfig.Selection, struct{}, struct{}) (*promptkit.Engine, error) {
|
||||||
scope := ConfigScope(func(context.Context, ckconfig.Selection, int, int) (*promptkit.Engine, error) {
|
|
||||||
cancel()
|
cancel()
|
||||||
return engine, nil
|
return engine, nil
|
||||||
})
|
})
|
||||||
|
acquired, err := scope.Acquire(ctx, ckconfig.Request{}, struct{}{}, struct{}{})
|
||||||
acquired, err := scope.Acquire(ctx, ckconfig.Request{Operation: ckconfig.Show}, 0, 0)
|
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 {
|
||||||
if !errors.Is(err, context.Canceled) || acquired.Value.Effective != nil {
|
t.Fatalf("canceled acquisition = %#v, %v", acquired, err)
|
||||||
t.Fatalf("cancelled scope acquisition = (%#v, %v), want cancellation and no value", 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())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user