Skip to content

Commit 7b7abbd

Browse files
authored
fix(windows): revert to use HTTP/2 to download codex (#328)
1 parent 2bcac27 commit 7b7abbd

3 files changed

Lines changed: 24 additions & 26 deletions

File tree

internal/api/agent_runtimes_test.go

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -50,34 +50,36 @@ func TestAgentRuntimeInstallE2ERetriesInterruptedWindowsDownload(t *testing.T) {
5050
t.Setenv(codexcli.EnvBinaryPath, target)
5151
binaryPayload := apiTestPEBinary("amd64")
5252
var downloads atomic.Int32
53-
downloadServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
53+
downloadServer := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
5454
if r.URL.Path != "/windows/amd64" || r.URL.Query().Get("package") != "codex-cli" {
5555
t.Errorf("download URL = %s, want /windows/amd64?package=codex-cli", r.URL.String())
5656
}
57+
if r.Proto != "HTTP/2.0" {
58+
t.Errorf("download protocol = %s, want HTTP/2.0", r.Proto)
59+
}
5760
if downloads.Add(1) == 1 {
58-
hijacker, ok := w.(http.Hijacker)
61+
flusher, ok := w.(http.Flusher)
5962
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)
63+
t.Error("download response does not support flushing")
6564
return
6665
}
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()
71-
return
66+
w.Header().Set("Content-Length", strconv.Itoa(len(binaryPayload)))
67+
w.WriteHeader(http.StatusOK)
68+
_, _ = w.Write(binaryPayload[:len(binaryPayload)/2])
69+
flusher.Flush()
70+
panic(http.ErrAbortHandler)
7271
}
7372
w.WriteHeader(http.StatusOK)
7473
_, _ = w.Write(binaryPayload)
7574
}))
75+
downloadServer.EnableHTTP2 = true
76+
downloadServer.StartTLS()
7677
defer downloadServer.Close()
7778
installer := codexcli.NewInstaller(codexcli.InstallerOptions{
78-
BaseURL: downloadServer.URL,
79-
GOOS: "windows",
80-
GOARCH: "amd64",
79+
HTTPClient: downloadServer.Client(),
80+
BaseURL: downloadServer.URL,
81+
GOOS: "windows",
82+
GOARCH: "amd64",
8183
})
8284
handler := &Handler{}
8385
handler.SetAgentRuntimeService(runtimecatalog.NewService(

internal/codexcli/installer.go

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ func NewInstaller(opts InstallerOptions) *Installer {
125125
}
126126
client := opts.HTTPClient
127127
if client == nil {
128-
client = defaultHTTPClient(goos)
128+
client = defaultHTTPClient()
129129
}
130130
baseURL := strings.TrimRight(strings.TrimSpace(opts.BaseURL), "/")
131131
if baseURL == "" {
@@ -144,15 +144,11 @@ func NewInstaller(opts InstallerOptions) *Installer {
144144
}
145145
}
146146

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
147+
func defaultHTTPClient() *http.Client {
148+
return &http.Client{
149+
Timeout: defaultInstallTimeout,
150+
Transport: http.DefaultTransport.(*http.Transport).Clone(),
154151
}
155-
return &http.Client{Timeout: defaultInstallTimeout, Transport: transport}
156152
}
157153

158154
func (i *Installer) Status() InstallStatus {

internal/codexcli/installer_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -314,12 +314,12 @@ func TestInstallerEnsureCanRetryAfterAutomaticRetriesAreExhausted(t *testing.T)
314314
}
315315
}
316316

317-
func TestInstallerDefaultHTTPClientUsesPlatformProtocol(t *testing.T) {
317+
func TestInstallerDefaultHTTPClientNegotiatesHTTP2OnEveryPlatform(t *testing.T) {
318318
for _, test := range []struct {
319319
goos string
320320
wantProtocol string
321321
}{
322-
{goos: "windows", wantProtocol: "HTTP/1.1"},
322+
{goos: "windows", wantProtocol: "HTTP/2.0"},
323323
{goos: "darwin", wantProtocol: "HTTP/2.0"},
324324
{goos: "linux", wantProtocol: "HTTP/2.0"},
325325
} {

0 commit comments

Comments
 (0)