From 7c0d13176b8d9c1bf943c6fd9dced93e9b7e1262 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Thu, 17 Sep 2026 13:52:52 -0400 Subject: [PATCH 1/3] update: a release download gets its own clock, not the check's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The release client was built as http.Client{Timeout: 3s} because three seconds is right for the launch check, and Go's Client.Timeout covers the WHOLE exchange including every body byte. The same client then downloaded the release asset, so a 57 MB download on a healthy link — about eight seconds here — was cut off three seconds in and a person read `read the release response: context deadline exceeded`. Client.Timeout is gone. The transport bounds only what has made no progress: a 5 s dial, a 5 s TLS handshake, a 10 s wait for response headers. Every call now takes its deadline from its context. The launch notice and `--check` share the one CheckTimeout constant, install metadata gets 10 s per request, and the asset gets a 30-second no-byte stall window plus a 15-minute ceiling — a dead link fails with a sentence, a slow one finishes. No Go deadline text reaches a person. A stall says `downloading codeaf-linux-amd64 stalled — no bytes for 30 s`, the ceiling says `took longer than 15 minutes`, and late headers say `did not start arriving within 10 s`. The stall window, the ceiling and the metadata window are injectable, so the tests prove the clocks with real slow, silent and trickling servers rather than waiting out the shipped ones. Co-Authored-By: Claude Fable 5.1 --- internal/update/check.go | 2 +- internal/update/client.go | 165 +++++++++++++++++----- internal/update/install.go | 126 ++++++++++++++++- internal/update/update_test.go | 242 +++++++++++++++++++++++++++++++++ 4 files changed, 494 insertions(+), 41 deletions(-) diff --git a/internal/update/check.go b/internal/update/check.go index f67cda79ac..a1e44906b8 100644 --- a/internal/update/check.go +++ b/internal/update/check.go @@ -80,7 +80,7 @@ func CheckLaunch(ctx context.Context, options CheckOptions) (Available, bool) { if options.Client == nil { return Available{}, false } - release, err := options.Client.Select(ctx, Choice{Channel: "stable"}) + release, err := options.Client.Check(ctx, Choice{Channel: "stable"}) if err != nil { return Available{}, false } diff --git a/internal/update/client.go b/internal/update/client.go index 1607932925..91fc583bfd 100644 --- a/internal/update/client.go +++ b/internal/update/client.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "net" "net/http" "net/url" "strings" @@ -28,20 +29,37 @@ const ( GitHubDownloadEnv = "CODEAF_GITHUB_DOWNLOAD" ) +const ( + // CheckTimeout is the whole-exchange budget for a launch check and for + // `codeaf update --check`. THE CHECK CLOCK NEVER COVERS AN INSTALL: a + // release asset is allowed to make steady progress for much longer. + CheckTimeout = 3 * time.Second + + networkSetupTimeout = 5 * time.Second + apiRequestTimeout = 10 * time.Second + downloadStallWindow = 30 * time.Second + downloadCeiling = 15 * time.Minute +) + const primaryRepository = "Agent-Field/codeaf" const legacyRepository = "Agent-Field/aforge-v2" // legacy-name // Client talks to the release API and release asset host. type Client struct { - HTTP *http.Client - APIBase string - DownloadBase string - Token string - Revision string + HTTP *http.Client + APIBase string + DownloadBase string + Token string + Revision string + CheckWindow time.Duration + APIWindow time.Duration + StallWindow time.Duration + DownloadCeiling time.Duration } // NewClient builds the release client from the same mirror and token inputs as -// the shell installer. The timeout covers the whole request, including bodies. +// the shell installer. The timeout belongs only to the small release check; +// every install request carries the clock for the particular thing it reads. func NewClient(revision string, timeout time.Duration) *Client { apiBase := strings.TrimRight(internalenv.Get(GitHubAPIEnv), "/") if apiBase == "" { @@ -56,19 +74,34 @@ func NewClient(revision string, timeout time.Duration) *Client { token = internalenv.Value("GH_TOKEN") } return &Client{ - HTTP: &http.Client{Timeout: timeout}, - APIBase: apiBase, - DownloadBase: downloadBase, - Token: token, - Revision: revision, + HTTP: defaultHTTP(), + APIBase: apiBase, + DownloadBase: downloadBase, + Token: token, + Revision: revision, + CheckWindow: timeout, + APIWindow: apiRequestTimeout, + StallWindow: downloadStallWindow, + DownloadCeiling: downloadCeiling, } } +// defaultHTTP bounds only the parts of a request that have made no progress. +// In particular, Client.Timeout stays zero: that field includes every body +// byte and was the three-second clock that cut a healthy 57 MB download off. +func defaultHTTP() *http.Client { + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.DialContext = (&net.Dialer{Timeout: networkSetupTimeout}).DialContext + transport.TLSHandshakeTimeout = networkSetupTimeout + transport.ResponseHeaderTimeout = apiRequestTimeout + return &http.Client{Transport: transport} +} + func (c *Client) httpClient() *http.Client { if c != nil && c.HTTP != nil { return c.HTTP } - return &http.Client{Timeout: 3 * time.Second} + return defaultHTTP() } // requestClient keeps the caller's redirect decisions while enforcing the @@ -78,6 +111,9 @@ func (c *Client) httpClient() *http.Client { func (c *Client) requestClient() *http.Client { base := c.httpClient() client := *base + // EVERY CALL GETS ITS DEADLINE FROM ITS CONTEXT. An injected or reused + // client must not smuggle a whole-body Client.Timeout back into downloads. + client.Timeout = 0 checkRedirect := base.CheckRedirect client.CheckRedirect = func(request *http.Request, via []*http.Request) error { if checkRedirect != nil { @@ -112,30 +148,57 @@ func (e *statusError) Error() string { return fmt.Sprintf("%s answered HTTP %d", e.resource, e.code) } -type requestError struct{ cause error } - -func (e *requestError) Error() string { return "release request failed: " + e.cause.Error() } -func (e *requestError) Unwrap() error { return e.cause } - func isStatus(err error, code int) bool { var status *statusError return errors.As(err, &status) && status.code == code } -func nameReleaseResource(err error, resource string) error { - var status *statusError - if errors.As(err, &status) { - return &statusError{code: status.code, resource: resource} +func (c *Client) checkWindow() time.Duration { + if c != nil && c.CheckWindow > 0 { + return c.CheckWindow + } + return CheckTimeout +} + +func (c *Client) apiWindow() time.Duration { + if c != nil && c.APIWindow > 0 { + return c.APIWindow + } + return apiRequestTimeout +} + +func (c *Client) stallWindow() time.Duration { + if c != nil && c.StallWindow > 0 { + return c.StallWindow + } + return downloadStallWindow +} + +func (c *Client) downloadCeiling() time.Duration { + if c != nil && c.DownloadCeiling > 0 { + return c.DownloadCeiling } - var request *requestError - if errors.As(err, &request) { - return fmt.Errorf("%s request failed: %w", resource, request.cause) + return downloadCeiling +} + +// Check selects one release inside the short whole-exchange window shared by +// the launch notice and the terminal's explicit check. Installation calls +// Select directly, where each metadata request receives its own API window. +func (c *Client) Check(ctx context.Context, choice Choice) (Release, error) { + window := c.checkWindow() + check, cancel := context.WithTimeout(ctx, window) + defer cancel() + release, err := c.Select(check, choice) + if err != nil && ctx.Err() == nil && errors.Is(check.Err(), context.DeadlineExceeded) { + return Release{}, fmt.Errorf("release API did not answer within %s", spellDuration(window)) } - return err + return release, err } -func (c *Client) get(ctx context.Context, rawURL, accept string, api bool) ([]byte, error) { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil) +func (c *Client) get(ctx context.Context, rawURL, accept string, api bool, resource string) ([]byte, error) { + requestContext, cancel := context.WithTimeout(ctx, c.apiWindow()) + defer cancel() + req, err := http.NewRequestWithContext(requestContext, http.MethodGet, rawURL, nil) if err != nil { return nil, err } @@ -148,24 +211,56 @@ func (c *Client) get(ctx context.Context, rawURL, accept string, api bool) ([]by } resp, err := c.requestClient().Do(req) if err != nil { - var requestURL *url.Error - if errors.As(err, &requestURL) { - err = requestURL.Err - } - return nil, &requestError{cause: err} + return nil, metadataRequestFailure(ctx, requestContext, err, resource, c.apiWindow()) } defer resp.Body.Close() if resp.StatusCode < 200 || resp.StatusCode >= 300 { _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 4096)) - return nil, &statusError{code: resp.StatusCode, resource: "release API"} + return nil, &statusError{code: resp.StatusCode, resource: resource} } body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("read the release response: %w", err) + return nil, metadataRequestFailure(ctx, requestContext, err, resource, c.apiWindow()) } return body, nil } +func metadataRequestFailure(parent, request context.Context, err error, resource string, window time.Duration) error { + if parent.Err() != nil { + if errors.Is(parent.Err(), context.DeadlineExceeded) { + return fmt.Errorf("%s did not answer before this update stopped", resource) + } + return fmt.Errorf("the %s request was stopped", resource) + } + if errors.Is(request.Err(), context.DeadlineExceeded) || networkTimedOut(err) { + return fmt.Errorf("%s did not answer within %s", resource, spellDuration(window)) + } + return fmt.Errorf("%s could not be reached", resource) +} + +func networkTimedOut(err error) bool { + var networkError net.Error + return errors.As(err, &networkError) && networkError.Timeout() +} + +func spellDuration(duration time.Duration) string { + switch { + case duration%time.Minute == 0: + minutes := duration / time.Minute + unit := "minutes" + if minutes == 1 { + unit = "minute" + } + return fmt.Sprintf("%d %s", minutes, unit) + case duration%time.Second == 0: + return fmt.Sprintf("%d s", duration/time.Second) + case duration%time.Millisecond == 0: + return fmt.Sprintf("%d ms", duration/time.Millisecond) + default: + return duration.String() + } +} + // Choice selects one release channel or one exact tag. type Choice struct { Channel string @@ -224,7 +319,7 @@ func (c *Client) selectRepository(ctx context.Context, repository, channel strin suffix = "releases?per_page=100" } rawURL := strings.TrimRight(c.APIBase, "/") + "/repos/" + repository + "/" + suffix - body, err := c.get(ctx, rawURL, "application/vnd.github+json", true) + body, err := c.get(ctx, rawURL, "application/vnd.github+json", true, "release API") if err != nil { return Release{}, err } diff --git a/internal/update/install.go b/internal/update/install.go index 320caaa644..c230e70890 100644 --- a/internal/update/install.go +++ b/internal/update/install.go @@ -6,11 +6,13 @@ import ( "encoding/hex" "errors" "fmt" + "io" "net/http" "os" "path/filepath" "runtime" "strings" + "time" ) // InstallOptions describes one in-place replacement. @@ -120,24 +122,138 @@ func installRepositories(first string) []string { func (c *Client) downloadRelease(ctx context.Context, release Release, asset, goos, goarch, extension string) ([]byte, string, []byte, error) { shownAsset := asset - body, err := c.get(ctx, c.assetURL(release, asset), "application/octet-stream", false) + body, err := c.downloadAsset(ctx, c.assetURL(release, asset), shownAsset) if isStatus(err, http.StatusNotFound) { legacyAsset := "aforge-" + goos + "-" + goarch + extension // legacy-name - body, err = c.get(ctx, c.assetURL(release, legacyAsset), "application/octet-stream", false) + body, err = c.downloadAsset(ctx, c.assetURL(release, legacyAsset), shownAsset) if err == nil { asset = legacyAsset } } if err != nil { - return nil, asset, nil, nameReleaseResource(err, shownAsset) + return nil, asset, nil, err } - checksums, err := c.get(ctx, c.assetURL(release, "checksums.txt"), "application/octet-stream", false) + checksums, err := c.get(ctx, c.assetURL(release, "checksums.txt"), "application/octet-stream", false, "checksums.txt") if err != nil { - return nil, asset, nil, nameReleaseResource(err, "checksums.txt") + return nil, asset, nil, err } return body, asset, checksums, nil } +type downloadRead struct { + bytes []byte + err error +} + +func (c *Client) downloadAsset(ctx context.Context, rawURL, asset string) ([]byte, error) { + ceiling := c.downloadCeiling() + downloadContext, cancel := context.WithTimeout(ctx, ceiling) + defer cancel() + + request, err := http.NewRequestWithContext(downloadContext, http.MethodGet, rawURL, nil) + if err != nil { + return nil, fmt.Errorf("%s could not start arriving", asset) + } + request.Header.Set("Accept", "application/octet-stream") + request.Header.Set("User-Agent", c.userAgent()) + response, err := c.requestClient().Do(request) + if err != nil { + switch { + case ctx.Err() != nil: + return nil, fmt.Errorf("downloading %s was stopped", asset) + case errors.Is(downloadContext.Err(), context.DeadlineExceeded): + return nil, downloadCeilingError(asset, ceiling) + case responseHeaderTimedOut(err): + return nil, fmt.Errorf("%s did not start arriving within %s", asset, spellDuration(apiRequestTimeout)) + default: + return nil, fmt.Errorf("%s could not start arriving", asset) + } + } + defer response.Body.Close() + if response.StatusCode < 200 || response.StatusCode >= 300 { + return nil, &statusError{code: response.StatusCode, resource: asset} + } + return readDownloadBody(ctx, downloadContext, response.Body, asset, c.stallWindow(), ceiling) +} + +func responseHeaderTimedOut(err error) bool { + return err != nil && strings.Contains(err.Error(), "timeout awaiting response headers") +} + +// readDownloadBody watches successful reads rather than wall time. A slow link +// that keeps delivering bytes is healthy; one silent read must not hold the +// installer forever even though the HTTP client has no whole-body timeout. +func readDownloadBody(parent, download context.Context, body io.Reader, asset string, stall, ceiling time.Duration) ([]byte, error) { + reads := make(chan downloadRead, 1) + done := make(chan struct{}) + defer close(done) + go func() { + buffer := make([]byte, 32*1024) + for { + count, err := body.Read(buffer) + var copied []byte + if count > 0 { + copied = append([]byte(nil), buffer[:count]...) + } + select { + case reads <- downloadRead{bytes: copied, err: err}: + case <-done: + return + } + if err != nil { + return + } + } + }() + + timer := time.NewTimer(stall) + defer timer.Stop() + var downloaded []byte + for { + select { + case <-download.Done(): + if parent.Err() != nil { + return nil, fmt.Errorf("downloading %s was stopped", asset) + } + return nil, downloadCeilingError(asset, ceiling) + case <-timer.C: + return nil, downloadStallError(asset, stall) + case read := <-reads: + if len(read.bytes) > 0 { + downloaded = append(downloaded, read.bytes...) + if !timer.Stop() { + select { + case <-timer.C: + default: + } + } + timer.Reset(stall) + } + if read.err == nil { + continue + } + if errors.Is(read.err, io.EOF) { + return downloaded, nil + } + if parent.Err() != nil { + return nil, fmt.Errorf("downloading %s was stopped", asset) + } + if errors.Is(download.Err(), context.DeadlineExceeded) { + return nil, downloadCeilingError(asset, ceiling) + } + return nil, fmt.Errorf("downloading %s stopped before it finished", asset) + } + } +} + +func downloadStallError(asset string, window time.Duration) error { + return fmt.Errorf("downloading %s stalled — no bytes for %s", asset, spellDuration(window)) +} + +func downloadCeilingError(asset string, ceiling time.Duration) error { + return fmt.Errorf("downloading %s took longer than %s", asset, spellDuration(ceiling)) +} + func checksumFor(raw []byte, name string) (string, bool) { for _, line := range strings.Split(string(raw), "\n") { fields := strings.Fields(line) diff --git a/internal/update/update_test.go b/internal/update/update_test.go index 1351e1ac70..7d9e729b51 100644 --- a/internal/update/update_test.go +++ b/internal/update/update_test.go @@ -171,6 +171,248 @@ func servedRelease(t *testing.T, asset []byte, checksum string) (*httptest.Serve return server, requests } +// TestTimeoutContractC1SlowAssetOutlivesTheCheckClock proves C1: the check +// clock cannot cut off an asset that continues to deliver bytes. +func TestTimeoutContractC1SlowAssetOutlivesTheCheckClock(t *testing.T) { + asset := make([]byte, 1<<20) + for index := range asset { + asset[index] = byte(index) + } + digest := sha256.Sum256(asset) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, request *http.Request) { + switch { + case strings.HasSuffix(request.URL.Path, "/checksums.txt"): + fmt.Fprintf(w, "%x codeaf-%s-%s\n", digest, runtime.GOOS, runtime.GOARCH) + case strings.HasSuffix(request.URL.Path, "/codeaf-"+runtime.GOOS+"-"+runtime.GOARCH): + flusher := w.(http.Flusher) + const chunks = 13 + for chunk := 0; chunk < chunks; chunk++ { + select { + case <-request.Context().Done(): + return + case <-time.After(500 * time.Millisecond): + } + start := len(asset) * chunk / chunks + end := len(asset) * (chunk + 1) / chunks + if _, err := w.Write(asset[start:end]); err != nil { + return + } + flusher.Flush() + } + default: + http.NotFound(w, request) + } + })) + defer server.Close() + + dir := t.TempDir() + target := filepath.Join(dir, "codeaf") + if err := os.WriteFile(target, []byte("old codeaf"), 0o700); err != nil { + t.Fatal(err) + } + client := releaseClient(server, "v0.1.0") + client.HTTP.Timeout = CheckTimeout + result, err := Install(context.Background(), InstallOptions{ + Client: client, Release: Release{Tag: "v0.2.0", Repository: primaryRepository}, Target: target, + }) + if err != nil { + t.Fatal(err) + } + installed, err := os.ReadFile(target) + if err != nil || string(installed) != string(asset) { + t.Fatalf("installed bytes = %d, error %v; want %d matching bytes", len(installed), err, len(asset)) + } + info, err := os.Stat(target) + if err != nil { + t.Fatal(err) + } + if info.Mode().Perm() != 0o755 || result.Path != target { + t.Fatalf("result = %+v mode = %v", result, info.Mode().Perm()) + } + if matches, _ := filepath.Glob(filepath.Join(dir, ".codeaf.tmp.*")); len(matches) != 0 { + t.Fatalf("temporary files remain: %v", matches) + } +} + +// TestTimeoutContractC2HangingLaunchCheckEndsInsideItsBudget proves C2 for the +// silent launch door with a server that accepts the request and never answers. +func TestTimeoutContractC2HangingLaunchCheckEndsInsideItsBudget(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, request *http.Request) { + <-request.Context().Done() + })) + defer server.Close() + started := time.Now() + answer, show := CheckLaunch(context.Background(), CheckOptions{ + Running: "v0.1.0", ProfileDir: t.TempDir(), Client: releaseClient(server, "v0.1.0"), + }) + if elapsed := time.Since(started); elapsed > 6*time.Second { + t.Fatalf("hanging launch check took %s, want at most 6s", elapsed) + } + if show || answer != (Available{}) { + t.Fatalf("hanging launch check returned %+v, show %t", answer, show) + } +} + +// TestTimeoutContractC3StallAndCeilingPreserveTheOriginal proves C3 with two +// real response bodies: one sends no bytes and one never stops trickling them. +func TestTimeoutContractC3StallAndCeilingPreserveTheOriginal(t *testing.T) { + assetName := "codeaf-" + runtime.GOOS + "-" + runtime.GOARCH + if got, want := downloadStallError(assetName, downloadStallWindow).Error(), "downloading "+assetName+" stalled — no bytes for 30 s"; got != want { + t.Fatalf("default stall sentence = %q, want %q", got, want) + } + if got, want := downloadCeilingError(assetName, downloadCeiling).Error(), "downloading "+assetName+" took longer than 15 minutes"; got != want { + t.Fatalf("default ceiling sentence = %q, want %q", got, want) + } + + for _, row := range []struct { + name string + stall time.Duration + ceiling time.Duration + serve func(http.ResponseWriter, *http.Request) + want string + upperMargin time.Duration + }{ + { + name: "stalled body", stall: 60 * time.Millisecond, ceiling: 2 * time.Second, + serve: func(w http.ResponseWriter, request *http.Request) { + w.WriteHeader(http.StatusOK) + w.(http.Flusher).Flush() + <-request.Context().Done() + }, + want: downloadStallError(assetName, 60*time.Millisecond).Error(), upperMargin: time.Second, + }, + { + name: "endless trickle", stall: 100 * time.Millisecond, ceiling: 140 * time.Millisecond, + serve: func(w http.ResponseWriter, request *http.Request) { + w.WriteHeader(http.StatusOK) + flusher := w.(http.Flusher) + flusher.Flush() + ticker := time.NewTicker(15 * time.Millisecond) + defer ticker.Stop() + for { + select { + case <-request.Context().Done(): + return + case <-ticker.C: + if _, err := w.Write([]byte("x")); err != nil { + return + } + flusher.Flush() + } + } + }, + want: downloadCeilingError(assetName, 140*time.Millisecond).Error(), upperMargin: time.Second, + }, + } { + t.Run(row.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, request *http.Request) { + if strings.HasSuffix(request.URL.Path, "/"+assetName) { + row.serve(w, request) + return + } + http.NotFound(w, request) + })) + defer server.Close() + dir := t.TempDir() + target := filepath.Join(dir, "codeaf") + original := []byte("original codeaf") + if err := os.WriteFile(target, original, 0o700); err != nil { + t.Fatal(err) + } + client := releaseClient(server, "v0.1.0") + client.StallWindow = row.stall + client.DownloadCeiling = row.ceiling + started := time.Now() + _, err := Install(context.Background(), InstallOptions{ + Client: client, Release: Release{Tag: "v0.2.0", Repository: primaryRepository}, Target: target, + }) + if err == nil || err.Error() != row.want { + t.Fatalf("error = %v, want %q", err, row.want) + } + if elapsed := time.Since(started); elapsed > row.upperMargin { + t.Fatalf("failure took %s, want at most %s", elapsed, row.upperMargin) + } + installed, readErr := os.ReadFile(target) + if readErr != nil || string(installed) != string(original) { + t.Fatalf("target = %q, error %v; want untouched original", installed, readErr) + } + info, statErr := os.Stat(target) + if statErr != nil { + t.Fatal(statErr) + } + if info.Mode().Perm() != 0o700 { + t.Fatalf("original mode = %v, want 0700", info.Mode().Perm()) + } + if matches, _ := filepath.Glob(filepath.Join(dir, ".codeaf.tmp.*")); len(matches) != 0 { + t.Fatalf("temporary files remain: %v", matches) + } + }) + } +} + +// TestTimeoutContractC4HangingReleaseAPINamesTheAPI proves C4 with an injected +// API window, so the test exercises the real deadline without waiting ten seconds. +func TestTimeoutContractC4HangingReleaseAPINamesTheAPI(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, request *http.Request) { + <-request.Context().Done() + })) + defer server.Close() + client := releaseClient(server, "v0.1.0") + client.APIWindow = 60 * time.Millisecond + started := time.Now() + _, err := client.Select(context.Background(), Choice{Channel: "stable"}) + if err == nil || err.Error() != "release API did not answer within 60 ms" { + t.Fatalf("error = %v", err) + } + if elapsed := time.Since(started); elapsed > time.Second { + t.Fatalf("hanging API took %s, want at most 1s", elapsed) + } + if strings.Contains(err.Error(), "codeaf-") || strings.Contains(err.Error(), "context deadline exceeded") { + t.Fatalf("API failure named an asset or Go's deadline: %q", err) + } +} + +// TestTimeoutContractD1AndD3DefaultTransportHasNoWholeBodyClock proves the +// default client's transport clocks and the exact delayed-header sentence. +func TestTimeoutContractD1AndD3DefaultTransportHasNoWholeBodyClock(t *testing.T) { + client := NewClient("v0.1.0", CheckTimeout) + if client.HTTP.Timeout != 0 { + t.Fatalf("Client.Timeout = %s, want zero", client.HTTP.Timeout) + } + transport, ok := client.HTTP.Transport.(*http.Transport) + if !ok { + t.Fatalf("transport = %T, want *http.Transport", client.HTTP.Transport) + } + if transport.TLSHandshakeTimeout != networkSetupTimeout || transport.ResponseHeaderTimeout != apiRequestTimeout { + t.Fatalf("transport TLS %s header %s", transport.TLSHandshakeTimeout, transport.ResponseHeaderTimeout) + } + + server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, request *http.Request) { + <-request.Context().Done() + })) + defer server.Close() + testTransport := server.Client().Transport.(*http.Transport).Clone() + testTransport.ResponseHeaderTimeout = 50 * time.Millisecond + client.HTTP = &http.Client{Transport: testTransport} + client.DownloadBase = server.URL + client.DownloadCeiling = time.Second + target := filepath.Join(t.TempDir(), "codeaf") + if err := os.WriteFile(target, []byte("old"), 0o755); err != nil { + t.Fatal(err) + } + _, err := Install(context.Background(), InstallOptions{ + Client: client, Release: Release{Tag: "v0.2.0", Repository: primaryRepository}, Target: target, + }) + want := assetHeaderTimeoutSentence(runtime.GOOS, runtime.GOARCH) + if err == nil || err.Error() != want { + t.Fatalf("error = %v, want %q", err, want) + } +} + +func assetHeaderTimeoutSentence(goos, goarch string) string { + return fmt.Sprintf("codeaf-%s-%s did not start arriving within 10 s", goos, goarch) +} + // TestC7CheckedInstallReplacesAtomicallyAndMismatchPreservesTheOriginal proves C7. func TestC7CheckedInstallReplacesAtomicallyAndMismatchPreservesTheOriginal(t *testing.T) { asset := []byte("new codeaf") From 3a224542e5252f8ab838e921e4840edd8e99c029 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Thu, 17 Sep 2026 13:52:57 -0400 Subject: [PATCH 2/3] update: both doors read the one check constant `codeaf update` and the chat surface each spelled `3*time.Second` when they built the release client, which is how a figure meant for one small request came to govern an install. Both now pass update.CheckTimeout, and the three seconds live in exactly one place. The terminal's `--check` takes the client's Check, which is the whole-exchange check deadline; a plain `codeaf update` keeps Select, so the metadata requests an install makes get the ten seconds they need. Co-Authored-By: Claude Fable 5.1 --- cmd/codeaf/chatv3_surface.go | 3 +- cmd/codeaf/update.go | 11 +++++-- cmd/codeaf/update_test.go | 62 ++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 5 deletions(-) diff --git a/cmd/codeaf/chatv3_surface.go b/cmd/codeaf/chatv3_surface.go index 0d7c620fc7..0d66069333 100644 --- a/cmd/codeaf/chatv3_surface.go +++ b/cmd/codeaf/chatv3_surface.go @@ -4,7 +4,6 @@ import ( "context" "log" "os" - "time" "github.com/Agent-Field/codeaf/internal/buildinfo" internalenv "github.com/Agent-Field/codeaf/internal/env" @@ -44,7 +43,7 @@ var ( // reads this package's sources rather than trusting the next door to remember. func runSurface(ctx context.Context, options tui3.Options) error { revision := surfaceRevision() - client := surfaceUpdateClient(revision, 3*time.Second) + client := surfaceUpdateClient(revision, codeupdate.CheckTimeout) restart := options.Restart if restart == nil { restart = &codeupdate.Plan{} diff --git a/cmd/codeaf/update.go b/cmd/codeaf/update.go index 0906efe9a2..c309fd320e 100644 --- a/cmd/codeaf/update.go +++ b/cmd/codeaf/update.go @@ -7,7 +7,6 @@ import ( "os" "os/exec" "strings" - "time" "github.com/Agent-Field/codeaf/internal/buildinfo" codeupdate "github.com/Agent-Field/codeaf/internal/update" @@ -72,8 +71,14 @@ func runUpdate(args []string) error { return exitStatus(2) } } - client := updateClient(running, 3*time.Second) - release, err := client.Select(context.Background(), choice) + client := updateClient(running, codeupdate.CheckTimeout) + var release codeupdate.Release + var err error + if *check { + release, err = client.Check(context.Background(), choice) + } else { + release, err = client.Select(context.Background(), choice) + } if err != nil { if *check { fmt.Fprintln(updateErr, "codeaf: could not check for an update:", err) diff --git a/cmd/codeaf/update_test.go b/cmd/codeaf/update_test.go index 69d3d77464..8fadd4f56f 100644 --- a/cmd/codeaf/update_test.go +++ b/cmd/codeaf/update_test.go @@ -77,6 +77,68 @@ func TestC9UpdateCheckUsesTheRealDoorAndItsThreeExitCodes(t *testing.T) { } } +// TestTimeoutContractC2HangingTerminalCheckExitsInsideItsBudget proves C2 for +// the real `codeaf update --check` door and its exit-1 failure road. +func TestTimeoutContractC2HangingTerminalCheckExitsInsideItsBudget(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, request *http.Request) { + <-request.Context().Done() + })) + defer server.Close() + client := &codeupdate.Client{HTTP: server.Client(), APIBase: server.URL, DownloadBase: server.URL} + stdout, stderr := withUpdateDoor(t, "v0.1.0", client, filepath.Join(t.TempDir(), "codeaf")) + started := time.Now() + if got := updateExit(runUpdate([]string{"--check"})); got != 1 { + t.Fatalf("exit = %d, want 1; stdout %q stderr %q", got, stdout.String(), stderr.String()) + } + if elapsed := time.Since(started); elapsed > 6*time.Second { + t.Fatalf("hanging terminal check took %s, want at most 6s", elapsed) + } + if stdout.Len() != 0 || !strings.Contains(stderr.String(), "codeaf: could not check for an update: release API did not answer within 3 s") { + t.Fatalf("stdout %q stderr %q", stdout.String(), stderr.String()) + } + if strings.Contains(stderr.String(), "context deadline exceeded") || strings.Contains(stderr.String(), "Client.Timeout") { + t.Fatalf("terminal check exposed Go's timeout: %q", stderr.String()) + } +} + +// TestTimeoutContractC4HangingInstallSelectionNamesTheAPI proves C4 through +// the terminal install door with a short injected API window. +func TestTimeoutContractC4HangingInstallSelectionNamesTheAPI(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, request *http.Request) { + <-request.Context().Done() + })) + defer server.Close() + dir := t.TempDir() + target := filepath.Join(dir, "codeaf") + original := []byte("original codeaf") + if err := os.WriteFile(target, original, 0o700); err != nil { + t.Fatal(err) + } + client := &codeupdate.Client{ + HTTP: server.Client(), APIBase: server.URL, DownloadBase: server.URL, + APIWindow: 60 * time.Millisecond, + } + _, _ = withUpdateDoor(t, "v0.1.0", client, target) + started := time.Now() + err := runUpdate(nil) + if err == nil || !strings.Contains(err.Error(), "could not select a release: release API did not answer within 60 ms") { + t.Fatalf("error = %v", err) + } + if elapsed := time.Since(started); elapsed > time.Second { + t.Fatalf("hanging install selection took %s, want at most 1s", elapsed) + } + if strings.Contains(err.Error(), "codeaf-") || strings.Contains(err.Error(), "context deadline exceeded") { + t.Fatalf("API failure named an asset or Go's deadline: %q", err) + } + installed, readErr := os.ReadFile(target) + if readErr != nil || string(installed) != string(original) { + t.Fatalf("target = %q, error %v; want untouched original", installed, readErr) + } + if matches, _ := filepath.Glob(filepath.Join(dir, ".codeaf.tmp.*")); len(matches) != 0 { + t.Fatalf("temporary files remain: %v", matches) + } +} + // TestC9bUpdateCheckNamesTheSelectedTagAndUsesItsChannelRules proves C9b. func TestC9bUpdateCheckNamesTheSelectedTagAndUsesItsChannelRules(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From 4ee3c170768973ee9341c64aa6d9fa9af1de3a56 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Thu, 17 Sep 2026 13:53:46 -0400 Subject: [PATCH 3/3] docs/changes: the update download's own clock (#1110) Co-Authored-By: Claude Fable 5.1 --- .../unreleased/1110-update-download-timeout.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 docs/changes/unreleased/1110-update-download-timeout.md diff --git a/docs/changes/unreleased/1110-update-download-timeout.md b/docs/changes/unreleased/1110-update-download-timeout.md new file mode 100644 index 0000000000..513b864c8c --- /dev/null +++ b/docs/changes/unreleased/1110-update-download-timeout.md @@ -0,0 +1,17 @@ +--- +kind: fixed +title: a release download finishes over a real network, on its own clock +pr: 1110 +surface: [chat, build] +invalidates: + - "#1060's `/update` and `codeaf update` were believed to install a release over any working network. They could not finish one at all: both doors built the release client with the three-second budget meant for the launch check, and Go's `http.Client.Timeout` covers the whole exchange including every body byte, so the 57,532,578-byte `codeaf-linux-amd64` — about eight seconds on a healthy link — was cut off three seconds in with `read the release response: context deadline exceeded`. `--check` passed everywhere because its answer is a few KB, and every hand test served the asset from a local mirror in milliseconds." + - "The release client no longer sets `http.Client.Timeout` at all, and there is no longer one clock over the whole road. Its transport bounds only what has made no progress — a five-second dial, a five-second TLS handshake, a ten-second wait for response headers — and every call takes its deadline from its context: the launch check and `codeaf update --check` keep a three-second whole-exchange deadline from the single `update.CheckTimeout` constant that both doors read, the release metadata an install reads gets ten seconds per request, and the asset download gets a thirty-second no-byte stall window plus a fifteen-minute ceiling. A dead link still fails in seconds; a slow one now succeeds." + - "A failed update could hand a person Go's own deadline text. It cannot now: a stall says `downloading codeaf-linux-amd64 stalled — no bytes for 30 s`, the ceiling says `downloading codeaf-linux-amd64 took longer than 15 minutes`, late headers say `codeaf-linux-amd64 did not start arriving within 10 s`, and a release request that does not answer names the release API and its window. The terminal road still ends with the curl line and the chat still offers it." +--- + +A contract item about time has to be proved with a slow server. Every hand test +behind #1060 downloaded the asset from a local mirror, which serves 57 MB in +milliseconds, so the one clock that mattered was the one nothing ever ran. The +tests here inject the stall window and the ceiling and then use real slow, +silent and trickling servers, and the two roads were walked again against the +published v0.2.0 release.