Skip to content

feat(copilot): opt-in HTTP cache for the Node fetch fetcher#317721

Merged
deepak1556 merged 5 commits into
mainfrom
robo/node_fetch_cache
May 21, 2026
Merged

feat(copilot): opt-in HTTP cache for the Node fetch fetcher#317721
deepak1556 merged 5 commits into
mainfrom
robo/node_fetch_cache

Conversation

@deepak1556
Copy link
Copy Markdown
Collaborator

Adds an opportunistic cache support to the Copilot Node fetch path. The cache is strictly opt-in per request and composes with the existing VSCode proxy and CA-injection patch.

  • __vscodeCreateFetchPatch({ interceptors }) lets the extension host build a second proxy-aware fetch with extra undici interceptors. The default __vscodePatchedFetch is unchanged.
  • NodeFetchFetcher builds an undici cache interceptor once at construction time and uses the factory to produce a cachedFetch that routes through both the proxy patch and the cache. Requests are tagged with an internal __copilotCachePatch marker (stripped before fetch); unmarked requests keep going through the regular patched fetch. When the host lacks the factory, caching is silently disabled so requests never bypass the proxy patch.
  • FetchOptions.cache?: boolean — opportunistic hint. Fetchers without cache support ignore it; fallback to other fetchers is unaffected.
  • Response.cacheStatus and FetchTelemetryEvent.cacheStatus: 'hit' | 'stale-hit' | 'revalidated' | 'miss' | 'bypass'.
  • New setting github.copilot.advanced.debug.nodeFetchCache: 'off' | 'memory' | 'persistent' (default 'memory'). 'persistent' uses undici's SQLite store under the extension's global storage (undici-cache.v1.sqlite) when available, otherwise falls back to memory.
  • New taggedCacheInterceptor wraps undici.interceptors.cache and stamps a private VSCODE_CACHE_STATUS_HEADER on the response so the base fetcher can read the outcome without parsing undici internals.
  • BaseFetchFetcher exposes an overridable _buildRequestInit hook and reports cacheStatus on Response and fetchTelemetry.

Notes

For #308310

@deepak1556 deepak1556 added this to the 1.122.0 milestone May 21, 2026
@deepak1556 deepak1556 requested a review from chrmarti May 21, 2026 07:47
@deepak1556 deepak1556 self-assigned this May 21, 2026
Copilot AI review requested due to automatic review settings May 21, 2026 07:47
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds opt-in HTTP caching support to Copilot’s Node fetch path (undici-based) while ensuring cached requests still route through VS Code’s proxy/CA-injection patching, and surfaces cache outcomes via Response.cacheStatus plus fetch telemetry.

Changes:

  • Exposes a host-provided __vscodeCreateFetchPatch({ interceptors }) factory to compose additional undici interceptors on top of the existing proxy-aware fetch patch.
  • Adds an opt-in cache hint (FetchOptions.cache?: boolean) with cache status propagation (Response.cacheStatus, telemetry) and a debug setting to choose cache mode (off/memory/persistent).
  • Introduces a taggedCacheInterceptor wrapper that stamps cache outcome onto responses for downstream attribution.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/vs/workbench/api/node/proxyResolver.ts Exposes a fetch-patch factory on globalThis to allow composing additional undici interceptors while preserving proxy/CA patching.
extensions/copilot/src/platform/telemetry/vscode-node/telemetryServiceImpl.ts Adds cacheStatus to fetch telemetry payload/classification and emits it as metadata.
extensions/copilot/src/platform/networking/vscode-node/test/fetcherServiceCrash.spec.ts Updates tests for the new FetcherService constructor dependency.
extensions/copilot/src/platform/networking/vscode-node/fetcherServiceImpl.ts Wires cache mode setting and persistent store location into NodeFetchFetcher, and forwards cacheStatus into fetch telemetry events.
extensions/copilot/src/platform/networking/node/test/taggedCacheInterceptor.spec.ts Adds unit tests for cache-status tagging and classification behavior.
extensions/copilot/src/platform/networking/node/taggedCacheInterceptor.ts Implements the cache interceptor wrapper that stamps cache outcome onto response headers.
extensions/copilot/src/platform/networking/node/nodeFetchFetcher.ts Adds opt-in cache routing via a request marker and composes the cache interceptor using the host fetch-patch factory.
extensions/copilot/src/platform/networking/node/baseFetchFetcher.ts Adds a request-init hook and reads stamped cache status to populate Response.cacheStatus.
extensions/copilot/src/platform/networking/common/fetcherService.ts Extends fetch options and response/telemetry types with cache-related fields.
extensions/copilot/src/platform/configuration/common/configurationService.ts Adds the advanced.debug.nodeFetchCache setting (off/memory/persistent).
Comments suppressed due to low confidence (2)

