feat(sites): gate previews behind a custom domain - #142
Conversation
|
@codex review |
🦋 Changeset detectedLatest commit: 4acf2af The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Greptile SummaryThe PR moves site previews from path-based URLs to custom-domain wildcard hosts.
Confidence Score: 4/5The PR should not merge until preview-domain reconciliation preserves the intended primary domain when multiple custom domains are attached. Adding another custom domain creates another preview wildcard, but reconciliation chooses whichever wildcard appears first and overwrites the recorded domain, so subsequent deploys can report and persist URLs under the wrong domain. Files Needing Attention: packages/cli/src/commands/sites/api.ts and packages/cli/src/commands/sites/domains/index.ts
|
| Filename | Overview |
|---|---|
| packages/cli/src/commands/sites/api.ts | Adds pull-zone hostname reconciliation, but selecting the first wildcard can overwrite the intended primary domain when multiple domains exist. |
| packages/cli/src/commands/sites/deploy.ts | Makes deployment mode depend on reconciled preview infrastructure and emits HTTP previews while wildcard TLS is pending. |
| packages/cli/src/commands/sites/domains/index.ts | Makes wildcard attachment and failed-state-write recovery retryable while supporting pending certificates. |
| packages/cli/src/core/hostnames/client.ts | Makes hostname attachment idempotent by verifying rejected duplicates against the pull zone. |
| packages/cli/src/commands/sites/router/source.ts | Removes path previews and HTML rewriting in favor of root-served production and preview hosts. |
| packages/cli/src/commands/sites/ci/workflow.ts | Generates domain-aware workflows that omit pull-request previews for domainless sites. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
D[sites deploy] --> H[Fetch pull-zone hostnames]
H --> W{Preview wildcard attached?}
W -->|No| P[Publish deployment to production]
W -->|Yes| R[Reconcile state.domain]
R --> F{--production?}
F -->|Yes| P
F -->|No| V[Serve immutable dpl-ID preview host]
A[sites domains add] --> X[Attach apex]
X --> Y[Attach preview wildcard]
Y --> S[Attempt wildcard certificate]
S --> R
Reviews (7): Last reviewed commit: "fix idempotent domain adds and scheme-aw..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: a9a41b4f29
ℹ️ 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".
| const { state, connection } = site; | ||
|
|
||
| // No custom domain means no preview hosts, so every deploy publishes; with a domain, previews are the default and --production is the publish switch. | ||
| const publish = args.production === true || !state.domain; |
There was a problem hiding this comment.
When custom-domain setup attaches the hostnames but recordSiteDomain cannot persist _bunny/site.json, that helper only warns and returns success, so create.ts can still scaffold a PR-preview workflow. On a PR, workflow.ts supplies production: false, but this expression treats the missing state.domain as publish = true and promotes the PR deploy to production. Do not infer the absence of an attached domain solely from this best-effort state field, or make domain-state persistence failure prevent preview CI from being enabled.
AGENTS.md reference: AGENTS.md:L402-L402
Useful? React with 👍 / 👎.
| @@ -1,12 +1,10 @@ | |||
| // The site's middleware Edge Script: maps hosts to `/deploys/{id}/` origin paths and rewrites root-absolute asset URLs in path-preview HTML (see AGENTS.md and the SOURCE comments below). BunnySDK hook and HTMLRewriter names are the platform contract. | |||
| // The site's middleware Edge Script: maps the apex to the published deploy dir and `dpl-{id}.preview.*` hosts to theirs (see AGENTS.md and the SOURCE comments below). BunnySDK hook names are the platform contract. | |||
There was a problem hiding this comment.
When adding *.preview.<domain> fails, attachPreviewWildcard catches the error and setupSiteDomain still records the apex domain. The router now supports only preview-host routing, so subsequent default deploys report a dpl-….preview.… URL even though the pull zone never accepted that wildcard; the existing failure message still promises the removed /deploys/<id>/ fallback. Propagate the wildcard failure or avoid enabling preview mode until a usable route exists.
AGENTS.md reference: AGENTS.md:L390-L390
Useful? React with 👍 / 👎.
| // Nothing to do: the deploy is already uploaded (and live, if publishing). | ||
| if (skipUpload && (alreadyLive || !publish)) { |
There was a problem hiding this comment.
When a domainless site's content matches its current live deploy, this no-op branch returns before the newly added domainless-site block, so repeat deploys never print the promised sites domains add hint. This is the common path after a user declines the one-time first-deploy prompt, leaving them without the intended follow-up guidance; emit the hint before returning from this branch.
AGENTS.md reference: AGENTS.md:L402-L402
Useful? React with 👍 / 👎.
|
@greptile-apps re-review |
| const wildcard = hostnames.find( | ||
| (h) => previewDomainFromWildcard(h.Value ?? "") !== undefined, | ||
| ); |
There was a problem hiding this comment.
When a site has multiple custom domains, previewZone selects the first preview wildcard returned by the pull-zone API rather than preserving the recorded primary domain. The subsequent reconciliation overwrites state.domain, causing production and preview URLs to use an unintended domain whose DNS may not be live.
Knowledge Base Used: CLI commands: sites (static site hosting/deploy)
2 participants
Domainless deploys now publish straight to production; attaching a domain unlocks the preview/--production flow via root-served
dpl-{id}.preview.{domain}hosts, so client-side routers (TanStack, React Router) work like production.Drop the
/deploys/{id}/path previews and the router's HTMLRewriter (broke SPA route matching; old deploys are no longer publicly browsable).sites ci initonly scaffolds PR preview deploys when a domain exists, otherwise the workflow deploys production on pushes to main only. A domainless site's first deploy offers to attach a domain and later deploys hint.