Add client-domain backup hosting#7
Conversation
|
🚅 Deployed to the dxd-webflow-scraper-pr-7 environment in WebFlow Archive
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis pull request introduces a static hosting/backup feature enabling sites to publish crawl archives to R2 and serve them via Cloudflare custom hostnames. It adds database tables for tracking publications and domains, a new Cloudflare hosting-worker, API routes for publication and domain management, background jobs for archive processing, and frontend UI for configuration and publishing workflows. Changes
Sequence DiagramssequenceDiagram
participant Client as Client
participant API as API Routes
participant Queue as Publication Queue
participant Storage as R2 Storage
participant DB as Database
Client->>API: POST /publish (siteId, crawlId)
API->>DB: Insert site_publications (pending)
API->>Queue: Enqueue publication job
Queue->>Queue: Process ZIP archive
Queue->>Storage: Download crawl ZIP
Queue->>Storage: Unzip & write files to R2
Queue->>DB: Update site_publications (published)
DB-->>API: Publication created
API-->>Client: Return publicationId
sequenceDiagram
participant User as User/Browser
participant HostingWorker as Hosting Worker
participant DB as Hyperdrive/Database
participant R2 as R2 Storage
participant CF as Cloudflare
User->>HostingWorker: GET backup.example.com/index.html
HostingWorker->>DB: Query siteDomains by hostname
DB-->>HostingWorker: Return site + activePublication
alt Active Publication Present
HostingWorker->>R2: Fetch object from r2Prefix/index.html
R2-->>HostingWorker: File stream + metadata
HostingWorker-->>User: 200 + file content + headers
else No Active Publication
HostingWorker-->>User: 404 + noindex headers
end
sequenceDiagram
participant Dashboard as Dashboard/UI
participant API as API Routes
participant CF as Cloudflare API
participant DB as Database
Dashboard->>API: POST /hosting/domains (hostname)
API->>DB: Insert siteDomains (provisioning)
API->>CF: createCustomHostname(hostname, target)
CF-->>API: Return ownership_verification, ssl_status
API->>DB: Update siteDomains (ownership + ssl fields)
DB-->>API: Domain record saved
API-->>Dashboard: Return domain + verification details
Dashboard-->>Dashboard: Display DNS/SSL verification status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 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)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c00dad83b
ℹ️ 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".
| } catch (error) { | ||
| return c.json({ error: "Hostname is already configured" }, 409); | ||
| } |
There was a problem hiding this comment.
Clean up Cloudflare hostname when domain insert fails
In POST /:siteId/domains, the Cloudflare custom hostname is provisioned before the DB write, but the insert failure path immediately returns 409 without deleting the Cloudflare resource. If two requests race on the same hostname or the insert fails for any DB reason, this leaves an orphaned custom-hostname in Cloudflare that still consumes quota and requires manual cleanup.
Useful? React with 👍 / 👎.
| const result = await syncCloudflareCustomHostname(c, domain.cloudflareHostnameId); | ||
| const fields = extractCloudflareDomainFields(result); | ||
| const [updated] = await db |
There was a problem hiding this comment.
Preserve Cloudflare linkage when sync is unavailable
This sync path updates the domain using extractCloudflareDomainFields(result) even when syncCloudflareCustomHostname returns null (which happens when Cloudflare env vars are missing). In that case, the update clears cloudflareHostnameId and resets status to pending_dns, so a single sync call in a misconfigured environment can permanently sever the record from its real Cloudflare hostname and prevent later cleanup/sync.
Useful? React with 👍 / 👎.
| await enqueuePublicationJob({ | ||
| siteId, | ||
| crawlId, | ||
| publicationId: publication.id, | ||
| activate: true, |
There was a problem hiding this comment.
Mark failed when auto-publication enqueue fails
enqueueAutoPublication inserts a site_publications row and then enqueues the publish job, but there is no local rollback/status update if enqueueing fails. Since the caller only logs and continues, Redis/queue outages leave permanently pending publications that never run, which makes publication history inaccurate and requires manual DB intervention.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/api/src/routes/sites.ts (1)
23-40:⚠️ Potential issue | 🟠 MajorPersist the new hosting fields on create.
The request schema accepts the hosting/billing fields, but the create path still drops them when inserting
sites. POST requests will silently losehostingAutoPublish,hostingBillingEmail,hostingPaymentLinkUrl, andhostingBillingStatus.Fix
const [site] = await db .insert(sites) .values({ name: data.name, url: data.url, concurrency: data.concurrency ?? 5, maxPages: data.maxPages, excludePatterns: data.excludePatterns, downloadBlacklist: normalizeDownloadBlacklistRules(data.downloadBlacklist), removeWebflowBadge: data.removeWebflowBadge ?? true, maxArchivesToKeep: data.maxArchivesToKeep ?? null, redirectsCsv: data.redirectsCsv, scheduleEnabled, scheduleCron, nextScheduledAt, storageType: data.storageType ?? "s3", storagePath: data.storagePath, + hostingAutoPublish: data.hostingAutoPublish ?? true, + hostingBillingEmail: data.hostingBillingEmail, + hostingPaymentLinkUrl: data.hostingPaymentLinkUrl, + hostingBillingStatus: data.hostingBillingStatus ?? "not_sent", })Also applies to: 97-114
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/routes/sites.ts` around lines 23 - 40, The create route is validating hosting fields in createSiteSchema but the site insert logic (the POST/create handler that calls into the sites insertion, e.g., the function that persists to the "sites" table) does not include hostingAutoPublish, hostingBillingEmail, hostingPaymentLinkUrl, or hostingBillingStatus, so those values are dropped; update the create handler to read these four fields from the validated input and include them in the object passed to the DB insert/update (same shape/names as in createSiteSchema) and ensure any nullable/optional handling (nullable strings, optional booleans/enums) matches the schema; also mirror the same change for the corresponding update path referenced in the comment block (lines ~97-114).apps/web/src/routes/sites/$siteId.tsx (1)
851-905:⚠️ Potential issue | 🟠 MajorMove the crawl actions out of the
Link.This row renders as a router
<Link>while also nesting a download<a>and a publish<button>inside it. That is invalid interactive nesting and can break keyboard/screen-reader behavior. Use a non-link container for the row, or extract the action buttons outside the clickable link.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/sites/`$siteId.tsx around lines 851 - 905, The row currently wraps interactive controls inside the router Link (the <Link> that maps to "/crawls/$crawlId"), which nests an <a> (crawlsApi.getDownloadUrl) and a <button> (publishMutation.mutate) and causes invalid interactive nesting; refactor the JSX so the clickable row (Link) only contains non-interactive content (e.g., StatusBadge, timestamp, page counts) and move the Download anchor and Publish button out of the Link into a sibling container (e.g., a div to the right) so they are independent elements; ensure the Download uses crawlsApi.getDownloadUrl(crawl.id) and the Publish button still calls publishMutation.mutate(crawl.id) with the same click handlers (preventDefault/stopPropagation where needed) and preserve styling and disabled state (publishMutation.isPending).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/queue/client.ts`:
- Around line 38-47: The code currently constructs a new BullMQ Queue named
"publication-jobs" inside addPublicationJob, causing Redis churn; instead create
and reuse a single publishQueue instance initialized once (e.g., alongside the
existing crawlQueue or as a top-level/module-scoped or class-level variable) and
remove the Queue construction from addPublicationJob so addPublicationJob simply
calls publishQueue.add(...). Locate the Queue construction for
"publication-jobs" and move its initialization to the module/class
initialization code where the crawl queue is created, ensure the variable name
publishQueue is used by addPublicationJob, and keep the existing
defaultJobOptions and jobId semantics.
In `@apps/api/src/routes/hosting.ts`:
- Around line 281-318: The code currently provisions the Cloudflare hostname via
createCloudflareCustomHostname and only then inserts the site_domains row, which
can orphan Cloudflare records on DB failure; change this by either (preferred)
inserting a reserved site_domains row first (use
db.insert(siteDomains).values({siteId, hostname, status: "provisioning",
...minimal fields}).returning()) and then call createCloudflareCustomHostname to
provision and update the row with cloudflareFields, or (if keeping current
order) narrow the catch around db.insert(siteDomains) to only treat
unique-constraint violations as 409 and, for any other insert error, call the
Cloudflare cleanup API to delete the created hostname (use
cloudflareFields.cloudflareHostnameId) before rethrowing/returning an error;
adjust the insert/update logic to update activePublicationId and cloudflare
fields after successful provisioning (referencing
createCloudflareCustomHostname, extractCloudflareDomainFields, cloudflareFields,
and db.insert(siteDomains)/db.update for locating code).
- Around line 350-369: The route handler for
app.post("/:siteId/domains/:domainId/sync") currently calls
extractCloudflareDomainFields() and updates the DB even when
syncCloudflareCustomHostname() returns null (missing Cloudflare credentials),
which clears Cloudflare metadata; change the handler to check the result of
syncCloudflareCustomHostname(c, domain.cloudflareHostnameId) and if it is null,
do not call extractCloudflareDomainFields or perform the update—instead return a
503 (or a no-op success) indicating Cloudflare config is unavailable; keep
references to syncCloudflareCustomHostname, extractCloudflareDomainFields, and
the update on siteDomains so you update only when a non-null result is returned.
- Around line 426-440: If existing.cloudflareHostnameId is present, ensure
Cloudflare cleanup succeeded before removing the DB row: call
deleteCloudflareCustomHostname(c, existing.cloudflareHostnameId) and check its
result/throwing behavior (or detect missing credentials) and if it fails or
indicates credentials are absent, return an error response and do not call
db.delete(siteDomains).where(...) — only proceed to delete when
deleteCloudflareCustomHostname reports success; use the existing variables and
functions (existing.cloudflareHostnameId, deleteCloudflareCustomHostname,
db.delete(siteDomains).where(...)) to implement this guard.
- Around line 45-165: The route file contains reusable Cloudflare and parsing
helpers that should be extracted into a separate module (e.g., hosting.utils.ts
or hosting.cloudflare.ts) to keep route handlers thin; move
getHostingCnameTarget, getCloudflareConfig, createCloudflareCustomHostname,
deleteCloudflareCustomHostname, syncCloudflareCustomHostname,
extractCloudflareDomainFields, and toOrigin into the new file, export them, and
replace the inline definitions in the route with imports from that module;
ensure all references in the route are updated to use the imported functions and
preserve existing behavior and error messages.
- Around line 60-141: The three functions createCloudflareCustomHostname,
deleteCloudflareCustomHostname, and syncCloudflareCustomHostname make fetch
calls with no timeout; wrap each fetch with an AbortController (create
controller, set a timer like 5s to call controller.abort(), pass
controller.signal to fetch) and clear the timer after the response is received;
catch aborts (check for DOMException/AbortError) and throw a clear error like
"Cloudflare request timed out" so callers can handle it. Ensure the same pattern
is applied in all three functions and that the controller is cleaned up on both
success and error paths.
In `@apps/web/src/routes/sites/`$siteId.tsx:
- Around line 738-749: The input uses defaultValue and must be made controlled:
introduce local state (e.g., redirectTarget, setRedirectTarget) initialized from
domain.redirectTargetOrigin || site.url, replace defaultValue with
value={redirectTarget}, update redirectTarget in the input's onChange handler,
call updateDomainMutation.mutate(...) on onBlur only if redirectTarget.trim()
differs from domain.redirectTargetOrigin || site.url, and add a useEffect that
resets redirectTarget whenever domain.redirectTargetOrigin or site.url changes
to keep the field in sync after refetches or syncDomainMutation.
- Around line 25-29: The polling query (useQuery with queryKey ["hosting",
siteId] returning hostingData) is overwriting in-progress edits by rehydrating
controlled inputs every refetch; change the hydration logic to only populate
form state on first successful load or when the form is clean: add local form
state (e.g., hostingFormState) and a boolean flag (e.g., isInitialized or
isDirty), set hostingFormState from hostingData only when hostingData exists and
isInitialized is false (or when isDirty is false), and mark isInitialized true
after the first hydrate; also update any useEffect that currently copies
hostingData into inputs (and the same pattern referenced around the other
hydration) to respect the isInitialized/isDirty guard so polling no longer
overwrites user edits.
In `@services/worker/src/processor.ts`:
- Around line 1350-1357: The new publication-jobs Worker (publicationWorker) and
its long-running processPublicationJob can leave DB rows stuck in "publishing"
if the worker dies; add crash-recovery by extending or reusing
reconcileOrphanedCrawls() to also detect publication rows stuck in "publishing"
(using PublicationJobData / the same DB model/table the worker updates) and
either mark them failed or transition them back to a requeueable state on
startup, ensuring this reconciliation runs before creating the
publicationWorker; update startup sequence to call the new
reconcileOrphanedPublications (or expand reconcileOrphanedCrawls) and perform
transitions inside the same transaction/locking semantics used elsewhere to
avoid races.
- Around line 733-799: The publication path in processPublicationJob performs
heavy download/unzip/upload work but runs without the existing exclusive runner;
wrap the core publish sequence (the archiveExists check, publishZipArchiveToR2
call and the subsequent sitePublications/siteDomains updates) inside the
runExclusiveWorkerJob(...) call so the publish runs exclusively (use
publicationId or a "publish:{publicationId}" key for exclusivity) and surface
errors back to the outer catch so existing failure handling remains unchanged;
adjust placement so quick DB checks (finding publication/crawl) remain outside
but the heavy publish+upload+update logic uses runExclusiveWorkerJob in place of
the current direct calls.
---
Outside diff comments:
In `@apps/api/src/routes/sites.ts`:
- Around line 23-40: The create route is validating hosting fields in
createSiteSchema but the site insert logic (the POST/create handler that calls
into the sites insertion, e.g., the function that persists to the "sites" table)
does not include hostingAutoPublish, hostingBillingEmail, hostingPaymentLinkUrl,
or hostingBillingStatus, so those values are dropped; update the create handler
to read these four fields from the validated input and include them in the
object passed to the DB insert/update (same shape/names as in createSiteSchema)
and ensure any nullable/optional handling (nullable strings, optional
booleans/enums) matches the schema; also mirror the same change for the
corresponding update path referenced in the comment block (lines ~97-114).
In `@apps/web/src/routes/sites/`$siteId.tsx:
- Around line 851-905: The row currently wraps interactive controls inside the
router Link (the <Link> that maps to "/crawls/$crawlId"), which nests an <a>
(crawlsApi.getDownloadUrl) and a <button> (publishMutation.mutate) and causes
invalid interactive nesting; refactor the JSX so the clickable row (Link) only
contains non-interactive content (e.g., StatusBadge, timestamp, page counts) and
move the Download anchor and Publish button out of the Link into a sibling
container (e.g., a div to the right) so they are independent elements; ensure
the Download uses crawlsApi.getDownloadUrl(crawl.id) and the Publish button
still calls publishMutation.mutate(crawl.id) with the same click handlers
(preventDefault/stopPropagation where needed) and preserve styling and disabled
state (publishMutation.isPending).
🪄 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: ASSERTIVE
Plan: Pro
Run ID: e5f3e47d-7bc4-4bd8-9691-03bea5c8b45a
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
SETUP-INFRASTRUCTURE.mdapps/api/src/app.tsapps/api/src/db/migrations/0003_add_static_hosting.sqlapps/api/src/db/migrations/0004_add_hosting_controls_and_billing.sqlapps/api/src/db/migrations/0005_add_hosting_redirect_mode.sqlapps/api/src/db/migrations/meta/_journal.jsonapps/api/src/db/schema.tsapps/api/src/env.tsapps/api/src/queue/client.tsapps/api/src/routes/hosting.tsapps/api/src/routes/sites.tsapps/api/wrangler.tomlapps/hosting-worker/package.jsonapps/hosting-worker/src/index.tsapps/hosting-worker/tsconfig.jsonapps/hosting-worker/wrangler.tomlapps/web/src/lib/api.tsapps/web/src/routes/sites/$siteId.tsxpackages/db/src/schema.tsservices/worker/package.jsonservices/worker/src/db.tsservices/worker/src/http.tsservices/worker/src/processor.ts
| async addPublicationJob(siteId, crawlId, publicationId, activate, autoPublish) { | ||
| const publishQueue = new Queue("publication-jobs", { | ||
| connection: redis, | ||
| defaultJobOptions: { | ||
| removeOnComplete: 100, | ||
| removeOnFail: 100, | ||
| attempts: 1, | ||
| }, | ||
| }); | ||
| await publishQueue.add("publish", { siteId, crawlId, publicationId, activate, autoPublish: autoPublish ?? false }, { jobId: publicationId }); |
There was a problem hiding this comment.
Reuse a single publication queue instance.
Creating a new BullMQ Queue on every addPublicationJob() call adds avoidable Redis churn and can leak queue resources over time. This should be initialized once alongside the crawl queue and reused.
♻️ Suggested fix
export function createNodeQueueClient(redisUrl: string): QueueClient {
const redis = new Redis(redisUrl, { maxRetriesPerRequest: null });
const queue = new Queue("crawl-jobs", {
connection: redis,
@@
});
+ const publicationQueue = new Queue("publication-jobs", {
+ connection: redis,
+ defaultJobOptions: {
+ removeOnComplete: 100,
+ removeOnFail: 100,
+ attempts: 1,
+ },
+ });
return {
@@
},
async addPublicationJob(siteId, crawlId, publicationId, activate, autoPublish) {
- const publishQueue = new Queue("publication-jobs", {
- connection: redis,
- defaultJobOptions: {
- removeOnComplete: 100,
- removeOnFail: 100,
- attempts: 1,
- },
- });
- await publishQueue.add("publish", { siteId, crawlId, publicationId, activate, autoPublish: autoPublish ?? false }, { jobId: publicationId });
+ await publicationQueue.add("publish", { siteId, crawlId, publicationId, activate, autoPublish: autoPublish ?? false }, { jobId: publicationId });
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/queue/client.ts` around lines 38 - 47, The code currently
constructs a new BullMQ Queue named "publication-jobs" inside addPublicationJob,
causing Redis churn; instead create and reuse a single publishQueue instance
initialized once (e.g., alongside the existing crawlQueue or as a
top-level/module-scoped or class-level variable) and remove the Queue
construction from addPublicationJob so addPublicationJob simply calls
publishQueue.add(...). Locate the Queue construction for "publication-jobs" and
move its initialization to the module/class initialization code where the crawl
queue is created, ensure the variable name publishQueue is used by
addPublicationJob, and keep the existing defaultJobOptions and jobId semantics.
| function getHostingCnameTarget(c: { env?: { HOSTING_CNAME_TARGET?: string } }): string { | ||
| const target = c.env?.HOSTING_CNAME_TARGET || process.env.HOSTING_CNAME_TARGET; | ||
| if (!target) { | ||
| throw new Error("HOSTING_CNAME_TARGET is required to create hosted domains"); | ||
| } | ||
| return target.replace(/^https?:\/\//, "").replace(/\/+$/, "").toLowerCase(); | ||
| } | ||
|
|
||
| function getCloudflareConfig(c: { env?: { CLOUDFLARE_ZONE_ID?: string; CLOUDFLARE_API_TOKEN?: string } }) { | ||
| const zoneId = c.env?.CLOUDFLARE_ZONE_ID || process.env.CLOUDFLARE_ZONE_ID; | ||
| const apiToken = c.env?.CLOUDFLARE_API_TOKEN || process.env.CLOUDFLARE_API_TOKEN; | ||
| if (!zoneId || !apiToken) return null; | ||
| return { zoneId, apiToken }; | ||
| } | ||
|
|
||
| async function createCloudflareCustomHostname( | ||
| c: { env?: { CLOUDFLARE_ZONE_ID?: string; CLOUDFLARE_API_TOKEN?: string } }, | ||
| hostname: string | ||
| ) { | ||
| const config = getCloudflareConfig(c); | ||
| if (!config) return null; | ||
|
|
||
| const response = await fetch(`https://api.cloudflare.com/client/v4/zones/${config.zoneId}/custom_hostnames`, { | ||
| method: "POST", | ||
| headers: { | ||
| Authorization: `Bearer ${config.apiToken}`, | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: JSON.stringify({ | ||
| hostname, | ||
| ssl: { | ||
| method: "txt", | ||
| type: "dv", | ||
| settings: { | ||
| min_tls_version: "1.2", | ||
| }, | ||
| }, | ||
| }), | ||
| }); | ||
|
|
||
| const payload = (await response.json().catch(() => null)) as any; | ||
| if (!response.ok || payload?.success === false) { | ||
| const message = payload?.errors?.[0]?.message || `Cloudflare custom hostname failed with ${response.status}`; | ||
| throw new Error(message); | ||
| } | ||
|
|
||
| return payload?.result ?? null; | ||
| } | ||
|
|
||
| async function deleteCloudflareCustomHostname( | ||
| c: { env?: { CLOUDFLARE_ZONE_ID?: string; CLOUDFLARE_API_TOKEN?: string } }, | ||
| cloudflareHostnameId: string | ||
| ) { | ||
| const config = getCloudflareConfig(c); | ||
| if (!config) return; | ||
|
|
||
| const response = await fetch( | ||
| `https://api.cloudflare.com/client/v4/zones/${config.zoneId}/custom_hostnames/${cloudflareHostnameId}`, | ||
| { | ||
| method: "DELETE", | ||
| headers: { | ||
| Authorization: `Bearer ${config.apiToken}`, | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| const payload = (await response.json().catch(() => null)) as any; | ||
| if (!response.ok || payload?.success === false) { | ||
| const message = payload?.errors?.[0]?.message || `Cloudflare custom hostname delete failed with ${response.status}`; | ||
| throw new Error(message); | ||
| } | ||
| } | ||
|
|
||
| async function syncCloudflareCustomHostname( | ||
| c: { env?: { CLOUDFLARE_ZONE_ID?: string; CLOUDFLARE_API_TOKEN?: string } }, | ||
| cloudflareHostnameId: string | ||
| ) { | ||
| const config = getCloudflareConfig(c); | ||
| if (!config) return null; | ||
|
|
||
| const response = await fetch( | ||
| `https://api.cloudflare.com/client/v4/zones/${config.zoneId}/custom_hostnames/${cloudflareHostnameId}`, | ||
| { | ||
| headers: { | ||
| Authorization: `Bearer ${config.apiToken}`, | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| const payload = (await response.json().catch(() => null)) as any; | ||
| if (!response.ok || payload?.success === false) { | ||
| const message = payload?.errors?.[0]?.message || `Cloudflare custom hostname sync failed with ${response.status}`; | ||
| throw new Error(message); | ||
| } | ||
|
|
||
| return payload?.result ?? null; | ||
| } | ||
|
|
||
| function extractCloudflareDomainFields(result: any) { | ||
| const ownership = result?.ownership_verification ?? result?.ownership_verification_http; | ||
| const ssl = result?.ssl; | ||
| const status = result?.status === "active" && ssl?.status === "active" ? "active" : "pending_dns"; | ||
|
|
||
| return { | ||
| cloudflareHostnameId: typeof result?.id === "string" ? result.id : null, | ||
| ownershipVerificationName: typeof ownership?.name === "string" ? ownership.name : null, | ||
| ownershipVerificationValue: typeof ownership?.value === "string" ? ownership.value : null, | ||
| sslStatus: typeof ssl?.status === "string" ? ssl.status : null, | ||
| status, | ||
| }; | ||
| } | ||
|
|
||
| function toOrigin(value: string | null | undefined): string | null { | ||
| if (!value) return null; | ||
| try { | ||
| return new URL(value).origin; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Move the Cloudflare/hostname helpers out of the route module.
This file is carrying reusable parsing and Cloudflare service logic inline, which makes the route layer much fatter than the repo rule allows. Please move these helpers into hosting.utils.ts/hosting.cloudflare.ts next to the route and keep the handlers focused on request/response orchestration.
As per coding guidelines: apps/api/src/routes/**/*.ts: Keep route logic in apps/api/src/routes thin; move reusable logic into *.utils.ts files next to routes
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/routes/hosting.ts` around lines 45 - 165, The route file
contains reusable Cloudflare and parsing helpers that should be extracted into a
separate module (e.g., hosting.utils.ts or hosting.cloudflare.ts) to keep route
handlers thin; move getHostingCnameTarget, getCloudflareConfig,
createCloudflareCustomHostname, deleteCloudflareCustomHostname,
syncCloudflareCustomHostname, extractCloudflareDomainFields, and toOrigin into
the new file, export them, and replace the inline definitions in the route with
imports from that module; ensure all references in the route are updated to use
the imported functions and preserve existing behavior and error messages.
| async function createCloudflareCustomHostname( | ||
| c: { env?: { CLOUDFLARE_ZONE_ID?: string; CLOUDFLARE_API_TOKEN?: string } }, | ||
| hostname: string | ||
| ) { | ||
| const config = getCloudflareConfig(c); | ||
| if (!config) return null; | ||
|
|
||
| const response = await fetch(`https://api.cloudflare.com/client/v4/zones/${config.zoneId}/custom_hostnames`, { | ||
| method: "POST", | ||
| headers: { | ||
| Authorization: `Bearer ${config.apiToken}`, | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: JSON.stringify({ | ||
| hostname, | ||
| ssl: { | ||
| method: "txt", | ||
| type: "dv", | ||
| settings: { | ||
| min_tls_version: "1.2", | ||
| }, | ||
| }, | ||
| }), | ||
| }); | ||
|
|
||
| const payload = (await response.json().catch(() => null)) as any; | ||
| if (!response.ok || payload?.success === false) { | ||
| const message = payload?.errors?.[0]?.message || `Cloudflare custom hostname failed with ${response.status}`; | ||
| throw new Error(message); | ||
| } | ||
|
|
||
| return payload?.result ?? null; | ||
| } | ||
|
|
||
| async function deleteCloudflareCustomHostname( | ||
| c: { env?: { CLOUDFLARE_ZONE_ID?: string; CLOUDFLARE_API_TOKEN?: string } }, | ||
| cloudflareHostnameId: string | ||
| ) { | ||
| const config = getCloudflareConfig(c); | ||
| if (!config) return; | ||
|
|
||
| const response = await fetch( | ||
| `https://api.cloudflare.com/client/v4/zones/${config.zoneId}/custom_hostnames/${cloudflareHostnameId}`, | ||
| { | ||
| method: "DELETE", | ||
| headers: { | ||
| Authorization: `Bearer ${config.apiToken}`, | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| const payload = (await response.json().catch(() => null)) as any; | ||
| if (!response.ok || payload?.success === false) { | ||
| const message = payload?.errors?.[0]?.message || `Cloudflare custom hostname delete failed with ${response.status}`; | ||
| throw new Error(message); | ||
| } | ||
| } | ||
|
|
||
| async function syncCloudflareCustomHostname( | ||
| c: { env?: { CLOUDFLARE_ZONE_ID?: string; CLOUDFLARE_API_TOKEN?: string } }, | ||
| cloudflareHostnameId: string | ||
| ) { | ||
| const config = getCloudflareConfig(c); | ||
| if (!config) return null; | ||
|
|
||
| const response = await fetch( | ||
| `https://api.cloudflare.com/client/v4/zones/${config.zoneId}/custom_hostnames/${cloudflareHostnameId}`, | ||
| { | ||
| headers: { | ||
| Authorization: `Bearer ${config.apiToken}`, | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| const payload = (await response.json().catch(() => null)) as any; | ||
| if (!response.ok || payload?.success === false) { | ||
| const message = payload?.errors?.[0]?.message || `Cloudflare custom hostname sync failed with ${response.status}`; | ||
| throw new Error(message); | ||
| } | ||
|
|
||
| return payload?.result ?? null; | ||
| } |
There was a problem hiding this comment.
Put explicit timeouts on the Cloudflare API calls.
createCloudflareCustomHostname(), deleteCloudflareCustomHostname(), and syncCloudflareCustomHostname() issue blocking network requests from the request path with no timeout or abort signal. A slow Cloudflare edge/API incident can pin these routes until the platform times them out.
Possible fix
+function withTimeout(ms = 10000): AbortSignal {
+ return AbortSignal.timeout(ms);
+}
+
const response = await fetch(`https://api.cloudflare.com/client/v4/zones/${config.zoneId}/custom_hostnames`, {
method: "POST",
+ signal: withTimeout(),
headers: {
Authorization: `Bearer ${config.apiToken}`,
"Content-Type": "application/json",
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/routes/hosting.ts` around lines 60 - 141, The three functions
createCloudflareCustomHostname, deleteCloudflareCustomHostname, and
syncCloudflareCustomHostname make fetch calls with no timeout; wrap each fetch
with an AbortController (create controller, set a timer like 5s to call
controller.abort(), pass controller.signal to fetch) and clear the timer after
the response is received; catch aborts (check for DOMException/AbortError) and
throw a clear error like "Cloudflare request timed out" so callers can handle
it. Ensure the same pattern is applied in all three functions and that the
controller is cleaned up on both success and error paths.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/setup-backup-hosting.md`:
- Around line 147-154: The ordered list uses distinct numeric prefixes (1, 2, 3)
which violates MD029; change each list item in the block that contains
"Enable/configure Cloudflare for SaaS / Custom Hostnames.", "Set the fallback
origin/target to:", and "Ensure custom hostnames can be created for client
domains like:" so every item uses the consistent unordered-style ordered marker
"1." (i.e., replace the 2. and 3. prefixes with 1.) to satisfy the MD029 rule
while leaving the content unchanged.
- Around line 253-268: The numbered steps in the backup-hosting flow trigger
MD029 because numbering is sequential across fenced blocks; normalize the
ordered list by changing every list item in that flow (the block starting "Open
the site in the internal dashboard." through "Publish a completed crawl or wait
for the next successful crawl to auto-publish.") to use "1." for each item so
all lines use the same numbered marker style; update the CNAME and TXT
instruction lines and any fenced code blocks in that section accordingly to
match the "1." style.
In `@services/worker/Dockerfile`:
- Line 17: Replace the non-deterministic dependency installation in the
Dockerfile that currently runs "bun install" with a locked-install invocation so
builds use the committed lockfile; update the RUN layer that invokes bun to use
the equivalent of a frozen lockfile flag (e.g., "--frozen-lockfile") and ensure
the repository's bun.lockb is copied into the build context before that RUN step
so the build fails if the lockfile and package.json drift.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 54d43fe7-7280-42e1-9490-e38649ff768b
📒 Files selected for processing (2)
docs/setup-backup-hosting.mdservices/worker/Dockerfile
| COPY services/worker/package.json services/worker/package.json | ||
|
|
||
| RUN bun install --frozen-lockfile | ||
| RUN bun install |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Dockerfile bun install commands =="
rg -n --iglob 'Dockerfile*' '\bbun install\b'
echo
echo "== Lockfile presence at repo root =="
fd -HI '^(bun\.lock|bun\.lockb)$'
echo
echo "== Non-exact dependency ranges in services/worker/package.json =="
python - <<'PY'
import json, pathlib
p = pathlib.Path("services/worker/package.json")
j = json.loads(p.read_text())
for section in ("dependencies", "devDependencies"):
for name, ver in j.get(section, {}).items():
if isinstance(ver, str) and ver[:1] in "^~><*":
print(f"{section}.{name} = {ver}")
PYRepository: designxdevelop/dxd-webflow-scraper
Length of output: 795
Reinstate lockfile enforcement for reproducible worker images.
Line 17 uses plain bun install without --frozen-lockfile, allowing dependency drift across builds given the caret ranges in package.json (e.g., bullmq ^5.52.0 can pull 5.53.0 on next build). This produces non-deterministic container artifacts.
Suggested fix
-RUN bun install
+RUN bun install --frozen-lockfile🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/worker/Dockerfile` at line 17, Replace the non-deterministic
dependency installation in the Dockerfile that currently runs "bun install" with
a locked-install invocation so builds use the committed lockfile; update the RUN
layer that invokes bun to use the equivalent of a frozen lockfile flag (e.g.,
"--frozen-lockfile") and ensure the repository's bun.lockb is copied into the
build context before that RUN step so the build fails if the lockfile and
package.json drift.
|
@cursoragent please fix all open comments/review for correctness. |
|
Implemented and pushed fixes for the currently open review comments on branch What was fixed
Verification runExecuted successfully:
(Worker typecheck initially needed Git / PR
|
Co-authored-by: Austin Thesing <athesing-point@users.noreply.github.com>


Summary
Verification
Summary by CodeRabbit
New Features
Documentation