[WIP] Fix plugin validation error for Claude and Codex

pkg/workflow/claude_engine.go

@@ -30,6 +30,7 @@ func NewClaudeEngine() *ClaudeEngine {
 			supportsWebFetch:       true, // Claude has built-in WebFetch support
 			supportsWebSearch:      true, // Claude has built-in WebSearch support
 			supportsFirewall:       true, // Claude supports network firewalling via AWF
+			supportsPlugins:        true, // Claude supports plugin installation
 		},
 	}
 }
@@ -126,6 +127,17 @@ func (e *ClaudeEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHub
 		steps = append(steps, npmSteps[1:]...) // Install Claude CLI and subsequent steps
 	}
 
+	// Add plugin installation steps if plugins are specified
+	if workflowData.PluginInfo != nil && len(workflowData.PluginInfo.Plugins) > 0 {
+		// Use plugin-specific token if provided, otherwise use top-level github-token
+		tokenToUse := workflowData.PluginInfo.CustomToken
+		if tokenToUse == "" {
+			tokenToUse = workflowData.GitHubToken
+		}
+		pluginSteps := GeneratePluginInstallationSteps(workflowData.PluginInfo.Plugins, "claude", tokenToUse)
+		steps = append(steps, pluginSteps...)
+	}
+
 	return steps
 }
 

pkg/workflow/codex_engine.go

@@ -41,6 +41,7 @@ func NewCodexEngine() *CodexEngine {
 			supportsWebFetch:       false, // Codex does not have built-in web-fetch support
 			supportsWebSearch:      true,  // Codex has built-in web-search support
 			supportsFirewall:       true,  // Codex supports network firewalling via AWF
+			supportsPlugins:        true,  // Codex supports plugin installation
 		},
 	}
 }
@@ -101,6 +102,17 @@ func (e *CodexEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHubA
 		}
 	}
 
+	// Add plugin installation steps if plugins are specified
+	if workflowData.PluginInfo != nil && len(workflowData.PluginInfo.Plugins) > 0 {
+		// Use plugin-specific token if provided, otherwise use top-level github-token
+		tokenToUse := workflowData.PluginInfo.CustomToken
+		if tokenToUse == "" {
+			tokenToUse = workflowData.GitHubToken
+		}
+		pluginSteps := GeneratePluginInstallationSteps(workflowData.PluginInfo.Plugins, "codex", tokenToUse)
+		steps = append(steps, pluginSteps...)
+	}
+
 	return steps
 }
 

pkg/workflow/copilot_engine_test.go

@@ -88,19 +88,25 @@ func TestOtherEnginesNoDefaultDetectionModel(t *testing.T) {
 	}
 }
 
