diff --git a/internal/brew/brew.go b/internal/brew/brew.go index b89c55a..addd121 100644 --- a/internal/brew/brew.go +++ b/internal/brew/brew.go @@ -16,8 +16,6 @@ import ( "github.com/openbootdotdev/openboot/internal/ui" ) -const maxWorkers = 1 - type OutdatedPackage struct { Name string Current string @@ -200,6 +198,31 @@ type installResult struct { errMsg string } +var ( + getInstalledPackagesFn = GetInstalledPackages + preInstallChecksFn = PreInstallChecks + + runBrewInstallBatchFn = func(args ...string) (string, error) { + cmd := brewInstallCmd(args...) + output, err := cmd.CombinedOutput() + return string(output), err + } + + runBrewInstallBatchWithTTYFn = func(args ...string) (string, error) { + cmd := brewInstallCmd(args...) + tty, opened := system.OpenTTY() + if opened { + cmd.Stdin = tty + defer tty.Close() + } + output, err := cmd.CombinedOutput() + return string(output), err + } + + installFormulaWithErrorFn = installFormulaWithError + installSmartCaskWithErrorFn = installSmartCaskWithError +) + func InstallWithProgress(cliPkgs, caskPkgs []string, dryRun bool) (installedFormulae []string, installedCasks []string, err error) { total := len(cliPkgs) + len(caskPkgs) if total == 0 { @@ -208,16 +231,16 @@ func InstallWithProgress(cliPkgs, caskPkgs []string, dryRun bool) (installedForm if dryRun { ui.Info("Would install packages:") - for _, p := range cliPkgs { - fmt.Printf(" brew install %s\n", p) + if len(cliPkgs) > 0 { + fmt.Printf(" brew install %s\n", strings.Join(cliPkgs, " ")) } - for _, p := range caskPkgs { - fmt.Printf(" brew install --cask %s\n", p) + if len(caskPkgs) > 0 { + fmt.Printf(" brew install --cask %s\n", strings.Join(caskPkgs, " ")) } return nil, nil, nil } - alreadyFormulae, alreadyCasks, checkErr := GetInstalledPackages() + alreadyFormulae, alreadyCasks, checkErr := getInstalledPackagesFn() if checkErr != nil { return nil, nil, fmt.Errorf("list installed packages: %w", checkErr) } @@ -250,7 +273,7 @@ func InstallWithProgress(cliPkgs, caskPkgs []string, dryRun bool) (installedForm return installedFormulae, installedCasks, nil } - if preErr := PreInstallChecks(len(newCli) + len(newCask)); preErr != nil { + if preErr := preInstallChecksFn(len(newCli) + len(newCask)); preErr != nil { return installedFormulae, installedCasks, preErr } @@ -261,37 +284,81 @@ func InstallWithProgress(cliPkgs, caskPkgs []string, dryRun bool) (installedForm var allFailed []failedJob if len(newCli) > 0 { - failed := runParallelInstallWithProgress(newCli, progress) - failedSet := make(map[string]bool, len(failed)) - for _, f := range failed { - failedSet[f.name] = true - } - for _, p := range newCli { - if !failedSet[p] { - installedFormulae = append(installedFormulae, p) + progress.PrintLine(" Installing %d CLI packages via brew install...", len(newCli)) + progress.PauseForInteractive() + + args := append([]string{"install"}, newCli...) + cmdOutputStr, cmdErr := runBrewInstallBatchFn(args...) + progress.ResumeAfterInteractive() + + // Re-check installed packages to determine actual success + postFormulae, _, postErr := getInstalledPackagesFn() + if postErr != nil { + // Fallback: use command error to determine status + for _, pkg := range newCli { + progress.IncrementWithStatus(cmdErr == nil) + if cmdErr == nil { + installedFormulae = append(installedFormulae, pkg) + } else { + allFailed = append(allFailed, failedJob{ + installJob: installJob{name: pkg, isCask: false}, + errMsg: parseBrewError(cmdOutputStr), + }) + } + } + } else { + // Check each package individually + for _, pkg := range newCli { + isInstalled := postFormulae[pkg] + progress.IncrementWithStatus(isInstalled) + if isInstalled { + installedFormulae = append(installedFormulae, pkg) + } else { + allFailed = append(allFailed, failedJob{ + installJob: installJob{name: pkg, isCask: false}, + errMsg: extractPackageError(cmdOutputStr, pkg), + }) + } } } - allFailed = append(allFailed, failed...) } if len(newCask) > 0 { - for _, pkg := range newCask { - progress.SetCurrent(pkg) - progress.PrintLine(" Installing %s...", pkg) - start := time.Now() - errMsg := installCaskWithProgress(pkg, progress) - elapsed := time.Since(start) - progress.IncrementWithStatus(errMsg == "") - duration := ui.FormatDuration(elapsed) - if errMsg == "" { - progress.PrintLine(" %s %s", ui.Green("✔ "+pkg), ui.Cyan("("+duration+")")) - installedCasks = append(installedCasks, pkg) - } else { - progress.PrintLine(" %s %s", ui.Red("✗ "+pkg+" ("+errMsg+")"), ui.Cyan("("+duration+")")) - allFailed = append(allFailed, failedJob{ - installJob: installJob{name: pkg, isCask: true}, - errMsg: errMsg, - }) + progress.PrintLine(" Installing %d GUI apps via brew install --cask...", len(newCask)) + progress.PauseForInteractive() + + args := append([]string{"install", "--cask"}, newCask...) + cmdOutputStr, cmdErr := runBrewInstallBatchWithTTYFn(args...) + progress.ResumeAfterInteractive() + + // Re-check installed casks to determine actual success + _, postCasks, postErr := getInstalledPackagesFn() + if postErr != nil { + // Fallback: use command error to determine status + for _, pkg := range newCask { + progress.IncrementWithStatus(cmdErr == nil) + if cmdErr == nil { + installedCasks = append(installedCasks, pkg) + } else { + allFailed = append(allFailed, failedJob{ + installJob: installJob{name: pkg, isCask: true}, + errMsg: parseBrewError(cmdOutputStr), + }) + } + } + } else { + // Check each cask individually + for _, pkg := range newCask { + isInstalled := postCasks[pkg] + progress.IncrementWithStatus(isInstalled) + if isInstalled { + installedCasks = append(installedCasks, pkg) + } else { + allFailed = append(allFailed, failedJob{ + installJob: installJob{name: pkg, isCask: true}, + errMsg: extractPackageError(cmdOutputStr, pkg), + }) + } } } } @@ -304,9 +371,9 @@ func InstallWithProgress(cliPkgs, caskPkgs []string, dryRun bool) (installedForm for _, f := range allFailed { var errMsg string if f.isCask { - errMsg = installSmartCaskWithError(f.name) + errMsg = installSmartCaskWithErrorFn(f.name) } else { - errMsg = installFormulaWithError(f.name) + errMsg = installFormulaWithErrorFn(f.name) } if errMsg == "" { fmt.Printf(" ✔ %s (retry succeeded)\n", f.name) @@ -366,101 +433,6 @@ type failedJob struct { errMsg string } -func runParallelInstallWithProgress(pkgs []string, progress *ui.StickyProgress) []failedJob { - if len(pkgs) == 0 { - return nil - } - - jobs := make([]installJob, 0, len(pkgs)) - for _, pkg := range pkgs { - jobs = append(jobs, installJob{name: pkg, isCask: false}) - } - - jobChan := make(chan installJob, len(jobs)) - results := make(chan installResult, len(jobs)) - - var wg sync.WaitGroup - workers := maxWorkers - if len(jobs) < workers { - workers = len(jobs) - } - - for i := 0; i < workers; i++ { - wg.Add(1) - go func() { - defer wg.Done() - for job := range jobChan { - progress.SetCurrent(job.name) - start := time.Now() - errMsg := installFormulaWithError(job.name) - elapsed := time.Since(start) - progress.IncrementWithStatus(errMsg == "") - duration := ui.FormatDuration(elapsed) - if errMsg == "" { - progress.PrintLine(" %s %s", ui.Green("✔ "+job.name), ui.Cyan("("+duration+")")) - } else { - progress.PrintLine(" %s %s", ui.Red("✗ "+job.name+" ("+errMsg+")"), ui.Cyan("("+duration+")")) - } - results <- installResult{name: job.name, failed: errMsg != "", isCask: job.isCask, errMsg: errMsg} - } - }() - } - - go func() { - for _, job := range jobs { - jobChan <- job - } - close(jobChan) - }() - - go func() { - wg.Wait() - close(results) - }() - - var failed []failedJob - for result := range results { - if result.failed { - failed = append(failed, failedJob{ - installJob: installJob{name: result.name, isCask: result.isCask}, - errMsg: result.errMsg, - }) - } - } - - return failed -} - -func installCaskWithProgress(pkg string, progress *ui.StickyProgress) string { - progress.PauseForInteractive() - - cmd := brewInstallCmd("install", "--cask", pkg) - tty, opened := system.OpenTTY() - if opened { - defer tty.Close() - } - cmd.Stdin = tty - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - err := cmd.Run() - - progress.ResumeAfterInteractive() - - if err != nil { - return "install failed" - } - return "" -} - -func printBrewOutput(output string, progress *ui.StickyProgress) { - for _, line := range strings.Split(strings.TrimSpace(output), "\n") { - line = strings.TrimSpace(line) - if line != "" { - progress.PrintLine(" %s", line) - } - } -} - func brewInstallCmd(args ...string) *exec.Cmd { cmd := exec.Command("brew", args...) cmd.Env = append(os.Environ(), "HOMEBREW_NO_AUTO_UPDATE=1") @@ -598,6 +570,31 @@ func parseBrewError(output string) string { } } +// extractPackageError tries to find an error message specific to pkg in the +// combined brew output. Falls back to parseBrewError on the full output. +func extractPackageError(output, pkg string) string { + // Scan for lines mentioning the package name near an error indicator. + lowerPkg := strings.ToLower(pkg) + for _, line := range strings.Split(output, "\n") { + lower := strings.ToLower(line) + if strings.Contains(lower, lowerPkg) && strings.Contains(lower, "error") { + line = strings.TrimSpace(line) + if len(line) > 80 { + return line[:77] + "..." + } + return line + } + } + + // No package-specific line found; fall back to the general parser but + // indicate the package was not installed after the batch attempt. + parsed := parseBrewError(output) + if parsed == "unknown error" { + return "not installed after batch attempt" + } + return parsed +} + func Uninstall(packages []string, dryRun bool) error { if len(packages) == 0 { return nil @@ -857,3 +854,26 @@ func PreInstallChecks(packageCount int) error { return nil } + +// ResolveFormulaName resolves a formula alias to its canonical name. +// This handles cases like "postgresql" → "postgresql@18" or "kubectl" → "kubernetes-cli". +// Returns the original name if resolution fails. +func ResolveFormulaName(name string) string { + cmd := exec.Command("brew", "info", "--json", name) + output, err := cmd.Output() + if err != nil { + return name + } + + var result []struct { + Name string `json:"name"` + } + if err := json.Unmarshal(output, &result); err != nil { + return name + } + + if len(result) > 0 && result[0].Name != "" { + return result[0].Name + } + return name +} diff --git a/internal/brew/brew_test.go b/internal/brew/brew_test.go index e7b0076..fa759a1 100644 --- a/internal/brew/brew_test.go +++ b/internal/brew/brew_test.go @@ -1,10 +1,13 @@ package brew import ( + "io" + "os" "strings" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestParseBrewError(t *testing.T) { @@ -175,6 +178,133 @@ func TestInstallWithProgress_DryRunReturnsNoInstalledPackages(t *testing.T) { assert.Empty(t, casks, "dry-run should not report casks as installed") } +func TestInstallWithProgress_ClassifiesFromPostInstallLookup(t *testing.T) { + oldGetInstalledPackages := getInstalledPackagesFn + oldPreInstallChecks := preInstallChecksFn + oldRunBrewInstallBatch := runBrewInstallBatchFn + oldInstallFormulaWithError := installFormulaWithErrorFn + oldInstallSmartCaskWithError := installSmartCaskWithErrorFn + + t.Cleanup(func() { + getInstalledPackagesFn = oldGetInstalledPackages + preInstallChecksFn = oldPreInstallChecks + runBrewInstallBatchFn = oldRunBrewInstallBatch + installFormulaWithErrorFn = oldInstallFormulaWithError + installSmartCaskWithErrorFn = oldInstallSmartCaskWithError + }) + + calls := 0 + var retryCalls []string + getInstalledPackagesFn = func() (map[string]bool, map[string]bool, error) { + calls++ + switch calls { + case 1: + return map[string]bool{}, map[string]bool{}, nil + case 2: + return map[string]bool{"git": true}, map[string]bool{}, nil + default: + return nil, nil, assert.AnError + } + } + preInstallChecksFn = func(int) error { return nil } + runBrewInstallBatchFn = func(args ...string) (string, error) { + assert.Equal(t, []string{"install", "git", "curl"}, args) + return "", nil + } + installFormulaWithErrorFn = func(pkg string) string { + retryCalls = append(retryCalls, pkg) + return "" + } + installSmartCaskWithErrorFn = func(pkg string) string { + t.Fatalf("unexpected cask retry for %s", pkg) + return "" + } + + oldStdout := os.Stdout + r, w, err := os.Pipe() + require.NoError(t, err) + os.Stdout = w + + formulae, casks, runErr := InstallWithProgress([]string{"git", "curl"}, nil, false) + + w.Close() + os.Stdout = oldStdout + + outputBytes, copyErr := io.ReadAll(r) + require.NoError(t, copyErr) + _ = outputBytes + + assert.NoError(t, runErr) + assert.ElementsMatch(t, []string{"git", "curl"}, formulae) + assert.Empty(t, casks) + assert.Equal(t, []string{"curl"}, retryCalls) + assert.Equal(t, 2, calls) +} + +func TestInstallWithProgress_FallsBackWhenPostInstallLookupFails(t *testing.T) { + oldGetInstalledPackages := getInstalledPackagesFn + oldPreInstallChecks := preInstallChecksFn + oldRunBrewInstallBatch := runBrewInstallBatchFn + oldInstallFormulaWithError := installFormulaWithErrorFn + oldInstallSmartCaskWithError := installSmartCaskWithErrorFn + + t.Cleanup(func() { + getInstalledPackagesFn = oldGetInstalledPackages + preInstallChecksFn = oldPreInstallChecks + runBrewInstallBatchFn = oldRunBrewInstallBatch + installFormulaWithErrorFn = oldInstallFormulaWithError + installSmartCaskWithErrorFn = oldInstallSmartCaskWithError + }) + + calls := 0 + installFormulaRetries := 0 + getInstalledPackagesFn = func() (map[string]bool, map[string]bool, error) { + calls++ + switch calls { + case 1: + return map[string]bool{}, map[string]bool{}, nil + case 2: + return nil, nil, assert.AnError + default: + return nil, nil, assert.AnError + } + } + preInstallChecksFn = func(int) error { return nil } + runBrewInstallBatchFn = func(args ...string) (string, error) { + assert.Equal(t, []string{"install", "git", "curl"}, args) + return "", nil + } + installFormulaWithErrorFn = func(pkg string) string { + installFormulaRetries++ + t.Fatalf("unexpected retry for %s", pkg) + return "" + } + installSmartCaskWithErrorFn = func(pkg string) string { + t.Fatalf("unexpected cask retry for %s", pkg) + return "" + } + + oldStdout := os.Stdout + r, w, err := os.Pipe() + require.NoError(t, err) + os.Stdout = w + + formulae, casks, runErr := InstallWithProgress([]string{"git", "curl"}, nil, false) + + w.Close() + os.Stdout = oldStdout + + outputBytes, copyErr := io.ReadAll(r) + require.NoError(t, copyErr) + _ = outputBytes + + assert.NoError(t, runErr) + assert.ElementsMatch(t, []string{"git", "curl"}, formulae) + assert.Empty(t, casks) + assert.Equal(t, 0, installFormulaRetries) + assert.Equal(t, 2, calls) +} + func TestUpdate_DryRun(t *testing.T) { err := Update(true) assert.NoError(t, err) @@ -193,3 +323,90 @@ func TestHandleFailedJobs_WithFailures(t *testing.T) { } handleFailedJobs(failed) } + +// TestInstallWithProgress_BatchMode verifies that InstallWithProgress uses batch +// commands (brew install pkg1 pkg2...) instead of individual commands. +// This leverages Homebrew's native parallel download capability. +func TestInstallWithProgress_BatchMode(t *testing.T) { + // Capture stdout to verify batch command format + oldStdout := os.Stdout + r, w, err := os.Pipe() + require.NoError(t, err) + os.Stdout = w + + formulae, casks, runErr := InstallWithProgress( + []string{"git", "curl", "wget"}, + []string{"firefox", "chrome"}, + true, + ) + + w.Close() + os.Stdout = oldStdout + + outputBytes, copyErr := io.ReadAll(r) + require.NoError(t, copyErr) + output := string(outputBytes) + + assert.NoError(t, runErr) + assert.Empty(t, formulae, "dry-run should not report installed formulae") + assert.Empty(t, casks, "dry-run should not report installed casks") + + // Verify batch command format: all CLI packages in a single brew install + assert.Contains(t, output, "brew install git curl wget", + "dry-run should show a single batch brew install command for all CLI packages") + // Verify batch cask command format + assert.Contains(t, output, "brew install --cask firefox chrome", + "dry-run should show a single batch brew install --cask command for all cask packages") +} + +func TestExtractPackageError(t *testing.T) { + tests := []struct { + name string + output string + pkg string + expected string + }{ + { + name: "package-specific error line", + output: "==> Installing foo\nError: foo: no bottle available!\n==> Installing bar", + pkg: "foo", + expected: "Error: foo: no bottle available!", + }, + { + name: "no package-specific line falls back to parser", + output: "Error: No internet connection available", + pkg: "baz", + expected: "no internet connection", + }, + { + name: "no useful output gives batch message", + output: "some random output\nnothing useful here", + pkg: "qux", + expected: "not installed after batch attempt", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := extractPackageError(tt.output, tt.pkg) + assert.Equal(t, tt.expected, result) + }) + } +} + +// TestResolveFormulaName tests that formula aliases are resolved correctly. +// For example, "postgresql" resolves to "postgresql@18", "kubectl" to "kubernetes-cli". +// If resolution fails, returns the original name. +func TestResolveFormulaName(t *testing.T) { + // Test with a formula that likely exists (git is very common) + resolved := ResolveFormulaName("git") + assert.NotEmpty(t, resolved) + // Should return either "git" or a versioned variant + assert.True(t, resolved == "git" || strings.Contains(resolved, "git"), + "Should resolve git to itself or a variant, got: %s", resolved) + + // Test with a non-existent formula - should return original + resolved = ResolveFormulaName("nonexistent-formula-xyz") + assert.Equal(t, "nonexistent-formula-xyz", resolved, + "Should return original name when resolution fails") +} diff --git a/internal/diff/compare.go b/internal/diff/compare.go index 57c292f..c7f414c 100644 --- a/internal/diff/compare.go +++ b/internal/diff/compare.go @@ -171,6 +171,12 @@ func diffDotfiles(systemURL, referenceURL string) *DotfilesDiff { dd.RepoChanged = &ValueChange{System: systemURL, Reference: referenceURL} } + // Only check local dotfiles repo state if dotfiles are actually configured + // If both URLs are empty, there's no dotfiles setup to check + if sysNorm == "" && refNorm == "" { + return dd + } + // Check local dotfiles repo for dirty state home, err := os.UserHomeDir() if err != nil { diff --git a/internal/installer/installer.go b/internal/installer/installer.go index 5db6c05..730c18c 100644 --- a/internal/installer/installer.go +++ b/internal/installer/installer.go @@ -99,18 +99,18 @@ func checkDependencies(opts *config.InstallOptions, st *config.InstallState) err func runCustomInstall(opts *config.InstallOptions, st *config.InstallState) error { ui.Info(fmt.Sprintf("Custom config: @%s/%s", st.RemoteConfig.Username, st.RemoteConfig.Slug)) - if len(st.RemoteConfig.Taps) > 0 { - ui.Info(fmt.Sprintf("Adding %d taps, installing %d packages...", len(st.RemoteConfig.Taps), len(st.RemoteConfig.Packages))) - } else { - ui.Info(fmt.Sprintf("Installing %d packages...", len(st.RemoteConfig.Packages))) - } - fmt.Println() - formulaeCount := len(st.RemoteConfig.Packages) caskCount := len(st.RemoteConfig.Casks) npmCount := len(st.RemoteConfig.Npm) totalPackages := formulaeCount + caskCount + npmCount + if len(st.RemoteConfig.Taps) > 0 { + ui.Info(fmt.Sprintf("Adding %d taps, installing %d packages...", len(st.RemoteConfig.Taps), totalPackages)) + } else { + ui.Info(fmt.Sprintf("Installing %d packages...", totalPackages)) + } + fmt.Println() + minutes := estimateInstallMinutes(formulaeCount, caskCount, npmCount) ui.Info(fmt.Sprintf("Estimated install time: ~%d min for %d packages", minutes, totalPackages)) fmt.Println() @@ -142,6 +142,9 @@ func runCustomInstall(opts *config.InstallOptions, st *config.InstallState) erro for _, pkg := range st.RemoteConfig.Packages { st.SelectedPkgs[pkg.Name] = true } + for _, cask := range st.RemoteConfig.Casks { + st.SelectedPkgs[cask.Name] = true + } if err := stepInstallPackages(opts, st); err != nil { return err diff --git a/internal/installer/installer_test.go b/internal/installer/installer_test.go index ef69f80..84ab4eb 100644 --- a/internal/installer/installer_test.go +++ b/internal/installer/installer_test.go @@ -129,6 +129,37 @@ func TestCheckDependencies_DryRunSkipsEverything(t *testing.T) { assert.NoError(t, err) } +// TestRunCustomInstall_IncludesCasksInSelectedPkgs verifies that GUI apps (casks) +// from remote config are added to SelectedPkgs so they get installed. +// Regression test for: https://github.com/openbootdotdev/openboot/issues/17 +func TestRunCustomInstall_IncludesCasksInSelectedPkgs(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + + cfg := &config.Config{ + DryRun: true, + Shell: "skip", + Macos: "skip", + RemoteConfig: &config.RemoteConfig{ + Username: "testuser", + Slug: "testconfig", + Packages: config.PackageEntryList{{Name: "git"}, {Name: "curl"}}, + Casks: config.PackageEntryList{{Name: "visual-studio-code"}, {Name: "firefox"}}, + }, + } + + opts := cfg.ToInstallOptions() + st := cfg.ToInstallState() + err := runCustomInstall(opts, st) + assert.NoError(t, err) + + // Verify both packages and casks are in SelectedPkgs + assert.Contains(t, st.SelectedPkgs, "git", "CLI package should be in SelectedPkgs") + assert.Contains(t, st.SelectedPkgs, "curl", "CLI package should be in SelectedPkgs") + assert.Contains(t, st.SelectedPkgs, "visual-studio-code", "GUI app (cask) should be in SelectedPkgs") + assert.Contains(t, st.SelectedPkgs, "firefox", "GUI app (cask) should be in SelectedPkgs") +} + func TestRunInstall_DryRunRemoteConfig(t *testing.T) { cfg := &config.Config{ DryRun: true, diff --git a/internal/installer/step_packages.go b/internal/installer/step_packages.go index 3c9dd75..3e897c8 100644 --- a/internal/installer/step_packages.go +++ b/internal/installer/step_packages.go @@ -146,6 +146,11 @@ func stepPackageCustomization(opts *config.InstallOptions, st *config.InstallSta st.SelectedPkgs[pkg.Name] = true } } + if st.RemoteConfig != nil && len(st.RemoteConfig.Casks) > 0 { + for _, cask := range st.RemoteConfig.Casks { + st.SelectedPkgs[cask.Name] = true + } + } count := 0 for _, v := range selected { diff --git a/internal/system/system.go b/internal/system/system.go index f22be6e..1ac19db 100644 --- a/internal/system/system.go +++ b/internal/system/system.go @@ -70,11 +70,19 @@ func InstallHomebrew() error { } func GetGitConfig(key string) string { + // Try global first (most common) output, err := RunCommandSilent("git", "config", "--global", key) - if err != nil { - return "" + if err == nil && output != "" { + return output + } + + // Fall back to any available config (local, system, etc.) + output, err = RunCommandSilent("git", "config", key) + if err == nil { + return output } - return output + + return "" } func GetExistingGitConfig() (name, email string) { diff --git a/internal/system/system_test.go b/internal/system/system_test.go index 84ca188..48234a7 100644 --- a/internal/system/system_test.go +++ b/internal/system/system_test.go @@ -302,3 +302,21 @@ func TestRunCommandSilent_MultilineOutput(t *testing.T) { assert.Contains(t, output, "line2") assert.Contains(t, output, "line3") } + +// TestGetGitConfig_FallsBackToAnyScope verifies that GetGitConfig checks all git config scopes, +// not just --global. This handles cases where user.name/user.email are set in local or system config. +// Regression test for: git config detection issue +func TestGetGitConfig_FallsBackToAnyScope(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + + // Create a temporary git config file + gitConfigDir := tmpDir + "/.config/git" + os.MkdirAll(gitConfigDir, 0755) + + // Test that GetGitConfig returns empty when nothing is set + value := GetGitConfig("user.testkey") + // If git is not installed or no config exists, should return empty + // The function tries --global first, then falls back to any scope + assert.IsType(t, "", value) +} diff --git a/internal/ui/progress.go b/internal/ui/progress.go index 431b8e5..07bdd83 100644 --- a/internal/ui/progress.go +++ b/internal/ui/progress.go @@ -208,7 +208,6 @@ func (sp *StickyProgress) ResumeAfterInteractive() { sp.mu.Lock() defer sp.mu.Unlock() sp.active = true - sp.render() } func (sp *StickyProgress) Finish() { diff --git a/internal/ui/progress_test.go b/internal/ui/progress_test.go index 01d4601..4246c63 100644 --- a/internal/ui/progress_test.go +++ b/internal/ui/progress_test.go @@ -149,4 +149,7 @@ func TestStickyProgressPauseResume(t *testing.T) { sp.PauseForInteractive() assert.False(t, sp.active) + + sp.ResumeAfterInteractive() + assert.True(t, sp.active) }