internal/exec/go_getter_utils.go (2)

160-168: Eliminate potential duplication with existing logic.

This RegisterCustomGetters seems to mirror the RegisterCustomDetectors pattern. Consider centralizing shared logic or converting both into a unified registration function to reduce redundancy.

---

207-220: Consider configurable symlink removal.

Automatically stripping symlinks is helpful, but give the user control to skip or confirm. This might be vital for some use cases or compliance requirements.

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

internal/exec/go_getter_utils.go (1)

202-231: Enhance symlink handling with additional safeguards.

While the implementation is functional, consider these improvements:

1. Pass AtmosConfig to removeSymlinks instead of using an empty config
2. Add protection against circular symlinks
3. Log removed symlink details for audit purposes

Here's a suggested enhancement:

 type CustomGitGetter struct {
     getter.GitGetter
+    AtmosConfig schema.AtmosConfiguration
 }

 func (c *CustomGitGetter) Get(dst string, url *url.URL) error {
     if err := c.GitGetter.Get(dst, url); err != nil {
         return err
     }
-    return removeSymlinks(dst)
+    return removeSymlinks(dst, c.AtmosConfig)
 }

-func removeSymlinks(root string) error {
+func removeSymlinks(root string, config schema.AtmosConfiguration) error {
+    visited := make(map[string]bool)
     return filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
         if err != nil {
             return err
         }
         if info.Mode()&os.ModeSymlink != 0 {
+            // Check for circular symlinks
+            realPath, err := filepath.EvalSymlinks(path)
+            if err != nil {
+                return fmt.Errorf("failed to evaluate symlink %s: %w", path, err)
+            }
+            if visited[realPath] {
+                return fmt.Errorf("circular symlink detected: %s", path)
+            }
+            visited[realPath] = true
+
+            // Log symlink details before removal
+            target, _ := os.Readlink(path)
-            u.LogWarning(schema.AtmosConfiguration{}, fmt.Sprintf("Removing symlink: %s", path))
+            u.LogWarning(config, fmt.Sprintf("Removing symlink: %s -> %s", path, target))
             return os.Remove(path)
         }
         return nil
     })
 }

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

internal/exec/copy_glob.go (1)

168-255: Consider simplifying copyToTargetWithPatterns.

The function logic is clear, but the length and multiple branches make it complex. Splitting out smaller helper functions (e.g., for handling the included vs excluded paths) might improve maintainability and ease of unit testing.

🧰 Tools

🪛 GitHub Check: Build (macos-latest, macos)

[failure] 184-184:
too many arguments in call to u.LogTrace

🪛 GitHub Check: Build (ubuntu-latest, linux)

[failure] 184-184:
too many arguments in call to u.LogTrace

tests/fixtures/scenarios/vendor/vendor.yaml (1)

54-64: Add a test case to validate the newly introduced globs.

The expanded vendor configuration is helpful for controlling which files are included or excluded. Remember to add or update an integration test verifying that only .tf and .md files are included while .tfvars files are excluded for this source.

Would you like help creating a dedicated test that specifically checks the behavior of these globs?

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

internal/exec/go_getter_utils.go (2)

202-215: Consider enhancing error context.

While the implementation is solid, consider wrapping the errors with additional context to aid in debugging:

 func (c *CustomGitGetter) Get(dst string, url *url.URL) error {
 	// Normal clone
 	if err := c.GitGetter.Get(dst, url); err != nil {
-		return err
+		return fmt.Errorf("git clone failed for %s: %w", url, err)
 	}
 	// Remove symlinks
-	return removeSymlinks(dst)
+	if err := removeSymlinks(dst); err != nil {
+		return fmt.Errorf("failed to remove symlinks in %s: %w", dst, err)
+	}
+	return nil
 }

---

219-231: Consider adding path validation and detailed logging.