-func TestOtherEnginesNoPluginSupport(t *testing.T) {
-	// Test that only Copilot engine supports plugins
-	engines := []CodingAgentEngine{
+func TestEnginesWithPluginSupport(t *testing.T) {
+	// Test that Copilot, Claude, and Codex engines support plugins
+	supportedEngines := []CodingAgentEngine{
+		NewCopilotEngine(),
 		NewClaudeEngine(),
 		NewCodexEngine(),
-		NewCustomEngine(),
 	}
 
-	for _, engine := range engines {
-		if engine.SupportsPlugins() {
-			t.Errorf("Expected engine '%s' to not support plugins, but it does", engine.GetID())
+	for _, engine := range supportedEngines {
+		if !engine.SupportsPlugins() {
+			t.Errorf("Expected engine '%s' to support plugins, but it doesn't", engine.GetID())
 		}
 	}
+
+	// Test that custom engine does not support plugins
+	customEngine := NewCustomEngine()
+	if customEngine.SupportsPlugins() {
+		t.Errorf("Expected custom engine to not support plugins, but it does")
+	}
 }
 
 func TestCopilotEngineInstallationSteps(t *testing.T) {

pkg/workflow/engine_validation_test.go

@@ -419,22 +419,20 @@ func TestValidatePluginSupport(t *testing.T) {
 			expectError: false,
 		},
 		{
-			name: "plugins with claude engine (not supported)",
+			name: "plugins with claude engine (supported)",
 			pluginInfo: &PluginInfo{
 				Plugins: []string{"org/plugin1"},
 			},
 			engineID:    "claude",
-			expectError: true,
-			errorMsg:    "does not support plugins",
+			expectError: false,
 		},
 		{
-			name: "plugins with codex engine (not supported)",
+			name: "plugins with codex engine (supported)",
 			pluginInfo: &PluginInfo{
 				Plugins: []string{"org/plugin1", "org/plugin2"},
 			},
 			engineID:    "codex",
-			expectError: true,
-			errorMsg:    "does not support plugins",
+			expectError: false,
 		},
 		{
 			name: "plugins with custom engine (not supported)",
@@ -479,24 +477,24 @@ func TestValidatePluginSupport(t *testing.T) {
 // TestValidatePluginSupportErrorMessage verifies the plugin validation error message quality
 func TestValidatePluginSupportErrorMessage(t *testing.T) {
 	compiler := NewCompiler()
-	claudeEngine, err := compiler.engineRegistry.GetEngine("claude")
+	customEngine, err := compiler.engineRegistry.GetEngine("custom")
 	if err != nil {
-		t.Fatalf("Failed to get claude engine: %v", err)
+		t.Fatalf("Failed to get custom engine: %v", err)
 	}
 
 	pluginInfo := &PluginInfo{
 		Plugins: []string{"org/plugin1", "org/plugin2"},
 	}
 
-	err = compiler.validatePluginSupport(pluginInfo, claudeEngine)
+	err = compiler.validatePluginSupport(pluginInfo, customEngine)
 	if err == nil {
-		t.Fatal("Expected validation to fail for plugins with claude engine")
+		t.Fatal("Expected validation to fail for plugins with custom engine")
 	}
 
 	errorMsg := err.Error()
 
 	// Error should mention the engine name
-	if !strings.Contains(errorMsg, "claude") {
+	if !strings.Contains(errorMsg, "custom") {
 		t.Errorf("Error message should mention the engine name, got: %s", errorMsg)
 	}
 
@@ -505,13 +503,84 @@ func TestValidatePluginSupportErrorMessage(t *testing.T) {
 		t.Errorf("Error message should list the plugins, got: %s", errorMsg)
 	}
 
-	// Error should mention copilot as a supported engine (since it's the only one that supports plugins)
+	// Error should mention the engines that support plugins (copilot, claude, codex)
 	if !strings.Contains(errorMsg, "copilot") {
 		t.Errorf("Error message should mention copilot as supported engine, got: %s", errorMsg)
 	}
+	if !strings.Contains(errorMsg, "claude") {
+		t.Errorf("Error message should mention claude as supported engine, got: %s", errorMsg)
+	}
+	if !strings.Contains(errorMsg, "codex") {
+		t.Errorf("Error message should mention codex as supported engine, got: %s", errorMsg)
+	}
 
 	// Error should provide actionable fixes
 	if !strings.Contains(errorMsg, "To fix this") {
 		t.Errorf("Error message should provide actionable fixes, got: %s", errorMsg)
 	}
 }
+
+// TestValidatePluginSupportErrorMessageListsAllSupportedEngines is a regression test
+// that ensures the validation error message dynamically lists all engines that support plugins.
+// This test will fail if a new engine with plugin support is added but the validation message
+// doesn't include it, ensuring the error messages stay in sync with engine capabilities.
+func TestValidatePluginSupportErrorMessageListsAllSupportedEngines(t *testing.T) {
+	compiler := NewCompiler()
+
+	// Get all engines that support plugins from the engine registry
+	var supportedEngines []string
+	for _, engineID := range compiler.engineRegistry.GetSupportedEngines() {
+		if engine, err := compiler.engineRegistry.GetEngine(engineID); err == nil {
+			if engine.SupportsPlugins() {
+				supportedEngines = append(supportedEngines, engineID)
+			}
+		}
+	}
+
+	// Ensure we have at least one engine that supports plugins
+	if len(supportedEngines) == 0 {
+		t.Fatal("Expected at least one engine to support plugins")
+	}
+
+	t.Logf("Engines that support plugins: %v", supportedEngines)
+
+	// Test with an engine that doesn't support plugins (custom engine)
+	customEngine, err := compiler.engineRegistry.GetEngine("custom")
+	if err != nil {
+		t.Fatalf("Failed to get custom engine: %v", err)
+	}
+
+	pluginInfo := &PluginInfo{
+		Plugins: []string{"org/test-plugin"},
+	}
+
+	err = compiler.validatePluginSupport(pluginInfo, customEngine)
+	if err == nil {
+		t.Fatal("Expected validation to fail for plugins with custom engine")
+	}
+
+	errorMsg := err.Error()
+
+	// Verify that the error message includes ALL engines that support plugins
+	for _, engineID := range supportedEngines {
+		if !strings.Contains(errorMsg, engineID) {
+			t.Errorf("Error message should list engine '%s' as supporting plugins, but it's missing.\nError message: %s",
+				engineID, errorMsg)
+		}
+	}
+
+	// Verify the error message uses the correct phrasing based on number of supported engines
+	if len(supportedEngines) == 1 {
+		expectedPhrase := "Only the '" + supportedEngines[0] + "' engine supports plugin installation"
+		if !strings.Contains(errorMsg, expectedPhrase) {
+			t.Errorf("Error message should contain phrase '%s' for single supported engine, got: %s",
+				expectedPhrase, errorMsg)
+		}
+	} else {
+		expectedPhrase := "The following engines support plugin installation:"
+		if !strings.Contains(errorMsg, expectedPhrase) {
+			t.Errorf("Error message should contain phrase '%s' for multiple supported engines, got: %s",
+				expectedPhrase, errorMsg)
+		}
+	}
+}

pkg/workflow/plugin_import_test.go

@@ -308,7 +308,7 @@ This workflow imports plugins from a shared workflow.
 		"Expected Copilot engine to install plugin from import")
 }
 
-func TestCompileWorkflowWithPluginImportsClaudeEngineRejectsPlugins(t *testing.T) {
+func TestCompileWorkflowWithPluginImportsClaudeEngineSupportsPlugins(t *testing.T) {
 	// Create a temporary directory for test files
 	tempDir := testutil.TempDir(t, "test-*")
 
@@ -339,10 +339,67 @@ This workflow uses Claude engine with imported plugins.
 	require.NoError(t, os.WriteFile(workflowPath, []byte(workflowContent), 0644),
 		"Failed to write workflow file")
 
-	// Compile the workflow - should fail because Claude doesn't support plugins
+	// Compile the workflow - should succeed because Claude now supports plugins
 	compiler := workflow.NewCompiler()
-	err := compiler.CompileWorkflow(workflowPath)
-	require.Error(t, err, "CompileWorkflow should fail because Claude doesn't support plugins")
-	assert.Contains(t, err.Error(), "does not support plugins",
-		"Error should mention that Claude doesn't support plugins")
+	require.NoError(t, compiler.CompileWorkflow(workflowPath),
+		"CompileWorkflow should succeed because Claude supports plugins")
+
+	// Read the generated lock file
+	lockFilePath := stringutil.MarkdownToLockFile(workflowPath)
+	lockFileContent, err := os.ReadFile(lockFilePath)
+	require.NoError(t, err, "Failed to read lock file")
+
+	workflowData := string(lockFileContent)
+
+	// Verify that Claude engine installs the plugin from import
+	assert.Contains(t, workflowData, "claude plugin install anthropic/plugin-one",
+		"Expected Claude engine to install plugin from import")
+}
+
+func TestCompileWorkflowWithPluginImportsCodexEngineSupportsPlugins(t *testing.T) {
+	// Create a temporary directory for test files
+	tempDir := testutil.TempDir(t, "test-*")
+
+	// Create a shared plugins file
+	sharedPluginsPath := filepath.Join(tempDir, "shared-plugins.md")
+	sharedPluginsContent := `---
+on: push
+plugins:
+  - openai/plugin-one
+---
+`
+	require.NoError(t, os.WriteFile(sharedPluginsPath, []byte(sharedPluginsContent), 0644),
+		"Failed to write shared plugins file")
+
+	// Create a workflow file that imports plugins and uses Codex engine
+	workflowPath := filepath.Join(tempDir, "test-workflow.md")
+	workflowContent := `---
+on: issues
+engine: codex
+imports:
+  - shared-plugins.md
+---
+
+# Test Workflow
+
+This workflow uses Codex engine with imported plugins.
+`
+	require.NoError(t, os.WriteFile(workflowPath, []byte(workflowContent), 0644),
+		"Failed to write workflow file")
+
+	// Compile the workflow - should succeed because Codex now supports plugins
+	compiler := workflow.NewCompiler()
+	require.NoError(t, compiler.CompileWorkflow(workflowPath),
+		"CompileWorkflow should succeed because Codex supports plugins")
+
+	// Read the generated lock file
+	lockFilePath := stringutil.MarkdownToLockFile(workflowPath)
+	lockFileContent, err := os.ReadFile(lockFilePath)
+	require.NoError(t, err, "Failed to read lock file")
+
+	workflowData := string(lockFileContent)
+
+	// Verify that Codex engine installs the plugin from import
+	assert.Contains(t, workflowData, "codex plugin install openai/plugin-one",
+		"Expected Codex engine to install plugin from import")
 }