Skip to content

Conversation

@jongwooo
Copy link
Contributor

This pull request introduces a new agent command group to the CLI, providing comprehensive lifecycle management for the morpher agent. The implementation includes commands for installing, uninstalling, upgrading, verifying, and checking the status of the agent, along with the supporting logic in the internal package. The changes are organized to improve user experience and maintainability by modularizing agent-related operations.

Agent command group and CLI integration:

  • Added the agent command group with subcommands for install, uninstall, upgrade, verify, and status, enabling users to manage the morpher agent directly from the CLI (cmd/agent/agent.go, cmd/root.go). [1] [2] [3]

Agent lifecycle operations:

  • Implemented the install, uninstall, upgrade, verify, and status commands, each with preflight checks, user prompts, and detailed output to guide users through agent management tasks (cmd/agent/install.go, cmd/agent/uninstall.go, cmd/agent/upgrade.go, cmd/agent/verify.go, cmd/agent/status.go). [1] [2] [3] [4] [5]

Internal agent logic:

  • Added the internal/agent/agent.go package, encapsulating all agent management logic such as installation, upgrade, verification, status checks, and helper methods for interacting with system services and files. This abstraction simplifies command implementations and centralizes agent-related operations.

User experience improvements:

  • Enhanced command outputs with clear status messages, summaries, and prompts for confirmation (e.g., during uninstall), as well as support for detailed status reporting via flags. [1] [2] [3]

Platform and privilege checks:

  • Added robust preflight checks for required privileges and platform compatibility (systemd/sudo/launchd), ensuring commands fail gracefully with helpful error messages when prerequisites are not met. [1] [2] [3]

These changes collectively provide a full-featured agent management experience within the CLI, with clear separation of concerns and improved reliability.

@jongwooo jongwooo requested a review from Copilot August 21, 2025 10:59
@jongwooo jongwooo self-assigned this Aug 21, 2025
@jongwooo jongwooo added the feat label Aug 21, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request introduces a comprehensive agent command group to the CLI, enabling lifecycle management of the morpher agent. The implementation includes agent installation, uninstallation, upgrade, verification, and status operations.

  • Added agent command group with subcommands for full lifecycle management
  • Refactored controller configuration from URL to separate IP and port settings
  • Implemented internal agent package with installation and service management logic

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
cmd/agent/agent.go Creates the main agent command group and registers subcommands
cmd/agent/install.go Implements agent installation with preflight checks
cmd/agent/uninstall.go Implements agent removal with confirmation prompts
cmd/agent/upgrade.go Implements agent upgrade functionality
cmd/agent/verify.go Implements agent installation verification
cmd/agent/status.go Implements agent status checking with optional detailed output
cmd/root.go Registers the agent command group with the root CLI
internal/agent/agent.go Core agent management logic and service operations
internal/agent/agent_test.go Comprehensive test coverage for agent functionality
internal/config/config.go Updates default configuration from URL to separate IP/port
internal/config/config_test.go Updates tests to reflect new configuration structure

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

// Fallback to curl.
if _, err := exec.LookPath("curl"); err == nil {
cmd := exec.Command("curl", "-o", dest, url)
return fmt.Errorf("curl failed: %w", cmd.Run())
Copy link

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

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

The error message indicates wget failed even when wget successfully downloads the file. The error should only be returned if cmd.Run() actually returns an error.

Suggested change
return fmt.Errorf("curl failed: %w", cmd.Run())
if err := cmd.Run(); err != nil {
return fmt.Errorf("wget failed: %w", err)
}
return nil
}
// Fallback to curl.
if _, err := exec.LookPath("curl"); err == nil {
cmd := exec.Command("curl", "-o", dest, url)
if err := cmd.Run(); err != nil {
return fmt.Errorf("curl failed: %w", err)
}
return nil

Copilot uses AI. Check for mistakes.
// Fallback to curl.
if _, err := exec.LookPath("curl"); err == nil {
cmd := exec.Command("curl", "-o", dest, url)
return fmt.Errorf("curl failed: %w", cmd.Run())
Copy link

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

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

The error message indicates curl failed even when curl successfully downloads the file. The error should only be returned if cmd.Run() actually returns an error.

Suggested change
return fmt.Errorf("curl failed: %w", cmd.Run())
if err := cmd.Run(); err != nil {
return fmt.Errorf("wget failed: %w", err)
}
return nil
}
// Fallback to curl.
if _, err := exec.LookPath("curl"); err == nil {
cmd := exec.Command("curl", "-o", dest, url)
if err := cmd.Run(); err != nil {
return fmt.Errorf("curl failed: %w", err)
}
return nil

Copilot uses AI. Check for mistakes.
}

func (a *Agent) removeServiceFile() error {
servicePath := fmt.Sprintf("/etc/systemd/system/%s.service", a.ServiceName)
Copy link

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

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

The hardcoded path '/etc/systemd/system/%s.service' assumes systemd is always available. This conflicts with the macOS support mentioned in upgrade preflight checks and should handle different service management systems.

Suggested change
servicePath := fmt.Sprintf("/etc/systemd/system/%s.service", a.ServiceName)
var servicePath string
switch runtime.GOOS {
case "linux":
servicePath = fmt.Sprintf("/etc/systemd/system/%s.service", a.ServiceName)
case "darwin":
servicePath = fmt.Sprintf("/Library/LaunchDaemons/%s.plist", a.ServiceName)
default:
return fmt.Errorf("unsupported OS for service file removal: %s", runtime.GOOS)
}

Copilot uses AI. Check for mistakes.
// Check if systemctl is available.
if _, err := os.Stat("/run/systemd/system"); os.IsNotExist(err) {
return fmt.Errorf("systemd is not available")
}
Copy link

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

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

The preflight check only supports systemd but the upgrade command shows support for launchd (macOS). The install preflight should be consistent with upgrade preflight.

Suggested change
}
// Check if systemd or launchd is available.
systemdAvailable := false
launchdAvailable := false
if _, err := os.Stat("/run/systemd/system"); err == nil {
systemdAvailable = true
}
// Check for launchctl (macOS launchd)
if _, err := os.Stat("/bin/launchctl"); err == nil {
launchdAvailable = true
} else if _, err := os.Stat("/usr/bin/launchctl"); err == nil {
launchdAvailable = true
}
if !systemdAvailable && !launchdAvailable {
return fmt.Errorf("neither systemd nor launchd is available")
}

Copilot uses AI. Check for mistakes.

// Check if systemctl is available.
if _, err := os.Stat("/run/systemd/system"); os.IsNotExist(err) {
return fmt.Errorf("systemd is not available")
Copy link

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

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

The uninstall preflight check only supports systemd but upgrade shows support for launchd (macOS). Platform support should be consistent across all commands.

Suggested change
return fmt.Errorf("systemd is not available")
switch runtime.GOOS {
case "linux":
// Check if systemctl is available.
if _, err := os.Stat("/run/systemd/system"); os.IsNotExist(err) {
return fmt.Errorf("systemd is not available")
}
case "darwin":
// Check if launchctl is available.
if _, err := os.Stat("/sbin/launchctl"); os.IsNotExist(err) {
return fmt.Errorf("launchd (launchctl) is not available")
}
default:
return fmt.Errorf("unsupported platform: %s", runtime.GOOS)

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant