diff --git a/pkg/workflow/awf_config.go b/pkg/workflow/awf_config.go index dec3cb29a6e..af16bb5d2e6 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,30 @@ func BuildAWFConfigJSON(config AWFCommandConfig) (string, error) { return jsonStr, nil } +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 + } + + 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 (e.g. a GitHub Actions expression) cannot be emitted in integer-only 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_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/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..2541d99c5fa 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,7 +344,10 @@ 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) - // Add MCP CLI bin directory to PATH when cli-proxy is enabled + 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 != "" { claudeCommandWithPath = fmt.Sprintf("%s && %s", mcpCLIPath, claudeCommandWithPath) } diff --git a/pkg/workflow/codex_engine.go b/pkg/workflow/codex_engine.go index be962046f2c..e43d78c12da 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,7 +360,10 @@ 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) } - // Add MCP CLI bin directory to PATH when cli-proxy is enabled + 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 != "" { 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..183e9a4c7a2 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,86 @@ 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/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) { + 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/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/engine-cli/bin:$PATH"`) + }) +} + +// 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 { + lines = append(lines, step...) + } + return lines } // TestDockerSbxNetworkIsolationAlwaysTrue verifies that isAWFNetworkIsolationEnabled diff --git a/pkg/workflow/nodejs.go b/pkg/workflow/nodejs.go index ae6a504ac30..d3b3908f126 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 from +// ${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 { + ignoreScriptsFlag = "" + } + + var installStep GitHubActionStep + if ExpressionPattern.MatchString(version) { + installStep = GitHubActionStep{ + " - name: " + stepName, + " run: |", + " 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, + } + 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/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, + " 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/engine-cli/bin for docker-sbx microVM runs. +func GetDockerSbxNpmCLIPathSetup(workflowData *WorkflowData) string { + if !isDockerSbxRuntime(workflowData) { + return "" + } + return `export PATH="${RUNNER_TEMP}/gh-aw/engine-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.