fix(adapter-static): resolve crawled links against the page, not the origin - #224
Conversation
🦋 Changeset detectedLatest commit: 9a4bdc1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe static crawler now resolves discovered relative links against the URL of the page containing each link. Redirect targets are also resolved against the current request URL, including pathname, search, and hash handling. URL helpers accept either string or URL bases and skip cross-origin results. Deduplication paths use the resolved page-relative destinations. A patch changeset documents the behavior. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/adapters/static/src/crawler.ts (1)
31-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for page-relative resolution.
Please cover
./siblingfrom/docs/reference/language, relative redirect targets with query/hash components, and off-origin targets remaining unenqueued. These paths are central to the PR behavior and currently have no unit coverage.Also applies to: 91-98
🤖 Prompt for 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. In `@packages/adapters/static/src/crawler.ts` around lines 31 - 41, Add regression tests for the crawler’s page-relative URL resolution around resolvePath and queueing: verify ./sibling from /docs/reference/language resolves to /docs/reference/sibling, relative redirect targets preserve query and hash components, and off-origin targets are not added to the queue. Cover both the normal link path and the corresponding redirect handling noted in the comment.
🤖 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 `@packages/adapters/static/src/crawler.ts`:
- Around line 90-94: The redirect handling around resolvePathWithHash must
reject a missing or blank Location header before resolving it against url. Only
call resolvePathWithHash and continue redirect processing when the header
contains a non-empty location; otherwise treat the 3xx response as an unhandled
redirect and avoid generating a self-redirect.
---
Nitpick comments:
In `@packages/adapters/static/src/crawler.ts`:
- Around line 31-41: Add regression tests for the crawler’s page-relative URL
resolution around resolvePath and queueing: verify ./sibling from
/docs/reference/language resolves to /docs/reference/sibling, relative redirect
targets preserve query and hash components, and off-origin targets are not added
to the queue. Cover both the normal link path and the corresponding redirect
handling noted in the comment.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 95238ee8-9069-4cf8-ad59-882e4baedb42
📒 Files selected for processing (2)
.changeset/crawler-relative-href-base.mdpackages/adapters/static/src/crawler.ts
97e8319 to
076a4c3
Compare
…origin `visit` passed `origin` as the base for every href it found, so a relative link lost the directory it was written in: `./concise-syntax` on `/docs/reference/language` resolved to `/concise-syntax`. That path 404s, and the crawler then wrote the 404 body out as `concise-syntax.html`, which a static host serves at 200 -- a thin, indexable duplicate for every relative link in the site. The page URL was already in scope; the inner `const path` shadowed it, which is likely how this went unnoticed. It is now the base, and the shadowing is gone. Redirect `Location` headers resolve against it too, which is what the fetch spec says a relative location means. `origin` is now parsed once into a `URL`, so every base is a `URL` and `resolveUrl` takes its same-origin check from `base.origin` instead of re-parsing the base on every link. Measured on markojs.com: 168 stray 404 pages against 57 real ones, of which 65 were doc-shaped. After the fix only one remains, and that one is a genuine broken link in the docs -- now emitted at the path it is actually written at.
076a4c3 to
9a4bdc1
Compare
2 participants
The crawler resolved every href it found against the origin rather than the page it was found on, so a relative link lost its directory:
./concise-syntaxon/docs/reference/languagewas crawled as/concise-syntax. That path 404s, and the crawler writes the 404 body out under it — a thin, indexable duplicate page for every relative link on the site. The innerconst pathshadowed the outer one, which is likely how this went unnoticed; it is nowfound.Redirect
Locationheaders resolve against the page for the same reason — per the fetch spec a relative location is relative to the request URL.resolveUrlnow derives its same-origin check from the base instead of taking the origin as both base and comparison, so off-origin links are still skipped.