The row lock and the advisory lock composed through the raw lane (#1155) - #1335
Conversation
#1155) `.forUpdate()` on a transaction-bound builder, carried by `first()` and `all()`: the scoped read runs first — operation access, the Access Filter and Field Visibility exactly as any read — and the engine then composes `SELECT <pk> FROM <table> WHERE <pk> IN ($1…$n) ORDER BY <pk> LIMIT $n+1 FOR UPDATE` through the contract-bound raw tag and runs it on the transaction's own executor, inside the terminal's engine origin. Table and key column come off the contract's storage, identifiers are quoted by the target package's helpers, and each key binds with the identity column's own codec — so a `uuid` key renders `$1::uuid` and a text key `$1`. Only rows the lock came back with are returned. `forUpdate()` outside a transaction is a compile error, not a throw: the generated bundle now names two faces per list (`PostList`, `PostTxList`) and `TransactionContext` is the context over the locking one. `advisoryLock(key)` joins it on that context, running `pg_advisory_xact_lock(hashtext($1))`. No keys, no statement — an empty scoped read returns its empty value rather than reaching `IN ()`, which is a Postgres syntax error. A key set over `ROW_LOCK_MAX_KEYS` is refused with a stack-owned error before the lock statement, and a bound over it before any statement at all. `aggregate()` and `nearest()` refuse the modifier through the terminal's own `PlanDispositions` rather than dropping it. No `forShare`, no `NOWAIT`, no `SKIP LOCKED`. Restores the #614 capacity-gate guarantee #1154 had to delete, re-expressed with `.forUpdate()` against a real database with genuinely overlapping racers, and adds the `forUpdate()` example to `packages/core/CLAUDE.md`. Implements #1155. Part of #1124. See ADR-0047 and ADR-0062. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Deployment failed for project stack-docs with the following error: Learn More: https://vercel.com/open-saas?upgradeToPro=build-rate-limit |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
🦋 Changeset detectedLatest commit: fb7bbf5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Coverage Report for Core Package Coverage (./packages/core)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Coverage Report for UI Package Coverage (./packages/ui)
File CoverageNo changed files found. |
Coverage Report for CLI Package Coverage (./packages/cli)
File Coverage
|
||||||||||||||||||||||||||||||||||||||
Coverage Report for Auth Package Coverage (./packages/auth)
File CoverageNo changed files found. |
Coverage Report for Storage Package Coverage (./packages/storage)
File CoverageNo changed files found. |
Coverage Report for RAG Package Coverage (./packages/rag)
File CoverageNo changed files found. |
Coverage Report for Storage S3 Package Coverage (./packages/storage-s3)
File CoverageNo changed files found. |
Coverage Report for Storage Vercel Package Coverage (./packages/storage-vercel)
File CoverageNo changed files found. |
borisno2
left a comment
borisno2
left a comment
There was a problem hiding this comment.
Code review — PR #1335 (effort: high)
Verdict: REQUEST CHANGES. The lock mechanism itself is sound and I could not break it: the SQL composition is safe, the transaction binding is real, and the contention proof is genuine. The blocking item is #1 — the flagship example the docs teach reads a column off the locked row, and that value is the pre-lock snapshot. Everything else is minor.
Reviewed by checking out the branch, installing, pnpm build --force, running the suites on both harnesses, and mutating production code to confirm each claim is falsifiable. All of the PR description's verification numbers reproduced exactly (core 1640/5 skipped; cli 410; Postgres 1608/23 skipped; lint 0 errors + 2 pre-existing warnings; prettier clean).
The three headline risks, answered
1. Raw SQL composition — clean. Every attacker-influenceable value is a bound parameter; the only text in the statement is fixed clause text plus contract-derived identifiers, and there is exactly one composition site (lockStatement) with no branch that bypasses quoting.
- Keys are wrapped in
param(key, { codecId })→ParamRef. I traced Prisma's tag:resolveInterpolation(@prisma/orm-family-sql) turns a barestring/numberintoParamRef.of(...)too, soadvisoryLock(key)— the one genuinely caller-supplied string in the file — is bound, not interpolated. Recorded params confirm it. quoteIdentifier(@prisma/orm-target-postgres) doubles embedded"and rejects null bytes/empty. I drovelane.lock()over a hostile synthetic contract (tableSlot" FOR UPDATE; DROP TABLE users; --, columnid" , (SELECT 1) AS "x, namespacepub"lic). The result stays inside one valid quoted identifier — no breakout — and both keys came through asparam-ref.
3. Transaction binding — real, and the tests catch its loss. createRowLockLane(client.raw, client.contract, _unsafeTransaction) passes the transaction's scope on both construction paths. I mutated both to client.runtime() (the pooled executor): the capacity gate fails with timeout exceeded when trying to connect — it goes looking for a second connection against each racer's max: 1 pool — and 6 further tests in lock.test.ts fail on PGlite. That is a real experiment, not a shape argument.
The compile-error half also holds, and the type-level proof is exercised rather than decorative. I flipped RowLockKey<Tx> to Tx extends true ? 'forUpdate' : 'forUpdate'; core's own typed-read-surface.test.ts:148,150 fails to compile. That assertion — Exact<Exclude<PromisedLockingMember, PromisedQueryMember>, 'forUpdate'> — is stronger than needed for this PR and is exactly what will catch a future forShare/skipLocked being added to the surface.
4. The concurrency test genuinely proves contention — verified by mutation. I removed ' FOR UPDATE' from the composed statement and ran against real PostgreSQL 14: expected [ true, true, true, true, true ] to have a length of 2 but got 5. All five racers admitted means they really do overlap in time — a test whose racers had serialised would still have answered 2 and passed for the wrong reason. The barrier is inside the callback and client.transaction issues BEGIN before invoking it, so all five transactions are open before any locks. Each racer has its own client, pool and connection.
Also confirmed the harness is adequate: the suite is describe.skipIf(escape.kind !== 'postgres'), and .github/workflows/test.yml:28 sets DATABASE_URL to Postgres for any PR whose base is not main — so this does run in CI on this PR rather than silently skipping.
The remaining risk areas
- 2. Locked ⊆ readable — holds. The scoped read resolves access first (
resolvePlan→nullshort-circuits), keys come only from rows it returned, andlocked()filters to keys the lock statement came back with. A vanished row is absent, not an error (lock.test.ts:247). TheNotelist in the fixture proves the access filter scopes before the lock: an unreadable row yields[]and zero raw statements. - 5. Excluded capabilities — genuinely excluded. No
forShare,NOWAITorSKIP LOCKEDanywhere on the surface, not even as an internal option:lockStatementhardcodes' FOR UPDATE'with no mode parameter, andRowLockKey<Tx>admits the single member.NOWAITappears only inside the test's own second-connection probe, which is a measurement, not a capability. - 6. Refusals before side effects — verified. Both refusal tests assert emptiness, and the over-bound one asserts
recorder.statements(all lanes, not just raw) is[], so the scoped read never ran either. - 7. Type integrity — clean. No
any, no casts, no@ts-ignorein the diff (every "any" hit is the English word in a comment).unknownappears only as parameter types on the internalcreateRowLockLane; the public entry point exports just the three error classes and the constant. - 8. Base is
prisma-8; changeset isminorfor both packages. Correct.
Findings
1. Medium (blocking) — a locked row carries its pre-lock column values, and the documented example depends on them.
locked() (packages/core/src/secured/read.ts:914) returns the row objects from statement 1; statement 2 re-validates only the key set. Under Read Committed each statement takes its own snapshot, so a column mutated and committed between the two reaches the caller stale.
Proved on PostgreSQL 14. A blocker holds the row lock; our transaction's statement 1 reads capacity = 2; statement 2 blocks; the blocker sets capacity = 0 and commits; statement 2 acquires and returns the id:
CAPACITY SEEN BY LOCK HOLDER: 2
CAPACITY ACTUALLY COMMITTED: 0
For contrast, a single-statement SELECT * FROM … FOR UPDATE in the same race returns 0 — Postgres re-evaluates after acquiring (EvalPlanQual). So .forUpdate() diverges from what FOR UPDATE means in SQL, in the direction a reader will not expect.
ADR-0047:32 already says the lock is "a mutex token, not protection for the parent's own data" — but the flagship example in the PR body, packages/core/CLAUDE.md, the root CLAUDE.md, .changeset/mellow-gates-hold.md and capacity-gate.test.ts:160 all read parent.capacity off the locked row and use it as the gate threshold. That is precisely relying on the parent's own data. The gate's main invariant is still safe (the Booking count is a separate statement issued after the lock, so it is fresh); it is the threshold that can be stale, and any app with an admin UI that edits capacity inherits a latent race.
Either re-read the needed columns in statement 2 and return those, or state plainly in the TSDoc and both CLAUDE.mds that only the identity is post-lock and change the example so it does not read a mutable column off the locked row.
2. Medium — a hook inside context.transaction() cannot take a row lock, and is told it is not in a transaction.
buildDbDelegate (packages/core/src/context/index.ts:1166) calls populateDbDelegate with no lock argument, so the lane is dropped on the contexts it rebuilds — write-pipeline.ts:262 (bindContextToTransaction) and access/field-visibility.ts:127. forUpdate is still installed as a member, so the call composes and then throws. Confirmed with a beforeOperation hook running inside context.transaction():
RowLockUnavailableError: all().forUpdate() needs a transaction … Compose the read inside
`context.transaction(async (tx) => …)` and reach it through `tx.db`.
It is already doing exactly that. This fails closed, which is the right direction — it refuses rather than taking a lock that would be released immediately — so it is not a security issue. But either thread the lane through, or give this case its own message.
3. Low — lock() has no zero-key guard of its own; at arity 0 the template is malformed.
With keys = [], lockStatement yields strings.length === 3 against values.length === 1. The tag never consumes the trailing fragment, emitting … IN ($1) ORDER BY "id" LIMIT — FOR UPDATE dropped and LIMIT left dangling, i.e. a Postgres syntax error. The only guard is if (keys.length === 0) return [] in locked() (read.ts:912), a module away. lock() is a member of the public RowLockLane and the suite drives it directly (lock.test.ts:262), so the guard belongs on both sides. Unreachable through the terminal today.
4. Low — RowLockUnavailableError conflates "no transaction" with "the lane could not be built".
createRowLockLane returns undefined when isRawLane/isContract reject the client, and every downstream refusal then asserts the caller is outside a transaction. A client whose raw/contract shape drifts (an rc bump, a hand-built double) inside a genuine transaction gets a message pointing the wrong way; advisoryLock() inherits the same read-oriented wording ("Compose the read inside…"). Worth distinguishing.
5. Low — tests/prisma8-double.ts was not updated for the new required contract member.
unsafe.ts adds readonly contract: object as required, but prisma8Double returns a literal typed UnsafeCapableClient without it. Confirmed a real error:
tests/prisma8-double.ts(45,3): error TS2741: Property 'contract' is missing in type
'{ sql: {}; raw: {}; orm: …; runtime: …; transaction: … }' but required in type 'UnsafeCapableClient'.
Nothing catches it because packages/core/tsconfig.json has "include": ["src/**/*"], so tests/ is never type-checked. Runtime degrades gracefully (contract undefined → isContract false → lane absent → fail-closed), but the type is wrong today.
Nit
lockLane's doc says a bound "the terminal will not honour" is refused before any statement, which is accurate for a composed .limit(). An unbounded .forUpdate().all() over a very large scoped set still materialises every row before lock() refuses — inherent, since the key count is not knowable earlier, but a defensive limit(ROW_LOCK_MAX_KEYS + 1) on the read would bound it.
Falsifiability spot-checks all passed: removing FOR UPDATE, moving the executor off the transaction, and opening the type gate each produced a failing test. This is well-tested work; the mechanism is right.
…n take one Review findings on #1335. The locked row carries its columns as of BEFORE the lock — the terminal reads first and locks second, and each statement takes its own snapshot — so the flagship capacity gate, which read `capacity` off the locked row and used it as the gate's limit, taught a pattern that is stale under contention. Both `CLAUDE.md`s, the changeset, `forUpdate()`'s own TSDoc and the gate suite now read the threshold in its own statement after the lock, the way the count already was, and each states the semantics. A new escape-only test holds the window open with a blocker and proves the two values differ. A hook running inside `context.transaction()` can now take a row lock: the lane travels on `AccessContext._rowLock`, so every path that rebuilds the `db` delegate hands it back. A write that opened its OWN transaction still drops it — the lane is a different transaction's. `lock()` guards arity zero itself, where the template is malformed rather than empty. `RowLockLaneUnavailableError` separates "this client cannot compose the statement" from `RowLockUnavailableError`'s "there is no transaction". The Prisma 8 test double declares the `contract` its type has required since #1155. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Addressed all five findings on 1. BLOCKING — the flagship example taught a stale thresholdYou are right, and the mechanism is behaving as designed — so this is a documentation fix, not a terminal change. ADR-0047 deliberately chose the two-statement shape, so the terminal still does not re-read; what changed is that the example no longer depends on the half of its answer that is pre-lock. The gate now reads both of its sides after the lock, each in its own statement: const held = await tx.db.Slot.where({ id: { equals: slotId } }).forUpdate().first()
if (held === null) return { booked: false }
// `held.capacity` is the row as of before the lock; this re-read is not,
// because no one else can commit an update to a row this transaction holds.
const slot = await tx.db.Slot.where({ id: { equals: slotId } }).first()
const { taken } = await tx.db.Booking.where({ slotId: { equals: slotId } }).aggregate(
(aggregate) => ({ taken: aggregate.count() }),
)
if (slot === null || taken >= slot.capacity) return { booked: false }Corrected in all five places you named — Test. Mutation — the one line back to 2. MEDIUM — a hook inside a transaction can now take the lockThreaded, rather than reworded. The lane travels on One boundary is deliberate and documented: Mutation — reverting 3. LOW — zero-key guard on
|
borisno2
left a comment
borisno2
left a comment
There was a problem hiding this comment.
Re-review — PR #1335 (follow-up, head 0b98a47a)
Verdict: APPROVE. Posted as a Comment review because the gh identity here is the PR author and GitHub blocks a self-Approve event — treat this as an approval.
Re-verified by checking out 0b98a47a, pnpm install, pnpm build --force (11/11), running the suites on both harnesses, and mutating production code to break each new assertion. Every number in the response comment reproduced exactly: core PGlite 1644 passed / 6 skipped; core on PostgreSQL 14 1613 passed / 37 skipped with only typed-read-surface.test.ts failing on CREATE EXTENSION vector (#1336, not this PR); cli 410; ui 611; rag 456; auth 501; storage 197 / s3 43 / vercel 56; create-opensaas-app 37. pnpm lint 0 errors + the 2 pre-existing warnings, prettier --check clean, manypkg check clean, check:prisma-error-codes clean with the allowlist unchanged.
The priority: can a lock ever be held by the wrong transaction?
No. I could not construct a path that takes a lock on a connection other than the one the surrounding work runs on, and the boundary the response describes turns out to be stronger than stated.
The pairing is structural. context._rowLock's executor is always _unsafeTransaction, and a transaction-bound context.ormHandle is always derived from the same opened object (transactionOpenerFor → ormHandleFor(config, tx.orm), which mints a fresh models object per transaction, so handle identity is a faithful proxy for transaction identity). I walked every path that can reach the lane after a rebind:
- A hook writing through a handed context (
bindContextToTransaction,write-pipeline.ts:263).opener = existingOwner ? undefined : context._transactionOpener, andrunInTransactiononly yields atxdifferent fromargs.ormHandlewhenopeneris defined. EveryWritePipelineArgsis constructed bypopulateDbDelegate, whereormHandleandcontext.ormHandleare the same value by construction — I checked all threerunWritePipelinecall sites. deriveResolveOutputContext(field-visibility.ts:127) spreads the context and rebuildsdbagainstcontext.ormHandle, so it cannot break the pairing.- The sudo surface and session substitution re-enter
getContextwith the sameormHandle,clientand_unsafeTransaction, so they build a fresh lane over the same transaction scope.transactionFacewrapsbase.sudo()/base.withSession()with the enclosinglock, itself over that scope. - A nested transaction. With
_transactionOwnerset,transaction()runsfn(transactionFace(returned, lock))on this context's own lane. InrunTransactionBody, all three branches derive handle and scope together:child(opened.ormHandle, opened.unsafe)alongsiderowLockSeat(client, opened.unsafe), andchild(ormHandle, _unsafeTransaction)for the join. The$transactionbranch yields no scope, so no lane and a refusal — the documented Known-limits branch, unreachable today.
And the invariant is stronger than the guard. _rowLock is set iff client && _unsafeTransaction; _transactionOpener is set iff client && !_unsafeTransaction (transactionOpenerFor returns undefined when already inside a transaction). They are mutually exclusive, so opener !== undefined ⟹ context._rowLock === undefined, and the drop branch of tx === context.ormHandle ? … : undefined is never reached with a lane present.
Two experiments confirm that rather than leaving it as reasoning:
- Replacing the guard with a
throwon exactly that condition and running the whole core suite on Postgres: the probe never fired (1613 passed / 37 skipped, only the pgvector file). - Removing the guard entirely and running the whole core suite on Postgres: nothing changed — same 1613/37.
And the harness genuinely detects a wrong-connection lock. Mutating rowLockSeat to build the lane over client.runtime() (the pooled executor) instead of the transaction scope fails 11 tests, including the new hook test and advisoryLock, with timeout exceeded when trying to connect and the capacity gate breaking. So a regression that moved the lock off the transaction's connection would not pass CI.
No capability regression on the other side either. The suite covers the write-pipeline hook path but nothing else, so I wrote a throwaway probe against real Postgres for the paths it does not: tx.sudo().db.X.forUpdate().all(), tx.withSession(s).db.X.forUpdate().all(), tx.sudo().advisoryLock(k), a nested tx.transaction(inner => inner.db.X.forUpdate().all()), and a field-level resolveOutput hook taking a lock (the deriveResolveOutputContext rebuild). All five take the lock, each issuing exactly one raw FOR UPDATE on the transaction's lane. Nothing is withheld where it should work.
The documentation fix — the argument holds, and I tested it
"this re-read cannot be stale, because no one else can commit an update to a row this transaction holds."
True, for every case the example covers. Every writer of that row — UPDATE and DELETE alike — must take the same row-level exclusive lock, so nobody can commit a change to it between our acquisition and our commit; and under Read Committed each new statement's snapshot includes everything committed before it started, so a change committed while our lock statement waited is visible to the re-read. Verified directly on PostgreSQL 14 with two connections:
- A competitor that had already read before the lock was taken (the case named in review): it read
2, we then took the lock, our re-read returned2, itsUPDATEblocked for as long as we held the lock, and its value landed only after our commit. That is correct serialisation — we committed first — not staleness. - A competitor that committed while our lock statement waited: statement 1 saw
5, the blocker committed9, our post-lock re-read returned9. Not the stale value.
One boundary worth knowing, and not a defect: at REPEATABLE READ the re-read would use the transaction snapshot, but the terminal's own lock statement raises first — I confirmed 40001: could not serialize access due to concurrent update. So it fails loudly rather than returning stale data, and ADR-0042 / types/context.ts:84 already pin the level as the connection default. The docs' "under Read Committed" wording is accurate.
The semantics are now stated in the changeset, both CLAUDE.mds, capacity-gate.test.ts and SecuredQuery.forUpdate's TSDoc, including the divergence from a single-statement SELECT … FOR UPDATE. The new escape-only gate test is a real characterisation test — it asserts onLockedRow === 2 and afterLock === 0 through the real terminal against real Postgres, so it would fail if the terminal ever started re-reading columns post-lock. All 3 gate tests pass on PostgreSQL 14.
The remaining fixes — each verified by breaking it
| Fix | Verified |
|---|---|
Zero-key guard on lock() (lock.ts:363, ahead of the ROW_LOCK_MAX_KEYS check) |
Removing it fails the new arity-0 test with 42601 syntax error at end of input |
RowLockLaneUnavailableError + unusableRowLockLane() seat |
Removing the ?? unusableRowLockLane() fails the new test: expected RowLockUnavailableError … to be an instance of RowLockLaneUnavailableError. The refusal lands in lockLane() via identity(), ahead of collection.all() in both runAll and runFirst — before any statement, as claimed |
contract: {} on prisma8Double |
Removing it reproduces TS2741: Property 'contract' is missing under an isolated tsconfig that includes tests/. Still uncovered by any checked config (packages/core/tsconfig.json includes src/**/* only) — filing that separately is the right call |
Nothing from the clean findings regressed
forShare / NOWAIT / SKIP LOCKED appear nowhere in production code; nowait survives only inside the test's own second-connection probe, which is a measurement. RowLockKey<Tx> still admits the single member, and typed-read-surface.test.ts:150's Exact<Exclude<PromisedLockingMember, PromisedQueryMember>, 'forUpdate'> is still falsifiable — flipping RowLockKey to always yield 'forUpdate' fails to compile at :148 and :150. The compile-time refusal outside a transaction still holds from a real emitted bundle (types-row-lock.test.ts, in the green 410). No any, no casts, no @ts-ignore in the diff; @ts-expect-error appears only where it is the assertion. unknown stays internal — parameter types on createRowLockLane and the two type guards; the public entry point exports the four error/constant names alone. Base is prisma-8, changeset minor for both packages.
Findings
Low — one place the semantics is still missing, and it is the one a consumer's editor shows.
packages/core/src/types/secured-list.ts:521–533 is the forUpdate() TSDoc on the typed generated surface: <List>TxList extends SecuredList<Contract, Remainder, 'List', true>, and the member comes from the RowLock mapped type. That is the docblock a project developer sees when hovering tx.db.Slot.…forUpdate(). It still carries only "the locked set is a subset of the readable one" and the vanished-row rule; the pre-lock-columns paragraph went into SecuredQuery.forUpdate in read.ts, which is core's own untyped engine view and not what a consumer hovers. By this repo's own comment rule — public API docblocks exist because they are what the consumer's editor shows — this is the single place the new semantics most needs to be stated and is not. Worth a short paragraph mirroring the read.ts one.
Low — the boundary foregrounded in the response is defensive, not live, and the test named for it does not exercise it.
Per the analysis above, _rowLock and _transactionOpener are mutually exclusive, so bindContextToTransaction's tx === context.ormHandle check never actually drops a lane that exists. The new test "the same hook under a write that opened its own transaction refuses" runs on database.context(anonymous) — a context that never had a lane — so it passes identically with the guard deleted (confirmed: whole core suite green with the guard removed). The guard is correct and I would keep it; it just guarantees an invariant that a different mechanism already guarantees. Worth one clause in the bindContextToTransaction docblock saying so, and a test name that does not imply it is load-bearing.
On the declined nit
I agree with declining the defensive limit(ROW_LOCK_MAX_KEYS + 1). It would change the statement the terminal issues — which this suite deliberately pins and which ADR-0059's stamping observes — and it degrades the refusal's diagnostic, since RowLockKeyLimitExceededError would always report 1001 rather than the true key count. On an unordered read it also silently picks an arbitrary 1001 rows, and it has to be reasoned about against offset/cursor. Same outcome for every compliant read, real trade-offs otherwise: a decision, not a fix. A follow-up issue is the right home if the unbounded materialisation cost turns out to matter.
The mechanism is right, the fixes are real and falsifiable, and the two remaining items are documentation. Ship it.
The pre-lock-columns paragraph landed on `SecuredQuery.forUpdate`, which is the engine's own untyped view. `tx.db.Slot.…forUpdate()` resolves to the `RowLock` mapped type in `types/secured-list.ts`, and that is the docblock a project developer's editor shows — so the semantics goes there too. Also stops the write-pipeline lane test from claiming a guard it does not exercise. `_rowLock` and `_transactionOpener` are set on mutually exclusive conditions, so `bindContextToTransaction`'s `tx === context.ormHandle` check never drops a lane that exists; the test runs on a context that never had one. Kept the guard, named the test for what it checks, and said in both places that the check is defence in depth. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Brings in #1330, which stops a directly-written foreign-key column being silently dropped. The two sides overlap in `context/index.ts` and `context/write-pipeline.ts` only, in disjoint functions — `serverAction` and `runWriteInTransaction` on the base's side, the lane construction and `bindContextToTransaction` on this branch's. Nothing here depended on the old behaviour: `capacity-gate.test.ts` declares its `slotId` as a plain `text()` column deliberately, and `lock.test.ts`'s `Booking.slot` relationship is never written. The interaction runs the other way — a probe against a REAL relationship shows the capacity-gate example this PR documents only works after #1330; before it, the written `slotId` came back undefined. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both residual findings addressed, plus the base merge (head
|
| Check | Result |
|---|---|
pnpm build --force |
11/11 successful |
pnpm lint |
0 errors, 2 pre-existing warnings |
prettier --check . |
clean; manypkg fix no changes |
check:prisma-error-codes |
clean, 17 literals / 5 files — allowlist unchanged |
stack-core test:coverage |
1665 passed / 6 skipped, exit 0 — no per-file threshold tripped (src/context 94.51% stmt / 96.18% line; write-pipeline.ts 97%) |
stack-core on PostgreSQL 14 |
1634 passed / 37 skipped — only typed-read-surface.test.ts, on CREATE EXTENSION vector (#1336, unrelated) |
stack-cli |
411 passed |
stack-ui / stack-auth / stack-rag |
628 / 501 (80 skipped) / 456 passed |
Working tree clean after every mutation experiment; each restored file re-verified against the suite.
The base moved again during verification (#1332). Three conflicts, all purely additive: `_config` and `_rowLock` are separate optional members that each side appended to `AccessContext`, to `getContext`'s context literal and to `bindContextToTransaction`'s. Both are kept in each. `deriveResolveOutputContext` needed nothing — it spreads the context, so it carries both new members already. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up: the base moved again mid-verification (head
|
| Check | Result |
|---|---|
pnpm build --force |
11/11 successful |
pnpm lint |
0 errors, 2 pre-existing warnings |
prettier --check . |
clean |
check:prisma-error-codes |
clean, 17 literals / 5 files — allowlist unchanged |
stack-core test:coverage |
1689 passed / 6 skipped, exit 0 — src/context 95.02% stmt / 96.56% line, no threshold tripped |
stack-core on PostgreSQL 14 |
1658 passed / 37 skipped — only typed-read-surface.test.ts on CREATE EXTENSION vector (#1336, unrelated) |
stack-cli |
411 passed |
stack-rag |
470 passed |
All 61 changeset entries on origin/prisma-8 survive byte-identical (blob hashes compared against the index), and mellow-gates-hold.md is unchanged at 8f62b5d — the same blob it has on 0b98a47a — still minor for both packages.
The blog case timed out on CI under coverage at vitest's 5 s default (#1291). Measured, the case costs ~80 ms uninstrumented and ~260 ms under coverage, so the failure was scheduling, not work: it runs alongside the tsc-spawning compile tests in this package, two of which this branch added. All eight fixtures get 30 s, with assertions unchanged. Also pins the seam between this branch's row-lock lane and the junction edge creation that landed in #1329. Both extend the context, both were merged into it silently, and the transaction rebind lists its members by hand (#1345). The new test drives both through every build path — plain, sudo(), withSession(), the transaction rebind and its derived contexts — and fails if either is dropped on any of them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Merged The seam with #1329Both this branch and #1329 extend New test: All paths carry both. No member is dropped. I checked the test actually has teeth rather than trusting a green result, by breaking the merged code deliberately:
The first break is worth calling out: my initial version of the test passed with the lock dropped from the rebind. The lock still cannot be taken on the wrong connection. PGlite serialises transactions, so the contention cases skip on the default harness. Run against a real Postgres 14 via
#1329's own purpose holds. The contract-equivalence timeoutI measured before changing the budget, and the causal story in the brief does not survive measurement — reporting it rather than quietly raising the number.
Against a 5000 ms default that is a 20–60x gap, so the CI failure was a scheduling stall, not work. Two corrections to the diagnosis:
What this branch did add is So contract generation is not dramatically slower — it is unchanged on this path. Budget set to Verification on the merged tree
Changesets: every changeset on |
1 participant
Implements #1155. Part of #1124.
.forUpdate()on a transaction-bound builder, carried byfirst()andall(), plustxContext.advisoryLock(key). See ADR-0047 and ADR-0062.What the terminal does
SELECT <pk> FROM <table> WHERE <pk> IN ($1…$n) ORDER BY <pk> LIMIT $n+1 FOR UPDATEthrough the contract-bound raw tag: table and key column off the contract's storage, identifiers quoted by@prisma/orm-postgres/target/sql-utils, each key bound with the identity column's own codec viaparam(). It runs on the transaction's executor, inside the terminal'swithOrigin('engine', …).Acceptance
uuidkey renders$1::uuid,cuid2(text) key renders$1,all()binds one placeholder per key withLIMIT $n+1. This suite is the one place asserting on rendered SQL; ADR-0062 grants the exception because the statement is the subject.EXPLAINon PGlite shows a lock node — the engine's own recorded statement isEXPLAINed verbatim on a connection of its own and the plan containsLockRows.IN ()is a Postgres syntax error, not an empty result.limitaboveROW_LOCK_MAX_KEYSis refused withRowLockKeyLimitExceededErrorbefore the read runs; a returned key set above it is refused before the lock statement.forUpdateon a non-transaction builder is a compile error — checked from a real emitted bundle (packages/cli/src/generator/types-row-lock.test.ts):error TS2339: Property 'forUpdate' does not exist on type 'ListQuery<Contract, Remainder, "Post", unknown, never, false>'.FOR UPDATE NOWAITfails55P03while the lock is held and succeeds after commit.Parked by #1154, picked up here
packages/core/src/secured/capacity-gate.test.ts), re-expressed with.forUpdate()against a real database. Five racers, each on its own client, pool and connection, against a capacity-two slot admit exactly two. The racers genuinely overlap: a barrier of five inside the transactions does not release until every one of them is open, so a run in which any two did not overlap hangs and fails rather than passing with the wrong answer. A control case in the same file — the same gate without the lock — admits all five, which is the built-in demonstration that the lock is what provides the guarantee.packages/core/CLAUDE.mdgains theforUpdate()example, consistent with the rootCLAUDE.md's description.Constraints honoured
forUpdate()only — noforShare, noNOWAIT, noSKIP LOCKED.aggregate()andnearest()refuse the modifier rather than silently dropping it:forUpdateis aReadPlanmember, so each terminal'sPlanDispositionshas to say what it does with it. The engine always emitsORDER BY <pk>. A table without a single-column primary key raisesRowLockIdentityError.Surface
StackContextgains a fourth type parameterTxDB(defaulting toDB, so every existing instantiation is unchanged) andStackTransactionContextbecomes a real interface carryingadvisoryLock. The generator emits<List>TxListbeside<List>Listand aTxDBinterface, and bindsTransactionContextto it.UnsafeCapableClientgainsreadonly contract: object— the row lock reads the identity column and its codec off the contract's storage.Verification
pnpm lint— 0 errors, 2 warnings (both pre-existing, inexamples/blogandpackages/cli/src/migration).pnpm exec prettier --check .— clean.pnpm manypkg fix— clean.pnpm build --force— 11/11 tasks.pnpm check:prisma-error-codes— clean, allowlist unchanged.pnpm build --force.DATABASE_URL): 1608 passed / 23 skipped, withtyped-read-surface.test.tsexcluded — that server has no pgvector and the file fails identically on this branch's base.Every new assertion was proved falsifiable by breaking the production code and watching it fail: dropping
ORDER BY(3 spelling tests), dropping the locked-subset filter (the vanished-row test,expected [ 'doomed', 'kept' ] to deeply equal [ 'kept' ]), dropping the no-keys guard (42601 syntax error at end of input), dropping either bound (both refusal tests), running the lock on the client's executor instead of the transaction's (7 tests, all deadlocking on the pool), and droppingFOR UPDATEfrom the statement (the capacity gate,expected [ true, true, true, true, true ] to have a length of 2).🤖 Generated with Claude Code