fix(driver-memory): refuse a cross-scope upsert by explicit id instead of landing a duplicate primary id

Patch round on the contract review of PR #16733, per the director seat's
ruling comment 5579651209.

F2 (code): `upsert`'s `data.id` arm scoped its conflict lookup to `visible`,
so an id addressed across the tenant wall missed and fell through to
`create`. `create` checks only DECLARED unique constraints and `id` is not
one, so the table ended up holding two rows with one primary id, which then
corrupts every id-addressed door for both tenants. It now refuses on this
driver's own existing "not found" contract. `driver-sql` is not the
precedent for falling through: it merges on the PRIMARY KEY regardless of
tenant and scopes only the readback, so the duplicate is unreachable there.
The code comment claiming otherwise is corrected.

F5 (pins): the F2 case, with merge-inside-scope and insert-of-a-fresh-id as
positive controls; and `deleteMany` under scope WITH a `where`, which takes
the matched-id rebuild path rather than the delete-all path.

F3 (changeset carriers): the `**BREAKING**` banner and the ADR-0087
`not-required (no-migration-prescription)` disposition, following the same
package's released precedent PR #7924. Level stays `minor`.

F4 (wording): the chokepoint claim is "every door that selects rows", with
`create`, `bulkCreate`, `syncSchema` and `dropTable` named as the exclusions.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TezFG8ZMrNH6n5VTNpPpdH

Author: claude

.changeset/memory-driver-read-side-tenant-scope.md

@@ -2,6 +2,8 @@
 '@objectstack/driver-memory': minor
 ---
 
+**BREAKING** — a caller that passes `DriverOptions.tenantId` to `@objectstack/driver-memory` now receives FEWER rows. The narrowed accept set was documented behaviour, not merely a defect: this package's own published docblock told that caller the driver never reads `DriverOptions.tenantId`. Same package, same subject and the same declaration as the released precedent #6915 / PR #7924 (`feat(driver-memory)!`), which shipped a refusal that was always owed and still declared it, so the release notes could say so. The bump level stays `minor` because the launch-window convention forbids `major`; during that window this banner, not the level, is the carrier.
+
 The in-memory driver now honours `DriverOptions.tenantId` / `tenantIds` instead of discarding them, so a scoped read no longer returns other organizations' rows.
 
 Two predicates decided "is this object tenant-scoped" and they disagreed on the default case. `Engine.buildDriverOptions` scopes unless the object opts OUT (`tenantId !== undefined && !isTenancyDisabled(schema) && !isFederated`); this driver's boot guard refuses only an explicit opt-IN (`tenancy.enabled === true`). An object that omits the `tenancy` block — the common case — was therefore scoped by the engine and invisible to the guard, and the driver did nothing with the scope: `tenantId`, `tenantIds` and `organization_id` occurred nowhere in `memory-driver.ts`. The read path knew nothing about tenants; the unique-constraint path did.
@@ -10,8 +12,12 @@ Measured on one app across two drivers, same build, same seed, same account: the
 
 ⚠️ **Every isolation measurement previously taken on this driver is void and must be re-taken.** The failure direction was toward exposure in the place where isolation is tested: a suite asserting "tenant A cannot see tenant B's rows" passed here not because isolation worked, but because both tenants' rows came back to everyone and the assertion had been written against a single tenant's fixture.
 
-The semantics are `driver-sql`'s, read off `applyTenantScope` and reproduced arm for arm rather than invented — `col = :tenantId OR col IS NULL` for the equality path, `col IN (…) OR col IS NULL` under the ADR-0105 D2 union posture, and the NULL arm keeps the #2734 global-row carve-out so a platform row that belongs to no organization stays visible to all of them. Every door that accepts a `DriverOptions` routes through one chokepoint: `find`, `findOne`, `count`, `aggregate` (both arms), `update`, `upsert`, `delete`, `updateMany`, `deleteMany`, `bulkUpdate` and `bulkDelete`. `distinct()` accepts no `DriverOptions` at all and is therefore still unscoped — the one door named rather than left to be discovered.
+The semantics are `driver-sql`'s, read off `applyTenantScope` and reproduced arm for arm rather than invented — `col = :tenantId OR col IS NULL` for the equality path, `col IN (…) OR col IS NULL` under the ADR-0105 D2 union posture, and the NULL arm keeps the #2734 global-row carve-out so a platform row that belongs to no organization stays visible to all of them. Every door that **selects rows** routes through one chokepoint: `find`, `findOne`, `count`, `aggregate` (both arms), `update`, `upsert`, `delete`, `updateMany`, `deleteMany`, `bulkUpdate` and `bulkDelete`. Four doors that take a `DriverOptions` are deliberately **not** routed through it: `create` and `bulkCreate` are the insert doors, which `driver-sql` scopes through `injectTenantOnInsert` rather than `applyTenantScope` and which the write half left out of this change does not stamp; `syncSchema` and `dropTable` are DDL, which `driver-sql` does not scope either. `distinct()` accepts no `DriverOptions` at all and is therefore still unscoped — the one selecting door named rather than left to be discovered.
+
+An `upsert` addressed by an explicit `id` refuses when a row carrying that id exists outside the caller's scope, on this driver's own "not found" contract, rather than inserting: `id` is the primary id here and `create` checks only declared unique constraints, so falling through would leave two rows carrying one id. `driver-sql` cannot reach that state — it merges on the PRIMARY KEY regardless of tenant and scopes only the readback.
 
 **What changes for an existing consumer.** A caller that passes no `tenantId` — every seed script, admin path and legacy call — is unaffected down to the array it allocates. A caller that does pass one, on an object carrying a tenant column and no `tenancy` declaration, now sees its own organization's rows plus organization-less rows, where it previously saw everything. That is the fix, and it is the reason a `single`-posture deployment is affected at all: `single` constrains the wall, not the number of organizations — one measured run held 13 `sys_organization` rows.
 
 Write-side tenancy is deliberately not included: nothing stamps a tenant column on insert the way `SqlDriver.injectTenantOnInsert` does, so the boot guard still refuses a walled posture and still refuses an object declaring `tenancy.enabled: true`. `declaresTenantScope`'s docstring is corrected in the same change — its load-bearing sentence, "every object in a single-tenant deployment omits the block", was false.
+
+<!-- adr-0087: not-required (no-migration-prescription) Nothing is retired or renamed: no authorable key, no option and no exported symbol is removed, so the ledger has no upgrade path to serve. The affected consumer is reached by this changeset body and by the driver's corrected refusal message, which is the same disposition the same package registered for the same subject in #6915 / PR #7924. -->

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

@@ -774,14 +774,42 @@ export class InMemoryDriver implements IDataDriver {
     let existingRecord: any = null;
 
     // [#16589] The conflict lookup is a read: a row belonging to another
-    // organization is not a conflict for this caller, so the upsert lands as an
-    // INSERT rather than silently rewriting a row it was never allowed to see.
-    // Matches `driver-sql`, which scopes its own upsert door.
+    // organization is not a conflict for this caller, so an upsert keyed on
+    // `conflictKeys` lands as an INSERT rather than silently rewriting a row it
+    // was never allowed to see.
+    //
+    // ⛔ `driver-sql` is NOT the precedent for that fall-through on the `id`
+    // arm. Its `INSERT … ON CONFLICT(id)` merges on the PRIMARY KEY regardless
+    // of tenant — "the verdict itself is tenant-independent regardless: `id` is
+    // the PRIMARY KEY, so at most one row in the table can carry it" — and only
+    // the READBACK is scoped. This store has no such key, so the `id` arm is
+    // handled separately below.
     const scope = this.tenantScope(object, options);
     const visible = scope ? table.filter(scope) : table;
 
     if (data.id) {
         existingRecord = visible.find(r => r.id === data.id);
+        // [#16589] The scope is the only thing that can have hidden the row: a
+        // row carrying `data.id` may sit in `table` and outside `visible`.
+        // Falling through to `create` there lands a SECOND row with the same
+        // primary id — `create` checks only DECLARED unique constraints and
+        // `id` is not one (pinned by `memory-bulk-create-atomicity.test.ts`) —
+        // and a duplicate primary id then corrupts every id-addressed door for
+        // BOTH tenants, since `update`/`delete` take the first matching index
+        // and `deleteMany` rebuilds the table from a matched-id set. Refuse on
+        // this driver's OWN existing "not found" contract instead, the same
+        // shape `update` and `delete` land on for a cross-tenant id.
+        //
+        // The refusal is raised in `strictMode` and outside it alike: unlike
+        // `update` (`| null`) and `delete` (`false`), `upsert`'s declared
+        // return carries no miss arm (#13878), so the quiet non-`strictMode`
+        // miss is not expressible here. The two alternatives were widening this
+        // door's declared return with an arm no caller was ever asked to
+        // narrow, or landing the duplicate id; both are worse than throwing.
+        if (!existingRecord && table.some(r => r.id === data.id)) {
+            this.logger.warn('Record not found for upsert', { object, id: data.id });
+            throw new Error(`Record with ID ${data.id} not found in ${object}`);
+        }
     } else if (conflictKeys && conflictKeys.length > 0) {
         existingRecord = visible.find(r => conflictKeys.every(key => r[key] === data[key]));
     }

packages/drivers/driver-memory/src/memory-tenant-scope.test.ts

@@ -341,6 +341,27 @@ describe('#16589 — the in-memory driver honours DriverOptions.tenantId', () =>
       expect(ids(await driver.find(object, {}))).toEqual(['b1']);
     });
 
+    it('deleteMany WITH a `where` deletes only this organization\'s matches', async () => {
+      const driver = makeDriver();
+      const object = await seed(driver);
+
+      // A different code path from the delete-all arm above, and the one worth
+      // pinning separately: it filters `visible`, collects `matchedIds`, then
+      // rebuilds the whole table from that set — so an id that crossed the wall
+      // would take the other organization's row with it. `contains 'one'`
+      // matches exactly one row in A (`a1`) and one in B (`b1`), which makes
+      // "scoped" and "unscoped" two different NUMBERS, not just two row lists.
+      expect(
+        await driver.deleteMany(
+          object,
+          { where: { type: 'comparison', field: 'name', operator: 'contains', value: 'one' } },
+          { tenantId: ORG_A },
+        ),
+      ).toBe(1);
+      // `b1` matched the filter and is the row that must survive it.
+      expect(ids(await driver.find(object, {}))).toEqual(['a2', 'b1', 'g1']);
+    });
+
     it('bulkUpdate and bulkDelete skip ids belonging to another organization', async () => {
       const driver = makeDriver();
       const object = await seed(driver);
@@ -369,6 +390,45 @@ describe('#16589 — the in-memory driver honours DriverOptions.tenantId', () =>
       expect(b1).toMatchObject({ organization_id: ORG_B });
     });
 
+    it('upsert by an id that exists OUTSIDE the scope refuses — never a second row with one primary id', async () => {
+      const driver = makeDriver();
+      const object = await seed(driver);
+
+      // The `conflictKeys` arm above may insert; the `id` arm may NOT. `id` is
+      // this store's primary id, and falling through to `create` here landed a
+      // SECOND row carrying `b1` — `create` checks only DECLARED unique
+      // constraints and `id` is not one. A duplicate primary id then corrupts
+      // every id-addressed door for BOTH tenants. ⛔ `driver-sql` is not the
+      // precedent for falling through: it merges on the PRIMARY KEY regardless
+      // of tenant and scopes only the readback, so the duplicate is unreachable
+      // there.
+      await expect(
+        driver.upsert(object, { id: 'b1', name: 'hijacked' }, undefined, { tenantId: ORG_A }),
+      ).rejects.toThrow(/Record with ID b1 not found/);
+
+      const after = await driver.find(object, {});
+      expect(ids(after)).toEqual(['a1', 'a2', 'b1', 'g1']);
+      // The assertion the whole finding is about — one row, not two.
+      expect(after.filter((r) => r.id === 'b1')).toHaveLength(1);
+      expect(after.find((r) => r.id === 'b1')).toMatchObject({ name: 'B one', organization_id: ORG_B });
+
+      // Two positive controls, so this reads as "the cross-wall id is refused"
+      // rather than "upsert by id is broken": inside the organization the same
+      // door still MERGES, and an id no row in the table carries still INSERTS.
+      expect(
+        await driver.upsert(object, { id: 'a1', name: 'renamed' }, undefined, { tenantId: ORG_A }),
+      ).toMatchObject({ id: 'a1', name: 'renamed' });
+      expect(
+        await driver.upsert(
+          object,
+          { id: 'a3', name: 'A three', organization_id: ORG_A },
+          undefined,
+          { tenantId: ORG_A },
+        ),
+      ).toMatchObject({ id: 'a3' });
+      expect(ids(await driver.find(object, {}))).toEqual(['a1', 'a2', 'a3', 'b1', 'g1']);
+    });
+
     it('distinct() is NOT scoped — the one door with no DriverOptions to scope by', async () => {
       const driver = makeDriver();
       const object = await seed(driver);