Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
|
Cursor Agent can help with this pull request. Just |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds route-level sitemap handles (returning null) to three auth routes and updates JSX sitemap indexing to skip several auth paths plus the Changes
Sequence Diagram(s)(Skipped — changes are configuration, small route exports, and tests; no multi-component control flow to visualize.) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Alphabetical ordering broken in skipped routes set
- Reordered the skipped static routes so
/signupappears before/sitemap.xmlto restore alphabetical ordering.
- Reordered the skipped static routes so
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@other/semantic-search/jsx-page-content.ts`:
- Line 45: The SKIPPED_PREFIX_ROUTES array currently only guards against '/me/'
descendants and misses the exact '/me' path; update SKIPPED_PREFIX_ROUTES (the
constant named SKIPPED_PREFIX_ROUTES in jsx-page-content.ts) to include the
exact '/me' entry (e.g., add '/me') so both '/me' and '/me/*' are covered by the
defensive filter.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/routes/login.tsxapp/routes/me_.passkeys.tsxapp/routes/me_.password.tsxother/semantic-search/__tests__/jsx-page-content.test.tsother/semantic-search/jsx-page-content.ts
| '/testimonials', | ||
| ]) | ||
| const SKIPPED_PREFIX_ROUTES = ['/blog/', '/calls/', '/chats/', '/talks/'] | ||
| const SKIPPED_PREFIX_ROUTES = ['/blog/', '/calls/', '/chats/', '/me/', '/talks/'] |
There was a problem hiding this comment.
Exclude exact /me path as well.
Line 45 skips /me/ descendants, but not /me itself. That leaves a gap in the defensive filter for authenticated pages.
Suggested patch
const SKIPPED_STATIC_ROUTES = new Set([
'/credits',
'/forgot-password',
'/login',
'/magic',
+ '/me',
'/reset-password',
'/resume',
'/sitemap.xml',
'/signup',
'/testimonials',
])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@other/semantic-search/jsx-page-content.ts` at line 45, The
SKIPPED_PREFIX_ROUTES array currently only guards against '/me/' descendants and
misses the exact '/me' path; update SKIPPED_PREFIX_ROUTES (the constant named
SKIPPED_PREFIX_ROUTES in jsx-page-content.ts) to include the exact '/me' entry
(e.g., add '/me') so both '/me' and '/me/*' are covered by the defensive filter.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Prevent authenticated pages (
/login,/me/*) from appearing in search results by adding sitemap opt-outs and a defensive indexer filter.Note
Low Risk
Low risk: changes are limited to sitemap/indexer filtering and related tests, but could affect what pages appear in search/indexing if the route allow/deny lists are incorrect.
Overview
Prevents authenticated/account-related pages from being discoverable via the JSX semantic-search index and sitemap-driven indexing.
Adds
handle.getSitemapEntries: () => nullto opt outloginand/meaccount routes from sitemap entries, and hardens the semantic-search crawler (jsx-page-content.ts) by skipping common auth/account paths (including/me/*) with updated tests to lock in the exclusions.Written by Cursor Bugbot for commit 43616bb. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
Chores
Tests