test: 添加单元测试（config/services/handlers，共 59 个用例）

config/config_test.go

@@ -0,0 +1,134 @@
+package config
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestMySQLDSN(t *testing.T) {
+	cfg := &Config{
+		MySQLUser:     "testuser",
+		MySQLPassword: "testpass",
+		MySQLHost:     "192.168.1.1",
+		MySQLPort:     "3307",
+		MySQLDatabase: "testdb",
+	}
+	got := cfg.MySQLDSN()
+	want := "testuser:testpass@tcp(192.168.1.1:3307)/testdb?charset=utf8mb4&parseTime=True&loc=Local"
+	if got != want {
+		t.Errorf("MySQLDSN() = %q, want %q", got, want)
+	}
+}
+
+func TestMySQLDSN_EmptyPassword(t *testing.T) {
+	cfg := &Config{
+		MySQLUser:     "root",
+		MySQLPassword: "",
+		MySQLHost:     "127.0.0.1",
+		MySQLPort:     "3306",
+		MySQLDatabase: "pr_helper",
+	}
+	got := cfg.MySQLDSN()
+	want := "root:@tcp(127.0.0.1:3306)/pr_helper?charset=utf8mb4&parseTime=True&loc=Local"
+	if got != want {
+		t.Errorf("MySQLDSN() = %q, want %q", got, want)
+	}
+}
+
+func TestReposDir(t *testing.T) {
+	cfg := &Config{DataDir: "/tmp/testdata"}
+	got := cfg.ReposDir()
+	want := filepath.Join("/tmp/testdata", "repos")
+	if got != want {
+		t.Errorf("ReposDir() = %q, want %q", got, want)
+	}
+}
+
+func TestLoad_Defaults(t *testing.T) {
+	// Clear all relevant env vars to test defaults
+	envVars := []string{"PORT", "GIN_MODE", "DATA_DIR", "SESSION_SECRET",
+		"MYSQL_HOST", "MYSQL_PORT", "MYSQL_USER", "MYSQL_PASSWORD", "MYSQL_DATABASE"}
+	origValues := make(map[string]string)
+	for _, k := range envVars {
+		origValues[k] = os.Getenv(k)
+		os.Unsetenv(k)
+	}
+	defer func() {
+		for k, v := range origValues {
+			if v != "" {
+				os.Setenv(k, v)
+			}
+		}
+	}()
+
+	cfg := Load()
+
+	if cfg.Port != "8080" {
+		t.Errorf("Port = %q, want %q", cfg.Port, "8080")
+	}
+	if cfg.GinMode != "debug" {
+		t.Errorf("GinMode = %q, want %q", cfg.GinMode, "debug")
+	}
+	if cfg.DataDir != "data" {
+		t.Errorf("DataDir = %q, want %q", cfg.DataDir, "data")
+	}
+	if cfg.MySQLHost != "127.0.0.1" {
+		t.Errorf("MySQLHost = %q, want %q", cfg.MySQLHost, "127.0.0.1")
+	}
+	if cfg.MySQLPort != "3306" {
+		t.Errorf("MySQLPort = %q, want %q", cfg.MySQLPort, "3306")
+	}
+	if cfg.MySQLUser != "root" {
+		t.Errorf("MySQLUser = %q, want %q", cfg.MySQLUser, "root")
+	}
+	if cfg.MySQLDatabase != "pr_helper" {
+		t.Errorf("MySQLDatabase = %q, want %q", cfg.MySQLDatabase, "pr_helper")
+	}
+	// Session secret should be auto-generated (64 hex chars)
+	if len(cfg.SessionSecret) != 64 {
+		t.Errorf("SessionSecret length = %d, want 64", len(cfg.SessionSecret))
+	}
+}
+
+func TestLoad_WithEnvVars(t *testing.T) {
+	os.Setenv("PORT", "9090")
+	os.Setenv("GIN_MODE", "release")
+	os.Setenv("SESSION_SECRET", "my-secret-key")
+	defer func() {
+		os.Unsetenv("PORT")
+		os.Unsetenv("GIN_MODE")
+		os.Unsetenv("SESSION_SECRET")
+	}()
+
+	cfg := Load()
+
+	if cfg.Port != "9090" {
+		t.Errorf("Port = %q, want %q", cfg.Port, "9090")
+	}
+	if cfg.GinMode != "release" {
+		t.Errorf("GinMode = %q, want %q", cfg.GinMode, "release")
+	}
+	if cfg.SessionSecret != "my-secret-key" {
+		t.Errorf("SessionSecret = %q, want %q", cfg.SessionSecret, "my-secret-key")
+	}
+}
+
+func TestLoad_CreatesDataDirs(t *testing.T) {
+	tmpDir := t.TempDir()
+	dataDir := filepath.Join(tmpDir, "test-data")
+	os.Setenv("DATA_DIR", dataDir)
+	defer os.Unsetenv("DATA_DIR")
+
+	Load()
+
+	// Verify data dir was created
+	if _, err := os.Stat(dataDir); os.IsNotExist(err) {
+		t.Errorf("DataDir %q was not created", dataDir)
+	}
+	// Verify repos subdir was created
+	reposDir := filepath.Join(dataDir, "repos")
+	if _, err := os.Stat(reposDir); os.IsNotExist(err) {
+		t.Errorf("ReposDir %q was not created", reposDir)
+	}
+}

handlers/middleware_test.go

@@ -0,0 +1,371 @@
+package handlers
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/HoHD/PR-Helper/models"
+	"github.com/gin-gonic/gin"
+)
+
+func init() {
+	gin.SetMode(gin.TestMode)
+}
+
+func TestGetCurrentUser_WithUser(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+
+	user := &models.User{
+		ID:        42,
+		Email:     "test@example.com",
+		CreatedAt: time.Now(),
+		UpdatedAt: time.Now(),
+	}
+	c.Set("user", user)
+
+	got := GetCurrentUser(c)
+	if got == nil {
+		t.Fatal("GetCurrentUser() returned nil")
+	}
+	if got.ID != 42 {
+		t.Errorf("ID = %d, want 42", got.ID)
+	}
+	if got.Email != "test@example.com" {
+		t.Errorf("Email = %q, want %q", got.Email, "test@example.com")
+	}
+}
+
+func TestGetCurrentUser_NoUser(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+
+	got := GetCurrentUser(c)
+	if got != nil {
+		t.Errorf("GetCurrentUser() = %v, want nil", got)
+	}
+}
+
+func TestGetCurrentUser_WrongType(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+
+	// Set a non-User value — should panic or return nil
+	defer func() {
+		if r := recover(); r == nil {
+			// If it doesn't panic, GetCurrentUser should still work somehow
+			// This documents the current behavior
+		}
+	}()
+	c.Set("user", "not-a-user")
+
+	// This will panic because of type assertion — that's expected behavior
+	GetCurrentUser(c)
+}
+
+func TestAuthRequired_NoSession_APIRequest(t *testing.T) {
+	// AuthRequired needs a *sql.DB, but for the no-session case
+	// it should respond 401 before hitting the DB.
+	// We can't easily test this without a session store setup,
+	// so we test the path detection logic instead.
+
+	// Test that /api/ prefix is correctly detected
+	apiPaths := []string{"/api/repos", "/api/settings", "/api/repos/1/graph"}
+	pagePaths := []string{"/", "/login", "/register", "/settings"}
+
+	for _, path := range apiPaths {
+		if !isAPIPath(path) {
+			t.Errorf("isAPIPath(%q) = false, want true", path)
+		}
+	}
+	for _, path := range pagePaths {
+		if isAPIPath(path) {
+			t.Errorf("isAPIPath(%q) = true, want false", path)
+		}
+	}
+}
+
+// isAPIPath replicates the path check logic from AuthRequired for testing.
+func isAPIPath(path string) bool {
+	return len(path) > 4 && path[:5] == "/api/"
+}
+
+func TestUserData_WithUser(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+
+	user := &models.User{
+		ID:    1,
+		Email: "user@test.com",
+	}
+	c.Set("user", user)
+
+	data := userData(c)
+	if data["User"] == nil {
+		t.Error("expected User in data")
+	}
+	u := data["User"].(*models.User)
+	if u.ID != 1 {
+		t.Errorf("User.ID = %d, want 1", u.ID)
+	}
+}
+
+func TestUserData_NoUser(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+
+	data := userData(c)
+	if data["User"] != nil {
+		t.Error("expected nil User in data")
+	}
+}
+
+func TestNewSettingsHandler(t *testing.T) {
+	h := NewSettingsHandler(nil)
+	if h == nil {
+		t.Fatal("NewSettingsHandler() returned nil")
+	}
+}
+
+func TestNewPageHandler(t *testing.T) {
+	h := NewPageHandler(nil)
+	if h == nil {
+		t.Fatal("NewPageHandler() returned nil")
+	}
+}
+
+func TestNewReposHandler(t *testing.T) {
+	h := NewReposHandler(nil, "/tmp/repos")
+	if h == nil {
+		t.Fatal("NewReposHandler() returned nil")
+	}
+}
+
+func TestNewGenerateHandler(t *testing.T) {
+	h := NewGenerateHandler(nil)
+	if h == nil {
+		t.Fatal("NewGenerateHandler() returned nil")
+	}
+}
+
+func TestNewReviewHandler(t *testing.T) {
+	h := NewReviewHandler(nil)
+	if h == nil {
+		t.Fatal("NewReviewHandler() returned nil")
+	}
+}
+
+func TestNewAuthHandler(t *testing.T) {
+	h := NewAuthHandler(nil)
+	if h == nil {
+		t.Fatal("NewAuthHandler() returned nil")
+	}
+}
+
+// TestDirSize_Handlers tests the handlers package's own dirSize function.
+func TestDirSize_Handlers(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	size := dirSize(tmpDir)
+	if size != 0 {
+		t.Errorf("dirSize() = %d, want 0", size)
+	}
+}
+
+// TestHandlerListRepos_Unauthorized tests that ListRepos returns 401 without user.
+func TestHandlerListRepos_Unauthorized(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest(http.MethodGet, "/api/repos", nil)
+
+	h := NewReposHandler(nil, "/tmp/repos")
+	h.ListRepos(c)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("status = %d, want %d", w.Code, http.StatusUnauthorized)
+	}
+}
+
+// TestHandlerDeleteRepo_Unauthorized tests that DeleteRepo returns 401 without user.
+func TestHandlerDeleteRepo_Unauthorized(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest(http.MethodDelete, "/api/repos/1", nil)
+
+	h := NewReposHandler(nil, "/tmp/repos")
+	h.DeleteRepo(c)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("status = %d, want %d", w.Code, http.StatusUnauthorized)
+	}
+}
+
+// TestHandlerGetSettings_Unauthorized tests that GetSettings returns 401 without user.
+func TestHandlerGetSettings_Unauthorized(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest(http.MethodGet, "/api/settings", nil)
+
+	h := NewSettingsHandler(nil)
+	h.GetSettings(c)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("status = %d, want %d", w.Code, http.StatusUnauthorized)
+	}
+}
+
+// TestHandlerUpdateSettings_Unauthorized tests that UpdateSettings returns 401 without user.
+func TestHandlerUpdateSettings_Unauthorized(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest(http.MethodPut, "/api/settings", nil)
+
+	h := NewSettingsHandler(nil)
+	h.UpdateSettings(c)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("status = %d, want %d", w.Code, http.StatusUnauthorized)
+	}
+}
+
+// TestHandlerGenerate_Unauthorized tests that Generate returns 401 without user.
+func TestHandlerGenerate_Unauthorized(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest(http.MethodPost, "/api/repos/1/generate", nil)
+
+	h := NewGenerateHandler(nil)
+	h.Generate(c)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("status = %d, want %d", w.Code, http.StatusUnauthorized)
+	}
+}
+
+// TestHandlerReview_Unauthorized tests that Review returns 401 without user.
+func TestHandlerReview_Unauthorized(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest(http.MethodPost, "/api/repos/1/review", nil)
+
+	h := NewReviewHandler(nil)
+	h.Review(c)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("status = %d, want %d", w.Code, http.StatusUnauthorized)
+	}
+}
+
+// TestHandlerSaveNotes_Unauthorized tests that SaveNotes returns 401 without user.
+func TestHandlerSaveNotes_Unauthorized(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest(http.MethodPost, "/api/repos/1/review/notes", nil)
+
+	h := NewReviewHandler(nil)
+	h.SaveNotes(c)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("status = %d, want %d", w.Code, http.StatusUnauthorized)
+	}
+}
+
+// TestHandlerGetNotes_Unauthorized tests that GetNotes returns 401 without user.
+func TestHandlerGetNotes_Unauthorized(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest(http.MethodGet, "/api/repos/1/review/notes", nil)
+
+	h := NewReviewHandler(nil)
+	h.GetNotes(c)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("status = %d, want %d", w.Code, http.StatusUnauthorized)
+	}
+}
+
+// TestHandlerListReviews_Unauthorized tests that ListReviews returns 401 without user.
+func TestHandlerListReviews_Unauthorized(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest(http.MethodGet, "/api/repos/1/review/analyses", nil)
+
+	h := NewReviewHandler(nil)
+	h.ListReviews(c)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("status = %d, want %d", w.Code, http.StatusUnauthorized)
+	}
+}
+
+// TestHandlerGetReview_Unauthorized tests that GetReview returns 401 without user.
+func TestHandlerGetReview_Unauthorized(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest(http.MethodGet, "/api/repos/1/review/analyses/1", nil)
+
+	h := NewReviewHandler(nil)
+	h.GetReview(c)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("status = %d, want %d", w.Code, http.StatusUnauthorized)
+	}
+}
+
+// TestHandlerGetGraph_Unauthorized tests that GetGraph returns 401 without user.
+func TestHandlerGetGraph_Unauthorized(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest(http.MethodGet, "/api/repos/1/graph", nil)
+
+	h := NewReposHandler(nil, "/tmp/repos")
+	h.GetGraph(c)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("status = %d, want %d", w.Code, http.StatusUnauthorized)
+	}
+}
+
+// TestHandlerGetDiff_Unauthorized tests that GetDiff returns 401 without user.
+func TestHandlerGetDiff_Unauthorized(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest(http.MethodGet, "/api/repos/1/diff", nil)
+
+	h := NewReposHandler(nil, "/tmp/repos")
+	h.GetDiff(c)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("status = %d, want %d", w.Code, http.StatusUnauthorized)
+	}
+}
+
+// TestHandlerGetRefs_Unauthorized tests that GetRefs returns 401 without user.
+func TestHandlerGetRefs_Unauthorized(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest(http.MethodGet, "/api/repos/1/refs", nil)
+
+	h := NewReposHandler(nil, "/tmp/repos")
+	h.GetRefs(c)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("status = %d, want %d", w.Code, http.StatusUnauthorized)
+	}
+}
+
+// TestHandlerGetCommits_Unauthorized tests that GetCommits returns 401 without user.
+func TestHandlerGetCommits_Unauthorized(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest(http.MethodGet, "/api/repos/1/commits", nil)
+
+	h := NewReposHandler(nil, "/tmp/repos")
+	h.GetCommits(c)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("status = %d, want %d", w.Code, http.StatusUnauthorized)
+	}
+}

services/cache_test.go

@@ -0,0 +1,207 @@
+package services
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+)
+
+func TestCleanupExpiredRepos(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	// Create "old" repo (modify time in the past)
+	oldDir := filepath.Join(tmpDir, "old-repo")
+	os.MkdirAll(oldDir, 0o755)
+	os.WriteFile(filepath.Join(oldDir, "file.txt"), []byte("old"), 0o644)
+	oldTime := time.Now().AddDate(0, 0, -10) // 10 days ago
+	os.Chtimes(oldDir, oldTime, oldTime)
+
+	// Create "new" repo (recent)
+	newDir := filepath.Join(tmpDir, "new-repo")
+	os.MkdirAll(newDir, 0o755)
+	os.WriteFile(filepath.Join(newDir, "file.txt"), []byte("new"), 0o644)
+
+	result, err := CleanupExpiredRepos(tmpDir, 7)
+	if err != nil {
+		t.Fatalf("CleanupExpiredRepos() error: %v", err)
+	}
+
+	if result.RemovedCount != 1 {
+		t.Errorf("RemovedCount = %d, want 1", result.RemovedCount)
+	}
+	if len(result.RemovedPaths) != 1 || result.RemovedPaths[0] != "old-repo" {
+		t.Errorf("RemovedPaths = %v, want [old-repo]", result.RemovedPaths)
+	}
+	if result.FreedBytes <= 0 {
+		t.Errorf("FreedBytes = %d, want > 0", result.FreedBytes)
+	}
+
+	// Verify new repo still exists
+	if _, err := os.Stat(newDir); os.IsNotExist(err) {
+		t.Error("new-repo should still exist")
+	}
+	// Verify old repo was removed
+	if _, err := os.Stat(oldDir); !os.IsNotExist(err) {
+		t.Error("old-repo should have been removed")
+	}
+}
+
+func TestCleanupExpiredRepos_NonexistentDir(t *testing.T) {
+	result, err := CleanupExpiredRepos("/nonexistent/path/12345", 7)
+	if err != nil {
+		t.Fatalf("CleanupExpiredRepos() error: %v", err)
+	}
+	if result.RemovedCount != 0 {
+		t.Errorf("RemovedCount = %d, want 0", result.RemovedCount)
+	}
+}
+
+func TestCleanupExpiredRepos_NoExpired(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	// Create only recent repos
+	for _, name := range []string{"repo1", "repo2"} {
+		dir := filepath.Join(tmpDir, name)
+		os.MkdirAll(dir, 0o755)
+	}
+
+	result, err := CleanupExpiredRepos(tmpDir, 7)
+	if err != nil {
+		t.Fatalf("CleanupExpiredRepos() error: %v", err)
+	}
+	if result.RemovedCount != 0 {
+		t.Errorf("RemovedCount = %d, want 0", result.RemovedCount)
+	}
+}
+
+func TestCleanupBySize(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	// Create repos with known sizes; set mod times so ordering is deterministic
+	for i, name := range []string{"oldest", "middle", "newest"} {
+		dir := filepath.Join(tmpDir, name)
+		os.MkdirAll(dir, 0o755)
+		// Each repo: ~100 bytes
+		os.WriteFile(filepath.Join(dir, "data.txt"), make([]byte, 100), 0o644)
+		modTime := time.Now().AddDate(0, 0, -10+i) // -10, -9, -8 days
+		os.Chtimes(dir, modTime, modTime)
+		os.Chtimes(filepath.Join(dir, "data.txt"), modTime, modTime)
+	}
+
+	// Set max to 150 bytes — should remove oldest repos until under limit
+	result, err := CleanupBySize(tmpDir, 0) // 0 MB = 0 bytes limit
+	if err != nil {
+		t.Fatalf("CleanupBySize() error: %v", err)
+	}
+
+	// With 0 MB limit, all repos should be removed
+	if result.RemovedCount != 3 {
+		t.Errorf("RemovedCount = %d, want 3", result.RemovedCount)
+	}
+}
+
+func TestCleanupBySize_UnderLimit(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	dir := filepath.Join(tmpDir, "small-repo")
+	os.MkdirAll(dir, 0o755)
+	os.WriteFile(filepath.Join(dir, "data.txt"), []byte("tiny"), 0o644)
+
+	result, err := CleanupBySize(tmpDir, 100) // 100 MB limit
+	if err != nil {
+		t.Fatalf("CleanupBySize() error: %v", err)
+	}
+	if result.RemovedCount != 0 {
+		t.Errorf("RemovedCount = %d, want 0", result.RemovedCount)
+	}
+}
+
+func TestCleanupBySize_NonexistentDir(t *testing.T) {
+	result, err := CleanupBySize("/nonexistent/path/12345", 100)
+	if err != nil {
+		t.Fatalf("CleanupBySize() error: %v", err)
+	}
+	if result.RemovedCount != 0 {
+		t.Errorf("RemovedCount = %d, want 0", result.RemovedCount)
+	}
+}
+
+func TestGetCacheStats(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	// Create 3 repo dirs with files
+	for _, name := range []string{"repo1", "repo2", "repo3"} {
+		dir := filepath.Join(tmpDir, name)
+		os.MkdirAll(dir, 0o755)
+		os.WriteFile(filepath.Join(dir, "data.txt"), make([]byte, 50), 0o644)
+	}
+	// Create a file (not a dir) — should be ignored
+	os.WriteFile(filepath.Join(tmpDir, "not-a-repo.txt"), []byte("ignored"), 0o644)
+
+	count, totalBytes := GetCacheStats(tmpDir)
+	if count != 3 {
+		t.Errorf("count = %d, want 3", count)
+	}
+	if totalBytes < 150 { // 3 * 50 bytes minimum
+		t.Errorf("totalBytes = %d, want >= 150", totalBytes)
+	}
+}
+
+func TestGetCacheStats_EmptyDir(t *testing.T) {
+	tmpDir := t.TempDir()
+	count, totalBytes := GetCacheStats(tmpDir)
+	if count != 0 {
+		t.Errorf("count = %d, want 0", count)
+	}
+	if totalBytes != 0 {
+		t.Errorf("totalBytes = %d, want 0", totalBytes)
+	}
+}
+
+func TestGetCacheStats_NonexistentDir(t *testing.T) {
+	count, totalBytes := GetCacheStats("/nonexistent/path/12345")
+	if count != 0 {
+		t.Errorf("count = %d, want 0", count)
+	}
+	if totalBytes != 0 {
+		t.Errorf("totalBytes = %d, want 0", totalBytes)
+	}
+}
+
+func TestSortReposByModTime(t *testing.T) {
+	now := time.Now()
+	repos := []repoEntry{
+		{name: "newest", modTime: now},
+		{name: "oldest", modTime: now.AddDate(0, 0, -10)},
+		{name: "middle", modTime: now.AddDate(0, 0, -5)},
+	}
+
+	sortReposByModTime(repos)
+
+	if repos[0].name != "oldest" {
+		t.Errorf("repos[0] = %q, want %q", repos[0].name, "oldest")
+	}
+	if repos[1].name != "middle" {
+		t.Errorf("repos[1] = %q, want %q", repos[1].name, "middle")
+	}
+	if repos[2].name != "newest" {
+		t.Errorf("repos[2] = %q, want %q", repos[2].name, "newest")
+	}
+}
+
+func TestSortReposByModTime_Empty(t *testing.T) {
+	repos := []repoEntry{}
+	sortReposByModTime(repos) // should not panic
+	if len(repos) != 0 {
+		t.Errorf("expected empty slice")
+	}
+}
+
+func TestSortReposByModTime_Single(t *testing.T) {
+	repos := []repoEntry{{name: "only", modTime: time.Now()}}
+	sortReposByModTime(repos) // should not panic
+	if repos[0].name != "only" {
+		t.Errorf("repos[0] = %q, want %q", repos[0].name, "only")
+	}
+}

