From eaed9f8740338abb84b5bb7237f120b4acf7ef63 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 2 Dec 2025 06:14:10 +0000 Subject: [PATCH 1/3] Initial plan From 9a64e22ec9635637b9875d89f2d6b8fe8eea4ef0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 2 Dec 2025 06:20:16 +0000 Subject: [PATCH 2/3] Initial plan for engine installation steps refactoring Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com> --- go.mod | 1 - go.sum | 2 -- 2 files changed, 3 deletions(-) diff --git a/go.mod b/go.mod index a0feeafefab..fc71c2e3489 100644 --- a/go.mod +++ b/go.mod @@ -56,7 +56,6 @@ require ( github.com/thlib/go-timezone-local v0.0.7 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect github.com/yosida95/uritemplate/v3 v3.0.2 // indirect - go.uber.org/atomic v1.11.0 // indirect go.uber.org/multierr v1.11.0 // indirect golang.org/x/oauth2 v0.30.0 // indirect golang.org/x/sys v0.38.0 // indirect diff --git a/go.sum b/go.sum index 4760a45808c..e32ea9c61ec 100644 --- a/go.sum +++ b/go.sum @@ -126,8 +126,6 @@ github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavM github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM= github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zIM+UJPGz4= github.com/yosida95/uritemplate/v3 v3.0.2/go.mod h1:ILOh0sOhIJR3+L/8afwt/kE++YT040gmv5BQTMR2HP4= -go.uber.org/atomic v1.11.0 h1:ZvwS0R+56ePWxUNi+Atn9dWONBPp/AUETXlHW0DxSjE= -go.uber.org/atomic v1.11.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI= From 502c34853e82b3303d820b27c0bfa14ebb930084 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 2 Dec 2025 06:34:17 +0000 Subject: [PATCH 3/3] refactor: standardize engine installation steps using template method pattern - Create EngineInstallConfig struct to centralize engine configuration - Implement GetBaseInstallationSteps() for common patterns (secret validation + npm install) - Refactor claude_engine.go to use base method + Claude-specific network hook steps - Refactor codex_engine.go to use base method directly (simplest case) - Refactor copilot_engine.go to use config struct for consistent configuration Resolves githubnext/gh-aw#4843 Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com> --- docs/src/content/docs/labs.mdx | 4 +-- pkg/workflow/claude_engine.go | 29 ++++++---------- pkg/workflow/codex_engine.go | 29 ++++++---------- pkg/workflow/copilot_engine.go | 33 ++++++++++++------ pkg/workflow/engine_helpers.go | 62 ++++++++++++++++++++++++++++++++++ 5 files changed, 106 insertions(+), 51 deletions(-) diff --git a/docs/src/content/docs/labs.mdx b/docs/src/content/docs/labs.mdx index 7393cf8caf4..36924ff9ec5 100644 --- a/docs/src/content/docs/labs.mdx +++ b/docs/src/content/docs/labs.mdx @@ -56,7 +56,7 @@ These are experimental agentic workflows used by the GitHub Next team to learn, | [Glossary Maintainer](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/glossary-maintainer.md) | copilot | [![Glossary Maintainer](https://github.com/githubnext/gh-aw/actions/workflows/glossary-maintainer.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/glossary-maintainer.lock.yml) | `0 10 * * 1-5` | - | | [Go Fan](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/go-fan.md) | claude | [![Go Fan](https://github.com/githubnext/gh-aw/actions/workflows/go-fan.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/go-fan.lock.yml) | `0 7 * * 1-5` | - | | [Go Logger Enhancement](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/go-logger.md) | claude | [![Go Logger Enhancement](https://github.com/githubnext/gh-aw/actions/workflows/go-logger.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/go-logger.lock.yml) | `0 12 * * *` | - | -| [Go Pattern Detector](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/go-pattern-detector.md) | claude | [![Go Pattern Detector](https://github.com/githubnext/gh-aw/actions/workflows/go-pattern-detector.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/go-pattern-detector.lock.yml) | - | - | +| [Go Pattern Detector](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/go-pattern-detector.md) | claude | [![Go Pattern Detector](https://github.com/githubnext/gh-aw/actions/workflows/go-pattern-detector.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/go-pattern-detector.lock.yml) | `0 14 * * 1-5` | - | | [Grumpy Code Reviewer 🔥](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/grumpy-reviewer.md) | copilot | [![Grumpy Code Reviewer 🔥](https://github.com/githubnext/gh-aw/actions/workflows/grumpy-reviewer.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/grumpy-reviewer.lock.yml) | - | `/grumpy` | | [Instructions Janitor](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/instructions-janitor.md) | claude | [![Instructions Janitor](https://github.com/githubnext/gh-aw/actions/workflows/instructions-janitor.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/instructions-janitor.lock.yml) | `0 9 * * *` | - | | [Issue Arborist](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/issue-arborist.md) | codex | [![Issue Arborist](https://github.com/githubnext/gh-aw/actions/workflows/issue-arborist.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/issue-arborist.lock.yml) | `0 9 * * *` | - | @@ -84,9 +84,9 @@ These are experimental agentic workflows used by the GitHub Next team to learn, | [Semantic Function Refactoring](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/semantic-function-refactor.md) | claude | [![Semantic Function Refactoring](https://github.com/githubnext/gh-aw/actions/workflows/semantic-function-refactor.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/semantic-function-refactor.lock.yml) | `0 8 * * *` | - | | [Smoke Claude](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/smoke-claude.md) | claude | [![Smoke Claude](https://github.com/githubnext/gh-aw/actions/workflows/smoke-claude.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/smoke-claude.lock.yml) | `0 0,6,12,18 * * *` | - | | [Smoke Codex](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/smoke-codex.md) | codex | [![Smoke Codex](https://github.com/githubnext/gh-aw/actions/workflows/smoke-codex.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/smoke-codex.lock.yml) | `0 0,6,12,18 * * *` | - | -| [Smoke Copilot](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/smoke-copilot-playwright.md) | copilot | [![Smoke Copilot](https://github.com/githubnext/gh-aw/actions/workflows/smoke-copilot-playwright.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/smoke-copilot-playwright.lock.yml) | `0 0,6,12,18 * * *` | - | | [Smoke Copilot](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/smoke-copilot.md) | copilot | [![Smoke Copilot](https://github.com/githubnext/gh-aw/actions/workflows/smoke-copilot.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/smoke-copilot.lock.yml) | `0 0,7,13,19 * * *` | - | | [Smoke Copilot No Firewall](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/smoke-copilot-no-firewall.md) | copilot | [![Smoke Copilot No Firewall](https://github.com/githubnext/gh-aw/actions/workflows/smoke-copilot-no-firewall.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/smoke-copilot-no-firewall.lock.yml) | `0 0,6,12,18 * * *` | - | +| [Smoke Copilot Playwright](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/smoke-copilot-playwright.md) | copilot | [![Smoke Copilot Playwright](https://github.com/githubnext/gh-aw/actions/workflows/smoke-copilot-playwright.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/smoke-copilot-playwright.lock.yml) | `0 0,6,12,18 * * *` | - | | [Smoke Detector - Smoke Test Failure Investigator](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/smoke-detector.md) | claude | [![Smoke Detector - Smoke Test Failure Investigator](https://github.com/githubnext/gh-aw/actions/workflows/smoke-detector.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/smoke-detector.lock.yml) | - | - | | [Smoke SRT](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/smoke-srt.md) | copilot | [![Smoke SRT](https://github.com/githubnext/gh-aw/actions/workflows/smoke-srt.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/smoke-srt.lock.yml) | - | - | | [Smoke SRT Custom Config](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/smoke-srt-custom-config.md) | copilot | [![Smoke SRT Custom Config](https://github.com/githubnext/gh-aw/actions/workflows/smoke-srt-custom-config.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/smoke-srt-custom-config.lock.yml) | - | - | diff --git a/pkg/workflow/claude_engine.go b/pkg/workflow/claude_engine.go index af9deb12e83..60763737122 100644 --- a/pkg/workflow/claude_engine.go +++ b/pkg/workflow/claude_engine.go @@ -35,25 +35,16 @@ func NewClaudeEngine() *ClaudeEngine { func (e *ClaudeEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHubActionStep { claudeLog.Printf("Generating installation steps for Claude engine: workflow=%s", workflowData.Name) - var steps []GitHubActionStep - - // Add secret validation step - Claude supports both CLAUDE_CODE_OAUTH_TOKEN and ANTHROPIC_API_KEY as fallback - secretValidation := GenerateMultiSecretValidationStep( - []string{"CLAUDE_CODE_OAUTH_TOKEN", "ANTHROPIC_API_KEY"}, - "Claude Code", - "https://githubnext.github.io/gh-aw/reference/engines/#anthropic-claude-code", - ) - steps = append(steps, secretValidation) - - // Use shared helper for standard npm installation - npmSteps := BuildStandardNpmEngineInstallSteps( - "@anthropic-ai/claude-code", - string(constants.DefaultClaudeCodeVersion), - "Install Claude Code CLI", - "claude", - workflowData, - ) - steps = append(steps, npmSteps...) + // Use base installation steps (secret validation + npm install) + steps := GetBaseInstallationSteps(EngineInstallConfig{ + Secrets: []string{"CLAUDE_CODE_OAUTH_TOKEN", "ANTHROPIC_API_KEY"}, + DocsURL: "https://githubnext.github.io/gh-aw/reference/engines/#anthropic-claude-code", + NpmPackage: "@anthropic-ai/claude-code", + Version: string(constants.DefaultClaudeCodeVersion), + Name: "Claude Code", + CliName: "claude", + InstallStepName: "Install Claude Code CLI", + }, workflowData) // Check if network permissions are configured (only for Claude engine) if workflowData.EngineConfig != nil && ShouldEnforceNetworkPermissions(workflowData.NetworkPermissions) { diff --git a/pkg/workflow/codex_engine.go b/pkg/workflow/codex_engine.go index 82bebdee868..6a3c3d995bd 100644 --- a/pkg/workflow/codex_engine.go +++ b/pkg/workflow/codex_engine.go @@ -47,25 +47,16 @@ func NewCodexEngine() *CodexEngine { func (e *CodexEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHubActionStep { codexEngineLog.Printf("Generating installation steps for Codex engine: workflow=%s", workflowData.Name) - var steps []GitHubActionStep - - // Add secret validation step - Codex supports both CODEX_API_KEY and OPENAI_API_KEY as fallback - secretValidation := GenerateMultiSecretValidationStep( - []string{"CODEX_API_KEY", "OPENAI_API_KEY"}, - "Codex", - "https://githubnext.github.io/gh-aw/reference/engines/#openai-codex", - ) - steps = append(steps, secretValidation) - - npmSteps := BuildStandardNpmEngineInstallSteps( - "@openai/codex", - string(constants.DefaultCodexVersion), - "Install Codex", - "codex", - workflowData, - ) - steps = append(steps, npmSteps...) - return steps + + // Use base installation steps (secret validation + npm install) + return GetBaseInstallationSteps(EngineInstallConfig{ + Secrets: []string{"CODEX_API_KEY", "OPENAI_API_KEY"}, + DocsURL: "https://githubnext.github.io/gh-aw/reference/engines/#openai-codex", + NpmPackage: "@openai/codex", + Version: string(constants.DefaultCodexVersion), + Name: "Codex", + CliName: "codex", + }, workflowData) } // GetDeclaredOutputFiles returns the output files that Codex may produce diff --git a/pkg/workflow/copilot_engine.go b/pkg/workflow/copilot_engine.go index 46149cd3fe5..066b88bcc2a 100644 --- a/pkg/workflow/copilot_engine.go +++ b/pkg/workflow/copilot_engine.go @@ -46,16 +46,27 @@ func (e *CopilotEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHu var steps []GitHubActionStep - // Add secret validation step with fallback to old secret name for backward compatibility + // Define engine configuration for shared validation + config := EngineInstallConfig{ + Secrets: []string{"COPILOT_GITHUB_TOKEN", "COPILOT_CLI_TOKEN"}, + DocsURL: "https://githubnext.github.io/gh-aw/reference/engines/#github-copilot-default", + NpmPackage: "@github/copilot", + Version: string(constants.DefaultCopilotVersion), + Name: "GitHub Copilot CLI", + CliName: "copilot", + InstallStepName: "Install GitHub Copilot CLI", + } + + // Add secret validation step secretValidation := GenerateMultiSecretValidationStep( - []string{"COPILOT_GITHUB_TOKEN", "COPILOT_CLI_TOKEN"}, - "GitHub Copilot CLI", - "https://githubnext.github.io/gh-aw/reference/engines/#github-copilot-default", + config.Secrets, + config.Name, + config.DocsURL, ) steps = append(steps, secretValidation) // Determine Copilot version - copilotVersion := string(constants.DefaultCopilotVersion) + copilotVersion := config.Version if workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" { copilotVersion = workflowData.EngineConfig.Version } @@ -68,20 +79,20 @@ func (e *CopilotEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHu var npmSteps []GitHubActionStep if installGlobally { npmSteps = BuildStandardNpmEngineInstallSteps( - "@github/copilot", + config.NpmPackage, copilotVersion, - "Install GitHub Copilot CLI", - "copilot", + config.InstallStepName, + config.CliName, workflowData, ) } else { // For SRT: install locally without -g flag copilotLog.Print("Using local Copilot installation for SRT compatibility") npmSteps = GenerateNpmInstallStepsWithScope( - "@github/copilot", + config.NpmPackage, copilotVersion, - "Install GitHub Copilot CLI", - "copilot", + config.InstallStepName, + config.CliName, true, // Include Node.js setup false, // Install locally, not globally ) diff --git a/pkg/workflow/engine_helpers.go b/pkg/workflow/engine_helpers.go index 89968e6d72e..821389e68d1 100644 --- a/pkg/workflow/engine_helpers.go +++ b/pkg/workflow/engine_helpers.go @@ -10,6 +10,68 @@ import ( var engineHelpersLog = logger.New("workflow:engine_helpers") +// EngineInstallConfig contains configuration for engine installation steps. +// This struct centralizes the configuration needed to generate the common +// installation steps shared by all engines (secret validation and npm installation). +type EngineInstallConfig struct { + // Secrets is a list of secret names to validate (at least one must be set) + Secrets []string + // DocsURL is the documentation URL shown when secret validation fails + DocsURL string + // NpmPackage is the npm package name (e.g., "@github/copilot") + NpmPackage string + // Version is the default version of the npm package + Version string + // Name is the engine display name for secret validation messages (e.g., "Claude Code") + Name string + // CliName is the CLI name used for cache key prefix (e.g., "copilot") + CliName string + // InstallStepName is the display name for the npm install step (e.g., "Install Claude Code CLI") + InstallStepName string +} + +// GetBaseInstallationSteps returns the common installation steps for an engine. +// This includes secret validation and npm package installation steps that are +// shared across all engines. +// +// Parameters: +// - config: Engine-specific configuration for installation +// - workflowData: The workflow data containing engine configuration +// +// Returns: +// - []GitHubActionStep: The base installation steps (secret validation + npm install) +func GetBaseInstallationSteps(config EngineInstallConfig, workflowData *WorkflowData) []GitHubActionStep { + engineHelpersLog.Printf("Generating base installation steps for %s engine: workflow=%s", config.Name, workflowData.Name) + + var steps []GitHubActionStep + + // Add secret validation step + secretValidation := GenerateMultiSecretValidationStep( + config.Secrets, + config.Name, + config.DocsURL, + ) + steps = append(steps, secretValidation) + + // Determine step name - use InstallStepName if provided, otherwise default to "Install " + stepName := config.InstallStepName + if stepName == "" { + stepName = fmt.Sprintf("Install %s", config.Name) + } + + // Add npm package installation steps + npmSteps := BuildStandardNpmEngineInstallSteps( + config.NpmPackage, + config.Version, + stepName, + config.CliName, + workflowData, + ) + steps = append(steps, npmSteps...) + + return steps +} + // ExtractAgentIdentifier extracts the agent identifier (filename without extension) from an agent file path. // This is used by the Copilot CLI which expects agent identifiers, not full paths. //