Skip to content
This repository was archived by the owner on Jan 23, 2026. It is now read-only.

[AGENT-130] Delete and Roll Back deployments#313

Merged
jhaynie merged 8 commits intomainfrom
AGENT-130
May 22, 2025
Merged

[AGENT-130] Delete and Roll Back deployments#313
jhaynie merged 8 commits intomainfrom
AGENT-130

Conversation

@pec1985
Copy link
Copy Markdown
Contributor

@pec1985 pec1985 commented May 19, 2025

Summary by CodeRabbit

  • New Features

    • Added a CLI command to enable rollback or deletion of cloud deployments, with options for project and deployment selection.
    • Introduced interactive prompts for selecting projects and deployments when not specified.
    • Added support for force actions and tag-based deployment selection.
    • Enabled listing, deletion, and rollback of deployments within projects through new deployment management features.
  • Bug Fixes

    • Improved error handling and user feedback during deployment management actions.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 2025

"""

Walkthrough

New functionality for managing cloud project deployments has been introduced. This includes listing, deleting, and rolling back deployments via new API calls and corresponding CLI commands. The CLI provides interactive and flag-driven options for selecting projects and deployments, supporting both rollback and deletion actions with optional confirmation prompts.

Changes

File(s) Change Summary
internal/project/project.go Added DeploymentListData struct and functions: ListDeployments, DeleteDeployment, and RollbackDeployment for deployment management via API.
cmd/cloud.go Introduced cloudRollbackCmd CLI command with helpers for selecting projects and deployments, supporting rollback and deletion actions.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CLI
    participant API

    User->>CLI: Run cloudRollbackCmd with flags
    CLI->>API: List Projects (if needed)
    API-->>CLI: Return Projects
    CLI->>User: Prompt for Project (if needed)
    User-->>CLI: Select Project
    CLI->>API: List Deployments (if needed)
    API-->>CLI: Return Deployments
    CLI->>User: Prompt for Deployment (if needed)
    User-->>CLI: Select Deployment
    alt --delete flag
        CLI->>API: Delete Deployment
        API-->>CLI: Return Success/Failure
    else
        CLI->>API: Rollback Deployment
        API-->>CLI: Return Success/Failure
    end
    CLI->>User: Show Success/Error Message
Loading

Assessment against linked issues

Objective Addressed Explanation
CLI: add undeploy and rollback (AGENT-130)

Poem

A rabbit with code in its fluffy white paws,
Now rolls back deployments, with barely a pause!
Delete or undo, just a flag or a click,
Choose your project and tag—make your pick.
With spinners and prompts, the cloud’s now in check,
🐇 Hopping through rollbacks—what’s next on the tech deck?
"""

✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 62e8358 and 5d06122.

