Repository navigation
feat(publish): custom-domain canonical/OG/sitemap host + tier caps (PR5) - #1709
Merged
Merged
Conversation
- Add resolvePrimaryPublishedHost() pure fn: earliest-activated custom domain wins; hostname lexicographic tiebreaker; pagespace.site fallback - Export ActiveDomainRecord type + canvas/primary-host subpath from lib - Thread primary host through publishCanvasPage (og:url, canonical, JSON-LD) and regeneratePublishedSiteFiles (sitemap <loc>, robots.txt Sitemap: directive) - Add includeSiteFiles flag to planCustomDomainMirror so robots.txt + sitemap.xml are mirrored to every active custom-domain prefix alongside page artifacts and 404.html - Export getActiveDomainRecords() from web custom-domain-mirror so publish-page.ts can load active domains without duplicating the query - Add maxCustomDomains per plan (free=0, pro=1, founder=3, business=10) to PlanDefinition.limits - Enforce tier cap in POST /api/drives/[driveId]/domains: 403 when limit=0 (plan not available) or count >= limit - Return limit from GET /api/drives/[driveId]/domains so the UI can surface it without a second round-trip - Wire clearCustomHost in cert/refresh route when active→cert_failed so stale content is purged on deactivation - Update CustomDomainsCard: show X/N counter, disable Add when at cap, show upgrade nudge for free tier, surface 403 tier errors in toast Tests: pure unit tests for primary-host; updated custom-domain-mirror tests; updated publish-page + route tests with getActiveDomainRecords mock; updated cert/refresh tests; new domains route tests for tier caps Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FWbcqFFxPncD7Xpu6XRdSY
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
📝 WalkthroughWalkthroughThe PR adds custom-domain caps, a deterministic primary host resolver, site-file mirroring for custom-domain hosts, updated published-site URL handling, and certificate-refresh cleanup for active domains that fail. ChangesCustom Domain Limits, Primary Host & Site File Mirroring
Sequence Diagram(s)sequenceDiagram
participant Dashboard
participant DomainsRoute
participant DB
participant CustomDomainsCard
Dashboard->>DomainsRoute: GET /api/drives/:driveId/domains
DomainsRoute->>DB: fetch domains and plan limit
DB-->>DomainsRoute: domains, limit
DomainsRoute-->>Dashboard: { domains, limit }
Dashboard->>CustomDomainsCard: render with limit
CustomDomainsCard->>CustomDomainsCard: show availability or cap state
sequenceDiagram
participant publishCanvasPage
participant getActiveDomainRecords
participant resolvePrimaryPublishedHost
participant regeneratePublishedSiteFiles
publishCanvasPage->>getActiveDomainRecords: driveId
getActiveDomainRecords-->>publishCanvasPage: ActiveDomainRecord[]
publishCanvasPage->>resolvePrimaryPublishedHost: subdomain, publishHost, activeDomains
resolvePrimaryPublishedHost-->>publishCanvasPage: primaryHost
regeneratePublishedSiteFiles->>getActiveDomainRecords: driveId
getActiveDomainRecords-->>regeneratePublishedSiteFiles: ActiveDomainRecord[]
regeneratePublishedSiteFiles->>resolvePrimaryPublishedHost: subdomain, publishHost, activeDomains
resolvePrimaryPublishedHost-->>regeneratePublishedSiteFiles: primaryHost
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/lib/canvas/custom-domain-mirror.ts (1)
218-225: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftActivation backfill copies stale host-dependent artifacts.
After this PR, page canonicals/OG URLs and site files depend on the active-domain set at render time.
mirrorDriveToCustomHost()still just copies the existing subdomain artifacts when a cert flips toactive, so a newly activated domain can serve pages,robots.txt, andsitemap.xmlthat still point at*.pagespace.siteuntil some later publish/regeneration happens. The activation path needs regeneration against the new active-domain set before mirroring.🤖 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 `@apps/web/src/lib/canvas/custom-domain-mirror.ts` around lines 218 - 225, The activation path in mirrorDriveToCustomHost currently reuses stale subdomain artifacts instead of regenerating them for the newly active host. Update the flow around planCustomDomainMirror and the copying logic so that when a cert becomes active, pages, robots.txt, and sitemap.xml are regenerated against the new active-domain set before copying/mirroring site files and root assets. Ensure the host-sensitive render step is tied to the activation path, not deferred to a later publish.
🤖 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
`@apps/web/src/app/api/drives/`[driveId]/domains/[domainId]/cert/refresh/route.ts:
- Around line 83-92: The custom-host purge in the cert status transition path is
fired-and-forgotten, so the route can return before `clearCustomHost()`
finishes. Update the `refresh` route handling for the `domain.status ===
'active' && nextStatus === 'cert_failed'` branch to either `await
clearCustomHost(domain.hostname)` before continuing or move the cleanup into a
supported `after()`-style post-response hook, and keep the existing
`loggers.api.warn` error handling around `clearCustomHost()`.
In `@apps/web/src/app/api/drives/`[driveId]/domains/route.ts:
- Around line 102-123: The cap check in the domains POST handler is race-prone
because getMaxCustomDomainsForDrive and the customDomains count are read before
db.insert() in the same flow, allowing concurrent requests to bypass the limit.
Update the route’s create path to enforce the limit atomically for each drive,
ideally by wrapping the check-and-insert logic in a transaction with a row-level
lock or another DB-backed guard, so the existing count cannot change between the
guard and the insert. Reference the route handler logic around the existing
maxAllowed/existingCount check and the db.insert(customDomains) call when making
the fix.
In `@apps/web/src/lib/canvas/custom-domain-mirror.ts`:
- Around line 26-32: The primary-host selection in getActiveDomainRecords is
using customDomains.createdAt, but that is the row creation time rather than the
time a domain became active. Update the data model and selection flow so
resolvePrimaryPublishedHost orders by a persisted activation timestamp (or
change the documented rule to match the existing field), and make sure
getActiveDomainRecords returns that activation field instead of createdAt for
active domains.
- Around line 109-115: Split the site-file mirroring flow in
custom-domain-mirror so robots.txt and sitemap.xml do not use
copyPublishedArtifact, since that helper applies HTML metadata via
MetadataDirective and ContentType. Update the mirroring logic around the
Promise.allSettled copy loop to route 404.html through copyPublishedArtifact as
before, but handle site files with a separate helper that preserves their
correct content types (text/plain for robots.txt and application/xml for
sitemap.xml). Keep the existing logging in the error path, and use the relevant
symbols copyPublishedArtifact and the mirroring loop in custom-domain-mirror.ts
to locate the change.
In `@apps/web/src/lib/canvas/publish-page.ts`:
- Around line 217-226: The publish response is now using the primary custom
domain even though only the subdomain artifact is guaranteed to be written in
publish-page.ts via resolvePrimaryPublishedHost and the later mirroring step is
best-effort. Update the logic around publishedUrl/result.url so it either waits
for the primary-host mirror to succeed before returning that host, or falls back
to the subdomain URL whenever the custom-domain copy is not confirmed. Use the
existing primaryHost, subdomain, and publishedUrl flow to keep the response
aligned with the actually persisted artifact.
In `@packages/lib/src/canvas/primary-host.ts`:
- Around line 23-25: The primary host selection is currently based on
`createdAt` in `ActiveDomainRecord`, which can pick the wrong canonical host
when activation order differs from record creation. Update `ActiveDomainRecord`
and `getActiveDomainRecords()` in `primary-host.ts` to carry the actual
activation timestamp instead of creation time, then sort by that activation
field so the earliest-activated custom domain is chosen consistently for
`canonical`, `og:url`, sitemap, and robots origins.
---
Outside diff comments:
In `@apps/web/src/lib/canvas/custom-domain-mirror.ts`:
- Around line 218-225: The activation path in mirrorDriveToCustomHost currently
reuses stale subdomain artifacts instead of regenerating them for the newly
active host. Update the flow around planCustomDomainMirror and the copying logic
so that when a cert becomes active, pages, robots.txt, and sitemap.xml are
regenerated against the new active-domain set before copying/mirroring site
files and root assets. Ensure the host-sensitive render step is tied to the
activation path, not deferred to a later publish.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 02de92f2-d3e8-4e8d-90f7-faddd2e1f23c
📒 Files selected for processing (16)
apps/web/src/app/api/drives/[driveId]/domains/[domainId]/cert/refresh/__tests__/route.test.tsapps/web/src/app/api/drives/[driveId]/domains/[domainId]/cert/refresh/route.tsapps/web/src/app/api/drives/[driveId]/domains/__tests__/route.test.tsapps/web/src/app/api/drives/[driveId]/domains/route.tsapps/web/src/app/api/pages/[pageId]/publish/__tests__/route.test.tsapps/web/src/app/dashboard/[driveId]/settings/general/page.tsxapps/web/src/lib/canvas/__tests__/custom-domain-mirror.test.tsapps/web/src/lib/canvas/__tests__/publish-page.test.tsapps/web/src/lib/canvas/custom-domain-mirror.tsapps/web/src/lib/canvas/publish-page.tsapps/web/src/lib/subscription/plans.tspackages/lib/package.jsonpackages/lib/src/canvas/__tests__/custom-domain-mirror.test.tspackages/lib/src/canvas/__tests__/primary-host.test.tspackages/lib/src/canvas/custom-domain-mirror.tspackages/lib/src/canvas/primary-host.ts
- Await clearCustomHost in cert/refresh (was fire-and-forget, should surface errors) - Regenerate site files before mirrorDriveToCustomHost on cert activation so sitemap/robots embed the custom domain as primary host before copying - Wrap POST /domains count-check + insert in db.transaction with drive-row SELECT FOR UPDATE to prevent concurrent over-cap inserts - Add copyPublishedSiteFileArtifact (MetadataDirective=COPY) for site files so robots.txt/sitemap.xml preserve their correct content-types when mirrored - Response URL in publishCanvasPage now uses subdomain (guaranteed write target) instead of primary custom domain (best-effort mirror) for reliability - Clarify ActiveDomainRecord.createdAt is earliest-registered not earliest-activated Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FWbcqFFxPncD7Xpu6XRdSY
Owner
Author
|
Outside-diff comment (activation backfill uses stale host-dependent artifacts): Fixed in 657200b. In |
…ost-commit clearCustomHost is now awaited (so response waits for cleanup) but errors are caught+logged rather than thrown. DB status is already committed before this call, so a storage failure should not make the response appear as a 500 error to the caller. Added test: clearCustomHost throw → still returns 200+cert_failed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FWbcqFFxPncD7Xpu6XRdSY
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@apps/web/src/app/api/drives/`[driveId]/domains/[domainId]/cert/refresh/route.ts:
- Around line 87-88: The cleanup in the cert refresh flow is only triggered on
the first active-to-cert_failed transition, so retries can skip purging stale
mirrored artifacts after a failed clearCustomHost() call. Update the route
handler logic in the cert refresh path to run the purge whenever nextStatus is
cert_failed, regardless of the current domain.status, and keep the
clearCustomHost() call as the idempotent cleanup step so repeated retries safely
no-op when nothing remains.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: fba41135-defd-4fdb-bdc4-02cab3d47e57
📒 Files selected for processing (10)
apps/web/src/app/api/drives/[driveId]/domains/[domainId]/cert/refresh/__tests__/route.test.tsapps/web/src/app/api/drives/[driveId]/domains/[domainId]/cert/refresh/route.tsapps/web/src/app/api/drives/[driveId]/domains/__tests__/route.test.tsapps/web/src/app/api/drives/[driveId]/domains/route.tsapps/web/src/lib/canvas/__tests__/custom-domain-mirror.test.tsapps/web/src/lib/canvas/__tests__/publish-page.test.tsapps/web/src/lib/canvas/custom-domain-mirror.tsapps/web/src/lib/canvas/publish-page.tsapps/web/src/lib/canvas/published-storage.tspackages/lib/src/canvas/primary-host.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/web/src/app/api/drives/[driveId]/domains/[domainId]/cert/refresh/tests/route.test.ts
- apps/web/src/lib/canvas/publish-page.ts
- apps/web/src/app/api/drives/[driveId]/domains/route.ts
- packages/lib/src/canvas/primary-host.ts
- apps/web/src/lib/canvas/custom-domain-mirror.ts
- apps/web/src/app/api/drives/[driveId]/domains/tests/route.test.ts
- apps/web/src/lib/canvas/tests/custom-domain-mirror.test.ts
…s, not just active→cert_failed Retryability fix: the previous condition only purged mirrored artifacts on the first active→cert_failed transition. If clearCustomHost() threw, the DB was already cert_failed, so retries saw a non-active status and skipped cleanup, leaving stale prefix artifacts indefinitely. Widening to nextStatus === 'cert_failed' is safe because clearCustomHost() deletes everything under published/<host>/ which is a no-op when the prefix is already empty. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FWbcqFFxPncD7Xpu6XRdSY
This was referenced Jun 25, 2026
3 of 4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Files changed
Test plan
🤖 Generated with Claude Code
https://claude.ai/code/session_01FWbcqFFxPncD7Xpu6XRdSY