diff --git a/CHANGELOG.md b/CHANGELOG.md index c274e40..e98934a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,48 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 While the CLI is pre-1.0, minor versions may carry breaking changes to the public `pkg/sdk` surface; these are always called out under **Breaking (SDK)**. +## [Unreleased] + +### Added + +- **`dhq servers create` / `dhq servers update`**: five deployment-configuration + flags — `--branch` (the server's preferred branch), `--auto-deploy` + (DeployHQ's native repository auto-deployment), `--atomic` (zero-downtime + deployments), `--atomic-strategy` (`copy_release` — the default — or + `copy_cache`) and `--atomic-retention` (past releases to keep; must be `>= 1`, + rejected locally otherwise, backend default `3`). Both commands accept the + same set; on `update`, only flags that are explicitly passed are sent, so an + update never disturbs a setting the operator did not name. `--atomic` must be + configured **before** the server's first deployment — the backend refuses to + change it once a deployment exists — and is supported only on `ssh`, `rsync`, + `digitalocean`, `hetzner_cloud` and `managed_vps` servers. On accounts without + atomic deployments enabled the atomic fields are stripped server-side and the + request still succeeds, so verify with + `dhq servers show -p --json atomic,atomic_strategy,atomic_retention` + rather than trusting the exit code (DHQ-691). +- **`dhq servers create` / `dhq servers update`**: `--branch ""` now unpins a + server so it falls back to the repository default. Previously the empty value + was dropped by `omitempty`, and the command reported success having changed + nothing. The backend accepts and persists a blank branch (it echoes it back as + `""`, not `null`). +- **`dhq servers create` / `dhq servers update`**: warn on stderr when `--atomic` + was requested but the server comes back with atomic off. On accounts without + atomic deployments enabled the backend strips the atomic params before + validation and returns 2xx, so this was previously a silent success. The + create/update response is the read-back the docs asked operators to perform, + so no extra request is made. stdout stays pure data. +- **`dhq servers create` / `dhq servers update`**: warn on stderr when + `--branch` is set on a server that belongs to a server group. The backend + resolves the branch as `server_group.branch || server.branch || + repository.branch`, and grouped servers are excluded from auto-deployment + entirely, so a branch stored on a grouped server never deploys — previously a + silent no-op. stdout stays pure data. +- **SDK**: `ServerCreateRequest` and `ServerUpdateRequest` gained matching + `Branch`, `AutoDeploy`, `Atomic`, `AtomicStrategy` and `AtomicRetention` + fields. Purely additive — no existing field changed, so this is not a breaking + change for importers of `github.com/deployhq/deployhq-cli/pkg/sdk`. `Branch` + is a `*string` so an explicitly-cleared branch survives serialisation. + ## [0.20.1] - 2026-07-24 ### Fixed @@ -76,5 +118,6 @@ rather than regressed. flags are no longer sent, so they are left untouched server-side instead of being cleared. +[Unreleased]: https://github.com/deployhq/deployhq-cli/compare/v0.20.1...HEAD [0.20.1]: https://github.com/deployhq/deployhq-cli/releases/tag/v0.20.1 [0.20.0]: https://github.com/deployhq/deployhq-cli/releases/tag/v0.20.0 diff --git a/internal/commands/servers.go b/internal/commands/servers.go index 2fb9e7c..b72280a 100644 --- a/internal/commands/servers.go +++ b/internal/commands/servers.go @@ -9,8 +9,216 @@ import ( "github.com/deployhq/deployhq-cli/internal/output" "github.com/deployhq/deployhq-cli/pkg/sdk" "github.com/spf13/cobra" + "github.com/spf13/pflag" ) +// atomicStrategyCopyRelease is DeployHQ's own default. `servers create` sends +// it explicitly when --atomic is enabled without a strategy, so the payload the +// CLI produces is deterministic rather than dependent on a backend default. +const atomicStrategyCopyRelease = "copy_release" + +// atomicStrategies are the values the DeployHQ backend accepts. +var atomicStrategies = []string{atomicStrategyCopyRelease, "copy_cache"} + +// serverDeploymentFlags binds the deployment-configuration flags shared by +// `dhq servers create` and `dhq servers update`, so the two commands cannot +// drift apart. +// +// Every field is applied only when its flag was explicitly supplied. That is +// what keeps `servers update` from mutating settings the operator never named, +// and it is also why the booleans and the retention count reach the SDK as +// pointers: an explicit `--auto-deploy=false` must serialise as `false`, while +// an omitted one must not appear in the request body at all. +// +// The CLI deliberately does NOT replicate the backend's protocol/account policy +// for atomic deployments. Those checks live server-side and their structured +// errors pass through untouched. +type serverDeploymentFlags struct { + branch string + autoDeploy bool + atomic bool + atomicStrategy string + atomicRetention int + + // flags is the set the values were registered on; it answers "was this + // flag actually supplied?" via Changed(). + flags *pflag.FlagSet +} + +// register adds the five flags to cmd and records the flag set. +func (f *serverDeploymentFlags) register(cmd *cobra.Command) { + cmd.Flags().StringVar(&f.branch, "branch", "", + `Branch this server deploys from, e.g. main or staging. `+ + `Pass --branch "" to unpin the server and fall back to the repository default. `+ + `Has no effect while the server belongs to a server group — the group's branch wins`) + cmd.Flags().BoolVar(&f.autoDeploy, "auto-deploy", false, + "Deploy automatically when new commits reach the server's branch. "+ + "The backend suppresses auto-deploy while the server belongs to a server group. "+ + "Use --auto-deploy=false to turn it off") + cmd.Flags().BoolVar(&f.atomic, "atomic", false, + "Enable zero-downtime (atomic) deployments. Must be set before the server's first deployment — "+ + "the backend rejects any change afterwards. Requires a supported protocol "+ + "(ssh, rsync, digitalocean, hetzner_cloud, managed_vps) and an account with "+ + "atomic deployments enabled. Use --atomic=false to turn it off") + cmd.Flags().StringVar(&f.atomicStrategy, "atomic-strategy", "", + "Atomic release strategy: copy_release or copy_cache (DeployHQ default: copy_release)") + cmd.Flags().IntVar(&f.atomicRetention, "atomic-retention", 0, + "Number of past atomic releases to keep; must be 1 or greater (DeployHQ default: 3)") + + f.flags = cmd.Flags() +} + +// supplied reports whether the named flag was explicitly given on the command +// line. An unregistered flag set (helper never registered) counts as "no". +func (f *serverDeploymentFlags) supplied(name string) bool { + return f.flags != nil && f.flags.Changed(name) +} + +// validate runs the purely local checks. It must be called before the command +// resolves a project or builds an API client, so a malformed invocation fails +// with zero network access and no credentials. +func (f *serverDeploymentFlags) validate() error { + if f.supplied("atomic-strategy") { + valid := false + for _, s := range atomicStrategies { + if f.atomicStrategy == s { + valid = true + break + } + } + if !valid { + return &output.UserError{ + Message: fmt.Sprintf("Invalid --atomic-strategy %q", f.atomicStrategy), + Hint: "Use one of: " + strings.Join(atomicStrategies, ", "), + } + } + } + + if f.supplied("atomic-retention") && f.atomicRetention < 1 { + return &output.UserError{ + Message: fmt.Sprintf("Invalid --atomic-retention %d", f.atomicRetention), + Hint: "Retention is the number of past releases to keep, so it must be 1 or greater (DeployHQ default: 3).", + } + } + + return nil +} + +// applyToCreate copies the supplied flags onto a create request. +// +// Create additionally pins the strategy to copy_release when --atomic is +// enabled without one, matching the backend default and making the emitted +// payload deterministic. Update deliberately does NOT do this. +func (f *serverDeploymentFlags) applyToCreate(req *sdk.ServerCreateRequest) { + if f.supplied("branch") { + v := f.branch + req.Branch = &v + } + if f.supplied("auto-deploy") { + v := f.autoDeploy + req.AutoDeploy = &v + } + if f.supplied("atomic") { + v := f.atomic + req.Atomic = &v + } + if f.supplied("atomic-strategy") { + req.AtomicStrategy = f.atomicStrategy + } + if f.supplied("atomic-retention") { + v := f.atomicRetention + req.AtomicRetention = &v + } + + if req.Atomic != nil && *req.Atomic && req.AtomicStrategy == "" { + req.AtomicStrategy = atomicStrategyCopyRelease + } +} + +// branchIsDormant reports whether a branch the operator just set will have no +// effect on deployments because the server belongs to a server group. +// +// The backend resolves a server's branch as +// `server_group&.branch&.presence || branch.presence || repository.branch`, and +// grouped servers are excluded from auto-deployment altogether (the group is +// the deployable). So a branch stored on a grouped server is inert — the write +// succeeds and is echoed back, but nothing ever deploys from it. The Rails UI +// sidesteps this by hiding the field for grouped servers; the API does not, so +// the CLI says it out loud instead of letting the operator believe it took. +func branchIsDormant(branchSupplied bool, server *sdk.Server) bool { + if !branchSupplied || server == nil { + return false + } + return server.ServerGroupIdentifier != nil && *server.ServerGroupIdentifier != "" +} + +// atomicNotApplied reports whether atomic deployments were requested but the +// server came back with them off. +// +// An account without atomic deployments enabled has `atomic`, `atomic_strategy` +// and `atomic_retention` stripped from the request by the backend's permit +// list, before any validation runs — so the call returns 2xx with atomic +// silently off and no error to surface. The three params are permitted as a +// group, so checking `atomic` alone covers all of them. The other two atomic +// failure modes (unsupported protocol, change after the first deployment) do +// return real validation errors and need no client-side detection. +func atomicNotApplied(atomicRequested bool, server *sdk.Server) bool { + if !atomicRequested || server == nil { + return false + } + return server.Atomic == nil || !*server.Atomic +} + +// warnIfAtomicNotApplied emits the silent-strip warning on stderr. The response +// the CLI already holds is the read-back the docs tell operators to perform, so +// this reports what the backend actually did rather than delegating the check +// to a reference doc an agent may never load. +func warnIfAtomicNotApplied(env *output.Envelope, atomicRequested bool, server *sdk.Server) { + if !atomicNotApplied(atomicRequested, server) { + return + } + env.Warn("Atomic deployments were requested but the server reports atomic=false — " + + "this account does not have atomic deployments enabled. The rest of the " + + "request was applied; ask an account admin to enable atomic deployments.") +} + +// warnIfBranchDormant emits the dormant-branch warning on stderr, keeping +// stdout pure data. +func warnIfBranchDormant(env *output.Envelope, branchSupplied bool, server *sdk.Server) { + if !branchIsDormant(branchSupplied, server) { + return + } + env.Warn("Branch saved, but it has no effect while this server belongs to server group %q — "+ + "deployments use the group's branch (or the repository default). "+ + "Remove the server from the group, or set the branch on the group instead.", + *server.ServerGroupIdentifier) +} + +// applyToUpdate copies the supplied flags onto an update request. Flags that +// were not given stay absent from the payload — silently writing a strategy or +// a retention the operator did not ask for would clobber existing settings. +func (f *serverDeploymentFlags) applyToUpdate(req *sdk.ServerUpdateRequest) { + if f.supplied("branch") { + v := f.branch + req.Branch = &v + } + if f.supplied("auto-deploy") { + v := f.autoDeploy + req.AutoDeploy = &v + } + if f.supplied("atomic") { + v := f.atomic + req.Atomic = &v + } + if f.supplied("atomic-strategy") { + req.AtomicStrategy = f.atomicStrategy + } + if f.supplied("atomic-retention") { + v := f.atomicRetention + req.AtomicRetention = &v + } +} + func newServersCmd() *cobra.Command { cmd := &cobra.Command{ Use: "servers", @@ -181,6 +389,8 @@ func newServersCreateCmd() *cobra.Command { var region, size, osImage string // Billing guardrail (mirrors the gate in `dhq launch`) var acceptCost bool + // Deployment configuration shared with `dhq servers update` + deployFlags := &serverDeploymentFlags{} cmd := &cobra.Command{ Use: "create", @@ -209,7 +419,12 @@ func newServersCreateCmd() *cobra.Command { # Managed VPS droplet (beta — requires managed-resources beta) dhq servers create -p my-app --name vps --protocol-type managed_vps \ - --region lon1 --size s-1vcpu-1gb`, + --region lon1 --size s-1vcpu-1gb + + # Staging Managed VPS deploying the staging branch with atomic releases + dhq servers create -p my-app --name staging --protocol-type managed_vps \ + --region lon1 --size s-1vcpu-1gb --accept-cost \ + --branch staging --auto-deploy --atomic --atomic-retention 5`, RunE: func(cmd *cobra.Command, args []string) error { if name == "" { return &output.UserError{Message: "Server name is required", Hint: "Use --name flag"} @@ -220,6 +435,11 @@ func newServersCreateCmd() *cobra.Command { Hint: "Use --protocol-type with one of: ssh, ftp, ftps, rsync, s3, s3_compatible, digitalocean, hetzner_cloud, heroku, netlify, shopify, static_hosting, managed_vps", } } + // Local, offline validation — runs before a project is resolved or a + // client is built, so a malformed invocation never touches the network. + if err := deployFlags.validate(); err != nil { + return err + } projectID, err := cliCtx.RequireProject() if err != nil { @@ -266,6 +486,7 @@ func newServersCreateCmd() *cobra.Command { Size: size, OSImage: osImage, } + deployFlags.applyToCreate(&req) // Static Hosting (beta) — nested attributes if protocolType == "static_hosting" && subdomain != "" { req.HostedWebsiteAttributes = &sdk.HostedWebsiteAttributes{ @@ -378,6 +599,9 @@ func newServersCreateCmd() *cobra.Command { } } + warnIfBranchDormant(env, deployFlags.supplied("branch"), server) + warnIfAtomicNotApplied(env, deployFlags.supplied("atomic") && deployFlags.atomic, server) + if env.WantsJSON() { return env.WriteJSON(output.NewResponse(server, fmt.Sprintf("Created server: %s", server.Name))) } @@ -392,6 +616,9 @@ func newServersCreateCmd() *cobra.Command { cmd.Flags().StringVar(&serverPath, "path", "", "Server path") cmd.Flags().StringVar(&environment, "environment", "", "Environment name") + // Deployment configuration (shared with `dhq servers update`) + deployFlags.register(cmd) + // SSH / FTP / FTPS / Rsync cmd.Flags().StringVar(&hostname, "hostname", "", "Server hostname or IP address (ssh, ftp, ftps, rsync)") cmd.Flags().StringVar(&username, "username", "", "Server username (ssh, ftp, ftps, rsync, digitalocean, hetzner_cloud)") @@ -447,13 +674,29 @@ func newServersCreateCmd() *cobra.Command { func newServersUpdateCmd() *cobra.Command { var name, serverPath, environment string + // Deployment configuration shared with `dhq servers create` + deployFlags := &serverDeploymentFlags{} cmd := &cobra.Command{ Use: "update ", Short: "Update a server", Args: cobra.ExactArgs(1), ValidArgsFunction: completeServerNames, + Example: ` # Point a server at a different branch + dhq servers update prod -p my-app --branch release + + # Turn atomic deployments on before the server's first deploy + dhq servers update prod -p my-app --atomic --atomic-strategy copy_cache --atomic-retention 5 + + # Turn auto-deploy off without touching any other setting + dhq servers update prod -p my-app --auto-deploy=false`, RunE: func(cmd *cobra.Command, args []string) error { + // Local, offline validation — runs before a project is resolved or a + // client is built, so a malformed invocation never touches the network. + if err := deployFlags.validate(); err != nil { + return err + } + projectID, err := cliCtx.RequireProject() if err != nil { return err @@ -465,12 +708,19 @@ func newServersUpdateCmd() *cobra.Command { } req := sdk.ServerUpdateRequest{Name: name, ServerPath: serverPath, Environment: environment} + // Only flags the operator actually supplied reach the payload, so an + // update never disturbs deployment settings that were left unnamed. + deployFlags.applyToUpdate(&req) + server, err := client.UpdateServer(cliCtx.Background(), projectID, args[0], req) if err != nil { return err } env := cliCtx.Envelope + warnIfBranchDormant(env, deployFlags.supplied("branch"), server) + warnIfAtomicNotApplied(env, deployFlags.supplied("atomic") && deployFlags.atomic, server) + if env.WantsJSON() { return env.WriteJSON(output.NewResponse(server, fmt.Sprintf("Updated server: %s", server.Name))) } @@ -482,6 +732,7 @@ func newServersUpdateCmd() *cobra.Command { cmd.Flags().StringVar(&name, "name", "", "Server name") cmd.Flags().StringVar(&serverPath, "path", "", "Server path") cmd.Flags().StringVar(&environment, "environment", "", "Environment name") + deployFlags.register(cmd) return cmd } diff --git a/internal/commands/servers_test.go b/internal/commands/servers_test.go new file mode 100644 index 0000000..3ce1267 --- /dev/null +++ b/internal/commands/servers_test.go @@ -0,0 +1,595 @@ +package commands + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "strings" + "sync" + "testing" + + "github.com/deployhq/deployhq-cli/pkg/sdk" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// ── helpers ─────────────────────────────────────────────────────────────────── + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +// blockNetwork installs a tripwire transport for the duration of the test. The +// SDK builds its http.Client with a nil Transport, so every request it makes +// goes through http.DefaultTransport — swapping that turns "did this command +// touch the network?" into an assertable fact rather than an assumption. +func blockNetwork(t *testing.T) *bool { + t.Helper() + called := false + orig := http.DefaultTransport + http.DefaultTransport = roundTripFunc(func(r *http.Request) (*http.Response, error) { + called = true + t.Errorf("unexpected HTTP request: %s %s", r.Method, r.URL) + return nil, fmt.Errorf("network blocked in test") + }) + t.Cleanup(func() { http.DefaultTransport = orig }) + return &called +} + +// withResolvableContext supplies credentials and a project through the env-var +// config layer so RequireProject() and Client() would both SUCCEED. That is +// what makes the "no HTTP call" assertions meaningful: the only thing standing +// between the command and the wire is the local flag validation. +func withResolvableContext(t *testing.T) { + t.Helper() + t.Setenv("DEPLOYHQ_ACCOUNT", "testco") + t.Setenv("DEPLOYHQ_EMAIL", "user@example.com") + t.Setenv("DEPLOYHQ_API_KEY", "test-key") + t.Setenv("DEPLOYHQ_PROJECT", "my-app") + t.Setenv("DEPLOYHQ_NO_TELEMETRY", "1") + + origCtx := cliCtx + t.Cleanup(func() { cliCtx = origCtx }) +} + +// capturedRequest records what a command actually put on the wire. +type capturedRequest struct { + mu sync.Mutex + count int + method string + path string + body map[string]any +} + +// server returns the decoded `server` object from the captured body. +func (c *capturedRequest) server(t *testing.T) map[string]any { + t.Helper() + c.mu.Lock() + defer c.mu.Unlock() + require.Equal(t, 1, c.count, "expected exactly one DeployHQ API request") + srv, ok := c.body["server"].(map[string]any) + require.True(t, ok, "request body must wrap a server object, got: %v", c.body) + return srv +} + +// captureRequest is blockNetwork's inverse: instead of failing the test, it +// records the outgoing request and answers with a canned server so the command +// completes its success path. This is what exercises the command → request +// seam end to end — the helper-level tests below construct requests directly +// and so cannot catch a command that never calls applyTo{Create,Update}. +func captureRequest(t *testing.T) *capturedRequest { + t.Helper() + cap := &capturedRequest{} + orig := http.DefaultTransport + http.DefaultTransport = roundTripFunc(func(r *http.Request) (*http.Response, error) { + cap.mu.Lock() + defer cap.mu.Unlock() + // Only the DeployHQ API counts. The root command also fires an update + // check against api.github.com after the command completes; that is + // pre-existing behaviour unrelated to what these tests assert. + if !strings.Contains(r.URL.Path, "/projects/") { + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(`{}`)), + Header: http.Header{"Content-Type": []string{"application/json"}}, + Request: r, + }, nil + } + cap.count++ + cap.method = r.Method + cap.path = r.URL.Path + if r.Body != nil { + raw, err := io.ReadAll(r.Body) + if err != nil { + return nil, err + } + // Decode leniently: a malformed body should surface as a failed + // assertion on the contents, not as a transport error. + _ = json.Unmarshal(raw, &cap.body) + } + return &http.Response{ + StatusCode: http.StatusCreated, + Body: io.NopCloser(strings.NewReader(`{"identifier":"srv-1","name":"staging","protocol_type":"ssh"}`)), + Header: http.Header{"Content-Type": []string{"application/json"}}, + Request: r, + }, nil + }) + t.Cleanup(func() { http.DefaultTransport = orig }) + return cap +} + +// runServersCmd executes the real root command with args, discarding output. +func runServersCmd(t *testing.T, args ...string) error { + t.Helper() + cmd := NewRootCmd("test") + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + cmd.SetArgs(args) + return cmd.Execute() +} + +// parseDeploymentFlags registers the shared deployment flags on a throwaway +// command and parses args into them — exercising the helper in isolation. +func parseDeploymentFlags(t *testing.T, args ...string) *serverDeploymentFlags { + t.Helper() + f := &serverDeploymentFlags{} + cmd := &cobra.Command{Use: "fake", RunE: func(*cobra.Command, []string) error { return nil }} + f.register(cmd) + require.NoError(t, cmd.Flags().Parse(args)) + return f +} + +var deploymentFlagNames = []string{ + "branch", "auto-deploy", "atomic", "atomic-strategy", "atomic-retention", +} + +// ── flag registration ───────────────────────────────────────────────────────── + +func TestServersCreate_RegistersDeploymentFlags(t *testing.T) { + root := NewRootCmd("test") + createCmd, _, err := root.Find([]string{"servers", "create"}) + require.NoError(t, err) + require.Equal(t, "create", createCmd.Name()) + + for _, name := range deploymentFlagNames { + assert.NotNil(t, createCmd.Flags().Lookup(name), + "--%s must be registered on servers create", name) + } +} + +func TestServersUpdate_RegistersDeploymentFlags(t *testing.T) { + root := NewRootCmd("test") + updateCmd, _, err := root.Find([]string{"servers", "update"}) + require.NoError(t, err) + require.Equal(t, "update", updateCmd.Name()) + + for _, name := range deploymentFlagNames { + assert.NotNil(t, updateCmd.Flags().Lookup(name), + "--%s must be registered on servers update", name) + } +} + +// The booleans must be usable as bare switches (--atomic) as well as with an +// explicit value (--atomic=false); pflag expresses that via NoOptDefVal. +func TestServersDeploymentFlags_BooleansAcceptBareForm(t *testing.T) { + root := NewRootCmd("test") + createCmd, _, err := root.Find([]string{"servers", "create"}) + require.NoError(t, err) + + for _, name := range []string{"auto-deploy", "atomic"} { + f := createCmd.Flags().Lookup(name) + require.NotNil(t, f) + assert.Equal(t, "true", f.NoOptDefVal, "--%s must work as a bare switch", name) + } +} + +// The help text has to carry the constraints an operator cannot discover from +// the flag name alone. +func TestServersCreate_DeploymentFlagHelpMentionsConstraints(t *testing.T) { + root := NewRootCmd("test") + createCmd, _, err := root.Find([]string{"servers", "create"}) + require.NoError(t, err) + + atomic := createCmd.Flags().Lookup("atomic").Usage + assert.Contains(t, atomic, "first deployment", + "--atomic help must warn that atomic is locked after the first deployment") + for _, proto := range []string{"ssh", "rsync", "digitalocean", "hetzner_cloud", "managed_vps"} { + assert.Contains(t, atomic, proto, "--atomic help must list the %s protocol", proto) + } + assert.Contains(t, atomic, "atomic deployments enabled", + "--atomic help must mention the account-level requirement") + + strategy := createCmd.Flags().Lookup("atomic-strategy").Usage + assert.Contains(t, strategy, "copy_release") + assert.Contains(t, strategy, "copy_cache") + + retention := createCmd.Flags().Lookup("atomic-retention").Usage + assert.Contains(t, retention, "1") +} + +func TestServersCreate_ExampleShowsAtomicManagedVPS(t *testing.T) { + root := NewRootCmd("test") + createCmd, _, err := root.Find([]string{"servers", "create"}) + require.NoError(t, err) + + assert.Contains(t, createCmd.Example, "--branch") + assert.Contains(t, createCmd.Example, "--atomic") + assert.Contains(t, createCmd.Example, "managed_vps") +} + +// ── local validation: fails before any network access ───────────────────────── + +func TestServersCreate_RejectsUnknownAtomicStrategy_NoHTTP(t *testing.T) { + withResolvableContext(t) + called := blockNetwork(t) + + err := runServersCmd(t, "servers", "create", + "--name", "staging", "--protocol-type", "ssh", + "--atomic", "--atomic-strategy", "rsync_release") + + require.Error(t, err) + assert.Contains(t, err.Error(), "Invalid --atomic-strategy") + assert.Contains(t, err.Error(), "copy_release") + assert.Contains(t, err.Error(), "copy_cache") + assert.False(t, *called, "validation must fail before any HTTP request") +} + +func TestServersUpdate_RejectsUnknownAtomicStrategy_NoHTTP(t *testing.T) { + withResolvableContext(t) + called := blockNetwork(t) + + err := runServersCmd(t, "servers", "update", "srv-1", + "--atomic-strategy", "nope") + + require.Error(t, err) + assert.Contains(t, err.Error(), "Invalid --atomic-strategy") + assert.False(t, *called, "validation must fail before any HTTP request") +} + +func TestServersCreate_RejectsZeroAtomicRetention_NoHTTP(t *testing.T) { + withResolvableContext(t) + called := blockNetwork(t) + + err := runServersCmd(t, "servers", "create", + "--name", "staging", "--protocol-type", "ssh", + "--atomic-retention", "0") + + require.Error(t, err) + assert.Contains(t, err.Error(), "Invalid --atomic-retention") + assert.False(t, *called, "validation must fail before any HTTP request") +} + +func TestServersCreate_RejectsNegativeAtomicRetention_NoHTTP(t *testing.T) { + withResolvableContext(t) + called := blockNetwork(t) + + err := runServersCmd(t, "servers", "create", + "--name", "staging", "--protocol-type", "ssh", + "--atomic-retention", "-1") + + require.Error(t, err) + assert.Contains(t, err.Error(), "Invalid --atomic-retention") + assert.False(t, *called, "validation must fail before any HTTP request") +} + +func TestServersUpdate_RejectsZeroAtomicRetention_NoHTTP(t *testing.T) { + withResolvableContext(t) + called := blockNetwork(t) + + err := runServersCmd(t, "servers", "update", "srv-1", "--atomic-retention", "0") + + require.Error(t, err) + assert.Contains(t, err.Error(), "Invalid --atomic-retention") + assert.False(t, *called, "validation must fail before any HTTP request") +} + +func TestServerDeploymentFlags_ValidateAcceptsSupportedStrategies(t *testing.T) { + for _, s := range []string{"copy_release", "copy_cache"} { + f := parseDeploymentFlags(t, "--atomic-strategy", s) + assert.NoError(t, f.validate(), "%s must be accepted", s) + } +} + +func TestServerDeploymentFlags_ValidateAcceptsRetentionOfOne(t *testing.T) { + f := parseDeploymentFlags(t, "--atomic-retention", "1") + assert.NoError(t, f.validate()) +} + +// A zero-valued flag that was never supplied must not trip the >= 1 check — +// otherwise every `servers update --name x` would fail. +func TestServerDeploymentFlags_ValidateIgnoresUnsuppliedFlags(t *testing.T) { + f := parseDeploymentFlags(t) + assert.NoError(t, f.validate()) +} + +// ── request application ─────────────────────────────────────────────────────── + +func TestServerDeploymentFlags_OmittedFlagsLeaveCreateRequestZero(t *testing.T) { + f := parseDeploymentFlags(t) + var req sdk.ServerCreateRequest + f.applyToCreate(&req) + + assert.Nil(t, req.Branch) + assert.Nil(t, req.AutoDeploy) + assert.Nil(t, req.Atomic) + assert.Empty(t, req.AtomicStrategy) + assert.Nil(t, req.AtomicRetention) +} + +func TestServerDeploymentFlags_OmittedFlagsLeaveUpdateRequestZero(t *testing.T) { + f := parseDeploymentFlags(t) + var req sdk.ServerUpdateRequest + f.applyToUpdate(&req) + + assert.Nil(t, req.Branch) + assert.Nil(t, req.AutoDeploy) + assert.Nil(t, req.Atomic) + assert.Empty(t, req.AtomicStrategy) + assert.Nil(t, req.AtomicRetention) +} + +func TestServerDeploymentFlags_ExplicitFalseSetsNonNilPointer(t *testing.T) { + f := parseDeploymentFlags(t, "--auto-deploy=false", "--atomic=false") + + var create sdk.ServerCreateRequest + f.applyToCreate(&create) + require.NotNil(t, create.AutoDeploy, "--auto-deploy=false must be sent, not dropped") + assert.False(t, *create.AutoDeploy) + require.NotNil(t, create.Atomic, "--atomic=false must be sent, not dropped") + assert.False(t, *create.Atomic) + // atomic=false must NOT pull in the copy_release default. + assert.Empty(t, create.AtomicStrategy) + + var update sdk.ServerUpdateRequest + f.applyToUpdate(&update) + require.NotNil(t, update.AutoDeploy) + assert.False(t, *update.AutoDeploy) + require.NotNil(t, update.Atomic) + assert.False(t, *update.Atomic) + assert.Empty(t, update.AtomicStrategy) +} + +// --atomic on create with no explicit strategy pins copy_release so the payload +// is deterministic. On update the same input must leave the strategy alone — +// writing one would clobber an existing copy_cache setting. +func TestServerDeploymentFlags_AtomicDefaultsStrategyOnCreateOnly(t *testing.T) { + f := parseDeploymentFlags(t, "--atomic") + + var create sdk.ServerCreateRequest + f.applyToCreate(&create) + require.NotNil(t, create.Atomic) + assert.True(t, *create.Atomic) + assert.Equal(t, "copy_release", create.AtomicStrategy) + + var update sdk.ServerUpdateRequest + f.applyToUpdate(&update) + require.NotNil(t, update.Atomic) + assert.True(t, *update.Atomic) + assert.Empty(t, update.AtomicStrategy, + "update must never write a strategy the operator did not supply") +} + +func TestServerDeploymentFlags_ExplicitStrategyWinsOverCreateDefault(t *testing.T) { + f := parseDeploymentFlags(t, "--atomic", "--atomic-strategy", "copy_cache") + + var create sdk.ServerCreateRequest + f.applyToCreate(&create) + assert.Equal(t, "copy_cache", create.AtomicStrategy) +} + +func TestServerDeploymentFlags_AppliesEveryFlag(t *testing.T) { + f := parseDeploymentFlags(t, + "--branch", "staging", + "--auto-deploy", + "--atomic", + "--atomic-strategy", "copy_cache", + "--atomic-retention", "5", + ) + + var create sdk.ServerCreateRequest + f.applyToCreate(&create) + require.NotNil(t, create.Branch) + assert.Equal(t, "staging", *create.Branch) + require.NotNil(t, create.AutoDeploy) + assert.True(t, *create.AutoDeploy) + require.NotNil(t, create.Atomic) + assert.True(t, *create.Atomic) + assert.Equal(t, "copy_cache", create.AtomicStrategy) + require.NotNil(t, create.AtomicRetention) + assert.Equal(t, 5, *create.AtomicRetention) + + var update sdk.ServerUpdateRequest + f.applyToUpdate(&update) + require.NotNil(t, update.Branch) + assert.Equal(t, "staging", *update.Branch) + require.NotNil(t, update.AutoDeploy) + assert.True(t, *update.AutoDeploy) + require.NotNil(t, update.Atomic) + assert.True(t, *update.Atomic) + assert.Equal(t, "copy_cache", update.AtomicStrategy) + require.NotNil(t, update.AtomicRetention) + assert.Equal(t, 5, *update.AtomicRetention) +} + +// Branch alone must not drag any atomic setting onto the wire. +func TestServerDeploymentFlags_BranchOnlyTouchesBranch(t *testing.T) { + f := parseDeploymentFlags(t, "--branch", "main") + + var update sdk.ServerUpdateRequest + f.applyToUpdate(&update) + require.NotNil(t, update.Branch) + assert.Equal(t, "main", *update.Branch) + assert.Nil(t, update.AutoDeploy) + assert.Nil(t, update.Atomic) + assert.Empty(t, update.AtomicStrategy) + assert.Nil(t, update.AtomicRetention) +} + +// ── command → request wiring (end to end) ───────────────────────────────────── +// +// These are the only tests that fail if `deployFlags.applyToCreate(&req)` or +// `deployFlags.applyToUpdate(&req)` is deleted from the command bodies. Every +// other test in this file constructs the request itself, so the two lines that +// actually make the feature work were previously uncovered. + +func TestServersCreate_SendsDeploymentSettingsOnTheWire(t *testing.T) { + withResolvableContext(t) + cap := captureRequest(t) + + err := runServersCmd(t, "servers", "create", + "--name", "staging", "--protocol-type", "ssh", + "--hostname", "h", "--username", "u", + "--branch", "staging", + "--auto-deploy=false", + "--atomic", + "--atomic-retention", "5", + ) + require.NoError(t, err) + + srv := cap.server(t) + assert.Equal(t, http.MethodPost, cap.method) + assert.Equal(t, "staging", srv["branch"]) + assert.Equal(t, false, srv["auto_deploy"], "explicit --auto-deploy=false must reach the wire") + assert.Equal(t, true, srv["atomic"]) + assert.Equal(t, "copy_release", srv["atomic_strategy"], "create pins the default strategy") + assert.Equal(t, float64(5), srv["atomic_retention"]) +} + +func TestServersUpdate_SendsOnlySuppliedSettingsOnTheWire(t *testing.T) { + withResolvableContext(t) + cap := captureRequest(t) + + err := runServersCmd(t, "servers", "update", "srv-1", "--branch", "main") + require.NoError(t, err) + + srv := cap.server(t) + assert.Equal(t, http.MethodPut, cap.method) + assert.Equal(t, "main", srv["branch"]) + // Requirement: an update never disturbs a setting the operator did not name. + for _, k := range []string{"auto_deploy", "atomic", "atomic_strategy", "atomic_retention"} { + assert.NotContains(t, srv, k, "%s must not be sent when its flag was omitted", k) + } +} + +// ── #1: an explicitly-cleared branch must reach the wire ────────────────────── +// +// The DeployHQ backend accepts and persists `branch: ""` — IGNORE_PARAMS_ON_BLANK +// covers only credential params, and every consumer resolves the branch with +// .presence, so an empty string reads as "unpinned, fall back to the repository +// default". A `string` field with omitempty silently swallowed that intent. + +func TestServersUpdate_ExplicitEmptyBranchReachesTheWire(t *testing.T) { + withResolvableContext(t) + cap := captureRequest(t) + + err := runServersCmd(t, "servers", "update", "srv-1", "--branch", "") + require.NoError(t, err) + + srv := cap.server(t) + require.Contains(t, srv, "branch", "an explicitly supplied empty --branch must be sent, not dropped") + assert.Equal(t, "", srv["branch"]) +} + +func TestServersUpdate_OmittedBranchStillAbsent(t *testing.T) { + withResolvableContext(t) + cap := captureRequest(t) + + err := runServersCmd(t, "servers", "update", "srv-1", "--name", "renamed") + require.NoError(t, err) + + srv := cap.server(t) + assert.Equal(t, "renamed", srv["name"]) + assert.NotContains(t, srv, "branch", "an omitted --branch must stay off the wire") +} + +// ── #2: a branch set on a grouped server is dormant ─────────────────────────── + +func TestBranchIsDormant(t *testing.T) { + grouped := "grp-1" + empty := "" + + cases := []struct { + name string + supplied bool + server *sdk.Server + wantDorma bool + }{ + {"supplied on grouped server", true, &sdk.Server{ServerGroupIdentifier: &grouped}, true}, + {"supplied on ungrouped server", true, &sdk.Server{}, false}, + {"supplied, group identifier empty", true, &sdk.Server{ServerGroupIdentifier: &empty}, false}, + {"not supplied, grouped", false, &sdk.Server{ServerGroupIdentifier: &grouped}, false}, + {"nil server", true, nil, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.wantDorma, branchIsDormant(tc.supplied, tc.server)) + }) + } +} + +// ── #4: atomic requested but not applied (silent account-level strip) ───────── + +func TestAtomicNotApplied(t *testing.T) { + yes, no := true, false + + cases := []struct { + name string + requested bool + server *sdk.Server + want bool + }{ + {"requested, came back false", true, &sdk.Server{Atomic: &no}, true}, + {"requested, came back absent", true, &sdk.Server{}, true}, + {"requested, came back true", true, &sdk.Server{Atomic: &yes}, false}, + {"not requested, came back false", false, &sdk.Server{Atomic: &no}, false}, + {"nil server", true, nil, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, atomicNotApplied(tc.requested, tc.server)) + }) + } +} + +// ── #5: the Managed VPS billing guardrail must precede the API call ────────── +// +// The gate is the only thing between a non-interactive invocation and a +// billable provisioning call, and this change inserted statements on both +// sides of it. blockNetwork turns "no request was made" into a fact. + +func TestServersCreate_ManagedVPSRequiresAcceptCost_NoHTTP(t *testing.T) { + withResolvableContext(t) + called := blockNetwork(t) + + err := runServersCmd(t, "servers", "create", + "--name", "vps", "--protocol-type", "managed_vps", + "--region", "lon1", "--size", "s-1vcpu-1gb", + // deployment flags on the same command must not let the gate be skipped + "--branch", "staging", "--atomic", + ) + require.Error(t, err) + assert.Contains(t, err.Error(), "accept-cost") + assert.False(t, *called, "the cost gate must fire before any API request") +} + +func TestServersCreate_ManagedVPSWithAcceptCost_Proceeds(t *testing.T) { + withResolvableContext(t) + cap := captureRequest(t) + + err := runServersCmd(t, "servers", "create", + "--name", "vps", "--protocol-type", "managed_vps", + "--region", "lon1", "--size", "s-1vcpu-1gb", "--accept-cost", + "--branch", "staging", + ) + require.NoError(t, err) + + srv := cap.server(t) + assert.Equal(t, "staging", srv["branch"]) + // provisioning params stay top-level siblings of `server` + cap.mu.Lock() + defer cap.mu.Unlock() + assert.Equal(t, "lon1", cap.body["region"]) + assert.Equal(t, "s-1vcpu-1gb", cap.body["size"]) +} diff --git a/pkg/sdk/integration_test.go b/pkg/sdk/integration_test.go index 882b4bf..9642e5f 100644 --- a/pkg/sdk/integration_test.go +++ b/pkg/sdk/integration_test.go @@ -103,7 +103,7 @@ func TestRealDeploymentShape(t *testing.T) { "enabled": true, "agent": null, "atomic": true, - "atomic_strategy": "symlink", + "atomic_strategy": "copy_release", "atomic_retention": 5, "use_compression": true, "use_accelerated_transfer": false, @@ -256,7 +256,7 @@ func TestRealServerShape(t *testing.T) { "enabled": true, "agent": null, "atomic": true, - "atomic_strategy": "symlink", + "atomic_strategy": "copy_release", "atomic_retention": 5, "use_compression": true, "use_accelerated_transfer": false, diff --git a/pkg/sdk/server_deployment_settings_test.go b/pkg/sdk/server_deployment_settings_test.go new file mode 100644 index 0000000..362c3ab --- /dev/null +++ b/pkg/sdk/server_deployment_settings_test.go @@ -0,0 +1,180 @@ +package sdk + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// captureCreateBody records the raw JSON body of a POST /projects/:id/servers +// call so tests can assert on exact key placement, not just decoded structs. +func captureCreateBody(t *testing.T, into *map[string]any) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodPost, r.Method) + require.NoError(t, json.NewDecoder(r.Body).Decode(into)) + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(Server{Identifier: "srv-1", Name: "staging"}) + })) +} + +// captureUpdateBody does the same for PUT /projects/:id/servers/:id. +func captureUpdateBody(t *testing.T, into *map[string]any) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodPut, r.Method) + require.NoError(t, json.NewDecoder(r.Body).Decode(into)) + _ = json.NewEncoder(w).Encode(Server{Identifier: "srv-1", Name: "staging"}) + })) +} + +func TestCreateServer_ManagedVPS_DeploymentSettingsNestedUnderServer(t *testing.T) { + // The five deployment settings belong INSIDE `server`; region/size/os_image + // stay top-level siblings (params[:region], not params[:server][:region]). + var body map[string]any + srv := captureCreateBody(t, &body) + defer srv.Close() + + c := newTestClient(t, srv) + _, err := c.CreateServer(context.Background(), "my-app", ServerCreateRequest{ + Name: "staging", + ProtocolType: "managed_vps", + ServerPath: "/srv/ops-agents", + Environment: "staging", + Branch: strPtr("staging"), + AutoDeploy: boolPtr(false), + Atomic: boolPtr(true), + AtomicStrategy: "copy_release", + AtomicRetention: intPtr(5), + Region: "lon1", + Size: "s-1vcpu-1gb", + OSImage: "ubuntu-24-04-x64", + }) + require.NoError(t, err) + + server, ok := body["server"].(map[string]any) + require.True(t, ok, "body must wrap the server object") + + assert.Equal(t, "staging", server["branch"]) + assert.Equal(t, false, server["auto_deploy"]) + assert.Equal(t, true, server["atomic"]) + assert.Equal(t, "copy_release", server["atomic_strategy"]) + assert.Equal(t, float64(5), server["atomic_retention"]) + + // Provisioning params remain top-level, never nested. + assert.Equal(t, "lon1", body["region"]) + assert.Equal(t, "s-1vcpu-1gb", body["size"]) + assert.Equal(t, "ubuntu-24-04-x64", body["os_image"]) + for _, k := range []string{"region", "size", "os_image"} { + _, nested := server[k] + assert.False(t, nested, "%s must not be nested inside server", k) + } +} + +func TestUpdateServer_SendsDeploymentSettingsWrapped(t *testing.T) { + var body map[string]any + srv := captureUpdateBody(t, &body) + defer srv.Close() + + c := newTestClient(t, srv) + _, err := c.UpdateServer(context.Background(), "my-app", "srv-1", ServerUpdateRequest{ + Branch: strPtr("main"), + AutoDeploy: boolPtr(true), + Atomic: boolPtr(true), + AtomicStrategy: "copy_cache", + AtomicRetention: intPtr(10), + }) + require.NoError(t, err) + + server, ok := body["server"].(map[string]any) + require.True(t, ok, "body must wrap the server object") + assert.Equal(t, "main", server["branch"]) + assert.Equal(t, true, server["auto_deploy"]) + assert.Equal(t, true, server["atomic"]) + assert.Equal(t, "copy_cache", server["atomic_strategy"]) + assert.Equal(t, float64(10), server["atomic_retention"]) +} + +func TestCreateServer_ExplicitFalseBooleansAreSent(t *testing.T) { + // omitempty on a bool would drop `false`; pointers keep the distinction + // between "not supplied" and "explicitly false". + var body map[string]any + srv := captureCreateBody(t, &body) + defer srv.Close() + + c := newTestClient(t, srv) + _, err := c.CreateServer(context.Background(), "my-app", ServerCreateRequest{ + Name: "staging", + ProtocolType: "ssh", + AutoDeploy: boolPtr(false), + Atomic: boolPtr(false), + }) + require.NoError(t, err) + + server := body["server"].(map[string]any) + require.Contains(t, server, "auto_deploy", "explicit false must survive serialisation") + require.Contains(t, server, "atomic", "explicit false must survive serialisation") + assert.Equal(t, false, server["auto_deploy"]) + assert.Equal(t, false, server["atomic"]) +} + +func TestUpdateServer_ExplicitFalseBooleansAreSent(t *testing.T) { + var body map[string]any + srv := captureUpdateBody(t, &body) + defer srv.Close() + + c := newTestClient(t, srv) + _, err := c.UpdateServer(context.Background(), "my-app", "srv-1", ServerUpdateRequest{ + AutoDeploy: boolPtr(false), + Atomic: boolPtr(false), + }) + require.NoError(t, err) + + server := body["server"].(map[string]any) + require.Contains(t, server, "auto_deploy") + require.Contains(t, server, "atomic") + assert.Equal(t, false, server["auto_deploy"]) + assert.Equal(t, false, server["atomic"]) +} + +func TestCreateServer_OmittedDeploymentSettingsAreAbsent(t *testing.T) { + // An unset field must not reach the wire — a zero-value atomic_retention + // would be rejected by the backend's `greater_than_or_equal_to: 1`. + var body map[string]any + srv := captureCreateBody(t, &body) + defer srv.Close() + + c := newTestClient(t, srv) + _, err := c.CreateServer(context.Background(), "my-app", ServerCreateRequest{ + Name: "staging", ProtocolType: "ssh", + }) + require.NoError(t, err) + + server := body["server"].(map[string]any) + for _, k := range []string{"branch", "auto_deploy", "atomic", "atomic_strategy", "atomic_retention"} { + assert.NotContains(t, server, k, "%s must be omitted when not supplied", k) + } +} + +func TestUpdateServer_OmittedDeploymentSettingsAreAbsent(t *testing.T) { + var body map[string]any + srv := captureUpdateBody(t, &body) + defer srv.Close() + + c := newTestClient(t, srv) + _, err := c.UpdateServer(context.Background(), "my-app", "srv-1", ServerUpdateRequest{Name: "renamed"}) + require.NoError(t, err) + + server := body["server"].(map[string]any) + assert.Equal(t, "renamed", server["name"]) + for _, k := range []string{"branch", "auto_deploy", "atomic", "atomic_strategy", "atomic_retention"} { + assert.NotContains(t, server, k, "%s must be omitted when not supplied", k) + } +} + +func intPtr(i int) *int { return &i } diff --git a/pkg/sdk/types.go b/pkg/sdk/types.go index 612b622..f4a6a57 100644 --- a/pkg/sdk/types.go +++ b/pkg/sdk/types.go @@ -137,6 +137,32 @@ type ServerCreateRequest struct { AgentID string `json:"agent_id,omitempty"` Enabled *bool `json:"enabled,omitempty"` + // Deployment configuration. Booleans and retention are pointers so an + // omitted setting stays off the wire while an explicit `false` is still sent. + // + // Branch is the server's preferred branch, e.g. "main" or "staging". + // It is a pointer so an explicitly-empty branch survives serialisation: + // the backend accepts and persists `branch: ""`, and every consumer resolves + // it with `.presence`, so empty means "unpinned — fall back to the + // repository default". A plain string with omitempty would silently drop + // that intent. Note the API echoes the cleared value back as `""`, not null. + Branch *string `json:"branch,omitempty"` + // AutoDeploy enables DeployHQ's native repository auto-deployment. The + // backend suppresses auto-deploy while a server belongs to a server group, + // regardless of this value. + AutoDeploy *bool `json:"auto_deploy,omitempty"` + // Atomic enables zero-downtime (atomic) deployments. Supported only on ssh, + // rsync, digitalocean, hetzner_cloud and managed_vps servers, and only for + // accounts with atomic deployments enabled. The backend REJECTS any change + // to this field once the server has been deployed to, so atomic must be + // configured before the server's first deployment. + Atomic *bool `json:"atomic,omitempty"` + // AtomicStrategy is "copy_release" (backend default) or "copy_cache". + AtomicStrategy string `json:"atomic_strategy,omitempty"` + // AtomicRetention is how many past releases to keep; must be >= 1 + // (backend default: 3). + AtomicRetention *int `json:"atomic_retention,omitempty"` + // SSH / FTP / FTPS / Rsync Hostname string `json:"hostname,omitempty"` Port *int `json:"port,omitempty"` @@ -190,6 +216,8 @@ type ServerCreateRequest struct { } // ServerUpdateRequest is the payload for updating a server. +// Only the fields set here are sent, so an update never disturbs settings the +// caller did not ask to change. type ServerUpdateRequest struct { Name string `json:"name,omitempty"` ProtocolType string `json:"protocol_type,omitempty"` @@ -197,6 +225,16 @@ type ServerUpdateRequest struct { Environment string `json:"environment,omitempty"` RootPath string `json:"root_path,omitempty"` Enabled *bool `json:"enabled,omitempty"` + + // Deployment configuration — same semantics as the matching fields on + // ServerCreateRequest. Atomic in particular cannot be changed once the + // server has been deployed to, and Branch is a pointer so `--branch ""` + // (unpin from a branch) reaches the wire instead of being dropped. + Branch *string `json:"branch,omitempty"` + AutoDeploy *bool `json:"auto_deploy,omitempty"` + Atomic *bool `json:"atomic,omitempty"` + AtomicStrategy string `json:"atomic_strategy,omitempty"` + AtomicRetention *int `json:"atomic_retention,omitempty"` } // ServerGroup represents a group of servers. diff --git a/skill-evals/deployhq/evals.json b/skill-evals/deployhq/evals.json index b8b2ff9..ca9a3c9 100644 --- a/skill-evals/deployhq/evals.json +++ b/skill-evals/deployhq/evals.json @@ -550,7 +550,63 @@ "flags": ["-p", "my-app", "-s", "staging", "--wait", "--timeout", "180"] }, "notes": "Agent should convert 3 minutes to 180 seconds for the --timeout flag." + }, + { + "id": "create-server-deployment-config", + "category": "servers", + "prompt": "Add a staging server to my-app that deploys the staging branch with zero-downtime deployments, and don't let DeployHQ deploy it automatically on push", + "expected": { + "command": "dhq servers create", + "flags": ["-p", "my-app", "--branch", "staging", "--auto-deploy=false", "--atomic"] + }, + "must_not_contain": ["api POST"], + "notes": "Deployment settings are first-class flags on servers create - no dhq api escape hatch needed. Atomic must be set at create time." + }, + { + "id": "update-server-branch", + "category": "servers", + "prompt": "Change server srv-001 in my-app so it deploys the main branch", + "expected": { + "command": "dhq servers update", + "args": ["srv-001"], + "flags": ["-p", "my-app", "--branch", "main"] + }, + "must_not_contain": ["api PUT", "api PATCH"], + "notes": "--branch is a first-class flag on servers update; omitted flags leave other settings untouched." + }, + { + "id": "update-server-auto-deploy", + "category": "servers", + "prompt": "Turn on DeployHQ's automatic deployments for server srv-001 in my-app", + "expected": { + "command": "dhq servers update", + "args": ["srv-001"], + "flags": ["-p", "my-app", "--auto-deploy"] + }, + "must_not_contain": ["api PUT", "api PATCH"], + "notes": "Native auto-deployment is toggled with --auto-deploy / --auto-deploy=false." + }, + { + "id": "update-atomic-retention", + "category": "servers", + "prompt": "Keep the last 10 releases on server srv-001 in my-app", + "expected": { + "command": "dhq servers update", + "args": ["srv-001"], + "flags": ["-p", "my-app", "--atomic-retention", "10"] + }, + "notes": "Retention is tunable on an existing atomic server; only atomic itself is locked after the first deployment. Must be >= 1." + }, + { + "id": "atomic-before-first-deployment", + "category": "gotchas", + "prompt": "Enable zero-downtime deployments on server srv-001 in my-app - it has been serving production traffic for months", + "expected": { + "command": "dhq deployments list", + "flags": ["-p", "my-app"] + }, + "must_not_contain": ["servers update srv-001 --atomic"], + "notes": "Atomic cannot be changed once a server has any deployment. The agent must check for existing deployments or warn the user rather than blindly issuing 'dhq servers update --atomic', which the backend rejects." } ] } - diff --git a/skills/deployhq/SKILL.md b/skills/deployhq/SKILL.md index d50335b..326d010 100644 --- a/skills/deployhq/SKILL.md +++ b/skills/deployhq/SKILL.md @@ -134,6 +134,9 @@ dhq api POST /projects//deployments --body '{"deployment":{...}}' - `dhq deploy --wait` blocks until deployment completes (use `--timeout` to cap) - Deployment `watch` uses TUI in TTY mode, append-only in pipes - `dhq env-vars create` prompts for value if `--value` is omitted (not agent-friendly — always pass `--value`) +- `dhq servers create` / `dhq servers update` configure deployment behaviour with `--branch` (preferred branch), `--auto-deploy` (DeployHQ's native auto-deployment), `--atomic`, `--atomic-strategy` and `--atomic-retention`. On `update`, only flags you explicitly pass are sent — omitted flags never disturb existing settings +- **`--atomic` must be set before the server's first deployment.** The backend refuses to change it once any deployment exists ("cannot be changed after a deployment has been made to this server") and there is no override — the only remedy is a new server. Set it at `create` time; check `dhq deployments list -p --json` before ever putting `--atomic` on an `update` +- `--atomic` fails **silently** on accounts without atomic deployments enabled: the fields are stripped server-side and the request still returns 2xx with atomic off. A zero exit code does not prove it applied — read it back with `dhq servers show -p --json atomic,atomic_strategy,atomic_retention` ## Triggers diff --git a/skills/deployhq/references/servers.md b/skills/deployhq/references/servers.md index 21d20ef..1cf2134 100644 --- a/skills/deployhq/references/servers.md +++ b/skills/deployhq/references/servers.md @@ -29,6 +29,79 @@ Create a server. Flags vary by protocol type. | `--path` | no | Deployment path | | `--environment` | no | Environment label | +**Deployment configuration flags (accepted by both `dhq servers create` and `dhq servers update`):** + +| Flag | Values | Description | +|------|--------|-------------| +| `--branch` | branch name, or `""` | The server's **preferred branch** (e.g. `main`, `staging`). `dhq deploy` deploys this branch when `-b/--branch` is not given. Pass `--branch ""` to **unpin** the server so it falls back to the repository default. **Inert on a grouped server** — see the warning below. | +| `--auto-deploy` | `[=true\|false]` (bare flag = `true`) | DeployHQ's **native repository auto-deployment** — DeployHQ deploys this server itself whenever the preferred branch receives a push. | +| `--atomic` | `[=true\|false]` (bare flag = `true`) | Zero-downtime (**atomic**) deployments — each deploy lands in a new release directory that is swapped in at the end. **Read the warning below before setting this.** | +| `--atomic-strategy` | `copy_release` \| `copy_cache` | How the new release directory is built. `copy_release` (default) copies the previous release, then uploads changes into the new release. `copy_cache` uploads changes into a cache directory and copies the new release from there. | +| `--atomic-retention` | integer `>= 1` | How many past releases to keep on the server. Backend default is `3`. The CLI rejects `0` and negative values locally, before any request is sent. | + +> **Warning — `--atomic` must be set BEFORE the server's first deployment.** +> The backend refuses to change `atomic` once *any* deployment exists for the +> server, failing with *"cannot be changed after a deployment has been made to +> this server"*. **Do not generate a `dhq servers update ... --atomic` command +> for a server that has already been deployed to** — check +> `dhq deployments list -p --json` first, or set `--atomic` at +> `dhq servers create` time. There is no CLI flag or API call that bypasses +> this; the only remedy is to create a new server with atomic enabled. + +Two further backend constraints on `--atomic` — note that they fail *differently*: + +| Constraint | What happens when violated | +|------------|----------------------------| +| Supported only on protocol types `ssh`, `rsync`, `digitalocean`, `hetzner_cloud`, `managed_vps` | **Loud** — real validation error: *"not supported on this server type"* | +| The account must have atomic deployments enabled | **Silent** — the request succeeds (2xx) but atomic is *not* applied | + +> **Warning — `--atomic` fails SILENTLY on accounts without atomic deployments +> enabled.** When the account is not permitted to use atomic deployments, the +> backend strips `atomic`, `atomic_strategy` and `atomic_retention` from the +> request before validation ever runs. The create/update returns **200/201 with +> no error**, and the server comes back with atomic left off. **A successful +> exit code does not prove atomic was applied.** After enabling it, always read +> the setting back: +> +> ```bash +> dhq servers show srv-001 -p my-app --json atomic,atomic_strategy,atomic_retention +> ``` +> +> If `atomic` is `false` after a command that exited 0, the account does not +> have atomic deployments enabled — ask an account admin rather than retrying. +> This is the only atomic failure mode that is silent: an unsupported protocol +> and a change after the first deployment both return real validation errors. + +> **Warning — `--branch` is INERT on a server that belongs to a server group.** +> The backend resolves a server's branch as +> `server_group.branch || server.branch || repository.branch`, so the group's +> branch always wins. Grouped servers are also excluded from auto-deployment +> entirely — the *group* is the deploy target, so even when the group's branch +> is blank the deploy uses the **repository** default, never the server's own +> branch. The write still succeeds and the value is echoed back, so this is a +> silent no-op. The CLI now warns on stderr when it detects this +> (`server_group_identifier` is set on the response), but the exit code is +> still 0. **To change the branch for a grouped server, set it on the server +> group, not the member server** — or remove the server from the group first. + +**Note on `--auto-deploy`:** native auto-deployment is suppressed while the +server belongs to a server group — the backend only auto-deploys a server when +`auto_deploy` is set *and* the server is not in a group. Setting +`--auto-deploy` on a grouped server stores the preference but has no effect +until the server leaves the group. Same dormancy pattern as `--branch` above. + +**Unpinning a branch:** `--branch ""` is sent explicitly (not dropped), and the +backend persists it as an empty string. Every branch consumer treats blank as +"fall back", so the server reverts to the repository default. Note the API +echoes the cleared value back as `""`, not `null` — a read-back check should +treat the two as equivalent. + +```bash +# Unpin a server from its branch; it reverts to the repository default +dhq servers update srv-001 -p my-app --branch "" --json +dhq servers show srv-001 -p my-app --json branch,preferred_branch +``` + **Static Hosting flags (beta, requires managed-resources beta on account):** | Flag | Description | @@ -103,14 +176,90 @@ dhq servers create -p my-app --name "My Site" --protocol-type static_hosting \ # Managed VPS (beta) # Note: requires managed-resources beta enabled; use `dhq launch` for guided setup dhq servers create -p my-app --name "My VPS" --protocol-type managed_vps \ - --region lon1 --size s-1vcpu-1gb --json + --region lon1 --size s-1vcpu-1gb --accept-cost --json + +# Server with deployment configuration set up front +dhq servers create -p my-app --name Production --protocol-type ssh \ + --hostname example.com --username deploy --use-ssh-keys \ + --branch main --auto-deploy --atomic --atomic-retention 5 --json +``` + +**Worked example — two-environment project (staging + production):** + +Staging tracks the `staging` branch and is deployed explicitly from the CLI +(native auto-deployment off). Production tracks `main` and is auto-deployed by +DeployHQ on every push. Both use atomic deployments, so `--atomic` is set here, +at create time — before either server has ever been deployed to. + +```bash +# Note: managed_vps is beta and requires managed-resources beta on the account; +# use `dhq launch` for guided setup of a first Managed VPS. + +# Staging — preferred branch `staging`, native auto-deploy DISABLED, atomic ON +dhq servers create -p my-app --name Staging --protocol-type managed_vps \ + --region lon1 --size s-1vcpu-1gb --accept-cost \ + --branch staging \ + --auto-deploy=false \ + --atomic --atomic-strategy copy_release --atomic-retention 3 --json + +# Production — preferred branch `main`, native auto-deploy ENABLED, atomic ON +dhq servers create -p my-app --name Production --protocol-type managed_vps \ + --region lon1 --size s-2vcpu-2gb --accept-cost \ + --branch main \ + --auto-deploy \ + --atomic --atomic-strategy copy_release --atomic-retention 5 --json + +# Verify atomic actually took effect — a 2xx does NOT prove it was applied +# (accounts without atomic deployments enabled have the fields stripped silently). +# Use the identifiers returned by the two create calls above. +dhq servers show -p my-app \ + --json atomic,atomic_strategy,atomic_retention +dhq servers show -p my-app \ + --json atomic,atomic_strategy,atomic_retention + +# Staging has no native auto-deployment, so ship it explicitly when you want to. +# No -b needed: the server's preferred branch (`staging`) is used. +dhq deploy -p my-app -s Staging --wait --json + +# Production needs no CLI deploy — DeployHQ auto-deploys `main` on push. +# Deploy it manually only when you want an out-of-band release: +dhq deploy -p my-app -s Production --wait --json ``` ### `dhq servers update ` -Update server settings. +Update server settings. Accepts the same **deployment configuration flags** as +`dhq servers create` (`--branch`, `--auto-deploy`, `--atomic`, +`--atomic-strategy`, `--atomic-retention`) — see the table in the `create` +section above for values and constraints. + +**Only the flags you explicitly pass are changed.** Omitted flags are never sent +to the API, so an update never disturbs a setting the operator did not name. + +> **Warning:** `--atomic` cannot be changed on a server that already has a +> deployment — the backend rejects it with *"cannot be changed after a +> deployment has been made to this server"*. Set it at `dhq servers create` +> time. See the full warning in the `create` section above. ```bash +# Rename only — branch, auto-deploy and atomic settings are left untouched dhq servers update srv-001 -p my-app --name "Production v2" --json + +# Change the preferred branch used by `dhq deploy` +dhq servers update srv-001 -p my-app --branch main --json + +# Turn DeployHQ's native auto-deployment on / off +dhq servers update srv-001 -p my-app --auto-deploy --json +dhq servers update srv-001 -p my-app --auto-deploy=false --json + +# Tune an atomic server's release handling +# (the before-first-deployment rule applies to `--atomic` itself) +dhq servers update srv-001 -p my-app --atomic-strategy copy_cache --json +dhq servers update srv-001 -p my-app --atomic-retention 10 --json + +# Enabling atomic — only valid while the server has NO deployments yet, +# and it can succeed with a 2xx without applying, so read the value back +dhq servers update srv-002 -p my-app --atomic --json +dhq servers show srv-002 -p my-app --json atomic,atomic_strategy,atomic_retention ``` ### `dhq servers delete `