From de2266af75a4b9697390fe33e8920165afeb61c6 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 26 Oct 2026 14:32:03 +0000 Subject: [PATCH] Restore cancellation checks before report delivery --- docs/roadmap/remediation.md | 12 +++- integrations/ckconfig/diff.go | 14 +--- integrations/ckconfig/explain.go | 11 +-- integrations/ckconfig/operations.go | 24 +++++++ .../ckconfig/report_cancellation_test.go | 69 +++++++++++++++++++ integrations/ckconfig/sources.go | 11 +-- integrations/ckprompt/command.go | 53 +++++++------- .../ckprompt/report_cancellation_test.go | 50 ++++++++++++++ integrations/ckprompt/validation_command.go | 6 +- 9 files changed, 193 insertions(+), 57 deletions(-) create mode 100644 integrations/ckconfig/report_cancellation_test.go create mode 100644 integrations/ckprompt/report_cancellation_test.go diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index c6ae737..13e371c 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Invocation reporting audit remediation -Status: remediation in progress; 3 of 7 findings complete. +Status: remediation in progress; 4 of 7 findings complete. ## Review scope @@ -248,7 +248,15 @@ public exact-wording guarantee. ## 4. Restore cancellation checks between report preparation and delivery -Priority: P2. Status: open. +Priority: P2. Status: complete. + +- [x] Completed: restored preparation-before-delivery cancellation checks for + PromptKit catalogs, definitions, requirements, and validation, and buffered + configuration diff/sources/YAML explanation reports. Cancellation retains its + operation cause without becoming a rendering failure; native streaming and + smoke-report behavior remain unchanged. Controlled preparation/serialization + checks cover ordinary and quiet modes. Validation: module suites pass in + `integrations/ckconfig` and `integrations/ckprompt`. ### Finding and evidence diff --git a/integrations/ckconfig/diff.go b/integrations/ckconfig/diff.go index 7168919..b328bf0 100644 --- a/integrations/ckconfig/diff.go +++ b/integrations/ckconfig/diff.go @@ -3,7 +3,6 @@ package ckconfig import ( "errors" "fmt" - "io" commandkit "gitea.maximumdirect.net/eric/CommandKit" ) @@ -31,16 +30,7 @@ func runConfigDiff[I, S any](invocation commandkit.Invocation, plan *configOpera if err := invocation.Context.Err(); err != nil { return configFailure("Configuration comparison was canceled.", acquired.Explanation, err) } - err = reporter.RenderOutput(func(writer io.Writer) error { - report, err := renderDiffReport(acquired.Value) - if err != nil { - return err - } - _, err = writer.Write(report) - return err + return renderPreparedConfigReport(invocation, acquired.Explanation, func() ([]byte, error) { + return renderDiffReport(acquired.Value) }) - if err != nil && invocation.Context.Err() != nil { - return configFailure("Configuration comparison was canceled.", acquired.Explanation, invocation.Context.Err()) - } - return commandkit.Success() } diff --git a/integrations/ckconfig/explain.go b/integrations/ckconfig/explain.go index db70000..2060914 100644 --- a/integrations/ckconfig/explain.go +++ b/integrations/ckconfig/explain.go @@ -94,18 +94,13 @@ func runConfigExplanation[I, S any](invocation commandkit.Invocation, plan *conf } return commandkit.Success() } - err := reporter.RenderOutput(func(writer io.Writer) error { + return renderPreparedConfigReport(invocation, acquired.Explanation, func() ([]byte, error) { var report bytes.Buffer if err := encodeYAML(&report, makeExplanationReport(name, explanation)); err != nil { - return err + return nil, err } - _, err := writer.Write(report.Bytes()) - return err + return report.Bytes(), nil }) - if err != nil && invocation.Context.Err() != nil { - return configFailure("Configuration explanation was canceled.", acquired.Explanation, invocation.Context.Err()) - } - return commandkit.Success() } func makeExplanationReport(scope string, explanation configkit.Explanation) explanationReport { diff --git a/integrations/ckconfig/operations.go b/integrations/ckconfig/operations.go index dcdaee9..a209266 100644 --- a/integrations/ckconfig/operations.go +++ b/integrations/ckconfig/operations.go @@ -2,6 +2,7 @@ package ckconfig import ( "fmt" + "io" commandkit "gitea.maximumdirect.net/eric/CommandKit" ) @@ -74,3 +75,26 @@ func configAcquisitionFailure( message := fmt.Sprintf("Unable to %s configuration scope %q.", operation, scope) return configFailure(message, acquired.Explanation, cause) } + +// Preserve the preparation/delivery cancellation checkpoint for buffered reports. +func renderPreparedConfigReport(invocation commandkit.Invocation, explanation commandkit.Explanation, prepare func() ([]byte, error)) commandkit.Outcome { + var canceled error + renderErr := invocation.Reporter().RenderOutput(func(writer io.Writer) error { + report, err := prepare() + if err != nil { + return err + } + if canceled = invocation.Context.Err(); canceled != nil { + return nil + } + _, err = writer.Write(report) + return err + }) + if canceled == nil && renderErr != nil { + canceled = invocation.Context.Err() + } + if canceled != nil { + return configFailure("Configuration report preparation was canceled.", explanation, canceled) + } + return commandkit.Success() +} diff --git a/integrations/ckconfig/report_cancellation_test.go b/integrations/ckconfig/report_cancellation_test.go new file mode 100644 index 0000000..5354175 --- /dev/null +++ b/integrations/ckconfig/report_cancellation_test.go @@ -0,0 +1,69 @@ +package ckconfig + +import ( + "context" + "errors" + "io" + "testing" + + commandkit "gitea.maximumdirect.net/eric/CommandKit" +) + +func TestBufferedReportCancellationBeforeDelivery(t *testing.T) { + for _, quiet := range []bool{false, true} { + t.Run(map[bool]string{false: "ordinary", true: "quiet"}[quiet], func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + prepared := false + app := newConfigTestApp(t, commandkit.NewCommand(commandkit.CommandSpec[struct{}]{ + Name: "report", + Run: func(invocation commandkit.Invocation, _ struct{}) commandkit.Outcome { + return renderPreparedConfigReport(invocation, commandkit.Explanation{Message: "selected context"}, func() ([]byte, error) { + prepared = true + cancel() + return []byte("prepared report"), nil + }) + }, + })) + args := []string{"report"} + if quiet { + args = append(args, "--quiet") + } + writes := 0 + got := app.Execute(ctx, args, commandkit.Streams{Out: reportWriteFunc(func(p []byte) (int, error) { + writes++ + return len(p), nil + })}) + if !prepared || writes != 0 || got.Status != 1 || !errors.Is(got.Cause, context.Canceled) || got.Explanation.Message != "selected context" { + t.Fatalf("prepared=%t writes=%d outcome=%#v", prepared, writes, got) + } + if got.Presentation.Output.RenderError != nil || got.Presentation.Output.WriteError != nil { + t.Fatalf("cancellation became a presentation failure: %#v", got.Presentation.Output) + } + }) + } +} + +type reportWriteFunc func([]byte) (int, error) + +func (write reportWriteFunc) Write(p []byte) (int, error) { return write(p) } + +func TestBufferedReportDoesNotRejectCancellationAfterDeliveryStarts(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + app := newConfigTestApp(t, commandkit.NewCommand(commandkit.CommandSpec[struct{}]{ + Name: "report", + Run: func(invocation commandkit.Invocation, _ struct{}) commandkit.Outcome { + return renderPreparedConfigReport(invocation, commandkit.Explanation{}, func() ([]byte, error) { + return []byte("report"), nil + }) + }, + })) + got := app.Execute(ctx, []string{"report"}, commandkit.Streams{Out: reportWriteFunc(func(p []byte) (int, error) { + cancel() + return io.Discard.Write(p) + })}) + if got.Status != 0 || got.Cause != nil { + t.Fatalf("post-delivery cancellation changed the operation outcome: %#v", got) + } +} diff --git a/integrations/ckconfig/sources.go b/integrations/ckconfig/sources.go index ef1b4e6..dfe3bb0 100644 --- a/integrations/ckconfig/sources.go +++ b/integrations/ckconfig/sources.go @@ -117,22 +117,17 @@ func runConfigSources[I, S any](invocation commandkit.Invocation, plan *configOp if err := invocation.Context.Err(); err != nil { return configFailure("Configuration source listing was canceled.", acquired.Explanation, err) } - err := reporter.RenderOutput(func(writer io.Writer) error { + return renderPreparedConfigReport(invocation, acquired.Explanation, func() ([]byte, error) { var report bytes.Buffer if plan.format == "yaml" { if err := encodeYAML(&report, document); err != nil { - return err + return nil, err } } else { writeSourceReportText(&report, document) } - _, err := writer.Write(report.Bytes()) - return err + return report.Bytes(), nil }) - if err != nil && invocation.Context.Err() != nil { - return configFailure("Configuration source listing was canceled.", acquired.Explanation, invocation.Context.Err()) - } - return commandkit.Success() } func writeSourceReportText(writer *bytes.Buffer, document sourceReport) { diff --git a/integrations/ckprompt/command.go b/integrations/ckprompt/command.go index 246367b..e7b9143 100644 --- a/integrations/ckprompt/command.go +++ b/integrations/ckprompt/command.go @@ -414,15 +414,9 @@ func runCatalogList[T any, I, S any]( return definitionFailure("PromptKit catalog listing was canceled.", acquired.Explanation, err) } } - _ = invocation.Reporter().RenderOutput(func(writer io.Writer) error { - output, err := encode(acquired.Value) - if err != nil { - return err - } - _, err = writer.Write(output) - return err + return renderPreparedPromptReport(invocation, acquired.Explanation, func() ([]byte, error) { + return encode(acquired.Value) }) - return commandkit.Success() } func runDefinitionInspection[I, S any](invocation commandkit.Invocation, suite Suite[I, S], input *definitionInput[I], prompt bool) commandkit.Outcome { @@ -456,7 +450,7 @@ func runDefinitionInspection[I, S any](invocation commandkit.Invocation, suite S return definitionFailure(fmt.Sprintf("Inspection of PromptKit prompt %q was canceled.", input.id), acquired.Explanation, err) } } - return renderPromptReport(invocation, acquired.Value, input.format) + return renderPromptReport(invocation, acquired.Value, input.format, acquired.Explanation) } request := ProfileRequest{Selection: input.selection, ID: input.id} @@ -473,7 +467,7 @@ func runDefinitionInspection[I, S any](invocation commandkit.Invocation, suite S return definitionFailure(fmt.Sprintf("Inspection of PromptKit profile %q was canceled.", input.id), acquired.Explanation, err) } } - return renderProfileReport(invocation, acquired.Value, input.format) + return renderProfileReport(invocation, acquired.Value, input.format, acquired.Explanation) } type promptInputReport struct { @@ -659,37 +653,50 @@ func runRequirementsInspection[I, S any](invocation commandkit.Invocation, suite return definitionFailure(fmt.Sprintf("Requirements inspection for PromptKit prompt %q was canceled.", input.inspection.PromptID), acquired.Explanation, err) } } - _ = invocation.Reporter().RenderOutput(func(writer io.Writer) error { - return encodePromptYAML(writer, requirementsReportFromInspection(acquired.Value)) + return renderPreparedPromptReport(invocation, acquired.Explanation, func() ([]byte, error) { + return encodePromptYAMLBytes(requirementsReportFromInspection(acquired.Value)) }) - return commandkit.Success() } -func renderPromptReport(invocation commandkit.Invocation, inspection *promptkit.PromptInspection, format string) commandkit.Outcome { - _ = invocation.Reporter().RenderOutput(func(writer io.Writer) error { +func renderPromptReport(invocation commandkit.Invocation, inspection *promptkit.PromptInspection, format string, explanation commandkit.Explanation) commandkit.Outcome { + return renderPreparedPromptReport(invocation, explanation, func() ([]byte, error) { report := promptReportFromInspection(inspection) if format == "yaml" { - return encodePromptYAML(writer, report) + return encodePromptYAMLBytes(report) } - _, err := io.WriteString(writer, formatPromptText(report)) - return err + return []byte(formatPromptText(report)), nil }) - return commandkit.Success() } -func renderProfileReport(invocation commandkit.Invocation, inspection *promptkit.ProfileInspection, format string) commandkit.Outcome { - _ = invocation.Reporter().RenderOutput(func(writer io.Writer) error { +func renderProfileReport(invocation commandkit.Invocation, inspection *promptkit.ProfileInspection, format string, explanation commandkit.Explanation) commandkit.Outcome { + return renderPreparedPromptReport(invocation, explanation, func() ([]byte, error) { report := profileReportFromInspection(inspection) if format == "yaml" { - return encodePromptYAML(writer, report) + return encodePromptYAMLBytes(report) } text, err := formatProfileText(report) + return []byte(text), err + }) +} + +// These reports were already prepared before delivery. Keep that boundary +// inside managed rendering, with cancellation owned by the command outcome. +func renderPreparedPromptReport(invocation commandkit.Invocation, explanation commandkit.Explanation, prepare func() ([]byte, error)) commandkit.Outcome { + var canceled error + _ = invocation.Reporter().RenderOutput(func(writer io.Writer) error { + output, err := prepare() if err != nil { return err } - _, err = io.WriteString(writer, text) + if canceled = promptKitContextError(invocation.Context); canceled != nil { + return nil + } + _, err = writer.Write(output) return err }) + if canceled != nil { + return definitionFailure("PromptKit report preparation was canceled.", explanation, canceled) + } return commandkit.Success() } diff --git a/integrations/ckprompt/report_cancellation_test.go b/integrations/ckprompt/report_cancellation_test.go new file mode 100644 index 0000000..2a27bdd --- /dev/null +++ b/integrations/ckprompt/report_cancellation_test.go @@ -0,0 +1,50 @@ +package ckprompt + +import ( + "context" + "errors" + "testing" + + commandkit "gitea.maximumdirect.net/eric/CommandKit" + promptkit "gitea.maximumdirect.net/eric/promptkit" +) + +type cancelDuringYAML struct{ cancel context.CancelFunc } + +func (value cancelDuringYAML) MarshalYAML() (any, error) { + value.cancel() + return "prepared", nil +} + +func TestInspectionCancellationDuringPreparationPreventsDelivery(t *testing.T) { + for _, format := range []string{"text", "yaml"} { + for _, mode := range []string{"ordinary", "quiet"} { + t.Run(format+"/"+mode, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + app := newPromptTestApp(t, Command(Suite[struct{}, struct{}]{ + InspectProfile: func(context.Context, ProfileRequest, struct{}, struct{}) (commandkit.Acquired[*promptkit.ProfileInspection], error) { + return commandkit.Acquired[*promptkit.ProfileInspection]{Value: &promptkit.ProfileInspection{ + ProfileID: "demo", + EffectiveModelParams: promptkit.ExecutionTarget{ExtraParams: map[string]any{ + "controlled": cancelDuringYAML{cancel}, + }}, + }}, nil + }, + })) + args := []string{"promptkit", "show", "profile", "demo", "--format", format} + if mode == "quiet" { + args = append(args, "--quiet") + } + output := &countingWriter{} + got := app.Execute(ctx, args, commandkit.Streams{Out: output}) + if got.Kind != commandkit.OutcomeOperational || got.Status != 1 || !errors.Is(got.Cause, context.Canceled) || output.calls != 0 { + t.Fatalf("outcome=%#v writes=%d", got, output.calls) + } + if got.Presentation.Output.RenderError != nil || got.Presentation.Output.WriteError != nil { + t.Fatalf("operation cancellation became a presentation failure: %#v", got.Presentation.Output) + } + }) + } + } +} diff --git a/integrations/ckprompt/validation_command.go b/integrations/ckprompt/validation_command.go index 1708a90..a605b58 100644 --- a/integrations/ckprompt/validation_command.go +++ b/integrations/ckprompt/validation_command.go @@ -3,7 +3,6 @@ package ckprompt import ( "errors" "fmt" - "io" commandkit "gitea.maximumdirect.net/eric/CommandKit" promptkit "gitea.maximumdirect.net/eric/promptkit" @@ -122,8 +121,7 @@ func runRequestValidation[I, S any](invocation commandkit.Invocation, suite Suit if err := promptKitContextError(invocation.Context); err != nil { return definitionFailure(fmt.Sprintf("Validation of the PromptKit request for prompt %q was canceled.", input.promptID), acquired.Explanation, err) } - _ = invocation.Reporter().RenderOutput(func(writer io.Writer) error { - return encodePromptYAML(writer, requestValidationReport{PromptID: input.promptID, Valid: true}) + return renderPreparedPromptReport(invocation, acquired.Explanation, func() ([]byte, error) { + return encodePromptYAMLBytes(requestValidationReport{PromptID: input.promptID, Valid: true}) }) - return commandkit.Success() }