From c25df2298866f063a0b6630238b4bfa381f270c2 Mon Sep 17 00:00:00 2001 From: Thomas Vilte Date: Thu, 30 Jul 2026 18:01:52 -0300 Subject: [PATCH 1/2] refactor(internal): reorganize packages and improve git staging --- internal/ai/cost_wrapper.go | 4 +- internal/ai/cost_wrapper_test.go | 2 +- internal/ai/gemini/helper.go | 36 ++++++++++++++++ internal/ai/gemini/helpers.go | 41 ------------------- internal/commands/stats/stats.go | 2 +- internal/commands/stats/stats_test.go | 2 +- internal/{services => }/cost/calculator.go | 0 .../{services => }/cost/calculator_test.go | 0 internal/{services => }/cost/manager.go | 0 internal/{services => }/cost/manager_test.go | 0 internal/git/git_service.go | 17 ++++++++ .../{services => }/routing/model_selector.go | 0 .../routing/model_selector_test.go | 0 13 files changed, 58 insertions(+), 46 deletions(-) delete mode 100644 internal/ai/gemini/helpers.go rename internal/{services => }/cost/calculator.go (100%) rename internal/{services => }/cost/calculator_test.go (100%) rename internal/{services => }/cost/manager.go (100%) rename internal/{services => }/cost/manager_test.go (100%) rename internal/{services => }/routing/model_selector.go (100%) rename internal/{services => }/routing/model_selector_test.go (100%) diff --git a/internal/ai/cost_wrapper.go b/internal/ai/cost_wrapper.go index 3994e3a..d264988 100644 --- a/internal/ai/cost_wrapper.go +++ b/internal/ai/cost_wrapper.go @@ -9,10 +9,10 @@ import ( "time" "github.com/thomas-vilte/matecommit/internal/cache" + "github.com/thomas-vilte/matecommit/internal/cost" "github.com/thomas-vilte/matecommit/internal/errors" "github.com/thomas-vilte/matecommit/internal/models" - "github.com/thomas-vilte/matecommit/internal/services/cost" - "github.com/thomas-vilte/matecommit/internal/services/routing" + "github.com/thomas-vilte/matecommit/internal/routing" ) type ConfirmationCallback func(result ConfirmationResult) (choice string, proceed bool) diff --git a/internal/ai/cost_wrapper_test.go b/internal/ai/cost_wrapper_test.go index ce8371c..3e6c510 100644 --- a/internal/ai/cost_wrapper_test.go +++ b/internal/ai/cost_wrapper_test.go @@ -7,8 +7,8 @@ import ( "time" "github.com/stretchr/testify/mock" + "github.com/thomas-vilte/matecommit/internal/cost" "github.com/thomas-vilte/matecommit/internal/models" - "github.com/thomas-vilte/matecommit/internal/services/cost" ) type mockProvider struct { diff --git a/internal/ai/gemini/helper.go b/internal/ai/gemini/helper.go index ad728ca..de9e927 100644 --- a/internal/ai/gemini/helper.go +++ b/internal/ai/gemini/helper.go @@ -113,3 +113,39 @@ func extractTextFromMap(respMap map[string]interface{}) string { return result.String() } + +// CleanLabels cleans and validates labels, keeping only the allowed ones. +// It accepts a list of labels to clean and a list of available labels from the repository. +// If availableLabels is empty, it falls back to a default list of common labels. +func CleanLabels(labels []string, availableLabels []string) []string { + allowedLabels := make(map[string]bool) + + if len(availableLabels) > 0 { + for _, l := range availableLabels { + allowedLabels[strings.ToLower(l)] = true + } + } else { + // Fallback to default list if no repo labels provided + defaultLabels := []string{ + "feature", "fix", "refactor", "docs", "test", "infra", + "enhancement", "bug", "good first issue", "help wanted", + "chore", "performance", "security", "tech-debt", "breaking-change", + } + for _, l := range defaultLabels { + allowedLabels[l] = true + } + } + + cleaned := make([]string, 0) + seen := make(map[string]bool) + + for _, label := range labels { + trimmed := strings.TrimSpace(strings.ToLower(label)) + if trimmed != "" && allowedLabels[trimmed] && !seen[trimmed] { + cleaned = append(cleaned, trimmed) + seen[trimmed] = true + } + } + + return cleaned +} diff --git a/internal/ai/gemini/helpers.go b/internal/ai/gemini/helpers.go deleted file mode 100644 index 74c6992..0000000 --- a/internal/ai/gemini/helpers.go +++ /dev/null @@ -1,41 +0,0 @@ -package gemini - -import ( - "strings" -) - -// CleanLabels cleans and validates labels, keeping only the allowed ones. -// It accepts a list of labels to clean and a list of available labels from the repository. -// If availableLabels is empty, it falls back to a default list of common labels. -func CleanLabels(labels []string, availableLabels []string) []string { - allowedLabels := make(map[string]bool) - - if len(availableLabels) > 0 { - for _, l := range availableLabels { - allowedLabels[strings.ToLower(l)] = true - } - } else { - // Fallback to default list if no repo labels provided - defaultLabels := []string{ - "feature", "fix", "refactor", "docs", "test", "infra", - "enhancement", "bug", "good first issue", "help wanted", - "chore", "performance", "security", "tech-debt", "breaking-change", - } - for _, l := range defaultLabels { - allowedLabels[l] = true - } - } - - cleaned := make([]string, 0) - seen := make(map[string]bool) - - for _, label := range labels { - trimmed := strings.TrimSpace(strings.ToLower(label)) - if trimmed != "" && allowedLabels[trimmed] && !seen[trimmed] { - cleaned = append(cleaned, trimmed) - seen[trimmed] = true - } - } - - return cleaned -} diff --git a/internal/commands/stats/stats.go b/internal/commands/stats/stats.go index 5268b67..fc152c4 100644 --- a/internal/commands/stats/stats.go +++ b/internal/commands/stats/stats.go @@ -9,8 +9,8 @@ import ( "github.com/fatih/color" "github.com/thomas-vilte/matecommit/internal/config" + "github.com/thomas-vilte/matecommit/internal/cost" "github.com/thomas-vilte/matecommit/internal/i18n" - "github.com/thomas-vilte/matecommit/internal/services/cost" "github.com/urfave/cli/v3" ) diff --git a/internal/commands/stats/stats_test.go b/internal/commands/stats/stats_test.go index a958cd0..c5a9cb0 100644 --- a/internal/commands/stats/stats_test.go +++ b/internal/commands/stats/stats_test.go @@ -10,8 +10,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/thomas-vilte/matecommit/internal/cost" "github.com/thomas-vilte/matecommit/internal/i18n" - "github.com/thomas-vilte/matecommit/internal/services/cost" ) func TestNewStatsCommand(t *testing.T) { diff --git a/internal/services/cost/calculator.go b/internal/cost/calculator.go similarity index 100% rename from internal/services/cost/calculator.go rename to internal/cost/calculator.go diff --git a/internal/services/cost/calculator_test.go b/internal/cost/calculator_test.go similarity index 100% rename from internal/services/cost/calculator_test.go rename to internal/cost/calculator_test.go diff --git a/internal/services/cost/manager.go b/internal/cost/manager.go similarity index 100% rename from internal/services/cost/manager.go rename to internal/cost/manager.go diff --git a/internal/services/cost/manager_test.go b/internal/cost/manager_test.go similarity index 100% rename from internal/services/cost/manager_test.go rename to internal/cost/manager_test.go diff --git a/internal/git/git_service.go b/internal/git/git_service.go index 065e9a5..18689f1 100644 --- a/internal/git/git_service.go +++ b/internal/git/git_service.go @@ -371,6 +371,23 @@ func (s *GitService) AddFileToStaging(ctx context.Context, file string) error { absFile = file } + // If the file has no pending change relative to the index (e.g. it was + // already staged by the user via a manual "git rm" or "git add"), "git + // add" has nothing to do — and once the file no longer exists on disk + // at all (a staged deletion), running it anyway fails with "pathspec + // did not match any files" instead of being the harmless no-op it + // should be. + precheckCmd := exec.CommandContext(ctx, "git", "status", "--porcelain", "--", absFile) + precheckCmd.Dir = repoRoot + precheckOutput, _ := precheckCmd.Output() + precheckLine := strings.TrimRight(string(precheckOutput), "\n") + if len(precheckLine) > 1 && precheckLine[1] == ' ' { + log.Debug("file already fully staged, skipping git add", + "file", file, + "status", precheckLine) + return nil + } + log.Debug("adding file to staging", "file", file, "abs_file", absFile, diff --git a/internal/services/routing/model_selector.go b/internal/routing/model_selector.go similarity index 100% rename from internal/services/routing/model_selector.go rename to internal/routing/model_selector.go diff --git a/internal/services/routing/model_selector_test.go b/internal/routing/model_selector_test.go similarity index 100% rename from internal/services/routing/model_selector_test.go rename to internal/routing/model_selector_test.go From 5639ec6a386718e6ed819f65a0eb5664e60b1679 Mon Sep 17 00:00:00 2001 From: Thomas Vilte Date: Thu, 30 Jul 2026 18:03:18 -0300 Subject: [PATCH 2/2] fix(git): handle already staged deleted files --- internal/git/git_service_test.go | 38 ++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/internal/git/git_service_test.go b/internal/git/git_service_test.go index 0c6c5a7..a059ccd 100644 --- a/internal/git/git_service_test.go +++ b/internal/git/git_service_test.go @@ -524,6 +524,44 @@ func TestAddFileToStaging(t *testing.T) { } }) + t.Run("Already fully staged deleted file is a no-op", func(t *testing.T) { + // Reproduces a real failure: if a file was already staged for + // deletion by the user directly (e.g. "git rm"), it no longer + // exists on disk or in the index diff, so running "git add" on it + // again used to fail with "pathspec did not match any files" + // instead of being the harmless no-op it should be. + tempDir := setupTestRepo(t) + defer cleanupTestRepo(t, tempDir) + + service := NewGitService() + testFile := "already-staged-deletion.txt" + + if err := os.WriteFile(testFile, []byte("test"), 0644); err != nil { + return + } + if err := service.AddFileToStaging(context.Background(), testFile); err != nil { + t.Fatalf("Error al agregar archivo al staging: %v", err) + } + if err := service.CreateCommit(context.Background(), "Commit inicial"); err != nil { + t.Fatalf("Error al crear commit inicial: %v", err) + } + + rmCmd := exec.Command("git", "rm", testFile) + if err := rmCmd.Run(); err != nil { + t.Fatalf("Error al hacer git rm: %v", err) + } + + if err := service.AddFileToStaging(context.Background(), testFile); err != nil { + t.Fatalf("AddFileToStaging debería ser un no-op silencioso, pero devolvió: %v", err) + } + + cmd := exec.Command("git", "diff", "--cached", "--name-status") + output, _ := cmd.Output() + if !strings.Contains(string(output), "D\t"+testFile) { + t.Error("La eliminación previamente stageada se perdió") + } + }) + t.Run("Non-existent file", func(t *testing.T) { tempDir := setupTestRepo(t) defer cleanupTestRepo(t, tempDir)