Repository navigation
fix(seo): stop marketing pages serving every deep path with a 200 - #3723
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@claude review |
There was a problem hiding this comment.
Claude Code Review
No review was started: this request came from a bot account. Manual reviews can only be requested by someone with write access to this repository. Ask a maintainer to comment @claude review, or have your automation post the comment from a user account with write access.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Looks good — small, well-scoped SEO fix that mirrors existing patterns in this codebase.
What was reviewed: the new isMarketing && slug.length > 0 guard in both page.tsx and generateMetadata (consistent with MARKETING_SECTIONS in lib/section-registry.ts, no hardcoded slugs); the new localeDoubledPaths202609 redirect array follows the same append-to-permanentRedirects pattern as prior entries like alertsMoveToObservability202608; confirmed next.config.mjs wires permanentRedirects into redirects() so the /kr/kr and /cn/cn (and .md variant) redirects take effect before the .md rewrite pipeline.
Extended reasoning...
Overview
The diff touches two files: app/[section]/[[...slug]]/page.tsx and lib/redirects.js. It adds a notFound() guard (and matching generateMetadata short-circuit) for marketing sections that receive a non-empty slug, fixing a bug where every deep path under a marketing page (e.g. /kr/anything) rendered the parent page with a 200 instead of 404ing. It also adds four permanent redirects to preserve the two already-indexed duplicate URLs (/kr/kr, /cn/cn, plus .md variants) rather than dropping that traffic to a 404.
Security risks
None. This is routing/SEO logic only — no auth, no data handling, no user input beyond URL path segments that were already reachable and rendered (just incorrectly, with a 200 instead of 404). The redirect destinations are static, hardcoded strings, not derived from user input, so there's no open-redirect concern.
Level of scrutiny
Low-to-moderate is appropriate here given the change is small, self-contained, and additive (a guard clause plus a redirect array), but I still traced through lib/section-registry.ts to confirm MARKETING_SECTIONS/MARKETING_SLUGS are used consistently rather than duplicated/hardcoded, and checked next.config.mjs to confirm permanentRedirects from lib/redirects.js is actually wired into the redirects() config and runs before the .md rewrite logic (Next.js redirects execute before rewrites), so the .md redirect entries will actually fire as intended.
Other factors
The change follows established repository conventions exactly (redirect arrays appended to permanentRedirects, guard logic keyed off the section registry's single source of truth) and the PR author's own verification notes (dev-server curl checks across all 24 marketing pages, confirming no regressions on non-marketing deep paths, and a build/redirect-count sanity check) are consistent with what the code does. No CODEOWNERS or nested CLAUDE.md concerns apply to these two files. No test suite regression risk since this is a routing guard with an existing generateStaticParams that already only generates the single marketing param (no test flakiness expected).
Marketing entries are single pages, and the `[section]` catch-all ignored the
slug when resolving them (`effectiveSlug = [section]`). So every
`/{marketing}/{anything}` path rendered the parent page with a 200 rather than
404ing -- an unlimited surface of indexable duplicates across all 24 marketing
pages, live in production:
/kr/total-garbage 200
/cn/xyz 200
/oss-friends/oss-friends 200
Google found and indexed two of them, and ranks them above the real pages:
`/kr/kr` takes 2,321 clicks against 910 for `/kr`, and `/cn/cn` 2,032 against
1,162 for `/cn` -- the self-referencing canonical on the duplicates is not
being honoured.
- The page component and `generateMetadata` now 404 a marketing section that
carries a slug, which closes the surface.
- `/kr/kr` and `/cn/cn` (plus their `.md` variants) get permanent redirects
instead, so the traffic those two have accumulated moves to the real pages
rather than dropping into a 404.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
31254ed to
e175b76
Compare
Marketing entries are single pages, and the
[section]catch-all ignores the slug when resolving them:So
/{marketing}/{anything}rendered the parent page with a 200 instead of 404ing. That is an unlimited surface of indexable duplicates across all 24 marketing pages, and it is live in production today:Google found two of them and ranks the duplicates above the real pages (last 90 days, Search Console):
/kr/kr/kr/cn/cn/cnBoth duplicates already carry a self-referencing canonical pointing at
/krand/cn; it is not being honoured./japan/japancorrectly 404s because/japanis aDEDICATED_MARKETING_SLUGwith its own route, which is why this only surfaced on the catch-all pages.The fix
generateMetadatanow 404 a marketing section that carries a slug. That closes the surface for all 24 pages. The metadata guard matters too — without it a deep path would still advertise a canonical URL for a route that 404s./kr/krand/cn/cnget permanent redirects (plus their.mdvariants) rather than 404s, so the traffic those two have accumulated moves to the real pages instead of being dropped. Everything else 404s, which is the correct answer for a URL that never existed — redirecting arbitrary paths to a parent is a soft 404 in its own right.Verification
Against the dev server.
All 24 marketing pages still serve:
The two indexed duplicates redirect:
Arbitrary deep paths now 404 (all were 200):
No regression on non-marketing sections, which legitimately have subpages:
Build:
npx next buildcompiled successfully, 966 static pages generated, no route conflicts.node -e 'require("./lib/redirects.js")'loads clean at 464 permanent redirects, no duplicate sources introduced.prettier --writeno-ops on both files.Found while triaging the September Ahrefs crawl; companion to #3716.
🤖 Generated with Claude Code
Note
Low Risk
Routing and redirect changes are scoped to marketing single-page sections; nested docs and other sections are unchanged, with a small SEO-sensitive surface.
Overview
Fixes an SEO duplicate-content bug where any
/{marketing-section}/{extra-segments}URL returned 200 with the parent marketing page, because marketing routes ignore the catch-all slug when resolving content.The catch-all page and
generateMetadatanow 404 when a marketing section has a non-empty slug, so bogus deep URLs are not indexable and metadata does not claim a canonical for routes that 404.Permanent redirects send indexed doubled locale paths (
/kr/kr,/cn/cn, plus.mdvariants) to/krand/cnso existing search traffic is preserved; other invalid marketing deep paths 404.Reviewed by Cursor Bugbot for commit e175b76. Bugbot is set up for automated code reviews on this repo. Configure here.
Greptile Summary
This PR closes an SEO duplicate-content surface by rejecting nested paths beneath single-page marketing sections and preserving accumulated traffic for the known Korean and Chinese doubled paths.
/kr/kr,/cn/cn, and their markdown variants.Confidence Score: 4/5
The implementation appears safe to merge, with only non-blocking automated regression coverage recommended for the new routing and redirect behavior.
The guards are aligned for marketing deep paths, optional slugs are handled safely, and all redirect destinations resolve through existing markdown and page routing; the remaining concern is that these SEO-sensitive contracts are protected only by manual verification.
Files Needing Attention: app/[section]/[[...slug]]/page.tsx
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(seo): stop marketing pages serving e..." | Re-trigger Greptile
Context used: