test(runtime): pin the ambient-vs-bare transaction split behind the single-connection stall (#7842) (#8077)

* test(runtime): pin the ambient-vs-bare transaction split behind the single-connection stall (#7842)

Executes the A/B that MetadataManager's listCache policy comment asserts and
that PR #7840 measured but left pinned only by prose: a metadata read issued
under a transaction the engine's ambient txStore can see is threaded onto the
loader's read and returns, while one opened directly on the driver stalls out
connection acquisition.

The negative direction is observed with a shortened acquireConnectionTimeout on
the fixture's driver (400ms) instead of knex's 60s default -- same shape, same
knex message, 453ms instead of 60s.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V

* test(runtime): register sys_metadata through the engine proxy so the fixture adds no TEST_DEBT (#7842)

`engine.registry.registerObject` requires a `packageId` (2-5 args); the
engine's own proxy defaults it. Measured: packages/runtime's hidden test layer
stays at 227, the recorded ledger value, with 0 errors from the new file.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: hotlong

packages/runtime/src/metadata-list-ambient-vs-bare-transaction.integration.test.ts

@@ -0,0 +1,246 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #7842 — executes the ambient-vs-bare split that `MetadataManager`'s
+ * `listCache` policy comment asserts.
+ *
+ * PR #7840 (#7708) re-anchored that comment onto a fresh measurement instead of
+ * a retired witness, and the measurement NARROWED the hazard rather than
+ * retiring it: on a real `ObjectQL` + real `SqlDriver` (better-sqlite3,
+ * `:memory:`, knex pool max 1), a metadata read issued while a transaction is
+ * open stalls out `acquireConnectionTimeout` — but ONLY when the transaction was
+ * opened somewhere the engine's ambient `txStore` (ADR-0034) cannot see it. A
+ * transaction opened through `engine.transaction()` is published into that store,
+ * `buildDriverOptions` threads it onto the loader's read, and the read returns at
+ * once on the connection the transaction already holds.
+ *
+ * That split lived only in a code comment and in PR prose, so nothing noticed if
+ * `buildDriverOptions`' ambient fallback, `DatabaseLoader._find()`'s option
+ * forwarding, or the SQLite pool config changed underneath it — at which point
+ * the comment silently becomes the next stale witness, which is the defect #7708
+ * existed to repair. This file is the executable half. It PINS current behaviour
+ * and changes none of it.
+ *
+ * ## The 60-second problem, and what this fixture does about it
+ *
+ * The negative direction is an acquire timeout, and knex's default is 60s — a
+ * 60-second test in the shared suite is worse than the gap it closes. The
+ * fixture's driver therefore declares its own short `acquireConnectionTimeout`
+ * ({@link ACQUIRE_MS}); `SqlDriverConfig` is `Knex.Config &` extras and the
+ * constructor forwards everything it does not itself consume, so the knob
+ * arrives at knex untouched. That shortens the WAIT, not the SHAPE: the read
+ * still fails on connection acquisition, with knex's own message, exactly as it
+ * does in production at 60s. `assertEffectiveTimeout` re-reads the value off the
+ * live knex client so a driver that stopped forwarding it cannot leave this test
+ * quietly measuring the default.
+ *
+ * ## Why the positive case asserts three things and not one
+ *
+ * "The read completed" passes trivially if no transaction was ever open — the
+ * test would then guard nothing while appearing to guard the whole hazard. So
+ * inside the callback all three of PR #7840's own false-negative checks are
+ * asserted: the driver counts one open transaction, the ambient store carries a
+ * handle, and — the load-bearing one — the `sys_metadata` read is observed
+ * ARRIVING AT THE DRIVER with that handle in its options. Only the third
+ * distinguishes "the ambient store was threaded onto the loader's read" from
+ * "the read happened to succeed on its own".
+ *
+ * Worked precedent: `sql-driver-sqlite-tx-guard.test.ts`, which keeps the
+ * driver's own `parentTrx` contract honest the same way. This lives in
+ * `packages/runtime` because it is the only package carrying `objectql` +
+ * `driver-sql` + `metadata` in one dependency closure.
+ */
+
+import { describe, it, expect, afterEach, vi } from 'vitest';
+import { ObjectQL } from '@objectstack/objectql';
+import { SqlDriver } from '@objectstack/driver-sql';
+import { DatabaseLoader } from '@objectstack/metadata';
+import { SysMetadataObject } from '@objectstack/metadata-core';
+
+/**
+ * The fixture's acquire bound, replacing knex's 60s default.
+ *
+ * Long enough that a loaded CI box cannot mistake scheduling jitter for a
+ * timeout, short enough that the negative case costs well under a second.
+ */
+const ACQUIRE_MS = 400;
+
+/** Upper bound for the negative case — an order of magnitude under knex's 60s default. */
+const STALL_CEILING_MS = 20_000;
+
+/** The seeded row `list('object')` must come back with. */
+const SEEDED_NAME = 'thing';
+
+interface Fixture {
+  engine: ObjectQL;
+  driver: SqlDriver;
+  loader: DatabaseLoader;
+  /** Every `driver.find` the fixture has observed, and whether it carried a transaction. */
+  reads: () => Array<{ object: string; hasTx: boolean }>;
+}
+
+let fixture: Fixture | null = null;
+
+afterEach(async () => {
+  vi.restoreAllMocks();
+  try {
+    await fixture?.engine.destroy();
+  } catch {
+    /* teardown is best-effort — a stalled pool must not mask the assertion */
+  }
+  fixture = null;
+});
+
+async function boot(): Promise<Fixture> {
+  const driver = new SqlDriver({
+    client: 'better-sqlite3',
+    connection: { filename: ':memory:' },
+    useNullAsDefault: true,
+    // See the header: shortens the wait, not the shape.
+    acquireConnectionTimeout: ACQUIRE_MS,
+  });
+  await driver.initObjects([SysMetadataObject as any]);
+
+  // Seeded through the driver rather than the engine: `sys_metadata` is
+  // `managedBy: 'engine-owned'`, and this row is fixture state, not a write
+  // whose path is under test.
+  await driver.create('sys_metadata', {
+    id: 'md_fixture_1',
+    type: 'object',
+    name: SEEDED_NAME,
+    scope: 'platform',
+    metadata: JSON.stringify({ name: SEEDED_NAME }),
+  });
+
+  const engine = new ObjectQL();
+  engine.registerDriver(driver, true);
+  await engine.init();
+  // The engine's own proxy, not `engine.registry.registerObject` — the registry
+  // method requires a `packageId` and the proxy supplies one, so this keeps the
+  // fixture out of the package's TEST_DEBT ledger (`check:type-check-debt`).
+  engine.registerObject(SysMetadataObject as any);
+
+  // `cache: { enabled: false }` is what makes every `list()` below a real
+  // database read; with the loader's own read-through cache on, the second call
+  // would be answered from memory and would never reach the pool at all.
+  const loader = new DatabaseLoader({
+    engine: engine as any,
+    cache: { enabled: false },
+  });
+
+  // Flush `ensureSchema()` OUTSIDE any transaction. On the engine path it runs
+  // the `project_id` → `environment_id` forward migration on a bare connection
+  // and swallows its own failure, so leaving it to the measured call would put
+  // one extra acquire timeout inside the number the negative case reports.
+  // Doubles as the issue's `control` row: no transaction open, read returns.
+  const control = await loader.list('object');
+  expect(control).toEqual([SEEDED_NAME]);
+
+  const spy = vi.spyOn(driver, 'find');
+  fixture = {
+    engine,
+    driver,
+    loader,
+    reads: () =>
+      spy.mock.calls.map((call) => ({
+        object: call[0] as string,
+        hasTx: (call[2] as { transaction?: unknown } | undefined)?.transaction !== undefined,
+      })),
+  };
+  return fixture;
+}
+
+/**
+ * The two facts the hazard is made of, re-read off the LIVE knex client.
+ *
+ * `pool.max === 1` is why a second connection is unobtainable; the acquire bound
+ * is why this test costs 0.4s instead of 60s. Asserting both here means a change
+ * to the SQLite pool config, or a driver that stops forwarding the knob, fails
+ * with the reason named rather than as a mysterious green (nothing to stall) or
+ * a mysterious hang (back at the default).
+ */
+function assertEffectiveTimeout(driver: SqlDriver): void {
+  const client = (driver as any).knex.client;
+  expect(client.pool.max).toBe(1);
+  expect(client.config.acquireConnectionTimeout).toBe(ACQUIRE_MS);
+}
+
+describe('#7842 metadata list under an open transaction: ambient vs bare (real ObjectQL + real SqlDriver)', () => {
+  it('AMBIENT — a transaction opened via engine.transaction() is threaded onto the loader read, which completes', async () => {
+    const { engine, driver, loader, reads } = await boot();
+    assertEffectiveTimeout(driver);
+
+    const before = reads().length;
+    let names: string[] = [];
+
+    const started = Date.now();
+    await engine.transaction(async () => {
+      // ── False-negative guard 1: a transaction really is open on the driver.
+      // Without this, "the read completed" also passes when `engine.transaction`
+      // silently degraded to a no-transaction path and there was never a
+      // single-connection hazard to survive.
+      expect((driver as any).activeTransactions).toBe(1);
+
+      // ── False-negative guard 2: it is published into the engine's ambient
+      // store (ADR-0034), which is the only thing `buildDriverOptions` can find
+      // when the caller passes no explicit handle — as the loader does not.
+      const ambient = (engine as any).txStore.getStore();
+      expect(ambient?.transaction).toBeDefined();
+
+      names = await loader.list('object');
+    });
+    const elapsed = Date.now() - started;
+
+    // The read returned real rows — not an empty degradation that would also
+    // satisfy "it completed".
+    expect(names).toEqual([SEEDED_NAME]);
+
+    // ── False-negative guard 3, the load-bearing one. The other two prove a
+    // transaction existed; only this proves the ambient store was THREADED onto
+    // the loader's read. Everything else here stays green if
+    // `buildDriverOptions` stops consulting `txStore` and the read merely runs
+    // on some other connection.
+    const inTx = reads().slice(before).filter((r) => r.object === 'sys_metadata');
+    expect(inTx.length).toBeGreaterThan(0);
+    expect(inTx.every((r) => r.hasTx)).toBe(true);
+
+    // And it did not stall on the way: the whole ambient case finishes inside
+    // the acquire bound the bare case exhausts.
+    expect(elapsed).toBeLessThan(STALL_CEILING_MS);
+  });
+
+  it('BARE — a transaction opened directly on the driver is invisible to the ambient store, and the loader read stalls out connection acquisition', async () => {
+    const { engine, driver, loader, reads } = await boot();
+    assertEffectiveTimeout(driver);
+
+    const before = reads().length;
+    const trx = await driver.beginTransaction();
+    try {
+      // Same first guard as above: the transaction is genuinely open and
+      // genuinely holding the pool's only connection.
+      expect((driver as any).activeTransactions).toBe(1);
+      // ...and nothing published it into the engine's ambient store, which is
+      // the entire difference between the two cases.
+      expect((engine as any).txStore.getStore()).toBeUndefined();
+
+      const started = Date.now();
+      await expect(loader.list('object')).rejects.toThrow(/Timeout acquiring a connection/i);
+      const elapsed = Date.now() - started;
+
+      // It failed by WAITING on the pool, not instantly for some other reason...
+      expect(elapsed).toBeGreaterThanOrEqual(ACQUIRE_MS / 2);
+      // ...and the fixture's bound, not knex's 60s default, is what ended the wait.
+      expect(elapsed).toBeLessThan(STALL_CEILING_MS);
+
+      // The negative direction's own false-negative guard: the read really was
+      // attempted and really did reach the driver — WITHOUT a transaction
+      // handle. A rejection alone cannot tell "nothing threaded the caller's
+      // transaction" apart from "the read never went out".
+      const bare = reads().slice(before).filter((r) => r.object === 'sys_metadata');
+      expect(bare.length).toBeGreaterThan(0);
+      expect(bare.every((r) => !r.hasTx)).toBe(true);
+    } finally {
+      await driver.rollback(trx);
+    }
+  });
+});