fix(objectql): relax registerObject packageId to optional (#12623)

.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).
+
+<!-- adr-0087: not-required (no-migration-prescription) A pure accept-set widening — no key, export or shape is removed, renamed or narrowed, so there is no tombstone and nothing for `objectstack migrate meta` to rewrite. Every existing caller keeps compiling and behaving identically; the only new capability is that a previously-latent-error call shape now type-checks. -->

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');
+ });
+});

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);
  }
 

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
  }
 }