diff --git a/.changeset/patch-remove-serena-local-mode.md b/.changeset/patch-remove-serena-local-mode.md new file mode 100644 index 00000000000..1797af7cfac --- /dev/null +++ b/.changeset/patch-remove-serena-local-mode.md @@ -0,0 +1,4 @@ +--- +"gh-aw": patch +--- +Remove Serena local mode, delete the unpinned start_serena_server.sh script, and add a codemod that automatically migrates `tools.serena.mode: local` to `tools.serena.mode: docker` so workflows keep compiling with the secure container-only setup. diff --git a/actions/setup/sh/start_serena_server.sh b/actions/setup/sh/start_serena_server.sh deleted file mode 100644 index 56ef2036fd7..00000000000 --- a/actions/setup/sh/start_serena_server.sh +++ /dev/null @@ -1,72 +0,0 @@ -#!/usr/bin/env bash -# Start Serena MCP HTTP Server -# This script starts the Serena MCP server using uvx and waits for it to become ready - -set -e - -# Ensure logs directory exists -mkdir -p /tmp/gh-aw/serena/logs - -echo "Starting Serena MCP server using uvx..." -echo " Port: $GH_AW_SERENA_PORT" -echo " Context: copilot" -echo " Project: $GITHUB_WORKSPACE" - -# Create initial server.log file -{ - echo "Serena MCP Server Log" - echo "Start time: $(date)" - echo "===========================================" - echo "" -} > /tmp/gh-aw/serena/logs/server.log - -# Start Serena with uvx in background with DEBUG enabled -nohup env DEBUG="*" uvx --from git+https://github.com/oraios/serena serena start-mcp-server \ - --transport streamable-http \ - --port "${GH_AW_SERENA_PORT}" \ - --context copilot \ - --project "${GITHUB_WORKSPACE}" \ - >> /tmp/gh-aw/serena/logs/server.log 2>&1 & - -SERVER_PID=$! -echo "Started Serena MCP server with PID $SERVER_PID" - -# Wait for server to be ready (max 30 seconds) -echo "Waiting for server to become ready..." -for i in {1..30}; do - # Check if process is still running - if ! kill -0 $SERVER_PID 2>/dev/null; then - echo "ERROR: Server process $SERVER_PID has died" - echo "Server log contents:" - cat /tmp/gh-aw/serena/logs/server.log - exit 1 - fi - - # Check if server is responding - if curl -s -o /dev/null -w '%{http_code}' "http://localhost:${GH_AW_SERENA_PORT}/health" | grep -q "200"; then - echo "Serena MCP server is ready (attempt $i/30)" - - # Print the startup log for debugging - echo "::notice::Serena MCP Server Startup Log" - echo "::group::Server Log Contents" - cat /tmp/gh-aw/serena/logs/server.log - echo "::endgroup::" - - break - fi - - if [ "$i" -eq 30 ]; then - echo "ERROR: Serena MCP server failed to start after 30 seconds" - echo "Process status: $(pgrep -f 'serena' || echo 'not running')" - echo "Server log contents:" - cat /tmp/gh-aw/serena/logs/server.log - echo "Checking port availability:" - netstat -tuln | grep "$GH_AW_SERENA_PORT" || echo "Port $GH_AW_SERENA_PORT not listening" - exit 1 - fi - - echo "Waiting for Serena MCP server... ($i/30)" - sleep 1 -done - -echo "Serena MCP server started successfully on port ${GH_AW_SERENA_PORT}" diff --git a/docs/src/content/docs/reference/frontmatter-full.md b/docs/src/content/docs/reference/frontmatter-full.md index c0c775c3346..d21e1814a80 100644 --- a/docs/src/content/docs/reference/frontmatter-full.md +++ b/docs/src/content/docs/reference/frontmatter-full.md @@ -1640,8 +1640,8 @@ tools: # (optional) version: null - # Serena execution mode: 'docker' (default, runs in container) or 'local' (runs - # locally with uvx and HTTP transport) + # Serena execution mode ('docker' is the only supported mode, runs in + # container) # (optional) mode: "docker" diff --git a/pkg/cli/codemod_serena_local_mode.go b/pkg/cli/codemod_serena_local_mode.go new file mode 100644 index 00000000000..f821e481892 --- /dev/null +++ b/pkg/cli/codemod_serena_local_mode.go @@ -0,0 +1,162 @@ +package cli + +import ( + "strings" + + "github.com/github/gh-aw/pkg/logger" +) + +var serenaLocalModeCodemodLog = logger.New("cli:codemod_serena_local_mode") + +// getSerenaLocalModeCodemod creates a codemod that replaces 'mode: local' with 'mode: docker' +// in tools.serena configurations. The 'local' mode executed serena via uvx directly from an +// unpinned git repository (supply chain risk) and has been removed. Docker is now the only +// supported mode. +func getSerenaLocalModeCodemod() Codemod { + return Codemod{ + ID: "serena-local-to-docker", + Name: "Migrate Serena 'mode: local' to 'mode: docker'", + Description: "Replaces 'mode: local' with 'mode: docker' in tools.serena configurations. The 'local' mode has been removed as it executed serena from an unpinned git repository (supply chain risk). Docker is now the only supported mode.", + IntroducedIn: "0.17.0", + Apply: func(content string, frontmatter map[string]any) (string, bool, error) { + // Check if tools.serena exists and has mode: local + toolsValue, hasTools := frontmatter["tools"] + if !hasTools { + return content, false, nil + } + + toolsMap, ok := toolsValue.(map[string]any) + if !ok { + return content, false, nil + } + + serenaValue, hasSerena := toolsMap["serena"] + if !hasSerena { + return content, false, nil + } + + serenaMap, ok := serenaValue.(map[string]any) + if !ok { + return content, false, nil + } + + modeValue, hasMode := serenaMap["mode"] + if !hasMode { + return content, false, nil + } + + modeStr, ok := modeValue.(string) + if !ok || modeStr != "local" { + return content, false, nil + } + + newContent, applied, err := applyFrontmatterLineTransform(content, replaceSerenaLocalModeWithDocker) + if applied { + serenaLocalModeCodemodLog.Print("Applied Serena local-to-docker migration") + } + return newContent, applied, err + }, + } +} + +// replaceSerenaLocalModeWithDocker replaces 'mode: local' with 'mode: docker' within the +// tools.serena block in frontmatter lines. +func replaceSerenaLocalModeWithDocker(lines []string) ([]string, bool) { + var result []string + var modified bool + var inTools bool + var toolsIndent string + var inSerena bool + var serenaIndent string + + for i, line := range lines { + trimmedLine := strings.TrimSpace(line) + + // Track entering the tools block + if strings.HasPrefix(trimmedLine, "tools:") && !inTools { + inTools = true + toolsIndent = getIndentation(line) + result = append(result, line) + continue + } + + // Check if we've left the tools block + if inTools && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if hasExitedBlock(line, toolsIndent) { + inTools = false + inSerena = false + } + } + + // Track entering the serena sub-block inside tools + if inTools && strings.HasPrefix(trimmedLine, "serena:") { + inSerena = true + serenaIndent = getIndentation(line) + result = append(result, line) + continue + } + + // Check if we've left the serena block + if inSerena && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if hasExitedBlock(line, serenaIndent) { + inSerena = false + } + } + + // Replace 'mode: local' with 'mode: docker' inside tools.serena + if inSerena && strings.HasPrefix(trimmedLine, "mode:") { + if strings.Contains(trimmedLine, "local") { + newLine, replaced := findAndReplaceValueInLine(line, "mode", "local", "docker") + if replaced { + result = append(result, newLine) + modified = true + serenaLocalModeCodemodLog.Printf("Replaced 'mode: local' with 'mode: docker' on line %d", i+1) + continue + } + } + } + + result = append(result, line) + } + + return result, modified +} + +// findAndReplaceValueInLine replaces oldValue with newValue for a specific key in a YAML line, +// preserving indentation and inline comments. +func findAndReplaceValueInLine(line, key, oldValue, newValue string) (string, bool) { + trimmedLine := strings.TrimSpace(line) + if !strings.HasPrefix(trimmedLine, key+":") { + return line, false + } + + leadingSpace := getIndentation(line) + _, afterColon, found := strings.Cut(line, ":") + if !found { + return line, false + } + + // Split on the first '#' to separate value from inline comment + commentIdx := strings.Index(afterColon, "#") + var valueSection, commentSection string + if commentIdx >= 0 { + valueSection = afterColon[:commentIdx] + commentSection = afterColon[commentIdx:] + } else { + valueSection = afterColon + commentSection = "" + } + + trimmedValue := strings.TrimSpace(valueSection) + if trimmedValue != oldValue { + return line, false + } + + // Preserve the whitespace between the colon and the value + spaceBeforeValue := valueSection[:strings.Index(valueSection, trimmedValue)] + newLine := leadingSpace + key + ":" + spaceBeforeValue + newValue + if commentSection != "" { + newLine += " " + commentSection + } + return newLine, true +} diff --git a/pkg/cli/codemod_serena_local_mode_test.go b/pkg/cli/codemod_serena_local_mode_test.go new file mode 100644 index 00000000000..3e730cbe866 --- /dev/null +++ b/pkg/cli/codemod_serena_local_mode_test.go @@ -0,0 +1,209 @@ +//go:build !integration + +package cli + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSerenaLocalModeCodemod(t *testing.T) { + codemod := getSerenaLocalModeCodemod() + + t.Run("replaces mode: local with mode: docker", func(t *testing.T) { + content := `--- +engine: copilot +tools: + serena: + mode: local + languages: + go: {} +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "serena": map[string]any{ + "mode": "local", + "languages": map[string]any{ + "go": map[string]any{}, + }, + }, + }, + } + + result, modified, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.True(t, modified, "Should modify content") + assert.Contains(t, result, "mode: docker", "Should have mode: docker") + assert.NotContains(t, result, "mode: local", "Should not contain mode: local") + }) + + t.Run("does not modify workflows without serena tool", func(t *testing.T) { + content := `--- +engine: copilot +tools: + github: null +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "github": nil, + }, + } + + result, modified, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.False(t, modified, "Should not modify content") + assert.Equal(t, content, result, "Content should remain unchanged") + }) + + t.Run("does not modify serena without mode field", func(t *testing.T) { + content := `--- +engine: copilot +tools: + serena: + languages: + go: {} +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "serena": map[string]any{ + "languages": map[string]any{ + "go": map[string]any{}, + }, + }, + }, + } + + result, modified, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.False(t, modified, "Should not modify content when mode is not set") + assert.Equal(t, content, result, "Content should remain unchanged") + }) + + t.Run("does not modify serena with mode: docker", func(t *testing.T) { + content := `--- +engine: copilot +tools: + serena: + mode: docker + languages: + go: {} +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "serena": map[string]any{ + "mode": "docker", + "languages": map[string]any{ + "go": map[string]any{}, + }, + }, + }, + } + + result, modified, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.False(t, modified, "Should not modify content when mode is already docker") + assert.Equal(t, content, result, "Content should remain unchanged") + }) + + t.Run("does not modify when tools section is absent", func(t *testing.T) { + content := `--- +engine: copilot +on: push +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "on": "push", + } + + result, modified, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.False(t, modified, "Should not modify content without tools") + assert.Equal(t, content, result, "Content should remain unchanged") + }) + + t.Run("preserves inline comments", func(t *testing.T) { + content := `--- +engine: copilot +tools: + serena: + mode: local # deprecated + languages: + typescript: {} +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "serena": map[string]any{ + "mode": "local", + "languages": map[string]any{ + "typescript": map[string]any{}, + }, + }, + }, + } + + result, modified, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.True(t, modified, "Should modify content") + assert.Contains(t, result, "mode: docker", "Should replace mode value") + assert.NotContains(t, result, "mode: local", "Should not contain mode: local") + assert.Contains(t, result, "# deprecated", "Should preserve inline comment") + }) + + t.Run("does not affect mode field outside tools.serena", func(t *testing.T) { + content := `--- +engine: copilot +tools: + github: + mode: local + serena: + mode: local +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "github": map[string]any{ + "mode": "local", + }, + "serena": map[string]any{ + "mode": "local", + }, + }, + } + + result, modified, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.True(t, modified, "Should modify content") + // GitHub tool mode should remain unchanged + assert.Contains(t, result, " mode: local", "GitHub tool mode should remain local") + // Serena mode should be changed to docker + assert.Contains(t, result, "mode: docker", "Serena mode should become docker") + }) +} diff --git a/pkg/cli/fix_codemods.go b/pkg/cli/fix_codemods.go index 2975951e63c..cd029f0e78f 100644 --- a/pkg/cli/fix_codemods.go +++ b/pkg/cli/fix_codemods.go @@ -42,5 +42,6 @@ func GetAllCodemods() []Codemod { getAssignToAgentDefaultAgentCodemod(), // Rename deprecated default-agent to name in assign-to-agent getPlaywrightDomainsCodemod(), // Migrate tools.playwright.allowed_domains to network.allowed getExpiresIntegerToStringCodemod(), // Convert expires integer (days) to string with 'd' suffix + getSerenaLocalModeCodemod(), // Replace tools.serena mode: local with mode: docker } } diff --git a/pkg/cli/fix_codemods_test.go b/pkg/cli/fix_codemods_test.go index 213f0e02436..947941eb76e 100644 --- a/pkg/cli/fix_codemods_test.go +++ b/pkg/cli/fix_codemods_test.go @@ -43,7 +43,7 @@ func TestGetAllCodemods_ReturnsAllCodemods(t *testing.T) { codemods := GetAllCodemods() // Verify we have the expected number of codemods - expectedCount := 24 + expectedCount := 25 assert.Len(t, codemods, expectedCount, "Should return all %d codemods", expectedCount) // Verify all codemods have required fields @@ -128,6 +128,7 @@ func TestGetAllCodemods_InExpectedOrder(t *testing.T) { "assign-to-agent-default-agent-to-name", "playwright-allowed-domains-migration", "expires-integer-to-string", + "serena-local-to-docker", } require.Len(t, codemods, len(expectedOrder), "Should have expected number of codemods") diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index daaf2f8201a..34c71795ffb 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -3400,8 +3400,8 @@ }, "mode": { "type": "string", - "description": "Serena execution mode: 'docker' (default, runs in container) or 'local' (runs locally with uvx and HTTP transport)", - "enum": ["docker", "local"], + "description": "Serena execution mode (only 'docker' is supported, runs in container)", + "enum": ["docker"], "default": "docker" }, "args": { diff --git a/pkg/workflow/docker.go b/pkg/workflow/docker.go index 1e6b95701c8..6a4339fb6e1 100644 --- a/pkg/workflow/docker.go +++ b/pkg/workflow/docker.go @@ -39,20 +39,17 @@ func collectDockerImages(tools map[string]any, workflowData *WorkflowData, actio } } - // Check for Serena tool (uses Docker image when not in local mode) + // Check for Serena tool (uses Docker image) if serenaTool, hasSerena := tools["serena"]; hasSerena { - // Only add if NOT using local mode (local mode uses uvx, not Docker) - if workflowData != nil && !isSerenaInLocalMode(workflowData.ParsedTools) { - // Select the appropriate Serena container image based on configured languages - // selectSerenaContainer() returns the base image path (e.g., "ghcr.io/github/serena-mcp-server") - // which we then tag with ":latest" to match the MCP config renderer - containerImage := selectSerenaContainer(serenaTool) - image := containerImage + ":latest" - if !imageSet[image] { - images = append(images, image) - imageSet[image] = true - dockerLog.Printf("Added Serena MCP server container: %s", image) - } + // Select the appropriate Serena container image based on configured languages + // selectSerenaContainer() returns the base image path (e.g., "ghcr.io/github/serena-mcp-server") + // which we then tag with ":latest" to match the MCP config renderer + containerImage := selectSerenaContainer(serenaTool) + image := containerImage + ":latest" + if !imageSet[image] { + images = append(images, image) + imageSet[image] = true + dockerLog.Printf("Added Serena MCP server container: %s", image) } } diff --git a/pkg/workflow/docker_predownload_test.go b/pkg/workflow/docker_predownload_test.go index 6fddd156900..1f5c223ff2a 100644 --- a/pkg/workflow/docker_predownload_test.go +++ b/pkg/workflow/docker_predownload_test.go @@ -113,27 +113,6 @@ Test workflow with Serena tool.`, }, expectStep: true, }, - { - name: "Serena tool in local mode does not generate docker image", - frontmatter: `--- -on: issues -strict: false -engine: claude -tools: - serena: - mode: local - languages: - go: - typescript: ---- - -# Test -Test workflow with Serena tool in local mode.`, - expectedImages: []string{ - "ghcr.io/github/gh-aw-mcpg:" + string(constants.DefaultMCPGatewayVersion), - }, - expectStep: true, - }, { name: "Serena tool with GitHub tool both generate images", frontmatter: `--- diff --git a/pkg/workflow/importable_tools_test.go b/pkg/workflow/importable_tools_test.go index 2bc611bab16..d1bc8396b65 100644 --- a/pkg/workflow/importable_tools_test.go +++ b/pkg/workflow/importable_tools_test.go @@ -934,175 +934,3 @@ Uses all imported neutral tools. t.Error("Expected compiled workflow to contain startup-timeout configuration (90 seconds)") } } - -// TestImportSerenaLocalMode tests serena with local mode using uvx -func TestImportSerenaLocalMode(t *testing.T) { - tempDir := testutil.TempDir(t, "test-*") - - // Create a shared workflow with serena in local mode - sharedPath := filepath.Join(tempDir, "shared-serena-local.md") - sharedContent := `--- -description: "Shared serena in local mode" -tools: - serena: - mode: local - languages: - go: {} ---- - -# Shared Serena Local Mode Configuration -` - if err := os.WriteFile(sharedPath, []byte(sharedContent), 0644); err != nil { - t.Fatalf("Failed to write shared file: %v", err) - } - - // Create main workflow that imports serena in local mode - workflowPath := filepath.Join(tempDir, "main-workflow.md") - workflowContent := `--- -on: issues -engine: copilot -imports: - - shared-serena-local.md -permissions: - contents: read - issues: read - pull-requests: read ---- - -# Main Workflow - -Uses imported serena in local mode. -` - if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil { - t.Fatalf("Failed to write workflow file: %v", err) - } - - // Compile the workflow - compiler := workflow.NewCompiler() - if err := compiler.CompileWorkflow(workflowPath); err != nil { - t.Fatalf("CompileWorkflow failed: %v", err) - } - - // Read the generated lock file - lockFilePath := stringutil.MarkdownToLockFile(workflowPath) - lockFileContent, err := os.ReadFile(lockFilePath) - if err != nil { - t.Fatalf("Failed to read lock file: %v", err) - } - - workflowData := string(lockFileContent) - - // Verify serena is configured in the MCP config - if !strings.Contains(workflowData, `"serena"`) { - t.Error("Expected compiled workflow to contain serena tool") - } - - // Verify HTTP transport is used (not container) - if !strings.Contains(workflowData, `"type": "http"`) { - t.Error("Expected serena local mode to use HTTP transport") - } - - // Verify port configuration - if !strings.Contains(workflowData, "GH_AW_SERENA_PORT") { - t.Error("Expected serena local mode to have port configuration") - } - - // Verify serena startup steps are present - if !strings.Contains(workflowData, "Start Serena MCP HTTP Server") { - t.Error("Expected serena local mode to have startup step") - } - - // Verify shell script is called - if !strings.Contains(workflowData, "start_serena_server.sh") { - t.Error("Expected serena local mode to call start_serena_server.sh") - } - - // Verify language runtime setup (Go in this case) - if !strings.Contains(workflowData, "Setup Go") { - t.Error("Expected serena local mode to setup Go runtime") - } - - // Verify uv runtime setup - if !strings.Contains(workflowData, "Setup uv") { - t.Error("Expected serena local mode to setup uv runtime") - } - - // Verify NO container is used - if strings.Contains(workflowData, "ghcr.io/github/serena-mcp-server:latest") { - t.Error("Did not expect serena local mode to use Docker container") - } -} - -// TestImportSerenaLocalModeMultipleLanguages tests serena local mode with multiple languages -func TestImportSerenaLocalModeMultipleLanguages(t *testing.T) { - tempDir := testutil.TempDir(t, "test-*") - - // Create a shared workflow with serena in local mode with multiple languages - sharedPath := filepath.Join(tempDir, "shared-serena-multi.md") - sharedContent := `--- -description: "Shared serena in local mode with multiple languages" -tools: - serena: - mode: local - languages: - go: - version: "1.21" - typescript: {} - python: {} ---- - -# Shared Serena Local Mode Multiple Languages -` - if err := os.WriteFile(sharedPath, []byte(sharedContent), 0644); err != nil { - t.Fatalf("Failed to write shared file: %v", err) - } - - // Create main workflow - workflowPath := filepath.Join(tempDir, "main-workflow.md") - workflowContent := `--- -on: issues -engine: copilot -imports: - - shared-serena-multi.md -permissions: - contents: read - issues: read ---- - -# Main Workflow -` - if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil { - t.Fatalf("Failed to write workflow file: %v", err) - } - - // Compile the workflow - compiler := workflow.NewCompiler() - if err := compiler.CompileWorkflow(workflowPath); err != nil { - t.Fatalf("CompileWorkflow failed: %v", err) - } - - // Read the generated lock file - lockFilePath := stringutil.MarkdownToLockFile(workflowPath) - lockFileContent, err := os.ReadFile(lockFilePath) - if err != nil { - t.Fatalf("Failed to read lock file: %v", err) - } - - workflowData := string(lockFileContent) - - // Verify multiple language runtimes are setup - if !strings.Contains(workflowData, "Setup Go") { - t.Error("Expected Go runtime setup") - } - if !strings.Contains(workflowData, "Setup Node.js") { - t.Error("Expected Node.js runtime setup for TypeScript") - } - if !strings.Contains(workflowData, "Setup Python") { - t.Error("Expected Python runtime setup") - } - - // Verify HTTP transport - if !strings.Contains(workflowData, `"type": "http"`) { - t.Error("Expected HTTP transport for serena local mode") - } -} diff --git a/pkg/workflow/mcp_config_comprehensive_test.go b/pkg/workflow/mcp_config_comprehensive_test.go index ef7b41f8933..eb03f12a413 100644 --- a/pkg/workflow/mcp_config_comprehensive_test.go +++ b/pkg/workflow/mcp_config_comprehensive_test.go @@ -1095,8 +1095,8 @@ func TestHasMCPConfigComprehensive(t *testing.T) { } } -// TestRenderSerenaMCPConfigLocalMode tests Serena in local mode with HTTP transport -func TestRenderSerenaMCPConfigLocalMode(t *testing.T) { +// TestRenderSerenaMCPConfigDockerMode tests Serena in docker mode with container transport +func TestRenderSerenaMCPConfigDockerMode(t *testing.T) { tests := []struct { name string serenaTool any @@ -1106,49 +1106,6 @@ func TestRenderSerenaMCPConfigLocalMode(t *testing.T) { expectedContent []string unexpectedContent []string }{ - { - name: "Serena local mode - Copilot format", - serenaTool: map[string]any{ - "mode": "local", - "languages": map[string]any{ - "go": map[string]any{}, - }, - }, - isLast: false, - includeCopilotFields: true, - inlineArgs: false, - expectedContent: []string{ - `"serena": {`, - `"type": "http"`, - `"url": "http://localhost:$GH_AW_SERENA_PORT"`, - }, - unexpectedContent: []string{ - `"container"`, - `"entrypoint"`, - `"Authorization"`, - }, - }, - { - name: "Serena local mode - Claude format", - serenaTool: map[string]any{ - "mode": "local", - "languages": map[string]any{ - "typescript": map[string]any{}, - }, - }, - isLast: true, - includeCopilotFields: false, - inlineArgs: false, - expectedContent: []string{ - `"serena": {`, - `"url": "http://localhost:$GH_AW_SERENA_PORT"`, - }, - unexpectedContent: []string{ - `"container"`, - `"type"`, - `"Authorization"`, - }, - }, { name: "Serena docker mode - should use container", serenaTool: map[string]any{ diff --git a/pkg/workflow/mcp_config_serena_renderer.go b/pkg/workflow/mcp_config_serena_renderer.go index c0515310e44..ba01a01fa1a 100644 --- a/pkg/workflow/mcp_config_serena_renderer.go +++ b/pkg/workflow/mcp_config_serena_renderer.go @@ -85,81 +85,61 @@ func selectSerenaContainer(serenaTool any) string { } // renderSerenaMCPConfigWithOptions generates the Serena MCP server configuration with engine-specific options -// Supports two modes: -// - "docker" (default): Uses Docker container with stdio transport (ghcr.io/github/serena-mcp-server:latest) -// - "local": Uses local uvx with HTTP transport on fixed port +// Uses Docker container with stdio transport (ghcr.io/github/serena-mcp-server:latest) func renderSerenaMCPConfigWithOptions(yaml *strings.Builder, serenaTool any, isLast bool, includeCopilotFields bool, inlineArgs bool) { customArgs := getSerenaCustomArgs(serenaTool) - // Determine the mode - check if serenaTool is a map with mode field - mode := "docker" // default - if toolMap, ok := serenaTool.(map[string]any); ok { - if modeStr, ok := toolMap["mode"].(string); ok { - mode = modeStr - } - } - yaml.WriteString(" \"serena\": {\n") - if mode == "local" { - // Local mode: use HTTP transport - // The MCP server is started in a separate step using uvx - if includeCopilotFields { - yaml.WriteString(" \"type\": \"http\",\n") - } + // Docker mode: use stdio transport (default behavior) + // Add type field for Copilot (per MCP Gateway Specification v1.0.0, use "stdio" for containerized servers) + if includeCopilotFields { + yaml.WriteString(" \"type\": \"stdio\",\n") + } + + // Select the appropriate Serena container based on requested languages + containerImage := selectSerenaContainer(serenaTool) + yaml.WriteString(" \"container\": \"" + containerImage + ":latest\",\n") - yaml.WriteString(" \"url\": \"http://localhost:$GH_AW_SERENA_PORT\"\n") + // Docker runtime args (--network host for network access) + if inlineArgs { + yaml.WriteString(" \"args\": [\"--network\", \"host\"],\n") } else { - // Docker mode: use stdio transport (default behavior) - // Add type field for Copilot (per MCP Gateway Specification v1.0.0, use "stdio" for containerized servers) - if includeCopilotFields { - yaml.WriteString(" \"type\": \"stdio\",\n") - } + yaml.WriteString(" \"args\": [\n") + yaml.WriteString(" \"--network\",\n") + yaml.WriteString(" \"host\"\n") + yaml.WriteString(" ],\n") + } - // Select the appropriate Serena container based on requested languages - containerImage := selectSerenaContainer(serenaTool) - yaml.WriteString(" \"container\": \"" + containerImage + ":latest\",\n") - - // Docker runtime args (--network host for network access) - if inlineArgs { - yaml.WriteString(" \"args\": [\"--network\", \"host\"],\n") - } else { - yaml.WriteString(" \"args\": [\n") - yaml.WriteString(" \"--network\",\n") - yaml.WriteString(" \"host\"\n") - yaml.WriteString(" ],\n") - } + // Serena entrypoint + yaml.WriteString(" \"entrypoint\": \"serena\",\n") - // Serena entrypoint - yaml.WriteString(" \"entrypoint\": \"serena\",\n") - - // Entrypoint args for Serena MCP server - // Security: Use GITHUB_WORKSPACE environment variable instead of template expansion to prevent template injection - if inlineArgs { - yaml.WriteString(" \"entrypointArgs\": [\"start-mcp-server\", \"--context\", \"codex\", \"--project\", \"\\${GITHUB_WORKSPACE}\"") - // Append custom args if present - writeArgsToYAMLInline(yaml, customArgs) - yaml.WriteString("],\n") - } else { - yaml.WriteString(" \"entrypointArgs\": [\n") - yaml.WriteString(" \"start-mcp-server\",\n") - yaml.WriteString(" \"--context\",\n") - yaml.WriteString(" \"codex\",\n") - yaml.WriteString(" \"--project\",\n") - yaml.WriteString(" \"\\${GITHUB_WORKSPACE}\"") - // Append custom args if present - writeArgsToYAML(yaml, customArgs, " ") - yaml.WriteString("\n") - yaml.WriteString(" ],\n") - } + // Entrypoint args for Serena MCP server + // Security: Use GITHUB_WORKSPACE environment variable instead of template expansion to prevent template injection + if inlineArgs { + yaml.WriteString(" \"entrypointArgs\": [\"start-mcp-server\", \"--context\", \"codex\", \"--project\", \"\\${GITHUB_WORKSPACE}\"") + // Append custom args if present + writeArgsToYAMLInline(yaml, customArgs) + yaml.WriteString("],\n") + } else { + yaml.WriteString(" \"entrypointArgs\": [\n") + yaml.WriteString(" \"start-mcp-server\",\n") + yaml.WriteString(" \"--context\",\n") + yaml.WriteString(" \"codex\",\n") + yaml.WriteString(" \"--project\",\n") + yaml.WriteString(" \"\\${GITHUB_WORKSPACE}\"") + // Append custom args if present + writeArgsToYAML(yaml, customArgs, " ") + yaml.WriteString("\n") + yaml.WriteString(" ],\n") + } - // Add volume mount for workspace access - // Security: Use GITHUB_WORKSPACE environment variable instead of template expansion to prevent template injection - yaml.WriteString(" \"mounts\": [\"\\${GITHUB_WORKSPACE}:\\${GITHUB_WORKSPACE}:rw\"]\n") + // Add volume mount for workspace access + // Security: Use GITHUB_WORKSPACE environment variable instead of template expansion to prevent template injection + yaml.WriteString(" \"mounts\": [\"\\${GITHUB_WORKSPACE}:\\${GITHUB_WORKSPACE}:rw\"]\n") - // Note: tools field is NOT included here - the converter script adds it back - // for Copilot. This keeps the gateway config compatible with the schema. - } + // Note: tools field is NOT included here - the converter script adds it back + // for Copilot. This keeps the gateway config compatible with the schema. if isLast { yaml.WriteString(" }\n") diff --git a/pkg/workflow/mcp_environment.go b/pkg/workflow/mcp_environment.go index 16d110bb6c5..8013fb27fc1 100644 --- a/pkg/workflow/mcp_environment.go +++ b/pkg/workflow/mcp_environment.go @@ -123,11 +123,6 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor envVars["GH_AW_SAFE_OUTPUTS_API_KEY"] = "${{ steps.safe-outputs-start.outputs.api_key }}" } - // Check if serena is in local mode and add its environment variables - if workflowData != nil && isSerenaInLocalMode(workflowData.ParsedTools) { - envVars["GH_AW_SERENA_PORT"] = "${{ steps.serena-config.outputs.serena_port }}" - } - // Check for agentic-workflows GITHUB_TOKEN if hasAgenticWorkflows { envVars["GITHUB_TOKEN"] = "${{ secrets.GITHUB_TOKEN }}" diff --git a/pkg/workflow/mcp_renderer.go b/pkg/workflow/mcp_renderer.go index 242aecaf252..a95d7eb4af3 100644 --- a/pkg/workflow/mcp_renderer.go +++ b/pkg/workflow/mcp_renderer.go @@ -276,58 +276,44 @@ func (r *MCPConfigRendererUnified) RenderSerenaMCP(yaml *strings.Builder, serena func (r *MCPConfigRendererUnified) renderSerenaTOML(yaml *strings.Builder, serenaTool any) { customArgs := getSerenaCustomArgs(serenaTool) - // Determine the mode - mode := "docker" // default - if toolMap, ok := serenaTool.(map[string]any); ok { - if modeStr, ok := toolMap["mode"].(string); ok { - mode = modeStr - } - } - yaml.WriteString(" \n") yaml.WriteString(" [mcp_servers.serena]\n") - if mode == "local" { - // Local mode: use HTTP transport - yaml.WriteString(" type = \"http\"\n") - yaml.WriteString(" url = \"http://localhost:$GH_AW_SERENA_PORT\"\n") - } else { - // Docker mode: use stdio transport (default) - // Select the appropriate Serena container based on requested languages - containerImage := selectSerenaContainer(serenaTool) - yaml.WriteString(" container = \"" + containerImage + ":latest\"\n") - - // Docker runtime args (--network host for network access) - yaml.WriteString(" args = [\n") - yaml.WriteString(" \"--network\",\n") - yaml.WriteString(" \"host\",\n") - yaml.WriteString(" ]\n") - - // Serena entrypoint - yaml.WriteString(" entrypoint = \"serena\"\n") - - // Entrypoint args for Serena MCP server - yaml.WriteString(" entrypointArgs = [\n") - yaml.WriteString(" \"start-mcp-server\",\n") - yaml.WriteString(" \"--context\",\n") - yaml.WriteString(" \"codex\",\n") - yaml.WriteString(" \"--project\",\n") - // Security: Use GITHUB_WORKSPACE environment variable instead of template expansion to prevent template injection - yaml.WriteString(" \"${GITHUB_WORKSPACE}\"") - - // Append custom args if present - for _, arg := range customArgs { - yaml.WriteString(",\n") - fmt.Fprintf(yaml, " \"%s\"", arg) - } + // Docker mode: use stdio transport (default) + // Select the appropriate Serena container based on requested languages + containerImage := selectSerenaContainer(serenaTool) + yaml.WriteString(" container = \"" + containerImage + ":latest\"\n") - yaml.WriteString("\n") - yaml.WriteString(" ]\n") + // Docker runtime args (--network host for network access) + yaml.WriteString(" args = [\n") + yaml.WriteString(" \"--network\",\n") + yaml.WriteString(" \"host\",\n") + yaml.WriteString(" ]\n") - // Add volume mount for workspace access - // Security: Use GITHUB_WORKSPACE environment variable instead of template expansion to prevent template injection - yaml.WriteString(" mounts = [\"${GITHUB_WORKSPACE}:${GITHUB_WORKSPACE}:rw\"]\n") + // Serena entrypoint + yaml.WriteString(" entrypoint = \"serena\"\n") + + // Entrypoint args for Serena MCP server + yaml.WriteString(" entrypointArgs = [\n") + yaml.WriteString(" \"start-mcp-server\",\n") + yaml.WriteString(" \"--context\",\n") + yaml.WriteString(" \"codex\",\n") + yaml.WriteString(" \"--project\",\n") + // Security: Use GITHUB_WORKSPACE environment variable instead of template expansion to prevent template injection + yaml.WriteString(" \"${GITHUB_WORKSPACE}\"") + + // Append custom args if present + for _, arg := range customArgs { + yaml.WriteString(",\n") + fmt.Fprintf(yaml, " \"%s\"", arg) } + + yaml.WriteString("\n") + yaml.WriteString(" ]\n") + + // Add volume mount for workspace access + // Security: Use GITHUB_WORKSPACE environment variable instead of template expansion to prevent template injection + yaml.WriteString(" mounts = [\"${GITHUB_WORKSPACE}:${GITHUB_WORKSPACE}:rw\"]\n") } // RenderSafeOutputsMCP generates the Safe Outputs MCP server configuration diff --git a/pkg/workflow/mcp_serena_config.go b/pkg/workflow/mcp_serena_config.go deleted file mode 100644 index 33fe17b1457..00000000000 --- a/pkg/workflow/mcp_serena_config.go +++ /dev/null @@ -1,43 +0,0 @@ -package workflow - -import ( - "strings" - - "github.com/github/gh-aw/pkg/logger" -) - -var serenaConfigLog = logger.New("workflow:mcp_serena_config") - -// isSerenaInLocalMode checks if Serena tool is configured with local mode -func isSerenaInLocalMode(tools *ToolsConfig) bool { - if tools == nil || tools.Serena == nil { - return false - } - serenaConfigLog.Printf("Serena tool mode: %s", tools.Serena.Mode) - return tools.Serena.Mode == "local" -} - -// generateSerenaLocalModeSteps generates steps to start Serena MCP server locally using uvx -func generateSerenaLocalModeSteps(yaml *strings.Builder) { - serenaConfigLog.Print("Generating Serena local mode startup steps") - // Step 1: Choose port for Serena HTTP server - yaml.WriteString(" - name: Generate Serena MCP Server Config\n") - yaml.WriteString(" id: serena-config\n") - yaml.WriteString(" run: |\n") - yaml.WriteString(" PORT=4000\n") - yaml.WriteString(" \n") - yaml.WriteString(" # Set output for next steps\n") - yaml.WriteString(" echo \"serena_port=${PORT}\" >> \"$GITHUB_OUTPUT\"\n") - yaml.WriteString(" \n") - yaml.WriteString(" echo \"Serena MCP server will run on port ${PORT}\"\n") - yaml.WriteString(" \n") - - // Step 2: Start the Serena HTTP server in the background using uvx - yaml.WriteString(" - name: Start Serena MCP HTTP Server\n") - yaml.WriteString(" id: serena-start\n") - yaml.WriteString(" env:\n") - yaml.WriteString(" DEBUG: '*'\n") - yaml.WriteString(" GH_AW_SERENA_PORT: ${{ steps.serena-config.outputs.serena_port }}\n") - yaml.WriteString(" GITHUB_WORKSPACE: ${{ github.workspace }}\n") - yaml.WriteString(" run: bash /opt/gh-aw/actions/start_serena_server.sh\n") -} diff --git a/pkg/workflow/mcp_setup_generator.go b/pkg/workflow/mcp_setup_generator.go index 5e6dfdbf55a..cbc22d1c541 100644 --- a/pkg/workflow/mcp_setup_generator.go +++ b/pkg/workflow/mcp_setup_generator.go @@ -434,11 +434,6 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any, yaml.WriteString(" \n") } - // Generate Serena MCP server startup steps if in local mode - if isSerenaInLocalMode(workflowData.ParsedTools) { - generateSerenaLocalModeSteps(yaml) - } - // The MCP gateway is always enabled, even when agent sandbox is disabled // Use the engine's RenderMCPConfig method yaml.WriteString(" - name: Start MCP Gateway\n") diff --git a/pkg/workflow/runtime_detection.go b/pkg/workflow/runtime_detection.go index 9facc53dabe..1a0e9929e4b 100644 --- a/pkg/workflow/runtime_detection.go +++ b/pkg/workflow/runtime_detection.go @@ -124,21 +124,8 @@ func detectFromMCPConfigs(tools *ToolsConfig, requirements map[string]*RuntimeRe allTools := tools.ToMap() log.Printf("Scanning %d MCP configurations for runtime commands", len(allTools)) - // Note: Serena and other built-in MCP servers now run in containers and do not + // Note: Serena and other built-in MCP servers run in containers and do not // require runtime detection. Language services are provided inside the containers. - // EXCEPTION: Serena in local mode uses uvx and requires uv runtime + language runtimes - - // Check if Serena is in local mode - if so, add uvx runtime and language runtimes - if tools.Serena != nil && tools.Serena.Mode == "local" { - runtimeSetupLog.Print("Serena configured in local mode, adding uvx runtime requirement") - uvRuntime := findRuntimeByID("uv") - if uvRuntime != nil { - updateRequiredRuntime(uvRuntime, "", requirements) - } - - // Add language runtimes based on configured languages - detectSerenaLanguageRuntimes(tools.Serena, requirements) - } // Scan custom MCP tools for runtime commands // Skip containerized MCP servers as they don't need host runtime setup @@ -187,71 +174,3 @@ func updateRequiredRuntime(runtime *Runtime, newVersion string, requirements map existing.Version = newVersion } } - -// detectSerenaLanguageRuntimes detects and adds runtime requirements for Serena language services -// when running in local mode. Maps Serena language names to runtime IDs. -func detectSerenaLanguageRuntimes(serenaConfig *SerenaToolConfig, requirements map[string]*RuntimeRequirement) { - if serenaConfig == nil { - return - } - - // Map of Serena language names to runtime IDs - languageToRuntime := map[string]string{ - "go": "go", - "typescript": "node", - "javascript": "node", - "python": "python", - "java": "java", - "rust": "rust", // rust is listed as a valid Serena language but has no knownRuntime entry, so no setup step is generated - "csharp": "dotnet", - } - - // Check ShortSyntax first (array format like ["go", "typescript"]) - if len(serenaConfig.ShortSyntax) > 0 { - for _, langName := range serenaConfig.ShortSyntax { - if runtimeID, ok := languageToRuntime[langName]; ok { - runtime := findRuntimeByID(runtimeID) - if runtime != nil { - runtimeSetupLog.Printf("Serena local mode: adding runtime for language '%s' -> '%s'", langName, runtimeID) - updateRequiredRuntime(runtime, "", requirements) - } else { - runtimeSetupLog.Printf("Serena local mode: runtime '%s' not found for language '%s'", runtimeID, langName) - } - } - } - return - } - - // Check Languages map (detailed configuration) - if serenaConfig.Languages != nil { - for langName, langConfig := range serenaConfig.Languages { - if runtimeID, ok := languageToRuntime[langName]; ok { - runtime := findRuntimeByID(runtimeID) - if runtime != nil { - // Use version from language config if specified - version := "" - if langConfig != nil && langConfig.Version != "" { - version = langConfig.Version - } - - runtimeSetupLog.Printf("Serena local mode: adding runtime for language '%s' -> '%s' (version: %s)", langName, runtimeID, version) - - // For Go, check if go-mod-file is specified - if runtimeID == "go" && langConfig != nil && langConfig.GoModFile != "" { - // Create a requirement with GoModFile set - req := &RuntimeRequirement{ - Runtime: runtime, - Version: version, - GoModFile: langConfig.GoModFile, - } - requirements[runtimeID] = req - } else { - updateRequiredRuntime(runtime, version, requirements) - } - } else { - runtimeSetupLog.Printf("Serena local mode: runtime '%s' not found for language '%s'", runtimeID, langName) - } - } - } - } -} diff --git a/pkg/workflow/strict_mode_serena_test.go b/pkg/workflow/strict_mode_serena_test.go index 96a8bf49301..fdfec8dba54 100644 --- a/pkg/workflow/strict_mode_serena_test.go +++ b/pkg/workflow/strict_mode_serena_test.go @@ -3,34 +3,9 @@ package workflow import ( - "strings" "testing" ) -// TestValidateStrictTools_SerenaLocalMode tests that serena local mode is rejected in strict mode -func TestValidateStrictTools_SerenaLocalMode(t *testing.T) { - compiler := NewCompiler() - frontmatter := map[string]any{ - "on": "push", - "tools": map[string]any{ - "serena": map[string]any{ - "mode": "local", - "languages": map[string]any{ - "go": map[string]any{}, - }, - }, - }, - } - - err := compiler.validateStrictTools(frontmatter) - if err == nil { - t.Error("Expected error for serena local mode in strict mode, got nil") - } - if err != nil && !strings.Contains(err.Error(), "serena tool with 'mode: local' is not allowed") { - t.Errorf("Expected error about serena local mode, got: %v", err) - } -} - // TestValidateStrictTools_SerenaDockerMode tests that serena docker mode is allowed in strict mode func TestValidateStrictTools_SerenaDockerMode(t *testing.T) { compiler := NewCompiler() diff --git a/pkg/workflow/strict_mode_validation.go b/pkg/workflow/strict_mode_validation.go index 8a257057439..e7a6b077c43 100644 --- a/pkg/workflow/strict_mode_validation.go +++ b/pkg/workflow/strict_mode_validation.go @@ -163,21 +163,6 @@ func (c *Compiler) validateStrictTools(frontmatter map[string]any) error { return nil } - // Check if serena is configured with local mode - serenaValue, hasSerena := toolsMap["serena"] - if hasSerena { - // Check if serena is a map (detailed configuration) - if serenaConfig, ok := serenaValue.(map[string]any); ok { - // Check if mode is set to "local" - if mode, hasMode := serenaConfig["mode"]; hasMode { - if modeStr, ok := mode.(string); ok && modeStr == "local" { - strictModeValidationLog.Printf("Serena local mode validation failed") - return errors.New("strict mode: serena tool with 'mode: local' is not allowed for security reasons. Local mode runs the MCP server directly on the host without containerization, bypassing security isolation. Use 'mode: docker' (default) instead, which runs Serena in a container. See: https://github.github.com/gh-aw/reference/tools/#serena") - } - } - } - } - // Check if cache-memory is configured with scope: repo cacheMemoryValue, hasCacheMemory := toolsMap["cache-memory"] if hasCacheMemory { diff --git a/pkg/workflow/tools_parser.go b/pkg/workflow/tools_parser.go index 810b458e967..25741dbb769 100644 --- a/pkg/workflow/tools_parser.go +++ b/pkg/workflow/tools_parser.go @@ -354,11 +354,6 @@ func parseSerenaTool(val any) *SerenaToolConfig { config.Version = version } - // Parse mode field - if mode, ok := configMap["mode"].(string); ok { - config.Mode = mode - } - if args, ok := configMap["args"].([]any); ok { config.Args = make([]string, 0, len(args)) for _, item := range args { diff --git a/pkg/workflow/tools_types.go b/pkg/workflow/tools_types.go index 32df7e391f1..ef3e148cc8b 100644 --- a/pkg/workflow/tools_types.go +++ b/pkg/workflow/tools_types.go @@ -311,7 +311,6 @@ type PlaywrightToolConfig struct { type SerenaToolConfig struct { Version string `yaml:"version,omitempty"` Args []string `yaml:"args,omitempty"` - Mode string `yaml:"mode,omitempty"` // "docker" (default) or "local" (uses uvx) Languages map[string]*SerenaLangConfig `yaml:"languages,omitempty"` // ShortSyntax stores the array of language names when using short syntax (e.g., ["go", "typescript"]) ShortSyntax []string `yaml:"-"`