Skip to content

chore: lockfile maintenance via pnpm dedupe and stale policy exception cleanup - #190

Merged
btravers merged 1 commit into
mainfrom
copilot/lockfile-maintenance
Aug 3, 2026
Merged

chore: lockfile maintenance via pnpm dedupe and stale policy exception cleanup#190
btravers merged 1 commit into
mainfrom
copilot/lockfile-maintenance

Conversation

Copilot AI commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

This PR performs lockfile maintenance to keep dependency resolution current and removes now-unnecessary release-age policy exceptions. It keeps security overrides intact while reducing lockfile drift.

  • Dependency maintenance

    • Re-resolved and deduped workspace dependencies (pnpm install --no-frozen-lockfile + pnpm dedupe).
    • Updated pnpm-lock.yaml with deduped transitive graph (notably collapsing duplicate transitive entries and moving to newer already-allowed transitive versions).
  • Policy cleanup (pnpm-workspace.yaml)

    • Removed temporary minimumReleaseAgeExclude entries that were only needed while patched versions were younger than the policy window:
      • fast-uri
      • brace-expansion
    • Left all vulnerability-driven overrides unchanged.
  • Resulting config delta (illustrative)

    minimumReleaseAgeExclude:
      - "@btravstack/commitlint"
      - "@btravstack/lefthook"
      - "@btravstack/oxlint"
      - "@btravstack/theme"
      - "@btravstack/tsconfig"
      - "@btravstack/typedoc"

Copilot AI changed the title chore: lockfile maintenance (pnpm dedupe + policy cleanup) chore: lockfile maintenance via pnpm dedupe and stale policy exception cleanup Aug 3, 2026
Copilot AI requested a review from btravers August 3, 2026 11:31
@btravers
btravers marked this pull request as ready for review August 3, 2026 12:08
Copilot AI review requested due to automatic review settings August 3, 2026 12:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR performs dependency graph maintenance for the monorepo by re-resolving/deduping the pnpm lockfile and cleaning up stale minimumReleaseAgeExclude exceptions now that the relevant patched releases are older than the policy window, while keeping the existing security overrides in place.

Changes:

  • Removed temporary minimumReleaseAgeExclude entries for fast-uri and brace-expansion from pnpm-workspace.yaml (overrides remain).
  • Refreshed pnpm-lock.yaml via re-resolution + dedupe, collapsing duplicate transitive entries and aligning to newer already-allowed transitive versions (e.g. rollup linux binary + obug).

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

File Description
pnpm-workspace.yaml Removes now-unnecessary release-age policy exclusions while retaining vulnerability-driven overrides.
pnpm-lock.yaml Updates the resolved dependency graph after pnpm install + pnpm dedupe, reducing lockfile drift and deduping transitive versions.
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@btravers
btravers merged commit fb1b5ec into main Aug 3, 2026
15 checks passed
@btravers
btravers deleted the copilot/lockfile-maintenance branch August 3, 2026 12:12
btravers added a commit that referenced this pull request Aug 4, 2026
…review feedback

brace-expansion 5.0.8 and fast-uri 3.1.4 were both pinned as security fixes;
both advisories have since been widened past those versions, so the pins were
stale. Lift them to 5.0.9 and 3.1.5, and add a 2.x brace-expansion pin for the
line @testcontainers/postgresql pulls in through archiver.

All three patched releases are younger than the 7-day minimumReleaseAge cutoff,
so re-add the temporary excludes PR #190 removed once the previous round had
matured, each naming its removal condition.

Also from PR review: start @unthrown/drizzle at 0.0.0 so the minor changeset
publishes 0.1.0 rather than skipping it, and fix a skill snippet that described
an awaited builder as an AsyncResult when it is a Result.
btravers added a commit that referenced this pull request Aug 4, 2026
…tion returning AsyncResult (#195)

* feat(drizzle): scaffold @unthrown/drizzle package

Add the package skeleton for the Drizzle ORM Postgres integration: two
entrypoints (`.` and `./node-postgres`), build/test/typecheck/docs
scripts, and catalog entries for drizzle-orm, pg, and the PGlite test
harness deps. No integration logic yet — just a placeholder export and
a smoke test to prove the package builds and runs.

@types/pg and pg pin to the latest release outside the workspace's
7-day minimumReleaseAge window (8.20.0 and 8.22.0) rather than the
brief's memory-written 8.15.6/8.16.3. drizzle-orm stays pinned to the
brief's 1.0.0-rc.4 per policy (we subclass its internals).

* feat(drizzle): add tagged constraint errors and qualifyPgError

Five TaggedError classes (UniqueConstraintViolation, ForeignKeyViolation,
CheckViolation, ExclusionViolation, NotNullViolation) and qualifyPgError,
which triages a Postgres driver failure into the modeled 23xxx
integrity-constraint codes or the defect channel. Also collapses the
package's typecheck script to a single tsc pass until Task 10 adds
types.test-d.ts and restores the two-pass form.

* test(drizzle): add PGlite wire-protocol test harness

PGlite (Postgres compiled to WASM) served over the real wire protocol
by pglite-socket, so `pg.Pool` connects to genuine SQLSTATE-emitting
Postgres in-process, no Docker required — same rationale as
@unthrown/prisma's in-memory SQLite harness.

maxConnections must be raised well above the pglite-socket default of
1: pg-pool destroys and reopens its connection on any query error,
including the ordinary 23505s these tests provoke on purpose, and the
replacement connection races the dying one's async teardown under the
default cap (see task-1-report.md). Port 0 + getServerConn() picks up
the OS-assigned port, avoiding a bind-then-reuse race.

Includes a fixture-isolation test proving two concurrent startPg()
instances never share tables, which task 9's connection-loss suite
depends on.

* fix(drizzle): defend partial-startup cleanup in the test harness

Both of startPg()'s partial-startup failure paths (server.start()
throwing, and an unparseable getServerConn() result) previously ran
their cleanup steps sequentially and un-defended, unlike stop(): a
failing server.stop() would skip db.close() (leaking the WASM
instance) and would replace the original diagnostic error instead of
surfacing alongside it.

Factor the collect-and-continue discipline stop() already used into a
shared collectErrors() helper, and route both startup failure paths
(now unified in a failStartup() closure) and stop() through it. The
original triggering error is always the one thrown; a cleanup failure
is appended into an AggregateError rather than replacing it.

Adds direct unit coverage of collectErrors()'s continue-past-failure
behavior with deliberately failing steps, since forcing a genuine
PGlite/pglite-socket failure to reach the startup paths isn't reachable
without contorting the harness's design.

* feat(drizzle): add AsyncResult prepared query and abstract session

* test(drizzle): cover UnthrownPgSession execute, arrays and objects

* feat(drizzle): add AsyncResult builder tree and UnthrownPgDatabase

Eight subclasses of drizzle's base pg-core builders — select, insert, update,
delete, count, raw, relational query and refresh-materialized-view — each
adding the execution half (_prepare/prepare/execute) over an AsyncResult, plus
UnthrownPgDatabase, the entry points that build them.

Awaitability is a one-line `then` per builder rather than drizzle's
`applyMixins(..., [QueryPromise])`: QueryPromise declares
`execute(): Promise<T>`, which contradicts ours, and applyMixins is @internal
and absent from drizzle's published .d.ts.

The session's row-mapper parameter is widened to `never[]` (PgRowMapper) so
drizzle's own mappers, which expect unknown[][], pass strictFunctionTypes.

* fix(drizzle): route query-compilation throws to the defect channel

`execute()` was `this._prepare().execute(pv)`, so only the driver call sat
inside `fromPromise`'s thunk. `_prepare()` runs `getSQL()` ->
`dialect.buildSelectQuery` -> `dialect.sqlToQuery`, which throws for a
type-legal, reachable mistake (selecting a column from an unjoined table).
Because `then` calls `execute()` synchronously, that throw escaped the
thenable and REJECTED the awaiting promise — contradicting the package's
stated contract and crashing a consumer folding with `match()` and no
try/catch.

Compilation now happens inside the boundary via one shared `runQuery` helper
in awaitable.ts, covering all seven compiling builders; `raw.ts` already held
a prepared query. `resultThen(this)` replaces the repeated `then` body.

`db.execute()` stays eager and is documented as such: it is the builder
factory, not the run, and deferring would cost PgRaw's synchronous accessors.

* fix(drizzle): compile raw-SQL session queries inside the boundary

`UnthrownPgSession.execute` / `arrays` / `objects` called
`dialect.sqlToQuery(...)` and `prepareQuery(...)` outside `fromPromise`,
so a compilation throw escaped synchronously — past a caller who has no
try/catch, because these methods promise a `Result`. The same bug was
fixed across the seven builders; this is the last site, and Task 7 is the
first caller of these methods.

