Skip to content

Commit 2bcac27

Browse files
xxx7xxxxxxx7xxxx
authored
fix(windows): Use HTTP/1.1 to download codex (#327)
1 parent f753361 commit 2bcac27

3 files changed

Lines changed: 190 additions & 62 deletions

File tree

‎internal/api/agent_runtimes_test.go‎

Lines changed: 31 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
package api
22

33
import (
4-
"archive/tar"
54
"bytes"
6-
"compress/gzip"
75
"encoding/binary"
86
"encoding/json"
97
"net/http"
108
"net/http/httptest"
119
"os"
1210
"path/filepath"
11+
"strconv"
1312
"sync/atomic"
1413
"testing"
1514

@@ -46,51 +45,49 @@ func TestAgentRuntimesListUsesCodexPathEnvironment(t *testing.T) {
4645
}
4746
}
4847

49-
func TestAgentRuntimeInstallE2ERetriesFailedDownload(t *testing.T) {
50-
target := filepath.Join(t.TempDir(), "bin", "codex")
48+
func TestAgentRuntimeInstallE2ERetriesInterruptedWindowsDownload(t *testing.T) {
49+
target := filepath.Join(t.TempDir(), "bin", "codex.exe")
5150
t.Setenv(codexcli.EnvBinaryPath, target)
52-
binaryPayload := apiTestMachOBinary("arm64")
53-
payload := apiTestCodexArchive(t, "codex-aarch64-apple-darwin", string(binaryPayload))
51+
binaryPayload := apiTestPEBinary("amd64")
5452
var downloads atomic.Int32
5553
downloadServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
56-
if r.URL.Path != "/macos/arm64" || r.URL.Query().Get("package") != "codex-cli" {
57-
t.Errorf("download URL = %s, want /macos/arm64?package=codex-cli", r.URL.String())
54+
if r.URL.Path != "/windows/amd64" || r.URL.Query().Get("package") != "codex-cli" {
55+
t.Errorf("download URL = %s, want /windows/amd64?package=codex-cli", r.URL.String())
5856
}
5957
if downloads.Add(1) == 1 {
60-
http.Error(w, "temporary upstream failure", http.StatusBadGateway)
58+
hijacker, ok := w.(http.Hijacker)
59+
if !ok {
60+
t.Fatal("download response does not support connection hijacking")
61+
}
62+
connection, buffered, err := hijacker.Hijack()
63+
if err != nil {
64+
t.Errorf("Hijack() error = %v", err)
65+
return
66+
}
67+
defer connection.Close()
68+
_, _ = buffered.WriteString("HTTP/1.1 200 OK\r\nContent-Length: " + strconv.Itoa(len(binaryPayload)) + "\r\n\r\n")
69+
_, _ = buffered.Write(binaryPayload[:len(binaryPayload)/2])
70+
_ = buffered.Flush()
6171
return
6272
}
6373
w.WriteHeader(http.StatusOK)
64-
_, _ = w.Write(payload)
74+
_, _ = w.Write(binaryPayload)
6575
}))
6676
defer downloadServer.Close()
6777
installer := codexcli.NewInstaller(codexcli.InstallerOptions{
6878
BaseURL: downloadServer.URL,
69-
GOOS: "darwin",
70-
GOARCH: "arm64",
79+
GOOS: "windows",
80+
GOARCH: "amd64",
7181
})
7282
handler := &Handler{}
7383
handler.SetAgentRuntimeService(runtimecatalog.NewService(
7484
runtimecatalog.WithCodexInstaller(installer),
75-
runtimecatalog.WithPlatform("darwin", "arm64"),
85+
runtimecatalog.WithPlatform("windows", "amd64"),
7686
))
7787
server := httptest.NewServer(handler.Routes())
7888
defer server.Close()
7989

8090
response := agentRuntimeRequest(t, server.Client(), http.MethodPost, server.URL+"/api/v1/agent-runtimes/codex/install")
81-
if response.StatusCode != http.StatusBadGateway {
82-
t.Fatalf("first POST status = %d, want 502; body=%s", response.StatusCode, readTestResponse(t, response))
83-
}
84-
_ = readTestResponse(t, response)
85-
86-
response = agentRuntimeRequest(t, server.Client(), http.MethodGet, server.URL+"/api/v1/agent-runtimes")
87-
var afterFailure []runtimecatalog.Runtime
88-
decodeTestJSON(t, response, &afterFailure)
89-
if got := afterFailure[0]; got.Status != string(codexcli.InstallStateFailed) || got.Message == "" {
90-
t.Fatalf("Codex after failure = %+v, want failed with message", got)
91-
}
92-
93-
response = agentRuntimeRequest(t, server.Client(), http.MethodPost, server.URL+"/api/v1/agent-runtimes/codex/install")
9491
var installed runtimecatalog.Runtime
9592
decodeTestJSON(t, response, &installed)
9693
if !installed.Installed || installed.Path != target || installed.Status != string(codexcli.InstallStateInstalled) {
@@ -137,35 +134,16 @@ func TestAgentRuntimeInstallRejectsClaudeCodeAndUnknownRuntime(t *testing.T) {
137134
}
138135
}
139136

140-
func apiTestCodexArchive(t *testing.T, name, body string) []byte {
141-
t.Helper()
142-
var compressed bytes.Buffer
143-
gzipWriter := gzip.NewWriter(&compressed)
144-
tarWriter := tar.NewWriter(gzipWriter)
145-
header := &tar.Header{Name: name, Mode: 0o755, Size: int64(len(body)), Typeflag: tar.TypeReg}
146-
if err := tarWriter.WriteHeader(header); err != nil {
147-
t.Fatalf("WriteHeader() error = %v", err)
148-
}
149-
if _, err := tarWriter.Write([]byte(body)); err != nil {
150-
t.Fatalf("Write() error = %v", err)
151-
}
152-
if err := tarWriter.Close(); err != nil {
153-
t.Fatalf("tar Close() error = %v", err)
154-
}
155-
if err := gzipWriter.Close(); err != nil {
156-
t.Fatalf("gzip Close() error = %v", err)
157-
}
158-
return compressed.Bytes()
159-
}
160-
161-
func apiTestMachOBinary(arch string) []byte {
162-
data := make([]byte, 32)
163-
copy(data[:4], []byte{0xcf, 0xfa, 0xed, 0xfe})
164-
cpuType := uint32(0x01000007)
137+
func apiTestPEBinary(arch string) []byte {
138+
data := make([]byte, 128)
139+
copy(data[:2], "MZ")
140+
binary.LittleEndian.PutUint32(data[0x3c:0x40], 0x40)
141+
copy(data[0x40:0x44], "PE\x00\x00")
142+
machine := uint16(0x8664)
165143
if arch == "arm64" {
166-
cpuType = 0x0100000c
144+
machine = 0xaa64
167145
}
168-
binary.LittleEndian.PutUint32(data[4:8], cpuType)
146+
binary.LittleEndian.PutUint16(data[0x44:0x46], machine)
169147
return data
170148
}
171149

‎internal/codexcli/installer.go‎

Lines changed: 92 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ const (
2323
DefaultDownloadBaseURL = "https://csgclaw.opencsg.com/codex-cli/latest"
2424
EnvDownloadBaseURL = "CSGCLAW_CODEX_DOWNLOAD_BASE_URL"
2525
defaultInstallTimeout = 15 * time.Minute
26+
defaultInstallAttempts = 3
27+
installRetryBaseDelay = 100 * time.Millisecond
2628
maxDownloadSize = int64(512 << 20)
2729
maxBinarySize = int64(512 << 20)
2830
maxArchiveExpandedSize = int64(768 << 20)
@@ -80,6 +82,31 @@ type downloadPlatform struct {
8082
archiveBinary string
8183
}
8284

85+
type retryableInstallError struct {
86+
err error
87+
}
88+
89+
func (e *retryableInstallError) Error() string {
90+
return e.err.Error()
91+
}
92+
93+
func (e *retryableInstallError) Unwrap() error {
94+
return e.err
95+
}
96+
97+
type downloadReader struct {
98+
reader io.Reader
99+
err error
100+
}
101+
102+
func (r *downloadReader) Read(buffer []byte) (int, error) {
103+
read, err := r.reader.Read(buffer)
104+
if err != nil && !errors.Is(err, io.EOF) {
105+
r.err = err
106+
}
107+
return read, err
108+
}
109+
83110
func NewInstaller(opts InstallerOptions) *Installer {
84111
goos := strings.ToLower(strings.TrimSpace(opts.GOOS))
85112
if goos == "" {
@@ -98,7 +125,7 @@ func NewInstaller(opts InstallerOptions) *Installer {
98125
}
99126
client := opts.HTTPClient
100127
if client == nil {
101-
client = &http.Client{Timeout: defaultInstallTimeout}
128+
client = defaultHTTPClient(goos)
102129
}
103130
baseURL := strings.TrimRight(strings.TrimSpace(opts.BaseURL), "/")
104131
if baseURL == "" {
@@ -117,6 +144,17 @@ func NewInstaller(opts InstallerOptions) *Installer {
117144
}
118145
}
119146

147+
func defaultHTTPClient(goos string) *http.Client {
148+
transport := http.DefaultTransport.(*http.Transport).Clone()
149+
if strings.EqualFold(strings.TrimSpace(goos), "windows") {
150+
protocols := new(http.Protocols)
151+
protocols.SetHTTP1(true)
152+
transport.Protocols = protocols
153+
transport.ForceAttemptHTTP2 = false
154+
}
155+
return &http.Client{Timeout: defaultInstallTimeout, Transport: transport}
156+
}
157+
120158
func (i *Installer) Status() InstallStatus {
121159
if i == nil {
122160
return InstallStatus{State: InstallStateFailed, Message: "Codex installer is not configured"}
@@ -199,7 +237,23 @@ func (i *Installer) install(ctx context.Context) (InstallStatus, error) {
199237
if err := os.MkdirAll(filepath.Dir(targetPath), 0o755); err != nil {
200238
return InstallStatus{}, fmt.Errorf("create Codex install directory: %w", err)
201239
}
240+
for attempt := 1; attempt <= defaultInstallAttempts; attempt++ {
241+
status, err := i.installAttempt(ctx, platform, downloadURL, targetPath)
242+
if err == nil {
243+
return status, nil
244+
}
245+
var retryable *retryableInstallError
246+
if !errors.As(err, &retryable) || attempt == defaultInstallAttempts {
247+
return InstallStatus{}, err
248+
}
249+
if err := waitForInstallRetry(ctx, attempt); err != nil {
250+
return InstallStatus{}, fmt.Errorf("retry Codex CLI download: %w", err)
251+
}
252+
}
253+
return InstallStatus{}, errors.New("install Codex CLI: exhausted download attempts")
254+
}
202255

256+
func (i *Installer) installAttempt(ctx context.Context, platform downloadPlatform, downloadURL, targetPath string) (InstallStatus, error) {
203257
req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, nil)
204258
if err != nil {
205259
return InstallStatus{}, fmt.Errorf("create Codex download request: %w", err)
@@ -208,16 +262,26 @@ func (i *Installer) install(ctx context.Context) (InstallStatus, error) {
208262
req.Header.Set("User-Agent", "csgclaw-codex-installer")
209263
resp, err := i.httpClient.Do(req)
210264
if err != nil {
211-
return InstallStatus{}, fmt.Errorf("download Codex CLI: %w", err)
265+
err = fmt.Errorf("download Codex CLI: %w", err)
266+
if ctx.Err() == nil {
267+
err = &retryableInstallError{err: err}
268+
}
269+
return InstallStatus{}, err
212270
}
213271
defer resp.Body.Close()
214272
if resp.StatusCode != http.StatusOK {
215273
message, _ := io.ReadAll(io.LimitReader(resp.Body, 4096))
216274
detail := strings.TrimSpace(string(message))
275+
var err error
217276
if detail != "" {
218-
return InstallStatus{}, fmt.Errorf("download Codex CLI: server returned %s: %s", resp.Status, detail)
277+
err = fmt.Errorf("download Codex CLI: server returned %s: %s", resp.Status, detail)
278+
} else {
279+
err = fmt.Errorf("download Codex CLI: server returned %s", resp.Status)
280+
}
281+
if retryableHTTPStatus(resp.StatusCode) {
282+
err = &retryableInstallError{err: err}
219283
}
220-
return InstallStatus{}, fmt.Errorf("download Codex CLI: server returned %s", resp.Status)
284+
return InstallStatus{}, err
221285
}
222286
if resp.ContentLength > maxDownloadSize {
223287
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) {
235299
}
236300
}()
237301

238-
limited := &io.LimitedReader{R: resp.Body, N: maxDownloadSize + 1}
302+
body := &downloadReader{reader: resp.Body}
303+
limited := &io.LimitedReader{R: body, N: maxDownloadSize + 1}
239304
if i.goos == "windows" {
240305
err = installWindowsBinary(temp, limited)
241306
} else {
242307
err = installUnixArchive(temp, limited, platform.archiveBinary)
243308
}
244309
if err != nil {
310+
if body.err != nil {
311+
err = &retryableInstallError{err: err}
312+
}
245313
return InstallStatus{}, err
246314
}
247315
if err := validatePlatformBinary(temp, i.goos, i.goarch); err != nil {
@@ -271,6 +339,25 @@ func (i *Installer) install(ctx context.Context) (InstallStatus, error) {
271339
return InstallStatus{State: InstallStateInstalled, Installed: true, Path: installedPath}, nil
272340
}
273341

342+
func retryableHTTPStatus(status int) bool {
343+
return status == http.StatusRequestTimeout ||
344+
status == http.StatusTooEarly ||
345+
status == http.StatusTooManyRequests ||
346+
status >= http.StatusInternalServerError && status < 600
347+
}
348+
349+
func waitForInstallRetry(ctx context.Context, attempt int) error {
350+
delay := installRetryBaseDelay << (attempt - 1)
351+
timer := time.NewTimer(delay)
352+
defer timer.Stop()
353+
select {
354+
case <-ctx.Done():
355+
return ctx.Err()
356+
case <-timer.C:
357+
return nil
358+
}
359+
}
360+
274361
func installWindowsBinary(dst *os.File, src io.Reader) error {
275362
written, err := io.Copy(dst, io.LimitReader(src, maxBinarySize+1))
276363
if err != nil {

‎internal/codexcli/installer_test.go‎

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ func TestInstallerEnsureCoalescesConcurrentDownloads(t *testing.T) {
258258
}
259259
}
260260

261-
func TestInstallerEnsureCanRetryAfterDownloadFailure(t *testing.T) {
261+
func TestInstallerEnsureAutomaticallyRetriesTemporaryDownloadFailure(t *testing.T) {
262262
target := filepath.Join(t.TempDir(), "codex")
263263
t.Setenv(EnvBinaryPath, target)
264264
payload := unixCodexArchive(t, []archiveEntry{{name: "codex-x86_64-unknown-linux-musl", body: string(testELFBinary("amd64"))}})
@@ -274,8 +274,33 @@ func TestInstallerEnsureCanRetryAfterDownloadFailure(t *testing.T) {
274274
defer server.Close()
275275
installer := NewInstaller(InstallerOptions{BaseURL: server.URL, GOOS: "linux", GOARCH: "amd64"})
276276

277+
status, err := installer.Ensure(context.Background())
278+
if err != nil || !status.Installed {
279+
t.Fatalf("Ensure() = %+v, %v; want installed after automatic retry", status, err)
280+
}
281+
if got := requests.Load(); got != 2 {
282+
t.Fatalf("download requests = %d, want 2", got)
283+
}
284+
}
285+
286+
func TestInstallerEnsureCanRetryAfterAutomaticRetriesAreExhausted(t *testing.T) {
287+
target := filepath.Join(t.TempDir(), "codex")
288+
t.Setenv(EnvBinaryPath, target)
289+
payload := unixCodexArchive(t, []archiveEntry{{name: "codex-x86_64-unknown-linux-musl", body: string(testELFBinary("amd64"))}})
290+
var requests atomic.Int32
291+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
292+
if requests.Add(1) <= defaultInstallAttempts {
293+
http.Error(w, "temporary failure", http.StatusBadGateway)
294+
return
295+
}
296+
w.WriteHeader(http.StatusOK)
297+
_, _ = w.Write(payload)
298+
}))
299+
defer server.Close()
300+
installer := NewInstaller(InstallerOptions{BaseURL: server.URL, GOOS: "linux", GOARCH: "amd64"})
301+
277302
if status, err := installer.Ensure(context.Background()); err == nil || status.State != InstallStateFailed {
278-
t.Fatalf("first Ensure() = %+v, %v; want failed", status, err)
303+
t.Fatalf("first Ensure() = %+v, %v; want failed after automatic retries", status, err)
279304
}
280305
if status := installer.Status(); status.State != InstallStateFailed || status.Message == "" {
281306
t.Fatalf("Status() = %+v, want retryable failure", status)
@@ -284,8 +309,46 @@ func TestInstallerEnsureCanRetryAfterDownloadFailure(t *testing.T) {
284309
if err != nil || !status.Installed {
285310
t.Fatalf("second Ensure() = %+v, %v; want installed", status, err)
286311
}
287-
if got := requests.Load(); got != 2 {
288-
t.Fatalf("download requests = %d, want 2", got)
312+
if got := requests.Load(); got != defaultInstallAttempts+1 {
313+
t.Fatalf("download requests = %d, want %d", got, defaultInstallAttempts+1)
314+
}
315+
}
316+
317+
func TestInstallerDefaultHTTPClientUsesPlatformProtocol(t *testing.T) {
318+
for _, test := range []struct {
319+
goos string
320+
wantProtocol string
321+
}{
322+
{goos: "windows", wantProtocol: "HTTP/1.1"},
323+
{goos: "darwin", wantProtocol: "HTTP/2.0"},
324+
{goos: "linux", wantProtocol: "HTTP/2.0"},
325+
} {
326+
t.Run(test.goos, func(t *testing.T) {
327+
negotiatedProtocol := make(chan string, 1)
328+
server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
329+
negotiatedProtocol <- r.Proto
330+
w.WriteHeader(http.StatusOK)
331+
}))
332+
server.EnableHTTP2 = true
333+
server.StartTLS()
334+
defer server.Close()
335+
336+
installer := NewInstaller(InstallerOptions{GOOS: test.goos})
337+
transport, ok := installer.httpClient.Transport.(*http.Transport)
338+
if !ok {
339+
t.Fatalf("HTTP transport = %T, want *http.Transport", installer.httpClient.Transport)
340+
}
341+
serverTransport := server.Client().Transport.(*http.Transport)
342+
transport.TLSClientConfig = serverTransport.TLSClientConfig.Clone()
343+
response, err := installer.httpClient.Get(server.URL)
344+
if err != nil {
345+
t.Fatalf("GET error = %v", err)
346+
}
347+
_ = response.Body.Close()
348+
if got := <-negotiatedProtocol; got != test.wantProtocol {
349+
t.Fatalf("negotiated protocol = %q, want %s", got, test.wantProtocol)
350+
}
351+
})
289352
}
290353
}
291354

0 commit comments

Comments
 (0)