Skip to content

Commit 1b67ec0

Browse files
author
David Noble
committed
feat(13.0(n) Phase 2): VariableResolver.Resolve cascade + envValue wrapper
Land the binding-layer's real source-precedence resolution. The skeleton Resolve (Phase 1) is replaced with the full cascade: Override → Flag → Env (envValue + op.Convert) → Config → Default Env values route through a new envValue wrapper that implements op.SourceConverter, picked up by op.Convert step 5. The cascade inside envValue.ConvertTo handles: string identity, time.Duration via time.ParseDuration, os.FileMode via strconv.ParseUint base 0, primitive Kind switch (bool/int*/uint*/float*), encoding.TextUnmarshaler probe, then json.Unmarshal as universal fallback. envValue.CanConvertTo returns false for op.Resource targets so Convert step 7 (registered Resource construction) wins routing for env strings naming a Resource. Crucially, op.Convert itself is NOT modified. The codebase's layering decision at pkg/op/parameter.go:236 stands: a plain Go `string` reaching a primitive-typed slot continues to error (the test case TestEnvValue_ThroughConvert/plain_string_still_errors_for_primitive guards this invariant). Only envValue-wrapped strings get the lenient parse — env-source semantics attach to the source identity at the type level, not to the target type. Cascade behaviors: - Resolve(env, parameters) aggregates errors; missing-required and type-mismatch errors fold into a single []error the caller (Phase 4 preflight) joins into the D5 envelope. - Get and Variables panic if called before Resolve (programmer error). - EnvPrefix is empty when app or app.Name is empty — the env step of the cascade is skipped in that case (parameter names alone are too generic to safely shadow process env). - camelCase parameter names work: env key is derived via strings.ToUpper(op.CamelToSnake(name)) (greedy-acronym rule per locked decision). Application.Flags normalization: - NewApplication stamps each user-supplied flag under its snake-case form (kebab→snake at construction; `--dry-run` lands under Flags["dry_run"]). - DryRun reads Flags["dry_run"] to match. - kebabToSnake helper added; idempotent on already-snake names. Codegen template (receiver_type.gen_test.go.template) flips the test-fixture Flags map literal from "dry-run" to "dry_run". The 17 provider gen test files are regenerated. Doc comments referring to the CLI flag form (`--dry-run`) remain unchanged — the kebab form is the user-visible flag name; only the internal Application.Flags map key changed. Consumer signature update: - cmd/devlore-test/devloretest/runner.go: resolver.Resolve(env, params) — Phase 1 wiring still passes nil parameters (Phase 4 wires the real preflight pass with graph.Parameters()). Build state: make build clean. make vet clean. All pkg/op/ and pkg/application/ unit tests green. Integration tests in cmd/devlore-test and cmd/star/star remain red pending Phase 5 (plan.run invocation materialization) — pre-existing; this commit does not change that posture. Issue: #283
1 parent e272abf commit 1b67ec0

26 files changed

Lines changed: 1321 additions & 129 deletions

‎cmd/devlore-test/devloretest/runner.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,9 @@ func (r *Runner) Start(ctx context.Context) (_ *Result, err error) {
319319
Config: r.sources.Config,
320320
Overrides: r.sources.Overrides,
321321
})
322-
resolveErrs := resolver.Resolve(nil)
322+
// graph is unbound at this point (op.Plan deferred Unbind); pass nil env. Phase 4 wires the real
323+
// preflight pass inside GraphExecutor.Run where env is live.
324+
resolveErrs := resolver.Resolve(nil, nil)
323325
if len(resolveErrs) > 0 {
324326
return nil, fmt.Errorf("variable resolution: %v", resolveErrs)
325327
}

‎pkg/application/application.go‎

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
package application
1515

