Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 6 additions & 24 deletions pkg/cli/dependency_graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment on lines 248 to +255
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

findGitRootForPath assumes the input is a file path and unconditionally does filepath.Dir(absPath) internally. Passing g.workflowsDir (a directory) means the git command runs from the parent of the workflows directory, which can be incorrect (e.g., if workflowsDir is already the repo root, it will run from its parent and fail). Consider either updating findGitRootForPath to handle directory inputs or passing a known file path within g.workflowsDir so the working directory is correct.

This issue also appears on line 248 of the same file.

Copilot uses AI. Check for mistakes.
fullPath, err := parser.ResolveIncludePath(importPath, baseDir, importCache)
if err != nil {
depGraphLog.Printf("Failed to resolve import path %s: %v", importPath, err)
Expand All @@ -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 {
Expand Down
5 changes: 3 additions & 2 deletions pkg/cli/logs_parsing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down
10 changes: 2 additions & 8 deletions pkg/parser/frontmatter_hash.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions pkg/typeutil/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Comment on lines +116 to +127
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ParseBool is a new exported utility but there are no unit tests covering its behavior (nil map, missing key, non-bool value, true/false). Since pkg/typeutil already has convert_test.go covering the other helpers, add focused tests there to prevent regressions.

Copilot uses AI. Check for mistakes.

// ConvertToFloat safely converts any value to float64, returning 0 on failure.
//
// Supported input types: float64, int, int64, and string (parsed via strconv.ParseFloat).
Expand Down
25 changes: 13 additions & 12 deletions pkg/workflow/claude_logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
}
Expand Down
19 changes: 8 additions & 11 deletions pkg/workflow/config_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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.
Expand Down
9 changes: 5 additions & 4 deletions pkg/workflow/frontmatter_extraction_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
83 changes: 15 additions & 68 deletions pkg/workflow/map_helpers.go
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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) }
Loading
Loading