diff --git a/apps/daemon/internal/agent/codex/rpc.go b/apps/daemon/internal/agent/codex/rpc.go index f3a99cf41..e94d73921 100644 --- a/apps/daemon/internal/agent/codex/rpc.go +++ b/apps/daemon/internal/agent/codex/rpc.go @@ -18,16 +18,10 @@ import ( obslog "github.com/MiniMax-AI/OpenAgentCore/internal/obs/log" ) -// rpcDefaultRequestTimeout caps a single JSON-RPC request waiting for a -// response. 60s is the same default mini-captain uses; it must be long -// enough for `thread/start` (which can spin up a fresh model context), -// short enough that a dead app-server doesn't pin the prompt forever. +// rpcDefaultRequestTimeout caps the response wait for a JSON-RPC request, +// including initialize and thread/start, after its write completes. const rpcDefaultRequestTimeout = 60 * time.Second -// rpcInitTimeout caps the initial JSON-RPC `initialize` handshake. -// Shorter than per-request so misconfigured environments fail fast. -const rpcInitTimeout = 10 * time.Second - // rpcKillTimeout bounds child teardown waits. const rpcKillTimeout = 3 * time.Second @@ -168,7 +162,7 @@ func NewJSONRPCClient(cfg JSONRPCConfig) *JSONRPCClient { // Three failure paths: // // - exec.LookPath / Start failure → returns the spawn error verbatim -// - initialize timeout → kills the child, returns context.DeadlineExceeded +// - initialize timeout or cancellation → kills the child, returns the error // - JSON-RPC error on initialize → kills the child, returns the error func (c *JSONRPCClient) Start(ctx context.Context, init InitializeParams) (InitializeResult, error) { args := append([]string{}, c.cfg.ExtraArgs...) @@ -202,9 +196,7 @@ func (c *JSONRPCClient) Start(ctx context.Context, init InitializeParams) (Initi go func() { defer c.readers.Done(); c.pumpStderr() }() go c.waitChild() - initCtx, cancel := context.WithTimeout(ctx, rpcInitTimeout) - defer cancel() - rawResult, err := c.Request(initCtx, "initialize", init) + rawResult, err := c.Request(ctx, "initialize", init) if err != nil { _ = c.Close() return InitializeResult{}, fmt.Errorf("codex rpc: initialize: %w", err) diff --git a/apps/daemon/internal/agent/codex/rpc_process_test.go b/apps/daemon/internal/agent/codex/rpc_process_test.go index d499a8bd2..a3c2e49cf 100644 --- a/apps/daemon/internal/agent/codex/rpc_process_test.go +++ b/apps/daemon/internal/agent/codex/rpc_process_test.go @@ -4,8 +4,10 @@ import ( "bufio" "context" "encoding/json" + "errors" "fmt" "os" + "strings" "testing" "time" ) @@ -70,11 +72,78 @@ func TestJSONRPCClientResponseTimeoutStartsAfterWrite(t *testing.T) { } } +func TestJSONRPCClientInitializeUsesDefaultRequestBudget(t *testing.T) { + client := NewJSONRPCClient(JSONRPCConfig{ + Binary: os.Args[0], ExtraArgs: []string{"-test.run=^TestJSONRPCClientFakeCodexProcess$", "--"}, + Env: append(os.Environ(), "CODEX_RPC_FAKE_PROCESS=1", "CODEX_RPC_FAKE_INITIALIZE_DELAY=11s"), + }) + t.Cleanup(func() { _ = client.Close() }) + ctx, cancel := context.WithTimeout(t.Context(), 30*time.Second) + defer cancel() + result, err := client.Start(ctx, InitializeParams{}) + if err != nil || result.UserAgent != "fake-codex" { + t.Fatalf("initialize after remote metadata work: result=%+v error=%v", result, err) + } +} + +func TestJSONRPCClientInitializeFailureReapsChild(t *testing.T) { + for _, mode := range []string{"request-timeout", "owner-cancel", "blocked-write-cancel"} { + t.Run(mode, func(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + cfg := JSONRPCConfig{ + Binary: os.Args[0], ExtraArgs: []string{"-test.run=^TestJSONRPCClientFakeCodexProcess$", "--"}, + Env: append(os.Environ(), "CODEX_RPC_FAKE_PROCESS=1", "CODEX_RPC_FAKE_INITIALIZE_DELAY=1m"), + } + if mode == "request-timeout" { + cfg.RequestTimeout = 100 * time.Millisecond + } + init := InitializeParams{} + if mode == "blocked-write-cancel" { + cfg.Env = append(cfg.Env, "CODEX_RPC_FAKE_BLOCK_INITIALIZE=1") + init.ClientInfo.Name = strings.Repeat("x", 4*1024*1024) + } + client := NewJSONRPCClient(cfg) + t.Cleanup(func() { _ = client.Close() }) + if mode != "request-timeout" { + client.OnNotification("test/initialize_waiting", func(json.RawMessage) { cancel() }) + } + _, err := client.Start(ctx, init) + if err == nil { + t.Fatal("initialization unexpectedly succeeded") + } + if mode == "request-timeout" && !strings.Contains(err.Error(), "initialize timed out after 100ms") { + t.Fatalf("configured request budget did not end initialization: %v", err) + } + if mode != "request-timeout" && !errors.Is(ctx.Err(), context.Canceled) { + t.Fatal("initialization did not observe the explicit owner cancellation", ctx.Err()) + } + select { + case <-client.Done(): + default: + t.Fatal("failed initialization returned before reaping its child") + } + if _, exited := client.process.ExitCode(); !exited || client.Alive() { + t.Fatal("failed initialization retained a live child") + } + }) + } +} + func TestJSONRPCClientFakeCodexProcess(t *testing.T) { if os.Getenv("CODEX_RPC_FAKE_PROCESS") != "1" { return } reader := bufio.NewReader(os.Stdin) + if os.Getenv("CODEX_RPC_FAKE_BLOCK_INITIALIZE") == "1" { + if _, err := reader.ReadByte(); err != nil { + os.Exit(4) + } + fmt.Println(`{"jsonrpc":"2.0","method":"test/initialize_waiting"}`) + for { + time.Sleep(time.Second) + } + } line, err := reader.ReadBytes('\n') if err != nil { fmt.Fprintf(os.Stderr, "read initialize: %v\n", err) @@ -87,6 +156,14 @@ func TestJSONRPCClientFakeCodexProcess(t *testing.T) { fmt.Fprintf(os.Stderr, "decode initialize: %v\n", err) os.Exit(2) } + if delay := os.Getenv("CODEX_RPC_FAKE_INITIALIZE_DELAY"); delay != "" { + duration, err := time.ParseDuration(delay) + if err != nil { + os.Exit(5) + } + fmt.Println(`{"jsonrpc":"2.0","method":"test/initialize_waiting"}`) + time.Sleep(duration) + } resp := map[string]any{ "jsonrpc": "2.0", "id": req.ID,