diff --git a/.changeset/registry-register-object-optional-package-id.md b/.changeset/registry-register-object-optional-package-id.md new file mode 100644 index 0000000000..85efa38c7a --- /dev/null +++ b/.changeset/registry-register-object-optional-package-id.md @@ -0,0 +1,53 @@ +--- +"@objectstack/objectql": minor +--- + +fix(objectql): widen `SchemaRegistry.registerObject`'s `packageId` to optional (#12623) + +**Public-API accept-set widening**, ruled by the maintainer (issue #12623 comment +5434929046, Option A) — shipped as `minor`: it is not a bug fix in behavior (the +underlying runtime path already treated a missing `packageId` as `undefined` +wherever no `tsc` program enforced the parameter's arity; see below), but it does +change the method's declared TypeScript contract, so it gets a real bump rather +than riding along as an implicit patch. + +`registerObject`'s second parameter, `packageId`, was `string` (required) while its +sibling `registerItem` already declared the identical parameter `packageId?: string` +(optional) — and both feed the same downstream call, +`applyProtection(item, { packageId })`, whose own comment documents the +package-less case as intended: *"bare `registerItem(type, item)` calls without a +package context still produce a clean item."* The mismatch made a supported shape +— `registry.registerObject(schema)` with no package context, used by 82 +single-argument call sites across `objectql`, `rest`, `runtime` and `plugins` — a +type error everywhere a `tsc` program actually read the call (14 sites in +`packages/rest`'s test layer, ledgered as `TS2554` against issue #5286; the other +68 sites had no gate reading them, so the error was latent rather than caught). + +**FROM → TO:** + +```ts +// FROM +registerObject(schema: ServiceObject, packageId: string, namespace?: string, ...): string + +// TO +registerObject(schema: ServiceObject, packageId?: string, namespace?: string, ...): string +``` + +No caller needs to change: every existing call already supplied `packageId` (or +relied on JS's lack of arity enforcement to omit it despite the stricter type), and +the runtime behavior for both cases is unchanged — `packageId?: string` carries +**no default value**. A bare call still passes `packageId: undefined` through to +`applyProtection`, which still leaves the registered item provenance-free (no +`_packageId`, no `_provenance`), exactly as it does today for every already-passing +call site. Pinned in +`packages/objectql/src/registry-register-object-optional-package-id.test.ts`, +which asserts on the registered item's key *absence* (not merely +`=== undefined`), paired with a positive control confirming a call that *does* +pass a `packageId` still gets provenance stamped. + +The exported `ObjectContributor` interface's `packageId` field widens from +`string` to `string | undefined` to match — the exact value `registerObject`'s +own parameter is called with, and the mechanically necessary consequence of the +widening above (`ObjectContributor.packageId` is that value's only home). + + diff --git a/packages/objectql/src/registry-register-object-optional-package-id.test.ts b/packages/objectql/src/registry-register-object-optional-package-id.test.ts new file mode 100644 index 0000000000..4bb1562fee --- /dev/null +++ b/packages/objectql/src/registry-register-object-optional-package-id.test.ts @@ -0,0 +1,63 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, beforeEach } from 'vitest'; +import { SchemaRegistry } from './registry'; +import type { ServiceObject } from '@objectstack/spec/data'; + +/** + * #12623 — `SchemaRegistry.registerObject`'s `packageId` parameter is + * optional (maintainer ruling, issue #12623 comment 5434929046, Option A), + * matching the sibling `registerItem` and the behaviour `applyProtection` + * (`packages/spec/src/shared/protection.zod.ts`) already documents as + * intended: "bare `registerItem(type, item)` calls without a package + * context still produce a clean item." + * + * The risk the ruling names by hand is NOT "packageId required" — it is + * "packageId optional WITH A DEFAULT", the way the engine facade defaults + * to `'__runtime__'` (`engine.ts`, deliberately out of scope here). Because + * `applyProtection` runs UNCONDITIONALLY on every `registerObject` call + * (`registry.ts`, `applyProtection(schema as any, { packageId })`), a + * quietly-defaulted `packageId` would stamp `_packageId` and + * `_provenance: 'package'` onto every bare-call registration — exactly the + * fixtures the helper's own comment says must stay clean — silently, at + * every one of the 82 single-argument call sites this repo carries. + * + * This pin is the whole test for that distinction. It asserts on KEY + * PRESENCE via `hasOwnProperty`, not `=== undefined`: a default that + * resolves to `undefined` at call time but still assigns the key (e.g. + * `applyProtection`'s own `_provenance = ctx.provenance ?? 'package'` + * pattern, misapplied one layer up) must still fail this pin, which + * `=== undefined` alone would miss. + */ +describe('SchemaRegistry.registerObject — optional packageId, no default (#12623)', () => { + let registry: SchemaRegistry; + beforeEach(() => { + registry = new SchemaRegistry({ multiTenant: false }); + }); + + it('a bare registerObject(schema) call — no packageId — produces a provenance-free item', () => { + const obj: ServiceObject = { name: 'bare_fixture', fields: { name: { type: 'text' } } } as ServiceObject; + + registry.registerObject(obj); + + const resolved = registry.getObject('bare_fixture'); + expect(resolved).toBeDefined(); + + // hasOwnProperty, not `=== undefined` — see file header. + expect(Object.prototype.hasOwnProperty.call(resolved, '_packageId')).toBe(false); + expect(Object.prototype.hasOwnProperty.call(resolved, '_provenance')).toBe(false); + }); + + it('positive control: registerObject(schema, packageId) DOES stamp provenance', () => { + const obj: ServiceObject = { name: 'packaged_fixture', fields: { name: { type: 'text' } } } as ServiceObject; + + registry.registerObject(obj, 'com.example.pkg'); + + const resolved = registry.getObject('packaged_fixture'); + expect(resolved).toBeDefined(); + expect(Object.prototype.hasOwnProperty.call(resolved, '_packageId')).toBe(true); + expect(Object.prototype.hasOwnProperty.call(resolved, '_provenance')).toBe(true); + expect((resolved as any)._packageId).toBe('com.example.pkg'); + expect((resolved as any)._provenance).toBe('package'); + }); +}); diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts index e931dfd694..01d752d5b1 100644 --- a/packages/objectql/src/registry.ts +++ b/packages/objectql/src/registry.ts @@ -72,7 +72,12 @@ export const DEFAULT_EXTENDER_PRIORITY = 200; * whichever layer is the base. */ export interface ObjectContributor { - packageId: string; + // Optional (#12623): `registerObject`'s own `packageId` parameter is now + // optional, and this is the exact value it was called with — a bare + // `registerObject(schema)` call stores `undefined` here, matching the + // runtime behavior every call site without a tsc test-layer gate already + // exercised (JS does not enforce TS arity). + packageId: string | undefined; namespace: string; ownership: ObjectOwnership; priority: number; @@ -1567,7 +1572,12 @@ export class SchemaRegistry { * @param schema - The object definition * @param packageId - The owning package ID, or — for an `overlay` — the * `sys_metadata` row's own binding (provenance ON the layer, never an - * ownership claim; ADR-0029 D9.9) + * ownership claim; ADR-0029 D9.9). Optional (#12623) — matches the + * sibling {@link registerItem}: a bare `registerObject(schema)` call + * passes `packageId: undefined` through to `applyProtection`, which + * leaves the item clean (no `_packageId`, no `_provenance`) rather than + * defaulting. Do not give this parameter a default value — that would + * silently stamp package provenance onto every bare-call fixture. * @param namespace - The package namespace (for FQN computation) * @param ownership - 'own' (single owner) | 'overlay' (tenant layer that * REPLACES the base at resolution; ADR-0029 D9) | 'extend' (additive merge) @@ -1577,7 +1587,7 @@ export class SchemaRegistry { */ registerObject( schema: ServiceObject, - packageId: string, + packageId?: string, namespace?: string, ownership: ObjectOwnership = 'own', priority: number = ownership === 'own' @@ -1640,8 +1650,12 @@ export class SchemaRegistry { const shortName = schema.name; const fqn = computeFQN(namespace, shortName); - // Ensure namespace is registered - if (namespace) { + // Ensure namespace is registered. [#12623] `packageId` is now optional + // on this method, so a namespace passed without one has no owner to + // record — skip rather than registering `undefined` as an owner (which + // would also mean widening `registerNamespace`'s own required-string + // contract, out of scope here). + if (namespace && packageId) { this.registerNamespace(namespace, packageId); } diff --git a/packages/rest/test-typecheck-debt.json b/packages/rest/test-typecheck-debt.json index 685eb62539..994d1cbb40 100644 --- a/packages/rest/test-typecheck-debt.json +++ b/packages/rest/test-typecheck-debt.json @@ -1,14 +1,8 @@ { "_comment": "Per-file tsc error debt of the @objectstack/rest TEST layer (#5286). `tsconfig.test.json` compiles `src/**/*.test.ts` — which `tsconfig.json` excludes and therefore no gate ever read — and every file below still carries errors from before that gate existed, almost all of them fixture literals annotated with a schema OUTPUT type (`z.infer`) while holding an authored INPUT literal. EXACT ratchet, judged by re-running tsc: a file that gains errors is red, a file that loses them is red until its number is re-recorded, a file that reaches zero is red until its entry is deleted, and a file NOT listed here may have no errors at all. Regenerate with: pnpm --filter @objectstack/rest gen:test-typecheck-debt", "entries": { - "src/export-integration.test.ts": 4, - "src/import-dryrun-parity.test.ts": 1, - "src/import-integration.test.ts": 3, - "src/import-job-integration.test.ts": 2, "src/meta-public-book-grant.test.ts": 1, "src/rest-batch-size-cap.test.ts": 1, - "src/rest-meta-save-receipt-envelope.test.ts": 3, - "src/rest-write-response-formula.test.ts": 1, "src/rest.test.ts": 4 } }