Remove the unreachable social-crawler rewrite and its page - #1554
Conversation
The middleware rewrote social crawlers to a hand-built /redditbot page, but that branch cannot run: it only matched the three-segment /category/@author/permlink form, and the category redirect above it 308s that form onto the bare canonical first, deliberately. Posts self-canonicalized to the bare /@author/permlink in May 2026, and the bare form never matched the pattern, so every shared link has been served the ordinary page for months. That is the right page to serve. htmlLimitedBots already gives crawlers blocking metadata in <head>, verified in production for redditbot, Twitterbot and facebookexternalhit, and it carries the canonical link, JSON-LD, og:site_name, og:locale and og:image:alt that the removed page had drifted out of sync on. Keeping a second, unreachable source of social metadata only invites the two to disagree. Tests pin the two facts that made the rewrite unreachable, and that the crawlers still resolve to the blocking-metadata list.
|
Warning Review limit reached
Next review available in: 5 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
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 |
PR Summary by QodoRemove unreachable social-crawler route and consolidate metadata
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo
1. Canonical test bypasses middleware
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Greptile SummaryThe PR removes the unreachable social-crawler route and middleware rewrite, relying on canonical entry-page metadata instead.
Confidence Score: 4/5The PR should not merge until TelegramBot is also confirmed in the manually deployed nginx cache-key map on every production origin. The application-side crawler classification is fixed, but the production cache boundary is configured outside the repository; updating and testing its markdown representation does not apply the required nginx change, so browser-cached HTML can still break Telegram previews. Files Needing Attention: docs/cache/nginx.md and apps/web/src/specs/features/next-middleware/social-bot-metadata.spec.ts
|
| Filename | Overview |
|---|---|
| apps/web/next.config.js | Adds TelegramBot to Next.js’s HTML-limited crawler regex so its metadata is rendered synchronously. |
| apps/web/src/middleware.ts | Removes the unreachable crawler-specific rewrite and documents why canonical entry routing supersedes it. |
| apps/web/src/app/(dynamicPages)/entry/[category]/[author]/[permlink]/redditbot/route.ts | Deletes the redundant crawler-only entry renderer. |
| apps/web/src/specs/features/next-middleware/social-bot-metadata.spec.ts | Covers crawler redirects and regex alignment, but its nginx assertion validates documentation rather than the manually deployed origin configuration. |
| docs/cache/nginx.md | Adds TelegramBot to the reference cache-key map, while production still requires an independent manual configuration rollout. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
T[TelegramBot request] --> N[Origin nginx cache]
B[Browser request] --> N
N --> K{Deployed bot cache-key map}
K -->|Telegram classified| H[Dedicated htmlbot cache entry]
K -->|Telegram absent| S[Browser-shared cache entry]
H --> X[Next.js blocking metadata]
S --> M[Potential browser-primed streamed response]
D[docs/cache/nginx.md] -. manual rollout .-> K
C[next.config.js htmlLimitedBots] --> X
Reviews (3): Last reviewed commit: "Mirror TelegramBot into the nginx cache-..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Review of the removal turned up a real gap: TelegramBot was in the rewrite's user-agent list but never in htmlLimitedBots, so Telegram has been receiving metadata streamed into the body, which a crawler that does not execute JS cannot read. It is in the list now, and the test walks all eight agents the removed rewrite used to catch rather than a sample, so a future removal from either list fails loudly. Also annotates the test helper's return type.
next.config.js carries an explicit warning that htmlLimitedBots is mirrored in the origin nginx SSR cache key, and that drift makes the setting silently stop working because a page primed by a browser is then served to a crawler from the wrong cache namespace. Adding TelegramBot on the app side alone did exactly that, so the documented map now carries it too. The real fix is the test: it parses the map out of docs/cache/nginx.md and fails on any disagreement in either direction. Verified it reports "in next.config but not nginx: expected [ 'TelegramBot' ]" when the doc is reverted. This drift is invisible in dev, where nothing is cached, so it needed to be something other than a reviewer noticing.⚠️ The deployed origin nginx map needs the same one-word addition; the repo copy is documentation, not the running config.
| map $http_user_agent $html_limited_bot { | ||
| default ""; | ||
| "~*(Mediapartners-Google|Chrome-Lighthouse|Slurp|DuckDuckBot|baiduspider|yandex|sogou|bitlybot|tumblr|vkShare|quora link preview|redditbot|ia_archiver|Bingbot|BingPreview|applebot|facebookexternalhit|facebookcatalog|Twitterbot|LinkedInBot|Slackbot|Discordbot|WhatsApp|SkypeUriPreview|Yeti)" "|htmlbot"; | ||
| "~*(Mediapartners-Google|Chrome-Lighthouse|Slurp|DuckDuckBot|baiduspider|yandex|sogou|bitlybot|tumblr|vkShare|quora link preview|redditbot|ia_archiver|Bingbot|BingPreview|applebot|facebookexternalhit|facebookcatalog|Twitterbot|LinkedInBot|Slackbot|Discordbot|TelegramBot|WhatsApp|SkypeUriPreview|Yeti)" "|htmlbot"; |
There was a problem hiding this comment.
If the production origin map has not been separately updated, this documentation-only change leaves TelegramBot in the browser cache namespace, causing browser-primed HTML with streamed metadata to produce degraded or missing Telegram link previews.
There was a problem hiding this comment.
Right, and that is the open item rather than a new one: the commit message and the earlier reply both call it out, and this PR deliberately does not change the running config.
Confirmed on both origins just now: each still carries the pre-change alternation in its $html_limited_bot map, and each has site configs using it in proxy_cache_key. Until that one-word addition is applied and nginx reloaded on both, Telegram keeps hashing into the browser namespace, exactly as described.
That edit is a production change to a hand-managed file, so it sits with the maintainer rather than in this PR. What this PR can do, and now does, is make the drift impossible to reintroduce silently: the parity test fails on any disagreement between htmlLimitedBots and the documented map, in either direction.
There was a problem hiding this comment.
That's a fair and thorough response. The parity test is the right enforcement mechanism here — it converts a silent drift risk into a failing test, which is strictly better than hoping the two stay in sync manually. Acknowledged that the production apply is a separate, hand-managed operation outside this PR's scope.
Comment withdrawn.
|
Correction to what I claimed on this PR, now that the deployed config has been updated and the behaviour measured. Telegram previews were never degraded. Telegram's crawler identifies as So the "TelegramBot is missing" finding, my reply built on it, and the follow-up about a browser-primed page reaching Telegram were all wrong in their premise. The listing is still worth having, since it no longer depends on Telegram happening to carry another crawler's name in its UA string, but it fixed nothing that was broken. The deployed What stands from this PR regardless: the unreachable route is gone, and the parity test now fails on any drift between |
1 participant
Closes #1553.
The middleware rewrote social crawlers to a purpose-built
/redditbotpage, but that branch cannot run. It only matched the three-segment/category/@author/permlinkform, andhandleCategoryEntryRedirect308s that form onto the bare canonical earlier in the same middleware (deliberately, per its own comment). Posts self-canonicalized to the bare/@author/permlinkin60a277a(2026-05-13), and the bare form never matched the pattern, so every shared link has been served the ordinary page for months.That is the page crawlers should get. Verified against production with real crawler user agents:
htmlLimitedBotsgives them blocking metadata in<head>with correctog:title,og:description,og:imageandtwitter:card=summary_large_image, and the canonical page also carries the canonical link, JSON-LD,og:site_name,og:localeandog:image:altthat the removed page lacked. Keeping a second, unreachable source of social metadata only invites the two to disagree, which it already had.Tests pin the two facts that made the rewrite unreachable (a crawler on the category URL is redirected like any visitor, the bare canonical is left alone) and that the crawler agents still resolve against the blocking-metadata list.
pnpm typecheckclean,pnpm testgreen (317 files, 2962 tests).On the second finding, declaring og:image dimensions
Measured rather than assumed, and it is not worth doing:
proxifyImageSrchardcodesmode: 'fit', and sources are not uniform (964x1280, 500x750, 1280x853 across sampled posts), so no size the site currently requests is deterministic. Declaring 1200x630 today would be declaring the wrong number.mode=cover&width=1200&height=630variant. Sampling five trending posts, the existing og variant was a Cloudflare HIT on 5 of 5 (age ~568s, warm because the page and JSON-LD already request it), while the cover variant missed on 2 of 5 and would be cold for every post: new imagehoster compute exactly when a crawler is first generating the card, plus a second cached copy of every post image at the edge.The crawler downloads the image regardless, so the gain is marginal. Left alone deliberately.