📒 Files selected for processing (2)
  • cmd/project.go (3 hunks)
  • internal/project/project.go (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
internal/project/project.go (2)
internal/util/api.go (1)
  • NewAPIClient (70-78)
internal/agent/agent.go (1)
  • Response (19-23)
🪛 golangci-lint (1.64.8)
cmd/project.go

836-836: printf: non-constant format string in call to github.com/agentuity/go-common/tui.ShowError

(govet)


876-876: printf: non-constant format string in call to github.com/agentuity/go-common/tui.ShowError

(govet)


920-920: ineffectual assignment to desc

(ineffassign)


960-960: ineffectual assignment to tags

(ineffassign)

🪛 GitHub Check: Build and Test (blacksmith-4vcpu-ubuntu-2204-arm)
cmd/project.go

[failure] 836-836:
non-constant format string in call to github.com/agentuity/go-common/tui.ShowError


[failure] 876-876:
non-constant format string in call to github.com/agentuity/go-common/tui.ShowError

🪛 GitHub Check: Build and Test (blacksmith-4vcpu-ubuntu-2204)
cmd/project.go

[failure] 836-836:
non-constant format string in call to github.com/agentuity/go-common/tui.ShowError


[failure] 876-876:
non-constant format string in call to github.com/agentuity/go-common/tui.ShowError

🪛 GitHub Actions: Go Build and Test
cmd/project.go

[error] 836-836: non-constant format string in call to github.com/agentuity/go-common/tui.ShowError

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Test CLI Upgrade Path (windows-latest)
🔇 Additional comments (6)
internal/project/project.go (4)

302-308: LGTM: Well-structured deployment data model

The DeploymentListData struct is well-designed with appropriate fields for tracking deployment metadata including ID, message, tags, active status, and creation time.


310-318: LGTM: Clean implementation of deployment listing

This function follows the established pattern in the codebase for API interactions. It properly initializes an API client, makes the request, and handles error cases appropriately with descriptive error messages.


320-331: LGTM: Good error handling for deletion workflow

The implementation properly handles both API errors and unsuccessful responses (where resp.Success is false) by returning appropriate error messages to the caller.


333-344: LGTM: Clean implementation of rollback functionality

This function follows the same error handling pattern as other API interactions in the file, ensuring consistent behavior throughout the codebase.

cmd/project.go (2)

901-933: LGTM: Well-implemented project selection helper

This helper function cleanly abstracts the project selection workflow that would otherwise be duplicated across the rollback and delete commands. It handles fetching projects, displaying them with appropriate formatting, and returning the selected project ID.

🧰 Tools
🪛 golangci-lint (1.64.8)

920-920: ineffectual assignment to desc

(ineffassign)


990-991: LGTM: Proper command registration

The new commands are correctly added as subcommands to the existing project command.

Comment thread cmd/project.go Outdated
Comment thread cmd/project.go Outdated
Comment thread cmd/project.go Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
cmd/project.go (1)

934-981: Fix ineffectual assignments and use provided prompt parameter

There are a couple of issues in this function:

  1. The first assignment to tags is ineffectual as it's immediately overwritten
  2. The prompt in line 979 is hardcoded to "Select a deployment to rollback" but should use the provided prompt parameter

Apply these changes:

-		tags := strings.Join(d.Tags, ", ")
-		if len(d.Tags) > 50 {
-			tags = strings.Join(d.Tags[:50], ", ") + "..."
-		} else {
-			tags = strings.Join(d.Tags, ", ")
-		}
+		// Format tags with truncation if needed
+		var tags string
+		if len(d.Tags) > 3 {
+			tags = strings.Join(d.Tags[:3], ", ") + "..."
+		} else {
+			tags = strings.Join(d.Tags, ", ")
+		}

And fix the prompt:

-	selectedDeployment := tui.Select(logger, "Select a deployment to rollback", "", deploymentOptions)
+	selectedDeployment := tui.Select(logger, prompt, "", deploymentOptions)
🧰 Tools
🪛 golangci-lint (1.64.8)

960-960: ineffectual assignment to tags

(ineffassign)

🧹 Nitpick comments (1)
cmd/project.go (1)

902-932: Remove unused variable assignment

The desc variable on line 920 is assigned but never used.

-		desc := p.Description
-		if desc == "" {
-			desc = emptyProjectDescription
-		}
🧰 Tools
🪛 golangci-lint (1.64.8)

920-920: ineffectual assignment to desc

(ineffassign)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 5d06122 and 936b3ba.

📒 Files selected for processing (1)
  • cmd/project.go (3 hunks)
🧰 Additional context used
🪛 golangci-lint (1.64.8)
cmd/project.go

920-920: ineffectual assignment to desc

(ineffassign)


960-960: ineffectual assignment to tags

(ineffassign)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Build and Test (windows-latest)
🔇 Additional comments (3)
cmd/project.go (3)

809-841: Looks good: Rollback deployment command implementation

The implementation of the rollback-deployment command is well-structured with appropriate error handling and user feedback.


843-882: Looks good: Delete deployment command implementation

The implementation of the delete-deployment command includes proper user confirmation flow and error handling.


990-991: Looks good: Command registration

The new commands are properly registered with the root command.

Comment thread cmd/project.go Outdated
Comment thread cmd/project.go Outdated
Comment thread cmd/project.go Outdated
Comment thread cmd/project.go Outdated
Comment thread cmd/project.go Outdated
Comment thread cmd/project.go Outdated
Comment thread cmd/project.go Outdated
Comment thread cmd/project.go Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
cmd/project.go (2)

859-906: 🛠️ Refactor suggestion

Fix ineffectual assignments and use the provided prompt parameter

There are a few issues in this function:

  1. The first assignment to tags is overwritten immediately
  2. The prompt parameter is ignored, using a hardcoded string instead
  3. The tag handling logic could be improved for readability

Apply these changes:

-		msg := strings.Join(d.Tags, ", ")
+		// Truncate message if too long
+		msg := d.Message
 		if len(d.Message) > 60 {
 			msg = d.Message[:57] + "..."
-		} else {
-			msg = d.Message
 		}
-		tags := strings.Join(d.Tags, ", ")
-		if len(d.Tags) > 50 {
-			tags = strings.Join(d.Tags[:50], ", ") + "..."
-		} else {
-			tags = strings.Join(d.Tags, ", ")
-		}
+		// Format tags with truncation if needed
+		var tags string
+		if len(d.Tags) > 3 {
+			tags = strings.Join(d.Tags[:3], ", ") + "..."
+		} else {
+			tags = strings.Join(d.Tags, ", ")
+		}

Also, use the provided prompt parameter:

-	selectedDeployment := tui.Select(logger, "Select a deployment to rollback", "", deploymentOptions)
+	selectedDeployment := tui.Select(logger, prompt, "", deploymentOptions)
🧰 Tools
🪛 golangci-lint (1.64.8)

859-859: func selectDeployment is unused

(unused)


904-904: ⚠️ Potential issue

Use the prompt parameter provided to the function

The function takes a prompt parameter but ignores it in favor of a hardcoded string.

-	selectedDeployment := tui.Select(logger, "Select a deployment to rollback", "", deploymentOptions)
+	selectedDeployment := tui.Select(logger, prompt, "", deploymentOptions)
🧹 Nitpick comments (5)
cmd/project.go (2)

826-857: Good helper function for project selection

This function nicely encapsulates the project selection workflow with clear user feedback and error handling.

However, static analysis shows that this function is currently unused within this file. If this is intended to be used elsewhere (e.g., from cloud.go), consider moving shared functionality to a common package.

🧰 Tools
🪛 golangci-lint (1.64.8)

827-827: func selectProject is unused

(unused)


845-845: ineffectual assignment to desc

(ineffassign)


842-850: Remove ineffectual assignment to 'desc'

The desc variable is assigned but never used in the option text, making it ineffectual.

-		desc := p.Description
-		if desc == "" {
-			desc = emptyProjectDescription
-		}
🧰 Tools
🪛 golangci-lint (1.64.8)

845-845: ineffectual assignment to desc

(ineffassign)

cmd/cloud.go (3)

792-796: Remove redundant return after os.Exit(1)

The return statement after os.Exit(1) is redundant since os.Exit(1) will terminate program execution.

 			if err != nil {
 				tui.ShowError("%s", err.Error())
 				os.Exit(1)
-				return
 			}

803-835: Fix ineffectual assignment to 'desc'

The desc variable is assigned but never used in the option text.

-		desc := p.Description
-		if desc == "" {
-			desc = "No description provided."
-		}
🧰 Tools
🪛 golangci-lint (1.64.8)

823-823: ineffectual assignment to desc

(ineffassign)


916-921: Add a force flag for non-TTY environments

Based on a previous comment, consider adding a --force flag to allow automation in non-TTY environments.

 	cloudRollbackCmd.Flags().String("project", "", "Project to rollback a deployment")
 	cloudRollbackCmd.Flags().String("dir", "", "The directory to the project to rollback if project is not specified")
 	cloudRollbackCmd.Flags().Bool("delete", false, "Delete the deployment instead of rolling back")
+	cloudRollbackCmd.Flags().Bool("force", false, "Skip confirmation prompts (useful for scripts and CI/CD pipelines)")
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 936b3ba and 397820f.

📒 Files selected for processing (2)
  • cmd/cloud.go (2 hunks)
  • cmd/project.go (2 hunks)
🧰 Additional context used
🪛 golangci-lint (1.64.8)
cmd/cloud.go

823-823: ineffectual assignment to desc

(ineffassign)


863-863: ineffectual assignment to tags

(ineffassign)

cmd/project.go

827-827: func selectProject is unused

(unused)


859-859: func selectDeployment is unused

(unused)


845-845: ineffectual assignment to desc

(ineffassign)

⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Build and Test (windows-latest)
  • GitHub Check: Test CLI Upgrade Path (windows-latest)
  • GitHub Check: Analyze (go)
🔇 Additional comments (2)
cmd/cloud.go (2)

675-686: Well-structured command with clear description

The command has a clear description and examples, which is excellent for users.


779-784: Good confirmation dialog for destructive operation

Asking for confirmation before deleting is a great practice for destructive operations.

Comment thread cmd/cloud.go
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

♻️ Duplicate comments (2)
cmd/cloud.go (2)

786-786: ⚠️ Potential issue

Fix non-constant format string in error handling

The call to tui.ShowError() is using a non-constant format string which may cause build failures. This function expects a format string as the first argument, followed by any variables to include.

-				tui.ShowError(err.Error())
+				tui.ShowError("Failed to delete deployment: %s", err.Error())

793-793: ⚠️ Potential issue

Fix non-constant format string in error handling

The call to tui.ShowError() is using a non-constant format string which may cause build failures. This function expects a format string as the first argument, followed by any variables to include.

-				tui.ShowError(err.Error())
+				tui.ShowError("Failed to rollback deployment: %s", err.Error())
🧹 Nitpick comments (3)
cmd/cloud.go (3)

857-867: Improve tag truncation logic

The current implementation truncates tags at a fixed character position, which could break in the middle of multi-byte characters. Consider using a safer truncation method or truncating after a certain number of tags rather than characters.

-		tags := strings.Join(d.Tags, ", ")
-		if len(tags) > 50 {
-			tags = tags[:50] + "..."
-		}
+		// Format tags with truncation if needed
+		var tags string
+		if len(d.Tags) > 3 {
+			tags = strings.Join(d.Tags[:3], ", ") + "..."
+		} else {
+			tags = strings.Join(d.Tags, ", ")
+		}

869-879: Enhance deployment selection display

The current format for displaying deployments could be improved for readability and alignment.

Consider a more structured format for the deployment display:

-				Text: fmt.Sprintf("%s  %s, tags: [%-50s], msg: [%s]", "✅", tui.Title(date.Format("2006-01-02 15:04:05")), tui.Bold(tags), tui.Bold(msg)),
+				Text: fmt.Sprintf("%s  %s | Tags: %-20s | %s", 
+                                 "✅", 
+                                 tui.Title(date.Format("2006-01-02 15:04:05")), 
+                                 tui.Bold(tags), 
+                                 tui.Bold(msg)),

Make a similar change for the inactive deployments.


914-919: Add alias for better usability

Adding an alias would make the command more intuitive to use, consistent with other commands in the CLI.

-	cloudRollbackCmd.Flags().String("tag", "", "Tag of the deployment to rollback")
+	cloudRollbackCmd.Flags().StringP("tag", "t", "", "Tag of the deployment to rollback")

Also consider adding an alias for the command itself:

-	rootCmd.AddCommand(cloudRollbackCmd)
+	rootCmd.AddCommand(cloudRollbackCmd)
+	cloudRollbackCmd.Aliases = []string{"undeploy"}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 397820f and d1c0725.

📒 Files selected for processing (2)
  • cmd/cloud.go (2 hunks)
  • cmd/project.go (1 hunks)
🧰 Additional context used
🪛 golangci-lint (1.64.8)
cmd/cloud.go

823-823: ineffectual assignment to desc

(ineffassign)

cmd/project.go

825-825: func selectProject is unused

(unused)


843-843: ineffectual assignment to desc

(ineffassign)

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Build and Test (windows-latest)
  • GitHub Check: Analyze (go)
🔇 Additional comments (1)
cmd/cloud.go (1)

675-799: Good implementation of rollback/delete command

The command structure is well organized with clear help text, examples, and handling of both interactive and flag-based options.

Comment thread cmd/project.go Outdated
Comment thread cmd/project.go Outdated
Comment thread cmd/cloud.go Outdated
Comment thread cmd/cloud.go Outdated
Comment thread cmd/cloud.go Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (2)
cmd/cloud.go (2)

813-845: Fix ineffectual assignment to desc

The desc variable is assigned but never used in creating the project option text.

-		desc := p.Description
-		if desc == "" {
-			desc = "No description provided."
-		}

The variable can be safely removed as it's not being used in the construction of the options.

🧰 Tools
🪛 golangci-lint (1.64.8)

833-833: ineffectual assignment to desc

(ineffassign)


675-810: 🛠️ Refactor suggestion

The rollback/delete command needs consistent error handling

The command looks good overall, but there's an inconsistency in error handling. At lines 804-806, it's using direct os.Exit(1) while other parts of the codebase use errsystem.New(...).ShowErrorAndExit().

-				tui.ShowError("%s", err.Error())
-				os.Exit(1)
-				return
+				errsystem.New(errsystem.ErrApiRequest, err, errsystem.WithContextMessage("Failed to rollback deployment")).ShowErrorAndExit()

Also, the return statement after os.Exit(1) is redundant as the program will terminate before reaching it.

🧹 Nitpick comments (1)
cmd/cloud.go (1)

847-892: Optimize tag string truncation logic

The truncation logic for tags could be improved by checking the length of the array before joining it. This would avoid potentially expensive string operations for long tag arrays.

-		tags := strings.Join(d.Tags, ", ")
-		if len(tags) > 50 {
-			tags = tags[:50] + "..."
-		}
+		var tags string
+		if len(d.Tags) > 3 {
+			tags = strings.Join(d.Tags[:3], ", ") + "..."
+		} else {
+			tags = strings.Join(d.Tags, ", ")
+		}

This change improves efficiency by only joining the tags once and limiting the number of tags shown rather than truncating the joined string.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between d1c0725 and 6ddcf9f.

📒 Files selected for processing (1)
  • cmd/cloud.go (2 hunks)
🧰 Additional context used
🪛 golangci-lint (1.64.8)
cmd/cloud.go

833-833: ineffectual assignment to desc

(ineffassign)

⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Build and Test (windows-latest)
  • GitHub Check: Test CLI Upgrade Path (windows-latest)
  • GitHub Check: Analyze (go)
🔇 Additional comments (2)
cmd/cloud.go (2)

779-791: Good implementation of the force flag

The implementation of the --force flag is done well. It allows skipping the confirmation prompts which is essential for CI/CD pipelines and scripting.


924-931: Comprehensive flag set for the rollback command

The command registration and flag setup is complete and well-structured, providing a good user experience with multiple ways to identify projects and deployments.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
cmd/cloud.go (1)

801-808: Use consistent error handling approach

The error handling pattern here differs from the rest of the codebase and includes an unnecessary os.Exit(1) call that will never be reached due to the return statement.

-				tui.ShowError("%s", err.Error())
-				os.Exit(1)
-				return
+				errsystem.New(errsystem.ErrApiRequest, err, errsystem.WithContextMessage("Failed to rollback deployment")).ShowErrorAndExit()
🧹 Nitpick comments (1)
cmd/cloud.go (1)

869-872: Consider a more flexible tag truncation approach

The current implementation may truncate tags at 50 characters, which could cut off important information. Consider using the number of tags instead of character length for truncation.

-		tags := strings.Join(d.Tags, ", ")
-		if len(tags) > 50 {
-			tags = tags[:50] + "..."
-		}
+		// Format tags with truncation if needed
+		var tags string
+		if len(d.Tags) > 3 {
+			tags = strings.Join(d.Tags[:3], ", ") + "..."
+		} else {
+			tags = strings.Join(d.Tags, ", ")
+		}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 6ddcf9f and 02db0d5.

📒 Files selected for processing (1)
  • cmd/cloud.go (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: Build and Test (macos-latest)
  • GitHub Check: Build and Test (windows-latest)
  • GitHub Check: Test CLI Upgrade Path (windows-latest)
  • GitHub Check: Analyze (go)
🔇 Additional comments (10)
cmd/cloud.go (10)

675-687: Implementation looks good

The command definition is well structured with clear documentation that explains both rollback and delete functionality. The examples provided are helpful for users to understand how to use the command.


688-693: LGTM - Standard initialization pattern

Good initialization pattern, setting up the logger, context with signal handling, and ensuring the user is logged in before proceeding.


694-724: Project selection logic handles multiple sources correctly

The implementation properly handles project selection from multiple sources in the correct priority order: directory path, project ID flag, or interactive selection. This provides good flexibility for users.


725-737: Clear user messaging based on action type

Appropriate question text customization based on whether the user is performing a rollback or delete operation.


738-767: Good support for tag-based deployment selection

The tag-based deployment selection is well implemented, allowing users to directly specify a deployment by tag rather than interactive selection.


779-791: Appropriate user confirmation

Good implementation of the force flag to skip confirmation in non-interactive environments, while ensuring users confirm potentially destructive operations in interactive mode.


813-841: Well-implemented project selection helper

The cloudSelectProject function provides a clean abstraction for project selection with good error handling and user feedback.


843-868: Well-implemented deployment selection helper

The initial part of the deployment selection function is well structured with appropriate error handling and formatting of dates.


873-888: Clear visual distinction for active deployments

Good use of a checkmark emoji to visually distinguish active deployments in the selection list, making it easier for users to identify the current deployment.


920-927: Command registration and flag configuration

The command registration and flag configuration is complete and follows the pattern established by other commands. The inclusion of a force flag is particularly useful for automation scenarios.

Comment thread cmd/cloud.go
@pec1985 pec1985 requested a review from jhaynie May 20, 2025 14:55
Comment thread cmd/cloud.go Outdated
Comment thread cmd/cloud.go Outdated
Comment thread cmd/cloud.go
Comment thread cmd/cloud.go
ID: d.ID,
Text: fmt.Sprintf(" %s, tags: [%-50s], msg: [%s]", tui.Title(date.Format("2006-01-02 15:04:05")), tui.Bold(tags), tui.Bold(msg)),
})
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't really love the way this looks... i might make some suggestions /changes here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, not sure what the best way is or what info to show of each deployment

