Revert safe-outputs MCP transport to agent-job HTTP sidecar

pkg/workflow/action_pins.go

@@ -134,31 +134,6 @@ func lookupContainerPin(image string, cache *ActionCache) (ContainerPin, bool) {
 	return ContainerPin{}, false
 }
 
-// resolveContainerImage returns the digest-pinned image reference when a cache or
-// embedded container pin exists for image; otherwise it returns the original image.
-func resolveContainerImage(image string, data *WorkflowData) string {
-	var cache *ActionCache
-	if data != nil {
-		cache = data.ActionCache
-	}
-	if pin, ok := lookupContainerPin(image, cache); ok && pin.PinnedImage != "" {
-		return pin.PinnedImage
-	}
-	return image
-}
-
-// resolveMCPGatewayContainerImage returns an MCP Gateway-compatible container
-// reference. MCP Gateway container fields accept image[:tag] but not digest
-// references, so digest-pinned images are normalized back to their base image.
-func resolveMCPGatewayContainerImage(image string, data *WorkflowData) string {
-	resolved := resolveContainerImage(image, data)
-	base, _, hasDigest := strings.Cut(resolved, "@")
-	if hasDigest {
-		return base
-	}
-	return resolved
-}
-
 // getActionPinWithData returns the pinned action reference for a given action@version,
 // delegating to pkg/actionpins with a PinContext built from WorkflowData.
 func getActionPinWithData(actionRepo, version string, data *WorkflowData) (string, error) {

pkg/workflow/docker.go

@@ -49,18 +49,6 @@ func collectDockerImages(tools map[string]any, workflowData *WorkflowData, actio
 		}
 	}
 
-	// Check for safe-outputs MCP server.
-	// Safe outputs run in the published gh-aw node container and must be part of
-	// the default predownload set and lock-file manifest whenever enabled.
-	if workflowData != nil && HasSafeOutputsEnabled(workflowData.SafeOutputs) {
-		image := constants.DefaultGhAwNodeImage
-		if !imageSet[image] {
-			images = append(images, image)
-			imageSet[image] = true
-			dockerLog.Printf("Added safe-outputs MCP server container: %s", image)
-		}
-	}
-
 	// Check for agentic-workflows tool
 	// In dev mode, the image is built locally in the workflow, so don't add to pull list
 	// In release/script mode, use alpine:latest which needs to be pulled

pkg/workflow/docker_pin_test.go

@@ -41,13 +41,6 @@ func TestApplyContainerPins(t *testing.T) {
 			expectedRefs:    []string{"ghcr.io/github/gh-aw-firewall/agent:0.27.0@sha256:3816d1692e6d96887b27f1e4f1d64b8d7edb43ed9d7506b8f203913cbb81c248"},
 			expectedDigests: []string{"sha256:3816d1692e6d96887b27f1e4f1d64b8d7edb43ed9d7506b8f203913cbb81c248"},
 		},
-		{
-			name:            "embedded gh-aw-node pin used when cache is absent",
-			images:          []string{constants.DefaultGhAwNodeImage},
-			pins:            nil,
-			expectedRefs:    []string{"ghcr.io/github/gh-aw-node@sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b"},
-			expectedDigests: []string{"sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b"},
-		},
 		{
 			name:   "pinned image replaced with digest reference",
 			images: []string{"node:lts-alpine"},
@@ -129,10 +122,11 @@ func TestCollectDockerImages_StoresInWorkflowData(t *testing.T) {
 	assert.Len(t, workflowData.DockerImagePins, len(workflowData.DockerImages), "pin count should match image count")
 }
 
-// TestCollectDockerImages_SafeOutputsAddsGhAwNodeImage verifies that enabling
-// safe-outputs adds the published gh-aw-node container to the default Docker pull
-// list and manifest data, while not falling back to node:lts-alpine.
-func TestCollectDockerImages_SafeOutputsAddsGhAwNodeImage(t *testing.T) {
+// TestCollectDockerImages_SafeOutputsNoLongerPullsNodeAlpine verifies that enabling
+// safe-outputs does not add node:lts-alpine to the Docker pull list. The safe-outputs
+// MCP server runs directly via system Node (not inside a Docker container), so pulling
+// the image is both unnecessary and fails when Docker Hub is unreachable.
+func TestCollectDockerImages_SafeOutputsNoLongerPullsNodeAlpine(t *testing.T) {
 	workflowData := &WorkflowData{
 		SafeOutputs: &SafeOutputsConfig{
 			CreateIssues: &CreateIssuesConfig{
@@ -143,16 +137,6 @@ func TestCollectDockerImages_SafeOutputsAddsGhAwNodeImage(t *testing.T) {
 
 	images := collectDockerImages(map[string]any{}, workflowData, ActionModeRelease)
 
-	pinnedGhAwNodeImage := resolveContainerImage(constants.DefaultGhAwNodeImage, nil)
-	assert.Contains(t, images, pinnedGhAwNodeImage,
-		"safe-outputs should add the gh-aw-node container image to the Docker pull list")
-	require.NotEmpty(t, workflowData.DockerImagePins, "DockerImagePins should be populated")
-	assert.Contains(t, workflowData.DockerImagePins, GHAWManifestContainer{
-		Image:       constants.DefaultGhAwNodeImage,
-		Digest:      "sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b",
-		PinnedImage: pinnedGhAwNodeImage,
-	}, "safe-outputs should add gh-aw-node to manifest container pins")
-
 	for _, img := range images {
 		assert.NotContains(t, img, constants.DefaultNodeAlpineLTSImage,
 			"safe-outputs should not add node:lts-alpine (or any digest-pinned form) to the Docker pull list")

pkg/workflow/docker_predownload_test.go

@@ -15,14 +15,11 @@ import (
 )
 
 func TestDockerImagePredownload(t *testing.T) {
-	pinnedGhAwNodeImage := resolveContainerImage(constants.DefaultGhAwNodeImage, nil)
-
 	// Representative sample - tests key docker image predownload scenarios
 	tests := []struct {
 		name           string
 		frontmatter    string
 		expectedImages []string
-		manifestImages []string
 		expectStep     bool
 	}{
 		{
@@ -80,7 +77,7 @@ Test workflow with custom MCP container.`,
 			expectStep: true,
 		},
 		{
-			name: "Safe outputs predownloads gh-aw-node",
+			name: "Safe outputs does not pull node:lts-alpine",
 			frontmatter: `---
 on: issues
 engine: claude
@@ -94,12 +91,8 @@ network:
 # Test
 Test workflow - safe outputs MCP server without GitHub tool.`,
 			expectedImages: []string{
-				pinnedGhAwNodeImage,
 				"ghcr.io/github/gh-aw-mcpg:" + string(constants.DefaultMCPGatewayVersion),
 			},
-			manifestImages: []string{
-				constants.DefaultGhAwNodeImage,
-			},
 			expectStep: true,
 		},
 	}
@@ -145,29 +138,6 @@ Test workflow - safe outputs MCP server without GitHub tool.`,
 						t.Errorf("Expected to find image '%s' in generated YAML", expectedImage)
 					}
 				}
-
-				if len(tt.manifestImages) > 0 {
-					manifest, err := ExtractGHAWManifestFromLockFile(string(yaml))
-					if err != nil {
-						t.Fatalf("Failed to parse gh-aw-manifest: %v", err)
-					}
-					if manifest == nil {
-						t.Fatal("Expected gh-aw-manifest to be present")
-					}
-
-					for _, expectedImage := range tt.manifestImages {
-						foundImage := false
-						for _, container := range manifest.Containers {
-							if container.Image == expectedImage {
-								foundImage = true
-								break
-							}
-						}
-						if !foundImage {
-							t.Fatalf("Expected gh-aw-manifest containers to include %q, got %#v", expectedImage, manifest.Containers)
-						}
-					}
-				}
 			}
 		})
 	}

pkg/workflow/mcp_api_key_masking_test.go

@@ -6,13 +6,12 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/github/gh-aw/pkg/constants"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-// TestSafeOutputsNoLongerGeneratesHTTPAPIKey verifies that safe-outputs no longer
-// creates an HTTP server API key now that it runs as a stdio MCP container.
+// TestSafeOutputsAPIKeyImmediateMasking verifies that the Safe Outputs API key
+// is masked immediately after generation, before any other operations.
 func TestSafeOutputsAPIKeyImmediateMasking(t *testing.T) {
 	workflowData := &WorkflowData{
 		SafeOutputs: &SafeOutputsConfig{
@@ -30,12 +29,45 @@ func TestSafeOutputsAPIKeyImmediateMasking(t *testing.T) {
 	require.NoError(t, compiler.generateMCPSetup(&yaml, workflowData.Tools, mockEngine, workflowData))
 	output := yaml.String()
 
-	assert.NotContains(t, output, "Generate Safe Outputs MCP Server Config",
-		"Safe outputs should not have a dedicated HTTP server config step")
-	assert.NotContains(t, output, "Start Safe Outputs MCP HTTP Server",
-		"Safe outputs should not launch a dedicated HTTP server")
-	assert.Contains(t, output, `"container": "`+resolveMCPGatewayContainerImage(constants.DefaultGhAwNodeImage, nil)+`"`,
-		"Safe outputs should run in the gh-aw node container")
+	// Find the Safe Outputs config generation section
+	configStart := strings.Index(output, "Generate Safe Outputs MCP Server Config")
+	require.Greater(t, configStart, -1, "Should find Safe Outputs config generation step")
+
+	// Extract just the run script for this step
+	runStart := strings.Index(output[configStart:], "run: |")
+	require.Greater(t, runStart, -1, "Should find run script")
+	runSection := output[configStart+runStart:]
+
+	// Find the next step or end of this step's run block
+	nextStepIdx := strings.Index(runSection, "\n      - name:")
+	if nextStepIdx > 0 {
+		runSection = runSection[:nextStepIdx]
+	}
+
+	// Find the API key generation line
+	keyGenIdx := strings.Index(runSection, "API_KEY=$(openssl rand -base64 45")
+	require.Greater(t, keyGenIdx, -1, "Should find API key generation")
+
+	// Find the masking line
+	maskIdx := strings.Index(runSection, "echo \"::add-mask::${API_KEY}\"")
+	require.Greater(t, maskIdx, -1, "Should find API key masking")
+
+	// Verify masking comes immediately after generation (no PORT or other operations in between)
+	betweenGenAndMask := runSection[keyGenIdx:maskIdx]
+
+	// Should only contain the key generation line and whitespace - no PORT assignment or other operations
+	lines := strings.Split(betweenGenAndMask, "\n")
+	require.LessOrEqual(t, len(lines), 2, "Should have at most 2 lines between generation and masking (generation line + empty line)")
+
+	// Verify no PORT assignment or other operations before masking
+	assert.NotContains(t, betweenGenAndMask, "PORT=", "PORT assignment should come after masking")
+	assert.NotContains(t, betweenGenAndMask, "Set outputs", "Output setting comment should come after masking")
+
+	// Verify masking comes before PORT assignment
+	portIdx := strings.Index(runSection, "PORT=")
+	if portIdx > 0 {
+		assert.Less(t, maskIdx, portIdx, "API key masking should come before PORT assignment")
+	}
 }
 
 // TestMCPScriptsAPIKeyImmediateMasking verifies that the MCP Scripts API key