refactor: split mixed-concern files identified by semantic function clustering analysis

pkg/cli/audit_cross_run_render.go

@@ -6,11 +6,11 @@ import (
 	"os"
 	"strconv"
 	"strings"
-	"time"
 
 	"github.com/github/gh-aw/pkg/console"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/stringutil"
+	"github.com/github/gh-aw/pkg/timeutil"
 )
 
 var crossRunRenderLog = logger.New("cli:audit_cross_run_render")
@@ -344,11 +344,7 @@ func formatRunIDs(ids []int64) string {
 
 // formatDurationNs formats a nanosecond duration as a human-readable string.
 func formatDurationNs(ns int64) string {
-	if ns <= 0 {
-		return "—"
-	}
-	d := time.Duration(ns)
-	return d.Round(time.Second).String()
+	return timeutil.FormatDurationNs(ns)
 }
 
 // safePercent returns percentage of part/total, returning 0 when total is 0.

pkg/cli/compile_batch_operations.go

@@ -37,6 +37,30 @@ import (
 
 var compileBatchOperationsLog = logger.New("cli:compile_batch_operations")
 
+// RunActionlintOnFiles runs actionlint on multiple lock files in a single batch.
+// This is more efficient than running actionlint once per file.
+func RunActionlintOnFiles(lockFiles []string, verbose bool, strict bool) error {
+	if len(lockFiles) == 0 {
+		return nil
+	}
+	return runActionlintOnFiles(lockFiles, verbose, strict)
+}
+
+// RunZizmorOnFiles runs zizmor on multiple lock files in a single batch.
+// This is more efficient than running zizmor once per file.
+func RunZizmorOnFiles(lockFiles []string, verbose bool, strict bool) error {
+	if len(lockFiles) == 0 {
+		return nil
+	}
+	return runZizmorOnFiles(lockFiles, verbose, strict)
+}
+
+// RunPoutineOnDirectory runs poutine security scanner once on a directory.
+// Poutine scans all workflows in a directory, so it only needs to run once.
+func RunPoutineOnDirectory(workflowDir string, verbose bool, strict bool) error {
+	return runPoutineOnDirectory(workflowDir, verbose, strict)
+}
+
 // runBatchLockFileTool runs a batch tool on lock files with uniform error handling
 func runBatchLockFileTool(toolName string, lockFiles []string, verbose bool, strict bool, runner func([]string, bool, bool) error) error {
 	if len(lockFiles) == 0 {

pkg/cli/compile_validation.go

@@ -15,30 +15,6 @@ import (
 
 var compileValidationLog = logger.New("cli:compile_validation")
 
-// RunActionlintOnFiles runs actionlint on multiple lock files in a single batch
-// This is more efficient than running actionlint once per file
-func RunActionlintOnFiles(lockFiles []string, verbose bool, strict bool) error {
-	if len(lockFiles) == 0 {
-		return nil
-	}
-	return runActionlintOnFiles(lockFiles, verbose, strict)
-}
-
-// RunZizmorOnFiles runs zizmor on multiple lock files in a single batch
-// This is more efficient than running zizmor once per file
-func RunZizmorOnFiles(lockFiles []string, verbose bool, strict bool) error {
-	if len(lockFiles) == 0 {
-		return nil
-	}
-	return runZizmorOnFiles(lockFiles, verbose, strict)
-}
-
-// RunPoutineOnDirectory runs poutine security scanner once on a directory
-// Poutine scans all workflows in a directory, so it only needs to run once
-func RunPoutineOnDirectory(workflowDir string, verbose bool, strict bool) error {
-	return runPoutineOnDirectory(workflowDir, verbose, strict)
-}
-
 // CompileWorkflowWithValidation compiles a workflow with always-on YAML validation for CLI usage
 func CompileWorkflowWithValidation(compiler *workflow.Compiler, filePath string, verbose bool, runZizmorPerFile bool, runPoutinePerFile bool, runActionlintPerFile bool, strict bool, validateActionSHAs bool) error {
 	compileValidationLog.Printf("Compiling workflow with validation: file=%s, strict=%v, validateSHAs=%v", filePath, strict, validateActionSHAs)

pkg/cli/mcp_error.go

@@ -0,0 +1,28 @@
+package cli
+
+import (
+	"encoding/json"
+
+	"github.com/modelcontextprotocol/go-sdk/jsonrpc"
+)
+
+// newMCPError creates a jsonrpc.Error with the given code, message, and optional data.
+// The data value is marshaled via mcpErrorData.
+func newMCPError(code int64, msg string, data any) error {
+	return &jsonrpc.Error{Code: code, Message: msg, Data: mcpErrorData(data)}
+}
+
+// mcpErrorData marshals data to JSON for use in jsonrpc.Error.Data field.
+// Returns nil if marshaling fails to avoid errors in error handling.
+func mcpErrorData(v any) json.RawMessage {
+	if v == nil {
+		return nil
+	}
+	data, err := json.Marshal(v)
+	if err != nil {
+		// Log the error but return nil to avoid breaking error handling
+		mcpLog.Printf("Failed to marshal error data: %v", err)
+		return nil
+	}
+	return data
+}

pkg/cli/mcp_server_helpers.go → pkg/cli/mcp_permissions.go

@@ -2,10 +2,8 @@ package cli
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
 	"fmt"
-	"os"
 	"strings"
 	"time"
 
@@ -14,67 +12,6 @@ import (
 	"github.com/modelcontextprotocol/go-sdk/jsonrpc"
 )
 
-// newMCPError creates a jsonrpc.Error with the given code, message, and optional data.
-// The data value is marshaled via mcpErrorData.
-func newMCPError(code int64, msg string, data any) error {
-	return &jsonrpc.Error{Code: code, Message: msg, Data: mcpErrorData(data)}
-}
-
-// mcpErrorData marshals data to JSON for use in jsonrpc.Error.Data field.
-// Returns nil if marshaling fails to avoid errors in error handling.
-func mcpErrorData(v any) json.RawMessage {
-	if v == nil {
-		return nil
-	}
-	data, err := json.Marshal(v)
-	if err != nil {
-		// Log the error but return nil to avoid breaking error handling
-		mcpLog.Printf("Failed to marshal error data: %v", err)
-		return nil
-	}
-	return data
-}
-
-// boolPtr returns a pointer to the given bool value, used for optional *bool fields.
-func boolPtr(b bool) *bool { return &b }
-
-// getRepository retrieves the current repository name (owner/repo format).
-// Results are cached for 1 hour to avoid repeated queries.
-// Checks GITHUB_REPOSITORY environment variable first, then falls back to gh repo view.
-func getRepository() (string, error) {
-	// Check cache first
-	if repo, ok := mcpCache.GetRepo(); ok {
-		mcpLog.Printf("Using cached repository: %s", repo)
-		return repo, nil
-	}
-
-	// Try GITHUB_REPOSITORY environment variable first
-	repo := os.Getenv("GITHUB_REPOSITORY")
-	if repo != "" {
-		mcpLog.Printf("Got repository from GITHUB_REPOSITORY: %s", repo)
-		mcpCache.SetRepo(repo)
-		return repo, nil
-	}
-
-	// Fall back to gh repo view
-	mcpLog.Print("Querying repository using gh repo view")
-	cmd := workflow.ExecGH("repo", "view", "--json", "nameWithOwner", "--jq", ".nameWithOwner")
-	output, err := cmd.Output()
-	if err != nil {
-		mcpLog.Printf("Failed to get repository: %v", err)
-		return "", fmt.Errorf("failed to get repository: %w", err)
-	}
-
-	repo = strings.TrimSpace(string(output))
-	if repo == "" {
-		return "", errors.New("repository not found")
-	}
-
-	mcpLog.Printf("Got repository from gh repo view: %s", repo)
-	mcpCache.SetRepo(repo)
-	return repo, nil
-}
-
 // queryActorRole queries the GitHub API to determine the actor's role in the repository.
 // Returns the permission level (admin, maintain, write, triage, read) or an error.
 // Results are cached for 1 hour to avoid excessive API calls.

pkg/cli/mcp_repository.go

@@ -0,0 +1,47 @@
+package cli
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"strings"
+
+	"github.com/github/gh-aw/pkg/workflow"
+)
+
+// getRepository retrieves the current repository name (owner/repo format).
+// Results are cached for 1 hour to avoid repeated queries.
+// Checks GITHUB_REPOSITORY environment variable first, then falls back to gh repo view.
+func getRepository() (string, error) {
+	// Check cache first
+	if repo, ok := mcpCache.GetRepo(); ok {
+		mcpLog.Printf("Using cached repository: %s", repo)
+		return repo, nil
+	}
+
+	// Try GITHUB_REPOSITORY environment variable first
+	repo := os.Getenv("GITHUB_REPOSITORY")
+	if repo != "" {
+		mcpLog.Printf("Got repository from GITHUB_REPOSITORY: %s", repo)
+		mcpCache.SetRepo(repo)
+		return repo, nil
+	}
+
+	// Fall back to gh repo view
+	mcpLog.Print("Querying repository using gh repo view")
+	cmd := workflow.ExecGH("repo", "view", "--json", "nameWithOwner", "--jq", ".nameWithOwner")
+	output, err := cmd.Output()
+	if err != nil {
+		mcpLog.Printf("Failed to get repository: %v", err)
+		return "", fmt.Errorf("failed to get repository: %w", err)
+	}
+
+	repo = strings.TrimSpace(string(output))
+	if repo == "" {
+		return "", errors.New("repository not found")
+	}
+
+	mcpLog.Printf("Got repository from gh repo view: %s", repo)
+	mcpCache.SetRepo(repo)
+	return repo, nil
+}

pkg/cli/token_usage.go

@@ -4,13 +4,13 @@ import (
 	"bufio"
 	"encoding/json"
 	"fmt"
-	"math"
 	"os"
 	"path/filepath"
 	"sort"
 	"strings"
 
 	"github.com/github/gh-aw/pkg/logger"
+	"github.com/github/gh-aw/pkg/timeutil"
 )
 
 var tokenUsageLog = logger.New("cli:token_usage")
@@ -236,18 +236,10 @@ func (s *TokenUsageSummary) AvgDurationMs() int {
 	return s.TotalDurationMs / s.TotalRequests
 }
 
-// FormatDurationMs formats milliseconds as a human-readable string
+// FormatDurationMs formats milliseconds as a human-readable string.
+// Deprecated: Use timeutil.FormatDurationMs instead.
 func FormatDurationMs(ms int) string {
-	if ms < 1000 {
-		return fmt.Sprintf("%dms", ms)
-	}
-	seconds := float64(ms) / 1000.0
-	if seconds < 60 {
-		return fmt.Sprintf("%.1fs", seconds)
-	}
-	minutes := int(seconds) / 60
-	secs := math.Mod(seconds, 60)
-	return fmt.Sprintf("%dm%.0fs", minutes, secs)
+	return timeutil.FormatDurationMs(ms)
 }
 
 // ModelRows returns the by-model data as sorted rows for console rendering

pkg/cli/util.go

@@ -0,0 +1,4 @@
+package cli
+
+// boolPtr returns a pointer to the given bool value, used for optional *bool fields.
+func boolPtr(b bool) *bool { return &b }

pkg/timeutil/format.go

@@ -2,6 +2,7 @@ package timeutil
 
 import (
 	"fmt"
+	"math"
 	"time"
 )
 
@@ -25,3 +26,28 @@ func FormatDuration(d time.Duration) string {
 	}
 	return fmt.Sprintf("%.1fh", d.Hours())
 }
+
+// FormatDurationMs formats a duration given in milliseconds as a human-readable string.
+// Examples: 500 -> "500ms", 1500 -> "1.5s", 90000 -> "1m30s"
+func FormatDurationMs(ms int) string {
+	if ms < 1000 {
+		return fmt.Sprintf("%dms", ms)
+	}
+	seconds := float64(ms) / 1000.0
+	if seconds < 60 {
+		return fmt.Sprintf("%.1fs", seconds)
+	}
+	minutes := int(seconds) / 60
+	secs := math.Mod(seconds, 60)
+	return fmt.Sprintf("%dm%.0fs", minutes, secs)
+}
+
+// FormatDurationNs formats a duration given in nanoseconds as a human-readable string.
+// Returns "—" for zero or negative values. Uses Go's standard duration rounding to seconds.
+func FormatDurationNs(ns int64) string {
+	if ns <= 0 {
+		return "—"
+	}
+	d := time.Duration(ns)
+	return d.Round(time.Second).String()
+}