diff --git a/.changeset/patch-add-imports-plugins.md b/.changeset/patch-add-imports-plugins.md new file mode 100644 index 00000000000..a293e3c1920 --- /dev/null +++ b/.changeset/patch-add-imports-plugins.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Add `imports.plugins` support with all Copilot CLI plugin spec formats, including shared-import merge/dedup and GitHub tree URL normalization to `OWNER/REPO:PATH`. diff --git a/.github/workflows/smoke-copilot.lock.yml b/.github/workflows/smoke-copilot.lock.yml index e37b97b1fee..6df53600f4b 100644 --- a/.github/workflows/smoke-copilot.lock.yml +++ b/.github/workflows/smoke-copilot.lock.yml @@ -28,7 +28,7 @@ # - shared/github-queries-mcp-script.md # - shared/reporting.md # -# gh-aw-metadata: {"schema_version":"v3","frontmatter_hash":"e61f4503d40b9de053152c2ae68538ce50b287317fefc7567d935a4d1499560e","agent_id":"copilot"} +# gh-aw-metadata: {"schema_version":"v3","frontmatter_hash":"f88b8901339f10f50b988e76b68374b73e5ff4f52fb4f5365dbcea92eba302bd","agent_id":"copilot"} name: "Smoke Copilot" "on": @@ -1518,6 +1518,10 @@ jobs: - name: Clean git credentials continue-on-error: true run: bash ${RUNNER_TEMP}/gh-aw/actions/clean_git_credentials.sh + - name: "Install Copilot plugin: github/copilot-plugins:advanced-security/skills/secret-scanning" + env: + GITHUB_TOKEN: ${{ github.token }} + run: copilot plugin install github/copilot-plugins:advanced-security/skills/secret-scanning - name: Execute GitHub Copilot CLI id: agentic_execution # Copilot CLI tool arguments (sorted): diff --git a/.github/workflows/smoke-copilot.md b/.github/workflows/smoke-copilot.md index 78db17a2881..9bc27edbe27 100644 --- a/.github/workflows/smoke-copilot.md +++ b/.github/workflows/smoke-copilot.md @@ -20,9 +20,12 @@ engine: id: copilot max-continuations: 2 imports: - - shared/gh.md - - shared/reporting.md - - shared/github-queries-mcp-script.md + aw: + - shared/gh.md + - shared/reporting.md + - shared/github-queries-mcp-script.md + plugins: + - https://github.com/github/copilot-plugins/tree/main/plugins/advanced-security/skills/secret-scanning network: allowed: - defaults diff --git a/pkg/cli/compile_imports_marketplaces_plugins_integration_test.go b/pkg/cli/compile_imports_marketplaces_plugins_integration_test.go new file mode 100644 index 00000000000..9ea35f21433 --- /dev/null +++ b/pkg/cli/compile_imports_marketplaces_plugins_integration_test.go @@ -0,0 +1,206 @@ +//go:build integration + +package cli + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// TestCompileCopilotImportsPlugins compiles the canonical Copilot workflow +// that uses imports.plugins and verifies that the compiled lock file contains +// the correct `copilot plugin install` setup steps before the agent execution step. +func TestCompileCopilotImportsPlugins(t *testing.T) { + setup := setupIntegrationTest(t) + defer setup.cleanup() + + srcPath := filepath.Join(projectRoot, "pkg/cli/workflows/test-copilot-imports-marketplaces-plugins.md") + dstPath := filepath.Join(setup.workflowsDir, "test-copilot-imports-plugins.md") + + srcContent, err := os.ReadFile(srcPath) + if err != nil { + t.Fatalf("Failed to read source workflow file %s: %v", srcPath, err) + } + if err := os.WriteFile(dstPath, srcContent, 0644); err != nil { + t.Fatalf("Failed to write workflow to test dir: %v", err) + } + + cmd := exec.Command(setup.binaryPath, "compile", dstPath) + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("CLI compile command failed: %v\nOutput: %s", err, string(output)) + } + + lockFilePath := filepath.Join(setup.workflowsDir, "test-copilot-imports-plugins.lock.yml") + lockContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockContentStr := string(lockContent) + + // Verify plugin install step + if !strings.Contains(lockContentStr, "copilot plugin install my-plugin") { + t.Errorf("Lock file should contain plugin install step\nLock file content:\n%s", lockContentStr) + } + + // Verify plugin step appears before the agent execution step + pluginInstallIdx := strings.Index(lockContentStr, "copilot plugin install my-plugin") + agentExecIdx := strings.Index(lockContentStr, "sudo -E awf") + if pluginInstallIdx == -1 || agentExecIdx == -1 { + t.Fatalf("Could not find all expected steps: plugin=%d, agent=%d", + pluginInstallIdx, agentExecIdx) + } + if pluginInstallIdx >= agentExecIdx { + t.Errorf("Plugin install step should appear before the agent execution step") + } + + t.Logf("Copilot plugins workflow compiled successfully to %s", lockFilePath) +} + +// TestCompileClaudeImportsPlugins compiles the canonical Claude workflow +// that uses imports.plugins and verifies that the compiled lock file contains +// the correct `claude plugin install` setup steps before the agent execution step. +func TestCompileClaudeImportsPlugins(t *testing.T) { + setup := setupIntegrationTest(t) + defer setup.cleanup() + + srcPath := filepath.Join(projectRoot, "pkg/cli/workflows/test-claude-imports-marketplaces-plugins.md") + dstPath := filepath.Join(setup.workflowsDir, "test-claude-imports-plugins.md") + + srcContent, err := os.ReadFile(srcPath) + if err != nil { + t.Fatalf("Failed to read source workflow file %s: %v", srcPath, err) + } + if err := os.WriteFile(dstPath, srcContent, 0644); err != nil { + t.Fatalf("Failed to write workflow to test dir: %v", err) + } + + cmd := exec.Command(setup.binaryPath, "compile", dstPath) + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("CLI compile command failed: %v\nOutput: %s", err, string(output)) + } + + lockFilePath := filepath.Join(setup.workflowsDir, "test-claude-imports-plugins.lock.yml") + lockContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockContentStr := string(lockContent) + + // Verify plugin install step + if !strings.Contains(lockContentStr, "claude plugin install my-plugin") { + t.Errorf("Lock file should contain plugin install step\nLock file content:\n%s", lockContentStr) + } + + t.Logf("Claude plugins workflow compiled successfully to %s", lockFilePath) +} + +// TestCompileCopilotImportsPluginsShared compiles the canonical Copilot +// workflow that imports a shared workflow (via imports.aw) which defines its own +// plugins, and verifies that the values are merged and deduplicated in the generated +// lock file. +func TestCompileCopilotImportsPluginsShared(t *testing.T) { + setup := setupIntegrationTest(t) + defer setup.cleanup() + + // Copy both the shared fixture and the main workflow into the test dir + sharedDir := filepath.Join(setup.workflowsDir, "shared") + if err := os.MkdirAll(sharedDir, 0755); err != nil { + t.Fatalf("Failed to create shared dir: %v", err) + } + + sharedSrc := filepath.Join(projectRoot, "pkg/cli/workflows/shared/marketplace-plugins.md") + sharedDst := filepath.Join(sharedDir, "marketplace-plugins.md") + sharedContent, err := os.ReadFile(sharedSrc) + if err != nil { + t.Fatalf("Failed to read shared workflow file %s: %v", sharedSrc, err) + } + if err := os.WriteFile(sharedDst, sharedContent, 0644); err != nil { + t.Fatalf("Failed to write shared workflow: %v", err) + } + + srcPath := filepath.Join(projectRoot, "pkg/cli/workflows/test-copilot-imports-marketplaces-plugins-shared.md") + dstPath := filepath.Join(setup.workflowsDir, "test-copilot-imports-plugins-shared.md") + srcContent, err := os.ReadFile(srcPath) + if err != nil { + t.Fatalf("Failed to read source workflow file %s: %v", srcPath, err) + } + if err := os.WriteFile(dstPath, srcContent, 0644); err != nil { + t.Fatalf("Failed to write workflow to test dir: %v", err) + } + + cmd := exec.Command(setup.binaryPath, "compile", dstPath) + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("CLI compile command failed: %v\nOutput: %s", err, string(output)) + } + + lockFilePath := filepath.Join(setup.workflowsDir, "test-copilot-imports-plugins-shared.lock.yml") + lockContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockContentStr := string(lockContent) + + // Main workflow's own plugin should be present + if !strings.Contains(lockContentStr, "copilot plugin install main-plugin") { + t.Errorf("Lock file should contain main plugin install step\nLock file content:\n%s", lockContentStr) + } + + // Shared workflow's plugin should also be present (merged) + if !strings.Contains(lockContentStr, "copilot plugin install shared-plugin") { + t.Errorf("Lock file should contain shared plugin install step\nLock file content:\n%s", lockContentStr) + } + + // Values should appear exactly once (deduplication) + if count := strings.Count(lockContentStr, "copilot plugin install main-plugin"); count != 1 { + t.Errorf("Main plugin step should appear exactly once, got %d\nLock file content:\n%s", count, lockContentStr) + } + + t.Logf("Copilot shared plugins workflow compiled successfully to %s", lockFilePath) +} + +// TestCompileCodexImportsPluginsError verifies that using imports.plugins with the +// Codex engine fails compilation with a clear error. +func TestCompileCodexImportsPluginsError(t *testing.T) { + setup := setupIntegrationTest(t) + defer setup.cleanup() + + const workflowContent = `--- +on: issues +permissions: + contents: read + issues: read +engine: codex +imports: + plugins: + - my-plugin +--- + +# Test Codex Imports Plugins + +Process the issue. +` + dstPath := filepath.Join(setup.workflowsDir, "test-codex-unsupported-imports.md") + if err := os.WriteFile(dstPath, []byte(workflowContent), 0644); err != nil { + t.Fatalf("Failed to write workflow: %v", err) + } + + cmd := exec.Command(setup.binaryPath, "compile", dstPath) + output, err := cmd.CombinedOutput() + outputStr := string(output) + + if err == nil { + t.Fatalf("Expected compile to fail for Codex with imports.plugins, but it succeeded\nOutput: %s", outputStr) + } + + if !strings.Contains(outputStr, "imports.plugins") { + t.Errorf("Error output should mention 'imports.plugins'\nOutput: %s", outputStr) + } + + t.Logf("Correctly rejected Codex workflow with imports.plugins: %s", outputStr) +} diff --git a/pkg/cli/workflows/shared/marketplace-plugins.md b/pkg/cli/workflows/shared/marketplace-plugins.md new file mode 100644 index 00000000000..306d7d49e3e --- /dev/null +++ b/pkg/cli/workflows/shared/marketplace-plugins.md @@ -0,0 +1,7 @@ +--- +imports: + plugins: + - shared-plugin +--- + +This shared workflow provides plugin configuration for testing imports merging. diff --git a/pkg/cli/workflows/test-claude-imports-marketplaces-plugins.md b/pkg/cli/workflows/test-claude-imports-marketplaces-plugins.md new file mode 100644 index 00000000000..9f71546fccb --- /dev/null +++ b/pkg/cli/workflows/test-claude-imports-marketplaces-plugins.md @@ -0,0 +1,17 @@ +--- +on: issues +permissions: + contents: read + issues: read +engine: claude +imports: + plugins: + - my-plugin +--- + +# Test Claude Imports Plugins + +This workflow tests that `imports.plugins` is compiled into +`claude plugin install` setup steps before the agent runs. + +Process the issue and respond with a helpful comment. diff --git a/pkg/cli/workflows/test-copilot-imports-marketplaces-plugins-shared.md b/pkg/cli/workflows/test-copilot-imports-marketplaces-plugins-shared.md new file mode 100644 index 00000000000..99f2696a75e --- /dev/null +++ b/pkg/cli/workflows/test-copilot-imports-marketplaces-plugins-shared.md @@ -0,0 +1,19 @@ +--- +on: issues +permissions: + contents: read + issues: read +engine: copilot +imports: + aw: + - shared/marketplace-plugins.md + plugins: + - main-plugin +--- + +# Test Copilot Imports Plugins with Shared Import + +This workflow tests that `imports.plugins` values from a shared +agentic workflow (imported via `imports.aw`) are merged with the main workflow's own values. + +Process the issue and respond with a helpful comment. diff --git a/pkg/cli/workflows/test-copilot-imports-marketplaces-plugins.md b/pkg/cli/workflows/test-copilot-imports-marketplaces-plugins.md new file mode 100644 index 00000000000..ade0b9b1f40 --- /dev/null +++ b/pkg/cli/workflows/test-copilot-imports-marketplaces-plugins.md @@ -0,0 +1,17 @@ +--- +on: issues +permissions: + contents: read + issues: read +engine: copilot +imports: + plugins: + - my-plugin +--- + +# Test Copilot Imports Plugins + +This workflow tests that `imports.plugins` is compiled into +`copilot plugin install` setup steps before the agent runs. + +Process the issue and respond with a helpful comment. diff --git a/pkg/parser/import_bfs.go b/pkg/parser/import_bfs.go index 1436fbd17e0..ba6adaa458e 100644 --- a/pkg/parser/import_bfs.go +++ b/pkg/parser/import_bfs.go @@ -60,7 +60,7 @@ func processImportsFromFrontmatterWithManifestAndSource(frontmatter map[string]a } } default: - return nil, errors.New("imports field must be an array or an object with 'aw'/'apm-packages' subfields") + return nil, errors.New("imports field must be an array or an object with 'aw', 'apm-packages', or 'plugins' subfields") } if len(importSpecs) == 0 { diff --git a/pkg/parser/import_field_extractor.go b/pkg/parser/import_field_extractor.go index a253aaad505..7994b65faeb 100644 --- a/pkg/parser/import_field_extractor.go +++ b/pkg/parser/import_field_extractor.go @@ -45,6 +45,8 @@ type importAccumulator struct { agentImportSpec string repositoryImports []string importInputs map[string]any + plugins []string + pluginsSet map[string]bool // First on.github-token / on.github-app found across all imported files (first-wins strategy) activationGitHubToken string activationGitHubApp string // JSON-encoded GitHubAppConfig @@ -62,6 +64,7 @@ func newImportAccumulator() *importAccumulator { skipRolesSet: make(map[string]bool), skipBotsSet: make(map[string]bool), importInputs: make(map[string]any), + pluginsSet: make(map[string]bool), } } @@ -288,6 +291,30 @@ func (acc *importAccumulator) extractAllImportFields(content []byte, item import } } + // Extract imports.plugins from imported file (merge into set to avoid duplicates) + if importsAny, hasImports := fm["imports"]; hasImports { + if importsMap, ok := importsAny.(map[string]any); ok { + if pluginsAny, hasPlugins := importsMap["plugins"]; hasPlugins { + switch pluginsVal := pluginsAny.(type) { + case []any: + for _, item := range pluginsVal { + if name, ok := item.(string); ok && name != "" && !acc.pluginsSet[name] { + acc.pluginsSet[name] = true + acc.plugins = append(acc.plugins, name) + } + } + case []string: + for _, name := range pluginsVal { + if name != "" && !acc.pluginsSet[name] { + acc.pluginsSet[name] = true + acc.plugins = append(acc.plugins, name) + } + } + } + } + } + } + return nil } @@ -319,6 +346,7 @@ func (acc *importAccumulator) toImportsResult(topologicalOrder []string) *Import MergedCaches: acc.caches, MergedJobs: acc.jobsBuilder.String(), MergedFeatures: acc.features, + MergedPlugins: acc.plugins, ImportedFiles: topologicalOrder, AgentFile: acc.agentFile, AgentImportSpec: acc.agentImportSpec, diff --git a/pkg/parser/import_processor.go b/pkg/parser/import_processor.go index edf9a1e45ba..d8da6fad2a2 100644 --- a/pkg/parser/import_processor.go +++ b/pkg/parser/import_processor.go @@ -39,6 +39,7 @@ type ImportsResult struct { MergedCaches []string // Merged cache configurations from all imports (appended in order) MergedJobs string // Merged jobs from imported YAML workflows (JSON format) MergedFeatures []map[string]any // Merged features configuration from all imports (parsed YAML structures) + MergedPlugins []string // Merged plugin specs from all imports (union, deduplicated) ImportedFiles []string // List of imported file paths (for manifest) AgentFile string // Path to custom agent file (if imported) AgentImportSpec string // Original import specification for agent file (e.g., "owner/repo/path@ref") diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 22ab3f984b4..a5c907cf8ec 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -66,7 +66,7 @@ ] }, "imports": { - "description": "Workflow specifications to import. Supports array form (list of paths) or object form with 'aw' (agentic workflow paths) and 'apm-packages' (APM packages) subfields.", + "description": "Workflow specifications to import. Supports array form (list of paths) or object form with 'aw' (agentic workflow paths), 'apm-packages' (APM packages), and 'plugins' (plugins to install) subfields.", "oneOf": [ { "type": "array", @@ -195,6 +195,15 @@ "additionalProperties": false } ] + }, + "plugins": { + "type": "array", + "description": "List of plugin specs to install via the agent's native package support before execution. Supports all spec formats accepted by the engine CLI: 'plugin@marketplace', 'OWNER/REPO', 'OWNER/REPO:PATH/TO/PLUGIN', Git URLs (https://github.com/o/r.git), or full GitHub tree URLs (auto-converted to OWNER/REPO:PATH format).", + "items": { + "type": "string", + "description": "Plugin spec to install (e.g., 'my-extension', 'my-plugin@my-marketplace', 'OWNER/REPO:path/to/plugin', or 'https://github.com/github/copilot-plugins/tree/main/plugins/my-plugin')." + }, + "examples": [["my-extension", "another-plugin"], ["github/copilot-plugins:plugins/advanced-security/skills/secret-scanning"]] } } } @@ -224,6 +233,9 @@ "private-key": "${{ secrets.APP_PRIVATE_KEY }}" } } + }, + { + "plugins": ["my-extension", "github/copilot-plugins:plugins/advanced-security/skills/secret-scanning"] } ] }, diff --git a/pkg/workflow/agent_validation.go b/pkg/workflow/agent_validation.go index 51b34071b66..48f5e6d4cc3 100644 --- a/pkg/workflow/agent_validation.go +++ b/pkg/workflow/agent_validation.go @@ -14,6 +14,7 @@ // - validateMaxTurnsSupport() - Validates max-turns feature support // - validateMaxContinuationsSupport() - Validates max-continuations feature support // - validateWebSearchSupport() - Validates web-search feature support (warning) +// - validateImportsProviderSupport() - Validates marketplace/plugin support (error if unsupported) // - validateWorkflowRunBranches() - Validates workflow_run has branch restrictions // // # Validation Patterns @@ -174,7 +175,28 @@ func (c *Compiler) validateWebSearchSupport(tools map[string]any, engine CodingA } } -// validateWorkflowRunBranches validates that workflow_run triggers include branch restrictions +// validateImportsProviderSupport validates that imports.plugins is only used with +// engines that implement the ImportsProvider interface. +// Codex and Gemini do not implement ImportsProvider; specifying plugins with those +// engines is an error. +func (c *Compiler) validateImportsProviderSupport(plugins []string, engine CodingAgentEngine) error { + if len(plugins) == 0 { + // Nothing to validate + return nil + } + + agentValidationLog.Printf("Validating ImportsProvider support for engine: %s", engine.GetID()) + + if _, ok := engine.(ImportsProvider); ok { + // Engine supports plugin installation + return nil + } + + agentValidationLog.Printf("Engine %s does not support ImportsProvider (imports.plugins)", engine.GetID()) + return fmt.Errorf("imports.plugins not supported: engine '%s' does not support plugin installation", + engine.GetID()) +} + // This is a security best practice to avoid running on all branches func (c *Compiler) validateWorkflowRunBranches(workflowData *WorkflowData, markdownPath string) error { if workflowData.On == "" { diff --git a/pkg/workflow/agentic_engine.go b/pkg/workflow/agentic_engine.go index c8324dad39e..c0cfad379e0 100644 --- a/pkg/workflow/agentic_engine.go +++ b/pkg/workflow/agentic_engine.go @@ -228,6 +228,19 @@ type ConfigRenderer interface { RenderConfig(target *ResolvedEngineTarget) ([]map[string]any, error) } +// ImportsProvider is an optional interface implemented by engines that support +// native plugin installation before agent execution. +// These setup steps run in the agent job before the main agent command, using the +// GitHub Actions token for authentication. +type ImportsProvider interface { + // GetPluginInstallSteps returns GitHub Actions steps that install plugins/extensions + // via the agent's native package support before execution. Each entry is a plugin + // specification recognised by the agent CLI (e.g. OWNER/REPO:PATH, plugin@marketplace). + // Returns an empty slice if the engine does not support plugin installation + // or if the provided list is empty. + GetPluginInstallSteps(plugins []string, workflowData *WorkflowData) []GitHubActionStep +} + // CodingAgentEngine is a composite interface that combines all focused interfaces // This maintains backward compatibility with existing code while allowing more flexibility // Implementations can choose to implement only the interfaces they need by embedding BaseEngine diff --git a/pkg/workflow/claude_engine_imports.go b/pkg/workflow/claude_engine_imports.go new file mode 100644 index 00000000000..206c7e74195 --- /dev/null +++ b/pkg/workflow/claude_engine_imports.go @@ -0,0 +1,30 @@ +// This file implements the ImportsProvider interface for the Claude engine. +// +// The Claude engine supports native plugin installation via the `claude` CLI +// before the main agent execution step. The GitHub Actions token is passed as +// GITHUB_TOKEN so the CLI can authenticate with GitHub-hosted registries. +// +// CLI command emitted: +// - Plugin: claude plugin install + +package workflow + +// GetPluginInstallSteps returns GitHub Actions steps that install Claude plugins +// via the `claude plugin install` command before agent execution. +func (e *ClaudeEngine) GetPluginInstallSteps(plugins []string, workflowData *WorkflowData) []GitHubActionStep { + if len(plugins) == 0 { + return nil + } + + var steps []GitHubActionStep + for _, name := range plugins { + step := GitHubActionStep{ + ` - name: "Install Claude plugin: ` + name + `"`, + " env:", + " GITHUB_TOKEN: ${{ github.token }}", + " run: claude plugin install " + name, + } + steps = append(steps, step) + } + return steps +} diff --git a/pkg/workflow/compiler_orchestrator_tools.go b/pkg/workflow/compiler_orchestrator_tools.go index 5e6db849ad6..d6ab348f511 100644 --- a/pkg/workflow/compiler_orchestrator_tools.go +++ b/pkg/workflow/compiler_orchestrator_tools.go @@ -18,6 +18,7 @@ type toolsProcessingResult struct { tools map[string]any runtimes map[string]any apmDependencies *APMDependenciesInfo // APM (Agent Package Manager) dependencies + plugins []string // Plugin specs from imports.plugins toolsTimeout int toolsStartupTimeout int markdownContent string @@ -165,6 +166,27 @@ func (c *Compiler) processToolsAndMarkdown(result *parser.FrontmatterResult, cle orchestratorToolsLog.Printf("Extracted %d APM dependencies from frontmatter", len(apmDependencies.Packages)) } + // Extract plugin specs from imports.plugins + plugins, err := extractPluginsFromFrontmatter(result.Frontmatter) + if err != nil { + return nil, err + } + if len(plugins) > 0 { + orchestratorToolsLog.Printf("Extracted %d plugin(s) from frontmatter", len(plugins)) + } + // Merge plugin specs from imported shared workflows (union, deduplicated) + if len(importsResult.MergedPlugins) > 0 { + orchestratorToolsLog.Printf("Merging %d plugin(s) from imported workflows", len(importsResult.MergedPlugins)) + plugins = mergeUnique(plugins, importsResult.MergedPlugins...) + } + + // Normalise plugin entries: full GitHub tree URLs are converted to the + // OWNER/REPO:PATH/TO/PLUGIN format accepted by the Copilot CLI ("subdirectory + // in a repository" spec). This format is a direct GitHub reference. + if len(plugins) > 0 { + plugins = normalizePlugins(plugins) + } + // Add MCP fetch server if needed (when web-fetch is requested but engine doesn't support it) tools, _ = AddMCPFetchServerIfNeeded(tools, agenticEngine) @@ -202,6 +224,11 @@ func (c *Compiler) processToolsAndMarkdown(result *parser.FrontmatterResult, cle // Validate web-search support for the current engine (warning only) c.validateWebSearchSupport(tools, agenticEngine) + // Validate that imports.plugins is only used with supported engines + if err := c.validateImportsProviderSupport(plugins, agenticEngine); err != nil { + return nil, err + } + // Process @include directives in markdown content markdownContent, includedMarkdownFiles, err := parser.ExpandIncludesWithManifest(result.Markdown, markdownDir, false) if err != nil { @@ -296,6 +323,7 @@ func (c *Compiler) processToolsAndMarkdown(result *parser.FrontmatterResult, cle tools: tools, runtimes: runtimes, apmDependencies: apmDependencies, + plugins: plugins, toolsTimeout: toolsTimeout, toolsStartupTimeout: toolsStartupTimeout, markdownContent: markdownContent, diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index 27705687863..f959b9d0248 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -187,6 +187,7 @@ func (c *Compiler) buildInitialWorkflowData( ParsedTools: NewTools(toolsResult.tools), Runtimes: toolsResult.runtimes, APMDependencies: toolsResult.apmDependencies, + Plugins: toolsResult.plugins, MarkdownContent: toolsResult.markdownContent, AI: engineSetup.engineSetting, EngineConfig: engineSetup.engineConfig, diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index f81bc0beee9..e8f317149cf 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -416,6 +416,7 @@ type WorkflowData struct { QmdConfig *QmdToolConfig // parsed qmd tool configuration (docs globs) Runtimes map[string]any // runtime version overrides from frontmatter APMDependencies *APMDependenciesInfo // APM (Agent Package Manager) dependency packages to install + Plugins []string // Plugin specs to install before agent execution (from imports.plugins) ToolsTimeout int // timeout in seconds for tool/MCP operations (0 = use engine default) ToolsStartupTimeout int // timeout in seconds for MCP server startup (0 = use engine default) Features map[string]any // feature flags and configuration options from frontmatter (supports bool and string values) diff --git a/pkg/workflow/compiler_yaml_main_job.go b/pkg/workflow/compiler_yaml_main_job.go index 85c6cfbea0d..6d5e2eddc7c 100644 --- a/pkg/workflow/compiler_yaml_main_job.go +++ b/pkg/workflow/compiler_yaml_main_job.go @@ -359,6 +359,20 @@ func (c *Compiler) generateMainJobSteps(yaml *strings.Builder, data *WorkflowDat } } + // Emit plugin installation steps before agent execution. + // These steps use the engine's native CLI package support (ImportsProvider interface). + // Each step sets GITHUB_TOKEN from the GitHub Actions token for authentication. + if importsProvider, ok := engine.(ImportsProvider); ok { + if len(data.Plugins) > 0 { + compilerYamlLog.Printf("Adding %d plugin install step(s) for %s", len(data.Plugins), engine.GetID()) + for _, step := range importsProvider.GetPluginInstallSteps(data.Plugins, data) { + for _, line := range step { + yaml.WriteString(line + "\n") + } + } + } + } + // Add AI execution step using the agentic engine compilerYamlLog.Printf("Generating engine execution steps for %s", engine.GetID()) c.generateEngineExecutionSteps(yaml, data, engine, logFileFull) diff --git a/pkg/workflow/copilot_engine_imports.go b/pkg/workflow/copilot_engine_imports.go new file mode 100644 index 00000000000..19b101004ad --- /dev/null +++ b/pkg/workflow/copilot_engine_imports.go @@ -0,0 +1,36 @@ +// This file implements the ImportsProvider interface for the Copilot engine. +// +// The Copilot engine supports native plugin installation via the `copilot` CLI +// before the main agent execution step. The GitHub Actions token is passed as +// GITHUB_TOKEN so the CLI can authenticate with GitHub-hosted registries. +// +// CLI command emitted: +// - Plugin: copilot plugin install +// +// Supported plugin spec formats (see https://docs.github.com/en/copilot/reference/copilot-cli-reference/cli-plugin-reference): +// - plugin@marketplace — plugin from a registered marketplace +// - OWNER/REPO — root of a GitHub repository +// - OWNER/REPO:PATH/TO/PLUGIN — subdirectory in a repository +// - https://github.com/o/r.git — any Git URL + +package workflow + +// GetPluginInstallSteps returns GitHub Actions steps that install Copilot plugins +// via the `copilot plugin install` command before agent execution. +func (e *CopilotEngine) GetPluginInstallSteps(plugins []string, workflowData *WorkflowData) []GitHubActionStep { + if len(plugins) == 0 { + return nil + } + + var steps []GitHubActionStep + for _, name := range plugins { + step := GitHubActionStep{ + ` - name: "Install Copilot plugin: ` + name + `"`, + " env:", + " GITHUB_TOKEN: ${{ github.token }}", + " run: copilot plugin install " + name, + } + steps = append(steps, step) + } + return steps +} diff --git a/pkg/workflow/frontmatter_extraction_metadata.go b/pkg/workflow/frontmatter_extraction_metadata.go index b31570d5f06..30f4f4da012 100644 --- a/pkg/workflow/frontmatter_extraction_metadata.go +++ b/pkg/workflow/frontmatter_extraction_metadata.go @@ -378,3 +378,47 @@ func extractAPMDependenciesFromValue(value any, fieldName string) (*APMDependenc frontmatterMetadataLog.Printf("Extracted %d APM dependency packages from %s (isolated=%v, github-app=%v, github-token=%v, version=%s, env=%d)", len(packages), fieldName, isolated, githubApp != nil, githubToken != "", version, len(env)) return &APMDependenciesInfo{Packages: packages, Isolated: isolated, GitHubApp: githubApp, GitHubToken: githubToken, Version: version, Env: env}, nil } + +// extractPluginsFromFrontmatter extracts plugin specs from the imports.plugins field. +// Plugins are agent-native extensions that are installed by the agent CLI before execution, +// enabling additional capabilities and tools within the agent's runtime. +// +// Supported format: +// +// imports: +// plugins: +// - my-plugin +// - another-plugin +// +// Returns an empty slice if the field is absent. +func extractPluginsFromFrontmatter(frontmatter map[string]any) ([]string, error) { + importsAny, hasImports := frontmatter["imports"] + if !hasImports { + return nil, nil + } + importsMap, ok := importsAny.(map[string]any) + if !ok { + return nil, nil + } + pluginsAny, hasPlugins := importsMap["plugins"] + if !hasPlugins { + return nil, nil + } + + var plugins []string + switch v := pluginsAny.(type) { + case []any: + for _, item := range v { + if s, ok := item.(string); ok && s != "" { + plugins = append(plugins, s) + } + } + case []string: + plugins = append(plugins, v...) + default: + return nil, errors.New("imports.plugins must be an array of strings") + } + + frontmatterMetadataLog.Printf("Extracted %d plugin(s) from imports.plugins", len(plugins)) + return plugins, nil +} diff --git a/pkg/workflow/imports_marketplaces_plugins_test.go b/pkg/workflow/imports_marketplaces_plugins_test.go new file mode 100644 index 00000000000..dd16891ba69 --- /dev/null +++ b/pkg/workflow/imports_marketplaces_plugins_test.go @@ -0,0 +1,468 @@ +//go:build !integration + +package workflow + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestExtractPluginsFromFrontmatter tests parsing of imports.plugins from frontmatter. +func TestExtractPluginsFromFrontmatter(t *testing.T) { + tests := []struct { + name string + frontmatter map[string]any + expected []string + wantErr bool + }{ + { + name: "no imports field returns nil", + frontmatter: map[string]any{}, + expected: nil, + }, + { + name: "imports is not a map returns nil", + frontmatter: map[string]any{"imports": []any{"some.md"}}, + expected: nil, + }, + { + name: "no plugins subfield returns nil", + frontmatter: map[string]any{ + "imports": map[string]any{"apm-packages": []any{"org/pkg"}}, + }, + expected: nil, + }, + { + name: "array form extracts plugin names", + frontmatter: map[string]any{ + "imports": map[string]any{ + "plugins": []any{"my-plugin", "another-plugin"}, + }, + }, + expected: []string{"my-plugin", "another-plugin"}, + }, + { + name: "empty strings in array are skipped", + frontmatter: map[string]any{ + "imports": map[string]any{ + "plugins": []any{"my-plugin", "", "other-plugin"}, + }, + }, + expected: []string{"my-plugin", "other-plugin"}, + }, + { + name: "invalid type returns error", + frontmatter: map[string]any{ + "imports": map[string]any{ + "plugins": 42, + }, + }, + wantErr: true, + }, + { + name: "coexists with aw and apm-packages", + frontmatter: map[string]any{ + "imports": map[string]any{ + "aw": []any{"shared/tools.md"}, + "apm-packages": []any{"org/pkg"}, + "plugins": []any{"my-plugin"}, + }, + }, + expected: []string{"my-plugin"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := extractPluginsFromFrontmatter(tt.frontmatter) + if tt.wantErr { + assert.Error(t, err, "Expected error for invalid plugins field") + return + } + require.NoError(t, err, "Should not error for valid input") + assert.Equal(t, tt.expected, result, "Plugin names should match") + }) + } +} + +// TestCopilotEngineGetPluginInstallSteps tests the Copilot engine's plugin install step generation. +func TestCopilotEngineGetPluginInstallSteps(t *testing.T) { + engine := NewCopilotEngine() + + t.Run("empty list returns nil", func(t *testing.T) { + steps := engine.GetPluginInstallSteps(nil, &WorkflowData{}) + assert.Nil(t, steps, "Should return nil for empty plugins") + }) + + t.Run("single plugin emits one step", func(t *testing.T) { + steps := engine.GetPluginInstallSteps([]string{"my-extension"}, &WorkflowData{}) + require.Len(t, steps, 1, "Should emit one step per plugin") + step := steps[0] + found := false + for _, line := range step { + if line == " run: copilot plugin install my-extension" { + found = true + } + } + assert.True(t, found, "Step should contain copilot plugin install command") + // Verify GITHUB_TOKEN is set + hasToken := false + for _, line := range step { + if line == " GITHUB_TOKEN: ${{ github.token }}" { + hasToken = true + } + } + assert.True(t, hasToken, "Step should set GITHUB_TOKEN from github.token") + }) + + t.Run("multiple plugins emit one step each", func(t *testing.T) { + steps := engine.GetPluginInstallSteps([]string{"plugin-a", "plugin-b", "plugin-c"}, &WorkflowData{}) + assert.Len(t, steps, 3, "Should emit one step per plugin name") + }) + + t.Run("OWNER/REPO:PATH spec is passed through", func(t *testing.T) { + spec := "github/copilot-plugins:advanced-security/skills/secret-scanning" + steps := engine.GetPluginInstallSteps([]string{spec}, &WorkflowData{}) + require.Len(t, steps, 1, "Should emit one step") + found := false + for _, line := range steps[0] { + if line == " run: copilot plugin install "+spec { + found = true + } + } + assert.True(t, found, "Step should contain the OWNER/REPO:PATH spec verbatim") + }) +} + +// TestClaudeEngineGetPluginInstallSteps tests the Claude engine's plugin install step generation. +func TestClaudeEngineGetPluginInstallSteps(t *testing.T) { + engine := NewClaudeEngine() + + t.Run("empty list returns nil", func(t *testing.T) { + steps := engine.GetPluginInstallSteps(nil, &WorkflowData{}) + assert.Nil(t, steps, "Should return nil for empty plugins") + }) + + t.Run("single plugin emits one step", func(t *testing.T) { + steps := engine.GetPluginInstallSteps([]string{"my-plugin"}, &WorkflowData{}) + require.Len(t, steps, 1, "Should emit one step per plugin") + step := steps[0] + found := false + for _, line := range step { + if line == " run: claude plugin install my-plugin" { + found = true + } + } + assert.True(t, found, "Step should contain claude plugin install command") + }) +} + +// TestImportsProviderInterface verifies that Copilot and Claude engines implement ImportsProvider. +func TestImportsProviderInterface(t *testing.T) { + t.Run("CopilotEngine implements ImportsProvider", func(t *testing.T) { + engine := NewCopilotEngine() + _, ok := any(engine).(ImportsProvider) + assert.True(t, ok, "CopilotEngine should implement ImportsProvider") + }) + + t.Run("ClaudeEngine implements ImportsProvider", func(t *testing.T) { + engine := NewClaudeEngine() + _, ok := any(engine).(ImportsProvider) + assert.True(t, ok, "ClaudeEngine should implement ImportsProvider") + }) + + // Codex and Gemini CLIs do not have native plugin CLI commands, + // so they intentionally do not implement ImportsProvider. + t.Run("CodexEngine does not implement ImportsProvider", func(t *testing.T) { + engine := NewCodexEngine() + _, ok := any(engine).(ImportsProvider) + assert.False(t, ok, "CodexEngine should not implement ImportsProvider (no native plugin CLI support)") + }) + + t.Run("GeminiEngine does not implement ImportsProvider", func(t *testing.T) { + engine := NewGeminiEngine() + _, ok := any(engine).(ImportsProvider) + assert.False(t, ok, "GeminiEngine should not implement ImportsProvider (no native plugin CLI support)") + }) +} + +// TestCompileWorkflow_Plugins tests that imports.plugins is compiled into install steps +// before agent execution. +func TestCompileWorkflow_Plugins(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "compile-imports-plugins-test*") + require.NoError(t, err, "Failed to create temp directory") + defer os.RemoveAll(tmpDir) + + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + err = os.MkdirAll(workflowsDir, 0755) + require.NoError(t, err, "Failed to create workflows directory") + + workflowContent := `--- +name: Test Plugins +on: + workflow_dispatch: +engine: copilot +imports: + plugins: + - my-extension +--- + +# Test Plugins + +Install a plugin before the agent runs. +` + workflowFile := filepath.Join(workflowsDir, "test-workflow.md") + err = os.WriteFile(workflowFile, []byte(workflowContent), 0600) + require.NoError(t, err, "Failed to write test workflow") + + originalDir, err := os.Getwd() + require.NoError(t, err, "Failed to get current directory") + defer os.Chdir(originalDir) //nolint:errcheck + + err = os.Chdir(tmpDir) + require.NoError(t, err, "Failed to change to temp directory") + + compiler := NewCompiler() + err = compiler.CompileWorkflow(workflowFile) + require.NoError(t, err, "Failed to compile workflow") + + lockFile := strings.Replace(workflowFile, ".md", ".lock.yml", 1) + yamlOutput, err := os.ReadFile(lockFile) + require.NoError(t, err, "Failed to read lock file") + require.NotEmpty(t, yamlOutput, "Compiled YAML should not be empty") + + yamlStr := string(yamlOutput) + + assert.Contains(t, yamlStr, "copilot plugin install my-extension", + "Should contain plugin installation step") + assert.Contains(t, yamlStr, "GITHUB_TOKEN: ${{ github.token }}", + "Plugin step should include GITHUB_TOKEN from github.token") + + // Step should appear before the agent execution step + pluginIdx := strings.Index(yamlStr, "copilot plugin install") + agentIdx := strings.Index(yamlStr, "copilot --add-dir") + if agentIdx == -1 { + agentIdx = strings.Index(yamlStr, "copilot --") + } + require.NotEqual(t, -1, agentIdx, "Agent execution step should be present") + assert.Less(t, pluginIdx, agentIdx, + "Plugin installation step should appear before agent execution") +} + +// TestCompileWorkflow_PluginsFromSharedImport verifies that imports.plugins defined in a +// shared workflow file are merged into the main workflow's Plugins. +func TestCompileWorkflow_PluginsFromSharedImport(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "compile-shared-imports-test*") + require.NoError(t, err, "Failed to create temp directory") + defer os.RemoveAll(tmpDir) + + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + err = os.MkdirAll(workflowsDir, 0755) + require.NoError(t, err, "Failed to create workflows directory") + + // Shared workflow that defines plugins + sharedContent := `--- +imports: + plugins: + - shared-plugin +--- +` + sharedFile := filepath.Join(workflowsDir, "shared-tools.md") + err = os.WriteFile(sharedFile, []byte(sharedContent), 0600) + require.NoError(t, err, "Failed to write shared workflow") + + // Main workflow imports the shared file and adds its own plugin + workflowContent := `--- +name: Test Shared Imports Merge +on: + workflow_dispatch: +engine: copilot +imports: + aw: + - shared-tools.md + plugins: + - main-plugin +--- + +# Test Shared Imports Merge + +Verify that plugins from imported workflows are merged. +` + workflowFile := filepath.Join(workflowsDir, "test-workflow.md") + err = os.WriteFile(workflowFile, []byte(workflowContent), 0600) + require.NoError(t, err, "Failed to write test workflow") + + originalDir, err := os.Getwd() + require.NoError(t, err, "Failed to get current directory") + defer os.Chdir(originalDir) //nolint:errcheck + + err = os.Chdir(tmpDir) + require.NoError(t, err, "Failed to change to temp directory") + + compiler := NewCompiler() + err = compiler.CompileWorkflow(workflowFile) + require.NoError(t, err, "Failed to compile workflow") + + lockFile := strings.Replace(workflowFile, ".md", ".lock.yml", 1) + yamlOutput, err := os.ReadFile(lockFile) + require.NoError(t, err, "Failed to read lock file") + + yamlStr := string(yamlOutput) + + assert.Contains(t, yamlStr, "copilot plugin install main-plugin", + "Main workflow plugin should be present") + assert.Contains(t, yamlStr, "copilot plugin install shared-plugin", + "Shared workflow plugin should be merged in") +} + +// TestCompileWorkflow_PluginsDeduplication verifies that importing the same plugin name +// from multiple shared workflows does not produce duplicate install steps. +func TestCompileWorkflow_PluginsDeduplication(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "compile-dedup-test*") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + // Two shared files that both declare the same plugin + for _, name := range []string{"shared-a.md", "shared-b.md"} { + content := `--- +imports: + plugins: + - common-plugin +--- +` + require.NoError(t, os.WriteFile(filepath.Join(workflowsDir, name), []byte(content), 0600)) + } + + workflowContent := `--- +name: Test Dedup +on: + workflow_dispatch: +engine: copilot +imports: + aw: + - shared-a.md + - shared-b.md +--- + +# Test Dedup +` + workflowFile := filepath.Join(workflowsDir, "test-workflow.md") + require.NoError(t, os.WriteFile(workflowFile, []byte(workflowContent), 0600)) + + originalDir, _ := os.Getwd() + defer os.Chdir(originalDir) //nolint:errcheck + require.NoError(t, os.Chdir(tmpDir)) + + compiler := NewCompiler() + require.NoError(t, compiler.CompileWorkflow(workflowFile)) + + lockFile := strings.Replace(workflowFile, ".md", ".lock.yml", 1) + yamlOutput, err := os.ReadFile(lockFile) + require.NoError(t, err) + yamlStr := string(yamlOutput) + + // Should appear exactly once despite being defined in two shared imports + count := strings.Count(yamlStr, "copilot plugin install common-plugin") + assert.Equal(t, 1, count, "Duplicate plugin name should appear only once") +} + +// TestValidateImportsProviderSupport tests that engines without ImportsProvider support +// return an error when plugins are specified. +func TestValidateImportsProviderSupport(t *testing.T) { + compiler := NewCompiler() + + tests := []struct { + name string + engine CodingAgentEngine + plugins []string + wantErr bool + errContains string + }{ + { + name: "copilot with plugin - no error", + engine: &CopilotEngine{BaseEngine: BaseEngine{id: "copilot", displayName: "GitHub Copilot"}}, + plugins: []string{"my-plugin"}, + wantErr: false, + }, + { + name: "claude with plugin - no error", + engine: &ClaudeEngine{BaseEngine: BaseEngine{id: "claude", displayName: "Claude"}}, + plugins: []string{"my-plugin"}, + wantErr: false, + }, + { + name: "codex with plugin - error", + engine: &CodexEngine{BaseEngine: BaseEngine{id: "codex", displayName: "Codex"}}, + plugins: []string{"my-plugin"}, + wantErr: true, + errContains: "imports.plugins", + }, + { + name: "gemini with plugin - error", + engine: &GeminiEngine{BaseEngine: BaseEngine{id: "gemini", displayName: "Gemini"}}, + plugins: []string{"my-plugin"}, + wantErr: true, + errContains: "imports.plugins", + }, + { + name: "any engine with empty list - no error", + engine: &CodexEngine{BaseEngine: BaseEngine{id: "codex", displayName: "Codex"}}, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := compiler.validateImportsProviderSupport(tt.plugins, tt.engine) + if tt.wantErr { + require.Error(t, err, "Expected error for engine %s", tt.engine.GetID()) + assert.Contains(t, err.Error(), tt.errContains, "Error message should mention the unsupported field") + assert.Contains(t, err.Error(), tt.engine.GetID(), "Error message should mention the engine ID") + } else { + assert.NoError(t, err, "Expected no error for engine %s", tt.engine.GetID()) + } + }) + } +} + +// TestCompileWorkflow_PluginsWithUnsupportedEngine verifies that using imports.plugins +// with an engine that does not implement ImportsProvider fails with a clear error. +func TestCompileWorkflow_PluginsWithUnsupportedEngine(t *testing.T) { + tmpDir := t.TempDir() + workflowFile := filepath.Join(tmpDir, ".github", "workflows", "test-workflow.md") + require.NoError(t, os.MkdirAll(filepath.Dir(workflowFile), 0755)) + + workflowContent := `--- +on: issues +permissions: + contents: read + issues: read +engine: gemini +imports: + plugins: + - my-plugin +--- + +# Test Gemini With Plugin + +Process the issue. +` + require.NoError(t, os.WriteFile(workflowFile, []byte(workflowContent), 0600)) + + originalDir, _ := os.Getwd() + defer os.Chdir(originalDir) //nolint:errcheck + require.NoError(t, os.Chdir(tmpDir)) + + compiler := NewCompiler() + err := compiler.CompileWorkflow(workflowFile) + require.Error(t, err, "Should fail when plugins are used with gemini engine") + assert.Contains(t, err.Error(), "imports.plugins", "Error should mention the unsupported field") +} diff --git a/pkg/workflow/plugin_url.go b/pkg/workflow/plugin_url.go new file mode 100644 index 00000000000..36e1ac0dba2 --- /dev/null +++ b/pkg/workflow/plugin_url.go @@ -0,0 +1,96 @@ +// Package workflow provides compiler and runtime support for agentic workflows. +// +// This file contains helpers for parsing full GitHub plugin URLs into +// OWNER/REPO:PATH/TO/PLUGIN plugin specs, enabling users to specify a +// skill/plugin by its GitHub URL instead of remembering the separate repo +// and path components. +package workflow + +import ( + "net/url" + "strings" +) + +// parseGitHubPluginURL attempts to parse a full GitHub tree URL into an +// OWNER/REPO:PATH/TO/PLUGIN plugin spec as required by the Copilot CLI +// plugin specification. +// +// Expected URL shape: +// +// https://github.com/{owner}/{repo}/tree/{branch}/{pluginPath...} +// +// Example: +// +// https://github.com/github/copilot-plugins/tree/main/plugins/advanced-security/skills/secret-scanning +// → pluginSpec: github/copilot-plugins:advanced-security/skills/secret-scanning +// +// The leading "plugins/" directory is stripped from the path because the +// Copilot CLI's OWNER/REPO:PATH format expects the path relative to the +// "plugins/" root of the repository, not the full path from repo root. +// +// The OWNER/REPO:PATH format ("subdirectory in a repository") is a direct +// GitHub reference that does not require marketplace registration — the +// Copilot CLI resolves it straight from the repository. +// +// Returns ("", false) when the input is not a recognisable GitHub tree URL, +// so the caller can treat the value as a plain plugin name instead. +func parseGitHubPluginURL(raw string) (pluginSpec string, ok bool) { + // Only consider values that look like URLs (have a scheme) + if !strings.HasPrefix(raw, "http://") && !strings.HasPrefix(raw, "https://") { + return "", false + } + + u, err := url.Parse(raw) + if err != nil || u.Host == "" { + return "", false + } + + // Path must match /{owner}/{repo}/tree/{branch}/{pluginPath…} + // SplitN with -1 to keep all trailing segments. + trimmed := strings.TrimPrefix(u.Path, "/") + parts := strings.SplitN(trimmed, "/", -1) + + // Minimum structure: owner / repo / tree / branch / at-least-one-path-segment + if len(parts) < 5 || parts[2] != "tree" { + return "", false + } + + owner := parts[0] + repo := parts[1] + // parts[3] is the branch name; everything after is the plugin path + pluginPath := strings.Join(parts[4:], "/") + + // Strip the leading "plugins/" segment: the Copilot CLI OWNER/REPO:PATH + // format expects the path relative to the "plugins/" directory, not + // the full path from the repo root. + pluginPath = strings.TrimPrefix(pluginPath, "plugins/") + + // Build the OWNER/REPO:PATH spec — the "subdirectory in a repository" + // format accepted by the Copilot CLI. + spec := owner + "/" + repo + ":" + pluginPath + return spec, true +} + +// normalizePlugins processes a list of raw plugin entries (plain names or full +// GitHub tree URLs) and returns normalizedPlugins: plugin install arguments +// ready to pass to the engine CLI. +// +// Full GitHub tree URLs are converted to OWNER/REPO:PATH/TO/PLUGIN format +// (e.g. `github/copilot-plugins:plugins/advanced-security/skills/secret-scanning`), +// which is the "subdirectory in a repository" form accepted by the Copilot CLI. +// This format is a direct GitHub reference and does NOT require a prior +// marketplace registration step. +// +// Plain plugin names (e.g. `my-plugin`, `my-plugin@marketplace`) pass through +// unchanged. +func normalizePlugins(plugins []string) []string { + normalized := make([]string, 0, len(plugins)) + for _, entry := range plugins { + if spec, ok := parseGitHubPluginURL(entry); ok { + normalized = append(normalized, spec) + } else { + normalized = append(normalized, entry) + } + } + return normalized +} diff --git a/pkg/workflow/plugin_url_test.go b/pkg/workflow/plugin_url_test.go new file mode 100644 index 00000000000..0c497eccd07 --- /dev/null +++ b/pkg/workflow/plugin_url_test.go @@ -0,0 +1,184 @@ +//go:build !integration + +package workflow + +import ( + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseGitHubPluginURL(t *testing.T) { + tests := []struct { + name string + input string + wantPluginSpec string + wantOK bool + }{ + { + name: "full GitHub tree URL", + input: "https://github.com/github/copilot-plugins/tree/main/plugins/advanced-security/skills/secret-scanning", + wantPluginSpec: "github/copilot-plugins:advanced-security/skills/secret-scanning", + wantOK: true, + }, + { + name: "shallow plugin path", + input: "https://github.com/acme/my-plugins/tree/main/tools/linter", + wantPluginSpec: "acme/my-plugins:tools/linter", + wantOK: true, + }, + { + name: "plain plugin name – not a URL", + input: "my-extension", + wantOK: false, + }, + { + name: "repo root URL without tree path", + input: "https://github.com/github/copilot-plugins", + wantOK: false, + }, + { + name: "GitHub URL missing plugin path segment", + input: "https://github.com/github/copilot-plugins/tree/main", + wantOK: false, + }, + { + name: "non-GitHub HTTPS URL", + input: "https://example.com/some/plugin", + wantOK: false, + }, + { + name: "empty string", + input: "", + wantOK: false, + }, + { + name: "http scheme", + input: "http://github.com/org/repo/tree/main/plugins/foo", + wantPluginSpec: "org/repo:foo", + wantOK: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pluginSpec, ok := parseGitHubPluginURL(tt.input) + assert.Equal(t, tt.wantOK, ok, "ok mismatch") + if tt.wantOK { + assert.Equal(t, tt.wantPluginSpec, pluginSpec, "pluginSpec mismatch") + } + }) + } +} + +func TestNormalizePlugins(t *testing.T) { + tests := []struct { + name string + input []string + wantNormalized []string + }{ + { + name: "plain names only", + input: []string{"plugin-a", "plugin-b"}, + wantNormalized: []string{"plugin-a", "plugin-b"}, + }, + { + name: "URL entry is converted to OWNER/REPO:PATH spec", + input: []string{ + "https://github.com/github/copilot-plugins/tree/main/plugins/advanced-security/skills/secret-scanning", + }, + wantNormalized: []string{ + "github/copilot-plugins:advanced-security/skills/secret-scanning", + }, + }, + { + name: "mix of plain names and URLs", + input: []string{ + "my-extension", + "https://github.com/github/copilot-plugins/tree/main/plugins/advanced-security/skills/secret-scanning", + "another-plugin", + }, + wantNormalized: []string{ + "my-extension", + "github/copilot-plugins:advanced-security/skills/secret-scanning", + "another-plugin", + }, + }, + { + name: "empty input", + input: nil, + wantNormalized: []string{}, + }, + { + name: "two URLs from same repo", + input: []string{ + "https://github.com/github/copilot-plugins/tree/main/plugins/advanced-security/skills/secret-scanning", + "https://github.com/github/copilot-plugins/tree/main/plugins/code-review/skills/review", + }, + wantNormalized: []string{ + "github/copilot-plugins:advanced-security/skills/secret-scanning", + "github/copilot-plugins:code-review/skills/review", + }, + }, + { + name: "plugin@marketplace spec passes through unchanged", + input: []string{ + "my-plugin@my-marketplace", + }, + wantNormalized: []string{ + "my-plugin@my-marketplace", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + normalized := normalizePlugins(tt.input) + assert.Equal(t, tt.wantNormalized, normalized, "normalized plugins mismatch") + }) + } +} + +// TestGitHubTreeURLConvertedToOwnerRepoPath verifies that a GitHub tree URL +// in imports.plugins is converted to the OWNER/REPO:PATH/TO/PLUGIN format +// accepted by the Copilot CLI ("subdirectory in a repository" spec). +func TestGitHubTreeURLConvertedToOwnerRepoPath(t *testing.T) { + tmpDir := t.TempDir() + workflowsDir := tmpDir + "/.github/workflows" + require.NoError(t, os.MkdirAll(workflowsDir, 0755), "create workflows dir") + + content := `--- +name: Test GitHub Tree URL Conversion +on: + workflow_dispatch: +engine: copilot +imports: + plugins: + - https://github.com/github/copilot-plugins/tree/main/plugins/advanced-security/skills/secret-scanning +--- +Test GitHub tree URL conversion to OWNER/REPO:PATH format. +` + workflowFile := workflowsDir + "/test-workflow.md" + require.NoError(t, os.WriteFile(workflowFile, []byte(content), 0600), "write workflow") + + orig, err := os.Getwd() + require.NoError(t, err) + defer os.Chdir(orig) //nolint:errcheck + require.NoError(t, os.Chdir(tmpDir)) + + compiler := NewCompiler() + require.NoError(t, compiler.CompileWorkflow(workflowFile), "compile workflow") + + lockFile := strings.Replace(workflowFile, ".md", ".lock.yml", 1) + lockYAML, err := os.ReadFile(lockFile) + require.NoError(t, err) + + yamlStr := string(lockYAML) + // Plugin install uses OWNER/REPO:PATH format + assert.Contains(t, yamlStr, + "copilot plugin install github/copilot-plugins:advanced-security/skills/secret-scanning", + "plugin install step must use OWNER/REPO:PATH format") +}