Skip to content

Commit 48b0fcf

Browse files
claude[bot]claude
andauthored
feat(objectql): publish SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE and isSystemWriteOrganizationRequiredError so consumers stop re-spelling the literal (#16156)
* feat(objectql): publish SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE and isSystemWriteOrganizationRequiredError (#14936) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ * test(objectql): pin the recognizer across the realm split, with its discriminating controls (#14936) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ * test(objectql): derive the wrong-case fixture from the published constant instead of spelling it (#14936) A lowercase error-code literal in a `code` position is an ADR-0112 D1 finding, and `check:error-code-casing` cannot tell a negative fixture from a real emission - it classified this site as `(emission)`. Deriving the value from SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE is not an opt-out: the gate's own output records that a code value with no literal at the position is out of reach for its patterns by construction. No `adr0112-ok:` suppression was added (the count stays 17) and KNOWN_LOWERCASE_CODES is untouched. It also makes the fixture track the constant rather than restate it - this card's own argument about consumers re-spelling literals, applied to its own test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 53cf263 commit 48b0fcf

4 files changed

Lines changed: 232 additions & 2 deletions

File tree

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
"@objectstack/objectql": minor
3+
---
4+
5+
`@objectstack/objectql` now publishes a recognizer for the org-less system-write refusal, so a consumer no longer has to choose between an unsound check and a re-spelled string.
6+
7+
`SystemWriteOrganizationRequiredError` has always documented that it is identified by `code` rather than `instanceof`, "so the check survives crossing a package boundary where two copies of this module can exist". The convention was correct; the affordance for following it was missing. This package declares **both** realms in its own `exports``import` to `dist/index.mjs`, `require` to `dist/index.js` — so a consumer that loads it through the other realm than the engine did holds a second copy of the module. Measured across that split from a real consumer package: same class identity (`A === B`) **false**, `instA instanceof A` within one realm **true**, `instA instanceof B` across the two **false**, and a `code` compare **true**. So `instanceof` against this class was unsound for every consumer, and it failed silently — a `catch` that simply never fires.
8+
9+
That left a consumer with one sound option: re-spelling `'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED'` as a literal. That spelling is what `check:error-code-provenance` counts as a stamp site, so recognising one engine refusal cost the consumer's package a provenance decision of its own, and left the string spelled in two places with the typo failure mode standing — a typo in a `catch` produces a branch that never fires rather than an error.
10+
11+
Two new exports close it, both from the package root:
12+
13+
- **`SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE`** — the code as a value. Same shape as this package's five existing published codes (`DUPLICATE_RECORD_CODE`, `HOOK_TARGET_REBIND_ERROR_CODE`, `HOOK_UNSCOPED_DATA_ACCESS_CODE`, `MULTI_UPDATE_HOOK_KEY_DIVERGENCE_CODE`, `EMPTY_CREDENTIAL_REFUSAL_CODE`) rather than a new abstraction. The class field now reads from it, so exactly one spelling of the string remains in the package and a typo at an import site is a compile error instead of a dead branch.
14+
- **`isSystemWriteOrganizationRequiredError(err): boolean`** — the code compare itself, so a consumer performs the sound check without authoring the string at all.
15+
16+
The predicate deliberately returns `boolean` and does **not** narrow to `err is SystemWriteOrganizationRequiredError`. A `code` compare is satisfied by any value carrying that code, including an envelope a transport rebuilt from the wire — #5437 withholds the prose and keeps the machine-readable code — so a type guard would promise `object`, `posture` and `reason` members such a value need not have, moving the unsoundness one layer down instead of removing it.
17+
18+
⛔ Nothing about the refusal itself changes: not its `code`, not its 500 status, not when it fires, and not the #8844 `derive-or-refuse` ruling behind it. `SystemWriteOrganizationRequiredError['code']` stays the literal type it was, which is what the existing cross-package consumer types its own constant from. This is purely an addition to what the package publishes.

packages/objectql/src/index.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,12 +372,23 @@ export type {
372372
// #8686's ruling. The refusal class is exported because a caller that catches
373373
// it identifies it by `code`, and the decision function because it is the
374374
// ruling's five binding points as one pure, directly-testable verdict.
375+
//
376+
// [#14936] `SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE` and
377+
// `isSystemWriteOrganizationRequiredError` are the AFFORDANCE that makes "by
378+
// `code`" followable without re-spelling the literal. Exporting the class was
379+
// never enough on its own: this package declares both realms in its `exports`,
380+
// so a consumer holding the other realm's copy gets `instanceof` === false,
381+
// silently. The code compare is the check that survives; these two are how a
382+
// consumer performs it without authoring the string itself, and so without
383+
// acquiring a `check:error-code-provenance` stamp site of its own.
375384
export {
376385
resolveSystemWriteOrganization,
377386
resolveTenantFieldName,
378387
isPlatformNamespaceObject,
379388
carriesOrganization,
389+
isSystemWriteOrganizationRequiredError,
380390
SystemWriteOrganizationRequiredError,
391+
SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE,
381392
ORGANIZATION_OBJECT,
382393
GLOBAL_TENANT,
383394
DEFAULT_TENANT_FIELD,

packages/objectql/src/system-write-organization.test.ts

Lines changed: 138 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,12 @@
3838
import { describe, it, expect, vi, afterEach } from 'vitest';
3939
import type { ExecutionContext } from '@objectstack/spec/kernel';
4040
import { ObjectQL } from './engine.js';
41-
import { resolveSystemWriteOrganization } from './tenancy/system-write-organization.js';
41+
import {
42+
resolveSystemWriteOrganization,
43+
isSystemWriteOrganizationRequiredError,
44+
SystemWriteOrganizationRequiredError,
45+
SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE,
46+
} from './tenancy/system-write-organization.js';
4247

4348
const ORG_ID = 'org_msokm9oaz0cal87q';
4449
const SECOND_ORG_ID = 'org_second';
@@ -371,3 +376,135 @@ describe('#8844 the exclusions — populations the refusal must not touch', () =
371376
expect(observed.filter((c) => c.object === 'sys_organization')).toEqual([]);
372377
});
373378
});
379+
380+
381+
// ── [#14936] The published recognizer ────────────────────────────────────────
382+
//
383+
// The card's measurement, taken from a real consumer package loading
384+
// `@objectstack/objectql` through each of the two realms its own `exports`
385+
// declares (`import` -> `dist/index.mjs`, `require` -> `dist/index.js`):
386+
//
387+
// SAME CLASS IDENTITY (A === B): false
388+
// instA instanceof A (same realm): true
389+
// instA instanceof B (CROSS-REALM): false
390+
// code compare survives the split: true
391+
//
392+
// So a consumer had exactly two options and both were bad: `instanceof`, which
393+
// is unsound across that split and fails SILENTLY, or re-spelling
394+
// `'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED'`, which the provenance gate counts
395+
// as a stamp site in the consumer's own package and which can drift from what
396+
// the engine throws. These pins cover the third option this card publishes.
397+
//
398+
// Each case below carries its DISCRIMINATING CONTROL, for the reason this
399+
// file's header already states: a suite that only asserted "the recognizer
400+
// says true" would stay green if the recognizer were `() => true`, and one
401+
// that only asserted the same-realm instance would stay green if the
402+
// recognizer were `instanceof`-based - which is the very defect being fixed.
403+
404+
/**
405+
* What a SECOND copy of this module produces: structurally the refusal,
406+
* nominally a different class. This is the CJS build's class arriving at a
407+
* consumer holding the ESM one (or the reverse) - the exact shape the card's
408+
* cross-realm measurement found, reproduced here without needing two builds.
409+
*/
410+
class SystemWriteOrganizationRequiredErrorOtherRealmCopy extends Error {
411+
readonly code = 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED' as const;
412+
readonly status = 500;
413+
constructor() {
414+
super('refused by the other realm\'s copy of this module');
415+
this.name = 'SystemWriteOrganizationRequiredError';
416+
}
417+
}
418+
419+
describe('#14936 the published recognizer for the org-less system-write refusal', () => {
420+
it('the published constant IS the code the thrown refusal carries, at its 500 status', () => {
421+
const err = new SystemWriteOrganizationRequiredError('dispatch_order', 'isolated', 'walled-posture');
422+
expect(err.code).toBe(SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE);
423+
// ADR-0112 envelope: `code` AND `status`. Asserting the throw alone would
424+
// stay green against an unrelated failure, and would not notice the status
425+
// moving off 500 - which #8844 ruled deliberately.
426+
expect(err.status).toBe(500);
427+
// The wire string, spelled once here on purpose: this is the TEST layer,
428+
// which `check:error-code-provenance` does not scan, so pinning it costs no
429+
// stamp site while making a silent rename of the constant impossible to
430+
// pass off as "still the same code".
431+
expect(SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE).toBe('ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED');
432+
});
433+
434+
it('recognises the refusal the engine actually throws', () => {
435+
const err = new SystemWriteOrganizationRequiredError(
436+
'dispatch_order', 'single', 'ambiguous-organization', 2,
437+
);
438+
expect(isSystemWriteOrganizationRequiredError(err)).toBe(true);
439+
});
440+
441+
it("recognises the OTHER realm's copy - the exact case `instanceof` gets wrong", () => {
442+
const fromOtherRealm = new SystemWriteOrganizationRequiredErrorOtherRealmCopy();
443+
// THE CONTROL, and the whole point of the card. Without this line the
444+
// assertion below would pass just as happily against an `instanceof`
445+
// implementation, i.e. against the defect.
446+
expect(fromOtherRealm instanceof SystemWriteOrganizationRequiredError).toBe(false);
447+
expect(isSystemWriteOrganizationRequiredError(fromOtherRealm)).toBe(true);
448+
});
449+
450+
it('recognises a transport-rebuilt envelope, which is WHY it does not narrow to the class', () => {
451+
// #5437 withholds the prose from the wire and keeps the machine-readable
452+
// `code`, so what a consumer catches downstream of a transport can be an
453+
// envelope carrying the code and nothing else.
454+
const wireEnvelope: Record<string, unknown> = {
455+
code: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED', status: 500,
456+
};
457+
expect(isSystemWriteOrganizationRequiredError(wireEnvelope)).toBe(true);
458+
// ...and it carries none of the class's own members. A type guard
459+
// (`err is SystemWriteOrganizationRequiredError`) would promise these,
460+
// turning a sound check into an unsound assertion one layer down - which
461+
// is why the predicate returns `boolean`.
462+
expect(wireEnvelope.object).toBeUndefined();
463+
expect(wireEnvelope.posture).toBeUndefined();
464+
expect(wireEnvelope.reason).toBeUndefined();
465+
});
466+
467+
it.each([
468+
['null', null],
469+
['undefined', undefined],
470+
['a bare string carrying the code', 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED'],
471+
['an Error with no code at all', new Error('boom')],
472+
['a DIFFERENT engine refusal', Object.assign(new Error('dup'), { code: 'DUPLICATE_RECORD' })],
473+
['a prefix lookalike', Object.assign(new Error('x'), { code: 'ERR_SYSTEM_WRITE_ORGANIZATION' })],
474+
['a suffix lookalike', Object.assign(new Error('x'), { code: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED_V2' })],
475+
// DERIVED from the constant, never spelled. A lowercase literal in a `code`
476+
// position is a real `check:error-code-casing` finding (ADR-0112 D1), and the
477+
// gate cannot tell a negative fixture from a real emission - it classified this
478+
// very site as `(emission)`. Deriving it is not an opt-out: the gate's own
479+
// output says a code value with NO literal at the position is out of reach for
480+
// its patterns BY CONSTRUCTION. It also makes the fixture track the constant
481+
// instead of restating it - the same argument this card makes about consumers
482+
// re-spelling literals, applied to its own test.
483+
['the code in the wrong case', Object.assign(new Error('x'), {
484+
code: SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE.toLowerCase(),
485+
})],
486+
])('refuses %s', (_label, value) => {
487+
expect(isSystemWriteOrganizationRequiredError(value)).toBe(false);
488+
});
489+
490+
it('keeps `code` a LITERAL type, which is what the cross-package consumer types itself from', () => {
491+
// `plugin-sharing/src/sharing-rule-service.ts` declares its own constant as
492+
// `SystemWriteOrganizationRequiredError['code']`. Had this refactor widened
493+
// the class field to `string`, that consumer would keep COMPILING while
494+
// silently losing the drift protection it asked for - so the widening is
495+
// pinned as a TYPE error rather than a value assertion. This file carries no
496+
// `test-typecheck-debt.json` entry, so a new error here is red on arrival.
497+
const pinned: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED' =
498+
new SystemWriteOrganizationRequiredError('dispatch_order', 'isolated', 'walled-posture').code;
499+
expect(pinned).toBe(SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE);
500+
});
501+
502+
it('publishes both names from the package BARREL, not only from the module', async () => {
503+
// The card's landing surface is "the module plus that package's index.ts
504+
// export" - a consumer reaches these by bare specifier, so an export that
505+
// exists only on the deep module is not the affordance that was asked for.
506+
const barrel = await import('./index.js');
507+
expect(barrel.SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE).toBe(SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE);
508+
expect(barrel.isSystemWriteOrganizationRequiredError).toBe(isSystemWriteOrganizationRequiredError);
509+
});
510+
});

packages/objectql/src/tenancy/system-write-organization.ts

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,42 @@ function buildRefusalMessage(
311311
);
312312
}
313313

314+
/**
315+
* The refusal's machine-readable code, published as a VALUE so a consumer can
316+
* recognise the refusal without re-spelling the literal (#14936).
317+
*
318+
* The class below mandates `code` over `instanceof`, and the measurement
319+
* behind that mandate is why this constant exists rather than staying implicit
320+
* in the class field: `@objectstack/objectql` declares BOTH realms in its own
321+
* `exports` (`import` -> `dist/index.mjs`, `require` -> `dist/index.js`), so a
322+
* consumer that loads this package through the other realm than the engine did
323+
* holds a SECOND copy of this module. Measured across that split from a real
324+
* consumer package:
325+
*
326+
* SAME CLASS IDENTITY (A === B): false
327+
* instA instanceof A (same realm): true
328+
* instA instanceof B (CROSS-REALM): false
329+
* code compare survives the split: true
330+
*
331+
* so `instanceof` against this class is unsound for every consumer and fails
332+
* SILENTLY - a `catch` that simply never fires.
333+
*
334+
* Deliberately the same shape as this package's five other published codes
335+
* (`DUPLICATE_RECORD_CODE`, `HOOK_TARGET_REBIND_ERROR_CODE`,
336+
* `HOOK_UNSCOPED_DATA_ACCESS_CODE`, `MULTI_UPDATE_HOOK_KEY_DIVERGENCE_CODE`,
337+
* `EMPTY_CREDENTIAL_REFUSAL_CODE`) rather than a new abstraction. The `*_CODE`
338+
* NAME is load-bearing, not cosmetic: it is the shape
339+
* `check:error-code-provenance`'s `constdef` pattern can see, so the one
340+
* remaining spelling of this string is a stamp site the ledger accounts for
341+
* under this package's own owner key - where
342+
* `ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` is already registered (#8844).
343+
* ⛔ Never rename it out of that shape to quiet the gate: a spelling the
344+
* gate cannot see is the failure mode the gate exists to catch, not a clean
345+
* result.
346+
*/
347+
export const SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE =
348+
'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED' as const;
349+
314350
/**
315351
* Binding point 2's refusal.
316352
*
@@ -326,7 +362,7 @@ function buildRefusalMessage(
326362
* through the engine's own ERROR log.
327363
*/
328364
export class SystemWriteOrganizationRequiredError extends Error {
329-
readonly code = 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED' as const;
365+
readonly code = SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE;
330366
readonly status = 500;
331367

332368
constructor(
@@ -339,3 +375,31 @@ export class SystemWriteOrganizationRequiredError extends Error {
339375
this.name = 'SystemWriteOrganizationRequiredError';
340376
}
341377
}
378+
379+
/**
380+
* Does `err` carry this module's refusal (#14936)?
381+
*
382+
* The recognizer a consumer should reach for instead of the two options it
383+
* otherwise has: `instanceof`, which the measurement on
384+
* {@link SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE} shows is unsound across the
385+
* dual build, or a re-spelled string literal, which acquires a stamp site in
386+
* the consumer's own package and can drift from what the engine throws. A
387+
* `code` compare is the only one of the three that survives the realm split,
388+
* and it is the convention this class's own docblock already mandates.
389+
*
390+
* ⛔ Deliberately returns `boolean` and does NOT narrow to
391+
* `err is SystemWriteOrganizationRequiredError`. A `code` compare is satisfied
392+
* by ANY value carrying that `code` - including an envelope a transport
393+
* rebuilt from the wire, which keeps the machine-readable `code` and drops
394+
* everything else - so a type guard would promise `object`, `posture` and
395+
* `reason` members such a value need not have, turning a sound check into an
396+
* unsound assertion one layer down. Read those fields off the caught value
397+
* only after checking for them; `code` is what this predicate guarantees.
398+
*/
399+
export function isSystemWriteOrganizationRequiredError(err: unknown): boolean {
400+
return (
401+
typeof err === 'object'
402+
&& err !== null
403+
&& (err as { code?: unknown }).code === SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE
404+
);
405+
}

0 commit comments

Comments
 (0)