Give control drivers a per-family transport, off the generic interface (TML-2840)#738
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (39)
💤 Files with no reviewable changes (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (30)
📝 WalkthroughWalkthroughBase control driver removes the SQL ChangesDriver Contract and Family-Specific Refactoring
🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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/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/emitter
@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
🧹 Nitpick comments (1)
packages/3-mongo-target/2-mongo-adapter/test/runner-deps.test.ts (1)
8-16: ⚡ Quick winAdd a regression case for mongo-typed drivers missing
db.This hunk improves the happy-path mock shape, but the failing-path coverage should also include
{ familyId: 'mongo', targetId: 'mongo' }withoutdbso the runtime guard behavior is locked in.Suggested test addition
it("throws when the mongo control driver doesn't expose a db property", () => { const driver = {} as unknown as ControlDriverInstance<'mongo', 'mongo'>; expect(() => extractDb(driver)).toThrowError( /Mongo control driver does not expose a db property/, ); }); + + it('throws when ids match mongo but db is missing', () => { + const driver = { + familyId: 'mongo', + targetId: 'mongo', + close: async () => {}, + } as unknown as ControlDriverInstance<'mongo', 'mongo'>; + expect(() => extractDb(driver)).toThrowError( + /Mongo control driver does not expose a db property/, + ); + });🤖 Prompt for 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. In `@packages/3-mongo-target/2-mongo-adapter/test/runner-deps.test.ts` around lines 8 - 16, Add a regression test in runner-deps.test.ts that asserts the runtime guard rejects a mongo-typed driver object that lacks the db property: create a mock driver object with { familyId: 'mongo', targetId: 'mongo' } (no db), cast to ControlDriverInstance<'mongo','mongo'> if needed, pass it into the same validation/registration path used by the existing happy-path test (the same variable names like driver or the function that consumes it), and assert the expected failure/throw or error result to lock in the guard behavior for mongo drivers missing db.
🤖 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 `@packages/3-mongo-target/2-mongo-adapter/src/core/mongo-control-driver.ts`:
- Around line 41-45: isMongoControlDriver currently only checks
familyId/targetId; strengthen it to also verify the Mongo-specific shape by
confirming the presence and types of required members (e.g., that the candidate
has a non-null "db" property and an "execute" method or other
MongoControlDriverInstance-specific functions). Update the predicate in
isMongoControlDriver to return true only when driver.familyId === 'mongo' &&
driver.targetId === 'mongo' && typeof (driver as any).execute === 'function' &&
(driver as any).db != null (or other required properties/methods from
MongoControlDriverInstance), so misclassified objects without DB or execute are
rejected.
---
Nitpick comments:
In `@packages/3-mongo-target/2-mongo-adapter/test/runner-deps.test.ts`:
- Around line 8-16: Add a regression test in runner-deps.test.ts that asserts
the runtime guard rejects a mongo-typed driver object that lacks the db
property: create a mock driver object with { familyId: 'mongo', targetId:
'mongo' } (no db), cast to ControlDriverInstance<'mongo','mongo'> if needed,
pass it into the same validation/registration path used by the existing
happy-path test (the same variable names like driver or the function that
consumes it), and assert the expected failure/throw or error result to lock in
the guard behavior for mongo drivers missing db.
🪄 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: 6300638f-d5e8-4b89-82c8-e8b5a34d3294
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (28)
packages/1-framework/1-core/framework-components/src/control/control-instances.tspackages/2-mongo-family/9-family/src/core/control-instance.tspackages/2-sql/1-core/contract/src/exports/types.tspackages/2-sql/1-core/contract/src/types.tspackages/2-sql/9-family/src/core/control-adapter.tspackages/2-sql/9-family/src/core/control-instance.tspackages/2-sql/9-family/src/core/migrations/types.tspackages/3-mongo-target/1-mongo-target/test/mongo-runner-integration.test.tspackages/3-mongo-target/2-mongo-adapter/package.jsonpackages/3-mongo-target/2-mongo-adapter/src/core/mongo-control-driver.tspackages/3-mongo-target/2-mongo-adapter/src/core/runner-deps.tspackages/3-mongo-target/2-mongo-adapter/src/exports/control.tspackages/3-mongo-target/2-mongo-adapter/test/runner-deps.test.tspackages/3-mongo-target/3-mongo-driver/src/exports/control.tspackages/3-mongo-target/3-mongo-driver/test/control-driver.test.tspackages/3-targets/3-targets/postgres/src/core/migrations/runner.tspackages/3-targets/3-targets/sqlite/src/core/migrations/runner.tspackages/3-targets/6-adapters/postgres/src/core/control-adapter.tspackages/3-targets/6-adapters/postgres/src/core/enum-control-hooks.tspackages/3-targets/6-adapters/postgres/src/core/marker-ledger.tspackages/3-targets/6-adapters/postgres/test/control-adapter.defaults.test.tspackages/3-targets/6-adapters/postgres/test/control-adapter.test.tspackages/3-targets/6-adapters/postgres/test/enum-control-hooks.basic.test.tspackages/3-targets/6-adapters/sqlite/src/core/control-adapter.tspackages/3-targets/6-adapters/sqlite/src/core/marker-ledger.tspackages/3-targets/7-drivers/postgres/src/exports/control.tspackages/3-targets/7-drivers/sqlite/src/core/control-driver.tstest/integration/test/family.introspect.test.ts
💤 Files with no reviewable changes (4)
- packages/3-mongo-target/3-mongo-driver/src/exports/control.ts
- packages/1-framework/1-core/framework-components/src/control/control-instances.ts
- packages/3-mongo-target/1-mongo-target/test/mongo-runner-integration.test.ts
- packages/3-mongo-target/3-mongo-driver/test/control-driver.test.ts
| constructor(db: Db, client: MongoClient) { | ||
| this.db = db; | ||
| this.#client = client; | ||
| this.#driver = MongoDriverImpl.fromDb(db); |
There was a problem hiding this comment.
This is stupid. Why does the control driver instantiate an execution-plane driver and keep it hidden? This is shitty code sharing
wmadden
left a comment
There was a problem hiding this comment.
Pre-emptive approval. One comment worth addressing before merge
| if (!isMongoControlDriver(driver)) { | ||
| throw new Error( | ||
| 'Mongo control driver does not expose a db property. ' + | ||
| 'Use mongoControlDriver.create() from `@prisma-next/driver-mongo/control`.', | ||
| ); | ||
| } |
There was a problem hiding this comment.
This error message doesn't match the condition
Add SqlControlDriverInstance<T extends string> to @prisma-next/sql-contract/types. It extends the generic ControlDriverInstance<'sql', T> and re-declares the query() signature verbatim. PostgresControlDriver and SqliteControlDriver now declare implements SqlControlDriverInstance<'postgres'/'sqlite'> respectively. The generic ControlDriverInstance still carries query() at this point so all existing SQL call sites continue to compile unchanged; the subsequent commit retypes them to SqlControlDriverInstance. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Propagate SqlControlDriverInstance<T> to every location in the SQL family and targets that declares driver parameters or local variables typed as ControlDriverInstance<'sql', ...>: - SqlControlFamilyInstance interface and implementation (control-instance.ts) - SqlControlAdapter SPI (control-adapter.ts) - CodecControlHooks.introspectTypes and SqlMigrationRunnerExecuteOptions (migrations/types.ts) - PostgresControlAdapter / SqliteControlAdapter (both control-adapter.ts) - PostgresMarkerWriteDriver / SqliteMarkerWriteDriver aliases (marker-ledger.ts) - introspectPostgresEnumTypes (enum-control-hooks.ts) - PostgresMigrationRunner.execute / SqliteMigrationRunner.execute (runner.ts) - Mock drivers in postgres control-adapter tests and enum-control-hooks test No SQL call site now depends on the generic ControlDriverInstance carrying query(). Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…re casts with predicates Changes: - MongoControlDriverInstance now extends both ControlDriverInstance<'mongo','mongo'> and MongoDriver, gaining execute() alongside the existing .db property. - MongoControlDriverImpl delegates execute() to a held MongoDriverImpl.fromDb(db), so existing callers of createMongoControlDriver(db, client) are unchanged. - Add isMongoControlDriver type predicate (exported from adapter-mongo/control); runner-deps.extractDb uses it in place of the previous bare cast. - asMongoDriver in the Mongo family now uses a type predicate (isMongoTargetDriver) instead of a bare cast; the family layer cannot import MongoControlDriverInstance (targets domain), so the predicate narrowed to the framework interface only. - Add @prisma-next/driver-mongo as a production dependency of adapter-mongo (the targets/adapters -> targets/drivers import is allowed; depcruiser only restricts the SQL driver direction). - Update runner-deps.test.ts to supply the fields the isMongoControlDriver predicate checks (familyId, targetId) in the mock driver. - query() stub is kept for now since ControlDriverInstance still declares it; it will be removed in the framework-cut commit. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…hrowing stub Framework cut: - ControlDriverInstance now carries only close() + inherited tags (no query). - MongoControlDriverImpl.query() stub removed (no longer required by the interface). - MongoControlDriver in @prisma-next/driver-mongo likewise drops its query() stub. - controlDriverFromDb helper in adapter-mongo/control drops the query property. Test cleanup: - Remove the query()-throws tests from mongo-runner-integration.test.ts and control-driver.test.ts (the behaviour they validated no longer exists). - Update family.introspect.test.ts in integration tests to type the mock driver as SqlControlDriverInstance<'postgres'> (ControlDriverInstance no longer carries query, so the mock needs the SQL-specific interface to satisfy familyInstance.introspect()). Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…trolDriver extends MongoDriverImpl - Add MongoControlDriverInstance interface to @prisma-next/mongo-lowering (with mongodb peer dep). It extends ControlDriverInstance<'mongo','mongo'>, MongoDriver, and adds `readonly db: Db`. - Make MongoDriverImpl subclassable: protected constructor, protected readonly db/client fields, protected execute*Command helpers (renamed from hard-private #db/#client/#execute* methods). - MongoControlDriver in driver-mongo now extends MongoDriverImpl and implements MongoControlDriverInstance, inheriting execute() and close() from the base class. It overrides db as public readonly to satisfy the interface's public carve-out. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…er-mongo dep - Remove MongoControlDriverImpl and createMongoControlDriver from adapter-mongo. The adapter no longer constructs the control driver; it only uses the interface. - MongoControlDriverInstance is now re-exported from @prisma-next/mongo-lowering in adapter-mongo's control exports (as a re-export from an exports/ file). - isMongoControlDriver predicate stays in the adapter, typed against the interface. - Remove @prisma-next/driver-mongo from adapter-mongo's dependencies; the adapter now depends only on the interface (via mongo-lowering) and never on the concretion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…Driver constructor All tests that previously called createMongoControlDriver(db, client) from @prisma-next/adapter-mongo/control now construct new MongoControlDriver(db, client) imported from @prisma-next/driver-mongo/control. Affected files span the target-mongo integration tests and the shared integration test suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
The guard is `!isMongoControlDriver(driver)` — a tag check — but the old message said "does not expose a db property", describing the property-presence check it replaced. Updated the message and the matching test assertion to reflect what the predicate actually checks. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
5b6b080 to
0b38cff
Compare
Now that TML-2840 (#738) gives MongoControlDriver a real execute(wireCommand), MongoControlAdapterImpl narrows the driver via the isMongoControlDriver predicate and dispatches through driver.execute directly. Deletes the controlDriverDb helper, the throwaway MongoDriverImpl.fromDb reach-around, and the source-level dependency on @prisma-next/driver-mongo (forbidden migration->runtime plane edge). The public @prisma-next/adapter-mongo/control free functions (readMarker, readAllMarkers, initMarker, updateMarker, readLedger, writeLedgerEntry) now take a MongoControlDriverInstance instead of a raw Db: the fabricated db-only driver cannot carry the wire transport, and the plane rule forbids adapter-mongo from constructing a real one. Tests build new MongoControlDriver(db, client) and pass it; driver-mongo is added as a devDependency where tests need it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
Now that TML-2840 (#738) gives MongoControlDriver a real execute(wireCommand), MongoControlAdapterImpl narrows the driver via the isMongoControlDriver predicate and dispatches through driver.execute directly. Deletes the controlDriverDb helper, the throwaway MongoDriverImpl.fromDb reach-around, and the source-level dependency on @prisma-next/driver-mongo (a forbidden migration->runtime plane edge). Removes the redundant readMarker / readAllMarkers / initMarker / updateMarker / readLedger / writeLedgerEntry free functions from @prisma-next/adapter-mongo/control: they were thin wrappers over a private MongoControlAdapterImpl singleton and added nothing. Callers (tests) now construct a MongoControlAdapterImpl plus a real MongoControlDriver(db, client) and invoke the adapter methods directly. driver-mongo is added as a devDependency where tests need it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
Now that TML-2840 (#738) gives MongoControlDriver a real execute(wireCommand), MongoControlAdapterImpl narrows the driver via the isMongoControlDriver predicate and dispatches through driver.execute directly. Deletes the controlDriverDb helper, the throwaway MongoDriverImpl.fromDb reach-around, and the source-level dependency on @prisma-next/driver-mongo (a forbidden migration->runtime plane edge). Removes the redundant readMarker / readAllMarkers / initMarker / updateMarker / readLedger / writeLedgerEntry free functions from @prisma-next/adapter-mongo/control: they were thin wrappers over a private MongoControlAdapterImpl singleton and added nothing. Callers (tests) now construct a MongoControlAdapterImpl plus a real MongoControlDriver(db, client) and invoke the adapter methods directly. driver-mongo is added as a devDependency where tests need it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
At a glance
The framework's target-agnostic control-driver interface used to hard-code SQL's transport. This PR takes it off:
The decision
A control driver is an execution-plane driver plus a couple of control-plane tags, and its transport is family-specific. So the generic interface every family implements should carry no transport at all — just the tags and
close(). Each family adds its own transport on its own interface.Why the generic interface was wrong
query(sql, params)is SQL's control transport, and that part is fine. The problem is that it sat on the interface every family must implement. SQL was happy; Mongo was not. Mongo has no SQL to run, so its control driver implementedquery()with a stub that just threw, and exposed a public.dbhandle as an escape hatch — so callers could reach Mongo's real transport,MongoDriver.execute(wireCommand), which lived on a separate interface the control driver didn't extend. The generic interface was forcing every non-SQL family to carry a method that makes no sense for it, and then leaking a side-channel to work around it.What changes, family by family
Generic —
ControlDriverInstancedropsquery; it's now tags +close(). The framework only threads the driver through (verify/sign/readMarker/readAllMarkers/readLedger/introspect) and never calls the transport, so nothing in the framework breaks.SQL —
querymoves, verbatim, onto a newSqlControlDriverInstance<T extends string> extends ControlDriverInstance<'sql', T>, in@prisma-next/sql-contract/typesso the SQL drivers, adapters, and family can all import it.PostgresControlDriverandSqliteControlDriverimplement it; every.querycall site narrows to it. No behaviour change.Mongo — each plane gets one interface and one concretion, and the concretions live together in
driver-mongowith the inheritance between them:mongo-lowering)MongoDriverMongoControlDriverInstance extends ControlDriverInstance<'mongo','mongo'>, MongoDriverdriver-mongo)MongoDriverImplMongoControlDriver extends MongoDriverImplThe production control driver —
MongoControlDriver, the one the descriptor'screate(url)builds — now subclassesMongoDriverImpl, so it inherits the realexecute(wireCommand)andclose()and just adds the control-plane tags + thedbcarve-out. No held driver instance, no constructing an execution driver from a rawDb. The throwingquery()stub is gone.adapter-mongodefines no driver: its old in-package control-driver class is deleted, it depends only on theMongoControlDriverInstanceinterface, and its dependency on the concretedriver-mongopackage is dropped entirely.Casts → predicates — the two bare
ascasts that used to narrow the driver (extractDb,asMongoDriver) become type predicates (isMongoControlDriverin the adapter,isMongoTargetDriverin the family). Net production bare-cast count drops.Scope: deliberately narrow
This PR only makes
execute()available on the Mongo control driver. It does not rewrite the Mongo marker/ledger path to use it — those methods still go throughextractDb(driver)+ the existingDb-based helpers, and.dbstays exposed (for marker/ledger + introspection). Routing marker/ledger throughexecuteand removing the.dbreach-around is TML-2825 (PR #719), which this PR unblocks.One subtlety: why two predicates, not one
2-mongo-family(the mongo domain) may only import from the framework domain, so it cannot seeMongoControlDriverInstance, which lives in the targets-domain adapter. The family seam therefore narrows only toControlDriverInstance<'mongo','mongo'>; the transport-bearing predicate lives in the adapter, whereexecute()is actually called.pnpm lint:depsenforces this boundary — the split is the boundary, not an oversight.Verification
pnpm build✅ ·pnpm typecheck✅ (135/135) ·pnpm lint:deps✅ —framework-componentsgains no dependency on any lowering/exec-driver package (control-plane types don't leak into runtime bundles), andadapter-mongono longer depends on the concretedriver-mongopackage at all (it consumes only interfaces). TheMongoControlDriverInstanceinterface lives inmongo-loweringwith atype-onlyDbreference —mongodbis an optional peer there, not a runtime dependency.family.introspectintegration test pass.@prisma/devserver was unavailable — uniform connection errors across all Postgres tests, including ones this PR doesn't touch). This is a pure type-level transport re-home for SQL, so those failures are environmental, not behavioural — CI is the authoritative Postgres signal.Alternatives considered
queryon the generic and just have Mongo additionally extendMongoDriver. Rejected — Mongo would still have to implement the throwingquery()stub, so the wart survives.SqlQueryable(the rich runtime interface:execute/executePrepared/explain). Rejected — the control plane never needs prepared/explain/codec-lowered execute, and pullingSqlQueryableinto control would drag runtime surface into control bundles and break tree-shaking. SQL's control transport stays a deliberately minimalquery.Refs: TML-2840. Related: TML-2825 (PR #719).
Summary by CodeRabbit
Refactor
Style / Types
Tests