Repository navigation
feat(publish): auto-allocate globally-unique publishSubdomain per drive - #1670
Merged
Merged
Conversation
Foundation for wildcard subdomains (pu/cname). Every drive now gets a globally-unique <sub>.pagespace.site identity at creation, not lazily on first Canvas publish. Fixes the slug-not-globally-unique resolution gap (two owners can each own a drive named "acme" — each now gets a distinct subdomain). - resolveUniquePublishSubdomain: pure allocator (normalize + de-dupe with -2/-3 suffix, skip reserved/invalid, clamp for length, fallback on empty) - allocateUniqueSubdomainWithRetry: pure race-recovery (re-read taken on unique-violation, advance suffix) — testable without a DB - allocatePublishSubdomain: DB-backed wrapper (works in/out of a tx) - Wired into all 6 drive-creation paths: drive-service, home-drive (web + admin), MCP drives route, AI drive-tools - Backfill script for existing drives (idempotent, collision-skipping) Existing explicit-subdomain publish path (user picks a custom name) is left intact — auto-allocation handles the default; the 409-on-collision explicit path still serves user choice. Phase 1 of the wildcard-subdomains epic; unblocks Phase 2 (homepage auto-serve) and the CNAME payoff.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
Warning Review limit reached
More reviews will be available in 30 minutes and 34 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThe PR implements automatic allocation of globally-unique ChangesPublish Subdomain Allocation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
…empotency Independent code review of PR 1670 found real defects; fixed all four: 1. Infinite loop on long bases with many collisions. clampBase reserved a static -NN (2-digit) budget, so once the suffix hit 3 digits (100+) the candidate exceeded 63 chars, failed validation, and the loop spun forever. Replaced with per-suffix dynamic clamping (clampBaseForSuffix): every candidate is provably <= 63 chars and validates, so the loop is bounded. Regression-tested with worst-case 120 collisions on a 63-char base. 2. Wrong import path in backfill lib. scripts/lib/publish-subdomain-backfill imported resolveUniquePublishSubdomain from @pagespace/lib/services/ subdomain-allocation, which does not export it (only imports it) and is not in the package export map. Fixed to import from @pagespace/lib/validators/subdomain (correct + already exported). Also added ./services/subdomain-allocation to the package export map for external consumers. 3. allocatePublishSubdomain not idempotent. Re-calling it for an already-provisioned drive would overwrite the existing subdomain. Now early-returns the existing value; the update is conditional on publishSubdomain being null, with race-recovery (re-read the winner value) when zero rows update. 4. Quadratic backfill. The runner re-fetched ALL subdomains every batch iteration. Now fetches the taken set once before the loop and keeps it current as allocations succeed (Set.add), so it is O(total) not O(batches x total).
![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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/src/lib/ai/tools/drive-tools.ts (1)
213-228:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrap insert + subdomain allocation in a single transaction.
This path can throw after
insertsucceeds but before subdomain allocation completes, so the tool reports failure while the drive already exists. That is a partial-write contract break.💡 Suggested fix
- const [newDrive] = await db.insert(drives).values({ - name: name.trim(), - slug, - ownerId: userId, - drivePrompt: driveContext || null, - updatedAt: new Date(), - }).returning({ - id: drives.id, - name: drives.name, - slug: drives.slug, - drivePrompt: drives.drivePrompt, - }); - - // Auto-allocate a globally-unique publish subdomain (idempotent with drive-service). - await allocatePublishSubdomain(newDrive.id, slug); + const newDrive = await db.transaction(async (tx) => { + const [createdDrive] = await tx.insert(drives).values({ + name: name.trim(), + slug, + ownerId: userId, + drivePrompt: driveContext || null, + updatedAt: new Date(), + }).returning({ + id: drives.id, + name: drives.name, + slug: drives.slug, + drivePrompt: drives.drivePrompt, + }); + + // Auto-allocate in the same transaction to avoid partial writes. + await allocatePublishSubdomain(createdDrive.id, slug, tx); + return createdDrive; + });🤖 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/ai/tools/drive-tools.ts` around lines 213 - 228, The insert operation on the drives table and the subsequent allocatePublishSubdomain function call are not wrapped in a transaction, creating a risk of partial writes if the subdomain allocation fails after the insert succeeds. Wrap both the db.insert(drives).values() operation and the allocatePublishSubdomain(newDrive.id, slug) call within a single database transaction to ensure atomicity, so either both operations complete successfully or both are rolled back together.apps/web/src/app/api/mcp/drives/route.ts (1)
50-59:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake drive creation and subdomain allocation atomic.
allocatePublishSubdomainis a second write that can fail after the drive insert succeeds, which returns 500 with a partially initialized drive already committed. That breaks API-level idempotency and leaves inconsistent state.💡 Suggested fix
- const [newDrive] = await db.insert(drives).values({ - name, - slug, - ownerId: userId, - updatedAt: new Date(), - }).returning(); - - // Auto-allocate a globally-unique publish subdomain (idempotent with drive-service). - await allocatePublishSubdomain(newDrive.id, slug); + const newDrive = await db.transaction(async (tx) => { + const [createdDrive] = await tx.insert(drives).values({ + name, + slug, + ownerId: userId, + updatedAt: new Date(), + }).returning(); + + // Auto-allocate a globally-unique publish subdomain in the same transaction. + await allocatePublishSubdomain(createdDrive.id, slug, tx); + return createdDrive; + });🤖 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/api/mcp/drives/route.ts` around lines 50 - 59, The drive insertion and allocatePublishSubdomain calls are separate operations where the second can fail after the first succeeds, leaving the database in an inconsistent state. Wrap both the db.insert(drives) operation and the allocatePublishSubdomain call inside a database transaction to ensure they either both succeed or both fail atomically, preventing partial initialization and maintaining API-level idempotency.packages/lib/src/services/drive-service.ts (1)
161-184:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake drive creation + subdomain allocation atomic.
If allocation fails after insert, the API errors but leaves a persisted drive without
publishSubdomain. Wrap both operations in a single transaction and passtxtoallocatePublishSubdomain.💡 Proposed fix
export async function createDrive( userId: string, input: CreateDriveInput ): Promise<DriveWithAccess> { const { name } = input; const slug = slugify(name); - - const [newDrive] = await db - .insert(drives) - .values({ - name, - slug, - ownerId: userId, - isTrashed: false, - trashedAt: null, - updatedAt: new Date(), - }) - .returning(); - - await allocatePublishSubdomain(newDrive.id, slug); + const newDrive = await db.transaction(async (tx) => { + const [created] = await tx + .insert(drives) + .values({ + name, + slug, + ownerId: userId, + isTrashed: false, + trashedAt: null, + updatedAt: new Date(), + }) + .returning(); + + await allocatePublishSubdomain(created.id, slug, tx); + return created; + });🤖 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/lib/src/services/drive-service.ts` around lines 161 - 184, The createDrive function performs database insertion and subdomain allocation as separate operations, leaving the drive in an inconsistent state if allocation fails after insert. Wrap both the db.insert operation for the drives table and the allocatePublishSubdomain function call in a single database transaction. Update allocatePublishSubdomain to accept a transaction parameter (tx) and use that transaction for its database operations instead of the default db connection. This ensures both operations either succeed together or both fail and rollback, preventing orphaned drives without publishSubdomain values.
🧹 Nitpick comments (3)
scripts/backfill-publish-subdomains.ts (1)
86-89: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winRecord collision winners in the in-memory taken set after
23505.After a unique-violation, that candidate is now taken by another writer. Adding it to
takenavoids repeated selection of the same losing candidate later in the same run.♻️ Suggested change
if (code === '23505') { skipped += 1; + taken.add(a.subdomain); console.warn(` ⚠️ collision on "${a.subdomain}" for drive ${a.driveId} — skipped`); } else { throw err; }🤖 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 `@scripts/backfill-publish-subdomains.ts` around lines 86 - 89, When a unique-violation error with code '23505' is encountered, the collision is logged but the subdomain is not recorded in the in-memory taken set. After the console.warn statement that logs the collision on a.subdomain, add the subdomain to the taken set to prevent the same candidate from being selected again in the current run. This ensures that once a subdomain is claimed by another writer, it won't be attempted again within the same execution.scripts/lib/publish-subdomain-backfill.ts (1)
35-38: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAvoid rebuilding the full taken array for every drive.
Line 37 creates a fresh
[...]copy of the entireSeton each iteration, which makes this loop unnecessarily expensive as batch size grows.♻️ Suggested change
export function computePublishSubdomainBackfill( missing: DriveBackfillRow[], takenSubdomains: string[], ): DriveSubdomainBackfillResult[] { - const taken = new Set(takenSubdomains); + const taken = new Set(takenSubdomains); + const takenList = [...takenSubdomains]; const results: DriveSubdomainBackfillResult[] = []; for (const drive of missing) { if (drive.publishSubdomain) continue; // defensive: skip drives that already have one - const subdomain = resolveUniquePublishSubdomain(drive.slug, [...taken]); + const subdomain = resolveUniquePublishSubdomain(drive.slug, takenList); taken.add(subdomain); // reserve within-run so the next missing drive can't take it + takenList.push(subdomain); results.push({ driveId: drive.id, subdomain }); } return results; }🤖 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 `@scripts/lib/publish-subdomain-backfill.ts` around lines 35 - 38, The resolveUniquePublishSubdomain function call is spreading the taken Set into a fresh array on every iteration of the loop over missing drives, which becomes increasingly expensive as the batch size grows. Instead of creating [...taken] inside the loop iteration, either modify resolveUniquePublishSubdomain to accept the Set directly rather than an array, or if the function must receive an array parameter, reconstruct the array only once per iteration rather than spreading the Set repeatedly during the loop execution in the for loop over missing drives.packages/lib/src/services/__tests__/drive-subdomain-allocation.test.ts (1)
19-66: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd regression tests for wrapped unique violations and
attempt()return forwarding.These two cases are core to race recovery and would have caught the current helper contract gap earlier.
🤖 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/lib/src/services/__tests__/drive-subdomain-allocation.test.ts` around lines 19 - 66, Add two additional regression test cases to the drive-subdomain-allocation test suite. First, add a test that verifies the allocateUniqueSubdomainWithRetry function correctly handles wrapped unique violations where the error code '23505' is nested or wrapped within a parent error object, ensuring it properly detects and retries on such wrapped violations. Second, add a test that verifies the function correctly forwards the return value from the attempt() function through to the caller, ensuring that when attempt() succeeds, its actual return value (not just a fixed string like 'ok') is properly returned as the result of allocateUniqueSubdomainWithRetry.
🤖 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/lib/src/services/drive-service.ts`:
- Around line 515-523: The `fetchTaken` function in the drive-service.ts file
performs a full-table read of all allocated subdomains on every allocation
attempt, creating an O(total_drives) bottleneck. Instead of querying all rows
with non-null publishSubdomain values, modify the query to filter at the
database level by narrowing the candidate reads to only the specific base/suffix
family being allocated (e.g., adding a WHERE clause that matches the target
subdomain pattern), or alternatively implement a database-first conflict
detection strategy that checks for conflicts with only the specific subdomain
being created rather than loading all existing subdomains into memory.
In `@packages/lib/src/services/subdomain-allocation.ts`:
- Around line 18-23: The allocateUniqueSubdomainWithRetry function's attempt
callback parameter currently has a return type of Promise<void>, but the actual
implementation in drive-service.ts returns a string (the subdomain) in
race-recovery scenarios. Update the attempt callback type signature to
Promise<string | void> to match the actual behavior, then modify the function
implementation to capture and return the resolved value from the attempt() call
when it returns a string, falling back to the provisional candidate only when
the return value is void or undefined.
- Around line 4-6: The isUniqueViolation function only checks for the PostgreSQL
unique constraint error code at the top level but does not handle cases where
Drizzle ORM wraps the error in a cause property. Update the isUniqueViolation
function to recursively check both the direct error object and any nested error
stored in the cause property for the error code 23505, following the same
pattern already implemented in
apps/web/src/app/api/commands/command-route-helpers.ts.
In `@scripts/backfill-publish-subdomains.ts`:
- Around line 76-81: The update statement that sets publishSubdomain on the
drives table currently guards only by the drive ID in the where clause. To
prevent concurrent writes from overwriting a publishSubdomain value that was set
between the read and write operations, add an additional condition to the where
clause to ensure that publishSubdomain IS NULL before performing the update.
This will prevent the script from overwriting values set by concurrent writers
and ensures accurate allocation counting.
---
Outside diff comments:
In `@apps/web/src/app/api/mcp/drives/route.ts`:
- Around line 50-59: The drive insertion and allocatePublishSubdomain calls are
separate operations where the second can fail after the first succeeds, leaving
the database in an inconsistent state. Wrap both the db.insert(drives) operation
and the allocatePublishSubdomain call inside a database transaction to ensure
they either both succeed or both fail atomically, preventing partial
initialization and maintaining API-level idempotency.
In `@apps/web/src/lib/ai/tools/drive-tools.ts`:
- Around line 213-228: The insert operation on the drives table and the
subsequent allocatePublishSubdomain function call are not wrapped in a
transaction, creating a risk of partial writes if the subdomain allocation fails
after the insert succeeds. Wrap both the db.insert(drives).values() operation
and the allocatePublishSubdomain(newDrive.id, slug) call within a single
database transaction to ensure atomicity, so either both operations complete
successfully or both are rolled back together.
In `@packages/lib/src/services/drive-service.ts`:
- Around line 161-184: The createDrive function performs database insertion and
subdomain allocation as separate operations, leaving the drive in an
inconsistent state if allocation fails after insert. Wrap both the db.insert
operation for the drives table and the allocatePublishSubdomain function call in
a single database transaction. Update allocatePublishSubdomain to accept a
transaction parameter (tx) and use that transaction for its database operations
instead of the default db connection. This ensures both operations either
succeed together or both fail and rollback, preventing orphaned drives without
publishSubdomain values.
---
Nitpick comments:
In `@packages/lib/src/services/__tests__/drive-subdomain-allocation.test.ts`:
- Around line 19-66: Add two additional regression test cases to the
drive-subdomain-allocation test suite. First, add a test that verifies the
allocateUniqueSubdomainWithRetry function correctly handles wrapped unique
violations where the error code '23505' is nested or wrapped within a parent
error object, ensuring it properly detects and retries on such wrapped
violations. Second, add a test that verifies the function correctly forwards the
return value from the attempt() function through to the caller, ensuring that
when attempt() succeeds, its actual return value (not just a fixed string like
'ok') is properly returned as the result of allocateUniqueSubdomainWithRetry.
In `@scripts/backfill-publish-subdomains.ts`:
- Around line 86-89: When a unique-violation error with code '23505' is
encountered, the collision is logged but the subdomain is not recorded in the
in-memory taken set. After the console.warn statement that logs the collision on
a.subdomain, add the subdomain to the taken set to prevent the same candidate
from being selected again in the current run. This ensures that once a subdomain
is claimed by another writer, it won't be attempted again within the same
execution.
In `@scripts/lib/publish-subdomain-backfill.ts`:
- Around line 35-38: The resolveUniquePublishSubdomain function call is
spreading the taken Set into a fresh array on every iteration of the loop over
missing drives, which becomes increasingly expensive as the batch size grows.
Instead of creating [...taken] inside the loop iteration, either modify
resolveUniquePublishSubdomain to accept the Set directly rather than an array,
or if the function must receive an array parameter, reconstruct the array only
once per iteration rather than spreading the Set repeatedly during the loop
execution in the for loop over missing drives.
🪄 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: 2de8195e-e311-4a44-96ff-ce460a8c6c70
📒 Files selected for processing (13)
apps/admin/src/lib/onboarding/home-drive.tsapps/web/src/app/api/mcp/drives/route.tsapps/web/src/lib/ai/tools/drive-tools.tsapps/web/src/lib/onboarding/home-drive.tspackages/lib/package.jsonpackages/lib/src/services/__tests__/drive-subdomain-allocation.test.tspackages/lib/src/services/drive-service.tspackages/lib/src/services/subdomain-allocation.tspackages/lib/src/validators/__tests__/subdomain-allocator.test.tspackages/lib/src/validators/subdomain.tsscripts/__tests__/backfill-publish-subdomains.test.tsscripts/backfill-publish-subdomains.tsscripts/lib/publish-subdomain-backfill.ts
Round-2 review found a blocker in the round-1 fix: the race-recovery addition to allocatePublishSubdomain re-reads and returns the actual persisted subdomain on a lost race, but allocateUniqueSubdomainWithRetry ignored attempt()'s return value and always returned the locally-computed candidate. Result: in a real race the function could return a subdomain that was never written to disk (e.g. "acme-2" while the DB holds "acme"). Fixed by widening attempt's signature to Promise<string | void> and returning persisted ?? candidate — so the actual on-disk value always wins. Added a regression test for the race-return contract and updated the two existing tests whose mocks used a non-void success sentinel. 43 unit tests green (was 42). The fragile race path is now covered.
…uard Addresses the CI failure and the remaining CodeRabbit findings on #1670. CI failure (apps/admin Unit Tests): - apps/admin/home-drive.ts now imports @pagespace/lib/services/drive-service, but the admin vitest config only had explicit string aliases for a handful of lib subpaths — no alias for services/drive-service. Converted the alias block to the array form with a regex catch-all for @pagespace/lib/* at the end (mirrors the tsconfig paths glob), so new lib imports don't each need an explicit alias. CodeRabbit finding (Major — isUniqueViolation missed Drizzle-wrapped errors): - The shallow check only looked at err.code, but Drizzle wraps the underlying PostgresError in .cause. A wrapped 23505 would bypass the retry loop and rethrow instead of recovering. Rewrote isUniqueViolation to walk the .cause chain recursively (mirrors apps/web/.../command-route-helpers.ts). 5 new unit tests cover raw, wrapped, deep, and non-matching cases. - Deduped: the publish route's local copy of isUniqueViolation is replaced with the shared import (one canonical definition). CodeRabbit finding (Major — backfill update not guarded): - The backfill UPDATE only guarded by id, so a concurrent writer setting publishSubdomain between read and write could be clobbered. Now guarded with where(id AND isNull(publishSubdomain)) + returning; zero-row updates are skipped (lost race). Collision check uses the shared cause-walking isUniqueViolation instead of a shallow code check. 48 unit tests green (was 43).
Follow-up to the CI fix. After wiring allocatePublishSubdomain into the drive-creation paths, three unit tests failed to load because importing @pagespace/lib/services/drive-service pulls transitive DB-schema imports (mcpTokens, driveMembers, pagePermissions) that those tests don't mock. The unit tests are about provisioning/route/tool behavior, not the allocator's internals — so the correct seam is to mock drive-service at the module boundary and stub allocatePublishSubdomain as a no-op. - apps/admin + apps/web home-drive.test.ts: added drive-service mock (allocatePublishSubdomain -> resolved 'home'). Fixes the mcpTokens load error that failed CI Unit Tests. - apps/web mcp/drives route.test.ts: added allocatePublishSubdomain to the existing drive-service mock. - apps/web drive-tools.test.ts: added drive-service mock covering the full import surface the tool now uses (getDriveAccessWithDrive, getDriveById, isValidDriveHomePage, updateDrive, allocatePublishSubdomain).
The existing packages/lib drive-service.test.ts has two createDrive unit tests that broke because createDrive now calls allocatePublishSubdomain, which issues db.select/update queries the tests didn't mock. This was the real CI Unit Tests failure (the admin home-drive mock fix resolved the load error, then this surfaced). - Added isNull/gt/asc to the operators mock (allocatePublishSubdomain and the backfill use them). - Both createDrive tests now mock the allocator's early-return select (returns an already-allocated subdomain), so the retry/update path isn't exercised — these tests are about createDrive's slug/role behavior, not the allocator, which has its own coverage. 81 unit tests green across the four touched test files.
…aken, backfill nitpicks - wrap createDrive (drive-service, MCP route, AI tools) in db.transaction so drive insert + subdomain allocation are atomic; no orphaned drive on alloc failure - narrow fetchTaken to LIKE '<base>%' to avoid O(total_drives) full-table read - add taken.add() after 23505 in backfill to avoid reselecting known-taken candidates - maintain parallel takenList in computePublishSubdomainBackfill to avoid Set spread on every iteration - mock db.transaction in createDrive unit tests (follow credit-funding test pattern) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HkYZ2FTnMPy43B7eXEn2Ry
Owner
Author
Review fixes (39f91cd)Addressed all CodeRabbit review feedback: Major — transaction atomicity (3 locations):
These were previously "outside diff" comments. All three callers now atomically rollback if subdomain allocation fails after the drive insert. Major — Nitpick — backfill Nitpick — avoid |
…typecheck CI) Parameters<Parameters<typeof db.transaction>[0]>[0] gives the actual PgTransaction type, avoiding the implicit-any / incompatible-parameters TS error in strict CI. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HkYZ2FTnMPy43B7eXEn2Ry
… backfill - route.test.ts: db mock now includes transaction() so the 'allows normal drive names' test reaches and tests the full POST handler path (was TypeError before) - publish-subdomain-backfill.ts: remove the now-dead Set (was only used for [..taken] spread before the takenList refactor; takenList alone suffices) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HkYZ2FTnMPy43B7eXEn2Ry
This was referenced Jun 25, 2026
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
Foundation for wildcard subdomains → drive homepage (the CNAME-for-custom-domains unblock). Every drive now gets a globally-unique
<sub>.pagespace.siteidentity at creation, not lazily on first Canvas publish.Unblocks Eric Elliott's reported blocker: "the drive homepage isn't automatically served from
*.pagespace.site/by default. This blocks CNAMEs." This PR is Phase 1 (drive identity); Phase 2 (homepage auto-serve) and the CNAME payoff build on it.Why
publishSubdomainis allocated lazily, Canvas-only — drives with no published Canvas page have no subdomain identity, so<drive>.pagespace.site/can't resolve.drives.slugis not globally unique (only per-owner), so resolving by slug is ambiguous. A globally-unique subdomain per drive fixes this.Changes
Pure logic (TDD, fully unit-tested — 40 tests):
resolveUniquePublishSubdomain(base, taken)— normalize slug, de-dupe with-2/-3suffix (matches the codebase'sresolveUniqueSlugconvention), skip reserved/invalid candidates, clamp for length, fallback todriveon empty input.allocateUniqueSubdomainWithRetry— pure race-recovery: re-read taken subdomains on a unique-violation, advance suffix, retry. Separated into its own module so it's testable without a DB.computePublishSubdomainBackfill— pure batch allocator for the backfill script.DB-backed + wiring:
allocatePublishSubdomain(driveId, base, tx?)— race-safe DB wrapper, works in/out of a transaction.drive-service.createDrive, home-drive provisioning (web + admin), MCP/api/mcp/drives, AIdrive-tools.db.transactionso a partial write (drive created, allocation failed) is impossible.scripts/backfill-publish-subdomains.ts— one-shot backfill for existing drives (idempotent, collision-skipping). Follows thebackfill-home-drivestemplate exactly.Performance:
fetchTakeninallocatePublishSubdomainqueries only the base-family (LIKE 'acme%') instead of a full-table scan — O(family size) not O(total drives).computePublishSubdomainBackfillmaintains a paralleltakenListarray soSetspread is O(1) per drive, not O(n).Unchanged (deliberate): the existing explicit-subdomain publish path (user picks a custom name, 409 on collision) stays — auto-allocation handles the default; the explicit path still serves user choice.
Test plan
packages/lib— 40 unit tests pass (subdomain-allocator,subdomainvalidator,drive-subdomain-allocation)isUniqueViolationtests include Drizzle-wrapped cause chain (drive-subdomain-allocation.test.ts)attempt()return-value forwarding test (race-winner path)createDriveunit tests updated fordb.transaction(follow credit-funding mock pattern, correct Drizzle tx type)scripts/backfill-publish-subdomains— unit test passes once workspace deps are linked (same@pagespace/libresolution as the existingbackfill-home-drivestest; pre-existing worktree env limitation)/api/drives, MCP, AI tool) gets apublishSubdomainat creationacmeget distinct subdomainsbun scripts/backfill-publish-subdomains.ts --dry-runagainst a DB with legacy drivesOut of scope (follow-on PRs)
published/<sub>/index.html+ auto-refresh on edit*.pagespace.siteDNS → Tigris)customDomainstable + CNAME verificationNotes
publishSubdomaincolumn + unique constraint already exist (migration0144).🤖 Generated with Claude Code
https://claude.ai/code/session_01HkYZ2FTnMPy43B7eXEn2Ry