The symlink removal implementation could be enhanced with additional safety checks and logging:

 func removeSymlinks(root string) error {
+	// Validate root path
+	if _, err := os.Stat(root); err != nil {
+		return fmt.Errorf("invalid root path %s: %w", root, err)
+	}
+
 	return filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
 		if info.Mode()&os.ModeSymlink != 0 {
-			u.LogWarning(schema.AtmosConfiguration{}, fmt.Sprintf("Removing symlink: %s", path))
+			target, err := os.Readlink(path)
+			if err != nil {
+				return fmt.Errorf("failed to read symlink %s: %w", path, err)
+			}
+			u.LogWarning(schema.AtmosConfiguration{}, fmt.Sprintf("Removing symlink: %s -> %s", path, target))
 			// It's a symlink, remove it
-			return os.Remove(path)
+			if err := os.Remove(path); err != nil {
+				return fmt.Errorf("failed to remove symlink %s: %w", path, err)
+			}
 		}
 		return nil
 	})
 }

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

internal/exec/copy_glob.go (1)

47-92: ⚠️ Potential issue

Fix LogTrace calls to match the updated function signature.

Based on the pipeline failures and past review comments, the LogTrace calls need to be updated.

Apply this fix to all LogTrace calls in the function:

-    u.LogTrace(atmosConfig, fmt.Sprintf("Error computing relative path for %q: %v", srcPath, err))
+    u.LogTrace(fmt.Sprintf("Error computing relative path for %q: %v", srcPath, err))

internal/exec/copy_glob.go (3)

16-45: Consider handling errors from deferred Close() calls.

While the implementation is solid, it's good practice to handle errors from Close() operations, especially for the destination file where data corruption could occur.

Here's how you could improve the error handling:

 func copyFile(atmosConfig schema.AtmosConfiguration, src, dst string) error {
     sourceFile, err := os.Open(src)
     if err != nil {
         return fmt.Errorf("opening source file %q: %w", src, err)
     }
-    defer sourceFile.Close()
+    defer func() {
+        if cerr := sourceFile.Close(); cerr != nil && err == nil {
+            err = fmt.Errorf("closing source file %q: %w", src, cerr)
+        }
+    }()
 
     if err := os.MkdirAll(filepath.Dir(dst), os.ModePerm); err != nil {
         return fmt.Errorf("creating destination directory for %q: %w", dst, err)
     }
 
     destinationFile, err := os.Create(dst)
     if err != nil {
         return fmt.Errorf("creating destination file %q: %w", dst, err)
     }
-    defer destinationFile.Close()
+    defer func() {
+        if cerr := destinationFile.Close(); cerr != nil && err == nil {
+            err = fmt.Errorf("closing destination file %q: %w", dst, cerr)
+        }
+    }()

---

139-166: Consider extracting pattern suffixes as constants.

The pattern matching logic is well-implemented, but the magic strings could be made more maintainable.

Consider this improvement:

+const (
+    singleLevelGlob = "/*"
+    recursiveGlob   = "/**"
+)
+
 func getMatchesForPattern(atmosConfig schema.AtmosConfiguration, sourceDir, pattern string) ([]string, error) {
     fullPattern := filepath.Join(sourceDir, pattern)
     matches, err := u.GetGlobMatches(fullPattern)
     if err != nil {
         return nil, fmt.Errorf("error getting glob matches for %q: %w", fullPattern, err)
     }
     if len(matches) == 0 {
-        if strings.HasSuffix(pattern, "/*") {
-            recursivePattern := strings.TrimSuffix(pattern, "/*") + "/**"
+        if strings.HasSuffix(pattern, singleLevelGlob) {
+            recursivePattern := strings.TrimSuffix(pattern, singleLevelGlob) + recursiveGlob

---

168-255: Consider pre-allocating the map with an estimated size.

The implementation is solid, but the map allocation could be optimized when processing inclusion patterns.

Consider this optimization:

     if len(s.IncludedPaths) > 0 {
-        filesToCopy := make(map[string]struct{})
+        // Pre-allocate map with estimated size based on number of patterns
+        filesToCopy := make(map[string]struct{}, len(s.IncludedPaths)*10)

This pre-allocation helps avoid map resizing during population, potentially improving performance when dealing with many files.

internal/exec/go_getter_utils.go (1)

202-233: Consider adding progress logging for symlink removal.

The symlink removal implementation is solid, but for better observability, consider adding debug logging for the number of symlinks removed.

Here's a suggested enhancement:

 func removeSymlinks(root string) error {
+    removedCount := 0
     err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
         if err != nil {
             return err
         }
         if info.Mode()&os.ModeSymlink != 0 {
-            return os.Remove(path)
+            if err := os.Remove(path); err != nil {
+                return err
+            }
+            removedCount++
             return nil
         }
         return nil
     })
+    if removedCount > 0 {
+        u.LogDebug(fmt.Sprintf("Removed %d symlinks from %s", removedCount, root))
+    }
     return err
 }

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

internal/exec/vendor_model_component.go (1)

209-212: Create a constant for the error wrapping format

Multiple occurrences of fmt.Errorf("%w", Err...) follow the same pattern and could use the wrapErrFmt constant for consistency.

Apply this diff:

-			return fmt.Errorf("%w", ErrLocalMixinURIEmpty)
+			return fmt.Errorf(wrapErrFmt, ErrLocalMixinURIEmpty, nil)

-		return fmt.Errorf("%w", ErrLocalMixinNotImplemented)
+		return fmt.Errorf(wrapErrFmt, ErrLocalMixinNotImplemented, nil)

Or, define a simpler format for unwrapped errors:

+const simpleErrFmt = "%w"

-			return fmt.Errorf("%w", ErrLocalMixinURIEmpty)
+			return fmt.Errorf(simpleErrFmt, ErrLocalMixinURIEmpty)

-		return fmt.Errorf("%w", ErrLocalMixinNotImplemented)
+		return fmt.Errorf(simpleErrFmt, ErrLocalMixinNotImplemented)

🧰 Tools

🪛 GitHub Check: golangci

[failure] 211-211: [golangci] internal/exec/vendor_model_component.go#L211
add-constant: string literal "%w" appears, at least, 4 times, create a named constant for it (revive)

internal/exec/vendor_model.go (1)

250-250: 🛠️ Refactor suggestion

Use the predefined error constants instead of dynamic errors

The code currently creates dynamic errors with errors.New(), but the project has defined static error constants in vendor_model_component.go that should be used for consistency.

Apply this diff to use the predefined error constants:

-				err:  fmt.Errorf(wrapErrFmt, errors.New("failed to create temp directory"), err),
+				err:  fmt.Errorf(wrapErrFmt, ErrCreateTempDir, err),

-				err:  fmt.Errorf(wrapErrFmt, errors.New("failed to set temp directory permissions"), err),
+				err:  fmt.Errorf(wrapErrFmt, ErrSetTempDirPermissions, err),

-					err:  fmt.Errorf(wrapErrFmt, errors.New("failed to download package"), err),
+					err:  fmt.Errorf(wrapErrFmt, ErrDownloadPackage, err),

-					err:  fmt.Errorf(wrapErrFmt, errors.New("failed to process OCI image"), err),
+					err:  fmt.Errorf(wrapErrFmt, ErrProcessOCIImage, err),

-					err:  fmt.Errorf(wrapErrFmt, errors.New("failed to copy package"), err),
+					err:  fmt.Errorf(wrapErrFmt, ErrCopyPackage, err),

-				err:  fmt.Errorf(wrapErrFmt, errors.New("failed to copy package"), err),
+				err:  fmt.Errorf(wrapErrFmt, ErrCopyPackage, err),

Note: You'll need to define ErrSetTempDirPermissions in the error variables section in vendor_model_component.go.

Also applies to: 256-256, 267-267, 275-275, 291-291, 303-303

🧰 Tools

🪛 GitHub Check: golangci

[failure] 250-250: [golangci] internal/exec/vendor_model.go#L250
do not define dynamic errors, use wrapped static errors instead: "errors.New("failed to create temp directory")" (err113)

internal/exec/vendor_model_component.go (2)

138-138: Consider defining a constant for permission value

The permission value 0o700 is a magic number that should be defined as a constant for better maintainability.

+ const tempDirPermissions os.FileMode = 0o700
- if err := os.Chmod(tempDir, 0o700); err != nil {
+ if err := os.Chmod(tempDir, tempDirPermissions); err != nil {

🧰 Tools

🪛 GitHub Check: golangci

[failure] 138-138: [golangci] internal/exec/vendor_model_component.go#L138
add-constant: avoid magic numbers like '0o700', create a named constant for it (revive)

---

170-170: Error handling improvements

Good refactoring of error handling to use standardized error variables with consistent formatting patterns (%w: %v). For the local mixin errors, consider also defining static error variables instead of inline error creation.

+ var (
+     ErrEmptyMixinURI = errors.New("local mixin URI cannot be empty")
+     ErrMixinNotImplemented = errors.New("local mixin installation not implemented")
+ )

  // Later in the code:
- return errors.New("local mixin URI cannot be empty")
+ return ErrEmptyMixinURI

- return errors.New("local mixin installation not implemented")
+ return ErrMixinNotImplemented

Also applies to: 175-176, 193-194, 203-205

🧰 Tools

🪛 GitHub Check: golangci

[failure] 170-170: [golangci] internal/exec/vendor_model_component.go#L170
add-constant: string literal "%w: %v" appears, at least, 4 times, create a named constant for it (revive)

internal/exec/vendor_model.go (1)

255-257: ⚠️ Potential issue

Fix error wrapping format

The fmt.Errorf calls are using incorrect formatting. When using wrapErrFmt which is defined as "%w", you should only provide one argument to wrap. The code is providing two arguments.

Apply this fix to all similar occurrences:

- err: fmt.Errorf(wrapErrFmt, errors.New("failed to create temp directory"), err),
+ err: fmt.Errorf("%w: %v", errors.New("failed to create temp directory"), err),

Or alternatively change the wrapErrFmt constant:

- const wrapErrFmt = "%w"
+ const wrapErrFmt = "%w: %v"

Also applies to: 262-264, 274-276, 283-285, 300-302, 313-315

🧰 Tools

🪛 golangci-lint (1.62.2)

256-256: printf: fmt.Errorf call needs 1 arg but has 2 args

(govet)

🪛 GitHub Check: golangci

[failure] 256-256: [golangci] internal/exec/vendor_model.go#L256
printf: fmt.Errorf call needs 1 arg but has 2 args (govet)

internal/exec/vendor_model.go (2)

23-25: 🛠️ Refactor suggestion

Consider refining the wrapErrFmt.
Currently set to "%w", but it's used with multiple error arguments below, causing format mismatches. A format string like "%w: %v" would be more appropriate for chaining.

---

254-254: ⚠️ Potential issue

Incorrect usage of fmt.Errorf with multiple arguments.
Each call uses %w but passes two errors, conflicting with the single format specifier. Below is a possible fix adjusting to "%w: %v".

- err:  fmt.Errorf(wrapErrFmt, ErrCreateTempDir, err),
+ err:  fmt.Errorf("%w: %v", ErrCreateTempDir, err),

- err:  fmt.Errorf(wrapErrFmt, ErrSetTempDirPermissions, err),
+ err:  fmt.Errorf("%w: %v", ErrSetTempDirPermissions, err),

- err:  fmt.Errorf(wrapErrFmt, ErrDownloadPackage, err),
+ err:  fmt.Errorf("%w: %v", ErrDownloadPackage, err),

- err:  fmt.Errorf(wrapErrFmt, ErrProcessOCIImage, err),
+ err:  fmt.Errorf("%w: %v", ErrProcessOCIImage, err),

- err:  fmt.Errorf(wrapErrFmt, ErrCopyPackage, err),
+ err:  fmt.Errorf("%w: %v", ErrCopyPackage, err),

- err:  fmt.Errorf(wrapErrFmt, ErrCopyPackageToTarget, err),
+ err:  fmt.Errorf("%w: %v", ErrCopyPackageToTarget, err),

Also applies to: 260-260, 271-271, 279-279, 295-295, 307-307

🧰 Tools

🪛 golangci-lint (1.62.2)

254-254: printf: fmt.Errorf call needs 1 arg but has 2 args

(govet)

🪛 GitHub Check: golangci

[failure] 254-254: [golangci] internal/exec/vendor_model.go#L254
printf: fmt.Errorf call needs 1 arg but has 2 args (govet)

internal/exec/vendor_model.go (1)

319-326: Inconsistent parameter passing in ExecuteInstall.

ExecuteInstall still accepts atmosConfig as a value but passes it as a pointer to downloadComponentAndInstall.

For consistency, update the function signature to use a pointer parameter:

-func ExecuteInstall(installer pkgVendor, dryRun bool, atmosConfig schema.AtmosConfiguration) tea.Cmd {
+func ExecuteInstall(installer pkgVendor, dryRun bool, atmosConfig *schema.AtmosConfiguration) tea.Cmd {

And then update line 321 to pass the pointer directly:

-    return downloadAndInstall(installer.atmosPackage, dryRun, atmosConfig)
+    return downloadAndInstall(installer.atmosPackage, dryRun, *atmosConfig)

Or alternatively, update downloadAndInstall to also accept a pointer.

internal/exec/vendor_utils.go (1)

430-430: 💡 Verification agent

🧩 Analysis chain

Undefined error variable in error formatting.

The error variable ErrFailedToInitializeTUIModelWithDetails is referenced but not defined in the provided code.

Fix this by adding the missing error variable definition:

+var ErrFailedToInitializeTUIModelWithDetails = fmt.Errorf("failed to initialize TUI model with details")

This should be added in a central error definitions file or at the top of this file.

---

🏁 Script executed:

#!/bin/bash
# Check if the error variable is defined elsewhere in the codebase
rg -g "*.go" "ErrFailedToInitializeTUIModelWithDetails" --no-filename

Length of output: 147

---

Undefined error variable detected in internal/exec/vendor_utils.go – please define ErrFailedToInitializeTUIModelWithDetails.

The rg search confirms that the variable is referenced at line 430 but isn't defined elsewhere in the repository. Please add the following definition, either in a central error definitions file or at the top of the file:

• Location: Internal/exec/vendor_utils.go (or a central errors file)
• Suggested fix:

+var ErrFailedToInitializeTUIModelWithDetails = fmt.Errorf("failed to initialize TUI model with details")

📝 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.

package exec

import (
	"fmt"
	// other imports...
)

// Add the missing error variable definition here (or in a central errors file)
var ErrFailedToInitializeTUIModelWithDetails = fmt.Errorf("failed to initialize TUI model with details")

// ... rest of the file code ...

func someFunctionThatCallsTUIModel(err error) error {
	// Existing logic
	return fmt.Errorf("%w: %v", ErrFailedToInitializeTUIModelWithDetails, err)
}

// ... rest of the file ...

🧰 Tools

🪛 GitHub Check: golangci

[failure] 430-430: [golangci] internal/exec/vendor_utils.go#L430
undefined: ErrFailedToInitializeTUIModelWithDetails) (typecheck)

---

[failure] 430-430: [golangci] internal/exec/vendor_utils.go#L430
undefined: ErrFailedToInitializeTUIModelWithDetails) (typecheck)

---

[failure] 430-430: [golangci] internal/exec/vendor_utils.go#L430
undefined: ErrFailedToInitializeTUIModelWithDetails (typecheck)

---

[failure] 430-430: [golangci] internal/exec/vendor_utils.go#L430
undefined: ErrFailedToInitializeTUIModelWithDetails) (typecheck)

---

[failure] 430-430: [golangci] internal/exec/vendor_utils.go#L430
undefined: ErrFailedToInitializeTUIModelWithDetails) (typecheck)

---

[failure] 430-430: [golangci] internal/exec/vendor_utils.go#L430
undefined: ErrFailedToInitializeTUIModelWithDetails) (typecheck)

---

[failure] 430-430: [golangci] internal/exec/vendor_utils.go#L430
undefined: ErrFailedToInitializeTUIModelWithDetails) (typecheck)

---

[failure] 430-430: [golangci] internal/exec/vendor_utils.go#L430
undefined: ErrFailedToInitializeTUIModelWithDetails) (typecheck)

---

[failure] 430-430: [golangci] internal/exec/vendor_utils.go#L430
undefined: ErrFailedToInitializeTUIModelWithDetails) (typecheck)

---

[failure] 430-430: [golangci] internal/exec/vendor_utils.go#L430
undefined: ErrFailedToInitializeTUIModelWithDetails) (typecheck)

---

[failure] 430-430: [golangci] internal/exec/vendor_utils.go#L430
undefined: ErrFailedToInitializeTUIModelWithDetails) (typecheck)

---

[failure] 430-430: [golangci] internal/exec/vendor_utils.go#L430
undefined: ErrFailedToInitializeTUIModelWithDetails) (typecheck)

---

[failure] 430-430: [golangci] internal/exec/vendor_utils.go#L430
undefined: ErrFailedToInitializeTUIModelWithDetails) (typecheck)

---

[failure] 430-430: [golangci] internal/exec/vendor_utils.go#L430
undefined: ErrFailedToInitializeTUIModelWithDetails) (typecheck)

🪛 GitHub Check: Build (macos-latest, macos)

[failure] 430-430:
undefined: ErrFailedToInitializeTUIModelWithDetails

🪛 GitHub Check: Build (ubuntu-latest, linux)

[failure] 430-430:
undefined: ErrFailedToInitializeTUIModelWithDetails

internal/exec/vendor_model.go (1)

329-331: 💡 Verification agent

🧩 Analysis chain

Undefined error variable.

The error variable ErrNoValidInstallerPackage is referenced but not defined in the provided code.

Define the missing error variable in a central error definitions file or at the top of this file:

+var ErrNoValidInstallerPackage = fmt.Errorf("no valid installer package")

---

🏁 Script executed:

#!/bin/bash
# Check if the error variable is defined elsewhere
rg -g "*.go" "ErrNoValidInstallerPackage" --no-filename

Length of output: 109

---

Attention: The error variable ErrNoValidInstallerPackage remains undefined. The shell search confirmed that aside from its usage in internal/exec/vendor_model.go (lines 329-331), there’s no definition anywhere in the repository. Please define it—either at the top of this file or in a centralized error definitions file—to ensure proper error handling. For example:

var ErrNoValidInstallerPackage = fmt.Errorf("no valid installer package")

🧰 Tools

🪛 GitHub Check: golangci

[failure] 330-330: [golangci] internal/exec/vendor_model.go#L330
undefined: ErrNoValidInstallerPackage

---

[failure] 330-330: [golangci] internal/exec/vendor_model.go#L330
undefined: ErrNoValidInstallerPackage

---

[failure] 330-330: [golangci] internal/exec/vendor_model.go#L330
undefined: ErrNoValidInstallerPackage

---

[failure] 330-330: [golangci] internal/exec/vendor_model.go#L330
undefined: ErrNoValidInstallerPackage

---

[failure] 330-330: [golangci] internal/exec/vendor_model.go#L330
undefined: ErrNoValidInstallerPackage

---

[failure] 330-330: [golangci] internal/exec/vendor_model.go#L330
undefined: ErrNoValidInstallerPackage

---

[failure] 330-330: [golangci] internal/exec/vendor_model.go#L330
undefined: ErrNoValidInstallerPackage

---

[failure] 330-330: [golangci] internal/exec/vendor_model.go#L330
undefined: ErrNoValidInstallerPackage

---

[failure] 330-330: [golangci] internal/exec/vendor_model.go#L330
undefined: ErrNoValidInstallerPackage

---

[failure] 330-330: [golangci] internal/exec/vendor_model.go#L330
undefined: ErrNoValidInstallerPackage

---

[failure] 330-330: [golangci] internal/exec/vendor_model.go#L330
undefined: ErrNoValidInstallerPackage

---

[failure] 330-330: [golangci] internal/exec/vendor_model.go#L330
undefined: ErrNoValidInstallerPackage

---

[failure] 330-330: [golangci] internal/exec/vendor_model.go#L330
undefined: ErrNoValidInstallerPackage

---

[failure] 330-330: [golangci] internal/exec/vendor_model.go#L330
undefined: ErrNoValidInstallerPackage

🪛 GitHub Check: Build (macos-latest, macos)

[failure] 330-330:
undefined: ErrNoValidInstallerPackage

🪛 GitHub Check: Build (ubuntu-latest, linux)

[failure] 330-330:
undefined: ErrNoValidInstallerPackage

internal/exec/error.go (1)

8-14: Constants look good but could use documentation.

The constants are well-defined with descriptive names, but adding documentation comments would enhance code readability and maintainability.

 const (
+	// progressWidth defines the width of the progress bar in characters
 	progressWidth               = 30
+	// getterTimeout defines the maximum time allowed for downloading packages
 	getterTimeout               = 10 * time.Minute
+	// componentTempDirPermissions defines file permissions for temporary directories (owner read/write/execute only)
 	componentTempDirPermissions = 0o700
+	// wrapErrFmtWithDetails defines the format string for wrapping errors with additional details
 	wrapErrFmtWithDetails       = "%w: %v"
+	// timeFormatBase defines the base for time format conversions
 	timeFormatBase              = 10
 )

internal/exec/error.go (1)

24-28: 🛠️ Refactor suggestion

Duplicate error messages detected.

The error variables ErrFailedToInitializeTUIModel and ErrFailedToInitializeTUIModelWithDetails have identical error messages. This creates potential confusion during error handling. Consider either:

1. Using different messages for each error type
2. Consolidating into a single error variable if they serve the same purpose
3. Creating a function that can wrap additional details if that's the intended difference

 var (
 	ErrDownloadPackage                       = errors.New("failed to download package")
 	ErrProcessOCIImage                       = errors.New("failed to process OCI image")
 	ErrCopyPackage                           = errors.New("failed to copy package")
 	ErrCreateTempDir                         = errors.New("failed to create temp directory")
 	ErrUnknownPackageType                    = errors.New("unknown package type")
 	ErrLocalMixinURICannotBeEmpty            = errors.New("local mixin URI cannot be empty")
 	ErrLocalMixinInstallationNotImplemented  = errors.New("local mixin installation not implemented")
 	ErrFailedToInitializeTUIModel            = errors.New("failed to initialize TUI model: verify terminal capabilities and permissions")
 	ErrSetTempDirPermissions                 = errors.New("failed to set temp directory permissions")
 	ErrCopyPackageToTarget                   = errors.New("failed to copy package to target")
 	ErrNoValidInstallerPackage               = errors.New("no valid installer package provided")
-	ErrFailedToInitializeTUIModelWithDetails = errors.New("failed to initialize TUI model: verify terminal capabilities and permissions")
+	// If you need both errors, use different messages
+	ErrFailedToInitializeTUIModelWithDetails = errors.New("failed to initialize TUI model with extended information: verify terminal capabilities and permissions")
 )

📝 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.

 var (
 	ErrDownloadPackage                       = errors.New("failed to download package")
 	ErrProcessOCIImage                       = errors.New("failed to process OCI image")
 	ErrCopyPackage                           = errors.New("failed to copy package")
 	ErrCreateTempDir                         = errors.New("failed to create temp directory")
 	ErrUnknownPackageType                    = errors.New("unknown package type")
 	ErrLocalMixinURICannotBeEmpty            = errors.New("local mixin URI cannot be empty")
 	ErrLocalMixinInstallationNotImplemented  = errors.New("local mixin installation not implemented")
 	ErrFailedToInitializeTUIModel            = errors.New("failed to initialize TUI model: verify terminal capabilities and permissions")
 	ErrSetTempDirPermissions                 = errors.New("failed to set temp directory permissions")
 	ErrCopyPackageToTarget                   = errors.New("failed to copy package to target")
 	ErrNoValidInstallerPackage               = errors.New("no valid installer package provided")
 	// If you need both errors, use different messages
 	ErrFailedToInitializeTUIModelWithDetails = errors.New("failed to initialize TUI model with extended information: verify terminal capabilities and permissions")
 )

internal/exec/vendor_model.go (1)

23-26: Add period at the end of comment

The comment on line 24 should end with a period to follow code formatting standards.

-	// WrapErrFmt is defined in error.go as wrapErrFmtWithDetails ("%w: %v")
+	// WrapErrFmt is defined in error.go as wrapErrFmtWithDetails ("%w: %v").

🧰 Tools

🪛 GitHub Check: golangci

[failure] 24-24: [golangci] internal/exec/vendor_model.go#L24
Comment should end in a period (godot)

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro