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
1 change: 1 addition & 0 deletions internal/app/wrapper/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const (
steerCommandResponseTimeout = 5 * time.Second
wrapperChildStopGrace = 2 * time.Second
wrapperChildWaitTimeout = 5 * time.Second
wrapperBootstrapTimeout = 15 * time.Second
)

type shutdownRequest struct {
Expand Down
36 changes: 35 additions & 1 deletion internal/app/wrapper/app_child_session.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package wrapper

import (
"context"
"fmt"
"io"
"os/exec"
"sync/atomic"
Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand Down
5 changes: 4 additions & 1 deletion internal/app/wrapper/app_child_session_claude.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package wrapper
import (
"context"
"encoding/json"
"io"
"os"
"strings"

Expand Down Expand Up @@ -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()
Expand Down
4 changes: 3 additions & 1 deletion internal/app/wrapper/app_child_session_opencode.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
68 changes: 68 additions & 0 deletions internal/app/wrapper/app_child_session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}