Skip to content

fix(db): disable prepared statements by default for pooler compat#286

Closed
garrytan wants to merge 1 commit into
masterfrom
fix/prepare-false-pooler-compat
Closed

fix(db): disable prepared statements by default for pooler compat#286
garrytan wants to merge 1 commit into
masterfrom
fix/prepare-false-pooler-compat

Conversation

@garrytan

Copy link
Copy Markdown
Owner

What

Adds prepare: false to the postgres.js connection options in db.ts.

Why

Supabase Supavisor and PgBouncer in transaction mode don't support prepared statements — session state isn't preserved between queries, so prepared statement references become dangling.

While ?prepare=false in the connection URL already handles this (postgres.js parses it correctly), setting it explicitly in the options object provides a safety net for:

  • Users who configure their DB URL without the query param
  • Any future refactoring that might construct the URL differently

Testing

  • All 1743 bun tests pass, 0 failures
  • Verified gbrain search and gbrain query work correctly with the change
  • Single line change, low risk

Impact

postgres.js defaults prepare: true. This makes gbrain work correctly with transaction-mode poolers out of the box.

Ref: https://supabase.com/docs/guides/database/connecting-to-postgres#connection-pooler

… compatibility

Supabase Supavisor and PgBouncer in transaction mode don't support
prepared statements because session state isn't preserved between queries.
While the URL param ?prepare=false handles this, setting it explicitly in
the connection options provides a safety net for any pooler configuration.

postgres.js defaults to prepare: true. This change makes gbrain work
correctly with transaction-mode poolers out of the box, matching the
recommendation in Supabase docs.

Ref: https://supabase.com/docs/guides/database/connecting-to-postgres#connection-pooler
garrytan added a commit that referenced this pull request Apr 22, 2026
closes #284, #286, #270) (#301)

* fix(migrate): v0_13_0 shells out to `gbrain` shim, not `process.execPath`

On bun-installed trees, process.execPath is the bun runtime itself.
`bun extract links ...` got reinterpreted as `bun run extract` and
crashed the upgrade mid-Phase B. The canonical shim on PATH already
wraps the right runtime+entrypoint; trust it.

Regression-guarded by test/migrations-v0_13_0.test.ts which greps
the source for `process.execPath` and `bun` invocations. This was
Bug 1 of tonight's v0.13 → v0.14 upgrade-night postmortem.

* fix(autopilot): resolveGbrainCliPath prefers shim, never returns .ts

argv[1] check used to short-circuit on /cli.ts, so bun-source installs
got a .ts path back. spawn() then failed EACCES because TypeScript
source isn't executable, and autopilot silently lost its worker.

Reordered probes: which gbrain (shim) first, then compiled execPath,
then argv[1] only if it ends in /gbrain. Deleted the .ts branch
entirely — no valid case exists.

Rewrote the existing test that enshrined the buggy .ts return.
Critical regression guard: resolver MUST NEVER return a .ts path
across any combination of argv[1] + execPath + shim availability.
This was Bug 4 of tonight's v0.13 → v0.14 upgrade-night postmortem.

* chore: bump version and changelog (v0.15.3)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(db): resolvePrepare() helper for PgBouncer transaction-mode pools

Adds port-6543 auto-detect with a 4-level precedence chain:
GBRAIN_PREPARE env var → ?prepare= URL param → port auto-detect → default.
Wires into the module-singleton connect() so the main CLI path no longer
hits "prepared statement does not exist" against Supabase transaction
pooler. Returns boolean | undefined; undefined means omit the option and
let postgres.js default (true) stand.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(postgres-engine): honor resolvePrepare in worker-instance pool

Without this, \`gbrain jobs work\` against a Supabase pooler URL hits
"prepared statement does not exist" under load even after the module
singleton was fixed in db.ts. Community PR #270 (@notjbg) caught this
second path that #284 had missed. Reuses the shared helper, no regex
duplication.

Co-Authored-By: Jonah Berg <jonah.berg.g@gmail.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(doctor): pgbouncer_prepare check

URL-only check (no DB roundtrip) that reads the configured URL via
loadConfig() and flags the footgun: port 6543 with prepared statements
still enabled. Warns with the exact env override (GBRAIN_PREPARE=false)
and URL-query alternative (?prepare=false). Works for both the module
singleton and worker-instance engines.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test: resolvePrepare precedence matrix + postgres-engine wiring guard

- test/resolve-prepare.test.ts: 11 cases covering env override, URL
  query param, port auto-detect, malformed URLs, postgres:// scheme,
  URL-encoded credentials. Uses bun:test — #284's original vitest file
  would never have run in this project.
- test/postgres-engine.test.ts: new source-level grep case asserting
  the worker-pool connect() branch calls db.resolvePrepare(url) and
  includes a typeof prepare === 'boolean' check. Mirrors the existing
  SET LOCAL regression guard. If anyone rips out the wiring, the build
  fails before shipping starts dropping rows.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore: bump version and changelog (v0.15.4)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Jonah Berg <jonah.berg.g@gmail.com>
@garrytan

Copy link
Copy Markdown
Owner Author

Closing — prepared-statements default landed in master.

Thanks for the report. If anything still reproduces on the latest release, please reopen with the version + repro.

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