fix(service-datasource): reject a pool block the memory arm cannot honour (#5931)

.changeset/datasource-memory-pool-loud-reject.md

@@ -0,0 +1,49 @@
+---
+"@objectstack/service-datasource": patch
+---
+
+fix(service-datasource): a `pool` block on a `memory` datasource is rejected, not dropped in silence (#5931)
+
+#5714 made a `pool` block the driver cannot honour a loud authoring error, but
+its ruling was scoped to the two sqlite arms — `memory` kept dropping it. The
+`memory` arm hands `InMemoryDriver` nothing but `buildMemoryConfig(spec)`, which
+reads `spec.config` and never `spec.pool`, so a sized pool reached nothing and
+said nothing. Measured through the real factory:
+
+```text
+memory   + pool{min:3,max:9}   driver config {"persistence":false}   pool undefined
+sqlite   + pool{min:3,max:9}   rejected (since #5714)
+postgres + pool{min:3,max:9}   knex config.pool {"min":3,"max":9}    live {min:3,max:9}
+```
+
+`memory` now joins `POOL_UNSUPPORTED_DRIVER_IDS`, so the same three doors that
+already rejected sqlite reject it: the Setup wizard's create/update, the
+boot-time auto-connect pre-pass, and the driver factory itself.
+
+**Behaviour change.** A datasource declaring `driver: 'memory'` (or `inmemory` /
+`in-memory` / `mingo`) together with a non-empty `pool` block used to load and
+run; it now throws at whichever door it arrives through. The fix is the one edit
+the message names — delete the `pool` block. Nothing is lost by deleting it: it
+configured nothing before. An absent or empty `pool` is unchanged, and every
+`memory` datasource without one builds exactly as it did. No declaration in this
+repo, the example apps included, carried the combination.
+
+**Its own explanation, not SQLite's.** SQLite is rejected because a second
+connection to `:memory:` opens a separate, empty database, so sizing the pool
+would split one datasource across several stores. That reasoning is false for
+`memory`: there is no connection at all — the store is a plain data structure in
+this process — so the message says that instead. Telling an author their driver
+picked a connection strategy for them would send them looking for a knob that
+does not exist. Reasons are now keyed by driver id, which makes an arm joining
+the set without writing one a type error.
+
+Maintainer ruling 2026-08-07, which also set the default for the next sister
+arm: when a declared key is silently dropped on one arm and an earlier ruling
+already made it a loud authoring error on a sibling, the new arm joins the
+existing rejection set rather than queueing for a ruling of its own — unless the
+original rationale was measured to be arm-specific.
+
+No API surface is added — `POOL_UNSUPPORTED_DRIVER_IDS`,
+`driverReadsDeclaredPool`, `unsupportedPoolIssue`, `unsupportedPoolMessage` and
+`assertDatasourcePoolSupported` keep the signatures #5714 published, and the
+sqlite arms' rejection text is byte-for-byte unchanged.

packages/services/service-datasource/src/__tests__/datasource-pool-support.test.ts

@@ -11,12 +11,25 @@
 // takes effect or is rejected out loud — never dropped. These tests pin the
 // rejection at each door it can come in through, and pin that the arms which
 // DO honour the block still do.
+//
+// #5931 — the SISTER ARM. `memory` dropped `pool` exactly as silently
+// (`buildMemoryConfig` reads `spec.config` only, and `InMemoryDriver` has no
+// pool concept whatsoever), but it was left out of the rejected set because
+// #5714's ruling was scoped to the two sqlite arms. Measured the same way:
+//
+//   memory + pool{min:3,max:9}   driver config {"persistence":false}   pool undefined
+//
+// The maintainer ruling of 2026-08-07 folds it in, with its own explanation
+// rather than SQLite's, and sets the default for the next sister arm (#6140).
+// The pin that used to hold `memory` OUT of the set is flipped below, not
+// deleted — it is the same fact, re-judged.
 
 import { describe, it, expect, vi } from 'vitest';
 import {
   POOL_UNSUPPORTED_DRIVER_IDS,
   driverReadsDeclaredPool,
   unsupportedPoolIssue,
+  unsupportedPoolMessage,
   assertDatasourcePoolSupported,
 } from '../datasource-pool-support.js';
 import { createDefaultDatasourceDriverFactory } from '../default-datasource-driver-factory.js';
@@ -29,8 +42,8 @@ import { DatasourceAdminService, type StoredDatasource } from '../datasource-adm
 import type { IDatasourceDriverFactory } from '../contracts/datasource-driver-factory.js';
 
 describe('#5714 — which driver arms read a declared `pool`', () => {
-  it('names exactly the two sqlite arms as unable to honour it', () => {
-    expect([...POOL_UNSUPPORTED_DRIVER_IDS]).toEqual(['sqlite', 'sqlite-wasm']);
+  it('names the two sqlite arms and `memory` as unable to honour it (#5931)', () => {
+    expect([...POOL_UNSUPPORTED_DRIVER_IDS]).toEqual(['memory', 'sqlite', 'sqlite-wasm']);
   });
 
   it('rejects every spelling of the sqlite arms, case-insensitively', () => {
@@ -53,17 +66,24 @@ describe('#5714 — which driver arms read a declared `pool`', () => {
     expect(unsupportedPoolIssue({ driver: 'com.vendor.snowflake', pool: { min: 3, max: 9 } })).toBeUndefined();
   });
 
-  // Deliberate, and filed rather than silently widened: `memory` reads no pool
-  // either, but the #5714 ruling authorised this tightening for the sqlite arms
-  // only. #5931 carries the decision.
-  it('leaves `memory` out of the rejected set (#5931), deliberately', () => {
-    expect(driverReadsDeclaredPool('memory')).toBe(true);
+  // THE FLIPPED PIN (#5931). This case used to assert the opposite — `memory`
+  // deliberately OUT of the rejected set, pending a ruling — and #5954 left it
+  // green on purpose so the hole would read as drawn rather than overlooked.
+  // The ruling of 2026-08-07 folded the arm in, so the same fact now has the
+  // opposite verdict. Every spelling, because `mingo` and `in-memory` build the
+  // very same driver.
+  it('rejects `memory` and every spelling of it (#5931 — the flipped pin)', () => {
+    for (const id of ['memory', 'inmemory', 'in-memory', 'mingo', 'Memory', '  MEMORY  ']) {
+      expect(driverReadsDeclaredPool(id), id).toBe(false);
+    }
   });
 
   it('treats an absent or empty block as no declaration', () => {
-    expect(unsupportedPoolIssue({ driver: 'sqlite' })).toBeUndefined();
-    expect(unsupportedPoolIssue({ driver: 'sqlite', pool: {} })).toBeUndefined();
-    expect(unsupportedPoolIssue({ driver: 'sqlite', pool: undefined })).toBeUndefined();
+    for (const driver of ['sqlite', 'memory']) {
+      expect(unsupportedPoolIssue({ driver }), driver).toBeUndefined();
+      expect(unsupportedPoolIssue({ driver, pool: {} }), driver).toBeUndefined();
+      expect(unsupportedPoolIssue({ driver, pool: undefined }), driver).toBeUndefined();
+    }
   });
 
   it('names the datasource and the one edit that fixes it', () => {
@@ -77,14 +97,73 @@ describe('#5714 — which driver arms read a declared `pool`', () => {
   // authoring mistake has a correction; suggesting an env var that boots past
   // it (or a different driver) sends the author away from the fix.
   it('offers no escape hatch and no "use another driver" advice', () => {
-    const msg = unsupportedPoolIssue({ driver: 'sqlite-wasm', pool: { max: 9 }, name: 'ds' }) ?? '';
-    expect(msg).not.toMatch(/OS_ALLOW_DRIVER_CONNECT_FAILURE/);
-    expect(msg).not.toMatch(/OS_[A-Z_]*=1/);
-    expect(msg).not.toMatch(/switch|instead use|change the driver/i);
+    for (const driver of ['sqlite-wasm', 'memory']) {
+      const msg = unsupportedPoolIssue({ driver, pool: { max: 9 }, name: 'ds' }) ?? '';
+      expect(msg, driver).not.toMatch(/OS_ALLOW_DRIVER_CONNECT_FAILURE/);
+      expect(msg, driver).not.toMatch(/OS_[A-Z_]*=1/);
+      expect(msg, driver).not.toMatch(/switch|instead use|change the driver/i);
+    }
+  });
+
+  // #5931 — the memory arm gets its OWN explanation. The ruling was explicit
+  // that SQLite's sentence must not be reused, and the reason is not style:
+  // "the driver owns the connection strategy" would send the author looking for
+  // a strategy knob, when the truth is that there is no connection to pool.
+  it('explains `memory` in its own terms, never SQLite\'s (#5931)', () => {
+    const msg = unsupportedPoolIssue({ driver: 'memory', pool: { min: 3, max: 9 }, name: 'scratch' }) ?? '';
+    expect(msg).toContain(`Datasource 'scratch'`);
+    expect(msg).toContain(`the 'memory' driver does not read it`);
+    expect(msg).toMatch(/no pool to size, and no connection to pool/);
+    expect(msg).toMatch(/plain data structure inside this process/);
+    // …and not one part of SQLite's reasoning, which is about a DIFFERENT
+    // failure (one datasource split across several stores).
+    expect(msg).not.toMatch(/SQLite/);
+    expect(msg).not.toMatch(/connection strategy is owned by the driver/);
+    expect(msg).not.toMatch(/SEPARATE, empty database/);
+    expect(msg).not.toMatch(/split one datasource's data/);
+    // The shared frame survives: the one edit that fixes it, and where the key
+    // stays meaningful.
+    expect(msg).toMatch(/Remove `pool` from this datasource declaration/);
+    expect(msg).toMatch(/postgres \/ mysql \/ mongo/);
+  });
+
+  it('quotes the spelling the author actually wrote, with the memory reason behind it', () => {
+    const msg = unsupportedPoolIssue({ driver: 'mingo', pool: { max: 2 }, name: 'scratch' }) ?? '';
+    expect(msg).toContain(`the 'mingo' driver does not read it`);
+    expect(msg).toMatch(/no pool to size, and no connection to pool/);
+  });
+
+  // The two sqlite arms' text is UNCHANGED by #5931 — pinned whole, against the
+  // literal as it stood on `origin/main` before this change, because "we only
+  // added an arm" is a claim about bytes.
+  it('leaves the sqlite arms\' message byte-for-byte as #5714 wrote it', () => {
+    const expected =
+      "Datasource 'crm_primary' declares a `pool` block, but the 'sqlite' driver does not read " +
+      'it: a SQLite connection strategy is owned by the driver, not by the datasource — one ' +
+      'connection per database, because a second connection to `:memory:` opens a SEPARATE, ' +
+      "empty database. Sizing it here would therefore split one datasource's data across " +
+      'several stores, so the block is rejected instead of dropped. Remove `pool` from this ' +
+      'datasource declaration; it stays meaningful on the pooled drivers ' +
+      '(postgres / mysql / mongo).';
+    expect(unsupportedPoolMessage('sqlite', 'crm_primary')).toBe(expected);
+    expect(unsupportedPoolMessage('sqlite-wasm', 'crm_primary'))
+      .toBe(expected.replace("the 'sqlite' driver", "the 'sqlite-wasm' driver"));
+  });
+
+  // Unreachable through `unsupportedPoolIssue` (nothing produces a message for a
+  // driver that reads the block), but the helper is exported and takes a plain
+  // string. The honest answer there is the part true of every rejected arm —
+  // never another arm's specific reasoning.
+  it('borrows no arm\'s reasoning for a driver that is not in the set', () => {
+    const msg = unsupportedPoolMessage('postgres');
+    expect(msg).toMatch(/nothing in the block reaches a connection/);
+    expect(msg).not.toMatch(/SQLite/);
+    expect(msg).not.toMatch(/plain data structure inside this process/);
   });
 
   it('assert throws exactly when the issue is reported', () => {
     expect(() => assertDatasourcePoolSupported({ driver: 'sqlite', pool: { max: 5 } })).toThrow(/does not read it/);
+    expect(() => assertDatasourcePoolSupported({ driver: 'memory', pool: { max: 5 } })).toThrow(/does not read it/);
     expect(() => assertDatasourcePoolSupported({ driver: 'postgres', pool: { max: 5 } })).not.toThrow();
   });
 });
@@ -119,6 +198,43 @@ describe('#5714 — the driver factory rejects a pool it cannot honour', () => {
     ).rejects.toThrow(/does not read it/);
   });
 
+  // #5931 — the arm this file used to pin as deliberately unguarded. Before the
+  // ruling this call RESOLVED, handing back an `InMemoryDriver` built from
+  // `buildMemoryConfig(spec)` alone, with `pool` nowhere in it.
+  it('memory + pool is rejected instead of built with the block dropped (#5931)', async () => {
+    await expect(
+      factory().create({
+        name: 'scratch',
+        driver: 'memory',
+        config: {},
+        pool: { min: 3, max: 9 },
+      }),
+    ).rejects.toThrow(/Datasource 'scratch' declares a `pool` block/);
+  });
+
+  it('rejects the `inmemory` alias too, and says why in memory terms', async () => {
+    const err = await Promise.resolve(
+      factory().create({ name: 'scratch', driver: 'inmemory', config: {}, pool: { max: 9 } }),
+    ).then(() => undefined, (e: Error) => e);
+    expect(err?.message).toMatch(/no pool to size, and no connection to pool/);
+    expect(err?.message).not.toMatch(/SQLite/);
+  });
+
+  it('memory WITHOUT a pool still builds exactly as before', async () => {
+    const handle: any = await factory().create({ name: 'scratch', driver: 'memory', config: {} });
+    const driver = handle.driver ?? handle;
+    expect(driver?.constructor?.name).toMatch(/InMemoryDriver$/);
+    // The #4083 shape is untouched: ephemeral unless the author opts in.
+    expect(driver.config?.persistence).toBe(false);
+    try { await handle.disconnect?.(); } catch { /* never connected */ }
+  });
+
+  it('memory with an EMPTY pool block still builds — nothing was declared', async () => {
+    const handle: any = await factory().create({ name: 'scratch', driver: 'memory', config: {}, pool: {} });
+    expect((handle.driver ?? handle)?.constructor?.name).toMatch(/InMemoryDriver$/);
+    try { await handle.disconnect?.(); } catch { /* never connected */ }
+  });
+
   it('sqlite WITHOUT a pool still builds exactly as before', async () => {
     const handle: any = await factory().create({
       name: 'crm_primary',
@@ -240,6 +356,46 @@ describe('#5714 — boot refuses a declared pool the driver cannot honour', () =
     ).resolves.toEqual([]);
   });
 
+  // #5931 — the same door, the sister arm. A memory datasource carrying a pool
+  // used to boot silently and run on a store no pool setting ever touched.
+  it('refuses a memory datasource carrying a pool, before anything is connected (#5931)', async () => {
+    const { service, factory, engine } = svc();
+    await expect(
+      service.connectDeclared({
+        datasources: [{ name: 'scratch', driver: 'memory', config: {}, pool: { min: 3, max: 9 } }],
+        objects: [],
+      }),
+    ).rejects.toThrow(/Datasource 'scratch' declares a `pool` block/);
+    expect((factory.create as any).mock.calls.length).toBe(0);
+    expect(engine.drivers.size).toBe(0);
+  });
+
+  it('names a memory offender alongside a sqlite one in the same throw', async () => {
+    const { service } = svc();
+    const err = await service
+      .connectDeclared({
+        datasources: [sqliteWithPool, { name: 'scratch', driver: 'memory', pool: { max: 4 } }],
+        objects: [],
+      })
+      .then(() => undefined, (e: Error) => e);
+    expect(err?.message).toMatch(/2 declared datasource\(s\)/);
+    expect(err?.message).toContain(`Datasource 'crm_primary'`);
+    expect(err?.message).toContain(`Datasource 'scratch'`);
+    // Each offender keeps its own explanation in the aggregate.
+    expect(err?.message).toMatch(/a SQLite connection strategy is owned by the driver/);
+    expect(err?.message).toMatch(/no pool to size, and no connection to pool/);
+  });
+
+  it('leaves a memory datasource with no pool block connecting as before', async () => {
+    const { service, engine } = svc();
+    const results = await service.connectDeclared({
+      datasources: [{ name: 'scratch', driver: 'memory', config: {}, autoConnect: true }],
+      objects: [],
+    });
+    expect(results.map((r) => r.status)).toEqual(['connected']);
+    expect(engine.drivers.has('scratch')).toBe(true);
+  });
+
   it('leaves a sqlite datasource with no pool block connecting as before', async () => {
     const { service, engine } = svc();
     const results = await service.connectDeclared({
@@ -328,6 +484,39 @@ describe('#5714 — the Setup wizard rejects it before the record is stored', ()
     expect(records[0]?.pool).toEqual({ min: 3, max: 9 });
   });
 
+  // #5931 — the wizard is the door an author is most likely to come through
+  // with `driver: memory`, since it is the dev/test choice the Setup UI offers.
+  it('create: a memory draft carrying a pool never reaches the store (#5931)', async () => {
+    const { service, records, registered } = adminHarness();
+    await expect(
+      service.createDatasource({
+        name: 'scratch',
+        driver: 'memory',
+        config: {},
+        pool: { min: 1, max: 5 },
+      }),
+    ).rejects.toThrow(/no pool to size, and no connection to pool/);
+    expect(records).toHaveLength(0);
+    expect(registered).toHaveLength(0);
+  });
+
+  it('create: a memory draft with no pool is stored as before', async () => {
+    const { service, records, registered } = adminHarness();
+    await service.createDatasource({ name: 'scratch', driver: 'memory', config: {} });
+    expect(records[0]?.name).toBe('scratch');
+    expect(records[0]?.pool).toBeUndefined();
+    expect(registered).toEqual(['scratch']);
+  });
+
+  it('update: switching a pooled datasource TO memory is rejected on the merged record (#5931)', async () => {
+    const { service } = adminHarness([
+      { name: 'reporting', driver: 'postgres', config: {}, pool: { min: 3, max: 9 }, origin: 'runtime' },
+    ]);
+    await expect(
+      service.updateDatasource('reporting', { driver: 'memory', config: {} }),
+    ).rejects.toThrow(/declares a `pool` block/);
+  });
+
   it('update: patching a pool onto a stored sqlite datasource is rejected', async () => {
     const { service } = adminHarness([
       { name: 'local_cache', driver: 'sqlite', config: { filename: ':memory:' }, origin: 'runtime' },