feat: tiered cache headers#958
Conversation
PR SummaryMedium Risk Overview Immutable applies to versioned release assets; mutable applies to directory listings, Reviewed by Cursor Bugbot for commit 82a6785. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Pull request overview
Implements tiered Cache-Control policies to better reflect which served resources are immutable (versioned release assets) vs mutable (indexes, directory listings, moving latest* aliases), addressing issue #160.
Changes:
- Introduces centralized cache decision logic (
isImmutablePath,cacheControlFor) and corresponding unit tests. - Applies cache policy based on the original (unsubstituted) request URL in both R2 and origin fetch paths.
- Replaces the single
successcache header with explicitimmutable/mutablepolicies and updates e2e expectations.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/request.ts | Adds getOriginalUrl() helper to consistently use the unsubstituted URL when present. |
| src/utils/cache.ts | Adds path/status-based cache policy selection for immutable vs mutable resources. |
| src/utils/cache.test.ts | Unit tests for cache policy/path classification logic. |
| src/providers/r2Provider.ts | Removes provider-owned cache-control policy and defers to middleware. |
| src/middleware/r2Middleware.ts | Sets cache-control per response using original URL + status; updates directory listing caching. |
| src/middleware/originMiddleware.ts | Applies tiered cache-control to origin responses using original URL pathname. |
| src/constants/cache.ts | Replaces success with immutable / mutable cache header strings. |
| e2e-tests/file.test.ts | Updates assertions for new mutable cache policy on index.json. |
| e2e-tests/fallback.test.ts | Updates assertions for new mutable cache policy on fallback-served mutable resources. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!hasTrailingSlash(request.urlObj.pathname)) { | ||
| // We always want directory listing requests to have a trailing slash | ||
| const url = request.unsubstitutedUrl ?? request.urlObj; | ||
| const url = getOriginalUrl(request); | ||
| return Response.redirect(`${url}/`, 301); | ||
| } |
| // Set by R2Middleware, which has access to the original request URL needed | ||
| // to decide between immutable/mutable/failure cache policies. | ||
| 'cache-control': '', |
| * Only fully-successful (200) responses are given a long-lived cache policy. | ||
| * Conditional/range responses (206/304/412) and errors keep the `failure` | ||
| * policy so they're always re-validated. | ||
| */ |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 82a6785. Configure here.
|
|
||
| let responseBody; | ||
| if (request.method === 'GET') { | ||
| responseBody = renderDirectoryListing( |
There was a problem hiding this comment.
Directory index mutable cache
Low Severity
When a directory request is satisfied by serving index.html instead of an HTML listing, setCacheControl still uses the trailing-slash pathname. isImmutablePath then treats the response as a mutable directory URL, so versioned static index files can miss the long-lived immutable cache policy they would get at the same path with index.html in the URL.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 82a6785. Configure here.
Fixes #160