They now go through the `runQuery` helper, which runs compilation inside
the same boundary that triages the driver's rejection. `PgQueryMode` is
exported alongside, so a driver implementing the abstract session can
name the parameter type it already has to accept.

* feat(drizzle): add node-postgres session with Result transactions

`NodePgUnthrownSession` speaks to a real pg client; `NodePgUnthrownTransaction`
is the handle its callback receives.

Ok commits, Err and Defect both roll back. An Err still re-surfaces typed,
so rolling back costs no information — and because rollback *is* returning
an Err, there is deliberately no `tx.rollback()`.

Since unthrown has no public way to mint a Defect, the sequence is written
as a promise that may reject and qualified exactly once at a `fromPromise`
boundary, then flattened with `flatMap`. The control statements go through
`runUnqualified()`: awaiting an AsyncResult never throws, so a COMMIT run
through the normal path would resolve to an Err nobody reads — reporting
success having committed nothing. A COMMIT that raises 23505 (a DEFERRABLE
constraint) is therefore still a modeled UniqueConstraintViolation, which
is why `transaction`'s E carries PgQueryError whatever the callback's is.

A rollback that fails takes over the outcome as a defect — the transaction's
state is then unknown — but carries what it was undoing in an AggregateError,
so it never destroys the failure it was rolling back.

Nested transactions are savepoints under the same rule, and a pooled client
is checked out for the transaction and released on every path, a rejecting
control statement included.

* fix(drizzle): pin pooled transactions to the checked-out connection

Three findings from review of the node-postgres session.

The pooling tests could not tell whether a transaction ran on the pooled
connection or on the pool: the fake pool delegated `query` to its client, so
both paths recorded into one array and every assertion passed either way. A
pool hands out an arbitrary connection per statement, so fanning a transaction
across it commits on a connection with no transaction open, leaves the work
uncommitted and returns a stale open transaction to the pool — the worst bug
available here, and invisible to the suite. The fake pool now records
separately, the pooling tests assert the pool itself ran nothing, and a
builder query inside a pooled transaction covers the realistic shape.

Savepoint names now come from a counter shared by every handle descended from
one transaction, not from nesting depth. Depth alone gives two sibling nested
transactions the same `sp1`; started concurrently — which `allAsync` makes easy
to write — the first `rollback to savepoint sp1` unwinds the other's work.

`#runSavepoint` built the nested handle with the four-argument form, so
`parseRqbJson` silently reverted to false one level down. It is threaded now.

* feat(drizzle): add node-postgres drizzle() factory and public exports

Replace drizzle-orm/node-postgres' own `drizzle()` rather than wrapping it:
every method on the returned database already speaks `AsyncResult`, so
migrating a call site is an import change. Keeps drizzle's connection-string,
`{ client }` and `{ connection }` forms, plus the positional-client form the
task brief specifies, discriminated by drizzle's own `isConfig` (a `pg.Client`
carries a `connection` property of its own, so a key-presence test would
misread one).

Wire `transaction` onto `NodePgUnthrownDatabase` as a one-line delegate to
`NodePgUnthrownSession.transaction` — the same shape drizzle's own async
database uses — which makes Task 7's `db.transaction(...)` TSDoc examples true.
The base `UnthrownPgDatabase` deliberately stays without it: its session's
transaction handle is unresolved, because the base facade is built underneath
the transaction class that extends it.

Decide the public surface: the root entry exports the dialect-agnostic pg-core
tree (database, session, prepared query, the eight builders and their result
types) so consumers can write type annotations; `./node-postgres` exports the
factory, database, config and session/transaction classes. Also exports
`PgQueryMode`, `PgRowMapper` and the Insert/Update/DeleteResult types, which
were module-local but appear in public signatures.

* fix(drizzle)!: give reads E = never, enforced at runtime, and drop the positional drizzle() form

Reads had `E = PgQueryError`, forcing every caller to enumerate five
integrity-constraint violations a SELECT can never raise. Narrow the four read
builders — select, $count, db.query.*, refresh materialized view — to
`AsyncResult<T, never>`, and route them through a new `runSafeQuery`
(`fromSafePromise`, the named "everything here is a defect" boundary) so the
runtime cannot contradict the type.

