Phase 1: Introduce EngineDefinition, EngineCatalog, and ResolvedEngineTarget

pkg/workflow/compiler_orchestrator_engine.go

@@ -199,19 +199,17 @@ func (c *Compiler) setupEngineAndImports(result *parser.FrontmatterResult, clean
 		}
 	}
 
-	// Validate the engine setting
-	orchestratorEngineLog.Printf("Validating engine setting: %s", engineSetting)
-	if err := c.validateEngine(engineSetting); err != nil {
-		orchestratorEngineLog.Printf("Engine validation failed: %v", err)
-		return nil, err
-	}
-
-	// Get the agentic engine instance
-	agenticEngine, err := c.getAgenticEngine(engineSetting)
+	// Validate the engine setting and resolve the runtime adapter via the catalog.
+	// This performs exact catalog lookup, prefix fallback, and returns a formatted
+	// validation error for unknown engines — replacing the separate validateEngine
+	// and getAgenticEngine calls.
+	orchestratorEngineLog.Printf("Resolving engine setting: %s", engineSetting)
+	resolvedEngine, err := c.engineCatalog.Resolve(engineSetting, engineConfig)
 	if err != nil {
-		orchestratorEngineLog.Printf("Failed to get agentic engine: %v", err)
+		orchestratorEngineLog.Printf("Engine resolution failed: %v", err)
 		return nil, err
 	}
+	agenticEngine := resolvedEngine.Runtime
 
 	log.Printf("AI engine: %s (%s)", agenticEngine.GetDisplayName(), engineSetting)
 	if agenticEngine.IsExperimental() && c.verbose {

pkg/workflow/compiler_types.go

@@ -98,6 +98,7 @@ type Compiler struct {
 	actionTag               string              // Override action SHA or tag for actions/setup (when set, overrides actionMode to release)
 	jobManager              *JobManager         // Manages jobs and dependencies
 	engineRegistry          *EngineRegistry     // Registry of available agentic engines
+	engineCatalog           *EngineCatalog      // Catalog of engine definitions backed by the registry
 	fileTracker             FileTracker         // Optional file tracker for tracking created files
 	warningCount            int                 // Number of warnings encountered during compilation
 	stepOrderTracker        *StepOrderTracker   // Tracks step ordering for validation
@@ -136,6 +137,7 @@ func NewCompiler(opts ...CompilerOption) *Compiler {
 		actionMode:        DetectActionMode(version), // Auto-detect action mode based on version
 		jobManager:        NewJobManager(),
 		engineRegistry:    GetGlobalEngineRegistry(),
+		engineCatalog:     NewEngineCatalog(GetGlobalEngineRegistry()),
 		stepOrderTracker:  NewStepOrderTracker(),
 		artifactManager:   NewArtifactManager(),
 		actionPinWarnings: make(map[string]bool), // Initialize warning cache

pkg/workflow/engine_definition.go

@@ -0,0 +1,185 @@
+// This file defines the engine definition layer: declarative metadata types for AI engines,
+// a catalog of registered definitions, and a resolved target that combines definition,
+// config, and runtime adapter.
+//
+// # Architecture
+//
+// The engine definition layer sits on top of the existing EngineRegistry runtime layer:
+//
+//	EngineDefinition  – declarative metadata for a single engine entry
+//	EngineCatalog     – registry of EngineDefinition entries with a Resolve() method
+//	ResolvedEngineTarget – result of resolving an engine ID: definition + config + runtime
+//
+// The existing EngineRegistry and CodingAgentEngine interfaces are unchanged; the catalog
+// is an additional layer that maps logical engine IDs to runtime adapters.
+//
+// # Built-in Engines
+//
+// NewEngineCatalog registers the four built-in engines: claude, codex, copilot, gemini.
+// Each EngineDefinition carries the engine's RuntimeID which maps to the corresponding
+// CodingAgentEngine registered in the EngineRegistry.
+//
+// # Resolve()
+//
+// EngineCatalog.Resolve() performs:
+//  1. Exact catalog ID lookup
+//  2. Runtime-ID prefix fallback (for backward compat, e.g. "codex-experimental")
+//  3. Formatted validation error when engine is unknown
+package workflow
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/github/gh-aw/pkg/constants"
+	"github.com/github/gh-aw/pkg/parser"
+)
+
+// ProviderSelection identifies the AI provider for an engine (e.g. "anthropic", "openai").
+type ProviderSelection struct {
+	Name string
+}
+
+// ModelSelection specifies the default and supported models for an engine.
+type ModelSelection struct {
+	Default   string
+	Supported []string
+}
+
+// AuthBinding maps a logical authentication role to a secret name.
+type AuthBinding struct {
+	Role   string
+	Secret string
+}
+
+// EngineDefinition holds the declarative metadata for an AI engine.
+// It is separate from the runtime adapter (CodingAgentEngine) to allow the catalog
+// layer to carry identity and provider information without coupling to implementation.
+type EngineDefinition struct {
+	ID          string
+	DisplayName string
+	Description string
+	RuntimeID   string // maps to the CodingAgentEngine registered in EngineRegistry
+	Provider    ProviderSelection
+	Models      ModelSelection
+	Auth        []AuthBinding
+	Options     map[string]any
+}
+
+// EngineCatalog is a collection of EngineDefinition entries backed by an EngineRegistry
+// for runtime adapter resolution.
+type EngineCatalog struct {
+	definitions map[string]*EngineDefinition
+	registry    *EngineRegistry
+}
+
+// ResolvedEngineTarget is the result of resolving an engine ID through the catalog.
+// It combines the EngineDefinition, the caller-supplied EngineConfig, and the resolved
+// CodingAgentEngine runtime adapter.
+type ResolvedEngineTarget struct {
+	Definition *EngineDefinition
+	Config     *EngineConfig     // resolved merged config supplied by the caller
+	Runtime    CodingAgentEngine // resolved adapter from the EngineRegistry
+}
+
+// NewEngineCatalog creates an EngineCatalog that wraps the given EngineRegistry and
+// pre-registers the four built-in engine definitions (claude, codex, copilot, gemini).
+func NewEngineCatalog(registry *EngineRegistry) *EngineCatalog {
+	catalog := &EngineCatalog{
+		definitions: make(map[string]*EngineDefinition),
+		registry:    registry,
+	}
+
+	catalog.Register(&EngineDefinition{
+		ID:          "claude",
+		DisplayName: "Claude Code",
+		Description: "Uses Claude Code with full MCP tool support and allow-listing",
+		RuntimeID:   "claude",
+		Provider:    ProviderSelection{Name: "anthropic"},
+		Auth: []AuthBinding{
+			{Role: "api-key", Secret: "ANTHROPIC_API_KEY"},
+		},
+	})
+
+	catalog.Register(&EngineDefinition{
+		ID:          "codex",
+		DisplayName: "Codex",
+		Description: "Uses OpenAI Codex CLI with MCP server support",
+		RuntimeID:   "codex",
+		Provider:    ProviderSelection{Name: "openai"},
+		Auth: []AuthBinding{
+			{Role: "api-key", Secret: "CODEX_API_KEY"},
+		},
+	})
+
+	catalog.Register(&EngineDefinition{
+		ID:          "copilot",
+		DisplayName: "GitHub Copilot CLI",
+		Description: "Uses GitHub Copilot CLI with MCP server support",
+		RuntimeID:   "copilot",
+		Provider:    ProviderSelection{Name: "github"},
+	})
+
+	catalog.Register(&EngineDefinition{
+		ID:          "gemini",
+		DisplayName: "Google Gemini CLI",
+		Description: "Google Gemini CLI with headless mode and LLM gateway support",
+		RuntimeID:   "gemini",
+		Provider:    ProviderSelection{Name: "google"},
+	})
+
+	return catalog
+}
+
+// Register adds or replaces an EngineDefinition in the catalog.
+func (c *EngineCatalog) Register(def *EngineDefinition) {
+	c.definitions[def.ID] = def
+}
+
+// Resolve returns a ResolvedEngineTarget for the given engine ID and config.
+// Resolution order:
+//  1. Exact match in the catalog by ID
+//  2. Prefix match in the underlying EngineRegistry (backward compat, e.g. "codex-experimental")
+//  3. Returns a formatted validation error when no match is found
+func (c *EngineCatalog) Resolve(id string, config *EngineConfig) (*ResolvedEngineTarget, error) {
+	// Exact catalog lookup
+	if def, ok := c.definitions[id]; ok {
+		runtime, err := c.registry.GetEngine(def.RuntimeID)
+		if err != nil {
+			return nil, fmt.Errorf("engine %q definition references unknown runtime %q: %w", id, def.RuntimeID, err)
+		}
+		return &ResolvedEngineTarget{Definition: def, Config: config, Runtime: runtime}, nil
+	}
+
+	// Fall back to runtime-ID prefix lookup for backward compat (e.g. "codex-experimental")
+	runtime, err := c.registry.GetEngineByPrefix(id)
+	if err == nil {
+		def := &EngineDefinition{
+			ID:          id,
+			DisplayName: runtime.GetDisplayName(),
+			Description: runtime.GetDescription(),
+			RuntimeID:   runtime.GetID(),
+		}
+		return &ResolvedEngineTarget{Definition: def, Config: config, Runtime: runtime}, nil
+	}
+
+	// Engine not found — produce a helpful validation error matching the existing format
+	validEngines := c.registry.GetSupportedEngines()
+	suggestions := parser.FindClosestMatches(id, validEngines, 1)
+	enginesStr := strings.Join(validEngines, ", ")
+
+	errMsg := fmt.Sprintf("invalid engine: %s. Valid engines are: %s.\n\nExample:\nengine: copilot\n\nSee: %s",
+		id,
+		enginesStr,
+		constants.DocsEnginesURL)
+
+	if len(suggestions) > 0 {
+		errMsg = fmt.Sprintf("invalid engine: %s. Valid engines are: %s.\n\nDid you mean: %s?\n\nExample:\nengine: copilot\n\nSee: %s",
+			id,
+			enginesStr,
+			suggestions[0],
+			constants.DocsEnginesURL)
+	}
+
+	return nil, fmt.Errorf("%s", errMsg)
+}

pkg/workflow/engine_definition_test.go

@@ -0,0 +1,130 @@
+//go:build !integration
+
+package workflow
+
+import (
+	"testing"
+
+	"github.com/github/gh-aw/pkg/constants"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestNewEngineCatalog_BuiltIns checks that all four built-in engines are registered
+// and resolve to the expected runtime adapters.
+func TestNewEngineCatalog_BuiltIns(t *testing.T) {
+	registry := NewEngineRegistry()
+	catalog := NewEngineCatalog(registry)
+
+	tests := []struct {
+		engineID    string
+		displayName string
+		provider    string
+	}{
+		{"claude", "Claude Code", "anthropic"},
+		{"codex", "Codex", "openai"},
+		{"copilot", "GitHub Copilot CLI", "github"},
+		{"gemini", "Google Gemini CLI", "google"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.engineID, func(t *testing.T) {
+			resolved, err := catalog.Resolve(tt.engineID, &EngineConfig{ID: tt.engineID})
+			require.NoError(t, err, "expected %s to resolve without error", tt.engineID)
+			require.NotNil(t, resolved, "expected non-nil ResolvedEngineTarget for %s", tt.engineID)
+
+			assert.Equal(t, tt.engineID, resolved.Definition.ID, "Definition.ID should match")
+			assert.Equal(t, tt.displayName, resolved.Definition.DisplayName, "Definition.DisplayName should match")
+			assert.Equal(t, tt.provider, resolved.Definition.Provider.Name, "Definition.Provider.Name should match")
+			assert.Equal(t, tt.engineID, resolved.Runtime.GetID(), "Runtime.GetID() should match engine ID")
+		})
+	}
+}
+
+// TestEngineCatalog_Resolve_LegacyStringFormat verifies that resolving via plain string
+// ("engine: claude") and object ID format ("engine.id: claude") produce the same runtime.
+func TestEngineCatalog_Resolve_LegacyStringFormat(t *testing.T) {
+	registry := NewEngineRegistry()
+	catalog := NewEngineCatalog(registry)
+
+	// Simulate "engine: claude" — EngineConfig built from string
+	stringConfig := &EngineConfig{ID: "claude"}
+	resolvedString, err := catalog.Resolve("claude", stringConfig)
+	require.NoError(t, err, "string-format engine should resolve without error")
+
+	// Simulate "engine:\n  id: claude" — same logical ID
+	objectConfig := &EngineConfig{ID: "claude"}
+	resolvedObject, err := catalog.Resolve("claude", objectConfig)
+	require.NoError(t, err, "object-format engine should resolve without error")
+
+	assert.Equal(t, resolvedString.Runtime.GetID(), resolvedObject.Runtime.GetID(),
+		"both formats should resolve to the same runtime adapter")
+	assert.Equal(t, resolvedString.Definition.ID, resolvedObject.Definition.ID,
+		"both formats should resolve to the same definition")
+}
+
+// TestEngineCatalog_Resolve_PrefixFallback verifies backward-compat prefix matching
+// (e.g. "codex-experimental" should resolve to the codex runtime).
+func TestEngineCatalog_Resolve_PrefixFallback(t *testing.T) {
+	registry := NewEngineRegistry()
+	catalog := NewEngineCatalog(registry)
+
+	resolved, err := catalog.Resolve("codex-experimental", &EngineConfig{ID: "codex-experimental"})
+	require.NoError(t, err, "prefix-matched engine should resolve without error")
+	require.NotNil(t, resolved, "expected non-nil ResolvedEngineTarget for prefix match")
+
+	assert.Equal(t, "codex", resolved.Runtime.GetID(), "prefix match should resolve to codex runtime")
+}
+
+// TestEngineCatalog_Resolve_UnknownEngine verifies that unknown engine IDs return a
+// descriptive validation error containing the engine ID, a list of valid engines, and
+// the documentation URL.
+func TestEngineCatalog_Resolve_UnknownEngine(t *testing.T) {
+	registry := NewEngineRegistry()
+	catalog := NewEngineCatalog(registry)
+
+	_, err := catalog.Resolve("nonexistent-engine", &EngineConfig{ID: "nonexistent-engine"})
+	require.Error(t, err, "unknown engine should return an error")
+	assert.Contains(t, err.Error(), "invalid engine",
+		"error should mention 'invalid engine', got: %s", err.Error())
+	assert.Contains(t, err.Error(), "nonexistent-engine",
+		"error should mention the unknown engine ID, got: %s", err.Error())
+	assert.Contains(t, err.Error(), string(constants.DocsEnginesURL),
+		"error should include the engines documentation URL, got: %s", err.Error())
+	assert.Contains(t, err.Error(), "engine: copilot",
+		"error should include an example, got: %s", err.Error())
+}
+
+// TestEngineCatalog_Resolve_ConfigPassthrough verifies that the EngineConfig passed to
+// Resolve is surfaced unchanged in the ResolvedEngineTarget.
+func TestEngineCatalog_Resolve_ConfigPassthrough(t *testing.T) {
+	registry := NewEngineRegistry()
+	catalog := NewEngineCatalog(registry)
+
+	cfg := &EngineConfig{ID: "copilot", Model: "gpt-4o", MaxTurns: "10"}
+	resolved, err := catalog.Resolve("copilot", cfg)
+	require.NoError(t, err, "copilot with config should resolve without error")
+	assert.Equal(t, cfg, resolved.Config, "resolved Config should be the same pointer passed in")
+}
+
+// TestEngineCatalog_Register_Custom verifies that a custom engine definition can be
+// registered and resolved via the catalog.
+func TestEngineCatalog_Register_Custom(t *testing.T) {
+	registry := NewEngineRegistry()
+	// Register a test engine in the registry so the catalog can look it up
+	registry.Register(NewCopilotEngine()) // reuse copilot as the backing runtime
+
+	catalog := NewEngineCatalog(registry)
+	catalog.Register(&EngineDefinition{
+		ID:          "my-custom-engine",
+		DisplayName: "My Custom Engine",
+		Description: "A custom engine for testing",
+		RuntimeID:   "copilot", // backed by copilot runtime
+		Provider:    ProviderSelection{Name: "custom"},
+	})
+
+	resolved, err := catalog.Resolve("my-custom-engine", &EngineConfig{ID: "my-custom-engine"})
+	require.NoError(t, err, "custom engine should resolve without error")
+	assert.Equal(t, "my-custom-engine", resolved.Definition.ID, "custom engine definition ID should match")
+	assert.Equal(t, "copilot", resolved.Runtime.GetID(), "custom engine should use copilot runtime")
+}