diff --git a/internal/app/wrapper/app.go b/internal/app/wrapper/app.go index 48c0f02c0..7fad0632a 100644 --- a/internal/app/wrapper/app.go +++ b/internal/app/wrapper/app.go @@ -29,6 +29,7 @@ const ( steerCommandResponseTimeout = 5 * time.Second wrapperChildStopGrace = 2 * time.Second wrapperChildWaitTimeout = 5 * time.Second + wrapperBootstrapTimeout = 15 * time.Second ) type shutdownRequest struct { diff --git a/internal/app/wrapper/app_child_session.go b/internal/app/wrapper/app_child_session.go index f87d599d9..dd1540483 100644 --- a/internal/app/wrapper/app_child_session.go +++ b/internal/app/wrapper/app_child_session.go @@ -2,6 +2,7 @@ package wrapper import ( "context" + "fmt" "io" "os/exec" "sync/atomic" @@ -49,7 +50,9 @@ func (a *App) launchCodexChildSession(ctx context.Context, rawLogger *debuglog.R } a.debugf("child started: binary=%s pid=%d cwd=%s", a.config.CodexRealBinary, cmd.Process.Pid, a.config.WorkspaceRoot) - bootstrappedStdout, err := a.bootstrapHeadlessCodex(childStdin, childStdout, rawLogger, reportProblem) + bootstrappedStdout, err := a.runChildBootstrap(ctx, wrapperBootstrapTimeout, childCancel, func() (io.Reader, error) { + return a.bootstrapHeadlessCodex(childStdin, childStdout, rawLogger, reportProblem) + }) if err != nil { childCancel() _ = cmd.Wait() @@ -73,6 +76,37 @@ func (a *App) launchCodexChildSession(ctx context.Context, rawLogger *debuglog.R }, nil } +// runChildBootstrap bounds the synthetic initialize handshake. A child that +// never answers initialize would otherwise block the launch forever, keeping +// the session process and its whole child tree resident and accumulating under +// repeated launches (issue #910). On timeout or parent cancellation the child +// context is cancelled so the process is killed and its stdout pipe closes, +// which unblocks the bootstrap goroutine and lets it drain into the buffered +// channel instead of leaking. The caller still owns reaping via cmd.Wait(). +func (a *App) runChildBootstrap(ctx context.Context, timeout time.Duration, childCancel context.CancelFunc, boot func() (io.Reader, error)) (io.Reader, error) { + type bootstrapResult struct { + stdout io.Reader + err error + } + done := make(chan bootstrapResult, 1) + go func() { + stdout, err := boot() + done <- bootstrapResult{stdout, err} + }() + timer := time.NewTimer(timeout) + defer timer.Stop() + select { + case res := <-done: + return res.stdout, res.err + case <-ctx.Done(): + childCancel() + return nil, ctx.Err() + case <-timer.C: + childCancel() + return nil, fmt.Errorf("child bootstrap: initialize response not received within %s", timeout) + } +} + func startChildSessionIO(ctx context.Context, session *childSession, parentStdout, parentStderr io.Writer, writeCh chan []byte, runtime backendRuntime, client *relayws.Client, commandResponses *commandResponseTracker, turnTracker *runtimeTurnTracker, activeGeneration *int64, generation int64, errCh chan<- error, debugf func(string, ...any), rawLogger *debuglog.RawLogger, reportProblem func(agentproto.ErrorInfo)) { if session == nil { return diff --git a/internal/app/wrapper/app_child_session_claude.go b/internal/app/wrapper/app_child_session_claude.go index ed395375f..b15a798f4 100644 --- a/internal/app/wrapper/app_child_session_claude.go +++ b/internal/app/wrapper/app_child_session_claude.go @@ -3,6 +3,7 @@ package wrapper import ( "context" "encoding/json" + "io" "os" "strings" @@ -31,7 +32,9 @@ func (a *App) launchClaudeChildSession(ctx context.Context, rawLogger *debuglog. } a.debugf("claude child started: binary=%s pid=%d cwd=%s", claudeBinary, cmd.Process.Pid, a.config.WorkspaceRoot) - bootstrappedStdout, err := a.bootstrapClaude(childStdin, childStdout, rawLogger, reportProblem) + bootstrappedStdout, err := a.runChildBootstrap(ctx, wrapperBootstrapTimeout, childCancel, func() (io.Reader, error) { + return a.bootstrapClaude(childStdin, childStdout, rawLogger, reportProblem) + }) if err != nil { childCancel() _ = cmd.Wait() diff --git a/internal/app/wrapper/app_child_session_opencode.go b/internal/app/wrapper/app_child_session_opencode.go index 84d1151f8..135090887 100644 --- a/internal/app/wrapper/app_child_session_opencode.go +++ b/internal/app/wrapper/app_child_session_opencode.go @@ -46,7 +46,9 @@ func (a *App) launchOpenCodeChildSession(ctx context.Context, rawLogger *debuglo } a.debugf("opencode child started: binary=%s pid=%d cwd=%s", openCodeBinary, cmd.Process.Pid, a.config.WorkspaceRoot) - bootstrappedStdout, err := a.bootstrapOpenCodeACP(translator, childStdin, childStdout, rawLogger, reportProblem) + bootstrappedStdout, err := a.runChildBootstrap(ctx, wrapperBootstrapTimeout, childCancel, func() (io.Reader, error) { + return a.bootstrapOpenCodeACP(translator, childStdin, childStdout, rawLogger, reportProblem) + }) if err != nil { childCancel() _ = cmd.Wait() diff --git a/internal/app/wrapper/app_child_session_test.go b/internal/app/wrapper/app_child_session_test.go index 7706ab521..5c1a1f669 100644 --- a/internal/app/wrapper/app_child_session_test.go +++ b/internal/app/wrapper/app_child_session_test.go @@ -385,3 +385,71 @@ func TestRestartChildSessionStopsCurrentIOBeforeLaunchingReplacement(t *testing. cancel() waitForSessionIOStopped(next, 2*time.Second) } + +func TestRunChildBootstrapReturnsBootstrappedStdout(t *testing.T) { + app := New(Config{Source: "headless", Version: "test"}) + stdout, err := app.runChildBootstrap(context.Background(), wrapperBootstrapTimeout, func() {}, func() (io.Reader, error) { + return strings.NewReader("bootstrapped"), nil + }) + if err != nil { + t.Fatalf("runChildBootstrap: %v", err) + } + data, err := io.ReadAll(stdout) + if err != nil { + t.Fatalf("read stdout: %v", err) + } + if string(data) != "bootstrapped" { + t.Fatalf("expected stdout %q, got %q", "bootstrapped", string(data)) + } +} + +func TestRunChildBootstrapTimesOutWedgedChild(t *testing.T) { + pipeReader, pipeWriter := io.Pipe() + defer pipeReader.Close() + cancelCalled := make(chan struct{}) + childCancel := func() { + // Simulate the kill path: cancelling the child closes its stdout pipe, + // which is what unblocks a wedged bootstrap read. + _ = pipeWriter.Close() + close(cancelCalled) + } + boot := func() (io.Reader, error) { + buf := make([]byte, 1) + if _, err := pipeReader.Read(buf); err != nil { + return nil, err + } + return strings.NewReader("unexpected"), nil + } + + app := New(Config{Source: "headless", Version: "test"}) + _, err := app.runChildBootstrap(context.Background(), 20*time.Millisecond, childCancel, boot) + if err == nil || !strings.Contains(err.Error(), "not received within") { + t.Fatalf("expected bootstrap timeout error, got %v", err) + } + select { + case <-cancelCalled: + case <-time.After(2 * time.Second): + t.Fatal("child cancel was not invoked on bootstrap timeout") + } +} + +func TestRunChildBootstrapHonorsParentCancel(t *testing.T) { + block := make(chan struct{}) + defer close(block) + ctx, cancelParent := context.WithCancel(context.Background()) + cancelParent() + cancelCalled := make(chan struct{}) + app := New(Config{Source: "headless", Version: "test"}) + _, err := app.runChildBootstrap(ctx, wrapperBootstrapTimeout, func() { close(cancelCalled) }, func() (io.Reader, error) { + <-block + return nil, errors.New("bootstrap should not complete") + }) + if !errors.Is(err, context.Canceled) { + t.Fatalf("expected context.Canceled, got %v", err) + } + select { + case <-cancelCalled: + case <-time.After(2 * time.Second): + t.Fatal("child cancel was not invoked on parent cancellation") + } +}