diff --git a/docs/src/content/docs/agent-factory-status.mdx b/docs/src/content/docs/agent-factory-status.mdx index 946cd1956e..ffe071b46c 100644 --- a/docs/src/content/docs/agent-factory-status.mdx +++ b/docs/src/content/docs/agent-factory-status.mdx @@ -70,6 +70,7 @@ These are experimental agentic workflows used by the GitHub Next team to learn, | [Daily Workflow Updater](https://github.com/github/gh-aw/blob/main/.github/workflows/daily-workflow-updater.md) | copilot | [![Daily Workflow Updater](https://github.com/github/gh-aw/actions/workflows/daily-workflow-updater.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/daily-workflow-updater.lock.yml) | - | - | | [DeepReport - Intelligence Gathering Agent](https://github.com/github/gh-aw/blob/main/.github/workflows/deep-report.md) | codex | [![DeepReport - Intelligence Gathering Agent](https://github.com/github/gh-aw/actions/workflows/deep-report.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/deep-report.lock.yml) | `0 15 * * 1-5` | - | | [Delight](https://github.com/github/gh-aw/blob/main/.github/workflows/delight.md) | copilot | [![Delight](https://github.com/github/gh-aw/actions/workflows/delight.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/delight.lock.yml) | - | - | +| [Dependabot Burner](https://github.com/github/gh-aw/blob/main/.github/workflows/dependabot-burner.md) | copilot | [![Dependabot Burner](https://github.com/github/gh-aw/actions/workflows/dependabot-burner.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/dependabot-burner.lock.yml) | - | - | | [Dependabot Dependency Checker](https://github.com/github/gh-aw/blob/main/.github/workflows/dependabot-go-checker.md) | copilot | [![Dependabot Dependency Checker](https://github.com/github/gh-aw/actions/workflows/dependabot-go-checker.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/dependabot-go-checker.lock.yml) | `0 9 * * 1,3,5` | - | | [Dependabot Project Manager](https://github.com/github/gh-aw/blob/main/.github/workflows/dependabot-project-manager.md) | copilot | [![Dependabot Project Manager](https://github.com/github/gh-aw/actions/workflows/dependabot-project-manager.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/dependabot-project-manager.lock.yml) | - | - | | [Dev](https://github.com/github/gh-aw/blob/main/.github/workflows/dev.md) | copilot | [![Dev](https://github.com/github/gh-aw/actions/workflows/dev.lock.yml/badge.svg)](https://github.com/github/gh-aw/actions/workflows/dev.lock.yml) | - | - | diff --git a/pkg/workflow/agentic_engine.go b/pkg/workflow/agentic_engine.go index 2c1a446549..c6b696b8d5 100644 --- a/pkg/workflow/agentic_engine.go +++ b/pkg/workflow/agentic_engine.go @@ -125,6 +125,10 @@ type CapabilityProvider interface { // SupportsFirewall returns true if this engine supports network firewalling/sandboxing // When true, the engine can enforce network restrictions defined in the workflow SupportsFirewall() bool + + // SupportsPlugins returns true if this engine supports plugin installation + // When true, plugins can be installed using the engine's plugin install command + SupportsPlugins() bool } // WorkflowExecutor handles workflow compilation and execution @@ -199,6 +203,7 @@ type BaseEngine struct { supportsWebFetch bool supportsWebSearch bool supportsFirewall bool + supportsPlugins bool } func (e *BaseEngine) GetID() string { @@ -241,6 +246,10 @@ func (e *BaseEngine) SupportsFirewall() bool { return e.supportsFirewall } +func (e *BaseEngine) SupportsPlugins() bool { + return e.supportsPlugins +} + // GetDeclaredOutputFiles returns an empty list by default (engines can override) func (e *BaseEngine) GetDeclaredOutputFiles() []string { return []string{} diff --git a/pkg/workflow/compiler_orchestrator_tools.go b/pkg/workflow/compiler_orchestrator_tools.go index 964f8a9a8c..c0dd1f8c6a 100644 --- a/pkg/workflow/compiler_orchestrator_tools.go +++ b/pkg/workflow/compiler_orchestrator_tools.go @@ -180,6 +180,12 @@ func (c *Compiler) processToolsAndMarkdown(result *parser.FrontmatterResult, cle orchestratorToolsLog.Printf("Merged plugins: %d total unique plugins", len(pluginInfo.Plugins)) } + // Validate plugin support for the current engine + if err := c.validatePluginSupport(pluginInfo, agenticEngine); err != nil { + orchestratorToolsLog.Printf("Plugin support validation failed: %v", err) + return nil, err + } + // Add MCP fetch server if needed (when web-fetch is requested but engine doesn't support it) tools, _ = AddMCPFetchServerIfNeeded(tools, agenticEngine) diff --git a/pkg/workflow/copilot_engine.go b/pkg/workflow/copilot_engine.go index cef6673186..4549c4075f 100644 --- a/pkg/workflow/copilot_engine.go +++ b/pkg/workflow/copilot_engine.go @@ -45,6 +45,7 @@ func NewCopilotEngine() *CopilotEngine { supportsWebFetch: true, // Copilot CLI has built-in web-fetch support supportsWebSearch: false, // Copilot CLI does not have built-in web-search support supportsFirewall: true, // Copilot supports network firewalling via AWF + supportsPlugins: true, // Copilot supports plugin installation }, } } diff --git a/pkg/workflow/copilot_engine_test.go b/pkg/workflow/copilot_engine_test.go index 0ec25e98d6..32ba74f235 100644 --- a/pkg/workflow/copilot_engine_test.go +++ b/pkg/workflow/copilot_engine_test.go @@ -42,6 +42,10 @@ func TestCopilotEngine(t *testing.T) { t.Error("Expected copilot engine to not support max-turns yet") } + if !engine.SupportsPlugins() { + t.Error("Expected copilot engine to support plugins") + } + // Test declared output files (session files are copied to logs folder) outputFiles := engine.GetDeclaredOutputFiles() if len(outputFiles) != 1 { @@ -84,6 +88,21 @@ func TestOtherEnginesNoDefaultDetectionModel(t *testing.T) { } } +func TestOtherEnginesNoPluginSupport(t *testing.T) { + // Test that only Copilot engine supports plugins + engines := []CodingAgentEngine{ + NewClaudeEngine(), + NewCodexEngine(), + NewCustomEngine(), + } + + for _, engine := range engines { + if engine.SupportsPlugins() { + t.Errorf("Expected engine '%s' to not support plugins, but it does", engine.GetID()) + } + } +} + func TestCopilotEngineInstallationSteps(t *testing.T) { engine := NewCopilotEngine() diff --git a/pkg/workflow/engine_validation.go b/pkg/workflow/engine_validation.go index 99f4dfa109..aeda0fbfff 100644 --- a/pkg/workflow/engine_validation.go +++ b/pkg/workflow/engine_validation.go @@ -146,3 +146,27 @@ func (c *Compiler) validateSingleEngineSpecification(mainEngineSetting string, i return "", fmt.Errorf("invalid engine configuration in included file, missing or invalid 'id' field. Expected string or object with 'id' field.\n\nExample (string):\nengine: copilot\n\nExample (object):\nengine:\n id: copilot\n model: gpt-4\n\nSee: %s", constants.DocsEnginesURL) } + +// validatePluginSupport validates that plugins are only used with engines that support them +func (c *Compiler) validatePluginSupport(pluginInfo *PluginInfo, agenticEngine CodingAgentEngine) error { + // No plugins specified, validation passes + if pluginInfo == nil || len(pluginInfo.Plugins) == 0 { + return nil + } + + engineValidationLog.Printf("Validating plugin support for engine: %s", agenticEngine.GetID()) + + // Check if the engine supports plugins + if !agenticEngine.SupportsPlugins() { + // Build error message listing the plugins that were specified + pluginsList := strings.Join(pluginInfo.Plugins, ", ") + + return fmt.Errorf("engine '%s' does not support plugins. The following plugins cannot be installed: %s\n\nOnly the 'copilot' engine currently supports plugin installation.\n\nTo fix this, either:\n1. Remove the 'plugins' field from your workflow\n2. Change to an engine that supports plugins (e.g., engine: copilot)\n\nSee: %s", + agenticEngine.GetID(), + pluginsList, + constants.DocsEnginesURL) + } + + engineValidationLog.Printf("Engine %s supports plugins: %d plugins to install", agenticEngine.GetID(), len(pluginInfo.Plugins)) + return nil +} diff --git a/pkg/workflow/engine_validation_test.go b/pkg/workflow/engine_validation_test.go index 1fae91338d..2a488cb9c4 100644 --- a/pkg/workflow/engine_validation_test.go +++ b/pkg/workflow/engine_validation_test.go @@ -394,3 +394,124 @@ func TestValidateEngineDidYouMean(t *testing.T) { }) } } + +// TestValidatePluginSupport tests the validatePluginSupport function +func TestValidatePluginSupport(t *testing.T) { + tests := []struct { + name string + pluginInfo *PluginInfo + engineID string + expectError bool + errorMsg string + }{ + { + name: "no plugins with copilot engine", + pluginInfo: nil, + engineID: "copilot", + expectError: false, + }, + { + name: "plugins with copilot engine (supported)", + pluginInfo: &PluginInfo{ + Plugins: []string{"org/plugin1", "org/plugin2"}, + }, + engineID: "copilot", + expectError: false, + }, + { + name: "plugins with claude engine (not supported)", + pluginInfo: &PluginInfo{ + Plugins: []string{"org/plugin1"}, + }, + engineID: "claude", + expectError: true, + errorMsg: "does not support plugins", + }, + { + name: "plugins with codex engine (not supported)", + pluginInfo: &PluginInfo{ + Plugins: []string{"org/plugin1", "org/plugin2"}, + }, + engineID: "codex", + expectError: true, + errorMsg: "does not support plugins", + }, + { + name: "plugins with custom engine (not supported)", + pluginInfo: &PluginInfo{ + Plugins: []string{"org/plugin1"}, + }, + engineID: "custom", + expectError: true, + errorMsg: "does not support plugins", + }, + { + name: "empty plugin list", + pluginInfo: &PluginInfo{Plugins: []string{}}, + engineID: "claude", + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + compiler := NewCompiler() + engine, err := compiler.engineRegistry.GetEngine(tt.engineID) + if err != nil { + t.Fatalf("Failed to get engine: %v", err) + } + + err = compiler.validatePluginSupport(tt.pluginInfo, engine) + + if tt.expectError && err == nil { + t.Error("Expected validation to fail but it succeeded") + } else if !tt.expectError && err != nil { + t.Errorf("Expected validation to succeed but it failed: %v", err) + } else if tt.expectError && err != nil && tt.errorMsg != "" { + if !strings.Contains(err.Error(), tt.errorMsg) { + t.Errorf("Expected error containing '%s', got '%s'", tt.errorMsg, err.Error()) + } + } + }) + } +} + +// TestValidatePluginSupportErrorMessage verifies the plugin validation error message quality +func TestValidatePluginSupportErrorMessage(t *testing.T) { + compiler := NewCompiler() + claudeEngine, err := compiler.engineRegistry.GetEngine("claude") + if err != nil { + t.Fatalf("Failed to get claude engine: %v", err) + } + + pluginInfo := &PluginInfo{ + Plugins: []string{"org/plugin1", "org/plugin2"}, + } + + err = compiler.validatePluginSupport(pluginInfo, claudeEngine) + if err == nil { + t.Fatal("Expected validation to fail for plugins with claude engine") + } + + errorMsg := err.Error() + + // Error should mention the engine name + if !strings.Contains(errorMsg, "claude") { + t.Errorf("Error message should mention the engine name, got: %s", errorMsg) + } + + // Error should list the plugins that can't be installed + if !strings.Contains(errorMsg, "org/plugin1") || !strings.Contains(errorMsg, "org/plugin2") { + t.Errorf("Error message should list the plugins, got: %s", errorMsg) + } + + // Error should mention copilot as the supported engine + if !strings.Contains(errorMsg, "copilot") { + t.Errorf("Error message should mention copilot 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) + } +}