From fd55de669685b2b96f81693914c2b0f2e585411f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Jul 2026 20:50:10 +0000 Subject: [PATCH 1/5] Initial plan From aed144b9f71d63755af4d3ae2260ed595f0b0bb0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Jul 2026 21:05:17 +0000 Subject: [PATCH 2/5] Fix docker-sbx compiler runtime support Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/workflow/awf_config.go | 38 ++++++++++++++++- pkg/workflow/awf_helpers.go | 5 ++- pkg/workflow/claude_engine.go | 15 ++++++- pkg/workflow/codex_engine.go | 19 ++++++++- pkg/workflow/docker_sbx_test.go | 76 ++++++++++++++++++++++++++++++++- pkg/workflow/nodejs.go | 51 ++++++++++++++++++++++ 6 files changed, 198 insertions(+), 6 deletions(-) diff --git a/pkg/workflow/awf_config.go b/pkg/workflow/awf_config.go index dec3cb29a6e..28724d7bda9 100644 --- a/pkg/workflow/awf_config.go +++ b/pkg/workflow/awf_config.go @@ -68,8 +68,10 @@ import ( "encoding/json" "fmt" "slices" + "strconv" "strings" "sync" + "time" "github.com/santhosh-tekuri/jsonschema/v6" @@ -295,6 +297,10 @@ type AWFContainerConfig struct { // Maps to: --image-tag ImageTag string `json:"imageTag,omitempty"` + // AgentTimeout is the maximum time (in minutes) the agent command may run. + // docker-sbx requires this so AWF passes a concrete timeout to sbx exec. + AgentTimeout int `json:"agentTimeout,omitempty"` + // DockerHostPathPrefix prefixes bind-mount source paths so the Docker daemon can // resolve runner filesystem paths. Required for ARC DinD sidecar runners where the // runner and daemon have separate filesystems. @@ -561,6 +567,10 @@ func BuildAWFConfigJSON(config AWFCommandConfig) (string, error) { // ── Container section ───────────────────────────────────────────────────── awfImageTag := buildAWFImageTagWithDigests(getAWFImageTag(firewallConfig), config.WorkflowData) agentRuntime := getAgentContainerRuntime(config.WorkflowData) + agentTimeout := 0 + if isDockerSbxRuntime(config.WorkflowData) { + agentTimeout = resolveAWFContainerAgentTimeoutMinutes(config.WorkflowData) + } // containerRuntime is only emitted when the effective AWF version supports it. // Gate here to avoid sending an unrecognised field to older AWF binaries. if !awfSupportsContainerRuntime(firewallConfig) { @@ -569,9 +579,10 @@ func BuildAWFConfigJSON(config AWFCommandConfig) (string, error) { } agentRuntime = "" } - if awfImageTag != "" || isArcDindTopology(config.WorkflowData) || agentRuntime != "" { + if awfImageTag != "" || isArcDindTopology(config.WorkflowData) || agentRuntime != "" || agentTimeout > 0 { container := &AWFContainerConfig{ ImageTag: awfImageTag, + AgentTimeout: agentTimeout, ContainerRuntime: agentRuntime, } // NOTE: dockerHostPathPrefix is intentionally NOT set for arc-dind topology. @@ -589,6 +600,9 @@ func BuildAWFConfigJSON(config AWFCommandConfig) (string, error) { if agentRuntime != "" { awfConfigLog.Printf("Container section: containerRuntime=%s", agentRuntime) } + if agentTimeout > 0 { + awfConfigLog.Printf("Container section: agentTimeout=%d", agentTimeout) + } } // ── Logging section ────────────────────────────────────────────────────── @@ -620,6 +634,28 @@ func BuildAWFConfigJSON(config AWFCommandConfig) (string, error) { return jsonStr, nil } +func resolveAWFContainerAgentTimeoutMinutes(workflowData *WorkflowData) int { + defaultTimeout := compilerenv.ResolveDefaultTimeoutMinutes(int(constants.DefaultAgenticWorkflowTimeout / time.Minute)) + if workflowData == nil || workflowData.TimeoutMinutes == "" { + return defaultTimeout + } + + rawTimeout := strings.TrimSpace(workflowData.TimeoutMinutes) + if after, ok := strings.CutPrefix(rawTimeout, "timeout-minutes:"); ok { + rawTimeout = strings.TrimSpace(after) + } + + timeoutMinutes, err := strconv.Atoi(rawTimeout) + if err == nil && timeoutMinutes > 0 { + return timeoutMinutes + } + + if rawTimeout != "" { + awfConfigLog.Printf("Container section: non-numeric timeout-minutes %q cannot be mapped to agentTimeout; using default %d", rawTimeout, defaultTimeout) + } + return defaultTimeout +} + // buildAWFTopologyAttachList returns container names that AWF should attach to // the internal awf-net network when network isolation mode is enabled. // The list always includes the MCP gateway and conditionally includes the diff --git a/pkg/workflow/awf_helpers.go b/pkg/workflow/awf_helpers.go index 4f48bb34c8b..48492681d8f 100644 --- a/pkg/workflow/awf_helpers.go +++ b/pkg/workflow/awf_helpers.go @@ -607,8 +607,9 @@ func BuildAWFArgs(config AWFCommandConfig) []string { var awfArgs []string - // Add TTY flag if needed (Claude requires this) - if config.UsesTTY { + // Add TTY flag if needed (Claude requires this), except for docker-sbx where + // sbx exec --tty can terminate long-running Claude sessions prematurely. + if config.UsesTTY && !isDockerSbxRuntime(config.WorkflowData) { awfArgs = append(awfArgs, "--tty") } diff --git a/pkg/workflow/claude_engine.go b/pkg/workflow/claude_engine.go index a51ad1b4306..f64b8a8ace9 100644 --- a/pkg/workflow/claude_engine.go +++ b/pkg/workflow/claude_engine.go @@ -128,6 +128,16 @@ func (e *ClaudeEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHub true, // Claude Code requires post-install scripts for native binaries false, // Agentic engine installs should not apply npm release-age cooldown ) + if isDockerSbxRuntime(workflowData) { + npmSteps = append(npmSteps, GenerateDockerSbxNpmCLIInstallStep( + "@anthropic-ai/claude-code", + version, + "Install Claude Code CLI in docker-sbx path", + "claude", + true, + false, + )) + } return BuildNpmEngineInstallStepsWithAWF(npmSteps, workflowData) } @@ -334,8 +344,11 @@ func (e *ClaudeEngine) GetExecutionSteps(workflowData *WorkflowData, logFile str // We prepend GetNpmBinPathSetup() to the engine command so it runs inside the AWF container. npmPathSetup := GetNpmBinPathSetup() claudeCommandWithPath := fmt.Sprintf(`%s && %s`, npmPathSetup, claudeCommand) + if dockerSbxCLIPath := GetDockerSbxNpmCLIPathSetup(workflowData); dockerSbxCLIPath != "" { + claudeCommandWithPath = fmt.Sprintf("%s && %s", dockerSbxCLIPath, claudeCommandWithPath) + } // Add MCP CLI bin directory to PATH when cli-proxy is enabled - if mcpCLIPath := GetMCPCLIPathSetup(workflowData); mcpCLIPath != "" { + if mcpCLIPath := GetMCPCLIPathSetup(workflowData); mcpCLIPath != "" && !isDockerSbxRuntime(workflowData) { claudeCommandWithPath = fmt.Sprintf("%s && %s", mcpCLIPath, claudeCommandWithPath) } diff --git a/pkg/workflow/codex_engine.go b/pkg/workflow/codex_engine.go index be962046f2c..4741e2c9e77 100644 --- a/pkg/workflow/codex_engine.go +++ b/pkg/workflow/codex_engine.go @@ -122,6 +122,20 @@ func (e *CodexEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHubA "codex", workflowData, ) + if isDockerSbxRuntime(workflowData) { + version := string(constants.DefaultCodexVersion) + if workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" { + version = workflowData.EngineConfig.Version + } + steps = append(steps, GenerateDockerSbxNpmCLIInstallStep( + "@openai/codex", + version, + "Install Codex CLI in docker-sbx path", + "codex", + false, + false, + )) + } // Add AWF installation step if firewall is enabled if isFirewallEnabled(workflowData) { @@ -346,8 +360,11 @@ func (e *CodexEngine) GetExecutionSteps(workflowData *WorkflowData, logFile stri } else { codexCommandWithSetup = fmt.Sprintf(`%s && INSTRUCTION="$(cat /tmp/gh-aw/aw-prompts/prompt.txt)" && %s`, npmPathSetup, codexCommand) } + if dockerSbxCLIPath := GetDockerSbxNpmCLIPathSetup(workflowData); dockerSbxCLIPath != "" { + codexCommandWithSetup = fmt.Sprintf("%s && %s", dockerSbxCLIPath, codexCommandWithSetup) + } // Add MCP CLI bin directory to PATH when cli-proxy is enabled - if mcpCLIPath := GetMCPCLIPathSetup(workflowData); mcpCLIPath != "" { + if mcpCLIPath := GetMCPCLIPathSetup(workflowData); mcpCLIPath != "" && !isDockerSbxRuntime(workflowData) { codexCommandWithSetup = fmt.Sprintf("%s && %s", mcpCLIPath, codexCommandWithSetup) } diff --git a/pkg/workflow/docker_sbx_test.go b/pkg/workflow/docker_sbx_test.go index f13f9451f9e..a47c813c28d 100644 --- a/pkg/workflow/docker_sbx_test.go +++ b/pkg/workflow/docker_sbx_test.go @@ -204,6 +204,29 @@ func TestDockerSbxAWFArgs(t *testing.T) { assert.True(t, found, "AWF args must include --container-runtime sbx for docker-sbx runtime") } +func TestDockerSbxAWFArgsSuppressesTTY(t *testing.T) { + config := AWFCommandConfig{ + EngineName: "claude", + UsesTTY: true, + WorkflowData: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "claude"}, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{Enabled: true, Version: string(constants.AWFContainerRuntimeMinVersion)}, + }, + SandboxConfig: &SandboxConfig{ + Agent: &AgentSandboxConfig{ + ID: "awf", + Runtime: AgentRuntimeDockerSbx, + SudoExplicitlyEnabled: true, + }, + }, + }, + } + + args := BuildAWFArgs(config) + assert.NotContains(t, strings.Join(args, " "), "--tty", "docker-sbx must suppress --tty to avoid sbx pty timeouts") +} + // TestDockerSbxAWFArgsAbsentByDefault verifies that --container-runtime sbx is NOT // added when no runtime is configured. func TestDockerSbxAWFArgsAbsentByDefault(t *testing.T) { @@ -261,7 +284,8 @@ func TestDockerSbxAWFConfigJSON(t *testing.T) { EngineName: "copilot", AllowedDomains: "github.com", WorkflowData: &WorkflowData{ - EngineConfig: &EngineConfig{ID: "copilot"}, + EngineConfig: &EngineConfig{ID: "copilot"}, + TimeoutMinutes: "timeout-minutes: 30", NetworkPermissions: &NetworkPermissions{ Firewall: &FirewallConfig{Enabled: true}, }, @@ -290,6 +314,56 @@ func TestDockerSbxAWFConfigJSON(t *testing.T) { // network.isolation must be true for docker-sbx. assert.Contains(t, jsonStr, `"isolation":true`, "AWF config JSON must have network.isolation: true for docker-sbx") + + // docker-sbx must pass a concrete agent timeout to AWF. + assert.Contains(t, jsonStr, `"agentTimeout":30`, + "AWF config JSON must include container.agentTimeout for docker-sbx") +} + +func TestDockerSbxEngineCLIWiring(t *testing.T) { + workflowData := &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ID: "claude"}, + SandboxConfig: &SandboxConfig{Agent: &AgentSandboxConfig{ID: "awf", Runtime: AgentRuntimeDockerSbx, SudoExplicitlyEnabled: true}}, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{Enabled: true}, + }, + } + + t.Run("claude install and execution use sbx-visible CLI path", func(t *testing.T) { + engine := NewClaudeEngine() + installSteps := engine.GetInstallationSteps(workflowData) + installContent := strings.Join(flattenSteps(installSteps), "\n") + assert.Contains(t, installContent, `npm install --prefix "${RUNNER_TEMP}/gh-aw/mcp-cli" @anthropic-ai/claude-code@`+string(constants.DefaultClaudeCodeVersion)) + assert.Contains(t, installContent, `ln -sf "../node_modules/.bin/claude" "${RUNNER_TEMP}/gh-aw/mcp-cli/bin/claude"`) + + execSteps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/test.log") + require.NotEmpty(t, execSteps) + execContent := strings.Join(execSteps[0], "\n") + assert.Contains(t, execContent, `export PATH="${RUNNER_TEMP}/gh-aw/mcp-cli/bin:$PATH"`) + }) + + t.Run("codex install and execution use sbx-visible CLI path", func(t *testing.T) { + engine := NewCodexEngine() + workflowData.EngineConfig = &EngineConfig{ID: "codex"} + installSteps := engine.GetInstallationSteps(workflowData) + installContent := strings.Join(flattenSteps(installSteps), "\n") + assert.Contains(t, installContent, `npm install --ignore-scripts --prefix "${RUNNER_TEMP}/gh-aw/mcp-cli" @openai/codex@`+string(constants.DefaultCodexVersion)) + assert.Contains(t, installContent, `ln -sf "../node_modules/.bin/codex" "${RUNNER_TEMP}/gh-aw/mcp-cli/bin/codex"`) + + execSteps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/test.log") + require.NotEmpty(t, execSteps) + execContent := strings.Join(execSteps[0], "\n") + assert.Contains(t, execContent, `export PATH="${RUNNER_TEMP}/gh-aw/mcp-cli/bin:$PATH"`) + }) +} + +func flattenSteps(steps []GitHubActionStep) []string { + var lines []string + for _, step := range steps { + lines = append(lines, step...) + } + return lines } // TestDockerSbxNetworkIsolationAlwaysTrue verifies that isAWFNetworkIsolationEnabled diff --git a/pkg/workflow/nodejs.go b/pkg/workflow/nodejs.go index ae6a504ac30..c2eece6cae7 100644 --- a/pkg/workflow/nodejs.go +++ b/pkg/workflow/nodejs.go @@ -236,6 +236,57 @@ func GetNpmBinPathSetup() string { return `: "${RUNNER_TOOL_CACHE:?RUNNER_TOOL_CACHE must be set}"; GH_AW_TOOL_CACHE="$RUNNER_TOOL_CACHE"; export PATH="$(find "$GH_AW_TOOL_CACHE" -maxdepth 5 -type d -name bin 2>/dev/null | tr '\n' ':')$PATH"; [ -n "$GOROOT" ] && export PATH="$GOROOT/bin:$PATH" || true; [ -n "$ERLANG_HOME" ] && export PATH="$ERLANG_HOME/bin:$PATH" || true` } +// GenerateDockerSbxNpmCLIInstallStep installs an npm CLI into a runner path that is +// visible inside the docker-sbx microVM, then creates a stable bin/ symlink for the +// engine command. +func GenerateDockerSbxNpmCLIInstallStep(packageName, version, stepName, commandName string, runInstallScripts bool, cooldownEnabled bool) GitHubActionStep { + ignoreScriptsFlag := "--ignore-scripts " + if runInstallScripts { + ignoreScriptsFlag = "" + } + + var installStep GitHubActionStep + if ExpressionPattern.MatchString(version) { + installStep = GitHubActionStep{ + " - name: " + stepName, + " run: |", + " mkdir -p \"${RUNNER_TEMP}/gh-aw/mcp-cli/bin\"", + fmt.Sprintf(` npm install %s--prefix "${RUNNER_TEMP}/gh-aw/mcp-cli" %s@"${ENGINE_VERSION}"`, ignoreScriptsFlag, packageName), + fmt.Sprintf(` ln -sf "../node_modules/.bin/%s" "${RUNNER_TEMP}/gh-aw/mcp-cli/bin/%s"`, commandName, commandName), + " env:", + " ENGINE_VERSION: " + version, + } + if cooldownEnabled { + installStep = append(installStep, fmt.Sprintf(" NPM_CONFIG_MIN_RELEASE_AGE: '%d'", npmDefaultCooldownDays)) + } + return installStep + } + + installStep = GitHubActionStep{ + " - name: " + stepName, + " run: |", + " mkdir -p \"${RUNNER_TEMP}/gh-aw/mcp-cli/bin\"", + fmt.Sprintf(` npm install %s--prefix "${RUNNER_TEMP}/gh-aw/mcp-cli" %s@%s`, ignoreScriptsFlag, packageName, version), + fmt.Sprintf(` ln -sf "../node_modules/.bin/%s" "${RUNNER_TEMP}/gh-aw/mcp-cli/bin/%s"`, commandName, commandName), + } + if cooldownEnabled { + installStep = append(installStep, + " env:", + fmt.Sprintf(" NPM_CONFIG_MIN_RELEASE_AGE: '%d'", npmDefaultCooldownDays), + ) + } + return installStep +} + +// GetDockerSbxNpmCLIPathSetup returns the PATH export needed for npm CLIs that were +// staged into ${RUNNER_TEMP}/gh-aw/mcp-cli/bin for docker-sbx microVM runs. +func GetDockerSbxNpmCLIPathSetup(workflowData *WorkflowData) string { + if !isDockerSbxRuntime(workflowData) { + return "" + } + return `export PATH="${RUNNER_TEMP}/gh-aw/mcp-cli/bin:$PATH"` +} + // GenerateNpmInstallStepsWithScope generates npm installation steps with control over global vs local installation. // By default, --ignore-scripts is added to the install command to prevent pre/post install // scripts from executing (supply chain security). Pass runInstallScripts=true to allow scripts. From 94b63676313fc41625eacb484fe4a6fd93f1cf8b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Jul 2026 21:09:31 +0000 Subject: [PATCH 3/5] Clarify docker-sbx timeout fallback handling Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/workflow/awf_config.go | 4 +++- pkg/workflow/docker_sbx_test.go | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/workflow/awf_config.go b/pkg/workflow/awf_config.go index 28724d7bda9..508c789da15 100644 --- a/pkg/workflow/awf_config.go +++ b/pkg/workflow/awf_config.go @@ -635,6 +635,8 @@ func BuildAWFConfigJSON(config AWFCommandConfig) (string, error) { } func resolveAWFContainerAgentTimeoutMinutes(workflowData *WorkflowData) int { + // Reuse the workflow-level default timeout so docker-sbx inherits the same + // runtime ceiling when top-level timeout-minutes is omitted or non-numeric. defaultTimeout := compilerenv.ResolveDefaultTimeoutMinutes(int(constants.DefaultAgenticWorkflowTimeout / time.Minute)) if workflowData == nil || workflowData.TimeoutMinutes == "" { return defaultTimeout @@ -651,7 +653,7 @@ func resolveAWFContainerAgentTimeoutMinutes(workflowData *WorkflowData) int { } if rawTimeout != "" { - awfConfigLog.Printf("Container section: non-numeric timeout-minutes %q cannot be mapped to agentTimeout; using default %d", rawTimeout, defaultTimeout) + awfConfigLog.Printf("Container section: non-numeric timeout-minutes %q (for example a GitHub Actions expression) cannot be emitted in integer-only agentTimeout; using default %d", rawTimeout, defaultTimeout) } return defaultTimeout } diff --git a/pkg/workflow/docker_sbx_test.go b/pkg/workflow/docker_sbx_test.go index a47c813c28d..71b736726d3 100644 --- a/pkg/workflow/docker_sbx_test.go +++ b/pkg/workflow/docker_sbx_test.go @@ -358,6 +358,8 @@ func TestDockerSbxEngineCLIWiring(t *testing.T) { }) } +// flattenSteps joins a small slice of GitHubActionStep values so docker-sbx tests can +// assert across multi-step install blocks without repeating nested loops in each case. func flattenSteps(steps []GitHubActionStep) []string { var lines []string for _, step := range steps { From a4722c7d14454bb8a97c74627085e3d7261c8016 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Jul 2026 21:12:55 +0000 Subject: [PATCH 4/5] Document docker-sbx CLI path handling Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/workflow/awf_config.go | 2 +- pkg/workflow/claude_engine.go | 3 ++- pkg/workflow/codex_engine.go | 3 ++- pkg/workflow/nodejs.go | 4 ++-- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/workflow/awf_config.go b/pkg/workflow/awf_config.go index 508c789da15..af16bb5d2e6 100644 --- a/pkg/workflow/awf_config.go +++ b/pkg/workflow/awf_config.go @@ -653,7 +653,7 @@ func resolveAWFContainerAgentTimeoutMinutes(workflowData *WorkflowData) int { } if rawTimeout != "" { - awfConfigLog.Printf("Container section: non-numeric timeout-minutes %q (for example a GitHub Actions expression) cannot be emitted in integer-only agentTimeout; using default %d", rawTimeout, defaultTimeout) + awfConfigLog.Printf("Container section: non-numeric timeout-minutes %q (e.g. a GitHub Actions expression) cannot be emitted in integer-only agentTimeout; using default %d", rawTimeout, defaultTimeout) } return defaultTimeout } diff --git a/pkg/workflow/claude_engine.go b/pkg/workflow/claude_engine.go index f64b8a8ace9..e83c1dc1b14 100644 --- a/pkg/workflow/claude_engine.go +++ b/pkg/workflow/claude_engine.go @@ -347,7 +347,8 @@ func (e *ClaudeEngine) GetExecutionSteps(workflowData *WorkflowData, logFile str if dockerSbxCLIPath := GetDockerSbxNpmCLIPathSetup(workflowData); dockerSbxCLIPath != "" { claudeCommandWithPath = fmt.Sprintf("%s && %s", dockerSbxCLIPath, claudeCommandWithPath) } - // Add MCP CLI bin directory to PATH when cli-proxy is enabled + // Add MCP CLI bin directory to PATH when cli-proxy is enabled. docker-sbx already + // prepends the same path above for the staged engine binary, so skip the duplicate. if mcpCLIPath := GetMCPCLIPathSetup(workflowData); mcpCLIPath != "" && !isDockerSbxRuntime(workflowData) { claudeCommandWithPath = fmt.Sprintf("%s && %s", mcpCLIPath, claudeCommandWithPath) } diff --git a/pkg/workflow/codex_engine.go b/pkg/workflow/codex_engine.go index 4741e2c9e77..f9ea632611c 100644 --- a/pkg/workflow/codex_engine.go +++ b/pkg/workflow/codex_engine.go @@ -363,7 +363,8 @@ func (e *CodexEngine) GetExecutionSteps(workflowData *WorkflowData, logFile stri if dockerSbxCLIPath := GetDockerSbxNpmCLIPathSetup(workflowData); dockerSbxCLIPath != "" { codexCommandWithSetup = fmt.Sprintf("%s && %s", dockerSbxCLIPath, codexCommandWithSetup) } - // Add MCP CLI bin directory to PATH when cli-proxy is enabled + // Add MCP CLI bin directory to PATH when cli-proxy is enabled. docker-sbx already + // prepends the same path above for the staged engine binary, so skip the duplicate. if mcpCLIPath := GetMCPCLIPathSetup(workflowData); mcpCLIPath != "" && !isDockerSbxRuntime(workflowData) { codexCommandWithSetup = fmt.Sprintf("%s && %s", mcpCLIPath, codexCommandWithSetup) } diff --git a/pkg/workflow/nodejs.go b/pkg/workflow/nodejs.go index c2eece6cae7..9363bc12e9b 100644 --- a/pkg/workflow/nodejs.go +++ b/pkg/workflow/nodejs.go @@ -237,8 +237,8 @@ func GetNpmBinPathSetup() string { } // GenerateDockerSbxNpmCLIInstallStep installs an npm CLI into a runner path that is -// visible inside the docker-sbx microVM, then creates a stable bin/ symlink for the -// engine command. +// visible inside the docker-sbx microVM, then creates a stable bin/ symlink from +// ${RUNNER_TEMP}/gh-aw/mcp-cli/bin/ to the package's node_modules/.bin entry. func GenerateDockerSbxNpmCLIInstallStep(packageName, version, stepName, commandName string, runInstallScripts bool, cooldownEnabled bool) GitHubActionStep { ignoreScriptsFlag := "--ignore-scripts " if runInstallScripts { From 5a10da8811ff76f3a1110e8971c716f22ba9c905 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Jul 2026 21:59:50 +0000 Subject: [PATCH 5/5] Fix docker-sbx engine CLI path isolation and timeout fallback tests Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/workflow/awf_config_test.go | 48 +++++++++++++++++++++++++++++++++ pkg/workflow/claude_engine.go | 5 ++-- pkg/workflow/codex_engine.go | 5 ++-- pkg/workflow/docker_sbx_test.go | 38 ++++++++++++++++++++++---- pkg/workflow/nodejs.go | 18 ++++++------- 5 files changed, 94 insertions(+), 20 deletions(-) diff --git a/pkg/workflow/awf_config_test.go b/pkg/workflow/awf_config_test.go index 19b54380f31..e341ee82c57 100644 --- a/pkg/workflow/awf_config_test.go +++ b/pkg/workflow/awf_config_test.go @@ -7,6 +7,7 @@ import ( "fmt" "strings" "testing" + "time" "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/workflow/compilerenv" @@ -14,6 +15,53 @@ import ( "github.com/stretchr/testify/require" ) +func TestResolveAWFContainerAgentTimeoutMinutes(t *testing.T) { + defaultFallback := int(constants.DefaultAgenticWorkflowTimeout / time.Minute) + + t.Run("uses numeric timeout-minutes when provided", func(t *testing.T) { + t.Setenv(compilerenv.DefaultTimeoutMinutes, "") + got := resolveAWFContainerAgentTimeoutMinutes(&WorkflowData{TimeoutMinutes: "timeout-minutes: 30"}) + assert.Equal(t, 30, got) + }) + + t.Run("falls back to default when timeout-minutes is omitted or non-numeric", func(t *testing.T) { + t.Setenv(compilerenv.DefaultTimeoutMinutes, "") + tests := []struct { + name string + data *WorkflowData + }{ + {name: "nil workflow data", data: nil}, + {name: "empty timeout", data: &WorkflowData{}}, + {name: "expression timeout", data: &WorkflowData{TimeoutMinutes: "timeout-minutes: ${{ inputs.timeout-minutes }}"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := resolveAWFContainerAgentTimeoutMinutes(tt.data) + assert.Equal(t, defaultFallback, got) + }) + } + }) + + t.Run("uses GH_AW_DEFAULT_TIMEOUT_MINUTES override for omitted and non-numeric values", func(t *testing.T) { + t.Setenv(compilerenv.DefaultTimeoutMinutes, "45") + tests := []struct { + name string + data *WorkflowData + }{ + {name: "missing timeout", data: &WorkflowData{}}, + {name: "expression timeout", data: &WorkflowData{TimeoutMinutes: "timeout-minutes: ${{ inputs.timeout-minutes }}"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := resolveAWFContainerAgentTimeoutMinutes(tt.data) + assert.Equal(t, 45, got) + }) + } + }) +} + // TestBuildAWFConfigJSON verifies that BuildAWFConfigJSON produces a valid JSON config // that contains the expected network, apiProxy, and container fields. func TestBuildAWFConfigJSON(t *testing.T) { diff --git a/pkg/workflow/claude_engine.go b/pkg/workflow/claude_engine.go index e83c1dc1b14..2541d99c5fa 100644 --- a/pkg/workflow/claude_engine.go +++ b/pkg/workflow/claude_engine.go @@ -347,9 +347,8 @@ func (e *ClaudeEngine) GetExecutionSteps(workflowData *WorkflowData, logFile str if dockerSbxCLIPath := GetDockerSbxNpmCLIPathSetup(workflowData); dockerSbxCLIPath != "" { claudeCommandWithPath = fmt.Sprintf("%s && %s", dockerSbxCLIPath, claudeCommandWithPath) } - // Add MCP CLI bin directory to PATH when cli-proxy is enabled. docker-sbx already - // prepends the same path above for the staged engine binary, so skip the duplicate. - if mcpCLIPath := GetMCPCLIPathSetup(workflowData); mcpCLIPath != "" && !isDockerSbxRuntime(workflowData) { + // Add MCP CLI bin directory to PATH when cli-proxy is enabled. + if mcpCLIPath := GetMCPCLIPathSetup(workflowData); mcpCLIPath != "" { claudeCommandWithPath = fmt.Sprintf("%s && %s", mcpCLIPath, claudeCommandWithPath) } diff --git a/pkg/workflow/codex_engine.go b/pkg/workflow/codex_engine.go index f9ea632611c..e43d78c12da 100644 --- a/pkg/workflow/codex_engine.go +++ b/pkg/workflow/codex_engine.go @@ -363,9 +363,8 @@ func (e *CodexEngine) GetExecutionSteps(workflowData *WorkflowData, logFile stri if dockerSbxCLIPath := GetDockerSbxNpmCLIPathSetup(workflowData); dockerSbxCLIPath != "" { codexCommandWithSetup = fmt.Sprintf("%s && %s", dockerSbxCLIPath, codexCommandWithSetup) } - // Add MCP CLI bin directory to PATH when cli-proxy is enabled. docker-sbx already - // prepends the same path above for the staged engine binary, so skip the duplicate. - if mcpCLIPath := GetMCPCLIPathSetup(workflowData); mcpCLIPath != "" && !isDockerSbxRuntime(workflowData) { + // Add MCP CLI bin directory to PATH when cli-proxy is enabled. + if mcpCLIPath := GetMCPCLIPathSetup(workflowData); mcpCLIPath != "" { codexCommandWithSetup = fmt.Sprintf("%s && %s", mcpCLIPath, codexCommandWithSetup) } diff --git a/pkg/workflow/docker_sbx_test.go b/pkg/workflow/docker_sbx_test.go index 71b736726d3..183e9a4c7a2 100644 --- a/pkg/workflow/docker_sbx_test.go +++ b/pkg/workflow/docker_sbx_test.go @@ -334,13 +334,41 @@ func TestDockerSbxEngineCLIWiring(t *testing.T) { engine := NewClaudeEngine() installSteps := engine.GetInstallationSteps(workflowData) installContent := strings.Join(flattenSteps(installSteps), "\n") - assert.Contains(t, installContent, `npm install --prefix "${RUNNER_TEMP}/gh-aw/mcp-cli" @anthropic-ai/claude-code@`+string(constants.DefaultClaudeCodeVersion)) - assert.Contains(t, installContent, `ln -sf "../node_modules/.bin/claude" "${RUNNER_TEMP}/gh-aw/mcp-cli/bin/claude"`) + assert.Contains(t, installContent, `npm install --prefix "${RUNNER_TEMP}/gh-aw/engine-cli" @anthropic-ai/claude-code@`+string(constants.DefaultClaudeCodeVersion)) + assert.Contains(t, installContent, `ln -sf "../node_modules/.bin/claude" "${RUNNER_TEMP}/gh-aw/engine-cli/bin/claude"`) execSteps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/test.log") require.NotEmpty(t, execSteps) execContent := strings.Join(execSteps[0], "\n") + assert.Contains(t, execContent, `export PATH="${RUNNER_TEMP}/gh-aw/engine-cli/bin:$PATH"`) + }) + + t.Run("docker-sbx keeps engine and MCP CLI PATH setup independent", func(t *testing.T) { + workflowData.ParsedTools = &ToolsConfig{ + CLIProxy: true, + Custom: map[string]MCPServerConfig{ + "myserver": {}, + }, + } + workflowData.Tools = map[string]any{ + "myserver": map[string]any{ + "mode": "remote", + }, + } + + engine := NewClaudeEngine() + execSteps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/test.log") + require.NotEmpty(t, execSteps) + execContent := strings.Join(execSteps[0], "\n") + assert.Contains(t, execContent, `export PATH="${RUNNER_TEMP}/gh-aw/mcp-cli/bin:$PATH"`) + assert.Contains(t, execContent, `export PATH="${RUNNER_TEMP}/gh-aw/engine-cli/bin:$PATH"`) + + mcpIdx := strings.Index(execContent, `export PATH="${RUNNER_TEMP}/gh-aw/mcp-cli/bin:$PATH"`) + engineIdx := strings.Index(execContent, `export PATH="${RUNNER_TEMP}/gh-aw/engine-cli/bin:$PATH"`) + assert.GreaterOrEqual(t, mcpIdx, 0) + assert.GreaterOrEqual(t, engineIdx, 0) + assert.Less(t, mcpIdx, engineIdx, "engine path export must run after MCP export so engine CLI takes PATH precedence") }) t.Run("codex install and execution use sbx-visible CLI path", func(t *testing.T) { @@ -348,13 +376,13 @@ func TestDockerSbxEngineCLIWiring(t *testing.T) { workflowData.EngineConfig = &EngineConfig{ID: "codex"} installSteps := engine.GetInstallationSteps(workflowData) installContent := strings.Join(flattenSteps(installSteps), "\n") - assert.Contains(t, installContent, `npm install --ignore-scripts --prefix "${RUNNER_TEMP}/gh-aw/mcp-cli" @openai/codex@`+string(constants.DefaultCodexVersion)) - assert.Contains(t, installContent, `ln -sf "../node_modules/.bin/codex" "${RUNNER_TEMP}/gh-aw/mcp-cli/bin/codex"`) + assert.Contains(t, installContent, `npm install --ignore-scripts --prefix "${RUNNER_TEMP}/gh-aw/engine-cli" @openai/codex@`+string(constants.DefaultCodexVersion)) + assert.Contains(t, installContent, `ln -sf "../node_modules/.bin/codex" "${RUNNER_TEMP}/gh-aw/engine-cli/bin/codex"`) execSteps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/test.log") require.NotEmpty(t, execSteps) execContent := strings.Join(execSteps[0], "\n") - assert.Contains(t, execContent, `export PATH="${RUNNER_TEMP}/gh-aw/mcp-cli/bin:$PATH"`) + assert.Contains(t, execContent, `export PATH="${RUNNER_TEMP}/gh-aw/engine-cli/bin:$PATH"`) }) } diff --git a/pkg/workflow/nodejs.go b/pkg/workflow/nodejs.go index 9363bc12e9b..d3b3908f126 100644 --- a/pkg/workflow/nodejs.go +++ b/pkg/workflow/nodejs.go @@ -238,7 +238,7 @@ func GetNpmBinPathSetup() string { // GenerateDockerSbxNpmCLIInstallStep installs an npm CLI into a runner path that is // visible inside the docker-sbx microVM, then creates a stable bin/ symlink from -// ${RUNNER_TEMP}/gh-aw/mcp-cli/bin/ to the package's node_modules/.bin entry. +// ${RUNNER_TEMP}/gh-aw/engine-cli/bin/ to the package's node_modules/.bin entry. func GenerateDockerSbxNpmCLIInstallStep(packageName, version, stepName, commandName string, runInstallScripts bool, cooldownEnabled bool) GitHubActionStep { ignoreScriptsFlag := "--ignore-scripts " if runInstallScripts { @@ -250,9 +250,9 @@ func GenerateDockerSbxNpmCLIInstallStep(packageName, version, stepName, commandN installStep = GitHubActionStep{ " - name: " + stepName, " run: |", - " mkdir -p \"${RUNNER_TEMP}/gh-aw/mcp-cli/bin\"", - fmt.Sprintf(` npm install %s--prefix "${RUNNER_TEMP}/gh-aw/mcp-cli" %s@"${ENGINE_VERSION}"`, ignoreScriptsFlag, packageName), - fmt.Sprintf(` ln -sf "../node_modules/.bin/%s" "${RUNNER_TEMP}/gh-aw/mcp-cli/bin/%s"`, commandName, commandName), + " mkdir -p \"${RUNNER_TEMP}/gh-aw/engine-cli/bin\"", + fmt.Sprintf(` npm install %s--prefix "${RUNNER_TEMP}/gh-aw/engine-cli" %s@"${ENGINE_VERSION}"`, ignoreScriptsFlag, packageName), + fmt.Sprintf(` ln -sf "../node_modules/.bin/%s" "${RUNNER_TEMP}/gh-aw/engine-cli/bin/%s"`, commandName, commandName), " env:", " ENGINE_VERSION: " + version, } @@ -265,9 +265,9 @@ func GenerateDockerSbxNpmCLIInstallStep(packageName, version, stepName, commandN installStep = GitHubActionStep{ " - name: " + stepName, " run: |", - " mkdir -p \"${RUNNER_TEMP}/gh-aw/mcp-cli/bin\"", - fmt.Sprintf(` npm install %s--prefix "${RUNNER_TEMP}/gh-aw/mcp-cli" %s@%s`, ignoreScriptsFlag, packageName, version), - fmt.Sprintf(` ln -sf "../node_modules/.bin/%s" "${RUNNER_TEMP}/gh-aw/mcp-cli/bin/%s"`, commandName, commandName), + " mkdir -p \"${RUNNER_TEMP}/gh-aw/engine-cli/bin\"", + fmt.Sprintf(` npm install %s--prefix "${RUNNER_TEMP}/gh-aw/engine-cli" %s@%s`, ignoreScriptsFlag, packageName, version), + fmt.Sprintf(` ln -sf "../node_modules/.bin/%s" "${RUNNER_TEMP}/gh-aw/engine-cli/bin/%s"`, commandName, commandName), } if cooldownEnabled { installStep = append(installStep, @@ -279,12 +279,12 @@ func GenerateDockerSbxNpmCLIInstallStep(packageName, version, stepName, commandN } // GetDockerSbxNpmCLIPathSetup returns the PATH export needed for npm CLIs that were -// staged into ${RUNNER_TEMP}/gh-aw/mcp-cli/bin for docker-sbx microVM runs. +// staged into ${RUNNER_TEMP}/gh-aw/engine-cli/bin for docker-sbx microVM runs. func GetDockerSbxNpmCLIPathSetup(workflowData *WorkflowData) string { if !isDockerSbxRuntime(workflowData) { return "" } - return `export PATH="${RUNNER_TEMP}/gh-aw/mcp-cli/bin:$PATH"` + return `export PATH="${RUNNER_TEMP}/gh-aw/engine-cli/bin:$PATH"` } // GenerateNpmInstallStepsWithScope generates npm installation steps with control over global vs local installation.