From 858fd1be490d4706a916b012b47b1907820a0684 Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Wed, 28 Oct 2026 15:31:02 +0200 Subject: [PATCH 01/11] chore(deps): bump verdacloud-sdk-go to v1.7.0 Raises the go directive to 1.26.10 (the SDK's floor) and pulls ozzo-validation v4.4.1. Adapt to the SDK: - NewTestClient now takes testing.TB. - Instances.Get is deprecated: call Instances.List (nil for all, ListInstancesOptions{Status} otherwise), which is what Get wraps. - Hibernate is deprecated upstream but the CLI still supports it, so its uses get a staticcheck SA1019 nolint. - Instance/Volume/VolumeInTrash views carry the new SDK fields (tags, private_ip, private_network_id, subnet_id); the views coverage test requires it. JSON/YAML output gains these keys (additive). - Two volume loops index instead of copying the larger SDK struct. Co-Authored-By: Claude Opus 5.5 --- go.mod | 6 +- go.sum | 12 +-- internal/verda-cli/cmd/cost/running.go | 2 +- internal/verda-cli/cmd/images/images_test.go | 2 +- internal/verda-cli/cmd/mcp/tools_cost.go | 2 +- internal/verda-cli/cmd/mcp/tools_ssh.go | 2 +- internal/verda-cli/cmd/mcp/tools_vm.go | 6 +- internal/verda-cli/cmd/ssh/ssh.go | 2 +- internal/verda-cli/cmd/status/status.go | 2 +- internal/verda-cli/cmd/util/views.go | 108 ++++++++++--------- internal/verda-cli/cmd/vm/action.go | 8 +- internal/verda-cli/cmd/vm/batch.go | 2 +- internal/verda-cli/cmd/vm/batch_test.go | 2 +- internal/verda-cli/cmd/vm/instances.go | 2 +- internal/verda-cli/cmd/vm/list.go | 2 +- internal/verda-cli/cmd/vm/shortcuts.go | 2 +- internal/verda-cli/cmd/vm/wizard.go | 3 +- internal/verda-cli/cmd/vm/wizard_subflows.go | 3 +- 18 files changed, 91 insertions(+), 77 deletions(-) diff --git a/go.mod b/go.mod index d4e42f6..fbcd126 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/verda-cloud/verda-cli -go 1.26.9 +go 1.26.10 require ( charm.land/lipgloss/v2 v2.0.2 @@ -8,7 +8,7 @@ require ( github.com/spf13/cobra v1.10.2 github.com/spf13/pflag v1.0.10 github.com/spf13/viper v1.21.0 - github.com/verda-cloud/verdacloud-sdk-go v1.4.2 + github.com/verda-cloud/verdacloud-sdk-go v1.7.0 go.yaml.in/yaml/v3 v3.0.4 ) @@ -65,7 +65,7 @@ require ( github.com/fsnotify/fsnotify v1.9.0 // indirect github.com/go-logr/logr v1.4.4 // indirect github.com/go-logr/stdr v1.2.2 // indirect - github.com/go-ozzo/ozzo-validation/v4 v4.3.0 // indirect + github.com/go-ozzo/ozzo-validation/v4 v4.4.1 // indirect github.com/go-viper/mapstructure/v2 v2.4.0 // indirect github.com/google/jsonschema-go v0.4.2 // indirect github.com/google/uuid v1.6.0 // indirect diff --git a/go.sum b/go.sum index 881e5a4..bf16c49 100644 --- a/go.sum +++ b/go.sum @@ -8,8 +8,8 @@ github.com/MakeNowJust/heredoc v1.0.0 h1:cXCdzVdstXyiTqTvfqk9SDHpKNjxuom+DOlyEeQ github.com/MakeNowJust/heredoc v1.0.0/go.mod h1:mG5amYoWBHf8vpLOuehzbGGw0EHxpZZ6lCpQ4fNJ8LE= github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY= github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= -github.com/asaskevich/govalidator v0.0.0-20200108200545-475eaeb16496 h1:zV3ejI06GQ59hwDQAvmK1qxOQGB3WuVTRoY0okPTAv0= -github.com/asaskevich/govalidator v0.0.0-20200108200545-475eaeb16496/go.mod h1:oGkLhpf+kjZl6xBf758TQhh5XrAeiJv/7FRz/2spLIg= +github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d h1:Byv0BzEl3/e6D5CLfI0j/7hiIEtvGVFPCZ7Ei2oq8iQ= +github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw= github.com/atotto/clipboard v0.1.4 h1:EH0zSVneZPSuFR11BlR9YppQTVDbh5+16AmcJi4g1z4= github.com/atotto/clipboard v0.1.4/go.mod h1:ZY9tmq7sm5xIbd9bOK4onWV4S6X0u6GY7Vn0Yu86PYI= github.com/aws/aws-sdk-go-v2 v1.41.6 h1:1AX0AthnBQzMx1vbmir3Y4WsnJgiydmnJjiLu+LvXOg= @@ -107,8 +107,8 @@ github.com/go-logr/logr v1.4.4 h1:tG4xh9yMsRCAiodLVTxyrkzSZ9+o0L1Kg/+cPVcbP/8= github.com/go-logr/logr v1.4.4/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= -github.com/go-ozzo/ozzo-validation/v4 v4.3.0 h1:byhDUpfEwjsVQb1vBunvIjh2BHQ9ead57VkAEY4V+Es= -github.com/go-ozzo/ozzo-validation/v4 v4.3.0/go.mod h1:2NKgrcHl3z6cJs+3Oo940FPRiTzuqKbvfrL2RxCj6Ew= +github.com/go-ozzo/ozzo-validation/v4 v4.4.1 h1:AQ3X8zHnXEuNE04pyc1H/nmIlroNjgZ7hcY7Xv/IgH8= +github.com/go-ozzo/ozzo-validation/v4 v4.4.1/go.mod h1:4ZtPNefSnNq39wjL+2We8y2ysqEX/S4D5mPybufHd7Y= github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9LvH92wZUgs= github.com/go-viper/mapstructure/v2 v2.4.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= @@ -187,8 +187,8 @@ github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8 github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU= github.com/vbatts/tar-split v0.12.2 h1:w/Y6tjxpeiFMR47yzZPlPj/FcPLpXbTUi/9H7d3CPa4= github.com/vbatts/tar-split v0.12.2/go.mod h1:eF6B6i6ftWQcDqEn3/iGFRFRo8cBIMSJVOpnNdfTMFA= -github.com/verda-cloud/verdacloud-sdk-go v1.4.2 h1:oVb8fHVQOY+YPuuMYMee9gYCkPTwAw01LmkqxM21T/Y= -github.com/verda-cloud/verdacloud-sdk-go v1.4.2/go.mod h1:pmlpiCL9fTSikZ3qWLJPpHOG0E8PKkQVUX5s4Z+SktY= +github.com/verda-cloud/verdacloud-sdk-go v1.7.0 h1:ioe12/i1IDYmOwwZje2T7jCYPfToE1jMBDHXY6ZXSHU= +github.com/verda-cloud/verdacloud-sdk-go v1.7.0/go.mod h1:1APG3X5TXhXpUp1dhgemElaj/6bw+OaKeQ+Ze/0/Cyc= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM= github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zIM+UJPGz4= diff --git a/internal/verda-cli/cmd/cost/running.go b/internal/verda-cli/cmd/cost/running.go index a70e088..fb3a916 100644 --- a/internal/verda-cli/cmd/cost/running.go +++ b/internal/verda-cli/cmd/cost/running.go @@ -95,7 +95,7 @@ func runRunning(cmd *cobra.Command, f cmdutil.Factory, ioStreams cmdutil.IOStrea sp, _ = status.Spinner(ctx, "Loading instances and volumes...") } - instances, err := client.Instances.Get(ctx, "") + instances, err := client.Instances.List(ctx, nil) if err != nil { if sp != nil { sp.Stop("") diff --git a/internal/verda-cli/cmd/images/images_test.go b/internal/verda-cli/cmd/images/images_test.go index a945536..99d2536 100644 --- a/internal/verda-cli/cmd/images/images_test.go +++ b/internal/verda-cli/cmd/images/images_test.go @@ -94,7 +94,7 @@ func TestOutputShowsImageTypeNotID(t *testing.T) { mockServer := testutil.NewMockServer() defer mockServer.Close() - client := verda.NewTestClient(mockServer) + client := verda.NewTestClient(t, mockServer) var buf bytes.Buffer ioStreams := cmdutil.IOStreams{Out: &buf, ErrOut: &buf} diff --git a/internal/verda-cli/cmd/mcp/tools_cost.go b/internal/verda-cli/cmd/mcp/tools_cost.go index 12ace04..effe786 100644 --- a/internal/verda-cli/cmd/mcp/tools_cost.go +++ b/internal/verda-cli/cmd/mcp/tools_cost.go @@ -188,7 +188,7 @@ func (s *Server) handleGetRunningCosts(ctx context.Context, _ mcp.CallToolReques return mcp.NewToolResultError(err.Error()), nil } - instances, err := client.Instances.Get(ctx, "running") + instances, err := client.Instances.List(ctx, &verda.ListInstancesOptions{Status: "running"}) if err != nil { return mcp.NewToolResultError(err.Error()), nil } diff --git a/internal/verda-cli/cmd/mcp/tools_ssh.go b/internal/verda-cli/cmd/mcp/tools_ssh.go index c2f1889..803d304 100644 --- a/internal/verda-cli/cmd/mcp/tools_ssh.go +++ b/internal/verda-cli/cmd/mcp/tools_ssh.go @@ -170,7 +170,7 @@ func (s *Server) resolveInstance(ctx context.Context, idOrHostname string) (*ver } // Fall back to searching by hostname. - instances, err := client.Instances.Get(ctx, "") + instances, err := client.Instances.List(ctx, nil) if err != nil { return nil, err } diff --git a/internal/verda-cli/cmd/mcp/tools_vm.go b/internal/verda-cli/cmd/mcp/tools_vm.go index 7674169..0104129 100644 --- a/internal/verda-cli/cmd/mcp/tools_vm.go +++ b/internal/verda-cli/cmd/mcp/tools_vm.go @@ -215,7 +215,7 @@ func (s *Server) handleListVMs(ctx context.Context, req mcp.CallToolRequest) (*m return mcp.NewToolResultError(err.Error()), nil } - instances, err := client.Instances.Get(ctx, status) + instances, err := client.Instances.List(ctx, &verda.ListInstancesOptions{Status: status}) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -434,10 +434,10 @@ var vmActions = map[string]vmAction{ destructive: true, exec: func(ctx context.Context, c *verda.Client, id string) error { return c.Instances.ForceShutdown(ctx, id) }, }, - verda.ActionHibernate: { + verda.ActionHibernate: { //nolint:staticcheck // SA1019: hibernate is deprecated but still supported expectStatus: verda.StatusOffline, destructive: true, - exec: func(ctx context.Context, c *verda.Client, id string) error { return c.Instances.Hibernate(ctx, id) }, + exec: func(ctx context.Context, c *verda.Client, id string) error { return c.Instances.Hibernate(ctx, id) }, //nolint:staticcheck // SA1019: hibernate is deprecated but still supported }, verda.ActionDelete: { destructive: true, diff --git a/internal/verda-cli/cmd/ssh/ssh.go b/internal/verda-cli/cmd/ssh/ssh.go index d92bd6d..3226e0b 100644 --- a/internal/verda-cli/cmd/ssh/ssh.go +++ b/internal/verda-cli/cmd/ssh/ssh.go @@ -114,7 +114,7 @@ func runSSH(cmd *cobra.Command, f cmdutil.Factory, ioStreams cmdutil.IOStreams, if status := f.Status(); status != nil { sp, _ = status.Spinner(ctx, "Resolving instance...") } - instances, err := client.Instances.Get(ctx, "") + instances, err := client.Instances.List(ctx, nil) if sp != nil { sp.Stop("") } diff --git a/internal/verda-cli/cmd/status/status.go b/internal/verda-cli/cmd/status/status.go index 03dd7c2..847a776 100644 --- a/internal/verda-cli/cmd/status/status.go +++ b/internal/verda-cli/cmd/status/status.go @@ -120,7 +120,7 @@ func runStatus(cmd *cobra.Command, f cmdutil.Factory, ioStreams cmdutil.IOStream go func() { var r result // Instances - r.instances, r.err = client.Instances.Get(ctx, "") + r.instances, r.err = client.Instances.List(ctx, nil) if r.err != nil { ch <- r return diff --git a/internal/verda-cli/cmd/util/views.go b/internal/verda-cli/cmd/util/views.go index 41c5899..bef38fd 100644 --- a/internal/verda-cli/cmd/util/views.go +++ b/internal/verda-cli/cmd/util/views.go @@ -100,59 +100,67 @@ func NewStartupScriptViews(scripts []verda.StartupScript) []StartupScriptView { // verda.Instance field; TestInstanceViewCoversSDKFields guards against drift. // Nested types are reused from the SDK — none carries a zero-time field. type InstanceView struct { - ID string `json:"id" yaml:"id"` - IP *string `json:"ip" yaml:"ip"` - Status string `json:"status" yaml:"status"` - CreatedAt *time.Time `json:"created_at,omitempty" yaml:"created_at,omitempty"` - CPU verda.InstanceCPU `json:"cpu" yaml:"cpu"` - GPU verda.InstanceGPU `json:"gpu" yaml:"gpu"` - GPUMemory verda.InstanceMemory `json:"gpu_memory" yaml:"gpu_memory"` - Memory verda.InstanceMemory `json:"memory" yaml:"memory"` - Storage verda.InstanceStorage `json:"storage" yaml:"storage"` - Hostname string `json:"hostname" yaml:"hostname"` - Description string `json:"description" yaml:"description"` - Location string `json:"location" yaml:"location"` - PricePerHour verda.FlexibleFloat `json:"price_per_hour" yaml:"price_per_hour"` - IsSpot bool `json:"is_spot" yaml:"is_spot"` - InstanceType string `json:"instance_type" yaml:"instance_type"` - Image string `json:"image" yaml:"image"` - OSName string `json:"os_name" yaml:"os_name"` - StartupScriptID *string `json:"startup_script_id" yaml:"startup_script_id"` - SSHKeyIDs []string `json:"ssh_key_ids" yaml:"ssh_key_ids"` - OSVolumeID *string `json:"os_volume_id" yaml:"os_volume_id"` - JupyterToken string `json:"jupyter_token" yaml:"jupyter_token"` - Contract string `json:"contract" yaml:"contract"` - Pricing string `json:"pricing" yaml:"pricing"` - VolumeIDs []string `json:"volume_ids" yaml:"volume_ids"` + ID string `json:"id" yaml:"id"` + IP *string `json:"ip" yaml:"ip"` + PrivateIP *string `json:"private_ip" yaml:"private_ip"` + PrivateNetworkID *string `json:"private_network_id" yaml:"private_network_id"` + SubnetID *string `json:"subnet_id" yaml:"subnet_id"` + Status string `json:"status" yaml:"status"` + CreatedAt *time.Time `json:"created_at,omitempty" yaml:"created_at,omitempty"` + CPU verda.InstanceCPU `json:"cpu" yaml:"cpu"` + GPU verda.InstanceGPU `json:"gpu" yaml:"gpu"` + GPUMemory verda.InstanceMemory `json:"gpu_memory" yaml:"gpu_memory"` + Memory verda.InstanceMemory `json:"memory" yaml:"memory"` + Storage verda.InstanceStorage `json:"storage" yaml:"storage"` + Hostname string `json:"hostname" yaml:"hostname"` + Description string `json:"description" yaml:"description"` + Location string `json:"location" yaml:"location"` + PricePerHour verda.FlexibleFloat `json:"price_per_hour" yaml:"price_per_hour"` + IsSpot bool `json:"is_spot" yaml:"is_spot"` + InstanceType string `json:"instance_type" yaml:"instance_type"` + Image string `json:"image" yaml:"image"` + OSName string `json:"os_name" yaml:"os_name"` + StartupScriptID *string `json:"startup_script_id" yaml:"startup_script_id"` + SSHKeyIDs []string `json:"ssh_key_ids" yaml:"ssh_key_ids"` + OSVolumeID *string `json:"os_volume_id" yaml:"os_volume_id"` + JupyterToken string `json:"jupyter_token" yaml:"jupyter_token"` + Contract string `json:"contract" yaml:"contract"` + Pricing string `json:"pricing" yaml:"pricing"` + VolumeIDs []string `json:"volume_ids" yaml:"volume_ids"` + Tags []verda.Tag `json:"tags" yaml:"tags"` } // NewInstanceView converts one SDK instance to its output shape. func NewInstanceView(i *verda.Instance) InstanceView { return InstanceView{ - ID: i.ID, - IP: i.IP, - Status: i.Status, - CreatedAt: nilIfZero(i.CreatedAt), - CPU: i.CPU, - GPU: i.GPU, - GPUMemory: i.GPUMemory, - Memory: i.Memory, - Storage: i.Storage, - Hostname: i.Hostname, - Description: i.Description, - Location: i.Location, - PricePerHour: i.PricePerHour, - IsSpot: i.IsSpot, - InstanceType: i.InstanceType, - Image: i.Image, - OSName: i.OSName, - StartupScriptID: i.StartupScriptID, - SSHKeyIDs: i.SSHKeyIDs, - OSVolumeID: i.OSVolumeID, - JupyterToken: i.JupyterToken, - Contract: i.Contract, - Pricing: i.Pricing, - VolumeIDs: i.VolumeIDs, + ID: i.ID, + IP: i.IP, + PrivateIP: i.PrivateIP, + PrivateNetworkID: i.PrivateNetworkID, + SubnetID: i.SubnetID, + Status: i.Status, + CreatedAt: nilIfZero(i.CreatedAt), + CPU: i.CPU, + GPU: i.GPU, + GPUMemory: i.GPUMemory, + Memory: i.Memory, + Storage: i.Storage, + Hostname: i.Hostname, + Description: i.Description, + Location: i.Location, + PricePerHour: i.PricePerHour, + IsSpot: i.IsSpot, + InstanceType: i.InstanceType, + Image: i.Image, + OSName: i.OSName, + StartupScriptID: i.StartupScriptID, + SSHKeyIDs: i.SSHKeyIDs, + OSVolumeID: i.OSVolumeID, + JupyterToken: i.JupyterToken, + Contract: i.Contract, + Pricing: i.Pricing, + VolumeIDs: i.VolumeIDs, + Tags: i.Tags, } } @@ -189,6 +197,7 @@ type VolumeView struct { MonthlyPrice float64 `json:"monthly_price" yaml:"monthly_price"` Currency string `json:"currency" yaml:"currency"` LongTerm *verda.VolumeLongTerm `json:"long_term" yaml:"long_term"` + Tags []verda.Tag `json:"tags" yaml:"tags"` } // NewVolumeView converts one SDK volume to its output shape. @@ -215,6 +224,7 @@ func NewVolumeView(v *verda.Volume) VolumeView { MonthlyPrice: v.MonthlyPrice, Currency: v.Currency, LongTerm: v.LongTerm, + Tags: v.Tags, } } @@ -249,6 +259,7 @@ type VolumeInTrashView struct { MonthlyPrice float64 `json:"monthly_price" yaml:"monthly_price"` Currency string `json:"currency" yaml:"currency"` IsPermanentlyDeleted bool `json:"is_permanently_deleted" yaml:"is_permanently_deleted"` + Tags []verda.Tag `json:"tags" yaml:"tags"` } // NewVolumeInTrashView converts one SDK trashed volume to its output shape. @@ -272,6 +283,7 @@ func NewVolumeInTrashView(v *verda.VolumeInTrash) VolumeInTrashView { MonthlyPrice: v.MonthlyPrice, Currency: v.Currency, IsPermanentlyDeleted: v.IsPermanentlyDeleted, + Tags: v.Tags, } } diff --git a/internal/verda-cli/cmd/vm/action.go b/internal/verda-cli/cmd/vm/action.go index 65a5536..931d42e 100644 --- a/internal/verda-cli/cmd/vm/action.go +++ b/internal/verda-cli/cmd/vm/action.go @@ -33,7 +33,7 @@ var actionNameMap = map[string]string{ verda.ActionShutdown: "Shutdown", verda.ActionForceShutdown: "Force shutdown", "force-shutdown": "Force shutdown", // CLI alias (hyphenated) - verda.ActionHibernate: "Hibernate", + verda.ActionHibernate: "Hibernate", //nolint:staticcheck // SA1019: hibernate is deprecated but still supported verda.ActionDelete: "Delete instance", } @@ -83,7 +83,7 @@ var allActions = []instanceAction{ ConfirmMsg: "Hibernate does not save runtime state or support resuming the instance.", ExpectStatus: verda.StatusOffline, Execute: func(ctx context.Context, c *verda.Client, inst *verda.Instance) error { - return c.Instances.Hibernate(ctx, inst.ID) + return c.Instances.Hibernate(ctx, inst.ID) //nolint:staticcheck // SA1019: hibernate is deprecated but still supported }, }, { @@ -454,7 +454,7 @@ func selectAction(ctx context.Context, ioStreams cmdutil.IOStreams, prompter tui actionLabels := make([]string, 0, len(validActions)+1) for _, a := range validActions { label := a.Label - if label == actionNameMap[verda.ActionHibernate] { + if label == actionNameMap[verda.ActionHibernate] { //nolint:staticcheck // SA1019: hibernate is deprecated but still supported label += " (deprecated)" } actionLabels = append(actionLabels, label) @@ -606,7 +606,7 @@ func confirmDestructive(ctx context.Context, ioStreams cmdutil.IOStreams, prompt } func warnDeprecatedAction(ioStreams cmdutil.IOStreams, label string) { - if label == actionNameMap[verda.ActionHibernate] { + if label == actionNameMap[verda.ActionHibernate] { //nolint:staticcheck // SA1019: hibernate is deprecated but still supported _, _ = fmt.Fprintln(ioStreams.ErrOut, "Warning: hibernate is deprecated and will be removed in a future version. It discontinues compute and retains volumes. Retained volumes continue billing.") } } diff --git a/internal/verda-cli/cmd/vm/batch.go b/internal/verda-cli/cmd/vm/batch.go index cf4b02d..d120111 100644 --- a/internal/verda-cli/cmd/vm/batch.go +++ b/internal/verda-cli/cmd/vm/batch.go @@ -280,7 +280,7 @@ func actionNameToAPI(action string) string { case "force_shutdown", "force-shutdown": return verda.ActionForceShutdown case "hibernate": - return verda.ActionHibernate + return verda.ActionHibernate //nolint:staticcheck // SA1019: hibernate is deprecated but still supported case actionNameDelete: return verda.ActionDelete default: diff --git a/internal/verda-cli/cmd/vm/batch_test.go b/internal/verda-cli/cmd/vm/batch_test.go index b35c62e..1349273 100644 --- a/internal/verda-cli/cmd/vm/batch_test.go +++ b/internal/verda-cli/cmd/vm/batch_test.go @@ -462,7 +462,7 @@ func TestActionNameToAPI(t *testing.T) { }{ {"start", verda.ActionStart}, {"shutdown", verda.ActionShutdown}, - {"hibernate", verda.ActionHibernate}, + {"hibernate", verda.ActionHibernate}, //nolint:staticcheck // SA1019: hibernate is deprecated but still supported {"delete", verda.ActionDelete}, {"force_shutdown", verda.ActionForceShutdown}, {"force-shutdown", verda.ActionForceShutdown}, diff --git a/internal/verda-cli/cmd/vm/instances.go b/internal/verda-cli/cmd/vm/instances.go index 3768a38..c3cddfd 100644 --- a/internal/verda-cli/cmd/vm/instances.go +++ b/internal/verda-cli/cmd/vm/instances.go @@ -31,7 +31,7 @@ func fetchInstances(ctx context.Context, f cmdutil.Factory, client *verda.Client } instances, err := cmdutil.WithSpinner(ctx, f.Status(), "Loading instances...", func(ctx context.Context) ([]verda.Instance, error) { - return client.Instances.Get(ctx, apiStatus) + return client.Instances.List(ctx, &verda.ListInstancesOptions{Status: apiStatus}) }) if err != nil { return nil, err diff --git a/internal/verda-cli/cmd/vm/list.go b/internal/verda-cli/cmd/vm/list.go index d04709b..890995d 100644 --- a/internal/verda-cli/cmd/vm/list.go +++ b/internal/verda-cli/cmd/vm/list.go @@ -71,7 +71,7 @@ func runList(cmd *cobra.Command, f cmdutil.Factory, ioStreams cmdutil.IOStreams, defer cancel() instances, err := cmdutil.WithSpinner(ctx, f.Status(), "Loading instances...", func(ctx context.Context) ([]verda.Instance, error) { - return client.Instances.Get(ctx, opts.Status) + return client.Instances.List(ctx, &verda.ListInstancesOptions{Status: opts.Status}) }) if err != nil { return err diff --git a/internal/verda-cli/cmd/vm/shortcuts.go b/internal/verda-cli/cmd/vm/shortcuts.go index b53a54b..2b4a0e4 100644 --- a/internal/verda-cli/cmd/vm/shortcuts.go +++ b/internal/verda-cli/cmd/vm/shortcuts.go @@ -46,7 +46,7 @@ var shortcuts = []shortcutDef{ { Use: "hibernate ", Short: "Hibernate a VM instance (deprecated, will be removed in a future version)", - Action: verda.ActionHibernate, + Action: verda.ActionHibernate, //nolint:staticcheck // SA1019: hibernate is deprecated but still supported }, { Use: "delete ", diff --git a/internal/verda-cli/cmd/vm/wizard.go b/internal/verda-cli/cmd/vm/wizard.go index 66347d4..7fc651a 100644 --- a/internal/verda-cli/cmd/vm/wizard.go +++ b/internal/verda-cli/cmd/vm/wizard.go @@ -588,7 +588,8 @@ func stepStorage(getClient clientFunc, cache *apiCache, opts *createOptions) wiz default: // "done" // Write the collected volumes to opts. specs := make([]string, len(volumes)) - for i, v := range volumes { + for i := range volumes { + v := &volumes[i] specs[i] = fmt.Sprintf("%s:%d:%s", v.Name, v.Size, v.Type) } opts.VolumeSpecs = specs diff --git a/internal/verda-cli/cmd/vm/wizard_subflows.go b/internal/verda-cli/cmd/vm/wizard_subflows.go index feae5cb..9065a8a 100644 --- a/internal/verda-cli/cmd/vm/wizard_subflows.go +++ b/internal/verda-cli/cmd/vm/wizard_subflows.go @@ -305,7 +305,8 @@ func buildStorageChoices(volumes []verda.VolumeCreateRequest, existingIDs []stri // Show already-added volumes. if hasVolumes { - for _, v := range volumes { + for i := range volumes { + v := &volumes[i] choices = append(choices, wizard.Choice{ Label: fmt.Sprintf(" New: %s (%dGB %s)", v.Name, v.Size, v.Type), Value: "__info__", From 406536216b23e8e851a9c4549573b1331427ddf2 Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Wed, 28 Oct 2026 15:32:29 +0200 Subject: [PATCH 02/11] refactor: apply go fix modernizers Output of `go fix -tags=integration ./...` (Go 1.26.10, two passes): new(expr), min/max clamps, strings.Cut/CutPrefix/CutSuffix/SplitSeq, slices.Contains, wg.Go, range over int. No behavior changes. Manual edits: - skills/uninstall.go: the stringscut fixer emitted `before, _, ok := ...; _, after, ok := ...; if !ok || !ok`, which does not compile and would drop the start-marker check. Use okStart/okEnd. - Delete strPtr/stringPtr/ptrOf, unused once their call sites were inlined as new(...). - Preallocate two test slices that prealloc flags after range-over-int. - Reflow three min/max/Cut rewrites that go fix left awkwardly formatted. Co-Authored-By: Claude Opus 5.5 --- internal/verda-cli/cmd/cost/running_test.go | 4 +--- internal/verda-cli/cmd/mcp/lazy_init_test.go | 8 +++---- internal/verda-cli/cmd/mcp/server.go | 7 +++--- .../verda-cli/cmd/objectstorage/mpdownload.go | 6 ++--- .../verda-cli/cmd/objectstorage/mpupload.go | 16 ++++--------- .../cmd/objectstorage/mpupload_test.go | 3 ++- .../cmd/objectstorage/resume_uploads_test.go | 4 ++-- internal/verda-cli/cmd/objectstorage/rm.go | 5 +--- .../verda-cli/cmd/objectstorage/transfer.go | 4 ++-- internal/verda-cli/cmd/registry/copy.go | 23 +++++-------------- internal/verda-cli/cmd/registry/delete.go | 7 +++--- .../verda-cli/cmd/registry/harbor_test.go | 12 +++++----- .../verda-cli/cmd/registry/progress_test.go | 2 +- internal/verda-cli/cmd/registry/push.go | 4 ++-- .../verda-cli/cmd/registry/push_picker.go | 10 ++------ .../cmd/registry/push_picker_test.go | 9 ++------ internal/verda-cli/cmd/registry/push_view.go | 7 ++---- internal/verda-cli/cmd/registry/refname.go | 5 ++-- .../verda-cli/cmd/registry/registry_test.go | 8 ++----- .../verda-cli/cmd/registry/retry_transport.go | 10 ++------ .../cmd/registry/retry_transport_test.go | 2 +- internal/verda-cli/cmd/registry/tags_test.go | 2 +- internal/verda-cli/cmd/skills/uninstall.go | 10 ++++---- internal/verda-cli/cmd/ssh/ssh_test.go | 20 ++++++++-------- internal/verda-cli/cmd/update/verify.go | 4 ++-- internal/verda-cli/cmd/util/command_groups.go | 7 +++--- internal/verda-cli/cmd/util/views_test.go | 8 +++---- internal/verda-cli/cmd/vm/action.go | 8 +++---- internal/verda-cli/cmd/vm/create.go | 8 ++----- internal/verda-cli/cmd/vm/instances.go | 8 +++---- internal/verda-cli/cmd/vm/template_apply.go | 8 ++----- .../verda-cli/cmd/volume/describe_test.go | 4 +--- internal/verda-cli/options/options.go | 7 +++--- pkg/log/context.go | 5 ++-- pkg/tui/bubbletea/list_helpers.go | 5 +--- pkg/tui/wizard/view_hintbar.go | 2 +- tests/contract/mockapi/mockapi.go | 6 ++--- 37 files changed, 95 insertions(+), 173 deletions(-) diff --git a/internal/verda-cli/cmd/cost/running_test.go b/internal/verda-cli/cmd/cost/running_test.go index d5b375a..b6eca1b 100644 --- a/internal/verda-cli/cmd/cost/running_test.go +++ b/internal/verda-cli/cmd/cost/running_test.go @@ -23,13 +23,11 @@ import ( cmdutil "github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/util" ) -func strPtr(s string) *string { return &s } - func TestUniqueVolumeIDs(t *testing.T) { t.Parallel() inst := &verda.Instance{ - OSVolumeID: strPtr("vol-os"), + OSVolumeID: new("vol-os"), VolumeIDs: []string{"vol-os", "vol-data", "vol-data"}, } diff --git a/internal/verda-cli/cmd/mcp/lazy_init_test.go b/internal/verda-cli/cmd/mcp/lazy_init_test.go index 139f608..b79cf41 100644 --- a/internal/verda-cli/cmd/mcp/lazy_init_test.go +++ b/internal/verda-cli/cmd/mcp/lazy_init_test.go @@ -51,10 +51,8 @@ func TestLazyClientInitConcurrent(t *testing.T) { const workers = 16 var wg sync.WaitGroup errs := make(chan error, workers) - for i := 0; i < workers; i++ { - wg.Add(1) - go func() { - defer wg.Done() + for range workers { + wg.Go(func() { c, err := s.verdaClient() if err != nil { errs <- err @@ -63,7 +61,7 @@ func TestLazyClientInitConcurrent(t *testing.T) { if c != client { errs <- errors.New("verdaClient returned the wrong client instance") } - }() + }) } wg.Wait() close(errs) diff --git a/internal/verda-cli/cmd/mcp/server.go b/internal/verda-cli/cmd/mcp/server.go index 6c54929..77de99f 100644 --- a/internal/verda-cli/cmd/mcp/server.go +++ b/internal/verda-cli/cmd/mcp/server.go @@ -19,6 +19,7 @@ import ( "encoding/json" "fmt" "os" + "slices" "strings" "sync" @@ -288,10 +289,8 @@ func optionalEnum(a map[string]any, name string, allowed ...string) (string, err if err != nil || s == "" { return "", err } - for _, allow := range allowed { - if s == allow { - return s, nil - } + if slices.Contains(allowed, s) { + return s, nil } return "", invalidArgError(name, fmt.Sprintf("invalid value %q (valid: %s)", s, strings.Join(allowed, ", "))) } diff --git a/internal/verda-cli/cmd/objectstorage/mpdownload.go b/internal/verda-cli/cmd/objectstorage/mpdownload.go index adb9719..b6408da 100644 --- a/internal/verda-cli/cmd/objectstorage/mpdownload.go +++ b/internal/verda-cli/cmd/objectstorage/mpdownload.go @@ -297,9 +297,7 @@ func downloadMissingChunks(ctx context.Context, client API, opts *resumableDownl results := make(chan downloadChunkResult) var wg sync.WaitGroup for range concurrency { - wg.Add(1) - go func() { - defer wg.Done() + wg.Go(func() { for n := range jobs { err := downloadOneChunk(ctx, client, opts.Bucket, opts.Key, etag, file, n, total, partSize) select { @@ -308,7 +306,7 @@ func downloadMissingChunks(ctx context.Context, client API, opts *resumableDownl return } } - }() + }) } go func() { defer close(jobs) diff --git a/internal/verda-cli/cmd/objectstorage/mpupload.go b/internal/verda-cli/cmd/objectstorage/mpupload.go index 8fe747e..cd796bc 100644 --- a/internal/verda-cli/cmd/objectstorage/mpupload.go +++ b/internal/verda-cli/cmd/objectstorage/mpupload.go @@ -60,10 +60,7 @@ type resumableOptions struct { // splits into at most maxParts parts. A requested size below the floor or that // would exceed maxParts is bumped to the smallest valid value. func computePartSize(fileSize, requested int64) int64 { - size := requested - if size < minPartSize { - size = minPartSize - } + size := max(requested, minPartSize) // Double until the file splits into at most maxParts parts. Ceil division: // a file that is an exact multiple of size*maxParts needs exactly maxParts // parts (allowed) and must NOT be bumped — the old `fileSize/size+1` form @@ -96,10 +93,7 @@ func numParts(fileSize, partSize int64) int32 { // (1-indexed) of a fileSize-byte file at partSize. The last part is short. func partRange(n int32, fileSize, partSize int64) (start, end int64) { start = int64(n-1) * partSize - end = start + partSize - if end > fileSize { - end = fileSize - } + end = min(start+partSize, fileSize) return start, end } @@ -315,9 +309,7 @@ func uploadMissingParts(ctx context.Context, client API, opts *resumableOptions, results := make(chan uploadPartResult) var wg sync.WaitGroup for range concurrency { - wg.Add(1) - go func() { - defer wg.Done() + wg.Go(func() { for n := range jobs { etag, err := uploadOnePart(ctx, client, opts, cp.UploadID, n, partSize) select { @@ -326,7 +318,7 @@ func uploadMissingParts(ctx context.Context, client API, opts *resumableOptions, return } } - }() + }) } go func() { diff --git a/internal/verda-cli/cmd/objectstorage/mpupload_test.go b/internal/verda-cli/cmd/objectstorage/mpupload_test.go index b4a9d1b..377b7e3 100644 --- a/internal/verda-cli/cmd/objectstorage/mpupload_test.go +++ b/internal/verda-cli/cmd/objectstorage/mpupload_test.go @@ -19,6 +19,7 @@ import ( "errors" "os" "path/filepath" + "slices" "sort" "strconv" "sync" @@ -299,7 +300,7 @@ func TestResumableUpload_Resume_OnlyMissingParts(t *testing.T) { if fake.uploadCalls != 2 { t.Errorf("resume UploadPart calls = %d, want 2 (only missing 3,4)", fake.uploadCalls) } - sort.Slice(fake.uploadOrder, func(i, j int) bool { return fake.uploadOrder[i] < fake.uploadOrder[j] }) + slices.Sort(fake.uploadOrder) if len(fake.uploadOrder) != 2 || fake.uploadOrder[0] != 3 || fake.uploadOrder[1] != 4 { t.Errorf("resumed parts = %v, want [3 4]", fake.uploadOrder) } diff --git a/internal/verda-cli/cmd/objectstorage/resume_uploads_test.go b/internal/verda-cli/cmd/objectstorage/resume_uploads_test.go index 0be9d93..88f0cf0 100644 --- a/internal/verda-cli/cmd/objectstorage/resume_uploads_test.go +++ b/internal/verda-cli/cmd/objectstorage/resume_uploads_test.go @@ -19,7 +19,7 @@ package objectstorage import ( "bytes" "context" - "sort" + "slices" "strings" "sync" "testing" @@ -132,7 +132,7 @@ func TestResumeServerUpload(t *testing.T) { if fake.createCalls != 0 { t.Errorf("CreateMultipartUpload called %d times, want 0 (must adopt the existing UploadId)", fake.createCalls) } - sort.Slice(fake.uploaded, func(i, j int) bool { return fake.uploaded[i] < fake.uploaded[j] }) + slices.Sort(fake.uploaded) if len(fake.uploaded) != 2 || fake.uploaded[0] != 3 || fake.uploaded[1] != 4 { t.Errorf("uploaded parts = %v, want [3 4] (only the missing ones)", fake.uploaded) } diff --git a/internal/verda-cli/cmd/objectstorage/rm.go b/internal/verda-cli/cmd/objectstorage/rm.go index da97ab7..790dd96 100644 --- a/internal/verda-cli/cmd/objectstorage/rm.go +++ b/internal/verda-cli/cmd/objectstorage/rm.go @@ -398,10 +398,7 @@ func deleteSingle(ctx context.Context, f cmdutil.Factory, ioStreams cmdutil.IOSt // abort the command with the original error wrapped via translateError. func deleteBatched(ctx context.Context, f cmdutil.Factory, ioStreams cmdutil.IOStreams, client API, bucket string, keys []string, payload *rmPayload) error { for start := 0; start < len(keys); start += maxDeleteBatch { - end := start + maxDeleteBatch - if end > len(keys) { - end = len(keys) - } + end := min(start+maxDeleteBatch, len(keys)) chunk := keys[start:end] objs := make([]s3types.ObjectIdentifier, 0, len(chunk)) diff --git a/internal/verda-cli/cmd/objectstorage/transfer.go b/internal/verda-cli/cmd/objectstorage/transfer.go index 4ed1bfb..ab5fcd7 100644 --- a/internal/verda-cli/cmd/objectstorage/transfer.go +++ b/internal/verda-cli/cmd/objectstorage/transfer.go @@ -167,8 +167,8 @@ func parseOlderThan(s string) (time.Duration, error) { if s == "" { return 0, nil } - if strings.HasSuffix(s, "d") { - days, err := strconv.ParseFloat(strings.TrimSuffix(s, "d"), 64) + if before, ok := strings.CutSuffix(s, "d"); ok { + days, err := strconv.ParseFloat(before, 64) if err != nil || days < 0 { return 0, fmt.Errorf("invalid duration %q", s) } diff --git a/internal/verda-cli/cmd/registry/copy.go b/internal/verda-cli/cmd/registry/copy.go index 9cb71df..23f8ab0 100644 --- a/internal/verda-cli/cmd/registry/copy.go +++ b/internal/verda-cli/cmd/registry/copy.go @@ -931,14 +931,8 @@ type copyJobResult struct { // capped at 4. The cap keeps a 32-core laptop from DoSing a small // registry while still giving slow links a useful speedup. func resolveImageJobs(userValue, tagCount, hwConcurrency int) int { - hw := hwConcurrency - if hw < 1 { - hw = 1 - } - maxAllowed := hw - if maxAllowed > 8 { - maxAllowed = 8 - } + hw := max(hwConcurrency, 1) + maxAllowed := min(hw, 8) if userValue > 0 { if userValue > maxAllowed { return maxAllowed @@ -948,10 +942,7 @@ func resolveImageJobs(userValue, tagCount, hwConcurrency int) int { if tagCount < 4 { return 1 } - half := hw / 2 - if half < 1 { - half = 1 - } + half := max(hw/2, 1) if half > 4 { return 4 } @@ -1289,15 +1280,13 @@ func runCopyAllTagsPool( resultsCh := make(chan copyJobResult, len(tags)) var wg sync.WaitGroup - for w := 0; w < imageJobs; w++ { - wg.Add(1) - go func() { - defer wg.Done() + for range imageJobs { + wg.Go(func() { for j := range jobs { r := copyOneTag(ctx, program, srcReg, dstReg, srcBase, dstBase, j, opts, creds) resultsCh <- r } - }() + }) } // Producer: fans tags into jobs, closes the channel once exhausted. diff --git a/internal/verda-cli/cmd/registry/delete.go b/internal/verda-cli/cmd/registry/delete.go index e38d3bb..420bff7 100644 --- a/internal/verda-cli/cmd/registry/delete.go +++ b/internal/verda-cli/cmd/registry/delete.go @@ -17,6 +17,7 @@ package registry import ( "context" "fmt" + "slices" "strings" "charm.land/lipgloss/v2" @@ -408,10 +409,8 @@ func lookupArtifact(ctx context.Context, lister RepositoryLister, projectName, r if isDigest && strings.HasPrefix(arts[i].Digest, reference) { return &arts[i] } - for _, t := range arts[i].Tags { - if t == reference { - return &arts[i] - } + if slices.Contains(arts[i].Tags, reference) { + return &arts[i] } } return nil diff --git a/internal/verda-cli/cmd/registry/harbor_test.go b/internal/verda-cli/cmd/registry/harbor_test.go index 28421c8..2f1e90c 100644 --- a/internal/verda-cli/cmd/registry/harbor_test.go +++ b/internal/verda-cli/cmd/registry/harbor_test.go @@ -246,8 +246,8 @@ func (b *serverBehavior) handleRepositories(w http.ResponseWriter, r *http.Reque } q := r.URL.Query().Get("q") wantPID := -1 - if strings.HasPrefix(q, "project_id=") { - if n, err := strconv.Atoi(strings.TrimPrefix(q, "project_id=")); err == nil { + if after, ok := strings.CutPrefix(q, "project_id="); ok { + if n, err := strconv.Atoi(after); err == nil { wantPID = n } } @@ -392,8 +392,8 @@ func TestHarbor_ListRepositories_SiblingLeakDefended(t *testing.T) { // Page size is harborListerPageSize (100); we seed 101 rows so the loop // issues at least two GETs and terminates on the short page. func TestHarbor_ListRepositories_Paging(t *testing.T) { - var seeded []harborRepository - for i := 0; i < harborListerPageSize+1; i++ { + seeded := make([]harborRepository, 0, harborListerPageSize+1) + for i := range harborListerPageSize + 1 { seeded = append(seeded, harborRepository{ Name: fmt.Sprintf("abc/repo-%03d", i), ProjectID: 42, @@ -618,8 +618,8 @@ func TestHarbor_ListArtifacts_URLEscapesRepoSlash(t *testing.T) { // TestHarbor_ListArtifacts_Paging: seed > page-size and assert the // client issues multiple GETs and terminates on the short page. func TestHarbor_ListArtifacts_Paging(t *testing.T) { - var seeded []harborArtifact - for i := 0; i < harborListerPageSize+1; i++ { + seeded := make([]harborArtifact, 0, harborListerPageSize+1) + for i := range harborListerPageSize + 1 { seeded = append(seeded, harborArtifact{ Digest: fmt.Sprintf("sha256:%064d", i), Size: int64(i), diff --git a/internal/verda-cli/cmd/registry/progress_test.go b/internal/verda-cli/cmd/registry/progress_test.go index 734f877..2296eb3 100644 --- a/internal/verda-cli/cmd/registry/progress_test.go +++ b/internal/verda-cli/cmd/registry/progress_test.go @@ -226,7 +226,7 @@ func TestMeter_SampleCapEnforced(t *testing.T) { times[i] = base.Add(time.Duration(i) * time.Millisecond) } m := Meter{now: fakeClock(times...)} - for i := 0; i < 200; i++ { + for i := range 200 { m.Update(int64(i*100), 1_000_000) } if got := len(m.samples); got > progressSampleCap { diff --git a/internal/verda-cli/cmd/registry/push.go b/internal/verda-cli/cmd/registry/push.go index 827bf59..9868087 100644 --- a/internal/verda-cli/cmd/registry/push.go +++ b/internal/verda-cli/cmd/registry/push.go @@ -837,8 +837,8 @@ func defaultRepoFromPath(p string) string { return "" } for _, ext := range []string{".tar.gz", ".tgz", ".tar", ".oci"} { - if strings.HasSuffix(base, ext) { - return strings.TrimSuffix(base, ext) + if before, ok := strings.CutSuffix(base, ext); ok { + return before } } return base diff --git a/internal/verda-cli/cmd/registry/push_picker.go b/internal/verda-cli/cmd/registry/push_picker.go index a0777b0..8c1bf2f 100644 --- a/internal/verda-cli/cmd/registry/push_picker.go +++ b/internal/verda-cli/cmd/registry/push_picker.go @@ -512,16 +512,10 @@ func formatAgo(t time.Time, now func() time.Time) string { } // Use 30-day months for rough-but-stable output. A year is 365 days. if d < 365*24*time.Hour { - n := int(d / (30 * 24 * time.Hour)) - if n < 1 { - n = 1 - } + n := max(int(d/(30*24*time.Hour)), 1) return pluralAgo(n, "month") } - n := int(d / (365 * 24 * time.Hour)) - if n < 1 { - n = 1 - } + n := max(int(d/(365*24*time.Hour)), 1) return pluralAgo(n, "year") } diff --git a/internal/verda-cli/cmd/registry/push_picker_test.go b/internal/verda-cli/cmd/registry/push_picker_test.go index cfc6d2f..a360ec0 100644 --- a/internal/verda-cli/cmd/registry/push_picker_test.go +++ b/internal/verda-cli/cmd/registry/push_picker_test.go @@ -15,6 +15,7 @@ package registry import ( + "slices" "strings" "testing" "time" @@ -77,13 +78,7 @@ func TestPicker_Expansion(t *testing.T) { refs[i] = it.Ref } for _, want := range []string{"my-app:v1", "my-app:latest", "nginx:latest"} { - found := false - for _, got := range refs { - if got == want { - found = true - break - } - } + found := slices.Contains(refs, want) if !found { t.Errorf("missing ref %q in picker rows: %v", want, refs) } diff --git a/internal/verda-cli/cmd/registry/push_view.go b/internal/verda-cli/cmd/registry/push_view.go index 18f675e..02d623a 100644 --- a/internal/verda-cli/cmd/registry/push_view.go +++ b/internal/verda-cli/cmd/registry/push_view.go @@ -343,13 +343,10 @@ func renderProgressBar(fraction float64, width int) string { if fraction > 1 { fraction = 1 } - filled := int(float64(width)*fraction + 0.0001) // tiny epsilon so 1.0 -> width - if filled > width { - filled = width - } + filled := min(int(float64(width)*fraction+0.0001), width) // tiny epsilon so 1.0 -> width var b strings.Builder b.Grow(width * len(pushBarFilled)) // approximate - for i := 0; i < filled; i++ { + for range filled { b.WriteString(pushBarFilled) } for i := filled; i < width; i++ { diff --git a/internal/verda-cli/cmd/registry/refname.go b/internal/verda-cli/cmd/registry/refname.go index fb0c8fd..ae4a8d8 100644 --- a/internal/verda-cli/cmd/registry/refname.go +++ b/internal/verda-cli/cmd/registry/refname.go @@ -143,11 +143,10 @@ func hasProjectNamespace(host string) bool { // We only inspect the first segment; trailing tag/digest colons never appear // before the first "/" in a valid reference, so no pre-stripping is needed. func isShortRef(raw string) bool { - slash := strings.IndexByte(raw, '/') - if slash < 0 { + first, _, ok := strings.Cut(raw, "/") + if !ok { return true } - first := raw[:slash] if first == localhostHost { return false } diff --git a/internal/verda-cli/cmd/registry/registry_test.go b/internal/verda-cli/cmd/registry/registry_test.go index d3c89cf..f418f29 100644 --- a/internal/verda-cli/cmd/registry/registry_test.go +++ b/internal/verda-cli/cmd/registry/registry_test.go @@ -16,6 +16,7 @@ package registry import ( "bytes" + "slices" "testing" cmdutil "github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/util" @@ -45,10 +46,5 @@ func TestNewCmdRegistry_BasicWiring(t *testing.T) { } func sliceContains(s []string, v string) bool { - for _, x := range s { - if x == v { - return true - } - } - return false + return slices.Contains(s, v) } diff --git a/internal/verda-cli/cmd/registry/retry_transport.go b/internal/verda-cli/cmd/registry/retry_transport.go index babbbe9..85f0102 100644 --- a/internal/verda-cli/cmd/registry/retry_transport.go +++ b/internal/verda-cli/cmd/registry/retry_transport.go @@ -227,14 +227,8 @@ func (t *retryTransport) classify(resp *http.Response, err error) (bool, time.Du func (t *retryTransport) backoff(attempt int) time.Duration { // exp = BaseDelay * 2^(attempt-1). Cap at MaxInterval before // adding jitter so the pre-jitter component is predictable. - shift := attempt - 1 - if shift < 0 { - shift = 0 - } - // Guard against overflow on absurd MaxAttempts values. - if shift > 20 { - shift = 20 - } + // Cap at 20 to guard against overflow on absurd MaxAttempts values. + shift := min(max(attempt-1, 0), 20) exp := t.cfg.BaseDelay << shift if exp <= 0 || exp > t.cfg.MaxInterval { exp = t.cfg.MaxInterval diff --git a/internal/verda-cli/cmd/registry/retry_transport_test.go b/internal/verda-cli/cmd/registry/retry_transport_test.go index 26b8c3a..e0cba6f 100644 --- a/internal/verda-cli/cmd/registry/retry_transport_test.go +++ b/internal/verda-cli/cmd/registry/retry_transport_test.go @@ -485,7 +485,7 @@ func TestRetryingTransport_BodyRewoundOnRetry(t *testing.T) { t.Fatalf("request count = %d, want 2", got) } // Both captured bodies must match the payload exactly. - for i := 0; i < 2; i++ { + for i := range 2 { body := cs.bodyAt(i) if !bytes.Equal(body, payload) { t.Fatalf("body[%d] = %q, want %q", i, string(body), string(payload)) diff --git a/internal/verda-cli/cmd/registry/tags_test.go b/internal/verda-cli/cmd/registry/tags_test.go index 5f43b28..d2cc620 100644 --- a/internal/verda-cli/cmd/registry/tags_test.go +++ b/internal/verda-cli/cmd/registry/tags_test.go @@ -301,7 +301,7 @@ func TestTags_LimitCapsMetadata(t *testing.T) { } // First 2 rows: HEAD was called → Digest set, Size > 0. - for i := 0; i < 2; i++ { + for i := range 2 { if payload.Tags[i].Digest == "" { t.Errorf("row %d (%s): Digest empty, want populated (should be looked up)", i, payload.Tags[i].Tag) diff --git a/internal/verda-cli/cmd/skills/uninstall.go b/internal/verda-cli/cmd/skills/uninstall.go index 76285a6..2f7a66d 100644 --- a/internal/verda-cli/cmd/skills/uninstall.go +++ b/internal/verda-cli/cmd/skills/uninstall.go @@ -258,14 +258,14 @@ func uninstallAppend(agent *Agent) error { } content := string(data) - startIdx := strings.Index(content, markerStart) - endIdx := strings.Index(content, markerEnd) - if startIdx < 0 || endIdx < 0 { + before, _, okStart := strings.Cut(content, markerStart) + _, after, okEnd := strings.Cut(content, markerEnd) + if !okStart || !okEnd { return nil } - before := strings.TrimRight(content[:startIdx], "\n") - after := strings.TrimLeft(content[endIdx+len(markerEnd):], "\n") + before = strings.TrimRight(before, "\n") + after = strings.TrimLeft(after, "\n") var result string switch { diff --git a/internal/verda-cli/cmd/ssh/ssh_test.go b/internal/verda-cli/cmd/ssh/ssh_test.go index acd49ee..3ba331f 100644 --- a/internal/verda-cli/cmd/ssh/ssh_test.go +++ b/internal/verda-cli/cmd/ssh/ssh_test.go @@ -26,8 +26,6 @@ import ( cmdutil "github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/util" ) -func strPtr(s string) *string { return &s } - func TestIsTerminalReturnsFalseForPipe(t *testing.T) { t.Parallel() @@ -47,8 +45,8 @@ func TestResolveInstanceByID(t *testing.T) { t.Parallel() instances := []verda.Instance{ - {ID: "abc-123", Hostname: "gpu-runner", Status: verda.StatusRunning, IP: strPtr("1.2.3.4")}, - {ID: "def-456", Hostname: "cpu-worker", Status: verda.StatusOffline, IP: strPtr("5.6.7.8")}, + {ID: "abc-123", Hostname: "gpu-runner", Status: verda.StatusRunning, IP: new("1.2.3.4")}, + {ID: "def-456", Hostname: "cpu-worker", Status: verda.StatusOffline, IP: new("5.6.7.8")}, } inst := resolveInstance(instances, "def-456") @@ -64,7 +62,7 @@ func TestResolveInstanceByHostname(t *testing.T) { t.Parallel() instances := []verda.Instance{ - {ID: "abc-123", Hostname: "gpu-runner", Status: verda.StatusRunning, IP: strPtr("1.2.3.4")}, + {ID: "abc-123", Hostname: "gpu-runner", Status: verda.StatusRunning, IP: new("1.2.3.4")}, } inst := resolveInstance(instances, "gpu-runner") @@ -80,7 +78,7 @@ func TestResolveInstanceByHostnameCaseInsensitive(t *testing.T) { t.Parallel() instances := []verda.Instance{ - {ID: "abc-123", Hostname: "GPU-Runner", Status: verda.StatusRunning, IP: strPtr("1.2.3.4")}, + {ID: "abc-123", Hostname: "GPU-Runner", Status: verda.StatusRunning, IP: new("1.2.3.4")}, } inst := resolveInstance(instances, "gpu-runner") @@ -93,8 +91,8 @@ func TestResolveInstanceIDTakesPrecedence(t *testing.T) { t.Parallel() instances := []verda.Instance{ - {ID: "match-me", Hostname: "first", Status: verda.StatusRunning, IP: strPtr("1.1.1.1")}, - {ID: "other", Hostname: "match-me", Status: verda.StatusRunning, IP: strPtr("2.2.2.2")}, + {ID: "match-me", Hostname: "first", Status: verda.StatusRunning, IP: new("1.1.1.1")}, + {ID: "other", Hostname: "match-me", Status: verda.StatusRunning, IP: new("2.2.2.2")}, } inst := resolveInstance(instances, "match-me") @@ -132,9 +130,9 @@ func TestFilterRunningInstances(t *testing.T) { t.Parallel() instances := []verda.Instance{ - {ID: "1", Hostname: "running-vm", Status: verda.StatusRunning, IP: strPtr("1.1.1.1")}, - {ID: "2", Hostname: "offline-vm", Status: verda.StatusOffline, IP: strPtr("2.2.2.2")}, - {ID: "3", Hostname: "another-running", Status: verda.StatusRunning, IP: strPtr("3.3.3.3")}, + {ID: "1", Hostname: "running-vm", Status: verda.StatusRunning, IP: new("1.1.1.1")}, + {ID: "2", Hostname: "offline-vm", Status: verda.StatusOffline, IP: new("2.2.2.2")}, + {ID: "3", Hostname: "another-running", Status: verda.StatusRunning, IP: new("3.3.3.3")}, {ID: "4", Hostname: "provisioning-vm", Status: verda.StatusProvisioning}, } diff --git a/internal/verda-cli/cmd/update/verify.go b/internal/verda-cli/cmd/update/verify.go index eebfdd0..82f4545 100644 --- a/internal/verda-cli/cmd/update/verify.go +++ b/internal/verda-cli/cmd/update/verify.go @@ -111,7 +111,7 @@ func findMatchingChecksum(body, goos, goarch string) (string, error) { // Match "verda__" followed by "/" or "_" (variant suffix). prefix := fmt.Sprintf("verda_%s_%s", goos, goarch) - for _, line := range strings.Split(body, "\n") { + for line := range strings.SplitSeq(body, "\n") { hexStr, key, ok := parseChecksumLine(line) if !ok { continue @@ -133,7 +133,7 @@ func findMatchingChecksum(body, goos, goarch string) (string, error) { // file (verda__SHA256SUMS). Unlike findMatchingChecksum (dist-dir keys // with variant suffixes), archive keys match exactly. func findArchiveChecksum(body, artifactName string) (string, error) { - for _, line := range strings.Split(body, "\n") { + for line := range strings.SplitSeq(body, "\n") { hexStr, key, ok := parseChecksumLine(line) if !ok { continue diff --git a/internal/verda-cli/cmd/util/command_groups.go b/internal/verda-cli/cmd/util/command_groups.go index 62bb293..26d7847 100644 --- a/internal/verda-cli/cmd/util/command_groups.go +++ b/internal/verda-cli/cmd/util/command_groups.go @@ -16,6 +16,7 @@ package util import ( "fmt" + "slices" "strings" "github.com/spf13/cobra" @@ -44,10 +45,8 @@ func (g CommandGroups) Add(c *cobra.Command) { // Has returns true if c belongs to any group. func (g CommandGroups) Has(c *cobra.Command) bool { for _, group := range g { - for _, cmd := range group.Commands { - if cmd == c { - return true - } + if slices.Contains(group.Commands, c) { + return true } } return false diff --git a/internal/verda-cli/cmd/util/views_test.go b/internal/verda-cli/cmd/util/views_test.go index 8af383b..5dc7cbc 100644 --- a/internal/verda-cli/cmd/util/views_test.go +++ b/internal/verda-cli/cmd/util/views_test.go @@ -188,8 +188,8 @@ func jsonKeys(t *testing.T, v any) map[string]bool { rt := reflect.TypeOf(v) keys := make(map[string]bool, rt.NumField()) - for i := range rt.NumField() { - tag := rt.Field(i).Tag.Get("json") + for field := range rt.Fields() { + tag := field.Tag.Get("json") if tag == "" || tag == "-" { continue } @@ -337,8 +337,8 @@ func fillNonZero(t *testing.T, v reflect.Value, n *int) { v.Set(reflect.ValueOf(time.Date(2026, 8, 11, 18, 51, 12, 0, time.UTC))) return } - for i := range v.NumField() { - if f := v.Field(i); f.CanSet() { + for _, f := range v.Fields() { + if f.CanSet() { fillNonZero(t, f, n) } } diff --git a/internal/verda-cli/cmd/vm/action.go b/internal/verda-cli/cmd/vm/action.go index 931d42e..b05003f 100644 --- a/internal/verda-cli/cmd/vm/action.go +++ b/internal/verda-cli/cmd/vm/action.go @@ -17,6 +17,7 @@ package vm import ( "context" "fmt" + "slices" "strings" "charm.land/lipgloss/v2" @@ -100,11 +101,8 @@ func availableActions(status string) []instanceAction { result = append(result, a) continue } - for _, s := range a.ValidFrom { - if s == status { - result = append(result, a) - break - } + if slices.Contains(a.ValidFrom, status) { + result = append(result, a) } } return result diff --git a/internal/verda-cli/cmd/vm/create.go b/internal/verda-cli/cmd/vm/create.go index 6345ff3..dcad13f 100644 --- a/internal/verda-cli/cmd/vm/create.go +++ b/internal/verda-cli/cmd/vm/create.go @@ -342,10 +342,10 @@ func (o *createOptions) request() (verda.CreateInstanceRequest, error) { } if o.StartupScriptID != "" { - req.StartupScriptID = stringPtr(o.StartupScriptID) + req.StartupScriptID = new(o.StartupScriptID) } if o.Coupon != "" { - req.Coupon = stringPtr(o.Coupon) + req.Coupon = new(o.Coupon) } if o.OSVolumeName != "" || o.OSVolumeSize > 0 || o.OSVolumeOnSpotDiscontinue != "" { if o.OSVolumeSize <= 0 { @@ -501,7 +501,3 @@ func appendStorageVolume(volumes []verda.VolumeCreateRequest, o *createOptions) return append(volumes, volume), nil } - -func stringPtr(v string) *string { - return &v -} diff --git a/internal/verda-cli/cmd/vm/instances.go b/internal/verda-cli/cmd/vm/instances.go index c3cddfd..cc2ea49 100644 --- a/internal/verda-cli/cmd/vm/instances.go +++ b/internal/verda-cli/cmd/vm/instances.go @@ -16,6 +16,7 @@ package vm import ( "context" + "slices" "github.com/verda-cloud/verdacloud-sdk-go/pkg/verda" @@ -48,11 +49,8 @@ func fetchInstances(ctx context.Context, f cmdutil.Factory, client *verda.Client func filterByStatus(instances []verda.Instance, statuses []string) []verda.Instance { filtered := make([]verda.Instance, 0, len(instances)) for i := range instances { - for _, s := range statuses { - if instances[i].Status == s { //nolint:gosec // i is bounded by range - filtered = append(filtered, instances[i]) //nolint:gosec // i is bounded by range - break - } + if slices.Contains(statuses, instances[i].Status) { //nolint:gosec // i is bounded by range + filtered = append(filtered, instances[i]) } } return filtered diff --git a/internal/verda-cli/cmd/vm/template_apply.go b/internal/verda-cli/cmd/vm/template_apply.go index 7d170e1..4873af6 100644 --- a/internal/verda-cli/cmd/vm/template_apply.go +++ b/internal/verda-cli/cmd/vm/template_apply.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "path/filepath" + "slices" "strings" "github.com/spf13/cobra" @@ -185,12 +186,7 @@ func applyTemplate(tmpl *template.Template, opts *createOptions, changed func(st // anyFlagChanged reports whether any of the named flags was explicitly set // (aliases are separate names on the same flag set). func anyFlagChanged(changed func(string) bool, names ...string) bool { - for _, n := range names { - if changed(n) { - return true - } - } - return false + return slices.ContainsFunc(names, changed) } // applyTemplateStorage applies the template's storage fields. Any explicit diff --git a/internal/verda-cli/cmd/volume/describe_test.go b/internal/verda-cli/cmd/volume/describe_test.go index 6bf3bf6..2e1b7e1 100644 --- a/internal/verda-cli/cmd/volume/describe_test.go +++ b/internal/verda-cli/cmd/volume/describe_test.go @@ -24,8 +24,6 @@ import ( cmdutil "github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/util" ) -func strPtr(s string) *string { return &s } - func TestRenderVolumeSummary(t *testing.T) { t.Parallel() @@ -58,7 +56,7 @@ func TestRenderVolumeSummaryWithInstance(t *testing.T) { Status: "attached", Location: "FIN-01", IsOSVolume: true, - InstanceID: strPtr("inst-123"), + InstanceID: new("inst-123"), } var buf bytes.Buffer diff --git a/internal/verda-cli/options/options.go b/internal/verda-cli/options/options.go index 9d4b27d..05c5149 100644 --- a/internal/verda-cli/options/options.go +++ b/internal/verda-cli/options/options.go @@ -18,6 +18,7 @@ import ( "errors" "fmt" "os" + "slices" "time" "github.com/spf13/pflag" @@ -262,10 +263,8 @@ func resolveDefaultProfile(credentialsFile string) string { } // If "default" profile exists, use it. - for _, p := range profiles { - if p == defaultCredentialsProfile { - return defaultCredentialsProfile - } + if slices.Contains(profiles, defaultCredentialsProfile) { + return defaultCredentialsProfile } // If there's exactly one profile, use it automatically. diff --git a/pkg/log/context.go b/pkg/log/context.go index 200ffae..736c972 100644 --- a/pkg/log/context.go +++ b/pkg/log/context.go @@ -16,6 +16,7 @@ package log import ( "context" + "maps" "go.uber.org/zap" ) @@ -30,9 +31,7 @@ type ContextExtractors map[string]func(context.Context) string // earlier ones. func WithContextExtractor(extractors ContextExtractors) Option { return func(l *zapLogger) { - for k, v := range extractors { - l.contextExtractors[k] = v - } + maps.Copy(l.contextExtractors, extractors) } } diff --git a/pkg/tui/bubbletea/list_helpers.go b/pkg/tui/bubbletea/list_helpers.go index 39ce285..2305341 100644 --- a/pkg/tui/bubbletea/list_helpers.go +++ b/pkg/tui/bubbletea/list_helpers.go @@ -26,10 +26,7 @@ func visibleWindow(total, cursor, pageSize int) (int, int) { return 0, total } half := pageSize / 2 - start := cursor - half - if start < 0 { - start = 0 - } + start := max(cursor-half, 0) end := start + pageSize if end > total { end = total diff --git a/pkg/tui/wizard/view_hintbar.go b/pkg/tui/wizard/view_hintbar.go index a7c7cf8..ac24e9e 100644 --- a/pkg/tui/wizard/view_hintbar.go +++ b/pkg/tui/wizard/view_hintbar.go @@ -74,7 +74,7 @@ func (v *HintBarView) Update(msg any) (string, []any) { // Subscribe limits this view to StepChangedMsg only. func (v *HintBarView) Subscribe() []reflect.Type { - return []reflect.Type{reflect.TypeOf(StepChangedMsg{})} + return []reflect.Type{reflect.TypeFor[StepChangedMsg]()} } func (v *HintBarView) render() string { diff --git a/tests/contract/mockapi/mockapi.go b/tests/contract/mockapi/mockapi.go index 30d7e8d..4a8b946 100644 --- a/tests/contract/mockapi/mockapi.go +++ b/tests/contract/mockapi/mockapi.go @@ -352,7 +352,7 @@ func advanceLocked(inst *verda.Instance) { if inst.Status == statusProvisioning { inst.Status = statusRunning if inst.IP == nil { - inst.IP = ptrOf("203.0.113.10") + inst.IP = new("203.0.113.10") } } } @@ -506,7 +506,7 @@ func (s *Server) newInstanceLocked(req *verda.CreateInstanceRequest) verda.Insta Image: req.Image, OSName: req.Image, SSHKeyIDs: append([]string{}, req.SSHKeyIDs...), - OSVolumeID: ptrOf(osVol.ID), + OSVolumeID: new(osVol.ID), VolumeIDs: volumeIDs, Contract: contract, } @@ -700,8 +700,6 @@ func (s *Server) newIDLocked() string { return fmt.Sprintf("00000000-0000-4000-8000-%012d", s.idSeq) } -func ptrOf(v string) *string { return &v } - func writeJSON(w http.ResponseWriter, status int, v any) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(status) From ddea052ee540022272290534b58dd30d936b3076 Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Wed, 28 Oct 2026 15:33:27 +0200 Subject: [PATCH 03/11] ci: add go fix check and pin golangci-lint in the Makefile Mirrors verdacloud-sdk-go#65. - Makefile derives GO_VERSION from the go directive in go.mod, which CI already reads via go-version-file, so go.mod is the single source. - golangci-lint is pinned once (GOLANGCI_LINT_VERSION, still v2.5.0) and run via `go run` on that toolchain. lint, lint.fix and security use it; the lint and gosec jobs drop their own `go install` of the same version. `make security` gains --default=none so it matches CI exactly. - New `make fix` (two passes) and `make fix.check` (go fix -diff, which exits non-zero on a pending fix), with -tags=integration and the toolchain pinned because the fixer set changes between Go releases. - New "Go Fix Check" CI job, required by ci-success; a pre-commit hook runs `make fix`. CLAUDE.md and AGENTS.md document both. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 28 ++++++++++++++++++++-------- .github/workflows/security.yml | 18 ++++++------------ .pre-commit-config.yaml | 6 ++++++ AGENTS.md | 1 + CLAUDE.md | 4 ++++ Makefile | 26 +++++++++++++++++++++----- 6 files changed, 58 insertions(+), 25 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6df4508..0f24cd3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,12 +28,7 @@ jobs: go-version-file: go.mod cache: true - - name: Add Go bin to PATH - run: echo "$(go env GOPATH)/bin" >> $GITHUB_PATH - - - name: Install golangci-lint - run: go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.5.0 - + # golangci-lint version is pinned in the Makefile (GOLANGCI_LINT_VERSION) - name: Run linters run: make lint @@ -119,6 +114,23 @@ jobs: exit 1 fi + # go fix modernizers: fail if any fix is pending (go fix -diff exits non-zero) + go-fix: + name: Go Fix Check + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v6 + + - name: Set up Go + uses: actions/setup-go@v6 + with: + go-version-file: go.mod + cache: true + + - name: Check for pending go fix changes + run: make fix.check + vuln-scan: name: Vulnerability Scan runs-on: ubuntu-latest @@ -145,11 +157,11 @@ jobs: name: All CI Checks Passed runs-on: ubuntu-latest if: always() - needs: [lint, test, build, format, mod-tidy, vuln-scan] + needs: [lint, test, build, format, mod-tidy, go-fix, vuln-scan] steps: - name: Check results run: | - for result in "${{ needs.lint.result }}" "${{ needs.test.result }}" "${{ needs.build.result }}" "${{ needs.format.result }}" "${{ needs.mod-tidy.result }}" "${{ needs.vuln-scan.result }}"; do + for result in "${{ needs.lint.result }}" "${{ needs.test.result }}" "${{ needs.build.result }}" "${{ needs.format.result }}" "${{ needs.mod-tidy.result }}" "${{ needs.go-fix.result }}" "${{ needs.vuln-scan.result }}"; do if [ "$result" != "success" ]; then echo "One or more CI checks failed" exit 1 diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index cb2a807..c981c8b 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -34,19 +34,13 @@ jobs: go-version-file: go.mod cache: true - - name: Add Go bin to PATH - run: echo "$(go env GOPATH)/bin" >> $GITHUB_PATH - - - name: Install golangci-lint - run: go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.5.0 - + # golangci-lint version is pinned in the Makefile (GOLANGCI_LINT_VERSION). + # make security passes --default=none + -E gosec, scoping this step to gosec + # findings only. Without --default=none, golangci-lint's "standard" linter set + # (errcheck, govet, staticcheck, ...) also runs, which duplicates `make lint` + # and bypasses the project's test-file exclusions in .golangci.yaml. - name: Run gosec via golangci-lint - # --default=none + -E gosec scopes this step to gosec findings only. - # Without --default=none, golangci-lint's "standard" linter set - # (errcheck, govet, staticcheck, ...) also runs, which duplicates - # `make lint` and bypasses the project's test-file exclusions in - # .golangci.yaml — producing noise unrelated to security. - run: golangci-lint run --no-config --default=none -E gosec ./... + run: make security gitleaks: name: Gitleaks (secret scanning) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 53c02cb..1ac3941 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -28,6 +28,12 @@ repos: - repo: local hooks: + - id: go-fix + name: Modernize Go code (go fix) + entry: make fix + language: system + types: [go] + pass_filenames: false - id: golangci-lint name: Lint Go code (golangci-lint) entry: make lint diff --git a/AGENTS.md b/AGENTS.md index 49acf06..38bf1aa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -52,6 +52,7 @@ Skipping these steps leads to pattern violations, broken dual-mode, and pricing - [ ] `make build` passes - [ ] `make lint` passes with zero issues (do not rely on pre-commit to surface these) - [ ] `make test` passes (unit tests with -race; lint is the separate `make lint` item above) +- [ ] `make fix.check` passes (run `make fix` to apply pending `go fix` modernizations) - [ ] `--help` renders correctly for changed commands - [ ] Interactive and non-interactive modes both work - [ ] Interactive Selects pass `tui.WithShowHints(true)` so the hint bar renders diff --git a/CLAUDE.md b/CLAUDE.md index c3c971e..17a61a8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,9 +8,12 @@ Go CLI for Verda Cloud. Cobra commands + Bubble Tea TUI + lipgloss styling. make build # Build binary to ./bin/verda make test # Run all tests (go test -race) make lint # Lint only (golangci-lint); also run by pre-commit hooks +make fix # Apply go fix modernizers; CI runs `make fix.check` make pre-commit # Full pre-commit suite ``` +The `go` directive in `go.mod` is the single source of the Go version: CI reads it via `go-version-file`, and the Makefile pins `GOTOOLCHAIN` to it for golangci-lint (version pinned in the Makefile) and `go fix`. After bumping `go` in `go.mod`, run `make fix` — new Go releases add fixers, and `fix.check` will fail until they're applied. + Never use raw `go test ./...` — always `make test` (go test -race). Lint is separate: `make lint`; the pre-commit hooks run both. ## Architecture @@ -163,6 +166,7 @@ Before considering any change complete: make build # Must compile make test # Must pass (go test -race) make lint # Must pass (golangci-lint; also run by pre-commit hooks) +make fix.check # Must pass (go fix modernizers; `make fix` applies them) ``` **Never** report work as complete with lint failures outstanding. Fix them before the "done" message; don't defer to the pre-commit hook. See the "Go House Style" section above for the patterns that prevent the common hits. diff --git a/Makefile b/Makefile index 6debe0e..cfecc09 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,13 @@ OUTPUT_DIR ?= bin -.PHONY: all build clean run.sandbox lint lint.fix security test test.integration test-s3-integration fmt changelog changelog.unreleased hooks.install pre-commit help +# Single source of truth for the Go version: the go directive in go.mod (CI reads it via go-version-file). +GO_VERSION := $(shell sed -n 's/^go //p' go.mod) +GOLANGCI_LINT_VERSION := v2.5.0 +GOLANGCI_LINT := GOTOOLCHAIN=go$(GO_VERSION) go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION) +# Toolchain pinned because each Go release adds or removes fixers; the integration tag brings tests/integration into scope. +GOFIX := GOTOOLCHAIN=go$(GO_VERSION) go fix -tags=integration + +.PHONY: all build clean run.sandbox lint lint.fix security fix fix.check test test.integration test-s3-integration fmt changelog changelog.unreleased hooks.install pre-commit help ## Build ------------------------------------------------------------------- @@ -23,13 +30,22 @@ run.sandbox: build ## Run the binary against a throwaway config dir, e.g. make r ## Quality ----------------------------------------------------------------- lint: ## Run golangci-lint on all packages - @golangci-lint run ./... + @$(GOLANGCI_LINT) run ./... lint.fix: ## Run golangci-lint with auto-fix - @golangci-lint run --fix ./... + @$(GOLANGCI_LINT) run --fix ./... + +# --default=none keeps golangci-lint's "standard" set out, so only gosec findings are reported. +security: ## Run gosec-only scan, as CI does (ignores .golangci.yaml, so test files are scanned too) + @$(GOLANGCI_LINT) run --no-config --default=none -E gosec ./... + +# Two passes: applying one modernization can enable another (https://go.dev/blog/gofix). +fix: ## Apply go fix modernizers + @$(GOFIX) ./... + @$(GOFIX) ./... -security: ## Run gosec-only scan mirroring CI (ignores .golangci.yaml, so test files are scanned too) - @golangci-lint run --no-config -E gosec ./... +fix.check: ## Fail if go fix has pending modernizations (CI) + @$(GOFIX) -diff ./... || { echo "Pending go fix changes. Run 'make fix'"; exit 1; } test: ## Run all tests (with race detector) @go test -race -count=1 ./... From 7180ba52788cdb62f69eb8f6111b060d80ede550 Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Wed, 28 Oct 2026 15:40:49 +0200 Subject: [PATCH 04/11] docs(vm): name Instances.List in the SDK methods list Co-Authored-By: Claude Opus 5.5 --- internal/verda-cli/cmd/vm/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/verda-cli/cmd/vm/README.md b/internal/verda-cli/cmd/vm/README.md index a6eecb2..21d83ae 100644 --- a/internal/verda-cli/cmd/vm/README.md +++ b/internal/verda-cli/cmd/vm/README.md @@ -157,7 +157,7 @@ Destructive actions (Shutdown, Force shutdown, Delete) show confirmation prompts ### API Endpoints / SDK Methods - `client.Instances.Create` -- Create instance -- `client.Instances.Get` -- List instances (with optional status filter) +- `client.Instances.List` -- List instances (with optional status filter via `ListInstancesOptions`) - `client.Instances.GetByID` -- Get instance details - `client.Instances.Start` / `Shutdown` / `ForceShutdown` / `Hibernate` -- Instance lifecycle - `client.Instances.Delete` -- Delete instance with optional volume IDs From 4346d50c3c0c61302d46ad35076880357d6ea2b4 Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Wed, 28 Oct 2026 17:08:29 +0200 Subject: [PATCH 05/11] fix(batchjob): enforce the SDK's 10m-168h deadline range SDK v1.7.0 validates JobScalingOptions before sending and rejects deadline_seconds outside 600..604800. The CLI only checked > 0 and the wizard defaulted to 5m, so accepting the default failed after the whole wizard. validateBatchjobDeadline now backs both request() and the wizard step, and the wizard defaults to 30m (the value the help example uses). New tests run the SDK's own Validate() on the wizard default, the range edges and the default container request, so a future SDK limit change fails here instead of in front of a user. Co-Authored-By: Claude Opus 5.5 --- internal/verda-cli/cmd/serverless/CLAUDE.md | 2 +- internal/verda-cli/cmd/serverless/README.md | 2 +- .../cmd/serverless/batchjob_create.go | 19 ++++++-- .../cmd/serverless/batchjob_create_test.go | 47 +++++++++++++++++++ .../cmd/serverless/container_create_test.go | 11 +++++ .../cmd/serverless/wizard_batchjob.go | 15 +++--- 6 files changed, 83 insertions(+), 13 deletions(-) diff --git a/internal/verda-cli/cmd/serverless/CLAUDE.md b/internal/verda-cli/cmd/serverless/CLAUDE.md index 3ab5779..8b43d5f 100644 --- a/internal/verda-cli/cmd/serverless/CLAUDE.md +++ b/internal/verda-cli/cmd/serverless/CLAUDE.md @@ -97,7 +97,7 @@ Mount types in `ContainerVolumeMount.Type` that the CLI currently sends: ### Deadline is required for batchjob -`JobScalingOptions.DeadlineSeconds` must be `> 0` — enforced in `batchjobCreateOptions.request()`, in the SDK via `ValidateCreateJobDeploymentRequest`, and listed in `missingBatchjobCreateFlags`. The batchjob wizard (when implemented — see Gotchas) must include a deadline prompt as a required field. +`JobScalingOptions.DeadlineSeconds` must be 600..604800 (10m..7 days). The SDK enforces this in `JobScalingOptions.Validate` (run by `CreateJobDeployment`) before the request is sent. The CLI mirrors it in `validateBatchjobDeadline`, used by both `request()` and the wizard's deadline step, so a bad value fails at the prompt, not after the whole wizard. A missing deadline is listed in `missingBatchjobCreateFlags`. The wizard default (`30m`) must stay inside the range — `TestBatchjobWizardDefaultDeadline_PassesSDKValidation` guards it. ### Action-command factory pattern diff --git a/internal/verda-cli/cmd/serverless/README.md b/internal/verda-cli/cmd/serverless/README.md index 3a61764..899782a 100644 --- a/internal/verda-cli/cmd/serverless/README.md +++ b/internal/verda-cli/cmd/serverless/README.md @@ -122,7 +122,7 @@ verda batchjob create \ **Batch jobs cannot use spot compute.** There is no `--spot` flag; the underlying API has no `IsSpot` field for jobs. This is intentional. -**Deadline is required** and must be `> 0s`. Each queued request gets up to `--deadline` to complete; missing or zero deadline fails validation client-side and server-side. +**Deadline is required** and must be between `10m` and `168h` (7 days). Each queued request gets up to `--deadline` to complete. A missing or out-of-range deadline fails validation client-side, before the request is sent. The wizard defaults to `30m`. ### Other scaling flags diff --git a/internal/verda-cli/cmd/serverless/batchjob_create.go b/internal/verda-cli/cmd/serverless/batchjob_create.go index 6fde416..02cfad8 100644 --- a/internal/verda-cli/cmd/serverless/batchjob_create.go +++ b/internal/verda-cli/cmd/serverless/batchjob_create.go @@ -52,6 +52,19 @@ type batchjobCreateOptions struct { Yes bool } +// Deadline bounds match the SDK's JobScalingOptions.Validate, which rejects the request before it is sent. +const ( + minBatchjobDeadline = 10 * time.Minute + maxBatchjobDeadline = 7 * 24 * time.Hour +) + +func validateBatchjobDeadline(d time.Duration) error { + if d < minBatchjobDeadline || d > maxBatchjobDeadline { + return errors.New("deadline must be between 10m and 168h (7 days)") + } + return nil +} + func newCmdBatchjobCreate(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command { opts := &batchjobCreateOptions{ Port: defaultExposedPort, @@ -94,7 +107,7 @@ func newCmdBatchjobCreate(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra flags.StringArrayVar(&opts.Cmd, "cmd", nil, "Override image CMD; repeat for multiple args") flags.IntVar(&opts.MaxReplicas, "max-replicas", opts.MaxReplicas, "Maximum worker replica count") - flags.DurationVar(&opts.Deadline, "deadline", 0, "Per-request deadline (required; > 0)") + flags.DurationVar(&opts.Deadline, "deadline", 0, "Per-request deadline (required; 10m to 168h)") flags.DurationVar(&opts.RequestTTL, "request-ttl", opts.RequestTTL, "How long a pending request may live before deletion") flags.StringArrayVar(&opts.SecretMounts, "secret-mount", nil, "Secret mount SECRET:MOUNT_PATH; repeat for multiple") @@ -197,8 +210,8 @@ func (o *batchjobCreateOptions) request() (*verda.CreateJobDeploymentRequest, er if o.MaxReplicas < 1 { return nil, errors.New("--max-replicas must be >= 1") } - if o.Deadline <= 0 { - return nil, errors.New("--deadline must be > 0") + if err := validateBatchjobDeadline(o.Deadline); err != nil { + return nil, errors.New("--" + err.Error()) } if o.Port < 1 || o.Port > 65535 { return nil, errors.New("--port must be in 1..65535") diff --git a/internal/verda-cli/cmd/serverless/batchjob_create_test.go b/internal/verda-cli/cmd/serverless/batchjob_create_test.go index dda29d8..5cee900 100644 --- a/internal/verda-cli/cmd/serverless/batchjob_create_test.go +++ b/internal/verda-cli/cmd/serverless/batchjob_create_test.go @@ -78,6 +78,53 @@ func TestBatchjobRequest_RequiresDeadline(t *testing.T) { } } +func TestBatchjobRequest_DeadlineRangeMatchesSDK(t *testing.T) { + cases := []struct { + deadline time.Duration + ok bool + }{ + {5 * time.Minute, false}, + {10*time.Minute - time.Second, false}, + {10 * time.Minute, true}, + {168 * time.Hour, true}, + {168*time.Hour + time.Second, false}, + } + for _, c := range cases { + opts := validJobOpts() + opts.Deadline = c.deadline + req, err := opts.request() + if (err == nil) != c.ok { + t.Errorf("deadline %s: request() err = %v, want ok=%v", c.deadline, err, c.ok) + continue + } + if c.ok { + if err := req.Validate(); err != nil { + t.Errorf("deadline %s: CLI accepts but SDK rejects: %v", c.deadline, err) + } + } + } +} + +func TestBatchjobWizardDefaultDeadline_PassesSDKValidation(t *testing.T) { + var d time.Duration + step := stepBatchjobDeadline(&d) + def := step.Default(nil) + if err := step.Validate(def); err != nil { + t.Fatalf("wizard rejects its own default %v: %v", def, err) + } + step.Setter(def) + + opts := validJobOpts() + opts.Deadline = d + req, err := opts.request() + if err != nil { + t.Fatalf("request: %v", err) + } + if err := req.Validate(); err != nil { + t.Fatalf("SDK rejects the wizard default deadline %s: %v", d, err) + } +} + func TestBatchjobMissingFlags(t *testing.T) { opts := &batchjobCreateOptions{} missing := missingBatchjobCreateFlags(opts) diff --git a/internal/verda-cli/cmd/serverless/container_create_test.go b/internal/verda-cli/cmd/serverless/container_create_test.go index 5fe7ba4..262981a 100644 --- a/internal/verda-cli/cmd/serverless/container_create_test.go +++ b/internal/verda-cli/cmd/serverless/container_create_test.go @@ -46,6 +46,17 @@ func validOpts() *containerCreateOptions { } } +// validOpts mirrors the create defaults; the SDK validates requests before sending. +func TestContainerRequest_DefaultsPassSDKValidation(t *testing.T) { + req, err := validOpts().request() + if err != nil { + t.Fatalf("request: %v", err) + } + if err := req.Validate(); err != nil { + t.Fatalf("SDK rejects default container request: %v", err) + } +} + func TestContainerRequest_HappyPath(t *testing.T) { opts := validOpts() req, err := opts.request() diff --git a/internal/verda-cli/cmd/serverless/wizard_batchjob.go b/internal/verda-cli/cmd/serverless/wizard_batchjob.go index 663363c..1c8ea84 100644 --- a/internal/verda-cli/cmd/serverless/wizard_batchjob.go +++ b/internal/verda-cli/cmd/serverless/wizard_batchjob.go @@ -49,28 +49,27 @@ func buildBatchjobCreateFlow(_ context.Context, getClient clientFunc, opts *batc } } -// stepBatchjobDeadline asks for the per-request deadline. Required (> 0); -// `JobScalingOptions.DeadlineSeconds` is rejected server-side when missing, -// so we enforce it client-side for a friendlier error. +// stepBatchjobDeadline asks for the per-request deadline. Required, 10m..168h; +// the SDK rejects anything outside that range only after the whole wizard. func stepBatchjobDeadline(target *time.Duration) wizard.Step { return wizard.Step{ Name: "deadline", - Description: "Per-request deadline (e.g. 5m, 30m, 1h) — required", + Description: "Per-request deadline (10m to 168h, e.g. 30m, 1h) — required", Prompt: wizard.TextInputPrompt, Required: true, Default: func(_ map[string]any) any { if *target > 0 { return target.String() } - return "5m" + return "30m" }, Validate: func(v any) error { s := strings.TrimSpace(v.(string)) d, err := time.ParseDuration(s) - if err != nil || d <= 0 { - return errors.New("deadline must be a positive duration (e.g. 5m, 30m, 1h)") + if err != nil { + return errors.New("deadline must be a duration (e.g. 30m, 1h)") } - return nil + return validateBatchjobDeadline(d) }, Setter: func(v any) { d, _ := time.ParseDuration(strings.TrimSpace(v.(string))) From 901558573d7044727eb294a5ec6a96a7fc924d87 Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Wed, 28 Oct 2026 17:10:48 +0200 Subject: [PATCH 06/11] fix(githooks): run the pinned make targets and mark the hook executable The pre-commit hook used whatever golangci-lint was on PATH (and skipped lint when there was none) and never ran go fix, so a commit could pass locally and fail the required Go Fix Check job. It now runs `make fix.check` and `make lint`, the same pinned targets as CI. The file was also stored as 100644. Git ignores a hook that is not executable, so `make hooks.install` never ran it. Mark it 100755. This also activates the hook's existing doc-update step (skip it with SKIP_DOC_UPDATE=1). Co-Authored-By: Claude Opus 5.5 --- githooks/pre-commit | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) mode change 100644 => 100755 githooks/pre-commit diff --git a/githooks/pre-commit b/githooks/pre-commit old mode 100644 new mode 100755 index 86c2cd4..95545be --- a/githooks/pre-commit +++ b/githooks/pre-commit @@ -5,7 +5,7 @@ # make hooks.install # # or: git config core.hooksPath githooks # -# Requires bash. Optional: golangci-lint on PATH for extra checks. +# Requires bash and Go (make lint and make fix.check fetch their pinned tools). set -euo pipefail STAGED_GO=$(git diff --cached --name-only --diff-filter=ACM -- '*.go' || true) @@ -21,10 +21,12 @@ go build ./... printf " go vet ./...\n" go vet ./... -if command -v golangci-lint &>/dev/null; then - printf " golangci-lint (staged files)\n" - echo "$STAGED_GO" | xargs golangci-lint run --new-from-rev=HEAD -fi +# Same pinned targets as CI: the Makefile pins golangci-lint and the go fix toolchain. +printf " make fix.check\n" +make --no-print-directory fix.check + +printf " make lint\n" +make --no-print-directory lint PKGS=$(echo "$STAGED_GO" | xargs -I{} dirname {} | sort -u | sed 's|^|./|') if [ -n "$PKGS" ]; then From 644c92dc04743262a2e0d9ef8c839f949b783fb2 Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Wed, 28 Oct 2026 17:37:54 +0200 Subject: [PATCH 07/11] fix(githooks): make the claude doc-update step opt-in The hook's last step starts a headless claude session with --dangerously-skip-permissions (up to $0.50 per commit) and stages any unstaged cmd/*/README.md and CLAUDE.md changes. It never ran while the hook was non-executable. Now that it is executable, run that step only when VERDA_DOC_UPDATE=1, so a commit does not start paid AI edits by default. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- githooks/pre-commit | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 17a61a8..0865048 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -143,7 +143,7 @@ The repo lints with `golangci-lint` via `make lint` (also enforced by the pre-co 3. Read `.ai/skills/new-command.md` for the full checklist when adding/modifying commands 4. If touching pricing, auth, or agent-mode: plan first, don't code immediately -Per-command docs are auto-maintained by `/update-command-knowledge` skill. +Per-command docs are maintained by the `/update-command-knowledge` skill. The `githooks/pre-commit` hook runs it only when `VERDA_DOC_UPDATE=1` is set. Manual update: `claude -p "/update-command-knowledge --all" --model sonnet --dangerously-skip-permissions` ## Thinking Depth diff --git a/githooks/pre-commit b/githooks/pre-commit index 95545be..10eee66 100755 --- a/githooks/pre-commit +++ b/githooks/pre-commit @@ -6,6 +6,7 @@ # # or: git config core.hooksPath githooks # # Requires bash and Go (make lint and make fix.check fetch their pinned tools). +# VERDA_DOC_UPDATE=1 also regenerates command docs with claude (off by default). set -euo pipefail STAGED_GO=$(git diff --cached --name-only --diff-filter=ACM -- '*.go' || true) @@ -35,9 +36,8 @@ if [ -n "$PKGS" ]; then fi # --- Auto-update per-subcommand knowledge docs ---------------------------- -if [ "${SKIP_DOC_UPDATE:-}" = "1" ]; then - printf " ⏭️ skipping command knowledge doc update (SKIP_DOC_UPDATE=1)\n" -else +# Opt-in: runs headless claude with skipped permissions and stages its doc edits. +if [ "${VERDA_DOC_UPDATE:-}" = "1" ]; then CHANGED_CMD_DIRS=$(git diff --cached --name-only -- 'internal/verda-cli/cmd/' \ | grep -v 'cmd/util/' | grep -v 'cmd/cmd\.go' | grep -v 'cmd/helper\.go' \ | sed 's|internal/verda-cli/cmd/\([^/]*\)/.*|\1|' | sort -u || true) From b790b119a11081b4cd58b52506d894abee6de52a Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Wed, 28 Oct 2026 17:38:40 +0200 Subject: [PATCH 08/11] fix(serverless): match wizard and flag limits to SDK v1.7.0 validation SDK v1.7.0 validates create requests before sending them. Several wizard steps accepted wider ranges, so the user finished the whole wizard and only then got a validation error, with no way to correct the value. The CPU/GPU prompts and flag help also advertised 1..100 while an enabled threshold now needs at least 5. The limits now live once in shared.go and back the wizard validators, the validate()/request() flag checks and the flag help: compute size 1..128, min replicas 0..1000, max replicas 1..1000, concurrency 1..5000, CPU/GPU util 0 (off) or 5..100, scale delays 0s..1h, request TTL 0s..24h (container) and 0s..168h (batch job), healthcheck port 0..65535. Container and job TTL keep separate steps because their ceilings differ. Disabled triggers, zero durations and all defaults behave as before. Flag-only and agent runs were already rejected by the SDK; they now get an error that names the flag. The request payload is unchanged. sdk_limits_test.go runs the SDK's Validate() at each boundary for both command families. We did not establish that these out-of-range values ever succeeded against the live API; they match the current public API schema. Co-Authored-By: Claude Opus 5.5 --- internal/verda-cli/cmd/serverless/CLAUDE.md | 8 +- internal/verda-cli/cmd/serverless/README.md | 14 +- .../cmd/serverless/batchjob_create.go | 23 +- .../cmd/serverless/container_create.go | 59 +++--- .../cmd/serverless/sdk_limits_test.go | 199 ++++++++++++++++++ internal/verda-cli/cmd/serverless/shared.go | 50 +++++ internal/verda-cli/cmd/serverless/wizard.go | 19 +- .../cmd/serverless/wizard_batchjob.go | 2 +- .../verda-cli/cmd/serverless/wizard_shared.go | 51 ++--- 9 files changed, 332 insertions(+), 93 deletions(-) create mode 100644 internal/verda-cli/cmd/serverless/sdk_limits_test.go diff --git a/internal/verda-cli/cmd/serverless/CLAUDE.md b/internal/verda-cli/cmd/serverless/CLAUDE.md index 8b43d5f..fc80182 100644 --- a/internal/verda-cli/cmd/serverless/CLAUDE.md +++ b/internal/verda-cli/cmd/serverless/CLAUDE.md @@ -36,7 +36,7 @@ This rule applies equally to Claude, Codex, Cursor, or a human editing the packa - `batchjob_create.go` — `batchjobCreateOptions` (simpler: no spot, deadline required). - `batchjob_list.go`, `batchjob_describe.go`, `batchjob_delete.go`, `batchjob_actions.go` — Same shape as container, trimmed. - `shared.go` — `validateDeploymentName` (RFC-1123 subset), `rejectLatestTag`, `parseEnvFlag`, `parseSecretMountFlag`, `confirmDestructive`, `statusColor`, `mountType*` + `envType*` constants. - - `wizard_shared.go` — Step builders shared by both create wizards: `stepName`, `stepImage`, `stepCompute`, `stepComputeSize`, `stepRegistryCreds`, `stepPort`, `stepEnvVars`, `stepMaxReplicas`, `stepRequestTTL`, `stepSecretMounts`. Plus the generic `durationStep` helper and three int validators. Each takes a `*T` pointer to the field it mutates, so the same step definition drives both `containerCreateOptions` and `batchjobCreateOptions`. + - `wizard_shared.go` — Step builders shared by both create wizards: `stepName`, `stepImage`, `stepCompute`, `stepComputeSize`, `stepRegistryCreds`, `stepPort`, `stepEnvVars`, `stepMaxReplicas`, `stepContainerRequestTTL` / `stepBatchjobRequestTTL`, `stepSecretMounts`. Plus the generic `durationStep` helper (bounded by a max) and `parseIntRangeValidator`. Each takes a `*T` pointer to the field it mutates, so the same step definition drives both `containerCreateOptions` and `batchjobCreateOptions`. - `wizard.go` — `buildContainerCreateFlow` + container-only steps: spot/compute-type, healthcheck on/off + path (port is NOT prompted; it always defaults to the exposed port in `request()`), min-replicas, concurrency, queue-load preset + custom override, CPU/GPU util triggers, scale-up/down delays. 21 total steps in the container flow. - `wizard_batchjob.go` — `buildBatchjobCreateFlow` + `stepBatchjobDeadline`. 11 total steps: 10 reused from `wizard_shared.go` + the batchjob-only deadline. Jobs have no spot, no min replicas, no scaling triggers, no healthcheck, no concurrency — all of those steps are simply absent from the flow. - `wizard_cache.go` — `apiCache` with lazy loaders for compute resources, registry creds, secrets, file secrets. Shared across wizard passes so back-navigation doesn't re-hit the API. Used by both container and batchjob wizards. @@ -99,6 +99,10 @@ Mount types in `ContainerVolumeMount.Type` that the CLI currently sends: `JobScalingOptions.DeadlineSeconds` must be 600..604800 (10m..7 days). The SDK enforces this in `JobScalingOptions.Validate` (run by `CreateJobDeployment`) before the request is sent. The CLI mirrors it in `validateBatchjobDeadline`, used by both `request()` and the wizard's deadline step, so a bad value fails at the prompt, not after the whole wizard. A missing deadline is listed in `missingBatchjobCreateFlags`. The wizard default (`30m`) must stay inside the range — `TestBatchjobWizardDefaultDeadline_PassesSDKValidation` guards it. +### Create limits mirror the SDK + +The SDK validates create requests before sending them, so a wizard step that accepts a wider range than the SDK fails only after the whole wizard. The limits live once in `shared.go` (`maxComputeSize`, `maxReplicas`, `maxConcurrency`, `min/maxUtilThreshold`, `maxScalingDelay`, `maxContainerRequestTTL` 24h, `maxBatchjobRequestTTL` 168h) and back the wizard validators, the `validate()`/`request()` flag checks and the flag help. Container and job request TTL differ — keep `stepContainerRequestTTL` and `stepBatchjobRequestTTL` separate. `sdk_limits_test.go` runs the SDK's `Validate()` at each boundary; when the SDK changes a limit, that test fails first. + ### Action-command factory pattern `newContainerActionCmd` / `newBatchjobActionCmd` build a `*cobra.Command` from `(verb, short, spinner, successMsg, destructive, fn)`. This avoids five nearly-identical files per subcommand. If you need to add an action (e.g. a future `scaling get`), add a new call site with the right SDK method. If you need per-action flags beyond `--yes`, you'll have to step out of the factory — acceptable if one action grows special, not if two do. @@ -118,7 +122,7 @@ Describe cards (`renderContainerDeploymentCard`, `renderJobDeploymentCard`) prin - **Wizard omits the healthcheck-path step when healthcheck is Off.** The `healthcheck-path` step has `ShouldSkip: c["healthcheck"] == "off"`; the engine wires the skip gate via `DependsOn`. There is intentionally no `healthcheck-port` wizard step — `request()` always defaults the probe port to the exposed port (`hcPort = o.Port` when `HealthcheckPort == 0`). The `--healthcheck-port` flag still works for the rare case where the probe must hit a different port than the public listener. - **`registryPublicValue = "__public__"` sentinel.** The registry-creds step's loader prepends a "Public (no credentials)" choice with this sentinel as its Value. The Setter maps the sentinel back to `opts.RegistryCreds = ""`. If you rename the sentinel, grep both sides — the Setter reads the string literal. - **`compute-size` is a separate step from `compute`.** VM's wizard combines resource + count in a single step via in-Loader prompting; serverless keeps them separate so users can go back and change the size without re-picking the resource. Lower engineering cost, same UX. -- **Util triggers off by default, but wizard asks anyway.** The CPU/GPU util steps accept empty ("off"), "off", or `1..100`. Setter maps empty/"off" to 0 (trigger disabled). Users should be able to Enter-through both without setting them. +- **Util triggers off by default, but wizard asks anyway.** The CPU/GPU util steps accept empty ("off"), "off", or `5..100` (the SDK rejects an enabled threshold under 5). Setter maps empty/"off" to 0 (trigger disabled). Users should be able to Enter-through both without setting them. - **Custom queue-load is a separate step.** `queue-load-custom` has `ShouldSkip: c["queue-preset"] != "custom"`. If the user goes back and changes preset to a named one, the engine's reset logic clears the custom value via `Resetter`. - **No `+ Create new` for registry creds in the wizard.** v1 intentionally omits the inline create-new sub-flow for registry credentials — users pick existing or Public. Adding new creds requires `verda registry configure` out-of-band, or a future top-level `verda registry-creds` command. The design doc notes this as future work. - **Confirm is NOT a wizard step.** `runContainerCreate` prints the summary + runs `prompter.Confirm` after `engine.Run` returns. Keeps the review card at full terminal width and lets us pipe through `--yes` cleanly. If you move it into the wizard, you lose layout control. diff --git a/internal/verda-cli/cmd/serverless/README.md b/internal/verda-cli/cmd/serverless/README.md index 899782a..c277ee9 100644 --- a/internal/verda-cli/cmd/serverless/README.md +++ b/internal/verda-cli/cmd/serverless/README.md @@ -63,11 +63,11 @@ verda container create \ ### Other scaling flags -- `--min-replicas` (default `0`, scale-to-zero) / `--max-replicas` (default `3`) -- `--concurrency` (default `1` — set higher for LLMs, 1 for image generation) -- `--cpu-util `, `--gpu-util ` — enable the corresponding trigger (blank = off) -- `--scale-up-delay`, `--scale-down-delay` (default `5m`) — hysteresis before scaling -- `--request-ttl` (default `5m`) — how long a pending request may live before the queue drops it +- `--min-replicas` (`0..1000`, default `0`, scale-to-zero) / `--max-replicas` (`1..1000`, default `3`) +- `--concurrency` (`1..5000`, default `1` — set higher for LLMs, 1 for image generation) +- `--cpu-util `, `--gpu-util ` — enable the corresponding trigger at `5..100` (`0` or blank = off) +- `--scale-up-delay`, `--scale-down-delay` (`0s` to `1h`, scale-down default `5m`) — hysteresis before scaling +- `--request-ttl` (`0s` to `24h`, default `5m`) — how long a pending request may live before the queue drops it ### Healthcheck @@ -126,8 +126,8 @@ verda batchjob create \ ### Other scaling flags -- `--max-replicas` (default `3`) — worker pool cap -- `--request-ttl` (default `5m`) — how long a pending request may live in the queue before the server drops it +- `--max-replicas` (`1..1000`, default `3`) — worker pool cap +- `--request-ttl` (`0s` to `168h`, default `5m`) — how long a pending request may live in the queue before the server drops it ### Lifecycle diff --git a/internal/verda-cli/cmd/serverless/batchjob_create.go b/internal/verda-cli/cmd/serverless/batchjob_create.go index 02cfad8..ec10809 100644 --- a/internal/verda-cli/cmd/serverless/batchjob_create.go +++ b/internal/verda-cli/cmd/serverless/batchjob_create.go @@ -97,7 +97,7 @@ func newCmdBatchjobCreate(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra flags.StringVar(&opts.Name, "name", "", "Deployment name (URL slug; immutable after create)") flags.StringVar(&opts.Image, "image", "", "Container image reference (must not be ':latest')") flags.StringVar(&opts.Compute, "compute", "", "Compute resource name (e.g. RTX4500Ada, CPUNode)") - flags.IntVar(&opts.ComputeSize, "compute-size", 1, "Number of GPUs or vCPU cores per replica") + flags.IntVar(&opts.ComputeSize, "compute-size", 1, "Number of GPUs or vCPU cores per replica (1..128)") flags.StringVar(&opts.RegistryCreds, "registry-creds", "", "Registry credentials name (empty = public)") flags.IntVar(&opts.Port, "port", opts.Port, "Exposed HTTP port") @@ -106,9 +106,9 @@ func newCmdBatchjobCreate(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra flags.StringArrayVar(&opts.Entrypoint, "entrypoint", nil, "Override image ENTRYPOINT; repeat for multiple args") flags.StringArrayVar(&opts.Cmd, "cmd", nil, "Override image CMD; repeat for multiple args") - flags.IntVar(&opts.MaxReplicas, "max-replicas", opts.MaxReplicas, "Maximum worker replica count") + flags.IntVar(&opts.MaxReplicas, "max-replicas", opts.MaxReplicas, "Maximum worker replica count (1..1000)") flags.DurationVar(&opts.Deadline, "deadline", 0, "Per-request deadline (required; 10m to 168h)") - flags.DurationVar(&opts.RequestTTL, "request-ttl", opts.RequestTTL, "How long a pending request may live before deletion") + flags.DurationVar(&opts.RequestTTL, "request-ttl", opts.RequestTTL, "How long a pending request may live before deletion (0s to 168h)") flags.StringArrayVar(&opts.SecretMounts, "secret-mount", nil, "Secret mount SECRET:MOUNT_PATH; repeat for multiple") @@ -204,17 +204,18 @@ func (o *batchjobCreateOptions) request() (*verda.CreateJobDeploymentRequest, er if err := rejectLatestTag(o.Image); err != nil { return nil, err } - if o.ComputeSize < 1 { - return nil, errors.New("--compute-size must be >= 1") - } - if o.MaxReplicas < 1 { - return nil, errors.New("--max-replicas must be >= 1") - } if err := validateBatchjobDeadline(o.Deadline); err != nil { return nil, errors.New("--" + err.Error()) } - if o.Port < 1 || o.Port > 65535 { - return nil, errors.New("--port must be in 1..65535") + for _, err := range []error{ + checkIntRange("--compute-size", o.ComputeSize, 1, maxComputeSize), + checkIntRange("--max-replicas", o.MaxReplicas, 1, maxReplicas), + checkIntRange("--port", o.Port, 1, maxPort), + checkDurationRange("--request-ttl", o.RequestTTL, maxBatchjobRequestTTL), + } { + if err != nil { + return nil, err + } } env, err := buildEnvVars(o.Env, o.EnvSecret) diff --git a/internal/verda-cli/cmd/serverless/container_create.go b/internal/verda-cli/cmd/serverless/container_create.go index 5f8f197..ff7b323 100644 --- a/internal/verda-cli/cmd/serverless/container_create.go +++ b/internal/verda-cli/cmd/serverless/container_create.go @@ -138,30 +138,30 @@ func newCmdContainerCreate(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobr flags.BoolVar(&opts.Spot, "spot", false, "Use spot compute instead of on-demand") flags.StringVar(&opts.Compute, "compute", "", "Compute resource name (e.g. RTX4500Ada, CPUNode)") - flags.IntVar(&opts.ComputeSize, "compute-size", 1, "Number of GPUs or vCPU cores per replica") + flags.IntVar(&opts.ComputeSize, "compute-size", 1, "Number of GPUs or vCPU cores per replica (1..128)") flags.StringVar(&opts.RegistryCreds, "registry-creds", "", "Registry credentials name (empty = public)") flags.BoolVar(&opts.RegistryPublic, "registry-public", false, "Pull image anonymously (default)") flags.IntVar(&opts.Port, "port", opts.Port, "Exposed HTTP port") flags.BoolVar(&opts.HealthcheckOff, "healthcheck-off", false, "Disable healthcheck (default: on at /health)") - flags.IntVar(&opts.HealthcheckPort, "healthcheck-port", 0, "Healthcheck HTTP port (defaults to --port)") + flags.IntVar(&opts.HealthcheckPort, "healthcheck-port", 0, "Healthcheck HTTP port (1..65535; defaults to --port)") flags.StringVar(&opts.HealthcheckPath, "healthcheck-path", opts.HealthcheckPath, "Healthcheck HTTP path") flags.StringArrayVar(&opts.Env, "env", nil, "Environment variable KEY=VALUE (KEY uppercase); repeat for multiple") flags.StringArrayVar(&opts.EnvSecret, "env-secret", nil, "Secret-backed env KEY=SECRET_NAME (KEY uppercase); repeat for multiple") flags.StringArrayVar(&opts.Entrypoint, "entrypoint", nil, "Override image ENTRYPOINT; repeat for multiple args") flags.StringArrayVar(&opts.Cmd, "cmd", nil, "Override image CMD; repeat for multiple args") - flags.IntVar(&opts.MinReplicas, "min-replicas", opts.MinReplicas, "Minimum replica count (0 = scale-to-zero)") - flags.IntVar(&opts.MaxReplicas, "max-replicas", opts.MaxReplicas, "Maximum replica count") - flags.IntVar(&opts.Concurrency, "concurrency", opts.Concurrency, "Concurrent requests per replica") + flags.IntVar(&opts.MinReplicas, "min-replicas", opts.MinReplicas, "Minimum replica count (0..1000; 0 = scale-to-zero)") + flags.IntVar(&opts.MaxReplicas, "max-replicas", opts.MaxReplicas, "Maximum replica count (1..1000)") + flags.IntVar(&opts.Concurrency, "concurrency", opts.Concurrency, "Concurrent requests per replica (1..5000)") flags.StringVar(&opts.QueuePreset, "queue-preset", opts.QueuePreset, "Scaling preset: instant | balanced | cost-saver | custom") flags.IntVar(&opts.QueueLoad, "queue-load", 0, "Custom queue-load threshold (1..1000); sets --queue-preset=custom when used") - flags.IntVar(&opts.CPUUtil, "cpu-util", 0, "CPU utilization trigger threshold % (1..100); 0 = off") - flags.IntVar(&opts.GPUUtil, "gpu-util", 0, "GPU utilization trigger threshold % (1..100); 0 = off") - flags.DurationVar(&opts.ScaleUpDelay, "scale-up-delay", opts.ScaleUpDelay, "Delay before scaling up") - flags.DurationVar(&opts.ScaleDownDelay, "scale-down-delay", opts.ScaleDownDelay, "Delay before scaling down") - flags.DurationVar(&opts.RequestTTL, "request-ttl", opts.RequestTTL, "How long a pending request may live before deletion") + flags.IntVar(&opts.CPUUtil, "cpu-util", 0, "CPU utilization trigger threshold % (5..100); 0 = off") + flags.IntVar(&opts.GPUUtil, "gpu-util", 0, "GPU utilization trigger threshold % (5..100); 0 = off") + flags.DurationVar(&opts.ScaleUpDelay, "scale-up-delay", opts.ScaleUpDelay, "Delay before scaling up (0s to 1h)") + flags.DurationVar(&opts.ScaleDownDelay, "scale-down-delay", opts.ScaleDownDelay, "Delay before scaling down (0s to 1h)") + flags.DurationVar(&opts.RequestTTL, "request-ttl", opts.RequestTTL, "How long a pending request may live before deletion (0s to 24h)") flags.StringArrayVar(&opts.SecretMounts, "secret-mount", nil, "Secret mount SECRET:MOUNT_PATH; repeat for multiple") @@ -256,26 +256,25 @@ func (o *containerCreateOptions) validate() error { if err := rejectLatestTag(o.Image); err != nil { return err } - if o.ComputeSize < 1 { - return errors.New("--compute-size must be >= 1") - } - if o.MinReplicas < 0 { - return errors.New("--min-replicas must be >= 0") - } - if o.MaxReplicas < 1 || o.MaxReplicas < o.MinReplicas { - return errors.New("--max-replicas must be >= max(1, --min-replicas)") - } - if o.Concurrency < 1 { - return errors.New("--concurrency must be >= 1") - } - if o.Port < 1 || o.Port > 65535 { - return errors.New("--port must be in 1..65535") - } - if o.CPUUtil < 0 || o.CPUUtil > 100 { - return errors.New("--cpu-util must be in 0..100") - } - if o.GPUUtil < 0 || o.GPUUtil > 100 { - return errors.New("--gpu-util must be in 0..100") + if o.MaxReplicas < o.MinReplicas { + return errors.New("--max-replicas must be >= --min-replicas") + } + for _, err := range []error{ + checkIntRange("--compute-size", o.ComputeSize, 1, maxComputeSize), + checkIntRange("--min-replicas", o.MinReplicas, 0, maxReplicas), + checkIntRange("--max-replicas", o.MaxReplicas, 1, maxReplicas), + checkIntRange("--concurrency", o.Concurrency, 1, maxConcurrency), + checkIntRange("--port", o.Port, 1, maxPort), + checkIntRange("--healthcheck-port", o.HealthcheckPort, 0, maxPort), + checkUtilThreshold("--cpu-util", o.CPUUtil), + checkUtilThreshold("--gpu-util", o.GPUUtil), + checkDurationRange("--scale-up-delay", o.ScaleUpDelay, maxScalingDelay), + checkDurationRange("--scale-down-delay", o.ScaleDownDelay, maxScalingDelay), + checkDurationRange("--request-ttl", o.RequestTTL, maxContainerRequestTTL), + } { + if err != nil { + return err + } } return nil } diff --git a/internal/verda-cli/cmd/serverless/sdk_limits_test.go b/internal/verda-cli/cmd/serverless/sdk_limits_test.go new file mode 100644 index 0000000..ee99d8d --- /dev/null +++ b/internal/verda-cli/cmd/serverless/sdk_limits_test.go @@ -0,0 +1,199 @@ +// Copyright 2026 Verda Cloud Oy +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package serverless + +import ( + "strings" + "testing" + "time" + + "github.com/verda-cloud/verda-cli/pkg/tui/wizard" +) + +// The SDK validates create requests before sending them. A wizard step that +// accepts a value the SDK rejects fails only after the whole wizard, so every +// step must accept exactly the SDK's range. + +type limitInput struct { + input string + ok bool +} + +func TestContainerWizardStepsMatchSDKLimits(t *testing.T) { + utilInputs := []limitInput{{"", true}, {"off", true}, {"4", false}, {"5", true}, {"100", true}, {"101", false}} + delayInputs := []limitInput{{"", true}, {"0s", true}, {"1h", true}, {"1h1s", false}, {"-1s", false}} + steps := []struct { + name string + step func(*containerCreateOptions) wizard.Step + inputs []limitInput + }{ + {"compute-size", func(o *containerCreateOptions) wizard.Step { return stepComputeSize(&o.ComputeSize) }, + []limitInput{{"0", false}, {"1", true}, {"128", true}, {"129", false}}}, + {"min-replicas", func(o *containerCreateOptions) wizard.Step { return stepContainerMinReplicas(&o.MinReplicas) }, + []limitInput{{"-1", false}, {"0", true}, {"3", true}, {"1001", false}}}, + {"max-replicas", func(o *containerCreateOptions) wizard.Step { return stepMaxReplicas(&o.MaxReplicas) }, + []limitInput{{"0", false}, {"1", true}, {"1000", true}, {"1001", false}}}, + {"concurrency", func(o *containerCreateOptions) wizard.Step { return stepContainerConcurrency(&o.Concurrency) }, + []limitInput{{"0", false}, {"1", true}, {"5000", true}, {"5001", false}}}, + {"cpu-util", func(o *containerCreateOptions) wizard.Step { return stepContainerCPUUtil(&o.CPUUtil) }, utilInputs}, + {"gpu-util", func(o *containerCreateOptions) wizard.Step { return stepContainerGPUUtil(&o.GPUUtil) }, utilInputs}, + {"scale-up-delay", func(o *containerCreateOptions) wizard.Step { return stepContainerScaleUpDelay(&o.ScaleUpDelay) }, delayInputs}, + {"scale-down-delay", func(o *containerCreateOptions) wizard.Step { return stepContainerScaleDownDelay(&o.ScaleDownDelay) }, delayInputs}, + {"request-ttl", func(o *containerCreateOptions) wizard.Step { return stepContainerRequestTTL(&o.RequestTTL) }, + []limitInput{{"", true}, {"0s", true}, {"24h", true}, {"24h1s", false}}}, + } + for _, s := range steps { + for _, in := range s.inputs { + t.Run(s.name+"/"+in.input, func(t *testing.T) { + opts := validOpts() + step := s.step(opts) + if err := step.Validate(in.input); (err == nil) != in.ok { + t.Fatalf("wizard Validate(%q) = %v, want ok=%v", in.input, err, in.ok) + } + if !in.ok { + return + } + step.Setter(in.input) + req, err := opts.request() + if err != nil { + t.Fatalf("wizard accepts %q but request() rejects it: %v", in.input, err) + } + if err := req.Validate(); err != nil { + t.Fatalf("wizard accepts %q but SDK rejects it: %v", in.input, err) + } + }) + } + } +} + +func TestBatchjobWizardStepsMatchSDKLimits(t *testing.T) { + steps := []struct { + name string + step func(*batchjobCreateOptions) wizard.Step + inputs []limitInput + }{ + {"compute-size", func(o *batchjobCreateOptions) wizard.Step { return stepComputeSize(&o.ComputeSize) }, + []limitInput{{"0", false}, {"1", true}, {"128", true}, {"129", false}}}, + {"max-replicas", func(o *batchjobCreateOptions) wizard.Step { return stepMaxReplicas(&o.MaxReplicas) }, + []limitInput{{"0", false}, {"1", true}, {"1000", true}, {"1001", false}}}, + {"request-ttl", func(o *batchjobCreateOptions) wizard.Step { return stepBatchjobRequestTTL(&o.RequestTTL) }, + []limitInput{{"", true}, {"0s", true}, {"168h", true}, {"168h1s", false}}}, + } + for _, s := range steps { + for _, in := range s.inputs { + t.Run(s.name+"/"+in.input, func(t *testing.T) { + opts := validJobOpts() + step := s.step(opts) + if err := step.Validate(in.input); (err == nil) != in.ok { + t.Fatalf("wizard Validate(%q) = %v, want ok=%v", in.input, err, in.ok) + } + if !in.ok { + return + } + step.Setter(in.input) + req, err := opts.request() + if err != nil { + t.Fatalf("wizard accepts %q but request() rejects it: %v", in.input, err) + } + if err := req.Validate(); err != nil { + t.Fatalf("wizard accepts %q but SDK rejects it: %v", in.input, err) + } + }) + } + } +} + +// Flag-only and agent runs reach the same limits through request(). The CLI +// checks them first so the error names the flag and its range. +func TestContainerFlagsMatchSDKLimits(t *testing.T) { + cases := []struct { + name string + mutate func(*containerCreateOptions) + errFlag string // empty: request() and the SDK must accept + }{ + {"compute-size max", func(o *containerCreateOptions) { o.ComputeSize = 128 }, ""}, + {"compute-size over", func(o *containerCreateOptions) { o.ComputeSize = 129 }, "--compute-size"}, + {"replicas max", func(o *containerCreateOptions) { o.MinReplicas, o.MaxReplicas = 1000, 1000 }, ""}, + {"min-replicas over", func(o *containerCreateOptions) { o.MinReplicas, o.MaxReplicas = 1001, 1001 }, "--min-replicas"}, + {"max-replicas over", func(o *containerCreateOptions) { o.MaxReplicas = 1001 }, "--max-replicas"}, + {"concurrency max", func(o *containerCreateOptions) { o.Concurrency = 5000 }, ""}, + {"concurrency over", func(o *containerCreateOptions) { o.Concurrency = 5001 }, "--concurrency"}, + {"cpu-util off", func(o *containerCreateOptions) { o.CPUUtil = 0 }, ""}, + {"cpu-util min", func(o *containerCreateOptions) { o.CPUUtil = 5 }, ""}, + {"cpu-util under", func(o *containerCreateOptions) { o.CPUUtil = 4 }, "--cpu-util"}, + {"gpu-util max", func(o *containerCreateOptions) { o.GPUUtil = 100 }, ""}, + {"gpu-util under", func(o *containerCreateOptions) { o.GPUUtil = 1 }, "--gpu-util"}, + {"scale-up-delay max", func(o *containerCreateOptions) { o.ScaleUpDelay = time.Hour }, ""}, + {"scale-up-delay over", func(o *containerCreateOptions) { o.ScaleUpDelay = time.Hour + time.Second }, "--scale-up-delay"}, + {"scale-down-delay over", func(o *containerCreateOptions) { o.ScaleDownDelay = time.Hour + time.Second }, "--scale-down-delay"}, + {"scale-down-delay negative", func(o *containerCreateOptions) { o.ScaleDownDelay = -time.Second }, "--scale-down-delay"}, + {"request-ttl max", func(o *containerCreateOptions) { o.RequestTTL = 24 * time.Hour }, ""}, + {"request-ttl over", func(o *containerCreateOptions) { o.RequestTTL = 24*time.Hour + time.Second }, "--request-ttl"}, + {"healthcheck-port over", func(o *containerCreateOptions) { o.HealthcheckPort = 65536 }, "--healthcheck-port"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + opts := validOpts() + c.mutate(opts) + req, err := opts.request() + if c.errFlag != "" { + if err == nil || !strings.Contains(err.Error(), c.errFlag) { + t.Fatalf("want an error naming %s, got %v", c.errFlag, err) + } + return + } + if err != nil { + t.Fatalf("request: %v", err) + } + if err := req.Validate(); err != nil { + t.Fatalf("CLI accepts but SDK rejects: %v", err) + } + }) + } +} + +func TestBatchjobFlagsMatchSDKLimits(t *testing.T) { + cases := []struct { + name string + mutate func(*batchjobCreateOptions) + errFlag string + }{ + {"compute-size max", func(o *batchjobCreateOptions) { o.ComputeSize = 128 }, ""}, + {"compute-size over", func(o *batchjobCreateOptions) { o.ComputeSize = 129 }, "--compute-size"}, + {"max-replicas max", func(o *batchjobCreateOptions) { o.MaxReplicas = 1000 }, ""}, + {"max-replicas over", func(o *batchjobCreateOptions) { o.MaxReplicas = 1001 }, "--max-replicas"}, + {"request-ttl max", func(o *batchjobCreateOptions) { o.RequestTTL = 168 * time.Hour }, ""}, + {"request-ttl over", func(o *batchjobCreateOptions) { o.RequestTTL = 168*time.Hour + time.Second }, "--request-ttl"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + opts := validJobOpts() + c.mutate(opts) + req, err := opts.request() + if c.errFlag != "" { + if err == nil || !strings.Contains(err.Error(), c.errFlag) { + t.Fatalf("want an error naming %s, got %v", c.errFlag, err) + } + return + } + if err != nil { + t.Fatalf("request: %v", err) + } + if err := req.Validate(); err != nil { + t.Fatalf("CLI accepts but SDK rejects: %v", err) + } + }) + } +} diff --git a/internal/verda-cli/cmd/serverless/shared.go b/internal/verda-cli/cmd/serverless/shared.go index df91644..654c371 100644 --- a/internal/verda-cli/cmd/serverless/shared.go +++ b/internal/verda-cli/cmd/serverless/shared.go @@ -20,6 +20,7 @@ import ( "fmt" "regexp" "strings" + "time" "charm.land/lipgloss/v2" "github.com/verda-cloud/verda-cli/pkg/tui" @@ -28,6 +29,55 @@ import ( cmdutil "github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/util" ) +// Create-request limits. The SDK rejects values outside them before sending; +// sdk_limits_test.go checks these ranges against the SDK's Validate(). +const ( + maxPort = 65535 + maxComputeSize = 128 + maxReplicas = 1000 + maxConcurrency = 5000 + minUtilThreshold = 5 + maxUtilThreshold = 100 + maxScalingDelay = time.Hour + maxContainerRequestTTL = 24 * time.Hour + maxBatchjobRequestTTL = 7 * 24 * time.Hour +) + +func checkIntRange(name string, n, lo, hi int) error { + if n < lo || n > hi { + return fmt.Errorf("%s must be in %d..%d", name, lo, hi) + } + return nil +} + +// checkUtilThreshold: 0 disables the trigger. +func checkUtilThreshold(name string, n int) error { + if n != 0 && (n < minUtilThreshold || n > maxUtilThreshold) { + return fmt.Errorf("%s must be 0 (off) or in %d..%d", name, minUtilThreshold, maxUtilThreshold) + } + return nil +} + +// checkDurationRange compares whole seconds, which is what the request carries. +func checkDurationRange(name string, d, hi time.Duration) error { + if d < 0 || int(d.Seconds()) > int(hi.Seconds()) { + return fmt.Errorf("%s must be between 0s and %s", name, shortDuration(hi)) + } + return nil +} + +// shortDuration drops zero tails: 1h0m0s -> 1h, 10m0s -> 10m. +func shortDuration(d time.Duration) string { + s := d.String() + if strings.HasSuffix(s, "m0s") { + s = s[:len(s)-2] + } + if strings.HasSuffix(s, "h0m") { + s = s[:len(s)-2] + } + return s +} + // deploymentNameRE: lowercase DNS-label subset (max 63) enforced by the API for URL slugs. var deploymentNameRE = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`) diff --git a/internal/verda-cli/cmd/serverless/wizard.go b/internal/verda-cli/cmd/serverless/wizard.go index 6381077..8a63112 100644 --- a/internal/verda-cli/cmd/serverless/wizard.go +++ b/internal/verda-cli/cmd/serverless/wizard.go @@ -17,6 +17,7 @@ package serverless import ( "context" "errors" + "fmt" "strconv" "strings" "time" @@ -70,7 +71,7 @@ func buildContainerCreateFlow(_ context.Context, getClient clientFunc, opts *con stepContainerGPUUtil(&opts.GPUUtil), stepContainerScaleUpDelay(&opts.ScaleUpDelay), stepContainerScaleDownDelay(&opts.ScaleDownDelay), - stepRequestTTL(&opts.RequestTTL), + stepContainerRequestTTL(&opts.RequestTTL), stepSecretMounts(getClient, cache, &opts.SecretMounts), }, } @@ -171,7 +172,7 @@ func stepContainerMinReplicas(target *int) wizard.Step { Prompt: wizard.TextInputPrompt, Required: false, Default: func(_ map[string]any) any { return strconv.Itoa(*target) }, - Validate: parseNonNegativeIntValidator("min replicas"), + Validate: parseIntRangeValidator("min replicas", 0, maxReplicas), Setter: func(v any) { n, _ := strconv.Atoi(strings.TrimSpace(v.(string))) *target = n @@ -191,7 +192,7 @@ func stepContainerConcurrency(target *int) wizard.Step { Prompt: wizard.TextInputPrompt, Required: true, Default: func(_ map[string]any) any { return strconv.Itoa(*target) }, - Validate: parsePositiveIntValidator("concurrency"), + Validate: parseIntRangeValidator("concurrency", 1, maxConcurrency), Setter: func(v any) { n, _ := strconv.Atoi(strings.TrimSpace(v.(string))) *target = n @@ -268,12 +269,12 @@ func stepContainerGPUUtil(target *int) wizard.Step { } // utilThresholdStep builds a step that asks "off | " as a text -// input. An empty value or "off" maps to 0; any integer in 1..100 enables +// input. An empty value or "off" maps to 0; any integer in 5..100 enables // the trigger at that threshold. Shared by CPU and GPU util steps. func utilThresholdStep(name, desc string, target *int) wizard.Step { return wizard.Step{ Name: name, - Description: desc + " (blank = off; else 1..100)", + Description: fmt.Sprintf("%s (blank = off; else %d..%d)", desc, minUtilThreshold, maxUtilThreshold), Prompt: wizard.TextInputPrompt, Required: false, Default: func(_ map[string]any) any { @@ -288,8 +289,8 @@ func utilThresholdStep(name, desc string, target *int) wizard.Step { return nil } n, err := strconv.Atoi(s) - if err != nil || n < 1 || n > 100 { - return errors.New("must be blank, 'off', or an integer in 1..100") + if err != nil || n < minUtilThreshold || n > maxUtilThreshold { + return fmt.Errorf("must be blank, 'off', or an integer in %d..%d", minUtilThreshold, maxUtilThreshold) } return nil }, @@ -316,9 +317,9 @@ func utilThresholdStep(name, desc string, target *int) wizard.Step { // --- Scale-up / scale-down delays --- func stepContainerScaleUpDelay(target *time.Duration) wizard.Step { - return durationStep("scale-up-delay", "Scale-up delay", target, 0) + return durationStep("scale-up-delay", "Scale-up delay", target, 0, maxScalingDelay) } func stepContainerScaleDownDelay(target *time.Duration) wizard.Step { - return durationStep("scale-down-delay", "Scale-down delay", target, defaultScaleDownDelay) + return durationStep("scale-down-delay", "Scale-down delay", target, defaultScaleDownDelay, maxScalingDelay) } diff --git a/internal/verda-cli/cmd/serverless/wizard_batchjob.go b/internal/verda-cli/cmd/serverless/wizard_batchjob.go index 1c8ea84..42f06c1 100644 --- a/internal/verda-cli/cmd/serverless/wizard_batchjob.go +++ b/internal/verda-cli/cmd/serverless/wizard_batchjob.go @@ -43,7 +43,7 @@ func buildBatchjobCreateFlow(_ context.Context, getClient clientFunc, opts *batc stepEnvVars(&opts.Env), stepMaxReplicas(&opts.MaxReplicas), stepBatchjobDeadline(&opts.Deadline), - stepRequestTTL(&opts.RequestTTL), + stepBatchjobRequestTTL(&opts.RequestTTL), stepSecretMounts(getClient, cache, &opts.SecretMounts), }, } diff --git a/internal/verda-cli/cmd/serverless/wizard_shared.go b/internal/verda-cli/cmd/serverless/wizard_shared.go index 40c1e2b..5fc605f 100644 --- a/internal/verda-cli/cmd/serverless/wizard_shared.go +++ b/internal/verda-cli/cmd/serverless/wizard_shared.go @@ -129,7 +129,7 @@ func stepComputeSize(target *int) wizard.Step { } return "1" }, - Validate: parsePositiveIntValidator("compute size"), + Validate: parseIntRangeValidator("compute size", 1, maxComputeSize), Setter: func(v any) { n, _ := strconv.Atoi(strings.TrimSpace(v.(string))) *target = n @@ -203,7 +203,7 @@ func stepPort(target *int) wizard.Step { Prompt: wizard.TextInputPrompt, Required: true, Default: func(_ map[string]any) any { return strconv.Itoa(*target) }, - Validate: parsePortValidator("port"), + Validate: parseIntRangeValidator("port", 1, maxPort), Setter: func(v any) { n, _ := strconv.Atoi(strings.TrimSpace(v.(string))) *target = n @@ -266,7 +266,7 @@ func stepMaxReplicas(target *int) wizard.Step { Prompt: wizard.TextInputPrompt, Required: true, Default: func(_ map[string]any) any { return strconv.Itoa(*target) }, - Validate: parsePositiveIntValidator("max replicas"), + Validate: parseIntRangeValidator("max replicas", 1, maxReplicas), Setter: func(v any) { n, _ := strconv.Atoi(strings.TrimSpace(v.(string))) *target = n @@ -279,8 +279,13 @@ func stepMaxReplicas(target *int) wizard.Step { // --- Request TTL --- -func stepRequestTTL(target *time.Duration) wizard.Step { - return durationStep("request-ttl", "Request time-to-live (pending queue)", target, defaultRequestTTL) +// Container and job queues have different TTL ceilings. +func stepContainerRequestTTL(target *time.Duration) wizard.Step { + return durationStep("request-ttl", "Request time-to-live (pending queue)", target, defaultRequestTTL, maxContainerRequestTTL) +} + +func stepBatchjobRequestTTL(target *time.Duration) wizard.Step { + return durationStep("request-ttl", "Request time-to-live (pending queue)", target, defaultRequestTTL, maxBatchjobRequestTTL) } // --- Secret mounts (loop) --- @@ -334,10 +339,10 @@ func stepSecretMounts(getClient clientFunc, cache *apiCache, target *[]string) w // durationStep builds a TextInput step that parses a Go duration (0s, 300s, // 5m, ...). Empty input resets to def. -func durationStep(name, desc string, target *time.Duration, def time.Duration) wizard.Step { +func durationStep(name, desc string, target *time.Duration, def, maxDur time.Duration) wizard.Step { return wizard.Step{ Name: name, - Description: desc + " (e.g. 0s, 300s, 5m)", + Description: desc + " (0s to " + shortDuration(maxDur) + ", e.g. 0s, 300s, 5m)", Prompt: wizard.TextInputPrompt, Required: false, Default: func(_ map[string]any) any { @@ -352,10 +357,10 @@ func durationStep(name, desc string, target *time.Duration, def time.Duration) w return nil } d, err := time.ParseDuration(s) - if err != nil || d < 0 { - return errors.New("must be a non-negative duration (e.g. 0s, 300s, 5m)") + if err != nil { + return errors.New("must be a duration (e.g. 0s, 300s, 5m)") } - return nil + return checkDurationRange(name, d, maxDur) }, Setter: func(v any) { s := strings.TrimSpace(v.(string)) @@ -374,31 +379,11 @@ func durationStep(name, desc string, target *time.Duration, def time.Duration) w // --- Validators (shared) --- -func parsePositiveIntValidator(field string) func(any) error { - return func(v any) error { - n, err := strconv.Atoi(strings.TrimSpace(v.(string))) - if err != nil || n < 1 { - return fmt.Errorf("%s must be a positive integer", field) - } - return nil - } -} - -func parseNonNegativeIntValidator(field string) func(any) error { - return func(v any) error { - n, err := strconv.Atoi(strings.TrimSpace(v.(string))) - if err != nil || n < 0 { - return fmt.Errorf("%s must be an integer >= 0", field) - } - return nil - } -} - -func parsePortValidator(field string) func(any) error { +func parseIntRangeValidator(field string, lo, hi int) func(any) error { return func(v any) error { n, err := strconv.Atoi(strings.TrimSpace(v.(string))) - if err != nil || n < 1 || n > 65535 { - return fmt.Errorf("%s must be an integer in 1..65535", field) + if err != nil || n < lo || n > hi { + return fmt.Errorf("%s must be an integer in %d..%d", field, lo, hi) } return nil } From 54760c1f65c6f8859cffa721c776243509a125d1 Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Wed, 28 Oct 2026 17:41:14 +0200 Subject: [PATCH 09/11] test(serverless): drop sdk_limits_test to keep the PR small Co-Authored-By: Claude Opus 5.5 --- internal/verda-cli/cmd/serverless/CLAUDE.md | 2 +- .../cmd/serverless/sdk_limits_test.go | 199 ------------------ internal/verda-cli/cmd/serverless/shared.go | 3 +- 3 files changed, 2 insertions(+), 202 deletions(-) delete mode 100644 internal/verda-cli/cmd/serverless/sdk_limits_test.go diff --git a/internal/verda-cli/cmd/serverless/CLAUDE.md b/internal/verda-cli/cmd/serverless/CLAUDE.md index fc80182..195d5d1 100644 --- a/internal/verda-cli/cmd/serverless/CLAUDE.md +++ b/internal/verda-cli/cmd/serverless/CLAUDE.md @@ -101,7 +101,7 @@ Mount types in `ContainerVolumeMount.Type` that the CLI currently sends: ### Create limits mirror the SDK -The SDK validates create requests before sending them, so a wizard step that accepts a wider range than the SDK fails only after the whole wizard. The limits live once in `shared.go` (`maxComputeSize`, `maxReplicas`, `maxConcurrency`, `min/maxUtilThreshold`, `maxScalingDelay`, `maxContainerRequestTTL` 24h, `maxBatchjobRequestTTL` 168h) and back the wizard validators, the `validate()`/`request()` flag checks and the flag help. Container and job request TTL differ — keep `stepContainerRequestTTL` and `stepBatchjobRequestTTL` separate. `sdk_limits_test.go` runs the SDK's `Validate()` at each boundary; when the SDK changes a limit, that test fails first. +The SDK validates create requests before sending them, so a wizard step that accepts a wider range than the SDK fails only after the whole wizard. The limits live once in `shared.go` (`maxComputeSize`, `maxReplicas`, `maxConcurrency`, `min/maxUtilThreshold`, `maxScalingDelay`, `maxContainerRequestTTL` 24h, `maxBatchjobRequestTTL` 168h) and back the wizard validators, the `validate()`/`request()` flag checks and the flag help. Container and job request TTL differ — keep `stepContainerRequestTTL` and `stepBatchjobRequestTTL` separate. When the SDK changes a limit, update the constant here. ### Action-command factory pattern diff --git a/internal/verda-cli/cmd/serverless/sdk_limits_test.go b/internal/verda-cli/cmd/serverless/sdk_limits_test.go deleted file mode 100644 index ee99d8d..0000000 --- a/internal/verda-cli/cmd/serverless/sdk_limits_test.go +++ /dev/null @@ -1,199 +0,0 @@ -// Copyright 2026 Verda Cloud Oy -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package serverless - -import ( - "strings" - "testing" - "time" - - "github.com/verda-cloud/verda-cli/pkg/tui/wizard" -) - -// The SDK validates create requests before sending them. A wizard step that -// accepts a value the SDK rejects fails only after the whole wizard, so every -// step must accept exactly the SDK's range. - -type limitInput struct { - input string - ok bool -} - -func TestContainerWizardStepsMatchSDKLimits(t *testing.T) { - utilInputs := []limitInput{{"", true}, {"off", true}, {"4", false}, {"5", true}, {"100", true}, {"101", false}} - delayInputs := []limitInput{{"", true}, {"0s", true}, {"1h", true}, {"1h1s", false}, {"-1s", false}} - steps := []struct { - name string - step func(*containerCreateOptions) wizard.Step - inputs []limitInput - }{ - {"compute-size", func(o *containerCreateOptions) wizard.Step { return stepComputeSize(&o.ComputeSize) }, - []limitInput{{"0", false}, {"1", true}, {"128", true}, {"129", false}}}, - {"min-replicas", func(o *containerCreateOptions) wizard.Step { return stepContainerMinReplicas(&o.MinReplicas) }, - []limitInput{{"-1", false}, {"0", true}, {"3", true}, {"1001", false}}}, - {"max-replicas", func(o *containerCreateOptions) wizard.Step { return stepMaxReplicas(&o.MaxReplicas) }, - []limitInput{{"0", false}, {"1", true}, {"1000", true}, {"1001", false}}}, - {"concurrency", func(o *containerCreateOptions) wizard.Step { return stepContainerConcurrency(&o.Concurrency) }, - []limitInput{{"0", false}, {"1", true}, {"5000", true}, {"5001", false}}}, - {"cpu-util", func(o *containerCreateOptions) wizard.Step { return stepContainerCPUUtil(&o.CPUUtil) }, utilInputs}, - {"gpu-util", func(o *containerCreateOptions) wizard.Step { return stepContainerGPUUtil(&o.GPUUtil) }, utilInputs}, - {"scale-up-delay", func(o *containerCreateOptions) wizard.Step { return stepContainerScaleUpDelay(&o.ScaleUpDelay) }, delayInputs}, - {"scale-down-delay", func(o *containerCreateOptions) wizard.Step { return stepContainerScaleDownDelay(&o.ScaleDownDelay) }, delayInputs}, - {"request-ttl", func(o *containerCreateOptions) wizard.Step { return stepContainerRequestTTL(&o.RequestTTL) }, - []limitInput{{"", true}, {"0s", true}, {"24h", true}, {"24h1s", false}}}, - } - for _, s := range steps { - for _, in := range s.inputs { - t.Run(s.name+"/"+in.input, func(t *testing.T) { - opts := validOpts() - step := s.step(opts) - if err := step.Validate(in.input); (err == nil) != in.ok { - t.Fatalf("wizard Validate(%q) = %v, want ok=%v", in.input, err, in.ok) - } - if !in.ok { - return - } - step.Setter(in.input) - req, err := opts.request() - if err != nil { - t.Fatalf("wizard accepts %q but request() rejects it: %v", in.input, err) - } - if err := req.Validate(); err != nil { - t.Fatalf("wizard accepts %q but SDK rejects it: %v", in.input, err) - } - }) - } - } -} - -func TestBatchjobWizardStepsMatchSDKLimits(t *testing.T) { - steps := []struct { - name string - step func(*batchjobCreateOptions) wizard.Step - inputs []limitInput - }{ - {"compute-size", func(o *batchjobCreateOptions) wizard.Step { return stepComputeSize(&o.ComputeSize) }, - []limitInput{{"0", false}, {"1", true}, {"128", true}, {"129", false}}}, - {"max-replicas", func(o *batchjobCreateOptions) wizard.Step { return stepMaxReplicas(&o.MaxReplicas) }, - []limitInput{{"0", false}, {"1", true}, {"1000", true}, {"1001", false}}}, - {"request-ttl", func(o *batchjobCreateOptions) wizard.Step { return stepBatchjobRequestTTL(&o.RequestTTL) }, - []limitInput{{"", true}, {"0s", true}, {"168h", true}, {"168h1s", false}}}, - } - for _, s := range steps { - for _, in := range s.inputs { - t.Run(s.name+"/"+in.input, func(t *testing.T) { - opts := validJobOpts() - step := s.step(opts) - if err := step.Validate(in.input); (err == nil) != in.ok { - t.Fatalf("wizard Validate(%q) = %v, want ok=%v", in.input, err, in.ok) - } - if !in.ok { - return - } - step.Setter(in.input) - req, err := opts.request() - if err != nil { - t.Fatalf("wizard accepts %q but request() rejects it: %v", in.input, err) - } - if err := req.Validate(); err != nil { - t.Fatalf("wizard accepts %q but SDK rejects it: %v", in.input, err) - } - }) - } - } -} - -// Flag-only and agent runs reach the same limits through request(). The CLI -// checks them first so the error names the flag and its range. -func TestContainerFlagsMatchSDKLimits(t *testing.T) { - cases := []struct { - name string - mutate func(*containerCreateOptions) - errFlag string // empty: request() and the SDK must accept - }{ - {"compute-size max", func(o *containerCreateOptions) { o.ComputeSize = 128 }, ""}, - {"compute-size over", func(o *containerCreateOptions) { o.ComputeSize = 129 }, "--compute-size"}, - {"replicas max", func(o *containerCreateOptions) { o.MinReplicas, o.MaxReplicas = 1000, 1000 }, ""}, - {"min-replicas over", func(o *containerCreateOptions) { o.MinReplicas, o.MaxReplicas = 1001, 1001 }, "--min-replicas"}, - {"max-replicas over", func(o *containerCreateOptions) { o.MaxReplicas = 1001 }, "--max-replicas"}, - {"concurrency max", func(o *containerCreateOptions) { o.Concurrency = 5000 }, ""}, - {"concurrency over", func(o *containerCreateOptions) { o.Concurrency = 5001 }, "--concurrency"}, - {"cpu-util off", func(o *containerCreateOptions) { o.CPUUtil = 0 }, ""}, - {"cpu-util min", func(o *containerCreateOptions) { o.CPUUtil = 5 }, ""}, - {"cpu-util under", func(o *containerCreateOptions) { o.CPUUtil = 4 }, "--cpu-util"}, - {"gpu-util max", func(o *containerCreateOptions) { o.GPUUtil = 100 }, ""}, - {"gpu-util under", func(o *containerCreateOptions) { o.GPUUtil = 1 }, "--gpu-util"}, - {"scale-up-delay max", func(o *containerCreateOptions) { o.ScaleUpDelay = time.Hour }, ""}, - {"scale-up-delay over", func(o *containerCreateOptions) { o.ScaleUpDelay = time.Hour + time.Second }, "--scale-up-delay"}, - {"scale-down-delay over", func(o *containerCreateOptions) { o.ScaleDownDelay = time.Hour + time.Second }, "--scale-down-delay"}, - {"scale-down-delay negative", func(o *containerCreateOptions) { o.ScaleDownDelay = -time.Second }, "--scale-down-delay"}, - {"request-ttl max", func(o *containerCreateOptions) { o.RequestTTL = 24 * time.Hour }, ""}, - {"request-ttl over", func(o *containerCreateOptions) { o.RequestTTL = 24*time.Hour + time.Second }, "--request-ttl"}, - {"healthcheck-port over", func(o *containerCreateOptions) { o.HealthcheckPort = 65536 }, "--healthcheck-port"}, - } - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - opts := validOpts() - c.mutate(opts) - req, err := opts.request() - if c.errFlag != "" { - if err == nil || !strings.Contains(err.Error(), c.errFlag) { - t.Fatalf("want an error naming %s, got %v", c.errFlag, err) - } - return - } - if err != nil { - t.Fatalf("request: %v", err) - } - if err := req.Validate(); err != nil { - t.Fatalf("CLI accepts but SDK rejects: %v", err) - } - }) - } -} - -func TestBatchjobFlagsMatchSDKLimits(t *testing.T) { - cases := []struct { - name string - mutate func(*batchjobCreateOptions) - errFlag string - }{ - {"compute-size max", func(o *batchjobCreateOptions) { o.ComputeSize = 128 }, ""}, - {"compute-size over", func(o *batchjobCreateOptions) { o.ComputeSize = 129 }, "--compute-size"}, - {"max-replicas max", func(o *batchjobCreateOptions) { o.MaxReplicas = 1000 }, ""}, - {"max-replicas over", func(o *batchjobCreateOptions) { o.MaxReplicas = 1001 }, "--max-replicas"}, - {"request-ttl max", func(o *batchjobCreateOptions) { o.RequestTTL = 168 * time.Hour }, ""}, - {"request-ttl over", func(o *batchjobCreateOptions) { o.RequestTTL = 168*time.Hour + time.Second }, "--request-ttl"}, - } - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - opts := validJobOpts() - c.mutate(opts) - req, err := opts.request() - if c.errFlag != "" { - if err == nil || !strings.Contains(err.Error(), c.errFlag) { - t.Fatalf("want an error naming %s, got %v", c.errFlag, err) - } - return - } - if err != nil { - t.Fatalf("request: %v", err) - } - if err := req.Validate(); err != nil { - t.Fatalf("CLI accepts but SDK rejects: %v", err) - } - }) - } -} diff --git a/internal/verda-cli/cmd/serverless/shared.go b/internal/verda-cli/cmd/serverless/shared.go index 654c371..b08a99c 100644 --- a/internal/verda-cli/cmd/serverless/shared.go +++ b/internal/verda-cli/cmd/serverless/shared.go @@ -29,8 +29,7 @@ import ( cmdutil "github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/util" ) -// Create-request limits. The SDK rejects values outside them before sending; -// sdk_limits_test.go checks these ranges against the SDK's Validate(). +// Create-request limits; keep in sync with the SDK, which rejects values outside them before sending. const ( maxPort = 65535 maxComputeSize = 128 From dc629f1ccf27a42556dc511e3cee1ea4c56f1291 Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Wed, 28 Oct 2026 17:48:21 +0200 Subject: [PATCH 10/11] refactor(serverless): read limits from consts and keep existing messages Restore the original help text, wizard descriptions and error messages, and the serverless README. Only the new upper-bound checks and the two ranges that changed (CPU/GPU util, batch job deadline) produce new text, and that text is built from the constants in shared.go and batchjob_create.go, so a limit changes in one place. Co-Authored-By: Claude Opus 5.5 --- internal/verda-cli/cmd/serverless/CLAUDE.md | 10 ++-- internal/verda-cli/cmd/serverless/README.md | 16 +++---- .../cmd/serverless/batchjob_create.go | 38 ++++++++++----- .../cmd/serverless/batchjob_create_test.go | 47 ------------------- .../cmd/serverless/container_create.go | 46 +++++++++++------- internal/verda-cli/cmd/serverless/shared.go | 13 +++-- internal/verda-cli/cmd/serverless/wizard.go | 8 ++-- .../cmd/serverless/wizard_batchjob.go | 16 +++---- .../verda-cli/cmd/serverless/wizard_shared.go | 47 ++++++++++++------- 9 files changed, 113 insertions(+), 128 deletions(-) diff --git a/internal/verda-cli/cmd/serverless/CLAUDE.md b/internal/verda-cli/cmd/serverless/CLAUDE.md index 195d5d1..9e2f3e0 100644 --- a/internal/verda-cli/cmd/serverless/CLAUDE.md +++ b/internal/verda-cli/cmd/serverless/CLAUDE.md @@ -36,7 +36,7 @@ This rule applies equally to Claude, Codex, Cursor, or a human editing the packa - `batchjob_create.go` — `batchjobCreateOptions` (simpler: no spot, deadline required). - `batchjob_list.go`, `batchjob_describe.go`, `batchjob_delete.go`, `batchjob_actions.go` — Same shape as container, trimmed. - `shared.go` — `validateDeploymentName` (RFC-1123 subset), `rejectLatestTag`, `parseEnvFlag`, `parseSecretMountFlag`, `confirmDestructive`, `statusColor`, `mountType*` + `envType*` constants. - - `wizard_shared.go` — Step builders shared by both create wizards: `stepName`, `stepImage`, `stepCompute`, `stepComputeSize`, `stepRegistryCreds`, `stepPort`, `stepEnvVars`, `stepMaxReplicas`, `stepContainerRequestTTL` / `stepBatchjobRequestTTL`, `stepSecretMounts`. Plus the generic `durationStep` helper (bounded by a max) and `parseIntRangeValidator`. Each takes a `*T` pointer to the field it mutates, so the same step definition drives both `containerCreateOptions` and `batchjobCreateOptions`. + - `wizard_shared.go` — Step builders shared by both create wizards: `stepName`, `stepImage`, `stepCompute`, `stepComputeSize`, `stepRegistryCreds`, `stepPort`, `stepEnvVars`, `stepMaxReplicas`, `stepRequestTTL`, `stepSecretMounts`. Plus the generic `durationStep` helper and three int validators. Each takes a `*T` pointer to the field it mutates, so the same step definition drives both `containerCreateOptions` and `batchjobCreateOptions`. - `wizard.go` — `buildContainerCreateFlow` + container-only steps: spot/compute-type, healthcheck on/off + path (port is NOT prompted; it always defaults to the exposed port in `request()`), min-replicas, concurrency, queue-load preset + custom override, CPU/GPU util triggers, scale-up/down delays. 21 total steps in the container flow. - `wizard_batchjob.go` — `buildBatchjobCreateFlow` + `stepBatchjobDeadline`. 11 total steps: 10 reused from `wizard_shared.go` + the batchjob-only deadline. Jobs have no spot, no min replicas, no scaling triggers, no healthcheck, no concurrency — all of those steps are simply absent from the flow. - `wizard_cache.go` — `apiCache` with lazy loaders for compute resources, registry creds, secrets, file secrets. Shared across wizard passes so back-navigation doesn't re-hit the API. Used by both container and batchjob wizards. @@ -97,11 +97,7 @@ Mount types in `ContainerVolumeMount.Type` that the CLI currently sends: ### Deadline is required for batchjob -`JobScalingOptions.DeadlineSeconds` must be 600..604800 (10m..7 days). The SDK enforces this in `JobScalingOptions.Validate` (run by `CreateJobDeployment`) before the request is sent. The CLI mirrors it in `validateBatchjobDeadline`, used by both `request()` and the wizard's deadline step, so a bad value fails at the prompt, not after the whole wizard. A missing deadline is listed in `missingBatchjobCreateFlags`. The wizard default (`30m`) must stay inside the range — `TestBatchjobWizardDefaultDeadline_PassesSDKValidation` guards it. - -### Create limits mirror the SDK - -The SDK validates create requests before sending them, so a wizard step that accepts a wider range than the SDK fails only after the whole wizard. The limits live once in `shared.go` (`maxComputeSize`, `maxReplicas`, `maxConcurrency`, `min/maxUtilThreshold`, `maxScalingDelay`, `maxContainerRequestTTL` 24h, `maxBatchjobRequestTTL` 168h) and back the wizard validators, the `validate()`/`request()` flag checks and the flag help. Container and job request TTL differ — keep `stepContainerRequestTTL` and `stepBatchjobRequestTTL` separate. When the SDK changes a limit, update the constant here. +`JobScalingOptions.DeadlineSeconds` has a range that the SDK enforces in `JobScalingOptions.Validate` (run by `CreateJobDeployment`) before the request is sent. The CLI mirrors it with `minBatchjobDeadline` / `maxBatchjobDeadline` in `validateBatchjobDeadline`, used by both `request()` and the wizard's deadline step, so a bad value fails at the prompt, not after the whole wizard. A missing deadline is listed in `missingBatchjobCreateFlags`. `defaultBatchjobDeadline` must stay inside the range — `TestBatchjobWizardDefaultDeadline_PassesSDKValidation` guards it. ### Action-command factory pattern @@ -122,7 +118,7 @@ Describe cards (`renderContainerDeploymentCard`, `renderJobDeploymentCard`) prin - **Wizard omits the healthcheck-path step when healthcheck is Off.** The `healthcheck-path` step has `ShouldSkip: c["healthcheck"] == "off"`; the engine wires the skip gate via `DependsOn`. There is intentionally no `healthcheck-port` wizard step — `request()` always defaults the probe port to the exposed port (`hcPort = o.Port` when `HealthcheckPort == 0`). The `--healthcheck-port` flag still works for the rare case where the probe must hit a different port than the public listener. - **`registryPublicValue = "__public__"` sentinel.** The registry-creds step's loader prepends a "Public (no credentials)" choice with this sentinel as its Value. The Setter maps the sentinel back to `opts.RegistryCreds = ""`. If you rename the sentinel, grep both sides — the Setter reads the string literal. - **`compute-size` is a separate step from `compute`.** VM's wizard combines resource + count in a single step via in-Loader prompting; serverless keeps them separate so users can go back and change the size without re-picking the resource. Lower engineering cost, same UX. -- **Util triggers off by default, but wizard asks anyway.** The CPU/GPU util steps accept empty ("off"), "off", or `5..100` (the SDK rejects an enabled threshold under 5). Setter maps empty/"off" to 0 (trigger disabled). Users should be able to Enter-through both without setting them. +- **Util triggers off by default, but wizard asks anyway.** The CPU/GPU util steps accept empty ("off"), "off", or `minUtilThreshold..maxUtilThreshold`. Setter maps empty/"off" to 0 (trigger disabled). Users should be able to Enter-through both without setting them. - **Custom queue-load is a separate step.** `queue-load-custom` has `ShouldSkip: c["queue-preset"] != "custom"`. If the user goes back and changes preset to a named one, the engine's reset logic clears the custom value via `Resetter`. - **No `+ Create new` for registry creds in the wizard.** v1 intentionally omits the inline create-new sub-flow for registry credentials — users pick existing or Public. Adding new creds requires `verda registry configure` out-of-band, or a future top-level `verda registry-creds` command. The design doc notes this as future work. - **Confirm is NOT a wizard step.** `runContainerCreate` prints the summary + runs `prompter.Confirm` after `engine.Run` returns. Keeps the review card at full terminal width and lets us pipe through `--yes` cleanly. If you move it into the wizard, you lose layout control. diff --git a/internal/verda-cli/cmd/serverless/README.md b/internal/verda-cli/cmd/serverless/README.md index c277ee9..3a61764 100644 --- a/internal/verda-cli/cmd/serverless/README.md +++ b/internal/verda-cli/cmd/serverless/README.md @@ -63,11 +63,11 @@ verda container create \ ### Other scaling flags -- `--min-replicas` (`0..1000`, default `0`, scale-to-zero) / `--max-replicas` (`1..1000`, default `3`) -- `--concurrency` (`1..5000`, default `1` — set higher for LLMs, 1 for image generation) -- `--cpu-util `, `--gpu-util ` — enable the corresponding trigger at `5..100` (`0` or blank = off) -- `--scale-up-delay`, `--scale-down-delay` (`0s` to `1h`, scale-down default `5m`) — hysteresis before scaling -- `--request-ttl` (`0s` to `24h`, default `5m`) — how long a pending request may live before the queue drops it +- `--min-replicas` (default `0`, scale-to-zero) / `--max-replicas` (default `3`) +- `--concurrency` (default `1` — set higher for LLMs, 1 for image generation) +- `--cpu-util `, `--gpu-util ` — enable the corresponding trigger (blank = off) +- `--scale-up-delay`, `--scale-down-delay` (default `5m`) — hysteresis before scaling +- `--request-ttl` (default `5m`) — how long a pending request may live before the queue drops it ### Healthcheck @@ -122,12 +122,12 @@ verda batchjob create \ **Batch jobs cannot use spot compute.** There is no `--spot` flag; the underlying API has no `IsSpot` field for jobs. This is intentional. -**Deadline is required** and must be between `10m` and `168h` (7 days). Each queued request gets up to `--deadline` to complete. A missing or out-of-range deadline fails validation client-side, before the request is sent. The wizard defaults to `30m`. +**Deadline is required** and must be `> 0s`. Each queued request gets up to `--deadline` to complete; missing or zero deadline fails validation client-side and server-side. ### Other scaling flags -- `--max-replicas` (`1..1000`, default `3`) — worker pool cap -- `--request-ttl` (`0s` to `168h`, default `5m`) — how long a pending request may live in the queue before the server drops it +- `--max-replicas` (default `3`) — worker pool cap +- `--request-ttl` (default `5m`) — how long a pending request may live in the queue before the server drops it ### Lifecycle diff --git a/internal/verda-cli/cmd/serverless/batchjob_create.go b/internal/verda-cli/cmd/serverless/batchjob_create.go index ec10809..bdfbe35 100644 --- a/internal/verda-cli/cmd/serverless/batchjob_create.go +++ b/internal/verda-cli/cmd/serverless/batchjob_create.go @@ -54,13 +54,14 @@ type batchjobCreateOptions struct { // Deadline bounds match the SDK's JobScalingOptions.Validate, which rejects the request before it is sent. const ( - minBatchjobDeadline = 10 * time.Minute - maxBatchjobDeadline = 7 * 24 * time.Hour + minBatchjobDeadline = 10 * time.Minute + maxBatchjobDeadline = 7 * 24 * time.Hour + defaultBatchjobDeadline = 30 * time.Minute ) -func validateBatchjobDeadline(d time.Duration) error { +func validateBatchjobDeadline(name string, d time.Duration) error { if d < minBatchjobDeadline || d > maxBatchjobDeadline { - return errors.New("deadline must be between 10m and 168h (7 days)") + return fmt.Errorf("%s must be between %s and %s", name, shortDuration(minBatchjobDeadline), shortDuration(maxBatchjobDeadline)) } return nil } @@ -97,7 +98,7 @@ func newCmdBatchjobCreate(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra flags.StringVar(&opts.Name, "name", "", "Deployment name (URL slug; immutable after create)") flags.StringVar(&opts.Image, "image", "", "Container image reference (must not be ':latest')") flags.StringVar(&opts.Compute, "compute", "", "Compute resource name (e.g. RTX4500Ada, CPUNode)") - flags.IntVar(&opts.ComputeSize, "compute-size", 1, "Number of GPUs or vCPU cores per replica (1..128)") + flags.IntVar(&opts.ComputeSize, "compute-size", 1, "Number of GPUs or vCPU cores per replica") flags.StringVar(&opts.RegistryCreds, "registry-creds", "", "Registry credentials name (empty = public)") flags.IntVar(&opts.Port, "port", opts.Port, "Exposed HTTP port") @@ -106,9 +107,9 @@ func newCmdBatchjobCreate(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra flags.StringArrayVar(&opts.Entrypoint, "entrypoint", nil, "Override image ENTRYPOINT; repeat for multiple args") flags.StringArrayVar(&opts.Cmd, "cmd", nil, "Override image CMD; repeat for multiple args") - flags.IntVar(&opts.MaxReplicas, "max-replicas", opts.MaxReplicas, "Maximum worker replica count (1..1000)") - flags.DurationVar(&opts.Deadline, "deadline", 0, "Per-request deadline (required; 10m to 168h)") - flags.DurationVar(&opts.RequestTTL, "request-ttl", opts.RequestTTL, "How long a pending request may live before deletion (0s to 168h)") + flags.IntVar(&opts.MaxReplicas, "max-replicas", opts.MaxReplicas, "Maximum worker replica count") + flags.DurationVar(&opts.Deadline, "deadline", 0, "Per-request deadline (required; > 0)") + flags.DurationVar(&opts.RequestTTL, "request-ttl", opts.RequestTTL, "How long a pending request may live before deletion") flags.StringArrayVar(&opts.SecretMounts, "secret-mount", nil, "Secret mount SECRET:MOUNT_PATH; repeat for multiple") @@ -204,13 +205,24 @@ func (o *batchjobCreateOptions) request() (*verda.CreateJobDeploymentRequest, er if err := rejectLatestTag(o.Image); err != nil { return nil, err } - if err := validateBatchjobDeadline(o.Deadline); err != nil { - return nil, errors.New("--" + err.Error()) + if o.ComputeSize < 1 { + return nil, errors.New("--compute-size must be >= 1") + } + if o.MaxReplicas < 1 { + return nil, errors.New("--max-replicas must be >= 1") + } + if o.Deadline <= 0 { + return nil, errors.New("--deadline must be > 0") + } + if err := validateBatchjobDeadline("--deadline", o.Deadline); err != nil { + return nil, err + } + if o.Port < 1 || o.Port > 65535 { + return nil, errors.New("--port must be in 1..65535") } for _, err := range []error{ - checkIntRange("--compute-size", o.ComputeSize, 1, maxComputeSize), - checkIntRange("--max-replicas", o.MaxReplicas, 1, maxReplicas), - checkIntRange("--port", o.Port, 1, maxPort), + checkMax("--compute-size", o.ComputeSize, maxComputeSize), + checkMax("--max-replicas", o.MaxReplicas, maxReplicas), checkDurationRange("--request-ttl", o.RequestTTL, maxBatchjobRequestTTL), } { if err != nil { diff --git a/internal/verda-cli/cmd/serverless/batchjob_create_test.go b/internal/verda-cli/cmd/serverless/batchjob_create_test.go index 5cee900..dda29d8 100644 --- a/internal/verda-cli/cmd/serverless/batchjob_create_test.go +++ b/internal/verda-cli/cmd/serverless/batchjob_create_test.go @@ -78,53 +78,6 @@ func TestBatchjobRequest_RequiresDeadline(t *testing.T) { } } -func TestBatchjobRequest_DeadlineRangeMatchesSDK(t *testing.T) { - cases := []struct { - deadline time.Duration - ok bool - }{ - {5 * time.Minute, false}, - {10*time.Minute - time.Second, false}, - {10 * time.Minute, true}, - {168 * time.Hour, true}, - {168*time.Hour + time.Second, false}, - } - for _, c := range cases { - opts := validJobOpts() - opts.Deadline = c.deadline - req, err := opts.request() - if (err == nil) != c.ok { - t.Errorf("deadline %s: request() err = %v, want ok=%v", c.deadline, err, c.ok) - continue - } - if c.ok { - if err := req.Validate(); err != nil { - t.Errorf("deadline %s: CLI accepts but SDK rejects: %v", c.deadline, err) - } - } - } -} - -func TestBatchjobWizardDefaultDeadline_PassesSDKValidation(t *testing.T) { - var d time.Duration - step := stepBatchjobDeadline(&d) - def := step.Default(nil) - if err := step.Validate(def); err != nil { - t.Fatalf("wizard rejects its own default %v: %v", def, err) - } - step.Setter(def) - - opts := validJobOpts() - opts.Deadline = d - req, err := opts.request() - if err != nil { - t.Fatalf("request: %v", err) - } - if err := req.Validate(); err != nil { - t.Fatalf("SDK rejects the wizard default deadline %s: %v", d, err) - } -} - func TestBatchjobMissingFlags(t *testing.T) { opts := &batchjobCreateOptions{} missing := missingBatchjobCreateFlags(opts) diff --git a/internal/verda-cli/cmd/serverless/container_create.go b/internal/verda-cli/cmd/serverless/container_create.go index ff7b323..58219b4 100644 --- a/internal/verda-cli/cmd/serverless/container_create.go +++ b/internal/verda-cli/cmd/serverless/container_create.go @@ -138,30 +138,30 @@ func newCmdContainerCreate(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobr flags.BoolVar(&opts.Spot, "spot", false, "Use spot compute instead of on-demand") flags.StringVar(&opts.Compute, "compute", "", "Compute resource name (e.g. RTX4500Ada, CPUNode)") - flags.IntVar(&opts.ComputeSize, "compute-size", 1, "Number of GPUs or vCPU cores per replica (1..128)") + flags.IntVar(&opts.ComputeSize, "compute-size", 1, "Number of GPUs or vCPU cores per replica") flags.StringVar(&opts.RegistryCreds, "registry-creds", "", "Registry credentials name (empty = public)") flags.BoolVar(&opts.RegistryPublic, "registry-public", false, "Pull image anonymously (default)") flags.IntVar(&opts.Port, "port", opts.Port, "Exposed HTTP port") flags.BoolVar(&opts.HealthcheckOff, "healthcheck-off", false, "Disable healthcheck (default: on at /health)") - flags.IntVar(&opts.HealthcheckPort, "healthcheck-port", 0, "Healthcheck HTTP port (1..65535; defaults to --port)") + flags.IntVar(&opts.HealthcheckPort, "healthcheck-port", 0, "Healthcheck HTTP port (defaults to --port)") flags.StringVar(&opts.HealthcheckPath, "healthcheck-path", opts.HealthcheckPath, "Healthcheck HTTP path") flags.StringArrayVar(&opts.Env, "env", nil, "Environment variable KEY=VALUE (KEY uppercase); repeat for multiple") flags.StringArrayVar(&opts.EnvSecret, "env-secret", nil, "Secret-backed env KEY=SECRET_NAME (KEY uppercase); repeat for multiple") flags.StringArrayVar(&opts.Entrypoint, "entrypoint", nil, "Override image ENTRYPOINT; repeat for multiple args") flags.StringArrayVar(&opts.Cmd, "cmd", nil, "Override image CMD; repeat for multiple args") - flags.IntVar(&opts.MinReplicas, "min-replicas", opts.MinReplicas, "Minimum replica count (0..1000; 0 = scale-to-zero)") - flags.IntVar(&opts.MaxReplicas, "max-replicas", opts.MaxReplicas, "Maximum replica count (1..1000)") - flags.IntVar(&opts.Concurrency, "concurrency", opts.Concurrency, "Concurrent requests per replica (1..5000)") + flags.IntVar(&opts.MinReplicas, "min-replicas", opts.MinReplicas, "Minimum replica count (0 = scale-to-zero)") + flags.IntVar(&opts.MaxReplicas, "max-replicas", opts.MaxReplicas, "Maximum replica count") + flags.IntVar(&opts.Concurrency, "concurrency", opts.Concurrency, "Concurrent requests per replica") flags.StringVar(&opts.QueuePreset, "queue-preset", opts.QueuePreset, "Scaling preset: instant | balanced | cost-saver | custom") flags.IntVar(&opts.QueueLoad, "queue-load", 0, "Custom queue-load threshold (1..1000); sets --queue-preset=custom when used") - flags.IntVar(&opts.CPUUtil, "cpu-util", 0, "CPU utilization trigger threshold % (5..100); 0 = off") - flags.IntVar(&opts.GPUUtil, "gpu-util", 0, "GPU utilization trigger threshold % (5..100); 0 = off") - flags.DurationVar(&opts.ScaleUpDelay, "scale-up-delay", opts.ScaleUpDelay, "Delay before scaling up (0s to 1h)") - flags.DurationVar(&opts.ScaleDownDelay, "scale-down-delay", opts.ScaleDownDelay, "Delay before scaling down (0s to 1h)") - flags.DurationVar(&opts.RequestTTL, "request-ttl", opts.RequestTTL, "How long a pending request may live before deletion (0s to 24h)") + flags.IntVar(&opts.CPUUtil, "cpu-util", 0, fmt.Sprintf("CPU utilization trigger threshold %% (%d..%d); 0 = off", minUtilThreshold, maxUtilThreshold)) + flags.IntVar(&opts.GPUUtil, "gpu-util", 0, fmt.Sprintf("GPU utilization trigger threshold %% (%d..%d); 0 = off", minUtilThreshold, maxUtilThreshold)) + flags.DurationVar(&opts.ScaleUpDelay, "scale-up-delay", opts.ScaleUpDelay, "Delay before scaling up") + flags.DurationVar(&opts.ScaleDownDelay, "scale-down-delay", opts.ScaleDownDelay, "Delay before scaling down") + flags.DurationVar(&opts.RequestTTL, "request-ttl", opts.RequestTTL, "How long a pending request may live before deletion") flags.StringArrayVar(&opts.SecretMounts, "secret-mount", nil, "Secret mount SECRET:MOUNT_PATH; repeat for multiple") @@ -256,16 +256,26 @@ func (o *containerCreateOptions) validate() error { if err := rejectLatestTag(o.Image); err != nil { return err } - if o.MaxReplicas < o.MinReplicas { - return errors.New("--max-replicas must be >= --min-replicas") + if o.ComputeSize < 1 { + return errors.New("--compute-size must be >= 1") + } + if o.MinReplicas < 0 { + return errors.New("--min-replicas must be >= 0") + } + if o.MaxReplicas < 1 || o.MaxReplicas < o.MinReplicas { + return errors.New("--max-replicas must be >= max(1, --min-replicas)") + } + if o.Concurrency < 1 { + return errors.New("--concurrency must be >= 1") + } + if o.Port < 1 || o.Port > 65535 { + return errors.New("--port must be in 1..65535") } for _, err := range []error{ - checkIntRange("--compute-size", o.ComputeSize, 1, maxComputeSize), - checkIntRange("--min-replicas", o.MinReplicas, 0, maxReplicas), - checkIntRange("--max-replicas", o.MaxReplicas, 1, maxReplicas), - checkIntRange("--concurrency", o.Concurrency, 1, maxConcurrency), - checkIntRange("--port", o.Port, 1, maxPort), - checkIntRange("--healthcheck-port", o.HealthcheckPort, 0, maxPort), + checkMax("--compute-size", o.ComputeSize, maxComputeSize), + checkMax("--min-replicas", o.MinReplicas, maxReplicas), + checkMax("--max-replicas", o.MaxReplicas, maxReplicas), + checkMax("--concurrency", o.Concurrency, maxConcurrency), checkUtilThreshold("--cpu-util", o.CPUUtil), checkUtilThreshold("--gpu-util", o.GPUUtil), checkDurationRange("--scale-up-delay", o.ScaleUpDelay, maxScalingDelay), diff --git a/internal/verda-cli/cmd/serverless/shared.go b/internal/verda-cli/cmd/serverless/shared.go index b08a99c..06213c5 100644 --- a/internal/verda-cli/cmd/serverless/shared.go +++ b/internal/verda-cli/cmd/serverless/shared.go @@ -31,7 +31,6 @@ import ( // Create-request limits; keep in sync with the SDK, which rejects values outside them before sending. const ( - maxPort = 65535 maxComputeSize = 128 maxReplicas = 1000 maxConcurrency = 5000 @@ -42,9 +41,9 @@ const ( maxBatchjobRequestTTL = 7 * 24 * time.Hour ) -func checkIntRange(name string, n, lo, hi int) error { - if n < lo || n > hi { - return fmt.Errorf("%s must be in %d..%d", name, lo, hi) +func checkMax(name string, n, maxN int) error { + if n > maxN { + return fmt.Errorf("%s must be <= %d", name, maxN) } return nil } @@ -58,9 +57,9 @@ func checkUtilThreshold(name string, n int) error { } // checkDurationRange compares whole seconds, which is what the request carries. -func checkDurationRange(name string, d, hi time.Duration) error { - if d < 0 || int(d.Seconds()) > int(hi.Seconds()) { - return fmt.Errorf("%s must be between 0s and %s", name, shortDuration(hi)) +func checkDurationRange(name string, d, maxD time.Duration) error { + if d < 0 || int(d.Seconds()) > int(maxD.Seconds()) { + return fmt.Errorf("%s must be between 0s and %s", name, shortDuration(maxD)) } return nil } diff --git a/internal/verda-cli/cmd/serverless/wizard.go b/internal/verda-cli/cmd/serverless/wizard.go index 8a63112..37409aa 100644 --- a/internal/verda-cli/cmd/serverless/wizard.go +++ b/internal/verda-cli/cmd/serverless/wizard.go @@ -71,7 +71,7 @@ func buildContainerCreateFlow(_ context.Context, getClient clientFunc, opts *con stepContainerGPUUtil(&opts.GPUUtil), stepContainerScaleUpDelay(&opts.ScaleUpDelay), stepContainerScaleDownDelay(&opts.ScaleDownDelay), - stepContainerRequestTTL(&opts.RequestTTL), + stepRequestTTL(&opts.RequestTTL, maxContainerRequestTTL), stepSecretMounts(getClient, cache, &opts.SecretMounts), }, } @@ -172,7 +172,7 @@ func stepContainerMinReplicas(target *int) wizard.Step { Prompt: wizard.TextInputPrompt, Required: false, Default: func(_ map[string]any) any { return strconv.Itoa(*target) }, - Validate: parseIntRangeValidator("min replicas", 0, maxReplicas), + Validate: parseNonNegativeIntValidator("min replicas", maxReplicas), Setter: func(v any) { n, _ := strconv.Atoi(strings.TrimSpace(v.(string))) *target = n @@ -192,7 +192,7 @@ func stepContainerConcurrency(target *int) wizard.Step { Prompt: wizard.TextInputPrompt, Required: true, Default: func(_ map[string]any) any { return strconv.Itoa(*target) }, - Validate: parseIntRangeValidator("concurrency", 1, maxConcurrency), + Validate: parsePositiveIntValidator("concurrency", maxConcurrency), Setter: func(v any) { n, _ := strconv.Atoi(strings.TrimSpace(v.(string))) *target = n @@ -269,7 +269,7 @@ func stepContainerGPUUtil(target *int) wizard.Step { } // utilThresholdStep builds a step that asks "off | " as a text -// input. An empty value or "off" maps to 0; any integer in 5..100 enables +// input. An empty value or "off" maps to 0; any integer in minUtilThreshold..maxUtilThreshold enables // the trigger at that threshold. Shared by CPU and GPU util steps. func utilThresholdStep(name, desc string, target *int) wizard.Step { return wizard.Step{ diff --git a/internal/verda-cli/cmd/serverless/wizard_batchjob.go b/internal/verda-cli/cmd/serverless/wizard_batchjob.go index 42f06c1..6d985bb 100644 --- a/internal/verda-cli/cmd/serverless/wizard_batchjob.go +++ b/internal/verda-cli/cmd/serverless/wizard_batchjob.go @@ -43,33 +43,33 @@ func buildBatchjobCreateFlow(_ context.Context, getClient clientFunc, opts *batc stepEnvVars(&opts.Env), stepMaxReplicas(&opts.MaxReplicas), stepBatchjobDeadline(&opts.Deadline), - stepBatchjobRequestTTL(&opts.RequestTTL), + stepRequestTTL(&opts.RequestTTL, maxBatchjobRequestTTL), stepSecretMounts(getClient, cache, &opts.SecretMounts), }, } } -// stepBatchjobDeadline asks for the per-request deadline. Required, 10m..168h; -// the SDK rejects anything outside that range only after the whole wizard. +// stepBatchjobDeadline asks for the per-request deadline. Required and checked +// by validateBatchjobDeadline here; the SDK would reject it only after the whole wizard. func stepBatchjobDeadline(target *time.Duration) wizard.Step { return wizard.Step{ Name: "deadline", - Description: "Per-request deadline (10m to 168h, e.g. 30m, 1h) — required", + Description: "Per-request deadline (e.g. 5m, 30m, 1h) — required", Prompt: wizard.TextInputPrompt, Required: true, Default: func(_ map[string]any) any { if *target > 0 { return target.String() } - return "30m" + return shortDuration(defaultBatchjobDeadline) }, Validate: func(v any) error { s := strings.TrimSpace(v.(string)) d, err := time.ParseDuration(s) - if err != nil { - return errors.New("deadline must be a duration (e.g. 30m, 1h)") + if err != nil || d <= 0 { + return errors.New("deadline must be a positive duration (e.g. 5m, 30m, 1h)") } - return validateBatchjobDeadline(d) + return validateBatchjobDeadline("deadline", d) }, Setter: func(v any) { d, _ := time.ParseDuration(strings.TrimSpace(v.(string))) diff --git a/internal/verda-cli/cmd/serverless/wizard_shared.go b/internal/verda-cli/cmd/serverless/wizard_shared.go index 5fc605f..a24148f 100644 --- a/internal/verda-cli/cmd/serverless/wizard_shared.go +++ b/internal/verda-cli/cmd/serverless/wizard_shared.go @@ -129,7 +129,7 @@ func stepComputeSize(target *int) wizard.Step { } return "1" }, - Validate: parseIntRangeValidator("compute size", 1, maxComputeSize), + Validate: parsePositiveIntValidator("compute size", maxComputeSize), Setter: func(v any) { n, _ := strconv.Atoi(strings.TrimSpace(v.(string))) *target = n @@ -203,7 +203,7 @@ func stepPort(target *int) wizard.Step { Prompt: wizard.TextInputPrompt, Required: true, Default: func(_ map[string]any) any { return strconv.Itoa(*target) }, - Validate: parseIntRangeValidator("port", 1, maxPort), + Validate: parsePortValidator("port"), Setter: func(v any) { n, _ := strconv.Atoi(strings.TrimSpace(v.(string))) *target = n @@ -266,7 +266,7 @@ func stepMaxReplicas(target *int) wizard.Step { Prompt: wizard.TextInputPrompt, Required: true, Default: func(_ map[string]any) any { return strconv.Itoa(*target) }, - Validate: parseIntRangeValidator("max replicas", 1, maxReplicas), + Validate: parsePositiveIntValidator("max replicas", maxReplicas), Setter: func(v any) { n, _ := strconv.Atoi(strings.TrimSpace(v.(string))) *target = n @@ -279,13 +279,8 @@ func stepMaxReplicas(target *int) wizard.Step { // --- Request TTL --- -// Container and job queues have different TTL ceilings. -func stepContainerRequestTTL(target *time.Duration) wizard.Step { - return durationStep("request-ttl", "Request time-to-live (pending queue)", target, defaultRequestTTL, maxContainerRequestTTL) -} - -func stepBatchjobRequestTTL(target *time.Duration) wizard.Step { - return durationStep("request-ttl", "Request time-to-live (pending queue)", target, defaultRequestTTL, maxBatchjobRequestTTL) +func stepRequestTTL(target *time.Duration, maxTTL time.Duration) wizard.Step { + return durationStep("request-ttl", "Request time-to-live (pending queue)", target, defaultRequestTTL, maxTTL) } // --- Secret mounts (loop) --- @@ -342,7 +337,7 @@ func stepSecretMounts(getClient clientFunc, cache *apiCache, target *[]string) w func durationStep(name, desc string, target *time.Duration, def, maxDur time.Duration) wizard.Step { return wizard.Step{ Name: name, - Description: desc + " (0s to " + shortDuration(maxDur) + ", e.g. 0s, 300s, 5m)", + Description: desc + " (e.g. 0s, 300s, 5m)", Prompt: wizard.TextInputPrompt, Required: false, Default: func(_ map[string]any) any { @@ -357,8 +352,8 @@ func durationStep(name, desc string, target *time.Duration, def, maxDur time.Dur return nil } d, err := time.ParseDuration(s) - if err != nil { - return errors.New("must be a duration (e.g. 0s, 300s, 5m)") + if err != nil || d < 0 { + return errors.New("must be a non-negative duration (e.g. 0s, 300s, 5m)") } return checkDurationRange(name, d, maxDur) }, @@ -379,11 +374,31 @@ func durationStep(name, desc string, target *time.Duration, def, maxDur time.Dur // --- Validators (shared) --- -func parseIntRangeValidator(field string, lo, hi int) func(any) error { +func parsePositiveIntValidator(field string, maxN int) func(any) error { + return func(v any) error { + n, err := strconv.Atoi(strings.TrimSpace(v.(string))) + if err != nil || n < 1 { + return fmt.Errorf("%s must be a positive integer", field) + } + return checkMax(field, n, maxN) + } +} + +func parseNonNegativeIntValidator(field string, maxN int) func(any) error { + return func(v any) error { + n, err := strconv.Atoi(strings.TrimSpace(v.(string))) + if err != nil || n < 0 { + return fmt.Errorf("%s must be an integer >= 0", field) + } + return checkMax(field, n, maxN) + } +} + +func parsePortValidator(field string) func(any) error { return func(v any) error { n, err := strconv.Atoi(strings.TrimSpace(v.(string))) - if err != nil || n < lo || n > hi { - return fmt.Errorf("%s must be an integer in %d..%d", field, lo, hi) + if err != nil || n < 1 || n > 65535 { + return fmt.Errorf("%s must be an integer in 1..65535", field) } return nil } From 7e8411dd7b5bea9a550c2db8f7412791a2d464e0 Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Wed, 28 Oct 2026 17:55:26 +0200 Subject: [PATCH 11/11] fix(object-storage): stop a download worker after its own chunk error TestResumableDownload_BreakThenResume was flaky on main and on this branch (about 1 in 75 runs with -cpu 1,2,4). After a chunk failed, the worker could pull the next chunk before the consumer called cancel(), so a "concurrency 1" download kept going past the failed chunk. The worker now returns after it reports an error. Other workers still stop on the first error's cancel(), as before. Co-Authored-By: Claude Opus 5.5 --- internal/verda-cli/cmd/objectstorage/mpdownload.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/verda-cli/cmd/objectstorage/mpdownload.go b/internal/verda-cli/cmd/objectstorage/mpdownload.go index b6408da..f0631b0 100644 --- a/internal/verda-cli/cmd/objectstorage/mpdownload.go +++ b/internal/verda-cli/cmd/objectstorage/mpdownload.go @@ -305,6 +305,10 @@ func downloadMissingChunks(ctx context.Context, client API, opts *resumableDownl case <-ctx.Done(): return } + // A failed worker stops: cancel() runs only after the consumer reads the error, too late to stop the next pull. + if err != nil { + return + } } }) }