Skip to content

[dead-code] chore: remove dead functions — 10 functions removed #20712

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 1 commit into from
Mar 12, 2026
Merged

[dead-code] chore: remove dead functions — 10 functions removed #20712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions pkg/workflow/compiler_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,14 +321,6 @@ func (c *Compiler) GetSharedActionCache() *ActionCache {
return cache
}

// GetArtifactManager returns the artifact manager for tracking uploads/downloads
func (c *Compiler) GetArtifactManager() *ArtifactManager {
if c.artifactManager == nil {
c.artifactManager = NewArtifactManager()
}
return c.artifactManager
}

// SkipIfMatchConfig holds the configuration for skip-if-match conditions
type SkipIfMatchConfig struct {
Query string // GitHub search query to check before running workflow
Expand Down
14 changes: 0 additions & 14 deletions pkg/workflow/engine_catalog_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,6 @@ func TestEngineCatalog_IDs(t *testing.T) {
assert.Equal(t, sorted, ids, "IDs() should return IDs in sorted order")
}

// TestEngineCatalog_DisplayNames verifies that DisplayNames() returns names in sorted ID order.
func TestEngineCatalog_DisplayNames(t *testing.T) {
registry := NewEngineRegistry()
catalog := NewEngineCatalog(registry)

names := catalog.DisplayNames()
require.NotEmpty(t, names, "DisplayNames() should return a non-empty list")
assert.Len(t, names, len(catalog.IDs()), "DisplayNames() should have same length as IDs()")

// Verify display names match expected values in sorted ID order (claude, codex, copilot, gemini)
expectedNames := []string{"Claude Code", "Codex", "GitHub Copilot CLI", "Google Gemini CLI"}
assert.Equal(t, expectedNames, names, "DisplayNames() should return display names in sorted ID order")
}

// TestEngineCatalog_All verifies that All() returns all definitions in sorted ID order.
func TestEngineCatalog_All(t *testing.T) {
registry := NewEngineRegistry()
Expand Down
10 changes: 0 additions & 10 deletions pkg/workflow/engine_definition.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,16 +213,6 @@ func (c *EngineCatalog) IDs() []string {
return ids
}

// DisplayNames returns a list of engine display names in sorted ID order.
func (c *EngineCatalog) DisplayNames() []string {
ids := c.IDs()
names := make([]string, 0, len(ids))
for _, id := range ids {
names = append(names, c.definitions[id].DisplayName)
}
return names
}

// All returns all engine definitions in sorted ID order.
func (c *EngineCatalog) All() []*EngineDefinition {
ids := c.IDs()
Expand Down
51 changes: 0 additions & 51 deletions pkg/workflow/engine_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,57 +191,6 @@ func (c *Compiler) validateEngineAuthDefinition(config *EngineConfig) error {
return nil
}

// validateEngine validates that the given engine ID is supported
func (c *Compiler) validateEngine(engineID string) error {
if engineID == "" {
engineValidationLog.Print("No engine ID specified, will use default")
return nil // Empty engine is valid (will use default)
}

engineValidationLog.Printf("Validating engine ID: %s", engineID)

// First try exact match
if c.engineRegistry.IsValidEngine(engineID) {
engineValidationLog.Printf("Engine ID %s is valid (exact match)", engineID)
return nil
}

// Try prefix match for backward compatibility (e.g., "codex-experimental")
engine, err := c.engineRegistry.GetEngineByPrefix(engineID)
if err == nil {
engineValidationLog.Printf("Engine ID %s matched by prefix to: %s", engineID, engine.GetID())
return nil
}

engineValidationLog.Printf("Engine ID %s not found: %v", engineID, err)

// Get list of valid engine IDs from the engine registry
validEngines := c.engineRegistry.GetSupportedEngines()

// Try to find close matches for "did you mean" suggestion
suggestions := parser.FindClosestMatches(engineID, validEngines, 1)

// Build comma-separated list of valid engines for error message
enginesStr := strings.Join(validEngines, ", ")

// Build error message with helpful context
errMsg := fmt.Sprintf("invalid engine: %s. Valid engines are: %s.\n\nExample:\nengine: copilot\n\nSee: %s",
engineID,
enginesStr,
constants.DocsEnginesURL)

// Add "did you mean" suggestion if we found a close match
if len(suggestions) > 0 {
errMsg = fmt.Sprintf("invalid engine: %s. Valid engines are: %s.\n\nDid you mean: %s?\n\nExample:\nengine: copilot\n\nSee: %s",
engineID,
enginesStr,
suggestions[0],
constants.DocsEnginesURL)
}

return fmt.Errorf("%s", errMsg)
}

// validateSingleEngineSpecification validates that only one engine field exists across all files
func (c *Compiler) validateSingleEngineSpecification(mainEngineSetting string, includedEnginesJSON []string) (string, error) {
var allEngines []string
Comment on lines 194 to 196
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file-level documentation still lists validateEngine() as a validation function, but that method was removed in this change. Please update the header comment to reflect the current entry points (e.g., validateEngineInlineDefinition, EngineCatalog.Resolve, and validateSingleEngineSpecification) so the docs don’t reference a non-existent function.

Copilot uses AI. Check for mistakes.
Expand Down
171 changes: 0 additions & 171 deletions pkg/workflow/engine_validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,89 +7,6 @@ import (
"testing"
)

// TestValidateEngine tests the validateEngine function
func TestValidateEngine(t *testing.T) {
tests := []struct {
name string
engineID string
expectError bool
errorMsg string
}{
{
name: "empty engine ID is valid (uses default)",
engineID: "",
expectError: false,
},
{
name: "copilot engine is valid",
engineID: "copilot",
expectError: false,
},
{
name: "claude engine is valid",
engineID: "claude",
expectError: false,
},
{
name: "codex engine is valid",
engineID: "codex",
expectError: false,
},
{
name: "invalid engine ID",
engineID: "invalid-engine",
expectError: true,
errorMsg: "invalid engine",
},
{
name: "unknown engine ID",
engineID: "gpt-7",
expectError: true,
errorMsg: "invalid engine",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
compiler := NewCompiler()
err := compiler.validateEngine(tt.engineID)

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())
}
}
})
}
}

