Skip to content

Commit 0f38ab0

Browse files
claude[bot]claude
andauthored
fix(driver-memory,driver-sql): an explicit tenancy opt-out survives a partial syncSchema re-registration (#16729) (#17221)
* fix(driver-memory,driver-sql): keep the explicit tenancy opt-out sticky across a partial re-registration `InMemoryDriver.syncSchema` recomputed its uniqueness constraints from whatever schema that call carried, so a second registration without a `tenancy` block fell through to the implicit `organization_id` heuristic and moved a `unique` field from one row per install to one row per organization. A duplicate the declaration refuses then landed, silently. `SqlDriver` has kept a sticky `tenantOptOutByTable` since #3249; this package had mirrored the inner `computeTenantField` and not the wrapper that consults the record. - driver-memory: publish `computeAndRecordTenantField` + `TenantOptOutRecord`; `InMemoryDriver` holds one record per instance and hands both declaration surfaces the same resolved column. `tenantFieldOf` is unchanged. - driver-sql: the shard leaf resolves through the record, keyed by the base table, instead of the bare `computeTenantField`. - objectql: `LifecycleObjectLike` declares `tenancy`, so the published type no longer refuses a key the Archiver's `cold.syncSchema` reads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU * test(driver-memory): guard the declared-index surface at the driver door too The field-level and declared-index constraints are wired through one resolved tenant column; the door-level pin covered only the field surface, so a fix that routed half of it would have stayed green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU * test(objectql): pin the tenancy declaration reaching the cold store The Archiver hands the cold driver the object it was given, and a driver resolves a uniqueness partition from `tenancy`. Pins the key on the published `LifecycleObjectLike` and on the `syncSchema` call in one assertion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6058cb2 commit 0f38ab0

9 files changed

Lines changed: 682 additions & 10 deletions

File tree

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
---
2+
"@objectstack/driver-memory": minor
3+
"@objectstack/driver-sql": patch
4+
"@objectstack/objectql": patch
5+
---
6+
7+
fix(driver-memory,driver-sql): an explicit `tenancy.enabled: false` opt-out is sticky, so a partial `syncSchema` re-registration no longer flips a platform-global object's UNIQUE partition (#16729)
8+
9+
## What was wrong
10+
11+
`InMemoryDriver.syncSchema` recomputed its uniqueness constraints from whatever
12+
schema THAT call happened to carry. A second registration without a `tenancy`
13+
block — the `{ name, fields }` shape — fell through to the implicit
14+
`organization_id` heuristic, so a `unique` field moved from **one row per
15+
install** (`scopeField: null`, which is what `tenancy.enabled: false` declares)
16+
to **one row per organization**. A duplicate the declaration refuses then
17+
landed. Measured at the driver door on `origin/main` `d61139f1ba`:
18+
19+
| sequence | second `key: 'K'`, different organization |
20+
|:--|:--|
21+
| register with `tenancy.enabled: false` | `REFUSED``UNIQUE_VIOLATION` / 409 |
22+
| …then re-register with `{ name, fields }` | **`LANDED`** |
23+
24+
`SqlDriver` running the same sequence refuses in **both** cases: it has kept a
25+
sticky `tenantOptOutByTable` since #3249. `driver-memory` had mirrored the inner
26+
`computeTenantField` and not the wrapper that consults the record, so "mirrors
27+
`computeTenantField` arm for arm" stayed literally true while the pair diverged.
28+
29+
It is silent in both directions — nothing logs the flip, and the refusal names
30+
the field, never the partition. That is the declared-vs-enforced shape Prime
31+
Directive #10 forbids, reached by a state change rather than by a missing check.
32+
33+
## What it does now
34+
35+
- **`@objectstack/driver-memory`** gains `computeAndRecordTenantField`, the
36+
sticky resolver, and the `TenantOptOutRecord` type for the per-instance record
37+
a driver owns. `InMemoryDriver` holds one and resolves through it, handing
38+
BOTH declaration surfaces — field-level `unique` and declared `indexes[]`
39+
the same resolved column. `uniqueConstraintsFromFields` and
40+
`uniqueConstraintsFromDeclaredIndexes` accept that column as an optional
41+
second argument; called with one argument they answer exactly as before.
42+
`tenantFieldOf` is unchanged and still a pure function of its argument.
43+
- **`@objectstack/driver-sql`**: the shard leaf resolved its tenant column with
44+
the BARE `computeTenantField`, so a `rotateShards` sweep carrying no `tenancy`
45+
block gave a shard an organization key part the base table's index does not
46+
have — one object, two partitions, decided by which physical table a row
47+
landed in. It now resolves through the record, keyed by the base table.
48+
- **`@objectstack/objectql`**: `LifecycleObjectLike` declares `tenancy`. The
49+
Archiver hands that object straight to `cold.syncSchema`, and the published
50+
type refused the key while the driver below read it — so an author writing a
51+
fresh literal was pushed into producing exactly the partial re-registration
52+
above. Same correction #16711 made where the shard leaf narrowed the key off
53+
the object it was handed.
54+
55+
The record is deliberately narrow. Only the explicit OPT-OUT is sticky: a
56+
declared `tenancy.tenantField` is not recorded, matching `SqlDriver`. An object
57+
that never declared the opt-out never enters the record, so a genuinely
58+
org-scoped object keeps its `organization_id` partition across a partial
59+
re-registration — an implementation answering `null` more often would not be
60+
stickier, it would be tenant isolation switched off. A carried `tenancy` block
61+
stays authoritative in both directions and CLEARS a recorded opt-out.
62+
63+
`@objectstack/driver-memory` is `minor` for the two new public-entry exports.
64+
The behaviour repairs themselves are `patch`: each restores an implementation to
65+
the `tenancy.enabled: false` contract (`isTenancyDisabled`, ADR-0066) it was
66+
already declaring, rather than replacing one legal published answer with
67+
another. The `objectql` entry is a published type WIDENING — a key the interface
68+
refused is now accepted, and nothing that compiled before stops compiling.

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,19 @@ export type { TenancyAwareSchema } from './memory-tenancy-guard.js';
3131
// identity and the scoping helpers, exported so a consumer can assert the
3232
// envelope (`code` AND `status`, never merely "it threw") without
3333
// string-matching the message.
34+
//
35+
// [#16729] `computeAndRecordTenantField` is published BESIDE `tenantFieldOf`,
36+
// not kept private, because publishing only the inner half is what let this
37+
// package diverge in the first place: `tenantFieldOf` mirrors
38+
// `SqlDriver.computeTenantField`, the stickiness lives in the wrapper AROUND
39+
// that function on the SQL side, and a reader who found only the inner half
40+
// exported mirrored only the inner half. The next driver reproducing this pair
41+
// sees both halves or repeats the same omission.
3442
export {
3543
UNIQUE_VIOLATION_CODE,
3644
UNIQUE_VIOLATION_STATUS,
3745
assertNoUniqueViolation,
46+
computeAndRecordTenantField,
3847
declaredIndexViolationError,
3948
isDeclaredIndexConstraint,
4049
tenantFieldOf,
@@ -48,6 +57,7 @@ export type {
4857
MemoryDeclaredIndexConstraint,
4958
MemoryUniqueConstraint,
5059
MemoryUniqueEnforcement,
60+
TenantOptOutRecord,
5161
UniqueAwareSchema,
5262
} from './memory-unique-constraint.js';
5363

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

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import {
4949
// module docblock.
5050
import {
5151
assertNoUniqueViolation,
52+
computeAndRecordTenantField,
5253
uniqueConstraintsFromDeclaredIndexes,
5354
uniqueConstraintsFromFields,
5455
type MemoryUniqueEnforcement,
@@ -404,6 +405,27 @@ export class InMemoryDriver implements IDataDriver {
404405
* the data it happens to hold.
405406
*/
406407
private uniqueConstraints: Map<string, MemoryUniqueEnforcement[]> = new Map();
408+
409+
/**
410+
* [#16729] Objects whose schema EXPLICITLY declared `tenancy.enabled: false`,
411+
* this driver's counterpart of `SqlDriver.tenantOptOutByTable` and the record
412+
* {@link computeAndRecordTenantField} maintains.
413+
*
414+
* Sticky across re-registrations on purpose: a later `syncSchema` that omits
415+
* the `tenancy` block must NOT resurrect org-scoping of the uniqueness key
416+
* via the implicit `organization_id` heuristic. Without it a platform-global
417+
* object's UNIQUE partition silently moved from one row per install to one
418+
* row per organization, and the duplicate its declaration refuses LANDED.
419+
*
420+
* Unlike {@link uniqueConstraints} it is deliberately NOT cleared by
421+
* `dropTable`. That map is cleared because a constraint outliving its table
422+
* would be ENFORCED over a table nobody declared; this record enforces
423+
* nothing on its own — it only decides which partition the NEXT declaration
424+
* resolves to, and the last authoritative word on this object was still
425+
* "platform-global". Dropping a table is not a schema declaring itself
426+
* tenant-scoped, and only such a declaration clears the record.
427+
*/
428+
private tenantOptOutByObject: Set<string> = new Set();
407429
private transactions: Map<string, MemoryTransaction> = new Map();
408430
private persistenceAdapter: PersistenceAdapterInterface | null = null;
409431

@@ -1978,9 +2000,18 @@ export class InMemoryDriver implements IDataDriver {
19782000
// an already-duplicated pair is reported by the first write that touches
19792001
// it — the same posture `driver-sql` takes when a unique index cannot be
19802002
// built over dirty data (it announces, it does not delete rows).
2003+
// [#16729] Resolve the tenant column through the STICKY record rather than
2004+
// from this call's schema alone. `syncSchema` is idempotent and is called
2005+
// again with whatever schema the caller happens to hold; a call carrying no
2006+
// `tenancy` block would otherwise fall through to the implicit
2007+
// `organization_id` heuristic and re-scope an object that declared itself
2008+
// platform-global. Both surfaces are handed the SAME resolved column, so
2009+
// the field-level and declared-index keys of one object cannot disagree
2010+
// about which partition it lives in.
2011+
const tenantField = computeAndRecordTenantField(this.tenantOptOutByObject, object, schema);
19812012
this.uniqueConstraints.set(object, [
1982-
...uniqueConstraintsFromFields(schema),
1983-
...uniqueConstraintsFromDeclaredIndexes(schema),
2013+
...uniqueConstraintsFromFields(schema, tenantField),
2014+
...uniqueConstraintsFromDeclaredIndexes(schema, tenantField),
19842015
]);
19852016
if (kinds.size > 0) {
19862017
const table = this.db[object];

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

Lines changed: 96 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -300,20 +300,105 @@ export function tenantFieldOf(schema: UniqueAwareSchema | null | undefined): str
300300
return null;
301301
}
302302

303+
/**
304+
* [#16729] One driver's record of the objects whose schema EXPLICITLY declared
305+
* `tenancy.enabled === false` — this package's counterpart of `SqlDriver`'s
306+
* `tenantOptOutByTable`.
307+
*
308+
* Owned by the DRIVER INSTANCE, never by this module. Two `InMemoryDriver`s in
309+
* one process are two independent stores (this driver's whole shape is
310+
* per-instance state), so a module-level record would let one store's
311+
* declaration decide another store's uniqueness partition — a fresh
312+
* cross-instance channel introduced by the fix for a cross-registration one.
313+
* `SqlDriver` holds its own record per instance for the same reason.
314+
*/
315+
export type TenantOptOutRecord = Set<string>;
316+
317+
/**
318+
* [#16729] {@link tenantFieldOf} + maintenance of the sticky explicit-opt-out
319+
* record. Mirrors `SqlDriver.computeAndRecordTenantField` arm for arm, and it
320+
* is the arm this package was missing.
321+
*
322+
* ## Why the mirror needed a SECOND function, not a change to the first
323+
*
324+
* `driver-memory` already reproduced `SqlDriver.computeTenantField` faithfully
325+
* as {@link tenantFieldOf}. The stickiness, though, does not live in that
326+
* function on the SQL side either: it lives in the WRAPPER around it. So the
327+
* inner half was mirrored and the outer half was not, and "this mirrors
328+
* `computeTenantField` arm for arm" stayed literally true while the pair as a
329+
* whole diverged. {@link tenantFieldOf} is therefore UNCHANGED and still
330+
* answers from the passed schema alone — a pure function of its argument is
331+
* what its own pins assert, and they remain correct.
332+
*
333+
* ## What the record buys
334+
*
335+
* A schema that carries a `tenancy` declaration is authoritative: it sets or
336+
* clears the record and is computed normally. A schema WITHOUT one — a partial
337+
* re-registration — preserves a previously declared opt-out instead of letting
338+
* the implicit `organization_id` heuristic re-scope a platform-global object.
339+
* Without it, a second `syncSchema` carrying only `{ name, fields }` silently
340+
* moves a `unique` field from ONE row per install (`scopeField: null`, which is
341+
* what `tenancy.enabled: false` declares) to one row per organization: a
342+
* duplicate the declaration refuses then LANDS, and nothing announces the
343+
* change — the declared-vs-enforced divergence Prime Directive #10 forbids,
344+
* reached by a state change rather than by a missing check.
345+
*
346+
* ## Only the OPT-OUT is sticky, deliberately
347+
*
348+
* A declared `tenancy.tenantField` is NOT recorded, so a partial
349+
* re-registration of a custom-tenant-column object still falls back to
350+
* `organization_id`. That is not an oversight: it is what `SqlDriver` does, and
351+
* this module's contract is to answer as `driver-sql` answers. Recording more
352+
* here would be a second, easier answer to "what does `unique` mean" — the
353+
* one-contract-two-numbers defect this module exists to close.
354+
*
355+
* A genuinely tenant-scoped object (no `tenancy` block, an `organization_id`
356+
* column) never enters the record, so it keeps its `organization_id` partition
357+
* across a partial re-registration exactly as before. An implementation that
358+
* answered `null` more often than this one would not be stickier, it would be
359+
* tenant isolation switched off.
360+
*/
361+
export function computeAndRecordTenantField(
362+
record: TenantOptOutRecord,
363+
key: string,
364+
schema: UniqueAwareSchema | null | undefined,
365+
): string | null {
366+
// A carried `tenancy` block is AUTHORITATIVE in both directions: it records a
367+
// fresh opt-out, and it CLEARS a stale one. `!= null` is the SQL side's test
368+
// — the declaration's PRESENCE is what makes it authoritative, so a
369+
// `tenancy: {}` that declares no opt-out clears the record too.
370+
if (schema?.tenancy != null) {
371+
if (isTenancyDisabled(schema)) record.add(key);
372+
else record.delete(key);
373+
return tenantFieldOf(schema);
374+
}
375+
if (record.has(key)) return null;
376+
return tenantFieldOf(schema);
377+
}
378+
303379
/**
304380
* The constraints an object's field-level `unique` declarations ask for.
305381
*
306382
* The single place a `unique` declaration becomes a constraint in this package,
307383
* so the create, update and update-many paths cannot disagree about what one
308384
* means — the same reason `uniqueIndexesFromFields` is the single place on the
309385
* SQL side.
386+
*
387+
* [#16729] `tenantField` is the RESOLVED tenant column. It defaults to
388+
* {@link tenantFieldOf} of this very schema, so the published one-argument call
389+
* answers exactly as before; a caller holding a {@link TenantOptOutRecord}
390+
* passes {@link computeAndRecordTenantField}'s answer instead, and a partial
391+
* re-registration then cannot re-scope an object that declared itself
392+
* platform-global. `driver-sql` threads the resolved column into
393+
* `syncDeclaredIndexes` rather than letting that leaf recompute it, for this
394+
* same reason.
310395
*/
311396
export function uniqueConstraintsFromFields(
312397
schema: UniqueAwareSchema | null | undefined,
398+
tenantField: string | null = tenantFieldOf(schema),
313399
): MemoryUniqueConstraint[] {
314400
const fields = schema?.fields;
315401
if (!fields) return [];
316-
const tenantField = tenantFieldOf(schema);
317402
const out: MemoryUniqueConstraint[] = [];
318403
for (const [name, field] of Object.entries(fields)) {
319404
const unique = (field as { unique?: unknown } | null | undefined)?.unique;
@@ -344,13 +429,22 @@ export function uniqueConstraintsFromFields(
344429
* `driver-memory` must not depend on `driver-sql`, so the arms are reproduced
345430
* and pinned here (`memory-declared-index-unique.test.ts`), the way
346431
* {@link tenantFieldOf} reproduces `SqlDriver.computeTenantField`.
432+
*
433+
* [#16729] `tenantField` is the RESOLVED tenant column. It defaults to
434+
* {@link tenantFieldOf} of this very schema, so the published one-argument call
435+
* answers exactly as before; a caller holding a {@link TenantOptOutRecord}
436+
* passes {@link computeAndRecordTenantField}'s answer instead, and a partial
437+
* re-registration then cannot re-scope an object that declared itself
438+
* platform-global. `driver-sql` threads the resolved column into
439+
* `syncDeclaredIndexes` rather than letting that leaf recompute it, for this
440+
* same reason.
347441
*/
348442
export function uniqueConstraintsFromDeclaredIndexes(
349443
schema: UniqueAwareSchema | null | undefined,
444+
tenantField: string | null = tenantFieldOf(schema),
350445
): MemoryDeclaredIndexConstraint[] {
351446
const declared = schema?.indexes;
352447
if (!Array.isArray(declared)) return [];
353-
const tenantField = tenantFieldOf(schema);
354448
const out: MemoryDeclaredIndexConstraint[] = [];
355449
for (const idx of declared) {
356450
// The same filter the SQL side applies, and nothing more: a non-string or

0 commit comments

Comments
 (0)