refactor: consolidate duplicate and misplaced functions identified in semantic clustering analysis

[Copilot]

Three functions were either exact duplicates across files or semantically misplaced relative to their containing file.

Changes

• safeUint64ToInt (exact duplicate): safeUint64ToIntForTimeout in mcp_scripts_parser.go was identical to safeUint64ToInt in frontmatter_extraction_metadata.go. Moved the canonical version to map_helpers.go (the existing home for numeric type conversion utilities), removed both local copies, updated callers.

• containsWorkflowDispatch / containsWorkflowCall (near-duplicate): Identical 15-line type-switch differing only by trigger string constant. Extracted containsTrigger(onSection any, triggerName string) bool into validation_helpers.go; both functions now delegate:

  func containsWorkflowDispatch(onSection any) bool {
      return containsTrigger(onSection, "workflow_dispatch")
  }

• formatYAMLValue (outlier): A 46-line YAML value serializer lived in runtime_step_generator.go. Moved to yaml.go alongside MarshalWithFieldOrder, OrderMapFields, and CleanYAMLNullValues.

Tests

Added table-driven tests for all three refactored functions:

• TestSafeUint64ToInt (map_helpers_test.go) — covers zero, normal values, max int boundary, and overflow cases (max uint64 and max int+1 both return 0).
• TestContainsTrigger (validation_helpers_test.go) — covers all three on: section forms (string / []any / map[string]any), nil input, unsupported types, and both workflow_dispatch / workflow_call trigger names.
• TestFormatYAMLValue (yaml_test.go) — covers every type branch: YAML keyword strings, numeric strings, booleans, all signed/unsigned integer widths, floats, and the default fallback.

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[pelikhan]

@copilot add tests for all refactored functions

[Copilot]

Pull request overview

Refactors workflow compilation helpers by consolidating duplicate/misplaced functions into shared helper files to reduce duplication and improve organization.

Changes:

• Moved formatYAMLValue from runtime_step_generator.go into yaml.go.
• Consolidated containsWorkflowDispatch/containsWorkflowCall logic via new containsTrigger helper in validation_helpers.go.
• Centralized safeUint64ToInt into map_helpers.go and updated callers (removing local duplicates).

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pkg/workflow/yaml.go	Adds shared formatYAMLValue YAML scalar formatting helper.
pkg/workflow/validation_helpers.go	Introduces containsTrigger helper for on: trigger checks.
pkg/workflow/runtime_step_generator.go	Removes local formatYAMLValue after moving to yaml.go.
pkg/workflow/mcp_scripts_parser.go	Switches timeout uint64 conversion to shared safeUint64ToInt.
pkg/workflow/map_helpers.go	Adds shared safeUint64ToInt numeric conversion helper.
pkg/workflow/frontmatter_extraction_metadata.go	Removes local safeUint64ToInt duplicate.
pkg/workflow/dispatch_workflow_validation.go	Delegates dispatch trigger detection to containsTrigger.
pkg/workflow/call_workflow_validation.go	Delegates call trigger detection to containsTrigger.

Comments suppressed due to low confidence (1)

pkg/workflow/yaml.go:360

• formatYAMLValue is now a shared YAML-serialization helper but there are no unit tests covering its quoting/escaping behavior (especially values containing single quotes, ${{ ... }} expressions, and numeric-looking strings). Adding focused tests in yaml_test.go would help prevent regressions and ensure the generated YAML remains valid.

// formatYAMLValue formats a value for YAML output, quoting strings and rendering
// booleans/numbers as unquoted scalars.
func formatYAMLValue(value any) string {
	switch v := value.(type) {
	case string:
		// Quote strings if they contain special characters or look like non-string types
		if v == "true" || v == "false" || v == "null" {
			return fmt.Sprintf("'%s'", v)
		}
		// Check if it's a number
		if _, err := fmt.Sscanf(v, "%f", new(float64)); err == nil {
			return fmt.Sprintf("'%s'", v)
		}
		// Return as-is for simple strings, quote for complex ones
		return fmt.Sprintf("'%s'", v)
	case bool:
		if v {
			return "true"
		}
		return "false"
	case int:
		return strconv.Itoa(v)
	case int8:
		return strconv.Itoa(int(v))
	case int16:
		return strconv.Itoa(int(v))
	case int32:
		return strconv.Itoa(int(v))
	case int64:
		return strconv.FormatInt(v, 10)
	case uint:
		return strconv.FormatUint(uint64(v), 10)
	case uint8:
		return strconv.FormatUint(uint64(v), 10)
	case uint16:
		return strconv.FormatUint(uint64(v), 10)
	case uint32:
		return strconv.FormatUint(uint64(v), 10)
	case uint64:
		return strconv.FormatUint(v, 10)
	case float32:
		return fmt.Sprintf("%v", v)
	case float64:
		return fmt.Sprintf("%v", v)
	default:
		// For other types, convert to string and quote
		return fmt.Sprintf("'%v'", v)
	}
}

---

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

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

@copilot add tests for all refactored functions

Added in 160d0f9:

• TestSafeUint64ToInt (map_helpers_test.go) — 6 cases: zero, typical value, large-within-range, max int, max uint64 overflow → 0, max int+1 overflow → 0
• TestContainsTrigger (validation_helpers_test.go) — 16 cases covering all three on: forms (string / []any / map[string]any), nil, unsupported type, and both workflow_dispatch / workflow_call trigger names
• TestFormatYAMLValue (yaml_test.go) — 25 cases covering every type branch: YAML keyword strings, numeric strings, bool, all int/uint widths, floats, and the default fallback