// TestValidateEngineErrorMessageQuality verifies that error messages follow the style guide
func TestValidateEngineErrorMessageQuality(t *testing.T) {
compiler := NewCompiler()
err := compiler.validateEngine("invalid-engine")

if err == nil {
t.Fatal("Expected validation to fail for invalid engine")
}

errorMsg := err.Error()

// Error should list all valid engine options
if !strings.Contains(errorMsg, "copilot") || !strings.Contains(errorMsg, "claude") ||
!strings.Contains(errorMsg, "codex") {
t.Errorf("Error message should list all valid engines (copilot, claude, codex), got: %s", errorMsg)
}

// Error should include an example
if !strings.Contains(errorMsg, "Example:") && !strings.Contains(errorMsg, "engine: copilot") {
t.Errorf("Error message should include an example, got: %s", errorMsg)
}
}

// TestValidateSingleEngineSpecification tests the validateSingleEngineSpecification function
func TestValidateSingleEngineSpecification(t *testing.T) {
tests := []struct {
Expand Down Expand Up @@ -296,94 +213,6 @@ func TestValidateSingleEngineSpecificationErrorMessageQuality(t *testing.T) {
})
}

// TestValidateEngineDidYouMean tests the "did you mean" suggestion feature
func TestValidateEngineDidYouMean(t *testing.T) {
tests := []struct {
name string
invalidEngine string
expectedSuggestion string
shouldHaveSuggestion bool
}{
{
name: "typo copiilot suggests copilot",
invalidEngine: "copiilot",
expectedSuggestion: "copilot",
shouldHaveSuggestion: true,
},
{
name: "typo claud suggests claude",
invalidEngine: "claud",
expectedSuggestion: "claude",
shouldHaveSuggestion: true,
},
{
name: "typo codec suggests codex",
invalidEngine: "codec",
expectedSuggestion: "codex",
shouldHaveSuggestion: true,
},
{
name: "case difference no suggestion (case-insensitive match)",
invalidEngine: "Copilot",
expectedSuggestion: "",
shouldHaveSuggestion: false,
},
{
name: "completely wrong gets no suggestion",
invalidEngine: "gpt4",
expectedSuggestion: "",
shouldHaveSuggestion: false,
},
{
name: "totally different gets no suggestion",
invalidEngine: "xyz",
expectedSuggestion: "",
shouldHaveSuggestion: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
compiler := NewCompiler()
err := compiler.validateEngine(tt.invalidEngine)

if err == nil {
t.Fatal("Expected validation to fail for invalid engine")
}

errorMsg := err.Error()

if tt.shouldHaveSuggestion {
// Should have "Did you mean: X?" suggestion
if !strings.Contains(errorMsg, "Did you mean:") {
t.Errorf("Expected 'Did you mean:' in error message, got: %s", errorMsg)
}

if !strings.Contains(errorMsg, tt.expectedSuggestion) {
t.Errorf("Expected suggestion '%s' in error message, got: %s",
tt.expectedSuggestion, errorMsg)
}
} else {
// Should NOT have "Did you mean:" suggestion
if strings.Contains(errorMsg, "Did you mean:") {
t.Errorf("Should not suggest anything for '%s', but got: %s",
tt.invalidEngine, errorMsg)
}
}

// All errors should still list valid engines
if !strings.Contains(errorMsg, "copilot") {
t.Errorf("Error should always list valid engines, got: %s", errorMsg)
}

// All errors should still include an example
if !strings.Contains(errorMsg, "Example:") {
t.Errorf("Error should always include an example, got: %s", errorMsg)
}
})
}
}

// TestValidatePluginSupport tests the validatePluginSupport function
func TestValidatePluginSupport(t *testing.T) {
tests := []struct {
Expand Down
48 changes: 0 additions & 48 deletions pkg/workflow/error_message_quality_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,21 +57,6 @@ func TestErrorMessageQuality(t *testing.T) {
},
shouldNotBeVague: true,
},
{
name: "invalid engine includes valid options and example",
testFunc: func() error {
c := NewCompiler()
return c.validateEngine("invalid-engine")
},
shouldContain: []string{
"invalid engine",
"Valid engines",
"copilot",
"claude",
"Example:",
},
shouldNotBeVague: true,
},
{
name: "MCP missing required property includes example",
testFunc: func() error {
Expand Down Expand Up @@ -138,20 +123,6 @@ func TestErrorMessageQuality(t *testing.T) {
shouldContain: nil,
shouldNotBeVague: false,
},
{
name: "invalid secret name includes format and example",
testFunc: func() error {
secrets := []string{"my-secret"} // Invalid: contains hyphen
return validateSecretReferences(secrets)
},
shouldContain: []string{
"invalid secret name",
"Start with an uppercase letter",
"uppercase",
"Examples:",
},
shouldNotBeVague: true,
},
{
name: "tracker-id type error shows actual type and example",
testFunc: func() error {
Expand Down Expand Up @@ -254,25 +225,6 @@ func TestErrorMessageQuality(t *testing.T) {
}
}

// TestMultipleEngineErrorMessage tests the specific error when multiple engines are defined
func TestMultipleEngineErrorMessage(t *testing.T) {
c := NewCompiler()

err := c.validateEngine("invalid")
require.Error(t, err)

// Should explain what's wrong
assert.Contains(t, err.Error(), "invalid engine")

// Should list valid options
assert.Contains(t, err.Error(), "copilot")
assert.Contains(t, err.Error(), "claude")
assert.Contains(t, err.Error(), "codex")

// Should include example
assert.Contains(t, err.Error(), "Example:")
}

// TestMCPValidationErrorQuality tests MCP validation error messages
func TestMCPValidationErrorQuality(t *testing.T) {
tests := []struct {
Expand Down
11 changes: 0 additions & 11 deletions pkg/workflow/mcp_playwright_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package workflow

import (
"strings"

"github.com/github/gh-aw/pkg/constants"
)

// PlaywrightDockerArgs represents the common Docker arguments for Playwright container
Expand All @@ -12,15 +10,6 @@ type PlaywrightDockerArgs struct {
MCPPackageVersion string // Version for NPM package (@playwright/mcp@version)
}

func getPlaywrightDockerImageVersion(playwrightConfig *PlaywrightToolConfig) string {
playwrightDockerImageVersion := string(constants.DefaultPlaywrightBrowserVersion) // Default Playwright browser Docker image version
// Extract version setting from tool properties
if playwrightConfig != nil && playwrightConfig.Version != "" {
playwrightDockerImageVersion = playwrightConfig.Version
}
return playwrightDockerImageVersion
}

// extractExpressionsFromPlaywrightArgs extracts all GitHub Actions expressions from playwright arguments
// Returns a map of environment variable names to their original expressions
// Uses the same ExpressionExtractor as used for shell script security
Expand Down
Loading
Loading