services/git_test.go

@@ -0,0 +1,421 @@
+package services
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/go-git/go-git/v5"
+	"github.com/go-git/go-git/v5/plumbing/object"
+)
+
+// initTestRepo creates a temp git repo with two commits on main branch.
+// Returns the repo path and the two commit hashes (first, second).
+func initTestRepo(t *testing.T) (string, string, string) {
+	t.Helper()
+	dir := t.TempDir()
+
+	repo, err := git.PlainInit(dir, false)
+	if err != nil {
+		t.Fatalf("PlainInit: %v", err)
+	}
+
+	w, err := repo.Worktree()
+	if err != nil {
+		t.Fatalf("Worktree: %v", err)
+	}
+
+	// Commit 1: create file
+	filePath := filepath.Join(dir, "hello.txt")
+	if err := os.WriteFile(filePath, []byte("hello world\n"), 0o644); err != nil {
+		t.Fatalf("WriteFile: %v", err)
+	}
+	w.Add("hello.txt")
+
+	hash1, err := w.Commit("initial commit", &git.CommitOptions{
+		Author: &object.Signature{
+			Name:  "Test",
+			Email: "test@example.com",
+			When:  time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC),
+		},
+	})
+	if err != nil {
+		t.Fatalf("Commit 1: %v", err)
+	}
+
+	// Commit 2: modify file
+	if err := os.WriteFile(filePath, []byte("hello world\nmodified\n"), 0o644); err != nil {
+		t.Fatalf("WriteFile: %v", err)
+	}
+	w.Add("hello.txt")
+
+	hash2, err := w.Commit("second commit", &git.CommitOptions{
+		Author: &object.Signature{
+			Name:  "Test",
+			Email: "test@example.com",
+			When:  time.Date(2024, 1, 2, 0, 0, 0, 0, time.UTC),
+		},
+	})
+	if err != nil {
+		t.Fatalf("Commit 2: %v", err)
+	}
+
+	return dir, hash1.String(), hash2.String()
+}
+
+func TestOpenRepo(t *testing.T) {
+	dir, _, _ := initTestRepo(t)
+
+	repo, err := OpenRepo(dir)
+	if err != nil {
+		t.Fatalf("OpenRepo() error: %v", err)
+	}
+	if repo == nil {
+		t.Fatal("OpenRepo() returned nil repo")
+	}
+}
+
+func TestOpenRepo_InvalidPath(t *testing.T) {
+	_, err := OpenRepo("/nonexistent/path/12345")
+	if err == nil {
+		t.Error("OpenRepo() expected error for invalid path")
+	}
+}
+
+func TestGetRefs(t *testing.T) {
+	dir, _, _ := initTestRepo(t)
+	repo, err := OpenRepo(dir)
+	if err != nil {
+		t.Fatalf("OpenRepo: %v", err)
+	}
+
+	refs, err := GetRefs(repo)
+	if err != nil {
+		t.Fatalf("GetRefs() error: %v", err)
+	}
+
+	// Should have at least one branch (main or master depending on git config)
+	found := false
+	for _, r := range refs {
+		if (r.Name == "main" || r.Name == "master") && !r.IsTag {
+			found = true
+			if r.Hash == "" {
+				t.Error("branch ref has empty hash")
+			}
+			if !r.IsHead {
+				t.Error("default branch should be HEAD")
+			}
+		}
+	}
+	if !found {
+		t.Errorf("expected default branch in refs, got %v", refs)
+	}
+}
+
+func TestGetGraph(t *testing.T) {
+	dir, hash1, hash2 := initTestRepo(t)
+	repo, err := OpenRepo(dir)
+	if err != nil {
+		t.Fatalf("OpenRepo: %v", err)
+	}
+
+	graph, err := GetGraph(repo, 100)
+	if err != nil {
+		t.Fatalf("GetGraph() error: %v", err)
+	}
+
+	if len(graph.Commits) != 2 {
+		t.Fatalf("commits count = %d, want 2", len(graph.Commits))
+	}
+
+	// The first commit in the graph should be the latest (hash2)
+	if graph.Commits[0].Hash != hash2 {
+		t.Errorf("commits[0].Hash = %q, want %q", graph.Commits[0].Hash, hash2)
+	}
+	if graph.Commits[0].Message != "second commit" {
+		t.Errorf("commits[0].Message = %q, want %q", graph.Commits[0].Message, "second commit")
+	}
+
+	// Second should be hash1
+	if graph.Commits[1].Hash != hash1 {
+		t.Errorf("commits[1].Hash = %q, want %q", graph.Commits[1].Hash, hash1)
+	}
+
+	// Should have one edge (hash2 -> hash1)
+	if len(graph.Edges) != 1 {
+		t.Fatalf("edges count = %d, want 1", len(graph.Edges))
+	}
+	if graph.Edges[0].Source != hash2 || graph.Edges[0].Target != hash1 {
+		t.Errorf("edge = {%s -> %s}, want {%s -> %s}",
+			graph.Edges[0].Source, graph.Edges[0].Target, hash2, hash1)
+	}
+
+	// Should have at least one ref
+	if len(graph.Refs) < 1 {
+		t.Error("expected at least 1 ref in graph")
+	}
+}
+
+func TestGetGraph_MaxCommits(t *testing.T) {
+	dir, _, _ := initTestRepo(t)
+	repo, err := OpenRepo(dir)
+	if err != nil {
+		t.Fatalf("OpenRepo: %v", err)
+	}
+
+	graph, err := GetGraph(repo, 1) // limit to 1 commit
+	if err != nil {
+		t.Fatalf("GetGraph() error: %v", err)
+	}
+
+	if len(graph.Commits) != 1 {
+		t.Errorf("commits count = %d, want 1", len(graph.Commits))
+	}
+}
+
+func TestGetDiff(t *testing.T) {
+	dir, hash1, hash2 := initTestRepo(t)
+	repo, err := OpenRepo(dir)
+	if err != nil {
+		t.Fatalf("OpenRepo: %v", err)
+	}
+
+	diff, err := GetDiff(repo, hash1, hash2)
+	if err != nil {
+		t.Fatalf("GetDiff() error: %v", err)
+	}
+
+	if diff == "" {
+		t.Error("expected non-empty diff")
+	}
+	// The diff should contain the added line
+	if !contains(diff, "modified") {
+		t.Errorf("diff should contain 'modified', got: %s", diff)
+	}
+}
+
+func TestGetDiffFiles(t *testing.T) {
+	dir, hash1, hash2 := initTestRepo(t)
+	repo, err := OpenRepo(dir)
+	if err != nil {
+		t.Fatalf("OpenRepo: %v", err)
+	}
+
+	files, err := GetDiffFiles(repo, hash1, hash2)
+	if err != nil {
+		t.Fatalf("GetDiffFiles() error: %v", err)
+	}
+
+	if len(files) != 1 {
+		t.Fatalf("files count = %d, want 1", len(files))
+	}
+	if files[0].Filename != "hello.txt" {
+		t.Errorf("filename = %q, want %q", files[0].Filename, "hello.txt")
+	}
+	if files[0].Patch == "" {
+		t.Error("expected non-empty patch")
+	}
+}
+
+func TestGetCommitLog(t *testing.T) {
+	dir, hash1, hash2 := initTestRepo(t)
+	repo, err := OpenRepo(dir)
+	if err != nil {
+		t.Fatalf("OpenRepo: %v", err)
+	}
+
+	commits, err := GetCommitLog(repo, "HEAD", 100)
+	if err != nil {
+		t.Fatalf("GetCommitLog() error: %v", err)
+	}
+
+	if len(commits) != 2 {
+		t.Fatalf("commits count = %d, want 2", len(commits))
+	}
+
+	// First commit should be the latest
+	if commits[0].Hash != hash2 {
+		t.Errorf("commits[0].Hash = %q, want %q", commits[0].Hash, hash2)
+	}
+	if commits[0].Author != "Test" {
+		t.Errorf("commits[0].Author = %q, want %q", commits[0].Author, "Test")
+	}
+	if commits[0].Email != "test@example.com" {
+		t.Errorf("commits[0].Email = %q, want %q", commits[0].Email, "test@example.com")
+	}
+
+	if commits[1].Hash != hash1 {
+		t.Errorf("commits[1].Hash = %q, want %q", commits[1].Hash, hash1)
+	}
+}
+
+func TestGetCommitLog_Limit(t *testing.T) {
+	dir, _, _ := initTestRepo(t)
+	repo, err := OpenRepo(dir)
+	if err != nil {
+		t.Fatalf("OpenRepo: %v", err)
+	}
+
+	commits, err := GetCommitLog(repo, "HEAD", 1)
+	if err != nil {
+		t.Fatalf("GetCommitLog() error: %v", err)
+	}
+
+	if len(commits) != 1 {
+		t.Errorf("commits count = %d, want 1", len(commits))
+	}
+}
+
+func TestGetDefaultBranch(t *testing.T) {
+	dir, _, _ := initTestRepo(t)
+
+	branch, err := GetDefaultBranch(dir)
+	if err != nil {
+		t.Fatalf("GetDefaultBranch() error: %v", err)
+	}
+
+	if branch != "main" && branch != "master" {
+		t.Errorf("GetDefaultBranch() = %q, want %q or %q", branch, "main", "master")
+	}
+}
+
+func TestGetDefaultBranch_InvalidPath(t *testing.T) {
+	_, err := GetDefaultBranch("/nonexistent/path/12345")
+	if err == nil {
+		t.Error("expected error for invalid path")
+	}
+}
+
+func TestGetRepoBranches(t *testing.T) {
+	dir, _, _ := initTestRepo(t)
+
+	branches, err := GetRepoBranches(dir)
+	if err != nil {
+		t.Fatalf("GetRepoBranches() error: %v", err)
+	}
+
+	found := false
+	for _, b := range branches {
+		if b == "main" || b == "master" {
+			found = true
+		}
+	}
+	if !found {
+		t.Errorf("expected default branch in branches, got %v", branches)
+	}
+}
+
+func TestGetRepoTags(t *testing.T) {
+	dir, _, _ := initTestRepo(t)
+
+	tags, err := GetRepoTags(dir)
+	if err != nil {
+		t.Fatalf("GetRepoTags() error: %v", err)
+	}
+
+	// No tags were created, should be empty
+	if len(tags) != 0 {
+		t.Errorf("expected 0 tags, got %v", tags)
+	}
+}
+
+func TestDirSize(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	// Create files with known sizes
+	os.WriteFile(filepath.Join(tmpDir, "a.txt"), make([]byte, 100), 0o644)
+	os.WriteFile(filepath.Join(tmpDir, "b.txt"), make([]byte, 200), 0o644)
+
+	size := dirSize(tmpDir)
+	if size < 300 {
+		t.Errorf("dirSize() = %d, want >= 300", size)
+	}
+}
+
+func TestDirSize_Empty(t *testing.T) {
+	tmpDir := t.TempDir()
+	size := dirSize(tmpDir)
+	if size != 0 {
+		t.Errorf("dirSize() = %d, want 0", size)
+	}
+}
+
+func TestDirSize_Nonexistent(t *testing.T) {
+	size := dirSize("/nonexistent/path/12345")
+	if size != 0 {
+		t.Errorf("dirSize() = %d, want 0", size)
+	}
+}
+
+func TestDirSize_NestedFiles(t *testing.T) {
+	tmpDir := t.TempDir()
+	subDir := filepath.Join(tmpDir, "sub")
+	os.MkdirAll(subDir, 0o755)
+	os.WriteFile(filepath.Join(tmpDir, "root.txt"), make([]byte, 50), 0o644)
+	os.WriteFile(filepath.Join(subDir, "nested.txt"), make([]byte, 75), 0o644)
+
+	size := dirSize(tmpDir)
+	if size < 125 {
+		t.Errorf("dirSize() = %d, want >= 125", size)
+	}
+}
+
+func TestCompareCommits(t *testing.T) {
+	dir, hash1, hash2 := initTestRepo(t)
+
+	diff, err := CompareCommits(dir, hash1, hash2)
+	if err != nil {
+		t.Fatalf("CompareCommits() error: %v", err)
+	}
+	if diff == "" {
+		t.Error("expected non-empty diff")
+	}
+}
+
+func TestCompareCommitsFiles(t *testing.T) {
+	dir, hash1, hash2 := initTestRepo(t)
+
+	files, err := CompareCommitsFiles(dir, hash1, hash2)
+	if err != nil {
+		t.Fatalf("CompareCommitsFiles() error: %v", err)
+	}
+	if len(files) != 1 {
+		t.Fatalf("files count = %d, want 1", len(files))
+	}
+	if files[0].Filename != "hello.txt" {
+		t.Errorf("filename = %q, want %q", files[0].Filename, "hello.txt")
+	}
+}
+
+func TestGetBranchCommits(t *testing.T) {
+	dir, _, _ := initTestRepo(t)
+
+	// Use the actual default branch name (main or master depending on git config)
+	branch, err := GetDefaultBranch(dir)
+	if err != nil {
+		t.Fatalf("GetDefaultBranch: %v", err)
+	}
+
+	commits, err := GetBranchCommits(dir, branch, 100)
+	if err != nil {
+		t.Fatalf("GetBranchCommits() error: %v", err)
+	}
+	if len(commits) != 2 {
+		t.Errorf("commits count = %d, want 2", len(commits))
+	}
+}
+
+// contains is a simple helper to check substring presence.
+func contains(s, substr string) bool {
+	return len(s) >= len(substr) && searchString(s, substr)
+}
+
+func searchString(s, substr string) bool {
+	for i := 0; i <= len(s)-len(substr); i++ {
+		if s[i:i+len(substr)] == substr {
+			return true
+		}
+	}
+	return false
+}

services/llm_test.go

@@ -0,0 +1,109 @@
+package services
+
+import "testing"
+
+func TestExtractJSON_FromCodeBlock(t *testing.T) {
+	input := "Here is the result:\n```json\n{\"key\": \"value\"}\n```\nDone."
+	got := extractJSON(input)
+	want := "{\"key\": \"value\"}"
+	if got != want {
+		t.Errorf("extractJSON() = %q, want %q", got, want)
+	}
+}
+
+func TestExtractJSON_FromPlainCodeBlock(t *testing.T) {
+	input := "Result:\n```\n[{\"a\": 1}]\n```\nEnd."
+	got := extractJSON(input)
+	want := "[{\"a\": 1}]"
+	if got != want {
+		t.Errorf("extractJSON() = %q, want %q", got, want)
+	}
+}
+
+func TestExtractJSON_RawObject(t *testing.T) {
+	input := "Some text before {\"score\": 8, \"overall\": \"good\"} after text"
+	got := extractJSON(input)
+	want := "{\"score\": 8, \"overall\": \"good\"}"
+	if got != want {
+		t.Errorf("extractJSON() = %q, want %q", got, want)
+	}
+}
+
+func TestExtractJSON_RawArray(t *testing.T) {
+	input := "Output: [{\"severity\": \"info\", \"description\": \"test\"}] end"
+	got := extractJSON(input)
+	want := "[{\"severity\": \"info\", \"description\": \"test\"}]"
+	if got != want {
+		t.Errorf("extractJSON() = %q, want %q", got, want)
+	}
+}
+
+func TestExtractJSON_NestedBrackets(t *testing.T) {
+	input := `{"outer": {"inner": [1, 2, 3]}}`
+	got := extractJSON(input)
+	if got != input {
+		t.Errorf("extractJSON() = %q, want %q", got, input)
+	}
+}
+
+func TestExtractJSON_NestedBracketsInText(t *testing.T) {
+	input := `prefix {"arr": [{"k": "v"}]} suffix`
+	got := extractJSON(input)
+	want := `{"arr": [{"k": "v"}]}`
+	if got != want {
+		t.Errorf("extractJSON() = %q, want %q", got, want)
+	}
+}
+
+func TestExtractJSON_NoJSON(t *testing.T) {
+	input := "This is plain text with no JSON at all"
+	got := extractJSON(input)
+	if got != input {
+		t.Errorf("extractJSON() = %q, want %q (same as input)", got, input)
+	}
+}
+
+func TestExtractJSON_EmptyString(t *testing.T) {
+	got := extractJSON("")
+	if got != "" {
+		t.Errorf("extractJSON(\"\") = %q, want %q", got, "")
+	}
+}
+
+func TestExtractJSON_CodeBlockWithLanguageAndNewline(t *testing.T) {
+	input := "```json\n[\n  {\"a\": 1},\n  {\"b\": 2}\n]\n```"
+	got := extractJSON(input)
+	want := "[\n  {\"a\": 1},\n  {\"b\": 2}\n]"
+	if got != want {
+		t.Errorf("extractJSON() = %q, want %q", got, want)
+	}
+}
+
+func TestExtractJSON_ArrayBeforeObject(t *testing.T) {
+	// When [ comes before { , array should be extracted
+	input := "[1, 2, 3] and {\"key\": \"val\"}"
+	got := extractJSON(input)
+	want := "[1, 2, 3]"
+	if got != want {
+		t.Errorf("extractJSON() = %q, want %q", got, want)
+	}
+}
+
+func TestExtractJSON_ObjectBeforeArray(t *testing.T) {
+	input := "{\"key\": \"val\"} and [1, 2, 3]"
+	got := extractJSON(input)
+	want := "{\"key\": \"val\"}"
+	if got != want {
+		t.Errorf("extractJSON() = %q, want %q", got, want)
+	}
+}
+
+func TestExtractJSON_MultipleCodeBlocks(t *testing.T) {
+	// Should extract from the first code block
+	input := "```json\n{\"first\": true}\n```\n\nSome text\n\n```json\n{\"second\": true}\n```"
+	got := extractJSON(input)
+	want := "{\"first\": true}"
+	if got != want {
+		t.Errorf("extractJSON() = %q, want %q", got, want)
+	}
+}

