Merge pull request #15 from bkildow/fix/hook-resilience

Harden Claude Code hook handlers against hangs and edge cases

Author: bkildow

cmd/claude.go

@@ -1,11 +1,14 @@
 package cmd
 
 import (
+	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"os"
 	"path/filepath"
+	"time"
 
 	"github.com/bkildow/wt-cli/internal/claude"
 	"github.com/bkildow/wt-cli/internal/config"
@@ -15,6 +18,14 @@ import (
 	"github.com/spf13/cobra"
 )
 
+// Timeouts for hook operations. Hooks run inside Claude Code's sandbox,
+// so they must complete promptly or risk blocking the agent session.
+const (
+	hookCreateTimeout  = 60 * time.Second
+	hookRemoveTimeout  = 30 * time.Second
+	hookPayloadTimeout = 5 * time.Second
+)
+
 // hookPayload is the JSON structure Claude Code sends on stdin for hook events.
 // See https://code.claude.com/docs/en/hooks for the payload schema.
 type hookPayload struct {
@@ -118,7 +129,8 @@ func runClaudeInit(cmd *cobra.Command, _ []string) error {
 }
 
 func runClaudeHookWorktreeCreate(cmd *cobra.Command, _ []string) error {
-	ctx := cmd.Context()
+	ctx, cancel := context.WithTimeout(cmd.Context(), hookCreateTimeout)
+	defer cancel()
 
 	hctx, err := loadHookContext()
 	if err != nil {
@@ -128,6 +140,7 @@ func runClaudeHookWorktreeCreate(cmd *cobra.Command, _ []string) error {
 	projectRoot, cfg := hctx.projectRoot, hctx.cfg
 	gitDir := project.GitDirPath(projectRoot, cfg)
 	runner := git.NewRunner(gitDir, false)
+	runner.BatchMode = true
 
 	// Ensure git excludes are configured (non-fatal if sandbox blocks it).
 	if err := project.EnsureGitExclude(gitDir, false); err != nil {
@@ -141,11 +154,19 @@ func runClaudeHookWorktreeCreate(cmd *cobra.Command, _ []string) error {
 	branch := hctx.payload.Name
 	worktreePath := filepath.Join(project.WorktreesPath(projectRoot, cfg), branch)
 
-	// If the worktree already exists, just return its path.
-	if _, err := os.Stat(worktreePath); err == nil {
+	// If the worktree already exists and is valid, just return its path.
+	gitMarker := filepath.Join(worktreePath, ".git")
+	if _, err := os.Stat(gitMarker); err == nil {
 		fmt.Println(worktreePath)
 		return nil
 	}
+	// Directory exists but is not a valid worktree — clean up leftover from failed create.
+	if _, err := os.Stat(worktreePath); err == nil {
+		ui.Warning("Directory exists but is not a valid worktree, recreating: " + worktreePath)
+		if err := os.RemoveAll(worktreePath); err != nil {
+			return fmt.Errorf("failed to clean up invalid worktree directory: %w", err)
+		}
+	}
 
 	hasRemote, err := runner.HasRemoteBranch(ctx, branch)
 	if err != nil {
@@ -192,7 +213,8 @@ func runClaudeHookWorktreeCreate(cmd *cobra.Command, _ []string) error {
 }
 
 func runClaudeHookWorktreeRemove(cmd *cobra.Command, _ []string) error {
-	ctx := cmd.Context()
+	ctx, cancel := context.WithTimeout(cmd.Context(), hookRemoveTimeout)
+	defer cancel()
 
 	hctx, err := loadHookContext()
 	if err != nil {
@@ -222,11 +244,12 @@ func runClaudeHookWorktreeRemove(cmd *cobra.Command, _ []string) error {
 
 	gitDir := project.GitDirPath(projectRoot, cfg)
 	runner := git.NewRunner(gitDir, false)
+	runner.BatchMode = true
 
 	// Force remove — Claude agents may have uncommitted changes.
 	ui.Step("Removing worktree: " + branch)
 	if err := runner.WorktreeRemove(ctx, worktreePath, true); err != nil {
-		ui.Warning("Worktree remove failed: " + err.Error())
+		return fmt.Errorf("worktree remove failed: %w", err)
 	}
 
 	if err := runner.BranchDelete(ctx, branch, false); err != nil {
@@ -252,6 +275,12 @@ func loadHookContext() (*hookContext, error) {
 		return nil, err
 	}
 
+	// Replace process stdin with /dev/null so downstream operations
+	// (teardown hooks, git commands) cannot read from Claude Code's pipe.
+	if devNull, err := os.Open(os.DevNull); err == nil {
+		os.Stdin = devNull
+	}
+
 	// Validate required fields based on event type.
 	switch payload.HookEventName {
 	case claude.HookWorktreeCreate:
@@ -276,17 +305,31 @@ func loadHookContext() (*hookContext, error) {
 }
 
 // readHookPayload parses the JSON hook payload from the given reader.
-// Uses json.NewDecoder so it returns as soon as one JSON object is read,
-// without waiting for EOF (which may never arrive from Claude Code's pipe).
+// Claude Code's pipe may never send EOF, so a timeout prevents indefinite blocking.
 func readHookPayload(r io.Reader) (hookPayload, error) {
-	var payload hookPayload
-	if err := json.NewDecoder(r).Decode(&payload); err != nil {
-		if err == io.EOF {
-			return hookPayload{}, fmt.Errorf("no payload received on stdin")
+	type result struct {
+		payload hookPayload
+		err     error
+	}
+	ch := make(chan result, 1)
+	go func() {
+		var p hookPayload
+		err := json.NewDecoder(r).Decode(&p)
+		ch <- result{p, err}
+	}()
+
+	select {
+	case res := <-ch:
+		if res.err != nil {
+			if errors.Is(res.err, io.EOF) {
+				return hookPayload{}, fmt.Errorf("no payload received on stdin")
+			}
+			return hookPayload{}, fmt.Errorf("invalid JSON payload: %w", res.err)
 		}
-		return hookPayload{}, fmt.Errorf("invalid JSON payload: %w", err)
+		return res.payload, nil
+	case <-time.After(hookPayloadTimeout):
+		return hookPayload{}, fmt.Errorf("timed out waiting for hook payload on stdin (no data received within 5s)")
 	}
-	return payload, nil
 }
 
 // resolveProjectRoot determines the project root from the hook payload.

cmd/claude_test.go

@@ -86,6 +86,21 @@ func TestReadHookPayload_noEOF(t *testing.T) {
 	_ = pw.Close()
 }
 
+func TestReadHookPayload_timeout(t *testing.T) {
+	// Simulate a pipe that is open but never sends any data.
+	// readHookPayload should time out instead of blocking forever.
+	pr, pw := io.Pipe()
+	defer func() { _ = pw.Close() }()
+
+	_, err := readHookPayload(pr)
+	if err == nil {
+		t.Fatal("expected timeout error")
+	}
+	if !strings.Contains(err.Error(), "timed out") {
+		t.Errorf("error = %q, want timeout message", err.Error())
+	}
+}
+
 func TestResolveProjectRoot_noProject(t *testing.T) {
 	payload := hookPayload{
 		Cwd: "/nonexistent/path",

cmd/run_setup.go

@@ -4,6 +4,8 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"os/signal"
+	"syscall"
 	"time"
 
 	lipgloss "charm.land/lipgloss/v2"
@@ -26,16 +28,18 @@ func newRunSetupCmd() *cobra.Command {
 	return cmd
 }
 
-func runRunSetup(cmd *cobra.Command, args []string) error {
-	ctx := cmd.Context()
-
+func runRunSetup(cmd *cobra.Command, _ []string) error {
 	worktreePath, _ := cmd.Flags().GetString("worktree-path")
 	projectRoot, _ := cmd.Flags().GetString("project-root")
 
 	if worktreePath == "" || projectRoot == "" {
 		return fmt.Errorf("--worktree-path and --project-root are required")
 	}
 
+	// Cancel context on SIGTERM/SIGINT so running hooks are stopped.
+	ctx, stop := signal.NotifyContext(cmd.Context(), syscall.SIGTERM, syscall.SIGINT)
+	defer stop()
+
 	cfg, err := config.Load(projectRoot)
 	if err != nil {
 		return err
@@ -66,6 +70,16 @@ func runRunSetup(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
+	// Mark setup as failed if we exit while still "running" (e.g., signal, panic).
+	defer func() {
+		if state.Status == project.SetupRunning {
+			state.Status = project.SetupFailed
+			state.Error = "setup process terminated unexpectedly"
+			state.CompletedAt = time.Now()
+			_ = project.WriteSetupState(worktreePath, state)
+		}
+	}()
+
 	var setupErr error
 
 	// Run serial hooks with progress tracking.

internal/git/git.go

@@ -6,6 +6,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"os"
 	"os/exec"
 	"strconv"
 	"strings"
@@ -45,14 +46,23 @@ type Git interface {
 }
 
 type Runner struct {
-	GitDir string
-	DryRun bool
+	GitDir    string
+	DryRun    bool
+	BatchMode bool // Suppress interactive prompts (for non-TTY environments like hooks)
 }
 
 func NewRunner(gitDir string, dryRun bool) *Runner {
 	return &Runner{GitDir: gitDir, DryRun: dryRun}
 }
 
+// batchEnv returns environment variables that suppress interactive git prompts.
+func batchEnv() []string {
+	env := os.Environ()
+	env = append(env, "GIT_TERMINAL_PROMPT=0")
+	env = append(env, "GIT_SSH_COMMAND=ssh -o BatchMode=yes")
+	return env
+}
+
 func (r *Runner) Run(ctx context.Context, args ...string) (string, error) {
 	fullArgs := append([]string{"--git-dir", r.GitDir}, args...)
 	cmdStr := "git " + strings.Join(fullArgs, " ")
@@ -67,6 +77,9 @@ func (r *Runner) Run(ctx context.Context, args ...string) (string, error) {
 	var stdout, stderr bytes.Buffer
 	cmd.Stdout = &stdout
 	cmd.Stderr = &stderr
+	if r.BatchMode {
+		cmd.Env = batchEnv()
+	}
 
 	if err := cmd.Run(); err != nil {
 		return "", fmt.Errorf("%s: %w\n%s", cmdStr, err, stderr.String())

internal/git/git_test.go

@@ -11,6 +11,40 @@ import (
 	"github.com/bkildow/wt-cli/internal/ui"
 )
 
+func TestBatchEnv(t *testing.T) {
+	env := batchEnv()
+
+	var hasTerminalPrompt, hasSSH bool
+	for _, v := range env {
+		if v == "GIT_TERMINAL_PROMPT=0" {
+			hasTerminalPrompt = true
+		}
+		if strings.HasPrefix(v, "GIT_SSH_COMMAND=") && strings.Contains(v, "BatchMode=yes") {
+			hasSSH = true
+		}
+	}
+	if !hasTerminalPrompt {
+		t.Error("batchEnv missing GIT_TERMINAL_PROMPT=0")
+	}
+	if !hasSSH {
+		t.Error("batchEnv missing GIT_SSH_COMMAND with BatchMode=yes")
+	}
+}
+
+func TestRunnerBatchMode(t *testing.T) {
+	// BatchMode defaults to false.
+	r := NewRunner("/tmp/fake", false)
+	if r.BatchMode {
+		t.Error("BatchMode should default to false")
+	}
+
+	// Setting BatchMode should not affect DryRun.
+	r.BatchMode = true
+	if r.DryRun {
+		t.Error("setting BatchMode should not affect DryRun")
+	}
+}
+
 func TestParseRemoteBranches(t *testing.T) {
 	tests := []struct {
 		name   string