From 7006fa276b4321bf13804b8338391fbd82d567f4 Mon Sep 17 00:00:00 2001 From: Greg Allen Date: Thu, 11 Jun 2026 13:43:20 -0400 Subject: [PATCH] fix(#2148): thread signal-aware context into sandbox exec ExecStreamReader created its own context.WithTimeout from context.Background(), disconnecting it from the CLI's signal-aware context. When a workflow was cancelled during agent execution, the subprocess continued running and the status comment defer never fired, leaving it stuck at "Started". Wire the parent context through Runtime.Run and into ExecStreamReader so SIGTERM/SIGINT cancellation propagates into the sandbox subprocess. This lets execCmd.Wait() return promptly on cancellation, allowing runAgent's defer to post the "Cancelled" status comment. Changes: - sandbox.ExecStreamReader: accept ctx parameter, use as parent for WithTimeout instead of context.Background() - sandbox.ExecContext: new context-aware variant of Exec - runtime.Runtime.Run: add ctx parameter to interface - runtime.ClaudeRuntime.Run: forward ctx to ExecStreamReader - cli.runAgent: pass signal-aware ctx to rt.Run Closes #2148 Signed-off-by: Claude Opus 4.6 Signed-off-by: Greg Allen --- ...5-06-openshell-native-sandbox-transport.md | 6 +-- internal/cli/run.go | 2 +- internal/runtime/claude.go | 5 +- internal/runtime/claude_test.go | 51 +++++++++++++++++++ internal/runtime/runtime.go | 3 +- internal/sandbox/sandbox.go | 18 +++++-- internal/sandbox/sandbox_test.go | 18 +++++++ 7 files changed, 93 insertions(+), 10 deletions(-) diff --git a/docs/superpowers/plans/2026-05-06-openshell-native-sandbox-transport.md b/docs/superpowers/plans/2026-05-06-openshell-native-sandbox-transport.md index f2d89f0e58..2dd7384d2d 100644 --- a/docs/superpowers/plans/2026-05-06-openshell-native-sandbox-transport.md +++ b/docs/superpowers/plans/2026-05-06-openshell-native-sandbox-transport.md @@ -291,8 +291,8 @@ Replace `SSHStreamReader` (lines 259-284) with: // ExecStreamReader runs a command inside a sandbox, returning an io.ReadCloser for // stdout so the caller can parse structured output. Stderr is forwarded to the // given writer. The caller must read stdout to completion, then call cmd.Wait(). -func ExecStreamReader(sandboxName, command string, timeout time.Duration, stderrW io.Writer) (io.ReadCloser, *exec.Cmd, context.CancelFunc, error) { - ctx, cancel := context.WithTimeout(context.Background(), timeout) +func ExecStreamReader(ctx context.Context, sandboxName, command string, timeout time.Duration, stderrW io.Writer) (io.ReadCloser, *exec.Cmd, context.CancelFunc, error) { + ctx, cancel := context.WithTimeout(ctx, timeout) timeoutSecs := fmt.Sprintf("%d", int(timeout.Seconds())) cmd := exec.CommandContext(ctx, "openshell", "sandbox", "exec", @@ -751,7 +751,7 @@ In `runAgentWithProgress` (line 820): stdout, cmd, cancel, err := sandbox.SSHStreamReader(sshConfigPath, sandboxName, claudeCmd, timeout, os.Stderr) // After: -stdout, cmd, cancel, err := sandbox.ExecStreamReader(sandboxName, claudeCmd, timeout, os.Stderr) +stdout, cmd, cancel, err := sandbox.ExecStreamReader(ctx, sandboxName, claudeCmd, timeout, os.Stderr) ``` Also update the error message on line 839: diff --git a/internal/cli/run.go b/internal/cli/run.go index 6e554919f4..b32ba9f4f6 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -762,7 +762,7 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep go runHeartbeat(printer, agentStart, timeout, heartbeatDone) var metrics agentruntime.RunMetrics - exitCode, runErr := rt.Run(agentruntime.RunParams{ + exitCode, runErr := rt.Run(ctx, agentruntime.RunParams{ SandboxName: sandboxName, AgentBaseName: agentBaseName, Model: h.Model, diff --git a/internal/runtime/claude.go b/internal/runtime/claude.go index 7280f3ae50..ee09e4e407 100644 --- a/internal/runtime/claude.go +++ b/internal/runtime/claude.go @@ -1,6 +1,7 @@ package runtime import ( + "context" "encoding/json" "fmt" "io" @@ -79,9 +80,9 @@ func (r ClaudeRuntime) Bootstrap(input BootstrapInput) error { return installClaudeHooks(sandboxName, hooksInput.ClaudeSandboxHooks()) } -func (ClaudeRuntime) Run(params RunParams, printer *ui.Printer, start time.Time, metrics *RunMetrics) (int, error) { +func (ClaudeRuntime) Run(ctx context.Context, params RunParams, printer *ui.Printer, start time.Time, metrics *RunMetrics) (int, error) { cmd := buildRunCommand(params) - stdout, execCmd, cancel, err := sandbox.ExecStreamReader(params.SandboxName, cmd, params.Timeout, os.Stderr) + stdout, execCmd, cancel, err := sandbox.ExecStreamReader(ctx, params.SandboxName, cmd, params.Timeout, os.Stderr) if err != nil { return -1, err } diff --git a/internal/runtime/claude_test.go b/internal/runtime/claude_test.go index aab71bcd65..06cdce57bc 100644 --- a/internal/runtime/claude_test.go +++ b/internal/runtime/claude_test.go @@ -1,17 +1,21 @@ package runtime import ( + "context" "encoding/json" "fmt" + "io" "os" "path/filepath" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/fullsend-ai/fullsend/internal/sandbox" + "github.com/fullsend-ai/fullsend/internal/ui" ) type bootstrapInput struct { @@ -283,3 +287,50 @@ func TestBuildPluginConfigs_EmptyPluginList(t *testing.T) { enabled := settings["enabledPlugins"].(map[string]any) assert.Len(t, enabled, 0) } + +func TestClaudeRuntime_Run_OpenshellNotInPath(t *testing.T) { + t.Setenv("PATH", "") + + var metrics RunMetrics + printer := ui.New(io.Discard) + + exitCode, err := ClaudeRuntime{}.Run(context.Background(), RunParams{ + SandboxName: "test-sandbox", + AgentBaseName: "test-agent", + RepoDir: "/sandbox/workspace/repo", + Timeout: 10 * time.Second, + }, printer, time.Now(), &metrics) + + assert.Error(t, err) + assert.Equal(t, -1, exitCode) +} + +func TestClaudeRuntime_Bootstrap_OpenshellNotInPath(t *testing.T) { + t.Setenv("PATH", "") + + agentDir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(agentDir, "agent.md"), []byte("test"), 0o644)) + + err := ClaudeRuntime{}.Bootstrap(bootstrapInput{ + sandboxName: "test-sandbox", + agentPath: agentDir, + }) + assert.Error(t, err) + assert.Contains(t, err.Error(), "creating runtime config dirs") +} + +func TestClaudeRuntime_ClearIterationArtifacts_OpenshellNotInPath(t *testing.T) { + t.Setenv("PATH", "") + + err := ClaudeRuntime{}.ClearIterationArtifacts("test-sandbox") + assert.Error(t, err) +} + +func TestClaudeRuntime_ExtractTranscripts_OpenshellNotInPath(t *testing.T) { + t.Setenv("PATH", "") + + outputDir := t.TempDir() + err := ClaudeRuntime{}.ExtractTranscripts("test-sandbox", "test-agent", outputDir) + assert.Error(t, err) + assert.Contains(t, err.Error(), "finding transcripts") +} diff --git a/internal/runtime/runtime.go b/internal/runtime/runtime.go index 5162f17eba..7de1e9b8e0 100644 --- a/internal/runtime/runtime.go +++ b/internal/runtime/runtime.go @@ -1,6 +1,7 @@ package runtime import ( + "context" "sync/atomic" "time" @@ -39,7 +40,7 @@ type Runtime interface { WorkspaceDir() string EnvExports() []string Bootstrap(input BootstrapInput) error - Run(params RunParams, printer *ui.Printer, start time.Time, metrics *RunMetrics) (exitCode int, err error) + Run(ctx context.Context, params RunParams, printer *ui.Printer, start time.Time, metrics *RunMetrics) (exitCode int, err error) ClearIterationArtifacts(sandboxName string) error } diff --git a/internal/sandbox/sandbox.go b/internal/sandbox/sandbox.go index 8800060833..39cdc63113 100644 --- a/internal/sandbox/sandbox.go +++ b/internal/sandbox/sandbox.go @@ -314,8 +314,15 @@ func Delete(name string) error { } // Exec runs a command inside a sandbox and returns stdout, stderr, and exit code. +// It uses context.Background() internally. Use ExecContext for cancellation support. func Exec(sandboxName, command string, timeout time.Duration) (stdout, stderr string, exitCode int, err error) { - ctx, cancel := context.WithTimeout(context.Background(), timeout+10*time.Second) + return ExecContext(context.Background(), sandboxName, command, timeout) +} + +// ExecContext is like Exec but accepts a parent context for cancellation. +// Cancelling the parent (e.g. on SIGTERM) terminates the subprocess. +func ExecContext(ctx context.Context, sandboxName, command string, timeout time.Duration) (stdout, stderr string, exitCode int, err error) { + ctx, cancel := context.WithTimeout(ctx, timeout+10*time.Second) defer cancel() timeoutSecs := fmt.Sprintf("%d", int(timeout.Seconds())) @@ -352,8 +359,13 @@ func Exec(sandboxName, command string, timeout time.Duration) (stdout, stderr st // ExecStreamReader runs a command inside a sandbox, returning an io.ReadCloser for // stdout so the caller can parse structured output. Stderr is forwarded to the // given writer. The caller must read stdout to completion, then call cmd.Wait(). -func ExecStreamReader(sandboxName, command string, timeout time.Duration, stderrW io.Writer) (io.ReadCloser, *exec.Cmd, context.CancelFunc, error) { - ctx, cancel := context.WithTimeout(context.Background(), timeout) +// +// The parent context is used as the base for the timeout context, so +// cancelling the parent (e.g. on SIGTERM) terminates the subprocess. This +// allows CLI-level signal handling to propagate into long-running sandbox +// commands. +func ExecStreamReader(ctx context.Context, sandboxName, command string, timeout time.Duration, stderrW io.Writer) (io.ReadCloser, *exec.Cmd, context.CancelFunc, error) { + ctx, cancel := context.WithTimeout(ctx, timeout) timeoutSecs := fmt.Sprintf("%d", int(timeout.Seconds())) cmd := exec.CommandContext(ctx, "openshell", "sandbox", "exec", diff --git a/internal/sandbox/sandbox_test.go b/internal/sandbox/sandbox_test.go index 17c875dd97..dac4dee8ee 100644 --- a/internal/sandbox/sandbox_test.go +++ b/internal/sandbox/sandbox_test.go @@ -1,6 +1,7 @@ package sandbox import ( + "context" "os" "path/filepath" "strings" @@ -128,6 +129,23 @@ func TestExec_OpenshellNotInPath(t *testing.T) { assert.Error(t, err) } +func TestExecContext_CancelledContext(t *testing.T) { + t.Setenv("PATH", "") + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + _, _, _, err := ExecContext(ctx, "test-sandbox", "echo hello", 10*time.Second) + assert.Error(t, err) +} + +func TestExecStreamReader_OpenshellNotInPath(t *testing.T) { + t.Setenv("PATH", "") + + _, _, _, err := ExecStreamReader(context.Background(), "test-sandbox", "echo hello", 10*time.Second, os.Stderr) + assert.Error(t, err) +} + func TestOsRootContainment(t *testing.T) { dir := t.TempDir()