From 3b40ba177524dfb3dad39a1154ca611ceab077b1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Feb 2026 19:40:15 +0000 Subject: [PATCH 1/2] Initial plan From ea41e64710cf6c1f96b3381adbf4c8d5944260bb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Feb 2026 19:50:57 +0000 Subject: [PATCH 2/2] Enable plugin support for Claude and Codex engines - Set supportsPlugins: true for Claude and Codex engines - Add plugin installation steps to Claude and Codex GetInstallationSteps() - Update tests to expect plugin support from Claude and Codex - Add regression test for validation error message listing all supported engines - Update TestOtherEnginesNoPluginSupport to TestEnginesWithPluginSupport Fixes #34439 - CI Failure Investigation - Run #34439 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/claude_engine.go | 12 ++++ pkg/workflow/codex_engine.go | 12 ++++ pkg/workflow/copilot_engine_test.go | 20 ++++-- pkg/workflow/engine_validation_test.go | 93 ++++++++++++++++++++++---- pkg/workflow/plugin_import_test.go | 69 +++++++++++++++++-- 5 files changed, 181 insertions(+), 25 deletions(-) diff --git a/pkg/workflow/claude_engine.go b/pkg/workflow/claude_engine.go index f2741d943d0..f5dee92716b 100644 --- a/pkg/workflow/claude_engine.go +++ b/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 } diff --git a/pkg/workflow/codex_engine.go b/pkg/workflow/codex_engine.go index 876a789e51b..37b97f936b7 100644 --- a/pkg/workflow/codex_engine.go +++ b/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 } diff --git a/pkg/workflow/copilot_engine_test.go b/pkg/workflow/copilot_engine_test.go index 32ba74f235e..f22bd4a8afd 100644 --- a/pkg/workflow/copilot_engine_test.go +++ b/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) { diff --git a/pkg/workflow/engine_validation_test.go b/pkg/workflow/engine_validation_test.go index 1d47b24bf85..8116477423d 100644 --- a/pkg/workflow/engine_validation_test.go +++ b/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) + } + } +} diff --git a/pkg/workflow/plugin_import_test.go b/pkg/workflow/plugin_import_test.go index 2bd75666db8..d399c885993 100644 --- a/pkg/workflow/plugin_import_test.go +++ b/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") }