The runtime half is the point. Declaring `never` while still qualifying would
let a 23xxx raised on a read path — a SELECT calling a volatile function that
writes — surface as an Err the type says is impossible; a type-exhaustive
mapErrCases would then throw NonExhaustiveError and the modelled error would
silently become a Defect. That is the trap @unthrown/prisma shipped by omitting
RecordNotFound from create/upsert's E. Covered by tests that provoke a real
23505 from a read and assert the Defect channel, verified to fail when only the
runtime half is reverted.

Writes are untouched: insert/update/delete, db.execute and transaction keep the
full PgQueryError union. `ResultThen` gains an `E` parameter defaulting to
PgQueryError, so the write builders are unchanged.

Also drop the positional `drizzle(pool)` overload. Upstream
node-postgres/driver.d.ts has only three forms, and a fourth spelling of
`{ client: pool }` breaks the promise that migrating a call site is purely an
import change. `isConfig` goes with it — the three remaining forms discriminate
on `typeof === "string"` alone.

* fix(drizzle): route a prepared read through the defect boundary too

`prepare(name).execute()` is a third way to run a read, next to `execute()` and
`await`, and it went through `UnthrownPgPreparedQuery.execute` — which qualifies.
So a prepared read produced `Err(UniqueConstraintViolation)` for the exact case
the builder's `E = never` says is impossible, making "a read has no modeled
failure" false wherever `prepare()` is used.

Add `UnthrownPgSafePreparedQuery`, whose `execute` routes through
`fromSafePromise` and returns `AsyncResult<T, never>`, and hand it back from the
three read builders that expose `prepare()` — select, db.query.*, and refresh
materialized view. (`$count` has none: drizzle's `PgCountBuilder` exposes no
`prepare`.) Writes are untouched and still return the qualifying class.

A subclass rather than a type parameter on `UnthrownPgPreparedQuery`: a
parameterised `E` must pick its boundary from an injected function, and the
injected default is not assignable to an unresolved `E`, so that shape needs a
cast exactly where type and runtime must not drift. Overriding `execute` needs
none — `AsyncResult` is covariant in `E` — and the narrow type is reachable only
through the class whose `execute` is the safe one.

Also correct the refresh-materialized-view rationale, which was factually wrong:
a refresh CAN raise 23505 against a unique index, and CONCURRENTLY requires one.
It stays a defect under the "would you branch on it?" rule — a view whose query
yields duplicates is a bug in the view definition.

Drop two bare `as never` casts in the spec by typing the helper at
`Result<unknown, never>`, which also narrows `isDefect` so `cause` needs none.

Move the relational-query test ahead of the error-provoking cases: pg-pool
destroys its connection on any query error, and the replacement racing the dying
one's teardown intermittently desynced that read's response framing.

* test(drizzle): cover SQLSTATE mapping, defect routing and transactions

Twenty-six cases against a real PostgreSQL (PGlite over the wire protocol):
the five modeled 23xxx codes with their constraint/table/column metadata, the
defect routing for 42601/42P01/22P02 and a genuinely lost connection, the
reads-are-E-never ruling proved by a refresh that raises a real 23505, and the
transaction semantics on both a pool and a bare pg.Client, each rollback
checked against the database rather than against the returned Result.

Every fixture keeps exactly ONE live connection. PGlite is a single backend and
pglite-socket multiplexes every TCP connection onto it, so a second connection
is contention on the one session rather than an independent one; a dedicated
probe connection reproduced a response-framing desync ("Received unexpected
parseComplete message from backend"). Driving the error-provoking blocks
through a bare pg.Client also removes the pool churn behind the Task 8 flake:
Pool.prototype.query releases a client WITH the error, which destroys and
reopens the connection, while a standalone client and a checked-out PoolClient
do not.

* test(drizzle): make the rollback assertions able to see an empty read

The six absence checks went through $count, whose countOf maps a missing cell
to 0 (keeping drizzle's own coercion). A read that came back EMPTY — the
response-framing desync this suite exists to be honest about — was therefore
indistinguishable from a genuine zero, and every toBe(0) passed vacuously. The
one failure mode the suite must be able to see was the one it could not.

Each is now a SET assertion over candidate ids that names at least one row which
must still be there, so a vacuous [] fails instead of passing. Demonstrated with
the driver stubbed to return an empty rowset: the old toBe(0) passes, the new
toEqual([1]) throws.

Also releases the lost-connection fixture on its failure path. solo.stop() is a
step of that test, not only its cleanup, so it takes an idempotence guard rather
than a second stop() in the finally — stop() closes the WASM instance and a
second db.close() would surface as an AggregateError out of the cleanup.

* test(drizzle): run the suite against a real PostgreSQL via testcontainers

PGlite (served over the wire protocol by `pglite-socket`) kept the suite
Docker-free, but electric-sql/pglite#958 makes it unusable: after an
errored extended-query batch it answers the client's `Sync` with a SECOND
`ReadyForQuery`, which real PostgreSQL never sends. Measured on 2000/2000
errored batches; when the two land in separate TCP segments `pg` has
already dispatched the next query, so the stray `Z` completes it early
with `rows: []` and everything after it desyncs. The integration suite
failed 7 runs in 8. Neither `@electric-sql/pglite` 0.5.4 nor
`pglite-socket` 0.2.7 fixes it, and the issue is still open.

`startPg()` keeps its contract — `{ pool, stop }` — so no spec changed
shape. What changed is underneath it: a vitest `globalSetup` starts ONE
`postgres:18.4-alpine` container for the whole run (pinned to an exact
patch, so a server upgrade can never move an error message or a SQLSTATE
under a run that changed no code), hands its address to every worker
through `provide`/`inject`, and stops it at the end; each `startPg()`
then costs a `CREATE DATABASE` rather than a container boot. Isolation
stays per database, so the concurrent-fixtures test is unchanged.

`stop()` keeps the `collectErrors`/`failStartup` discipline: pool first,
then `DROP DATABASE IF EXISTS ... WITH (FORCE)` — FORCE because a spec
may deliberately still hold a connection — with every step attempted
independently. A pool probe gives the partial-startup path a real
trigger and proves the fresh database is reachable before a spec's first
query. Verified: zero `unthrown_*` databases survive a run, and no
container outlives it.

12/12 consecutive clean runs (170 tests), 5.3s -> ~2.0s per run.

* test(drizzle): restore the range-and-gist EXCLUDE constraint

The 23P01 case had been standing in a scalar `EXCLUDE (room WITH =)`,
because PGlite ships no `btree_gist` (`CREATE EXTENSION btree_gist`
fails 0A000). A real PostgreSQL has it, so the constraint is now the
canonical form the feature exists for — `EXCLUDE USING gist (room WITH
=, during WITH &&)`, no two bookings of one room overlapping in time —
and the DDL's limitation note is gone.

`during` is a `tstzrange`, declared through drizzle's `customType` since
it has no built-in column for one, and the two inserts now overlap in
time rather than merely repeating a room. PostgreSQL derives the
constraint name from every column in it, so the assertion follows to
`bookings_room_during_excl` — still the server's own name, not one the
schema chose. Verified to raise a genuine 23P01.

Also updates the comments that were PGlite rationale and would now read
as false: the single-multiplexed-backend note explaining why most of the
file drives a bare `pg.Client` (it stays, for what it covers on its own
account), and the desync notes on `rowsUnder`/`survivorsOf` — the paired
positive controls themselves are untouched, and so is every assertion
bar the constraint name above. The lost-connection case gains a
`pool.on("error")` listener: `stop()` now drops the database `WITH
(FORCE)`, and an unlistened `error` event on a `pg.Pool` throws.

* test(drizzle): stop a failing admin close from orphaning a database

Review finding 1. `runOnAdminDatabase` collapsed `client.query(statement)`
and `client.end()` into one error list and rethrew whichever came out, so
a `CREATE DATABASE` that SUCCEEDED on a connection that then failed to
close rejected `startPg` before the pool existed: the caller never got a
`stop()`, and the database was orphaned until the container died. The
comment above the call claimed the opposite postcondition — "nothing to
release if this fails: no database was created".

The two steps are now collected separately, and the helper throws if and
only if the STATEMENT failed. A close failure on the success path is
swallowed deliberately: `pg` destroys the socket on its way out of a
failing `end()`, so there is no resource left for a caller to act on, and
reporting it would break the biconditional that is the only thing keeping
a fresh database attached to a handle that can drop it. The close is
still attempted either way — a failed statement cannot skip it — and a
genuine pair still surfaces as an `AggregateError`. Both the doc comment
and the call-site comment now state the contract they rely on.

Covered by a new case that forces the failure (`pg.Client#end()` does not
fail on its own): the spy closes the connection for real, then throws, so
the test leaks nothing and only the REPORTING is broken. Verified to fail
against the previous implementation —

    × still hands back a usable fixture when the admin connection will
      not close
    Error: admin connection refused to close

* test(drizzle): assert the dropped-database cause, fix two stale comments

Review findings 2 and 3.

Two comments still explained the PGlite failure as pool churn against the
one multiplexed backend. That was an early hypothesis the investigation
inverted — the churn was a mitigation, not the fault; the real cause is
the duplicate `ReadyForQuery` after an errored extended-query batch
(pglite#958), which test-harness.ts already records. A reader was getting
the discredited account from two spec files and the correct one from the
harness. Both now point at #958 and say the pool-churn theory was wrong.

The lost-connection case also no longer described what it provokes. The
old harness took the server away; `stop()` now drops the database `WITH
(FORCE)`, so the query that follows hits either a terminated backend or,
far more often, a reconnect that finds no such database. It is renamed to
"routes a database dropped underneath a live pool to the defect channel",
and `expectDefectCause`'s discarded return is now asserted rather than
dropped — unlike its three sibling defect cases, which each pin a
SQLSTATE.

A probe loop measured 3D000 (invalid_catalog_name) on 4/4 iterations, so
that is the outcome in practice; the socket-death arm is kept in the same
single assertion because it is a genuine race, and pinning only 3D000
would make an unobserved race exactly the kind of flake this harness was
rewritten to remove. Verified live by mutating the pattern, which reports
the real cause rather than passing vacuously.

* test(drizzle): add type-level assertions for per-operation error channels

Pins the package's central typing promise: reads infer `E = never` (select,
$count, db.query.*, refresh materialized view — through all three routes,
including `prepare(name).execute()`), writes carry the full `PgQueryError`
union (insert/update/delete, `db.execute(sql`…`)`, transaction), and the
matcher's exhaustiveness and defect subtraction hold on top of them.

Over-narrowing an error channel is the trap @unthrown/prisma already paid for:
a write's `E` omitted an error the runtime still produced, so a type-exhaustive
`mapErrCases` threw NonExhaustiveError and the modeled error silently became a
defect. These assertions guard both directions.

Restores the two-pass `typecheck` script — tsconfig.test-d.json had no inputs
before this file existed, which is why Task 3 collapsed it to one pass.

* docs(drizzle): correct the preparable-read-path count in the type tests

* test(drizzle): pin the success channel alongside every err-only read assertion

`ErrOf<R>` answers `never` for anything that is not a `Result` at all, so an
err-only `Equal<ErrChannel<…>, never>` pinned "empty channel OR not wrapped".
Eight read assertions had no `OkChannel` pin or `.get()` on the same route and
so passed with the body replaced by plain rows — including both `findMany`
routes and the prepared refresh, and the prepared paths are exactly where a
builder most plausibly stops routing through `UnthrownPgSafePreparedQuery`.

Pairs each with an `OkChannel` pin, which forces the value to still BE a
`Result` carrying the right rows. Also pins the two-argument factory form
(the only place relations flow through the string overload) and drops the
export block, matching packages/core/src/types.test-d.ts.

* docs(drizzle): add the how-to page, wire the API reference, update CLAUDE.md and the agent skill

Ship the documentation for @unthrown/drizzle and make its API reference part of
the site:

- docs/how-to/use-with-drizzle.md, mirroring the Prisma page: the factory's call
  forms, reads inferring E = never (and why that is enforced at runtime rather
  than merely declared), the five modelled SQLSTATEs, the defect channel with a
  recoverDefect retry wrapper for 40001/40P01, transactions and the "rollback is
  returning an Err" rule, and the db.$client escape hatch.
- Wire the package into the VitePress site: guide sidebar, API sidebar,
  copy-docs, and the docs workspace dependency.
- CLAUDE.md: a packages/drizzle bullet in the monorepo layout, plus the Docker
  requirement its testcontainers harness introduces.
- The agent skill gains a Drizzle section (it is a hand-maintained second copy
  and drifts).

Also correct three things the prose got wrong. The README hung .mapErrCases and
.flatMap directly off a query builder, which is a thenable with no such methods,
and pointed a ^? annotation at the wrong expression; db.ts's @example blocks
were inherited verbatim from drizzle and showed awaited builders as if they
yielded rows. src/docs-examples.test-d.ts now compiles every sample the package
ships in prose, so they cannot rot silently again.

build:docs is warning-free: the 39 TypeDoc warnings were @internal helpers
linked with {@link} (unlinked), a @param bound to the wrong overload, and the
module-local ConstraintFields (intentionallyNotExported).

* docs(drizzle): correct the savepoint scheme in CLAUDE.md and tighten the docs mirror

CLAUDE.md described the savepoint naming as "depth-based naming, drizzle's
scheme, collides under concurrently-started nested transactions" — a
parenthetical that had lost its contrast marker, so it read as three
appositives describing our own counter. The source says the opposite: the
counter is shared across every handle descended from one transaction, and
claimed before anything is issued, precisely BECAUSE depth-based naming would
give two concurrently-started nested transactions the same `sp1`. As written
the spec asserted the hazard the implementation exists to remove.

The docs mirror opened "Every code sample this package ships in prose,
compiled", which overclaimed twice: it is a hand-maintained mirror rather than
an extraction, and it checks shapes against a locally declared schema. It also
skipped the one sample most able to rot on its own — the `db.$client` hand-off
to a stock drizzle database, whose shape depends on a foreign factory's
signature across the peer range. That sample is now covered (verified
non-vacuous: breaking `client:` fails the typecheck), and the header states
what is not guaranteed, with the three deliberate divergences numbered and
marked at the sites where they occur.

* fix(drizzle): apply the final whole-branch review wave

Seven items from the final review before merge. No behaviour outside them.

Important 1 — unify the public class naming. The four `UnthrownPg*` exports
become `PgUnthrownDatabase`, `PgUnthrownSession`, `PgUnthrownPreparedQuery` and
`PgUnthrownSafePreparedQuery`, matching both the twelve `PgUnthrown*` siblings
already in `index.ts` and drizzle's own uniform `PgAsync*` tree. `NodePgUnthrown*`
is left alone — it already mirrors drizzle's `NodePgSession`/`NodePgDatabase`.
Settled now because it is public API and unpublished. Noted in the existing
changeset rather than a second one.

Important 2 — restore the query text on failures. `runUnqualified` wraps a driver
rejection in drizzle's own `DrizzleQueryError` before triage, exactly as
`PgAsyncPreparedQuery.execute` does, so a defect names the failing statement and
its params; node-postgres' `DatabaseError` carries `code`/`constraint`/`table`/
`column`/`detail` but not the SQL. Triage is unaffected: `qualifyPgError` already
read the SQLSTATE through one `cause` level, and the earlier "needs the raw
SQLSTATE" rationale was simply wrong.

Important 3 — guard `runScope` against a non-`Result` callback return.
`isOk`/`isErr` read `.tag`, which throws on `null`/`undefined` — the shape a JS
caller produces by forgetting the `return` in `async (tx) => { await ... }`. That
TypeError escaped before any ROLLBACK, so the pooled client went back with BEGIN
still open and the next borrower ran inside a stale transaction. An `isResult`
check now routes anything out of contract to the undo path, and core's own
non-`Result` guard mints the defect.

Minors: drop the scaffold `smoke.spec.ts`; omit an empty transaction-config
clause (`begin `/`set transaction ` were syntax errors, and an empty
`setTransaction` now issues nothing); order `undoScope`'s `AggregateError` as
`[thrown, original]`, core's failure-observer convention; reword the pglite-era
harness test name.

Every fix is pinned by a test proven to fail without it, including a real-Postgres
case asserting the pooled client comes back clean. Existing assertions whose shape
the wrapper changed were strengthened to identity checks, never weakened.

* fix: lift security overrides past the widened advisories and correct review feedback

brace-expansion 5.0.8 and fast-uri 3.1.4 were both pinned as security fixes;
both advisories have since been widened past those versions, so the pins were
stale. Lift them to 5.0.9 and 3.1.5, and add a 2.x brace-expansion pin for the
line @testcontainers/postgresql pulls in through archiver.

All three patched releases are younger than the 7-day minimumReleaseAge cutoff,
so re-add the temporary excludes PR #190 removed once the previous round had
matured, each naming its removal condition.

Also from PR review: start @unthrown/drizzle at 0.0.0 so the minor changeset
publishes 0.1.0 rather than skipping it, and fix a skill snippet that described
an awaited builder as an AsyncResult when it is a Result.
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.

3 participants