refactor: eliminate duplicate git root detection and typeutil alias wrappers (#25283)

Author: Copilot

pkg/cli/dependency_graph.go

@@ -247,7 +247,12 @@ func (g *DependencyGraph) resolveImportPath(importPath string, baseDir string) s
 
 	// If that fails, try resolving with parser's cache-aware resolution
 	// Note: We create a minimal cache here just for resolution
-	importCache := parser.NewImportCache(g.findGitRoot())
+	gitRoot, err := findGitRootForPath(g.workflowsDir)
+	if err != nil {
+		depGraphLog.Printf("Failed to find git root, using fallback: %v", err)
+		gitRoot = g.workflowsDir
+	}
+	importCache := parser.NewImportCache(gitRoot)
 	fullPath, err := parser.ResolveIncludePath(importPath, baseDir, importCache)
 	if err != nil {
 		depGraphLog.Printf("Failed to resolve import path %s: %v", importPath, err)
@@ -258,29 +263,6 @@ func (g *DependencyGraph) resolveImportPath(importPath string, baseDir string) s
 	return fullPath
 }
 
-// findGitRoot finds the git repository root
-func (g *DependencyGraph) findGitRoot() string {
-	depGraphLog.Printf("Finding git root starting from: %s", g.workflowsDir)
-	// Start from workflows directory and walk up
-	dir := g.workflowsDir
-	for {
-		gitDir := filepath.Join(dir, ".git")
-		if _, err := os.Stat(gitDir); err == nil {
-			depGraphLog.Printf("Found git root at: %s", dir)
-			return dir
-		}
-		parent := filepath.Dir(dir)
-		if parent == dir {
-			// Reached filesystem root
-			depGraphLog.Printf("Reached filesystem root, no .git directory found")
-			break
-		}
-		dir = parent
-	}
-	depGraphLog.Printf("Using fallback git root: %s", g.workflowsDir)
-	return g.workflowsDir // Fallback to workflows dir
-}
-
 // GetAffectedWorkflows returns the list of workflows that need to be recompiled
 // when the given file is modified
 func (g *DependencyGraph) GetAffectedWorkflows(modifiedPath string) []string {

pkg/cli/logs_parsing_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/github/gh-aw/pkg/testutil"
+	"github.com/github/gh-aw/pkg/typeutil"
 	"github.com/github/gh-aw/pkg/workflow"
 )
 
@@ -165,7 +166,7 @@ func TestConvertToInt(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		result := workflow.ConvertToInt(tt.value)
+		result := typeutil.ConvertToInt(tt.value)
 		if result != tt.expected {
 			t.Errorf("ConvertToInt(%v) = %d, expected %d", tt.value, result, tt.expected)
 		}
@@ -186,7 +187,7 @@ func TestConvertToFloat(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		result := workflow.ConvertToFloat(tt.value)
+		result := typeutil.ConvertToFloat(tt.value)
 		if result != tt.expected {
 			t.Errorf("ConvertToFloat(%v) = %f, expected %f", tt.value, result, tt.expected)
 		}

pkg/parser/frontmatter_hash.go

@@ -14,21 +14,15 @@ import (
 	"strings"
 
 	"github.com/github/gh-aw/pkg/logger"
+	"github.com/github/gh-aw/pkg/typeutil"
 )
 
 var frontmatterHashLog = logger.New("parser:frontmatter_hash")
 
 // parseBoolFromFrontmatter extracts a boolean value from a frontmatter map.
 // Returns false if the key is absent, the map is nil, or the value is not a bool.
 func parseBoolFromFrontmatter(m map[string]any, key string) bool {
-	if m == nil {
-		return false
-	}
-	if v, ok := m[key]; ok {
-		b, _ := v.(bool)
-		return b
-	}
-	return false
+	return typeutil.ParseBool(m, key)
 }
 
 // FileReader is a function type that reads file content

pkg/typeutil/convert.go

@@ -11,6 +11,9 @@
 //     the caller needs to distinguish "missing/invalid" from a zero value, or when string
 //     inputs are not expected (e.g. YAML config field parsing).
 //
+// Bool Extraction:
+//   - ParseBool() - Extract a bool from map[string]any by key; returns false on missing, nil map, or non-bool.
+//
 // Safe Conversions (return 0 on overflow or invalid input):
 //   - SafeUint64ToInt() - Convert uint64 to int, returning 0 on overflow
 //   - SafeUintToInt() - Convert uint to int, returning 0 on overflow
@@ -110,6 +113,19 @@ func ConvertToInt(val any) int {
 	return 0
 }
 
+// ParseBool extracts a boolean value from a map[string]any by key.
+// Returns false if the map is nil, the key is absent, or the value is not a bool.
+func ParseBool(m map[string]any, key string) bool {
+	if m == nil {
+		return false
+	}
+	if v, ok := m[key]; ok {
+		b, _ := v.(bool)
+		return b
+	}
+	return false
+}
+
 // ConvertToFloat safely converts any value to float64, returning 0 on failure.
 //
 // Supported input types: float64, int, int64, and string (parsed via strconv.ParseFloat).

pkg/workflow/claude_logs.go

@@ -7,6 +7,7 @@ import (
 	"strings"
 
 	"github.com/github/gh-aw/pkg/logger"
+	"github.com/github/gh-aw/pkg/typeutil"
 )
 
 var claudeLogsLog = logger.New("workflow:claude_logs")
@@ -106,18 +107,18 @@ func (e *ClaudeEngine) extractClaudeResultMetrics(line string) LogMetrics {
 
 	// Extract total_cost_usd directly
 	if totalCost, exists := jsonData["total_cost_usd"]; exists {
-		if cost := ConvertToFloat(totalCost); cost > 0 {
+		if cost := typeutil.ConvertToFloat(totalCost); cost > 0 {
 			metrics.EstimatedCost = cost
 		}
 	}
 
 	// Extract usage information with all token types
 	if usage, exists := jsonData["usage"]; exists {
 		if usageMap, ok := usage.(map[string]any); ok {
-			inputTokens := ConvertToInt(usageMap["input_tokens"])
-			outputTokens := ConvertToInt(usageMap["output_tokens"])
-			cacheCreationTokens := ConvertToInt(usageMap["cache_creation_input_tokens"])
-			cacheReadTokens := ConvertToInt(usageMap["cache_read_input_tokens"])
+			inputTokens := typeutil.ConvertToInt(usageMap["input_tokens"])
+			outputTokens := typeutil.ConvertToInt(usageMap["output_tokens"])
+			cacheCreationTokens := typeutil.ConvertToInt(usageMap["cache_creation_input_tokens"])
+			cacheReadTokens := typeutil.ConvertToInt(usageMap["cache_read_input_tokens"])
 
 			totalTokens := inputTokens + outputTokens + cacheCreationTokens + cacheReadTokens
 			if totalTokens > 0 {
@@ -128,7 +129,7 @@ func (e *ClaudeEngine) extractClaudeResultMetrics(line string) LogMetrics {
 
 	// Extract number of turns
 	if numTurns, exists := jsonData["num_turns"]; exists {
-		if turns := ConvertToInt(numTurns); turns > 0 {
+		if turns := typeutil.ConvertToInt(numTurns); turns > 0 {
 			metrics.Turns = turns
 		}
 	}
@@ -241,18 +242,18 @@ func (e *ClaudeEngine) parseClaudeJSONLog(logContent string, verbose bool) LogMe
 			if typeStr, ok := entryType.(string); ok && typeStr == "result" {
 				// Found the result payload, extract cost and token data
 				if totalCost, exists := entry["total_cost_usd"]; exists {
-					if cost := ConvertToFloat(totalCost); cost > 0 {
+					if cost := typeutil.ConvertToFloat(totalCost); cost > 0 {
 						metrics.EstimatedCost = cost
 					}
 				}
 
 				// Extract usage information with all token types
 				if usage, exists := entry["usage"]; exists {
 					if usageMap, ok := usage.(map[string]any); ok {
-						inputTokens := ConvertToInt(usageMap["input_tokens"])
-						outputTokens := ConvertToInt(usageMap["output_tokens"])
-						cacheCreationTokens := ConvertToInt(usageMap["cache_creation_input_tokens"])
-						cacheReadTokens := ConvertToInt(usageMap["cache_read_input_tokens"])
+						inputTokens := typeutil.ConvertToInt(usageMap["input_tokens"])
+						outputTokens := typeutil.ConvertToInt(usageMap["output_tokens"])
+						cacheCreationTokens := typeutil.ConvertToInt(usageMap["cache_creation_input_tokens"])
+						cacheReadTokens := typeutil.ConvertToInt(usageMap["cache_read_input_tokens"])
 
 						totalTokens := inputTokens + outputTokens + cacheCreationTokens + cacheReadTokens
 						if totalTokens > 0 {
@@ -263,7 +264,7 @@ func (e *ClaudeEngine) parseClaudeJSONLog(logContent string, verbose bool) LogMe
 
 				// Extract number of turns
 				if numTurns, exists := entry["num_turns"]; exists {
-					if turns := ConvertToInt(numTurns); turns > 0 {
+					if turns := typeutil.ConvertToInt(numTurns); turns > 0 {
 						metrics.Turns = turns
 					}
 				}

pkg/workflow/config_helpers.go

@@ -41,6 +41,7 @@ import (
 	"fmt"
 
 	"github.com/github/gh-aw/pkg/logger"
+	"github.com/github/gh-aw/pkg/typeutil"
 	"github.com/goccy/go-yaml"
 )
 
@@ -182,18 +183,14 @@ func preprocessExpiresField(configData map[string]any, log *logger.Logger) bool
 // Returns the boolean value, or false if not present or invalid.
 // If log is provided, it will log the extracted value for debugging.
 func ParseBoolFromConfig(m map[string]any, key string, log *logger.Logger) bool {
-	if value, exists := m[key]; exists {
-		if log != nil {
-			log.Printf("Parsing %s from config", key)
-		}
-		if boolValue, ok := value.(bool); ok {
-			if log != nil {
-				log.Printf("Parsed %s from config: %t", key, boolValue)
-			}
-			return boolValue
-		}
+	if log != nil {
+		log.Printf("Parsing %s from config", key)
+	}
+	result := typeutil.ParseBool(m, key)
+	if log != nil {
+		log.Printf("Parsed %s from config: %t", key, result)
 	}
-	return false
+	return result
 }
 
 // unmarshalConfig unmarshals a config value from a map into a typed struct using YAML.

pkg/workflow/frontmatter_extraction_metadata.go

@@ -7,6 +7,7 @@ import (
 	"strings"
 
 	"github.com/github/gh-aw/pkg/logger"
+	"github.com/github/gh-aw/pkg/typeutil"
 )
 
 var frontmatterMetadataLog = logger.New("workflow:frontmatter_extraction_metadata")
@@ -172,9 +173,9 @@ func (c *Compiler) extractToolsTimeout(tools map[string]any) (string, error) {
 		case int64:
 			timeout = int(v)
 		case uint:
-			timeout = safeUintToInt(v) // Safe conversion to prevent overflow (alert #418)
+			timeout = typeutil.SafeUintToInt(v) // Safe conversion to prevent overflow (alert #418)
 		case uint64:
-			timeout = safeUint64ToInt(v) // Safe conversion to prevent overflow (alert #416)
+			timeout = typeutil.SafeUint64ToInt(v) // Safe conversion to prevent overflow (alert #416)
 		case float64:
 			timeout = int(v)
 		default:
@@ -221,9 +222,9 @@ func (c *Compiler) extractToolsStartupTimeout(tools map[string]any) (string, err
 		case int64:
 			timeout = int(v)
 		case uint:
-			timeout = safeUintToInt(v) // Safe conversion to prevent overflow (alert #417)
+			timeout = typeutil.SafeUintToInt(v) // Safe conversion to prevent overflow (alert #417)
 		case uint64:
-			timeout = safeUint64ToInt(v) // Safe conversion to prevent overflow (alert #415)
+			timeout = typeutil.SafeUint64ToInt(v) // Safe conversion to prevent overflow (alert #415)
 		case float64:
 			timeout = int(v)
 		default:

pkg/workflow/map_helpers.go

@@ -1,69 +1,39 @@
-// This file provides generic map and type conversion utilities.
+// This file provides generic map utilities.
 //
 // This file contains low-level helper functions for working with map[string]any
-// structures and type conversions. These utilities are used throughout the workflow
-// compilation process to safely parse and manipulate configuration data.
+// structures. These utilities are used throughout the workflow compilation process
+// to safely parse and manipulate configuration data.
 //
 // # Organization Rationale
 //
 // These functions are grouped in a helper file because they:
 //   - Provide generic, reusable utilities (used by 10+ files)
-//   - Have no specific domain focus (work with any map/type data)
+//   - Have no specific domain focus (work with any map data)
 //   - Are small, stable functions (< 50 lines each)
 //   - Follow clear, single-purpose patterns
 //
 // This follows the helper file conventions documented in skills/developer/SKILL.md.
 //
 // # Key Functions
 //
-// Type Conversion (delegated to pkg/typeutil for general-purpose reuse):
-//   - parseIntValue() - Strictly parse numeric types to int; returns (value, ok). Use when
-//     the caller needs to distinguish "missing/invalid" from a zero value, or when string
-//     inputs are not expected (e.g. YAML config field parsing). Delegates to typeutil.ParseIntValue.
-//   - safeUint64ToInt() - Convert uint64 to int, returning 0 on overflow. Delegates to typeutil.SafeUint64ToInt.
-//   - safeUintToInt() - Convert uint to int, returning 0 on overflow. Delegates to typeutil.SafeUintToInt.
-//   - ConvertToInt() - Leniently convert any value (int/int64/float64/string) to int, returning 0
-//     on failure. Use when the input may come from heterogeneous sources such as JSON metrics,
-//     log parsing, or user-provided strings where a zero default on failure is acceptable.
-//     Delegates to typeutil.ConvertToInt.
-//   - ConvertToFloat() - Safely convert any value (float64/int/int64/string) to float64.
-//     Delegates to typeutil.ConvertToFloat.
-//
 // Map Operations:
 //   - excludeMapKeys() - Create new map excluding specified keys
 //   - sortedMapKeys() - Return sorted keys of a map[string]string
 //
-// These utilities handle common type conversion and map manipulation patterns that
-// occur frequently during YAML-to-struct parsing and configuration processing.
-
-package workflow
-
-import (
-	"sort"
-
-	"github.com/github/gh-aw/pkg/typeutil"
-)
-
-// parseIntValue strictly parses numeric types to int, returning (value, true) on success
-// and (0, false) for any unrecognized or non-numeric type.
-//
-// Use this when the caller needs to distinguish a missing/invalid value from a legitimate
-// zero, or when string inputs are not expected (e.g. YAML config field parsing where the
-// YAML library has already produced a typed numeric value).
-//
-// For lenient conversion that also handles string inputs and returns 0 on failure, use
-// ConvertToInt instead.
+// For type conversion utilities, use pkg/typeutil directly:
+//   - typeutil.ParseIntValue() - Strictly parse numeric types to int; returns (value, ok).
+//   - typeutil.SafeUint64ToInt() - Convert uint64 to int, returning 0 on overflow.
+//   - typeutil.SafeUintToInt() - Convert uint to int, returning 0 on overflow.
+//   - typeutil.ConvertToInt() - Leniently convert any value to int, returning 0 on failure.
+//   - typeutil.ConvertToFloat() - Safely convert any value to float64.
+//   - typeutil.ParseBool() - Extract a bool from map[string]any by key.
 //
-// This is a package-private alias for typeutil.ParseIntValue.
-func parseIntValue(value any) (int, bool) { return typeutil.ParseIntValue(value) }
+// These utilities handle common map manipulation patterns that occur frequently
+// during YAML-to-struct parsing and configuration processing.
 
-// safeUint64ToInt safely converts uint64 to int, returning 0 if overflow would occur.
-// This is a package-private alias for typeutil.SafeUint64ToInt.
-func safeUint64ToInt(u uint64) int { return typeutil.SafeUint64ToInt(u) }
+package workflow
 
-// safeUintToInt safely converts uint to int, returning 0 if overflow would occur.
-// This is a package-private alias for typeutil.SafeUintToInt.
-func safeUintToInt(u uint) int { return typeutil.SafeUintToInt(u) }
+import "sort"
 
 // excludeMapKeys creates a new map excluding the specified keys
 func excludeMapKeys(original map[string]any, excludeKeys ...string) map[string]any {
@@ -91,26 +61,3 @@ func sortedMapKeys(m map[string]string) []string {
 	sort.Strings(keys)
 	return keys
 }
-
-// ConvertToInt leniently converts any value to int, returning 0 on failure.
-//
-// Unlike parseIntValue, this function also handles string inputs via strconv.Atoi,
-// making it suitable for heterogeneous sources such as JSON metrics, log-parsed data,
-// or user-provided configuration where a zero default on failure is acceptable and
-// the caller does not need to distinguish "invalid" from a genuine zero.
-//
-// For strict numeric-only parsing where the caller must distinguish missing/invalid
-// values from zero, use parseIntValue instead.
-//
-// This is a workflow-package alias for typeutil.ConvertToInt. For new code outside
-// this package, prefer using typeutil.ConvertToInt directly.
-func ConvertToInt(val any) int { return typeutil.ConvertToInt(val) }
-
-// ConvertToFloat safely converts any value to float64, returning 0 on failure.
-//
-// Supported input types: float64, int, int64, and string (parsed via strconv.ParseFloat).
-// Returns 0 for any other type or for strings that cannot be parsed as a float.
-//
-// This is a workflow-package alias for typeutil.ConvertToFloat. For new code outside
-// this package, prefer using typeutil.ConvertToFloat directly.
-func ConvertToFloat(val any) float64 { return typeutil.ConvertToFloat(val) }