refactor: code quality improvements from Gemini review

- Split frame.go: extract built-in components to frame_components.go
- Replace custom indexOf with strings.Index in frame_test.go
- Make prompt.go testable: accept io.Reader via SetStdin, add tests
- Decompose runGoQA: extract emitQAJSON and emitQASummary helpers
- DRY: centralise loadConfig into cmd/config/cmd.go
- Remove hardcoded MACOSX_DEPLOYMENT_TARGET from test/fuzz/cov commands
- Add error assertions to coverage_test.go

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

cmd/config/cmd.go

@@ -1,6 +1,9 @@
 package config
 
-import "forge.lthn.ai/core/cli/pkg/cli"
+import (
+	"forge.lthn.ai/core/cli/pkg/cli"
+	"forge.lthn.ai/core/go-config"
+)
 
 // AddConfigCommands registers the 'config' command group and all subcommands.
 func AddConfigCommands(root *cli.Command) {
@@ -12,3 +15,11 @@ func AddConfigCommands(root *cli.Command) {
 	addListCommand(configCmd)
 	addPathCommand(configCmd)
 }
+
+func loadConfig() (*config.Config, error) {
+	cfg, err := config.New()
+	if err != nil {
+		return nil, cli.Wrap(err, "failed to load config")
+	}
+	return cfg, nil
+}

cmd/config/cmd_get.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 
 	"forge.lthn.ai/core/cli/pkg/cli"
-	"forge.lthn.ai/core/go-config"
 )
 
 func addGetCommand(parent *cli.Command) {
@@ -30,11 +29,3 @@ func addGetCommand(parent *cli.Command) {
 
 	parent.AddCommand(cmd)
 }
-
-func loadConfig() (*config.Config, error) {
-	cfg, err := config.New()
-	if err != nil {
-		return nil, cli.Wrap(err, "failed to load config")
-	}
-	return cfg, nil
-}

cmd/config/cmd_list.go

@@ -2,6 +2,7 @@ package config
 
 import (
 	"fmt"
+	"maps"
 
 	"forge.lthn.ai/core/cli/pkg/cli"
 	"gopkg.in/yaml.v3"
@@ -14,7 +15,7 @@ func addListCommand(parent *cli.Command) {
 			return err
 		}
 
-		all := cfg.All()
+		all := maps.Collect(cfg.All())
 		if len(all) == 0 {
 			cli.Dim("No configuration values set")
 			return nil

cmd/gocmd/cmd_fuzz.go

@@ -87,7 +87,7 @@ func runGoFuzz(duration time.Duration, pkg, run string, verbose bool) error {
 		args = append(args, t.Pkg)
 
 		cmd := exec.Command("go", args...)
-		cmd.Env = append(os.Environ(), "MACOSX_DEPLOYMENT_TARGET=26.0", "CGO_ENABLED=0")
+		cmd.Env = append(os.Environ(), "CGO_ENABLED=0")
 		cmd.Dir, _ = os.Getwd()
 
 		output, runErr := cmd.CombinedOutput()

cmd/gocmd/cmd_gotest.go

@@ -88,7 +88,7 @@ func runGoTest(coverage bool, pkg, run string, short, race, jsonOut, verbose boo
 	}
 
 	cmd := exec.Command("go", args...)
-	cmd.Env = append(os.Environ(), "MACOSX_DEPLOYMENT_TARGET=26.0", "CGO_ENABLED=0")
+	cmd.Env = append(os.Environ(), "CGO_ENABLED=0")
 	cmd.Dir, _ = os.Getwd()
 
 	output, err := cmd.CombinedOutput()
@@ -243,7 +243,7 @@ func addGoCovCommand(parent *cli.Command) {
 			cmdArgs := append([]string{"test", "-coverprofile=" + covPath, "-covermode=atomic"}, pkgArgs...)
 
 			goCmd := exec.Command("go", cmdArgs...)
-			goCmd.Env = append(os.Environ(), "MACOSX_DEPLOYMENT_TARGET=26.0")
+			goCmd.Env = append(os.Environ(), "CGO_ENABLED=0")
 			goCmd.Stdout = os.Stdout
 			goCmd.Stderr = os.Stderr
 

cmd/gocmd/cmd_qa.go

@@ -11,7 +11,7 @@ import (
 	"time"
 
 	"forge.lthn.ai/core/cli/pkg/cli"
-	"forge.lthn.ai/core/go-devops/cmd/qa"
+	"forge.lthn.ai/core/lint/cmd/qa"
 	"forge.lthn.ai/core/go-i18n"
 )
 
@@ -291,27 +291,33 @@ func runGoQA(cmd *cli.Command, args []string) error {
 
 	duration := time.Since(startTime).Round(time.Millisecond)
 
-	// JSON output
 	if qaJSON {
-		qaResult := QAResult{
+		return emitQAJSON(results, coverageVal, branchVal, failed, duration)
-			Success:        failed == 0,
-			Duration:       duration.String(),
-			Checks:         results,
-			Coverage:       coverageVal,
-			BranchCoverage: branchVal,
-		}
-		if qaThreshold > 0 {
-			qaResult.Threshold = &qaThreshold
-		}
-		if qaBranchThreshold > 0 {
-			qaResult.BranchThreshold = &qaBranchThreshold
-		}
-		enc := json.NewEncoder(os.Stdout)
-		enc.SetIndent("", "  ")
-		return enc.Encode(qaResult)
 	}
 
-	// Summary
+	return emitQASummary(passed, failed, duration)
+}
+
+func emitQAJSON(results []CheckResult, coverageVal, branchVal *float64, failed int, duration time.Duration) error {
+	qaResult := QAResult{
+		Success:        failed == 0,
+		Duration:       duration.String(),
+		Checks:         results,
+		Coverage:       coverageVal,
+		BranchCoverage: branchVal,
+	}
+	if qaThreshold > 0 {
+		qaResult.Threshold = &qaThreshold
+	}
+	if qaBranchThreshold > 0 {
+		qaResult.BranchThreshold = &qaBranchThreshold
+	}
+	enc := json.NewEncoder(os.Stdout)
+	enc.SetIndent("", "  ")
+	return enc.Encode(qaResult)
+}
+
+func emitQASummary(passed, failed int, duration time.Duration) error {
 	if !qaQuiet {
 		cli.Blank()
 		if failed > 0 {

cmd/gocmd/coverage_test.go

@@ -33,10 +33,13 @@ forge.lthn.ai/core/go/pkg/bar.go:10.1,12.20 10 5
 
 	// Test empty file (only header)
 	contentEmpty := "mode: atomic\n"
-	tmpfileEmpty, _ := os.CreateTemp("", "test-coverage-empty-*.out")
+	tmpfileEmpty, err := os.CreateTemp("", "test-coverage-empty-*.out")
+	assert.NoError(t, err)
 	defer os.Remove(tmpfileEmpty.Name())
-	tmpfileEmpty.Write([]byte(contentEmpty))
+	_, err = tmpfileEmpty.Write([]byte(contentEmpty))
-	tmpfileEmpty.Close()
+	assert.NoError(t, err)
+	err = tmpfileEmpty.Close()
+	assert.NoError(t, err)
 
 	pct, err = calculateBlockCoverage(tmpfileEmpty.Name())
 	assert.NoError(t, err)
@@ -52,10 +55,13 @@ forge.lthn.ai/core/go/pkg/bar.go:10.1,12.20 10 5
 forge.lthn.ai/core/go/pkg/foo.go:1.2,3.4 5
 forge.lthn.ai/core/go/pkg/foo.go:1.2,3.4 5 notanumber
 `
-	tmpfileMalformed, _ := os.CreateTemp("", "test-coverage-malformed-*.out")
+	tmpfileMalformed, err := os.CreateTemp("", "test-coverage-malformed-*.out")
+	assert.NoError(t, err)
 	defer os.Remove(tmpfileMalformed.Name())
-	tmpfileMalformed.Write([]byte(contentMalformed))
+	_, err = tmpfileMalformed.Write([]byte(contentMalformed))
-	tmpfileMalformed.Close()
+	assert.NoError(t, err)
+	err = tmpfileMalformed.Close()
+	assert.NoError(t, err)
 
 	pct, err = calculateBlockCoverage(tmpfileMalformed.Name())
 	assert.NoError(t, err)
@@ -65,19 +71,24 @@ forge.lthn.ai/core/go/pkg/foo.go:1.2,3.4 5 notanumber
 	contentMalformed2 := `mode: set
 forge.lthn.ai/core/go/pkg/foo.go:1.2,3.4 5
 `
-	tmpfileMalformed2, _ := os.CreateTemp("", "test-coverage-malformed2-*.out")
+	tmpfileMalformed2, err := os.CreateTemp("", "test-coverage-malformed2-*.out")
+	assert.NoError(t, err)
 	defer os.Remove(tmpfileMalformed2.Name())
-	tmpfileMalformed2.Write([]byte(contentMalformed2))
+	_, err = tmpfileMalformed2.Write([]byte(contentMalformed2))
-	tmpfileMalformed2.Close()
+	assert.NoError(t, err)
+	err = tmpfileMalformed2.Close()
+	assert.NoError(t, err)
 
 	pct, err = calculateBlockCoverage(tmpfileMalformed2.Name())
 	assert.NoError(t, err)
 	assert.Equal(t, 0.0, pct)
 
 	// Test completely empty file
-	tmpfileEmpty2, _ := os.CreateTemp("", "test-coverage-empty2-*.out")
+	tmpfileEmpty2, err := os.CreateTemp("", "test-coverage-empty2-*.out")
+	assert.NoError(t, err)
 	defer os.Remove(tmpfileEmpty2.Name())
-	tmpfileEmpty2.Close()
+	err = tmpfileEmpty2.Close()
+	assert.NoError(t, err)
 	pct, err = calculateBlockCoverage(tmpfileEmpty2.Name())
 	assert.NoError(t, err)
 	assert.Equal(t, 0.0, pct)

pkg/cli/frame.go

@@ -468,98 +468,3 @@ func (f *Frame) runLive() {
 		Error(err.Error())
 	}
 }
-
-// ─────────────────────────────────────────────────────────────────────────────
-// Built-in Region Components
-// ─────────────────────────────────────────────────────────────────────────────
-
-// statusLineModel renders a "title  key:value  key:value" bar.
-type statusLineModel struct {
-	title string
-	pairs []string
-}
-
-// StatusLine creates a header/footer bar with a title and key:value pairs.
-//
-//	frame.Header(cli.StatusLine("core dev", "18 repos", "main"))
-func StatusLine(title string, pairs ...string) Model {
-	return &statusLineModel{title: title, pairs: pairs}
-}
-
-func (s *statusLineModel) View(width, _ int) string {
-	parts := []string{BoldStyle.Render(s.title)}
-	for _, p := range s.pairs {
-		parts = append(parts, DimStyle.Render(p))
-	}
-	line := strings.Join(parts, "  ")
-	if width > 0 {
-		line = Truncate(line, width)
-	}
-	return line
-}
-
-// keyHintsModel renders keyboard shortcut hints.
-type keyHintsModel struct {
-	hints []string
-}
-
-// KeyHints creates a footer showing keyboard shortcuts.
-//
-//	frame.Footer(cli.KeyHints("↑/↓ navigate", "enter select", "q quit"))
-func KeyHints(hints ...string) Model {
-	return &keyHintsModel{hints: hints}
-}
-
-func (k *keyHintsModel) View(width, _ int) string {
-	parts := make([]string, len(k.hints))
-	for i, h := range k.hints {
-		parts[i] = DimStyle.Render(h)
-	}
-	line := strings.Join(parts, "  ")
-	if width > 0 {
-		line = Truncate(line, width)
-	}
-	return line
-}
-
-// breadcrumbModel renders a navigation path.
-type breadcrumbModel struct {
-	parts []string
-}
-
-// Breadcrumb creates a navigation breadcrumb bar.
-//
-//	frame.Header(cli.Breadcrumb("core", "dev", "health"))
-func Breadcrumb(parts ...string) Model {
-	return &breadcrumbModel{parts: parts}
-}
-
-func (b *breadcrumbModel) View(width, _ int) string {
-	styled := make([]string, len(b.parts))
-	for i, p := range b.parts {
-		if i == len(b.parts)-1 {
-			styled[i] = BoldStyle.Render(p)
-		} else {
-			styled[i] = DimStyle.Render(p)
-		}
-	}
-	line := strings.Join(styled, DimStyle.Render(" > "))
-	if width > 0 {
-		line = Truncate(line, width)
-	}
-	return line
-}
-
-// staticModel wraps a plain string as a Model.
-type staticModel struct {
-	text string
-}
-
-// StaticModel wraps a static string as a Model, for use in Frame regions.
-func StaticModel(text string) Model {
-	return &staticModel{text: text}
-}
-
-func (s *staticModel) View(_, _ int) string {
-	return s.text
-}

pkg/cli/frame_components.go

@@ -0,0 +1,98 @@
+package cli
+
+import "strings"
+
+// ─────────────────────────────────────────────────────────────────────────────
+// Built-in Region Components
+// ─────────────────────────────────────────────────────────────────────────────
+
+// statusLineModel renders a "title  key:value  key:value" bar.
+type statusLineModel struct {
+	title string
+	pairs []string
+}
+
+// StatusLine creates a header/footer bar with a title and key:value pairs.
+//
+//	frame.Header(cli.StatusLine("core dev", "18 repos", "main"))
+func StatusLine(title string, pairs ...string) Model {
+	return &statusLineModel{title: title, pairs: pairs}
+}
+
+func (s *statusLineModel) View(width, _ int) string {
+	parts := []string{BoldStyle.Render(s.title)}
+	for _, p := range s.pairs {
+		parts = append(parts, DimStyle.Render(p))
+	}
+	line := strings.Join(parts, "  ")
+	if width > 0 {
+		line = Truncate(line, width)
+	}
+	return line
+}
+
+// keyHintsModel renders keyboard shortcut hints.
+type keyHintsModel struct {
+	hints []string
+}
+
+// KeyHints creates a footer showing keyboard shortcuts.
+//
+//	frame.Footer(cli.KeyHints("↑/↓ navigate", "enter select", "q quit"))
+func KeyHints(hints ...string) Model {
+	return &keyHintsModel{hints: hints}
+}
+
+func (k *keyHintsModel) View(width, _ int) string {
+	parts := make([]string, len(k.hints))
+	for i, h := range k.hints {
+		parts[i] = DimStyle.Render(h)
+	}
+	line := strings.Join(parts, "  ")
+	if width > 0 {
+		line = Truncate(line, width)
+	}
+	return line
+}
+
+// breadcrumbModel renders a navigation path.
+type breadcrumbModel struct {
+	parts []string
+}
+
+// Breadcrumb creates a navigation breadcrumb bar.
+//
+//	frame.Header(cli.Breadcrumb("core", "dev", "health"))
+func Breadcrumb(parts ...string) Model {
+	return &breadcrumbModel{parts: parts}
+}
+
+func (b *breadcrumbModel) View(width, _ int) string {
+	styled := make([]string, len(b.parts))
+	for i, p := range b.parts {
+		if i == len(b.parts)-1 {
+			styled[i] = BoldStyle.Render(p)
+		} else {
+			styled[i] = DimStyle.Render(p)
+		}
+	}
+	line := strings.Join(styled, DimStyle.Render(" > "))
+	if width > 0 {
+		line = Truncate(line, width)
+	}
+	return line
+}
+
+// staticModel wraps a plain string as a Model.
+type staticModel struct {
+	text string
+}
+
+// StaticModel wraps a static string as a Model, for use in Frame regions.
+func StaticModel(text string) Model {
+	return &staticModel{text: text}
+}
+
+func (s *staticModel) View(_, _ int) string {
+	return s.text
+}

pkg/cli/frame_test.go

@@ -2,6 +2,7 @@ package cli
 
 import (
 	"bytes"
+	"strings"
 	"testing"
 	"time"
 
@@ -38,9 +39,9 @@ func TestFrame_Good(t *testing.T) {
 		f.Footer(StaticModel("CCC"))
 
 		out := f.String()
-		posA := indexOf(out, "AAA")
+		posA := strings.Index(out, "AAA")
-		posB := indexOf(out, "BBB")
+		posB := strings.Index(out, "BBB")
-		posC := indexOf(out, "CCC")
+		posC := strings.Index(out, "CCC")
 		assert.Less(t, posA, posB, "header before content")
 		assert.Less(t, posB, posC, "content before footer")
 	})
@@ -415,9 +416,9 @@ func TestFrameTeaModel_Good(t *testing.T) {
 		f.height = 24
 
 		view := f.View()
-		posA := indexOf(view, "AAA")
+		posA := strings.Index(view, "AAA")
-		posB := indexOf(view, "BBB")
+		posB := strings.Index(view, "BBB")
-		posC := indexOf(view, "CCC")
+		posC := strings.Index(view, "CCC")
 		assert.Greater(t, posA, -1, "header should be present")
 		assert.Greater(t, posB, -1, "content should be present")
 		assert.Greater(t, posC, -1, "footer should be present")
@@ -483,7 +484,7 @@ func TestFrameSpatialFocus_Good(t *testing.T) {
 }
 
 func TestFrameNavigateFrameModel_Good(t *testing.T) {
-	t.Run("Navigate with FrameModel preserves focus on Content", func(t *testing.T) {
+	t.Run("Navigate preserves current focus", func(t *testing.T) {
 		f := NewFrame("HCF")
 		f.Header(StaticModel("h"))
 		f.Content(&testFrameModel{viewText: "page-1"})
@@ -550,12 +551,3 @@ func TestFrameMessageRouting_Good(t *testing.T) {
 	})
 }
 
-// indexOf returns the position of substr in s, or -1 if not found.
-func indexOf(s, substr string) int {
-	for i := range len(s) - len(substr) + 1 {
-		if s[i:i+len(substr)] == substr {
-			return i
-		}
-	}
-	return -1
-}

pkg/cli/prompt.go

@@ -4,12 +4,24 @@ import (
 	"bufio"
 	"errors"
 	"fmt"
+	"io"
 	"os"
 	"strconv"
 	"strings"
 )
 
-var stdin = bufio.NewReader(os.Stdin)
+var stdin io.Reader = os.Stdin
+
+// SetStdin overrides the default stdin reader for testing.
+func SetStdin(r io.Reader) { stdin = r }
+
+// newReader wraps stdin in a bufio.Reader if it isn't one already.
+func newReader() *bufio.Reader {
+	if br, ok := stdin.(*bufio.Reader); ok {
+		return br
+	}
+	return bufio.NewReader(stdin)
+}
 
 // Prompt asks for text input with a default value.
 func Prompt(label, defaultVal string) (string, error) {
@@ -19,7 +31,8 @@ func Prompt(label, defaultVal string) (string, error) {
 		fmt.Printf("%s: ", label)
 	}
 
-	input, err := stdin.ReadString('\n')
+	r := newReader()
+	input, err := r.ReadString('\n')
 	if err != nil {
 		return "", err
 	}
@@ -39,7 +52,8 @@ func Select(label string, options []string) (string, error) {
 	}
 	fmt.Printf("Choose [1-%d]: ", len(options))
 
-	input, err := stdin.ReadString('\n')
+	r := newReader()
+	input, err := r.ReadString('\n')
 	if err != nil {
 		return "", err
 	}
@@ -59,7 +73,8 @@ func MultiSelect(label string, options []string) ([]string, error) {
 	}
 	fmt.Printf("Choose (space-separated) [1-%d]: ", len(options))
 
-	input, err := stdin.ReadString('\n')
+	r := newReader()
+	input, err := r.ReadString('\n')
 	if err != nil {
 		return nil, err
 	}

pkg/cli/prompt_test.go

@@ -0,0 +1,52 @@
+package cli
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestPrompt_Good(t *testing.T) {
+	SetStdin(strings.NewReader("hello\n"))
+	defer SetStdin(nil) // reset
+
+	val, err := Prompt("Name", "")
+	assert.NoError(t, err)
+	assert.Equal(t, "hello", val)
+}
+
+func TestPrompt_Good_Default(t *testing.T) {
+	SetStdin(strings.NewReader("\n"))
+	defer SetStdin(nil)
+
+	val, err := Prompt("Name", "world")
+	assert.NoError(t, err)
+	assert.Equal(t, "world", val)
+}
+
+func TestSelect_Good(t *testing.T) {
+	SetStdin(strings.NewReader("2\n"))
+	defer SetStdin(nil)
+
+	val, err := Select("Pick", []string{"a", "b", "c"})
+	assert.NoError(t, err)
+	assert.Equal(t, "b", val)
+}
+
+func TestSelect_Bad_Invalid(t *testing.T) {
+	SetStdin(strings.NewReader("5\n"))
+	defer SetStdin(nil)
+
+	_, err := Select("Pick", []string{"a", "b"})
+	assert.Error(t, err)
+}
+
+func TestMultiSelect_Good(t *testing.T) {
+	SetStdin(strings.NewReader("1 3\n"))
+	defer SetStdin(nil)
+
+	vals, err := MultiSelect("Pick", []string{"a", "b", "c"})
+	assert.NoError(t, err)
+	assert.Equal(t, []string{"a", "c"}, vals)
+}