Skip to content

Commit e43b634

Browse files
github-actions[bot]Copilotdsyme
authored
chore: remove 9 unreachable dead functions (#20101)
Deleted functions identified by deadcode analyzer as unreachable from production entry points. Functions removed: 1. renderSafeOutputsMCPConfig (pkg/workflow/mcp_config_builtin.go) 2. renderCustomMCPConfigWrapper (pkg/workflow/mcp_config_custom.go) 3. renderPlaywrightMCPConfig (pkg/workflow/mcp_config_playwright_renderer.go) 4. getTypeString (pkg/workflow/mcp_config_validation.go) 5. generatePlaywrightDockerArgs (pkg/workflow/mcp_playwright_config.go) 6. generateRepoMemoryPushSteps (pkg/workflow/repo_memory.go) 7. GenerateSafeInputGoToolScriptForInspector (pkg/workflow/safe_inputs_generator.go) 8. IsSafeInputsHTTPMode (pkg/workflow/safe_inputs_parser.go) 9. getSafeInputsEnvVars (pkg/workflow/safe_inputs_renderer.go) Also removed 11 test-only functions that exclusively tested the deleted functions. Build verification: all tests pass, vet passes, fmt passes. Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Don Syme <dsyme@users.noreply.github.com>
1 parent cc0d007 commit e43b634

16 files changed

Lines changed: 1 addition & 1002 deletions

‎pkg/workflow/mcp_benchmark_test.go‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -3,50 +3,9 @@
33
package workflow
44

55
import (
6-
"strings"
76
"testing"
87
)
98

10-
// BenchmarkRenderPlaywrightMCPConfig benchmarks Playwright MCP config generation
11-
func BenchmarkRenderPlaywrightMCPConfig(b *testing.B) {
12-
playwrightTool := map[string]any{
13-
"container": "mcr.microsoft.com/playwright:v1.41.0",
14-
"args": []any{"--debug"},
15-
}
16-
playwrightConfig := parsePlaywrightTool(playwrightTool)
17-
18-
for b.Loop() {
19-
var yaml strings.Builder
20-
renderPlaywrightMCPConfig(&yaml, playwrightConfig, true)
21-
}
22-
}
23-
24-
// BenchmarkGeneratePlaywrightDockerArgs benchmarks Playwright args generation
25-
func BenchmarkGeneratePlaywrightDockerArgs(b *testing.B) {
26-
playwrightTool := map[string]any{
27-
"container": "mcr.microsoft.com/playwright:v1.41.0",
28-
}
29-
playwrightConfig := parsePlaywrightTool(playwrightTool)
30-
31-
for b.Loop() {
32-
_ = generatePlaywrightDockerArgs(playwrightConfig)
33-
}
34-
}
35-
36-
// BenchmarkRenderPlaywrightMCPConfig_Complex benchmarks complex Playwright config
37-
func BenchmarkRenderPlaywrightMCPConfig_Complex(b *testing.B) {
38-
playwrightTool := map[string]any{
39-
"container": "mcr.microsoft.com/playwright:v1.41.0",
40-
"args": []any{"--debug", "--timeout", "30000"},
41-
}
42-
playwrightConfig := parsePlaywrightTool(playwrightTool)
43-
44-
for b.Loop() {
45-
var yaml strings.Builder
46-
renderPlaywrightMCPConfig(&yaml, playwrightConfig, true)
47-
}
48-
}
49-
509
// BenchmarkExtractExpressionsFromPlaywrightArgs benchmarks expression extraction
5110
func BenchmarkExtractExpressionsFromPlaywrightArgs(b *testing.B) {
5211
customArgs := []string{"--debug", "--timeout", "${{ github.event.inputs.timeout }}"}

‎pkg/workflow/mcp_config_builtin.go‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -108,13 +108,6 @@ import (
108108

109109
var mcpBuiltinLog = logger.New("workflow:mcp-config-builtin")
110110

111-
// renderSafeOutputsMCPConfig generates the Safe Outputs MCP server configuration
112-
// This is a shared function used by both Claude and Custom engines
113-
func renderSafeOutputsMCPConfig(yaml *strings.Builder, isLast bool, workflowData *WorkflowData) {
114-
mcpBuiltinLog.Print("Rendering Safe Outputs MCP configuration")
115-
renderSafeOutputsMCPConfigWithOptions(yaml, isLast, false, workflowData)
116-
}
117-
118111
// renderSafeOutputsMCPConfigWithOptions generates the Safe Outputs MCP server configuration with engine-specific options
119112
// Now uses HTTP transport instead of stdio, similar to safe-inputs
120113
// The server is started in a separate step before the agent job

‎pkg/workflow/mcp_config_custom.go‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,32 +16,6 @@ import (
1616

1717
var mcpCustomLog = logger.New("workflow:mcp-config-custom")
1818

19-
// renderCustomMCPConfigWrapper generates custom MCP server configuration wrapper
20-
// This is a shared function used by both Claude and Custom engines
21-
func renderCustomMCPConfigWrapper(yaml *strings.Builder, toolName string, toolConfig map[string]any, isLast bool) error {
22-
mcpCustomLog.Printf("Rendering custom MCP config wrapper for tool: %s", toolName)
23-
fmt.Fprintf(yaml, " \"%s\": {\n", toolName)
24-
25-
// Use the shared MCP config renderer with JSON format
26-
renderer := MCPConfigRenderer{
27-
IndentLevel: " ",
28-
Format: "json",
29-
}
30-
31-
err := renderSharedMCPConfig(yaml, toolName, toolConfig, renderer)
32-
if err != nil {
33-
return err
34-
}
35-
36-
if isLast {
37-
yaml.WriteString(" }\n")
38-
} else {
39-
yaml.WriteString(" },\n")
40-
}
41-
42-
return nil
43-
}
44-
4519
// renderCustomMCPConfigWrapperWithContext generates custom MCP server configuration wrapper with workflow context
4620
// This version includes workflowData to determine if localhost URLs should be rewritten
4721
func renderCustomMCPConfigWrapperWithContext(yaml *strings.Builder, toolName string, toolConfig map[string]any, isLast bool, workflowData *WorkflowData) error {

‎pkg/workflow/mcp_config_playwright_renderer.go‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,6 @@ import (
6565

6666
var mcpPlaywrightLog = logger.New("workflow:mcp_config_playwright_renderer")
6767

68-
// renderPlaywrightMCPConfig generates the Playwright MCP server configuration
69-
// Uses Docker container to launch Playwright MCP for consistent browser environment
70-
// This is a shared function used by both Claude and Custom engines
71-
func renderPlaywrightMCPConfig(yaml *strings.Builder, playwrightConfig *PlaywrightToolConfig, isLast bool) {
72-
mcpPlaywrightLog.Print("Rendering Playwright MCP configuration")
73-
renderPlaywrightMCPConfigWithOptions(yaml, playwrightConfig, isLast, false, false)
74-
}
75-
7668
// renderPlaywrightMCPConfigWithOptions generates the Playwright MCP server configuration with engine-specific options
7769
// Per MCP Gateway Specification v1.0.0 section 3.2.1, stdio-based MCP servers MUST be containerized.
7870
// Uses MCP Gateway spec format: container, entrypointArgs, mounts, and args fields.

‎pkg/workflow/mcp_config_shared_test.go‎

Lines changed: 0 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -7,144 +7,6 @@ import (
77
"testing"
88
)
99

10-
// TestRenderSafeOutputsMCPConfigShared tests the shared renderSafeOutputsMCPConfig function
11-
func TestRenderSafeOutputsMCPConfigShared(t *testing.T) {
12-
tests := []struct {
13-
name string
14-
isLast bool
15-
wantContains []string
16-
wantEnding string
17-
}{
18-
{
19-
name: "safe outputs config not last",
20-
isLast: false,
21-
wantContains: []string{
22-
`"safeoutputs": {`,
23-
`"type": "http"`,
24-
`"url": "http://host.docker.internal:$GH_AW_SAFE_OUTPUTS_PORT"`,
25-
`"headers": {`,
26-
`"Authorization": "$GH_AW_SAFE_OUTPUTS_API_KEY"`,
27-
},
28-
wantEnding: "},\n",
29-
},
30-
{
31-
name: "safe outputs config is last",
32-
isLast: true,
33-
wantContains: []string{
34-
`"safeoutputs": {`,
35-
`"type": "http"`,
36-
`"url": "http://host.docker.internal:$GH_AW_SAFE_OUTPUTS_PORT"`,
37-
},
38-
wantEnding: "}\n",
39-
},
40-
}
41-
42-
for _, tt := range tests {
43-
t.Run(tt.name, func(t *testing.T) {
44-
var yaml strings.Builder
45-
renderSafeOutputsMCPConfig(&yaml, tt.isLast, nil)
46-
47-
result := yaml.String()
48-
49-
// Check all required strings are present
50-
for _, want := range tt.wantContains {
51-
if !strings.Contains(result, want) {
52-
t.Errorf("renderSafeOutputsMCPConfig() result missing %q\nGot:\n%s", want, result)
53-
}
54-
}
55-
56-
// Check correct ending
57-
if !strings.HasSuffix(result, tt.wantEnding) {
58-
// Show last part of result for debugging, but handle short strings
59-
endSnippet := result
60-
if len(result) > 10 {
61-
endSnippet = result[len(result)-10:]
62-
}
63-
t.Errorf("renderSafeOutputsMCPConfig() ending = %q, want suffix %q", endSnippet, tt.wantEnding)
64-
}
65-
})
66-
}
67-
}
68-
69-
// TestRenderCustomMCPConfigWrapperShared tests the shared renderCustomMCPConfigWrapper function
70-
func TestRenderCustomMCPConfigWrapperShared(t *testing.T) {
71-
tests := []struct {
72-
name string
73-
toolName string
74-
toolConfig map[string]any
75-
isLast bool
76-
wantContains []string
77-
wantEnding string
78-
wantError bool
79-
}{
80-
{
81-
name: "custom MCP config not last",
82-
toolName: "my-tool",
83-
toolConfig: map[string]any{
84-
"command": "node",
85-
"args": []string{"server.js"},
86-
},
87-
isLast: false,
88-
wantContains: []string{
89-
`"my-tool": {`,
90-
`"command": "node"`,
91-
},
92-
wantEnding: "},\n",
93-
wantError: false,
94-
},
95-
{
96-
name: "custom MCP config is last",
97-
toolName: "another-tool",
98-
toolConfig: map[string]any{
99-
"command": "python",
100-
"args": []string{"-m", "server"},
101-
},
102-
isLast: true,
103-
wantContains: []string{
104-
`"another-tool": {`,
105-
`"command": "python"`,
106-
},
107-
wantEnding: "}\n",
108-
wantError: false,
109-
},
110-
}
111-
112-
for _, tt := range tests {
113-
t.Run(tt.name, func(t *testing.T) {
114-
var yaml strings.Builder
115-
err := renderCustomMCPConfigWrapper(&yaml, tt.toolName, tt.toolConfig, tt.isLast)
116-
117-
if (err != nil) != tt.wantError {
118-
t.Errorf("renderCustomMCPConfigWrapper() error = %v, wantError %v", err, tt.wantError)
119-
return
120-
}
121-
122-
if tt.wantError {
123-
return
124-
}
125-
126-
result := yaml.String()
127-
128-
// Check all required strings are present
129-
for _, want := range tt.wantContains {
130-
if !strings.Contains(result, want) {
131-
t.Errorf("renderCustomMCPConfigWrapper() result missing %q\nGot:\n%s", want, result)
132-
}
133-
}
134-
135-
// Check correct ending
136-
if !strings.HasSuffix(result, tt.wantEnding) {
137-
// Show last part of result for debugging, but handle short strings
138-
endSnippet := result
139-
if len(result) > 10 {
140-
endSnippet = result[len(result)-10:]
141-
}
142-
t.Errorf("renderCustomMCPConfigWrapper() ending = %q, want suffix %q", endSnippet, tt.wantEnding)
143-
}
144-
})
145-
}
146-
}
147-
14810
// TestEngineMethodsDelegateToShared ensures engine methods properly delegate to shared functions
14911
func TestEngineMethodsDelegateToShared(t *testing.T) {
15012
t.Run("Claude engine Playwright delegation via unified renderer", func(t *testing.T) {

‎pkg/workflow/mcp_config_utils_test.go‎

Lines changed: 0 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -7,113 +7,6 @@ import (
77
"testing"
88
)
99

10-
func TestGetTypeString(t *testing.T) {
11-
tests := []struct {
12-
name string
13-
value any
14-
want string
15-
}{
16-
{
17-
name: "nil value",
18-
value: nil,
19-
want: "null",
20-
},
21-
{
22-
name: "int value",
23-
value: 42,
24-
want: "number",
25-
},
26-
{
27-
name: "int64 value",
28-
value: int64(100),
29-
want: "number",
30-
},
31-
{
32-
name: "float64 value",
33-
value: 3.14,
34-
want: "number",
35-
},
36-
{
37-
name: "float32 value",
38-
value: float32(2.71),
39-
want: "number",
40-
},
41-
{
42-
name: "boolean true",
43-
value: true,
44-
want: "boolean",
45-
},
46-
{
47-
name: "boolean false",
48-
value: false,
49-
want: "boolean",
50-
},
51-
{
52-
name: "string value",
53-
value: "hello world",
54-
want: "string",
55-
},
56-
{
57-
name: "empty string",
58-
value: "",
59-
want: "string",
60-
},
61-
{
62-
name: "object (map[string]any)",
63-
value: map[string]any{
64-
"key": "value",
65-
},
66-
want: "object",
67-
},
68-
{
69-
name: "empty object",
70-
value: map[string]any{},
71-
want: "object",
72-
},
73-
{
74-
name: "array of strings",
75-
value: []string{"a", "b", "c"},
76-
want: "array",
77-
},
78-
{
79-
name: "array of ints",
80-
value: []int{1, 2, 3},
81-
want: "array",
82-
},
83-
{
84-
name: "array of any",
85-
value: []any{"mixed", 123, true},
86-
want: "array",
87-
},
88-
{
89-
name: "empty array",
90-
value: []string{},
91-
want: "array",
92-
},
93-
{
94-
name: "array of objects",
95-
value: []map[string]any{{"key": "value"}},
96-
want: "array",
97-
},
98-
{
99-
name: "unknown type (struct)",
100-
value: struct {
101-
Name string
102-
}{Name: "test"},
103-
want: "unknown",
104-
},
105-
}
106-
107-
for _, tt := range tests {
108-
t.Run(tt.name, func(t *testing.T) {
109-
got := getTypeString(tt.value)
110-
if got != tt.want {
111-
t.Errorf("getTypeString(%v) = %v, want %v", tt.value, got, tt.want)
112-
}
113-
})
114-
}
115-
}
116-
11710
func TestWriteArgsToYAMLInline(t *testing.T) {
11811
tests := []struct {
11912
name string

0 commit comments

Comments
 (0)