fix(driver-sql): give redshift its connect-timeout row so the 10s dialect bound applies

.changeset/redshift-connect-timeout-bound.md

@@ -0,0 +1,5 @@
+---
+"@objectstack/driver-sql": patch
+---
+
+A `redshift` datasource now gets the 10s dialect connect-timeout bound instead of silently degrading to the 15s pool backstop (#11784). `SqlDriver` answers three separate questions about a knex `client` name from three separate tables, and `redshift` was a member of the wire-protocol one (`POSTGRES_WIRE_CLIENTS`, which #11389 put it in so it gets the calendar-day parser pin) while absent from `DIALECT_CONNECT_TIMEOUT`. It reaches the server through the `pg` driver — knex's `Client_Redshift` literally `extends Client_PG` — so it has `connectionTimeoutMillis` and would have obeyed it; it just never received it, and `withConnectBound` skipped the injection. Nothing errored and nothing was logged: the bound was simply 50% looser than the method's own docblock declares ("the effective bound" at 10s, with `pool.createTimeoutMillis` a "strictly looser backstop, reached only by a dialect that has no connect-timeout knob (SQLite) or ignores the one we set"). A `redshift` host is neither of those. The practical consequence is the framework#3769 failure shape — an endpoint that accepts the TCP connection and never completes the handshake makes every query WAIT rather than fail, and the wait was bounded 5s later than declared, with knex's inaccurate "the pool is probably full" wording instead of pg's `timeout expired`. A host that sets its own `connectionTimeoutMillis` or `pool.createTimeoutMillis` is still left alone. `redshift` gains **no** SQL-emission identity from this: the connect-timeout knob is a property of the npm driver doing the connecting, not of which DDL dialect gets compiled, so this is independent of the open support-scope decision (#11756).

packages/drivers/driver-sql/src/sql-driver-11389-date-tz-skew.test.ts

@@ -211,20 +211,40 @@ describe('#11389 — a Postgres `date` never becomes a JS Date', () => {
  });
 
  it('is not gated on the connect-timeout table — the two lists are not the same list', async () => {
- // `redshift` speaks the pg wire protocol but has no connect-timeout knob,
- // so it is absent from DIALECT_CONNECT_TIMEOUT. Measured while fixing
- // #11389: with the session pins reached only through that table's early
- // return, redshift silently opted out of a fix it needs. This asserts the
- // two concerns really are independent — no timeout injected, pin applied.
+ // ⚠️ This case used to read `redshift`'s ABSENCE from DIALECT_CONNECT_TIMEOUT
+ // as its demonstration: pin applied, no timeout injected, therefore the two
+ // concerns are independent. #11784 gave redshift the row it was missing, so
+ // that vehicle is gone. The timeout assertion is INVERTED here rather than
+ // deleted, because the fact this case exists to pin is #11389's — a pg-wire
+ // client reaches the calendar-day parser hook — and that is unchanged.
  const rec = recordingPgConnection();
  const driver = make({ client: 'redshift', connection: 'postgres://u:p@host:5439/d' });
- // knex parses a URL connection into its own object either way, so the
- // readable signal is that no timeout key was injected into it.
- expect((driver as any).knex.client.config.connection.connectionTimeoutMillis).toBeUndefined();
+ expect((driver as any).knex.client.config.connection.connectionTimeoutMillis).toBe(10_000);
  await runAfterCreate(driver, rec.connection);
  expect([...rec.registered.keys()].sort((a, b) => a - b)).toEqual([OID_DATE, OID_DATE_ARRAY]);
  });
 
+ it('tripwire: a session-pinned client with no connect-timeout row re-arms the early return', () => {
+ // The hazard `withConnectBound`'s note describes needs a client that needs a
+ // session pin but has no timeout row. #11784 removed the last one, so there
+ // is nothing to observe — which is exactly why this is a TRIPWIRE and not an
+ // assertion about behaviour. It goes red the moment someone adds such a
+ // client, and points them at the note before they "simplify" the
+ // fall-through into an early return that would silently opt it out.
+ const timeoutTable = Object.keys((SqlDriver as any).DIALECT_CONNECT_TIMEOUT);
+ const sessionPinned = [
+ ...((SqlDriver as any).POSTGRES_WIRE_CLIENTS as ReadonlySet<string>), // #11389 calendar-day pin
+ ...((SqlDriver as any).MYSQL_EMIT_CLIENTS as ReadonlySet<string>), // #3942 UTC session pin
+ ];
+ expect(
+ sessionPinned.filter((c) => !timeoutTable.includes(c)),
+ 'A client that needs a session pin now has no DIALECT_CONNECT_TIMEOUT row. That is ' +
+ 'ALLOWED — but it re-arms the hazard documented inside withConnectBound: the session ' +
+ 'pins must stay reachable when `dialect` is falsy. Confirm the fall-through is intact, ' +
+ 'then update this expectation and refresh that note with the live example you just made.',
+ ).toEqual([]);
+ });
+
  it('chains a host-supplied afterCreate rather than replacing it', async () => {
  const seen: string[] = [];
  const rec = recordingPgConnection();

packages/drivers/driver-sql/src/sql-driver-11550-dialect-client-spellings.test.ts

@@ -241,6 +241,14 @@ describe('#11550 — emission identity is not wire identity (#11756 stays open)'
  expect(d.nowDefaultSql('date').toUpperCase(), spelling).toContain('CURRENT_TIMESTAMP');
  // …while still getting the #11389 calendar-day wire hook.
  expect(emit('POSTGRES_WIRE_CLIENTS'), spelling).toContain(spelling);
+ // …and, since #11784, the connect-timeout bound too. Both are properties
+ // of the npm driver doing the connecting (`pg`), which is why neither
+ // waits on #11756: knex's Client_Redshift/Client_CockroachDB extend
+ // Client_PG for the WIRE while overriding the query compiler for
+ // EMISSION. Timeout row present, emission identity still false — the
+ // independence #11784 asserted, pinned rather than argued.
+ expect((SqlDriver as any).DIALECT_CONNECT_TIMEOUT[spelling], spelling)
+ .toEqual({ key: 'connectionTimeoutMillis', urlKey: 'connectionString' });
  }
  });
 
@@ -275,12 +283,21 @@ describe('#11550 — the client-keyed tables now derive from one source', () =>
  }
  });
 
- it('deriving the table changed no membership', () => {
- // The refactor half must be a no-op. `redshift`'s ABSENCE is load-bearing —
- // `withConnectBound`'s comment cites it as the reason its early return may
- // not skip the session pins.
+ it('membership is the derivation plus its literal extensions, and nothing else', () => {
+ // #11550's refactor half had to be a no-op, and this pinned that. #11784
+ // then added `redshift` — a real membership change, deliberate, and the only
+ // one since. The comment that stood here cited redshift's ABSENCE as
+ // load-bearing documentation for `withConnectBound`'s early-return note;
+ // that example was retired together with the row, and the note now carries
+ // its reasoning directly instead of leaning on this table.
+ //
+ // ⚠️ `cockroachdb` and `redshift` must stay LITERAL extensions of
+ // POSTGRES_EMIT_CLIENTS here. This list now coincides with
+ // POSTGRES_WIRE_CLIENTS, and spelling it as that set instead would grant
+ // both of them SQL-emission identity as a silent refactor side effect —
+ // which is #11756's open decision, not this table's.
  expect(Object.keys(table()).sort()).toEqual(
- ['cockroachdb', 'mysql', 'mysql2', 'pg', 'postgres', 'postgresql'],
+ ['cockroachdb', 'mysql', 'mysql2', 'pg', 'postgres', 'postgresql', 'redshift'],
  );
  });
 

packages/drivers/driver-sql/src/sql-driver-connect-bound.test.ts

@@ -126,6 +126,55 @@ describe('SqlDriver — connection-attempt bound (framework#3769)', () => {
  expect((d as any).knex.client.config.connection).toEqual({ filename: ':memory:' });
  });
 
+ // #11784 — `redshift` reaches the server through the `pg` driver (knex's
+ // `Client_Redshift extends Client_PG`), so it HAS `connectionTimeoutMillis`
+ // and obeys it; it just carried no DIALECT_CONNECT_TIMEOUT row, so nothing
+ // was injected and the attempt fell through to the strictly looser 15s pool
+ // backstop. Nothing errored and nothing was logged — the before-state is
+ // silence, so "it works" is not evidence and the INJECTED CONFIG is pinned.
+ //
+ // The two controls are load-bearing. `pg` already had a row, so a broken
+ // injection path fails them together instead of looking redshift-specific;
+ // `better-sqlite3` legitimately has none (a file open has no handshake), so
+ // an injector that stopped reading the table and timed everything would be
+ // caught rather than read as a pass.
+ it('bounds a redshift connect attempt at the dialect timeout, not the pool backstop (#11784)', () => {
+ const injected = (client: string, connection: any) =>
+ (make({ client, connection, useNullAsDefault: true }) as any).knex.client.config.connection;
+
+ // subject — URL form moves into the pg URL slot with the timeout alongside
+ expect(injected('redshift', 'postgres://u:p@host:5439/d')).toEqual({
+ connectionString: 'postgres://u:p@host:5439/d',
+ connectionTimeoutMillis: 10_000,
+ });
+ // subject — object form gains the key and disturbs nothing else
+ expect(injected('redshift', { host: 'wh.eu-west-1.redshift.amazonaws.com', port: 5439, database: 'dev' }))
+ .toEqual({
+ host: 'wh.eu-west-1.redshift.amazonaws.com', port: 5439, database: 'dev',
+ connectionTimeoutMillis: 10_000,
+ });
+
+ // positive control — a client that already had a row
+ expect(injected('pg', { host: 'db', database: 'app' }).connectionTimeoutMillis).toBe(10_000);
+ // negative control — a dialect that legitimately has no such knob
+ expect(injected('better-sqlite3', { filename: ':memory:' }).connectionTimeoutMillis).toBeUndefined();
+
+ // The two bounds must not be equal (knex wins a tie and the accurate
+ // message is never seen). Redshift now takes the strict 10s dialect bound
+ // with the 15s pool value still strictly looser behind it — which is the
+ // whole of what #11784 restores.
+ const d = make({ client: 'redshift', connection: 'postgres://u:p@host:5439/d' });
+ expect((d as any).knex.client.config.pool.createTimeoutMillis).toBe(15_000);
+ });
+
+ it("leaves a redshift host's own explicit connect timeout alone (#11784)", () => {
+ const d = make({
+ client: 'redshift',
+ connection: { host: 'wh.redshift.amazonaws.com', connectionTimeoutMillis: 60_000 },
+ });
+ expect((d as any).knex.client.config.connection.connectionTimeoutMillis).toBe(60_000);
+ });
+
  it('leaves a function-valued connection alone — the host builds each one itself', () => {
  const provider = () => ({ host: 'x' });
  const d = make({ client: 'pg', connection: provider });

packages/drivers/driver-sql/src/sql-driver.ts

@@ -4576,14 +4576,30 @@ export class SqlDriver implements IDataDriver {
  */
  private static readonly DIALECT_CONNECT_TIMEOUT: Record<string, { key: string; urlKey: string }> =
  Object.fromEntries<{ key: string; urlKey: string }>([
- // Every spelling that means Postgres SQL, EXTENDED by `cockroachdb`:
- // a separate knex dialect, but one that reaches the server through the
- // same `pg` driver and therefore takes the same knob. Deriving the pg
- // arm from {@link POSTGRES_EMIT_CLIENTS} is what keeps this table from
- // drifting away from the getters again (#11550) — widening recognition
- // now widens this with it. `redshift` still carries no entry; see the
- // note in {@link withConnectBound} for why that absence is load-bearing.
- ...[...SqlDriver.POSTGRES_EMIT_CLIENTS, 'cockroachdb'].map(
+ // Every spelling that means Postgres SQL, EXTENDED by `cockroachdb` and
+ // `redshift`: separate knex dialects, but ones that reach the server
+ // through the same `pg` driver and therefore take the same knob. Knex's
+ // `Client_Redshift` literally `extends Client_PG`, so the settings object
+ // it hands to `pg.Client` honours `connectionTimeoutMillis` exactly as
+ // `pg` does. Deriving the pg arm from {@link POSTGRES_EMIT_CLIENTS} is
+ // what keeps this table from drifting away from the getters again
+ // (#11550) — widening recognition now widens this with it.
+ //
+ // `redshift`'s row arrived with #11784. It had the knob and would have
+ // obeyed it, but carried no entry, so its connection attempt fell through
+ // to the strictly looser 15s `pool.createTimeoutMillis` backstop while
+ // the docblock above called 10s "the effective bound". Nothing errored
+ // and nothing was logged — the bound was simply 50% looser, for one
+ // client name.
+ //
+ // ⚠️ This arm and {@link POSTGRES_WIRE_CLIENTS} now happen to hold the
+ // same five names. That is a fact about today's membership, NOT an
+ // invariant, and must not be refactored into one: the two answer
+ // different questions (how do I spell the connect timeout vs. which npm
+ // package parses the wire), and unioning them would hand `redshift` and
+ // `cockroachdb` SQL-emission identity as a silent side effect — the open
+ // decision #11756, and #11550's subject, not this table's to make.
+ ...[...SqlDriver.POSTGRES_EMIT_CLIENTS, 'cockroachdb', 'redshift'].map(
  (c): [string, { key: string; urlKey: string }] =>
  [c, { key: 'connectionTimeoutMillis', urlKey: 'connectionString' }],
  ),
@@ -4606,10 +4622,24 @@ export class SqlDriver implements IDataDriver {
  // `!dialect` — sqlite, or a client with no connect-timeout knob — means
  // there is no TIMEOUT to inject. It deliberately does not skip the session
  // pins below: those answer a different question (what does a value MEAN on
- // this connection), and the two lists are not the same list. Measured while
- // fixing #11389: `redshift` speaks the pg wire protocol, and therefore
- // needs the calendar-day pin, but carries no entry here — so a `return`
- // placed at this point silently opted it out of a fix it needs.
+ // this connection), and the two lists are not the same list.
+ //
+ // ⚠️ The worked example that used to stand here was RETIRED by #11784,
+ // which gave `redshift` the connect-timeout row it was missing. Until then
+ // `redshift` was in {@link POSTGRES_WIRE_CLIENTS} (so it needed #11389's
+ // calendar-day pin) yet absent from {@link DIALECT_CONNECT_TIMEOUT} — so a
+ // `return` placed at this point silently opted it out of a fix it needed.
+ // That was measured, not hypothetical, which is why the example is recorded
+ // here rather than dropped.
+ //
+ // The hazard outlives the example. Every client needing a session pin
+ // happens to have a timeout row TODAY; that is current membership, not a
+ // property either table promises. The next pg-wire client added without a
+ // connect-timeout knob restores the exact bug — and it would fail the way
+ // this one did, in silence, with every test green. So: no `return` here,
+ // and the two tables stay separate. The tripwire that goes red the moment
+ // those memberships diverge again lives in
+ // `sql-driver-11389-date-tz-skew.test.ts`.
  const dialect = SqlDriver.DIALECT_CONNECT_TIMEOUT[String(knexConfig.client ?? '')];
  if (dialect) {
  const conn = knexConfig.connection;