1616
import (
17+
"strings"
18+
1719
"github.com/spf13/cobra"
1820
"github.com/spf13/pflag"
1921
)
@@ -32,8 +34,9 @@ type Application struct {
3234
Name string
3335

3436
// Flags carries values parsed from command-line arguments. Consumed by [op.VariableResolver] under
35-
// [op.VariableSourceKindFlag]. Keys are the cobra/pflag flag names (kebab-case as the user typed them);
36-
// values are the typed Go value pflag parsed.
37+
// [op.VariableSourceKindFlag]. Keys are snake-case (normalized from cobra/pflag's kebab-case at
38+
// [NewApplication] time so `--dry-run` lands under `Flags["dry_run"]`); values are the typed Go value
39+
// pflag parsed.
3740
//
3841
// Populated by [NewApplication] via [pflag.FlagSet.Visit] — only flags the user explicitly supplied are
3942
// present. A lookup for a flag the user did not pass returns the zero value via map-zero semantics.
@@ -49,20 +52,26 @@ type Application struct {
4952
}
5053

5154
// DryRun reports whether the user supplied `--dry-run` on the active command. Reads
52-
// [Application.Flags] under the canonical key "dry-run" (the cobra flag name verbatim). Returns false when
53-
// the flag was not supplied, when its value is not a bool, or when [Application.Flags] is nil.
55+
// [Application.Flags] under the canonical snake-case key "dry_run" (normalized from cobra's "dry-run"
56+
// at [NewApplication] time). Returns false when the flag was not supplied, when its value is not a
57+
// bool, or when [Application.Flags] is nil.
5458
//
5559
// Returns:
5660
// - `bool`: true when `--dry-run` was supplied; false otherwise.
5761
func (a *Application) DryRun() bool {
5862

59-
v, _ := a.Flags["dry-run"].(bool)
63+
v, _ := a.Flags["dry_run"].(bool)
6064
return v
6165
}
6266

6367
// NewApplication constructs an [Application] from a cobra command's parsed flag state. Walks the command's
6468
// merged pflag set (cobra merges persistent + local automatically when [cobra.Command.Flags] is called on
65-
// the leaf) and stamps each user-supplied flag into [Application.Flags]. Defaults are not stamped.
69+
// the leaf) and stamps each user-supplied flag into [Application.Flags] under its snake-case form (cobra's
70+
// kebab-case flag name with hyphens replaced by underscores). Defaults are not stamped.
71+
//
72+
// The kebab→snake normalization makes [Application.Flags] uniform with [op.VariableResolver]'s parameter
73+
// name conventions (snake_case throughout); the resolver's flag-source step matches the parameter name
74+
// verbatim against the snake-case keys.
6675
//
6776
// Config and Overrides are left nil. Tools that source either layer populate them via direct field
6877
// assignment after construction.
@@ -77,7 +86,7 @@ func NewApplication(name string, cmd *cobra.Command) *Application {
7786

7887
flags := make(map[string]any)
7988
cmd.Flags().Visit(func(f *pflag.Flag) {
80-
flags[f.Name] = flagValue(cmd, f)
89+
flags[kebabToSnake(f.Name)] = flagValue(cmd, f)
8190
})
8291

8392
return &Application{
@@ -86,6 +95,19 @@ func NewApplication(name string, cmd *cobra.Command) *Application {
8695
}
8796
}
8897

98+
// kebabToSnake converts a cobra/pflag kebab-case flag name to snake_case by replacing every '-' with
99+
// '_'. Idempotent on already-snake names.
100+
//
101+
// Parameters:
102+
// - `name`: the kebab-case flag name (e.g., "dry-run", "target-root").
103+
//
104+
// Returns:
105+
// - `string`: the snake-case form (e.g., "dry_run", "target_root").
106+
func kebabToSnake(name string) string {
107+
108+
return strings.ReplaceAll(name, "-", "_")
109+
}
110+
89111
// flagValue extracts the typed Go value of a [pflag.Flag] by switching on its declared type. The pflag
90112
// typed accessors are called on the [cobra.Command]'s merged FlagSet because each accessor handles the
91113
// not-found case via its second return value — we already know the flag exists (Visit yielded it), so the
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
// SPDX-License-Identifier: SSPL-1.0
2+
// Copyright (c) 2025-2026 Noble Factor. All rights reserved.
3+
4+
package application
5+
6+
import (
7+
"testing"
8+
9+
"github.com/spf13/cobra"
10+
)
11+
12+
func TestDryRun_Snake(t *testing.T) {
13+
14+
t.Run("true under snake key", func(t *testing.T) {
15+
app := &Application{Flags: map[string]any{"dry_run": true}}
16+
if !app.DryRun() {
17+
t.Error("DryRun() = false, want true")
18+
}
19+
})
20+
21+
t.Run("missing returns false", func(t *testing.T) {
22+
app := &Application{Flags: map[string]any{}}
23+
if app.DryRun() {
24+
t.Error("DryRun() with missing key should be false")
25+
}
26+
})
27+
28+
t.Run("nil Flags returns false", func(t *testing.T) {
29+
app := &Application{}
30+
if app.DryRun() {
31+
t.Error("DryRun() with nil Flags should be false")
32+
}
33+
})
34+
35+
t.Run("wrong type returns false", func(t *testing.T) {
36+
app := &Application{Flags: map[string]any{"dry_run": "true"}}
37+
if app.DryRun() {
38+
t.Error("DryRun() with non-bool value should be false")
39+
}
40+
})
41+
42+
t.Run("kebab key is NOT honored (post-normalization world)", func(t *testing.T) {
43+
app := &Application{Flags: map[string]any{"dry-run": true}}
44+
if app.DryRun() {
45+
t.Error("DryRun() should not read kebab-case key; normalization happens at NewApplication time")
46+
}
47+
})
48+
}
49+
50+
func TestNewApplication_KebabToSnake(t *testing.T) {
51+
52+
cmd := &cobra.Command{Use: "test"}
53+
cmd.Flags().Bool("dry-run", false, "")
54+
cmd.Flags().String("target-root", "", "")
55+
cmd.Flags().String("layer", "", "")
56+
57+
if err := cmd.ParseFlags([]string{"--dry-run", "--target-root", "/tmp/x", "--layer", "personal"}); err != nil {
58+
t.Fatalf("ParseFlags: %v", err)
59+
}
60+
61+
app := NewApplication("writ", cmd)
62+
63+
if app.Name != "writ" {
64+
t.Errorf("Name = %q, want writ", app.Name)
65+
}
66+
67+
for _, key := range []string{"dry_run", "target_root", "layer"} {
68+
if _, ok := app.Flags[key]; !ok {
69+
t.Errorf("Flags missing snake-case key %q (have %v)", key, app.Flags)
70+
}
71+
}
72+
73+
for _, key := range []string{"dry-run", "target-root"} {
74+
if _, ok := app.Flags[key]; ok {
75+
t.Errorf("Flags should NOT have kebab-case key %q after normalization", key)
76+
}
77+
}
78+
79+
if v, _ := app.Flags["dry_run"].(bool); !v {
80+
t.Errorf("dry_run = %v, want true", v)
81+
}
82+
if v, _ := app.Flags["target_root"].(string); v != "/tmp/x" {
83+
t.Errorf("target_root = %q, want /tmp/x", v)
84+
}
85+
}
86+
87+
func TestNewApplication_UnsetFlagsNotStamped(t *testing.T) {
88+
89+
cmd := &cobra.Command{Use: "test"}
90+
cmd.Flags().Bool("dry-run", false, "")
91+
cmd.Flags().String("target-root", "default", "")
92+
93+
// User passes only --dry-run; --target-root is left at default.
94+
if err := cmd.ParseFlags([]string{"--dry-run"}); err != nil {
95+
t.Fatalf("ParseFlags: %v", err)
96+
}
97+
98+
app := NewApplication("writ", cmd)
99+
100+
if _, ok := app.Flags["dry_run"]; !ok {
101+
t.Error("user-supplied flag should be stamped")
102+
}
103+
if _, ok := app.Flags["target_root"]; ok {
104+
t.Error("unset flag (default value) should NOT be stamped")
105+
}
106+
}
107+
108+
func TestKebabToSnake(t *testing.T) {
109+
110+
tests := []struct {
111+
in, want string
112+
}{
113+
{"dry-run", "dry_run"},
114+
{"target-root", "target_root"},
115+
{"layer", "layer"}, // no hyphens: unchanged.
116+
{"already_snake", "already_snake"}, // idempotent.
117+
{"a-b-c-d", "a_b_c_d"},
118+
{"", ""},
119+
}
120+
121+
for _, tt := range tests {
122+
t.Run(tt.in, func(t *testing.T) {
123+
if got := kebabToSnake(tt.in); got != tt.want {
124+
t.Errorf("kebabToSnake(%q) = %q, want %q", tt.in, got, tt.want)
125+
}
126+
})
127+
}
128+
}

‎pkg/op/env_value.go‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
// SPDX-License-Identifier: SSPL-1.0
2+
// Copyright (c) 2025-2026 Noble Factor. All rights reserved.
3+
4+
package op
5+
6+
import (
7+
"encoding"
8+
"encoding/json"
9+
"fmt"
10+
"os"
11+
"reflect"
12+
"strconv"
13+
"time"
14+
)
15+
16+
// Compile-time interface check.
17+
var _ SourceConverter = envValue("")
18+
19+
var (
20+
// envValueTimeDurationType is the `[reflect.Type]` of `[time.Duration]`.
21+
envValueTimeDurationType = reflect.TypeOf(time.Duration(0))
22+
23+
// envValueFileModeType is the `[reflect.Type]` of `[os.FileMode]`.
24+
envValueFileModeType = reflect.TypeOf(os.FileMode(0))
25+
26+
// envValueResourceType is the `[reflect.Type]` of the `[Resource]` interface for the Resource-target `opt-out`.
27+
envValueResourceType = reflect.TypeOf((*Resource)(nil)).Elem()
28+
)
29+
30+
// envValue wraps a raw env-sourced string so the [Convert] cascade routes string parsing through [SourceConverter].
31+
//
32+
// The wrapper exists because plain `string` cannot implement [SourceConverter] (Go forbids methods on built-in types),
33+
// and routing string-source parsing through [Convert] directly would corrupt the cascade. See the layering note at the
34+
// head of `parameter.go`'s [parseDefaultExpression] for the rationale that keeps source-syntax parsing out of [Convert].
35+
//
36+
// envValue keeps env-source string parsing localized: env strings get the lenient cascade (primitives → stdlib
37+
// special-cases → [encoding.TextUnmarshaler] → [json.Unmarshal]), while plain Go strings from starlark slot fills and
38+
// immediate-mode Go calls continue to fail on primitive targets as they always have. [envValue.CanConvertTo] returns
39+
// false for [Resource] targets, so [Convert] step 7 (registered Resource construction) wins routing for env strings
40+
// naming a Resource.
41+
type envValue string
42+
43+
// region EXPORTED METHODS
44+
45+
// region Behaviors
46+
47+
// CanConvertTo reports whether [envValue.ConvertTo] can produce a value of the given target.
48+
//
49+
// Returns false for nil targets and for targets implementing [Resource] (the cascade falls through to [Convert] step 7,
50+
// which routes through the registered Resource constructor with the original env value). Returns true for every other
51+
// target; [envValue.ConvertTo] performs the actual parsing and may return its own error if no strategy succeeds.
52+
//
53+
// Parameters:
54+
// - `target`: the parsed-value type.
55+
//
56+
// Returns:
57+
// - `bool`: false when `target` is nil or implements [Resource]; true otherwise.
58+
func (envValue) CanConvertTo(target reflect.Type) bool {
59+
60+
if target == nil {
61+
return false
62+
}
63+
if target.Implements(envValueResourceType) {
64+
return false
65+
}
66+
return true
67+
}
68+
69+
// ConvertTo parses the wrapped env string into a value of target. Tries (in order):
70+
//
71+
// 1. `string` identity when `target.Kind() == reflect.String`.
72+
// 2. [time.Duration] via [time.ParseDuration] handles "30s", "1h15m", etc.
73+
// 3. [os.FileMode] via [strconv.ParseUint] base 0 handles "0o755", "0x1ff", "493".
74+
// 4. `primitive` [reflect.Kind] switch for bool/int*/uint*/float*/complex* via strconv with base 0.
75+
// 5. [encoding.TextUnmarshaler] on a fresh `*target` probe.
76+
// 6. [json.Unmarshal] universal fallback.
77+
//
78+
// Parameters:
79+
// - `target`: the parsed-value type.
80+
//
81+
// Returns:
82+
// - `any`: the parsed value, dynamic-type-equal to target.
83+
// - `error`: non-nil when no parsing strategy succeeds.
84+
func (e envValue) ConvertTo(target reflect.Type) (any, error) {
85+
86+
raw := string(e)
87+
88+
if target == nil {
89+
return nil, fmt.Errorf("envValue.ConvertTo: nil target")
90+
}
91+
92+
if target.Kind() == reflect.String {
93+
return reflect.ValueOf(raw).Convert(target).Interface(), nil
94+
}
95+
96+
if target == envValueTimeDurationType {
97+
d, err := time.ParseDuration(raw)
98+
if err != nil {
99+
return nil, fmt.Errorf("parse %q as time.Duration: %w", raw, err)
100+
}
101+
return d, nil
102+
}
103+
104+
if target == envValueFileModeType {
105+
v, err := strconv.ParseUint(raw, 0, 32)
106+
if err != nil {
107+
return nil, fmt.Errorf("parse %q as os.FileMode: %w", raw, err)
108+
}
109+
return os.FileMode(v), nil
110+
}
111+
112+
switch target.Kind() {
113+
114+
case reflect.Bool:
115+
v, err := strconv.ParseBool(raw)
116+
if err != nil {
117+
return nil, fmt.Errorf("parse %q as %s: %w", raw, target, err)
118+
}
119+
return reflect.ValueOf(v).Convert(target).Interface(), nil
120+
121+
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
122+
v, err := strconv.ParseInt(raw, 0, target.Bits())
123+
if err != nil {
124+
return nil, fmt.Errorf("parse %q as %s: %w", raw, target, err)
125+
}
126+
return reflect.ValueOf(v).Convert(target).Interface(), nil
127+
128+
case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
129+
v, err := strconv.ParseUint(raw, 0, target.Bits())
130+
if err != nil {
131+
return nil, fmt.Errorf("parse %q as %s: %w", raw, target, err)
132+
}
133+
return reflect.ValueOf(v).Convert(target).Interface(), nil
134+
135+
case reflect.Float32, reflect.Float64:
136+
v, err := strconv.ParseFloat(raw, target.Bits())
137+
if err != nil {
138+
return nil, fmt.Errorf("parse %q as %s: %w", raw, target, err)
139+
}
140+
return reflect.ValueOf(v).Convert(target).Interface(), nil
141+
142+
case reflect.Complex64, reflect.Complex128:
143+
v, err := strconv.ParseComplex(raw, target.Bits())
144+
if err != nil {
145+
return nil, fmt.Errorf("parse %q as %s: %w", raw, target, err)
146+
}
147+
return reflect.ValueOf(v).Convert(target).Interface(), nil
148+
}
149+
150+
probe := reflect.New(target).Interface()
151+
152+
if tu, ok := probe.(encoding.TextUnmarshaler); ok {
153+
if err := tu.UnmarshalText([]byte(raw)); err != nil {
154+
return nil, fmt.Errorf("UnmarshalText %q into %s: %w", raw, target, err)
155+
}
156+
return reflect.ValueOf(probe).Elem().Interface(), nil
157+
}
158+
159+
out := reflect.New(target).Interface()
160+
161+
if err := json.Unmarshal([]byte(raw), out); err != nil {
162+
return nil, fmt.Errorf("unmarshal %q as %s: %w", raw, target, err)
163+
}
164+
return reflect.ValueOf(out).Elem().Interface(), nil
165+
}
166+
167+
// endregion
168+
169+
// endregion

0 commit comments

Comments
 (0)