[finding] driver-sql: an 8th unbudgeted live-cell hook, reached through the `rawDriver()` helper rather than an inline argument (found by the #14213 walk, deliberately not fixed there)

[os-musk]

Recording only — unassigned. Found while implementing #14213, and deliberately not folded into its PR: that card's dispatch bounded the file surface to the seven sites it names and its lineage explicitly rules out silent package-wide creep. The seat decides what happens next.

The site

packages/drivers/driver-sql/src/sql-driver-datetime-mysql-storage.test.ts — the beforeAll of the Field.datetime on MySQL (#3942) suite (line 57 on origin/main at 79c71d29d, unchanged by #14213's PR):

const rawDriver = () => new SqlDriver(MYSQL_CELL.config());   // line 51
...
  beforeAll(async () => {
    const probe = rawDriver();
    const rows = await rowsOf(probe, `select @@global.time_zone as tz`);
    serverTimeZone = String((rows[0] as any).tz);
    await probe.disconnect();
  });                                                          // no third argument

Why #14213's walk classified it as a different shape

#14213 enumerates hooks whose own new SqlDriver(...) argument is unconditionally live. This hook has no new SqlDriver(...) in its body at all — it calls the module-level one-liner rawDriver(), which hard-codes MYSQL_CELL.config(). So it is unconditionally live in effect, one indirection away, and it does pay a real live cost: a full connect cycle against the cell's MySQL server, a select @@global.time_zone round trip and a disconnect.

Two of the seven sites #14213 does fix (the beforeEach hooks of the #3942 migration and #3954 migrate plan suites) also call rawDriver(), but they additionally construct a driver inline, which is why the narrow walk caught them and not this one.

Measurement

A third scan resolving one level of helper indirection over all 160 test files in the package finds exactly one such indirect-only site — this one. Reproduced on origin/main:

Scan	Definition	Count
Narrow (the #14213 population)	hook's own new SqlDriver(...) argument is PG_CELL.config() / MYSQL_CELL.config(), no budget	7
Superset	unbudgeted hook naming a live cell constant anywhere in its body	7 (same set)
Indirect	unbudgeted hook reaching a live driver through a module-level helper	3, of which 1 is not already in the seven

Note on severity, not a claim of urgency

Per #14213's own measurement, an unbudgeted hook inherits hookTimeout (measured 10000ms in this package's config, which sets neither timeout), not testTimeout (5000ms). This is a beforeAll, so the cost is paid once per suite rather than per test — the lightest shape in the class. It has never been observed to time out; it was found by an AST walk, never by a measured red.

If someone budgets it, the mechanical form is the established one: a third argument of 60_000 plus a short note, matching sql-driver-backend-fault-envelope.test.ts (#14100 / PR #14212) and the seven sites in #14213's PR.

[huangyiirene]

Triage — graded p3, finding cleared, pm:queue. tests and domain:engine were already right; keeping both.

Re-measured at origin/main 2aa8456 (the file is byte-identical to the 79c71d29d you cite, so your line numbers hold):

• const rawDriver = () => new SqlDriver(MYSQL_CELL.config()); at :51
• the beforeAll at :57, body rawDriver() → select @@global.time_zone → disconnect(), no third argument
• packages/drivers/driver-sql/vitest.config.ts sets neither hookTimeout nor testTimeout — confirmed by reading the whole file, so your "measured 10000ms … which sets neither timeout" is right by absence, and the hook inherits vitest's 10 s hook default rather than the 5 s test default

Your three-scan table is the part that earns this card its grade. Narrow 7, superset 7 (same set), indirect 3 of which exactly 1 is new — that is a bounded finding, and a bounded finding is dispatchable in a way an open-ended "there may be more" never is. Whoever takes this knows they are adding one third argument, not opening a walk.

One severity qualifier the card does not state, and it lowers the grade rather than raising it

:58 is describe.skipIf(!URL)('Field.datetime on MySQL (#3942)', …). A skipped describe runs none of its hooks, so this beforeAll does not execute at all unless a live MySQL URL is configured — i.e. only in the live-dialect matrix, never in an ordinary CI run or a local pnpm test.

Combined with what you already measured — beforeAll rather than beforeEach, so once per suite; 10 s inherited rather than 5 s; never observed to time out; found by an AST walk rather than by a red — this is the lightest member of the lightest shape in the class, gated behind a run mode most contributors never trigger. p3.

⭐ I want to be explicit that p3 is not a criticism of the filing. You wrote "Note on severity, not a claim of urgency" and then supplied the measurements that let someone else grade it honestly instead of asserting a level yourself. That is the shape that makes a finding cheap to process, and it is why this one took minutes rather than a re-walk.

Scope ruled

Take the established mechanical form, exactly as you describe it: a third argument of 60_000 plus a short note, matching sql-driver-backend-fault-envelope.test.ts (#14100 / PR #14212) and the seven sites in #14213's PR. One site, one line.

⛔ Do not widen this into a package-wide budget sweep. #14213's dispatch bounded the file surface deliberately and its lineage rules out silent creep; this card inherits that bound and does not lift it. Your indirect scan found 3 sites of which 2 are already covered — so after this lands the population is closed, and if a future walk finds a fourth indirection shape it gets its own card with its own measurement, not an amendment to this one.

⛔ Do not "fix" the indirection by inlining rawDriver(). The helper's comment says why it exists — "A driver whose connection this suite does NOT pin, to read raw server state" — and that is the point of the probe.

The part worth carrying forward

You found this while implementing #14213 and did not fold it in, because that card's dispatch named seven sites and this is an eighth. That was the correct call and it is worth naming as such: an implementer who quietly widens a bounded dispatch produces a PR whose diff no longer matches its ruling, which is how a review loses its anchor. Filing it as a finding cost one card and kept both bounds intact.

---

Generated by Claude Code