Skip to content

Preview Database Isolation Fix#2185

Merged
shomix merged 6 commits into
mainfrom
shomix-p-fix-netlify-neon-preview-bug
Jul 17, 2026
Merged

Preview Database Isolation Fix#2185
shomix merged 6 commits into
mainfrom
shomix-p-fix-netlify-neon-preview-bug

Conversation

@shomix

@shomix shomix commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Every PR preview was sharing one database connection. When two PRs were open at the same time, opening or closing one could point another PR's preview at the wrong database — or break it entirely. This showed up as failed sign-ins and 502 errors on active previews, even though the setup workflow reported success.

The fix

Each PR preview now gets its own database, scoped to that PR's branch. Opening, updating, or closing one PR no longer affects any other PR's preview. Production is unaffected.

Everything is contained in .github/workflows/neon-preview-branches.yml:

  • When a PR opens, its database URL is saved as a value tied to that PR's branch, so PRs can't overwrite each other.
  • When a PR closes, only that PR's own database URL is removed. Other PRs and production keep theirs.
  • The workflow now fails loudly if the Netlify API call fails, instead of reporting a false success.
  • Database connection strings are stored as secrets.

Testing

Verified with two concurrent preview PRs: each had its own working database and sign-in, and opening or closing one did not affect the other.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

builder-io-integration[bot]

This comment was marked as outdated.

@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Here's a visual recap of what changed:

Visual recap

Open the full interactive recap

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

builder-io-integration[bot]

This comment was marked as outdated.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

Comment thread templates/clips/app/routes/r.$recordingId.tsx Outdated
builder-io-integration[bot]

This comment was marked as outdated.

builder-io-integration[bot]

This comment was marked as outdated.

@shomix
shomix requested a review from steve8708 July 16, 2026 18:01
@shomix
shomix requested a review from 3mdistal July 16, 2026 18:01

@builder-io-integration builder-io-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Builder reviewed your changes and found 2 potential issues 🔴

Review Details

Incremental Code Review Summary

The latest commit replaces the prior REVIEW_ID/fail-closed deployment approach with Netlify branch-scoped environment values. This removes the need to retry a failed preview and preserves the existing runtime variable names, while surgical deletion avoids removing values belonging to other branches. The workflow's explicit HTTP checks, jq payload construction, secret marking, and branch filtering are good patterns. One new standard-risk correctness issue remains in the POST/PATCH update path.

Key Finding

  • 🟡 MEDIUM — Repeated PR updates may create duplicate branch-scoped Netlify values, while cleanup deletes only the first matching value. This can leave stale database credentials accumulated in the site environment and make subsequent value selection ambiguous.

The previously open preview-retry comment was verified fixed and resolved before this review.

🧪 Browser testing: Will run after this review (PR changes deployment/runtime configuration)

Comment thread .github/workflows/neon-preview-branches.yml
Comment thread .github/workflows/neon-preview-branches.yml
@shomix
shomix merged commit 666b78f into main Jul 17, 2026
89 checks passed
@shomix
shomix deleted the shomix-p-fix-netlify-neon-preview-bug branch July 17, 2026 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants