Resolve pg bindings by structure, not instanceof, to survive bundle dupes - #969
Conversation
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughPostgreSQL runtime exports structural pool/client guards and uses them for binding resolution. Supabase replaces constructor-based pool detection with the shared guard, while PostgreSQL mocks, tests, and upgrade instructions cover the updated behavior. ChangesPostgreSQL binding detection
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@prisma-next/extension-author-tools
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@prisma-next/middleware-cache
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/extension-postgis
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/extension-supabase
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/ts-render
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/cli-telemetry
@prisma-next/config-loader
@prisma-next/emitter
@prisma-next/language-server
@prisma-next/migration-tools
prisma-next
@prisma-next/vite-plugin-contract-emit
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-query-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
size-limit report 📦
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@skills/extension-author/prisma-next-extension-upgrade/upgrades/0.14-to-0.15/instructions.md`:
- Around line 590-596: Revise the paragraph describing Pool/Client resolution to
state that shape-based guards preserve existing same-copy behavior while
additionally accepting structurally compatible cross-copy or duck-typed handles.
Replace the universal “at boot” failure wording with the specific
binding-resolution paths, including connect() and toPool(), and retain the
statement that no extension-author action is required.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 37843398-d6c2-4bae-9c35-b6d3be99b5a3
📒 Files selected for processing (1)
skills/extension-author/prisma-next-extension-upgrade/upgrades/0.14-to-0.15/instructions.md
…upes resolvePostgresBinding and the supabase toPool helper identified pg Pool/Client handles with instanceof. When an app bundler produces two copies of pg (a phantom-dependency duplication), the caller`s Pool is an instance of a different Pool class than the one imported here, so the instanceof checks fall through and resolvePostgresBinding throws "Unable to determine pg binding type from pg input" — crashing any bundled pnPostgres() service at boot before any connection is attempted. Replace the three instanceof checks with cast-free structural predicates isPgPool/isPgClient (in-operator narrowing on disjoint members: Pool has totalCount/idleCount/waitingCount, Client has escapeIdentifier/ escapeLiteral). These are structural, so they hold across duplicate pg module copies. The predicates are exported from @prisma-next/postgres/runtime and reused by the supabase extension. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
The pg binding fix touches packages/3-extensions/ (postgres + supabase), so the per-pr-declaration upgrade-coverage gate requires a declaration in the in-flight 0.14-to-0.15 instructions. The change is additive plus a bug fix with no extension-author action, so record it as an incidental substrate diff. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
c4fb0fc to
8e28ceb
Compare
Linked issue
n/a — surfaced from a reported catalog-service boot-crash-loop; no tracking issue exists yet.
At a glance
@prisma-next/postgresnow identifies a caller-suppliedpghandle by shape, not class identity:Before,
resolvePostgresBindingdidpgBinding instanceof PgPool/instanceof PgClient. When an app bundler ships two copies ofpg, the caller'sPoolis an instance of a differentPoolclass than the one imported here, both checks fall through, and it throwsUnable to determine pg binding type from pg input— crashing any bundledpnPostgres()service at boot, before any connection is attempted.Summary
A bundle that duplicates
pg(the classic phantom-dependency situation an app bundler creates) breaks the runtime'sinstanceof-based binding resolution, becauseinstanceofcompares against one specific module'sPool/Clientclass. This replaces the class-identity checks with structural predicates that hold across duplicate module copies, killing this class of boot crash regardless of how a downstream bundler resolvespg.Decision
Two things ship:
resolvePostgresBinding(and the supabasetoPoolhelper) identifyPoolvsClientby disjoint structural members instead ofinstanceof.PoolhastotalCount/idleCount/waitingCount;ClienthasescapeIdentifier/escapeLiteral.in-operator narrowing means zero new casts.isPgPool/isPgClientare exported from@prisma-next/postgres/runtimeand reused by the supabase extension, so all three formerinstanceofsites share one definition rather than each carrying its own class-identity check.The explicit escape hatch —
binding: { kind: 'pgPool', pool }— is unchanged and remains the unambiguous override.How it fits together
pgvalue import (the file keeps only the type-only import, so it no longer pulls pg's runtime into whatever bundles it).instanceofbranches inresolvePostgresBindingforisPgPool/isPgClient; the "neither Pool nor Client" throw is untouched.toPoolhelper at the sharedisPgPoolin supabase.ts, guarding the optionaloptions.pgforundefinedfirst.Behavior changes & evidence
Pool(orClient) that is not aninstanceofthis package'spgcopy now resolves correctly instead of throwing at boot — binding.ts. The regression test asserts the exact duplicate-copy condition: a duck-typed pool wherepool instanceof Pool === falseyetisPgPool(pool) === true, andresolvePostgresBindingreturns{ kind: 'pgPool', pool }— postgres/test/binding.test.ts.Reviewer notes
in-operator discriminators are disjoint on real pg (verified against pg 8.21.0):Poolhas noescapeIdentifier;Clienthas noidleCount. Order of the two checks is therefore not significant.vi.mock('pg', …)fixtures (postgres.test.ts, postgres-close.test.ts) had fakePool/Clientclasses that only implementedconnect/query/end. Once resolution became structural those fakes matched neither predicate, so I added the missing members. The edits are additive — no assertion changed — and the "throws for unrecognized pg input" test is untouched.prisma-nextpatch that is orthogonal to, and stronger than, fixing the phantompgdependency on the bundler side: it survives any bundler that duplicatespg, not just the one that motivated it. The version bump / publish is a separate release step, not included here.Testing performed
pnpm --filter @prisma-next/postgres test— 118 passedpnpm --filter @prisma-next/extension-supabase test— 53 passedpnpm --filter @prisma-next/postgres build(refreshes the/runtimeexport the supabase extension consumes)pnpm --filter @prisma-next/postgres typecheckandpnpm --filter @prisma-next/extension-supabase typecheck— cleanpnpm lint:deps— no violations (1187 modules cruised)Test-first: the duck-typed-pool assertions were written first and confirmed failing against the
instanceofcode before the fix.Skill update
n/a — internal only.
isPgPool/isPgClientare runtime type guards, not part of the CLI or authoring surface.Alternatives considered
binding: { kind: 'pgPool', pool }at the call site (compose-side fix). Correct and supported, but it only fixes the one app that adopts it; every otherpnPostgres()consumer stays exposed. The structural fix here removes the hazard for all of them.connect+query. Insufficient: bothPoolandClientexpose both as functions, so it cannot tell them apart and would misroute aClientinto the pool driver.instanceofwith a structural fallback. More code for no benefit — the structural check subsumes theinstanceofcase and also removes the need for thepgvalue import.Checklist
git commit -s) per the DCO.TML-NNNN:form — no Linear ticket exists for this reported boot-crash; a descriptive title is used instead.Summary by CodeRabbit