Repository navigation
feat(canvas): per-domain landing & 404 page overrides - #1996
Conversation
Generalizes the pagespace.ai/prism-style platform-owned custom domain mechanism into a self-service feature: any drive owner can now give a custom domain its own landing (root) page and 404 page, instead of every domain on a drive being a byte-for-byte identical mirror. - packages/db/src/schema/custom-domains.ts: nullable publishLandingPageId/publishNotFoundPageId columns (migration 0193) - packages/lib/src/canvas/custom-domain-mirror.ts: pure resolvers (resolveHostRootCopies, resolveBackfillRootCopy) deciding which page's artifact backs a given host's root/404, with override-wins precedence - apps/web/src/lib/canvas/custom-domain-mirror.ts + apps/web/src/lib/canvas/publish-page.ts: mirror pipeline wiring, including a verified fallback to the drive-wide 404 when an override page fails to render (e.g. trashed after being set) - apps/web/src/app/api/drives/[driveId]/domains/[domainId]/route.ts: PATCH accepts publishLandingPageId/publishNotFoundPageId - apps/web/src/app/dashboard/[driveId]/settings/domains/page.tsx: per-domain landing/404 page pickers in the Domains & Publishing settings UI Also adds the docs-routing-domain-landing-pages-epic plan, which covers the originating motivation: routing pagespace.ai/docs to published Canvas content the same way pagespace.ai/prism already works (that routing change itself lives in the separate PageSpace-Deploy repo and ships independently). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011vSSxCSyGbgjVXWihK9eEC
# Conflicts: # packages/db/drizzle/meta/0193_snapshot.json # packages/db/drizzle/meta/_journal.json
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughPer-domain landing and 404 page overrides were added across database schema, API validation, publishing mirrors, certificate flows, and the Domains settings UI. Custom-domain site files now support host-specific override rendering with fallback behavior. ChangesDomain publishing overrides
Schema snapshot updates
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant DomainSettings
participant DomainPatchAPI
participant CustomDomainMirror
participant PublishedStorage
DomainSettings->>DomainPatchAPI: PATCH landing or 404 page override
DomainPatchAPI->>CustomDomainMirror: re-mirror affected host
CustomDomainMirror->>PublishedStorage: render or copy host artifacts
PublishedStorage-->>DomainSettings: updated domain state after revalidation
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23d19215df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 197, |
There was a problem hiding this comment.
Adding the 0197 journal entry while this diff deletes/renames packages/db/drizzle/meta/0193_snapshot.json leaves the existing idx 193 journal entry with no corresponding snapshot file. Drizzle Kit relies on these historical snapshots when generating future migrations, so a checkout after this commit can fail or produce invalid diffs when it tries to load 0193_snapshot.json; please keep 0193_snapshot.json and add 0197_snapshot.json as a new file instead of moving the old snapshot.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1a79f21 — restored packages/db/drizzle/meta/0193_snapshot.json from master rather than leaving it deleted. You're right that this would have broken drizzle-kit generate for anyone checking out this history later; I confirmed by re-running bun run db:generate afterward, which now correctly reports "No schema changes, nothing to migrate" instead of re-diffing against a missing snapshot.
| include404: !overrideRendered, | ||
| includeSiteFiles: true, | ||
| }); | ||
| const allCopies = rootCopy ? [...copies, rootCopy] : copies; |
There was a problem hiding this comment.
When a host previously had a landing override and the override is reset while the drive default root is absent/unpublished (or a new override points at an unpublished page), rootCopy is null, so this backfill only copies paths/site files and leaves the old published/<host>/index.html in place. The API then reports the landing page reset even though the domain root continues serving the prior override until some later unpublish deletes it; delete the host root when there is no replacement root copy.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1a79f21 — mirrorDriveToCustomHost now deletes the host's stale root object (published/<host>/index.html) whenever the backfill resolves no valid root copy, instead of silently leaving prior content live. Added 3 regression tests covering: override reset with no drive home page, override pointing at an unpublished page, and the positive case (root copy resolves → no delete).
# Conflicts: # packages/db/drizzle/meta/0197_snapshot.json # packages/db/drizzle/meta/_journal.json
- Restore packages/db/drizzle/meta/0193_snapshot.json, which was deleted instead of preserved during an earlier merge-conflict resolution. Drizzle Kit needs every historical snapshot to generate future migrations correctly — deleting one breaks the chain for anyone who checks out this history later, even though the migration itself (0193_awesome_titania, from master) still applies fine today. - mirrorDriveToCustomHost: clear a custom-domain host's stale root object when the backfill resolves no valid root copy (override reset with no drive-wide home page, or a new override pointing at a page that isn't published yet). Previously the backfill just skipped the root copy in that case, leaving prior content (an old override, or an old home-page mirror) silently still live at that host's "/" even though the API had already reported the change as applied. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011vSSxCSyGbgjVXWihK9eEC
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/web/src/app/dashboard/[driveId]/settings/domains/page.tsx (2)
294-339: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting a shared domain-override PATCH helper.
handleSetDomainLandingPageandhandleSetDomainNotFoundPageare nearly identical — same fetch logic, error handling, and revalidation, differing only in the field name, saving-state setter, and toast messages. Extracting a generic helper would reduce duplication and make future override fields trivial to add.This is a minor maintainability improvement and can be deferred.
♻️ Proposed shared helper
+async function handleSetDomainOverride( + driveId: string, + domainId: string, + field: 'publishLandingPageId' | 'publishNotFoundPageId', + pageId: string | null, + setSaving: (id: string | null) => void, + savingGuard: string | null, + mutateDomains: () => Promise<void>, +) { + if (savingGuard) return; + setSaving(domainId); + try { + const res = await fetchWithAuth(`/api/drives/${driveId}/domains/${domainId}`, { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ [field]: pageId }), + }); + if (!res.ok) { + const data = await res.json().catch(() => ({})) as { error?: string }; + toast.error(data.error ?? `Failed to update ${field === 'publishLandingPageId' ? 'landing page' : '404 page'}`); + return; + } + await mutateDomains(); + const label = field === 'publishLandingPageId' ? 'Landing page' : '404 page'; + toast.success(pageId ? `${label} updated` : `${label} reset to the drive default`); + } catch { + toast.error(`Failed to update ${field === 'publishLandingPageId' ? 'landing page' : '404 page'}`); + } finally { + setSaving(null); + } +}Then the two handlers become one-liners:
const handleSetDomainLandingPage = async (domainId: string, pageId: string | null) => { - if (savingLandingPageId) return; - setSavingLandingPageId(domainId); - try { - const res = await fetchWithAuth(`/api/drives/${driveId}/domains/${domainId}`, { - method: 'PATCH', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ publishLandingPageId: pageId }), - }); - if (!res.ok) { - const data = await res.json().catch(() => ({})) as { error?: string }; - toast.error(data.error ?? 'Failed to update landing page'); - return; - } - await mutateDomains(); - toast.success(pageId ? 'Landing page updated' : 'Landing page reset to the drive default'); - } catch { - toast.error('Failed to update landing page'); - } finally { - setSavingLandingPageId(null); - } + handleSetDomainOverride(driveId, domainId, 'publishLandingPageId', pageId, setSavingLandingPageId, savingLandingPageId, mutateDomains); }; const handleSetDomainNotFoundPage = async (domainId: string, pageId: string | null) => { - if (savingNotFoundPageId) return; - setSavingNotFoundPageId(domainId); - try { - const res = await fetchWithAuth(`/api/drives/${driveId}/domains/${domainId}`, { - method: 'PATCH', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ publishNotFoundPageId: pageId }), - }); - if (!res.ok) { - const data = await res.json().catch(() => ({})) as { error?: string }; - toast.error(data.error ?? 'Failed to update 404 page'); - return; - } - await mutateDomains(); - toast.success(pageId ? '404 page updated' : '404 page reset to the drive default'); - } catch { - toast.error('Failed to update 404 page'); - } finally { - setSavingNotFoundPageId(null); - } + handleSetDomainOverride(driveId, domainId, 'publishNotFoundPageId', pageId, setSavingNotFoundPageId, savingNotFoundPageId, mutateDomains); };🤖 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/app/dashboard/`[driveId]/settings/domains/page.tsx around lines 294 - 339, Extract the duplicated PATCH, error handling, domain revalidation, and saving-state cleanup from handleSetDomainLandingPage and handleSetDomainNotFoundPage into a shared helper that accepts the override field name, page ID, saving-state setter, and success/error messages. Refactor both handlers to call this helper while preserving their existing payloads and toast behavior.
1091-1117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo per-row saving indicator for page override pickers.
When a landing or 404 page override is being saved, the
PagePickerPopoveris disabled viaanyBusybut shows no spinner or loading state on the affected row. Other per-row actions (verify, cert, remove, primary) all have individualis*booleans that render aLoader2spinner. The newsavingLandingPageId/savingNotFoundPageIdstates are only consumed inanyBusyand never reachDomainRowas per-row flags.This is a minor UX gap — users get no visual feedback on which domain's picker is saving. Consider passing
isSavingLandingPage/isSavingNotFoundPagebooleans toDomainRowand showing a spinner in the picker label area.🤖 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/app/dashboard/`[driveId]/settings/domains/page.tsx around lines 1091 - 1117, Expose per-domain saving flags from the page override save state through DomainRow, deriving isSavingLandingPage and isSavingNotFoundPage by comparing the saving IDs with domain.id. Pass these flags from the parent and render a Loader2 spinner beside each corresponding picker label while saving, while retaining the existing disabled behavior.
🤖 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]/route.ts:
- Around line 78-159: Validate publishLandingPageId and publishNotFoundPageId
before any database mutation, including the primary-domain transaction in the
PATCH handler. Refactor the update flow so primary and override fields are
applied atomically in one transaction, or otherwise ensure all validation
completes before committing; preserve the existing Canvas/drive ownership checks
in isValidDriveNotFoundPage. Add a regression test covering { isPrimary: true,
publishLandingPageId: "invalid" } that asserts a 400 response and no
primary-domain change.
In `@tasks/docs-routing-domain-landing-pages-epic.md`:
- Line 88: The PATCH update flow must not treat the domain update as complete
while `mirrorDriveToCustomHost` is only running in an unawaited catch chain.
Update the relevant PATCH handler to either await the single-domain re-mirror
before returning success or enqueue it through the existing publish-job
mechanism, matching other publish-triggering endpoints, and preserve appropriate
error handling.
- Around line 109-112: The documentation should distinguish hard deletion from
soft-trash behavior: retain onDelete: 'set null' for deleted pages, but
explicitly require resolver fallback and test coverage when an override page is
trashed, since the FK does not clear trashed references. Update the
mirror-pipeline and API validation test requirements accordingly.
---
Nitpick comments:
In `@apps/web/src/app/dashboard/`[driveId]/settings/domains/page.tsx:
- Around line 294-339: Extract the duplicated PATCH, error handling, domain
revalidation, and saving-state cleanup from handleSetDomainLandingPage and
handleSetDomainNotFoundPage into a shared helper that accepts the override field
name, page ID, saving-state setter, and success/error messages. Refactor both
handlers to call this helper while preserving their existing payloads and toast
behavior.
- Around line 1091-1117: Expose per-domain saving flags from the page override
save state through DomainRow, deriving isSavingLandingPage and
isSavingNotFoundPage by comparing the saving IDs with domain.id. Pass these
flags from the parent and render a Loader2 spinner beside each corresponding
picker label while saving, while retaining the existing disabled behavior.
🪄 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: 410cf86f-2c36-4883-8eb2-f87eb2d3c6a2
📒 Files selected for processing (26)
apps/web/src/app/api/drives/[driveId]/domains/[domainId]/__tests__/route.test.tsapps/web/src/app/api/drives/[driveId]/domains/[domainId]/cert/refresh/__tests__/route.test.tsapps/web/src/app/api/drives/[driveId]/domains/[domainId]/route.tsapps/web/src/app/api/drives/[driveId]/domains/[domainId]/verify/__tests__/route.test.tsapps/web/src/app/api/drives/[driveId]/domains/[domainId]/verify/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/route.tsapps/web/src/app/dashboard/[driveId]/settings/domains/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/__tests__/published-storage.test.tsapps/web/src/lib/canvas/__tests__/reconcile-cert.test.tsapps/web/src/lib/canvas/custom-domain-mirror.tsapps/web/src/lib/canvas/publish-page.tsapps/web/src/lib/canvas/published-storage.tsapps/web/src/lib/canvas/reconcile-cert.tspackages/db/drizzle/0198_skinny_prowler.sqlpackages/db/drizzle/meta/0198_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/custom-domains.tspackages/lib/src/canvas/__tests__/custom-domain-mirror.test.tspackages/lib/src/canvas/custom-domain-mirror.tspackages/lib/src/canvas/primary-host.tsplan.mdtasks/docs-routing-domain-landing-pages-epic.md
…ailure
Independent review caught a regression in the stale-root-delete added for
the prior fix: the pre-existing homeRootExists probe's catch-all swallows
ANY error (not just 404), not just genuine absence. That was harmless
before (a null rootCopy was a pure no-op) but is now destructive (null
rootCopy triggers a delete) — a transient S3/Tigris read failure would
delete a live, currently-correct custom-domain homepage until the next
backfill happens to succeed.
Fixed by tracking probe failure separately from confirmed absence, and
skipping the delete (not just the copy) when the probe itself failed —
matching the existing fail-safe precedent in reconcile-cert.ts ("a
DNS-valid domain must not lose its content because a read hit a
transient blip").
Also documented (not fixed, out of scope for this PR) a related but
lower-severity finding: mirrorDriveToCustomHost has no per-host locking
across its several unsynchronized call sites, so a sufficiently
adversarial interleaving of two overlapping calls could still delete
freshly-written root content. This predates the stale-root delete (the
copy path has the same shape) and is self-healing on the next backfill;
a proper fix needs per-host serialization, a larger change than this
feature's scope.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011vSSxCSyGbgjVXWihK9eEC
|
Ran an independent deep-review pass on the diff after the two Codex fixes above and it caught a regression I introduced in the stale-root-delete fix itself: the pre-existing Fixed in 450ef65 — probe failure is now tracked separately from confirmed absence, and the delete is skipped (not just the copy) when the probe itself failed, matching the existing fail-safe precedent already established in Also documented (not fixed) a related, lower-severity finding from the same pass: |
…rror CodeRabbit review on PR #1996: - A mixed payload like { isPrimary: true, publishLandingPageId: "invalid" } could commit the isPrimary transaction (audit log, page re-render) before the landing-page validation ran, so a 400 response could still carry a real, persisted primary-domain change. All validation now runs before either mutation block. Added a regression test covering this exact case. - The landing/404 override block called mirrorDriveToCustomHost(...).catch() without awaiting, inconsistent with the isPrimary block's awaited republishDriveCanonical/regeneratePublishedSiteFiles calls. Now awaited with the same try/catch/log pattern. - epic.md: clarified that onDelete: 'set null' only fires on hard delete, not soft-trash — the trashed-page case relies on the resolver-level fallback and API validation, both of which need their own test coverage.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
publishLandingPageId/publishNotFoundPageIdtocustom_domains(migration0198_skinny_prowler.sql), with pure resolvers (resolveHostRootCopies,resolveBackfillRootCopy) deciding which page's artifact backs a given host's root/404, override-wins precedence.apps/web/src/lib/canvas/custom-domain-mirror.ts,publish-page.ts), the domain PATCH API, and a settings-UI picker (apps/web/src/app/dashboard/[driveId]/settings/domains/page.tsx).isPrimarychange alongside an invalidpublishLandingPageId) is rejected atomically instead of partially applying.tasks/docs-routing-domain-landing-pages-epic.md, the plan this work grew out of (routingpagespace.ai/docsto published Canvas content the same way/prismalready works — that routing change itself lives in the separatePageSpace-Deployrepo and ships independently of this PR).Test plan
bunx turbo run typecheck— clean across the monorepo (post-merge with master)bunx turbo run build— apps/web production build succeedspackages/lib/src/canvas,apps/web/src/lib/canvas,apps/web/src/app/api/drives) — 1140+ tests passing, including new regression coverage for the 404-fallback fix and the mixed-payload validation-ordering fix🤖 Generated with Claude Code
https://claude.ai/code/session_011vSSxCSyGbgjVXWihK9eEC
Summary by CodeRabbit
New Features
Bug Fixes