Skip to content

fix(cli): correct ssh-commands timing values and clarify build-configs default vs override#14

Merged
facundofarias merged 1 commit into
mainfrom
fix/build-configs-and-ssh-timing-validation
May 29, 2026
Merged

fix(cli): correct ssh-commands timing values and clarify build-configs default vs override#14
facundofarias merged 1 commit into
mainfrom
fix/build-configs-and-ssh-timing-validation

Conversation

@facundofarias
Copy link
Copy Markdown
Contributor

@facundofarias facundofarias commented May 29, 2026

Summary

Two high-failure-rate bugs surfaced from 30-day Mixpanel telemetry on cli_command events:

  • ssh-commands update — 100% of failures (102/229 calls) were API 422 timing is not included in the list. The --timing flag advertised "before or after", but the API's actual valid values are all, first, after_first (per Command::TIMING in the deployhq Rails model). Fixed the help text on both create and update, changed create's default from "after" (invalid) to "all", added client-side validation before the API call, and added omitempty to Command in the request struct so partial updates don't send an empty string.
  • build-configs update — 100% of failures (165/165 calls) were API 404 record_not_found. The update/delete endpoints scope lookups to non_default_environments, but list returns the default config too — users naturally try to update what they see. Expanded the Long description to explain the default vs override distinction and surfaced (default) / (override) on every row of the list output so the constraint is visible before users hit the wall.

No SDK signature changes. No breaking changes to flag names.

Test plan

  • go build ./...
  • go vet ./...
  • go test ./internal/commands/... ./pkg/sdk/...
  • golangci-lint run ./internal/commands/... ./pkg/sdk/...
  • Smoke against staging: dhq ssh-commands update <id> --timing first succeeds; --timing after now fails client-side with a helpful hint.
  • Smoke against staging: dhq build-configs list shows (default) / (override) column; dhq build-configs --help mentions the override workflow.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes & Improvements
    • Enhanced build-configs list output to display config type as "(default)" or "(override)" labels.
    • Added validation for SSH command --timing option; only accepts: all, first, after_first.
    • Made the Command field optional in SSH command creation requests.

Review Change Stack

Two high-impact bugs surfaced from Mixpanel telemetry (30d window):

- ssh-commands update: 100% of failures were API 422 "timing is not
  included in the list" (102/229 calls). The --timing flag advertised
  "before or after" but the API's valid values are "all", "first", and
  "after_first" (see Command::TIMING in the deployhq Rails model).
  Fix --timing help text on create and update, change create's default
  from "after" (invalid) to "all", validate client-side before the API
  call, and add omitempty to Command so partial updates don't send an
  empty string.

- build-configs update: 100% of failures were API 404 record_not_found
  (165/165 calls). The update/delete endpoints scope lookups to
  non_default_environments, but the list endpoint returns the default
  config too — users naturally try to update what they see. Expand the
  Long description to explain the default/override distinction and
  surface (default)/(override) on every row of the list output so the
  constraint is visible before users try to update.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: afb9868c-56d7-4568-b6cd-3f8e00c18ae7

📥 Commits

Reviewing files that changed from the base of the PR and between 0387838 and e1e2d6a.

📒 Files selected for processing (3)
  • internal/commands/build_configs.go
  • internal/commands/ssh_commands.go
  • pkg/sdk/ssh_commands.go

Walkthrough

This pull request improves CLI command handling across two areas: it enhances the build configs list output to clearly indicate configuration type as default or override, and it adds server-side validation for SSH command timing values while updating the SDK request schema to allow optional command fields during creation.

Changes

Build Configs Display Enhancement

Layer / File(s) Summary
Help text and Type field display
internal/commands/build_configs.go
Command help text is extended with guidance on default vs override configs. The list table output replaces default status formatting with a computed Type field that labels each row as (default) or (override).

SSH Timing Validation and Request Schema

Layer / File(s) Summary
Request schema optionality
pkg/sdk/ssh_commands.go
SSHCommandCreateRequest.Command is marked optional in JSON serialization by adding omitempty to its struct tag.
Timing validation logic and integration
internal/commands/ssh_commands.go
Introduces validSSHTimings list and validateSSHTiming function enforcing membership in all, first, and after_first. Create and update command paths call the validator before executing. Update flag description and create flag default value are changed to reflect accepted timing values as all.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and concisely summarizes the main changes: fixing SSH command timing validation and clarifying build-config default vs override behavior in the CLI.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/build-configs-and-ssh-timing-validation

Comment @coderabbitai help to get the list of available commands and usage tips.

@facundofarias facundofarias merged commit 65ed510 into main May 29, 2026
3 checks passed
@facundofarias facundofarias deleted the fix/build-configs-and-ssh-timing-validation branch May 29, 2026 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant