Skip to content

fix(ci): run the format check on docs-only PRs and migrate against the direct endpoint - #5168

Merged
steve8708 merged 2 commits into
mainfrom
steve8708/beta-migration-and-lint-gate
Sep 16, 2026
Merged

steve8708 merged 2 commits into
mainfrom
steve8708/beta-migration-and-lint-gate

Conversation

@steve8708

Copy link
Copy Markdown
Contributor

Two independent CI/deploy fixes for failures seen on 2026-09-15/16. Both are boundary fixes with tests that fail without them.

1. Docs-only PRs skipped Lint & format, so unformatted docs reached main

classifyChangedPaths forced every check to false when a change set was docs-only. But fmt:check runs oxfmt --check . over the whole tree, including .md/.mdx, so docs are exactly what a docs-only PR can still break. Unformatted docs landed on main three times on 09-15 (#3903, #4999, #5075) and turned the required Lint & format check red on every unrelated open PR until someone noticed and fixed main (#5106, #5135, #5141).

Fix: keep the docs-only short-circuit, but leave lint on. Two existing assertions encoded the old "docs-only selects nothing" contract and are updated to "selects lint only".

Red/Green (git apply -R of only the ci-change-scope.ts hunk):

  • Red: ✖ keeps the format check on for a docs-only change set, 3 failures.
  • Green: 15/15 pass.

2. Beta release migrations could run against a pooled endpoint and fail read-only

netlify-migration-url.ts returned the first postgres… URL it found and never rejected a pooled host. Neon's PgBouncer runs in transaction mode, so a pooled session can land read-only and reject DDL with cannot execute UPDATE/CREATE TABLE in a read-only transaction. The workflow's three retries reuse the same URL, so they fail identically. Overnight this hit calendar, mail and clips in separate publishes, each failing Run the beta release migration against production.

Fix: strip the Neon -pooler host segment from whatever URL is selected, using the same rule and rationale as getMigrationDatabaseUrl() in packages/core/src/db/client.ts. Adds scripts/netlify-migration-url.test.ts (the file had no tests) and a test:netlify-migration-url script.

Red/Green (git apply -R of only the netlify-migration-url.ts hunk):

  • Red: 2 failures, including ✖ returns the direct endpoint when Netlify only exposes a pooled URL.
  • Green: 4/4 pass.

Verification

  • node --test scripts/ci-change-scope.test.ts: 15/15.
  • node --test scripts/netlify-migration-url.test.ts: 4/4.
  • oxfmt --check clean on all changed files.
  • oxlint does not cover these paths: .oxlintrc.json lists scripts/** under ignorePatterns.

🤖 Generated with Claude Code

builder-io-integration[bot]

This comment was marked as outdated.

@github-actions

github-actions Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Here's a visual recap of what changed:

Visual recap

Open the full interactive recap

@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 — looks good ✅

Review Details

Incremental Code Review Summary

The latest commit addresses both findings from the previous review: the non-full lint workflow now includes .md/.mdx files in the formatter input, and scripts/netlify-migration-url.test.ts is included in the Netlify workflow test command. The existing docs-only scope behavior, Neon pooler normalization, connection-string precedence, and requested-context handling remain coherent. Two independent review agents found no new confirmed issues, and the focused tests and formatting checks were reported passing.

Risk remains standard because this changes CI gating and deployment migration endpoint selection, but the updated implementation is appropriately scoped and has regression coverage. The two prior review threads were resolved after verifying their fixes.

🧪 Browser testing: Skipped — PR only modifies backend/config/docs/tests, no UI impact

@steve8708
steve8708 merged commit 0d9cbcc into main Sep 16, 2026
57 of 70 checks passed
@steve8708
steve8708 deleted the steve8708/beta-migration-and-lint-gate branch September 16, 2026 13:20
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.

1 participant