fix(shell): bound command execution with timeout + context cancellation

shell ran cmd.Run() on a plain exec.Command with no timeout and no context,
so a stuck command (network read that never returns, interactive prompt,
infinite loop) wedged the agent forever — and Ctrl-C could not recover
because the loop's drain blocks on the tool goroutine. The sibling
parallel_shell already had a timeout; plain shell, the most-used tool, did
not.

- SetContext ties execution to the agent context (the loop already calls it
  on context-aware tools), so Ctrl-C / turn timeout kills the command now.
- exec.CommandContext + a generous 30m backstop timeout bounds genuinely
  stuck commands for unattended runs (serve, telegram) with no human to
  interrupt. WaitDelay guarantees Run() returns even if the killed process
  leaves children holding the pipes.
- Cancellation/timeout surface as clear errors, not opaque "signal: killed".

Tests: a sleeping command now returns promptly via both the timeout and
context-cancellation paths.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: jkyberneees

cmd/odek/shell.go

@@ -2,15 +2,25 @@ package main
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"fmt"
 	"os/exec"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/BackendStack21/odek/internal/danger"
 )
 
+// defaultShellTimeout bounds a single shell command. It is deliberately
+// generous — the goal is to stop a genuinely stuck command (a network read
+// that never returns, an interactive prompt, an infinite loop) from wedging
+// the agent forever, NOT to kill legitimate long builds or test suites. When
+// the agent context is cancelled (Ctrl-C, turn timeout) the command is killed
+// immediately regardless of this backstop.
+const defaultShellTimeout = 30 * time.Minute
+
 // shellTool is odek's built-in tool that lets the agent run shell commands.
 //
 // This is the only built-in tool — it's enough for reading files, running
@@ -61,8 +71,20 @@ type shellTool struct {
 	// ttyPath is the path to the terminal device for approval prompts.
 	// Overridden in tests to mock user input. Only used when approver is nil.
 	ttyPath string
+
+	// ctx, when set via SetContext, ties command execution to the agent's
+	// lifetime: cancelling it (Ctrl-C, turn timeout) kills the running command.
+	ctx context.Context
+
+	// timeout bounds a single command. Zero falls back to defaultShellTimeout.
+	timeout time.Duration
 }
 
+// SetContext lets the agent loop propagate its context so a running command is
+// killed when the turn is cancelled. Implements the loop's optional
+// context-aware tool interface.
+func (t *shellTool) SetContext(ctx context.Context) { t.ctx = ctx }
+
 func (t *shellTool) Name() string { return "shell" }
 
 func (t *shellTool) Description() string {
@@ -113,13 +135,40 @@ func (t *shellTool) Call(args string) (string, error) {
 		return "", err
 	}
 
-	cmd := t.buildCmd(input.Command)
+	// Bound execution: cancel with the agent context (Ctrl-C / turn timeout)
+	// and a generous backstop timeout so a stuck command can never wedge the
+	// agent forever.
+	base := t.ctx
+	if base == nil {
+		base = context.Background()
+	}
+	timeout := t.timeout
+	if timeout <= 0 {
+		timeout = defaultShellTimeout
+	}
+	ctx, cancel := context.WithTimeout(base, timeout)
+	defer cancel()
+
+	cmd := t.buildCmd(ctx, input.Command)
+	// WaitDelay guarantees Run() returns even if the killed process leaves
+	// children holding the output pipes open after the context fires.
+	cmd.WaitDelay = 5 * time.Second
 
 	var outBuf, errBuf bytes.Buffer
 	cmd.Stdout = &outBuf
 	cmd.Stderr = &errBuf
 
 	err := cmd.Run()
+
+	// Surface cancellation/timeout as a clear, actionable error rather than an
+	// opaque "signal: killed".
+	if ctxErr := ctx.Err(); ctxErr != nil {
+		if ctxErr == context.DeadlineExceeded {
+			return "", fmt.Errorf("shell: command timed out after %s (still running? it was killed): %s", timeout, input.Command)
+		}
+		return "", fmt.Errorf("shell: command cancelled: %s", input.Command)
+	}
+
 	output := strings.TrimSpace(outBuf.String())
 	stderrStr := strings.TrimSpace(errBuf.String())
 	if stderrStr != "" {
@@ -200,9 +249,9 @@ func (t *shellTool) promptUser(cmd, description string) error {
 //
 // When running on the host (default), the command executes via "sh -c"
 // in odek's current working directory.
-func (t *shellTool) buildCmd(command string) *exec.Cmd {
+func (t *shellTool) buildCmd(ctx context.Context, command string) *exec.Cmd {
 	if t.containerName != "" {
-		return exec.Command("docker", "exec", "-w", "/workspace", t.containerName, "sh", "-c", command)
+		return exec.CommandContext(ctx, "docker", "exec", "-w", "/workspace", t.containerName, "sh", "-c", command)
 	}
-	return exec.Command("sh", "-c", command)
+	return exec.CommandContext(ctx, "sh", "-c", command)
 }

cmd/odek/shell_test.go

@@ -1,11 +1,13 @@
 package main
 
 import (
+	"context"
 	"encoding/json"
 	"os"
 	"os/exec"
 	"strings"
 	"testing"
+	"time"
 )
 
 func TestShellTool_Name(t *testing.T) {
@@ -15,6 +17,60 @@ func TestShellTool_Name(t *testing.T) {
 	}
 }
 
+// TestShellTool_Timeout verifies a stuck command can no longer wedge the agent:
+// a tiny per-tool timeout kills the command and Call returns promptly with a
+// clear timeout error instead of blocking forever.
+func TestShellTool_Timeout(t *testing.T) {
+	st := &shellTool{timeout: 200 * time.Millisecond}
+	done := make(chan struct{})
+	var out string
+	var err error
+	go func() {
+		out, err = st.Call(`{"command":"sleep 30"}`)
+		close(done)
+	}()
+	select {
+	case <-done:
+	case <-time.After(5 * time.Second):
+		t.Fatal("Call did not return after the command timeout — agent would hang")
+	}
+	if err == nil {
+		t.Fatalf("expected a timeout error, got output %q", out)
+	}
+	if !strings.Contains(err.Error(), "timed out") {
+		t.Errorf("error should mention the timeout, got: %v", err)
+	}
+}
+
+// TestShellTool_ContextCancellation verifies Ctrl-C / turn cancellation kills a
+// running command immediately via the agent context.
+func TestShellTool_ContextCancellation(t *testing.T) {
+	st := &shellTool{}
+	ctx, cancel := context.WithCancel(context.Background())
+	st.SetContext(ctx)
+
+	done := make(chan struct{})
+	var err error
+	go func() {
+		_, err = st.Call(`{"command":"sleep 30"}`)
+		close(done)
+	}()
+	time.Sleep(100 * time.Millisecond)
+	cancel()
+
+	select {
+	case <-done:
+	case <-time.After(5 * time.Second):
+		t.Fatal("Call did not return after context cancellation — Ctrl-C would not work")
+	}
+	if err == nil {
+		t.Fatal("expected a cancellation error")
+	}
+	if !strings.Contains(err.Error(), "cancelled") {
+		t.Errorf("error should mention cancellation, got: %v", err)
+	}
+}
+
 func TestShellTool_Description(t *testing.T) {
 	st := &shellTool{}
 	desc := st.Description()
@@ -130,7 +186,7 @@ func TestShellTool_Call_StdoutAndStderr(t *testing.T) {
 
 func TestShellTool_BuildCmd_Local(t *testing.T) {
 	st := &shellTool{}
-	cmd := st.buildCmd("echo test")
+	cmd := st.buildCmd(context.Background(), "echo test")
 	args := cmd.Args
 	if args[0] != "sh" || args[1] != "-c" || args[2] != "echo test" {
 		t.Errorf("local cmd args = %v, want [sh -c 'echo test']", args)
@@ -139,7 +195,7 @@ func TestShellTool_BuildCmd_Local(t *testing.T) {
 
 func TestShellTool_BuildCmd_Docker(t *testing.T) {
 	st := &shellTool{containerName: "odek-12345"}
-	cmd := st.buildCmd("echo test")
+	cmd := st.buildCmd(context.Background(), "echo test")
 	args := cmd.Args
 	expected := []string{"docker", "exec", "-w", "/workspace", "odek-12345", "sh", "-c", "echo test"}
 	if !stringSlicesEqual(args, expected) {
@@ -303,7 +359,7 @@ func TestShellTool_CheckApproval(t *testing.T) {
 
 func TestShellTool_BuildCmd_Default(t *testing.T) {
 	st := &shellTool{}
-	cmd := st.buildCmd("echo hello")
+	cmd := st.buildCmd(context.Background(), "echo hello")
 	if cmd.Args[0] != "sh" {
 		t.Errorf("expected sh, got %s", cmd.Args[0])
 	}