From 1c24a6e684a634a2ad3a59db85593c9111d085bb Mon Sep 17 00:00:00 2001 From: Wanjohi Date: Wed, 2 Sep 2026 12:31:41 +0300 Subject: [PATCH] fix(ci): the web job has been failing on two separate bugs (#311) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `bun run db:push` has been failing on **every pull request** with `error: Script not found "db:push"`. A red check has meant nothing for as long as that's been true. There are **two independent causes**, and fixing only the reported one leaves the job red. ## 1. The script isn't at the root `db:push` lives in `packages/core/package.json`; CI runs from the root. Added root passthroughs for `db:migrate` and `db:push`, so the command CI runs is also the one a human can run. ## 2. `drizzle.config.ts` enabled TLS for any `DATABASE_URL` ```ts ssl: !!process.env.DATABASE_URL ? { rejectUnauthorized: false } : false ``` That's true for *any* URL — so it failed against every plain Postgres, **including CI's own `postgres:18-alpine` service container**. And `drizzle-kit` reports that failure as a spinner and a non-zero exit with no message attached, which is why it would have been maddening to find from a log. Measured against a local container: | | result | |---|---| | `DATABASE_URL` set (TLS on) | migrations fail, no error text | | `DATABASE_URL` unset, same database | all seven apply | TLS is now decided by the connection string: an explicit `sslmode` wins, otherwise a local host gets none (it doesn't speak TLS at all) and any other host gets TLS without chain verification, which is what a hosted Postgres usually needs. The URL is parsed once rather than eight times. ## CI now applies migrations instead of `push` `drizzle-kit push` diffs the schema against whatever is already in the database and is a development tool — CI wants exactly what's committed in `packages/core/migrations`. And `push` under `strict: true` asks for confirmation, which on a runner is a **hang**, not a failure. ## Verified Locally against `postgres:18-alpine` from an empty database, running exactly what the workflow runs: ``` bun install --frozen-lockfile ✓ bun run db:migrate ✓ 7 migrations applied bun test ✓ 113 pass, 0 fail, 297 expect() calls ``` Worth landing ahead of #310 so that a red check starts meaning something again.

Greptile Summary

This PR repairs the database-backed web CI job by exposing core database commands at the workspace root, applying committed migrations instead of schema push, and selecting PostgreSQL TLS behavior from the connection URL. - Adds root passthrough scripts for database migration and schema push commands. - Adds the core `drizzle-kit migrate` command and runs it in CI. - Disables TLS for local PostgreSQL while honoring explicit `sslmode` settings. - Keeps migration and test steps pointed at the same temporary CI database.

Confidence Score: 5/5

The PR appears safe to merge, with the migration command, working directory, connection settings, and test database remaining aligned. The changed CI path reaches the committed migration history through the intended core package configuration, uses plaintext for the local PostgreSQL service, and then tests against the same migrated database; no changed-code defect remains.

Important Files Changed

| Filename | Overview | |----------|----------| | .github/workflows/ci.yml | Replaces schema push with committed migration execution while preserving the shared CI database URL. | | package.json | Adds root-level passthroughs to the core package's database commands. | | packages/core/drizzle.config.ts | Parses the database URL once and selects TLS based on explicit mode or local-versus-remote host inference. | | packages/core/package.json | Adds the `drizzle-kit migrate` script consumed by the root command and CI workflow. |

Flowchart

```mermaid %%{init: {'theme': 'neutral'}}%% flowchart LR PR[Pull request or main push] --> CI[Web CI job] CI --> PG[PostgreSQL 18 service] CI --> Install[Bun frozen install] Install --> Root[Root db:migrate script] Root --> Core[packages/core db:migrate] Core --> Config[drizzle.config.ts] Config --> Migrations[Committed migrations] Migrations --> PG PG --> Tests[Bun tests] ``` Reviews (1): Last reviewed commit: ["fix(ci): the web job has been failing on..."](https://github.com/nestrilabs/nestri/commit/203e882fbd7ab1578922f63e2c8986dce0b78c6f) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=59430932) --- .github/workflows/ci.yml | 9 +++++-- package.json | 2 ++ packages/core/drizzle.config.ts | 44 +++++++++++++++++++++++++++------ packages/core/package.json | 3 ++- 4 files changed, 47 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6379d579..034b1c56 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,8 +29,13 @@ jobs: bun-version: 1.3.11 - name: Install run: bun install --frozen-lockfile - - name: Push migrations - run: bun run db:push + # Apply the committed migrations rather than `drizzle-kit push`. Two + # reasons: `push` diffs the schema against whatever is in the database and + # is a development tool, whereas CI wants exactly what is in + # `packages/core/migrations`; and `push` under `strict: true` asks for + # confirmation, which on a runner is a hang rather than a failure. + - name: Apply migrations + run: bun run db:migrate env: DATABASE_URL: postgres://postgres:postgres@localhost:5432/nestri - name: Test diff --git a/package.json b/package.json index a57e4ad7..3b3bb5ea 100644 --- a/package.json +++ b/package.json @@ -20,6 +20,8 @@ "type": "module", "scripts": { "dev": "alchemy dev", + "db:migrate": "bun run --cwd packages/core db:migrate", + "db:push": "bun run --cwd packages/core db:push", "test": "test", "deploy:sandbox": "alchemy deploy --stage sandbox --yes", "deploy:production": "alchemy deploy --stage production --yes" diff --git a/packages/core/drizzle.config.ts b/packages/core/drizzle.config.ts index fda9b062..233a7f0a 100644 --- a/packages/core/drizzle.config.ts +++ b/packages/core/drizzle.config.ts @@ -1,5 +1,35 @@ import { defineConfig } from 'drizzle-kit'; +const url = process.env.DATABASE_URL ? new URL(process.env.DATABASE_URL) : null; + +/** + * Whether to speak TLS, decided by the connection string rather than by + * whether one exists. + * + * The previous form was `ssl: !!process.env.DATABASE_URL ? { rejectUnauthorized: + * false } : false`, which turned TLS on for *any* `DATABASE_URL` — so it failed + * against every plain Postgres, including CI's own `postgres:18-alpine` service + * container, and `drizzle-kit` reports that failure as a spinner and a non-zero + * exit with no message attached. Measured 2026-09-02: with the URL set, + * migrations fail silently; with it unset against the same database, all seven + * apply. + */ +function sslFor(u: URL | null): false | { rejectUnauthorized: boolean } { + if (!u) return false; + + // An explicit sslmode in the URL wins, always. + const mode = u.searchParams.get('sslmode'); + if (mode) { + if (mode === 'disable') return false; + return { rejectUnauthorized: mode === 'verify-full' }; + } + + // Otherwise infer: a local Postgres does not speak TLS at all, and a hosted + // one nearly always does — usually behind a chain we have no root for. + const local = ['localhost', '127.0.0.1', '::1', ''].includes(u.hostname); + return local ? false : { rejectUnauthorized: false }; +} + export default defineConfig({ verbose: true, strict: true, @@ -7,13 +37,11 @@ export default defineConfig({ dialect: 'postgresql', schema: './src/**/*.sql.ts', dbCredentials: { - host: process.env.DATABASE_URL ? new URL(process.env.DATABASE_URL).hostname : 'localhost', - port: process.env.DATABASE_URL ? Number(new URL(process.env.DATABASE_URL).port || 5432) : 5432, - user: process.env.DATABASE_URL ? new URL(process.env.DATABASE_URL).username : 'postgres', - password: process.env.DATABASE_URL ? new URL(process.env.DATABASE_URL).password : 'postgres', - database: process.env.DATABASE_URL - ? new URL(process.env.DATABASE_URL).pathname.slice(1) - : 'nestri', - ssl: !!process.env.DATABASE_URL ? { rejectUnauthorized: false } : false + host: url?.hostname || 'localhost', + port: Number(url?.port || 5432), + user: url?.username || 'postgres', + password: url?.password || 'postgres', + database: url?.pathname.slice(1) || 'nestri', + ssl: sslFor(url) } }); diff --git a/packages/core/package.json b/packages/core/package.json index f7ce8f9b..14c153f5 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -13,7 +13,8 @@ }, "scripts": { "db": "drizzle-kit", - "db:push": "drizzle-kit push" + "db:push": "drizzle-kit push", + "db:migrate": "drizzle-kit migrate" }, "dependencies": { "@nestri/auth": "workspace:",