Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion pkg/workflow/awf_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,10 @@ import (
"encoding/json"
"fmt"
"slices"
"strconv"
"strings"
"sync"
"time"

"github.com/santhosh-tekuri/jsonschema/v6"

Expand Down Expand Up @@ -295,6 +297,10 @@ type AWFContainerConfig struct {
// Maps to: --image-tag <value>
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.
Expand Down Expand Up @@ -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) {
Expand All @@ -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.
Expand All @@ -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 ──────────────────────────────────────────────────────
Expand Down Expand Up @@ -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)
Comment on lines +650 to +656
}
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
Expand Down
48 changes: 48 additions & 0 deletions pkg/workflow/awf_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,61 @@ import (
"fmt"
"strings"
"testing"
"time"

"github.com/github/gh-aw/pkg/constants"
"github.com/github/gh-aw/pkg/workflow/compilerenv"
"github.com/stretchr/testify/assert"
"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) {
Expand Down
5 changes: 3 additions & 2 deletions pkg/workflow/awf_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down
15 changes: 14 additions & 1 deletion pkg/workflow/claude_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}
Expand Down
19 changes: 18 additions & 1 deletion pkg/workflow/codex_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
Expand Down
106 changes: 105 additions & 1 deletion pkg/workflow/docker_sbx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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},
},
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading