From 3de50530041ac99274564bede60248611d0b5926 Mon Sep 17 00:00:00 2001 From: Thiago Durante Date: Mon, 17 Aug 2026 15:13:16 +0200 Subject: [PATCH 1/2] fix(launch): correct plan_limit_reached guidance and its dead links The `plan_limit_reached` next-step was wrong twice over. 1. "Free plans support 1 site." is no longer true. Free-plan accounts cannot provision Managed VPS or Static Hosting at all -- both now require a paid plan AND a payment method the metered-resource admission gate accepts. 2. It linked https://app.deployhq.com/account/plan and .../account/billing. Neither is a route. DeployHQ scopes account pages to the account's own subdomain (config/routes.rb wraps them in `constraints subdomain:`), and the real paths are /account/packages and /account/payment_details. So a blocked user following this message got a 404 and still did not learn what to fix. launchCheckPlanLimits now takes the account subdomain (already in scope at the callsite, from the same credentials that produced the client) and builds both URLs from it. Both branches name both requirements, because AccountCapabilities carries only a boolean per resource -- no reason code -- so the CLI genuinely cannot tell whether the plan or the payment method is the problem and must not guess. The specs now assert the URLs and pin both regressions: no `app.deployhq.com`, no "Free plans support". The stale free-plan claim was also embedded in the agent skill reference (skills/deployhq/references/launch.md), which is go:embed-ed into the binary and shipped to every agent that installs the skill; corrected there too. Note two OTHER `app.deployhq.com` links remain in launch.go (lines 565 and 785, both for beta_features, and they disagree with each other about whether the account goes in the host or the path). Left alone as out of scope -- worth a follow-up. Companion to deployhq/deployhq (server-side gate + support articles) and deployhq/billy (card verification signals), both on branch `metered-abuse-controls`. Deploy the DeployHQ change before tagging a CLI release that describes it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014GzfWqNKcEVTDSdXahZWT3 --- CHANGELOG.md | 15 +++++++++++++++ internal/commands/launch.go | 23 +++++++++++++++++++---- internal/commands/launch_test.go | 16 +++++++++++++--- skills/deployhq/references/launch.md | 2 +- 4 files changed, 48 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5199e72..251e538 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,21 @@ 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] + +### Fixed + +- **`dhq launch`**: the `plan_limit_reached` guidance was wrong in two ways and + is corrected. It claimed "Free plans support 1 site", which is no longer true — + free plans cannot provision Managed VPS or Static Hosting at all, both now + requiring a paid plan and an accepted payment method. It also linked + `app.deployhq.com/account/plan` and `app.deployhq.com/account/billing`, neither + of which is a real route: DeployHQ account pages live on the account's own + subdomain, at `https://.deployhq.com/account/packages` and + `.../account/payment_details`. A blocked user following the old message + therefore hit a 404 and still did not know what to fix. The same "free plans + support 1 site" claim is corrected in the embedded agent skill reference. + ## [0.21.0] - 2026-08-06 ### Added diff --git a/internal/commands/launch.go b/internal/commands/launch.go index 083497f..65e21ea 100644 --- a/internal/commands/launch.go +++ b/internal/commands/launch.go @@ -408,7 +408,7 @@ func runLaunch(env *output.Envelope, cfg launchConfig) error { // ── Step 8: Plan / limit pre-flight ───────────────────────────────────── // Only apply eligibility gates when we have real capability data. if capsKnown { - if err := launchCheckPlanLimits(env, cfg, caps); err != nil { + if err := launchCheckPlanLimits(env, cfg, caps, accountSubdomain); err != nil { return err } } @@ -1243,13 +1243,28 @@ func projectNameFromRemote(remote string) string { // ── Plan / limit pre-flight ─────────────────────────────────────────────────── -func launchCheckPlanLimits(env *output.Envelope, cfg launchConfig, caps *sdk.AccountCapabilities) error { +// accountSubdomain is the account permalink, which is also its DeployHQ +// subdomain -- account pages live at https://.deployhq.com/account/... +// and NOT under a shared app host. +func launchCheckPlanLimits(env *output.Envelope, cfg launchConfig, caps *sdk.AccountCapabilities, accountSubdomain string) error { + // Both metered resources are refused for the same two reasons, and + // AccountCapabilities carries only a boolean per resource -- no reason code -- + // so the CLI cannot tell which of the two applies and must name both. + nextStep := func(resource string) string { + return fmt.Sprintf( + "%s requires a paid plan and an accepted payment method. Review your plan at "+ + "https://%s.deployhq.com/account/packages and your payment details at "+ + "https://%s.deployhq.com/account/payment_details", + resource, accountSubdomain, accountSubdomain, + ) + } + if cfg.targetProtocol == detect.ProtocolStaticHosting && !caps.StaticHostingEligible { // Not eligible = plan limit or billing wall return &launchError{ Reason: reasonPlanLimitReached, Message: "Your account cannot provision Static Hosting sites", - NextStep: "Check your plan or billing at https://app.deployhq.com/account/plan. Free plans support 1 site.", + NextStep: nextStep("Static Hosting"), Details: map[string]string{"target": detect.ProtocolStaticHosting}, } } @@ -1257,7 +1272,7 @@ func launchCheckPlanLimits(env *output.Envelope, cfg launchConfig, caps *sdk.Acc return &launchError{ Reason: reasonPlanLimitReached, Message: "Your account cannot provision Managed VPS servers", - NextStep: "Ensure your billing details are set up at https://app.deployhq.com/account/billing", + NextStep: nextStep("Managed VPS"), Details: map[string]string{"target": detect.ProtocolManagedVPS}, } } diff --git a/internal/commands/launch_test.go b/internal/commands/launch_test.go index ae41755..d17bf6e 100644 --- a/internal/commands/launch_test.go +++ b/internal/commands/launch_test.go @@ -881,11 +881,18 @@ func TestLaunchCheckPlanLimits_StaticIneligible(t *testing.T) { ManagedVPSEligible: true, } cfg := launchConfig{targetProtocol: "static_hosting"} - err := launchCheckPlanLimits(env, cfg, caps) + err := launchCheckPlanLimits(env, cfg, caps, "acme") require.Error(t, err) var le *launchError require.True(t, isLaunchErr(err, &le)) assert.Equal(t, reasonPlanLimitReached, le.Reason) + // The guidance must point at pages that exist, on the account's own + // subdomain. It previously named app.deployhq.com/account/plan, which is + // not a route, and claimed free plans support one site, which they do not. + assert.Contains(t, le.NextStep, "https://acme.deployhq.com/account/packages") + assert.Contains(t, le.NextStep, "https://acme.deployhq.com/account/payment_details") + assert.NotContains(t, le.NextStep, "app.deployhq.com") + assert.NotContains(t, le.NextStep, "Free plans support") } func TestLaunchCheckPlanLimits_VPSIneligible(t *testing.T) { @@ -895,11 +902,14 @@ func TestLaunchCheckPlanLimits_VPSIneligible(t *testing.T) { ManagedVPSEligible: false, } cfg := launchConfig{targetProtocol: "managed_vps"} - err := launchCheckPlanLimits(env, cfg, caps) + err := launchCheckPlanLimits(env, cfg, caps, "acme") require.Error(t, err) var le *launchError require.True(t, isLaunchErr(err, &le)) assert.Equal(t, reasonPlanLimitReached, le.Reason) + assert.Contains(t, le.NextStep, "https://acme.deployhq.com/account/packages") + assert.Contains(t, le.NextStep, "https://acme.deployhq.com/account/payment_details") + assert.NotContains(t, le.NextStep, "app.deployhq.com") } func TestLaunchCheckPlanLimits_BothEligible_NoError(t *testing.T) { @@ -911,7 +921,7 @@ func TestLaunchCheckPlanLimits_BothEligible_NoError(t *testing.T) { } for _, proto := range []string{"static_hosting", "managed_vps"} { cfg := launchConfig{targetProtocol: proto} - assert.NoError(t, launchCheckPlanLimits(env, cfg, caps)) + assert.NoError(t, launchCheckPlanLimits(env, cfg, caps, "acme")) } } diff --git a/skills/deployhq/references/launch.md b/skills/deployhq/references/launch.md index e023f4d..108039f 100644 --- a/skills/deployhq/references/launch.md +++ b/skills/deployhq/references/launch.md @@ -63,7 +63,7 @@ On failure the error carries a stable `reason`, a `retryable` boolean, and a `ne | `beta_enroll_required` | Managed-resources beta not enabled and the user isn't an admin — `details.admin_required=true`; an admin enables it (or use your own server via `dhq init`) | | `accept_cost_required` | Managed VPS requested non-interactively without `--accept-cost` — re-run with `--accept-cost` | | `repo_unreachable` | No git remote DeployHQ can deploy from — push a remote / connect a provider first | -| `plan_limit_reached` | Free-plan limit hit (e.g. 1 static site) — upgrade or remove an existing resource | +| `plan_limit_reached` | The account cannot provision this managed resource — Managed VPS and Static Hosting both require a paid plan and an accepted payment method. Upgrade the plan, fix the billing details, or remove an existing resource if a per-plan cap was hit | | `subdomain_taken` | Static Hosting subdomain already in use — choose another `--subdomain` | | `rate_limited` | Per-account provisioning rate limit hit (HTTP 429) — **retryable** (`retryable: true`); back off for `details.retry_after` seconds and re-run the same command. Distinct from `plan_limit_reached` (a hard 422 cap) | | `provision_failed` | The server failed to provision — check the named resource; retry | From f7119cd27e1f9759f738295b84d3890300da5a5f Mon Sep 17 00:00:00 2001 From: Thiago Durante Date: Tue, 18 Aug 2026 14:19:24 +0200 Subject: [PATCH 2/2] fix(launch): build the plan-limit links from the client, not the raw account Codex P2, and it defeated the point of this PR for a whole class of users. pkg/sdk/client.go explicitly tolerates DEPLOYHQ_ACCOUNT being given as a full hostname and trims the suffix -- its own comment names the failure it is avoiding ("otherwise we'd build .deployhq.com.deployhq.com"). This guidance interpolated the RAW credential, so anyone using that supported form got https://acme.deployhq.com.deployhq.com/account/packages: still a dead link, which is precisely the bug the rewrite was meant to fix. launchCheckPlanLimits now takes the *sdk.Client (already in scope at the callsite) and reads Client.Account(), which derives the subdomain from the already-normalised base URL. Reusing that accessor rather than duplicating the TrimSuffix matters twice over: the normalisation stays in one place instead of two that can drift, and a WithBaseURL override is honoured, which the hardcoded ".deployhq.com" also got wrong. Test coverage: - a full-hostname account produces acme.deployhq.com and never the doubled host - the ineligible cases now also pin Retryable == false and Details["target"] (CodeRabbit): the PR description asserts those fields are unchanged, and nothing was holding that contract in place for JSON consumers. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014GzfWqNKcEVTDSdXahZWT3 --- CHANGELOG.md | 3 +++ internal/commands/launch.go | 19 +++++++++---- internal/commands/launch_test.go | 46 +++++++++++++++++++++++++++++--- 3 files changed, 60 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 251e538..1a2f7d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,9 @@ While the CLI is pre-1.0, minor versions may carry breaking changes to the publi `.../account/payment_details`. A blocked user following the old message therefore hit a 404 and still did not know what to fix. The same "free plans support 1 site" claim is corrected in the embedded agent skill reference. + The links are built from the SDK client's normalised account rather than the + raw credential, so users who set `DEPLOYHQ_ACCOUNT` to a full hostname + (`acme.deployhq.com`) no longer get `acme.deployhq.com.deployhq.com`. ## [0.21.0] - 2026-08-06 diff --git a/internal/commands/launch.go b/internal/commands/launch.go index 65e21ea..92b029c 100644 --- a/internal/commands/launch.go +++ b/internal/commands/launch.go @@ -408,7 +408,7 @@ func runLaunch(env *output.Envelope, cfg launchConfig) error { // ── Step 8: Plan / limit pre-flight ───────────────────────────────────── // Only apply eligibility gates when we have real capability data. if capsKnown { - if err := launchCheckPlanLimits(env, cfg, caps, accountSubdomain); err != nil { + if err := launchCheckPlanLimits(env, cfg, caps, client); err != nil { return err } } @@ -1243,10 +1243,19 @@ func projectNameFromRemote(remote string) string { // ── Plan / limit pre-flight ─────────────────────────────────────────────────── -// accountSubdomain is the account permalink, which is also its DeployHQ -// subdomain -- account pages live at https://.deployhq.com/account/... -// and NOT under a shared app host. -func launchCheckPlanLimits(env *output.Envelope, cfg launchConfig, caps *sdk.AccountCapabilities, accountSubdomain string) error { +// Account pages live at https://.deployhq.com/account/... and NOT under +// a shared app host, so the links have to be built per account. +// +// The subdomain is taken from the CLIENT, not from the raw credential. Users may +// supply DEPLOYHQ_ACCOUNT as a full hostname ("acme.deployhq.com") -- pkg/sdk +// explicitly tolerates that and trims the suffix -- so interpolating the raw +// value produced "acme.deployhq.com.deployhq.com", a dead link, which is exactly +// the defect this guidance was rewritten to fix. Client.Account() derives it +// from the already-normalised base URL, so a WithBaseURL override is honoured +// too, and the normalisation lives in one place rather than two that can drift. +func launchCheckPlanLimits(env *output.Envelope, cfg launchConfig, caps *sdk.AccountCapabilities, client *sdk.Client) error { + accountSubdomain := client.Account() + // Both metered resources are refused for the same two reasons, and // AccountCapabilities carries only a boolean per resource -- no reason code -- // so the CLI cannot tell which of the two applies and must name both. diff --git a/internal/commands/launch_test.go b/internal/commands/launch_test.go index d17bf6e..bc4fe44 100644 --- a/internal/commands/launch_test.go +++ b/internal/commands/launch_test.go @@ -873,6 +873,40 @@ func TestLaunchError_ErrorMethodNoNextStep(t *testing.T) { // ── Integration: plan_limit_reached ────────────────────────────────────────── +// testClient builds a client for the plan-limit tests. Deliberately a real +// sdk.Client rather than a bare string: the URLs under test are derived from +// the client's normalised base URL, and that normalisation is the thing these +// tests exist to protect. +func testClient(t *testing.T) *sdk.Client { + t.Helper() + c, err := sdk.NewPublic("acme") + require.NoError(t, err) + return c +} + +// Codex P2. pkg/sdk/client.go tolerates DEPLOYHQ_ACCOUNT being given as a full +// hostname and trims the suffix — its comment names this exact failure. Building +// the guidance URLs from the RAW credential therefore produced +// acme.deployhq.com.deployhq.com, i.e. still a dead link, which is the very bug +// this PR exists to fix. Derived from the client now, so the normalisation (and +// any WithBaseURL override) is honoured in one place. +func TestLaunchCheckPlanLimits_NormalisesFullHostnameAccount(t *testing.T) { + env, _, _ := testLaunchEnvelope() + client, err := sdk.NewPublic("acme.deployhq.com") + require.NoError(t, err) + + caps := &sdk.AccountCapabilities{BetaFeatures: true, StaticHostingEligible: false} + cfg := launchConfig{targetProtocol: "static_hosting"} + + checkErr := launchCheckPlanLimits(env, cfg, caps, client) + require.Error(t, checkErr) + var le *launchError + require.True(t, isLaunchErr(checkErr, &le)) + + assert.Contains(t, le.NextStep, "https://acme.deployhq.com/account/packages") + assert.NotContains(t, le.NextStep, "deployhq.com.deployhq.com") +} + func TestLaunchCheckPlanLimits_StaticIneligible(t *testing.T) { env, _, _ := testLaunchEnvelope() caps := &sdk.AccountCapabilities{ @@ -881,7 +915,7 @@ func TestLaunchCheckPlanLimits_StaticIneligible(t *testing.T) { ManagedVPSEligible: true, } cfg := launchConfig{targetProtocol: "static_hosting"} - err := launchCheckPlanLimits(env, cfg, caps, "acme") + err := launchCheckPlanLimits(env, cfg, caps, testClient(t)) require.Error(t, err) var le *launchError require.True(t, isLaunchErr(err, &le)) @@ -893,6 +927,10 @@ func TestLaunchCheckPlanLimits_StaticIneligible(t *testing.T) { assert.Contains(t, le.NextStep, "https://acme.deployhq.com/account/payment_details") assert.NotContains(t, le.NextStep, "app.deployhq.com") assert.NotContains(t, le.NextStep, "Free plans support") + // The PR contract says these fields are unchanged; pin them so a future + // error-construction change cannot break JSON consumers silently. + assert.False(t, le.Retryable) + assert.Equal(t, detect.ProtocolStaticHosting, le.Details["target"]) } func TestLaunchCheckPlanLimits_VPSIneligible(t *testing.T) { @@ -902,7 +940,7 @@ func TestLaunchCheckPlanLimits_VPSIneligible(t *testing.T) { ManagedVPSEligible: false, } cfg := launchConfig{targetProtocol: "managed_vps"} - err := launchCheckPlanLimits(env, cfg, caps, "acme") + err := launchCheckPlanLimits(env, cfg, caps, testClient(t)) require.Error(t, err) var le *launchError require.True(t, isLaunchErr(err, &le)) @@ -910,6 +948,8 @@ func TestLaunchCheckPlanLimits_VPSIneligible(t *testing.T) { assert.Contains(t, le.NextStep, "https://acme.deployhq.com/account/packages") assert.Contains(t, le.NextStep, "https://acme.deployhq.com/account/payment_details") assert.NotContains(t, le.NextStep, "app.deployhq.com") + assert.False(t, le.Retryable) + assert.Equal(t, detect.ProtocolManagedVPS, le.Details["target"]) } func TestLaunchCheckPlanLimits_BothEligible_NoError(t *testing.T) { @@ -921,7 +961,7 @@ func TestLaunchCheckPlanLimits_BothEligible_NoError(t *testing.T) { } for _, proto := range []string{"static_hosting", "managed_vps"} { cfg := launchConfig{targetProtocol: proto} - assert.NoError(t, launchCheckPlanLimits(env, cfg, caps, "acme")) + assert.NoError(t, launchCheckPlanLimits(env, cfg, caps, testClient(t))) } }