diff --git a/CHANGELOG.md b/CHANGELOG.md index 5199e72..1a2f7d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,24 @@ 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. + 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 ### Added diff --git a/internal/commands/launch.go b/internal/commands/launch.go index 083497f..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); err != nil { + if err := launchCheckPlanLimits(env, cfg, caps, client); err != nil { return err } } @@ -1243,13 +1243,37 @@ func projectNameFromRemote(remote string) string { // ── Plan / limit pre-flight ─────────────────────────────────────────────────── -func launchCheckPlanLimits(env *output.Envelope, cfg launchConfig, caps *sdk.AccountCapabilities) 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. + 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 +1281,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..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,11 +915,22 @@ func TestLaunchCheckPlanLimits_StaticIneligible(t *testing.T) { ManagedVPSEligible: true, } cfg := launchConfig{targetProtocol: "static_hosting"} - err := launchCheckPlanLimits(env, cfg, caps) + err := launchCheckPlanLimits(env, cfg, caps, testClient(t)) 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") + // 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) { @@ -895,11 +940,16 @@ func TestLaunchCheckPlanLimits_VPSIneligible(t *testing.T) { ManagedVPSEligible: false, } cfg := launchConfig{targetProtocol: "managed_vps"} - err := launchCheckPlanLimits(env, cfg, caps) + err := launchCheckPlanLimits(env, cfg, caps, testClient(t)) 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") + assert.False(t, le.Retryable) + assert.Equal(t, detect.ProtocolManagedVPS, le.Details["target"]) } func TestLaunchCheckPlanLimits_BothEligible_NoError(t *testing.T) { @@ -911,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)) + assert.NoError(t, launchCheckPlanLimits(env, cfg, caps, testClient(t))) } } 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 |