Comment thread cmd/project.go Outdated
Comment thread cmd/project.go Outdated
@jhaynie
Copy link
Copy Markdown
Member

jhaynie commented May 21, 2025

tried to test this but i'm seeing this when i select a project (agentuity rollback)

image

@pec1985
Copy link
Copy Markdown
Contributor Author

pec1985 commented May 22, 2025

tried to test this but i'm seeing this when i select a project (agentuity rollback)

image

rollback.mov

Unsure why its not working for you, but I'll investigate right now. Could be an API issue

@pec1985
Copy link
Copy Markdown
Contributor Author

pec1985 commented May 22, 2025

Tested it with the production config as well
https://github.com/user-attachments/assets/84d09c71-1de4-4c35-911b-f10516c82200

Copy link
Copy Markdown
Member

@jhaynie jhaynie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works great. small UI adjustments. 🚢 it

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
cmd/cloud.go (1)

866-869: 🛠️ Refactor suggestion

Ineffectual assignment to tags variable

The initial assignment to tags is overwritten immediately in some cases, making the first assignment potentially redundant.

-		tags := strings.Join(d.Tags, ", ")
-		if len(tags) > 50 {
-			tags = tags[:50] + "..."
-		}
+		var tags string
+		if len(d.Tags) > 0 {
+			tags = strings.Join(d.Tags, ", ")
+			if len(tags) > 50 {
+				tags = tags[:50] + "..."
+			}
+		}
🧹 Nitpick comments (1)
cmd/cloud.go (1)