services/review_test.go

@@ -0,0 +1,107 @@
+package services
+
+import "testing"
+
+func TestCountDiffLines(t *testing.T) {
+	tests := []struct {
+		name  string
+		patch string
+		want  int
+	}{
+		{
+			name: "standard diff with additions and deletions",
+			patch: `--- a/file.go
++++ b/file.go
+@@ -1,5 +1,6 @@
+ package main
++import "fmt"
++import "os"
+-func old() {}
++func new() {}
+ func keep() {}`,
+			want: 4, // +import, +import, -func, +func
+		},
+		{
+			name:  "only additions",
+			patch: "+++ b/file.go\n@@ -0,0 +1,3 @@\n+line1\n+line2\n+line3",
+			want:  3,
+		},
+		{
+			name:  "only deletions",
+			patch: "--- a/file.go\n@@ -1,3 +0,0 @@\n-line1\n-line2\n-line3",
+			want:  3,
+		},
+		{
+			name:  "empty patch",
+			patch: "",
+			want:  0,
+		},
+		{
+			name: "context only lines (no +/- prefix)",
+			patch: `@@ -1,3 +1,3 @@
+ func unchanged() {}
+ 	return nil
+ }`,
+			want: 0,
+		},
+		{
+			name: "header lines excluded",
+			patch: `--- a/old.go
++++ b/new.go
+@@ -1 +1 @@
+-old
++new`,
+			want: 2, // -old and +new (headers excluded)
+		},
+		{
+			name:  "mixed content with no changes",
+			patch: "just some\nplain text\nlines",
+			want:  0,
+		},
+		{
+			name:  "single addition",
+			patch: "+added line",
+			want:  1,
+		},
+		{
+			name:  "single deletion",
+			patch: "-removed line",
+			want:  1,
+		},
+		{
+			name:  "line starting with plus in content context",
+			patch: " +not a change, just context\n+actual addition",
+			want:  1, // only the line starting with + at position 0
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := countDiffLines(tt.patch)
+			if got != tt.want {
+				t.Errorf("countDiffLines() = %d, want %d", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestCountDiffLines_MultiFileDiff(t *testing.T) {
+	patch := `--- a/file1.go
++++ b/file1.go
+@@ -1,3 +1,4 @@
+ package main
++import "fmt"
+ func main() {
++	fmt.Println("hello")
+ }
+--- a/file2.go
++++ b/file2.go
+@@ -1,2 +1,2 @@
+-old line
++new line`
+	got := countDiffLines(patch)
+	want := 4 // +import, +fmt.Println, -old line, +new line
+	if got != want {
+		t.Errorf("countDiffLines() = %d, want %d", got, want)
+	}
+}