From 50ed81e5afb87f280cac5d81f65ea79d098aea22 Mon Sep 17 00:00:00 2001 From: Thiago Durante Date: Wed, 5 Aug 2026 08:52:58 +0200 Subject: [PATCH 1/3] feat(servers): expose branch, auto-deploy and atomic deployment flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add five first-class deployment-configuration flags to `dhq servers create` and `dhq servers update`, so automation no longer has to drop down to the generic `dhq api` escape hatch: --branch, --auto-deploy, --atomic, --atomic-strategy, --atomic-retention A shared `serverDeploymentFlags` helper backs both commands so they cannot drift apart. Only flags the operator explicitly supplied are sent, so an update never disturbs a setting that was not named. Booleans and retention reach the SDK as pointers: an omitted flag stays off the wire while an explicit `--auto-deploy=false` still serialises as `false`, and a zero retention is never sent into the backend's `>= 1` validation. Validation of `--atomic-strategy` and `--atomic-retention` runs before the project is resolved and before the API client is built, so a malformed invocation fails offline with no credentials touched. Branch handling, verified against the DeployHQ Rails source: * `--branch ""` unpins a server so it falls back to the repository default. `IGNORE_PARAMS_ON_BLANK` covers only credential params, so the backend accepts and persists a blank branch; every consumer resolves it with `.presence`. `Branch` is therefore a `*string` — as a plain string with omitempty the empty value was dropped and the command reported success having changed nothing. Note the API echoes it back as `""`, not null. * Setting `--branch` on a server in a server group now warns on stderr. 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. The write succeeds and is echoed back, making it a silent no-op; Rails hides the field in its UI, the API does not. stdout stays pure data. Tests: adds a capturing transport that pins the command -> request seam end to end. Both call sites could previously be deleted with the entire suite still green; the new tests fail when the wiring is removed (mutation-tested). Suite goes 853 -> 863, clean under -race and go vet. Refs DHQ-691 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Hibw5xsRNGDkYzr1hXsDQz --- CHANGELOG.md | 37 ++ internal/commands/servers.go | 221 ++++++++- internal/commands/servers_test.go | 530 +++++++++++++++++++++ pkg/sdk/server_deployment_settings_test.go | 180 +++++++ pkg/sdk/types.go | 38 ++ skill-evals/deployhq/evals.json | 57 ++- skills/deployhq/SKILL.md | 3 + skills/deployhq/references/servers.md | 151 +++++- 8 files changed, 1214 insertions(+), 3 deletions(-) create mode 100644 internal/commands/servers_test.go create mode 100644 pkg/sdk/server_deployment_settings_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index c274e40..0d36781 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,42 @@ 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 + `--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 +112,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..20a56ea 100644 --- a/internal/commands/servers.go +++ b/internal/commands/servers.go @@ -9,8 +9,186 @@ 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 != "" +} + +// 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 +359,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 +389,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 +405,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 +456,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 +569,8 @@ func newServersCreateCmd() *cobra.Command { } } + warnIfBranchDormant(env, deployFlags.supplied("branch"), server) + if env.WantsJSON() { return env.WriteJSON(output.NewResponse(server, fmt.Sprintf("Created server: %s", server.Name))) } @@ -392,6 +585,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 +643,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 +677,18 @@ 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) + if env.WantsJSON() { return env.WriteJSON(output.NewResponse(server, fmt.Sprintf("Updated server: %s", server.Name))) } @@ -482,6 +700,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..37c542d --- /dev/null +++ b/internal/commands/servers_test.go @@ -0,0 +1,530 @@ +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)) + }) + } +} 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..1a7de14 100644 --- a/skill-evals/deployhq/evals.json +++ b/skill-evals/deployhq/evals.json @@ -550,7 +550,62 @@ "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"] + }, + "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..0d0d6de 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 | @@ -104,13 +177,89 @@ dhq servers create -p my-app --name "My Site" --protocol-type static_hosting \ # 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 + +# 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 \ + --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 \ + --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 ` From d816eaba8bc1b02e6cdd31c684fb37ebaa280584 Mon Sep 17 00:00:00 2001 From: Thiago Durante Date: Wed, 5 Aug 2026 09:00:21 +0200 Subject: [PATCH 2/3] fix(servers): surface silent atomic strip, pin billing guard, correct fixture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the DHQ-691 review. Four issues, all verified against the DeployHQ Rails source or by control test. Report the silent atomic strip instead of documenting it. An account without atomic deployments enabled has `atomic`, `atomic_strategy` and `atomic_retention` removed by the backend's permit list before any validation runs, so the call returns 2xx with atomic off and no error. The CLI already holds the create/update response — which is exactly the read-back the docs told operators to perform — so it now compares intent against the returned server and warns on stderr. No extra request; stdout stays pure data. The three params are permitted as a group, so checking `atomic` covers all of them; the other two atomic failure modes (unsupported protocol, change after the first deployment) return real validation errors and need no client check. Pin the Managed VPS billing guard with a regression test. The `--accept-cost` gate is the only thing between a non-interactive invocation and a billable provisioning call, and this branch inserted statements on both sides of it. The pre-existing test only asserted the flag was registered and never ran the command. The new test drives the real command with a network tripwire, so it fails if the guard is ever moved below the API call (mutation-verified). Make the atomic-before-first-deployment eval able to fail. run-evals.sh only checks that a response contains the expected command, so a response emitting both the deployments-list check and the forbidden `servers update --atomic` scored as a pass — the eval could not fail for the footgun it was added to catch. Adds the matching must_not_contain. Correct a fabricated value in the golden fixture. integration_test.go carried `"atomic_strategy": "symlink"` in two places, but no such strategy exists in the backend: the column defaults to copy_release, the only behavioural branch tests for copy_cache, the UI offers just those two, and no migration mentions symlink. CLAUDE.md advertises that file as validating against real API JSON, so the fixture was actively misleading. Suite goes 863 -> 871, clean under -race and go vet. Refs DHQ-691 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Hibw5xsRNGDkYzr1hXsDQz --- CHANGELOG.md | 6 +++ internal/commands/servers.go | 32 +++++++++++++++ internal/commands/servers_test.go | 65 +++++++++++++++++++++++++++++++ pkg/sdk/integration_test.go | 4 +- skill-evals/deployhq/evals.json | 1 + 5 files changed, 106 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d36781..e98934a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,12 @@ While the CLI is pre-1.0, minor versions may carry breaking changes to the publi 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 || diff --git a/internal/commands/servers.go b/internal/commands/servers.go index 20a56ea..b72280a 100644 --- a/internal/commands/servers.go +++ b/internal/commands/servers.go @@ -152,6 +152,36 @@ func branchIsDormant(branchSupplied bool, server *sdk.Server) bool { 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) { @@ -570,6 +600,7 @@ 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))) @@ -688,6 +719,7 @@ func newServersUpdateCmd() *cobra.Command { 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))) diff --git a/internal/commands/servers_test.go b/internal/commands/servers_test.go index 37c542d..3ce1267 100644 --- a/internal/commands/servers_test.go +++ b/internal/commands/servers_test.go @@ -528,3 +528,68 @@ func TestBranchIsDormant(t *testing.T) { }) } } + +// ── #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/skill-evals/deployhq/evals.json b/skill-evals/deployhq/evals.json index 1a7de14..ca9a3c9 100644 --- a/skill-evals/deployhq/evals.json +++ b/skill-evals/deployhq/evals.json @@ -605,6 +605,7 @@ "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." } ] From 681f3bc99ef4b353f34a3c2cc77b39ff0b9c6de1 Mon Sep 17 00:00:00 2001 From: Thiago Durante Date: Wed, 5 Aug 2026 09:33:49 +0200 Subject: [PATCH 3/3] docs(servers): add --accept-cost to the Managed VPS examples Every `dhq servers create --protocol-type managed_vps` example in the servers reference passed `--json` but omitted `--accept-cost`, so each one fails before creating anything: "Managed VPS creation requires --accept-cost (free for early customers during beta, billed monthly afterwards)" The guard at internal/commands/servers.go:514-519 fires whenever the session is non-TTY, non-interactive, or in JSON mode, so the examples break in any agent or CI context, not only when --json is present. Verified against the built binary with credentials resolvable: without the flag the command stops at the gate, with it the request reaches the API. Fixes the two examples added by this branch's worked two-environment section, plus the pre-existing "My VPS" example above them, which carried the same defect on main and would have been left broken next to the corrected ones. Caught by Codex on #38. Refs DHQ-691 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Hibw5xsRNGDkYzr1hXsDQz --- skills/deployhq/references/servers.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/skills/deployhq/references/servers.md b/skills/deployhq/references/servers.md index 0d0d6de..1cf2134 100644 --- a/skills/deployhq/references/servers.md +++ b/skills/deployhq/references/servers.md @@ -176,7 +176,7 @@ 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 \ @@ -197,14 +197,14 @@ at create time — before either server has ever been deployed to. # 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 \ + --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 \ + --region lon1 --size s-2vcpu-2gb --accept-cost \ --branch main \ --auto-deploy \ --atomic --atomic-strategy copy_release --atomic-retention 5 --json