Commit Graph
2 Commits
Author SHA1 Message Date
Cian Johnston 6079c514ee fix: follow-up fixes for conditional VCS requests (#27711)
Follow-ups from #27627 

- Memoizes `Config.Git()` with a mutex so the provider's ETag response
cache survives across calls. Only successful construction is cached;
errors are retried.
- Moves the HTTP client onto `Config.HTTPClient`, wired through
`ConvertConfig`, so `Git()` no longer takes a per-call argument that
would be silently ignored after memoization.
- `newGitHub` and `newGitLab` now return `(Provider, error)`,
eliminating the typed-nil-interface class in `gitprovider.New` rather
than the single instance.
- Gates the 304 branch on a `haveCached` flag instead of a nil body
check.
- Only caches bodies that decode successfully, preventing poisoned
entries.
- Keys the response cache on the full token digest rather than a
truncated prefix.
- Tests added: `TestConfigGitMemoizesProvider`,
`TestConfigGitRetriesOnConstructorError`,
`TestGitLabConstructorErrorReturnsNilInterface`,
`TestResponseCacheStore`,
`TestConditionalRequestReuse/MalformedResponseNotCached`;
`TestConvertYAML/CustomScopesAndEndpoint` now asserts
`Config.HTTPClient` wiring.

Follow-ups tracked in #28139, #28140, #28141, #28142.

> 🤖 Generated by Coder Agents on behalf of @johnstcn.
2026-08-18 09:00:20 +01:00
Josh FreeandCopilot 2acfe7e829 feat(coderd/externalauth/gitprovider): use conditional requests for GitHub JSON reads (#27628)
Fixes #27627.

## What

The chat diff-status gitsync worker polls open pull requests on a fixed
10s interval and re-downloads the full JSON body every tick, even when
nothing changed, because the GitHub client never sends `If-None-Match` /
ETag.

This adds a small, concurrency-safe, bounded in-memory ETag+body cache
(`coderd/externalauth/gitprovider/conditional.go`) and wires it into
`githubProvider.decodeJSON`. When an ETag is cached for a request, we
send `If-None-Match`; on `304 Not Modified` we decode the cached body;
on `200` we cache `{etag, body}` when an ETag is present and the body is
under a size cap.

## Why

`304` responses do not count against GitHub's primary rate limit, but
full `200`s do. Today every unchanged poll burns quota that the same
token also needs for interactive Git and API operations, so busy
instances can hit rate-limit errors and stalls elsewhere. Unchanged PRs
now revalidate for free with no behavior change; only genuine changes
transfer a body.

## Details

- Cache key = request URL + a hash of the token, so one token's response
is never served under another; raw tokens are not retained.
- Bounded by entry count (LRU eviction, default 2048) and per-body size
(1 MiB) to cap memory.
- Scope limited to the JSON reads through `decodeJSON`; the raw-diff
path (`fetchDiff`, up to `MaxDiffSize`) is intentionally left out to
avoid caching large bodies.

## Tests

`TestConditionalRequestReuse` in `github_test.go` covers:
- `NotModifiedReusesCachedBody` — a warm poll sends `If-None-Match` with
the prior ETag and reuses the cached body on `304`, yielding the same
result with exactly two upstream requests.
- `DifferentTokenDoesNotShareCache` — a different token never sends
another token's cached ETag.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4959e1f9-f8e6-4e97-a487-f395a0123c79
2026-07-30 17:36:39 +01:00