diff --git a/config.go b/config.go index 302df8e..23998a8 100644 --- a/config.go +++ b/config.go @@ -74,6 +74,7 @@ type ConfigScope[C any] struct { } // Selector binds one typed command flag to selected configuration scopes. +// A required binding is required only when a selected scope uses it. type Selector[C any] struct { Binding Flag[C] Scopes []string @@ -101,6 +102,8 @@ type configScopesInput struct { } type selectorState[C any] struct { + name string + required bool scopes []string supplied bool apply func(*C) diff --git a/config_test.go b/config_test.go index 8d44e5b..c6b5005 100644 --- a/config_test.go +++ b/config_test.go @@ -217,3 +217,41 @@ func TestConfigPreflightRejectsUnsupportedAggregateAndInapplicableSelector(t *te func noopConfigAcquire[C any](context.Context, ConfigRequest, C) (Acquired[ConfigView], error) { return Acquired[ConfigView]{}, nil } + +func TestRequiredConfigSelectorOnlyAppliesToSelectedScopes(t *testing.T) { + app, _ := New(Identity{Name: "example"}) + calls := 0 + acquire := func(context.Context, ConfigRequest, string) (Acquired[ConfigView], error) { + calls++ + return Acquired[ConfigView]{}, nil + } + err := RegisterConfig(app, ConfigSuite[string]{ + Selectors: []Selector[string]{{Scopes: []string{"app"}, Binding: ValueFlag[string](FlagMeta{Name: "campaign", Required: true}, nil, parseString, func(c *string, v Supplied[string]) { *c = v.Value })}}, + Scopes: []ConfigScope[string]{{Name: "app", Operations: []ConfigOperation{ConfigValidate}, Acquire: acquire}, {Name: "promptkit", Operations: []ConfigOperation{ConfigValidate}, Acquire: acquire}}, + }) + if err != nil { + t.Fatal(err) + } + for _, tc := range []struct { + args []string + kind OutcomeKind + calls int + }{ + {[]string{"--scope", "promptkit"}, OutcomeSuccess, 1}, + {nil, OutcomeUsage, 0}, + {[]string{"--scope", "all"}, OutcomeUsage, 0}, + {[]string{"--scope", "all", "--campaign", "spring"}, OutcomeSuccess, 2}, + {[]string{"--scope", "promptkit", "--campaign", "spring"}, OutcomeUsage, 0}, + {[]string{"--help"}, OutcomeSuccess, 0}, + } { + calls = 0 + var out bytes.Buffer + result := app.Execute(context.Background(), append([]string{"config", "validate"}, tc.args...), Streams{Out: &out}) + if result.Kind != tc.kind || calls != tc.calls { + t.Fatalf("%v: result=%#v calls=%d", tc.args, result, calls) + } + if len(tc.args) > 0 && tc.args[0] == "--help" && !strings.Contains(out.String(), "required where applicable") { + t.Fatalf("help=%q", out.String()) + } + } +} diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 144a981..974ce93 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Initial implementation review and remediation -Status: findings recorded; implementation fixes pending. +Status: remediation in progress. Date: 2026-10-07. @@ -17,7 +17,13 @@ review; prior completion notes are historical evidence, not a new validation result. The triggering cases below follow from the current code paths. Focused regression checks are proposed completion evidence for subsequent fixes. -This document records remediation work only. Implementation remains unchanged. +The findings below describe the review baseline. Completion is tracked here as +implementation fixes land. + +## Remediation progress + +- R-01 complete: required selectors are checked after scope selection; unrelated + scopes remain usable, and help describes conditional requiredness. ## Architecture assessment diff --git a/selectors.go b/selectors.go index 3611971..b564986 100644 --- a/selectors.go +++ b/selectors.go @@ -27,10 +27,15 @@ func configInputFlags[C any](suite ConfigSuite[C], formatDefault string) []Flag[ } func adaptSelector[C any](selector Selector[C]) Flag[configInput[C]] { - return adaptContextSelector(selector, + flag := adaptContextSelector(selector, func(input *configInput[C]) *C { return &input.SelectorContext }, func(input *configInput[C], state selectorState[C]) { input.selectors = append(input.selectors, state) }, ) + if flag.binding.meta.Required { + flag.binding.meta.Required = false // Scope-aware preflight enforces this requirement. + flag.binding.meta.Usage += " (required where applicable)" + } + return flag } func adaptContextSelector[C any, I any](selector Selector[C], selectContext func(*I) *C, record func(*I, selectorState[C])) Flag[I] { @@ -53,6 +58,8 @@ func adaptContextSelector[C any, I any](selector Selector[C], selectContext func return nil, err } state := selectorState[C]{ + name: binding.meta.Name, + required: binding.meta.Required, scopes: append([]string(nil), selector.Scopes...), supplied: len(values) > 0, apply: apply, @@ -128,9 +135,6 @@ func selectConfigScopes[C any](suite ConfigSuite[C], input configInput[C], opera } } for _, selector := range input.selectors { - if !selector.supplied { - continue - } applies := false for _, scopeName := range scopes { if selectorApplies(selector.scopes, scopeName) { @@ -138,7 +142,10 @@ func selectConfigScopes[C any](suite ConfigSuite[C], input configInput[C], opera break } } - if !applies { + if applies && selector.required && !selector.supplied { + return nil, fmt.Errorf("required flag --%s was not supplied", selector.name) + } + if selector.supplied && !applies { return nil, fmt.Errorf("selector is not applicable to configuration scope %q", input.Scope) } }