perf: use batch brew install with native parallelism

[jerryjrxie]

Summary

Simplifies package installation by piping Homebrew's native output directly to the user instead of building custom progress indicators.

Problem

The custom progress implementation was complex, inaccurate, and duplicated what Homebrew already does well. We were capturing output, parsing it, and building our own progress bars.

Solution

Just let brew show its output:

• Pipe brew install stdout/stderr directly to the terminal
• Homebrew already has excellent progress indicators
• Much simpler code, better user experience

Changes

- Captured brew output into string
- Parsed output to detect success/failure per package  
- Built custom progress bars
- Incremented counters manually

+ cmd.Stdout = os.Stdout
+ cmd.Stderr = os.Stderr
+ cmd.Run()
+ progress.IncrementWithStatus(err == nil)

91 lines removed → much simpler

Benefits

1. Familiar - Users see Homebrew's native progress bars
2. Accurate - Real download speeds and ETA from brew
3. Password prompts - Still work via TTY
4. Maintainable - 64 insertions, 91 deletions

Testing

• Tested with batch installs (CLI + casks)
• Password prompts work correctly
• Progress bar shows completion count at end
• go vet passes

Stacked on PR #18

Includes the GUI apps fix from #18.

[github-actions]

👋 Thanks for opening this pull request!

🎉 This is your first PR to OpenBoot — welcome!

Before merging:

• Code follows existing patterns in the codebase
• go build ./... and go vet ./... pass
• Commit message is clear and descriptive

@fullstackjam will review this soon. Thanks for contributing! 🚀

[Copilot]

Pull request overview

This PR simplifies Homebrew package installation by removing the custom parallel worker pool and switching to a single batch brew install ... invocation (letting Homebrew manage native parallel downloads). It also carries forward the stacked fix to ensure remote configs include casks (GUI apps) in selected packages.

Changes:

• Replace per-formula concurrent brew install <pkg> calls with one batch brew install <pkg1> <pkg2> ... for CLI packages, and update dry-run output accordingly.
• Remove the custom runParallelInstallWithProgress worker-pool implementation.
• Ensure casks from remote configs are included in SelectedPkgs (installer + package customization step).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
internal/brew/brew.go	Switch CLI installs to batch brew install and remove parallel worker pool; adjust dry-run printing.
internal/installer/installer.go	Include remote-config casks in SelectedPkgs during custom installs.
internal/installer/step_packages.go	Include remote-config casks in SelectedPkgs during package customization.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The per-package success detection for the batch brew install is not reliable and will misclassify installs. In the error case (err != nil), strings.Contains(outputStr, pkg) is neither a failure indicator nor a success indicator: successful packages will often appear in output and be marked failed, and packages that never ran may be marked installed if they don't appear in the output. This can also cause incorrect state tracking (packages marked installed when they are not) and incorrect progress status (currently always increments as success).

A more robust approach is to run the batch install, then re-check installed formulae (e.g., call GetInstalledPackages() again) and classify each pkg based on whether it is now installed; only those still missing should be added to allFailed / retried, and IncrementWithStatus should use the real success value.

Suggested change

	args := append([]string{"install"}, newCli...)
	cmd := brewInstallCmd(args...)
	output, err := cmd.CombinedOutput()
	outputStr := string(output)
	for _, pkg := range newCli {
	progress.IncrementWithStatus(true)
	if err == nil || !strings.Contains(outputStr, pkg) {
	installedFormulae = append(installedFormulae, pkg)
	} else {
	errMsg := parseBrewError(outputStr)
	if errMsg == "" {
	errMsg = "install failed"
	}
	allFailed = append(allFailed, failedJob{
	installJob: installJob{name: pkg, isCask: false},
	errMsg: errMsg,
	})
	}
	args := append([]string{"install"}, newCli...)
	cmd := brewInstallCmd(args...)
	output, err := cmd.CombinedOutput()
	outputStr := string(output)
	installedAfter, installedErr := GetInstalledPackages()
	if installedErr != nil {
	// Fallback to output-based heuristic if we cannot query installed packages.
	for _, pkg := range newCli {
	// Preserve previous behavior: always increment as success in this path.
	progress.IncrementWithStatus(true)
	if err == nil || !strings.Contains(outputStr, pkg) {
	installedFormulae = append(installedFormulae, pkg)
	} else {
	errMsg := parseBrewError(outputStr)
	if errMsg == "" {
	errMsg = "install failed"
	}
	allFailed = append(allFailed, failedJob{
	installJob: installJob{name: pkg, isCask: false},
	errMsg: errMsg,
	})
	}
	}
	} else {
	installedSet := make(map[string]struct{}, len(installedAfter))
	for _, name := range installedAfter {
	installedSet[name] = struct{}{}
	}
	for _, pkg := range newCli {
	_, installed := installedSet[pkg]
	progress.IncrementWithStatus(installed)
	if installed {
	installedFormulae = append(installedFormulae, pkg)
	} else {
	errMsg := parseBrewError(outputStr)
	if errMsg == "" {
	errMsg = "install failed"
	}
	allFailed = append(allFailed, failedJob{
	installJob: installJob{name: pkg, isCask: false},
	errMsg: errMsg,
	})
	}
	}

[Copilot]

runCustomInstall now includes casks in SelectedPkgs, but the earlier status message still reports only len(st.RemoteConfig.Packages) as the number of packages to install. This will undercount when remote configs include casks (and/or npm). Consider updating that ui.Info(...) to use the already-computed totals (formulae+casks+npm) or at least include cask count so the message matches what will actually be installed.

Suggested change

	totalBrewPkgs := len(st.RemoteConfig.Packages) + len(st.RemoteConfig.Casks)
	totalNpmPkgs := len(st.RemoteConfig.Npm)
	if totalBrewPkgs > 0 {
	if totalNpmPkgs > 0 {
	ui.Info(fmt.Sprintf("Installing %d Homebrew packages (including casks) and %d npm packages...", totalBrewPkgs, totalNpmPkgs))
	} else {
	ui.Info(fmt.Sprintf("Installing %d Homebrew packages (including casks)...", totalBrewPkgs))
	}
	} else if totalNpmPkgs > 0 {
	ui.Info(fmt.Sprintf("Installing %d npm packages...", totalNpmPkgs))
	}

[Copilot]

This change introduces new non-dry-run behavior in InstallWithProgress (batch install + per-package classification) but current tests only cover the dry-run path. Adding unit-level coverage around the new classification path would help prevent regressions (e.g., by refactoring to inject the brew command runner and an "installed packages" provider so failures/partial successes can be simulated deterministically).