Skip to content

Commit 4160cfc

Browse files
authored
Refactor: Extract engine validation methods to dedicated file (#3491)
1 parent 51e8bef commit 4160cfc

2 files changed

Lines changed: 118 additions & 76 deletions

File tree

pkg/workflow/engine.go

Lines changed: 0 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -257,31 +257,6 @@ func (c *Compiler) ExtractEngineConfig(frontmatter map[string]any) (string, *Eng
257257
return "", nil
258258
}
259259

260-
// validateEngine validates that the given engine ID is supported
261-
func (c *Compiler) validateEngine(engineID string) error {
262-
if engineID == "" {
263-
engineLog.Print("No engine ID specified, will use default")
264-
return nil // Empty engine is valid (will use default)
265-
}
266-
267-
engineLog.Printf("Validating engine ID: %s", engineID)
268-
269-
// First try exact match
270-
if c.engineRegistry.IsValidEngine(engineID) {
271-
engineLog.Printf("Engine ID %s is valid (exact match)", engineID)
272-
return nil
273-
}
274-
275-
// Try prefix match for backward compatibility (e.g., "codex-experimental")
276-
engine, err := c.engineRegistry.GetEngineByPrefix(engineID)
277-
if err == nil {
278-
engineLog.Printf("Engine ID %s matched by prefix to: %s", engineID, engine.GetID())
279-
} else {
280-
engineLog.Printf("Engine ID %s not found: %v", engineID, err)
281-
}
282-
return err
283-
}
284-
285260
// getAgenticEngine returns the agentic engine for the given engine setting
286261
func (c *Compiler) getAgenticEngine(engineSetting string) (CodingAgentEngine, error) {
287262
if engineSetting == "" {
@@ -311,57 +286,6 @@ func (c *Compiler) getAgenticEngine(engineSetting string) (CodingAgentEngine, er
311286
return engine, err
312287
}
313288

314-
// validateSingleEngineSpecification validates that only one engine field exists across all files
315-
func (c *Compiler) validateSingleEngineSpecification(mainEngineSetting string, includedEnginesJSON []string) (string, error) {
316-
var allEngines []string
317-
318-
// Add main engine if specified
319-
if mainEngineSetting != "" {
320-
allEngines = append(allEngines, mainEngineSetting)
321-
}
322-
323-
// Add included engines
324-
for _, engineJSON := range includedEnginesJSON {
325-
if engineJSON != "" {
326-
allEngines = append(allEngines, engineJSON)
327-
}
328-
}
329-
330-
// Check count
331-
if len(allEngines) == 0 {
332-
return "", nil // No engine specified anywhere, will use default
333-
}
334-
335-
if len(allEngines) > 1 {
336-
return "", fmt.Errorf("multiple engine fields found. Only one engine field is allowed across the main workflow and all included files. Remove engine specifications to have only one")
337-
}
338-
339-
// Exactly one engine found - parse and return it
340-
if mainEngineSetting != "" {
341-
return mainEngineSetting, nil
342-
}
343-
344-
// Must be from included file
345-
var firstEngine any
346-
if err := json.Unmarshal([]byte(includedEnginesJSON[0]), &firstEngine); err != nil {
347-
return "", fmt.Errorf("failed to parse included engine configuration: %w", err)
348-
}
349-
350-
// Handle string format
351-
if engineStr, ok := firstEngine.(string); ok {
352-
return engineStr, nil
353-
} else if engineObj, ok := firstEngine.(map[string]any); ok {
354-
// Handle object format - return the ID
355-
if id, hasID := engineObj["id"]; hasID {
356-
if idStr, ok := id.(string); ok {
357-
return idStr, nil
358-
}
359-
}
360-
}
361-
362-
return "", fmt.Errorf("invalid engine configuration in included file")
363-
}
364-
365289
// extractEngineConfigFromJSON parses engine configuration from JSON string (from included files)
366290
func (c *Compiler) extractEngineConfigFromJSON(engineJSON string) (*EngineConfig, error) {
367291
if engineJSON == "" {

pkg/workflow/engine_validation.go

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
// Package workflow provides engine validation for agentic workflows.
2+
//
3+
// # Engine Validation
4+
//
5+
// This file validates engine configurations used in agentic workflows.
6+
// Validation ensures that engine IDs are supported and that only one engine
7+
// specification exists across the main workflow and all included files.
8+
//
9+
// # Validation Functions
10+
//
11+
// - validateEngine() - Validates that a given engine ID is supported
12+
// - validateSingleEngineSpecification() - Validates that only one engine field exists across all files
13+
//
14+
// # Validation Pattern: Engine Registry
15+
//
16+
// Engine validation uses the compiler's engine registry:
17+
// - Supports exact engine ID matching (e.g., "copilot", "claude")
18+
// - Supports prefix matching for backward compatibility (e.g., "codex-experimental")
19+
// - Empty engine IDs are valid and use the default engine
20+
// - Detailed logging of validation steps for debugging
21+
//
22+
// # When to Add Validation Here
23+
//
24+
// Add validation to this file when:
25+
// - It validates engine IDs or engine configurations
26+
// - It checks engine registry entries
27+
// - It validates engine-specific settings
28+
// - It validates engine field consistency across imports
29+
//
30+
// For engine configuration extraction, see engine.go.
31+
// For general validation, see validation.go.
32+
// For detailed documentation, see specs/validation-architecture.md
33+
package workflow
34+
35+
import (
36+
"encoding/json"
37+
"fmt"
38+
39+
"github.com/githubnext/gh-aw/pkg/logger"
40+
)
41+
42+
var engineValidationLog = logger.New("workflow:engine_validation")
43+
44+
// validateEngine validates that the given engine ID is supported
45+
func (c *Compiler) validateEngine(engineID string) error {
46+
if engineID == "" {
47+
engineValidationLog.Print("No engine ID specified, will use default")
48+
return nil // Empty engine is valid (will use default)
49+
}
50+
51+
engineValidationLog.Printf("Validating engine ID: %s", engineID)
52+
53+
// First try exact match
54+
if c.engineRegistry.IsValidEngine(engineID) {
55+
engineValidationLog.Printf("Engine ID %s is valid (exact match)", engineID)
56+
return nil
57+
}
58+
59+
// Try prefix match for backward compatibility (e.g., "codex-experimental")
60+
engine, err := c.engineRegistry.GetEngineByPrefix(engineID)
61+
if err == nil {
62+
engineValidationLog.Printf("Engine ID %s matched by prefix to: %s", engineID, engine.GetID())
63+
} else {
64+
engineValidationLog.Printf("Engine ID %s not found: %v", engineID, err)
65+
}
66+
return err
67+
}
68+
69+
// validateSingleEngineSpecification validates that only one engine field exists across all files
70+
func (c *Compiler) validateSingleEngineSpecification(mainEngineSetting string, includedEnginesJSON []string) (string, error) {
71+
var allEngines []string
72+
73+
// Add main engine if specified
74+
if mainEngineSetting != "" {
75+
allEngines = append(allEngines, mainEngineSetting)
76+
}
77+
78+
// Add included engines
79+
for _, engineJSON := range includedEnginesJSON {
80+
if engineJSON != "" {
81+
allEngines = append(allEngines, engineJSON)
82+
}
83+
}
84+
85+
// Check count
86+
if len(allEngines) == 0 {
87+
return "", nil // No engine specified anywhere, will use default
88+
}
89+
90+
if len(allEngines) > 1 {
91+
return "", fmt.Errorf("multiple engine fields found. Only one engine field is allowed across the main workflow and all included files. Remove engine specifications to have only one")
92+
}
93+
94+
// Exactly one engine found - parse and return it
95+
if mainEngineSetting != "" {
96+
return mainEngineSetting, nil
97+
}
98+
99+
// Must be from included file
100+
var firstEngine any
101+
if err := json.Unmarshal([]byte(includedEnginesJSON[0]), &firstEngine); err != nil {
102+
return "", fmt.Errorf("failed to parse included engine configuration: %w", err)
103+
}
104+
105+
// Handle string format
106+
if engineStr, ok := firstEngine.(string); ok {
107+
return engineStr, nil
108+
} else if engineObj, ok := firstEngine.(map[string]any); ok {
109+
// Handle object format - return the ID
110+
if id, hasID := engineObj["id"]; hasID {
111+
if idStr, ok := id.(string); ok {
112+
return idStr, nil
113+
}
114+
}
115+
}
116+
117+
return "", fmt.Errorf("invalid engine configuration in included file")
118+
}

0 commit comments

Comments
 (0)