fix: match skip prefixes on a path boundary#83
Conversation
cloudflare, netlify and vercel tested skip prefixes with a bare startsWith, so with the default ["/admin", "/api/", "/_"] a real page like /administrator matched /admin and was silently excluded from negotiation, its markdown twin, and the Link header. Use the same boundary-aware check as the deno and fastly adapters: match the exact path or a prefix/ subpath (a prefix already ending in / still matches via startsWith).
|
@abhay-codes07 is attempting to deploy a commit to the Dodo Payments Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
This PR fixes an over-broad skip-prefix match in the Cloudflare, Netlify, and Vercel adapters by making skip-prefix checks path-boundary-aware (matching deno/fastly behavior), preventing legitimate pages like /administrator from being skipped.
Changes:
- Updated
shouldSkipin Cloudflare/Netlify/Vercel to only match prefixes on an exact path orprefix/boundary (while preservingprefix/behavior for prefixes ending with/). - Added regression tests in each adapter ensuring
/administratoris negotiated to its markdown twin for bot UAs. - Added a patch changeset for the three affected packages.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/vercel/src/middleware.ts | Updates shouldSkip to enforce path-boundary prefix matching. |
| packages/vercel/test/middleware.test.ts | Adds regression test ensuring /administrator is not skipped and serves markdown to bot UA. |
| packages/netlify/src/worker.ts | Updates shouldSkip to enforce path-boundary prefix matching. |
| packages/netlify/test/worker.test.ts | Adds regression test ensuring /administrator is not skipped and serves markdown to bot UA. |
| packages/cloudflare/src/worker.ts | Updates shouldSkip to enforce path-boundary prefix matching. |
| packages/cloudflare/test/worker.test.ts | Adds regression test ensuring /administrator is not skipped and serves markdown to bot UA. |
| .changeset/skip-prefix-boundary.md | Records a patch release note for the three adapters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![dodo-squirrels[bot]](https://avatars.githubusercontent.com/in/3728163?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: fix: match skip prefixes on a path boundary
Verdict: LGTM — approving. One pre-existing edge worth being aware of, called out below (non-blocking).
Reviewed end-to-end and exercised the shouldSkip semantics against the default prefixes.
Root cause (confirmed on main)
cloudflare / netlify / vercel matched skip prefixes with a bare pathname.startsWith(p). With the default ["/admin", "/api/", "/_"], a real page like /administrator or /admins matched /admin and was silently excluded from negotiation, its .md twin, and the Link header. Verified the boundary cases:
| Path | main | fix | |
|---|---|---|---|
/admin |
skip | skip | ✓ exact |
/admin/users |
skip | skip | ✓ subpath |
/administrator |
skip | not skipped | ✓ the fix |
/admins |
skip | not skipped | ✓ the fix |
/api/v1 |
skip | skip | ✓ (/api/ ends in /) |
/apiary |
not skipped | not skipped | ✓ |
Consistency with reference adapters — verified
The changeset says deno + fastly already use this boundary check. I confirmed: both ship the identical shouldSkip (exact match → endsWith("/") → startsWith(p + "/")) and the same DEFAULT_SKIP_PREFIXES = ["/admin", "/api/", "/_"]. So this PR brings the three lagging adapters to parity with the reference implementations rather than inventing new behavior.
Pre-existing edge (non-blocking, for awareness)
The /_ default no longer matches /_next/image or /_next/data/... without a file extension, because /_ (no trailing slash) now only matches /_ exactly or /_/…, and Next.js uses /_next/…. Extension-bearing assets (/_next/static/x.js) are still covered by the extension list. This is not a regression this PR introduces — deno/fastly already behaved this way with the same default — but if catching all /_next/* internals matters, the default prefix arguably wants to be /_next (or /_/ semantics revisited) in a follow-up across all six adapters. Out of scope here.
Tests
- Each of the three adapters adds the
/administratorregression test. Suites green locally: cloudflare 24, netlify 28, vercel 29.
Correct, consistent with the reference adapters, changeset covers the three packages. Approving.
Summary
cloudflare, netlify and vercel matched skip prefixes with a bare
startsWith, so with the default["/admin", "/api/", "/_"]a real page like/administratormatched/adminand was silently excluded from negotiation, its markdown twin, and the Link header. This adopts the same boundary-aware check the deno and fastly adapters already use.Closes #82.
Changes
shouldSkip: a prefix matches only the exact path or aprefix/subpath (a prefix already ending in/, like/api/, still matches viastartsWith). Asset paths under/_remain covered by the extension list./administratorwith a bot UA and a/administrator.mdtwin is now negotiated to markdown instead of skipped.Behavior for
/admin,/admin/...,/api/..., and/_next/*.jsis unchanged.Type
AEO_SPEC_VERSION)Verification
bun run buildpassesbun run testpasses (cloudflare 24, netlify 28, vercel 29)bun run typecheckpassesdualmark verify(no examples touched)/spec/: ran sync-spec (no spec files touched)Changeset
cc @thepushkaraj @aagarwal1012. This just brings cloudflare/netlify/vercel in line with the deno/fastly
shouldSkip.