773-774: Comment doesn't match implementation

The comment states "this will never happen because we've already checked the deployment id" but there's no prior check that guarantees selectedDeployment won't be empty.

-				// this will never happen because we've already checked the deployment id
+				// User didn't select a deployment or selection was cancelled
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9bc7451 and 970e7fd.

📒 Files selected for processing (1)
  • cmd/cloud.go (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Test CLI Upgrade Path (windows-latest)
  • GitHub Check: Analyze (go)
🔇 Additional comments (7)
cmd/cloud.go (7)

675-686: Command structure looks well-defined

The command definition properly includes usage examples and clear descriptions of its functionality to rollback or delete deployments.


694-735: Project selection logic handles different input methods appropriately

The implementation properly handles multiple ways to specify a project:

  1. From a directory using --dir
  2. From a project ID using --project
  3. Via interactive selection if neither is provided

This provides good flexibility for both interactive and scripted usage.


738-776: Tag-based deployment selection works as expected

The code appropriately handles both tag-based selection and interactive selection of deployments, with clear error messaging when no deployment matches the provided tag.


777-791: Confirmation flow handles non-interactive usage

The implementation correctly uses the --force flag to bypass confirmation prompts, which is essential for scripted/automated usage. The confirmation message changes based on the operation type (rollback vs delete).


861-865: Message truncation is handled well

The code correctly checks if the message length exceeds 60 characters and truncates it appropriately with an ellipsis to maintain a clean UI.


872-881: Display format for active vs inactive deployments

The code clearly distinguishes active deployments (with a checkmark) from inactive ones, making it easy for users to identify the currently active deployment.

Note: A previous comment from jhaynie mentioned "i don't really love the way this looks..." - consider revisiting this display format after more feedback.


917-923: Command registration and flag definitions are comprehensive

The command is correctly registered with both rootCmd and cloudCmd, and includes all necessary flags for functionality, including --force for non-interactive usage. This supports both interactive and scripted workflows.

Comment thread cmd/cloud.go
Comment on lines +852 to +853
os.Exit(1)
}
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use errsystem instead of direct os.Exit

Direct calls to os.Exit(1) are inconsistent with the project's error handling pattern.

-		tui.ShowWarning("no deployments found for this project")
-		os.Exit(1)
+		errsystem.New(errsystem.ErrApiRequest, fmt.Errorf("no deployments found for this project")).ShowErrorAndExit()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
os.Exit(1)
}
if len(deployments) == 0 {
- tui.ShowWarning("no deployments found for this project")
- os.Exit(1)
+ errsystem.New(
+ errsystem.ErrApiRequest,
+ fmt.Errorf("no deployments found for this project"),
+ ).ShowErrorAndExit()
}
🤖 Prompt for AI Agents
In cmd/cloud.go around lines 852 to 853, replace the direct call to os.Exit(1)
with the project's standard error handling function errsystem. Identify the
appropriate errsystem call that signals a fatal error and use it instead of
os.Exit to maintain consistency with the project's error handling pattern.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't a unexpected system error which is when we use errsystem vs an expected one.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jhaynie I appreciate the clarification. You're absolutely right - this is an expected condition rather than an unexpected system error. I misunderstood the error handling pattern in the codebase. The current implementation with tui.ShowWarning() followed by os.Exit(1) is the correct approach for expected error conditions.

(_/)
(•ᴥ•)
/ >📝


✏️ Learnings added
Learnt from: jhaynie
PR: agentuity/cli#313
File: cmd/cloud.go:852-853
Timestamp: 2025-05-22T18:27:10.355Z
Learning: In the agentuity CLI codebase, there's a specific error handling pattern: errsystem is used exclusively for unexpected system errors, while expected error conditions use tui.ShowWarning() and direct os.Exit(1) calls.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@jhaynie jhaynie merged commit a6ed694 into main May 22, 2025
14 checks passed
@jhaynie jhaynie deleted the AGENT-130 branch May 22, 2025 18:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants