refactor(repo): General improvements

Author: pedronauck

cmd/container.go

@@ -97,7 +97,7 @@ func InitCommands() error {
 func addOrchestratorCommands(ctx context.Context, c *container) error {
 	log := logger.FromContext(ctx).Named("cmd.container")
 	// Initialize extended repositories for orchestrators
-	gitExtRepo, err := repository.NewGitExtendedRepository()
+	gitExtRepo, err := repository.NewGitExtendedRepositoryWithTimeout(c.cfg.GitPushTimeoutMinutes)
 	if err != nil {
 		return fmt.Errorf("failed to initialize git extended repository: %w", err)
 	}

internal/config/config.go

@@ -13,13 +13,14 @@ import (
 )
 
 type Config struct {
-	GithubToken string `mapstructure:"github_token"`
-	GithubOwner string `mapstructure:"github_owner"`
-	GithubRepo  string `mapstructure:"github_repo"`
-	ToolsDir    string `mapstructure:"tools_dir"`
-	NpmToken    string `mapstructure:"npm_token"`
-	LogLevel    string `mapstructure:"log_level"`
-	LogFormat   string `mapstructure:"log_format"`
+	GithubToken           string `mapstructure:"github_token"`
+	GithubOwner           string `mapstructure:"github_owner"`
+	GithubRepo            string `mapstructure:"github_repo"`
+	ToolsDir              string `mapstructure:"tools_dir"`
+	NpmToken              string `mapstructure:"npm_token"`
+	LogLevel              string `mapstructure:"log_level"`
+	LogFormat             string `mapstructure:"log_format"`
+	GitPushTimeoutMinutes int    `mapstructure:"git_push_timeout_minutes"`
 }
 
 var configFileCandidates = []string{".pr-release", ".compozy-release"}
@@ -29,7 +30,12 @@ func DefaultConfig() *Config {
 	if isCI() {
 		logFormat = "console"
 	}
-	return &Config{ToolsDir: "tools", LogLevel: "info", LogFormat: logFormat}
+	return &Config{
+		ToolsDir:              "tools",
+		LogLevel:              "info",
+		LogFormat:             logFormat,
+		GitPushTimeoutMinutes: 2,
+	}
 }
 
 func isCI() bool {
@@ -75,6 +81,9 @@ func (c *Config) Validate() error {
 	if err := validateLogFormat(c.LogFormat); err != nil {
 		return err
 	}
+	if c.GitPushTimeoutMinutes < 1 || c.GitPushTimeoutMinutes > 30 {
+		return fmt.Errorf("git_push_timeout_minutes must be between 1 and 30, got %d", c.GitPushTimeoutMinutes)
+	}
 	return nil
 }
 
@@ -146,63 +155,52 @@ func ValidateGitHubOwnerRepo(owner, repo string) error {
 	return nil
 }
 
+func bindEnvironmentVariables(v *viper.Viper) error {
+	bindings := map[string][]string{
+		"github_token": {
+			"GITHUB_TOKEN",
+			"PR_RELEASE_GITHUB_TOKEN",
+			"COMPOZY_RELEASE_GITHUB_TOKEN",
+			"RELEASE_TOKEN",
+		},
+		"github_owner": {"GITHUB_OWNER", "PR_RELEASE_GITHUB_OWNER", "COMPOZY_RELEASE_GITHUB_OWNER"},
+		"github_repo":  {"GITHUB_REPO", "PR_RELEASE_GITHUB_REPO", "COMPOZY_RELEASE_GITHUB_REPO"},
+		"tools_dir":    {"TOOLS_DIR", "PR_RELEASE_TOOLS_DIR", "COMPOZY_RELEASE_TOOLS_DIR"},
+		"log_level":    {"LOG_LEVEL", "PR_RELEASE_LOG_LEVEL", "COMPOZY_RELEASE_LOG_LEVEL"},
+		"log_format":   {"LOG_FORMAT", "PR_RELEASE_LOG_FORMAT", "COMPOZY_RELEASE_LOG_FORMAT"},
+		"npm_token":    {"NPM_TOKEN", "PR_RELEASE_NPM_TOKEN", "COMPOZY_RELEASE_NPM_TOKEN"},
+		"git_push_timeout_minutes": {
+			"GIT_PUSH_TIMEOUT_MINUTES",
+			"PR_RELEASE_GIT_PUSH_TIMEOUT_MINUTES",
+			"COMPOZY_RELEASE_GIT_PUSH_TIMEOUT_MINUTES",
+		},
+	}
+	for key, envs := range bindings {
+		if err := v.BindEnv(append([]string{key}, envs...)...); err != nil {
+			return fmt.Errorf("failed to bind %s env: %w", key, err)
+		}
+	}
+	return nil
+}
+
+func setConfigDefaults(v *viper.Viper) {
+	defaults := DefaultConfig()
+	v.SetDefault("tools_dir", defaults.ToolsDir)
+	v.SetDefault("log_level", defaults.LogLevel)
+	v.SetDefault("log_format", defaults.LogFormat)
+	v.SetDefault("git_push_timeout_minutes", defaults.GitPushTimeoutMinutes)
+}
+
 func LoadConfig() (*Config, error) {
 	v := viper.New()
 	v.SetConfigType("yaml")
 	v.AddConfigPath(".")
 	v.AutomaticEnv()
 	v.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
-	if err := v.BindEnv(
-		"github_token",
-		"GITHUB_TOKEN",
-		"PR_RELEASE_GITHUB_TOKEN",
-		"COMPOZY_RELEASE_GITHUB_TOKEN",
-		"RELEASE_TOKEN",
-	); err != nil {
-		return nil, fmt.Errorf("failed to bind github_token env: %w", err)
-	}
-	if err := v.BindEnv(
-		"github_owner",
-		"GITHUB_OWNER",
-		"PR_RELEASE_GITHUB_OWNER",
-		"COMPOZY_RELEASE_GITHUB_OWNER",
-	); err != nil {
-		return nil, fmt.Errorf("failed to bind github_owner env: %w", err)
-	}
-	if err := v.BindEnv(
-		"github_repo",
-		"GITHUB_REPO",
-		"PR_RELEASE_GITHUB_REPO",
-		"COMPOZY_RELEASE_GITHUB_REPO",
-	); err != nil {
-		return nil, fmt.Errorf("failed to bind github_repo env: %w", err)
-	}
-	if err := v.BindEnv("tools_dir", "TOOLS_DIR", "PR_RELEASE_TOOLS_DIR", "COMPOZY_RELEASE_TOOLS_DIR"); err != nil {
-		return nil, fmt.Errorf("failed to bind tools_dir env: %w", err)
-	}
-	if err := v.BindEnv(
-		"log_level",
-		"LOG_LEVEL",
-		"PR_RELEASE_LOG_LEVEL",
-		"COMPOZY_RELEASE_LOG_LEVEL",
-	); err != nil {
-		return nil, fmt.Errorf("failed to bind log_level env: %w", err)
-	}
-	if err := v.BindEnv(
-		"log_format",
-		"LOG_FORMAT",
-		"PR_RELEASE_LOG_FORMAT",
-		"COMPOZY_RELEASE_LOG_FORMAT",
-	); err != nil {
-		return nil, fmt.Errorf("failed to bind log_format env: %w", err)
-	}
-	if err := v.BindEnv("npm_token", "NPM_TOKEN", "PR_RELEASE_NPM_TOKEN", "COMPOZY_RELEASE_NPM_TOKEN"); err != nil {
-		return nil, fmt.Errorf("failed to bind npm_token env: %w", err)
+	if err := bindEnvironmentVariables(v); err != nil {
+		return nil, err
 	}
-	defaults := DefaultConfig()
-	v.SetDefault("tools_dir", defaults.ToolsDir)
-	v.SetDefault("log_level", defaults.LogLevel)
-	v.SetDefault("log_format", defaults.LogFormat)
+	setConfigDefaults(v)
 	for _, name := range configFileCandidates {
 		v.SetConfigName(name)
 		if err := v.ReadInConfig(); err != nil {

internal/orchestrator/compensating_acts.go

@@ -261,16 +261,11 @@ func (ca *CompensatingActions) branchExistsLocally(ctx context.Context, branchNa
 }
 
 func (ca *CompensatingActions) branchExistsRemotely(ctx context.Context, branchName string) bool {
-	branches, err := ca.gitRepo.ListRemoteBranches(ctx)
+	exists, err := ca.gitRepo.RemoteBranchExists(ctx, branchName)
 	if err != nil {
 		return false
 	}
-	for _, branch := range branches {
-		if strings.HasSuffix(branch, "/"+branchName) {
-			return true
-		}
-	}
-	return false
+	return exists
 }
 
 func (ca *CompensatingActions) fileHasChanges(ctx context.Context, file string) bool {

internal/orchestrator/mocks_test.go

@@ -99,6 +99,10 @@ func (m *mockGitExtendedRepository) ListRemoteBranches(ctx context.Context) ([]s
 	}
 	return nil, args.Error(1)
 }
+func (m *mockGitExtendedRepository) RemoteBranchExists(ctx context.Context, branchName string) (bool, error) {
+	args := m.Called(ctx, branchName)
+	return args.Bool(0), args.Error(1)
+}
 func (m *mockGitExtendedRepository) GetFileStatus(ctx context.Context, path string) (string, error) {
 	args := m.Called(ctx, path)
 	return args.String(0), args.Error(1)

internal/orchestrator/pr_release.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"slices"
 	"strings"
+	"time"
 
 	"github.com/compozy/releasepr/internal/domain"
 	"github.com/compozy/releasepr/internal/logger"
@@ -372,7 +373,7 @@ func (o *PRReleaseOrchestrator) buildAndExecuteWorkflow(
 	// Add all workflow steps
 	o.addCheckChangesStep(saga, cfg, compensator, wctx)
 	o.addCalculateVersionStep(saga, cfg, compensator, wctx)
-	o.addCreateBranchStep(saga, compensator, wctx, originalBranch)
+	o.addCreateBranchStep(saga, cfg, compensator, wctx, originalBranch)
 	o.addUpdatePackagesStep(saga, compensator, wctx)
 	o.addGenerateChangelogStep(saga, compensator, wctx)
 	o.addCommitChangesStep(saga, compensator, wctx)
@@ -461,6 +462,7 @@ func (o *PRReleaseOrchestrator) addCalculateVersionStep(
 
 func (o *PRReleaseOrchestrator) addCreateBranchStep(
 	saga *SagaExecutor,
+	cfg PRReleaseConfig,
 	compensator *CompensatingActions,
 	wctx *workflowContext,
 	originalBranch string,
@@ -472,49 +474,39 @@ func (o *PRReleaseOrchestrator) addCreateBranchStep(
 			if wctx.version == "" {
 				return map[string]any{"skip": true}, nil
 			}
-			wctx.branchName = fmt.Sprintf("release/%s", wctx.version)
-			o.logger(ctx).Info("Determined release branch", zap.String("branch", wctx.branchName))
-			if err := ValidateBranchName(wctx.branchName); err != nil {
-				return nil, fmt.Errorf("invalid branch name: %w", err)
-			}
-			saga.SetBranchName(wctx.branchName)
-			branches, err := o.gitRepo.ListLocalBranches(ctx)
+			branchName, err := o.prepareBranchName(ctx, saga, wctx)
 			if err != nil {
-				return nil, fmt.Errorf("failed to list local branches: %w", err)
+				return nil, err
 			}
-			branchExists := slices.Contains(branches, wctx.branchName)
-			remoteBranches, err := o.gitRepo.ListRemoteBranches(ctx)
+			branchExists, remoteExists, err := o.checkBranchExistence(ctx, branchName)
 			if err != nil {
-				return nil, fmt.Errorf("failed to list remote branches: %w", err)
+				return nil, err
 			}
-			remoteExists := slices.Contains(remoteBranches, fmt.Sprintf("origin/%s", wctx.branchName))
-			switch {
-			case branchExists && remoteExists:
-				o.logger(ctx).Info(
-					"Reusing existing branch",
-					zap.String("branch", wctx.branchName),
-					zap.Bool("local", true),
-					zap.Bool("remote", true),
+			if cfg.ForceRelease {
+				branchExists, remoteExists, err = o.forceDeleteBranch(
+					ctx,
+					branchName,
+					branchExists,
+					remoteExists,
+					originalBranch,
 				)
-			case branchExists:
-				o.logger(ctx).Info("Checking out existing local branch", zap.String("branch", wctx.branchName))
-			case remoteExists:
-				o.logger(ctx).Info("Checking out remote branch", zap.String("branch", wctx.branchName))
-			default:
-				o.logger(ctx).Info("Creating new branch", zap.String("branch", wctx.branchName))
+				if err != nil {
+					return nil, err
+				}
 			}
+			o.logBranchStatus(ctx, branchName, branchExists, remoteExists)
 			wctx.createdInSession = !branchExists
 			if wctx.createdInSession {
-				if err := o.createReleaseBranch(ctx, wctx.branchName); err != nil {
-					return nil, fmt.Errorf("failed to create release branch %s: %w", wctx.branchName, err)
+				if err := o.createReleaseBranch(ctx, branchName); err != nil {
+					return nil, fmt.Errorf("failed to create release branch %s: %w", branchName, err)
 				}
 			}
-			if err := o.gitRepo.CheckoutBranch(ctx, wctx.branchName); err != nil {
-				return nil, fmt.Errorf("failed to checkout release branch %s: %w", wctx.branchName, err)
+			if err := o.gitRepo.CheckoutBranch(ctx, branchName); err != nil {
+				return nil, fmt.Errorf("failed to checkout release branch %s: %w", branchName, err)
 			}
-			o.logger(ctx).Info("Checked out release branch", zap.String("branch", wctx.branchName))
+			o.logger(ctx).Info("Checked out release branch", zap.String("branch", branchName))
 			return map[string]any{
-				"branch_name":        wctx.branchName,
+				"branch_name":        branchName,
 				"original_branch":    originalBranch,
 				"created_in_session": wctx.createdInSession,
 				"remote_exists":      remoteExists,
@@ -524,6 +516,98 @@ func (o *PRReleaseOrchestrator) addCreateBranchStep(
 	})
 }
 
+func (o *PRReleaseOrchestrator) prepareBranchName(
+	ctx context.Context,
+	saga *SagaExecutor,
+	wctx *workflowContext,
+) (string, error) {
+	wctx.branchName = fmt.Sprintf("release/%s", wctx.version)
+	o.logger(ctx).Info("Determined release branch", zap.String("branch", wctx.branchName))
+	if err := ValidateBranchName(wctx.branchName); err != nil {
+		return "", fmt.Errorf("invalid branch name: %w", err)
+	}
+	saga.SetBranchName(wctx.branchName)
+	return wctx.branchName, nil
+}
+
+func (o *PRReleaseOrchestrator) checkBranchExistence(ctx context.Context, branchName string) (bool, bool, error) {
+	branches, err := o.gitRepo.ListLocalBranches(ctx)
+	if err != nil {
+		return false, false, fmt.Errorf("failed to list local branches: %w", err)
+	}
+	branchExists := slices.Contains(branches, branchName)
+	checkCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
+	defer cancel()
+	remoteExists, err := o.gitRepo.RemoteBranchExists(checkCtx, branchName)
+	if err != nil {
+		o.logger(ctx).Warn("Failed to check remote branch, assuming it doesn't exist",
+			zap.String("branch", branchName),
+			zap.Error(err))
+		remoteExists = false
+	}
+	return branchExists, remoteExists, nil
+}
+
+func (o *PRReleaseOrchestrator) forceDeleteBranch(
+	ctx context.Context,
+	branchName string,
+	branchExists, remoteExists bool,
+	originalBranch string,
+) (bool, bool, error) {
+	if !branchExists && !remoteExists {
+		return branchExists, remoteExists, nil
+	}
+	o.logger(ctx).Info("Force flag set, deleting existing branch",
+		zap.String("branch", branchName),
+		zap.Bool("local_exists", branchExists),
+		zap.Bool("remote_exists", remoteExists))
+	if branchExists {
+		if err := o.gitRepo.CheckoutBranch(ctx, originalBranch); err != nil {
+			return branchExists, remoteExists, fmt.Errorf(
+				"failed to checkout original branch %s: %w",
+				originalBranch,
+				err,
+			)
+		}
+		if err := o.gitRepo.DeleteBranch(ctx, branchName); err != nil {
+			return branchExists, remoteExists, fmt.Errorf("failed to delete local branch %s: %w", branchName, err)
+		}
+		o.logger(ctx).Info("Deleted local branch", zap.String("branch", branchName))
+	}
+	if remoteExists {
+		if err := o.gitRepo.DeleteRemoteBranch(ctx, branchName); err != nil {
+			o.logger(ctx).Warn("Failed to delete remote branch, will force push",
+				zap.String("branch", branchName),
+				zap.Error(err))
+		} else {
+			o.logger(ctx).Info("Deleted remote branch", zap.String("branch", branchName))
+		}
+	}
+	return false, false, nil
+}
+
+func (o *PRReleaseOrchestrator) logBranchStatus(
+	ctx context.Context,
+	branchName string,
+	branchExists, remoteExists bool,
+) {
+	switch {
+	case branchExists && remoteExists:
+		o.logger(ctx).Info(
+			"Reusing existing branch",
+			zap.String("branch", branchName),
+			zap.Bool("local", true),
+			zap.Bool("remote", true),
+		)
+	case branchExists:
+		o.logger(ctx).Info("Checking out existing local branch", zap.String("branch", branchName))
+	case remoteExists:
+		o.logger(ctx).Info("Checking out remote branch", zap.String("branch", branchName))
+	default:
+		o.logger(ctx).Info("Creating new branch", zap.String("branch", branchName))
+	}
+}
+
 func (o *PRReleaseOrchestrator) addUpdatePackagesStep(
 	saga *SagaExecutor,
 	compensator *CompensatingActions,