diff --git a/internal/api/agent_runtimes_test.go b/internal/api/agent_runtimes_test.go index dc73d985..4c87b997 100644 --- a/internal/api/agent_runtimes_test.go +++ b/internal/api/agent_runtimes_test.go @@ -1,15 +1,14 @@ package api import ( - "archive/tar" "bytes" - "compress/gzip" "encoding/binary" "encoding/json" "net/http" "net/http/httptest" "os" "path/filepath" + "strconv" "sync/atomic" "testing" @@ -46,51 +45,49 @@ func TestAgentRuntimesListUsesCodexPathEnvironment(t *testing.T) { } } -func TestAgentRuntimeInstallE2ERetriesFailedDownload(t *testing.T) { - target := filepath.Join(t.TempDir(), "bin", "codex") +func TestAgentRuntimeInstallE2ERetriesInterruptedWindowsDownload(t *testing.T) { + target := filepath.Join(t.TempDir(), "bin", "codex.exe") t.Setenv(codexcli.EnvBinaryPath, target) - binaryPayload := apiTestMachOBinary("arm64") - payload := apiTestCodexArchive(t, "codex-aarch64-apple-darwin", string(binaryPayload)) + binaryPayload := apiTestPEBinary("amd64") var downloads atomic.Int32 downloadServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != "/macos/arm64" || r.URL.Query().Get("package") != "codex-cli" { - t.Errorf("download URL = %s, want /macos/arm64?package=codex-cli", r.URL.String()) + if r.URL.Path != "/windows/amd64" || r.URL.Query().Get("package") != "codex-cli" { + t.Errorf("download URL = %s, want /windows/amd64?package=codex-cli", r.URL.String()) } if downloads.Add(1) == 1 { - http.Error(w, "temporary upstream failure", http.StatusBadGateway) + hijacker, ok := w.(http.Hijacker) + if !ok { + t.Fatal("download response does not support connection hijacking") + } + connection, buffered, err := hijacker.Hijack() + if err != nil { + t.Errorf("Hijack() error = %v", err) + return + } + defer connection.Close() + _, _ = buffered.WriteString("HTTP/1.1 200 OK\r\nContent-Length: " + strconv.Itoa(len(binaryPayload)) + "\r\n\r\n") + _, _ = buffered.Write(binaryPayload[:len(binaryPayload)/2]) + _ = buffered.Flush() return } w.WriteHeader(http.StatusOK) - _, _ = w.Write(payload) + _, _ = w.Write(binaryPayload) })) defer downloadServer.Close() installer := codexcli.NewInstaller(codexcli.InstallerOptions{ BaseURL: downloadServer.URL, - GOOS: "darwin", - GOARCH: "arm64", + GOOS: "windows", + GOARCH: "amd64", }) handler := &Handler{} handler.SetAgentRuntimeService(runtimecatalog.NewService( runtimecatalog.WithCodexInstaller(installer), - runtimecatalog.WithPlatform("darwin", "arm64"), + runtimecatalog.WithPlatform("windows", "amd64"), )) server := httptest.NewServer(handler.Routes()) defer server.Close() response := agentRuntimeRequest(t, server.Client(), http.MethodPost, server.URL+"/api/v1/agent-runtimes/codex/install") - if response.StatusCode != http.StatusBadGateway { - t.Fatalf("first POST status = %d, want 502; body=%s", response.StatusCode, readTestResponse(t, response)) - } - _ = readTestResponse(t, response) - - response = agentRuntimeRequest(t, server.Client(), http.MethodGet, server.URL+"/api/v1/agent-runtimes") - var afterFailure []runtimecatalog.Runtime - decodeTestJSON(t, response, &afterFailure) - if got := afterFailure[0]; got.Status != string(codexcli.InstallStateFailed) || got.Message == "" { - t.Fatalf("Codex after failure = %+v, want failed with message", got) - } - - response = agentRuntimeRequest(t, server.Client(), http.MethodPost, server.URL+"/api/v1/agent-runtimes/codex/install") var installed runtimecatalog.Runtime decodeTestJSON(t, response, &installed) if !installed.Installed || installed.Path != target || installed.Status != string(codexcli.InstallStateInstalled) { @@ -137,35 +134,16 @@ func TestAgentRuntimeInstallRejectsClaudeCodeAndUnknownRuntime(t *testing.T) { } } -func apiTestCodexArchive(t *testing.T, name, body string) []byte { - t.Helper() - var compressed bytes.Buffer - gzipWriter := gzip.NewWriter(&compressed) - tarWriter := tar.NewWriter(gzipWriter) - header := &tar.Header{Name: name, Mode: 0o755, Size: int64(len(body)), Typeflag: tar.TypeReg} - if err := tarWriter.WriteHeader(header); err != nil { - t.Fatalf("WriteHeader() error = %v", err) - } - if _, err := tarWriter.Write([]byte(body)); err != nil { - t.Fatalf("Write() error = %v", err) - } - if err := tarWriter.Close(); err != nil { - t.Fatalf("tar Close() error = %v", err) - } - if err := gzipWriter.Close(); err != nil { - t.Fatalf("gzip Close() error = %v", err) - } - return compressed.Bytes() -} - -func apiTestMachOBinary(arch string) []byte { - data := make([]byte, 32) - copy(data[:4], []byte{0xcf, 0xfa, 0xed, 0xfe}) - cpuType := uint32(0x01000007) +func apiTestPEBinary(arch string) []byte { + data := make([]byte, 128) + copy(data[:2], "MZ") + binary.LittleEndian.PutUint32(data[0x3c:0x40], 0x40) + copy(data[0x40:0x44], "PE\x00\x00") + machine := uint16(0x8664) if arch == "arm64" { - cpuType = 0x0100000c + machine = 0xaa64 } - binary.LittleEndian.PutUint32(data[4:8], cpuType) + binary.LittleEndian.PutUint16(data[0x44:0x46], machine) return data } diff --git a/internal/codexcli/installer.go b/internal/codexcli/installer.go index 8821c1e9..793e5889 100644 --- a/internal/codexcli/installer.go +++ b/internal/codexcli/installer.go @@ -23,6 +23,8 @@ const ( DefaultDownloadBaseURL = "https://csgclaw.opencsg.com/codex-cli/latest" EnvDownloadBaseURL = "CSGCLAW_CODEX_DOWNLOAD_BASE_URL" defaultInstallTimeout = 15 * time.Minute + defaultInstallAttempts = 3 + installRetryBaseDelay = 100 * time.Millisecond maxDownloadSize = int64(512 << 20) maxBinarySize = int64(512 << 20) maxArchiveExpandedSize = int64(768 << 20) @@ -80,6 +82,31 @@ type downloadPlatform struct { archiveBinary string } +type retryableInstallError struct { + err error +} + +func (e *retryableInstallError) Error() string { + return e.err.Error() +} + +func (e *retryableInstallError) Unwrap() error { + return e.err +} + +type downloadReader struct { + reader io.Reader + err error +} + +func (r *downloadReader) Read(buffer []byte) (int, error) { + read, err := r.reader.Read(buffer) + if err != nil && !errors.Is(err, io.EOF) { + r.err = err + } + return read, err +} + func NewInstaller(opts InstallerOptions) *Installer { goos := strings.ToLower(strings.TrimSpace(opts.GOOS)) if goos == "" { @@ -98,7 +125,7 @@ func NewInstaller(opts InstallerOptions) *Installer { } client := opts.HTTPClient if client == nil { - client = &http.Client{Timeout: defaultInstallTimeout} + client = defaultHTTPClient(goos) } baseURL := strings.TrimRight(strings.TrimSpace(opts.BaseURL), "/") if baseURL == "" { @@ -117,6 +144,17 @@ func NewInstaller(opts InstallerOptions) *Installer { } } +func defaultHTTPClient(goos string) *http.Client { + transport := http.DefaultTransport.(*http.Transport).Clone() + if strings.EqualFold(strings.TrimSpace(goos), "windows") { + protocols := new(http.Protocols) + protocols.SetHTTP1(true) + transport.Protocols = protocols + transport.ForceAttemptHTTP2 = false + } + return &http.Client{Timeout: defaultInstallTimeout, Transport: transport} +} + func (i *Installer) Status() InstallStatus { if i == nil { return InstallStatus{State: InstallStateFailed, Message: "Codex installer is not configured"} @@ -199,7 +237,23 @@ func (i *Installer) install(ctx context.Context) (InstallStatus, error) { if err := os.MkdirAll(filepath.Dir(targetPath), 0o755); err != nil { return InstallStatus{}, fmt.Errorf("create Codex install directory: %w", err) } + for attempt := 1; attempt <= defaultInstallAttempts; attempt++ { + status, err := i.installAttempt(ctx, platform, downloadURL, targetPath) + if err == nil { + return status, nil + } + var retryable *retryableInstallError + if !errors.As(err, &retryable) || attempt == defaultInstallAttempts { + return InstallStatus{}, err + } + if err := waitForInstallRetry(ctx, attempt); err != nil { + return InstallStatus{}, fmt.Errorf("retry Codex CLI download: %w", err) + } + } + return InstallStatus{}, errors.New("install Codex CLI: exhausted download attempts") +} +func (i *Installer) installAttempt(ctx context.Context, platform downloadPlatform, downloadURL, targetPath string) (InstallStatus, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, nil) if err != nil { return InstallStatus{}, fmt.Errorf("create Codex download request: %w", err) @@ -208,16 +262,26 @@ func (i *Installer) install(ctx context.Context) (InstallStatus, error) { req.Header.Set("User-Agent", "csgclaw-codex-installer") resp, err := i.httpClient.Do(req) if err != nil { - return InstallStatus{}, fmt.Errorf("download Codex CLI: %w", err) + err = fmt.Errorf("download Codex CLI: %w", err) + if ctx.Err() == nil { + err = &retryableInstallError{err: err} + } + return InstallStatus{}, err } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { message, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) detail := strings.TrimSpace(string(message)) + var err error if detail != "" { - return InstallStatus{}, fmt.Errorf("download Codex CLI: server returned %s: %s", resp.Status, detail) + err = fmt.Errorf("download Codex CLI: server returned %s: %s", resp.Status, detail) + } else { + err = fmt.Errorf("download Codex CLI: server returned %s", resp.Status) + } + if retryableHTTPStatus(resp.StatusCode) { + err = &retryableInstallError{err: err} } - return InstallStatus{}, fmt.Errorf("download Codex CLI: server returned %s", resp.Status) + return InstallStatus{}, err } if resp.ContentLength > maxDownloadSize { return InstallStatus{}, fmt.Errorf("download Codex CLI: package size %d exceeds limit %d", resp.ContentLength, maxDownloadSize) @@ -235,13 +299,17 @@ func (i *Installer) install(ctx context.Context) (InstallStatus, error) { } }() - limited := &io.LimitedReader{R: resp.Body, N: maxDownloadSize + 1} + body := &downloadReader{reader: resp.Body} + limited := &io.LimitedReader{R: body, N: maxDownloadSize + 1} if i.goos == "windows" { err = installWindowsBinary(temp, limited) } else { err = installUnixArchive(temp, limited, platform.archiveBinary) } if err != nil { + if body.err != nil { + err = &retryableInstallError{err: err} + } return InstallStatus{}, err } if err := validatePlatformBinary(temp, i.goos, i.goarch); err != nil { @@ -271,6 +339,25 @@ func (i *Installer) install(ctx context.Context) (InstallStatus, error) { return InstallStatus{State: InstallStateInstalled, Installed: true, Path: installedPath}, nil } +func retryableHTTPStatus(status int) bool { + return status == http.StatusRequestTimeout || + status == http.StatusTooEarly || + status == http.StatusTooManyRequests || + status >= http.StatusInternalServerError && status < 600 +} + +func waitForInstallRetry(ctx context.Context, attempt int) error { + delay := installRetryBaseDelay << (attempt - 1) + timer := time.NewTimer(delay) + defer timer.Stop() + select { + case <-ctx.Done(): + return ctx.Err() + case <-timer.C: + return nil + } +} + func installWindowsBinary(dst *os.File, src io.Reader) error { written, err := io.Copy(dst, io.LimitReader(src, maxBinarySize+1)) if err != nil { diff --git a/internal/codexcli/installer_test.go b/internal/codexcli/installer_test.go index c9158fbf..90827162 100644 --- a/internal/codexcli/installer_test.go +++ b/internal/codexcli/installer_test.go @@ -258,7 +258,7 @@ func TestInstallerEnsureCoalescesConcurrentDownloads(t *testing.T) { } } -func TestInstallerEnsureCanRetryAfterDownloadFailure(t *testing.T) { +func TestInstallerEnsureAutomaticallyRetriesTemporaryDownloadFailure(t *testing.T) { target := filepath.Join(t.TempDir(), "codex") t.Setenv(EnvBinaryPath, target) payload := unixCodexArchive(t, []archiveEntry{{name: "codex-x86_64-unknown-linux-musl", body: string(testELFBinary("amd64"))}}) @@ -274,8 +274,33 @@ func TestInstallerEnsureCanRetryAfterDownloadFailure(t *testing.T) { defer server.Close() installer := NewInstaller(InstallerOptions{BaseURL: server.URL, GOOS: "linux", GOARCH: "amd64"}) + status, err := installer.Ensure(context.Background()) + if err != nil || !status.Installed { + t.Fatalf("Ensure() = %+v, %v; want installed after automatic retry", status, err) + } + if got := requests.Load(); got != 2 { + t.Fatalf("download requests = %d, want 2", got) + } +} + +func TestInstallerEnsureCanRetryAfterAutomaticRetriesAreExhausted(t *testing.T) { + target := filepath.Join(t.TempDir(), "codex") + t.Setenv(EnvBinaryPath, target) + payload := unixCodexArchive(t, []archiveEntry{{name: "codex-x86_64-unknown-linux-musl", body: string(testELFBinary("amd64"))}}) + var requests atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if requests.Add(1) <= defaultInstallAttempts { + http.Error(w, "temporary failure", http.StatusBadGateway) + return + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write(payload) + })) + defer server.Close() + installer := NewInstaller(InstallerOptions{BaseURL: server.URL, GOOS: "linux", GOARCH: "amd64"}) + if status, err := installer.Ensure(context.Background()); err == nil || status.State != InstallStateFailed { - t.Fatalf("first Ensure() = %+v, %v; want failed", status, err) + t.Fatalf("first Ensure() = %+v, %v; want failed after automatic retries", status, err) } if status := installer.Status(); status.State != InstallStateFailed || status.Message == "" { t.Fatalf("Status() = %+v, want retryable failure", status) @@ -284,8 +309,46 @@ func TestInstallerEnsureCanRetryAfterDownloadFailure(t *testing.T) { if err != nil || !status.Installed { t.Fatalf("second Ensure() = %+v, %v; want installed", status, err) } - if got := requests.Load(); got != 2 { - t.Fatalf("download requests = %d, want 2", got) + if got := requests.Load(); got != defaultInstallAttempts+1 { + t.Fatalf("download requests = %d, want %d", got, defaultInstallAttempts+1) + } +} + +func TestInstallerDefaultHTTPClientUsesPlatformProtocol(t *testing.T) { + for _, test := range []struct { + goos string + wantProtocol string + }{ + {goos: "windows", wantProtocol: "HTTP/1.1"}, + {goos: "darwin", wantProtocol: "HTTP/2.0"}, + {goos: "linux", wantProtocol: "HTTP/2.0"}, + } { + t.Run(test.goos, func(t *testing.T) { + negotiatedProtocol := make(chan string, 1) + server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + negotiatedProtocol <- r.Proto + w.WriteHeader(http.StatusOK) + })) + server.EnableHTTP2 = true + server.StartTLS() + defer server.Close() + + installer := NewInstaller(InstallerOptions{GOOS: test.goos}) + transport, ok := installer.httpClient.Transport.(*http.Transport) + if !ok { + t.Fatalf("HTTP transport = %T, want *http.Transport", installer.httpClient.Transport) + } + serverTransport := server.Client().Transport.(*http.Transport) + transport.TLSClientConfig = serverTransport.TLSClientConfig.Clone() + response, err := installer.httpClient.Get(server.URL) + if err != nil { + t.Fatalf("GET error = %v", err) + } + _ = response.Body.Close() + if got := <-negotiatedProtocol; got != test.wantProtocol { + t.Fatalf("negotiated protocol = %q, want %s", got, test.wantProtocol) + } + }) } }