Preserve User Intent When Parsing Pointers (#336)

* Only set a field if the flag was set by the user, otherwise set it to nil. This prevents accidental overrides from Cobra defaults (eg. if --auto-deploy isn't set the field will be nil), allows for better type support and cleaner parsing code

* Refactor types/service.go to use typed input from cobra parsing for serviceType, Runtime and region

* Use typed values in clone and add test helper

GitOrigin-RevId: cf88047a428e2d80ec414f7e0f2eacd93f968eb1

Author: mosslilley

pkg/command/inputs.go

@@ -171,6 +171,50 @@ func getBoolValue(flags *pflag.FlagSet, args []string, tag string) (*bool, error
 	return &val, nil
 }
 
+func shouldSetPointerField(flags *pflag.FlagSet, cliTag string) bool {
+	return isArg(cliTag) || flags.Changed(cliTag)
+}
+
+func setStringPointerField(elemField reflect.Value, shouldSet bool, val *string) {
+	if !shouldSet || val == nil {
+		elemField.SetZero()
+		return
+	}
+	ptr := reflect.New(elemField.Type().Elem())
+	ptr.Elem().SetString(*val)
+	elemField.Set(ptr)
+}
+
+func setIntPointerField(elemField reflect.Value, shouldSet bool, val *int) {
+	if !shouldSet || val == nil {
+		elemField.SetZero()
+		return
+	}
+	ptr := reflect.New(elemField.Type().Elem())
+	ptr.Elem().SetInt(int64(*val))
+	elemField.Set(ptr)
+}
+
+func setFloat64PointerField(elemField reflect.Value, shouldSet bool, val *float64) {
+	if !shouldSet || val == nil {
+		elemField.SetZero()
+		return
+	}
+	ptr := reflect.New(elemField.Type().Elem())
+	ptr.Elem().SetFloat(*val)
+	elemField.Set(ptr)
+}
+
+func setBoolPointerField(elemField reflect.Value, shouldSet bool, val *bool) {
+	if !shouldSet || val == nil {
+		elemField.SetZero()
+		return
+	}
+	ptr := reflect.New(elemField.Type().Elem())
+	ptr.Elem().SetBool(*val)
+	elemField.Set(ptr)
+}
+
 func ParseCommandInteractiveOnly(cmd *cobra.Command, args []string, v any) error {
 	format := GetFormatFromContext(cmd.Context())
 	if !format.Interactive() {
@@ -221,25 +265,25 @@ func ParseCommand(cmd *cobra.Command, args []string, v any) error {
 				if err != nil {
 					return err
 				}
-				elemField.Set(reflect.ValueOf(val))
+				setStringPointerField(elemField, shouldSetPointerField(flags, cliTag), val)
 			case reflect.Int:
 				val, err := getIntValue(flags, args, cliTag)
 				if err != nil {
 					return err
 				}
-				elemField.Set(reflect.ValueOf(val))
+				setIntPointerField(elemField, shouldSetPointerField(flags, cliTag), val)
 			case reflect.Float64:
 				val, err := getFloat64Value(flags, args, cliTag)
 				if err != nil {
 					return err
 				}
-				elemField.Set(reflect.ValueOf(val))
+				setFloat64PointerField(elemField, shouldSetPointerField(flags, cliTag), val)
 			case reflect.Bool:
 				val, err := getBoolValue(flags, args, cliTag)
 				if err != nil {
 					return err
 				}
-				elemField.Set(reflect.ValueOf(val))
+				setBoolPointerField(elemField, shouldSetPointerField(flags, cliTag), val)
 			}
 		case reflect.Slice:
 			switch field.Type.Elem().Kind() {

pkg/command/inputs_test.go

@@ -95,6 +95,36 @@ func TestParseCommand(t *testing.T) {
 		require.Equal(t, "bar", *v.Foo)
 	})
 
+	t.Run("unset pointer flag remains nil", func(t *testing.T) {
+		type testStruct struct {
+			Foo *bool `cli:"foo"`
+		}
+		var v testStruct
+		cmd := &cobra.Command{}
+		cmd.Flags().Bool("foo", true, "")
+		require.NoError(t, cmd.ParseFlags([]string{}))
+
+		err := command.ParseCommand(cmd, []string{}, &v)
+		require.NoError(t, err)
+		require.Nil(t, v.Foo)
+	})
+
+	t.Run("parse pointer alias type", func(t *testing.T) {
+		type myString string
+		type testStruct struct {
+			Foo *myString `cli:"foo"`
+		}
+		var v testStruct
+		cmd := &cobra.Command{}
+		cmd.Flags().String("foo", "", "")
+		require.NoError(t, cmd.ParseFlags([]string{"--foo", "bar"}))
+
+		err := command.ParseCommand(cmd, []string{}, &v)
+		require.NoError(t, err)
+		require.NotNil(t, v.Foo)
+		require.Equal(t, myString("bar"), *v.Foo)
+	})
+
 	t.Run("parse slice", func(t *testing.T) {
 		type testStruct struct {
 			Foo []string `cli:"foo"`

pkg/service/clone.go

@@ -75,7 +75,7 @@ func extractCloneSourceDefaults(source *client.Service) sourceDefaults {
 }
 
 func applyBaseDefaults(input *servicetypes.Service, defaults sourceDefaults) {
-	input.Type = withDefaultFromValue(input.Type, defaults.serviceType)
+	input.Type = withDefaultFromValue(input.Type, servicetypes.ServiceType(defaults.serviceType))
 	input.RootDirectory = withDefault(input.RootDirectory, defaults.rootDirectory)
 	input.EnvironmentID = withDefault(input.EnvironmentID, defaults.environmentID)
 }
@@ -112,7 +112,7 @@ func applyRuntimeDefaults(input *servicetypes.Service, defaults sourceDefaults)
 		return
 	}
 
-	input.Runtime = withDefaultFromValue(input.Runtime, *defaults.runtime)
+	input.Runtime = withDefaultFromValue(input.Runtime, servicetypes.ServiceRuntime(*defaults.runtime))
 }
 
 func applyRegistryCredentialDefault(input *servicetypes.Service, defaults sourceDefaults) {
@@ -136,8 +136,11 @@ func withDefault(dst *string, src *string) *string {
 	return pointers.From(*src)
 }
 
-func withDefaultFromValue[S ~string](dst *string, src S) *string {
-	return withDefault(dst, pointers.From(string(src)))
+func withDefaultFromValue[T ~string](dst *T, src T) *T {
+	if dst != nil {
+		return dst
+	}
+	return pointers.From(src)
 }
 
 // RuntimeFromSourceService extracts runtime from a service when that service type has a runtime field.

pkg/service/clone_test.go

@@ -32,12 +32,12 @@ func TestCLIInputFromSource(t *testing.T) {
 
 		ServiceFromAPI(&input, source)
 
-		require.Equal(t, "web_service", *input.Type)
+		require.Equal(t, servicetypes.ServiceType("web_service"), *input.Type)
 		require.Equal(t, "https://github.com/renderinc/api", *input.Repo)
 		require.Equal(t, "master", *input.Branch)
 		require.Equal(t, "services/api", *input.RootDirectory)
 		require.Equal(t, "evm-123", *input.EnvironmentID)
-		require.Equal(t, "node", *input.Runtime)
+		require.Equal(t, servicetypes.ServiceRuntime("node"), *input.Runtime)
 	})
 
 	t.Run("hydrates image-backed defaults", func(t *testing.T) {
@@ -61,7 +61,7 @@ func TestCLIInputFromSource(t *testing.T) {
 
 		require.Equal(t, "docker.io/org/app:latest", *input.Image)
 		require.Equal(t, "rgc-123", *input.RegistryCredential)
-		require.Equal(t, "image", *input.Runtime)
+		require.Equal(t, servicetypes.ServiceRuntime("image"), *input.Runtime)
 	})
 
 	t.Run("hydrates docker runtime registry credential from docker details", func(t *testing.T) {
@@ -95,7 +95,7 @@ func TestCLIInputFromSource(t *testing.T) {
 
 		ServiceFromAPI(&input, source)
 
-		require.Equal(t, "docker", *input.Runtime)
+		require.Equal(t, servicetypes.ServiceRuntime("docker"), *input.Runtime)
 		require.Equal(t, "rgc-456", *input.RegistryCredential)
 	})
 
@@ -119,12 +119,12 @@ func TestCLIInputFromSource(t *testing.T) {
 
 		ServiceFromAPI(&input, source)
 
-		require.Equal(t, "background_worker", *input.Type)
+		require.Equal(t, servicetypes.ServiceType("background_worker"), *input.Type)
 		require.Equal(t, "https://github.com/renderinc/worker", *input.Repo)
 		require.Equal(t, "main", *input.Branch)
 		require.Equal(t, "workers/processor", *input.RootDirectory)
 		require.Equal(t, "evm-456", *input.EnvironmentID)
-		require.Equal(t, "python", *input.Runtime)
+		require.Equal(t, servicetypes.ServiceRuntime("python"), *input.Runtime)
 	})
 
 	t.Run("hydrates static site defaults", func(t *testing.T) {
@@ -147,7 +147,7 @@ func TestCLIInputFromSource(t *testing.T) {
 
 		ServiceFromAPI(&input, source)
 
-		require.Equal(t, "static_site", *input.Type)
+		require.Equal(t, servicetypes.ServiceType("static_site"), *input.Type)
 		require.Equal(t, "https://github.com/renderinc/docs", *input.Repo)
 		require.Equal(t, "main", *input.Branch)
 		require.Equal(t, "website", *input.RootDirectory)
@@ -182,18 +182,18 @@ func TestCLIInputFromSource(t *testing.T) {
 		input := servicetypes.Service{
 			Name:    "clone-explicit",
 			From:    pointers.From("srv-source"),
-			Type:    pointers.From("private_service"),
+			Type:    svcTypeRaw("private_service"),
 			Repo:    pointers.From("https://github.com/org/custom"),
 			Branch:  pointers.From("feature-x"),
-			Runtime: pointers.From("docker"),
+			Runtime: svcRuntime(servicetypes.ServiceRuntimeDocker),
 		}
 
 		ServiceFromAPI(&input, source)
 
-		require.Equal(t, "private_service", *input.Type)
+		require.Equal(t, servicetypes.ServiceType("private_service"), *input.Type)
 		require.Equal(t, "https://github.com/org/custom", *input.Repo)
 		require.Equal(t, "feature-x", *input.Branch)
-		require.Equal(t, "docker", *input.Runtime)
+		require.Equal(t, servicetypes.ServiceRuntime("docker"), *input.Runtime)
 	})
 
 	t.Run("does not copy repo defaults when image is explicitly provided", func(t *testing.T) {
@@ -214,7 +214,7 @@ func TestCLIInputFromSource(t *testing.T) {
 		require.Equal(t, "docker.io/custom/image:latest", *input.Image)
 		require.Nil(t, input.Repo)
 		require.Nil(t, input.Branch)
-		require.Equal(t, "image", *input.Runtime)
+		require.Equal(t, servicetypes.ServiceRuntime("image"), *input.Runtime)
 	})
 
 	t.Run("does not copy image defaults when repo is explicitly provided", func(t *testing.T) {

pkg/service/create.go

@@ -5,7 +5,7 @@ import (
 
 	"github.com/render-oss/cli/pkg/client"
 	"github.com/render-oss/cli/pkg/pointers"
-	"github.com/render-oss/cli/pkg/types"
+	types "github.com/render-oss/cli/pkg/types"
 	servicetypes "github.com/render-oss/cli/pkg/types/service"
 )
 
@@ -29,10 +29,10 @@ func BuildCreateRequest(cliInput servicetypes.Service, ownerID string) (client.C
 		return client.CreateServiceJSONRequestBody{}, err
 	}
 
-	typedServiceType := toTypedFromAlias[servicetypes.ServiceType, client.ServiceType](serviceType)
-	typedRuntime := toTypedFromAlias[servicetypes.ServiceRuntime, client.ServiceRuntime](runtime)
-	typedRegion := toTypedFromAlias[types.Region, client.Region](region)
-	typedPlan := toTypedFromAlias[string, client.Plan](cliInput.Plan)
+	typedServiceType := toClientType(serviceType)
+	typedRuntime := toClientRuntime(runtime)
+	typedRegion := toClientRegion(region)
+	typedPlan := toClientPlan(cliInput.Plan)
 
 	// When an image is provided without an explicit runtime, default to "image" runtime.
 	// This allows users to omit --runtime when using --image.
@@ -99,14 +99,6 @@ func BuildCreateRequest(cliInput servicetypes.Service, ownerID string) (client.C
 	return body, nil
 }
 
-func toTypedFromAlias[S ~string, T ~string](value *S) *T {
-	if value == nil {
-		return nil
-	}
-	typed := T(*value)
-	return &typed
-}
-
 func parseEnvVarInputs(values []string) ([]client.EnvVarInput, error) {
 	if len(values) == 0 {
 		return nil, nil
@@ -310,3 +302,35 @@ func buildCronEnvSpecificDetails(buildCommand *string, cronCommand *string) (*cl
 
 	return envSpecificDetails, nil
 }
+
+func toClientType(value *servicetypes.ServiceType) *client.ServiceType {
+	if value == nil {
+		return nil
+	}
+	typed := client.ServiceType(*value)
+	return &typed
+}
+
+func toClientRuntime(value *servicetypes.ServiceRuntime) *client.ServiceRuntime {
+	if value == nil {
+		return nil
+	}
+	typed := client.ServiceRuntime(*value)
+	return &typed
+}
+
+func toClientRegion(value *types.Region) *client.Region {
+	if value == nil {
+		return nil
+	}
+	typed := client.Region(*value)
+	return &typed
+}
+
+func toClientPlan(value *string) *client.Plan {
+	if value == nil {
+		return nil
+	}
+	typed := client.Plan(*value)
+	return &typed
+}