diff --git a/docs/roadmap/audit/catalog.md b/docs/roadmap/audit/catalog.md index bbf3c4b..778c727 100644 --- a/docs/roadmap/audit/catalog.md +++ b/docs/roadmap/audit/catalog.md @@ -1,6 +1,6 @@ # Consolidated audit catalog -Status: **27 distinct open findings: five P2 and 22 P3**, consolidated from 30 +Status: **26 distinct open findings: four P2 and 22 P3**, consolidated from 30 numbered entries in seven audit documents. Reviewed audit state: `db732e3a1de55c844f77870e1c6db683c2685d5d`. @@ -9,7 +9,7 @@ decisions. The linked original entries retain code locations, evidence, contract analysis, and detailed remediation options. Consolidation alone does not resolve a finding or select a proposed contract change. Explicit decisions recorded below supersede the -original entries' alternatives; all findings remain open until remediated. +original entries' alternatives; findings remain open until remediated. Original priorities are retained. Within those priorities, incorrect outcomes and request isolation lead; blocked validation, resource retention, and consumer contracts precede scaling and maintainability @@ -19,7 +19,7 @@ The [remediation analysis](remediation-analysis.md) records the completed design review and its rationale. All choices identified for discussion have been resolved; no further product or architectural decision is required before implementation planning. Accepted conclusions are recorded in this catalog; -all 27 findings still require remediation and validation. +the remaining findings still require remediation and validation. Three pairs describe the same underlying issue: correctness C2/concurrency N2, correctness C4/concurrency N3, and complexity M6/responsibility R1. Other @@ -38,6 +38,12 @@ these boundaries. ## F01 — P2: Aggregate smoke testing can stop on cancellation and return success +**Complete.** The later cancellation checkpoint retains a cause when work is +omitted, without duplicating a cause already recorded. Focused aggregate and +cancellation tests pass in the aligned workspace; the new progress-delivery +regression failed against the original behavior for both successful and failed +provider calls, and also covers cancellation after the last completed check. + In ckprompt's `runSmokeAggregate`, cancellation triggered during progress delivery can stop the remaining smoke kinds without entering the operation cause. A partial aggregate can consequently return status 0; cancellation can diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 8483bea..671c4e7 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,10 +1,10 @@ # Audit remediation implementation plan -Status: planned; no stage implemented by this document. +Status: in progress; Stage 1 complete. -Baseline: `d492c7747175c01a53261206c2b4356e53453621`. Source remains the audited -implementation; intervening changes recorded decisions and retired the completed -older remediation. The [roadmap](audit-remediation.md) owns the target and scope, +Baseline: `d492c7747175c01a53261206c2b4356e53453621`. Before this sprint, +intervening changes recorded decisions and retired the completed older +remediation. The [roadmap](audit-remediation.md) owns the target and scope, the [catalog](audit/catalog.md) owns accepted decisions and finding status, and its original audit links own defect evidence. The [analysis](audit/remediation-analysis.md) explains the selected designs. @@ -107,6 +107,13 @@ not converted into a passed check or a request for per-stage approval. ## Stage 1 — Retain aggregate smoke cancellation +**Complete.** The progress-delivery cancellation regression reproduced both +the false success and missing joined cause before the fix. The focused aligned +workspace checks passed: `go test -mod=readonly -run 'TestSmokeAggregate|TestSmokeTestCancellation' ./integrations/ckprompt` +with Go 1.27.0, `GOTOOLCHAIN=local`, and `GOPROXY=off`. Tests cover partial +results with and without a provider error, retained diagnostics and explanation, +omitted callbacks, and cancellation after the final check. + Findings: F01. Primary source: `integrations/ckprompt/smoke_command.go`. 1. In runSmokeAggregate, retain cancellation observed at the later checkpoint diff --git a/integrations/ckprompt/smoke_aggregate_test.go b/integrations/ckprompt/smoke_aggregate_test.go index 8bb4e4a..e528a6c 100644 --- a/integrations/ckprompt/smoke_aggregate_test.go +++ b/integrations/ckprompt/smoke_aggregate_test.go @@ -3,9 +3,11 @@ package ckprompt import ( "bytes" "context" + "encoding/json" "errors" "io" "reflect" + "strings" "testing" "go.maximumdirect.net/ckconfig" @@ -109,6 +111,79 @@ func TestSmokeAggregateCoordinatesConcreteRequestsAndRetainsErrors(t *testing.T) } } +type cancelProgressWriter struct { + bytes.Buffer + cancel context.CancelFunc +} + +func (writer *cancelProgressWriter) Write(p []byte) (int, error) { + writer.cancel() + return writer.Buffer.Write(p) +} + +func TestSmokeAggregateCancellationDuringProgress(t *testing.T) { + providerErr := errors.New("provider failed") + for _, test := range []struct { + name string + err error + cancelAt int + }{ + {name: "partial success", cancelAt: 1}, + {name: "partial provider failure", err: providerErr, cancelAt: 1}, + {name: "last completed test", cancelAt: 4}, + } { + t.Run(test.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + var kinds []promptkit.SmokeTestKind + app := newPromptTestApp(t, Command(Suite[struct{}, struct{}]{ + SmokeTest: func(_ context.Context, request SmokeRequest, _, _ struct{}) (commandkit.Acquired[*promptkit.SmokeTestReport], error) { + kinds = append(kinds, request.Request.Kind) + acquired := commandkit.Acquired[*promptkit.SmokeTestReport]{Value: &promptkit.SmokeTestReport{Kind: request.Request.Kind, Completed: true}} + if len(kinds) == test.cancelAt { + acquired.Diagnostics = []commandkit.Diagnostic{{Severity: commandkit.SeverityProgress, Write: func(w io.Writer) error { _, err := io.WriteString(w, "progress evidence\n"); return err }}} + acquired.Explanation = commandkit.Explanation{Message: "provider explanation"} + return acquired, test.err + } + return acquired, nil + }, + })) + var output bytes.Buffer + diagnostics := &cancelProgressWriter{cancel: cancel} + outcome := app.Execute(ctx, []string{"promptkit", "test", "all", "--progress", "--format=json"}, commandkit.Streams{Out: &output, Err: diagnostics}) + if len(kinds) != test.cancelAt || !strings.Contains(diagnostics.String(), "progress evidence") { + t.Fatalf("kinds=%v diagnostics=%q", kinds, diagnostics.String()) + } + if test.cancelAt < 4 { + if outcome.Kind != commandkit.OutcomeOperational || !errors.Is(outcome.Cause, context.Canceled) || (test.err != nil && (!errors.Is(outcome.Cause, test.err) || !strings.Contains(diagnostics.String(), "provider explanation"))) { + t.Fatalf("partial outcome=%#v diagnostics=%q", outcome, diagnostics.String()) + } + } else if outcome.Kind != commandkit.OutcomeSuccess || outcome.Cause != nil { + t.Fatalf("completed outcome=%#v", outcome) + } + var report struct { + Tests []struct { + Kind string `json:"kind"` + Report *struct { + Completed bool `json:"completed"` + } `json:"report"` + } `json:"tests"` + } + if err := json.Unmarshal(output.Bytes(), &report); err != nil { + t.Fatal(err) + } + if len(report.Tests) != len(kinds) { + t.Fatalf("report=%s", output.String()) + } + for i, entry := range report.Tests { + if entry.Kind != string(kinds[i]) || entry.Report == nil || !entry.Report.Completed { + t.Fatalf("report entry=%#v", entry) + } + } + }) + } +} + func TestSmokeAggregateCancellationRetainsPartialResults(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) diff --git a/integrations/ckprompt/smoke_command.go b/integrations/ckprompt/smoke_command.go index d55886e..925ee41 100644 --- a/integrations/ckprompt/smoke_command.go +++ b/integrations/ckprompt/smoke_command.go @@ -173,6 +173,9 @@ func runSmokeAggregate[I, S any](invocation commandkit.Invocation, suite Suite[I } } if err := promptKitContextError(invocation.Context); err != nil { + if len(results) < len(kinds) && !errors.Is(errors.Join(operationErrors...), err) { + operationErrors = append(operationErrors, err) + } break } }