Skip to content

fix(copilot): propagate parent context instead of using context.Background() in internal functions #52

@jongio

Description

@jongio

Summary

Several functions create fresh context.Background() contexts internally instead of accepting a parent context parameter, preventing callers from propagating cancellation or deadlines.

Current Pattern

internal/update/check.go (line 170):

func fetchLatestVersion() (string, error) {
    client := newSecureClient(apiTimeout)
    req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, latestReleaseURL, nil)
    // ...
}

internal/update/update.go (lines 188, 225):

req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, url, nil)
// ...
req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, checksumURL, nil)

internal/tui/components/reindex.go (line 41):

ctx, cancel := context.WithCancel(context.Background())

internal/copilot/client.go (lines 300, 363, 422, 499, 555, 609):
Multiple operations create context.WithTimeout(context.Background(), ...) instead of deriving from a caller-supplied context.

Idiomatic Replacement

Accept ctx context.Context as the first parameter per Go convention, and derive child contexts from it:

func fetchLatestVersion(ctx context.Context) (string, error) {
    client := newSecureClient(apiTimeout)
    req, err := http.NewRequestWithContext(ctx, http.MethodGet, latestReleaseURL, nil)
    // ...
}

For cases where a timeout is needed, derive from the parent:

initCtx, initCancel := context.WithTimeout(ctx, searchOperationTimeout)

Why It Matters

  • Using context.Background() in internal functions breaks the cancellation chain — if the application is shutting down, these HTTP requests and operations continue running until their own timeout expires
  • Go convention: "Incoming requests to a server should create a Context, and outgoing calls to servers should accept a Context" (Go blog)
  • The update check and download functions do network I/O that should be cancellable by the caller
  • This also prevents proper tracing/observability since context values are lost

Files Affected

File Lines Issue
internal/update/check.go 170 fetchLatestVersion uses context.Background()
internal/update/update.go 188, 225 download and checksum fetch use context.Background()
internal/copilot/client.go 300, 363, 422, 499, 555, 609 SDK init and search create root contexts
internal/tui/components/reindex.go 41 Reindex creates root context

Metadata

Metadata

Assignees

No one assigned

    Labels

    automatedCreated by automationidiomatic-auditIdiomatic code audit finding

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions