From d2434b95801ed95ba9fd80b3e279593efa4282a3 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 11 Oct 2026 21:43:30 +0000 Subject: [PATCH] Prepare both configuration comparison inputs --- docs/roadmap/implementation.md | 28 +- .../ckconfig/comparison_input_test.go | 6 +- .../ckconfig/comparison_preparation_test.go | 345 ++++++++++++++++++ integrations/ckconfig/input.go | 68 +++- 4 files changed, 414 insertions(+), 33 deletions(-) create mode 100644 integrations/ckconfig/comparison_preparation_test.go diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 65eac6d..f02ee9d 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -169,12 +169,11 @@ remain supplied. The `diff` command defaults to YAML, accepts explicit YAML, and rejects text and unsupported formats. Its help exposes both right-side flags; validate, show, sources, and explain do not, and `--path` remains explain-only. Comparison scope preflight defaults to `app` and rejects `all`, unknown, and -unsupported scopes before suite factories run. Valid comparison preparation -stores the resolved right selection without constructing application input or -shared state. The existing show default and other operation defaults remain -unchanged. Focused tests and the full ckconfig and dependent ckp suites pass. -Separate typed pair preparation remains in Stage 4; acquisition and delivery -remain in Stage 8. +unsupported scopes before suite factories run. Preflight resolves right-side +fields before pair preparation. The existing show default and other operation +defaults remain unchanged. Focused tests and the full ckconfig and dependent ckp +suites passed at this point in the sequence. Separate typed pair preparation is +completed in Stage 4; acquisition and delivery remain in Stage 8. ## Stage 4: Prepare separate typed inputs for the selected pair @@ -205,11 +204,18 @@ shared state remains uninitialized until execution. values are usage errors; `ErrFlagWiring` remains operational. Both stop before state creation and provider calls. Side wording is not a new exact-text contract. -**Completion evidence:** equal selections still prepare both reached inputs; -applicable required selectors, explicit false/zero, parser conversion once, and -repeated-value independence. Verify preflight/check failures stop state/provider -work. Help converts supplied values without invoking suite factories/providers. -Do not assert calls that the accepted early-failure contract allows to be skipped. +**Completion evidence — complete:** comparison plans now hold a dedicated +prepared pair with its selected scope and two `ComparisonSide` values; ordinary +scope inputs remain separate, and the pair is stored only after both inputs pass +checks. The shared helper initializes each reached input, applies the captured +scope bindings, and checks it with that side's selection. Tests cover equal +selections preparing twice, explicit false/zero and required selector handling, +one conversion per supplied occurrence, independent repeated slices, nil factory +zero values, preserved caller aliases, wrapped right-side wiring classification +and context, and absence of a partial plan after a side failure. Shared state is +created only after both checks pass. Help converts even an inapplicable supplied +selector while leaving input/check/state factories untouched. Focused comparison +tests and the full ckconfig and dependent ckp suites pass. ## Stage 5: Implement typed native comparison and diagnostic retention diff --git a/integrations/ckconfig/comparison_input_test.go b/integrations/ckconfig/comparison_input_test.go index b89328b..13a533b 100644 --- a/integrations/ckconfig/comparison_input_test.go +++ b/integrations/ckconfig/comparison_input_test.go @@ -107,7 +107,7 @@ func TestComparisonHelpLimitsFlagsAndDefaultsFormatToYAML(t *testing.T) { } } -func TestComparisonPreparationAcceptsOnlyYAMLWithoutCreatingInputs(t *testing.T) { +func TestComparisonPreparationAcceptsOnlyYAML(t *testing.T) { inputCalls, stateCalls := 0, 0 prepared := captureSuite(Suite[struct{}, struct{}]{ NewInput: func() struct{} { inputCalls++; return struct{}{} }, @@ -147,8 +147,8 @@ func TestComparisonPreparationAcceptsOnlyYAMLWithoutCreatingInputs(t *testing.T) t.Errorf("comparison format %q was accepted", format) } } - if inputCalls != 0 || stateCalls != 0 { - t.Fatalf("comparison preparation created input/state: %d/%d", inputCalls, stateCalls) + if inputCalls != 2 || stateCalls != 0 { + t.Fatalf("comparison preparation calls input/state: %d/%d", inputCalls, stateCalls) } } diff --git a/integrations/ckconfig/comparison_preparation_test.go b/integrations/ckconfig/comparison_preparation_test.go new file mode 100644 index 0000000..b03fed9 --- /dev/null +++ b/integrations/ckconfig/comparison_preparation_test.go @@ -0,0 +1,345 @@ +package ckconfig + +import ( + "context" + "errors" + "io" + "strconv" + "strings" + "testing" + + commandkit "gitea.maximumdirect.net/eric/CommandKit" + configkit "gitea.maximumdirect.net/eric/ConfigKit" +) + +func TestComparisonPreparesEqualSelectionsWithSeparateTypedInputs(t *testing.T) { + type input struct { + Enabled commandkit.Supplied[bool] + Limit commandkit.Supplied[int] + Tags commandkit.Supplied[[]string] + } + var inputCalls, checkCalls, stateCalls int + var parseCalls int + var preparedInputs []*input + var selections []Selection + enabled := commandkit.BoolFlag[*input](commandkit.FlagMeta{Name: "enabled"}, true, func(value **input, supplied commandkit.Supplied[bool]) { + (*value).Enabled = supplied + }) + limit := commandkit.ValueFlag[*input](commandkit.FlagMeta{Name: "limit", Required: true}, (*int)(nil), func(raw string) (int, error) { + parseCalls++ + return strconv.Atoi(raw) + }, func(value **input, supplied commandkit.Supplied[int]) { + (*value).Limit = supplied + }) + tags := commandkit.RepeatFlag[*input](commandkit.FlagMeta{Name: "tag"}, nil, func(raw string) (string, error) { + parseCalls++ + return raw, nil + }, func(value **input, supplied commandkit.Supplied[[]string]) { + (*value).Tags = supplied + }) + suite := Suite[*input, struct{}]{ + NewInput: func() *input { + inputCalls++ + return &input{} + }, + NewState: func() struct{} { + stateCalls++ + if checkCalls != 2 { + t.Errorf("shared state created after %d input checks", checkCalls) + } + if len(preparedInputs) != 2 || preparedInputs[0] == preparedInputs[1] { + t.Errorf("inputs were not initialized separately: %#v", preparedInputs) + } else { + left, right := preparedInputs[0].Tags.Value, preparedInputs[1].Tags.Value + if len(left) != 2 || len(right) != 2 || &left[0] == &right[0] { + t.Errorf("repeated values share storage: left=%#v right=%#v", left, right) + } else { + left[0] = "changed" + if right[0] != "alpha" { + t.Errorf("mutating one side changed the other: %#v", right) + } + } + } + return struct{}{} + }, + Selectors: []Selector[*input]{{Binding: enabled}, {Binding: limit}, {Binding: tags}}, + CheckInput: func(scope string, selection Selection, value *input) error { + checkCalls++ + preparedInputs = append(preparedInputs, value) + selections = append(selections, selection) + if scope != "app" || value == nil { + return errors.New("unexpected scope input") + } + if !value.Enabled.Set || value.Enabled.Value { + return errors.New("explicit false was not preserved") + } + if !value.Limit.Set || value.Limit.Value != 0 { + return errors.New("explicit zero was not preserved") + } + if !value.Tags.Set || len(value.Tags.Value) != 2 || value.Tags.Value[0] != "alpha" || value.Tags.Value[1] != "beta" { + return errors.New("repeated input was not preserved") + } + return nil + }, + Scopes: []Scope[*input, struct{}]{{ + Name: "app", Operations: []Operation{Diff}, + Compare: func(context.Context, ComparisonRequest[*input], struct{}) (commandkit.Acquired[[]configkit.Difference], error) { + return commandkit.Acquired[[]configkit.Difference]{}, nil + }, + }}, + } + app := newConfigTestApp(t, Command(suite)) + outcome := app.Execute(context.Background(), []string{ + "config", "diff", "--config", "same.yml", "--against-config", "same.yml", "--overlay=", + "--enabled=false", "--limit=0", "--tag", "alpha", "--tag", "beta", + }, commandkit.Streams{Out: io.Discard, Err: io.Discard}) + if outcome.Kind == commandkit.OutcomeUsage { + t.Fatalf("valid comparison inputs rejected: %#v", outcome) + } + if inputCalls != 2 || checkCalls != 2 || stateCalls != 1 { + t.Fatalf("preparation calls input=%d check=%d state=%d", inputCalls, checkCalls, stateCalls) + } + if parseCalls != 3 { + t.Fatalf("typed values were converted %d times, want once per supplied occurrence", parseCalls) + } + wantSelection := Selection{ + Config: commandkit.Supplied[string]{Value: "same.yml", Set: true}, + Overlay: commandkit.Supplied[string]{Set: true}, + } + if len(selections) != 2 || selections[0] != wantSelection || selections[1] != wantSelection { + t.Fatalf("side selections = %#v, want equal %#v values", selections, wantSelection) + } +} + +func TestComparisonPlanStoresPairSeparatelyFromOrdinaryInputs(t *testing.T) { + type input struct{ ID int } + inputCalls, checkCalls, stateCalls := 0, 0, 0 + prepared := captureSuite(Suite[input, struct{}]{ + NewInput: func() input { + inputCalls++ + return input{ID: inputCalls} + }, + NewState: func() struct{} { stateCalls++; return struct{}{} }, + CheckInput: func(scope string, selection Selection, value input) error { + checkCalls++ + if scope != "app" || value.ID != checkCalls { + return errors.New("comparison inputs were not prepared in order") + } + return nil + }, + Scopes: []Scope[input, struct{}]{{ + Name: "app", Operations: []Operation{Diff}, + Compare: func(context.Context, ComparisonRequest[input], struct{}) (commandkit.Acquired[[]configkit.Difference], error) { + return commandkit.Acquired[[]configkit.Difference]{}, nil + }, + }}, + }) + left := Selection{Config: commandkit.Supplied[string]{Value: "before.yml", Set: true}} + right := Selection{Overlay: commandkit.Supplied[string]{Set: true}} + commandInput := &configCommandInput[input, struct{}]{ + selection: left, + against: right, + format: commandkit.Supplied[string]{Value: "yaml"}, + } + if err := prepareConfigOperation(prepared, Diff, commandInput); err != nil { + t.Fatal(err) + } + plan := commandInput.plan + if plan == nil || plan.comparison == nil || len(plan.scopes) != 0 { + t.Fatalf("comparison was not stored separately: %#v", plan) + } + if plan.comparison.scope.scope.Name != "app" { + t.Fatalf("comparison scope = %q", plan.comparison.scope.scope.Name) + } + wantRight := Selection{Config: left.Config, Overlay: right.Overlay} + request := plan.comparison.request + if request.Left.Selection != left || request.Right.Selection != wantRight || request.Left.Input.ID != 1 || request.Right.Input.ID != 2 { + t.Fatalf("prepared request = %#v", request) + } + if inputCalls != 2 || checkCalls != 2 || stateCalls != 0 { + t.Fatalf("preparation calls input=%d check=%d state=%d", inputCalls, checkCalls, stateCalls) + } +} + +func TestComparisonDoesNotRetainPartialPlanAfterSideFailure(t *testing.T) { + checkCalls := 0 + failure := errors.New("right input rejected") + prepared := captureSuite(Suite[struct{}, struct{}]{ + CheckInput: func(string, Selection, struct{}) error { + checkCalls++ + if checkCalls == 2 { + return failure + } + return nil + }, + Scopes: []Scope[struct{}, struct{}]{{ + Name: "app", Operations: []Operation{Diff}, + Compare: func(context.Context, ComparisonRequest[struct{}], struct{}) (commandkit.Acquired[[]configkit.Difference], error) { + return commandkit.Acquired[[]configkit.Difference]{}, nil + }, + }}, + }) + commandInput := &configCommandInput[struct{}, struct{}]{format: commandkit.Supplied[string]{Value: "yaml"}} + if err := prepareConfigOperation(prepared, Diff, commandInput); !errors.Is(err, failure) { + t.Fatalf("right-side input failure = %v", err) + } + if commandInput.plan != nil || checkCalls != 2 { + t.Fatalf("failed pair retained partial state: plan=%#v checks=%d", commandInput.plan, checkCalls) + } +} + +func TestComparisonNilInputFactoryUsesZeroValue(t *testing.T) { + checkCalls := 0 + prepared := captureSuite(Suite[*int, struct{}]{ + CheckInput: func(_ string, _ Selection, value *int) error { + checkCalls++ + if value != nil { + return errors.New("nil factory did not supply the zero value") + } + return nil + }, + Scopes: []Scope[*int, struct{}]{{ + Name: "app", Operations: []Operation{Diff}, + Compare: func(context.Context, ComparisonRequest[*int], struct{}) (commandkit.Acquired[[]configkit.Difference], error) { + return commandkit.Acquired[[]configkit.Difference]{}, nil + }, + }}, + }) + commandInput := &configCommandInput[*int, struct{}]{format: commandkit.Supplied[string]{Value: "yaml"}} + if err := prepareConfigOperation(prepared, Diff, commandInput); err != nil { + t.Fatal(err) + } + request := commandInput.plan.comparison.request + if checkCalls != 2 || request.Left.Input != nil || request.Right.Input != nil { + t.Fatalf("nil input value changed: checks=%d request=%#v", checkCalls, request) + } +} + +func TestComparisonPreservesCallerOwnedInputAliases(t *testing.T) { + shared := new(int) + inputCalls, checkCalls := 0, 0 + prepared := captureSuite(Suite[*int, struct{}]{ + NewInput: func() *int { inputCalls++; return shared }, + CheckInput: func(_ string, _ Selection, value *int) error { + checkCalls++ + if value != shared { + return errors.New("caller-owned alias was replaced") + } + return nil + }, + Scopes: []Scope[*int, struct{}]{{ + Name: "app", Operations: []Operation{Diff}, + Compare: func(context.Context, ComparisonRequest[*int], struct{}) (commandkit.Acquired[[]configkit.Difference], error) { + return commandkit.Acquired[[]configkit.Difference]{}, nil + }, + }}, + }) + commandInput := &configCommandInput[*int, struct{}]{format: commandkit.Supplied[string]{Value: "yaml"}} + if err := prepareConfigOperation(prepared, Diff, commandInput); err != nil { + t.Fatal(err) + } + request := commandInput.plan.comparison.request + if inputCalls != 2 || checkCalls != 2 || request.Left.Input != shared || request.Right.Input != shared { + t.Fatalf("caller-owned alias changed: calls=%d/%d request=%#v", inputCalls, checkCalls, request) + } +} + +func TestComparisonInputFailureStopsBeforeStateAndPreservesWiringClassification(t *testing.T) { + var inputCalls, checkCalls, stateCalls int + suite := Suite[struct{}, struct{}]{ + NewInput: func() struct{} { inputCalls++; return struct{}{} }, + NewState: func() struct{} { stateCalls++; return struct{}{} }, + CheckInput: func(string, Selection, struct{}) error { + checkCalls++ + if checkCalls == 2 { + return commandkit.ErrFlagWiring + } + return nil + }, + Scopes: []Scope[struct{}, struct{}]{{ + Name: "app", Operations: []Operation{Diff}, + Compare: func(context.Context, ComparisonRequest[struct{}], struct{}) (commandkit.Acquired[[]configkit.Difference], error) { + return commandkit.Acquired[[]configkit.Difference]{}, nil + }, + }}, + } + app := newConfigTestApp(t, Command(suite)) + outcome := app.Execute(context.Background(), []string{"config", "diff"}, commandkit.Streams{Out: io.Discard, Err: io.Discard}) + if outcome.Kind != commandkit.OutcomeOperational || !errors.Is(outcome.Cause, commandkit.ErrFlagWiring) { + t.Fatalf("right-side wiring failure = %#v", outcome) + } + if !strings.Contains(outcome.Cause.Error(), `scope "app" right input`) { + t.Fatalf("right-side error lacks scope context: %v", outcome.Cause) + } + if inputCalls != 2 || checkCalls != 2 || stateCalls != 0 { + t.Fatalf("failed preparation calls input=%d check=%d state=%d", inputCalls, checkCalls, stateCalls) + } +} + +func TestComparisonMissingRequiredSelectorIsUsageBeforeState(t *testing.T) { + type input struct{ Token commandkit.Supplied[string] } + var inputCalls, checkCalls, stateCalls int + token := commandkit.ValueFlag(commandkit.FlagMeta{Name: "token", Required: true}, (*string)(nil), parseConfigString, func(value *input, supplied commandkit.Supplied[string]) { + value.Token = supplied + }) + suite := Suite[input, struct{}]{ + NewInput: func() input { inputCalls++; return input{} }, + NewState: func() struct{} { stateCalls++; return struct{}{} }, + Selectors: []Selector[input]{{Binding: token, Scopes: []string{"app"}}}, + CheckInput: func(string, Selection, input) error { checkCalls++; return nil }, + Scopes: []Scope[input, struct{}]{{ + Name: "app", Operations: []Operation{Diff}, + Compare: func(context.Context, ComparisonRequest[input], struct{}) (commandkit.Acquired[[]configkit.Difference], error) { + return commandkit.Acquired[[]configkit.Difference]{}, nil + }, + }}, + } + app := newConfigTestApp(t, Command(suite)) + outcome := app.Execute(context.Background(), []string{"config", "diff"}, commandkit.Streams{Out: io.Discard, Err: io.Discard}) + if outcome.Kind != commandkit.OutcomeUsage || !errors.Is(outcome.Cause, commandkit.ErrFlagInput) { + t.Fatalf("missing required selector = %#v", outcome) + } + if inputCalls != 1 || checkCalls != 0 || stateCalls != 0 { + t.Fatalf("missing selector calls input=%d check=%d state=%d", inputCalls, checkCalls, stateCalls) + } +} + +func TestComparisonHelpConvertsInapplicableSelectorsWithoutBuildingInputs(t *testing.T) { + type input struct{ Tenant int } + parseErr := errors.New("invalid tenant") + var parseCalls, inputCalls, checkCalls, stateCalls int + tenant := commandkit.ValueFlag(commandkit.FlagMeta{Name: "tenant", Required: true}, (*int)(nil), func(raw string) (int, error) { + parseCalls++ + if raw == "bad" { + return 0, parseErr + } + return strconv.Atoi(raw) + }, func(value *input, supplied commandkit.Supplied[int]) { + value.Tenant = supplied.Value + }) + makeScope := func(name string) Scope[input, struct{}] { + return Scope[input, struct{}]{Name: name, Operations: []Operation{Diff}, Compare: func(context.Context, ComparisonRequest[input], struct{}) (commandkit.Acquired[[]configkit.Difference], error) { + return commandkit.Acquired[[]configkit.Difference]{}, nil + }} + } + suite := Suite[input, struct{}]{ + NewInput: func() input { inputCalls++; return input{} }, + NewState: func() struct{} { stateCalls++; return struct{}{} }, + Selectors: []Selector[input]{{Binding: tenant, Scopes: []string{"promptkit"}}}, + CheckInput: func(string, Selection, input) error { checkCalls++; return nil }, + Scopes: []Scope[input, struct{}]{makeScope("app"), makeScope("promptkit")}, + } + app := newConfigTestApp(t, Command(suite)) + var output strings.Builder + app.Execute(context.Background(), []string{"config", "diff", "--scope", "app", "--tenant=7", "--help"}, commandkit.Streams{Out: &output, Err: io.Discard}) + if !strings.Contains(output.String(), "--tenant") || parseCalls != 1 { + t.Fatalf("help did not convert the inapplicable supplied selector: calls=%d help=%q", parseCalls, output.String()) + } + outcome := app.Execute(context.Background(), []string{"config", "diff", "--scope", "app", "--tenant=bad", "--help"}, commandkit.Streams{Out: io.Discard, Err: io.Discard}) + if outcome.Kind != commandkit.OutcomeUsage || !errors.Is(outcome.Cause, parseErr) || parseCalls != 2 { + t.Fatalf("malformed help selector outcome=%#v conversions=%d", outcome, parseCalls) + } + if inputCalls != 0 || checkCalls != 0 || stateCalls != 0 { + t.Fatalf("help built suite work: input=%d check=%d state=%d", inputCalls, checkCalls, stateCalls) + } +} diff --git a/integrations/ckconfig/input.go b/integrations/ckconfig/input.go index fbc7bf9..bebf93a 100644 --- a/integrations/ckconfig/input.go +++ b/integrations/ckconfig/input.go @@ -23,14 +23,20 @@ type selectedScopeInput[I, S any] struct { input I } +type preparedComparison[I, S any] struct { + scope *preparedScope[I, S] + request ComparisonRequest[I] +} + type configOperationPlan[I, S any] struct { - prepared *preparedSuite[I, S] - operation Operation - selection Selection - scopeName string - format string - path configkit.Path - scopes []selectedScopeInput[I, S] + prepared *preparedSuite[I, S] + operation Operation + selection Selection + scopeName string + format string + path configkit.Path + scopes []selectedScopeInput[I, S] + comparison *preparedComparison[I, S] } func newConfigOperationCommand[I, S any](prepared *preparedSuite[I, S], operation Operation) commandkit.Command { @@ -171,30 +177,54 @@ func prepareConfigOperation[I, S any](prepared *preparedSuite[I, S], operation O } if operation == Diff { input.against = inheritComparisonSelection(input.selection, input.against) + scope := selected[0] + left, err := prepareScopeInput(prepared, input.values, scope, input.selection) + if err != nil { + return fmt.Errorf("configuration scope %q left input: %w", scope.scope.Name, err) + } + right, err := prepareScopeInput(prepared, input.values, scope, input.against) + if err != nil { + return fmt.Errorf("configuration scope %q right input: %w", scope.scope.Name, err) + } + plan.comparison = &preparedComparison[I, S]{ + scope: scope, + request: ComparisonRequest[I]{ + Left: ComparisonSide[I]{Selection: input.selection, Input: left}, + Right: ComparisonSide[I]{Selection: input.against, Input: right}, + }, + } input.plan = plan return nil } for _, scope := range selected { - var value I - if prepared.newInput != nil { - value = prepared.newInput() - } - // Availability was checked above, so every binding for this scope - // belongs to the operation's captured union. - if err := input.values.ApplyTo(&value, scope.bindings...); err != nil { + value, err := prepareScopeInput(prepared, input.values, scope, input.selection) + if err != nil { return fmt.Errorf("configuration scope %q: %w", scope.scope.Name, err) } - if prepared.checkInput != nil { - if err := prepared.checkInput(scope.scope.Name, input.selection, value); err != nil { - return fmt.Errorf("configuration scope %q: %w", scope.scope.Name, err) - } - } plan.scopes = append(plan.scopes, selectedScopeInput[I, S]{scope: scope, input: value}) } input.plan = plan return nil } +func prepareScopeInput[I, S any](prepared *preparedSuite[I, S], values commandkit.FlagValues[I], scope *preparedScope[I, S], selection Selection) (I, error) { + var value I + if prepared.newInput != nil { + value = prepared.newInput() + } + // Availability was checked before this helper, so every binding for the + // selected scope belongs to the operation's captured union. + if err := values.ApplyTo(&value, scope.bindings...); err != nil { + return value, err + } + if prepared.checkInput != nil { + if err := prepared.checkInput(scope.scope.Name, selection, value); err != nil { + return value, err + } + } + return value, nil +} + func inheritComparisonSelection(left, right Selection) Selection { if !right.Config.Set { right.Config = left.Config