feat: dhq launch — one-command deploy to Static Hosting & Managed VPS#25
Open
thdurante wants to merge 23 commits into
Open
feat: dhq launch — one-command deploy to Static Hosting & Managed VPS#25thdurante wants to merge 23 commits into
thdurante wants to merge 23 commits into
Conversation
Add Server nested provisioning blocks (static_hosting, managed_vps) and the ServerCreateRequest managed params. Those params (hosted_website_attributes, region, size, os_image) are hoisted by CreateServer to top-level siblings of `server` — the backend reads params[:region] / params[:hosted_website_attributes], not params[:server][:...]. Includes create-shape contract tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
EnrollBeta (POST /beta/enrollments), GetAccountCapabilities (GET /profile — non-admin-readable), ListManagedHostingRegions/Sizes, GetServerProvisioningState plus LiveURL/ProvisioningStatus/IsProvisioning helpers reading the nested blocks. Signup now sends terms_accepted + client, models email_verified and the 2FA 422 fallback (TwoFactorError). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ribution Adds the two protocols to the create wizard/flags (--subdomain/--spa-mode/ --subdirectory, --region/--size/--os-image) and protocol lists. The signup command sends terms_accepted and client=dhq-cli for attribution. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
internal/detect reads root project files and returns {Framework, SuggestedProtocol,
BuildCommand, OutputDir, SPA}, mirroring the web funnel's coarse mapping
(node-app -> static_hosting, server-runtime -> managed_vps).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
New `dhq launch` command, non-interactive-first (D7): auth/bootstrap →
framework detect → capability + beta-enroll pre-flight → repo-deployability
check → target select → project/repo → plan-limit pre-flight → provision
(static or managed VPS, polling to active) → build config → deploy → print
the live URL → persist to .deployhq.toml. Agent/CI surface: --json, --dry-run
(no side effects, shows cost), --accept-cost (billable VPS guardrail — never
provisions under --yes alone), --non-interactive/--interactive, harness
auto-detection, and deterministic structured errors {error,next_step,details}
(auth_required, beta_enroll_required, accept_cost_required, repo_unreachable,
plan_limit_reached, subdomain_taken, provision_failed, deploy_failed). In
non-interactive mode beta enrollment is attempted directly (idempotent +
admin-gated server-side). Registered in root; deploy's no-project hint points
to launch.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rors, dry-run Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the dhq launch entry to the agent-metadata table consumed by `dhq commands --json` (and by agents): interactive, idempotent re-runs, requires_confirmation (provisions billable resources), supports_json, safe_for_automation, resource_types project/server/deployment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
README feature section + Quick Start entry, and the agent skill guide (SKILL.md command table, a one-command decision tree, a trigger) plus a new references/launch.md covering the flag set, the non-interactive/--json/--dry-run contract, the --accept-cost billable guardrail, and the structured-error reasons agents can branch on. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add Server (string) and Target (string) fields to config.Config with mapstructure + json tags so `dhq launch` can persist and re-read the provisioned server identifier across runs. Also add "server" and "target" to the Keys slice so the source-tracking and env-binding logic covers them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…beta, caps, branch, detection, cleanup Fix 1 (idempotency): add serverID to launchConfig, read from .deployhq.toml via resolveLaunchConfig; in runLaunch call GetServerProvisioningState before provisioning — skip CreateServer when an existing server resolves. Fix 2 (dry-run side effects): move dry-run exit to BEFORE beta enrollment and any mutation; pass command ctx to launchDryRun instead of context.Background(). Fix 3 (interactive target → 403): evaluate isManagedTarget and run beta enrollment AFTER target selection so interactive picks are covered before the enroll attempt. Fix 5 (provision failure cleanup): on provision error in runLaunch, run launchDeployFailureCleanup (same path as deploy failures) so --cleanup-on-failure and named-resource hint apply to provision timeouts/failures too. Fix 6 (repo errors swallowed): treat repo-connect failures as terminal launchErrors (reasonRepoUnreachable) on the project-reuse, auto-select, and interactive-pick paths — not just the new-project path. Fix 7 (detection defaults): seed cfg.subdirectory from detection.OutputDir and cfg.spaMode from detection.SPA before calling launchProvisionStatic when flags unset. Fix 8 (caps 404 → plan_limit_reached): introduce capsKnown bool; when caps endpoint returns 404 set capsKnown=false and skip the eligibility/plan-limit gate so the backend's CreateServer is the authority. Fix 9 (branch hardcoded): replace `"main"` fallback in launchEnsureRepo with detectDefaultBranch() which tries git symbolic-ref then origin/HEAD; returns "" so the API resolves the repo default when local git provides nothing. Cheap fixes: - Use errors.As in writeLaunchError (not bare type assertion) for wrapped errors. - Terminate pollProvisioningState immediately on 401/403/404 (non-retryable). - Add runGitCommand helper for git sub-commands. - Extract launchGetCaps helper to avoid duplicated caps-fetch logic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ed_vps A billable Managed VPS must never be created non-interactively without explicit cost acknowledgement. Mirror the gate from `dhq launch --vps`: - Add --accept-cost bool flag to newServersCreateCmd. - In non-interactive mode (non-TTY, --non-interactive, --json) require the flag or return a UserError before any API call. - In interactive mode, prompt to confirm when --accept-cost is absent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t gate, cleanup, repo errors New tests: - TestLaunchIdempotent_SecondRunSkipsProvision: re-run with a persisted serverID must call GET /servers/:id and NOT POST /servers. - TestLaunchDryRun_NoBetaEnroll: --dry-run must never POST /beta/enrollments even when caps.BetaFeatures is false. - TestServersCreate_ManagedVPS_RequiresAcceptCost_NonInteractive: --accept-cost flag is registered; non-interactive managed_vps create without it returns error. - TestLaunchProvisionFailure_CleanupOnFailure_DeletesCalled: provision failure with --cleanup-on-failure issues DELETE /servers/:id. - TestLaunchEnsureProject_RepoConnectFailure_IsTerminal: CreateRepository 422 returns a repo_unreachable launchError before provision. Also: - Update isLaunchErr helper to use errors.As (wrapped errors). - Add "errors" import. - Update launchDryRun calls to pass t.Context() (new signature). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new dhq launch command implementing detect → provision → deploy for DeployHQ-managed Static Hosting and Managed VPS, plus framework detection, SDK managed-hosting helpers, servers CLI flags/guardrails, config persistence, extensive tests, and documentation updates. ChangesManaged Hosting Launch Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
internal/commands/launch_test.go (2)
27-27: 💤 Low valueConsider configuring Logger writer destination.
The Logger is initialized as an empty struct. Based on the comment at lines 832-834, tests don't need a real log file. Consider explicitly setting the Logger's writer to
io.Discardor one of the test buffers to ensure log writes don't fail or panic if the Logger implementation expects a configured writer.📝 Suggested fix
env := &output.Envelope{ Stdout: &stdout, Stderr: &stderr, - Logger: &output.Logger{}, + Logger: &output.Logger{Writer: io.Discard}, IsTTY: false, NonInteractive: true, JSONMode: false,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/commands/launch_test.go` at line 27, The Logger in the test setup is created as an empty output.Logger{} which may lack a configured writer and cause panics on write; update the test initialization that sets Logger to a configured instance by assigning its writer (e.g., set the Logger.Writer field to io.Discard or a test buffer) so log writes in the launch_test.go test use a safe destination; locate the logger setup where Logger: &output.Logger{} is created and replace it with an instance whose writer is explicitly set.
711-738: ⚡ Quick winTest validates flag presence but not the cost-acceptance guard behavior.
The test name
TestServersCreate_ManagedVPS_RequiresAcceptCost_NonInteractivesuggests it validates that creating a managed_vps server without--accept-costreturns an error in non-interactive mode. However, the test only verifies that the--accept-costflag is registered (line 735) and that no API calls were made before the flag check (line 737).Consider adding an actual command execution test that:
- Calls
servers createwithmanaged_vpsprotocol- Without
--accept-costin non-interactive mode- Expects a UserError before any API call
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/commands/launch_test.go` around lines 711 - 738, The test currently only checks flag registration; update TestServersCreate_ManagedVPS_RequiresAcceptCost_NonInteractive to actually execute the servers create command in non-interactive mode and assert the accept-cost guard fires: set cliCtx to a non-interactive context (use the shared cliCtx variable), build the root command via NewRootCmd("test"), set args to run "servers create" with --protocol=managed_vps but without --accept-cost, execute the command (via ExecuteC/Execute) and assert it returns a UserError and that apiCalled remains false (no API requests were made). Ensure you reset cliCtx after the test and keep the existing apiCalled/http test server setup and assertions.docs/SKILL.md (1)
110-145: ⚡ Quick winAdd language identifiers to fenced code blocks.
The fenced code blocks at lines 110, 116, 122, 129, 135, and 141 are missing language identifiers, which prevents proper syntax highlighting. Add
bashorshellidentifiers to these blocks.📝 Suggested fix
### "Check account beta/managed-offerings eligibility" -``` +```bash dhq api GET /account/capabilities --json"Enable managed-resources beta from CLI"
-
+bash
dhq api POST /beta/enrollments --body '{"protocol":"static_hosting"}'### "List Managed VPS regions and sizes" -``` +```bash dhq api GET /managed_hosting/regions --json dhq api GET /managed_hosting/sizes --json"Create a Static Hosting server"
-
+bash
dhq servers create -p --name "My Site" --protocol-type static_hosting
--subdomain --subdirectory dist --json### "Create a Managed VPS server" -``` +```bash dhq servers create -p <project> --name "My VPS" --protocol-type managed_vps \ --region lon1 --size s-1vcpu-1gb --json"Poll provisioning status"
-
+bash
dhq servers show -p --json🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/SKILL.md` around lines 110 - 145, Add bash language identifiers to the fenced code blocks that contain the dhq CLI examples so they get syntax highlighting: locate the blocks starting with the commands "dhq api GET /account/capabilities --json", "dhq api POST /beta/enrollments --body '{\"protocol\":\"static_hosting\"}'", the two-line block with "dhq api GET /managed_hosting/regions --json" and "dhq api GET /managed_hosting/sizes --json", the static hosting create block starting "dhq servers create -p <project> --name \"My Site\" --protocol-type static_hosting ...", the managed VPS create block starting "dhq servers create -p <project> --name \"My VPS\" --protocol-type managed_vps ...", and the polling block "dhq servers show <server-id> -p <project> --json"; for each, change the opening triple backticks from ``` to ```bash (or ```shell) so the blocks render with shell highlighting.Source: Linters/SAST tools
internal/commands/launch.go (1)
182-194: 💤 Low valueConsider adding explicit SPA mode flags for non-interactive override.
Detection automatically sets
spaModewhen a framework is recognized as an SPA (line 354), and the interactive flow prompts for it (lines 1005-1011). However, non-interactive users cannot override the detection result if they need to force SPA or non-SPA routing.Adding
--spaand--no-spaflags would allow explicit control in CI/agent contexts when detection guesses incorrectly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/commands/launch.go` around lines 182 - 194, Add explicit SPA override flags to the command by declaring new boolean flag variables (e.g., flagSPA and flagNoSPA) via cmd.Flags().BoolVar calls alongside the existing flags (matching the style used for flagStatic, flagVPS, flagNonInteract, etc.); then make the SPA detection/assignment logic (the spaMode decision around the detection at the spa detection location and the interactive prompt handling at the interactive prompt location) respect these flags: if flagSPA is true force spaMode=true, if flagNoSPA is true force spaMode=false, and have these explicit flags take precedence over automatic detection and interactive prompts (including in non-interactive/--yes flows). Ensure mutual exclusion/validation so both flags cannot be set simultaneously and update any help text to document --spa and --no-spa behavior.internal/detect/detect.go (1)
155-155: ⚡ Quick winOverly broad Hugo detection heuristic may cause false positives.
The
baseURLsubstring check in a genericconfig.tomlcould match non-Hugo projects (e.g., a JSON config for a custom SSG). Per the package's design principle (line 13), false negatives are preferred over false positives.Consider requiring a Hugo-specific config file (
hugo.toml,hugo.yaml,hugo.json) OR tightening theconfig.tomlcheck to verify additional Hugo-specific keys (e.g.,theme,languageCode) to reduce false-positive risk.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/detect/detect.go` at line 155, The current Hugo detection in detect.go uses has(...) or checks config.toml for the substring "baseURL", which is too broad and causes false positives; update the conditional in the Hugo detection (the has, readFile, and fileContains usage) to either only accept explicit Hugo config files (hugo.toml, hugo.yaml, hugo.json) or, if config.toml must be supported, require additional Hugo-specific keys (e.g., "theme", "languageCode", or "baseURL =" with TOML syntax) by checking for multiple key substrings in the fileContents returned by readFile before returning true; ensure you modify the same conditional that currently reads has("hugo.toml", "hugo.yaml", "hugo.json") || (has("config.toml") && fileContains(readFile("config.toml"), "baseURL")) so false positives are avoided.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/commands/launch_test.go`:
- Around line 104-130: TestLaunchErrorAuthRequired_NonInteractive currently only
checks flag registration; either rename it to TestLaunchCommandFlagRegistration
or implement the promised auth_required behavior: set up cliCtx to a minimal
stub that simulates "no credentials" (override cliCtx), create the root command
via NewRootCmd("test"), set args ["launch","--non-interactive",...], execute the
command (cmd.Execute() or ExecuteC()) and assert the returned error is a
*launchError whose reason equals reasonAuthRequired; reference
TestLaunchErrorAuthRequired_NonInteractive, cliCtx, NewRootCmd,
cmd.Execute/ExecuteC, launchError and reasonAuthRequired when making the
changes.
In `@internal/commands/launch.go`:
- Around line 1399-1419: The function launchDeployFailureCleanup computes
liveURL and urlDisplay but never uses urlDisplay (and liveURL is only used to
set urlDisplay), causing a linter error; remove the unused urlDisplay variable
and its assignment (and if liveURL is not used elsewhere in
launchDeployFailureCleanup remove its assignment too) or alternatively
incorporate liveURL/urlDisplay into the user-facing messages (e.g., replace
server.Identifier or server.Name with urlDisplay) — update the code in
launchDeployFailureCleanup to either delete the urlDisplay and liveURL lines or
to use urlDisplay in the env.Warn / env.Status messages so no unused variable
remains.
In `@internal/detect/detect.go`:
- Around line 119-130: Remove the ineffectual initial assignment to
FrameworkDjango and set the framework only once based on the actual condition:
call has("manage.py") and fileContains(readFile("requirements.txt"), "django")
to pick FrameworkDjango otherwise use FrameworkUnknown, then return
Result{Framework: framework, SuggestedProtocol: ProtocolManagedVPS}; update the
block around the has("requirements.txt", "Pipfile", "pyproject.toml") check to
avoid assigning framework twice (refer to symbols: has, fileContains, readFile,
FrameworkDjango, FrameworkUnknown, Result, ProtocolManagedVPS).
---
Nitpick comments:
In `@docs/SKILL.md`:
- Around line 110-145: Add bash language identifiers to the fenced code blocks
that contain the dhq CLI examples so they get syntax highlighting: locate the
blocks starting with the commands "dhq api GET /account/capabilities --json",
"dhq api POST /beta/enrollments --body '{\"protocol\":\"static_hosting\"}'", the
two-line block with "dhq api GET /managed_hosting/regions --json" and "dhq api
GET /managed_hosting/sizes --json", the static hosting create block starting
"dhq servers create -p <project> --name \"My Site\" --protocol-type
static_hosting ...", the managed VPS create block starting "dhq servers create
-p <project> --name \"My VPS\" --protocol-type managed_vps ...", and the polling
block "dhq servers show <server-id> -p <project> --json"; for each, change the
opening triple backticks from ``` to ```bash (or ```shell) so the blocks render
with shell highlighting.
In `@internal/commands/launch_test.go`:
- Line 27: The Logger in the test setup is created as an empty output.Logger{}
which may lack a configured writer and cause panics on write; update the test
initialization that sets Logger to a configured instance by assigning its writer
(e.g., set the Logger.Writer field to io.Discard or a test buffer) so log writes
in the launch_test.go test use a safe destination; locate the logger setup where
Logger: &output.Logger{} is created and replace it with an instance whose writer
is explicitly set.
- Around line 711-738: The test currently only checks flag registration; update
TestServersCreate_ManagedVPS_RequiresAcceptCost_NonInteractive to actually
execute the servers create command in non-interactive mode and assert the
accept-cost guard fires: set cliCtx to a non-interactive context (use the shared
cliCtx variable), build the root command via NewRootCmd("test"), set args to run
"servers create" with --protocol=managed_vps but without --accept-cost, execute
the command (via ExecuteC/Execute) and assert it returns a UserError and that
apiCalled remains false (no API requests were made). Ensure you reset cliCtx
after the test and keep the existing apiCalled/http test server setup and
assertions.
In `@internal/commands/launch.go`:
- Around line 182-194: Add explicit SPA override flags to the command by
declaring new boolean flag variables (e.g., flagSPA and flagNoSPA) via
cmd.Flags().BoolVar calls alongside the existing flags (matching the style used
for flagStatic, flagVPS, flagNonInteract, etc.); then make the SPA
detection/assignment logic (the spaMode decision around the detection at the spa
detection location and the interactive prompt handling at the interactive prompt
location) respect these flags: if flagSPA is true force spaMode=true, if
flagNoSPA is true force spaMode=false, and have these explicit flags take
precedence over automatic detection and interactive prompts (including in
non-interactive/--yes flows). Ensure mutual exclusion/validation so both flags
cannot be set simultaneously and update any help text to document --spa and
--no-spa behavior.
In `@internal/detect/detect.go`:
- Line 155: The current Hugo detection in detect.go uses has(...) or checks
config.toml for the substring "baseURL", which is too broad and causes false
positives; update the conditional in the Hugo detection (the has, readFile, and
fileContains usage) to either only accept explicit Hugo config files (hugo.toml,
hugo.yaml, hugo.json) or, if config.toml must be supported, require additional
Hugo-specific keys (e.g., "theme", "languageCode", or "baseURL =" with TOML
syntax) by checking for multiple key substrings in the fileContents returned by
readFile before returning true; ensure you modify the same conditional that
currently reads has("hugo.toml", "hugo.yaml", "hugo.json") ||
(has("config.toml") && fileContains(readFile("config.toml"), "baseURL")) so
false positives are avoided.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8d315b8c-7917-4638-a5bd-657d4183c889
📒 Files selected for processing (24)
README.mddocs/SKILL.mdinternal/commands/agent_metadata.gointernal/commands/agent_metadata_test.gointernal/commands/deploy.gointernal/commands/init.gointernal/commands/launch.gointernal/commands/launch_test.gointernal/commands/root.gointernal/commands/servers.gointernal/commands/signup.gointernal/config/config.gointernal/detect/detect.gointernal/detect/detect_test.gopkg/sdk/managed_hosting.gopkg/sdk/managed_hosting_test.gopkg/sdk/server_protocols_test.gopkg/sdk/servers.gopkg/sdk/signup.gopkg/sdk/signup_test.gopkg/sdk/types.goskills/deployhq/SKILL.mdskills/deployhq/references/launch.mdskills/deployhq/references/servers.md
CI runs golangci-lint (stricter than go vet). Remove the dead liveURL/urlDisplay in launchDeployFailureCleanup (the failure message uses server.Name/Identifier), simplify the Django detection branch (drop the always-overwritten initial assignment), and convert the dry-run target if/else to a tagged switch (QF1003). No behaviour change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CodeRabbit: TestLaunchErrorAuthRequired_NonInteractive only asserted flag registration (already covered by TestLaunchCommandFlagSet) despite its name. Remove it and add TestLaunchAuthRequired_JSONReason, which verifies the auth_required structured-error contract. The full no-creds flow can't be unit-tested deterministically because Credentials() falls back to the OS keyring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/commands/launch.go (1)
1161-1172:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRegion selection index mismatch causes wrong region to be selected.
regionItemscontains only available regions, sorIdxis an index into that filtered subset. However, the lookup loop comparesrIdxagainsti(the index into the fullregionsslice). When unavailable regions precede available ones, the indices diverge and the wrong region is selected—or no region matches at all.🐛 Proposed fix
if rIdx, _, rErr := regionPrompt.Run(); rErr == nil { - // find slug from items - for i, r := range regions { - if r.Available { - if i == rIdx { - region = r.Slug - selectedRegion = r - break - } - } - } + // Map rIdx (index into filtered available-only list) to the actual region. + availIdx := 0 + for _, r := range regions { + if r.Available { + if availIdx == rIdx { + region = r.Slug + selectedRegion = r + break + } + availIdx++ + } + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/commands/launch.go` around lines 1161 - 1172, The region selection uses rIdx from regionPrompt.Run() which indexes into the filtered list of available regions (regionItems) but the code compares rIdx against i over the full regions slice, causing mismatches; fix by mapping rIdx to the available region directly—either index into the filtered slice (regionItems or a new availableRegions slice) to set region and selectedRegion, or change the loop over regions to maintain a separate available counter that increments only for r.Available and matches rIdx before assigning region/selectedRegion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@internal/commands/launch.go`:
- Around line 1161-1172: The region selection uses rIdx from regionPrompt.Run()
which indexes into the filtered list of available regions (regionItems) but the
code compares rIdx against i over the full regions slice, causing mismatches;
fix by mapping rIdx to the available region directly—either index into the
filtered slice (regionItems or a new availableRegions slice) to set region and
selectedRegion, or change the loop over regions to maintain a separate available
counter that increments only for r.Available and matches rIdx before assigning
region/selectedRegion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f2cb3d0d-8ab8-4299-8a07-6064a17ec2d1
📒 Files selected for processing (3)
internal/commands/launch.gointernal/commands/launch_test.gointernal/detect/detect.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/detect/detect.go
- internal/commands/launch_test.go
Introduce internal/commands/metered.go with meteredResourcesInBeta and a small set of helpers (managedVPSAcknowledgePhrase, managedVPSCostDescription, managedRunningCostTail, betaFreeSuffix). While the metered managed resources (Managed VPS, Static Hosting) are in beta, the CLI presents them as free for early customers and frames the listed monthly rate as the post-beta price. Route every piece of runtime cost/acknowledgement copy in `dhq launch` and `dhq servers create` through these helpers, plus the target-selection menu and the agent metadata comment, so flipping one variable updates all of it. Update the affected tests to assert on the stable `--accept-cost` substring rather than the word "billable". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Update README, the deployhq skill guide, and the launch reference so the Managed VPS / Static Hosting copy says "free for early customers during beta, billed monthly afterwards" instead of "billable". Add a Pricing note in launch.md pointing at the meteredResourcesInBeta switch so the docs are updated in the same change when the resources go GA. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author
|
@claude review |
Author
|
@codex review |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
skills/deployhq/references/launch.md (1)
71-76: ⚡ Quick winConsider making the maintenance instruction more specific.
Line 75 correctly identifies the
meteredResourcesInBetaswitch and notes that "this beta wording" needs updating. However, the instruction doesn't specify exactly which lines contain beta pricing copy (lines 39, 51, and 75 itself). When a maintainer flips the switch, they'll need to search for mentions.Consider adding explicit line references or a more specific instruction, for example: "update the beta wording on lines 39, 51, and this note itself."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/deployhq/references/launch.md` around lines 71 - 76, The note about flipping the meteredResourcesInBeta flag is vague; when you toggle the boolean named meteredResourcesInBeta in internal/commands/metered.go, also update the beta-pricing copy in this document—specifically replace the beta wording at the three occurrences: the two earlier pricing lines (the copies currently at lines referenced in the review: the pricing mentions near the start of the doc and the mid-doc paragraph) and the maintenance note itself—so search the file for the beta pricing sentences and update those three locations together with the switch flip to remove/replace "beta" wording and adjust the stated monthly rate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@skills/deployhq/references/launch.md`:
- Around line 71-76: The note about flipping the meteredResourcesInBeta flag is
vague; when you toggle the boolean named meteredResourcesInBeta in
internal/commands/metered.go, also update the beta-pricing copy in this
document—specifically replace the beta wording at the three occurrences: the two
earlier pricing lines (the copies currently at lines referenced in the review:
the pricing mentions near the start of the doc and the mid-doc paragraph) and
the maintenance note itself—so search the file for the beta pricing sentences
and update those three locations together with the switch flip to remove/replace
"beta" wording and adjust the stated monthly rate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 07f33b7f-b77b-49bd-8bcd-21eb7c9c6a79
📒 Files selected for processing (9)
README.mdinternal/commands/agent_metadata.gointernal/commands/agent_metadata_test.gointernal/commands/launch.gointernal/commands/launch_test.gointernal/commands/metered.gointernal/commands/servers.goskills/deployhq/SKILL.mdskills/deployhq/references/launch.md
✅ Files skipped from review due to trivial changes (3)
- internal/commands/metered.go
- README.md
- skills/deployhq/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (5)
- internal/commands/agent_metadata.go
- internal/commands/agent_metadata_test.go
- internal/commands/servers.go
- internal/commands/launch_test.go
- internal/commands/launch.go
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d08017925
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Add APIError.IsRateLimited() (HTTP 429) and an IsRateLimited(err) helper, plus an APIError.RetryAfter field parsed from the Retry-After response header (integer-seconds form) in parseAPIError. This lets callers distinguish the metered-resource provisioning rate limit (429, retryable) from the cap / kill-switch (422, hard wall) — see deployhq/deployhq#933. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Map a 429 from POST /servers (provisioning rate limit) to a new rate_limited launch reason instead of letting it fall through to provision_failed. The structured error is marked retryable and carries the Retry-After backoff in details.retry_after, so an agent/CI backs off and re-runs rather than failing hard. Also make writeLaunchError treat the wrapped launchError's own Reason as authoritative (a rate_limited / subdomain_taken error surfaced through a generic call site keeps its true reason), and add a retryable boolean to the --json error payload. Document the reason in the launch reference. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Managed VPS size selection now shows readable, tier-named labels instead of the raw droplet slug: a price-ranked tier (Starter/Standard/Plus/Pro) plus specs and price, e.g. "Starter · 1 vCPU · 1 GB RAM · 25 GB SSD · $6.00/mo (s-1vcpu-1gb)" — the slug stays visible so the equivalent --size flag is discoverable. The configuration summary's Size line shows specs + slug too, and falls back to the API Description / bare slug when structured fields are absent. Tiers rank by price (not list position), so ordering is stable. Also close the post-launch rollback guidance gap: a successful launch now prints how to roll back, and the launch reference clarifies that both targets roll back identically via `dhq rollback <deployment>` (redeploys the previous revision) — there's no separate static-vs-VPS command. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dhq launch was POSTing the detected static build command to
/projects/:id/build_configs with a {build_config:{build_commands}} body — a
path/shape that doesn't exist (the SDK's build-command path is
/projects/:id/build_commands with {build_command:{command}}). Because the
error was only warned, the build command silently never got set, so the first
Static Hosting deploy could publish unbuilt sources instead of the generated
dist/public output. (P1 from review.)
Rewrite launchApplyBuildConfig → launchApplyBuildCommand to use the SDK's
CreateBuildCommand, mirroring the web onboarding wizard
(Onboarding::ProjectCreator), which creates project build commands and needs no
separate build environment. Skip when the project already has build commands
(idempotent re-runs / reused --project don't duplicate), keep failure
non-fatal but now surface a discoverable manual-fix hint, and cap the
description at 100 runes like the wizard.
Tests: full branch coverage for launchApplyBuildCommand (correct endpoint+body,
skip-when-exists, empty no-op, list-error-still-creates, create-error-warns,
description truncation) plus a static provision→build-command integration test
proving the command lands on /build_commands during a real static launch.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a spec-validating test client (newSpecValidatingClient) whose transport checks every outgoing request against a committed snapshot of the backend's generated OpenAPI document (/docs.json → internal/commands/testdata/ openapi.json, refreshed via script/update-openapi-fixture.sh). A request to an undocumented path, or with a body violating the documented schema, fails the SDK call with an "openapi spec violation" error — so tests prove the CLI speaks the backend's actual contract, not just what the hand-written fakes accept. Harness self-tests recreate the build-command P1 (POST /build_configs → rejected as undocumented) and a missing required `server` key. Swap the launch build-command and static provision→build-command integration tests onto the validating client, and add real provisioning-poll coverage: provisioning→active terminates (and stops on the first active), and an "error" status stops polling with a structured provision_failed reason. pollProvisioningState's initial backoff becomes a package var as a test seam so the sequence runs without real waits. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The committed OpenAPI fixture pins the contract this CLI was built against (lockfile model) — additive backend changes keep passing, but a breaking backend change would only surface when the fixture is refreshed. This scheduled job closes that gap: it fetches the live spec (repository variable DHQ_SPEC_URL, no-op when unset), substitutes it for the fixture, and re-runs the contract tests. A failure means the backend drifted in a way that breaks the requests this CLI sends — refresh the fixture and adapt the SDK in one PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
dhq launch— take a project folder to a live URL on DeployHQ's own infrastructure (Static Hosting or a Managed VPS) in one command. Plus themanaged_vps/static_hostingprotocols, SDK methods, framework detection, and agent docs.How
dhq launchworksOne command, end to end:
DEPLOYHQ_*env vars, or the keyring; offers signup/login when interactive.package.json,Gemfile,composer.json, …) and suggests a target: static frameworks (Vite, Next, Hugo, …) → Static Hosting; server runtimes (Rails, Django, Express, …) → Managed VPS. Detection also pre-fills the build command and output directory. It runs locally on purpose: the backend's detector (Detection::FromFileList, used by the web onboarding wizard) inspects a repo already connected via a provider, which doesn't exist yet at this point in the flow — the CLI mirrors that wizard's mapping. (Possible follow-up: a public API endpoint accepting a filename list so the CLI reuses backend detection once authed.).deployhq.toml) and connects the git remote.--accept-costnon-interactively), then provisions. Both poll the server until it reportsactive..deployhq.toml, so the next deploy is justdhq deploy. Re-runninglaunchresolves the existing project/server instead of provisioning twice.On failure, the error carries a stable
reason+retryableflag (see below), and--cleanup-on-failuretears down a server whose deploy failed.launchis the funnel for DeployHQ's two managed offerings only. The interactive target menu's third option — "Use my own server (SSH/FTP/…)" — hands off todhq init, and every existing protocol (SSH, FTP/FTPS, rsync, S3, DigitalOcean, Hetzner, Heroku, Netlify, Shopify, …) keeps working throughdhq servers create/dhq initexactly as before.What this adds
dhq launch— the flow above, non-interactive-first: prompts only fill missing values when a TTY is present; agents/CI get--json,--dry-run(no side effects), and fail-fast structured errors.auth_required,beta_enroll_required,accept_cost_required,repo_unreachable,plan_limit_reached,subdomain_taken,rate_limited,provision_failed,deploy_failed— each with aretryableboolean so agents know to back off vs. stop.managed_vps/static_hostingjoin the existing protocol list ondhq servers create(SSH, FTP/FTPS, rsync, S3, and the rest are untouched), plus the SDK methods,internal/detectframework detection, and agent docs (README, SKILL guide,dhq commandsmetadata).Depends on (deploy-order blocker)
Requires the deployhq backend PR deployhq/deployhq#926 (beta-enroll + capability + server-show provisioning endpoints) to be live in production. Gate the public CLI release until that ships to prod.
Pricing copy — free during beta
Managed VPS and Static Hosting are free for early customers while in beta; the listed monthly rate applies once the beta ends. All runtime copy derives from a single switch —
meteredResourcesInBetaininternal/commands/metered.go— so flipping it tofalseat GA updates every cost/acknowledgement string (flag help, cost lines, the non-interactive gate, the failure warning, the target menu). The markdown docs repeat the wording and are flagged to update in the same change.Safety / review
--accept-cost— enforced in bothdhq launchanddhq servers create(--yesalone is not enough).--cleanup-on-failure).rate_limitedreason carrying theRetry-Afterbackoff — distinct from the 422 cap (plan_limit_reached), so CI backs off instead of failing hard.--dry-runhas no side effects, and the detected build command is created through the real/build_commandsendpoint (it previously went to a nonexistent path and silently never landed).Testing
Three layers, all hermetic — no live backend in CI:
httptestfakes: target resolution, cost gate, idempotent re-runs, failure cleanup, build-command creation, and the provisioning poll (provisioning → activeterminates;errorstops withprovision_failedinstead of spinning).newSpecValidatingClientwraps the test client with a transport that validates every outgoing request against a committed snapshot of the backend's generated OpenAPI doc (internal/commands/testdata/openapi.json, refreshed viascript/update-openapi-fixture.sh). A request to an undocumented path or with a schema-violating body fails the test — this is exactly the bug class review caught (the build command was POSTed to a path that doesn't exist), and it can't recur silently now..github/workflows/openapi-drift.yml) — the fixture pins the contract this CLI was built against (lockfile model; additive backend changes stay green). To catch breaking backend drift, a weekday-morning job fetches the live spec and re-runs the contract tests against it. It no-ops until theDHQ_SPEC_URLrepository variable is set — point it at staging's/docs.jsononce #926 deploys there.Existing CI needed no changes — everything runs under the normal
go test ./...+golangci-lintjobs (all green, 0 lint issues).Summary by CodeRabbit
New Features
Documentation