Fix skill.stack-auth.com CDN serving HTML to curl#1452
Conversation
Add Vary: Sec-Fetch-Mode, Sec-Fetch-Dest so Vercel caches the markdown response (agents/curl) separately from the HTML landing page (browsers). Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR fixes incorrect CDN caching behavior on skill.stack-auth.com by ensuring the route’s HTML-vs-markdown content negotiation is reflected in the CDN cache key.
Changes:
- Adds a
Vary: Sec-Fetch-Mode, Sec-Fetch-Destresponse header so CDN caches the HTML (browser navigation) and markdown (curl/agents) variants separately.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryThis PR adds
Confidence Score: 4/5Safe to merge — the change is a one-line header addition that correctly reflects the existing content-negotiation logic. The two header names in Vary exactly mirror the two conditions checked in wantsHtml(), so the cache partitioning is logically consistent. The only open question is whether Vercel's CDN edge actually uses Sec-Fetch-* request headers as cache-key discriminators; some CDN layers strip browser-metadata headers before consulting the cache. If they are stripped, the Vary would be present in responses but have no effect on CDN behavior, leaving the original problem unresolved. apps/skills/src/app/route.ts — worth verifying that Vercel's edge cache honours Sec-Fetch-* as Vary dimensions after deploy. Important Files Changed
Sequence DiagramsequenceDiagram
participant B as Browser
participant C as curl/Agent
participant CDN as Vercel CDN
participant O as Origin
B->>CDN: GET / with Sec-Fetch-Mode navigate
CDN->>O: Cache miss — forward
O-->>CDN: 200 HTML + Vary Sec-Fetch-Mode Sec-Fetch-Dest
CDN-->>B: HTML cached under navigate key
C->>CDN: GET / no Sec-Fetch headers
CDN->>O: Cache miss different Vary key
O-->>CDN: 200 Markdown + Vary Sec-Fetch-Mode Sec-Fetch-Dest
CDN-->>C: Markdown cached under absent key
B->>CDN: GET / with Sec-Fetch-Mode navigate
CDN-->>B: HTML from cache
C->>CDN: GET / no Sec-Fetch headers
CDN-->>C: Markdown from cache
Reviews (1): Last reviewed commit: "Fix skill.stack-auth.com CDN serving HTM..." | Re-trigger Greptile |
📝 WalkthroughWalkthroughAdded a ChangesCache Header Configuration
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/skills/src/app/route.ts`:
- Around line 213-214: Remove the manually-set "Vary" header (the "Vary":
"Sec-Fetch-Mode, Sec-Fetch-Dest" entry) from the route handler in
apps/skills/src/app/route.ts because Next.js 16 will overwrite it; instead
either add segment-level caching (e.g., export const revalidate = <seconds>) to
let Next.js manage cache variants, or move variant caching to your
CDN/infrastructure (Cloudflare/Nginx) as described in the review comment; also
remove any manual Cache-Control headers in this handler so Next.js can control
caching via segment exports and cache-tag mechanisms.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 74552292-4ab6-48d3-a226-d81d00fd1ece
📒 Files selected for processing (1)
apps/skills/src/app/route.ts
## Summary Follow-up to #1452. `Sec-Fetch-Mode` / `Sec-Fetch-Dest` didn't reliably split HTML vs. markdown at the CDN edge, so curl could still get the HTML landing page. Switch to the `Accept` header: - Browsers send `Accept: text/html,...` on top-level navigations. - `curl`, `fetch()`, and agent fetchers send `*/*` or omit `Accept`. - Serve HTML only when `text/html` is explicitly listed; everything else gets `SKILL.md`. - `Vary` updated to `Accept` to match. ## Test plan - [ ] Deploy preview - [ ] `curl -sSL https://skill.stack-auth.com/ | head -3` returns markdown frontmatter - [ ] Browser load of `https://skill.stack-auth.com/` still shows the HTML landing page - [ ] Purge Vercel cache if stale variants persist <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved content format negotiation for skill resources to correctly serve HTML or markdown based on client requests. * **Chores** * Optimized caching behavior for edge and CDN services to enhance global content distribution efficiency. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/hexclave/stack-auth/pull/1454?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Vary: Sec-Fetch-Mode, Sec-Fetch-Deston the skills app root route so the CDN caches markdown and HTML responses separately.curl https://skill.stack-auth.com/could return the browser HTML landing page (cached from aSec-Fetch-Mode: navigaterequest) instead of the canonicalSKILL.mdbody.Context
The route already content-negotiates: browsers with
Sec-Fetch-Mode: navigateget HTML;curland agents get markdown. WithoutVary, Vercel served a single cached variant to all clients.Test plan
curl -sSL https://skill.stack-auth.com/ | head -3returns markdown (---frontmatter), not<!doctype html>https://skill.stack-auth.com/in a browser — still shows the HTML landing pageMade with Cursor
Summary by CodeRabbit