extensions/copilot/src/platform/networking/node/taggedCacheInterceptor.ts:86

  • readHeader returns only the first element when the header value is a string[]. For headers like warning this can drop additional warnings and lead to incorrect stale-hit classification. Consider checking all values (and/or joining) when the value is an array.
	if (typeof value === 'string') {
		return value;
	}
	if (Array.isArray(value) && typeof value[0] === 'string') {
		return value[0];

extensions/copilot/src/platform/networking/node/taggedCacheInterceptor.ts:94

  • stampStatus skips stamping when undici supplies headers in raw wire format (string[]). In that case BaseFetchFetcher will always report cacheStatus: 'bypass' even if the cache served the response. Consider supporting the array form by appending the header key/value pair (and/or normalizing to an object) before forwarding to the downstream handler.
function stampStatus(headers: unknown, status: CacheStatus): void {
	if (headers && typeof headers === 'object' && !Array.isArray(headers)) {
		(headers as Record<string, string>)[VSCODE_CACHE_STATUS_HEADER] = status;
	}

Comment thread extensions/copilot/src/platform/networking/node/nodeFetchFetcher.ts
@deepak1556 deepak1556 force-pushed the robo/node_fetch_cache branch 3 times, most recently from c097dad to 85c8868 Compare May 21, 2026 09:55
@deepak1556 deepak1556 marked this pull request as ready for review May 21, 2026 09:55
@deepak1556 deepak1556 marked this pull request as draft May 21, 2026 14:36
Adds an opportunistic cache support to the Copilot Node fetch path. The cache
is strictly opt-in per request and composes with the existing VSCode proxy
and CA-injection patch.

- `__vscodeCreateFetchPatch({ interceptors })` lets the extension host
  build a second proxy-aware `fetch` with extra undici interceptors. The
  default `__vscodePatchedFetch` is unchanged.
- `NodeFetchFetcher` builds an undici cache interceptor once at
  construction time and uses the factory to produce a `cachedFetch` that
  routes through both the proxy patch and the cache. Requests are tagged
  with an internal `__copilotCachePatch` marker (stripped before fetch);
  unmarked requests keep going through the regular patched fetch. When
  the host lacks the factory, caching is silently disabled so requests
  never bypass the proxy patch.
- `FetchOptions.cache?: boolean` — opportunistic hint. Fetchers without
  cache support ignore it; fallback to other fetchers is unaffected.
- `Response.cacheStatus` and `FetchTelemetryEvent.cacheStatus`:
  `'hit' | 'stale-hit' | 'revalidated' | 'miss' | 'bypass'`.
- New setting `github.copilot.advanced.debug.nodeFetchCache`:
  `'off' | 'memory' | 'persistent'` (default `'memory'`). `'persistent'`
  uses undici's SQLite store under the extension's global storage
  (`undici-cache.v1.sqlite`) when available, otherwise falls back to
  memory.
- New `taggedCacheInterceptor` wraps `undici.interceptors.cache` and
  stamps a private `VSCODE_CACHE_STATUS_HEADER` on the response so the
  base fetcher can read the outcome without parsing undici internals.
- `BaseFetchFetcher` exposes an overridable `_buildRequestInit` hook and
  reports `cacheStatus` on `Response` and `fetchTelemetry`.

Notes
- No behavior change for callers that don't set `cache: true`.
- The cache interceptor is constructed once per fetcher instance; the
  composed dispatcher chain is reused so connection pooling is preserved.
- Depends on microsoft/vscode-proxy-agent#100

For #308310
- drop the `age` header gate from classify(): undici's cache interceptor
only adds if-modified-since / if-none-match when revalidating a stored
entry, so `state.conditional` alone is a sufficient signal. The age header
is not guaranteed on a revalidated 200, which caused 'revalidated' to be
misreported as 'miss'.

- the etag integration test used
`cache-control: max-age=0, must-revalidate`, which undici treats as
already-stale on arrival and refuses to store (cache-handler.js bails when
`now >= absoluteStaleAt`), so there was nothing to revalidate on the second
call. Switch the origin to `public, max-age=60` and pass
`cache-control: no-cache` on the second request to drive undici's
needsRevalidation() path, which dispatches with if-none-match and serves
the cached body on 304.
@deepak1556 deepak1556 force-pushed the robo/node_fetch_cache branch from 85c8868 to cd23554 Compare May 21, 2026 15:15
@deepak1556 deepak1556 marked this pull request as ready for review May 21, 2026 15:38
chrmarti
chrmarti previously approved these changes May 21, 2026
Copy link
Copy Markdown
Collaborator

@chrmarti chrmarti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@deepak1556 deepak1556 enabled auto-merge (squash) May 21, 2026 16:14
@deepak1556 deepak1556 merged commit 9cd7264 into main May 21, 2026
39 of 40 checks passed
@deepak1556 deepak1556 deleted the robo/node_fetch_cache branch May 21, 2026 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants