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
7 changes: 7 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,13 @@ runs:
openshell gateway add http://127.0.0.1:8080 --local --name local
openshell gateway select local

- name: Pre-pull sandbox image
shell: bash
run: |
IMAGE="${FULLSEND_SANDBOX_IMAGE:-ghcr.io/fullsend-ai/fullsend-code:latest}"
echo "Pre-pulling sandbox image: ${IMAGE}"
timeout 300 podman pull -- "${IMAGE}" || echo "::warning::Image pre-pull failed; sandbox create will pull on demand"

- name: Install validation dependencies
shell: bash
run: pip install --quiet "jsonschema>=4.18.0"
Expand Down
3 changes: 2 additions & 1 deletion internal/cli/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,8 @@ func runAgent(agentName, fullsendDir, outputBase, targetRepo, fullsendBinary str
createStart := time.Now()
printer.StepStart("Creating sandbox: " + sandboxName)

if err := sandbox.Create(sandboxName, h.Providers, h.Image, h.Policy); err != nil {
readyTimeout := time.Duration(h.SandboxTimeoutSeconds) * time.Second
if err := sandbox.CreateWithRetry(sandboxName, h.Providers, h.Image, h.Policy, sandbox.DefaultMaxCreateAttempts, readyTimeout); err != nil {
printer.StepFail("Failed to create sandbox")
return fmt.Errorf("creating sandbox: %w", err)
}
Expand Down
8 changes: 6 additions & 2 deletions internal/harness/harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,9 @@ type Harness struct {
AgentInput string `yaml:"agent_input,omitempty"`
ValidationLoop *ValidationLoop `yaml:"validation_loop,omitempty"`
RunnerEnv map[string]string `yaml:"runner_env,omitempty"`
TimeoutMinutes int `yaml:"timeout_minutes,omitempty"`
Security *SecurityConfig `yaml:"security,omitempty"`
TimeoutMinutes int `yaml:"timeout_minutes,omitempty"`
SandboxTimeoutSeconds int `yaml:"sandbox_timeout_seconds,omitempty"`
Security *SecurityConfig `yaml:"security,omitempty"`
}

// Load reads a harness YAML file from path, unmarshals it, and validates it.
Expand Down Expand Up @@ -248,6 +249,9 @@ func (h *Harness) Validate() error {
if h.TimeoutMinutes < 0 {
return fmt.Errorf("timeout_minutes must be non-negative, got %d", h.TimeoutMinutes)
}
if h.SandboxTimeoutSeconds != 0 && (h.SandboxTimeoutSeconds < 30 || h.SandboxTimeoutSeconds > 600) {
return fmt.Errorf("sandbox_timeout_seconds must be 0 (default) or between 30 and 600, got %d", h.SandboxTimeoutSeconds)
}
for i, hf := range h.HostFiles {
if hf.Src == "" {
return fmt.Errorf("host_files[%d]: src is required", i)
Expand Down
55 changes: 55 additions & 0 deletions internal/harness/harness_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,61 @@ func TestValidate_NegativeTimeout(t *testing.T) {
assert.Contains(t, err.Error(), "timeout_minutes must be non-negative")
}

func TestValidate_NegativeSandboxTimeout(t *testing.T) {
h := &Harness{Agent: "agents/test.md", SandboxTimeoutSeconds: -1}
err := h.Validate()
require.Error(t, err)
assert.Contains(t, err.Error(), "sandbox_timeout_seconds must be 0 (default) or between 30 and 600")
}

func TestValidate_SandboxTimeoutTooSmall(t *testing.T) {
h := &Harness{Agent: "agents/test.md", SandboxTimeoutSeconds: 10}
err := h.Validate()
require.Error(t, err)
assert.Contains(t, err.Error(), "sandbox_timeout_seconds must be 0 (default) or between 30 and 600")
}

func TestValidate_SandboxTimeoutTooLarge(t *testing.T) {
h := &Harness{Agent: "agents/test.md", SandboxTimeoutSeconds: 601}
err := h.Validate()
require.Error(t, err)
assert.Contains(t, err.Error(), "sandbox_timeout_seconds must be 0 (default) or between 30 and 600")
}

func TestValidate_SandboxTimeoutAtMin(t *testing.T) {
h := &Harness{Agent: "agents/test.md", SandboxTimeoutSeconds: 30}
require.NoError(t, h.Validate())
}

func TestValidate_SandboxTimeoutAtMax(t *testing.T) {
h := &Harness{Agent: "agents/test.md", SandboxTimeoutSeconds: 600}
require.NoError(t, h.Validate())
}

func TestValidate_ZeroSandboxTimeout(t *testing.T) {
h := &Harness{Agent: "agents/test.md", SandboxTimeoutSeconds: 0}
require.NoError(t, h.Validate())
}

func TestValidate_PositiveSandboxTimeout(t *testing.T) {
h := &Harness{Agent: "agents/test.md", SandboxTimeoutSeconds: 180}
require.NoError(t, h.Validate())
}

func TestLoad_SandboxTimeoutField(t *testing.T) {
content := `
agent: agents/test.md
sandbox_timeout_seconds: 180
`
dir := t.TempDir()
path := filepath.Join(dir, "test.yaml")
require.NoError(t, os.WriteFile(path, []byte(content), 0o644))

h, err := Load(path)
require.NoError(t, err)
assert.Equal(t, 180, h.SandboxTimeoutSeconds)
}

func TestLoad_ModelField(t *testing.T) {
content := `
agent: agents/test.md
Expand Down
88 changes: 78 additions & 10 deletions internal/sandbox/sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,15 @@ const (
// SandboxClaudeConfig is the Claude config directory inside the sandbox.
SandboxClaudeConfig = "/tmp/claude-config" //nolint:gosec // not a credential

createTimeout = 65 * time.Second
readyTimeout = 60 * time.Second
readyTimeout = 120 * time.Second
readyPoll = 2 * time.Second
readyCtxBuffer = 10 * time.Second
maxReadyTimeout = 600 * time.Second
transferTimeout = 5 * time.Minute

DefaultMaxCreateAttempts = 3
retryInitialBackoff = 5 * time.Second
retryMaxBackoff = 15 * time.Second
)

func sanitizeDownload(localDir string) error {
Expand Down Expand Up @@ -149,12 +154,75 @@ func CheckGateway() error {
return nil
}

// effectiveReadyTimeout returns the sandbox ready timeout to use. Priority:
// explicit override (from harness config) > FULLSEND_SANDBOX_READY_TIMEOUT
// env var > package default.
func effectiveReadyTimeout(override time.Duration) time.Duration {
t := readyTimeout
if override > 0 {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] correctness

The backward-compatible Create() wrapper now silently retries 3 times via CreateWithRetry, changing observable behavior (longer wall-clock time on failure, sandbox deletion side effects between attempts) for any future caller who expects single-attempt semantics.

Suggested fix: Update the Create() godoc to explicitly note that it retries up to DefaultMaxCreateAttempts times with exponential backoff.

t = override
} else if envVal := os.Getenv("FULLSEND_SANDBOX_READY_TIMEOUT"); envVal != "" {
if d, err := time.ParseDuration(envVal); err == nil && d > 0 {
t = d
}
}
if t > maxReadyTimeout {
t = maxReadyTimeout
}
return t
}

// Create creates a persistent OpenShell sandbox and waits for it to be ready.
// If providers are given, they are passed as --provider flags. If image is
// non-empty, it is passed as --from to start the sandbox from a container image.
// If policy is non-empty, it is applied at creation time via --policy.
// It retries up to DefaultMaxCreateAttempts times with exponential backoff,
// deleting the failed sandbox between attempts.
func Create(name string, providers []string, image, policy string) error {
ctx, cancel := context.WithTimeout(context.Background(), createTimeout)
return CreateWithRetry(name, providers, image, policy, DefaultMaxCreateAttempts, 0)
}

// CreateWithRetry creates a sandbox, retrying up to maxAttempts times with
// exponential backoff on failure. Between attempts the failed sandbox is
// deleted to avoid name conflicts. If readyTimeoutOverride is positive, it
// overrides the default ready timeout.
func CreateWithRetry(name string, providers []string, image, policy string, maxAttempts int, readyTimeoutOverride time.Duration) error {
if maxAttempts < 1 {
return fmt.Errorf("maxAttempts must be >= 1, got %d", maxAttempts)
}

timeout := effectiveReadyTimeout(readyTimeoutOverride)

var lastErr error
for attempt := 1; attempt <= maxAttempts; attempt++ {
lastErr = createOnce(name, providers, image, policy, timeout)
if lastErr == nil {
return nil
}

if delErr := Delete(name); delErr != nil {
fmt.Fprintf(os.Stderr, " Warning: cleanup of sandbox %s failed: %v\n", name, delErr)
}

if attempt < maxAttempts {
shift := uint(attempt - 1)
if shift > 30 {
shift = 30
}
backoff := retryInitialBackoff * time.Duration(1<<shift)
if backoff > retryMaxBackoff {
backoff = retryMaxBackoff
}
fmt.Fprintf(os.Stderr, " Sandbox creation attempt %d/%d failed (%v), retrying in %s...\n", attempt, maxAttempts, lastErr, backoff)
time.Sleep(backoff)
}
}
return fmt.Errorf("sandbox creation failed after %d attempts: %w", maxAttempts, lastErr)
}

// createOnce creates a persistent OpenShell sandbox and waits for it to be
// ready. If providers are given, they are passed as --provider flags. If image
// is non-empty, it is passed as --from to start the sandbox from a container
// image. If policy is non-empty, it is applied at creation time via --policy.
func createOnce(name string, providers []string, image, policy string, timeout time.Duration) error {
ctx, cancel := context.WithTimeout(context.Background(), timeout+readyCtxBuffer)
defer cancel()

args := []string{
Expand Down Expand Up @@ -182,17 +250,17 @@ func Create(name string, providers []string, image, policy string) error {
out, err := cmd.CombinedOutput()

if err != nil {
check := exec.Command("openshell", "sandbox", "get", name)
check := exec.CommandContext(ctx, "openshell", "sandbox", "get", name)
if checkErr := check.Run(); checkErr != nil {
return fmt.Errorf("sandbox create failed: %s", string(out))
}
}

// Wait for sandbox to be fully ready (image pull can take a while).
deadline := time.Now().Add(readyTimeout)
deadline := time.Now().Add(timeout)
var lastOutput, lastStderr string
for time.Now().Before(deadline) {
check := exec.Command("openshell", "sandbox", "get", name)
check := exec.CommandContext(ctx, "openshell", "sandbox", "get", name)
var stdoutBuf, stderrBuf strings.Builder
check.Stdout = &stdoutBuf
check.Stderr = &stderrBuf
Expand All @@ -212,7 +280,7 @@ func Create(name string, providers []string, image, policy string) error {
containerLogs := collectPodmanLogs(name)

return fmt.Errorf("sandbox %q not ready after %s\nstdout: %s\nstderr: %s\nsupervisor logs: %s\ngateway logs: %s\ncontainer logs: %s",
name, readyTimeout, lastOutput, lastStderr, supervisorLogs, gatewayLogs, containerLogs)
name, timeout, lastOutput, lastStderr, supervisorLogs, gatewayLogs, containerLogs)
}

// Delete deletes a sandbox, returning any error for the caller to log.
Expand Down
66 changes: 66 additions & 0 deletions internal/sandbox/sandbox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,72 @@ func TestSanitizeDownload_EmptyDir(t *testing.T) {
assert.NoError(t, err)
}

func TestEffectiveReadyTimeout_Default(t *testing.T) {
t.Setenv("FULLSEND_SANDBOX_READY_TIMEOUT", "")
got := effectiveReadyTimeout(0)
assert.Equal(t, readyTimeout, got)
}

func TestEffectiveReadyTimeout_Override(t *testing.T) {
got := effectiveReadyTimeout(90 * time.Second)
assert.Equal(t, 90*time.Second, got)
}

func TestEffectiveReadyTimeout_EnvVar(t *testing.T) {
t.Setenv("FULLSEND_SANDBOX_READY_TIMEOUT", "180s")
got := effectiveReadyTimeout(0)
assert.Equal(t, 180*time.Second, got)
}

func TestEffectiveReadyTimeout_OverrideTakesPrecedenceOverEnv(t *testing.T) {
t.Setenv("FULLSEND_SANDBOX_READY_TIMEOUT", "180s")
got := effectiveReadyTimeout(90 * time.Second)
assert.Equal(t, 90*time.Second, got)
}

func TestEffectiveReadyTimeout_InvalidEnvVar(t *testing.T) {
t.Setenv("FULLSEND_SANDBOX_READY_TIMEOUT", "not-a-duration")
got := effectiveReadyTimeout(0)
assert.Equal(t, readyTimeout, got)
}

func TestEffectiveReadyTimeout_NegativeEnvVar(t *testing.T) {
t.Setenv("FULLSEND_SANDBOX_READY_TIMEOUT", "-30s")
got := effectiveReadyTimeout(0)
assert.Equal(t, readyTimeout, got)
}

func TestCreateWithRetry_OpenshellNotInPath(t *testing.T) {
t.Setenv("PATH", "")

err := CreateWithRetry("test-sandbox", nil, "", "", 1, 0)
assert.Error(t, err)
assert.Contains(t, err.Error(), "sandbox creation failed after 1 attempts")
}

func TestCreateWithRetry_ZeroAttempts(t *testing.T) {
err := CreateWithRetry("test-sandbox", nil, "", "", 0, 0)
assert.Error(t, err)
assert.Contains(t, err.Error(), "maxAttempts must be >= 1")
}

func TestCreateWithRetry_NegativeAttempts(t *testing.T) {
err := CreateWithRetry("test-sandbox", nil, "", "", -1, 0)
assert.Error(t, err)
assert.Contains(t, err.Error(), "maxAttempts must be >= 1")
}

func TestEffectiveReadyTimeout_CappedAtMax(t *testing.T) {
got := effectiveReadyTimeout(999 * time.Second)
assert.Equal(t, maxReadyTimeout, got)
}

func TestEffectiveReadyTimeout_EnvVarCappedAtMax(t *testing.T) {
t.Setenv("FULLSEND_SANDBOX_READY_TIMEOUT", "1h")
got := effectiveReadyTimeout(0)
assert.Equal(t, maxReadyTimeout, got)
}

func TestUploadDir_OpenshellNotInPath(t *testing.T) {
dir := t.TempDir()
t.Setenv("PATH", "")
Expand Down
Loading