feat: add folders, global SSH key download, and account resources#31
Open
MartaKar wants to merge 2 commits into
Open
feat: add folders, global SSH key download, and account resources#31MartaKar wants to merge 2 commits into
MartaKar wants to merge 2 commits into
Conversation
Extend CLI coverage of the DeployHQ API (DHQ-639): - folders (project categories): dhq folders list|create|update|delete - global SSH key download: dhq ssh-keys download <id> [--output] - users: dhq users list|show|create|update|delete|resend-invitation - account: dhq account get|update|billing - profile: dhq profile get|update - api-keys: dhq api-keys create|delete Each resource ships an SDK client, cobra command, agent metadata, assist allowlist entry, and unit tests. Verified end-to-end against a live backend.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughAdds SDK and CLI support for account, profile, users, folders, API keys, and SSH private-key downloads. Registers the commands, adds JSON and human-readable output paths, expands agent metadata and prompt allowlists, and adds endpoint and CLI tests. ChangesResource command expansion
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ 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: 5
🧹 Nitpick comments (6)
pkg/sdk/profile_test.go (1)
18-25: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse a literal recorded JSON fixture for the profile response.
Encoding
Profilehere reuses the SDK tags being tested and can hide wire-format regressions. Return the actual response JSON shape instead.As per coding guidelines,
**/*_test.goshould usehttptest.NewServerfor recorded response shapes andintegration_test.gofor real API JSON golden validation.🤖 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 `@pkg/sdk/profile_test.go` around lines 18 - 25, Replace the json.Encoder-based Profile serialization in the test handler with a literal JSON fixture matching the recorded API response shape, so the test does not reuse Profile’s JSON tags. Keep the handler served through httptest.NewServer, and reserve real API JSON golden validation for integration_test.go.Source: Coding guidelines
pkg/sdk/account_test.go (1)
19-23: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse literal recorded JSON fixtures for response-shape tests.
These handlers serialize
AccountandBillingStatusfrom the same SDK types being tested, so incorrect JSON tags or wire shapes could still round-trip successfully. Return captured JSON payloads instead.As per coding guidelines,
**/*_test.goshould usehttptest.NewServerfor recorded response shapes andintegration_test.gofor real API JSON golden validation.Also applies to: 68-68, 99-102
🤖 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 `@pkg/sdk/account_test.go` around lines 19 - 23, Replace the test server responses in the account response-shape tests with literal recorded JSON fixtures instead of encoding `Account` or `BillingStatus` values, so serialization mistakes cannot round-trip undetected. Update the handlers in the affected tests, including the locations around the account and billing cases, while continuing to use `httptest.NewServer`; reserve real API JSON golden validation for `integration_test.go`.Source: Coding guidelines
pkg/sdk/folders.go (1)
17-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNaming:
FolderCreateRequestis reused for update, not just create.Minor naming clarity nit; consider
FolderRequestsince the same type backs bothCreateFolderandUpdateFolder.🤖 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 `@pkg/sdk/folders.go` around lines 17 - 21, Rename FolderCreateRequest to FolderRequest because it is used by both CreateFolder and UpdateFolder, and update all references consistently, including method signatures, comments, and any related documentation.internal/commands/folders_test.go (1)
13-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd execution coverage for folder handlers.
The tests do not invoke list/create/update/delete, so output precedence, API payloads, JSON responses, and
--namevalidation are untested. Add command execution tests for these paths.🤖 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/folders_test.go` around lines 13 - 62, Expand the folder command tests beyond registration and help checks by executing the list, create, update, and delete handlers. Reuse the existing command/test setup and mock API dependencies to assert output precedence, request payloads, JSON responses, and required --name validation, covering each command’s success and relevant validation paths.internal/commands/ssh_keys_test.go (1)
11-34: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winExercise the download command, not only its registration.
These tests never execute the command, so regressions in
--output,0600permissions, empty-key rejection, JSON output, raw stdout, and write-error handling can pass unnoticed. Add command-level execution tests using the existing test client/server setup.🤖 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/ssh_keys_test.go` around lines 11 - 34, Expand TestSSHKeysDownloadCommand_Help and related SSH key tests to execute ssh-keys download through the existing test client/server setup, not just inspect help or metadata. Cover --output file creation with 0600 permissions, empty-key rejection, JSON and raw stdout modes, and write-error handling, using the command’s actual execution path and asserting results and errors.internal/commands/users.go (1)
112-158: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce positional-parameter risk in
userRequestFromFlags/addUserFlags.Both helpers take 10-11 positional pointer params shared across two call sites each (create/update). A future flag insertion out of order (e.g. swapping two
*boolparams) would silently misassign fields since the compiler can't catch same-type-category swaps.♻️ Suggested refactor: bundle flags into a struct
type userFlagVars struct { firstName, lastName, email, timeZone string allProjects, admin, manageUsers, manageBilling, manageAgents, createProjects bool } func addUserFlags(cmd *cobra.Command, v *userFlagVars) { cmd.Flags().StringVar(&v.firstName, "first-name", "", "First name") // ... etc } func userRequestFromFlags(cmd *cobra.Command, v *userFlagVars) sdk.UserRequest { req := sdk.UserRequest{} if cmd.Flags().Changed("first-name") { req.FirstName = &v.firstName } // ... etc return req }🤖 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/users.go` around lines 112 - 158, Reduce positional-parameter risk in userRequestFromFlags and addUserFlags by introducing a userFlagVars struct containing all string and boolean flag values. Change both helpers and their create/update call sites to accept the struct pointer, bind flags through its fields, and populate sdk.UserRequest pointers from those fields only when the corresponding flags changed.
🤖 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/agent_metadata.go`:
- Around line 279-283: Update the `dhq ssh-keys download` metadata entry to set
`RequiresConfirmation: true` and `SafeForAutomation: false`; remove
`SupportsJSON` unless its JSON output excludes the `private_key` material.
In `@internal/commands/api_keys.go`:
- Around line 71-93: The newAPIKeysDeleteCmd deletion flow lacks the required
confirmation gate. Before calling client.DeleteAPIKey, invoke the shared
cliCtx.RequiresConfirmation path with the API key identifier and return any
resulting error, ensuring non-interactive or declined confirmations prevent the
API call.
In `@internal/commands/ssh_keys.go`:
- Around line 162-176: Update writeSecureFile to avoid truncating the
destination directly: create a uniquely named temporary file in the destination
directory with 0600 permissions, write and close the complete contents, then
atomically rename it over the target. Ensure all failure paths close the file
and remove the temporary file, while preserving the existing permission
enforcement.
- Line 150: Handle the error returned by fmt.Fprintln when writing privateKey to
env.Stdout in the relevant command handler. If the write fails, propagate the
error through the handler’s existing error-return path instead of reporting
success.
In `@pkg/sdk/users.go`:
- Around line 54-93: Escape user identifiers with url.PathEscape before
interpolating them into endpoint paths in GetUser, UpdateUser, DeleteUser, and
ResendUserInvitation; add the required net/url import and use the escaped value
consistently when constructing each /users/... URL.
---
Nitpick comments:
In `@internal/commands/folders_test.go`:
- Around line 13-62: Expand the folder command tests beyond registration and
help checks by executing the list, create, update, and delete handlers. Reuse
the existing command/test setup and mock API dependencies to assert output
precedence, request payloads, JSON responses, and required --name validation,
covering each command’s success and relevant validation paths.
In `@internal/commands/ssh_keys_test.go`:
- Around line 11-34: Expand TestSSHKeysDownloadCommand_Help and related SSH key
tests to execute ssh-keys download through the existing test client/server
setup, not just inspect help or metadata. Cover --output file creation with 0600
permissions, empty-key rejection, JSON and raw stdout modes, and write-error
handling, using the command’s actual execution path and asserting results and
errors.
In `@internal/commands/users.go`:
- Around line 112-158: Reduce positional-parameter risk in userRequestFromFlags
and addUserFlags by introducing a userFlagVars struct containing all string and
boolean flag values. Change both helpers and their create/update call sites to
accept the struct pointer, bind flags through its fields, and populate
sdk.UserRequest pointers from those fields only when the corresponding flags
changed.
In `@pkg/sdk/account_test.go`:
- Around line 19-23: Replace the test server responses in the account
response-shape tests with literal recorded JSON fixtures instead of encoding
`Account` or `BillingStatus` values, so serialization mistakes cannot round-trip
undetected. Update the handlers in the affected tests, including the locations
around the account and billing cases, while continuing to use
`httptest.NewServer`; reserve real API JSON golden validation for
`integration_test.go`.
In `@pkg/sdk/folders.go`:
- Around line 17-21: Rename FolderCreateRequest to FolderRequest because it is
used by both CreateFolder and UpdateFolder, and update all references
consistently, including method signatures, comments, and any related
documentation.
In `@pkg/sdk/profile_test.go`:
- Around line 18-25: Replace the json.Encoder-based Profile serialization in the
test handler with a literal JSON fixture matching the recorded API response
shape, so the test does not reuse Profile’s JSON tags. Keep the handler served
through httptest.NewServer, and reserve real API JSON golden validation for
integration_test.go.
🪄 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: 3d7af336-04dd-45aa-ada9-f9aa86d2a8b1
📒 Files selected for processing (27)
internal/assist/prompt.gointernal/commands/account.gointernal/commands/account_test.gointernal/commands/agent_metadata.gointernal/commands/api_keys.gointernal/commands/api_keys_test.gointernal/commands/folders.gointernal/commands/folders_test.gointernal/commands/profile.gointernal/commands/profile_test.gointernal/commands/root.gointernal/commands/ssh_keys.gointernal/commands/ssh_keys_test.gointernal/commands/users.gointernal/commands/users_test.gopkg/sdk/account.gopkg/sdk/account_test.gopkg/sdk/api_keys.gopkg/sdk/api_keys_test.gopkg/sdk/folders.gopkg/sdk/folders_test.gopkg/sdk/profile.gopkg/sdk/profile_test.gopkg/sdk/ssh_keys.gopkg/sdk/ssh_keys_test.gopkg/sdk/users.gopkg/sdk/users_test.go
- check errcheck on best-effort stdout write (fixes failing golangci-lint) - write downloaded private keys atomically (temp file + rename) so an interrupted write can't destroy an existing key - mark 'ssh-keys download' as requires-confirmation / not safe-for-automation since it emits raw private key material
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.
Summary
Extends the CLI's coverage of the DeployHQ API (DHQ-639). Adds six previously-missing resources, each with an SDK client, cobra command, agent metadata, assist allowlist entry, and unit tests.
dhq folders list|create|update|delete/folders(project categories)dhq ssh-keys download <id> [--output]GET /ssh_keys/:id/download_private_keydhq users list|show|create|update|delete|resend-invitation/usersdhq account get|update|billing/account,/account/billing_statusdhq profile get|update/profiledhq api-keys create|delete/security/api_keysNotes
/folders), so request/response shapes,identifier-vs-idkeying, and PATCH-vs-PUT usage match the real API. Users/account/profile updates usePATCH; folders usesPUTper its endpoint.--output, which is written with secure0600permissions (tightened even on a pre-existing file).account.project_limit, billingscheduled_change, folderposition) are modeled as pointers/omitempty with explicit null round-trip tests.Testing
go test ./...),go vetclean.--json/--quiet/--tableoutput, and 422/404/403 error paths.Summary by CodeRabbit
accountcommands for viewing/updating account details and checking billing status.profilecommands to get and update your authenticated profile.api-keyscommands to create and delete API keys, with plaintext key shown once.folderscommands to list, create, update/rename, and delete account folders.usersteam member commands (list/show/create/update/delete and resend invitations).ssh-keys downloadto retrieve private keys, supporting JSON output and optional secure file writing.